Hi!
Some background can be found here:
https://lists.freedesktop.org/archives/dri-devel/2018-August/187182.html
The "10 times" discriminator in patch 2/2 can certainly be discussed...
Cheers,
Peter
Changes since v1 https://lkml.org/lkml/2018/8/24/187
- added {} to an if body for symmetry
- reformatted comments a little bit
- spelling/grammar fixes
Peter Rosin (2):
drm/atmel-hlcdc: prefer a higher rate clock as pixel-clock base
drm/atmel-hlcdc: allow selecting a higher pixel-clock than requested
drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c | 30 ++++++++++++++++++++------
1 file changed, 23 insertions(+), 7 deletions(-)
--
2.11.0
But only if the highest pixel-clock frequency lower than requested
is significantly less accurate than the lowest frequency higher than
requested.
I pulled "10 times" as the discriminator out of the hat, and went with
that.
This is useful, if e.g. the target pixel-clock is 65MHz and the sys_clk
is 132MHz. In this case the highest possible pixel-clock lower than the
requested 65MHz is 52.8MHz, which is almost 20% off (and outside the
spec for the panel). The lowest possible pixel-clock higher than 65MHz
is 66MHz, which is a *much* better match, and only 1.5% off.
Signed-off-by: Peter Rosin <[email protected]>
---
drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
index 0d9d1042752a..9e34bce089d0 100644
--- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
+++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
@@ -116,6 +116,18 @@ static void atmel_hlcdc_crtc_mode_set_nofb(struct drm_crtc *c)
div = DIV_ROUND_UP(prate, mode_rate);
if (ATMEL_HLCDC_CLKDIV(div) & ~ATMEL_HLCDC_CLKDIV_MASK)
div = ATMEL_HLCDC_CLKDIV_MASK;
+ } else {
+ int div_low = prate / mode_rate;
+
+ if (div_low >= 2 &&
+ ((prate / div_low - mode_rate) <
+ 10 * (mode_rate - prate / div)))
+ /*
+ * At least 10 times better when using a higher
+ * frequency than requested, instead of a lower.
+ * So, go with that.
+ */
+ div = div_low;
}
cfg |= ATMEL_HLCDC_CLKDIV(div);
--
2.11.0
If the divider used to get the pixel-clock is small, the granularity
of the frequencies possible for the pixel-clock is quite coarse. E.g.
requesting a pixel-clock of 65MHz with a sys_clk of 132MHz results
in the divider being set to 3 ending up with 44MHz.
By preferring the doubled sys_clk as base, the divider instead ends
up as 5 yielding a pixel-clock of 52.8Mhz, which is a definite
improvement.
While at it, clamp the divider so that it does not overflow in case
it gets big.
Signed-off-by: Peter Rosin <[email protected]>
---
drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c | 18 +++++++++++-------
1 file changed, 11 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
index c38a479ada98..0d9d1042752a 100644
--- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
+++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
@@ -101,18 +101,22 @@ static void atmel_hlcdc_crtc_mode_set_nofb(struct drm_crtc *c)
(adj->crtc_hdisplay - 1) |
((adj->crtc_vdisplay - 1) << 16));
- cfg = 0;
+ cfg = ATMEL_HLCDC_CLKSEL;
- prate = clk_get_rate(crtc->dc->hlcdc->sys_clk);
+ prate = 2 * clk_get_rate(crtc->dc->hlcdc->sys_clk);
mode_rate = adj->crtc_clock * 1000;
- if ((prate / 2) < mode_rate) {
- prate *= 2;
- cfg |= ATMEL_HLCDC_CLKSEL;
- }
div = DIV_ROUND_UP(prate, mode_rate);
- if (div < 2)
+ if (div < 2) {
div = 2;
+ } else if (ATMEL_HLCDC_CLKDIV(div) & ~ATMEL_HLCDC_CLKDIV_MASK) {
+ /* The divider ended up too big, try a lower base rate. */
+ cfg &= ~ATMEL_HLCDC_CLKSEL;
+ prate /= 2;
+ div = DIV_ROUND_UP(prate, mode_rate);
+ if (ATMEL_HLCDC_CLKDIV(div) & ~ATMEL_HLCDC_CLKDIV_MASK)
+ div = ATMEL_HLCDC_CLKDIV_MASK;
+ }
cfg |= ATMEL_HLCDC_CLKDIV(div);
--
2.11.0
On Fri, 24 Aug 2018 11:24:56 +0200
Peter Rosin <[email protected]> wrote:
> Hi!
>
> Some background can be found here:
> https://lists.freedesktop.org/archives/dri-devel/2018-August/187182.html
>
> The "10 times" discriminator in patch 2/2 can certainly be discussed...
Queued to drm-misc-next.
Thanks,
Boris
>
> Cheers,
> Peter
>
> Changes since v1 https://lkml.org/lkml/2018/8/24/187
>
> - added {} to an if body for symmetry
> - reformatted comments a little bit
> - spelling/grammar fixes
>
> Peter Rosin (2):
> drm/atmel-hlcdc: prefer a higher rate clock as pixel-clock base
> drm/atmel-hlcdc: allow selecting a higher pixel-clock than requested
>
> drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c | 30 ++++++++++++++++++++------
> 1 file changed, 23 insertions(+), 7 deletions(-)
>