[Intel-gfx] [PATCH v4 2/5] drm/i915/guc: Wait for ucode DMA transfer completion

[Michal Wajdeczko]

On Wed, 08 Nov 2017 21:36:25 +0100, Chris Wilson  
<chris at chris-wilson.co.uk> wrote:

> Quoting Michal Wajdeczko (2017-11-03 15:18:13)
>> We silently assumed that DMA transfer will be completed
>> within assumed timeout and thus we were waiting only at
>> last step for GuC to become ready. Add intermediate wait
>> to catch unexpected delays in DMA transfer.
>>
>> Signed-off-by: Michal Wajdeczko <michal.wajdeczko at intel.com>
>> Cc: Chris Wilson <chris at chris-wilson.co.uk>
>> Cc: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
>> Cc: Sagar Arun Kamble <sagar.a.kamble at intel.com>
>> Reviewed-by: Sagar Arun Kamble <sagar.a.kamble at intel.com>
>> ---
>>  drivers/gpu/drm/i915/intel_guc_fw.c | 9 ++++++++-
>>  1 file changed, 8 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_guc_fw.c  
>> b/drivers/gpu/drm/i915/intel_guc_fw.c
>> index c4f4526..74a61fe 100644
>> --- a/drivers/gpu/drm/i915/intel_guc_fw.c
>> +++ b/drivers/gpu/drm/i915/intel_guc_fw.c
>> @@ -160,6 +160,8 @@ static int guc_xfer_ucode(struct intel_guc *guc,  
>> struct i915_vma *vma)
>>         struct drm_i915_private *dev_priv = guc_to_i915(guc);
>>         struct intel_uc_fw *guc_fw = &guc->fw;
>>         unsigned long offset;
>> +       u32 status;
>> +       int ret;
>>
>>         /*
>>          * The header plus uCode will be copied to WOPCM via DMA,  
>> excluding any
>> @@ -182,7 +184,12 @@ static int guc_xfer_ucode(struct intel_guc *guc,  
>> struct i915_vma *vma)
>>         /* Finally start the DMA */
>>         I915_WRITE(DMA_CTRL, _MASKED_BIT_ENABLE(UOS_MOVE | START_DMA));
>>
>> -       return 0;
>> +       /* Wait for DMA to finish */
>> +       ret = __intel_wait_for_register_fw(dev_priv, DMA_CTRL,  
>> START_DMA, 0,
>> +                                          2, 100, &status);
>
> To use _fw, you need to either hold forcewake or be sure you don't need
> it, and you also need to be sure that either you have serialisation
> around the routine or that it is not subject to the various concurrent
> mmio access bugs. I haven't seen anything that suggests they have been
> taken into account.

Note that this function is called by guc_fw_xfer() inside

	intel_uncore_forcewake_get(dev_priv, FORCEWAKE_ALL);
	...
	intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL);

and for serialization (with similar code in huc_ucode_xfer) we just
rely on code sequence in intel_uc_init_hw ... is this not enough ?

Michal