On 07/05/2016 08:56 AM, Florian Echtler wrote:
> Hello Hans,
>
> On 05.07.2016 08:41, Hans Verkuil wrote:
>> On 05/31/2016 10:15 PM, Florian Echtler wrote:
>>> The device hardware is always running at 60 FPS, so report this both via
>>> PARM_IOCTL and ENUM_FRAMEINTERVALS.
>>>
>>> Signed-off-by: Martin Kaltenbrunner <modin@xxxxxxx>
>>> Signed-off-by: Florian Echtler <floe@xxxxxxxxxxxxxx>
>>> ---
>>> drivers/input/touchscreen/sur40.c | 20 ++++++++++++++++++--
>>> 1 file changed, 18 insertions(+), 2 deletions(-)
>>>
>>> @@ -880,6 +893,9 @@ static const struct v4l2_ioctl_ops sur40_video_ioctl_ops = {
>>> .vidioc_enum_framesizes = sur40_vidioc_enum_framesizes,
>>> .vidioc_enum_frameintervals = sur40_vidioc_enum_frameintervals,
>>>
>>> + .vidioc_g_parm = sur40_ioctl_parm,
>>> + .vidioc_s_parm = sur40_ioctl_parm,
>>
>> Why is s_parm added when you can't change the framerate?
>
> Oh, I thought it's mandatory to always have s_parm if you have g_parm
> (even if it always returns the same values).
>
>> Same questions for the
>> enum_frameintervals function: it doesn't hurt to have it, but if there is only
>> one unchangeable framerate, then it doesn't make much sense.
>
> If you don't have enum_frameintervals, how would you find out about the
> framerate otherwise? Is g_parm itself enough already for all userspace
> tools?
It should be. The enum_frameintervals function is much newer than g_parm.
Frankly, I have the same problem with enum_framesizes: it reports only one
size. These two enum ioctls are normally only implemented if there are at
least two choices. If there is no choice, then G_FMT will return the size
and G_PARM the framerate and there is no need to enumerate anything.
The problem is that the spec is ambiguous as to the requirements if there
is only one choice for size and interval. Are the enum ioctls allowed in
that case? Personally I think there is nothing against that. But should
S_PARM also be allowed even though it can't actually change the frameperiod?
Don't bother making changes yet, let me think about this for a bit.
Regards,
Hans
>
>> Sorry, missed this when I reviewed this the first time around.
>
> No problem.
>
> Best, Florian
>
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
