On Wed, 12 Jun 2019, Andres Rodriguez <andresx7@xxxxxxxxx> wrote:
> DisplayID blocks allow embedding of CEA blocks. The payloads are
> identical to traditional top level CEA extension blocks, but the header
> is slightly different.
>
> This change allows the CEA parser to find a CEA block inside a DisplayID
> block. Additionally, it adds support for parsing the embedded CTA
> header. No further changes are necessary due to payload parity.
>
> This change enables audio support for the Valve Index HMD.
>
> Signed-off-by: Andres Rodriguez <andresx7@xxxxxxxxx>
> ---
> drivers/gpu/drm/drm_edid.c | 79 +++++++++++++++++++++++++++++++++----
> include/drm/drm_displayid.h | 1 +
> 2 files changed, 72 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 649cfd8b4200..3e71c6ae48d4 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -1339,6 +1339,8 @@ MODULE_PARM_DESC(edid_fixup,
>
> static void drm_get_displayid(struct drm_connector *connector,
> struct edid *edid);
> +static u8 *drm_find_displayid_extension(const struct edid *edid);
> +static int validate_displayid(u8 *displayid, int length, int idx);
>
> static int drm_edid_block_checksum(const u8 *raw_edid)
> {
> @@ -2885,7 +2887,40 @@ static u8 *drm_find_edid_extension(const struct edid *edid, int ext_id)
>
> static u8 *drm_find_cea_extension(const struct edid *edid)
> {
> - return drm_find_edid_extension(edid, CEA_EXT);
> + int ret;
> + int idx = 1;
> + int length = EDID_LENGTH;
> + struct displayid_block *block;
> + u8 *cea = NULL;
> + u8 *displayid = NULL;
Unnecessary initializations.
> +
> + cea = drm_find_edid_extension(edid, CEA_EXT);
> +
> + /* CEA blocks can also be found embedded in a DisplayID block */
> + if (!cea) {
It's customary to return early to reduce indent on the following code,
i.e.
if (cea)
return cea;
> + displayid = drm_find_displayid_extension(edid);
> + if (!displayid)
> + return NULL;
> +
> + ret = validate_displayid(displayid, length, idx);
> + if (ret)
> + return NULL;
> +
> + idx += sizeof(struct displayid_hdr);
> + while (block = (struct displayid_block *)&displayid[idx],
> + idx + sizeof(struct displayid_block) <= length &&
> + idx + sizeof(struct displayid_block) + block->num_bytes <= length &&
> + block->num_bytes > 0) {
I'm sure there's a for loop in there somewhere desperate to get out. The
above is unnecessarily tricky.
> + idx += block->num_bytes + sizeof(struct displayid_block);
> + switch (block->tag) {
> + case DATA_BLOCK_CTA:
> + cea = (u8 *)block;
> + break;
> + }
Looks like an if statement here. Why do you continue the while loop
after you've found the block?
BR,
Jani.
> + }
> + }
> +
> + return cea;
> }
>
> static u8 *drm_find_displayid_extension(const struct edid *edid)
> @@ -3616,13 +3651,38 @@ cea_revision(const u8 *cea)
> static int
> cea_db_offsets(const u8 *cea, int *start, int *end)
> {
> - /* Data block offset in CEA extension block */
> - *start = 4;
> - *end = cea[2];
> - if (*end == 0)
> - *end = 127;
> - if (*end < 4 || *end > 127)
> - return -ERANGE;
> +
> + /* DisplayID CTA extension blocks and top-level CEA EDID
> + * blocks headers differ in two byte definitions:
> + * 1) Byte 2 of the header specifies length differently,
> + * 2) Byte 3 is only present in the CEA top level block.
> + *
> + * The different definitions for byte 2 follow.
> + *
> + * DisplayID CTA extension block defines byte 2 as:
> + * Number of payload bytes
> + *
> + * CEA EDID block defines byte 2 as:
> + * Byte number (decimal) within this block where the 18-byte
> + * DTDs begin. If no non-DTD data is present in this extension
> + * block, the value should be set to 04h (the byte after next).
> + * If set to 00h, there are no DTDs present in this block and
> + * no non-DTD data.
> + */
> + if (cea[0] == DATA_BLOCK_CTA) {
> + *start = 3;
> + *end = *start + cea[2];
> + } else if (cea[0] == CEA_EXT) {
> + *start = 4;
> + *end = cea[2];
> + /* Data block offset in CEA extension block */
> + if (*end == 0)
> + *end = 127;
> + if (*end < 4 || *end > 127)
> + return -ERANGE;
> + } else
> + return -ENOTSUPP;
> +
> return 0;
> }
>
> @@ -5240,6 +5300,9 @@ static int drm_parse_display_id(struct drm_connector *connector,
> case DATA_BLOCK_TYPE_1_DETAILED_TIMING:
> /* handled in mode gathering code. */
> break;
> + case DATA_BLOCK_CTA:
> + /* handled in the cea parser code. */
> + break;
> default:
> DRM_DEBUG_KMS("found DisplayID tag 0x%x, unhandled\n", block->tag);
> break;
> diff --git a/include/drm/drm_displayid.h b/include/drm/drm_displayid.h
> index c0d4df6a606f..c7af857f4764 100644
> --- a/include/drm/drm_displayid.h
> +++ b/include/drm/drm_displayid.h
> @@ -40,6 +40,7 @@
> #define DATA_BLOCK_DISPLAY_INTERFACE 0x0f
> #define DATA_BLOCK_STEREO_DISPLAY_INTERFACE 0x10
> #define DATA_BLOCK_TILED_DISPLAY 0x12
> +#define DATA_BLOCK_CTA 0x81
>
> #define DATA_BLOCK_VENDOR_SPECIFIC 0x7f
--
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
