Re: [PATCH 3/4] drm/i915/lspcon: Remove DPCD compare based resume time workaround

[Imre Deak <imre.deak@intel.com>]

On Sat, Jan 28, 2017 at 10:36:16AM +0530, Sharma, Shashank wrote:
> Regards
> 
> Shashank
> 
> 
> On 1/27/2017 3:09 PM, Imre Deak wrote:
> >This effectively reverts
> >commit 489375c866c111f16cea93b2467ebe59c9022cc7
> >Author: Imre Deak <imre.deak@intel.com>
> >Date:   Mon Oct 24 19:33:31 2016 +0300
> >
> >     drm/i915/lspcon: Add workaround for resuming in PCON mode
> >
> >The workaround was added without considering that HPD is low during
> >the failed AUX transfers the WA fixed. Since the previous patch we
> >wait for HPD to get asserted. My tests also show that this happens
> >_after_ the DPCD reads start to return correct values. This
> >suggests that we don't need this WA any more, let's try to remove
> >it to reduce the clutter.
> >
> >Cc: Shashank Sharma <shashank.sharma@intel.com>
> >Cc: Jani Nikula <jani.nikula@linux.intel.com>
> >Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> >Signed-off-by: Imre Deak <imre.deak@intel.com>
> >---
> >  drivers/gpu/drm/i915/intel_drv.h    |  1 -
> >  drivers/gpu/drm/i915/intel_lspcon.c | 17 ++---------------
> >  2 files changed, 2 insertions(+), 16 deletions(-)
> >
> >diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> >index b9cde11..b2882ff 100644
> >--- a/drivers/gpu/drm/i915/intel_drv.h
> >+++ b/drivers/gpu/drm/i915/intel_drv.h
> >@@ -989,7 +989,6 @@ struct intel_dp {
> >  struct intel_lspcon {
> >  	bool active;
> >  	enum drm_lspcon_mode mode;
> >-	bool desc_valid;
> >  };
> >  struct intel_digital_port {
> >diff --git a/drivers/gpu/drm/i915/intel_lspcon.c b/drivers/gpu/drm/i915/intel_lspcon.c
> >index c300647..71cbe9c 100644
> >--- a/drivers/gpu/drm/i915/intel_lspcon.c
> >+++ b/drivers/gpu/drm/i915/intel_lspcon.c
> >@@ -162,21 +162,8 @@ static void lspcon_resume_in_pcon_wa(struct intel_lspcon *lspcon)
> >  	struct drm_i915_private *dev_priv = to_i915(dig_port->base.base.dev);
> >  	unsigned long start = jiffies;
> >-	if (!lspcon->desc_valid)
> >-		return;
> >-
> >  	while (1) {
> >-		struct intel_dp_desc desc;
> >-
> >-		/*
> >-		 * The w/a only applies in PCON mode and we don't expect any
> >-		 * AUX errors.
> >-		 */
> >-		if (!__intel_dp_read_desc(intel_dp, &desc))
> >-			return;
> >-
> >-		if (intel_digital_port_connected(dev_priv, dig_port) &&
> >-		    !memcmp(&intel_dp->desc, &desc, sizeof(desc))) {
> >+		if (intel_digital_port_connected(dev_priv, dig_port)) {
> Again, does it matter, as in PCON mode live_status will be always true ?

See my answer for the previous patch.

> - Shashank
> >  			DRM_DEBUG_KMS("LSPCON recovering in PCON mode after %u ms\n",
> >  				      jiffies_to_msecs(jiffies - start));
> >  			return;
> >@@ -253,7 +240,7 @@ bool lspcon_init(struct intel_digital_port *intel_dig_port)
> >  		return false;
> >  	}
> >-	lspcon->desc_valid = intel_dp_read_desc(dp);
> >+	intel_dp_read_desc(dp);
> >  	DRM_DEBUG_KMS("Success: LSPCON init\n");
> >  	return true;
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx