From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga03.intel.com ([134.134.136.65]:27029 "EHLO mga03.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932377AbdJZTgK (ORCPT ); Thu, 26 Oct 2017 15:36:10 -0400 Date: Thu, 26 Oct 2017 12:40:59 -0700 From: Manasi Navare To: Jani Nikula Cc: intel-gfx@lists.freedesktop.org, stable@vger.kernel.org Subject: Re: [Intel-gfx] [PATCH 1/2] drm/i915/edp: read edp display control registers unconditionally Message-ID: <20171026194058.GD19276@intel.com> References: <20171026142932.17737-1-jani.nikula@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20171026142932.17737-1-jani.nikula@intel.com> Sender: stable-owner@vger.kernel.org List-ID: 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. > Cc: Ville Syrj�l� > Cc: stable@vger.kernel.org > Signed-off-by: Jani Nikula Reviewed-by: Manasi Navare > --- > 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