Add iterators for EDID blocks and CEA data blocks, to iterate through all data blocks across all CEA extensions and CTA blocks in DisplayID data blocks. Fix all code assuming only one CEA extension. Fix code assuming CTA blocks contain everything that CEA extensions do. Sprinkle a bunch of cleanups on top.
This is completely UNTESTED, didn't even smoke test it. It builds. ;)
This superseeds parts of [1] and [2].
BR, Jani.
[1] https://patchwork.freedesktop.org/series/101241/ [2] https://patchwork.freedesktop.org/patch/msgid/20220321044330.27723-1-cooper....
Cc: Shawn C Lee shawn.c.lee@intel.com Cc: Cooper Chiou cooper.chiou@intel.com Cc: william.tseng@intel.com Cc: ankit.k.nautiyal@intel.com Cc: ville.syrjala@linux.intel.com Cc: Drew Davenport ddavenport@chromium.org
Jani Nikula (19): drm/edid: add drm_edid_extension_block_count() and drm_edid_size() drm: use drm_edid_extension_block_count() and drm_edid_size() drm/edid: clean up CEA data block tag definitions drm/edid: add iterator for EDID base and extension blocks drm/edid: add iterator for CEA data blocks drm/edid: clean up cea_db_is_*() functions drm/edid: convert add_cea_modes() to use cea db iter drm/edid: convert drm_edid_to_speaker_allocation() to use cea db iter drm/edid: convert drm_edid_to_sad() to use cea db iter drm/edid: convert drm_detect_hdmi_monitor() to use cea db iter drm/edid: convert drm_detect_monitor_audio() to use cea db iter drm/edid: convert drm_parse_cea_ext() to use cea db iter drm/edid: convert drm_edid_to_eld() to use cea db iter drm/edid: sunset the old unused cea data block iterators drm/edid: restore some type safety to cea_db_*() functions drm/edid: detect basic audio only on CEA extension drm/edid: detect color formats and CEA revision only on CEA extension drm/edid: skip CEA extension scan in drm_edid_to_eld() just for CEA rev drm/edid: sunset drm_find_cea_extension()
drivers/gpu/drm/drm_connector.c | 2 +- drivers/gpu/drm/drm_debugfs.c | 3 +- drivers/gpu/drm/drm_edid.c | 781 ++++++++++++++++++-------------- include/drm/drm_edid.h | 2 + 4 files changed, 455 insertions(+), 333 deletions(-)
Add abstractions for getting the number of EDID extension blocks and the total EDID size in bytes.
Signed-off-by: Jani Nikula jani.nikula@intel.com --- drivers/gpu/drm/drm_edid.c | 18 ++++++++++++++++++ include/drm/drm_edid.h | 2 ++ 2 files changed, 20 insertions(+)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 561f53831e29..f4b49693e666 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -2198,6 +2198,24 @@ struct edid *drm_get_edid_switcheroo(struct drm_connector *connector, } EXPORT_SYMBOL(drm_get_edid_switcheroo);
+/** + * drm_edid_extension_block_count - get the number of EDID extension blocks + */ +u8 drm_edid_extension_block_count(const struct edid *edid) +{ + return edid->extensions; +} +EXPORT_SYMBOL(drm_edid_extension_block_count); + +/** + * drm_edid_size - get the EDID size in bytes + */ +size_t drm_edid_size(const struct edid *edid) +{ + return (drm_edid_extension_block_count(edid) + 1) * EDID_LENGTH; +} +EXPORT_SYMBOL(drm_edid_size); + /** * drm_edid_duplicate - duplicate an EDID and the extensions * @edid: EDID to duplicate diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h index 144c495b99c4..7a19daa00c0c 100644 --- a/include/drm/drm_edid.h +++ b/include/drm/drm_edid.h @@ -564,6 +564,8 @@ struct edid *drm_get_edid(struct drm_connector *connector, u32 drm_edid_get_panel_id(struct i2c_adapter *adapter); struct edid *drm_get_edid_switcheroo(struct drm_connector *connector, struct i2c_adapter *adapter); +u8 drm_edid_extension_block_count(const struct edid *edid); +size_t drm_edid_size(const struct edid *edid); struct edid *drm_edid_duplicate(const struct edid *edid); int drm_add_edid_modes(struct drm_connector *connector, struct edid *edid); int drm_add_override_edid_modes(struct drm_connector *connector);
On Tue, Mar 22, 2022 at 11:40:30PM +0200, Jani Nikula wrote:
Add abstractions for getting the number of EDID extension blocks and the total EDID size in bytes.
Signed-off-by: Jani Nikula jani.nikula@intel.com
drivers/gpu/drm/drm_edid.c | 18 ++++++++++++++++++ include/drm/drm_edid.h | 2 ++ 2 files changed, 20 insertions(+)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 561f53831e29..f4b49693e666 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -2198,6 +2198,24 @@ struct edid *drm_get_edid_switcheroo(struct drm_connector *connector, } EXPORT_SYMBOL(drm_get_edid_switcheroo);
+/**
- drm_edid_extension_block_count - get the number of EDID extension blocks
- */
+u8 drm_edid_extension_block_count(const struct edid *edid)
It's just a number, so could be 'int' IMO.
Reviewed-by: Ville Syrjälä ville.syrjala@linux.intel.com
+{
- return edid->extensions;
+} +EXPORT_SYMBOL(drm_edid_extension_block_count);
+/**
- drm_edid_size - get the EDID size in bytes
- */
+size_t drm_edid_size(const struct edid *edid) +{
- return (drm_edid_extension_block_count(edid) + 1) * EDID_LENGTH;
+} +EXPORT_SYMBOL(drm_edid_size);
/**
- drm_edid_duplicate - duplicate an EDID and the extensions
- @edid: EDID to duplicate
diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h index 144c495b99c4..7a19daa00c0c 100644 --- a/include/drm/drm_edid.h +++ b/include/drm/drm_edid.h @@ -564,6 +564,8 @@ struct edid *drm_get_edid(struct drm_connector *connector, u32 drm_edid_get_panel_id(struct i2c_adapter *adapter); struct edid *drm_get_edid_switcheroo(struct drm_connector *connector, struct i2c_adapter *adapter); +u8 drm_edid_extension_block_count(const struct edid *edid); +size_t drm_edid_size(const struct edid *edid); struct edid *drm_edid_duplicate(const struct edid *edid); int drm_add_edid_modes(struct drm_connector *connector, struct edid *edid); int drm_add_override_edid_modes(struct drm_connector *connector); -- 2.30.2
Use the block count and size helpers in all drm core code.
Signed-off-by: Jani Nikula jani.nikula@intel.com --- drivers/gpu/drm/drm_connector.c | 2 +- drivers/gpu/drm/drm_debugfs.c | 3 +-- drivers/gpu/drm/drm_edid.c | 14 +++++++------- 3 files changed, 9 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c index 76a8c707c34b..cfed43e61380 100644 --- a/drivers/gpu/drm/drm_connector.c +++ b/drivers/gpu/drm/drm_connector.c @@ -2138,7 +2138,7 @@ int drm_connector_update_edid_property(struct drm_connector *connector, return 0;
if (edid) - size = EDID_LENGTH * (1 + edid->extensions); + size = drm_edid_size(edid);
/* Set the display info, using edid if available, otherwise * resetting the values to defaults. This duplicates the work diff --git a/drivers/gpu/drm/drm_debugfs.c b/drivers/gpu/drm/drm_debugfs.c index 7f1b82dbaebb..a832ef6b33fe 100644 --- a/drivers/gpu/drm/drm_debugfs.c +++ b/drivers/gpu/drm/drm_debugfs.c @@ -362,8 +362,7 @@ static ssize_t edid_write(struct file *file, const char __user *ubuf, if (len == 5 && !strncmp(buf, "reset", 5)) { connector->override_edid = false; ret = drm_connector_update_edid_property(connector, NULL); - } else if (len < EDID_LENGTH || - EDID_LENGTH * (1 + edid->extensions) > len) + } else if (len < EDID_LENGTH || drm_edid_size(edid) > len) ret = -EINVAL; else { connector->override_edid = false; diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index f4b49693e666..b96906774433 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -1643,8 +1643,8 @@ bool drm_edid_are_equal(const struct edid *edid1, const struct edid *edid2) return false;
if (edid1) { - edid1_len = EDID_LENGTH * (1 + edid1->extensions); - edid2_len = EDID_LENGTH * (1 + edid2->extensions); + edid1_len = drm_edid_size(edid1); + edid2_len = drm_edid_size(edid2);
if (edid1_len != edid2_len) return false; @@ -1770,7 +1770,7 @@ bool drm_edid_is_valid(struct edid *edid) if (!edid) return false;
- for (i = 0; i <= edid->extensions; i++) + for (i = 0; i <= drm_edid_extension_block_count(edid); i++) if (!drm_edid_block_valid(raw + i * EDID_LENGTH, i, true, NULL)) return false;
@@ -2224,7 +2224,7 @@ EXPORT_SYMBOL(drm_edid_size); */ struct edid *drm_edid_duplicate(const struct edid *edid) { - return kmemdup(edid, (edid->extensions + 1) * EDID_LENGTH, GFP_KERNEL); + return kmemdup(edid, drm_edid_size(edid), GFP_KERNEL); } EXPORT_SYMBOL(drm_edid_duplicate);
@@ -3353,17 +3353,17 @@ const u8 *drm_find_edid_extension(const struct edid *edid, int i;
/* No EDID or EDID extensions */ - if (edid == NULL || edid->extensions == 0) + if (edid == NULL || drm_edid_extension_block_count(edid) == 0) return NULL;
/* Find CEA extension */ - for (i = *ext_index; i < edid->extensions; i++) { + for (i = *ext_index; i < drm_edid_extension_block_count(edid); i++) { edid_ext = (const u8 *)edid + EDID_LENGTH * (i + 1); if (edid_ext[0] == ext_id) break; }
- if (i >= edid->extensions) + if (i >= drm_edid_extension_block_count(edid)) return NULL;
*ext_index = i + 1;
On Tue, Mar 22, 2022 at 11:40:31PM +0200, Jani Nikula wrote:
Use the block count and size helpers in all drm core code.
Signed-off-by: Jani Nikula jani.nikula@intel.com
drivers/gpu/drm/drm_connector.c | 2 +- drivers/gpu/drm/drm_debugfs.c | 3 +-- drivers/gpu/drm/drm_edid.c | 14 +++++++------- 3 files changed, 9 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c index 76a8c707c34b..cfed43e61380 100644 --- a/drivers/gpu/drm/drm_connector.c +++ b/drivers/gpu/drm/drm_connector.c @@ -2138,7 +2138,7 @@ int drm_connector_update_edid_property(struct drm_connector *connector, return 0;
if (edid)
size = EDID_LENGTH * (1 + edid->extensions);
size = drm_edid_size(edid);/* Set the display info, using edid if available, otherwise
- resetting the values to defaults. This duplicates the work
diff --git a/drivers/gpu/drm/drm_debugfs.c b/drivers/gpu/drm/drm_debugfs.c index 7f1b82dbaebb..a832ef6b33fe 100644 --- a/drivers/gpu/drm/drm_debugfs.c +++ b/drivers/gpu/drm/drm_debugfs.c @@ -362,8 +362,7 @@ static ssize_t edid_write(struct file *file, const char __user *ubuf, if (len == 5 && !strncmp(buf, "reset", 5)) { connector->override_edid = false; ret = drm_connector_update_edid_property(connector, NULL);
- } else if (len < EDID_LENGTH ||
EDID_LENGTH * (1 + edid->extensions) > len)
- } else if (len < EDID_LENGTH || drm_edid_size(edid) > len) ret = -EINVAL; else { connector->override_edid = false;
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index f4b49693e666..b96906774433 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -1643,8 +1643,8 @@ bool drm_edid_are_equal(const struct edid *edid1, const struct edid *edid2) return false;
if (edid1) {
edid1_len = EDID_LENGTH * (1 + edid1->extensions);edid2_len = EDID_LENGTH * (1 + edid2->extensions);
edid1_len = drm_edid_size(edid1);edid2_len = drm_edid_size(edid2);if (edid1_len != edid2_len) return false;
@@ -1770,7 +1770,7 @@ bool drm_edid_is_valid(struct edid *edid) if (!edid) return false;
- for (i = 0; i <= edid->extensions; i++)
- for (i = 0; i <= drm_edid_extension_block_count(edid); i++) if (!drm_edid_block_valid(raw + i * EDID_LENGTH, i, true, NULL))
Maybe we should also have drm_edid_block_count(), drm_edid_block_data(), drm_edid_extension_block_data() etc.?
Reviewed-by: Ville Syrjälä ville.syrjala@linux.intel.com
return false;@@ -2224,7 +2224,7 @@ EXPORT_SYMBOL(drm_edid_size); */ struct edid *drm_edid_duplicate(const struct edid *edid) {
- return kmemdup(edid, (edid->extensions + 1) * EDID_LENGTH, GFP_KERNEL);
- return kmemdup(edid, drm_edid_size(edid), GFP_KERNEL);
} EXPORT_SYMBOL(drm_edid_duplicate);
@@ -3353,17 +3353,17 @@ const u8 *drm_find_edid_extension(const struct edid *edid, int i;
/* No EDID or EDID extensions */
- if (edid == NULL || edid->extensions == 0)
if (edid == NULL || drm_edid_extension_block_count(edid) == 0) return NULL;
/* Find CEA extension */
- for (i = *ext_index; i < edid->extensions; i++) {
- for (i = *ext_index; i < drm_edid_extension_block_count(edid); i++) { edid_ext = (const u8 *)edid + EDID_LENGTH * (i + 1); if (edid_ext[0] == ext_id) break; }
- if (i >= edid->extensions)
if (i >= drm_edid_extension_block_count(edid)) return NULL;
*ext_index = i + 1;
-- 2.30.2
Add prefixed names, group, sort, add references.
Signed-off-by: Jani Nikula jani.nikula@intel.com --- drivers/gpu/drm/drm_edid.c | 59 +++++++++++++++++++++----------------- 1 file changed, 32 insertions(+), 27 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index b96906774433..6c188539493e 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -3329,15 +3329,20 @@ add_detailed_modes(struct drm_connector *connector, struct edid *edid, return closure.modes; }
-#define AUDIO_BLOCK 0x01 -#define VIDEO_BLOCK 0x02 -#define VENDOR_BLOCK 0x03 -#define SPEAKER_BLOCK 0x04 -#define HDR_STATIC_METADATA_BLOCK 0x6 -#define USE_EXTENDED_TAG 0x07 -#define EXT_VIDEO_CAPABILITY_BLOCK 0x00 -#define EXT_VIDEO_DATA_BLOCK_420 0x0E -#define EXT_VIDEO_CAP_BLOCK_Y420CMDB 0x0F +/* CEA-861-F Table 44 CEA Data Block Tag Codes */ +#define CEA_DB_AUDIO 1 +#define CEA_DB_VIDEO 2 +#define CEA_DB_VENDOR 3 +#define CEA_DB_SPEAKER 4 +#define CEA_DB_EXTENDED_TAG 7 + +/* CEA-861-F Table 46 CEA Data Block Tag Codes */ +#define CEA_EXT_DB_VIDEO_CAP 0 +#define CEA_EXT_DB_VENDOR 1 +#define CEA_EXT_DB_HDR_STATIC_METADATA 6 /* CEA-861.3 2005 */ +#define CEA_EXT_DB_420_VIDEO_DATA 14 +#define CEA_EXT_DB_420_VIDEO_CAP_MAP 15 + #define EDID_BASIC_AUDIO (1 << 6) #define EDID_CEA_YCRCB444 (1 << 5) #define EDID_CEA_YCRCB422 (1 << 4) @@ -4220,7 +4225,7 @@ cea_db_offsets(const u8 *cea, int *start, int *end)
static bool cea_db_is_hdmi_vsdb(const u8 *db) { - if (cea_db_tag(db) != VENDOR_BLOCK) + if (cea_db_tag(db) != CEA_DB_VENDOR) return false;
if (cea_db_payload_len(db) < 5) @@ -4231,7 +4236,7 @@ static bool cea_db_is_hdmi_vsdb(const u8 *db)
static bool cea_db_is_hdmi_forum_vsdb(const u8 *db) { - if (cea_db_tag(db) != VENDOR_BLOCK) + if (cea_db_tag(db) != CEA_DB_VENDOR) return false;
if (cea_db_payload_len(db) < 7) @@ -4242,7 +4247,7 @@ static bool cea_db_is_hdmi_forum_vsdb(const u8 *db)
static bool cea_db_is_microsoft_vsdb(const u8 *db) { - if (cea_db_tag(db) != VENDOR_BLOCK) + if (cea_db_tag(db) != CEA_DB_VENDOR) return false;
if (cea_db_payload_len(db) != 21) @@ -4253,13 +4258,13 @@ static bool cea_db_is_microsoft_vsdb(const u8 *db)
static bool cea_db_is_vcdb(const u8 *db) { - if (cea_db_tag(db) != USE_EXTENDED_TAG) + if (cea_db_tag(db) != CEA_DB_EXTENDED_TAG) return false;
if (cea_db_payload_len(db) != 2) return false;
- if (cea_db_extended_tag(db) != EXT_VIDEO_CAPABILITY_BLOCK) + if (cea_db_extended_tag(db) != CEA_EXT_DB_VIDEO_CAP) return false;
return true; @@ -4267,13 +4272,13 @@ static bool cea_db_is_vcdb(const u8 *db)
static bool cea_db_is_y420cmdb(const u8 *db) { - if (cea_db_tag(db) != USE_EXTENDED_TAG) + if (cea_db_tag(db) != CEA_DB_EXTENDED_TAG) return false;
if (!cea_db_payload_len(db)) return false;
- if (cea_db_extended_tag(db) != EXT_VIDEO_CAP_BLOCK_Y420CMDB) + if (cea_db_extended_tag(db) != CEA_EXT_DB_420_VIDEO_CAP_MAP) return false;
return true; @@ -4281,13 +4286,13 @@ static bool cea_db_is_y420cmdb(const u8 *db)
static bool cea_db_is_y420vdb(const u8 *db) { - if (cea_db_tag(db) != USE_EXTENDED_TAG) + if (cea_db_tag(db) != CEA_DB_EXTENDED_TAG) return false;
if (!cea_db_payload_len(db)) return false;
- if (cea_db_extended_tag(db) != EXT_VIDEO_DATA_BLOCK_420) + if (cea_db_extended_tag(db) != CEA_EXT_DB_420_VIDEO_DATA) return false;
return true; @@ -4354,7 +4359,7 @@ add_cea_modes(struct drm_connector *connector, struct edid *edid) db = &cea[i]; dbl = cea_db_payload_len(db);
- if (cea_db_tag(db) == VIDEO_BLOCK) { + if (cea_db_tag(db) == CEA_DB_VIDEO) { video = db + 1; video_len = dbl; modes += do_cea_modes(connector, video, dbl); @@ -4428,10 +4433,10 @@ static void fixup_detailed_cea_mode_clock(struct drm_display_mode *mode)
static bool cea_db_is_hdmi_hdr_metadata_block(const u8 *db) { - if (cea_db_tag(db) != USE_EXTENDED_TAG) + if (cea_db_tag(db) != CEA_DB_EXTENDED_TAG) return false;
- if (db[1] != HDR_STATIC_METADATA_BLOCK) + if (db[1] != CEA_EXT_DB_HDR_STATIC_METADATA) return false;
if (cea_db_payload_len(db) < 3) @@ -4622,7 +4627,7 @@ static void drm_edid_to_eld(struct drm_connector *connector, struct edid *edid) dbl = cea_db_payload_len(db);
switch (cea_db_tag(db)) { - case AUDIO_BLOCK: + case CEA_DB_AUDIO: /* Audio Data Block, contains SADs */ sad_count = min(dbl / 3, 15 - total_sad_count); if (sad_count >= 1) @@ -4630,12 +4635,12 @@ static void drm_edid_to_eld(struct drm_connector *connector, struct edid *edid) &db[1], sad_count * 3); total_sad_count += sad_count; break; - case SPEAKER_BLOCK: + case CEA_DB_SPEAKER: /* Speaker Allocation Data Block */ if (dbl >= 1) eld[DRM_ELD_SPEAKER] = db[1]; break; - case VENDOR_BLOCK: + case CEA_DB_VENDOR: /* HDMI Vendor-Specific Data Block */ if (cea_db_is_hdmi_vsdb(db)) drm_parse_hdmi_vsdb_audio(connector, db); @@ -4696,7 +4701,7 @@ int drm_edid_to_sad(struct edid *edid, struct cea_sad **sads) for_each_cea_db(cea, i, start, end) { const u8 *db = &cea[i];
- if (cea_db_tag(db) == AUDIO_BLOCK) { + if (cea_db_tag(db) == CEA_DB_AUDIO) { int j;
dbl = cea_db_payload_len(db); @@ -4758,7 +4763,7 @@ int drm_edid_to_speaker_allocation(struct edid *edid, u8 **sadb) for_each_cea_db(cea, i, start, end) { const u8 *db = &cea[i];
- if (cea_db_tag(db) == SPEAKER_BLOCK) { + if (cea_db_tag(db) == CEA_DB_SPEAKER) { dbl = cea_db_payload_len(db);
/* Speaker Allocation Data Block */ @@ -4888,7 +4893,7 @@ bool drm_detect_monitor_audio(struct edid *edid) goto end;
for_each_cea_db(edid_ext, i, start_offset, end_offset) { - if (cea_db_tag(&edid_ext[i]) == AUDIO_BLOCK) { + if (cea_db_tag(&edid_ext[i]) == CEA_DB_AUDIO) { has_audio = true; for (j = 1; j < cea_db_payload_len(&edid_ext[i]) + 1; j += 3) DRM_DEBUG_KMS("CEA audio format %d\n",
On Tue, Mar 22, 2022 at 11:40:32PM +0200, Jani Nikula wrote:
Add prefixed names, group, sort, add references.
Signed-off-by: Jani Nikula jani.nikula@intel.com
drivers/gpu/drm/drm_edid.c | 59 +++++++++++++++++++++----------------- 1 file changed, 32 insertions(+), 27 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index b96906774433..6c188539493e 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -3329,15 +3329,20 @@ add_detailed_modes(struct drm_connector *connector, struct edid *edid, return closure.modes; }
-#define AUDIO_BLOCK 0x01 -#define VIDEO_BLOCK 0x02 -#define VENDOR_BLOCK 0x03 -#define SPEAKER_BLOCK 0x04 -#define HDR_STATIC_METADATA_BLOCK 0x6 -#define USE_EXTENDED_TAG 0x07 -#define EXT_VIDEO_CAPABILITY_BLOCK 0x00 -#define EXT_VIDEO_DATA_BLOCK_420 0x0E -#define EXT_VIDEO_CAP_BLOCK_Y420CMDB 0x0F +/* CEA-861-F Table 44 CEA Data Block Tag Codes */
Table 55 in CTA-861-G, if we want to use a more recent reference. Though IIRC someone did say CTA-861-H might already be out as well.
+#define CEA_DB_AUDIO 1 +#define CEA_DB_VIDEO 2 +#define CEA_DB_VENDOR 3 +#define CEA_DB_SPEAKER 4 +#define CEA_DB_EXTENDED_TAG 7
+/* CEA-861-F Table 46 CEA Data Block Tag Codes */
Table 57 in CTA-861-G.
Reviewed-by: Ville Syrjälä ville.syrjala@linux.intel.com
+#define CEA_EXT_DB_VIDEO_CAP 0 +#define CEA_EXT_DB_VENDOR 1 +#define CEA_EXT_DB_HDR_STATIC_METADATA 6 /* CEA-861.3 2005 */ +#define CEA_EXT_DB_420_VIDEO_DATA 14 +#define CEA_EXT_DB_420_VIDEO_CAP_MAP 15
#define EDID_BASIC_AUDIO (1 << 6) #define EDID_CEA_YCRCB444 (1 << 5) #define EDID_CEA_YCRCB422 (1 << 4) @@ -4220,7 +4225,7 @@ cea_db_offsets(const u8 *cea, int *start, int *end)
static bool cea_db_is_hdmi_vsdb(const u8 *db) {
- if (cea_db_tag(db) != VENDOR_BLOCK)
if (cea_db_tag(db) != CEA_DB_VENDOR) return false;
if (cea_db_payload_len(db) < 5)
@@ -4231,7 +4236,7 @@ static bool cea_db_is_hdmi_vsdb(const u8 *db)
static bool cea_db_is_hdmi_forum_vsdb(const u8 *db) {
- if (cea_db_tag(db) != VENDOR_BLOCK)
if (cea_db_tag(db) != CEA_DB_VENDOR) return false;
if (cea_db_payload_len(db) < 7)
@@ -4242,7 +4247,7 @@ static bool cea_db_is_hdmi_forum_vsdb(const u8 *db)
static bool cea_db_is_microsoft_vsdb(const u8 *db) {
- if (cea_db_tag(db) != VENDOR_BLOCK)
if (cea_db_tag(db) != CEA_DB_VENDOR) return false;
if (cea_db_payload_len(db) != 21)
@@ -4253,13 +4258,13 @@ static bool cea_db_is_microsoft_vsdb(const u8 *db)
static bool cea_db_is_vcdb(const u8 *db) {
- if (cea_db_tag(db) != USE_EXTENDED_TAG)
if (cea_db_tag(db) != CEA_DB_EXTENDED_TAG) return false;
if (cea_db_payload_len(db) != 2) return false;
- if (cea_db_extended_tag(db) != EXT_VIDEO_CAPABILITY_BLOCK)
if (cea_db_extended_tag(db) != CEA_EXT_DB_VIDEO_CAP) return false;
return true;
@@ -4267,13 +4272,13 @@ static bool cea_db_is_vcdb(const u8 *db)
static bool cea_db_is_y420cmdb(const u8 *db) {
- if (cea_db_tag(db) != USE_EXTENDED_TAG)
if (cea_db_tag(db) != CEA_DB_EXTENDED_TAG) return false;
if (!cea_db_payload_len(db)) return false;
- if (cea_db_extended_tag(db) != EXT_VIDEO_CAP_BLOCK_Y420CMDB)
if (cea_db_extended_tag(db) != CEA_EXT_DB_420_VIDEO_CAP_MAP) return false;
return true;
@@ -4281,13 +4286,13 @@ static bool cea_db_is_y420cmdb(const u8 *db)
static bool cea_db_is_y420vdb(const u8 *db) {
- if (cea_db_tag(db) != USE_EXTENDED_TAG)
if (cea_db_tag(db) != CEA_DB_EXTENDED_TAG) return false;
if (!cea_db_payload_len(db)) return false;
- if (cea_db_extended_tag(db) != EXT_VIDEO_DATA_BLOCK_420)
if (cea_db_extended_tag(db) != CEA_EXT_DB_420_VIDEO_DATA) return false;
return true;
@@ -4354,7 +4359,7 @@ add_cea_modes(struct drm_connector *connector, struct edid *edid) db = &cea[i]; dbl = cea_db_payload_len(db);
if (cea_db_tag(db) == VIDEO_BLOCK) {
if (cea_db_tag(db) == CEA_DB_VIDEO) { video = db + 1; video_len = dbl; modes += do_cea_modes(connector, video, dbl);@@ -4428,10 +4433,10 @@ static void fixup_detailed_cea_mode_clock(struct drm_display_mode *mode)
static bool cea_db_is_hdmi_hdr_metadata_block(const u8 *db) {
- if (cea_db_tag(db) != USE_EXTENDED_TAG)
- if (cea_db_tag(db) != CEA_DB_EXTENDED_TAG) return false;
- if (db[1] != HDR_STATIC_METADATA_BLOCK)
if (db[1] != CEA_EXT_DB_HDR_STATIC_METADATA) return false;
if (cea_db_payload_len(db) < 3)
@@ -4622,7 +4627,7 @@ static void drm_edid_to_eld(struct drm_connector *connector, struct edid *edid) dbl = cea_db_payload_len(db);
switch (cea_db_tag(db)) {
case AUDIO_BLOCK:
case CEA_DB_AUDIO: /* Audio Data Block, contains SADs */ sad_count = min(dbl / 3, 15 - total_sad_count); if (sad_count >= 1)@@ -4630,12 +4635,12 @@ static void drm_edid_to_eld(struct drm_connector *connector, struct edid *edid) &db[1], sad_count * 3); total_sad_count += sad_count; break;
case SPEAKER_BLOCK:
case CEA_DB_SPEAKER: /* Speaker Allocation Data Block */ if (dbl >= 1) eld[DRM_ELD_SPEAKER] = db[1]; break;
case VENDOR_BLOCK:
case CEA_DB_VENDOR: /* HDMI Vendor-Specific Data Block */ if (cea_db_is_hdmi_vsdb(db)) drm_parse_hdmi_vsdb_audio(connector, db);@@ -4696,7 +4701,7 @@ int drm_edid_to_sad(struct edid *edid, struct cea_sad **sads) for_each_cea_db(cea, i, start, end) { const u8 *db = &cea[i];
if (cea_db_tag(db) == AUDIO_BLOCK) {
if (cea_db_tag(db) == CEA_DB_AUDIO) { int j; dbl = cea_db_payload_len(db);@@ -4758,7 +4763,7 @@ int drm_edid_to_speaker_allocation(struct edid *edid, u8 **sadb) for_each_cea_db(cea, i, start, end) { const u8 *db = &cea[i];
if (cea_db_tag(db) == SPEAKER_BLOCK) {
if (cea_db_tag(db) == CEA_DB_SPEAKER) { dbl = cea_db_payload_len(db); /* Speaker Allocation Data Block */@@ -4888,7 +4893,7 @@ bool drm_detect_monitor_audio(struct edid *edid) goto end;
for_each_cea_db(edid_ext, i, start_offset, end_offset) {
if (cea_db_tag(&edid_ext[i]) == AUDIO_BLOCK) {
if (cea_db_tag(&edid_ext[i]) == CEA_DB_AUDIO) { has_audio = true; for (j = 1; j < cea_db_payload_len(&edid_ext[i]) + 1; j += 3) DRM_DEBUG_KMS("CEA audio format %d\n",-- 2.30.2
Add an iterator abstraction for going through all the EDID blocks.
Signed-off-by: Jani Nikula jani.nikula@intel.com --- drivers/gpu/drm/drm_edid.c | 46 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 6c188539493e..31d132fcd0ca 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -3348,6 +3348,52 @@ add_detailed_modes(struct drm_connector *connector, struct edid *edid, #define EDID_CEA_YCRCB422 (1 << 4) #define EDID_CEA_VCDB_QS (1 << 6)
+/* + * EDID base and extension block iterator. + * + * struct drm_edid_iter iter; + * const u8 *block; + * + * drm_edid_iter_begin(edid, &iter); + * drm_edid_iter_for_each(block, &iter) { + * // do stuff with block + * } + * drm_edid_iter_end(&iter); + */ +struct drm_edid_iter { + const struct edid *edid; + + /* Current block index. */ + int index; +}; + +static void drm_edid_iter_begin(const struct edid *edid, + struct drm_edid_iter *iter) +{ + memset(iter, 0, sizeof(*iter)); + + iter->edid = edid; +} + +static const void *__drm_edid_iter_next(struct drm_edid_iter *iter) +{ + if (!iter->edid) + return NULL; + + if (iter->index > drm_edid_extension_block_count(iter->edid)) + return NULL; + + return (const u8 *)iter->edid + EDID_LENGTH * iter->index++; +} + +#define drm_edid_iter_for_each(__block, __iter) \ + while (((__block) = __drm_edid_iter_next(__iter))) + +static void drm_edid_iter_end(struct drm_edid_iter *iter) +{ + memset(iter, 0, sizeof(*iter)); +} + /* * Search EDID for CEA extension block. */
Add an iterator for CEA Data Blocks across CEA extensions and CTA DisplayID Data Blocks.
Signed-off-by: Jani Nikula jani.nikula@intel.com --- drivers/gpu/drm/drm_edid.c | 198 ++++++++++++++++++++++++++++++++++--- 1 file changed, 186 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 31d132fcd0ca..c12c3cbab274 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -4196,24 +4196,12 @@ do_hdmi_vsdb_modes(struct drm_connector *connector, const u8 *db, u8 len, return modes; }
-static int -cea_db_payload_len(const u8 *db) -{ - return db[0] & 0x1f; -} - static int cea_db_extended_tag(const u8 *db) { return db[1]; }
-static int -cea_db_tag(const u8 *db) -{ - return db[0] >> 5; -} - static int cea_revision(const u8 *cea) { @@ -4269,6 +4257,192 @@ cea_db_offsets(const u8 *cea, int *start, int *end) return 0; }
+/* + * CEA Data Block iterator. + * + * Iterate through all CEA Data Blocks in both EDID CEA extensions and CTA Data + * Blocks in DisplayID extension blocks. + * + * struct cea_db *db: + * struct cea_db_iter iter; + * + * cea_db_iter_edid_begin(edid, &iter); + * cea_db_iter_for_each(db, &iter) { + * // do stuff with db + * } + * cea_db_iter_end(&iter); + */ +struct cea_db_iter { + struct drm_edid_iter edid_iter; + struct displayid_iter displayid_iter; + + /* Current Data Block Collection. */ + const u8 *collection; + + /* Current Data Block index in current collection. */ + int index; + + /* End index in current collection. */ + int end; +}; + +/* CEA-861-F section 7.5 CEA Extension Version 3 and Table 43 */ +struct cea_db { + u8 tag_length; + u8 data[]; +} __packed; + +static int cea_db_tag(const void *_db) +{ + /* FIXME: Transition to passing struct cea_db * everywhere. */ + const struct cea_db *db = _db; + + return db->tag_length >> 5; +} + +static int cea_db_payload_len(const void *_db) +{ + /* FIXME: Transition to passing struct cea_db * everywhere. */ + const struct cea_db *db = _db; + + return db->tag_length & 0x1f; +} + +static const void *cea_db_data(const struct cea_db *db) +{ + return db->data; +} + +static void cea_db_iter_edid_begin(const struct edid *edid, struct cea_db_iter *iter) +{ + memset(iter, 0, sizeof(*iter)); + + drm_edid_iter_begin(edid, &iter->edid_iter); + displayid_iter_edid_begin(edid, &iter->displayid_iter); +} + +static const struct cea_db * +__cea_db_iter_current_block(const struct cea_db_iter *iter) +{ + const struct cea_db *db; + + if (!iter->collection) + return NULL; + + db = (const struct cea_db *)&iter->collection[iter->index]; + + if (iter->index + sizeof(*db) <= iter->end && + iter->index + sizeof(*db) + cea_db_payload_len(db) <= iter->end) + return db; + + return NULL; +} + +/* + * References: + * - VESA E-EDID v1.4 + * - CEA-861-F section 7.5 CEA Extension Version 3 + */ +static const void *__cea_db_iter_edid_next(struct cea_db_iter *iter) +{ + const u8 *ext; + + drm_edid_iter_for_each(ext, &iter->edid_iter) { + /* Only support CEA extension revision 3+ */ + if (ext[0] != CEA_EXT || cea_revision(ext) < 3) + continue; + + iter->index = 4; + iter->end = ext[2]; + if (iter->end == 0) + iter->end = 127; + if (iter->end < 4 || iter->end > 127) + continue; + + return ext; + } + + return NULL; +} + +/* + * References: + * - DisplayID v1.3 Appendix C: CEA Data Block within a DisplayID Data Block + * - DisplayID v2.0 section 4.10 CTA DisplayID Data Block + * + * Note that the above do not specify any connection between DisplayID Data + * Block revision and CEA Extension versions. + */ +static const void *__cea_db_iter_displayid_next(struct cea_db_iter *iter) +{ + const struct displayid_block *block; + + displayid_iter_for_each(block, &iter->displayid_iter) { + if (block->tag != DATA_BLOCK_CTA) + continue; + + iter->index = sizeof(*block); + iter->end = iter->index + block->num_bytes; + + return block; + } + + return NULL; +} + +static const struct cea_db *__cea_db_iter_next(struct cea_db_iter *iter) +{ + const struct cea_db *db; + + if (iter->collection) { + /* Current collection should always be valid. */ + db = __cea_db_iter_current_block(iter); + if (WARN_ON(!db)) { + iter->collection = NULL; + return NULL; + } + + /* Next block in CEA Data Block Collection */ + iter->index += sizeof(*db) + cea_db_payload_len(db); + + db = __cea_db_iter_current_block(iter); + if (db) + return db; + } + + for (;;) { + /* + * Find the next CEA Data Block Collection. First iterate all + * the EDID CEA extensions, then all the DisplayID CTA blocks. + * + * Per DisplayID v1.3 Appendix B: DisplayID as an EDID + * Extension, it's recommended that DisplayID extensions are + * exposed after all of the CEA extensions. + */ + iter->collection = __cea_db_iter_edid_next(iter); + if (!iter->collection) + iter->collection = __cea_db_iter_displayid_next(iter); + + if (!iter->collection) + return NULL; + + db = __cea_db_iter_current_block(iter); + if (db) + return db; + } +} + +#define cea_db_iter_for_each(__db, __iter) \ + while (((__db) = __cea_db_iter_next(__iter))) + +static void cea_db_iter_end(struct cea_db_iter *iter) +{ + displayid_iter_end(&iter->displayid_iter); + drm_edid_iter_end(&iter->edid_iter); + + memset(iter, 0, sizeof(*iter)); +} + static bool cea_db_is_hdmi_vsdb(const u8 *db) { if (cea_db_tag(db) != CEA_DB_VENDOR)
On Tue, Mar 22, 2022 at 11:40:34PM +0200, Jani Nikula wrote:
Add an iterator for CEA Data Blocks across CEA extensions and CTA DisplayID Data Blocks.
Signed-off-by: Jani Nikula jani.nikula@intel.com
drivers/gpu/drm/drm_edid.c | 198 ++++++++++++++++++++++++++++++++++--- 1 file changed, 186 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 31d132fcd0ca..c12c3cbab274 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -4196,24 +4196,12 @@ do_hdmi_vsdb_modes(struct drm_connector *connector, const u8 *db, u8 len, return modes; }
-static int -cea_db_payload_len(const u8 *db) -{
- return db[0] & 0x1f;
-}
static int cea_db_extended_tag(const u8 *db) { return db[1]; }
-static int -cea_db_tag(const u8 *db) -{
- return db[0] >> 5;
-}
static int cea_revision(const u8 *cea) { @@ -4269,6 +4257,192 @@ cea_db_offsets(const u8 *cea, int *start, int *end) return 0; }
+/*
- CEA Data Block iterator.
- Iterate through all CEA Data Blocks in both EDID CEA extensions and CTA Data
- Blocks in DisplayID extension blocks.
- struct cea_db *db:
- struct cea_db_iter iter;
- cea_db_iter_edid_begin(edid, &iter);
- cea_db_iter_for_each(db, &iter) {
// do stuff with db
- }
- cea_db_iter_end(&iter);
- */
+struct cea_db_iter {
- struct drm_edid_iter edid_iter;
- struct displayid_iter displayid_iter;
- /* Current Data Block Collection. */
- const u8 *collection;
- /* Current Data Block index in current collection. */
- int index;
- /* End index in current collection. */
- int end;
+};
+/* CEA-861-F section 7.5 CEA Extension Version 3 and Table 43 */ +struct cea_db {
- u8 tag_length;
- u8 data[];
+} __packed;
+static int cea_db_tag(const void *_db) +{
- /* FIXME: Transition to passing struct cea_db * everywhere. */
- const struct cea_db *db = _db;
- return db->tag_length >> 5;
+}
+static int cea_db_payload_len(const void *_db) +{
- /* FIXME: Transition to passing struct cea_db * everywhere. */
- const struct cea_db *db = _db;
- return db->tag_length & 0x1f;
+}
+static const void *cea_db_data(const struct cea_db *db) +{
- return db->data;
+}
+static void cea_db_iter_edid_begin(const struct edid *edid, struct cea_db_iter *iter) +{
- memset(iter, 0, sizeof(*iter));
- drm_edid_iter_begin(edid, &iter->edid_iter);
- displayid_iter_edid_begin(edid, &iter->displayid_iter);
+}
+static const struct cea_db * +__cea_db_iter_current_block(const struct cea_db_iter *iter) +{
- const struct cea_db *db;
- if (!iter->collection)
return NULL;- db = (const struct cea_db *)&iter->collection[iter->index];
- if (iter->index + sizeof(*db) <= iter->end &&
iter->index + sizeof(*db) + cea_db_payload_len(db) <= iter->end)return db;- return NULL;
+}
+/*
- References:
- VESA E-EDID v1.4
- CEA-861-F section 7.5 CEA Extension Version 3
- */
+static const void *__cea_db_iter_edid_next(struct cea_db_iter *iter) +{
- const u8 *ext;
- drm_edid_iter_for_each(ext, &iter->edid_iter) {
/* Only support CEA extension revision 3+ */if (ext[0] != CEA_EXT || cea_revision(ext) < 3)continue;iter->index = 4;iter->end = ext[2];if (iter->end == 0)iter->end = 127;if (iter->end < 4 || iter->end > 127)continue;return ext;- }
- return NULL;
+}
+/*
- References:
- DisplayID v1.3 Appendix C: CEA Data Block within a DisplayID Data Block
- DisplayID v2.0 section 4.10 CTA DisplayID Data Block
- Note that the above do not specify any connection between DisplayID Data
- Block revision and CEA Extension versions.
- */
+static const void *__cea_db_iter_displayid_next(struct cea_db_iter *iter) +{
- const struct displayid_block *block;
- displayid_iter_for_each(block, &iter->displayid_iter) {
if (block->tag != DATA_BLOCK_CTA)continue;iter->index = sizeof(*block);iter->end = iter->index + block->num_bytes;
I'd like to keep the comment from cea_db_offsets() reminding us why we can trust this thing.
Overall looks pretty nice to my eyes. Reviewed-by: Ville Syrjälä ville.syrjala@linux.intel.com
return block;- }
- return NULL;
+}
+static const struct cea_db *__cea_db_iter_next(struct cea_db_iter *iter) +{
- const struct cea_db *db;
- if (iter->collection) {
/* Current collection should always be valid. */db = __cea_db_iter_current_block(iter);if (WARN_ON(!db)) {iter->collection = NULL;return NULL;}/* Next block in CEA Data Block Collection */iter->index += sizeof(*db) + cea_db_payload_len(db);db = __cea_db_iter_current_block(iter);if (db)return db;- }
- for (;;) {
/** Find the next CEA Data Block Collection. First iterate all* the EDID CEA extensions, then all the DisplayID CTA blocks.** Per DisplayID v1.3 Appendix B: DisplayID as an EDID* Extension, it's recommended that DisplayID extensions are* exposed after all of the CEA extensions.*/iter->collection = __cea_db_iter_edid_next(iter);if (!iter->collection)iter->collection = __cea_db_iter_displayid_next(iter);if (!iter->collection)return NULL;db = __cea_db_iter_current_block(iter);if (db)return db;- }
+}
+#define cea_db_iter_for_each(__db, __iter) \
- while (((__db) = __cea_db_iter_next(__iter)))
+static void cea_db_iter_end(struct cea_db_iter *iter) +{
- displayid_iter_end(&iter->displayid_iter);
- drm_edid_iter_end(&iter->edid_iter);
- memset(iter, 0, sizeof(*iter));
+}
static bool cea_db_is_hdmi_vsdb(const u8 *db) { if (cea_db_tag(db) != CEA_DB_VENDOR) -- 2.30.2
Abstract helpers for matching vendor data blocks and extended tags, and use them to simplify all the cea_db_is_*() functions.
Take void pointer as parameter to allow transitional use for both u8 * and struct cea_db *.
Signed-off-by: Jani Nikula jani.nikula@intel.com --- drivers/gpu/drm/drm_edid.c | 113 ++++++++++++------------------------- 1 file changed, 37 insertions(+), 76 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index c12c3cbab274..a0a5a7271658 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -4196,12 +4196,6 @@ do_hdmi_vsdb_modes(struct drm_connector *connector, const u8 *db, u8 len, return modes; }
-static int -cea_db_extended_tag(const u8 *db) -{ - return db[1]; -} - static int cea_revision(const u8 *cea) { @@ -4313,6 +4307,22 @@ static const void *cea_db_data(const struct cea_db *db) return db->data; }
+static bool cea_db_is_extended_tag(const struct cea_db *db, int tag) +{ + return (cea_db_tag(db) == CEA_DB_EXTENDED_TAG && + cea_db_payload_len(db) >= 1 && + db->data[0] == tag); +} + +static bool cea_db_is_vendor(const struct cea_db *db, int vendor_oui) +{ + const u8 *data = cea_db_data(db); + + return (cea_db_tag(db) == CEA_DB_VENDOR && + cea_db_payload_len(db) >= 3 && + oui(data[2], data[1], data[0]) == vendor_oui); +} + static void cea_db_iter_edid_begin(const struct edid *edid, struct cea_db_iter *iter) { memset(iter, 0, sizeof(*iter)); @@ -4443,79 +4453,44 @@ static void cea_db_iter_end(struct cea_db_iter *iter) memset(iter, 0, sizeof(*iter)); }
-static bool cea_db_is_hdmi_vsdb(const u8 *db) +static bool cea_db_is_hdmi_vsdb(const void *db) { - if (cea_db_tag(db) != CEA_DB_VENDOR) - return false; - - if (cea_db_payload_len(db) < 5) - return false; - - return oui(db[3], db[2], db[1]) == HDMI_IEEE_OUI; + return (cea_db_is_vendor(db, HDMI_IEEE_OUI) && + cea_db_payload_len(db) >= 5); }
-static bool cea_db_is_hdmi_forum_vsdb(const u8 *db) +static bool cea_db_is_hdmi_forum_vsdb(const void *db) { - if (cea_db_tag(db) != CEA_DB_VENDOR) - return false; - - if (cea_db_payload_len(db) < 7) - return false; - - return oui(db[3], db[2], db[1]) == HDMI_FORUM_IEEE_OUI; + return (cea_db_is_vendor(db, HDMI_FORUM_IEEE_OUI) && + cea_db_payload_len(db) >= 7); }
-static bool cea_db_is_microsoft_vsdb(const u8 *db) +static bool cea_db_is_microsoft_vsdb(const void *db) { - if (cea_db_tag(db) != CEA_DB_VENDOR) - return false; - - if (cea_db_payload_len(db) != 21) - return false; - - return oui(db[3], db[2], db[1]) == MICROSOFT_IEEE_OUI; + return (cea_db_is_vendor(db, MICROSOFT_IEEE_OUI) && + cea_db_payload_len(db) == 21); }
-static bool cea_db_is_vcdb(const u8 *db) +static bool cea_db_is_vcdb(const void *db) { - if (cea_db_tag(db) != CEA_DB_EXTENDED_TAG) - return false; - - if (cea_db_payload_len(db) != 2) - return false; - - if (cea_db_extended_tag(db) != CEA_EXT_DB_VIDEO_CAP) - return false; - - return true; + return (cea_db_is_extended_tag(db, CEA_EXT_DB_VIDEO_CAP) && + cea_db_payload_len(db) == 2); }
-static bool cea_db_is_y420cmdb(const u8 *db) +static bool cea_db_is_y420cmdb(const void *db) { - if (cea_db_tag(db) != CEA_DB_EXTENDED_TAG) - return false; - - if (!cea_db_payload_len(db)) - return false; - - if (cea_db_extended_tag(db) != CEA_EXT_DB_420_VIDEO_CAP_MAP) - return false; - - return true; + return cea_db_is_extended_tag(db, CEA_EXT_DB_420_VIDEO_CAP_MAP); }
-static bool cea_db_is_y420vdb(const u8 *db) +static bool cea_db_is_y420vdb(const void *db) { - if (cea_db_tag(db) != CEA_DB_EXTENDED_TAG) - return false; - - if (!cea_db_payload_len(db)) - return false; - - if (cea_db_extended_tag(db) != CEA_EXT_DB_420_VIDEO_DATA) - return false; + return cea_db_is_extended_tag(db, CEA_EXT_DB_420_VIDEO_DATA); +}
- return true; +static bool cea_db_is_hdmi_hdr_metadata_block(const void *db) +{ + return (cea_db_is_extended_tag(db, CEA_EXT_DB_HDR_STATIC_METADATA) && + cea_db_payload_len(db) >= 3); }
#define for_each_cea_db(cea, i, start, end) \ @@ -4651,20 +4626,6 @@ static void fixup_detailed_cea_mode_clock(struct drm_display_mode *mode) mode->clock = clock; }
-static bool cea_db_is_hdmi_hdr_metadata_block(const u8 *db) -{ - if (cea_db_tag(db) != CEA_DB_EXTENDED_TAG) - return false; - - if (db[1] != CEA_EXT_DB_HDR_STATIC_METADATA) - return false; - - if (cea_db_payload_len(db) < 3) - return false; - - return true; -} - static uint8_t eotf_supported(const u8 *edid_ext) { return edid_ext[2] &
On Tue, Mar 22, 2022 at 11:40:35PM +0200, Jani Nikula wrote:
Abstract helpers for matching vendor data blocks and extended tags, and use them to simplify all the cea_db_is_*() functions.
Take void pointer as parameter to allow transitional use for both u8 * and struct cea_db *.
Signed-off-by: Jani Nikula jani.nikula@intel.com
drivers/gpu/drm/drm_edid.c | 113 ++++++++++++------------------------- 1 file changed, 37 insertions(+), 76 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index c12c3cbab274..a0a5a7271658 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -4196,12 +4196,6 @@ do_hdmi_vsdb_modes(struct drm_connector *connector, const u8 *db, u8 len, return modes; }
-static int -cea_db_extended_tag(const u8 *db) -{
- return db[1];
-}
static int cea_revision(const u8 *cea) { @@ -4313,6 +4307,22 @@ static const void *cea_db_data(const struct cea_db *db) return db->data; }
+static bool cea_db_is_extended_tag(const struct cea_db *db, int tag) +{
- return (cea_db_tag(db) == CEA_DB_EXTENDED_TAG &&
cea_db_payload_len(db) >= 1 &&db->data[0] == tag);+}
nit: not a huge fan of the redundant parens in all of these
+static bool cea_db_is_vendor(const struct cea_db *db, int vendor_oui)
I'd probably make the tag/oui unsigned.
Reviewed-by: Ville Syrjälä ville.syrjala@linux.intel.com
+{
- const u8 *data = cea_db_data(db);
- return (cea_db_tag(db) == CEA_DB_VENDOR &&
cea_db_payload_len(db) >= 3 &&oui(data[2], data[1], data[0]) == vendor_oui);+}
static void cea_db_iter_edid_begin(const struct edid *edid, struct cea_db_iter *iter) { memset(iter, 0, sizeof(*iter)); @@ -4443,79 +4453,44 @@ static void cea_db_iter_end(struct cea_db_iter *iter) memset(iter, 0, sizeof(*iter)); }
-static bool cea_db_is_hdmi_vsdb(const u8 *db) +static bool cea_db_is_hdmi_vsdb(const void *db) {
- if (cea_db_tag(db) != CEA_DB_VENDOR)
return false;- if (cea_db_payload_len(db) < 5)
return false;- return oui(db[3], db[2], db[1]) == HDMI_IEEE_OUI;
- return (cea_db_is_vendor(db, HDMI_IEEE_OUI) &&
cea_db_payload_len(db) >= 5);}
-static bool cea_db_is_hdmi_forum_vsdb(const u8 *db) +static bool cea_db_is_hdmi_forum_vsdb(const void *db) {
- if (cea_db_tag(db) != CEA_DB_VENDOR)
return false;- if (cea_db_payload_len(db) < 7)
return false;- return oui(db[3], db[2], db[1]) == HDMI_FORUM_IEEE_OUI;
- return (cea_db_is_vendor(db, HDMI_FORUM_IEEE_OUI) &&
cea_db_payload_len(db) >= 7);}
-static bool cea_db_is_microsoft_vsdb(const u8 *db) +static bool cea_db_is_microsoft_vsdb(const void *db) {
- if (cea_db_tag(db) != CEA_DB_VENDOR)
return false;- if (cea_db_payload_len(db) != 21)
return false;- return oui(db[3], db[2], db[1]) == MICROSOFT_IEEE_OUI;
- return (cea_db_is_vendor(db, MICROSOFT_IEEE_OUI) &&
cea_db_payload_len(db) == 21);}
-static bool cea_db_is_vcdb(const u8 *db) +static bool cea_db_is_vcdb(const void *db) {
- if (cea_db_tag(db) != CEA_DB_EXTENDED_TAG)
return false;- if (cea_db_payload_len(db) != 2)
return false;- if (cea_db_extended_tag(db) != CEA_EXT_DB_VIDEO_CAP)
return false;- return true;
- return (cea_db_is_extended_tag(db, CEA_EXT_DB_VIDEO_CAP) &&
cea_db_payload_len(db) == 2);}
-static bool cea_db_is_y420cmdb(const u8 *db) +static bool cea_db_is_y420cmdb(const void *db) {
- if (cea_db_tag(db) != CEA_DB_EXTENDED_TAG)
return false;- if (!cea_db_payload_len(db))
return false;- if (cea_db_extended_tag(db) != CEA_EXT_DB_420_VIDEO_CAP_MAP)
return false;- return true;
- return cea_db_is_extended_tag(db, CEA_EXT_DB_420_VIDEO_CAP_MAP);
}
-static bool cea_db_is_y420vdb(const u8 *db) +static bool cea_db_is_y420vdb(const void *db) {
- if (cea_db_tag(db) != CEA_DB_EXTENDED_TAG)
return false;- if (!cea_db_payload_len(db))
return false;- if (cea_db_extended_tag(db) != CEA_EXT_DB_420_VIDEO_DATA)
return false;
- return cea_db_is_extended_tag(db, CEA_EXT_DB_420_VIDEO_DATA);
+}
- return true;
+static bool cea_db_is_hdmi_hdr_metadata_block(const void *db) +{
- return (cea_db_is_extended_tag(db, CEA_EXT_DB_HDR_STATIC_METADATA) &&
cea_db_payload_len(db) >= 3);}
#define for_each_cea_db(cea, i, start, end) \ @@ -4651,20 +4626,6 @@ static void fixup_detailed_cea_mode_clock(struct drm_display_mode *mode) mode->clock = clock; }
-static bool cea_db_is_hdmi_hdr_metadata_block(const u8 *db) -{
- if (cea_db_tag(db) != CEA_DB_EXTENDED_TAG)
return false;- if (db[1] != CEA_EXT_DB_HDR_STATIC_METADATA)
return false;- if (cea_db_payload_len(db) < 3)
return false;- return true;
-}
static uint8_t eotf_supported(const u8 *edid_ext) { return edid_ext[2] & -- 2.30.2
On Wed, 23 Mar 2022, Ville Syrjälä ville.syrjala@linux.intel.com wrote:
On Tue, Mar 22, 2022 at 11:40:35PM +0200, Jani Nikula wrote:
Abstract helpers for matching vendor data blocks and extended tags, and use them to simplify all the cea_db_is_*() functions.
Take void pointer as parameter to allow transitional use for both u8 * and struct cea_db *.
Signed-off-by: Jani Nikula jani.nikula@intel.com
drivers/gpu/drm/drm_edid.c | 113 ++++++++++++------------------------- 1 file changed, 37 insertions(+), 76 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index c12c3cbab274..a0a5a7271658 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -4196,12 +4196,6 @@ do_hdmi_vsdb_modes(struct drm_connector *connector, const u8 *db, u8 len, return modes; }
-static int -cea_db_extended_tag(const u8 *db) -{
- return db[1];
-}
static int cea_revision(const u8 *cea) { @@ -4313,6 +4307,22 @@ static const void *cea_db_data(const struct cea_db *db) return db->data; }
+static bool cea_db_is_extended_tag(const struct cea_db *db, int tag) +{
- return (cea_db_tag(db) == CEA_DB_EXTENDED_TAG &&
cea_db_payload_len(db) >= 1 &&db->data[0] == tag);+}
nit: not a huge fan of the redundant parens in all of these
I guess that's something I've picked up from Python, makes the subsequent lines align nicely. Otherwise it's less pretty.
+static bool cea_db_is_vendor(const struct cea_db *db, int vendor_oui)
I'd probably make the tag/oui unsigned.
Only chose int because the oui() function returns an int; maybe that should eventually be turned into unsigned int.
Reviewed-by: Ville Syrjälä ville.syrjala@linux.intel.com
+{
- const u8 *data = cea_db_data(db);
- return (cea_db_tag(db) == CEA_DB_VENDOR &&
cea_db_payload_len(db) >= 3 &&oui(data[2], data[1], data[0]) == vendor_oui);+}
static void cea_db_iter_edid_begin(const struct edid *edid, struct cea_db_iter *iter) { memset(iter, 0, sizeof(*iter)); @@ -4443,79 +4453,44 @@ static void cea_db_iter_end(struct cea_db_iter *iter) memset(iter, 0, sizeof(*iter)); }
-static bool cea_db_is_hdmi_vsdb(const u8 *db) +static bool cea_db_is_hdmi_vsdb(const void *db) {
- if (cea_db_tag(db) != CEA_DB_VENDOR)
return false;- if (cea_db_payload_len(db) < 5)
return false;- return oui(db[3], db[2], db[1]) == HDMI_IEEE_OUI;
- return (cea_db_is_vendor(db, HDMI_IEEE_OUI) &&
cea_db_payload_len(db) >= 5);}
-static bool cea_db_is_hdmi_forum_vsdb(const u8 *db) +static bool cea_db_is_hdmi_forum_vsdb(const void *db) {
- if (cea_db_tag(db) != CEA_DB_VENDOR)
return false;- if (cea_db_payload_len(db) < 7)
return false;- return oui(db[3], db[2], db[1]) == HDMI_FORUM_IEEE_OUI;
- return (cea_db_is_vendor(db, HDMI_FORUM_IEEE_OUI) &&
cea_db_payload_len(db) >= 7);}
-static bool cea_db_is_microsoft_vsdb(const u8 *db) +static bool cea_db_is_microsoft_vsdb(const void *db) {
- if (cea_db_tag(db) != CEA_DB_VENDOR)
return false;- if (cea_db_payload_len(db) != 21)
return false;- return oui(db[3], db[2], db[1]) == MICROSOFT_IEEE_OUI;
- return (cea_db_is_vendor(db, MICROSOFT_IEEE_OUI) &&
cea_db_payload_len(db) == 21);}
-static bool cea_db_is_vcdb(const u8 *db) +static bool cea_db_is_vcdb(const void *db) {
- if (cea_db_tag(db) != CEA_DB_EXTENDED_TAG)
return false;- if (cea_db_payload_len(db) != 2)
return false;- if (cea_db_extended_tag(db) != CEA_EXT_DB_VIDEO_CAP)
return false;- return true;
- return (cea_db_is_extended_tag(db, CEA_EXT_DB_VIDEO_CAP) &&
cea_db_payload_len(db) == 2);}
-static bool cea_db_is_y420cmdb(const u8 *db) +static bool cea_db_is_y420cmdb(const void *db) {
- if (cea_db_tag(db) != CEA_DB_EXTENDED_TAG)
return false;- if (!cea_db_payload_len(db))
return false;- if (cea_db_extended_tag(db) != CEA_EXT_DB_420_VIDEO_CAP_MAP)
return false;- return true;
- return cea_db_is_extended_tag(db, CEA_EXT_DB_420_VIDEO_CAP_MAP);
}
-static bool cea_db_is_y420vdb(const u8 *db) +static bool cea_db_is_y420vdb(const void *db) {
- if (cea_db_tag(db) != CEA_DB_EXTENDED_TAG)
return false;- if (!cea_db_payload_len(db))
return false;- if (cea_db_extended_tag(db) != CEA_EXT_DB_420_VIDEO_DATA)
return false;
- return cea_db_is_extended_tag(db, CEA_EXT_DB_420_VIDEO_DATA);
+}
- return true;
+static bool cea_db_is_hdmi_hdr_metadata_block(const void *db) +{
- return (cea_db_is_extended_tag(db, CEA_EXT_DB_HDR_STATIC_METADATA) &&
cea_db_payload_len(db) >= 3);}
#define for_each_cea_db(cea, i, start, end) \ @@ -4651,20 +4626,6 @@ static void fixup_detailed_cea_mode_clock(struct drm_display_mode *mode) mode->clock = clock; }
-static bool cea_db_is_hdmi_hdr_metadata_block(const u8 *db) -{
- if (cea_db_tag(db) != CEA_DB_EXTENDED_TAG)
return false;- if (db[1] != CEA_EXT_DB_HDR_STATIC_METADATA)
return false;- if (cea_db_payload_len(db) < 3)
return false;- return true;
-}
static uint8_t eotf_supported(const u8 *edid_ext) { return edid_ext[2] & -- 2.30.2
Iterate through all CEA EDID extension blocks and DisplayID CTA data blocks to add CEA modes.
Signed-off-by: Jani Nikula jani.nikula@intel.com --- drivers/gpu/drm/drm_edid.c | 67 ++++++++++++++++++-------------------- 1 file changed, 31 insertions(+), 36 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index a0a5a7271658..d92ce5d540c3 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -4539,46 +4539,41 @@ static void drm_parse_y420cmdb_bitmap(struct drm_connector *connector, static int add_cea_modes(struct drm_connector *connector, struct edid *edid) { - const u8 *cea = drm_find_cea_extension(edid); - const u8 *db, *hdmi = NULL, *video = NULL; - u8 dbl, hdmi_len, video_len = 0; + const struct cea_db *db; + struct cea_db_iter iter; int modes = 0;
- if (cea && cea_revision(cea) >= 3) { - int i, start, end; - - if (cea_db_offsets(cea, &start, &end)) - return 0; - - for_each_cea_db(cea, i, start, end) { - db = &cea[i]; - dbl = cea_db_payload_len(db); - - if (cea_db_tag(db) == CEA_DB_VIDEO) { - video = db + 1; - video_len = dbl; - modes += do_cea_modes(connector, video, dbl); - } else if (cea_db_is_hdmi_vsdb(db)) { - hdmi = db; - hdmi_len = dbl; - } else if (cea_db_is_y420vdb(db)) { - const u8 *vdb420 = &db[2]; - - /* Add 4:2:0(only) modes present in EDID */ - modes += do_y420vdb_modes(connector, - vdb420, - dbl - 1); - } + cea_db_iter_edid_begin(edid, &iter); + cea_db_iter_for_each(db, &iter) { + const u8 *hdmi = NULL, *video = NULL; + u8 hdmi_len = 0, video_len = 0; + + if (cea_db_tag(db) == CEA_DB_VIDEO) { + video = cea_db_data(db); + video_len = cea_db_payload_len(db); + modes += do_cea_modes(connector, video, video_len); + } else if (cea_db_is_hdmi_vsdb(db)) { + /* FIXME: Switch to use cea_db_data() */ + hdmi = (const u8 *)db; + hdmi_len = cea_db_payload_len(db); + } else if (cea_db_is_y420vdb(db)) { + const u8 *vdb420 = cea_db_data(db) + 1; + + /* Add 4:2:0(only) modes present in EDID */ + modes += do_y420vdb_modes(connector, vdb420, + cea_db_payload_len(db) - 1); } - }
- /* - * We parse the HDMI VSDB after having added the cea modes as we will - * be patching their flags when the sink supports stereo 3D. - */ - if (hdmi) - modes += do_hdmi_vsdb_modes(connector, hdmi, hdmi_len, video, - video_len); + /* + * We parse the HDMI VSDB after having added the cea modes as we + * will be patching their flags when the sink supports stereo + * 3D. + */ + if (hdmi) + modes += do_hdmi_vsdb_modes(connector, hdmi, hdmi_len, + video, video_len); + } + cea_db_iter_end(&iter);
return modes; }
Use the cea db iterator for speaker allocation. We'll still stop at the first speaker data block, but not at the first CEA extension if that doesn't have the info.
Signed-off-by: Jani Nikula jani.nikula@intel.com --- drivers/gpu/drm/drm_edid.c | 47 ++++++++++++-------------------------- 1 file changed, 15 insertions(+), 32 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index d92ce5d540c3..992b3578a73f 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -4916,42 +4916,25 @@ EXPORT_SYMBOL(drm_edid_to_sad); */ int drm_edid_to_speaker_allocation(struct edid *edid, u8 **sadb) { + const struct cea_db *db; + struct cea_db_iter iter; int count = 0; - int i, start, end, dbl; - const u8 *cea;
- cea = drm_find_cea_extension(edid); - if (!cea) { - DRM_DEBUG_KMS("SAD: no CEA Extension found\n"); - return 0; - } - - if (cea_revision(cea) < 3) { - DRM_DEBUG_KMS("SAD: wrong CEA revision\n"); - return 0; - } - - if (cea_db_offsets(cea, &start, &end)) { - DRM_DEBUG_KMS("SAD: invalid data block offsets\n"); - return -EPROTO; - } - - for_each_cea_db(cea, i, start, end) { - const u8 *db = &cea[i]; - - if (cea_db_tag(db) == CEA_DB_SPEAKER) { - dbl = cea_db_payload_len(db); - - /* Speaker Allocation Data Block */ - if (dbl == 3) { - *sadb = kmemdup(&db[1], dbl, GFP_KERNEL); - if (!*sadb) - return -ENOMEM; - count = dbl; - break; - } + cea_db_iter_edid_begin(edid, &iter); + cea_db_iter_for_each(db, &iter) { + if (cea_db_tag(db) == CEA_DB_SPEAKER && + cea_db_payload_len(db) == 3) { + *sadb = kmemdup(db->data, cea_db_payload_len(db), + GFP_KERNEL); + if (!*sadb) + return -ENOMEM; + count = cea_db_payload_len(db); + break; } } + cea_db_iter_end(&iter); + + DRM_DEBUG_KMS("Found %d Speaker Allocation Data Blocks\n", count);
return count; }
Use the cea db iterator for short audio descriptors. We'll still stop at the first audio data block, but not at the first CEA extension if that doesn't have the info.
Signed-off-by: Jani Nikula jani.nikula@intel.com --- drivers/gpu/drm/drm_edid.c | 34 +++++++++------------------------- 1 file changed, 9 insertions(+), 25 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 992b3578a73f..e341790521d6 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -4854,40 +4854,21 @@ static void drm_edid_to_eld(struct drm_connector *connector, struct edid *edid) */ int drm_edid_to_sad(struct edid *edid, struct cea_sad **sads) { + const struct cea_db *db; + struct cea_db_iter iter; int count = 0; - int i, start, end, dbl; - const u8 *cea; - - cea = drm_find_cea_extension(edid); - if (!cea) { - DRM_DEBUG_KMS("SAD: no CEA Extension found\n"); - return 0; - } - - if (cea_revision(cea) < 3) { - DRM_DEBUG_KMS("SAD: wrong CEA revision\n"); - return 0; - } - - if (cea_db_offsets(cea, &start, &end)) { - DRM_DEBUG_KMS("SAD: invalid data block offsets\n"); - return -EPROTO; - } - - for_each_cea_db(cea, i, start, end) { - const u8 *db = &cea[i];
+ cea_db_iter_edid_begin(edid, &iter); + cea_db_iter_for_each(db, &iter) { if (cea_db_tag(db) == CEA_DB_AUDIO) { int j;
- dbl = cea_db_payload_len(db); - - count = dbl / 3; /* SAD is 3B */ + count = cea_db_payload_len(db) / 3; /* SAD is 3B */ *sads = kcalloc(count, sizeof(**sads), GFP_KERNEL); if (!*sads) return -ENOMEM; for (j = 0; j < count; j++) { - const u8 *sad = &db[1 + j * 3]; + const u8 *sad = &db->data[j * 3];
(*sads)[j].format = (sad[0] & 0x78) >> 3; (*sads)[j].channels = sad[0] & 0x7; @@ -4897,6 +4878,9 @@ int drm_edid_to_sad(struct edid *edid, struct cea_sad **sads) break; } } + cea_db_iter_end(&iter); + + DRM_DEBUG_KMS("Found %d Short Audio Descriptors\n", count);
return count; }
On Tue, Mar 22, 2022 at 11:40:38PM +0200, Jani Nikula wrote:
Use the cea db iterator for short audio descriptors. We'll still stop at the first audio data block, but not at the first CEA extension if that doesn't have the info.
This stuff should probably be converted over to the drm_edid_to_eld() approach which looks up all the SADs from the whole EDID. But that's something for amdgpu/radeon folks to think about since they're the only user.
Signed-off-by: Jani Nikula jani.nikula@intel.com
drivers/gpu/drm/drm_edid.c | 34 +++++++++------------------------- 1 file changed, 9 insertions(+), 25 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 992b3578a73f..e341790521d6 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -4854,40 +4854,21 @@ static void drm_edid_to_eld(struct drm_connector *connector, struct edid *edid) */ int drm_edid_to_sad(struct edid *edid, struct cea_sad **sads) {
- const struct cea_db *db;
- struct cea_db_iter iter; int count = 0;
- int i, start, end, dbl;
- const u8 *cea;
- cea = drm_find_cea_extension(edid);
- if (!cea) {
DRM_DEBUG_KMS("SAD: no CEA Extension found\n");return 0;- }
- if (cea_revision(cea) < 3) {
DRM_DEBUG_KMS("SAD: wrong CEA revision\n");return 0;- }
- if (cea_db_offsets(cea, &start, &end)) {
DRM_DEBUG_KMS("SAD: invalid data block offsets\n");return -EPROTO;- }
- for_each_cea_db(cea, i, start, end) {
const u8 *db = &cea[i];
- cea_db_iter_edid_begin(edid, &iter);
- cea_db_iter_for_each(db, &iter) { if (cea_db_tag(db) == CEA_DB_AUDIO) { int j;
dbl = cea_db_payload_len(db);count = dbl / 3; /* SAD is 3B */
count = cea_db_payload_len(db) / 3; /* SAD is 3B */ *sads = kcalloc(count, sizeof(**sads), GFP_KERNEL); if (!*sads) return -ENOMEM; for (j = 0; j < count; j++) {
const u8 *sad = &db[1 + j * 3];
const u8 *sad = &db->data[j * 3]; (*sads)[j].format = (sad[0] & 0x78) >> 3; (*sads)[j].channels = sad[0] & 0x7;@@ -4897,6 +4878,9 @@ int drm_edid_to_sad(struct edid *edid, struct cea_sad **sads) break; } }
cea_db_iter_end(&iter);
DRM_DEBUG_KMS("Found %d Short Audio Descriptors\n", count);
return count;
}
2.30.2
Iterate through all CEA data blocks, not just the first CEA extension.
Signed-off-by: Jani Nikula jani.nikula@intel.com --- drivers/gpu/drm/drm_edid.c | 25 +++++++++++-------------- 1 file changed, 11 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index e341790521d6..399e69dc1aed 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -4978,27 +4978,24 @@ EXPORT_SYMBOL(drm_av_sync_delay); */ bool drm_detect_hdmi_monitor(struct edid *edid) { - const u8 *edid_ext; - int i; - int start_offset, end_offset; - - edid_ext = drm_find_cea_extension(edid); - if (!edid_ext) - return false; - - if (cea_db_offsets(edid_ext, &start_offset, &end_offset)) - return false; + const struct cea_db *db; + struct cea_db_iter iter; + bool hdmi = false;
/* * Because HDMI identifier is in Vendor Specific Block, * search it from all data blocks of CEA extension. */ - for_each_cea_db(edid_ext, i, start_offset, end_offset) { - if (cea_db_is_hdmi_vsdb(&edid_ext[i])) - return true; + cea_db_iter_edid_begin(edid, &iter); + cea_db_iter_for_each(db, &iter) { + if (cea_db_is_hdmi_vsdb(db)) { + hdmi = true; + break; + } } + cea_db_iter_end(&iter);
- return false; + return hdmi; } EXPORT_SYMBOL(drm_detect_hdmi_monitor);
Iterate through all CEA data blocks.
Signed-off-by: Jani Nikula jani.nikula@intel.com --- drivers/gpu/drm/drm_edid.c | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 399e69dc1aed..77763d94dd93 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -5013,10 +5013,10 @@ EXPORT_SYMBOL(drm_detect_hdmi_monitor); */ bool drm_detect_monitor_audio(struct edid *edid) { + const struct cea_db *db; + struct cea_db_iter iter; const u8 *edid_ext; - int i, j; bool has_audio = false; - int start_offset, end_offset;
edid_ext = drm_find_cea_extension(edid); if (!edid_ext) @@ -5029,18 +5029,21 @@ bool drm_detect_monitor_audio(struct edid *edid) goto end; }
- if (cea_db_offsets(edid_ext, &start_offset, &end_offset)) - goto end; + cea_db_iter_edid_begin(edid, &iter); + cea_db_iter_for_each(db, &iter) { + if (cea_db_tag(db) == CEA_DB_AUDIO) { + const u8 *data = cea_db_data(db); + int i;
- for_each_cea_db(edid_ext, i, start_offset, end_offset) { - if (cea_db_tag(&edid_ext[i]) == CEA_DB_AUDIO) { - has_audio = true; - for (j = 1; j < cea_db_payload_len(&edid_ext[i]) + 1; j += 3) + for (i = 0; i < cea_db_payload_len(db); i += 3) DRM_DEBUG_KMS("CEA audio format %d\n", - (edid_ext[i + j] >> 3) & 0xf); - goto end; + (data[i] >> 3) & 0xf); + has_audio = true; + break; } } + cea_db_iter_end(&iter); + end: return has_audio; }
Iterate through all CEA data blocks across all CEA extensions and DisplayID data blocks.
Signed-off-by: Jani Nikula jani.nikula@intel.com --- drivers/gpu/drm/drm_edid.c | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 77763d94dd93..d4bf9ef09c3c 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -5342,8 +5342,9 @@ static void drm_parse_cea_ext(struct drm_connector *connector, const struct edid *edid) { struct drm_display_info *info = &connector->display_info; + const struct cea_db *db; + struct cea_db_iter iter; const u8 *edid_ext; - int i, start, end;
edid_ext = drm_find_cea_extension(edid); if (!edid_ext) @@ -5358,25 +5359,25 @@ static void drm_parse_cea_ext(struct drm_connector *connector, if (edid_ext[3] & EDID_CEA_YCRCB422) info->color_formats |= DRM_COLOR_FORMAT_YCBCR422;
- if (cea_db_offsets(edid_ext, &start, &end)) - return; - - for_each_cea_db(edid_ext, i, start, end) { - const u8 *db = &edid_ext[i]; + cea_db_iter_edid_begin(edid, &iter); + cea_db_iter_for_each(db, &iter) { + /* FIXME: convert parsers to use struct cea_db */ + const u8 *data = (const u8 *)db;
if (cea_db_is_hdmi_vsdb(db)) - drm_parse_hdmi_vsdb_video(connector, db); + drm_parse_hdmi_vsdb_video(connector, data); if (cea_db_is_hdmi_forum_vsdb(db)) - drm_parse_hdmi_forum_vsdb(connector, db); + drm_parse_hdmi_forum_vsdb(connector, data); if (cea_db_is_microsoft_vsdb(db)) - drm_parse_microsoft_vsdb(connector, db); + drm_parse_microsoft_vsdb(connector, data); if (cea_db_is_y420cmdb(db)) - drm_parse_y420cmdb_bitmap(connector, db); + drm_parse_y420cmdb_bitmap(connector, data); if (cea_db_is_vcdb(db)) - drm_parse_vcdb(connector, db); + drm_parse_vcdb(connector, data); if (cea_db_is_hdmi_hdr_metadata_block(db)) - drm_parse_hdr_metadata_block(connector, db); + drm_parse_hdr_metadata_block(connector, data); } + cea_db_iter_end(&iter); }
static
Iterate through all CEA data blocks across all CEA extensions and DisplayID data blocks. This may gather more data than before, and if there's duplicated data, some is overwritten by whichever comes last.
Signed-off-by: Jani Nikula jani.nikula@intel.com --- drivers/gpu/drm/drm_edid.c | 64 +++++++++++++++++--------------------- 1 file changed, 29 insertions(+), 35 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index d4bf9ef09c3c..ccbaa91b171d 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -4758,12 +4758,12 @@ static void clear_eld(struct drm_connector *connector) */ static void drm_edid_to_eld(struct drm_connector *connector, struct edid *edid) { + const struct cea_db *db; + struct cea_db_iter iter; uint8_t *eld = connector->eld; const u8 *cea; - const u8 *db; int total_sad_count = 0; int mnl; - int dbl;
clear_eld(connector);
@@ -4789,43 +4789,37 @@ static void drm_edid_to_eld(struct drm_connector *connector, struct edid *edid) eld[DRM_ELD_PRODUCT_CODE0] = edid->prod_code[0]; eld[DRM_ELD_PRODUCT_CODE1] = edid->prod_code[1];
- if (cea_revision(cea) >= 3) { - int i, start, end; + cea_db_iter_edid_begin(edid, &iter); + cea_db_iter_for_each(db, &iter) { + const u8 *data = cea_db_data(db); + int len = cea_db_payload_len(db); int sad_count;
- if (cea_db_offsets(cea, &start, &end)) { - start = 0; - end = 0; - } - - for_each_cea_db(cea, i, start, end) { - db = &cea[i]; - dbl = cea_db_payload_len(db); - - switch (cea_db_tag(db)) { - case CEA_DB_AUDIO: - /* Audio Data Block, contains SADs */ - sad_count = min(dbl / 3, 15 - total_sad_count); - if (sad_count >= 1) - memcpy(&eld[DRM_ELD_CEA_SAD(mnl, total_sad_count)], - &db[1], sad_count * 3); - total_sad_count += sad_count; - break; - case CEA_DB_SPEAKER: - /* Speaker Allocation Data Block */ - if (dbl >= 1) - eld[DRM_ELD_SPEAKER] = db[1]; - break; - case CEA_DB_VENDOR: - /* HDMI Vendor-Specific Data Block */ - if (cea_db_is_hdmi_vsdb(db)) - drm_parse_hdmi_vsdb_audio(connector, db); - break; - default: - break; - } + switch (cea_db_tag(db)) { + case CEA_DB_AUDIO: + /* Audio Data Block, contains SADs */ + sad_count = min(len / 3, 15 - total_sad_count); + if (sad_count >= 1) + memcpy(&eld[DRM_ELD_CEA_SAD(mnl, total_sad_count)], + data, sad_count * 3); + total_sad_count += sad_count; + break; + case CEA_DB_SPEAKER: + /* Speaker Allocation Data Block */ + if (len >= 1) + eld[DRM_ELD_SPEAKER] = data[0]; + break; + case CEA_DB_VENDOR: + /* HDMI Vendor-Specific Data Block */ + if (cea_db_is_hdmi_vsdb(db)) + drm_parse_hdmi_vsdb_audio(connector, (const u8 *)db); + break; + default: + break; } } + cea_db_iter_end(&iter); + eld[DRM_ELD_SAD_COUNT_CONN_TYPE] |= total_sad_count << DRM_ELD_SAD_COUNT_SHIFT;
if (connector->connector_type == DRM_MODE_CONNECTOR_DisplayPort ||
All CEA data block iteration has now been converted to the new cea db iterators.
Signed-off-by: Jani Nikula jani.nikula@intel.com --- drivers/gpu/drm/drm_edid.c | 45 -------------------------------------- 1 file changed, 45 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index ccbaa91b171d..0400c4024de7 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -4209,48 +4209,6 @@ cea_revision(const u8 *cea) return cea[1]; }
-static int -cea_db_offsets(const u8 *cea, int *start, int *end) -{ - /* DisplayID CTA extension blocks and top-level CEA EDID - * block header definitions differ in the following bytes: - * 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) { - /* - * for_each_displayid_db() has already verified - * that these stay within expected bounds. - */ - *start = 3; - *end = *start + cea[2]; - } else if (cea[0] == CEA_EXT) { - /* Data block offset in CEA extension block */ - *start = 4; - *end = cea[2]; - if (*end == 0) - *end = 127; - if (*end < 4 || *end > 127) - return -ERANGE; - } else { - return -EOPNOTSUPP; - } - - return 0; -} - /* * CEA Data Block iterator. * @@ -4493,9 +4451,6 @@ static bool cea_db_is_hdmi_hdr_metadata_block(const void *db) cea_db_payload_len(db) >= 3); }
-#define for_each_cea_db(cea, i, start, end) \ - for ((i) = (start); (i) < (end) && (i) + cea_db_payload_len(&(cea)[(i)]) < (end); (i) += cea_db_payload_len(&(cea)[(i)]) + 1) - static void drm_parse_y420cmdb_bitmap(struct drm_connector *connector, const u8 *db) {
During the transition, we accepted a void pointer for a poor C programmer's version of polymorphism. Switch the functions to use struct cea_db * to regain some more type safety.
Signed-off-by: Jani Nikula jani.nikula@intel.com --- drivers/gpu/drm/drm_edid.c | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 0400c4024de7..b3aedeefed82 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -4244,11 +4244,8 @@ struct cea_db { u8 data[]; } __packed;
-static int cea_db_tag(const void *_db) +static int cea_db_tag(const struct cea_db *db) { - /* FIXME: Transition to passing struct cea_db * everywhere. */ - const struct cea_db *db = _db; - return db->tag_length >> 5; }
@@ -4411,41 +4408,41 @@ static void cea_db_iter_end(struct cea_db_iter *iter) memset(iter, 0, sizeof(*iter)); }
-static bool cea_db_is_hdmi_vsdb(const void *db) +static bool cea_db_is_hdmi_vsdb(const struct cea_db *db) { return (cea_db_is_vendor(db, HDMI_IEEE_OUI) && cea_db_payload_len(db) >= 5); }
-static bool cea_db_is_hdmi_forum_vsdb(const void *db) +static bool cea_db_is_hdmi_forum_vsdb(const struct cea_db *db) { return (cea_db_is_vendor(db, HDMI_FORUM_IEEE_OUI) && cea_db_payload_len(db) >= 7); }
-static bool cea_db_is_microsoft_vsdb(const void *db) +static bool cea_db_is_microsoft_vsdb(const struct cea_db *db) { return (cea_db_is_vendor(db, MICROSOFT_IEEE_OUI) && cea_db_payload_len(db) == 21); }
-static bool cea_db_is_vcdb(const void *db) +static bool cea_db_is_vcdb(const struct cea_db *db) { return (cea_db_is_extended_tag(db, CEA_EXT_DB_VIDEO_CAP) && cea_db_payload_len(db) == 2); }
-static bool cea_db_is_y420cmdb(const void *db) +static bool cea_db_is_y420cmdb(const struct cea_db *db) { return cea_db_is_extended_tag(db, CEA_EXT_DB_420_VIDEO_CAP_MAP); }
-static bool cea_db_is_y420vdb(const void *db) +static bool cea_db_is_y420vdb(const struct cea_db *db) { return cea_db_is_extended_tag(db, CEA_EXT_DB_420_VIDEO_DATA); }
-static bool cea_db_is_hdmi_hdr_metadata_block(const void *db) +static bool cea_db_is_hdmi_hdr_metadata_block(const struct cea_db *db) { return (cea_db_is_extended_tag(db, CEA_EXT_DB_HDR_STATIC_METADATA) && cea_db_payload_len(db) >= 3);
The CTA data block in DisplayID does not have the bits from byte 3 that a CEA extension has. Only look for them in CEA extensions, but also look for them in all CEA extensions.
References: https://patchwork.freedesktop.org/patch/msgid/20220321044330.27723-1-cooper.... Signed-off-by: Jani Nikula jani.nikula@intel.com --- drivers/gpu/drm/drm_edid.c | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index b3aedeefed82..b6675f8638bb 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -4959,16 +4959,21 @@ EXPORT_SYMBOL(drm_detect_hdmi_monitor); */ bool drm_detect_monitor_audio(struct edid *edid) { + struct drm_edid_iter edid_iter; const struct cea_db *db; struct cea_db_iter iter; - const u8 *edid_ext; + const u8 *cea; bool has_audio = false;
- edid_ext = drm_find_cea_extension(edid); - if (!edid_ext) - goto end; - - has_audio = ((edid_ext[3] & EDID_BASIC_AUDIO) != 0); + drm_edid_iter_begin(edid, &edid_iter); + drm_edid_iter_for_each(cea, &edid_iter) { + if (cea[0] == CEA_EXT) { + has_audio = cea[3] & EDID_BASIC_AUDIO; + if (has_audio) + break; + } + } + drm_edid_iter_end(&edid_iter);
if (has_audio) { DRM_DEBUG_KMS("Monitor has basic audio support\n");
The CTA data block in DisplayID does not have the bits from byte 3 that a CEA extension has. Only look for them in CEA extensions, but also look for them in all CEA extensions.
The DisplayID CTA data block version does not seem to match the CEA revision either. Ignore it for the purpose of CEA revision.
Signed-off-by: Jani Nikula jani.nikula@intel.com --- drivers/gpu/drm/drm_edid.c | 31 ++++++++++++++++++++----------- 1 file changed, 20 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index b6675f8638bb..f40427dc5236 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -5293,22 +5293,31 @@ static void drm_parse_cea_ext(struct drm_connector *connector, const struct edid *edid) { struct drm_display_info *info = &connector->display_info; + struct drm_edid_iter edid_iter; const struct cea_db *db; struct cea_db_iter iter; - const u8 *edid_ext; + const u8 *cea;
- edid_ext = drm_find_cea_extension(edid); - if (!edid_ext) - return; + drm_edid_iter_begin(edid, &edid_iter); + drm_edid_iter_for_each(cea, &edid_iter) { + if (cea[0] != CEA_EXT) + continue;
- info->cea_rev = edid_ext[1]; + if (!info->cea_rev) + info->cea_rev = cea[1];
- /* The existence of a CEA block should imply RGB support */ - info->color_formats = DRM_COLOR_FORMAT_RGB444; - if (edid_ext[3] & EDID_CEA_YCRCB444) - info->color_formats |= DRM_COLOR_FORMAT_YCBCR444; - if (edid_ext[3] & EDID_CEA_YCRCB422) - info->color_formats |= DRM_COLOR_FORMAT_YCBCR422; + if (info->cea_rev != cea[1]) + DRM_DEBUG_KMS("CEA extension version mismatch %u != %u\n", + info->cea_rev, cea[1]); + + /* The existence of a CEA block should imply RGB support */ + info->color_formats = DRM_COLOR_FORMAT_RGB444; + if (cea[3] & EDID_CEA_YCRCB444) + info->color_formats |= DRM_COLOR_FORMAT_YCBCR444; + if (cea[3] & EDID_CEA_YCRCB422) + info->color_formats |= DRM_COLOR_FORMAT_YCBCR422; + } + drm_edid_iter_end(&edid_iter);
cea_db_iter_edid_begin(edid, &iter); cea_db_iter_for_each(db, &iter) {
The DisplayID CTA data block version does not necessarily match the CEA revision. Simplify by postponing drm_edid_to_eld() slightly, and reusing the CEA revision extracted by drm_parse_cea_ext().
By not bailing out early in drm_edid_to_eld() we may end up filling meaningless data to the ELD. However, the main decision for audio is not the ELD, but rather drm_detect_monitor_audio() called by drivers.
(Arguably a future cleanup could do that in drm_add_edid_modes() and cache the result in the connector.)
Signed-off-by: Jani Nikula jani.nikula@intel.com --- drivers/gpu/drm/drm_edid.c | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index f40427dc5236..dfaa21f00941 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -4710,10 +4710,10 @@ static void clear_eld(struct drm_connector *connector) */ static void drm_edid_to_eld(struct drm_connector *connector, struct edid *edid) { + struct drm_display_info *info = &connector->display_info; const struct cea_db *db; struct cea_db_iter iter; uint8_t *eld = connector->eld; - const u8 *cea; int total_sad_count = 0; int mnl;
@@ -4722,16 +4722,10 @@ static void drm_edid_to_eld(struct drm_connector *connector, struct edid *edid) if (!edid) return;
- cea = drm_find_cea_extension(edid); - if (!cea) { - DRM_DEBUG_KMS("ELD: no CEA Extension found\n"); - return; - } - mnl = get_monitor_name(edid, &eld[DRM_ELD_MONITOR_NAME_STRING]); DRM_DEBUG_KMS("ELD monitor %s\n", &eld[DRM_ELD_MONITOR_NAME_STRING]);
- eld[DRM_ELD_CEA_EDID_VER_MNL] = cea[1] << DRM_ELD_CEA_EDID_VER_SHIFT; + eld[DRM_ELD_CEA_EDID_VER_MNL] = info->cea_rev << DRM_ELD_CEA_EDID_VER_SHIFT; eld[DRM_ELD_CEA_EDID_VER_MNL] |= mnl;
eld[DRM_ELD_VER] = DRM_ELD_VER_CEA861D; @@ -5681,8 +5675,6 @@ int drm_add_edid_modes(struct drm_connector *connector, struct edid *edid) return 0; }
- drm_edid_to_eld(connector, edid); - /* * CEA-861-F adds ycbcr capability map block, for HDMI 2.0 sinks. * To avoid multiple parsing of same block, lets parse that map @@ -5690,6 +5682,9 @@ int drm_add_edid_modes(struct drm_connector *connector, struct edid *edid) */ quirks = drm_add_display_info(connector, edid);
+ /* Depends on info->cea_rev set by drm_add_display_info() above */ + drm_edid_to_eld(connector, edid); + /* * EDID spec says modes should be preferred in this order: * - preferred detailed mode
Convert drm_find_cea_extension() to a predicate function to check if the EDID has a CEA extension or a DisplayID CTA data block. This is mainly to avoid adding new users that only find the first CEA extension.
Signed-off-by: Jani Nikula jani.nikula@intel.com --- drivers/gpu/drm/drm_edid.c | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index dfaa21f00941..84314b65b75b 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -3422,30 +3422,29 @@ const u8 *drm_find_edid_extension(const struct edid *edid, return edid_ext; }
-static const u8 *drm_find_cea_extension(const struct edid *edid) +/* Return true if the EDID has a CEA extension or a DisplayID CTA data block */ +static bool drm_edid_has_cea_extension(const struct edid *edid) { const struct displayid_block *block; struct displayid_iter iter; - const u8 *cea; int ext_index = 0; + bool found = false;
/* Look for a top level CEA extension block */ - /* FIXME: make callers iterate through multiple CEA ext blocks? */ - cea = drm_find_edid_extension(edid, CEA_EXT, &ext_index); - if (cea) - return cea; + if (drm_find_edid_extension(edid, CEA_EXT, &ext_index)) + return true;
/* CEA blocks can also be found embedded in a DisplayID block */ displayid_iter_edid_begin(edid, &iter); displayid_iter_for_each(block, &iter) { if (block->tag == DATA_BLOCK_CTA) { - cea = (const u8 *)block; + found = true; break; } } displayid_iter_end(&iter);
- return cea; + return found; }
static __always_inline const struct drm_display_mode *cea_mode_for_vic(u8 vic) @@ -3715,7 +3714,7 @@ add_alternate_cea_modes(struct drm_connector *connector, struct edid *edid) int modes = 0;
/* Don't add CEA modes if the CEA extension block is missing */ - if (!drm_find_cea_extension(edid)) + if (!drm_edid_has_cea_extension(edid)) return 0;
/*
On Tue, Mar 22, 2022 at 11:40:48PM +0200, Jani Nikula wrote:
Convert drm_find_cea_extension() to a predicate function to check if the EDID has a CEA extension or a DisplayID CTA data block. This is mainly to avoid adding new users that only find the first CEA extension.
Signed-off-by: Jani Nikula jani.nikula@intel.com
drivers/gpu/drm/drm_edid.c | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index dfaa21f00941..84314b65b75b 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -3422,30 +3422,29 @@ const u8 *drm_find_edid_extension(const struct edid *edid, return edid_ext; }
-static const u8 *drm_find_cea_extension(const struct edid *edid) +/* Return true if the EDID has a CEA extension or a DisplayID CTA data block */ +static bool drm_edid_has_cea_extension(const struct edid *edid) { const struct displayid_block *block; struct displayid_iter iter;
- const u8 *cea; int ext_index = 0;
bool found = false;
/* Look for a top level CEA extension block */
- /* FIXME: make callers iterate through multiple CEA ext blocks? */
- cea = drm_find_edid_extension(edid, CEA_EXT, &ext_index);
- if (cea)
return cea;
if (drm_find_edid_extension(edid, CEA_EXT, &ext_index))
return true;/* CEA blocks can also be found embedded in a DisplayID block */ displayid_iter_edid_begin(edid, &iter); displayid_iter_for_each(block, &iter) { if (block->tag == DATA_BLOCK_CTA) {
cea = (const u8 *)block;
found = true; break;
- return cea;
- return found;
}
static __always_inline const struct drm_display_mode *cea_mode_for_vic(u8 vic) @@ -3715,7 +3714,7 @@ add_alternate_cea_modes(struct drm_connector *connector, struct edid *edid) int modes = 0;
/* Don't add CEA modes if the CEA extension block is missing */
- if (!drm_find_cea_extension(edid))
- if (!drm_edid_has_cea_extension(edid))
I'm thinking we could just do if (modes) modes += add_alternate_cea_modes(...); at the end of add_cea_modes().
Or perhaps if (found) modes += add_alternate_cea_modes(...); if we think that adding the alternate modes would be apporpriate even when add_cea_modes() didn't add anything. Not sure.
Yes, that would still introduce a sligth change in behaviour in case we have a CEA ext block/DisplayID CTA block without any of the video/hdmi/y420vdb blocks, but that edge case doesn't feel like a deal-breaker to me.
On Tue, Mar 22, 2022 at 11:40:29PM +0200, Jani Nikula wrote:
Add iterators for EDID blocks and CEA data blocks, to iterate through all data blocks across all CEA extensions and CTA blocks in DisplayID data blocks. Fix all code assuming only one CEA extension. Fix code assuming CTA blocks contain everything that CEA extensions do. Sprinkle a bunch of cleanups on top.
This is completely UNTESTED, didn't even smoke test it. It builds. ;)
Despite that it's now Reviewed-by: Ville Syrjälä ville.syrjala@linux.intel.com
Left a few minor comments here and there.
This superseeds parts of [1] and [2].
BR, Jani.
[1] https://patchwork.freedesktop.org/series/101241/ [2] https://patchwork.freedesktop.org/patch/msgid/20220321044330.27723-1-cooper....
Cc: Shawn C Lee shawn.c.lee@intel.com Cc: Cooper Chiou cooper.chiou@intel.com Cc: william.tseng@intel.com Cc: ankit.k.nautiyal@intel.com Cc: ville.syrjala@linux.intel.com Cc: Drew Davenport ddavenport@chromium.org
Jani Nikula (19): drm/edid: add drm_edid_extension_block_count() and drm_edid_size() drm: use drm_edid_extension_block_count() and drm_edid_size() drm/edid: clean up CEA data block tag definitions drm/edid: add iterator for EDID base and extension blocks drm/edid: add iterator for CEA data blocks drm/edid: clean up cea_db_is_*() functions drm/edid: convert add_cea_modes() to use cea db iter drm/edid: convert drm_edid_to_speaker_allocation() to use cea db iter drm/edid: convert drm_edid_to_sad() to use cea db iter drm/edid: convert drm_detect_hdmi_monitor() to use cea db iter drm/edid: convert drm_detect_monitor_audio() to use cea db iter drm/edid: convert drm_parse_cea_ext() to use cea db iter drm/edid: convert drm_edid_to_eld() to use cea db iter drm/edid: sunset the old unused cea data block iterators drm/edid: restore some type safety to cea_db_*() functions drm/edid: detect basic audio only on CEA extension drm/edid: detect color formats and CEA revision only on CEA extension drm/edid: skip CEA extension scan in drm_edid_to_eld() just for CEA rev drm/edid: sunset drm_find_cea_extension()
drivers/gpu/drm/drm_connector.c | 2 +- drivers/gpu/drm/drm_debugfs.c | 3 +- drivers/gpu/drm/drm_edid.c | 781 ++++++++++++++++++-------------- include/drm/drm_edid.h | 2 + 4 files changed, 455 insertions(+), 333 deletions(-)
-- 2.30.2
dri-devel@lists.freedesktop.org
-
Jani Nikula -
Ville Syrjälä