Re: [PATCH 2/9] drm/edid: pass a timing pointer to is_display_descriptor()

On Fri, Mar 25, 2022 at 02:25:24PM +0200, Jani Nikula wrote:

Use struct member access instead of direct offsets to avoid lots of casts all over the place.

Use BUILD_BUG_ON() for sanity check.

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

---

drivers/gpu/drm/drm_edid.c | 26 +++++++++++++++----------- 1 file changed, 15 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 1b552fe54f38..48707eef1dc2 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -2331,10 +2331,14 @@ struct drm_display_mode *drm_mode_find_dmt(struct drm_device *dev, } EXPORT_SYMBOL(drm_mode_find_dmt);

-static bool is_display_descriptor(const u8 d[18], u8 tag) +static bool is_display_descriptor(const struct detailed_timing *timing, u8 type) {

• return d[0] == 0x00 && d[1] == 0x00 &&

• d[2] == 0x00 && d[3] == tag;
  

• BUILD_BUG_ON(offsetof(typeof(*timing), pixel_clock) != 0);
• BUILD_BUG_ON(offsetof(typeof(*timing), data.other_data.pad1) != 2);
• BUILD_BUG_ON(offsetof(typeof(*timing), data.other_data.type) != 3);
• return timing->pixel_clock == 0 && timing->data.other_data.pad1 == 0 &&

This would probably be less confusing if we moved pixel_clock into pixel_data and just had matching padding bytes/etc. in other_data.

The names of all structs are also rather weird. We should probably change them to match the spec terminology a bit more closely: 18 byte descriptor,detailed timing descriptor,display descriptor. But that's a separate topic.

• timing->data.other_data.type == type;
  

}

static bool is_detailed_timing_descriptor(const u8 d[18]) @@ -2405,7 +2409,7 @@ is_rb(struct detailed_timing *t, void *data) { u8 *r = (u8 *)t;

• if (!is_display_descriptor(r, EDID_DETAIL_MONITOR_RANGE))

• if (!is_display_descriptor(t, EDID_DETAIL_MONITOR_RANGE)) return;

  if (r[15] & 0x10)

@@ -2431,7 +2435,7 @@ find_gtf2(struct detailed_timing *t, void *data) { u8 *r = (u8 *)t;

• if (!is_display_descriptor(r, EDID_DETAIL_MONITOR_RANGE))

• if (!is_display_descriptor(t, EDID_DETAIL_MONITOR_RANGE)) return;

  if (r[10] == 0x02)

@@ -2987,7 +2991,7 @@ do_inferred_modes(struct detailed_timing *timing, void *c) struct detailed_non_pixel *data = &timing->data.other_data; struct detailed_data_monitor_range *range = &data->data.range;

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

• if (!is_display_descriptor(timing, EDID_DETAIL_MONITOR_RANGE)) return;

  closure->modes += drm_dmt_modes_for_range(closure->connector,

@@ -3067,7 +3071,7 @@ do_established_modes(struct detailed_timing *timing, void *c) { struct detailed_mode_closure *closure = c;

• if (!is_display_descriptor((const u8 *)timing, EDID_DETAIL_EST_TIMINGS))

• if (!is_display_descriptor(timing, EDID_DETAIL_EST_TIMINGS)) return;

  closure->modes += drm_est3_modes(closure->connector, timing);

@@ -3122,7 +3126,7 @@ do_standard_modes(struct detailed_timing *timing, void *c) struct edid *edid = closure->edid; int i;

• if (!is_display_descriptor((const u8 *)timing, EDID_DETAIL_STD_MODES))

• if (!is_display_descriptor(timing, EDID_DETAIL_STD_MODES)) return;

  for (i = 0; i < 6; i++) {

@@ -3231,7 +3235,7 @@ do_cvt_mode(struct detailed_timing *timing, void *c) { struct detailed_mode_closure *closure = c;

• if (!is_display_descriptor((const u8 *)timing, EDID_DETAIL_CVT_3BYTE))

• if (!is_display_descriptor(timing, EDID_DETAIL_CVT_3BYTE)) return;

  closure->modes += drm_cvt_modes(closure->connector, timing);

@@ -4491,7 +4495,7 @@ drm_parse_hdmi_vsdb_audio(struct drm_connector *connector, const u8 *db) static void monitor_name(struct detailed_timing *t, void *data) {

• if (!is_display_descriptor((const u8 *)t, EDID_DETAIL_MONITOR_NAME))

• if (!is_display_descriptor(t, EDID_DETAIL_MONITOR_NAME)) return;

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

@@ -5226,7 +5230,7 @@ void get_monitor_range(struct detailed_timing *timing, const struct detailed_non_pixel *data = &timing->data.other_data; const struct detailed_data_monitor_range *range = &data->data.range;

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

• if (!is_display_descriptor(timing, EDID_DETAIL_MONITOR_RANGE)) return;

  /*

-- 2.30.2