Hi Geert,
Thanks for the feedback.
> Subject: Re: [PATCH v8 4/5] media: renesas: vsp1: Add
> VSP1_HAS_NON_ZERO_LBA feature bit
>
> Hi Biju,
>
> On Tue, Apr 19, 2022 at 8:18 PM Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> wrote:
> > As per HW manual V3M and RZ/G2L SoC's has non zero LIF buffer
> > attributes. So introduce a feature bit for handling the same.
> >
> > This patch also add separate device info structure for V3M and V3H
> > SoC, as both these SoC's share the same model ID, but V3H does not
> > have VSP1_HAS_NON_ZERO_LBA feature bit.
> >
> > Signed-off-by: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> > ---
> > v8:
> > * New patch
>
> Thanks for your patch!
>
> > --- a/drivers/media/platform/renesas/vsp1/vsp1_drv.c
> > +++ b/drivers/media/platform/renesas/vsp1/vsp1_drv.c
> > @@ -782,6 +782,7 @@ static const struct vsp1_device_info
> vsp1_device_infos[] = {
> > }, {
> > .version = VI6_IP_VERSION_MODEL_VSPD_V3,
> > .model = "VSP2-D",
> > + .soc = VI6_IP_VERSION_SOC_V3H,
> > .gen = 3,
> > .features = VSP1_HAS_BRS | VSP1_HAS_BRU,
> > .lif_count = 1,
> > @@ -789,6 +790,17 @@ static const struct vsp1_device_info
> vsp1_device_infos[] = {
> > .uif_count = 1,
> > .wpf_count = 1,
> > .num_bru_inputs = 5,
> > + }, {
> > + .version = VI6_IP_VERSION_MODEL_VSPD_V3,
> > + .model = "VSP2-D",
> > + .soc = VI6_IP_VERSION_SOC_V3M,
> > + .gen = 3,
> > + .features = VSP1_HAS_BRS | VSP1_HAS_BRU |
> VSP1_HAS_NON_ZERO_LBA,
> > + .lif_count = 1,
> > + .rpf_count = 5,
> > + .uif_count = 1,
> > + .wpf_count = 1,
> > + .num_bru_inputs = 5,
> > }, {
> > .version = VI6_IP_VERSION_MODEL_VSPDL_GEN3,
> > .model = "VSP2-DL",
> > @@ -832,8 +844,12 @@ static const struct vsp1_device_info
> *vsp1_lookup_info(struct vsp1_device *vsp1)
> > for (i = 0; i < ARRAY_SIZE(vsp1_device_infos); ++i) {
> > info = &vsp1_device_infos[i];
> >
> > - if ((vsp1->version & VI6_IP_VERSION_MODEL_MASK) == info-
> >version)
> > + if ((vsp1->version & VI6_IP_VERSION_MODEL_MASK) == info-
> >version) {
> > + if (info->version ==
> VI6_IP_VERSION_MODEL_VSPD_V3 &&
> > + ((vsp1->version & VI6_IP_VERSION_SOC_MASK)
> > + != info->soc))
>
> This check looks fragile to me.
> What about
>
> if (info->soc && (vsp1->version & VI6_IP_VERSION_SOC_MASK) != info-
> >soc))
>
> ?
Fine by me , as the unique value is 'Model + SoC' and we already matched 'Model,
above. So far, SoCs with LBA HW feature has nonzero 'SoC' value.
Therefore, the generic 'SoC' check is fine here.
Cheers,
Biju
>
> > + continue;
> > break;
> > + }
> > }
>
