Hi Ville, On Fri, Nov 06, 2015 at 03:08:33PM +0200, ville.syrjala@xxxxxxxxxxxxxxx wrote: > From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > > 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@xxxxxxxxx> 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@xxxxxxxxxxxxxxx> > --- > 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@xxxxxxxxxxxxxxxxxxxxx > http://lists.freedesktop.org/mailman/listinfo/intel-gfx _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
