[2/3] drm/edid: Iterate through all DispID ext blocks
diff mbox series

Message ID 20200527130310.27099-2-ville.syrjala@linux.intel.com
State New
Headers show
Series
  • [1/3] drm/edid: Allow looking for ext blocks starting from a specified index
Related show

Commit Message

Ville Syrjälä May 27, 2020, 1:03 p.m. UTC
From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Apparently there are EDIDs in the wild with multiple DispID extension
blocks. Iterate through them all.

In one particular case the tile information is specicied in the
second DispID ext block, and since the current parser only looks
at the first DispID ext block we don't notice that we're dealing
with a tiled display.

While at it change a few functions to return void since we have
no use for the errno.

References: https://gitlab.freedesktop.org/drm/intel/-/issues/27
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/drm_edid.c | 84 +++++++++++++++++---------------------
 1 file changed, 38 insertions(+), 46 deletions(-)

Comments

Souza, Jose July 1, 2020, midnight UTC | #1
On Wed, 2020-05-27 at 16:03 +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Apparently there are EDIDs in the wild with multiple DispID extension
> blocks. Iterate through them all.
> 
> In one particular case the tile information is specicied in the
> second DispID ext block, and since the current parser only looks
> at the first DispID ext block we don't notice that we're dealing
> with a tiled display.
> 
> While at it change a few functions to return void since we have
> no use for the errno.

With the change in the previous patch:
Reviewed-by: José Roberto de Souza <jose.souza@intel.com>

> 
> References: https://gitlab.freedesktop.org/drm/intel/-/issues/27
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/drm_edid.c | 84 +++++++++++++++++---------------------
>  1 file changed, 38 insertions(+), 46 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index f2531d51dfa2..dcb23563d29d 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -3248,6 +3248,7 @@ static u8 *drm_find_cea_extension(const struct edid *edid)
>  	int ext_index;
>  
>  	/* Look for a top level CEA extension block */
> +	/* FIXME: make callers iterate through multiple CEA ext blocks? */
>  	ext_index = 0;
>  	cea = drm_find_edid_extension(edid, CEA_EXT, &ext_index);
>  	if (cea)
> @@ -3255,20 +3256,20 @@ static u8 *drm_find_cea_extension(const struct edid *edid)
>  
>  	/* CEA blocks can also be found embedded in a DisplayID block */
>  	ext_index = 0;
> -	displayid = drm_find_displayid_extension(edid, &length, &idx,
> -						 &ext_index);
> -	if (!displayid)
> -		return NULL;
> +	for (;;) {
> +		displayid = drm_find_displayid_extension(edid, &length, &idx,
> +							 &ext_index);
> +		if (!displayid)
> +			return NULL;
>  
> -	idx += sizeof(struct displayid_hdr);
> -	for_each_displayid_db(displayid, block, idx, length) {
> -		if (block->tag == DATA_BLOCK_CTA) {
> -			cea = (u8 *)block;
> -			break;
> +		idx += sizeof(struct displayid_hdr);
> +		for_each_displayid_db(displayid, block, idx, length) {
> +			if (block->tag == DATA_BLOCK_CTA)
> +				return (u8 *)block;
>  		}
>  	}
>  
> -	return cea;
> +	return NULL;
>  }
>  
>  static __always_inline const struct drm_display_mode *cea_mode_for_vic(u8 vic)
> @@ -5205,19 +5206,22 @@ static int add_displayid_detailed_modes(struct drm_connector *connector,
>  	int num_modes = 0;
>  	int ext_index = 0;
>  
> -	displayid = drm_find_displayid_extension(edid, &length, &idx,
> -						 &ext_index);
> -	if (!displayid)
> -		return 0;
> -
> -	idx += sizeof(struct displayid_hdr);
> -	for_each_displayid_db(displayid, block, idx, length) {
> -		switch (block->tag) {
> -		case DATA_BLOCK_TYPE_1_DETAILED_TIMING:
> -			num_modes += add_displayid_detailed_1_modes(connector, block);
> +	for (;;) {
> +		displayid = drm_find_displayid_extension(edid, &length, &idx,
> +							 &ext_index);
> +		if (!displayid)
>  			break;
> +
> +		idx += sizeof(struct displayid_hdr);
> +		for_each_displayid_db(displayid, block, idx, length) {
> +			switch (block->tag) {
> +			case DATA_BLOCK_TYPE_1_DETAILED_TIMING:
> +				num_modes += add_displayid_detailed_1_modes(connector, block);
> +				break;
> +			}
>  		}
>  	}
> +
>  	return num_modes;
>  }
>  
> @@ -5797,8 +5801,8 @@ drm_hdmi_vendor_infoframe_from_display_mode(struct hdmi_vendor_infoframe *frame,
>  }
>  EXPORT_SYMBOL(drm_hdmi_vendor_infoframe_from_display_mode);
>  
> -static int drm_parse_tiled_block(struct drm_connector *connector,
> -				 const struct displayid_block *block)
> +static void drm_parse_tiled_block(struct drm_connector *connector,
> +				  const struct displayid_block *block)
>  {
>  	const struct displayid_tiled_block *tile = (struct displayid_tiled_block *)block;
>  	u16 w, h;
> @@ -5836,7 +5840,7 @@ static int drm_parse_tiled_block(struct drm_connector *connector,
>  		tg = drm_mode_create_tile_group(connector->dev, tile->topology_id);
>  	}
>  	if (!tg)
> -		return -ENOMEM;
> +		return;
>  
>  	if (connector->tile_group != tg) {
>  		/* if we haven't got a pointer,
> @@ -5848,14 +5852,12 @@ static int drm_parse_tiled_block(struct drm_connector *connector,
>  	} else
>  		/* if same tile group, then release the ref we just took. */
>  		drm_mode_put_tile_group(connector->dev, tg);
> -	return 0;
>  }
>  
> -static int drm_displayid_parse_tiled(struct drm_connector *connector,
> -				     const u8 *displayid, int length, int idx)
> +static void drm_displayid_parse_tiled(struct drm_connector *connector,
> +				      const u8 *displayid, int length, int idx)
>  {
>  	const struct displayid_block *block;
> -	int ret;
>  
>  	idx += sizeof(struct displayid_hdr);
>  	for_each_displayid_db(displayid, block, idx, length) {
> @@ -5864,16 +5866,13 @@ static int drm_displayid_parse_tiled(struct drm_connector *connector,
>  
>  		switch (block->tag) {
>  		case DATA_BLOCK_TILED_DISPLAY:
> -			ret = drm_parse_tiled_block(connector, block);
> -			if (ret)
> -				return ret;
> +			drm_parse_tiled_block(connector, block);
>  			break;
>  		default:
>  			DRM_DEBUG_KMS("found DisplayID tag 0x%x, unhandled\n", block->tag);
>  			break;
>  		}
>  	}
> -	return 0;
>  }
>  
>  void drm_update_tile_info(struct drm_connector *connector,
> @@ -5882,26 +5881,19 @@ void drm_update_tile_info(struct drm_connector *connector,
>  	const void *displayid = NULL;
>  	int ext_index = 0;
>  	int length, idx;
> -	int ret;
>  
>  	connector->has_tile = false;
> -	displayid = drm_find_displayid_extension(edid, &length, &idx,
> -						 &ext_index);
> -	if (!displayid) {
> -		/* drop reference to any tile group we had */
> -		goto out_drop_ref;
> +	for (;;) {
> +		displayid = drm_find_displayid_extension(edid, &length, &idx,
> +							 &ext_index);
> +		if (!displayid)
> +			break;
> +
> +		drm_displayid_parse_tiled(connector, displayid, length, idx);
>  	}
>  
> -	ret = drm_displayid_parse_tiled(connector, displayid, length, idx);
> -	if (ret < 0)
> -		goto out_drop_ref;
> -	if (!connector->has_tile)
> -		goto out_drop_ref;
> -	return;
> -out_drop_ref:
> -	if (connector->tile_group) {
> +	if (!connector->has_tile && connector->tile_group) {
>  		drm_mode_put_tile_group(connector->dev, connector->tile_group);
>  		connector->tile_group = NULL;
>  	}
> -	return;
>  }

Patch
diff mbox series

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index f2531d51dfa2..dcb23563d29d 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -3248,6 +3248,7 @@  static u8 *drm_find_cea_extension(const struct edid *edid)
 	int ext_index;
 
 	/* Look for a top level CEA extension block */
+	/* FIXME: make callers iterate through multiple CEA ext blocks? */
 	ext_index = 0;
 	cea = drm_find_edid_extension(edid, CEA_EXT, &ext_index);
 	if (cea)
@@ -3255,20 +3256,20 @@  static u8 *drm_find_cea_extension(const struct edid *edid)
 
 	/* CEA blocks can also be found embedded in a DisplayID block */
 	ext_index = 0;
-	displayid = drm_find_displayid_extension(edid, &length, &idx,
-						 &ext_index);
-	if (!displayid)
-		return NULL;
+	for (;;) {
+		displayid = drm_find_displayid_extension(edid, &length, &idx,
+							 &ext_index);
+		if (!displayid)
+			return NULL;
 
-	idx += sizeof(struct displayid_hdr);
-	for_each_displayid_db(displayid, block, idx, length) {
-		if (block->tag == DATA_BLOCK_CTA) {
-			cea = (u8 *)block;
-			break;
+		idx += sizeof(struct displayid_hdr);
+		for_each_displayid_db(displayid, block, idx, length) {
+			if (block->tag == DATA_BLOCK_CTA)
+				return (u8 *)block;
 		}
 	}
 
-	return cea;
+	return NULL;
 }
 
 static __always_inline const struct drm_display_mode *cea_mode_for_vic(u8 vic)
@@ -5205,19 +5206,22 @@  static int add_displayid_detailed_modes(struct drm_connector *connector,
 	int num_modes = 0;
 	int ext_index = 0;
 
-	displayid = drm_find_displayid_extension(edid, &length, &idx,
-						 &ext_index);
-	if (!displayid)
-		return 0;
-
-	idx += sizeof(struct displayid_hdr);
-	for_each_displayid_db(displayid, block, idx, length) {
-		switch (block->tag) {
-		case DATA_BLOCK_TYPE_1_DETAILED_TIMING:
-			num_modes += add_displayid_detailed_1_modes(connector, block);
+	for (;;) {
+		displayid = drm_find_displayid_extension(edid, &length, &idx,
+							 &ext_index);
+		if (!displayid)
 			break;
+
+		idx += sizeof(struct displayid_hdr);
+		for_each_displayid_db(displayid, block, idx, length) {
+			switch (block->tag) {
+			case DATA_BLOCK_TYPE_1_DETAILED_TIMING:
+				num_modes += add_displayid_detailed_1_modes(connector, block);
+				break;
+			}
 		}
 	}
+
 	return num_modes;
 }
 
@@ -5797,8 +5801,8 @@  drm_hdmi_vendor_infoframe_from_display_mode(struct hdmi_vendor_infoframe *frame,
 }
 EXPORT_SYMBOL(drm_hdmi_vendor_infoframe_from_display_mode);
 
-static int drm_parse_tiled_block(struct drm_connector *connector,
-				 const struct displayid_block *block)
+static void drm_parse_tiled_block(struct drm_connector *connector,
+				  const struct displayid_block *block)
 {
 	const struct displayid_tiled_block *tile = (struct displayid_tiled_block *)block;
 	u16 w, h;
@@ -5836,7 +5840,7 @@  static int drm_parse_tiled_block(struct drm_connector *connector,
 		tg = drm_mode_create_tile_group(connector->dev, tile->topology_id);
 	}
 	if (!tg)
-		return -ENOMEM;
+		return;
 
 	if (connector->tile_group != tg) {
 		/* if we haven't got a pointer,
@@ -5848,14 +5852,12 @@  static int drm_parse_tiled_block(struct drm_connector *connector,
 	} else
 		/* if same tile group, then release the ref we just took. */
 		drm_mode_put_tile_group(connector->dev, tg);
-	return 0;
 }
 
-static int drm_displayid_parse_tiled(struct drm_connector *connector,
-				     const u8 *displayid, int length, int idx)
+static void drm_displayid_parse_tiled(struct drm_connector *connector,
+				      const u8 *displayid, int length, int idx)
 {
 	const struct displayid_block *block;
-	int ret;
 
 	idx += sizeof(struct displayid_hdr);
 	for_each_displayid_db(displayid, block, idx, length) {
@@ -5864,16 +5866,13 @@  static int drm_displayid_parse_tiled(struct drm_connector *connector,
 
 		switch (block->tag) {
 		case DATA_BLOCK_TILED_DISPLAY:
-			ret = drm_parse_tiled_block(connector, block);
-			if (ret)
-				return ret;
+			drm_parse_tiled_block(connector, block);
 			break;
 		default:
 			DRM_DEBUG_KMS("found DisplayID tag 0x%x, unhandled\n", block->tag);
 			break;
 		}
 	}
-	return 0;
 }
 
 void drm_update_tile_info(struct drm_connector *connector,
@@ -5882,26 +5881,19 @@  void drm_update_tile_info(struct drm_connector *connector,
 	const void *displayid = NULL;
 	int ext_index = 0;
 	int length, idx;
-	int ret;
 
 	connector->has_tile = false;
-	displayid = drm_find_displayid_extension(edid, &length, &idx,
-						 &ext_index);
-	if (!displayid) {
-		/* drop reference to any tile group we had */
-		goto out_drop_ref;
+	for (;;) {
+		displayid = drm_find_displayid_extension(edid, &length, &idx,
+							 &ext_index);
+		if (!displayid)
+			break;
+
+		drm_displayid_parse_tiled(connector, displayid, length, idx);
 	}
 
-	ret = drm_displayid_parse_tiled(connector, displayid, length, idx);
-	if (ret < 0)
-		goto out_drop_ref;
-	if (!connector->has_tile)
-		goto out_drop_ref;
-	return;
-out_drop_ref:
-	if (connector->tile_group) {
+	if (!connector->has_tile && connector->tile_group) {
 		drm_mode_put_tile_group(connector->dev, connector->tile_group);
 		connector->tile_group = NULL;
 	}
-	return;
 }