[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