[Intel-gfx] [PATCH 02/12] drm/i915/guc: Keep the ctx_pool_vaddr mapped, for easy access

[Chris Wilson]

On Wed, Mar 22, 2017 at 02:42:27AM -0700, Oscar Mateo wrote:
> 
> 
> On 03/22/2017 02:45 AM, Chris Wilson wrote:
> >On Tue, Mar 21, 2017 at 02:02:47AM -0700, Oscar Mateo wrote:
> >>The GuC descriptor is big in size. If we use a local definition of
> >>guc_desc we have a chance to overflow stack, so avoid it.
> >>
> >>Also, Chris abhors scatterlists :)
> >>
> >>v2: Rebased, helper function to retrieve the context descriptor,
> >>s/ctx_pool_vma/ctx_pool/
> >>
> >>v3: Zero out guc_context_desc before initialization
> >>
> >>v4: Do not do arithmetic on void pointers
> >>
> >>Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio at intel.com>
> >>Reviewed-by: Chris Wilson <chris at chris-wilson.co.uk>
> >>Reviewed-by: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
> >>Signed-off-by: Oscar Mateo <oscar.mateo at intel.com>
> >>---
> >>  drivers/gpu/drm/i915/i915_guc_submission.c | 93 +++++++++++++++---------------
> >>  drivers/gpu/drm/i915/intel_guc_loader.c    |  2 +-
> >>  drivers/gpu/drm/i915/intel_uc.h            |  3 +-
> >>  3 files changed, 48 insertions(+), 50 deletions(-)
> >>
> >>diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
> >>index 21dadc1..5870cec 100644
> >>--- a/drivers/gpu/drm/i915/i915_guc_submission.c
> >>+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
> >>@@ -133,6 +133,12 @@ static int __guc_deallocate_doorbell(struct intel_guc *guc, u32 ctx_index)
> >>  	return intel_guc_send(guc, action, ARRAY_SIZE(action));
> >>  }
> >>+static struct guc_context_desc *__get_context_desc(struct i915_guc_client *client)
> >>+{
> >>+	return (struct guc_context_desc *)((char *)client->guc->ctx_pool_vaddr +
> >We can use gccisms like using void * for arithmetic computations in the
> >kernel, i.e.
> >	void *base = client->guc->ctx_pool_vaddr;
> >	return base + sizeof(struct guc_context_desc) * client->ctx_index;
> >is just fine.
> >-Chris
> 
> :_(
> I got convinced by a previous review comment that arithmetic with
> void pointers was frowned upon.
> Ok, thanks!

Joonas mentioned trying:

	struct guc_context_desc *base = client->guc->ctx_pool_vaddr;
	return base + client->ctx_index;

or
	return &base[client->ctx_index];

for even more idiomatic C.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre