Re: [PATCH 01/11] drm/radeon: switch over to ttm_bo_init_reserved

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

Am 19.05.22 um 14:54 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 01/11] drm/radeon: switch over to ttm_bo_init_reserved
> >
> > Use the new interface instead.
> >
> > Signed-off-by: Christian König <christian.koenig@xxxxxxx>
> > ---
> > drivers/gpu/drm/radeon/radeon_object.c | 17 +++++++++++------
> > 1 file changed, 11 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/radeon/radeon_object.c
> > b/drivers/gpu/drm/radeon/radeon_object.c
> > index 6c4a6802ca96..1d414ff4ab0c 100644
> > --- a/drivers/gpu/drm/radeon/radeon_object.c
> > +++ b/drivers/gpu/drm/radeon/radeon_object.c
> > @@ -133,9 +133,12 @@ int radeon_bo_create(struct radeon_device *rdev,
> > 		     struct dma_resv *resv,
> > 		     struct radeon_bo **bo_ptr)
> > {
> > -	struct radeon_bo *bo;
> > -	enum ttm_bo_type type;
> > 	unsigned long page_align = roundup(byte_align, PAGE_SIZE) >>
> > PAGE_SHIFT;
> > +
> > +	/* Kernel allocation are uninterruptible */
> > +	struct ttm_operation_ctx ctx = { !kernel, false };
> > +	enum ttm_bo_type type;
> > +	struct radeon_bo *bo;
> > 	int r;
> >
> > 	size = ALIGN(size, PAGE_SIZE);
> > @@ -200,11 +203,13 @@ int radeon_bo_create(struct radeon_device *rdev,
> > #endif
> >
> > 	radeon_ttm_placement_from_domain(bo, domain);
> > -	/* Kernel allocation are uninterruptible */
> > 	down_read(&rdev->pm.mclk_lock);
> > -	r = ttm_bo_init(&rdev->mman.bdev, &bo->tbo, size, type,
> > -			&bo->placement, page_align, !kernel, sg, resv,
> > -			&radeon_ttm_bo_destroy);
> > +	r = ttm_bo_init_reserved(&rdev->mman.bdev, &bo->tbo, size, type,
> > +				 &bo->placement, page_align, &ctx, sg, resv,
> > +				 &radeon_ttm_bo_destroy);
> > +        if (!r)
> > +		ttm_bo_unreserve(&bo->tbo);
> > +
> Hi Christian,
>
> I am not understanding this unreserve.
>
> The original code path does not have it.  It looks like tt_bo_init will do this, but only if !resv.
>
> Should this be:
>      if (!resv)
>            ttm_bo_unreserve(&bo->tbo);

Ah, yes good point. That's a bug.

Thanks,
Christian.

> ?
>
> M
>
>
> > 	up_read(&rdev->pm.mclk_lock);
> > 	if (unlikely(r != 0)) {
> > 		return r;
> > --
> > 2.25.1