2018-09-19 11:31:09

by Chris Chiu

[permalink] [raw]
Subject: [PATCH] drm/i915: re-check the hotplug with a delayed work

I have few ASUS laptops, X705FD(Intel i7-8565), X560UD(Intel i5-8250U)
and X530UN(Intel i7-8550U) share the same problem. The HDMI connector
status stays 'connected' even the HDMI cable has been unplugged.
Then the status in sysfs would never change since then until we
do 'xrandr' to reprobe the devices. It would also cause the audio
output path cannot correctly swicth based on the connector status.

This commit kicks off a delayed work when the status remains unchanged
in the first hotplug event handling, which may not be the perfect
timing in some special cases.

Signed-off-by: Chris Chiu <[email protected]>
---
drivers/gpu/drm/i915/i915_drv.h | 1 +
drivers/gpu/drm/i915/intel_hotplug.c | 35 +++++++++++++++++++++++++++++++----
2 files changed, 32 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index d51d8574a679..78e2cf09cc10 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -286,6 +286,7 @@ struct i915_hotplug {
} stats[HPD_NUM_PINS];
u32 event_bits;
struct delayed_work reenable_work;
+ struct delayed_work recheck_work;

struct intel_digital_port *irq_port[I915_MAX_PORTS];
u32 long_port_mask;
diff --git a/drivers/gpu/drm/i915/intel_hotplug.c b/drivers/gpu/drm/i915/intel_hotplug.c
index 43aa92beff2a..089a24588ec8 100644
--- a/drivers/gpu/drm/i915/intel_hotplug.c
+++ b/drivers/gpu/drm/i915/intel_hotplug.c
@@ -349,14 +349,15 @@ static void i915_digport_work_func(struct work_struct *work)
}
}

+#define HPD_RECHECK_DELAY (2 * 1000)
+
/*
* Handle hotplug events outside the interrupt handler proper.
*/
-static void i915_hotplug_work_func(struct work_struct *work)
+static void do_i915_hotplug_check(struct work_struct *work,
+ struct drm_i915_private *dev_priv,
+ struct drm_device *dev, bool do_recheck)
{
- struct drm_i915_private *dev_priv =
- container_of(work, struct drm_i915_private, hotplug.hotplug_work);
- struct drm_device *dev = &dev_priv->drm;
struct intel_connector *intel_connector;
struct intel_encoder *intel_encoder;
struct drm_connector *connector;
@@ -396,8 +397,31 @@ static void i915_hotplug_work_func(struct work_struct *work)

if (changed)
drm_kms_helper_hotplug_event(dev);
+ else if (do_recheck) {
+ spin_lock_irq(&dev_priv->irq_lock);
+ dev_priv->hotplug.event_bits |= hpd_event_bits;
+ spin_unlock_irq(&dev_priv->irq_lock);
+ schedule_delayed_work(&dev_priv->hotplug.recheck_work, msecs_to_jiffies(HPD_RECHECK_DELAY));
+ }
}

+static void i915_hotplug_work_func(struct work_struct *work)
+{
+ struct drm_i915_private *dev_priv =
+ container_of(work, struct drm_i915_private, hotplug.hotplug_work);
+ struct drm_device *dev = &dev_priv->drm;
+
+ do_i915_hotplug_check(work, dev_priv, dev, true);
+}
+
+static void i915_hotplug_recheck_func(struct work_struct *work)
+{
+ struct drm_i915_private *dev_priv =
+ container_of(work, struct drm_i915_private, hotplug.recheck_work.work);
+ struct drm_device *dev = &dev_priv->drm;
+
+ do_i915_hotplug_check(work, dev_priv, dev, false);
+}

/**
* intel_hpd_irq_handler - main hotplug irq handler
@@ -619,6 +643,8 @@ void intel_hpd_init_work(struct drm_i915_private *dev_priv)
INIT_WORK(&dev_priv->hotplug.poll_init_work, i915_hpd_poll_init_work);
INIT_DELAYED_WORK(&dev_priv->hotplug.reenable_work,
intel_hpd_irq_storm_reenable_work);
+ INIT_DELAYED_WORK(&dev_priv->hotplug.recheck_work,
+ i915_hotplug_recheck_func);
}

void intel_hpd_cancel_work(struct drm_i915_private *dev_priv)
@@ -635,6 +661,7 @@ void intel_hpd_cancel_work(struct drm_i915_private *dev_priv)
cancel_work_sync(&dev_priv->hotplug.hotplug_work);
cancel_work_sync(&dev_priv->hotplug.poll_init_work);
cancel_delayed_work_sync(&dev_priv->hotplug.reenable_work);
+ cancel_delayed_work_sync(&dev_priv->hotplug.recheck_work);
}

bool intel_hpd_disable(struct drm_i915_private *dev_priv, enum hpd_pin pin)
--
2.11.0



2018-09-19 11:46:55

by Chris Wilson

[permalink] [raw]
Subject: Re: [Intel-gfx] [PATCH] drm/i915: re-check the hotplug with a delayed work

Quoting Chris Chiu (2018-09-19 12:29:33)
> I have few ASUS laptops, X705FD(Intel i7-8565), X560UD(Intel i5-8250U)
> and X530UN(Intel i7-8550U) share the same problem. The HDMI connector
> status stays 'connected' even the HDMI cable has been unplugged.
> Then the status in sysfs would never change since then until we
> do 'xrandr' to reprobe the devices. It would also cause the audio
> output path cannot correctly swicth based on the connector status.
>
> This commit kicks off a delayed work when the status remains unchanged
> in the first hotplug event handling, which may not be the perfect
> timing in some special cases.
>
> Signed-off-by: Chris Chiu <[email protected]>
> ---
> drivers/gpu/drm/i915/i915_drv.h | 1 +
> drivers/gpu/drm/i915/intel_hotplug.c | 35 +++++++++++++++++++++++++++++++----
> 2 files changed, 32 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index d51d8574a679..78e2cf09cc10 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -286,6 +286,7 @@ struct i915_hotplug {
> } stats[HPD_NUM_PINS];
> u32 event_bits;
> struct delayed_work reenable_work;
> + struct delayed_work recheck_work;
>
> struct intel_digital_port *irq_port[I915_MAX_PORTS];
> u32 long_port_mask;
> diff --git a/drivers/gpu/drm/i915/intel_hotplug.c b/drivers/gpu/drm/i915/intel_hotplug.c
> index 43aa92beff2a..089a24588ec8 100644
> --- a/drivers/gpu/drm/i915/intel_hotplug.c
> +++ b/drivers/gpu/drm/i915/intel_hotplug.c
> @@ -349,14 +349,15 @@ static void i915_digport_work_func(struct work_struct *work)
> }
> }
>
> +#define HPD_RECHECK_DELAY (2 * 1000)

Or just (2 * HZ)

> /*
> * Handle hotplug events outside the interrupt handler proper.
> */
> -static void i915_hotplug_work_func(struct work_struct *work)
> +static void do_i915_hotplug_check(struct work_struct *work,
> + struct drm_i915_private *dev_priv,
> + struct drm_device *dev, bool do_recheck)

struct drm_i915_private == struct drm_device, no need for tautology

Local struct drm_device *dev = &i915->drm; are eliminated by the
compiler.
-Chris

2018-09-19 12:28:23

by Jani Nikula

[permalink] [raw]
Subject: Re: [PATCH] drm/i915: re-check the hotplug with a delayed work

On Wed, 19 Sep 2018, Chris Chiu <[email protected]> wrote:
> I have few ASUS laptops, X705FD(Intel i7-8565), X560UD(Intel i5-8250U)
> and X530UN(Intel i7-8550U) share the same problem. The HDMI connector
> status stays 'connected' even the HDMI cable has been unplugged.
> Then the status in sysfs would never change since then until we
> do 'xrandr' to reprobe the devices. It would also cause the audio
> output path cannot correctly swicth based on the connector status.
>
> This commit kicks off a delayed work when the status remains unchanged
> in the first hotplug event handling, which may not be the perfect
> timing in some special cases.

Please see [1].

BR,
Jani.


[1] http://patchwork.freedesktop.org/patch/msgid/[email protected]

>
> Signed-off-by: Chris Chiu <[email protected]>
> ---
> drivers/gpu/drm/i915/i915_drv.h | 1 +
> drivers/gpu/drm/i915/intel_hotplug.c | 35 +++++++++++++++++++++++++++++++----
> 2 files changed, 32 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index d51d8574a679..78e2cf09cc10 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -286,6 +286,7 @@ struct i915_hotplug {
> } stats[HPD_NUM_PINS];
> u32 event_bits;
> struct delayed_work reenable_work;
> + struct delayed_work recheck_work;
>
> struct intel_digital_port *irq_port[I915_MAX_PORTS];
> u32 long_port_mask;
> diff --git a/drivers/gpu/drm/i915/intel_hotplug.c b/drivers/gpu/drm/i915/intel_hotplug.c
> index 43aa92beff2a..089a24588ec8 100644
> --- a/drivers/gpu/drm/i915/intel_hotplug.c
> +++ b/drivers/gpu/drm/i915/intel_hotplug.c
> @@ -349,14 +349,15 @@ static void i915_digport_work_func(struct work_struct *work)
> }
> }
>
> +#define HPD_RECHECK_DELAY (2 * 1000)
> +
> /*
> * Handle hotplug events outside the interrupt handler proper.
> */
> -static void i915_hotplug_work_func(struct work_struct *work)
> +static void do_i915_hotplug_check(struct work_struct *work,
> + struct drm_i915_private *dev_priv,
> + struct drm_device *dev, bool do_recheck)
> {
> - struct drm_i915_private *dev_priv =
> - container_of(work, struct drm_i915_private, hotplug.hotplug_work);
> - struct drm_device *dev = &dev_priv->drm;
> struct intel_connector *intel_connector;
> struct intel_encoder *intel_encoder;
> struct drm_connector *connector;
> @@ -396,8 +397,31 @@ static void i915_hotplug_work_func(struct work_struct *work)
>
> if (changed)
> drm_kms_helper_hotplug_event(dev);
> + else if (do_recheck) {
> + spin_lock_irq(&dev_priv->irq_lock);
> + dev_priv->hotplug.event_bits |= hpd_event_bits;
> + spin_unlock_irq(&dev_priv->irq_lock);
> + schedule_delayed_work(&dev_priv->hotplug.recheck_work, msecs_to_jiffies(HPD_RECHECK_DELAY));
> + }
> }
>
> +static void i915_hotplug_work_func(struct work_struct *work)
> +{
> + struct drm_i915_private *dev_priv =
> + container_of(work, struct drm_i915_private, hotplug.hotplug_work);
> + struct drm_device *dev = &dev_priv->drm;
> +
> + do_i915_hotplug_check(work, dev_priv, dev, true);
> +}
> +
> +static void i915_hotplug_recheck_func(struct work_struct *work)
> +{
> + struct drm_i915_private *dev_priv =
> + container_of(work, struct drm_i915_private, hotplug.recheck_work.work);
> + struct drm_device *dev = &dev_priv->drm;
> +
> + do_i915_hotplug_check(work, dev_priv, dev, false);
> +}
>
> /**
> * intel_hpd_irq_handler - main hotplug irq handler
> @@ -619,6 +643,8 @@ void intel_hpd_init_work(struct drm_i915_private *dev_priv)
> INIT_WORK(&dev_priv->hotplug.poll_init_work, i915_hpd_poll_init_work);
> INIT_DELAYED_WORK(&dev_priv->hotplug.reenable_work,
> intel_hpd_irq_storm_reenable_work);
> + INIT_DELAYED_WORK(&dev_priv->hotplug.recheck_work,
> + i915_hotplug_recheck_func);
> }
>
> void intel_hpd_cancel_work(struct drm_i915_private *dev_priv)
> @@ -635,6 +661,7 @@ void intel_hpd_cancel_work(struct drm_i915_private *dev_priv)
> cancel_work_sync(&dev_priv->hotplug.hotplug_work);
> cancel_work_sync(&dev_priv->hotplug.poll_init_work);
> cancel_delayed_work_sync(&dev_priv->hotplug.reenable_work);
> + cancel_delayed_work_sync(&dev_priv->hotplug.recheck_work);
> }
>
> bool intel_hpd_disable(struct drm_i915_private *dev_priv, enum hpd_pin pin)

--
Jani Nikula, Intel Open Source Graphics Center