On Wed, Jun 22, 2022 at 01:59:19PM +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.
>
> Since the new drm_edid_read*() functions no longer call the connector
> updates directly, and the read and update are separated, we'll need this
> new function for the connector update.
>
> This is all in drm_edid.c simply to keep struct drm_edid opaque.
>
> v2:
> - Share code with drm_connector_update_edid_property() (Ville)
> - Add comment about override EDID handling
>
> Signed-off-by: Jani Nikula <jani.nikula@xxxxxxxxx>
Had to take notes to figure who did/does what. But it does look
like non-static stuff should end up doing the same thing before
and after this patch, apart from the new function that is.
Reviewed-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> ---
> drivers/gpu/drm/drm_edid.c | 103 ++++++++++++++++++++++++++++---------
> include/drm/drm_edid.h | 2 +
> 2 files changed, 81 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index c3f0f0a5a8a9..41b3de52b8f1 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -6160,8 +6160,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;
> @@ -6227,31 +6227,12 @@ static int drm_edid_connector_update(struct drm_connector *connector,
> static void _drm_update_tile_info(struct drm_connector *connector,
> const struct drm_edid *drm_edid);
>
> -static int _drm_connector_update_edid_property(struct drm_connector *connector,
> +static int _drm_edid_connector_property_update(struct drm_connector *connector,
> const struct drm_edid *drm_edid)
> {
> struct drm_device *dev = connector->dev;
> int ret;
>
> - /* ignore requests to set edid when overridden */
> - if (connector->override_edid)
> - return 0;
> -
> - /*
> - * Set the display info, using edid if available, otherwise resetting
> - * the values to defaults. This duplicates the work done in
> - * drm_add_edid_modes, but that function is not consistently called
> - * before this one in all drivers and the computation is cheap enough
> - * that it seems better to duplicate it rather than attempt to ensure
> - * some arbitrary ordering of calls.
> - */
> - if (drm_edid)
> - update_display_info(connector, drm_edid);
> - else
> - drm_reset_display_info(connector);
> -
> - _drm_update_tile_info(connector, drm_edid);
> -
> if (connector->edid_blob_ptr) {
> const struct edid *old_edid = connector->edid_blob_ptr->data;
>
> @@ -6297,6 +6278,76 @@ static int _drm_connector_update_edid_property(struct drm_connector *connector,
> return ret;
> }
>
> +/**
> + * 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)
> +{
> + int count;
> +
> + /*
> + * FIXME: Reconcile the differences in override_edid handling between
> + * this and drm_connector_update_edid_property().
> + *
> + * If override_edid is set, and the EDID passed in here originates from
> + * drm_edid_read() and friends, it will be the override EDID, and there
> + * are no issues. drm_connector_update_edid_property() ignoring requests
> + * to set the EDID dates back to a time when override EDID was not
> + * handled at the low level EDID read.
> + *
> + * The only way the EDID passed in here can be different from the
> + * override EDID is when a driver passes in an EDID that does *not*
> + * originate from drm_edid_read() and friends, or passes in a stale
> + * cached version. This, in turn, is a question of when an override EDID
> + * set via debugfs should take effect.
> + */
> +
> + count = _drm_edid_connector_update(connector, drm_edid);
> +
> + _drm_update_tile_info(connector, drm_edid);
> +
> + /* Note: Ignore errors for now. */
> + _drm_edid_connector_property_update(connector, drm_edid);
> +
> + return count;
> +}
> +EXPORT_SYMBOL(drm_edid_connector_update);
> +
> +static int _drm_connector_update_edid_property(struct drm_connector *connector,
> + const struct drm_edid *drm_edid)
> +{
> + /* ignore requests to set edid when overridden */
> + if (connector->override_edid)
> + return 0;
> +
> + /*
> + * Set the display info, using edid if available, otherwise resetting
> + * the values to defaults. This duplicates the work done in
> + * drm_add_edid_modes, but that function is not consistently called
> + * before this one in all drivers and the computation is cheap enough
> + * that it seems better to duplicate it rather than attempt to ensure
> + * some arbitrary ordering of calls.
> + */
> + if (drm_edid)
> + update_display_info(connector, drm_edid);
> + else
> + drm_reset_display_info(connector);
> +
> + _drm_update_tile_info(connector, drm_edid);
> +
> + return _drm_edid_connector_property_update(connector, drm_edid);
> +}
> +
> /**
> * drm_connector_update_edid_property - update the edid property of a connector
> * @connector: drm connector
> @@ -6308,6 +6359,8 @@ static int _drm_connector_update_edid_property(struct drm_connector *connector,
> * 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.
> */
> @@ -6330,6 +6383,8 @@ EXPORT_SYMBOL(drm_connector_update_edid_property);
> * &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)
> @@ -6342,8 +6397,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 9d2d78135dee..aeb2fa95bc04 100644
> --- a/include/drm/drm_edid.h
> +++ b/include/drm/drm_edid.h
> @@ -603,6 +603,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
