Re: [PATCH 4/4] drm/i915/intel_dsi: Read back pclk set by GOP and use that as pclk

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Tue, Jun 19, 2018 at 10:18:27PM +0200, Hans de Goede wrote:
> On BYT and CHT the GOP sometimes initializes the pclk at a (slightly)
> different frequency then the pclk which we've calculated.
> 
> This commit makes the DSI code read-back the pclk set by the GOP and
> if that is within a reasonable margin of the calculated pclk, uses
> that instead.
> 
> This fixes the first modeset being a full modeset instead of a
> fast modeset on systems where the GOP pclk is different.
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  drivers/gpu/drm/i915/intel_dsi_vbt.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dsi_vbt.c b/drivers/gpu/drm/i915/intel_dsi_vbt.c
> index 4d6ffa7b3e7b..d4cc6099012c 100644
> --- a/drivers/gpu/drm/i915/intel_dsi_vbt.c
> +++ b/drivers/gpu/drm/i915/intel_dsi_vbt.c
> @@ -517,6 +517,7 @@ bool intel_dsi_vbt_init(struct intel_dsi *intel_dsi, u16 panel_id)
>  	u32 mul;
>  	u16 burst_mode_ratio;
>  	enum port port;
> +	enum pipe pipe;
>  
>  	DRM_DEBUG_KMS("\n");
>  
> @@ -583,6 +584,19 @@ bool intel_dsi_vbt_init(struct intel_dsi *intel_dsi, u16 panel_id)
>  	} else
>  		burst_mode_ratio = 100;
>  
> +	/*
> +	 * On BYT / CRC the GOP sometimes picks a slightly different pclk,
> +	 * read back the GOP configured pclk and prefer it over ours.
> +	 */
> +	if ((IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) &&
> +	    intel_dsi_get_hw_state(&intel_dsi->base, &pipe)) {
> +		u32 gop = intel_dsi_get_pclk(&intel_dsi->base, bpp, NULL);
> +
> +		DRM_DEBUG_KMS("Calculated pclk %d GOP %d\n", pclk, gop);
> +		if (gop >= (pclk * 9 / 10) && gop <= (pclk * 11 / 10))
> +			pclk = gop;
> +	}

Is the gop acually picking a different clock that what we pick in the
end, or is it just that the value in the vbt doesn't quite match what we
(and the gop) end up using on account of limitations of the pll?

For that particular problem I think I had patches long ago to go through
the pll computation during init so that we basically fix up the slightly
bogus clock from the vbt.

Any kind of hack that involves reading out the hardware state should go
into something like intel_sanitize_encoder(). Actually by that time we
have already read out the hw state, so it shouldn't require any
modifications to the existing dsi code itself.

> +
>  	intel_dsi->burst_mode_ratio = burst_mode_ratio;
>  	intel_dsi->pclk = pclk;
>  
> -- 
> 2.17.1

-- 
Ville Syrjälä
Intel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel