diff mbox series

[v8,4/5] media: renesas: vsp1: Add VSP1_HAS_NON_ZERO_LBA feature bit

Message ID 20220419181757.63346-5-biju.das.jz@bp.renesas.com (mailing list archive)
State Superseded
Headers
Series Add support for RZ/G2L VSPD |

Commit Message

Biju Das April 19, 2022, 6:17 p.m. UTC 
  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@bp.renesas.com>
---
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(-)

Comments

Geert Uytterhoeven April 27, 2022, 12:43 p.m. UTC | #1 
Hi Biju,

On Tue, Apr 19, 2022 at 8:18 PM Biju Das <biju.das.jz@bp.renesas.com> 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@bp.renesas.com>
> ---
> 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))

?

> +                               continue;
>                         break;
> +               }
>         }

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
Biju Das April 27, 2022, 1:23 p.m. UTC | #2 
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@bp.renesas.com>
> 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@bp.renesas.com>
> > ---
> > 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;
> > +               }
> >         }
>
diff mbox series

Patch

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 c2e242c503d2..181f2ed8604b 100644
--- 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))
+				continue;
 			break;
+		}
 	}
 
 	if (i == ARRAY_SIZE(vsp1_device_infos)) {
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));