Re: [PROTO][PATCH 04/10] drm: rcar-du: lvds: LVDS PLL support

Hi Ulrich,

Thank you for the patch.

On Tuesday, 14 August 2018 16:49:58 EEST Ulrich Hecht wrote:

In R-Car D3 and E3, the DU dot clock can be sourced from the LVDS PLL. This patch enables that PLL if present.

Based on patch by Koji Matsuoka koji.matsuoka.xm@renesas.com.

Signed-off-by: Ulrich Hecht uli+renesas@fpond.eu

drivers/gpu/drm/rcar-du/rcar_du_crtc.c | 3 + drivers/gpu/drm/rcar-du/rcar_du_crtc.h | 2 + drivers/gpu/drm/rcar-du/rcar_du_drv.c | 3 +- drivers/gpu/drm/rcar-du/rcar_du_drv.h | 1 + drivers/gpu/drm/rcar-du/rcar_du_group.c | 11 +- drivers/gpu/drm/rcar-du/rcar_lvds.c | 212 ++++++++++++++++++++++++++++ drivers/gpu/drm/rcar-du/rcar_lvds_regs.h | 46 ++++++-

The DU and LVDS encoder changes should be split in two patches.

7 files changed, 274 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c index 9153e7a..a903456 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c +++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c @@ -275,6 +275,9 @@ static void rcar_du_crtc_set_display_timing(struct rcar_du_crtc *rcrtc) mode_clock, extrate, rate, escr); }

• if (rcar_du_has(rcdu, RCAR_DU_FEATURE_LVDS_PLL))

• escr = 0;
  

• rcar_du_group_write(rcrtc->group, rcrtc->index % 2 ? ESCR2 : ESCR, escr); rcar_du_group_write(rcrtc->group, rcrtc->index % 2 ? OTAR2 : OTAR, 0);

diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.h b/drivers/gpu/drm/rcar-du/rcar_du_crtc.h index 7680cb2..65de551 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.h +++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.h @@ -44,6 +44,7 @@ struct rcar_du_vsp;

• @group: CRTC group this CRTC belongs to
• @vsp: VSP feeding video to this CRTC
• @vsp_pipe: index of the VSP pipeline feeding video to this CRTC

• • @lvds_ch: index of LVDS
  */

struct rcar_du_crtc { struct drm_crtc crtc; @@ -67,6 +68,7 @@ struct rcar_du_crtc { struct rcar_du_group *group; struct rcar_du_vsp *vsp; unsigned int vsp_pipe;

• int lvds_ch;

This field is never set or used.

};

#define to_rcar_crtc(c) container_of(c, struct rcar_du_crtc, crtc) diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.c b/drivers/gpu/drm/rcar-du/rcar_du_drv.c index d930996..3338ef5 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_drv.c +++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.c @@ -299,7 +299,8 @@ static const struct rcar_du_device_info rcar_du_r8a77995_info = { .features = RCAR_DU_FEATURE_CRTC_IRQ_CLOCK | RCAR_DU_FEATURE_EXT_CTRL_REGS | RCAR_DU_FEATURE_VSP1_SOURCE

•   | RCAR_DU_FEATURE_R8A77995_REGS,
  

•   | RCAR_DU_FEATURE_R8A77995_REGS
  

•   | RCAR_DU_FEATURE_LVDS_PLL,
  

  .quirks = RCAR_DU_QUIRK_TVM_MASTER_ONLY, .channels_mask = BIT(1) | BIT(0), .routes = {

diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.h b/drivers/gpu/drm/rcar-du/rcar_du_drv.h index 9355b58..6009b7d 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_drv.h +++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.h @@ -32,6 +32,7 @@ struct rcar_du_device; #define RCAR_DU_FEATURE_VSP1_SOURCE (1 << 2) /* Has inputs from VSP1 */ #define RCAR_DU_FEATURE_R8A77965_REGS (1 << 3) /* Use R8A77965 registers

*/

#define RCAR_DU_FEATURE_R8A77995_REGS (1 << 4) /* Use R8A77995 registers

*/

+#define RCAR_DU_FEATURE_LVDS_PLL (1 << 5) /* Use PLL in LVDS */

The feature bit should tell if the LVDS encore has a PLL. Whether to use it or not should be a runtime decision.

#define RCAR_DU_QUIRK_ALIGN_128B (1 << 0) /* Align pitches to 128 bytes

*/

#define RCAR_DU_QUIRK_TVM_MASTER_ONLY (1 << 1) /* Does not have TV switch/sync modes */ diff --git a/drivers/gpu/drm/rcar-du/rcar_du_group.c b/drivers/gpu/drm/rcar-du/rcar_du_group.c index 371d780..44681e3 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_group.c +++ b/drivers/gpu/drm/rcar-du/rcar_du_group.c @@ -126,8 +126,15 @@ static void rcar_du_group_setup(struct rcar_du_group *rgrp) * are setup through per-group registers, only available when * the group has two channels. */

• if ((rcdu->info->gen < 3 && rgrp->index == 0) ||
  

•     (rcdu->info->gen == 3 &&  rgrp->num_crtcs > 1))
  

• if (rcar_du_has(rcdu, RCAR_DU_FEATURE_LVDS_PLL))
  

• 	rcar_du_group_write(rgrp,
  

• 			    DIDSR, DIDSR_CODE |
  

• 			    DIDSR_LCDS_LVDS0(1) |
  

• 			    DIDSR_LCDS_LVDS0(0) |
  

• 			    DIDSR_PDCS_CLK(1, 0) |
  

• 			    DIDSR_PDCS_CLK(0, 0));
  

• else if ((rcdu->info->gen < 3 && rgrp->index == 0) ||
  

• 	 (rcdu->info->gen == 3 &&  rgrp->num_crtcs > 1))
  rcar_du_group_write(rgrp, DIDSR, DIDSR_CODE);
  

  }

diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c b/drivers/gpu/drm/rcar-du/rcar_lvds.c index 4c39de3..cd55576 100644 --- a/drivers/gpu/drm/rcar-du/rcar_lvds.c +++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c @@ -23,6 +23,8 @@ #include <drm/drm_panel.h>

#include "rcar_lvds_regs.h" +#include "rcar_du_crtc.h" +#include "rcar_du_drv.h"

That's a layering violation that I'd like to avoid.

/* Keep in sync with the LVDCR0.LVMD hardware register values. */ enum rcar_lvds_mode { @@ -65,6 +67,15 @@ struct rcar_lvds { #define connector_to_rcar_lvds(connector) \ container_of(connector, struct rcar_lvds, connector)

+struct pll_info {

• unsigned int pllclk;
• unsigned int diff;
• unsigned int clk_n;
• unsigned int clk_m;
• unsigned int clk_e;
• unsigned int div;

+};

static void rcar_lvds_write(struct rcar_lvds *lvds, u32 reg, u32 data) { iowrite32(data, lvds->mmio + reg); @@ -155,6 +166,198 @@ static u32 rcar_lvds_lvdpllcr_gen3(unsigned int freq) return LVDPLLCR_PLLDIVCNT_148M; }

+static void rcar_lvds_pll_calc(struct rcar_du_crtc *rcrtc,

• 		     struct pll_info *pll, unsigned int in_fre,
  

• 		     unsigned int mode_freq, bool edivider)
  

+{

• unsigned long diff = (unsigned long)-1;
• unsigned long fout, fpfd, fvco, n, m, e, div;
• bool clk_diff_set = true;
• if (in_fre < 12000000 || in_fre > 192000000)

• return;
  

• for (n = 0; n < 127; n++) {

• if (n + 1 < 60 || n + 1 > 120)
  

• 	continue;
  

Seriously ? :-)

• for (m = 0; m < 7; m++) {
  

• 	for (e = 0; e < 1; e++) {
  

• 		if (edivider)
  

• 			fout = (((in_fre / 1000) * (n + 1)) /
  

• 				((m + 1) * (e + 1) * 2)) *
  

• 				1000;
  

• 		else
  

• 			fout = (((in_fre / 1000) * (n + 1)) /
  

• 				(m + 1)) * 1000;
  

• 		if (fout > 1039500000)
  

• 			continue;
  

• 		fpfd  = (in_fre / (m + 1));
  

• 		if (fpfd < 12000000 || fpfd > 24000000)
  

• 			continue;
  

• 		fvco  = (((in_fre / 1000) * (n + 1)) / (m + 1))
  

• 			 * 1000;
  

• 		if (fvco < 900000000 || fvco > 1800000000)
  

• 			continue;
  

• 		fout = fout / 7; /* 7 divider */
  

• 		for (div = 0; div < 64; div++) {
  

That's a lot of iterations for the four nested loops, surely it can be simplified.

• 			diff = abs((long)(fout / (div + 1)) -
  

• 			       (long)mode_freq);
  

• 			if (clk_diff_set ||
  

• 			    (diff == 0 ||
  

• 			    pll->diff > diff)) {
  

• 				pll->diff = diff;
  

• 				pll->clk_n = n;
  

• 				pll->clk_m = m;
  

• 				pll->clk_e = e;
  

• 				pll->pllclk = fout;
  

• 				pll->div = div;
  

• 				clk_diff_set = false;
  

• 				if (diff == 0)
  

• 					return;
  

• 			}
  

• 		}
  

• 	}
  

• }
  

• }

+}

+static void rcar_lvds_pll_pre_start(struct rcar_lvds *lvds,

• 		    struct rcar_du_crtc *rcrtc)
  

+{

• const struct drm_display_mode *mode =

• 		&rcrtc->crtc.state->adjusted_mode;
  

• unsigned int mode_freq = mode->clock * 1000;
• unsigned int ext_clk = 0;
• struct pll_info *lvds_pll[2];
• u32 clksel, cksel;
• int i, ret;
• if (rcrtc->extclock)

• ext_clk = clk_get_rate(rcrtc->extclock);
  

• else

• dev_warn(lvds->dev, "external clock is not set\n");
  

• dev_dbg(lvds->dev, "external clock %d Hz\n", ext_clk);
• if (lvds->enabled)

• return;
  

• for (i = 0; i < 2; i++) {

• lvds_pll[i] = kzalloc(sizeof(*lvds_pll), GFP_KERNEL);
  

• if (!lvds_pll[i])
  

• 	return;
  

Memory leak if kzalloc() fails in any but the first iteration.

Why do you need dynamic allocation at all ?

• }
• /* software reset release */
• reset_control_deassert(lvds->rst);
• ret = clk_prepare_enable(lvds->clock);
• if (ret < 0)

• goto end;
  

• for (i = 0; i < 2; i++) {

• bool edivider;
  

• if (i == 0)
  

• 	edivider = true;
  

• else
  

• 	edivider = false;
  

• rcar_lvds_pll_calc(rcrtc, lvds_pll[i], ext_clk,
  

• 			 mode_freq, edivider);
  

• }
• dev_dbg(lvds->dev, "mode_frequency %d Hz\n", mode_freq);
• if (lvds_pll[1]->diff >= lvds_pll[0]->diff) {

• /* use E-edivider */
  

• i = 0;
  

• clksel = LVDPLLCR_OUTCLKSEL_AFTER |
  

• 	 LVDPLLCR_STP_CLKOUTE1_EN;
  

• } else {

• /* do not use E-divider */
  

• i = 1;
  

• clksel = LVDPLLCR_OUTCLKSEL_BEFORE |
  

• 	 LVDPLLCR_STP_CLKOUTE1_DIS;
  

• }
• dev_dbg(lvds->dev,

• "E-divider %s\n", (i == 0 ? "is used" : "is not used"));
  

• dev_dbg(lvds->dev,

• "pllclk:%u, n:%u, m:%u, e:%u, diff:%u, div:%u\n",
  

•  lvds_pll[i]->pllclk, lvds_pll[i]->clk_n, lvds_pll[i]->clk_m,
  

•  lvds_pll[i]->clk_e, lvds_pll[i]->diff, lvds_pll[i]->div);
  

• if (rcrtc->extal_use)

• cksel = LVDPLLCR_CKSEL_EXTAL;
  

• else

• cksel = LVDPLLCR_CKSEL_DU_DOTCLKIN(rcrtc->index);
  

• rcar_lvds_write(lvds, LVDPLLCR, LVDPLLCR_PLLON |

• 	LVDPLLCR_OCKSEL_7 | clksel | LVDPLLCR_CLKOUT_ENABLE |
  

• 	cksel | (lvds_pll[i]->clk_e << 10) |
  

• 	(lvds_pll[i]->clk_n << 3) | lvds_pll[i]->clk_m);
  

• if (lvds_pll[i]->div > 0)

• rcar_lvds_write(lvds, LVDDIV, LVDDIV_DIVSEL |
  

• 		LVDDIV_DIVRESET | lvds_pll[i]->div);
  

• else

• rcar_lvds_write(lvds, LVDDIV, 0);
  

• dev_dbg(lvds->dev, "LVDPLLCR: 0x%x\n",

• ioread32(lvds->mmio + LVDPLLCR));
  

• dev_dbg(lvds->dev, "LVDDIV: 0x%x\n",

• ioread32(lvds->mmio + LVDDIV));
  

+end:

• for (i = 0; i < 2; i++)

• kfree(lvds_pll[i]);
  

+}

+static void rcar_lvds_pll_start(struct rcar_lvds *lvds,

• 	       struct rcar_du_crtc *rcrtc)
  

+{

• u32 lvdhcr, lvdcr0;
• rcar_lvds_write(lvds, LVDCTRCR, LVDCTRCR_CTR3SEL_ZERO |

• 	LVDCTRCR_CTR2SEL_DISP | LVDCTRCR_CTR1SEL_VSYNC |
  

• 	LVDCTRCR_CTR0SEL_HSYNC);
  

• lvdhcr = LVDCHCR_CHSEL_CH(0, 0) | LVDCHCR_CHSEL_CH(1, 1) |

•  LVDCHCR_CHSEL_CH(2, 2) | LVDCHCR_CHSEL_CH(3, 3);
  

• rcar_lvds_write(lvds, LVDCHCR, lvdhcr);
• rcar_lvds_write(lvds, LVDSTRIPE, 0);
• /* Turn all the channels on. */
• rcar_lvds_write(lvds, LVDCR1,

• 	LVDCR1_CHSTBY(3) | LVDCR1_CHSTBY(2) |
  

• 	LVDCR1_CHSTBY(1) | LVDCR1_CHSTBY(0) |
  

• 	LVDCR1_CLKSTBY);
  

• /*

• * Turn the PLL on, set it to LVDS normal mode, wait for the startup
  

• * delay and turn the output on.
  

• */
  

• lvdcr0 = (lvds->mode << LVDCR0_LVMD_SHIFT) | LVDCR0_PWD;
• rcar_lvds_write(lvds, LVDCR0, lvdcr0);
• lvdcr0 |= LVDCR0_LVEN;
• rcar_lvds_write(lvds, LVDCR0, lvdcr0);
• lvdcr0 |= LVDCR0_LVRES;
• rcar_lvds_write(lvds, LVDCR0, lvdcr0);
• lvds->enabled = true;

+}

static void rcar_lvds_enable(struct drm_bridge *bridge) { struct rcar_lvds *lvds = bridge_to_rcar_lvds(bridge); @@ -164,6 +367,7 @@ static void rcar_lvds_enable(struct drm_bridge *bridge) * do we get a state pointer? */ struct drm_crtc *crtc = lvds->bridge.encoder->crtc;

• struct rcar_du_device *rcdu = to_rcar_crtc(crtc)->group->dev; u32 lvdpllcr; u32 lvdhcr; u32 lvdcr0;

@@ -171,6 +375,12 @@ static void rcar_lvds_enable(struct drm_bridge *bridge)

WARN_ON(lvds->enabled);

• if (rcar_du_has(rcdu, RCAR_DU_FEATURE_LVDS_PLL)) {

This should be turned into an LVDS encoder feature bit.

• rcar_lvds_pll_pre_start(lvds, to_rcar_crtc(crtc));
  

• rcar_lvds_pll_start(lvds, to_rcar_crtc(crtc));
  

• return;
  

• }
• ret = clk_prepare_enable(lvds->clock); if (ret < 0) return;

@@ -264,6 +474,7 @@ static void rcar_lvds_disable(struct drm_bridge *bridge)

rcar_lvds_write(lvds, LVDCR0, 0); rcar_lvds_write(lvds, LVDCR1, 0);

• rcar_lvds_write(lvds, LVDPLLCR, 0);

  clk_disable_unprepare(lvds->clock);

@@ -522,6 +733,7 @@ static const struct of_device_id rcar_lvds_of_table[] = { { .compatible = "renesas,r8a7795-lvds", .data = &rcar_lvds_gen3_info }, { .compatible = "renesas,r8a7796-lvds", .data = &rcar_lvds_gen3_info }, { .compatible = "renesas,r8a77970-lvds", .data = &rcar_lvds_r8a77970_info },

• { .compatible = "renesas,r8a77995-lvds", .data = &rcar_lvds_gen3_info },

{ } };

diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds_regs.h b/drivers/gpu/drm/rcar-du/rcar_lvds_regs.h index 2896835..e37db95 100644 --- a/drivers/gpu/drm/rcar-du/rcar_lvds_regs.h +++ b/drivers/gpu/drm/rcar-du/rcar_lvds_regs.h @@ -21,7 +21,7 @@ #define LVDCR0_PLLON (1 << 4) #define LVDCR0_PWD (1 << 2) /* Gen3 only */ #define LVDCR0_BEN (1 << 2) /* Gen2 only */ -#define LVDCR0_LVEN (1 << 1) /* Gen2 only */ +#define LVDCR0_LVEN (1 << 1) #define LVDCR0_LVRES (1 << 0)

#define LVDCR1 0x0004 @@ -46,6 +46,24 @@ #define LVDPLLCR_PLLDIVCNT_148M (0x046c1 << 0) #define LVDPLLCR_PLLDIVCNT_MASK (0x7ffff << 0)

+/* R-Car D3 */ +#define LVDPLLCR_PLLON (1 << 22) +#define LVDPLLCR_PLLSEL_PLL0 (0 << 20) +#define LVDPLLCR_PLLSEL_LVX (1 << 20) +#define LVDPLLCR_PLLSEL_PLL1 (2 << 20) +#define LVDPLLCR_CKSEL_LVX (1 << 17) +#define LVDPLLCR_CKSEL_EXTAL (3 << 17) +#define LVDPLLCR_CKSEL_DU_DOTCLKIN0 (5 << 17) +#define LVDPLLCR_CKSEL_DU_DOTCLKIN1 (7 << 17) +#define LVDPLLCR_OCKSEL_7 (0 << 16) +#define LVDPLLCR_OCKSEL_NOT_DIVIDED (1 << 16) +#define LVDPLLCR_STP_CLKOUTE1_DIS (0 << 14) +#define LVDPLLCR_STP_CLKOUTE1_EN (1 << 14) +#define LVDPLLCR_OUTCLKSEL_BEFORE (0 << 12) +#define LVDPLLCR_OUTCLKSEL_AFTER (1 << 12) +#define LVDPLLCR_CLKOUT_DISABLE (0 << 11) +#define LVDPLLCR_CLKOUT_ENABLE (1 << 11)

#define LVDCTRCR 0x000c #define LVDCTRCR_CTR3SEL_ZERO (0 << 12) #define LVDCTRCR_CTR3SEL_ODD (1 << 12) @@ -74,4 +92,30 @@ #define LVDCHCR_CHSEL_CH(n, c) ((((c) - (n)) & 3) << ((n) * 4)) #define LVDCHCR_CHSEL_MASK(n) (3 << ((n) * 4))

+#define LVDSTRIPE 0x0014 +#define LVDSTRIPE_ST_TRGSEL_DISP (0 << 2) +#define LVDSTRIPE_ST_TRGSEL_HSYNC_R (1 << 2) +#define LVDSTRIPE_ST_TRGSEL_HSYNC_F (2 << 2)

+#define LVDSTRIPE_ST_SWAP_NORMAL (0 << 1) +#define LVDSTRIPE_ST_SWAP_SWAP (1 << 1) +#define LVDSTRIPE_ST_ON (1 << 0)

+#define LVDSCR 0x0018 +#define LVDSCR_DEPTH_DP1 (0 << 29) +#define LVDSCR_DEPTH_DP2 (1 << 29) +#define LVDSCR_DEPTH_DP3 (2 << 29) +#define LVDSCR_BANDSET_10KHZ_LESS_THAN (1 << 28) +#define LVDSCR_SDIV_SR1 (0 << 22) +#define LVDSCR_SDIV_SR2 (1 << 22) +#define LVDSCR_SDIV_SR4 (2 << 22) +#define LVDSCR_SDIV_SR8 (3 << 22) +#define LVDSCR_MODE_DOWN (1 << 21) +#define LVDSCR_RSTN_ENABLE (1 << 20)

+#define LVDDIV 0x001c +#define LVDDIV_DIVSEL (1 << 8) +#define LVDDIV_DIVRESET (1 << 7) +#define LVDDIV_DIVSTP (1 << 6)

#endif /* __RCAR_LVDS_REGS_H__ */