+ Aishwarya
On Mon, Jan 16, 2023 at 03:08:27AM +0200, Laurent Pinchart wrote:
> Hi Philipp,
>
> (CC'ing Francesco)
>
> My apologies for not having noticed the patch.
>
> On Fri, Jan 13, 2023 at 12:24:56PM +0100, Philipp Zabel wrote:
> > Hi,
> >
> > On Wed, Aug 10, 2022 at 12:48:48PM +0200, Philipp Zabel wrote:
> > > Asynchronous subdevice probing on imx-media with imx6-mipi-csi2 is
> > > broken since commit 1f391df44607 ("media: v4l2-async: Use endpoints in
> > > __v4l2_async_nf_add_fwnode_remote()").
> > >
> > > This is a side effect of imx6-mipi-csi2 having a single subdevice with
> > > four separate source ports connected to different subdevices. Before,
> > > the imx-media-csi and video-mux devices registered async sub-devices
> > > via device fwnode that matched the imx6-mipi-csi2 device on their
> > > respective notifiers. This caused the first asd to be put on the
> > > notifier waiting list, and the other three to return -EEXIST and be
> > > ignored.
> > >
> > > With async subdev registration via endpoint fwnode, all four asds are
> > > distinct and three of them keep dangling on their notifiers after the
> > > first one is bound.
> > >
> > > This patch modifies __v4l2_async_nf_has_async_subdev() to consider
> > > asds matching different fwnode endpoints of the same sub-device equal
> > > if the latter is already bound and matches via device fwnode. Further,
> > > v4l2_async_register_subdev() is modified to remove dangling duplicate
> > > asds that were registered before the sub-device was available to check
> > > its fwnode.
>
> To make sure I understand this correctly, you need both changes, with
> the change in __v4l2_async_nf_has_async_subdev() meant to address asds
> being added after the subdev has been registered, and the change in
> v4l2_async_register_subdev() meant to address asds that have been added
> before ?
>
> The imx6 ipu drivers implement a "clever hack" to handle the
> multi-endpoint issue that was never officially supported by v4l2-async.
> Obviously, as it has worked so far, leaving it broken isn't a very nice
> option. The fix feels a bit like a hack though, and a better solution
> would be to allow subdevs to be matched multiple times, by multiple
> consumers. That's a more intrusive change though, so I could be OK with
> this as a short term fix, assuming it doesn't break anything else.
>
> I would however want to ensure this doesn't get abused by new drivers.
> Could we add a dev_warn() somewhere to indicate that multi-endpoint
> matching is not supported and shouldn't be used until fixed ? Sakari,
> what do you think ?
>
> > > Fixes: 1f391df44607 ("media: v4l2-async: Use endpoints in __v4l2_async_nf_add_fwnode_remote()")
> > > Signed-off-by: Philipp Zabel <p.zabel@xxxxxxxxxxxxxx>
> >
> > Any comments on this? The issue persists on v6.2-rc3.
>
> Francesco, does this fix your issue ?
Aishwarya, can you try this patch [0] and verify if it solves the issue?
Just provide a Tested-by tag eventually (and hopefully :-)
Francesco
[0] https://lore.kernel.org/all/20220810104848.846783-1-p.zabel@xxxxxxxxxxxxxx/
