[Intel-gfx] ✓ Fi.CI.BAT: success for series starting with [1/3] drm/i915/guc: Release GuC interrupts in i915_guc_submission_disable

[Kamble, Sagar A]

On 3/13/2017 3:17 PM, Chris Wilson wrote:
> 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.

Yes.

>
>> 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

Agree that bits used will be fixed and no need to dynamically determine.
Other bits if needed will be configured by GuC SLPC and in that case Host
RPS flows will not update the registers. So the current implementation looks fine.

Thanks
Sagar