| Message ID | 20220825132144.2619239-5-biju.das.jz@bp.renesas.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add support for RZ/G2L VSPD | expand |
Hi Biju, Thank you for the patch. On Thu, Aug 25, 2022 at 02:21:43PM +0100, Biju Das wrote: > As per HW manual V3M and RZ/G2L SoCs has nonzero LIF buffer > attributes. So, introduce a feature bit for handling the same. > > This patch also adds separate device info structure for V3M and V3H > SoCs, as both these SoCs 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@bp.renesas.com> > --- > v12->v13: > * No change > v11->v12: > * No change > v10->v11: > * No change > v9->v10: > * No change > v8->v9: > * Used generic check for matching SoCs with LBA feature. > v8: > * New patch > --- > drivers/media/platform/renesas/vsp1/vsp1.h | 1 + > drivers/media/platform/renesas/vsp1/vsp1_drv.c | 18 +++++++++++++++++- > drivers/media/platform/renesas/vsp1/vsp1_lif.c | 3 +-- > 3 files changed, 19 insertions(+), 3 deletions(-) > > diff --git a/drivers/media/platform/renesas/vsp1/vsp1.h b/drivers/media/platform/renesas/vsp1/vsp1.h > index ff4435705abb..2f6f0c6ae555 100644 > --- a/drivers/media/platform/renesas/vsp1/vsp1.h > +++ b/drivers/media/platform/renesas/vsp1/vsp1.h > @@ -55,6 +55,7 @@ struct vsp1_uif; > #define VSP1_HAS_HGT BIT(8) > #define VSP1_HAS_BRS BIT(9) > #define VSP1_HAS_EXT_DL BIT(10) > +#define VSP1_HAS_NON_ZERO_LBA BIT(11) > > struct vsp1_device_info { > u32 version; > diff --git a/drivers/media/platform/renesas/vsp1/vsp1_drv.c b/drivers/media/platform/renesas/vsp1/vsp1_drv.c > index 223dd5f557ba..c0d814d9044e 100644 > --- a/drivers/media/platform/renesas/vsp1/vsp1_drv.c > +++ b/drivers/media/platform/renesas/vsp1/vsp1_drv.c > @@ -787,6 +787,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, > @@ -794,6 +795,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", > @@ -837,8 +849,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->soc && > + ((vsp1->version & VI6_IP_VERSION_SOC_MASK) != info->soc)) > + continue; > return info; > + } How about making this more readable ? u32 model; u32 soc; ... model = vsp1->version & VI6_IP_VERSION_MODEL_MASK; soc = vsp1->version & VI6_IP_VERSION_SOC_MASK; for (i = 0; i < ARRAY_SIZE(vsp1_device_infos); ++i) { info = &vsp1_device_infos[i]; if (model == info->version && (!info->soc || soc == info->soc)) return info; } Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > } > > dev_err(vsp1->dev, "unsupported IP version 0x%08x\n", vsp1->version); > diff --git a/drivers/media/platform/renesas/vsp1/vsp1_lif.c b/drivers/media/platform/renesas/vsp1/vsp1_lif.c > index 6a6857ac9327..9adb892edcdc 100644 > --- a/drivers/media/platform/renesas/vsp1/vsp1_lif.c > +++ b/drivers/media/platform/renesas/vsp1/vsp1_lif.c > @@ -135,8 +135,7 @@ static void lif_configure_stream(struct vsp1_entity *entity, > * 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 (vsp1_feature(entity->vsp1, VSP1_HAS_NON_ZERO_LBA)) > vsp1_lif_write(lif, dlb, VI6_LIF_LBA, > VI6_LIF_LBA_LBA0 | > (1536 << VI6_LIF_LBA_LBA1_SHIFT));
Hi Laurent, Thanks for the feedback. > Subject: Re: [PATCH v13 4/5] media: renesas: vsp1: Add > VSP1_HAS_NON_ZERO_LBA feature bit > > Hi Biju, > > Thank you for the patch. > > On Thu, Aug 25, 2022 at 02:21:43PM +0100, Biju Das wrote: > > As per HW manual V3M and RZ/G2L SoCs has nonzero LIF buffer > > attributes. So, introduce a feature bit for handling the same. > > > > This patch also adds separate device info structure for V3M and V3H > > SoCs, as both these SoCs 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@bp.renesas.com> > > --- > > v12->v13: > > * No change > > v11->v12: > > * No change > > v10->v11: > > * No change > > v9->v10: > > * No change > > v8->v9: > > * Used generic check for matching SoCs with LBA feature. > > v8: > > * New patch > > --- > > drivers/media/platform/renesas/vsp1/vsp1.h | 1 + > > drivers/media/platform/renesas/vsp1/vsp1_drv.c | 18 > > +++++++++++++++++- drivers/media/platform/renesas/vsp1/vsp1_lif.c | > > 3 +-- > > 3 files changed, 19 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/media/platform/renesas/vsp1/vsp1.h > > b/drivers/media/platform/renesas/vsp1/vsp1.h > > index ff4435705abb..2f6f0c6ae555 100644 > > --- a/drivers/media/platform/renesas/vsp1/vsp1.h > > +++ b/drivers/media/platform/renesas/vsp1/vsp1.h > > @@ -55,6 +55,7 @@ struct vsp1_uif; > > #define VSP1_HAS_HGT BIT(8) > > #define VSP1_HAS_BRS BIT(9) > > #define VSP1_HAS_EXT_DL BIT(10) > > +#define VSP1_HAS_NON_ZERO_LBA BIT(11) > > > > struct vsp1_device_info { > > u32 version; > > diff --git a/drivers/media/platform/renesas/vsp1/vsp1_drv.c > > b/drivers/media/platform/renesas/vsp1/vsp1_drv.c > > index 223dd5f557ba..c0d814d9044e 100644 > > --- a/drivers/media/platform/renesas/vsp1/vsp1_drv.c > > +++ b/drivers/media/platform/renesas/vsp1/vsp1_drv.c > > @@ -787,6 +787,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, > > @@ -794,6 +795,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", > > @@ -837,8 +849,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->soc && > > + ((vsp1->version & VI6_IP_VERSION_SOC_MASK) != > info->soc)) > > + continue; > > return info; > > + } > > How about making this more readable ? > > u32 model; > u32 soc; > > ... > > model = vsp1->version & VI6_IP_VERSION_MODEL_MASK; > soc = vsp1->version & VI6_IP_VERSION_SOC_MASK; > > for (i = 0; i < ARRAY_SIZE(vsp1_device_infos); ++i) { > info = &vsp1_device_infos[i]; > > if (model == info->version && (!info->soc || soc == info- > >soc)) > return info; > } OK, it is much better logic. Will fix this in next version. Cheers, Biju > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > > } > > > > dev_err(vsp1->dev, "unsupported IP version 0x%08x\n", > > vsp1->version); diff --git > > a/drivers/media/platform/renesas/vsp1/vsp1_lif.c > > b/drivers/media/platform/renesas/vsp1/vsp1_lif.c > > index 6a6857ac9327..9adb892edcdc 100644 > > --- a/drivers/media/platform/renesas/vsp1/vsp1_lif.c > > +++ b/drivers/media/platform/renesas/vsp1/vsp1_lif.c > > @@ -135,8 +135,7 @@ static void lif_configure_stream(struct > vsp1_entity *entity, > > * 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 (vsp1_feature(entity->vsp1, VSP1_HAS_NON_ZERO_LBA)) > > vsp1_lif_write(lif, dlb, VI6_LIF_LBA, > > VI6_LIF_LBA_LBA0 | > > (1536 << VI6_LIF_LBA_LBA1_SHIFT)); > > -- > Regards, > > Laurent Pinchart
diff --git a/drivers/media/platform/renesas/vsp1/vsp1.h b/drivers/media/platform/renesas/vsp1/vsp1.h index ff4435705abb..2f6f0c6ae555 100644 --- a/drivers/media/platform/renesas/vsp1/vsp1.h +++ b/drivers/media/platform/renesas/vsp1/vsp1.h @@ -55,6 +55,7 @@ struct vsp1_uif; #define VSP1_HAS_HGT BIT(8) #define VSP1_HAS_BRS BIT(9) #define VSP1_HAS_EXT_DL BIT(10) +#define VSP1_HAS_NON_ZERO_LBA BIT(11) struct vsp1_device_info { u32 version; diff --git a/drivers/media/platform/renesas/vsp1/vsp1_drv.c b/drivers/media/platform/renesas/vsp1/vsp1_drv.c index 223dd5f557ba..c0d814d9044e 100644 --- a/drivers/media/platform/renesas/vsp1/vsp1_drv.c +++ b/drivers/media/platform/renesas/vsp1/vsp1_drv.c @@ -787,6 +787,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, @@ -794,6 +795,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", @@ -837,8 +849,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->soc && + ((vsp1->version & VI6_IP_VERSION_SOC_MASK) != info->soc)) + continue; return info; + } } dev_err(vsp1->dev, "unsupported IP version 0x%08x\n", vsp1->version); diff --git a/drivers/media/platform/renesas/vsp1/vsp1_lif.c b/drivers/media/platform/renesas/vsp1/vsp1_lif.c index 6a6857ac9327..9adb892edcdc 100644 --- a/drivers/media/platform/renesas/vsp1/vsp1_lif.c +++ b/drivers/media/platform/renesas/vsp1/vsp1_lif.c @@ -135,8 +135,7 @@ static void lif_configure_stream(struct vsp1_entity *entity, * 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 (vsp1_feature(entity->vsp1, VSP1_HAS_NON_ZERO_LBA)) vsp1_lif_write(lif, dlb, VI6_LIF_LBA, VI6_LIF_LBA_LBA0 | (1536 << VI6_LIF_LBA_LBA1_SHIFT));
As per HW manual V3M and RZ/G2L SoCs has nonzero LIF buffer attributes. So, introduce a feature bit for handling the same. This patch also adds separate device info structure for V3M and V3H SoCs, as both these SoCs 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@bp.renesas.com> --- v12->v13: * No change v11->v12: * No change v10->v11: * No change v9->v10: * No change v8->v9: * Used generic check for matching SoCs with LBA feature. v8: * New patch --- drivers/media/platform/renesas/vsp1/vsp1.h | 1 + drivers/media/platform/renesas/vsp1/vsp1_drv.c | 18 +++++++++++++++++- drivers/media/platform/renesas/vsp1/vsp1_lif.c | 3 +-- 3 files changed, 19 insertions(+), 3 deletions(-)