If there's no need to change it, it should be const. There's more to be done, but start off with changes that make follow-up work easier. No functional changes.

Reviewed-by: Ville Syrjälä ville.syrjala@linux.intel.com Signed-off-by: Jani Nikula jani.nikula@intel.com --- drivers/gpu/drm/drm_edid.c | 58 ++++++++++++++++++------------------- include/drm/drm_displayid.h | 4 +-- 2 files changed, 31 insertions(+), 31 deletions(-)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index c2bbe7bee7b6..d510b827a1f8 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -1585,7 +1585,7 @@ module_param_named(edid_fixup, edid_fixup, int, 0400); MODULE_PARM_DESC(edid_fixup, "Minimum number of valid EDID header bytes (0-8, default 6)");

-static int validate_displayid(u8 *displayid, int length, int idx); +static int validate_displayid(const u8 *displayid, int length, int idx);

static int drm_edid_block_checksum(const u8 *raw_edid) { @@ -3241,10 +3241,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, int *ext_index) +static const u8 *drm_find_edid_extension(const struct edid *edid, + int ext_id, int *ext_index) { - u8 *edid_ext = NULL; + const u8 *edid_ext = NULL; int i;

/* No EDID or EDID extensions */ @@ -3253,7 +3253,7 @@ static u8 *drm_find_edid_extension(const struct edid *edid,

/* Find CEA extension */ for (i = *ext_index; 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; } @@ -3267,12 +3267,12 @@ static u8 *drm_find_edid_extension(const struct edid *edid, }

-static u8 *drm_find_displayid_extension(const struct edid *edid, - int *length, int *idx, - int *ext_index) +static const u8 *drm_find_displayid_extension(const struct edid *edid, + int *length, int *idx, + int *ext_index) { - u8 *displayid = drm_find_edid_extension(edid, DISPLAYID_EXT, ext_index); - struct displayid_hdr *base; + const u8 *displayid = drm_find_edid_extension(edid, DISPLAYID_EXT, ext_index); + const struct displayid_hdr *base; int ret;

if (!displayid) @@ -3286,18 +3286,18 @@ static u8 *drm_find_displayid_extension(const struct edid *edid, if (ret) return NULL;

- base = (struct displayid_hdr *)&displayid[*idx]; + base = (const struct displayid_hdr *)&displayid[*idx]; *length = *idx + sizeof(*base) + base->bytes;

return displayid; }

-static u8 *drm_find_cea_extension(const struct edid *edid) +static const u8 *drm_find_cea_extension(const struct edid *edid) { int length, idx; - struct displayid_block *block; - u8 *cea; - u8 *displayid; + const struct displayid_block *block; + const u8 *cea; + const u8 *displayid; int ext_index;

/* Look for a top level CEA extension block */ @@ -3318,7 +3318,7 @@ static u8 *drm_find_cea_extension(const struct edid *edid) idx += sizeof(struct displayid_hdr); for_each_displayid_db(displayid, block, idx, length) { if (block->tag == DATA_BLOCK_CTA) - return (u8 *)block; + return (const u8 *)block; } }

@@ -4503,8 +4503,8 @@ 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; + const u8 *db; int total_sad_count = 0; int mnl; int dbl; @@ -4600,7 +4600,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) { @@ -4619,7 +4619,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; @@ -4631,7 +4631,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; @@ -4755,7 +4755,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;

@@ -4793,7 +4793,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; @@ -5287,13 +5287,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, dispid_length; 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); @@ -5359,7 +5359,7 @@ 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; int i; @@ -5387,9 +5387,9 @@ static int add_displayid_detailed_1_modes(struct drm_connector *connector, static int add_displayid_detailed_modes(struct drm_connector *connector, struct edid *edid) { - u8 *displayid; + const u8 *displayid; int length, idx; - struct displayid_block *block; + const struct displayid_block *block; int num_modes = 0; int ext_index = 0;

diff --git a/include/drm/drm_displayid.h b/include/drm/drm_displayid.h index 77941efb5426..f141c0eff083 100644 --- a/include/drm/drm_displayid.h +++ b/include/drm/drm_displayid.h @@ -93,11 +93,11 @@ struct displayid_detailed_timing_block { };

#define for_each_displayid_db(displayid, block, idx, length) \ - for ((block) = (struct displayid_block *)&(displayid)[idx]; \ + for ((block) = (const struct displayid_block *)&(displayid)[idx]; \ (idx) + sizeof(struct displayid_block) <= (length) && \ (idx) + sizeof(struct displayid_block) + (block)->num_bytes <= (length) && \ (block)->num_bytes > 0; \ (idx) += sizeof(struct displayid_block) + (block)->num_bytes, \ - (block) = (struct displayid_block *)&(displayid)[idx]) + (block) = (const struct displayid_block *)&(displayid)[idx])

#endif

Iterating DisplayID blocks across sections (in EDID extensions) is unnecessarily complicated for the caller. Implement DisplayID iterators to go through all blocks in all sections.

Usage example:

const struct displayid_block *block; struct displayid_iter iter;

displayid_iter_edid_begin(edid, &iter); displayid_iter_for_each(block, &iter) { /* operate on block */ } displayid_iter_end(&iter);

When DisplayID is stored in EDID extensions, the DisplayID sections map to extensions as described in VESA DisplayID v1.3 Appendix B: DisplayID as an EDID Extension. This is implemented here.

When DisplayID is stored in its dedicated DDC device 0xA4, according to VESA E-DDC v1.3, different rules apply for the structure. This is not implemented here, as we don't currently use it, but the idea is you'd have a different call for beginning the iteration, for example simply:

displayid_iter_begin(displayid, &iter);

instead of displayid_iter_edid_begin(), and everything else would be hidden away in the iterator functions.

v2: - sizeof(struct displayid_block) -> sizeof(*block) (Ville) - remove __ prefix from displayid_iter_block

Reviewed-by: Ville Syrjälä ville.syrjala@linux.intel.com Signed-off-by: Jani Nikula jani.nikula@intel.com --- drivers/gpu/drm/drm_displayid.c | 74 +++++++++++++++++++++++++++++++++ include/drm/drm_displayid.h | 18 ++++++++ 2 files changed, 92 insertions(+)

diff --git a/drivers/gpu/drm/drm_displayid.c b/drivers/gpu/drm/drm_displayid.c index 908bbe6feb61..5b809bb9fecc 100644 --- a/drivers/gpu/drm/drm_displayid.c +++ b/drivers/gpu/drm/drm_displayid.c @@ -57,3 +57,77 @@ const u8 *drm_find_displayid_extension(const struct edid *edid,

return displayid; } + +void displayid_iter_edid_begin(const struct edid *edid, + struct displayid_iter *iter) +{ + memset(iter, 0, sizeof(*iter)); + + iter->edid = edid; +} + +static const struct displayid_block * +displayid_iter_block(const struct displayid_iter *iter) +{ + const struct displayid_block *block; + + if (!iter->section) + return NULL; + + block = (const struct displayid_block *)&iter->section[iter->idx]; + + if (iter->idx + sizeof(*block) <= iter->length && + iter->idx + sizeof(*block) + block->num_bytes <= iter->length && + block->num_bytes > 0) + return block; + + return NULL; +} + +const struct displayid_block * +__displayid_iter_next(struct displayid_iter *iter) +{ + const struct displayid_block *block; + + if (!iter->edid) + return NULL; + + if (iter->section) { + /* current block should always be valid */ + block = displayid_iter_block(iter); + if (WARN_ON(!block)) { + iter->section = NULL; + iter->edid = NULL; + return NULL; + } + + /* next block in section */ + iter->idx += sizeof(*block) + block->num_bytes; + + block = displayid_iter_block(iter); + if (block) + return block; + } + + for (;;) { + iter->section = drm_find_displayid_extension(iter->edid, + &iter->length, + &iter->idx, + &iter->ext_index); + if (!iter->section) { + iter->edid = NULL; + return NULL; + } + + iter->idx += sizeof(struct displayid_hdr); + + block = displayid_iter_block(iter); + if (block) + return block; + } +} + +void displayid_iter_end(struct displayid_iter *iter) +{ + memset(iter, 0, sizeof(*iter)); +} diff --git a/include/drm/drm_displayid.h b/include/drm/drm_displayid.h index 3c6db22a518a..27e06c98db17 100644 --- a/include/drm/drm_displayid.h +++ b/include/drm/drm_displayid.h @@ -108,4 +108,22 @@ const u8 *drm_find_displayid_extension(const struct edid *edid, int *length, int *idx, int *ext_index);

+/* DisplayID iteration */ +struct displayid_iter { + const struct edid *edid; + + const u8 *section; + int length; + int idx; + int ext_index; +}; + +void displayid_iter_edid_begin(const struct edid *edid, + struct displayid_iter *iter); +const struct displayid_block * +__displayid_iter_next(struct displayid_iter *iter); +#define displayid_iter_for_each(__block, __iter) \ + while (((__block) = __displayid_iter_next(__iter))) +void displayid_iter_end(struct displayid_iter *iter); + #endif

Neatly reduce displayid boilerplate in code. No functional changes.

Reviewed-by: Ville Syrjälä ville.syrjala@linux.intel.com Signed-off-by: Jani Nikula jani.nikula@intel.com --- drivers/gpu/drm/drm_edid.c | 25 +++++++++---------------- 1 file changed, 9 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index fbaa7d679cb2..4526e2557dca 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -3266,35 +3266,28 @@ const u8 *drm_find_edid_extension(const struct edid *edid,

static const u8 *drm_find_cea_extension(const struct edid *edid) { - int length, idx; const struct displayid_block *block; + struct displayid_iter iter; const u8 *cea; - const u8 *displayid; - int ext_index; + int ext_index = 0;

/* Look for a top level CEA extension block */ /* FIXME: make callers iterate through multiple CEA ext blocks? */ - ext_index = 0; cea = drm_find_edid_extension(edid, CEA_EXT, &ext_index); if (cea) return cea;

/* CEA blocks can also be found embedded in a DisplayID block */ - ext_index = 0; - for (;;) { - displayid = drm_find_displayid_extension(edid, &length, &idx, - &ext_index); - if (!displayid) - return NULL; - - idx += sizeof(struct displayid_hdr); - for_each_displayid_db(displayid, block, idx, length) { - if (block->tag == DATA_BLOCK_CTA) - return (const u8 *)block; + displayid_iter_edid_begin(edid, &iter); + displayid_iter_for_each(block, &iter) { + if (block->tag == DATA_BLOCK_CTA) { + cea = (const u8 *)block; + break; } } + displayid_iter_end(&iter);

- return NULL; + return cea; }

static __always_inline const struct drm_display_mode *cea_mode_for_vic(u8 vic)

The DisplayID specifications explicitly call out 0 as a valid payload length for data blocks. The mere presence of a data block, or the information coded in the block specific data (bits 7:3 in offset 1), may be enough to convey the necessary information.

Signed-off-by: Jani Nikula jani.nikula@intel.com --- drivers/gpu/drm/drm_displayid.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_displayid.c b/drivers/gpu/drm/drm_displayid.c index 902ff6114b68..e0b9e58a9dc8 100644 --- a/drivers/gpu/drm/drm_displayid.c +++ b/drivers/gpu/drm/drm_displayid.c @@ -77,8 +77,7 @@ displayid_iter_block(const struct displayid_iter *iter) block = (const struct displayid_block *)&iter->section[iter->idx];

if (iter->idx + sizeof(*block) <= iter->length && - iter->idx + sizeof(*block) + block->num_bytes <= iter->length && - block->num_bytes > 0) + iter->idx + sizeof(*block) + block->num_bytes <= iter->length) return block;

return NULL;

Avoid any confusion with High Dynamic Range. No functional changes.

Signed-off-by: Jani Nikula jani.nikula@intel.com --- drivers/gpu/drm/drm_displayid.c | 10 +++++----- include/drm/drm_displayid.h | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/drm_displayid.c b/drivers/gpu/drm/drm_displayid.c index e0b9e58a9dc8..32da557b960f 100644 --- a/drivers/gpu/drm/drm_displayid.c +++ b/drivers/gpu/drm/drm_displayid.c @@ -11,9 +11,9 @@ static int validate_displayid(const u8 *displayid, int length, int idx) { int i, dispid_length; u8 csum = 0; - const struct displayid_hdr *base; + const struct displayid_header *base;

- base = (const struct displayid_hdr *)&displayid[idx]; + base = (const struct displayid_header *)&displayid[idx];

DRM_DEBUG_KMS("base revision 0x%x, length %d, %d %d\n", base->rev, base->bytes, base->prod_id, base->ext_count); @@ -38,7 +38,7 @@ static const u8 *drm_find_displayid_extension(const struct edid *edid, int *ext_index) { const u8 *displayid = drm_find_edid_extension(edid, DISPLAYID_EXT, ext_index); - const struct displayid_hdr *base; + const struct displayid_header *base; int ret;

if (!displayid) @@ -52,7 +52,7 @@ static const u8 *drm_find_displayid_extension(const struct edid *edid, if (ret) return NULL;

- base = (const struct displayid_hdr *)&displayid[*idx]; + base = (const struct displayid_header *)&displayid[*idx]; *length = *idx + sizeof(*base) + base->bytes;

return displayid; @@ -118,7 +118,7 @@ __displayid_iter_next(struct displayid_iter *iter) return NULL; }

- iter->idx += sizeof(struct displayid_hdr); + iter->idx += sizeof(struct displayid_header);

block = displayid_iter_block(iter); if (block) diff --git a/include/drm/drm_displayid.h b/include/drm/drm_displayid.h index 10ee863f1734..ec64d141f578 100644 --- a/include/drm/drm_displayid.h +++ b/include/drm/drm_displayid.h @@ -56,7 +56,7 @@ struct edid; #define PRODUCT_TYPE_REPEATER 5 #define PRODUCT_TYPE_DIRECT_DRIVE 6

-struct displayid_hdr { +struct displayid_header { u8 rev; u8 bytes; u8 prod_id;