The function needs access to drm_edid.c internals more than drm_connector.c. We can make drm_reset_display_info(), drm_add_display_info() and drm_update_tile_info() static. There will be more benefits with follow-up struct drm_edid refactoring.

Signed-off-by: Jani Nikula jani.nikula@intel.com --- drivers/gpu/drm/drm_connector.c | 74 ------------------------- drivers/gpu/drm/drm_crtc_internal.h | 3 - drivers/gpu/drm/drm_edid.c | 86 +++++++++++++++++++++++++++-- 3 files changed, 81 insertions(+), 82 deletions(-)

diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c index 28ea0f8196b9..2b9a8972eff1 100644 --- a/drivers/gpu/drm/drm_connector.c +++ b/drivers/gpu/drm/drm_connector.c @@ -2078,80 +2078,6 @@ int drm_connector_set_tile_property(struct drm_connector *connector) } EXPORT_SYMBOL(drm_connector_set_tile_property);

-/** - * 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_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 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 (edid) - drm_add_display_info(connector, edid); - else - drm_reset_display_info(connector); - - drm_update_tile_info(connector, edid); - - if (connector->edid_blob_ptr) { - old_edid = (const struct edid *)connector->edid_blob_ptr->data; - if (old_edid) { - if (!drm_edid_are_equal(edid, old_edid)) { - DRM_DEBUG_KMS("[CONNECTOR:%d:%s] Edid was changed.\n", - connector->base.id, connector->name); - - connector->epoch_counter += 1; - DRM_DEBUG_KMS("Updating change counter to %llu\n", - connector->epoch_counter); - } - } - } - - drm_object_property_set_value(&connector->base, - dev->mode_config.non_desktop_property, - connector->display_info.non_desktop); - - ret = drm_property_replace_global_blob(dev, - &connector->edid_blob_ptr, - size, - edid, - &connector->base, - dev->mode_config.edid_property); - if (ret) - return ret; - return drm_connector_set_tile_property(connector); -} -EXPORT_SYMBOL(drm_connector_update_edid_property); - /** * drm_connector_set_link_status_property - Set link status property of a connector * @connector: drm connector diff --git a/drivers/gpu/drm/drm_crtc_internal.h b/drivers/gpu/drm/drm_crtc_internal.h index 63279e984342..aecab5308bae 100644 --- a/drivers/gpu/drm/drm_crtc_internal.h +++ b/drivers/gpu/drm/drm_crtc_internal.h @@ -286,6 +286,3 @@ int drm_mode_page_flip_ioctl(struct drm_device *dev,

/* drm_edid.c */ void drm_mode_fixup_1366x768(struct drm_display_mode *mode); -void drm_reset_display_info(struct drm_connector *connector); -u32 drm_add_display_info(struct drm_connector *connector, const struct edid *edid); -void drm_update_tile_info(struct drm_connector *connector, const struct edid *edid); diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 2bdaf1e34a9d..36bf7b0fe8d9 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -5928,8 +5928,7 @@ static void drm_update_mso(struct drm_connector *connector, /* A connector has no EDID information, so we've got no EDID to compute quirks from. Reset * all of the values which would have been set from EDID */ -void -drm_reset_display_info(struct drm_connector *connector) +static void drm_reset_display_info(struct drm_connector *connector) { struct drm_display_info *info = &connector->display_info;

@@ -6043,7 +6042,7 @@ static u32 update_display_info(struct drm_connector *connector, return quirks; }

-u32 drm_add_display_info(struct drm_connector *connector, const struct edid *edid) +static u32 drm_add_display_info(struct drm_connector *connector, const struct edid *edid) { struct drm_edid drm_edid;

@@ -6207,6 +6206,83 @@ 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); + +/** + * 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_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 + * 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 (edid) + drm_add_display_info(connector, edid); + else + drm_reset_display_info(connector); + + drm_update_tile_info(connector, edid); + + if (connector->edid_blob_ptr) { + old_edid = (const struct edid *)connector->edid_blob_ptr->data; + if (old_edid) { + if (!drm_edid_are_equal(edid, old_edid)) { + DRM_DEBUG_KMS("[CONNECTOR:%d:%s] Edid was changed.\n", + connector->base.id, connector->name); + + connector->epoch_counter += 1; + DRM_DEBUG_KMS("Updating change counter to %llu\n", + connector->epoch_counter); + } + } + } + + drm_object_property_set_value(&connector->base, + dev->mode_config.non_desktop_property, + connector->display_info.non_desktop); + + ret = drm_property_replace_global_blob(dev, + &connector->edid_blob_ptr, + size, + edid, + &connector->base, + dev->mode_config.edid_property); + if (ret) + return ret; + return drm_connector_set_tile_property(connector); +} +EXPORT_SYMBOL(drm_connector_update_edid_property); + /** * drm_add_edid_modes - add modes from EDID data, if available * @connector: connector we're probing @@ -6645,8 +6721,8 @@ static void _drm_update_tile_info(struct drm_connector *connector, } }

-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 edid *edid) { struct drm_edid drm_edid;

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 --- 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)); -}

Bail out on all errors, debug log all errors, and convert to drm device based debug logging.

Signed-off-by: Jani Nikula jani.nikula@intel.com --- drivers/gpu/drm/drm_edid.c | 41 ++++++++++++++++++++++++++------------ 1 file changed, 28 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 62967db78139..e360e1a269f4 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -6231,29 +6231,44 @@ static int _drm_connector_update_edid_property(struct drm_connector *connector,

if (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); - - connector->epoch_counter += 1; - DRM_DEBUG_KMS("Updating change counter to %llu\n", - connector->epoch_counter); + connector->epoch_counter++; + drm_dbg_kms(dev, "[CONNECTOR:%d:%s] EDID changed, epoch counter %llu\n", + connector->base.id, connector->name, + connector->epoch_counter); } } }

- drm_object_property_set_value(&connector->base, - dev->mode_config.non_desktop_property, - connector->display_info.non_desktop); - ret = drm_property_replace_global_blob(dev, &connector->edid_blob_ptr, 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); + if (ret) { + drm_dbg_kms(dev, "[CONNECTOR:%d:%s] EDID property update failed (%d)\n", + connector->base.id, connector->name, ret); + goto out; + } + + 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); + goto out; + } + + 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); + goto out; + } + +out: + return ret; }

/**

Add functions drm_edid_override_set() and drm_edid_override_reset() to support "edid_override" connector debugfs, and to hide the details about it in drm_edid.c. No functional changes at this time.

Also note in the connector.override_edid flag kernel-doc that this is only supposed to be modified by the code doing debugfs EDID override handling. Currently, it is still being modified by amdgpu in create_eml_sink() and handle_edid_mgmt() for reasons unknown. This was added in commit 4562236b3bc0 ("drm/amd/dc: Add dc display driver (v2)") and later moved to amdgpu_dm.c in commit e7b07ceef2a6 ("drm/amd/display: Merge amdgpu_dm_types and amdgpu_dm").

Signed-off-by: Jani Nikula jani.nikula@intel.com --- drivers/gpu/drm/drm_crtc_internal.h | 2 ++ drivers/gpu/drm/drm_debugfs.c | 21 +++++---------------- drivers/gpu/drm/drm_edid.c | 26 ++++++++++++++++++++++++++ include/drm/drm_connector.h | 6 +++++- 4 files changed, 38 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/drm_crtc_internal.h b/drivers/gpu/drm/drm_crtc_internal.h index aecab5308bae..56041b604881 100644 --- a/drivers/gpu/drm/drm_crtc_internal.h +++ b/drivers/gpu/drm/drm_crtc_internal.h @@ -286,3 +286,5 @@ int drm_mode_page_flip_ioctl(struct drm_device *dev,

/* drm_edid.c */ void drm_mode_fixup_1366x768(struct drm_display_mode *mode); +int drm_edid_override_set(struct drm_connector *connector, const void *edid, size_t size); +int drm_edid_override_reset(struct drm_connector *connector); diff --git a/drivers/gpu/drm/drm_debugfs.c b/drivers/gpu/drm/drm_debugfs.c index fb04b7a984de..493922069c90 100644 --- a/drivers/gpu/drm/drm_debugfs.c +++ b/drivers/gpu/drm/drm_debugfs.c @@ -350,31 +350,20 @@ static ssize_t edid_write(struct file *file, const char __user *ubuf, struct seq_file *m = file->private_data; struct drm_connector *connector = m->private; char *buf; - struct edid *edid; int ret;

buf = memdup_user(ubuf, len); if (IS_ERR(buf)) return PTR_ERR(buf);

- edid = (struct edid *) buf; - - 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) - ret = -EINVAL; - else { - connector->override_edid = false; - ret = drm_connector_update_edid_property(connector, edid); - if (!ret) - connector->override_edid = true; - } + if (len == 5 && !strncmp(buf, "reset", 5)) + ret = drm_edid_override_reset(connector); + else + ret = drm_edid_override_set(connector, buf, len);

kfree(buf);

- return (ret) ? ret : len; + return ret ? ret : len; }

/* diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index e360e1a269f4..c3f0f0a5a8a9 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -2161,6 +2161,32 @@ static struct edid *drm_get_override_edid(struct drm_connector *connector, return IS_ERR(override) ? NULL : override; }

+/* For debugfs edid_override implementation */ +int drm_edid_override_set(struct drm_connector *connector, const void *edid, + size_t size) +{ + int ret; + + if (size < EDID_LENGTH || edid_size(edid) > size) + return -EINVAL; + + connector->override_edid = false; + + ret = drm_connector_update_edid_property(connector, edid); + if (!ret) + connector->override_edid = true; + + return ret; +} + +/* For debugfs edid_override implementation */ +int drm_edid_override_reset(struct drm_connector *connector) +{ + connector->override_edid = false; + + return drm_connector_update_edid_property(connector, NULL); +} + /** * drm_add_override_edid_modes - add modes from override/firmware EDID * @connector: connector we're probing diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h index 94b422b55cc1..a1705d6b3fba 100644 --- a/include/drm/drm_connector.h +++ b/include/drm/drm_connector.h @@ -1527,7 +1527,11 @@ struct drm_connector { struct drm_cmdline_mode cmdline_mode; /** @force: a DRM_FORCE_<foo> state for forced mode sets */ enum drm_connector_force force; - /** @override_edid: has the EDID been overwritten through debugfs for testing? */ + /** + * @override_edid: has the EDID been overwritten through debugfs for + * testing? Do not modify outside of drm_edid_override_set() and + * drm_edid_override_reset(). + */ bool override_edid; /** @epoch_counter: used to detect any other changes in connector, besides status */ u64 epoch_counter;

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@intel.com --- 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);

Add a helper function to be used as the "default" .get_modes() hook. This also works as an example of what the driver .get_modes() hooks are supposed to do regarding the new drm_edid_read*() and drm_edid_connector_update() calls.

Signed-off-by: Jani Nikula jani.nikula@intel.com Reviewed-by: Ville Syrjälä ville.syrjala@linux.intel.com --- drivers/gpu/drm/drm_probe_helper.c | 34 ++++++++++++++++++++++++++++++ include/drm/drm_probe_helper.h | 1 + 2 files changed, 35 insertions(+)

diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c index a8d26b29bfa0..bb427c5a4f1f 100644 --- a/drivers/gpu/drm/drm_probe_helper.c +++ b/drivers/gpu/drm/drm_probe_helper.c @@ -1049,3 +1049,37 @@ int drm_connector_helper_get_modes_from_ddc(struct drm_connector *connector) return count; } EXPORT_SYMBOL(drm_connector_helper_get_modes_from_ddc); + +/** + * drm_connector_helper_get_modes - Read EDID and update connector. + * @connector: The connector + * + * Read the EDID using drm_edid_read() (which requires that connector->ddc is + * set), and update the connector using the EDID. + * + * This can be used as the "default" connector helper .get_modes() hook if the + * driver does not need any special processing. This is sets the example what + * custom .get_modes() hooks should do regarding EDID read and connector update. + * + * Returns: Number of modes. + */ +int drm_connector_helper_get_modes(struct drm_connector *connector) +{ + const struct drm_edid *drm_edid; + int count; + + drm_edid = drm_edid_read(connector); + + /* + * Unconditionally update the connector. If the EDID was read + * successfully, fill in the connector information derived from the + * EDID. Otherwise, if the EDID is NULL, clear the connector + * information. + */ + count = drm_edid_connector_update(connector, drm_edid); + + drm_edid_free(drm_edid); + + return count; +} +EXPORT_SYMBOL(drm_connector_helper_get_modes); diff --git a/include/drm/drm_probe_helper.h b/include/drm/drm_probe_helper.h index c80cab7a53b7..8075e02aa865 100644 --- a/include/drm/drm_probe_helper.h +++ b/include/drm/drm_probe_helper.h @@ -27,5 +27,6 @@ void drm_kms_helper_poll_enable(struct drm_device *dev); bool drm_kms_helper_is_poll_worker(void);

int drm_connector_helper_get_modes_from_ddc(struct drm_connector *connector); +int drm_connector_helper_get_modes(struct drm_connector *connector);

#endif

Convert all the connectors that use cached connector edid and detect_edid to drm_edid.

v2: Don't leak opregion fallback EDID (Ville)

Signed-off-by: Jani Nikula jani.nikula@intel.com --- .../gpu/drm/i915/display/intel_connector.c | 4 +- .../drm/i915/display/intel_display_types.h | 4 +- drivers/gpu/drm/i915/display/intel_dp.c | 77 +++++++++++-------- drivers/gpu/drm/i915/display/intel_hdmi.c | 26 ++++--- drivers/gpu/drm/i915/display/intel_lvds.c | 37 +++++---- 5 files changed, 81 insertions(+), 67 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_connector.c b/drivers/gpu/drm/i915/display/intel_connector.c index 1dcc268927a2..d83b2a64f618 100644 --- a/drivers/gpu/drm/i915/display/intel_connector.c +++ b/drivers/gpu/drm/i915/display/intel_connector.c @@ -95,12 +95,12 @@ void intel_connector_destroy(struct drm_connector *connector) { struct intel_connector *intel_connector = to_intel_connector(connector);

- kfree(intel_connector->detect_edid); + drm_edid_free(intel_connector->detect_edid);

intel_hdcp_cleanup(intel_connector);

if (!IS_ERR_OR_NULL(intel_connector->edid)) - kfree(intel_connector->edid); + drm_edid_free(intel_connector->edid);

intel_panel_fini(intel_connector);

diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h index 0da9b208d56e..d476df0ac9df 100644 --- a/drivers/gpu/drm/i915/display/intel_display_types.h +++ b/drivers/gpu/drm/i915/display/intel_display_types.h @@ -592,8 +592,8 @@ struct intel_connector { struct intel_panel panel;

/* Cached EDID for eDP and LVDS. May hold ERR_PTR for invalid EDID. */ - struct edid *edid; - struct edid *detect_edid; + const struct drm_edid *edid; + const struct drm_edid *detect_edid;

/* Number of times hotplug detection was tried after an HPD interrupt */ int hotplug_retries; diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c index 32292c0be2bd..4ee35317cf2a 100644 --- a/drivers/gpu/drm/i915/display/intel_dp.c +++ b/drivers/gpu/drm/i915/display/intel_dp.c @@ -3577,12 +3577,11 @@ static u8 intel_dp_autotest_edid(struct intel_dp *intel_dp) intel_dp->aux.i2c_defer_count); intel_dp->compliance.test_data.edid = INTEL_DP_RESOLUTION_FAILSAFE; } else { - struct edid *block = intel_connector->detect_edid; + /* FIXME: Get rid of drm_edid_raw() */ + const struct edid *block = drm_edid_raw(intel_connector->detect_edid);

- /* We have to write the checksum - * of the last block read - */ - block += intel_connector->detect_edid->extensions; + /* We have to write the checksum of the last block read */ + block += block->extensions;

if (drm_dp_dpcd_writeb(&intel_dp->aux, DP_TEST_EDID_CHECKSUM, block->checksum) <= 0) @@ -4461,7 +4460,7 @@ bool intel_digital_port_connected(struct intel_encoder *encoder) return is_connected; }

-static struct edid * +static const struct drm_edid * intel_dp_get_edid(struct intel_dp *intel_dp) { struct intel_connector *intel_connector = intel_dp->attached_connector; @@ -4472,18 +4471,22 @@ intel_dp_get_edid(struct intel_dp *intel_dp) if (IS_ERR(intel_connector->edid)) return NULL;

- return drm_edid_duplicate(intel_connector->edid); + return drm_edid_dup(intel_connector->edid); } else - return drm_get_edid(&intel_connector->base, - &intel_dp->aux.ddc); + return drm_edid_read_ddc(&intel_connector->base, + &intel_dp->aux.ddc); }

static void intel_dp_update_dfp(struct intel_dp *intel_dp, - const struct edid *edid) + const struct drm_edid *drm_edid) { struct drm_i915_private *i915 = dp_to_i915(intel_dp); struct intel_connector *connector = intel_dp->attached_connector; + const struct edid *edid; + + /* FIXME: Get rid of drm_edid_raw() */ + edid = drm_edid_raw(drm_edid);

intel_dp->dfp.max_bpc = drm_dp_downstream_max_bpc(intel_dp->dpcd, @@ -4583,21 +4586,24 @@ intel_dp_set_edid(struct intel_dp *intel_dp) { struct drm_i915_private *i915 = dp_to_i915(intel_dp); struct intel_connector *connector = intel_dp->attached_connector; - struct edid *edid; + const struct drm_edid *drm_edid; + const struct edid *edid; bool vrr_capable;

intel_dp_unset_edid(intel_dp); - edid = intel_dp_get_edid(intel_dp); - connector->detect_edid = edid; + drm_edid = intel_dp_get_edid(intel_dp); + connector->detect_edid = drm_edid;

vrr_capable = intel_vrr_is_capable(connector); drm_dbg_kms(&i915->drm, "[CONNECTOR:%d:%s] VRR capable: %s\n", connector->base.base.id, connector->base.name, str_yes_no(vrr_capable)); drm_connector_set_vrr_capable_property(&connector->base, vrr_capable);

- intel_dp_update_dfp(intel_dp, edid); + intel_dp_update_dfp(intel_dp, drm_edid); intel_dp_update_420(intel_dp);

+ /* FIXME: Get rid of drm_edid_raw() */ + edid = drm_edid_raw(drm_edid); if (edid && edid->input & DRM_EDID_INPUT_DIGITAL) { intel_dp->has_hdmi_sink = drm_detect_hdmi_monitor(edid); intel_dp->has_audio = drm_detect_monitor_audio(edid); @@ -4612,7 +4618,7 @@ intel_dp_unset_edid(struct intel_dp *intel_dp) struct intel_connector *connector = intel_dp->attached_connector;

drm_dp_cec_unset_edid(&intel_dp->aux); - kfree(connector->detect_edid); + drm_edid_free(connector->detect_edid); connector->detect_edid = NULL;

intel_dp->has_hdmi_sink = false; @@ -4776,12 +4782,11 @@ intel_dp_force(struct drm_connector *connector) static int intel_dp_get_modes(struct drm_connector *connector) { struct intel_connector *intel_connector = to_intel_connector(connector); - struct edid *edid; + const struct drm_edid *drm_edid; int num_modes = 0;

- edid = intel_connector->detect_edid; - if (edid) - num_modes = intel_connector_update_modes(connector, edid); + drm_edid = intel_connector->detect_edid; + num_modes = drm_edid_connector_update(connector, drm_edid);

/* Also add fixed mode, which may or may not be present in EDID */ if (intel_dp_is_edp(intel_attached_dp(intel_connector))) @@ -4790,7 +4795,7 @@ static int intel_dp_get_modes(struct drm_connector *connector) if (num_modes) return num_modes;

- if (!edid) { + if (!drm_edid) { struct intel_dp *intel_dp = intel_attached_dp(intel_connector); struct drm_display_mode *mode;

@@ -5198,7 +5203,7 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp, struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base; bool has_dpcd; enum pipe pipe = INVALID_PIPE; - struct edid *edid; + const struct drm_edid *drm_edid;

if (!intel_dp_is_edp(intel_dp)) return true; @@ -5231,29 +5236,33 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp, }

mutex_lock(&dev->mode_config.mutex); - edid = drm_get_edid(connector, &intel_dp->aux.ddc); - if (!edid) { + drm_edid = drm_edid_read_ddc(connector, &intel_dp->aux.ddc); + if (!drm_edid) { + const struct edid *edid; + /* Fallback to EDID from ACPI OpRegion, if any */ + /* FIXME: Make intel_opregion_get_edid() return drm_edid */ edid = intel_opregion_get_edid(intel_connector); - if (edid) + if (edid) { + drm_edid = drm_edid_alloc(edid, (edid->extensions + 1) * EDID_LENGTH); drm_dbg_kms(&dev_priv->drm, "[CONNECTOR:%d:%s] Using OpRegion EDID\n", connector->base.id, connector->name); - } - if (edid) { - if (drm_add_edid_modes(connector, edid)) { - drm_connector_update_edid_property(connector, edid); - } else { kfree(edid); - edid = ERR_PTR(-EINVAL); + } + } + if (drm_edid) { + if (!drm_edid_connector_update(connector, drm_edid)) { + drm_edid_free(drm_edid); + drm_edid = ERR_PTR(-EINVAL); } } else { - edid = ERR_PTR(-ENOENT); + drm_edid = ERR_PTR(-ENOENT); } - intel_connector->edid = edid; + intel_connector->edid = drm_edid;

- intel_bios_init_panel(dev_priv, &intel_connector->panel, - encoder->devdata, IS_ERR(edid) ? NULL : edid); + intel_bios_init_panel(dev_priv, &intel_connector->panel, encoder->devdata, + IS_ERR_OR_NULL(drm_edid) ? NULL : drm_edid_raw(drm_edid));

intel_panel_add_edid_fixed_modes(intel_connector, intel_connector->panel.vbt.drrs_type != DRRS_TYPE_NONE, diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c index 1ae09431f53a..db33a0ccd7ee 100644 --- a/drivers/gpu/drm/i915/display/intel_hdmi.c +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c @@ -2340,7 +2340,7 @@ intel_hdmi_unset_edid(struct drm_connector *connector) intel_hdmi->dp_dual_mode.type = DRM_DP_DUAL_MODE_NONE; intel_hdmi->dp_dual_mode.max_tmds_clock = 0;

- kfree(to_intel_connector(connector)->detect_edid); + drm_edid_free(to_intel_connector(connector)->detect_edid); to_intel_connector(connector)->detect_edid = NULL; }

@@ -2407,7 +2407,8 @@ intel_hdmi_set_edid(struct drm_connector *connector) struct drm_i915_private *dev_priv = to_i915(connector->dev); struct intel_hdmi *intel_hdmi = intel_attached_hdmi(to_intel_connector(connector)); intel_wakeref_t wakeref; - struct edid *edid; + const struct drm_edid *drm_edid; + const struct edid *edid; bool connected = false; struct i2c_adapter *i2c;

@@ -2415,21 +2416,24 @@ intel_hdmi_set_edid(struct drm_connector *connector)

i2c = intel_gmbus_get_adapter(dev_priv, intel_hdmi->ddc_bus);

- edid = drm_get_edid(connector, i2c); + drm_edid = drm_edid_read_ddc(connector, i2c);

- if (!edid && !intel_gmbus_is_forced_bit(i2c)) { + if (!drm_edid && !intel_gmbus_is_forced_bit(i2c)) { drm_dbg_kms(&dev_priv->drm, "HDMI GMBUS EDID read failed, retry using GPIO bit-banging\n"); intel_gmbus_force_bit(i2c, true); - edid = drm_get_edid(connector, i2c); + drm_edid = drm_edid_read_ddc(connector, i2c); intel_gmbus_force_bit(i2c, false); }

- intel_hdmi_dp_dual_mode_detect(connector, edid != NULL); + intel_hdmi_dp_dual_mode_detect(connector, drm_edid != NULL);

intel_display_power_put(dev_priv, POWER_DOMAIN_GMBUS, wakeref);

- to_intel_connector(connector)->detect_edid = edid; + to_intel_connector(connector)->detect_edid = drm_edid; + + /* FIXME: Get rid of drm_edid_raw() */ + edid = drm_edid_raw(drm_edid); if (edid && edid->input & DRM_EDID_INPUT_DIGITAL) { intel_hdmi->has_audio = drm_detect_monitor_audio(edid); intel_hdmi->has_hdmi_sink = drm_detect_hdmi_monitor(edid); @@ -2501,13 +2505,11 @@ intel_hdmi_force(struct drm_connector *connector)

static int intel_hdmi_get_modes(struct drm_connector *connector) { - struct edid *edid; + const struct drm_edid *drm_edid;

- edid = to_intel_connector(connector)->detect_edid; - if (edid == NULL) - return 0; + drm_edid = to_intel_connector(connector)->detect_edid;

- return intel_connector_update_modes(connector, edid); + return drm_edid_connector_update(connector, drm_edid); }

static struct i2c_adapter * diff --git a/drivers/gpu/drm/i915/display/intel_lvds.c b/drivers/gpu/drm/i915/display/intel_lvds.c index 730480ac3300..741152fd90eb 100644 --- a/drivers/gpu/drm/i915/display/intel_lvds.c +++ b/drivers/gpu/drm/i915/display/intel_lvds.c @@ -479,7 +479,7 @@ static int intel_lvds_get_modes(struct drm_connector *connector)

/* use cached edid if we have one */ if (!IS_ERR_OR_NULL(intel_connector->edid)) - return drm_add_edid_modes(connector, intel_connector->edid); + return drm_edid_connector_update(connector, intel_connector->edid);

return intel_panel_get_modes(intel_connector); } @@ -829,7 +829,7 @@ void intel_lvds_init(struct drm_i915_private *dev_priv) struct intel_connector *intel_connector; struct drm_connector *connector; struct drm_encoder *encoder; - struct edid *edid; + const struct drm_edid *drm_edid; i915_reg_t lvds_reg; u32 lvds; u8 pin; @@ -948,27 +948,30 @@ void intel_lvds_init(struct drm_i915_private *dev_priv) * preferred mode is the right one. */ mutex_lock(&dev->mode_config.mutex); - if (vga_switcheroo_handler_flags() & VGA_SWITCHEROO_CAN_SWITCH_DDC) + if (vga_switcheroo_handler_flags() & VGA_SWITCHEROO_CAN_SWITCH_DDC) { + const struct edid *edid; + + /* FIXME: Make drm_get_edid_switcheroo() return drm_edid */ edid = drm_get_edid_switcheroo(connector, - intel_gmbus_get_adapter(dev_priv, pin)); - else - edid = drm_get_edid(connector, - intel_gmbus_get_adapter(dev_priv, pin)); - if (edid) { - if (drm_add_edid_modes(connector, edid)) { - drm_connector_update_edid_property(connector, - edid); - } else { - kfree(edid); - edid = ERR_PTR(-EINVAL); + intel_gmbus_get_adapter(dev_priv, pin)); + if (edid) + drm_edid = drm_edid_alloc(edid, (edid->extensions + 1) * EDID_LENGTH); + } else { + drm_edid = drm_edid_read_ddc(connector, + intel_gmbus_get_adapter(dev_priv, pin)); + } + if (drm_edid) { + if (!drm_edid_connector_update(connector, drm_edid)) { + drm_edid_free(drm_edid); + drm_edid = ERR_PTR(-EINVAL); } } else { - edid = ERR_PTR(-ENOENT); + drm_edid = ERR_PTR(-ENOENT); } - intel_connector->edid = edid; + intel_connector->edid = drm_edid;

intel_bios_init_panel(dev_priv, &intel_connector->panel, NULL, - IS_ERR(edid) ? NULL : edid); + IS_ERR_OR_NULL(drm_edid) ? NULL : drm_edid_raw(drm_edid));

/* Try EDID first */ intel_panel_add_edid_fixed_modes(intel_connector,

Rewrite edid_filter_invalid_blocks() to filter invalid blocks in-place. The main motivation is to not rely on passed in information on invalid block count or the allocation size, which will be helpful in follow-up work on HF-EEODB.

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 | 43 ++++++++++++++++++++------------------ 1 file changed, 23 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 1c761e12820e..a80ea0aa7b32 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -2020,33 +2020,37 @@ bool drm_edid_is_valid(struct edid *edid) } EXPORT_SYMBOL(drm_edid_is_valid);

-static struct edid *edid_filter_invalid_blocks(const struct edid *edid, - int invalid_blocks, +static struct edid *edid_filter_invalid_blocks(struct edid *edid, size_t *alloc_size) { - struct edid *new, *dest_block; - int valid_extensions = edid->extensions - invalid_blocks; - int i; + struct edid *new; + int i, valid_blocks = 0;

- *alloc_size = edid_size_by_blocks(valid_extensions + 1); + for (i = 0; i < edid_block_count(edid); i++) { + const void *src_block = edid_block_data(edid, i);

- new = kmalloc(*alloc_size, GFP_KERNEL); - if (!new) - goto out; + if (edid_block_valid(src_block, i == 0)) { + void *dst_block = (void *)edid_block_data(edid, valid_blocks);

- dest_block = new; - for (i = 0; i < edid_block_count(edid); i++) { - const void *block = edid_block_data(edid, i); + memmove(dst_block, src_block, EDID_LENGTH); + valid_blocks++; + } + }

- if (edid_block_valid(block, i == 0)) - memcpy(dest_block++, block, EDID_LENGTH); + /* We already trusted the base block to be valid here... */ + if (WARN_ON(!valid_blocks)) { + kfree(edid); + return NULL; }

- new->extensions = valid_extensions; - new->checksum = edid_block_compute_checksum(new); + edid->extensions = valid_blocks - 1; + edid->checksum = edid_block_compute_checksum(edid);

-out: - kfree(edid); + *alloc_size = edid_size_by_blocks(valid_blocks); + + new = krealloc(edid, *alloc_size, GFP_KERNEL); + if (!new) + kfree(edid);

return new; } @@ -2316,8 +2320,7 @@ static struct edid *_drm_do_get_edid(struct drm_connector *connector, if (invalid_blocks) { connector_bad_edid(connector, edid, edid_block_count(edid));

- edid = edid_filter_invalid_blocks(edid, invalid_blocks, - &alloc_size); + edid = edid_filter_invalid_blocks(edid, &alloc_size); }

ok:

Take the HF-EEODB extension count override into account.

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 | 13 +++++++++++++ 1 file changed, 13 insertions(+)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index fa3a3e294560..bbc25e3b7220 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -1629,6 +1629,19 @@ static int drm_edid_block_count(const struct drm_edid *drm_edid) /* Starting point */ num_blocks = edid_block_count(drm_edid->edid);

+ /* HF-EEODB override */ + if (drm_edid->size >= edid_size_by_blocks(2)) { + int eeodb; + + /* + * Note: HF-EEODB may specify a smaller extension count than the + * regular one. Unlike in buffer allocation, here we can use it. + */ + eeodb = edid_hfeeodb_block_count(drm_edid->edid); + if (eeodb) + num_blocks = eeodb; + } + /* Limit by allocated size */ num_blocks = min(num_blocks, (int)drm_edid->size / EDID_LENGTH);