[Intel-gfx] [P v4 02/11] drm/i915/guc: Move GuC boot param initialization out of xfer

[Michal Wajdeczko]

On Thu, 12 Oct 2017 00:55:29 +0200, Daniele Ceraolo Spurio  
<daniele.ceraolospurio at intel.com> wrote:

>
>
> On 10/10/17 07:51, Michal Wajdeczko wrote:
>> We want to keep ucode xfer functions separate from other
>> initialization. Once separated, add explicit forcewake.

...

>> +	intel_uncore_forcewake_get(dev_priv, FORCEWAKE_ALL);
>
> I don't think we need this explicit forcewake if we use I915_WRITE(),  
> because that should already wake the required wells. If you want to  
> explicitly take forcewake then we can use I915_WRITE_FW(). Also,  
> FORCEWAKE_BLITTER should be enough.

I would stay with both explicit forcewake and I915_WRITE() first to
be aligned with similar code in send_mmio and second to follow usage
rules listed in description of I915_WRITE_FW().

> I'd also add a comment to say that the register are power context saved  
> so it's ok to release forcewake here and take it again at xfer time.
>

...

>> +++ b/drivers/gpu/drm/i915/intel_uc.c
>> @@ -195,6 +195,7 @@ int intel_uc_init_hw(struct drm_i915_private  
>> *dev_priv)
>>   			goto err_submission;
>>     		intel_huc_init_hw(&dev_priv->huc);
>> +		intel_guc_init_params(guc);
>
> Is the value of the registers lost/corrupted on guc reset? if not we  
> could just move this outside the retry loop to avoid doing it more than  
> once.
>

In theory they should survive as they are defined as software registers.
However, I'm not sure we can trust Guc (that just failed to load) that
it didn't corrupt them. Also as we retry only for Gen9 cost of this extra
initialization should be minimal.

Thanks for the review,
Michal