Re: [PATCH v2 03/11] drm/edid: slightly restructure timing and non-timing descriptor structs

On Mon, 28 Mar 2022, Jani Nikula jani.nikula@intel.com wrote:

The pixel clock is conceptually part of the detailed timings, while it's just zero padding for display descriptors. Modify the structures to reflect this. Rename struct detailed_non_pixel to edid_display_descriptor to better reflect spec while at it. (Further struct renames are left for follow-up work.)

Suggested-by: Ville Syrjälä ville.syrjala@linux.intel.com Signed-off-by: Jani Nikula jani.nikula@intel.com

This one's missing:

Cc: Harry Wentland harry.wentland@amd.com Cc: Leo Li sunpeng.li@amd.com Cc: Rodrigo Siqueira Rodrigo.Siqueira@amd.com Cc: amd-gfx@lists.freedesktop.org

---

drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 6 +++--- drivers/gpu/drm/drm_edid.c | 12 ++++++------ include/drm/drm_edid.h | 9 +++++---- 3 files changed, 14 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c index b30656959fd8..e477f4b42b6b 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -11537,7 +11537,7 @@ void amdgpu_dm_update_freesync_caps(struct drm_connector *connector, { int i = 0; struct detailed_timing *timing;

• struct detailed_non_pixel *data;

• struct edid_display_descriptor *data; struct detailed_data_monitor_range *range; struct amdgpu_dm_connector *amdgpu_dm_connector = to_amdgpu_dm_connector(connector);

@@ -11592,7 +11592,7 @@ void amdgpu_dm_update_freesync_caps(struct drm_connector *connector, for (i = 0; i < 4; i++) {

		timing	= &edid->detailed_timings[i];

• 		data	= &timing->data.other_data;
  

• 		data	= &timing->data.descriptor;
  	range	= &data->data.range;
  	/*
  	 * Check if monitor has continuous frequency mode
  

@@ -11629,7 +11629,7 @@ void amdgpu_dm_update_freesync_caps(struct drm_connector *connector, i = parse_hdmi_amd_vsdb(amdgpu_dm_connector, edid, &vsdb_info); if (i >= 0 && vsdb_info.freesync_supported) { timing = &edid->detailed_timings[i];

• 	data    = &timing->data.other_data;
  

• 	data    = &timing->data.descriptor;
  
  amdgpu_dm_connector->min_vfreq = vsdb_info.min_refresh_rate_hz;
  amdgpu_dm_connector->max_vfreq = vsdb_info.max_refresh_rate_hz;
  

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 13d05062d68c..ac80681d64f6 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -2742,7 +2742,7 @@ static struct drm_display_mode *drm_mode_detailed(struct drm_device *dev, if (quirks & EDID_QUIRK_135_CLOCK_TOO_HIGH) mode->clock = 1088 * 10; else

• mode->clock = le16_to_cpu(timing->pixel_clock) * 10;
  

• mode->clock = le16_to_cpu(pt->pixel_clock) * 10;
  
  

  mode->hdisplay = hactive; mode->hsync_start = mode->hdisplay + hsync_offset;

@@ -2984,7 +2984,7 @@ static void do_inferred_modes(struct detailed_timing *timing, void *c) { struct detailed_mode_closure *closure = c;

• struct detailed_non_pixel *data = &timing->data.other_data;

• struct edid_display_descriptor *data = &timing->data.descriptor; struct detailed_data_monitor_range *range = &data->data.range;

  if (!is_display_descriptor((const u8 *)timing, EDID_DETAIL_MONITOR_RANGE))

@@ -3117,7 +3117,7 @@ static void do_standard_modes(struct detailed_timing *timing, void *c) { struct detailed_mode_closure *closure = c;

• struct detailed_non_pixel *data = &timing->data.other_data;

• struct edid_display_descriptor *data = &timing->data.descriptor; struct drm_connector *connector = closure->connector; struct edid *edid = closure->edid; int i;

@@ -3187,7 +3187,7 @@ static int drm_cvt_modes(struct drm_connector *connector, for (i = 0; i < 4; i++) { int width, height;

• cvt = &(timing->data.other_data.data.cvt[i]);
  

• cvt = &(timing->data.descriptor.data.cvt[i]);
  
  

  if (!memcmp(cvt->code, empty, 3)) continue;

@@ -4494,7 +4494,7 @@ monitor_name(struct detailed_timing *t, void *data) if (!is_display_descriptor((const u8 *)t, EDID_DETAIL_MONITOR_NAME)) return;

• *(u8 **)data = t->data.other_data.data.str.str;

• *(u8 **)data = t->data.descriptor.data.str.str;

}

static int get_monitor_name(struct edid *edid, char name[13]) @@ -5223,7 +5223,7 @@ void get_monitor_range(struct detailed_timing *timing, void *info_monitor_range) { struct drm_monitor_range_info *monitor_range = info_monitor_range;

• const struct detailed_non_pixel *data = &timing->data.other_data;

• const struct edid_display_descriptor *data = &timing->data.descriptor; const struct detailed_data_monitor_range *range = &data->data.range;

  if (!is_display_descriptor((const u8 *)timing, EDID_DETAIL_MONITOR_RANGE))

diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h index 144c495b99c4..8e322ef173a8 100644 --- a/include/drm/drm_edid.h +++ b/include/drm/drm_edid.h @@ -68,6 +68,7 @@ struct std_timing {

/* If detailed data is pixel timing */ struct detailed_pixel_timing {

• __le16 pixel_clock; /* non-zero, need to multiply by 10 KHz */ u8 hactive_lo; u8 hblank_lo; u8 hactive_hblank_hi;

@@ -142,8 +143,9 @@ struct cvt_timing { u8 code[3]; } __attribute__((packed));

-struct detailed_non_pixel {

• u8 pad1;

+struct edid_display_descriptor {

• u16 pad0; /* 0 for Display Descriptor */
• u8 pad1; /* 0 for Display Descriptor */ u8 type; /* ff=serial, fe=string, fd=monitor range, fc=monitor name fb=color point data, fa=standard timing data, f9=undefined, f8=mfg. reserved */

@@ -168,10 +170,9 @@ struct detailed_non_pixel { #define EDID_DETAIL_MONITOR_SERIAL 0xff

struct detailed_timing {

• __le16 pixel_clock; /* need to multiply by 10 KHz */ union { struct detailed_pixel_timing pixel_data;

• struct detailed_non_pixel other_data;
  

• struct edid_display_descriptor descriptor;
  

  } data;

} __attribute__((packed));