* [PATCH 1/2] drm/i915/edp: read edp display control registers unconditionally
@ 2017-10-26 14:29 Jani Nikula
2017-10-26 14:55 ` Ville Syrjälä
2017-10-26 19:40 ` [Intel-gfx] " Manasi Navare
0 siblings, 2 replies; 4+ messages in thread
From: Jani Nikula @ 2017-10-26 14:29 UTC (permalink / raw)
To: intel-gfx; +Cc: jani.nikula, Ville Syrjälä, stable
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).
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>
---
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
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH 1/2] drm/i915/edp: read edp display control registers unconditionally
2017-10-26 14:29 [PATCH 1/2] drm/i915/edp: read edp display control registers unconditionally Jani Nikula
@ 2017-10-26 14:55 ` Ville Syrjälä
2017-10-26 19:40 ` [Intel-gfx] " Manasi Navare
1 sibling, 0 replies; 4+ messages in thread
From: Ville Syrjälä @ 2017-10-26 14:55 UTC (permalink / raw)
To: Jani Nikula; +Cc: intel-gfx, stable
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).
>
> 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>
Series is
Reviewed-by: Ville Syrj�l� <ville.syrjala@linux.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
--
Ville Syrj�l�
Intel OTC
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Intel-gfx] [PATCH 1/2] drm/i915/edp: read edp display control registers unconditionally
2017-10-26 14:29 [PATCH 1/2] drm/i915/edp: read edp display control registers unconditionally Jani Nikula
2017-10-26 14:55 ` Ville Syrjälä
@ 2017-10-26 19:40 ` Manasi Navare
2017-10-30 9:55 ` Jani Nikula
1 sibling, 1 reply; 4+ messages in thread
From: Manasi Navare @ 2017-10-26 19:40 UTC (permalink / raw)
To: Jani Nikula; +Cc: intel-gfx, stable
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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Intel-gfx] [PATCH 1/2] drm/i915/edp: read edp display control registers unconditionally
2017-10-26 19:40 ` [Intel-gfx] " Manasi Navare
@ 2017-10-30 9:55 ` Jani Nikula
0 siblings, 0 replies; 4+ messages in thread
From: Jani Nikula @ 2017-10-30 9:55 UTC (permalink / raw)
To: Manasi Navare; +Cc: intel-gfx, stable
On Thu, 26 Oct 2017, Manasi Navare <manasi.d.navare@intel.com> wrote:
> This is the second wierd eDP panel case/bug that I have seen recently
> where it doesnt not behave as per the spec.
It'll teach you to treat the displays as external devices that try to
fuzz our driver... and there's a fine balance between being defensive
and yet trying to get a picture on screen no matter what.
BR,
Jani.
--
Jani Nikula, Intel Open Source Technology Center
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2017-10-30 9:54 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-10-26 14:29 [PATCH 1/2] drm/i915/edp: read edp display control registers unconditionally Jani Nikula
2017-10-26 14:55 ` Ville Syrjälä
2017-10-26 19:40 ` [Intel-gfx] " Manasi Navare
2017-10-30 9:55 ` Jani Nikula
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).