Thread (10 messages) 10 messages, 2 authors, 2018-12-03

[PATCH v6 2/2] media: platform: Add Aspeed Video Engine driver

From: Hans Verkuil <hidden>
Date: 2018-12-03 21:09:43
Also in: linux-devicetree, linux-media, lkml

On 12/03/2018 09:37 PM, Eddie James wrote:

<snip>
quoted
quoted
quoted
quoted
+static int aspeed_video_start(struct aspeed_video *video)
+{
+	int rc;
+
+	aspeed_video_on(video);
+
+	aspeed_video_init_regs(video);
+
+	rc = aspeed_video_get_resolution(video);
+	if (rc)
+		return rc;
+
+	/*
+	 * Set the timings here since the device was just opened for the first
+	 * time.
+	 */
+	video->active_timings = video->detected_timings;
What happens if no valid signal was detected?

My recommendation is to fallback to some default timings (VGA?) if no valid
initial timings were found.

The expectation is that applications will always call QUERY_DV_TIMINGS first,
so it is really not all that important what the initial active_timings are,
as long as they are valid timings (valid as in: something that the hardware
can support).
See just above, this call returns with a failure if no signal is
detected, meaning the device cannot be opened. The only valid timings
are the detected timings.
That's wrong. You must ALWAYS be able to open the device. If not valid
resolution is detected, just fallback to some default.
Why must open always succeed? What use is a video device that cannot 
provide any video?
You always must be able to open the video device so applications can call
QUERYCAP. In fact, any ioctl that returns state information (G_FMT, G_CTRL,
G_INPUT, ENUM_*, etc) can always be called, regardless of whether there is
a video signal or if video streaming is in progress.

With this restriction I cannot even run an application that waits for the
SOURCE_CHANGE event to start streaming, such as 'v4l2-ctl --stream-mmap'
does because the open() will fail immediately.

Sorry, this is really wrong.

Regards,

	Hans
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help