Hi Jacopo,
On Tuesday, 21 August 2018 00:49:41 EEST Laurent Pinchart wrote:
> From: Jacopo Mondi <jacopo@xxxxxxxxxx>
>
> DU channels not equipped with a DPLL use an SoC internal (provided by
> the CPG) or external clock source combined with a DU internal divider to
> generate the desired output dot clock frequency.
>
> The current clock selection procedure does not fully exploit the ability
> of external clock sources to generate the exact dot clock frequency by
> themselves, but relies instead on tuning the internal DU clock divider
> only, resulting in a less precise clock generation process.
>
> When possible, and desirable, ask the external clock source for the
> exact output dot clock frequency, and select the clock source that
> produces the frequency closest to the desired output dot clock.
>
> This patch specifically targets platforms (like Salvator-X[S] and ULCBs)
> where the DU's input dotclock.in is generated by the versaclock VC5
> clock source, which is capable of generating the exact rate the DU needs
> as pixel clock output.
>
> This patch fixes higher resolution modes which requires an high pixel
> clock output currently not working on non-HDMI DU channel (such as
> 1920x1080@60Hz on the VGA output).
Just for the record, with this patch the following modes (as printed by
modetest) on the VGA output now produce correct result with my monitor:
1920x1080 60 1920 2008 2052 2200 1080 1084 1089 1125 flags: nhsync, nvsync
1600x1200 60 1600 1664 1856 2160 1200 1201 1204 1250 flags: phsync, pvsync
1360x768 60 1360 1424 1536 1792 768 771 777 795 flags: phsync, pvsync
The second mode used to not display at all, with a message telling that
timings were out of range, and the other two modes used to produce a displayed
image partly shifted or scaled out of the screen boundaries.
The following modes still produce an image partly out of the screen
boundaries.
1600x900 60 1600 1624 1704 1800 900 901 904 1000 flags: phsync, pvsync
1280x960 60 1280 1376 1488 1800 960 961 964 1000 flags: phsync, pvsync
1366x768 60 1366 1436 1579 1792 768 771 774 798 flags: phsync, pvsync
1366x768 60 1366 1380 1436 1500 768 769 772 800 flags: phsync, pvsync
1280x720 60 1280 1390 1430 1650 720 725 730 750 flags: phsync, pvsync
And this one is reported to be out of range.
1920x1200 60 1920 2056 2256 2592 1200 1203 1209 1245 flags: nhsync, pvsync
There is thus still room for improvement (some of the issues are possibly due
to my monitor though), but there's also an improvement, and no noticeable
regression.
> Fixes: 1b30dbde8596 ("drm: rcar-du: Add support for external pixel clock")
> Signed-off-by: Jacopo Mondi <jacopo@xxxxxxxxxx>
> [Factor out code to a helper function]
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@xxxxxxxxxxxxxxxx>
> ---
> drivers/gpu/drm/rcar-du/rcar_du_crtc.c | 86 ++++++++++++++++++++----------
> 1 file changed, 55 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c index f8068170905a..2c9405458bbf
> 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> @@ -165,6 +165,48 @@ static void rcar_du_dpll_divider(struct rcar_du_crtc
> *rcrtc, best_diff);
> }
>
> +struct du_clk_params {
> + struct clk *clk;
> + unsigned long rate;
> + unsigned long diff;
> + u32 escr;
> +};
> +
> +static void rcar_du_escr_divider(struct rcar_du_crtc *rcrtc, struct clk
> *clk, + unsigned long target, u32 escr,
> + struct du_clk_params *params)
> +{
> + unsigned long rate;
> + unsigned long diff;
> + u32 div;
> +
> + /*
> + * If the target rate has already been achieved perfectly we can't do
> + * better.
> + */
> + if (params->diff == 0)
> + return;
> +
> + /*
> + * Compute the input clock rate and internal divisor values to obtain
> + * the clock rate closest to the target frequency.
> + */
> + rate = clk_round_rate(clk, target);
> + div = clamp(DIV_ROUND_CLOSEST(rate, target), 1UL, 64UL) - 1;
> + diff = abs(rate / (div + 1) - target);
> +
> + /*
> + * If the resulting frequency is better than any previously obtained,
> + * store the parameters.
> + */
> + if (diff < params->diff) {
> + params->clk = clk;
> + params->rate = rate;
> + params->diff = diff;
> + params->escr = escr | div;
> + }
> +}
> +
> static const struct soc_device_attribute rcar_du_r8a7795_es1[] = {
> { .soc_id = "r8a7795", .revision = "ES1.*" },
> { /* sentinel */ }
> @@ -225,42 +267,24 @@ static void rcar_du_crtc_set_display_timing(struct
> rcar_du_crtc *rcrtc)
>
> escr = ESCR_DCLKSEL_DCLKIN | div;
> } else {
> - unsigned long clk;
> - u32 div;
> -
> - /*
> - * Compute the clock divisor and select the internal or external
> - * dot clock based on the requested frequency.
> - */
> - clk = clk_get_rate(rcrtc->clock);
> - div = DIV_ROUND_CLOSEST(clk, mode_clock);
> - div = clamp(div, 1U, 64U) - 1;
> -
> - escr = ESCR_DCLKSEL_CLKS | div;
> -
> - if (rcrtc->extclock) {
> - unsigned long extclk;
> - unsigned long extrate;
> - unsigned long rate;
> - u32 extdiv;
> + struct du_clk_params params = { .diff = (unsigned long)-1 };
>
> - extclk = clk_get_rate(rcrtc->extclock);
> - extdiv = DIV_ROUND_CLOSEST(extclk, mode_clock);
> - extdiv = clamp(extdiv, 1U, 64U) - 1;
> + rcar_du_escr_divider(rcrtc, rcrtc->clock, mode_clock,
> + ESCR_DCLKSEL_CLKS, ¶ms);
> + if (rcrtc->extclock)
> + rcar_du_escr_divider(rcrtc, rcrtc->extclock, mode_clock,
> + ESCR_DCLKSEL_DCLKIN, ¶ms);
>
> - extrate = extclk / (extdiv + 1);
> - rate = clk / (div + 1);
> + dev_dbg(rcrtc->group->dev->dev, "mode clock %lu %s rate %lu\n",
> + mode_clock, params.clk == rcrtc->clock ? "cpg" : "ext",
> + params.rate);
>
> - if (abs((long)extrate - (long)mode_clock) <
> - abs((long)rate - (long)mode_clock))
> - 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);
> - }
> + clk_set_rate(params.clk, params.rate);
> + escr = params.escr;
> }
>
> + dev_dbg(rcrtc->group->dev->dev, "%s: ESCR 0x%08x\n", __func__, escr);
> +
> rcar_du_group_write(rcrtc->group, rcrtc->index % 2 ? ESCR2 : ESCR,
> escr);
> rcar_du_group_write(rcrtc->group, rcrtc->index % 2 ? OTAR2 : OTAR, 0);
--
Regards,
Laurent Pinchart
