diff mbox

[v2,2/2] v4l: Add support for STD ioctls on subdev nodes

Message ID 20180517143016.13501-3-niklas.soderlund+renesas@ragnatech.se (mailing list archive)
State New, archived
Headers show

Commit Message

Niklas Söderlund May 17, 2018, 2:30 p.m. UTC
There is no way to control the standard of subdevices which are part of
a media device. The ioctls which exists all target video devices
explicitly and the idea is that the video device should talk to the
subdevice. For subdevices part of a media graph this is not possible and
the standard must be controlled on the subdev device directly.

Add four new ioctls to be able to directly interact with subdevices and
control the video standard; VIDIOC_SUBDEV_ENUMSTD, VIDIOC_SUBDEV_G_STD,
VIDIOC_SUBDEV_S_STD and VIDIOC_SUBDEV_QUERYSTD.

Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>

---

* Changes since v1
- Added VIDIOC_SUBDEV_ENUMSTD.
---
 .../media/uapi/v4l/vidioc-enumstd.rst         | 11 ++++++----
 Documentation/media/uapi/v4l/vidioc-g-std.rst | 14 ++++++++----
 .../media/uapi/v4l/vidioc-querystd.rst        | 11 ++++++----
 drivers/media/v4l2-core/v4l2-subdev.c         | 22 +++++++++++++++++++
 include/uapi/linux/v4l2-subdev.h              |  4 ++++
 5 files changed, 50 insertions(+), 12 deletions(-)

Comments

Mauro Carvalho Chehab June 28, 2018, 11:37 a.m. UTC | #1
Em Thu, 17 May 2018 16:30:16 +0200
Niklas Söderlund         <niklas.soderlund+renesas@ragnatech.se> escreveu:

> There is no way to control the standard of subdevices which are part of
> a media device. The ioctls which exists all target video devices
> explicitly and the idea is that the video device should talk to the
> subdevice. For subdevices part of a media graph this is not possible and
> the standard must be controlled on the subdev device directly.

Why isn't it possible? A media pipeline should have at least a video
devnode where the standard ioctls will be issued.

So, I don't see why you would need to explicitly set the standard inside
a sub-device.

The way I see, inside a given pipeline, all subdevs should be using the
same video standard (maybe except for a m2m device with would have some
coded that would be doing format conversion).

Am I missing something?

Thanks,
Mauro
Hans Verkuil June 28, 2018, 12:47 p.m. UTC | #2
On 06/28/18 13:37, Mauro Carvalho Chehab wrote:
> Em Thu, 17 May 2018 16:30:16 +0200
> Niklas Söderlund         <niklas.soderlund+renesas@ragnatech.se> escreveu:
> 
>> There is no way to control the standard of subdevices which are part of
>> a media device. The ioctls which exists all target video devices
>> explicitly and the idea is that the video device should talk to the
>> subdevice. For subdevices part of a media graph this is not possible and
>> the standard must be controlled on the subdev device directly.
> 
> Why isn't it possible? A media pipeline should have at least a video
> devnode where the standard ioctls will be issued.

Not for an MC-centric device like the r-car or imx. It's why we have v4l-subdev
ioctls for the DV_TIMINGS API, but the corresponding SDTV standards API is
missing.

And in a complex scenario there is nothing preventing you from having multiple
SDTV inputs, some of which need PAL-BG, some SECAM, some NTSC (less likely)
which are all composed together (think security cameras or something like that).

You definitely cannot set the standard from a video device. If nothing else,
it would be completely inconsistent with how HDMI inputs work.

The whole point of MC centric devices is that you *don't* control subdevs
from video nodes.

Regards,

	Hans

> So, I don't see why you would need to explicitly set the standard inside
> a sub-device.
> 
> The way I see, inside a given pipeline, all subdevs should be using the
> same video standard (maybe except for a m2m device with would have some
> coded that would be doing format conversion).
> 
> Am I missing something?
> 
> Thanks,
> Mauro
>
Mauro Carvalho Chehab June 29, 2018, 10:06 a.m. UTC | #3
Em Thu, 28 Jun 2018 14:47:05 +0200
Hans Verkuil <hverkuil@xs4all.nl> escreveu:

> On 06/28/18 13:37, Mauro Carvalho Chehab wrote:
> > Em Thu, 17 May 2018 16:30:16 +0200
> > Niklas Söderlund         <niklas.soderlund+renesas@ragnatech.se> escreveu:
> >   
> >> There is no way to control the standard of subdevices which are part of
> >> a media device. The ioctls which exists all target video devices
> >> explicitly and the idea is that the video device should talk to the
> >> subdevice. For subdevices part of a media graph this is not possible and
> >> the standard must be controlled on the subdev device directly.  
> > 
> > Why isn't it possible? A media pipeline should have at least a video
> > devnode where the standard ioctls will be issued.  
> 
> Not for an MC-centric device like the r-car or imx. It's why we have v4l-subdev
> ioctls for the DV_TIMINGS API, but the corresponding SDTV standards API is
> missing.
> 
> And in a complex scenario there is nothing preventing you from having multiple
> SDTV inputs, some of which need PAL-BG, some SECAM, some NTSC (less likely)
> which are all composed together (think security cameras or something like that).
> 
> You definitely cannot set the standard from a video device. If nothing else,
> it would be completely inconsistent with how HDMI inputs work.
> 
> The whole point of MC centric devices is that you *don't* control subdevs
> from video nodes.

Well, the way it is, this change is disruptive, as, as far as I remember,
MC-based devices with tvp5150 already sets STD via the /dev/video device.

If we're willing to add it, we'll need to be clear when one approach
should be taken, and be clear that, if the SUBDEV version is used, the
driver should not support the non-subdev option.

> 
> Regards,
> 
> 	Hans
> 
> > So, I don't see why you would need to explicitly set the standard inside
> > a sub-device.
> > 
> > The way I see, inside a given pipeline, all subdevs should be using the
> > same video standard (maybe except for a m2m device with would have some
> > coded that would be doing format conversion).
> > 
> > Am I missing something?
> > 
> > Thanks,
> > Mauro
> >   
> 



Thanks,
Mauro
Hans Verkuil June 29, 2018, 10:26 a.m. UTC | #4
On 06/29/18 12:06, Mauro Carvalho Chehab wrote:
> Em Thu, 28 Jun 2018 14:47:05 +0200
> Hans Verkuil <hverkuil@xs4all.nl> escreveu:
> 
>> On 06/28/18 13:37, Mauro Carvalho Chehab wrote:
>>> Em Thu, 17 May 2018 16:30:16 +0200
>>> Niklas Söderlund         <niklas.soderlund+renesas@ragnatech.se> escreveu:
>>>   
>>>> There is no way to control the standard of subdevices which are part of
>>>> a media device. The ioctls which exists all target video devices
>>>> explicitly and the idea is that the video device should talk to the
>>>> subdevice. For subdevices part of a media graph this is not possible and
>>>> the standard must be controlled on the subdev device directly.  
>>>
>>> Why isn't it possible? A media pipeline should have at least a video
>>> devnode where the standard ioctls will be issued.  
>>
>> Not for an MC-centric device like the r-car or imx. It's why we have v4l-subdev
>> ioctls for the DV_TIMINGS API, but the corresponding SDTV standards API is
>> missing.
>>
>> And in a complex scenario there is nothing preventing you from having multiple
>> SDTV inputs, some of which need PAL-BG, some SECAM, some NTSC (less likely)
>> which are all composed together (think security cameras or something like that).
>>
>> You definitely cannot set the standard from a video device. If nothing else,
>> it would be completely inconsistent with how HDMI inputs work.
>>
>> The whole point of MC centric devices is that you *don't* control subdevs
>> from video nodes.
> 
> Well, the way it is, this change is disruptive, as, as far as I remember,
> MC-based devices with tvp5150 already sets STD via the /dev/video device.

Really? Which driver? I am not aware of this and I think you are mistaken.
Remember that we are talking about MC-centric drivers. em28xx is not MC-centric,
even though it has a media device.

> 
> If we're willing to add it, we'll need to be clear when one approach
> should be taken, and be clear that, if the SUBDEV version is used, the
> driver should not support the non-subdev option.

Of course, but in the case of em28xx the tvp5150 v4l-subdev node is never
created, so this is not a problem.

Regards,

	Hans

> 
>>
>> Regards,
>>
>> 	Hans
>>
>>> So, I don't see why you would need to explicitly set the standard inside
>>> a sub-device.
>>>
>>> The way I see, inside a given pipeline, all subdevs should be using the
>>> same video standard (maybe except for a m2m device with would have some
>>> coded that would be doing format conversion).
>>>
>>> Am I missing something?
>>>
>>> Thanks,
>>> Mauro
>>>   
>>
> 
> 
> 
> Thanks,
> Mauro
>
Mauro Carvalho Chehab June 29, 2018, 12:28 p.m. UTC | #5
Em Fri, 29 Jun 2018 12:26:20 +0200
Hans Verkuil <hverkuil@xs4all.nl> escreveu:

> On 06/29/18 12:06, Mauro Carvalho Chehab wrote:
> > Em Thu, 28 Jun 2018 14:47:05 +0200
> > Hans Verkuil <hverkuil@xs4all.nl> escreveu:
> >   
> >> On 06/28/18 13:37, Mauro Carvalho Chehab wrote:  
> >>> Em Thu, 17 May 2018 16:30:16 +0200
> >>> Niklas Söderlund         <niklas.soderlund+renesas@ragnatech.se> escreveu:
> >>>     
> >>>> There is no way to control the standard of subdevices which are part of
> >>>> a media device. The ioctls which exists all target video devices
> >>>> explicitly and the idea is that the video device should talk to the
> >>>> subdevice. For subdevices part of a media graph this is not possible and
> >>>> the standard must be controlled on the subdev device directly.    
> >>>
> >>> Why isn't it possible? A media pipeline should have at least a video
> >>> devnode where the standard ioctls will be issued.    
> >>
> >> Not for an MC-centric device like the r-car or imx. It's why we have v4l-subdev
> >> ioctls for the DV_TIMINGS API, but the corresponding SDTV standards API is
> >> missing.
> >>
> >> And in a complex scenario there is nothing preventing you from having multiple
> >> SDTV inputs, some of which need PAL-BG, some SECAM, some NTSC (less likely)
> >> which are all composed together (think security cameras or something like that).
> >>
> >> You definitely cannot set the standard from a video device. If nothing else,
> >> it would be completely inconsistent with how HDMI inputs work.
> >>
> >> The whole point of MC centric devices is that you *don't* control subdevs
> >> from video nodes.  
> > 
> > Well, the way it is, this change is disruptive, as, as far as I remember,
> > MC-based devices with tvp5150 already sets STD via the /dev/video device.  
> 
> Really? Which driver? I am not aware of this and I think you are mistaken.
> Remember that we are talking about MC-centric drivers. em28xx is not MC-centric,
> even though it has a media device.

OMAP3. There are some boards out there with tvp5150.

> 
> > 
> > If we're willing to add it, we'll need to be clear when one approach
> > should be taken, and be clear that, if the SUBDEV version is used, the
> > driver should not support the non-subdev option.  
> 
> Of course, but in the case of em28xx the tvp5150 v4l-subdev node is never
> created, so this is not a problem.
> 
> Regards,
> 
> 	Hans
> 
> >   
> >>
> >> Regards,
> >>
> >> 	Hans
> >>  
> >>> So, I don't see why you would need to explicitly set the standard inside
> >>> a sub-device.
> >>>
> >>> The way I see, inside a given pipeline, all subdevs should be using the
> >>> same video standard (maybe except for a m2m device with would have some
> >>> coded that would be doing format conversion).
> >>>
> >>> Am I missing something?
> >>>
> >>> Thanks,
> >>> Mauro
> >>>     
> >>  
> > 
> > 
> > 
> > Thanks,
> > Mauro
> >   
> 



Thanks,
Mauro
Hans Verkuil June 29, 2018, 12:32 p.m. UTC | #6
On 06/29/18 14:28, Mauro Carvalho Chehab wrote:
> Em Fri, 29 Jun 2018 12:26:20 +0200
> Hans Verkuil <hverkuil@xs4all.nl> escreveu:
> 
>> On 06/29/18 12:06, Mauro Carvalho Chehab wrote:
>>> Em Thu, 28 Jun 2018 14:47:05 +0200
>>> Hans Verkuil <hverkuil@xs4all.nl> escreveu:
>>>   
>>>> On 06/28/18 13:37, Mauro Carvalho Chehab wrote:  
>>>>> Em Thu, 17 May 2018 16:30:16 +0200
>>>>> Niklas Söderlund         <niklas.soderlund+renesas@ragnatech.se> escreveu:
>>>>>     
>>>>>> There is no way to control the standard of subdevices which are part of
>>>>>> a media device. The ioctls which exists all target video devices
>>>>>> explicitly and the idea is that the video device should talk to the
>>>>>> subdevice. For subdevices part of a media graph this is not possible and
>>>>>> the standard must be controlled on the subdev device directly.    
>>>>>
>>>>> Why isn't it possible? A media pipeline should have at least a video
>>>>> devnode where the standard ioctls will be issued.    
>>>>
>>>> Not for an MC-centric device like the r-car or imx. It's why we have v4l-subdev
>>>> ioctls for the DV_TIMINGS API, but the corresponding SDTV standards API is
>>>> missing.
>>>>
>>>> And in a complex scenario there is nothing preventing you from having multiple
>>>> SDTV inputs, some of which need PAL-BG, some SECAM, some NTSC (less likely)
>>>> which are all composed together (think security cameras or something like that).
>>>>
>>>> You definitely cannot set the standard from a video device. If nothing else,
>>>> it would be completely inconsistent with how HDMI inputs work.
>>>>
>>>> The whole point of MC centric devices is that you *don't* control subdevs
>>>> from video nodes.  
>>>
>>> Well, the way it is, this change is disruptive, as, as far as I remember,
>>> MC-based devices with tvp5150 already sets STD via the /dev/video device.  
>>
>> Really? Which driver? I am not aware of this and I think you are mistaken.
>> Remember that we are talking about MC-centric drivers. em28xx is not MC-centric,
>> even though it has a media device.
> 
> OMAP3. There are some boards out there with tvp5150.

There is no standards support in omap3isp. It only supports sensors, not
analog or digital video receivers. So if they support SDTV, then they hacked
the omap3isp driver.

Regards,

	Hans

> 
>>
>>>
>>> If we're willing to add it, we'll need to be clear when one approach
>>> should be taken, and be clear that, if the SUBDEV version is used, the
>>> driver should not support the non-subdev option.  
>>
>> Of course, but in the case of em28xx the tvp5150 v4l-subdev node is never
>> created, so this is not a problem.
>>
>> Regards,
>>
>> 	Hans
>>
>>>   
>>>>
>>>> Regards,
>>>>
>>>> 	Hans
>>>>  
>>>>> So, I don't see why you would need to explicitly set the standard inside
>>>>> a sub-device.
>>>>>
>>>>> The way I see, inside a given pipeline, all subdevs should be using the
>>>>> same video standard (maybe except for a m2m device with would have some
>>>>> coded that would be doing format conversion).
>>>>>
>>>>> Am I missing something?
>>>>>
>>>>> Thanks,
>>>>> Mauro
>>>>>     
>>>>  
>>>
>>>
>>>
>>> Thanks,
>>> Mauro
>>>   
>>
> 
> 
> 
> Thanks,
> Mauro
>
Niklas Söderlund July 4, 2018, 6:33 a.m. UTC | #7
Hi Mauro, Hans

On 2018-06-28 14:47:05 +0200, Hans Verkuil wrote:
> On 06/28/18 13:37, Mauro Carvalho Chehab wrote:
> > Em Thu, 17 May 2018 16:30:16 +0200
> > Niklas Söderlund         <niklas.soderlund+renesas@ragnatech.se> escreveu:
> > 
> >> There is no way to control the standard of subdevices which are part of
> >> a media device. The ioctls which exists all target video devices
> >> explicitly and the idea is that the video device should talk to the
> >> subdevice. For subdevices part of a media graph this is not possible and
> >> the standard must be controlled on the subdev device directly.
> > 
> > Why isn't it possible? A media pipeline should have at least a video
> > devnode where the standard ioctls will be issued.
> 
> Not for an MC-centric device like the r-car or imx. It's why we have v4l-subdev
> ioctls for the DV_TIMINGS API, but the corresponding SDTV standards API is
> missing.

I can only agree with Hans here, we already have this subdevice API for 
DV_DIMINGS.

    #define VIDIOC_SUBDEV_S_DV_TIMINGS              _IOWR('V', 87, struct v4l2_dv_timings)
    #define VIDIOC_SUBDEV_G_DV_TIMINGS              _IOWR('V', 88, struct v4l2_dv_timings)
    #define VIDIOC_SUBDEV_ENUM_DV_TIMINGS           _IOWR('V', 98, struct v4l2_enum_dv_timings)
    #define VIDIOC_SUBDEV_QUERY_DV_TIMINGS          _IOR('V', 99, struct v4l2_dv_timings)
    #define VIDIOC_SUBDEV_DV_TIMINGS_CAP            _IOWR('V', 100, struct v4l2_dv_timings_cap)

And to me it both makes perfect sens to add this for SDTV ioctls and it 
solves a real use case for me. Currently on the R-Car Gen3 boards which 
uses a MC-centric model I have no way of controlling video standard of 
my CVBS input. The adv748x driver defaults to NTSC which means when I 
test using a PAL source I'm out of luck.

Without a change that allows controlling the video standard of the 
subdevice I don't see how this can be solved. As Hans have outlined 
using the video device node to control the subdevices is not how things 
are done for MC-centric setups.

Mauro, after the discussion in this thread are you willing to take this 
patch? If not I'm happy to keep working on scratching my itch, but a 
solution where the video device node is used to set the standard of the 
subdev I feel is the wrong way of doing things. Can you see a different  
solution to my issue that works in a MC-centric environment?

> 
> And in a complex scenario there is nothing preventing you from having multiple
> SDTV inputs, some of which need PAL-BG, some SECAM, some NTSC (less likely)
> which are all composed together (think security cameras or something like that).
> 
> You definitely cannot set the standard from a video device. If nothing else,
> it would be completely inconsistent with how HDMI inputs work.
> 
> The whole point of MC centric devices is that you *don't* control subdevs
> from video nodes.
> 
> Regards,
> 
> 	Hans
> 
> > So, I don't see why you would need to explicitly set the standard inside
> > a sub-device.
> > 
> > The way I see, inside a given pipeline, all subdevs should be using the
> > same video standard (maybe except for a m2m device with would have some
> > coded that would be doing format conversion).
> > 
> > Am I missing something?
> > 
> > Thanks,
> > Mauro
> > 
>
Mauro Carvalho Chehab July 5, 2018, 12:44 p.m. UTC | #8
Javier,

How standard settings work with the current OMAP3 drivers with tvp5150?

Regards,
Mauro


Em Thu, 17 May 2018 16:30:16 +0200
Niklas Söderlund         <niklas.soderlund+renesas@ragnatech.se> escreveu:

> There is no way to control the standard of subdevices which are part of
> a media device. The ioctls which exists all target video devices
> explicitly and the idea is that the video device should talk to the
> subdevice. For subdevices part of a media graph this is not possible and
> the standard must be controlled on the subdev device directly.
> 
> Add four new ioctls to be able to directly interact with subdevices and
> control the video standard; VIDIOC_SUBDEV_ENUMSTD, VIDIOC_SUBDEV_G_STD,
> VIDIOC_SUBDEV_S_STD and VIDIOC_SUBDEV_QUERYSTD.
> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> 
> ---
> 
> * Changes since v1
> - Added VIDIOC_SUBDEV_ENUMSTD.
> ---
>  .../media/uapi/v4l/vidioc-enumstd.rst         | 11 ++++++----
>  Documentation/media/uapi/v4l/vidioc-g-std.rst | 14 ++++++++----
>  .../media/uapi/v4l/vidioc-querystd.rst        | 11 ++++++----
>  drivers/media/v4l2-core/v4l2-subdev.c         | 22 +++++++++++++++++++
>  include/uapi/linux/v4l2-subdev.h              |  4 ++++
>  5 files changed, 50 insertions(+), 12 deletions(-)
> 
> diff --git a/Documentation/media/uapi/v4l/vidioc-enumstd.rst b/Documentation/media/uapi/v4l/vidioc-enumstd.rst
> index b7fda29f46a139a0..2644a62acd4b6822 100644
> --- a/Documentation/media/uapi/v4l/vidioc-enumstd.rst
> +++ b/Documentation/media/uapi/v4l/vidioc-enumstd.rst
> @@ -2,14 +2,14 @@
>  
>  .. _VIDIOC_ENUMSTD:
>  
> -********************
> -ioctl VIDIOC_ENUMSTD
> -********************
> +*******************************************
> +ioctl VIDIOC_ENUMSTD, VIDIOC_SUBDEV_ENUMSTD
> +*******************************************
>  
>  Name
>  ====
>  
> -VIDIOC_ENUMSTD - Enumerate supported video standards
> +VIDIOC_ENUMSTD - VIDIOC_SUBDEV_ENUMSTD - Enumerate supported video standards
>  
>  
>  Synopsis
> @@ -18,6 +18,9 @@ Synopsis
>  .. c:function:: int ioctl( int fd, VIDIOC_ENUMSTD, struct v4l2_standard *argp )
>      :name: VIDIOC_ENUMSTD
>  
> +.. c:function:: int ioctl( int fd, VIDIOC_SUBDEV_ENUMSTD, struct v4l2_standard *argp )
> +    :name: VIDIOC_SUBDEV_ENUMSTD
> +
>  
>  Arguments
>  =========
> diff --git a/Documentation/media/uapi/v4l/vidioc-g-std.rst b/Documentation/media/uapi/v4l/vidioc-g-std.rst
> index 90791ab51a5371b8..8d94f0404df270db 100644
> --- a/Documentation/media/uapi/v4l/vidioc-g-std.rst
> +++ b/Documentation/media/uapi/v4l/vidioc-g-std.rst
> @@ -2,14 +2,14 @@
>  
>  .. _VIDIOC_G_STD:
>  
> -********************************
> -ioctl VIDIOC_G_STD, VIDIOC_S_STD
> -********************************
> +**************************************************************************
> +ioctl VIDIOC_G_STD, VIDIOC_S_STD, VIDIOC_SUBDEV_G_STD, VIDIOC_SUBDEV_S_STD
> +**************************************************************************
>  
>  Name
>  ====
>  
> -VIDIOC_G_STD - VIDIOC_S_STD - Query or select the video standard of the current input
> +VIDIOC_G_STD - VIDIOC_S_STD - VIDIOC_SUBDEV_G_STD - VIDIOC_SUBDEV_S_STD - Query or select the video standard of the current input
>  
>  
>  Synopsis
> @@ -21,6 +21,12 @@ Synopsis
>  .. c:function:: int ioctl( int fd, VIDIOC_S_STD, const v4l2_std_id *argp )
>      :name: VIDIOC_S_STD
>  
> +.. c:function:: int ioctl( int fd, VIDIOC_SUBDEV_G_STD, v4l2_std_id *argp )
> +    :name: VIDIOC_SUBDEV_G_STD
> +
> +.. c:function:: int ioctl( int fd, VIDIOC_SUBDEV_S_STD, const v4l2_std_id *argp )
> +    :name: VIDIOC_SUBDEV_S_STD
> +
>  
>  Arguments
>  =========
> diff --git a/Documentation/media/uapi/v4l/vidioc-querystd.rst b/Documentation/media/uapi/v4l/vidioc-querystd.rst
> index cf40bca19b9f8665..a8385cc7481869dd 100644
> --- a/Documentation/media/uapi/v4l/vidioc-querystd.rst
> +++ b/Documentation/media/uapi/v4l/vidioc-querystd.rst
> @@ -2,14 +2,14 @@
>  
>  .. _VIDIOC_QUERYSTD:
>  
> -*********************
> -ioctl VIDIOC_QUERYSTD
> -*********************
> +*********************************************
> +ioctl VIDIOC_QUERYSTD, VIDIOC_SUBDEV_QUERYSTD
> +*********************************************
>  
>  Name
>  ====
>  
> -VIDIOC_QUERYSTD - Sense the video standard received by the current input
> +VIDIOC_QUERYSTD - VIDIOC_SUBDEV_QUERYSTD - Sense the video standard received by the current input
>  
>  
>  Synopsis
> @@ -18,6 +18,9 @@ Synopsis
>  .. c:function:: int ioctl( int fd, VIDIOC_QUERYSTD, v4l2_std_id *argp )
>      :name: VIDIOC_QUERYSTD
>  
> +.. c:function:: int ioctl( int fd, VIDIOC_SUBDEV_QUERYSTD, v4l2_std_id *argp )
> +    :name: VIDIOC_SUBDEV_QUERYSTD
> +
>  
>  Arguments
>  =========
> diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
> index f9eed938d3480b74..27a2c633f2323f5f 100644
> --- a/drivers/media/v4l2-core/v4l2-subdev.c
> +++ b/drivers/media/v4l2-core/v4l2-subdev.c
> @@ -494,6 +494,28 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg)
>  
>  	case VIDIOC_SUBDEV_S_DV_TIMINGS:
>  		return v4l2_subdev_call(sd, video, s_dv_timings, arg);
> +
> +	case VIDIOC_SUBDEV_G_STD:
> +		return v4l2_subdev_call(sd, video, g_std, arg);
> +
> +	case VIDIOC_SUBDEV_S_STD: {
> +		v4l2_std_id *std = arg;
> +
> +		return v4l2_subdev_call(sd, video, s_std, *std);
> +	}
> +
> +	case VIDIOC_SUBDEV_ENUMSTD: {
> +		struct v4l2_standard *p = arg;
> +		v4l2_std_id id;
> +
> +		if (v4l2_subdev_call(sd, video, g_tvnorms, &id))
> +			return -EINVAL;
> +
> +		return v4l_video_std_enumstd(p, id);
> +	}
> +
> +	case VIDIOC_SUBDEV_QUERYSTD:
> +		return v4l2_subdev_call(sd, video, querystd, arg);
>  #endif
>  	default:
>  		return v4l2_subdev_call(sd, core, ioctl, cmd, arg);
> diff --git a/include/uapi/linux/v4l2-subdev.h b/include/uapi/linux/v4l2-subdev.h
> index c95a53e6743cb040..03970ce3074193e6 100644
> --- a/include/uapi/linux/v4l2-subdev.h
> +++ b/include/uapi/linux/v4l2-subdev.h
> @@ -170,8 +170,12 @@ struct v4l2_subdev_selection {
>  #define VIDIOC_SUBDEV_G_SELECTION		_IOWR('V', 61, struct v4l2_subdev_selection)
>  #define VIDIOC_SUBDEV_S_SELECTION		_IOWR('V', 62, struct v4l2_subdev_selection)
>  /* The following ioctls are identical to the ioctls in videodev2.h */
> +#define VIDIOC_SUBDEV_G_STD			_IOR('V', 23, v4l2_std_id)
> +#define VIDIOC_SUBDEV_S_STD			_IOW('V', 24, v4l2_std_id)
> +#define VIDIOC_SUBDEV_ENUMSTD			_IOWR('V', 25, struct v4l2_standard)
>  #define VIDIOC_SUBDEV_G_EDID			_IOWR('V', 40, struct v4l2_edid)
>  #define VIDIOC_SUBDEV_S_EDID			_IOWR('V', 41, struct v4l2_edid)
> +#define VIDIOC_SUBDEV_QUERYSTD			_IOR('V', 63, v4l2_std_id)
>  #define VIDIOC_SUBDEV_S_DV_TIMINGS		_IOWR('V', 87, struct v4l2_dv_timings)
>  #define VIDIOC_SUBDEV_G_DV_TIMINGS		_IOWR('V', 88, struct v4l2_dv_timings)
>  #define VIDIOC_SUBDEV_ENUM_DV_TIMINGS		_IOWR('V', 98, struct v4l2_enum_dv_timings)



Thanks,
Mauro
Hans Verkuil July 5, 2018, 1:12 p.m. UTC | #9
On 05/07/18 14:44, Mauro Carvalho Chehab wrote:
> Javier,
> 
> How standard settings work with the current OMAP3 drivers with tvp5150?

It looks like tvp5150 uses autodetect of the standard, which in general is
not a good thing to do since different standards have different buffer
sizes. But this chip can scale, so it might scale PAL to NTSC or vice versa
if the standard switches mid-stream. Or it only detects the standard when
it starts streaming, I'm not sure.

In any case, this is not normal behavior, for almost all analog video
receivers you need to be able to set the std explicitly.

Regards,

	Hans

> 
> Regards,
> Mauro
> 
> 
> Em Thu, 17 May 2018 16:30:16 +0200
> Niklas Söderlund         <niklas.soderlund+renesas@ragnatech.se> escreveu:
> 
>> There is no way to control the standard of subdevices which are part of
>> a media device. The ioctls which exists all target video devices
>> explicitly and the idea is that the video device should talk to the
>> subdevice. For subdevices part of a media graph this is not possible and
>> the standard must be controlled on the subdev device directly.
>>
>> Add four new ioctls to be able to directly interact with subdevices and
>> control the video standard; VIDIOC_SUBDEV_ENUMSTD, VIDIOC_SUBDEV_G_STD,
>> VIDIOC_SUBDEV_S_STD and VIDIOC_SUBDEV_QUERYSTD.
>>
>> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
>>
>> ---
>>
>> * Changes since v1
>> - Added VIDIOC_SUBDEV_ENUMSTD.
>> ---
>>  .../media/uapi/v4l/vidioc-enumstd.rst         | 11 ++++++----
>>  Documentation/media/uapi/v4l/vidioc-g-std.rst | 14 ++++++++----
>>  .../media/uapi/v4l/vidioc-querystd.rst        | 11 ++++++----
>>  drivers/media/v4l2-core/v4l2-subdev.c         | 22 +++++++++++++++++++
>>  include/uapi/linux/v4l2-subdev.h              |  4 ++++
>>  5 files changed, 50 insertions(+), 12 deletions(-)
>>
>> diff --git a/Documentation/media/uapi/v4l/vidioc-enumstd.rst b/Documentation/media/uapi/v4l/vidioc-enumstd.rst
>> index b7fda29f46a139a0..2644a62acd4b6822 100644
>> --- a/Documentation/media/uapi/v4l/vidioc-enumstd.rst
>> +++ b/Documentation/media/uapi/v4l/vidioc-enumstd.rst
>> @@ -2,14 +2,14 @@
>>  
>>  .. _VIDIOC_ENUMSTD:
>>  
>> -********************
>> -ioctl VIDIOC_ENUMSTD
>> -********************
>> +*******************************************
>> +ioctl VIDIOC_ENUMSTD, VIDIOC_SUBDEV_ENUMSTD
>> +*******************************************
>>  
>>  Name
>>  ====
>>  
>> -VIDIOC_ENUMSTD - Enumerate supported video standards
>> +VIDIOC_ENUMSTD - VIDIOC_SUBDEV_ENUMSTD - Enumerate supported video standards
>>  
>>  
>>  Synopsis
>> @@ -18,6 +18,9 @@ Synopsis
>>  .. c:function:: int ioctl( int fd, VIDIOC_ENUMSTD, struct v4l2_standard *argp )
>>      :name: VIDIOC_ENUMSTD
>>  
>> +.. c:function:: int ioctl( int fd, VIDIOC_SUBDEV_ENUMSTD, struct v4l2_standard *argp )
>> +    :name: VIDIOC_SUBDEV_ENUMSTD
>> +
>>  
>>  Arguments
>>  =========
>> diff --git a/Documentation/media/uapi/v4l/vidioc-g-std.rst b/Documentation/media/uapi/v4l/vidioc-g-std.rst
>> index 90791ab51a5371b8..8d94f0404df270db 100644
>> --- a/Documentation/media/uapi/v4l/vidioc-g-std.rst
>> +++ b/Documentation/media/uapi/v4l/vidioc-g-std.rst
>> @@ -2,14 +2,14 @@
>>  
>>  .. _VIDIOC_G_STD:
>>  
>> -********************************
>> -ioctl VIDIOC_G_STD, VIDIOC_S_STD
>> -********************************
>> +**************************************************************************
>> +ioctl VIDIOC_G_STD, VIDIOC_S_STD, VIDIOC_SUBDEV_G_STD, VIDIOC_SUBDEV_S_STD
>> +**************************************************************************
>>  
>>  Name
>>  ====
>>  
>> -VIDIOC_G_STD - VIDIOC_S_STD - Query or select the video standard of the current input
>> +VIDIOC_G_STD - VIDIOC_S_STD - VIDIOC_SUBDEV_G_STD - VIDIOC_SUBDEV_S_STD - Query or select the video standard of the current input
>>  
>>  
>>  Synopsis
>> @@ -21,6 +21,12 @@ Synopsis
>>  .. c:function:: int ioctl( int fd, VIDIOC_S_STD, const v4l2_std_id *argp )
>>      :name: VIDIOC_S_STD
>>  
>> +.. c:function:: int ioctl( int fd, VIDIOC_SUBDEV_G_STD, v4l2_std_id *argp )
>> +    :name: VIDIOC_SUBDEV_G_STD
>> +
>> +.. c:function:: int ioctl( int fd, VIDIOC_SUBDEV_S_STD, const v4l2_std_id *argp )
>> +    :name: VIDIOC_SUBDEV_S_STD
>> +
>>  
>>  Arguments
>>  =========
>> diff --git a/Documentation/media/uapi/v4l/vidioc-querystd.rst b/Documentation/media/uapi/v4l/vidioc-querystd.rst
>> index cf40bca19b9f8665..a8385cc7481869dd 100644
>> --- a/Documentation/media/uapi/v4l/vidioc-querystd.rst
>> +++ b/Documentation/media/uapi/v4l/vidioc-querystd.rst
>> @@ -2,14 +2,14 @@
>>  
>>  .. _VIDIOC_QUERYSTD:
>>  
>> -*********************
>> -ioctl VIDIOC_QUERYSTD
>> -*********************
>> +*********************************************
>> +ioctl VIDIOC_QUERYSTD, VIDIOC_SUBDEV_QUERYSTD
>> +*********************************************
>>  
>>  Name
>>  ====
>>  
>> -VIDIOC_QUERYSTD - Sense the video standard received by the current input
>> +VIDIOC_QUERYSTD - VIDIOC_SUBDEV_QUERYSTD - Sense the video standard received by the current input
>>  
>>  
>>  Synopsis
>> @@ -18,6 +18,9 @@ Synopsis
>>  .. c:function:: int ioctl( int fd, VIDIOC_QUERYSTD, v4l2_std_id *argp )
>>      :name: VIDIOC_QUERYSTD
>>  
>> +.. c:function:: int ioctl( int fd, VIDIOC_SUBDEV_QUERYSTD, v4l2_std_id *argp )
>> +    :name: VIDIOC_SUBDEV_QUERYSTD
>> +
>>  
>>  Arguments
>>  =========
>> diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
>> index f9eed938d3480b74..27a2c633f2323f5f 100644
>> --- a/drivers/media/v4l2-core/v4l2-subdev.c
>> +++ b/drivers/media/v4l2-core/v4l2-subdev.c
>> @@ -494,6 +494,28 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg)
>>  
>>  	case VIDIOC_SUBDEV_S_DV_TIMINGS:
>>  		return v4l2_subdev_call(sd, video, s_dv_timings, arg);
>> +
>> +	case VIDIOC_SUBDEV_G_STD:
>> +		return v4l2_subdev_call(sd, video, g_std, arg);
>> +
>> +	case VIDIOC_SUBDEV_S_STD: {
>> +		v4l2_std_id *std = arg;
>> +
>> +		return v4l2_subdev_call(sd, video, s_std, *std);
>> +	}
>> +
>> +	case VIDIOC_SUBDEV_ENUMSTD: {
>> +		struct v4l2_standard *p = arg;
>> +		v4l2_std_id id;
>> +
>> +		if (v4l2_subdev_call(sd, video, g_tvnorms, &id))
>> +			return -EINVAL;
>> +
>> +		return v4l_video_std_enumstd(p, id);
>> +	}
>> +
>> +	case VIDIOC_SUBDEV_QUERYSTD:
>> +		return v4l2_subdev_call(sd, video, querystd, arg);
>>  #endif
>>  	default:
>>  		return v4l2_subdev_call(sd, core, ioctl, cmd, arg);
>> diff --git a/include/uapi/linux/v4l2-subdev.h b/include/uapi/linux/v4l2-subdev.h
>> index c95a53e6743cb040..03970ce3074193e6 100644
>> --- a/include/uapi/linux/v4l2-subdev.h
>> +++ b/include/uapi/linux/v4l2-subdev.h
>> @@ -170,8 +170,12 @@ struct v4l2_subdev_selection {
>>  #define VIDIOC_SUBDEV_G_SELECTION		_IOWR('V', 61, struct v4l2_subdev_selection)
>>  #define VIDIOC_SUBDEV_S_SELECTION		_IOWR('V', 62, struct v4l2_subdev_selection)
>>  /* The following ioctls are identical to the ioctls in videodev2.h */
>> +#define VIDIOC_SUBDEV_G_STD			_IOR('V', 23, v4l2_std_id)
>> +#define VIDIOC_SUBDEV_S_STD			_IOW('V', 24, v4l2_std_id)
>> +#define VIDIOC_SUBDEV_ENUMSTD			_IOWR('V', 25, struct v4l2_standard)
>>  #define VIDIOC_SUBDEV_G_EDID			_IOWR('V', 40, struct v4l2_edid)
>>  #define VIDIOC_SUBDEV_S_EDID			_IOWR('V', 41, struct v4l2_edid)
>> +#define VIDIOC_SUBDEV_QUERYSTD			_IOR('V', 63, v4l2_std_id)
>>  #define VIDIOC_SUBDEV_S_DV_TIMINGS		_IOWR('V', 87, struct v4l2_dv_timings)
>>  #define VIDIOC_SUBDEV_G_DV_TIMINGS		_IOWR('V', 88, struct v4l2_dv_timings)
>>  #define VIDIOC_SUBDEV_ENUM_DV_TIMINGS		_IOWR('V', 98, struct v4l2_enum_dv_timings)
> 
> 
> 
> Thanks,
> Mauro
>
Javier Martinez Canillas July 8, 2018, 1:11 p.m. UTC | #10
[adding Marco Felsch since he has been working on this driver]

On 07/05/2018 03:12 PM, Hans Verkuil wrote:
> On 05/07/18 14:44, Mauro Carvalho Chehab wrote:
>> Javier,
>>
>> How standard settings work with the current OMAP3 drivers with tvp5150?
> 
> It looks like tvp5150 uses autodetect of the standard, which in general is

That's correct, the driver uses standard autodetect.

> not a good thing to do since different standards have different buffer
> sizes. But this chip can scale, so it might scale PAL to NTSC or vice versa
> if the standard switches mid-stream. Or it only detects the standard when
> it starts streaming, I'm not sure.
>

Not sure about this either, IIUC switching the standard mid-stream won't work.

> In any case, this is not normal behavior, for almost all analog video
> receivers you need to be able to set the std explicitly.
>

Indeed. I see that Marco's recent series [0] add supports for the .querystd [1]
and .g_std [2] callbacks to the tvp5150 driver, so that way user-space can get
back the detected standard.

[0]: https://www.spinics.net/lists/linux-media/msg136869.html
[1]: https://www.spinics.net/lists/linux-media/msg136872.html
[2]: https://www.spinics.net/lists/linux-media/msg136875.html

> Regards,
> 
> 	Hans
>

Best regards,
Marco Felsch July 11, 2018, 10:39 a.m. UTC | #11
Hi Javier,

On 18-07-08 15:11, Javier Martinez Canillas wrote:
> [adding Marco Felsch since he has been working on this driver]
> 
> On 07/05/2018 03:12 PM, Hans Verkuil wrote:
> > On 05/07/18 14:44, Mauro Carvalho Chehab wrote:
> >> Javier,
> >>
> >> How standard settings work with the current OMAP3 drivers with tvp5150?
> > 
> > It looks like tvp5150 uses autodetect of the standard, which in general is
> 
> That's correct, the driver uses standard autodetect.
> 
> > not a good thing to do since different standards have different buffer
> > sizes. But this chip can scale, so it might scale PAL to NTSC or vice versa
> > if the standard switches mid-stream. Or it only detects the standard when
> > it starts streaming, I'm not sure.
> >
> 
> Not sure about this either, IIUC switching the standard mid-stream won't work.

As far as I know, the detection happens after a sync lost event.

> > In any case, this is not normal behavior, for almost all analog video
> > receivers you need to be able to set the std explicitly.
> >
> 
> Indeed. I see that Marco's recent series [0] add supports for the .querystd [1]
> and .g_std [2] callbacks to the tvp5150 driver, so that way user-space can get
> back the detected standard.
> 
> [0]: https://www.spinics.net/lists/linux-media/msg136869.html
> [1]: https://www.spinics.net/lists/linux-media/msg136872.html
> [2]: https://www.spinics.net/lists/linux-media/msg136875.html

I tought the std will be set by the v4l2_subdev_video_ops.s_std()
operation. If the user change the std manually, the autodection is
disabled.

> > Regards,
> > 
> > 	Hans
> >
> 
> Best regards,

Best regards,
Marco

> -- 
> Javier Martinez Canillas
> Software Engineer - Desktop Hardware Enablement
> Red Hat
> 
> >>
> >> Regards,
> >> Mauro
> >>
> >>
> >> Em Thu, 17 May 2018 16:30:16 +0200
> >> Niklas Söderlund         <niklas.soderlund+renesas@ragnatech.se> escreveu:
> >>
> >>> There is no way to control the standard of subdevices which are part of
> >>> a media device. The ioctls which exists all target video devices
> >>> explicitly and the idea is that the video device should talk to the
> >>> subdevice. For subdevices part of a media graph this is not possible and
> >>> the standard must be controlled on the subdev device directly.
> >>>
> >>> Add four new ioctls to be able to directly interact with subdevices and
> >>> control the video standard; VIDIOC_SUBDEV_ENUMSTD, VIDIOC_SUBDEV_G_STD,
> >>> VIDIOC_SUBDEV_S_STD and VIDIOC_SUBDEV_QUERYSTD.
> >>>
> >>> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> >>>
> >>> ---
> >>>
> >>> * Changes since v1
> >>> - Added VIDIOC_SUBDEV_ENUMSTD.
> >>> ---
> >>>  .../media/uapi/v4l/vidioc-enumstd.rst         | 11 ++++++----
> >>>  Documentation/media/uapi/v4l/vidioc-g-std.rst | 14 ++++++++----
> >>>  .../media/uapi/v4l/vidioc-querystd.rst        | 11 ++++++----
> >>>  drivers/media/v4l2-core/v4l2-subdev.c         | 22 +++++++++++++++++++
> >>>  include/uapi/linux/v4l2-subdev.h              |  4 ++++
> >>>  5 files changed, 50 insertions(+), 12 deletions(-)
> >>>
> >>> diff --git a/Documentation/media/uapi/v4l/vidioc-enumstd.rst b/Documentation/media/uapi/v4l/vidioc-enumstd.rst
> >>> index b7fda29f46a139a0..2644a62acd4b6822 100644
> >>> --- a/Documentation/media/uapi/v4l/vidioc-enumstd.rst
> >>> +++ b/Documentation/media/uapi/v4l/vidioc-enumstd.rst
> >>> @@ -2,14 +2,14 @@
> >>>  
> >>>  .. _VIDIOC_ENUMSTD:
> >>>  
> >>> -********************
> >>> -ioctl VIDIOC_ENUMSTD
> >>> -********************
> >>> +*******************************************
> >>> +ioctl VIDIOC_ENUMSTD, VIDIOC_SUBDEV_ENUMSTD
> >>> +*******************************************
> >>>  
> >>>  Name
> >>>  ====
> >>>  
> >>> -VIDIOC_ENUMSTD - Enumerate supported video standards
> >>> +VIDIOC_ENUMSTD - VIDIOC_SUBDEV_ENUMSTD - Enumerate supported video standards
> >>>  
> >>>  
> >>>  Synopsis
> >>> @@ -18,6 +18,9 @@ Synopsis
> >>>  .. c:function:: int ioctl( int fd, VIDIOC_ENUMSTD, struct v4l2_standard *argp )
> >>>      :name: VIDIOC_ENUMSTD
> >>>  
> >>> +.. c:function:: int ioctl( int fd, VIDIOC_SUBDEV_ENUMSTD, struct v4l2_standard *argp )
> >>> +    :name: VIDIOC_SUBDEV_ENUMSTD
> >>> +
> >>>  
> >>>  Arguments
> >>>  =========
> >>> diff --git a/Documentation/media/uapi/v4l/vidioc-g-std.rst b/Documentation/media/uapi/v4l/vidioc-g-std.rst
> >>> index 90791ab51a5371b8..8d94f0404df270db 100644
> >>> --- a/Documentation/media/uapi/v4l/vidioc-g-std.rst
> >>> +++ b/Documentation/media/uapi/v4l/vidioc-g-std.rst
> >>> @@ -2,14 +2,14 @@
> >>>  
> >>>  .. _VIDIOC_G_STD:
> >>>  
> >>> -********************************
> >>> -ioctl VIDIOC_G_STD, VIDIOC_S_STD
> >>> -********************************
> >>> +**************************************************************************
> >>> +ioctl VIDIOC_G_STD, VIDIOC_S_STD, VIDIOC_SUBDEV_G_STD, VIDIOC_SUBDEV_S_STD
> >>> +**************************************************************************
> >>>  
> >>>  Name
> >>>  ====
> >>>  
> >>> -VIDIOC_G_STD - VIDIOC_S_STD - Query or select the video standard of the current input
> >>> +VIDIOC_G_STD - VIDIOC_S_STD - VIDIOC_SUBDEV_G_STD - VIDIOC_SUBDEV_S_STD - Query or select the video standard of the current input
> >>>  
> >>>  
> >>>  Synopsis
> >>> @@ -21,6 +21,12 @@ Synopsis
> >>>  .. c:function:: int ioctl( int fd, VIDIOC_S_STD, const v4l2_std_id *argp )
> >>>      :name: VIDIOC_S_STD
> >>>  
> >>> +.. c:function:: int ioctl( int fd, VIDIOC_SUBDEV_G_STD, v4l2_std_id *argp )
> >>> +    :name: VIDIOC_SUBDEV_G_STD
> >>> +
> >>> +.. c:function:: int ioctl( int fd, VIDIOC_SUBDEV_S_STD, const v4l2_std_id *argp )
> >>> +    :name: VIDIOC_SUBDEV_S_STD
> >>> +
> >>>  
> >>>  Arguments
> >>>  =========
> >>> diff --git a/Documentation/media/uapi/v4l/vidioc-querystd.rst b/Documentation/media/uapi/v4l/vidioc-querystd.rst
> >>> index cf40bca19b9f8665..a8385cc7481869dd 100644
> >>> --- a/Documentation/media/uapi/v4l/vidioc-querystd.rst
> >>> +++ b/Documentation/media/uapi/v4l/vidioc-querystd.rst
> >>> @@ -2,14 +2,14 @@
> >>>  
> >>>  .. _VIDIOC_QUERYSTD:
> >>>  
> >>> -*********************
> >>> -ioctl VIDIOC_QUERYSTD
> >>> -*********************
> >>> +*********************************************
> >>> +ioctl VIDIOC_QUERYSTD, VIDIOC_SUBDEV_QUERYSTD
> >>> +*********************************************
> >>>  
> >>>  Name
> >>>  ====
> >>>  
> >>> -VIDIOC_QUERYSTD - Sense the video standard received by the current input
> >>> +VIDIOC_QUERYSTD - VIDIOC_SUBDEV_QUERYSTD - Sense the video standard received by the current input
> >>>  
> >>>  
> >>>  Synopsis
> >>> @@ -18,6 +18,9 @@ Synopsis
> >>>  .. c:function:: int ioctl( int fd, VIDIOC_QUERYSTD, v4l2_std_id *argp )
> >>>      :name: VIDIOC_QUERYSTD
> >>>  
> >>> +.. c:function:: int ioctl( int fd, VIDIOC_SUBDEV_QUERYSTD, v4l2_std_id *argp )
> >>> +    :name: VIDIOC_SUBDEV_QUERYSTD
> >>> +
> >>>  
> >>>  Arguments
> >>>  =========
> >>> diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
> >>> index f9eed938d3480b74..27a2c633f2323f5f 100644
> >>> --- a/drivers/media/v4l2-core/v4l2-subdev.c
> >>> +++ b/drivers/media/v4l2-core/v4l2-subdev.c
> >>> @@ -494,6 +494,28 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg)
> >>>  
> >>>  	case VIDIOC_SUBDEV_S_DV_TIMINGS:
> >>>  		return v4l2_subdev_call(sd, video, s_dv_timings, arg);
> >>> +
> >>> +	case VIDIOC_SUBDEV_G_STD:
> >>> +		return v4l2_subdev_call(sd, video, g_std, arg);
> >>> +
> >>> +	case VIDIOC_SUBDEV_S_STD: {
> >>> +		v4l2_std_id *std = arg;
> >>> +
> >>> +		return v4l2_subdev_call(sd, video, s_std, *std);
> >>> +	}
> >>> +
> >>> +	case VIDIOC_SUBDEV_ENUMSTD: {
> >>> +		struct v4l2_standard *p = arg;
> >>> +		v4l2_std_id id;
> >>> +
> >>> +		if (v4l2_subdev_call(sd, video, g_tvnorms, &id))
> >>> +			return -EINVAL;
> >>> +
> >>> +		return v4l_video_std_enumstd(p, id);
> >>> +	}
> >>> +
> >>> +	case VIDIOC_SUBDEV_QUERYSTD:
> >>> +		return v4l2_subdev_call(sd, video, querystd, arg);
> >>>  #endif
> >>>  	default:
> >>>  		return v4l2_subdev_call(sd, core, ioctl, cmd, arg);
> >>> diff --git a/include/uapi/linux/v4l2-subdev.h b/include/uapi/linux/v4l2-subdev.h
> >>> index c95a53e6743cb040..03970ce3074193e6 100644
> >>> --- a/include/uapi/linux/v4l2-subdev.h
> >>> +++ b/include/uapi/linux/v4l2-subdev.h
> >>> @@ -170,8 +170,12 @@ struct v4l2_subdev_selection {
> >>>  #define VIDIOC_SUBDEV_G_SELECTION		_IOWR('V', 61, struct v4l2_subdev_selection)
> >>>  #define VIDIOC_SUBDEV_S_SELECTION		_IOWR('V', 62, struct v4l2_subdev_selection)
> >>>  /* The following ioctls are identical to the ioctls in videodev2.h */
> >>> +#define VIDIOC_SUBDEV_G_STD			_IOR('V', 23, v4l2_std_id)
> >>> +#define VIDIOC_SUBDEV_S_STD			_IOW('V', 24, v4l2_std_id)
> >>> +#define VIDIOC_SUBDEV_ENUMSTD			_IOWR('V', 25, struct v4l2_standard)
> >>>  #define VIDIOC_SUBDEV_G_EDID			_IOWR('V', 40, struct v4l2_edid)
> >>>  #define VIDIOC_SUBDEV_S_EDID			_IOWR('V', 41, struct v4l2_edid)
> >>> +#define VIDIOC_SUBDEV_QUERYSTD			_IOR('V', 63, v4l2_std_id)
> >>>  #define VIDIOC_SUBDEV_S_DV_TIMINGS		_IOWR('V', 87, struct v4l2_dv_timings)
> >>>  #define VIDIOC_SUBDEV_G_DV_TIMINGS		_IOWR('V', 88, struct v4l2_dv_timings)
> >>>  #define VIDIOC_SUBDEV_ENUM_DV_TIMINGS		_IOWR('V', 98, struct v4l2_enum_dv_timings)
> >>
> >>
> >>
> >> Thanks,
> >> Mauro
> >>
> >
Javier Martinez Canillas July 13, 2018, 9:18 a.m. UTC | #12
On 07/11/2018 12:39 PM, Marco Felsch wrote:
> Hi Javier,
> 
> On 18-07-08 15:11, Javier Martinez Canillas wrote:
>> [adding Marco Felsch since he has been working on this driver]
>>
>> On 07/05/2018 03:12 PM, Hans Verkuil wrote:
>>> On 05/07/18 14:44, Mauro Carvalho Chehab wrote:
>>>> Javier,
>>>>
>>>> How standard settings work with the current OMAP3 drivers with tvp5150?
>>>
>>> It looks like tvp5150 uses autodetect of the standard, which in general is
>>
>> That's correct, the driver uses standard autodetect.
>>
>>> not a good thing to do since different standards have different buffer
>>> sizes. But this chip can scale, so it might scale PAL to NTSC or vice versa
>>> if the standard switches mid-stream. Or it only detects the standard when
>>> it starts streaming, I'm not sure.
>>>
>>
>> Not sure about this either, IIUC switching the standard mid-stream won't work.
> 
> As far as I know, the detection happens after a sync lost event.
>

Ah, good to know.
 
>>> In any case, this is not normal behavior, for almost all analog video
>>> receivers you need to be able to set the std explicitly.
>>>
>>
>> Indeed. I see that Marco's recent series [0] add supports for the .querystd [1]
>> and .g_std [2] callbacks to the tvp5150 driver, so that way user-space can get
>> back the detected standard.
>>
>> [0]: https://www.spinics.net/lists/linux-media/msg136869.html
>> [1]: https://www.spinics.net/lists/linux-media/msg136872.html
>> [2]: https://www.spinics.net/lists/linux-media/msg136875.html
> 
> I tought the std will be set by the v4l2_subdev_video_ops.s_std()
> operation. If the user change the std manually, the autodection is
> disabled.
>

Yes, what I tried to say is that user-space won't have a way to know which std
to set without a .querystd, or know what std was autodetected withou a .g_std.

Best regards,
Marco Felsch July 13, 2018, 10:54 a.m. UTC | #13
Hi Javier,

On 18-07-13 11:18, Javier Martinez Canillas wrote:
> On 07/11/2018 12:39 PM, Marco Felsch wrote:
> > Hi Javier,
> > 
> > On 18-07-08 15:11, Javier Martinez Canillas wrote:
> >> [adding Marco Felsch since he has been working on this driver]
> >>
> >> On 07/05/2018 03:12 PM, Hans Verkuil wrote:
> >>> On 05/07/18 14:44, Mauro Carvalho Chehab wrote:
> >>>> Javier,
> >>>>
> >>>> How standard settings work with the current OMAP3 drivers with tvp5150?
> >>>
> >>> It looks like tvp5150 uses autodetect of the standard, which in general is
> >>
> >> That's correct, the driver uses standard autodetect.
> >>
> >>> not a good thing to do since different standards have different buffer
> >>> sizes. But this chip can scale, so it might scale PAL to NTSC or vice versa
> >>> if the standard switches mid-stream. Or it only detects the standard when
> >>> it starts streaming, I'm not sure.
> >>>
> >>
> >> Not sure about this either, IIUC switching the standard mid-stream won't work.
> > 
> > As far as I know, the detection happens after a sync lost event.
> >
> 
> Ah, good to know.
>  
> >>> In any case, this is not normal behavior, for almost all analog video
> >>> receivers you need to be able to set the std explicitly.
> >>>
> >>
> >> Indeed. I see that Marco's recent series [0] add supports for the .querystd [1]
> >> and .g_std [2] callbacks to the tvp5150 driver, so that way user-space can get
> >> back the detected standard.
> >>
> >> [0]: https://www.spinics.net/lists/linux-media/msg136869.html
> >> [1]: https://www.spinics.net/lists/linux-media/msg136872.html
> >> [2]: https://www.spinics.net/lists/linux-media/msg136875.html
> > 
> > I tought the std will be set by the v4l2_subdev_video_ops.s_std()
> > operation. If the user change the std manually, the autodection is
> > disabled.
> >
> 
> Yes, what I tried to say is that user-space won't have a way to know which std
> to set without a .querystd, or know what std was autodetected withou a .g_std.

Now I got you. Yes, thats the only way, as far as I know.

> Best regards,

Regards,
Marco
diff mbox

Patch

diff --git a/Documentation/media/uapi/v4l/vidioc-enumstd.rst b/Documentation/media/uapi/v4l/vidioc-enumstd.rst
index b7fda29f46a139a0..2644a62acd4b6822 100644
--- a/Documentation/media/uapi/v4l/vidioc-enumstd.rst
+++ b/Documentation/media/uapi/v4l/vidioc-enumstd.rst
@@ -2,14 +2,14 @@ 
 
 .. _VIDIOC_ENUMSTD:
 
-********************
-ioctl VIDIOC_ENUMSTD
-********************
+*******************************************
+ioctl VIDIOC_ENUMSTD, VIDIOC_SUBDEV_ENUMSTD
+*******************************************
 
 Name
 ====
 
-VIDIOC_ENUMSTD - Enumerate supported video standards
+VIDIOC_ENUMSTD - VIDIOC_SUBDEV_ENUMSTD - Enumerate supported video standards
 
 
 Synopsis
@@ -18,6 +18,9 @@  Synopsis
 .. c:function:: int ioctl( int fd, VIDIOC_ENUMSTD, struct v4l2_standard *argp )
     :name: VIDIOC_ENUMSTD
 
+.. c:function:: int ioctl( int fd, VIDIOC_SUBDEV_ENUMSTD, struct v4l2_standard *argp )
+    :name: VIDIOC_SUBDEV_ENUMSTD
+
 
 Arguments
 =========
diff --git a/Documentation/media/uapi/v4l/vidioc-g-std.rst b/Documentation/media/uapi/v4l/vidioc-g-std.rst
index 90791ab51a5371b8..8d94f0404df270db 100644
--- a/Documentation/media/uapi/v4l/vidioc-g-std.rst
+++ b/Documentation/media/uapi/v4l/vidioc-g-std.rst
@@ -2,14 +2,14 @@ 
 
 .. _VIDIOC_G_STD:
 
-********************************
-ioctl VIDIOC_G_STD, VIDIOC_S_STD
-********************************
+**************************************************************************
+ioctl VIDIOC_G_STD, VIDIOC_S_STD, VIDIOC_SUBDEV_G_STD, VIDIOC_SUBDEV_S_STD
+**************************************************************************
 
 Name
 ====
 
-VIDIOC_G_STD - VIDIOC_S_STD - Query or select the video standard of the current input
+VIDIOC_G_STD - VIDIOC_S_STD - VIDIOC_SUBDEV_G_STD - VIDIOC_SUBDEV_S_STD - Query or select the video standard of the current input
 
 
 Synopsis
@@ -21,6 +21,12 @@  Synopsis
 .. c:function:: int ioctl( int fd, VIDIOC_S_STD, const v4l2_std_id *argp )
     :name: VIDIOC_S_STD
 
+.. c:function:: int ioctl( int fd, VIDIOC_SUBDEV_G_STD, v4l2_std_id *argp )
+    :name: VIDIOC_SUBDEV_G_STD
+
+.. c:function:: int ioctl( int fd, VIDIOC_SUBDEV_S_STD, const v4l2_std_id *argp )
+    :name: VIDIOC_SUBDEV_S_STD
+
 
 Arguments
 =========
diff --git a/Documentation/media/uapi/v4l/vidioc-querystd.rst b/Documentation/media/uapi/v4l/vidioc-querystd.rst
index cf40bca19b9f8665..a8385cc7481869dd 100644
--- a/Documentation/media/uapi/v4l/vidioc-querystd.rst
+++ b/Documentation/media/uapi/v4l/vidioc-querystd.rst
@@ -2,14 +2,14 @@ 
 
 .. _VIDIOC_QUERYSTD:
 
-*********************
-ioctl VIDIOC_QUERYSTD
-*********************
+*********************************************
+ioctl VIDIOC_QUERYSTD, VIDIOC_SUBDEV_QUERYSTD
+*********************************************
 
 Name
 ====
 
-VIDIOC_QUERYSTD - Sense the video standard received by the current input
+VIDIOC_QUERYSTD - VIDIOC_SUBDEV_QUERYSTD - Sense the video standard received by the current input
 
 
 Synopsis
@@ -18,6 +18,9 @@  Synopsis
 .. c:function:: int ioctl( int fd, VIDIOC_QUERYSTD, v4l2_std_id *argp )
     :name: VIDIOC_QUERYSTD
 
+.. c:function:: int ioctl( int fd, VIDIOC_SUBDEV_QUERYSTD, v4l2_std_id *argp )
+    :name: VIDIOC_SUBDEV_QUERYSTD
+
 
 Arguments
 =========
diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
index f9eed938d3480b74..27a2c633f2323f5f 100644
--- a/drivers/media/v4l2-core/v4l2-subdev.c
+++ b/drivers/media/v4l2-core/v4l2-subdev.c
@@ -494,6 +494,28 @@  static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg)
 
 	case VIDIOC_SUBDEV_S_DV_TIMINGS:
 		return v4l2_subdev_call(sd, video, s_dv_timings, arg);
+
+	case VIDIOC_SUBDEV_G_STD:
+		return v4l2_subdev_call(sd, video, g_std, arg);
+
+	case VIDIOC_SUBDEV_S_STD: {
+		v4l2_std_id *std = arg;
+
+		return v4l2_subdev_call(sd, video, s_std, *std);
+	}
+
+	case VIDIOC_SUBDEV_ENUMSTD: {
+		struct v4l2_standard *p = arg;
+		v4l2_std_id id;
+
+		if (v4l2_subdev_call(sd, video, g_tvnorms, &id))
+			return -EINVAL;
+
+		return v4l_video_std_enumstd(p, id);
+	}
+
+	case VIDIOC_SUBDEV_QUERYSTD:
+		return v4l2_subdev_call(sd, video, querystd, arg);
 #endif
 	default:
 		return v4l2_subdev_call(sd, core, ioctl, cmd, arg);
diff --git a/include/uapi/linux/v4l2-subdev.h b/include/uapi/linux/v4l2-subdev.h
index c95a53e6743cb040..03970ce3074193e6 100644
--- a/include/uapi/linux/v4l2-subdev.h
+++ b/include/uapi/linux/v4l2-subdev.h
@@ -170,8 +170,12 @@  struct v4l2_subdev_selection {
 #define VIDIOC_SUBDEV_G_SELECTION		_IOWR('V', 61, struct v4l2_subdev_selection)
 #define VIDIOC_SUBDEV_S_SELECTION		_IOWR('V', 62, struct v4l2_subdev_selection)
 /* The following ioctls are identical to the ioctls in videodev2.h */
+#define VIDIOC_SUBDEV_G_STD			_IOR('V', 23, v4l2_std_id)
+#define VIDIOC_SUBDEV_S_STD			_IOW('V', 24, v4l2_std_id)
+#define VIDIOC_SUBDEV_ENUMSTD			_IOWR('V', 25, struct v4l2_standard)
 #define VIDIOC_SUBDEV_G_EDID			_IOWR('V', 40, struct v4l2_edid)
 #define VIDIOC_SUBDEV_S_EDID			_IOWR('V', 41, struct v4l2_edid)
+#define VIDIOC_SUBDEV_QUERYSTD			_IOR('V', 63, v4l2_std_id)
 #define VIDIOC_SUBDEV_S_DV_TIMINGS		_IOWR('V', 87, struct v4l2_dv_timings)
 #define VIDIOC_SUBDEV_G_DV_TIMINGS		_IOWR('V', 88, struct v4l2_dv_timings)
 #define VIDIOC_SUBDEV_ENUM_DV_TIMINGS		_IOWR('V', 98, struct v4l2_enum_dv_timings)