RE: [PATCH] drm/amdgpu: fetch default VDDC curve voltages (v2)

2020-02-03 Thread Quan, Evan 
Just found the boot_overdrive_table curve voltags need also be updated.
Could you help to update them also?
I think the code pieces like below should work.

static int navi10_set_default_od_settings(struct smu_context *smu, bool 
initialize) {
-   OverDriveTable_t *od_table;
+   OverDriveTable_t *od_table, *boot_od_table;
int ret = 0;

ret = smu_v11_0_set_default_od_settings(smu, initialize, 
sizeof(OverDriveTable_t));
@@ -1985,6 +1985,7 @@ static int navi10_set_default_od_settings(struct 
smu_context *smu, bool initiali
return ret;

od_table = (OverDriveTable_t *)smu->smu_table.overdrive_table;
+   boot_od_table = (OverDriveTable_t *)smu->smu_table.boot_overdrive_table;
if (initialize) {
ret = navi10_setup_od_limits(smu);
if (ret) {
@@ -1998,6 +1999,7 @@ static int navi10_set_default_od_settings(struct 
smu_context *smu, bool initiali

od_table->GfxclkFreq1);
if (ret)
od_table->GfxclkVolt1 = 0;
+   boot_od_table->GfxclkVolt1 = 
od_table->GfxclkVolt1;
}

if (!od_table->GfxclkVolt2) {
@@ -2006,6 +2008,7 @@ static int navi10_set_default_od_settings(struct 
smu_context *smu, bool initiali

od_table->GfxclkFreq2);
if (ret)
od_table->GfxclkVolt2 = 0;
+   boot_od_table->GfxclkVolt2 = 
od_table->GfxclkVolt2;
}

if (!od_table->GfxclkVolt3) {
@@ -2014,6 +2017,7 @@ static int navi10_set_default_od_settings(struct 
smu_context *smu, bool initiali

od_table->GfxclkFreq3);
if (ret)
od_table->GfxclkVolt3 = 0;
+   boot_od_table->GfxclkVolt3 = 
od_table->GfxclkVolt3;
}
}
}

-Original Message-
From: Quan, Evan 
Sent: Tuesday, February 4, 2020 10:03 AM
To: Alex Deucher ; amd-gfx@lists.freedesktop.org
Cc: Deucher, Alexander 
Subject: RE: [PATCH] drm/amdgpu: fetch default VDDC curve voltages (v2)

Reviewed-by: Evan Quan 

-Original Message-
From: amd-gfx  On Behalf Of Alex Deucher
Sent: Tuesday, February 4, 2020 4:36 AM
To: amd-gfx@lists.freedesktop.org
Cc: Deucher, Alexander 
Subject: [PATCH] drm/amdgpu: fetch default VDDC curve voltages (v2)

Ask the SMU for the default VDDC curve voltage values.  This properly reports 
the VDDC values in the OD interface.

v2: only update if the original values are 0

Bug: 
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitlab.freedesktop.org%2Fdrm%2Famd%2Fissues%2F1020data=02%7C01%7Cevan.quan%40amd.com%7Cdd73683ddcd645aac3dd08d7a8e8bf0b%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637163589962864398sdata=dvhhU0TYEqBoVQc0ZPBkxZT%2FsWzNkggXSETsc9wj190%3Dreserved=0
Signed-off-by: Alex Deucher 
---
 drivers/gpu/drm/amd/powerplay/navi10_ppt.c | 50 +-
 1 file changed, 49 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/powerplay/navi10_ppt.c 
b/drivers/gpu/drm/amd/powerplay/navi10_ppt.c
index 26cfccc57331..a1c1257cf2cb 100644
--- a/drivers/gpu/drm/amd/powerplay/navi10_ppt.c
+++ b/drivers/gpu/drm/amd/powerplay/navi10_ppt.c
@@ -119,6 +119,8 @@ static struct smu_11_0_cmn2aisc_mapping 
navi10_message_map[SMU_MSG_MAX_COUNT] =
MSG_MAP(PowerDownJpeg,  PPSMC_MSG_PowerDownJpeg),
MSG_MAP(BacoAudioD3PME, PPSMC_MSG_BacoAudioD3PME),
MSG_MAP(ArmD3,  PPSMC_MSG_ArmD3),
+   MSG_MAP(GetVoltageByDpm, PPSMC_MSG_GetVoltageByDpm),
+   MSG_MAP(GetVoltageByDpmOverdrive,
PPSMC_MSG_GetVoltageByDpmOverdrive),
 };
 
 static struct smu_11_0_cmn2aisc_mapping navi10_clk_map[SMU_CLK_COUNT] = { @@ 
-1932,6 +1934,28 @@ static int navi10_od_setting_check_range(struct 
smu_11_0_overdrive_table *od_tab
return 0;
 }
 
+static int navi10_overdrive_get_gfx_clk_base_voltage(struct smu_context *smu,
+uint16_t *voltage,
+uint32_t freq)
+{
+   uint32_t param = (freq & 0x) | (PPCLK_GFXCLK << 16);
+   uint32_t value = 0;
+   int ret;
+
+   ret = smu_send_smc_msg_with_param(smu,
+ SMU_MSG_GetVoltageByDpm,
+ param);
+   if (ret) {
+   pr_err("[GetBaseVoltage] failed to get GFXCLK AVFS voltage from 
SMU!");
+   return ret;
+   }
+
+   smu_read_smc_arg(smu, );
+   *voltage

RE: [PATCH 2/2] drm/amdgpu/smu10: fix smu10_get_clock_by_type_with_latency

2020-02-03 Thread Quan, Evan 
Reviewed-by: Evan Quan 

-Original Message-
From: amd-gfx  On Behalf Of Alex Deucher
Sent: Wednesday, January 29, 2020 3:47 AM
To: amd-gfx@lists.freedesktop.org
Cc: Deucher, Alexander 
Subject: [PATCH 2/2] drm/amdgpu/smu10: fix smu10_get_clock_by_type_with_latency

Only send non-0 clocks to DC for validation.  This mirrors what the windows 
driver does.

Bug: 
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitlab.freedesktop.org%2Fdrm%2Famd%2Fissues%2F963data=02%7C01%7Cevan.quan%40amd.com%7C201c9325bf144200f84208d7a42ae179%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637158377025664935sdata=n61wTpS6nqQjPoeM4gDHg9fu79rQIBgtOir%2B%2FJzvj5E%3Dreserved=0
Signed-off-by: Alex Deucher 
---
 drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.c | 15 +--
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.c 
b/drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.c
index 4e8ab139bb3b..273126cfc37d 100644
--- a/drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.c
+++ b/drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.c
@@ -1026,12 +1026,15 @@ static int smu10_get_clock_by_type_with_latency(struct 
pp_hwmgr *hwmgr,
 
clocks->num_levels = 0;
for (i = 0; i < pclk_vol_table->count; i++) {
-   clocks->data[i].clocks_in_khz = pclk_vol_table->entries[i].clk 
* 10;
-   clocks->data[i].latency_in_us = latency_required ?
-   smu10_get_mem_latency(hwmgr,
-   pclk_vol_table->entries[i].clk) 
:
-   0;
-   clocks->num_levels++;
+   if (pclk_vol_table->entries[i].clk) {
+   clocks->data[clocks->num_levels].clocks_in_khz =
+   pclk_vol_table->entries[i].clk * 10;
+   clocks->data[clocks->num_levels].latency_in_us = 
latency_required ?
+   smu10_get_mem_latency(hwmgr,
+ 
pclk_vol_table->entries[i].clk) :
+   0;
+   clocks->num_levels++;
+   }
}
 
return 0;
--
2.24.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfxdata=02%7C01%7Cevan.quan%40amd.com%7C201c9325bf144200f84208d7a42ae179%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637158377025674893sdata=JpmMpRxri5d8oNC8YeMG2SVtJjBucL8H4gmrNDYpVi0%3Dreserved=0
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

RE: [PATCH 3/3] drm/amdgpu/smu10: fix smu10_get_clock_by_type_with_voltage

2020-02-03 Thread Quan, Evan 
Reviewed-by: Evan Quan 

-Original Message-
From: amd-gfx  On Behalf Of Alex Deucher
Sent: Thursday, January 30, 2020 2:02 AM
To: amd-gfx@lists.freedesktop.org
Cc: Deucher, Alexander 
Subject: [PATCH 3/3] drm/amdgpu/smu10: fix smu10_get_clock_by_type_with_voltage

Cull out 0 clocks to avoid a warning in DC.

Bug: 
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitlab.freedesktop.org%2Fdrm%2Famd%2Fissues%2F963data=02%7C01%7Cevan.quan%40amd.com%7Cdb4d6e0d0d9940cf353b08d7a4e54d7f%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637159177099605051sdata=qugcyFk3Qu6xc%2ByuuhAVZ8Am2KRrhp7zZl37p023TW8%3Dreserved=0
Signed-off-by: Alex Deucher 
---
 drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.c | 8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.c 
b/drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.c
index 273126cfc37d..689072a312a7 100644
--- a/drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.c
+++ b/drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.c
@@ -1080,9 +1080,11 @@ static int smu10_get_clock_by_type_with_voltage(struct 
pp_hwmgr *hwmgr,
 
clocks->num_levels = 0;
for (i = 0; i < pclk_vol_table->count; i++) {
-   clocks->data[i].clocks_in_khz = pclk_vol_table->entries[i].clk  
* 10;
-   clocks->data[i].voltage_in_mv = pclk_vol_table->entries[i].vol;
-   clocks->num_levels++;
+   if (pclk_vol_table->entries[i].clk) {
+   clocks->data[clocks->num_levels].clocks_in_khz = 
pclk_vol_table->entries[i].clk  * 10;
+   clocks->data[clocks->num_levels].voltage_in_mv = 
pclk_vol_table->entries[i].vol;
+   clocks->num_levels++;
+   }
}
 
return 0;
-- 
2.24.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfxdata=02%7C01%7Cevan.quan%40amd.com%7Cdb4d6e0d0d9940cf353b08d7a4e54d7f%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637159177099605051sdata=B33UJMbMy0tRJkG%2BM7ajYKktcka%2BDz1ehc8tbpOCh0s%3Dreserved=0
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

RE: [PATCH] drm/amdgpu: fetch default VDDC curve voltages (v2)

2020-02-03 Thread Quan, Evan 
Reviewed-by: Evan Quan 

-Original Message-
From: amd-gfx  On Behalf Of Alex Deucher
Sent: Tuesday, February 4, 2020 4:36 AM
To: amd-gfx@lists.freedesktop.org
Cc: Deucher, Alexander 
Subject: [PATCH] drm/amdgpu: fetch default VDDC curve voltages (v2)

Ask the SMU for the default VDDC curve voltage values.  This properly reports 
the VDDC values in the OD interface.

v2: only update if the original values are 0

Bug: 
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitlab.freedesktop.org%2Fdrm%2Famd%2Fissues%2F1020data=02%7C01%7Cevan.quan%40amd.com%7Cdd73683ddcd645aac3dd08d7a8e8bf0b%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637163589962864398sdata=dvhhU0TYEqBoVQc0ZPBkxZT%2FsWzNkggXSETsc9wj190%3Dreserved=0
Signed-off-by: Alex Deucher 
---
 drivers/gpu/drm/amd/powerplay/navi10_ppt.c | 50 +-
 1 file changed, 49 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/powerplay/navi10_ppt.c 
b/drivers/gpu/drm/amd/powerplay/navi10_ppt.c
index 26cfccc57331..a1c1257cf2cb 100644
--- a/drivers/gpu/drm/amd/powerplay/navi10_ppt.c
+++ b/drivers/gpu/drm/amd/powerplay/navi10_ppt.c
@@ -119,6 +119,8 @@ static struct smu_11_0_cmn2aisc_mapping 
navi10_message_map[SMU_MSG_MAX_COUNT] =
MSG_MAP(PowerDownJpeg,  PPSMC_MSG_PowerDownJpeg),
MSG_MAP(BacoAudioD3PME, PPSMC_MSG_BacoAudioD3PME),
MSG_MAP(ArmD3,  PPSMC_MSG_ArmD3),
+   MSG_MAP(GetVoltageByDpm, PPSMC_MSG_GetVoltageByDpm),
+   MSG_MAP(GetVoltageByDpmOverdrive,
PPSMC_MSG_GetVoltageByDpmOverdrive),
 };
 
 static struct smu_11_0_cmn2aisc_mapping navi10_clk_map[SMU_CLK_COUNT] = { @@ 
-1932,6 +1934,28 @@ static int navi10_od_setting_check_range(struct 
smu_11_0_overdrive_table *od_tab
return 0;
 }
 
+static int navi10_overdrive_get_gfx_clk_base_voltage(struct smu_context *smu,
+uint16_t *voltage,
+uint32_t freq)
+{
+   uint32_t param = (freq & 0x) | (PPCLK_GFXCLK << 16);
+   uint32_t value = 0;
+   int ret;
+
+   ret = smu_send_smc_msg_with_param(smu,
+ SMU_MSG_GetVoltageByDpm,
+ param);
+   if (ret) {
+   pr_err("[GetBaseVoltage] failed to get GFXCLK AVFS voltage from 
SMU!");
+   return ret;
+   }
+
+   smu_read_smc_arg(smu, );
+   *voltage = (uint16_t)value;
+
+   return 0;
+}
+
 static int navi10_setup_od_limits(struct smu_context *smu) {
struct smu_11_0_overdrive_table *overdrive_table = NULL;
struct smu_11_0_powerplay_table *powerplay_table = NULL; @@ -1958,16 
+1982,40 @@ static int navi10_set_default_od_settings(struct smu_context *smu, 
bool initiali
if (ret)
return ret;
 
+   od_table = (OverDriveTable_t *)smu->smu_table.overdrive_table;
if (initialize) {
ret = navi10_setup_od_limits(smu);
if (ret) {
pr_err("Failed to retrieve board OD limits\n");
return ret;
}
+   if (od_table) {
+   if (!od_table->GfxclkVolt1) {
+   ret = 
navi10_overdrive_get_gfx_clk_base_voltage(smu,
+   
_table->GfxclkVolt1,
+   
od_table->GfxclkFreq1);
+   if (ret)
+   od_table->GfxclkVolt1 = 0;
+   }
+
+   if (!od_table->GfxclkVolt2) {
+   ret = 
navi10_overdrive_get_gfx_clk_base_voltage(smu,
+   
_table->GfxclkVolt2,
+   
od_table->GfxclkFreq2);
+   if (ret)
+   od_table->GfxclkVolt2 = 0;
+   }
 
+   if (!od_table->GfxclkVolt3) {
+   ret = 
navi10_overdrive_get_gfx_clk_base_voltage(smu,
+   
_table->GfxclkVolt3,
+   
od_table->GfxclkFreq3);
+   if (ret)
+   od_table->GfxclkVolt3 = 0;
+   }
+   }
}
 
-   od_table = (OverDriveTable_t *)smu->smu_table.overdrive_table;
if (od_table) {
navi10_dump_od_table(od_table);
}
--
2.24.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org

Re: [PATCH 2/2] drm/amdgpu/smu10: fix smu10_get_clock_by_type_with_latency

2020-02-03 Thread Alex Deucher 
Ping?

On Tue, Jan 28, 2020 at 2:47 PM Alex Deucher  wrote:
>
> Only send non-0 clocks to DC for validation.  This mirrors
> what the windows driver does.
>
> Bug: https://gitlab.freedesktop.org/drm/amd/issues/963
> Signed-off-by: Alex Deucher 
> ---
>  drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.c | 15 +--
>  1 file changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.c 
> b/drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.c
> index 4e8ab139bb3b..273126cfc37d 100644
> --- a/drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.c
> +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.c
> @@ -1026,12 +1026,15 @@ static int 
> smu10_get_clock_by_type_with_latency(struct pp_hwmgr *hwmgr,
>
> clocks->num_levels = 0;
> for (i = 0; i < pclk_vol_table->count; i++) {
> -   clocks->data[i].clocks_in_khz = 
> pclk_vol_table->entries[i].clk * 10;
> -   clocks->data[i].latency_in_us = latency_required ?
> -   smu10_get_mem_latency(hwmgr,
> -   
> pclk_vol_table->entries[i].clk) :
> -   0;
> -   clocks->num_levels++;
> +   if (pclk_vol_table->entries[i].clk) {
> +   clocks->data[clocks->num_levels].clocks_in_khz =
> +   pclk_vol_table->entries[i].clk * 10;
> +   clocks->data[clocks->num_levels].latency_in_us = 
> latency_required ?
> +   smu10_get_mem_latency(hwmgr,
> + 
> pclk_vol_table->entries[i].clk) :
> +   0;
> +   clocks->num_levels++;
> +   }
> }
>
> return 0;
> --
> 2.24.1
>
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 3/3] drm/amdgpu/smu10: fix smu10_get_clock_by_type_with_voltage

2020-02-03 Thread Alex Deucher 
Ping?

On Wed, Jan 29, 2020 at 1:01 PM Alex Deucher  wrote:
>
> Cull out 0 clocks to avoid a warning in DC.
>
> Bug: https://gitlab.freedesktop.org/drm/amd/issues/963
> Signed-off-by: Alex Deucher 
> ---
>  drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.c | 8 +---
>  1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.c 
> b/drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.c
> index 273126cfc37d..689072a312a7 100644
> --- a/drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.c
> +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.c
> @@ -1080,9 +1080,11 @@ static int smu10_get_clock_by_type_with_voltage(struct 
> pp_hwmgr *hwmgr,
>
> clocks->num_levels = 0;
> for (i = 0; i < pclk_vol_table->count; i++) {
> -   clocks->data[i].clocks_in_khz = 
> pclk_vol_table->entries[i].clk  * 10;
> -   clocks->data[i].voltage_in_mv = 
> pclk_vol_table->entries[i].vol;
> -   clocks->num_levels++;
> +   if (pclk_vol_table->entries[i].clk) {
> +   clocks->data[clocks->num_levels].clocks_in_khz = 
> pclk_vol_table->entries[i].clk  * 10;
> +   clocks->data[clocks->num_levels].voltage_in_mv = 
> pclk_vol_table->entries[i].vol;
> +   clocks->num_levels++;
> +   }
> }
>
> return 0;
> --
> 2.24.1
>
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm/amdgpu/display: fix logic inversion in program_timing_sync()

2020-02-03 Thread Alex Deucher 
Ping?

On Fri, Jan 10, 2020 at 3:11 PM Alex Deucher  wrote:
>
> It looks like we should be reducing the group size when we don't
> have a plane rather than when we do.
>
> Bug: https://gitlab.freedesktop.org/drm/amd/issues/781
> Fixes: 5fc0cbfad45648 ("drm/amd/display: determine if a pipe is synced by 
> plane state")
> Signed-off-by: Alex Deucher 
> ---
>  drivers/gpu/drm/amd/display/dc/core/dc.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/display/dc/core/dc.c 
> b/drivers/gpu/drm/amd/display/dc/core/dc.c
> index 3d89904003f0..01b27726d9c5 100644
> --- a/drivers/gpu/drm/amd/display/dc/core/dc.c
> +++ b/drivers/gpu/drm/amd/display/dc/core/dc.c
> @@ -1003,9 +1003,9 @@ static void program_timing_sync(
> status->timing_sync_info.master = false;
>
> }
> -   /* remove any other pipes with plane as they have already 
> been synced */
> +   /* remove any other pipes without plane as they have already 
> been synced */
> for (j = j + 1; j < group_size; j++) {
> -   if (pipe_set[j]->plane_state) {
> +   if (!pipe_set[j]->plane_state) {
> group_size--;
> pipe_set[j] = pipe_set[group_size];
> j--;
> --
> 2.24.1
>
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH v2] drm/radeon: have the callers of set_memory_*() check the return value

2020-02-03 Thread Tianlin Li 

> On Feb 3, 2020, at 11:16 AM, Christian König  wrote:
> 
> Am 03.02.20 um 17:18 schrieb Tianlin Li:
>> Right now several architectures allow their set_memory_*() family of
>> functions to fail,
> 
> Oh, that is a detail I previously didn't recognized. Which architectures are 
> that?
> 
> Cause the RS400/480, RS690 and RS740 which are affected by this are 
> integrated in the south-bridge.

At least x86 is. 
Some details: 
https://lore.kernel.org/netdev/20180628213459.28631-4-dan...@iogearbox.net/ 


>>  but callers may not be checking the return values.
>> If set_memory_*() returns with an error, call-site assumptions may be
>> infact wrong to assume that it would either succeed or not succeed at
>> all. Ideally, the failure of set_memory_*() should be passed up the
>> call stack, and callers should examine the failure and deal with it.
>> 
>> Need to fix the callers and add the __must_check attribute. They also
>> may not provide any level of atomicity, in the sense that the memory
>> protections may be left incomplete on failure. This issue likely has a
>> few steps on effects architectures:
>> 1)Have all callers of set_memory_*() helpers check the return value.
>> 2)Add __must_check to all set_memory_*() helpers so that new uses do
>> not ignore the return value.
>> 3)Add atomicity to the calls so that the memory protections aren't left
>> in a partial state.
>> 
>> This series is part of step 1. Make drm/radeon check the return value of
>> set_memory_*().
>> 
>> Signed-off-by: Tianlin Li 
> 
> Reviewed-by: Christian König  >
> 
>> ---
>> v2:
>> The hardware is too old to be tested on and the code cannot be simply
>> removed from the kernel, so this is the solution for the short term.
>> - Just print an error when something goes wrong
>> - Remove patch 2.
>> v1:
>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Flkml%2F20200107192555.20606-1-tli%40digitalocean.com%2Fdata=02%7C01%7Cchristian.koenig%40amd.com%7Cba2176d2ca834214e6b108d7a8c4bb1d%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637163435227030235sdata=mDhUEi3vmxahjsdrZOr83OEIWNBHefO8lkXST%2FW32CE%3Dreserved=0
>>  
>> 
>> ---
>>  drivers/gpu/drm/radeon/radeon_gart.c | 10 ++
>>  1 file changed, 6 insertions(+), 4 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/radeon/radeon_gart.c 
>> b/drivers/gpu/drm/radeon/radeon_gart.c
>> index f178ba321715..a2cc864aa08d 100644
>> --- a/drivers/gpu/drm/radeon/radeon_gart.c
>> +++ b/drivers/gpu/drm/radeon/radeon_gart.c
>> @@ -80,8 +80,9 @@ int radeon_gart_table_ram_alloc(struct radeon_device *rdev)
>>  #ifdef CONFIG_X86
>>  if (rdev->family == CHIP_RS400 || rdev->family == CHIP_RS480 ||
>>  rdev->family == CHIP_RS690 || rdev->family == CHIP_RS740) {
>> -set_memory_uc((unsigned long)ptr,
>> -  rdev->gart.table_size >> PAGE_SHIFT);
>> +if (set_memory_uc((unsigned long)ptr,
>> +  rdev->gart.table_size >> PAGE_SHIFT))
>> +DRM_ERROR("set_memory_uc failed.\n");
>>  }
>>  #endif
>>  rdev->gart.ptr = ptr;
>> @@ -106,8 +107,9 @@ void radeon_gart_table_ram_free(struct radeon_device 
>> *rdev)
>>  #ifdef CONFIG_X86
>>  if (rdev->family == CHIP_RS400 || rdev->family == CHIP_RS480 ||
>>  rdev->family == CHIP_RS690 || rdev->family == CHIP_RS740) {
>> -set_memory_wb((unsigned long)rdev->gart.ptr,
>> -  rdev->gart.table_size >> PAGE_SHIFT);
>> +if (set_memory_wb((unsigned long)rdev->gart.ptr,
>> +  rdev->gart.table_size >> PAGE_SHIFT))
>> +DRM_ERROR("set_memory_wb failed.\n");
>>  }
>>  #endif
>>  pci_free_consistent(rdev->pdev, rdev->gart.table_size,

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH V5] drm: Add support for DP 1.4 Compliance edid corruption test

2020-02-03 Thread Rodrigo Siqueira 
Hi Jerry,

First of all, thanks for your patch. You can see some comments inline,
just simple things.

On 01/31, Jerry (Fangzhi) Zuo wrote:
> Unlike DP 1.2 edid corruption test, DP 1.4 requires to calculate
> real CRC value of the last edid data block, and write it back.
> Current edid CRC calculates routine adds the last CRC byte,
> and check if non-zero.
> 
> This behavior is not accurate; actually, we need to return
> the actual CRC value when corruption is detected.
> This commit changes this issue by returning the calculated CRC,
> and initiate the required sequence.
> 
> Change since v5
> - Obtain real CRC value before dumping bad edid
> 
> Change since v4
> - Fix for CI.CHECKPATCH
> 
> Change since v3
> - Fix a minor typo.
> 
> Change since v2
> - Rewrite checksum computation routine to avoid duplicated code.
> - Rename to avoid confusion.
> 
> Change since v1
> - Have separate routine for returning real CRC.
> 
> Signed-off-by: Jerry (Fangzhi) Zuo 
> ---
>  drivers/gpu/drm/drm_dp_helper.c | 35 +
>  drivers/gpu/drm/drm_edid.c  | 23 ++
>  include/drm/drm_connector.h |  6 ++
>  include/drm/drm_dp_helper.h |  3 +++
>  4 files changed, 63 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
> index f629fc5494a4..18b285fa1a42 100644
> --- a/drivers/gpu/drm/drm_dp_helper.c
> +++ b/drivers/gpu/drm/drm_dp_helper.c
> @@ -351,6 +351,41 @@ int drm_dp_dpcd_read_link_status(struct drm_dp_aux *aux,
>  }
>  EXPORT_SYMBOL(drm_dp_dpcd_read_link_status);
>  
> +/**
> + * drm_dp_send_real_edid_checksum() - send back real edid checksum value
> + * @aux: DisplayPort AUX channel
> + * @real_edid_checksum: real edid checksum for the last block
> + *
> + * Returns true on success

I think this should be:

Returns:
True on success...

> + */
> +bool drm_dp_send_real_edid_checksum(struct drm_dp_aux *aux,
> +u8 real_edid_checksum)

Use tabs intead of space

> +{
> + u8 link_edid_read = 0, auto_test_req = 0, test_resp = 0;
> +
> + drm_dp_dpcd_read(aux, DP_DEVICE_SERVICE_IRQ_VECTOR, _test_req, 1);

drm_dp_dpcd_read() documentation says:

 [..]
 Returns the number of bytes transferred on success, or a negative error
 code on failure. [..][1]

How about catching the return value of drm_dp_dpcd_read() and handle it?

1. drivers/gpu/drm/drm_dp_helper.c

> + auto_test_req &= DP_AUTOMATED_TEST_REQUEST;
> +
> + drm_dp_dpcd_read(aux, DP_TEST_REQUEST, _edid_read, 1);

Same

> + link_edid_read &= DP_TEST_LINK_EDID_READ;
> +
> + if (!auto_test_req || !link_edid_read) {
> + DRM_DEBUG_KMS("Source DUT does not support TEST_EDID_READ\n");

I'm not 100% sure, but I think that drm_dbg_kms() represents the new
approach for handling debug messages. Could you confirm that? If so,
could you update it?

> + return false;
> + }
> +
> + drm_dp_dpcd_write(aux, DP_DEVICE_SERVICE_IRQ_VECTOR, _test_req, 1);
> +
> + /* send back checksum for the last edid extension block data */
> + drm_dp_dpcd_write(aux, DP_TEST_EDID_CHECKSUM, _edid_checksum, 1);

Again, how about handling the return from drm_dp_dpcd_write?

> +
> + test_resp |= DP_TEST_EDID_CHECKSUM_WRITE;
> + drm_dp_dpcd_write(aux, DP_TEST_RESPONSE, _resp, 1);
> +
> + return true;
> +}
> +EXPORT_SYMBOL(drm_dp_send_real_edid_checksum);
> +
>  /**
>   * drm_dp_downstream_max_clock() - extract branch device max
>   * pixel rate for legacy VGA
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 99769d6c9f84..f064e75fb4c5 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -1590,11 +1590,22 @@ static int validate_displayid(u8 *displayid, int 
> length, int idx);
>  static int drm_edid_block_checksum(const u8 *raw_edid)
>  {
>   int i;
> - u8 csum = 0;
> - for (i = 0; i < EDID_LENGTH; i++)
> + u8 csum = 0, crc = 0;
> +
> + for (i = 0; i < EDID_LENGTH - 1; i++)
>   csum += raw_edid[i];
>  
> - return csum;
> + crc = 0x100 - csum;
> +
> + return crc;
> +}
> +
> +static bool drm_edid_block_checksum_diff(const u8 *raw_edid, u8 
> real_checksum)
> +{
> + if (raw_edid[EDID_LENGTH - 1] != real_checksum)
> + return true;
> + else
> + return false;
>  }
>  
>  static bool drm_edid_is_zero(const u8 *in_edid, int length)
> @@ -1652,7 +1663,7 @@ bool drm_edid_block_valid(u8 *raw_edid, int block, bool 
> print_bad_edid,
>   }
>  
>   csum = drm_edid_block_checksum(raw_edid);
> - if (csum) {
> + if (drm_edid_block_checksum_diff(raw_edid, csum)) {
>   if (edid_corrupt)
>   *edid_corrupt = true;
>  
> @@ -1793,6 +1804,10 @@ static void connector_bad_edid(struct drm_connector 
> *connector,
>  u8 *edid, int num_blocks)
>  {
>   int i;
> + u8

Re: [PATCH v4 05/22] drm/amdgpu: Convert to CRTC VBLANK callbacks

2020-02-03 Thread Alex Deucher 
On Thu, Jan 23, 2020 at 9:00 AM Thomas Zimmermann  wrote:
>
> VBLANK callbacks in struct drm_driver are deprecated in favor of
> their equivalents in struct drm_crtc_funcs. Convert amdgpu over.
>
> v2:
> * don't wrap existing functions; change signature instead
>
> Signed-off-by: Thomas Zimmermann 

Reviewed-by: Alex Deucher 

> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu.h   |  6 +++---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_display.c   |  4 ++--
>  drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c   |  4 
>  drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c   | 21 +++
>  drivers/gpu/drm/amd/amdgpu/dce_v10_0.c|  4 
>  drivers/gpu/drm/amd/amdgpu/dce_v11_0.c|  4 
>  drivers/gpu/drm/amd/amdgpu/dce_v6_0.c |  4 
>  drivers/gpu/drm/amd/amdgpu/dce_v8_0.c |  4 
>  drivers/gpu/drm/amd/amdgpu/dce_virtual.c  |  4 
>  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 10 +
>  10 files changed, 43 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index f42e8d467c12..2319fdfc42e5 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -1191,9 +1191,9 @@ void amdgpu_driver_postclose_kms(struct drm_device *dev,
>  int amdgpu_device_ip_suspend(struct amdgpu_device *adev);
>  int amdgpu_device_suspend(struct drm_device *dev, bool fbcon);
>  int amdgpu_device_resume(struct drm_device *dev, bool fbcon);
> -u32 amdgpu_get_vblank_counter_kms(struct drm_device *dev, unsigned int pipe);
> -int amdgpu_enable_vblank_kms(struct drm_device *dev, unsigned int pipe);
> -void amdgpu_disable_vblank_kms(struct drm_device *dev, unsigned int pipe);
> +u32 amdgpu_get_vblank_counter_kms(struct drm_crtc *crtc);
> +int amdgpu_enable_vblank_kms(struct drm_crtc *crtc);
> +void amdgpu_disable_vblank_kms(struct drm_crtc *crtc);
>  long amdgpu_kms_compat_ioctl(struct file *filp, unsigned int cmd,
>  unsigned long arg);
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
> index a1e769d4417d..ad9c9546a64f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
> @@ -99,7 +99,7 @@ static void amdgpu_display_flip_work_func(struct 
> work_struct *__work)
>  & (DRM_SCANOUTPOS_VALID | DRM_SCANOUTPOS_IN_VBLANK)) ==
> (DRM_SCANOUTPOS_VALID | DRM_SCANOUTPOS_IN_VBLANK) &&
> (int)(work->target_vblank -
> - amdgpu_get_vblank_counter_kms(adev->ddev, 
> amdgpu_crtc->crtc_id)) > 0) {
> + amdgpu_get_vblank_counter_kms(crtc)) > 0) {
> schedule_delayed_work(>flip_work, 
> usecs_to_jiffies(1000));
> return;
> }
> @@ -219,7 +219,7 @@ int amdgpu_display_crtc_page_flip_target(struct drm_crtc 
> *crtc,
> if (!adev->enable_virtual_display)
> work->base = amdgpu_bo_gpu_offset(new_abo);
> work->target_vblank = target - (uint32_t)drm_crtc_vblank_count(crtc) +
> -   amdgpu_get_vblank_counter_kms(dev, work->crtc_id);
> +   amdgpu_get_vblank_counter_kms(crtc);
>
> /* we borrow the event spin lock for protecting flip_wrok */
> spin_lock_irqsave(>dev->event_lock, flags);
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> index 955b78f1bba4..bc2fa428013f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> @@ -1388,10 +1388,6 @@ static struct drm_driver kms_driver = {
> .postclose = amdgpu_driver_postclose_kms,
> .lastclose = amdgpu_driver_lastclose_kms,
> .unload = amdgpu_driver_unload_kms,
> -   .get_vblank_counter = amdgpu_get_vblank_counter_kms,
> -   .enable_vblank = amdgpu_enable_vblank_kms,
> -   .disable_vblank = amdgpu_disable_vblank_kms,
> -   .get_vblank_timestamp = drm_calc_vbltimestamp_from_scanoutpos,
> .irq_handler = amdgpu_irq_handler,
> .ioctls = amdgpu_ioctls_kms,
> .gem_free_object_unlocked = amdgpu_gem_object_free,
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> index 60591dbc2097..98c196de27a4 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> @@ -1110,14 +1110,15 @@ void amdgpu_driver_postclose_kms(struct drm_device 
> *dev,
>  /**
>   * amdgpu_get_vblank_counter_kms - get frame count
>   *
> - * @dev: drm dev pointer
> - * @pipe: crtc to get the frame count from
> + * @crtc: crtc to get the frame count from
>   *
>   * Gets the frame count on the requested crtc (all asics).
>   * Returns frame count on success, -EINVAL on failure.
>   */
> -u32 amdgpu_get_vblank_counter_kms(struct drm_device *dev, unsigned int pipe)
> +u32 amdgpu_get_vblank_counter_kms(struct drm_crtc *crtc)

Re: [PATCH v4 04/22] drm/amdgpu: Convert to struct drm_crtc_helper_funcs.get_scanout_position()

2020-02-03 Thread Alex Deucher 
On Thu, Jan 23, 2020 at 9:00 AM Thomas Zimmermann  wrote:
>
> The callback struct drm_driver.get_scanout_position() is deprecated in
> favor of struct drm_crtc_helper_funcs.get_scanout_position(). Convert
> amdgpu over.
>
> Signed-off-by: Thomas Zimmermann 

Reviewed-by: Alex Deucher 

> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_display.c   | 12 
>  drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c   | 11 ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h  |  5 +
>  drivers/gpu/drm/amd/amdgpu/dce_v10_0.c|  1 +
>  drivers/gpu/drm/amd/amdgpu/dce_v11_0.c|  1 +
>  drivers/gpu/drm/amd/amdgpu/dce_v6_0.c |  1 +
>  drivers/gpu/drm/amd/amdgpu/dce_v8_0.c |  1 +
>  drivers/gpu/drm/amd/amdgpu/dce_virtual.c  |  1 +
>  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c |  3 ++-
>  9 files changed, 24 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
> index 4e699071d144..a1e769d4417d 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
> @@ -914,3 +914,15 @@ int amdgpu_display_crtc_idx_to_irq_type(struct 
> amdgpu_device *adev, int crtc)
> return AMDGPU_CRTC_IRQ_NONE;
> }
>  }
> +
> +bool amdgpu_crtc_get_scanout_position(struct drm_crtc *crtc,
> +   bool in_vblank_irq, int *vpos,
> +   int *hpos, ktime_t *stime, ktime_t *etime,
> +   const struct drm_display_mode *mode)
> +{
> +   struct drm_device *dev = crtc->dev;
> +   unsigned int pipe = crtc->index;
> +
> +   return amdgpu_display_get_crtc_scanoutpos(dev, pipe, 0, vpos, hpos,
> + stime, etime, mode);
> +}
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> index a9c4edca70c9..955b78f1bba4 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> @@ -1377,16 +1377,6 @@ int amdgpu_file_to_fpriv(struct file *filp, struct 
> amdgpu_fpriv **fpriv)
> return 0;
>  }
>
> -static bool
> -amdgpu_get_crtc_scanout_position(struct drm_device *dev, unsigned int pipe,
> -bool in_vblank_irq, int *vpos, int *hpos,
> -ktime_t *stime, ktime_t *etime,
> -const struct drm_display_mode *mode)
> -{
> -   return amdgpu_display_get_crtc_scanoutpos(dev, pipe, 0, vpos, hpos,
> - stime, etime, mode);
> -}
> -
>  static struct drm_driver kms_driver = {
> .driver_features =
> DRIVER_USE_AGP | DRIVER_ATOMIC |
> @@ -1402,7 +1392,6 @@ static struct drm_driver kms_driver = {
> .enable_vblank = amdgpu_enable_vblank_kms,
> .disable_vblank = amdgpu_disable_vblank_kms,
> .get_vblank_timestamp = drm_calc_vbltimestamp_from_scanoutpos,
> -   .get_scanout_position = amdgpu_get_crtc_scanout_position,
> .irq_handler = amdgpu_irq_handler,
> .ioctls = amdgpu_ioctls_kms,
> .gem_free_object_unlocked = amdgpu_gem_object_free,
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
> index eb9975f4decb..37ba07e2feb5 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
> @@ -612,6 +612,11 @@ void amdgpu_panel_mode_fixup(struct drm_encoder *encoder,
>  struct drm_display_mode *adjusted_mode);
>  int amdgpu_display_crtc_idx_to_irq_type(struct amdgpu_device *adev, int 
> crtc);
>
> +bool amdgpu_crtc_get_scanout_position(struct drm_crtc *crtc,
> +   bool in_vblank_irq, int *vpos,
> +   int *hpos, ktime_t *stime, ktime_t *etime,
> +   const struct drm_display_mode *mode);
> +
>  /* fbdev layer */
>  int amdgpu_fbdev_init(struct amdgpu_device *adev);
>  void amdgpu_fbdev_fini(struct amdgpu_device *adev);
> diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c 
> b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
> index 40d2ac723dd6..bdc1e0f036d4 100644
> --- a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
> @@ -2685,6 +2685,7 @@ static const struct drm_crtc_helper_funcs 
> dce_v10_0_crtc_helper_funcs = {
> .prepare = dce_v10_0_crtc_prepare,
> .commit = dce_v10_0_crtc_commit,
> .disable = dce_v10_0_crtc_disable,
> +   .get_scanout_position = amdgpu_crtc_get_scanout_position,
>  };
>
>  static int dce_v10_0_crtc_init(struct amdgpu_device *adev, int index)
> diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c 
> b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
> index 898ef72d423c..0319da5f7bf9 100644
> --- a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
> @@

[PATCH] drm/amdgpu: fetch default VDDC curve voltages (v2)

2020-02-03 Thread Alex Deucher 
Ask the SMU for the default VDDC curve voltage values.  This
properly reports the VDDC values in the OD interface.

v2: only update if the original values are 0

Bug: https://gitlab.freedesktop.org/drm/amd/issues/1020
Signed-off-by: Alex Deucher 
---
 drivers/gpu/drm/amd/powerplay/navi10_ppt.c | 50 +-
 1 file changed, 49 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/powerplay/navi10_ppt.c 
b/drivers/gpu/drm/amd/powerplay/navi10_ppt.c
index 26cfccc57331..a1c1257cf2cb 100644
--- a/drivers/gpu/drm/amd/powerplay/navi10_ppt.c
+++ b/drivers/gpu/drm/amd/powerplay/navi10_ppt.c
@@ -119,6 +119,8 @@ static struct smu_11_0_cmn2aisc_mapping 
navi10_message_map[SMU_MSG_MAX_COUNT] =
MSG_MAP(PowerDownJpeg,  PPSMC_MSG_PowerDownJpeg),
MSG_MAP(BacoAudioD3PME, PPSMC_MSG_BacoAudioD3PME),
MSG_MAP(ArmD3,  PPSMC_MSG_ArmD3),
+   MSG_MAP(GetVoltageByDpm, PPSMC_MSG_GetVoltageByDpm),
+   MSG_MAP(GetVoltageByDpmOverdrive,
PPSMC_MSG_GetVoltageByDpmOverdrive),
 };
 
 static struct smu_11_0_cmn2aisc_mapping navi10_clk_map[SMU_CLK_COUNT] = {
@@ -1932,6 +1934,28 @@ static int navi10_od_setting_check_range(struct 
smu_11_0_overdrive_table *od_tab
return 0;
 }
 
+static int navi10_overdrive_get_gfx_clk_base_voltage(struct smu_context *smu,
+uint16_t *voltage,
+uint32_t freq)
+{
+   uint32_t param = (freq & 0x) | (PPCLK_GFXCLK << 16);
+   uint32_t value = 0;
+   int ret;
+
+   ret = smu_send_smc_msg_with_param(smu,
+ SMU_MSG_GetVoltageByDpm,
+ param);
+   if (ret) {
+   pr_err("[GetBaseVoltage] failed to get GFXCLK AVFS voltage from 
SMU!");
+   return ret;
+   }
+
+   smu_read_smc_arg(smu, );
+   *voltage = (uint16_t)value;
+
+   return 0;
+}
+
 static int navi10_setup_od_limits(struct smu_context *smu) {
struct smu_11_0_overdrive_table *overdrive_table = NULL;
struct smu_11_0_powerplay_table *powerplay_table = NULL;
@@ -1958,16 +1982,40 @@ static int navi10_set_default_od_settings(struct 
smu_context *smu, bool initiali
if (ret)
return ret;
 
+   od_table = (OverDriveTable_t *)smu->smu_table.overdrive_table;
if (initialize) {
ret = navi10_setup_od_limits(smu);
if (ret) {
pr_err("Failed to retrieve board OD limits\n");
return ret;
}
+   if (od_table) {
+   if (!od_table->GfxclkVolt1) {
+   ret = 
navi10_overdrive_get_gfx_clk_base_voltage(smu,
+   
_table->GfxclkVolt1,
+   
od_table->GfxclkFreq1);
+   if (ret)
+   od_table->GfxclkVolt1 = 0;
+   }
+
+   if (!od_table->GfxclkVolt2) {
+   ret = 
navi10_overdrive_get_gfx_clk_base_voltage(smu,
+   
_table->GfxclkVolt2,
+   
od_table->GfxclkFreq2);
+   if (ret)
+   od_table->GfxclkVolt2 = 0;
+   }
 
+   if (!od_table->GfxclkVolt3) {
+   ret = 
navi10_overdrive_get_gfx_clk_base_voltage(smu,
+   
_table->GfxclkVolt3,
+   
od_table->GfxclkFreq3);
+   if (ret)
+   od_table->GfxclkVolt3 = 0;
+   }
+   }
}
 
-   od_table = (OverDriveTable_t *)smu->smu_table.overdrive_table;
if (od_table) {
navi10_dump_od_table(od_table);
}
-- 
2.24.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH] drm/amdgpu: fetch default VDDC curve voltages

2020-02-03 Thread Alex Deucher 
Ask the SMU for the default VDDC curve voltage values.  This
properly reports the VDDC values in the OD interface.

Bug: https://gitlab.freedesktop.org/drm/amd/issues/1020
Signed-off-by: Alex Deucher 
---
 drivers/gpu/drm/amd/powerplay/navi10_ppt.c | 44 +-
 1 file changed, 43 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/powerplay/navi10_ppt.c 
b/drivers/gpu/drm/amd/powerplay/navi10_ppt.c
index 26cfccc57331..48db28ddd1d7 100644
--- a/drivers/gpu/drm/amd/powerplay/navi10_ppt.c
+++ b/drivers/gpu/drm/amd/powerplay/navi10_ppt.c
@@ -119,6 +119,8 @@ static struct smu_11_0_cmn2aisc_mapping 
navi10_message_map[SMU_MSG_MAX_COUNT] =
MSG_MAP(PowerDownJpeg,  PPSMC_MSG_PowerDownJpeg),
MSG_MAP(BacoAudioD3PME, PPSMC_MSG_BacoAudioD3PME),
MSG_MAP(ArmD3,  PPSMC_MSG_ArmD3),
+   MSG_MAP(GetVoltageByDpm, PPSMC_MSG_GetVoltageByDpm),
+   MSG_MAP(GetVoltageByDpmOverdrive,
PPSMC_MSG_GetVoltageByDpmOverdrive),
 };
 
 static struct smu_11_0_cmn2aisc_mapping navi10_clk_map[SMU_CLK_COUNT] = {
@@ -1932,6 +1934,28 @@ static int navi10_od_setting_check_range(struct 
smu_11_0_overdrive_table *od_tab
return 0;
 }
 
+static int navi10_overdrive_get_gfx_clk_base_voltage(struct smu_context *smu,
+uint16_t *voltage,
+uint32_t freq)
+{
+   uint32_t param = (freq & 0x) | (PPCLK_GFXCLK << 16);
+   uint32_t value = 0;
+   int ret;
+
+   ret = smu_send_smc_msg_with_param(smu,
+ SMU_MSG_GetVoltageByDpm,
+ param);
+   if (ret) {
+   pr_err("[GetBaseVoltage] failed to get GFXCLK AVFS voltage from 
SMU!");
+   return ret;
+   }
+
+   smu_read_smc_arg(smu, );
+   *voltage = (uint16_t)value;
+
+   return 0;
+}
+
 static int navi10_setup_od_limits(struct smu_context *smu) {
struct smu_11_0_overdrive_table *overdrive_table = NULL;
struct smu_11_0_powerplay_table *powerplay_table = NULL;
@@ -1958,16 +1982,34 @@ static int navi10_set_default_od_settings(struct 
smu_context *smu, bool initiali
if (ret)
return ret;
 
+   od_table = (OverDriveTable_t *)smu->smu_table.overdrive_table;
if (initialize) {
ret = navi10_setup_od_limits(smu);
if (ret) {
pr_err("Failed to retrieve board OD limits\n");
return ret;
}
+   if (od_table) {
+   ret = navi10_overdrive_get_gfx_clk_base_voltage(smu,
+   
_table->GfxclkVolt1,
+   
od_table->GfxclkFreq1);
+   if (ret)
+   od_table->GfxclkVolt1 = 0;
+
+   ret = navi10_overdrive_get_gfx_clk_base_voltage(smu,
+   
_table->GfxclkVolt2,
+   
od_table->GfxclkFreq2);
+   if (ret)
+   od_table->GfxclkVolt2 = 0;
 
+   ret = navi10_overdrive_get_gfx_clk_base_voltage(smu,
+   
_table->GfxclkVolt3,
+   
od_table->GfxclkFreq3);
+   if (ret)
+   od_table->GfxclkVolt3 = 0;
+   }
}
 
-   od_table = (OverDriveTable_t *)smu->smu_table.overdrive_table;
if (od_table) {
navi10_dump_od_table(od_table);
}
-- 
2.24.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH v3] drm/amdkfd: Add queue information to sysfs

2020-02-03 Thread Amber Lin 
Provide compute queues information in sysfs under /sys/class/kfd/kfd/proc.
The format is /sys/class/kfd/kfd/proc//queues//XX where
XX are size, type, and gpuid three files to represent queue size, queue
type, and the GPU this queue uses.  folder and files underneath
are generated when a queue is created. They are removed when the queue is
destroyed.

Signed-off-by: Amber Lin 
---
 drivers/gpu/drm/amd/amdkfd/kfd_priv.h  |  7 ++
 drivers/gpu/drm/amd/amdkfd/kfd_process.c   | 90 ++
 .../gpu/drm/amd/amdkfd/kfd_process_queue_manager.c |  2 +
 3 files changed, 99 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h 
b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
index c0b0def..f805f55 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
@@ -41,6 +41,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "amd_shared.h"
 
@@ -503,6 +504,9 @@ struct queue {
struct kfd_process  *process;
struct kfd_dev  *device;
void *gws;
+
+   /* procfs */
+   struct kobject kobj;
 };
 
 /*
@@ -730,6 +734,7 @@ struct kfd_process {
 
/* Kobj for our procfs */
struct kobject *kobj;
+   struct kobject *kobj_queues;
struct attribute attr_pasid;
 };
 
@@ -836,6 +841,8 @@ extern struct device *kfd_device;
 /* KFD's procfs */
 void kfd_procfs_init(void);
 void kfd_procfs_shutdown(void);
+int kfd_procfs_add_queue(struct queue *q);
+void kfd_procfs_del_queue(struct queue *q);
 
 /* Topology */
 int kfd_topology_init(void);
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
index 25b90f7..98dcbb9 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
@@ -132,6 +132,88 @@ void kfd_procfs_shutdown(void)
}
 }
 
+static ssize_t kfd_procfs_queue_show(struct kobject *kobj,
+struct attribute *attr, char *buffer)
+{
+   struct queue *q = container_of(kobj, struct queue, kobj);
+
+   if (!strcmp(attr->name, "size"))
+   return snprintf(buffer, PAGE_SIZE, "%llu",
+   q->properties.queue_size);
+   else if (!strcmp(attr->name, "type"))
+   return snprintf(buffer, PAGE_SIZE, "%d", q->properties.type);
+   else if (!strcmp(attr->name, "gpuid"))
+   return snprintf(buffer, PAGE_SIZE, "%u", q->device->id);
+   else
+   pr_err("Invalid attribute");
+
+   return 0;
+}
+
+static struct attribute attr_queue_size = {
+   .name = "size",
+   .mode = KFD_SYSFS_FILE_MODE
+};
+
+static struct attribute attr_queue_type = {
+   .name = "type",
+   .mode = KFD_SYSFS_FILE_MODE
+};
+
+static struct attribute attr_queue_gpuid = {
+   .name = "gpuid",
+   .mode = KFD_SYSFS_FILE_MODE
+};
+
+static struct attribute *procfs_queue_attrs[] = {
+   _queue_size,
+   _queue_type,
+   _queue_gpuid,
+   NULL
+};
+
+static const struct sysfs_ops procfs_queue_ops = {
+   .show = kfd_procfs_queue_show,
+};
+
+static struct kobj_type procfs_queue_type = {
+   .sysfs_ops = _queue_ops,
+   .default_attrs = procfs_queue_attrs,
+};
+
+int kfd_procfs_add_queue(struct queue *q)
+{
+   struct kfd_process *proc;
+   int ret;
+
+   if (!q || !q->process)
+   return -EINVAL;
+   proc = q->process;
+
+   /* Create proc//queues/ folder */
+   if (!proc->kobj_queues)
+   return -EFAULT;
+   ret = kobject_init_and_add(>kobj, _queue_type,
+   proc->kobj_queues, "%u", q->properties.queue_id);
+   if (ret < 0) {
+   pr_warn("Creating proc//queues/%u failed",
+   q->properties.queue_id);
+   kobject_put(>kobj);
+   return ret;
+   }
+
+   return 0;
+}
+
+void kfd_procfs_del_queue(struct queue *q)
+{
+   if (!q)
+   return;
+
+   kobject_del(>kobj);
+   kobject_put(>kobj);
+}
+
 int kfd_process_create_wq(void)
 {
if (!kfd_process_wq)
@@ -323,6 +405,11 @@ struct kfd_process *kfd_create_process(struct file *filep)
if (ret)
pr_warn("Creating pasid for pid %d failed",
(int)process->lead_thread->pid);
+
+   process->kobj_queues = kobject_create_and_add("queues",
+   process->kobj);
+   if (!process->kobj_queues)
+   pr_warn("Creating KFD proc/queues folder failed");
}
 out:
if (!IS_ERR(process))
@@ -457,6 +544,9 @@ static void kfd_process_wq_release(struct work_struct *work)
/* Remove the procfs files */
if (p->kobj) {
sysfs_remove_file(p->kobj, >attr_pasid);
+   kobject_del(p->kobj_queues);
+   kobject_put(p->kobj_queues);
+

Re: [PATCH v2] drm/radeon: have the callers of set_memory_*() check the return value

2020-02-03 Thread Christian König 

Am 03.02.20 um 17:18 schrieb Tianlin Li:

Right now several architectures allow their set_memory_*() family of
functions to fail,


Oh, that is a detail I previously didn't recognized. Which architectures 
are that?


Cause the RS400/480, RS690 and RS740 which are affected by this are 
integrated in the south-bridge.



  but callers may not be checking the return values.
If set_memory_*() returns with an error, call-site assumptions may be
infact wrong to assume that it would either succeed or not succeed at
all. Ideally, the failure of set_memory_*() should be passed up the
call stack, and callers should examine the failure and deal with it.

Need to fix the callers and add the __must_check attribute. They also
may not provide any level of atomicity, in the sense that the memory
protections may be left incomplete on failure. This issue likely has a
few steps on effects architectures:
1)Have all callers of set_memory_*() helpers check the return value.
2)Add __must_check to all set_memory_*() helpers so that new uses do
not ignore the return value.
3)Add atomicity to the calls so that the memory protections aren't left
in a partial state.

This series is part of step 1. Make drm/radeon check the return value of
set_memory_*().

Signed-off-by: Tianlin Li 


Reviewed-by: Christian König 


---
v2:
The hardware is too old to be tested on and the code cannot be simply
removed from the kernel, so this is the solution for the short term.
- Just print an error when something goes wrong
- Remove patch 2.
v1:
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Flkml%2F20200107192555.20606-1-tli%40digitalocean.com%2Fdata=02%7C01%7Cchristian.koenig%40amd.com%7Cba2176d2ca834214e6b108d7a8c4bb1d%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637163435227030235sdata=mDhUEi3vmxahjsdrZOr83OEIWNBHefO8lkXST%2FW32CE%3Dreserved=0
---
  drivers/gpu/drm/radeon/radeon_gart.c | 10 ++
  1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_gart.c 
b/drivers/gpu/drm/radeon/radeon_gart.c
index f178ba321715..a2cc864aa08d 100644
--- a/drivers/gpu/drm/radeon/radeon_gart.c
+++ b/drivers/gpu/drm/radeon/radeon_gart.c
@@ -80,8 +80,9 @@ int radeon_gart_table_ram_alloc(struct radeon_device *rdev)
  #ifdef CONFIG_X86
if (rdev->family == CHIP_RS400 || rdev->family == CHIP_RS480 ||
rdev->family == CHIP_RS690 || rdev->family == CHIP_RS740) {
-   set_memory_uc((unsigned long)ptr,
- rdev->gart.table_size >> PAGE_SHIFT);
+   if (set_memory_uc((unsigned long)ptr,
+ rdev->gart.table_size >> PAGE_SHIFT))
+   DRM_ERROR("set_memory_uc failed.\n");
}
  #endif
rdev->gart.ptr = ptr;
@@ -106,8 +107,9 @@ void radeon_gart_table_ram_free(struct radeon_device *rdev)
  #ifdef CONFIG_X86
if (rdev->family == CHIP_RS400 || rdev->family == CHIP_RS480 ||
rdev->family == CHIP_RS690 || rdev->family == CHIP_RS740) {
-   set_memory_wb((unsigned long)rdev->gart.ptr,
- rdev->gart.table_size >> PAGE_SHIFT);
+   if (set_memory_wb((unsigned long)rdev->gart.ptr,
+ rdev->gart.table_size >> PAGE_SHIFT))
+   DRM_ERROR("set_memory_wb failed.\n");
}
  #endif
pci_free_consistent(rdev->pdev, rdev->gart.table_size,


___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH v2] drm/radeon: have the callers of set_memory_*() check the return value

2020-02-03 Thread Tianlin Li 
Right now several architectures allow their set_memory_*() family of  
functions to fail, but callers may not be checking the return values.
If set_memory_*() returns with an error, call-site assumptions may be
infact wrong to assume that it would either succeed or not succeed at  
all. Ideally, the failure of set_memory_*() should be passed up the 
call stack, and callers should examine the failure and deal with it. 

Need to fix the callers and add the __must_check attribute. They also 
may not provide any level of atomicity, in the sense that the memory 
protections may be left incomplete on failure. This issue likely has a 
few steps on effects architectures:
1)Have all callers of set_memory_*() helpers check the return value.
2)Add __must_check to all set_memory_*() helpers so that new uses do  
not ignore the return value.
3)Add atomicity to the calls so that the memory protections aren't left 
in a partial state.

This series is part of step 1. Make drm/radeon check the return value of  
set_memory_*().

Signed-off-by: Tianlin Li 
---
v2:
The hardware is too old to be tested on and the code cannot be simply
removed from the kernel, so this is the solution for the short term. 
- Just print an error when something goes wrong
- Remove patch 2.  
v1:
https://lore.kernel.org/lkml/20200107192555.20606-1-...@digitalocean.com/
---
 drivers/gpu/drm/radeon/radeon_gart.c | 10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_gart.c 
b/drivers/gpu/drm/radeon/radeon_gart.c
index f178ba321715..a2cc864aa08d 100644
--- a/drivers/gpu/drm/radeon/radeon_gart.c
+++ b/drivers/gpu/drm/radeon/radeon_gart.c
@@ -80,8 +80,9 @@ int radeon_gart_table_ram_alloc(struct radeon_device *rdev)
 #ifdef CONFIG_X86
if (rdev->family == CHIP_RS400 || rdev->family == CHIP_RS480 ||
rdev->family == CHIP_RS690 || rdev->family == CHIP_RS740) {
-   set_memory_uc((unsigned long)ptr,
- rdev->gart.table_size >> PAGE_SHIFT);
+   if (set_memory_uc((unsigned long)ptr,
+ rdev->gart.table_size >> PAGE_SHIFT))
+   DRM_ERROR("set_memory_uc failed.\n");
}
 #endif
rdev->gart.ptr = ptr;
@@ -106,8 +107,9 @@ void radeon_gart_table_ram_free(struct radeon_device *rdev)
 #ifdef CONFIG_X86
if (rdev->family == CHIP_RS400 || rdev->family == CHIP_RS480 ||
rdev->family == CHIP_RS690 || rdev->family == CHIP_RS740) {
-   set_memory_wb((unsigned long)rdev->gart.ptr,
- rdev->gart.table_size >> PAGE_SHIFT);
+   if (set_memory_wb((unsigned long)rdev->gart.ptr,
+ rdev->gart.table_size >> PAGE_SHIFT))
+   DRM_ERROR("set_memory_wb failed.\n");
}
 #endif
pci_free_consistent(rdev->pdev, rdev->gart.table_size,
-- 
2.17.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: KASAN: use-after-free Read in vgem_gem_dumb_create

2020-02-03 Thread Christian König 

Am 03.02.20 um 10:06 schrieb Dan Carpenter:

On Sun, Feb 02, 2020 at 02:19:18PM +0100, Daniel Vetter wrote:

On Fri, Jan 31, 2020 at 11:28 PM syzbot
 wrote:

Hello,

syzbot found the following crash on:

HEAD commit:39bed42d Merge tag 'for-linus-hmm' of git://git.kernel.org..
git tree:   upstream
console output: 
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fsyzkaller.appspot.com%2Fx%2Flog.txt%3Fx%3D179465bee0data=02%7C01%7Cchristian.koenig%40amd.com%7C529f2273b8374f38560108d7a88862eb%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637163176051177627sdata=3goGqBs4%2BjkjCeV2bX5VTB%2F1PRLEP5bzq5Ec%2BN7fKHs%3Dreserved=0
kernel config:  
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fsyzkaller.appspot.com%2Fx%2F.config%3Fx%3D2646535f8818ae25data=02%7C01%7Cchristian.koenig%40amd.com%7C529f2273b8374f38560108d7a88862eb%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637163176051177627sdata=SnlKln%2FAG%2BVRVjSrOSJjUE%2BhSDf35wTqzWLCAyGQVss%3Dreserved=0
dashboard link: 
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fsyzkaller.appspot.com%2Fbug%3Fextid%3D0dc774d419e916c8data=02%7C01%7Cchristian.koenig%40amd.com%7C529f2273b8374f38560108d7a88862eb%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637163176051177627sdata=33EJNAWjTm6Edi1J0oPBfs8epb%2BQ2cpAKlzl1sT40CQ%3Dreserved=0
compiler:   gcc (GCC) 9.0.0 20181231 (experimental)
syz repro:  
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fsyzkaller.appspot.com%2Fx%2Frepro.syz%3Fx%3D16251279e0data=02%7C01%7Cchristian.koenig%40amd.com%7C529f2273b8374f38560108d7a88862eb%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637163176051177627sdata=zmUyyp7znqQfLzzNZ80bNgCILAjeMeCVVr7xf7CHaWk%3Dreserved=0

The bug was bisected to:

commit 7611750784664db46d0db95631e322aeb263dde7
Author: Alex Deucher 
Date:   Wed Jun 21 16:31:41 2017 +

 drm/amdgpu: use kernel is_power_of_2 rather than local version

bisection log:  
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fsyzkaller.appspot.com%2Fx%2Fbisect.txt%3Fx%3D11628df1e0data=02%7C01%7Cchristian.koenig%40amd.com%7C529f2273b8374f38560108d7a88862eb%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637163176051177627sdata=5QpTG4iU%2FOt22L3jxRbNxtVPZZ2EvBAcFGZdqVnVCbU%3Dreserved=0
final crash:
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fsyzkaller.appspot.com%2Fx%2Freport.txt%3Fx%3D13628df1e0data=02%7C01%7Cchristian.koenig%40amd.com%7C529f2273b8374f38560108d7a88862eb%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637163176051177627sdata=hN6UZnFR2nIMPMspjIF7S82oXstaRl%2BLAzmz5yujPac%3Dreserved=0
console output: 
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fsyzkaller.appspot.com%2Fx%2Flog.txt%3Fx%3D15628df1e0data=02%7C01%7Cchristian.koenig%40amd.com%7C529f2273b8374f38560108d7a88862eb%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637163176051177627sdata=LHXMANOURDv3EsqTSvHSBZnPEzGQoJU1RbeqYExCaGk%3Dreserved=0

IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+0dc774d419e91...@syzkaller.appspotmail.com
Fixes: 761175078466 ("drm/amdgpu: use kernel is_power_of_2 rather than local 
version")

Aside: This bisect line is complete nonsense ... I'm kinda at the
point where I'm assuming that syzbot bisect results are garbage, which
is maybe not what we want. I guess much stricter filtering for noise
is needed, dunno.
-Danile

With race conditions the git bisect is often nonsense.


Which makes sense, but we can still try to sanitize the result. I'm not 
familiar with the test case, but I think it doesn't even compile the 
amdgpu driver.


So skipping all patches of stuff you don't even compile would make not 
only the result of bisecting quite a bit more reliable, but also speed 
the process up quite a bit.


But no good idea to how teach that to a compile bot or the git bisect 
command.


Regards,
Christian.



regards,
dan carpenter



___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: KASAN: use-after-free Read in vgem_gem_dumb_create

2020-02-03 Thread Dan Carpenter 
On Sun, Feb 02, 2020 at 02:19:18PM +0100, Daniel Vetter wrote:
> On Fri, Jan 31, 2020 at 11:28 PM syzbot
>  wrote:
> >
> > Hello,
> >
> > syzbot found the following crash on:
> >
> > HEAD commit:39bed42d Merge tag 'for-linus-hmm' of git://git.kernel.org..
> > git tree:   upstream
> > console output: https://syzkaller.appspot.com/x/log.txt?x=179465bee0
> > kernel config:  https://syzkaller.appspot.com/x/.config?x=2646535f8818ae25
> > dashboard link: https://syzkaller.appspot.com/bug?extid=0dc774d419e916c8
> > compiler:   gcc (GCC) 9.0.0 20181231 (experimental)
> > syz repro:  https://syzkaller.appspot.com/x/repro.syz?x=16251279e0
> >
> > The bug was bisected to:
> >
> > commit 7611750784664db46d0db95631e322aeb263dde7
> > Author: Alex Deucher 
> > Date:   Wed Jun 21 16:31:41 2017 +
> >
> > drm/amdgpu: use kernel is_power_of_2 rather than local version
> >
> > bisection log:  https://syzkaller.appspot.com/x/bisect.txt?x=11628df1e0
> > final crash:https://syzkaller.appspot.com/x/report.txt?x=13628df1e0
> > console output: https://syzkaller.appspot.com/x/log.txt?x=15628df1e0
> >
> > IMPORTANT: if you fix the bug, please add the following tag to the commit:
> > Reported-by: syzbot+0dc774d419e91...@syzkaller.appspotmail.com
> > Fixes: 761175078466 ("drm/amdgpu: use kernel is_power_of_2 rather than 
> > local version")
> 
> Aside: This bisect line is complete nonsense ... I'm kinda at the
> point where I'm assuming that syzbot bisect results are garbage, which
> is maybe not what we want. I guess much stricter filtering for noise
> is needed, dunno.
> -Danile

With race conditions the git bisect is often nonsense.

regards,
dan carpenter

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Fails to lauch X on laptop with Ryzen 7 4700U

2020-02-03 Thread Chris Chiu 
We are working with new laptops that have the Ryzen 7 4700U.

It fails to launch X so I can only access via the virtual terminal.
I tried with the latest mainline kernel and kernel from
https://cgit.freedesktop.org/~agd5f/linux but no luck. I also boot
the kernel with parameter amdgpu.exp_hw_support=1, but
the system freezes after loading amdgpu and I can't even switch
to the virtual terminal.

I post the bug description and related information on
https://gitlab.freedesktop.org/drm/amd/issues/1031.

Please kindly advise what I should do next.

Thanks
Chris
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH v2] drm/amdkfd: Make process queues logs less verbose

2020-02-03 Thread Julian Sax 
During normal usage, especially if jobs are started and stopped in rapid
succession, the kernel log is filled with messages like this:

[38732.522910] Restoring PASID 0x8003 queues
[38732.666767] Evicting PASID 0x8003 queues
[38732.714074] Restoring PASID 0x8003 queues
[38732.815633] Evicting PASID 0x8003 queues
[38732.834961] Restoring PASID 0x8003 queues
[38732.840536] Evicting PASID 0x8003 queues
[38732.869846] Restoring PASID 0x8003 queues
[38732.893655] Evicting PASID 0x8003 queues
[38732.927975] Restoring PASID 0x8003 queues

According to [1], these messages are expected, but they carry little
value for the end user, so turn them into debug messages.

[1] https://github.com/RadeonOpenCompute/ROCm/issues/343

Signed-off-by: Julian Sax 
---
v2: fixed indenting of following lines

 .../drm/amd/amdkfd/kfd_device_queue_manager.c| 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
index 2870553a2ce0..13bd588c4419 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
@@ -604,8 +604,8 @@ static int evict_process_queues_nocpsch(struct 
device_queue_manager *dqm,
goto out;

pdd = qpd_to_pdd(qpd);
-   pr_info_ratelimited("Evicting PASID 0x%x queues\n",
-   pdd->process->pasid);
+   pr_debug_ratelimited("Evicting PASID 0x%x queues\n",
+pdd->process->pasid);

/* Mark all queues as evicted. Deactivate all active queues on
 * the qpd.
@@ -650,8 +650,8 @@ static int evict_process_queues_cpsch(struct 
device_queue_manager *dqm,
goto out;

pdd = qpd_to_pdd(qpd);
-   pr_info_ratelimited("Evicting PASID 0x%x queues\n",
-   pdd->process->pasid);
+   pr_debug_ratelimited("Evicting PASID 0x%x queues\n",
+pdd->process->pasid);

/* Mark all queues as evicted. Deactivate all active queues on
 * the qpd.
@@ -696,8 +696,8 @@ static int restore_process_queues_nocpsch(struct 
device_queue_manager *dqm,
goto out;
}

-   pr_info_ratelimited("Restoring PASID 0x%x queues\n",
-   pdd->process->pasid);
+   pr_debug_ratelimited("Restoring PASID 0x%x queues\n",
+pdd->process->pasid);

/* Update PD Base in QPD */
qpd->page_table_base = pd_base;
@@ -772,8 +772,8 @@ static int restore_process_queues_cpsch(struct 
device_queue_manager *dqm,
goto out;
}

-   pr_info_ratelimited("Restoring PASID 0x%x queues\n",
-   pdd->process->pasid);
+   pr_debug_ratelimited("Restoring PASID 0x%x queues\n",
+pdd->process->pasid);

/* Update PD Base in QPD */
qpd->page_table_base = pd_base;
--
2.24.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm/amdkfd: Make process queues logs less verbose

2020-02-03 Thread Julian Sax 
Joe Perches  writes:

> On Sun, 2020-02-02 at 00:11 +0100, Julian Sax wrote:
>> During normal usage, especially if jobs are started and stopped in rapid
>> succession, the kernel log is filled with messages like this:
>>
>> [38732.522910] Restoring PASID 0x8003 queues
>> [38732.666767] Evicting PASID 0x8003 queues
>> [38732.714074] Restoring PASID 0x8003 queues
>> [38732.815633] Evicting PASID 0x8003 queues
>> [38732.834961] Restoring PASID 0x8003 queues
>> [38732.840536] Evicting PASID 0x8003 queues
>> [38732.869846] Restoring PASID 0x8003 queues
>> [38732.893655] Evicting PASID 0x8003 queues
>> [38732.927975] Restoring PASID 0x8003 queues
>>
>> According to [1], these messages are expected, but they carry little
>> value for the end user, so turn them into debug messages.
>>
>> [1] https://github.com/RadeonOpenCompute/ROCm/issues/343
>
> trivia:
>
>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c 
>> b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> []
>> @@ -604,7 +604,7 @@ static int evict_process_queues_nocpsch(struct 
>> device_queue_manager *dqm,
>>  goto out;
>>
>>  pdd = qpd_to_pdd(qpd);
>> -pr_info_ratelimited("Evicting PASID 0x%x queues\n",
>> +pr_debug_ratelimited("Evicting PASID 0x%x queues\n",
>>  pdd->process->pasid);
>
> It would be nicer to realign all the subsequent lines in a
> single statement to the now moved open parenthesis.
>
>>
>>  /* Mark all queues as evicted. Deactivate all active queues on
>> @@ -650,7 +650,7 @@ static int evict_process_queues_cpsch(struct 
>> device_queue_manager *dqm,
>>  goto out;
>>
>>  pdd = qpd_to_pdd(qpd);
>> -pr_info_ratelimited("Evicting PASID 0x%x queues\n",
>> +pr_debug_ratelimited("Evicting PASID 0x%x queues\n",
>>  pdd->process->pasid);
>
> etc...

Yeah, absolutely, thanks for pointing that out. v2 coming shortly.

Julian
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH] drm/amdkfd: Make process queues logs less verbose

2020-02-03 Thread Julian Sax 
During normal usage, especially if jobs are started and stopped in rapid
succession, the kernel log is filled with messages like this:

[38732.522910] Restoring PASID 0x8003 queues
[38732.666767] Evicting PASID 0x8003 queues
[38732.714074] Restoring PASID 0x8003 queues
[38732.815633] Evicting PASID 0x8003 queues
[38732.834961] Restoring PASID 0x8003 queues
[38732.840536] Evicting PASID 0x8003 queues
[38732.869846] Restoring PASID 0x8003 queues
[38732.893655] Evicting PASID 0x8003 queues
[38732.927975] Restoring PASID 0x8003 queues

According to [1], these messages are expected, but they carry little
value for the end user, so turn them into debug messages.

[1] https://github.com/RadeonOpenCompute/ROCm/issues/343

Signed-off-by: Julian Sax 
---
 drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
index 984c2f2b24b6..3dc0e8c3b91b 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
@@ -604,7 +604,7 @@ static int evict_process_queues_nocpsch(struct 
device_queue_manager *dqm,
goto out;

pdd = qpd_to_pdd(qpd);
-   pr_info_ratelimited("Evicting PASID 0x%x queues\n",
+   pr_debug_ratelimited("Evicting PASID 0x%x queues\n",
pdd->process->pasid);

/* Mark all queues as evicted. Deactivate all active queues on
@@ -650,7 +650,7 @@ static int evict_process_queues_cpsch(struct 
device_queue_manager *dqm,
goto out;

pdd = qpd_to_pdd(qpd);
-   pr_info_ratelimited("Evicting PASID 0x%x queues\n",
+   pr_debug_ratelimited("Evicting PASID 0x%x queues\n",
pdd->process->pasid);

/* Mark all queues as evicted. Deactivate all active queues on
@@ -696,7 +696,7 @@ static int restore_process_queues_nocpsch(struct 
device_queue_manager *dqm,
goto out;
}

-   pr_info_ratelimited("Restoring PASID 0x%x queues\n",
+   pr_debug_ratelimited("Restoring PASID 0x%x queues\n",
pdd->process->pasid);

/* Update PD Base in QPD */
@@ -772,7 +772,7 @@ static int restore_process_queues_cpsch(struct 
device_queue_manager *dqm,
goto out;
}

-   pr_info_ratelimited("Restoring PASID 0x%x queues\n",
+   pr_debug_ratelimited("Restoring PASID 0x%x queues\n",
pdd->process->pasid);

/* Update PD Base in QPD */
--
2.24.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm/amdkfd: Make process queues logs less verbose

2020-02-03 Thread Joe Perches 
On Sun, 2020-02-02 at 00:11 +0100, Julian Sax wrote:
> During normal usage, especially if jobs are started and stopped in rapid
> succession, the kernel log is filled with messages like this:
> 
> [38732.522910] Restoring PASID 0x8003 queues
> [38732.666767] Evicting PASID 0x8003 queues
> [38732.714074] Restoring PASID 0x8003 queues
> [38732.815633] Evicting PASID 0x8003 queues
> [38732.834961] Restoring PASID 0x8003 queues
> [38732.840536] Evicting PASID 0x8003 queues
> [38732.869846] Restoring PASID 0x8003 queues
> [38732.893655] Evicting PASID 0x8003 queues
> [38732.927975] Restoring PASID 0x8003 queues
> 
> According to [1], these messages are expected, but they carry little
> value for the end user, so turn them into debug messages.
> 
> [1] https://github.com/RadeonOpenCompute/ROCm/issues/343

trivia:

> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c 
> b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
[]
> @@ -604,7 +604,7 @@ static int evict_process_queues_nocpsch(struct 
> device_queue_manager *dqm,
>   goto out;
> 
>   pdd = qpd_to_pdd(qpd);
> - pr_info_ratelimited("Evicting PASID 0x%x queues\n",
> + pr_debug_ratelimited("Evicting PASID 0x%x queues\n",
>   pdd->process->pasid);

It would be nicer to realign all the subsequent lines in a
single statement to the now moved open parenthesis.

> 
>   /* Mark all queues as evicted. Deactivate all active queues on
> @@ -650,7 +650,7 @@ static int evict_process_queues_cpsch(struct 
> device_queue_manager *dqm,
>   goto out;
> 
>   pdd = qpd_to_pdd(qpd);
> - pr_info_ratelimited("Evicting PASID 0x%x queues\n",
> + pr_debug_ratelimited("Evicting PASID 0x%x queues\n",
>   pdd->process->pasid);

etc...


___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH -next] drm/amdgpu: remove unnecessary conversion to bool

2020-02-03 Thread Chen Zhou 
Fixes coccicheck warnings:

./drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c:2198:40-45: WARNING:
conversion to bool not needed here
./drivers/gpu/drm/amd/amdgpu/amdgpu_device.c:2884:68-73: WARNING:
conversion to bool not needed here

Signed-off-by: Chen Zhou 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 2 +-
 drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 8df7727..eb5e3f7 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -2881,7 +2881,7 @@ int amdgpu_device_init(struct amdgpu_device *adev,
INIT_WORK(>xgmi_reset_work, amdgpu_device_xgmi_reset_func);
 
adev->gfx.gfx_off_req_count = 1;
-   adev->pm.ac_power = power_supply_is_system_supplied() > 0 ? true : 
false;
+   adev->pm.ac_power = power_supply_is_system_supplied() > 0;
 
/* Registers mapping */
/* TODO: block userspace mapping of io register */
diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c 
b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
index 5d49253..9d479a0 100644
--- a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
@@ -2195,7 +2195,7 @@ static int sdma_v4_0_set_powergating_state(void *handle,
switch (adev->asic_type) {
case CHIP_RAVEN:
sdma_v4_1_update_power_gating(adev,
-   state == AMD_PG_STATE_GATE ? true : false);
+   state == AMD_PG_STATE_GATE);
break;
default:
break;
-- 
2.7.4

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: KASAN: use-after-free Read in vgem_gem_dumb_create

2020-02-03 Thread syzbot 
syzbot has found a reproducer for the following crash on:

HEAD commit:94f2630b Merge tag '5.6-rc-small-smb3-fix-for-stable' of g..
git tree:   upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=11d6c776e0
kernel config:  https://syzkaller.appspot.com/x/.config?x=99db4e42d047be3
dashboard link: https://syzkaller.appspot.com/bug?extid=0dc774d419e916c8
compiler:   gcc (GCC) 9.0.0 20181231 (experimental)
syz repro:  https://syzkaller.appspot.com/x/repro.syz?x=152385bee0
C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=123210a1e0

The bug was bisected to:

commit 7611750784664db46d0db95631e322aeb263dde7
Author: Alex Deucher 
Date:   Wed Jun 21 16:31:41 2017 +

drm/amdgpu: use kernel is_power_of_2 rather than local version

bisection log:  https://syzkaller.appspot.com/x/bisect.txt?x=11628df1e0
final crash:https://syzkaller.appspot.com/x/report.txt?x=13628df1e0
console output: https://syzkaller.appspot.com/x/log.txt?x=15628df1e0

IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+0dc774d419e91...@syzkaller.appspotmail.com
Fixes: 761175078466 ("drm/amdgpu: use kernel is_power_of_2 rather than local 
version")

==
BUG: KASAN: use-after-free in vgem_gem_dumb_create+0x238/0x250 
drivers/gpu/drm/vgem/vgem_drv.c:221
Read of size 8 at addr 88809a2ee908 by task syz-executor815/10244

CPU: 1 PID: 10244 Comm: syz-executor815 Not tainted 5.5.0-syzkaller #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 
01/01/2011
Call Trace:
 __dump_stack lib/dump_stack.c:77 [inline]
 dump_stack+0x197/0x210 lib/dump_stack.c:118
 print_address_description.constprop.0.cold+0xd4/0x30b mm/kasan/report.c:374
 __kasan_report.cold+0x1b/0x32 mm/kasan/report.c:506
 kasan_report+0x12/0x20 mm/kasan/common.c:641
 __asan_report_load8_noabort+0x14/0x20 mm/kasan/generic_report.c:135
 vgem_gem_dumb_create+0x238/0x250 drivers/gpu/drm/vgem/vgem_drv.c:221
 drm_mode_create_dumb+0x282/0x310 drivers/gpu/drm/drm_dumb_buffers.c:94
 drm_mode_create_dumb_ioctl+0x26/0x30 drivers/gpu/drm/drm_dumb_buffers.c:100
 drm_ioctl_kernel+0x244/0x300 drivers/gpu/drm/drm_ioctl.c:786
 drm_ioctl+0x54e/0xa60 drivers/gpu/drm/drm_ioctl.c:886
 vfs_ioctl fs/ioctl.c:47 [inline]
 ksys_ioctl+0x123/0x180 fs/ioctl.c:747
 __do_sys_ioctl fs/ioctl.c:756 [inline]
 __se_sys_ioctl fs/ioctl.c:754 [inline]
 __x64_sys_ioctl+0x73/0xb0 fs/ioctl.c:754
 do_syscall_64+0xfa/0x790 arch/x86/entry/common.c:294
 entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x44c3e9
Code: e8 8c e7 ff ff 48 83 c4 18 c3 0f 1f 80 00 00 00 00 48 89 f8 48 89 f7 48 
89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 0f 83 
1b cc fb ff c3 66 2e 0f 1f 84 00 00 00 00
RSP: 002b:7f40263e9db8 EFLAGS: 0246 ORIG_RAX: 0010
RAX: ffda RBX: 006ddc58 RCX: 0044c3e9
RDX: 2000 RSI: c02064b2 RDI: 0004
RBP: 006ddc50 R08:  R09: 
R10:  R11: 0246 R12: 006ddc5c
R13: 7ffed0d4362f R14: 7f40263ea9c0 R15: 006ddc5c

Allocated by task 10244:
 save_stack+0x23/0x90 mm/kasan/common.c:72
 set_track mm/kasan/common.c:80 [inline]
 __kasan_kmalloc mm/kasan/common.c:515 [inline]
 __kasan_kmalloc.constprop.0+0xcf/0xe0 mm/kasan/common.c:488
 kasan_kmalloc+0x9/0x10 mm/kasan/common.c:529
 kmem_cache_alloc_trace+0x158/0x790 mm/slab.c:3551
 kmalloc include/linux/slab.h:556 [inline]
 kzalloc include/linux/slab.h:670 [inline]
 __vgem_gem_create+0x49/0x100 drivers/gpu/drm/vgem/vgem_drv.c:165
 vgem_gem_create drivers/gpu/drm/vgem/vgem_drv.c:194 [inline]
 vgem_gem_dumb_create+0xd7/0x250 drivers/gpu/drm/vgem/vgem_drv.c:217
 drm_mode_create_dumb+0x282/0x310 drivers/gpu/drm/drm_dumb_buffers.c:94
 drm_mode_create_dumb_ioctl+0x26/0x30 drivers/gpu/drm/drm_dumb_buffers.c:100
 drm_ioctl_kernel+0x244/0x300 drivers/gpu/drm/drm_ioctl.c:786
 drm_ioctl+0x54e/0xa60 drivers/gpu/drm/drm_ioctl.c:886
 vfs_ioctl fs/ioctl.c:47 [inline]
 ksys_ioctl+0x123/0x180 fs/ioctl.c:747
 __do_sys_ioctl fs/ioctl.c:756 [inline]
 __se_sys_ioctl fs/ioctl.c:754 [inline]
 __x64_sys_ioctl+0x73/0xb0 fs/ioctl.c:754
 do_syscall_64+0xfa/0x790 arch/x86/entry/common.c:294
 entry_SYSCALL_64_after_hwframe+0x49/0xbe

Freed by task 10244:
 save_stack+0x23/0x90 mm/kasan/common.c:72
 set_track mm/kasan/common.c:80 [inline]
 kasan_set_free_info mm/kasan/common.c:337 [inline]
 __kasan_slab_free+0x102/0x150 mm/kasan/common.c:476
 kasan_slab_free+0xe/0x10 mm/kasan/common.c:485
 __cache_free mm/slab.c:3426 [inline]
 kfree+0x10a/0x2c0 mm/slab.c:3757
 vgem_gem_free_object+0xbe/0xe0 drivers/gpu/drm/vgem/vgem_drv.c:68
 drm_gem_object_free+0x100/0x220 drivers/gpu/drm/drm_gem.c:983
 kref_put include/linux/kref.h:65 [inline]
 drm_gem_object_put_unlocked drivers/gpu/drm/drm_gem.c:1017 [inline]