* [PATCH] drm/amdgpu: Avoid direct cast to amdgpu_ttm_tt
@ 2022-07-26 23:43 Rajneesh Bhardwaj
2022-07-26 23:55 ` Felix Kuehling
0 siblings, 1 reply; 2+ messages in thread
From: Rajneesh Bhardwaj @ 2022-07-26 23:43 UTC (permalink / raw)
To: amd-gfx
Cc: alexander.deucher, Felix.Kuehling, Rajneesh Bhardwaj,
Christian König
For typesafety, use container_of() instead of implicit cast from struct
ttm_tt to struct amdgpu_ttm_tt.
Cc: Christian König <christian.koenig@amd.com>
Signed-off-by: Rajneesh Bhardwaj <rajneesh.bhardwaj@amd.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 44 ++++++++++++++-----------
1 file changed, 24 insertions(+), 20 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index be0efaae79a9..cd6aa206a59e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -637,6 +637,8 @@ struct amdgpu_ttm_tt {
#endif
};
+#define ttm_to_amdgpu_ttm_tt(ttm) container_of(ttm, struct amdgpu_ttm_tt, ttm)
+
#ifdef CONFIG_DRM_AMDGPU_USERPTR
/*
* amdgpu_ttm_tt_get_user_pages - get device accessible pages that back user
@@ -648,7 +650,7 @@ struct amdgpu_ttm_tt {
int amdgpu_ttm_tt_get_user_pages(struct amdgpu_bo *bo, struct page **pages)
{
struct ttm_tt *ttm = bo->tbo.ttm;
- struct amdgpu_ttm_tt *gtt = (void *)ttm;
+ struct amdgpu_ttm_tt *gtt = ttm_to_amdgpu_ttm_tt(ttm);
unsigned long start = gtt->userptr;
struct vm_area_struct *vma;
struct mm_struct *mm;
@@ -702,7 +704,7 @@ int amdgpu_ttm_tt_get_user_pages(struct amdgpu_bo *bo, struct page **pages)
*/
bool amdgpu_ttm_tt_get_user_pages_done(struct ttm_tt *ttm)
{
- struct amdgpu_ttm_tt *gtt = (void *)ttm;
+ struct amdgpu_ttm_tt *gtt = ttm_to_amdgpu_ttm_tt(ttm);
bool r = false;
if (!gtt || !gtt->userptr)
@@ -751,7 +753,7 @@ static int amdgpu_ttm_tt_pin_userptr(struct ttm_device *bdev,
struct ttm_tt *ttm)
{
struct amdgpu_device *adev = amdgpu_ttm_adev(bdev);
- struct amdgpu_ttm_tt *gtt = (void *)ttm;
+ struct amdgpu_ttm_tt *gtt = ttm_to_amdgpu_ttm_tt(ttm);
int write = !(gtt->userflags & AMDGPU_GEM_USERPTR_READONLY);
enum dma_data_direction direction = write ?
DMA_BIDIRECTIONAL : DMA_TO_DEVICE;
@@ -788,7 +790,7 @@ static void amdgpu_ttm_tt_unpin_userptr(struct ttm_device *bdev,
struct ttm_tt *ttm)
{
struct amdgpu_device *adev = amdgpu_ttm_adev(bdev);
- struct amdgpu_ttm_tt *gtt = (void *)ttm;
+ struct amdgpu_ttm_tt *gtt = ttm_to_amdgpu_ttm_tt(ttm);
int write = !(gtt->userflags & AMDGPU_GEM_USERPTR_READONLY);
enum dma_data_direction direction = write ?
DMA_BIDIRECTIONAL : DMA_TO_DEVICE;
@@ -822,7 +824,7 @@ static void amdgpu_ttm_gart_bind(struct amdgpu_device *adev,
{
struct amdgpu_bo *abo = ttm_to_amdgpu_bo(tbo);
struct ttm_tt *ttm = tbo->ttm;
- struct amdgpu_ttm_tt *gtt = (void *)ttm;
+ struct amdgpu_ttm_tt *gtt = ttm_to_amdgpu_ttm_tt(ttm);
if (amdgpu_bo_encrypted(abo))
flags |= AMDGPU_PTE_TMZ;
@@ -860,7 +862,7 @@ static int amdgpu_ttm_backend_bind(struct ttm_device *bdev,
struct ttm_resource *bo_mem)
{
struct amdgpu_device *adev = amdgpu_ttm_adev(bdev);
- struct amdgpu_ttm_tt *gtt = (void*)ttm;
+ struct amdgpu_ttm_tt *gtt = ttm_to_amdgpu_ttm_tt(ttm);
uint64_t flags;
int r;
@@ -927,7 +929,8 @@ int amdgpu_ttm_alloc_gart(struct ttm_buffer_object *bo)
{
struct amdgpu_device *adev = amdgpu_ttm_adev(bo->bdev);
struct ttm_operation_ctx ctx = { false, false };
- struct amdgpu_ttm_tt *gtt = (void *)bo->ttm;
+ struct ttm_tt *ttm = bo->ttm;
+ struct amdgpu_ttm_tt *gtt = ttm_to_amdgpu_ttm_tt(ttm);
struct ttm_placement placement;
struct ttm_place placements;
struct ttm_resource *tmp;
@@ -998,7 +1001,7 @@ static void amdgpu_ttm_backend_unbind(struct ttm_device *bdev,
struct ttm_tt *ttm)
{
struct amdgpu_device *adev = amdgpu_ttm_adev(bdev);
- struct amdgpu_ttm_tt *gtt = (void *)ttm;
+ struct amdgpu_ttm_tt *gtt = ttm_to_amdgpu_ttm_tt(ttm);
/* if the pages have userptr pinning then clear that first */
if (gtt->userptr) {
@@ -1025,7 +1028,7 @@ static void amdgpu_ttm_backend_unbind(struct ttm_device *bdev,
static void amdgpu_ttm_backend_destroy(struct ttm_device *bdev,
struct ttm_tt *ttm)
{
- struct amdgpu_ttm_tt *gtt = (void *)ttm;
+ struct amdgpu_ttm_tt *gtt = ttm_to_amdgpu_ttm_tt(ttm);
if (gtt->usertask)
put_task_struct(gtt->usertask);
@@ -1079,7 +1082,7 @@ static int amdgpu_ttm_tt_populate(struct ttm_device *bdev,
struct ttm_operation_ctx *ctx)
{
struct amdgpu_device *adev = amdgpu_ttm_adev(bdev);
- struct amdgpu_ttm_tt *gtt = (void *)ttm;
+ struct amdgpu_ttm_tt *gtt = ttm_to_amdgpu_ttm_tt(ttm);
pgoff_t i;
int ret;
@@ -1113,7 +1116,7 @@ static int amdgpu_ttm_tt_populate(struct ttm_device *bdev,
static void amdgpu_ttm_tt_unpopulate(struct ttm_device *bdev,
struct ttm_tt *ttm)
{
- struct amdgpu_ttm_tt *gtt = (void *)ttm;
+ struct amdgpu_ttm_tt *gtt = ttm_to_amdgpu_ttm_tt(ttm);
struct amdgpu_device *adev;
pgoff_t i;
@@ -1171,18 +1174,19 @@ int amdgpu_ttm_tt_set_userptr(struct ttm_buffer_object *bo,
uint64_t addr, uint32_t flags)
{
struct amdgpu_ttm_tt *gtt;
+ struct ttm_tt *ttm = bo->ttm;
- if (!bo->ttm) {
+ if (!ttm) {
/* TODO: We want a separate TTM object type for userptrs */
- bo->ttm = amdgpu_ttm_tt_create(bo, 0);
- if (bo->ttm == NULL)
+ ttm = amdgpu_ttm_tt_create(bo, 0);
+ if (ttm == NULL)
return -ENOMEM;
}
/* Set TTM_TT_FLAG_EXTERNAL before populate but after create. */
- bo->ttm->page_flags |= TTM_TT_FLAG_EXTERNAL;
+ ttm->page_flags |= TTM_TT_FLAG_EXTERNAL;
- gtt = (void *)bo->ttm;
+ gtt = ttm_to_amdgpu_ttm_tt(ttm);
gtt->userptr = addr;
gtt->userflags = flags;
@@ -1199,7 +1203,7 @@ int amdgpu_ttm_tt_set_userptr(struct ttm_buffer_object *bo,
*/
struct mm_struct *amdgpu_ttm_tt_get_usermm(struct ttm_tt *ttm)
{
- struct amdgpu_ttm_tt *gtt = (void *)ttm;
+ struct amdgpu_ttm_tt *gtt = ttm_to_amdgpu_ttm_tt(ttm);
if (gtt == NULL)
return NULL;
@@ -1218,7 +1222,7 @@ struct mm_struct *amdgpu_ttm_tt_get_usermm(struct ttm_tt *ttm)
bool amdgpu_ttm_tt_affect_userptr(struct ttm_tt *ttm, unsigned long start,
unsigned long end, unsigned long *userptr)
{
- struct amdgpu_ttm_tt *gtt = (void *)ttm;
+ struct amdgpu_ttm_tt *gtt = ttm_to_amdgpu_ttm_tt(ttm);
unsigned long size;
if (gtt == NULL || !gtt->userptr)
@@ -1241,7 +1245,7 @@ bool amdgpu_ttm_tt_affect_userptr(struct ttm_tt *ttm, unsigned long start,
*/
bool amdgpu_ttm_tt_is_userptr(struct ttm_tt *ttm)
{
- struct amdgpu_ttm_tt *gtt = (void *)ttm;
+ struct amdgpu_ttm_tt *gtt = ttm_to_amdgpu_ttm_tt(ttm);
if (gtt == NULL || !gtt->userptr)
return false;
@@ -1254,7 +1258,7 @@ bool amdgpu_ttm_tt_is_userptr(struct ttm_tt *ttm)
*/
bool amdgpu_ttm_tt_is_readonly(struct ttm_tt *ttm)
{
- struct amdgpu_ttm_tt *gtt = (void *)ttm;
+ struct amdgpu_ttm_tt *gtt = ttm_to_amdgpu_ttm_tt(ttm);
if (gtt == NULL)
return false;
--
2.17.1
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] drm/amdgpu: Avoid direct cast to amdgpu_ttm_tt
2022-07-26 23:43 [PATCH] drm/amdgpu: Avoid direct cast to amdgpu_ttm_tt Rajneesh Bhardwaj
@ 2022-07-26 23:55 ` Felix Kuehling
0 siblings, 0 replies; 2+ messages in thread
From: Felix Kuehling @ 2022-07-26 23:55 UTC (permalink / raw)
To: Rajneesh Bhardwaj, amd-gfx; +Cc: alexander.deucher, Christian König
Am 2022-07-26 um 19:43 schrieb Rajneesh Bhardwaj:
> For typesafety, use container_of() instead of implicit cast from struct
> ttm_tt to struct amdgpu_ttm_tt.
>
> Cc: Christian König <christian.koenig@amd.com>
> Signed-off-by: Rajneesh Bhardwaj <rajneesh.bhardwaj@amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 44 ++++++++++++++-----------
> 1 file changed, 24 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> index be0efaae79a9..cd6aa206a59e 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> @@ -637,6 +637,8 @@ struct amdgpu_ttm_tt {
> #endif
> };
>
> +#define ttm_to_amdgpu_ttm_tt(ttm) container_of(ttm, struct amdgpu_ttm_tt, ttm)
> +
> #ifdef CONFIG_DRM_AMDGPU_USERPTR
> /*
> * amdgpu_ttm_tt_get_user_pages - get device accessible pages that back user
> @@ -648,7 +650,7 @@ struct amdgpu_ttm_tt {
> int amdgpu_ttm_tt_get_user_pages(struct amdgpu_bo *bo, struct page **pages)
> {
> struct ttm_tt *ttm = bo->tbo.ttm;
> - struct amdgpu_ttm_tt *gtt = (void *)ttm;
> + struct amdgpu_ttm_tt *gtt = ttm_to_amdgpu_ttm_tt(ttm);
> unsigned long start = gtt->userptr;
> struct vm_area_struct *vma;
> struct mm_struct *mm;
> @@ -702,7 +704,7 @@ int amdgpu_ttm_tt_get_user_pages(struct amdgpu_bo *bo, struct page **pages)
> */
> bool amdgpu_ttm_tt_get_user_pages_done(struct ttm_tt *ttm)
> {
> - struct amdgpu_ttm_tt *gtt = (void *)ttm;
> + struct amdgpu_ttm_tt *gtt = ttm_to_amdgpu_ttm_tt(ttm);
> bool r = false;
>
> if (!gtt || !gtt->userptr)
> @@ -751,7 +753,7 @@ static int amdgpu_ttm_tt_pin_userptr(struct ttm_device *bdev,
> struct ttm_tt *ttm)
> {
> struct amdgpu_device *adev = amdgpu_ttm_adev(bdev);
> - struct amdgpu_ttm_tt *gtt = (void *)ttm;
> + struct amdgpu_ttm_tt *gtt = ttm_to_amdgpu_ttm_tt(ttm);
> int write = !(gtt->userflags & AMDGPU_GEM_USERPTR_READONLY);
> enum dma_data_direction direction = write ?
> DMA_BIDIRECTIONAL : DMA_TO_DEVICE;
> @@ -788,7 +790,7 @@ static void amdgpu_ttm_tt_unpin_userptr(struct ttm_device *bdev,
> struct ttm_tt *ttm)
> {
> struct amdgpu_device *adev = amdgpu_ttm_adev(bdev);
> - struct amdgpu_ttm_tt *gtt = (void *)ttm;
> + struct amdgpu_ttm_tt *gtt = ttm_to_amdgpu_ttm_tt(ttm);
> int write = !(gtt->userflags & AMDGPU_GEM_USERPTR_READONLY);
> enum dma_data_direction direction = write ?
> DMA_BIDIRECTIONAL : DMA_TO_DEVICE;
> @@ -822,7 +824,7 @@ static void amdgpu_ttm_gart_bind(struct amdgpu_device *adev,
> {
> struct amdgpu_bo *abo = ttm_to_amdgpu_bo(tbo);
> struct ttm_tt *ttm = tbo->ttm;
> - struct amdgpu_ttm_tt *gtt = (void *)ttm;
> + struct amdgpu_ttm_tt *gtt = ttm_to_amdgpu_ttm_tt(ttm);
>
> if (amdgpu_bo_encrypted(abo))
> flags |= AMDGPU_PTE_TMZ;
> @@ -860,7 +862,7 @@ static int amdgpu_ttm_backend_bind(struct ttm_device *bdev,
> struct ttm_resource *bo_mem)
> {
> struct amdgpu_device *adev = amdgpu_ttm_adev(bdev);
> - struct amdgpu_ttm_tt *gtt = (void*)ttm;
> + struct amdgpu_ttm_tt *gtt = ttm_to_amdgpu_ttm_tt(ttm);
> uint64_t flags;
> int r;
>
> @@ -927,7 +929,8 @@ int amdgpu_ttm_alloc_gart(struct ttm_buffer_object *bo)
> {
> struct amdgpu_device *adev = amdgpu_ttm_adev(bo->bdev);
> struct ttm_operation_ctx ctx = { false, false };
> - struct amdgpu_ttm_tt *gtt = (void *)bo->ttm;
> + struct ttm_tt *ttm = bo->ttm;
> + struct amdgpu_ttm_tt *gtt = ttm_to_amdgpu_ttm_tt(ttm);
Is the extra ttm local variable really needed? It's only used once in
this function, right here.
> struct ttm_placement placement;
> struct ttm_place placements;
> struct ttm_resource *tmp;
> @@ -998,7 +1001,7 @@ static void amdgpu_ttm_backend_unbind(struct ttm_device *bdev,
> struct ttm_tt *ttm)
> {
> struct amdgpu_device *adev = amdgpu_ttm_adev(bdev);
> - struct amdgpu_ttm_tt *gtt = (void *)ttm;
> + struct amdgpu_ttm_tt *gtt = ttm_to_amdgpu_ttm_tt(ttm);
>
> /* if the pages have userptr pinning then clear that first */
> if (gtt->userptr) {
> @@ -1025,7 +1028,7 @@ static void amdgpu_ttm_backend_unbind(struct ttm_device *bdev,
> static void amdgpu_ttm_backend_destroy(struct ttm_device *bdev,
> struct ttm_tt *ttm)
> {
> - struct amdgpu_ttm_tt *gtt = (void *)ttm;
> + struct amdgpu_ttm_tt *gtt = ttm_to_amdgpu_ttm_tt(ttm);
>
> if (gtt->usertask)
> put_task_struct(gtt->usertask);
> @@ -1079,7 +1082,7 @@ static int amdgpu_ttm_tt_populate(struct ttm_device *bdev,
> struct ttm_operation_ctx *ctx)
> {
> struct amdgpu_device *adev = amdgpu_ttm_adev(bdev);
> - struct amdgpu_ttm_tt *gtt = (void *)ttm;
> + struct amdgpu_ttm_tt *gtt = ttm_to_amdgpu_ttm_tt(ttm);
> pgoff_t i;
> int ret;
>
> @@ -1113,7 +1116,7 @@ static int amdgpu_ttm_tt_populate(struct ttm_device *bdev,
> static void amdgpu_ttm_tt_unpopulate(struct ttm_device *bdev,
> struct ttm_tt *ttm)
> {
> - struct amdgpu_ttm_tt *gtt = (void *)ttm;
> + struct amdgpu_ttm_tt *gtt = ttm_to_amdgpu_ttm_tt(ttm);
> struct amdgpu_device *adev;
> pgoff_t i;
>
> @@ -1171,18 +1174,19 @@ int amdgpu_ttm_tt_set_userptr(struct ttm_buffer_object *bo,
> uint64_t addr, uint32_t flags)
> {
> struct amdgpu_ttm_tt *gtt;
> + struct ttm_tt *ttm = bo->ttm;
>
> - if (!bo->ttm) {
> + if (!ttm) {
> /* TODO: We want a separate TTM object type for userptrs */
> - bo->ttm = amdgpu_ttm_tt_create(bo, 0);
You're removing the assignment to bo->ttm here. I don't see where you're
adding it back. Basically you're storing the new ttm in a local
variable, and it will be forgotten when the function returns.
Regards,
Felix
> - if (bo->ttm == NULL)
> + ttm = amdgpu_ttm_tt_create(bo, 0);
> + if (ttm == NULL)
> return -ENOMEM;
> }
>
> /* Set TTM_TT_FLAG_EXTERNAL before populate but after create. */
> - bo->ttm->page_flags |= TTM_TT_FLAG_EXTERNAL;
> + ttm->page_flags |= TTM_TT_FLAG_EXTERNAL;
>
> - gtt = (void *)bo->ttm;
> + gtt = ttm_to_amdgpu_ttm_tt(ttm);
> gtt->userptr = addr;
> gtt->userflags = flags;
>
> @@ -1199,7 +1203,7 @@ int amdgpu_ttm_tt_set_userptr(struct ttm_buffer_object *bo,
> */
> struct mm_struct *amdgpu_ttm_tt_get_usermm(struct ttm_tt *ttm)
> {
> - struct amdgpu_ttm_tt *gtt = (void *)ttm;
> + struct amdgpu_ttm_tt *gtt = ttm_to_amdgpu_ttm_tt(ttm);
>
> if (gtt == NULL)
> return NULL;
> @@ -1218,7 +1222,7 @@ struct mm_struct *amdgpu_ttm_tt_get_usermm(struct ttm_tt *ttm)
> bool amdgpu_ttm_tt_affect_userptr(struct ttm_tt *ttm, unsigned long start,
> unsigned long end, unsigned long *userptr)
> {
> - struct amdgpu_ttm_tt *gtt = (void *)ttm;
> + struct amdgpu_ttm_tt *gtt = ttm_to_amdgpu_ttm_tt(ttm);
> unsigned long size;
>
> if (gtt == NULL || !gtt->userptr)
> @@ -1241,7 +1245,7 @@ bool amdgpu_ttm_tt_affect_userptr(struct ttm_tt *ttm, unsigned long start,
> */
> bool amdgpu_ttm_tt_is_userptr(struct ttm_tt *ttm)
> {
> - struct amdgpu_ttm_tt *gtt = (void *)ttm;
> + struct amdgpu_ttm_tt *gtt = ttm_to_amdgpu_ttm_tt(ttm);
>
> if (gtt == NULL || !gtt->userptr)
> return false;
> @@ -1254,7 +1258,7 @@ bool amdgpu_ttm_tt_is_userptr(struct ttm_tt *ttm)
> */
> bool amdgpu_ttm_tt_is_readonly(struct ttm_tt *ttm)
> {
> - struct amdgpu_ttm_tt *gtt = (void *)ttm;
> + struct amdgpu_ttm_tt *gtt = ttm_to_amdgpu_ttm_tt(ttm);
>
> if (gtt == NULL)
> return false;
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2022-07-26 23:56 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-26 23:43 [PATCH] drm/amdgpu: Avoid direct cast to amdgpu_ttm_tt Rajneesh Bhardwaj
2022-07-26 23:55 ` Felix Kuehling
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).