[PATCH 2/2] drm/i915: Handle GEN6_PMINTRMSK for proper masking of RPS interrupts with GuC submission

[Kamble, Sagar A]

Thanks for the review.

Sorry I did not check my trybot mails and posted this patch on public 
ML. Will incorporate the suggestions and post new version.


On 9/28/2016 1:18 AM, Chris Wilson wrote:
> On Tue, Sep 27, 2016 at 11:55:43PM +0530, Sagar Arun Kamble wrote:
>> Driver needs to ensure that it doesn't mask the PM interrupts, which are
>> unmasked/needed by GuC firmware. For that, Driver maintains a bitmask of
>> interrupts to be kept enabled, pm_intr_keep.
> This is still not right. I agree that we are missing a sanitize during
> pm/rps setup, but if the guc is imposing new constraints, then it is
> responsible for the registers as well.
Ok. Will add the sanitization in reset_rps_interrupts.
Will have to ensure RPS gets enabled post GuC load though.
>   
>> By default, RP Up/Down Threshold Interrupt bits (and others) in
>> GEN6_PMINTRMSK register were unmasked (by BIOS) before GuC Load. Hence
>> Driver was keeping them unmasked always assuming GuC needed them.
>> As an optimization, Driver needs to mask these bits in order to
>> avoid redundant UP threshold interrupts when frequency is set to maximum,
>> RP0 or Down threshold interrupts when frequency is set to minimum, RPn.
>>
>> This patch will mask all bits in GEN6_PMINTRMSK before GuC is loaded to
>> ensure pm_intr_keep reflects only interrupts that are needed by GuC. Post
>> GuC load, writing cur_freq will restore bits other than pm_intr_keep.
>>
>> Cc: Chris Wilson <chris at chris-wilson.co.uk>
>> Signed-off-by: Sagar Arun Kamble <sagar.a.kamble at intel.com>
>> ---
>>   drivers/gpu/drm/i915/i915_gem.c  | 13 +++++++++++++
>>   drivers/gpu/drm/i915/intel_drv.h |  1 +
>>   drivers/gpu/drm/i915/intel_pm.c  |  2 +-
>>   3 files changed, 15 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
>> index 1418c1c..cafb4d0 100644
>> --- a/drivers/gpu/drm/i915/i915_gem.c
>> +++ b/drivers/gpu/drm/i915/i915_gem.c
>> @@ -4399,12 +4399,25 @@ i915_gem_init_hw(struct drm_device *dev)
>>   
>>   	intel_mocs_init_l3cc_table(dev);
>>   
>> +	/*
>> +	 * Mask all interrupts to know which interrupts are needed by GuC.
>> +	 * Restore host side interrupt masks post load.
>> +	*/
>> +	I915_WRITE(GEN6_PMINTRMSK, gen6_sanitize_rps_pm_mask(dev_priv, ~0u));
>> +
>>   	/* We can't enable contexts until all firmware is loaded */
>>   	ret = intel_guc_setup(dev);
>>   	if (ret)
>>   		goto out;
>>   
>>   out:
>> +	/*
>> +	 * Below write will ensure mask for RPS interrupts is restored back
>> +	 * w.r.t cur_freq, particularly post reset.
>> +	 */
>> +	I915_WRITE(GEN6_PMINTRMSK,
>> +		   gen6_rps_pm_mask(dev_priv, dev_priv->rps.cur_freq));
> Which is pointless as we should do that anyway when enabling rps interrupts.
During reset, enabling of gt power save happened prior to guc load hence 
updating focefully here.
Will change based on the sanitization change.
> -Chris
>