On Mon, 2026-05-18 at 15:01 +0000, Matt Coster wrote: > Hi Brajesh, > > On 12/05/2026 07:47, Brajesh Gupta wrote: > > Fix context ID value for the FW common context by moving the context > > allocation earlier. > > "Fix" is pretty vague here – can you describe the current behaviour, and > why that's wrong?
Updated > > I also assume this fixes a previous change, so can you please add a > Fixes: tag referencing the commit which introduced the faulty behaviour > (and possibly a Cc: stable tag, if needed)? > Didn't include fix tag as Context ID is not used currently in driver so nothing is broken. > > Also an old incorrect comment was removed. > > This detail probably doesn't need to be mentioned. > Dropped > Cheers, > Matt > > > > > Signed-off-by: Brajesh Gupta <[email protected]> > > --- > > drivers/gpu/drm/imagination/pvr_context.c | 30 > > ++++++++++++++++-------------- > > 1 file changed, 16 insertions(+), 14 deletions(-) > > > > diff --git a/drivers/gpu/drm/imagination/pvr_context.c > > b/drivers/gpu/drm/imagination/pvr_context.c > > index 8de70c30b9de..eba4694400b5 100644 > > --- a/drivers/gpu/drm/imagination/pvr_context.c > > +++ b/drivers/gpu/drm/imagination/pvr_context.c > > @@ -318,10 +318,14 @@ int pvr_context_create(struct pvr_file *pvr_file, > > struct drm_pvr_ioctl_create_co > > goto err_put_vm; > > } > > > > - err = pvr_context_create_queues(ctx, args, ctx->data); > > + err = xa_alloc(&pvr_dev->ctx_ids, &ctx->ctx_id, ctx, xa_limit_32b, > > GFP_KERNEL); > > if (err) > > goto err_free_ctx_data; > > > > + err = pvr_context_create_queues(ctx, args, ctx->data); > > + if (err) > > + goto err_free_ctx_id; > > + > > err = init_fw_objs(ctx, args, ctx->data); > > if (err) > > goto err_destroy_queues; > > @@ -329,23 +333,12 @@ int pvr_context_create(struct pvr_file *pvr_file, > > struct drm_pvr_ioctl_create_co > > err = pvr_fw_object_create(pvr_dev, ctx_size, > > PVR_BO_FW_FLAGS_DEVICE_UNCACHED, > > ctx_fw_data_init, ctx, &ctx->fw_obj); > > if (err) > > - goto err_free_ctx_data; > > + goto err_destroy_queues; > > > > - err = xa_alloc(&pvr_dev->ctx_ids, &ctx->ctx_id, ctx, xa_limit_32b, > > GFP_KERNEL); > > + err = xa_alloc(&pvr_file->ctx_handles, &args->handle, ctx, > > xa_limit_32b, GFP_KERNEL); > > if (err) > > goto err_destroy_fw_obj; > > > > - err = xa_alloc(&pvr_file->ctx_handles, &args->handle, ctx, > > xa_limit_32b, GFP_KERNEL); > > - if (err) { > > - /* > > - * It's possible that another thread could have taken a > > reference on the context at > > - * this point as it is in the ctx_ids xarray. Therefore > > instead of directly > > - * destroying the context, drop a reference instead. > > - */ > > - pvr_context_put(ctx); > > - return err; > > - } > > - > > spin_lock(&pvr_dev->ctx_list_lock); > > list_add_tail(&ctx->file_link, &pvr_file->contexts); > > spin_unlock(&pvr_dev->ctx_list_lock); > > @@ -358,6 +351,15 @@ int pvr_context_create(struct pvr_file *pvr_file, > > struct drm_pvr_ioctl_create_co > > err_destroy_queues: > > pvr_context_destroy_queues(ctx); > > > > +err_free_ctx_id: > > + /* > > + * Ctx_id is not exposed to userspace and not visible yet within > > + * the kernel/FW, plus a matching context handle (exposed to > > userspace) > > + * hasn't been allocated yet, so it is safe to remove ctx_id > > + * from the ctx_ids xarray. > > + */ > > + xa_erase(&pvr_dev->ctx_ids, ctx->ctx_id); > > + > > err_free_ctx_data: > > kfree(ctx->data); > > > > > > -- > > 2.43.0 > > > > Thanks, Brajesh
