[PATCH/RFC 1/5] drm/edid: Reorganise the DisplayID parsing code
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Wed Aug 21 18:50:01 UTC 2019
This change started as an attempt to remove the forward declaration of
validate_displayid(), and ended up reorganising the DisplayID parsing
code to fix serial intertwined issues.
The drm_parse_display_id() function, which parses a DisplayID block, is
made aware of whether the DisplayID comes from an EDID extension block
or is direct DisplayID data. This is a layering violation, this should
be handled in the caller. Similarly, the validate_displayid() function
should not take an offset in the data buffer, it should also receive a
pointer to the DisplayID data.
In order to simplify the callers of drm_find_displayid_extension(), the
function is modified to return a pointer to the DisplayID data, not to
the EDID extension block. The pointer can then be used directly for
validation and parsing, without the need to add an offset.
For consistency reasons the validate_displayid() function is renamed to
drm_displayid_is_valid() and modified to return a bool, and the
drm_parse_display_id() renamed to drm_parse_displayid().
Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
---
drivers/gpu/drm/drm_edid.c | 104 ++++++++++++++++++-------------------
1 file changed, 52 insertions(+), 52 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 82a4ceed3fcf..7c6bc5183b60 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -1342,7 +1342,6 @@ MODULE_PARM_DESC(edid_fixup,
static void drm_get_displayid(struct drm_connector *connector,
struct edid *edid);
-static int validate_displayid(u8 *displayid, int length, int idx);
static int drm_edid_block_checksum(const u8 *raw_edid)
{
@@ -2927,17 +2926,49 @@ static u8 *drm_find_edid_extension(const struct edid *edid, int ext_id)
return edid_ext;
}
+static bool drm_displayid_is_valid(u8 *displayid, unsigned int length)
+{
+ struct displayid_hdr *base;
+ u8 csum = 0;
+ int i;
+
+ base = (struct displayid_hdr *)displayid;
+
+ DRM_DEBUG_KMS("base revision 0x%x, length %d, %d %d\n",
+ base->rev, base->bytes, base->prod_id, base->ext_count);
+
+ if (base->bytes + 5 > length)
+ return false;
+
+ for (i = 0; i <= base->bytes + 5; i++)
+ csum += displayid[i];
+
+ if (csum) {
+ DRM_NOTE("DisplayID checksum invalid, remainder is %d\n", csum);
+ return false;
+ }
+
+ return true;
+}
static u8 *drm_find_displayid_extension(const struct edid *edid)
{
- return drm_find_edid_extension(edid, DISPLAYID_EXT);
+ u8 *ext = drm_find_edid_extension(edid, DISPLAYID_EXT);
+
+ if (!ext)
+ return NULL;
+
+ /*
+ * Skip the EDID extension block tag number to return the DisplayID
+ * data.
+ */
+ return ext + 1;
}
static u8 *drm_find_cea_extension(const struct edid *edid)
{
- int ret;
- int idx = 1;
- int length = EDID_LENGTH;
+ int idx;
+ int length = EDID_LENGTH - 1;
struct displayid_block *block;
u8 *cea;
u8 *displayid;
@@ -2952,11 +2983,10 @@ static u8 *drm_find_cea_extension(const struct edid *edid)
if (!displayid)
return NULL;
- ret = validate_displayid(displayid, length, idx);
- if (ret)
+ if (!drm_displayid_is_valid(displayid, length))
return NULL;
- idx += sizeof(struct displayid_hdr);
+ idx = sizeof(struct displayid_hdr);
for_each_displayid_db(displayid, block, idx, length) {
if (block->tag == DATA_BLOCK_CTA) {
cea = (u8 *)block;
@@ -4711,29 +4741,6 @@ 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)
-{
- int i;
- u8 csum = 0;
- struct displayid_hdr *base;
-
- base = (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);
-
- if (base->bytes + 5 > length - idx)
- return -EINVAL;
- for (i = idx; i <= base->bytes + 5; i++) {
- csum += displayid[i];
- }
- if (csum) {
- DRM_NOTE("DisplayID checksum invalid, remainder is %d\n", csum);
- return -EINVAL;
- }
- return 0;
-}
-
static struct drm_display_mode *drm_mode_displayid_detailed(struct drm_device *dev,
struct displayid_detailed_timings_1 *timings)
{
@@ -4809,9 +4816,8 @@ static int add_displayid_detailed_modes(struct drm_connector *connector,
struct edid *edid)
{
u8 *displayid;
- int ret;
- int idx = 1;
- int length = EDID_LENGTH;
+ int idx;
+ int length = EDID_LENGTH - 1;
struct displayid_block *block;
int num_modes = 0;
@@ -4819,11 +4825,10 @@ static int add_displayid_detailed_modes(struct drm_connector *connector,
if (!displayid)
return 0;
- ret = validate_displayid(displayid, length, idx);
- if (ret)
+ if (!drm_displayid_is_valid(displayid, length))
return 0;
- idx += sizeof(struct displayid_hdr);
+ idx = sizeof(struct displayid_hdr);
for_each_displayid_db(displayid, block, idx, length) {
switch (block->tag) {
case DATA_BLOCK_TYPE_1_DETAILED_TIMING:
@@ -5424,23 +5429,17 @@ static int drm_parse_tiled_block(struct drm_connector *connector,
return 0;
}
-static int drm_parse_display_id(struct drm_connector *connector,
- u8 *displayid, int length,
- bool is_edid_extension)
+static int drm_parse_displayid(struct drm_connector *connector,
+ u8 *displayid, int length)
{
- /* if this is an EDID extension the first byte will be 0x70 */
- int idx = 0;
struct displayid_block *block;
+ int idx;
int ret;
- if (is_edid_extension)
- idx = 1;
+ if (!drm_displayid_is_valid(displayid, length))
+ return -EINVAL;
- ret = validate_displayid(displayid, length, idx);
- if (ret)
- return ret;
-
- idx += sizeof(struct displayid_hdr);
+ idx = sizeof(struct displayid_hdr);
for_each_displayid_db(displayid, block, idx, length) {
DRM_DEBUG_KMS("block id 0x%x, rev %d, len %d\n",
block->tag, block->rev, block->num_bytes);
@@ -5468,8 +5467,9 @@ static int drm_parse_display_id(struct drm_connector *connector,
static void drm_get_displayid(struct drm_connector *connector,
struct edid *edid)
{
- void *displayid = NULL;
+ void *displayid;
int ret;
+
connector->has_tile = false;
displayid = drm_find_displayid_extension(edid);
if (!displayid) {
@@ -5477,16 +5477,16 @@ static void drm_get_displayid(struct drm_connector *connector,
goto out_drop_ref;
}
- ret = drm_parse_display_id(connector, displayid, EDID_LENGTH, true);
+ ret = drm_parse_displayid(connector, displayid, EDID_LENGTH - 1);
if (ret < 0)
goto out_drop_ref;
if (!connector->has_tile)
goto out_drop_ref;
return;
+
out_drop_ref:
if (connector->tile_group) {
drm_mode_put_tile_group(connector->dev, connector->tile_group);
connector->tile_group = NULL;
}
- return;
}
--
Regards,
Laurent Pinchart
More information about the dri-devel
mailing list