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

[Goel, Akash]

On 8/18/2016 8:25 PM, Imre Deak wrote:
> 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.
>
So sorry, there was a typo in my last mail, instead of writing 'system 
suspend would be aborted', I wrote 'system suspend would be performed'.

>> 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.
>
Thanks much for clarifying. I will go through that function again.

Best regards
Akash
> --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
>>>>>