[PATCH 1/1] drm/amdgpu: add param to specify fw bo location for front-door loading

[PATCH 1/1] drm/amdgpu: add param to specify fw bo location for front-door loading

[Le Ma]

This param can help isolating data path issues on new systems in early phase.

Change-Id: I0a972dd74fe2aad6b56628cea32ad72dcd17e283
Signed-off-by: Le Ma <le.ma@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h       | 2 ++
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c   | 5 +++++
 drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c   | 2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c | 3 ++-
 4 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 616b6c911767..9da14436a373 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -254,6 +254,8 @@ extern int amdgpu_agp;
 
 extern int amdgpu_wbrf;
 
+extern int fw_bo_location;
+
 #define AMDGPU_VM_MAX_NUM_CTX			4096
 #define AMDGPU_SG_THRESHOLD			(256*1024*1024)
 #define AMDGPU_WAIT_IDLE_TIMEOUT_IN_MS	        3000
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index 880137774b4e..852cec98ff26 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -210,6 +210,7 @@ int amdgpu_seamless = -1; /* auto */
 uint amdgpu_debug_mask;
 int amdgpu_agp = -1; /* auto */
 int amdgpu_wbrf = -1;
+int fw_bo_location = -1;
 
 static void amdgpu_drv_delayed_reset_work_handler(struct work_struct *work);
 
@@ -989,6 +990,10 @@ MODULE_PARM_DESC(wbrf,
 	"Enable Wifi RFI interference mitigation (0 = disabled, 1 = enabled, -1 = auto(default)");
 module_param_named(wbrf, amdgpu_wbrf, int, 0444);
 
+MODULE_PARM_DESC(fw_bo_location,
+	"location to put firmware bo for frontdoor loading (-1 = auto (default), 0 = on ram, 1 = on vram");
+module_param(fw_bo_location, int, 0644);
+
 /* These devices are not supported by amdgpu.
  * They are supported by the mach64, r128, radeon drivers
  */
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
index 1bf975b8d083..2addbdf88394 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
@@ -466,7 +466,7 @@ static int psp_sw_init(void *handle)
 	}
 
 	ret = amdgpu_bo_create_kernel(adev, PSP_1_MEG, PSP_1_MEG,
-				      amdgpu_sriov_vf(adev) ?
+				      (amdgpu_sriov_vf(adev) || fw_bo_location == 1) ?
 				      AMDGPU_GEM_DOMAIN_VRAM : AMDGPU_GEM_DOMAIN_GTT,
 				      &psp->fw_pri_bo,
 				      &psp->fw_pri_mc_addr,
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c
index b14127429f30..1f67914568f6 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c
@@ -1062,7 +1062,8 @@ int amdgpu_ucode_create_bo(struct amdgpu_device *adev)
 {
 	if (adev->firmware.load_type != AMDGPU_FW_LOAD_DIRECT) {
 		amdgpu_bo_create_kernel(adev, adev->firmware.fw_size, PAGE_SIZE,
-			amdgpu_sriov_vf(adev) ? AMDGPU_GEM_DOMAIN_VRAM : AMDGPU_GEM_DOMAIN_GTT,
+			(amdgpu_sriov_vf(adev) || fw_bo_location == 1) ?
+			AMDGPU_GEM_DOMAIN_VRAM : AMDGPU_GEM_DOMAIN_GTT,
 			&adev->firmware.fw_buf,
 			&adev->firmware.fw_buf_mc,
 			&adev->firmware.fw_buf_ptr);
-- 
2.38.1

RE: [PATCH 1/1] drm/amdgpu: add param to specify fw bo location for front-door loading

[Zhang, Hawking]

[AMD Official Use Only - General]

Reviewed-by: Hawking Zhang <Hawking.Zhang@amd.com>

Regards,
Hawking
-----Original Message-----
From: Ma, Le <Le.Ma@amd.com>
Sent: Wednesday, January 3, 2024 16:07
To: amd-gfx@lists.freedesktop.org
Cc: Zhang, Hawking <Hawking.Zhang@amd.com>; Lazar, Lijo <Lijo.Lazar@amd.com>
Subject: [PATCH 1/1] drm/amdgpu: add param to specify fw bo location for front-door loading

This param can help isolating data path issues on new systems in early phase.

Change-Id: I0a972dd74fe2aad6b56628cea32ad72dcd17e283
Signed-off-by: Le Ma <le.ma@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h       | 2 ++
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c   | 5 +++++
 drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c   | 2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c | 3 ++-
 4 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 616b6c911767..9da14436a373 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -254,6 +254,8 @@ extern int amdgpu_agp;

 extern int amdgpu_wbrf;

+extern int fw_bo_location;
+
 #define AMDGPU_VM_MAX_NUM_CTX                  4096
 #define AMDGPU_SG_THRESHOLD                    (256*1024*1024)
 #define AMDGPU_WAIT_IDLE_TIMEOUT_IN_MS         3000
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index 880137774b4e..852cec98ff26 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -210,6 +210,7 @@ int amdgpu_seamless = -1; /* auto */  uint amdgpu_debug_mask;  int amdgpu_agp = -1; /* auto */  int amdgpu_wbrf = -1;
+int fw_bo_location = -1;

 static void amdgpu_drv_delayed_reset_work_handler(struct work_struct *work);

@@ -989,6 +990,10 @@ MODULE_PARM_DESC(wbrf,
        "Enable Wifi RFI interference mitigation (0 = disabled, 1 = enabled, -1 = auto(default)");  module_param_named(wbrf, amdgpu_wbrf, int, 0444);

+MODULE_PARM_DESC(fw_bo_location,
+       "location to put firmware bo for frontdoor loading (-1 = auto
+(default), 0 = on ram, 1 = on vram"); module_param(fw_bo_location, int,
+0644);
+
 /* These devices are not supported by amdgpu.
  * They are supported by the mach64, r128, radeon drivers
  */
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
index 1bf975b8d083..2addbdf88394 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
@@ -466,7 +466,7 @@ static int psp_sw_init(void *handle)
        }

        ret = amdgpu_bo_create_kernel(adev, PSP_1_MEG, PSP_1_MEG,
-                                     amdgpu_sriov_vf(adev) ?
+                                     (amdgpu_sriov_vf(adev) || fw_bo_location == 1) ?
                                      AMDGPU_GEM_DOMAIN_VRAM : AMDGPU_GEM_DOMAIN_GTT,
                                      &psp->fw_pri_bo,
                                      &psp->fw_pri_mc_addr,
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c
index b14127429f30..1f67914568f6 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c
@@ -1062,7 +1062,8 @@ int amdgpu_ucode_create_bo(struct amdgpu_device *adev)  {
        if (adev->firmware.load_type != AMDGPU_FW_LOAD_DIRECT) {
                amdgpu_bo_create_kernel(adev, adev->firmware.fw_size, PAGE_SIZE,
-                       amdgpu_sriov_vf(adev) ? AMDGPU_GEM_DOMAIN_VRAM : AMDGPU_GEM_DOMAIN_GTT,
+                       (amdgpu_sriov_vf(adev) || fw_bo_location == 1) ?
+                       AMDGPU_GEM_DOMAIN_VRAM : AMDGPU_GEM_DOMAIN_GTT,
                        &adev->firmware.fw_buf,
                        &adev->firmware.fw_buf_mc,
                        &adev->firmware.fw_buf_ptr);
--
2.38.1

Re: [PATCH 1/1] drm/amdgpu: add param to specify fw bo location for front-door loading

[Christian König]

Am 03.01.24 um 09:06 schrieb Le Ma:
> This param can help isolating data path issues on new systems in early phase.
>
> Change-Id: I0a972dd74fe2aad6b56628cea32ad72dcd17e283
> Signed-off-by: Le Ma <le.ma@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu.h       | 2 ++
>   drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c   | 5 +++++
>   drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c   | 2 +-
>   drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c | 3 ++-
>   4 files changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index 616b6c911767..9da14436a373 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -254,6 +254,8 @@ extern int amdgpu_agp;
>   
>   extern int amdgpu_wbrf;
>   
> +extern int fw_bo_location;

Well first of all all extern defines should have an amdgpu_ prefix.

Then absolutely NAK for upstreaming this. This is *not* a parameter an 
end user should be allowed to mess with.

Regards,
Christian.

> +
>   #define AMDGPU_VM_MAX_NUM_CTX			4096
>   #define AMDGPU_SG_THRESHOLD			(256*1024*1024)
>   #define AMDGPU_WAIT_IDLE_TIMEOUT_IN_MS	        3000
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> index 880137774b4e..852cec98ff26 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> @@ -210,6 +210,7 @@ int amdgpu_seamless = -1; /* auto */
>   uint amdgpu_debug_mask;
>   int amdgpu_agp = -1; /* auto */
>   int amdgpu_wbrf = -1;
> +int fw_bo_location = -1;
>   
>   static void amdgpu_drv_delayed_reset_work_handler(struct work_struct *work);
>   
> @@ -989,6 +990,10 @@ MODULE_PARM_DESC(wbrf,
>   	"Enable Wifi RFI interference mitigation (0 = disabled, 1 = enabled, -1 = auto(default)");
>   module_param_named(wbrf, amdgpu_wbrf, int, 0444);
>   
> +MODULE_PARM_DESC(fw_bo_location,
> +	"location to put firmware bo for frontdoor loading (-1 = auto (default), 0 = on ram, 1 = on vram");
> +module_param(fw_bo_location, int, 0644);
> +
>   /* These devices are not supported by amdgpu.
>    * They are supported by the mach64, r128, radeon drivers
>    */
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> index 1bf975b8d083..2addbdf88394 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> @@ -466,7 +466,7 @@ static int psp_sw_init(void *handle)
>   	}
>   
>   	ret = amdgpu_bo_create_kernel(adev, PSP_1_MEG, PSP_1_MEG,
> -				      amdgpu_sriov_vf(adev) ?
> +				      (amdgpu_sriov_vf(adev) || fw_bo_location == 1) ?
>   				      AMDGPU_GEM_DOMAIN_VRAM : AMDGPU_GEM_DOMAIN_GTT,
>   				      &psp->fw_pri_bo,
>   				      &psp->fw_pri_mc_addr,
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c
> index b14127429f30..1f67914568f6 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c
> @@ -1062,7 +1062,8 @@ int amdgpu_ucode_create_bo(struct amdgpu_device *adev)
>   {
>   	if (adev->firmware.load_type != AMDGPU_FW_LOAD_DIRECT) {
>   		amdgpu_bo_create_kernel(adev, adev->firmware.fw_size, PAGE_SIZE,
> -			amdgpu_sriov_vf(adev) ? AMDGPU_GEM_DOMAIN_VRAM : AMDGPU_GEM_DOMAIN_GTT,
> +			(amdgpu_sriov_vf(adev) || fw_bo_location == 1) ?
> +			AMDGPU_GEM_DOMAIN_VRAM : AMDGPU_GEM_DOMAIN_GTT,
>   			&adev->firmware.fw_buf,
>   			&adev->firmware.fw_buf_mc,
>   			&adev->firmware.fw_buf_ptr);