[Intel-gfx] [PATCH] drm/i915/guc: Make sure vma containing firmware is GuC mappable

[Daniele Ceraolo Spurio]

On 11/01/17 07:17, Michał Winiarski wrote:
> Since commit 4741da925fa3 ("drm/i915/guc: Assert that all GGTT offsets used
> by the GuC are mappable"), we're asserting that GuC firmware is in the
> GuC mappable range.
> Except we're not pinning the object with bias, which means it's possible
> to trigger this assert. Let's add a proper bias.
>
> Cc: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio at intel.com>
> Signed-off-by: Michał Winiarski <michal.winiarski at intel.com>
> ---
>  drivers/gpu/drm/i915/intel_guc_loader.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c
> index aa2b866..5a6ab87 100644
> --- a/drivers/gpu/drm/i915/intel_guc_loader.c
> +++ b/drivers/gpu/drm/i915/intel_guc_loader.c
> @@ -360,7 +360,8 @@ static int guc_ucode_xfer(struct drm_i915_private *dev_priv)
>  		return ret;
>  	}
>
> -	vma = i915_gem_object_ggtt_pin(guc_fw->guc_fw_obj, NULL, 0, 0, 0);
> +	vma = i915_gem_object_ggtt_pin(guc_fw->guc_fw_obj, NULL, 0, 0,
> +				       PIN_OFFSET_BIAS | GUC_WOPCM_TOP);
>  	if (IS_ERR(vma)) {
>  		DRM_DEBUG_DRIVER("pin failed %d\n", (int)PTR_ERR(vma));
>  		return PTR_ERR(vma);
>

This patch made me think about this again and actually I'm not sure 
anymore that there is an offset requirement for the firmware object. 
With the way we load the firmware the GuC should never access it in GGTT 
because it is first copied in WOPCM via DMA, which should be able to 
access the whole address range. I've asked a GuC dev but he has not been 
able to confirm if there are any offset limitation with the DMA transfer 
or not and unfortunately I don't have a platform to test this on at the 
moment. I'll try to get my hands on a new SKL and double check.
Anyway, I'm happy to merge this while we clarify the requirement because 
the firmware vma is immediately unpinned after the transfer so there 
should be no risk of unneeded ggtt fragmentation; it also looks 
generally cleaner to me to handle all guc-related objects the same way.

Thanks,
Daniele