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@xxxxxxxxx> > Cc: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > Cc: stable@xxxxxxxxxxxxxxx > Signed-off-by: Jani Nikula <jani.nikula@xxxxxxxxx> Reviewed-by: Manasi Navare <manasi.d.navare@xxxxxxxxx> > --- > 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@xxxxxxxxxxxxxxxxxxxxx > https://lists.freedesktop.org/mailman/listinfo/intel-gfx