On Tue, Oct 10, 2023 at 03:01:53PM +0200, Hans Verkuil wrote:
> On 10/10/23 14:24, Sakari Ailus wrote:
> > Hi Hans,
> >
> > On Tue, Oct 10, 2023 at 01:52:21PM +0200, Hans Verkuil wrote:
> >> On 10/10/23 13:49, Sakari Ailus wrote:
> >>> Hi Hans,
> >>>
> >>> On Tue, Oct 10, 2023 at 12:24:58PM +0200, Hans de Goede wrote:
> >>>> Since the stream API is still experimental it is currently locked away
> >>>> behind the internal, default disabled, v4l2_subdev_enable_streams_api flag.
> >>>>
> >>>> Advertising V4L2_SUBDEV_CAP_STREAMS when the streams API is disabled
> >>>> confuses userspace. E.g. it causes the following libcamera error:
> >>>>
> >>>> ERROR SimplePipeline simple.cpp:1497 Failed to reset routes for
> >>>> /dev/v4l-subdev1: Inappropriate ioctl for device
> >>>>
> >>>> Don't report V4L2_SUBDEV_CAP_STREAMS when the streams API is disabled
> >>>> to avoid problems like this.
> >>>>
> >>>> Reported-by: Dennis Bonke <admin@xxxxxxxxxxxxxxx>
> >>>> Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx>
> >>>> ---
> >>>> -Clearing the V4L2_SUBDEV_FL_STREAMS flag from sd.flags might seem
> >>>> appealing as an alternative fix. But this causes various v4l2-core bits
> >>>> to enter different code paths which confuses drivers which set
> >>>> V4L2_SUBDEV_FL_STREAMS, so this is a bad idea.
> >>>
> >>> Thanks, this apparently had been missed while disabling the API.
> >>>
> >>> Probably also should be added:
> >>>
> >>> Fixes: 9a6b5bf4c1bb ("media: add V4L2_SUBDEV_CAP_STREAMS")
> >>> Cc: stable@xxxxxxxxxxxxxxx # for >= 6.3
> >>>
> >>> Also cc'd Tomi.
> >>
> >> Should this be queued up as a 6.6 fix?
> >
> > I wonder what the criteria is these days.
> >
> > I'd think it's unlikely anything is or will be broken by this in practice.
> > The further this exists, though, increases the likelihood for that to
> > happen.
> >
>
> 1) Regressions: i.e. it worked before, but no longer in v6.6.
> 2) Compilation errors, typically due to Kconfig problems.
> 3) For new code that appeared in v6.6: serious bugs causing kernel oopses
> or other bad behavior during normal use. (I.e., the 'Oh shit, I never
> tested that!' bugs)
>
> Generally a lot of these fixes deal with error paths etc., those can
> often wait for the next kernel.
>
> There are no doubt more reasons for considering patches for v6.6, but those
> three are no-brainers...
>
> And there is of course a gray area where you could lean either way.
>
> For this particular patch it would affect imx8-isi-crossbar.c in 6.4 onwards,
> and starting with 6.6 it is also used in the ds90ub9xx drivers according to
> git grep.
I think it'd be better to get it to 6.6 right away. If you take this,
please add:
Acked-by: Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx>
--
Sakari Ailus
