Re: [Intel-gfx] [RFC 02/19] drm: use drm_edid_extension_block_count() and drm_edid_size()

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

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

-- 
Ville Syrjälä
Intel