On Mon, Mar 13, 2017 at 10:28:34AM +0530, Kamble, Sagar A wrote: > > > On 3/12/2017 6:29 PM, Chris Wilson wrote: > >On Sat, Mar 11, 2017 at 04:17:34AM -0000, Patchwork wrote: > >>== Series Details == > >> > >>Series: series starting with [1/3] drm/i915/guc: Release GuC interrupts in i915_guc_submission_disable > >>URL : https://patchwork.freedesktop.org/series/21090/ > >>State : success > >> > >>== Summary == > >> > >>Series 21090v1 Series without cover letter > >>https://patchwork.freedesktop.org/api/1.0/series/21090/revisions/1/mbox/ > >> > >>Test kms_pipe_crc_basic: > >> Subgroup hang-read-crc-pipe-b: > >> dmesg-warn -> PASS (fi-byt-j1900) > >Applied, thanks for the quick fix. It is looking much neater now as well > >:) > >-Chris > > Thanks Chris. > I feel unmasking of ARAT_EXPIRED is hard coding the behavior with GuC. > Ideally rps enabling should happen post GuC load in reset path like in load time flow. The catch though is that we don't go through a rps disable sequence point across reset. We might be able to do an explict disable/enable pair now. > That way instead of hard coding interrupts to be kept as ARAT_EXPIRED we will be able to derive from bits unmasked by GuC in PMINTRMSK. > So before GuC load, we should be resetting RPS interrupts (making PMINRMSK=~0u) and then derive interrupts to be kept unmasked by Host. > And then enable RPS. Current state is fine as we know GuC isn't using other PM interrupts. (might use some of those in SLPC) But the set of bits used by guc will be fixed depending on what mode we are in, and should already be setup by time we reset. You just have a slightly more elaborate guc interrupts enable/disable sequence, I don't see that as making anything simpler or more elegant yet - but anticipate enlightenment. -Chris -- Chris Wilson, Intel Open Source Technology Centre _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx