On Wed, Aug 13, 2014 at 01:26:40PM +0200, Daniel Vetter wrote:
> On Tue, Aug 12, 2014 at 03:15:17PM +0100, Chris Wilson wrote:
> > Rather than take and release the console_lock() around a non-existent
> > DRM_I915_FBDEV, move the lock acquisation into the callee where it will
> > be compiled out by the config option entirely. This includes moving the
> > deferred fb_set_suspend() dance and encapsulating it entirely within
> > intel_fbdev.c.
> >
> > Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> > Cc: Daniel Vetter <daniel.vetter@xxxxxxxx>
> > ---
> > drivers/gpu/drm/i915/i915_dma.c | 5 ++---
> > drivers/gpu/drm/i915/i915_drv.c | 26 +-------------------------
> > drivers/gpu/drm/i915/i915_drv.h | 8 --------
> > drivers/gpu/drm/i915/intel_fbdev.c | 33 +++++++++++++++++++++++++++++++++
> > 4 files changed, 36 insertions(+), 36 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> > index 49149406fcd8..d0b3411fc13c 100644
> > --- a/drivers/gpu/drm/i915/i915_dma.c
> > +++ b/drivers/gpu/drm/i915/i915_dma.c
> > @@ -1339,8 +1339,6 @@ static int i915_load_modeset_init(struct drm_device *dev)
> > if (ret)
> > goto cleanup_irq;
> >
> > - INIT_WORK(&dev_priv->console_resume_work, intel_console_resume);
> > -
> > intel_modeset_gem_init(dev);
> >
> > /* Always safe in the mode setting case. */
> > @@ -1835,6 +1833,8 @@ int i915_driver_unload(struct drm_device *dev)
> > return ret;
> > }
> >
> > + flush_scheduled_work();
>
> Tbh I prefer the explicit work flushing and keeping it in dev_priv. We can
> shovel it into the I915_FBDEV #ifdef that's already there though.
>
> > +
> > i915_perf_unregister(dev);
> > intel_fini_runtime_pm(dev_priv);
> >
> > @@ -1859,7 +1859,6 @@ int i915_driver_unload(struct drm_device *dev)
> > if (drm_core_check_feature(dev, DRIVER_MODESET)) {
> > intel_fbdev_fini(dev);
> > intel_modeset_cleanup(dev);
> > - cancel_work_sync(&dev_priv->console_resume_work);
> >
> > /*
> > * free the memory space allocated for the child device
> > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> > index 35b119072c11..9adafc7c5819 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.c
> > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > @@ -558,9 +558,7 @@ static int i915_drm_freeze(struct drm_device *dev)
> > intel_uncore_forcewake_reset(dev, false);
> > intel_opregion_fini(dev);
> >
> > - console_lock();
> > intel_fbdev_set_suspend(dev, FBINFO_STATE_SUSPENDED);
>
> This one here must be synchronous.
Right, but notice that we synchronize the work afterwards. I thought if
we were careful enough to call the waiter that waited for additional
work items to be completed, it would be sufficient.
> > +static void intel_fbdev_set_suspend_worker(struct work_struct *work)
> > +{
> > + struct set_suspend_work *ss = container_of(work, typeof(*ss), work);
> > + intel_fbdev_set_suspend(ss->dev, ss->state);
>
> This still trylocks so ends up busy-looping through launching more work
> items. Probably better also do this synchronously.
Considered that, but I decided that trying to keep the locking
straightforward was better.
I'll look at putting the work item back onto the dev_priv so that we can
cleanly flush it.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
