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@xxxxxxxxx> > >>Reviewed-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> > >>Reviewed-by: Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx> > >>Signed-off-by: Oscar Mateo <oscar.mateo@xxxxxxxxx> > >>--- > >> 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 _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx