| Message ID | 20240716123519.1884-2-christian.koenig@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/9] drm/amdgpu: use GEM references instead of TTMs | expand |
On Tue, Jul 16, 2024 at 02:35:11PM +0200, Christian König wrote: > Instead of a TTM reference grab a GEM reference whenever necessary. > > Signed-off-by: Christian König <christian.koenig@amd.com> > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 8 ++++---- > drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 7 ++----- > 2 files changed, 6 insertions(+), 9 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c > index 67c234bcf89f..6be3d7cd1c51 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c > @@ -87,11 +87,11 @@ static const struct vm_operations_struct amdgpu_gem_vm_ops = { > > static void amdgpu_gem_object_free(struct drm_gem_object *gobj) > { > - struct amdgpu_bo *robj = gem_to_amdgpu_bo(gobj); > + struct amdgpu_bo *aobj = gem_to_amdgpu_bo(gobj); > > - if (robj) { > - amdgpu_hmm_unregister(robj); > - amdgpu_bo_unref(&robj); > + if (aobj) { > + amdgpu_hmm_unregister(aobj); > + ttm_bo_put(&aobj->tbo); So, this relates to my comment in patch number #9 about dropping the TTM ref count. If TTM only uses the GEM ref count, could we convert this ttm_bo_put to something like ttm_bo_fini here (also in Xe and any other drivers with code like this)? I think that might be cleaner. Matt > } > } > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c > index 8d8c39be6129..6c187e310034 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c > @@ -853,7 +853,7 @@ struct amdgpu_bo *amdgpu_bo_ref(struct amdgpu_bo *bo) > if (bo == NULL) > return NULL; > > - ttm_bo_get(&bo->tbo); > + drm_gem_object_get(&bo->tbo.base); > return bo; > } > > @@ -865,13 +865,10 @@ struct amdgpu_bo *amdgpu_bo_ref(struct amdgpu_bo *bo) > */ > void amdgpu_bo_unref(struct amdgpu_bo **bo) > { > - struct ttm_buffer_object *tbo; > - > if ((*bo) == NULL) > return; > > - tbo = &((*bo)->tbo); > - ttm_bo_put(tbo); > + drm_gem_object_get(&(*bo)->tbo.base); > *bo = NULL; > } > > -- > 2.34.1 >
Am 16.07.24 um 23:53 schrieb Matthew Brost: > On Tue, Jul 16, 2024 at 02:35:11PM +0200, Christian König wrote: >> Instead of a TTM reference grab a GEM reference whenever necessary. >> >> Signed-off-by: Christian König <christian.koenig@amd.com> >> --- >> drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 8 ++++---- >> drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 7 ++----- >> 2 files changed, 6 insertions(+), 9 deletions(-) >> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c >> index 67c234bcf89f..6be3d7cd1c51 100644 >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c >> @@ -87,11 +87,11 @@ static const struct vm_operations_struct amdgpu_gem_vm_ops = { >> >> static void amdgpu_gem_object_free(struct drm_gem_object *gobj) >> { >> - struct amdgpu_bo *robj = gem_to_amdgpu_bo(gobj); >> + struct amdgpu_bo *aobj = gem_to_amdgpu_bo(gobj); >> >> - if (robj) { >> - amdgpu_hmm_unregister(robj); >> - amdgpu_bo_unref(&robj); >> + if (aobj) { >> + amdgpu_hmm_unregister(aobj); >> + ttm_bo_put(&aobj->tbo); > So, this relates to my comment in patch number #9 about dropping the TTM > ref count. If TTM only uses the GEM ref count, could we convert this > ttm_bo_put to something like ttm_bo_fini here (also in Xe and any other > drivers with code like this)? That's exactly what I was planning to do as a follow up. Regards, Christian. > > I think that might be cleaner. > > Matt > >> } >> } >> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c >> index 8d8c39be6129..6c187e310034 100644 >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c >> @@ -853,7 +853,7 @@ struct amdgpu_bo *amdgpu_bo_ref(struct amdgpu_bo *bo) >> if (bo == NULL) >> return NULL; >> >> - ttm_bo_get(&bo->tbo); >> + drm_gem_object_get(&bo->tbo.base); >> return bo; >> } >> >> @@ -865,13 +865,10 @@ struct amdgpu_bo *amdgpu_bo_ref(struct amdgpu_bo *bo) >> */ >> void amdgpu_bo_unref(struct amdgpu_bo **bo) >> { >> - struct ttm_buffer_object *tbo; >> - >> if ((*bo) == NULL) >> return; >> >> - tbo = &((*bo)->tbo); >> - ttm_bo_put(tbo); >> + drm_gem_object_get(&(*bo)->tbo.base); >> *bo = NULL; >> } >> >> -- >> 2.34.1 >>
On Wed, Jul 17, 2024 at 04:53:16PM +0200, Christian König wrote: > Am 16.07.24 um 23:53 schrieb Matthew Brost: > > On Tue, Jul 16, 2024 at 02:35:11PM +0200, Christian König wrote: > > > Instead of a TTM reference grab a GEM reference whenever necessary. > > > > > > Signed-off-by: Christian König <christian.koenig@amd.com> > > > --- > > > drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 8 ++++---- > > > drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 7 ++----- > > > 2 files changed, 6 insertions(+), 9 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c > > > index 67c234bcf89f..6be3d7cd1c51 100644 > > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c > > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c > > > @@ -87,11 +87,11 @@ static const struct vm_operations_struct amdgpu_gem_vm_ops = { > > > static void amdgpu_gem_object_free(struct drm_gem_object *gobj) > > > { > > > - struct amdgpu_bo *robj = gem_to_amdgpu_bo(gobj); > > > + struct amdgpu_bo *aobj = gem_to_amdgpu_bo(gobj); > > > - if (robj) { > > > - amdgpu_hmm_unregister(robj); > > > - amdgpu_bo_unref(&robj); > > > + if (aobj) { > > > + amdgpu_hmm_unregister(aobj); > > > + ttm_bo_put(&aobj->tbo); > > So, this relates to my comment in patch number #9 about dropping the TTM > > ref count. If TTM only uses the GEM ref count, could we convert this > > ttm_bo_put to something like ttm_bo_fini here (also in Xe and any other > > drivers with code like this)? > > That's exactly what I was planning to do as a follow up. > Cool, glad we are aligned. Matt > Regards, > Christian. > > > > > I think that might be cleaner. > > > > Matt > > > > > } > > > } > > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c > > > index 8d8c39be6129..6c187e310034 100644 > > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c > > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c > > > @@ -853,7 +853,7 @@ struct amdgpu_bo *amdgpu_bo_ref(struct amdgpu_bo *bo) > > > if (bo == NULL) > > > return NULL; > > > - ttm_bo_get(&bo->tbo); > > > + drm_gem_object_get(&bo->tbo.base); > > > return bo; > > > } > > > @@ -865,13 +865,10 @@ struct amdgpu_bo *amdgpu_bo_ref(struct amdgpu_bo *bo) > > > */ > > > void amdgpu_bo_unref(struct amdgpu_bo **bo) > > > { > > > - struct ttm_buffer_object *tbo; > > > - > > > if ((*bo) == NULL) > > > return; > > > - tbo = &((*bo)->tbo); > > > - ttm_bo_put(tbo); > > > + drm_gem_object_get(&(*bo)->tbo.base); > > > *bo = NULL; > > > } > > > -- > > > 2.34.1 > > > >
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c index 67c234bcf89f..6be3d7cd1c51 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c @@ -87,11 +87,11 @@ static const struct vm_operations_struct amdgpu_gem_vm_ops = { static void amdgpu_gem_object_free(struct drm_gem_object *gobj) { - struct amdgpu_bo *robj = gem_to_amdgpu_bo(gobj); + struct amdgpu_bo *aobj = gem_to_amdgpu_bo(gobj); - if (robj) { - amdgpu_hmm_unregister(robj); - amdgpu_bo_unref(&robj); + if (aobj) { + amdgpu_hmm_unregister(aobj); + ttm_bo_put(&aobj->tbo); } } diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c index 8d8c39be6129..6c187e310034 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c @@ -853,7 +853,7 @@ struct amdgpu_bo *amdgpu_bo_ref(struct amdgpu_bo *bo) if (bo == NULL) return NULL; - ttm_bo_get(&bo->tbo); + drm_gem_object_get(&bo->tbo.base); return bo; } @@ -865,13 +865,10 @@ struct amdgpu_bo *amdgpu_bo_ref(struct amdgpu_bo *bo) */ void amdgpu_bo_unref(struct amdgpu_bo **bo) { - struct ttm_buffer_object *tbo; - if ((*bo) == NULL) return; - tbo = &((*bo)->tbo); - ttm_bo_put(tbo); + drm_gem_object_get(&(*bo)->tbo.base); *bo = NULL; }
Instead of a TTM reference grab a GEM reference whenever necessary. Signed-off-by: Christian König <christian.koenig@amd.com> --- drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 8 ++++---- drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 7 ++----- 2 files changed, 6 insertions(+), 9 deletions(-)