On Mon, Jul 03, 2017 at 07:53:44PM +0530, Shashank Sharma wrote:

CEA-861-F introduces extended tag codes for EDID extension blocks, which indicates the actual type of the data block. The code for using exteded tag is 0x7, whereas in the existing code, the corresponding macro is named as "VIDEO_CAPABILITY_BLOCK"

This patch renames the macro and usages from "VIDEO_CAPABILITY_BLOCK" to "CEA_EXTENDED_TAG"

V2: Add extended tag code check for video capabilitiy block (ville)

Cc: Ville Syrjala ville.syrjala@linux.intel.com Signed-off-by: Shashank Sharma shashank.sharma@intel.com

---

drivers/gpu/drm/drm_edid.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 2e55599..5bf9ffe 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -2566,7 +2566,8 @@ add_detailed_modes(struct drm_connector *connector, struct edid *edid, #define VIDEO_BLOCK 0x02 #define VENDOR_BLOCK 0x03 #define SPEAKER_BLOCK 0x04 -#define VIDEO_CAPABILITY_BLOCK 0x07 +#define CEA_EXTENDED_TAG 0x07 +#define VIDEO_CAPBLK_EXT_TAG 0x0

We don't have the _TAG suffix in the other defines either. So maybe something like this?

... #define SPEAKER_BLOCK 0x04 #define USE_EXTENDED_TAG 0x07 #define EXT_VIDEO_CAPABILITY_BLOCK 0x00

#define EDID_BASIC_AUDIO (1 << 6) #define EDID_CEA_YCRCB444 (1 << 5) #define EDID_CEA_YCRCB422 (1 << 4) @@ -3218,6 +3219,12 @@ cea_db_payload_len(const u8 *db) }

static int +cea_db_extended_tag(const u8 *db) +{

• return db[1];

+}

+static int cea_db_tag(const u8 *db) { return db[0] >> 5; @@ -3793,7 +3800,9 @@ bool drm_rgb_quant_range_selectable(struct edid *edid) return false;

for_each_cea_db(edid_ext, i, start, end) {

• if (cea_db_tag(&edid_ext[i]) == VIDEO_CAPABILITY_BLOCK &&
  

• if (cea_db_tag(&edid_ext[i]) == CEA_EXTENDED_TAG &&
  

•     cea_db_extended_tag(&edid_ext[i]) ==
  

• 	VIDEO_CAPBLK_EXT_TAG &&
  

  cea_db_payload_len(&edid_ext[i]) == 2) {

And I suggest doing the length check before the ext tag check, otherwise we're potentially accessing data outside the block. Well, as long as there's *something* there the current order will work, but IMO it's better to be a little paranoid when parsing stuff.

	DRM_DEBUG_KMS("CEA VCDB 0x%02x\n", edid_ext[i + 2]);
	return edid_ext[i + 2] & EDID_CEA_VCDB_QS;

-- 2.7.4