[Intel-gfx] [PATCH v4 2/3] drm: Introduce change counter to drm_connector

[Maarten Lankhorst]

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 at 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