Hi Sergei,

Thank you for the patch.

On Thursday, 11 January 2018 18:54:35 EET Sergei Shtylyov wrote:
> Add support for the R-Car  V3M  (R8A77970) SoC to the DU driver (this SoC
> has only  1 display port). Note that there are some differences  with the
> other R-Car gen3 SoCs in the LVDS encoder part, e.g. LVDPLLCR has the same
> layout  as  on the R-Car gen2 SoCs...
> 
> Signed-off-by: Sergei Shtylyov <sergei.shtyl...@cogentembedded.com>

Could you please rebase this series on top of the LVDS rework posted as 
"[PATCH 00/10] R-Car DU: Convert LVDS code to bridge driver" (https://
www.spinics.net/lists/dri-devel/msg161931.html) ? It should make it easier to 
implement support for V3M. Please then split the DU and LVDS drivers changes 
in two patches.

> ---
>  Documentation/devicetree/bindings/display/renesas,du.txt |    1

Please split the DT bindings changes to a separate patch.

>  drivers/gpu/drm/rcar-du/rcar_du_drv.c                    |   23 +++++++++++
>  drivers/gpu/drm/rcar-du/rcar_du_drv.h                    |    1
>  drivers/gpu/drm/rcar-du/rcar_du_group.c                  |   10 +++---
>  drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c                |   20 +++++++----
>  5 files changed, 45 insertions(+), 10 deletions(-)
> 
> Index: linux/Documentation/devicetree/bindings/display/renesas,du.txt
> ===================================================================
> --- linux.orig/Documentation/devicetree/bindings/display/renesas,du.txt
> +++ linux/Documentation/devicetree/bindings/display/renesas,du.txt
> @@ -13,6 +13,7 @@ Required Properties:
>      - "renesas,du-r8a7794" for R8A7794 (R-Car E2) compatible DU
>      - "renesas,du-r8a7795" for R8A7795 (R-Car H3) compatible DU
>      - "renesas,du-r8a7796" for R8A7796 (R-Car M3-W) compatible DU
> +    - "renesas,du-r8a77970" for R8A77970 (R-Car V3M) compatible DU
> 
>    - reg: A list of base address and length of each memory resource, one for
> each entry in the reg-names property.

You also need to update the ports table further down in this file.

> Index: linux/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> ===================================================================
> --- linux.orig/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> +++ linux/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> @@ -258,6 +258,28 @@ static const struct rcar_du_device_info
>       .dpll_ch =  BIT(1),
>  };
> 
> +static const struct rcar_du_device_info rcar_du_r8a77970_info = {
> +     .gen = 3,
> +     .model = R8A77970,
> +     .features = RCAR_DU_FEATURE_CRTC_IRQ_CLOCK
> +               | RCAR_DU_FEATURE_EXT_CTRL_REGS
> +               | RCAR_DU_FEATURE_VSP1_SOURCE,
> +     .num_crtcs = 1,
> +     .routes = {
> +             /* R8A77970 has one RGB output and one LVDS output. */
> +             [RCAR_DU_OUTPUT_DPAD0] = {
> +                     .possible_crtcs = BIT(0),
> +                     .port = 1,
> +             },
> +             [RCAR_DU_OUTPUT_LVDS0] = {
> +                     .possible_crtcs = BIT(0),
> +                     .port = 0,
> +             },

All the other SoCs have DPAD0 as port 0. Unless there's a specific need for a 
different implementation with V3M I'd keep the same order.

> +     },
> +     .num_lvds = 1,
> +     .dpll_ch  = BIT(1),

This doesn't seem to be correct, there's no DPLL in V3M.

> +};
> +
>  static const struct of_device_id rcar_du_of_table[] = {
>       { .compatible = "renesas,du-r8a7743", .data = &rzg1_du_r8a7743_info },
>       { .compatible = "renesas,du-r8a7745", .data = &rzg1_du_r8a7745_info },
> @@ -269,6 +291,7 @@ static const struct of_device_id rcar_du
>       { .compatible = "renesas,du-r8a7794", .data = &rcar_du_r8a7794_info },
>       { .compatible = "renesas,du-r8a7795", .data = &rcar_du_r8a7795_info },
>       { .compatible = "renesas,du-r8a7796", .data = &rcar_du_r8a7796_info },
> +     { .compatible = "renesas,du-r8a77970", .data = &rcar_du_r8a77970_info },
>       { }
>  };
> Index: linux/drivers/gpu/drm/rcar-du/rcar_du_drv.h
> ===================================================================
> --- linux.orig/drivers/gpu/drm/rcar-du/rcar_du_drv.h
> +++ linux/drivers/gpu/drm/rcar-du/rcar_du_drv.h
> @@ -59,6 +59,7 @@ enum rcar_du_model {
>       R8A7794,
>       R8A7795,
>       R8A7796,
> +     R8A77970,
>  };
> 
>  /*
> Index: linux/drivers/gpu/drm/rcar-du/rcar_du_group.c
> ===================================================================
> --- linux.orig/drivers/gpu/drm/rcar-du/rcar_du_group.c
> +++ linux/drivers/gpu/drm/rcar-du/rcar_du_group.c
> @@ -133,10 +133,12 @@ static void rcar_du_group_setup(struct r
>       rcar_du_group_write(rgrp, DORCR, DORCR_PG1D_DS1 | DORCR_DPRS);
> 
>       /* Apply planes to CRTCs association. */
> -     mutex_lock(&rgrp->lock);
> -     rcar_du_group_write(rgrp, DPTSR, (rgrp->dptsr_planes << 16) |
> -                         rgrp->dptsr_planes);
> -     mutex_unlock(&rgrp->lock);
> +     if (rcdu->info->num_crtcs > 1) {
> +             mutex_lock(&rgrp->lock);
> +             rcar_du_group_write(rgrp, DPTSR, (rgrp->dptsr_planes << 16) |
> +                                 rgrp->dptsr_planes);
> +             mutex_unlock(&rgrp->lock);
> +     }

Shouldn't you skip writing to the DPTSR register if there's a single DPTSR in 
the group ? That would then apply to M3-W as well, which doesn't have the 
DPTSR2 register. I'd split this change to a separate patch.

>  }
> 
>  /*
> Index: linux/drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c
> ===================================================================
> --- linux.orig/drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c
> +++ linux/drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c
> @@ -71,8 +71,10 @@ static int rcar_du_lvdsenc_start(struct
>                                struct rcar_du_crtc *rcrtc)
>  {
>       u32 lvdhcr, lvdpllcr, lvdcr0 = lvds->mode << LVDCR0_LVMD_SHIFT;
> +     const struct rcar_du_device_info *info = lvds->dev->info;
>       const struct drm_display_mode *mode = &rcrtc->crtc.mode;
> -     unsigned int gen = lvds->dev->info->gen;
> +     unsigned int gen = info->gen;
> +     enum rcar_du_model model = info->model;
>       unsigned int freq = mode->clock;
>       int ret;
> 
> @@ -104,7 +106,7 @@ static int rcar_du_lvdsenc_start(struct
>       rcar_lvds_write(lvds, LVDCHCR, lvdhcr);
> 
>       /* PLL clock configuration */
> -     if (gen < 3)
> +     if (gen < 3 || model == R8A77970)
>               lvdpllcr = rcar_lvds_gen2_lvdpllcr(freq);
>       else
>               lvdpllcr = rcar_lvds_gen3_lvdpllcr(freq);
> @@ -136,6 +138,12 @@ static int rcar_du_lvdsenc_start(struct
>               rcar_lvds_write(lvds, LVDCR0, lvdcr0);
>       }
> 
> +     /* Turn on the LVDS PHY on R-Car V3M. */
> +     if (model == R8A77970) {
> +             lvdcr0 |= LVDCR0_LVEN;
> +             rcar_lvds_write(lvds, LVDCR0, lvdcr0);
> +     }
> +
>       /* Wait for the startup delay. */
>       usleep_range(100, 150);
> 
> @@ -177,14 +185,14 @@ int rcar_du_lvdsenc_enable(struct rcar_d
>  void rcar_du_lvdsenc_atomic_check(struct rcar_du_lvdsenc *lvds,
>                                 struct drm_display_mode *mode)
>  {
> -     struct rcar_du_device *rcdu = lvds->dev;
> +     const struct rcar_du_device_info *info = lvds->dev->info;
> 
>       /*
>        * The internal LVDS encoder has a restricted clock frequency operating
> -      * range (30MHz to 150MHz on Gen2, 25.175MHz to 148.5MHz on Gen3). Clamp
> -      * the clock accordingly.
> +      * range (30MHz to 150MHz on Gen2 and R-Car V3M, 25.175MHz to 148.5MHz
> +      * on Gen3). Clamp the clock accordingly.
>        */
> -     if (rcdu->info->gen < 3)
> +     if (info->gen < 3 || info->model == R8A77970)
>               mode->clock = clamp(mode->clock, 30000, 150000);

According to the datasheet the frequency range for V3M is the same as for the 
H3 and M3 SoCs. The range seems to have changed starting in datasheet version 
0.52. I would fix the range in a separate patch first.

If you want I can send patches to fix this issue and the previous one, or you 
can write them and include them in v2. Let me know what you'd prefer.

>       else
>               mode->clock = clamp(mode->clock, 25175, 148500);

-- 
Regards,

Laurent Pinchart

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Reply via email to