HI Laurent,
thanks for the patch rework
On Tue, Aug 21, 2018 at 01:12:40AM +0300, Laurent Pinchart wrote:
> 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)
I don't see the rcrtc parameter ever being used in this function.
Do you want to keep it anyhow?
> > +{
> > + 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,
s/obtained,/obtained one,/ ?
Will get back with some testing results on a different VGA monitor...
Thanks
j
> > + * 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
>
>
>
Attachment:
signature.asc
Description: PGP signature
_______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
