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.
Regards,
Hans
