+Hans

Hans, do you have any comments?

On Mon, May 02, 2022 at 09:53:13AM -0700, Bjorn Andersson wrote:
> In some implementations, such as the Qualcomm platforms, the display
> driver has no way to query the current HPD state and as such it's
> impossible to distinguish between disconnect and attention events.
> 
> Add a parameter to drm_connector_oob_hotplug_event() to pass the HPD
> state.
> 
> Also push the test for unchanged state in the displayport altmode driver
> into the i915 driver, to allow other drivers to act upon each update.
> 
> Signed-off-by: Bjorn Andersson <bjorn.anders...@linaro.org>
> ---
> 
> Changes since v3:
> - Transition to drm_connector_status instead of custom hpd_state 
> 
>  drivers/gpu/drm/drm_connector.c          |  6 ++++--
>  drivers/gpu/drm/i915/display/intel_dp.c  | 17 ++++++++++++++---
>  drivers/gpu/drm/i915/i915_drv.h          |  3 +++
>  drivers/usb/typec/altmodes/displayport.c | 10 +++-------
>  include/drm/drm_connector.h              |  6 ++++--
>  5 files changed, 28 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index 1c48d162c77e..e86c69f0640f 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -2794,6 +2794,7 @@ struct drm_connector 
> *drm_connector_find_by_fwnode(struct fwnode_handle *fwnode)
>  /**
>   * drm_connector_oob_hotplug_event - Report out-of-band hotplug event to 
> connector
>   * @connector_fwnode: fwnode_handle to report the event on
> + * @status: hot plug detect logical state
>   *
>   * On some hardware a hotplug event notification may come from outside the 
> display
>   * driver / device. An example of this is some USB Type-C setups where the 
> hardware
> @@ -2803,7 +2804,8 @@ struct drm_connector 
> *drm_connector_find_by_fwnode(struct fwnode_handle *fwnode)
>   * This function can be used to report these out-of-band events after 
> obtaining
>   * a drm_connector reference through calling drm_connector_find_by_fwnode().
>   */
> -void drm_connector_oob_hotplug_event(struct fwnode_handle *connector_fwnode)
> +void drm_connector_oob_hotplug_event(struct fwnode_handle *connector_fwnode,
> +                                  enum drm_connector_status status)
>  {
>       struct drm_connector *connector;
>  
> @@ -2812,7 +2814,7 @@ void drm_connector_oob_hotplug_event(struct 
> fwnode_handle *connector_fwnode)
>               return;
>  
>       if (connector->funcs->oob_hotplug_event)
> -             connector->funcs->oob_hotplug_event(connector);
> +             connector->funcs->oob_hotplug_event(connector, status);
>  
>       drm_connector_put(connector);
>  }
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c 
> b/drivers/gpu/drm/i915/display/intel_dp.c
> index e4a79c11fd25..56cc023f7bbd 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -4951,15 +4951,26 @@ static int intel_dp_connector_atomic_check(struct 
> drm_connector *conn,
>       return intel_modeset_synced_crtcs(state, conn);
>  }
>  
> -static void intel_dp_oob_hotplug_event(struct drm_connector *connector)
> +static void intel_dp_oob_hotplug_event(struct drm_connector *connector,
> +                                    enum drm_connector_status hpd_state)
>  {
>       struct intel_encoder *encoder = 
> intel_attached_encoder(to_intel_connector(connector));
>       struct drm_i915_private *i915 = to_i915(connector->dev);
> +     bool hpd_high = hpd_state == connector_status_connected;
> +     unsigned int hpd_pin = encoder->hpd_pin;
> +     bool need_work = false;
>  
>       spin_lock_irq(&i915->irq_lock);
> -     i915->hotplug.event_bits |= BIT(encoder->hpd_pin);
> +     if (hpd_high != test_bit(hpd_pin, 
> &i915->hotplug.oob_hotplug_last_state)) {
> +             i915->hotplug.event_bits |= BIT(hpd_pin);
> +
> +             __assign_bit(hpd_pin, &i915->hotplug.oob_hotplug_last_state, 
> hpd_high);
> +             need_work = true;
> +     }
>       spin_unlock_irq(&i915->irq_lock);
> -     queue_delayed_work(system_wq, &i915->hotplug.hotplug_work, 0);
> +
> +     if (need_work)
> +             queue_delayed_work(system_wq, &i915->hotplug.hotplug_work, 0);
>  }
>  
>  static const struct drm_connector_funcs intel_dp_connector_funcs = {
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 24111bf42ce0..96c088bb5522 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -135,6 +135,9 @@ struct i915_hotplug {
>       /* Whether or not to count short HPD IRQs in HPD storms */
>       u8 hpd_short_storm_enabled;
>  
> +     /* Last state reported by oob_hotplug_event for each encoder */
> +     unsigned long oob_hotplug_last_state;
> +
>       /*
>        * if we get a HPD irq from DP and a HPD irq from non-DP
>        * the non-DP HPD could block the workqueue on a mode config
> diff --git a/drivers/usb/typec/altmodes/displayport.c 
> b/drivers/usb/typec/altmodes/displayport.c
> index c1d8c23baa39..9360ca177c7d 100644
> --- a/drivers/usb/typec/altmodes/displayport.c
> +++ b/drivers/usb/typec/altmodes/displayport.c
> @@ -59,7 +59,6 @@ struct dp_altmode {
>       struct typec_displayport_data data;
>  
>       enum dp_state state;
> -     bool hpd;
>  
>       struct mutex lock; /* device lock */
>       struct work_struct work;
> @@ -143,10 +142,8 @@ static int dp_altmode_status_update(struct dp_altmode 
> *dp)
>               if (!ret)
>                       dp->state = DP_STATE_CONFIGURE;
>       } else {
> -             if (dp->hpd != hpd) {
> -                     drm_connector_oob_hotplug_event(dp->connector_fwnode);
> -                     dp->hpd = hpd;
> -             }
> +             drm_connector_oob_hotplug_event(dp->connector_fwnode,
> +                                             hpd ? 
> connector_status_connected : connector_status_disconnected);
>       }
>  
>       return ret;
> @@ -573,8 +570,7 @@ void dp_altmode_remove(struct typec_altmode *alt)
>       cancel_work_sync(&dp->work);
>  
>       if (dp->connector_fwnode) {
> -             if (dp->hpd)
> -                     drm_connector_oob_hotplug_event(dp->connector_fwnode);
> +             drm_connector_oob_hotplug_event(dp->connector_fwnode, 
> connector_status_disconnected);
>  
>               fwnode_handle_put(dp->connector_fwnode);
>       }
> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> index 3ac4bf87f257..886aa1861ed9 100644
> --- a/include/drm/drm_connector.h
> +++ b/include/drm/drm_connector.h
> @@ -1141,7 +1141,8 @@ struct drm_connector_funcs {
>        * This will get called when a hotplug-event for a drm-connector
>        * has been received from a source outside the display driver / device.
>        */
> -     void (*oob_hotplug_event)(struct drm_connector *connector);
> +     void (*oob_hotplug_event)(struct drm_connector *connector,
> +                               enum drm_connector_status status);
>  
>       /**
>        * @debugfs_init:
> @@ -1749,7 +1750,8 @@ drm_connector_is_unregistered(struct drm_connector 
> *connector)
>               DRM_CONNECTOR_UNREGISTERED;
>  }
>  
> -void drm_connector_oob_hotplug_event(struct fwnode_handle *connector_fwnode);
> +void drm_connector_oob_hotplug_event(struct fwnode_handle *connector_fwnode,
> +                                  enum drm_connector_status status);
>  const char *drm_get_connector_type_name(unsigned int connector_type);
>  const char *drm_get_connector_status_name(enum drm_connector_status status);
>  const char *drm_get_subpixel_order_name(enum subpixel_order order);
> -- 
> 2.35.1

-- 
heikki

Reply via email to