Many drivers would benefit from using drm helper to compare edid, rather than bothering with own implementation.

v2: Added documentation for this function.

Signed-off-by: Stanislav Lisovskiy stanislav.lisovskiy@intel.com --- drivers/gpu/drm/drm_edid.c | 33 +++++++++++++++++++++++++++++++++ include/drm/drm_edid.h | 9 +++++++++ 2 files changed, 42 insertions(+)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 82a4ceed3fcf..6cd086ea6253 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -1362,6 +1362,39 @@ static bool drm_edid_is_zero(const u8 *in_edid, int length) return true; }

+/** + * drm_edid_are_equal - compare two edid blobs. + * @edid1: pointer to first blob + * @edid2: pointer to second blob + * This helper can be used during probing to determine if + * edid had changed. + */ +bool drm_edid_are_equal(struct edid *edid1, struct edid *edid2) +{ + int edid1_len, edid2_len; + bool edid1_present = edid1 != NULL; + bool edid2_present = edid2 != NULL; + + if (edid1_present != edid2_present) + return false; + + if (edid1) { + + edid1_len = EDID_LENGTH * (1 + edid1->extensions); + edid2_len = EDID_LENGTH * (1 + edid2->extensions); + + if (edid1_len != edid2_len) + return false; + + if (memcmp(edid1, edid2, edid1_len)) + return false; + } + + return true; +} +EXPORT_SYMBOL(drm_edid_are_equal); + + /** * drm_edid_block_valid - Sanity check the EDID block (base or extension) * @raw_edid: pointer to raw EDID block diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h index b9719418c3d2..716964f63312 100644 --- a/include/drm/drm_edid.h +++ b/include/drm/drm_edid.h @@ -354,6 +354,15 @@ drm_load_edid_firmware(struct drm_connector *connector) } #endif

+/** + * drm_edid_are_equal - compare two edid blobs. + * @edid1: pointer to first blob + * @edid2: pointer to second blob + * This helper can be used during probing to determine if + * edid had changed. + */ +bool drm_edid_are_equal(struct edid *edid1, struct edid *edid2); + int drm_hdmi_avi_infoframe_from_display_mode(struct hdmi_avi_infoframe *frame, struct drm_connector *connector,

Added edid checking to dp and hdmi edid setting functions, which are called from detect hooks. The result currently is propagated to calling layer using drm_connector->change_counter(proposed by Daniel Vetter). drm_helper_hpd_irq_event and intel_encoder_hotplug are currently both responsible for checking if this counter or connection status is changed.

There are conflicting parts in drm and i915 which attempt to do the same job - drm_helper_hpd_irq_event attempts to check connector status changes and then call hotplug, just as i915_hotplug_work_func, which calls encoder->hotplug hook which in turn calls generic intel_encoder_hotplug function which attempts to probe if output has changed. Looks like both needs to be changed, so added edid checking also to intel_encoder_hotplug function which is called both for hdmi and dp.

v2: Renamed change counter to epoch counter. Fixed type name.

v3: Fixed rebase conflict

v4: Remove duplicate drm_edid_equal checks from hdmi and dp, lets use only once edid property is getting updated and increment epoch counter from there. Also lets now call drm_connector_update_edid_property right after we get edid always to make sure there is a unified way to handle edid change, without having to change tons of source code as currently drm_connector_update_edid_property is called only in certain cases like reprobing and not right after edid is actually updated.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105540 Signed-off-by: Stanislav Lisovskiy stanislav.lisovskiy@intel.com --- drivers/gpu/drm/drm_connector.c | 16 +++++++++++++++ drivers/gpu/drm/drm_edid.c | 3 +++ drivers/gpu/drm/drm_probe_helper.c | 2 +- drivers/gpu/drm/i915/display/intel_hotplug.c | 21 +++++++++++++++----- 4 files changed, 36 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c index 18b1ad2a4eee..828e57a740d7 100644 --- a/drivers/gpu/drm/drm_connector.c +++ b/drivers/gpu/drm/drm_connector.c @@ -1867,6 +1867,7 @@ int drm_connector_update_edid_property(struct drm_connector *connector, 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) @@ -1888,6 +1889,20 @@ int drm_connector_update_edid_property(struct drm_connector *connector, else drm_reset_display_info(connector);

+ if (connector->edid_blob_ptr) { + old_edid = (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); @@ -1898,6 +1913,7 @@ int drm_connector_update_edid_property(struct drm_connector *connector, edid, &connector->base, dev->mode_config.edid_property); + if (ret) return ret; return drm_connector_set_tile_property(connector); diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 6cd086ea6253..7a327aec00d7 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -1803,6 +1803,9 @@ struct edid *drm_get_edid(struct drm_connector *connector, edid = drm_do_get_edid(connector, drm_do_probe_ddc_edid, adapter); if (edid) drm_get_displayid(connector, edid); + + drm_connector_update_edid_property(connector, edid); + return edid; } EXPORT_SYMBOL(drm_get_edid); diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c index 5131ae56e676..d896d6b5d3b4 100644 --- a/drivers/gpu/drm/drm_probe_helper.c +++ b/drivers/gpu/drm/drm_probe_helper.c @@ -813,7 +813,7 @@ bool drm_helper_hpd_irq_event(struct drm_device *dev) changed = true; }

- /* Check changing of edid when a connector status still remains + /* Check changing of epoch counter when a connector status still remains * as "connector_status_connected". */ if (connector->status == connector_status_connected && diff --git a/drivers/gpu/drm/i915/display/intel_hotplug.c b/drivers/gpu/drm/i915/display/intel_hotplug.c index fc29046d48ea..5fc1d54b3e6a 100644 --- a/drivers/gpu/drm/i915/display/intel_hotplug.c +++ b/drivers/gpu/drm/i915/display/intel_hotplug.c @@ -280,23 +280,34 @@ intel_encoder_hotplug(struct intel_encoder *encoder, { struct drm_device *dev = connector->base.dev; enum drm_connector_status old_status; + u64 old_epoch_counter; + bool ret = false;

WARN_ON(!mutex_is_locked(&dev->mode_config.mutex)); old_status = connector->base.status;

+ old_epoch_counter = connector->base.epoch_counter; + connector->base.status = drm_helper_probe_detect(&connector->base, NULL, false);

- if (old_status == connector->base.status) - return INTEL_HOTPLUG_UNCHANGED; + if (old_status != connector->base.status) + ret = true; + + if (old_epoch_counter != connector->base.epoch_counter) + ret = true;

- DRM_DEBUG_KMS("[CONNECTOR:%d:%s] status updated from %s to %s\n", + if (ret) { + DRM_DEBUG_KMS("[CONNECTOR:%d:%s] status updated from %s to %s(change counter %llu)\n", connector->base.base.id, connector->base.name, drm_get_connector_status_name(old_status), - drm_get_connector_status_name(connector->base.status)); + drm_get_connector_status_name(connector->base.status), + connector->base.epoch_counter); + return INTEL_HOTPLUG_CHANGED; + }

- return INTEL_HOTPLUG_CHANGED; + return INTEL_HOTPLUG_UNCHANGED; }

static bool intel_encoder_has_hpd_pulse(struct intel_encoder *encoder)