RE: [PATCH] drm/edid: ignore the CEA modes not defined in CEA-861-D

["Tseng, William" <william.tseng@intel.com>]

Thanks, Ville Syrjälä.

It seems the change regarding drm_mode_cea_vic() works for the issue.

If so, three video formats with the same resolution, say 2560x1080, may be probed; two from SAD and one from DTD.
Then, depending on which is selected, any of the three video formats may be set to the HDMI 1.4 sink.
I am wondering if this works for all the other HDMI 1.4 sinks.
What do you think of it?


Regards
William

-----Original Message-----
From: Ville Syrjälä <ville.syrjala@linux.intel.com> 
Sent: Wednesday, June 1, 2022 12:42 AM
To: Tseng, William <william.tseng@intel.com>
Cc: dri-devel@lists.freedesktop.org; Wayne Lin <waynelin@amd.com>; Lee, Shawn C <shawn.c.lee@intel.com>
Subject: Re: [PATCH] drm/edid: ignore the CEA modes not defined in CEA-861-D

On Tue, May 31, 2022 at 06:34:21PM +0800, William Tseng wrote:
> This is a workaround for HDMI 1.4 sink which has a CEA mode with 
> higher vic than what is defined in CEA-861-D.
> 
> As an example, a HDMI 1.4 sink has the video format 2560x1080p to be 
> displayed and the video format is indicated by both SVD (with vic 90 
> and pictuure aspect ratio 64:27) and DTD.  When connecting to such 
> sink, source can't output the video format in SVD because an error is 
> returned by drm_hdmi_avi_infoframe_from_display_mode(), which can't 
> fill the infoframe with pictuure aspect ratio 64:27 and the vic, which 
> is originally 90 and is changed to 0 by drm_mode_cea_vic().

Hmm. I think either we need to change the logic in
drm_hdmi_avi_infoframe_from_display_mode() somehow to accept this or we need to strip the aspect ratio from such modes when we probe them.

The first option might be nicer, and something like this might even work:

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 929fc0e46751..3d5c76acf42a 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -6082,7 +6082,8 @@ static u8 drm_mode_hdmi_vic(const struct drm_connector *connector,  }
 
 static u8 drm_mode_cea_vic(const struct drm_connector *connector,
-			   const struct drm_display_mode *mode)
+			   const struct drm_display_mode *mode,
+			   bool is_hdmi2_sink)
 {
 	u8 vic;
 
@@ -6102,7 +6103,7 @@ static u8 drm_mode_cea_vic(const struct drm_connector *connector,
 	 * HDMI 2.0 VIC range: 1 <= VIC <= 107 (CEA-861-F). So we
 	 * have to make sure we dont break HDMI 1.4 sinks.
 	 */
-	if (!is_hdmi2_sink(connector) && vic > 64)
+	if (!is_hdmi2_sink && vic > 64)
 		return 0;
 
 	return vic;
@@ -6133,7 +6134,7 @@ drm_hdmi_avi_infoframe_from_display_mode(struct hdmi_avi_infoframe *frame,
 	if (mode->flags & DRM_MODE_FLAG_DBLCLK)
 		frame->pixel_repeat = 1;
 
-	vic = drm_mode_cea_vic(connector, mode);
+	vic = drm_mode_cea_vic(connector, mode, true);
 	hdmi_vic = drm_mode_hdmi_vic(connector, mode);
 
 	frame->picture_aspect = HDMI_PICTURE_ASPECT_NONE; @@ -6177,7 +6178,7 @@ drm_hdmi_avi_infoframe_from_display_mode(struct hdmi_avi_infoframe *frame,
 		picture_aspect = HDMI_PICTURE_ASPECT_NONE;
 	}
 
-	frame->video_code = vic;
+	frame->video_code = drm_mode_cea_vic(connector, mode, 
+is_hdmi2_sink(connector));
 	frame->picture_aspect = picture_aspect;
 	frame->active_aspect = HDMI_ACTIVE_ASPECT_PICTURE;
 	frame->scan_mode = HDMI_SCAN_MODE_UNDERSCAN;

But I don't quite remember off hand how the CEA VIC vs. HDMI VIC decision plays into this, so there might be some issues with that approach.

--
Ville Syrjälä
Intel