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

[Chris Wilson]

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