| Message ID | 20181018162752.2241-3-tzimmermann@suse.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Provide struct ttm_global for TTM global state | expand |
On Fri, Oct 19, 2018 at 12:27:51AM +0800, Thomas Zimmermann wrote: > Unified initialization and relesae of the global TTM state is provided > by struct ttm_global and its interfaces. > > Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> Reviewed-by: Huang Rui <ray.huang@amd.com> > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 63 ++----------------------- > drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h | 4 +- > 2 files changed, 7 insertions(+), 60 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c > index 3a6802846698..70b0e8c77bb4 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c > @@ -65,33 +65,6 @@ static void amdgpu_ttm_debugfs_fini(struct amdgpu_device *adev); > * Global memory. > */ > > -/** > - * amdgpu_ttm_mem_global_init - Initialize and acquire reference to > - * memory object > - * > - * @ref: Object for initialization. > - * > - * This is called by drm_global_item_ref() when an object is being > - * initialized. > - */ > -static int amdgpu_ttm_mem_global_init(struct drm_global_reference *ref) > -{ > - return ttm_mem_global_init(ref->object); > -} > - > -/** > - * amdgpu_ttm_mem_global_release - Drop reference to a memory object > - * > - * @ref: Object being removed > - * > - * This is called by drm_global_item_unref() when an object is being > - * released. > - */ > -static void amdgpu_ttm_mem_global_release(struct drm_global_reference *ref) > -{ > - ttm_mem_global_release(ref->object); > -} > - > /** > * amdgpu_ttm_global_init - Initialize global TTM memory reference structures. > * > @@ -102,35 +75,15 @@ static void amdgpu_ttm_mem_global_release(struct drm_global_reference *ref) > */ > static int amdgpu_ttm_global_init(struct amdgpu_device *adev) > { > - struct drm_global_reference *global_ref; > int r; > > /* ensure reference is false in case init fails */ > adev->mman.mem_global_referenced = false; > > - global_ref = &adev->mman.mem_global_ref; > - global_ref->global_type = DRM_GLOBAL_TTM_MEM; > - global_ref->size = sizeof(struct ttm_mem_global); > - global_ref->init = &amdgpu_ttm_mem_global_init; > - global_ref->release = &amdgpu_ttm_mem_global_release; > - r = drm_global_item_ref(global_ref); > + r = ttm_global_init(&adev->mman.glob); > if (r) { > - DRM_ERROR("Failed setting up TTM memory accounting " > - "subsystem.\n"); > - goto error_mem; > - } > - > - adev->mman.bo_global_ref.mem_glob = > - adev->mman.mem_global_ref.object; > - global_ref = &adev->mman.bo_global_ref.ref; > - global_ref->global_type = DRM_GLOBAL_TTM_BO; > - global_ref->size = sizeof(struct ttm_bo_global); > - global_ref->init = &ttm_bo_global_ref_init; > - global_ref->release = &ttm_bo_global_ref_release; > - r = drm_global_item_ref(global_ref); > - if (r) { > - DRM_ERROR("Failed setting up TTM BO subsystem.\n"); > - goto error_bo; > + DRM_ERROR("Failed setting up TTM subsystem.\n"); > + return r; > } > > mutex_init(&adev->mman.gtt_window_lock); > @@ -138,19 +91,13 @@ static int amdgpu_ttm_global_init(struct amdgpu_device *adev) > adev->mman.mem_global_referenced = true; > > return 0; > - > -error_bo: > - drm_global_item_unref(&adev->mman.mem_global_ref); > -error_mem: > - return r; > } > > static void amdgpu_ttm_global_fini(struct amdgpu_device *adev) > { > if (adev->mman.mem_global_referenced) { > mutex_destroy(&adev->mman.gtt_window_lock); > - drm_global_item_unref(&adev->mman.bo_global_ref.ref); > - drm_global_item_unref(&adev->mman.mem_global_ref); > + ttm_global_release(&adev->mman.glob); > adev->mman.mem_global_referenced = false; > } > } > @@ -1765,7 +1712,7 @@ int amdgpu_ttm_init(struct amdgpu_device *adev) > } > /* No others user of address space so set it to 0 */ > r = ttm_bo_device_init(&adev->mman.bdev, > - adev->mman.bo_global_ref.ref.object, > + ttm_global_get_bo_global(&adev->mman.glob), > &amdgpu_bo_driver, > adev->ddev->anon_inode->i_mapping, > DRM_FILE_PAGE_OFFSET, > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h > index fe8f276e9811..c3a7fe3ead3a 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h > @@ -26,6 +26,7 @@ > > #include "amdgpu.h" > #include <drm/gpu_scheduler.h> > +#include <drm/ttm/ttm_global.h> > > #define AMDGPU_PL_GDS (TTM_PL_PRIV + 0) > #define AMDGPU_PL_GWS (TTM_PL_PRIV + 1) > @@ -39,8 +40,7 @@ > #define AMDGPU_GTT_NUM_TRANSFER_WINDOWS 2 > > struct amdgpu_mman { > - struct ttm_bo_global_ref bo_global_ref; > - struct drm_global_reference mem_global_ref; > + struct ttm_global glob; > struct ttm_bo_device bdev; > bool mem_global_referenced; > bool initialized; > -- > 2.19.1 >
On 10/19/2018 12:27 AM, Thomas Zimmermann wrote: > Unified initialization and relesae of the global TTM state is provided > by struct ttm_global and its interfaces. > > Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 63 ++----------------------- > drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h | 4 +- > 2 files changed, 7 insertions(+), 60 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c > index 3a6802846698..70b0e8c77bb4 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c > @@ -65,33 +65,6 @@ static void amdgpu_ttm_debugfs_fini(struct amdgpu_device *adev); > * Global memory. > */ > > -/** > - * amdgpu_ttm_mem_global_init - Initialize and acquire reference to > - * memory object > - * > - * @ref: Object for initialization. > - * > - * This is called by drm_global_item_ref() when an object is being > - * initialized. > - */ > -static int amdgpu_ttm_mem_global_init(struct drm_global_reference *ref) > -{ > - return ttm_mem_global_init(ref->object); > -} > - > -/** > - * amdgpu_ttm_mem_global_release - Drop reference to a memory object > - * > - * @ref: Object being removed > - * > - * This is called by drm_global_item_unref() when an object is being > - * released. > - */ > -static void amdgpu_ttm_mem_global_release(struct drm_global_reference *ref) > -{ > - ttm_mem_global_release(ref->object); > -} > - > /** > * amdgpu_ttm_global_init - Initialize global TTM memory reference structures. > * > @@ -102,35 +75,15 @@ static void amdgpu_ttm_mem_global_release(struct drm_global_reference *ref) > */ > static int amdgpu_ttm_global_init(struct amdgpu_device *adev) > { > - struct drm_global_reference *global_ref; > int r; > > /* ensure reference is false in case init fails */ > adev->mman.mem_global_referenced = false; > > - global_ref = &adev->mman.mem_global_ref; > - global_ref->global_type = DRM_GLOBAL_TTM_MEM; > - global_ref->size = sizeof(struct ttm_mem_global); > - global_ref->init = &amdgpu_ttm_mem_global_init; > - global_ref->release = &amdgpu_ttm_mem_global_release; > - r = drm_global_item_ref(global_ref); > + r = ttm_global_init(&adev->mman.glob); > if (r) { > - DRM_ERROR("Failed setting up TTM memory accounting " > - "subsystem.\n"); > - goto error_mem; > - } > - > - adev->mman.bo_global_ref.mem_glob = > - adev->mman.mem_global_ref.object; Seems to miss this action. Or are you going to replace struct ttm_bo_global_ref bo_global_ref with struct ttm_global glob -> struct drm_global_reference bo_ref if so, may need to remove ttm_bo_global_ref_init() and struct ttm_bo_global_ref at the same time. Regards, Jerry > - global_ref = &adev->mman.bo_global_ref.ref; > - global_ref->global_type = DRM_GLOBAL_TTM_BO; > - global_ref->size = sizeof(struct ttm_bo_global); > - global_ref->init = &ttm_bo_global_ref_init; > - global_ref->release = &ttm_bo_global_ref_release; > - r = drm_global_item_ref(global_ref); > - if (r) { > - DRM_ERROR("Failed setting up TTM BO subsystem.\n"); > - goto error_bo; > + DRM_ERROR("Failed setting up TTM subsystem.\n"); > + return r; > } > > mutex_init(&adev->mman.gtt_window_lock); > @@ -138,19 +91,13 @@ static int amdgpu_ttm_global_init(struct amdgpu_device *adev) > adev->mman.mem_global_referenced = true; > > return 0; > - > -error_bo: > - drm_global_item_unref(&adev->mman.mem_global_ref); > -error_mem: > - return r; > } > > static void amdgpu_ttm_global_fini(struct amdgpu_device *adev) > { > if (adev->mman.mem_global_referenced) { > mutex_destroy(&adev->mman.gtt_window_lock); > - drm_global_item_unref(&adev->mman.bo_global_ref.ref); > - drm_global_item_unref(&adev->mman.mem_global_ref); > + ttm_global_release(&adev->mman.glob); > adev->mman.mem_global_referenced = false; > } > } > @@ -1765,7 +1712,7 @@ int amdgpu_ttm_init(struct amdgpu_device *adev) > } > /* No others user of address space so set it to 0 */ > r = ttm_bo_device_init(&adev->mman.bdev, > - adev->mman.bo_global_ref.ref.object, > + ttm_global_get_bo_global(&adev->mman.glob), > &amdgpu_bo_driver, > adev->ddev->anon_inode->i_mapping, > DRM_FILE_PAGE_OFFSET, > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h > index fe8f276e9811..c3a7fe3ead3a 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h > @@ -26,6 +26,7 @@ > > #include "amdgpu.h" > #include <drm/gpu_scheduler.h> > +#include <drm/ttm/ttm_global.h> > > #define AMDGPU_PL_GDS (TTM_PL_PRIV + 0) > #define AMDGPU_PL_GWS (TTM_PL_PRIV + 1) > @@ -39,8 +40,7 @@ > #define AMDGPU_GTT_NUM_TRANSFER_WINDOWS 2 > > struct amdgpu_mman { > - struct ttm_bo_global_ref bo_global_ref; > - struct drm_global_reference mem_global_ref; > + struct ttm_global glob; > struct ttm_bo_device bdev; > bool mem_global_referenced; > bool initialized;
Hi, thanks for reviewing the patch. Am 19.10.18 um 08:18 schrieb Zhang, Jerry(Junwei): > On 10/19/2018 12:27 AM, Thomas Zimmermann wrote: >> /** >> * amdgpu_ttm_global_init - Initialize global TTM memory reference >> structures. >> * >> @@ -102,35 +75,15 @@ static void amdgpu_ttm_mem_global_release(struct >> drm_global_reference *ref) >> */ >> static int amdgpu_ttm_global_init(struct amdgpu_device *adev) >> { >> - struct drm_global_reference *global_ref; >> int r; >> /* ensure reference is false in case init fails */ >> adev->mman.mem_global_referenced = false; >> - global_ref = &adev->mman.mem_global_ref; >> - global_ref->global_type = DRM_GLOBAL_TTM_MEM; >> - global_ref->size = sizeof(struct ttm_mem_global); >> - global_ref->init = &amdgpu_ttm_mem_global_init; >> - global_ref->release = &amdgpu_ttm_mem_global_release; >> - r = drm_global_item_ref(global_ref); >> + r = ttm_global_init(&adev->mman.glob); >> if (r) { >> - DRM_ERROR("Failed setting up TTM memory accounting " >> - "subsystem.\n"); >> - goto error_mem; >> - } >> - >> - adev->mman.bo_global_ref.mem_glob = >> - adev->mman.mem_global_ref.object; > > Seems to miss this action. Should be fine. This was a workaround for setting up the global BO's memory. It received the value that is here stored in adev->mman.bo_global_ref.mem_glob. Earlier this week, a patch set was merged into the TTM tree that cleans up the init interfaces of ttm_bo_global. [1] There's now int ttm_bo_global_init(struct ttm_bo_global *glob, struct ttm_mem_global *mem_glob) which takes the the global memory as second argument. In patch [1/3] we call ttm_bo_global_init() from ttm_global_init_bo() with the correct global memory. So this workaround is not needed any longer. > Or are you going to replace > struct ttm_bo_global_ref bo_global_ref > with > struct ttm_global glob -> struct drm_global_reference bo_ref > > if so, may need to remove ttm_bo_global_ref_init() and struct > ttm_bo_global_ref at the same time. > > I have a patch set for removing ttm_bo_global_ref and its interfaces, but it has to wait until all drivers have been converted to struct ttm_global. struct drm_global_reference will then become an implementation detail of struct ttm_global and can be removed from DRM's public interfaces. Best regards Thomas [1] https://cgit.freedesktop.org/~agd5f/linux/commit/?h=drm-next-4.21-wip&id=1fbb364ae523177bd0fac81d32befb3c9eb1b37e > Regards, > Jerry > >> - global_ref = &adev->mman.bo_global_ref.ref; >> - global_ref->global_type = DRM_GLOBAL_TTM_BO; >> - global_ref->size = sizeof(struct ttm_bo_global); >> - global_ref->init = &ttm_bo_global_ref_init; >> - global_ref->release = &ttm_bo_global_ref_release; >> - r = drm_global_item_ref(global_ref); >> - if (r) { >> - DRM_ERROR("Failed setting up TTM BO subsystem.\n"); >> - goto error_bo; >> + DRM_ERROR("Failed setting up TTM subsystem.\n"); >> + return r; >> } >> mutex_init(&adev->mman.gtt_window_lock); >> @@ -138,19 +91,13 @@ static int amdgpu_ttm_global_init(struct >> amdgpu_device *adev) >> adev->mman.mem_global_referenced = true; >> return 0; >> - >> -error_bo: >> - drm_global_item_unref(&adev->mman.mem_global_ref); >> -error_mem: >> - return r; >> } >> static void amdgpu_ttm_global_fini(struct amdgpu_device *adev) >> { >> if (adev->mman.mem_global_referenced) { >> mutex_destroy(&adev->mman.gtt_window_lock); >> - drm_global_item_unref(&adev->mman.bo_global_ref.ref); >> - drm_global_item_unref(&adev->mman.mem_global_ref); >> + ttm_global_release(&adev->mman.glob); >> adev->mman.mem_global_referenced = false; >> } >> } >> @@ -1765,7 +1712,7 @@ int amdgpu_ttm_init(struct amdgpu_device *adev) >> } >> /* No others user of address space so set it to 0 */ >> r = ttm_bo_device_init(&adev->mman.bdev, >> - adev->mman.bo_global_ref.ref.object, >> + ttm_global_get_bo_global(&adev->mman.glob), >> &amdgpu_bo_driver, >> adev->ddev->anon_inode->i_mapping, >> DRM_FILE_PAGE_OFFSET, >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h >> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h >> index fe8f276e9811..c3a7fe3ead3a 100644 >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h >> @@ -26,6 +26,7 @@ >> #include "amdgpu.h" >> #include <drm/gpu_scheduler.h> >> +#include <drm/ttm/ttm_global.h> >> #define AMDGPU_PL_GDS (TTM_PL_PRIV + 0) >> #define AMDGPU_PL_GWS (TTM_PL_PRIV + 1) >> @@ -39,8 +40,7 @@ >> #define AMDGPU_GTT_NUM_TRANSFER_WINDOWS 2 >> struct amdgpu_mman { >> - struct ttm_bo_global_ref bo_global_ref; >> - struct drm_global_reference mem_global_ref; >> + struct ttm_global glob; >> struct ttm_bo_device bdev; >> bool mem_global_referenced; >> bool initialized; >
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c index 3a6802846698..70b0e8c77bb4 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c @@ -65,33 +65,6 @@ static void amdgpu_ttm_debugfs_fini(struct amdgpu_device *adev); * Global memory. */ -/** - * amdgpu_ttm_mem_global_init - Initialize and acquire reference to - * memory object - * - * @ref: Object for initialization. - * - * This is called by drm_global_item_ref() when an object is being - * initialized. - */ -static int amdgpu_ttm_mem_global_init(struct drm_global_reference *ref) -{ - return ttm_mem_global_init(ref->object); -} - -/** - * amdgpu_ttm_mem_global_release - Drop reference to a memory object - * - * @ref: Object being removed - * - * This is called by drm_global_item_unref() when an object is being - * released. - */ -static void amdgpu_ttm_mem_global_release(struct drm_global_reference *ref) -{ - ttm_mem_global_release(ref->object); -} - /** * amdgpu_ttm_global_init - Initialize global TTM memory reference structures. * @@ -102,35 +75,15 @@ static void amdgpu_ttm_mem_global_release(struct drm_global_reference *ref) */ static int amdgpu_ttm_global_init(struct amdgpu_device *adev) { - struct drm_global_reference *global_ref; int r; /* ensure reference is false in case init fails */ adev->mman.mem_global_referenced = false; - global_ref = &adev->mman.mem_global_ref; - global_ref->global_type = DRM_GLOBAL_TTM_MEM; - global_ref->size = sizeof(struct ttm_mem_global); - global_ref->init = &amdgpu_ttm_mem_global_init; - global_ref->release = &amdgpu_ttm_mem_global_release; - r = drm_global_item_ref(global_ref); + r = ttm_global_init(&adev->mman.glob); if (r) { - DRM_ERROR("Failed setting up TTM memory accounting " - "subsystem.\n"); - goto error_mem; - } - - adev->mman.bo_global_ref.mem_glob = - adev->mman.mem_global_ref.object; - global_ref = &adev->mman.bo_global_ref.ref; - global_ref->global_type = DRM_GLOBAL_TTM_BO; - global_ref->size = sizeof(struct ttm_bo_global); - global_ref->init = &ttm_bo_global_ref_init; - global_ref->release = &ttm_bo_global_ref_release; - r = drm_global_item_ref(global_ref); - if (r) { - DRM_ERROR("Failed setting up TTM BO subsystem.\n"); - goto error_bo; + DRM_ERROR("Failed setting up TTM subsystem.\n"); + return r; } mutex_init(&adev->mman.gtt_window_lock); @@ -138,19 +91,13 @@ static int amdgpu_ttm_global_init(struct amdgpu_device *adev) adev->mman.mem_global_referenced = true; return 0; - -error_bo: - drm_global_item_unref(&adev->mman.mem_global_ref); -error_mem: - return r; } static void amdgpu_ttm_global_fini(struct amdgpu_device *adev) { if (adev->mman.mem_global_referenced) { mutex_destroy(&adev->mman.gtt_window_lock); - drm_global_item_unref(&adev->mman.bo_global_ref.ref); - drm_global_item_unref(&adev->mman.mem_global_ref); + ttm_global_release(&adev->mman.glob); adev->mman.mem_global_referenced = false; } } @@ -1765,7 +1712,7 @@ int amdgpu_ttm_init(struct amdgpu_device *adev) } /* No others user of address space so set it to 0 */ r = ttm_bo_device_init(&adev->mman.bdev, - adev->mman.bo_global_ref.ref.object, + ttm_global_get_bo_global(&adev->mman.glob), &amdgpu_bo_driver, adev->ddev->anon_inode->i_mapping, DRM_FILE_PAGE_OFFSET, diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h index fe8f276e9811..c3a7fe3ead3a 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h @@ -26,6 +26,7 @@ #include "amdgpu.h" #include <drm/gpu_scheduler.h> +#include <drm/ttm/ttm_global.h> #define AMDGPU_PL_GDS (TTM_PL_PRIV + 0) #define AMDGPU_PL_GWS (TTM_PL_PRIV + 1) @@ -39,8 +40,7 @@ #define AMDGPU_GTT_NUM_TRANSFER_WINDOWS 2 struct amdgpu_mman { - struct ttm_bo_global_ref bo_global_ref; - struct drm_global_reference mem_global_ref; + struct ttm_global glob; struct ttm_bo_device bdev; bool mem_global_referenced; bool initialized;
Unified initialization and relesae of the global TTM state is provided by struct ttm_global and its interfaces. Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> --- drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 63 ++----------------------- drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h | 4 +- 2 files changed, 7 insertions(+), 60 deletions(-)