From: Hans Verkuil <hverkuil@xs4all.nl>
To: Eddie James <eajames@linux.ibm.com>, linux-kernel@vger.kernel.org
Cc: mark.rutland@arm.com, devicetree@vger.kernel.org,
linux-aspeed@lists.ozlabs.org, robh+dt@kernel.org,
mchehab@kernel.org, linux-media@vger.kernel.org
Subject: Re: [PATCH v6 2/2] media: platform: Add Aspeed Video Engine driver
Date: Mon, 3 Dec 2018 22:09:35 +0100 [thread overview]
Message-ID: <8b1b1f60-d21c-c0a2-2597-8bdb77f5f7c9@xs4all.nl> (raw)
In-Reply-To: <6c92cb5c-2134-e5d8-1a6b-2d311731cef3@linux.ibm.com>
On 12/03/2018 09:37 PM, Eddie James wrote:
<snip>
>>>>> +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
next prev parent reply other threads:[~2018-12-03 21:09 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-11-27 19:37 [PATCH v6 0/2] media: platform: Add Aspeed Video Engine driver Eddie James
2018-11-27 19:37 ` [PATCH v6 1/2] dt-bindings: media: Add Aspeed Video Engine binding documentation Eddie James
2018-11-27 19:37 ` [PATCH v6 2/2] media: platform: Add Aspeed Video Engine driver Eddie James
2018-12-03 11:04 ` Hans Verkuil
2018-12-03 15:43 ` Hans Verkuil
2018-12-03 15:43 ` Hans Verkuil
2018-12-03 16:39 ` Eddie James
2018-12-03 20:14 ` Hans Verkuil
2018-12-03 20:37 ` Eddie James
2018-12-03 21:09 ` Hans Verkuil [this message]
2018-12-03 10:17 ` [PATCH v6 0/2] " Hans Verkuil
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=8b1b1f60-d21c-c0a2-2597-8bdb77f5f7c9@xs4all.nl \
--to=hverkuil@xs4all.nl \
--cc=devicetree@vger.kernel.org \
--cc=eajames@linux.ibm.com \
--cc=linux-aspeed@lists.ozlabs.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-media@vger.kernel.org \
--cc=mark.rutland@arm.com \
--cc=mchehab@kernel.org \
--cc=robh+dt@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).