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

[Jani Nikula <jani.nikula@linux.intel.com>]

On Wed, 17 Oct 2018, Hans de Goede <j.w.r.degoede@gmail.com> 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.

I assume we don't do the fast path because intel_pipe_config_compare()
returns false due to crtc_clock and port_clock mismatch. The dmesg
should tell you the reason with drm.debugs enabled.

Now, the clock checks are already "fuzzy", and should account for slight
variations. I think the goal was the same as here, plus IIUC we may lose
some accuracy on the hardware roundtrip.

Please try adding more tolerance in intel_fuzzy_clock_check() and see if
that helps. The code looks like it allows 5% diff, but it's 2.5%.

Regardless of whether we end up changing the tolerance or adjusting the
state based on the hardware readout or what, I think doing the
adjustment in intel_dsi_vbt.c init is the wrong place. We shouldn't have
anything that depends on accessing the hardware there. I believe that's
what Ville was trying to say in [1].

BR,
Jani.


[1] http://mid.mail-archive.com/20180706141652.GK5565@intel.com


>
> Changes in v2:
> -Use intel_encoder_current_mode() to get the pclk setup by the GOP
>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  drivers/gpu/drm/i915/intel_dsi_vbt.c | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_dsi_vbt.c b/drivers/gpu/drm/i915/intel_dsi_vbt.c
> index ac83d6b89ae0..3387b187105c 100644
> --- a/drivers/gpu/drm/i915/intel_dsi_vbt.c
> +++ b/drivers/gpu/drm/i915/intel_dsi_vbt.c
> @@ -506,6 +506,7 @@ bool intel_dsi_vbt_init(struct intel_dsi *intel_dsi, u16 panel_id)
>  	struct mipi_config *mipi_config = dev_priv->vbt.dsi.config;
>  	struct mipi_pps_data *pps = dev_priv->vbt.dsi.pps;
>  	struct drm_display_mode *mode = dev_priv->vbt.lfp_lvds_vbt_mode;
> +	struct drm_display_mode *curr;
>  	u32 bpp;
>  	u32 tlpx_ns, extra_byte_count, bitrate, tlpx_ui;
>  	u32 ui_num, ui_den;
> @@ -583,6 +584,23 @@ 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)) {
> +		curr = intel_encoder_current_mode(&intel_dsi->base);
> +		if (curr) {
> +			DRM_DEBUG_KMS("Calculated pclk %d GOP %d\n",
> +				      pclk, curr->clock);
> +			if (curr->clock >= (pclk * 9 / 10) &&
> +			    curr->clock <= (pclk * 11 / 10))
> +				pclk = curr->clock;
> +
> +			kfree(curr);
> +		}
> +	}
> +
>  	intel_dsi->burst_mode_ratio = burst_mode_ratio;
>  	intel_dsi->pclk = pclk;

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx