Re: [RFC v1 3/6] drm/displayid: add new displayid section/block iterators

On Tue, Mar 09, 2021 at 03:54:11PM +0200, Jani Nikula wrote:

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.

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..88070267aac9 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(struct displayid_block) <= iter->length &&

•    iter->idx + sizeof(struct displayid_block) + block->num_bytes <= iter->length &&
  

sizeof(*block) perhaps

•    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(struct displayid_block) + block->num_bytes;
  

ditto

Looks like this should do the same thing the current thing does, or at least I couldn't immediately spot nay differences.

Reviewed-by: Ville Syrjälä ville.syrjala@linux.intel.com

• 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

2.20.1

---

dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel