Re: [PATCH v2 8/8] drm/i915/lspcon: Add workaround for resuming in PCON mode

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

On Mon, 24 Oct 2016, Imre Deak <imre.deak@intel.com> wrote:
> On my APL the LSPCON firmware resumes in PCON mode as opposed to the
> expected LS mode. It also appears to be in a state where AUX DPCD reads
> will succeed but return garbage recovering only after a few hundreds of
> milliseconds. After the recovery time DPCD reads will result in the
> correct values and things will continue to work. If I2C over AUX is
> attempted during this recovery time (implying an AUX write transaction)
> the firmware won't recover and will stay in this broken state.
>
> As a workaround check if the firmware is in PCON state after resume and
> if so wait until the correct DPCD values are returned. For this we
> compare the branch descriptor with the one we cached during init time.
> If the firmware was in the LS state, we skip the w/a and continue as
> before.
>
> v2:
> - Use the DP descriptor value cached in intel_dp. (Jani)
> - Get to intel_dp using container_of(), instead of a cached ptr.
>   (Shashank)
> - Use usleep_range() instead of msleep().
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=98353
> Cc: Shashank Sharma <shashank.sharma@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Signed-off-by: Imre Deak <imre.deak@intel.com>

I don't think we properly understand what's going on with the device. In
the mean time, limp on with this if this fixes stuff.

Acked-by: Jani Nikula <jani.nikula@intel.com>

> ---
>  drivers/gpu/drm/i915/intel_dp.c     |  2 +-
>  drivers/gpu/drm/i915/intel_drv.h    |  3 +++
>  drivers/gpu/drm/i915/intel_lspcon.c | 37 ++++++++++++++++++++++++++++++++++++-
>  3 files changed, 40 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 043993f..e9be955 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -1451,7 +1451,7 @@ static void intel_dp_print_rates(struct intel_dp *intel_dp)
>  	DRM_DEBUG_KMS("common rates: %s\n", str);
>  }
>  
> -static bool
> +bool
>  __intel_dp_read_desc(struct intel_dp *intel_dp, struct intel_dp_desc *desc)
>  {
>  	u32 base = drm_dp_is_branch(intel_dp->dpcd) ? DP_BRANCH_OUI :
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 45f55b5..f2b6c59 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -974,6 +974,7 @@ struct intel_dp {
>  struct intel_lspcon {
>  	bool active;
>  	enum drm_lspcon_mode mode;
> +	bool desc_valid;
>  };
>  
>  struct intel_digital_port {
> @@ -1461,6 +1462,8 @@ static inline unsigned int intel_dp_unused_lane_mask(int lane_count)
>  }
>  
>  bool intel_dp_read_dpcd(struct intel_dp *intel_dp);
> +bool __intel_dp_read_desc(struct intel_dp *intel_dp,
> +			  struct intel_dp_desc *desc);
>  bool intel_dp_read_desc(struct intel_dp *intel_dp);
>  
>  /* intel_dp_aux_backlight.c */
> diff --git a/drivers/gpu/drm/i915/intel_lspcon.c b/drivers/gpu/drm/i915/intel_lspcon.c
> index 3dc5a0b..daa5234 100644
> --- a/drivers/gpu/drm/i915/intel_lspcon.c
> +++ b/drivers/gpu/drm/i915/intel_lspcon.c
> @@ -97,8 +97,43 @@ static bool lspcon_probe(struct intel_lspcon *lspcon)
>  	return true;
>  }
>  
> +static void lspcon_resume_in_pcon_wa(struct intel_lspcon *lspcon)
> +{
> +	struct intel_dp *intel_dp = lspcon_to_intel_dp(lspcon);
> +	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 (!memcmp(&intel_dp->desc, &desc, sizeof(desc))) {
> +			DRM_DEBUG_KMS("LSPCON recovering in PCON mode after %u ms\n",
> +				      jiffies_to_msecs(jiffies - start));
> +			return;
> +		}
> +
> +		if (time_after(jiffies, start + msecs_to_jiffies(1000)))
> +			break;
> +
> +		usleep_range(10000, 15000);
> +	}
> +
> +	DRM_DEBUG_KMS("LSPCON DP descriptor mismatch after resume\n");
> +}
> +
>  void lspcon_resume(struct intel_lspcon *lspcon)
>  {
> +	lspcon_resume_in_pcon_wa(lspcon);
> +
>  	if (lspcon_change_mode(lspcon, DRM_LSPCON_MODE_PCON, true))
>  		DRM_ERROR("LSPCON resume failed\n");
>  	else
> @@ -143,7 +178,7 @@ bool lspcon_init(struct intel_digital_port *intel_dig_port)
>  		return false;
>  	}
>  
> -	intel_dp_read_desc(dp);
> +	lspcon->desc_valid = intel_dp_read_desc(dp);
>  
>  	DRM_DEBUG_KMS("Success: LSPCON init\n");
>  	return true;

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