Hi Geert,
Thanks for the feedback.
> Subject: Re: [PATCH v3 3/3] media: vsp1: Add support for RZ/G2L VSPD
>
> Hi Biju,
>
> On Fri, Mar 11, 2022 at 11:38 AM Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> wrote:
> > The RZ/G2L VSPD provides a single VSPD instance. It has the following
> > sub modules MAU, CTU, RPF, DPR, LUT, BRS, WPF and LIF.
> >
> > The VSPD block on RZ/G2L does not have a version register, so added a
> > new compatible string "renesas,rzg2l-vsp2" with a data pointer
> > containing the info structure. Also the reset line is shared with the DU
> module.
> >
> > Signed-off-by: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> > Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx>
> > ---
> > v2->v3:
> > * Fixed version comparison in vsp1_lookup()
>
> Thanks for the update!
>
> > --- a/drivers/media/platform/vsp1/vsp1_lif.c
> > +++ b/drivers/media/platform/vsp1/vsp1_lif.c
> > @@ -130,13 +131,15 @@ static void lif_configure_stream(struct
> vsp1_entity *entity,
> > VI6_LIF_CTRL_REQSEL | VI6_LIF_CTRL_LIF_EN);
> >
> > /*
> > - * On R-Car V3M the LIF0 buffer attribute register has to be set
> to a
> > - * non-default value to guarantee proper operation (otherwise
> artifacts
> > - * may appear on the output). The value required by the manual
> is not
> > - * explained but is likely a buffer size or threshold.
> > + * On R-Car V3M and RZ/G2L the LIF0 buffer attribute register
> has to be
> > + * set to a non-default value to guarantee proper operation
> (otherwise
> > + * artifacts may appear on the output). The value required by
> the
> > + * manual is not explained but is likely a buffer size or
> threshold.
> > */
> > - if ((entity->vsp1->version & VI6_IP_VERSION_MASK) ==
> > - (VI6_IP_VERSION_MODEL_VSPD_V3 | VI6_IP_VERSION_SOC_V3M))
> > + if (((entity->vsp1->version & VI6_IP_VERSION_MASK) ==
> > + (VI6_IP_VERSION_MODEL_VSPD_V3 | VI6_IP_VERSION_SOC_V3M)) ||
> > + ((entity->vsp1->version & VI6_IP_VERSION_MASK) ==
> > + VI6_IP_VERSION_MODEL_VSPD_RZG2L))
> > vsp1_lif_write(lif, dlb, VI6_LIF_LBA,
> > VI6_LIF_LBA_LBA0 |
> > (1536 << VI6_LIF_LBA_LBA1_SHIFT));
>
> LGTM, although using a feature bit in vsp1_device_info, or just a
> switch() statement here, would probably be better.
I will go with switch statement, as V3M(0x1801) and V3H(0x1802) shares same info
and SoC separation to be done here to identify the V3M.
Cheers,
Biju
>
> Reviewed-by: Geert Uytterhoeven <geert+renesas@xxxxxxxxx>
>
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-
> m68k.org
>
> In personal conversations with technical people, I call myself a hacker.
> But when I'm talking to journalists I just say "programmer" or something
> like that.
> -- Linus Torvalds
