* [RESEND PATCH 1/1] v4l2-subdev: Try formats are only available if subdev API is enabled
@ 2019-05-31 11:54 Sakari Ailus
2019-05-31 18:40 ` Janusz Krzysztofik
0 siblings, 1 reply; 3+ messages in thread
From: Sakari Ailus @ 2019-05-31 11:54 UTC (permalink / raw)
To: linux-media; +Cc: hverkuil, Janusz Krzysztofik
Return an error for which == V4L2_SUBDEV_FORMAT_TRY if
CONFIG_VIDEO_V4L2_SUBDEV_API is not enabled. This is because the try
formats are not available in that case. Do the check here so that drivers
don't need to.
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Depends-on: ("media: v4l2-subdev: Verify v4l2_subdev_call() pad config argument")
---
drivers/media/v4l2-core/v4l2-subdev.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
index 34219e489be27..88b4b9d7c41be 100644
--- a/drivers/media/v4l2-core/v4l2-subdev.c
+++ b/drivers/media/v4l2-core/v4l2-subdev.c
@@ -122,7 +122,10 @@ static int subdev_close(struct file *file)
static inline int check_which(__u32 which)
{
- if (which != V4L2_SUBDEV_FORMAT_TRY &&
+ if (
+#ifdef CONFIG_VIDEO_V4L2_SUBDEV_API
+ which != V4L2_SUBDEV_FORMAT_TRY &&
+#endif
which != V4L2_SUBDEV_FORMAT_ACTIVE)
return -EINVAL;
--
2.11.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [RESEND PATCH 1/1] v4l2-subdev: Try formats are only available if subdev API is enabled
2019-05-31 11:54 [RESEND PATCH 1/1] v4l2-subdev: Try formats are only available if subdev API is enabled Sakari Ailus
@ 2019-05-31 18:40 ` Janusz Krzysztofik
2019-06-02 23:08 ` Sakari Ailus
0 siblings, 1 reply; 3+ messages in thread
From: Janusz Krzysztofik @ 2019-05-31 18:40 UTC (permalink / raw)
To: Sakari Ailus; +Cc: linux-media, hverkuil, Janusz Krzysztofik
Hi Sakari,
On Friday, May 31, 2019 1:54:26 PM CEST Sakari Ailus wrote:
> Return an error for which == V4L2_SUBDEV_FORMAT_TRY if
> CONFIG_VIDEO_V4L2_SUBDEV_API is not enabled. This is because the try
> formats are not available in that case.
As far as I can see, there are 21 V4L2 subdevice drivers which don't support
V4L2 subdev API but implement V4L2_SUBDEV_FORMAT_TRY operation mode somehow.
Someone already took that decision in the past and replaced video operation
callbacks with pad operation callbacks supporting V4L2_SUBDEV_FORMAT_TRY
without the requirement for V4L2 subdev API support.
With this change in place, those drivers will loose that functionality if
built without CONFIG_VIDEO_V4L2_SUBDEV_API, but will happily work as before
otherwise. That's probably not exactly what you intended.
> Do the check here so that drivers
> don't need to.
So only beneficiaries of this change will be V4L2 subdevice drivers which don't
support V4L2 subdev API and don't support V4L2_SUBDEV_FORMAT_TRY. Existing
drivers of that kind already have that check in place and I can hardly imagine
someone is going to optimize them by removing it. Any new drivers will
probably support V4L2 subdev API from the beginning so they'll not benefit from
this patch at all as they won't build without CONFIG_VIDEO_V4L2_SUBDEV_API.
I think that if a driver doesn't support V4L2_SUBDEV_FORMAT_TRY, it should
simply check for that value itself and respond accordingly, no matter if it
supports V4L2 subdev API or not. As time passes, old drivers will be either
depreciated or refreshed to handle V4L2_SUBDEV_FORMAT_TRY the V4L2 subdev API
way, as we would like them to.
Thanks,
Janusz
>
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> Depends-on: ("media: v4l2-subdev: Verify v4l2_subdev_call() pad config
argument")
> ---
> drivers/media/v4l2-core/v4l2-subdev.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/
v4l2-subdev.c
> index 34219e489be27..88b4b9d7c41be 100644
> --- a/drivers/media/v4l2-core/v4l2-subdev.c
> +++ b/drivers/media/v4l2-core/v4l2-subdev.c
> @@ -122,7 +122,10 @@ static int subdev_close(struct file *file)
>
> static inline int check_which(__u32 which)
> {
> - if (which != V4L2_SUBDEV_FORMAT_TRY &&
> + if (
> +#ifdef CONFIG_VIDEO_V4L2_SUBDEV_API
> + which != V4L2_SUBDEV_FORMAT_TRY &&
> +#endif
> which != V4L2_SUBDEV_FORMAT_ACTIVE)
> return -EINVAL;
>
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [RESEND PATCH 1/1] v4l2-subdev: Try formats are only available if subdev API is enabled
2019-05-31 18:40 ` Janusz Krzysztofik
@ 2019-06-02 23:08 ` Sakari Ailus
0 siblings, 0 replies; 3+ messages in thread
From: Sakari Ailus @ 2019-06-02 23:08 UTC (permalink / raw)
To: Janusz Krzysztofik; +Cc: linux-media, hverkuil
Hi Janusz,
On Fri, May 31, 2019 at 08:40:26PM +0200, Janusz Krzysztofik wrote:
> Hi Sakari,
>
> On Friday, May 31, 2019 1:54:26 PM CEST Sakari Ailus wrote:
> > Return an error for which == V4L2_SUBDEV_FORMAT_TRY if
> > CONFIG_VIDEO_V4L2_SUBDEV_API is not enabled. This is because the try
> > formats are not available in that case.
>
> As far as I can see, there are 21 V4L2 subdevice drivers which don't support
> V4L2 subdev API but implement V4L2_SUBDEV_FORMAT_TRY operation mode somehow.
> Someone already took that decision in the past and replaced video operation
> callbacks with pad operation callbacks supporting V4L2_SUBDEV_FORMAT_TRY
> without the requirement for V4L2 subdev API support.
>
> With this change in place, those drivers will loose that functionality if
> built without CONFIG_VIDEO_V4L2_SUBDEV_API, but will happily work as before
> otherwise. That's probably not exactly what you intended.
Right. Indeed. The old format video ops were converted to use the pad
ops, even for drivers that knew nothing about pads. When writing this
patch, I missed the try_fmt was actually still supported the same way. Not
many drivers use it, but it's still there.
I believe your earlier patchset almost covers the thinkable pitfalls that
could arise from supporting both legacy subdev format and subder pad format
semantics. I'll submit another patch, let's see if people can poke holes
into that one. :-)
>
> > Do the check here so that drivers
> > don't need to.
>
> So only beneficiaries of this change will be V4L2 subdevice drivers which don't
> support V4L2 subdev API and don't support V4L2_SUBDEV_FORMAT_TRY. Existing
> drivers of that kind already have that check in place and I can hardly imagine
> someone is going to optimize them by removing it. Any new drivers will
> probably support V4L2 subdev API from the beginning so they'll not benefit from
> this patch at all as they won't build without CONFIG_VIDEO_V4L2_SUBDEV_API.
>
> I think that if a driver doesn't support V4L2_SUBDEV_FORMAT_TRY, it should
> simply check for that value itself and respond accordingly, no matter if it
> supports V4L2 subdev API or not. As time passes, old drivers will be either
> depreciated or refreshed to handle V4L2_SUBDEV_FORMAT_TRY the V4L2 subdev API
> way, as we would like them to.
>
> Thanks,
> Janusz
>
>
> >
> > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> > Depends-on: ("media: v4l2-subdev: Verify v4l2_subdev_call() pad config
> argument")
> > ---
> > drivers/media/v4l2-core/v4l2-subdev.c | 5 ++++-
> > 1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/
> v4l2-subdev.c
> > index 34219e489be27..88b4b9d7c41be 100644
> > --- a/drivers/media/v4l2-core/v4l2-subdev.c
> > +++ b/drivers/media/v4l2-core/v4l2-subdev.c
> > @@ -122,7 +122,10 @@ static int subdev_close(struct file *file)
> >
> > static inline int check_which(__u32 which)
> > {
> > - if (which != V4L2_SUBDEV_FORMAT_TRY &&
> > + if (
> > +#ifdef CONFIG_VIDEO_V4L2_SUBDEV_API
> > + which != V4L2_SUBDEV_FORMAT_TRY &&
> > +#endif
> > which != V4L2_SUBDEV_FORMAT_ACTIVE)
> > return -EINVAL;
> >
> >
>
>
>
>
--
Regards,
Sakari Ailus
sakari.ailus@linux.intel.com
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2019-06-02 23:09 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-31 11:54 [RESEND PATCH 1/1] v4l2-subdev: Try formats are only available if subdev API is enabled Sakari Ailus
2019-05-31 18:40 ` Janusz Krzysztofik
2019-06-02 23:08 ` Sakari Ailus
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.