All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jani Nikula <jani.nikula@intel.com>
To: joonas.lahtinen@linux.intel.com, rodrigo.vivi@intel.com,
	airlied@linux.ie
Cc: Chris Chiu <chiu@endlessm.com>,
	intel-gfx@lists.freedesktop.org, linux@endlessm.com,
	linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org
Subject: Re: [PATCH] drm/i915: re-check the hotplug with a delayed work
Date: Wed, 19 Sep 2018 15:27:05 +0300	[thread overview]
Message-ID: <87h8ily8o6.fsf@intel.com> (raw)
In-Reply-To: <20180919112933.47382-1-chiu@endlessm.com>

On Wed, 19 Sep 2018, Chris Chiu <chiu@endlessm.com> 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/87k1nhy9ie.fsf@intel.com

>
> Signed-off-by: Chris Chiu <chiu@endlessm.com>
> ---
>  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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

WARNING: multiple messages have this Message-ID (diff)
From: Jani Nikula <jani.nikula@intel.com>
To: Chris Chiu <chiu@endlessm.com>,
	joonas.lahtinen@linux.intel.com, rodrigo.vivi@intel.com,
	airlied@linux.ie
Cc: intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org,
	linux-kernel@vger.kernel.org, linux@endlessm.com,
	Chris Chiu <chiu@endlessm.com>
Subject: Re: [PATCH] drm/i915: re-check the hotplug with a delayed work
Date: Wed, 19 Sep 2018 15:27:05 +0300	[thread overview]
Message-ID: <87h8ily8o6.fsf@intel.com> (raw)
In-Reply-To: <20180919112933.47382-1-chiu@endlessm.com>

On Wed, 19 Sep 2018, Chris Chiu <chiu@endlessm.com> 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/87k1nhy9ie.fsf@intel.com

>
> Signed-off-by: Chris Chiu <chiu@endlessm.com>
> ---
>  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

  parent reply	other threads:[~2018-09-19 12:27 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-09-19 11:29 [PATCH] drm/i915: re-check the hotplug with a delayed work Chris Chiu
2018-09-19 11:29 ` Chris Chiu
2018-09-19 11:43 ` Chris Wilson
2018-09-19 11:43   ` [Intel-gfx] " Chris Wilson
2018-09-19 12:17 ` ✗ Fi.CI.BAT: failure for " Patchwork
2018-09-19 12:27 ` Jani Nikula [this message]
2018-09-19 12:27   ` [PATCH] " Jani Nikula

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87h8ily8o6.fsf@intel.com \
    --to=jani.nikula@intel.com \
    --cc=airlied@linux.ie \
    --cc=chiu@endlessm.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=joonas.lahtinen@linux.intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@endlessm.com \
    --cc=rodrigo.vivi@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.