[PATCH v1 0/3] Send a hotplug when edid changes

[PATCH v1 0/3] Send a hotplug when edid changes

[Stanislav Lisovskiy]

This series introduce to drm a way to determine if something else
except connection_status had changed during probing, which
can be used by other drivers as well. Another i915 specific part
uses this approach to determine if edid had changed without
changing the connection status and send a hotplug event.

Stanislav Lisovskiy (3):
  drm: Add helper to compare edids.
  drm: Introduce change counter to drm_connector
  drm/i915: Send hotplug event if edid had changed.

 drivers/gpu/drm/drm_connector.c              |  1 +
 drivers/gpu/drm/drm_edid.c                   | 26 ++++++++++++++++++
 drivers/gpu/drm/drm_probe_helper.c           | 29 ++++++++++++++++++--
 drivers/gpu/drm/i915/display/intel_dp.c      | 16 ++++++++++-
 drivers/gpu/drm/i915/display/intel_hdmi.c    | 16 +++++++++--
 drivers/gpu/drm/i915/display/intel_hotplug.c | 20 ++++++++++----
 include/drm/drm_connector.h                  |  2 ++
 include/drm/drm_edid.h                       |  2 ++
 8 files changed, 101 insertions(+), 11 deletions(-)

-- 
2.17.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v1 1/3] drm: Add helper to compare edids.

[Stanislav Lisovskiy]

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

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

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 9d8f2b952004..5f0f5197924d 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -1361,6 +1361,32 @@ static bool drm_edid_is_zero(const u8 *in_edid, int length)
 	return true;
 }
 
+
+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;
+}
+
+
 /**
  * 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..8d28a6001287 100644
--- a/include/drm/drm_edid.h
+++ b/include/drm/drm_edid.h
@@ -354,6 +354,8 @@ drm_load_edid_firmware(struct drm_connector *connector)
 }
 #endif
 
+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,
-- 
2.17.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v1 2/3] drm: Introduce change counter to drm_connector

[Stanislav Lisovskiy]

This counter will be used by drm_helper_probe_detect caller to determine
if something else had changed except connection status,
like for example edid. Hardware specific drivers are responsible
for updating this counter when some change is detected to notify
the drm part, which can trigger for example hotplug event.

Currently there is no way to propagate that to a calling layer,
as we send only connection_status update, however as we see with
edid the changes can be broader.

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    |  1 +
 drivers/gpu/drm/drm_probe_helper.c | 29 +++++++++++++++++++++++++++--
 include/drm/drm_connector.h        |  2 ++
 3 files changed, 30 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index 3ccdcf3dfcde..531983707d7f 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -245,6 +245,7 @@ int drm_connector_init(struct drm_device *dev,
 	INIT_LIST_HEAD(&connector->modes);
 	mutex_init(&connector->mutex);
 	connector->edid_blob_ptr = NULL;
+	connector->change_counter = 0;
 	connector->tile_blob_ptr = NULL;
 	connector->status = connector_status_unknown;
 	connector->display_info.panel_orientation =
diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c
index ef2c468205a2..e8f41fa336c3 100644
--- a/drivers/gpu/drm/drm_probe_helper.c
+++ b/drivers/gpu/drm/drm_probe_helper.c
@@ -776,6 +776,7 @@ bool drm_helper_hpd_irq_event(struct drm_device *dev)
 	struct drm_connector_list_iter conn_iter;
 	enum drm_connector_status old_status;
 	bool changed = false;
+	uint64_t old_change_counter;
 
 	if (!dev->mode_config.poll_enabled)
 		return false;
@@ -789,20 +790,44 @@ bool drm_helper_hpd_irq_event(struct drm_device *dev)
 
 		old_status = connector->status;
 
+		old_change_counter = connector->change_counter;
+
+		DRM_DEBUG_KMS("[CONNECTOR:%d:%s] Old change counter %llu\n", connector->base.id,
+			      connector->name,
+			      old_change_counter);
+
 		connector->status = drm_helper_probe_detect(connector, NULL, false);
 		DRM_DEBUG_KMS("[CONNECTOR:%d:%s] status updated from %s to %s\n",
 			      connector->base.id,
 			      connector->name,
 			      drm_get_connector_status_name(old_status),
 			      drm_get_connector_status_name(connector->status));
-		if (old_status != connector->status)
+
+		DRM_DEBUG_KMS("[CONNECTOR:%d:%s] New change counter %llu\n",
+			      connector->base.id,
+			      connector->name,
+			      connector->change_counter);
+
+		if (old_status != connector->status) {
 			changed = true;
+		}
+
+		/* Check changing of edid when a connector status still remains
+		 * as "connector_status_connected".
+		 */
+		if (connector->status == connector_status_connected &&
+		    old_status == connector_status_connected &&
+		    old_change_counter != connector->change_counter) {
+			changed = true;
+		}
 	}
 	drm_connector_list_iter_end(&conn_iter);
 	mutex_unlock(&dev->mode_config.mutex);
 
-	if (changed)
+	if (changed) {
 		drm_kms_helper_hotplug_event(dev);
+		DRM_DEBUG_KMS("Sent hotplug event\n");
+	}
 
 	return changed;
 }
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index c6f8486d8b8f..feaa4eb673f6 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -1155,6 +1155,8 @@ struct drm_connector {
 	/** @override_edid: has the EDID been overwritten through debugfs for testing? */
 	bool override_edid;
 
+	uint64_t change_counter;
+
 #define DRM_CONNECTOR_MAX_ENCODER 3
 	/**
 	 * @encoder_ids: Valid encoders for this connector. Please only use
-- 
2.17.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v1 0/3] Send a hotplug when edid changes

[Daniel Vetter]

On Thu, Jun 27, 2019 at 10:00:14AM +0300, Stanislav Lisovskiy wrote:
> This series introduce to drm a way to determine if something else
> except connection_status had changed during probing, which
> can be used by other drivers as well. Another i915 specific part
> uses this approach to determine if edid had changed without
> changing the connection status and send a hotplug event.
> 
> Stanislav Lisovskiy (3):
>   drm: Add helper to compare edids.
>   drm: Introduce change counter to drm_connector
>   drm/i915: Send hotplug event if edid had changed.

There was a huge discussion a while back on what this all should look
like, with Pekka and Paul and others. This here seems to fall quite a bit
short on that.

https://lists.freedesktop.org/archives/dri-devel/2019-May/217588.html

Also please cc all the people involved in that previous discussion.

Wrt the patches: Documentation seems to be awol at least, that definitely
needs to be fixed. Maybe fix that, resend, and then use that to move the
overall discussion forward with everyone?
-Daniel

> 
>  drivers/gpu/drm/drm_connector.c              |  1 +
>  drivers/gpu/drm/drm_edid.c                   | 26 ++++++++++++++++++
>  drivers/gpu/drm/drm_probe_helper.c           | 29 ++++++++++++++++++--
>  drivers/gpu/drm/i915/display/intel_dp.c      | 16 ++++++++++-
>  drivers/gpu/drm/i915/display/intel_hdmi.c    | 16 +++++++++--
>  drivers/gpu/drm/i915/display/intel_hotplug.c | 20 ++++++++++----
>  include/drm/drm_connector.h                  |  2 ++
>  include/drm/drm_edid.h                       |  2 ++
>  8 files changed, 101 insertions(+), 11 deletions(-)
> 
> -- 
> 2.17.1
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v1 0/3] Send a hotplug when edid changes

[Lisovskiy, Stanislav]

On Thu, 2019-06-27 at 13:29 +0200, Daniel Vetter wrote:
> On Thu, Jun 27, 2019 at 10:00:14AM +0300, Stanislav Lisovskiy wrote:
> > This series introduce to drm a way to determine if something else
> > except connection_status had changed during probing, which
> > can be used by other drivers as well. Another i915 specific part
> > uses this approach to determine if edid had changed without
> > changing the connection status and send a hotplug event.
> > 
> > Stanislav Lisovskiy (3):
> >   drm: Add helper to compare edids.
> >   drm: Introduce change counter to drm_connector
> >   drm/i915: Send hotplug event if edid had changed.
> 
> There was a huge discussion a while back on what this all should look
> like, with Pekka and Paul and others. This here seems to fall quite a
> bit
> short on that.
> 
> https://lists.freedesktop.org/archives/dri-devel/2019-May/217588.html
> 
> Also please cc all the people involved in that previous discussion.
> 
> Wrt the patches: Documentation seems to be awol at least, that
> definitely
> needs to be fixed. Maybe fix that, resend, and then use that to move
> the
> overall discussion forward with everyone?
> -Daniel

Yes, I read some part of it. Also used your idea about the counter as a
way to propagate connector updates, such as edid change and etc, from
here:

https://lists.freedesktop.org/archives/dri-devel/2019-April/214572.html

As currently we analyze only connection status and nothing else.
Gwang-Gyeong is now doing some other task, so his patches were
transfered to me - however some changes had to be done to make it
work(used kms_chamelium edid_change tests to check).

I will include all people into discussion and documentation seems to be
a good idea as well.

> 
> > 
> >  drivers/gpu/drm/drm_connector.c              |  1 +
> >  drivers/gpu/drm/drm_edid.c                   | 26
> > ++++++++++++++++++
> >  drivers/gpu/drm/drm_probe_helper.c           | 29
> > ++++++++++++++++++--
> >  drivers/gpu/drm/i915/display/intel_dp.c      | 16 ++++++++++-
> >  drivers/gpu/drm/i915/display/intel_hdmi.c    | 16 +++++++++--
> >  drivers/gpu/drm/i915/display/intel_hotplug.c | 20 ++++++++++----
> >  include/drm/drm_connector.h                  |  2 ++
> >  include/drm/drm_edid.h                       |  2 ++
> >  8 files changed, 101 insertions(+), 11 deletions(-)
> > 
> > -- 
> > 2.17.1
> > 
> 
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx