| Message ID | 1490348452-20851-2-git-send-email-tomi.valkeinen@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 03/24/17 11:40, Tomi Valkeinen wrote: > DRA7 errata i886 (FPDLink PLL Unlocks With Certain SoC PLL M/N Values) > says that FPDLink is sensitive to jitter on the vout clock, and that low > PLL M and N values result in more jitter than high M and N values. > > This patch implements a workaround for the problem by changing the PLL > setup to search for clocks starting from high M and N values, instead of > low values. This should not cause any functional change, and only > reduces the jitter. > > Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com> > --- > drivers/gpu/drm/omapdrm/dss/pll.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/omapdrm/dss/pll.c b/drivers/gpu/drm/omapdrm/dss/pll.c > index 0a76c89cdc2e..65c478d85caa 100644 > --- a/drivers/gpu/drm/omapdrm/dss/pll.c > +++ b/drivers/gpu/drm/omapdrm/dss/pll.c > @@ -231,7 +231,7 @@ bool dss_pll_calc_a(const struct dss_pll *pll, unsigned long clkin, > > pll_max = pll_max ? pll_max : ULONG_MAX; > > - for (n = n_start; n <= n_stop; ++n) { > + for (n = n_stop; n >= n_start; --n) { The patch looks simple like this, but just reading the code without a hint what is going on may be confusing. Loops beginning from stop and ending in start and all. Could these variables be called [nm]_max and [nm]_min, or maybe one simple comment to tell why we are going from max to min, or both? > fint = clkin / n; > > m_start = max(DIV_ROUND_UP(DIV_ROUND_UP(pll_min, fint), 2), > @@ -240,7 +240,7 @@ bool dss_pll_calc_a(const struct dss_pll *pll, unsigned long clkin, > (unsigned)(pll_hw_max / fint / 2), > hw->m_max); > > - for (m = m_start; m <= m_stop; ++m) { > + for (m = m_stop; m >= m_start; --m) { > clkdco = 2 * m * fint; > > if (func(n, m, fint, clkdco, data)) >
On 28/03/17 08:45, Jyri Sarha wrote: > On 03/24/17 11:40, Tomi Valkeinen wrote: >> DRA7 errata i886 (FPDLink PLL Unlocks With Certain SoC PLL M/N Values) >> says that FPDLink is sensitive to jitter on the vout clock, and that low >> PLL M and N values result in more jitter than high M and N values. >> >> This patch implements a workaround for the problem by changing the PLL >> setup to search for clocks starting from high M and N values, instead of >> low values. This should not cause any functional change, and only >> reduces the jitter. >> >> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com> >> --- >> drivers/gpu/drm/omapdrm/dss/pll.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/gpu/drm/omapdrm/dss/pll.c b/drivers/gpu/drm/omapdrm/dss/pll.c >> index 0a76c89cdc2e..65c478d85caa 100644 >> --- a/drivers/gpu/drm/omapdrm/dss/pll.c >> +++ b/drivers/gpu/drm/omapdrm/dss/pll.c >> @@ -231,7 +231,7 @@ bool dss_pll_calc_a(const struct dss_pll *pll, unsigned long clkin, >> >> pll_max = pll_max ? pll_max : ULONG_MAX; >> >> - for (n = n_start; n <= n_stop; ++n) { >> + for (n = n_stop; n >= n_start; --n) { > > The patch looks simple like this, but just reading the code without a > hint what is going on may be confusing. Loops beginning from stop and > ending in start and all. > > Could these variables be called [nm]_max and [nm]_min, or maybe one > simple comment to tell why we are going from max to min, or both? Indeed, it's confusing. I did both. Tomi
diff --git a/drivers/gpu/drm/omapdrm/dss/pll.c b/drivers/gpu/drm/omapdrm/dss/pll.c index 0a76c89cdc2e..65c478d85caa 100644 --- a/drivers/gpu/drm/omapdrm/dss/pll.c +++ b/drivers/gpu/drm/omapdrm/dss/pll.c @@ -231,7 +231,7 @@ bool dss_pll_calc_a(const struct dss_pll *pll, unsigned long clkin, pll_max = pll_max ? pll_max : ULONG_MAX; - for (n = n_start; n <= n_stop; ++n) { + for (n = n_stop; n >= n_start; --n) { fint = clkin / n; m_start = max(DIV_ROUND_UP(DIV_ROUND_UP(pll_min, fint), 2), @@ -240,7 +240,7 @@ bool dss_pll_calc_a(const struct dss_pll *pll, unsigned long clkin, (unsigned)(pll_hw_max / fint / 2), hw->m_max); - for (m = m_start; m <= m_stop; ++m) { + for (m = m_stop; m >= m_start; --m) { clkdco = 2 * m * fint; if (func(n, m, fint, clkdco, data))
DRA7 errata i886 (FPDLink PLL Unlocks With Certain SoC PLL M/N Values) says that FPDLink is sensitive to jitter on the vout clock, and that low PLL M and N values result in more jitter than high M and N values. This patch implements a workaround for the problem by changing the PLL setup to search for clocks starting from high M and N values, instead of low values. This should not cause any functional change, and only reduces the jitter. Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com> --- drivers/gpu/drm/omapdrm/dss/pll.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)