Re: [PATCH 3/3] drm/omap: fix i886 work-around

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

Hi Tomi,

Thank you for the patch.

On Tuesday 13 Jun 2017 12:02:10 Tomi Valkeinen wrote:
> 7d267f068a8b4944d52e8b0ae4c8fcc1c1c5c5ba ("drm/omap: work-around for
> errata i886") changed how the PLL dividers and multipliers are
> calculated. While the new way should work fine for all the PLLs, it
> breaks omap5 PLLs. The issues seen are rather odd: seemed that the
> output clock rate is half of what we asked. It is unclear what's causing
> there issues.

Does this patch result in PLL parameters for half of the expected rate, or 
does it compute the expected parameters, with the PLL producing half of the 
expected rate for an unknown reason ?

> As a work-around this patch adds a "errata_i886" flag, which is set only
> for DRA7's PLLs, and the PLL setup is done according to that flag.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> ---
>  drivers/gpu/drm/omapdrm/dss/dss.h       |  3 +++
>  drivers/gpu/drm/omapdrm/dss/pll.c       | 29 ++++++++++++++++++++---------
>  drivers/gpu/drm/omapdrm/dss/video-pll.c |  2 ++
>  3 files changed, 25 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/omapdrm/dss/dss.h
> b/drivers/gpu/drm/omapdrm/dss/dss.h index 5dd29c98143a..8ea3daa9aed9 100644
> --- a/drivers/gpu/drm/omapdrm/dss/dss.h
> +++ b/drivers/gpu/drm/omapdrm/dss/dss.h
> @@ -174,6 +174,9 @@ struct dss_pll_hw {
>  	bool has_freqsel;
>  	bool has_selfreqdco;
>  	bool has_refsel;
> +
> +	/* DRA7 errata i886: use high N & M to avoid jitter */
> +	bool errata_i886;
>  };
> 
>  struct dss_pll {
> diff --git a/drivers/gpu/drm/omapdrm/dss/pll.c
> b/drivers/gpu/drm/omapdrm/dss/pll.c index 5e221302768b..9d9d9d42009b 100644
> --- a/drivers/gpu/drm/omapdrm/dss/pll.c
> +++ b/drivers/gpu/drm/omapdrm/dss/pll.c
> @@ -215,8 +215,8 @@ bool dss_pll_calc_a(const struct dss_pll *pll, unsigned
> long clkin, dss_pll_calc_func func, void *data)
>  {
>  	const struct dss_pll_hw *hw = pll->hw;
> -	int n, n_min, n_max;
> -	int m, m_min, m_max;
> +	int n, n_start, n_stop, n_inc;
> +	int m, m_start, m_stop, m_inc;
>  	unsigned long fint, clkdco;
>  	unsigned long pll_hw_max;
>  	unsigned long fint_hw_min, fint_hw_max;
> @@ -226,22 +226,33 @@ bool dss_pll_calc_a(const struct dss_pll *pll,
> unsigned long clkin, fint_hw_min = hw->fint_min;
>  	fint_hw_max = hw->fint_max;
> 
> -	n_min = max(DIV_ROUND_UP(clkin, fint_hw_max), 1ul);
> -	n_max = min((unsigned)(clkin / fint_hw_min), hw->n_max);
> +	n_start = max(DIV_ROUND_UP(clkin, fint_hw_max), 1ul);
> +	n_stop = min((unsigned)(clkin / fint_hw_min), hw->n_max);
> +	n_inc = 1;
> +
> +	if (hw->errata_i886) {
> +		swap(n_start, n_stop);
> +		n_inc = -1;
> +	}
> 
>  	pll_max = pll_max ? pll_max : ULONG_MAX;
> 
> -	/* Try to find high N & M to avoid jitter (DRA7 errata i886) */
> -	for (n = n_max; n >= n_min; --n) {
> +	for (n = n_start; n != n_stop; n += n_inc) {
>  		fint = clkin / n;
> 
> -		m_min = max(DIV_ROUND_UP(DIV_ROUND_UP(pll_min, fint), 2),
> +		m_start = max(DIV_ROUND_UP(DIV_ROUND_UP(pll_min, fint), 2),
>  				1ul);
> -		m_max = min3((unsigned)(pll_max / fint / 2),
> +		m_stop = min3((unsigned)(pll_max / fint / 2),
>  				(unsigned)(pll_hw_max / fint / 2),
>  				hw->m_max);
> +		m_inc = 1;
> +
> +		if (hw->errata_i886) {
> +			swap(m_start, m_stop);
> +			m_inc = -1;
> +		}
> 
> -		for (m = m_max; m >= m_min; --m) {
> +		for (m = m_start; m != m_stop; m += m_inc) {
>  			clkdco = 2 * m * fint;
> 
>  			if (func(n, m, fint, clkdco, data))
> diff --git a/drivers/gpu/drm/omapdrm/dss/video-pll.c
> b/drivers/gpu/drm/omapdrm/dss/video-pll.c index 7429de928d4e..8201ecf826d9
> 100644
> --- a/drivers/gpu/drm/omapdrm/dss/video-pll.c
> +++ b/drivers/gpu/drm/omapdrm/dss/video-pll.c
> @@ -131,6 +131,8 @@ static const struct dss_pll_hw dss_dra7_video_pll_hw = {
> .mX_lsb[3] = 5,
> 
>  	.has_refsel = true,
> +
> +	.errata_i886 = true,
>  };
> 
>  struct dss_pll *dss_video_pll_init(struct platform_device *pdev, int id,

-- 
Regards,

Laurent Pinchart

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel