[PATCH v3 1/1] drm/i915: Sanitize PMINTRMSK and enable powersave post GuC load on reset

[Kamble, Sagar A]

On 9/28/2016 1:41 PM, Chris Wilson wrote:
> On Wed, Sep 28, 2016 at 11:30:37AM +0530, Sagar Arun Kamble wrote:
>> v3: Sanitizing PMINTRMSK in reset_rps_interrupts. (Chris)
>>      Moving enable_gt_powersave past gem_init_hw in reset path for GuC
>>      load to complete. Commit message updated.
>> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
>> index 9c15432..96e31c9 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.c
>> +++ b/drivers/gpu/drm/i915/i915_drv.c
>> @@ -1793,6 +1793,12 @@ void i915_reset(struct drm_i915_private *dev_priv)
>>   		goto error;
>>   	}
>>   
>> +	if (dev_priv->gt.awake) {
>> +		intel_enable_gt_powersave(dev_priv);
>> +		if (INTEL_GEN(dev_priv) >= 6)
>> +			gen6_rps_busy(dev_priv);
>> +	}
>> +
>>   wakeup:
>>   	wake_up_bit(&error->flags, I915_RESET_IN_PROGRESS);
>>   	return;
>> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
>> index 1418c1c..42b7634 100644
>> --- a/drivers/gpu/drm/i915/i915_gem.c
>> +++ b/drivers/gpu/drm/i915/i915_gem.c
>> @@ -2629,12 +2629,7 @@ void i915_gem_reset(struct drm_i915_private *dev_priv)
>>   
>>   	i915_gem_restore_fences(&dev_priv->drm);
>>   
>> -	if (dev_priv->gt.awake) {
>> -		intel_sanitize_gt_powersave(dev_priv);
>> -		intel_enable_gt_powersave(dev_priv);
>> -		if (INTEL_GEN(dev_priv) >= 6)
>> -			gen6_rps_busy(dev_priv);
>> -	}
> Hmm, don't like plonking this into the driver level as this is all about
> redoing the operations that were performed at the GEM level.
>
> i915_gem_init_hw() with a comment would be a working compromise.
previously flow was: gem_reset (sanitize and enable gt powersave), 
gem_init_hw (init rings,guc setup)
I changed now it to: gem_reset (sanitize gt powersave), gem_init_hw, 
enable gt powersave
Taking enable to gem_init_hw will break the driver load path as 
init_gt_powersave is done post gem_init_hw.
Will it be OK for enable gt powersave to start post gem_init_hw as 
requests that were in flight would get submitted as part of 
gem_init_hw(elsp_submit_ports).
Or should we not handle reset case w.r.t pm_intr_keep sanitize in guc 
load path?
> -Chris
>