Quoting Sujaritha (2019-03-21 20:02:36) > > 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@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. I'd go just after. The assert is that GEM is idle, so ready to suspend the FW. Worksforme. -Chris _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx