[Intel-gfx] [PATCH 02/12] drm/i915/guc: Allocate separate shared data object for GuC communication

[Daniele Ceraolo Spurio]

On 09/10/17 07:52, Michał Winiarski wrote:
> We were using first page of kernel context render state for sharing data
> with GuC. While it's justified by the fact that those pages are not used
> (note, GuC still enforces this layout and refuses to work if we remove
> the extra page in front), it's also confusing (why are we using this
> particular page?). Let's allocate a separate object instead.
> 
> Suggested-by: Daniele Ceraolo Spurio <daniele.ceraolospurio at intel.com>
> Signed-off-by: Michał Winiarski <michal.winiarski at intel.com>
> Cc: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio at intel.com>
> Cc: Jeff McGee <jeff.mcgee at intel.com>
> Cc: Michal Wajdeczko <michal.wajdeczko at intel.com>
> Cc: Oscar Mateo <oscar.mateo at intel.com>

+Michel (engine and watchdog reset with GuC use the shared page)

> ---
>   drivers/gpu/drm/i915/i915_guc_submission.c | 36 +++++++++++++++++++++++++++++-
>   drivers/gpu/drm/i915/intel_guc.c           |  8 ++-----
>   drivers/gpu/drm/i915/intel_guc.h           |  2 ++
>   3 files changed, 39 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
> index 8983d53af229..30f026566001 100644
> --- a/drivers/gpu/drm/i915/i915_guc_submission.c
> +++ b/drivers/gpu/drm/i915/i915_guc_submission.c
> @@ -437,6 +437,33 @@ static void guc_stage_desc_fini(struct intel_guc *guc,
>   	memset(desc, 0, sizeof(*desc));
>   }
>   
> +static int guc_shared_data_create(struct intel_guc *guc)
> +{
> +	struct i915_vma *vma;
> +	void *vaddr;
> +
> +	vma = intel_guc_allocate_vma(guc, PAGE_SIZE);
> +	if (IS_ERR(vma))
> +		return PTR_ERR(vma);
> +
> +	vaddr = i915_gem_object_pin_map(vma->obj, I915_MAP_WB);
> +	if (IS_ERR(vaddr)) {
> +		i915_vma_unpin_and_release(&vma);
> +		return PTR_ERR(vaddr);
> +	}
> +
> +	guc->shared_data = vma;
> +	guc->shared_data_vaddr = vaddr;

I've noticed that this is now the 3rd place where we allocate and 
immediately pin a vma for guc (the other 2 being 
guc_stage_desc_pool_create and ctch_init). Maybe we can move the 2 
operations to a more common helper (and do the same for the cleanup), 
but it's probably better to do it after all the ongoing GuC re-org has 
settled down. In the meantime, this patch is:

Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio at intel.com>

Daniele