Discussion:
[Intel-gfx] [PATCH] drm/edid: Reset more of the display info
Ville Syrjala
2018-04-24 13:02:50 UTC
Permalink
From: Ville Syrjälä <***@linux.intel.com>

We're currently failing to reset everything in display_info.hdmi
which will potentially cause us to use stale information when
swapping monitors. Eg. if the user replaces a HDMI 2.0 monitor
with a HDMI 1.x monitor we will continue to think that the monitor
supports scrambling. That will lead to a black screen since the
HDMI 1.x monitor won't understand the scrambled signal.

Fix the problem by clearing display_info.hdmi fully. And while at
eliminate some duplicated code by calling drm_reset_display_info()
in drm_add_display_info().

Cc: Antony Chen <***@qnap.com>
Cc: Shashank Sharma <***@intel.com>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105655
Signed-off-by: Ville Syrjälä <***@linux.intel.com>
---
drivers/gpu/drm/drm_edid.c | 11 +++--------
1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 134069f36482..39f1db4acda4 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -4451,6 +4451,7 @@ drm_reset_display_info(struct drm_connector *connector)
info->max_tmds_clock = 0;
info->dvi_dual = false;
info->has_hdmi_infoframe = false;
+ memset(&info->hdmi, 0, sizeof(info->hdmi));

info->non_desktop = 0;
}
@@ -4462,17 +4463,11 @@ u32 drm_add_display_info(struct drm_connector *connector, const struct edid *edi

u32 quirks = edid_get_quirks(edid);

+ drm_reset_display_info(connector);
+
info->width_mm = edid->width_cm * 10;
info->height_mm = edid->height_cm * 10;

- /* driver figures it out in this case */
- info->bpc = 0;
- info->color_formats = 0;
- info->cea_rev = 0;
- info->max_tmds_clock = 0;
- info->dvi_dual = false;
- info->has_hdmi_infoframe = false;
-
info->non_desktop = !!(quirks & EDID_QUIRK_NON_DESKTOP);

DRM_DEBUG_KMS("non_desktop set to %d\n", info->non_desktop);
--
2.16.1
Daniel Vetter
2018-04-24 14:18:37 UTC
Permalink
Post by Ville Syrjala
We're currently failing to reset everything in display_info.hdmi
which will potentially cause us to use stale information when
swapping monitors. Eg. if the user replaces a HDMI 2.0 monitor
with a HDMI 1.x monitor we will continue to think that the monitor
supports scrambling. That will lead to a black screen since the
HDMI 1.x monitor won't understand the scrambled signal.
Fix the problem by clearing display_info.hdmi fully. And while at
eliminate some duplicated code by calling drm_reset_display_info()
in drm_add_display_info().
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105655
---
drivers/gpu/drm/drm_edid.c | 11 +++--------
1 file changed, 3 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 134069f36482..39f1db4acda4 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -4451,6 +4451,7 @@ drm_reset_display_info(struct drm_connector *connector)
info->max_tmds_clock = 0;
info->dvi_dual = false;
info->has_hdmi_infoframe = false;
+ memset(&info->hdmi, 0, sizeof(info->hdmi));
info->non_desktop = 0;
}
@@ -4462,17 +4463,11 @@ u32 drm_add_display_info(struct drm_connector *connector, const struct edid *edi
u32 quirks = edid_get_quirks(edid);
+ drm_reset_display_info(connector);
+
Strictly speaking this is a separate bugfix, for the case where you
immediately go from one output to a different one. Keith already fixed the
case where at least somewhere in between you go to the disconnected state
in:

commit 170178fe99dd212bf25e70c89bc4b6e195564ffc
Author: Keith Packard <***@keithp.com>
Date: Wed Dec 13 00:44:26 2017 -0800

drm: Update edid-derived drm_display_info fields at edid property set [v2]
Post by Ville Syrjala
info->width_mm = edid->width_cm * 10;
info->height_mm = edid->height_cm * 10;
- /* driver figures it out in this case */
- info->bpc = 0;
- info->color_formats = 0;
- info->cea_rev = 0;
- info->max_tmds_clock = 0;
- info->dvi_dual = false;
- info->has_hdmi_infoframe = false;
-
info->non_desktop = !!(quirks & EDID_QUIRK_NON_DESKTOP);
DRM_DEBUG_KMS("non_desktop set to %d\n", info->non_desktop);
--
2.16.1
_______________________________________________
Intel-gfx mailing list
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
Ville Syrjälä
2018-04-24 14:26:30 UTC
Permalink
Post by Daniel Vetter
Post by Ville Syrjala
We're currently failing to reset everything in display_info.hdmi
which will potentially cause us to use stale information when
swapping monitors. Eg. if the user replaces a HDMI 2.0 monitor
with a HDMI 1.x monitor we will continue to think that the monitor
supports scrambling. That will lead to a black screen since the
HDMI 1.x monitor won't understand the scrambled signal.
Fix the problem by clearing display_info.hdmi fully. And while at
eliminate some duplicated code by calling drm_reset_display_info()
in drm_add_display_info().
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105655
---
drivers/gpu/drm/drm_edid.c | 11 +++--------
1 file changed, 3 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 134069f36482..39f1db4acda4 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -4451,6 +4451,7 @@ drm_reset_display_info(struct drm_connector *connector)
info->max_tmds_clock = 0;
info->dvi_dual = false;
info->has_hdmi_infoframe = false;
+ memset(&info->hdmi, 0, sizeof(info->hdmi));
info->non_desktop = 0;
}
@@ -4462,17 +4463,11 @@ u32 drm_add_display_info(struct drm_connector *connector, const struct edid *edi
u32 quirks = edid_get_quirks(edid);
+ drm_reset_display_info(connector);
+
Strictly speaking this is a separate bugfix, for the case where you
immediately go from one output to a different one. Keith already fixed the
case where at least somewhere in between you go to the disconnected state
commit 170178fe99dd212bf25e70c89bc4b6e195564ffc
Date: Wed Dec 13 00:44:26 2017 -0800
drm: Update edid-derived drm_display_info fields at edid property set [v2]
The drm_reset_display_info() call is just the "deduplicate the code"
part. The memset() is the bugfix, and applies in all cases.
Post by Daniel Vetter
Post by Ville Syrjala
info->width_mm = edid->width_cm * 10;
info->height_mm = edid->height_cm * 10;
- /* driver figures it out in this case */
- info->bpc = 0;
- info->color_formats = 0;
- info->cea_rev = 0;
- info->max_tmds_clock = 0;
- info->dvi_dual = false;
- info->has_hdmi_infoframe = false;
-
info->non_desktop = !!(quirks & EDID_QUIRK_NON_DESKTOP);
DRM_DEBUG_KMS("non_desktop set to %d\n", info->non_desktop);
--
2.16.1
_______________________________________________
Intel-gfx mailing list
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
--
Ville Syrjälä
Intel
Daniel Vetter
2018-04-24 19:36:29 UTC
Permalink
Post by Ville Syrjälä
Post by Daniel Vetter
Post by Ville Syrjala
We're currently failing to reset everything in display_info.hdmi
which will potentially cause us to use stale information when
swapping monitors. Eg. if the user replaces a HDMI 2.0 monitor
with a HDMI 1.x monitor we will continue to think that the monitor
supports scrambling. That will lead to a black screen since the
HDMI 1.x monitor won't understand the scrambled signal.
Fix the problem by clearing display_info.hdmi fully. And while at
eliminate some duplicated code by calling drm_reset_display_info()
in drm_add_display_info().
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105655
---
drivers/gpu/drm/drm_edid.c | 11 +++--------
1 file changed, 3 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 134069f36482..39f1db4acda4 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -4451,6 +4451,7 @@ drm_reset_display_info(struct drm_connector *connector)
info->max_tmds_clock = 0;
info->dvi_dual = false;
info->has_hdmi_infoframe = false;
+ memset(&info->hdmi, 0, sizeof(info->hdmi));
info->non_desktop = 0;
}
@@ -4462,17 +4463,11 @@ u32 drm_add_display_info(struct drm_connector *connector, const struct edid *edi
u32 quirks = edid_get_quirks(edid);
+ drm_reset_display_info(connector);
+
Strictly speaking this is a separate bugfix, for the case where you
immediately go from one output to a different one. Keith already fixed the
case where at least somewhere in between you go to the disconnected state
commit 170178fe99dd212bf25e70c89bc4b6e195564ffc
Date: Wed Dec 13 00:44:26 2017 -0800
drm: Update edid-derived drm_display_info fields at edid property set [v2]
The drm_reset_display_info() call is just the "deduplicate the code"
part. The memset() is the bugfix, and applies in all cases.
Hm ... I guess my brain wasn't fully working.

Reviewed-by: Daniel Vetter <***@ffwll.ch>

as-is.
-Daniel
Post by Ville Syrjälä
Post by Daniel Vetter
Post by Ville Syrjala
info->width_mm = edid->width_cm * 10;
info->height_mm = edid->height_cm * 10;
- /* driver figures it out in this case */
- info->bpc = 0;
- info->color_formats = 0;
- info->cea_rev = 0;
- info->max_tmds_clock = 0;
- info->dvi_dual = false;
- info->has_hdmi_infoframe = false;
-
info->non_desktop = !!(quirks & EDID_QUIRK_NON_DESKTOP);
DRM_DEBUG_KMS("non_desktop set to %d\n", info->non_desktop);
--
2.16.1
_______________________________________________
Intel-gfx mailing list
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
--
Ville Syrjälä
Intel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
Ville Syrjälä
2018-04-25 14:09:10 UTC
Permalink
Post by Daniel Vetter
Post by Ville Syrjälä
Post by Daniel Vetter
Post by Ville Syrjala
We're currently failing to reset everything in display_info.hdmi
which will potentially cause us to use stale information when
swapping monitors. Eg. if the user replaces a HDMI 2.0 monitor
with a HDMI 1.x monitor we will continue to think that the monitor
supports scrambling. That will lead to a black screen since the
HDMI 1.x monitor won't understand the scrambled signal.
Fix the problem by clearing display_info.hdmi fully. And while at
eliminate some duplicated code by calling drm_reset_display_info()
in drm_add_display_info().
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105655
---
drivers/gpu/drm/drm_edid.c | 11 +++--------
1 file changed, 3 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 134069f36482..39f1db4acda4 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -4451,6 +4451,7 @@ drm_reset_display_info(struct drm_connector *connector)
info->max_tmds_clock = 0;
info->dvi_dual = false;
info->has_hdmi_infoframe = false;
+ memset(&info->hdmi, 0, sizeof(info->hdmi));
info->non_desktop = 0;
}
@@ -4462,17 +4463,11 @@ u32 drm_add_display_info(struct drm_connector *connector, const struct edid *edi
u32 quirks = edid_get_quirks(edid);
+ drm_reset_display_info(connector);
+
Strictly speaking this is a separate bugfix, for the case where you
immediately go from one output to a different one. Keith already fixed the
case where at least somewhere in between you go to the disconnected state
commit 170178fe99dd212bf25e70c89bc4b6e195564ffc
Date: Wed Dec 13 00:44:26 2017 -0800
drm: Update edid-derived drm_display_info fields at edid property set [v2]
The drm_reset_display_info() call is just the "deduplicate the code"
part. The memset() is the bugfix, and applies in all cases.
Hm ... I guess my brain wasn't fully working.
as-is.
Cool. Thanks.

Pushed to drm-misc-fixes with
Post by Daniel Vetter
-Daniel
Post by Ville Syrjälä
Post by Daniel Vetter
Post by Ville Syrjala
info->width_mm = edid->width_cm * 10;
info->height_mm = edid->height_cm * 10;
- /* driver figures it out in this case */
- info->bpc = 0;
- info->color_formats = 0;
- info->cea_rev = 0;
- info->max_tmds_clock = 0;
- info->dvi_dual = false;
- info->has_hdmi_infoframe = false;
-
info->non_desktop = !!(quirks & EDID_QUIRK_NON_DESKTOP);
DRM_DEBUG_KMS("non_desktop set to %d\n", info->non_desktop);
--
2.16.1
_______________________________________________
Intel-gfx mailing list
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
--
Ville Syrjälä
Intel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
--
Ville Syrjälä
Intel
Ville Syrjälä
2018-04-25 14:10:32 UTC
Permalink
Hi all,
The patch works: drivers/gpu/drm/drm_edid.c
Thanks for your help.
What are the steps to close the issue in freedesktop? Append the patch by Ville
Syrjälä, then I close it?
The bug is now resolved. Thanks for reporting the problem and testing
the fix.
Antony
Post by Ville Syrjala
Post by Ville Syrjälä
Post by Daniel Vetter
Post by Ville Syrjala
We're currently failing to reset everything in display_info.hdmi
which will potentially cause us to use stale information when
swapping monitors. Eg. if the user replaces a HDMI 2.0 monitor
with a HDMI 1.x monitor we will continue to think that the monitor
supports scrambling. That will lead to a black screen since the
HDMI 1.x monitor won't understand the scrambled signal.
Fix the problem by clearing display_info.hdmi fully. And while at
eliminate some duplicated code by calling drm_reset_display_info()
in drm_add_display_info().
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105655
---
drivers/gpu/drm/drm_edid.c | 11 +++--------
1 file changed, 3 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 134069f36482..39f1db4acda4 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -4451,6 +4451,7 @@ drm_reset_display_info(struct drm_connector
*connector)
Post by Ville Syrjälä
Post by Daniel Vetter
Post by Ville Syrjala
info->max_tmds_clock = 0;
info->dvi_dual = false;
info->has_hdmi_infoframe = false;
+ memset(&info->hdmi, 0, sizeof(info->hdmi));
info->non_desktop = 0;
}
@@ -4462,17 +4463,11 @@ u32 drm_add_display_info(struct
drm_connector *connector, const struct edid *edi
Post by Ville Syrjälä
Post by Daniel Vetter
Post by Ville Syrjala
u32 quirks = edid_get_quirks(edid);
+ drm_reset_display_info(connector);
+
Strictly speaking this is a separate bugfix, for the case where you
immediately go from one output to a different one. Keith already fixed
the
Post by Ville Syrjälä
Post by Daniel Vetter
case where at least somewhere in between you go to the disconnected
state
Post by Ville Syrjälä
Post by Daniel Vetter
commit 170178fe99dd212bf25e70c89bc4b6e195564ffc
Date: Wed Dec 13 00:44:26 2017 -0800
drm: Update edid-derived drm_display_info fields at edid property
set [v2]
Post by Ville Syrjälä
The drm_reset_display_info() call is just the "deduplicate the code"
part. The memset() is the bugfix, and applies in all cases.
Hm ... I guess my brain wasn't fully working.
as-is.
-Daniel
Post by Ville Syrjälä
Post by Daniel Vetter
Post by Ville Syrjala
info->width_mm = edid->width_cm * 10;
info->height_mm = edid->height_cm * 10;
- /* driver figures it out in this case */
- info->bpc = 0;
- info->color_formats = 0;
- info->cea_rev = 0;
- info->max_tmds_clock = 0;
- info->dvi_dual = false;
- info->has_hdmi_infoframe = false;
-
info->non_desktop = !!(quirks & EDID_QUIRK_NON_DESKTOP);
DRM_DEBUG_KMS("non_desktop set to %d\n", info->non_desktop);
--
2.16.1
_______________________________________________
Intel-gfx mailing list
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
--
Ville Syrjälä
Intel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
--
Ville Syrjälä
Intel
Patchwork
2018-04-24 14:22:06 UTC
Permalink
== Series Details ==

Series: drm/edid: Reset more of the display info
URL : https://patchwork.freedesktop.org/series/42191/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4086 -> Patchwork_8788 =

== Summary - SUCCESS ==

No regressions found.

External URL: https://patchwork.freedesktop.org/api/1.0/series/42191/revisions/1/mbox/

== Possible new issues ==

Here are the unknown changes that may have been introduced in Patchwork_8788:

=== IGT changes ===

==== Possible regressions ====

***@kms_psr_sink_crc@psr_basic:
{fi-hsw-4200u}: NOTRUN -> FAIL


== Known issues ==

Here are the changes found in Patchwork_8788 that come from known issues:

=== IGT changes ===

==== Issues hit ====

***@kms_pipe_crc_basic@suspend-read-crc-pipe-c:
fi-ivb-3520m: PASS -> DMESG-WARN (fdo#106084)


{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).

fdo#106084 https://bugs.freedesktop.org/show_bug.cgi?id=106084


== Participating hosts (36 -> 34) ==

Additional (1): fi-hsw-4200u
Missing (3): fi-ctg-p8600 fi-ilk-m540 fi-skl-6700hq


== Build changes ==

* Linux: CI_DRM_4086 -> Patchwork_8788

CI_DRM_4086: 10d9666c9a49d47bcb5be8fa4116ef6508155aff @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_4446: e5e8dafc991ee922ec159491c680caff0cfe9235 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_8788: 7b7203fdea70afbb0a94169624dca292c4623d28 @ git://anongit.freedesktop.org/gfx-ci/linux
piglit_4446: a2f486679f467cd6e82578384f56d4aabaa8cf2e @ git://anongit.freedesktop.org/piglit


== Linux commits ==

7b7203fdea70 drm/edid: Reset more of the display info

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8788/issues.html
Patchwork
2018-04-24 17:46:38 UTC
Permalink
== Series Details ==

Series: drm/edid: Reset more of the display info
URL : https://patchwork.freedesktop.org/series/42191/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4086_full -> Patchwork_8788_full =

== Summary - WARNING ==

Minor unknown changes coming with Patchwork_8788_full need to be verified
manually.

If you think the reported changes have nothing to do with the changes
introduced in Patchwork_8788_full, please notify your bug team to allow them
to document this new failure mode, which will reduce false positives in CI.

External URL: https://patchwork.freedesktop.org/api/1.0/series/42191/revisions/1/mbox/

== Possible new issues ==

Here are the unknown changes that may have been introduced in Patchwork_8788_full:

=== IGT changes ===

==== Warnings ====

***@kms_plane@plane-panning-bottom-right-pipe-a-planes:
shard-glk: SKIP -> PASS +122


== Known issues ==

Here are the changes found in Patchwork_8788_full that come from known issues:

=== IGT changes ===

==== Issues hit ====

***@kms_flip@flip-vs-expired-vblank-interruptible:
shard-glk: PASS -> FAIL (fdo#102887, fdo#105363)

***@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-render:
shard-apl: PASS -> FAIL (fdo#103167)

***@kms_rotation_crc@primary-rotation-180:
shard-hsw: PASS -> FAIL (fdo#103925)


==== Possible fixes ====

***@kms_flip@flip-vs-expired-vblank:
shard-glk: FAIL (fdo#102887, fdo#105363) -> PASS

***@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-onoff:
shard-apl: FAIL (fdo#103167) -> PASS


fdo#102887 https://bugs.freedesktop.org/show_bug.cgi?id=102887
fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
fdo#103925 https://bugs.freedesktop.org/show_bug.cgi?id=103925
fdo#105363 https://bugs.freedesktop.org/show_bug.cgi?id=105363


== Participating hosts (6 -> 4) ==

Missing (2): shard-glkb shard-kbl


== Build changes ==

* Linux: CI_DRM_4086 -> Patchwork_8788

CI_DRM_4086: 10d9666c9a49d47bcb5be8fa4116ef6508155aff @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_4446: e5e8dafc991ee922ec159491c680caff0cfe9235 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_8788: 7b7203fdea70afbb0a94169624dca292c4623d28 @ git://anongit.freedesktop.org/gfx-ci/linux
piglit_4446: a2f486679f467cd6e82578384f56d4aabaa8cf2e @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8788/shards.html
Loading...