Re: [PATCH 2/3] drm/amdgpu: Replace TTM initialization/release with ttm_global
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
>> +#include
>> #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_globa
Re: [PATCH 2/3] drm/amdgpu: Replace TTM initialization/release with ttm_global
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
---
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
+#include
#define AMDGPU_PL_GDS (TTM_PL_PRIV + 0)
#define AMDGPU_PL_GWS (TTM_PL_PRIV + 1)
@@ -39,8 +40,7 @@
#define AMDGPU_GTT_NUM_T
Re: [PATCH 2/3] drm/amdgpu: Replace TTM initialization/release with ttm_global
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
Reviewed-by: Huang Rui
> ---
> 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
> +#include
>
> #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
[PATCH 2/3] drm/amdgpu: Replace TTM initialization/release with ttm_global
Unified initialization and relesae of the global TTM state is provided
by struct ttm_global and its interfaces.
Signed-off-by: Thomas Zimmermann
---
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
+#include
#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_WINDOWS2
struct amdgpu_mman {
- struct ttm_bo_global_refbo_global_ref;
- struct drm_global_reference mem_global_ref;
+ struct ttm_global glob;
struct ttm_bo_devicebdev;
boolmem_global_referenced;
bool
