diff mbox series

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

Message ID d0fd3a830b0ebc082250fb6c1381bb8383715263.1615297748.git.jani.nikula@intel.com (mailing list archive)
State New, archived
Headers show
Series drm/edid: overhaul displayid iterator | expand

Commit Message

Jani Nikula March 9, 2021, 1:54 p.m. UTC 
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(+)

Comments

Ville Syrjala March 10, 2021, 7:10 p.m. UTC | #1 
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
diff mbox series

Patch

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 &&
+	    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;
+
+		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