+cc linux-renesas-soc
On Fri, May 09, 2025 at 04:17:34PM +0200, Tommaso Merciai wrote:
> Hi Daniel,
> Thanks for your patch.
>
> On Tue, May 06, 2025 at 01:50:12PM +0100, Daniel Scally wrote:
> > From: Daniel Scally <dan.scally+renesas@xxxxxxxxxxxxxxxx>
> >
> > The rzg2l_csi2_calc_mbps() function currently tries to calculate the
> > link frequency for a CSI2 bus using the V4L2_CID_PIXEL_RATE control
> > of the remote subdevice. Switch the function to v4l2_get_link_freq()
> > which correctly targets V4L2_CID_LINK_FREQ before falling back on
> > V4L2_CID_PIXEL_RATE if the former is unavailable.
> >
> > Reviewed-by: Jacopo Mondi <jacopo.mondi@xxxxxxxxxxxxxxxx>
> > Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@xxxxxxxxxxxxxxxx>
> > Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx>
> > Tested-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx>
> > Signed-off-by: Daniel Scally <dan.scally+renesas@xxxxxxxxxxxxxxxx>
> > ---
> > Changes in v4:
> >
> > - Used separate s64 variable as return value for v4l2_get_link_freq()
> > and as the mbps variable for do_div() to avoid compilation warnings.
> >
> > Changes in v3:
> >
> > - Fixed mbps sign
> >
> > Changes in v2:
> >
> > - None
> >
> > .../platform/renesas/rzg2l-cru/rzg2l-csi2.c | 27 +++++++++----------
> > 1 file changed, 12 insertions(+), 15 deletions(-)
> >
> > diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c
> > index 9243306e2aa9..8870c2cb8104 100644
> > --- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c
> > +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c
> > @@ -282,28 +282,25 @@ static int rzg2l_csi2_calc_mbps(struct rzg2l_csi2 *csi2)
> > const struct rzg2l_csi2_format *format;
> > const struct v4l2_mbus_framefmt *fmt;
> > struct v4l2_subdev_state *state;
> > - struct v4l2_ctrl *ctrl;
> > u64 mbps;
> > -
> > - /* Read the pixel rate control from remote. */
> > - ctrl = v4l2_ctrl_find(source->ctrl_handler, V4L2_CID_PIXEL_RATE);
> > - if (!ctrl) {
> > - dev_err(csi2->dev, "no pixel rate control in subdev %s\n",
> > - source->name);
> > - return -EINVAL;
> > - }
> > + s64 ret;
> >
> > state = v4l2_subdev_lock_and_get_active_state(&csi2->subdev);
> > fmt = v4l2_subdev_state_get_format(state, RZG2L_CSI2_SINK);
> > format = rzg2l_csi2_code_to_fmt(fmt->code);
> > v4l2_subdev_unlock_state(state);
> >
> > - /*
> > - * Calculate hsfreq in Mbps
> > - * hsfreq = (pixel_rate * bits_per_sample) / number_of_lanes
> > - */
> > - mbps = v4l2_ctrl_g_ctrl_int64(ctrl) * format->bpp;
> > - do_div(mbps, csi2->lanes * 1000000);
> > + /* Read the link frequency from remote subdevice. */
> > + ret = v4l2_get_link_freq(source->ctrl_handler, format->bpp,
> > + csi2->lanes);
> > + if (ret < 0) {
> > + dev_err(csi2->dev, "can't retrieve link freq from subdev %s\n",
> > + source->name);
> > + return -EINVAL;
> > + }
> > +
> > + mbps = ret;
> > + do_div(mbps, 1000000);
> >
> > return mbps;
> > }
>
> I tested this series with an imx219 image sensor connected to the CSI-2
> RX IP of RZ/G3E:
>
> Some notes:
>
> - pixel_rate = link_freq * 2 * nr_of_lanes / bits_per_sample
> - hsfreq = (pixel_rate * bits_per_sample) / number_of_lanes
>
> Then hsfreq should be:
>
> - hsfreq = link_freq * 2
>
> Please correct me if I'm wrong.
>
>
> After applying this series. I'm getting the following issue testing the
> imx219 sensor with SRGGB8_1X8 and SGRBG10_1X10 color format.
>
>
> [ 947.305876] rzg2l-cru 16000000.video: Invalid MB address 0xe8bf8300 (out of range)
> [ 947.305876] rzg2l-cru 16000000.video: Invalid MB address 0xe8bf8300 (out of range)
> [ 947.339165] rzg2l-cru 16000000.video: Invalid MB address 0xe8c9e900 (out of range)
> [ 947.339165] rzg2l-cru 16000000.video: Invalid MB address 0xe8c9e900 (out of range)
>
>
> I'm suspecting that this could be related to this formula.
> Let me know what do you think.
>
> Thanks in advance!
>
> Tested using:
>
> root@smarc-rzg3e:~# media-ctl -p
> Media controller API version 6.15.0
>
> Media device information
> ------------------------
> driver rzg2l_cru
> model renesas,r9a09g047-cru
> serial
> bus info platform:16000000.video
> hw revision 0x0
> driver version 6.15.0
>
> Device topology
> - entity 1: csi-16000400.csi2 (2 pads, 2 links, 0 routes)
> type V4L2 subdev subtype Unknown flags 0
> device node name /dev/v4l-subdev0
> pad0: Sink
> [stream:0 fmt:SGRBG10_1X10/640x480 field:none]
> <- "imx219 0-0010":0 [ENABLED,IMMUTABLE]
> pad1: Source
> [stream:0 fmt:SGRBG10_1X10/640x480 field:none]
> -> "cru-ip-16000000.video":0 [ENABLED,IMMUTABLE]
>
> - entity 4: cru-ip-16000000.video (2 pads, 2 links, 0 routes)
> type V4L2 subdev subtype Unknown flags 0
> device node name /dev/v4l-subdev1
> pad0: Sink
> [stream:0 fmt:SGRBG10_1X10/640x480 field:none]
> <- "csi-16000400.csi2":1 [ENABLED,IMMUTABLE]
> pad1: Source
> [stream:0 fmt:SGRBG10_1X10/640x480 field:none]
> -> "CRU output":0 [ENABLED,IMMUTABLE]
>
> - entity 7: imx219 0-0010 (1 pad, 1 link, 0 routes)
> type V4L2 subdev subtype Sensor flags 0
> device node name /dev/v4l-subdev2
> pad0: Source
> [stream:0 fmt:SGRBG10_1X10/640x480 field:none colorspace:raw xfer:none ycbcr:601 quantization:full-range
> crop.bounds:(8,8)/3280x2464
> crop:(1008,760)/1280x960]
> -> "csi-16000400.csi2":0 [ENABLED,IMMUTABLE]
>
> - entity 17: CRU output (1 pad, 1 link)
> type Node subtype V4L flags 0
> device node name /dev/video0
> pad0: Sink
> <- "cru-ip-16000000.video":1 [ENABLED,IMMUTABLE]
>
> # IMX219 TESTING SGRBG10_1X10
> v4l2-ctl -c horizontal_flip=1 -d /dev/v4l-subdev2
>
> media-ctl -d /dev/media0 --set-v4l2 '"imx219 0-0010":0[fmt:SGRBG10_1X10/640x480]'
> media-ctl -d /dev/media0 --set-v4l2 '"csi-16000400.csi2":0[fmt:SGRBG10_1X10/640x480]'
> media-ctl -d /dev/media0 --set-v4l2 '"cru-ip-16000000.video":0[fmt:SGRBG10_1X10/640x480]'
>
> v4l2-ctl -d0 --set-fmt-video pixelformat=CR10,width=640,height=480
> v4l2-ctl -d0 --stream-mmap --stream-count=100
>
>
> # IMX219 TESTING SRGGB8_1X8
> media-ctl -d /dev/media0 --set-v4l2 '"imx219 0-0010":0[fmt:SRGGB8_1X8/1920x1080]'
> media-ctl -d /dev/media0 --set-v4l2 '"csi-16000400.csi2":0[fmt:SRGGB8_1X8/1920x1080]'
> media-ctl -d /dev/media0 --set-v4l2 '"cru-ip-16000000.video":0[fmt:SRGGB8_1X8/1920x1080]'
>
>
> v4l2-ctl -d0 --set-fmt-video pixelformat=RGGB,width=1920,height=1080
> v4l2-ctl -d0 --stream-mmap --stream-count=32
>
> Thanks & Regards,
> Tommaso
>
> > --
> > 2.34.1
> >
>
