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

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

[Ramalingam C]

On 2019-06-28 at 11:24:52 +0300, Stanislav Lisovskiy wrote:
> 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 9d8f2b952004..eaad5155fbdd 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -1361,6 +1361,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
extra line here is preferred.
> + * This helper can be used during probing to determine if
> + * edid had changed.
bool is implicit. if you want you can explain the return value.
> + */
> +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.
> + */
Do we need kdoc for function declaration too!? Should be sufficient for
definition alone.

-Ram
> +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

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

[Ramalingam C]

On 2019-06-28 at 11:24:53 +0300, Stanislav Lisovskiy wrote:
> 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 3ccdcf3dfcde..065eee61859e 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->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 ef2c468205a2..5857053174da 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_epoch_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_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) {
{} is not required here.
>  			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 c6f8486d8b8f..a296bdac085f 100644
> --- a/include/drm/drm_connector.h
> +++ b/include/drm/drm_connector.h
> @@ -1155,6 +1155,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 */
Might want to wrap at 80char.

-Ram
> +	uint64_t epoch_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 v2 3/3] drm/i915: Send hotplug event if edid had changed.

[Ramalingam C]

On 2019-06-28 at 11:24:54 +0300, Stanislav Lisovskiy wrote:
> 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.
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105540
> Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
> ---
>  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 +++++++++++++++-----
>  3 files changed, 43 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index 4336df46fe78..c2ed02478cf9 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -5510,10 +5510,24 @@ static void
>  intel_dp_set_edid(struct intel_dp *intel_dp)
>  {
>  	struct intel_connector *intel_connector = intel_dp->attached_connector;
> +	struct drm_connector *connector = &intel_connector->base;
>  	struct edid *edid;
> +	struct edid *old_edid;
>  
> -	intel_dp_unset_edid(intel_dp);
>  	edid = intel_dp_get_edid(intel_dp);
> +	old_edid = intel_connector->detect_edid;
> +
> +	if (!drm_edid_are_equal(edid, old_edid)) {
> +		DRM_DEBUG_KMS("[CONNECTOR:%d:%s] Edid was changed! Updating blob property.\n",
> +		    connector->base.id, connector->name);
> +
> +		connector->epoch_counter += 1;
> +		DRM_DEBUG_KMS("Updating change counter to %llu\n", connector->epoch_counter);
> +
> +		intel_connector_update_modes(&intel_connector->base, edid);
> +	}
> +
> +	intel_dp_unset_edid(intel_dp);
>  	intel_connector->detect_edid = edid;
>  
>  	intel_dp->has_audio = drm_detect_monitor_audio(edid);
> diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c
> index 0ebec69bbbfc..f892c7b795ce 100644
> --- a/drivers/gpu/drm/i915/display/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
> @@ -2503,7 +2503,7 @@ 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(connector);
>  	intel_wakeref_t wakeref;
> -	struct edid *edid;
> +	struct edid *edid, *old_edid;
>  	bool connected = false;
>  	struct i2c_adapter *i2c;
>  
> @@ -2524,11 +2524,22 @@ intel_hdmi_set_edid(struct drm_connector *connector)
>  
>  	intel_display_power_put(dev_priv, POWER_DOMAIN_GMBUS, wakeref);
>  
> +	old_edid = to_intel_connector(connector)->detect_edid;
> +
> +	if (!drm_edid_are_equal(edid, old_edid)) {
> +		intel_connector_update_modes(connector, edid);
> +		DRM_DEBUG_KMS("Updating change counter to %llu\n", connector->epoch_counter);
> +		connector->epoch_counter += 1;
> +
> +		DRM_DEBUG_KMS("[CONNECTOR:%d:%s] Edid was changed! Updating blob property.\n",
> +		    connector->base.id, connector->name);
> +	}
> +	intel_hdmi_unset_edid(connector);
>  	to_intel_connector(connector)->detect_edid = edid;
> +
This and next changes are unrelated with this commit. Might want to keep
it for separate patch.

-Ram
>  	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);
> -
>  		connected = true;
>  	}
>  
> @@ -2555,7 +2566,6 @@ intel_hdmi_detect(struct drm_connector *connector, bool force)
>  	    !intel_digital_port_connected(encoder))
>  		goto out;
>  
> -	intel_hdmi_unset_edid(connector);
>  
>  	if (intel_hdmi_set_edid(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 ea3de4acc850..f9d9e963196a 100644
> --- a/drivers/gpu/drm/i915/display/intel_hotplug.c
> +++ b/drivers/gpu/drm/i915/display/intel_hotplug.c
> @@ -271,23 +271,33 @@ bool 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 false;
> +	if (old_epoch_counter != connector->base.epoch_counter)
> +		ret = true;
>  
> -	DRM_DEBUG_KMS("[CONNECTOR:%d:%s] status updated from %s to %s\n",
> +	if (old_status != connector->base.status)
> +		ret = true;
> +
> +	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 true;
> +	return ret;
>  }
>  
>  static bool intel_encoder_has_hpd_pulse(struct intel_encoder *encoder)
> -- 
> 2.17.1
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 3/3] drm/i915: Send hotplug event if edid had changed.

[Ramalingam C]

On 2019-06-28 at 17:06:09 +0530, Lisovskiy, Stanislav wrote:
> On Fri, 2019-06-28 at 09:54 +0530, Ramalingam C wrote:
> > On 2019-06-28 at 11:24:54 +0300, Stanislav Lisovskiy wrote:
> > > 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.
> > > 
> > > 
> 
> > > @@ -2524,11 +2524,22 @@ intel_hdmi_set_edid(struct drm_connector
> > > *connector)
> > >  
> > >  	intel_display_power_put(dev_priv, POWER_DOMAIN_GMBUS, wakeref);
> > >  
> > > +	old_edid = to_intel_connector(connector)->detect_edid;
> > > +
> > > +	if (!drm_edid_are_equal(edid, old_edid)) {
> > > +		intel_connector_update_modes(connector, edid);
> > > +		DRM_DEBUG_KMS("Updating change counter to %llu\n",
> > > connector->epoch_counter);
> > > +		connector->epoch_counter += 1;
> > > +
> > > +		DRM_DEBUG_KMS("[CONNECTOR:%d:%s] Edid was changed!
> > > Updating blob property.\n",
> > > +		    connector->base.id, connector->name);
> > > +	}
> > > +	intel_hdmi_unset_edid(connector);
> > >  	to_intel_connector(connector)->detect_edid = edid;
> > > +
> 
> > 
> > This and next changes are unrelated with this commit. Might want to
> > keep
> > it for separate patch.
> 
> What do you mean by unrelated? I thought dependent changes should go in
> one patch series and here we are taking into use epoch_counter for
> i915, which was introduced in previous 2 separate drm patches from that
> series.
> 
> The drm changes should obviously always go first here, otherwise this
> patch will fail - I wouldn't even be able to send it for testing if
> that would be in another series.
Meant the new line addition and line removal. just usual suggestions.
you can ignore if you prefer. apart from that changes looks good.

-Ram
> 
> > 
> > -Ram
> > >  	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);
> > > -
> > >  		connected = true;
> > >  	}
> > >  
> > > @@ -2555,7 +2566,6 @@ intel_hdmi_detect(struct drm_connector
> > > *connector, bool force)
> > >  	    !intel_digital_port_connected(encoder))
> > >  		goto out;
> > >  
> > > -	intel_hdmi_unset_edid(connector);
> > >  
> > >  	if (intel_hdmi_set_edid(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 ea3de4acc850..f9d9e963196a 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_hotplug.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_hotplug.c
> > > @@ -271,23 +271,33 @@ bool 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 false;
> > > +	if (old_epoch_counter != connector->base.epoch_counter)
> > > +		ret = true;
> > >  
> > > -	DRM_DEBUG_KMS("[CONNECTOR:%d:%s] status updated from %s to
> > > %s\n",
> > > +	if (old_status != connector->base.status)
> > > +		ret = true;
> > > +
> > > +	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 true;
> > > +	return ret;
> > >  }
> > >  
> > >  static bool intel_encoder_has_hpd_pulse(struct intel_encoder
> > > *encoder)
> > > -- 
> > > 2.17.1
> > > 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v2 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                   | 33 ++++++++++++++++++++
 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                  |  3 ++
 include/drm/drm_edid.h                       |  9 ++++++
 8 files changed, 116 insertions(+), 11 deletions(-)

-- 
2.17.1

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

[PATCH v2 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.

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 9d8f2b952004..eaad5155fbdd 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -1361,6 +1361,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

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

[PATCH v2 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.

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 3ccdcf3dfcde..065eee61859e 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->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 ef2c468205a2..5857053174da 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_epoch_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_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 c6f8486d8b8f..a296bdac085f 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -1155,6 +1155,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

[PATCH v2 3/3] drm/i915: Send hotplug event if edid had changed.

[Stanislav Lisovskiy]

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.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105540
Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
---
 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 +++++++++++++++-----
 3 files changed, 43 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index 4336df46fe78..c2ed02478cf9 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -5510,10 +5510,24 @@ static void
 intel_dp_set_edid(struct intel_dp *intel_dp)
 {
 	struct intel_connector *intel_connector = intel_dp->attached_connector;
+	struct drm_connector *connector = &intel_connector->base;
 	struct edid *edid;
+	struct edid *old_edid;
 
-	intel_dp_unset_edid(intel_dp);
 	edid = intel_dp_get_edid(intel_dp);
+	old_edid = intel_connector->detect_edid;
+
+	if (!drm_edid_are_equal(edid, old_edid)) {
+		DRM_DEBUG_KMS("[CONNECTOR:%d:%s] Edid was changed! Updating blob property.\n",
+		    connector->base.id, connector->name);
+
+		connector->epoch_counter += 1;
+		DRM_DEBUG_KMS("Updating change counter to %llu\n", connector->epoch_counter);
+
+		intel_connector_update_modes(&intel_connector->base, edid);
+	}
+
+	intel_dp_unset_edid(intel_dp);
 	intel_connector->detect_edid = edid;
 
 	intel_dp->has_audio = drm_detect_monitor_audio(edid);
diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c
index 0ebec69bbbfc..f892c7b795ce 100644
--- a/drivers/gpu/drm/i915/display/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
@@ -2503,7 +2503,7 @@ 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(connector);
 	intel_wakeref_t wakeref;
-	struct edid *edid;
+	struct edid *edid, *old_edid;
 	bool connected = false;
 	struct i2c_adapter *i2c;
 
@@ -2524,11 +2524,22 @@ intel_hdmi_set_edid(struct drm_connector *connector)
 
 	intel_display_power_put(dev_priv, POWER_DOMAIN_GMBUS, wakeref);
 
+	old_edid = to_intel_connector(connector)->detect_edid;
+
+	if (!drm_edid_are_equal(edid, old_edid)) {
+		intel_connector_update_modes(connector, edid);
+		DRM_DEBUG_KMS("Updating change counter to %llu\n", connector->epoch_counter);
+		connector->epoch_counter += 1;
+
+		DRM_DEBUG_KMS("[CONNECTOR:%d:%s] Edid was changed! Updating blob property.\n",
+		    connector->base.id, connector->name);
+	}
+	intel_hdmi_unset_edid(connector);
 	to_intel_connector(connector)->detect_edid = 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);
-
 		connected = true;
 	}
 
@@ -2555,7 +2566,6 @@ intel_hdmi_detect(struct drm_connector *connector, bool force)
 	    !intel_digital_port_connected(encoder))
 		goto out;
 
-	intel_hdmi_unset_edid(connector);
 
 	if (intel_hdmi_set_edid(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 ea3de4acc850..f9d9e963196a 100644
--- a/drivers/gpu/drm/i915/display/intel_hotplug.c
+++ b/drivers/gpu/drm/i915/display/intel_hotplug.c
@@ -271,23 +271,33 @@ bool 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 false;
+	if (old_epoch_counter != connector->base.epoch_counter)
+		ret = true;
 
-	DRM_DEBUG_KMS("[CONNECTOR:%d:%s] status updated from %s to %s\n",
+	if (old_status != connector->base.status)
+		ret = true;
+
+	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 true;
+	return ret;
 }
 
 static bool intel_encoder_has_hpd_pulse(struct intel_encoder *encoder)
-- 
2.17.1

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

✗ Fi.CI.CHECKPATCH: warning for Send a hotplug when edid changes (rev2)

[Patchwork]

== Series Details ==

Series: Send a hotplug when edid changes (rev2)
URL   : https://patchwork.freedesktop.org/series/62816/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
4a0b026f1681 drm: Add helper to compare edids.
-:32: CHECK:COMPARISON_TO_NULL: Comparison to NULL could be written "edid1"
#32: FILE: drivers/gpu/drm/drm_edid.c:1375:
+	bool edid1_present = edid1 != NULL;

-:33: CHECK:COMPARISON_TO_NULL: Comparison to NULL could be written "edid2"
#33: FILE: drivers/gpu/drm/drm_edid.c:1376:
+	bool edid2_present = edid2 != NULL;

-:39: CHECK:BRACES: Blank lines aren't necessary after an open brace '{'
#39: FILE: drivers/gpu/drm/drm_edid.c:1382:
+	if (edid1) {
+

-:54: CHECK:LINE_SPACING: Please don't use multiple blank lines
#54: FILE: drivers/gpu/drm/drm_edid.c:1397:
+
+

total: 0 errors, 0 warnings, 4 checks, 54 lines checked
0910fd34e6ab drm: Introduce change counter to drm_connector
-:42: CHECK:PREFER_KERNEL_TYPES: Prefer kernel type 'u64' over 'uint64_t'
#42: FILE: drivers/gpu/drm/drm_probe_helper.c:779:
+	uint64_t old_epoch_counter;

-:69: WARNING:BRACES: braces {} are not necessary for single statement blocks
#69: FILE: drivers/gpu/drm/drm_probe_helper.c:811:
+		if (old_status != connector->status) {
 			changed = true;
+		}

-:102: CHECK:PREFER_KERNEL_TYPES: Prefer kernel type 'u64' over 'uint64_t'
#102: FILE: include/drm/drm_connector.h:1285:
+	uint64_t epoch_counter;

total: 0 errors, 1 warnings, 2 checks, 69 lines checked
7d356e798c65 drm/i915: Send hotplug event if edid had changed.
-:8: WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#8: 
to calling layer using drm_connector->change_counter(proposed by Daniel Vetter).

-:45: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#45: FILE: drivers/gpu/drm/i915/display/intel_dp.c:5522:
+		DRM_DEBUG_KMS("[CONNECTOR:%d:%s] Edid was changed! Updating blob property.\n",
+		    connector->base.id, connector->name);

-:82: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#82: FILE: drivers/gpu/drm/i915/display/intel_hdmi.c:2535:
+		DRM_DEBUG_KMS("[CONNECTOR:%d:%s] Edid was changed! Updating blob property.\n",
+		    connector->base.id, connector->name);

-:132: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#132: FILE: drivers/gpu/drm/i915/display/intel_hotplug.c:293:
+		DRM_DEBUG_KMS("[CONNECTOR:%d:%s] status updated from %s to %s(change counter %llu)\n",
 		      connector->base.base.id,

total: 0 errors, 1 warnings, 3 checks, 101 lines checked

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

Re: [PATCH v2 3/3] drm/i915: Send hotplug event if edid had changed.

[Lisovskiy, Stanislav]

On Fri, 2019-06-28 at 09:54 +0530, Ramalingam C wrote:
> On 2019-06-28 at 11:24:54 +0300, Stanislav Lisovskiy wrote:
> > 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.
> > 
> > 

> > @@ -2524,11 +2524,22 @@ intel_hdmi_set_edid(struct drm_connector
> > *connector)
> >  
> >  	intel_display_power_put(dev_priv, POWER_DOMAIN_GMBUS, wakeref);
> >  
> > +	old_edid = to_intel_connector(connector)->detect_edid;
> > +
> > +	if (!drm_edid_are_equal(edid, old_edid)) {
> > +		intel_connector_update_modes(connector, edid);
> > +		DRM_DEBUG_KMS("Updating change counter to %llu\n",
> > connector->epoch_counter);
> > +		connector->epoch_counter += 1;
> > +
> > +		DRM_DEBUG_KMS("[CONNECTOR:%d:%s] Edid was changed!
> > Updating blob property.\n",
> > +		    connector->base.id, connector->name);
> > +	}
> > +	intel_hdmi_unset_edid(connector);
> >  	to_intel_connector(connector)->detect_edid = edid;
> > +

> 
> This and next changes are unrelated with this commit. Might want to
> keep
> it for separate patch.

What do you mean by unrelated? I thought dependent changes should go in
one patch series and here we are taking into use epoch_counter for
i915, which was introduced in previous 2 separate drm patches from that
series.

The drm changes should obviously always go first here, otherwise this
patch will fail - I wouldn't even be able to send it for testing if
that would be in another series.

> 
> -Ram
> >  	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);
> > -
> >  		connected = true;
> >  	}
> >  
> > @@ -2555,7 +2566,6 @@ intel_hdmi_detect(struct drm_connector
> > *connector, bool force)
> >  	    !intel_digital_port_connected(encoder))
> >  		goto out;
> >  
> > -	intel_hdmi_unset_edid(connector);
> >  
> >  	if (intel_hdmi_set_edid(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 ea3de4acc850..f9d9e963196a 100644
> > --- a/drivers/gpu/drm/i915/display/intel_hotplug.c
> > +++ b/drivers/gpu/drm/i915/display/intel_hotplug.c
> > @@ -271,23 +271,33 @@ bool 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 false;
> > +	if (old_epoch_counter != connector->base.epoch_counter)
> > +		ret = true;
> >  
> > -	DRM_DEBUG_KMS("[CONNECTOR:%d:%s] status updated from %s to
> > %s\n",
> > +	if (old_status != connector->base.status)
> > +		ret = true;
> > +
> > +	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 true;
> > +	return ret;
> >  }
> >  
> >  static bool intel_encoder_has_hpd_pulse(struct intel_encoder
> > *encoder)
> > -- 
> > 2.17.1
> > 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.BAT: success for Send a hotplug when edid changes (rev2)

[Patchwork]

== Series Details ==

Series: Send a hotplug when edid changes (rev2)
URL   : https://patchwork.freedesktop.org/series/62816/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_6379 -> Patchwork_13467
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13467/

Known issues
------------

  Here are the changes found in Patchwork_13467 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_mmap_gtt@basic-small-bo:
    - fi-icl-u3:          [PASS][1] -> [DMESG-WARN][2] ([fdo#107724])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6379/fi-icl-u3/igt@gem_mmap_gtt@basic-small-bo.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13467/fi-icl-u3/igt@gem_mmap_gtt@basic-small-bo.html

  * igt@kms_chamelium@hdmi-hpd-fast:
    - fi-kbl-7567u:       [PASS][3] -> [FAIL][4] ([fdo#109485])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6379/fi-kbl-7567u/igt@kms_chamelium@hdmi-hpd-fast.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13467/fi-kbl-7567u/igt@kms_chamelium@hdmi-hpd-fast.html

  
#### Possible fixes ####

  * igt@gem_mmap_gtt@basic-write-gtt-no-prefault:
    - fi-icl-u3:          [DMESG-WARN][5] ([fdo#107724]) -> [PASS][6] +1 similar issue
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6379/fi-icl-u3/igt@gem_mmap_gtt@basic-write-gtt-no-prefault.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13467/fi-icl-u3/igt@gem_mmap_gtt@basic-write-gtt-no-prefault.html

  
  [fdo#107724]: https://bugs.freedesktop.org/show_bug.cgi?id=107724
  [fdo#109485]: https://bugs.freedesktop.org/show_bug.cgi?id=109485


Participating hosts (51 -> 41)
------------------------------

  Additional (1): fi-icl-dsi 
  Missing    (11): fi-kbl-soraka fi-ilk-m540 fi-bsw-n3050 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-pnv-d510 fi-icl-y fi-icl-guc fi-byt-clapper fi-bdw-samus 


Build changes
-------------

  * Linux: CI_DRM_6379 -> Patchwork_13467

  CI_DRM_6379: 12f8aba06508afb9b668a2496b2fb296ed21e377 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5072: 5f6cf7070b249975bac4d747a9c44d81c94c4381 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_13467: 7d356e798c65ba3c92755a663f10978e947a70e4 @ git://anongit.freedesktop.org/gfx-ci/linux


== Kernel 32bit build ==

Warning: Kernel 32bit buildtest failed:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13467/build_32bit.log

  CALL    scripts/checksyscalls.sh
  CALL    scripts/atomic/check-atomics.sh
  CHK     include/generated/compile.h
Kernel: arch/x86/boot/bzImage is ready  (#1)
  Building modules, stage 2.
  MODPOST 112 modules
ERROR: "__udivdi3" [drivers/gpu/drm/amd/amdgpu/amdgpu.ko] undefined!
ERROR: "__divdi3" [drivers/gpu/drm/amd/amdgpu/amdgpu.ko] undefined!
scripts/Makefile.modpost:91: recipe for target '__modpost' failed
make[1]: *** [__modpost] Error 1
Makefile:1287: recipe for target 'modules' failed
make: *** [modules] Error 2


== Linux commits ==

7d356e798c65 drm/i915: Send hotplug event if edid had changed.
0910fd34e6ab drm: Introduce change counter to drm_connector
4a0b026f1681 drm: Add helper to compare edids.

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13467/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.IGT: failure for Send a hotplug when edid changes (rev2)

[Patchwork]

== Series Details ==

Series: Send a hotplug when edid changes (rev2)
URL   : https://patchwork.freedesktop.org/series/62816/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_6379_full -> Patchwork_13467_full
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_13467_full absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_13467_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_13467_full:

### IGT changes ###

#### Possible regressions ####

  * igt@kms_flip@busy-flip:
    - shard-glk:          [PASS][1] -> [FAIL][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6379/shard-glk2/igt@kms_flip@busy-flip.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13467/shard-glk5/igt@kms_flip@busy-flip.html

  
Known issues
------------

  Here are the changes found in Patchwork_13467_full that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_eio@reset-stress:
    - shard-kbl:          [PASS][3] -> [FAIL][4] ([fdo#109661])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6379/shard-kbl4/igt@gem_eio@reset-stress.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13467/shard-kbl6/igt@gem_eio@reset-stress.html

  * igt@i915_pm_rpm@i2c:
    - shard-hsw:          [PASS][5] -> [FAIL][6] ([fdo#104097])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6379/shard-hsw5/igt@i915_pm_rpm@i2c.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13467/shard-hsw6/igt@i915_pm_rpm@i2c.html

  * igt@kms_big_fb@yf-tiled-32bpp-rotate-180:
    - shard-apl:          [PASS][7] -> [INCOMPLETE][8] ([fdo#103927])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6379/shard-apl5/igt@kms_big_fb@yf-tiled-32bpp-rotate-180.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13467/shard-apl2/igt@kms_big_fb@yf-tiled-32bpp-rotate-180.html

  * igt@kms_cursor_crc@pipe-a-cursor-suspend:
    - shard-kbl:          [PASS][9] -> [DMESG-WARN][10] ([fdo#108566]) +1 similar issue
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6379/shard-kbl4/igt@kms_cursor_crc@pipe-a-cursor-suspend.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13467/shard-kbl6/igt@kms_cursor_crc@pipe-a-cursor-suspend.html

  * igt@kms_cursor_crc@pipe-b-cursor-suspend:
    - shard-apl:          [PASS][11] -> [DMESG-WARN][12] ([fdo#108566]) +1 similar issue
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6379/shard-apl4/igt@kms_cursor_crc@pipe-b-cursor-suspend.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13467/shard-apl5/igt@kms_cursor_crc@pipe-b-cursor-suspend.html

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-pri-indfb-draw-render:
    - shard-iclb:         [PASS][13] -> [FAIL][14] ([fdo#103167]) +1 similar issue
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6379/shard-iclb2/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-pri-indfb-draw-render.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13467/shard-iclb5/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-pri-indfb-draw-render.html

  * igt@kms_frontbuffer_tracking@fbc-2p-scndscrn-cur-indfb-draw-pwrite:
    - shard-hsw:          [PASS][15] -> [SKIP][16] ([fdo#109271]) +27 similar issues
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6379/shard-hsw7/igt@kms_frontbuffer_tracking@fbc-2p-scndscrn-cur-indfb-draw-pwrite.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13467/shard-hsw1/igt@kms_frontbuffer_tracking@fbc-2p-scndscrn-cur-indfb-draw-pwrite.html

  * igt@kms_plane_alpha_blend@pipe-b-coverage-7efc:
    - shard-skl:          [PASS][17] -> [FAIL][18] ([fdo#108145] / [fdo#110403])
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6379/shard-skl9/igt@kms_plane_alpha_blend@pipe-b-coverage-7efc.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13467/shard-skl6/igt@kms_plane_alpha_blend@pipe-b-coverage-7efc.html

  * igt@kms_psr@psr2_sprite_mmap_gtt:
    - shard-iclb:         [PASS][19] -> [SKIP][20] ([fdo#109441])
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6379/shard-iclb2/igt@kms_psr@psr2_sprite_mmap_gtt.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13467/shard-iclb5/igt@kms_psr@psr2_sprite_mmap_gtt.html

  
#### Possible fixes ####

  * igt@i915_selftest@mock_requests:
    - shard-skl:          [INCOMPLETE][21] ([fdo#110550]) -> [PASS][22]
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6379/shard-skl9/igt@i915_selftest@mock_requests.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13467/shard-skl7/igt@i915_selftest@mock_requests.html

  * igt@i915_suspend@debugfs-reader:
    - shard-kbl:          [DMESG-WARN][23] ([fdo#108566]) -> [PASS][24] +2 similar issues
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6379/shard-kbl3/igt@i915_suspend@debugfs-reader.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13467/shard-kbl1/igt@i915_suspend@debugfs-reader.html

  * igt@kms_busy@extended-modeset-hang-oldfb-with-reset-render-b:
    - shard-snb:          [SKIP][25] ([fdo#109271] / [fdo#109278]) -> [PASS][26]
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6379/shard-snb5/igt@kms_busy@extended-modeset-hang-oldfb-with-reset-render-b.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13467/shard-snb5/igt@kms_busy@extended-modeset-hang-oldfb-with-reset-render-b.html

  * igt@kms_color@pipe-c-ctm-red-to-blue:
    - shard-skl:          [FAIL][27] ([fdo#107201]) -> [PASS][28]
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6379/shard-skl2/igt@kms_color@pipe-c-ctm-red-to-blue.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13467/shard-skl10/igt@kms_color@pipe-c-ctm-red-to-blue.html

  * igt@kms_cursor_legacy@flip-vs-cursor-toggle:
    - shard-skl:          [FAIL][29] ([fdo#102670] / [fdo#106081]) -> [PASS][30]
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6379/shard-skl5/igt@kms_cursor_legacy@flip-vs-cursor-toggle.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13467/shard-skl2/igt@kms_cursor_legacy@flip-vs-cursor-toggle.html

  * igt@kms_flip@2x-flip-vs-expired-vblank:
    - shard-glk:          [FAIL][31] ([fdo#105363]) -> [PASS][32]
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6379/shard-glk1/igt@kms_flip@2x-flip-vs-expired-vblank.html
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13467/shard-glk3/igt@kms_flip@2x-flip-vs-expired-vblank.html

  * igt@kms_flip@flip-vs-expired-vblank:
    - shard-skl:          [FAIL][33] ([fdo#105363]) -> [PASS][34]
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6379/shard-skl7/igt@kms_flip@flip-vs-expired-vblank.html
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13467/shard-skl2/igt@kms_flip@flip-vs-expired-vblank.html

  * igt@kms_frontbuffer_tracking@fbc-2p-scndscrn-spr-indfb-draw-pwrite:
    - shard-hsw:          [SKIP][35] ([fdo#109271]) -> [PASS][36] +16 similar issues
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6379/shard-hsw1/igt@kms_frontbuffer_tracking@fbc-2p-scndscrn-spr-indfb-draw-pwrite.html
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13467/shard-hsw4/igt@kms_frontbuffer_tracking@fbc-2p-scndscrn-spr-indfb-draw-pwrite.html

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-shrfb-draw-pwrite:
    - shard-iclb:         [FAIL][37] ([fdo#103167]) -> [PASS][38] +5 similar issues
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6379/shard-iclb8/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-shrfb-draw-pwrite.html
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13467/shard-iclb7/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-shrfb-draw-pwrite.html

  * igt@kms_frontbuffer_tracking@fbcpsr-rgb101010-draw-mmap-cpu:
    - shard-iclb:         [INCOMPLETE][39] ([fdo#106978] / [fdo#107713]) -> [PASS][40]
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6379/shard-iclb1/igt@kms_frontbuffer_tracking@fbcpsr-rgb101010-draw-mmap-cpu.html
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13467/shard-iclb4/igt@kms_frontbuffer_tracking@fbcpsr-rgb101010-draw-mmap-cpu.html

  * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
    - shard-snb:          [SKIP][41] ([fdo#109271]) -> [PASS][42] +8 similar issues
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6379/shard-snb5/igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b.html
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13467/shard-snb5/igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b.html

  * igt@kms_plane@pixel-format-pipe-a-planes-source-clamping:
    - shard-hsw:          [INCOMPLETE][43] ([fdo#103540]) -> [PASS][44]
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6379/shard-hsw7/igt@kms_plane@pixel-format-pipe-a-planes-source-clamping.html
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13467/shard-hsw2/igt@kms_plane@pixel-format-pipe-a-planes-source-clamping.html

  * igt@kms_plane@plane-panning-bottom-right-suspend-pipe-b-planes:
    - shard-apl:          [DMESG-WARN][45] ([fdo#108566]) -> [PASS][46] +3 similar issues
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6379/shard-apl6/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-b-planes.html
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13467/shard-apl2/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-b-planes.html

  * igt@kms_plane_alpha_blend@pipe-c-coverage-7efc:
    - shard-skl:          [FAIL][47] ([fdo#108145] / [fdo#110403]) -> [PASS][48]
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6379/shard-skl2/igt@kms_plane_alpha_blend@pipe-c-coverage-7efc.html
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13467/shard-skl10/igt@kms_plane_alpha_blend@pipe-c-coverage-7efc.html

  * igt@kms_psr@psr2_sprite_plane_move:
    - shard-iclb:         [SKIP][49] ([fdo#109441]) -> [PASS][50] +2 similar issues
   [49]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6379/shard-iclb7/igt@kms_psr@psr2_sprite_plane_move.html
   [50]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13467/shard-iclb2/igt@kms_psr@psr2_sprite_plane_move.html

  * igt@kms_sysfs_edid_timing:
    - shard-iclb:         [FAIL][51] ([fdo#100047]) -> [PASS][52]
   [51]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6379/shard-iclb3/igt@kms_sysfs_edid_timing.html
   [52]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13467/shard-iclb8/igt@kms_sysfs_edid_timing.html

  * igt@tools_test@tools_test:
    - shard-glk:          [SKIP][53] ([fdo#109271]) -> [PASS][54]
   [53]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6379/shard-glk2/igt@tools_test@tools_test.html
   [54]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13467/shard-glk5/igt@tools_test@tools_test.html

  
#### Warnings ####

  * igt@kms_cursor_legacy@2x-long-cursor-vs-flip-atomic:
    - shard-hsw:          [FAIL][55] ([fdo#105767]) -> [SKIP][56] ([fdo#109271])
   [55]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6379/shard-hsw4/igt@kms_cursor_legacy@2x-long-cursor-vs-flip-atomic.html
   [56]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13467/shard-hsw1/igt@kms_cursor_legacy@2x-long-cursor-vs-flip-atomic.html

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-mmap-wc:
    - shard-skl:          [FAIL][57] ([fdo#108040]) -> [FAIL][58] ([fdo#103167])
   [57]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6379/shard-skl5/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-mmap-wc.html
   [58]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13467/shard-skl2/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-mmap-wc.html

  
  [fdo#100047]: https://bugs.freedesktop.org/show_bug.cgi?id=100047
  [fdo#102670]: https://bugs.freedesktop.org/show_bug.cgi?id=102670
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103540]: https://bugs.freedesktop.org/show_bug.cgi?id=103540
  [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
  [fdo#104097]: https://bugs.freedesktop.org/show_bug.cgi?id=104097
  [fdo#105363]: https://bugs.freedesktop.org/show_bug.cgi?id=105363
  [fdo#105767]: https://bugs.freedesktop.org/show_bug.cgi?id=105767
  [fdo#106081]: https://bugs.freedesktop.org/show_bug.cgi?id=106081
  [fdo#106978]: https://bugs.freedesktop.org/show_bug.cgi?id=106978
  [fdo#107201]: https://bugs.freedesktop.org/show_bug.cgi?id=107201
  [fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
  [fdo#108040]: https://bugs.freedesktop.org/show_bug.cgi?id=108040
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#108566]: https://bugs.freedesktop.org/show_bug.cgi?id=108566
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
  [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
  [fdo#109661]: https://bugs.freedesktop.org/show_bug.cgi?id=109661
  [fdo#110403]: https://bugs.freedesktop.org/show_bug.cgi?id=110403
  [fdo#110550]: https://bugs.freedesktop.org/show_bug.cgi?id=110550


Participating hosts (10 -> 10)
------------------------------

  No changes in participating hosts


Build changes
-------------

  * Linux: CI_DRM_6379 -> Patchwork_13467

  CI_DRM_6379: 12f8aba06508afb9b668a2496b2fb296ed21e377 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5072: 5f6cf7070b249975bac4d747a9c44d81c94c4381 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_13467: 7d356e798c65ba3c92755a663f10978e947a70e4 @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13467/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Lyude Paul]

Sorry for the late response! I like the idea here and I've brought up edid
comparison a couple times. Hopefully this isn't overkill, but I had a little
more in mind then just a helper like this (and I've had this on my mind for a
while!

When it comes to suspend/resume reprobing, I think there's more work then just
comparing edids that are redundant. I think most drivers have connectors that
fall into one of the following classes:

 * Always "in-sync" with events, e.g. event handling does not stop just
   because the device is suspended. This is actually true in some cases for
   nouveau and amdgpu, where both drivers can rely on ACPI firmware to send
   events while the device is suspended.
 * Only in-sync with events while the device is awake. Of course, that's the
   whole reason for these patches!

From what I understand based on previous discussions with some other intel
engineers back when they were trying to make the i915 suspend/resume process
faster, hotplug probing can be a pretty significant timesink in some cases.
Additionally, it's not always nessecarily everywhere if some connectors are
able to stay in sync, and that might be a benefit.

Additionally, I think there's a number of other parts of the process that I
would imagine every driver would end up needing to implement. A couple rather
simple examples: skipping edid comparisons for disconnected or newly-connected
connectors, assuming that failure to read an EDID on a connected connector
that could have it's EDID read before suspend means we have to consider said
connector to be changed, etc. So why not add helpers to handle all of this
boilerplate as well?

An idea I had at one point would be to add the ability to mark when a driver
believes a DRM connector has gone "out of sync" like I mentioned above. A
simple example: on laptops I've observed with nouveau that supported ACPI
hotplug events, ACPI hotplug events only ever seemed to come if a connector
was plugged in - not if a connector was removed. If we use this logic during
the runtime resume, we could ascertain that unless an ACPI hotplug event was
received before we resumed that we can actually skip reprobing any connectors
that were disconnected at runtime suspend and only probe the ones that were
connected.

So, maybe we could have helpers like this:

/* Inform DRM that we've disabled our primary means of receiving HPD
 * events.
 */
drm_connector_suspend_hpd()

/* A helper that goes through and performs basic connector reprobing and
 * EDID comparisons on connectors marked with
 * drm_connector_reprobe_on_hpd_resume(). Fires off an HPD event if any
 * connector changes are found/returns an int/etc. etc. whatever.
 *
 * To be called by the driver *after* it has re-enabled HPD detection
 * for all of it's connectors.
 */
drm_connector_resume_hpd()

/* Indicate to DRM that the given connector no longer has HPD, and will need
 * to be reprobed on resume
 */
drm_connector_reprobe_on_hpd_resume()

/* Convienence function to indicate to DRM that all connectors have lost
 * their primary means to receive HPD events, and will need to be
 * reprobed on resume. Useful for scenarios like S3 suspend.
 */
drm_connector_reprobe_all_on_hpd_resume()

I think this would also fit in nicely with the new hotplug uevent ideas
that have been floating around recently, since we could then use these
helpers to compress all of the connector changes that happened over a
s/r cycle into a single event (or, no event!)

On Fri, 2019-06-28 at 11:24 +0300, Stanislav Lisovskiy wrote:
> 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 9d8f2b952004..eaad5155fbdd 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -1361,6 +1361,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,
-- 
Cheers,
	Lyude Paul

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

✗ Fi.CI.BAT: failure for Send a hotplug when edid changes (rev3)

[Patchwork]

== Series Details ==

Series: Send a hotplug when edid changes (rev3)
URL   : https://patchwork.freedesktop.org/series/62816/
State : failure

== Summary ==

Applying: drm: Add helper to compare edids.
Applying: drm: Introduce change counter to drm_connector
Applying: drm/i915: Send hotplug event if edid had changed.
Using index info to reconstruct a base tree...
M	drivers/gpu/drm/i915/display/intel_dp.c
M	drivers/gpu/drm/i915/display/intel_hdmi.c
M	drivers/gpu/drm/i915/display/intel_hotplug.c
Falling back to patching base and 3-way merge...
Auto-merging drivers/gpu/drm/i915/display/intel_hotplug.c
CONFLICT (content): Merge conflict in drivers/gpu/drm/i915/display/intel_hotplug.c
Auto-merging drivers/gpu/drm/i915/display/intel_hdmi.c
Auto-merging drivers/gpu/drm/i915/display/intel_dp.c
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch' to see the failed patch
Patch failed at 0003 drm/i915: Send hotplug event if edid had changed.
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

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