On Thu, Aug 19, 2021 at 03:44:53PM +0200, Maxime Ripard wrote:
> The drm_helper_hpd_irq_event() function is iterating over all the
> connectors when an hotplug event is detected.
>
> During that iteration, it will call each connector detect function and
> figure out if its status changed.
>
> Finally, if any connector changed, it will notify the user-space and the
> clients that something changed on the DRM device.
>
> This is supposed to be used for drivers that don't have a hotplug
> interrupt for individual connectors. However, drivers that can use an
> interrupt for a single connector are left in the dust and can either
> reimplement the logic used during the iteration for each connector or
> use that helper and iterate over all connectors all the time.
>
> Since both are suboptimal, let's create a helper that will only perform
> the status detection on a single connector.
>
> Signed-off-by: Maxime Ripard <maxime@xxxxxxxxxx>
> ---
> drivers/gpu/drm/drm_probe_helper.c | 105 ++++++++++++++++++++---------
> include/drm/drm_probe_helper.h | 1 +
> 2 files changed, 74 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c
> index 5606bca3caa8..7e3cbb4333ce 100644
> --- a/drivers/gpu/drm/drm_probe_helper.c
> +++ b/drivers/gpu/drm/drm_probe_helper.c
> @@ -795,6 +795,77 @@ void drm_kms_helper_poll_fini(struct drm_device *dev)
> }
> EXPORT_SYMBOL(drm_kms_helper_poll_fini);
>
> +static bool
> +_drm_connector_helper_hpd_irq_event(struct drm_connector *connector,
> + bool notify)
> +{
> + struct drm_device *dev = connector->dev;
> + enum drm_connector_status old_status;
> + u64 old_epoch_counter;
> + bool changed = false;
> +
> + /* Only handle HPD capable connectors. */
> + drm_WARN_ON(dev, !(connector->polled & DRM_CONNECTOR_POLL_HPD));
> +
> + drm_WARN_ON(dev, !mutex_is_locked(&dev->mode_config.mutex));
> +
> + old_status = connector->status;
> + old_epoch_counter = connector->epoch_counter;
> +
> + DRM_DEBUG_KMS("[CONNECTOR:%d:%s] Old epoch 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));
> +
> + DRM_DEBUG_KMS("[CONNECTOR:%d:%s] New epoch counter %llu\n",
> + connector->base.id,
> + connector->name,
> + connector->epoch_counter);
> +
> + /*
> + * Check if epoch counter had changed, meaning that we need
> + * to send a uevent.
> + */
> + if (old_epoch_counter != connector->epoch_counter)
> + changed = true;
> +
> + if (changed && notify) {
> + drm_kms_helper_hotplug_event(dev);
> + DRM_DEBUG_KMS("Sent hotplug event\n");
> + }
> +
> + return changed;
> +}
> +
> +/**
> + * drm_connector_helper_hpd_irq_event - hotplug processing
> + * @connector: drm_connector
> + *
> + * Drivers can use this helper function to run a detect cycle on a connector
> + * which has the DRM_CONNECTOR_POLL_HPD flag set in its &polled member.
> + *
> + * This helper function is useful for drivers which can track hotplug
> + * interrupts for a single connector.
> + *
> + * This function must be called with the mode setting locks held.
> + *
> + * Note that a connector can be both polled and probed from the hotplug
> + * handler, in case the hotplug interrupt is known to be unreliable.
> + */
Adding a sentence here to point to the other helper for global hpd irq and
vice versa would be neat.
-Daniel
> +bool drm_connector_helper_hpd_irq_event(struct drm_connector *connector)
> +{
> + return _drm_connector_helper_hpd_irq_event(connector, true);
> +}
> +EXPORT_SYMBOL(drm_connector_helper_hpd_irq_event);
> +
> /**
> * drm_helper_hpd_irq_event - hotplug processing
> * @dev: drm_device
> @@ -822,9 +893,7 @@ bool drm_helper_hpd_irq_event(struct drm_device *dev)
> {
> struct drm_connector *connector;
> struct drm_connector_list_iter conn_iter;
> - enum drm_connector_status old_status;
> bool changed = false;
> - u64 old_epoch_counter;
>
> if (!dev->mode_config.poll_enabled)
> return false;
> @@ -832,37 +901,9 @@ bool drm_helper_hpd_irq_event(struct drm_device *dev)
> mutex_lock(&dev->mode_config.mutex);
> drm_connector_list_iter_begin(dev, &conn_iter);
> drm_for_each_connector_iter(connector, &conn_iter) {
> - /* Only handle HPD capable connectors. */
> - if (!(connector->polled & DRM_CONNECTOR_POLL_HPD))
> - continue;
> -
> - old_status = connector->status;
> -
> - old_epoch_counter = connector->epoch_counter;
> -
> - DRM_DEBUG_KMS("[CONNECTOR:%d:%s] Old epoch 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));
> -
> - DRM_DEBUG_KMS("[CONNECTOR:%d:%s] New epoch counter %llu\n",
> - connector->base.id,
> - connector->name,
> - connector->epoch_counter);
> -
> - /*
> - * Check if epoch counter had changed, meaning that we need
> - * to send a uevent.
> - */
> - if (old_epoch_counter != connector->epoch_counter)
> + if (_drm_connector_helper_hpd_irq_event(connector,
> + false))
> changed = true;
> -
> }
> drm_connector_list_iter_end(&conn_iter);
> mutex_unlock(&dev->mode_config.mutex);
> diff --git a/include/drm/drm_probe_helper.h b/include/drm/drm_probe_helper.h
> index 8d3ed2834d34..04c57564c397 100644
> --- a/include/drm/drm_probe_helper.h
> +++ b/include/drm/drm_probe_helper.h
> @@ -18,6 +18,7 @@ int drm_helper_probe_detect(struct drm_connector *connector,
> void drm_kms_helper_poll_init(struct drm_device *dev);
> void drm_kms_helper_poll_fini(struct drm_device *dev);
> bool drm_helper_hpd_irq_event(struct drm_device *dev);
> +bool drm_connector_helper_hpd_irq_event(struct drm_connector *connector);
> void drm_kms_helper_hotplug_event(struct drm_device *dev);
>
> void drm_kms_helper_poll_disable(struct drm_device *dev);
> --
> 2.31.1
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
