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...@renesas.com>.
>
> Signed-off-by: Ulrich Hecht <uli+rene...@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__ */
--
Regards,
Laurent Pinchart
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
