diff mbox series

[v6,3/3] media: renesas: vsp1: Add support for RZ/G2L VSPD

Message ID 20220316115551.29222-4-biju.das.jz@bp.renesas.com (mailing list archive)
State New, archived
Headers show
Series Add support for RZ/G2L VSPD | expand

Commit Message

Biju Das March 16, 2022, 11:55 a.m. UTC
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@bp.renesas.com>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
v5->v6:
 * Rebased to media_staging and updated commit header
 * Removed the extra tab from rzg2l_vsp2_device_info
 * Changed the function vsp1_lookup->vsp1_lookup_info and
   all info match related code moved here.
 * Add VI6_IP_VERSION_VSP and VI6_IP_VERSION_VSP_SW macros to
   distinguish HW & SW IP_VSP_Version.
 * Used 0x80 for RZG2L VSPD model and SoC identification
 * Updated Switch() for LIF0 buffer attribute handling.
v4->v5:
 * Fixed typo VI6_IP_VERSION_MODEL_MASK->VI6_IP_VERSION_MASK
 * To be consistent with other SoC's, introduced VI6_IP_VERSION_SOC_G2L
   for RZ/G2L SoC's.
v3->v4:
 * Added Rb tag from Geert
 * Add switch() for LIF0 buffer attribute handling for RZ/G2L and V3M
v2->v3:
 * Fixed version comparison in vsp1_lookup()
v1->v2:
 * Changed the compatible from vsp2-rzg2l->rzg2l-vsp2
 * Added standalone device info for rzg2l-vsp2.
 * Added vsp1_lookup helper function.
 * Updated comments for LIF0 buffer attribute register
 * Used last ID for rzg2l-vsp2.
RFC->v1:
 * Used data pointer containing info structure to retrieve version information
RFC:
 * https://patchwork.kernel.org/project/linux-renesas-soc/patch/20220112174612.10773-21-biju.das.jz@bp.renesas.com/
---
 .../media/platform/renesas/vsp1/vsp1_drv.c    | 56 ++++++++++++++-----
 .../media/platform/renesas/vsp1/vsp1_lif.c    | 18 ++++--
 .../media/platform/renesas/vsp1/vsp1_regs.h   |  8 +++
 3 files changed, 62 insertions(+), 20 deletions(-)

Comments

Kieran Bingham April 14, 2022, 8:39 a.m. UTC | #1
Quoting Biju Das (2022-03-16 11:55:51)
> 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@bp.renesas.com>
> Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
> v5->v6:
>  * Rebased to media_staging and updated commit header
>  * Removed the extra tab from rzg2l_vsp2_device_info
>  * Changed the function vsp1_lookup->vsp1_lookup_info and
>    all info match related code moved here.
>  * Add VI6_IP_VERSION_VSP and VI6_IP_VERSION_VSP_SW macros to
>    distinguish HW & SW IP_VSP_Version.
>  * Used 0x80 for RZG2L VSPD model and SoC identification
>  * Updated Switch() for LIF0 buffer attribute handling.
> v4->v5:
>  * Fixed typo VI6_IP_VERSION_MODEL_MASK->VI6_IP_VERSION_MASK
>  * To be consistent with other SoC's, introduced VI6_IP_VERSION_SOC_G2L
>    for RZ/G2L SoC's.
> v3->v4:
>  * Added Rb tag from Geert
>  * Add switch() for LIF0 buffer attribute handling for RZ/G2L and V3M
> v2->v3:
>  * Fixed version comparison in vsp1_lookup()
> v1->v2:
>  * Changed the compatible from vsp2-rzg2l->rzg2l-vsp2
>  * Added standalone device info for rzg2l-vsp2.
>  * Added vsp1_lookup helper function.
>  * Updated comments for LIF0 buffer attribute register
>  * Used last ID for rzg2l-vsp2.
> RFC->v1:
>  * Used data pointer containing info structure to retrieve version information
> RFC:
>  * https://patchwork.kernel.org/project/linux-renesas-soc/patch/20220112174612.10773-21-biju.das.jz@bp.renesas.com/
> ---
>  .../media/platform/renesas/vsp1/vsp1_drv.c    | 56 ++++++++++++++-----
>  .../media/platform/renesas/vsp1/vsp1_lif.c    | 18 ++++--
>  .../media/platform/renesas/vsp1/vsp1_regs.h   |  8 +++
>  3 files changed, 62 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/media/platform/renesas/vsp1/vsp1_drv.c b/drivers/media/platform/renesas/vsp1/vsp1_drv.c
> index 159b68fa0829..f1f52c0c1c59 100644
> --- a/drivers/media/platform/renesas/vsp1/vsp1_drv.c
> +++ b/drivers/media/platform/renesas/vsp1/vsp1_drv.c
> @@ -812,11 +812,47 @@ static const struct vsp1_device_info vsp1_device_infos[] = {
>         },
>  };
>  
> +static const struct vsp1_device_info rzg2l_vsp2_device_info = {
> +       .version = VI6_IP_VERSION_MODEL_VSPD_RZG2L,
> +       .model = "VSP2-D",
> +       .gen = 3,
> +       .features = VSP1_HAS_BRS | VSP1_HAS_WPF_VFLIP | VSP1_HAS_EXT_DL,
> +       .lif_count = 1,
> +       .rpf_count = 2,
> +       .wpf_count = 1,
> +};
> +
> +static const struct vsp1_device_info *vsp1_lookup_info(struct vsp1_device *vsp1)
> +{
> +       const struct vsp1_device_info *info;
> +       unsigned int i;
> +
> +       /*
> +        * Try the info stored in match data first for devices that don't have
> +        * a version register.
> +        */
> +       info = of_device_get_match_data(vsp1->dev);
> +       if (info)

Presumably - as this will not call vsp1_read(vsp1, VI6_IP_VERSION), we
could/should always set vsp1->version here, or'ing in the _SW flag with
the derived version and SoC identifiers from the info structure.

> +               return info;
> +
> +       vsp1->version = vsp1_read(vsp1, VI6_IP_VERSION);
> +
> +       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)
> +                       return info;
> +       }
> +
> +       dev_err(vsp1->dev, "unsupported IP version 0x%08x\n", vsp1->version);
> +
> +       return NULL;
> +}
> +
>  static int vsp1_probe(struct platform_device *pdev)
>  {
>         struct vsp1_device *vsp1;
>         struct device_node *fcp_node;
> -       unsigned int i;
>         int ret;
>         int irq;
>  
> @@ -872,25 +908,16 @@ static int vsp1_probe(struct platform_device *pdev)
>         if (ret < 0)
>                 goto done;
>  
> -       vsp1->version = vsp1_read(vsp1, VI6_IP_VERSION);
> -
> -       for (i = 0; i < ARRAY_SIZE(vsp1_device_infos); ++i) {
> -               if ((vsp1->version & VI6_IP_VERSION_MODEL_MASK) ==
> -                   vsp1_device_infos[i].version) {
> -                       vsp1->info = &vsp1_device_infos[i];
> -                       break;
> -               }
> -       }
> -
> +       vsp1->info = vsp1_lookup_info(vsp1);
>         if (!vsp1->info) {
> -               dev_err(&pdev->dev, "unsupported IP version 0x%08x\n",
> -                       vsp1->version);
>                 vsp1_device_put(vsp1);
>                 ret = -ENXIO;
>                 goto done;
>         }
>  
> -       dev_dbg(&pdev->dev, "IP version 0x%08x\n", vsp1->version);
> +       if ((vsp1->version & VI6_IP_VERSION_VSP_MASK) != VI6_IP_VERSION_VSP)
> +               vsp1->version = VI6_IP_VERSION_VSP_SW | vsp1->info->version |
> +                               VI6_IP_VERSION_SOC_RZG2L;

It seems odd to have this specific version assignment here. Shouldn't
that be set during vsp1_lookup_info() in the case that there is a match
from of_device_get_match_data()? That way it would be extendable by
adding just a new vsp1_device_info structure for the next platform that
has this issue. This implies that they will 'always' be RZG2L but that
information should live in the vsp1_device_info structure I think.

Could be handled when/if we get a new device added I guess, but I think
that VI6_IP_VERSION_SOC_RZG2L should be something that is retrieved from
the vsp1_device_info structure.

Re-reading the vsp1_lookup_info() function - it does seem like something
suited to there, as the vsp1->version is never read from hardware in the
new case.

>  
>         /*
>          * Previous use of the hardware (e.g. by the bootloader) could leave
> @@ -941,6 +968,7 @@ static int vsp1_remove(struct platform_device *pdev)
>  static const struct of_device_id vsp1_of_match[] = {
>         { .compatible = "renesas,vsp1" },
>         { .compatible = "renesas,vsp2" },
> +       { .compatible = "renesas,rzg2l-vsp2", .data = &rzg2l_vsp2_device_info },
>         { },
>  };
>  MODULE_DEVICE_TABLE(of, vsp1_of_match);
> diff --git a/drivers/media/platform/renesas/vsp1/vsp1_lif.c b/drivers/media/platform/renesas/vsp1/vsp1_lif.c
> index 6a6857ac9327..e36ed2d2b22b 100644
> --- a/drivers/media/platform/renesas/vsp1/vsp1_lif.c
> +++ b/drivers/media/platform/renesas/vsp1/vsp1_lif.c
> @@ -107,6 +107,7 @@ static void lif_configure_stream(struct vsp1_entity *entity,
>  
>         case VI6_IP_VERSION_MODEL_VSPDL_GEN3:
>         case VI6_IP_VERSION_MODEL_VSPD_V3:
> +       case VI6_IP_VERSION_MODEL_VSPD_RZG2L:
>                 hbth = 0;
>                 obth = 1500;
>                 lbth = 0;
> @@ -130,16 +131,21 @@ 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))
> +       switch (entity->vsp1->version) {
> +       case (VI6_IP_VERSION_VSP | VI6_IP_VERSION_MODEL_VSPD_V3 |
> +             VI6_IP_VERSION_SOC_V3M):
> +       case (VI6_IP_VERSION_VSP_SW | VI6_IP_VERSION_MODEL_VSPD_RZG2L |
> +             VI6_IP_VERSION_SOC_RZG2L):

If this is going to grow - I would think it would be better served with
a feature flag - although this isn't so much of a feature, and more of a
quirk, so I wonder if that would push us closer to getting a quirks
flag.

I'm weary that this may not scale otherwise, but ... for now this works,
but I think it means we have multiple ways of handling platform specific
code already.


>                 vsp1_lif_write(lif, dlb, VI6_LIF_LBA,
>                                VI6_LIF_LBA_LBA0 |
>                                (1536 << VI6_LIF_LBA_LBA1_SHIFT));
> +               break;
> +       }
>  }
>  
>  static const struct vsp1_entity_operations lif_entity_ops = {
> diff --git a/drivers/media/platform/renesas/vsp1/vsp1_regs.h b/drivers/media/platform/renesas/vsp1/vsp1_regs.h
> index fae7286eb01e..e66553c42e50 100644
> --- a/drivers/media/platform/renesas/vsp1/vsp1_regs.h
> +++ b/drivers/media/platform/renesas/vsp1/vsp1_regs.h
> @@ -767,6 +767,8 @@
>  #define VI6_IP_VERSION_MODEL_VSPDL_GEN3        (0x19 << 8)
>  #define VI6_IP_VERSION_MODEL_VSPBS_GEN3        (0x1a << 8)
>  #define VI6_IP_VERSION_MODEL_VSPD_V3U  (0x1c << 8)
> +/* RZ/G2L SoC's have no version register, So use 0x80 as the model version */
> +#define VI6_IP_VERSION_MODEL_VSPD_RZG2L        (0x80 << 8)
>  
>  #define VI6_IP_VERSION_SOC_MASK                (0xff << 0)
>  #define VI6_IP_VERSION_SOC_H2          (0x01 << 0)
> @@ -780,6 +782,12 @@
>  #define VI6_IP_VERSION_SOC_M3N         (0x04 << 0)
>  #define VI6_IP_VERSION_SOC_E3          (0x04 << 0)
>  #define VI6_IP_VERSION_SOC_V3U         (0x05 << 0)
> +/* RZ/G2L SoC's have no version register, So use 0x80 for SoC Identification */
> +#define VI6_IP_VERSION_SOC_RZG2L       (0x80 << 0)
> +
> +#define VI6_IP_VERSION_VSP_MASK                (0xffff << 16)
> +#define VI6_IP_VERSION_VSP             (0x0101 << 16) /* HW VSP version */

Is this constant on all supported platforms? both Gen2 and Gen3? (Is
there a gen1?). Does it need to be specified to the generation?

There's nothing specifically complex there or blocking I don't think -
so with comments considered as required:

Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>

> +#define VI6_IP_VERSION_VSP_SW          (0xfffe << 16) /* SW VSP version */
>  
>  /* -----------------------------------------------------------------------------
>   * RPF CLUT Registers
> -- 
> 2.17.1
>
Biju Das April 14, 2022, 11:34 a.m. UTC | #2
Hi Kieran,

Thanks for the feedback.

> Subject: Re: [PATCH v6 3/3] media: renesas: vsp1: Add support for RZ/G2L
> VSPD
> 
> Quoting Biju Das (2022-03-16 11:55:51)
> > 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@bp.renesas.com>
> > Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > ---
> > v5->v6:
> >  * Rebased to media_staging and updated commit header
> >  * Removed the extra tab from rzg2l_vsp2_device_info
> >  * Changed the function vsp1_lookup->vsp1_lookup_info and
> >    all info match related code moved here.
> >  * Add VI6_IP_VERSION_VSP and VI6_IP_VERSION_VSP_SW macros to
> >    distinguish HW & SW IP_VSP_Version.
> >  * Used 0x80 for RZG2L VSPD model and SoC identification
> >  * Updated Switch() for LIF0 buffer attribute handling.
> > v4->v5:
> >  * Fixed typo VI6_IP_VERSION_MODEL_MASK->VI6_IP_VERSION_MASK
> >  * To be consistent with other SoC's, introduced VI6_IP_VERSION_SOC_G2L
> >    for RZ/G2L SoC's.
> > v3->v4:
> >  * Added Rb tag from Geert
> >  * Add switch() for LIF0 buffer attribute handling for RZ/G2L and V3M
> > v2->v3:
> >  * Fixed version comparison in vsp1_lookup()
> > v1->v2:
> >  * Changed the compatible from vsp2-rzg2l->rzg2l-vsp2
> >  * Added standalone device info for rzg2l-vsp2.
> >  * Added vsp1_lookup helper function.
> >  * Updated comments for LIF0 buffer attribute register
> >  * Used last ID for rzg2l-vsp2.
> > RFC->v1:
> >  * Used data pointer containing info structure to retrieve version
> > information
> > RFC:
> > ---
> >  .../media/platform/renesas/vsp1/vsp1_drv.c    | 56 ++++++++++++++-----
> >  .../media/platform/renesas/vsp1/vsp1_lif.c    | 18 ++++--
> >  .../media/platform/renesas/vsp1/vsp1_regs.h   |  8 +++
> >  3 files changed, 62 insertions(+), 20 deletions(-)
> >
> > diff --git a/drivers/media/platform/renesas/vsp1/vsp1_drv.c
> > b/drivers/media/platform/renesas/vsp1/vsp1_drv.c
> > index 159b68fa0829..f1f52c0c1c59 100644
> > --- a/drivers/media/platform/renesas/vsp1/vsp1_drv.c
> > +++ b/drivers/media/platform/renesas/vsp1/vsp1_drv.c
> > @@ -812,11 +812,47 @@ static const struct vsp1_device_info
> vsp1_device_infos[] = {
> >         },
> >  };
> >
> > +static const struct vsp1_device_info rzg2l_vsp2_device_info = {
> > +       .version = VI6_IP_VERSION_MODEL_VSPD_RZG2L,
> > +       .model = "VSP2-D",
> > +       .gen = 3,
> > +       .features = VSP1_HAS_BRS | VSP1_HAS_WPF_VFLIP | VSP1_HAS_EXT_DL,
> > +       .lif_count = 1,
> > +       .rpf_count = 2,
> > +       .wpf_count = 1,
> > +};
> > +
> > +static const struct vsp1_device_info *vsp1_lookup_info(struct
> > +vsp1_device *vsp1) {
> > +       const struct vsp1_device_info *info;
> > +       unsigned int i;
> > +
> > +       /*
> > +        * Try the info stored in match data first for devices that
> don't have
> > +        * a version register.
> > +        */
> > +       info = of_device_get_match_data(vsp1->dev);
> > +       if (info)
> 
> Presumably - as this will not call vsp1_read(vsp1, VI6_IP_VERSION), we
> could/should always set vsp1->version here, or'ing in the _SW flag with
> the derived version and SoC identifiers from the info structure.

OK, I have prototyped as per your suggestion, and it looks good.

Here it is

      if (info) {
              vsp1->quirks = LIF_BUF_ATTR_QUIRKS;
              vsp1->version = VI6_IP_VERSION_VSP_SW | info->version |
                              VI6_IP_VERSION_SOC_RZG2L;



> 
> > +               return info;
> > +
> > +       vsp1->version = vsp1_read(vsp1, VI6_IP_VERSION);
> > +
> > +       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)

Here it is 
              if ((vsp1->version & VI6_IP_VERSION_MODEL_MASK) == info->version) {
                       if ((vsp1->version & VI6_IP_VERSION_MASK) ==
                         (VI6_IP_VERSION_MODEL_VSPD_V3 | VI6_IP_VERSION_SOC_V3M))
                               vsp1->quirks = LIF_BUF_ATTR_QUIRKS;


> > +                       return info;
> > +       }
> > +
> > +       dev_err(vsp1->dev, "unsupported IP version 0x%08x\n",
> > + vsp1->version);
> > +
> > +       return NULL;
> > +}
> > +
> >  static int vsp1_probe(struct platform_device *pdev)  {
> >         struct vsp1_device *vsp1;
> >         struct device_node *fcp_node;
> > -       unsigned int i;
> >         int ret;
> >         int irq;
> >
> > @@ -872,25 +908,16 @@ static int vsp1_probe(struct platform_device
> *pdev)
> >         if (ret < 0)
> >                 goto done;
> >
> > -       vsp1->version = vsp1_read(vsp1, VI6_IP_VERSION);
> > -
> > -       for (i = 0; i < ARRAY_SIZE(vsp1_device_infos); ++i) {
> > -               if ((vsp1->version & VI6_IP_VERSION_MODEL_MASK) ==
> > -                   vsp1_device_infos[i].version) {
> > -                       vsp1->info = &vsp1_device_infos[i];
> > -                       break;
> > -               }
> > -       }
> > -
> > +       vsp1->info = vsp1_lookup_info(vsp1);
> >         if (!vsp1->info) {
> > -               dev_err(&pdev->dev, "unsupported IP version 0x%08x\n",
> > -                       vsp1->version);
> >                 vsp1_device_put(vsp1);
> >                 ret = -ENXIO;
> >                 goto done;
> >         }
> >
> > -       dev_dbg(&pdev->dev, "IP version 0x%08x\n", vsp1->version);
> > +       if ((vsp1->version & VI6_IP_VERSION_VSP_MASK) !=
> VI6_IP_VERSION_VSP)
> > +               vsp1->version = VI6_IP_VERSION_VSP_SW | vsp1->info-
> >version |
> > +                               VI6_IP_VERSION_SOC_RZG2L;
> 
> It seems odd to have this specific version assignment here. Shouldn't that
> be set during vsp1_lookup_info() in the case that there is a match from
> of_device_get_match_data()? That way it would be extendable by adding just
> a new vsp1_device_info structure for the next platform that has this
> issue. This implies that they will 'always' be RZG2L but that information
> should live in the vsp1_device_info structure I think.
> 

We can remove this assignment from here and move vsp1_lookup_info.

> Could be handled when/if we get a new device added I guess, but I think
> that VI6_IP_VERSION_SOC_RZG2L should be something that is retrieved from
> the vsp1_device_info structure.
> 
> Re-reading the vsp1_lookup_info() function - it does seem like something
> suited to there, as the vsp1->version is never read from hardware in the
> new case.

Ok, Agreed.

> 
> >
> >         /*
> >          * Previous use of the hardware (e.g. by the bootloader) could
> > leave @@ -941,6 +968,7 @@ static int vsp1_remove(struct
> > platform_device *pdev)  static const struct of_device_id vsp1_of_match[]
> = {
> >         { .compatible = "renesas,vsp1" },
> >         { .compatible = "renesas,vsp2" },
> > +       { .compatible = "renesas,rzg2l-vsp2", .data =
> > + &rzg2l_vsp2_device_info },
> >         { },
> >  };
> >  MODULE_DEVICE_TABLE(of, vsp1_of_match); diff --git
> > a/drivers/media/platform/renesas/vsp1/vsp1_lif.c
> > b/drivers/media/platform/renesas/vsp1/vsp1_lif.c
> > index 6a6857ac9327..e36ed2d2b22b 100644
> > --- a/drivers/media/platform/renesas/vsp1/vsp1_lif.c
> > +++ b/drivers/media/platform/renesas/vsp1/vsp1_lif.c
> > @@ -107,6 +107,7 @@ static void lif_configure_stream(struct
> > vsp1_entity *entity,
> >
> >         case VI6_IP_VERSION_MODEL_VSPDL_GEN3:
> >         case VI6_IP_VERSION_MODEL_VSPD_V3:
> > +       case VI6_IP_VERSION_MODEL_VSPD_RZG2L:
> >                 hbth = 0;
> >                 obth = 1500;
> >                 lbth = 0;
> > @@ -130,16 +131,21 @@ 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))
> > +       switch (entity->vsp1->version) {
> > +       case (VI6_IP_VERSION_VSP | VI6_IP_VERSION_MODEL_VSPD_V3 |
> > +             VI6_IP_VERSION_SOC_V3M):
> > +       case (VI6_IP_VERSION_VSP_SW | VI6_IP_VERSION_MODEL_VSPD_RZG2L |
> > +             VI6_IP_VERSION_SOC_RZG2L):
> 
> If this is going to grow - I would think it would be better served with a
> feature flag - although this isn't so much of a feature, and more of a
> quirk, so I wonder if that would push us closer to getting a quirks flag.
> 
> I'm weary that this may not scale otherwise, but ... for now this works,
> but I think it means we have multiple ways of handling platform specific
> code already.

Here it is

if (lif->quirks)
          vsp1_lif_write(lif, dlb, VI6_LIF_LBA,
                           VI6_LIF_LBA_LBA0 |
                            (1536 << VI6_LIF_LBA_LBA1_SHIFT));

And the below change in vsp1_lif_create

  lif->quirks = vsp1->quirks & LIF_BUF_ATTR_QUIRKS;

> 
> 
> >                 vsp1_lif_write(lif, dlb, VI6_LIF_LBA,
> >                                VI6_LIF_LBA_LBA0 |
> >                                (1536 << VI6_LIF_LBA_LBA1_SHIFT));
> > +               break;
> > +       }
> >  }
> >
> >  static const struct vsp1_entity_operations lif_entity_ops = { diff
> > --git a/drivers/media/platform/renesas/vsp1/vsp1_regs.h
> > b/drivers/media/platform/renesas/vsp1/vsp1_regs.h
> > index fae7286eb01e..e66553c42e50 100644
> > --- a/drivers/media/platform/renesas/vsp1/vsp1_regs.h
> > +++ b/drivers/media/platform/renesas/vsp1/vsp1_regs.h
> > @@ -767,6 +767,8 @@
> >  #define VI6_IP_VERSION_MODEL_VSPDL_GEN3        (0x19 << 8)
> >  #define VI6_IP_VERSION_MODEL_VSPBS_GEN3        (0x1a << 8)
> >  #define VI6_IP_VERSION_MODEL_VSPD_V3U  (0x1c << 8)
> > +/* RZ/G2L SoC's have no version register, So use 0x80 as the model
> version */
> > +#define VI6_IP_VERSION_MODEL_VSPD_RZG2L        (0x80 << 8)
> >
> >  #define VI6_IP_VERSION_SOC_MASK                (0xff << 0)
> >  #define VI6_IP_VERSION_SOC_H2          (0x01 << 0)
> > @@ -780,6 +782,12 @@
> >  #define VI6_IP_VERSION_SOC_M3N         (0x04 << 0)
> >  #define VI6_IP_VERSION_SOC_E3          (0x04 << 0)
> >  #define VI6_IP_VERSION_SOC_V3U         (0x05 << 0)
> > +/* RZ/G2L SoC's have no version register, So use 0x80 for SoC
> Identification */
> > +#define VI6_IP_VERSION_SOC_RZG2L       (0x80 << 0)
> > +
> > +#define VI6_IP_VERSION_VSP_MASK                (0xffff << 16)
> > +#define VI6_IP_VERSION_VSP             (0x0101 << 16) /* HW VSP version
> */
> 
> Is this constant on all supported platforms? both Gen2 and Gen3? (Is there
> a gen1?). Does it need to be specified to the generation?

I have checked Gen1 and Gen2 HW manual I don't find this info. So I would like
to remove this macro as it is unused after quirk changes.

I am planning to send V7 with these changes, please let me know if you have any feedback.

Cheers,
Biju

> 
> There's nothing specifically complex there or blocking I don't think - so
> with comments considered as required:
> 
> Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> 
> > +#define VI6_IP_VERSION_VSP_SW          (0xfffe << 16) /* SW VSP version
> */
> >
> >  /* --------------------------------------------------------------------
> ---------
> >   * RPF CLUT Registers
> > --
> > 2.17.1
> >
diff mbox series

Patch

diff --git a/drivers/media/platform/renesas/vsp1/vsp1_drv.c b/drivers/media/platform/renesas/vsp1/vsp1_drv.c
index 159b68fa0829..f1f52c0c1c59 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_drv.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_drv.c
@@ -812,11 +812,47 @@  static const struct vsp1_device_info vsp1_device_infos[] = {
 	},
 };
 
+static const struct vsp1_device_info rzg2l_vsp2_device_info = {
+	.version = VI6_IP_VERSION_MODEL_VSPD_RZG2L,
+	.model = "VSP2-D",
+	.gen = 3,
+	.features = VSP1_HAS_BRS | VSP1_HAS_WPF_VFLIP | VSP1_HAS_EXT_DL,
+	.lif_count = 1,
+	.rpf_count = 2,
+	.wpf_count = 1,
+};
+
+static const struct vsp1_device_info *vsp1_lookup_info(struct vsp1_device *vsp1)
+{
+	const struct vsp1_device_info *info;
+	unsigned int i;
+
+	/*
+	 * Try the info stored in match data first for devices that don't have
+	 * a version register.
+	 */
+	info = of_device_get_match_data(vsp1->dev);
+	if (info)
+		return info;
+
+	vsp1->version = vsp1_read(vsp1, VI6_IP_VERSION);
+
+	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)
+			return info;
+	}
+
+	dev_err(vsp1->dev, "unsupported IP version 0x%08x\n", vsp1->version);
+
+	return NULL;
+}
+
 static int vsp1_probe(struct platform_device *pdev)
 {
 	struct vsp1_device *vsp1;
 	struct device_node *fcp_node;
-	unsigned int i;
 	int ret;
 	int irq;
 
@@ -872,25 +908,16 @@  static int vsp1_probe(struct platform_device *pdev)
 	if (ret < 0)
 		goto done;
 
-	vsp1->version = vsp1_read(vsp1, VI6_IP_VERSION);
-
-	for (i = 0; i < ARRAY_SIZE(vsp1_device_infos); ++i) {
-		if ((vsp1->version & VI6_IP_VERSION_MODEL_MASK) ==
-		    vsp1_device_infos[i].version) {
-			vsp1->info = &vsp1_device_infos[i];
-			break;
-		}
-	}
-
+	vsp1->info = vsp1_lookup_info(vsp1);
 	if (!vsp1->info) {
-		dev_err(&pdev->dev, "unsupported IP version 0x%08x\n",
-			vsp1->version);
 		vsp1_device_put(vsp1);
 		ret = -ENXIO;
 		goto done;
 	}
 
-	dev_dbg(&pdev->dev, "IP version 0x%08x\n", vsp1->version);
+	if ((vsp1->version & VI6_IP_VERSION_VSP_MASK) != VI6_IP_VERSION_VSP)
+		vsp1->version = VI6_IP_VERSION_VSP_SW | vsp1->info->version |
+				VI6_IP_VERSION_SOC_RZG2L;
 
 	/*
 	 * Previous use of the hardware (e.g. by the bootloader) could leave
@@ -941,6 +968,7 @@  static int vsp1_remove(struct platform_device *pdev)
 static const struct of_device_id vsp1_of_match[] = {
 	{ .compatible = "renesas,vsp1" },
 	{ .compatible = "renesas,vsp2" },
+	{ .compatible = "renesas,rzg2l-vsp2", .data = &rzg2l_vsp2_device_info },
 	{ },
 };
 MODULE_DEVICE_TABLE(of, vsp1_of_match);
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_lif.c b/drivers/media/platform/renesas/vsp1/vsp1_lif.c
index 6a6857ac9327..e36ed2d2b22b 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_lif.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_lif.c
@@ -107,6 +107,7 @@  static void lif_configure_stream(struct vsp1_entity *entity,
 
 	case VI6_IP_VERSION_MODEL_VSPDL_GEN3:
 	case VI6_IP_VERSION_MODEL_VSPD_V3:
+	case VI6_IP_VERSION_MODEL_VSPD_RZG2L:
 		hbth = 0;
 		obth = 1500;
 		lbth = 0;
@@ -130,16 +131,21 @@  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))
+	switch (entity->vsp1->version) {
+	case (VI6_IP_VERSION_VSP | VI6_IP_VERSION_MODEL_VSPD_V3 |
+	      VI6_IP_VERSION_SOC_V3M):
+	case (VI6_IP_VERSION_VSP_SW | VI6_IP_VERSION_MODEL_VSPD_RZG2L |
+	      VI6_IP_VERSION_SOC_RZG2L):
 		vsp1_lif_write(lif, dlb, VI6_LIF_LBA,
 			       VI6_LIF_LBA_LBA0 |
 			       (1536 << VI6_LIF_LBA_LBA1_SHIFT));
+		break;
+	}
 }
 
 static const struct vsp1_entity_operations lif_entity_ops = {
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_regs.h b/drivers/media/platform/renesas/vsp1/vsp1_regs.h
index fae7286eb01e..e66553c42e50 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_regs.h
+++ b/drivers/media/platform/renesas/vsp1/vsp1_regs.h
@@ -767,6 +767,8 @@ 
 #define VI6_IP_VERSION_MODEL_VSPDL_GEN3	(0x19 << 8)
 #define VI6_IP_VERSION_MODEL_VSPBS_GEN3	(0x1a << 8)
 #define VI6_IP_VERSION_MODEL_VSPD_V3U	(0x1c << 8)
+/* RZ/G2L SoC's have no version register, So use 0x80 as the model version */
+#define VI6_IP_VERSION_MODEL_VSPD_RZG2L	(0x80 << 8)
 
 #define VI6_IP_VERSION_SOC_MASK		(0xff << 0)
 #define VI6_IP_VERSION_SOC_H2		(0x01 << 0)
@@ -780,6 +782,12 @@ 
 #define VI6_IP_VERSION_SOC_M3N		(0x04 << 0)
 #define VI6_IP_VERSION_SOC_E3		(0x04 << 0)
 #define VI6_IP_VERSION_SOC_V3U		(0x05 << 0)
+/* RZ/G2L SoC's have no version register, So use 0x80 for SoC Identification */
+#define VI6_IP_VERSION_SOC_RZG2L	(0x80 << 0)
+
+#define VI6_IP_VERSION_VSP_MASK		(0xffff << 16)
+#define VI6_IP_VERSION_VSP		(0x0101 << 16) /* HW VSP version */
+#define VI6_IP_VERSION_VSP_SW		(0xfffe << 16) /* SW VSP version */
 
 /* -----------------------------------------------------------------------------
  * RPF CLUT Registers