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

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