[PATCH 0/3] drm/i915: Module unload fixes

[PATCH 0/3] drm/i915: Module unload fixes

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

This series fixes an rpm leak during driver unload, and reorganizes the fbdev
init/cleanup to make a bit more sense.

Ville Syrjälä (3):
  drm/i915: Kill intel_runtime_pm_disable()
  drm/i915: Do fbdev fini first during unload
  drm/i915: Move the fbdev async_schedule() into intel_fbdev.c

 drivers/gpu/drm/i915/i915_dma.c         |  7 +++----
 drivers/gpu/drm/i915/intel_drv.h        |  4 ++--
 drivers/gpu/drm/i915/intel_fbdev.c      |  7 ++++++-
 drivers/gpu/drm/i915/intel_runtime_pm.c | 17 -----------------
 4 files changed, 11 insertions(+), 24 deletions(-)

-- 
2.4.10

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

[PATCH 1/3] drm/i915: Kill intel_runtime_pm_disable()

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

intel_runtime_pm_disable() takes an extra rpm reference which combined
with the one we leak from intel_display_set_init_power() leaves the
usage count at <original>+1 after the driver has been unloaded.
The original ref is dropped explicitly in intel_runtime_pm_enable().
So the next time we load the driver we can no longer do runtime PM ever.

This used to work, but
commit 292b990e86ab ("drm/i915: Update power domains on readout.")
broke things by not dropping the init power domain during fbdev
teardown. Based on the comment in intel_power_domains_fini(), the
way it used to to work wasn't intentional. As in we weren't supposed
to drop the init power during driver unload. And since we no longer
do, we now leak an extra rpm reference.

So fix things by throwing intel_runtime_pm_disable() to the bin, so
that the only leaked reference comes from the init power domain.

Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Daniel Stone <daniels@collabora.com>
Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
Fixes: 292b990e86ab ("drm/i915: Update power domains on readout.")
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_runtime_pm.c | 17 -----------------
 1 file changed, 17 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
index 1017555..bdc9ed4 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -1847,21 +1847,6 @@ int intel_power_domains_init(struct drm_i915_private *dev_priv)
 	return 0;
 }
 
-static void intel_runtime_pm_disable(struct drm_i915_private *dev_priv)
-{
-	struct drm_device *dev = dev_priv->dev;
-	struct device *device = &dev->pdev->dev;
-
-	if (!HAS_RUNTIME_PM(dev))
-		return;
-
-	if (!intel_enable_rc6(dev))
-		return;
-
-	/* Make sure we're not suspended first. */
-	pm_runtime_get_sync(device);
-}
-
 /**
  * intel_power_domains_fini - finalizes the power domain structures
  * @dev_priv: i915 device instance
@@ -1872,8 +1857,6 @@ static void intel_runtime_pm_disable(struct drm_i915_private *dev_priv)
  */
 void intel_power_domains_fini(struct drm_i915_private *dev_priv)
 {
-	intel_runtime_pm_disable(dev_priv);
-
 	/* The i915.ko module is still not prepared to be loaded when
 	 * the power well is not enabled, so just enable it in case
 	 * we're going to unload/reload. */
-- 
2.4.10

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

[PATCH 2/3] drm/i915: Do fbdev fini first during unload

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

We set up fbdev last during load, so doing the fbdev cleanup should be
first.

We weren't supposed to drop the init power during driver load, but since
the fbdev teardown happened after intel_power_domains_fini() that could
have happened due in one of two ways. First it could have happened
during the modeset caused by normal fbdev cleanup. But in addition it
could have happened already via the intel_fbdev_initial_config() since
that is executed asynhronously, and the async_synchronize_full() was
done during fbdev cleanup, after intel_power_domains_fini(). All of
that got eliminated by
commit 292b990e86abc ("drm/i915: Update power domains on readout.")
since we now drop the init power synchronously during driver load.

So there is no real bug wrt. the init power anymore, but still it seems
better to do the fbdev cleanup first, before we've potentially cleaned
up something else important.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_dma.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index ffcb9c6..c58048f 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1132,6 +1132,8 @@ int i915_driver_unload(struct drm_device *dev)
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	int ret;
 
+	intel_fbdev_fini(dev);
+
 	i915_audio_component_cleanup(dev_priv);
 
 	ret = i915_gem_suspend(dev);
@@ -1154,8 +1156,6 @@ int i915_driver_unload(struct drm_device *dev)
 
 	acpi_video_unregister();
 
-	intel_fbdev_fini(dev);
-
 	drm_vblank_cleanup(dev);
 
 	intel_modeset_cleanup(dev);
-- 
2.4.10

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

[PATCH 3/3] drm/i915: Move the fbdev async_schedule() into intel_fbdev.c

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reading the driver load/unload code leaves one confused as there's
an async_schedule() in the load, but not async_synchronize_full()
in sight. In fact it's hidden inside intel_fbdev.c. So let's move the
async_schedule() into intel_fbdev.c as well so that it's next to the
async_synchronize_full(), which should make the relationship easier
to see.

Plus this way we won't schedule a nop function call when fbdev is
disabled. And we were passing a pointer to a static inline
function to async_schedule(), which seems rather dubious to me.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_dma.c    | 3 +--
 drivers/gpu/drm/i915/intel_drv.h   | 4 ++--
 drivers/gpu/drm/i915/intel_fbdev.c | 7 ++++++-
 3 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index c58048f..cae3d78 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -28,7 +28,6 @@
 
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
-#include <linux/async.h>
 #include <drm/drmP.h>
 #include <drm/drm_crtc_helper.h>
 #include <drm/drm_fb_helper.h>
@@ -437,7 +436,7 @@ static int i915_load_modeset_init(struct drm_device *dev)
 	 * scanning against hotplug events. Hence do this first and ignore the
 	 * tiny window where we will loose hotplug notifactions.
 	 */
-	async_schedule(intel_fbdev_initial_config, dev_priv);
+	intel_fbdev_initial_config_async(dev);
 
 	drm_kms_helper_poll_init(dev);
 
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 00d9882..50c78b6 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1298,7 +1298,7 @@ void intel_dvo_init(struct drm_device *dev);
 /* legacy fbdev emulation in intel_fbdev.c */
 #ifdef CONFIG_DRM_FBDEV_EMULATION
 extern int intel_fbdev_init(struct drm_device *dev);
-extern void intel_fbdev_initial_config(void *data, async_cookie_t cookie);
+extern void intel_fbdev_initial_config_async(struct drm_device *dev);
 extern void intel_fbdev_fini(struct drm_device *dev);
 extern void intel_fbdev_set_suspend(struct drm_device *dev, int state, bool synchronous);
 extern void intel_fbdev_output_poll_changed(struct drm_device *dev);
@@ -1309,7 +1309,7 @@ static inline int intel_fbdev_init(struct drm_device *dev)
 	return 0;
 }
 
-static inline void intel_fbdev_initial_config(void *data, async_cookie_t cookie)
+static inline void intel_fbdev_initial_config_async(struct drm_device *dev)
 {
 }
 
diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c
index 840d6bf..fe1fdb6 100644
--- a/drivers/gpu/drm/i915/intel_fbdev.c
+++ b/drivers/gpu/drm/i915/intel_fbdev.c
@@ -702,7 +702,7 @@ int intel_fbdev_init(struct drm_device *dev)
 	return 0;
 }
 
-void intel_fbdev_initial_config(void *data, async_cookie_t cookie)
+static void intel_fbdev_initial_config(void *data, async_cookie_t cookie)
 {
 	struct drm_i915_private *dev_priv = data;
 	struct intel_fbdev *ifbdev = dev_priv->fbdev;
@@ -711,6 +711,11 @@ void intel_fbdev_initial_config(void *data, async_cookie_t cookie)
 	drm_fb_helper_initial_config(&ifbdev->helper, ifbdev->preferred_bpp);
 }
 
+void intel_fbdev_initial_config_async(struct drm_device *dev)
+{
+	async_schedule(intel_fbdev_initial_config, to_i915(dev));
+}
+
 void intel_fbdev_fini(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
-- 
2.4.10

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

Re: [PATCH 3/3] drm/i915: Move the fbdev async_schedule() into intel_fbdev.c

[Chris Wilson]

On Fri, Nov 06, 2015 at 03:08:33PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Reading the driver load/unload code leaves one confused as there's
> an async_schedule() in the load, but not async_synchronize_full()
> in sight. In fact it's hidden inside intel_fbdev.c. So let's move the
> async_schedule() into intel_fbdev.c as well so that it's next to the
> async_synchronize_full(), which should make the relationship easier
> to see.
> 
> Plus this way we won't schedule a nop function call when fbdev is
> disabled. And we were passing a pointer to a static inline
> function to async_schedule(), which seems rather dubious to me.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/3] drm/i915: Do fbdev fini first during unload

[Chris Wilson]

On Fri, Nov 06, 2015 at 03:08:32PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> We set up fbdev last during load, so doing the fbdev cleanup should be
> first.
> 
> We weren't supposed to drop the init power during driver load, but since
> the fbdev teardown happened after intel_power_domains_fini() that could
> have happened due in one of two ways. First it could have happened
> during the modeset caused by normal fbdev cleanup. But in addition it
> could have happened already via the intel_fbdev_initial_config() since
> that is executed asynhronously, and the async_synchronize_full() was
> done during fbdev cleanup, after intel_power_domains_fini(). All of
> that got eliminated by
> commit 292b990e86abc ("drm/i915: Update power domains on readout.")
> since we now drop the init power synchronously during driver load.
> 
> So there is no real bug wrt. the init power anymore, but still it seems
> better to do the fbdev cleanup first, before we've potentially cleaned
> up something else important.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>

Hmm, a good onion and some nice cheese make a fine sandwich.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/3] drm/i915: Do fbdev fini first during unload

[Ville Syrjälä]

On Fri, Nov 06, 2015 at 03:08:32PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> We set up fbdev last during load, so doing the fbdev cleanup should be
> first.
> 
> We weren't supposed to drop the init power during driver load, but since
                                                           ^^^^

That should have said "unload".

> the fbdev teardown happened after intel_power_domains_fini() that could
> have happened due in one of two ways. First it could have happened
> during the modeset caused by normal fbdev cleanup. But in addition it
> could have happened already via the intel_fbdev_initial_config() since
> that is executed asynhronously, and the async_synchronize_full() was
> done during fbdev cleanup, after intel_power_domains_fini(). All of
> that got eliminated by
> commit 292b990e86abc ("drm/i915: Update power domains on readout.")
> since we now drop the init power synchronously during driver load.
> 
> So there is no real bug wrt. the init power anymore, but still it seems
> better to do the fbdev cleanup first, before we've potentially cleaned
> up something else important.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_dma.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> index ffcb9c6..c58048f 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -1132,6 +1132,8 @@ int i915_driver_unload(struct drm_device *dev)
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	int ret;
>  
> +	intel_fbdev_fini(dev);
> +
>  	i915_audio_component_cleanup(dev_priv);
>  
>  	ret = i915_gem_suspend(dev);
> @@ -1154,8 +1156,6 @@ int i915_driver_unload(struct drm_device *dev)
>  
>  	acpi_video_unregister();
>  
> -	intel_fbdev_fini(dev);
> -
>  	drm_vblank_cleanup(dev);
>  
>  	intel_modeset_cleanup(dev);
> -- 
> 2.4.10

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/3] drm/i915: Move the fbdev async_schedule() into intel_fbdev.c

[Jesse Barnes]

On 11/06/2015 05:08 AM, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Reading the driver load/unload code leaves one confused as there's
> an async_schedule() in the load, but not async_synchronize_full()
> in sight. In fact it's hidden inside intel_fbdev.c. So let's move the
> async_schedule() into intel_fbdev.c as well so that it's next to the
> async_synchronize_full(), which should make the relationship easier
> to see.
> 
> Plus this way we won't schedule a nop function call when fbdev is
> disabled. And we were passing a pointer to a static inline
> function to async_schedule(), which seems rather dubious to me.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_dma.c    | 3 +--
>  drivers/gpu/drm/i915/intel_drv.h   | 4 ++--
>  drivers/gpu/drm/i915/intel_fbdev.c | 7 ++++++-
>  3 files changed, 9 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> index c58048f..cae3d78 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -28,7 +28,6 @@
>  
>  #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>  
> -#include <linux/async.h>
>  #include <drm/drmP.h>
>  #include <drm/drm_crtc_helper.h>
>  #include <drm/drm_fb_helper.h>
> @@ -437,7 +436,7 @@ static int i915_load_modeset_init(struct drm_device *dev)
>  	 * scanning against hotplug events. Hence do this first and ignore the
>  	 * tiny window where we will loose hotplug notifactions.
>  	 */
> -	async_schedule(intel_fbdev_initial_config, dev_priv);
> +	intel_fbdev_initial_config_async(dev);
>  
>  	drm_kms_helper_poll_init(dev);
>  
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 00d9882..50c78b6 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1298,7 +1298,7 @@ void intel_dvo_init(struct drm_device *dev);
>  /* legacy fbdev emulation in intel_fbdev.c */
>  #ifdef CONFIG_DRM_FBDEV_EMULATION
>  extern int intel_fbdev_init(struct drm_device *dev);
> -extern void intel_fbdev_initial_config(void *data, async_cookie_t cookie);
> +extern void intel_fbdev_initial_config_async(struct drm_device *dev);
>  extern void intel_fbdev_fini(struct drm_device *dev);
>  extern void intel_fbdev_set_suspend(struct drm_device *dev, int state, bool synchronous);
>  extern void intel_fbdev_output_poll_changed(struct drm_device *dev);
> @@ -1309,7 +1309,7 @@ static inline int intel_fbdev_init(struct drm_device *dev)
>  	return 0;
>  }
>  
> -static inline void intel_fbdev_initial_config(void *data, async_cookie_t cookie)
> +static inline void intel_fbdev_initial_config_async(struct drm_device *dev)
>  {
>  }
>  
> diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c
> index 840d6bf..fe1fdb6 100644
> --- a/drivers/gpu/drm/i915/intel_fbdev.c
> +++ b/drivers/gpu/drm/i915/intel_fbdev.c
> @@ -702,7 +702,7 @@ int intel_fbdev_init(struct drm_device *dev)
>  	return 0;
>  }
>  
> -void intel_fbdev_initial_config(void *data, async_cookie_t cookie)
> +static void intel_fbdev_initial_config(void *data, async_cookie_t cookie)
>  {
>  	struct drm_i915_private *dev_priv = data;
>  	struct intel_fbdev *ifbdev = dev_priv->fbdev;
> @@ -711,6 +711,11 @@ void intel_fbdev_initial_config(void *data, async_cookie_t cookie)
>  	drm_fb_helper_initial_config(&ifbdev->helper, ifbdev->preferred_bpp);
>  }
>  
> +void intel_fbdev_initial_config_async(struct drm_device *dev)
> +{
> +	async_schedule(intel_fbdev_initial_config, to_i915(dev));
> +}
> +
>  void intel_fbdev_fini(struct drm_device *dev)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> 

Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/3] drm/i915: Move the fbdev async_schedule() into intel_fbdev.c

[Lukas Wunner]

Hi Ville,

On Fri, Nov 06, 2015 at 03:08:33PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Reading the driver load/unload code leaves one confused as there's
> an async_schedule() in the load, but not async_synchronize_full()
> in sight. In fact it's hidden inside intel_fbdev.c. So let's move the
> async_schedule() into intel_fbdev.c as well so that it's next to the
> async_synchronize_full(), which should make the relationship easier
> to see.

Hm, what do you think about solving it the other way round, i.e. moving
the async_synchronize_full() to i915_driver_unload()? Incidentally I was
working on this same part of the code and that's how I solved it. This way
it's possible to call intel_fbdev_fini() from intel_fbdev_initial_config().
With your solution this would deadlock.

Link: https://github.com/l1k/linux/commit/aa12badac846
Message-Id: <aa12badac846f24b49d83768146b62e2ac159eb3.1446987413.git.lukas@wunner.de>

Thanks,

Lukas


> Plus this way we won't schedule a nop function call when fbdev is
> disabled. And we were passing a pointer to a static inline
> function to async_schedule(), which seems rather dubious to me.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_dma.c    | 3 +--
>  drivers/gpu/drm/i915/intel_drv.h   | 4 ++--
>  drivers/gpu/drm/i915/intel_fbdev.c | 7 ++++++-
>  3 files changed, 9 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> index c58048f..cae3d78 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -28,7 +28,6 @@
>  
>  #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>  
> -#include <linux/async.h>
>  #include <drm/drmP.h>
>  #include <drm/drm_crtc_helper.h>
>  #include <drm/drm_fb_helper.h>
> @@ -437,7 +436,7 @@ static int i915_load_modeset_init(struct drm_device *dev)
>  	 * scanning against hotplug events. Hence do this first and ignore the
>  	 * tiny window where we will loose hotplug notifactions.
>  	 */
> -	async_schedule(intel_fbdev_initial_config, dev_priv);
> +	intel_fbdev_initial_config_async(dev);
>  
>  	drm_kms_helper_poll_init(dev);
>  
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 00d9882..50c78b6 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1298,7 +1298,7 @@ void intel_dvo_init(struct drm_device *dev);
>  /* legacy fbdev emulation in intel_fbdev.c */
>  #ifdef CONFIG_DRM_FBDEV_EMULATION
>  extern int intel_fbdev_init(struct drm_device *dev);
> -extern void intel_fbdev_initial_config(void *data, async_cookie_t cookie);
> +extern void intel_fbdev_initial_config_async(struct drm_device *dev);
>  extern void intel_fbdev_fini(struct drm_device *dev);
>  extern void intel_fbdev_set_suspend(struct drm_device *dev, int state, bool synchronous);
>  extern void intel_fbdev_output_poll_changed(struct drm_device *dev);
> @@ -1309,7 +1309,7 @@ static inline int intel_fbdev_init(struct drm_device *dev)
>  	return 0;
>  }
>  
> -static inline void intel_fbdev_initial_config(void *data, async_cookie_t cookie)
> +static inline void intel_fbdev_initial_config_async(struct drm_device *dev)
>  {
>  }
>  
> diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c
> index 840d6bf..fe1fdb6 100644
> --- a/drivers/gpu/drm/i915/intel_fbdev.c
> +++ b/drivers/gpu/drm/i915/intel_fbdev.c
> @@ -702,7 +702,7 @@ int intel_fbdev_init(struct drm_device *dev)
>  	return 0;
>  }
>  
> -void intel_fbdev_initial_config(void *data, async_cookie_t cookie)
> +static void intel_fbdev_initial_config(void *data, async_cookie_t cookie)
>  {
>  	struct drm_i915_private *dev_priv = data;
>  	struct intel_fbdev *ifbdev = dev_priv->fbdev;
> @@ -711,6 +711,11 @@ void intel_fbdev_initial_config(void *data, async_cookie_t cookie)
>  	drm_fb_helper_initial_config(&ifbdev->helper, ifbdev->preferred_bpp);
>  }
>  
> +void intel_fbdev_initial_config_async(struct drm_device *dev)
> +{
> +	async_schedule(intel_fbdev_initial_config, to_i915(dev));
> +}
> +
>  void intel_fbdev_fini(struct drm_device *dev)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> -- 
> 2.4.10
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/3] drm/i915: Move the fbdev async_schedule() into intel_fbdev.c

[Ville Syrjälä]

On Sun, Nov 08, 2015 at 05:44:37PM +0100, Lukas Wunner wrote:
> Hi Ville,
> 
> On Fri, Nov 06, 2015 at 03:08:33PM +0200, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Reading the driver load/unload code leaves one confused as there's
> > an async_schedule() in the load, but not async_synchronize_full()
> > in sight. In fact it's hidden inside intel_fbdev.c. So let's move the
> > async_schedule() into intel_fbdev.c as well so that it's next to the
> > async_synchronize_full(), which should make the relationship easier
> > to see.
> 
> Hm, what do you think about solving it the other way round, i.e. moving
> the async_synchronize_full() to i915_driver_unload()? Incidentally I was
> working on this same part of the code and that's how I solved it. This way
> it's possible to call intel_fbdev_fini() from intel_fbdev_initial_config().
> With your solution this would deadlock.
> 
> Link: https://github.com/l1k/linux/commit/aa12badac846
> Message-Id: <aa12badac846f24b49d83768146b62e2ac159eb3.1446987413.git.lukas@wunner.de>
> 

I think I'd still like to hide it all in intel_fbdev.c. You could just
split the fbdev_fini() into two parts; one doing the real work, and the
second one just doing async_synchronize + call the first one.

> Thanks,
> 
> Lukas
> 
> 
> > Plus this way we won't schedule a nop function call when fbdev is
> > disabled. And we were passing a pointer to a static inline
> > function to async_schedule(), which seems rather dubious to me.
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_dma.c    | 3 +--
> >  drivers/gpu/drm/i915/intel_drv.h   | 4 ++--
> >  drivers/gpu/drm/i915/intel_fbdev.c | 7 ++++++-
> >  3 files changed, 9 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> > index c58048f..cae3d78 100644
> > --- a/drivers/gpu/drm/i915/i915_dma.c
> > +++ b/drivers/gpu/drm/i915/i915_dma.c
> > @@ -28,7 +28,6 @@
> >  
> >  #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> >  
> > -#include <linux/async.h>
> >  #include <drm/drmP.h>
> >  #include <drm/drm_crtc_helper.h>
> >  #include <drm/drm_fb_helper.h>
> > @@ -437,7 +436,7 @@ static int i915_load_modeset_init(struct drm_device *dev)
> >  	 * scanning against hotplug events. Hence do this first and ignore the
> >  	 * tiny window where we will loose hotplug notifactions.
> >  	 */
> > -	async_schedule(intel_fbdev_initial_config, dev_priv);
> > +	intel_fbdev_initial_config_async(dev);
> >  
> >  	drm_kms_helper_poll_init(dev);
> >  
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > index 00d9882..50c78b6 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -1298,7 +1298,7 @@ void intel_dvo_init(struct drm_device *dev);
> >  /* legacy fbdev emulation in intel_fbdev.c */
> >  #ifdef CONFIG_DRM_FBDEV_EMULATION
> >  extern int intel_fbdev_init(struct drm_device *dev);
> > -extern void intel_fbdev_initial_config(void *data, async_cookie_t cookie);
> > +extern void intel_fbdev_initial_config_async(struct drm_device *dev);
> >  extern void intel_fbdev_fini(struct drm_device *dev);
> >  extern void intel_fbdev_set_suspend(struct drm_device *dev, int state, bool synchronous);
> >  extern void intel_fbdev_output_poll_changed(struct drm_device *dev);
> > @@ -1309,7 +1309,7 @@ static inline int intel_fbdev_init(struct drm_device *dev)
> >  	return 0;
> >  }
> >  
> > -static inline void intel_fbdev_initial_config(void *data, async_cookie_t cookie)
> > +static inline void intel_fbdev_initial_config_async(struct drm_device *dev)
> >  {
> >  }
> >  
> > diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c
> > index 840d6bf..fe1fdb6 100644
> > --- a/drivers/gpu/drm/i915/intel_fbdev.c
> > +++ b/drivers/gpu/drm/i915/intel_fbdev.c
> > @@ -702,7 +702,7 @@ int intel_fbdev_init(struct drm_device *dev)
> >  	return 0;
> >  }
> >  
> > -void intel_fbdev_initial_config(void *data, async_cookie_t cookie)
> > +static void intel_fbdev_initial_config(void *data, async_cookie_t cookie)
> >  {
> >  	struct drm_i915_private *dev_priv = data;
> >  	struct intel_fbdev *ifbdev = dev_priv->fbdev;
> > @@ -711,6 +711,11 @@ void intel_fbdev_initial_config(void *data, async_cookie_t cookie)
> >  	drm_fb_helper_initial_config(&ifbdev->helper, ifbdev->preferred_bpp);
> >  }
> >  
> > +void intel_fbdev_initial_config_async(struct drm_device *dev)
> > +{
> > +	async_schedule(intel_fbdev_initial_config, to_i915(dev));
> > +}
> > +
> >  void intel_fbdev_fini(struct drm_device *dev)
> >  {
> >  	struct drm_i915_private *dev_priv = dev->dev_private;
> > -- 
> > 2.4.10
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/3] drm/i915: Move the fbdev async_schedule() into intel_fbdev.c

[Lukas Wunner]

Hi Ville,

On Mon, Nov 09, 2015 at 01:00:50PM +0200, Ville Syrjälä wrote:
> On Sun, Nov 08, 2015 at 05:44:37PM +0100, Lukas Wunner wrote:
> > Hi Ville,
> > 
> > On Fri, Nov 06, 2015 at 03:08:33PM +0200, ville.syrjala@linux.intel.com wrote:
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > 
> > > Reading the driver load/unload code leaves one confused as there's
> > > an async_schedule() in the load, but not async_synchronize_full()
> > > in sight. In fact it's hidden inside intel_fbdev.c. So let's move the
> > > async_schedule() into intel_fbdev.c as well so that it's next to the
> > > async_synchronize_full(), which should make the relationship easier
> > > to see.
> > 
> > Hm, what do you think about solving it the other way round, i.e. moving
> > the async_synchronize_full() to i915_driver_unload()? Incidentally I was
> > working on this same part of the code and that's how I solved it. This way
> > it's possible to call intel_fbdev_fini() from intel_fbdev_initial_config().
> > With your solution this would deadlock.
> > 
> > Link: https://github.com/l1k/linux/commit/aa12badac846
> > Message-Id: <aa12badac846f24b49d83768146b62e2ac159eb3.1446987413.git.lukas@wunner.de>
> > 
> 
> I think I'd still like to hide it all in intel_fbdev.c. You could just
> split the fbdev_fini() into two parts; one doing the real work, and the
> second one just doing async_synchronize + call the first one.

Looking at this with a fresh pair of eyeballs I realized I could simply
call async_synchronize_full() conditionally if (!current_is_async()),
thereby differentiating between fbdev_fini() being called from
i915_driver_unload() versus intel_fbdev_initial_config().
If your patch gets pushed, I think I'll rebase and solve it like that.

Thanks for your input,

Lukas

> 
> > Thanks,
> > 
> > Lukas
> > 
> > 
> > > Plus this way we won't schedule a nop function call when fbdev is
> > > disabled. And we were passing a pointer to a static inline
> > > function to async_schedule(), which seems rather dubious to me.
> > > 
> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/i915_dma.c    | 3 +--
> > >  drivers/gpu/drm/i915/intel_drv.h   | 4 ++--
> > >  drivers/gpu/drm/i915/intel_fbdev.c | 7 ++++++-
> > >  3 files changed, 9 insertions(+), 5 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> > > index c58048f..cae3d78 100644
> > > --- a/drivers/gpu/drm/i915/i915_dma.c
> > > +++ b/drivers/gpu/drm/i915/i915_dma.c
> > > @@ -28,7 +28,6 @@
> > >  
> > >  #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> > >  
> > > -#include <linux/async.h>
> > >  #include <drm/drmP.h>
> > >  #include <drm/drm_crtc_helper.h>
> > >  #include <drm/drm_fb_helper.h>
> > > @@ -437,7 +436,7 @@ static int i915_load_modeset_init(struct drm_device *dev)
> > >  	 * scanning against hotplug events. Hence do this first and ignore the
> > >  	 * tiny window where we will loose hotplug notifactions.
> > >  	 */
> > > -	async_schedule(intel_fbdev_initial_config, dev_priv);
> > > +	intel_fbdev_initial_config_async(dev);
> > >  
> > >  	drm_kms_helper_poll_init(dev);
> > >  
> > > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > > index 00d9882..50c78b6 100644
> > > --- a/drivers/gpu/drm/i915/intel_drv.h
> > > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > > @@ -1298,7 +1298,7 @@ void intel_dvo_init(struct drm_device *dev);
> > >  /* legacy fbdev emulation in intel_fbdev.c */
> > >  #ifdef CONFIG_DRM_FBDEV_EMULATION
> > >  extern int intel_fbdev_init(struct drm_device *dev);
> > > -extern void intel_fbdev_initial_config(void *data, async_cookie_t cookie);
> > > +extern void intel_fbdev_initial_config_async(struct drm_device *dev);
> > >  extern void intel_fbdev_fini(struct drm_device *dev);
> > >  extern void intel_fbdev_set_suspend(struct drm_device *dev, int state, bool synchronous);
> > >  extern void intel_fbdev_output_poll_changed(struct drm_device *dev);
> > > @@ -1309,7 +1309,7 @@ static inline int intel_fbdev_init(struct drm_device *dev)
> > >  	return 0;
> > >  }
> > >  
> > > -static inline void intel_fbdev_initial_config(void *data, async_cookie_t cookie)
> > > +static inline void intel_fbdev_initial_config_async(struct drm_device *dev)
> > >  {
> > >  }
> > >  
> > > diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c
> > > index 840d6bf..fe1fdb6 100644
> > > --- a/drivers/gpu/drm/i915/intel_fbdev.c
> > > +++ b/drivers/gpu/drm/i915/intel_fbdev.c
> > > @@ -702,7 +702,7 @@ int intel_fbdev_init(struct drm_device *dev)
> > >  	return 0;
> > >  }
> > >  
> > > -void intel_fbdev_initial_config(void *data, async_cookie_t cookie)
> > > +static void intel_fbdev_initial_config(void *data, async_cookie_t cookie)
> > >  {
> > >  	struct drm_i915_private *dev_priv = data;
> > >  	struct intel_fbdev *ifbdev = dev_priv->fbdev;
> > > @@ -711,6 +711,11 @@ void intel_fbdev_initial_config(void *data, async_cookie_t cookie)
> > >  	drm_fb_helper_initial_config(&ifbdev->helper, ifbdev->preferred_bpp);
> > >  }
> > >  
> > > +void intel_fbdev_initial_config_async(struct drm_device *dev)
> > > +{
> > > +	async_schedule(intel_fbdev_initial_config, to_i915(dev));
> > > +}
> > > +
> > >  void intel_fbdev_fini(struct drm_device *dev)
> > >  {
> > >  	struct drm_i915_private *dev_priv = dev->dev_private;
> > > -- 
> > > 2.4.10
> > > 
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Ville Syrjälä
> Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/3] drm/i915: Kill intel_runtime_pm_disable()

[Jesse Barnes]

On 11/06/2015 05:08 AM, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> intel_runtime_pm_disable() takes an extra rpm reference which combined
> with the one we leak from intel_display_set_init_power() leaves the
> usage count at <original>+1 after the driver has been unloaded.
> The original ref is dropped explicitly in intel_runtime_pm_enable().
> So the next time we load the driver we can no longer do runtime PM ever.
> 
> This used to work, but
> commit 292b990e86ab ("drm/i915: Update power domains on readout.")
> broke things by not dropping the init power domain during fbdev
> teardown. Based on the comment in intel_power_domains_fini(), the
> way it used to to work wasn't intentional. As in we weren't supposed
> to drop the init power during driver unload. And since we no longer
> do, we now leak an extra rpm reference.
> 
> So fix things by throwing intel_runtime_pm_disable() to the bin, so
> that the only leaked reference comes from the init power domain.
> 
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Daniel Stone <daniels@collabora.com>
> Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
> Fixes: 292b990e86ab ("drm/i915: Update power domains on readout.")
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_runtime_pm.c | 17 -----------------
>  1 file changed, 17 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index 1017555..bdc9ed4 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -1847,21 +1847,6 @@ int intel_power_domains_init(struct drm_i915_private *dev_priv)
>  	return 0;
>  }
>  
> -static void intel_runtime_pm_disable(struct drm_i915_private *dev_priv)
> -{
> -	struct drm_device *dev = dev_priv->dev;
> -	struct device *device = &dev->pdev->dev;
> -
> -	if (!HAS_RUNTIME_PM(dev))
> -		return;
> -
> -	if (!intel_enable_rc6(dev))
> -		return;
> -
> -	/* Make sure we're not suspended first. */
> -	pm_runtime_get_sync(device);
> -}
> -
>  /**
>   * intel_power_domains_fini - finalizes the power domain structures
>   * @dev_priv: i915 device instance
> @@ -1872,8 +1857,6 @@ static void intel_runtime_pm_disable(struct drm_i915_private *dev_priv)
>   */
>  void intel_power_domains_fini(struct drm_i915_private *dev_priv)
>  {
> -	intel_runtime_pm_disable(dev_priv);
> -
>  	/* The i915.ko module is still not prepared to be loaded when
>  	 * the power well is not enabled, so just enable it in case
>  	 * we're going to unload/reload. */
> 

Yeah I guess this is fine.  Will we still disable RPM on unload?  What's
the expected behavior here?  Cc'ing Rafael.

Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/3] drm/i915: Kill intel_runtime_pm_disable()

[Rafael J. Wysocki]

On Tuesday, November 10, 2015 09:20:56 AM Jesse Barnes wrote:
> On 11/06/2015 05:08 AM, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > intel_runtime_pm_disable() takes an extra rpm reference which combined
> > with the one we leak from intel_display_set_init_power() leaves the
> > usage count at <original>+1 after the driver has been unloaded.
> > The original ref is dropped explicitly in intel_runtime_pm_enable().
> > So the next time we load the driver we can no longer do runtime PM ever.
> > 
> > This used to work, but
> > commit 292b990e86ab ("drm/i915: Update power domains on readout.")
> > broke things by not dropping the init power domain during fbdev
> > teardown. Based on the comment in intel_power_domains_fini(), the
> > way it used to to work wasn't intentional. As in we weren't supposed
> > to drop the init power during driver unload. And since we no longer
> > do, we now leak an extra rpm reference.
> > 
> > So fix things by throwing intel_runtime_pm_disable() to the bin, so
> > that the only leaked reference comes from the init power domain.
> > 
> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > Cc: Daniel Stone <daniels@collabora.com>
> > Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
> > Fixes: 292b990e86ab ("drm/i915: Update power domains on readout.")
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_runtime_pm.c | 17 -----------------
> >  1 file changed, 17 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > index 1017555..bdc9ed4 100644
> > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > @@ -1847,21 +1847,6 @@ int intel_power_domains_init(struct drm_i915_private *dev_priv)
> >  	return 0;
> >  }
> >  
> > -static void intel_runtime_pm_disable(struct drm_i915_private *dev_priv)
> > -{
> > -	struct drm_device *dev = dev_priv->dev;
> > -	struct device *device = &dev->pdev->dev;
> > -
> > -	if (!HAS_RUNTIME_PM(dev))
> > -		return;
> > -
> > -	if (!intel_enable_rc6(dev))
> > -		return;
> > -
> > -	/* Make sure we're not suspended first. */
> > -	pm_runtime_get_sync(device);
> > -}
> > -
> >  /**
> >   * intel_power_domains_fini - finalizes the power domain structures
> >   * @dev_priv: i915 device instance
> > @@ -1872,8 +1857,6 @@ static void intel_runtime_pm_disable(struct drm_i915_private *dev_priv)
> >   */
> >  void intel_power_domains_fini(struct drm_i915_private *dev_priv)
> >  {
> > -	intel_runtime_pm_disable(dev_priv);
> > -
> >  	/* The i915.ko module is still not prepared to be loaded when
> >  	 * the power well is not enabled, so just enable it in case
> >  	 * we're going to unload/reload. */
> > 
> 
> Yeah I guess this is fine.  Will we still disable RPM on unload?  What's
> the expected behavior here?  Cc'ing Rafael.

If that's a PCI device, you don't have to do anything with it.  In fact,
you aren't expected to do anything with it even.

Thanks,
Rafael

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

Re: [PATCH 3/3] drm/i915: Move the fbdev async_schedule() into intel_fbdev.c

[Ville Syrjälä]

On Tue, Nov 10, 2015 at 05:27:55PM +0100, Lukas Wunner wrote:
> Hi Ville,
> 
> On Mon, Nov 09, 2015 at 01:00:50PM +0200, Ville Syrjälä wrote:
> > On Sun, Nov 08, 2015 at 05:44:37PM +0100, Lukas Wunner wrote:
> > > Hi Ville,
> > > 
> > > On Fri, Nov 06, 2015 at 03:08:33PM +0200, ville.syrjala@linux.intel.com wrote:
> > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > 
> > > > Reading the driver load/unload code leaves one confused as there's
> > > > an async_schedule() in the load, but not async_synchronize_full()
> > > > in sight. In fact it's hidden inside intel_fbdev.c. So let's move the
> > > > async_schedule() into intel_fbdev.c as well so that it's next to the
> > > > async_synchronize_full(), which should make the relationship easier
> > > > to see.
> > > 
> > > Hm, what do you think about solving it the other way round, i.e. moving
> > > the async_synchronize_full() to i915_driver_unload()? Incidentally I was
> > > working on this same part of the code and that's how I solved it. This way
> > > it's possible to call intel_fbdev_fini() from intel_fbdev_initial_config().
> > > With your solution this would deadlock.
> > > 
> > > Link: https://github.com/l1k/linux/commit/aa12badac846
> > > Message-Id: <aa12badac846f24b49d83768146b62e2ac159eb3.1446987413.git.lukas@wunner.de>
> > > 
> > 
> > I think I'd still like to hide it all in intel_fbdev.c. You could just
> > split the fbdev_fini() into two parts; one doing the real work, and the
> > second one just doing async_synchronize + call the first one.
> 
> Looking at this with a fresh pair of eyeballs I realized I could simply
> call async_synchronize_full() conditionally if (!current_is_async()),
> thereby differentiating between fbdev_fini() being called from
> i915_driver_unload() versus intel_fbdev_initial_config().
> If your patch gets pushed, I think I'll rebase and solve it like that.
 
OK.

Series pushed to dinq. Thanks for the reviews.

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx