Re: [PATCH] Revert "drm/amdgpu: drop runtime pm disablement quirk on several sienna cichlid cards"

[Lazar, Lijo]

On 9/7/2022 10:23 AM, Guchun Chen wrote:

This reverts commit e2994d23d8afa2fb465fdb8cf544b736f67ab8ba.

Frequent BACO enter/exit will cause EMI failure, so disable runtime PM
on these server SKUs.



Apart from this, any BACO entry/exit in quick succession could cause the 
same failure. If BACO is used for reset in these SKUs, that also may not 
work.


Thanks,
Lijo


Signed-off-by: Guchun Chen 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 13 +
  1 file changed, 13 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
index 1369c25448dc..4f6473faaf24 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
@@ -43,6 +43,17 @@
  #include "amdgpu_display.h"
  #include "amdgpu_ras.h"
  
+static void amdgpu_runtime_pm_quirk(struct amdgpu_device *adev)

+{
+   /*
+* Add below quirk on several sienna_cichlid cards to disable
+* runtime pm to fix EMI failures.
+*/
+   if (((adev->pdev->device == 0x73A1) && (adev->pdev->revision == 0x00)) 
||
+   ((adev->pdev->device == 0x73BF) && (adev->pdev->revision == 0xCF)))
+   adev->pm.rpm_mode = AMDGPU_RUNPM_NONE;
+}
+
  void amdgpu_unregister_gpu_instance(struct amdgpu_device *adev)
  {
struct amdgpu_gpu_instance *gpu_instance;
@@ -176,6 +187,8 @@ int amdgpu_driver_load_kms(struct amdgpu_device *adev, 
unsigned long flags)
break;
}
  
+		amdgpu_runtime_pm_quirk(adev);

+
if (adev->pm.rpm_mode == AMDGPU_RUNPM_BACO)
dev_info(adev->dev, "Using BACO for runtime pm\n");
}

RE: [PATCH V2] drm/amdgpu: Adjust removal control flow for smu v13_0_2

[Chai, Thomas]

[AMD Official Use Only - General]

Yes, I will add the sequence adjustment to the comment.


-
Best Regards,
Thomas

From: Zhang, Hawking 
Sent: Wednesday, September 7, 2022 11:42 AM
To: Chai, Thomas ; amd-gfx@lists.freedesktop.org
Cc: Zhou1, Tao ; Wang, Yang(Kevin) 
Subject: Re: [PATCH V2] drm/amdgpu: Adjust removal control flow for smu v13_0_2

Thanks.

Can you please share more details to help me understand the sequence adjustment 
in suspend?

Regards,
Hawking

From: Chai, Thomas mailto:yipeng.c...@amd.com>>
Date: Wednesday, September 7, 2022 at 11:29
To: Zhang, Hawking mailto:hawking.zh...@amd.com>>, 
amd-gfx@lists.freedesktop.org 
mailto:amd-gfx@lists.freedesktop.org>>
Cc: Zhou1, Tao mailto:tao.zh...@amd.com>>, Wang, Yang(Kevin) 
mailto:kevinyang.w...@amd.com>>
Subject: RE: [PATCH V2] drm/amdgpu: Adjust removal control flow for smu v13_0_2

[AMD Official Use Only - General]

OK, I will update patch.


-
Best Regards,
Thomas

From: Zhang, Hawking mailto:hawking.zh...@amd.com>>
Sent: Wednesday, September 7, 2022 10:40 AM
To: Chai, Thomas mailto:yipeng.c...@amd.com>>; 
amd-gfx@lists.freedesktop.org
Cc: Chai, Thomas mailto:yipeng.c...@amd.com>>; Zhou1, Tao 
mailto:tao.zh...@amd.com>>; Wang, Yang(Kevin) 
mailto:kevinyang.w...@amd.com>>
Subject: Re: [PATCH V2] drm/amdgpu: Adjust removal control flow for smu v13_0_2


[AMD Official Use Only - General]

+static void amdgpu_device_gpu_reset(struct amdgpu_device *adev)
+{
+   struct amdgpu_reset_context reset_context;
+
+   memset(_context, 0, sizeof(reset_context));
+   reset_context.method = AMD_RESET_METHOD_NONE;
+   reset_context.reset_req_dev = adev;
+   set_bit(AMDGPU_NEED_FULL_RESET, _context.flags);
+   set_bit(AMDGPU_RESET_FOR_DEVICE_REMOVE, _context.flags);
+
+   amdgpu_device_gpu_recover(adev, NULL, _context);
+}

This wrapper is kind of confusing. Let's keep amdgpu_device_gpu_recover as the 
only entry point for recovery handling. If possible, please drop this wrapper,  
initialize reset_context and call amdgpu_device_gpu_recover directly


+   /* If in_remove is true, psp_hw_fini should be executed after
+*  psp_suspend to free psp shared buffers.
+*/
+   if (adev->in_remove && (adev->ip_blocks[i].version->type == 
AMD_IP_BLOCK_TYPE_PSP))
+   continue;
Can you please share more details to help me understand the sequence adjustment 
here?

Regards,
Hawking

From: Chai, Thomas mailto:yipeng.c...@amd.com>>
Date: Tuesday, September 6, 2022 at 15:48
To: amd-gfx@lists.freedesktop.org 
mailto:amd-gfx@lists.freedesktop.org>>
Cc: Chai, Thomas mailto:yipeng.c...@amd.com>>, Zhang, 
Hawking mailto:hawking.zh...@amd.com>>, Zhou1, Tao 
mailto:tao.zh...@amd.com>>, Wang, Yang(Kevin) 
mailto:kevinyang.w...@amd.com>>, Chai, Thomas 
mailto:yipeng.c...@amd.com>>
Subject: [PATCH V2] drm/amdgpu: Adjust removal control flow for smu v13_0_2
Adjust removal control flow for smu v13_0_2:
   During amdgpu uninstallation, when removing the first
device, the kernel needs to first send a mode1reset message
to all gpu devices. Otherwise, smu initialization will fail
the next time amdgpu is installed.

V2:
1. Update commit comments.
2. Remove the global variable amdgpu_device_remove_cnt
   and add a variable to the structure amdgpu_hive_info.
3. Use hive to detect the first removed device instead of
   a global variable.

Signed-off-by: YiPeng Chai mailto:yipeng.c...@amd.com>>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h|  3 ++
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 40 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c| 35 +++
 drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c| 16 -
 drivers/gpu/drm/amd/amdgpu/amdgpu_reset.h  |  1 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.h   |  1 +
 drivers/gpu/drm/amd/pm/amdgpu_pm.c |  6 +++-
 7 files changed, 99 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 79bb6fd83094..465295318830 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -997,6 +997,9 @@ struct amdgpu_device {
 boolin_s4;
 boolin_s0ix;

+   /* uninstall */
+   boolin_remove;
+
 enum pp_mp1_state   mp1_state;
 struct amdgpu_doorbell_index doorbell_index;

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 62b26f0e37b0..1402717673f7 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -2999,6 +2999,13 @@ static int amdgpu_device_ip_suspend_phase2(struct 
amdgpu_device *adev)
 DRM_ERROR("suspend of 

RE: [PATCH] Revert "drm/amdgpu: drop runtime pm disablement quirk on several sienna cichlid cards"

[Quan, Evan]

[AMD Official Use Only - General]

Reviewed-by: Evan Quan 

> -Original Message-
> From: Chen, Guchun 
> Sent: Wednesday, September 7, 2022 12:53 PM
> To: amd-gfx@lists.freedesktop.org; Deucher, Alexander
> ; Zhang, Hawking
> ; Lazar, Lijo ; Quan, Evan
> ; Feng, Kenneth 
> Cc: Chen, Guchun 
> Subject: [PATCH] Revert "drm/amdgpu: drop runtime pm disablement quirk
> on several sienna cichlid cards"
> 
> This reverts commit e2994d23d8afa2fb465fdb8cf544b736f67ab8ba.
> 
> Frequent BACO enter/exit will cause EMI failure, so disable runtime PM
> on these server SKUs.
> 
> Signed-off-by: Guchun Chen 
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 13 +
>  1 file changed, 13 insertions(+)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> index 1369c25448dc..4f6473faaf24 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> @@ -43,6 +43,17 @@
>  #include "amdgpu_display.h"
>  #include "amdgpu_ras.h"
> 
> +static void amdgpu_runtime_pm_quirk(struct amdgpu_device *adev)
> +{
> + /*
> +  * Add below quirk on several sienna_cichlid cards to disable
> +  * runtime pm to fix EMI failures.
> +  */
> + if (((adev->pdev->device == 0x73A1) && (adev->pdev->revision ==
> 0x00)) ||
> + ((adev->pdev->device == 0x73BF) && (adev->pdev->revision ==
> 0xCF)))
> + adev->pm.rpm_mode = AMDGPU_RUNPM_NONE;
> +}
> +
>  void amdgpu_unregister_gpu_instance(struct amdgpu_device *adev)
>  {
>   struct amdgpu_gpu_instance *gpu_instance;
> @@ -176,6 +187,8 @@ int amdgpu_driver_load_kms(struct amdgpu_device
> *adev, unsigned long flags)
>   break;
>   }
> 
> + amdgpu_runtime_pm_quirk(adev);
> +
>   if (adev->pm.rpm_mode == AMDGPU_RUNPM_BACO)
>   dev_info(adev->dev, "Using BACO for runtime
> pm\n");
>   }
> --
> 2.25.1

[PATCH] Revert "drm/amdgpu: drop runtime pm disablement quirk on several sienna cichlid cards"

[Guchun Chen]

This reverts commit e2994d23d8afa2fb465fdb8cf544b736f67ab8ba.

Frequent BACO enter/exit will cause EMI failure, so disable runtime PM
on these server SKUs.

Signed-off-by: Guchun Chen 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 13 +
 1 file changed, 13 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
index 1369c25448dc..4f6473faaf24 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
@@ -43,6 +43,17 @@
 #include "amdgpu_display.h"
 #include "amdgpu_ras.h"
 
+static void amdgpu_runtime_pm_quirk(struct amdgpu_device *adev)
+{
+   /*
+* Add below quirk on several sienna_cichlid cards to disable
+* runtime pm to fix EMI failures.
+*/
+   if (((adev->pdev->device == 0x73A1) && (adev->pdev->revision == 0x00)) 
||
+   ((adev->pdev->device == 0x73BF) && (adev->pdev->revision == 0xCF)))
+   adev->pm.rpm_mode = AMDGPU_RUNPM_NONE;
+}
+
 void amdgpu_unregister_gpu_instance(struct amdgpu_device *adev)
 {
struct amdgpu_gpu_instance *gpu_instance;
@@ -176,6 +187,8 @@ int amdgpu_driver_load_kms(struct amdgpu_device *adev, 
unsigned long flags)
break;
}
 
+   amdgpu_runtime_pm_quirk(adev);
+
if (adev->pm.rpm_mode == AMDGPU_RUNPM_BACO)
dev_info(adev->dev, "Using BACO for runtime pm\n");
}
-- 
2.25.1

RE: [PATCH] drm/amdgpu: getting fan speed pwm for vega10 properly

[Quan, Evan]

[Public]

Thanks Alex and Yury.
The changes seem reasonable to me. Feel free to add my RB: Reviewed-by: Evan 
Quan 

BR
Evan
From: Deucher, Alexander 
Sent: Tuesday, September 6, 2022 11:05 PM
To: Yury Zhuravlev ; amd-gfx@lists.freedesktop.org; Quan, 
Evan ; Feng, Kenneth 
Subject: Re: [PATCH] drm/amdgpu: getting fan speed pwm for vega10 properly


[Public]

@Quan, Evan, @Feng, 
Kenneth can you take a look?

Thanks,

Alex

From: amd-gfx 
mailto:amd-gfx-boun...@lists.freedesktop.org>>
 on behalf of Yury Zhuravlev mailto:stalk...@gmail.com>>
Sent: Friday, September 2, 2022 1:24 PM
To: amd-gfx@lists.freedesktop.org 
mailto:amd-gfx@lists.freedesktop.org>>
Subject: [PATCH] drm/amdgpu: getting fan speed pwm for vega10 properly

Hello,

During the setup, the fan manager 
https://github.com/markusressel/fan2go
 I found that my Vega56 was not working correctly. This fan manager expects 
what read PWM value should be the same as you wrote before, but it's not the 
case. PWM value was volatile, and what is more critical, if I wrote 200, after 
reading I saw ~70-100, which is very confusing.
After that, I started reading the amdgpu driver, and how fan speed works, and I 
found what PWM value was calculated from RPM speed and not correct for my case 
(different BIOS or fan configuration?).
Because it looked wrong, I started looking into different implementations and 
found that Vega20 used mmCG_FDO_CTRL1 and mmCG_THERMAL_STATUS registers to 
calculate the PWM value.
I also checked how we set PWM for Vega10 and found the same registers. After 
that, I copy-pasted the function from Vega20 to Vega10, and it started working 
much better. It still has some fluctuation, but as I understand, this behavior 
is expected.

I have no in-depth information about amdgpu, and the original function may have 
been for some reason (maybe for some broken BIOS?), but I suppose somebody 
forgot to backport this code after prototype implementation.

It would be my first patch here. Sorry if I skipped some procedures, will be 
appreciated it if you help me.
Also, sorry for the patch in the attachment, I have not been using any mail 
programs for the last six years, only web clients, and it's strange to do it 
nowadays (PRs much more common...).

Regards,

Re: [PATCH V2] drm/amdgpu: Adjust removal control flow for smu v13_0_2

[Zhang, Hawking]

Thanks.

Can you please share more details to help me understand the sequence adjustment 
in suspend?

Regards,
Hawking

From: Chai, Thomas 
Date: Wednesday, September 7, 2022 at 11:29
To: Zhang, Hawking , amd-gfx@lists.freedesktop.org 

Cc: Zhou1, Tao , Wang, Yang(Kevin) 
Subject: RE: [PATCH V2] drm/amdgpu: Adjust removal control flow for smu v13_0_2

[AMD Official Use Only - General]

OK, I will update patch.


-
Best Regards,
Thomas

From: Zhang, Hawking 
Sent: Wednesday, September 7, 2022 10:40 AM
To: Chai, Thomas ; amd-gfx@lists.freedesktop.org
Cc: Chai, Thomas ; Zhou1, Tao ; Wang, 
Yang(Kevin) 
Subject: Re: [PATCH V2] drm/amdgpu: Adjust removal control flow for smu v13_0_2


[AMD Official Use Only - General]

+static void amdgpu_device_gpu_reset(struct amdgpu_device *adev)
+{
+   struct amdgpu_reset_context reset_context;
+
+   memset(_context, 0, sizeof(reset_context));
+   reset_context.method = AMD_RESET_METHOD_NONE;
+   reset_context.reset_req_dev = adev;
+   set_bit(AMDGPU_NEED_FULL_RESET, _context.flags);
+   set_bit(AMDGPU_RESET_FOR_DEVICE_REMOVE, _context.flags);
+
+   amdgpu_device_gpu_recover(adev, NULL, _context);
+}

This wrapper is kind of confusing. Let’s keep amdgpu_device_gpu_recover as the 
only entry point for recovery handling. If possible, please drop this wrapper,  
initialize reset_context and call amdgpu_device_gpu_recover directly


+   /* If in_remove is true, psp_hw_fini should be executed after
+*  psp_suspend to free psp shared buffers.
+*/
+   if (adev->in_remove && (adev->ip_blocks[i].version->type == 
AMD_IP_BLOCK_TYPE_PSP))
+   continue;
Can you please share more details to help me understand the sequence adjustment 
here?

Regards,
Hawking

From: Chai, Thomas mailto:yipeng.c...@amd.com>>
Date: Tuesday, September 6, 2022 at 15:48
To: amd-gfx@lists.freedesktop.org 
mailto:amd-gfx@lists.freedesktop.org>>
Cc: Chai, Thomas mailto:yipeng.c...@amd.com>>, Zhang, 
Hawking mailto:hawking.zh...@amd.com>>, Zhou1, Tao 
mailto:tao.zh...@amd.com>>, Wang, Yang(Kevin) 
mailto:kevinyang.w...@amd.com>>, Chai, Thomas 
mailto:yipeng.c...@amd.com>>
Subject: [PATCH V2] drm/amdgpu: Adjust removal control flow for smu v13_0_2
Adjust removal control flow for smu v13_0_2:
   During amdgpu uninstallation, when removing the first
device, the kernel needs to first send a mode1reset message
to all gpu devices. Otherwise, smu initialization will fail
the next time amdgpu is installed.

V2:
1. Update commit comments.
2. Remove the global variable amdgpu_device_remove_cnt
   and add a variable to the structure amdgpu_hive_info.
3. Use hive to detect the first removed device instead of
   a global variable.

Signed-off-by: YiPeng Chai mailto:yipeng.c...@amd.com>>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h|  3 ++
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 40 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c| 35 +++
 drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c| 16 -
 drivers/gpu/drm/amd/amdgpu/amdgpu_reset.h  |  1 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.h   |  1 +
 drivers/gpu/drm/amd/pm/amdgpu_pm.c |  6 +++-
 7 files changed, 99 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 79bb6fd83094..465295318830 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -997,6 +997,9 @@ struct amdgpu_device {
 boolin_s4;
 boolin_s0ix;

+   /* uninstall */
+   boolin_remove;
+
 enum pp_mp1_state   mp1_state;
 struct amdgpu_doorbell_index doorbell_index;

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 62b26f0e37b0..1402717673f7 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -2999,6 +2999,13 @@ static int amdgpu_device_ip_suspend_phase2(struct 
amdgpu_device *adev)
 DRM_ERROR("suspend of IP block <%s> failed %d\n",
   adev->ip_blocks[i].version->funcs->name, r);
 }
+
+   /* If in_remove is true, psp_hw_fini should be executed after
+*  psp_suspend to free psp shared buffers.
+*/
+   if (adev->in_remove && (adev->ip_blocks[i].version->type == 
AMD_IP_BLOCK_TYPE_PSP))
+   continue;
+
 adev->ip_blocks[i].status.hw = false;
 /* handle putting the SMC in the appropriate state */
 if(!amdgpu_sriov_vf(adev)){
@@ -4739,6 +4746,7 @@ int amdgpu_do_asic_reset(struct list_head 
*device_list_handle,
 struct amdgpu_device *tmp_adev = NULL;
 

RE: [PATCH V2] drm/amdgpu: Adjust removal control flow for smu v13_0_2

[Chai, Thomas]

[AMD Official Use Only - General]

OK, I will update patch.


-
Best Regards,
Thomas

From: Zhang, Hawking 
Sent: Wednesday, September 7, 2022 10:40 AM
To: Chai, Thomas ; amd-gfx@lists.freedesktop.org
Cc: Chai, Thomas ; Zhou1, Tao ; Wang, 
Yang(Kevin) 
Subject: Re: [PATCH V2] drm/amdgpu: Adjust removal control flow for smu v13_0_2


[AMD Official Use Only - General]

+static void amdgpu_device_gpu_reset(struct amdgpu_device *adev)
+{
+   struct amdgpu_reset_context reset_context;
+
+   memset(_context, 0, sizeof(reset_context));
+   reset_context.method = AMD_RESET_METHOD_NONE;
+   reset_context.reset_req_dev = adev;
+   set_bit(AMDGPU_NEED_FULL_RESET, _context.flags);
+   set_bit(AMDGPU_RESET_FOR_DEVICE_REMOVE, _context.flags);
+
+   amdgpu_device_gpu_recover(adev, NULL, _context);
+}

This wrapper is kind of confusing. Let's keep amdgpu_device_gpu_recover as the 
only entry point for recovery handling. If possible, please drop this wrapper,  
initialize reset_context and call amdgpu_device_gpu_recover directly


+   /* If in_remove is true, psp_hw_fini should be executed after
+*  psp_suspend to free psp shared buffers.
+*/
+   if (adev->in_remove && (adev->ip_blocks[i].version->type == 
AMD_IP_BLOCK_TYPE_PSP))
+   continue;
Can you please share more details to help me understand the sequence adjustment 
here?

Regards,
Hawking

From: Chai, Thomas mailto:yipeng.c...@amd.com>>
Date: Tuesday, September 6, 2022 at 15:48
To: amd-gfx@lists.freedesktop.org 
mailto:amd-gfx@lists.freedesktop.org>>
Cc: Chai, Thomas mailto:yipeng.c...@amd.com>>, Zhang, 
Hawking mailto:hawking.zh...@amd.com>>, Zhou1, Tao 
mailto:tao.zh...@amd.com>>, Wang, Yang(Kevin) 
mailto:kevinyang.w...@amd.com>>, Chai, Thomas 
mailto:yipeng.c...@amd.com>>
Subject: [PATCH V2] drm/amdgpu: Adjust removal control flow for smu v13_0_2
Adjust removal control flow for smu v13_0_2:
   During amdgpu uninstallation, when removing the first
device, the kernel needs to first send a mode1reset message
to all gpu devices. Otherwise, smu initialization will fail
the next time amdgpu is installed.

V2:
1. Update commit comments.
2. Remove the global variable amdgpu_device_remove_cnt
   and add a variable to the structure amdgpu_hive_info.
3. Use hive to detect the first removed device instead of
   a global variable.

Signed-off-by: YiPeng Chai mailto:yipeng.c...@amd.com>>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h|  3 ++
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 40 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c| 35 +++
 drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c| 16 -
 drivers/gpu/drm/amd/amdgpu/amdgpu_reset.h  |  1 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.h   |  1 +
 drivers/gpu/drm/amd/pm/amdgpu_pm.c |  6 +++-
 7 files changed, 99 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 79bb6fd83094..465295318830 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -997,6 +997,9 @@ struct amdgpu_device {
 boolin_s4;
 boolin_s0ix;

+   /* uninstall */
+   boolin_remove;
+
 enum pp_mp1_state   mp1_state;
 struct amdgpu_doorbell_index doorbell_index;

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 62b26f0e37b0..1402717673f7 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -2999,6 +2999,13 @@ static int amdgpu_device_ip_suspend_phase2(struct 
amdgpu_device *adev)
 DRM_ERROR("suspend of IP block <%s> failed %d\n",
   adev->ip_blocks[i].version->funcs->name, r);
 }
+
+   /* If in_remove is true, psp_hw_fini should be executed after
+*  psp_suspend to free psp shared buffers.
+*/
+   if (adev->in_remove && (adev->ip_blocks[i].version->type == 
AMD_IP_BLOCK_TYPE_PSP))
+   continue;
+
 adev->ip_blocks[i].status.hw = false;
 /* handle putting the SMC in the appropriate state */
 if(!amdgpu_sriov_vf(adev)){
@@ -4739,6 +4746,7 @@ int amdgpu_do_asic_reset(struct list_head 
*device_list_handle,
 struct amdgpu_device *tmp_adev = NULL;
 bool need_full_reset, skip_hw_reset, vram_lost = false;
 int r = 0;
+   bool gpu_reset_for_dev_remove = 0;

 /* Try reset handler method first */
 tmp_adev = list_first_entry(device_list_handle, struct amdgpu_device,
@@ -4758,6 +4766,10 @@ int amdgpu_do_asic_reset(struct list_head 
*device_list_handle,
 

Re: [PATCH V2] drm/amdgpu: Adjust removal control flow for smu v13_0_2

[Zhang, Hawking]

[AMD Official Use Only - General]

+static void amdgpu_device_gpu_reset(struct amdgpu_device *adev)
+{
+   struct amdgpu_reset_context reset_context;
+
+   memset(_context, 0, sizeof(reset_context));
+   reset_context.method = AMD_RESET_METHOD_NONE;
+   reset_context.reset_req_dev = adev;
+   set_bit(AMDGPU_NEED_FULL_RESET, _context.flags);
+   set_bit(AMDGPU_RESET_FOR_DEVICE_REMOVE, _context.flags);
+
+   amdgpu_device_gpu_recover(adev, NULL, _context);
+}

This wrapper is kind of confusing. Let’s keep amdgpu_device_gpu_recover as the 
only entry point for recovery handling. If possible, please drop this wrapper,  
initialize reset_context and call amdgpu_device_gpu_recover directly


+   /* If in_remove is true, psp_hw_fini should be executed after
+*  psp_suspend to free psp shared buffers.
+*/
+   if (adev->in_remove && (adev->ip_blocks[i].version->type == 
AMD_IP_BLOCK_TYPE_PSP))
+   continue;

Can you please share more details to help me understand the sequence adjustment 
here?

Regards,
Hawking

From: Chai, Thomas 
Date: Tuesday, September 6, 2022 at 15:48
To: amd-gfx@lists.freedesktop.org 
Cc: Chai, Thomas , Zhang, Hawking , 
Zhou1, Tao , Wang, Yang(Kevin) , 
Chai, Thomas 
Subject: [PATCH V2] drm/amdgpu: Adjust removal control flow for smu v13_0_2
Adjust removal control flow for smu v13_0_2:
   During amdgpu uninstallation, when removing the first
device, the kernel needs to first send a mode1reset message
to all gpu devices. Otherwise, smu initialization will fail
the next time amdgpu is installed.

V2:
1. Update commit comments.
2. Remove the global variable amdgpu_device_remove_cnt
   and add a variable to the structure amdgpu_hive_info.
3. Use hive to detect the first removed device instead of
   a global variable.

Signed-off-by: YiPeng Chai 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h|  3 ++
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 40 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c| 35 +++
 drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c| 16 -
 drivers/gpu/drm/amd/amdgpu/amdgpu_reset.h  |  1 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.h   |  1 +
 drivers/gpu/drm/amd/pm/amdgpu_pm.c |  6 +++-
 7 files changed, 99 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 79bb6fd83094..465295318830 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -997,6 +997,9 @@ struct amdgpu_device {
 boolin_s4;
 boolin_s0ix;

+   /* uninstall */
+   boolin_remove;
+
 enum pp_mp1_state   mp1_state;
 struct amdgpu_doorbell_index doorbell_index;

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 62b26f0e37b0..1402717673f7 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -2999,6 +2999,13 @@ static int amdgpu_device_ip_suspend_phase2(struct 
amdgpu_device *adev)
 DRM_ERROR("suspend of IP block <%s> failed %d\n",
   adev->ip_blocks[i].version->funcs->name, r);
 }
+
+   /* If in_remove is true, psp_hw_fini should be executed after
+*  psp_suspend to free psp shared buffers.
+*/
+   if (adev->in_remove && (adev->ip_blocks[i].version->type == 
AMD_IP_BLOCK_TYPE_PSP))
+   continue;
+
 adev->ip_blocks[i].status.hw = false;
 /* handle putting the SMC in the appropriate state */
 if(!amdgpu_sriov_vf(adev)){
@@ -4739,6 +4746,7 @@ int amdgpu_do_asic_reset(struct list_head 
*device_list_handle,
 struct amdgpu_device *tmp_adev = NULL;
 bool need_full_reset, skip_hw_reset, vram_lost = false;
 int r = 0;
+   bool gpu_reset_for_dev_remove = 0;

 /* Try reset handler method first */
 tmp_adev = list_first_entry(device_list_handle, struct amdgpu_device,
@@ -4758,6 +4766,10 @@ int amdgpu_do_asic_reset(struct list_head 
*device_list_handle,
 test_bit(AMDGPU_NEED_FULL_RESET, _context->flags);
 skip_hw_reset = test_bit(AMDGPU_SKIP_HW_RESET, _context->flags);

+   gpu_reset_for_dev_remove =
+   test_bit(AMDGPU_RESET_FOR_DEVICE_REMOVE, _context->flags) 
&&
+   test_bit(AMDGPU_NEED_FULL_RESET, _context->flags);
+
 /*
  * ASIC reset has to be done on all XGMI hive nodes ASAP
  * to allow proper links negotiation in FW (within 1 sec)
@@ -4802,6 +4814,16 @@ int amdgpu_do_asic_reset(struct list_head 
*device_list_handle,
 amdgpu_ras_intr_cleared();
 }

+   /* Fixed the problem 

RE: [PATCH] drm/amdgpu: correct doorbell range/size value for CSDMA_DOORBELL_RANGE

[Du, Xiaojian]

[AMD Official Use Only - General]

Reviewed-by: Xiaojian Du  

Thanks,
Xiaojian

-Original Message-
From: Zhang, Yifan  
Sent: Tuesday, September 6, 2022 1:45 PM
To: amd-gfx@lists.freedesktop.org
Cc: Deucher, Alexander ; Huang, Tim 
; Du, Xiaojian ; Zhang, Yifan 

Subject: [PATCH] drm/amdgpu: correct doorbell range/size value for 
CSDMA_DOORBELL_RANGE

current function mixes CSDMA_DOORBELL_RANGE and SDMA0_DOORBELL_RANGE range/size 
manipulation, while these 2 registers have difference size field mask. Remove 
range/size manipulation for SDMA0_DOORBELL_RANGE.

Signed-off-by: Yifan Zhang 
---
 drivers/gpu/drm/amd/amdgpu/nbio_v7_7.c | 6 --
 1 file changed, 6 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/nbio_v7_7.c 
b/drivers/gpu/drm/amd/amdgpu/nbio_v7_7.c
index 1dc95ef21da6..f30bc826a878 100644
--- a/drivers/gpu/drm/amd/amdgpu/nbio_v7_7.c
+++ b/drivers/gpu/drm/amd/amdgpu/nbio_v7_7.c
@@ -68,12 +68,6 @@ static void nbio_v7_7_sdma_doorbell_range(struct 
amdgpu_device *adev, int instan
doorbell_range = REG_SET_FIELD(doorbell_range,
   GDC0_BIF_CSDMA_DOORBELL_RANGE,
   SIZE, doorbell_size);
-   doorbell_range = REG_SET_FIELD(doorbell_range,
-  GDC0_BIF_SDMA0_DOORBELL_RANGE,
-  OFFSET, doorbell_index);
-   doorbell_range = REG_SET_FIELD(doorbell_range,
-  GDC0_BIF_SDMA0_DOORBELL_RANGE,
-  SIZE, doorbell_size);
} else {
doorbell_range = REG_SET_FIELD(doorbell_range,
   GDC0_BIF_SDMA0_DOORBELL_RANGE,
--
2.37.1

[linux-next:master] BUILD REGRESSION 840126e36e8ff272cb63158646433fa1324533d9

[kernel test robot]

tree/branch: 
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master
branch HEAD: 840126e36e8ff272cb63158646433fa1324533d9  Add linux-next specific 
files for 20220906

Error/Warning reports:

https://lore.kernel.org/linux-mm/202209021204.dclzollr-...@intel.com
https://lore.kernel.org/linux-mm/202209042337.fqi69rlv-...@intel.com
https://lore.kernel.org/linux-mm/202209060229.dvuyxjbv-...@intel.com
https://lore.kernel.org/linux-mm/202209070728.o3stvgvt-...@intel.com
https://lore.kernel.org/llvm/202208312208.hjwleien-...@intel.com

Error/Warning: (recently discovered and may have been fixed)

ERROR: modpost: "__aeabi_ldivmod" [drivers/gpu/drm/vkms/vkms.ko] undefined!
ERROR: modpost: "__aeabi_uldivmod" [drivers/gpu/drm/vkms/vkms.ko] undefined!
ERROR: modpost: "__divdi3" [drivers/gpu/drm/vkms/vkms.ko] undefined!
ERROR: modpost: "__udivdi3" [drivers/gpu/drm/vkms/vkms.ko] undefined!
arm-linux-gnueabi-ld: vkms_formats.c:(.text+0x1e98): undefined reference to 
`__divdi3'
drivers/base/regmap/regmap-mmio.c:221:17: error: implicit declaration of 
function 'writesb'; did you mean 'writeb'? 
[-Werror=implicit-function-declaration]
drivers/base/regmap/regmap-mmio.c:224:17: error: implicit declaration of 
function 'writesw'; did you mean 'writew'? 
[-Werror=implicit-function-declaration]
drivers/base/regmap/regmap-mmio.c:227:17: error: implicit declaration of 
function 'writesl'; did you mean 'writel'? 
[-Werror=implicit-function-declaration]
drivers/base/regmap/regmap-mmio.c:231:17: error: implicit declaration of 
function 'writesq'; did you mean 'writeq'? 
[-Werror=implicit-function-declaration]
drivers/base/regmap/regmap-mmio.c:231:17: error: implicit declaration of 
function 'writesq'; did you mean 'writesl'? 
[-Werror=implicit-function-declaration]
drivers/base/regmap/regmap-mmio.c:358:17: error: implicit declaration of 
function 'readsb'; did you mean 'readb'? [-Werror=implicit-function-declaration]
drivers/base/regmap/regmap-mmio.c:361:17: error: implicit declaration of 
function 'readsw'; did you mean 'readw'? [-Werror=implicit-function-declaration]
drivers/base/regmap/regmap-mmio.c:364:17: error: implicit declaration of 
function 'readsl'; did you mean 'readl'? [-Werror=implicit-function-declaration]
drivers/base/regmap/regmap-mmio.c:368:17: error: implicit declaration of 
function 'readsq'; did you mean 'readq'? [-Werror=implicit-function-declaration]
drivers/base/regmap/regmap-mmio.c:368:17: error: implicit declaration of 
function 'readsq'; did you mean 'readsl'? 
[-Werror=implicit-function-declaration]
drivers/gpu/drm/amd/amdgpu/imu_v11_0_3.c:139:6: warning: no previous prototype 
for 'imu_v11_0_3_program_rlc_ram' [-Wmissing-prototypes]
drivers/gpu/drm/drm_atomic_helper.c:802: warning: expecting prototype for 
drm_atomic_helper_check_wb_connector_state(). Prototype was for 
drm_atomic_helper_check_wb_encoder_state() instead
drivers/gpu/drm/vkms/vkms_formats.c:(.text+0x4b0): undefined reference to 
`__divdi3'
drivers/gpu/drm/vkms/vkms_formats.c:259: undefined reference to `__divdi3'
drivers/gpu/drm/vkms/vkms_plane.c:105 vkms_plane_atomic_update() warn: variable 
dereferenced before check 'fb' (see line 103)
drivers/scsi/qla2xxx/qla_os.c:2854:23: warning: assignment to 'struct 
trace_array *' from 'int' makes pointer from integer without a cast 
[-Wint-conversion]
drivers/usb/host/ehci-platform.c:56:19: warning: 'hcd_name' defined but not 
used [-Wunused-const-variable=]
drivers/usb/host/ohci-platform.c:44:19: warning: 'hcd_name' defined but not 
used [-Wunused-const-variable=]
include/linux/string.h:303:42: warning: 'strnlen' specified bound 4 exceeds 
source size 3 [-Wstringop-overread]
kernel/bpf/memalloc.c:344 bpf_mem_alloc_destroy() error: potentially 
dereferencing uninitialized 'c'.
kismet: WARNING: unmet direct dependencies detected for PINCTRL_IMX when 
selected by PINCTRL_IMX8MM
ld: drivers/gpu/drm/vkms/vkms_formats.c:260: undefined reference to `__divdi3'
ld: vkms_formats.c:(.text+0x47f): undefined reference to `__divdi3'
mips-linux-ld: vkms_formats.c:(.text+0x384): undefined reference to `__divdi3'
mips-linux-ld: vkms_formats.c:(.text.argb_u16_to_RGB565+0xd0): undefined 
reference to `__divdi3'
mipsel-linux-ld: drivers/gpu/drm/vkms/vkms_formats.c:(.text+0x4d8): undefined 
reference to `__divdi3'
sound/soc/codecs/tas2562.c:442:13: warning: variable 'ret' set but not used 
[-Wunused-but-set-variable]
vkms_formats.c:(.text+0x455): undefined reference to `__divdi3'
vkms_formats.c:(.text.argb_u16_to_RGB565+0xb0): undefined reference to 
`__divdi3'

Unverified Error/Warning (likely false positive, please contact us if 
interested):

drivers/usb/host/ehci-atmel.c:28:19: warning: unused variable 'hcd_name' 
[-Wunused-const-variable]
drivers/usb/host/ehci-exynos.c:35:19: warning: unused variable 'hcd_name' 
[-Wunused-const-variable]
drivers/usb/host/ehci-npcm7xx.c:27:19: warning: unused variable 'hcd_name' 
[-Wunused-const-variable]
drivers/usb/

[RFC PATCH] drm/amdgpu: Set MTYPE in PTE based on BO flags

[Felix Kuehling]

The same BO may need different MTYPEs and SNOOP flags in PTEs depending
on its current location relative to the mappint GPU. Setting MTYPEs from
clients ahead of time is not practical for coherent memory sharing.
Instead determine the correct MTYPE for the desired coherence model and
current BO location when updating the page tables.

To maintain backwards compatibility with MTYPE-selection in
AMDGPU_VA_OP_MAP, the coherence-model-based MTYPE selection is only
applied if it chooses an MTYPE other than MTYPE_NC (the default).

Add two AMDGPU_GEM_CREATE_... flags to indicate the coherence model. The
default if no flag is specified is non-coherent (i.e. coarse-grained
coherent at dispatch boundaries).

Update amdgpu_amdkfd_gpuvm.c to use this new method to choose the
correct MTYPE depending on the current memory location.

Suggested-by: Christian König 
Signed-off-by: Felix Kuehling 
---
 .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c  | 59 ++
 drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c|  7 ++
 drivers/gpu/drm/amd/amdgpu/gmc_v11_0.c|  7 ++
 drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c | 77 +--
 include/uapi/drm/amdgpu_drm.h | 14 
 5 files changed, 105 insertions(+), 59 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
index cbd593f7d553..19d72d323355 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
@@ -404,63 +404,15 @@ static int vm_update_pds(struct amdgpu_vm *vm, struct 
amdgpu_sync *sync)
 
 static uint64_t get_pte_flags(struct amdgpu_device *adev, struct kgd_mem *mem)
 {
-   struct amdgpu_device *bo_adev = amdgpu_ttm_adev(mem->bo->tbo.bdev);
-   bool coherent = mem->alloc_flags & KFD_IOC_ALLOC_MEM_FLAGS_COHERENT;
-   bool uncached = mem->alloc_flags & KFD_IOC_ALLOC_MEM_FLAGS_UNCACHED;
-   uint32_t mapping_flags;
-   uint64_t pte_flags;
-   bool snoop = false;
+   uint32_t mapping_flags = AMDGPU_VM_PAGE_READABLE |
+AMDGPU_VM_MTYPE_DEFAULT;
 
-   mapping_flags = AMDGPU_VM_PAGE_READABLE;
if (mem->alloc_flags & KFD_IOC_ALLOC_MEM_FLAGS_WRITABLE)
mapping_flags |= AMDGPU_VM_PAGE_WRITEABLE;
if (mem->alloc_flags & KFD_IOC_ALLOC_MEM_FLAGS_EXECUTABLE)
mapping_flags |= AMDGPU_VM_PAGE_EXECUTABLE;
 
-   switch (adev->asic_type) {
-   case CHIP_ARCTURUS:
-   case CHIP_ALDEBARAN:
-   if (mem->alloc_flags & KFD_IOC_ALLOC_MEM_FLAGS_VRAM) {
-   if (bo_adev == adev) {
-   if (uncached)
-   mapping_flags |= AMDGPU_VM_MTYPE_UC;
-   else if (coherent)
-   mapping_flags |= AMDGPU_VM_MTYPE_CC;
-   else
-   mapping_flags |= AMDGPU_VM_MTYPE_RW;
-   if (adev->asic_type == CHIP_ALDEBARAN &&
-   adev->gmc.xgmi.connected_to_cpu)
-   snoop = true;
-   } else {
-   if (uncached || coherent)
-   mapping_flags |= AMDGPU_VM_MTYPE_UC;
-   else
-   mapping_flags |= AMDGPU_VM_MTYPE_NC;
-   if (amdgpu_xgmi_same_hive(adev, bo_adev))
-   snoop = true;
-   }
-   } else {
-   if (uncached || coherent)
-   mapping_flags |= AMDGPU_VM_MTYPE_UC;
-   else
-   mapping_flags |= AMDGPU_VM_MTYPE_NC;
-   snoop = true;
-   }
-   break;
-   default:
-   if (uncached || coherent)
-   mapping_flags |= AMDGPU_VM_MTYPE_UC;
-   else
-   mapping_flags |= AMDGPU_VM_MTYPE_NC;
-
-   if (!(mem->alloc_flags & KFD_IOC_ALLOC_MEM_FLAGS_VRAM))
-   snoop = true;
-   }
-
-   pte_flags = amdgpu_gem_va_map_flags(adev, mapping_flags);
-   pte_flags |= snoop ? AMDGPU_PTE_SNOOPED : 0;
-
-   return pte_flags;
+   return amdgpu_gem_va_map_flags(adev, mapping_flags);
 }
 
 /**
@@ -1670,6 +1622,11 @@ int amdgpu_amdkfd_gpuvm_alloc_memory_of_gpu(
}
}
 
+   if (flags & KFD_IOC_ALLOC_MEM_FLAGS_COHERENT)
+   alloc_flags |= AMDGPU_GEM_CREATE_COHERENT;
+   if (flags & KFD_IOC_ALLOC_MEM_FLAGS_UNCACHED)
+   alloc_flags |= AMDGPU_GEM_CREATE_UNCACHED;
+
*mem = kzalloc(sizeof(struct kgd_mem), GFP_KERNEL);
if (!*mem) {
ret = -ENOMEM;
diff --git 

Re: [PATCH 1/1] drm/amdkfd: Remove prefault before migrating to VRAM

[Felix Kuehling]

On 2022-09-06 15:34, Philip Yang wrote:

If svm range no back system memory pages, migrate the range to GPU VRAM
don't need prefault on system pages to migrate pages. Instead we just
alloc VRAM pages and setup migrate->dst with the corresponding device
page and skip the page migration. The device page will be inserted to
PTE. Then CPU page fault will migrate the page back to system memory.


There are some English grammar issues that make this explanation hard to 
understand. Let me try to fix that:


   Prefaulting potentially allocates system memory pages before a
   migration. This adds unnecessary overhead. Instead we can skip
   unallocated pages in the migration and just point migrate->dst to a
   0-initialized VRAM page directly. Then the VRAM page will be
   inserted to the PTE. A subsequent CPU page fault will migrate the
   page back to system memory.


As we discussed offline, there is still one potential issue with CPU 
page faults in a child process with COW mappings that we need to address 
before submitting this patch.


Thanks,
  Felix




Signed-off-by: Philip Yang 
---
  drivers/gpu/drm/amd/amdkfd/kfd_migrate.c | 12 +---
  drivers/gpu/drm/amd/amdkfd/kfd_svm.c | 22 --
  drivers/gpu/drm/amd/amdkfd/kfd_svm.h |  2 --
  3 files changed, 5 insertions(+), 31 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
index 373e5bfd4e91..d3ebbde21241 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
@@ -322,12 +322,13 @@ svm_migrate_copy_to_vram(struct amdgpu_device *adev, 
struct svm_range *prange,
for (i = j = 0; i < npages; i++) {
struct page *spage;
  
+		dst[i] = cursor.start + (j << PAGE_SHIFT);

+   migrate->dst[i] = svm_migrate_addr_to_pfn(adev, dst[i]);
+   svm_migrate_get_vram_page(prange, migrate->dst[i]);
+   migrate->dst[i] = migrate_pfn(migrate->dst[i]);
+
spage = migrate_pfn_to_page(migrate->src[i]);
if (spage && !is_zone_device_page(spage)) {
-   dst[i] = cursor.start + (j << PAGE_SHIFT);
-   migrate->dst[i] = svm_migrate_addr_to_pfn(adev, dst[i]);
-   svm_migrate_get_vram_page(prange, migrate->dst[i]);
-   migrate->dst[i] = migrate_pfn(migrate->dst[i]);
src[i] = dma_map_page(dev, spage, 0, PAGE_SIZE,
  DMA_TO_DEVICE);
r = dma_mapping_error(dev, src[i]);
@@ -522,9 +523,6 @@ svm_migrate_ram_to_vram(struct svm_range *prange, uint32_t 
best_loc,
pr_debug("svms 0x%p [0x%lx 0x%lx] to gpu 0x%x\n", prange->svms,
 prange->start, prange->last, best_loc);
  
-	/* FIXME: workaround for page locking bug with invalid pages */

-   svm_range_prefault(prange, mm, SVM_ADEV_PGMAP_OWNER(adev));
-
start = prange->start << PAGE_SHIFT;
end = (prange->last + 1) << PAGE_SHIFT;
  
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c

index 11074cc8c333..cf5b4005534c 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
@@ -3181,28 +3181,6 @@ svm_range_best_prefetch_location(struct svm_range 
*prange)
return best_loc;
  }
  
-/* FIXME: This is a workaround for page locking bug when some pages are

- * invalid during migration to VRAM
- */
-void svm_range_prefault(struct svm_range *prange, struct mm_struct *mm,
-   void *owner)
-{
-   struct hmm_range *hmm_range;
-   int r;
-
-   if (prange->validated_once)
-   return;
-
-   r = amdgpu_hmm_range_get_pages(>notifier, mm, NULL,
-  prange->start << PAGE_SHIFT,
-  prange->npages, _range,
-  false, true, owner);
-   if (!r) {
-   amdgpu_hmm_range_get_pages_done(hmm_range);
-   prange->validated_once = true;
-   }
-}
-
  /* svm_range_trigger_migration - start page migration if prefetch loc changed
   * @mm: current process mm_struct
   * @prange: svm range structure
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.h 
b/drivers/gpu/drm/amd/amdkfd/kfd_svm.h
index cfac13ad06ef..012c53729516 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.h
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.h
@@ -181,8 +181,6 @@ void schedule_deferred_list_work(struct svm_range_list 
*svms);
  void svm_range_dma_unmap(struct device *dev, dma_addr_t *dma_addr,
 unsigned long offset, unsigned long npages);
  void svm_range_free_dma_mappings(struct svm_range *prange);
-void svm_range_prefault(struct svm_range *prange, struct mm_struct *mm,
-   void *owner);
  int svm_range_get_info(struct kfd_process *p, uint32_t *num_svm_ranges,

Re: Build regressions/improvements in v6.0-rc4

[Kees Cook]

On Mon, Sep 05, 2022 at 09:46:01AM +0200, Geert Uytterhoeven wrote:
> On Mon, 5 Sep 2022, Geert Uytterhoeven wrote:
> > JFYI, when comparing v6.0-rc4[1] to v6.0-rc3[3], the summaries are:
> >  - build errors: +3/-16
> 
>   + /kisskb/src/arch/sh/kernel/machvec.c: error: array subscript 'struct 
> sh_machine_vector[0]' is partly outside array bounds of 'long int[1]' 
> [-Werror=array-bounds]:  => 105:33
> 
> sh4-gcc11/sh-allyesconfig (-Werror)

Interesting -- I wonder why this suddenly appeared. I think the fix is
the common "linker addresses need to be char arrays" fix:

diff --git a/arch/sh/include/asm/sections.h b/arch/sh/include/asm/sections.h
index 8edb824049b9..0cb0ca149ac3 100644
--- a/arch/sh/include/asm/sections.h
+++ b/arch/sh/include/asm/sections.h
@@ -4,7 +4,7 @@
 
 #include 
 
-extern long __machvec_start, __machvec_end;
+extern char __machvec_start[], __machvec_end[];
 extern char __uncached_start, __uncached_end;
 extern char __start_eh_frame[], __stop_eh_frame[];
 
diff --git a/arch/sh/kernel/machvec.c b/arch/sh/kernel/machvec.c
index d606679a211e..57efaf5b82ae 100644
--- a/arch/sh/kernel/machvec.c
+++ b/arch/sh/kernel/machvec.c
@@ -20,8 +20,8 @@
 #define MV_NAME_SIZE 32
 
 #define for_each_mv(mv) \
-   for ((mv) = (struct sh_machine_vector *)&__machvec_start; \
-(mv) && (unsigned long)(mv) < (unsigned long)&__machvec_end; \
+   for ((mv) = (struct sh_machine_vector *)__machvec_start; \
+(mv) && (unsigned long)(mv) < (unsigned long)__machvec_end; \
 (mv)++)
 
 static struct sh_machine_vector * __init get_mv_byname(const char *name)
@@ -87,8 +87,8 @@ void __init sh_mv_setup(void)
if (!machvec_selected) {
unsigned long machvec_size;
 
-   machvec_size = ((unsigned long)&__machvec_end -
-   (unsigned long)&__machvec_start);
+   machvec_size = ((unsigned long)__machvec_end -
+   (unsigned long)__machvec_start);
 
/*
 * Sanity check for machvec section alignment. Ensure
@@ -102,7 +102,7 @@ void __init sh_mv_setup(void)
 * vector (usually the only one) from .machvec.init.
 */
if (machvec_size >= sizeof(struct sh_machine_vector))
-   sh_mv = *(struct sh_machine_vector *)&__machvec_start;
+   sh_mv = *(struct sh_machine_vector *)__machvec_start;
}
 
pr_notice("Booting machvec: %s\n", get_system_type());

> 
>   + 
> /kisskb/src/drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn32/display_mode_vba_32.c:
>  error: the frame size of 2144 bytes is larger than 2048 bytes 
> [-Werror=frame-larger-than=]:  => 3768:1
> 
> x86_64-gcc8/x86-allmodconfig (in function 
> dml32_ModeSupportAndSystemConfigurationFull())

This I don't know about it, but looks like a recent commit: dda4fb85e433f
Given it's a 2000 line function, I assume it could be improved! :)

>   + /kisskb/src/include/linux/fortify-string.h: error: call to 
> '__write_overflow_field' declared with attribute warning: detected write 
> beyond size of field (1st parameter); maybe use struct_group()? 
> [-Werror=attribute-warning]:  => 258:25
> 
> s390x-gcc11/s390-allyesconfig (inlined from 'copy_process' at 
> /kisskb/src/kernel/fork.c:2200:2)

This is:

memset(>irqtrace, 0, sizeof(p->irqtrace));

p->irqtrace is:

struct irqtrace_events  irqtrace;

But that's a whole object destination... why would only s390 warn?

-Kees

-- 
Kees Cook

Re: [PATCH] drm/amd/display: Fix register definitions for DCN32/321

[Pillai, Aurabindo]

[AMD Official Use Only - General]

Thank you Siqueira.

--

Regards,
Jay

From: Siqueira, Rodrigo 
Sent: Tuesday, September 6, 2022 11:56 AM
To: Pillai, Aurabindo ; amd-gfx@lists.freedesktop.org 

Cc: Wentland, Harry ; Deucher, Alexander 

Subject: Re: [PATCH] drm/amd/display: Fix register definitions for DCN32/321



On 2022-09-01 15:27, Aurabindo Pillai wrote:
> [Why & How]
> Fix the instatiation sequence for MPC registers and add a few other
> missing register definitions that were ommited erroneously when copying
> them over to enable runtime initialization of reigster offsets for
> DCN32/321
>
> Signed-off-by: Aurabindo Pillai 
> ---
>   .../drm/amd/display/dc/dcn32/dcn32_resource.c |  27 +--
>   .../drm/amd/display/dc/dcn32/dcn32_resource.h | 216 --
>   .../amd/display/dc/dcn321/dcn321_resource.c   |  24 +-
>   3 files changed, 166 insertions(+), 101 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/display/dc/dcn32/dcn32_resource.c 
> b/drivers/gpu/drm/amd/display/dc/dcn32/dcn32_resource.c
> index ef0a6d468a10..9d3b8568351e 100644
> --- a/drivers/gpu/drm/amd/display/dc/dcn32/dcn32_resource.c
> +++ b/drivers/gpu/drm/amd/display/dc/dcn32/dcn32_resource.c
> @@ -461,22 +461,17 @@ static const struct dcn20_dsc_mask dsc_mask = {
>   };
>
>   static struct dcn30_mpc_registers mpc_regs;
> -#define dcn_mpc_regs_init()\
> - ( \
> - MPC_REG_LIST_DCN3_0_RI(0),\
> - MPC_REG_LIST_DCN3_0_RI(1),\
> - MPC_REG_LIST_DCN3_0_RI(2),\
> - MPC_REG_LIST_DCN3_0_RI(3),\
> - MPC_OUT_MUX_REG_LIST_DCN3_0_RI(0),\
> - MPC_OUT_MUX_REG_LIST_DCN3_0_RI(1),\
> - MPC_OUT_MUX_REG_LIST_DCN3_0_RI(2),\
> - MPC_OUT_MUX_REG_LIST_DCN3_0_RI(3),\
> - MPC_MCM_REG_LIST_DCN32_RI(0),\
> - MPC_MCM_REG_LIST_DCN32_RI(1),\
> - MPC_MCM_REG_LIST_DCN32_RI(2),\
> - MPC_MCM_REG_LIST_DCN32_RI(3),\
> - MPC_DWB_MUX_REG_LIST_DCN3_0_RI(0)\
> - )
> +
> +#define dcn_mpc_regs_init() \
> + MPC_REG_LIST_DCN3_2_RI(0),\
> + MPC_REG_LIST_DCN3_2_RI(1),\
> + MPC_REG_LIST_DCN3_2_RI(2),\
> + MPC_REG_LIST_DCN3_2_RI(3),\
> + MPC_OUT_MUX_REG_LIST_DCN3_0_RI(0),\
> + MPC_OUT_MUX_REG_LIST_DCN3_0_RI(1),\
> + MPC_OUT_MUX_REG_LIST_DCN3_0_RI(2),\
> + MPC_OUT_MUX_REG_LIST_DCN3_0_RI(3),\
> + MPC_DWB_MUX_REG_LIST_DCN3_0_RI(0)
>
>   static const struct dcn30_mpc_shift mpc_shift = {
>MPC_COMMON_MASK_SH_LIST_DCN32(__SHIFT)
> diff --git a/drivers/gpu/drm/amd/display/dc/dcn32/dcn32_resource.h 
> b/drivers/gpu/drm/amd/display/dc/dcn32/dcn32_resource.h
> index 60d8fad16eee..4c931905223d 100644
> --- a/drivers/gpu/drm/amd/display/dc/dcn32/dcn32_resource.h
> +++ b/drivers/gpu/drm/amd/display/dc/dcn32/dcn32_resource.h
> @@ -222,7 +222,8 @@ void dcn32_determine_det_override(struct dc_state 
> *context, display_e2e_pipe_par
> SRI_ARR(DP_MSA_TIMING_PARAM4, DP, id),
>\
> SRI_ARR(DP_MSE_RATE_CNTL, DP, id), SRI_ARR(DP_MSE_RATE_UPDATE, DP, 
> id),  \
> SRI_ARR(DP_PIXEL_FORMAT, DP, id), SRI_ARR(DP_SEC_CNTL, DP, id),   
>\
> -  SRI_ARR(DP_SEC_CNTL2, DP, id), SRI_ARR(DP_SEC_CNTL6, DP, id),  
>   \
> +  SRI_ARR(DP_SEC_CNTL1, DP, id), SRI_ARR(DP_SEC_CNTL2, DP, id),  
>   \
> +  SRI_ARR(DP_SEC_CNTL5, DP, id), SRI_ARR(DP_SEC_CNTL6, DP, id),  
>   \
> SRI_ARR(DP_STEER_FIFO, DP, id), SRI_ARR(DP_VID_M, DP, id),
>\
> SRI_ARR(DP_VID_N, DP, id), SRI_ARR(DP_VID_STREAM_CNTL, DP, id),   
>\
> SRI_ARR(DP_VID_TIMING, DP, id), SRI_ARR(DP_SEC_AUD_N, DP, id),
>\
> @@ -735,75 +736,6 @@ void dcn32_determine_det_override(struct dc_state 
> *context, display_e2e_pipe_par
>   #define MPC_DWB_MUX_REG_LIST_DCN3_0_RI(inst)
>\
> SRII_DWB(DWB_MUX, MUX, MPC_DWB, inst)
>
> -#define MPC_MCM_REG_LIST_DCN32_RI(inst)  
>   \
> -  ( \
> -  SRII(MPCC_MCM_SHAPER_CONTROL, MPCC_MCM, inst), 
>   \
> -  SRII(MPCC_MCM_SHAPER_OFFSET_R, MPCC_MCM, inst),
>   \
> -  SRII(MPCC_MCM_SHAPER_OFFSET_G, MPCC_MCM, inst),
>   \
> -  SRII(MPCC_MCM_SHAPER_OFFSET_B, MPCC_MCM, inst),
>   \
> -  SRII(MPCC_MCM_SHAPER_SCALE_R, MPCC_MCM, inst), 
>   \
> -  SRII(MPCC_MCM_SHAPER_SCALE_G_B, MPCC_MCM, inst),   
>   \
> -  SRII(MPCC_MCM_SHAPER_LUT_INDEX, MPCC_MCM, inst),   
>   \
> -  SRII(MPCC_MCM_SHAPER_LUT_DATA, MPCC_MCM, inst),
>   \
> -  SRII(MPCC_MCM_SHAPER_LUT_WRITE_EN_MASK, MPCC_MCM, inst),   
>   \
> -  SRII(MPCC_MCM_SHAPER_RAMA_START_CNTL_B, MPCC_MCM, inst),   
>   \
> -  SRII(MPCC_MCM_SHAPER_RAMA_START_CNTL_G, 

[PATCH] drm/amdgpu: use dirty framebuffer helper

[Hamza Mahfooz]

Currently, we aren't handling DRM_IOCTL_MODE_DIRTYFB. So, use
drm_atomic_helper_dirtyfb() as the dirty callback in the amdgpu_fb_funcs
struct.

Signed-off-by: Hamza Mahfooz 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
index c20922a5af9f..5b09c8f4fe95 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
@@ -38,6 +38,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -496,6 +497,7 @@ bool amdgpu_display_ddc_probe(struct amdgpu_connector 
*amdgpu_connector,
 static const struct drm_framebuffer_funcs amdgpu_fb_funcs = {
.destroy = drm_gem_fb_destroy,
.create_handle = drm_gem_fb_create_handle,
+   .dirty = drm_atomic_helper_dirtyfb,
 };
 
 uint32_t amdgpu_display_supported_domains(struct amdgpu_device *adev,
-- 
2.37.2

Re: [PATCH v6 1/6] drm/ttm: Add new callbacks to ttm res mgr

[Daniel Vetter]

On Tue, Aug 16, 2022 at 10:33:16AM +0530, Arunpravin Paneer Selvam wrote:
> 
> 
> On 8/15/2022 4:35 PM, Christian König wrote:
> > Am 12.08.22 um 15:30 schrieb Arunpravin Paneer Selvam:
> > > We are adding two new callbacks to ttm resource manager
> > > function to handle intersection and compatibility of
> > > placement and resources.
> > > 
> > > v2: move the amdgpu and ttm_range_manager changes to
> > >  separate patches (Christian)
> > > v3: rename "intersect" to "intersects" (Matthew)
> > > v4: move !place check to the !res if and return false
> > >  in ttm_resource_compatible() function (Christian)
> > > v5: move bits of code from patch number 6 to avoid
> > >  temporary driver breakup (Christian)
> > > 
> > > Signed-off-by: Christian König 
> > > Signed-off-by: Arunpravin Paneer Selvam
> > > 
> > 
> > Patch #6 could still be cleaned up more now that we have the workaround
> > code in patch #1, but that not really a must have.
> > 
> > Reviewed-by: Christian König  for the entire
> > series.
> > 
> > Do you already have commit rights?
> 
> Hi Christian,
> I applied for drm-misc commit rights, waiting for the project maintainers to
> approve my request.

Why do all drivers have to implement the current behaviour? Can we have a
default implementation, which gets called if nothing is set instead?

I'm a bit confused why the bloat here ...

Also please document new callbacks precisely with inline kerneldoc. I know
ttm docs aren't great yet, but they don't get better if we don't start
somewhere. I think the in-depth comments for modeset vfuncs (e.g. in
drm_modeset_helper_vtables.h) are a good standard here.
-Daniel

> 
> Thanks,
> Arun
> > 
> > Regards,
> > Christian.
> > 
> > > ---
> > >   drivers/gpu/drm/ttm/ttm_bo.c   |  9 ++--
> > >   drivers/gpu/drm/ttm/ttm_resource.c | 77 +-
> > >   include/drm/ttm/ttm_resource.h | 40 
> > >   3 files changed, 119 insertions(+), 7 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
> > > index c1bd006a5525..f066e8124c50 100644
> > > --- a/drivers/gpu/drm/ttm/ttm_bo.c
> > > +++ b/drivers/gpu/drm/ttm/ttm_bo.c
> > > @@ -518,6 +518,9 @@ static int ttm_bo_evict(struct ttm_buffer_object
> > > *bo,
> > >   bool ttm_bo_eviction_valuable(struct ttm_buffer_object *bo,
> > >     const struct ttm_place *place)
> > >   {
> > > +    struct ttm_resource *res = bo->resource;
> > > +    struct ttm_device *bdev = bo->bdev;
> > > +
> > >   dma_resv_assert_held(bo->base.resv);
> > >   if (bo->resource->mem_type == TTM_PL_SYSTEM)
> > >   return true;
> > > @@ -525,11 +528,7 @@ bool ttm_bo_eviction_valuable(struct
> > > ttm_buffer_object *bo,
> > >   /* Don't evict this BO if it's outside of the
> > >    * requested placement range
> > >    */
> > > -    if (place->fpfn >= (bo->resource->start +
> > > bo->resource->num_pages) ||
> > > -    (place->lpfn && place->lpfn <= bo->resource->start))
> > > -    return false;
> > > -
> > > -    return true;
> > > +    return ttm_resource_intersects(bdev, res, place, bo->base.size);
> > >   }
> > >   EXPORT_SYMBOL(ttm_bo_eviction_valuable);
> > >   diff --git a/drivers/gpu/drm/ttm/ttm_resource.c
> > > b/drivers/gpu/drm/ttm/ttm_resource.c
> > > index 20f9adcc3235..0d1f862a582b 100644
> > > --- a/drivers/gpu/drm/ttm/ttm_resource.c
> > > +++ b/drivers/gpu/drm/ttm/ttm_resource.c
> > > @@ -253,10 +253,84 @@ void ttm_resource_free(struct
> > > ttm_buffer_object *bo, struct ttm_resource **res)
> > >   }
> > >   EXPORT_SYMBOL(ttm_resource_free);
> > >   +/**
> > > + * ttm_resource_intersects - test for intersection
> > > + *
> > > + * @bdev: TTM device structure
> > > + * @res: The resource to test
> > > + * @place: The placement to test
> > > + * @size: How many bytes the new allocation needs.
> > > + *
> > > + * Test if @res intersects with @place and @size. Used for testing
> > > if evictions
> > > + * are valueable or not.
> > > + *
> > > + * Returns true if the res placement intersects with @place and @size.
> > > + */
> > > +bool ttm_resource_intersects(struct ttm_device *bdev,
> > > + struct ttm_resource *res,
> > > + const struct ttm_place *place,
> > > + size_t size)
> > > +{
> > > +    struct ttm_resource_manager *man;
> > > +
> > > +    if (!res)
> > > +    return false;
> > > +
> > > +    if (!place)
> > > +    return true;
> > > +
> > > +    man = ttm_manager_type(bdev, res->mem_type);
> > > +    if (!man->func->intersects) {
> > > +    if (place->fpfn >= (res->start + res->num_pages) ||
> > > +    (place->lpfn && place->lpfn <= res->start))
> > > +    return false;
> > > +
> > > +    return true;
> > > +    }
> > > +
> > > +    return man->func->intersects(man, res, place, size);
> > > +}
> > > +
> > > +/**
> > > + * ttm_resource_compatible - test for compatibility
> > > + *
> > > + * @bdev: TTM device 

[PATCH 1/1] drm/amdkfd: Remove prefault before migrating to VRAM

[Philip Yang]

If svm range no back system memory pages, migrate the range to GPU VRAM
don't need prefault on system pages to migrate pages. Instead we just
alloc VRAM pages and setup migrate->dst with the corresponding device
page and skip the page migration. The device page will be inserted to
PTE. Then CPU page fault will migrate the page back to system memory.

Signed-off-by: Philip Yang 
---
 drivers/gpu/drm/amd/amdkfd/kfd_migrate.c | 12 +---
 drivers/gpu/drm/amd/amdkfd/kfd_svm.c | 22 --
 drivers/gpu/drm/amd/amdkfd/kfd_svm.h |  2 --
 3 files changed, 5 insertions(+), 31 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
index 373e5bfd4e91..d3ebbde21241 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
@@ -322,12 +322,13 @@ svm_migrate_copy_to_vram(struct amdgpu_device *adev, 
struct svm_range *prange,
for (i = j = 0; i < npages; i++) {
struct page *spage;
 
+   dst[i] = cursor.start + (j << PAGE_SHIFT);
+   migrate->dst[i] = svm_migrate_addr_to_pfn(adev, dst[i]);
+   svm_migrate_get_vram_page(prange, migrate->dst[i]);
+   migrate->dst[i] = migrate_pfn(migrate->dst[i]);
+
spage = migrate_pfn_to_page(migrate->src[i]);
if (spage && !is_zone_device_page(spage)) {
-   dst[i] = cursor.start + (j << PAGE_SHIFT);
-   migrate->dst[i] = svm_migrate_addr_to_pfn(adev, dst[i]);
-   svm_migrate_get_vram_page(prange, migrate->dst[i]);
-   migrate->dst[i] = migrate_pfn(migrate->dst[i]);
src[i] = dma_map_page(dev, spage, 0, PAGE_SIZE,
  DMA_TO_DEVICE);
r = dma_mapping_error(dev, src[i]);
@@ -522,9 +523,6 @@ svm_migrate_ram_to_vram(struct svm_range *prange, uint32_t 
best_loc,
pr_debug("svms 0x%p [0x%lx 0x%lx] to gpu 0x%x\n", prange->svms,
 prange->start, prange->last, best_loc);
 
-   /* FIXME: workaround for page locking bug with invalid pages */
-   svm_range_prefault(prange, mm, SVM_ADEV_PGMAP_OWNER(adev));
-
start = prange->start << PAGE_SHIFT;
end = (prange->last + 1) << PAGE_SHIFT;
 
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
index 11074cc8c333..cf5b4005534c 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
@@ -3181,28 +3181,6 @@ svm_range_best_prefetch_location(struct svm_range 
*prange)
return best_loc;
 }
 
-/* FIXME: This is a workaround for page locking bug when some pages are
- * invalid during migration to VRAM
- */
-void svm_range_prefault(struct svm_range *prange, struct mm_struct *mm,
-   void *owner)
-{
-   struct hmm_range *hmm_range;
-   int r;
-
-   if (prange->validated_once)
-   return;
-
-   r = amdgpu_hmm_range_get_pages(>notifier, mm, NULL,
-  prange->start << PAGE_SHIFT,
-  prange->npages, _range,
-  false, true, owner);
-   if (!r) {
-   amdgpu_hmm_range_get_pages_done(hmm_range);
-   prange->validated_once = true;
-   }
-}
-
 /* svm_range_trigger_migration - start page migration if prefetch loc changed
  * @mm: current process mm_struct
  * @prange: svm range structure
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.h 
b/drivers/gpu/drm/amd/amdkfd/kfd_svm.h
index cfac13ad06ef..012c53729516 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.h
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.h
@@ -181,8 +181,6 @@ void schedule_deferred_list_work(struct svm_range_list 
*svms);
 void svm_range_dma_unmap(struct device *dev, dma_addr_t *dma_addr,
 unsigned long offset, unsigned long npages);
 void svm_range_free_dma_mappings(struct svm_range *prange);
-void svm_range_prefault(struct svm_range *prange, struct mm_struct *mm,
-   void *owner);
 int svm_range_get_info(struct kfd_process *p, uint32_t *num_svm_ranges,
   uint64_t *svm_priv_data_size);
 int kfd_criu_checkpoint_svm(struct kfd_process *p,
-- 
2.35.1

Re: [PATCH v4 00/41] DYNDBG: opt-in class'd debug for modules, use in drm.

[Daniel Vetter]

On Fri, Aug 12, 2022 at 08:03:47AM +0200, Greg KH wrote:
> On Thu, Aug 11, 2022 at 06:52:40PM +0200, Daniel Vetter wrote:
> > On Wed, Aug 03, 2022 at 04:13:05PM -0400, Jason Baron wrote:
> > > 
> > > 
> > > On 8/3/22 15:56, jim.cro...@gmail.com wrote:
> > > > On Wed, Jul 20, 2022 at 9:32 AM Jim Cromie  wrote:
> > > >>
> > > > 
> > > >> Hi Jason, Greg, DRM-folk,
> > > >>
> > > >> This adds 'typed' "class FOO" support to dynamic-debug, where 'typed'
> > > >> means either DISJOINT (like drm debug categories), or VERBOSE (like
> > > >> nouveau debug-levels).  Use it in DRM modules: core, helpers, and in
> > > >> drivers i915, amdgpu, nouveau.
> > > >>
> > > > 
> > > > This revision fell over, on a conflict with something in drm-MUMBLE
> > > > 
> > > > Error: patch 
> > > > https://urldefense.com/v3/__https://patchwork.freedesktop.org/api/1.0/series/106427/revisions/2/mbox/__;!!GjvTz_vk!UCPl5Uf32cDVwwysMTfaLwoGLWomargFXuR8HjBA3xsUOjxXHXC5hneAkP4iWK91yc-LjjJxWW89-51Z$
> > > >  
> > > > not applied
> > > > Applying: dyndbg: fix static_branch manipulation
> > > > Applying: dyndbg: fix module.dyndbg handling
> > > > Applying: dyndbg: show both old and new in change-info
> > > > Applying: dyndbg: reverse module walk in cat control
> > > > Applying: dyndbg: reverse module.callsite walk in cat control
> > > > Applying: dyndbg: use ESCAPE_SPACE for cat control
> > > > Applying: dyndbg: let query-modname override actual module name
> > > > Applying: dyndbg: add test_dynamic_debug module
> > > > Applying: dyndbg: drop EXPORTed dynamic_debug_exec_queries
> > > > 
> > > > Jason,
> > > > those above are decent maintenance patches, particularly the drop 
> > > > export.
> > > > It would be nice to trim this unused api this cycle.
> > > 
> > > Hi Jim,
> > > 
> > > Agreed - I was thinking the same thing. Feel free to add
> > > Acked-by: Jason Baron  to those first 9.
> > 
> > Does Greg KH usually pick up dyndbg patches or someone else or do I need
> > to do something? Would be great to get some movement here since -rc1 goes
> > out and merging will restart next week.
> 
> Yes, I can take these into my tree after -rc1 is out.

[uncovering from an absolutely impressive cascade of holes :-(]

Did this happen and I can stop worrying here? I'd like to make sure these
drm debug infra improvements keep moving.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [PATCH] drm/amdgpu: cleanup coding style in amdgpu_amdkfd_gpuvm.c

[Deucher, Alexander]

[Public]

Yeah, seems to be a mix.  I don't have a strong opinion as long as it has MIT.

Alex


From: Kuehling, Felix 
Sent: Tuesday, September 6, 2022 9:46 AM
To: Jingyu Wang ; Deucher, Alexander 
; Koenig, Christian ; Pan, 
Xinhui ; airl...@linux.ie ; 
dan...@ffwll.ch 
Cc: amd-gfx@lists.freedesktop.org ; 
dri-de...@lists.freedesktop.org ; 
linux-ker...@vger.kernel.org 
Subject: Re: [PATCH] drm/amdgpu: cleanup coding style in amdgpu_amdkfd_gpuvm.c


Am 2022-09-05 um 04:38 schrieb Jingyu Wang:
> Fix everything checkpatch.pl complained about in amdgpu_amdkfd_gpuvm.c
>
> Signed-off-by: Jingyu Wang 
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> index cbd593f7d553..eff596c60c89 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> @@ -1,3 +1,4 @@
> +// SPDX-License-Identifier: MIT

I'm not sure if this is correct. We've used "GPL-2.0 OR MIT" in KFD. In
amdgpu there is currently a mix of licenses. Alex, do you want to make a
call on a consistent one to use in amdgpu?

Other than that, this patch is

Reviewed-by: Felix Kuehling 


>   /*
>* Copyright 2014-2018 Advanced Micro Devices, Inc.
>*
> @@ -1612,6 +1613,7 @@ size_t amdgpu_amdkfd_get_available_memory(struct 
> amdgpu_device *adev)
>uint64_t reserved_for_pt =
>ESTIMATE_PT_SIZE(amdgpu_amdkfd_total_mem_size);
>size_t available;
> +
>spin_lock(_mem_limit.mem_limit_lock);
>available = adev->gmc.real_vram_size
>- adev->kfd.vram_used_aligned
> @@ -2216,7 +2218,7 @@ int amdgpu_amdkfd_gpuvm_get_vm_fault_info(struct 
> amdgpu_device *adev,
>   {
>if (atomic_read(>gmc.vm_fault_info_updated) == 1) {
>*mem = *adev->gmc.vm_fault_info;
> - mb();
> + mb(); /* make sure read happened */
>atomic_set(>gmc.vm_fault_info_updated, 0);
>}
>return 0;
>
> base-commit: e47eb90a0a9ae20b82635b9b99a8d0979b757ad8

Re: [PATCH] drm/amdgpu: cleanup coding style in amdgpu_kms.c

[kernel test robot]

Hi Jingyu,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on e47eb90a0a9ae20b82635b9b99a8d0979b757ad8]

url:
https://github.com/intel-lab-lkp/linux/commits/Jingyu-Wang/drm-amdgpu-cleanup-coding-style-in-amdgpu_kms-c/20220906-104802
base:   e47eb90a0a9ae20b82635b9b99a8d0979b757ad8
config: arm64-randconfig-r031-20220906 
(https://download.01.org/0day-ci/archive/20220907/202209070101.lerxi9zr-...@intel.com/config)
compiler: clang version 16.0.0 (https://github.com/llvm/llvm-project 
c55b41d5199d2394dd6cdb8f52180d8b81d809d4)
reproduce (this is a W=1 build):
wget 
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O 
~/bin/make.cross
chmod +x ~/bin/make.cross
# install arm64 cross compiling tool for clang build
# apt-get install binutils-aarch64-linux-gnu
# 
https://github.com/intel-lab-lkp/linux/commit/778e4a57afd0db6a6a752487e1a1cda253cd9682
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review 
Jingyu-Wang/drm-amdgpu-cleanup-coding-style-in-amdgpu_kms-c/20220906-104802
git checkout 778e4a57afd0db6a6a752487e1a1cda253cd9682
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 
O=build_dir ARCH=arm64 SHELL=/bin/bash

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot 

All errors (new ones prefixed by >>):

>> drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c:554:62: error: too few arguments 
>> provided to function-like macro invocation
   ret = copy_to_user(out, , min_t((size_t)size, 
sizeof(ip)));
  ^
   include/linux/minmax.h:104:9: note: macro 'min_t' defined here
   #define min_t(type, x, y)   __careful_cmp((type)(x), (type)(y), <)
   ^
>> drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c:554:32: error: use of undeclared 
>> identifier 'min_t'; did you mean 'minfo'?
   ret = copy_to_user(out, , min_t((size_t)size, 
sizeof(ip)));
^
minfo
   drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c:514:27: note: 'minfo' declared here
   struct amdgpu_mode_info *minfo = >mode_info;
^
>> drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c:1385:3: error: expected expression
   TA_FW_NAME(XGMI),
   ^
   drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c:1384:27: note: expanded from macro 
'TA_FW_NAME'
   #define TA_FW_NAME(type) ([TA_FW_TYPE_PSP_##type] = #type)
 ^
   drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c:1386:3: error: expected expression
   TA_FW_NAME(RAS),
   ^
   drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c:1384:27: note: expanded from macro 
'TA_FW_NAME'
   #define TA_FW_NAME(type) ([TA_FW_TYPE_PSP_##type] = #type)
 ^
   drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c:1387:3: error: expected expression
   TA_FW_NAME(HDCP),
   ^
   drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c:1384:27: note: expanded from macro 
'TA_FW_NAME'
   #define TA_FW_NAME(type) ([TA_FW_TYPE_PSP_##type] = #type)
 ^
   drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c:1388:3: error: expected expression
   TA_FW_NAME(DTM),
   ^
   drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c:1384:27: note: expanded from macro 
'TA_FW_NAME'
   #define TA_FW_NAME(type) ([TA_FW_TYPE_PSP_##type] = #type)
 ^
   drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c:1389:3: error: expected expression
   TA_FW_NAME(RAP),
   ^
   drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c:1384:27: note: expanded from macro 
'TA_FW_NAME'
   #define TA_FW_NAME(type) ([TA_FW_TYPE_PSP_##type] = #type)
 ^
   drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c:1390:3: error: expected expression
   TA_FW_NAME(SECUREDISPLAY),
   ^
   drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c:1384:27: note: expanded from macro 
'TA_FW_NAME'
   #define TA_FW_NAME(type) ([TA_FW_TYPE_PSP_##type] = #type)
 ^
   8 errors generated.


vim +554 drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c

   494  
   495  /*
   496   * Userspace get information ioctl
   497   */
   498  /**
   499   * amdgpu_info_ioctl - answer a device specific request.
   500   *
   501   * @dev: drm device pointer
   502   * @data: request object
   503   * @filp: drm filp
   504   *
   505   * This function is used to pass device specific parameters to the 
userspace
   506   * drivers.  Examples include: pci device id, pipeline parms, tiling 
params,
   507   * etc. (all asics).
   508   

[RFC PATCH v2 9/9] drm/amd/display: enable DRM shaper and 3D LUT properties

[Melissa Wen]

Shaper LUT and 3D LUT programming is done, so make the DRM color
properties available.

Signed-off-by: Melissa Wen 
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c
index c89594f3a5cb..b165a0c269fa 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c
@@ -452,6 +452,12 @@ int amdgpu_dm_crtc_init(struct amdgpu_display_manager *dm,
dm->adev->mode_info.crtcs[crtc_index] = acrtc;
drm_crtc_enable_color_mgmt(>base, MAX_COLOR_LUT_ENTRIES,
   true, MAX_COLOR_LUT_ENTRIES);
+
+   if (dm->dc->caps.color.mpc.num_3dluts)
+   drm_crtc_enable_lut3d(>base,
+ MAX_COLOR_LUT_ENTRIES,
+ MAX_COLOR_3DLUT_ENTRIES);
+
drm_mode_crtc_set_gamma_size(>base, 
MAX_COLOR_LEGACY_LUT_ENTRIES);
 
return 0;
-- 
2.35.1

[RFC PATCH v2 8/9] drm/amd/display: update lut3d and shaper lut to stream

[Melissa Wen]

It follows the same path of out_transfer_func for stream updates, since
shaper LUT and 3D LUT is programmed in funcs.set_output_transfer_func()
and this function is called in the atomic commit_tail when
update_flags.bits.out_tf is set.

Signed-off-by: Melissa Wen 
---
 drivers/gpu/drm/amd/display/dc/core/dc.c | 10 +-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/display/dc/core/dc.c 
b/drivers/gpu/drm/amd/display/dc/core/dc.c
index 9860bf38c547..d1fa87ddf1dd 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc.c
@@ -2405,7 +2405,7 @@ static enum surface_update_type 
check_update_surfaces_for_stream(
stream_update->integer_scaling_update)
su_flags->bits.scaling = 1;
 
-   if (stream_update->out_transfer_func)
+   if (stream_update->out_transfer_func || 
stream_update->lut3d_func)
su_flags->bits.out_tf = 1;
 
if (stream_update->abm_level)
@@ -2754,6 +2754,14 @@ static void copy_stream_update_to_stream(struct dc *dc,
   sizeof(struct dc_transfer_func_distributed_points));
}
 
+   if (update->func_shaper &&
+   stream->func_shaper != update->func_shaper)
+   stream->func_shaper = update->func_shaper;
+
+   if (update->lut3d_func &&
+   stream->lut3d_func != update->lut3d_func)
+   stream->lut3d_func = update->lut3d_func;
+
if (update->hdr_static_metadata)
stream->hdr_static_metadata = *update->hdr_static_metadata;
 
-- 
2.35.1

[RFC PATCH v2 7/9] drm/amd/display: add user shaper LUT support to amdgpu_dm color pipeline

[Melissa Wen]

Now, we can use shaper LUT to delinearize and/or normalize the color
space for a more efficient 3D LUT support (so far, only for DRM atomic
color mgmt). If a degamma 1D LUT is passed to linearize the color space,
a custom shaper 1D LUT can be used before applying 3D LUT.

Signed-off-by: Melissa Wen 
---
 .../amd/display/amdgpu_dm/amdgpu_dm_color.c   | 105 +++---
 1 file changed, 87 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c
index 9777252191b1..b590fc83a88c 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c
@@ -352,18 +352,64 @@ static int __set_input_tf(struct dc_transfer_func *func,
return res ? 0 : -ENOMEM;
 }
 
-static int amdgpu_dm_atomic_shaper_lut(struct dc_stream_state *stream,
-  struct dc_transfer_func *func_shaper_new)
+static int __set_func_shaper(struct dc_transfer_func *shaper_func,
+const struct drm_color_lut *lut, uint32_t lut_size)
 {
+   struct dc_gamma *gamma = NULL;
+   struct calculate_buffer cal_buffer = {0};
+   bool res;
+
+   ASSERT(lut && lut_size == MAX_COLOR_LUT_ENTRIES);
+
+   cal_buffer.buffer_index = -1;
+
+   gamma = dc_create_gamma();
+   if (!gamma)
+   return -ENOMEM;
+
+   gamma->num_entries = lut_size;
+   __drm_lut_to_dc_gamma(lut, gamma, false);
+
+   /*
+* Color module doesn't like calculating gamma params
+* on top of a linear input. But degamma params can be used
+* instead to simulate this.
+*/
+   gamma->type = GAMMA_CUSTOM;
+   res = mod_color_calculate_degamma_params(NULL, shaper_func, gamma, 
true);
+
+   dc_gamma_release();
+
+   return res ? 0 : -ENOMEM;
+}
 
-   /* We don't get DRM shaper LUT yet. We assume the input color space is
-* already delinearized, so we don't need a shaper LUT and we can just
-* BYPASS.  However, checking dcn30_set_mpc_shaper_3dlut() it seems
+static int amdgpu_dm_atomic_shaper_lut(struct dc_stream_state *stream,
+  struct dc_transfer_func *func_shaper_new,
+  const struct drm_color_lut *shaper_lut,
+  uint32_t shaper_size)
+{
+   /* If no DRM shaper LUT, we assume the input color space is already
+* delinearized, so we don't need a shaper LUT and we can just BYPASS
+* However, checking dcn30_set_mpc_shaper_3dlut() it seems
 * that setting shaper LUT to BYPASS is not currently supported in the
 * DC level, since shaper LUT programming just fails without params.
 */
-   func_shaper_new->type = TF_TYPE_BYPASS;
-   func_shaper_new->tf = TRANSFER_FUNCTION_LINEAR;
+   if (!shaper_size) {
+   func_shaper_new->type = TF_TYPE_BYPASS;
+   func_shaper_new->tf = TRANSFER_FUNCTION_LINEAR;
+   } else {
+   int r;
+
+   /* If DRM shaper LUT is set, we follow the same behavior of the
+* atomic regamma and assume a linear base */
+   func_shaper_new->type = TF_TYPE_DISTRIBUTED_POINTS;
+   func_shaper_new->tf = TRANSFER_FUNCTION_LINEAR;
+
+   r = __set_func_shaper(func_shaper_new, shaper_lut,
+   shaper_size);
+   if (r)
+   return r;
+   }
 
stream->func_shaper = func_shaper_new;
 
@@ -427,12 +473,27 @@ static uint32_t amdgpu_dm_get_3dlut_size(uint32_t 
lut_size,
 int amdgpu_dm_verify_3dlut_size(const struct drm_crtc_state *crtc_state,
struct amdgpu_device *adev)
 {
-   const struct drm_color_lut *lut3d = NULL;
+   const struct drm_color_lut *shaper = NULL, *lut3d = NULL;
uint32_t exp_size, size;
 
+   /* shaper LUT is only available if 3D LUT color caps*/
+   exp_size = amdgpu_dm_get_3dlut_size(MAX_COLOR_LUT_ENTRIES, adev);
+
+   shaper = __extract_blob_lut(crtc_state->shaper_lut, );
+   if (shaper && size != exp_size) {
+   DRM_DEBUG_DRIVER(
+   "Invalid Shaper LUT size. Should be %u but got %u.\n",
+   exp_size, size);
+   return -EINVAL;
+   }
+
exp_size = amdgpu_dm_get_3dlut_size(MAX_COLOR_3DLUT_ENTRIES, adev);
lut3d = __extract_blob_lut(crtc_state->lut3d, );
 
+   /* shaper LUT implies 3D LUT. See dcn30_set_output_transfer_func() */
+   if (shaper && !lut3d)
+   DRM_DEBUG_DRIVER("Shaper LUT is set without 3D LUT.\n");
+
if (lut3d && size != exp_size) {
DRM_DEBUG_DRIVER("Invalid Gamma 3D LUT size. Should be %u but 
got %u.\n",
 exp_size, size);
@@ -444,6 +505,8 @@ int 

[RFC PATCH v2 6/9] drm/amd/display: add user 3D LUT support to the amdgpu_dm color pipeline

[Melissa Wen]

Map DRM 3D LUT in the atomic color mgmt pipeline to DC. 3D LUT works
better in a non-linear color space, therefore using a degamma to
linearize the input space may produce unexpected results. The next patch
introduces shaper LUT support that can be used to delinearize the color
space before applying 3D LUT conversion.

Note that there is no implicit sRGB degamma/regamma in the original
implementation for DRM atomic color mgmt. Atomic degamma/regamma 1D LUT
is applied considering a linear base.
For reference, see IGT test amdgpu/amd_color and commit
cf020d49b3c4 ("drm/amd/display: Rework CRTC color management")

dc_acquire_release_mpc_3dlut initializes the bits required to program
3DLUT in DC MPC hw block, that is applied by set_output_transfer_func().
But we should verify if the timing to acquire and release shaper and 3D
LUTs from the resource pool in this patch is correct.

Signed-off-by: Melissa Wen 
---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c |   6 +
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h |   5 +
 .../amd/display/amdgpu_dm/amdgpu_dm_color.c   | 149 +-
 .../gpu/drm/amd/display/dc/core/dc_stream.c   |  13 ++
 4 files changed, 172 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index dbe76b85552e..05d96978925c 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -9435,6 +9435,12 @@ static int amdgpu_dm_atomic_check(struct drm_device *dev,
goto fail;
}
 
+   ret = amdgpu_dm_verify_3dlut_size(new_crtc_state, adev);
+   if (ret) {
+   DRM_DEBUG_DRIVER("amdgpu_dm_verify_lut_sizes() 
failed\n");
+   goto fail;
+   }
+
if (!new_crtc_state->enable)
continue;
 
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
index b44faaad9b0b..435cf6b4efc1 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
@@ -784,12 +784,17 @@ void amdgpu_dm_update_freesync_caps(struct drm_connector 
*connector,
 
 void amdgpu_dm_trigger_timing_sync(struct drm_device *dev);
 
+/* 3D LUT max size is 17x17x17 */
+#define MAX_COLOR_3DLUT_ENTRIES 4913
+/* 1D LUT degamma, regamma and shaper*/
 #define MAX_COLOR_LUT_ENTRIES 4096
 /* Legacy gamm LUT users such as X doesn't like large LUT sizes */
 #define MAX_COLOR_LEGACY_LUT_ENTRIES 256
 
 void amdgpu_dm_init_color_mod(void);
 int amdgpu_dm_verify_lut_sizes(const struct drm_crtc_state *crtc_state);
+int amdgpu_dm_verify_3dlut_size(const struct drm_crtc_state *crtc_state,
+   struct amdgpu_device *adev);
 int amdgpu_dm_update_crtc_color_mgmt(struct dm_crtc_state *crtc);
 int amdgpu_dm_update_plane_color_mgmt(struct dm_crtc_state *crtc,
  struct dc_plane_state *dc_plane_state);
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c
index 54d95745f0f0..9777252191b1 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c
@@ -352,6 +352,131 @@ static int __set_input_tf(struct dc_transfer_func *func,
return res ? 0 : -ENOMEM;
 }
 
+static int amdgpu_dm_atomic_shaper_lut(struct dc_stream_state *stream,
+  struct dc_transfer_func *func_shaper_new)
+{
+
+   /* We don't get DRM shaper LUT yet. We assume the input color space is
+* already delinearized, so we don't need a shaper LUT and we can just
+* BYPASS.  However, checking dcn30_set_mpc_shaper_3dlut() it seems
+* that setting shaper LUT to BYPASS is not currently supported in the
+* DC level, since shaper LUT programming just fails without params.
+*/
+   func_shaper_new->type = TF_TYPE_BYPASS;
+   func_shaper_new->tf = TRANSFER_FUNCTION_LINEAR;
+
+   stream->func_shaper = func_shaper_new;
+
+   return 0;
+}
+
+static void __to_dc_lut3d_color(struct dc_rgb *rgb,
+   const struct drm_color_lut lut,
+   int bit_precision)
+{
+   rgb->red = drm_color_lut_extract(lut.red, bit_precision);
+   rgb->green = drm_color_lut_extract(lut.green, bit_precision);
+   rgb->blue  = drm_color_lut_extract(lut.blue, bit_precision);
+}
+
+static void __drm_3dlut_to_dc_3dlut(const struct drm_color_lut *lut,
+   uint32_t lut_size,
+   struct dc_3dlut *lut3d)
+{
+   int lut_i, i;
+
+   ASSERT(lut3d && lut_size == MAX_COLOR_3DLUT_ENTRIES);
+
+   /* So far, only supports 17x17x17 3D LUT with 12-bit*/
+   lut3d->lut_3d.use_tetrahedral_9 = false;

[RFC PATCH v2 5/9] drm/amd/display: encapsulate atomic regamma operation

[Melissa Wen]

We are introducing DRM 3D LUT property to DM color pipeline in the next
patch, but so far, only for atomic interface. By checking
.set_output_transfer_func in DC drivers with MPC 3D LUT support, we can
verify that regamma is only programmed when 3D LUT programming fails. As
a groundwork to introduce 3D LUT programming and better understand each
step, detach atomic regamma programming from the crtc colocr updating
code.

Signed-off-by: Melissa Wen 
---
 .../amd/display/amdgpu_dm/amdgpu_dm_color.c   | 52 ---
 1 file changed, 33 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c
index b54ef1392895..54d95745f0f0 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c
@@ -291,6 +291,36 @@ static int __set_output_tf(struct dc_transfer_func *func,
return res ? 0 : -ENOMEM;
 }
 
+static int amdgpu_dm_set_atomic_regamma(struct dc_stream_state *stream,
+   const struct drm_color_lut *regamma_lut,
+   uint32_t regamma_size)
+{
+   int ret = 0;
+
+   if (regamma_size) {
+   /* CRTC RGM goes into RGM LUT.
+*
+* Note: here there is no implicit sRGB regamma. We are using
+* degamma calculation from color module to calculate the curve
+* from a linear base.
+*/
+   stream->out_transfer_func->type = TF_TYPE_DISTRIBUTED_POINTS;
+   stream->out_transfer_func->tf = TRANSFER_FUNCTION_LINEAR;
+
+   ret = __set_output_tf(stream->out_transfer_func, regamma_lut,
+ regamma_size);
+   } else {
+   /*
+* No CRTC RGM means we can just put the block into bypass
+* since we don't have any plane level adjustments using it.
+*/
+   stream->out_transfer_func->type = TF_TYPE_BYPASS;
+   stream->out_transfer_func->tf = TRANSFER_FUNCTION_LINEAR;
+   }
+
+   return ret;
+}
+
 /**
  * __set_input_tf - calculates the input transfer function based on expected
  * input space.
@@ -438,27 +468,11 @@ int amdgpu_dm_update_crtc_color_mgmt(struct dm_crtc_state 
*crtc)
regamma_size, has_rom);
if (r)
return r;
-   } else if (has_regamma) {
-   /* CRTC RGM goes into RGM LUT.
-*
-* Note: here there is no implicit sRGB regamma. We are using
-* degamma calculation from color module to calculate the curve
-* from a linear base.
-*/
-   stream->out_transfer_func->type = TF_TYPE_DISTRIBUTED_POINTS;
-   stream->out_transfer_func->tf = TRANSFER_FUNCTION_LINEAR;
-
-   r = __set_output_tf(stream->out_transfer_func, regamma_lut,
-   regamma_size);
+   } else {
+   regamma_size = has_regamma ? regamma_size : 0;
+   r = amdgpu_dm_set_atomic_regamma(stream, regamma_lut, 
regamma_size);
if (r)
return r;
-   } else {
-   /*
-* No CRTC RGM means we can just put the block into bypass
-* since we don't have any plane level adjustments using it.
-*/
-   stream->out_transfer_func->type = TF_TYPE_BYPASS;
-   stream->out_transfer_func->tf = TRANSFER_FUNCTION_LINEAR;
}
 
/*
-- 
2.35.1

[RFC PATCH v2 3/9] drm/drm_color_mgmt: add shaper LUT to color mgmt properties

[Melissa Wen]

Shaper LUT is used to shape the contect after blending, i.e.,
de-linearize space before applying 3D LUT color correction. In the next
patch, we are adding 3D LUT property to DRM color mgmt.

Signed-off-by: Melissa Wen 
---
 drivers/gpu/drm/drm_atomic_state_helper.c |  4 +++
 drivers/gpu/drm/drm_atomic_uapi.c | 10 
 drivers/gpu/drm/drm_color_mgmt.c  | 31 ++-
 drivers/gpu/drm/drm_fb_helper.c   |  3 +++
 drivers/gpu/drm/drm_mode_config.c | 14 ++
 include/drm/drm_crtc.h| 14 --
 include/drm/drm_mode_config.h | 12 +
 7 files changed, 79 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c 
b/drivers/gpu/drm/drm_atomic_state_helper.c
index 3b6d3bdbd099..bf3222fbf4bb 100644
--- a/drivers/gpu/drm/drm_atomic_state_helper.c
+++ b/drivers/gpu/drm/drm_atomic_state_helper.c
@@ -139,8 +139,11 @@ void __drm_atomic_helper_crtc_duplicate_state(struct 
drm_crtc *crtc,
drm_property_blob_get(state->degamma_lut);
if (state->ctm)
drm_property_blob_get(state->ctm);
+   if (state->shaper_lut)
+   drm_property_blob_get(state->shaper_lut);
if (state->gamma_lut)
drm_property_blob_get(state->gamma_lut);
+
state->mode_changed = false;
state->active_changed = false;
state->planes_changed = false;
@@ -212,6 +215,7 @@ void __drm_atomic_helper_crtc_destroy_state(struct 
drm_crtc_state *state)
drm_property_blob_put(state->mode_blob);
drm_property_blob_put(state->degamma_lut);
drm_property_blob_put(state->ctm);
+   drm_property_blob_put(state->shaper_lut);
drm_property_blob_put(state->gamma_lut);
 }
 EXPORT_SYMBOL(__drm_atomic_helper_crtc_destroy_state);
diff --git a/drivers/gpu/drm/drm_atomic_uapi.c 
b/drivers/gpu/drm/drm_atomic_uapi.c
index 434f3d4cb8a2..ee78f4f5976d 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -429,6 +429,14 @@ static int drm_atomic_crtc_set_property(struct drm_crtc 
*crtc,
);
state->color_mgmt_changed |= replaced;
return ret;
+   } else if (property == config->shaper_lut_property) {
+   ret = drm_atomic_replace_property_blob_from_id(dev,
+   >shaper_lut,
+   val,
+   -1, sizeof(struct drm_color_lut),
+   );
+   state->color_mgmt_changed |= replaced;
+   return ret;
} else if (property == config->gamma_lut_property) {
ret = drm_atomic_replace_property_blob_from_id(dev,
>gamma_lut,
@@ -480,6 +488,8 @@ drm_atomic_crtc_get_property(struct drm_crtc *crtc,
*val = (state->degamma_lut) ? state->degamma_lut->base.id : 0;
else if (property == config->ctm_property)
*val = (state->ctm) ? state->ctm->base.id : 0;
+   else if (property == config->shaper_lut_property)
+   *val = (state->shaper_lut) ? state->shaper_lut->base.id : 0;
else if (property == config->gamma_lut_property)
*val = (state->gamma_lut) ? state->gamma_lut->base.id : 0;
else if (property == config->prop_out_fence_ptr)
diff --git a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c
index 17c6c3eefcd6..873cca35f078 100644
--- a/drivers/gpu/drm/drm_color_mgmt.c
+++ b/drivers/gpu/drm/drm_color_mgmt.c
@@ -69,6 +69,24 @@
  * boot-up state too. Drivers can access the blob for the color conversion
  * matrix through _crtc_state.ctm.
  *
+ * “SHAPER_LUT”:
+ * Blob property to set the shaper lut shaping pixel data after the color
+ * transformation matrix and before applying 3D Lookup Table (3D LUT). It
+ * can be used to delinearize content to get an effective 3D LUT mapping.
+ * The data is interpreted as an array of  drm_color_lut elements.
+ *
+ * Setting this to NULL (blob property value set to 0) means the output
+ * color is identical to the input color. This is generally the driver
+ * boot-up state too. Drivers can access this blob through
+ * _crtc_state.gamma_lut.
+ *
+ * “SHAPER_LUT_SIZE”:
+ * Unsigned range property to give the size of the shaper lookup table to
+ * be set on the SHAPER_LUT property (the size depends on the underlying
+ * hardware). If drivers support multiple LUT sizes then they should
+ * publish the largest size, and sub-sample smaller sized LUTs
+ * appropriately.
+ *
  * “GAMMA_LUT”:
  * Blob property to set the gamma lookup table (LUT) mapping pixel data
  * after the transformation matrix to data sent to the connector. The
@@ -153,13 +171,12 @@ EXPORT_SYMBOL(drm_color_ctm_s31_32_to_qm_n);
  * @has_ctm: whether to attach 

[RFC PATCH v2 4/9] drm/drm_color_mgmt: add 3D LUT to color mgmt properties

[Melissa Wen]

Add 3D LUT for gammar correction using a 3D lookup table. The position
in the color correction pipeline where 3D LUT is applied depends on hw
design, being after CTM or gamma. If just after CTM, a shaper lut must
be set to shape the content for a non-linear space. That details should
be handled by the driver according to its color capabilities.

Signed-off-by: Melissa Wen 
---
 drivers/gpu/drm/drm_atomic_state_helper.c |  3 ++
 drivers/gpu/drm/drm_atomic_uapi.c | 10 
 drivers/gpu/drm/drm_color_mgmt.c  | 58 +++
 drivers/gpu/drm/drm_fb_helper.c   |  2 +
 drivers/gpu/drm/drm_mode_config.c | 14 ++
 include/drm/drm_color_mgmt.h  |  4 ++
 include/drm/drm_crtc.h| 12 -
 include/drm/drm_mode_config.h | 13 +
 8 files changed, 115 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c 
b/drivers/gpu/drm/drm_atomic_state_helper.c
index bf3222fbf4bb..cad579c66248 100644
--- a/drivers/gpu/drm/drm_atomic_state_helper.c
+++ b/drivers/gpu/drm/drm_atomic_state_helper.c
@@ -141,6 +141,8 @@ void __drm_atomic_helper_crtc_duplicate_state(struct 
drm_crtc *crtc,
drm_property_blob_get(state->ctm);
if (state->shaper_lut)
drm_property_blob_get(state->shaper_lut);
+   if (state->lut3d)
+   drm_property_blob_get(state->lut3d);
if (state->gamma_lut)
drm_property_blob_get(state->gamma_lut);
 
@@ -216,6 +218,7 @@ void __drm_atomic_helper_crtc_destroy_state(struct 
drm_crtc_state *state)
drm_property_blob_put(state->degamma_lut);
drm_property_blob_put(state->ctm);
drm_property_blob_put(state->shaper_lut);
+   drm_property_blob_put(state->lut3d);
drm_property_blob_put(state->gamma_lut);
 }
 EXPORT_SYMBOL(__drm_atomic_helper_crtc_destroy_state);
diff --git a/drivers/gpu/drm/drm_atomic_uapi.c 
b/drivers/gpu/drm/drm_atomic_uapi.c
index ee78f4f5976d..041b00faebe9 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -437,6 +437,14 @@ static int drm_atomic_crtc_set_property(struct drm_crtc 
*crtc,
);
state->color_mgmt_changed |= replaced;
return ret;
+   } else if (property == config->lut3d_property) {
+   ret = drm_atomic_replace_property_blob_from_id(dev,
+   >lut3d,
+   val,
+   -1, sizeof(struct drm_color_lut),
+   );
+   state->color_mgmt_changed |= replaced;
+   return ret;
} else if (property == config->gamma_lut_property) {
ret = drm_atomic_replace_property_blob_from_id(dev,
>gamma_lut,
@@ -490,6 +498,8 @@ drm_atomic_crtc_get_property(struct drm_crtc *crtc,
*val = (state->ctm) ? state->ctm->base.id : 0;
else if (property == config->shaper_lut_property)
*val = (state->shaper_lut) ? state->shaper_lut->base.id : 0;
+   else if (property == config->lut3d_property)
+   *val = (state->lut3d) ? state->lut3d->base.id : 0;
else if (property == config->gamma_lut_property)
*val = (state->gamma_lut) ? state->gamma_lut->base.id : 0;
else if (property == config->prop_out_fence_ptr)
diff --git a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c
index 873cca35f078..a87bfb866bcb 100644
--- a/drivers/gpu/drm/drm_color_mgmt.c
+++ b/drivers/gpu/drm/drm_color_mgmt.c
@@ -87,6 +87,25 @@
  * publish the largest size, and sub-sample smaller sized LUTs
  * appropriately.
  *
+ * “LUT3D”:
+ * Blob property to set the 3D LUT mapping pixel data after the color
+ * transformation matrix and before gamma 1D lut correction. The
+ * data is interpreted as an array of  drm_color_lut elements.
+ * Hardware might choose not to use the full precision of the LUT
+ * elements.
+ *
+ * Setting this to NULL (blob property value set to 0) means a the output
+ * color is identical to the input color. This is generally the driver
+ * boot-up state too. Drivers can access this blob through
+ * _crtc_state.gamma_lut.
+ *
+ * “LUT3D_SIZE”:
+ * Unsigned range property to give the size of the 3D lookup table to be
+ * set on the LUT3D property (the size depends on the underlying
+ * hardware). If drivers support multiple LUT sizes then they should
+ * publish the largest size, and sub-sample smaller sized LUTs
+ * appropriately.
+ *
  * “GAMMA_LUT”:
  * Blob property to set the gamma lookup table (LUT) mapping pixel data
  * after the transformation matrix to data sent to the connector. The
@@ -208,6 +227,45 @@ void drm_crtc_enable_color_mgmt(struct drm_crtc *crtc,
 }
 

[RFC PATCH v2 2/9] drm/amd/display: add comments to describe DM crtc color mgmt behavior

[Melissa Wen]

Describe some expected behavior of the AMD DM color mgmt programming.

Signed-off-by: Melissa Wen 
---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c | 13 -
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c
index 10a29d131424..b54ef1392895 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c
@@ -428,12 +428,23 @@ int amdgpu_dm_update_crtc_color_mgmt(struct dm_crtc_state 
*crtc)
stream->out_transfer_func->type = TF_TYPE_DISTRIBUTED_POINTS;
stream->out_transfer_func->tf = TRANSFER_FUNCTION_SRGB;
 
+   /* Note: even if we pass has_rom as parameter here, we never
+* actually use ROM because the color module only takes the ROM
+* path if transfer_func->type == PREDEFINED.
+*
+* See more in mod_color_calculate_regamma_params()
+*/
r = __set_legacy_tf(stream->out_transfer_func, regamma_lut,
regamma_size, has_rom);
if (r)
return r;
} else if (has_regamma) {
-   /* If atomic regamma, CRTC RGM goes into RGM LUT. */
+   /* CRTC RGM goes into RGM LUT.
+*
+* Note: here there is no implicit sRGB regamma. We are using
+* degamma calculation from color module to calculate the curve
+* from a linear base.
+*/
stream->out_transfer_func->type = TF_TYPE_DISTRIBUTED_POINTS;
stream->out_transfer_func->tf = TRANSFER_FUNCTION_LINEAR;
 
-- 
2.35.1

[RFC PATCH v2 1/9] drm/amd/display: remove unused regamma condition

[Melissa Wen]

The function __set_output_tf is only called by
amdgpu_dm_update_crtc_color_mgmt() when using atomic regamma. In this
situation, func->tf == TRANSFER_FUNCTION_LINEAR (the original if
condition) and it never falls into tf != LINEAR (the else condition).
Therefore, remove unused condition to avoid misunderstandings here.

Signed-off-by: Melissa Wen 
---
 .../amd/display/amdgpu_dm/amdgpu_dm_color.c   | 32 ++-
 1 file changed, 10 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c
index a4cb23d059bd..10a29d131424 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c
@@ -255,14 +255,13 @@ static int __set_legacy_tf(struct dc_transfer_func *func,
  * @func: transfer function
  * @lut: lookup table that defines the color space
  * @lut_size: size of respective lut
- * @has_rom: if ROM can be used for hardcoded curve
  *
  * Returns:
  * 0 in case of success. -ENOMEM if fails.
  */
 static int __set_output_tf(struct dc_transfer_func *func,
-  const struct drm_color_lut *lut, uint32_t lut_size,
-  bool has_rom)
+  const struct drm_color_lut *lut,
+  uint32_t lut_size)
 {
struct dc_gamma *gamma = NULL;
struct calculate_buffer cal_buffer = {0};
@@ -279,24 +278,13 @@ static int __set_output_tf(struct dc_transfer_func *func,
gamma->num_entries = lut_size;
__drm_lut_to_dc_gamma(lut, gamma, false);
 
-   if (func->tf == TRANSFER_FUNCTION_LINEAR) {
-   /*
-* Color module doesn't like calculating regamma params
-* on top of a linear input. But degamma params can be used
-* instead to simulate this.
-*/
-   gamma->type = GAMMA_CUSTOM;
-   res = mod_color_calculate_degamma_params(NULL, func,
-   gamma, true);
-   } else {
-   /*
-* Assume sRGB. The actual mapping will depend on whether the
-* input was legacy or not.
-*/
-   gamma->type = GAMMA_CS_TFM_1D;
-   res = mod_color_calculate_regamma_params(func, gamma, false,
-has_rom, NULL, 
_buffer);
-   }
+   /*
+* Color module doesn't like calculating regamma params
+* on top of a linear input. But degamma params can be used
+* instead to simulate this.
+*/
+   gamma->type = GAMMA_CUSTOM;
+   res = mod_color_calculate_degamma_params(NULL, func, gamma, true);
 
dc_gamma_release();
 
@@ -450,7 +438,7 @@ int amdgpu_dm_update_crtc_color_mgmt(struct dm_crtc_state 
*crtc)
stream->out_transfer_func->tf = TRANSFER_FUNCTION_LINEAR;
 
r = __set_output_tf(stream->out_transfer_func, regamma_lut,
-   regamma_size, has_rom);
+   regamma_size);
if (r)
return r;
} else {
-- 
2.35.1

[RFC PATCH v2 0/9] Enable 3D LUT to AMD display drivers

[Melissa Wen]

Hi,

>From all feedback at [3DLUT_RFC] and an extensive AMD driver
examination, here I am back with a first attempt to wire up a user 3D
LUT (post-blending) to DC interface via DM CRTC color mgmt :)

I'm following some specific approaches to handle user shaper LUT and
user 3D LUT that I would like to validate if the path taken is correct.

I used a modified version [igt_tests] of Ville's IGT 3D LUT test to
verify that the shaper and 3D LUT programming is happening. However, I
still have doubts about hw behavior and DC MPC's current implementation
for 3D LUT.

Despite some initial patches for code cleanup and DRM interface, my
focus here is the inclusion of a user 3D LUT in the Display Manager,
which is done in the last five patches of this series:

- drm/amd/display: enable DRM shaper and 3D LUT properties
- drm/amd/display: update lut3d and shaper lut to stream
- drm/amd/display: add user shaper LUT support to amdgpu_dm color pipeline
- drm/amd/display: add user 3D LUT support to the amdgpu_dm color pipeline
- drm/amd/display: encapsulate atomic regamma operation

Things to take into account:

- 3D LUT (and shaper LUT) is only available in the atomic pipeline (I
  didn't work on any implicit conversions that are done in the legacy
  path)

- Therefore, I'm not doing any implicit conversions for shaper LUT
  considering the input space, which means: it's set or not. When there
  is no shaper LUT, it's set to BYPASS, but unfortunately, it seems that
  the BYPASS mode for shaper LUT is not supported in the current DC
  dcn30_set_mpc_shaper_3dlut(), since it returns false when
  mpc3_program_shaper returns false (no params). Is the combination of a
  user 3D LUT with a bypassed shaper LUT accepted by the hw?

- I also see in dcn30_set_mpc_shaper_3dlut() that some bits need to be
  set in lut3d_func to have the 3D LUT programmed on the MPC block. In
  this sense, I used the dc_acquire_release_mpc_3dlut() function to get
  the lut3d_func from the resource pool, but I'm not sure if the timing to
  acquire and release the lut3d_func from the resource pool is correct
  (and if I can really use it directly or I should make a copy).

- Still, on this topic, I use for lut3d the same bit.out_tf to update
  the stream in the commit_tail because it triggers
  .set_output_transfer_func that is in charge of setting both OGAM and 3D
  LUT on MPC. There is a chance I got it wrong here, so I appreciate any
  input on this topic.

- Finally, in set_output_transfer_func, AFAIU, even if a user OGAM is
  set, it won't be programmed if the shaper LUT and 3D LUT programming
  are successful. However, if shaper/3DLUT programming fails, OGAM can be
  considered. Should DM only accept DRM regamma if no DRM 3D LUT is
  passed, or allowing the programming of both is still desirable?

Regarding the other patches:

- drm/drm_color_mgmt: add 3D LUT to color mgmt properties
- drm/drm_color_mgmt: add shaper LUT to color mgmt properties

Here, an initial DRM 3D LUT interface is exposed to enable the entire
kernel path and is only available for the atomic pipeline. So far, it
only includes the LUT data and its size, but improvements on this
interface may also add stride and bit depth [VA_API].
Additionally, I'm aware of the current work on exposing a color pipeline
API [KMS_pipe_API].

I'm including some minor changes in this series that I made to better
understand the current DM color mgmt behavior:

- drm/amd/display: add comments to describe DM crtc color mgmt behavior
- drm/amd/display: remove unused regamma condition

...but, there are other code cleanup patches that I'm not including in
this series to avoid unnecessary noise.

You can check the entire work here:
https://gitlab.freedesktop.org/mwen/linux-amd/-/commits/drm_lut3d

[igt_tests] IGT exploratory tests here:
https://gitlab.freedesktop.org/mwen/igt-gpu-tools/-/commits/kms_color_3dlut

[3DLUT_RFC] 
https://lore.kernel.org/amd-gfx/20220619223104.667413-1-m...@igalia.com/
[VA_API] 
http://intel.github.io/libva/structVAProcFilterParameterBuffer3DLUT.html
[KMS_pipe_API] https://gitlab.freedesktop.org/pq/color-and-hdr/-/issues/11

Let me know your thoughts.

Thanks in advance,

Melissa

Melissa Wen (9):
  drm/amd/display: remove unused regamma condition
  drm/amd/display: add comments to describe DM crtc color mgmt behavior
  drm/drm_color_mgmt: add shaper LUT to color mgmt properties
  drm/drm_color_mgmt: add 3D LUT to color mgmt properties
  drm/amd/display: encapsulate atomic regamma operation
  drm/amd/display: add user 3D LUT support to the amdgpu_dm color
pipeline
  drm/amd/display: add user shaper LUT support to amdgpu_dm color
pipeline
  drm/amd/display: update lut3d and shaper lut to stream
  drm/amd/display: enable DRM shaper and 3D LUT properties

 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c |   6 +
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h |   5 +
 .../amd/display/amdgpu_dm/amdgpu_dm_color.c   | 297 --
 

Re: [PATCH] drm/amd/display: fix memory leak when using debugfs_lookup()

[Greg Kroah-Hartman]

On Tue, Sep 06, 2022 at 11:39:44AM -0400, Rodrigo Siqueira Jordao wrote:
> 
> 
> On 2022-09-06 11:06, Greg Kroah-Hartman wrote:
> > On Tue, Sep 06, 2022 at 10:52:28AM -0400, Rodrigo Siqueira Jordao wrote:
> > > 
> > > 
> > > On 2022-09-02 09:10, Greg Kroah-Hartman wrote:
> > > > On Fri, Sep 02, 2022 at 03:01:05PM +0200, Greg Kroah-Hartman wrote:
> > > > > When calling debugfs_lookup() the result must have dput() called on 
> > > > > it,
> > > > > otherwise the memory will leak over time.  Fix this up by properly
> > > > > calling dput().
> > > > > 
> > > > > Cc: Harry Wentland 
> > > > > Cc: Leo Li 
> > > > > Cc: Rodrigo Siqueira 
> > > > > Cc: Alex Deucher 
> > > > > Cc: "Christian König" 
> > > > > Cc: "Pan, Xinhui" 
> > > > > Cc: David Airlie 
> > > > > Cc: Daniel Vetter 
> > > > > Cc: Wayne Lin 
> > > > > Cc: hersen wu 
> > > > > Cc: Wenjing Liu 
> > > > > Cc: Patrik Jakobsson 
> > > > > Cc: Thelford Williams 
> > > > > Cc: Fangzhi Zuo 
> > > > > Cc: Yongzhi Liu 
> > > > > Cc: Mikita Lipski 
> > > > > Cc: Jiapeng Chong 
> > > > > Cc: Bhanuprakash Modem 
> > > > > Cc: Sean Paul 
> > > > > Cc: amd-gfx@lists.freedesktop.org
> > > > > Cc: dri-de...@lists.freedesktop.org
> > > > > Signed-off-by: Greg Kroah-Hartman 
> > > > > ---
> > > > 
> > > > Despite a zillion cc: items, I forgot to cc: stable on this.  Can the
> > > > maintainer add that here, or do you all want me to resend it with that
> > > > item added?
> > > > 
> > > > thanks,
> > > > 
> > > > greg k-h
> > > 
> > > Hi Greg,
> > > 
> > > Reviewed-by: Rodrigo Siqueira 
> > > 
> > > Is 'Cc: sta...@vger.kernel.org' enough here? I can make this change 
> > > before I
> > > merge it into amd-staging-drm-next.
> > 
> > Yes, please add the cc: stable@ line to the body of the patch, sorry I
> > forgot that.
> > 
> 
> Change applied to amd-staging-drm-next, with the Cc to the stable branch.

Wonderful, thanks!

Re: [PATCH] drm/amd/display: fix memory leak when using debugfs_lookup()

[Greg Kroah-Hartman]

On Tue, Sep 06, 2022 at 10:52:28AM -0400, Rodrigo Siqueira Jordao wrote:
> 
> 
> On 2022-09-02 09:10, Greg Kroah-Hartman wrote:
> > On Fri, Sep 02, 2022 at 03:01:05PM +0200, Greg Kroah-Hartman wrote:
> > > When calling debugfs_lookup() the result must have dput() called on it,
> > > otherwise the memory will leak over time.  Fix this up by properly
> > > calling dput().
> > > 
> > > Cc: Harry Wentland 
> > > Cc: Leo Li 
> > > Cc: Rodrigo Siqueira 
> > > Cc: Alex Deucher 
> > > Cc: "Christian König" 
> > > Cc: "Pan, Xinhui" 
> > > Cc: David Airlie 
> > > Cc: Daniel Vetter 
> > > Cc: Wayne Lin 
> > > Cc: hersen wu 
> > > Cc: Wenjing Liu 
> > > Cc: Patrik Jakobsson 
> > > Cc: Thelford Williams 
> > > Cc: Fangzhi Zuo 
> > > Cc: Yongzhi Liu 
> > > Cc: Mikita Lipski 
> > > Cc: Jiapeng Chong 
> > > Cc: Bhanuprakash Modem 
> > > Cc: Sean Paul 
> > > Cc: amd-gfx@lists.freedesktop.org
> > > Cc: dri-de...@lists.freedesktop.org
> > > Signed-off-by: Greg Kroah-Hartman 
> > > ---
> > 
> > Despite a zillion cc: items, I forgot to cc: stable on this.  Can the
> > maintainer add that here, or do you all want me to resend it with that
> > item added?
> > 
> > thanks,
> > 
> > greg k-h
> 
> Hi Greg,
> 
> Reviewed-by: Rodrigo Siqueira 
> 
> Is 'Cc: sta...@vger.kernel.org' enough here? I can make this change before I
> merge it into amd-staging-drm-next.

Yes, please add the cc: stable@ line to the body of the patch, sorry I
forgot that.

thanks,

greg k-h

Re: [PATCH] drm/amd/display: Fix register definitions for DCN32/321

[Rodrigo Siqueira Jordao]

On 2022-09-01 15:27, Aurabindo Pillai wrote:

[Why & How]
Fix the instatiation sequence for MPC registers and add a few other
missing register definitions that were ommited erroneously when copying
them over to enable runtime initialization of reigster offsets for
DCN32/321

Signed-off-by: Aurabindo Pillai 
---
  .../drm/amd/display/dc/dcn32/dcn32_resource.c |  27 +--
  .../drm/amd/display/dc/dcn32/dcn32_resource.h | 216 --
  .../amd/display/dc/dcn321/dcn321_resource.c   |  24 +-
  3 files changed, 166 insertions(+), 101 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/dcn32/dcn32_resource.c 
b/drivers/gpu/drm/amd/display/dc/dcn32/dcn32_resource.c
index ef0a6d468a10..9d3b8568351e 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn32/dcn32_resource.c
+++ b/drivers/gpu/drm/amd/display/dc/dcn32/dcn32_resource.c
@@ -461,22 +461,17 @@ static const struct dcn20_dsc_mask dsc_mask = {
  };
  
  static struct dcn30_mpc_registers mpc_regs;

-#define dcn_mpc_regs_init()\
-   ( \
-   MPC_REG_LIST_DCN3_0_RI(0),\
-   MPC_REG_LIST_DCN3_0_RI(1),\
-   MPC_REG_LIST_DCN3_0_RI(2),\
-   MPC_REG_LIST_DCN3_0_RI(3),\
-   MPC_OUT_MUX_REG_LIST_DCN3_0_RI(0),\
-   MPC_OUT_MUX_REG_LIST_DCN3_0_RI(1),\
-   MPC_OUT_MUX_REG_LIST_DCN3_0_RI(2),\
-   MPC_OUT_MUX_REG_LIST_DCN3_0_RI(3),\
-   MPC_MCM_REG_LIST_DCN32_RI(0),\
-   MPC_MCM_REG_LIST_DCN32_RI(1),\
-   MPC_MCM_REG_LIST_DCN32_RI(2),\
-   MPC_MCM_REG_LIST_DCN32_RI(3),\
-   MPC_DWB_MUX_REG_LIST_DCN3_0_RI(0)\
-   )
+
+#define dcn_mpc_regs_init() \
+   MPC_REG_LIST_DCN3_2_RI(0),\
+   MPC_REG_LIST_DCN3_2_RI(1),\
+   MPC_REG_LIST_DCN3_2_RI(2),\
+   MPC_REG_LIST_DCN3_2_RI(3),\
+   MPC_OUT_MUX_REG_LIST_DCN3_0_RI(0),\
+   MPC_OUT_MUX_REG_LIST_DCN3_0_RI(1),\
+   MPC_OUT_MUX_REG_LIST_DCN3_0_RI(2),\
+   MPC_OUT_MUX_REG_LIST_DCN3_0_RI(3),\
+   MPC_DWB_MUX_REG_LIST_DCN3_0_RI(0)
  
  static const struct dcn30_mpc_shift mpc_shift = {

MPC_COMMON_MASK_SH_LIST_DCN32(__SHIFT)
diff --git a/drivers/gpu/drm/amd/display/dc/dcn32/dcn32_resource.h 
b/drivers/gpu/drm/amd/display/dc/dcn32/dcn32_resource.h
index 60d8fad16eee..4c931905223d 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn32/dcn32_resource.h
+++ b/drivers/gpu/drm/amd/display/dc/dcn32/dcn32_resource.h
@@ -222,7 +222,8 @@ void dcn32_determine_det_override(struct dc_state *context, 
display_e2e_pipe_par
SRI_ARR(DP_MSA_TIMING_PARAM4, DP, id),  
 \
SRI_ARR(DP_MSE_RATE_CNTL, DP, id), SRI_ARR(DP_MSE_RATE_UPDATE, DP, id), 
 \
SRI_ARR(DP_PIXEL_FORMAT, DP, id), SRI_ARR(DP_SEC_CNTL, DP, id), 
 \
-  SRI_ARR(DP_SEC_CNTL2, DP, id), SRI_ARR(DP_SEC_CNTL6, DP, id),
\
+  SRI_ARR(DP_SEC_CNTL1, DP, id), SRI_ARR(DP_SEC_CNTL2, DP, id),
\
+  SRI_ARR(DP_SEC_CNTL5, DP, id), SRI_ARR(DP_SEC_CNTL6, DP, id),
\
SRI_ARR(DP_STEER_FIFO, DP, id), SRI_ARR(DP_VID_M, DP, id),  
 \
SRI_ARR(DP_VID_N, DP, id), SRI_ARR(DP_VID_STREAM_CNTL, DP, id), 
 \
SRI_ARR(DP_VID_TIMING, DP, id), SRI_ARR(DP_SEC_AUD_N, DP, id),  
 \
@@ -735,75 +736,6 @@ void dcn32_determine_det_override(struct dc_state 
*context, display_e2e_pipe_par
  #define MPC_DWB_MUX_REG_LIST_DCN3_0_RI(inst)  
 \
SRII_DWB(DWB_MUX, MUX, MPC_DWB, inst)
  
-#define MPC_MCM_REG_LIST_DCN32_RI(inst)\

-  ( \
-  SRII(MPCC_MCM_SHAPER_CONTROL, MPCC_MCM, inst),   
\
-  SRII(MPCC_MCM_SHAPER_OFFSET_R, MPCC_MCM, inst),  
\
-  SRII(MPCC_MCM_SHAPER_OFFSET_G, MPCC_MCM, inst),  
\
-  SRII(MPCC_MCM_SHAPER_OFFSET_B, MPCC_MCM, inst),  
\
-  SRII(MPCC_MCM_SHAPER_SCALE_R, MPCC_MCM, inst),   
\
-  SRII(MPCC_MCM_SHAPER_SCALE_G_B, MPCC_MCM, inst), 
\
-  SRII(MPCC_MCM_SHAPER_LUT_INDEX, MPCC_MCM, inst), 
\
-  SRII(MPCC_MCM_SHAPER_LUT_DATA, MPCC_MCM, inst),  
\
-  SRII(MPCC_MCM_SHAPER_LUT_WRITE_EN_MASK, MPCC_MCM, inst), 
\
-  SRII(MPCC_MCM_SHAPER_RAMA_START_CNTL_B, MPCC_MCM, inst), 
\
-  SRII(MPCC_MCM_SHAPER_RAMA_START_CNTL_G, MPCC_MCM, inst), 
\
-  SRII(MPCC_MCM_SHAPER_RAMA_START_CNTL_R, MPCC_MCM, inst), 
\
-  SRII(MPCC_MCM_SHAPER_RAMA_END_CNTL_B, MPCC_MCM, inst),   
\
-  SRII(MPCC_MCM_SHAPER_RAMA_END_CNTL_G, MPCC_MCM, inst),   
\
-  SRII(MPCC_MCM_SHAPER_RAMA_END_CNTL_R, MPCC_MCM, inst),   
\
-  SRII(MPCC_MCM_SHAPER_RAMA_REGION_0_1, MPCC_MCM, inst),   
\
-  SRII(MPCC_MCM_SHAPER_RAMA_REGION_2_3, 

Re: [PATCH v2 1/2] drm/amdgpu: Move HDP remapping earlier during init

[Lazar, Lijo]

On 9/6/2022 8:55 PM, Alex Deucher wrote:

On Mon, Sep 5, 2022 at 1:27 AM Lazar, Lijo  wrote:




On 9/2/2022 1:09 AM, Alex Deucher wrote:

How about this?  We should just move HDP remap to gmc hw_init since
that is mainly what uses it anyway.



Sorry, I missed to R-B the previous version. Did you find any problem
when common block is initialized first?



One of the users on the bug report reported an issue with that patch,
that said, that user seems to be seeing a slightly different issue
since he is on a gfx9 card and the original reporter was on gfx10.
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.kernel.org%2Fshow_bug.cgi%3Fid%3D216373data=05%7C01%7Clijo.lazar%40amd.com%7C5d6d4da1be4a4194b7cc08da901c0bb4%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637980747398632310%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7Csdata=hHoh2YpQ2vpjsRTEqI1vIRY4YQmzfFi5%2FG%2Fnh6vNuds%3Dreserved=0



Yes, I see Bjorn already pointed to another potential issue in LTR 
enablement. So regardless of init-common-first patch, the new error will 
be reported and that is unrelated to the original reported error through 
AER. I still think init-common-first patch is good.


Thanks,
Lijo


Alex



I think that patch provides a consistent IP init sequence during cold
init and resume.

Thanks,
Lijo


Alex

On Tue, Aug 30, 2022 at 2:05 PM Alex Deucher  wrote:


On Tue, Aug 30, 2022 at 12:06 PM Lazar, Lijo  wrote:




On 8/30/2022 8:39 PM, Alex Deucher wrote:

On Tue, Aug 30, 2022 at 10:45 AM Lazar, Lijo  wrote:




On 8/30/2022 7:18 PM, Alex Deucher wrote:

On Tue, Aug 30, 2022 at 12:05 AM Lazar, Lijo  wrote:




On 8/29/2022 10:20 PM, Alex Deucher wrote:

On Mon, Aug 29, 2022 at 4:18 AM Lijo Lazar  wrote:


HDP flush is used early in the init sequence as part of memory controller
block initialization. Hence remapping of HDP registers needed for flush
needs to happen earlier.

This also fixes the Unsupported Request error reported through AER during
driver load. The error happens as a write happens to the remap offset
before real remapping is done.

Link: 
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.kernel.org%2Fshow_bug.cgi%3Fid%3D216373data=05%7C01%7Clijo.lazar%40amd.com%7C5d6d4da1be4a4194b7cc08da901c0bb4%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637980747398632310%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7Csdata=hHoh2YpQ2vpjsRTEqI1vIRY4YQmzfFi5%2FG%2Fnh6vNuds%3Dreserved=0

The error was unnoticed before and got visible because of the commit
referenced below. This doesn't fix anything in the commit below, rather
fixes the issue in amdgpu exposed by the commit. The reference is only
to associate this commit with below one so that both go together.

Fixes: 8795e182b02d ("PCI/portdrv: Don't disable AER reporting in 
get_port_device_capability()")

Reported-by: Tom Seewald 
Signed-off-by: Lijo Lazar 
Cc: sta...@vger.kernel.org


How about something like the attached patch rather than these two
patches?  It's a bit bigger but seems cleaner and more defensive in my
opinion.



Whenever device goes to suspend/reset and then comes back, remap offset
has to be set back to 0 to make sure it doesn't use the wrong offset
when the register assumes default values again.

To avoid the if-check in hdp_flush (which is more frequent), another way
is to initialize the remap offset to default offset during early init
and hw fini/suspend sequences. It won't be obvious (even with this
patch) as to when remap offset vs default offset is used though.


On resume, the common IP is resumed first so it will always be set.
The only case that is a problem is init because we init GMC out of
order.  We could init common before GMC in amdgpu_device_ip_init().  I
think that should be fine, but I wasn't sure if there might be some
fallout from that on certain cards.



There are other places where an IP order is forced like in
amdgpu_device_ip_reinit_early_sriov(). This also may not affect this
case as remapping is not done for VF.

Agree that a better way is to have the common IP to be inited first.


How about these patches?



Looks good to me. BTW, is nbio 7.7 for an APU (in which case hdp flush
is not expected to be used)?


It would be used in some cases, e.g., GPU VM passthrough where we use
the BAR rather than the carve out.

Alex




Thanks,
Lijo


Alex




Thanks,
Lijo


Alex



Thanks,
Lijo


Alex


---
v2:
 Take care of IP resume cases (Alex Deucher)
 Add NULL check to nbio.funcs to cover older (GFXv8) ASICs (Felix 
Kuehling)
 Add more details in commit message and associate with AER patch 
(Bjorn
Helgaas)

  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 24 ++
  drivers/gpu/drm/amd/amdgpu/nv.c|  6 --
  drivers/gpu/drm/amd/amdgpu/soc15.c |  6 --
  

Re: [PATCH linux-next] drm/amd/display: Remove the unneeded result variable

[Rodrigo Siqueira Jordao]

On 2022-09-02 03:54, cgel@gmail.com wrote:

From: zhang songyi 

Return the enable_link_dp() directly instead of storing it in another
redundant variable.

Reported-by: Zeal Robot 
Signed-off-by: zhang songyi 
---
  drivers/gpu/drm/amd/display/dc/core/dc_link.c | 6 +-
  1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_link.c 
b/drivers/gpu/drm/amd/display/dc/core/dc_link.c
index f9b798b7933c..4ab27e231337 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc_link.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_link.c
@@ -2077,11 +2077,7 @@ static enum dc_status enable_link_edp(
struct dc_state *state,
struct pipe_ctx *pipe_ctx)
  {
-   enum dc_status status;
-
-   status = enable_link_dp(state, pipe_ctx);
-
-   return status;
+   return enable_link_dp(state, pipe_ctx);
  }
  
  static enum dc_status enable_link_dp_mst(


LGTM,

Reviewed-by: Rodrigo Siqueira 

and applied to amd-staging-drm-next.

Thanks
Siqueira

Re: amd iommu configuration

[Felix Kuehling]

On 2022-09-06 10:52, Robin Murphy wrote:

On 2022-09-05 15:13, Vasant Hegde wrote:

Steven,

[+Felix, amd-fgx list]


On 9/3/2022 4:29 AM, Steven J Abner wrote:

Hi
I was referred to you from linux-ker...@vger.kernel.org about the 
following issue.

Here is as was written:
On 9/1/22 11:36, Steven J Abner wrote:
Hi
Building a kernel tailored for AMD 2400g on ASRock B450 using 
5.18.12 as base.
I stumbled across an odd situation and which lacked Kconfig info and 
lead to

oddity.
/drivers/iommu/amd/Kconfig states 'config AMD_IOMMU_V2' is 
'tristate' but unlike

many
other tristate configures doesn't mention that module name is 
'iommu_v2.ko' and

loading should be done by adding to modules-load.d.

The oddity is that by loading as module is as follows (differences):

builtin iommu_v2 version dmesg:
amdgpu: HMM registered 2048MB device memory
amdgpu: Topology: Add APU node [0x0:0x0]
amdgpu: Topology: Add APU node [0x15dd:0x1002]
AMD-Vi: AMD IOMMUv2 loaded and initialized
kfd kfd: amdgpu: added device 1002:15dd
kfd kfd: amdgpu: Allocated 3969056 bytes on gart
memmap_init_zone_device initialised 524288 pages in 0ms


IOMMU V2 modules provides IOMMU feature like attaching device to
process. I think amdgpu uses those features if available.
So in this case amdgpu is using those IOMMU features.



module not loaded due to missing iommu.conf dmesg:
amdgpu: CRAT table disabled by module option
amdgpu: Topology: Add CPU node
amdgpu: Virtual CRAT table created for CPU
kfd kfd: amdgpu: GC IP 090100 not supported in kfd

module load through iommu.conf dmesg:
amdgpu: CRAT table disabled by module option
amdgpu: Topology: Add CPU node
amdgpu: Virtual CRAT table created for CPU
AMD-Vi: AMD IOMMUv2 loaded and initialized
kfd kfd: amdgpu: GC IP 090100 not supported in kfd

Note, only difference on witk/without iommu.conf is:
AMD-Vi: AMD IOMMUv2 loaded and initialized


I think in this case iommu_v2.ko module got loaded after GPU
initialized. Hence amdgpu is not using iommu v2 features.




So does this mean missing features by not having builtin?
If not, should Kconfig have hint about module and loading?


@Felix,
   I see that drivers/gpu/drm/amd/amdkfd/Kconfig contains below line
 imply AMD_IOMMU_V2 if X86_64


   Should we change `s/imply/select` ?


"select" might help when KFD is built-in, but it probably still wants 
a MODULE_SOFTDEP() to enforce load order when they're both modular.


We don't want to make it a hard dependency because only a small subset 
of all AMD GPUs use iommuv2, and we have fallbacks in place for even 
those to work without it.


Regards,
  Felix




Robin.

Re: [PATCH] drm/amd/display: fix memory leak when using debugfs_lookup()

[Rodrigo Siqueira Jordao]

On 2022-09-06 11:06, Greg Kroah-Hartman wrote:

On Tue, Sep 06, 2022 at 10:52:28AM -0400, Rodrigo Siqueira Jordao wrote:



On 2022-09-02 09:10, Greg Kroah-Hartman wrote:

On Fri, Sep 02, 2022 at 03:01:05PM +0200, Greg Kroah-Hartman wrote:

When calling debugfs_lookup() the result must have dput() called on it,
otherwise the memory will leak over time.  Fix this up by properly
calling dput().

Cc: Harry Wentland 
Cc: Leo Li 
Cc: Rodrigo Siqueira 
Cc: Alex Deucher 
Cc: "Christian König" 
Cc: "Pan, Xinhui" 
Cc: David Airlie 
Cc: Daniel Vetter 
Cc: Wayne Lin 
Cc: hersen wu 
Cc: Wenjing Liu 
Cc: Patrik Jakobsson 
Cc: Thelford Williams 
Cc: Fangzhi Zuo 
Cc: Yongzhi Liu 
Cc: Mikita Lipski 
Cc: Jiapeng Chong 
Cc: Bhanuprakash Modem 
Cc: Sean Paul 
Cc: amd-gfx@lists.freedesktop.org
Cc: dri-de...@lists.freedesktop.org
Signed-off-by: Greg Kroah-Hartman 
---


Despite a zillion cc: items, I forgot to cc: stable on this.  Can the
maintainer add that here, or do you all want me to resend it with that
item added?

thanks,

greg k-h


Hi Greg,

Reviewed-by: Rodrigo Siqueira 

Is 'Cc: sta...@vger.kernel.org' enough here? I can make this change before I
merge it into amd-staging-drm-next.


Yes, please add the cc: stable@ line to the body of the patch, sorry I
forgot that.



Change applied to amd-staging-drm-next, with the Cc to the stable branch.

Thanks
Siqueira


thanks,

greg k-h

Re: [PATCH] drm/amdgpu: recleanup coding style in amdgpu_fence.c

[Alex Deucher]

On Mon, Sep 5, 2022 at 3:40 AM Jingyu Wang  wrote:
>
> Fix everything checkpatch.pl complained about in amdgpu_fence.c,
> modified use "} else {", sent it again, thx.
>
> Signed-off-by: Jingyu Wang 
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c | 15 +--
>  1 file changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
> index 8adeb7469f1e..0759d86d92da 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
> @@ -1,3 +1,4 @@
> +// SPDX-License-Identifier: MIT
>  /*
>   * Copyright 2009 Jerome Glisse.
>   * All Rights Reserved.
> @@ -42,7 +43,6 @@
>  #include "amdgpu_reset.h"
>
>  /*
> - * Fences

What is the point of this change?

Alex

>   * Fences mark an event in the GPUs pipeline and are used
>   * for GPU/CPU synchronization.  When the fence is written,
>   * it is expected that all buffers associated with that fence
> @@ -139,7 +139,7 @@ static u32 amdgpu_fence_read(struct amdgpu_ring *ring)
>   * Returns 0 on success, -ENOMEM on failure.
>   */
>  int amdgpu_fence_emit(struct amdgpu_ring *ring, struct dma_fence **f, struct 
> amdgpu_job *job,
> - unsigned flags)
> + unsigned int flags)
>  {
> struct amdgpu_device *adev = ring->adev;
> struct dma_fence *fence;
> @@ -173,11 +173,11 @@ int amdgpu_fence_emit(struct amdgpu_ring *ring, struct 
> dma_fence **f, struct amd
>adev->fence_context + ring->idx, seq);
> /* Against remove in amdgpu_job_{free, free_cb} */
> dma_fence_get(fence);
> -   }
> -   else
> +   } else {
> dma_fence_init(fence, _fence_ops,
>>fence_drv.lock,
>adev->fence_context + ring->idx, seq);
> +   }
> }
>
> amdgpu_ring_emit_fence(ring, ring->fence_drv.gpu_addr,
> @@ -393,7 +393,7 @@ signed long amdgpu_fence_wait_polling(struct amdgpu_ring 
> *ring,
>   * Returns the number of emitted fences on the ring.  Used by the
>   * dynpm code to ring track activity.
>   */
> -unsigned amdgpu_fence_count_emitted(struct amdgpu_ring *ring)
> +unsigned int amdgpu_fence_count_emitted(struct amdgpu_ring *ring)
>  {
> uint64_t emitted;
>
> @@ -422,7 +422,7 @@ unsigned amdgpu_fence_count_emitted(struct amdgpu_ring 
> *ring)
>   */
>  int amdgpu_fence_driver_start_ring(struct amdgpu_ring *ring,
>struct amdgpu_irq_src *irq_src,
> -  unsigned irq_type)
> +  unsigned int irq_type)
>  {
> struct amdgpu_device *adev = ring->adev;
> uint64_t index;
> @@ -594,6 +594,7 @@ void amdgpu_fence_driver_hw_init(struct amdgpu_device 
> *adev)
>
> for (i = 0; i < AMDGPU_MAX_RINGS; i++) {
> struct amdgpu_ring *ring = adev->rings[i];
> +
> if (!ring || !ring->fence_drv.initialized)
> continue;
>
> @@ -772,6 +773,7 @@ static int amdgpu_debugfs_fence_info_show(struct seq_file 
> *m, void *unused)
>
> for (i = 0; i < AMDGPU_MAX_RINGS; ++i) {
> struct amdgpu_ring *ring = adev->rings[i];
> +
> if (!ring || !ring->fence_drv.initialized)
> continue;
>
> @@ -845,6 +847,7 @@ static void amdgpu_debugfs_reset_work(struct work_struct 
> *work)
>   reset_work);
>
> struct amdgpu_reset_context reset_context;
> +
> memset(_context, 0, sizeof(reset_context));
>
> reset_context.method = AMD_RESET_METHOD_NONE;
>
> base-commit: e47eb90a0a9ae20b82635b9b99a8d0979b757ad8
> --
> 2.34.1
>

Re: [PATCH] drm/amdkfd: print address in hex format rather than decimal

[Felix Kuehling]

On 2022-09-06 01:46, Yifan Zhang wrote:

Addresses should be printed in hex format.

Signed-off-by: Yifan Zhang 


Reviewed-by: Felix Kuehling 



---
  drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
index cbd593f7d553..2170db83e41d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
@@ -1728,7 +1728,7 @@ int amdgpu_amdkfd_gpuvm_alloc_memory_of_gpu(
add_kgd_mem_to_kfd_bo_list(*mem, avm->process_info, user_addr);
  
  	if (user_addr) {

-   pr_debug("creating userptr BO for user_addr = %llu\n", 
user_addr);
+   pr_debug("creating userptr BO for user_addr = %llx\n", 
user_addr);
ret = init_user_pages(*mem, user_addr, criu_resume);
if (ret)
goto allocate_init_user_pages_failed;

Re: [PATCH v2 1/2] drm/amdgpu: Move HDP remapping earlier during init

[Alex Deucher]

On Mon, Sep 5, 2022 at 1:27 AM Lazar, Lijo  wrote:
>
>
>
> On 9/2/2022 1:09 AM, Alex Deucher wrote:
> > How about this?  We should just move HDP remap to gmc hw_init since
> > that is mainly what uses it anyway.
> >
>
> Sorry, I missed to R-B the previous version. Did you find any problem
> when common block is initialized first?
>

One of the users on the bug report reported an issue with that patch,
that said, that user seems to be seeing a slightly different issue
since he is on a gfx9 card and the original reporter was on gfx10.
https://bugzilla.kernel.org/show_bug.cgi?id=216373

Alex


> I think that patch provides a consistent IP init sequence during cold
> init and resume.
>
> Thanks,
> Lijo
>
> > Alex
> >
> > On Tue, Aug 30, 2022 at 2:05 PM Alex Deucher  wrote:
> >>
> >> On Tue, Aug 30, 2022 at 12:06 PM Lazar, Lijo  wrote:
> >>>
> >>>
> >>>
> >>> On 8/30/2022 8:39 PM, Alex Deucher wrote:
>  On Tue, Aug 30, 2022 at 10:45 AM Lazar, Lijo  wrote:
> >
> >
> >
> > On 8/30/2022 7:18 PM, Alex Deucher wrote:
> >> On Tue, Aug 30, 2022 at 12:05 AM Lazar, Lijo  
> >> wrote:
> >>>
> >>>
> >>>
> >>> On 8/29/2022 10:20 PM, Alex Deucher wrote:
>  On Mon, Aug 29, 2022 at 4:18 AM Lijo Lazar  
>  wrote:
> >
> > HDP flush is used early in the init sequence as part of memory 
> > controller
> > block initialization. Hence remapping of HDP registers needed for 
> > flush
> > needs to happen earlier.
> >
> > This also fixes the Unsupported Request error reported through AER 
> > during
> > driver load. The error happens as a write happens to the remap 
> > offset
> > before real remapping is done.
> >
> > Link: 
> > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.kernel.org%2Fshow_bug.cgi%3Fid%3D216373data=05%7C01%7Clijo.lazar%40amd.com%7C4e59ae0f8ed54aa7c5a208da8c51aa29%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637976579623485975%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7Csdata=WTcd9ImY7Oba0MT6oQ7%2B7UEBkdS6azvqgYoK%2B%2F4mJPg%3Dreserved=0
> >
> > The error was unnoticed before and got visible because of the commit
> > referenced below. This doesn't fix anything in the commit below, 
> > rather
> > fixes the issue in amdgpu exposed by the commit. The reference is 
> > only
> > to associate this commit with below one so that both go together.
> >
> > Fixes: 8795e182b02d ("PCI/portdrv: Don't disable AER reporting in 
> > get_port_device_capability()")
> >
> > Reported-by: Tom Seewald 
> > Signed-off-by: Lijo Lazar 
> > Cc: sta...@vger.kernel.org
> 
>  How about something like the attached patch rather than these two
>  patches?  It's a bit bigger but seems cleaner and more defensive in 
>  my
>  opinion.
> 
> >>>
> >>> Whenever device goes to suspend/reset and then comes back, remap 
> >>> offset
> >>> has to be set back to 0 to make sure it doesn't use the wrong offset
> >>> when the register assumes default values again.
> >>>
> >>> To avoid the if-check in hdp_flush (which is more frequent), another 
> >>> way
> >>> is to initialize the remap offset to default offset during early init
> >>> and hw fini/suspend sequences. It won't be obvious (even with this
> >>> patch) as to when remap offset vs default offset is used though.
> >>
> >> On resume, the common IP is resumed first so it will always be set.
> >> The only case that is a problem is init because we init GMC out of
> >> order.  We could init common before GMC in amdgpu_device_ip_init().  I
> >> think that should be fine, but I wasn't sure if there might be some
> >> fallout from that on certain cards.
> >>
> >
> > There are other places where an IP order is forced like in
> > amdgpu_device_ip_reinit_early_sriov(). This also may not affect this
> > case as remapping is not done for VF.
> >
> > Agree that a better way is to have the common IP to be inited first.
> 
>  How about these patches?
> 
> >>>
> >>> Looks good to me. BTW, is nbio 7.7 for an APU (in which case hdp flush
> >>> is not expected to be used)?
> >>
> >> It would be used in some cases, e.g., GPU VM passthrough where we use
> >> the BAR rather than the carve out.
> >>
> >> Alex
> >>
> >>
> >>>
> >>> Thanks,
> >>> Lijo
> >>>
>  Alex
> 
> 
> >
> > Thanks,
> > Lijo
> >
> >> Alex
> >>
> >>>
> >>> Thanks,
> >>> Lijo
> >>>
>  Alex
> 
> > ---
> > v2:
> > Take care of IP resume cases (Alex Deucher)
> > Add NULL check to nbio.funcs to cover older 

Re: [PATCH] drm/amdgpu: cleanup coding style in amdgpu_atpx_handler.c

[Alex Deucher]

On Mon, Sep 5, 2022 at 2:29 AM Jingyu Wang  wrote:
>
> Fix everything checkpatch.pl complained about in amdgpu_atpx_handler.c
>
> Signed-off-by: Jingyu Wang 
> ---
>  .../gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c  | 27 +++
>  1 file changed, 16 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c
> index d6d986be906a..911d6a130ec5 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c
> @@ -74,24 +74,29 @@ struct atpx_mux {
> u16 mux;
>  } __packed;
>
> -bool amdgpu_has_atpx(void) {
> +bool amdgpu_has_atpx(void)
> +{
> return amdgpu_atpx_priv.atpx_detected;
>  }
>
> -bool amdgpu_has_atpx_dgpu_power_cntl(void) {
> +bool amdgpu_has_atpx_dgpu_power_cntl(void)
> +{
> return amdgpu_atpx_priv.atpx.functions.power_cntl;
>  }
>
> -bool amdgpu_is_atpx_hybrid(void) {
> +bool amdgpu_is_atpx_hybrid(void)
> +{
> return amdgpu_atpx_priv.atpx.is_hybrid;
>  }
>
> -bool amdgpu_atpx_dgpu_req_power_for_displays(void) {
> +bool amdgpu_atpx_dgpu_req_power_for_displays(void)
> +{
> return amdgpu_atpx_priv.atpx.dgpu_req_power_for_displays;
>  }
>
>  #if defined(CONFIG_ACPI)
> -void *amdgpu_atpx_get_dhandle(void) {
> +void *amdgpu_atpx_get_dhandle(void)
> +{
> return amdgpu_atpx_priv.dhandle;
>  }
>  #endif
> @@ -134,7 +139,7 @@ static union acpi_object *amdgpu_atpx_call(acpi_handle 
> handle, int function,
>
> /* Fail only if calling the method fails and ATPX is supported */
> if (ACPI_FAILURE(status) && status != AE_NOT_FOUND) {
> -   printk("failed to evaluate ATPX got %s\n",
> +   DRM_WARN("failed to evaluate ATPX got %s\n",

I'm not sure these need to be warnings.  Also, please use the
dev_info() functions instead so we can tell which device on the system
is reporting the issue.

>acpi_format_exception(status));
> kfree(buffer.pointer);
> return NULL;
> @@ -190,7 +195,7 @@ static int amdgpu_atpx_validate(struct amdgpu_atpx *atpx)
>
> size = *(u16 *) info->buffer.pointer;
> if (size < 10) {
> -   printk("ATPX buffer is too small: %zu\n", size);
> +   DRM_WARN("ATPX buffer is too small: %zu\n", size);
> kfree(info);
> return -EINVAL;
> }
> @@ -223,11 +228,11 @@ static int amdgpu_atpx_validate(struct amdgpu_atpx 
> *atpx)
> atpx->is_hybrid = false;
> if (valid_bits & ATPX_MS_HYBRID_GFX_SUPPORTED) {
> if (amdgpu_atpx_priv.quirks & AMDGPU_PX_QUIRK_FORCE_ATPX) {
> -   printk("ATPX Hybrid Graphics, forcing to ATPX\n");
> +   DRM_WARN("ATPX Hybrid Graphics, forcing to ATPX\n");
> atpx->functions.power_cntl = true;
> atpx->is_hybrid = false;
> } else {
> -   printk("ATPX Hybrid Graphics\n");
> +   DRM_WARN("ATPX Hybrid Graphics\n");

These are definitely not warnings.  Please use dev_info() here.

> /*
>  * Disable legacy PM methods only when pcie port PM 
> is usable,
>  * otherwise the device might fail to power off or 
> power on.
> @@ -269,7 +274,7 @@ static int amdgpu_atpx_verify_interface(struct 
> amdgpu_atpx *atpx)
>
> size = *(u16 *) info->buffer.pointer;
> if (size < 8) {
> -   printk("ATPX buffer is too small: %zu\n", size);
> +   DRM_WARN("ATPX buffer is too small: %zu\n", size);
> err = -EINVAL;
> goto out;
> }
> @@ -278,7 +283,7 @@ static int amdgpu_atpx_verify_interface(struct 
> amdgpu_atpx *atpx)
> memcpy(, info->buffer.pointer, size);
>
> /* TODO: check version? */
> -   printk("ATPX version %u, functions 0x%08x\n",
> +   DRM_WARN("ATPX version %u, functions 0x%08x\n",

Same here.

>output.version, output.function_bits);
>
> amdgpu_atpx_parse_functions(>functions, output.function_bits);
>
> base-commit: e47eb90a0a9ae20b82635b9b99a8d0979b757ad8
> --
> 2.34.1
>

Re: [PATCH] drm/amdgpu: getting fan speed pwm for vega10 properly

[Deucher, Alexander]

[Public]

@Quan, Evan, @Feng, 
Kenneth can you take a look?

Thanks,

Alex

From: amd-gfx  on behalf of Yury 
Zhuravlev 
Sent: Friday, September 2, 2022 1:24 PM
To: amd-gfx@lists.freedesktop.org 
Subject: [PATCH] drm/amdgpu: getting fan speed pwm for vega10 properly

Hello,

During the setup, the fan manager 
https://github.com/markusressel/fan2go
 I found that my Vega56 was not working correctly. This fan manager expects 
what read PWM value should be the same as you wrote before, but it's not the 
case. PWM value was volatile, and what is more critical, if I wrote 200, after 
reading I saw ~70-100, which is very confusing.
After that, I started reading the amdgpu driver, and how fan speed works, and I 
found what PWM value was calculated from RPM speed and not correct for my case 
(different BIOS or fan configuration?).
Because it looked wrong, I started looking into different implementations and 
found that Vega20 used mmCG_FDO_CTRL1 and mmCG_THERMAL_STATUS registers to 
calculate the PWM value.
I also checked how we set PWM for Vega10 and found the same registers. After 
that, I copy-pasted the function from Vega20 to Vega10, and it started working 
much better. It still has some fluctuation, but as I understand, this behavior 
is expected.

I have no in-depth information about amdgpu, and the original function may have 
been for some reason (maybe for some broken BIOS?), but I suppose somebody 
forgot to backport this code after prototype implementation.

It would be my first patch here. Sorry if I skipped some procedures, will be 
appreciated it if you help me.
Also, sorry for the patch in the attachment, I have not been using any mail 
programs for the last six years, only web clients, and it's strange to do it 
nowadays (PRs much more common...).

Regards,
diff --git a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega10_thermal.c b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega10_thermal.c
index dad3e3741a4e..190af79f3236 100644
--- a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega10_thermal.c
+++ b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega10_thermal.c
@@ -67,22 +67,21 @@ int vega10_fan_ctrl_get_fan_speed_info(struct pp_hwmgr *hwmgr,
 int vega10_fan_ctrl_get_fan_speed_pwm(struct pp_hwmgr *hwmgr,
 		uint32_t *speed)
 {
-	uint32_t current_rpm;
-	uint32_t percent = 0;
-
-	if (hwmgr->thermal_controller.fanInfo.bNoFan)
-		return 0;
+	struct amdgpu_device *adev = hwmgr->adev;
+	uint32_t duty100, duty;
+	uint64_t tmp64;
 
-	if (vega10_get_current_rpm(hwmgr, _rpm))
-		return -1;
+	duty100 = REG_GET_FIELD(RREG32_SOC15(THM, 0, mmCG_FDO_CTRL1),
+CG_FDO_CTRL1, FMAX_DUTY100);
+	duty = REG_GET_FIELD(RREG32_SOC15(THM, 0, mmCG_THERMAL_STATUS),
+CG_THERMAL_STATUS, FDO_PWM_DUTY);
 
-	if (hwmgr->thermal_controller.
-			advanceFanControlParameters.usMaxFanRPM != 0)
-		percent = current_rpm * 255 /
-			hwmgr->thermal_controller.
-			advanceFanControlParameters.usMaxFanRPM;
+	if (!duty100)
+		return -EINVAL;
 
-	*speed = MIN(percent, 255);
+	tmp64 = (uint64_t)duty * 255;
+	do_div(tmp64, duty100);
+	*speed = MIN((uint32_t)tmp64, 255);
 
 	return 0;
 }

Re: amd iommu configuration

[Robin Murphy]

On 2022-09-05 15:13, Vasant Hegde wrote:

Steven,

[+Felix, amd-fgx list]


On 9/3/2022 4:29 AM, Steven J Abner wrote:

Hi
I was referred to you from linux-ker...@vger.kernel.org about the following 
issue.
Here is as was written:
On 9/1/22 11:36, Steven J Abner wrote:
Hi
Building a kernel tailored for AMD 2400g on ASRock B450 using 5.18.12 as base.
I stumbled across an odd situation and which lacked Kconfig info and lead to
oddity.
/drivers/iommu/amd/Kconfig states 'config AMD_IOMMU_V2' is 'tristate' but unlike
many
other tristate configures doesn't mention that module name is 'iommu_v2.ko' and
loading should be done by adding to modules-load.d.

The oddity is that by loading as module is as follows (differences):

builtin iommu_v2 version dmesg:
amdgpu: HMM registered 2048MB device memory
amdgpu: Topology: Add APU node [0x0:0x0]
amdgpu: Topology: Add APU node [0x15dd:0x1002]
AMD-Vi: AMD IOMMUv2 loaded and initialized
kfd kfd: amdgpu: added device 1002:15dd
kfd kfd: amdgpu: Allocated 3969056 bytes on gart
memmap_init_zone_device initialised 524288 pages in 0ms


IOMMU V2 modules provides IOMMU feature like attaching device to
process. I think amdgpu uses those features if available.
So in this case amdgpu is using those IOMMU features.



module not loaded due to missing iommu.conf dmesg:
amdgpu: CRAT table disabled by module option
amdgpu: Topology: Add CPU node
amdgpu: Virtual CRAT table created for CPU
kfd kfd: amdgpu: GC IP 090100 not supported in kfd

module load through iommu.conf dmesg:
amdgpu: CRAT table disabled by module option
amdgpu: Topology: Add CPU node
amdgpu: Virtual CRAT table created for CPU
AMD-Vi: AMD IOMMUv2 loaded and initialized
kfd kfd: amdgpu: GC IP 090100 not supported in kfd

Note, only difference on witk/without iommu.conf is:
AMD-Vi: AMD IOMMUv2 loaded and initialized


I think in this case iommu_v2.ko module got loaded after GPU
initialized. Hence amdgpu is not using iommu v2 features.




So does this mean missing features by not having builtin?
If not, should Kconfig have hint about module and loading?


@Felix,
   I see that drivers/gpu/drm/amd/amdkfd/Kconfig contains below line
 imply AMD_IOMMU_V2 if X86_64


   Should we change `s/imply/select` ?


"select" might help when KFD is built-in, but it probably still wants a 
MODULE_SOFTDEP() to enforce load order when they're both modular.


Robin.

Re: [PATCH] drm/amdgpu: prevent toc firmware memory leak

[Deucher, Alexander]

[Public]

Reviewed-by: Alex Deucher 

From: Chen, Guchun 
Sent: Friday, September 2, 2022 2:11 AM
To: amd-gfx@lists.freedesktop.org ; Deucher, 
Alexander ; Koenig, Christian 
; Pan, Xinhui 
Cc: Chen, Guchun 
Subject: [PATCH] drm/amdgpu: prevent toc firmware memory leak

It's missed in psp fini.

Signed-off-by: Guchun Chen 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
index 28ca0a94b8a5..cfcaf890a6a1 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
@@ -496,11 +496,14 @@ static int psp_sw_fini(void *handle)
 release_firmware(psp->ta_fw);
 psp->ta_fw = NULL;
 }
-   if (adev->psp.cap_fw) {
+   if (psp->cap_fw) {
 release_firmware(psp->cap_fw);
 psp->cap_fw = NULL;
 }
-
+   if (psp->toc_fw) {
+   release_firmware(psp->toc_fw);
+   psp->toc_fw = NULL;
+   }
 if (adev->ip_versions[MP0_HWIP][0] == IP_VERSION(11, 0, 0) ||
 adev->ip_versions[MP0_HWIP][0] == IP_VERSION(11, 0, 7))
 psp_sysfs_fini(adev);
--
2.25.1

Re: [PATCH] drm/amd/display: fix memory leak when using debugfs_lookup()

[Rodrigo Siqueira Jordao]

On 2022-09-02 09:10, Greg Kroah-Hartman wrote:

On Fri, Sep 02, 2022 at 03:01:05PM +0200, Greg Kroah-Hartman wrote:

When calling debugfs_lookup() the result must have dput() called on it,
otherwise the memory will leak over time.  Fix this up by properly
calling dput().

Cc: Harry Wentland 
Cc: Leo Li 
Cc: Rodrigo Siqueira 
Cc: Alex Deucher 
Cc: "Christian König" 
Cc: "Pan, Xinhui" 
Cc: David Airlie 
Cc: Daniel Vetter 
Cc: Wayne Lin 
Cc: hersen wu 
Cc: Wenjing Liu 
Cc: Patrik Jakobsson 
Cc: Thelford Williams 
Cc: Fangzhi Zuo 
Cc: Yongzhi Liu 
Cc: Mikita Lipski 
Cc: Jiapeng Chong 
Cc: Bhanuprakash Modem 
Cc: Sean Paul 
Cc: amd-gfx@lists.freedesktop.org
Cc: dri-de...@lists.freedesktop.org
Signed-off-by: Greg Kroah-Hartman 
---


Despite a zillion cc: items, I forgot to cc: stable on this.  Can the
maintainer add that here, or do you all want me to resend it with that
item added?

thanks,

greg k-h


Hi Greg,

Reviewed-by: Rodrigo Siqueira 

Is 'Cc: sta...@vger.kernel.org' enough here? I can make this change 
before I merge it into amd-staging-drm-next.


Thanks
Siqueira

Re: [PATCH] drm/amdgpu: cleanup coding style in amdgpu_amdkfd_gpuvm.c

[Felix Kuehling]

Am 2022-09-05 um 04:38 schrieb Jingyu Wang:

Fix everything checkpatch.pl complained about in amdgpu_amdkfd_gpuvm.c

Signed-off-by: Jingyu Wang 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 4 +++-
  1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
index cbd593f7d553..eff596c60c89 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
@@ -1,3 +1,4 @@
+// SPDX-License-Identifier: MIT


I'm not sure if this is correct. We've used "GPL-2.0 OR MIT" in KFD. In 
amdgpu there is currently a mix of licenses. Alex, do you want to make a 
call on a consistent one to use in amdgpu?


Other than that, this patch is

Reviewed-by: Felix Kuehling 



  /*
   * Copyright 2014-2018 Advanced Micro Devices, Inc.
   *
@@ -1612,6 +1613,7 @@ size_t amdgpu_amdkfd_get_available_memory(struct 
amdgpu_device *adev)
uint64_t reserved_for_pt =
ESTIMATE_PT_SIZE(amdgpu_amdkfd_total_mem_size);
size_t available;
+
spin_lock(_mem_limit.mem_limit_lock);
available = adev->gmc.real_vram_size
- adev->kfd.vram_used_aligned
@@ -2216,7 +2218,7 @@ int amdgpu_amdkfd_gpuvm_get_vm_fault_info(struct 
amdgpu_device *adev,
  {
if (atomic_read(>gmc.vm_fault_info_updated) == 1) {
*mem = *adev->gmc.vm_fault_info;
-   mb();
+   mb(); /* make sure read happened */
atomic_set(>gmc.vm_fault_info_updated, 0);
}
return 0;

base-commit: e47eb90a0a9ae20b82635b9b99a8d0979b757ad8

Re: [PATCH] drm/amdgpu: cleanup coding style in amdgpu_drv.c

[Robin Murphy]

On 2022-09-04 20:15, Jingyu Wang wrote:
[...]

@@ -565,8 +566,8 @@ module_param_named(timeout_period, 
amdgpu_watchdog_timer.period, uint, 0644);
   */
  #ifdef CONFIG_DRM_AMDGPU_SI
  
-#if defined(CONFIG_DRM_RADEON) || defined(CONFIG_DRM_RADEON_MODULE)

-int amdgpu_si_support = 0;
+#if IS_ENABLED(CONFIG_DRM_RADEON) || defined(CONFIG_DRM_RADEON_MODULE)


Hint: read the checkpatch warning again more closely, and consider what 
IS_ENABLED() does and therefore why this is still not quite right.


Robin.


+int amdgpu_si_support;
  MODULE_PARM_DESC(si_support, "SI support (1 = enabled, 0 = disabled 
(default))");
  #else
  int amdgpu_si_support = 1;

Re: [PATCH] drm/amdgpu: cleanup coding style in amdgpu_kms.c

[kernel test robot]

Hi Jingyu,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on e47eb90a0a9ae20b82635b9b99a8d0979b757ad8]

url:
https://github.com/intel-lab-lkp/linux/commits/Jingyu-Wang/drm-amdgpu-cleanup-coding-style-in-amdgpu_kms-c/20220906-104802
base:   e47eb90a0a9ae20b82635b9b99a8d0979b757ad8
config: mips-allyesconfig 
(https://download.01.org/0day-ci/archive/20220906/202209061955.bbpvthp7-...@intel.com/config)
compiler: mips-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
wget 
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O 
~/bin/make.cross
chmod +x ~/bin/make.cross
# 
https://github.com/intel-lab-lkp/linux/commit/778e4a57afd0db6a6a752487e1a1cda253cd9682
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review 
Jingyu-Wang/drm-amdgpu-cleanup-coding-style-in-amdgpu_kms-c/20220906-104802
git checkout 778e4a57afd0db6a6a752487e1a1cda253cd9682
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 
O=build_dir ARCH=mips SHELL=/bin/bash

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot 

All errors (new ones prefixed by >>):

   drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c: In function 'amdgpu_info_ioctl':
>> drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c:554:76: error: macro "min_t" 
>> requires 3 arguments, but only 2 given
 554 | ret = copy_to_user(out, , min_t((size_t)size, 
sizeof(ip)));
 |  
  ^
   In file included from include/linux/kernel.h:26,
from include/linux/cpumask.h:10,
from include/linux/smp.h:13,
from arch/mips/include/asm/cpu-type.h:12,
from arch/mips/include/asm/timex.h:19,
from include/linux/timex.h:67,
from include/linux/time32.h:13,
from include/linux/time.h:60,
from include/linux/ktime.h:24,
from drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h:26,
from drivers/gpu/drm/amd/amdgpu/amdgpu.h:43,
from drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c:29:
   include/linux/minmax.h:104: note: macro "min_t" defined here
 104 | #define min_t(type, x, y)   __careful_cmp((type)(x), (type)(y), 
<)
 | 
>> drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c:554:46: error: 'min_t' undeclared 
>> (first use in this function)
 554 | ret = copy_to_user(out, , min_t((size_t)size, 
sizeof(ip)));
 |  ^
   drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c:554:46: note: each undeclared 
identifier is reported only once for each function it appears in
   drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c: In function 
'amdgpu_debugfs_firmware_info_show':
>> drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c:1384:27: error: expected expression 
>> before '[' token
1384 | #define TA_FW_NAME(type) ([TA_FW_TYPE_PSP_##type] = #type)
 |   ^
   drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c:1385:17: note: in expansion of macro 
'TA_FW_NAME'
1385 | TA_FW_NAME(XGMI),
 | ^~


vim +/min_t +554 drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c

   494  
   495  /*
   496   * Userspace get information ioctl
   497   */
   498  /**
   499   * amdgpu_info_ioctl - answer a device specific request.
   500   *
   501   * @dev: drm device pointer
   502   * @data: request object
   503   * @filp: drm filp
   504   *
   505   * This function is used to pass device specific parameters to the 
userspace
   506   * drivers.  Examples include: pci device id, pipeline parms, tiling 
params,
   507   * etc. (all asics).
   508   * Returns 0 on success, -EINVAL on failure.
   509   */
   510  int amdgpu_info_ioctl(struct drm_device *dev, void *data, struct 
drm_file *filp)
   511  {
   512  struct amdgpu_device *adev = drm_to_adev(dev);
   513  struct drm_amdgpu_info *info = data;
   514  struct amdgpu_mode_info *minfo = >mode_info;
   515  void __user *out = (void __user 
*)(uintptr_t)info->return_pointer;
   516  uint32_t size = info->return_size;
   517  struct drm_crtc *crtc;
   518  uint32_t ui32 = 0;
   519  uint64_t ui64 = 0;
   520  int i, found;
   521  int ui32_size = sizeof(ui32);
   522  
   523  if (!info->return_size || !info->return_pointer)
   524  return -EINVAL;
   525  
   526  switch (info->query) {
   527  case AMDGPU_INFO_ACCEL_WORKING:
   528  ui32 = adev->accel_working;
   529  

Re: Gang submit

[Christian König]

Hi Monk,


If one gfx ib and one compute ib are in a gang, can they run literally  in 
parallel on GPU ?


Yes, that is essentially the functionality of gang submit.

The driver stack must guarantee that those IBs run at the same time 
because they use a ring buffer to communicate with each other.



if one gfx ib and one compute ib are belong to two gang, they will be put to 
the gfx and compute ring one by one (e.g:  gang1-gfx-ib scheduled and signaled, 
and then gang2-compute-ib scheduled )


Yes, gang submission should never overlap or otherwise you can run into 
lockups.


Regards,
Christian.

Am 06.09.22 um 03:43 schrieb Liu, Monk:

[AMD Official Use Only - General]

Hi Christian



A gang submission guarantees that multiple IBs can run on different engines at 
the same time.
The effect is that as long as members of a gang are waiting to be submitted no 
other gang can start pushing jobs to the hardware and so deadlocks are 
effectively prevented.

Could you please help to explain or confirm:

1) If one gfx ib and one compute ib are in a gang, can they run literally  in 
parallel on GPU ?
2) if one gfx ib and one compute ib are belong to two gang, they will be put to 
the gfx and compute ring one by one (e.g:  gang1-gfx-ib scheduled and signaled, 
and then gang2-compute-ib scheduled )

Thanks
---
Monk Liu | Cloud GPU & Virtualization Solution | AMD
---
we are hiring software manager for CVS core team
---

-Original Message-
From: amd-gfx  On Behalf Of Christian 
König
Sent: 2022年3月3日 16:23
To: amd-gfx@lists.freedesktop.org; Olsak, Marek 
Subject: Gang submit

Hi guys,

this patch set implements the the requirement for so called gang submissions in 
the CS interface.

A gang submission guarantees that multiple IBs can run on different engines at 
the same time.

This is implemented by keeping a global per-device gang around represented by a 
dma_fence which signals as soon as all jobs in a gang are pushed to the 
hardware.

The effect is that as long as members of a gang are waiting to be submitted no 
other gang can start pushing jobs to the hardware and so deadlocks are 
effectively prevented.

The whole set is based on top of my dma_resv_usage work and a few patches 
merged over from amd-staging-drm-next, so it won't easily apply anywhere.

Please review and comment,
Christian.

Re: [PATCH v2] drm/ttm: update bulk move object of ghost BO

[Christian König]

Am 06.09.22 um 10:46 schrieb ZhenGuo Yin:

[Why]
Ghost BO is released with non-empty bulk move object. There is a
warning trace:
WARNING: CPU: 19 PID: 1582 at ttm/ttm_bo.c:366 ttm_bo_release+0x2e1/0x2f0 
[amdttm]
Call Trace:
   amddma_resv_reserve_fences+0x10d/0x1f0 [amdkcl]
   amdttm_bo_put+0x28/0x30 [amdttm]
   amdttm_bo_move_accel_cleanup+0x126/0x200 [amdttm]
   amdgpu_bo_move+0x1a8/0x770 [amdgpu]
   ttm_bo_handle_move_mem+0xb0/0x140 [amdttm]
   amdttm_bo_validate+0xbf/0x100 [amdttm]

[How]
The resource of ghost BO should be moved to LRU directly, instead of
using bulk move. The bulk move object of ghost BO should set to NULL
before function ttm_bo_move_to_lru_tail_unlocked.

v2: set bulk move to NULL manually if no resource associated with ghost BO

Fixed: 5b951e487fd6bf5f ("drm/ttm: fix bulk move handling v2")
Signed-off-by: ZhenGuo Yin 


Reviewed-by: Christian König 

Going to push that to drm-misc-fixes in a minute.

Thanks,
Christian.


---
  drivers/gpu/drm/ttm/ttm_bo_util.c | 3 +++
  1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c 
b/drivers/gpu/drm/ttm/ttm_bo_util.c
index 1cbfb00c1d65..57a27847206f 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_util.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
@@ -239,6 +239,9 @@ static int ttm_buffer_object_transfer(struct 
ttm_buffer_object *bo,
if (fbo->base.resource) {
ttm_resource_set_bo(fbo->base.resource, >base);
bo->resource = NULL;
+   ttm_bo_set_bulk_move(>base, NULL);
+   } else {
+   fbo->base.bulk_move = NULL;
}
  
  	dma_resv_init(>base.base._resv);

[PATCH v2] drm/ttm: update bulk move object of ghost BO

[ZhenGuo Yin]

[Why]
Ghost BO is released with non-empty bulk move object. There is a
warning trace:
WARNING: CPU: 19 PID: 1582 at ttm/ttm_bo.c:366 ttm_bo_release+0x2e1/0x2f0 
[amdttm]
Call Trace:
  amddma_resv_reserve_fences+0x10d/0x1f0 [amdkcl]
  amdttm_bo_put+0x28/0x30 [amdttm]
  amdttm_bo_move_accel_cleanup+0x126/0x200 [amdttm]
  amdgpu_bo_move+0x1a8/0x770 [amdgpu]
  ttm_bo_handle_move_mem+0xb0/0x140 [amdttm]
  amdttm_bo_validate+0xbf/0x100 [amdttm]

[How]
The resource of ghost BO should be moved to LRU directly, instead of
using bulk move. The bulk move object of ghost BO should set to NULL
before function ttm_bo_move_to_lru_tail_unlocked.

v2: set bulk move to NULL manually if no resource associated with ghost BO

Fixed: 5b951e487fd6bf5f ("drm/ttm: fix bulk move handling v2")
Signed-off-by: ZhenGuo Yin 
---
 drivers/gpu/drm/ttm/ttm_bo_util.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c 
b/drivers/gpu/drm/ttm/ttm_bo_util.c
index 1cbfb00c1d65..57a27847206f 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_util.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
@@ -239,6 +239,9 @@ static int ttm_buffer_object_transfer(struct 
ttm_buffer_object *bo,
if (fbo->base.resource) {
ttm_resource_set_bo(fbo->base.resource, >base);
bo->resource = NULL;
+   ttm_bo_set_bulk_move(>base, NULL);
+   } else {
+   fbo->base.bulk_move = NULL;
}
 
dma_resv_init(>base.base._resv);
-- 
2.35.1

[PATCH V2] drm/amdgpu: Adjust removal control flow for smu v13_0_2

[YiPeng Chai]

Adjust removal control flow for smu v13_0_2:
   During amdgpu uninstallation, when removing the first
device, the kernel needs to first send a mode1reset message
to all gpu devices. Otherwise, smu initialization will fail
the next time amdgpu is installed.

V2:
1. Update commit comments.
2. Remove the global variable amdgpu_device_remove_cnt
   and add a variable to the structure amdgpu_hive_info.
3. Use hive to detect the first removed device instead of
   a global variable.

Signed-off-by: YiPeng Chai 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h|  3 ++
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 40 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c| 35 +++
 drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c| 16 -
 drivers/gpu/drm/amd/amdgpu/amdgpu_reset.h  |  1 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.h   |  1 +
 drivers/gpu/drm/amd/pm/amdgpu_pm.c |  6 +++-
 7 files changed, 99 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 79bb6fd83094..465295318830 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -997,6 +997,9 @@ struct amdgpu_device {
boolin_s4;
boolin_s0ix;
 
+   /* uninstall */
+   boolin_remove;
+
enum pp_mp1_state   mp1_state;
struct amdgpu_doorbell_index doorbell_index;
 
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 62b26f0e37b0..1402717673f7 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -2999,6 +2999,13 @@ static int amdgpu_device_ip_suspend_phase2(struct 
amdgpu_device *adev)
DRM_ERROR("suspend of IP block <%s> failed %d\n",
  adev->ip_blocks[i].version->funcs->name, r);
}
+
+   /* If in_remove is true, psp_hw_fini should be executed after
+*  psp_suspend to free psp shared buffers.
+*/
+   if (adev->in_remove && (adev->ip_blocks[i].version->type == 
AMD_IP_BLOCK_TYPE_PSP))
+   continue;
+
adev->ip_blocks[i].status.hw = false;
/* handle putting the SMC in the appropriate state */
if(!amdgpu_sriov_vf(adev)){
@@ -4739,6 +4746,7 @@ int amdgpu_do_asic_reset(struct list_head 
*device_list_handle,
struct amdgpu_device *tmp_adev = NULL;
bool need_full_reset, skip_hw_reset, vram_lost = false;
int r = 0;
+   bool gpu_reset_for_dev_remove = 0;
 
/* Try reset handler method first */
tmp_adev = list_first_entry(device_list_handle, struct amdgpu_device,
@@ -4758,6 +4766,10 @@ int amdgpu_do_asic_reset(struct list_head 
*device_list_handle,
test_bit(AMDGPU_NEED_FULL_RESET, _context->flags);
skip_hw_reset = test_bit(AMDGPU_SKIP_HW_RESET, _context->flags);
 
+   gpu_reset_for_dev_remove =
+   test_bit(AMDGPU_RESET_FOR_DEVICE_REMOVE, _context->flags) 
&&
+   test_bit(AMDGPU_NEED_FULL_RESET, _context->flags);
+
/*
 * ASIC reset has to be done on all XGMI hive nodes ASAP
 * to allow proper links negotiation in FW (within 1 sec)
@@ -4802,6 +4814,16 @@ int amdgpu_do_asic_reset(struct list_head 
*device_list_handle,
amdgpu_ras_intr_cleared();
}
 
+   /* Fixed the problem that BIOS signature errors and psp bootloader
+* failure to load kdb on next amdgpu install.
+*/
+   if (gpu_reset_for_dev_remove) {
+   list_for_each_entry(tmp_adev, device_list_handle, reset_list)
+   amdgpu_device_ip_resume_phase1(tmp_adev);
+
+   goto end;
+   }
+
list_for_each_entry(tmp_adev, device_list_handle, reset_list) {
if (need_full_reset) {
/* post card */
@@ -5124,6 +5146,11 @@ int amdgpu_device_gpu_recover(struct amdgpu_device *adev,
bool need_emergency_restart = false;
bool audio_suspended = false;
int tmp_vram_lost_counter;
+   bool gpu_reset_for_dev_remove = false;
+
+   gpu_reset_for_dev_remove =
+   test_bit(AMDGPU_RESET_FOR_DEVICE_REMOVE, 
_context->flags) &&
+   test_bit(AMDGPU_NEED_FULL_RESET, 
_context->flags);
 
/*
 * Special case: RAS triggered and full reset isn't supported
@@ -5159,8 +5186,11 @@ int amdgpu_device_gpu_recover(struct amdgpu_device *adev,
 */
INIT_LIST_HEAD(_list);
if (!amdgpu_sriov_vf(adev) && (adev->gmc.xgmi.num_physical_nodes > 1)) {
-   list_for_each_entry(tmp_adev, >device_list, gmc.xgmi.head)
+   list_for_each_entry(tmp_adev, >device_list, 
gmc.xgmi.head) {

[PATCH] drm/amdgpu: recleanup coding style in amdgpu_fence.c

[Jingyu Wang]

Fix everything checkpatch.pl complained about in amdgpu_fence.c,
modified use "} else {", sent it again, thx.

Signed-off-by: Jingyu Wang 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c | 15 +--
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
index 8adeb7469f1e..0759d86d92da 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
@@ -1,3 +1,4 @@
+// SPDX-License-Identifier: MIT
 /*
  * Copyright 2009 Jerome Glisse.
  * All Rights Reserved.
@@ -42,7 +43,6 @@
 #include "amdgpu_reset.h"
 
 /*
- * Fences
  * Fences mark an event in the GPUs pipeline and are used
  * for GPU/CPU synchronization.  When the fence is written,
  * it is expected that all buffers associated with that fence
@@ -139,7 +139,7 @@ static u32 amdgpu_fence_read(struct amdgpu_ring *ring)
  * Returns 0 on success, -ENOMEM on failure.
  */
 int amdgpu_fence_emit(struct amdgpu_ring *ring, struct dma_fence **f, struct 
amdgpu_job *job,
- unsigned flags)
+ unsigned int flags)
 {
struct amdgpu_device *adev = ring->adev;
struct dma_fence *fence;
@@ -173,11 +173,11 @@ int amdgpu_fence_emit(struct amdgpu_ring *ring, struct 
dma_fence **f, struct amd
   adev->fence_context + ring->idx, seq);
/* Against remove in amdgpu_job_{free, free_cb} */
dma_fence_get(fence);
-   }
-   else
+   } else {
dma_fence_init(fence, _fence_ops,
   >fence_drv.lock,
   adev->fence_context + ring->idx, seq);
+   }
}
 
amdgpu_ring_emit_fence(ring, ring->fence_drv.gpu_addr,
@@ -393,7 +393,7 @@ signed long amdgpu_fence_wait_polling(struct amdgpu_ring 
*ring,
  * Returns the number of emitted fences on the ring.  Used by the
  * dynpm code to ring track activity.
  */
-unsigned amdgpu_fence_count_emitted(struct amdgpu_ring *ring)
+unsigned int amdgpu_fence_count_emitted(struct amdgpu_ring *ring)
 {
uint64_t emitted;
 
@@ -422,7 +422,7 @@ unsigned amdgpu_fence_count_emitted(struct amdgpu_ring 
*ring)
  */
 int amdgpu_fence_driver_start_ring(struct amdgpu_ring *ring,
   struct amdgpu_irq_src *irq_src,
-  unsigned irq_type)
+  unsigned int irq_type)
 {
struct amdgpu_device *adev = ring->adev;
uint64_t index;
@@ -594,6 +594,7 @@ void amdgpu_fence_driver_hw_init(struct amdgpu_device *adev)
 
for (i = 0; i < AMDGPU_MAX_RINGS; i++) {
struct amdgpu_ring *ring = adev->rings[i];
+
if (!ring || !ring->fence_drv.initialized)
continue;
 
@@ -772,6 +773,7 @@ static int amdgpu_debugfs_fence_info_show(struct seq_file 
*m, void *unused)
 
for (i = 0; i < AMDGPU_MAX_RINGS; ++i) {
struct amdgpu_ring *ring = adev->rings[i];
+
if (!ring || !ring->fence_drv.initialized)
continue;
 
@@ -845,6 +847,7 @@ static void amdgpu_debugfs_reset_work(struct work_struct 
*work)
  reset_work);
 
struct amdgpu_reset_context reset_context;
+
memset(_context, 0, sizeof(reset_context));
 
reset_context.method = AMD_RESET_METHOD_NONE;

base-commit: e47eb90a0a9ae20b82635b9b99a8d0979b757ad8
-- 
2.34.1

Re: Build regressions/improvements in v6.0-rc4

[Geert Uytterhoeven]

On Mon, 5 Sep 2022, Geert Uytterhoeven wrote:

JFYI, when comparing v6.0-rc4[1] to v6.0-rc3[3], the summaries are:
 - build errors: +3/-16


  + /kisskb/src/arch/sh/kernel/machvec.c: error: array subscript 'struct 
sh_machine_vector[0]' is partly outside array bounds of 'long int[1]' 
[-Werror=array-bounds]:  => 105:33

sh4-gcc11/sh-allyesconfig (-Werror)

  + 
/kisskb/src/drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn32/display_mode_vba_32.c:
 error: the frame size of 2144 bytes is larger than 2048 bytes 
[-Werror=frame-larger-than=]:  => 3768:1

x86_64-gcc8/x86-allmodconfig (in function 
dml32_ModeSupportAndSystemConfigurationFull())

  + /kisskb/src/include/linux/fortify-string.h: error: call to 
'__write_overflow_field' declared with attribute warning: detected write beyond 
size of field (1st parameter); maybe use struct_group()? 
[-Werror=attribute-warning]:  => 258:25

s390x-gcc11/s390-allyesconfig (inlined from 'copy_process' at 
/kisskb/src/kernel/fork.c:2200:2)


[1] 
http://kisskb.ellerman.id.au/kisskb/branch/linus/head/7e18e42e4b280c85b76967a9106a13ca61c16179/
 (all 135 configs)
[3] 
http://kisskb.ellerman.id.au/kisskb/branch/linus/head/b90cb1053190353cc30f0fef0ef1f378ccc063c5/
 (all 135 configs)


Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

[PATCH] drm/amdgpu: cleanup coding style in amdgpu_amdkfd_gpuvm.c

[Jingyu Wang]

Fix everything checkpatch.pl complained about in amdgpu_amdkfd_gpuvm.c

Signed-off-by: Jingyu Wang 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
index cbd593f7d553..eff596c60c89 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
@@ -1,3 +1,4 @@
+// SPDX-License-Identifier: MIT
 /*
  * Copyright 2014-2018 Advanced Micro Devices, Inc.
  *
@@ -1612,6 +1613,7 @@ size_t amdgpu_amdkfd_get_available_memory(struct 
amdgpu_device *adev)
uint64_t reserved_for_pt =
ESTIMATE_PT_SIZE(amdgpu_amdkfd_total_mem_size);
size_t available;
+
spin_lock(_mem_limit.mem_limit_lock);
available = adev->gmc.real_vram_size
- adev->kfd.vram_used_aligned
@@ -2216,7 +2218,7 @@ int amdgpu_amdkfd_gpuvm_get_vm_fault_info(struct 
amdgpu_device *adev,
 {
if (atomic_read(>gmc.vm_fault_info_updated) == 1) {
*mem = *adev->gmc.vm_fault_info;
-   mb();
+   mb(); /* make sure read happened */
atomic_set(>gmc.vm_fault_info_updated, 0);
}
return 0;

base-commit: e47eb90a0a9ae20b82635b9b99a8d0979b757ad8
-- 
2.34.1

[PATCH] drm/amdgpu: cleanup coding style in amdgpu_device.c

[Jingyu Wang]

Fix some checkpatch.pl complained about in amdgpu_device.c

Signed-off-by: Jingyu Wang 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 136 +++--
 1 file changed, 69 insertions(+), 67 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index afaa1056e039..05d9aa3b5131 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -149,7 +149,7 @@ static ssize_t amdgpu_device_get_pcie_replay_count(struct 
device *dev,
return sysfs_emit(buf, "%llu\n", cnt);
 }
 
-static DEVICE_ATTR(pcie_replay_count, S_IRUGO,
+static DEVICE_ATTR(pcie_replay_count, 0444,
amdgpu_device_get_pcie_replay_count, NULL);
 
 static void amdgpu_device_get_pcie_info(struct amdgpu_device *adev);
@@ -173,7 +173,7 @@ static ssize_t amdgpu_device_get_product_name(struct device 
*dev,
return sysfs_emit(buf, "%s\n", adev->product_name);
 }
 
-static DEVICE_ATTR(product_name, S_IRUGO,
+static DEVICE_ATTR(product_name, 0444,
amdgpu_device_get_product_name, NULL);
 
 /**
@@ -195,7 +195,7 @@ static ssize_t amdgpu_device_get_product_number(struct 
device *dev,
return sysfs_emit(buf, "%s\n", adev->product_number);
 }
 
-static DEVICE_ATTR(product_number, S_IRUGO,
+static DEVICE_ATTR(product_number, 0444,
amdgpu_device_get_product_number, NULL);
 
 /**
@@ -217,7 +217,7 @@ static ssize_t amdgpu_device_get_serial_number(struct 
device *dev,
return sysfs_emit(buf, "%s\n", adev->serial);
 }
 
-static DEVICE_ATTR(serial_number, S_IRUGO,
+static DEVICE_ATTR(serial_number, 0444,
amdgpu_device_get_serial_number, NULL);
 
 /**
@@ -360,11 +360,11 @@ size_t amdgpu_device_aper_access(struct amdgpu_device 
*adev, loff_t pos,
 
if (write) {
memcpy_toio(addr, buf, count);
-   mb();
+   mb(); /* make sure io happens */
amdgpu_device_flush_hdp(adev, NULL);
} else {
amdgpu_device_invalidate_hdp(adev, NULL);
-   mb();
+   mb(); /* make sure io happens */
memcpy_fromio(buf, addr, count);
}
 
@@ -472,7 +472,7 @@ uint32_t amdgpu_device_rreg(struct amdgpu_device *adev,
  * MMIO register read with bytes helper functions
  * @offset:bytes offset from MMIO start
  *
-*/
+ */
 
 /**
  * amdgpu_mm_rreg8 - read a memory mapped IO register
@@ -497,7 +497,7 @@ uint8_t amdgpu_mm_rreg8(struct amdgpu_device *adev, 
uint32_t offset)
  * @offset:bytes offset from MMIO start
  * @value: the value want to be written to the register
  *
-*/
+ */
 /**
  * amdgpu_mm_wreg8 - read a memory mapped IO register
  *
@@ -615,11 +615,10 @@ 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) {
+   if (index < adev->doorbell.num_doorbells)
writel(v, adev->doorbell.ptr + index);
-   } else {
+   else
DRM_ERROR("writing beyond doorbell aperture: 0x%08x!\n", index);
-   }
 }
 
 /**
@@ -659,11 +658,10 @@ 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) {
+   if (index < adev->doorbell.num_doorbells)
atomic64_set((atomic64_t *)(adev->doorbell.ptr + index), v);
-   } else {
+   else
DRM_ERROR("writing beyond doorbell aperture: 0x%08x!\n", index);
-   }
 }
 
 /**
@@ -958,7 +956,7 @@ static void amdgpu_device_vram_scratch_fini(struct 
amdgpu_device *adev)
  * @registers: pointer to the register array
  * @array_size: size of the register array
  *
- * Programs an array or registers with and and or masks.
+ * Programs an array or registers with and or masks.
  * This is a helper for setting golden registers.
  */
 void amdgpu_device_program_register_sequence(struct amdgpu_device *adev,
@@ -971,7 +969,7 @@ void amdgpu_device_program_register_sequence(struct 
amdgpu_device *adev,
if (array_size % 3)
return;
 
-   for (i = 0; i < array_size; i +=3) {
+   for (i = 0; i < array_size; i += 3) {
reg = registers[i + 0];
and_mask = registers[i + 1];
or_mask = registers[i + 2];
@@ -1200,7 +1198,7 @@ int amdgpu_device_resize_fb_bar(struct amdgpu_device 
*adev)
int rbar_size = pci_rebar_bytes_to_size(adev->gmc.real_vram_size);
struct pci_bus *root;
struct resource *res;
-   unsigned i;
+   unsigned int i;
u16 cmd;
int r;
 
@@ -1292,6 +1290,7 @@ bool amdgpu_device_need_post(struct amdgpu_device *adev)
if (adev->asic_type == CHIP_FIJI) {
  

[PATCH] drm/amdgpu: cleanup coding style in amdgpu_kms.c

[Jingyu Wang]

Fix some checkpatch.pl complained about in amdgpu_kms.c

Signed-off-by: Jingyu Wang 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 16 +---
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
index 77668c3dae5b..1f90a096232d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
@@ -532,6 +532,7 @@ int amdgpu_info_ioctl(struct drm_device *dev, void *data, 
struct drm_file *filp)
crtc = (struct drm_crtc *)minfo->crtcs[i];
if (crtc && crtc->base.id == info->mode_crtc.id) {
struct amdgpu_crtc *amdgpu_crtc = 
to_amdgpu_crtc(crtc);
+
ui32 = amdgpu_crtc->crtc_id;
found = 1;
break;
@@ -550,7 +551,7 @@ int amdgpu_info_ioctl(struct drm_device *dev, void *data, 
struct drm_file *filp)
if (ret)
return ret;
 
-   ret = copy_to_user(out, , min((size_t)size, sizeof(ip)));
+   ret = copy_to_user(out, , min_t((size_t)size, sizeof(ip)));
return ret ? -EFAULT : 0;
}
case AMDGPU_INFO_HW_IP_COUNT: {
@@ -696,17 +697,18 @@ int amdgpu_info_ioctl(struct drm_device *dev, void *data, 
struct drm_file *filp)
? -EFAULT : 0;
}
case AMDGPU_INFO_READ_MMR_REG: {
-   unsigned n, alloc_size;
+   unsigned int n, alloc_size;
uint32_t *regs;
-   unsigned se_num = (info->read_mmr_reg.instance >>
+   unsigned int se_num = (info->read_mmr_reg.instance >>
   AMDGPU_INFO_MMR_SE_INDEX_SHIFT) &
  AMDGPU_INFO_MMR_SE_INDEX_MASK;
-   unsigned sh_num = (info->read_mmr_reg.instance >>
+   unsigned int sh_num = (info->read_mmr_reg.instance >>
   AMDGPU_INFO_MMR_SH_INDEX_SHIFT) &
  AMDGPU_INFO_MMR_SH_INDEX_MASK;
 
/* set full masks if the userspace set all bits
-* in the bitfields */
+* in the bitfields
+*/
if (se_num == AMDGPU_INFO_MMR_SE_INDEX_MASK)
se_num = 0x;
else if (se_num >= AMDGPU_GFX_MAX_SE)
@@ -830,7 +832,7 @@ int amdgpu_info_ioctl(struct drm_device *dev, void *data, 
struct drm_file *filp)
return ret;
}
case AMDGPU_INFO_VCE_CLOCK_TABLE: {
-   unsigned i;
+   unsigned int i;
struct drm_amdgpu_info_vce_clock_table vce_clk_table = {};
struct amd_vce_state *vce_state;
 
@@ -1379,7 +1381,7 @@ static int amdgpu_debugfs_firmware_info_show(struct 
seq_file *m, void *unused)
int ret, i;
 
static const char *ta_fw_name[TA_FW_TYPE_MAX_INDEX] = {
-#define TA_FW_NAME(type) [TA_FW_TYPE_PSP_##type] = #type
+#define TA_FW_NAME(type) ([TA_FW_TYPE_PSP_##type] = #type)
TA_FW_NAME(XGMI),
TA_FW_NAME(RAS),
TA_FW_NAME(HDCP),

base-commit: e47eb90a0a9ae20b82635b9b99a8d0979b757ad8
-- 
2.34.1

[PATCH] drm/amdgpu: cleanup coding style in amdgpu_amdkfd.c

[Jingyu Wang]

Fix everything checkpatch.pl complained about in amdgpu_amdkfd.c

Signed-off-by: Jingyu Wang 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
index 091415a4abf0..4f5bd96000ec 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
@@ -1,3 +1,4 @@
+// SPDX-License-Identifier: MIT
 /*
  * Copyright 2014 Advanced Micro Devices, Inc.
  *
@@ -130,6 +131,7 @@ static void amdgpu_amdkfd_reset_work(struct work_struct 
*work)
  kfd.reset_work);
 
struct amdgpu_reset_context reset_context;
+
memset(_context, 0, sizeof(reset_context));
 
reset_context.method = AMD_RESET_METHOD_NONE;

base-commit: e47eb90a0a9ae20b82635b9b99a8d0979b757ad8
-- 
2.34.1

[GIT PULL] Immutable backlight-detect-refactor branch between acpi, drm-* and pdx86

[Hans de Goede]

Hi All,

Now that all patches have been reviewed/acked here is an immutable 
backlight-detect-refactor
branch with 6.0-rc1 + the v5 patch-set, for merging into the relevant (acpi, 
drm-* and pdx86)
subsystems.

Please pull this branch into the relevant subsystems.

I will merge this into the review-hans branch of the pdx86 git tree today and
from there it will move to for-next once the builders have successfully 
build-tested
the merge.

Regards,

Hans


The following changes since commit 568035b01cfb107af8d2e4bd2fb9aea22cf5b868:

  Linux 6.0-rc1 (2022-08-14 15:50:18 -0700)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git 
tags/backlight-detect-refactor-1

for you to fetch changes up to 4f96b1bc156e7076f6efedc2a76a8c7e897c7977:

  drm/todo: Add entry about dealing with brightness control on devices with > 1 
panel (2022-09-03 12:17:27 +0200)


Immutable backlight-detect-refactor branch between acpi, drm-* and pdx86

Tag (immutable branch) with v6.0-rc1 + the (acpi/x86) backlight
detect refactor work. For merging into the acpi, drm-* and pdx86
subsystems.


Hans de Goede (31):
  ACPI: video: Add acpi_video_backlight_use_native() helper
  drm/i915: Don't register backlight when another backlight should be used 
(v2)
  drm/amdgpu: Don't register backlight when another backlight should be 
used (v3)
  drm/radeon: Don't register backlight when another backlight should be 
used (v3)
  drm/nouveau: Don't register backlight when another backlight should be 
used (v2)
  ACPI: video: Drop backlight_device_get_by_type() call from 
acpi_video_get_backlight_type()
  ACPI: video: Remove acpi_video_bus from list before tearing it down
  ACPI: video: Simplify acpi_video_unregister_backlight()
  ACPI: video: Make backlight class device registration a separate step (v2)
  ACPI: video: Remove code to unregister acpi_video backlight when a native 
backlight registers
  drm/i915: Call acpi_video_register_backlight() (v3)
  drm/nouveau: Register ACPI video backlight when nv_backlight registration 
fails (v2)
  drm/amdgpu: Register ACPI video backlight when skipping amdgpu backlight 
registration
  drm/radeon: Register ACPI video backlight when skipping radeon backlight 
registration
  platform/x86: nvidia-wmi-ec-backlight: Move fw interface definitions to a 
header (v2)
  ACPI: video: Refactor acpi_video_get_backlight_type() a bit
  ACPI: video: Add Nvidia WMI EC brightness control detection (v3)
  ACPI: video: Add Apple GMUX brightness control detection
  platform/x86: nvidia-wmi-ec-backlight: Use acpi_video_get_backlight_type()
  platform/x86: apple-gmux: Stop calling acpi/video.h functions
  platform/x86: toshiba_acpi: Stop using acpi_video_set_dmi_backlight_type()
  platform/x86: acer-wmi: Move backlight DMI quirks to acpi/video_detect.c
  platform/x86: asus-wmi: Drop DMI chassis-type check from backlight 
handling
  platform/x86: asus-wmi: Move acpi_backlight=vendor quirks to ACPI 
video_detect.c
  platform/x86: asus-wmi: Move acpi_backlight=native quirks to ACPI 
video_detect.c
  platform/x86: samsung-laptop: Move acpi_backlight=[vendor|native] quirks 
to ACPI video_detect.c
  ACPI: video: Remove acpi_video_set_dmi_backlight_type()
  ACPI: video: Drop "Samsung X360" acpi_backlight=native quirk
  ACPI: video: Drop NL5x?U, PF4NU1F and PF5?U?? acpi_backlight=native quirks
  ACPI: video: Fix indentation of video_detect_dmi_table[] entries
  drm/todo: Add entry about dealing with brightness control on devices with 
> 1 panel

 Documentation/gpu/todo.rst |  68 
 MAINTAINERS|   1 +
 drivers/acpi/Kconfig   |   1 +
 drivers/acpi/acpi_video.c  |  64 ++-
 drivers/acpi/video_detect.c| 428 -
 drivers/gpu/drm/Kconfig|  14 +
 drivers/gpu/drm/amd/amdgpu/atombios_encoders.c |  14 +-
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c  |   9 +
 drivers/gpu/drm/gma500/Kconfig |   2 +
 drivers/gpu/drm/i915/Kconfig   |   2 +
 drivers/gpu/drm/i915/display/intel_acpi.c  |  27 ++
 drivers/gpu/drm/i915/display/intel_acpi.h  |   3 +
 drivers/gpu/drm/i915/display/intel_backlight.c |   7 +
 drivers/gpu/drm/i915/display/intel_display.c   |   2 +-
 drivers/gpu/drm/nouveau/nouveau_acpi.c |  10 +
 drivers/gpu/drm/nouveau/nouveau_acpi.h |   4 +
 drivers/gpu/drm/nouveau/nouveau_backlight.c|  13 +
 drivers/gpu/drm/radeon/atombios_encoders.c |   7 +
 drivers/gpu/drm/radeon/radeon_encoders.c   |  11 +-
 

回复: Re: [PATCH] drm:Fix the blank screen problem of some 1920x1080 75Hz monitors using R520 graphics card

[钟沛]

Thanks for your reply!We found that in the amdgpu_pll_compute function, when the target_clock is the value contained in the drm_dmt_modes defined in drm_edid.c, the diff is 0. When target_clock is some special value, we cannot find a diff value of 0, so we need to find the smallest diff value to fit the current target_clock. For the monitor that has the blank screen problem here, we found that when the ref_div_max is 128, the diff value is smaller and the blank screen problem can be solved. We tested some other monitors and added log printing to the code. We found that this change did not affect those monitors, and in the analysis of the logs, we found that the solution with a smaller diff value always displayed normally.Changing the value of ref_div_max from 128 to 100 can solve the blank screen problem of some monitors, but it will also cause some normal monitors to go black, so is it a more reasonable solution to determine the value of ref_div_max according to the value of diff?Thank you for taking the time to read my email.Best Regards.
        主　题：Re: [PATCH] drm:Fix the blank screen problem of some 1920x1080 75Hz monitors using R520 graphics card
            日　期：2022-09-05 14:05
            发件人：Christian König
            收件人：钟沛alexander.deuc...@amd.comxinhui.pan@amd.comairlied@linux.iedaniel@ffwll.chisabba...@riseup.net
            
        
        Am 05.09.22 um 05:23 schrieb zhongpei:> We found that in the scenario of AMD R520 graphics card> and some 1920x1080 monitors,when we switch the refresh rate> of the monitor to 75Hz,the monitor will have a blank screen problem,> and the restart cannot be restored.After testing, it is found that> when we limit the maximum value of ref_div_max to 128,> the problem can be solved.In order to keep the previous modification> to be compatible with other monitors,we added a judgment> when finding the minimum diff value in the loop of the> amdgpu_pll_compute/radeon_compute_pll_avivo function.> If no diff value of 0 is found when the maximum value of ref_div_max> is limited to 100,continue to search when it is 128,> and take the parameter with the smallest diff value.Well that's at least better than what I've seen in previous tries to fix this.But as far as I can see this will certainly break some other monitors, so that is pretty much a NAK.Regards,Christian.>> Signed-off-by: zhongpei > --->   drivers/gpu/drm/amd/amdgpu/amdgpu_pll.c | 17 +>   drivers/gpu/drm/radeon/radeon_display.c | 15 +++>   2 files changed, 24 insertions(+), 8 deletions(-)>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pll.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_pll.c> index 0bb2466d539a..0c298faa0f94 100644> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pll.c> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pll.c> @@ -84,12 +84,13 @@ static void amdgpu_pll_reduce_ratio(unsigned *nom, unsigned *den,>   static void amdgpu_pll_get_fb_ref_div(struct amdgpu_device *adev, unsigned int nom,>         unsigned int den, unsigned int post_div,>         unsigned int fb_div_max, unsigned int ref_div_max,> -      unsigned int *fb_div, unsigned int *ref_div)> +      unsigned int ref_div_limit, unsigned int *fb_div,> +      unsigned int *ref_div)>   {>   >   	/* limit reference * post divider to a maximum */>   	if (adev->family == AMDGPU_FAMILY_SI)> -		ref_div_max = min(100 / post_div, ref_div_max);> +		ref_div_max = min(ref_div_limit / post_div, ref_div_max);>   	else>   		ref_div_max = min(128 / post_div, ref_div_max);>   > @@ -136,6 +137,7 @@ void amdgpu_pll_compute(struct amdgpu_device *adev,>   	unsigned ref_div_min, ref_div_max, ref_div;>   	unsigned post_div_best, diff_best;>   	unsigned nom, den;> +	unsigned ref_div_limit, ref_limit_best;>   >   	/* determine allowed feedback divider range */>   	fb_div_min = pll->min_feedback_div;> @@ -204,11 +206,12 @@ void amdgpu_pll_compute(struct amdgpu_device *adev,>   	else>   		post_div_best = post_div_max;>   	diff_best = ~0;> +	ref_div_limit = ref_limit_best = 100;>   >   	for (post_div = post_div_min; post_div <= post_div_max; ++post_div) {>   		unsigned diff;>   		amdgpu_pll_get_fb_ref_div(adev, nom, den, post_div, fb_div_max,> -	  ref_div_max, _div, _div);> +	  ref_div_max, ref_div_limit, _div, _div);>   		diff = abs(target_clock - (pll->reference_freq * fb_div) />   			(ref_div * post_div));>   > @@ -217,13 +220,19 @@ void amdgpu_pll_compute(struct amdgpu_device *adev,>   >   			post_div_best = post_div;>   			diff_best = diff;> +			ref_limit_best = ref_div_limit;>   		}> +		if (post_div >= post_div_max && diff_best != 0 && ref_div_limit != 128) {> +			ref_div_limit = 128;> +			post_div = post_div_min - 1;> +		}> +>   	}>   	post_div = post_div_best;>   >   	/* get the feedback and reference divider for the optimal value */>   	amdgpu_pll_get_fb_ref_div(adev, nom, den, post_div, fb_div_max, ref_div_max,> -  _div, _div);> +  ref_limit_best, _div, _div);>   >   	/* reduce the numbers to a simpler