| Message ID | 20240716123519.1884-10-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:19PM +0200, Christian König wrote: > Prevent drivers from using this directly. > This is a good change. Early on in Xe, our reference counting for BOs was flawed (and incorrect) due to confusion between GEM ref count and TTM ref count. Is there any way we can just eliminate this and use GEM ref count? I'm still confused about why we seemingly have two reference counts for the same object. Matt > Signed-off-by: Christian König <christian.koenig@amd.com> > --- > drivers/gpu/drm/ttm/ttm_bo_internal.h | 10 ++++++++++ > include/drm/ttm/ttm_bo.h | 10 ---------- > 2 files changed, 10 insertions(+), 10 deletions(-) > > diff --git a/drivers/gpu/drm/ttm/ttm_bo_internal.h b/drivers/gpu/drm/ttm/ttm_bo_internal.h > index 6a7305efd778..9d8b747a34db 100644 > --- a/drivers/gpu/drm/ttm/ttm_bo_internal.h > +++ b/drivers/gpu/drm/ttm/ttm_bo_internal.h > @@ -27,6 +27,16 @@ > > #include <drm/ttm/ttm_bo.h> > > +/** > + * ttm_bo_get - reference a struct ttm_buffer_object > + * > + * @bo: The buffer object. > + */ > +static inline void ttm_bo_get(struct ttm_buffer_object *bo) > +{ > + kref_get(&bo->kref); > +} > + > /** > * ttm_bo_get_unless_zero - reference a struct ttm_buffer_object unless > * its refcount has already reached zero. > diff --git a/include/drm/ttm/ttm_bo.h b/include/drm/ttm/ttm_bo.h > index 31ec7fd34eeb..8c1577d8793c 100644 > --- a/include/drm/ttm/ttm_bo.h > +++ b/include/drm/ttm/ttm_bo.h > @@ -229,16 +229,6 @@ struct ttm_lru_walk { > s64 ttm_lru_walk_for_evict(struct ttm_lru_walk *walk, struct ttm_device *bdev, > struct ttm_resource_manager *man, s64 target); > > -/** > - * ttm_bo_get - reference a struct ttm_buffer_object > - * > - * @bo: The buffer object. > - */ > -static inline void ttm_bo_get(struct ttm_buffer_object *bo) > -{ > - kref_get(&bo->kref); > -} > - > /** > * ttm_bo_reserve: > * > -- > 2.34.1 >
On Tue, Jul 16, 2024 at 02:35:19PM +0200, Christian König wrote: > Prevent drivers from using this directly. > > Signed-off-by: Christian König <christian.koenig@amd.com> Reviewed-by: Matthew Brost <matthew.brost@intel.com> > --- > drivers/gpu/drm/ttm/ttm_bo_internal.h | 10 ++++++++++ > include/drm/ttm/ttm_bo.h | 10 ---------- > 2 files changed, 10 insertions(+), 10 deletions(-) > > diff --git a/drivers/gpu/drm/ttm/ttm_bo_internal.h b/drivers/gpu/drm/ttm/ttm_bo_internal.h > index 6a7305efd778..9d8b747a34db 100644 > --- a/drivers/gpu/drm/ttm/ttm_bo_internal.h > +++ b/drivers/gpu/drm/ttm/ttm_bo_internal.h > @@ -27,6 +27,16 @@ > > #include <drm/ttm/ttm_bo.h> > > +/** > + * ttm_bo_get - reference a struct ttm_buffer_object > + * > + * @bo: The buffer object. > + */ > +static inline void ttm_bo_get(struct ttm_buffer_object *bo) > +{ > + kref_get(&bo->kref); > +} > + > /** > * ttm_bo_get_unless_zero - reference a struct ttm_buffer_object unless > * its refcount has already reached zero. > diff --git a/include/drm/ttm/ttm_bo.h b/include/drm/ttm/ttm_bo.h > index 31ec7fd34eeb..8c1577d8793c 100644 > --- a/include/drm/ttm/ttm_bo.h > +++ b/include/drm/ttm/ttm_bo.h > @@ -229,16 +229,6 @@ struct ttm_lru_walk { > s64 ttm_lru_walk_for_evict(struct ttm_lru_walk *walk, struct ttm_device *bdev, > struct ttm_resource_manager *man, s64 target); > > -/** > - * ttm_bo_get - reference a struct ttm_buffer_object > - * > - * @bo: The buffer object. > - */ > -static inline void ttm_bo_get(struct ttm_buffer_object *bo) > -{ > - kref_get(&bo->kref); > -} > - > /** > * ttm_bo_reserve: > * > -- > 2.34.1 >
Am 16.07.24 um 23:37 schrieb Matthew Brost: > On Tue, Jul 16, 2024 at 02:35:19PM +0200, Christian König wrote: >> Prevent drivers from using this directly. >> > This is a good change. Early on in Xe, our reference counting for BOs > was flawed (and incorrect) due to confusion between GEM ref count and > TTM ref count. > > Is there any way we can just eliminate this and use GEM ref count? I'm > still confused about why we seemingly have two reference counts for the > same object. I don't want to give your nightmares but initially we had *four* reference counts for the same object! I've already removed one of it quite a while ago because I ran into similar issues, but currently we have three: 1. The GEM kref 2. The GEM handle count 3. The TTM kref The handle count and GEM kref make sense since the handle count controls when to release DMA-buf and the GEM kref controls when to release the structure and backing store. The TTM kref is another structure reference created because historically TTM BOs were not based on GEM objects. Since this is no longer the case we can probably nuke that one. Regards, Christian. > > Matt > >> Signed-off-by: Christian König <christian.koenig@amd.com> >> --- >> drivers/gpu/drm/ttm/ttm_bo_internal.h | 10 ++++++++++ >> include/drm/ttm/ttm_bo.h | 10 ---------- >> 2 files changed, 10 insertions(+), 10 deletions(-) >> >> diff --git a/drivers/gpu/drm/ttm/ttm_bo_internal.h b/drivers/gpu/drm/ttm/ttm_bo_internal.h >> index 6a7305efd778..9d8b747a34db 100644 >> --- a/drivers/gpu/drm/ttm/ttm_bo_internal.h >> +++ b/drivers/gpu/drm/ttm/ttm_bo_internal.h >> @@ -27,6 +27,16 @@ >> >> #include <drm/ttm/ttm_bo.h> >> >> +/** >> + * ttm_bo_get - reference a struct ttm_buffer_object >> + * >> + * @bo: The buffer object. >> + */ >> +static inline void ttm_bo_get(struct ttm_buffer_object *bo) >> +{ >> + kref_get(&bo->kref); >> +} >> + >> /** >> * ttm_bo_get_unless_zero - reference a struct ttm_buffer_object unless >> * its refcount has already reached zero. >> diff --git a/include/drm/ttm/ttm_bo.h b/include/drm/ttm/ttm_bo.h >> index 31ec7fd34eeb..8c1577d8793c 100644 >> --- a/include/drm/ttm/ttm_bo.h >> +++ b/include/drm/ttm/ttm_bo.h >> @@ -229,16 +229,6 @@ struct ttm_lru_walk { >> s64 ttm_lru_walk_for_evict(struct ttm_lru_walk *walk, struct ttm_device *bdev, >> struct ttm_resource_manager *man, s64 target); >> >> -/** >> - * ttm_bo_get - reference a struct ttm_buffer_object >> - * >> - * @bo: The buffer object. >> - */ >> -static inline void ttm_bo_get(struct ttm_buffer_object *bo) >> -{ >> - kref_get(&bo->kref); >> -} >> - >> /** >> * ttm_bo_reserve: >> * >> -- >> 2.34.1 >>
diff --git a/drivers/gpu/drm/ttm/ttm_bo_internal.h b/drivers/gpu/drm/ttm/ttm_bo_internal.h index 6a7305efd778..9d8b747a34db 100644 --- a/drivers/gpu/drm/ttm/ttm_bo_internal.h +++ b/drivers/gpu/drm/ttm/ttm_bo_internal.h @@ -27,6 +27,16 @@ #include <drm/ttm/ttm_bo.h> +/** + * ttm_bo_get - reference a struct ttm_buffer_object + * + * @bo: The buffer object. + */ +static inline void ttm_bo_get(struct ttm_buffer_object *bo) +{ + kref_get(&bo->kref); +} + /** * ttm_bo_get_unless_zero - reference a struct ttm_buffer_object unless * its refcount has already reached zero. diff --git a/include/drm/ttm/ttm_bo.h b/include/drm/ttm/ttm_bo.h index 31ec7fd34eeb..8c1577d8793c 100644 --- a/include/drm/ttm/ttm_bo.h +++ b/include/drm/ttm/ttm_bo.h @@ -229,16 +229,6 @@ struct ttm_lru_walk { s64 ttm_lru_walk_for_evict(struct ttm_lru_walk *walk, struct ttm_device *bdev, struct ttm_resource_manager *man, s64 target); -/** - * ttm_bo_get - reference a struct ttm_buffer_object - * - * @bo: The buffer object. - */ -static inline void ttm_bo_get(struct ttm_buffer_object *bo) -{ - kref_get(&bo->kref); -} - /** * ttm_bo_reserve: *
Prevent drivers from using this directly. Signed-off-by: Christian König <christian.koenig@amd.com> --- drivers/gpu/drm/ttm/ttm_bo_internal.h | 10 ++++++++++ include/drm/ttm/ttm_bo.h | 10 ---------- 2 files changed, 10 insertions(+), 10 deletions(-)