[PATCH] drm/edid: Clear EDID Deep Color Modes in drm_reset_display_info()

[PATCH] drm/edid: Clear EDID Deep Color Modes in drm_reset_display_info()

[Maxime Ripard]

Even though we have the other drm_display_info fields reset, the DC
modes are missing.

This shouldn't be an issue since it's explicitely reset every time a new
EDID is parsed.

Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
---
 drivers/gpu/drm/drm_edid.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 5251925e3b92..a76224653e6f 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -5340,6 +5340,9 @@ drm_reset_display_info(struct drm_connector *connector)
 	info->rgb_quant_range_selectable = false;
 	memset(&info->hdmi, 0, sizeof(info->hdmi));
 
+	info->edid_hdmi_rgb444_dc_modes = 0;
+	info->edid_hdmi_ycbcr444_dc_modes = 0;
+
 	info->non_desktop = 0;
 	memset(&info->monitor_range, 0, sizeof(info->monitor_range));
 
-- 
2.34.1

Re: [PATCH] drm/edid: Clear EDID Deep Color Modes in drm_reset_display_info()

[Ville Syrjälä]

On Tue, Jan 25, 2022 at 10:32:51AM +0100, Maxime Ripard wrote:
> Even though we have the other drm_display_info fields reset, the DC
> modes are missing.
> 
> This shouldn't be an issue since it's explicitely reset every time a new
> EDID is parsed.

It's not really obvious that it can't be an issue. I think we might
be saved by info->bpc getting reset to 8 by drm_parse_hdmi_vsdb_video().
But the dc_modes bitfields themselves may be left with stale values.

Hmm, actually drm_add_display_info() re-populates info->bpc *after* 
drm_parse_hdmi_vsdb_video() so we could in theory end up with bpc>8 
and dc_modes!=0 even if the display doesn't support any HDMI deep 
color modes. Though I suppose a display that doesn't do deep color
but declares its max bpc as >8 is a bit nuts. I could maybe imagine
it happening if the display supports >8 bpc with DP but not with 
HDMI, and doesn't bother customizing the EDID sufficiently for
each interface.

> 
> Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Maxime Ripard <maxime@cerno.tech>

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> ---
>  drivers/gpu/drm/drm_edid.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 5251925e3b92..a76224653e6f 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -5340,6 +5340,9 @@ drm_reset_display_info(struct drm_connector *connector)
>  	info->rgb_quant_range_selectable = false;
>  	memset(&info->hdmi, 0, sizeof(info->hdmi));

We should probably just move more stuff into that .hdmi struct...

>  
> +	info->edid_hdmi_rgb444_dc_modes = 0;
> +	info->edid_hdmi_ycbcr444_dc_modes = 0;
> +
>  	info->non_desktop = 0;
>  	memset(&info->monitor_range, 0, sizeof(info->monitor_range));
>  
> -- 
> 2.34.1

-- 
Ville Syrjälä
Intel

Re: [PATCH] drm/edid: Clear EDID Deep Color Modes in drm_reset_display_info()

[Jani Nikula]

On Tue, 25 Jan 2022, Maxime Ripard <maxime@cerno.tech> wrote:
> Even though we have the other drm_display_info fields reset, the DC
> modes are missing.
>
> This shouldn't be an issue since it's explicitely reset every time a new
> EDID is parsed.

Side note, and hijacking the thread a bit: I think it's a historical
mistake we've started accumulating stuff in struct drm_display_info that
is *not* derived from the EDID or DisplayID. I think
drm_reset_display_info() should just be one memset() and that's
it. Adding stuff that should not be reset here feels wrong.

Maybe we should just move all the non-EDID derived members directly to
struct drm_connector? Or some other sub-struct.

In the mean time, this patch seems like the right thing to do,

Reviewed-by: Jani Nikula <jani.nikula@intel.com>


BR,
Jani.

>
> Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Maxime Ripard <maxime@cerno.tech>
> ---
>  drivers/gpu/drm/drm_edid.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 5251925e3b92..a76224653e6f 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -5340,6 +5340,9 @@ drm_reset_display_info(struct drm_connector *connector)
>  	info->rgb_quant_range_selectable = false;
>  	memset(&info->hdmi, 0, sizeof(info->hdmi));
>  
> +	info->edid_hdmi_rgb444_dc_modes = 0;
> +	info->edid_hdmi_ycbcr444_dc_modes = 0;
> +
>  	info->non_desktop = 0;
>  	memset(&info->monitor_range, 0, sizeof(info->monitor_range));

-- 
Jani Nikula, Intel Open Source Graphics Center

Re: (subset) [PATCH] drm/edid: Clear EDID Deep Color Modes in drm_reset_display_info()

[Maxime Ripard]

On Tue, 25 Jan 2022 10:32:51 +0100, Maxime Ripard wrote:
> Even though we have the other drm_display_info fields reset, the DC
> modes are missing.
> 
> This shouldn't be an issue since it's explicitely reset every time a new
> EDID is parsed.
> 
> 
> [...]

Applied to drm/drm-misc (drm-misc-next).

Thanks!
Maxime

[PATCH] drm/edid: Clear EDID Deep Color Modes in drm_reset_display_info()

[Maxime Ripard]

Even though we have the other drm_display_info fields reset, the DC
modes are missing.

This shouldn't be an issue since it's explicitly reset every time a new
EDID is parsed.

Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20220125093251.594772-1-maxime@cerno.tech
---
 drivers/gpu/drm/drm_edid.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index a504542238ed..a7663f9a11d2 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -5340,6 +5340,9 @@ drm_reset_display_info(struct drm_connector *connector)
 	info->rgb_quant_range_selectable = false;
 	memset(&info->hdmi, 0, sizeof(info->hdmi));
 
+	info->edid_hdmi_rgb444_dc_modes = 0;
+	info->edid_hdmi_ycbcr444_dc_modes = 0;
+
 	info->non_desktop = 0;
 	memset(&info->monitor_range, 0, sizeof(info->monitor_range));
 
-- 
2.34.1

Re: [PATCH] drm/edid: Clear EDID Deep Color Modes in drm_reset_display_info()

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 692 bytes --]

On Tue, Jan 25, 2022 at 03:29:28PM +0100, Maxime Ripard wrote:
> Even though we have the other drm_display_info fields reset, the DC
> modes are missing.
> 
> This shouldn't be an issue since it's explicitly reset every time a new
> EDID is parsed.
> 
> Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Maxime Ripard <maxime@cerno.tech>
> Reviewed-by: Jani Nikula <jani.nikula@intel.com>
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Link: https://patchwork.freedesktop.org/patch/msgid/20220125093251.594772-1-maxime@cerno.tech

Sorry, this one can be ignored, I ended up sending the patch instead of
the b4 thanks email.

maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]