Hi Tomi,
Thank you for the patch.
On Fri, May 30, 2025 at 04:50:36PM +0300, Tomi Valkeinen wrote:
> With modern drivers supporting link-freq, we don't need to do any
> calculations based on the bpp and number of lanes when figuring out the
> link frequency. However, the code currently always runs code to get the
> bpp and number of lanes.
>
> Optimize the rcsi2_calc_mbps() so that we only do that when needed, i.e.
> when querying the link-freq is not supported by the upstream subdevice.
>
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@xxxxxxxxxxxxxxxx>
> ---
> drivers/media/platform/renesas/rcar-csi2.c | 50 +++++++++++++++++-------------
> 1 file changed, 29 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/media/platform/renesas/rcar-csi2.c b/drivers/media/platform/renesas/rcar-csi2.c
> index 90973f3cba38..e0a0fd96459b 100644
> --- a/drivers/media/platform/renesas/rcar-csi2.c
> +++ b/drivers/media/platform/renesas/rcar-csi2.c
> @@ -1001,15 +1001,10 @@ static int rcsi2_get_active_lanes(struct rcar_csi2 *priv,
> static int rcsi2_calc_mbps(struct rcar_csi2 *priv,
> struct v4l2_subdev_state *state)
> {
> - const struct rcar_csi2_format *format;
> - struct v4l2_mbus_framefmt *fmt;
> struct media_pad *remote_pad;
> struct v4l2_subdev *source;
> - unsigned int lanes;
> - unsigned int bpp;
> s64 freq;
> u64 mbps;
> - int ret;
>
> if (!priv->remote)
> return -ENODEV;
> @@ -1017,28 +1012,41 @@ static int rcsi2_calc_mbps(struct rcar_csi2 *priv,
> source = priv->remote;
> remote_pad = &source->entity.pads[priv->remote_pad];
>
> - ret = rcsi2_get_active_lanes(priv, &lanes);
> - if (ret)
> - return ret;
> + /*
> + * First try to get the real link freq. If that fails, try the heuristic
> + * method with bpp and lanes (but that only works for one route).
> + */
> + freq = v4l2_get_link_freq(remote_pad, 0, 0);
> + if (freq < 0) {
> + const struct rcar_csi2_format *format;
> + struct v4l2_mbus_framefmt *fmt;
This can be const.
Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@xxxxxxxxxxxxxxxx>
> + unsigned int lanes;
> + unsigned int bpp;
> + int ret;
>
> - fmt = v4l2_subdev_state_get_format(state, RCAR_CSI2_SINK);
> - if (!fmt)
> - return -EINVAL;
> + ret = rcsi2_get_active_lanes(priv, &lanes);
> + if (ret)
> + return ret;
>
> - format = rcsi2_code_to_fmt(fmt->code);
> - if (!format)
> - return -EINVAL;
> + fmt = v4l2_subdev_state_get_format(state, RCAR_CSI2_SINK);
> + if (!fmt)
> + return -EINVAL;
>
> - bpp = format->bpp;
> + format = rcsi2_code_to_fmt(fmt->code);
> + if (!format)
> + return -EINVAL;
>
> - freq = v4l2_get_link_freq(remote_pad, bpp, 2 * lanes);
> - if (freq < 0) {
> - int ret = (int)freq;
> + bpp = format->bpp;
>
> - dev_err(priv->dev, "failed to get link freq for %s: %d\n",
> - source->name, ret);
> + freq = v4l2_get_link_freq(remote_pad, bpp, 2 * lanes);
> + if (freq < 0) {
> + int ret = (int)freq;
>
> - return ret;
> + dev_err(priv->dev, "failed to get link freq for %s: %d\n",
> + source->name, ret);
> +
> + return ret;
> + }
> }
>
> mbps = div_u64(freq * 2, MEGA);
--
Regards,
Laurent Pinchart
