Re: [PATCH 05/16] drm: rcar-du: lvds: D3/E3 support

[jacopo mondi <jacopo@jmondi.org>]

[-- Attachment #1: Type: text/plain, Size: 21344 bytes --]

Hi Laurent,

On Tue, Sep 04, 2018 at 03:10:16PM +0300, Laurent Pinchart wrote:
> The LVDS encoders in the D3 and E3 SoCs differ significantly from those
> in the other R-Car Gen3 family members:
>
> - The LVDS PLL architecture is more complex and requires computing PLL
>   parameters manually.
> - The PLL uses external clocks as inputs, which need to be retrieved
>   from DT.
> - In addition to the different PLL setup, the startup sequence has
>   changed *again* (seems someone had trouble making his/her mind).
>
> Supporting all this requires DT bindings extensions for external clocks,
> brand new PLL setup code, and a few quirks to handle the differences in
> the startup sequence.
>
> The implementation doesn't support all hardware features yet, namely
>
> - Using the LV[01] clocks generated by the CPG as PLL input.
> - Providing the LVDS PLL clock to the DU for use with the RGB output.
>
> Those features can be added later when the need will arise.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> ---
>  drivers/gpu/drm/rcar-du/rcar_lvds.c      | 365 +++++++++++++++++++++++++++----
>  drivers/gpu/drm/rcar-du/rcar_lvds_regs.h |  43 +++-
>  2 files changed, 361 insertions(+), 47 deletions(-)
>
> diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c b/drivers/gpu/drm/rcar-du/rcar_lvds.c
> index ce0eb68c3416..aac4acbcfbfc 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_lvds.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c
> @@ -24,6 +24,8 @@
>
>  #include "rcar_lvds_regs.h"
>
> +struct rcar_lvds;
> +
>  /* Keep in sync with the LVDCR0.LVMD hardware register values. */
>  enum rcar_lvds_mode {
>  	RCAR_LVDS_MODE_JEIDA = 0,
> @@ -31,14 +33,16 @@ enum rcar_lvds_mode {
>  	RCAR_LVDS_MODE_VESA = 4,
>  };
>
> -#define RCAR_LVDS_QUIRK_LANES	(1 << 0)	/* LVDS lanes 1 and 3 inverted */
> -#define RCAR_LVDS_QUIRK_GEN2_PLLCR (1 << 1)	/* LVDPLLCR has gen2 layout */
> -#define RCAR_LVDS_QUIRK_GEN3_LVEN (1 << 2)	/* LVEN bit needs to be set */
> -						/* on R8A77970/R8A7799x */
> +#define RCAR_LVDS_QUIRK_LANES		BIT(0)	/* LVDS lanes 1 and 3 inverted */
> +#define RCAR_LVDS_QUIRK_GEN3_LVEN	BIT(1)	/* LVEN bit needs to be set on R8A77970/R8A7799x */
> +#define RCAR_LVDS_QUIRK_PWD		BIT(2)	/* PWD bit available (all of Gen3 but E3) */
> +#define RCAR_LVDS_QUIRK_EXT_PLL		BIT(3)	/* Has extended PLL */
> +#define RCAR_LVDS_QUIRK_DUAL_LINK	BIT(4)	/* Supports dual-link operation */
>
>  struct rcar_lvds_device_info {
>  	unsigned int gen;
>  	unsigned int quirks;
> +	void (*pll_setup)(struct rcar_lvds *lvds, unsigned int freq);
>  };
>
>  struct rcar_lvds {
> @@ -52,7 +56,11 @@ struct rcar_lvds {
>  	struct drm_panel *panel;
>
>  	void __iomem *mmio;
> -	struct clk *clock;
> +	struct {
> +		struct clk *mod;		/* CPG module clock */
> +		struct clk *extal;		/* External clock */
> +		struct clk *dotclkin[2];	/* External DU clocks */
> +	} clocks;
>  	bool enabled;
>
>  	struct drm_display_mode display_mode;
> @@ -128,33 +136,222 @@ static const struct drm_connector_funcs rcar_lvds_conn_funcs = {
>  };
>
>  /* -----------------------------------------------------------------------------
> - * Bridge
> + * PLL Setup
>   */
>
> -static u32 rcar_lvds_lvdpllcr_gen2(unsigned int freq)
> +static void rcar_lvds_pll_setup_gen2(struct rcar_lvds *lvds, unsigned int freq)
> +{
> +	u32 val;
> +
> +	if (freq < 39000000)
> +		val = LVDPLLCR_CEEN | LVDPLLCR_COSEL | LVDPLLCR_PLLDLYCNT_38M;
> +	else if (freq < 61000000)
> +		val = LVDPLLCR_CEEN | LVDPLLCR_COSEL | LVDPLLCR_PLLDLYCNT_60M;
> +	else if (freq < 121000000)
> +		val = LVDPLLCR_CEEN | LVDPLLCR_COSEL | LVDPLLCR_PLLDLYCNT_121M;
> +	else
> +		val = LVDPLLCR_PLLDLYCNT_150M;
> +
> +	rcar_lvds_write(lvds, LVDPLLCR, val);
> +}
> +
> +static void rcar_lvds_pll_setup_gen3(struct rcar_lvds *lvds, unsigned int freq)
>  {
> -	if (freq < 39000)
> -		return LVDPLLCR_CEEN | LVDPLLCR_COSEL | LVDPLLCR_PLLDLYCNT_38M;
> -	else if (freq < 61000)
> -		return LVDPLLCR_CEEN | LVDPLLCR_COSEL | LVDPLLCR_PLLDLYCNT_60M;
> -	else if (freq < 121000)
> -		return LVDPLLCR_CEEN | LVDPLLCR_COSEL | LVDPLLCR_PLLDLYCNT_121M;
> +	u32 val;
> +
> +	if (freq < 42000000)
> +		val = LVDPLLCR_PLLDIVCNT_42M;
> +	else if (freq < 85000000)
> +		val = LVDPLLCR_PLLDIVCNT_85M;
> +	else if (freq < 128000000)
> +		val = LVDPLLCR_PLLDIVCNT_128M;
>  	else
> -		return LVDPLLCR_PLLDLYCNT_150M;
> +		val = LVDPLLCR_PLLDIVCNT_148M;
> +
> +	rcar_lvds_write(lvds, LVDPLLCR, val);
> +}
> +
> +struct pll_info {
> +	struct clk *clk;
> +	unsigned long diff;
> +	unsigned int pll_m;
> +	unsigned int pll_n;
> +	unsigned int pll_e;
> +	unsigned int div;
> +};
> +
> +static void rcar_lvds_d3_e3_pll_calc(struct rcar_lvds *lvds, struct clk *clk,
> +				     unsigned long target, struct pll_info *pll)

Do you think it is worth mentioning d3_e3 in the function name? I know
it's not a big deal, but in future generation this PLL circuit may be
re-used.

> +{
> +	unsigned long fin;
> +	unsigned int m_min;
> +	unsigned int m_max;
> +	unsigned int m;
> +
> +	if (!clk)
> +		return;
> +
> +	/*
> +	 * The LVDS PLL is made of a pre-divider and a multiplier (strangerly
> +	 * enough called M and N respectively), followed by a post-divider E.
> +	 *
> +	 *         ,-----.         ,-----.     ,-----.         ,-----.
> +	 * Fin --> | 1/M | -Fpdf-> | PFD | --> | VCO | -Fvco-> | 1/E | --> Fout
> +	 *         `-----'     ,-> |     |     `-----'   |     `-----'
> +	 *                     |   `-----'               |
> +	 *                     |         ,-----.         |
> +	 *                     `-------- | 1/N | <-------'
> +	 *                               `-----'
> +	 *
> +	 * The clock output by the PLL is then further divided by a programmable
> +	 * divider DIV to achieve the desired target frequency. Finally, an
> +	 * optional fixed /7 divider is used to convert the bit clock to a pixel
> +	 * clock (as LVDS transmits 7 bits per lane per clock sample).
> +	 *
> +	 *          ,-------.     ,-----.     |\
> +	 * Fout --> | 1/DIV | --> | 1/7 | --> | |
> +	 *          `-------'  |  `-----'     | | --> dot clock
> +	 *                     `------------> | |
> +	 *                                    |/
> +	 *
> +	 * The /7 divider is optional when the LVDS PLL is used to generate a
> +	 * dot clock for the DU RGB output, without using the LVDS encoder. We
> +	 * don't support this configuration yet.
> +	 *
> +	 * The PLL allowed input frequency range is 12 MHz to 192 MHz.
> +	 */
> +
> +	fin = clk_get_rate(clk);
> +	if (fin < 12000000 || fin > 192000000)
> +		return;
> +
> +	/*
> +	 * The comparison frequency range is 12 MHz to 24 MHz, which limits the
> +	 * allowed values for the pre-divider M (normal range 1-8).
> +	 *
> +	 * Fpfd = Fin / M
> +	 */
> +	m_min = max_t(unsigned int, 1, DIV_ROUND_UP(fin, 24000000));
> +	m_max = min_t(unsigned int, 8, fin / 12000000);
> +
> +	for (m = m_min; m <= m_max; ++m) {
> +		unsigned long fpfd;
> +		unsigned int n_min;
> +		unsigned int n_max;
> +		unsigned int n;
> +
> +		/*
> +		 * The VCO operating range is 900 Mhz to 1800 MHz, which limits
> +		 * the allowed values for the multiplier N (normal range
> +		 * 60-120).
> +		 *
> +		 * Fvco = Fin * N / M
> +		 */
> +		fpfd = fin / m;
> +		n_min = max_t(unsigned int, 60, DIV_ROUND_UP(900000000, fpfd));
> +		n_max = min_t(unsigned int, 120, 1800000000 / fpfd);
> +
> +		for (n = n_min; n < n_max; ++n) {
> +			unsigned long fvco;
> +			unsigned int e_min;
> +			unsigned int e;
> +
> +			/*
> +			 * The output frequency is limited to 1039.5 MHz,
> +			 * limiting again the allowed values for the
> +			 * post-divider E (normal value 1, 2 or 4).
> +			 *
> +			 * Fout = Fvco / E
> +			 */
> +			fvco = fpfd * n;
> +			e_min = fvco > 1039500000 ? 1 : 0;
> +
> +			for (e = e_min; e < 3; ++e) {
> +				unsigned long fout;
> +				unsigned long diff;
> +				unsigned int div;
> +
> +				/*
> +				 * Finally we have a programable divider after
> +				 * the PLL, followed by a an optional fixed /7
> +				 * divider.
> +				 */
> +				fout = fvco / (1 << e) / 7;
> +				div = DIV_ROUND_CLOSEST(fout, target);
> +				diff = abs(fout / div - target);
> +
> +				if (diff < pll->diff) {
> +					pll->clk = clk;
> +					pll->diff = diff;
> +					pll->pll_m = m;
> +					pll->pll_n = n;
> +					pll->pll_e = e;
> +					pll->div = div;
> +
> +					if (diff == 0)
> +						goto done;
> +				}
> +			}
> +		}
> +	}

Very nice :)

> +
> +done:
> +#if defined(CONFIG_DEBUG) || defined(CONFIG_DYNAMIC_DEBUG)
> +	{
> +		unsigned long output = fin * pll->pll_n / pll->pll_m
> +				     / (1 << pll->pll_e) / 7 / pll->div;
> +		int error = (long)(output - target) * 10000 / (long)target;
> +
> +		dev_dbg(lvds->dev,
> +			"%pC %lu Hz -> Fout %lu Hz (target %lu Hz, error %d.%02u%%), PLL M/N/E/DIV %u/%u/%u/%u\n",
> +			clk, fin, output, target, error / 100,
> +			error < 0 ? -error % 100 : error % 100,
> +			pll->pll_m, pll->pll_n, pll->pll_e, pll->div);
> +	}
> +#endif

I know you know about this already, but

../drivers/gpu/drm/rcar-du/rcar_lvds.c:298:1: error: label at end of
compound statement

I'm still not sure what actually disturbs gcc here

>  }
>
> -static u32 rcar_lvds_lvdpllcr_gen3(unsigned int freq)
> +static void rcar_lvds_pll_setup_d3_e3(struct rcar_lvds *lvds, unsigned int freq)
>  {
> -	if (freq < 42000)
> -		return LVDPLLCR_PLLDIVCNT_42M;
> -	else if (freq < 85000)
> -		return LVDPLLCR_PLLDIVCNT_85M;
> -	else if (freq < 128000)
> -		return LVDPLLCR_PLLDIVCNT_128M;
> +	struct drm_crtc *crtc = lvds->bridge.encoder->crtc;
> +	struct pll_info pll = { .diff = (unsigned long)-1 };
> +	u32 lvdpllcr;
> +
> +	if (lvds->clocks.dotclkin[0] || lvds->clocks.dotclkin[1]) {
> +		rcar_lvds_d3_e3_pll_calc(lvds, lvds->clocks.dotclkin[0],
> +					 freq, &pll);
> +		rcar_lvds_d3_e3_pll_calc(lvds, lvds->clocks.dotclkin[1],
> +					 freq, &pll);
> +	} else if (lvds->clocks.extal) {
> +		rcar_lvds_d3_e3_pll_calc(lvds, lvds->clocks.extal,
> +					 freq, &pll);
> +	}

here it's either ((dotclkin[0] or dotclock[1]) or extal). Are they
mutually exclusive? Can't we try all of them? The probe routine
guarantees we have at least of of them...

> +
> +	lvdpllcr = LVDPLLCR_PLLON | LVDPLLCR_CLKOUT
> +		 | LVDPLLCR_PLLN(pll.pll_n - 1) | LVDPLLCR_PLLM(pll.pll_m - 1);
> +
> +	if (pll.clk == lvds->clocks.extal)
> +		lvdpllcr |= LVDPLLCR_CKSEL_EXTAL;
> +	else
> +		lvdpllcr |= LVDPLLCR_CKSEL_DU_DOTCLKIN(drm_crtc_index(crtc));

Here you select du_clkin[0] or du_clkin[1] based on the DU index (btw,
the drm_crtc_index() function is funny, it simply "crtc->index" no
checks, no validation, what's the benefit of using it?).

Looking at the LVDS PLL block diagram for D3/E3 (Figure 37.3) I see
that both clkin[0] and clkin[1] could be used independently from the du
index. Shouldn't we use the one performing better? (now how to make
sure it gets not selected twice in case of both DU0 and DU1 using the
LVDS PLL it's another problem)

> +
> +	if (pll.pll_e > 0)
> +		lvdpllcr |= LVDPLLCR_STP_CLKOUTE | LVDPLLCR_OUTCLKSEL
> +			 |  LVDPLLCR_PLLE(pll.pll_e - 1);
> +
> +	rcar_lvds_write(lvds, LVDPLLCR, lvdpllcr);
> +
> +	if (pll.div > 1)
> +		rcar_lvds_write(lvds, LVDDIV, LVDDIV_DIVSEL |
> +				LVDDIV_DIVRESET | LVDDIV_DIV(pll.div - 1));
>  	else
> -		return LVDPLLCR_PLLDIVCNT_148M;
> +		rcar_lvds_write(lvds, LVDDIV, 0);
>  }
>
> +/* -----------------------------------------------------------------------------
> + * Bridge
> + */
> +
>  static void rcar_lvds_enable(struct drm_bridge *bridge)
>  {
>  	struct rcar_lvds *lvds = bridge_to_rcar_lvds(bridge);
> @@ -164,14 +361,13 @@ static void rcar_lvds_enable(struct drm_bridge *bridge)
>  	 * do we get a state pointer?
>  	 */
>  	struct drm_crtc *crtc = lvds->bridge.encoder->crtc;
> -	u32 lvdpllcr;
>  	u32 lvdhcr;
>  	u32 lvdcr0;
>  	int ret;
>
>  	WARN_ON(lvds->enabled);
>
> -	ret = clk_prepare_enable(lvds->clock);
> +	ret = clk_prepare_enable(lvds->clocks.mod);
>  	if (ret < 0)
>  		return;
>
> @@ -196,12 +392,13 @@ static void rcar_lvds_enable(struct drm_bridge *bridge)
>
>  	rcar_lvds_write(lvds, LVDCHCR, lvdhcr);
>
> +	if (lvds->info->quirks & RCAR_LVDS_QUIRK_DUAL_LINK) {
> +		/* Disable dual-link mode. */
> +		rcar_lvds_write(lvds, LVDSTRIPE, 0);
> +	}
> +
>  	/* PLL clock configuration. */
> -	if (lvds->info->quirks & RCAR_LVDS_QUIRK_GEN2_PLLCR)
> -		lvdpllcr = rcar_lvds_lvdpllcr_gen2(mode->clock);
> -	else
> -		lvdpllcr = rcar_lvds_lvdpllcr_gen3(mode->clock);
> -	rcar_lvds_write(lvds, LVDPLLCR, lvdpllcr);
> +	lvds->info->pll_setup(lvds, mode->clock * 1000);
>
>  	/* Set the LVDS mode and select the input. */
>  	lvdcr0 = lvds->mode << LVDCR0_LVMD_SHIFT;
> @@ -220,11 +417,16 @@ static void rcar_lvds_enable(struct drm_bridge *bridge)
>  		rcar_lvds_write(lvds, LVDCR0, lvdcr0);
>  	}
>
> -	/* Turn the PLL on. */
> -	lvdcr0 |= LVDCR0_PLLON;
> -	rcar_lvds_write(lvds, LVDCR0, lvdcr0);
> +	if (!(lvds->info->quirks & RCAR_LVDS_QUIRK_EXT_PLL)) {
> +		/*
> +		 * Turn the PLL on (simple PLL only, extended PLL is fully
> +		 * controlled through LVDPLLCR).
> +		 */
> +		lvdcr0 |= LVDCR0_PLLON;
> +		rcar_lvds_write(lvds, LVDCR0, lvdcr0);
> +	}
>
> -	if (lvds->info->gen > 2) {
> +	if (lvds->info->quirks & RCAR_LVDS_QUIRK_PWD) {
>  		/* Set LVDS normal mode. */
>  		lvdcr0 |= LVDCR0_PWD;
>  		rcar_lvds_write(lvds, LVDCR0, lvdcr0);
> @@ -236,8 +438,10 @@ static void rcar_lvds_enable(struct drm_bridge *bridge)
>  		rcar_lvds_write(lvds, LVDCR0, lvdcr0);
>  	}
>
> -	/* Wait for the startup delay. */
> -	usleep_range(100, 150);
> +	if (!(lvds->info->quirks & RCAR_LVDS_QUIRK_EXT_PLL)) {
> +		/* Wait for the PLL startup delay (simple PLL only). */
> +		usleep_range(100, 150);
> +	}
>
>  	/* Turn the output on. */
>  	lvdcr0 |= LVDCR0_LVRES;
> @@ -264,8 +468,9 @@ 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);
> +	clk_disable_unprepare(lvds->clocks.mod);
>
>  	lvds->enabled = false;
>  }
> @@ -446,6 +651,60 @@ static int rcar_lvds_parse_dt(struct rcar_lvds *lvds)
>  	return ret;
>  }
>
> +static struct clk *rcar_lvds_get_clock(struct rcar_lvds *lvds, const char *name,
> +				       bool optional)
> +{
> +	struct clk *clk;
> +
> +	clk = devm_clk_get(lvds->dev, name);

I wish we had clk_get_optional() as we have gpiod_get_optional().
There are probably good reasons if it's not there though...

> +	if (!IS_ERR(clk))
> +		return clk;
> +
> +	if (PTR_ERR(clk) == -ENOENT && optional)
> +		return NULL;
> +
> +	if (PTR_ERR(clk) != -EPROBE_DEFER)
> +		dev_err(lvds->dev, "failed to get %s clock\n",
> +			name ? name : "module");
> +
> +	return clk;
> +}
> +
> +static int rcar_lvds_get_clocks(struct rcar_lvds *lvds)
> +{
> +	lvds->clocks.mod = rcar_lvds_get_clock(lvds, NULL, false);
> +	if (IS_ERR(lvds->clocks.mod))
> +		return PTR_ERR(lvds->clocks.mod);
> +
> +	/*
> +	 * LVDS encoders without an extended PLL have no external clock inputs.
> +	 */
> +	if (!(lvds->info->quirks & RCAR_LVDS_QUIRK_EXT_PLL))
> +		return 0;
> +
> +	lvds->clocks.extal = rcar_lvds_get_clock(lvds, "extal", true);
> +	if (IS_ERR(lvds->clocks.extal))
> +		return PTR_ERR(lvds->clocks.extal);
> +
> +	lvds->clocks.dotclkin[0] = rcar_lvds_get_clock(lvds, "dclkin.0", true);
> +	if (IS_ERR(lvds->clocks.dotclkin[0]))
> +		return PTR_ERR(lvds->clocks.dotclkin[0]);
> +
> +	lvds->clocks.dotclkin[1] = rcar_lvds_get_clock(lvds, "dclkin.1", true);
> +	if (IS_ERR(lvds->clocks.dotclkin[1]))
> +		return PTR_ERR(lvds->clocks.dotclkin[1]);
> +
> +	/* At least one input to the PLL must be available. */
> +	if (!lvds->clocks.extal && !lvds->clocks.dotclkin[0] &&
> +	    !lvds->clocks.dotclkin[1]) {
> +		dev_err(lvds->dev,
> +			"no input clock (extal, dclkin.0 or dclkin.1)\n");
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
>  static int rcar_lvds_probe(struct platform_device *pdev)
>  {
>  	struct rcar_lvds *lvds;
> @@ -475,11 +734,9 @@ static int rcar_lvds_probe(struct platform_device *pdev)
>  	if (IS_ERR(lvds->mmio))
>  		return PTR_ERR(lvds->mmio);
>
> -	lvds->clock = devm_clk_get(&pdev->dev, NULL);
> -	if (IS_ERR(lvds->clock)) {
> -		dev_err(&pdev->dev, "failed to get clock\n");
> -		return PTR_ERR(lvds->clock);
> -	}
> +	ret = rcar_lvds_get_clocks(lvds);
> +	if (ret < 0)
> +		return ret;
>
>  	drm_bridge_add(&lvds->bridge);
>
> @@ -497,21 +754,39 @@ static int rcar_lvds_remove(struct platform_device *pdev)
>
>  static const struct rcar_lvds_device_info rcar_lvds_gen2_info = {
>  	.gen = 2,
> -	.quirks = RCAR_LVDS_QUIRK_GEN2_PLLCR,
> +	.pll_setup = rcar_lvds_pll_setup_gen2,
>  };
>
>  static const struct rcar_lvds_device_info rcar_lvds_r8a7790_info = {
>  	.gen = 2,
> -	.quirks = RCAR_LVDS_QUIRK_GEN2_PLLCR | RCAR_LVDS_QUIRK_LANES,
> +	.quirks = RCAR_LVDS_QUIRK_LANES,
> +	.pll_setup = rcar_lvds_pll_setup_gen2,
>  };
>
>  static const struct rcar_lvds_device_info rcar_lvds_gen3_info = {
>  	.gen = 3,
> +	.quirks = RCAR_LVDS_QUIRK_PWD,
> +	.pll_setup = rcar_lvds_pll_setup_gen3,
>  };
>
>  static const struct rcar_lvds_device_info rcar_lvds_r8a77970_info = {
>  	.gen = 3,
> -	.quirks = RCAR_LVDS_QUIRK_GEN2_PLLCR | RCAR_LVDS_QUIRK_GEN3_LVEN,
> +	.quirks = RCAR_LVDS_QUIRK_PWD | RCAR_LVDS_QUIRK_GEN3_LVEN,
> +	.pll_setup = rcar_lvds_pll_setup_gen2,
> +};
> +
> +static const struct rcar_lvds_device_info rcar_lvds_r8a77990_info = {
> +	.gen = 3,
> +	.quirks = RCAR_LVDS_QUIRK_GEN3_LVEN | RCAR_LVDS_QUIRK_EXT_PLL
> +		| RCAR_LVDS_QUIRK_DUAL_LINK,
> +	.pll_setup = rcar_lvds_pll_setup_d3_e3,
> +};
> +
> +static const struct rcar_lvds_device_info rcar_lvds_r8a77995_info = {
> +	.gen = 3,
> +	.quirks = RCAR_LVDS_QUIRK_GEN3_LVEN | RCAR_LVDS_QUIRK_PWD
> +		| RCAR_LVDS_QUIRK_EXT_PLL | RCAR_LVDS_QUIRK_DUAL_LINK,
> +	.pll_setup = rcar_lvds_pll_setup_d3_e3,
>  };
>
>  static const struct of_device_id rcar_lvds_of_table[] = {
> @@ -523,6 +798,8 @@ static const struct of_device_id rcar_lvds_of_table[] = {
>  	{ .compatible = "renesas,r8a7796-lvds", .data = &rcar_lvds_gen3_info },
>  	{ .compatible = "renesas,r8a77970-lvds", .data = &rcar_lvds_r8a77970_info },
>  	{ .compatible = "renesas,r8a77980-lvds", .data = &rcar_lvds_gen3_info },
> +	{ .compatible = "renesas,r8a77990-lvds", .data = &rcar_lvds_r8a77990_info },
> +	{ .compatible = "renesas,r8a77995-lvds", .data = &rcar_lvds_r8a77995_info },
>  	{ }
>  };
>
> diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds_regs.h b/drivers/gpu/drm/rcar-du/rcar_lvds_regs.h
> index 4870f50d9bec..87149f2f8056 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_lvds_regs.h
> +++ b/drivers/gpu/drm/rcar-du/rcar_lvds_regs.h
> @@ -18,7 +18,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
> @@ -27,21 +27,36 @@
>  #define LVDCR1_CLKSTBY			(3 << 0)
>
>  #define LVDPLLCR			0x0008
> +/* Gen2 & V3M */
>  #define LVDPLLCR_CEEN			(1 << 14)
>  #define LVDPLLCR_FBEN			(1 << 13)
>  #define LVDPLLCR_COSEL			(1 << 12)
> -/* Gen2 */
>  #define LVDPLLCR_PLLDLYCNT_150M		(0x1bf << 0)
>  #define LVDPLLCR_PLLDLYCNT_121M		(0x22c << 0)
>  #define LVDPLLCR_PLLDLYCNT_60M		(0x77b << 0)
>  #define LVDPLLCR_PLLDLYCNT_38M		(0x69a << 0)
>  #define LVDPLLCR_PLLDLYCNT_MASK		(0x7ff << 0)
> -/* Gen3 */
> +/* Gen3 but V3M,D3 and E3 */
>  #define LVDPLLCR_PLLDIVCNT_42M		(0x014cb << 0)
>  #define LVDPLLCR_PLLDIVCNT_85M		(0x00a45 << 0)
>  #define LVDPLLCR_PLLDIVCNT_128M		(0x006c3 << 0)
>  #define LVDPLLCR_PLLDIVCNT_148M		(0x046c1 << 0)
>  #define LVDPLLCR_PLLDIVCNT_MASK		(0x7ffff << 0)
> +/* D3 and E3 */
> +#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_DOTCLKIN(n)	((5 + (n) * 2) << 17)
> +#define LVDPLLCR_OCKSEL			(1 << 16)
> +#define LVDPLLCR_STP_CLKOUTE		(1 << 14)
> +#define LVDPLLCR_OUTCLKSEL		(1 << 12)
> +#define LVDPLLCR_CLKOUT			(1 << 11)
> +#define LVDPLLCR_PLLE(n)		((n) << 10)
> +#define LVDPLLCR_PLLN(n)		((n) << 3)
> +#define LVDPLLCR_PLLM(n)		((n) << 0)
>
>  #define LVDCTRCR			0x000c
>  #define LVDCTRCR_CTR3SEL_ZERO		(0 << 12)
> @@ -71,4 +86,26 @@
>  #define LVDCHCR_CHSEL_CH(n, c)		((((c) - (n)) & 3) << ((n) * 4))
>  #define LVDCHCR_CHSEL_MASK(n)		(3 << ((n) * 4))
>
> +/* All registers below are specific to D3 and E3 */
> +#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		(1 << 1)
> +#define LVDSTRIPE_ST_ON			(1 << 0)
> +
> +#define LVDSCR				0x0018
> +#define LVDSCR_DEPTH(n)			(((n) - 1) << 29)
> +#define LVDSCR_BANDSET			(1 << 28)
> +#define LVDSCR_TWGCNT(n)		((((n) - 256) / 16) << 24)
> +#define LVDSCR_SDIV(n)			((n) << 22)
> +#define LVDSCR_MODE			(1 << 21)
> +#define LVDSCR_RSTN			(1 << 20)
> +
> +#define LVDDIV				0x001c
> +#define LVDDIV_DIVSEL			(1 << 8)
> +#define LVDDIV_DIVRESET			(1 << 7)
> +#define LVDDIV_DIVSTP			(1 << 6)
> +#define LVDDIV_DIV(n)			((n) << 0)
> +
>  #endif /* __RCAR_LVDS_REGS_H__ */
> --
> Regards,
>
> Laurent Pinchart
>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]