Re: [PATCH v4 4/4] media: platform: Add Renesas RZ/G2L CRU driver

[Lad, Prabhakar]

Hi Sakari,

On Mon, Oct 31, 2022 at 8:19 AM Sakari Ailus
<sakari.ailus@xxxxxxxxxxxxxxx> wrote:
>
> Hi Prabhakar,
>
> On Sun, Oct 30, 2022 at 10:32:43PM +0000, Lad, Prabhakar wrote:
> > Hi Sakari,
> >
> > On Fri, Oct 28, 2022 at 1:22 PM Sakari Ailus
> > <sakari.ailus@xxxxxxxxxxxxxxx> wrote:
> > >
> > > Hi Prabhakar,
> > >
> > > On Thu, Oct 27, 2022 at 08:04:40PM +0100, Lad, Prabhakar wrote:
> > > ...
> > > > > > +static int rzg2l_cru_ip_s_stream(struct v4l2_subdev *sd, int enable)
> > > > > > +{
> > > > > > +     struct rzg2l_cru_dev *cru;
> > > > > > +     int ret;
> > > > > > +
> > > > > > +     cru = v4l2_get_subdevdata(sd);
> > > > > > +
> > > > > > +     if (!cru->is_csi)
> > > > > > +             return -EINVAL;
> > > > > > +
> > > > > > +     ret = v4l2_subdev_call(cru->ip.remote, video, s_stream, enable);
> > > > >
> > > > > It's up to the driver how call pre_streamon() and post_streamoff(), as long
> > > > > as it takes place on both sides of s_stream().
> > > > >
> > > > > In other words, as it seems your device doesn't need anything special, you
> > > > > could waive implemeting the callbacks yourself and call pre_streamon() and
> > > > > post_streamoff() here.
> > > > >
> > > > Here the cru->ip.remote = CSI, in the rzg2l_cru_set_stream(1) where we
> > > > are calling pre_streamon()/post_streamoff() callbacks the subdev is
> > > > CRU-IP. So the calls from rzg2l_cru_set_stream() land into
> > > > rzg2l_cru_ip_pre_streamon() and rzg2l_cru_ip_post_streamoff() which
> > > > are calling pre_streamon/post_streamoff for the CSI subdev.
> > >
> > > Again, you should call the source sub-device's pre_streamon and
> > > post_streamoff from the s_stream handler (not from
> > > rzg2l_cru_ip_pre_streamon or rzg2l_cru_ip_post_streamoff).
> > >
> > > Starting streaming takes place link by link. This allows a driver to omit
> > > implementing pre_streamon and post_streamon callbacks if it doesn't need
> > > them.
> > >
> > Thank you for the explanation that makes sense now to me.
> >
> > Now with this approach the initialization sequence of CSI + CRU won't
> > align as per the HW manual. Unfortunately I'll have to switch back on
> > exporting the functions. I hope that's okay?
>
> It is not.
>
> What exactly would you like to do that you can't with the
> pre_streamon/post_streamoff callbacks called from s_stream?
>
The initialization sequence for MIPI CSI [0]. As per [0] we need to
initialize the CSI2 dphy first then setup the AXI (part of CRU driver)
and then later MIPI CSI2 link (part of csi driver) and lastly turn on
clock and link (in the cru driver).

So as per the current implementation we have the below:
1] CRU IP subdev is calling pre_stream for the CSI2 subdev in its
pre_stream on callback - This is where the CSI2 DPHY is initialized
2] Later in the flow we initialize the AXI part - ie part of
rzg2l_cru_set_stream
3] We call s_stream in rzg2l_cru_set_stream - This lands into CSI2
subdev to initialize the MIPI CSI2 Link
4] In the rzg2l_cru_set_stream we setup up the vclk and enable link reception

[0] https://ibb.co/QpHNkLh

Cheers,
Prabhakar