All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4 0/3] Send a hotplug when edid changes
@ 2019-09-05 10:37 Stanislav Lisovskiy
  2019-09-05 10:37 ` [PATCH v4 1/3] drm: Add helper to compare edids Stanislav Lisovskiy
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Stanislav Lisovskiy @ 2019-09-05 10:37 UTC (permalink / raw)
  To: dri-devel
  Cc: simon.ser, daniel.vetter, intel-gfx, martin.peres,
	Stanislav.Lisovskiy, jani.saarinen

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              | 17 +++++++++
 drivers/gpu/drm/drm_edid.c                   | 36 ++++++++++++++++++++
 drivers/gpu/drm/drm_probe_helper.c           | 29 ++++++++++++++--
 drivers/gpu/drm/i915/display/intel_hotplug.c | 21 +++++++++---
 include/drm/drm_connector.h                  |  3 ++
 include/drm/drm_edid.h                       |  9 +++++
 6 files changed, 108 insertions(+), 7 deletions(-)

-- 
2.17.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v4 1/3] drm: Add helper to compare edids.
  2019-09-05 10:37 [PATCH v4 0/3] Send a hotplug when edid changes Stanislav Lisovskiy
@ 2019-09-05 10:37 ` Stanislav Lisovskiy
  2019-09-05 10:37 ` [PATCH v4 2/3] drm: Introduce change counter to drm_connector Stanislav Lisovskiy
  2019-09-05 10:37 ` [PATCH v4 3/3] drm/i915: Send hotplug event if edid had changed Stanislav Lisovskiy
  2 siblings, 0 replies; 6+ messages in thread
From: Stanislav Lisovskiy @ 2019-09-05 10:37 UTC (permalink / raw)
  To: dri-devel
  Cc: simon.ser, daniel.vetter, intel-gfx, martin.peres,
	Stanislav.Lisovskiy, jani.saarinen

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,
-- 
2.17.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v4 2/3] drm: Introduce change counter to drm_connector
  2019-09-05 10:37 [PATCH v4 0/3] Send a hotplug when edid changes Stanislav Lisovskiy
  2019-09-05 10:37 ` [PATCH v4 1/3] drm: Add helper to compare edids Stanislav Lisovskiy
@ 2019-09-05 10:37 ` Stanislav Lisovskiy
  2019-09-05 11:01   ` Maarten Lankhorst
  2019-09-05 10:37 ` [PATCH v4 3/3] drm/i915: Send hotplug event if edid had changed Stanislav Lisovskiy
  2 siblings, 1 reply; 6+ messages in thread
From: Stanislav Lisovskiy @ 2019-09-05 10:37 UTC (permalink / raw)
  To: dri-devel
  Cc: simon.ser, daniel.vetter, intel-gfx, martin.peres,
	Stanislav.Lisovskiy, jani.saarinen

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.

v2: Added documentation for the new counter. Rename change_counter to
    epoch_counter.

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        |  3 +++
 3 files changed, 31 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index 4c766624b20d..18b1ad2a4eee 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -246,6 +246,7 @@ int drm_connector_init(struct drm_device *dev,
 	INIT_LIST_HEAD(&connector->modes);
 	mutex_init(&connector->mutex);
 	connector->edid_blob_ptr = NULL;
+	connector->epoch_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 351cbc40f0f8..5131ae56e676 100644
--- a/drivers/gpu/drm/drm_probe_helper.c
+++ b/drivers/gpu/drm/drm_probe_helper.c
@@ -777,6 +777,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_epoch_counter;
 
 	if (!dev->mode_config.poll_enabled)
 		return false;
@@ -790,20 +791,44 @@ bool drm_helper_hpd_irq_event(struct drm_device *dev)
 
 		old_status = connector->status;
 
+		old_epoch_counter = connector->epoch_counter;
+
+		DRM_DEBUG_KMS("[CONNECTOR:%d:%s] Old change counter %llu\n", connector->base.id,
+			      connector->name,
+			      old_epoch_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->epoch_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_epoch_counter != connector->epoch_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 681cb590f952..a3cc7d0927d6 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -1288,6 +1288,9 @@ struct drm_connector {
 	/** @override_edid: has the EDID been overwritten through debugfs for testing? */
 	bool override_edid;
 
+	/** @epoch_counter: used to detect any other changes in connector, besides status */
+	uint64_t epoch_counter;
+
 #define DRM_CONNECTOR_MAX_ENCODER 3
 	/**
 	 * @encoder_ids: Valid encoders for this connector. Please only use
-- 
2.17.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v4 3/3] drm/i915: Send hotplug event if edid had changed.
  2019-09-05 10:37 [PATCH v4 0/3] Send a hotplug when edid changes Stanislav Lisovskiy
  2019-09-05 10:37 ` [PATCH v4 1/3] drm: Add helper to compare edids Stanislav Lisovskiy
  2019-09-05 10:37 ` [PATCH v4 2/3] drm: Introduce change counter to drm_connector Stanislav Lisovskiy
@ 2019-09-05 10:37 ` Stanislav Lisovskiy
  2 siblings, 0 replies; 6+ messages in thread
From: Stanislav Lisovskiy @ 2019-09-05 10:37 UTC (permalink / raw)
  To: dri-devel
  Cc: simon.ser, daniel.vetter, intel-gfx, martin.peres,
	Stanislav.Lisovskiy, jani.saarinen

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)
-- 
2.17.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v4 2/3] drm: Introduce change counter to drm_connector
  2019-09-05 10:37 ` [PATCH v4 2/3] drm: Introduce change counter to drm_connector Stanislav Lisovskiy
@ 2019-09-05 11:01   ` Maarten Lankhorst
  2019-09-05 11:21     ` Lisovskiy, Stanislav
  0 siblings, 1 reply; 6+ messages in thread
From: Maarten Lankhorst @ 2019-09-05 11:01 UTC (permalink / raw)
  To: Stanislav Lisovskiy, dri-devel; +Cc: daniel.vetter, intel-gfx, martin.peres

Op 05-09-2019 om 12:37 schreef 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.
>
> v2: Added documentation for the new counter. Rename change_counter to
>     epoch_counter.
>
> 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        |  3 +++
>  3 files changed, 31 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index 4c766624b20d..18b1ad2a4eee 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -246,6 +246,7 @@ int drm_connector_init(struct drm_device *dev,
>  	INIT_LIST_HEAD(&connector->modes);
>  	mutex_init(&connector->mutex);
>  	connector->edid_blob_ptr = NULL;
> +	connector->epoch_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 351cbc40f0f8..5131ae56e676 100644
> --- a/drivers/gpu/drm/drm_probe_helper.c
> +++ b/drivers/gpu/drm/drm_probe_helper.c
> @@ -777,6 +777,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_epoch_counter;
>  
>  	if (!dev->mode_config.poll_enabled)
>  		return false;
> @@ -790,20 +791,44 @@ bool drm_helper_hpd_irq_event(struct drm_device *dev)
>  
>  		old_status = connector->status;
>  
> +		old_epoch_counter = connector->epoch_counter;
> +
> +		DRM_DEBUG_KMS("[CONNECTOR:%d:%s] Old change counter %llu\n", connector->base.id,
> +			      connector->name,
> +			      old_epoch_counter);
> +
>  		connector->status = drm_helper_probe_detect(connector, NULL, false);
Pass a u32 *changed as argument to drm_helper_probe_detect? would require signature change,
>  		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->epoch_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_epoch_counter != connector->epoch_counter) {
> +			changed = true;
> +		}
Could we bump the epoch counter for any event being sent out? Would make more sense.
>  	}
>  	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 681cb590f952..a3cc7d0927d6 100644
> --- a/include/drm/drm_connector.h
> +++ b/include/drm/drm_connector.h
> @@ -1288,6 +1288,9 @@ struct drm_connector {
>  	/** @override_edid: has the EDID been overwritten through debugfs for testing? */
>  	bool override_edid;
>  
> +	/** @epoch_counter: used to detect any other changes in connector, besides status */
> +	uint64_t epoch_counter;
> +
>  #define DRM_CONNECTOR_MAX_ENCODER 3
>  	/**
>  	 * @encoder_ids: Valid encoders for this connector. Please only use


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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v4 2/3] drm: Introduce change counter to drm_connector
  2019-09-05 11:01   ` Maarten Lankhorst
@ 2019-09-05 11:21     ` Lisovskiy, Stanislav
  0 siblings, 0 replies; 6+ messages in thread
From: Lisovskiy, Stanislav @ 2019-09-05 11:21 UTC (permalink / raw)
  To: dri-devel, maarten.lankhorst; +Cc: daniel.vetter, intel-gfx, Peres, Martin

On Thu, 2019-09-05 at 13:01 +0200, Maarten Lankhorst wrote:
> Op 05-09-2019 om 12:37 schreef 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.
> > 
> > v2: Added documentation for the new counter. Rename change_counter
> > to
> >     epoch_counter.
> > 
> > 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        |  3 +++
> >  3 files changed, 31 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_connector.c
> > b/drivers/gpu/drm/drm_connector.c
> > index 4c766624b20d..18b1ad2a4eee 100644
> > --- a/drivers/gpu/drm/drm_connector.c
> > +++ b/drivers/gpu/drm/drm_connector.c
> > @@ -246,6 +246,7 @@ int drm_connector_init(struct drm_device *dev,
> >  	INIT_LIST_HEAD(&connector->modes);
> >  	mutex_init(&connector->mutex);
> >  	connector->edid_blob_ptr = NULL;
> > +	connector->epoch_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 351cbc40f0f8..5131ae56e676 100644
> > --- a/drivers/gpu/drm/drm_probe_helper.c
> > +++ b/drivers/gpu/drm/drm_probe_helper.c
> > @@ -777,6 +777,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_epoch_counter;
> >  
> >  	if (!dev->mode_config.poll_enabled)
> >  		return false;
> > @@ -790,20 +791,44 @@ bool drm_helper_hpd_irq_event(struct
> > drm_device *dev)
> >  
> >  		old_status = connector->status;
> >  
> > +		old_epoch_counter = connector->epoch_counter;
> > +
> > +		DRM_DEBUG_KMS("[CONNECTOR:%d:%s] Old change counter
> > %llu\n", connector->base.id,
> > +			      connector->name,
> > +			      old_epoch_counter);
> > +
> >  		connector->status = drm_helper_probe_detect(connector,
> > NULL, false);
> 
> Pass a u32 *changed as argument to drm_helper_probe_detect? would
> require signature change,
> >  		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->epoch_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_epoch_counter != connector->epoch_counter) {
> > +			changed = true;
> > +		}
> 
> Could we bump the epoch counter for any event being sent out? Would
> make more sense.

I was thinking about this. That would be quite sane approach I think.
But then we might have to change also many other places where it checks
connection status only. I will take a look as it anyways looks better
than checking both epoch and connection status at the same time.

> >  	}
> >  	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 681cb590f952..a3cc7d0927d6 100644
> > --- a/include/drm/drm_connector.h
> > +++ b/include/drm/drm_connector.h
> > @@ -1288,6 +1288,9 @@ struct drm_connector {
> >  	/** @override_edid: has the EDID been overwritten through
> > debugfs for testing? */
> >  	bool override_edid;
> >  
> > +	/** @epoch_counter: used to detect any other changes in
> > connector, besides status */
> > +	uint64_t epoch_counter;
> > +
> >  #define DRM_CONNECTOR_MAX_ENCODER 3
> >  	/**
> >  	 * @encoder_ids: Valid encoders for this connector. Please only
> > use
> 
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2019-09-05 11:21 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-05 10:37 [PATCH v4 0/3] Send a hotplug when edid changes Stanislav Lisovskiy
2019-09-05 10:37 ` [PATCH v4 1/3] drm: Add helper to compare edids Stanislav Lisovskiy
2019-09-05 10:37 ` [PATCH v4 2/3] drm: Introduce change counter to drm_connector Stanislav Lisovskiy
2019-09-05 11:01   ` Maarten Lankhorst
2019-09-05 11:21     ` Lisovskiy, Stanislav
2019-09-05 10:37 ` [PATCH v4 3/3] drm/i915: Send hotplug event if edid had changed Stanislav Lisovskiy

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.