[Intel-gfx] [PATCH 3/4] drm/i915/lspcon: Wait for expected LSPCON mode to settle
Sharma, Shashank
shashank.sharma at intel.com
Tue Nov 22 15:36:05 UTC 2016
Regards
Shashank
On 11/22/2016 12:45 AM, Imre Deak wrote:
> Some LSPCON adaptors may return an incorrect LSPCON mode right after
> waking from DP Sleep state. This is the case at least for the ParadTech
> PS175 adaptor, both when waking because of exiting the DP Sleep to
> active state, or due to any other AUX CH transfer. We can determine the
> current expected mode based on whether the DPCD area is accessible,
> since according to the LSPCON spec this area is only accesible
> in PCON mode.
>
> This wait will avoid us trying to change the mode, while the current
> expected mode hasn't settled yet and start link training before the
> adaptor thinks it's in PCON mode after waking from DP Sleep state.
>
> Cc: Shashank Sharma <shashank.sharma at intel.com>
> Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
> Cc: Jani Nikula <jani.nikula at intel.com>
> Signed-off-by: Imre Deak <imre.deak at intel.com>
> ---
> drivers/gpu/drm/i915/intel_dp.c | 5 +++
> drivers/gpu/drm/i915/intel_drv.h | 1 +
> drivers/gpu/drm/i915/intel_lspcon.c | 70 ++++++++++++++++++++++++++++++++-----
> 3 files changed, 68 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 16c19d78..8026a83 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -2401,6 +2401,8 @@ void intel_dp_sink_dpms(struct intel_dp *intel_dp, int mode)
> ret = drm_dp_dpcd_writeb(&intel_dp->aux, DP_SET_POWER,
> DP_SET_POWER_D3);
> } else {
> + struct intel_lspcon *lspcon = dp_to_lspcon(intel_dp);
> +
> /*
> * When turning on, we need to retry for 1ms to give the sink
> * time to wake up.
> @@ -2412,6 +2414,9 @@ void intel_dp_sink_dpms(struct intel_dp *intel_dp, int mode)
> break;
> msleep(1);
> }
> +
> + if (ret == 1 && lspcon->active)
> + lspcon_wait_pcon_mode(lspcon);
> }
>
> if (ret != 1)
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index cf47e8a..d165904 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1830,4 +1830,5 @@ void intel_color_load_luts(struct drm_crtc_state *crtc_state);
> /* intel_lspcon.c */
> bool lspcon_init(struct intel_digital_port *intel_dig_port);
> void lspcon_resume(struct intel_lspcon *lspcon);
> +void lspcon_wait_pcon_mode(struct intel_lspcon *lspcon);
> #endif /* __INTEL_DRV_H__ */
> diff --git a/drivers/gpu/drm/i915/intel_lspcon.c b/drivers/gpu/drm/i915/intel_lspcon.c
> index 5013124..281127d 100644
> --- a/drivers/gpu/drm/i915/intel_lspcon.c
> +++ b/drivers/gpu/drm/i915/intel_lspcon.c
> @@ -35,16 +35,54 @@ static struct intel_dp *lspcon_to_intel_dp(struct intel_lspcon *lspcon)
> return &dig_port->dp;
> }
>
> +static const char *lspcon_mode_name(enum drm_lspcon_mode mode)
> +{
> + switch (mode) {
> + case DRM_LSPCON_MODE_PCON:
> + return "PCON";
> + case DRM_LSPCON_MODE_LS:
> + return "LS";
> + case DRM_LSPCON_MODE_INVALID:
> + return "INVALID";
> + default:
> + MISSING_CASE(mode);
> + return "INVALID";
> + }
> +}
> +
> static enum drm_lspcon_mode lspcon_get_current_mode(struct intel_lspcon *lspcon)
> {
> - enum drm_lspcon_mode current_mode = DRM_LSPCON_MODE_INVALID;
> + enum drm_lspcon_mode current_mode;
> struct i2c_adapter *adapter = &lspcon_to_intel_dp(lspcon)->aux.ddc;
>
> - if (drm_lspcon_get_mode(adapter, ¤t_mode))
> + if (drm_lspcon_get_mode(adapter, ¤t_mode)) {
> DRM_ERROR("Error reading LSPCON mode\n");
> - else
> - DRM_DEBUG_KMS("Current LSPCON mode %s\n",
> - current_mode == DRM_LSPCON_MODE_PCON ? "PCON" : "LS");
> + return DRM_LSPCON_MODE_INVALID;
> + }
> + return current_mode;
> +}
> +
> +static enum drm_lspcon_mode lspcon_wait_mode(struct intel_lspcon *lspcon,
> + enum drm_lspcon_mode mode)
> +{
> + enum drm_lspcon_mode current_mode;
> +
> + current_mode = lspcon_get_current_mode(lspcon);
> + if (current_mode == mode || current_mode == DRM_LSPCON_MODE_INVALID)
> + goto out;
> +
> + DRM_DEBUG_KMS("Waiting for LSPCON mode %s to settle\n",
> + lspcon_mode_name(mode));
> +
Can we remove above lines, and start from below lines ? I guess wait_for
checks the condition first and
then executes wait ? Also, a comment stating 100ms wait ?
- Shashank
> + wait_for((current_mode = lspcon_get_current_mode(lspcon)) == mode ||
> + current_mode == DRM_LSPCON_MODE_INVALID, 100);
> + if (current_mode != mode)
> + DRM_DEBUG_KMS("LSPCON mode hasn't settled\n");
> +
> +out:
> + DRM_DEBUG_KMS("Current LSPCON mode %s\n",
> + lspcon_mode_name(current_mode));
> +
> return current_mode;
> }
>
> @@ -97,8 +135,10 @@ static bool lspcon_probe(struct intel_lspcon *lspcon)
> {
> enum drm_dp_dual_mode_type adaptor_type;
> struct i2c_adapter *adapter = &lspcon_to_intel_dp(lspcon)->aux.ddc;
> + enum drm_lspcon_mode expected_mode;
>
> - lspcon_wake_native_aux_ch(lspcon);
> + expected_mode = lspcon_wake_native_aux_ch(lspcon) ?
> + DRM_LSPCON_MODE_PCON : DRM_LSPCON_MODE_LS;
>
> /* Lets probe the adaptor and check its type */
> adaptor_type = drm_dp_dual_mode_detect(adapter);
> @@ -110,7 +150,7 @@ static bool lspcon_probe(struct intel_lspcon *lspcon)
>
> /* Yay ... got a LSPCON device */
> DRM_DEBUG_KMS("LSPCON detected\n");
> - lspcon->mode = lspcon_get_current_mode(lspcon);
> + lspcon->mode = lspcon_wait_mode(lspcon, expected_mode);
> lspcon->active = true;
> return true;
> }
> @@ -150,8 +190,17 @@ static void lspcon_resume_in_pcon_wa(struct intel_lspcon *lspcon)
>
> void lspcon_resume(struct intel_lspcon *lspcon)
> {
> - if (lspcon_wake_native_aux_ch(lspcon))
> + enum drm_lspcon_mode expected_mode;
> +
> + if (lspcon_wake_native_aux_ch(lspcon)) {
> + expected_mode = DRM_LSPCON_MODE_PCON;
> lspcon_resume_in_pcon_wa(lspcon);
> + } else {
> + expected_mode = DRM_LSPCON_MODE_LS;
> + }
> +
> + if (lspcon_wait_mode(lspcon, expected_mode) == DRM_LSPCON_MODE_PCON)
> + return;
>
> if (lspcon_change_mode(lspcon, DRM_LSPCON_MODE_PCON, true))
> DRM_ERROR("LSPCON resume failed\n");
> @@ -159,6 +208,11 @@ void lspcon_resume(struct intel_lspcon *lspcon)
> DRM_DEBUG_KMS("LSPCON resume success\n");
> }
>
> +void lspcon_wait_pcon_mode(struct intel_lspcon *lspcon)
> +{
> + lspcon_wait_mode(lspcon, DRM_LSPCON_MODE_PCON);
> +}
> +
> bool lspcon_init(struct intel_digital_port *intel_dig_port)
> {
> struct intel_dp *dp = &intel_dig_port->dp;
More information about the Intel-gfx
mailing list