Re: [Intel-gfx] [PATCH v3 02/13] drm/edid: convert drm_connector_update_edid_property() to struct drm_edid

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

On Wed, Jun 22, 2022 at 01:59:16PM +0300, Jani Nikula wrote:
> Make drm_connector_update_edid_property() a thin wrapper around a struct
> drm_edid based version of the same.
> 
> This lets us remove the legacy drm_update_tile_info() and
> drm_add_display_info() functions altogether.
> 
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> ---
>  drivers/gpu/drm/drm_edid.c | 81 ++++++++++++++++----------------------
>  1 file changed, 35 insertions(+), 46 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 36bf7b0fe8d9..62967db78139 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -6042,14 +6042,6 @@ static u32 update_display_info(struct drm_connector *connector,
>  	return quirks;
>  }
>  
> -static u32 drm_add_display_info(struct drm_connector *connector, const struct edid *edid)
> -{
> -	struct drm_edid drm_edid;
> -
> -	return update_display_info(connector,
> -				   drm_edid_legacy_init(&drm_edid, edid));
> -}
> -
>  static struct drm_display_mode *drm_mode_displayid_detailed(struct drm_device *dev,
>  							    struct displayid_detailed_timings_1 *timings,
>  							    bool type_7)
> @@ -6206,38 +6198,19 @@ static int drm_edid_connector_update(struct drm_connector *connector,
>  	return num_modes;
>  }
>  
> -static void drm_update_tile_info(struct drm_connector *connector,
> -				 const struct edid *edid);
> +static void _drm_update_tile_info(struct drm_connector *connector,
> +				  const struct drm_edid *drm_edid);
>  
> -/**
> - * drm_connector_update_edid_property - update the edid property of a connector
> - * @connector: drm connector
> - * @edid: new value of the edid property
> - *
> - * This function creates a new blob modeset object and assigns its id to the
> - * connector's edid property.
> - * Since we also parse tile information from EDID's displayID block, we also
> - * set the connector's tile property here. See drm_connector_set_tile_property()
> - * for more details.
> - *
> - * Returns:
> - * Zero on success, negative errno on failure.
> - */
> -int drm_connector_update_edid_property(struct drm_connector *connector,
> -				       const struct edid *edid)
> +static int _drm_connector_update_edid_property(struct drm_connector *connector,
> +					       const struct drm_edid *drm_edid)
>  {
>  	struct drm_device *dev = connector->dev;
> -	size_t size = 0;
>  	int ret;
> -	const struct edid *old_edid;
>  
>  	/* ignore requests to set edid when overridden */
>  	if (connector->override_edid)
>  		return 0;
>  
> -	if (edid)
> -		size = EDID_LENGTH * (1 + edid->extensions);
> -
>  	/*
>  	 * Set the display info, using edid if available, otherwise resetting
>  	 * the values to defaults. This duplicates the work done in
> @@ -6246,17 +6219,18 @@ int drm_connector_update_edid_property(struct drm_connector *connector,
>  	 * that it seems better to duplicate it rather than attempt to ensure
>  	 * some arbitrary ordering of calls.
>  	 */
> -	if (edid)
> -		drm_add_display_info(connector, edid);
> +	if (drm_edid)
> +		update_display_info(connector, drm_edid);
>  	else
>  		drm_reset_display_info(connector);
>  
> -	drm_update_tile_info(connector, edid);
> +	_drm_update_tile_info(connector, drm_edid);
>  
>  	if (connector->edid_blob_ptr) {
> -		old_edid = (const struct edid *)connector->edid_blob_ptr->data;
> +		const struct edid *old_edid = connector->edid_blob_ptr->data;
> +
>  		if (old_edid) {
> -			if (!drm_edid_are_equal(edid, old_edid)) {
> +			if (!drm_edid_are_equal(drm_edid ? drm_edid->edid : NULL, old_edid)) {
>  				DRM_DEBUG_KMS("[CONNECTOR:%d:%s] Edid was changed.\n",
>  					      connector->base.id, connector->name);
>  
> @@ -6273,14 +6247,37 @@ int drm_connector_update_edid_property(struct drm_connector *connector,
>  
>  	ret = drm_property_replace_global_blob(dev,
>  					       &connector->edid_blob_ptr,
> -					       size,
> -					       edid,
> +					       drm_edid ? drm_edid->size : 0,
> +					       drm_edid ? drm_edid->edid : NULL,
>  					       &connector->base,
>  					       dev->mode_config.edid_property);
>  	if (ret)
>  		return ret;
>  	return drm_connector_set_tile_property(connector);
>  }
> +
> +/**
> + * drm_connector_update_edid_property - update the edid property of a connector
> + * @connector: drm connector
> + * @edid: new value of the edid property
> + *
> + * This function creates a new blob modeset object and assigns its id to the
> + * connector's edid property.
> + * Since we also parse tile information from EDID's displayID block, we also
> + * set the connector's tile property here. See drm_connector_set_tile_property()
> + * for more details.
> + *
> + * Returns:
> + * Zero on success, negative errno on failure.
> + */
> +int drm_connector_update_edid_property(struct drm_connector *connector,
> +				       const struct edid *edid)
> +{
> +	struct drm_edid drm_edid;
> +
> +	return _drm_connector_update_edid_property(connector,
> +						   drm_edid_legacy_init(&drm_edid, edid));
> +}
>  EXPORT_SYMBOL(drm_connector_update_edid_property);
>  
>  /**
> @@ -6720,11 +6717,3 @@ static void _drm_update_tile_info(struct drm_connector *connector,
>  		connector->tile_group = NULL;
>  	}
>  }
> -
> -static void drm_update_tile_info(struct drm_connector *connector,
> -				 const struct edid *edid)
> -{
> -	struct drm_edid drm_edid;
> -
> -	_drm_update_tile_info(connector, drm_edid_legacy_init(&drm_edid, edid));
> -}
> -- 
> 2.30.2

-- 
Ville Syrjälä
Intel