[Intel-gfx] [PATCH v1 04/13] drm/edid: add drm_edid_connector_update()

[Ville Syrjälä]

On Tue, May 24, 2022 at 01:39:26PM +0300, Jani Nikula wrote:
> Add a new function drm_edid_connector_update() to replace the
> combination of calls drm_connector_update_edid_property() and
> drm_add_edid_modes(). Usually they are called in the drivers in this
> order, however the former needs information from the latter.
> 
> This is all in drm_edid.c simply to keep struct drm_edid opaque.
> 
> Cc: David Airlie <airlied at linux.ie>
> Cc: Daniel Vetter <daniel at ffwll.ch>
> Signed-off-by: Jani Nikula <jani.nikula at intel.com>
> ---
>  drivers/gpu/drm/drm_connector.c |  2 +
>  drivers/gpu/drm/drm_edid.c      | 71 +++++++++++++++++++++++++++++++--
>  include/drm/drm_edid.h          |  2 +
>  3 files changed, 71 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index 1c48d162c77e..ae9c640a641a 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -2088,6 +2088,8 @@ EXPORT_SYMBOL(drm_connector_set_tile_property);
>   * set the connector's tile property here. See drm_connector_set_tile_property()
>   * for more details.
>   *
> + * This function is deprecated. Use drm_edid_connector_update() instead.
> + *
>   * Returns:
>   * Zero on success, negative errno on failure.
>   */
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 0d640e7dcff7..09328298aabf 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -6127,8 +6127,8 @@ static int add_displayid_detailed_modes(struct drm_connector *connector,
>  	return num_modes;
>  }
>  
> -static int drm_edid_connector_update(struct drm_connector *connector,
> -				     const struct drm_edid *drm_edid)
> +static int _drm_edid_connector_update(struct drm_connector *connector,
> +				      const struct drm_edid *drm_edid)
>  {
>  	int num_modes = 0;
>  	u32 quirks;
> @@ -6191,6 +6191,67 @@ 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 drm_edid *drm_edid);
> +
> +/**
> + * drm_edid_connector_update - Update connector information from EDID
> + * @connector: Connector
> + * @drm_edid: EDID
> + *
> + * Update the connector mode list, display info, ELD, HDR metadata, relevant
> + * properties, etc. from the passed in EDID.
> + *
> + * If EDID is NULL, reset the information.
> + *
> + * Return: The number of modes added or 0 if we couldn't find any.
> + */
> +int drm_edid_connector_update(struct drm_connector *connector,
> +			      const struct drm_edid *drm_edid)
> +{
> +	struct drm_device *dev = connector->dev;
> +	const struct edid *old_edid = connector->edid_blob_ptr ?
> +		connector->edid_blob_ptr->data : NULL;
> +	const struct edid *edid = drm_edid ? drm_edid->edid : NULL;
> +	size_t size = drm_edid ? drm_edid->size : 0;
> +	int count, ret;
> +
> +	count = _drm_edid_connector_update(connector, drm_edid);
> +
> +	_drm_update_tile_info(connector, drm_edid);
> +
> +	if (old_edid && !drm_edid_are_equal(edid, old_edid)) {
> +		connector->epoch_counter++;
> +
> +		drm_dbg_kms(dev, "[CONNECTOR:%d:%s] EDID changed, epoch counter %llu\n",
> +			    connector->base.id, connector->name,
> +			    connector->epoch_counter);

There was some recent discussion on the blob IDs changing willy nilly.
We could avoid some of that by bailing out here if nothing changed.
But that's not really something that should be done in this patch.


> +	}
> +
> +	ret = drm_property_replace_global_blob(dev, &connector->edid_blob_ptr,
> +					       size, edid,
> +					       &connector->base,
> +					       dev->mode_config.edid_property);
> +	if (ret)
> +		drm_dbg_kms(dev, "[CONNECTOR:%d:%s] EDID property update failed (%d)\n",
> +			    connector->base.id, connector->name, ret);
> +
> +	ret = drm_object_property_set_value(&connector->base,
> +					    dev->mode_config.non_desktop_property,
> +					    connector->display_info.non_desktop);
> +	if (ret)
> +		drm_dbg_kms(dev, "[CONNECTOR:%d:%s] Non-desktop property update failed (%d)\n",
> +			    connector->base.id, connector->name, ret);
> +
> +	ret = drm_connector_set_tile_property(connector);
> +	if (ret)
> +		drm_dbg_kms(dev, "[CONNECTOR:%d:%s] Tile property update failed (%d)\n",
> +			    connector->base.id, connector->name, ret);

I was a bit confused why this function is needed, until I recalled that
the EDID read thing you introduce in the earlier patch doesn't do
any of this (unlike the old function). Would be good to point that
out in the commit message.

> +
> +	return count;
> +}
> +EXPORT_SYMBOL(drm_edid_connector_update);
> +
>  /**
>   * drm_add_edid_modes - add modes from EDID data, if available
>   * @connector: connector we're probing
> @@ -6200,6 +6261,8 @@ static int drm_edid_connector_update(struct drm_connector *connector,
>   * &drm_display_info structure and ELD in @connector with any information which
>   * can be derived from the edid.
>   *
> + * This function is deprecated. Use drm_edid_connector_update() instead.
> + *
>   * Return: The number of modes added or 0 if we couldn't find any.
>   */
>  int drm_add_edid_modes(struct drm_connector *connector, struct edid *edid)
> @@ -6212,8 +6275,8 @@ int drm_add_edid_modes(struct drm_connector *connector, struct edid *edid)
>  		edid = NULL;
>  	}
>  
> -	return drm_edid_connector_update(connector,
> -					 drm_edid_legacy_init(&drm_edid, edid));
> +	return _drm_edid_connector_update(connector,
> +					  drm_edid_legacy_init(&drm_edid, edid));
>  }
>  EXPORT_SYMBOL(drm_add_edid_modes);
>  
> diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
> index b729e18968dd..289b956d31ea 100644
> --- a/include/drm/drm_edid.h
> +++ b/include/drm/drm_edid.h
> @@ -590,6 +590,8 @@ const struct drm_edid *drm_edid_read_ddc(struct drm_connector *connector,
>  const struct drm_edid *drm_edid_read_custom(struct drm_connector *connector,
>  					    int (*read_block)(void *context, u8 *buf, unsigned int block, size_t len),
>  					    void *context);
> +int drm_edid_connector_update(struct drm_connector *connector,
> +			      const struct drm_edid *edid);
>  const u8 *drm_find_edid_extension(const struct drm_edid *drm_edid,
>  				  int ext_id, int *ext_index);
>  
> -- 
> 2.30.2

-- 
Ville Syrjälä
Intel