Re: [PATCH 4/4] drm/i915/dsi: Read back pclk set by GOP and use that as pclk (v3)

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

On Fri, May 24, 2019 at 06:30:20PM +0200, Hans de Goede wrote:
> 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.
> 
> Changes in v2:
> -Use intel_encoder_current_mode() to get the pclk setup by the GOP
> 
> Changes in v3:
> -Back to the readback approach, skipping the dsi_pll.ctrl / .dev checks
>  in intel_pipe_config_compare() when adjust is set leads to:
>  [drm:pipe_config_err [i915]] *ERROR* mismatch in dsi_pll.ctrl (...)
>  [drm:pipe_config_err [i915]] *ERROR* mismatch in dsi_pll.div (...)
> -Do the readback and pclk overriding from vlv_dsi_init(), rather then from
>  intel_dsi_vbt_init() as the vbt code should not be touching the hw
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  drivers/gpu/drm/i915/vlv_dsi.c | 22 ++++++++++++++++++----
>  1 file changed, 18 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/vlv_dsi.c b/drivers/gpu/drm/i915/vlv_dsi.c
> index 3329ccf3b346..49975dd84ff4 100644
> --- a/drivers/gpu/drm/i915/vlv_dsi.c
> +++ b/drivers/gpu/drm/i915/vlv_dsi.c
> @@ -1701,7 +1701,7 @@ void vlv_dsi_init(struct drm_i915_private *dev_priv)
>  	struct drm_encoder *encoder;
>  	struct intel_connector *intel_connector;
>  	struct drm_connector *connector;
> -	struct drm_display_mode *fixed_mode;
> +	struct drm_display_mode *current_mode, *fixed_mode;
>  	enum port port;
>  
>  	DRM_DEBUG_KMS("\n");
> @@ -1745,6 +1745,9 @@ void vlv_dsi_init(struct drm_i915_private *dev_priv)
>  	intel_connector->get_hw_state = intel_connector_get_hw_state;
>  
>  	intel_encoder->port = port;
> +	intel_encoder->type = INTEL_OUTPUT_DSI;
> +	intel_encoder->power_domain = POWER_DOMAIN_PORT_DSI;
> +	intel_encoder->cloneable = 0;
>  
>  	/*
>  	 * On BYT/CHV, pipe A maps to MIPI DSI port A, pipe B maps to MIPI DSI
> @@ -1782,6 +1785,20 @@ void vlv_dsi_init(struct drm_i915_private *dev_priv)
>  		goto err;
>  	}
>  
> +	/* Use clock read-back from current hw-state for fastboot */
> +	current_mode = intel_encoder_current_mode(intel_encoder);
> +	if (current_mode) {
> +		DRM_DEBUG_KMS("Calculated pclk %d GOP %d\n",
> +			      intel_dsi->pclk, current_mode->clock);
> +		if (intel_fuzzy_clock_check(intel_dsi->pclk,
> +					    current_mode->clock)) {
> +			DRM_DEBUG_KMS("Using GOP pclk\n");
> +			intel_dsi->pclk = current_mode->clock;
> +		}

I wonder if we should be checking whether the mode is otherwise
identical to whatever we got from VBT? Though I suppose that shouldn't
really happen.

The whole dsi clock handling is a proper mess, but looks like ->pclk
is supposed to be the burst clock so I think this should end up doing
more or less the right thing because we seem to stuffing the DPLL
readout into the mode->clock.

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> +
> +		kfree(current_mode);
> +	}
> +
>  	vlv_dphy_param_init(intel_dsi);
>  
>  	/*
> @@ -1799,9 +1816,6 @@ void vlv_dsi_init(struct drm_i915_private *dev_priv)
>  		}
>  	}
>  
> -	intel_encoder->type = INTEL_OUTPUT_DSI;
> -	intel_encoder->power_domain = POWER_DOMAIN_PORT_DSI;
> -	intel_encoder->cloneable = 0;
>  	drm_connector_init(dev, connector, &intel_dsi_connector_funcs,
>  			   DRM_MODE_CONNECTOR_DSI);
>  
> -- 
> 2.21.0

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