Hi Prabhakar, On Mon, Oct 31, 2022 at 12:40:28PM +0000, Lad, Prabhakar wrote: > 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 How is this changed by calling the pre_streamon/post_streamoff callbacks from s_stream? I understand the pipeline device-wise is: ... -> CSI-2 -> CRU I wonder if it would be faster to discuss this on #linux-media. -- Kind regards, Sakari Ailus