Re: [Intel-gfx] [PATCH 2/2] drm/i915: Restrict usage of live status check
On Fri, 11 Mar 2016, Shashank Sharma wrote:
> [ text/plain ]
> This patch restricts usage of live status check for HDMI detection.
> While testing certain (monitor + cable) combinations with various
> intel platforms, it seems that live status register is not reliable
> on some older devices. So limit the live_status check from VLV onwards.
>
> This fixes a regression introduced in:
> commit 237ed86c693d8a8e4db476976aeb30df4deac74b
> Author: Sonika Jindal
> Date: Tue Sep 15 09:44:20 2015 +0530
> drm/i915: Check live status before reading edid
This is in v4.4, which means the fix will need to be backported too, so
please send a patch fixing the bug *first*, instead of depending on an
optimization patch.
BR,
Jani.
>
> Signed-off-by: Shashank Sharma
> ---
> drivers/gpu/drm/i915/intel_hdmi.c | 18 +-
> 1 file changed, 13 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c
> b/drivers/gpu/drm/i915/intel_hdmi.c
> index b523a2f..ca1fb57 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -1397,6 +1397,7 @@ intel_hdmi_detect(struct drm_connector *connector, bool
> force)
> enum drm_connector_status status;
> struct intel_hdmi *intel_hdmi = intel_attached_hdmi(connector);
> struct drm_i915_private *dev_priv = to_i915(connector->dev);
> + struct drm_device *dev = connector->dev;
> bool live_status = true;
> unsigned int try = 8;
>
> @@ -1404,14 +1405,21 @@ intel_hdmi_detect(struct drm_connector *connector,
> bool force)
> connector->base.id, connector->name);
>
> intel_display_power_get(dev_priv, POWER_DOMAIN_GMBUS);
> - live_status = intel_digital_port_connected(dev_priv,
> - hdmi_to_dig_port(intel_hdmi));
> - while (!live_status && try--) {
> - msleep(10);
> +
> + /*
> + * The live status register doesn't work reliably with certain
> + * cables/monitors, on old platforms. So restrict the live status
> + * check to be applied from VLV onwards.
> + */
> + if (INTEL_INFO(dev)->gen >= 7 && !IS_IVYBRIDGE(dev)) {
> live_status = intel_digital_port_connected(dev_priv,
> hdmi_to_dig_port(intel_hdmi));
> + while (!live_status && try--) {
> + msleep(10);
> + live_status = intel_digital_port_connected(dev_priv,
> + hdmi_to_dig_port(intel_hdmi));
> + }
> }
> -
> intel_hdmi_unset_edid(connector);
>
> if (intel_hdmi_set_edid(connector, live_status)) {
--
Jani Nikula, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/2] drm/i915: Restrict usage of live status check
Right now live status check is helping in many scenarios, including:
- HDMI compliance
- HDMI connector status to be shown to user space
- While running HDCP compliance, we pass a train of hot-plugs/unplugs via
analyzers, and without live status
check and HDMI optimization patches, our software state machine is in a soup.
I don't see any other way of solving all the above problems, without this, and
fixing the regression too on the
old devices. So if you think we should remove this, we should also have some
alternative solution for above listed
problems. Our hotplug handling is not the best way it should be done (both in
HW and SW), and we will end
up something like this only.
We can't say that we are not going to bother about these problem, as we
shouldn't focus only on the
upstream use cases, but Android and Chrome cases also, which are commercial
targets, where meeting the
product quality gets even more difficult.
If you agree, we can discus more about how to make this work, else the only
thing I can come up with is we will revert
the HDMI optimization and live status check from upstream, and keep them local
on Android branches.
Regards
Shashank
-Original Message-
From: Ville Syrjälä [mailto:ville.syrj...@linux.intel.com]
Sent: Saturday, March 12, 2016 12:54 AM
To: Sharma, Shashank
Cc: intel-gfx@lists.freedesktop.org; Jindal, Sonika; Kumar, Shobhit; Wang, Gary
C; Vetter, Daniel
Subject: Re: [PATCH 2/2] drm/i915: Restrict usage of live status check
On Fri, Mar 11, 2016 at 11:51:00PM +0530, Shashank Sharma wrote:
> This patch restricts usage of live status check for HDMI detection.
> While testing certain (monitor + cable) combinations with various
> intel platforms, it seems that live status register is not reliable
> on some older devices. So limit the live_status check from VLV onwards.
>
> This fixes a regression introduced in:
> commit 237ed86c693d8a8e4db476976aeb30df4deac74b
> Author: Sonika Jindal
> Date: Tue Sep 15 09:44:20 2015 +0530
> drm/i915: Check live status before reading edid
>
> Signed-off-by: Shashank Sharma
> ---
> drivers/gpu/drm/i915/intel_hdmi.c | 18 +-
> 1 file changed, 13 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c
> b/drivers/gpu/drm/i915/intel_hdmi.c
> index b523a2f..ca1fb57 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -1397,6 +1397,7 @@ intel_hdmi_detect(struct drm_connector *connector, bool
> force)
> enum drm_connector_status status;
> struct intel_hdmi *intel_hdmi = intel_attached_hdmi(connector);
> struct drm_i915_private *dev_priv = to_i915(connector->dev);
> + struct drm_device *dev = connector->dev;
> bool live_status = true;
> unsigned int try = 8;
>
> @@ -1404,14 +1405,21 @@ intel_hdmi_detect(struct drm_connector *connector,
> bool force)
> connector->base.id, connector->name);
>
> intel_display_power_get(dev_priv, POWER_DOMAIN_GMBUS);
> - live_status = intel_digital_port_connected(dev_priv,
> - hdmi_to_dig_port(intel_hdmi));
> - while (!live_status && try--) {
> - msleep(10);
> +
> + /*
> + * The live status register doesn't work reliably with certain
> + * cables/monitors, on old platforms. So restrict the live status
> + * check to be applied from VLV onwards.
> + */
> + if (INTEL_INFO(dev)->gen >= 7 && !IS_IVYBRIDGE(dev)) {
> live_status = intel_digital_port_connected(dev_priv,
> hdmi_to_dig_port(intel_hdmi));
> + while (!live_status && try--) {
> + msleep(10);
> + live_status = intel_digital_port_connected(dev_priv,
> + hdmi_to_dig_port(intel_hdmi));
> + }
> }
> -
I would avoid all that churn and just do
if (whatever)
live_status = true;
after we're done with the loop.
That might at least hang on to some of the benefits of the live status check
for these platforms, whether or not the live status really works or not.
Not that I particularly like this random choice of platforms approach either.
> intel_hdmi_unset_edid(connector);
>
> if (intel_hdmi_set_edid(connector, live_status)) {
> --
> 1.9.1
--
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/2] drm/i915: Restrict usage of live status check
On Fri, Mar 11, 2016 at 11:51:00PM +0530, Shashank Sharma wrote:
> This patch restricts usage of live status check for HDMI detection.
> While testing certain (monitor + cable) combinations with various
> intel platforms, it seems that live status register is not reliable
> on some older devices. So limit the live_status check from VLV onwards.
>
> This fixes a regression introduced in:
> commit 237ed86c693d8a8e4db476976aeb30df4deac74b
> Author: Sonika Jindal
> Date: Tue Sep 15 09:44:20 2015 +0530
> drm/i915: Check live status before reading edid
>
> Signed-off-by: Shashank Sharma
> ---
> drivers/gpu/drm/i915/intel_hdmi.c | 18 +-
> 1 file changed, 13 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c
> b/drivers/gpu/drm/i915/intel_hdmi.c
> index b523a2f..ca1fb57 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -1397,6 +1397,7 @@ intel_hdmi_detect(struct drm_connector *connector, bool
> force)
> enum drm_connector_status status;
> struct intel_hdmi *intel_hdmi = intel_attached_hdmi(connector);
> struct drm_i915_private *dev_priv = to_i915(connector->dev);
> + struct drm_device *dev = connector->dev;
> bool live_status = true;
> unsigned int try = 8;
>
> @@ -1404,14 +1405,21 @@ intel_hdmi_detect(struct drm_connector *connector,
> bool force)
> connector->base.id, connector->name);
>
> intel_display_power_get(dev_priv, POWER_DOMAIN_GMBUS);
> - live_status = intel_digital_port_connected(dev_priv,
> - hdmi_to_dig_port(intel_hdmi));
> - while (!live_status && try--) {
> - msleep(10);
> +
> + /*
> + * The live status register doesn't work reliably with certain
> + * cables/monitors, on old platforms. So restrict the live status
> + * check to be applied from VLV onwards.
> + */
> + if (INTEL_INFO(dev)->gen >= 7 && !IS_IVYBRIDGE(dev)) {
> live_status = intel_digital_port_connected(dev_priv,
> hdmi_to_dig_port(intel_hdmi));
> + while (!live_status && try--) {
> + msleep(10);
> + live_status = intel_digital_port_connected(dev_priv,
> + hdmi_to_dig_port(intel_hdmi));
> + }
> }
> -
I would avoid all that churn and just do
if (whatever)
live_status = true;
after we're done with the loop.
That might at least hang on to some of the benefits of the live status
check for these platforms, whether or not the live status really works
or not.
Not that I particularly like this random choice of platforms approach
either.
> intel_hdmi_unset_edid(connector);
>
> if (intel_hdmi_set_edid(connector, live_status)) {
> --
> 1.9.1
--
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/2] drm/i915: Restrict usage of live status check
This patch restricts usage of live status check for HDMI detection.
While testing certain (monitor + cable) combinations with various
intel platforms, it seems that live status register is not reliable
on some older devices. So limit the live_status check from VLV onwards.
This fixes a regression introduced in:
commit 237ed86c693d8a8e4db476976aeb30df4deac74b
Author: Sonika Jindal
Date: Tue Sep 15 09:44:20 2015 +0530
drm/i915: Check live status before reading edid
Signed-off-by: Shashank Sharma
---
drivers/gpu/drm/i915/intel_hdmi.c | 18 +-
1 file changed, 13 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c
b/drivers/gpu/drm/i915/intel_hdmi.c
index b523a2f..ca1fb57 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -1397,6 +1397,7 @@ intel_hdmi_detect(struct drm_connector *connector, bool
force)
enum drm_connector_status status;
struct intel_hdmi *intel_hdmi = intel_attached_hdmi(connector);
struct drm_i915_private *dev_priv = to_i915(connector->dev);
+ struct drm_device *dev = connector->dev;
bool live_status = true;
unsigned int try = 8;
@@ -1404,14 +1405,21 @@ intel_hdmi_detect(struct drm_connector *connector, bool
force)
connector->base.id, connector->name);
intel_display_power_get(dev_priv, POWER_DOMAIN_GMBUS);
- live_status = intel_digital_port_connected(dev_priv,
- hdmi_to_dig_port(intel_hdmi));
- while (!live_status && try--) {
- msleep(10);
+
+ /*
+ * The live status register doesn't work reliably with certain
+ * cables/monitors, on old platforms. So restrict the live status
+ * check to be applied from VLV onwards.
+ */
+ if (INTEL_INFO(dev)->gen >= 7 && !IS_IVYBRIDGE(dev)) {
live_status = intel_digital_port_connected(dev_priv,
hdmi_to_dig_port(intel_hdmi));
+ while (!live_status && try--) {
+ msleep(10);
+ live_status = intel_digital_port_connected(dev_priv,
+ hdmi_to_dig_port(intel_hdmi));
+ }
}
-
intel_hdmi_unset_edid(connector);
if (intel_hdmi_set_edid(connector, live_status)) {
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
