* [PATCH v2 1/8] drm/amdgpu: add UAPI for allocating doorbell memory
2023-02-14 16:15 [PATCH v2 0/8] Re-design doorbell framework for usermode queues Shashank Sharma
@ 2023-02-14 16:15 ` Shashank Sharma
2023-02-14 18:22 ` Christian König
2023-02-14 16:15 ` [PATCH v2 2/8] drm/amdgpu: replace aper_base_kaddr with vram_aper_base_kaddr Shashank Sharma
` (6 subsequent siblings)
7 siblings, 1 reply; 28+ messages in thread
From: Shashank Sharma @ 2023-02-14 16:15 UTC (permalink / raw)
To: amd-gfx; +Cc: alexander.deucher, christian.koenig, Arvind.Yadav,
shashank.sharma
From: Alex Deucher <alexander.deucher@amd.com>
This patch adds flags for a new gem domain AMDGPU_GEM_DOMAIN_DOORBELL
in the UAPI layer.
V2: Drop 'memory' from description (Christian)
Cc: Alex Deucher <alexander.deucher@amd.com>
Cc: Christian Koenig <christian.koenig@amd.com>
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
include/uapi/drm/amdgpu_drm.h | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/include/uapi/drm/amdgpu_drm.h b/include/uapi/drm/amdgpu_drm.h
index 4038abe8505a..cc5d551abda5 100644
--- a/include/uapi/drm/amdgpu_drm.h
+++ b/include/uapi/drm/amdgpu_drm.h
@@ -94,6 +94,9 @@ extern "C" {
*
* %AMDGPU_GEM_DOMAIN_OA Ordered append, used by 3D or Compute engines
* for appending data.
+ *
+ * %AMDGPU_GEM_DOMAIN_DOORBELL Doorbell. It is an MMIO region for
+ * signalling user mode queues.
*/
#define AMDGPU_GEM_DOMAIN_CPU 0x1
#define AMDGPU_GEM_DOMAIN_GTT 0x2
@@ -101,12 +104,14 @@ extern "C" {
#define AMDGPU_GEM_DOMAIN_GDS 0x8
#define AMDGPU_GEM_DOMAIN_GWS 0x10
#define AMDGPU_GEM_DOMAIN_OA 0x20
+#define AMDGPU_GEM_DOMAIN_DOORBELL 0x40
#define AMDGPU_GEM_DOMAIN_MASK (AMDGPU_GEM_DOMAIN_CPU | \
AMDGPU_GEM_DOMAIN_GTT | \
AMDGPU_GEM_DOMAIN_VRAM | \
AMDGPU_GEM_DOMAIN_GDS | \
AMDGPU_GEM_DOMAIN_GWS | \
- AMDGPU_GEM_DOMAIN_OA)
+ AMDGPU_GEM_DOMAIN_OA | \
+ AMDGPU_GEM_DOMAIN_DOORBELL)
/* Flag that CPU access will be required for the case of VRAM domain */
#define AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED (1 << 0)
--
2.34.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH v2 1/8] drm/amdgpu: add UAPI for allocating doorbell memory
2023-02-14 16:15 ` [PATCH v2 1/8] drm/amdgpu: add UAPI for allocating doorbell memory Shashank Sharma
@ 2023-02-14 18:22 ` Christian König
2023-02-14 19:02 ` Shashank Sharma
0 siblings, 1 reply; 28+ messages in thread
From: Christian König @ 2023-02-14 18:22 UTC (permalink / raw)
To: Shashank Sharma, amd-gfx; +Cc: alexander.deucher, Arvind.Yadav
Am 14.02.23 um 17:15 schrieb Shashank Sharma:
> From: Alex Deucher <alexander.deucher@amd.com>
>
> This patch adds flags for a new gem domain AMDGPU_GEM_DOMAIN_DOORBELL
> in the UAPI layer.
>
> V2: Drop 'memory' from description (Christian)
>
> Cc: Alex Deucher <alexander.deucher@amd.com>
> Cc: Christian Koenig <christian.koenig@amd.com>
> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> ---
> include/uapi/drm/amdgpu_drm.h | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/include/uapi/drm/amdgpu_drm.h b/include/uapi/drm/amdgpu_drm.h
> index 4038abe8505a..cc5d551abda5 100644
> --- a/include/uapi/drm/amdgpu_drm.h
> +++ b/include/uapi/drm/amdgpu_drm.h
> @@ -94,6 +94,9 @@ extern "C" {
> *
> * %AMDGPU_GEM_DOMAIN_OA Ordered append, used by 3D or Compute engines
> * for appending data.
> + *
> + * %AMDGPU_GEM_DOMAIN_DOORBELL Doorbell. It is an MMIO region for
> + * signalling user mode queues.
Maybe write "for signaling events to the firmware, used especially for
user mode queues.".
With or without that Reviewed-by: Christian König <christian.koenig@amd.com>
Christian.
> */
> #define AMDGPU_GEM_DOMAIN_CPU 0x1
> #define AMDGPU_GEM_DOMAIN_GTT 0x2
> @@ -101,12 +104,14 @@ extern "C" {
> #define AMDGPU_GEM_DOMAIN_GDS 0x8
> #define AMDGPU_GEM_DOMAIN_GWS 0x10
> #define AMDGPU_GEM_DOMAIN_OA 0x20
> +#define AMDGPU_GEM_DOMAIN_DOORBELL 0x40
> #define AMDGPU_GEM_DOMAIN_MASK (AMDGPU_GEM_DOMAIN_CPU | \
> AMDGPU_GEM_DOMAIN_GTT | \
> AMDGPU_GEM_DOMAIN_VRAM | \
> AMDGPU_GEM_DOMAIN_GDS | \
> AMDGPU_GEM_DOMAIN_GWS | \
> - AMDGPU_GEM_DOMAIN_OA)
> + AMDGPU_GEM_DOMAIN_OA | \
> + AMDGPU_GEM_DOMAIN_DOORBELL)
>
> /* Flag that CPU access will be required for the case of VRAM domain */
> #define AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED (1 << 0)
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v2 1/8] drm/amdgpu: add UAPI for allocating doorbell memory
2023-02-14 18:22 ` Christian König
@ 2023-02-14 19:02 ` Shashank Sharma
0 siblings, 0 replies; 28+ messages in thread
From: Shashank Sharma @ 2023-02-14 19:02 UTC (permalink / raw)
To: Christian König, amd-gfx; +Cc: alexander.deucher, Arvind.Yadav
On 14/02/2023 19:22, Christian König wrote:
> Am 14.02.23 um 17:15 schrieb Shashank Sharma:
>> From: Alex Deucher <alexander.deucher@amd.com>
>>
>> This patch adds flags for a new gem domain AMDGPU_GEM_DOMAIN_DOORBELL
>> in the UAPI layer.
>>
>> V2: Drop 'memory' from description (Christian)
>>
>> Cc: Alex Deucher <alexander.deucher@amd.com>
>> Cc: Christian Koenig <christian.koenig@amd.com>
>> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
>> ---
>> include/uapi/drm/amdgpu_drm.h | 7 ++++++-
>> 1 file changed, 6 insertions(+), 1 deletion(-)
>>
>> diff --git a/include/uapi/drm/amdgpu_drm.h
>> b/include/uapi/drm/amdgpu_drm.h
>> index 4038abe8505a..cc5d551abda5 100644
>> --- a/include/uapi/drm/amdgpu_drm.h
>> +++ b/include/uapi/drm/amdgpu_drm.h
>> @@ -94,6 +94,9 @@ extern "C" {
>> *
>> * %AMDGPU_GEM_DOMAIN_OA Ordered append, used by 3D or Compute
>> engines
>> * for appending data.
>> + *
>> + * %AMDGPU_GEM_DOMAIN_DOORBELL Doorbell. It is an MMIO region for
>> + * signalling user mode queues.
>
> Maybe write "for signaling events to the firmware, used especially for
> user mode queues.".
>
> With or without that Reviewed-by: Christian König
> <christian.koenig@amd.com>
>
Will add that, thanks.
- Shashank
> Christian.
>
>> */
>> #define AMDGPU_GEM_DOMAIN_CPU 0x1
>> #define AMDGPU_GEM_DOMAIN_GTT 0x2
>> @@ -101,12 +104,14 @@ extern "C" {
>> #define AMDGPU_GEM_DOMAIN_GDS 0x8
>> #define AMDGPU_GEM_DOMAIN_GWS 0x10
>> #define AMDGPU_GEM_DOMAIN_OA 0x20
>> +#define AMDGPU_GEM_DOMAIN_DOORBELL 0x40
>> #define AMDGPU_GEM_DOMAIN_MASK (AMDGPU_GEM_DOMAIN_CPU | \
>> AMDGPU_GEM_DOMAIN_GTT | \
>> AMDGPU_GEM_DOMAIN_VRAM | \
>> AMDGPU_GEM_DOMAIN_GDS | \
>> AMDGPU_GEM_DOMAIN_GWS | \
>> - AMDGPU_GEM_DOMAIN_OA)
>> + AMDGPU_GEM_DOMAIN_OA | \
>> + AMDGPU_GEM_DOMAIN_DOORBELL)
>> /* Flag that CPU access will be required for the case of VRAM
>> domain */
>> #define AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED (1 << 0)
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v2 2/8] drm/amdgpu: replace aper_base_kaddr with vram_aper_base_kaddr
2023-02-14 16:15 [PATCH v2 0/8] Re-design doorbell framework for usermode queues Shashank Sharma
2023-02-14 16:15 ` [PATCH v2 1/8] drm/amdgpu: add UAPI for allocating doorbell memory Shashank Sharma
@ 2023-02-14 16:15 ` Shashank Sharma
2023-02-14 18:24 ` Christian König
2023-02-14 16:15 ` [PATCH v2 3/8] drm/amdgpu: rename gmc.aper_base/size Shashank Sharma
` (5 subsequent siblings)
7 siblings, 1 reply; 28+ messages in thread
From: Shashank Sharma @ 2023-02-14 16:15 UTC (permalink / raw)
To: amd-gfx
Cc: alexander.deucher, Christian Koenig, christian.koenig,
Arvind.Yadav, shashank.sharma
From: Alex Deucher <alexander.deucher@amd.com>
To differentiate it from the doorbell BAR.
V2: Added Christian's A-B
Acked-by: Christian Koenig <christian.koenig@amid.com>
Cc: Alex Deucher <alexander.deucher@amd.com>
Cc: Christian Koenig <christian.koenig@amd.com>
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 10 +++++-----
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 14 +++++++-------
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h | 2 +-
drivers/gpu/drm/amd/amdgpu/psp_v11_0.c | 10 +++++-----
drivers/gpu/drm/amd/amdgpu/psp_v13_0.c | 10 +++++-----
5 files changed, 23 insertions(+), 23 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 2f28a8c02f64..0b6a394e109b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -354,12 +354,12 @@ size_t amdgpu_device_aper_access(struct amdgpu_device *adev, loff_t pos,
size_t count = 0;
uint64_t last;
- if (!adev->mman.aper_base_kaddr)
+ if (!adev->mman.vram_aper_base_kaddr)
return 0;
last = min(pos + size, adev->gmc.visible_vram_size);
if (last > pos) {
- addr = adev->mman.aper_base_kaddr + pos;
+ addr = adev->mman.vram_aper_base_kaddr + pos;
count = last - pos;
if (write) {
@@ -3954,9 +3954,9 @@ static void amdgpu_device_unmap_mmio(struct amdgpu_device *adev)
iounmap(adev->rmmio);
adev->rmmio = NULL;
- if (adev->mman.aper_base_kaddr)
- iounmap(adev->mman.aper_base_kaddr);
- adev->mman.aper_base_kaddr = NULL;
+ if (adev->mman.vram_aper_base_kaddr)
+ iounmap(adev->mman.vram_aper_base_kaddr);
+ adev->mman.vram_aper_base_kaddr = NULL;
/* Memory manager related */
if (!adev->gmc.xgmi.connected_to_cpu) {
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 55e0284b2bdd..73b831b47892 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -578,9 +578,9 @@ static int amdgpu_ttm_io_mem_reserve(struct ttm_device *bdev,
if ((mem->bus.offset + bus_size) > adev->gmc.visible_vram_size)
return -EINVAL;
- if (adev->mman.aper_base_kaddr &&
+ if (adev->mman.vram_aper_base_kaddr &&
mem->placement & TTM_PL_FLAG_CONTIGUOUS)
- mem->bus.addr = (u8 *)adev->mman.aper_base_kaddr +
+ mem->bus.addr = (u8 *)adev->mman.vram_aper_base_kaddr +
mem->bus.offset;
mem->bus.offset += adev->gmc.aper_base;
@@ -1752,12 +1752,12 @@ int amdgpu_ttm_init(struct amdgpu_device *adev)
#ifdef CONFIG_64BIT
#ifdef CONFIG_X86
if (adev->gmc.xgmi.connected_to_cpu)
- adev->mman.aper_base_kaddr = ioremap_cache(adev->gmc.aper_base,
+ adev->mman.vram_aper_base_kaddr = ioremap_cache(adev->gmc.aper_base,
adev->gmc.visible_vram_size);
else
#endif
- adev->mman.aper_base_kaddr = ioremap_wc(adev->gmc.aper_base,
+ adev->mman.vram_aper_base_kaddr = ioremap_wc(adev->gmc.aper_base,
adev->gmc.visible_vram_size);
#endif
@@ -1904,9 +1904,9 @@ void amdgpu_ttm_fini(struct amdgpu_device *adev)
if (drm_dev_enter(adev_to_drm(adev), &idx)) {
- if (adev->mman.aper_base_kaddr)
- iounmap(adev->mman.aper_base_kaddr);
- adev->mman.aper_base_kaddr = NULL;
+ if (adev->mman.vram_aper_base_kaddr)
+ iounmap(adev->mman.vram_aper_base_kaddr);
+ adev->mman.vram_aper_base_kaddr = NULL;
drm_dev_exit(idx);
}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
index e2cd5894afc9..929bc8abac28 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
@@ -50,7 +50,7 @@ struct amdgpu_gtt_mgr {
struct amdgpu_mman {
struct ttm_device bdev;
bool initialized;
- void __iomem *aper_base_kaddr;
+ void __iomem *vram_aper_base_kaddr;
/* buffer handling */
const struct amdgpu_buffer_funcs *buffer_funcs;
diff --git a/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c b/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c
index bd3e3e23a939..f39d4f593a2f 100644
--- a/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c
@@ -611,10 +611,10 @@ static int psp_v11_0_memory_training(struct psp_context *psp, uint32_t ops)
*/
sz = GDDR6_MEM_TRAINING_ENCROACHED_SIZE;
- if (adev->gmc.visible_vram_size < sz || !adev->mman.aper_base_kaddr) {
- DRM_ERROR("visible_vram_size %llx or aper_base_kaddr %p is not initialized.\n",
+ if (adev->gmc.visible_vram_size < sz || !adev->mman.vram_aper_base_kaddr) {
+ DRM_ERROR("visible_vram_size %llx or vram_aper_base_kaddr %p is not initialized.\n",
adev->gmc.visible_vram_size,
- adev->mman.aper_base_kaddr);
+ adev->mman.vram_aper_base_kaddr);
return -EINVAL;
}
@@ -625,7 +625,7 @@ static int psp_v11_0_memory_training(struct psp_context *psp, uint32_t ops)
}
if (drm_dev_enter(adev_to_drm(adev), &idx)) {
- memcpy_fromio(buf, adev->mman.aper_base_kaddr, sz);
+ memcpy_fromio(buf, adev->mman.vram_aper_base_kaddr, sz);
ret = psp_v11_0_memory_training_send_msg(psp, PSP_BL__DRAM_LONG_TRAIN);
if (ret) {
DRM_ERROR("Send long training msg failed.\n");
@@ -634,7 +634,7 @@ static int psp_v11_0_memory_training(struct psp_context *psp, uint32_t ops)
return ret;
}
- memcpy_toio(adev->mman.aper_base_kaddr, buf, sz);
+ memcpy_toio(adev->mman.vram_aper_base_kaddr, buf, sz);
adev->hdp.funcs->flush_hdp(adev, NULL);
vfree(buf);
drm_dev_exit(idx);
diff --git a/drivers/gpu/drm/amd/amdgpu/psp_v13_0.c b/drivers/gpu/drm/amd/amdgpu/psp_v13_0.c
index e6a26a7e5e5e..9605c0971c11 100644
--- a/drivers/gpu/drm/amd/amdgpu/psp_v13_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/psp_v13_0.c
@@ -510,10 +510,10 @@ static int psp_v13_0_memory_training(struct psp_context *psp, uint32_t ops)
*/
sz = GDDR6_MEM_TRAINING_ENCROACHED_SIZE;
- if (adev->gmc.visible_vram_size < sz || !adev->mman.aper_base_kaddr) {
- dev_err(adev->dev, "visible_vram_size %llx or aper_base_kaddr %p is not initialized.\n",
+ if (adev->gmc.visible_vram_size < sz || !adev->mman.vram_aper_base_kaddr) {
+ dev_err(adev->dev, "visible_vram_size %llx or vram_aper_base_kaddr %p is not initialized.\n",
adev->gmc.visible_vram_size,
- adev->mman.aper_base_kaddr);
+ adev->mman.vram_aper_base_kaddr);
return -EINVAL;
}
@@ -524,7 +524,7 @@ static int psp_v13_0_memory_training(struct psp_context *psp, uint32_t ops)
}
if (drm_dev_enter(adev_to_drm(adev), &idx)) {
- memcpy_fromio(buf, adev->mman.aper_base_kaddr, sz);
+ memcpy_fromio(buf, adev->mman.vram_aper_base_kaddr, sz);
ret = psp_v13_0_memory_training_send_msg(psp, PSP_BL__DRAM_LONG_TRAIN);
if (ret) {
DRM_ERROR("Send long training msg failed.\n");
@@ -533,7 +533,7 @@ static int psp_v13_0_memory_training(struct psp_context *psp, uint32_t ops)
return ret;
}
- memcpy_toio(adev->mman.aper_base_kaddr, buf, sz);
+ memcpy_toio(adev->mman.vram_aper_base_kaddr, buf, sz);
adev->hdp.funcs->flush_hdp(adev, NULL);
vfree(buf);
drm_dev_exit(idx);
--
2.34.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH v2 2/8] drm/amdgpu: replace aper_base_kaddr with vram_aper_base_kaddr
2023-02-14 16:15 ` [PATCH v2 2/8] drm/amdgpu: replace aper_base_kaddr with vram_aper_base_kaddr Shashank Sharma
@ 2023-02-14 18:24 ` Christian König
2023-02-14 19:03 ` Shashank Sharma
0 siblings, 1 reply; 28+ messages in thread
From: Christian König @ 2023-02-14 18:24 UTC (permalink / raw)
To: Shashank Sharma, amd-gfx
Cc: alexander.deucher, Arvind.Yadav, Christian Koenig
Am 14.02.23 um 17:15 schrieb Shashank Sharma:
> From: Alex Deucher <alexander.deucher@amd.com>
>
> To differentiate it from the doorbell BAR.
Since we removed the manual ioremap() for the doorbell BAR today we
don't really need that patch any more, don't we?
On the other hand renaming the field still makes a lot of sense for
better documenting what it's good for.
Christian.
>
> V2: Added Christian's A-B
>
> Acked-by: Christian Koenig <christian.koenig@amid.com>
>
> Cc: Alex Deucher <alexander.deucher@amd.com>
> Cc: Christian Koenig <christian.koenig@amd.com>
> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 10 +++++-----
> drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 14 +++++++-------
> drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h | 2 +-
> drivers/gpu/drm/amd/amdgpu/psp_v11_0.c | 10 +++++-----
> drivers/gpu/drm/amd/amdgpu/psp_v13_0.c | 10 +++++-----
> 5 files changed, 23 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index 2f28a8c02f64..0b6a394e109b 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -354,12 +354,12 @@ size_t amdgpu_device_aper_access(struct amdgpu_device *adev, loff_t pos,
> size_t count = 0;
> uint64_t last;
>
> - if (!adev->mman.aper_base_kaddr)
> + if (!adev->mman.vram_aper_base_kaddr)
> return 0;
>
> last = min(pos + size, adev->gmc.visible_vram_size);
> if (last > pos) {
> - addr = adev->mman.aper_base_kaddr + pos;
> + addr = adev->mman.vram_aper_base_kaddr + pos;
> count = last - pos;
>
> if (write) {
> @@ -3954,9 +3954,9 @@ static void amdgpu_device_unmap_mmio(struct amdgpu_device *adev)
>
> iounmap(adev->rmmio);
> adev->rmmio = NULL;
> - if (adev->mman.aper_base_kaddr)
> - iounmap(adev->mman.aper_base_kaddr);
> - adev->mman.aper_base_kaddr = NULL;
> + if (adev->mman.vram_aper_base_kaddr)
> + iounmap(adev->mman.vram_aper_base_kaddr);
> + adev->mman.vram_aper_base_kaddr = NULL;
>
> /* Memory manager related */
> if (!adev->gmc.xgmi.connected_to_cpu) {
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> index 55e0284b2bdd..73b831b47892 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> @@ -578,9 +578,9 @@ static int amdgpu_ttm_io_mem_reserve(struct ttm_device *bdev,
> if ((mem->bus.offset + bus_size) > adev->gmc.visible_vram_size)
> return -EINVAL;
>
> - if (adev->mman.aper_base_kaddr &&
> + if (adev->mman.vram_aper_base_kaddr &&
> mem->placement & TTM_PL_FLAG_CONTIGUOUS)
> - mem->bus.addr = (u8 *)adev->mman.aper_base_kaddr +
> + mem->bus.addr = (u8 *)adev->mman.vram_aper_base_kaddr +
> mem->bus.offset;
>
> mem->bus.offset += adev->gmc.aper_base;
> @@ -1752,12 +1752,12 @@ int amdgpu_ttm_init(struct amdgpu_device *adev)
> #ifdef CONFIG_64BIT
> #ifdef CONFIG_X86
> if (adev->gmc.xgmi.connected_to_cpu)
> - adev->mman.aper_base_kaddr = ioremap_cache(adev->gmc.aper_base,
> + adev->mman.vram_aper_base_kaddr = ioremap_cache(adev->gmc.aper_base,
> adev->gmc.visible_vram_size);
>
> else
> #endif
> - adev->mman.aper_base_kaddr = ioremap_wc(adev->gmc.aper_base,
> + adev->mman.vram_aper_base_kaddr = ioremap_wc(adev->gmc.aper_base,
> adev->gmc.visible_vram_size);
> #endif
>
> @@ -1904,9 +1904,9 @@ void amdgpu_ttm_fini(struct amdgpu_device *adev)
>
> if (drm_dev_enter(adev_to_drm(adev), &idx)) {
>
> - if (adev->mman.aper_base_kaddr)
> - iounmap(adev->mman.aper_base_kaddr);
> - adev->mman.aper_base_kaddr = NULL;
> + if (adev->mman.vram_aper_base_kaddr)
> + iounmap(adev->mman.vram_aper_base_kaddr);
> + adev->mman.vram_aper_base_kaddr = NULL;
>
> drm_dev_exit(idx);
> }
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
> index e2cd5894afc9..929bc8abac28 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
> @@ -50,7 +50,7 @@ struct amdgpu_gtt_mgr {
> struct amdgpu_mman {
> struct ttm_device bdev;
> bool initialized;
> - void __iomem *aper_base_kaddr;
> + void __iomem *vram_aper_base_kaddr;
>
> /* buffer handling */
> const struct amdgpu_buffer_funcs *buffer_funcs;
> diff --git a/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c b/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c
> index bd3e3e23a939..f39d4f593a2f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c
> @@ -611,10 +611,10 @@ static int psp_v11_0_memory_training(struct psp_context *psp, uint32_t ops)
> */
> sz = GDDR6_MEM_TRAINING_ENCROACHED_SIZE;
>
> - if (adev->gmc.visible_vram_size < sz || !adev->mman.aper_base_kaddr) {
> - DRM_ERROR("visible_vram_size %llx or aper_base_kaddr %p is not initialized.\n",
> + if (adev->gmc.visible_vram_size < sz || !adev->mman.vram_aper_base_kaddr) {
> + DRM_ERROR("visible_vram_size %llx or vram_aper_base_kaddr %p is not initialized.\n",
> adev->gmc.visible_vram_size,
> - adev->mman.aper_base_kaddr);
> + adev->mman.vram_aper_base_kaddr);
> return -EINVAL;
> }
>
> @@ -625,7 +625,7 @@ static int psp_v11_0_memory_training(struct psp_context *psp, uint32_t ops)
> }
>
> if (drm_dev_enter(adev_to_drm(adev), &idx)) {
> - memcpy_fromio(buf, adev->mman.aper_base_kaddr, sz);
> + memcpy_fromio(buf, adev->mman.vram_aper_base_kaddr, sz);
> ret = psp_v11_0_memory_training_send_msg(psp, PSP_BL__DRAM_LONG_TRAIN);
> if (ret) {
> DRM_ERROR("Send long training msg failed.\n");
> @@ -634,7 +634,7 @@ static int psp_v11_0_memory_training(struct psp_context *psp, uint32_t ops)
> return ret;
> }
>
> - memcpy_toio(adev->mman.aper_base_kaddr, buf, sz);
> + memcpy_toio(adev->mman.vram_aper_base_kaddr, buf, sz);
> adev->hdp.funcs->flush_hdp(adev, NULL);
> vfree(buf);
> drm_dev_exit(idx);
> diff --git a/drivers/gpu/drm/amd/amdgpu/psp_v13_0.c b/drivers/gpu/drm/amd/amdgpu/psp_v13_0.c
> index e6a26a7e5e5e..9605c0971c11 100644
> --- a/drivers/gpu/drm/amd/amdgpu/psp_v13_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/psp_v13_0.c
> @@ -510,10 +510,10 @@ static int psp_v13_0_memory_training(struct psp_context *psp, uint32_t ops)
> */
> sz = GDDR6_MEM_TRAINING_ENCROACHED_SIZE;
>
> - if (adev->gmc.visible_vram_size < sz || !adev->mman.aper_base_kaddr) {
> - dev_err(adev->dev, "visible_vram_size %llx or aper_base_kaddr %p is not initialized.\n",
> + if (adev->gmc.visible_vram_size < sz || !adev->mman.vram_aper_base_kaddr) {
> + dev_err(adev->dev, "visible_vram_size %llx or vram_aper_base_kaddr %p is not initialized.\n",
> adev->gmc.visible_vram_size,
> - adev->mman.aper_base_kaddr);
> + adev->mman.vram_aper_base_kaddr);
> return -EINVAL;
> }
>
> @@ -524,7 +524,7 @@ static int psp_v13_0_memory_training(struct psp_context *psp, uint32_t ops)
> }
>
> if (drm_dev_enter(adev_to_drm(adev), &idx)) {
> - memcpy_fromio(buf, adev->mman.aper_base_kaddr, sz);
> + memcpy_fromio(buf, adev->mman.vram_aper_base_kaddr, sz);
> ret = psp_v13_0_memory_training_send_msg(psp, PSP_BL__DRAM_LONG_TRAIN);
> if (ret) {
> DRM_ERROR("Send long training msg failed.\n");
> @@ -533,7 +533,7 @@ static int psp_v13_0_memory_training(struct psp_context *psp, uint32_t ops)
> return ret;
> }
>
> - memcpy_toio(adev->mman.aper_base_kaddr, buf, sz);
> + memcpy_toio(adev->mman.vram_aper_base_kaddr, buf, sz);
> adev->hdp.funcs->flush_hdp(adev, NULL);
> vfree(buf);
> drm_dev_exit(idx);
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v2 2/8] drm/amdgpu: replace aper_base_kaddr with vram_aper_base_kaddr
2023-02-14 18:24 ` Christian König
@ 2023-02-14 19:03 ` Shashank Sharma
0 siblings, 0 replies; 28+ messages in thread
From: Shashank Sharma @ 2023-02-14 19:03 UTC (permalink / raw)
To: Christian König, amd-gfx
Cc: alexander.deucher, Arvind.Yadav, Christian Koenig
On 14/02/2023 19:24, Christian König wrote:
> Am 14.02.23 um 17:15 schrieb Shashank Sharma:
>> From: Alex Deucher <alexander.deucher@amd.com>
>>
>> To differentiate it from the doorbell BAR.
>
> Since we removed the manual ioremap() for the doorbell BAR today we
> don't really need that patch any more, don't we?
>
> On the other hand renaming the field still makes a lot of sense for
> better documenting what it's good for.
>
> Christian.
>
Exactly, I was also in two minds about this patch, but then realized
that if nothing, its making stuff more readable.
- Shashank
>>
>> V2: Added Christian's A-B
>>
>> Acked-by: Christian Koenig <christian.koenig@amid.com>
>>
>> Cc: Alex Deucher <alexander.deucher@amd.com>
>> Cc: Christian Koenig <christian.koenig@amd.com>
>> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
>> ---
>> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 10 +++++-----
>> drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 14 +++++++-------
>> drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h | 2 +-
>> drivers/gpu/drm/amd/amdgpu/psp_v11_0.c | 10 +++++-----
>> drivers/gpu/drm/amd/amdgpu/psp_v13_0.c | 10 +++++-----
>> 5 files changed, 23 insertions(+), 23 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> index 2f28a8c02f64..0b6a394e109b 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> @@ -354,12 +354,12 @@ size_t amdgpu_device_aper_access(struct
>> amdgpu_device *adev, loff_t pos,
>> size_t count = 0;
>> uint64_t last;
>> - if (!adev->mman.aper_base_kaddr)
>> + if (!adev->mman.vram_aper_base_kaddr)
>> return 0;
>> last = min(pos + size, adev->gmc.visible_vram_size);
>> if (last > pos) {
>> - addr = adev->mman.aper_base_kaddr + pos;
>> + addr = adev->mman.vram_aper_base_kaddr + pos;
>> count = last - pos;
>> if (write) {
>> @@ -3954,9 +3954,9 @@ static void amdgpu_device_unmap_mmio(struct
>> amdgpu_device *adev)
>> iounmap(adev->rmmio);
>> adev->rmmio = NULL;
>> - if (adev->mman.aper_base_kaddr)
>> - iounmap(adev->mman.aper_base_kaddr);
>> - adev->mman.aper_base_kaddr = NULL;
>> + if (adev->mman.vram_aper_base_kaddr)
>> + iounmap(adev->mman.vram_aper_base_kaddr);
>> + adev->mman.vram_aper_base_kaddr = NULL;
>> /* Memory manager related */
>> if (!adev->gmc.xgmi.connected_to_cpu) {
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>> index 55e0284b2bdd..73b831b47892 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>> @@ -578,9 +578,9 @@ static int amdgpu_ttm_io_mem_reserve(struct
>> ttm_device *bdev,
>> if ((mem->bus.offset + bus_size) >
>> adev->gmc.visible_vram_size)
>> return -EINVAL;
>> - if (adev->mman.aper_base_kaddr &&
>> + if (adev->mman.vram_aper_base_kaddr &&
>> mem->placement & TTM_PL_FLAG_CONTIGUOUS)
>> - mem->bus.addr = (u8 *)adev->mman.aper_base_kaddr +
>> + mem->bus.addr = (u8 *)adev->mman.vram_aper_base_kaddr +
>> mem->bus.offset;
>> mem->bus.offset += adev->gmc.aper_base;
>> @@ -1752,12 +1752,12 @@ int amdgpu_ttm_init(struct amdgpu_device *adev)
>> #ifdef CONFIG_64BIT
>> #ifdef CONFIG_X86
>> if (adev->gmc.xgmi.connected_to_cpu)
>> - adev->mman.aper_base_kaddr = ioremap_cache(adev->gmc.aper_base,
>> + adev->mman.vram_aper_base_kaddr =
>> ioremap_cache(adev->gmc.aper_base,
>> adev->gmc.visible_vram_size);
>> else
>> #endif
>> - adev->mman.aper_base_kaddr = ioremap_wc(adev->gmc.aper_base,
>> + adev->mman.vram_aper_base_kaddr =
>> ioremap_wc(adev->gmc.aper_base,
>> adev->gmc.visible_vram_size);
>> #endif
>> @@ -1904,9 +1904,9 @@ void amdgpu_ttm_fini(struct amdgpu_device *adev)
>> if (drm_dev_enter(adev_to_drm(adev), &idx)) {
>> - if (adev->mman.aper_base_kaddr)
>> - iounmap(adev->mman.aper_base_kaddr);
>> - adev->mman.aper_base_kaddr = NULL;
>> + if (adev->mman.vram_aper_base_kaddr)
>> + iounmap(adev->mman.vram_aper_base_kaddr);
>> + adev->mman.vram_aper_base_kaddr = NULL;
>> drm_dev_exit(idx);
>> }
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
>> index e2cd5894afc9..929bc8abac28 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
>> @@ -50,7 +50,7 @@ struct amdgpu_gtt_mgr {
>> struct amdgpu_mman {
>> struct ttm_device bdev;
>> bool initialized;
>> - void __iomem *aper_base_kaddr;
>> + void __iomem *vram_aper_base_kaddr;
>> /* buffer handling */
>> const struct amdgpu_buffer_funcs *buffer_funcs;
>> diff --git a/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c
>> b/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c
>> index bd3e3e23a939..f39d4f593a2f 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c
>> @@ -611,10 +611,10 @@ static int psp_v11_0_memory_training(struct
>> psp_context *psp, uint32_t ops)
>> */
>> sz = GDDR6_MEM_TRAINING_ENCROACHED_SIZE;
>> - if (adev->gmc.visible_vram_size < sz ||
>> !adev->mman.aper_base_kaddr) {
>> - DRM_ERROR("visible_vram_size %llx or aper_base_kaddr %p
>> is not initialized.\n",
>> + if (adev->gmc.visible_vram_size < sz ||
>> !adev->mman.vram_aper_base_kaddr) {
>> + DRM_ERROR("visible_vram_size %llx or
>> vram_aper_base_kaddr %p is not initialized.\n",
>> adev->gmc.visible_vram_size,
>> - adev->mman.aper_base_kaddr);
>> + adev->mman.vram_aper_base_kaddr);
>> return -EINVAL;
>> }
>> @@ -625,7 +625,7 @@ static int psp_v11_0_memory_training(struct
>> psp_context *psp, uint32_t ops)
>> }
>> if (drm_dev_enter(adev_to_drm(adev), &idx)) {
>> - memcpy_fromio(buf, adev->mman.aper_base_kaddr, sz);
>> + memcpy_fromio(buf, adev->mman.vram_aper_base_kaddr, sz);
>> ret = psp_v11_0_memory_training_send_msg(psp,
>> PSP_BL__DRAM_LONG_TRAIN);
>> if (ret) {
>> DRM_ERROR("Send long training msg failed.\n");
>> @@ -634,7 +634,7 @@ static int psp_v11_0_memory_training(struct
>> psp_context *psp, uint32_t ops)
>> return ret;
>> }
>> - memcpy_toio(adev->mman.aper_base_kaddr, buf, sz);
>> + memcpy_toio(adev->mman.vram_aper_base_kaddr, buf, sz);
>> adev->hdp.funcs->flush_hdp(adev, NULL);
>> vfree(buf);
>> drm_dev_exit(idx);
>> diff --git a/drivers/gpu/drm/amd/amdgpu/psp_v13_0.c
>> b/drivers/gpu/drm/amd/amdgpu/psp_v13_0.c
>> index e6a26a7e5e5e..9605c0971c11 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/psp_v13_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/psp_v13_0.c
>> @@ -510,10 +510,10 @@ static int psp_v13_0_memory_training(struct
>> psp_context *psp, uint32_t ops)
>> */
>> sz = GDDR6_MEM_TRAINING_ENCROACHED_SIZE;
>> - if (adev->gmc.visible_vram_size < sz ||
>> !adev->mman.aper_base_kaddr) {
>> - dev_err(adev->dev, "visible_vram_size %llx or
>> aper_base_kaddr %p is not initialized.\n",
>> + if (adev->gmc.visible_vram_size < sz ||
>> !adev->mman.vram_aper_base_kaddr) {
>> + dev_err(adev->dev, "visible_vram_size %llx or
>> vram_aper_base_kaddr %p is not initialized.\n",
>> adev->gmc.visible_vram_size,
>> - adev->mman.aper_base_kaddr);
>> + adev->mman.vram_aper_base_kaddr);
>> return -EINVAL;
>> }
>> @@ -524,7 +524,7 @@ static int psp_v13_0_memory_training(struct
>> psp_context *psp, uint32_t ops)
>> }
>> if (drm_dev_enter(adev_to_drm(adev), &idx)) {
>> - memcpy_fromio(buf, adev->mman.aper_base_kaddr, sz);
>> + memcpy_fromio(buf, adev->mman.vram_aper_base_kaddr, sz);
>> ret = psp_v13_0_memory_training_send_msg(psp,
>> PSP_BL__DRAM_LONG_TRAIN);
>> if (ret) {
>> DRM_ERROR("Send long training msg failed.\n");
>> @@ -533,7 +533,7 @@ static int psp_v13_0_memory_training(struct
>> psp_context *psp, uint32_t ops)
>> return ret;
>> }
>> - memcpy_toio(adev->mman.aper_base_kaddr, buf, sz);
>> + memcpy_toio(adev->mman.vram_aper_base_kaddr, buf, sz);
>> adev->hdp.funcs->flush_hdp(adev, NULL);
>> vfree(buf);
>> drm_dev_exit(idx);
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v2 3/8] drm/amdgpu: rename gmc.aper_base/size
2023-02-14 16:15 [PATCH v2 0/8] Re-design doorbell framework for usermode queues Shashank Sharma
2023-02-14 16:15 ` [PATCH v2 1/8] drm/amdgpu: add UAPI for allocating doorbell memory Shashank Sharma
2023-02-14 16:15 ` [PATCH v2 2/8] drm/amdgpu: replace aper_base_kaddr with vram_aper_base_kaddr Shashank Sharma
@ 2023-02-14 16:15 ` Shashank Sharma
2023-02-14 18:25 ` Christian König
2023-02-14 16:15 ` [PATCH v2 4/8] drm/amdgpu: rename doorbell variables Shashank Sharma
` (4 subsequent siblings)
7 siblings, 1 reply; 28+ messages in thread
From: Shashank Sharma @ 2023-02-14 16:15 UTC (permalink / raw)
To: amd-gfx; +Cc: alexander.deucher, christian.koenig, Arvind.Yadav,
shashank.sharma
From: Alex Deucher <alexander.deucher@amd.com>
This patch renames aper_base and aper_size parameters (in adev->gmc),
to vram_aper_base and vram_aper_size, to differentiate it from the
doorbell BAR.
V2: rebase
Cc: Alex Deucher <alexander.deucher@amd.com>
Cc: Christian Koenig <christian.koenig@amd.com>
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
Signed-off-by: Shashank Sharma <shashank.sharma@amd.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c | 2 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 6 +++---
drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c | 2 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h | 4 ++--
drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 12 ++++++------
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 8 ++++----
drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c | 2 +-
drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c | 10 +++++-----
drivers/gpu/drm/amd/amdgpu/gmc_v11_0.c | 10 +++++-----
drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c | 6 +++---
drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c | 12 ++++++------
drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c | 10 +++++-----
drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c | 10 +++++-----
drivers/gpu/drm/amd/amdkfd/kfd_migrate.c | 4 ++--
14 files changed, 49 insertions(+), 49 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
index f99d4873bf22..58689b2a2d1c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
@@ -438,7 +438,7 @@ void amdgpu_amdkfd_get_local_mem_info(struct amdgpu_device *adev,
mem_info->vram_width = adev->gmc.vram_width;
pr_debug("Address base: %pap public 0x%llx private 0x%llx\n",
- &adev->gmc.aper_base,
+ &adev->gmc.vram_aper_base,
mem_info->local_mem_size_public,
mem_info->local_mem_size_private);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 0b6a394e109b..45588b7919fe 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -3961,7 +3961,7 @@ static void amdgpu_device_unmap_mmio(struct amdgpu_device *adev)
/* Memory manager related */
if (!adev->gmc.xgmi.connected_to_cpu) {
arch_phys_wc_del(adev->gmc.vram_mtrr);
- arch_io_free_memtype_wc(adev->gmc.aper_base, adev->gmc.aper_size);
+ arch_io_free_memtype_wc(adev->gmc.vram_aper_base, adev->gmc.vram_aper_size);
}
}
@@ -5562,14 +5562,14 @@ bool amdgpu_device_is_peer_accessible(struct amdgpu_device *adev,
uint64_t address_mask = peer_adev->dev->dma_mask ?
~*peer_adev->dev->dma_mask : ~((1ULL << 32) - 1);
resource_size_t aper_limit =
- adev->gmc.aper_base + adev->gmc.aper_size - 1;
+ adev->gmc.vram_aper_base + adev->gmc.vram_aper_size - 1;
bool p2p_access =
!adev->gmc.xgmi.connected_to_cpu &&
!(pci_p2pdma_distance(adev->pdev, peer_adev->dev, false) < 0);
return pcie_p2p && p2p_access && (adev->gmc.visible_vram_size &&
adev->gmc.real_vram_size == adev->gmc.visible_vram_size &&
- !(adev->gmc.aper_base & address_mask ||
+ !(adev->gmc.vram_aper_base & address_mask ||
aper_limit & address_mask));
#else
return false;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
index 02a4c93673ce..c7e64e234de6 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
@@ -775,7 +775,7 @@ uint64_t amdgpu_gmc_vram_pa(struct amdgpu_device *adev, struct amdgpu_bo *bo)
*/
uint64_t amdgpu_gmc_vram_cpu_pa(struct amdgpu_device *adev, struct amdgpu_bo *bo)
{
- return amdgpu_bo_gpu_offset(bo) - adev->gmc.vram_start + adev->gmc.aper_base;
+ return amdgpu_bo_gpu_offset(bo) - adev->gmc.vram_start + adev->gmc.vram_aper_base;
}
int amdgpu_gmc_vram_checking(struct amdgpu_device *adev)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
index 0305b660cd17..bb7076ecbf01 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
@@ -167,8 +167,8 @@ struct amdgpu_gmc {
* gart/vram_start/end field as the later is from
* GPU's view and aper_base is from CPU's view.
*/
- resource_size_t aper_size;
- resource_size_t aper_base;
+ resource_size_t vram_aper_size;
+ resource_size_t vram_aper_base;
/* for some chips with <= 32MB we need to lie
* about vram size near mc fb location */
u64 mc_vram_size;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index 25a68d8888e0..b48c9fd60c43 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -1046,8 +1046,8 @@ int amdgpu_bo_init(struct amdgpu_device *adev)
/* On A+A platform, VRAM can be mapped as WB */
if (!adev->gmc.xgmi.connected_to_cpu) {
/* reserve PAT memory space to WC for VRAM */
- int r = arch_io_reserve_memtype_wc(adev->gmc.aper_base,
- adev->gmc.aper_size);
+ int r = arch_io_reserve_memtype_wc(adev->gmc.vram_aper_base,
+ adev->gmc.vram_aper_size);
if (r) {
DRM_ERROR("Unable to set WC memtype for the aperture base\n");
@@ -1055,13 +1055,13 @@ int amdgpu_bo_init(struct amdgpu_device *adev)
}
/* Add an MTRR for the VRAM */
- adev->gmc.vram_mtrr = arch_phys_wc_add(adev->gmc.aper_base,
- adev->gmc.aper_size);
+ adev->gmc.vram_mtrr = arch_phys_wc_add(adev->gmc.vram_aper_base,
+ adev->gmc.vram_aper_size);
}
DRM_INFO("Detected VRAM RAM=%lluM, BAR=%lluM\n",
adev->gmc.mc_vram_size >> 20,
- (unsigned long long)adev->gmc.aper_size >> 20);
+ (unsigned long long)adev->gmc.vram_aper_size >> 20);
DRM_INFO("RAM width %dbits %s\n",
adev->gmc.vram_width, amdgpu_vram_names[adev->gmc.vram_type]);
return amdgpu_ttm_init(adev);
@@ -1083,7 +1083,7 @@ void amdgpu_bo_fini(struct amdgpu_device *adev)
if (!adev->gmc.xgmi.connected_to_cpu) {
arch_phys_wc_del(adev->gmc.vram_mtrr);
- arch_io_free_memtype_wc(adev->gmc.aper_base, adev->gmc.aper_size);
+ arch_io_free_memtype_wc(adev->gmc.vram_aper_base, adev->gmc.vram_aper_size);
}
drm_dev_exit(idx);
}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 73b831b47892..0e8f580769ab 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -583,7 +583,7 @@ static int amdgpu_ttm_io_mem_reserve(struct ttm_device *bdev,
mem->bus.addr = (u8 *)adev->mman.vram_aper_base_kaddr +
mem->bus.offset;
- mem->bus.offset += adev->gmc.aper_base;
+ mem->bus.offset += adev->gmc.vram_aper_base;
mem->bus.is_iomem = true;
break;
default:
@@ -600,7 +600,7 @@ static unsigned long amdgpu_ttm_io_mem_pfn(struct ttm_buffer_object *bo,
amdgpu_res_first(bo->resource, (u64)page_offset << PAGE_SHIFT, 0,
&cursor);
- return (adev->gmc.aper_base + cursor.start) >> PAGE_SHIFT;
+ return (adev->gmc.vram_aper_base + cursor.start) >> PAGE_SHIFT;
}
/**
@@ -1752,12 +1752,12 @@ int amdgpu_ttm_init(struct amdgpu_device *adev)
#ifdef CONFIG_64BIT
#ifdef CONFIG_X86
if (adev->gmc.xgmi.connected_to_cpu)
- adev->mman.vram_aper_base_kaddr = ioremap_cache(adev->gmc.aper_base,
+ adev->mman.vram_aper_base_kaddr = ioremap_cache(adev->gmc.vram_aper_base,
adev->gmc.visible_vram_size);
else
#endif
- adev->mman.vram_aper_base_kaddr = ioremap_wc(adev->gmc.aper_base,
+ adev->mman.vram_aper_base_kaddr = ioremap_wc(adev->gmc.vram_aper_base,
adev->gmc.visible_vram_size);
#endif
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
index 9fa1d814508a..d66caad04c24 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
@@ -649,7 +649,7 @@ int amdgpu_vram_mgr_alloc_sgt(struct amdgpu_device *adev,
*/
amdgpu_res_first(res, offset, length, &cursor);
for_each_sgtable_sg((*sgt), sg, i) {
- phys_addr_t phys = cursor.start + adev->gmc.aper_base;
+ phys_addr_t phys = cursor.start + adev->gmc.vram_aper_base;
size_t size = cursor.size;
dma_addr_t addr;
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
index 21e46817d82d..b2e4f4f06bdb 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
@@ -825,18 +825,18 @@ static int gmc_v10_0_mc_init(struct amdgpu_device *adev)
if (r)
return r;
}
- adev->gmc.aper_base = pci_resource_start(adev->pdev, 0);
- adev->gmc.aper_size = pci_resource_len(adev->pdev, 0);
+ adev->gmc.vram_aper_base = pci_resource_start(adev->pdev, 0);
+ adev->gmc.vram_aper_size = pci_resource_len(adev->pdev, 0);
#ifdef CONFIG_X86_64
if ((adev->flags & AMD_IS_APU) && !amdgpu_passthrough(adev)) {
- adev->gmc.aper_base = adev->gfxhub.funcs->get_mc_fb_offset(adev);
- adev->gmc.aper_size = adev->gmc.real_vram_size;
+ adev->gmc.vram_aper_base = adev->gfxhub.funcs->get_mc_fb_offset(adev);
+ adev->gmc.vram_aper_size = adev->gmc.real_vram_size;
}
#endif
/* In case the PCI BAR is larger than the actual amount of vram */
- adev->gmc.visible_vram_size = adev->gmc.aper_size;
+ adev->gmc.visible_vram_size = adev->gmc.vram_aper_size;
if (adev->gmc.visible_vram_size > adev->gmc.real_vram_size)
adev->gmc.visible_vram_size = adev->gmc.real_vram_size;
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v11_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v11_0.c
index 4326078689cd..f993ce264c3f 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v11_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v11_0.c
@@ -692,17 +692,17 @@ static int gmc_v11_0_mc_init(struct amdgpu_device *adev)
if (r)
return r;
}
- adev->gmc.aper_base = pci_resource_start(adev->pdev, 0);
- adev->gmc.aper_size = pci_resource_len(adev->pdev, 0);
+ adev->gmc.vram_aper_base = pci_resource_start(adev->pdev, 0);
+ adev->gmc.vram_aper_size = pci_resource_len(adev->pdev, 0);
#ifdef CONFIG_X86_64
if ((adev->flags & AMD_IS_APU) && !amdgpu_passthrough(adev)) {
- adev->gmc.aper_base = adev->mmhub.funcs->get_mc_fb_offset(adev);
- adev->gmc.aper_size = adev->gmc.real_vram_size;
+ adev->gmc.vram_aper_base = adev->mmhub.funcs->get_mc_fb_offset(adev);
+ adev->gmc.vram_aper_size = adev->gmc.real_vram_size;
}
#endif
/* In case the PCI BAR is larger than the actual amount of vram */
- adev->gmc.visible_vram_size = adev->gmc.aper_size;
+ adev->gmc.visible_vram_size = adev->gmc.vram_aper_size;
if (adev->gmc.visible_vram_size > adev->gmc.real_vram_size)
adev->gmc.visible_vram_size = adev->gmc.real_vram_size;
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
index ec291d28edff..cd159309e9e5 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
@@ -324,9 +324,9 @@ static int gmc_v6_0_mc_init(struct amdgpu_device *adev)
if (r)
return r;
}
- adev->gmc.aper_base = pci_resource_start(adev->pdev, 0);
- adev->gmc.aper_size = pci_resource_len(adev->pdev, 0);
- adev->gmc.visible_vram_size = adev->gmc.aper_size;
+ adev->gmc.vram_aper_base = pci_resource_start(adev->pdev, 0);
+ adev->gmc.vram_aper_size = pci_resource_len(adev->pdev, 0);
+ adev->gmc.visible_vram_size = adev->gmc.vram_aper_size;
/* set the gart size */
if (amdgpu_gart_size == -1) {
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
index 979da6f510e8..8ee9731a0c8c 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
@@ -377,20 +377,20 @@ static int gmc_v7_0_mc_init(struct amdgpu_device *adev)
if (r)
return r;
}
- adev->gmc.aper_base = pci_resource_start(adev->pdev, 0);
- adev->gmc.aper_size = pci_resource_len(adev->pdev, 0);
+ adev->gmc.vram_aper_base = pci_resource_start(adev->pdev, 0);
+ adev->gmc.vram_aper_size = pci_resource_len(adev->pdev, 0);
#ifdef CONFIG_X86_64
if ((adev->flags & AMD_IS_APU) &&
- adev->gmc.real_vram_size > adev->gmc.aper_size &&
+ adev->gmc.real_vram_size > adev->gmc.vram_aper_size &&
!amdgpu_passthrough(adev)) {
- adev->gmc.aper_base = ((u64)RREG32(mmMC_VM_FB_OFFSET)) << 22;
- adev->gmc.aper_size = adev->gmc.real_vram_size;
+ adev->gmc.vram_aper_base = ((u64)RREG32(mmMC_VM_FB_OFFSET)) << 22;
+ adev->gmc.vram_aper_size = adev->gmc.real_vram_size;
}
#endif
/* In case the PCI BAR is larger than the actual amount of vram */
- adev->gmc.visible_vram_size = adev->gmc.aper_size;
+ adev->gmc.visible_vram_size = adev->gmc.vram_aper_size;
if (adev->gmc.visible_vram_size > adev->gmc.real_vram_size)
adev->gmc.visible_vram_size = adev->gmc.real_vram_size;
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
index 382dde1ce74c..259d797358f1 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
@@ -577,18 +577,18 @@ static int gmc_v8_0_mc_init(struct amdgpu_device *adev)
if (r)
return r;
}
- adev->gmc.aper_base = pci_resource_start(adev->pdev, 0);
- adev->gmc.aper_size = pci_resource_len(adev->pdev, 0);
+ adev->gmc.vram_aper_base = pci_resource_start(adev->pdev, 0);
+ adev->gmc.vram_aper_size = pci_resource_len(adev->pdev, 0);
#ifdef CONFIG_X86_64
if ((adev->flags & AMD_IS_APU) && !amdgpu_passthrough(adev)) {
- adev->gmc.aper_base = ((u64)RREG32(mmMC_VM_FB_OFFSET)) << 22;
- adev->gmc.aper_size = adev->gmc.real_vram_size;
+ adev->gmc.vram_aper_base = ((u64)RREG32(mmMC_VM_FB_OFFSET)) << 22;
+ adev->gmc.vram_aper_size = adev->gmc.real_vram_size;
}
#endif
/* In case the PCI BAR is larger than the actual amount of vram */
- adev->gmc.visible_vram_size = adev->gmc.aper_size;
+ adev->gmc.visible_vram_size = adev->gmc.vram_aper_size;
if (adev->gmc.visible_vram_size > adev->gmc.real_vram_size)
adev->gmc.visible_vram_size = adev->gmc.real_vram_size;
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
index 08d6cf79fb15..a7074995d97e 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
@@ -1509,8 +1509,8 @@ static int gmc_v9_0_mc_init(struct amdgpu_device *adev)
if (r)
return r;
}
- adev->gmc.aper_base = pci_resource_start(adev->pdev, 0);
- adev->gmc.aper_size = pci_resource_len(adev->pdev, 0);
+ adev->gmc.vram_aper_base = pci_resource_start(adev->pdev, 0);
+ adev->gmc.vram_aper_size = pci_resource_len(adev->pdev, 0);
#ifdef CONFIG_X86_64
/*
@@ -1528,16 +1528,16 @@ static int gmc_v9_0_mc_init(struct amdgpu_device *adev)
if (((adev->flags & AMD_IS_APU) && !amdgpu_passthrough(adev)) ||
(adev->gmc.xgmi.supported &&
adev->gmc.xgmi.connected_to_cpu)) {
- adev->gmc.aper_base =
+ adev->gmc.vram_aper_base =
adev->gfxhub.funcs->get_mc_fb_offset(adev) +
adev->gmc.xgmi.physical_node_id *
adev->gmc.xgmi.node_segment_size;
- adev->gmc.aper_size = adev->gmc.real_vram_size;
+ adev->gmc.vram_aper_size = adev->gmc.real_vram_size;
}
#endif
/* In case the PCI BAR is larger than the actual amount of vram */
- adev->gmc.visible_vram_size = adev->gmc.aper_size;
+ adev->gmc.visible_vram_size = adev->gmc.vram_aper_size;
if (adev->gmc.visible_vram_size > adev->gmc.real_vram_size)
adev->gmc.visible_vram_size = adev->gmc.real_vram_size;
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
index 10048ce16aea..c86c6705b470 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
@@ -1002,8 +1002,8 @@ int svm_migrate_init(struct amdgpu_device *adev)
*/
size = ALIGN(adev->gmc.real_vram_size, 2ULL << 20);
if (adev->gmc.xgmi.connected_to_cpu) {
- pgmap->range.start = adev->gmc.aper_base;
- pgmap->range.end = adev->gmc.aper_base + adev->gmc.aper_size - 1;
+ pgmap->range.start = adev->gmc.vram_aper_base;
+ pgmap->range.end = adev->gmc.vram_aper_base + adev->gmc.vram_aper_size - 1;
pgmap->type = MEMORY_DEVICE_COHERENT;
} else {
res = devm_request_free_mem_region(adev->dev, &iomem_resource, size);
--
2.34.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH v2 3/8] drm/amdgpu: rename gmc.aper_base/size
2023-02-14 16:15 ` [PATCH v2 3/8] drm/amdgpu: rename gmc.aper_base/size Shashank Sharma
@ 2023-02-14 18:25 ` Christian König
0 siblings, 0 replies; 28+ messages in thread
From: Christian König @ 2023-02-14 18:25 UTC (permalink / raw)
To: Shashank Sharma, amd-gfx; +Cc: alexander.deucher, Arvind.Yadav
Am 14.02.23 um 17:15 schrieb Shashank Sharma:
> From: Alex Deucher <alexander.deucher@amd.com>
>
> This patch renames aper_base and aper_size parameters (in adev->gmc),
> to vram_aper_base and vram_aper_size, to differentiate it from the
> doorbell BAR.
>
> V2: rebase
>
> Cc: Alex Deucher <alexander.deucher@amd.com>
> Cc: Christian Koenig <christian.koenig@amd.com>
>
> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> Signed-off-by: Shashank Sharma <shashank.sharma@amd.com>
Acked-by: Christian König <christian.koenig@amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c | 2 +-
> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 6 +++---
> drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c | 2 +-
> drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h | 4 ++--
> drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 12 ++++++------
> drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 8 ++++----
> drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c | 2 +-
> drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c | 10 +++++-----
> drivers/gpu/drm/amd/amdgpu/gmc_v11_0.c | 10 +++++-----
> drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c | 6 +++---
> drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c | 12 ++++++------
> drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c | 10 +++++-----
> drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c | 10 +++++-----
> drivers/gpu/drm/amd/amdkfd/kfd_migrate.c | 4 ++--
> 14 files changed, 49 insertions(+), 49 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
> index f99d4873bf22..58689b2a2d1c 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
> @@ -438,7 +438,7 @@ void amdgpu_amdkfd_get_local_mem_info(struct amdgpu_device *adev,
> mem_info->vram_width = adev->gmc.vram_width;
>
> pr_debug("Address base: %pap public 0x%llx private 0x%llx\n",
> - &adev->gmc.aper_base,
> + &adev->gmc.vram_aper_base,
> mem_info->local_mem_size_public,
> mem_info->local_mem_size_private);
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index 0b6a394e109b..45588b7919fe 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -3961,7 +3961,7 @@ static void amdgpu_device_unmap_mmio(struct amdgpu_device *adev)
> /* Memory manager related */
> if (!adev->gmc.xgmi.connected_to_cpu) {
> arch_phys_wc_del(adev->gmc.vram_mtrr);
> - arch_io_free_memtype_wc(adev->gmc.aper_base, adev->gmc.aper_size);
> + arch_io_free_memtype_wc(adev->gmc.vram_aper_base, adev->gmc.vram_aper_size);
> }
> }
>
> @@ -5562,14 +5562,14 @@ bool amdgpu_device_is_peer_accessible(struct amdgpu_device *adev,
> uint64_t address_mask = peer_adev->dev->dma_mask ?
> ~*peer_adev->dev->dma_mask : ~((1ULL << 32) - 1);
> resource_size_t aper_limit =
> - adev->gmc.aper_base + adev->gmc.aper_size - 1;
> + adev->gmc.vram_aper_base + adev->gmc.vram_aper_size - 1;
> bool p2p_access =
> !adev->gmc.xgmi.connected_to_cpu &&
> !(pci_p2pdma_distance(adev->pdev, peer_adev->dev, false) < 0);
>
> return pcie_p2p && p2p_access && (adev->gmc.visible_vram_size &&
> adev->gmc.real_vram_size == adev->gmc.visible_vram_size &&
> - !(adev->gmc.aper_base & address_mask ||
> + !(adev->gmc.vram_aper_base & address_mask ||
> aper_limit & address_mask));
> #else
> return false;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
> index 02a4c93673ce..c7e64e234de6 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
> @@ -775,7 +775,7 @@ uint64_t amdgpu_gmc_vram_pa(struct amdgpu_device *adev, struct amdgpu_bo *bo)
> */
> uint64_t amdgpu_gmc_vram_cpu_pa(struct amdgpu_device *adev, struct amdgpu_bo *bo)
> {
> - return amdgpu_bo_gpu_offset(bo) - adev->gmc.vram_start + adev->gmc.aper_base;
> + return amdgpu_bo_gpu_offset(bo) - adev->gmc.vram_start + adev->gmc.vram_aper_base;
> }
>
> int amdgpu_gmc_vram_checking(struct amdgpu_device *adev)
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
> index 0305b660cd17..bb7076ecbf01 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
> @@ -167,8 +167,8 @@ struct amdgpu_gmc {
> * gart/vram_start/end field as the later is from
> * GPU's view and aper_base is from CPU's view.
> */
> - resource_size_t aper_size;
> - resource_size_t aper_base;
> + resource_size_t vram_aper_size;
> + resource_size_t vram_aper_base;
> /* for some chips with <= 32MB we need to lie
> * about vram size near mc fb location */
> u64 mc_vram_size;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> index 25a68d8888e0..b48c9fd60c43 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> @@ -1046,8 +1046,8 @@ int amdgpu_bo_init(struct amdgpu_device *adev)
> /* On A+A platform, VRAM can be mapped as WB */
> if (!adev->gmc.xgmi.connected_to_cpu) {
> /* reserve PAT memory space to WC for VRAM */
> - int r = arch_io_reserve_memtype_wc(adev->gmc.aper_base,
> - adev->gmc.aper_size);
> + int r = arch_io_reserve_memtype_wc(adev->gmc.vram_aper_base,
> + adev->gmc.vram_aper_size);
>
> if (r) {
> DRM_ERROR("Unable to set WC memtype for the aperture base\n");
> @@ -1055,13 +1055,13 @@ int amdgpu_bo_init(struct amdgpu_device *adev)
> }
>
> /* Add an MTRR for the VRAM */
> - adev->gmc.vram_mtrr = arch_phys_wc_add(adev->gmc.aper_base,
> - adev->gmc.aper_size);
> + adev->gmc.vram_mtrr = arch_phys_wc_add(adev->gmc.vram_aper_base,
> + adev->gmc.vram_aper_size);
> }
>
> DRM_INFO("Detected VRAM RAM=%lluM, BAR=%lluM\n",
> adev->gmc.mc_vram_size >> 20,
> - (unsigned long long)adev->gmc.aper_size >> 20);
> + (unsigned long long)adev->gmc.vram_aper_size >> 20);
> DRM_INFO("RAM width %dbits %s\n",
> adev->gmc.vram_width, amdgpu_vram_names[adev->gmc.vram_type]);
> return amdgpu_ttm_init(adev);
> @@ -1083,7 +1083,7 @@ void amdgpu_bo_fini(struct amdgpu_device *adev)
>
> if (!adev->gmc.xgmi.connected_to_cpu) {
> arch_phys_wc_del(adev->gmc.vram_mtrr);
> - arch_io_free_memtype_wc(adev->gmc.aper_base, adev->gmc.aper_size);
> + arch_io_free_memtype_wc(adev->gmc.vram_aper_base, adev->gmc.vram_aper_size);
> }
> drm_dev_exit(idx);
> }
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> index 73b831b47892..0e8f580769ab 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> @@ -583,7 +583,7 @@ static int amdgpu_ttm_io_mem_reserve(struct ttm_device *bdev,
> mem->bus.addr = (u8 *)adev->mman.vram_aper_base_kaddr +
> mem->bus.offset;
>
> - mem->bus.offset += adev->gmc.aper_base;
> + mem->bus.offset += adev->gmc.vram_aper_base;
> mem->bus.is_iomem = true;
> break;
> default:
> @@ -600,7 +600,7 @@ static unsigned long amdgpu_ttm_io_mem_pfn(struct ttm_buffer_object *bo,
>
> amdgpu_res_first(bo->resource, (u64)page_offset << PAGE_SHIFT, 0,
> &cursor);
> - return (adev->gmc.aper_base + cursor.start) >> PAGE_SHIFT;
> + return (adev->gmc.vram_aper_base + cursor.start) >> PAGE_SHIFT;
> }
>
> /**
> @@ -1752,12 +1752,12 @@ int amdgpu_ttm_init(struct amdgpu_device *adev)
> #ifdef CONFIG_64BIT
> #ifdef CONFIG_X86
> if (adev->gmc.xgmi.connected_to_cpu)
> - adev->mman.vram_aper_base_kaddr = ioremap_cache(adev->gmc.aper_base,
> + adev->mman.vram_aper_base_kaddr = ioremap_cache(adev->gmc.vram_aper_base,
> adev->gmc.visible_vram_size);
>
> else
> #endif
> - adev->mman.vram_aper_base_kaddr = ioremap_wc(adev->gmc.aper_base,
> + adev->mman.vram_aper_base_kaddr = ioremap_wc(adev->gmc.vram_aper_base,
> adev->gmc.visible_vram_size);
> #endif
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
> index 9fa1d814508a..d66caad04c24 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
> @@ -649,7 +649,7 @@ int amdgpu_vram_mgr_alloc_sgt(struct amdgpu_device *adev,
> */
> amdgpu_res_first(res, offset, length, &cursor);
> for_each_sgtable_sg((*sgt), sg, i) {
> - phys_addr_t phys = cursor.start + adev->gmc.aper_base;
> + phys_addr_t phys = cursor.start + adev->gmc.vram_aper_base;
> size_t size = cursor.size;
> dma_addr_t addr;
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
> index 21e46817d82d..b2e4f4f06bdb 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
> @@ -825,18 +825,18 @@ static int gmc_v10_0_mc_init(struct amdgpu_device *adev)
> if (r)
> return r;
> }
> - adev->gmc.aper_base = pci_resource_start(adev->pdev, 0);
> - adev->gmc.aper_size = pci_resource_len(adev->pdev, 0);
> + adev->gmc.vram_aper_base = pci_resource_start(adev->pdev, 0);
> + adev->gmc.vram_aper_size = pci_resource_len(adev->pdev, 0);
>
> #ifdef CONFIG_X86_64
> if ((adev->flags & AMD_IS_APU) && !amdgpu_passthrough(adev)) {
> - adev->gmc.aper_base = adev->gfxhub.funcs->get_mc_fb_offset(adev);
> - adev->gmc.aper_size = adev->gmc.real_vram_size;
> + adev->gmc.vram_aper_base = adev->gfxhub.funcs->get_mc_fb_offset(adev);
> + adev->gmc.vram_aper_size = adev->gmc.real_vram_size;
> }
> #endif
>
> /* In case the PCI BAR is larger than the actual amount of vram */
> - adev->gmc.visible_vram_size = adev->gmc.aper_size;
> + adev->gmc.visible_vram_size = adev->gmc.vram_aper_size;
> if (adev->gmc.visible_vram_size > adev->gmc.real_vram_size)
> adev->gmc.visible_vram_size = adev->gmc.real_vram_size;
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v11_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v11_0.c
> index 4326078689cd..f993ce264c3f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v11_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v11_0.c
> @@ -692,17 +692,17 @@ static int gmc_v11_0_mc_init(struct amdgpu_device *adev)
> if (r)
> return r;
> }
> - adev->gmc.aper_base = pci_resource_start(adev->pdev, 0);
> - adev->gmc.aper_size = pci_resource_len(adev->pdev, 0);
> + adev->gmc.vram_aper_base = pci_resource_start(adev->pdev, 0);
> + adev->gmc.vram_aper_size = pci_resource_len(adev->pdev, 0);
>
> #ifdef CONFIG_X86_64
> if ((adev->flags & AMD_IS_APU) && !amdgpu_passthrough(adev)) {
> - adev->gmc.aper_base = adev->mmhub.funcs->get_mc_fb_offset(adev);
> - adev->gmc.aper_size = adev->gmc.real_vram_size;
> + adev->gmc.vram_aper_base = adev->mmhub.funcs->get_mc_fb_offset(adev);
> + adev->gmc.vram_aper_size = adev->gmc.real_vram_size;
> }
> #endif
> /* In case the PCI BAR is larger than the actual amount of vram */
> - adev->gmc.visible_vram_size = adev->gmc.aper_size;
> + adev->gmc.visible_vram_size = adev->gmc.vram_aper_size;
> if (adev->gmc.visible_vram_size > adev->gmc.real_vram_size)
> adev->gmc.visible_vram_size = adev->gmc.real_vram_size;
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
> index ec291d28edff..cd159309e9e5 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
> @@ -324,9 +324,9 @@ static int gmc_v6_0_mc_init(struct amdgpu_device *adev)
> if (r)
> return r;
> }
> - adev->gmc.aper_base = pci_resource_start(adev->pdev, 0);
> - adev->gmc.aper_size = pci_resource_len(adev->pdev, 0);
> - adev->gmc.visible_vram_size = adev->gmc.aper_size;
> + adev->gmc.vram_aper_base = pci_resource_start(adev->pdev, 0);
> + adev->gmc.vram_aper_size = pci_resource_len(adev->pdev, 0);
> + adev->gmc.visible_vram_size = adev->gmc.vram_aper_size;
>
> /* set the gart size */
> if (amdgpu_gart_size == -1) {
> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
> index 979da6f510e8..8ee9731a0c8c 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
> @@ -377,20 +377,20 @@ static int gmc_v7_0_mc_init(struct amdgpu_device *adev)
> if (r)
> return r;
> }
> - adev->gmc.aper_base = pci_resource_start(adev->pdev, 0);
> - adev->gmc.aper_size = pci_resource_len(adev->pdev, 0);
> + adev->gmc.vram_aper_base = pci_resource_start(adev->pdev, 0);
> + adev->gmc.vram_aper_size = pci_resource_len(adev->pdev, 0);
>
> #ifdef CONFIG_X86_64
> if ((adev->flags & AMD_IS_APU) &&
> - adev->gmc.real_vram_size > adev->gmc.aper_size &&
> + adev->gmc.real_vram_size > adev->gmc.vram_aper_size &&
> !amdgpu_passthrough(adev)) {
> - adev->gmc.aper_base = ((u64)RREG32(mmMC_VM_FB_OFFSET)) << 22;
> - adev->gmc.aper_size = adev->gmc.real_vram_size;
> + adev->gmc.vram_aper_base = ((u64)RREG32(mmMC_VM_FB_OFFSET)) << 22;
> + adev->gmc.vram_aper_size = adev->gmc.real_vram_size;
> }
> #endif
>
> /* In case the PCI BAR is larger than the actual amount of vram */
> - adev->gmc.visible_vram_size = adev->gmc.aper_size;
> + adev->gmc.visible_vram_size = adev->gmc.vram_aper_size;
> if (adev->gmc.visible_vram_size > adev->gmc.real_vram_size)
> adev->gmc.visible_vram_size = adev->gmc.real_vram_size;
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
> index 382dde1ce74c..259d797358f1 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
> @@ -577,18 +577,18 @@ static int gmc_v8_0_mc_init(struct amdgpu_device *adev)
> if (r)
> return r;
> }
> - adev->gmc.aper_base = pci_resource_start(adev->pdev, 0);
> - adev->gmc.aper_size = pci_resource_len(adev->pdev, 0);
> + adev->gmc.vram_aper_base = pci_resource_start(adev->pdev, 0);
> + adev->gmc.vram_aper_size = pci_resource_len(adev->pdev, 0);
>
> #ifdef CONFIG_X86_64
> if ((adev->flags & AMD_IS_APU) && !amdgpu_passthrough(adev)) {
> - adev->gmc.aper_base = ((u64)RREG32(mmMC_VM_FB_OFFSET)) << 22;
> - adev->gmc.aper_size = adev->gmc.real_vram_size;
> + adev->gmc.vram_aper_base = ((u64)RREG32(mmMC_VM_FB_OFFSET)) << 22;
> + adev->gmc.vram_aper_size = adev->gmc.real_vram_size;
> }
> #endif
>
> /* In case the PCI BAR is larger than the actual amount of vram */
> - adev->gmc.visible_vram_size = adev->gmc.aper_size;
> + adev->gmc.visible_vram_size = adev->gmc.vram_aper_size;
> if (adev->gmc.visible_vram_size > adev->gmc.real_vram_size)
> adev->gmc.visible_vram_size = adev->gmc.real_vram_size;
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> index 08d6cf79fb15..a7074995d97e 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> @@ -1509,8 +1509,8 @@ static int gmc_v9_0_mc_init(struct amdgpu_device *adev)
> if (r)
> return r;
> }
> - adev->gmc.aper_base = pci_resource_start(adev->pdev, 0);
> - adev->gmc.aper_size = pci_resource_len(adev->pdev, 0);
> + adev->gmc.vram_aper_base = pci_resource_start(adev->pdev, 0);
> + adev->gmc.vram_aper_size = pci_resource_len(adev->pdev, 0);
>
> #ifdef CONFIG_X86_64
> /*
> @@ -1528,16 +1528,16 @@ static int gmc_v9_0_mc_init(struct amdgpu_device *adev)
> if (((adev->flags & AMD_IS_APU) && !amdgpu_passthrough(adev)) ||
> (adev->gmc.xgmi.supported &&
> adev->gmc.xgmi.connected_to_cpu)) {
> - adev->gmc.aper_base =
> + adev->gmc.vram_aper_base =
> adev->gfxhub.funcs->get_mc_fb_offset(adev) +
> adev->gmc.xgmi.physical_node_id *
> adev->gmc.xgmi.node_segment_size;
> - adev->gmc.aper_size = adev->gmc.real_vram_size;
> + adev->gmc.vram_aper_size = adev->gmc.real_vram_size;
> }
>
> #endif
> /* In case the PCI BAR is larger than the actual amount of vram */
> - adev->gmc.visible_vram_size = adev->gmc.aper_size;
> + adev->gmc.visible_vram_size = adev->gmc.vram_aper_size;
> if (adev->gmc.visible_vram_size > adev->gmc.real_vram_size)
> adev->gmc.visible_vram_size = adev->gmc.real_vram_size;
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
> index 10048ce16aea..c86c6705b470 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
> @@ -1002,8 +1002,8 @@ int svm_migrate_init(struct amdgpu_device *adev)
> */
> size = ALIGN(adev->gmc.real_vram_size, 2ULL << 20);
> if (adev->gmc.xgmi.connected_to_cpu) {
> - pgmap->range.start = adev->gmc.aper_base;
> - pgmap->range.end = adev->gmc.aper_base + adev->gmc.aper_size - 1;
> + pgmap->range.start = adev->gmc.vram_aper_base;
> + pgmap->range.end = adev->gmc.vram_aper_base + adev->gmc.vram_aper_size - 1;
> pgmap->type = MEMORY_DEVICE_COHERENT;
> } else {
> res = devm_request_free_mem_region(adev->dev, &iomem_resource, size);
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v2 4/8] drm/amdgpu: rename doorbell variables
2023-02-14 16:15 [PATCH v2 0/8] Re-design doorbell framework for usermode queues Shashank Sharma
` (2 preceding siblings ...)
2023-02-14 16:15 ` [PATCH v2 3/8] drm/amdgpu: rename gmc.aper_base/size Shashank Sharma
@ 2023-02-14 16:15 ` Shashank Sharma
2023-02-14 18:27 ` Christian König
2023-02-14 16:15 ` [PATCH v2 5/8] drm/amdgpu: accommodate DOMAIN/PL_DOORBELL Shashank Sharma
` (3 subsequent siblings)
7 siblings, 1 reply; 28+ messages in thread
From: Shashank Sharma @ 2023-02-14 16:15 UTC (permalink / raw)
To: amd-gfx; +Cc: alexander.deucher, christian.koenig, Arvind.Yadav,
shashank.sharma
From: Alex Deucher <alexander.deucher@amd.com>
This patch:
- renames the adev->doorbell.base to adev->doorbell.doorbell_aper_base
- renames the adev->doorbell.size to adev->doorbell.doorbell_aper_size
- moves the adev->doorbell.ptr to adev->mman.doorbell_aper_base_kaddr
rest of the changes are just to accommodate these variable name changes.
V2: Mered 2 patches into this one doorbell clean-up patch.
Cc: Alex Deucher <alexander.deucher@amd.com>
Cc: Christian Koenig <christian.koenig@amd.com>
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
Signed-off-by: Shashank Sharma <shashank.sharma@amd.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c | 8 ++---
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 34 ++++++++++----------
drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell.h | 5 ++-
drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c | 2 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h | 1 +
drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c | 2 +-
drivers/gpu/drm/amd/amdgpu/nbio_v2_3.c | 4 +--
drivers/gpu/drm/amd/amdgpu/nbio_v4_3.c | 4 +--
drivers/gpu/drm/amd/amdgpu/nbio_v6_1.c | 4 +--
drivers/gpu/drm/amd/amdgpu/nbio_v7_2.c | 4 +--
drivers/gpu/drm/amd/amdgpu/nbio_v7_4.c | 4 +--
drivers/gpu/drm/amd/amdgpu/nbio_v7_7.c | 4 +--
12 files changed, 38 insertions(+), 38 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
index 58689b2a2d1c..0493c64e9d0a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
@@ -106,13 +106,13 @@ static void amdgpu_doorbell_get_kfd_info(struct amdgpu_device *adev,
* not initialized as AMDGPU manages the whole
* doorbell space.
*/
- *aperture_base = adev->doorbell.base;
+ *aperture_base = adev->doorbell.doorbell_aper_base;
*aperture_size = 0;
*start_offset = 0;
- } else if (adev->doorbell.size > adev->doorbell.num_doorbells *
+ } else if (adev->doorbell.doorbell_aper_size > adev->doorbell.num_doorbells *
sizeof(u32)) {
- *aperture_base = adev->doorbell.base;
- *aperture_size = adev->doorbell.size;
+ *aperture_base = adev->doorbell.doorbell_aper_base;
+ *aperture_size = adev->doorbell.doorbell_aper_size;
*start_offset = adev->doorbell.num_doorbells * sizeof(u32);
} else {
*aperture_base = 0;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 45588b7919fe..43c1b67c2778 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -597,7 +597,7 @@ u32 amdgpu_mm_rdoorbell(struct amdgpu_device *adev, u32 index)
return 0;
if (index < adev->doorbell.num_doorbells) {
- return readl(adev->doorbell.ptr + index);
+ return readl(adev->mman.doorbell_aper_base_kaddr + index);
} else {
DRM_ERROR("reading beyond doorbell aperture: 0x%08x!\n", index);
return 0;
@@ -620,7 +620,7 @@ void amdgpu_mm_wdoorbell(struct amdgpu_device *adev, u32 index, u32 v)
return;
if (index < adev->doorbell.num_doorbells) {
- writel(v, adev->doorbell.ptr + index);
+ writel(v, adev->mman.doorbell_aper_base_kaddr + index);
} else {
DRM_ERROR("writing beyond doorbell aperture: 0x%08x!\n", index);
}
@@ -641,7 +641,7 @@ u64 amdgpu_mm_rdoorbell64(struct amdgpu_device *adev, u32 index)
return 0;
if (index < adev->doorbell.num_doorbells) {
- return atomic64_read((atomic64_t *)(adev->doorbell.ptr + index));
+ return atomic64_read((atomic64_t *)(adev->mman.doorbell_aper_base_kaddr + index));
} else {
DRM_ERROR("reading beyond doorbell aperture: 0x%08x!\n", index);
return 0;
@@ -664,7 +664,7 @@ void amdgpu_mm_wdoorbell64(struct amdgpu_device *adev, u32 index, u64 v)
return;
if (index < adev->doorbell.num_doorbells) {
- atomic64_set((atomic64_t *)(adev->doorbell.ptr + index), v);
+ atomic64_set((atomic64_t *)(adev->mman.doorbell_aper_base_kaddr + index), v);
} else {
DRM_ERROR("writing beyond doorbell aperture: 0x%08x!\n", index);
}
@@ -1035,10 +1035,10 @@ static int amdgpu_device_doorbell_init(struct amdgpu_device *adev)
/* No doorbell on SI hardware generation */
if (adev->asic_type < CHIP_BONAIRE) {
- adev->doorbell.base = 0;
- adev->doorbell.size = 0;
+ adev->doorbell.doorbell_aper_base = 0;
+ adev->doorbell.doorbell_aper_size = 0;
adev->doorbell.num_doorbells = 0;
- adev->doorbell.ptr = NULL;
+ adev->mman.doorbell_aper_base_kaddr = NULL;
return 0;
}
@@ -1048,15 +1048,15 @@ static int amdgpu_device_doorbell_init(struct amdgpu_device *adev)
amdgpu_asic_init_doorbell_index(adev);
/* doorbell bar mapping */
- adev->doorbell.base = pci_resource_start(adev->pdev, 2);
- adev->doorbell.size = pci_resource_len(adev->pdev, 2);
+ adev->doorbell.doorbell_aper_base = pci_resource_start(adev->pdev, 2);
+ adev->doorbell.doorbell_aper_size = pci_resource_len(adev->pdev, 2);
if (adev->enable_mes) {
adev->doorbell.num_doorbells =
- adev->doorbell.size / sizeof(u32);
+ adev->doorbell.doorbell_aper_size / sizeof(u32);
} else {
adev->doorbell.num_doorbells =
- min_t(u32, adev->doorbell.size / sizeof(u32),
+ min_t(u32, adev->doorbell.doorbell_aper_size / sizeof(u32),
adev->doorbell_index.max_assignment+1);
if (adev->doorbell.num_doorbells == 0)
return -EINVAL;
@@ -1071,10 +1071,10 @@ static int amdgpu_device_doorbell_init(struct amdgpu_device *adev)
adev->doorbell.num_doorbells += 0x400;
}
- adev->doorbell.ptr = ioremap(adev->doorbell.base,
- adev->doorbell.num_doorbells *
- sizeof(u32));
- if (adev->doorbell.ptr == NULL)
+ adev->mman.doorbell_aper_base_kaddr = ioremap(adev->doorbell.doorbell_aper_base,
+ adev->doorbell.num_doorbells *
+ sizeof(u32));
+ if (adev->mman.doorbell_aper_base_kaddr == NULL)
return -ENOMEM;
return 0;
@@ -1089,8 +1089,8 @@ static int amdgpu_device_doorbell_init(struct amdgpu_device *adev)
*/
static void amdgpu_device_doorbell_fini(struct amdgpu_device *adev)
{
- iounmap(adev->doorbell.ptr);
- adev->doorbell.ptr = NULL;
+ iounmap(adev->mman.doorbell_aper_base_kaddr);
+ adev->mman.doorbell_aper_base_kaddr = NULL;
}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell.h
index 7199b6b0be81..526b6b4a86dd 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell.h
@@ -26,9 +26,8 @@
*/
struct amdgpu_doorbell {
/* doorbell mmio */
- resource_size_t base;
- resource_size_t size;
- u32 __iomem *ptr;
+ resource_size_t doorbell_aper_base;
+ resource_size_t doorbell_aper_size;
u32 num_doorbells; /* Number of doorbells actually reserved for amdgpu. */
};
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c
index 0c546245793b..b79fb369f0f6 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c
@@ -126,7 +126,7 @@ static int amdgpu_mes_doorbell_init(struct amdgpu_device *adev)
roundup(doorbell_start_offset,
amdgpu_mes_doorbell_process_slice(adev));
- doorbell_aperture_size = adev->doorbell.size;
+ doorbell_aperture_size = adev->doorbell.doorbell_aper_size;
doorbell_aperture_size =
rounddown(doorbell_aperture_size,
amdgpu_mes_doorbell_process_slice(adev));
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
index 929bc8abac28..967b265dbfa1 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
@@ -51,6 +51,7 @@ struct amdgpu_mman {
struct ttm_device bdev;
bool initialized;
void __iomem *vram_aper_base_kaddr;
+ u32 __iomem *doorbell_aper_base_kaddr;
/* buffer handling */
const struct amdgpu_buffer_funcs *buffer_funcs;
diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
index f202b45c413c..7722da8e7cb4 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
@@ -3526,7 +3526,7 @@ static int gfx_v9_0_kiq_init_register(struct amdgpu_ring *ring)
*/
if (check_if_enlarge_doorbell_range(adev))
WREG32_SOC15(GC, 0, mmCP_MEC_DOORBELL_RANGE_UPPER,
- (adev->doorbell.size - 4));
+ (adev->doorbell.doorbell_aper_size - 4));
else
WREG32_SOC15(GC, 0, mmCP_MEC_DOORBELL_RANGE_UPPER,
(adev->doorbell_index.userqueue_end * 2) << 2);
diff --git a/drivers/gpu/drm/amd/amdgpu/nbio_v2_3.c b/drivers/gpu/drm/amd/amdgpu/nbio_v2_3.c
index aa761ff3a5fa..c5fd58d5fef9 100644
--- a/drivers/gpu/drm/amd/amdgpu/nbio_v2_3.c
+++ b/drivers/gpu/drm/amd/amdgpu/nbio_v2_3.c
@@ -173,9 +173,9 @@ static void nbio_v2_3_enable_doorbell_selfring_aperture(struct amdgpu_device *ad
DOORBELL_SELFRING_GPA_APER_SIZE, 0);
WREG32_SOC15(NBIO, 0, mmBIF_BX_PF_DOORBELL_SELFRING_GPA_APER_BASE_LOW,
- lower_32_bits(adev->doorbell.base));
+ lower_32_bits(adev->doorbell.doorbell_aper_base));
WREG32_SOC15(NBIO, 0, mmBIF_BX_PF_DOORBELL_SELFRING_GPA_APER_BASE_HIGH,
- upper_32_bits(adev->doorbell.base));
+ upper_32_bits(adev->doorbell.doorbell_aper_base));
}
WREG32_SOC15(NBIO, 0, mmBIF_BX_PF_DOORBELL_SELFRING_GPA_APER_CNTL,
diff --git a/drivers/gpu/drm/amd/amdgpu/nbio_v4_3.c b/drivers/gpu/drm/amd/amdgpu/nbio_v4_3.c
index 15eb3658d70e..9d716ec71f28 100644
--- a/drivers/gpu/drm/amd/amdgpu/nbio_v4_3.c
+++ b/drivers/gpu/drm/amd/amdgpu/nbio_v4_3.c
@@ -169,9 +169,9 @@ static void nbio_v4_3_enable_doorbell_selfring_aperture(struct amdgpu_device *ad
DOORBELL_SELFRING_GPA_APER_SIZE, 0);
WREG32_SOC15(NBIO, 0, regBIF_BX_PF0_DOORBELL_SELFRING_GPA_APER_BASE_LOW,
- lower_32_bits(adev->doorbell.base));
+ lower_32_bits(adev->doorbell.doorbell_aper_base));
WREG32_SOC15(NBIO, 0, regBIF_BX_PF0_DOORBELL_SELFRING_GPA_APER_BASE_HIGH,
- upper_32_bits(adev->doorbell.base));
+ upper_32_bits(adev->doorbell.doorbell_aper_base));
}
WREG32_SOC15(NBIO, 0, regBIF_BX_PF0_DOORBELL_SELFRING_GPA_APER_CNTL,
diff --git a/drivers/gpu/drm/amd/amdgpu/nbio_v6_1.c b/drivers/gpu/drm/amd/amdgpu/nbio_v6_1.c
index 37615a77287b..19e175cc7340 100644
--- a/drivers/gpu/drm/amd/amdgpu/nbio_v6_1.c
+++ b/drivers/gpu/drm/amd/amdgpu/nbio_v6_1.c
@@ -121,9 +121,9 @@ static void nbio_v6_1_enable_doorbell_selfring_aperture(struct amdgpu_device *ad
REG_SET_FIELD(tmp, BIF_BX_PF0_DOORBELL_SELFRING_GPA_APER_CNTL, DOORBELL_SELFRING_GPA_APER_SIZE, 0);
WREG32_SOC15(NBIO, 0, mmBIF_BX_PF0_DOORBELL_SELFRING_GPA_APER_BASE_LOW,
- lower_32_bits(adev->doorbell.base));
+ lower_32_bits(adev->doorbell.doorbell_aper_base));
WREG32_SOC15(NBIO, 0, mmBIF_BX_PF0_DOORBELL_SELFRING_GPA_APER_BASE_HIGH,
- upper_32_bits(adev->doorbell.base));
+ upper_32_bits(adev->doorbell.doorbell_aper_base));
}
WREG32_SOC15(NBIO, 0, mmBIF_BX_PF0_DOORBELL_SELFRING_GPA_APER_CNTL, tmp);
diff --git a/drivers/gpu/drm/amd/amdgpu/nbio_v7_2.c b/drivers/gpu/drm/amd/amdgpu/nbio_v7_2.c
index 31776b12e4c4..bb2f1857b1e2 100644
--- a/drivers/gpu/drm/amd/amdgpu/nbio_v7_2.c
+++ b/drivers/gpu/drm/amd/amdgpu/nbio_v7_2.c
@@ -175,10 +175,10 @@ static void nbio_v7_2_enable_doorbell_selfring_aperture(struct amdgpu_device *ad
WREG32_SOC15(NBIO, 0,
regBIF_BX_PF0_DOORBELL_SELFRING_GPA_APER_BASE_LOW,
- lower_32_bits(adev->doorbell.base));
+ lower_32_bits(adev->doorbell.doorbell_aper_base));
WREG32_SOC15(NBIO, 0,
regBIF_BX_PF0_DOORBELL_SELFRING_GPA_APER_BASE_HIGH,
- upper_32_bits(adev->doorbell.base));
+ upper_32_bits(adev->doorbell.doorbell_aper_base));
}
WREG32_SOC15(NBIO, 0, regBIF_BX_PF0_DOORBELL_SELFRING_GPA_APER_CNTL,
diff --git a/drivers/gpu/drm/amd/amdgpu/nbio_v7_4.c b/drivers/gpu/drm/amd/amdgpu/nbio_v7_4.c
index 19455a725939..ee1982bb06aa 100644
--- a/drivers/gpu/drm/amd/amdgpu/nbio_v7_4.c
+++ b/drivers/gpu/drm/amd/amdgpu/nbio_v7_4.c
@@ -223,9 +223,9 @@ static void nbio_v7_4_enable_doorbell_selfring_aperture(struct amdgpu_device *ad
REG_SET_FIELD(tmp, DOORBELL_SELFRING_GPA_APER_CNTL, DOORBELL_SELFRING_GPA_APER_SIZE, 0);
WREG32_SOC15(NBIO, 0, mmDOORBELL_SELFRING_GPA_APER_BASE_LOW,
- lower_32_bits(adev->doorbell.base));
+ lower_32_bits(adev->doorbell.doorbell_aper_base));
WREG32_SOC15(NBIO, 0, mmDOORBELL_SELFRING_GPA_APER_BASE_HIGH,
- upper_32_bits(adev->doorbell.base));
+ upper_32_bits(adev->doorbell.doorbell_aper_base));
}
WREG32_SOC15(NBIO, 0, mmDOORBELL_SELFRING_GPA_APER_CNTL, tmp);
diff --git a/drivers/gpu/drm/amd/amdgpu/nbio_v7_7.c b/drivers/gpu/drm/amd/amdgpu/nbio_v7_7.c
index def89379b51a..180d50bcb40f 100644
--- a/drivers/gpu/drm/amd/amdgpu/nbio_v7_7.c
+++ b/drivers/gpu/drm/amd/amdgpu/nbio_v7_7.c
@@ -132,10 +132,10 @@ static void nbio_v7_7_enable_doorbell_selfring_aperture(struct amdgpu_device *ad
WREG32_SOC15(NBIO, 0,
regBIF_BX_PF0_DOORBELL_SELFRING_GPA_APER_BASE_LOW,
- lower_32_bits(adev->doorbell.base));
+ lower_32_bits(adev->doorbell.doorbell_aper_base));
WREG32_SOC15(NBIO, 0,
regBIF_BX_PF0_DOORBELL_SELFRING_GPA_APER_BASE_HIGH,
- upper_32_bits(adev->doorbell.base));
+ upper_32_bits(adev->doorbell.doorbell_aper_base));
}
WREG32_SOC15(NBIO, 0, regBIF_BX_PF0_DOORBELL_SELFRING_GPA_APER_CNTL,
--
2.34.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH v2 4/8] drm/amdgpu: rename doorbell variables
2023-02-14 16:15 ` [PATCH v2 4/8] drm/amdgpu: rename doorbell variables Shashank Sharma
@ 2023-02-14 18:27 ` Christian König
2023-02-14 19:16 ` Shashank Sharma
0 siblings, 1 reply; 28+ messages in thread
From: Christian König @ 2023-02-14 18:27 UTC (permalink / raw)
To: Shashank Sharma, amd-gfx; +Cc: alexander.deucher, Arvind.Yadav
Am 14.02.23 um 17:15 schrieb Shashank Sharma:
> From: Alex Deucher <alexander.deucher@amd.com>
>
> This patch:
> - renames the adev->doorbell.base to adev->doorbell.doorbell_aper_base
> - renames the adev->doorbell.size to adev->doorbell.doorbell_aper_size
> - moves the adev->doorbell.ptr to adev->mman.doorbell_aper_base_kaddr
>
> rest of the changes are just to accommodate these variable name changes.
>
> V2: Mered 2 patches into this one doorbell clean-up patch.
>
> Cc: Alex Deucher <alexander.deucher@amd.com>
> Cc: Christian Koenig <christian.koenig@amd.com>
> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> Signed-off-by: Shashank Sharma <shashank.sharma@amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c | 8 ++---
> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 34 ++++++++++----------
> drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell.h | 5 ++-
> drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c | 2 +-
> drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h | 1 +
> drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c | 2 +-
> drivers/gpu/drm/amd/amdgpu/nbio_v2_3.c | 4 +--
> drivers/gpu/drm/amd/amdgpu/nbio_v4_3.c | 4 +--
> drivers/gpu/drm/amd/amdgpu/nbio_v6_1.c | 4 +--
> drivers/gpu/drm/amd/amdgpu/nbio_v7_2.c | 4 +--
> drivers/gpu/drm/amd/amdgpu/nbio_v7_4.c | 4 +--
> drivers/gpu/drm/amd/amdgpu/nbio_v7_7.c | 4 +--
> 12 files changed, 38 insertions(+), 38 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
> index 58689b2a2d1c..0493c64e9d0a 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
> @@ -106,13 +106,13 @@ static void amdgpu_doorbell_get_kfd_info(struct amdgpu_device *adev,
> * not initialized as AMDGPU manages the whole
> * doorbell space.
> */
> - *aperture_base = adev->doorbell.base;
> + *aperture_base = adev->doorbell.doorbell_aper_base;
> *aperture_size = 0;
> *start_offset = 0;
> - } else if (adev->doorbell.size > adev->doorbell.num_doorbells *
> + } else if (adev->doorbell.doorbell_aper_size > adev->doorbell.num_doorbells *
> sizeof(u32)) {
> - *aperture_base = adev->doorbell.base;
> - *aperture_size = adev->doorbell.size;
> + *aperture_base = adev->doorbell.doorbell_aper_base;
> + *aperture_size = adev->doorbell.doorbell_aper_size;
Well that now looks a bit duplicated and you completely remove
doorbell_aper_base_kaddr later on, right?
Christian.
> *start_offset = adev->doorbell.num_doorbells * sizeof(u32);
> } else {
> *aperture_base = 0;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index 45588b7919fe..43c1b67c2778 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -597,7 +597,7 @@ u32 amdgpu_mm_rdoorbell(struct amdgpu_device *adev, u32 index)
> return 0;
>
> if (index < adev->doorbell.num_doorbells) {
> - return readl(adev->doorbell.ptr + index);
> + return readl(adev->mman.doorbell_aper_base_kaddr + index);
> } else {
> DRM_ERROR("reading beyond doorbell aperture: 0x%08x!\n", index);
> return 0;
> @@ -620,7 +620,7 @@ void amdgpu_mm_wdoorbell(struct amdgpu_device *adev, u32 index, u32 v)
> return;
>
> if (index < adev->doorbell.num_doorbells) {
> - writel(v, adev->doorbell.ptr + index);
> + writel(v, adev->mman.doorbell_aper_base_kaddr + index);
> } else {
> DRM_ERROR("writing beyond doorbell aperture: 0x%08x!\n", index);
> }
> @@ -641,7 +641,7 @@ u64 amdgpu_mm_rdoorbell64(struct amdgpu_device *adev, u32 index)
> return 0;
>
> if (index < adev->doorbell.num_doorbells) {
> - return atomic64_read((atomic64_t *)(adev->doorbell.ptr + index));
> + return atomic64_read((atomic64_t *)(adev->mman.doorbell_aper_base_kaddr + index));
> } else {
> DRM_ERROR("reading beyond doorbell aperture: 0x%08x!\n", index);
> return 0;
> @@ -664,7 +664,7 @@ void amdgpu_mm_wdoorbell64(struct amdgpu_device *adev, u32 index, u64 v)
> return;
>
> if (index < adev->doorbell.num_doorbells) {
> - atomic64_set((atomic64_t *)(adev->doorbell.ptr + index), v);
> + atomic64_set((atomic64_t *)(adev->mman.doorbell_aper_base_kaddr + index), v);
> } else {
> DRM_ERROR("writing beyond doorbell aperture: 0x%08x!\n", index);
> }
> @@ -1035,10 +1035,10 @@ static int amdgpu_device_doorbell_init(struct amdgpu_device *adev)
>
> /* No doorbell on SI hardware generation */
> if (adev->asic_type < CHIP_BONAIRE) {
> - adev->doorbell.base = 0;
> - adev->doorbell.size = 0;
> + adev->doorbell.doorbell_aper_base = 0;
> + adev->doorbell.doorbell_aper_size = 0;
> adev->doorbell.num_doorbells = 0;
> - adev->doorbell.ptr = NULL;
> + adev->mman.doorbell_aper_base_kaddr = NULL;
> return 0;
> }
>
> @@ -1048,15 +1048,15 @@ static int amdgpu_device_doorbell_init(struct amdgpu_device *adev)
> amdgpu_asic_init_doorbell_index(adev);
>
> /* doorbell bar mapping */
> - adev->doorbell.base = pci_resource_start(adev->pdev, 2);
> - adev->doorbell.size = pci_resource_len(adev->pdev, 2);
> + adev->doorbell.doorbell_aper_base = pci_resource_start(adev->pdev, 2);
> + adev->doorbell.doorbell_aper_size = pci_resource_len(adev->pdev, 2);
>
> if (adev->enable_mes) {
> adev->doorbell.num_doorbells =
> - adev->doorbell.size / sizeof(u32);
> + adev->doorbell.doorbell_aper_size / sizeof(u32);
> } else {
> adev->doorbell.num_doorbells =
> - min_t(u32, adev->doorbell.size / sizeof(u32),
> + min_t(u32, adev->doorbell.doorbell_aper_size / sizeof(u32),
> adev->doorbell_index.max_assignment+1);
> if (adev->doorbell.num_doorbells == 0)
> return -EINVAL;
> @@ -1071,10 +1071,10 @@ static int amdgpu_device_doorbell_init(struct amdgpu_device *adev)
> adev->doorbell.num_doorbells += 0x400;
> }
>
> - adev->doorbell.ptr = ioremap(adev->doorbell.base,
> - adev->doorbell.num_doorbells *
> - sizeof(u32));
> - if (adev->doorbell.ptr == NULL)
> + adev->mman.doorbell_aper_base_kaddr = ioremap(adev->doorbell.doorbell_aper_base,
> + adev->doorbell.num_doorbells *
> + sizeof(u32));
> + if (adev->mman.doorbell_aper_base_kaddr == NULL)
> return -ENOMEM;
>
> return 0;
> @@ -1089,8 +1089,8 @@ static int amdgpu_device_doorbell_init(struct amdgpu_device *adev)
> */
> static void amdgpu_device_doorbell_fini(struct amdgpu_device *adev)
> {
> - iounmap(adev->doorbell.ptr);
> - adev->doorbell.ptr = NULL;
> + iounmap(adev->mman.doorbell_aper_base_kaddr);
> + adev->mman.doorbell_aper_base_kaddr = NULL;
> }
>
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell.h
> index 7199b6b0be81..526b6b4a86dd 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell.h
> @@ -26,9 +26,8 @@
> */
> struct amdgpu_doorbell {
> /* doorbell mmio */
> - resource_size_t base;
> - resource_size_t size;
> - u32 __iomem *ptr;
> + resource_size_t doorbell_aper_base;
> + resource_size_t doorbell_aper_size;
> u32 num_doorbells; /* Number of doorbells actually reserved for amdgpu. */
> };
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c
> index 0c546245793b..b79fb369f0f6 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c
> @@ -126,7 +126,7 @@ static int amdgpu_mes_doorbell_init(struct amdgpu_device *adev)
> roundup(doorbell_start_offset,
> amdgpu_mes_doorbell_process_slice(adev));
>
> - doorbell_aperture_size = adev->doorbell.size;
> + doorbell_aperture_size = adev->doorbell.doorbell_aper_size;
> doorbell_aperture_size =
> rounddown(doorbell_aperture_size,
> amdgpu_mes_doorbell_process_slice(adev));
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
> index 929bc8abac28..967b265dbfa1 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
> @@ -51,6 +51,7 @@ struct amdgpu_mman {
> struct ttm_device bdev;
> bool initialized;
> void __iomem *vram_aper_base_kaddr;
> + u32 __iomem *doorbell_aper_base_kaddr;
>
> /* buffer handling */
> const struct amdgpu_buffer_funcs *buffer_funcs;
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> index f202b45c413c..7722da8e7cb4 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> @@ -3526,7 +3526,7 @@ static int gfx_v9_0_kiq_init_register(struct amdgpu_ring *ring)
> */
> if (check_if_enlarge_doorbell_range(adev))
> WREG32_SOC15(GC, 0, mmCP_MEC_DOORBELL_RANGE_UPPER,
> - (adev->doorbell.size - 4));
> + (adev->doorbell.doorbell_aper_size - 4));
> else
> WREG32_SOC15(GC, 0, mmCP_MEC_DOORBELL_RANGE_UPPER,
> (adev->doorbell_index.userqueue_end * 2) << 2);
> diff --git a/drivers/gpu/drm/amd/amdgpu/nbio_v2_3.c b/drivers/gpu/drm/amd/amdgpu/nbio_v2_3.c
> index aa761ff3a5fa..c5fd58d5fef9 100644
> --- a/drivers/gpu/drm/amd/amdgpu/nbio_v2_3.c
> +++ b/drivers/gpu/drm/amd/amdgpu/nbio_v2_3.c
> @@ -173,9 +173,9 @@ static void nbio_v2_3_enable_doorbell_selfring_aperture(struct amdgpu_device *ad
> DOORBELL_SELFRING_GPA_APER_SIZE, 0);
>
> WREG32_SOC15(NBIO, 0, mmBIF_BX_PF_DOORBELL_SELFRING_GPA_APER_BASE_LOW,
> - lower_32_bits(adev->doorbell.base));
> + lower_32_bits(adev->doorbell.doorbell_aper_base));
> WREG32_SOC15(NBIO, 0, mmBIF_BX_PF_DOORBELL_SELFRING_GPA_APER_BASE_HIGH,
> - upper_32_bits(adev->doorbell.base));
> + upper_32_bits(adev->doorbell.doorbell_aper_base));
> }
>
> WREG32_SOC15(NBIO, 0, mmBIF_BX_PF_DOORBELL_SELFRING_GPA_APER_CNTL,
> diff --git a/drivers/gpu/drm/amd/amdgpu/nbio_v4_3.c b/drivers/gpu/drm/amd/amdgpu/nbio_v4_3.c
> index 15eb3658d70e..9d716ec71f28 100644
> --- a/drivers/gpu/drm/amd/amdgpu/nbio_v4_3.c
> +++ b/drivers/gpu/drm/amd/amdgpu/nbio_v4_3.c
> @@ -169,9 +169,9 @@ static void nbio_v4_3_enable_doorbell_selfring_aperture(struct amdgpu_device *ad
> DOORBELL_SELFRING_GPA_APER_SIZE, 0);
>
> WREG32_SOC15(NBIO, 0, regBIF_BX_PF0_DOORBELL_SELFRING_GPA_APER_BASE_LOW,
> - lower_32_bits(adev->doorbell.base));
> + lower_32_bits(adev->doorbell.doorbell_aper_base));
> WREG32_SOC15(NBIO, 0, regBIF_BX_PF0_DOORBELL_SELFRING_GPA_APER_BASE_HIGH,
> - upper_32_bits(adev->doorbell.base));
> + upper_32_bits(adev->doorbell.doorbell_aper_base));
> }
>
> WREG32_SOC15(NBIO, 0, regBIF_BX_PF0_DOORBELL_SELFRING_GPA_APER_CNTL,
> diff --git a/drivers/gpu/drm/amd/amdgpu/nbio_v6_1.c b/drivers/gpu/drm/amd/amdgpu/nbio_v6_1.c
> index 37615a77287b..19e175cc7340 100644
> --- a/drivers/gpu/drm/amd/amdgpu/nbio_v6_1.c
> +++ b/drivers/gpu/drm/amd/amdgpu/nbio_v6_1.c
> @@ -121,9 +121,9 @@ static void nbio_v6_1_enable_doorbell_selfring_aperture(struct amdgpu_device *ad
> REG_SET_FIELD(tmp, BIF_BX_PF0_DOORBELL_SELFRING_GPA_APER_CNTL, DOORBELL_SELFRING_GPA_APER_SIZE, 0);
>
> WREG32_SOC15(NBIO, 0, mmBIF_BX_PF0_DOORBELL_SELFRING_GPA_APER_BASE_LOW,
> - lower_32_bits(adev->doorbell.base));
> + lower_32_bits(adev->doorbell.doorbell_aper_base));
> WREG32_SOC15(NBIO, 0, mmBIF_BX_PF0_DOORBELL_SELFRING_GPA_APER_BASE_HIGH,
> - upper_32_bits(adev->doorbell.base));
> + upper_32_bits(adev->doorbell.doorbell_aper_base));
> }
>
> WREG32_SOC15(NBIO, 0, mmBIF_BX_PF0_DOORBELL_SELFRING_GPA_APER_CNTL, tmp);
> diff --git a/drivers/gpu/drm/amd/amdgpu/nbio_v7_2.c b/drivers/gpu/drm/amd/amdgpu/nbio_v7_2.c
> index 31776b12e4c4..bb2f1857b1e2 100644
> --- a/drivers/gpu/drm/amd/amdgpu/nbio_v7_2.c
> +++ b/drivers/gpu/drm/amd/amdgpu/nbio_v7_2.c
> @@ -175,10 +175,10 @@ static void nbio_v7_2_enable_doorbell_selfring_aperture(struct amdgpu_device *ad
>
> WREG32_SOC15(NBIO, 0,
> regBIF_BX_PF0_DOORBELL_SELFRING_GPA_APER_BASE_LOW,
> - lower_32_bits(adev->doorbell.base));
> + lower_32_bits(adev->doorbell.doorbell_aper_base));
> WREG32_SOC15(NBIO, 0,
> regBIF_BX_PF0_DOORBELL_SELFRING_GPA_APER_BASE_HIGH,
> - upper_32_bits(adev->doorbell.base));
> + upper_32_bits(adev->doorbell.doorbell_aper_base));
> }
>
> WREG32_SOC15(NBIO, 0, regBIF_BX_PF0_DOORBELL_SELFRING_GPA_APER_CNTL,
> diff --git a/drivers/gpu/drm/amd/amdgpu/nbio_v7_4.c b/drivers/gpu/drm/amd/amdgpu/nbio_v7_4.c
> index 19455a725939..ee1982bb06aa 100644
> --- a/drivers/gpu/drm/amd/amdgpu/nbio_v7_4.c
> +++ b/drivers/gpu/drm/amd/amdgpu/nbio_v7_4.c
> @@ -223,9 +223,9 @@ static void nbio_v7_4_enable_doorbell_selfring_aperture(struct amdgpu_device *ad
> REG_SET_FIELD(tmp, DOORBELL_SELFRING_GPA_APER_CNTL, DOORBELL_SELFRING_GPA_APER_SIZE, 0);
>
> WREG32_SOC15(NBIO, 0, mmDOORBELL_SELFRING_GPA_APER_BASE_LOW,
> - lower_32_bits(adev->doorbell.base));
> + lower_32_bits(adev->doorbell.doorbell_aper_base));
> WREG32_SOC15(NBIO, 0, mmDOORBELL_SELFRING_GPA_APER_BASE_HIGH,
> - upper_32_bits(adev->doorbell.base));
> + upper_32_bits(adev->doorbell.doorbell_aper_base));
> }
>
> WREG32_SOC15(NBIO, 0, mmDOORBELL_SELFRING_GPA_APER_CNTL, tmp);
> diff --git a/drivers/gpu/drm/amd/amdgpu/nbio_v7_7.c b/drivers/gpu/drm/amd/amdgpu/nbio_v7_7.c
> index def89379b51a..180d50bcb40f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/nbio_v7_7.c
> +++ b/drivers/gpu/drm/amd/amdgpu/nbio_v7_7.c
> @@ -132,10 +132,10 @@ static void nbio_v7_7_enable_doorbell_selfring_aperture(struct amdgpu_device *ad
>
> WREG32_SOC15(NBIO, 0,
> regBIF_BX_PF0_DOORBELL_SELFRING_GPA_APER_BASE_LOW,
> - lower_32_bits(adev->doorbell.base));
> + lower_32_bits(adev->doorbell.doorbell_aper_base));
> WREG32_SOC15(NBIO, 0,
> regBIF_BX_PF0_DOORBELL_SELFRING_GPA_APER_BASE_HIGH,
> - upper_32_bits(adev->doorbell.base));
> + upper_32_bits(adev->doorbell.doorbell_aper_base));
> }
>
> WREG32_SOC15(NBIO, 0, regBIF_BX_PF0_DOORBELL_SELFRING_GPA_APER_CNTL,
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v2 4/8] drm/amdgpu: rename doorbell variables
2023-02-14 18:27 ` Christian König
@ 2023-02-14 19:16 ` Shashank Sharma
0 siblings, 0 replies; 28+ messages in thread
From: Shashank Sharma @ 2023-02-14 19:16 UTC (permalink / raw)
To: Christian König, amd-gfx; +Cc: alexander.deucher, Arvind.Yadav
On 14/02/2023 19:27, Christian König wrote:
>
>
> Am 14.02.23 um 17:15 schrieb Shashank Sharma:
>> From: Alex Deucher <alexander.deucher@amd.com>
>>
>> This patch:
>> - renames the adev->doorbell.base to adev->doorbell.doorbell_aper_base
>> - renames the adev->doorbell.size to adev->doorbell.doorbell_aper_size
>> - moves the adev->doorbell.ptr to adev->mman.doorbell_aper_base_kaddr
>>
>> rest of the changes are just to accommodate these variable name changes.
>>
>> V2: Mered 2 patches into this one doorbell clean-up patch.
>>
>> Cc: Alex Deucher <alexander.deucher@amd.com>
>> Cc: Christian Koenig <christian.koenig@amd.com>
>> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
>> Signed-off-by: Shashank Sharma <shashank.sharma@amd.com>
>> ---
>> drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c | 8 ++---
>> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 34 ++++++++++----------
>> drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell.h | 5 ++-
>> drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c | 2 +-
>> drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h | 1 +
>> drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c | 2 +-
>> drivers/gpu/drm/amd/amdgpu/nbio_v2_3.c | 4 +--
>> drivers/gpu/drm/amd/amdgpu/nbio_v4_3.c | 4 +--
>> drivers/gpu/drm/amd/amdgpu/nbio_v6_1.c | 4 +--
>> drivers/gpu/drm/amd/amdgpu/nbio_v7_2.c | 4 +--
>> drivers/gpu/drm/amd/amdgpu/nbio_v7_4.c | 4 +--
>> drivers/gpu/drm/amd/amdgpu/nbio_v7_7.c | 4 +--
>> 12 files changed, 38 insertions(+), 38 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
>> index 58689b2a2d1c..0493c64e9d0a 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
>> @@ -106,13 +106,13 @@ static void amdgpu_doorbell_get_kfd_info(struct
>> amdgpu_device *adev,
>> * not initialized as AMDGPU manages the whole
>> * doorbell space.
>> */
>> - *aperture_base = adev->doorbell.base;
>> + *aperture_base = adev->doorbell.doorbell_aper_base;
>> *aperture_size = 0;
>> *start_offset = 0;
>> - } else if (adev->doorbell.size > adev->doorbell.num_doorbells *
>> + } else if (adev->doorbell.doorbell_aper_size >
>> adev->doorbell.num_doorbells *
>> sizeof(u32)) {
>> - *aperture_base = adev->doorbell.base;
>> - *aperture_size = adev->doorbell.size;
>> + *aperture_base = adev->doorbell.doorbell_aper_base;
>> + *aperture_size = adev->doorbell.doorbell_aper_size;
>
> Well that now looks a bit duplicated and you completely remove
> doorbell_aper_base_kaddr later on, right?
>
> Christian.
>
Yeah, I still kept it as this cleanup is a bit peculiar and not easy to
follow on review, but your right, lets keep it cleaner.
I will take remove doorbell_aper_base_kaddr.
- Shashank
>> *start_offset = adev->doorbell.num_doorbells * sizeof(u32);
>> } else {
>> *aperture_base = 0;
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> index 45588b7919fe..43c1b67c2778 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> @@ -597,7 +597,7 @@ u32 amdgpu_mm_rdoorbell(struct amdgpu_device
>> *adev, u32 index)
>> return 0;
>> if (index < adev->doorbell.num_doorbells) {
>> - return readl(adev->doorbell.ptr + index);
>> + return readl(adev->mman.doorbell_aper_base_kaddr + index);
>> } else {
>> DRM_ERROR("reading beyond doorbell aperture: 0x%08x!\n",
>> index);
>> return 0;
>> @@ -620,7 +620,7 @@ void amdgpu_mm_wdoorbell(struct amdgpu_device
>> *adev, u32 index, u32 v)
>> return;
>> if (index < adev->doorbell.num_doorbells) {
>> - writel(v, adev->doorbell.ptr + index);
>> + writel(v, adev->mman.doorbell_aper_base_kaddr + index);
>> } else {
>> DRM_ERROR("writing beyond doorbell aperture: 0x%08x!\n",
>> index);
>> }
>> @@ -641,7 +641,7 @@ u64 amdgpu_mm_rdoorbell64(struct amdgpu_device
>> *adev, u32 index)
>> return 0;
>> if (index < adev->doorbell.num_doorbells) {
>> - return atomic64_read((atomic64_t *)(adev->doorbell.ptr +
>> index));
>> + return atomic64_read((atomic64_t
>> *)(adev->mman.doorbell_aper_base_kaddr + index));
>> } else {
>> DRM_ERROR("reading beyond doorbell aperture: 0x%08x!\n",
>> index);
>> return 0;
>> @@ -664,7 +664,7 @@ void amdgpu_mm_wdoorbell64(struct amdgpu_device
>> *adev, u32 index, u64 v)
>> return;
>> if (index < adev->doorbell.num_doorbells) {
>> - atomic64_set((atomic64_t *)(adev->doorbell.ptr + index), v);
>> + atomic64_set((atomic64_t
>> *)(adev->mman.doorbell_aper_base_kaddr + index), v);
>> } else {
>> DRM_ERROR("writing beyond doorbell aperture: 0x%08x!\n",
>> index);
>> }
>> @@ -1035,10 +1035,10 @@ static int amdgpu_device_doorbell_init(struct
>> amdgpu_device *adev)
>> /* No doorbell on SI hardware generation */
>> if (adev->asic_type < CHIP_BONAIRE) {
>> - adev->doorbell.base = 0;
>> - adev->doorbell.size = 0;
>> + adev->doorbell.doorbell_aper_base = 0;
>> + adev->doorbell.doorbell_aper_size = 0;
>> adev->doorbell.num_doorbells = 0;
>> - adev->doorbell.ptr = NULL;
>> + adev->mman.doorbell_aper_base_kaddr = NULL;
>> return 0;
>> }
>> @@ -1048,15 +1048,15 @@ static int
>> amdgpu_device_doorbell_init(struct amdgpu_device *adev)
>> amdgpu_asic_init_doorbell_index(adev);
>> /* doorbell bar mapping */
>> - adev->doorbell.base = pci_resource_start(adev->pdev, 2);
>> - adev->doorbell.size = pci_resource_len(adev->pdev, 2);
>> + adev->doorbell.doorbell_aper_base =
>> pci_resource_start(adev->pdev, 2);
>> + adev->doorbell.doorbell_aper_size = pci_resource_len(adev->pdev,
>> 2);
>> if (adev->enable_mes) {
>> adev->doorbell.num_doorbells =
>> - adev->doorbell.size / sizeof(u32);
>> + adev->doorbell.doorbell_aper_size / sizeof(u32);
>> } else {
>> adev->doorbell.num_doorbells =
>> - min_t(u32, adev->doorbell.size / sizeof(u32),
>> + min_t(u32, adev->doorbell.doorbell_aper_size / sizeof(u32),
>> adev->doorbell_index.max_assignment+1);
>> if (adev->doorbell.num_doorbells == 0)
>> return -EINVAL;
>> @@ -1071,10 +1071,10 @@ static int amdgpu_device_doorbell_init(struct
>> amdgpu_device *adev)
>> adev->doorbell.num_doorbells += 0x400;
>> }
>> - adev->doorbell.ptr = ioremap(adev->doorbell.base,
>> - adev->doorbell.num_doorbells *
>> - sizeof(u32));
>> - if (adev->doorbell.ptr == NULL)
>> + adev->mman.doorbell_aper_base_kaddr =
>> ioremap(adev->doorbell.doorbell_aper_base,
>> + adev->doorbell.num_doorbells *
>> + sizeof(u32));
>> + if (adev->mman.doorbell_aper_base_kaddr == NULL)
>> return -ENOMEM;
>> return 0;
>> @@ -1089,8 +1089,8 @@ static int amdgpu_device_doorbell_init(struct
>> amdgpu_device *adev)
>> */
>> static void amdgpu_device_doorbell_fini(struct amdgpu_device *adev)
>> {
>> - iounmap(adev->doorbell.ptr);
>> - adev->doorbell.ptr = NULL;
>> + iounmap(adev->mman.doorbell_aper_base_kaddr);
>> + adev->mman.doorbell_aper_base_kaddr = NULL;
>> }
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell.h
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell.h
>> index 7199b6b0be81..526b6b4a86dd 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell.h
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell.h
>> @@ -26,9 +26,8 @@
>> */
>> struct amdgpu_doorbell {
>> /* doorbell mmio */
>> - resource_size_t base;
>> - resource_size_t size;
>> - u32 __iomem *ptr;
>> + resource_size_t doorbell_aper_base;
>> + resource_size_t doorbell_aper_size;
>> u32 num_doorbells; /* Number of doorbells
>> actually reserved for amdgpu. */
>> };
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c
>> index 0c546245793b..b79fb369f0f6 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c
>> @@ -126,7 +126,7 @@ static int amdgpu_mes_doorbell_init(struct
>> amdgpu_device *adev)
>> roundup(doorbell_start_offset,
>> amdgpu_mes_doorbell_process_slice(adev));
>> - doorbell_aperture_size = adev->doorbell.size;
>> + doorbell_aperture_size = adev->doorbell.doorbell_aper_size;
>> doorbell_aperture_size =
>> rounddown(doorbell_aperture_size,
>> amdgpu_mes_doorbell_process_slice(adev));
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
>> index 929bc8abac28..967b265dbfa1 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
>> @@ -51,6 +51,7 @@ struct amdgpu_mman {
>> struct ttm_device bdev;
>> bool initialized;
>> void __iomem *vram_aper_base_kaddr;
>> + u32 __iomem *doorbell_aper_base_kaddr;
>> /* buffer handling */
>> const struct amdgpu_buffer_funcs *buffer_funcs;
>> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
>> b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
>> index f202b45c413c..7722da8e7cb4 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
>> @@ -3526,7 +3526,7 @@ static int gfx_v9_0_kiq_init_register(struct
>> amdgpu_ring *ring)
>> */
>> if (check_if_enlarge_doorbell_range(adev))
>> WREG32_SOC15(GC, 0, mmCP_MEC_DOORBELL_RANGE_UPPER,
>> - (adev->doorbell.size - 4));
>> + (adev->doorbell.doorbell_aper_size - 4));
>> else
>> WREG32_SOC15(GC, 0, mmCP_MEC_DOORBELL_RANGE_UPPER,
>> (adev->doorbell_index.userqueue_end * 2) << 2);
>> diff --git a/drivers/gpu/drm/amd/amdgpu/nbio_v2_3.c
>> b/drivers/gpu/drm/amd/amdgpu/nbio_v2_3.c
>> index aa761ff3a5fa..c5fd58d5fef9 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/nbio_v2_3.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/nbio_v2_3.c
>> @@ -173,9 +173,9 @@ static void
>> nbio_v2_3_enable_doorbell_selfring_aperture(struct amdgpu_device *ad
>> DOORBELL_SELFRING_GPA_APER_SIZE, 0);
>> WREG32_SOC15(NBIO, 0,
>> mmBIF_BX_PF_DOORBELL_SELFRING_GPA_APER_BASE_LOW,
>> - lower_32_bits(adev->doorbell.base));
>> + lower_32_bits(adev->doorbell.doorbell_aper_base));
>> WREG32_SOC15(NBIO, 0,
>> mmBIF_BX_PF_DOORBELL_SELFRING_GPA_APER_BASE_HIGH,
>> - upper_32_bits(adev->doorbell.base));
>> + upper_32_bits(adev->doorbell.doorbell_aper_base));
>> }
>> WREG32_SOC15(NBIO, 0,
>> mmBIF_BX_PF_DOORBELL_SELFRING_GPA_APER_CNTL,
>> diff --git a/drivers/gpu/drm/amd/amdgpu/nbio_v4_3.c
>> b/drivers/gpu/drm/amd/amdgpu/nbio_v4_3.c
>> index 15eb3658d70e..9d716ec71f28 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/nbio_v4_3.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/nbio_v4_3.c
>> @@ -169,9 +169,9 @@ static void
>> nbio_v4_3_enable_doorbell_selfring_aperture(struct amdgpu_device *ad
>> DOORBELL_SELFRING_GPA_APER_SIZE, 0);
>> WREG32_SOC15(NBIO, 0,
>> regBIF_BX_PF0_DOORBELL_SELFRING_GPA_APER_BASE_LOW,
>> - lower_32_bits(adev->doorbell.base));
>> + lower_32_bits(adev->doorbell.doorbell_aper_base));
>> WREG32_SOC15(NBIO, 0,
>> regBIF_BX_PF0_DOORBELL_SELFRING_GPA_APER_BASE_HIGH,
>> - upper_32_bits(adev->doorbell.base));
>> + upper_32_bits(adev->doorbell.doorbell_aper_base));
>> }
>> WREG32_SOC15(NBIO, 0,
>> regBIF_BX_PF0_DOORBELL_SELFRING_GPA_APER_CNTL,
>> diff --git a/drivers/gpu/drm/amd/amdgpu/nbio_v6_1.c
>> b/drivers/gpu/drm/amd/amdgpu/nbio_v6_1.c
>> index 37615a77287b..19e175cc7340 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/nbio_v6_1.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/nbio_v6_1.c
>> @@ -121,9 +121,9 @@ static void
>> nbio_v6_1_enable_doorbell_selfring_aperture(struct amdgpu_device *ad
>> REG_SET_FIELD(tmp,
>> BIF_BX_PF0_DOORBELL_SELFRING_GPA_APER_CNTL,
>> DOORBELL_SELFRING_GPA_APER_SIZE, 0);
>> WREG32_SOC15(NBIO, 0,
>> mmBIF_BX_PF0_DOORBELL_SELFRING_GPA_APER_BASE_LOW,
>> - lower_32_bits(adev->doorbell.base));
>> + lower_32_bits(adev->doorbell.doorbell_aper_base));
>> WREG32_SOC15(NBIO, 0,
>> mmBIF_BX_PF0_DOORBELL_SELFRING_GPA_APER_BASE_HIGH,
>> - upper_32_bits(adev->doorbell.base));
>> + upper_32_bits(adev->doorbell.doorbell_aper_base));
>> }
>> WREG32_SOC15(NBIO, 0,
>> mmBIF_BX_PF0_DOORBELL_SELFRING_GPA_APER_CNTL, tmp);
>> diff --git a/drivers/gpu/drm/amd/amdgpu/nbio_v7_2.c
>> b/drivers/gpu/drm/amd/amdgpu/nbio_v7_2.c
>> index 31776b12e4c4..bb2f1857b1e2 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/nbio_v7_2.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/nbio_v7_2.c
>> @@ -175,10 +175,10 @@ static void
>> nbio_v7_2_enable_doorbell_selfring_aperture(struct amdgpu_device *ad
>> WREG32_SOC15(NBIO, 0,
>> regBIF_BX_PF0_DOORBELL_SELFRING_GPA_APER_BASE_LOW,
>> - lower_32_bits(adev->doorbell.base));
>> + lower_32_bits(adev->doorbell.doorbell_aper_base));
>> WREG32_SOC15(NBIO, 0,
>> regBIF_BX_PF0_DOORBELL_SELFRING_GPA_APER_BASE_HIGH,
>> - upper_32_bits(adev->doorbell.base));
>> + upper_32_bits(adev->doorbell.doorbell_aper_base));
>> }
>> WREG32_SOC15(NBIO, 0,
>> regBIF_BX_PF0_DOORBELL_SELFRING_GPA_APER_CNTL,
>> diff --git a/drivers/gpu/drm/amd/amdgpu/nbio_v7_4.c
>> b/drivers/gpu/drm/amd/amdgpu/nbio_v7_4.c
>> index 19455a725939..ee1982bb06aa 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/nbio_v7_4.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/nbio_v7_4.c
>> @@ -223,9 +223,9 @@ static void
>> nbio_v7_4_enable_doorbell_selfring_aperture(struct amdgpu_device *ad
>> REG_SET_FIELD(tmp, DOORBELL_SELFRING_GPA_APER_CNTL,
>> DOORBELL_SELFRING_GPA_APER_SIZE, 0);
>> WREG32_SOC15(NBIO, 0, mmDOORBELL_SELFRING_GPA_APER_BASE_LOW,
>> - lower_32_bits(adev->doorbell.base));
>> + lower_32_bits(adev->doorbell.doorbell_aper_base));
>> WREG32_SOC15(NBIO, 0, mmDOORBELL_SELFRING_GPA_APER_BASE_HIGH,
>> - upper_32_bits(adev->doorbell.base));
>> + upper_32_bits(adev->doorbell.doorbell_aper_base));
>> }
>> WREG32_SOC15(NBIO, 0, mmDOORBELL_SELFRING_GPA_APER_CNTL, tmp);
>> diff --git a/drivers/gpu/drm/amd/amdgpu/nbio_v7_7.c
>> b/drivers/gpu/drm/amd/amdgpu/nbio_v7_7.c
>> index def89379b51a..180d50bcb40f 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/nbio_v7_7.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/nbio_v7_7.c
>> @@ -132,10 +132,10 @@ static void
>> nbio_v7_7_enable_doorbell_selfring_aperture(struct amdgpu_device *ad
>> WREG32_SOC15(NBIO, 0,
>> regBIF_BX_PF0_DOORBELL_SELFRING_GPA_APER_BASE_LOW,
>> - lower_32_bits(adev->doorbell.base));
>> + lower_32_bits(adev->doorbell.doorbell_aper_base));
>> WREG32_SOC15(NBIO, 0,
>> regBIF_BX_PF0_DOORBELL_SELFRING_GPA_APER_BASE_HIGH,
>> - upper_32_bits(adev->doorbell.base));
>> + upper_32_bits(adev->doorbell.doorbell_aper_base));
>> }
>> WREG32_SOC15(NBIO, 0,
>> regBIF_BX_PF0_DOORBELL_SELFRING_GPA_APER_CNTL,
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v2 5/8] drm/amdgpu: accommodate DOMAIN/PL_DOORBELL
2023-02-14 16:15 [PATCH v2 0/8] Re-design doorbell framework for usermode queues Shashank Sharma
` (3 preceding siblings ...)
2023-02-14 16:15 ` [PATCH v2 4/8] drm/amdgpu: rename doorbell variables Shashank Sharma
@ 2023-02-14 16:15 ` Shashank Sharma
2023-02-14 18:31 ` Christian König
2023-02-14 16:15 ` [PATCH v2 6/8] drm/amdgpu: get doorbell memory Shashank Sharma
` (2 subsequent siblings)
7 siblings, 1 reply; 28+ messages in thread
From: Shashank Sharma @ 2023-02-14 16:15 UTC (permalink / raw)
To: amd-gfx; +Cc: alexander.deucher, christian.koenig, Arvind.Yadav,
shashank.sharma
From: Alex Deucher <alexander.deucher@amd.com>
This patch adds changes:
- to accommodate the new GEM domain DOORBELL
- to accommodate the new TTM PL DOORBELL
to manage doorbell allocations as GEM Objects.
V2: Addressed reviwe comments from Christian
- drop the doorbell changes for pinning/unpinning
- drop the doorbell changes for dma-buf map
- drop the doorbell changes for sgt
- no need to handle TTM_PL_FLAG_CONTIGUOUS for doorbell
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
Signed-off-by: Shashank Sharma <shashank.sharma@amd.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 11 ++++++++++-
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 11 ++++++++++-
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h | 1 +
3 files changed, 21 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index b48c9fd60c43..ff9979fecfd2 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -147,6 +147,14 @@ void amdgpu_bo_placement_from_domain(struct amdgpu_bo *abo, u32 domain)
c++;
}
+ if (domain & AMDGPU_GEM_DOMAIN_DOORBELL) {
+ places[c].fpfn = 0;
+ places[c].lpfn = 0;
+ places[c].mem_type = AMDGPU_PL_DOORBELL;
+ places[c].flags = 0;
+ c++;
+ }
+
if (domain & AMDGPU_GEM_DOMAIN_GTT) {
places[c].fpfn = 0;
places[c].lpfn = 0;
@@ -466,7 +474,7 @@ static bool amdgpu_bo_validate_size(struct amdgpu_device *adev,
goto fail;
}
- /* TODO add more domains checks, such as AMDGPU_GEM_DOMAIN_CPU */
+ /* TODO add more domains checks, such as AMDGPU_GEM_DOMAIN_CPU, AMDGPU_GEM_DOMAIN_DOORBELL */
return true;
fail:
@@ -1014,6 +1022,7 @@ void amdgpu_bo_unpin(struct amdgpu_bo *bo)
} else if (bo->tbo.resource->mem_type == TTM_PL_TT) {
atomic64_sub(amdgpu_bo_size(bo), &adev->gart_pin_size);
}
+
}
static const char *amdgpu_vram_names[] = {
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 0e8f580769ab..e9dc24191fc8 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -128,6 +128,7 @@ static void amdgpu_evict_flags(struct ttm_buffer_object *bo,
case AMDGPU_PL_GDS:
case AMDGPU_PL_GWS:
case AMDGPU_PL_OA:
+ case AMDGPU_PL_DOORBELL:
placement->num_placement = 0;
placement->num_busy_placement = 0;
return;
@@ -500,9 +501,11 @@ static int amdgpu_bo_move(struct ttm_buffer_object *bo, bool evict,
if (old_mem->mem_type == AMDGPU_PL_GDS ||
old_mem->mem_type == AMDGPU_PL_GWS ||
old_mem->mem_type == AMDGPU_PL_OA ||
+ old_mem->mem_type == AMDGPU_PL_DOORBELL ||
new_mem->mem_type == AMDGPU_PL_GDS ||
new_mem->mem_type == AMDGPU_PL_GWS ||
- new_mem->mem_type == AMDGPU_PL_OA) {
+ new_mem->mem_type == AMDGPU_PL_OA ||
+ new_mem->mem_type == AMDGPU_PL_DOORBELL) {
/* Nothing to save here */
ttm_bo_move_null(bo, new_mem);
goto out;
@@ -586,6 +589,11 @@ static int amdgpu_ttm_io_mem_reserve(struct ttm_device *bdev,
mem->bus.offset += adev->gmc.vram_aper_base;
mem->bus.is_iomem = true;
break;
+ case AMDGPU_PL_DOORBELL:
+ mem->bus.offset = mem->start << PAGE_SHIFT;
+ mem->bus.offset += adev->doorbell.doorbell_aper_base;
+ mem->bus.is_iomem = true;
+ break;
default:
return -EINVAL;
}
@@ -1267,6 +1275,7 @@ uint64_t amdgpu_ttm_tt_pde_flags(struct ttm_tt *ttm, struct ttm_resource *mem)
flags |= AMDGPU_PTE_VALID;
if (mem && (mem->mem_type == TTM_PL_TT ||
+ mem->mem_type == AMDGPU_PL_DOORBELL ||
mem->mem_type == AMDGPU_PL_PREEMPT)) {
flags |= AMDGPU_PTE_SYSTEM;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
index 967b265dbfa1..9cf5d8419965 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
@@ -33,6 +33,7 @@
#define AMDGPU_PL_GWS (TTM_PL_PRIV + 1)
#define AMDGPU_PL_OA (TTM_PL_PRIV + 2)
#define AMDGPU_PL_PREEMPT (TTM_PL_PRIV + 3)
+#define AMDGPU_PL_DOORBELL (TTM_PL_PRIV + 4)
#define AMDGPU_GTT_MAX_TRANSFER_SIZE 512
#define AMDGPU_GTT_NUM_TRANSFER_WINDOWS 2
--
2.34.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH v2 5/8] drm/amdgpu: accommodate DOMAIN/PL_DOORBELL
2023-02-14 16:15 ` [PATCH v2 5/8] drm/amdgpu: accommodate DOMAIN/PL_DOORBELL Shashank Sharma
@ 2023-02-14 18:31 ` Christian König
2023-02-14 19:24 ` Shashank Sharma
0 siblings, 1 reply; 28+ messages in thread
From: Christian König @ 2023-02-14 18:31 UTC (permalink / raw)
To: Shashank Sharma, amd-gfx; +Cc: alexander.deucher, Arvind.Yadav
Am 14.02.23 um 17:15 schrieb Shashank Sharma:
> From: Alex Deucher <alexander.deucher@amd.com>
>
> This patch adds changes:
> - to accommodate the new GEM domain DOORBELL
> - to accommodate the new TTM PL DOORBELL
>
> to manage doorbell allocations as GEM Objects.
>
> V2: Addressed reviwe comments from Christian
> - drop the doorbell changes for pinning/unpinning
> - drop the doorbell changes for dma-buf map
> - drop the doorbell changes for sgt
> - no need to handle TTM_PL_FLAG_CONTIGUOUS for doorbell
>
> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> Signed-off-by: Shashank Sharma <shashank.sharma@amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 11 ++++++++++-
> drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 11 ++++++++++-
> drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h | 1 +
> 3 files changed, 21 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> index b48c9fd60c43..ff9979fecfd2 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> @@ -147,6 +147,14 @@ void amdgpu_bo_placement_from_domain(struct amdgpu_bo *abo, u32 domain)
> c++;
> }
>
> + if (domain & AMDGPU_GEM_DOMAIN_DOORBELL) {
> + places[c].fpfn = 0;
> + places[c].lpfn = 0;
> + places[c].mem_type = AMDGPU_PL_DOORBELL;
> + places[c].flags = 0;
> + c++;
> + }
> +
Mhm, do we need to increase AMDGPU_BO_MAX_PLACEMENTS?
I think the answer is *no* since mixing DOORBELL with CPU, GTT or VRAM
placement doesn't make sense, but do we enforce that somewhere?
> if (domain & AMDGPU_GEM_DOMAIN_GTT) {
> places[c].fpfn = 0;
> places[c].lpfn = 0;
> @@ -466,7 +474,7 @@ static bool amdgpu_bo_validate_size(struct amdgpu_device *adev,
> goto fail;
> }
>
> - /* TODO add more domains checks, such as AMDGPU_GEM_DOMAIN_CPU */
> + /* TODO add more domains checks, such as AMDGPU_GEM_DOMAIN_CPU, AMDGPU_GEM_DOMAIN_DOORBELL */
Should we enforce that user space can only allocate 1 page doorbells?
> return true;
>
> fail:
> @@ -1014,6 +1022,7 @@ void amdgpu_bo_unpin(struct amdgpu_bo *bo)
> } else if (bo->tbo.resource->mem_type == TTM_PL_TT) {
> atomic64_sub(amdgpu_bo_size(bo), &adev->gart_pin_size);
> }
> +
Unrelated change.
Regards,
Christian.
> }
>
> static const char *amdgpu_vram_names[] = {
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> index 0e8f580769ab..e9dc24191fc8 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> @@ -128,6 +128,7 @@ static void amdgpu_evict_flags(struct ttm_buffer_object *bo,
> case AMDGPU_PL_GDS:
> case AMDGPU_PL_GWS:
> case AMDGPU_PL_OA:
> + case AMDGPU_PL_DOORBELL:
> placement->num_placement = 0;
> placement->num_busy_placement = 0;
> return;
> @@ -500,9 +501,11 @@ static int amdgpu_bo_move(struct ttm_buffer_object *bo, bool evict,
> if (old_mem->mem_type == AMDGPU_PL_GDS ||
> old_mem->mem_type == AMDGPU_PL_GWS ||
> old_mem->mem_type == AMDGPU_PL_OA ||
> + old_mem->mem_type == AMDGPU_PL_DOORBELL ||
> new_mem->mem_type == AMDGPU_PL_GDS ||
> new_mem->mem_type == AMDGPU_PL_GWS ||
> - new_mem->mem_type == AMDGPU_PL_OA) {
> + new_mem->mem_type == AMDGPU_PL_OA ||
> + new_mem->mem_type == AMDGPU_PL_DOORBELL) {
> /* Nothing to save here */
> ttm_bo_move_null(bo, new_mem);
> goto out;
> @@ -586,6 +589,11 @@ static int amdgpu_ttm_io_mem_reserve(struct ttm_device *bdev,
> mem->bus.offset += adev->gmc.vram_aper_base;
> mem->bus.is_iomem = true;
> break;
> + case AMDGPU_PL_DOORBELL:
> + mem->bus.offset = mem->start << PAGE_SHIFT;
> + mem->bus.offset += adev->doorbell.doorbell_aper_base;
> + mem->bus.is_iomem = true;
> + break;
> default:
> return -EINVAL;
> }
> @@ -1267,6 +1275,7 @@ uint64_t amdgpu_ttm_tt_pde_flags(struct ttm_tt *ttm, struct ttm_resource *mem)
> flags |= AMDGPU_PTE_VALID;
>
> if (mem && (mem->mem_type == TTM_PL_TT ||
> + mem->mem_type == AMDGPU_PL_DOORBELL ||
> mem->mem_type == AMDGPU_PL_PREEMPT)) {
> flags |= AMDGPU_PTE_SYSTEM;
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
> index 967b265dbfa1..9cf5d8419965 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
> @@ -33,6 +33,7 @@
> #define AMDGPU_PL_GWS (TTM_PL_PRIV + 1)
> #define AMDGPU_PL_OA (TTM_PL_PRIV + 2)
> #define AMDGPU_PL_PREEMPT (TTM_PL_PRIV + 3)
> +#define AMDGPU_PL_DOORBELL (TTM_PL_PRIV + 4)
>
> #define AMDGPU_GTT_MAX_TRANSFER_SIZE 512
> #define AMDGPU_GTT_NUM_TRANSFER_WINDOWS 2
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v2 5/8] drm/amdgpu: accommodate DOMAIN/PL_DOORBELL
2023-02-14 18:31 ` Christian König
@ 2023-02-14 19:24 ` Shashank Sharma
2023-02-15 6:17 ` Christian König
0 siblings, 1 reply; 28+ messages in thread
From: Shashank Sharma @ 2023-02-14 19:24 UTC (permalink / raw)
To: Christian König, amd-gfx; +Cc: alexander.deucher, Arvind.Yadav
On 14/02/2023 19:31, Christian König wrote:
> Am 14.02.23 um 17:15 schrieb Shashank Sharma:
>> From: Alex Deucher <alexander.deucher@amd.com>
>>
>> This patch adds changes:
>> - to accommodate the new GEM domain DOORBELL
>> - to accommodate the new TTM PL DOORBELL
>>
>> to manage doorbell allocations as GEM Objects.
>>
>> V2: Addressed reviwe comments from Christian
>> - drop the doorbell changes for pinning/unpinning
>> - drop the doorbell changes for dma-buf map
>> - drop the doorbell changes for sgt
>> - no need to handle TTM_PL_FLAG_CONTIGUOUS for doorbell
>>
>> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
>> Signed-off-by: Shashank Sharma <shashank.sharma@amd.com>
>> ---
>> drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 11 ++++++++++-
>> drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 11 ++++++++++-
>> drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h | 1 +
>> 3 files changed, 21 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>> index b48c9fd60c43..ff9979fecfd2 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>> @@ -147,6 +147,14 @@ void amdgpu_bo_placement_from_domain(struct
>> amdgpu_bo *abo, u32 domain)
>> c++;
>> }
>> + if (domain & AMDGPU_GEM_DOMAIN_DOORBELL) {
>> + places[c].fpfn = 0;
>> + places[c].lpfn = 0;
>> + places[c].mem_type = AMDGPU_PL_DOORBELL;
>> + places[c].flags = 0;
>> + c++;
>> + }
>> +
>
> Mhm, do we need to increase AMDGPU_BO_MAX_PLACEMENTS?
>
> I think the answer is *no* since mixing DOORBELL with CPU, GTT or VRAM
> placement doesn't make sense, but do we enforce that somewhere?
I am not sure why do we need that ?
>
>> if (domain & AMDGPU_GEM_DOMAIN_GTT) {
>> places[c].fpfn = 0;
>> places[c].lpfn = 0;
>> @@ -466,7 +474,7 @@ static bool amdgpu_bo_validate_size(struct
>> amdgpu_device *adev,
>> goto fail;
>> }
>> - /* TODO add more domains checks, such as AMDGPU_GEM_DOMAIN_CPU */
>> + /* TODO add more domains checks, such as AMDGPU_GEM_DOMAIN_CPU,
>> AMDGPU_GEM_DOMAIN_DOORBELL */
>
> Should we enforce that user space can only allocate 1 page doorbells?
>
Should we add a per-PID basis check ?
- Shashank
>> return true;
>> fail:
>> @@ -1014,6 +1022,7 @@ void amdgpu_bo_unpin(struct amdgpu_bo *bo)
>> } else if (bo->tbo.resource->mem_type == TTM_PL_TT) {
>> atomic64_sub(amdgpu_bo_size(bo), &adev->gart_pin_size);
>> }
>> +
>
> Unrelated change.
Noted
- Shashank
>
> Regards,
> Christian.
>
>> }
>> static const char *amdgpu_vram_names[] = {
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>> index 0e8f580769ab..e9dc24191fc8 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>> @@ -128,6 +128,7 @@ static void amdgpu_evict_flags(struct
>> ttm_buffer_object *bo,
>> case AMDGPU_PL_GDS:
>> case AMDGPU_PL_GWS:
>> case AMDGPU_PL_OA:
>> + case AMDGPU_PL_DOORBELL:
>> placement->num_placement = 0;
>> placement->num_busy_placement = 0;
>> return;
>> @@ -500,9 +501,11 @@ static int amdgpu_bo_move(struct
>> ttm_buffer_object *bo, bool evict,
>> if (old_mem->mem_type == AMDGPU_PL_GDS ||
>> old_mem->mem_type == AMDGPU_PL_GWS ||
>> old_mem->mem_type == AMDGPU_PL_OA ||
>> + old_mem->mem_type == AMDGPU_PL_DOORBELL ||
>> new_mem->mem_type == AMDGPU_PL_GDS ||
>> new_mem->mem_type == AMDGPU_PL_GWS ||
>> - new_mem->mem_type == AMDGPU_PL_OA) {
>> + new_mem->mem_type == AMDGPU_PL_OA ||
>> + new_mem->mem_type == AMDGPU_PL_DOORBELL) {
>> /* Nothing to save here */
>> ttm_bo_move_null(bo, new_mem);
>> goto out;
>> @@ -586,6 +589,11 @@ static int amdgpu_ttm_io_mem_reserve(struct
>> ttm_device *bdev,
>> mem->bus.offset += adev->gmc.vram_aper_base;
>> mem->bus.is_iomem = true;
>> break;
>> + case AMDGPU_PL_DOORBELL:
>> + mem->bus.offset = mem->start << PAGE_SHIFT;
>> + mem->bus.offset += adev->doorbell.doorbell_aper_base;
>> + mem->bus.is_iomem = true;
>> + break;
>> default:
>> return -EINVAL;
>> }
>> @@ -1267,6 +1275,7 @@ uint64_t amdgpu_ttm_tt_pde_flags(struct ttm_tt
>> *ttm, struct ttm_resource *mem)
>> flags |= AMDGPU_PTE_VALID;
>> if (mem && (mem->mem_type == TTM_PL_TT ||
>> + mem->mem_type == AMDGPU_PL_DOORBELL ||
>> mem->mem_type == AMDGPU_PL_PREEMPT)) {
>> flags |= AMDGPU_PTE_SYSTEM;
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
>> index 967b265dbfa1..9cf5d8419965 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
>> @@ -33,6 +33,7 @@
>> #define AMDGPU_PL_GWS (TTM_PL_PRIV + 1)
>> #define AMDGPU_PL_OA (TTM_PL_PRIV + 2)
>> #define AMDGPU_PL_PREEMPT (TTM_PL_PRIV + 3)
>> +#define AMDGPU_PL_DOORBELL (TTM_PL_PRIV + 4)
>> #define AMDGPU_GTT_MAX_TRANSFER_SIZE 512
>> #define AMDGPU_GTT_NUM_TRANSFER_WINDOWS 2
>
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v2 5/8] drm/amdgpu: accommodate DOMAIN/PL_DOORBELL
2023-02-14 19:24 ` Shashank Sharma
@ 2023-02-15 6:17 ` Christian König
2023-02-15 13:32 ` Shashank Sharma
0 siblings, 1 reply; 28+ messages in thread
From: Christian König @ 2023-02-15 6:17 UTC (permalink / raw)
To: Shashank Sharma, amd-gfx; +Cc: alexander.deucher, Arvind.Yadav
Am 14.02.23 um 20:24 schrieb Shashank Sharma:
>
> On 14/02/2023 19:31, Christian König wrote:
>> Am 14.02.23 um 17:15 schrieb Shashank Sharma:
>>> From: Alex Deucher <alexander.deucher@amd.com>
>>>
>>> This patch adds changes:
>>> - to accommodate the new GEM domain DOORBELL
>>> - to accommodate the new TTM PL DOORBELL
>>>
>>> to manage doorbell allocations as GEM Objects.
>>>
>>> V2: Addressed reviwe comments from Christian
>>> - drop the doorbell changes for pinning/unpinning
>>> - drop the doorbell changes for dma-buf map
>>> - drop the doorbell changes for sgt
>>> - no need to handle TTM_PL_FLAG_CONTIGUOUS for doorbell
>>>
>>> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
>>> Signed-off-by: Shashank Sharma <shashank.sharma@amd.com>
>>> ---
>>> drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 11 ++++++++++-
>>> drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 11 ++++++++++-
>>> drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h | 1 +
>>> 3 files changed, 21 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>>> index b48c9fd60c43..ff9979fecfd2 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>>> @@ -147,6 +147,14 @@ void amdgpu_bo_placement_from_domain(struct
>>> amdgpu_bo *abo, u32 domain)
>>> c++;
>>> }
>>> + if (domain & AMDGPU_GEM_DOMAIN_DOORBELL) {
>>> + places[c].fpfn = 0;
>>> + places[c].lpfn = 0;
>>> + places[c].mem_type = AMDGPU_PL_DOORBELL;
>>> + places[c].flags = 0;
>>> + c++;
>>> + }
>>> +
>>
>> Mhm, do we need to increase AMDGPU_BO_MAX_PLACEMENTS?
>>
>> I think the answer is *no* since mixing DOORBELL with CPU, GTT or
>> VRAM placement doesn't make sense, but do we enforce that somewhere?
> I am not sure why do we need that ?
Userspace could otherwise specify DOORBEEL|CPU|GTT|VRAM as placement
which would overrun the array and be illegal.
>>
>>> if (domain & AMDGPU_GEM_DOMAIN_GTT) {
>>> places[c].fpfn = 0;
>>> places[c].lpfn = 0;
>>> @@ -466,7 +474,7 @@ static bool amdgpu_bo_validate_size(struct
>>> amdgpu_device *adev,
>>> goto fail;
>>> }
>>> - /* TODO add more domains checks, such as
>>> AMDGPU_GEM_DOMAIN_CPU */
>>> + /* TODO add more domains checks, such as
>>> AMDGPU_GEM_DOMAIN_CPU, AMDGPU_GEM_DOMAIN_DOORBELL */
>>
>> Should we enforce that user space can only allocate 1 page doorbells?
>>
> Should we add a per-PID basis check ?
No, just a check that the allocation size of the doorbell BOs is just 1
page.
Christian.
>
> - Shashank
>
>>> return true;
>>> fail:
>>> @@ -1014,6 +1022,7 @@ void amdgpu_bo_unpin(struct amdgpu_bo *bo)
>>> } else if (bo->tbo.resource->mem_type == TTM_PL_TT) {
>>> atomic64_sub(amdgpu_bo_size(bo), &adev->gart_pin_size);
>>> }
>>> +
>>
>> Unrelated change.
>
> Noted
>
> - Shashank
>
>>
>> Regards,
>> Christian.
>>
>>> }
>>> static const char *amdgpu_vram_names[] = {
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>>> index 0e8f580769ab..e9dc24191fc8 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>>> @@ -128,6 +128,7 @@ static void amdgpu_evict_flags(struct
>>> ttm_buffer_object *bo,
>>> case AMDGPU_PL_GDS:
>>> case AMDGPU_PL_GWS:
>>> case AMDGPU_PL_OA:
>>> + case AMDGPU_PL_DOORBELL:
>>> placement->num_placement = 0;
>>> placement->num_busy_placement = 0;
>>> return;
>>> @@ -500,9 +501,11 @@ static int amdgpu_bo_move(struct
>>> ttm_buffer_object *bo, bool evict,
>>> if (old_mem->mem_type == AMDGPU_PL_GDS ||
>>> old_mem->mem_type == AMDGPU_PL_GWS ||
>>> old_mem->mem_type == AMDGPU_PL_OA ||
>>> + old_mem->mem_type == AMDGPU_PL_DOORBELL ||
>>> new_mem->mem_type == AMDGPU_PL_GDS ||
>>> new_mem->mem_type == AMDGPU_PL_GWS ||
>>> - new_mem->mem_type == AMDGPU_PL_OA) {
>>> + new_mem->mem_type == AMDGPU_PL_OA ||
>>> + new_mem->mem_type == AMDGPU_PL_DOORBELL) {
>>> /* Nothing to save here */
>>> ttm_bo_move_null(bo, new_mem);
>>> goto out;
>>> @@ -586,6 +589,11 @@ static int amdgpu_ttm_io_mem_reserve(struct
>>> ttm_device *bdev,
>>> mem->bus.offset += adev->gmc.vram_aper_base;
>>> mem->bus.is_iomem = true;
>>> break;
>>> + case AMDGPU_PL_DOORBELL:
>>> + mem->bus.offset = mem->start << PAGE_SHIFT;
>>> + mem->bus.offset += adev->doorbell.doorbell_aper_base;
>>> + mem->bus.is_iomem = true;
>>> + break;
>>> default:
>>> return -EINVAL;
>>> }
>>> @@ -1267,6 +1275,7 @@ uint64_t amdgpu_ttm_tt_pde_flags(struct ttm_tt
>>> *ttm, struct ttm_resource *mem)
>>> flags |= AMDGPU_PTE_VALID;
>>> if (mem && (mem->mem_type == TTM_PL_TT ||
>>> + mem->mem_type == AMDGPU_PL_DOORBELL ||
>>> mem->mem_type == AMDGPU_PL_PREEMPT)) {
>>> flags |= AMDGPU_PTE_SYSTEM;
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
>>> index 967b265dbfa1..9cf5d8419965 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
>>> @@ -33,6 +33,7 @@
>>> #define AMDGPU_PL_GWS (TTM_PL_PRIV + 1)
>>> #define AMDGPU_PL_OA (TTM_PL_PRIV + 2)
>>> #define AMDGPU_PL_PREEMPT (TTM_PL_PRIV + 3)
>>> +#define AMDGPU_PL_DOORBELL (TTM_PL_PRIV + 4)
>>> #define AMDGPU_GTT_MAX_TRANSFER_SIZE 512
>>> #define AMDGPU_GTT_NUM_TRANSFER_WINDOWS 2
>>
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v2 5/8] drm/amdgpu: accommodate DOMAIN/PL_DOORBELL
2023-02-15 6:17 ` Christian König
@ 2023-02-15 13:32 ` Shashank Sharma
2023-02-15 13:36 ` Christian König
0 siblings, 1 reply; 28+ messages in thread
From: Shashank Sharma @ 2023-02-15 13:32 UTC (permalink / raw)
To: Christian König, amd-gfx; +Cc: alexander.deucher, Arvind.Yadav
On 15/02/2023 07:17, Christian König wrote:
> Am 14.02.23 um 20:24 schrieb Shashank Sharma:
>>
>> On 14/02/2023 19:31, Christian König wrote:
>>> Am 14.02.23 um 17:15 schrieb Shashank Sharma:
>>>> From: Alex Deucher <alexander.deucher@amd.com>
>>>>
>>>> This patch adds changes:
>>>> - to accommodate the new GEM domain DOORBELL
>>>> - to accommodate the new TTM PL DOORBELL
>>>>
>>>> to manage doorbell allocations as GEM Objects.
>>>>
>>>> V2: Addressed reviwe comments from Christian
>>>> - drop the doorbell changes for pinning/unpinning
>>>> - drop the doorbell changes for dma-buf map
>>>> - drop the doorbell changes for sgt
>>>> - no need to handle TTM_PL_FLAG_CONTIGUOUS for doorbell
>>>>
>>>> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
>>>> Signed-off-by: Shashank Sharma <shashank.sharma@amd.com>
>>>> ---
>>>> drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 11 ++++++++++-
>>>> drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 11 ++++++++++-
>>>> drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h | 1 +
>>>> 3 files changed, 21 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>>>> index b48c9fd60c43..ff9979fecfd2 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>>>> @@ -147,6 +147,14 @@ void amdgpu_bo_placement_from_domain(struct
>>>> amdgpu_bo *abo, u32 domain)
>>>> c++;
>>>> }
>>>> + if (domain & AMDGPU_GEM_DOMAIN_DOORBELL) {
>>>> + places[c].fpfn = 0;
>>>> + places[c].lpfn = 0;
>>>> + places[c].mem_type = AMDGPU_PL_DOORBELL;
>>>> + places[c].flags = 0;
>>>> + c++;
>>>> + }
>>>> +
>>>
>>> Mhm, do we need to increase AMDGPU_BO_MAX_PLACEMENTS?
>>>
>>> I think the answer is *no* since mixing DOORBELL with CPU, GTT or
>>> VRAM placement doesn't make sense, but do we enforce that somewhere?
>> I am not sure why do we need that ?
>
> Userspace could otherwise specify DOORBEEL|CPU|GTT|VRAM as placement
> which would overrun the array and be illegal.
Now when I understand this, how can we enforce this ? A size check that
blocks places to go over a certain value, which is fixed for boorbell ?
>
>>>
>>>> if (domain & AMDGPU_GEM_DOMAIN_GTT) {
>>>> places[c].fpfn = 0;
>>>> places[c].lpfn = 0;
>>>> @@ -466,7 +474,7 @@ static bool amdgpu_bo_validate_size(struct
>>>> amdgpu_device *adev,
>>>> goto fail;
>>>> }
>>>> - /* TODO add more domains checks, such as
>>>> AMDGPU_GEM_DOMAIN_CPU */
>>>> + /* TODO add more domains checks, such as
>>>> AMDGPU_GEM_DOMAIN_CPU, AMDGPU_GEM_DOMAIN_DOORBELL */
>>>
>>> Should we enforce that user space can only allocate 1 page doorbells?
>>>
>> Should we add a per-PID basis check ?
>
> No, just a check that the allocation size of the doorbell BOs is just
> 1 page.
Noted
- Shashank
>
> Christian.
>
>>
>> - Shashank
>>
>>>> return true;
>>>> fail:
>>>> @@ -1014,6 +1022,7 @@ void amdgpu_bo_unpin(struct amdgpu_bo *bo)
>>>> } else if (bo->tbo.resource->mem_type == TTM_PL_TT) {
>>>> atomic64_sub(amdgpu_bo_size(bo), &adev->gart_pin_size);
>>>> }
>>>> +
>>>
>>> Unrelated change.
>>
>> Noted
>>
>> - Shashank
>>
>>>
>>> Regards,
>>> Christian.
>>>
>>>> }
>>>> static const char *amdgpu_vram_names[] = {
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>>>> index 0e8f580769ab..e9dc24191fc8 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>>>> @@ -128,6 +128,7 @@ static void amdgpu_evict_flags(struct
>>>> ttm_buffer_object *bo,
>>>> case AMDGPU_PL_GDS:
>>>> case AMDGPU_PL_GWS:
>>>> case AMDGPU_PL_OA:
>>>> + case AMDGPU_PL_DOORBELL:
>>>> placement->num_placement = 0;
>>>> placement->num_busy_placement = 0;
>>>> return;
>>>> @@ -500,9 +501,11 @@ static int amdgpu_bo_move(struct
>>>> ttm_buffer_object *bo, bool evict,
>>>> if (old_mem->mem_type == AMDGPU_PL_GDS ||
>>>> old_mem->mem_type == AMDGPU_PL_GWS ||
>>>> old_mem->mem_type == AMDGPU_PL_OA ||
>>>> + old_mem->mem_type == AMDGPU_PL_DOORBELL ||
>>>> new_mem->mem_type == AMDGPU_PL_GDS ||
>>>> new_mem->mem_type == AMDGPU_PL_GWS ||
>>>> - new_mem->mem_type == AMDGPU_PL_OA) {
>>>> + new_mem->mem_type == AMDGPU_PL_OA ||
>>>> + new_mem->mem_type == AMDGPU_PL_DOORBELL) {
>>>> /* Nothing to save here */
>>>> ttm_bo_move_null(bo, new_mem);
>>>> goto out;
>>>> @@ -586,6 +589,11 @@ static int amdgpu_ttm_io_mem_reserve(struct
>>>> ttm_device *bdev,
>>>> mem->bus.offset += adev->gmc.vram_aper_base;
>>>> mem->bus.is_iomem = true;
>>>> break;
>>>> + case AMDGPU_PL_DOORBELL:
>>>> + mem->bus.offset = mem->start << PAGE_SHIFT;
>>>> + mem->bus.offset += adev->doorbell.doorbell_aper_base;
>>>> + mem->bus.is_iomem = true;
>>>> + break;
>>>> default:
>>>> return -EINVAL;
>>>> }
>>>> @@ -1267,6 +1275,7 @@ uint64_t amdgpu_ttm_tt_pde_flags(struct
>>>> ttm_tt *ttm, struct ttm_resource *mem)
>>>> flags |= AMDGPU_PTE_VALID;
>>>> if (mem && (mem->mem_type == TTM_PL_TT ||
>>>> + mem->mem_type == AMDGPU_PL_DOORBELL ||
>>>> mem->mem_type == AMDGPU_PL_PREEMPT)) {
>>>> flags |= AMDGPU_PTE_SYSTEM;
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
>>>> index 967b265dbfa1..9cf5d8419965 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
>>>> @@ -33,6 +33,7 @@
>>>> #define AMDGPU_PL_GWS (TTM_PL_PRIV + 1)
>>>> #define AMDGPU_PL_OA (TTM_PL_PRIV + 2)
>>>> #define AMDGPU_PL_PREEMPT (TTM_PL_PRIV + 3)
>>>> +#define AMDGPU_PL_DOORBELL (TTM_PL_PRIV + 4)
>>>> #define AMDGPU_GTT_MAX_TRANSFER_SIZE 512
>>>> #define AMDGPU_GTT_NUM_TRANSFER_WINDOWS 2
>>>
>
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v2 5/8] drm/amdgpu: accommodate DOMAIN/PL_DOORBELL
2023-02-15 13:32 ` Shashank Sharma
@ 2023-02-15 13:36 ` Christian König
2023-02-15 13:45 ` Shashank Sharma
0 siblings, 1 reply; 28+ messages in thread
From: Christian König @ 2023-02-15 13:36 UTC (permalink / raw)
To: Shashank Sharma, amd-gfx; +Cc: alexander.deucher, Arvind.Yadav
Am 15.02.23 um 14:32 schrieb Shashank Sharma:
>
> On 15/02/2023 07:17, Christian König wrote:
>> Am 14.02.23 um 20:24 schrieb Shashank Sharma:
>>>
>>> On 14/02/2023 19:31, Christian König wrote:
>>>> Am 14.02.23 um 17:15 schrieb Shashank Sharma:
>>>>> From: Alex Deucher <alexander.deucher@amd.com>
>>>>>
>>>>> This patch adds changes:
>>>>> - to accommodate the new GEM domain DOORBELL
>>>>> - to accommodate the new TTM PL DOORBELL
>>>>>
>>>>> to manage doorbell allocations as GEM Objects.
>>>>>
>>>>> V2: Addressed reviwe comments from Christian
>>>>> - drop the doorbell changes for pinning/unpinning
>>>>> - drop the doorbell changes for dma-buf map
>>>>> - drop the doorbell changes for sgt
>>>>> - no need to handle TTM_PL_FLAG_CONTIGUOUS for doorbell
>>>>>
>>>>> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
>>>>> Signed-off-by: Shashank Sharma <shashank.sharma@amd.com>
>>>>> ---
>>>>> drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 11 ++++++++++-
>>>>> drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 11 ++++++++++-
>>>>> drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h | 1 +
>>>>> 3 files changed, 21 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>>>>> index b48c9fd60c43..ff9979fecfd2 100644
>>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>>>>> @@ -147,6 +147,14 @@ void amdgpu_bo_placement_from_domain(struct
>>>>> amdgpu_bo *abo, u32 domain)
>>>>> c++;
>>>>> }
>>>>> + if (domain & AMDGPU_GEM_DOMAIN_DOORBELL) {
>>>>> + places[c].fpfn = 0;
>>>>> + places[c].lpfn = 0;
>>>>> + places[c].mem_type = AMDGPU_PL_DOORBELL;
>>>>> + places[c].flags = 0;
>>>>> + c++;
>>>>> + }
>>>>> +
>>>>
>>>> Mhm, do we need to increase AMDGPU_BO_MAX_PLACEMENTS?
>>>>
>>>> I think the answer is *no* since mixing DOORBELL with CPU, GTT or
>>>> VRAM placement doesn't make sense, but do we enforce that somewhere?
>>> I am not sure why do we need that ?
>>
>> Userspace could otherwise specify DOORBEEL|CPU|GTT|VRAM as placement
>> which would overrun the array and be illegal.
>
> Now when I understand this, how can we enforce this ? A size check
> that blocks places to go over a certain value, which is fixed for
> boorbell ?
In amdgpu_bo_create() we should have a check that if GDS, GWS, OA and
DOORBELL are specified they are specified all alone. In other words
those domains can't be mixed with anything else.
Christian.
>
>>
>>>>
>>>>> if (domain & AMDGPU_GEM_DOMAIN_GTT) {
>>>>> places[c].fpfn = 0;
>>>>> places[c].lpfn = 0;
>>>>> @@ -466,7 +474,7 @@ static bool amdgpu_bo_validate_size(struct
>>>>> amdgpu_device *adev,
>>>>> goto fail;
>>>>> }
>>>>> - /* TODO add more domains checks, such as
>>>>> AMDGPU_GEM_DOMAIN_CPU */
>>>>> + /* TODO add more domains checks, such as
>>>>> AMDGPU_GEM_DOMAIN_CPU, AMDGPU_GEM_DOMAIN_DOORBELL */
>>>>
>>>> Should we enforce that user space can only allocate 1 page doorbells?
>>>>
>>> Should we add a per-PID basis check ?
>>
>> No, just a check that the allocation size of the doorbell BOs is just
>> 1 page.
>
> Noted
>
> - Shashank
>
>>
>> Christian.
>>
>>>
>>> - Shashank
>>>
>>>>> return true;
>>>>> fail:
>>>>> @@ -1014,6 +1022,7 @@ void amdgpu_bo_unpin(struct amdgpu_bo *bo)
>>>>> } else if (bo->tbo.resource->mem_type == TTM_PL_TT) {
>>>>> atomic64_sub(amdgpu_bo_size(bo), &adev->gart_pin_size);
>>>>> }
>>>>> +
>>>>
>>>> Unrelated change.
>>>
>>> Noted
>>>
>>> - Shashank
>>>
>>>>
>>>> Regards,
>>>> Christian.
>>>>
>>>>> }
>>>>> static const char *amdgpu_vram_names[] = {
>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>>>>> index 0e8f580769ab..e9dc24191fc8 100644
>>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>>>>> @@ -128,6 +128,7 @@ static void amdgpu_evict_flags(struct
>>>>> ttm_buffer_object *bo,
>>>>> case AMDGPU_PL_GDS:
>>>>> case AMDGPU_PL_GWS:
>>>>> case AMDGPU_PL_OA:
>>>>> + case AMDGPU_PL_DOORBELL:
>>>>> placement->num_placement = 0;
>>>>> placement->num_busy_placement = 0;
>>>>> return;
>>>>> @@ -500,9 +501,11 @@ static int amdgpu_bo_move(struct
>>>>> ttm_buffer_object *bo, bool evict,
>>>>> if (old_mem->mem_type == AMDGPU_PL_GDS ||
>>>>> old_mem->mem_type == AMDGPU_PL_GWS ||
>>>>> old_mem->mem_type == AMDGPU_PL_OA ||
>>>>> + old_mem->mem_type == AMDGPU_PL_DOORBELL ||
>>>>> new_mem->mem_type == AMDGPU_PL_GDS ||
>>>>> new_mem->mem_type == AMDGPU_PL_GWS ||
>>>>> - new_mem->mem_type == AMDGPU_PL_OA) {
>>>>> + new_mem->mem_type == AMDGPU_PL_OA ||
>>>>> + new_mem->mem_type == AMDGPU_PL_DOORBELL) {
>>>>> /* Nothing to save here */
>>>>> ttm_bo_move_null(bo, new_mem);
>>>>> goto out;
>>>>> @@ -586,6 +589,11 @@ static int amdgpu_ttm_io_mem_reserve(struct
>>>>> ttm_device *bdev,
>>>>> mem->bus.offset += adev->gmc.vram_aper_base;
>>>>> mem->bus.is_iomem = true;
>>>>> break;
>>>>> + case AMDGPU_PL_DOORBELL:
>>>>> + mem->bus.offset = mem->start << PAGE_SHIFT;
>>>>> + mem->bus.offset += adev->doorbell.doorbell_aper_base;
>>>>> + mem->bus.is_iomem = true;
>>>>> + break;
>>>>> default:
>>>>> return -EINVAL;
>>>>> }
>>>>> @@ -1267,6 +1275,7 @@ uint64_t amdgpu_ttm_tt_pde_flags(struct
>>>>> ttm_tt *ttm, struct ttm_resource *mem)
>>>>> flags |= AMDGPU_PTE_VALID;
>>>>> if (mem && (mem->mem_type == TTM_PL_TT ||
>>>>> + mem->mem_type == AMDGPU_PL_DOORBELL ||
>>>>> mem->mem_type == AMDGPU_PL_PREEMPT)) {
>>>>> flags |= AMDGPU_PTE_SYSTEM;
>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
>>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
>>>>> index 967b265dbfa1..9cf5d8419965 100644
>>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
>>>>> @@ -33,6 +33,7 @@
>>>>> #define AMDGPU_PL_GWS (TTM_PL_PRIV + 1)
>>>>> #define AMDGPU_PL_OA (TTM_PL_PRIV + 2)
>>>>> #define AMDGPU_PL_PREEMPT (TTM_PL_PRIV + 3)
>>>>> +#define AMDGPU_PL_DOORBELL (TTM_PL_PRIV + 4)
>>>>> #define AMDGPU_GTT_MAX_TRANSFER_SIZE 512
>>>>> #define AMDGPU_GTT_NUM_TRANSFER_WINDOWS 2
>>>>
>>
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v2 5/8] drm/amdgpu: accommodate DOMAIN/PL_DOORBELL
2023-02-15 13:36 ` Christian König
@ 2023-02-15 13:45 ` Shashank Sharma
0 siblings, 0 replies; 28+ messages in thread
From: Shashank Sharma @ 2023-02-15 13:45 UTC (permalink / raw)
To: Christian König, amd-gfx; +Cc: alexander.deucher, Arvind.Yadav
On 15/02/2023 14:36, Christian König wrote:
> Am 15.02.23 um 14:32 schrieb Shashank Sharma:
>>
>> On 15/02/2023 07:17, Christian König wrote:
>>> Am 14.02.23 um 20:24 schrieb Shashank Sharma:
>>>>
>>>> On 14/02/2023 19:31, Christian König wrote:
>>>>> Am 14.02.23 um 17:15 schrieb Shashank Sharma:
>>>>>> From: Alex Deucher <alexander.deucher@amd.com>
>>>>>>
>>>>>> This patch adds changes:
>>>>>> - to accommodate the new GEM domain DOORBELL
>>>>>> - to accommodate the new TTM PL DOORBELL
>>>>>>
>>>>>> to manage doorbell allocations as GEM Objects.
>>>>>>
>>>>>> V2: Addressed reviwe comments from Christian
>>>>>> - drop the doorbell changes for pinning/unpinning
>>>>>> - drop the doorbell changes for dma-buf map
>>>>>> - drop the doorbell changes for sgt
>>>>>> - no need to handle TTM_PL_FLAG_CONTIGUOUS for doorbell
>>>>>>
>>>>>> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
>>>>>> Signed-off-by: Shashank Sharma <shashank.sharma@amd.com>
>>>>>> ---
>>>>>> drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 11 ++++++++++-
>>>>>> drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 11 ++++++++++-
>>>>>> drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h | 1 +
>>>>>> 3 files changed, 21 insertions(+), 2 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>>>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>>>>>> index b48c9fd60c43..ff9979fecfd2 100644
>>>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>>>>>> @@ -147,6 +147,14 @@ void amdgpu_bo_placement_from_domain(struct
>>>>>> amdgpu_bo *abo, u32 domain)
>>>>>> c++;
>>>>>> }
>>>>>> + if (domain & AMDGPU_GEM_DOMAIN_DOORBELL) {
>>>>>> + places[c].fpfn = 0;
>>>>>> + places[c].lpfn = 0;
>>>>>> + places[c].mem_type = AMDGPU_PL_DOORBELL;
>>>>>> + places[c].flags = 0;
>>>>>> + c++;
>>>>>> + }
>>>>>> +
>>>>>
>>>>> Mhm, do we need to increase AMDGPU_BO_MAX_PLACEMENTS?
>>>>>
>>>>> I think the answer is *no* since mixing DOORBELL with CPU, GTT or
>>>>> VRAM placement doesn't make sense, but do we enforce that somewhere?
>>>> I am not sure why do we need that ?
>>>
>>> Userspace could otherwise specify DOORBEEL|CPU|GTT|VRAM as placement
>>> which would overrun the array and be illegal.
>>
>> Now when I understand this, how can we enforce this ? A size check
>> that blocks places to go over a certain value, which is fixed for
>> boorbell ?
>
> In amdgpu_bo_create() we should have a check that if GDS, GWS, OA and
> DOORBELL are specified they are specified all alone. In other words
> those domains can't be mixed with anything else.
>
> Christian.
>
Got it, let me check this out.
- Shashank
>>
>>>
>>>>>
>>>>>> if (domain & AMDGPU_GEM_DOMAIN_GTT) {
>>>>>> places[c].fpfn = 0;
>>>>>> places[c].lpfn = 0;
>>>>>> @@ -466,7 +474,7 @@ static bool amdgpu_bo_validate_size(struct
>>>>>> amdgpu_device *adev,
>>>>>> goto fail;
>>>>>> }
>>>>>> - /* TODO add more domains checks, such as
>>>>>> AMDGPU_GEM_DOMAIN_CPU */
>>>>>> + /* TODO add more domains checks, such as
>>>>>> AMDGPU_GEM_DOMAIN_CPU, AMDGPU_GEM_DOMAIN_DOORBELL */
>>>>>
>>>>> Should we enforce that user space can only allocate 1 page doorbells?
>>>>>
>>>> Should we add a per-PID basis check ?
>>>
>>> No, just a check that the allocation size of the doorbell BOs is
>>> just 1 page.
>>
>> Noted
>>
>> - Shashank
>>
>>>
>>> Christian.
>>>
>>>>
>>>> - Shashank
>>>>
>>>>>> return true;
>>>>>> fail:
>>>>>> @@ -1014,6 +1022,7 @@ void amdgpu_bo_unpin(struct amdgpu_bo *bo)
>>>>>> } else if (bo->tbo.resource->mem_type == TTM_PL_TT) {
>>>>>> atomic64_sub(amdgpu_bo_size(bo), &adev->gart_pin_size);
>>>>>> }
>>>>>> +
>>>>>
>>>>> Unrelated change.
>>>>
>>>> Noted
>>>>
>>>> - Shashank
>>>>
>>>>>
>>>>> Regards,
>>>>> Christian.
>>>>>
>>>>>> }
>>>>>> static const char *amdgpu_vram_names[] = {
>>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>>>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>>>>>> index 0e8f580769ab..e9dc24191fc8 100644
>>>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>>>>>> @@ -128,6 +128,7 @@ static void amdgpu_evict_flags(struct
>>>>>> ttm_buffer_object *bo,
>>>>>> case AMDGPU_PL_GDS:
>>>>>> case AMDGPU_PL_GWS:
>>>>>> case AMDGPU_PL_OA:
>>>>>> + case AMDGPU_PL_DOORBELL:
>>>>>> placement->num_placement = 0;
>>>>>> placement->num_busy_placement = 0;
>>>>>> return;
>>>>>> @@ -500,9 +501,11 @@ static int amdgpu_bo_move(struct
>>>>>> ttm_buffer_object *bo, bool evict,
>>>>>> if (old_mem->mem_type == AMDGPU_PL_GDS ||
>>>>>> old_mem->mem_type == AMDGPU_PL_GWS ||
>>>>>> old_mem->mem_type == AMDGPU_PL_OA ||
>>>>>> + old_mem->mem_type == AMDGPU_PL_DOORBELL ||
>>>>>> new_mem->mem_type == AMDGPU_PL_GDS ||
>>>>>> new_mem->mem_type == AMDGPU_PL_GWS ||
>>>>>> - new_mem->mem_type == AMDGPU_PL_OA) {
>>>>>> + new_mem->mem_type == AMDGPU_PL_OA ||
>>>>>> + new_mem->mem_type == AMDGPU_PL_DOORBELL) {
>>>>>> /* Nothing to save here */
>>>>>> ttm_bo_move_null(bo, new_mem);
>>>>>> goto out;
>>>>>> @@ -586,6 +589,11 @@ static int amdgpu_ttm_io_mem_reserve(struct
>>>>>> ttm_device *bdev,
>>>>>> mem->bus.offset += adev->gmc.vram_aper_base;
>>>>>> mem->bus.is_iomem = true;
>>>>>> break;
>>>>>> + case AMDGPU_PL_DOORBELL:
>>>>>> + mem->bus.offset = mem->start << PAGE_SHIFT;
>>>>>> + mem->bus.offset += adev->doorbell.doorbell_aper_base;
>>>>>> + mem->bus.is_iomem = true;
>>>>>> + break;
>>>>>> default:
>>>>>> return -EINVAL;
>>>>>> }
>>>>>> @@ -1267,6 +1275,7 @@ uint64_t amdgpu_ttm_tt_pde_flags(struct
>>>>>> ttm_tt *ttm, struct ttm_resource *mem)
>>>>>> flags |= AMDGPU_PTE_VALID;
>>>>>> if (mem && (mem->mem_type == TTM_PL_TT ||
>>>>>> + mem->mem_type == AMDGPU_PL_DOORBELL ||
>>>>>> mem->mem_type == AMDGPU_PL_PREEMPT)) {
>>>>>> flags |= AMDGPU_PTE_SYSTEM;
>>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
>>>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
>>>>>> index 967b265dbfa1..9cf5d8419965 100644
>>>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
>>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
>>>>>> @@ -33,6 +33,7 @@
>>>>>> #define AMDGPU_PL_GWS (TTM_PL_PRIV + 1)
>>>>>> #define AMDGPU_PL_OA (TTM_PL_PRIV + 2)
>>>>>> #define AMDGPU_PL_PREEMPT (TTM_PL_PRIV + 3)
>>>>>> +#define AMDGPU_PL_DOORBELL (TTM_PL_PRIV + 4)
>>>>>> #define AMDGPU_GTT_MAX_TRANSFER_SIZE 512
>>>>>> #define AMDGPU_GTT_NUM_TRANSFER_WINDOWS 2
>>>>>
>>>
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v2 6/8] drm/amdgpu: get doorbell memory
2023-02-14 16:15 [PATCH v2 0/8] Re-design doorbell framework for usermode queues Shashank Sharma
` (4 preceding siblings ...)
2023-02-14 16:15 ` [PATCH v2 5/8] drm/amdgpu: accommodate DOMAIN/PL_DOORBELL Shashank Sharma
@ 2023-02-14 16:15 ` Shashank Sharma
2023-02-14 16:15 ` [PATCH v2 7/8] drm/amdgpu: create doorbell kernel object Shashank Sharma
2023-02-14 16:15 ` [PATCH v2 8/8] drm/amdgpu: start using kernel doorbell bo Shashank Sharma
7 siblings, 0 replies; 28+ messages in thread
From: Shashank Sharma @ 2023-02-14 16:15 UTC (permalink / raw)
To: amd-gfx; +Cc: alexander.deucher, christian.koenig, Arvind.Yadav,
shashank.sharma
From: Alex Deucher <alexander.deucher@amd.com>
This patch adds section for doorbell memory in memory status
reporting functions like vm/bo_get_memory.
V2: Rebase
Cc: Alex Deucher <alexander.deucher@amd.com>
Cc: Christian Koenig <christian.koenig@amd.com>
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
Signed-off-by: Shashank Sharma <shashank.sharma@amd.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_fdinfo.c | 4 ++--
drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 9 ++++++++-
drivers/gpu/drm/amd/amdgpu/amdgpu_object.h | 3 ++-
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 15 ++++++++-------
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h | 3 ++-
5 files changed, 22 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fdinfo.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_fdinfo.c
index 99a7855ab1bc..202df09ba5de 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fdinfo.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fdinfo.c
@@ -60,7 +60,7 @@ void amdgpu_show_fdinfo(struct seq_file *m, struct file *f)
struct amdgpu_fpriv *fpriv = file->driver_priv;
struct amdgpu_vm *vm = &fpriv->vm;
- uint64_t vram_mem = 0, gtt_mem = 0, cpu_mem = 0;
+ uint64_t vram_mem = 0, gtt_mem = 0, cpu_mem = 0, doorbell_mem = 0;
ktime_t usage[AMDGPU_HW_IP_NUM];
uint32_t bus, dev, fn, domain;
unsigned int hw_ip;
@@ -75,7 +75,7 @@ void amdgpu_show_fdinfo(struct seq_file *m, struct file *f)
if (ret)
return;
- amdgpu_vm_get_memory(vm, &vram_mem, >t_mem, &cpu_mem);
+ amdgpu_vm_get_memory(vm, &vram_mem, >t_mem, &cpu_mem, &doorbell_mem);
amdgpu_bo_unreserve(vm->root.bo);
amdgpu_ctx_mgr_usage(&fpriv->ctx_mgr, usage);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index ff9979fecfd2..90e97abc1454 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -1275,7 +1275,8 @@ void amdgpu_bo_move_notify(struct ttm_buffer_object *bo,
}
void amdgpu_bo_get_memory(struct amdgpu_bo *bo, uint64_t *vram_mem,
- uint64_t *gtt_mem, uint64_t *cpu_mem)
+ uint64_t *gtt_mem, uint64_t *cpu_mem,
+ uint64_t *doorbell_mem)
{
unsigned int domain;
@@ -1287,6 +1288,9 @@ void amdgpu_bo_get_memory(struct amdgpu_bo *bo, uint64_t *vram_mem,
case AMDGPU_GEM_DOMAIN_GTT:
*gtt_mem += amdgpu_bo_size(bo);
break;
+ case AMDGPU_GEM_DOMAIN_DOORBELL:
+ *doorbell_mem += amdgpu_bo_size(bo);
+ break;
case AMDGPU_GEM_DOMAIN_CPU:
default:
*cpu_mem += amdgpu_bo_size(bo);
@@ -1565,6 +1569,9 @@ u64 amdgpu_bo_print_info(int id, struct amdgpu_bo *bo, struct seq_file *m)
case AMDGPU_GEM_DOMAIN_GTT:
placement = " GTT";
break;
+ case AMDGPU_GEM_DOMAIN_DOORBELL:
+ placement = "DOOR";
+ break;
case AMDGPU_GEM_DOMAIN_CPU:
default:
placement = " CPU";
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
index 93207badf83f..bf9759758f0d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
@@ -326,7 +326,8 @@ int amdgpu_bo_sync_wait(struct amdgpu_bo *bo, void *owner, bool intr);
u64 amdgpu_bo_gpu_offset(struct amdgpu_bo *bo);
u64 amdgpu_bo_gpu_offset_no_check(struct amdgpu_bo *bo);
void amdgpu_bo_get_memory(struct amdgpu_bo *bo, uint64_t *vram_mem,
- uint64_t *gtt_mem, uint64_t *cpu_mem);
+ uint64_t *gtt_mem, uint64_t *cpu_mem,
+ uint64_t *doorbell_mem);
void amdgpu_bo_add_to_shadow_list(struct amdgpu_bo_vm *vmbo);
int amdgpu_bo_restore_shadow(struct amdgpu_bo *shadow,
struct dma_fence **fence);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index dc379dc22c77..1561d138945b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -918,7 +918,8 @@ int amdgpu_vm_update_range(struct amdgpu_device *adev, struct amdgpu_vm *vm,
}
void amdgpu_vm_get_memory(struct amdgpu_vm *vm, uint64_t *vram_mem,
- uint64_t *gtt_mem, uint64_t *cpu_mem)
+ uint64_t *gtt_mem, uint64_t *cpu_mem,
+ uint64_t *doorbell_mem)
{
struct amdgpu_bo_va *bo_va, *tmp;
@@ -927,37 +928,37 @@ void amdgpu_vm_get_memory(struct amdgpu_vm *vm, uint64_t *vram_mem,
if (!bo_va->base.bo)
continue;
amdgpu_bo_get_memory(bo_va->base.bo, vram_mem,
- gtt_mem, cpu_mem);
+ gtt_mem, cpu_mem, doorbell_mem);
}
list_for_each_entry_safe(bo_va, tmp, &vm->evicted, base.vm_status) {
if (!bo_va->base.bo)
continue;
amdgpu_bo_get_memory(bo_va->base.bo, vram_mem,
- gtt_mem, cpu_mem);
+ gtt_mem, cpu_mem, doorbell_mem);
}
list_for_each_entry_safe(bo_va, tmp, &vm->relocated, base.vm_status) {
if (!bo_va->base.bo)
continue;
amdgpu_bo_get_memory(bo_va->base.bo, vram_mem,
- gtt_mem, cpu_mem);
+ gtt_mem, cpu_mem, doorbell_mem);
}
list_for_each_entry_safe(bo_va, tmp, &vm->moved, base.vm_status) {
if (!bo_va->base.bo)
continue;
amdgpu_bo_get_memory(bo_va->base.bo, vram_mem,
- gtt_mem, cpu_mem);
+ gtt_mem, cpu_mem, doorbell_mem);
}
list_for_each_entry_safe(bo_va, tmp, &vm->invalidated, base.vm_status) {
if (!bo_va->base.bo)
continue;
amdgpu_bo_get_memory(bo_va->base.bo, vram_mem,
- gtt_mem, cpu_mem);
+ gtt_mem, cpu_mem, doorbell_mem);
}
list_for_each_entry_safe(bo_va, tmp, &vm->done, base.vm_status) {
if (!bo_va->base.bo)
continue;
amdgpu_bo_get_memory(bo_va->base.bo, vram_mem,
- gtt_mem, cpu_mem);
+ gtt_mem, cpu_mem, doorbell_mem);
}
spin_unlock(&vm->status_lock);
}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
index 094bb4807303..b8ac7d311c8b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
@@ -458,7 +458,8 @@ void amdgpu_vm_set_task_info(struct amdgpu_vm *vm);
void amdgpu_vm_move_to_lru_tail(struct amdgpu_device *adev,
struct amdgpu_vm *vm);
void amdgpu_vm_get_memory(struct amdgpu_vm *vm, uint64_t *vram_mem,
- uint64_t *gtt_mem, uint64_t *cpu_mem);
+ uint64_t *gtt_mem, uint64_t *cpu_mem,
+ uint64_t *doorbell_mem);
int amdgpu_vm_pt_clear(struct amdgpu_device *adev, struct amdgpu_vm *vm,
struct amdgpu_bo_vm *vmbo, bool immediate);
--
2.34.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH v2 7/8] drm/amdgpu: create doorbell kernel object
2023-02-14 16:15 [PATCH v2 0/8] Re-design doorbell framework for usermode queues Shashank Sharma
` (5 preceding siblings ...)
2023-02-14 16:15 ` [PATCH v2 6/8] drm/amdgpu: get doorbell memory Shashank Sharma
@ 2023-02-14 16:15 ` Shashank Sharma
2023-02-14 18:35 ` Christian König
2023-02-14 16:15 ` [PATCH v2 8/8] drm/amdgpu: start using kernel doorbell bo Shashank Sharma
7 siblings, 1 reply; 28+ messages in thread
From: Shashank Sharma @ 2023-02-14 16:15 UTC (permalink / raw)
To: amd-gfx
Cc: alexander.deucher, Shashank Sharma, christian.koenig,
Arvind.Yadav, shashank.sharma
From: Shashank Sharma <contactshashanksharma@gmail.com>
This patch does the following:
- Initializes TTM range management for domain DOORBELL.
- Introduces a kernel bo for doorbell management in form of mman.doorbell_kernel_bo.
This bo holds the kernel doorbell space now.
- Removes ioremapping of doorbell-kernel memory, as its not required now.
V2:
- Addressed review comments from Christian:
- do not use kernel_create_at(0), use kernel_create() instead.
- do not use ttm_resource_manager, use range_manager instead.
- do not ioremap doorbell, TTM will do that.
- Split one big patch into 2
Cc: Alex Deucher <alexander.deucher@amd.com>
Cc: Christian Koenig <christian.koenig@amd.com>
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
Signed-off-by: Shashank Sharma <shashank.sharma@amd.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 22 ++++++++++++++++++++++
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h | 7 +++++++
2 files changed, 29 insertions(+)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index e9dc24191fc8..086e83c17c0f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -1879,12 +1879,32 @@ int amdgpu_ttm_init(struct amdgpu_device *adev)
return r;
}
+ r = amdgpu_ttm_init_on_chip(adev, AMDGPU_PL_DOORBELL, adev->doorbell.doorbell_aper_size);
+ if (r) {
+ DRM_ERROR("Failed initializing oa heap.\n");
+ return r;
+ }
+
if (amdgpu_bo_create_kernel(adev, PAGE_SIZE, PAGE_SIZE,
AMDGPU_GEM_DOMAIN_GTT,
&adev->mman.sdma_access_bo, NULL,
&adev->mman.sdma_access_ptr))
DRM_WARN("Debug VRAM access will use slowpath MM access\n");
+ /* Create a doorbell BO for kernel usages */
+ r = amdgpu_bo_create_kernel(adev,
+ adev->mman.doorbell_kernel_bo_size,
+ PAGE_SIZE,
+ AMDGPU_GEM_DOMAIN_DOORBELL,
+ &adev->mman.doorbell_kernel_bo,
+ &adev->mman.doorbell_gpu_addr,
+ (void **)&adev->mman.doorbell_cpu_addr);
+
+ if (r) {
+ DRM_ERROR("Failed to create doorbell BO, err=%d\n", r);
+ return r;
+ }
+
return 0;
}
@@ -1908,6 +1928,8 @@ void amdgpu_ttm_fini(struct amdgpu_device *adev)
NULL, NULL);
amdgpu_bo_free_kernel(&adev->mman.sdma_access_bo, NULL,
&adev->mman.sdma_access_ptr);
+ amdgpu_bo_free_kernel(&adev->mman.doorbell_kernel_bo,
+ NULL, (void **)&adev->mman.doorbell_cpu_addr);
amdgpu_ttm_fw_reserve_vram_fini(adev);
amdgpu_ttm_drv_reserve_vram_fini(adev);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
index 9cf5d8419965..50748ff1dd3c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
@@ -97,6 +97,13 @@ struct amdgpu_mman {
/* PAGE_SIZE'd BO for process memory r/w over SDMA. */
struct amdgpu_bo *sdma_access_bo;
void *sdma_access_ptr;
+
+ /* doorbells reserved for the kernel driver */
+ u32 num_kernel_doorbells; /* Number of doorbells actually reserved for kernel */
+ uint64_t doorbell_kernel_bo_size;
+ uint64_t doorbell_gpu_addr;
+ struct amdgpu_bo *doorbell_kernel_bo;
+ u32 *doorbell_cpu_addr;
};
struct amdgpu_copy_mem {
--
2.34.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH v2 7/8] drm/amdgpu: create doorbell kernel object
2023-02-14 16:15 ` [PATCH v2 7/8] drm/amdgpu: create doorbell kernel object Shashank Sharma
@ 2023-02-14 18:35 ` Christian König
2023-02-14 19:26 ` Shashank Sharma
0 siblings, 1 reply; 28+ messages in thread
From: Christian König @ 2023-02-14 18:35 UTC (permalink / raw)
To: Shashank Sharma, amd-gfx; +Cc: alexander.deucher, Shashank Sharma, Arvind.Yadav
Am 14.02.23 um 17:15 schrieb Shashank Sharma:
> From: Shashank Sharma <contactshashanksharma@gmail.com>
>
> This patch does the following:
> - Initializes TTM range management for domain DOORBELL.
> - Introduces a kernel bo for doorbell management in form of mman.doorbell_kernel_bo.
> This bo holds the kernel doorbell space now.
> - Removes ioremapping of doorbell-kernel memory, as its not required now.
>
> V2:
> - Addressed review comments from Christian:
> - do not use kernel_create_at(0), use kernel_create() instead.
> - do not use ttm_resource_manager, use range_manager instead.
> - do not ioremap doorbell, TTM will do that.
> - Split one big patch into 2
>
> Cc: Alex Deucher <alexander.deucher@amd.com>
> Cc: Christian Koenig <christian.koenig@amd.com>
> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> Signed-off-by: Shashank Sharma <shashank.sharma@amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 22 ++++++++++++++++++++++
> drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h | 7 +++++++
> 2 files changed, 29 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> index e9dc24191fc8..086e83c17c0f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> @@ -1879,12 +1879,32 @@ int amdgpu_ttm_init(struct amdgpu_device *adev)
> return r;
> }
>
> + r = amdgpu_ttm_init_on_chip(adev, AMDGPU_PL_DOORBELL, adev->doorbell.doorbell_aper_size);
> + if (r) {
> + DRM_ERROR("Failed initializing oa heap.\n");
> + return r;
> + }
> +
> if (amdgpu_bo_create_kernel(adev, PAGE_SIZE, PAGE_SIZE,
> AMDGPU_GEM_DOMAIN_GTT,
> &adev->mman.sdma_access_bo, NULL,
> &adev->mman.sdma_access_ptr))
> DRM_WARN("Debug VRAM access will use slowpath MM access\n");
>
> + /* Create a doorbell BO for kernel usages */
> + r = amdgpu_bo_create_kernel(adev,
> + adev->mman.doorbell_kernel_bo_size,
> + PAGE_SIZE,
> + AMDGPU_GEM_DOMAIN_DOORBELL,
> + &adev->mman.doorbell_kernel_bo,
> + &adev->mman.doorbell_gpu_addr,
> + (void **)&adev->mman.doorbell_cpu_addr);
> +
> + if (r) {
> + DRM_ERROR("Failed to create doorbell BO, err=%d\n", r);
> + return r;
> + }
> +
I would even move this before the SDMA VRAM buffer since the later is
only nice to have while the doorbell is mandatory to have.
> return 0;
> }
>
> @@ -1908,6 +1928,8 @@ void amdgpu_ttm_fini(struct amdgpu_device *adev)
> NULL, NULL);
> amdgpu_bo_free_kernel(&adev->mman.sdma_access_bo, NULL,
> &adev->mman.sdma_access_ptr);
> + amdgpu_bo_free_kernel(&adev->mman.doorbell_kernel_bo,
> + NULL, (void **)&adev->mman.doorbell_cpu_addr);
> amdgpu_ttm_fw_reserve_vram_fini(adev);
> amdgpu_ttm_drv_reserve_vram_fini(adev);
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
> index 9cf5d8419965..50748ff1dd3c 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
> @@ -97,6 +97,13 @@ struct amdgpu_mman {
> /* PAGE_SIZE'd BO for process memory r/w over SDMA. */
> struct amdgpu_bo *sdma_access_bo;
> void *sdma_access_ptr;
> +
> + /* doorbells reserved for the kernel driver */
> + u32 num_kernel_doorbells; /* Number of doorbells actually reserved for kernel */
> + uint64_t doorbell_kernel_bo_size;
That looks like duplicated information. We should only keep either the
number of kernel doorbells or the kernel doorbell bo size around, not both.
And BTW please no comment after structure members.
Christian.
> + uint64_t doorbell_gpu_addr;
> + struct amdgpu_bo *doorbell_kernel_bo;
> + u32 *doorbell_cpu_addr;
> };
>
> struct amdgpu_copy_mem {
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v2 7/8] drm/amdgpu: create doorbell kernel object
2023-02-14 18:35 ` Christian König
@ 2023-02-14 19:26 ` Shashank Sharma
2023-02-16 13:13 ` Shashank Sharma
0 siblings, 1 reply; 28+ messages in thread
From: Shashank Sharma @ 2023-02-14 19:26 UTC (permalink / raw)
To: Christian König, amd-gfx
Cc: alexander.deucher, Shashank Sharma, Arvind.Yadav
On 14/02/2023 19:35, Christian König wrote:
> Am 14.02.23 um 17:15 schrieb Shashank Sharma:
>> From: Shashank Sharma <contactshashanksharma@gmail.com>
>>
>> This patch does the following:
>> - Initializes TTM range management for domain DOORBELL.
>> - Introduces a kernel bo for doorbell management in form of
>> mman.doorbell_kernel_bo.
>> This bo holds the kernel doorbell space now.
>> - Removes ioremapping of doorbell-kernel memory, as its not required
>> now.
>>
>> V2:
>> - Addressed review comments from Christian:
>> - do not use kernel_create_at(0), use kernel_create() instead.
>> - do not use ttm_resource_manager, use range_manager instead.
>> - do not ioremap doorbell, TTM will do that.
>> - Split one big patch into 2
>>
>> Cc: Alex Deucher <alexander.deucher@amd.com>
>> Cc: Christian Koenig <christian.koenig@amd.com>
>> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
>> Signed-off-by: Shashank Sharma <shashank.sharma@amd.com>
>> ---
>> drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 22 ++++++++++++++++++++++
>> drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h | 7 +++++++
>> 2 files changed, 29 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>> index e9dc24191fc8..086e83c17c0f 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>> @@ -1879,12 +1879,32 @@ int amdgpu_ttm_init(struct amdgpu_device *adev)
>> return r;
>> }
>> + r = amdgpu_ttm_init_on_chip(adev, AMDGPU_PL_DOORBELL,
>> adev->doorbell.doorbell_aper_size);
>> + if (r) {
>> + DRM_ERROR("Failed initializing oa heap.\n");
>> + return r;
>> + }
>> +
>> if (amdgpu_bo_create_kernel(adev, PAGE_SIZE, PAGE_SIZE,
>> AMDGPU_GEM_DOMAIN_GTT,
>> &adev->mman.sdma_access_bo, NULL,
>> &adev->mman.sdma_access_ptr))
>> DRM_WARN("Debug VRAM access will use slowpath MM access\n");
>> + /* Create a doorbell BO for kernel usages */
>> + r = amdgpu_bo_create_kernel(adev,
>> + adev->mman.doorbell_kernel_bo_size,
>> + PAGE_SIZE,
>> + AMDGPU_GEM_DOMAIN_DOORBELL,
>> + &adev->mman.doorbell_kernel_bo,
>> + &adev->mman.doorbell_gpu_addr,
>> + (void **)&adev->mman.doorbell_cpu_addr);
>> +
>> + if (r) {
>> + DRM_ERROR("Failed to create doorbell BO, err=%d\n", r);
>> + return r;
>> + }
>> +
>
> I would even move this before the SDMA VRAM buffer since the later is
> only nice to have while the doorbell is mandatory to have.
Agree,
>
>> return 0;
>> }
>> @@ -1908,6 +1928,8 @@ void amdgpu_ttm_fini(struct amdgpu_device *adev)
>> NULL, NULL);
>> amdgpu_bo_free_kernel(&adev->mman.sdma_access_bo, NULL,
>> &adev->mman.sdma_access_ptr);
>> + amdgpu_bo_free_kernel(&adev->mman.doorbell_kernel_bo,
>> + NULL, (void **)&adev->mman.doorbell_cpu_addr);
>> amdgpu_ttm_fw_reserve_vram_fini(adev);
>> amdgpu_ttm_drv_reserve_vram_fini(adev);
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
>> index 9cf5d8419965..50748ff1dd3c 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
>> @@ -97,6 +97,13 @@ struct amdgpu_mman {
>> /* PAGE_SIZE'd BO for process memory r/w over SDMA. */
>> struct amdgpu_bo *sdma_access_bo;
>> void *sdma_access_ptr;
>> +
>> + /* doorbells reserved for the kernel driver */
>> + u32 num_kernel_doorbells; /* Number of doorbells
>> actually reserved for kernel */
>> + uint64_t doorbell_kernel_bo_size;
>
> That looks like duplicated information. We should only keep either the
> number of kernel doorbells or the kernel doorbell bo size around, not
> both.
Yeah, agree. I can remove one of these two.
>
> And BTW please no comment after structure members.
>
Noted,
- Shashank
> Christian.
>
>> + uint64_t doorbell_gpu_addr;
>> + struct amdgpu_bo *doorbell_kernel_bo;
>> + u32 *doorbell_cpu_addr;
>> };
>> struct amdgpu_copy_mem {
>
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v2 7/8] drm/amdgpu: create doorbell kernel object
2023-02-14 19:26 ` Shashank Sharma
@ 2023-02-16 13:13 ` Shashank Sharma
0 siblings, 0 replies; 28+ messages in thread
From: Shashank Sharma @ 2023-02-16 13:13 UTC (permalink / raw)
To: Christian König, amd-gfx
Cc: alexander.deucher, Shashank Sharma, Arvind.Yadav
On 14/02/2023 20:26, Shashank Sharma wrote:
>
> On 14/02/2023 19:35, Christian König wrote:
>> Am 14.02.23 um 17:15 schrieb Shashank Sharma:
>>> From: Shashank Sharma <contactshashanksharma@gmail.com>
>>>
>>> This patch does the following:
>>> - Initializes TTM range management for domain DOORBELL.
>>> - Introduces a kernel bo for doorbell management in form of
>>> mman.doorbell_kernel_bo.
>>> This bo holds the kernel doorbell space now.
>>> - Removes ioremapping of doorbell-kernel memory, as its not required
>>> now.
>>>
>>> V2:
>>> - Addressed review comments from Christian:
>>> - do not use kernel_create_at(0), use kernel_create() instead.
>>> - do not use ttm_resource_manager, use range_manager instead.
>>> - do not ioremap doorbell, TTM will do that.
>>> - Split one big patch into 2
>>>
>>> Cc: Alex Deucher <alexander.deucher@amd.com>
>>> Cc: Christian Koenig <christian.koenig@amd.com>
>>> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
>>> Signed-off-by: Shashank Sharma <shashank.sharma@amd.com>
>>> ---
>>> drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 22 ++++++++++++++++++++++
>>> drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h | 7 +++++++
>>> 2 files changed, 29 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>>> index e9dc24191fc8..086e83c17c0f 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>>> @@ -1879,12 +1879,32 @@ int amdgpu_ttm_init(struct amdgpu_device *adev)
>>> return r;
>>> }
>>> + r = amdgpu_ttm_init_on_chip(adev, AMDGPU_PL_DOORBELL,
>>> adev->doorbell.doorbell_aper_size);
>>> + if (r) {
>>> + DRM_ERROR("Failed initializing oa heap.\n");
>>> + return r;
>>> + }
>>> +
>>> if (amdgpu_bo_create_kernel(adev, PAGE_SIZE, PAGE_SIZE,
>>> AMDGPU_GEM_DOMAIN_GTT,
>>> &adev->mman.sdma_access_bo, NULL,
>>> &adev->mman.sdma_access_ptr))
>>> DRM_WARN("Debug VRAM access will use slowpath MM access\n");
>>> + /* Create a doorbell BO for kernel usages */
>>> + r = amdgpu_bo_create_kernel(adev,
>>> + adev->mman.doorbell_kernel_bo_size,
>>> + PAGE_SIZE,
>>> + AMDGPU_GEM_DOMAIN_DOORBELL,
>>> + &adev->mman.doorbell_kernel_bo,
>>> + &adev->mman.doorbell_gpu_addr,
>>> + (void **)&adev->mman.doorbell_cpu_addr);
>>> +
>>> + if (r) {
>>> + DRM_ERROR("Failed to create doorbell BO, err=%d\n", r);
>>> + return r;
>>> + }
>>> +
>>
>> I would even move this before the SDMA VRAM buffer since the later is
>> only nice to have while the doorbell is mandatory to have.
> Agree,
>>
>>> return 0;
>>> }
>>> @@ -1908,6 +1928,8 @@ void amdgpu_ttm_fini(struct amdgpu_device
>>> *adev)
>>> NULL, NULL);
>>> amdgpu_bo_free_kernel(&adev->mman.sdma_access_bo, NULL,
>>> &adev->mman.sdma_access_ptr);
>>> + amdgpu_bo_free_kernel(&adev->mman.doorbell_kernel_bo,
>>> + NULL, (void **)&adev->mman.doorbell_cpu_addr);
>>> amdgpu_ttm_fw_reserve_vram_fini(adev);
>>> amdgpu_ttm_drv_reserve_vram_fini(adev);
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
>>> index 9cf5d8419965..50748ff1dd3c 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
>>> @@ -97,6 +97,13 @@ struct amdgpu_mman {
>>> /* PAGE_SIZE'd BO for process memory r/w over SDMA. */
>>> struct amdgpu_bo *sdma_access_bo;
>>> void *sdma_access_ptr;
>>> +
>>> + /* doorbells reserved for the kernel driver */
>>> + u32 num_kernel_doorbells; /* Number of doorbells
>>> actually reserved for kernel */
>>> + uint64_t doorbell_kernel_bo_size;
>>
>> That looks like duplicated information. We should only keep either
>> the number of kernel doorbells or the kernel doorbell bo size around,
>> not both.
> Yeah, agree. I can remove one of these two.
On a second thought, while doing some experiments with doorbells I
realized that we might want to keep both of these, as:
num_kernel_doorbell = actual doorbells reserved for kernel,
doorbell_kernel_bo_size = max (PAGE_SIZE, num_kernel_doorbell* sizeof(u32))
I will have to update the code to reflect that as well.
- Shashank
>>
>> And BTW please no comment after structure members.
>>
> Noted,
>
> - Shashank
>
>> Christian.
>>
>>> + uint64_t doorbell_gpu_addr;
>>> + struct amdgpu_bo *doorbell_kernel_bo;
>>> + u32 *doorbell_cpu_addr;
>>> };
>>> struct amdgpu_copy_mem {
>>
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v2 8/8] drm/amdgpu: start using kernel doorbell bo
2023-02-14 16:15 [PATCH v2 0/8] Re-design doorbell framework for usermode queues Shashank Sharma
` (6 preceding siblings ...)
2023-02-14 16:15 ` [PATCH v2 7/8] drm/amdgpu: create doorbell kernel object Shashank Sharma
@ 2023-02-14 16:15 ` Shashank Sharma
2023-02-14 18:40 ` Christian König
7 siblings, 1 reply; 28+ messages in thread
From: Shashank Sharma @ 2023-02-14 16:15 UTC (permalink / raw)
To: amd-gfx
Cc: alexander.deucher, Shashank Sharma, christian.koenig,
Arvind.Yadav, shashank.sharma
From: Shashank Sharma <contactshashanksharma@gmail.com>
This patch does the following:
- Adds new variables like mman.doorbell_bo_size/gpu_addr/cpu_addr.
The cpu_addr ptr will be used now for doorbell read/write from
doorbell BAR.
- Adjusts the existing code to use kernel doorbell BO's size and its
cpu_address.
Cc: Alex Deucher <alexander.deucher@amd.com>
Cc: Christian Koenig <christian.koenig@amd.com>
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
Signed-off-by: Shashank Sharma <shashank.sharma@amd.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c | 5 ++-
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 33 +++++++++-----------
drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell.h | 1 -
3 files changed, 16 insertions(+), 23 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
index 0493c64e9d0a..87f486f522ae 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
@@ -109,11 +109,10 @@ static void amdgpu_doorbell_get_kfd_info(struct amdgpu_device *adev,
*aperture_base = adev->doorbell.doorbell_aper_base;
*aperture_size = 0;
*start_offset = 0;
- } else if (adev->doorbell.doorbell_aper_size > adev->doorbell.num_doorbells *
- sizeof(u32)) {
+ } else if (adev->doorbell.doorbell_aper_size > adev->mman.doorbell_kernel_bo_size) {
*aperture_base = adev->doorbell.doorbell_aper_base;
*aperture_size = adev->doorbell.doorbell_aper_size;
- *start_offset = adev->doorbell.num_doorbells * sizeof(u32);
+ *start_offset = adev->mman.doorbell_kernel_bo_size;
} else {
*aperture_base = 0;
*aperture_size = 0;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 43c1b67c2778..fde199434579 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -596,8 +596,8 @@ u32 amdgpu_mm_rdoorbell(struct amdgpu_device *adev, u32 index)
if (amdgpu_device_skip_hw_access(adev))
return 0;
- if (index < adev->doorbell.num_doorbells) {
- return readl(adev->mman.doorbell_aper_base_kaddr + index);
+ if (index < adev->mman.num_kernel_doorbells) {
+ return readl(adev->mman.doorbell_cpu_addr + index);
} else {
DRM_ERROR("reading beyond doorbell aperture: 0x%08x!\n", index);
return 0;
@@ -619,8 +619,8 @@ void amdgpu_mm_wdoorbell(struct amdgpu_device *adev, u32 index, u32 v)
if (amdgpu_device_skip_hw_access(adev))
return;
- if (index < adev->doorbell.num_doorbells) {
- writel(v, adev->mman.doorbell_aper_base_kaddr + index);
+ if (index < adev->mman.num_kernel_doorbells) {
+ writel(v, adev->mman.doorbell_cpu_addr + index);
} else {
DRM_ERROR("writing beyond doorbell aperture: 0x%08x!\n", index);
}
@@ -640,8 +640,8 @@ u64 amdgpu_mm_rdoorbell64(struct amdgpu_device *adev, u32 index)
if (amdgpu_device_skip_hw_access(adev))
return 0;
- if (index < adev->doorbell.num_doorbells) {
- return atomic64_read((atomic64_t *)(adev->mman.doorbell_aper_base_kaddr + index));
+ if (index < adev->mman.num_kernel_doorbells) {
+ return atomic64_read((atomic64_t *)(adev->mman.doorbell_cpu_addr + index));
} else {
DRM_ERROR("reading beyond doorbell aperture: 0x%08x!\n", index);
return 0;
@@ -663,8 +663,8 @@ void amdgpu_mm_wdoorbell64(struct amdgpu_device *adev, u32 index, u64 v)
if (amdgpu_device_skip_hw_access(adev))
return;
- if (index < adev->doorbell.num_doorbells) {
- atomic64_set((atomic64_t *)(adev->mman.doorbell_aper_base_kaddr + index), v);
+ if (index < adev->mman.num_kernel_doorbells) {
+ atomic64_set((atomic64_t *)(adev->mman.doorbell_cpu_addr + index), v);
} else {
DRM_ERROR("writing beyond doorbell aperture: 0x%08x!\n", index);
}
@@ -1037,7 +1037,7 @@ static int amdgpu_device_doorbell_init(struct amdgpu_device *adev)
if (adev->asic_type < CHIP_BONAIRE) {
adev->doorbell.doorbell_aper_base = 0;
adev->doorbell.doorbell_aper_size = 0;
- adev->doorbell.num_doorbells = 0;
+ adev->mman.num_kernel_doorbells = 0;
adev->mman.doorbell_aper_base_kaddr = NULL;
return 0;
}
@@ -1052,13 +1052,13 @@ static int amdgpu_device_doorbell_init(struct amdgpu_device *adev)
adev->doorbell.doorbell_aper_size = pci_resource_len(adev->pdev, 2);
if (adev->enable_mes) {
- adev->doorbell.num_doorbells =
+ adev->mman.num_kernel_doorbells =
adev->doorbell.doorbell_aper_size / sizeof(u32);
} else {
- adev->doorbell.num_doorbells =
+ adev->mman.num_kernel_doorbells =
min_t(u32, adev->doorbell.doorbell_aper_size / sizeof(u32),
adev->doorbell_index.max_assignment+1);
- if (adev->doorbell.num_doorbells == 0)
+ if (adev->mman.num_kernel_doorbells == 0)
return -EINVAL;
/* For Vega, reserve and map two pages on doorbell BAR since SDMA
@@ -1068,15 +1068,10 @@ static int amdgpu_device_doorbell_init(struct amdgpu_device *adev)
* the max num_doorbells should + 1 page (0x400 in dword)
*/
if (adev->asic_type >= CHIP_VEGA10)
- adev->doorbell.num_doorbells += 0x400;
+ adev->mman.num_kernel_doorbells += 0x400;
}
- adev->mman.doorbell_aper_base_kaddr = ioremap(adev->doorbell.doorbell_aper_base,
- adev->doorbell.num_doorbells *
- sizeof(u32));
- if (adev->mman.doorbell_aper_base_kaddr == NULL)
- return -ENOMEM;
-
+ adev->mman.doorbell_kernel_bo_size = adev->mman.num_kernel_doorbells * sizeof(u32);
return 0;
}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell.h
index 526b6b4a86dd..7bdff4f926ad 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell.h
@@ -28,7 +28,6 @@ struct amdgpu_doorbell {
/* doorbell mmio */
resource_size_t doorbell_aper_base;
resource_size_t doorbell_aper_size;
- u32 num_doorbells; /* Number of doorbells actually reserved for amdgpu. */
};
/* Reserved doorbells for amdgpu (including multimedia).
--
2.34.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH v2 8/8] drm/amdgpu: start using kernel doorbell bo
2023-02-14 16:15 ` [PATCH v2 8/8] drm/amdgpu: start using kernel doorbell bo Shashank Sharma
@ 2023-02-14 18:40 ` Christian König
2023-02-14 19:28 ` Shashank Sharma
0 siblings, 1 reply; 28+ messages in thread
From: Christian König @ 2023-02-14 18:40 UTC (permalink / raw)
To: Shashank Sharma, amd-gfx; +Cc: alexander.deucher, Shashank Sharma, Arvind.Yadav
Am 14.02.23 um 17:15 schrieb Shashank Sharma:
> From: Shashank Sharma <contactshashanksharma@gmail.com>
>
> This patch does the following:
>
> - Adds new variables like mman.doorbell_bo_size/gpu_addr/cpu_addr.
> The cpu_addr ptr will be used now for doorbell read/write from
> doorbell BAR.
> - Adjusts the existing code to use kernel doorbell BO's size and its
> cpu_address.
>
> Cc: Alex Deucher <alexander.deucher@amd.com>
> Cc: Christian Koenig <christian.koenig@amd.com>
> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> Signed-off-by: Shashank Sharma <shashank.sharma@amd.com>
Maybe squash this one together with the previous patch.
But see below.
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c | 5 ++-
> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 33 +++++++++-----------
> drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell.h | 1 -
> 3 files changed, 16 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
> index 0493c64e9d0a..87f486f522ae 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
> @@ -109,11 +109,10 @@ static void amdgpu_doorbell_get_kfd_info(struct amdgpu_device *adev,
> *aperture_base = adev->doorbell.doorbell_aper_base;
> *aperture_size = 0;
> *start_offset = 0;
> - } else if (adev->doorbell.doorbell_aper_size > adev->doorbell.num_doorbells *
> - sizeof(u32)) {
> + } else if (adev->doorbell.doorbell_aper_size > adev->mman.doorbell_kernel_bo_size) {
> *aperture_base = adev->doorbell.doorbell_aper_base;
> *aperture_size = adev->doorbell.doorbell_aper_size;
> - *start_offset = adev->doorbell.num_doorbells * sizeof(u32);
> + *start_offset = adev->mman.doorbell_kernel_bo_size;
> } else {
> *aperture_base = 0;
> *aperture_size = 0;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index 43c1b67c2778..fde199434579 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -596,8 +596,8 @@ u32 amdgpu_mm_rdoorbell(struct amdgpu_device *adev, u32 index)
> if (amdgpu_device_skip_hw_access(adev))
> return 0;
>
> - if (index < adev->doorbell.num_doorbells) {
> - return readl(adev->mman.doorbell_aper_base_kaddr + index);
> + if (index < adev->mman.num_kernel_doorbells) {
> + return readl(adev->mman.doorbell_cpu_addr + index);
> } else {
> DRM_ERROR("reading beyond doorbell aperture: 0x%08x!\n", index);
> return 0;
> @@ -619,8 +619,8 @@ void amdgpu_mm_wdoorbell(struct amdgpu_device *adev, u32 index, u32 v)
> if (amdgpu_device_skip_hw_access(adev))
> return;
>
> - if (index < adev->doorbell.num_doorbells) {
> - writel(v, adev->mman.doorbell_aper_base_kaddr + index);
> + if (index < adev->mman.num_kernel_doorbells) {
> + writel(v, adev->mman.doorbell_cpu_addr + index);
> } else {
> DRM_ERROR("writing beyond doorbell aperture: 0x%08x!\n", index);
> }
> @@ -640,8 +640,8 @@ u64 amdgpu_mm_rdoorbell64(struct amdgpu_device *adev, u32 index)
> if (amdgpu_device_skip_hw_access(adev))
> return 0;
>
> - if (index < adev->doorbell.num_doorbells) {
> - return atomic64_read((atomic64_t *)(adev->mman.doorbell_aper_base_kaddr + index));
> + if (index < adev->mman.num_kernel_doorbells) {
> + return atomic64_read((atomic64_t *)(adev->mman.doorbell_cpu_addr + index));
> } else {
> DRM_ERROR("reading beyond doorbell aperture: 0x%08x!\n", index);
> return 0;
> @@ -663,8 +663,8 @@ void amdgpu_mm_wdoorbell64(struct amdgpu_device *adev, u32 index, u64 v)
> if (amdgpu_device_skip_hw_access(adev))
> return;
>
> - if (index < adev->doorbell.num_doorbells) {
> - atomic64_set((atomic64_t *)(adev->mman.doorbell_aper_base_kaddr + index), v);
> + if (index < adev->mman.num_kernel_doorbells) {
> + atomic64_set((atomic64_t *)(adev->mman.doorbell_cpu_addr + index), v);
> } else {
> DRM_ERROR("writing beyond doorbell aperture: 0x%08x!\n", index);
> }
> @@ -1037,7 +1037,7 @@ static int amdgpu_device_doorbell_init(struct amdgpu_device *adev)
> if (adev->asic_type < CHIP_BONAIRE) {
> adev->doorbell.doorbell_aper_base = 0;
> adev->doorbell.doorbell_aper_size = 0;
> - adev->doorbell.num_doorbells = 0;
> + adev->mman.num_kernel_doorbells = 0;
> adev->mman.doorbell_aper_base_kaddr = NULL;
> return 0;
> }
> @@ -1052,13 +1052,13 @@ static int amdgpu_device_doorbell_init(struct amdgpu_device *adev)
> adev->doorbell.doorbell_aper_size = pci_resource_len(adev->pdev, 2);
>
> if (adev->enable_mes) {
> - adev->doorbell.num_doorbells =
> + adev->mman.num_kernel_doorbells =
> adev->doorbell.doorbell_aper_size / sizeof(u32);
> } else {
> - adev->doorbell.num_doorbells =
> + adev->mman.num_kernel_doorbells =
> min_t(u32, adev->doorbell.doorbell_aper_size / sizeof(u32),
> adev->doorbell_index.max_assignment+1);
> - if (adev->doorbell.num_doorbells == 0)
> + if (adev->mman.num_kernel_doorbells == 0)
> return -EINVAL;
>
> /* For Vega, reserve and map two pages on doorbell BAR since SDMA
> @@ -1068,15 +1068,10 @@ static int amdgpu_device_doorbell_init(struct amdgpu_device *adev)
> * the max num_doorbells should + 1 page (0x400 in dword)
> */
> if (adev->asic_type >= CHIP_VEGA10)
> - adev->doorbell.num_doorbells += 0x400;
> + adev->mman.num_kernel_doorbells += 0x400;
> }
>
> - adev->mman.doorbell_aper_base_kaddr = ioremap(adev->doorbell.doorbell_aper_base,
> - adev->doorbell.num_doorbells *
> - sizeof(u32));
> - if (adev->mman.doorbell_aper_base_kaddr == NULL)
> - return -ENOMEM;
> -
> + adev->mman.doorbell_kernel_bo_size = adev->mman.num_kernel_doorbells * sizeof(u32);
I would just keep the kernel_bo_size around and make the
num_kernel_doorbells a local variable.
Christian.
> return 0;
> }
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell.h
> index 526b6b4a86dd..7bdff4f926ad 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell.h
> @@ -28,7 +28,6 @@ struct amdgpu_doorbell {
> /* doorbell mmio */
> resource_size_t doorbell_aper_base;
> resource_size_t doorbell_aper_size;
> - u32 num_doorbells; /* Number of doorbells actually reserved for amdgpu. */
> };
>
> /* Reserved doorbells for amdgpu (including multimedia).
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v2 8/8] drm/amdgpu: start using kernel doorbell bo
2023-02-14 18:40 ` Christian König
@ 2023-02-14 19:28 ` Shashank Sharma
2023-02-15 6:18 ` Christian König
0 siblings, 1 reply; 28+ messages in thread
From: Shashank Sharma @ 2023-02-14 19:28 UTC (permalink / raw)
To: Christian König, amd-gfx
Cc: alexander.deucher, Shashank Sharma, Arvind.Yadav
On 14/02/2023 19:40, Christian König wrote:
> Am 14.02.23 um 17:15 schrieb Shashank Sharma:
>> From: Shashank Sharma <contactshashanksharma@gmail.com>
>>
>> This patch does the following:
>>
>> - Adds new variables like mman.doorbell_bo_size/gpu_addr/cpu_addr.
>> The cpu_addr ptr will be used now for doorbell read/write from
>> doorbell BAR.
>> - Adjusts the existing code to use kernel doorbell BO's size and its
>> cpu_address.
>>
>> Cc: Alex Deucher <alexander.deucher@amd.com>
>> Cc: Christian Koenig <christian.koenig@amd.com>
>> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
>> Signed-off-by: Shashank Sharma <shashank.sharma@amd.com>
>
> Maybe squash this one together with the previous patch.
I just split it from the last patch in this series, thought it was too
scattered and might not be
easy to review :D
>
> But see below.
>
>> ---
>> drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c | 5 ++-
>> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 33 +++++++++-----------
>> drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell.h | 1 -
>> 3 files changed, 16 insertions(+), 23 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
>> index 0493c64e9d0a..87f486f522ae 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
>> @@ -109,11 +109,10 @@ static void amdgpu_doorbell_get_kfd_info(struct
>> amdgpu_device *adev,
>> *aperture_base = adev->doorbell.doorbell_aper_base;
>> *aperture_size = 0;
>> *start_offset = 0;
>> - } else if (adev->doorbell.doorbell_aper_size >
>> adev->doorbell.num_doorbells *
>> - sizeof(u32)) {
>> + } else if (adev->doorbell.doorbell_aper_size >
>> adev->mman.doorbell_kernel_bo_size) {
>> *aperture_base = adev->doorbell.doorbell_aper_base;
>> *aperture_size = adev->doorbell.doorbell_aper_size;
>> - *start_offset = adev->doorbell.num_doorbells * sizeof(u32);
>> + *start_offset = adev->mman.doorbell_kernel_bo_size;
>> } else {
>> *aperture_base = 0;
>> *aperture_size = 0;
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> index 43c1b67c2778..fde199434579 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> @@ -596,8 +596,8 @@ u32 amdgpu_mm_rdoorbell(struct amdgpu_device
>> *adev, u32 index)
>> if (amdgpu_device_skip_hw_access(adev))
>> return 0;
>> - if (index < adev->doorbell.num_doorbells) {
>> - return readl(adev->mman.doorbell_aper_base_kaddr + index);
>> + if (index < adev->mman.num_kernel_doorbells) {
>> + return readl(adev->mman.doorbell_cpu_addr + index);
>> } else {
>> DRM_ERROR("reading beyond doorbell aperture: 0x%08x!\n",
>> index);
>> return 0;
>> @@ -619,8 +619,8 @@ void amdgpu_mm_wdoorbell(struct amdgpu_device
>> *adev, u32 index, u32 v)
>> if (amdgpu_device_skip_hw_access(adev))
>> return;
>> - if (index < adev->doorbell.num_doorbells) {
>> - writel(v, adev->mman.doorbell_aper_base_kaddr + index);
>> + if (index < adev->mman.num_kernel_doorbells) {
>> + writel(v, adev->mman.doorbell_cpu_addr + index);
>> } else {
>> DRM_ERROR("writing beyond doorbell aperture: 0x%08x!\n",
>> index);
>> }
>> @@ -640,8 +640,8 @@ u64 amdgpu_mm_rdoorbell64(struct amdgpu_device
>> *adev, u32 index)
>> if (amdgpu_device_skip_hw_access(adev))
>> return 0;
>> - if (index < adev->doorbell.num_doorbells) {
>> - return atomic64_read((atomic64_t
>> *)(adev->mman.doorbell_aper_base_kaddr + index));
>> + if (index < adev->mman.num_kernel_doorbells) {
>> + return atomic64_read((atomic64_t
>> *)(adev->mman.doorbell_cpu_addr + index));
>> } else {
>> DRM_ERROR("reading beyond doorbell aperture: 0x%08x!\n",
>> index);
>> return 0;
>> @@ -663,8 +663,8 @@ void amdgpu_mm_wdoorbell64(struct amdgpu_device
>> *adev, u32 index, u64 v)
>> if (amdgpu_device_skip_hw_access(adev))
>> return;
>> - if (index < adev->doorbell.num_doorbells) {
>> - atomic64_set((atomic64_t
>> *)(adev->mman.doorbell_aper_base_kaddr + index), v);
>> + if (index < adev->mman.num_kernel_doorbells) {
>> + atomic64_set((atomic64_t *)(adev->mman.doorbell_cpu_addr +
>> index), v);
>> } else {
>> DRM_ERROR("writing beyond doorbell aperture: 0x%08x!\n",
>> index);
>> }
>> @@ -1037,7 +1037,7 @@ static int amdgpu_device_doorbell_init(struct
>> amdgpu_device *adev)
>> if (adev->asic_type < CHIP_BONAIRE) {
>> adev->doorbell.doorbell_aper_base = 0;
>> adev->doorbell.doorbell_aper_size = 0;
>> - adev->doorbell.num_doorbells = 0;
>> + adev->mman.num_kernel_doorbells = 0;
>> adev->mman.doorbell_aper_base_kaddr = NULL;
>> return 0;
>> }
>> @@ -1052,13 +1052,13 @@ static int amdgpu_device_doorbell_init(struct
>> amdgpu_device *adev)
>> adev->doorbell.doorbell_aper_size =
>> pci_resource_len(adev->pdev, 2);
>> if (adev->enable_mes) {
>> - adev->doorbell.num_doorbells =
>> + adev->mman.num_kernel_doorbells =
>> adev->doorbell.doorbell_aper_size / sizeof(u32);
>> } else {
>> - adev->doorbell.num_doorbells =
>> + adev->mman.num_kernel_doorbells =
>> min_t(u32, adev->doorbell.doorbell_aper_size /
>> sizeof(u32),
>> adev->doorbell_index.max_assignment+1);
>> - if (adev->doorbell.num_doorbells == 0)
>> + if (adev->mman.num_kernel_doorbells == 0)
>> return -EINVAL;
>> /* For Vega, reserve and map two pages on doorbell BAR
>> since SDMA
>> @@ -1068,15 +1068,10 @@ static int amdgpu_device_doorbell_init(struct
>> amdgpu_device *adev)
>> * the max num_doorbells should + 1 page (0x400 in dword)
>> */
>> if (adev->asic_type >= CHIP_VEGA10)
>> - adev->doorbell.num_doorbells += 0x400;
>> + adev->mman.num_kernel_doorbells += 0x400;
>> }
>> - adev->mman.doorbell_aper_base_kaddr =
>> ioremap(adev->doorbell.doorbell_aper_base,
>> - adev->doorbell.num_doorbells *
>> - sizeof(u32));
>> - if (adev->mman.doorbell_aper_base_kaddr == NULL)
>> - return -ENOMEM;
>> -
>> + adev->mman.doorbell_kernel_bo_size =
>> adev->mman.num_kernel_doorbells * sizeof(u32);
>
> I would just keep the kernel_bo_size around and make the
> num_kernel_doorbells a local variable.
>
Noted,
- Shashank
> Christian.
>
>> return 0;
>> }
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell.h
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell.h
>> index 526b6b4a86dd..7bdff4f926ad 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell.h
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell.h
>> @@ -28,7 +28,6 @@ struct amdgpu_doorbell {
>> /* doorbell mmio */
>> resource_size_t doorbell_aper_base;
>> resource_size_t doorbell_aper_size;
>> - u32 num_doorbells; /* Number of doorbells actually
>> reserved for amdgpu. */
>> };
>> /* Reserved doorbells for amdgpu (including multimedia).
>
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v2 8/8] drm/amdgpu: start using kernel doorbell bo
2023-02-14 19:28 ` Shashank Sharma
@ 2023-02-15 6:18 ` Christian König
0 siblings, 0 replies; 28+ messages in thread
From: Christian König @ 2023-02-15 6:18 UTC (permalink / raw)
To: Shashank Sharma, amd-gfx; +Cc: alexander.deucher, Shashank Sharma, Arvind.Yadav
Am 14.02.23 um 20:28 schrieb Shashank Sharma:
>
> On 14/02/2023 19:40, Christian König wrote:
>> Am 14.02.23 um 17:15 schrieb Shashank Sharma:
>>> From: Shashank Sharma <contactshashanksharma@gmail.com>
>>>
>>> This patch does the following:
>>>
>>> - Adds new variables like mman.doorbell_bo_size/gpu_addr/cpu_addr.
>>> The cpu_addr ptr will be used now for doorbell read/write from
>>> doorbell BAR.
>>> - Adjusts the existing code to use kernel doorbell BO's size and its
>>> cpu_address.
>>>
>>> Cc: Alex Deucher <alexander.deucher@amd.com>
>>> Cc: Christian Koenig <christian.koenig@amd.com>
>>> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
>>> Signed-off-by: Shashank Sharma <shashank.sharma@amd.com>
>>
>> Maybe squash this one together with the previous patch.
>
> I just split it from the last patch in this series, thought it was too
> scattered and might not be
>
> easy to review :D
Yeah, ok good point as well :D
Christian.
>
>
>>
>> But see below.
>>
>>> ---
>>> drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c | 5 ++-
>>> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 33
>>> +++++++++-----------
>>> drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell.h | 1 -
>>> 3 files changed, 16 insertions(+), 23 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
>>> index 0493c64e9d0a..87f486f522ae 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
>>> @@ -109,11 +109,10 @@ static void
>>> amdgpu_doorbell_get_kfd_info(struct amdgpu_device *adev,
>>> *aperture_base = adev->doorbell.doorbell_aper_base;
>>> *aperture_size = 0;
>>> *start_offset = 0;
>>> - } else if (adev->doorbell.doorbell_aper_size >
>>> adev->doorbell.num_doorbells *
>>> - sizeof(u32)) {
>>> + } else if (adev->doorbell.doorbell_aper_size >
>>> adev->mman.doorbell_kernel_bo_size) {
>>> *aperture_base = adev->doorbell.doorbell_aper_base;
>>> *aperture_size = adev->doorbell.doorbell_aper_size;
>>> - *start_offset = adev->doorbell.num_doorbells * sizeof(u32);
>>> + *start_offset = adev->mman.doorbell_kernel_bo_size;
>>> } else {
>>> *aperture_base = 0;
>>> *aperture_size = 0;
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>> index 43c1b67c2778..fde199434579 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>> @@ -596,8 +596,8 @@ u32 amdgpu_mm_rdoorbell(struct amdgpu_device
>>> *adev, u32 index)
>>> if (amdgpu_device_skip_hw_access(adev))
>>> return 0;
>>> - if (index < adev->doorbell.num_doorbells) {
>>> - return readl(adev->mman.doorbell_aper_base_kaddr + index);
>>> + if (index < adev->mman.num_kernel_doorbells) {
>>> + return readl(adev->mman.doorbell_cpu_addr + index);
>>> } else {
>>> DRM_ERROR("reading beyond doorbell aperture: 0x%08x!\n",
>>> index);
>>> return 0;
>>> @@ -619,8 +619,8 @@ void amdgpu_mm_wdoorbell(struct amdgpu_device
>>> *adev, u32 index, u32 v)
>>> if (amdgpu_device_skip_hw_access(adev))
>>> return;
>>> - if (index < adev->doorbell.num_doorbells) {
>>> - writel(v, adev->mman.doorbell_aper_base_kaddr + index);
>>> + if (index < adev->mman.num_kernel_doorbells) {
>>> + writel(v, adev->mman.doorbell_cpu_addr + index);
>>> } else {
>>> DRM_ERROR("writing beyond doorbell aperture: 0x%08x!\n",
>>> index);
>>> }
>>> @@ -640,8 +640,8 @@ u64 amdgpu_mm_rdoorbell64(struct amdgpu_device
>>> *adev, u32 index)
>>> if (amdgpu_device_skip_hw_access(adev))
>>> return 0;
>>> - if (index < adev->doorbell.num_doorbells) {
>>> - return atomic64_read((atomic64_t
>>> *)(adev->mman.doorbell_aper_base_kaddr + index));
>>> + if (index < adev->mman.num_kernel_doorbells) {
>>> + return atomic64_read((atomic64_t
>>> *)(adev->mman.doorbell_cpu_addr + index));
>>> } else {
>>> DRM_ERROR("reading beyond doorbell aperture: 0x%08x!\n",
>>> index);
>>> return 0;
>>> @@ -663,8 +663,8 @@ void amdgpu_mm_wdoorbell64(struct amdgpu_device
>>> *adev, u32 index, u64 v)
>>> if (amdgpu_device_skip_hw_access(adev))
>>> return;
>>> - if (index < adev->doorbell.num_doorbells) {
>>> - atomic64_set((atomic64_t
>>> *)(adev->mman.doorbell_aper_base_kaddr + index), v);
>>> + if (index < adev->mman.num_kernel_doorbells) {
>>> + atomic64_set((atomic64_t *)(adev->mman.doorbell_cpu_addr +
>>> index), v);
>>> } else {
>>> DRM_ERROR("writing beyond doorbell aperture: 0x%08x!\n",
>>> index);
>>> }
>>> @@ -1037,7 +1037,7 @@ static int amdgpu_device_doorbell_init(struct
>>> amdgpu_device *adev)
>>> if (adev->asic_type < CHIP_BONAIRE) {
>>> adev->doorbell.doorbell_aper_base = 0;
>>> adev->doorbell.doorbell_aper_size = 0;
>>> - adev->doorbell.num_doorbells = 0;
>>> + adev->mman.num_kernel_doorbells = 0;
>>> adev->mman.doorbell_aper_base_kaddr = NULL;
>>> return 0;
>>> }
>>> @@ -1052,13 +1052,13 @@ static int
>>> amdgpu_device_doorbell_init(struct amdgpu_device *adev)
>>> adev->doorbell.doorbell_aper_size =
>>> pci_resource_len(adev->pdev, 2);
>>> if (adev->enable_mes) {
>>> - adev->doorbell.num_doorbells =
>>> + adev->mman.num_kernel_doorbells =
>>> adev->doorbell.doorbell_aper_size / sizeof(u32);
>>> } else {
>>> - adev->doorbell.num_doorbells =
>>> + adev->mman.num_kernel_doorbells =
>>> min_t(u32, adev->doorbell.doorbell_aper_size /
>>> sizeof(u32),
>>> adev->doorbell_index.max_assignment+1);
>>> - if (adev->doorbell.num_doorbells == 0)
>>> + if (adev->mman.num_kernel_doorbells == 0)
>>> return -EINVAL;
>>> /* For Vega, reserve and map two pages on doorbell BAR
>>> since SDMA
>>> @@ -1068,15 +1068,10 @@ static int
>>> amdgpu_device_doorbell_init(struct amdgpu_device *adev)
>>> * the max num_doorbells should + 1 page (0x400 in dword)
>>> */
>>> if (adev->asic_type >= CHIP_VEGA10)
>>> - adev->doorbell.num_doorbells += 0x400;
>>> + adev->mman.num_kernel_doorbells += 0x400;
>>> }
>>> - adev->mman.doorbell_aper_base_kaddr =
>>> ioremap(adev->doorbell.doorbell_aper_base,
>>> - adev->doorbell.num_doorbells *
>>> - sizeof(u32));
>>> - if (adev->mman.doorbell_aper_base_kaddr == NULL)
>>> - return -ENOMEM;
>>> -
>>> + adev->mman.doorbell_kernel_bo_size =
>>> adev->mman.num_kernel_doorbells * sizeof(u32);
>>
>> I would just keep the kernel_bo_size around and make the
>> num_kernel_doorbells a local variable.
>>
> Noted,
>
> - Shashank
>
>> Christian.
>>
>>> return 0;
>>> }
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell.h
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell.h
>>> index 526b6b4a86dd..7bdff4f926ad 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell.h
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell.h
>>> @@ -28,7 +28,6 @@ struct amdgpu_doorbell {
>>> /* doorbell mmio */
>>> resource_size_t doorbell_aper_base;
>>> resource_size_t doorbell_aper_size;
>>> - u32 num_doorbells; /* Number of doorbells
>>> actually reserved for amdgpu. */
>>> };
>>> /* Reserved doorbells for amdgpu (including multimedia).
>>
^ permalink raw reply [flat|nested] 28+ messages in thread