[PATCH 1/2] drm: rcar-du: Restrict DPLL duty cycle workaround to H3 ES1.x

[PATCH 1/2] drm: rcar-du: Restrict DPLL duty cycle workaround to H3 ES1.x

[Laurent Pinchart]

The H3 ES1.x exhibits dot clock duty cycle stability issues. We can work
around them by configuring the DPLL to twice the desired frequency,
coupled with a /2 post-divider. This isn't needed on other SoCs and
breaks HDMI output on M3-W for a currently unknown reason, so restrict
the workaround to H3 ES1.x.

>From an implementation point of view, move work around handling outside
of the rcar_du_dpll_divider() function by requesting a x2 DPLL output
frequency explicitly. The existing post-divider calculation mechanism
will then take care of dividing the clock by two automatically.

While at it, print a more useful debugging message to ease debugging
clock rate issues.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/gpu/drm/rcar-du/rcar_du_crtc.c | 37 +++++++++++++++++++++++++---------
 1 file changed, 27 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
index 4ed6f2340af0..9ae0d7b5fb69 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
@@ -13,6 +13,7 @@
 
 #include <linux/clk.h>
 #include <linux/mutex.h>
+#include <linux/sys_soc.h>
 
 #include <drm/drmP.h>
 #include <drm/drm_atomic.h>
@@ -129,10 +130,8 @@ static void rcar_du_dpll_divider(struct rcar_du_crtc *rcrtc,
 			for (fdpll = 1; fdpll < 32; fdpll++) {
 				unsigned long output;
 
-				/* 1/2 (FRQSEL=1) for duty rate 50% */
 				output = input * (n + 1) / (m + 1)
-				       / (fdpll + 1) / 2;
-
+				       / (fdpll + 1);
 				if (output >= 400000000)
 					continue;
 
@@ -158,6 +157,11 @@ static void rcar_du_dpll_divider(struct rcar_du_crtc *rcrtc,
 		 best_diff);
 }
 
+static const struct soc_device_attribute rcar_du_r8a7795_es1[] = {
+	{ .soc_id = "r8a7795", .revision = "ES1.*" },
+	{ /* sentinel */ }
+};
+
 static void rcar_du_crtc_set_display_timing(struct rcar_du_crtc *rcrtc)
 {
 	const struct drm_display_mode *mode = &rcrtc->crtc.state->adjusted_mode;
@@ -185,7 +189,20 @@ static void rcar_du_crtc_set_display_timing(struct rcar_du_crtc *rcrtc)
 
 		extclk = clk_get_rate(rcrtc->extclock);
 		if (rcdu->info->dpll_ch & (1 << rcrtc->index)) {
-			rcar_du_dpll_divider(rcrtc, &dpll, extclk, mode_clock);
+			unsigned long target = mode_clock;
+
+			/*
+			 * The H3 ES1.x exhibits dot clock duty cycle stability
+			 * issues. We can work around them by configuring the
+			 * DPLL to twice the desired frequency, coupled with a
+			 * /2 post-divider. This isn't needed on other SoCs and
+			 * breaks HDMI output on M3-W for a currently unknown
+			 * reason, so restrict the workaround to H3 ES1.x.
+			 */
+			if (soc_device_match(rcar_du_r8a7795_es1))
+				target *= 2;
+
+			rcar_du_dpll_divider(rcrtc, &dpll, extclk, target);
 			extclk = dpll.output;
 		}
 
@@ -197,8 +214,6 @@ static void rcar_du_crtc_set_display_timing(struct rcar_du_crtc *rcrtc)
 
 		if (abs((long)extrate - (long)mode_clock) <
 		    abs((long)rate - (long)mode_clock)) {
-			dev_dbg(rcrtc->group->dev->dev,
-				"crtc%u: using external clock\n", rcrtc->index);
 
 			if (rcdu->info->dpll_ch & (1 << rcrtc->index)) {
 				u32 dpllcr = DPLLCR_CODE | DPLLCR_CLKE
@@ -215,12 +230,14 @@ static void rcar_du_crtc_set_display_timing(struct rcar_du_crtc *rcrtc)
 
 				rcar_du_group_write(rcrtc->group, DPLLCR,
 						    dpllcr);
-
-				escr = ESCR_DCLKSEL_DCLKIN | 1;
-			} else {
-				escr = ESCR_DCLKSEL_DCLKIN | extdiv;
 			}
+
+			escr = ESCR_DCLKSEL_DCLKIN | extdiv;
 		}
+
+		dev_dbg(rcrtc->group->dev->dev,
+			"mode clock %lu extrate %lu rate %lu ESCR 0x%08x\n",
+			mode_clock, extrate, rate, escr);
 	}
 
 	rcar_du_group_write(rcrtc->group, rcrtc->index % 2 ? ESCR2 : ESCR,
-- 
Regards,

Laurent Pinchart

[PATCH 2/2] drm: rcar-du: Add HDMI outputs to R8A7796 device description

[Laurent Pinchart]

Update the device description with the HDMI output.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/gpu/drm/rcar-du/rcar_du_drv.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.c b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
index d6a0255181cc..fc0ae0947b8f 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_drv.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
@@ -180,19 +180,25 @@ static const struct rcar_du_device_info rcar_du_r8a7796_info = {
 		  | RCAR_DU_FEATURE_VSP1_SOURCE,
 	.num_crtcs = 3,
 	.routes = {
-		/* R8A7796 has one RGB output, one LVDS output and one
-		 * (currently unsupported) HDMI output.
+		/*
+		 * R8A7796 has one RGB output, one LVDS output and one HDMI
+		 * output.
 		 */
 		[RCAR_DU_OUTPUT_DPAD0] = {
 			.possible_crtcs = BIT(2),
 			.port = 0,
 		},
+		[RCAR_DU_OUTPUT_HDMI0] = {
+			.possible_crtcs = BIT(1),
+			.port = 1,
+		},
 		[RCAR_DU_OUTPUT_LVDS0] = {
 			.possible_crtcs = BIT(0),
 			.port = 2,
 		},
 	},
 	.num_lvds = 1,
+	.dpll_ch =  BIT(1),
 };
 
 static const struct of_device_id rcar_du_of_table[] = {
-- 
Regards,

Laurent Pinchart

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

Re: [PATCH 1/2] drm: rcar-du: Restrict DPLL duty cycle workaround to H3 ES1.x

[Geert Uytterhoeven]

Hi Laurent,

On Wed, Jun 21, 2017 at 11:04 AM, Laurent Pinchart
<laurent.pinchart+renesas@ideasonboard.com> wrote:
> --- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> @@ -158,6 +157,11 @@ static void rcar_du_dpll_divider(struct rcar_du_crtc *rcrtc,
>                  best_diff);
>  }
>
> +static const struct soc_device_attribute rcar_du_r8a7795_es1[] = {
> +       { .soc_id = "r8a7795", .revision = "ES1.*" },
> +       { /* sentinel */ }
> +};
> +
>  static void rcar_du_crtc_set_display_timing(struct rcar_du_crtc *rcrtc)
>  {
>         const struct drm_display_mode *mode = &rcrtc->crtc.state->adjusted_mode;
> @@ -185,7 +189,20 @@ static void rcar_du_crtc_set_display_timing(struct rcar_du_crtc *rcrtc)
>
>                 extclk = clk_get_rate(rcrtc->extclock);
>                 if (rcdu->info->dpll_ch & (1 << rcrtc->index)) {
> -                       rcar_du_dpll_divider(rcrtc, &dpll, extclk, mode_clock);
> +                       unsigned long target = mode_clock;
> +
> +                       /*
> +                        * The H3 ES1.x exhibits dot clock duty cycle stability
> +                        * issues. We can work around them by configuring the
> +                        * DPLL to twice the desired frequency, coupled with a
> +                        * /2 post-divider. This isn't needed on other SoCs and
> +                        * breaks HDMI output on M3-W for a currently unknown
> +                        * reason, so restrict the workaround to H3 ES1.x.
> +                        */
> +                       if (soc_device_match(rcar_du_r8a7795_es1))

Matching is done over and over again, on every display enable/resume.
Can it be done at probe time?
Or in rcar_du_crtc_create(), which has access to rcrtc?

> +                               target *= 2;

You can also store the multiplier in rcar_du_r8a7795_es1.data
(store 1 in .data of the sentinel), removing the need for an if clause.

> +
> +                       rcar_du_dpll_divider(rcrtc, &dpll, extclk, target);
>                         extclk = dpll.output;

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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/2] drm: rcar-du: Add HDMI outputs to R8A7796 device description

[Kieran Bingham]

Hi Laurent,

This looks good to me.

Table 35.1 (in the DU datasheet) certainly shows that there is only an HDMI-IF0
on the M3, but it's amusing that (and I was confused by the fact that) my
r8a7796 board (Salvator-X) still has the HDMI1 populated. Of course I presume
this is populated to keep the boards the same but is not connected in the chip.

Anyway, that's irrelevant to this patch so ...

On 21/06/17 10:04, Laurent Pinchart wrote:
> Update the device description with the HDMI output.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

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

> ---
>  drivers/gpu/drm/rcar-du/rcar_du_drv.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.c b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> index d6a0255181cc..fc0ae0947b8f 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> @@ -180,19 +180,25 @@ static const struct rcar_du_device_info rcar_du_r8a7796_info = {
>  		  | RCAR_DU_FEATURE_VSP1_SOURCE,
>  	.num_crtcs = 3,
>  	.routes = {
> -		/* R8A7796 has one RGB output, one LVDS output and one
> -		 * (currently unsupported) HDMI output.
> +		/*
> +		 * R8A7796 has one RGB output, one LVDS output and one HDMI
> +		 * output.
>  		 */
>  		[RCAR_DU_OUTPUT_DPAD0] = {
>  			.possible_crtcs = BIT(2),
>  			.port = 0,
>  		},
> +		[RCAR_DU_OUTPUT_HDMI0] = {
> +			.possible_crtcs = BIT(1),
> +			.port = 1,
> +		},
>  		[RCAR_DU_OUTPUT_LVDS0] = {
>  			.possible_crtcs = BIT(0),
>  			.port = 2,
>  		},
>  	},
>  	.num_lvds = 1,
> +	.dpll_ch =  BIT(1),
>  };
>  
>  static const struct of_device_id rcar_du_of_table[] = {
> 

Re: [PATCH 2/2] drm: rcar-du: Add HDMI outputs to R8A7796 device description

[Geert Uytterhoeven]

Hi Kieran,

On Wed, Jul 12, 2017 at 3:51 PM, Kieran Bingham
<kieran.bingham+renesas@ideasonboard.com> wrote:
> Table 35.1 (in the DU datasheet) certainly shows that there is only an HDMI-IF0
> on the M3, but it's amusing that (and I was confused by the fact that) my
> r8a7796 board (Salvator-X) still has the HDMI1 populated. Of course I presume
> this is populated to keep the boards the same but is not connected in the chip.

Indeed.

BTW, the M3-W version also has unconnected USB3 and SATA connectors.

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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/2] drm: rcar-du: Add HDMI outputs to R8A7796 device description

[Kieran Bingham]

Hi Geert,

> Indeed.
> 
> BTW, the M3-W version also has unconnected USB3 and SATA connectors.

Thanks for the heads up :) - I've just put a post it note over those, +HDMI1-OUT
(or rather the text on the top lid) to prevent any confusion for me down the line.

--
Kieran