Re: [PATCH 2/3] media: platform: rzg2l-cru: rzg2l-video: Retrieve virtual channel information

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

On Sun, Sep 08, 2024 at 09:23:52PM +0100, Lad, Prabhakar wrote:
> On Sat, Sep 7, 2024 at 10:09 PM Lad, Prabhakar wrote:
> > On Sat, Sep 7, 2024 at 12:10 AM Laurent Pinchart wrote:
>
> <snip>
>
> > > > +
> > > >  int rzg2l_cru_start_image_processing(struct rzg2l_cru_dev *cru)
> > > >  {
> > > >       struct v4l2_mbus_framefmt *fmt = rzg2l_cru_ip_get_src_fmt(cru);
> > > >       unsigned long flags;
> > > >       int ret;
> > > >
> > > > +     ret = rzg2l_cru_get_virtual_channel(cru);
> > > > +     if (ret < 0)
> > > > +             return ret;
> > > > +     cru->csi.channel = ret;
> > >
> > > How about passing the value to the function that needs it, instead of
> > > storing it in cru->csi.channel ? You can do that on top and drop the
> > > csi.channel field.
> >
> > OK, let me check if this can be done.
>
> The virtual channel number is programmed in rzg2l_cru_csi2_setup() [0]
> call, below is the code flow of the call. This code flow is called by
> spinlock held.
> 
> rzg2l_cru_start_image_processing()
>     rzg2l_cru_initialize_image_conv()
>         rzg2l_cru_csi2_setup()
> 
> When rzg2l_cru_get_virtual_channel() is called directly in
> rzg2l_cru_csi2_setup() function we get "BUG: Invalid wait context"
> (when PROVE_LOCKING is enabled) this is due to we do a mutex lock as
> part of v4l2_subdev_lock_and_get_active_state() in get_frame_desc.

I didn't mean calling rzg2l_cru_get_virtual_channel() from
rzg2l_cru_csi2_setup(), but passing the virtual channel number from
rzg2l_cru_start_image_processing() to rzg2l_cru_initialize_image_conv()
and then to rzg2l_cru_csi2_setup().

> So probably I'll leave this as it is now.
> 
> [0] https://elixir.bootlin.com/linux/v6.10.8/source/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c#L311

-- 
Regards,

Laurent Pinchart