Re: [PATCH v4 1/2] drm/i915: Protect fbdev across slow or failed initialisation

[Lukas Wunner <lukas@wunner.de>]

Hi Chris,

On Thu, Mar 31, 2016 at 05:13:55PM +0100, Chris Wilson wrote:
> On Thu, Mar 31, 2016 at 07:05:21PM +0300, Joonas Lahtinen wrote:
> > On to, 2016-03-31 at 14:57 +0100, Chris Wilson wrote:
> > >  void intel_fbdev_restore_mode(struct drm_device *dev)
> > >  {
> > > -	int ret;
> > > -	struct drm_i915_private *dev_priv = dev->dev_private;
> > > -	struct intel_fbdev *ifbdev = dev_priv->fbdev;
> > > -	struct drm_fb_helper *fb_helper;
> > > +	struct intel_fbdev *ifbdev = intel_fbdev_get_if_active(dev);
> > >  
> > > -	async_synchronize_full();
> > 
> > What's with the async_synchronize_full() begin removed completely?
> 
> Because it's not just wrong, but completely broken imo.
> 
> During suspend, we want to freeze the async task not flush.
> Then here and during resume we skip the restoration of the unregistered
> fbdev, and then when the task is woken it can complete the registration
> and present the vanilla ifbdev.

No. The fbdev initialization is guaranteed to have finished before
suspend. So "we want to freeze the async task" is wrong thinking.
There is no async task to freeze.

Please read the commit message of a7442b93cf32 ("drm/i915: Fix races
on fbdev"):

"a device is never suspended until its ->probe callback (and all
asynchronous tasks it scheduled) have finished. See dpm_prepare(),
which calls wait_for_device_probe(), which calls async_synchronize_full()."

Best regards,

Lukas

> During hibernation, we really just want to cancel the task and start
> from scratch on resume.
> -Chris
> 
> -- 
> Chris Wilson, Intel Open Source Technology Centre
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx