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@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> > 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@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 -- Ville Syrjälä Intel OTC _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
