[Intel-gfx] [PATCH 19/19] drm/i915: Sync against the GuC log buffer flush work item on system suspend

[Imre Deak]

On to, 2016-08-18 at 20:05 +0530, Goel, Akash wrote:
> 
> On 8/18/2016 7:48 PM, Imre Deak wrote:
> > On to, 2016-08-18 at 19:17 +0530, Goel, Akash wrote:
> > > [...]
> > > Thanks for the inputs. Sorry not familiar with freezable WQ semantics.
> > > But after looking at code, this is what I understood :-
> > > 1. freezable Workqueues will be frozen before the system suspend
> > >     callbacks are invoked for the devices.
> > 
> > Yes.
> > 
> > > 2. Any work item queued after the WQ is marked frozen will be scheduled
> > >     later, on resume.
> > 
> > Yes.
> > 
> > > 3. But if a work item was already present in the freezable Workqueue,
> > >     before it was frozen and it did not complete, then system suspend
> > >     itself will be aborted.
> > 
> > System suspend will be aborted only if any kernel thread didn't
> > complete within a reasonable amount of time (freeze_timeout_msecs, 20
> > sec by default). Otherwise already queued items will be properly
> > waited upon and suspend will proceed.
> Sorry for getting this wrong.
> What I understood is that even if there are pending work items on
> freezable WQ after freeze_timeout_msecs, then also system suspend would 
> be performed.

In case of timeout suspend_prepare()->suspend_freeze_processes()
->freeze_kernel_threads()->try_to_freeze_tasks() will return -EBUSY and
suspend will fail.

> Sorry couldn't find an explicit/synchronous wait in kernel for the 
> pending work items for freezable WQs, but it doesn't matter.

The above try_to_freeze_tasks() will wait until
freeze_workqueues_busy() indicates that there are no work items active
on any freezable queues.

--Imre

> 
> > 
> > > 4. So if the log.flush_wq is marked as freezable, then flush of
> > >     work item will not be required for the system suspend case.
> > >     And runtime suspend case is already covered with rpm get/put
> > >     around register access in work item function.
> > 
> > Yes.
> > 
> > > 
> > > It seems there are 2 config options CONFIG_SUSPEND_FREEZER
> > 
> > This is set whenever system suspend is enabled.
> > 
> > > and
> > > CONFIG_FREEZER
> > 
> > This is set except for one platform (powerpc), where I assume freezing
> > of the tasks is achieved in a different way. In any case it doesn't
> > matter for us.
> > 
> Many thanks for providing all this info.
> 
> Will then mark the log.flush_wq as freezable.
> 
> Best regards
> Akash
> > --Imre
> > 
> > > which have to be enabled for all the above to happen.
> > > If these config options will always be enabled then probably marking
> > > log.flush_wq would work.
> > > 
> > > Please kindly confirm whether I understood correctly or not, accordingly
> > > will proceed further.
> > > 
> > > Best regards
> > > Akash
> > > 
> > > 
> > > 
> > > > --Imre
> > > >