Re: [PATCH 12/13] drm/amdgpu: add domain info in bo_create_kernel_at

2023-02-06 Thread Christian König

Am 06.02.23 um 18:01 schrieb Alex Deucher:

On Mon, Feb 6, 2023 at 11:51 AM Christian König
 wrote:

Am 03.02.23 um 20:08 schrieb Shashank Sharma:

From: Shashank Sharma 

This patch adds a domain input variable for amdgpu_bo_create_kernel_at,
so that it could be used for both VRAM and DOORBELL domains objects. It
also adds supporting code for existing callers.

We should probably drop this one as well.

We just removed the domain from the function because we only have BIOS
reserved regions in VRAM, never anywhere else.

Allocating a doorbell for the kernel is not really an use case for
amdgpu_bo_create_kernel_at().

We just need some way to guarantee that the kernel always gets the
first page.  It's required for SR-IOV compatibility.


That should be guaranteed when we use ttm_range_manager() since that one 
gives you pages in the order you allocate them.


If the first page is already taken then bo_create_kernel_at() won't help 
either, you just get a error returned.


Just allocating and returning the error yourself does the same here.

Christian.



Alex


Christian.


Signed-off-by: Shashank Sharma 
---
   drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 4 ++--
   drivers/gpu/drm/amd/amdgpu/amdgpu_object.h | 2 +-
   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c| 7 +++
   drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c   | 1 +
   4 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index ef1f3106bc69..dec391fa42dc 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -367,7 +367,7 @@ int amdgpu_bo_create_kernel(struct amdgpu_device *adev,
* 0 on success, negative error code otherwise.
*/
   int amdgpu_bo_create_kernel_at(struct amdgpu_device *adev,
-uint64_t offset, uint64_t size,
+uint64_t offset, uint64_t size, uint32_t domain,
  struct amdgpu_bo **bo_ptr, void **cpu_addr)
   {
   struct ttm_operation_ctx ctx = { false, false };
@@ -378,7 +378,7 @@ int amdgpu_bo_create_kernel_at(struct amdgpu_device *adev,
   size = ALIGN(size, PAGE_SIZE);

   r = amdgpu_bo_create_reserved(adev, size, PAGE_SIZE,
-   AMDGPU_GEM_DOMAIN_VRAM, bo_ptr, NULL,
+   domain, bo_ptr, NULL,
 cpu_addr);
   if (r)
   return r;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
index bf9759758f0d..b2b7e55ac486 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
@@ -284,7 +284,7 @@ int amdgpu_bo_create_kernel(struct amdgpu_device *adev,
   u32 domain, struct amdgpu_bo **bo_ptr,
   u64 *gpu_addr, void **cpu_addr);
   int amdgpu_bo_create_kernel_at(struct amdgpu_device *adev,
-uint64_t offset, uint64_t size,
+uint64_t offset, uint64_t size, uint32_t domain,
  struct amdgpu_bo **bo_ptr, void **cpu_addr);
   int amdgpu_bo_create_user(struct amdgpu_device *adev,
 struct amdgpu_bo_param *bp,
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 08355f981313..4cec90debe46 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -1591,6 +1591,7 @@ static int amdgpu_ttm_fw_reserve_vram_init(struct 
amdgpu_device *adev)
   return amdgpu_bo_create_kernel_at(adev,
 adev->mman.fw_vram_usage_start_offset,
 adev->mman.fw_vram_usage_size,
+   AMDGPU_GEM_DOMAIN_VRAM,
 >mman.fw_vram_usage_reserved_bo,
 >mman.fw_vram_usage_va);
   }
@@ -1616,6 +1617,7 @@ static int amdgpu_ttm_drv_reserve_vram_init(struct 
amdgpu_device *adev)
   return amdgpu_bo_create_kernel_at(adev,
 adev->mman.drv_vram_usage_start_offset,
 adev->mman.drv_vram_usage_size,
+   AMDGPU_GEM_DOMAIN_VRAM,
 >mman.drv_vram_usage_reserved_bo,
 >mman.drv_vram_usage_va);
   }
@@ -1696,6 +1698,7 @@ static int amdgpu_ttm_reserve_tmr(struct amdgpu_device 
*adev)
   ret = amdgpu_bo_create_kernel_at(adev,
ctx->c2p_train_data_offset,
ctx->train_data_size,
+  AMDGPU_GEM_DOMAIN_VRAM,
>c2p_bo,
NULL);

Re: [PATCH 12/13] drm/amdgpu: add domain info in bo_create_kernel_at

2023-02-06 Thread Alex Deucher
On Mon, Feb 6, 2023 at 11:51 AM Christian König
 wrote:
>
> Am 03.02.23 um 20:08 schrieb Shashank Sharma:
> > From: Shashank Sharma 
> >
> > This patch adds a domain input variable for amdgpu_bo_create_kernel_at,
> > so that it could be used for both VRAM and DOORBELL domains objects. It
> > also adds supporting code for existing callers.
>
> We should probably drop this one as well.
>
> We just removed the domain from the function because we only have BIOS
> reserved regions in VRAM, never anywhere else.
>
> Allocating a doorbell for the kernel is not really an use case for
> amdgpu_bo_create_kernel_at().

We just need some way to guarantee that the kernel always gets the
first page.  It's required for SR-IOV compatibility.

Alex

>
> Christian.
>
> >
> > Signed-off-by: Shashank Sharma 
> > ---
> >   drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 4 ++--
> >   drivers/gpu/drm/amd/amdgpu/amdgpu_object.h | 2 +-
> >   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c| 7 +++
> >   drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c   | 1 +
> >   4 files changed, 11 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c 
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> > index ef1f3106bc69..dec391fa42dc 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> > @@ -367,7 +367,7 @@ int amdgpu_bo_create_kernel(struct amdgpu_device *adev,
> >* 0 on success, negative error code otherwise.
> >*/
> >   int amdgpu_bo_create_kernel_at(struct amdgpu_device *adev,
> > -uint64_t offset, uint64_t size,
> > +uint64_t offset, uint64_t size, uint32_t 
> > domain,
> >  struct amdgpu_bo **bo_ptr, void **cpu_addr)
> >   {
> >   struct ttm_operation_ctx ctx = { false, false };
> > @@ -378,7 +378,7 @@ int amdgpu_bo_create_kernel_at(struct amdgpu_device 
> > *adev,
> >   size = ALIGN(size, PAGE_SIZE);
> >
> >   r = amdgpu_bo_create_reserved(adev, size, PAGE_SIZE,
> > -   AMDGPU_GEM_DOMAIN_VRAM, bo_ptr, NULL,
> > +   domain, bo_ptr, NULL,
> > cpu_addr);
> >   if (r)
> >   return r;
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h 
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> > index bf9759758f0d..b2b7e55ac486 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> > @@ -284,7 +284,7 @@ int amdgpu_bo_create_kernel(struct amdgpu_device *adev,
> >   u32 domain, struct amdgpu_bo **bo_ptr,
> >   u64 *gpu_addr, void **cpu_addr);
> >   int amdgpu_bo_create_kernel_at(struct amdgpu_device *adev,
> > -uint64_t offset, uint64_t size,
> > +uint64_t offset, uint64_t size, uint32_t 
> > domain,
> >  struct amdgpu_bo **bo_ptr, void **cpu_addr);
> >   int amdgpu_bo_create_user(struct amdgpu_device *adev,
> > struct amdgpu_bo_param *bp,
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c 
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> > index 08355f981313..4cec90debe46 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> > @@ -1591,6 +1591,7 @@ static int amdgpu_ttm_fw_reserve_vram_init(struct 
> > amdgpu_device *adev)
> >   return amdgpu_bo_create_kernel_at(adev,
> > 
> > adev->mman.fw_vram_usage_start_offset,
> > adev->mman.fw_vram_usage_size,
> > +   AMDGPU_GEM_DOMAIN_VRAM,
> > 
> > >mman.fw_vram_usage_reserved_bo,
> > >mman.fw_vram_usage_va);
> >   }
> > @@ -1616,6 +1617,7 @@ static int amdgpu_ttm_drv_reserve_vram_init(struct 
> > amdgpu_device *adev)
> >   return amdgpu_bo_create_kernel_at(adev,
> > 
> > adev->mman.drv_vram_usage_start_offset,
> > adev->mman.drv_vram_usage_size,
> > +   AMDGPU_GEM_DOMAIN_VRAM,
> > 
> > >mman.drv_vram_usage_reserved_bo,
> > >mman.drv_vram_usage_va);
> >   }
> > @@ -1696,6 +1698,7 @@ static int amdgpu_ttm_reserve_tmr(struct 
> > amdgpu_device *adev)
> >   ret = amdgpu_bo_create_kernel_at(adev,
> >ctx->c2p_train_data_offset,
> >ctx->train_data_size,
> > +  AMDGPU_GEM_DOMAIN_VRAM,
> >>c2p_bo,
> >NULL);
> >   if (ret) {

Re: [PATCH 12/13] drm/amdgpu: add domain info in bo_create_kernel_at

2023-02-06 Thread Christian König

Am 03.02.23 um 20:08 schrieb Shashank Sharma:

From: Shashank Sharma 

This patch adds a domain input variable for amdgpu_bo_create_kernel_at,
so that it could be used for both VRAM and DOORBELL domains objects. It
also adds supporting code for existing callers.


We should probably drop this one as well.

We just removed the domain from the function because we only have BIOS 
reserved regions in VRAM, never anywhere else.


Allocating a doorbell for the kernel is not really an use case for 
amdgpu_bo_create_kernel_at().


Christian.



Signed-off-by: Shashank Sharma 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 4 ++--
  drivers/gpu/drm/amd/amdgpu/amdgpu_object.h | 2 +-
  drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c| 7 +++
  drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c   | 1 +
  4 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index ef1f3106bc69..dec391fa42dc 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -367,7 +367,7 @@ int amdgpu_bo_create_kernel(struct amdgpu_device *adev,
   * 0 on success, negative error code otherwise.
   */
  int amdgpu_bo_create_kernel_at(struct amdgpu_device *adev,
-  uint64_t offset, uint64_t size,
+  uint64_t offset, uint64_t size, uint32_t domain,
   struct amdgpu_bo **bo_ptr, void **cpu_addr)
  {
struct ttm_operation_ctx ctx = { false, false };
@@ -378,7 +378,7 @@ int amdgpu_bo_create_kernel_at(struct amdgpu_device *adev,
size = ALIGN(size, PAGE_SIZE);
  
  	r = amdgpu_bo_create_reserved(adev, size, PAGE_SIZE,

- AMDGPU_GEM_DOMAIN_VRAM, bo_ptr, NULL,
+ domain, bo_ptr, NULL,
  cpu_addr);
if (r)
return r;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
index bf9759758f0d..b2b7e55ac486 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
@@ -284,7 +284,7 @@ int amdgpu_bo_create_kernel(struct amdgpu_device *adev,
u32 domain, struct amdgpu_bo **bo_ptr,
u64 *gpu_addr, void **cpu_addr);
  int amdgpu_bo_create_kernel_at(struct amdgpu_device *adev,
-  uint64_t offset, uint64_t size,
+  uint64_t offset, uint64_t size, uint32_t domain,
   struct amdgpu_bo **bo_ptr, void **cpu_addr);
  int amdgpu_bo_create_user(struct amdgpu_device *adev,
  struct amdgpu_bo_param *bp,
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 08355f981313..4cec90debe46 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -1591,6 +1591,7 @@ static int amdgpu_ttm_fw_reserve_vram_init(struct 
amdgpu_device *adev)
return amdgpu_bo_create_kernel_at(adev,
  adev->mman.fw_vram_usage_start_offset,
  adev->mman.fw_vram_usage_size,
+ AMDGPU_GEM_DOMAIN_VRAM,
  >mman.fw_vram_usage_reserved_bo,
  >mman.fw_vram_usage_va);
  }
@@ -1616,6 +1617,7 @@ static int amdgpu_ttm_drv_reserve_vram_init(struct 
amdgpu_device *adev)
return amdgpu_bo_create_kernel_at(adev,
  
adev->mman.drv_vram_usage_start_offset,
  adev->mman.drv_vram_usage_size,
+ AMDGPU_GEM_DOMAIN_VRAM,
  
>mman.drv_vram_usage_reserved_bo,
  >mman.drv_vram_usage_va);
  }
@@ -1696,6 +1698,7 @@ static int amdgpu_ttm_reserve_tmr(struct amdgpu_device 
*adev)
ret = amdgpu_bo_create_kernel_at(adev,
 ctx->c2p_train_data_offset,
 ctx->train_data_size,
+AMDGPU_GEM_DOMAIN_VRAM,
 >c2p_bo,
 NULL);
if (ret) {
@@ -1709,6 +1712,7 @@ static int amdgpu_ttm_reserve_tmr(struct amdgpu_device 
*adev)
ret = amdgpu_bo_create_kernel_at(adev,
adev->gmc.real_vram_size - 
adev->mman.discovery_tmr_size,
adev->mman.discovery_tmr_size,
+   AMDGPU_GEM_DOMAIN_VRAM,
>mman.discovery_memory,
NULL);
if (ret) {
@@ -1816,18 +1820,21 @@ int 

[PATCH 12/13] drm/amdgpu: add domain info in bo_create_kernel_at

2023-02-03 Thread Shashank Sharma
From: Shashank Sharma 

This patch adds a domain input variable for amdgpu_bo_create_kernel_at,
so that it could be used for both VRAM and DOORBELL domains objects. It
also adds supporting code for existing callers.

Signed-off-by: Shashank Sharma 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 4 ++--
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.h | 2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c| 7 +++
 drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c   | 1 +
 4 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index ef1f3106bc69..dec391fa42dc 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -367,7 +367,7 @@ int amdgpu_bo_create_kernel(struct amdgpu_device *adev,
  * 0 on success, negative error code otherwise.
  */
 int amdgpu_bo_create_kernel_at(struct amdgpu_device *adev,
-  uint64_t offset, uint64_t size,
+  uint64_t offset, uint64_t size, uint32_t domain,
   struct amdgpu_bo **bo_ptr, void **cpu_addr)
 {
struct ttm_operation_ctx ctx = { false, false };
@@ -378,7 +378,7 @@ int amdgpu_bo_create_kernel_at(struct amdgpu_device *adev,
size = ALIGN(size, PAGE_SIZE);
 
r = amdgpu_bo_create_reserved(adev, size, PAGE_SIZE,
- AMDGPU_GEM_DOMAIN_VRAM, bo_ptr, NULL,
+ domain, bo_ptr, NULL,
  cpu_addr);
if (r)
return r;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
index bf9759758f0d..b2b7e55ac486 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
@@ -284,7 +284,7 @@ int amdgpu_bo_create_kernel(struct amdgpu_device *adev,
u32 domain, struct amdgpu_bo **bo_ptr,
u64 *gpu_addr, void **cpu_addr);
 int amdgpu_bo_create_kernel_at(struct amdgpu_device *adev,
-  uint64_t offset, uint64_t size,
+  uint64_t offset, uint64_t size, uint32_t domain,
   struct amdgpu_bo **bo_ptr, void **cpu_addr);
 int amdgpu_bo_create_user(struct amdgpu_device *adev,
  struct amdgpu_bo_param *bp,
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 08355f981313..4cec90debe46 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -1591,6 +1591,7 @@ static int amdgpu_ttm_fw_reserve_vram_init(struct 
amdgpu_device *adev)
return amdgpu_bo_create_kernel_at(adev,
  adev->mman.fw_vram_usage_start_offset,
  adev->mman.fw_vram_usage_size,
+ AMDGPU_GEM_DOMAIN_VRAM,
  >mman.fw_vram_usage_reserved_bo,
  >mman.fw_vram_usage_va);
 }
@@ -1616,6 +1617,7 @@ static int amdgpu_ttm_drv_reserve_vram_init(struct 
amdgpu_device *adev)
return amdgpu_bo_create_kernel_at(adev,
  
adev->mman.drv_vram_usage_start_offset,
  adev->mman.drv_vram_usage_size,
+ AMDGPU_GEM_DOMAIN_VRAM,
  
>mman.drv_vram_usage_reserved_bo,
  >mman.drv_vram_usage_va);
 }
@@ -1696,6 +1698,7 @@ static int amdgpu_ttm_reserve_tmr(struct amdgpu_device 
*adev)
ret = amdgpu_bo_create_kernel_at(adev,
 ctx->c2p_train_data_offset,
 ctx->train_data_size,
+AMDGPU_GEM_DOMAIN_VRAM,
 >c2p_bo,
 NULL);
if (ret) {
@@ -1709,6 +1712,7 @@ static int amdgpu_ttm_reserve_tmr(struct amdgpu_device 
*adev)
ret = amdgpu_bo_create_kernel_at(adev,
adev->gmc.real_vram_size - 
adev->mman.discovery_tmr_size,
adev->mman.discovery_tmr_size,
+   AMDGPU_GEM_DOMAIN_VRAM,
>mman.discovery_memory,
NULL);
if (ret) {
@@ -1816,18 +1820,21 @@ int amdgpu_ttm_init(struct amdgpu_device *adev)
 * avoid display artifacts while transitioning between pre-OS
 * and driver.  */
r = amdgpu_bo_create_kernel_at(adev, 0, adev->mman.stolen_vga_size,
+  AMDGPU_GEM_DOMAIN_VRAM,