[Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915/guc: GuC suspend path cleanup

[Sujaritha]

On 3/21/19 1:08 PM, Chris Wilson wrote:
> Quoting Sujaritha (2019-03-21 19:41:17)
>> On 3/21/19 12:37 PM, Chris Wilson wrote:
>>> Quoting Patchwork (2019-03-21 19:26:27)
>>>> == Series Details ==
>>>>
>>>> Series: drm/i915/guc: GuC suspend path cleanup
>>>> URL   : https://patchwork.freedesktop.org/series/58370/
>>>> State : failure
>>>>
>>>> == Summary ==
>>>>
>>>> CI Bug Log - changes from CI_DRM_5789 -> Patchwork_12553
>>>> ====================================================
>>>>
>>>> Summary
>>>> -------
>>>>
>>>>     **FAILURE**
>>>>
>>>>     Serious unknown changes coming with Patchwork_12553 absolutely need to be
>>>>     verified manually.
>>>>     
>>>>     If you think the reported changes have nothing to do with the changes
>>>>     introduced in Patchwork_12553, please notify your bug team to allow them
>>>>     to document this new failure mode, which will reduce false positives in CI.
>>>>
>>>>     External URL: https://patchwork.freedesktop.org/api/1.0/series/58370/revisions/1/mbox/
>>>>
>>>> Possible new issues
>>>> -------------------
>>>>
>>>>     Here are the unknown changes that may have been introduced in Patchwork_12553:
>>>>
>>>> ### IGT changes ###
>>>>
>>>> #### Possible regressions ####
>>>>
>>>>     * igt at gem_exec_suspend@basic-s3:
>>>>       - fi-apl-guc:         PASS -> DMESG-WARN
>>> That says we turned the guc off before completing the idle sequence, so
>>> the intel_uc_suspend() has to be after the flush_workqueues.
>>> -Chris
>>
>> But shouldn't this be taken care of by the switch_to_kernel_context_sync ?
> Hmm, no, we can't flush the retire worker there (because of
> struct_mutex). But it should be taken care! Something to work on :)
>
>> And would be better have uc_suspend after drain_delayed_work instead of
>>
>> just after flush_workqueue ?
> Basically right at the end; you don't need struct_mutex right? And the
> assert that the gt is !awake fits in with the intent to switch guc off.
> -Chris


Yes at the end, before the GEM_BUG_ON. The struct_mutex is not required.

-Sujaritha