public inbox for intel-gfx@lists.freedesktop.org
 help / color / mirror / Atom feed
* Re: [PATCH RESEND FOR CI] drm/i915: Fix races on fbdev
@ 2016-03-03 18:30 Lukas Wunner
  2016-03-04 10:17 ` Jani Nikula
  2016-03-04 12:16 ` Damien Lespiau
  0 siblings, 2 replies; 4+ messages in thread
From: Lukas Wunner @ 2016-03-03 18:30 UTC (permalink / raw)
  To: intel-gfx, Damien Lespiau; +Cc: gustav.fagerlind, Li, Weinan Z

Hi Damien, Hi Daniel,

I've submitted the patch below for the third time now in an attempt
to get CI to test it, again to no avail. This time I didn't set the
In-Reply-To header and only submitted it as a single patch instead
of as a series because I expected this might confuse CI.

Nevertheless CI misclassified it under "Series" instead of "Patches",
reports that the series consists of 0 patches and doesn't test it:
https://patchwork.freedesktop.org/series/4068/

Earlier attempts:
https://patchwork.freedesktop.org/series/3453/
https://patchwork.freedesktop.org/series/3126/

Sorry, I give up.

Best regards,

Lukas

On Thu, Mar 03, 2016 at 06:02:53PM +0100, Lukas Wunner wrote:
> The ->lastclose callback invokes intel_fbdev_restore_mode() and has
> been witnessed to run before intel_fbdev_initial_config_async()
> has finished.
> 
> We might likewise receive hotplug events or be suspended before
> we've had a chance to fully set up the fbdev.
> 
> Fix by waiting for the asynchronous thread to finish.
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=93580
> Reported-by: Gustav Fägerlind <gustav.fagerlind@gmail.com>
> Reported-by: "Li, Weinan Z" <weinan.z.li@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: stable@vger.kernel.org
> Signed-off-by: Lukas Wunner <lukas@wunner.de>
> ---
>  drivers/gpu/drm/i915/i915_dma.c    | 8 +++-----
>  drivers/gpu/drm/i915/intel_fbdev.c | 4 ++++
>  2 files changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> index 4aa3db6..9d76dfb 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -430,11 +430,9 @@ static int i915_load_modeset_init(struct drm_device *dev)
>  	 * Some ports require correctly set-up hpd registers for detection to
>  	 * work properly (leading to ghost connected connector status), e.g. VGA
>  	 * on gm45.  Hence we can only set up the initial fbdev config after hpd
> -	 * irqs are fully enabled. Now we should scan for the initial config
> -	 * only once hotplug handling is enabled, but due to screwed-up locking
> -	 * around kms/fbdev init we can't protect the fdbev initial config
> -	 * scanning against hotplug events. Hence do this first and ignore the
> -	 * tiny window where we will loose hotplug notifactions.
> +	 * irqs are fully enabled. We protect the fbdev initial config scanning
> +	 * against hotplug events by waiting in intel_fbdev_output_poll_changed
> +	 * until the asynchronous thread has finished.
>  	 */
>  	intel_fbdev_initial_config_async(dev);
>  
> diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c
> index ae9cf6f..936c3d7 100644
> --- a/drivers/gpu/drm/i915/intel_fbdev.c
> +++ b/drivers/gpu/drm/i915/intel_fbdev.c
> @@ -754,6 +754,7 @@ void intel_fbdev_set_suspend(struct drm_device *dev, int state, bool synchronous
>  	struct intel_fbdev *ifbdev = dev_priv->fbdev;
>  	struct fb_info *info;
>  
> +	async_synchronize_full();
>  	if (!ifbdev)
>  		return;
>  
> @@ -800,6 +801,8 @@ void intel_fbdev_set_suspend(struct drm_device *dev, int state, bool synchronous
>  void intel_fbdev_output_poll_changed(struct drm_device *dev)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> +
> +	async_synchronize_full();
>  	if (dev_priv->fbdev)
>  		drm_fb_helper_hotplug_event(&dev_priv->fbdev->helper);
>  }
> @@ -811,6 +814,7 @@ void intel_fbdev_restore_mode(struct drm_device *dev)
>  	struct intel_fbdev *ifbdev = dev_priv->fbdev;
>  	struct drm_fb_helper *fb_helper;
>  
> +	async_synchronize_full();
>  	if (!ifbdev)
>  		return;
>  
> -- 
> 1.8.5.2 (Apple Git-48)
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 4+ messages in thread
* [PATCH RESEND FOR CI] drm/i915: Fix races on fbdev
@ 2016-03-03 17:02 Lukas Wunner
  0 siblings, 0 replies; 4+ messages in thread
From: Lukas Wunner @ 2016-03-03 17:02 UTC (permalink / raw)
  To: intel-gfx; +Cc: gustav.fagerlind, Li, Weinan Z

The ->lastclose callback invokes intel_fbdev_restore_mode() and has
been witnessed to run before intel_fbdev_initial_config_async()
has finished.

We might likewise receive hotplug events or be suspended before
we've had a chance to fully set up the fbdev.

Fix by waiting for the asynchronous thread to finish.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=93580
Reported-by: Gustav Fägerlind <gustav.fagerlind@gmail.com>
Reported-by: "Li, Weinan Z" <weinan.z.li@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: stable@vger.kernel.org
Signed-off-by: Lukas Wunner <lukas@wunner.de>
---
 drivers/gpu/drm/i915/i915_dma.c    | 8 +++-----
 drivers/gpu/drm/i915/intel_fbdev.c | 4 ++++
 2 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 4aa3db6..9d76dfb 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -430,11 +430,9 @@ static int i915_load_modeset_init(struct drm_device *dev)
 	 * Some ports require correctly set-up hpd registers for detection to
 	 * work properly (leading to ghost connected connector status), e.g. VGA
 	 * on gm45.  Hence we can only set up the initial fbdev config after hpd
-	 * irqs are fully enabled. Now we should scan for the initial config
-	 * only once hotplug handling is enabled, but due to screwed-up locking
-	 * around kms/fbdev init we can't protect the fdbev initial config
-	 * scanning against hotplug events. Hence do this first and ignore the
-	 * tiny window where we will loose hotplug notifactions.
+	 * irqs are fully enabled. We protect the fbdev initial config scanning
+	 * against hotplug events by waiting in intel_fbdev_output_poll_changed
+	 * until the asynchronous thread has finished.
 	 */
 	intel_fbdev_initial_config_async(dev);
 
diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c
index ae9cf6f..936c3d7 100644
--- a/drivers/gpu/drm/i915/intel_fbdev.c
+++ b/drivers/gpu/drm/i915/intel_fbdev.c
@@ -754,6 +754,7 @@ void intel_fbdev_set_suspend(struct drm_device *dev, int state, bool synchronous
 	struct intel_fbdev *ifbdev = dev_priv->fbdev;
 	struct fb_info *info;
 
+	async_synchronize_full();
 	if (!ifbdev)
 		return;
 
@@ -800,6 +801,8 @@ void intel_fbdev_set_suspend(struct drm_device *dev, int state, bool synchronous
 void intel_fbdev_output_poll_changed(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
+
+	async_synchronize_full();
 	if (dev_priv->fbdev)
 		drm_fb_helper_hotplug_event(&dev_priv->fbdev->helper);
 }
@@ -811,6 +814,7 @@ void intel_fbdev_restore_mode(struct drm_device *dev)
 	struct intel_fbdev *ifbdev = dev_priv->fbdev;
 	struct drm_fb_helper *fb_helper;
 
+	async_synchronize_full();
 	if (!ifbdev)
 		return;
 
-- 
1.8.5.2 (Apple Git-48)

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2016-03-04 12:16 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-03-03 18:30 [PATCH RESEND FOR CI] drm/i915: Fix races on fbdev Lukas Wunner
2016-03-04 10:17 ` Jani Nikula
2016-03-04 12:16 ` Damien Lespiau
  -- strict thread matches above, loose matches on Subject: below --
2016-03-03 17:02 Lukas Wunner

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox