Re: [PATCH 02/11] drm/nouveau: switch over to ttm_bo_init_reserved

[Christian König <christian.koenig@xxxxxxx>]

Am 19.05.22 um 15:19 schrieb Ruhl, Michael J:
> > -----Original Message-----
> > From: dri-devel <dri-devel-bounces@xxxxxxxxxxxxxxxxxxxxx> On Behalf Of
> > Christian König
> > Sent: Thursday, May 19, 2022 5:55 AM
> > To: intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> > Cc: matthew.william.auld@xxxxxxxxx; Christian König
> > <christian.koenig@xxxxxxx>; dri-devel@xxxxxxxxxxxxxxxxxxxxx
> > Subject: [PATCH 02/11] drm/nouveau: switch over to ttm_bo_init_reserved
> >
> > Use the new interface instead.
> >
> > Signed-off-by: Christian König <christian.koenig@xxxxxxx>
> > ---
> > drivers/gpu/drm/nouveau/nouveau_bo.c | 10 +++++++---
> > 1 file changed, 7 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c
> > b/drivers/gpu/drm/nouveau/nouveau_bo.c
> > index 05076e530e7d..858b9382036c 100644
> > --- a/drivers/gpu/drm/nouveau/nouveau_bo.c
> > +++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
> > @@ -302,19 +302,23 @@ nouveau_bo_init(struct nouveau_bo *nvbo, u64
> > size, int align, u32 domain,
> > 		struct sg_table *sg, struct dma_resv *robj)
> > {
> > 	int type = sg ? ttm_bo_type_sg : ttm_bo_type_device;
> > +	struct ttm_operation_ctx ctx = { false, false };
> > 	int ret;
> >
> > 	nouveau_bo_placement_set(nvbo, domain, 0);
> > 	INIT_LIST_HEAD(&nvbo->io_reserve_lru);
> >
> > -	ret = ttm_bo_init(nvbo->bo.bdev, &nvbo->bo, size, type,
> > -			  &nvbo->placement, align >> PAGE_SHIFT, false, sg,
> > -			  robj, nouveau_bo_del_ttm);
> > +	ret = ttm_bo_init_reserved(nvbo->bo.bdev, &nvbo->bo, size, type,
> > +				   &nvbo->placement, align >> PAGE_SHIFT,
> > &ctx,
> > +				   sg, robj, nouveau_bo_del_ttm);
> > 	if (ret) {
> > 		/* ttm will call nouveau_bo_del_ttm if it fails.. */
> > 		return ret;
> > 	}
> >
> > +	if (!robj)
> > +		ttm_bo_unreserve(&nvbo->bo);
> > +
> Ok, this implies that patch 1 does have an issue.
>
> I see this usage in patch 1, 2, and 3.  Would it make sense to move this
> _unreserve to ttm_bo_init_reserved?

Well the whole point of ttm_bo_init_reserved is that you need to do the 
un-reserve manually.

But yeah, you are right. It would just make much more sense to rename 
ttm_bo_init() instead of adjusting all of it's users.

Thanks,
Christian.

> Mike
>
> > 	return 0;
> > }
> >
> > --
> > 2.25.1