[PATCH 1/8] drm/edid: Check the number of detailed timing descriptors in the CEA ext block
From: Ville Syrjälä ville.syrjala@linux.intel.com
CEA-861 says : "d = offset for the byte following the reserved data block. If no data is provided in the reserved data block, then d=4. If no DTDs are provided, then d=0."
So let's not look for DTDs when d==0. In fact let's just make that <4 since those values would just mean that he DTDs overlap the block header. And let's also check that d isn't so big as to declare the descriptors to live past the block end, although the code does already survive that case as we'd just end up with a negative number of descriptors and the loop would not do anything.
Cc: Allen Chen allen.chen@ite.com.tw Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com --- drivers/gpu/drm/drm_edid.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 99769d6c9f84..1b6e544cf5c7 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -2201,10 +2201,13 @@ typedef void detailed_cb(struct detailed_timing *timing, void *closure); static void cea_for_each_detailed_block(u8 *ext, detailed_cb *cb, void *closure) { - int i, n = 0; + int i, n; u8 d = ext[0x02]; u8 *det_base = ext + d;
+ if (d < 4 || d > 127) + return; + n = (127 - d) / 18; for (i = 0; i < n; i++) cb((struct detailed_timing *)(det_base + 18 * i), closure);
From: Ville Syrjälä ville.syrjala@linux.intel.com
Currently we assume any 18 byte descriptor to be a display descritor if only the tag byte matches the expected value. But for detailed timing descriptors that same byte is just the lower 8 bits of hblank, and as such can match any display descriptor tag. To properly validate that the 18 byte descriptor is in fact a display descriptor we must also examine bytes 0-2 (just byte 1 should actually suffice but the spec does say that bytes 0 and 2 must also always be zero for display descriptors so we check those too).
Unlike Allen's original proposed patch to just fix is_rb() we roll this out across the board to fix everything.
Cc: Allen Chen allen.chen@ite.com.tw Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com --- drivers/gpu/drm/drm_edid.c | 65 ++++++++++++++++++++++++-------------- 1 file changed, 41 insertions(+), 24 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 1b6e544cf5c7..96ae1fde4ce2 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -2196,6 +2196,12 @@ 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) +{ + return d[0] == 0x00 && d[1] == 0x00 && + d[2] == 0x00 && d[3] == tag; +} + typedef void detailed_cb(struct detailed_timing *timing, void *closure);
static void @@ -2257,9 +2263,12 @@ static void is_rb(struct detailed_timing *t, void *data) { u8 *r = (u8 *)t; - if (r[3] == EDID_DETAIL_MONITOR_RANGE) - if (r[15] & 0x10) - *(bool *)data = true; + + if (!is_display_descriptor(r, EDID_DETAIL_MONITOR_RANGE)) + return; + + if (r[15] & 0x10) + *(bool *)data = true; }
/* EDID 1.4 defines this explicitly. For EDID 1.3, we guess, badly. */ @@ -2279,7 +2288,11 @@ static void find_gtf2(struct detailed_timing *t, void *data) { u8 *r = (u8 *)t; - if (r[3] == EDID_DETAIL_MONITOR_RANGE && r[10] == 0x02) + + if (!is_display_descriptor(r, EDID_DETAIL_MONITOR_RANGE)) + return; + + if (r[10] == 0x02) *(u8 **)data = r; }
@@ -2818,7 +2831,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 (data->type != EDID_DETAIL_MONITOR_RANGE) + if (!is_display_descriptor((const u8 *)timing, EDID_DETAIL_MONITOR_RANGE)) return;
closure->modes += drm_dmt_modes_for_range(closure->connector, @@ -2897,10 +2910,11 @@ static void do_established_modes(struct detailed_timing *timing, void *c) { struct detailed_mode_closure *closure = c; - struct detailed_non_pixel *data = &timing->data.other_data;
- if (data->type == EDID_DETAIL_EST_TIMINGS) - closure->modes += drm_est3_modes(closure->connector, timing); + if (!is_display_descriptor((const u8 *)timing, EDID_DETAIL_EST_TIMINGS)) + return; + + closure->modes += drm_est3_modes(closure->connector, timing); }
/** @@ -2949,19 +2963,19 @@ do_standard_modes(struct detailed_timing *timing, void *c) struct detailed_non_pixel *data = &timing->data.other_data; struct drm_connector *connector = closure->connector; struct edid *edid = closure->edid; + int i;
- if (data->type == EDID_DETAIL_STD_MODES) { - int i; - for (i = 0; i < 6; i++) { - struct std_timing *std; - struct drm_display_mode *newmode; + if (!is_display_descriptor((const u8 *)timing, EDID_DETAIL_STD_MODES)) + return;
- std = &data->data.timings[i]; - newmode = drm_mode_std(connector, edid, std); - if (newmode) { - drm_mode_probed_add(connector, newmode); - closure->modes++; - } + for (i = 0; i < 6; i++) { + struct std_timing *std = &data->data.timings[i]; + struct drm_display_mode *newmode; + + newmode = drm_mode_std(connector, edid, std); + if (newmode) { + drm_mode_probed_add(connector, newmode); + closure->modes++; } } } @@ -3056,10 +3070,11 @@ static void do_cvt_mode(struct detailed_timing *timing, void *c) { struct detailed_mode_closure *closure = c; - struct detailed_non_pixel *data = &timing->data.other_data;
- if (data->type == EDID_DETAIL_CVT_3BYTE) - closure->modes += drm_cvt_modes(closure->connector, timing); + if (!is_display_descriptor((const u8 *)timing, EDID_DETAIL_CVT_3BYTE)) + return; + + closure->modes += drm_cvt_modes(closure->connector, timing); }
static int @@ -4285,8 +4300,10 @@ drm_parse_hdmi_vsdb_audio(struct drm_connector *connector, const u8 *db) static void monitor_name(struct detailed_timing *t, void *data) { - if (t->data.other_data.type == EDID_DETAIL_MONITOR_NAME) - *(u8 **)data = t->data.other_data.data.str.str; + if (!is_display_descriptor((const u8 *)t, EDID_DETAIL_MONITOR_NAME)) + return; + + *(u8 **)data = t->data.other_data.data.str.str; }
static int get_monitor_name(struct edid *edid, char name[13])
On Fri, Jan 24, 2020 at 3:02 PM Ville Syrjala ville.syrjala@linux.intel.com wrote:
From: Ville Syrjälä ville.syrjala@linux.intel.com
Currently we assume any 18 byte descriptor to be a display descritor if only the tag byte matches the expected value. But for detailed timing descriptors that same byte is just the lower 8 bits of hblank, and as such can match any display descriptor tag. To properly validate that the 18 byte descriptor is in fact a display descriptor we must also examine bytes 0-2 (just byte 1 should actually suffice but the spec does say that bytes 0 and 2 must also always be zero for display descriptors so we check those too).
Unlike Allen's original proposed patch to just fix is_rb() we roll this out across the board to fix everything.
Cc: Allen Chen allen.chen@ite.com.tw Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com
Acked-by: Alex Deucher alexander.deucher@amd.com
drivers/gpu/drm/drm_edid.c | 65 ++++++++++++++++++++++++-------------- 1 file changed, 41 insertions(+), 24 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 1b6e544cf5c7..96ae1fde4ce2 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -2196,6 +2196,12 @@ 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) +{
return d[0] == 0x00 && d[1] == 0x00 &&d[2] == 0x00 && d[3] == tag;+}
typedef void detailed_cb(struct detailed_timing *timing, void *closure);
static void @@ -2257,9 +2263,12 @@ static void is_rb(struct detailed_timing *t, void *data) { u8 *r = (u8 *)t;
if (r[3] == EDID_DETAIL_MONITOR_RANGE)if (r[15] & 0x10)*(bool *)data = true;
if (!is_display_descriptor(r, EDID_DETAIL_MONITOR_RANGE))return;if (r[15] & 0x10)*(bool *)data = true;}
/* EDID 1.4 defines this explicitly. For EDID 1.3, we guess, badly. */ @@ -2279,7 +2288,11 @@ static void find_gtf2(struct detailed_timing *t, void *data) { u8 *r = (u8 *)t;
if (r[3] == EDID_DETAIL_MONITOR_RANGE && r[10] == 0x02)
if (!is_display_descriptor(r, EDID_DETAIL_MONITOR_RANGE))return;if (r[10] == 0x02) *(u8 **)data = r;}
@@ -2818,7 +2831,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 (data->type != EDID_DETAIL_MONITOR_RANGE)
if (!is_display_descriptor((const u8 *)timing, EDID_DETAIL_MONITOR_RANGE)) return; closure->modes += drm_dmt_modes_for_range(closure->connector,@@ -2897,10 +2910,11 @@ static void do_established_modes(struct detailed_timing *timing, void *c) { struct detailed_mode_closure *closure = c;
struct detailed_non_pixel *data = &timing->data.other_data;if (data->type == EDID_DETAIL_EST_TIMINGS)closure->modes += drm_est3_modes(closure->connector, timing);
if (!is_display_descriptor((const u8 *)timing, EDID_DETAIL_EST_TIMINGS))return;closure->modes += drm_est3_modes(closure->connector, timing);}
/** @@ -2949,19 +2963,19 @@ do_standard_modes(struct detailed_timing *timing, void *c) struct detailed_non_pixel *data = &timing->data.other_data; struct drm_connector *connector = closure->connector; struct edid *edid = closure->edid;
int i;
if (data->type == EDID_DETAIL_STD_MODES) {int i;for (i = 0; i < 6; i++) {struct std_timing *std;struct drm_display_mode *newmode;
if (!is_display_descriptor((const u8 *)timing, EDID_DETAIL_STD_MODES))return;
std = &data->data.timings[i];newmode = drm_mode_std(connector, edid, std);if (newmode) {drm_mode_probed_add(connector, newmode);closure->modes++;}
for (i = 0; i < 6; i++) {struct std_timing *std = &data->data.timings[i];struct drm_display_mode *newmode;newmode = drm_mode_std(connector, edid, std);if (newmode) {drm_mode_probed_add(connector, newmode);closure->modes++; } }} @@ -3056,10 +3070,11 @@ static void do_cvt_mode(struct detailed_timing *timing, void *c) { struct detailed_mode_closure *closure = c;
struct detailed_non_pixel *data = &timing->data.other_data;if (data->type == EDID_DETAIL_CVT_3BYTE)closure->modes += drm_cvt_modes(closure->connector, timing);
if (!is_display_descriptor((const u8 *)timing, EDID_DETAIL_CVT_3BYTE))return;closure->modes += drm_cvt_modes(closure->connector, timing);}
static int @@ -4285,8 +4300,10 @@ drm_parse_hdmi_vsdb_audio(struct drm_connector *connector, const u8 *db) static void monitor_name(struct detailed_timing *t, void *data) {
if (t->data.other_data.type == EDID_DETAIL_MONITOR_NAME)*(u8 **)data = t->data.other_data.data.str.str;
if (!is_display_descriptor((const u8 *)t, EDID_DETAIL_MONITOR_NAME))return;*(u8 **)data = t->data.other_data.data.str.str;}
static int get_monitor_name(struct edid *edid, char name[13])
2.24.1
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
-----Original Message----- From: Intel-gfx intel-gfx-bounces@lists.freedesktop.org On Behalf Of Alex Deucher Sent: Tuesday, January 28, 2020 4:06 AM To: Ville Syrjala ville.syrjala@linux.intel.com Cc: Allen Chen allen.chen@ite.com.tw; Intel Graphics Development <intel- gfx@lists.freedesktop.org>; Maling list - DRI developers <dri- devel@lists.freedesktop.org> Subject: Re: [Intel-gfx] [PATCH 2/8] drm/edid: Don't accept any old garbage as a display descriptor
On Fri, Jan 24, 2020 at 3:02 PM Ville Syrjala ville.syrjala@linux.intel.com wrote:
From: Ville Syrjälä ville.syrjala@linux.intel.com
Currently we assume any 18 byte descriptor to be a display descritor if only the tag byte matches the expected value. But for detailed timing descriptors that same byte is just the lower 8 bits of hblank, and as such can match any display descriptor tag. To properly validate that the 18 byte descriptor is in fact a display descriptor we must also examine bytes 0-2 (just byte 1 should actually suffice but the spec does say that bytes 0 and 2 must also always be zero for display descriptors so we check those too).
Unlike Allen's original proposed patch to just fix is_rb() we roll this out across the board to fix everything.
Cc: Allen Chen allen.chen@ite.com.tw Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com
Acked-by: Alex Deucher alexander.deucher@amd.com
Looks Good to me. Reviewed-by: Uma Shankar uma.shankar@intel.com
drivers/gpu/drm/drm_edid.c | 65 ++++++++++++++++++++++++-------------- 1 file changed, 41 insertions(+), 24 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 1b6e544cf5c7..96ae1fde4ce2 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -2196,6 +2196,12 @@ 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) {
return d[0] == 0x00 && d[1] == 0x00 &&d[2] == 0x00 && d[3] == tag; }typedef void detailed_cb(struct detailed_timing *timing, void *closure);
static void @@ -2257,9 +2263,12 @@ static void is_rb(struct detailed_timing *t, void *data) { u8 *r = (u8 *)t;
if (r[3] == EDID_DETAIL_MONITOR_RANGE)if (r[15] & 0x10)*(bool *)data = true;
if (!is_display_descriptor(r, EDID_DETAIL_MONITOR_RANGE))return;if (r[15] & 0x10)*(bool *)data = true;}
/* EDID 1.4 defines this explicitly. For EDID 1.3, we guess, badly. */ @@ -2279,7 +2288,11 @@ static void find_gtf2(struct detailed_timing *t, void *data) { u8 *r = (u8 *)t;
if (r[3] == EDID_DETAIL_MONITOR_RANGE && r[10] == 0x02)
if (!is_display_descriptor(r, EDID_DETAIL_MONITOR_RANGE))return;if (r[10] == 0x02) *(u8 **)data = r;}
@@ -2818,7 +2831,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 (data->type != EDID_DETAIL_MONITOR_RANGE)
if (!is_display_descriptor((const u8 *)timing,EDID_DETAIL_MONITOR_RANGE)) return;
closure->modes += drm_dmt_modes_for_range(closure->connector,@@ -2897,10 +2910,11 @@ static void do_established_modes(struct detailed_timing *timing, void *c) { struct detailed_mode_closure *closure = c;
struct detailed_non_pixel *data = &timing->data.other_data;if (data->type == EDID_DETAIL_EST_TIMINGS)closure->modes += drm_est3_modes(closure->connector, timing);
if (!is_display_descriptor((const u8 *)timing, EDID_DETAIL_EST_TIMINGS))return;closure->modes += drm_est3_modes(closure->connector, timing);}
/** @@ -2949,19 +2963,19 @@ do_standard_modes(struct detailed_timing *timing,
void *c)
struct detailed_non_pixel *data = &timing->data.other_data; struct drm_connector *connector = closure->connector; struct edid *edid = closure->edid;
int i;
if (data->type == EDID_DETAIL_STD_MODES) {int i;for (i = 0; i < 6; i++) {struct std_timing *std;struct drm_display_mode *newmode;
if (!is_display_descriptor((const u8 *)timing, EDID_DETAIL_STD_MODES))return;
std = &data->data.timings[i];newmode = drm_mode_std(connector, edid, std);if (newmode) {drm_mode_probed_add(connector, newmode);closure->modes++;}
for (i = 0; i < 6; i++) {struct std_timing *std = &data->data.timings[i];struct drm_display_mode *newmode;newmode = drm_mode_std(connector, edid, std);if (newmode) {drm_mode_probed_add(connector, newmode);closure->modes++; } }} @@ -3056,10 +3070,11 @@ static void do_cvt_mode(struct detailed_timing *timing, void *c) { struct detailed_mode_closure *closure = c;
struct detailed_non_pixel *data = &timing->data.other_data;if (data->type == EDID_DETAIL_CVT_3BYTE)closure->modes += drm_cvt_modes(closure->connector, timing);
if (!is_display_descriptor((const u8 *)timing, EDID_DETAIL_CVT_3BYTE))return;closure->modes += drm_cvt_modes(closure->connector, timing);}
static int @@ -4285,8 +4300,10 @@ drm_parse_hdmi_vsdb_audio(struct drm_connector *connector, const u8 *db) static void monitor_name(struct detailed_timing *t, void *data) {
if (t->data.other_data.type == EDID_DETAIL_MONITOR_NAME)*(u8 **)data = t->data.other_data.data.str.str;
if (!is_display_descriptor((const u8 *)t, EDID_DETAIL_MONITOR_NAME))return;*(u8 **)data = t->data.other_data.data.str.str;}
static int get_monitor_name(struct edid *edid, char name[13])
2.24.1
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
From: Ville Syrjälä ville.syrjala@linux.intel.com
Let's introduce is_detailed_timing_descritor() as the opposite counterpart of is_display_descriptor().
Cc: Allen Chen allen.chen@ite.com.tw Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com --- drivers/gpu/drm/drm_edid.c | 42 ++++++++++++++++++++++---------------- 1 file changed, 24 insertions(+), 18 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 96ae1fde4ce2..d6bce58b27ac 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -2202,6 +2202,11 @@ static bool is_display_descriptor(const u8 d[18], u8 tag) d[2] == 0x00 && d[3] == tag; }
+static bool is_detailed_timing_descriptor(const u8 d[18]) +{ + return d[0] != 0x00 || d[1] != 0x00; +} + typedef void detailed_cb(struct detailed_timing *timing, void *closure);
static void @@ -3101,27 +3106,28 @@ do_detailed_mode(struct detailed_timing *timing, void *c) struct detailed_mode_closure *closure = c; struct drm_display_mode *newmode;
- if (timing->pixel_clock) { - newmode = drm_mode_detailed(closure->connector->dev, - closure->edid, timing, - closure->quirks); - if (!newmode) - return; + if (!is_detailed_timing_descriptor((const u8 *)timing)) + return; + + newmode = drm_mode_detailed(closure->connector->dev, + closure->edid, timing, + closure->quirks); + if (!newmode) + return;
- if (closure->preferred) - newmode->type |= DRM_MODE_TYPE_PREFERRED; + if (closure->preferred) + newmode->type |= DRM_MODE_TYPE_PREFERRED;
- /* - * Detailed modes are limited to 10kHz pixel clock resolution, - * so fix up anything that looks like CEA/HDMI mode, but the clock - * is just slightly off. - */ - fixup_detailed_cea_mode_clock(newmode); + /* + * Detailed modes are limited to 10kHz pixel clock resolution, + * so fix up anything that looks like CEA/HDMI mode, but the clock + * is just slightly off. + */ + fixup_detailed_cea_mode_clock(newmode);
- drm_mode_probed_add(closure->connector, newmode); - closure->modes++; - closure->preferred = false; - } + drm_mode_probed_add(closure->connector, newmode); + closure->modes++; + closure->preferred = false; }
/*
On Fri, Jan 24, 2020 at 3:02 PM Ville Syrjala ville.syrjala@linux.intel.com wrote:
From: Ville Syrjälä ville.syrjala@linux.intel.com
Let's introduce is_detailed_timing_descritor() as the opposite counterpart of is_display_descriptor().
Cc: Allen Chen allen.chen@ite.com.tw Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com
Acked-by: Alex Deucher alexander.deucher@amd.com
drivers/gpu/drm/drm_edid.c | 42 ++++++++++++++++++++++---------------- 1 file changed, 24 insertions(+), 18 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 96ae1fde4ce2..d6bce58b27ac 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -2202,6 +2202,11 @@ static bool is_display_descriptor(const u8 d[18], u8 tag) d[2] == 0x00 && d[3] == tag; }
+static bool is_detailed_timing_descriptor(const u8 d[18]) +{
return d[0] != 0x00 || d[1] != 0x00;+}
typedef void detailed_cb(struct detailed_timing *timing, void *closure);
static void @@ -3101,27 +3106,28 @@ do_detailed_mode(struct detailed_timing *timing, void *c) struct detailed_mode_closure *closure = c; struct drm_display_mode *newmode;
if (timing->pixel_clock) {newmode = drm_mode_detailed(closure->connector->dev,closure->edid, timing,closure->quirks);if (!newmode)return;
if (!is_detailed_timing_descriptor((const u8 *)timing))return;newmode = drm_mode_detailed(closure->connector->dev,closure->edid, timing,closure->quirks);if (!newmode)return;
if (closure->preferred)newmode->type |= DRM_MODE_TYPE_PREFERRED;
if (closure->preferred)newmode->type |= DRM_MODE_TYPE_PREFERRED;
/** Detailed modes are limited to 10kHz pixel clock resolution,* so fix up anything that looks like CEA/HDMI mode, but the clock* is just slightly off.*/fixup_detailed_cea_mode_clock(newmode);
/** Detailed modes are limited to 10kHz pixel clock resolution,* so fix up anything that looks like CEA/HDMI mode, but the clock* is just slightly off.*/fixup_detailed_cea_mode_clock(newmode);
drm_mode_probed_add(closure->connector, newmode);closure->modes++;closure->preferred = false;}
drm_mode_probed_add(closure->connector, newmode);closure->modes++;closure->preferred = false;}
/*
2.24.1
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
-----Original Message----- From: Intel-gfx intel-gfx-bounces@lists.freedesktop.org On Behalf Of Alex Deucher Sent: Tuesday, January 28, 2020 4:06 AM To: Ville Syrjala ville.syrjala@linux.intel.com Cc: Allen Chen allen.chen@ite.com.tw; Intel Graphics Development <intel- gfx@lists.freedesktop.org>; Maling list - DRI developers <dri- devel@lists.freedesktop.org> Subject: Re: [Intel-gfx] [PATCH 3/8] drm/edid: Introduce is_detailed_timing_descritor()
On Fri, Jan 24, 2020 at 3:02 PM Ville Syrjala ville.syrjala@linux.intel.com wrote:
From: Ville Syrjälä ville.syrjala@linux.intel.com
Let's introduce is_detailed_timing_descritor() as the opposite counterpart of is_display_descriptor().
Cc: Allen Chen allen.chen@ite.com.tw Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com
Acked-by: Alex Deucher alexander.deucher@amd.com
Looks good. Reviewed-by: Uma Shankar uma.shankar@intel.com
drivers/gpu/drm/drm_edid.c | 42 ++++++++++++++++++++++---------------- 1 file changed, 24 insertions(+), 18 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 96ae1fde4ce2..d6bce58b27ac 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -2202,6 +2202,11 @@ static bool is_display_descriptor(const u8 d[18], u8
tag)
d[2] == 0x00 && d[3] == tag; }+static bool is_detailed_timing_descriptor(const u8 d[18]) {
return d[0] != 0x00 || d[1] != 0x00; }typedef void detailed_cb(struct detailed_timing *timing, void *closure);
static void @@ -3101,27 +3106,28 @@ do_detailed_mode(struct detailed_timing *timing,
void *c)
struct detailed_mode_closure *closure = c; struct drm_display_mode *newmode;
if (timing->pixel_clock) {newmode = drm_mode_detailed(closure->connector->dev,closure->edid, timing,closure->quirks);if (!newmode)return;
if (!is_detailed_timing_descriptor((const u8 *)timing))return;newmode = drm_mode_detailed(closure->connector->dev,closure->edid, timing,closure->quirks);if (!newmode)return;
if (closure->preferred)newmode->type |= DRM_MODE_TYPE_PREFERRED;
if (closure->preferred)newmode->type |= DRM_MODE_TYPE_PREFERRED;
/** Detailed modes are limited to 10kHz pixel clock resolution,* so fix up anything that looks like CEA/HDMI mode, but the clock* is just slightly off.*/fixup_detailed_cea_mode_clock(newmode);
/** Detailed modes are limited to 10kHz pixel clock resolution,* so fix up anything that looks like CEA/HDMI mode, but the clock* is just slightly off.*/fixup_detailed_cea_mode_clock(newmode);
drm_mode_probed_add(closure->connector, newmode);closure->modes++;closure->preferred = false;}
drm_mode_probed_add(closure->connector, newmode);closure->modes++;closure->preferred = false;}
/*
2.24.1
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
From: Ville Syrjälä ville.syrjala@linux.intel.com
Nuke some whitespace that shouldn't be there.
Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com --- drivers/gpu/drm/drm_edid.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index d6bce58b27ac..3df5744026b0 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -2842,7 +2842,7 @@ do_inferred_modes(struct detailed_timing *timing, void *c) closure->modes += drm_dmt_modes_for_range(closure->connector, closure->edid, timing); - + if (!version_greater(closure->edid, 1, 1)) return; /* GTF not defined yet */
@@ -3084,7 +3084,7 @@ do_cvt_mode(struct detailed_timing *timing, void *c)
static int add_cvt_modes(struct drm_connector *connector, struct edid *edid) -{ +{ struct detailed_mode_closure closure = { .connector = connector, .edid = edid, @@ -4342,7 +4342,7 @@ void drm_edid_get_monitor_name(struct edid *edid, char *name, int bufsize) { int name_length; char buf[13]; - + if (bufsize <= 0) return;
Title should be s/i915/edid/ , with that fixed: Reviewed-by: Alex Deucher alexander.deucher@amd.com
On Fri, Jan 24, 2020 at 3:03 PM Ville Syrjala ville.syrjala@linux.intel.com wrote:
From: Ville Syrjälä ville.syrjala@linux.intel.com
Nuke some whitespace that shouldn't be there.
Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com
drivers/gpu/drm/drm_edid.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index d6bce58b27ac..3df5744026b0 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -2842,7 +2842,7 @@ do_inferred_modes(struct detailed_timing *timing, void *c) closure->modes += drm_dmt_modes_for_range(closure->connector, closure->edid, timing);
if (!version_greater(closure->edid, 1, 1)) return; /* GTF not defined yet */@@ -3084,7 +3084,7 @@ do_cvt_mode(struct detailed_timing *timing, void *c)
static int add_cvt_modes(struct drm_connector *connector, struct edid *edid) -{ +{ struct detailed_mode_closure closure = { .connector = connector, .edid = edid, @@ -4342,7 +4342,7 @@ void drm_edid_get_monitor_name(struct edid *edid, char *name, int bufsize) { int name_length; char buf[13];
if (bufsize <= 0) return;-- 2.24.1
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
-----Original Message----- From: Intel-gfx intel-gfx-bounces@lists.freedesktop.org On Behalf Of Alex Deucher Sent: Tuesday, January 28, 2020 3:58 AM To: Ville Syrjala ville.syrjala@linux.intel.com Cc: Intel Graphics Development intel-gfx@lists.freedesktop.org; Maling list - DRI developers dri-devel@lists.freedesktop.org Subject: Re: [Intel-gfx] [PATCH 4/8] drm/i915: Clear out spurious whitespace
Title should be s/i915/edid/ , with that fixed: Reviewed-by: Alex Deucher alexander.deucher@amd.com
Yeah with the title fixed, this is Reviewed-by: Uma Shankar uma.shankar@intel.com
On Fri, Jan 24, 2020 at 3:03 PM Ville Syrjala ville.syrjala@linux.intel.com wrote:
From: Ville Syrjälä ville.syrjala@linux.intel.com
Nuke some whitespace that shouldn't be there.
Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com
drivers/gpu/drm/drm_edid.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index d6bce58b27ac..3df5744026b0 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -2842,7 +2842,7 @@ do_inferred_modes(struct detailed_timing *timing, void
*c)
closure->modes += drm_dmt_modes_for_range(closure->connector, closure->edid, timing);
if (!version_greater(closure->edid, 1, 1)) return; /* GTF not defined yet */@@ -3084,7 +3084,7 @@ do_cvt_mode(struct detailed_timing *timing, void *c)
static int add_cvt_modes(struct drm_connector *connector, struct edid *edid) -{ +{ struct detailed_mode_closure closure = { .connector = connector, .edid = edid, @@ -4342,7 +4342,7 @@ void drm_edid_get_monitor_name(struct edid *edid, char *name, int bufsize) { int name_length; char buf[13];
if (bufsize <= 0) return;-- 2.24.1
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
From: Ville Syrjälä ville.syrjala@linux.intel.com
After much head scratching I managed to convince myself that for_each_displayid_db() has already done the bounds checks for the DispID CEA data block. Which is why we don't need to repeat them in cea_db_offsets(). To avoid having to go through that pain again in the future add a comment which explains this fact.
Cc: Andres Rodriguez andresx7@gmail.com Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com --- drivers/gpu/drm/drm_edid.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 3df5744026b0..0369a54e3d32 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -4001,6 +4001,10 @@ cea_db_offsets(const u8 *cea, int *start, int *end) * no non-DTD data. */ if (cea[0] == DATA_BLOCK_CTA) { + /* + * for_each_displayid_db() has already verified + * that these stay within expected bounds. + */ *start = 3; *end = *start + cea[2]; } else if (cea[0] == CEA_EXT) {
On Fri, Jan 24, 2020 at 3:03 PM Ville Syrjala ville.syrjala@linux.intel.com wrote:
From: Ville Syrjälä ville.syrjala@linux.intel.com
After much head scratching I managed to convince myself that for_each_displayid_db() has already done the bounds checks for the DispID CEA data block. Which is why we don't need to repeat them in cea_db_offsets(). To avoid having to go through that pain again in the future add a comment which explains this fact.
Cc: Andres Rodriguez andresx7@gmail.com Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com
drivers/gpu/drm/drm_edid.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 3df5744026b0..0369a54e3d32 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -4001,6 +4001,10 @@ cea_db_offsets(const u8 *cea, int *start, int *end) * no non-DTD data. */ if (cea[0] == DATA_BLOCK_CTA) {
/** for_each_displayid_db() has already verified* that these stay within expected bounds.*/
I think the preferred format is to have the start of the comment be on the first line after the /* with that fixed: Acked-by: Alex Deucher alexander.deucher@amd.com
*start = 3; *end = *start + cea[2]; } else if (cea[0] == CEA_EXT) {-- 2.24.1
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
On Mon, Jan 27, 2020 at 05:30:42PM -0500, Alex Deucher wrote:
On Fri, Jan 24, 2020 at 3:03 PM Ville Syrjala ville.syrjala@linux.intel.com wrote:
From: Ville Syrjälä ville.syrjala@linux.intel.com
After much head scratching I managed to convince myself that for_each_displayid_db() has already done the bounds checks for the DispID CEA data block. Which is why we don't need to repeat them in cea_db_offsets(). To avoid having to go through that pain again in the future add a comment which explains this fact.
Cc: Andres Rodriguez andresx7@gmail.com Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com
drivers/gpu/drm/drm_edid.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 3df5744026b0..0369a54e3d32 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -4001,6 +4001,10 @@ cea_db_offsets(const u8 *cea, int *start, int *end) * no non-DTD data. */ if (cea[0] == DATA_BLOCK_CTA) {
/** for_each_displayid_db() has already verified* that these stay within expected bounds.*/I think the preferred format is to have the start of the comment be on the first line after the /* with that fixed:
Nope.
Acked-by: Alex Deucher alexander.deucher@amd.com
*start = 3; *end = *start + cea[2]; } else if (cea[0] == CEA_EXT) {-- 2.24.1
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
-----Original Message----- From: dri-devel dri-devel-bounces@lists.freedesktop.org On Behalf Of Ville Syrjälä Sent: Tuesday, January 28, 2020 5:14 PM To: Alex Deucher alexdeucher@gmail.com Cc: Intel Graphics Development intel-gfx@lists.freedesktop.org; Andres Rodriguez andresx7@gmail.com; Maling list - DRI developers <dri- devel@lists.freedesktop.org> Subject: Re: [PATCH 5/8] drm/edid: Document why we don't bounds check the DispID CEA block start/end
On Mon, Jan 27, 2020 at 05:30:42PM -0500, Alex Deucher wrote:
On Fri, Jan 24, 2020 at 3:03 PM Ville Syrjala ville.syrjala@linux.intel.com wrote:
From: Ville Syrjälä ville.syrjala@linux.intel.com
After much head scratching I managed to convince myself that for_each_displayid_db() has already done the bounds checks for the DispID CEA data block. Which is why we don't need to repeat them in cea_db_offsets(). To avoid having to go through that pain again in the future add a comment which explains this fact.
Cc: Andres Rodriguez andresx7@gmail.com Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com
drivers/gpu/drm/drm_edid.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 3df5744026b0..0369a54e3d32 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -4001,6 +4001,10 @@ cea_db_offsets(const u8 *cea, int *start, int *end) * no non-DTD data. */ if (cea[0] == DATA_BLOCK_CTA) {
/** for_each_displayid_db() has already verified* that these stay within expected bounds.*/I think the preferred format is to have the start of the comment be on the first line after the /* with that fixed:
Nope.
Yes the style is correct here, comment is apt as well. Reviewed-by: Uma Shankar uma.shankar@intel.com
Acked-by: Alex Deucher alexander.deucher@amd.com
*start = 3; *end = *start + cea[2]; } else if (cea[0] == CEA_EXT) {-- 2.24.1
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
-- Ville Syrjälä Intel _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
From: Ville Syrjälä ville.syrjala@linux.intel.com
I don't understand what the DispID CEA data block revision means. The spec doesn't say. I guess some DispID must have a value of >= 3 in there or else we generally wouldn't even parse the CEA data blocks. Or does all this code actually not do anything?
Cc: Andres Rodriguez andresx7@gmail.com Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com --- drivers/gpu/drm/drm_edid.c | 7 +++++++ 1 file changed, 7 insertions(+)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 0369a54e3d32..fd9b724067a7 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -3977,6 +3977,13 @@ cea_db_tag(const u8 *db) static int cea_revision(const u8 *cea) { + /* + * FIXME is this correct for the DispID variant? + * The DispID spec doesn't really specify whether + * this is the revision of the CEA extension or + * the DispID CEA data block. And the only value + * given as an example is 0. + */ return cea[1]; }
On Fri, Jan 24, 2020 at 3:02 PM Ville Syrjala ville.syrjala@linux.intel.com wrote:
From: Ville Syrjälä ville.syrjala@linux.intel.com
I don't understand what the DispID CEA data block revision means. The spec doesn't say. I guess some DispID must have a value of >= 3 in there or else we generally wouldn't even parse the CEA data blocks. Or does all this code actually not do anything?
Cc: Andres Rodriguez andresx7@gmail.com Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com
drivers/gpu/drm/drm_edid.c | 7 +++++++ 1 file changed, 7 insertions(+)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 0369a54e3d32..fd9b724067a7 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -3977,6 +3977,13 @@ cea_db_tag(const u8 *db) static int cea_revision(const u8 *cea) {
/** FIXME is this correct for the DispID variant?* The DispID spec doesn't really specify whether* this is the revision of the CEA extension or* the DispID CEA data block. And the only value* given as an example is 0.*/
Same comment as the previous patch regarding the comment formatting.
Alex
return cea[1];}
-- 2.24.1
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
-----Original Message----- From: Intel-gfx intel-gfx-bounces@lists.freedesktop.org On Behalf Of Ville Syrjala Sent: Saturday, January 25, 2020 1:32 AM To: dri-devel@lists.freedesktop.org Cc: intel-gfx@lists.freedesktop.org; Andres Rodriguez andresx7@gmail.com Subject: [Intel-gfx] [PATCH 6/8] drm/edid: Add a FIXME about DispID CEA data block revision
From: Ville Syrjälä ville.syrjala@linux.intel.com
I don't understand what the DispID CEA data block revision means. The spec doesn't say. I guess some DispID must have a value of >= 3 in there or else we generally wouldn't even parse the CEA data blocks. Or does all this code actually not do anything?
This signifies the CTA extension revision (byte 1 of the block). As per the spec, seems like Version 1 is legacy and 2 is deprecated. So version >=3 is checked here. Refer section 7.3 of CTA-861-G
Cc: Andres Rodriguez andresx7@gmail.com Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com
drivers/gpu/drm/drm_edid.c | 7 +++++++ 1 file changed, 7 insertions(+)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 0369a54e3d32..fd9b724067a7 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -3977,6 +3977,13 @@ cea_db_tag(const u8 *db) static int cea_revision(const u8 *cea) {
- /*
* FIXME is this correct for the DispID variant?* The DispID spec doesn't really specify whether* this is the revision of the CEA extension or* the DispID CEA data block. And the only value* given as an example is 0.*/}
-- 2.24.1
Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Mon, Feb 03, 2020 at 08:15:51PM +0000, Shankar, Uma wrote:
-----Original Message----- From: Intel-gfx intel-gfx-bounces@lists.freedesktop.org On Behalf Of Ville Syrjala Sent: Saturday, January 25, 2020 1:32 AM To: dri-devel@lists.freedesktop.org Cc: intel-gfx@lists.freedesktop.org; Andres Rodriguez andresx7@gmail.com Subject: [Intel-gfx] [PATCH 6/8] drm/edid: Add a FIXME about DispID CEA data block revision
From: Ville Syrjälä ville.syrjala@linux.intel.com
I don't understand what the DispID CEA data block revision means. The spec doesn't say. I guess some DispID must have a value of >= 3 in there or else we generally wouldn't even parse the CEA data blocks. Or does all this code actually not do anything?
This signifies the CTA extension revision (byte 1 of the block). As per the spec, seems like Version 1 is legacy and 2 is deprecated. So version >=3 is checked here. Refer section 7.3 of CTA-861-G
The confusion is about the revision field in the DispID CTA block, not in the CTA extension block.
Cc: Andres Rodriguez andresx7@gmail.com Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com
drivers/gpu/drm/drm_edid.c | 7 +++++++ 1 file changed, 7 insertions(+)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 0369a54e3d32..fd9b724067a7 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -3977,6 +3977,13 @@ cea_db_tag(const u8 *db) static int cea_revision(const u8 *cea) {
- /*
* FIXME is this correct for the DispID variant?* The DispID spec doesn't really specify whether* this is the revision of the CEA extension or* the DispID CEA data block. And the only value* given as an example is 0.*/}
-- 2.24.1
Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
-----Original Message----- From: Ville Syrjälä ville.syrjala@linux.intel.com Sent: Tuesday, February 4, 2020 7:02 PM To: Shankar, Uma uma.shankar@intel.com Cc: dri-devel@lists.freedesktop.org; intel-gfx@lists.freedesktop.org; Andres Rodriguez andresx7@gmail.com Subject: Re: [Intel-gfx] [PATCH 6/8] drm/edid: Add a FIXME about DispID CEA data block revision
On Mon, Feb 03, 2020 at 08:15:51PM +0000, Shankar, Uma wrote:
-----Original Message----- From: Intel-gfx intel-gfx-bounces@lists.freedesktop.org On Behalf Of Ville Syrjala Sent: Saturday, January 25, 2020 1:32 AM To: dri-devel@lists.freedesktop.org Cc: intel-gfx@lists.freedesktop.org; Andres Rodriguez andresx7@gmail.com Subject: [Intel-gfx] [PATCH 6/8] drm/edid: Add a FIXME about DispID CEA data block revision
From: Ville Syrjälä ville.syrjala@linux.intel.com
I don't understand what the DispID CEA data block revision means. The spec doesn't say. I guess some DispID must have a value of >= 3 in there or else we generally wouldn't even parse the CEA data blocks. Or does all this code actually not do anything?
This signifies the CTA extension revision (byte 1 of the block). As per the spec, seems like Version 1 is legacy and 2 is deprecated. So version >=3 is
checked here.
Refer section 7.3 of CTA-861-G
The confusion is about the revision field in the DispID CTA block, not in the CTA extension block.
Oh ok, got the ambiguity here. Not sure if we actually get >3 here as value for the block revision, totally unclear from spec, default being 0. Good to have this comment till we get some clarity on its significance. Thanks for the clarification.
Reviewed-by: Uma Shankar uma.shankar@intel.com
Cc: Andres Rodriguez andresx7@gmail.com Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com
drivers/gpu/drm/drm_edid.c | 7 +++++++ 1 file changed, 7 insertions(+)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 0369a54e3d32..fd9b724067a7 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -3977,6 +3977,13 @@ cea_db_tag(const u8 *db) static int cea_revision(const u8 *cea) {
- /*
* FIXME is this correct for the DispID variant?* The DispID spec doesn't really specify whether* this is the revision of the CEA extension or* the DispID CEA data block. And the only value* given as an example is 0.*/}
-- 2.24.1
Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
-- Ville Syrjälä Intel
From: Ville Syrjälä ville.syrjala@linux.intel.com
Let's try to make a lot more stuff const in the edid parser.
The "downside" is that we can no longer mangle the EDID in the middle of the parsing to apply quirks (drm_mode_detailed()). I don't really think mangling the blob itself is such a great idea anyway so I won't miss that part. But if we do want it back I guess we should do the mangling in one explicit place before we otherwise parse the EDID.
Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com --- drivers/gpu/drm/drm_connector.c | 4 +- drivers/gpu/drm/drm_edid.c | 303 ++++++++++++++++++-------------- include/drm/drm_connector.h | 4 +- 3 files changed, 176 insertions(+), 135 deletions(-)
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c index f632ca05960e..92a5cd6ff6b1 100644 --- a/drivers/gpu/drm/drm_connector.c +++ b/drivers/gpu/drm/drm_connector.c @@ -2377,7 +2377,7 @@ EXPORT_SYMBOL(drm_mode_put_tile_group); * tile group or NULL if not found. */ struct drm_tile_group *drm_mode_get_tile_group(struct drm_device *dev, - char topology[8]) + const u8 topology[8]) { struct drm_tile_group *tg; int id; @@ -2407,7 +2407,7 @@ EXPORT_SYMBOL(drm_mode_get_tile_group); * new tile group or NULL. */ struct drm_tile_group *drm_mode_create_tile_group(struct drm_device *dev, - char topology[8]) + const u8 topology[8]) { struct drm_tile_group *tg; int ret; diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index fd9b724067a7..8e76efe1654d 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -88,7 +88,7 @@
struct detailed_mode_closure { struct drm_connector *connector; - struct edid *edid; + const struct edid *edid; bool preferred; u32 quirks; int modes; @@ -1584,8 +1584,8 @@ MODULE_PARM_DESC(edid_fixup, "Minimum number of valid EDID header bytes (0-8, default 6)");
static void drm_get_displayid(struct drm_connector *connector, - struct edid *edid); -static int validate_displayid(u8 *displayid, int length, int idx); + const struct edid *edid); +static int validate_displayid(const u8 *displayid, int length, int idx);
static int drm_edid_block_checksum(const u8 *raw_edid) { @@ -2207,41 +2207,41 @@ static bool is_detailed_timing_descriptor(const u8 d[18]) return d[0] != 0x00 || d[1] != 0x00; }
-typedef void detailed_cb(struct detailed_timing *timing, void *closure); +typedef void detailed_cb(const struct detailed_timing *timing, void *closure);
static void -cea_for_each_detailed_block(u8 *ext, detailed_cb *cb, void *closure) +cea_for_each_detailed_block(const u8 *ext, detailed_cb *cb, void *closure) { int i, n; u8 d = ext[0x02]; - u8 *det_base = ext + d; + const u8 *det_base = ext + d;
if (d < 4 || d > 127) return;
n = (127 - d) / 18; for (i = 0; i < n; i++) - cb((struct detailed_timing *)(det_base + 18 * i), closure); + cb((const struct detailed_timing *)(det_base + 18 * i), closure); }
static void -vtb_for_each_detailed_block(u8 *ext, detailed_cb *cb, void *closure) +vtb_for_each_detailed_block(const u8 *ext, detailed_cb *cb, void *closure) { unsigned int i, n = min((int)ext[0x02], 6); - u8 *det_base = ext + 5; + const u8 *det_base = ext + 5;
if (ext[0x01] != 1) return; /* unknown version */
for (i = 0; i < n; i++) - cb((struct detailed_timing *)(det_base + 18 * i), closure); + cb((const struct detailed_timing *)(det_base + 18 * i), closure); }
static void -drm_for_each_detailed_block(u8 *raw_edid, detailed_cb *cb, void *closure) +drm_for_each_detailed_block(const u8 *raw_edid, detailed_cb *cb, void *closure) { + const struct edid *edid = (struct edid *)raw_edid; int i; - struct edid *edid = (struct edid *)raw_edid;
if (edid == NULL) return; @@ -2250,7 +2250,7 @@ drm_for_each_detailed_block(u8 *raw_edid, detailed_cb *cb, void *closure) cb(&(edid->detailed_timings[i]), closure);
for (i = 1; i <= raw_edid[0x7e]; i++) { - u8 *ext = raw_edid + (i * EDID_LENGTH); + const u8 *ext = raw_edid + (i * EDID_LENGTH); switch (*ext) { case CEA_EXT: cea_for_each_detailed_block(ext, cb, closure); @@ -2264,81 +2264,105 @@ drm_for_each_detailed_block(u8 *raw_edid, detailed_cb *cb, void *closure) } }
+struct bool_closure { + bool ret; +}; + static void -is_rb(struct detailed_timing *t, void *data) +is_rb(const struct detailed_timing *t, void *c) { - u8 *r = (u8 *)t; + struct bool_closure *closure = c; + const u8 *r = (const u8 *)t;
if (!is_display_descriptor(r, EDID_DETAIL_MONITOR_RANGE)) return;
if (r[15] & 0x10) - *(bool *)data = true; + closure->ret = true; }
/* EDID 1.4 defines this explicitly. For EDID 1.3, we guess, badly. */ static bool -drm_monitor_supports_rb(struct edid *edid) +drm_monitor_supports_rb(const struct edid *edid) { if (edid->revision >= 4) { - bool ret = false; - drm_for_each_detailed_block((u8 *)edid, is_rb, &ret); - return ret; + struct bool_closure closure = { + .ret = false, + }; + + drm_for_each_detailed_block((u8 *)edid, is_rb, &closure); + + return closure.ret; }
return ((edid->input & DRM_EDID_INPUT_DIGITAL) != 0); }
+struct data_closure { + const u8 *data; +}; + static void -find_gtf2(struct detailed_timing *t, void *data) +do_find_gtf2(const struct detailed_timing *t, void *c) { - u8 *r = (u8 *)t; + struct data_closure *closure = c; + const u8 *r = (const u8 *)t;
if (!is_display_descriptor(r, EDID_DETAIL_MONITOR_RANGE)) return;
if (r[10] == 0x02) - *(u8 **)data = r; + closure->data = r; +} + +static const u8 * +find_gtf2_descriptor(const struct edid *edid) +{ + struct data_closure closure = {}; + + drm_for_each_detailed_block((u8 *)edid, do_find_gtf2, &closure); + + return closure.data; }
/* Secondary GTF curve kicks in above some break frequency */ static int -drm_gtf2_hbreak(struct edid *edid) +drm_gtf2_hbreak(const struct edid *edid) { - u8 *r = NULL; - drm_for_each_detailed_block((u8 *)edid, find_gtf2, &r); - return r ? (r[12] * 2) : 0; + const u8 *r = find_gtf2_descriptor(edid); + + return r ? r[12] * 2 : 0; }
static int -drm_gtf2_2c(struct edid *edid) +drm_gtf2_2c(const struct edid *edid) { - u8 *r = NULL; - drm_for_each_detailed_block((u8 *)edid, find_gtf2, &r); + const u8 *r = find_gtf2_descriptor(edid); + return r ? r[13] : 0; }
static int -drm_gtf2_m(struct edid *edid) +drm_gtf2_m(const struct edid *edid) { - u8 *r = NULL; - drm_for_each_detailed_block((u8 *)edid, find_gtf2, &r); - return r ? (r[15] << 8) + r[14] : 0; + const u8 *r = find_gtf2_descriptor(edid); + + return r ? (r[15] << 8) | r[14] : 0; }
static int -drm_gtf2_k(struct edid *edid) +drm_gtf2_k(const struct edid *edid) { - u8 *r = NULL; - drm_for_each_detailed_block((u8 *)edid, find_gtf2, &r); + const u8 *r = find_gtf2_descriptor(edid); + return r ? r[16] : 0; }
static int -drm_gtf2_2j(struct edid *edid) +drm_gtf2_2j(const struct edid *edid) { - u8 *r = NULL; - drm_for_each_detailed_block((u8 *)edid, find_gtf2, &r); + const u8 *r = find_gtf2_descriptor(edid); + return r ? r[17] : 0; }
@@ -2346,7 +2370,7 @@ drm_gtf2_2j(struct edid *edid) * standard_timing_level - get std. timing level(CVT/GTF/DMT) * @edid: EDID block to scan */ -static int standard_timing_level(struct edid *edid) +static int standard_timing_level(const struct edid *edid) { if (edid->revision >= 2) { if (edid->revision >= 4 && (edid->features & DRM_EDID_FEATURE_DEFAULT_GTF)) @@ -2381,8 +2405,9 @@ bad_std_timing(u8 a, u8 b) * and convert them into a real mode using CVT/GTF/DMT. */ static struct drm_display_mode * -drm_mode_std(struct drm_connector *connector, struct edid *edid, - struct std_timing *t) +drm_mode_std(struct drm_connector *connector, + const struct edid *edid, + const struct std_timing *t) { struct drm_device *dev = connector->dev; struct drm_display_mode *m, *mode = NULL; @@ -2500,7 +2525,7 @@ drm_mode_std(struct drm_connector *connector, struct edid *edid, */ static void drm_mode_do_interlace_quirk(struct drm_display_mode *mode, - struct detailed_pixel_timing *pt) + const struct detailed_pixel_timing *pt) { int i; static const struct { @@ -2543,12 +2568,12 @@ drm_mode_do_interlace_quirk(struct drm_display_mode *mode, * return a new struct drm_display_mode. */ static struct drm_display_mode *drm_mode_detailed(struct drm_device *dev, - struct edid *edid, - struct detailed_timing *timing, + const struct edid *edid, + const struct detailed_timing *timing, u32 quirks) { struct drm_display_mode *mode; - struct detailed_pixel_timing *pt = &timing->data.pixel_data; + const struct detailed_pixel_timing *pt = &timing->data.pixel_data; unsigned hactive = (pt->hactive_hblank_hi & 0xf0) << 4 | pt->hactive_lo; unsigned vactive = (pt->vactive_vblank_hi & 0xf0) << 4 | pt->vactive_lo; unsigned hblank = (pt->hactive_hblank_hi & 0xf) << 8 | pt->hblank_lo; @@ -2590,9 +2615,9 @@ static struct drm_display_mode *drm_mode_detailed(struct drm_device *dev, return NULL;
if (quirks & EDID_QUIRK_135_CLOCK_TOO_HIGH) - timing->pixel_clock = cpu_to_le16(1088); - - mode->clock = le16_to_cpu(timing->pixel_clock) * 10; + mode->clock = 1088 * 10; + else + mode->clock = le16_to_cpu(timing->pixel_clock) * 10;
mode->hdisplay = hactive; mode->hsync_start = mode->hdisplay + hsync_offset; @@ -2613,14 +2638,14 @@ static struct drm_display_mode *drm_mode_detailed(struct drm_device *dev, drm_mode_do_interlace_quirk(mode, pt);
if (quirks & EDID_QUIRK_DETAILED_SYNC_PP) { - pt->misc |= DRM_EDID_PT_HSYNC_POSITIVE | DRM_EDID_PT_VSYNC_POSITIVE; + mode->flags |= DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_NVSYNC; + } else { + mode->flags |= (pt->misc & DRM_EDID_PT_HSYNC_POSITIVE) ? + DRM_MODE_FLAG_PHSYNC : DRM_MODE_FLAG_NHSYNC; + mode->flags |= (pt->misc & DRM_EDID_PT_VSYNC_POSITIVE) ? + DRM_MODE_FLAG_PVSYNC : DRM_MODE_FLAG_NVSYNC; }
- mode->flags |= (pt->misc & DRM_EDID_PT_HSYNC_POSITIVE) ? - DRM_MODE_FLAG_PHSYNC : DRM_MODE_FLAG_NHSYNC; - mode->flags |= (pt->misc & DRM_EDID_PT_VSYNC_POSITIVE) ? - DRM_MODE_FLAG_PVSYNC : DRM_MODE_FLAG_NVSYNC; - set_size: mode->width_mm = pt->width_mm_lo | (pt->width_height_mm_hi & 0xf0) << 4; mode->height_mm = pt->height_mm_lo | (pt->width_height_mm_hi & 0xf) << 8; @@ -2644,7 +2669,7 @@ static struct drm_display_mode *drm_mode_detailed(struct drm_device *dev,
static bool mode_in_hsync_range(const struct drm_display_mode *mode, - struct edid *edid, u8 *t) + const struct edid *edid, const u8 *t) { int hsync, hmin, hmax;
@@ -2661,7 +2686,7 @@ mode_in_hsync_range(const struct drm_display_mode *mode,
static bool mode_in_vsync_range(const struct drm_display_mode *mode, - struct edid *edid, u8 *t) + const struct edid *edid, const u8 *t) { int vsync, vmin, vmax;
@@ -2677,7 +2702,7 @@ mode_in_vsync_range(const struct drm_display_mode *mode, }
static u32 -range_pixel_clock(struct edid *edid, u8 *t) +range_pixel_clock(const struct edid *edid, const u8 *t) { /* unspecified */ if (t[9] == 0 || t[9] == 255) @@ -2692,11 +2717,12 @@ range_pixel_clock(struct edid *edid, u8 *t) }
static bool -mode_in_range(const struct drm_display_mode *mode, struct edid *edid, - struct detailed_timing *timing) +mode_in_range(const struct drm_display_mode *mode, + const struct edid *edid, + const struct detailed_timing *timing) { u32 max_clock; - u8 *t = (u8 *)timing; + const u8 *t = (const u8 *)timing;
if (!mode_in_hsync_range(mode, edid, t)) return false; @@ -2738,8 +2764,9 @@ static bool valid_inferred_mode(const struct drm_connector *connector, }
static int -drm_dmt_modes_for_range(struct drm_connector *connector, struct edid *edid, - struct detailed_timing *timing) +drm_dmt_modes_for_range(struct drm_connector *connector, + const struct edid *edid, + const struct detailed_timing *timing) { int i, modes = 0; struct drm_display_mode *newmode; @@ -2773,8 +2800,9 @@ void drm_mode_fixup_1366x768(struct drm_display_mode *mode) }
static int -drm_gtf_modes_for_range(struct drm_connector *connector, struct edid *edid, - struct detailed_timing *timing) +drm_gtf_modes_for_range(struct drm_connector *connector, + const struct edid *edid, + const struct detailed_timing *timing) { int i, modes = 0; struct drm_display_mode *newmode; @@ -2801,8 +2829,9 @@ drm_gtf_modes_for_range(struct drm_connector *connector, struct edid *edid, }
static int -drm_cvt_modes_for_range(struct drm_connector *connector, struct edid *edid, - struct detailed_timing *timing) +drm_cvt_modes_for_range(struct drm_connector *connector, + const struct edid *edid, + const struct detailed_timing *timing) { int i, modes = 0; struct drm_display_mode *newmode; @@ -2830,11 +2859,11 @@ drm_cvt_modes_for_range(struct drm_connector *connector, struct edid *edid, }
static void -do_inferred_modes(struct detailed_timing *timing, void *c) +do_inferred_modes(const struct detailed_timing *timing, void *c) { struct detailed_mode_closure *closure = c; - struct detailed_non_pixel *data = &timing->data.other_data; - struct detailed_data_monitor_range *range = &data->data.range; + 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)) return; @@ -2868,7 +2897,8 @@ do_inferred_modes(struct detailed_timing *timing, void *c) }
static int -add_inferred_modes(struct drm_connector *connector, struct edid *edid) +add_inferred_modes(struct drm_connector *connector, + const struct edid *edid) { struct detailed_mode_closure closure = { .connector = connector, @@ -2876,18 +2906,20 @@ add_inferred_modes(struct drm_connector *connector, struct edid *edid) };
if (version_greater(edid, 1, 0)) - drm_for_each_detailed_block((u8 *)edid, do_inferred_modes, + drm_for_each_detailed_block((const u8 *)edid, + do_inferred_modes, &closure);
return closure.modes; }
static int -drm_est3_modes(struct drm_connector *connector, struct detailed_timing *timing) +drm_est3_modes(struct drm_connector *connector, + const struct detailed_timing *timing) { int i, j, m, modes = 0; struct drm_display_mode *mode; - u8 *est = ((u8 *)timing) + 6; + const u8 *est = ((const u8 *)timing) + 6;
for (i = 0; i < 6; i++) { for (j = 7; j >= 0; j--) { @@ -2912,7 +2944,7 @@ drm_est3_modes(struct drm_connector *connector, struct detailed_timing *timing) }
static void -do_established_modes(struct detailed_timing *timing, void *c) +do_established_modes(const struct detailed_timing *timing, void *c) { struct detailed_mode_closure *closure = c;
@@ -2931,7 +2963,8 @@ do_established_modes(struct detailed_timing *timing, void *c) * (defined above). Tease them out and add them to the global modes list. */ static int -add_established_modes(struct drm_connector *connector, struct edid *edid) +add_established_modes(struct drm_connector *connector, + const struct edid *edid) { struct drm_device *dev = connector->dev; unsigned long est_bits = edid->established_timings.t1 | @@ -2962,19 +2995,19 @@ add_established_modes(struct drm_connector *connector, struct edid *edid) }
static void -do_standard_modes(struct detailed_timing *timing, void *c) +do_standard_modes(const struct detailed_timing *timing, void *c) { struct detailed_mode_closure *closure = c; - struct detailed_non_pixel *data = &timing->data.other_data; + const struct detailed_non_pixel *data = &timing->data.other_data; struct drm_connector *connector = closure->connector; - struct edid *edid = closure->edid; + const struct edid *edid = closure->edid; int i;
if (!is_display_descriptor((const u8 *)timing, EDID_DETAIL_STD_MODES)) return;
for (i = 0; i < 6; i++) { - struct std_timing *std = &data->data.timings[i]; + const struct std_timing *std = &data->data.timings[i]; struct drm_display_mode *newmode;
newmode = drm_mode_std(connector, edid, std); @@ -2994,7 +3027,8 @@ do_standard_modes(struct detailed_timing *timing, void *c) * GTF or CVT. Grab them from @edid and add them to the list. */ static int -add_standard_modes(struct drm_connector *connector, struct edid *edid) +add_standard_modes(struct drm_connector *connector, + const struct edid *edid) { int i, modes = 0; struct detailed_mode_closure closure = { @@ -3023,18 +3057,18 @@ add_standard_modes(struct drm_connector *connector, struct edid *edid) }
static int drm_cvt_modes(struct drm_connector *connector, - struct detailed_timing *timing) + const struct detailed_timing *timing) { int i, j, modes = 0; struct drm_display_mode *newmode; struct drm_device *dev = connector->dev; - struct cvt_timing *cvt; const int rates[] = { 60, 85, 75, 60, 50 }; const u8 empty[3] = { 0, 0, 0 };
for (i = 0; i < 4; i++) { int uninitialized_var(width), height; - cvt = &(timing->data.other_data.data.cvt[i]); + const struct cvt_timing *cvt = + &timing->data.other_data.data.cvt[i];
if (!memcmp(cvt->code, empty, 3)) continue; @@ -3072,7 +3106,7 @@ static int drm_cvt_modes(struct drm_connector *connector, }
static void -do_cvt_mode(struct detailed_timing *timing, void *c) +do_cvt_mode(const struct detailed_timing *timing, void *c) { struct detailed_mode_closure *closure = c;
@@ -3083,7 +3117,8 @@ do_cvt_mode(struct detailed_timing *timing, void *c) }
static int -add_cvt_modes(struct drm_connector *connector, struct edid *edid) +add_cvt_modes(struct drm_connector *connector, + const struct edid *edid) { struct detailed_mode_closure closure = { .connector = connector, @@ -3101,7 +3136,7 @@ add_cvt_modes(struct drm_connector *connector, struct edid *edid) static void fixup_detailed_cea_mode_clock(struct drm_display_mode *mode);
static void -do_detailed_mode(struct detailed_timing *timing, void *c) +do_detailed_mode(const struct detailed_timing *timing, void *c) { struct detailed_mode_closure *closure = c; struct drm_display_mode *newmode; @@ -3137,8 +3172,8 @@ do_detailed_mode(struct detailed_timing *timing, void *c) * @quirks: quirks to apply */ static int -add_detailed_modes(struct drm_connector *connector, struct edid *edid, - u32 quirks) +add_detailed_modes(struct drm_connector *connector, + struct edid *edid, u32 quirks) { struct detailed_mode_closure closure = { .connector = connector, @@ -3173,9 +3208,10 @@ add_detailed_modes(struct drm_connector *connector, struct edid *edid, /* * Search EDID for CEA extension block. */ -static u8 *drm_find_edid_extension(const struct edid *edid, int ext_id) +static const u8 *drm_find_edid_extension(const struct edid *edid, + int ext_id) { - u8 *edid_ext = NULL; + const u8 *edid_ext = NULL; int i;
/* No EDID or EDID extensions */ @@ -3184,7 +3220,7 @@ static u8 *drm_find_edid_extension(const struct edid *edid, int ext_id)
/* Find CEA extension */ for (i = 0; i < edid->extensions; i++) { - edid_ext = (u8 *)edid + EDID_LENGTH * (i + 1); + edid_ext = (const u8 *)edid + EDID_LENGTH * (i + 1); if (edid_ext[0] == ext_id) break; } @@ -3196,19 +3232,19 @@ static u8 *drm_find_edid_extension(const struct edid *edid, int ext_id) }
-static u8 *drm_find_displayid_extension(const struct edid *edid) +static const u8 *drm_find_displayid_extension(const struct edid *edid) { return drm_find_edid_extension(edid, DISPLAYID_EXT); }
-static u8 *drm_find_cea_extension(const struct edid *edid) +static const u8 *drm_find_cea_extension(const struct edid *edid) { int ret; int idx = 1; int length = EDID_LENGTH; - struct displayid_block *block; - u8 *cea; - u8 *displayid; + const struct displayid_block *block; + const u8 *cea; + const u8 *displayid;
/* Look for a top level CEA extension block */ cea = drm_find_edid_extension(edid, CEA_EXT); @@ -4315,28 +4351,30 @@ drm_parse_hdmi_vsdb_audio(struct drm_connector *connector, const u8 *db) }
static void -monitor_name(struct detailed_timing *t, void *data) +monitor_name(const struct detailed_timing *t, void *c) { + struct data_closure *closure = c; + if (!is_display_descriptor((const u8 *)t, EDID_DETAIL_MONITOR_NAME)) return;
- *(u8 **)data = t->data.other_data.data.str.str; + closure->data = t->data.other_data.data.str.str; }
static int get_monitor_name(struct edid *edid, char name[13]) { - char *edid_name = NULL; + struct data_closure closure = {}; int mnl;
if (!edid || !name) return 0;
- drm_for_each_detailed_block((u8 *)edid, monitor_name, &edid_name); - for (mnl = 0; edid_name && mnl < 13; mnl++) { - if (edid_name[mnl] == 0x0a) + drm_for_each_detailed_block((const u8 *)edid, monitor_name, &closure); + for (mnl = 0; closure.data && mnl < 13; mnl++) { + if (closure.data[mnl] == 0x0a) break;
- name[mnl] = edid_name[mnl]; + name[mnl] = closure.data[mnl]; }
return mnl; @@ -4386,11 +4424,9 @@ static void clear_eld(struct drm_connector *connector) static void drm_edid_to_eld(struct drm_connector *connector, struct edid *edid) { uint8_t *eld = connector->eld; - u8 *cea; - u8 *db; + const u8 *cea; int total_sad_count = 0; int mnl; - int dbl;
clear_eld(connector);
@@ -4425,8 +4461,8 @@ static void drm_edid_to_eld(struct drm_connector *connector, struct edid *edid) }
for_each_cea_db(cea, i, start, end) { - db = &cea[i]; - dbl = cea_db_payload_len(db); + const u8 *db = &cea[i]; + int dbl = cea_db_payload_len(db);
switch (cea_db_tag(db)) { int sad_count; @@ -4484,7 +4520,7 @@ int drm_edid_to_sad(struct edid *edid, struct cea_sad **sads) { int count = 0; int i, start, end, dbl; - u8 *cea; + const u8 *cea;
cea = drm_find_cea_extension(edid); if (!cea) { @@ -4503,7 +4539,7 @@ int drm_edid_to_sad(struct edid *edid, struct cea_sad **sads) }
for_each_cea_db(cea, i, start, end) { - u8 *db = &cea[i]; + const u8 *db = &cea[i];
if (cea_db_tag(db) == AUDIO_BLOCK) { int j; @@ -4514,7 +4550,7 @@ int drm_edid_to_sad(struct edid *edid, struct cea_sad **sads) if (!*sads) return -ENOMEM; for (j = 0; j < count; j++) { - u8 *sad = &db[1 + j * 3]; + const u8 *sad = &db[1 + j * 3];
(*sads)[j].format = (sad[0] & 0x78) >> 3; (*sads)[j].channels = sad[0] & 0x7; @@ -4635,7 +4671,7 @@ EXPORT_SYMBOL(drm_av_sync_delay); */ bool drm_detect_hdmi_monitor(struct edid *edid) { - u8 *edid_ext; + const u8 *edid_ext; int i; int start_offset, end_offset;
@@ -4673,7 +4709,7 @@ EXPORT_SYMBOL(drm_detect_hdmi_monitor); */ bool drm_detect_monitor_audio(struct edid *edid) { - u8 *edid_ext; + const u8 *edid_ext; int i, j; bool has_audio = false; int start_offset, end_offset; @@ -5017,13 +5053,13 @@ u32 drm_add_display_info(struct drm_connector *connector, const struct edid *edi return quirks; }
-static int validate_displayid(u8 *displayid, int length, int idx) +static int validate_displayid(const u8 *displayid, int length, int idx) { int i; u8 csum = 0; - struct displayid_hdr *base; + const struct displayid_hdr *base;
- base = (struct displayid_hdr *)&displayid[idx]; + base = (const struct displayid_hdr *)&displayid[idx];
DRM_DEBUG_KMS("base revision 0x%x, length %d, %d %d\n", base->rev, base->bytes, base->prod_id, base->ext_count); @@ -5041,7 +5077,7 @@ static int validate_displayid(u8 *displayid, int length, int idx) }
static struct drm_display_mode *drm_mode_displayid_detailed(struct drm_device *dev, - struct displayid_detailed_timings_1 *timings) + const struct displayid_detailed_timings_1 *timings) { struct drm_display_mode *mode; unsigned pixel_clock = (timings->pixel_clock[0] | @@ -5057,6 +5093,7 @@ static struct drm_display_mode *drm_mode_displayid_detailed(struct drm_device *d unsigned vsync_width = (timings->vsw[0] | timings->vsw[1] << 8) + 1; bool hsync_positive = (timings->hsync[1] >> 7) & 0x1; bool vsync_positive = (timings->vsync[1] >> 7) & 0x1; + mode = drm_mode_create(dev); if (!mode) return NULL; @@ -5086,9 +5123,10 @@ static struct drm_display_mode *drm_mode_displayid_detailed(struct drm_device *d }
static int add_displayid_detailed_1_modes(struct drm_connector *connector, - struct displayid_block *block) + const struct displayid_block *block) { - struct displayid_detailed_timing_block *det = (struct displayid_detailed_timing_block *)block; + const struct displayid_detailed_timing_block *det = + (const struct displayid_detailed_timing_block *)block; int i; int num_timings; struct drm_display_mode *newmode; @@ -5099,7 +5137,7 @@ static int add_displayid_detailed_1_modes(struct drm_connector *connector,
num_timings = block->num_bytes / 20; for (i = 0; i < num_timings; i++) { - struct displayid_detailed_timings_1 *timings = &det->timings[i]; + const struct displayid_detailed_timings_1 *timings = &det->timings[i];
newmode = drm_mode_displayid_detailed(connector->dev, timings); if (!newmode) @@ -5112,13 +5150,13 @@ static int add_displayid_detailed_1_modes(struct drm_connector *connector, }
static int add_displayid_detailed_modes(struct drm_connector *connector, - struct edid *edid) + const struct edid *edid) { - u8 *displayid; + const u8 *displayid; int ret; int idx = 1; int length = EDID_LENGTH; - struct displayid_block *block; + const struct displayid_block *block; int num_modes = 0;
displayid = drm_find_displayid_extension(edid); @@ -5720,9 +5758,10 @@ drm_hdmi_vendor_infoframe_from_display_mode(struct hdmi_vendor_infoframe *frame, EXPORT_SYMBOL(drm_hdmi_vendor_infoframe_from_display_mode);
static int drm_parse_tiled_block(struct drm_connector *connector, - struct displayid_block *block) + const struct displayid_block *block) { - struct displayid_tiled_block *tile = (struct displayid_tiled_block *)block; + const struct displayid_tiled_block *tile = + (const struct displayid_tiled_block *)block; u16 w, h; u8 tile_v_loc, tile_h_loc; u8 num_v_tile, num_h_tile; @@ -5774,12 +5813,12 @@ static int drm_parse_tiled_block(struct drm_connector *connector, }
static int drm_parse_display_id(struct drm_connector *connector, - u8 *displayid, int length, + const u8 *displayid, int length, bool is_edid_extension) { /* if this is an EDID extension the first byte will be 0x70 */ int idx = 0; - struct displayid_block *block; + const struct displayid_block *block; int ret;
if (is_edid_extension) @@ -5815,11 +5854,13 @@ static int drm_parse_display_id(struct drm_connector *connector, }
static void drm_get_displayid(struct drm_connector *connector, - struct edid *edid) + const struct edid *edid) { - void *displayid = NULL; + const void *displayid; int ret; + connector->has_tile = false; + displayid = drm_find_displayid_extension(edid); if (!displayid) { /* drop reference to any tile group we had */ diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h index 2113500b4075..c0f9ce3f4b24 100644 --- a/include/drm/drm_connector.h +++ b/include/drm/drm_connector.h @@ -1580,9 +1580,9 @@ struct drm_tile_group { };
struct drm_tile_group *drm_mode_create_tile_group(struct drm_device *dev, - char topology[8]); + const u8 topology[8]); struct drm_tile_group *drm_mode_get_tile_group(struct drm_device *dev, - char topology[8]); + const u8 topology[8]); void drm_mode_put_tile_group(struct drm_device *dev, struct drm_tile_group *tg);
On Fri, Jan 24, 2020 at 3:03 PM Ville Syrjala ville.syrjala@linux.intel.com wrote:
From: Ville Syrjälä ville.syrjala@linux.intel.com
Let's try to make a lot more stuff const in the edid parser.
The "downside" is that we can no longer mangle the EDID in the middle of the parsing to apply quirks (drm_mode_detailed()). I don't really think mangling the blob itself is such a great idea anyway so I won't miss that part. But if we do want it back I guess we should do the mangling in one explicit place before we otherwise parse the EDID.
Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com
I generally agree, but are there any userspace expectations that they will be getting a corrected EDID in some cases?
Alex
drivers/gpu/drm/drm_connector.c | 4 +- drivers/gpu/drm/drm_edid.c | 303 ++++++++++++++++++-------------- include/drm/drm_connector.h | 4 +- 3 files changed, 176 insertions(+), 135 deletions(-)
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c index f632ca05960e..92a5cd6ff6b1 100644 --- a/drivers/gpu/drm/drm_connector.c +++ b/drivers/gpu/drm/drm_connector.c @@ -2377,7 +2377,7 @@ EXPORT_SYMBOL(drm_mode_put_tile_group);
- tile group or NULL if not found.
*/ struct drm_tile_group *drm_mode_get_tile_group(struct drm_device *dev,
char topology[8])
const u8 topology[8]){ struct drm_tile_group *tg; int id; @@ -2407,7 +2407,7 @@ EXPORT_SYMBOL(drm_mode_get_tile_group);
- new tile group or NULL.
*/ struct drm_tile_group *drm_mode_create_tile_group(struct drm_device *dev,
char topology[8])
const u8 topology[8]){ struct drm_tile_group *tg; int ret; diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index fd9b724067a7..8e76efe1654d 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -88,7 +88,7 @@
struct detailed_mode_closure { struct drm_connector *connector;
struct edid *edid;
const struct edid *edid; bool preferred; u32 quirks; int modes;@@ -1584,8 +1584,8 @@ MODULE_PARM_DESC(edid_fixup, "Minimum number of valid EDID header bytes (0-8, default 6)");
static void drm_get_displayid(struct drm_connector *connector,
struct edid *edid);-static int validate_displayid(u8 *displayid, int length, int idx);
const struct edid *edid);+static int validate_displayid(const u8 *displayid, int length, int idx);
static int drm_edid_block_checksum(const u8 *raw_edid) { @@ -2207,41 +2207,41 @@ static bool is_detailed_timing_descriptor(const u8 d[18]) return d[0] != 0x00 || d[1] != 0x00; }
-typedef void detailed_cb(struct detailed_timing *timing, void *closure); +typedef void detailed_cb(const struct detailed_timing *timing, void *closure);
static void -cea_for_each_detailed_block(u8 *ext, detailed_cb *cb, void *closure) +cea_for_each_detailed_block(const u8 *ext, detailed_cb *cb, void *closure) { int i, n; u8 d = ext[0x02];
u8 *det_base = ext + d;
const u8 *det_base = ext + d; if (d < 4 || d > 127) return; n = (127 - d) / 18; for (i = 0; i < n; i++)
cb((struct detailed_timing *)(det_base + 18 * i), closure);
cb((const struct detailed_timing *)(det_base + 18 * i), closure);}
static void -vtb_for_each_detailed_block(u8 *ext, detailed_cb *cb, void *closure) +vtb_for_each_detailed_block(const u8 *ext, detailed_cb *cb, void *closure) { unsigned int i, n = min((int)ext[0x02], 6);
u8 *det_base = ext + 5;
const u8 *det_base = ext + 5; if (ext[0x01] != 1) return; /* unknown version */ for (i = 0; i < n; i++)
cb((struct detailed_timing *)(det_base + 18 * i), closure);
cb((const struct detailed_timing *)(det_base + 18 * i), closure);}
static void -drm_for_each_detailed_block(u8 *raw_edid, detailed_cb *cb, void *closure) +drm_for_each_detailed_block(const u8 *raw_edid, detailed_cb *cb, void *closure) {
const struct edid *edid = (struct edid *)raw_edid; int i;
struct edid *edid = (struct edid *)raw_edid; if (edid == NULL) return;@@ -2250,7 +2250,7 @@ drm_for_each_detailed_block(u8 *raw_edid, detailed_cb *cb, void *closure) cb(&(edid->detailed_timings[i]), closure);
for (i = 1; i <= raw_edid[0x7e]; i++) {
u8 *ext = raw_edid + (i * EDID_LENGTH);
const u8 *ext = raw_edid + (i * EDID_LENGTH); switch (*ext) { case CEA_EXT: cea_for_each_detailed_block(ext, cb, closure);@@ -2264,81 +2264,105 @@ drm_for_each_detailed_block(u8 *raw_edid, detailed_cb *cb, void *closure) } }
+struct bool_closure {
bool ret;+};
static void -is_rb(struct detailed_timing *t, void *data) +is_rb(const struct detailed_timing *t, void *c) {
u8 *r = (u8 *)t;
struct bool_closure *closure = c;const u8 *r = (const u8 *)t; if (!is_display_descriptor(r, EDID_DETAIL_MONITOR_RANGE)) return; if (r[15] & 0x10)
*(bool *)data = true;
closure->ret = true;}
/* EDID 1.4 defines this explicitly. For EDID 1.3, we guess, badly. */ static bool -drm_monitor_supports_rb(struct edid *edid) +drm_monitor_supports_rb(const struct edid *edid) { if (edid->revision >= 4) {
bool ret = false;drm_for_each_detailed_block((u8 *)edid, is_rb, &ret);return ret;
struct bool_closure closure = {.ret = false,};drm_for_each_detailed_block((u8 *)edid, is_rb, &closure);return closure.ret; } return ((edid->input & DRM_EDID_INPUT_DIGITAL) != 0);}
+struct data_closure {
const u8 *data;+};
static void -find_gtf2(struct detailed_timing *t, void *data) +do_find_gtf2(const struct detailed_timing *t, void *c) {
u8 *r = (u8 *)t;
struct data_closure *closure = c;const u8 *r = (const u8 *)t; if (!is_display_descriptor(r, EDID_DETAIL_MONITOR_RANGE)) return; if (r[10] == 0x02)
*(u8 **)data = r;
closure->data = r;+}
+static const u8 * +find_gtf2_descriptor(const struct edid *edid) +{
struct data_closure closure = {};drm_for_each_detailed_block((u8 *)edid, do_find_gtf2, &closure);return closure.data;}
/* Secondary GTF curve kicks in above some break frequency */ static int -drm_gtf2_hbreak(struct edid *edid) +drm_gtf2_hbreak(const struct edid *edid) {
u8 *r = NULL;drm_for_each_detailed_block((u8 *)edid, find_gtf2, &r);return r ? (r[12] * 2) : 0;
const u8 *r = find_gtf2_descriptor(edid);return r ? r[12] * 2 : 0;}
static int -drm_gtf2_2c(struct edid *edid) +drm_gtf2_2c(const struct edid *edid) {
u8 *r = NULL;drm_for_each_detailed_block((u8 *)edid, find_gtf2, &r);
const u8 *r = find_gtf2_descriptor(edid);return r ? r[13] : 0;}
static int -drm_gtf2_m(struct edid *edid) +drm_gtf2_m(const struct edid *edid) {
u8 *r = NULL;drm_for_each_detailed_block((u8 *)edid, find_gtf2, &r);return r ? (r[15] << 8) + r[14] : 0;
const u8 *r = find_gtf2_descriptor(edid);return r ? (r[15] << 8) | r[14] : 0;}
static int -drm_gtf2_k(struct edid *edid) +drm_gtf2_k(const struct edid *edid) {
u8 *r = NULL;drm_for_each_detailed_block((u8 *)edid, find_gtf2, &r);
const u8 *r = find_gtf2_descriptor(edid);return r ? r[16] : 0;}
static int -drm_gtf2_2j(struct edid *edid) +drm_gtf2_2j(const struct edid *edid) {
u8 *r = NULL;drm_for_each_detailed_block((u8 *)edid, find_gtf2, &r);
const u8 *r = find_gtf2_descriptor(edid);return r ? r[17] : 0;}
@@ -2346,7 +2370,7 @@ drm_gtf2_2j(struct edid *edid)
- standard_timing_level - get std. timing level(CVT/GTF/DMT)
- @edid: EDID block to scan
*/ -static int standard_timing_level(struct edid *edid) +static int standard_timing_level(const struct edid *edid) { if (edid->revision >= 2) { if (edid->revision >= 4 && (edid->features & DRM_EDID_FEATURE_DEFAULT_GTF)) @@ -2381,8 +2405,9 @@ bad_std_timing(u8 a, u8 b)
- and convert them into a real mode using CVT/GTF/DMT.
*/ static struct drm_display_mode * -drm_mode_std(struct drm_connector *connector, struct edid *edid,
struct std_timing *t)+drm_mode_std(struct drm_connector *connector,
const struct edid *edid,const struct std_timing *t){ struct drm_device *dev = connector->dev; struct drm_display_mode *m, *mode = NULL; @@ -2500,7 +2525,7 @@ drm_mode_std(struct drm_connector *connector, struct edid *edid, */ static void drm_mode_do_interlace_quirk(struct drm_display_mode *mode,
struct detailed_pixel_timing *pt)
const struct detailed_pixel_timing *pt){ int i; static const struct { @@ -2543,12 +2568,12 @@ drm_mode_do_interlace_quirk(struct drm_display_mode *mode,
- return a new struct drm_display_mode.
*/ static struct drm_display_mode *drm_mode_detailed(struct drm_device *dev,
struct edid *edid,struct detailed_timing *timing,
const struct edid *edid,const struct detailed_timing *timing, u32 quirks){ struct drm_display_mode *mode;
struct detailed_pixel_timing *pt = &timing->data.pixel_data;
const struct detailed_pixel_timing *pt = &timing->data.pixel_data; unsigned hactive = (pt->hactive_hblank_hi & 0xf0) << 4 | pt->hactive_lo; unsigned vactive = (pt->vactive_vblank_hi & 0xf0) << 4 | pt->vactive_lo; unsigned hblank = (pt->hactive_hblank_hi & 0xf) << 8 | pt->hblank_lo;@@ -2590,9 +2615,9 @@ static struct drm_display_mode *drm_mode_detailed(struct drm_device *dev, return NULL;
if (quirks & EDID_QUIRK_135_CLOCK_TOO_HIGH)
timing->pixel_clock = cpu_to_le16(1088);mode->clock = le16_to_cpu(timing->pixel_clock) * 10;
mode->clock = 1088 * 10;elsemode->clock = le16_to_cpu(timing->pixel_clock) * 10; mode->hdisplay = hactive; mode->hsync_start = mode->hdisplay + hsync_offset;@@ -2613,14 +2638,14 @@ static struct drm_display_mode *drm_mode_detailed(struct drm_device *dev, drm_mode_do_interlace_quirk(mode, pt);
if (quirks & EDID_QUIRK_DETAILED_SYNC_PP) {
pt->misc |= DRM_EDID_PT_HSYNC_POSITIVE | DRM_EDID_PT_VSYNC_POSITIVE;
mode->flags |= DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_NVSYNC;} else {mode->flags |= (pt->misc & DRM_EDID_PT_HSYNC_POSITIVE) ?DRM_MODE_FLAG_PHSYNC : DRM_MODE_FLAG_NHSYNC;mode->flags |= (pt->misc & DRM_EDID_PT_VSYNC_POSITIVE) ?DRM_MODE_FLAG_PVSYNC : DRM_MODE_FLAG_NVSYNC; }
mode->flags |= (pt->misc & DRM_EDID_PT_HSYNC_POSITIVE) ?DRM_MODE_FLAG_PHSYNC : DRM_MODE_FLAG_NHSYNC;mode->flags |= (pt->misc & DRM_EDID_PT_VSYNC_POSITIVE) ?DRM_MODE_FLAG_PVSYNC : DRM_MODE_FLAG_NVSYNC;set_size: mode->width_mm = pt->width_mm_lo | (pt->width_height_mm_hi & 0xf0) << 4; mode->height_mm = pt->height_mm_lo | (pt->width_height_mm_hi & 0xf) << 8; @@ -2644,7 +2669,7 @@ static struct drm_display_mode *drm_mode_detailed(struct drm_device *dev,
static bool mode_in_hsync_range(const struct drm_display_mode *mode,
struct edid *edid, u8 *t)
const struct edid *edid, const u8 *t){ int hsync, hmin, hmax;
@@ -2661,7 +2686,7 @@ mode_in_hsync_range(const struct drm_display_mode *mode,
static bool mode_in_vsync_range(const struct drm_display_mode *mode,
struct edid *edid, u8 *t)
const struct edid *edid, const u8 *t){ int vsync, vmin, vmax;
@@ -2677,7 +2702,7 @@ mode_in_vsync_range(const struct drm_display_mode *mode, }
static u32 -range_pixel_clock(struct edid *edid, u8 *t) +range_pixel_clock(const struct edid *edid, const u8 *t) { /* unspecified */ if (t[9] == 0 || t[9] == 255) @@ -2692,11 +2717,12 @@ range_pixel_clock(struct edid *edid, u8 *t) }
static bool -mode_in_range(const struct drm_display_mode *mode, struct edid *edid,
struct detailed_timing *timing)+mode_in_range(const struct drm_display_mode *mode,
const struct edid *edid,const struct detailed_timing *timing){ u32 max_clock;
u8 *t = (u8 *)timing;
const u8 *t = (const u8 *)timing; if (!mode_in_hsync_range(mode, edid, t)) return false;@@ -2738,8 +2764,9 @@ static bool valid_inferred_mode(const struct drm_connector *connector, }
static int -drm_dmt_modes_for_range(struct drm_connector *connector, struct edid *edid,
struct detailed_timing *timing)+drm_dmt_modes_for_range(struct drm_connector *connector,
const struct edid *edid,const struct detailed_timing *timing){ int i, modes = 0; struct drm_display_mode *newmode; @@ -2773,8 +2800,9 @@ void drm_mode_fixup_1366x768(struct drm_display_mode *mode) }
static int -drm_gtf_modes_for_range(struct drm_connector *connector, struct edid *edid,
struct detailed_timing *timing)+drm_gtf_modes_for_range(struct drm_connector *connector,
const struct edid *edid,const struct detailed_timing *timing){ int i, modes = 0; struct drm_display_mode *newmode; @@ -2801,8 +2829,9 @@ drm_gtf_modes_for_range(struct drm_connector *connector, struct edid *edid, }
static int -drm_cvt_modes_for_range(struct drm_connector *connector, struct edid *edid,
struct detailed_timing *timing)+drm_cvt_modes_for_range(struct drm_connector *connector,
const struct edid *edid,const struct detailed_timing *timing){ int i, modes = 0; struct drm_display_mode *newmode; @@ -2830,11 +2859,11 @@ drm_cvt_modes_for_range(struct drm_connector *connector, struct edid *edid, }
static void -do_inferred_modes(struct detailed_timing *timing, void *c) +do_inferred_modes(const struct detailed_timing *timing, void *c) { struct detailed_mode_closure *closure = c;
struct detailed_non_pixel *data = &timing->data.other_data;struct detailed_data_monitor_range *range = &data->data.range;
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)) return;@@ -2868,7 +2897,8 @@ do_inferred_modes(struct detailed_timing *timing, void *c) }
static int -add_inferred_modes(struct drm_connector *connector, struct edid *edid) +add_inferred_modes(struct drm_connector *connector,
const struct edid *edid){ struct detailed_mode_closure closure = { .connector = connector, @@ -2876,18 +2906,20 @@ add_inferred_modes(struct drm_connector *connector, struct edid *edid) };
if (version_greater(edid, 1, 0))
drm_for_each_detailed_block((u8 *)edid, do_inferred_modes,
drm_for_each_detailed_block((const u8 *)edid,do_inferred_modes, &closure); return closure.modes;}
static int -drm_est3_modes(struct drm_connector *connector, struct detailed_timing *timing) +drm_est3_modes(struct drm_connector *connector,
const struct detailed_timing *timing){ int i, j, m, modes = 0; struct drm_display_mode *mode;
u8 *est = ((u8 *)timing) + 6;
const u8 *est = ((const u8 *)timing) + 6; for (i = 0; i < 6; i++) { for (j = 7; j >= 0; j--) {@@ -2912,7 +2944,7 @@ drm_est3_modes(struct drm_connector *connector, struct detailed_timing *timing) }
static void -do_established_modes(struct detailed_timing *timing, void *c) +do_established_modes(const struct detailed_timing *timing, void *c) { struct detailed_mode_closure *closure = c;
@@ -2931,7 +2963,8 @@ do_established_modes(struct detailed_timing *timing, void *c)
- (defined above). Tease them out and add them to the global modes list.
*/ static int -add_established_modes(struct drm_connector *connector, struct edid *edid) +add_established_modes(struct drm_connector *connector,
const struct edid *edid){ struct drm_device *dev = connector->dev; unsigned long est_bits = edid->established_timings.t1 | @@ -2962,19 +2995,19 @@ add_established_modes(struct drm_connector *connector, struct edid *edid) }
static void -do_standard_modes(struct detailed_timing *timing, void *c) +do_standard_modes(const struct detailed_timing *timing, void *c) { struct detailed_mode_closure *closure = c;
struct detailed_non_pixel *data = &timing->data.other_data;
const struct detailed_non_pixel *data = &timing->data.other_data; struct drm_connector *connector = closure->connector;
struct edid *edid = closure->edid;
const struct edid *edid = closure->edid; int i; if (!is_display_descriptor((const u8 *)timing, EDID_DETAIL_STD_MODES)) return; for (i = 0; i < 6; i++) {
struct std_timing *std = &data->data.timings[i];
const struct std_timing *std = &data->data.timings[i]; struct drm_display_mode *newmode; newmode = drm_mode_std(connector, edid, std);@@ -2994,7 +3027,8 @@ do_standard_modes(struct detailed_timing *timing, void *c)
- GTF or CVT. Grab them from @edid and add them to the list.
*/ static int -add_standard_modes(struct drm_connector *connector, struct edid *edid) +add_standard_modes(struct drm_connector *connector,
const struct edid *edid){ int i, modes = 0; struct detailed_mode_closure closure = { @@ -3023,18 +3057,18 @@ add_standard_modes(struct drm_connector *connector, struct edid *edid) }
static int drm_cvt_modes(struct drm_connector *connector,
struct detailed_timing *timing)
const struct detailed_timing *timing){ int i, j, modes = 0; struct drm_display_mode *newmode; struct drm_device *dev = connector->dev;
struct cvt_timing *cvt; const int rates[] = { 60, 85, 75, 60, 50 }; const u8 empty[3] = { 0, 0, 0 }; for (i = 0; i < 4; i++) { int uninitialized_var(width), height;cvt = &(timing->data.other_data.data.cvt[i]);
const struct cvt_timing *cvt =&timing->data.other_data.data.cvt[i]; if (!memcmp(cvt->code, empty, 3)) continue;@@ -3072,7 +3106,7 @@ static int drm_cvt_modes(struct drm_connector *connector, }
static void -do_cvt_mode(struct detailed_timing *timing, void *c) +do_cvt_mode(const struct detailed_timing *timing, void *c) { struct detailed_mode_closure *closure = c;
@@ -3083,7 +3117,8 @@ do_cvt_mode(struct detailed_timing *timing, void *c) }
static int -add_cvt_modes(struct drm_connector *connector, struct edid *edid) +add_cvt_modes(struct drm_connector *connector,
const struct edid *edid){ struct detailed_mode_closure closure = { .connector = connector, @@ -3101,7 +3136,7 @@ add_cvt_modes(struct drm_connector *connector, struct edid *edid) static void fixup_detailed_cea_mode_clock(struct drm_display_mode *mode);
static void -do_detailed_mode(struct detailed_timing *timing, void *c) +do_detailed_mode(const struct detailed_timing *timing, void *c) { struct detailed_mode_closure *closure = c; struct drm_display_mode *newmode; @@ -3137,8 +3172,8 @@ do_detailed_mode(struct detailed_timing *timing, void *c)
- @quirks: quirks to apply
*/ static int -add_detailed_modes(struct drm_connector *connector, struct edid *edid,
u32 quirks)+add_detailed_modes(struct drm_connector *connector,
struct edid *edid, u32 quirks){ struct detailed_mode_closure closure = { .connector = connector, @@ -3173,9 +3208,10 @@ add_detailed_modes(struct drm_connector *connector, struct edid *edid, /*
- Search EDID for CEA extension block.
*/ -static u8 *drm_find_edid_extension(const struct edid *edid, int ext_id) +static const u8 *drm_find_edid_extension(const struct edid *edid,
int ext_id){
u8 *edid_ext = NULL;
const u8 *edid_ext = NULL; int i; /* No EDID or EDID extensions */@@ -3184,7 +3220,7 @@ static u8 *drm_find_edid_extension(const struct edid *edid, int ext_id)
/* Find CEA extension */ for (i = 0; i < edid->extensions; i++) {
edid_ext = (u8 *)edid + EDID_LENGTH * (i + 1);
edid_ext = (const u8 *)edid + EDID_LENGTH * (i + 1); if (edid_ext[0] == ext_id) break; }@@ -3196,19 +3232,19 @@ static u8 *drm_find_edid_extension(const struct edid *edid, int ext_id) }
-static u8 *drm_find_displayid_extension(const struct edid *edid) +static const u8 *drm_find_displayid_extension(const struct edid *edid) { return drm_find_edid_extension(edid, DISPLAYID_EXT); }
-static u8 *drm_find_cea_extension(const struct edid *edid) +static const u8 *drm_find_cea_extension(const struct edid *edid) { int ret; int idx = 1; int length = EDID_LENGTH;
struct displayid_block *block;u8 *cea;u8 *displayid;
const struct displayid_block *block;const u8 *cea;const u8 *displayid; /* Look for a top level CEA extension block */ cea = drm_find_edid_extension(edid, CEA_EXT);@@ -4315,28 +4351,30 @@ drm_parse_hdmi_vsdb_audio(struct drm_connector *connector, const u8 *db) }
static void -monitor_name(struct detailed_timing *t, void *data) +monitor_name(const struct detailed_timing *t, void *c) {
struct data_closure *closure = c;if (!is_display_descriptor((const u8 *)t, EDID_DETAIL_MONITOR_NAME)) return;
*(u8 **)data = t->data.other_data.data.str.str;
closure->data = t->data.other_data.data.str.str;}
static int get_monitor_name(struct edid *edid, char name[13]) {
char *edid_name = NULL;
struct data_closure closure = {}; int mnl; if (!edid || !name) return 0;
drm_for_each_detailed_block((u8 *)edid, monitor_name, &edid_name);for (mnl = 0; edid_name && mnl < 13; mnl++) {if (edid_name[mnl] == 0x0a)
drm_for_each_detailed_block((const u8 *)edid, monitor_name, &closure);for (mnl = 0; closure.data && mnl < 13; mnl++) {if (closure.data[mnl] == 0x0a) break;
name[mnl] = edid_name[mnl];
name[mnl] = closure.data[mnl]; } return mnl;@@ -4386,11 +4424,9 @@ static void clear_eld(struct drm_connector *connector) static void drm_edid_to_eld(struct drm_connector *connector, struct edid *edid) { uint8_t *eld = connector->eld;
u8 *cea;u8 *db;
const u8 *cea; int total_sad_count = 0; int mnl;
int dbl; clear_eld(connector);@@ -4425,8 +4461,8 @@ static void drm_edid_to_eld(struct drm_connector *connector, struct edid *edid) }
for_each_cea_db(cea, i, start, end) {
db = &cea[i];dbl = cea_db_payload_len(db);
const u8 *db = &cea[i];int dbl = cea_db_payload_len(db); switch (cea_db_tag(db)) { int sad_count;@@ -4484,7 +4520,7 @@ int drm_edid_to_sad(struct edid *edid, struct cea_sad **sads) { int count = 0; int i, start, end, dbl;
u8 *cea;
const u8 *cea; cea = drm_find_cea_extension(edid); if (!cea) {@@ -4503,7 +4539,7 @@ int drm_edid_to_sad(struct edid *edid, struct cea_sad **sads) }
for_each_cea_db(cea, i, start, end) {
u8 *db = &cea[i];
const u8 *db = &cea[i]; if (cea_db_tag(db) == AUDIO_BLOCK) { int j;@@ -4514,7 +4550,7 @@ int drm_edid_to_sad(struct edid *edid, struct cea_sad **sads) if (!*sads) return -ENOMEM; for (j = 0; j < count; j++) {
u8 *sad = &db[1 + j * 3];
const u8 *sad = &db[1 + j * 3]; (*sads)[j].format = (sad[0] & 0x78) >> 3; (*sads)[j].channels = sad[0] & 0x7;@@ -4635,7 +4671,7 @@ EXPORT_SYMBOL(drm_av_sync_delay); */ bool drm_detect_hdmi_monitor(struct edid *edid) {
u8 *edid_ext;
const u8 *edid_ext; int i; int start_offset, end_offset;@@ -4673,7 +4709,7 @@ EXPORT_SYMBOL(drm_detect_hdmi_monitor); */ bool drm_detect_monitor_audio(struct edid *edid) {
u8 *edid_ext;
const u8 *edid_ext; int i, j; bool has_audio = false; int start_offset, end_offset;@@ -5017,13 +5053,13 @@ u32 drm_add_display_info(struct drm_connector *connector, const struct edid *edi return quirks; }
-static int validate_displayid(u8 *displayid, int length, int idx) +static int validate_displayid(const u8 *displayid, int length, int idx) { int i; u8 csum = 0;
struct displayid_hdr *base;
const struct displayid_hdr *base;
base = (struct displayid_hdr *)&displayid[idx];
base = (const struct displayid_hdr *)&displayid[idx]; DRM_DEBUG_KMS("base revision 0x%x, length %d, %d %d\n", base->rev, base->bytes, base->prod_id, base->ext_count);@@ -5041,7 +5077,7 @@ static int validate_displayid(u8 *displayid, int length, int idx) }
static struct drm_display_mode *drm_mode_displayid_detailed(struct drm_device *dev,
struct displayid_detailed_timings_1 *timings)
const struct displayid_detailed_timings_1 *timings){ struct drm_display_mode *mode; unsigned pixel_clock = (timings->pixel_clock[0] | @@ -5057,6 +5093,7 @@ static struct drm_display_mode *drm_mode_displayid_detailed(struct drm_device *d unsigned vsync_width = (timings->vsw[0] | timings->vsw[1] << 8) + 1; bool hsync_positive = (timings->hsync[1] >> 7) & 0x1; bool vsync_positive = (timings->vsync[1] >> 7) & 0x1;
mode = drm_mode_create(dev); if (!mode) return NULL;@@ -5086,9 +5123,10 @@ static struct drm_display_mode *drm_mode_displayid_detailed(struct drm_device *d }
static int add_displayid_detailed_1_modes(struct drm_connector *connector,
struct displayid_block *block)
const struct displayid_block *block){
struct displayid_detailed_timing_block *det = (struct displayid_detailed_timing_block *)block;
const struct displayid_detailed_timing_block *det =(const struct displayid_detailed_timing_block *)block; int i; int num_timings; struct drm_display_mode *newmode;@@ -5099,7 +5137,7 @@ static int add_displayid_detailed_1_modes(struct drm_connector *connector,
num_timings = block->num_bytes / 20; for (i = 0; i < num_timings; i++) {
struct displayid_detailed_timings_1 *timings = &det->timings[i];
const struct displayid_detailed_timings_1 *timings = &det->timings[i]; newmode = drm_mode_displayid_detailed(connector->dev, timings); if (!newmode)@@ -5112,13 +5150,13 @@ static int add_displayid_detailed_1_modes(struct drm_connector *connector, }
static int add_displayid_detailed_modes(struct drm_connector *connector,
struct edid *edid)
const struct edid *edid){
u8 *displayid;
const u8 *displayid; int ret; int idx = 1; int length = EDID_LENGTH;
struct displayid_block *block;
const struct displayid_block *block; int num_modes = 0; displayid = drm_find_displayid_extension(edid);@@ -5720,9 +5758,10 @@ drm_hdmi_vendor_infoframe_from_display_mode(struct hdmi_vendor_infoframe *frame, EXPORT_SYMBOL(drm_hdmi_vendor_infoframe_from_display_mode);
static int drm_parse_tiled_block(struct drm_connector *connector,
struct displayid_block *block)
const struct displayid_block *block){
struct displayid_tiled_block *tile = (struct displayid_tiled_block *)block;
const struct displayid_tiled_block *tile =(const struct displayid_tiled_block *)block; u16 w, h; u8 tile_v_loc, tile_h_loc; u8 num_v_tile, num_h_tile;@@ -5774,12 +5813,12 @@ static int drm_parse_tiled_block(struct drm_connector *connector, }
static int drm_parse_display_id(struct drm_connector *connector,
u8 *displayid, int length,
const u8 *displayid, int length, bool is_edid_extension){ /* if this is an EDID extension the first byte will be 0x70 */ int idx = 0;
struct displayid_block *block;
const struct displayid_block *block; int ret; if (is_edid_extension)@@ -5815,11 +5854,13 @@ static int drm_parse_display_id(struct drm_connector *connector, }
static void drm_get_displayid(struct drm_connector *connector,
struct edid *edid)
const struct edid *edid){
void *displayid = NULL;
const void *displayid; int ret;connector->has_tile = false;displayid = drm_find_displayid_extension(edid); if (!displayid) { /* drop reference to any tile group we had */diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h index 2113500b4075..c0f9ce3f4b24 100644 --- a/include/drm/drm_connector.h +++ b/include/drm/drm_connector.h @@ -1580,9 +1580,9 @@ struct drm_tile_group { };
struct drm_tile_group *drm_mode_create_tile_group(struct drm_device *dev,
char topology[8]);
const u8 topology[8]);struct drm_tile_group *drm_mode_get_tile_group(struct drm_device *dev,
char topology[8]);
const u8 topology[8]);void drm_mode_put_tile_group(struct drm_device *dev, struct drm_tile_group *tg);
-- 2.24.1
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
On Mon, Jan 27, 2020 at 05:38:15PM -0500, Alex Deucher wrote:
On Fri, Jan 24, 2020 at 3:03 PM Ville Syrjala ville.syrjala@linux.intel.com wrote:
From: Ville Syrjälä ville.syrjala@linux.intel.com
Let's try to make a lot more stuff const in the edid parser.
The "downside" is that we can no longer mangle the EDID in the middle of the parsing to apply quirks (drm_mode_detailed()). I don't really think mangling the blob itself is such a great idea anyway so I won't miss that part. But if we do want it back I guess we should do the mangling in one explicit place before we otherwise parse the EDID.
Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com
I generally agree, but are there any userspace expectations that they will be getting a corrected EDID in some cases?
Not sure. I think the the only thing we're fixing up is some DTDs so at least there's a better way for userspace to get the fixed information (getconnector ioctl). I guess Xorg is still parsing the EDID though, but it should have more or less the same quirks in its parser.
Alex
drivers/gpu/drm/drm_connector.c | 4 +- drivers/gpu/drm/drm_edid.c | 303 ++++++++++++++++++-------------- include/drm/drm_connector.h | 4 +- 3 files changed, 176 insertions(+), 135 deletions(-)
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c index f632ca05960e..92a5cd6ff6b1 100644 --- a/drivers/gpu/drm/drm_connector.c +++ b/drivers/gpu/drm/drm_connector.c @@ -2377,7 +2377,7 @@ EXPORT_SYMBOL(drm_mode_put_tile_group);
- tile group or NULL if not found.
*/ struct drm_tile_group *drm_mode_get_tile_group(struct drm_device *dev,
char topology[8])
const u8 topology[8]){ struct drm_tile_group *tg; int id; @@ -2407,7 +2407,7 @@ EXPORT_SYMBOL(drm_mode_get_tile_group);
- new tile group or NULL.
*/ struct drm_tile_group *drm_mode_create_tile_group(struct drm_device *dev,
char topology[8])
const u8 topology[8]){ struct drm_tile_group *tg; int ret; diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index fd9b724067a7..8e76efe1654d 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -88,7 +88,7 @@
struct detailed_mode_closure { struct drm_connector *connector;
struct edid *edid;
const struct edid *edid; bool preferred; u32 quirks; int modes;@@ -1584,8 +1584,8 @@ MODULE_PARM_DESC(edid_fixup, "Minimum number of valid EDID header bytes (0-8, default 6)");
static void drm_get_displayid(struct drm_connector *connector,
struct edid *edid);-static int validate_displayid(u8 *displayid, int length, int idx);
const struct edid *edid);+static int validate_displayid(const u8 *displayid, int length, int idx);
static int drm_edid_block_checksum(const u8 *raw_edid) { @@ -2207,41 +2207,41 @@ static bool is_detailed_timing_descriptor(const u8 d[18]) return d[0] != 0x00 || d[1] != 0x00; }
-typedef void detailed_cb(struct detailed_timing *timing, void *closure); +typedef void detailed_cb(const struct detailed_timing *timing, void *closure);
static void -cea_for_each_detailed_block(u8 *ext, detailed_cb *cb, void *closure) +cea_for_each_detailed_block(const u8 *ext, detailed_cb *cb, void *closure) { int i, n; u8 d = ext[0x02];
u8 *det_base = ext + d;
const u8 *det_base = ext + d; if (d < 4 || d > 127) return; n = (127 - d) / 18; for (i = 0; i < n; i++)
cb((struct detailed_timing *)(det_base + 18 * i), closure);
cb((const struct detailed_timing *)(det_base + 18 * i), closure);}
static void -vtb_for_each_detailed_block(u8 *ext, detailed_cb *cb, void *closure) +vtb_for_each_detailed_block(const u8 *ext, detailed_cb *cb, void *closure) { unsigned int i, n = min((int)ext[0x02], 6);
u8 *det_base = ext + 5;
const u8 *det_base = ext + 5; if (ext[0x01] != 1) return; /* unknown version */ for (i = 0; i < n; i++)
cb((struct detailed_timing *)(det_base + 18 * i), closure);
cb((const struct detailed_timing *)(det_base + 18 * i), closure);}
static void -drm_for_each_detailed_block(u8 *raw_edid, detailed_cb *cb, void *closure) +drm_for_each_detailed_block(const u8 *raw_edid, detailed_cb *cb, void *closure) {
const struct edid *edid = (struct edid *)raw_edid; int i;
struct edid *edid = (struct edid *)raw_edid; if (edid == NULL) return;@@ -2250,7 +2250,7 @@ drm_for_each_detailed_block(u8 *raw_edid, detailed_cb *cb, void *closure) cb(&(edid->detailed_timings[i]), closure);
for (i = 1; i <= raw_edid[0x7e]; i++) {
u8 *ext = raw_edid + (i * EDID_LENGTH);
const u8 *ext = raw_edid + (i * EDID_LENGTH); switch (*ext) { case CEA_EXT: cea_for_each_detailed_block(ext, cb, closure);@@ -2264,81 +2264,105 @@ drm_for_each_detailed_block(u8 *raw_edid, detailed_cb *cb, void *closure) } }
+struct bool_closure {
bool ret;+};
static void -is_rb(struct detailed_timing *t, void *data) +is_rb(const struct detailed_timing *t, void *c) {
u8 *r = (u8 *)t;
struct bool_closure *closure = c;const u8 *r = (const u8 *)t; if (!is_display_descriptor(r, EDID_DETAIL_MONITOR_RANGE)) return; if (r[15] & 0x10)
*(bool *)data = true;
closure->ret = true;}
/* EDID 1.4 defines this explicitly. For EDID 1.3, we guess, badly. */ static bool -drm_monitor_supports_rb(struct edid *edid) +drm_monitor_supports_rb(const struct edid *edid) { if (edid->revision >= 4) {
bool ret = false;drm_for_each_detailed_block((u8 *)edid, is_rb, &ret);return ret;
struct bool_closure closure = {.ret = false,};drm_for_each_detailed_block((u8 *)edid, is_rb, &closure);return closure.ret; } return ((edid->input & DRM_EDID_INPUT_DIGITAL) != 0);}
+struct data_closure {
const u8 *data;+};
static void -find_gtf2(struct detailed_timing *t, void *data) +do_find_gtf2(const struct detailed_timing *t, void *c) {
u8 *r = (u8 *)t;
struct data_closure *closure = c;const u8 *r = (const u8 *)t; if (!is_display_descriptor(r, EDID_DETAIL_MONITOR_RANGE)) return; if (r[10] == 0x02)
*(u8 **)data = r;
closure->data = r;+}
+static const u8 * +find_gtf2_descriptor(const struct edid *edid) +{
struct data_closure closure = {};drm_for_each_detailed_block((u8 *)edid, do_find_gtf2, &closure);return closure.data;}
/* Secondary GTF curve kicks in above some break frequency */ static int -drm_gtf2_hbreak(struct edid *edid) +drm_gtf2_hbreak(const struct edid *edid) {
u8 *r = NULL;drm_for_each_detailed_block((u8 *)edid, find_gtf2, &r);return r ? (r[12] * 2) : 0;
const u8 *r = find_gtf2_descriptor(edid);return r ? r[12] * 2 : 0;}
static int -drm_gtf2_2c(struct edid *edid) +drm_gtf2_2c(const struct edid *edid) {
u8 *r = NULL;drm_for_each_detailed_block((u8 *)edid, find_gtf2, &r);
const u8 *r = find_gtf2_descriptor(edid);return r ? r[13] : 0;}
static int -drm_gtf2_m(struct edid *edid) +drm_gtf2_m(const struct edid *edid) {
u8 *r = NULL;drm_for_each_detailed_block((u8 *)edid, find_gtf2, &r);return r ? (r[15] << 8) + r[14] : 0;
const u8 *r = find_gtf2_descriptor(edid);return r ? (r[15] << 8) | r[14] : 0;}
static int -drm_gtf2_k(struct edid *edid) +drm_gtf2_k(const struct edid *edid) {
u8 *r = NULL;drm_for_each_detailed_block((u8 *)edid, find_gtf2, &r);
const u8 *r = find_gtf2_descriptor(edid);return r ? r[16] : 0;}
static int -drm_gtf2_2j(struct edid *edid) +drm_gtf2_2j(const struct edid *edid) {
u8 *r = NULL;drm_for_each_detailed_block((u8 *)edid, find_gtf2, &r);
const u8 *r = find_gtf2_descriptor(edid);return r ? r[17] : 0;}
@@ -2346,7 +2370,7 @@ drm_gtf2_2j(struct edid *edid)
- standard_timing_level - get std. timing level(CVT/GTF/DMT)
- @edid: EDID block to scan
*/ -static int standard_timing_level(struct edid *edid) +static int standard_timing_level(const struct edid *edid) { if (edid->revision >= 2) { if (edid->revision >= 4 && (edid->features & DRM_EDID_FEATURE_DEFAULT_GTF)) @@ -2381,8 +2405,9 @@ bad_std_timing(u8 a, u8 b)
- and convert them into a real mode using CVT/GTF/DMT.
*/ static struct drm_display_mode * -drm_mode_std(struct drm_connector *connector, struct edid *edid,
struct std_timing *t)+drm_mode_std(struct drm_connector *connector,
const struct edid *edid,const struct std_timing *t){ struct drm_device *dev = connector->dev; struct drm_display_mode *m, *mode = NULL; @@ -2500,7 +2525,7 @@ drm_mode_std(struct drm_connector *connector, struct edid *edid, */ static void drm_mode_do_interlace_quirk(struct drm_display_mode *mode,
struct detailed_pixel_timing *pt)
const struct detailed_pixel_timing *pt){ int i; static const struct { @@ -2543,12 +2568,12 @@ drm_mode_do_interlace_quirk(struct drm_display_mode *mode,
- return a new struct drm_display_mode.
*/ static struct drm_display_mode *drm_mode_detailed(struct drm_device *dev,
struct edid *edid,struct detailed_timing *timing,
const struct edid *edid,const struct detailed_timing *timing, u32 quirks){ struct drm_display_mode *mode;
struct detailed_pixel_timing *pt = &timing->data.pixel_data;
const struct detailed_pixel_timing *pt = &timing->data.pixel_data; unsigned hactive = (pt->hactive_hblank_hi & 0xf0) << 4 | pt->hactive_lo; unsigned vactive = (pt->vactive_vblank_hi & 0xf0) << 4 | pt->vactive_lo; unsigned hblank = (pt->hactive_hblank_hi & 0xf) << 8 | pt->hblank_lo;@@ -2590,9 +2615,9 @@ static struct drm_display_mode *drm_mode_detailed(struct drm_device *dev, return NULL;
if (quirks & EDID_QUIRK_135_CLOCK_TOO_HIGH)
timing->pixel_clock = cpu_to_le16(1088);mode->clock = le16_to_cpu(timing->pixel_clock) * 10;
mode->clock = 1088 * 10;elsemode->clock = le16_to_cpu(timing->pixel_clock) * 10; mode->hdisplay = hactive; mode->hsync_start = mode->hdisplay + hsync_offset;@@ -2613,14 +2638,14 @@ static struct drm_display_mode *drm_mode_detailed(struct drm_device *dev, drm_mode_do_interlace_quirk(mode, pt);
if (quirks & EDID_QUIRK_DETAILED_SYNC_PP) {
pt->misc |= DRM_EDID_PT_HSYNC_POSITIVE | DRM_EDID_PT_VSYNC_POSITIVE;
mode->flags |= DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_NVSYNC;} else {mode->flags |= (pt->misc & DRM_EDID_PT_HSYNC_POSITIVE) ?DRM_MODE_FLAG_PHSYNC : DRM_MODE_FLAG_NHSYNC;mode->flags |= (pt->misc & DRM_EDID_PT_VSYNC_POSITIVE) ?DRM_MODE_FLAG_PVSYNC : DRM_MODE_FLAG_NVSYNC; }
mode->flags |= (pt->misc & DRM_EDID_PT_HSYNC_POSITIVE) ?DRM_MODE_FLAG_PHSYNC : DRM_MODE_FLAG_NHSYNC;mode->flags |= (pt->misc & DRM_EDID_PT_VSYNC_POSITIVE) ?DRM_MODE_FLAG_PVSYNC : DRM_MODE_FLAG_NVSYNC;set_size: mode->width_mm = pt->width_mm_lo | (pt->width_height_mm_hi & 0xf0) << 4; mode->height_mm = pt->height_mm_lo | (pt->width_height_mm_hi & 0xf) << 8; @@ -2644,7 +2669,7 @@ static struct drm_display_mode *drm_mode_detailed(struct drm_device *dev,
static bool mode_in_hsync_range(const struct drm_display_mode *mode,
struct edid *edid, u8 *t)
const struct edid *edid, const u8 *t){ int hsync, hmin, hmax;
@@ -2661,7 +2686,7 @@ mode_in_hsync_range(const struct drm_display_mode *mode,
static bool mode_in_vsync_range(const struct drm_display_mode *mode,
struct edid *edid, u8 *t)
const struct edid *edid, const u8 *t){ int vsync, vmin, vmax;
@@ -2677,7 +2702,7 @@ mode_in_vsync_range(const struct drm_display_mode *mode, }
static u32 -range_pixel_clock(struct edid *edid, u8 *t) +range_pixel_clock(const struct edid *edid, const u8 *t) { /* unspecified */ if (t[9] == 0 || t[9] == 255) @@ -2692,11 +2717,12 @@ range_pixel_clock(struct edid *edid, u8 *t) }
static bool -mode_in_range(const struct drm_display_mode *mode, struct edid *edid,
struct detailed_timing *timing)+mode_in_range(const struct drm_display_mode *mode,
const struct edid *edid,const struct detailed_timing *timing){ u32 max_clock;
u8 *t = (u8 *)timing;
const u8 *t = (const u8 *)timing; if (!mode_in_hsync_range(mode, edid, t)) return false;@@ -2738,8 +2764,9 @@ static bool valid_inferred_mode(const struct drm_connector *connector, }
static int -drm_dmt_modes_for_range(struct drm_connector *connector, struct edid *edid,
struct detailed_timing *timing)+drm_dmt_modes_for_range(struct drm_connector *connector,
const struct edid *edid,const struct detailed_timing *timing){ int i, modes = 0; struct drm_display_mode *newmode; @@ -2773,8 +2800,9 @@ void drm_mode_fixup_1366x768(struct drm_display_mode *mode) }
static int -drm_gtf_modes_for_range(struct drm_connector *connector, struct edid *edid,
struct detailed_timing *timing)+drm_gtf_modes_for_range(struct drm_connector *connector,
const struct edid *edid,const struct detailed_timing *timing){ int i, modes = 0; struct drm_display_mode *newmode; @@ -2801,8 +2829,9 @@ drm_gtf_modes_for_range(struct drm_connector *connector, struct edid *edid, }
static int -drm_cvt_modes_for_range(struct drm_connector *connector, struct edid *edid,
struct detailed_timing *timing)+drm_cvt_modes_for_range(struct drm_connector *connector,
const struct edid *edid,const struct detailed_timing *timing){ int i, modes = 0; struct drm_display_mode *newmode; @@ -2830,11 +2859,11 @@ drm_cvt_modes_for_range(struct drm_connector *connector, struct edid *edid, }
static void -do_inferred_modes(struct detailed_timing *timing, void *c) +do_inferred_modes(const struct detailed_timing *timing, void *c) { struct detailed_mode_closure *closure = c;
struct detailed_non_pixel *data = &timing->data.other_data;struct detailed_data_monitor_range *range = &data->data.range;
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)) return;@@ -2868,7 +2897,8 @@ do_inferred_modes(struct detailed_timing *timing, void *c) }
static int -add_inferred_modes(struct drm_connector *connector, struct edid *edid) +add_inferred_modes(struct drm_connector *connector,
const struct edid *edid){ struct detailed_mode_closure closure = { .connector = connector, @@ -2876,18 +2906,20 @@ add_inferred_modes(struct drm_connector *connector, struct edid *edid) };
if (version_greater(edid, 1, 0))
drm_for_each_detailed_block((u8 *)edid, do_inferred_modes,
drm_for_each_detailed_block((const u8 *)edid,do_inferred_modes, &closure); return closure.modes;}
static int -drm_est3_modes(struct drm_connector *connector, struct detailed_timing *timing) +drm_est3_modes(struct drm_connector *connector,
const struct detailed_timing *timing){ int i, j, m, modes = 0; struct drm_display_mode *mode;
u8 *est = ((u8 *)timing) + 6;
const u8 *est = ((const u8 *)timing) + 6; for (i = 0; i < 6; i++) { for (j = 7; j >= 0; j--) {@@ -2912,7 +2944,7 @@ drm_est3_modes(struct drm_connector *connector, struct detailed_timing *timing) }
static void -do_established_modes(struct detailed_timing *timing, void *c) +do_established_modes(const struct detailed_timing *timing, void *c) { struct detailed_mode_closure *closure = c;
@@ -2931,7 +2963,8 @@ do_established_modes(struct detailed_timing *timing, void *c)
- (defined above). Tease them out and add them to the global modes list.
*/ static int -add_established_modes(struct drm_connector *connector, struct edid *edid) +add_established_modes(struct drm_connector *connector,
const struct edid *edid){ struct drm_device *dev = connector->dev; unsigned long est_bits = edid->established_timings.t1 | @@ -2962,19 +2995,19 @@ add_established_modes(struct drm_connector *connector, struct edid *edid) }
static void -do_standard_modes(struct detailed_timing *timing, void *c) +do_standard_modes(const struct detailed_timing *timing, void *c) { struct detailed_mode_closure *closure = c;
struct detailed_non_pixel *data = &timing->data.other_data;
const struct detailed_non_pixel *data = &timing->data.other_data; struct drm_connector *connector = closure->connector;
struct edid *edid = closure->edid;
const struct edid *edid = closure->edid; int i; if (!is_display_descriptor((const u8 *)timing, EDID_DETAIL_STD_MODES)) return; for (i = 0; i < 6; i++) {
struct std_timing *std = &data->data.timings[i];
const struct std_timing *std = &data->data.timings[i]; struct drm_display_mode *newmode; newmode = drm_mode_std(connector, edid, std);@@ -2994,7 +3027,8 @@ do_standard_modes(struct detailed_timing *timing, void *c)
- GTF or CVT. Grab them from @edid and add them to the list.
*/ static int -add_standard_modes(struct drm_connector *connector, struct edid *edid) +add_standard_modes(struct drm_connector *connector,
const struct edid *edid){ int i, modes = 0; struct detailed_mode_closure closure = { @@ -3023,18 +3057,18 @@ add_standard_modes(struct drm_connector *connector, struct edid *edid) }
static int drm_cvt_modes(struct drm_connector *connector,
struct detailed_timing *timing)
const struct detailed_timing *timing){ int i, j, modes = 0; struct drm_display_mode *newmode; struct drm_device *dev = connector->dev;
struct cvt_timing *cvt; const int rates[] = { 60, 85, 75, 60, 50 }; const u8 empty[3] = { 0, 0, 0 }; for (i = 0; i < 4; i++) { int uninitialized_var(width), height;cvt = &(timing->data.other_data.data.cvt[i]);
const struct cvt_timing *cvt =&timing->data.other_data.data.cvt[i]; if (!memcmp(cvt->code, empty, 3)) continue;@@ -3072,7 +3106,7 @@ static int drm_cvt_modes(struct drm_connector *connector, }
static void -do_cvt_mode(struct detailed_timing *timing, void *c) +do_cvt_mode(const struct detailed_timing *timing, void *c) { struct detailed_mode_closure *closure = c;
@@ -3083,7 +3117,8 @@ do_cvt_mode(struct detailed_timing *timing, void *c) }
static int -add_cvt_modes(struct drm_connector *connector, struct edid *edid) +add_cvt_modes(struct drm_connector *connector,
const struct edid *edid){ struct detailed_mode_closure closure = { .connector = connector, @@ -3101,7 +3136,7 @@ add_cvt_modes(struct drm_connector *connector, struct edid *edid) static void fixup_detailed_cea_mode_clock(struct drm_display_mode *mode);
static void -do_detailed_mode(struct detailed_timing *timing, void *c) +do_detailed_mode(const struct detailed_timing *timing, void *c) { struct detailed_mode_closure *closure = c; struct drm_display_mode *newmode; @@ -3137,8 +3172,8 @@ do_detailed_mode(struct detailed_timing *timing, void *c)
- @quirks: quirks to apply
*/ static int -add_detailed_modes(struct drm_connector *connector, struct edid *edid,
u32 quirks)+add_detailed_modes(struct drm_connector *connector,
struct edid *edid, u32 quirks){ struct detailed_mode_closure closure = { .connector = connector, @@ -3173,9 +3208,10 @@ add_detailed_modes(struct drm_connector *connector, struct edid *edid, /*
- Search EDID for CEA extension block.
*/ -static u8 *drm_find_edid_extension(const struct edid *edid, int ext_id) +static const u8 *drm_find_edid_extension(const struct edid *edid,
int ext_id){
u8 *edid_ext = NULL;
const u8 *edid_ext = NULL; int i; /* No EDID or EDID extensions */@@ -3184,7 +3220,7 @@ static u8 *drm_find_edid_extension(const struct edid *edid, int ext_id)
/* Find CEA extension */ for (i = 0; i < edid->extensions; i++) {
edid_ext = (u8 *)edid + EDID_LENGTH * (i + 1);
edid_ext = (const u8 *)edid + EDID_LENGTH * (i + 1); if (edid_ext[0] == ext_id) break; }@@ -3196,19 +3232,19 @@ static u8 *drm_find_edid_extension(const struct edid *edid, int ext_id) }
-static u8 *drm_find_displayid_extension(const struct edid *edid) +static const u8 *drm_find_displayid_extension(const struct edid *edid) { return drm_find_edid_extension(edid, DISPLAYID_EXT); }
-static u8 *drm_find_cea_extension(const struct edid *edid) +static const u8 *drm_find_cea_extension(const struct edid *edid) { int ret; int idx = 1; int length = EDID_LENGTH;
struct displayid_block *block;u8 *cea;u8 *displayid;
const struct displayid_block *block;const u8 *cea;const u8 *displayid; /* Look for a top level CEA extension block */ cea = drm_find_edid_extension(edid, CEA_EXT);@@ -4315,28 +4351,30 @@ drm_parse_hdmi_vsdb_audio(struct drm_connector *connector, const u8 *db) }
static void -monitor_name(struct detailed_timing *t, void *data) +monitor_name(const struct detailed_timing *t, void *c) {
struct data_closure *closure = c;if (!is_display_descriptor((const u8 *)t, EDID_DETAIL_MONITOR_NAME)) return;
*(u8 **)data = t->data.other_data.data.str.str;
closure->data = t->data.other_data.data.str.str;}
static int get_monitor_name(struct edid *edid, char name[13]) {
char *edid_name = NULL;
struct data_closure closure = {}; int mnl; if (!edid || !name) return 0;
drm_for_each_detailed_block((u8 *)edid, monitor_name, &edid_name);for (mnl = 0; edid_name && mnl < 13; mnl++) {if (edid_name[mnl] == 0x0a)
drm_for_each_detailed_block((const u8 *)edid, monitor_name, &closure);for (mnl = 0; closure.data && mnl < 13; mnl++) {if (closure.data[mnl] == 0x0a) break;
name[mnl] = edid_name[mnl];
name[mnl] = closure.data[mnl]; } return mnl;@@ -4386,11 +4424,9 @@ static void clear_eld(struct drm_connector *connector) static void drm_edid_to_eld(struct drm_connector *connector, struct edid *edid) { uint8_t *eld = connector->eld;
u8 *cea;u8 *db;
const u8 *cea; int total_sad_count = 0; int mnl;
int dbl; clear_eld(connector);@@ -4425,8 +4461,8 @@ static void drm_edid_to_eld(struct drm_connector *connector, struct edid *edid) }
for_each_cea_db(cea, i, start, end) {
db = &cea[i];dbl = cea_db_payload_len(db);
const u8 *db = &cea[i];int dbl = cea_db_payload_len(db); switch (cea_db_tag(db)) { int sad_count;@@ -4484,7 +4520,7 @@ int drm_edid_to_sad(struct edid *edid, struct cea_sad **sads) { int count = 0; int i, start, end, dbl;
u8 *cea;
const u8 *cea; cea = drm_find_cea_extension(edid); if (!cea) {@@ -4503,7 +4539,7 @@ int drm_edid_to_sad(struct edid *edid, struct cea_sad **sads) }
for_each_cea_db(cea, i, start, end) {
u8 *db = &cea[i];
const u8 *db = &cea[i]; if (cea_db_tag(db) == AUDIO_BLOCK) { int j;@@ -4514,7 +4550,7 @@ int drm_edid_to_sad(struct edid *edid, struct cea_sad **sads) if (!*sads) return -ENOMEM; for (j = 0; j < count; j++) {
u8 *sad = &db[1 + j * 3];
const u8 *sad = &db[1 + j * 3]; (*sads)[j].format = (sad[0] & 0x78) >> 3; (*sads)[j].channels = sad[0] & 0x7;@@ -4635,7 +4671,7 @@ EXPORT_SYMBOL(drm_av_sync_delay); */ bool drm_detect_hdmi_monitor(struct edid *edid) {
u8 *edid_ext;
const u8 *edid_ext; int i; int start_offset, end_offset;@@ -4673,7 +4709,7 @@ EXPORT_SYMBOL(drm_detect_hdmi_monitor); */ bool drm_detect_monitor_audio(struct edid *edid) {
u8 *edid_ext;
const u8 *edid_ext; int i, j; bool has_audio = false; int start_offset, end_offset;@@ -5017,13 +5053,13 @@ u32 drm_add_display_info(struct drm_connector *connector, const struct edid *edi return quirks; }
-static int validate_displayid(u8 *displayid, int length, int idx) +static int validate_displayid(const u8 *displayid, int length, int idx) { int i; u8 csum = 0;
struct displayid_hdr *base;
const struct displayid_hdr *base;
base = (struct displayid_hdr *)&displayid[idx];
base = (const struct displayid_hdr *)&displayid[idx]; DRM_DEBUG_KMS("base revision 0x%x, length %d, %d %d\n", base->rev, base->bytes, base->prod_id, base->ext_count);@@ -5041,7 +5077,7 @@ static int validate_displayid(u8 *displayid, int length, int idx) }
static struct drm_display_mode *drm_mode_displayid_detailed(struct drm_device *dev,
struct displayid_detailed_timings_1 *timings)
const struct displayid_detailed_timings_1 *timings){ struct drm_display_mode *mode; unsigned pixel_clock = (timings->pixel_clock[0] | @@ -5057,6 +5093,7 @@ static struct drm_display_mode *drm_mode_displayid_detailed(struct drm_device *d unsigned vsync_width = (timings->vsw[0] | timings->vsw[1] << 8) + 1; bool hsync_positive = (timings->hsync[1] >> 7) & 0x1; bool vsync_positive = (timings->vsync[1] >> 7) & 0x1;
mode = drm_mode_create(dev); if (!mode) return NULL;@@ -5086,9 +5123,10 @@ static struct drm_display_mode *drm_mode_displayid_detailed(struct drm_device *d }
static int add_displayid_detailed_1_modes(struct drm_connector *connector,
struct displayid_block *block)
const struct displayid_block *block){
struct displayid_detailed_timing_block *det = (struct displayid_detailed_timing_block *)block;
const struct displayid_detailed_timing_block *det =(const struct displayid_detailed_timing_block *)block; int i; int num_timings; struct drm_display_mode *newmode;@@ -5099,7 +5137,7 @@ static int add_displayid_detailed_1_modes(struct drm_connector *connector,
num_timings = block->num_bytes / 20; for (i = 0; i < num_timings; i++) {
struct displayid_detailed_timings_1 *timings = &det->timings[i];
const struct displayid_detailed_timings_1 *timings = &det->timings[i]; newmode = drm_mode_displayid_detailed(connector->dev, timings); if (!newmode)@@ -5112,13 +5150,13 @@ static int add_displayid_detailed_1_modes(struct drm_connector *connector, }
static int add_displayid_detailed_modes(struct drm_connector *connector,
struct edid *edid)
const struct edid *edid){
u8 *displayid;
const u8 *displayid; int ret; int idx = 1; int length = EDID_LENGTH;
struct displayid_block *block;
const struct displayid_block *block; int num_modes = 0; displayid = drm_find_displayid_extension(edid);@@ -5720,9 +5758,10 @@ drm_hdmi_vendor_infoframe_from_display_mode(struct hdmi_vendor_infoframe *frame, EXPORT_SYMBOL(drm_hdmi_vendor_infoframe_from_display_mode);
static int drm_parse_tiled_block(struct drm_connector *connector,
struct displayid_block *block)
const struct displayid_block *block){
struct displayid_tiled_block *tile = (struct displayid_tiled_block *)block;
const struct displayid_tiled_block *tile =(const struct displayid_tiled_block *)block; u16 w, h; u8 tile_v_loc, tile_h_loc; u8 num_v_tile, num_h_tile;@@ -5774,12 +5813,12 @@ static int drm_parse_tiled_block(struct drm_connector *connector, }
static int drm_parse_display_id(struct drm_connector *connector,
u8 *displayid, int length,
const u8 *displayid, int length, bool is_edid_extension){ /* if this is an EDID extension the first byte will be 0x70 */ int idx = 0;
struct displayid_block *block;
const struct displayid_block *block; int ret; if (is_edid_extension)@@ -5815,11 +5854,13 @@ static int drm_parse_display_id(struct drm_connector *connector, }
static void drm_get_displayid(struct drm_connector *connector,
struct edid *edid)
const struct edid *edid){
void *displayid = NULL;
const void *displayid; int ret;connector->has_tile = false;displayid = drm_find_displayid_extension(edid); if (!displayid) { /* drop reference to any tile group we had */diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h index 2113500b4075..c0f9ce3f4b24 100644 --- a/include/drm/drm_connector.h +++ b/include/drm/drm_connector.h @@ -1580,9 +1580,9 @@ struct drm_tile_group { };
struct drm_tile_group *drm_mode_create_tile_group(struct drm_device *dev,
char topology[8]);
const u8 topology[8]);struct drm_tile_group *drm_mode_get_tile_group(struct drm_device *dev,
char topology[8]);
const u8 topology[8]);void drm_mode_put_tile_group(struct drm_device *dev, struct drm_tile_group *tg);
-- 2.24.1
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
-----Original Message----- From: Intel-gfx intel-gfx-bounces@lists.freedesktop.org On Behalf Of Ville Syrjälä Sent: Tuesday, January 28, 2020 5:19 PM To: Alex Deucher alexdeucher@gmail.com Cc: Intel Graphics Development intel-gfx@lists.freedesktop.org; Maling list - DRI developers dri-devel@lists.freedesktop.org Subject: Re: [Intel-gfx] [PATCH 7/8] drm/edid: Constify lots of things
On Mon, Jan 27, 2020 at 05:38:15PM -0500, Alex Deucher wrote:
On Fri, Jan 24, 2020 at 3:03 PM Ville Syrjala ville.syrjala@linux.intel.com wrote:
From: Ville Syrjälä ville.syrjala@linux.intel.com
Let's try to make a lot more stuff const in the edid parser.
The "downside" is that we can no longer mangle the EDID in the middle of the parsing to apply quirks (drm_mode_detailed()). I don't really think mangling the blob itself is such a great idea anyway so I won't miss that part. But if we do want it back I guess we should do the mangling in one explicit place before we otherwise parse the EDID.
Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com
I generally agree, but are there any userspace expectations that they will be getting a corrected EDID in some cases?
Not sure. I think the the only thing we're fixing up is some DTDs so at least there's a better way for userspace to get the fixed information (getconnector ioctl). I guess Xorg is still parsing the EDID though, but it should have more or less the same quirks in its parser.
Theoretically, there may be a possibility of userspace getting out of sync in case EDID exposed to user is different to the quirked version getting used in kernel. I feel what you said in commit message looks good, where we can do the quirks at one place and share the same to userspace. Later just use that to proceed with parsing with constantifed edid in kernel.
Regards, Uma Shankar
Alex
drivers/gpu/drm/drm_connector.c | 4 +- drivers/gpu/drm/drm_edid.c | 303 ++++++++++++++++++-------------- include/drm/drm_connector.h | 4 +- 3 files changed, 176 insertions(+), 135 deletions(-)
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c index f632ca05960e..92a5cd6ff6b1 100644 --- a/drivers/gpu/drm/drm_connector.c +++ b/drivers/gpu/drm/drm_connector.c @@ -2377,7 +2377,7 @@ EXPORT_SYMBOL(drm_mode_put_tile_group);
- tile group or NULL if not found.
*/ struct drm_tile_group *drm_mode_get_tile_group(struct drm_device *dev,
char topology[8])
const u8 topology[8]){ struct drm_tile_group *tg; int id; @@ -2407,7 +2407,7 @@ EXPORT_SYMBOL(drm_mode_get_tile_group);
- new tile group or NULL.
*/ struct drm_tile_group *drm_mode_create_tile_group(struct drm_device *dev,
char topology[8])
const u8- topology[8])
{ struct drm_tile_group *tg; int ret; diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index fd9b724067a7..8e76efe1654d 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -88,7 +88,7 @@
struct detailed_mode_closure { struct drm_connector *connector;
struct edid *edid;
const struct edid *edid; bool preferred; u32 quirks; int modes;@@ -1584,8 +1584,8 @@ MODULE_PARM_DESC(edid_fixup, "Minimum number of valid EDID header bytes (0-8, default 6)");
static void drm_get_displayid(struct drm_connector *connector,
struct edid *edid);-static int validate_displayid(u8 *displayid, int length, int idx);
const struct edid *edid); static int+validate_displayid(const u8 *displayid, int length, int idx);
static int drm_edid_block_checksum(const u8 *raw_edid) { @@ -2207,41 +2207,41 @@ static bool is_detailed_timing_descriptor(const u8 d[18]) return d[0] != 0x00 || d[1] != 0x00; }
-typedef void detailed_cb(struct detailed_timing *timing, void *closure); +typedef void detailed_cb(const struct detailed_timing *timing, void +*closure);
static void -cea_for_each_detailed_block(u8 *ext, detailed_cb *cb, void *closure) +cea_for_each_detailed_block(const u8 *ext, detailed_cb *cb, void +*closure) { int i, n; u8 d = ext[0x02];
u8 *det_base = ext + d;
const u8 *det_base = ext + d; if (d < 4 || d > 127) return; n = (127 - d) / 18; for (i = 0; i < n; i++)
cb((struct detailed_timing *)(det_base + 18 * i), closure);
cb((const struct detailed_timing *)(det_base + 18 *- i), closure);
}
static void -vtb_for_each_detailed_block(u8 *ext, detailed_cb *cb, void *closure) +vtb_for_each_detailed_block(const u8 *ext, detailed_cb *cb, void +*closure) { unsigned int i, n = min((int)ext[0x02], 6);
u8 *det_base = ext + 5;
const u8 *det_base = ext + 5; if (ext[0x01] != 1) return; /* unknown version */ for (i = 0; i < n; i++)
cb((struct detailed_timing *)(det_base + 18 * i), closure);
cb((const struct detailed_timing *)(det_base + 18 *- i), closure);
}
static void -drm_for_each_detailed_block(u8 *raw_edid, detailed_cb *cb, void *closure) +drm_for_each_detailed_block(const u8 *raw_edid, detailed_cb *cb, +void *closure) {
const struct edid *edid = (struct edid *)raw_edid; int i;
struct edid *edid = (struct edid *)raw_edid; if (edid == NULL) return;@@ -2250,7 +2250,7 @@ drm_for_each_detailed_block(u8 *raw_edid,
detailed_cb *cb, void *closure)
cb(&(edid->detailed_timings[i]), closure); for (i = 1; i <= raw_edid[0x7e]; i++) {
u8 *ext = raw_edid + (i * EDID_LENGTH);
const u8 *ext = raw_edid + (i * EDID_LENGTH); switch (*ext) { case CEA_EXT: cea_for_each_detailed_block(ext, cb,closure); @@ -2264,81 +2264,105 @@ drm_for_each_detailed_block(u8
*raw_edid, detailed_cb *cb, void *closure)
}}
+struct bool_closure {
bool ret;+};
static void -is_rb(struct detailed_timing *t, void *data) +is_rb(const struct detailed_timing *t, void *c) {
u8 *r = (u8 *)t;
struct bool_closure *closure = c;const u8 *r = (const u8 *)t; if (!is_display_descriptor(r, EDID_DETAIL_MONITOR_RANGE)) return; if (r[15] & 0x10)
*(bool *)data = true;
closure->ret = true;}
/* EDID 1.4 defines this explicitly. For EDID 1.3, we guess, badly. */ static bool -drm_monitor_supports_rb(struct edid *edid) +drm_monitor_supports_rb(const struct edid *edid) { if (edid->revision >= 4) {
bool ret = false;drm_for_each_detailed_block((u8 *)edid, is_rb, &ret);return ret;
struct bool_closure closure = {.ret = false,};drm_for_each_detailed_block((u8 *)edid, is_rb,&closure);
return closure.ret; } return ((edid->input & DRM_EDID_INPUT_DIGITAL) != 0); }+struct data_closure {
const u8 *data;+};
static void -find_gtf2(struct detailed_timing *t, void *data) +do_find_gtf2(const struct detailed_timing *t, void *c) {
u8 *r = (u8 *)t;
struct data_closure *closure = c;const u8 *r = (const u8 *)t; if (!is_display_descriptor(r, EDID_DETAIL_MONITOR_RANGE)) return; if (r[10] == 0x02)
*(u8 **)data = r;
closure->data = r;+}
+static const u8 * +find_gtf2_descriptor(const struct edid *edid) {
struct data_closure closure = {};drm_for_each_detailed_block((u8 *)edid, do_find_gtf2,- &closure);
return closure.data;}
/* Secondary GTF curve kicks in above some break frequency */ static int -drm_gtf2_hbreak(struct edid *edid) +drm_gtf2_hbreak(const struct edid *edid) {
u8 *r = NULL;drm_for_each_detailed_block((u8 *)edid, find_gtf2, &r);return r ? (r[12] * 2) : 0;
const u8 *r = find_gtf2_descriptor(edid);return r ? r[12] * 2 : 0;}
static int -drm_gtf2_2c(struct edid *edid) +drm_gtf2_2c(const struct edid *edid) {
u8 *r = NULL;drm_for_each_detailed_block((u8 *)edid, find_gtf2, &r);
const u8 *r = find_gtf2_descriptor(edid);return r ? r[13] : 0;}
static int -drm_gtf2_m(struct edid *edid) +drm_gtf2_m(const struct edid *edid) {
u8 *r = NULL;drm_for_each_detailed_block((u8 *)edid, find_gtf2, &r);return r ? (r[15] << 8) + r[14] : 0;
const u8 *r = find_gtf2_descriptor(edid);return r ? (r[15] << 8) | r[14] : 0;}
static int -drm_gtf2_k(struct edid *edid) +drm_gtf2_k(const struct edid *edid) {
u8 *r = NULL;drm_for_each_detailed_block((u8 *)edid, find_gtf2, &r);
const u8 *r = find_gtf2_descriptor(edid);return r ? r[16] : 0;}
static int -drm_gtf2_2j(struct edid *edid) +drm_gtf2_2j(const struct edid *edid) {
u8 *r = NULL;drm_for_each_detailed_block((u8 *)edid, find_gtf2, &r);
const u8 *r = find_gtf2_descriptor(edid);return r ? r[17] : 0;}
@@ -2346,7 +2370,7 @@ drm_gtf2_2j(struct edid *edid)
- standard_timing_level - get std. timing level(CVT/GTF/DMT)
- @edid: EDID block to scan
*/ -static int standard_timing_level(struct edid *edid) +static int standard_timing_level(const struct edid *edid) { if (edid->revision >= 2) { if (edid->revision >= 4 && (edid->features & DRM_EDID_FEATURE_DEFAULT_GTF)) @@ -2381,8 +2405,9 @@
bad_std_timing(u8 a, u8 b)
- and convert them into a real mode using CVT/GTF/DMT.
*/ static struct drm_display_mode * -drm_mode_std(struct drm_connector *connector, struct edid *edid,
struct std_timing *t)+drm_mode_std(struct drm_connector *connector,
const struct edid *edid,const struct std_timing *t){ struct drm_device *dev = connector->dev; struct drm_display_mode *m, *mode = NULL; @@ -2500,7 +2525,7 @@ drm_mode_std(struct drm_connector *connector, struct edid *edid, */ static void drm_mode_do_interlace_quirk(struct drm_display_mode *mode,
struct detailed_pixel_timing *pt)
const struct detailed_pixel_timing *pt){ int i; static const struct { @@ -2543,12 +2568,12 @@ drm_mode_do_interlace_quirk(struct
drm_display_mode *mode,
- return a new struct drm_display_mode.
*/ static struct drm_display_mode *drm_mode_detailed(struct drm_device *dev,
struct edid *edid,struct detailed_timing *timing,
const struct edid *edid,const struct- detailed_timing *timing, u32 quirks) { struct drm_display_mode *mode;
struct detailed_pixel_timing *pt = &timing->data.pixel_data;
const struct detailed_pixel_timing *pt =- &timing->data.pixel_data; unsigned hactive = (pt->hactive_hblank_hi & 0xf0) << 4 | pt->hactive_lo; unsigned vactive = (pt->vactive_vblank_hi & 0xf0) << 4 | pt->vactive_lo; unsigned hblank = (pt->hactive_hblank_hi & 0xf) << 8 |
pt->hblank_lo; @@ -2590,9 +2615,9 @@ static struct drm_display_mode
*drm_mode_detailed(struct drm_device *dev,
return NULL; if (quirks & EDID_QUIRK_135_CLOCK_TOO_HIGH)
timing->pixel_clock = cpu_to_le16(1088);mode->clock = le16_to_cpu(timing->pixel_clock) * 10;
mode->clock = 1088 * 10;elsemode->clock = le16_to_cpu(timing->pixel_clock) * 10; mode->hdisplay = hactive; mode->hsync_start = mode->hdisplay + hsync_offset; @@-2613,14 +2638,14 @@ static struct drm_display_mode
*drm_mode_detailed(struct drm_device *dev,
drm_mode_do_interlace_quirk(mode, pt); if (quirks & EDID_QUIRK_DETAILED_SYNC_PP) {
pt->misc |= DRM_EDID_PT_HSYNC_POSITIVE |DRM_EDID_PT_VSYNC_POSITIVE;
mode->flags |= DRM_MODE_FLAG_PHSYNC |DRM_MODE_FLAG_NVSYNC;
} else {mode->flags |= (pt->misc & DRM_EDID_PT_HSYNC_POSITIVE) ?DRM_MODE_FLAG_PHSYNC : DRM_MODE_FLAG_NHSYNC;mode->flags |= (pt->misc & DRM_EDID_PT_VSYNC_POSITIVE) ?DRM_MODE_FLAG_PVSYNC : DRM_MODE_FLAG_NVSYNC; }
mode->flags |= (pt->misc & DRM_EDID_PT_HSYNC_POSITIVE) ?DRM_MODE_FLAG_PHSYNC : DRM_MODE_FLAG_NHSYNC;mode->flags |= (pt->misc & DRM_EDID_PT_VSYNC_POSITIVE) ?DRM_MODE_FLAG_PVSYNC : DRM_MODE_FLAG_NVSYNC;set_size: mode->width_mm = pt->width_mm_lo | (pt->width_height_mm_hi & 0xf0)
<< 4;
mode->height_mm = pt->height_mm_lo | (pt->width_height_mm_hi& 0xf) << 8; @@ -2644,7 +2669,7 @@ static struct drm_display_mode *drm_mode_detailed(struct drm_device *dev,
static bool mode_in_hsync_range(const struct drm_display_mode *mode,
struct edid *edid, u8 *t)
const struct edid *edid, const u8 *t){ int hsync, hmin, hmax;
@@ -2661,7 +2686,7 @@ mode_in_hsync_range(const struct drm_display_mode *mode,
static bool mode_in_vsync_range(const struct drm_display_mode *mode,
struct edid *edid, u8 *t)
const struct edid *edid, const u8 *t){ int vsync, vmin, vmax;
@@ -2677,7 +2702,7 @@ mode_in_vsync_range(const struct drm_display_mode *mode, }
static u32 -range_pixel_clock(struct edid *edid, u8 *t) +range_pixel_clock(const struct edid *edid, const u8 *t) { /* unspecified */ if (t[9] == 0 || t[9] == 255) @@ -2692,11 +2717,12 @@ range_pixel_clock(struct edid *edid, u8 *t) }
static bool -mode_in_range(const struct drm_display_mode *mode, struct edid *edid,
struct detailed_timing *timing)+mode_in_range(const struct drm_display_mode *mode,
const struct edid *edid,const struct detailed_timing *timing){ u32 max_clock;
u8 *t = (u8 *)timing;
const u8 *t = (const u8 *)timing; if (!mode_in_hsync_range(mode, edid, t)) return false;@@ -2738,8 +2764,9 @@ static bool valid_inferred_mode(const struct drm_connector *connector, }
static int -drm_dmt_modes_for_range(struct drm_connector *connector, struct edid
*edid,
struct detailed_timing *timing)+drm_dmt_modes_for_range(struct drm_connector *connector,
const struct edid *edid,const struct detailed_timing *timing){ int i, modes = 0; struct drm_display_mode *newmode; @@ -2773,8 +2800,9 @@ void drm_mode_fixup_1366x768(struct drm_display_mode *mode) }
static int -drm_gtf_modes_for_range(struct drm_connector *connector, struct edid *edid,
struct detailed_timing *timing)+drm_gtf_modes_for_range(struct drm_connector *connector,
const struct edid *edid,const struct detailed_timing *timing){ int i, modes = 0; struct drm_display_mode *newmode; @@ -2801,8 +2829,9 @@ drm_gtf_modes_for_range(struct drm_connector *connector, struct edid *edid, }
static int -drm_cvt_modes_for_range(struct drm_connector *connector, struct edid *edid,
struct detailed_timing *timing)+drm_cvt_modes_for_range(struct drm_connector *connector,
const struct edid *edid,const struct detailed_timing *timing){ int i, modes = 0; struct drm_display_mode *newmode; @@ -2830,11 +2859,11 @@ drm_cvt_modes_for_range(struct drm_connector *connector, struct edid *edid, }
static void -do_inferred_modes(struct detailed_timing *timing, void *c) +do_inferred_modes(const struct detailed_timing *timing, void *c) { struct detailed_mode_closure *closure = c;
struct detailed_non_pixel *data = &timing->data.other_data;struct detailed_data_monitor_range *range = &data->data.range;
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))
return;@@ -2868,7 +2897,8 @@ do_inferred_modes(struct detailed_timing *timing, void *c) }
static int -add_inferred_modes(struct drm_connector *connector, struct edid *edid) +add_inferred_modes(struct drm_connector *connector,
const struct edid *edid){ struct detailed_mode_closure closure = { .connector = connector, @@ -2876,18 +2906,20 @@ add_inferred_modes(struct drm_connector *connector, struct edid *edid) };
if (version_greater(edid, 1, 0))
drm_for_each_detailed_block((u8 *)edid, do_inferred_modes,
drm_for_each_detailed_block((const u8 *)edid,do_inferred_modes, &closure); return closure.modes;}
static int -drm_est3_modes(struct drm_connector *connector, struct detailed_timing *timing) +drm_est3_modes(struct drm_connector *connector,
const struct detailed_timing *timing){ int i, j, m, modes = 0; struct drm_display_mode *mode;
u8 *est = ((u8 *)timing) + 6;
const u8 *est = ((const u8 *)timing) + 6; for (i = 0; i < 6; i++) { for (j = 7; j >= 0; j--) { @@ -2912,7 +2944,7 @@drm_est3_modes(struct drm_connector *connector, struct detailed_timing *timing) }
static void -do_established_modes(struct detailed_timing *timing, void *c) +do_established_modes(const struct detailed_timing *timing, void *c) { struct detailed_mode_closure *closure = c;
@@ -2931,7 +2963,8 @@ do_established_modes(struct detailed_timing *timing,
void *c)
- (defined above). Tease them out and add them to the global modes list.
*/ static int -add_established_modes(struct drm_connector *connector, struct edid *edid) +add_established_modes(struct drm_connector *connector,
const struct edid *edid){ struct drm_device *dev = connector->dev; unsigned long est_bits = edid->established_timings.t1 | @@ -2962,19 +2995,19 @@ add_established_modes(struct drm_connector *connector, struct edid *edid) }
static void -do_standard_modes(struct detailed_timing *timing, void *c) +do_standard_modes(const struct detailed_timing *timing, void *c) { struct detailed_mode_closure *closure = c;
struct detailed_non_pixel *data = &timing->data.other_data;
const struct detailed_non_pixel *data =- &timing->data.other_data; struct drm_connector *connector = closure->connector;
struct edid *edid = closure->edid;
const struct edid *edid = closure->edid; int i; if (!is_display_descriptor((const u8 *)timing, EDID_DETAIL_STD_MODES)) return; for (i = 0; i < 6; i++) {
struct std_timing *std = &data->data.timings[i];
const struct std_timing *std =&data->data.timings[i]; struct drm_display_mode *newmode;
newmode = drm_mode_std(connector, edid, std); @@-2994,7 +3027,8 @@ do_standard_modes(struct detailed_timing *timing, void
*c)
- GTF or CVT. Grab them from @edid and add them to the list.
*/ static int -add_standard_modes(struct drm_connector *connector, struct edid *edid) +add_standard_modes(struct drm_connector *connector,
const struct edid *edid){ int i, modes = 0; struct detailed_mode_closure closure = { @@ -3023,18 +3057,18 @@ add_standard_modes(struct drm_connector *connector, struct edid *edid) }
static int drm_cvt_modes(struct drm_connector *connector,
struct detailed_timing *timing)
const struct detailed_timing *timing){ int i, j, modes = 0; struct drm_display_mode *newmode; struct drm_device *dev = connector->dev;
struct cvt_timing *cvt; const int rates[] = { 60, 85, 75, 60, 50 }; const u8 empty[3] = { 0, 0, 0 }; for (i = 0; i < 4; i++) { int uninitialized_var(width), height;cvt = &(timing->data.other_data.data.cvt[i]);
const struct cvt_timing *cvt =&timing->data.other_data.data.cvt[i]; if (!memcmp(cvt->code, empty, 3)) continue;@@ -3072,7 +3106,7 @@ static int drm_cvt_modes(struct drm_connector *connector, }
static void -do_cvt_mode(struct detailed_timing *timing, void *c) +do_cvt_mode(const struct detailed_timing *timing, void *c) { struct detailed_mode_closure *closure = c;
@@ -3083,7 +3117,8 @@ do_cvt_mode(struct detailed_timing *timing, void *c) }
static int -add_cvt_modes(struct drm_connector *connector, struct edid *edid) +add_cvt_modes(struct drm_connector *connector,
const struct edid *edid){ struct detailed_mode_closure closure = { .connector = connector, @@ -3101,7 +3136,7 @@ add_cvt_modes(struct drm_connector *connector, struct edid *edid) static void fixup_detailed_cea_mode_clock(struct drm_display_mode *mode);
static void -do_detailed_mode(struct detailed_timing *timing, void *c) +do_detailed_mode(const struct detailed_timing *timing, void *c) { struct detailed_mode_closure *closure = c; struct drm_display_mode *newmode; @@ -3137,8 +3172,8 @@ do_detailed_mode(struct detailed_timing *timing, void *c)
- @quirks: quirks to apply
*/ static int -add_detailed_modes(struct drm_connector *connector, struct edid *edid,
u32 quirks)+add_detailed_modes(struct drm_connector *connector,
struct edid *edid, u32 quirks){ struct detailed_mode_closure closure = { .connector = connector, @@ -3173,9 +3208,10 @@ add_detailed_modes(struct drm_connector *connector, struct edid *edid, /*
- Search EDID for CEA extension block.
*/ -static u8 *drm_find_edid_extension(const struct edid *edid, int ext_id) +static const u8 *drm_find_edid_extension(const struct edid *edid,
int ext_id){
u8 *edid_ext = NULL;
const u8 *edid_ext = NULL; int i; /* No EDID or EDID extensions */ @@ -3184,7 +3220,7 @@static u8 *drm_find_edid_extension(const struct edid *edid, int ext_id)
/* Find CEA extension */ for (i = 0; i < edid->extensions; i++) {
edid_ext = (u8 *)edid + EDID_LENGTH * (i + 1);
edid_ext = (const u8 *)edid + EDID_LENGTH * (i + 1); if (edid_ext[0] == ext_id) break; }@@ -3196,19 +3232,19 @@ static u8 *drm_find_edid_extension(const struct edid *edid, int ext_id) }
-static u8 *drm_find_displayid_extension(const struct edid *edid) +static const u8 *drm_find_displayid_extension(const struct edid +*edid) { return drm_find_edid_extension(edid, DISPLAYID_EXT); }
-static u8 *drm_find_cea_extension(const struct edid *edid) +static const u8 *drm_find_cea_extension(const struct edid *edid) { int ret; int idx = 1; int length = EDID_LENGTH;
struct displayid_block *block;u8 *cea;u8 *displayid;
const struct displayid_block *block;const u8 *cea;const u8 *displayid; /* Look for a top level CEA extension block */ cea = drm_find_edid_extension(edid, CEA_EXT); @@ -4315,28+4351,30 @@ drm_parse_hdmi_vsdb_audio(struct drm_connector *connector, const u8 *db) }
static void -monitor_name(struct detailed_timing *t, void *data) +monitor_name(const struct detailed_timing *t, void *c) {
struct data_closure *closure = c;if (!is_display_descriptor((const u8 *)t, EDID_DETAIL_MONITOR_NAME)) return;
*(u8 **)data = t->data.other_data.data.str.str;
closure->data = t->data.other_data.data.str.str;}
static int get_monitor_name(struct edid *edid, char name[13]) {
char *edid_name = NULL;
struct data_closure closure = {}; int mnl; if (!edid || !name) return 0;
drm_for_each_detailed_block((u8 *)edid, monitor_name, &edid_name);for (mnl = 0; edid_name && mnl < 13; mnl++) {if (edid_name[mnl] == 0x0a)
drm_for_each_detailed_block((const u8 *)edid, monitor_name, &closure);for (mnl = 0; closure.data && mnl < 13; mnl++) {if (closure.data[mnl] == 0x0a) break;
name[mnl] = edid_name[mnl];
name[mnl] = closure.data[mnl]; } return mnl;@@ -4386,11 +4424,9 @@ static void clear_eld(struct drm_connector *connector) static void drm_edid_to_eld(struct drm_connector *connector, struct edid *edid) { uint8_t *eld = connector->eld;
u8 *cea;u8 *db;
const u8 *cea; int total_sad_count = 0; int mnl;
int dbl; clear_eld(connector);@@ -4425,8 +4461,8 @@ static void drm_edid_to_eld(struct drm_connector
*connector, struct edid *edid)
} for_each_cea_db(cea, i, start, end) {
db = &cea[i];dbl = cea_db_payload_len(db);
const u8 *db = &cea[i];int dbl = cea_db_payload_len(db); switch (cea_db_tag(db)) { int sad_count; @@ -4484,7 +4520,7 @@int drm_edid_to_sad(struct edid *edid, struct cea_sad **sads) { int count = 0; int i, start, end, dbl;
u8 *cea;
const u8 *cea; cea = drm_find_cea_extension(edid); if (!cea) {@@ -4503,7 +4539,7 @@ int drm_edid_to_sad(struct edid *edid, struct cea_sad
**sads)
} for_each_cea_db(cea, i, start, end) {
u8 *db = &cea[i];
const u8 *db = &cea[i]; if (cea_db_tag(db) == AUDIO_BLOCK) { int j;@@ -4514,7 +4550,7 @@ int drm_edid_to_sad(struct edid *edid, struct cea_sad
**sads)
if (!*sads) return -ENOMEM; for (j = 0; j < count; j++) {
u8 *sad = &db[1 + j * 3];
const u8 *sad = &db[1 + j * 3]; (*sads)[j].format = (sad[0] & 0x78) >> 3; (*sads)[j].channels = sad[0] & 0x7;@@ -4635,7 +4671,7 @@ EXPORT_SYMBOL(drm_av_sync_delay); */ bool drm_detect_hdmi_monitor(struct edid *edid) {
u8 *edid_ext;
const u8 *edid_ext; int i; int start_offset, end_offset;@@ -4673,7 +4709,7 @@ EXPORT_SYMBOL(drm_detect_hdmi_monitor); */ bool drm_detect_monitor_audio(struct edid *edid) {
u8 *edid_ext;
const u8 *edid_ext; int i, j; bool has_audio = false; int start_offset, end_offset; @@ -5017,13 +5053,13 @@ u32drm_add_display_info(struct drm_connector *connector, const struct edid *edi return quirks; }
-static int validate_displayid(u8 *displayid, int length, int idx) +static int validate_displayid(const u8 *displayid, int length, int +idx) { int i; u8 csum = 0;
struct displayid_hdr *base;
const struct displayid_hdr *base;
base = (struct displayid_hdr *)&displayid[idx];
base = (const struct displayid_hdr *)&displayid[idx]; DRM_DEBUG_KMS("base revision 0x%x, length %d, %d %d\n", base->rev, base->bytes, base->prod_id,base->ext_count); @@ -5041,7 +5077,7 @@ static int validate_displayid(u8 *displayid, int length, int idx) }
static struct drm_display_mode *drm_mode_displayid_detailed(struct
drm_device *dev,
struct displayid_detailed_timings_1 *timings)
const- struct displayid_detailed_timings_1 *timings)
{ struct drm_display_mode *mode; unsigned pixel_clock = (timings->pixel_clock[0] | @@ -5057,6 +5093,7 @@ static struct drm_display_mode
*drm_mode_displayid_detailed(struct drm_device *d
unsigned vsync_width = (timings->vsw[0] | timings->vsw[1] << 8) + 1; bool hsync_positive = (timings->hsync[1] >> 7) & 0x1; bool vsync_positive = (timings->vsync[1] >> 7) & 0x1;
mode = drm_mode_create(dev); if (!mode) return NULL;@@ -5086,9 +5123,10 @@ static struct drm_display_mode *drm_mode_displayid_detailed(struct drm_device *d }
static int add_displayid_detailed_1_modes(struct drm_connector *connector,
struct displayid_block *block)
const struct- displayid_block *block)
{
struct displayid_detailed_timing_block *det = (structdisplayid_detailed_timing_block *)block;
const struct displayid_detailed_timing_block *det =(const struct displayid_detailed_timing_block- *)block; int i; int num_timings; struct drm_display_mode *newmode; @@ -5099,7 +5137,7 @@
static int add_displayid_detailed_1_modes(struct drm_connector *connector,
num_timings = block->num_bytes / 20; for (i = 0; i < num_timings; i++) {
struct displayid_detailed_timings_1 *timings = &det->timings[i];
const struct displayid_detailed_timings_1 *timings =&det->timings[i];
newmode = drm_mode_displayid_detailed(connector->dev, timings); if (!newmode)@@ -5112,13 +5150,13 @@ static int add_displayid_detailed_1_modes(struct drm_connector *connector, }
static int add_displayid_detailed_modes(struct drm_connector *connector,
struct edid *edid)
const struct edid *edid){
u8 *displayid;
const u8 *displayid; int ret; int idx = 1; int length = EDID_LENGTH;
struct displayid_block *block;
const struct displayid_block *block; int num_modes = 0; displayid = drm_find_displayid_extension(edid);@@ -5720,9 +5758,10 @@ drm_hdmi_vendor_infoframe_from_display_mode(struct hdmi_vendor_infoframe *frame, EXPORT_SYMBOL(drm_hdmi_vendor_infoframe_from_display_mode);
static int drm_parse_tiled_block(struct drm_connector *connector,
struct displayid_block *block)
const struct displayid_block- *block)
{
struct displayid_tiled_block *tile = (struct displayid_tiled_block *)block;
const struct displayid_tiled_block *tile =(const struct displayid_tiled_block *)block; u16 w, h; u8 tile_v_loc, tile_h_loc; u8 num_v_tile, num_h_tile;@@ -5774,12 +5813,12 @@ static int drm_parse_tiled_block(struct drm_connector *connector, }
static int drm_parse_display_id(struct drm_connector *connector,
u8 *displayid, int length,
const u8 *displayid, int length, bool is_edid_extension) { /* if this is an EDID extension the first byte will be 0x70 */ int idx = 0;
struct displayid_block *block;
const struct displayid_block *block; int ret; if (is_edid_extension)@@ -5815,11 +5854,13 @@ static int drm_parse_display_id(struct drm_connector *connector, }
static void drm_get_displayid(struct drm_connector *connector,
struct edid *edid)
const struct edid *edid){
void *displayid = NULL;
const void *displayid; int ret;connector->has_tile = false;displayid = drm_find_displayid_extension(edid); if (!displayid) { /* drop reference to any tile group we had */ diff--git a/include/drm/drm_connector.h b/include/drm/drm_connector.h index 2113500b4075..c0f9ce3f4b24 100644 --- a/include/drm/drm_connector.h +++ b/include/drm/drm_connector.h @@ -1580,9 +1580,9 @@ struct drm_tile_group { };
struct drm_tile_group *drm_mode_create_tile_group(struct drm_device *dev,
char topology[8]);
const u8- topology[8]);
struct drm_tile_group *drm_mode_get_tile_group(struct drm_device *dev,
char topology[8]);
const u8- topology[8]);
void drm_mode_put_tile_group(struct drm_device *dev, struct drm_tile_group *tg);
-- 2.24.1
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
-- Ville Syrjälä Intel _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
From: Ville Syrjälä ville.syrjala@linux.intel.com
I'm curious if there are any bogus 18 byte descriptors around. Let's dump them out if we encounter them.
Not sure we'd actually want this, but at least I get to see if our CI has anything that hits this :)
Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com --- drivers/gpu/drm/drm_edid.c | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 8e76efe1654d..4d8303e56536 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -2202,6 +2202,12 @@ static bool is_display_descriptor(const u8 d[18], u8 tag) d[2] == 0x00 && d[3] == tag; }
+static bool is_any_display_descriptor(const u8 d[18]) +{ + return d[0] == 0x00 && d[1] == 0x00 && + d[2] == 0x00; +} + static bool is_detailed_timing_descriptor(const u8 d[18]) { return d[0] != 0x00 || d[1] != 0x00; @@ -2209,6 +2215,15 @@ static bool is_detailed_timing_descriptor(const u8 d[18])
typedef void detailed_cb(const struct detailed_timing *timing, void *closure);
+static void do_detailed_block(const u8 d[18], detailed_cb *cb, void *closure) +{ + if (!is_detailed_timing_descriptor(d) && + !is_any_display_descriptor(d)) + DRM_WARN("Unrecognized 18 byte descriptor: %*ph\n", 18, d); + + cb((const struct detailed_timing *)d, closure); +} + static void cea_for_each_detailed_block(const u8 *ext, detailed_cb *cb, void *closure) { @@ -2221,7 +2236,7 @@ cea_for_each_detailed_block(const u8 *ext, detailed_cb *cb, void *closure)
n = (127 - d) / 18; for (i = 0; i < n; i++) - cb((const struct detailed_timing *)(det_base + 18 * i), closure); + do_detailed_block(det_base + 18 * i, cb, closure); }
static void @@ -2234,7 +2249,7 @@ vtb_for_each_detailed_block(const u8 *ext, detailed_cb *cb, void *closure) return; /* unknown version */
for (i = 0; i < n; i++) - cb((const struct detailed_timing *)(det_base + 18 * i), closure); + do_detailed_block(det_base + 18 * i, cb, closure); }
static void @@ -2247,7 +2262,8 @@ drm_for_each_detailed_block(const u8 *raw_edid, detailed_cb *cb, void *closure) return;
for (i = 0; i < EDID_DETAILED_TIMINGS; i++) - cb(&(edid->detailed_timings[i]), closure); + do_detailed_block((const u8 *)&edid->detailed_timings[i], + cb, closure);
for (i = 1; i <= raw_edid[0x7e]; i++) { const u8 *ext = raw_edid + (i * EDID_LENGTH);
On Fri, Jan 24, 2020 at 3:03 PM Ville Syrjala ville.syrjala@linux.intel.com wrote:
From: Ville Syrjälä ville.syrjala@linux.intel.com
I'm curious if there are any bogus 18 byte descriptors around. Let's dump them out if we encounter them.
Not sure we'd actually want this, but at least I get to see if our CI has anything that hits this :)
Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com
Acked-by: Alex Deucher alexander.deucher@amd.com
drivers/gpu/drm/drm_edid.c | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 8e76efe1654d..4d8303e56536 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -2202,6 +2202,12 @@ static bool is_display_descriptor(const u8 d[18], u8 tag) d[2] == 0x00 && d[3] == tag; }
+static bool is_any_display_descriptor(const u8 d[18]) +{
return d[0] == 0x00 && d[1] == 0x00 &&d[2] == 0x00;+}
static bool is_detailed_timing_descriptor(const u8 d[18]) { return d[0] != 0x00 || d[1] != 0x00; @@ -2209,6 +2215,15 @@ static bool is_detailed_timing_descriptor(const u8 d[18])
typedef void detailed_cb(const struct detailed_timing *timing, void *closure);
+static void do_detailed_block(const u8 d[18], detailed_cb *cb, void *closure) +{
if (!is_detailed_timing_descriptor(d) &&!is_any_display_descriptor(d))DRM_WARN("Unrecognized 18 byte descriptor: %*ph\n", 18, d);cb((const struct detailed_timing *)d, closure);+}
static void cea_for_each_detailed_block(const u8 *ext, detailed_cb *cb, void *closure) { @@ -2221,7 +2236,7 @@ cea_for_each_detailed_block(const u8 *ext, detailed_cb *cb, void *closure)
n = (127 - d) / 18; for (i = 0; i < n; i++)
cb((const struct detailed_timing *)(det_base + 18 * i), closure);
do_detailed_block(det_base + 18 * i, cb, closure);}
static void @@ -2234,7 +2249,7 @@ vtb_for_each_detailed_block(const u8 *ext, detailed_cb *cb, void *closure) return; /* unknown version */
for (i = 0; i < n; i++)
cb((const struct detailed_timing *)(det_base + 18 * i), closure);
do_detailed_block(det_base + 18 * i, cb, closure);}
static void @@ -2247,7 +2262,8 @@ drm_for_each_detailed_block(const u8 *raw_edid, detailed_cb *cb, void *closure) return;
for (i = 0; i < EDID_DETAILED_TIMINGS; i++)
cb(&(edid->detailed_timings[i]), closure);
do_detailed_block((const u8 *)&edid->detailed_timings[i],cb, closure); for (i = 1; i <= raw_edid[0x7e]; i++) { const u8 *ext = raw_edid + (i * EDID_LENGTH);-- 2.24.1
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
-----Original Message----- From: Intel-gfx intel-gfx-bounces@lists.freedesktop.org On Behalf Of Alex Deucher Sent: Tuesday, January 28, 2020 4:09 AM To: Ville Syrjala ville.syrjala@linux.intel.com Cc: Intel Graphics Development intel-gfx@lists.freedesktop.org; Maling list - DRI developers dri-devel@lists.freedesktop.org Subject: Re: [Intel-gfx] [PATCH 8/8] drm/edid: Dump bogus 18 byte descriptors
On Fri, Jan 24, 2020 at 3:03 PM Ville Syrjala ville.syrjala@linux.intel.com wrote:
From: Ville Syrjälä ville.syrjala@linux.intel.com
I'm curious if there are any bogus 18 byte descriptors around. Let's dump them out if we encounter them.
Not sure we'd actually want this, but at least I get to see if our CI has anything that hits this :)
Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com
Acked-by: Alex Deucher alexander.deucher@amd.com
Looks good to me as well. Reviewed-by: Uma Shankar uma.shankar@intel.com
drivers/gpu/drm/drm_edid.c | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 8e76efe1654d..4d8303e56536 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -2202,6 +2202,12 @@ static bool is_display_descriptor(const u8 d[18], u8
tag)
d[2] == 0x00 && d[3] == tag; }+static bool is_any_display_descriptor(const u8 d[18]) {
return d[0] == 0x00 && d[1] == 0x00 &&d[2] == 0x00;+}
static bool is_detailed_timing_descriptor(const u8 d[18]) { return d[0] != 0x00 || d[1] != 0x00; @@ -2209,6 +2215,15 @@ static bool is_detailed_timing_descriptor(const u8 d[18])
typedef void detailed_cb(const struct detailed_timing *timing, void *closure);
+static void do_detailed_block(const u8 d[18], detailed_cb *cb, void +*closure) {
if (!is_detailed_timing_descriptor(d) &&!is_any_display_descriptor(d))DRM_WARN("Unrecognized 18 byte descriptor: %*ph\n",+18, d);
cb((const struct detailed_timing *)d, closure); }static void cea_for_each_detailed_block(const u8 *ext, detailed_cb *cb, void *closure) { @@ -2221,7 +2236,7 @@ cea_for_each_detailed_block(const u8 *ext, detailed_cb *cb, void *closure)
n = (127 - d) / 18; for (i = 0; i < n; i++)
cb((const struct detailed_timing *)(det_base + 18 * i), closure);
do_detailed_block(det_base + 18 * i, cb, closure);}
static void @@ -2234,7 +2249,7 @@ vtb_for_each_detailed_block(const u8 *ext, detailed_cb
*cb, void *closure)
return; /* unknown version */ for (i = 0; i < n; i++)
cb((const struct detailed_timing *)(det_base + 18 * i), closure);
do_detailed_block(det_base + 18 * i, cb, closure);}
static void @@ -2247,7 +2262,8 @@ drm_for_each_detailed_block(const u8 *raw_edid,
detailed_cb *cb, void *closure)
return; for (i = 0; i < EDID_DETAILED_TIMINGS; i++)
cb(&(edid->detailed_timings[i]), closure);
do_detailed_block((const u8 *)&edid->detailed_timings[i],cb, closure); for (i = 1; i <= raw_edid[0x7e]; i++) { const u8 *ext = raw_edid + (i * EDID_LENGTH);-- 2.24.1
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Fri, Jan 24, 2020 at 3:03 PM Ville Syrjala ville.syrjala@linux.intel.com wrote:
From: Ville Syrjälä ville.syrjala@linux.intel.com
CEA-861 says : "d = offset for the byte following the reserved data block. If no data is provided in the reserved data block, then d=4. If no DTDs are provided, then d=0."
So let's not look for DTDs when d==0. In fact let's just make that <4 since those values would just mean that he DTDs overlap the block header. And let's also check that d isn't so big as to declare the descriptors to live past the block end, although the code does already survive that case as we'd just end up with a negative number of descriptors and the loop would not do anything.
Cc: Allen Chen allen.chen@ite.com.tw Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com
Acked-by: Alex Deucher alexander.deucher@amd.com
drivers/gpu/drm/drm_edid.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 99769d6c9f84..1b6e544cf5c7 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -2201,10 +2201,13 @@ typedef void detailed_cb(struct detailed_timing *timing, void *closure); static void cea_for_each_detailed_block(u8 *ext, detailed_cb *cb, void *closure) {
int i, n = 0;
int i, n; u8 d = ext[0x02]; u8 *det_base = ext + d;if (d < 4 || d > 127)return;n = (127 - d) / 18; for (i = 0; i < n; i++) cb((struct detailed_timing *)(det_base + 18 * i), closure);-- 2.24.1
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
-----Original Message----- From: dri-devel dri-devel-bounces@lists.freedesktop.org On Behalf Of Alex Deucher Sent: Tuesday, January 28, 2020 4:04 AM To: Ville Syrjala ville.syrjala@linux.intel.com Cc: Allen Chen allen.chen@ite.com.tw; Intel Graphics Development <intel- gfx@lists.freedesktop.org>; Maling list - DRI developers <dri- devel@lists.freedesktop.org> Subject: Re: [PATCH 1/8] drm/edid: Check the number of detailed timing descriptors in the CEA ext block
On Fri, Jan 24, 2020 at 3:03 PM Ville Syrjala ville.syrjala@linux.intel.com wrote:
From: Ville Syrjälä ville.syrjala@linux.intel.com
CEA-861 says : "d = offset for the byte following the reserved data block. If no data is provided in the reserved data block, then d=4. If no DTDs are provided, then d=0."
So let's not look for DTDs when d==0. In fact let's just make that <4 since those values would just mean that he DTDs overlap the block header. And let's also check that d isn't so big as to declare the descriptors to live past the block end, although the code does already survive that case as we'd just end up with a negative number of descriptors and the loop would not do anything.
Cc: Allen Chen allen.chen@ite.com.tw Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com
Acked-by: Alex Deucher alexander.deucher@amd.com
Looks good to me as well. Reviewed-by: Uma Shankar uma.shankar@intel.com
drivers/gpu/drm/drm_edid.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 99769d6c9f84..1b6e544cf5c7 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -2201,10 +2201,13 @@ typedef void detailed_cb(struct detailed_timing *timing, void *closure); static void cea_for_each_detailed_block(u8 *ext, detailed_cb *cb, void *closure) {
int i, n = 0;
int i, n; u8 d = ext[0x02]; u8 *det_base = ext + d;if (d < 4 || d > 127)return;n = (127 - d) / 18; for (i = 0; i < n; i++) cb((struct detailed_timing *)(det_base + 18 * i),closure);
2.24.1
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
On Fri, Jan 24, 2020 at 10:02:24PM +0200, Ville Syrjala wrote:
From: Ville Syrjälä ville.syrjala@linux.intel.com
CEA-861 says : "d = offset for the byte following the reserved data block. If no data is provided in the reserved data block, then d=4. If no DTDs are provided, then d=0."
So let's not look for DTDs when d==0. In fact let's just make that <4 since those values would just mean that he DTDs overlap the block header. And let's also check that d isn't so big as to declare the descriptors to live past the block end, although the code does already survive that case as we'd just end up with a negative number of descriptors and the loop would not do anything.
Cc: Allen Chen allen.chen@ite.com.tw Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com
Hm I think edid parsing is like the perfect use-case for some in-kernel unit tests ... In case anyone feels motivated? -Daniel
drivers/gpu/drm/drm_edid.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 99769d6c9f84..1b6e544cf5c7 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -2201,10 +2201,13 @@ typedef void detailed_cb(struct detailed_timing *timing, void *closure); static void cea_for_each_detailed_block(u8 *ext, detailed_cb *cb, void *closure) {
- int i, n = 0;
int i, n; u8 d = ext[0x02]; u8 *det_base = ext + d;
if (d < 4 || d > 127)
return;n = (127 - d) / 18; for (i = 0; i < n; i++) cb((struct detailed_timing *)(det_base + 18 * i), closure);
-- 2.24.1
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
On Tue, Jan 28, 2020 at 04:17:58PM +0100, Daniel Vetter wrote:
On Fri, Jan 24, 2020 at 10:02:24PM +0200, Ville Syrjala wrote:
From: Ville Syrjälä ville.syrjala@linux.intel.com
CEA-861 says : "d = offset for the byte following the reserved data block. If no data is provided in the reserved data block, then d=4. If no DTDs are provided, then d=0."
So let's not look for DTDs when d==0. In fact let's just make that <4 since those values would just mean that he DTDs overlap the block header. And let's also check that d isn't so big as to declare the descriptors to live past the block end, although the code does already survive that case as we'd just end up with a negative number of descriptors and the loop would not do anything.
Cc: Allen Chen allen.chen@ite.com.tw Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com
Hm I think edid parsing is like the perfect use-case for some in-kernel unit tests ... In case anyone feels motivated?
Another idea I've been putting off is simply shoving the parser into userspace. Kinda looks like with fork_usermode_blob() we could embed the executable into the kernel/module and thus avoid the problem of actually shipping the binary somehow.
On Tue, Jan 28, 2020 at 5:15 PM Ville Syrjälä ville.syrjala@linux.intel.com wrote:
On Tue, Jan 28, 2020 at 04:17:58PM +0100, Daniel Vetter wrote:
On Fri, Jan 24, 2020 at 10:02:24PM +0200, Ville Syrjala wrote:
From: Ville Syrjälä ville.syrjala@linux.intel.com
CEA-861 says : "d = offset for the byte following the reserved data block. If no data is provided in the reserved data block, then d=4. If no DTDs are provided, then d=0."
So let's not look for DTDs when d==0. In fact let's just make that <4 since those values would just mean that he DTDs overlap the block header. And let's also check that d isn't so big as to declare the descriptors to live past the block end, although the code does already survive that case as we'd just end up with a negative number of descriptors and the loop would not do anything.
Cc: Allen Chen allen.chen@ite.com.tw Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com
Hm I think edid parsing is like the perfect use-case for some in-kernel unit tests ... In case anyone feels motivated?
Another idea I've been putting off is simply shoving the parser into userspace. Kinda looks like with fork_usermode_blob() we could embed the executable into the kernel/module and thus avoid the problem of actually shipping the binary somehow.
"How to run unit tests without losing hair" is essentially what kunit tries to solve. I think we should cut over to that (it's merged now, should be good enough for at least the edid parser, mocking stuff isn't there there), and then make sure CI runs that stuff for us. Then we could convert over at least the unit test like selftests eventually too. -Daniel
On Tue, Jan 28, 2020 at 05:18:34PM +0100, Daniel Vetter wrote:
On Tue, Jan 28, 2020 at 5:15 PM Ville Syrjälä ville.syrjala@linux.intel.com wrote:
On Tue, Jan 28, 2020 at 04:17:58PM +0100, Daniel Vetter wrote:
On Fri, Jan 24, 2020 at 10:02:24PM +0200, Ville Syrjala wrote:
From: Ville Syrjälä ville.syrjala@linux.intel.com
CEA-861 says : "d = offset for the byte following the reserved data block. If no data is provided in the reserved data block, then d=4. If no DTDs are provided, then d=0."
So let's not look for DTDs when d==0. In fact let's just make that <4 since those values would just mean that he DTDs overlap the block header. And let's also check that d isn't so big as to declare the descriptors to live past the block end, although the code does already survive that case as we'd just end up with a negative number of descriptors and the loop would not do anything.
Cc: Allen Chen allen.chen@ite.com.tw Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com
Hm I think edid parsing is like the perfect use-case for some in-kernel unit tests ... In case anyone feels motivated?
Another idea I've been putting off is simply shoving the parser into userspace. Kinda looks like with fork_usermode_blob() we could embed the executable into the kernel/module and thus avoid the problem of actually shipping the binary somehow.
"How to run unit tests without losing hair" is essentially what kunit tries to solve. I think we should cut over to that (it's merged now, should be good enough for at least the edid parser, mocking stuff isn't there there), and then make sure CI runs that stuff for us. Then we could convert over at least the unit test like selftests eventually too.
I meant run it in userspace *always*, not just for testing.
dri-devel@lists.freedesktop.org
-
Alex Deucher -
Daniel Vetter -
Shankar, Uma -
Ville Syrjala -
Ville Syrjälä