Re: [Intel-gfx] [PATCH 1/2] drm/i915/edp: read edp display control registers unconditionally

[Manasi Navare <manasi.d.navare@intel.com>]

On Thu, Oct 26, 2017 at 05:29:31PM +0300, Jani Nikula wrote:
> Per my reading of the eDP spec, DP_DPCD_DISPLAY_CONTROL_CAPABLE bit in
> DP_EDP_CONFIGURATION_CAP should be set if the eDP display control
> registers starting at offset DP_EDP_DPCD_REV are "enabled". Currently we
> check the bit before reading the registers, and DP_EDP_DPCD_REV is the
> only way to detect eDP revision.
> 
> Turns out there are (likely buggy) displays that require eDP 1.4+
> features, such as supported link rates and link rate select, but do not
> have the bit set. Read the display control registers
> unconditionally. They are supposed to read zero anyway if they are not
> supported, so there should be no harm in this.
> 
> This fixes the referenced bug by enabling the eDP version check, and
> thus reading of the supported link rates. The panel in question has 0 in
> DP_MAX_LINK_RATE which is only supported in eDP 1.4+. Without the
> supported link rates method we default to RBR which is insufficient for
> the panel native mode. As a curiosity, the panel also has a bogus value
> of 0x12 in DP_EDP_DPCD_REV, but that passes our check for >= DP_EDP_14
> (which is 0x03).
>

This is the second wierd eDP panel case/bug that I have seen recently
where it doesnt not behave as per the spec.
Good catch, the fix looks good to me.
 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=103400
> Reported-and-tested-by: Nicolas P. <issun.artiste@gmail.com>
> Cc: Ville Syrj�l� <ville.syrjala@linux.intel.com>
> Cc: stable@vger.kernel.org
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>

Reviewed-by: Manasi Navare <manasi.d.navare@intel.com>

> ---
>  drivers/gpu/drm/i915/intel_dp.c | 13 ++++++++++---
>  1 file changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index aa75f55eeb61..158438bb0389 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -3735,9 +3735,16 @@ intel_edp_init_dpcd(struct intel_dp *intel_dp)
>  
>  	}
>  
> -	/* Read the eDP Display control capabilities registers */
> -	if ((intel_dp->dpcd[DP_EDP_CONFIGURATION_CAP] & DP_DPCD_DISPLAY_CONTROL_CAPABLE) &&
> -	    drm_dp_dpcd_read(&intel_dp->aux, DP_EDP_DPCD_REV,
> +	/*
> +	 * Read the eDP display control registers.
> +	 *
> +	 * Do this independent of DP_DPCD_DISPLAY_CONTROL_CAPABLE bit in
> +	 * DP_EDP_CONFIGURATION_CAP, because some buggy displays do not have it
> +	 * set, but require eDP 1.4+ detection (e.g. for supported link rates
> +	 * method). The display control registers should read zero if they're
> +	 * not supported anyway.
> +	 */
> +	if (drm_dp_dpcd_read(&intel_dp->aux, DP_EDP_DPCD_REV,
>  			     intel_dp->edp_dpcd, sizeof(intel_dp->edp_dpcd)) ==
>  			     sizeof(intel_dp->edp_dpcd))
>  		DRM_DEBUG_KMS("EDP DPCD : %*ph\n", (int) sizeof(intel_dp->edp_dpcd),
> -- 
> 2.11.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx