* [PATCH 0/3] enhanced edid driver compatibility
@ 2022-02-24 14:16 Lee Shawn C
2022-02-24 14:16 ` [PATCH 1/3] drm/edid: parse multiple CEA extension block Lee Shawn C
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Lee Shawn C @ 2022-02-24 14:16 UTC (permalink / raw)
To: dri-devel; +Cc: Lee Shawn C
Support to parse multiple CEA extension blocks and HF-EEODB to
extend drm edid driver's capability.
Lee Shawn C (3):
drm/edid: parse multiple CEA extension block
drm/edid: read HF-EEODB ext block
drm/edid: parse HF-EEODB CEA extension block
drivers/gpu/drm/drm_connector.c | 5 +-
drivers/gpu/drm/drm_displayid.c | 5 +-
drivers/gpu/drm/drm_edid.c | 200 ++++++++++++++++++++++----------
include/drm/drm_edid.h | 4 +-
4 files changed, 151 insertions(+), 63 deletions(-)
--
2.17.1
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 1/3] drm/edid: parse multiple CEA extension block
2022-02-24 14:16 [PATCH 0/3] enhanced edid driver compatibility Lee Shawn C
@ 2022-02-24 14:16 ` Lee Shawn C
2022-02-25 17:47 ` Ville Syrjälä
2022-02-24 14:16 ` [PATCH 2/3] drm/edid: read HF-EEODB ext block Lee Shawn C
2022-02-24 14:16 ` [PATCH 3/3] drm/edid: parse HF-EEODB CEA extension block Lee Shawn C
2 siblings, 1 reply; 8+ messages in thread
From: Lee Shawn C @ 2022-02-24 14:16 UTC (permalink / raw)
To: dri-devel; +Cc: Ankit Nautiyal, Lee Shawn C
Try to find and parse more CEA ext blocks if edid->extensions
is greater than one.
Cc: Jani Nikula <jani.nikula@linux.intel.com>
Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
Cc: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
Signed-off-by: Lee Shawn C <shawn.c.lee@intel.com>
---
drivers/gpu/drm/drm_edid.c | 110 ++++++++++++++++++++-----------------
1 file changed, 60 insertions(+), 50 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index a504542238ed..19426f28b411 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -3353,16 +3353,14 @@ const u8 *drm_find_edid_extension(const struct edid *edid,
return edid_ext;
}
-static const u8 *drm_find_cea_extension(const struct edid *edid)
+static const u8 *drm_find_cea_extension(const struct edid *edid, int *ext_index)
{
const struct displayid_block *block;
struct displayid_iter iter;
const u8 *cea;
- int ext_index = 0;
- /* 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);
+ /* Look for a CEA extension block from ext_index */
+ cea = drm_find_edid_extension(edid, CEA_EXT, ext_index);
if (cea)
return cea;
@@ -3643,10 +3641,10 @@ add_alternate_cea_modes(struct drm_connector *connector, struct edid *edid)
struct drm_device *dev = connector->dev;
struct drm_display_mode *mode, *tmp;
LIST_HEAD(list);
- int modes = 0;
+ int modes = 0, ext_index = 0;
/* Don't add CEA modes if the CEA extension block is missing */
- if (!drm_find_cea_extension(edid))
+ if (!drm_find_cea_extension(edid, &ext_index))
return 0;
/*
@@ -4321,46 +4319,58 @@ 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 *cea = NULL;
const u8 *db, *hdmi = NULL, *video = NULL;
u8 dbl, hdmi_len, video_len = 0;
- int modes = 0;
+ int modes = 0, ext_index = 0;
- if (cea && cea_revision(cea) >= 3) {
- int i, start, end;
+ for (;;) {
+ cea = drm_find_cea_extension(edid, &ext_index);
- if (cea_db_offsets(cea, &start, &end))
- return 0;
+ if (!cea)
+ break;
- for_each_cea_db(cea, i, start, end) {
- db = &cea[i];
- dbl = cea_db_payload_len(db);
+ if (cea && cea_revision(cea) >= 3) {
+ int i, start, end;
+
+ if (cea_db_offsets(cea, &start, &end))
+ continue;
- if (cea_db_tag(db) == VIDEO_BLOCK) {
- 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);
+ for_each_cea_db(cea, i, start, end) {
+ db = &cea[i];
+ dbl = cea_db_payload_len(db);
+
+ if (cea_db_tag(db) == VIDEO_BLOCK) {
+ 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);
+ }
}
}
- }
- /*
- * 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);
+ hdmi = NULL;
+ video = NULL;
+ hdmi_len = 0;
+ video_len = 0;
+ }
+ }
return modes;
}
@@ -4562,7 +4572,7 @@ static void drm_edid_to_eld(struct drm_connector *connector, struct edid *edid)
uint8_t *eld = connector->eld;
const u8 *cea;
const u8 *db;
- int total_sad_count = 0;
+ int total_sad_count = 0, ext_index = 0;
int mnl;
int dbl;
@@ -4571,7 +4581,7 @@ static void drm_edid_to_eld(struct drm_connector *connector, struct edid *edid)
if (!edid)
return;
- cea = drm_find_cea_extension(edid);
+ cea = drm_find_cea_extension(edid, &ext_index);
if (!cea) {
DRM_DEBUG_KMS("ELD: no CEA Extension found\n");
return;
@@ -4655,11 +4665,11 @@ 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)
{
- int count = 0;
+ int count = 0, ext_index = 0;
int i, start, end, dbl;
const u8 *cea;
- cea = drm_find_cea_extension(edid);
+ cea = drm_find_cea_extension(edid, &ext_index);
if (!cea) {
DRM_DEBUG_KMS("SAD: no CEA Extension found\n");
return 0;
@@ -4717,11 +4727,11 @@ EXPORT_SYMBOL(drm_edid_to_sad);
*/
int drm_edid_to_speaker_allocation(struct edid *edid, u8 **sadb)
{
- int count = 0;
+ int count = 0, ext_index = 0;
int i, start, end, dbl;
const u8 *cea;
- cea = drm_find_cea_extension(edid);
+ cea = drm_find_cea_extension(edid, &ext_index);
if (!cea) {
DRM_DEBUG_KMS("SAD: no CEA Extension found\n");
return 0;
@@ -4813,10 +4823,10 @@ EXPORT_SYMBOL(drm_av_sync_delay);
bool drm_detect_hdmi_monitor(struct edid *edid)
{
const u8 *edid_ext;
- int i;
+ int i, ext_index = 0;
int start_offset, end_offset;
- edid_ext = drm_find_cea_extension(edid);
+ edid_ext = drm_find_cea_extension(edid, &ext_index);
if (!edid_ext)
return false;
@@ -4851,11 +4861,11 @@ EXPORT_SYMBOL(drm_detect_hdmi_monitor);
bool drm_detect_monitor_audio(struct edid *edid)
{
const u8 *edid_ext;
- int i, j;
+ int i, j, ext_index = 0;
bool has_audio = false;
int start_offset, end_offset;
- edid_ext = drm_find_cea_extension(edid);
+ edid_ext = drm_find_cea_extension(edid, &ext_index);
if (!edid_ext)
goto end;
@@ -5177,9 +5187,9 @@ static void drm_parse_cea_ext(struct drm_connector *connector,
{
struct drm_display_info *info = &connector->display_info;
const u8 *edid_ext;
- int i, start, end;
+ int i, start, end, ext_index = 0;
- edid_ext = drm_find_cea_extension(edid);
+ edid_ext = drm_find_cea_extension(edid, &ext_index);
if (!edid_ext)
return;
--
2.17.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 2/3] drm/edid: read HF-EEODB ext block
2022-02-24 14:16 [PATCH 0/3] enhanced edid driver compatibility Lee Shawn C
2022-02-24 14:16 ` [PATCH 1/3] drm/edid: parse multiple CEA extension block Lee Shawn C
@ 2022-02-24 14:16 ` Lee Shawn C
2022-02-25 18:09 ` Ville Syrjälä
2022-02-24 14:16 ` [PATCH 3/3] drm/edid: parse HF-EEODB CEA extension block Lee Shawn C
2 siblings, 1 reply; 8+ messages in thread
From: Lee Shawn C @ 2022-02-24 14:16 UTC (permalink / raw)
To: dri-devel; +Cc: Ankit Nautiyal, Lee Shawn C
Support to read HF_EEODB block that request by HDMI 2.1 specification.
Cc: Jani Nikula <jani.nikula@linux.intel.com>
Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
Cc: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
Signed-off-by: Lee Shawn C <shawn.c.lee@intel.com>
---
drivers/gpu/drm/drm_connector.c | 5 ++-
drivers/gpu/drm/drm_edid.c | 76 ++++++++++++++++++++++++++++++---
include/drm/drm_edid.h | 2 +
3 files changed, 77 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index a50c82bc2b2f..0f9e3ef00be7 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -2137,8 +2137,11 @@ int drm_connector_update_edid_property(struct drm_connector *connector,
if (connector->override_edid)
return 0;
- if (edid)
+ if (edid) {
size = EDID_LENGTH * (1 + edid->extensions);
+ if (drm_edid_is_hf_eeodb_blk_available(edid))
+ size = EDID_LENGTH * (1 + drm_edid_read_hf_eeodb_blk_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_edid.c b/drivers/gpu/drm/drm_edid.c
index 19426f28b411..056e735ef932 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -1991,7 +1991,7 @@ struct edid *drm_do_get_edid(struct drm_connector *connector,
void *data)
{
int i, j = 0, valid_extensions = 0;
- u8 *edid, *new;
+ u8 *edid, *new, ext_eeodb_blk_size;
struct edid *override;
override = drm_get_override_edid(connector);
@@ -2051,7 +2051,40 @@ struct edid *drm_do_get_edid(struct drm_connector *connector,
}
kfree(edid);
+ return (struct edid *)new;
+ }
+
+ if (drm_edid_is_hf_eeodb_blk_available((struct edid *)edid)) {
+ ext_eeodb_blk_size = drm_edid_read_hf_eeodb_blk_size((struct edid *)edid);
+
+ // no more ext blk wait for read
+ if (ext_eeodb_blk_size <= 1)
+ return (struct edid *)edid;
+
+ new = krealloc(edid, (ext_eeodb_blk_size + 1) * EDID_LENGTH, GFP_KERNEL);
+ if (!new)
+ goto out;
edid = new;
+
+ valid_extensions = ext_eeodb_blk_size - 1;
+ for (j = 2; j <= ext_eeodb_blk_size; j++) {
+ u8 *block = edid + j * EDID_LENGTH;
+
+ for (i = 0; i < 4; i++) {
+ if (get_edid_block(data, block, j, EDID_LENGTH))
+ goto out;
+ if (drm_edid_block_valid(block, j, false, NULL))
+ break;
+ }
+
+ if (i == 4)
+ valid_extensions--;
+ }
+
+ if (valid_extensions != ext_eeodb_blk_size - 1) {
+ DRM_ERROR("Not able to retrieve proper EDID contain HF-EEODB data.\n");
+ goto out;
+ }
}
return (struct edid *)edid;
@@ -3315,15 +3348,17 @@ add_detailed_modes(struct drm_connector *connector, struct edid *edid,
#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_CAPABILITY_BLOCK 0x00
+#define HDR_STATIC_METADATA_BLOCK 0x06
+#define USE_EXTENDED_TAG 0x07
#define EXT_VIDEO_DATA_BLOCK_420 0x0E
-#define EXT_VIDEO_CAP_BLOCK_Y420CMDB 0x0F
+#define EXT_VIDEO_CAP_BLOCK_Y420CMDB 0x0F
+#define EXT_VIDEO_HF_EEODB_DATA_BLOCK 0x78
#define EDID_BASIC_AUDIO (1 << 6)
#define EDID_CEA_YCRCB444 (1 << 5)
#define EDID_CEA_YCRCB422 (1 << 4)
#define EDID_CEA_VCDB_QS (1 << 6)
+#define HF_EEODB_LENGTH 2
/*
* Search EDID for CEA extension block.
@@ -4273,6 +4308,37 @@ static bool cea_db_is_y420vdb(const u8 *db)
return true;
}
+static bool cea_db_is_hdmi_forum_eeodb(const u8 *db)
+{
+ if (cea_db_tag(db) != USE_EXTENDED_TAG)
+ return false;
+
+ if (cea_db_payload_len(db) != HF_EEODB_LENGTH)
+ return false;
+
+ if (cea_db_extended_tag(db) != EXT_VIDEO_HF_EEODB_DATA_BLOCK)
+ return false;
+
+ return true;
+}
+
+bool drm_edid_is_hf_eeodb_blk_available(const struct edid *edid)
+{
+ const u8 *eeodb_header = (u8 *)edid + EDID_LENGTH + 4;
+
+ if (!edid->extensions)
+ return false;
+
+ return cea_db_is_hdmi_forum_eeodb(eeodb_header);
+}
+EXPORT_SYMBOL_GPL(drm_edid_is_hf_eeodb_blk_available);
+
+u8 drm_edid_read_hf_eeodb_blk_size(const struct edid *edid)
+{
+ return ((u8 *)edid)[EDID_LENGTH + 6];
+}
+EXPORT_SYMBOL_GPL(drm_edid_read_hf_eeodb_blk_size);
+
#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)
diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
index 144c495b99c4..22872d9731a8 100644
--- a/include/drm/drm_edid.h
+++ b/include/drm/drm_edid.h
@@ -593,5 +593,7 @@ drm_display_mode_from_cea_vic(struct drm_device *dev,
const u8 *drm_find_edid_extension(const struct edid *edid,
int ext_id, int *ext_index);
+bool drm_edid_is_hf_eeodb_blk_available(const struct edid *edid);
+u8 drm_edid_read_hf_eeodb_blk_size(const struct edid *edid);
#endif /* __DRM_EDID_H__ */
--
2.17.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 3/3] drm/edid: parse HF-EEODB CEA extension block
2022-02-24 14:16 [PATCH 0/3] enhanced edid driver compatibility Lee Shawn C
2022-02-24 14:16 ` [PATCH 1/3] drm/edid: parse multiple CEA extension block Lee Shawn C
2022-02-24 14:16 ` [PATCH 2/3] drm/edid: read HF-EEODB ext block Lee Shawn C
@ 2022-02-24 14:16 ` Lee Shawn C
2 siblings, 0 replies; 8+ messages in thread
From: Lee Shawn C @ 2022-02-24 14:16 UTC (permalink / raw)
To: dri-devel; +Cc: Ankit Nautiyal, Lee Shawn C
While adding CEA modes, try to get available EEODB block
number. Then based on it to parse numbers of ext blocks,
retrieve CEA information and add more CEA modes.
Cc: Jani Nikula <jani.nikula@linux.intel.com>
Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
Cc: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
Signed-off-by: Lee Shawn C <shawn.c.lee@intel.com>
---
drivers/gpu/drm/drm_displayid.c | 5 ++++-
drivers/gpu/drm/drm_edid.c | 34 ++++++++++++++++++---------------
include/drm/drm_edid.h | 2 +-
3 files changed, 24 insertions(+), 17 deletions(-)
diff --git a/drivers/gpu/drm/drm_displayid.c b/drivers/gpu/drm/drm_displayid.c
index 32da557b960f..dc649a9efaa2 100644
--- a/drivers/gpu/drm/drm_displayid.c
+++ b/drivers/gpu/drm/drm_displayid.c
@@ -37,7 +37,10 @@ static const u8 *drm_find_displayid_extension(const struct edid *edid,
int *length, int *idx,
int *ext_index)
{
- const u8 *displayid = drm_find_edid_extension(edid, DISPLAYID_EXT, ext_index);
+ const u8 *displayid = drm_find_edid_extension(edid,
+ DISPLAYID_EXT,
+ ext_index,
+ edid->extensions);
const struct displayid_header *base;
int ret;
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 056e735ef932..5e43d5ac0a5e 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -3364,23 +3364,23 @@ add_detailed_modes(struct drm_connector *connector, struct edid *edid,
* Search EDID for CEA extension block.
*/
const u8 *drm_find_edid_extension(const struct edid *edid,
- int ext_id, int *ext_index)
+ int ext_id, int *ext_index, int num_ext_blk)
{
const u8 *edid_ext = NULL;
int i;
/* No EDID or EDID extensions */
- if (edid == NULL || edid->extensions == 0)
+ if (edid == NULL || edid->extensions == 0 || *ext_index >= num_ext_blk)
return NULL;
/* Find CEA extension */
- for (i = *ext_index; i < edid->extensions; i++) {
+ for (i = *ext_index; i < num_ext_blk; i++) {
edid_ext = (const u8 *)edid + EDID_LENGTH * (i + 1);
if (edid_ext[0] == ext_id)
break;
}
- if (i >= edid->extensions)
+ if (i >= num_ext_blk)
return NULL;
*ext_index = i + 1;
@@ -3388,14 +3388,15 @@ const u8 *drm_find_edid_extension(const struct edid *edid,
return edid_ext;
}
-static const u8 *drm_find_cea_extension(const struct edid *edid, int *ext_index)
+static const u8 *drm_find_cea_extension(const struct edid *edid,
+ int *ext_index, int num_ext_blk)
{
const struct displayid_block *block;
struct displayid_iter iter;
const u8 *cea;
/* Look for a CEA extension block from ext_index */
- cea = drm_find_edid_extension(edid, CEA_EXT, ext_index);
+ cea = drm_find_edid_extension(edid, CEA_EXT, ext_index, num_ext_blk);
if (cea)
return cea;
@@ -3679,7 +3680,7 @@ add_alternate_cea_modes(struct drm_connector *connector, struct edid *edid)
int modes = 0, ext_index = 0;
/* Don't add CEA modes if the CEA extension block is missing */
- if (!drm_find_cea_extension(edid, &ext_index))
+ if (!drm_find_cea_extension(edid, &ext_index, edid->extensions))
return 0;
/*
@@ -4387,11 +4388,14 @@ add_cea_modes(struct drm_connector *connector, struct edid *edid)
{
const u8 *cea = NULL;
const u8 *db, *hdmi = NULL, *video = NULL;
- u8 dbl, hdmi_len, video_len = 0;
+ u8 dbl, hdmi_len, video_len = 0, num_ext_blk = edid->extensions;
int modes = 0, ext_index = 0;
+ if (num_ext_blk && drm_edid_is_hf_eeodb_blk_available(edid))
+ num_ext_blk = drm_edid_read_hf_eeodb_blk_size(edid);
+
for (;;) {
- cea = drm_find_cea_extension(edid, &ext_index);
+ cea = drm_find_cea_extension(edid, &ext_index, num_ext_blk);
if (!cea)
break;
@@ -4647,7 +4651,7 @@ static void drm_edid_to_eld(struct drm_connector *connector, struct edid *edid)
if (!edid)
return;
- cea = drm_find_cea_extension(edid, &ext_index);
+ cea = drm_find_cea_extension(edid, &ext_index, edid->extensions);
if (!cea) {
DRM_DEBUG_KMS("ELD: no CEA Extension found\n");
return;
@@ -4735,7 +4739,7 @@ int drm_edid_to_sad(struct edid *edid, struct cea_sad **sads)
int i, start, end, dbl;
const u8 *cea;
- cea = drm_find_cea_extension(edid, &ext_index);
+ cea = drm_find_cea_extension(edid, &ext_index, edid->extensions);
if (!cea) {
DRM_DEBUG_KMS("SAD: no CEA Extension found\n");
return 0;
@@ -4797,7 +4801,7 @@ int drm_edid_to_speaker_allocation(struct edid *edid, u8 **sadb)
int i, start, end, dbl;
const u8 *cea;
- cea = drm_find_cea_extension(edid, &ext_index);
+ cea = drm_find_cea_extension(edid, &ext_index, edid->extensions);
if (!cea) {
DRM_DEBUG_KMS("SAD: no CEA Extension found\n");
return 0;
@@ -4892,7 +4896,7 @@ bool drm_detect_hdmi_monitor(struct edid *edid)
int i, ext_index = 0;
int start_offset, end_offset;
- edid_ext = drm_find_cea_extension(edid, &ext_index);
+ edid_ext = drm_find_cea_extension(edid, &ext_index, edid->extensions);
if (!edid_ext)
return false;
@@ -4931,7 +4935,7 @@ bool drm_detect_monitor_audio(struct edid *edid)
bool has_audio = false;
int start_offset, end_offset;
- edid_ext = drm_find_cea_extension(edid, &ext_index);
+ edid_ext = drm_find_cea_extension(edid, &ext_index, edid->extensions);
if (!edid_ext)
goto end;
@@ -5255,7 +5259,7 @@ static void drm_parse_cea_ext(struct drm_connector *connector,
const u8 *edid_ext;
int i, start, end, ext_index = 0;
- edid_ext = drm_find_cea_extension(edid, &ext_index);
+ edid_ext = drm_find_cea_extension(edid, &ext_index, edid->extensions);
if (!edid_ext)
return;
diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
index 22872d9731a8..749969ca040a 100644
--- a/include/drm/drm_edid.h
+++ b/include/drm/drm_edid.h
@@ -591,7 +591,7 @@ struct drm_display_mode *
drm_display_mode_from_cea_vic(struct drm_device *dev,
u8 video_code);
const u8 *drm_find_edid_extension(const struct edid *edid,
- int ext_id, int *ext_index);
+ int ext_id, int *ext_index, int num_ext_blk);
bool drm_edid_is_hf_eeodb_blk_available(const struct edid *edid);
u8 drm_edid_read_hf_eeodb_blk_size(const struct edid *edid);
--
2.17.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 1/3] drm/edid: parse multiple CEA extension block
2022-02-24 14:16 ` [PATCH 1/3] drm/edid: parse multiple CEA extension block Lee Shawn C
@ 2022-02-25 17:47 ` Ville Syrjälä
2022-02-27 14:49 ` Lee, Shawn C
0 siblings, 1 reply; 8+ messages in thread
From: Ville Syrjälä @ 2022-02-25 17:47 UTC (permalink / raw)
To: Lee Shawn C; +Cc: Ankit Nautiyal, dri-devel
On Thu, Feb 24, 2022 at 10:16:23PM +0800, Lee Shawn C wrote:
> Try to find and parse more CEA ext blocks if edid->extensions
> is greater than one.
>
> Cc: Jani Nikula <jani.nikula@linux.intel.com>
> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
> Cc: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> Signed-off-by: Lee Shawn C <shawn.c.lee@intel.com>
> ---
> drivers/gpu/drm/drm_edid.c | 110 ++++++++++++++++++++-----------------
> 1 file changed, 60 insertions(+), 50 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index a504542238ed..19426f28b411 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -3353,16 +3353,14 @@ const u8 *drm_find_edid_extension(const struct edid *edid,
> return edid_ext;
> }
>
> -static const u8 *drm_find_cea_extension(const struct edid *edid)
> +static const u8 *drm_find_cea_extension(const struct edid *edid, int *ext_index)
> {
> const struct displayid_block *block;
> struct displayid_iter iter;
> const u8 *cea;
> - int ext_index = 0;
>
> - /* 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);
> + /* Look for a CEA extension block from ext_index */
> + cea = drm_find_edid_extension(edid, CEA_EXT, ext_index);
> if (cea)
> return cea;
>
> @@ -3643,10 +3641,10 @@ add_alternate_cea_modes(struct drm_connector *connector, struct edid *edid)
> struct drm_device *dev = connector->dev;
> struct drm_display_mode *mode, *tmp;
> LIST_HEAD(list);
> - int modes = 0;
> + int modes = 0, ext_index = 0;
>
> /* Don't add CEA modes if the CEA extension block is missing */
> - if (!drm_find_cea_extension(edid))
> + if (!drm_find_cea_extension(edid, &ext_index))
> return 0;
>
> /*
> @@ -4321,46 +4319,58 @@ 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 *cea = NULL;
> const u8 *db, *hdmi = NULL, *video = NULL;
> u8 dbl, hdmi_len, video_len = 0;
> - int modes = 0;
> + int modes = 0, ext_index = 0;
>
> - if (cea && cea_revision(cea) >= 3) {
> - int i, start, end;
> + for (;;) {
> + cea = drm_find_cea_extension(edid, &ext_index);
Please split this into two patches:
1. do the *ext_index stuff
2. do the loop
>
> - if (cea_db_offsets(cea, &start, &end))
> - return 0;
> + if (!cea)
> + break;
>
> - for_each_cea_db(cea, i, start, end) {
> - db = &cea[i];
> - dbl = cea_db_payload_len(db);
> + if (cea && cea_revision(cea) >= 3) {
> + int i, start, end;
> +
> + if (cea_db_offsets(cea, &start, &end))
> + continue;
>
> - if (cea_db_tag(db) == VIDEO_BLOCK) {
> - 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);
> + for_each_cea_db(cea, i, start, end) {
> + db = &cea[i];
> + dbl = cea_db_payload_len(db);
> +
> + if (cea_db_tag(db) == VIDEO_BLOCK) {
> + 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);
> + }
> }
> }
> - }
>
> - /*
> - * 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) {
Looks like you're potentially using stale information here from
previous loops. Please move all the possible variables to a tighter
scope so this can't happen.
> + modes += do_hdmi_vsdb_modes(connector, hdmi, hdmi_len, video,
> + video_len);
> + hdmi = NULL;
> + video = NULL;
> + hdmi_len = 0;
> + video_len = 0;
> + }
> + }
>
> return modes;
> }
> @@ -4562,7 +4572,7 @@ static void drm_edid_to_eld(struct drm_connector *connector, struct edid *edid)
> uint8_t *eld = connector->eld;
> const u8 *cea;
> const u8 *db;
> - int total_sad_count = 0;
> + int total_sad_count = 0, ext_index = 0;
> int mnl;
> int dbl;
>
> @@ -4571,7 +4581,7 @@ static void drm_edid_to_eld(struct drm_connector *connector, struct edid *edid)
> if (!edid)
> return;
>
> - cea = drm_find_cea_extension(edid);
> + cea = drm_find_cea_extension(edid, &ext_index);
> if (!cea) {
> DRM_DEBUG_KMS("ELD: no CEA Extension found\n");
> return;
> @@ -4655,11 +4665,11 @@ 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)
> {
> - int count = 0;
> + int count = 0, ext_index = 0;
> int i, start, end, dbl;
> const u8 *cea;
>
> - cea = drm_find_cea_extension(edid);
> + cea = drm_find_cea_extension(edid, &ext_index);
> if (!cea) {
> DRM_DEBUG_KMS("SAD: no CEA Extension found\n");
> return 0;
> @@ -4717,11 +4727,11 @@ EXPORT_SYMBOL(drm_edid_to_sad);
> */
> int drm_edid_to_speaker_allocation(struct edid *edid, u8 **sadb)
> {
> - int count = 0;
> + int count = 0, ext_index = 0;
> int i, start, end, dbl;
> const u8 *cea;
>
> - cea = drm_find_cea_extension(edid);
> + cea = drm_find_cea_extension(edid, &ext_index);
> if (!cea) {
> DRM_DEBUG_KMS("SAD: no CEA Extension found\n");
> return 0;
> @@ -4813,10 +4823,10 @@ EXPORT_SYMBOL(drm_av_sync_delay);
> bool drm_detect_hdmi_monitor(struct edid *edid)
> {
> const u8 *edid_ext;
> - int i;
> + int i, ext_index = 0;
> int start_offset, end_offset;
>
> - edid_ext = drm_find_cea_extension(edid);
> + edid_ext = drm_find_cea_extension(edid, &ext_index);
> if (!edid_ext)
> return false;
>
> @@ -4851,11 +4861,11 @@ EXPORT_SYMBOL(drm_detect_hdmi_monitor);
> bool drm_detect_monitor_audio(struct edid *edid)
> {
> const u8 *edid_ext;
> - int i, j;
> + int i, j, ext_index = 0;
> bool has_audio = false;
> int start_offset, end_offset;
>
> - edid_ext = drm_find_cea_extension(edid);
> + edid_ext = drm_find_cea_extension(edid, &ext_index);
> if (!edid_ext)
> goto end;
>
> @@ -5177,9 +5187,9 @@ static void drm_parse_cea_ext(struct drm_connector *connector,
> {
> struct drm_display_info *info = &connector->display_info;
> const u8 *edid_ext;
> - int i, start, end;
> + int i, start, end, ext_index = 0;
>
> - edid_ext = drm_find_cea_extension(edid);
> + edid_ext = drm_find_cea_extension(edid, &ext_index);
> if (!edid_ext)
> return;
>
> --
> 2.17.1
--
Ville Syrjälä
Intel
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/3] drm/edid: read HF-EEODB ext block
2022-02-24 14:16 ` [PATCH 2/3] drm/edid: read HF-EEODB ext block Lee Shawn C
@ 2022-02-25 18:09 ` Ville Syrjälä
2022-02-27 15:11 ` Lee, Shawn C
0 siblings, 1 reply; 8+ messages in thread
From: Ville Syrjälä @ 2022-02-25 18:09 UTC (permalink / raw)
To: Lee Shawn C; +Cc: Ankit Nautiyal, dri-devel
On Thu, Feb 24, 2022 at 10:16:24PM +0800, Lee Shawn C wrote:
> Support to read HF_EEODB block that request by HDMI 2.1 specification.
Please spell out what that thing is and why it exists.
>
> Cc: Jani Nikula <jani.nikula@linux.intel.com>
> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
> Cc: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> Signed-off-by: Lee Shawn C <shawn.c.lee@intel.com>
> ---
> drivers/gpu/drm/drm_connector.c | 5 ++-
> drivers/gpu/drm/drm_edid.c | 76 ++++++++++++++++++++++++++++++---
> include/drm/drm_edid.h | 2 +
> 3 files changed, 77 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index a50c82bc2b2f..0f9e3ef00be7 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -2137,8 +2137,11 @@ int drm_connector_update_edid_property(struct drm_connector *connector,
> if (connector->override_edid)
> return 0;
>
> - if (edid)
> + if (edid) {
> size = EDID_LENGTH * (1 + edid->extensions);
> + if (drm_edid_is_hf_eeodb_blk_available(edid))
> + size = EDID_LENGTH * (1 + drm_edid_read_hf_eeodb_blk_size(edid));
I think we just want a small helper function that gives us the number
of ext blocks whether it be from the base block or from this thing.
> + }
>
> /* 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_edid.c b/drivers/gpu/drm/drm_edid.c
> index 19426f28b411..056e735ef932 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -1991,7 +1991,7 @@ struct edid *drm_do_get_edid(struct drm_connector *connector,
> void *data)
> {
> int i, j = 0, valid_extensions = 0;
> - u8 *edid, *new;
> + u8 *edid, *new, ext_eeodb_blk_size;
> struct edid *override;
>
> override = drm_get_override_edid(connector);
> @@ -2051,7 +2051,40 @@ struct edid *drm_do_get_edid(struct drm_connector *connector,
> }
>
> kfree(edid);
> + return (struct edid *)new;
> + }
> +
> + if (drm_edid_is_hf_eeodb_blk_available((struct edid *)edid)) {
> + ext_eeodb_blk_size = drm_edid_read_hf_eeodb_blk_size((struct edid *)edid);
> +
> + // no more ext blk wait for read
> + if (ext_eeodb_blk_size <= 1)
> + return (struct edid *)edid;
> +
> + new = krealloc(edid, (ext_eeodb_blk_size + 1) * EDID_LENGTH, GFP_KERNEL);
> + if (!new)
> + goto out;
> edid = new;
> +
> + valid_extensions = ext_eeodb_blk_size - 1;
> + for (j = 2; j <= ext_eeodb_blk_size; j++) {
> + u8 *block = edid + j * EDID_LENGTH;
> +
> + for (i = 0; i < 4; i++) {
> + if (get_edid_block(data, block, j, EDID_LENGTH))
> + goto out;
> + if (drm_edid_block_valid(block, j, false, NULL))
> + break;
> + }
> +
> + if (i == 4)
> + valid_extensions--;
> + }
> +
> + if (valid_extensions != ext_eeodb_blk_size - 1) {
> + DRM_ERROR("Not able to retrieve proper EDID contain HF-EEODB data.\n");
> + goto out;
> + }
> }
>
> return (struct edid *)edid;
> @@ -3315,15 +3348,17 @@ add_detailed_modes(struct drm_connector *connector, struct edid *edid,
> #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_CAPABILITY_BLOCK 0x00
> +#define HDR_STATIC_METADATA_BLOCK 0x06
> +#define USE_EXTENDED_TAG 0x07
> #define EXT_VIDEO_DATA_BLOCK_420 0x0E
> -#define EXT_VIDEO_CAP_BLOCK_Y420CMDB 0x0F
> +#define EXT_VIDEO_CAP_BLOCK_Y420CMDB 0x0F
> +#define EXT_VIDEO_HF_EEODB_DATA_BLOCK 0x78
> #define EDID_BASIC_AUDIO (1 << 6)
> #define EDID_CEA_YCRCB444 (1 << 5)
> #define EDID_CEA_YCRCB422 (1 << 4)
> #define EDID_CEA_VCDB_QS (1 << 6)
> +#define HF_EEODB_LENGTH 2
>
> /*
> * Search EDID for CEA extension block.
> @@ -4273,6 +4308,37 @@ static bool cea_db_is_y420vdb(const u8 *db)
> return true;
> }
>
> +static bool cea_db_is_hdmi_forum_eeodb(const u8 *db)
> +{
> + if (cea_db_tag(db) != USE_EXTENDED_TAG)
> + return false;
> +
> + if (cea_db_payload_len(db) != HF_EEODB_LENGTH)
We don't have any defines for the length of other blocks. Don't see why
this one should be different.
Also, the spec says
"The Sources that are compliant to This Specification will not
be adversely affected by the presence of the additional bytes within
the Length field range."
> + return false;
> +
> + if (cea_db_extended_tag(db) != EXT_VIDEO_HF_EEODB_DATA_BLOCK)
> + return false;
> +
> + return true;
> +}
> +
> +bool drm_edid_is_hf_eeodb_blk_available(const struct edid *edid)
> +{
> + const u8 *eeodb_header = (u8 *)edid + EDID_LENGTH + 4;
> +
> + if (!edid->extensions)
> + return false;
> +
> + return cea_db_is_hdmi_forum_eeodb(eeodb_header);
> +}
> +EXPORT_SYMBOL_GPL(drm_edid_is_hf_eeodb_blk_available);
> +
> +u8 drm_edid_read_hf_eeodb_blk_size(const struct edid *edid)
> +{
> + return ((u8 *)edid)[EDID_LENGTH + 6];
Not a big fan of these hardcoded offsets and stuff. Seems too
easy to screw up.
> +}
> +EXPORT_SYMBOL_GPL(drm_edid_read_hf_eeodb_blk_size);
> +
> #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)
>
> diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
> index 144c495b99c4..22872d9731a8 100644
> --- a/include/drm/drm_edid.h
> +++ b/include/drm/drm_edid.h
> @@ -593,5 +593,7 @@ drm_display_mode_from_cea_vic(struct drm_device *dev,
> const u8 *drm_find_edid_extension(const struct edid *edid,
> int ext_id, int *ext_index);
>
> +bool drm_edid_is_hf_eeodb_blk_available(const struct edid *edid);
> +u8 drm_edid_read_hf_eeodb_blk_size(const struct edid *edid);
>
> #endif /* __DRM_EDID_H__ */
> --
> 2.17.1
--
Ville Syrjälä
Intel
^ permalink raw reply [flat|nested] 8+ messages in thread
* RE: [PATCH 1/3] drm/edid: parse multiple CEA extension block
2022-02-25 17:47 ` Ville Syrjälä
@ 2022-02-27 14:49 ` Lee, Shawn C
0 siblings, 0 replies; 8+ messages in thread
From: Lee, Shawn C @ 2022-02-27 14:49 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: Nautiyal, Ankit K, dri-devel
On Saturday, February 26, 2022 1:47 AM, Ville Syrjälä wrote:
>On Thu, Feb 24, 2022 at 10:16:23PM +0800, Lee Shawn C wrote:
>> Try to find and parse more CEA ext blocks if edid->extensions is
>> greater than one.
>>
>> Cc: Jani Nikula <jani.nikula@linux.intel.com>
>> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
>> Cc: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
>> Signed-off-by: Lee Shawn C <shawn.c.lee@intel.com>
>> ---
>> drivers/gpu/drm/drm_edid.c | 110
>> ++++++++++++++++++++-----------------
>> 1 file changed, 60 insertions(+), 50 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
>> index a504542238ed..19426f28b411 100644
>> --- a/drivers/gpu/drm/drm_edid.c
>> +++ b/drivers/gpu/drm/drm_edid.c
>> @@ -3353,16 +3353,14 @@ const u8 *drm_find_edid_extension(const struct edid *edid,
>> return edid_ext;
>> }
>>
>> -static const u8 *drm_find_cea_extension(const struct edid *edid)
>> +static const u8 *drm_find_cea_extension(const struct edid *edid, int
>> +*ext_index)
>> {
>> const struct displayid_block *block;
>> struct displayid_iter iter;
>> const u8 *cea;
>> - int ext_index = 0;
>>
>> - /* 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);
>> + /* Look for a CEA extension block from ext_index */
>> + cea = drm_find_edid_extension(edid, CEA_EXT, ext_index);
>> if (cea)
>> return cea;
>>
>> @@ -3643,10 +3641,10 @@ add_alternate_cea_modes(struct drm_connector *connector, struct edid *edid)
>> struct drm_device *dev = connector->dev;
>> struct drm_display_mode *mode, *tmp;
>> LIST_HEAD(list);
>> - int modes = 0;
>> + int modes = 0, ext_index = 0;
>>
>> /* Don't add CEA modes if the CEA extension block is missing */
>> - if (!drm_find_cea_extension(edid))
>> + if (!drm_find_cea_extension(edid, &ext_index))
>> return 0;
>>
>> /*
>> @@ -4321,46 +4319,58 @@ 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 *cea = NULL;
>> const u8 *db, *hdmi = NULL, *video = NULL;
>> u8 dbl, hdmi_len, video_len = 0;
>> - int modes = 0;
>> + int modes = 0, ext_index = 0;
>>
>> - if (cea && cea_revision(cea) >= 3) {
>> - int i, start, end;
>> + for (;;) {
>> + cea = drm_find_cea_extension(edid, &ext_index);
>
>Please split this into two patches:
>1. do the *ext_index stuff
>2. do the loop
>
Thanks for comment! Will split this patch to two in next version.
>>
>> - if (cea_db_offsets(cea, &start, &end))
>> - return 0;
>> + if (!cea)
>> + break;
>>
>> - for_each_cea_db(cea, i, start, end) {
>> - db = &cea[i];
>> - dbl = cea_db_payload_len(db);
>> + if (cea && cea_revision(cea) >= 3) {
>> + int i, start, end;
>> +
>> + if (cea_db_offsets(cea, &start, &end))
>> + continue;
>>
>> - if (cea_db_tag(db) == VIDEO_BLOCK) {
>> - 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);
>> + for_each_cea_db(cea, i, start, end) {
>> + db = &cea[i];
>> + dbl = cea_db_payload_len(db);
>> +
>> + if (cea_db_tag(db) == VIDEO_BLOCK) {
>> + 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);
>> + }
>> }
>> }
>> - }
>>
>> - /*
>> - * 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) {
>
>Looks like you're potentially using stale information here from previous loops. Please move all the possible variables to a tighter scope so this can't happen.
>
Yes, you are right! I will modify these codes and send patch v2 for review later.
Best regards,
Shawn
>> + modes += do_hdmi_vsdb_modes(connector, hdmi, hdmi_len, video,
>> + video_len);
>> + hdmi = NULL;
>> + video = NULL;
>> + hdmi_len = 0;
>> + video_len = 0;
>> + }
>> + }
>>
>> return modes;
>> }
>> @@ -4562,7 +4572,7 @@ static void drm_edid_to_eld(struct drm_connector *connector, struct edid *edid)
>> uint8_t *eld = connector->eld;
>> const u8 *cea;
>> const u8 *db;
>> - int total_sad_count = 0;
>> + int total_sad_count = 0, ext_index = 0;
>> int mnl;
>> int dbl;
>>
>> @@ -4571,7 +4581,7 @@ static void drm_edid_to_eld(struct drm_connector *connector, struct edid *edid)
>> if (!edid)
>> return;
>>
>> - cea = drm_find_cea_extension(edid);
>> + cea = drm_find_cea_extension(edid, &ext_index);
>> if (!cea) {
>> DRM_DEBUG_KMS("ELD: no CEA Extension found\n");
>> return;
>> @@ -4655,11 +4665,11 @@ 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) {
>> - int count = 0;
>> + int count = 0, ext_index = 0;
>> int i, start, end, dbl;
>> const u8 *cea;
>>
>> - cea = drm_find_cea_extension(edid);
>> + cea = drm_find_cea_extension(edid, &ext_index);
>> if (!cea) {
>> DRM_DEBUG_KMS("SAD: no CEA Extension found\n");
>> return 0;
>> @@ -4717,11 +4727,11 @@ EXPORT_SYMBOL(drm_edid_to_sad);
>> */
>> int drm_edid_to_speaker_allocation(struct edid *edid, u8 **sadb) {
>> - int count = 0;
>> + int count = 0, ext_index = 0;
>> int i, start, end, dbl;
>> const u8 *cea;
>>
>> - cea = drm_find_cea_extension(edid);
>> + cea = drm_find_cea_extension(edid, &ext_index);
>> if (!cea) {
>> DRM_DEBUG_KMS("SAD: no CEA Extension found\n");
>> return 0;
>> @@ -4813,10 +4823,10 @@ EXPORT_SYMBOL(drm_av_sync_delay); bool
>> drm_detect_hdmi_monitor(struct edid *edid) {
>> const u8 *edid_ext;
>> - int i;
>> + int i, ext_index = 0;
>> int start_offset, end_offset;
>>
>> - edid_ext = drm_find_cea_extension(edid);
>> + edid_ext = drm_find_cea_extension(edid, &ext_index);
>> if (!edid_ext)
>> return false;
>>
>> @@ -4851,11 +4861,11 @@ EXPORT_SYMBOL(drm_detect_hdmi_monitor);
>> bool drm_detect_monitor_audio(struct edid *edid) {
>> const u8 *edid_ext;
>> - int i, j;
>> + int i, j, ext_index = 0;
>> bool has_audio = false;
>> int start_offset, end_offset;
>>
>> - edid_ext = drm_find_cea_extension(edid);
>> + edid_ext = drm_find_cea_extension(edid, &ext_index);
>> if (!edid_ext)
>> goto end;
>>
>> @@ -5177,9 +5187,9 @@ static void drm_parse_cea_ext(struct
>> drm_connector *connector, {
>> struct drm_display_info *info = &connector->display_info;
>> const u8 *edid_ext;
>> - int i, start, end;
>> + int i, start, end, ext_index = 0;
>>
>> - edid_ext = drm_find_cea_extension(edid);
>> + edid_ext = drm_find_cea_extension(edid, &ext_index);
>> if (!edid_ext)
>> return;
>>
>> --
>> 2.17.1
>
>--
>Ville Syrjälä
>Intel
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* RE: [PATCH 2/3] drm/edid: read HF-EEODB ext block
2022-02-25 18:09 ` Ville Syrjälä
@ 2022-02-27 15:11 ` Lee, Shawn C
0 siblings, 0 replies; 8+ messages in thread
From: Lee, Shawn C @ 2022-02-27 15:11 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: Nautiyal, Ankit K, dri-devel
On Saturday, February 26, 2022 2:09 AM, Ville Syrjälä wrote:
>On Thu, Feb 24, 2022 at 10:16:24PM +0800, Lee Shawn C wrote:
>> Support to read HF_EEODB block that request by HDMI 2.1 specification.
>
>Please spell out what that thing is and why it exists.
>
>>
>> Cc: Jani Nikula <jani.nikula@linux.intel.com>
>> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
>> Cc: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
>> Signed-off-by: Lee Shawn C <shawn.c.lee@intel.com>
>> ---
>> drivers/gpu/drm/drm_connector.c | 5 ++-
>> drivers/gpu/drm/drm_edid.c | 76 ++++++++++++++++++++++++++++++---
>> include/drm/drm_edid.h | 2 +
>> 3 files changed, 77 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_connector.c
>> b/drivers/gpu/drm/drm_connector.c index a50c82bc2b2f..0f9e3ef00be7
>> 100644
>> --- a/drivers/gpu/drm/drm_connector.c
>> +++ b/drivers/gpu/drm/drm_connector.c
>> @@ -2137,8 +2137,11 @@ int drm_connector_update_edid_property(struct drm_connector *connector,
>> if (connector->override_edid)
>> return 0;
>>
>> - if (edid)
>> + if (edid) {
>> size = EDID_LENGTH * (1 + edid->extensions);
>> + if (drm_edid_is_hf_eeodb_blk_available(edid))
>> + size = EDID_LENGTH * (1 + drm_edid_read_hf_eeodb_blk_size(edid));
>
>I think we just want a small helper function that gives us the number of ext blocks whether it be from the base block or from this thing.
>
>> + }
>>
>> /* 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_edid.c b/drivers/gpu/drm/drm_edid.c index
>> 19426f28b411..056e735ef932 100644
>> --- a/drivers/gpu/drm/drm_edid.c
>> +++ b/drivers/gpu/drm/drm_edid.c
>> @@ -1991,7 +1991,7 @@ struct edid *drm_do_get_edid(struct drm_connector *connector,
>> void *data)
>> {
>> int i, j = 0, valid_extensions = 0;
>> - u8 *edid, *new;
>> + u8 *edid, *new, ext_eeodb_blk_size;
>> struct edid *override;
>>
>> override = drm_get_override_edid(connector); @@ -2051,7 +2051,40 @@
>> struct edid *drm_do_get_edid(struct drm_connector *connector,
>> }
>>
>> kfree(edid);
>> + return (struct edid *)new;
>> + }
>> +
>> + if (drm_edid_is_hf_eeodb_blk_available((struct edid *)edid)) {
>> + ext_eeodb_blk_size = drm_edid_read_hf_eeodb_blk_size((struct edid
>> +*)edid);
>> +
>> + // no more ext blk wait for read
>> + if (ext_eeodb_blk_size <= 1)
>> + return (struct edid *)edid;
>> +
>> + new = krealloc(edid, (ext_eeodb_blk_size + 1) * EDID_LENGTH, GFP_KERNEL);
>> + if (!new)
>> + goto out;
>> edid = new;
>> +
>> + valid_extensions = ext_eeodb_blk_size - 1;
>> + for (j = 2; j <= ext_eeodb_blk_size; j++) {
>> + u8 *block = edid + j * EDID_LENGTH;
>> +
>> + for (i = 0; i < 4; i++) {
>> + if (get_edid_block(data, block, j, EDID_LENGTH))
>> + goto out;
>> + if (drm_edid_block_valid(block, j, false, NULL))
>> + break;
>> + }
>> +
>> + if (i == 4)
>> + valid_extensions--;
>> + }
>> +
>> + if (valid_extensions != ext_eeodb_blk_size - 1) {
>> + DRM_ERROR("Not able to retrieve proper EDID contain HF-EEODB data.\n");
>> + goto out;
>> + }
>> }
>>
>> return (struct edid *)edid;
>> @@ -3315,15 +3348,17 @@ add_detailed_modes(struct drm_connector *connector, struct edid *edid,
>> #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_CAPABILITY_BLOCK 0x00
>> +#define HDR_STATIC_METADATA_BLOCK 0x06
>> +#define USE_EXTENDED_TAG 0x07
>> #define EXT_VIDEO_DATA_BLOCK_420 0x0E
>> -#define EXT_VIDEO_CAP_BLOCK_Y420CMDB 0x0F
>> +#define EXT_VIDEO_CAP_BLOCK_Y420CMDB 0x0F
>> +#define EXT_VIDEO_HF_EEODB_DATA_BLOCK 0x78
>> #define EDID_BASIC_AUDIO (1 << 6)
>> #define EDID_CEA_YCRCB444 (1 << 5)
>> #define EDID_CEA_YCRCB422 (1 << 4)
>> #define EDID_CEA_VCDB_QS (1 << 6)
>> +#define HF_EEODB_LENGTH 2
>>
>> /*
>> * Search EDID for CEA extension block.
>> @@ -4273,6 +4308,37 @@ static bool cea_db_is_y420vdb(const u8 *db)
>> return true;
>> }
>>
>> +static bool cea_db_is_hdmi_forum_eeodb(const u8 *db) {
>> + if (cea_db_tag(db) != USE_EXTENDED_TAG)
>> + return false;
>> +
>> + if (cea_db_payload_len(db) != HF_EEODB_LENGTH)
>
>We don't have any defines for the length of other blocks. Don't see why this one should be different.
>
>Also, the spec says
>"The Sources that are compliant to This Specification will not be adversely affected by the presence of the additional bytes within the Length field range."
>
Below is what I got from spec. That's why I added definition and check its value in block header.
". Length [5 bits] Total length of data block, not including this byte. For this Data Block, Sinks that follow This Specification shall set the Length field to 2."
>> + return false;
>> +
>> + if (cea_db_extended_tag(db) != EXT_VIDEO_HF_EEODB_DATA_BLOCK)
>> + return false;
>> +
>> + return true;
>> +}
>> +
>> +bool drm_edid_is_hf_eeodb_blk_available(const struct edid *edid) {
>> + const u8 *eeodb_header = (u8 *)edid + EDID_LENGTH + 4;
>> +
>> + if (!edid->extensions)
>> + return false;
>> +
>> + return cea_db_is_hdmi_forum_eeodb(eeodb_header);
>> +}
>> +EXPORT_SYMBOL_GPL(drm_edid_is_hf_eeodb_blk_available);
>> +
>> +u8 drm_edid_read_hf_eeodb_blk_size(const struct edid *edid) {
>> + return ((u8 *)edid)[EDID_LENGTH + 6];
>
>Not a big fan of these hardcoded offsets and stuff. Seems too easy to screw up.
>
My opinion to add cea_db_is_hdmi_forum_eeodb() here to make sure it is EEODB. If so, then allow this function return [EDID_LENGTH + 6] byte value.
Please let me know if any other comments. Thanks!
Best regards,
Shawn
>> +}
>> +EXPORT_SYMBOL_GPL(drm_edid_read_hf_eeodb_blk_size);
>> +
>> #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)
>>
>> diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h index
>> 144c495b99c4..22872d9731a8 100644
>> --- a/include/drm/drm_edid.h
>> +++ b/include/drm/drm_edid.h
>> @@ -593,5 +593,7 @@ drm_display_mode_from_cea_vic(struct drm_device
>> *dev, const u8 *drm_find_edid_extension(const struct edid *edid,
>> int ext_id, int *ext_index);
>>
>> +bool drm_edid_is_hf_eeodb_blk_available(const struct edid *edid);
>> +u8 drm_edid_read_hf_eeodb_blk_size(const struct edid *edid);
>>
>> #endif /* __DRM_EDID_H__ */
>> --
>> 2.17.1
>
>--
>Ville Syrjälä
>Intel
>
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2022-02-27 15:11 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-24 14:16 [PATCH 0/3] enhanced edid driver compatibility Lee Shawn C
2022-02-24 14:16 ` [PATCH 1/3] drm/edid: parse multiple CEA extension block Lee Shawn C
2022-02-25 17:47 ` Ville Syrjälä
2022-02-27 14:49 ` Lee, Shawn C
2022-02-24 14:16 ` [PATCH 2/3] drm/edid: read HF-EEODB ext block Lee Shawn C
2022-02-25 18:09 ` Ville Syrjälä
2022-02-27 15:11 ` Lee, Shawn C
2022-02-24 14:16 ` [PATCH 3/3] drm/edid: parse HF-EEODB CEA extension block Lee Shawn C
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.