[PATCH] drm/amdgpu: Fix 'fw_name' buffer size to prevent truncations in amdgpu_mes_init_microcode

2024-03-20 Thread Srinivasan Shanmugam
The snprintf function is used to write a formatted string into fw_name.
The format of the string is "amdgpu/%s_mes%s.bin", where %s is replaced
by the string in ucode_prefix and the second %s is replaced by either
"_2" or "1" depending on the condition pipe == AMDGPU_MES_SCHED_PIPE.

The length of the string "amdgpu/%s_mes%s.bin" is 16 characters plus the
length of ucode_prefix and the length of the string "_2" or "1". The
size of ucode_prefix is 30, so the maximum length of ucode_prefix is 29
characters (since one character is needed for the null terminator).
Therefore, the maximum possible length of the string written into
fw_name is 16 + 29 + 2 = 47 characters.

The size of fw_name is 40, so if the length of the string written into
fw_name is more than 39 characters (since one character is needed for
the null terminator), it will be truncated by the snprintf function, and
thus warnings will be seen.

By increasing the size of fw_name to 50, we ensure that fw_name is
large enough to hold the maximum possible length of the string, so the
snprintf function will not truncate the output.

Fixes the below with gcc W=1:
drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c: In function 
‘amdgpu_mes_init_microcode’:
drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c:1482:66: warning: ‘%s’ directive output 
may be truncated writing up to 1 bytes into a region of size between 0 and 29 
[-Wformat-truncation=]
 1482 | snprintf(fw_name, sizeof(fw_name), 
"amdgpu/%s_mes%s.bin",
  |  ^~
drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c:1482:17: note: ‘snprintf’ output 
between 16 and 46 bytes into a destination of size 40
 1482 | snprintf(fw_name, sizeof(fw_name), 
"amdgpu/%s_mes%s.bin",
  | 
^
 1483 |  ucode_prefix,
  |  ~
 1484 |  pipe == AMDGPU_MES_SCHED_PIPE ? "" : "1");
  |  ~
drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c:1477:66: warning: ‘%s’ directive output 
may be truncated writing 1 byte into a region of size between 0 and 29 
[-Wformat-truncation=]
 1477 | snprintf(fw_name, sizeof(fw_name), 
"amdgpu/%s_mes%s.bin",
  |  ^~
 1478 |  ucode_prefix,
 1479 |  pipe == AMDGPU_MES_SCHED_PIPE ? "_2" : "1");
  | ~~~
drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c:1477:17: note: ‘snprintf’ output 
between 17 and 46 bytes into a destination of size 40
 1477 | snprintf(fw_name, sizeof(fw_name), 
"amdgpu/%s_mes%s.bin",
  | 
^
 1478 |  ucode_prefix,
  |  ~
 1479 |  pipe == AMDGPU_MES_SCHED_PIPE ? "_2" : "1");
  |  ~~~
drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c:1477:66: warning: ‘%s’ directive output 
may be truncated writing 2 bytes into a region of size between 0 and 29 
[-Wformat-truncation=]
 1477 | snprintf(fw_name, sizeof(fw_name), 
"amdgpu/%s_mes%s.bin",
  |  ^~
 1478 |  ucode_prefix,
 1479 |  pipe == AMDGPU_MES_SCHED_PIPE ? "_2" : "1");
  |  
drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c:1477:17: note: ‘snprintf’ output 
between 18 and 47 bytes into a destination of size 40
 1477 | snprintf(fw_name, sizeof(fw_name), 
"amdgpu/%s_mes%s.bin",
  | 
^
 1478 |  ucode_prefix,
  |  ~
 1479 |  pipe == AMDGPU_MES_SCHED_PIPE ? "_2" : "1");
  |  ~~~
drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c:1489:62: warning: ‘_mes.bin’ directive 
output may be truncated writing 8 bytes into a region of size between 4 and 33 
[-Wformat-truncation=]
 1489 | snprintf(fw_name, sizeof(fw_name), "amdgpu/%s_mes.bin",
  |  ^~~~
drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c:1489:17: note: ‘snprintf’ output 
between 16 and 45 bytes into a destination of size 40
 1489 | snprintf(fw_name, sizeof(fw_name), "amdgpu/%s_mes.bin",
  | ^~~
 1490 |  ucode_prefix);
  |  ~

Cc: Alex Deucher 
Cc: 

[PATCH v2] drm/amdgpu: Fix format character cut-off issues in amdgpu_vcn_early_init()

2024-03-20 Thread Srinivasan Shanmugam
Reducing the size of ucode_prefix to 25 in the amdgpu_vcn_early_init
function. This would ensure that the total number of characters being
written into fw_name does not exceed its size of 40.

Fixes the below with gcc W=1:
drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c: In function ‘amdgpu_vcn_early_init’:
drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c:102:66: warning: ‘snprintf’ output may 
be truncated before the last format character [-Wformat-truncation=]
  102 | snprintf(fw_name, sizeof(fw_name), "amdgpu/%s.bin", 
ucode_prefix);
  |  ^
drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c:102:17: note: ‘snprintf’ output between 
12 and 41 bytes into a destination of size 40
  102 | snprintf(fw_name, sizeof(fw_name), "amdgpu/%s.bin", 
ucode_prefix);
  | 
^
drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c:102:66: warning: ‘snprintf’ output may 
be truncated before the last format character [-Wformat-truncation=]
  102 | snprintf(fw_name, sizeof(fw_name), "amdgpu/%s.bin", 
ucode_prefix);
  |  ^
drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c:102:17: note: ‘snprintf’ output between 
12 and 41 bytes into a destination of size 40
  102 | snprintf(fw_name, sizeof(fw_name), "amdgpu/%s.bin", 
ucode_prefix);
  | 
^
drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c:105:73: warning: ‘.bin’ directive 
output may be truncated writing 4 bytes into a region of size between 2 and 31 
[-Wformat-truncation=]
  105 | snprintf(fw_name, sizeof(fw_name), 
"amdgpu/%s_%d.bin", ucode_prefix, i);
  | 
^~~~
drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c:105:25: note: ‘snprintf’ output between 
14 and 43 bytes into a destination of size 40
  105 | snprintf(fw_name, sizeof(fw_name), 
"amdgpu/%s_%d.bin", ucode_prefix, i);
  | 
^~~

Cc: Alex Deucher 
Cc: Christian König 
Suggested-by: Lijo Lazar 
Signed-off-by: Srinivasan Shanmugam 
---
v2:
   - Reduced ucode_prefix instead of changing fw_name (Lijo)
   - updated commit message

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

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
index 9c514a606a2f..bb85772b1374 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
@@ -93,7 +93,7 @@ static void amdgpu_vcn_idle_work_handler(struct work_struct 
*work);
 
 int amdgpu_vcn_early_init(struct amdgpu_device *adev)
 {
-   char ucode_prefix[30];
+   char ucode_prefix[25];
char fw_name[40];
int r, i;
 
-- 
2.34.1



[PATCH v2] drm/amdgpu: refactor code to split devcoredump code

2024-03-20 Thread Sunil Khatri
Refractor devcoredump code into new files since its
functionality is expanded further and better to slit
and devcoredump to have its own file.

v2: Fix the build failure caught by arm compiler
of implicit function declaration with #ifdef

Cc: Ivan Lipski 
Signed-off-by: Sunil Khatri 
---
 drivers/gpu/drm/amd/amdgpu/Makefile   |   2 +-
 .../gpu/drm/amd/amdgpu/amdgpu_dev_coredump.c  | 218 ++
 .../gpu/drm/amd/amdgpu/amdgpu_dev_coredump.h  |  47 
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c|   4 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_reset.c | 191 ---
 drivers/gpu/drm/amd/amdgpu/amdgpu_reset.h |  16 --
 6 files changed, 270 insertions(+), 208 deletions(-)
 create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_dev_coredump.c
 create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_dev_coredump.h

diff --git a/drivers/gpu/drm/amd/amdgpu/Makefile 
b/drivers/gpu/drm/amd/amdgpu/Makefile
index 535e3936cfe0..1f6b56ec99f6 100644
--- a/drivers/gpu/drm/amd/amdgpu/Makefile
+++ b/drivers/gpu/drm/amd/amdgpu/Makefile
@@ -81,7 +81,7 @@ amdgpu-y += amdgpu_device.o amdgpu_doorbell_mgr.o 
amdgpu_kms.o \
amdgpu_umc.o smu_v11_0_i2c.o amdgpu_fru_eeprom.o amdgpu_rap.o \
amdgpu_fw_attestation.o amdgpu_securedisplay.o \
amdgpu_eeprom.o amdgpu_mca.o amdgpu_psp_ta.o amdgpu_lsdma.o \
-   amdgpu_ring_mux.o amdgpu_xcp.o amdgpu_seq64.o amdgpu_aca.o
+   amdgpu_ring_mux.o amdgpu_xcp.o amdgpu_seq64.o amdgpu_aca.o 
amdgpu_dev_coredump.o
 
 amdgpu-$(CONFIG_PROC_FS) += amdgpu_fdinfo.o
 
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_dev_coredump.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_dev_coredump.c
new file mode 100644
index ..f3a0f5857598
--- /dev/null
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_dev_coredump.c
@@ -0,0 +1,218 @@
+// SPDX-License-Identifier: MIT
+/*
+ * Copyright 2024 Advanced Micro Devices, Inc.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR
+ * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
+ * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ * OTHER DEALINGS IN THE SOFTWARE.
+ *
+ */
+
+#include 
+
+#ifndef CONFIG_DEV_COREDUMP
+void amdgpu_coredump(struct amdgpu_device *adev, bool vram_lost,
+struct amdgpu_reset_context *reset_context)
+{
+}
+#else
+
+#include 
+#include "amdgpu_dev_coredump.h"
+
+const char *hw_ip_names[MAX_HWIP] = {
+   [GC_HWIP]   = "GC",
+   [HDP_HWIP]  = "HDP",
+   [SDMA0_HWIP]= "SDMA0",
+   [SDMA1_HWIP]= "SDMA1",
+   [SDMA2_HWIP]= "SDMA2",
+   [SDMA3_HWIP]= "SDMA3",
+   [SDMA4_HWIP]= "SDMA4",
+   [SDMA5_HWIP]= "SDMA5",
+   [SDMA6_HWIP]= "SDMA6",
+   [SDMA7_HWIP]= "SDMA7",
+   [LSDMA_HWIP]= "LSDMA",
+   [MMHUB_HWIP]= "MMHUB",
+   [ATHUB_HWIP]= "ATHUB",
+   [NBIO_HWIP] = "NBIO",
+   [MP0_HWIP]  = "MP0",
+   [MP1_HWIP]  = "MP1",
+   [UVD_HWIP]  = "UVD/JPEG/VCN",
+   [VCN1_HWIP] = "VCN1",
+   [VCE_HWIP]  = "VCE",
+   [VPE_HWIP]  = "VPE",
+   [DF_HWIP]   = "DF",
+   [DCE_HWIP]  = "DCE",
+   [OSSSYS_HWIP]   = "OSSSYS",
+   [SMUIO_HWIP]= "SMUIO",
+   [PWR_HWIP]  = "PWR",
+   [NBIF_HWIP] = "NBIF",
+   [THM_HWIP]  = "THM",
+   [CLK_HWIP]  = "CLK",
+   [UMC_HWIP]  = "UMC",
+   [RSMU_HWIP] = "RSMU",
+   [XGMI_HWIP] = "XGMI",
+   [DCI_HWIP]  = "DCI",
+   [PCIE_HWIP] = "PCIE",
+};
+
+static ssize_t
+amdgpu_devcoredump_read(char *buffer, loff_t offset, size_t count,
+   void *data, size_t datalen)
+{
+   struct drm_printer p;
+   struct amdgpu_coredump_info *coredump = data;
+   struct drm_print_iterator iter;
+   struct amdgpu_vm_fault_info *fault_info;
+   int i, 

[PATCH 1/2] drm/amdgpu: add socket id parameter for psp query address cmd

2024-03-20 Thread Tao Zhou
And set the socket id.

Signed-off-by: Tao Zhou 
---
 drivers/gpu/drm/amd/amdgpu/ta_ras_if.h |  1 +
 drivers/gpu/drm/amd/amdgpu/umc_v12_0.c | 14 +++---
 2 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/ta_ras_if.h 
b/drivers/gpu/drm/amd/amdgpu/ta_ras_if.h
index 056d4df8fa1f..3ac56a9645eb 100644
--- a/drivers/gpu/drm/amd/amdgpu/ta_ras_if.h
+++ b/drivers/gpu/drm/amd/amdgpu/ta_ras_if.h
@@ -146,6 +146,7 @@ struct ta_ras_mca_addr {
uint32_t ch_inst;
uint32_t umc_inst;
uint32_t node_inst;
+   uint32_t socket_id;
 };
 
 struct ta_ras_phy_addr {
diff --git a/drivers/gpu/drm/amd/amdgpu/umc_v12_0.c 
b/drivers/gpu/drm/amd/amdgpu/umc_v12_0.c
index 77af4e25ff46..0a9cc87e98d0 100644
--- a/drivers/gpu/drm/amd/amdgpu/umc_v12_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/umc_v12_0.c
@@ -268,7 +268,7 @@ static void umc_v12_0_mca_addr_to_pa(struct amdgpu_device 
*adev,
 static void umc_v12_0_convert_error_address(struct amdgpu_device *adev,
struct ras_err_data *err_data, 
uint64_t err_addr,
uint32_t ch_inst, uint32_t umc_inst,
-   uint32_t node_inst)
+   uint32_t node_inst, uint32_t 
socket_id)
 {
uint32_t col, row, row_xor, bank, channel_index;
uint64_t soc_pa, retired_page, column;
@@ -280,6 +280,7 @@ static void umc_v12_0_convert_error_address(struct 
amdgpu_device *adev,
addr_in.ma.ch_inst = ch_inst;
addr_in.ma.umc_inst = umc_inst;
addr_in.ma.node_inst = node_inst;
+   addr_in.ma.socket_id = socket_id;
 
if (psp_ras_query_address(>psp, _in, _out))
/* fallback to old path if fail to get pa from psp */
@@ -331,6 +332,7 @@ static int umc_v12_0_query_error_address(struct 
amdgpu_device *adev,
struct ras_err_data *err_data = (struct ras_err_data *)data;
uint64_t umc_reg_offset =
get_umc_v12_0_reg_offset(adev, node_inst, umc_inst, ch_inst);
+   uint32_t socket_id = 0;
 
mc_umc_status_addr =
SOC15_REG_OFFSET(UMC, 0, regMCA_UMC_UMC0_MCUMC_STATUST0);
@@ -357,8 +359,13 @@ static int umc_v12_0_query_error_address(struct 
amdgpu_device *adev,
 
err_addr = REG_GET_FIELD(err_addr, MCA_UMC_UMC0_MCUMC_ADDRT0, 
ErrorAddr);
 
+   if (!adev->aid_mask &&
+   adev->smuio.funcs &&
+   adev->smuio.funcs->get_socket_id)
+   socket_id = adev->smuio.funcs->get_socket_id(adev);
+
umc_v12_0_convert_error_address(adev, err_data, err_addr,
-   ch_inst, umc_inst, node_inst);
+   ch_inst, umc_inst, node_inst, 
socket_id);
}
 
/* clear umc status */
@@ -450,7 +457,8 @@ static void 
umc_v12_0_ecc_info_query_ras_error_address(struct amdgpu_device *ade
err_data, err_addr,
MCA_IPID_LO_2_UMC_CH(InstanceIdLo),
MCA_IPID_LO_2_UMC_INST(InstanceIdLo),
-   err_info->mcm_info.die_id);
+   err_info->mcm_info.die_id,
+   err_info->mcm_info.socket_id);
}
 
/* Delete error address node from list and free memory 
*/
-- 
2.34.1



[PATCH 2/2] drm/amdgpu: simplify convert_error_address interface for UMC v12

2024-03-20 Thread Tao Zhou
Replace separate parameters with struct ta_ras_query_address_input.

Signed-off-by: Tao Zhou 
---
 drivers/gpu/drm/amd/amdgpu/umc_v12_0.c | 57 ++
 1 file changed, 30 insertions(+), 27 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/umc_v12_0.c 
b/drivers/gpu/drm/amd/amdgpu/umc_v12_0.c
index 0a9cc87e98d0..d0fcfcb3404f 100644
--- a/drivers/gpu/drm/amd/amdgpu/umc_v12_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/umc_v12_0.c
@@ -266,26 +266,19 @@ static void umc_v12_0_mca_addr_to_pa(struct amdgpu_device 
*adev,
 }
 
 static void umc_v12_0_convert_error_address(struct amdgpu_device *adev,
-   struct ras_err_data *err_data, 
uint64_t err_addr,
-   uint32_t ch_inst, uint32_t umc_inst,
-   uint32_t node_inst, uint32_t 
socket_id)
+   struct ras_err_data *err_data,
+   struct ta_ras_query_address_input 
*addr_in)
 {
uint32_t col, row, row_xor, bank, channel_index;
-   uint64_t soc_pa, retired_page, column;
-   struct ta_ras_query_address_input addr_in;
+   uint64_t soc_pa, retired_page, column, err_addr;
struct ta_ras_query_address_output addr_out;
 
-   addr_in.addr_type = TA_RAS_MCA_TO_PA;
-   addr_in.ma.err_addr = err_addr;
-   addr_in.ma.ch_inst = ch_inst;
-   addr_in.ma.umc_inst = umc_inst;
-   addr_in.ma.node_inst = node_inst;
-   addr_in.ma.socket_id = socket_id;
-
-   if (psp_ras_query_address(>psp, _in, _out))
+   err_addr = addr_in->ma.err_addr;
+   addr_in->addr_type = TA_RAS_MCA_TO_PA;
+   if (psp_ras_query_address(>psp, addr_in, _out))
/* fallback to old path if fail to get pa from psp */
-   umc_v12_0_mca_addr_to_pa(adev, err_addr, ch_inst, umc_inst,
-   node_inst, _out);
+   umc_v12_0_mca_addr_to_pa(adev, err_addr, addr_in->ma.ch_inst,
+   addr_in->ma.umc_inst, addr_in->ma.node_inst, 
_out);
 
soc_pa = addr_out.pa.pa;
bank = addr_out.pa.bank;
@@ -310,7 +303,7 @@ static void umc_v12_0_convert_error_address(struct 
amdgpu_device *adev,
"Error Address(PA):0x%-10llx Row:0x%-4x Col:0x%-2x 
Bank:0x%x Channel:0x%x\n",
retired_page, row, col, bank, channel_index);
amdgpu_umc_fill_error_record(err_data, err_addr,
-   retired_page, channel_index, umc_inst);
+   retired_page, channel_index, addr_in->ma.umc_inst);
 
/* shift R13 bit */
retired_page ^= (0x1ULL << UMC_V12_0_PA_R13_BIT);
@@ -318,7 +311,7 @@ static void umc_v12_0_convert_error_address(struct 
amdgpu_device *adev,
"Error Address(PA):0x%-10llx Row:0x%-4x Col:0x%-2x 
Bank:0x%x Channel:0x%x\n",
retired_page, row_xor, col, bank, channel_index);
amdgpu_umc_fill_error_record(err_data, err_addr,
-   retired_page, channel_index, umc_inst);
+   retired_page, channel_index, addr_in->ma.umc_inst);
}
 }
 
@@ -326,13 +319,13 @@ static int umc_v12_0_query_error_address(struct 
amdgpu_device *adev,
uint32_t node_inst, uint32_t umc_inst,
uint32_t ch_inst, void *data)
 {
+   struct ras_err_data *err_data = (struct ras_err_data *)data;
+   struct ta_ras_query_address_input addr_in;
uint64_t mc_umc_status_addr;
uint64_t mc_umc_status, err_addr;
uint64_t mc_umc_addrt0;
-   struct ras_err_data *err_data = (struct ras_err_data *)data;
uint64_t umc_reg_offset =
get_umc_v12_0_reg_offset(adev, node_inst, umc_inst, ch_inst);
-   uint32_t socket_id = 0;
 
mc_umc_status_addr =
SOC15_REG_OFFSET(UMC, 0, regMCA_UMC_UMC0_MCUMC_STATUST0);
@@ -362,10 +355,16 @@ static int umc_v12_0_query_error_address(struct 
amdgpu_device *adev,
if (!adev->aid_mask &&
adev->smuio.funcs &&
adev->smuio.funcs->get_socket_id)
-   socket_id = adev->smuio.funcs->get_socket_id(adev);
+   addr_in.ma.socket_id = 
adev->smuio.funcs->get_socket_id(adev);
+   else
+   addr_in.ma.socket_id = 0;
+
+   addr_in.ma.err_addr = err_addr;
+   addr_in.ma.ch_inst = ch_inst;
+   addr_in.ma.umc_inst = umc_inst;
+   addr_in.ma.node_inst = node_inst;
 
-   umc_v12_0_convert_error_address(adev, err_data, err_addr,
-   ch_inst, umc_inst, node_inst, 
socket_id);
+   umc_v12_0_convert_error_address(adev, err_data, _in);
}
 
/* clear umc status */
@@ -425,12 +424,16 @@ static void 

RE: [PATCH] drm/amdkfd: fix TLB flush after unmap for GFX9.4.2

2024-03-20 Thread Kasiviswanathan, Harish
[AMD Official Use Only - General]

Reviewed-by: Harish Kasiviswanathan 

-Original Message-
From: amd-gfx  On Behalf Of Eric Huang
Sent: Wednesday, March 20, 2024 4:25 PM
To: amd-gfx@lists.freedesktop.org
Cc: Huang, JinHuiEric 
Subject: [PATCH] drm/amdkfd: fix TLB flush after unmap for GFX9.4.2

TLB flush after unmap accidentially was removed on
gfx9.4.2. It is to add it back.

Signed-off-by: Eric Huang 
---
 drivers/gpu/drm/amd/amdkfd/kfd_priv.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h 
b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
index 42d40560cd30..a81ef232fdef 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
@@ -1473,7 +1473,7 @@ static inline void kfd_flush_tlb(struct 
kfd_process_device *pdd,

 static inline bool kfd_flush_tlb_after_unmap(struct kfd_dev *dev)
 {
-   return KFD_GC_VERSION(dev) > IP_VERSION(9, 4, 2) ||
+   return KFD_GC_VERSION(dev) >= IP_VERSION(9, 4, 2) ||
   (KFD_GC_VERSION(dev) == IP_VERSION(9, 4, 1) && 
dev->sdma_fw_version >= 18) ||
   KFD_GC_VERSION(dev) == IP_VERSION(9, 4, 0);
 }
--
2.34.1



RE: [PATCH v2 1/9] drm/amd/pm: Add support for DPM policies

2024-03-20 Thread Wang, Yang(Kevin)
[AMD Official Use Only - General]

-Original Message-
From: amd-gfx  On Behalf Of Lijo Lazar
Sent: Thursday, March 14, 2024 7:56 PM
To: amd-gfx@lists.freedesktop.org
Cc: Zhang, Hawking ; Deucher, Alexander 
; Liu, Shuzhou (Bill) 
Subject: [PATCH v2 1/9] drm/amd/pm: Add support for DPM policies

Add support to set/get information about different DPM policies. The support is 
only available on SOCs which use swsmu architecture.

A DPM policy type may be defined with different levels. For example, a policy 
may be defined to select Pstate preference and then later a pstate preference 
may be chosen.

Signed-off-by: Lijo Lazar 
Reviewed-by: Hawking Zhang 
---
v2: Add NULL checks before accessing smu_dpm_policy_ctxt

 .../gpu/drm/amd/include/kgd_pp_interface.h| 16 
 drivers/gpu/drm/amd/pm/amdgpu_dpm.c   | 29 ++
 drivers/gpu/drm/amd/pm/amdgpu_pm.c| 92 ++
 drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h   |  4 +
 drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c | 95 +++
 drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h | 29 ++
 6 files changed, 265 insertions(+)

diff --git a/drivers/gpu/drm/amd/include/kgd_pp_interface.h 
b/drivers/gpu/drm/amd/include/kgd_pp_interface.h
index afb930b70615..84dd819ccc06 100644
--- a/drivers/gpu/drm/amd/include/kgd_pp_interface.h
+++ b/drivers/gpu/drm/amd/include/kgd_pp_interface.h
@@ -273,6 +273,22 @@ enum pp_xgmi_plpd_mode {
XGMI_PLPD_COUNT,
 };

+enum pp_pm_policy {
+   PP_PM_POLICY_NONE = -1,
+   PP_PM_POLICY_SOC_PSTATE = 0,
+   PP_PM_POLICY_NUM,
+};
+
+enum pp_policy_soc_pstate {
+   SOC_PSTATE_DEFAULT = 0,
+   SOC_PSTATE_0,
+   SOC_PSTATE_1,
+   SOC_PSTATE_2,
+   SOC_PSTAT_COUNT,
+};
+
+#define PP_POLICY_MAX_LEVELS 5
+
 #define PP_GROUP_MASK0xF000
 #define PP_GROUP_SHIFT   28

diff --git a/drivers/gpu/drm/amd/pm/amdgpu_dpm.c 
b/drivers/gpu/drm/amd/pm/amdgpu_dpm.c
index f84bfed50681..db3addd07120 100644
--- a/drivers/gpu/drm/amd/pm/amdgpu_dpm.c
+++ b/drivers/gpu/drm/amd/pm/amdgpu_dpm.c
@@ -411,6 +411,35 @@ int amdgpu_dpm_set_xgmi_plpd_mode(struct amdgpu_device 
*adev, int mode)
return ret;
 }

+ssize_t amdgpu_dpm_get_pm_policy_info(struct amdgpu_device *adev, char
+*buf) {
+   struct smu_context *smu = adev->powerplay.pp_handle;
+   int ret = -EOPNOTSUPP;
+
+   if (is_support_sw_smu(adev)) {
+   mutex_lock(>pm.mutex);
+   ret = smu_get_pm_policy_info(smu, buf);
+   mutex_unlock(>pm.mutex);
+   }
+
+   return ret;
+}
+
+int amdgpu_dpm_set_pm_policy(struct amdgpu_device *adev, int policy_type,
+int policy_level)
+{
+   struct smu_context *smu = adev->powerplay.pp_handle;
+   int ret = -EOPNOTSUPP;
+
+   if (is_support_sw_smu(adev)) {
+   mutex_lock(>pm.mutex);
+   ret = smu_set_pm_policy(smu, policy_type, policy_level);
+   mutex_unlock(>pm.mutex);
+   }
+
+   return ret;
+}
+
 int amdgpu_dpm_enable_mgpu_fan_boost(struct amdgpu_device *adev)  {
void *pp_handle = adev->powerplay.pp_handle; diff --git 
a/drivers/gpu/drm/amd/pm/amdgpu_pm.c b/drivers/gpu/drm/amd/pm/amdgpu_pm.c
index efc631bddf4a..7ee11c2e3c61 100644
--- a/drivers/gpu/drm/amd/pm/amdgpu_pm.c
+++ b/drivers/gpu/drm/amd/pm/amdgpu_pm.c
@@ -2179,6 +2179,96 @@ static ssize_t amdgpu_set_xgmi_plpd_policy(struct device 
*dev,
return count;
 }

+static ssize_t amdgpu_get_pm_policy(struct device *dev,
+   struct device_attribute *attr, char *buf) {
+   struct drm_device *ddev = dev_get_drvdata(dev);
+   struct amdgpu_device *adev = drm_to_adev(ddev);
+
+   if (amdgpu_in_reset(adev))
+   return -EPERM;
+   if (adev->in_suspend && !adev->in_runpm)
+   return -EPERM;
+
+   return amdgpu_dpm_get_pm_policy_info(adev, buf); }
+
+static ssize_t amdgpu_set_pm_policy(struct device *dev,
+   struct device_attribute *attr,
+   const char *buf, size_t count)
+{
+   struct drm_device *ddev = dev_get_drvdata(dev);
+   struct amdgpu_device *adev = drm_to_adev(ddev);
+   int policy_type, ret, num_params = 0;
+   char delimiter[] = " \n\t";
+   char tmp_buf[128];
+   char *tmp, *param;
+   long val;
+
+   if (amdgpu_in_reset(adev))
+   return -EPERM;
+   if (adev->in_suspend && !adev->in_runpm)
+   return -EPERM;
+
+   count = min(count, sizeof(tmp_buf));
+   memcpy(tmp_buf, buf, count);
+   tmp_buf[count - 1] = '\0';
+   tmp = tmp_buf;
+
+   tmp = skip_spaces(tmp);
+   if (strncmp(tmp, "soc_pstate", strlen("soc_pstate")) == 0) {

[Kevin]:
It is better to use 'sizeof() - 1' instead of strlen() function to calculate 
const string size.

Best Regards,
Kevin
+   policy_type = PP_PM_POLICY_SOC_PSTATE;
+   tmp += 

RE: [PATCH] drm/amdkfd: Check cgroup when returning DMABuf info

2024-03-20 Thread Joshi, Mukul
[AMD Official Use Only - General]

> -Original Message-
> From: amd-gfx  On Behalf Of Wu,
> David
> Sent: Wednesday, March 20, 2024 8:02 PM
> To: amd-gfx@lists.freedesktop.org
> Subject: Re: [PATCH] drm/amdkfd: Check cgroup when returning DMABuf info
>
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
>
>
> On 3/20/2024 4:21 PM, Felix Kuehling wrote:
> > On 2024-03-18 16:12, Felix Kuehling wrote:
> >>
> >> On 2024-03-15 14:17, Mukul Joshi wrote:
> >>> Check cgroup permissions when returning DMA-buf info and based on
> >>> cgroup check return the id of the GPU that has access to the BO.
> >>>
> >>> Signed-off-by: Mukul Joshi 
> >>> ---
> >>>   drivers/gpu/drm/amd/amdkfd/kfd_chardev.c | 4 ++--
> >>>   1 file changed, 2 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> >>> b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> >>> index dfa8c69532d4..f9631f4b1a02 100644
> >>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> >>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> >>> @@ -1523,7 +1523,7 @@ static int kfd_ioctl_get_dmabuf_info(struct
> >>> file *filep,
> >>> /* Find a KFD GPU device that supports the get_dmabuf_info
> >>> query */
> >>>   for (i = 0; kfd_topology_enum_kfd_devices(i, ) == 0; i++)
> >>> -if (dev)
> >>> +if (dev && !kfd_devcgroup_check_permission(dev))
> >>>   break;
> >>>   if (!dev)
> >>>   return -EINVAL;
> >>> @@ -1545,7 +1545,7 @@ static int kfd_ioctl_get_dmabuf_info(struct
> >>> file *filep,
> >>>   if (xcp_id >= 0)
> >>>   args->gpu_id = dmabuf_adev->kfd.dev->nodes[xcp_id]->id;
> >>>   else
> >>> -args->gpu_id = dmabuf_adev->kfd.dev->nodes[0]->id;
> >>> +args->gpu_id = dev->id;
> >>
> >> If I remember correctly, this was meant as a fallback in case for GTT
> >> BOs where the exporting partition wasn't known and the application
> >> didn't have access to the first partition. I think the way you wrote
> >> this, it could also change the behaviour (report the wrong GPU ID) on
> >> single-partition GPUs, which is probably not intended.
> >
> > Never mind. I double checked: On single-partition GPUs, bo->xcp_id
> > always seems to be 0. So your code won't change the behaviour here.
> > The patch is
> should bo->xcp_id be >= 0 for all cases? if yes then I think all tests can be
> moved. (like below)
> args->gpu_id = dmabuf_adev->kfd.dev->nodes[xcp_id]->id;
>
No it can be -1 on GPUs that support spatial partitioning, especially for GTT 
BOs.

Regards,
Mukul

> David
> >
> > Reviewed-by: Felix Kuehling 
> >
> >
> >
> >> Maybe this would preserve the behaviour for that case:
> >>
> >> ...
> >> -else
> >> +else if
> >> (!kfd_devcgroup_check_permission(dmabuf_adev->kfd.dev->nodes[0]))
> >>  args->gpu_id = dmabuf_adev->kfd.dev->nodes[0]->id;
> >> +else
> >> +args->gpu_id = dev->id;
> >>
> >> Or maybe a more general solution would make DMABuf import work when
> >> the exporter is really unknown or not even a GPU. This came up not so
> >> long ago in the context of interop with 3rd-party devices. This may
> >> require user mode changes as well.
> >>
> >> Regards,
> >>   Felix
> >>
> >>
> >>>   args->flags = flags;
> >>> /* Copy metadata buffer to user mode */



Re: [PATCH] drm/amdkfd: Check cgroup when returning DMABuf info

2024-03-20 Thread Wu, David




On 3/20/2024 4:21 PM, Felix Kuehling wrote:

On 2024-03-18 16:12, Felix Kuehling wrote:


On 2024-03-15 14:17, Mukul Joshi wrote:

Check cgroup permissions when returning DMA-buf info and
based on cgroup check return the id of the GPU that has
access to the BO.

Signed-off-by: Mukul Joshi 
---
  drivers/gpu/drm/amd/amdkfd/kfd_chardev.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c

index dfa8c69532d4..f9631f4b1a02 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
@@ -1523,7 +1523,7 @@ static int kfd_ioctl_get_dmabuf_info(struct 
file *filep,
    /* Find a KFD GPU device that supports the get_dmabuf_info 
query */

  for (i = 0; kfd_topology_enum_kfd_devices(i, ) == 0; i++)
-    if (dev)
+    if (dev && !kfd_devcgroup_check_permission(dev))
  break;
  if (!dev)
  return -EINVAL;
@@ -1545,7 +1545,7 @@ static int kfd_ioctl_get_dmabuf_info(struct 
file *filep,

  if (xcp_id >= 0)
  args->gpu_id = dmabuf_adev->kfd.dev->nodes[xcp_id]->id;
  else
-    args->gpu_id = dmabuf_adev->kfd.dev->nodes[0]->id;
+    args->gpu_id = dev->id;


If I remember correctly, this was meant as a fallback in case for GTT 
BOs where the exporting partition wasn't known and the application 
didn't have access to the first partition. I think the way you wrote 
this, it could also change the behaviour (report the wrong GPU ID) on 
single-partition GPUs, which is probably not intended.


Never mind. I double checked: On single-partition GPUs, bo->xcp_id 
always seems to be 0. So your code won't change the behaviour here. 
The patch is
should bo->xcp_id be >= 0 for all cases? if yes then I think all tests 
can be moved. (like below)

args->gpu_id = dmabuf_adev->kfd.dev->nodes[xcp_id]->id;

David


Reviewed-by: Felix Kuehling 




Maybe this would preserve the behaviour for that case:

...
-    else
+    else if 
(!kfd_devcgroup_check_permission(dmabuf_adev->kfd.dev->nodes[0]))

 args->gpu_id = dmabuf_adev->kfd.dev->nodes[0]->id;
+    else
+    args->gpu_id = dev->id;

Or maybe a more general solution would make DMABuf import work when 
the exporter is really unknown or not even a GPU. This came up not so 
long ago in the context of interop with 3rd-party devices. This may 
require user mode changes as well.


Regards,
  Felix



  args->flags = flags;
    /* Copy metadata buffer to user mode */




[PATCH] drm/amdkfd: Cleanup workqueue during module unload

2024-03-20 Thread Mukul Joshi
Destroy the high priority workqueue that handles interrupts
during KFD node cleanup.

Signed-off-by: Mukul Joshi 
---
 drivers/gpu/drm/amd/amdkfd/kfd_interrupt.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_interrupt.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_interrupt.c
index dd3c43c1ad70..9b6b6e882593 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_interrupt.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_interrupt.c
@@ -104,6 +104,8 @@ void kfd_interrupt_exit(struct kfd_node *node)
 */
flush_workqueue(node->ih_wq);
 
+   destroy_workqueue(node->ih_wq);
+
kfifo_free(>ih_fifo);
 }
 
-- 
2.35.1



[PATCH] drm/amdkfd: fix TLB flush after unmap for GFX9.4.2

2024-03-20 Thread Eric Huang
TLB flush after unmap accidentially was removed on
gfx9.4.2. It is to add it back.

Signed-off-by: Eric Huang 
---
 drivers/gpu/drm/amd/amdkfd/kfd_priv.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h 
b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
index 42d40560cd30..a81ef232fdef 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
@@ -1473,7 +1473,7 @@ static inline void kfd_flush_tlb(struct 
kfd_process_device *pdd,
 
 static inline bool kfd_flush_tlb_after_unmap(struct kfd_dev *dev)
 {
-   return KFD_GC_VERSION(dev) > IP_VERSION(9, 4, 2) ||
+   return KFD_GC_VERSION(dev) >= IP_VERSION(9, 4, 2) ||
   (KFD_GC_VERSION(dev) == IP_VERSION(9, 4, 1) && 
dev->sdma_fw_version >= 18) ||
   KFD_GC_VERSION(dev) == IP_VERSION(9, 4, 0);
 }
-- 
2.34.1



Re: [PATCH] drm/amdkfd: Check cgroup when returning DMABuf info

2024-03-20 Thread Felix Kuehling

On 2024-03-18 16:12, Felix Kuehling wrote:


On 2024-03-15 14:17, Mukul Joshi wrote:

Check cgroup permissions when returning DMA-buf info and
based on cgroup check return the id of the GPU that has
access to the BO.

Signed-off-by: Mukul Joshi 
---
  drivers/gpu/drm/amd/amdkfd/kfd_chardev.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c

index dfa8c69532d4..f9631f4b1a02 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
@@ -1523,7 +1523,7 @@ static int kfd_ioctl_get_dmabuf_info(struct 
file *filep,
    /* Find a KFD GPU device that supports the get_dmabuf_info 
query */

  for (i = 0; kfd_topology_enum_kfd_devices(i, ) == 0; i++)
-    if (dev)
+    if (dev && !kfd_devcgroup_check_permission(dev))
  break;
  if (!dev)
  return -EINVAL;
@@ -1545,7 +1545,7 @@ static int kfd_ioctl_get_dmabuf_info(struct 
file *filep,

  if (xcp_id >= 0)
  args->gpu_id = dmabuf_adev->kfd.dev->nodes[xcp_id]->id;
  else
-    args->gpu_id = dmabuf_adev->kfd.dev->nodes[0]->id;
+    args->gpu_id = dev->id;


If I remember correctly, this was meant as a fallback in case for GTT 
BOs where the exporting partition wasn't known and the application 
didn't have access to the first partition. I think the way you wrote 
this, it could also change the behaviour (report the wrong GPU ID) on 
single-partition GPUs, which is probably not intended.


Never mind. I double checked: On single-partition GPUs, bo->xcp_id 
always seems to be 0. So your code won't change the behaviour here. The 
patch is


Reviewed-by: Felix Kuehling 




Maybe this would preserve the behaviour for that case:

...
-    else
+    else if 
(!kfd_devcgroup_check_permission(dmabuf_adev->kfd.dev->nodes[0]))

 args->gpu_id = dmabuf_adev->kfd.dev->nodes[0]->id;
+    else
+    args->gpu_id = dev->id;

Or maybe a more general solution would make DMABuf import work when 
the exporter is really unknown or not even a GPU. This came up not so 
long ago in the context of interop with 3rd-party devices. This may 
require user mode changes as well.


Regards,
  Felix



  args->flags = flags;
    /* Copy metadata buffer to user mode */


Re: [PATCH] Revert "drm/amd/display: Fix sending VSC (+ colorimetry) packets for DP/eDP displays without PSR"

2024-03-20 Thread Rodrigo Siqueira Jordao




On 3/12/24 9:24 AM, Harry Wentland wrote:

This causes flicker on a bunch of eDP panels. The info_packet code
also caused regressions on other OSes that we haven't' seen on Linux
yet, but that is likely due to the fact that we haven't had a chance
to test those environments on Linux.

We'll need to revisit this.

This reverts commit bfd4e0b7eb4467f9db5bb37268565afec6cf513e.

Closes: https://gitlab.freedesktop.org/drm/amd/-/issues/3207
Closes: https://gitlab.freedesktop.org/drm/amd/-/issues/3151
Signed-off-by: Harry Wentland 
---
  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c   |  8 +++-
  .../amd/display/modules/info_packet/info_packet.c   | 13 +
  2 files changed, 8 insertions(+), 13 deletions(-)

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 cffb2655177c..6a61eb4148ad 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -6302,9 +6302,8 @@ create_stream_for_sink(struct drm_connector *connector,
  
  	if (stream->signal == SIGNAL_TYPE_HDMI_TYPE_A)

mod_build_hf_vsif_infopacket(stream, >vsp_infopacket);
-   else if (stream->signal == SIGNAL_TYPE_DISPLAY_PORT ||
-stream->signal == SIGNAL_TYPE_DISPLAY_PORT_MST ||
-stream->signal == SIGNAL_TYPE_EDP) {
+
+   if (stream->link->psr_settings.psr_feature_enabled || 
stream->link->replay_settings.replay_feature_enabled) {
//
// should decide stream support vsc sdp colorimetry capability
// before building vsc info packet
@@ -6320,9 +6319,8 @@ create_stream_for_sink(struct drm_connector *connector,
if (stream->out_transfer_func->tf == TRANSFER_FUNCTION_GAMMA22)
tf = TRANSFER_FUNC_GAMMA_22;
mod_build_vsc_infopacket(stream, >vsc_infopacket, 
stream->output_color_space, tf);
+   aconnector->psr_skip_count = AMDGPU_DM_PSR_ENTRY_DELAY;
  
-		if (stream->link->psr_settings.psr_feature_enabled)

-   aconnector->psr_skip_count = AMDGPU_DM_PSR_ENTRY_DELAY;
}
  finish:
dc_sink_release(sink);
diff --git a/drivers/gpu/drm/amd/display/modules/info_packet/info_packet.c 
b/drivers/gpu/drm/amd/display/modules/info_packet/info_packet.c
index 738ee763f24a..84f9b412a4f1 100644
--- a/drivers/gpu/drm/amd/display/modules/info_packet/info_packet.c
+++ b/drivers/gpu/drm/amd/display/modules/info_packet/info_packet.c
@@ -147,15 +147,12 @@ void mod_build_vsc_infopacket(const struct 
dc_stream_state *stream,
}
  
  	/* VSC packet set to 4 for PSR-SU, or 2 for PSR1 */

-   if (stream->link->psr_settings.psr_feature_enabled) {
-   if (stream->link->psr_settings.psr_version == 
DC_PSR_VERSION_SU_1)
-   vsc_packet_revision = vsc_packet_rev4;
-   else if (stream->link->psr_settings.psr_version == 
DC_PSR_VERSION_1)
-   vsc_packet_revision = vsc_packet_rev2;
-   }
-
-   if (stream->link->replay_settings.config.replay_supported)
+   if (stream->link->psr_settings.psr_version == DC_PSR_VERSION_SU_1)
vsc_packet_revision = vsc_packet_rev4;
+   else if (stream->link->replay_settings.config.replay_supported)
+   vsc_packet_revision = vsc_packet_rev4;
+   else if (stream->link->psr_settings.psr_version == DC_PSR_VERSION_1)
+   vsc_packet_revision = vsc_packet_rev2;
  
  	/* Update to revision 5 for extended colorimetry support */

if (stream->use_vsc_sdp_for_colorimetry)


Reviewed-by: Rodrigo Siqueira 



[PATCH] drm/amdgpu: refactor code to split devcoredump code

2024-03-20 Thread Sunil Khatri
Refractor devcoredump code into new files since its
functionality is expanded further and better to slit
and devcoredump to have its own file.

Signed-off-by: Sunil Khatri 
---
 drivers/gpu/drm/amd/amdgpu/Makefile   |   2 +-
 .../gpu/drm/amd/amdgpu/amdgpu_dev_coredump.c  | 218 ++
 .../gpu/drm/amd/amdgpu/amdgpu_dev_coredump.h  |  46 
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c|   4 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_reset.c | 191 ---
 drivers/gpu/drm/amd/amdgpu/amdgpu_reset.h |  16 --
 6 files changed, 269 insertions(+), 208 deletions(-)
 create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_dev_coredump.c
 create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_dev_coredump.h

diff --git a/drivers/gpu/drm/amd/amdgpu/Makefile 
b/drivers/gpu/drm/amd/amdgpu/Makefile
index 535e3936cfe0..1f6b56ec99f6 100644
--- a/drivers/gpu/drm/amd/amdgpu/Makefile
+++ b/drivers/gpu/drm/amd/amdgpu/Makefile
@@ -81,7 +81,7 @@ amdgpu-y += amdgpu_device.o amdgpu_doorbell_mgr.o 
amdgpu_kms.o \
amdgpu_umc.o smu_v11_0_i2c.o amdgpu_fru_eeprom.o amdgpu_rap.o \
amdgpu_fw_attestation.o amdgpu_securedisplay.o \
amdgpu_eeprom.o amdgpu_mca.o amdgpu_psp_ta.o amdgpu_lsdma.o \
-   amdgpu_ring_mux.o amdgpu_xcp.o amdgpu_seq64.o amdgpu_aca.o
+   amdgpu_ring_mux.o amdgpu_xcp.o amdgpu_seq64.o amdgpu_aca.o 
amdgpu_dev_coredump.o
 
 amdgpu-$(CONFIG_PROC_FS) += amdgpu_fdinfo.o
 
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_dev_coredump.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_dev_coredump.c
new file mode 100644
index ..f3a0f5857598
--- /dev/null
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_dev_coredump.c
@@ -0,0 +1,218 @@
+// SPDX-License-Identifier: MIT
+/*
+ * Copyright 2024 Advanced Micro Devices, Inc.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR
+ * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
+ * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ * OTHER DEALINGS IN THE SOFTWARE.
+ *
+ */
+
+#include 
+
+#ifndef CONFIG_DEV_COREDUMP
+void amdgpu_coredump(struct amdgpu_device *adev, bool vram_lost,
+struct amdgpu_reset_context *reset_context)
+{
+}
+#else
+
+#include 
+#include "amdgpu_dev_coredump.h"
+
+const char *hw_ip_names[MAX_HWIP] = {
+   [GC_HWIP]   = "GC",
+   [HDP_HWIP]  = "HDP",
+   [SDMA0_HWIP]= "SDMA0",
+   [SDMA1_HWIP]= "SDMA1",
+   [SDMA2_HWIP]= "SDMA2",
+   [SDMA3_HWIP]= "SDMA3",
+   [SDMA4_HWIP]= "SDMA4",
+   [SDMA5_HWIP]= "SDMA5",
+   [SDMA6_HWIP]= "SDMA6",
+   [SDMA7_HWIP]= "SDMA7",
+   [LSDMA_HWIP]= "LSDMA",
+   [MMHUB_HWIP]= "MMHUB",
+   [ATHUB_HWIP]= "ATHUB",
+   [NBIO_HWIP] = "NBIO",
+   [MP0_HWIP]  = "MP0",
+   [MP1_HWIP]  = "MP1",
+   [UVD_HWIP]  = "UVD/JPEG/VCN",
+   [VCN1_HWIP] = "VCN1",
+   [VCE_HWIP]  = "VCE",
+   [VPE_HWIP]  = "VPE",
+   [DF_HWIP]   = "DF",
+   [DCE_HWIP]  = "DCE",
+   [OSSSYS_HWIP]   = "OSSSYS",
+   [SMUIO_HWIP]= "SMUIO",
+   [PWR_HWIP]  = "PWR",
+   [NBIF_HWIP] = "NBIF",
+   [THM_HWIP]  = "THM",
+   [CLK_HWIP]  = "CLK",
+   [UMC_HWIP]  = "UMC",
+   [RSMU_HWIP] = "RSMU",
+   [XGMI_HWIP] = "XGMI",
+   [DCI_HWIP]  = "DCI",
+   [PCIE_HWIP] = "PCIE",
+};
+
+static ssize_t
+amdgpu_devcoredump_read(char *buffer, loff_t offset, size_t count,
+   void *data, size_t datalen)
+{
+   struct drm_printer p;
+   struct amdgpu_coredump_info *coredump = data;
+   struct drm_print_iterator iter;
+   struct amdgpu_vm_fault_info *fault_info;
+   int i, ver;
+
+   iter.data = buffer;
+   iter.offset = 0;
+   iter.start = offset;
+   iter.remain = 

Re: [PATCH] drm/amdkfd: Check cgroup when returning DMABuf info

2024-03-20 Thread Felix Kuehling



On 2024-03-20 15:09, Joshi, Mukul wrote:

[AMD Official Use Only - General]


-Original Message-
From: Kuehling, Felix 
Sent: Monday, March 18, 2024 4:13 PM
To: Joshi, Mukul ; amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH] drm/amdkfd: Check cgroup when returning DMABuf info


On 2024-03-15 14:17, Mukul Joshi wrote:

Check cgroup permissions when returning DMA-buf info and based on
cgroup check return the id of the GPU that has access to the BO.

Signed-off-by: Mukul Joshi 
---
   drivers/gpu/drm/amd/amdkfd/kfd_chardev.c | 4 ++--
   1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
index dfa8c69532d4..f9631f4b1a02 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
@@ -1523,7 +1523,7 @@ static int kfd_ioctl_get_dmabuf_info(struct file
*filep,

 /* Find a KFD GPU device that supports the get_dmabuf_info query */
 for (i = 0; kfd_topology_enum_kfd_devices(i, ) == 0; i++)
-   if (dev)
+   if (dev && !kfd_devcgroup_check_permission(dev))
 break;
 if (!dev)
 return -EINVAL;
@@ -1545,7 +1545,7 @@ static int kfd_ioctl_get_dmabuf_info(struct file

*filep,

 if (xcp_id >= 0)
 args->gpu_id = dmabuf_adev->kfd.dev->nodes[xcp_id]->id;
 else
-   args->gpu_id = dmabuf_adev->kfd.dev->nodes[0]->id;
+   args->gpu_id = dev->id;

If I remember correctly, this was meant as a fallback in case for GTT BOs where
the exporting partition wasn't known and the application didn't have access to
the first partition. I think the way you wrote this, it could also change the
behaviour (report the wrong GPU ID) on single-partition GPUs, which is
probably not intended. Maybe this would preserve the behaviour for that
case:


Can you please explain why this could be a issue on a single partition GPU?


What would xcp_id be on a single-partition GPU? If it's < 0, then your 
patch changes the behaviour. Instead or returning the GPU ID from the 
GPU where the memory was allocated, it returns some arbitrary GPU that 
the application has access to.


Regards,
  Felix




Regards,
Mukul


   ...
- else
+ else if (!kfd_devcgroup_check_permission(dmabuf_adev->kfd.dev-

nodes[0]))

   args->gpu_id = dmabuf_adev->kfd.dev->nodes[0]->id;
+ else
+ args->gpu_id = dev->id;

Or maybe a more general solution would make DMABuf import work when
the
exporter is really unknown or not even a GPU. This came up not so long
ago in the context of interop with 3rd-party devices. This may require
user mode changes as well.

Regards,
Felix



 args->flags = flags;

 /* Copy metadata buffer to user mode */


RE: [PATCH] drm/amdkfd: Check cgroup when returning DMABuf info

2024-03-20 Thread Joshi, Mukul
[AMD Official Use Only - General]

> -Original Message-
> From: Kuehling, Felix 
> Sent: Monday, March 18, 2024 4:13 PM
> To: Joshi, Mukul ; amd-gfx@lists.freedesktop.org
> Subject: Re: [PATCH] drm/amdkfd: Check cgroup when returning DMABuf info
>
>
> On 2024-03-15 14:17, Mukul Joshi wrote:
> > Check cgroup permissions when returning DMA-buf info and based on
> > cgroup check return the id of the GPU that has access to the BO.
> >
> > Signed-off-by: Mukul Joshi 
> > ---
> >   drivers/gpu/drm/amd/amdkfd/kfd_chardev.c | 4 ++--
> >   1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> > b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> > index dfa8c69532d4..f9631f4b1a02 100644
> > --- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> > +++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> > @@ -1523,7 +1523,7 @@ static int kfd_ioctl_get_dmabuf_info(struct file
> > *filep,
> >
> > /* Find a KFD GPU device that supports the get_dmabuf_info query */
> > for (i = 0; kfd_topology_enum_kfd_devices(i, ) == 0; i++)
> > -   if (dev)
> > +   if (dev && !kfd_devcgroup_check_permission(dev))
> > break;
> > if (!dev)
> > return -EINVAL;
> > @@ -1545,7 +1545,7 @@ static int kfd_ioctl_get_dmabuf_info(struct file
> *filep,
> > if (xcp_id >= 0)
> > args->gpu_id = dmabuf_adev->kfd.dev->nodes[xcp_id]->id;
> > else
> > -   args->gpu_id = dmabuf_adev->kfd.dev->nodes[0]->id;
> > +   args->gpu_id = dev->id;
>
> If I remember correctly, this was meant as a fallback in case for GTT BOs 
> where
> the exporting partition wasn't known and the application didn't have access to
> the first partition. I think the way you wrote this, it could also change the
> behaviour (report the wrong GPU ID) on single-partition GPUs, which is
> probably not intended. Maybe this would preserve the behaviour for that
> case:
>
Can you please explain why this could be a issue on a single partition GPU?

Regards,
Mukul

>   ...
> - else
> + else if (!kfd_devcgroup_check_permission(dmabuf_adev->kfd.dev-
> >nodes[0]))
>   args->gpu_id = dmabuf_adev->kfd.dev->nodes[0]->id;
> + else
> + args->gpu_id = dev->id;
>
> Or maybe a more general solution would make DMABuf import work when
> the
> exporter is really unknown or not even a GPU. This came up not so long
> ago in the context of interop with 3rd-party devices. This may require
> user mode changes as well.
>
> Regards,
>Felix
>
>
> > args->flags = flags;
> >
> > /* Copy metadata buffer to user mode */


[PATCH v2] drm/amdgpu: Fix use after free in trace_amdgpu_bo_move

2024-03-20 Thread Tvrtko Ursulin
From: Tvrtko Ursulin 

Pipelined object migration will free up the old bo->resource, meaning
the tracepoint added in 94aeb4117343 ("drm/amdgpu: fix ftrace event
amdgpu_bo_move always move on same heap") will trigger an use after free
when it dereferences the cached old_mem.

Fix it by caching the memory type locally, which is the only thing
tracepoint wants to know about.

While at it convert the whole function to use the cached memory types for
consistency.

v2:
 * Fix compilation.

Signed-off-by: Tvrtko Ursulin 
Fixes: 94aeb4117343 ("drm/amdgpu: fix ftrace event amdgpu_bo_move always move 
on same heap")
Closes: https://gitlab.freedesktop.org/drm/amd/-/issues/3171
Cc: Beyond Wang 
Cc: Christian König 
Cc: Alex Deucher 
---
Beware this is a speculative fix for now based only on source code
analysis and backtraces from 3171. It is also a bit on the churny side so
I am happy to minimize it. But most importantly, given how I don't have
any experience in amdgpu, I am looking for domain experts to either
confirm or disprove my analysis.
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 47 -
 1 file changed, 22 insertions(+), 25 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 8722beba494e..81189aab5a04 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -452,10 +452,11 @@ static int amdgpu_bo_move(struct ttm_buffer_object *bo, 
bool evict,
struct amdgpu_device *adev;
struct amdgpu_bo *abo;
struct ttm_resource *old_mem = bo->resource;
+   uint32_t new_mem_type = new_mem->mem_type;
+   uint32_t old_mem_type;
int r;
 
-   if (new_mem->mem_type == TTM_PL_TT ||
-   new_mem->mem_type == AMDGPU_PL_PREEMPT) {
+   if (new_mem_type == TTM_PL_TT || new_mem_type == AMDGPU_PL_PREEMPT) {
r = amdgpu_ttm_backend_bind(bo->bdev, bo->ttm, new_mem);
if (r)
return r;
@@ -464,20 +465,18 @@ static int amdgpu_bo_move(struct ttm_buffer_object *bo, 
bool evict,
abo = ttm_to_amdgpu_bo(bo);
adev = amdgpu_ttm_adev(bo->bdev);
 
-   if (!old_mem || (old_mem->mem_type == TTM_PL_SYSTEM &&
-bo->ttm == NULL)) {
+   if (!old_mem || (old_mem->mem_type == TTM_PL_SYSTEM && bo->ttm == 
NULL)) {
ttm_bo_move_null(bo, new_mem);
goto out;
}
-   if (old_mem->mem_type == TTM_PL_SYSTEM &&
-   (new_mem->mem_type == TTM_PL_TT ||
-new_mem->mem_type == AMDGPU_PL_PREEMPT)) {
+   old_mem_type = old_mem->mem_type;
+   if (old_mem_type == TTM_PL_SYSTEM &&
+   (new_mem_type == TTM_PL_TT || new_mem_type == AMDGPU_PL_PREEMPT)) {
ttm_bo_move_null(bo, new_mem);
goto out;
}
-   if ((old_mem->mem_type == TTM_PL_TT ||
-old_mem->mem_type == AMDGPU_PL_PREEMPT) &&
-   new_mem->mem_type == TTM_PL_SYSTEM) {
+   if ((old_mem_type == TTM_PL_TT || old_mem_type == AMDGPU_PL_PREEMPT) &&
+   new_mem_type == TTM_PL_SYSTEM) {
r = ttm_bo_wait_ctx(bo, ctx);
if (r)
return r;
@@ -488,22 +487,22 @@ static int amdgpu_bo_move(struct ttm_buffer_object *bo, 
bool evict,
goto out;
}
 
-   if (old_mem->mem_type == AMDGPU_PL_GDS ||
-   old_mem->mem_type == AMDGPU_PL_GWS ||
-   old_mem->mem_type == AMDGPU_PL_OA ||
-   old_mem->mem_type == AMDGPU_PL_DOORBELL ||
-   new_mem->mem_type == AMDGPU_PL_GDS ||
-   new_mem->mem_type == AMDGPU_PL_GWS ||
-   new_mem->mem_type == AMDGPU_PL_OA ||
-   new_mem->mem_type == AMDGPU_PL_DOORBELL) {
+   if (old_mem_type == AMDGPU_PL_GDS ||
+   old_mem_type == AMDGPU_PL_GWS ||
+   old_mem_type == AMDGPU_PL_OA ||
+   old_mem_type == AMDGPU_PL_DOORBELL ||
+   new_mem_type == AMDGPU_PL_GDS ||
+   new_mem_type == AMDGPU_PL_GWS ||
+   new_mem_type == AMDGPU_PL_OA ||
+   new_mem_type == AMDGPU_PL_DOORBELL) {
/* Nothing to save here */
ttm_bo_move_null(bo, new_mem);
goto out;
}
 
if (bo->type == ttm_bo_type_device &&
-   new_mem->mem_type == TTM_PL_VRAM &&
-   old_mem->mem_type != TTM_PL_VRAM) {
+   new_mem_type == TTM_PL_VRAM &&
+   old_mem_type != TTM_PL_VRAM) {
/* amdgpu_bo_fault_reserve_notify will re-set this if the CPU
 * accesses the BO after it's moved.
 */
@@ -511,10 +510,8 @@ static int amdgpu_bo_move(struct ttm_buffer_object *bo, 
bool evict,
}
 
if (adev->mman.buffer_funcs_enabled) {
-   if (((old_mem->mem_type == TTM_PL_SYSTEM &&
- new_mem->mem_type == TTM_PL_VRAM) ||
-(old_mem->mem_type == TTM_PL_VRAM &&
-   

Re: [PATCH] drm/amdgpu: Fix format character cut-off issues in amdgpu_vcn_early_init()

2024-03-20 Thread Lazar, Lijo



On 3/20/2024 8:28 PM, SRINIVASAN SHANMUGAM wrote:
> 
> On 3/20/2024 3:12 PM, Lazar, Lijo wrote:
>>
>> On 3/20/2024 2:15 PM, Srinivasan Shanmugam wrote:
>>> The issue was present in the lines where 'fw_name' was being formatted.
>>> This fix ensures that the output is not truncated
>>>
>>> Fixes the below with gcc W=1:
>>> drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c: In function
>>> ‘amdgpu_vcn_early_init’:
>>> drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c:102:66: warning: ‘snprintf’
>>> output may be truncated before the last format character
>>> [-Wformat-truncation=]
>>>    102 | snprintf(fw_name, sizeof(fw_name),
>>> "amdgpu/%s.bin", ucode_prefix);
>>>   
>>> |  ^
>>> drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c:102:17: note: ‘snprintf’
>>> output between 12 and 41 bytes into a destination of size 40
>>>    102 | snprintf(fw_name, sizeof(fw_name),
>>> "amdgpu/%s.bin", ucode_prefix);
>>>    |
>>> ^
>>> drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c:102:66: warning: ‘snprintf’
>>> output may be truncated before the last format character
>>> [-Wformat-truncation=]
>>>    102 | snprintf(fw_name, sizeof(fw_name),
>>> "amdgpu/%s.bin", ucode_prefix);
>>>   
>>> |  ^
>>> drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c:102:17: note: ‘snprintf’
>>> output between 12 and 41 bytes into a destination of size 40
>>>    102 | snprintf(fw_name, sizeof(fw_name),
>>> "amdgpu/%s.bin", ucode_prefix);
>>>    |
>>> ^
>>> drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c:105:73: warning: ‘.bin’
>>> directive output may be truncated writing 4 bytes into a region of
>>> size between 2 and 31 [-Wformat-truncation=]
>>>    105 | snprintf(fw_name, sizeof(fw_name),
>>> "amdgpu/%s_%d.bin", ucode_prefix, i);
>>>   
>>> | 
>>> ^~~~
>>> drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c:105:25: note: ‘snprintf’
>>> output between 14 and 43 bytes into a destination of size 40
>>>    105 | snprintf(fw_name, sizeof(fw_name),
>>> "amdgpu/%s_%d.bin", ucode_prefix, i);
>>>    |
>>> ^~~
>>>
>>> Cc: Alex Deucher 
>>> Cc: Christian König 
>>> Suggested-by: Lijo Lazar 
>>> Signed-off-by: Srinivasan Shanmugam 
>>> ---
>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c | 2 +-
>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
>>> index 9c514a606a2f..f994ee6c548d 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
>>> @@ -94,7 +94,7 @@ static void amdgpu_vcn_idle_work_handler(struct
>>> work_struct *work);
>>>   int amdgpu_vcn_early_init(struct amdgpu_device *adev)
>>>   {
>>>   char ucode_prefix[30];
>> Hi Srini,
>>
>> Sorry, if I miscommunicated. Suggestion was to reduce prefix size to 25
>> as the max prefix length is possibly length of dimgrey_cavefish_vcn.
> Hi Lijo,
> 
> my mistake, the fw_name size must have been 53.
> 
> How 53? -> The size of ucode_prefix is 30, so the maximum length of
> ucode_prefix is 29 characters (since one character is needed for the
> null terminator). The maximum number of digits in an integer is 10.
> Therefore, the maximum possible length of the string written into
> fw_name is 14 + 29 + 10 = 53 characters.
> 
> On the other hand reducing ucode_prefix to 25 from 30:
> 
> 1. The length of the string "amdgpu/%s.bin" is 12 characters plus the
> length of ucode_prefix. The length of the string "amdgpu/%s_%d.bin" is
> 14 characters plus the length of ucode_prefix and the number of digits
> in i.
> 

> If ucode_prefix is 25 characters long, the maximum length of the string
> written into fw_name is 14 + 25 + 1 (for a single digit i) = 40
> characters, which is exactly the size of fw_name.
> 
> Is that this solution assumes that i will not be more than 9 (a single
> digit)?. If i can be a number with more than one digit, should we need
> to increase the size of fw_name accordingly.?
> 

With 25, fw_name size required is 24 (no null char) + 13 (rest of string
including null char) + 1 (digit). 'i' is assumed to be a single digit.
(We don't need to calculate this, that warning already gives this
calculation).

Assumption is we won't be having more than 10 (again, no random number
usage for suffix, only single digit usage) variants of VCN FW that needs
to be loaded for a single SOC.

> 2. If you reduce the size of ucode_prefix to 25, it means that it can
> store a version string of up to 24 characters (since one character is
> needed 

[linux-next:master] BUILD REGRESSION 72fb52fb0ac44b6a1edd9bc390e44bce3acccd26

2024-03-20 Thread kernel test robot
tree/branch: 
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master
branch HEAD: 72fb52fb0ac44b6a1edd9bc390e44bce3acccd26  Add linux-next specific 
files for 20240320

Error/Warning ids grouped by kconfigs:

gcc_recent_errors
|-- alpha-allyesconfig
|   |-- 
drivers-gpu-drm-amd-amdgpu-amdgpu_vcn.c:warning:.bin-directive-output-may-be-truncated-writing-bytes-into-a-region-of-size-between-and
|   `-- 
fs-ubifs-journal.c:warning:expecting-prototype-for-wake_up_reservation().-Prototype-was-for-add_or_start_queue()-instead
|-- arc-allmodconfig
|   `-- 
fs-ubifs-journal.c:warning:expecting-prototype-for-wake_up_reservation().-Prototype-was-for-add_or_start_queue()-instead
|-- arc-allyesconfig
|   `-- 
fs-ubifs-journal.c:warning:expecting-prototype-for-wake_up_reservation().-Prototype-was-for-add_or_start_queue()-instead
|-- arc-randconfig-002-20240320
|   `-- 
fs-ubifs-journal.c:warning:expecting-prototype-for-wake_up_reservation().-Prototype-was-for-add_or_start_queue()-instead
|-- arm-allmodconfig
|   |-- 
arch-arm-mach-omap2-prm33xx.c:warning:expecting-prototype-for-am33xx_prm_global_warm_sw_reset().-Prototype-was-for-am33xx_prm_global_sw_reset()-instead
|   |-- 
drivers-gpu-drm-amd-amdgpu-amdgpu_vcn.c:warning:.bin-directive-output-may-be-truncated-writing-bytes-into-a-region-of-size-between-and
|   `-- 
fs-ubifs-journal.c:warning:expecting-prototype-for-wake_up_reservation().-Prototype-was-for-add_or_start_queue()-instead
|-- arm-allyesconfig
|   |-- 
arch-arm-mach-omap2-prm33xx.c:warning:expecting-prototype-for-am33xx_prm_global_warm_sw_reset().-Prototype-was-for-am33xx_prm_global_sw_reset()-instead
|   |-- 
drivers-gpu-drm-amd-amdgpu-amdgpu_vcn.c:warning:.bin-directive-output-may-be-truncated-writing-bytes-into-a-region-of-size-between-and
|   `-- 
fs-ubifs-journal.c:warning:expecting-prototype-for-wake_up_reservation().-Prototype-was-for-add_or_start_queue()-instead
|-- arm-randconfig-001-20240320
|   `-- 
arch-arm-mach-omap2-prm33xx.c:warning:expecting-prototype-for-am33xx_prm_global_warm_sw_reset().-Prototype-was-for-am33xx_prm_global_sw_reset()-instead
|-- arm64-defconfig
|   `-- 
fs-ubifs-journal.c:warning:expecting-prototype-for-wake_up_reservation().-Prototype-was-for-add_or_start_queue()-instead
|-- csky-allmodconfig
|   |-- 
drivers-gpu-drm-amd-amdgpu-amdgpu_vcn.c:warning:.bin-directive-output-may-be-truncated-writing-bytes-into-a-region-of-size-between-and
|   `-- 
fs-ubifs-journal.c:warning:expecting-prototype-for-wake_up_reservation().-Prototype-was-for-add_or_start_queue()-instead
|-- csky-allyesconfig
|   |-- 
drivers-gpu-drm-amd-amdgpu-amdgpu_vcn.c:warning:.bin-directive-output-may-be-truncated-writing-bytes-into-a-region-of-size-between-and
|   `-- 
fs-ubifs-journal.c:warning:expecting-prototype-for-wake_up_reservation().-Prototype-was-for-add_or_start_queue()-instead
|-- i386-allmodconfig
|   `-- 
fs-ubifs-journal.c:warning:expecting-prototype-for-wake_up_reservation().-Prototype-was-for-add_or_start_queue()-instead
|-- i386-allyesconfig
|   `-- 
fs-ubifs-journal.c:warning:expecting-prototype-for-wake_up_reservation().-Prototype-was-for-add_or_start_queue()-instead
|-- i386-buildonly-randconfig-002-20240320
|   `-- 
fs-ubifs-journal.c:warning:expecting-prototype-for-wake_up_reservation().-Prototype-was-for-add_or_start_queue()-instead
|-- i386-randconfig-006-20240320
|   `-- 
fs-ubifs-journal.c:warning:expecting-prototype-for-wake_up_reservation().-Prototype-was-for-add_or_start_queue()-instead
|-- i386-randconfig-015-20240320
|   `-- 
fs-ubifs-journal.c:warning:expecting-prototype-for-wake_up_reservation().-Prototype-was-for-add_or_start_queue()-instead
|-- loongarch-allmodconfig
|   |-- 
drivers-gpu-drm-amd-amdgpu-amdgpu_vcn.c:warning:.bin-directive-output-may-be-truncated-writing-bytes-into-a-region-of-size-between-and
|   `-- 
fs-ubifs-journal.c:warning:expecting-prototype-for-wake_up_reservation().-Prototype-was-for-add_or_start_queue()-instead
|-- loongarch-defconfig
|   |-- 
drivers-gpu-drm-amd-amdgpu-amdgpu_vcn.c:warning:.bin-directive-output-may-be-truncated-writing-bytes-into-a-region-of-size-between-and
|   `-- 
fs-ubifs-journal.c:warning:expecting-prototype-for-wake_up_reservation().-Prototype-was-for-add_or_start_queue()-instead
|-- loongarch-randconfig-002-20240320
|   `-- 
drivers-gpu-drm-amd-amdgpu-amdgpu_vcn.c:warning:.bin-directive-output-may-be-truncated-writing-bytes-into-a-region-of-size-between-and
|-- m68k-allmodconfig
|   `-- 
fs-ubifs-journal.c:warning:expecting-prototype-for-wake_up_reservation().-Prototype-was-for-add_or_start_queue()-instead
|-- m68k-allyesconfig
|   `-- 
fs-ubifs-journal.c:warning:expecting-prototype-for-wake_up_reservation().-Prototype-was-for-add_or_start_queue()-instead
|-- microblaze-allmodconfig
|   |-- 
drivers-gpu-drm-amd-amdgpu-amdgpu_vcn.c:warning:.bin-directive-output-may-be-truncated-writing-bytes-into-a-region-of-size-between-and
|   `-- 
fs-ubifs-journal.c:warning:expecting-prototype-for-wake_up_reservation

Re: [PATCH] drm/amdgpu: Fix format character cut-off issues in amdgpu_vcn_early_init()

2024-03-20 Thread SRINIVASAN SHANMUGAM



On 3/20/2024 3:12 PM, Lazar, Lijo wrote:


On 3/20/2024 2:15 PM, Srinivasan Shanmugam wrote:

The issue was present in the lines where 'fw_name' was being formatted.
This fix ensures that the output is not truncated

Fixes the below with gcc W=1:
drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c: In function ‘amdgpu_vcn_early_init’:
drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c:102:66: warning: ‘snprintf’ output may 
be truncated before the last format character [-Wformat-truncation=]
   102 | snprintf(fw_name, sizeof(fw_name), "amdgpu/%s.bin", 
ucode_prefix);
   |  ^
drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c:102:17: note: ‘snprintf’ output between 
12 and 41 bytes into a destination of size 40
   102 | snprintf(fw_name, sizeof(fw_name), "amdgpu/%s.bin", 
ucode_prefix);
   | 
^
drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c:102:66: warning: ‘snprintf’ output may 
be truncated before the last format character [-Wformat-truncation=]
   102 | snprintf(fw_name, sizeof(fw_name), "amdgpu/%s.bin", 
ucode_prefix);
   |  ^
drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c:102:17: note: ‘snprintf’ output between 
12 and 41 bytes into a destination of size 40
   102 | snprintf(fw_name, sizeof(fw_name), "amdgpu/%s.bin", 
ucode_prefix);
   | 
^
drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c:105:73: warning: ‘.bin’ directive 
output may be truncated writing 4 bytes into a region of size between 2 and 31 
[-Wformat-truncation=]
   105 | snprintf(fw_name, sizeof(fw_name), 
"amdgpu/%s_%d.bin", ucode_prefix, i);
   |
 ^~~~
drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c:105:25: note: ‘snprintf’ output between 
14 and 43 bytes into a destination of size 40
   105 | snprintf(fw_name, sizeof(fw_name), 
"amdgpu/%s_%d.bin", ucode_prefix, i);
   | 
^~~

Cc: Alex Deucher 
Cc: Christian König 
Suggested-by: Lijo Lazar 
Signed-off-by: Srinivasan Shanmugam 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
index 9c514a606a2f..f994ee6c548d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
@@ -94,7 +94,7 @@ static void amdgpu_vcn_idle_work_handler(struct work_struct 
*work);
  int amdgpu_vcn_early_init(struct amdgpu_device *adev)
  {
char ucode_prefix[30];

Hi Srini,

Sorry, if I miscommunicated. Suggestion was to reduce prefix size to 25
as the max prefix length is possibly length of dimgrey_cavefish_vcn.

Hi Lijo,

my mistake, the fw_name size must have been 53.

How 53? -> The size of ucode_prefix is 30, so the maximum length of 
ucode_prefix is 29 characters (since one character is needed for the 
null terminator). The maximum number of digits in an integer is 10. 
Therefore, the maximum possible length of the string written into 
fw_name is 14 + 29 + 10 = 53 characters.


On the other hand reducing ucode_prefix to 25 from 30:

1. The length of the string "amdgpu/%s.bin" is 12 characters plus the 
length of ucode_prefix. The length of the string "amdgpu/%s_%d.bin" is 
14 characters plus the length of ucode_prefix and the number of digits in i.


If ucode_prefix is 25 characters long, the maximum length of the string 
written into fw_name is 14 + 25 + 1 (for a single digit i) = 40 
characters, which is exactly the size of fw_name.


Is that this solution assumes that i will not be more than 9 (a single 
digit)?. If i can be a number with more than one digit, should we need 
to increase the size of fw_name accordingly.?


2. If you reduce the size of ucode_prefix to 25, it means that it can 
store a version string of up to 24 characters (since one character is 
needed for the null terminator).


For example: if tomorrow, if we get something like 
dimgrey_cavefish_smc_xxxyyyzz - then in this case yyyzz would be lost? 
is that in "amdgpu_ucode_ip_version_decode " & 
"amdgpu_ucode_legacy_naming" , is that are we always ensuring that it 
will never be longer than 24 characters?


Thanks,
Srini


With fw_name[42] also, you may run into 43 bytes (30 prefix + 13 for
others) warning.

Thanks,
Lijo


-   char fw_name[40];
+   char fw_name[42];
int r, i;
  
  	for (i = 0; i < adev->vcn.num_vcn_inst; i++) {


Re: [PATCH] drm/amdgpu: Fix the runtime pm mode error

2024-03-20 Thread Lazar, Lijo



On 3/20/2024 6:54 PM, Alex Deucher wrote:
> On Wed, Mar 20, 2024 at 6:17 AM Ma Jun  wrote:
>>
>> Because of the logic error, Arcturus and vega20 currently
>> use the AMDGPU_RUNPM_NONE for runtime pm even though they
>> support BACO. So, the code is optimized to fix this error.
>>
>> Signed-off-by: Ma Jun 
>> ---
>>  drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 56 -
>>  1 file changed, 27 insertions(+), 29 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c 
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
>> index 1f92fb1e7421..70cf2d0c7683 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
>> @@ -150,42 +150,40 @@ int amdgpu_driver_load_kms(struct amdgpu_device *adev, 
>> unsigned long flags)
>> }
>>
>> adev->pm.rpm_mode = AMDGPU_RUNPM_NONE;
>> -   if (amdgpu_device_supports_px(dev) &&
>> -   (amdgpu_runtime_pm != 0)) { /* enable PX as runtime mode */
>> -   adev->pm.rpm_mode = AMDGPU_RUNPM_PX;
>> -   dev_info(adev->dev, "Using ATPX for runtime pm\n");
>> -   } else if (amdgpu_device_supports_boco(dev) &&
>> -  (amdgpu_runtime_pm != 0)) { /* enable boco as runtime 
>> mode */
>> -   adev->pm.rpm_mode = AMDGPU_RUNPM_BOCO;
>> -   dev_info(adev->dev, "Using BOCO for runtime pm\n");
>> -   } else if (amdgpu_device_supports_baco(dev) &&
>> -  (amdgpu_runtime_pm != 0)) {
>> -   switch (adev->asic_type) {
>> -   case CHIP_VEGA20:
>> -   case CHIP_ARCTURUS:
>> -   /* enable BACO as runpm mode if runpm=1 */
>> -   if (amdgpu_runtime_pm > 0)
>> -   adev->pm.rpm_mode = AMDGPU_RUNPM_BACO;
>> -   break;
>> -   case CHIP_VEGA10:
>> -   /* enable BACO as runpm mode if noretry=0 */
>> -   if (!adev->gmc.noretry)
>> +   if (amdgpu_runtime_pm > 0) {
>> +   adev->pm.rpm_mode = AMDGPU_RUNPM_BACO;
>> +   dev_info(adev->dev, "Forcing BACO for runtime pm\n");
> 
> Does this need special handling for BAMACO?  Setting
> amdgpu_runtime_pm=2 is supposed to set BAMACO and 1 is supposed to
> force BACO.
> 

Also, based on the comment it appears as if runpm is not intended to be
enabled by default on Vega20/Arcturus (unless forced by module parameter).

Thanks,
Lijo

> Alex
> 
>> +   } else if (amdgpu_runtime_pm != 0) {
>> +   if (amdgpu_device_supports_px(dev)) { /* enable PX as 
>> runtime mode */
>> +   adev->pm.rpm_mode = AMDGPU_RUNPM_PX;
>> +   dev_info(adev->dev, "Using ATPX for runtime pm\n");
>> +   } else if (amdgpu_device_supports_boco(dev)) { /* enable 
>> boco as runtime mode */
>> +   adev->pm.rpm_mode = AMDGPU_RUNPM_BOCO;
>> +   dev_info(adev->dev, "Using BOCO for runtime pm\n");
>> +   } else if (amdgpu_device_supports_baco(dev)) {
>> +   switch (adev->asic_type) {
>> +   case CHIP_VEGA10:
>> +   /* enable BACO as runpm mode if noretry=0 */
>> +   if (!adev->gmc.noretry)
>> +   adev->pm.rpm_mode = 
>> AMDGPU_RUNPM_BACO;
>> +   break;
>> +   default:
>> +   /* enable BACO as runpm mode on CI+ */
>> adev->pm.rpm_mode = AMDGPU_RUNPM_BACO;
>> -   break;
>> -   default:
>> -   /* enable BACO as runpm mode on CI+ */
>> -   adev->pm.rpm_mode = AMDGPU_RUNPM_BACO;
>> -   break;
>> -   }
>> +   break;
>> +   }
>>
>> -   if (adev->pm.rpm_mode == AMDGPU_RUNPM_BACO)
>> -   dev_info(adev->dev, "Using BACO for runtime pm\n");
>> +   if (adev->pm.rpm_mode == AMDGPU_RUNPM_BACO)
>> +   dev_info(adev->dev, "Using BACO for runtime 
>> pm\n");
>> +   }
>> }
>>
>> +   if (adev->pm.rpm_mode == AMDGPU_RUNPM_NONE)
>> +   dev_info(adev->dev, "No PM mode for runtime pm\n");
>> +
>> /* Call ACPI methods: require modeset init
>>  * but failure is not fatal
>>  */
>> -
>> acpi_status = amdgpu_acpi_init(adev);
>> if (acpi_status)
>> dev_dbg(dev->dev, "Error during ACPI methods call\n");
>> --
>> 2.34.1
>>


Re: [PATCH] drm/amdgpu: Remove pci address checks from acpi_vfct_bios

2024-03-20 Thread Christian König

Am 19.03.24 um 16:04 schrieb Kurt Kartaltepe:

On Tue, Mar 19, 2024 at 2:54 AM Christian König
  wrote:


Well what problems do you run into? The ACPI and BIOS assignments
usually work much better than whatever the Linux PCI subsystem comes up
with.

Perhaps its easier to show the lspci output for the BIOS assignment
and we can agree it's far from helpful

+-04.1-[64-c3]00.0-[65-68]--+-01.0-[66]00.0-[67]00.0
  Intel Corporation JHL7540 Thunderbolt 3 USB Controller [Titan Ridge
DD 2018]
|   +-02.0-[67]--
|   \-04.0-[68]--

In this case the bios has assigned the upstream port 65-68, for its 3
downstreams 66,67,68, and then assigned the upstream port of the
device's own bridge to 67.

In this case not only did BIOS produce an invalid topology but it also
does not provide any space at the first upstream or downstream ports
which the current PCI implementation would require to assign bus
numbers if I understand it correctly.


Can you provide the full output of lspci -. As far as I can see that 
doesn't looks so invalid to me.



The PCI subsystem in the Linux kernel for example can't handle back to
back resources behind multiple downstream bridges.

So when the BIOS fails to assign something it's extremely unlikely that
the Linux kernel will do the right thing either.

I'm not sure this is still the case, the PCI subsystem with realloc
(and assign-busses for x86) deals with enumerating this topology which
reports multiple bridges just fine.


Well that is just a very very old workaround for a buggy BIOS on 20 year 
old laptops. The last reference I could find for hardware which actually 
needed it is this:


commit 8c4b2cf9af9b4ecc29d4f0ec4ecc8e94dc4432d7
Author: Bernhard Kaindl 
Date:   Sat Feb 18 01:36:55 2006 -0800

    [PATCH] PCI: PCI/Cardbus cards hidden, needs pci=assign-busses to fix


So as far as I know nobody had to use that in ages and I wouldn't expect 
that this option actually works correctly on any modern hardware.


Especially not anything PCIe based since it messes up the ACPI to PCIe 
device mappings. That amdgpu doesn't work is just the tip of the iceberg 
here.



  The same configuration as above
produces this bus numbering (with hpbussize=20)


+-04.1-[24-66]00.0-[25-66]--+-01.0-[26-45]00.0-[27-29]--+-01.0-[28]00.0
  Intel Corporation DG2 [Arc A750]
|   |
 \-04.0-[29]00.0  Intel Corporation DG2 Audio Controller
|   +-02.0-[46]00.0  Intel
Corporation JHL7540 Thunderbolt 3 USB Controller [Titan Ridge DD 2018]
|   \-04.0-[47-66]--

The Linux kernel doesnt do the right thing without these features, and
these are not the default. So you may be right that by default it does
not recover from the situation of well.


Given the bus allocation at the root port I can imagine a more
aggressive than default but less aggressive than `assign-busses`
reallocation scheme could deal with both preserving root allocations
like the APU and renumbering things behind upstream ports. That might
be a better approach than renumbering even the root bus devices.


The bus assignment code in the PCI subsystem is made to support hotplug, 
not completely re-number the root hubs from scratch. That is just a hack 
somebody came up with two decades ago to get some Cardbus slots in 
laptops working.


I'm not sure yet what's going wrong with the Thunderbold controller, but 
completely re-assigning bus numbers is certainly the wrong approach.


Regards,
Christian.




Regards,
Christian.


Re: [PATCH] drm/amdgpu: Fix the runtime pm mode error

2024-03-20 Thread Alex Deucher
On Wed, Mar 20, 2024 at 6:17 AM Ma Jun  wrote:
>
> Because of the logic error, Arcturus and vega20 currently
> use the AMDGPU_RUNPM_NONE for runtime pm even though they
> support BACO. So, the code is optimized to fix this error.
>
> Signed-off-by: Ma Jun 
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 56 -
>  1 file changed, 27 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> index 1f92fb1e7421..70cf2d0c7683 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> @@ -150,42 +150,40 @@ int amdgpu_driver_load_kms(struct amdgpu_device *adev, 
> unsigned long flags)
> }
>
> adev->pm.rpm_mode = AMDGPU_RUNPM_NONE;
> -   if (amdgpu_device_supports_px(dev) &&
> -   (amdgpu_runtime_pm != 0)) { /* enable PX as runtime mode */
> -   adev->pm.rpm_mode = AMDGPU_RUNPM_PX;
> -   dev_info(adev->dev, "Using ATPX for runtime pm\n");
> -   } else if (amdgpu_device_supports_boco(dev) &&
> -  (amdgpu_runtime_pm != 0)) { /* enable boco as runtime mode 
> */
> -   adev->pm.rpm_mode = AMDGPU_RUNPM_BOCO;
> -   dev_info(adev->dev, "Using BOCO for runtime pm\n");
> -   } else if (amdgpu_device_supports_baco(dev) &&
> -  (amdgpu_runtime_pm != 0)) {
> -   switch (adev->asic_type) {
> -   case CHIP_VEGA20:
> -   case CHIP_ARCTURUS:
> -   /* enable BACO as runpm mode if runpm=1 */
> -   if (amdgpu_runtime_pm > 0)
> -   adev->pm.rpm_mode = AMDGPU_RUNPM_BACO;
> -   break;
> -   case CHIP_VEGA10:
> -   /* enable BACO as runpm mode if noretry=0 */
> -   if (!adev->gmc.noretry)
> +   if (amdgpu_runtime_pm > 0) {
> +   adev->pm.rpm_mode = AMDGPU_RUNPM_BACO;
> +   dev_info(adev->dev, "Forcing BACO for runtime pm\n");

Does this need special handling for BAMACO?  Setting
amdgpu_runtime_pm=2 is supposed to set BAMACO and 1 is supposed to
force BACO.

Alex

> +   } else if (amdgpu_runtime_pm != 0) {
> +   if (amdgpu_device_supports_px(dev)) { /* enable PX as runtime 
> mode */
> +   adev->pm.rpm_mode = AMDGPU_RUNPM_PX;
> +   dev_info(adev->dev, "Using ATPX for runtime pm\n");
> +   } else if (amdgpu_device_supports_boco(dev)) { /* enable boco 
> as runtime mode */
> +   adev->pm.rpm_mode = AMDGPU_RUNPM_BOCO;
> +   dev_info(adev->dev, "Using BOCO for runtime pm\n");
> +   } else if (amdgpu_device_supports_baco(dev)) {
> +   switch (adev->asic_type) {
> +   case CHIP_VEGA10:
> +   /* enable BACO as runpm mode if noretry=0 */
> +   if (!adev->gmc.noretry)
> +   adev->pm.rpm_mode = AMDGPU_RUNPM_BACO;
> +   break;
> +   default:
> +   /* enable BACO as runpm mode on CI+ */
> adev->pm.rpm_mode = AMDGPU_RUNPM_BACO;
> -   break;
> -   default:
> -   /* enable BACO as runpm mode on CI+ */
> -   adev->pm.rpm_mode = AMDGPU_RUNPM_BACO;
> -   break;
> -   }
> +   break;
> +   }
>
> -   if (adev->pm.rpm_mode == AMDGPU_RUNPM_BACO)
> -   dev_info(adev->dev, "Using BACO for runtime pm\n");
> +   if (adev->pm.rpm_mode == AMDGPU_RUNPM_BACO)
> +   dev_info(adev->dev, "Using BACO for runtime 
> pm\n");
> +   }
> }
>
> +   if (adev->pm.rpm_mode == AMDGPU_RUNPM_NONE)
> +   dev_info(adev->dev, "No PM mode for runtime pm\n");
> +
> /* Call ACPI methods: require modeset init
>  * but failure is not fatal
>  */
> -
> acpi_status = amdgpu_acpi_init(adev);
> if (acpi_status)
> dev_dbg(dev->dev, "Error during ACPI methods call\n");
> --
> 2.34.1
>


[PATCH] drm/amdgpu: remove invalid resource->start check

2024-03-20 Thread Christian König
The majority of those where removed in the patch aed01a68047b
drm/amdgpu: Remove TTM resource->start visible VRAM condition v2

But this one was missed because it's working on the resource and not the
BO. Since we also no longer use a fake start address for visible BOs
this will now trigger invalid mapping errors.

Fixes: aed01a68047b ("drm/amdgpu: Remove TTM resource->start visible VRAM 
condition v2")
Signed-off-by: Christian König 
CC: sta...@vger.kernel.org
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index b0ed10f4de60..6bd7e71c5ff6 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -573,9 +573,6 @@ static int amdgpu_ttm_io_mem_reserve(struct ttm_device 
*bdev,
break;
case TTM_PL_VRAM:
mem->bus.offset = mem->start << PAGE_SHIFT;
-   /* check if it's visible */
-   if ((mem->bus.offset + bus_size) > adev->gmc.visible_vram_size)
-   return -EINVAL;
 
if (adev->mman.aper_base_kaddr &&
mem->placement & TTM_PL_FLAG_CONTIGUOUS)
-- 
2.34.1



Re: [PATCH] drm/amdgpu: Remove pci address checks from acpi_vfct_bios

2024-03-20 Thread Kurt Kartaltepe
On Tue, Mar 19, 2024 at 2:54 AM Christian König
 wrote:
>
>
> Well what problems do you run into? The ACPI and BIOS assignments
> usually work much better than whatever the Linux PCI subsystem comes up
> with.

Perhaps its easier to show the lspci output for the BIOS assignment
and we can agree it's far from helpful

   +-04.1-[64-c3]00.0-[65-68]--+-01.0-[66]00.0-[67]00.0
 Intel Corporation JHL7540 Thunderbolt 3 USB Controller [Titan Ridge
DD 2018]
   |   +-02.0-[67]--
   |   \-04.0-[68]--

In this case the bios has assigned the upstream port 65-68, for its 3
downstreams 66,67,68, and then assigned the upstream port of the
device's own bridge to 67.

In this case not only did BIOS produce an invalid topology but it also
does not provide any space at the first upstream or downstream ports
which the current PCI implementation would require to assign bus
numbers if I understand it correctly.

>
> The PCI subsystem in the Linux kernel for example can't handle back to
> back resources behind multiple downstream bridges.
>
> So when the BIOS fails to assign something it's extremely unlikely that
> the Linux kernel will do the right thing either.

I'm not sure this is still the case, the PCI subsystem with realloc
(and assign-busses for x86) deals with enumerating this topology which
reports multiple bridges just fine. The same configuration as above
produces this bus numbering (with hpbussize=20)

   
+-04.1-[24-66]00.0-[25-66]--+-01.0-[26-45]00.0-[27-29]--+-01.0-[28]00.0
 Intel Corporation DG2 [Arc A750]
   |   |
\-04.0-[29]00.0  Intel Corporation DG2 Audio Controller
   |   +-02.0-[46]00.0  Intel
Corporation JHL7540 Thunderbolt 3 USB Controller [Titan Ridge DD 2018]
   |   \-04.0-[47-66]--

The Linux kernel doesnt do the right thing without these features, and
these are not the default. So you may be right that by default it does
not recover from the situation of well.


Given the bus allocation at the root port I can imagine a more
aggressive than default but less aggressive than `assign-busses`
reallocation scheme could deal with both preserving root allocations
like the APU and renumbering things behind upstream ports. That might
be a better approach than renumbering even the root bus devices.

>
> Regards,
> Christian.


[PATCH] drm/amdgpu: Fix the runtime pm mode error

2024-03-20 Thread Ma Jun
Because of the logic error, Arcturus and vega20 currently
use the AMDGPU_RUNPM_NONE for runtime pm even though they
support BACO. So, the code is optimized to fix this error.

Signed-off-by: Ma Jun 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 56 -
 1 file changed, 27 insertions(+), 29 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
index 1f92fb1e7421..70cf2d0c7683 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
@@ -150,42 +150,40 @@ int amdgpu_driver_load_kms(struct amdgpu_device *adev, 
unsigned long flags)
}
 
adev->pm.rpm_mode = AMDGPU_RUNPM_NONE;
-   if (amdgpu_device_supports_px(dev) &&
-   (amdgpu_runtime_pm != 0)) { /* enable PX as runtime mode */
-   adev->pm.rpm_mode = AMDGPU_RUNPM_PX;
-   dev_info(adev->dev, "Using ATPX for runtime pm\n");
-   } else if (amdgpu_device_supports_boco(dev) &&
-  (amdgpu_runtime_pm != 0)) { /* enable boco as runtime mode */
-   adev->pm.rpm_mode = AMDGPU_RUNPM_BOCO;
-   dev_info(adev->dev, "Using BOCO for runtime pm\n");
-   } else if (amdgpu_device_supports_baco(dev) &&
-  (amdgpu_runtime_pm != 0)) {
-   switch (adev->asic_type) {
-   case CHIP_VEGA20:
-   case CHIP_ARCTURUS:
-   /* enable BACO as runpm mode if runpm=1 */
-   if (amdgpu_runtime_pm > 0)
-   adev->pm.rpm_mode = AMDGPU_RUNPM_BACO;
-   break;
-   case CHIP_VEGA10:
-   /* enable BACO as runpm mode if noretry=0 */
-   if (!adev->gmc.noretry)
+   if (amdgpu_runtime_pm > 0) {
+   adev->pm.rpm_mode = AMDGPU_RUNPM_BACO;
+   dev_info(adev->dev, "Forcing BACO for runtime pm\n");
+   } else if (amdgpu_runtime_pm != 0) {
+   if (amdgpu_device_supports_px(dev)) { /* enable PX as runtime 
mode */
+   adev->pm.rpm_mode = AMDGPU_RUNPM_PX;
+   dev_info(adev->dev, "Using ATPX for runtime pm\n");
+   } else if (amdgpu_device_supports_boco(dev)) { /* enable boco 
as runtime mode */
+   adev->pm.rpm_mode = AMDGPU_RUNPM_BOCO;
+   dev_info(adev->dev, "Using BOCO for runtime pm\n");
+   } else if (amdgpu_device_supports_baco(dev)) {
+   switch (adev->asic_type) {
+   case CHIP_VEGA10:
+   /* enable BACO as runpm mode if noretry=0 */
+   if (!adev->gmc.noretry)
+   adev->pm.rpm_mode = AMDGPU_RUNPM_BACO;
+   break;
+   default:
+   /* enable BACO as runpm mode on CI+ */
adev->pm.rpm_mode = AMDGPU_RUNPM_BACO;
-   break;
-   default:
-   /* enable BACO as runpm mode on CI+ */
-   adev->pm.rpm_mode = AMDGPU_RUNPM_BACO;
-   break;
-   }
+   break;
+   }
 
-   if (adev->pm.rpm_mode == AMDGPU_RUNPM_BACO)
-   dev_info(adev->dev, "Using BACO for runtime pm\n");
+   if (adev->pm.rpm_mode == AMDGPU_RUNPM_BACO)
+   dev_info(adev->dev, "Using BACO for runtime 
pm\n");
+   }
}
 
+   if (adev->pm.rpm_mode == AMDGPU_RUNPM_NONE)
+   dev_info(adev->dev, "No PM mode for runtime pm\n");
+
/* Call ACPI methods: require modeset init
 * but failure is not fatal
 */
-
acpi_status = amdgpu_acpi_init(adev);
if (acpi_status)
dev_dbg(dev->dev, "Error during ACPI methods call\n");
-- 
2.34.1



Re: [PATCH] drm/amdgpu: Fix format character cut-off issues in amdgpu_vcn_early_init()

2024-03-20 Thread Lazar, Lijo



On 3/20/2024 2:15 PM, Srinivasan Shanmugam wrote:
> The issue was present in the lines where 'fw_name' was being formatted.
> This fix ensures that the output is not truncated
> 
> Fixes the below with gcc W=1:
> drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c: In function ‘amdgpu_vcn_early_init’:
> drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c:102:66: warning: ‘snprintf’ output 
> may be truncated before the last format character [-Wformat-truncation=]
>   102 | snprintf(fw_name, sizeof(fw_name), "amdgpu/%s.bin", 
> ucode_prefix);
>   |  ^
> drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c:102:17: note: ‘snprintf’ output 
> between 12 and 41 bytes into a destination of size 40
>   102 | snprintf(fw_name, sizeof(fw_name), "amdgpu/%s.bin", 
> ucode_prefix);
>   | 
> ^
> drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c:102:66: warning: ‘snprintf’ output 
> may be truncated before the last format character [-Wformat-truncation=]
>   102 | snprintf(fw_name, sizeof(fw_name), "amdgpu/%s.bin", 
> ucode_prefix);
>   |  ^
> drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c:102:17: note: ‘snprintf’ output 
> between 12 and 41 bytes into a destination of size 40
>   102 | snprintf(fw_name, sizeof(fw_name), "amdgpu/%s.bin", 
> ucode_prefix);
>   | 
> ^
> drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c:105:73: warning: ‘.bin’ directive 
> output may be truncated writing 4 bytes into a region of size between 2 and 
> 31 [-Wformat-truncation=]
>   105 | snprintf(fw_name, sizeof(fw_name), 
> "amdgpu/%s_%d.bin", ucode_prefix, i);
>   |   
>   ^~~~
> drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c:105:25: note: ‘snprintf’ output 
> between 14 and 43 bytes into a destination of size 40
>   105 | snprintf(fw_name, sizeof(fw_name), 
> "amdgpu/%s_%d.bin", ucode_prefix, i);
>   | 
> ^~~
> 
> Cc: Alex Deucher 
> Cc: Christian König 
> Suggested-by: Lijo Lazar 
> Signed-off-by: Srinivasan Shanmugam 
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
> index 9c514a606a2f..f994ee6c548d 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
> @@ -94,7 +94,7 @@ static void amdgpu_vcn_idle_work_handler(struct work_struct 
> *work);
>  int amdgpu_vcn_early_init(struct amdgpu_device *adev)
>  {
>   char ucode_prefix[30];

Hi Srini,

Sorry, if I miscommunicated. Suggestion was to reduce prefix size to 25
as the max prefix length is possibly length of dimgrey_cavefish_vcn.

With fw_name[42] also, you may run into 43 bytes (30 prefix + 13 for
others) warning.

Thanks,
Lijo

> - char fw_name[40];
> + char fw_name[42];
>   int r, i;
>  
>   for (i = 0; i < adev->vcn.num_vcn_inst; i++) {


Re: [PATCH] drm/amdgpu: Fix format character cut-off issues in amdgpu_vcn_early_init()

2024-03-20 Thread Christian König

Am 20.03.24 um 09:45 schrieb Srinivasan Shanmugam:

The issue was present in the lines where 'fw_name' was being formatted.
This fix ensures that the output is not truncated

Fixes the below with gcc W=1:
drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c: In function ‘amdgpu_vcn_early_init’:
drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c:102:66: warning: ‘snprintf’ output may 
be truncated before the last format character [-Wformat-truncation=]
   102 | snprintf(fw_name, sizeof(fw_name), "amdgpu/%s.bin", 
ucode_prefix);
   |  ^
drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c:102:17: note: ‘snprintf’ output between 
12 and 41 bytes into a destination of size 40
   102 | snprintf(fw_name, sizeof(fw_name), "amdgpu/%s.bin", 
ucode_prefix);
   | 
^
drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c:102:66: warning: ‘snprintf’ output may 
be truncated before the last format character [-Wformat-truncation=]
   102 | snprintf(fw_name, sizeof(fw_name), "amdgpu/%s.bin", 
ucode_prefix);
   |  ^
drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c:102:17: note: ‘snprintf’ output between 
12 and 41 bytes into a destination of size 40
   102 | snprintf(fw_name, sizeof(fw_name), "amdgpu/%s.bin", 
ucode_prefix);
   | 
^
drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c:105:73: warning: ‘.bin’ directive 
output may be truncated writing 4 bytes into a region of size between 2 and 31 
[-Wformat-truncation=]
   105 | snprintf(fw_name, sizeof(fw_name), 
"amdgpu/%s_%d.bin", ucode_prefix, i);
   |
 ^~~~
drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c:105:25: note: ‘snprintf’ output between 
14 and 43 bytes into a destination of size 40
   105 | snprintf(fw_name, sizeof(fw_name), 
"amdgpu/%s_%d.bin", ucode_prefix, i);
   | 
^~~

Cc: Alex Deucher 
Cc: Christian König 
Suggested-by: Lijo Lazar 
Signed-off-by: Srinivasan Shanmugam 


Acked-by: Christian König 


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

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
index 9c514a606a2f..f994ee6c548d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
@@ -94,7 +94,7 @@ static void amdgpu_vcn_idle_work_handler(struct work_struct 
*work);
  int amdgpu_vcn_early_init(struct amdgpu_device *adev)
  {
char ucode_prefix[30];
-   char fw_name[40];
+   char fw_name[42];
int r, i;
  
  	for (i = 0; i < adev->vcn.num_vcn_inst; i++) {




[PATCH] drm/amdgpu: Fix format character cut-off issues in amdgpu_vcn_early_init()

2024-03-20 Thread Srinivasan Shanmugam
The issue was present in the lines where 'fw_name' was being formatted.
This fix ensures that the output is not truncated

Fixes the below with gcc W=1:
drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c: In function ‘amdgpu_vcn_early_init’:
drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c:102:66: warning: ‘snprintf’ output may 
be truncated before the last format character [-Wformat-truncation=]
  102 | snprintf(fw_name, sizeof(fw_name), "amdgpu/%s.bin", 
ucode_prefix);
  |  ^
drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c:102:17: note: ‘snprintf’ output between 
12 and 41 bytes into a destination of size 40
  102 | snprintf(fw_name, sizeof(fw_name), "amdgpu/%s.bin", 
ucode_prefix);
  | 
^
drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c:102:66: warning: ‘snprintf’ output may 
be truncated before the last format character [-Wformat-truncation=]
  102 | snprintf(fw_name, sizeof(fw_name), "amdgpu/%s.bin", 
ucode_prefix);
  |  ^
drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c:102:17: note: ‘snprintf’ output between 
12 and 41 bytes into a destination of size 40
  102 | snprintf(fw_name, sizeof(fw_name), "amdgpu/%s.bin", 
ucode_prefix);
  | 
^
drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c:105:73: warning: ‘.bin’ directive 
output may be truncated writing 4 bytes into a region of size between 2 and 31 
[-Wformat-truncation=]
  105 | snprintf(fw_name, sizeof(fw_name), 
"amdgpu/%s_%d.bin", ucode_prefix, i);
  | 
^~~~
drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c:105:25: note: ‘snprintf’ output between 
14 and 43 bytes into a destination of size 40
  105 | snprintf(fw_name, sizeof(fw_name), 
"amdgpu/%s_%d.bin", ucode_prefix, i);
  | 
^~~

Cc: Alex Deucher 
Cc: Christian König 
Suggested-by: Lijo Lazar 
Signed-off-by: Srinivasan Shanmugam 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
index 9c514a606a2f..f994ee6c548d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
@@ -94,7 +94,7 @@ static void amdgpu_vcn_idle_work_handler(struct work_struct 
*work);
 int amdgpu_vcn_early_init(struct amdgpu_device *adev)
 {
char ucode_prefix[30];
-   char fw_name[40];
+   char fw_name[42];
int r, i;
 
for (i = 0; i < adev->vcn.num_vcn_inst; i++) {
-- 
2.34.1



[PATCH 22/22] drm/amd/display: 3.2.278

2024-03-20 Thread Tom Chung
From: Aric Cyr 

This version brings along following fixes:
- Fix some bound and NULL check
- Fix nonseamless transition from ODM + MPO to ODM + subvp
- Allow Z8 when stutter threshold is not met
- Remove plane and stream pointers from dc scratch
- Remove read/write to external register
- Increase number of hpo dp link encoders
- Increase clock table size
- Add new IPS config mode
- Build scaling params when a new plane is appended
- Refactor DML2 interfaces
- Allow idle opts for no flip case on PSR panel

Acked-by: Tom Chung 
Signed-off-by: Aric Cyr 
---
 drivers/gpu/drm/amd/display/dc/dc.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/display/dc/dc.h 
b/drivers/gpu/drm/amd/display/dc/dc.h
index dd54de5f3b2f..6300ae2ea1f7 100644
--- a/drivers/gpu/drm/amd/display/dc/dc.h
+++ b/drivers/gpu/drm/amd/display/dc/dc.h
@@ -51,7 +51,7 @@ struct aux_payload;
 struct set_config_cmd_payload;
 struct dmub_notification;
 
-#define DC_VER "3.2.277"
+#define DC_VER "3.2.278"
 
 #define MAX_SURFACES 3
 #define MAX_PLANES 6
-- 
2.34.1



[PATCH 21/22] drm/amd/display: Skip pipe if the pipe idx not set properly

2024-03-20 Thread Tom Chung
From: Muhammad Ahmed 

[why]
Driver crashes when pipe idx not set properly

[how]
Add code to skip the pipe that idx not set properly

Reviewed-by: Charlene Liu 
Acked-by: Tom Chung 
Signed-off-by: Muhammad Ahmed 
---
 drivers/gpu/drm/amd/display/dc/dml2/dml2_utils.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/drivers/gpu/drm/amd/display/dc/dml2/dml2_utils.c 
b/drivers/gpu/drm/amd/display/dc/dml2/dml2_utils.c
index ac2676bb9fcb..bedf1cd390df 100644
--- a/drivers/gpu/drm/amd/display/dc/dml2/dml2_utils.c
+++ b/drivers/gpu/drm/amd/display/dc/dml2/dml2_utils.c
@@ -327,6 +327,8 @@ void dml2_calculate_rq_and_dlg_params(const struct dc *dc, 
struct dc_state *cont
dml_pipe_idx = 
dml2_helper_find_dml_pipe_idx_by_stream_id(in_ctx, 
context->res_ctx.pipe_ctx[dc_pipe_ctx_index].stream->stream_id);
}
 
+   if (dml_pipe_idx == 0x)
+   continue;

ASSERT(in_ctx->v20.scratch.dml_to_dc_pipe_mapping.dml_pipe_idx_to_stream_id_valid[dml_pipe_idx]);

ASSERT(in_ctx->v20.scratch.dml_to_dc_pipe_mapping.dml_pipe_idx_to_stream_id[dml_pipe_idx]
 == context->res_ctx.pipe_ctx[dc_pipe_ctx_index].stream->stream_id);
 
@@ -474,6 +476,9 @@ bool dml2_verify_det_buffer_configuration(struct 
dml2_context *in_ctx, struct dc
dml_pipe_idx = find_dml_pipe_idx_by_plane_id(in_ctx, 
plane_id);
else
dml_pipe_idx = 
dml2_helper_find_dml_pipe_idx_by_stream_id(in_ctx, 
display_state->res_ctx.pipe_ctx[i].stream->stream_id);
+
+   if (dml_pipe_idx == 0x)
+   continue;
total_det_allocated += 
dml_get_det_buffer_size_kbytes(_ctx->v20.dml_core_ctx, dml_pipe_idx);
if (total_det_allocated > max_det_size) {
need_recalculation = true;
-- 
2.34.1



[PATCH 19/22] drm/amd/display: Modify DHCUB waterwark structures and functions

2024-03-20 Thread Tom Chung
From: Dillon Varone 

[WHY]
Converting the watermark set structure to a union and modifying some interfaces
to accommodate future usage.

Reviewed-by: Chaitanya Dhere 
Acked-by: Tom Chung 
Signed-off-by: Dillon Varone 
---
 .../gpu/drm/amd/display/dc/dcn10/dcn10_hubbub.c  |  8 
 .../gpu/drm/amd/display/dc/dcn10/dcn10_hubbub.h  | 10 +-
 .../gpu/drm/amd/display/dc/dcn20/dcn20_hubbub.c  |  2 +-
 .../gpu/drm/amd/display/dc/dcn20/dcn20_hubbub.h  |  2 +-
 .../drm/amd/display/dc/dcn201/dcn201_hubbub.c|  2 +-
 .../gpu/drm/amd/display/dc/dcn21/dcn21_hubbub.c  |  8 
 .../gpu/drm/amd/display/dc/dcn21/dcn21_hubbub.h  |  8 
 .../gpu/drm/amd/display/dc/dcn30/dcn30_hubbub.c  |  2 +-
 .../gpu/drm/amd/display/dc/dcn30/dcn30_hubbub.h  |  2 +-
 .../gpu/drm/amd/display/dc/dcn31/dcn31_hubbub.c  |  8 
 .../gpu/drm/amd/display/dc/dcn32/dcn32_hubbub.c  | 10 +-
 .../gpu/drm/amd/display/dc/dcn32/dcn32_hubbub.h  |  8 
 .../gpu/drm/amd/display/dc/dcn35/dcn35_hubbub.c  |  4 ++--
 .../drm/amd/display/dc/hwss/dcn20/dcn20_hwseq.c  | 16 ++--
 drivers/gpu/drm/amd/display/dc/inc/core_types.h  |  2 +-
 drivers/gpu/drm/amd/display/dc/inc/hw/dchubbub.h |  2 +-
 .../gpu/drm/amd/display/dc/inc/hw/mem_input.h| 12 +++-
 17 files changed, 56 insertions(+), 50 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hubbub.c 
b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hubbub.c
index d51f1ce02874..d9ade9ee0aeb 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hubbub.c
+++ b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hubbub.c
@@ -242,7 +242,7 @@ void hubbub1_wm_change_req_wa(struct hubbub *hubbub)
 
 bool hubbub1_program_urgent_watermarks(
struct hubbub *hubbub,
-   struct dcn_watermark_set *watermarks,
+   union dcn_watermark_set *watermarks,
unsigned int refclk_mhz,
bool safe_to_lower)
 {
@@ -356,7 +356,7 @@ bool hubbub1_program_urgent_watermarks(
 
 bool hubbub1_program_stutter_watermarks(
struct hubbub *hubbub,
-   struct dcn_watermark_set *watermarks,
+   union dcn_watermark_set *watermarks,
unsigned int refclk_mhz,
bool safe_to_lower)
 {
@@ -501,7 +501,7 @@ bool hubbub1_program_stutter_watermarks(
 
 bool hubbub1_program_pstate_watermarks(
struct hubbub *hubbub,
-   struct dcn_watermark_set *watermarks,
+   union dcn_watermark_set *watermarks,
unsigned int refclk_mhz,
bool safe_to_lower)
 {
@@ -582,7 +582,7 @@ bool hubbub1_program_pstate_watermarks(
 
 bool hubbub1_program_watermarks(
struct hubbub *hubbub,
-   struct dcn_watermark_set *watermarks,
+   union dcn_watermark_set *watermarks,
unsigned int refclk_mhz,
bool safe_to_lower)
 {
diff --git a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hubbub.h 
b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hubbub.h
index 4201b7627030..d1f9e63944c8 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hubbub.h
+++ b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hubbub.h
@@ -409,7 +409,7 @@ struct dcn10_hubbub {
const struct dcn_hubbub_shift *shifts;
const struct dcn_hubbub_mask *masks;
unsigned int debug_test_index_pstate;
-   struct dcn_watermark_set watermarks;
+   union dcn_watermark_set watermarks;
 };
 
 void hubbub1_update_dchub(
@@ -423,7 +423,7 @@ void hubbub1_wm_change_req_wa(struct hubbub *hubbub);
 
 bool hubbub1_program_watermarks(
struct hubbub *hubbub,
-   struct dcn_watermark_set *watermarks,
+   union dcn_watermark_set *watermarks,
unsigned int refclk_mhz,
bool safe_to_lower);
 
@@ -446,17 +446,17 @@ void hubbub1_construct(struct hubbub *hubbub,
 
 bool hubbub1_program_urgent_watermarks(
struct hubbub *hubbub,
-   struct dcn_watermark_set *watermarks,
+   union dcn_watermark_set *watermarks,
unsigned int refclk_mhz,
bool safe_to_lower);
 bool hubbub1_program_stutter_watermarks(
struct hubbub *hubbub,
-   struct dcn_watermark_set *watermarks,
+   union dcn_watermark_set *watermarks,
unsigned int refclk_mhz,
bool safe_to_lower);
 bool hubbub1_program_pstate_watermarks(
struct hubbub *hubbub,
-   struct dcn_watermark_set *watermarks,
+   union dcn_watermark_set *watermarks,
unsigned int refclk_mhz,
bool safe_to_lower);
 
diff --git a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_hubbub.c 
b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_hubbub.c
index 6eebcb22e317..c6f859871d11 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_hubbub.c
+++ b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_hubbub.c
@@ 

[PATCH 20/22] drm/amd/display: [FW Promotion] Release 0.0.210.0

2024-03-20 Thread Tom Chung
From: Anthony Koo 

 - Add Display PHY FSM command interface for automated testing

Acked-by: Tom Chung 
Signed-off-by: Anthony Koo 
---
 .../gpu/drm/amd/display/dmub/inc/dmub_cmd.h   | 39 ++-
 1 file changed, 38 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/display/dmub/inc/dmub_cmd.h 
b/drivers/gpu/drm/amd/display/dmub/inc/dmub_cmd.h
index 995a54459335..3bd9911b6f3a 100644
--- a/drivers/gpu/drm/amd/display/dmub/inc/dmub_cmd.h
+++ b/drivers/gpu/drm/amd/display/dmub/inc/dmub_cmd.h
@@ -824,6 +824,10 @@ enum dmub_cmd_vbios_type {
 * Query DP alt status on a transmitter.
 */
DMUB_CMD__VBIOS_TRANSMITTER_QUERY_DP_ALT  = 26,
+   /**
+* Control PHY FSM
+*/
+   DMUB_CMD__VBIOS_TRANSMITTER_SET_PHY_FSM  = 29,
/**
 * Controls domain power gating
 */
@@ -2345,6 +2349,7 @@ enum dmub_phy_fsm_state {
DMUB_PHY_FSM_POWER_DOWN,
DMUB_PHY_FSM_PLL_EN,
DMUB_PHY_FSM_TX_EN,
+   DMUB_PHY_FSM_TX_EN_TEST_MODE,
DMUB_PHY_FSM_FAST_LP,
DMUB_PHY_FSM_P2_PLL_OFF_CPM,
DMUB_PHY_FSM_P2_PLL_OFF_PG,
@@ -3494,7 +3499,7 @@ enum hw_lock_client {
/**
 * Replay is the client of HW Lock Manager.
 */
-   HW_LOCK_CLIENT_REPLAY   = 4,
+   HW_LOCK_CLIENT_REPLAY   = 4,
/**
 * Invalid client.
 */
@@ -4188,6 +4193,34 @@ struct dmub_rb_cmd_transmitter_query_dp_alt {
struct dmub_rb_cmd_transmitter_query_dp_alt_data data; /**< payload */
 };
 
+struct phy_test_mode {
+   uint8_t mode;
+   uint8_t pat0;
+   uint8_t pad[2];
+};
+
+/**
+ * Data passed in/out in a DMUB_CMD__VBIOS_TRANSMITTER_SET_PHY_FSM command.
+ */
+struct dmub_rb_cmd_transmitter_set_phy_fsm_data {
+   uint8_t phy_id; /**< 0=UNIPHYA, 1=UNIPHYB, 2=UNIPHYC, 3=UNIPHYD, 
4=UNIPHYE, 5=UNIPHYF */
+   uint8_t mode; /**< HDMI/DP/DP2 etc */
+   uint8_t lane_num; /**< Number of lanes */
+   uint32_t symclk_100Hz; /**< PLL symclock in 100hz */
+   struct phy_test_mode test_mode;
+   enum dmub_phy_fsm_state state;
+   uint32_t status;
+   uint8_t pad;
+};
+
+/**
+ * Definition of a DMUB_CMD__VBIOS_TRANSMITTER_SET_PHY_FSM command.
+ */
+struct dmub_rb_cmd_transmitter_set_phy_fsm {
+   struct dmub_cmd_header header; /**< header */
+   struct dmub_rb_cmd_transmitter_set_phy_fsm_data data; /**< payload */
+};
+
 /**
  * Maximum number of bytes a chunk sent to DMUB for parsing
  */
@@ -4558,6 +4591,10 @@ union dmub_rb_cmd {
 * Definition of a DMUB_CMD__VBIOS_TRANSMITTER_QUERY_DP_ALT command.
 */
struct dmub_rb_cmd_transmitter_query_dp_alt query_dp_alt;
+   /**
+* Definition of a DMUB_CMD__VBIOS_TRANSMITTER_SET_PHY_FSM command.
+*/
+   struct dmub_rb_cmd_transmitter_set_phy_fsm set_phy_fsm;
/**
 * Definition of a DMUB_CMD__DPIA_DIG1_CONTROL command.
 */
-- 
2.34.1



[PATCH 18/22] drm/amd/display: Remove plane and stream pointers from dc scratch

2024-03-20 Thread Tom Chung
From: Alvin Lee 

[Why]
Remove several plane and stream pointers from dc for code
refactoring.

Reviewed-by: Wenjing Liu 
Acked-by: Tom Chung 
Signed-off-by: Alvin Lee 
---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 18 ++---
 .../amd/display/amdgpu_dm/amdgpu_dm_color.c   | 42 +-
 drivers/gpu/drm/amd/display/dc/core/dc.c  | 80 +++
 .../drm/amd/display/dc/core/dc_hw_sequencer.c |  8 +-
 .../gpu/drm/amd/display/dc/core/dc_resource.c |  2 +-
 .../gpu/drm/amd/display/dc/core/dc_stream.c   | 16 +---
 .../gpu/drm/amd/display/dc/core/dc_surface.c  | 47 ++-
 drivers/gpu/drm/amd/display/dc/dc.h   | 26 +++---
 drivers/gpu/drm/amd/display/dc/dc_stream.h|  2 +-
 drivers/gpu/drm/amd/display/dc/dc_types.h |  2 +-
 .../drm/amd/display/dc/dpp/dcn20/dcn20_dpp.h  |  2 +-
 .../amd/display/dc/dpp/dcn20/dcn20_dpp_cm.c   | 10 +--
 .../drm/amd/display/dc/dpp/dcn30/dcn30_dpp.c  | 10 +--
 .../amd/display/dc/hwss/dce110/dce110_hwseq.c | 15 ++--
 .../amd/display/dc/hwss/dcn10/dcn10_hwseq.c   | 24 +++---
 .../amd/display/dc/hwss/dcn20/dcn20_hwseq.c   | 63 +++
 .../amd/display/dc/hwss/dcn30/dcn30_hwseq.c   | 66 +++
 .../amd/display/dc/hwss/dcn30/dcn30_hwseq.h   |  5 +-
 .../amd/display/dc/hwss/dcn32/dcn32_hwseq.c   | 73 -
 .../drm/amd/display/dc/hwss/hw_sequencer.h|  5 +-
 drivers/gpu/drm/amd/display/dc/inc/hw/dpp.h   |  2 +-
 21 files changed, 218 insertions(+), 300 deletions(-)

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 6bfe327dfb4f..399f736207d0 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -5700,8 +5700,8 @@ static void fill_stream_properties_from_drm_display_mode(
 
timing_out->aspect_ratio = get_aspect_ratio(mode_in);
 
-   stream->out_transfer_func->type = TF_TYPE_PREDEFINED;
-   stream->out_transfer_func->tf = TRANSFER_FUNCTION_SRGB;
+   stream->out_transfer_func.type = TF_TYPE_PREDEFINED;
+   stream->out_transfer_func.tf = TRANSFER_FUNCTION_SRGB;
if (stream->signal == SIGNAL_TYPE_HDMI_TYPE_A) {
if (!adjust_colour_depth_from_display_info(timing_out, info) &&
drm_mode_is_420_also(info, mode_in) &&
@@ -6320,7 +6320,7 @@ create_stream_for_sink(struct drm_connector *connector,
if 
(stream->link->dpcd_caps.dprx_feature.bits.VSC_SDP_COLORIMETRY_SUPPORTED)
stream->use_vsc_sdp_for_colorimetry = true;
}
-   if (stream->out_transfer_func->tf == TRANSFER_FUNCTION_GAMMA22)
+   if (stream->out_transfer_func.tf == TRANSFER_FUNCTION_GAMMA22)
tf = TRANSFER_FUNC_GAMMA_22;
mod_build_vsc_infopacket(stream, >vsc_infopacket, 
stream->output_color_space, tf);
 
@@ -8395,13 +8395,13 @@ static void amdgpu_dm_commit_planes(struct 
drm_atomic_state *state,
 
bundle->surface_updates[planes_count].surface = dc_plane;
if (new_pcrtc_state->color_mgmt_changed) {
-   bundle->surface_updates[planes_count].gamma = 
dc_plane->gamma_correction;
-   bundle->surface_updates[planes_count].in_transfer_func 
= dc_plane->in_transfer_func;
+   bundle->surface_updates[planes_count].gamma = 
_plane->gamma_correction;
+   bundle->surface_updates[planes_count].in_transfer_func 
= _plane->in_transfer_func;

bundle->surface_updates[planes_count].gamut_remap_matrix = 
_plane->gamut_remap_matrix;
bundle->surface_updates[planes_count].hdr_mult = 
dc_plane->hdr_mult;
-   bundle->surface_updates[planes_count].func_shaper = 
dc_plane->in_shaper_func;
-   bundle->surface_updates[planes_count].lut3d_func = 
dc_plane->lut3d_func;
-   bundle->surface_updates[planes_count].blend_tf = 
dc_plane->blend_tf;
+   bundle->surface_updates[planes_count].func_shaper = 
_plane->in_shaper_func;
+   bundle->surface_updates[planes_count].lut3d_func = 
_plane->lut3d_func;
+   bundle->surface_updates[planes_count].blend_tf = 
_plane->blend_tf;
}
 
amdgpu_dm_plane_fill_dc_scaling_info(dm->adev, new_plane_state,
@@ -8614,7 +8614,7 @@ static void amdgpu_dm_commit_planes(struct 
drm_atomic_state *state,
bundle->stream_update.output_csc_transform =
_state->stream->csc_color_matrix;
bundle->stream_update.out_transfer_func =
-   acrtc_state->stream->out_transfer_func;
+   _state->stream->out_transfer_func;
bundle->stream_update.lut3d_func =
(struct dc_3dlut *) 

[PATCH 17/22] drm/amd/display: Increase number of hpo dp link encoders

2024-03-20 Thread Tom Chung
From: Sridevi Arvindekar 

[Why]
Number of hpo dp2 link encoders is increased.
Instances are changed.

[How]
Increased size in resource pool, init for each instance

Reviewed-by: Alvin Lee 
Acked-by: Tom Chung 
Signed-off-by: Sridevi Arvindekar 
---
 drivers/gpu/drm/amd/display/dc/inc/hw/hw_shared.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/display/dc/inc/hw/hw_shared.h 
b/drivers/gpu/drm/amd/display/dc/inc/hw/hw_shared.h
index dcae23faeee3..c1835ad6550f 100644
--- a/drivers/gpu/drm/amd/display/dc/inc/hw/hw_shared.h
+++ b/drivers/gpu/drm/amd/display/dc/inc/hw/hw_shared.h
@@ -47,7 +47,7 @@
 #define MAX_DIG_LINK_ENCODERS 7
 #define MAX_DWB_PIPES  1
 #define MAX_HPO_DP2_ENCODERS   4
-#define MAX_HPO_DP2_LINK_ENCODERS  2
+#define MAX_HPO_DP2_LINK_ENCODERS  4
 
 struct gamma_curve {
uint32_t offset;
-- 
2.34.1



[PATCH 16/22] drm/amd/display: Increase clock table size

2024-03-20 Thread Tom Chung
From: Sung Joon Kim 

[why]
To prevent out of bounds error, we need
to increase the clock table size.

Reviewed-by: Xi Liu 
Acked-by: Tom Chung 
Signed-off-by: Sung Joon Kim 
---
 .../amd/display/dc/dml2/dml2_translation_helper.c  | 14 +-
 1 file changed, 5 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/dml2/dml2_translation_helper.c 
b/drivers/gpu/drm/amd/display/dc/dml2/dml2_translation_helper.c
index 0a4dff45731f..cf98411d0799 100644
--- a/drivers/gpu/drm/amd/display/dc/dml2/dml2_translation_helper.c
+++ b/drivers/gpu/drm/amd/display/dc/dml2/dml2_translation_helper.c
@@ -29,10 +29,7 @@
 #include "dml2_translation_helper.h"
 
 #define NUM_DCFCLK_STAS 5
-
-#if defined(CONFIG_DRM_AMD_DC_DCN3_51)
 #define NUM_DCFCLK_STAS_NEW 8
-#endif
 
 void dml2_init_ip_params(struct dml2_context *dml2, const struct dc *in_dc, 
struct ip_params_st *out)
 {
@@ -258,21 +255,20 @@ void dml2_init_soc_states(struct dml2_context *dml2, 
const struct dc *in_dc,
struct dml2_policy_build_synthetic_soc_states_scratch *s = 
>v20.scratch.create_scratch.build_synthetic_socbb_scratch;
struct dml2_policy_build_synthetic_soc_states_params *p = 
>v20.scratch.build_synthetic_socbb_params;
unsigned int dcfclk_stas_mhz[NUM_DCFCLK_STAS];
-#if defined(CONFIG_DRM_AMD_DC_DCN3_51)
unsigned int dcfclk_stas_mhz_new[NUM_DCFCLK_STAS_NEW];
unsigned int dml_project = dml2->v20.dml_core_ctx.project;
-#endif
+
unsigned int i = 0;
unsigned int transactions_per_mem_clock = 16; // project specific, 
depends on used Memory type
 
-   p->dcfclk_stas_mhz = dcfclk_stas_mhz;
-   p->num_dcfclk_stas = NUM_DCFCLK_STAS;
-#if defined(CONFIG_DRM_AMD_DC_DCN3_51)
if (dml_project == dml_project_dcn351) {
p->dcfclk_stas_mhz = dcfclk_stas_mhz_new;
p->num_dcfclk_stas = NUM_DCFCLK_STAS_NEW;
+   } else {
+   p->dcfclk_stas_mhz = dcfclk_stas_mhz;
+   p->num_dcfclk_stas = NUM_DCFCLK_STAS;
}
-#endif
+
p->in_bbox = in_bbox;
p->out_states = out;
p->in_states = >v20.scratch.create_scratch.in_states;
-- 
2.34.1



[PATCH 15/22] drm/amd/display: Allow Z8 when stutter threshold is not met for dcn35

2024-03-20 Thread Tom Chung
From: Bhawanpreet Lakha 

[Why]
Some panels don't meet the stutter threshold (4k etc), this leads to
power regressions. Allow z8 for panels that don't meet the threshold
but support PSR/replay

Reviewed-by: Nicholas Kazlauskas 
Acked-by: Tom Chung 
Signed-off-by: Bhawanpreet Lakha 
---
 drivers/gpu/drm/amd/display/dc/dml/dcn35/dcn35_fpu.c | 2 +-
 drivers/gpu/drm/amd/display/dc/dml2/dml2_wrapper.c   | 9 +
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/display/dc/dml/dcn35/dcn35_fpu.c 
b/drivers/gpu/drm/amd/display/dc/dml/dcn35/dcn35_fpu.c
index 33ea89f20449..714c2fe03c5f 100644
--- a/drivers/gpu/drm/amd/display/dc/dml/dcn35/dcn35_fpu.c
+++ b/drivers/gpu/drm/amd/display/dc/dml/dcn35/dcn35_fpu.c
@@ -603,7 +603,7 @@ void dcn35_decide_zstate_support(struct dc *dc, struct 
dc_state *context)
if (is_pwrseq0 && allow_z10)
support = DCN_ZSTATE_SUPPORT_ALLOW;
else if (is_pwrseq0 && (is_psr || is_replay))
-   support = allow_z8 ? 
DCN_ZSTATE_SUPPORT_ALLOW_Z8_Z10_ONLY : DCN_ZSTATE_SUPPORT_ALLOW_Z10_ONLY;
+   support = DCN_ZSTATE_SUPPORT_ALLOW_Z8_Z10_ONLY;
else if (allow_z8)
support = DCN_ZSTATE_SUPPORT_ALLOW_Z8_ONLY;
 
diff --git a/drivers/gpu/drm/amd/display/dc/dml2/dml2_wrapper.c 
b/drivers/gpu/drm/amd/display/dc/dml2/dml2_wrapper.c
index 72cca367062e..e2489eaabb20 100644
--- a/drivers/gpu/drm/amd/display/dc/dml2/dml2_wrapper.c
+++ b/drivers/gpu/drm/amd/display/dc/dml2/dml2_wrapper.c
@@ -570,6 +570,7 @@ static bool dml2_validate_and_build_resource(const struct 
dc *in_dc, struct dc_s
struct dml2_dcn_clocks out_clks;
unsigned int result = 0;
bool need_recalculation = false;
+   uint32_t cstate_enter_plus_exit_z8_ns;
 
if (!context || context->stream_count == 0)
return true;
@@ -641,6 +642,14 @@ static bool dml2_validate_and_build_resource(const struct 
dc *in_dc, struct dc_s

dml2_extract_watermark_set(>bw_ctx.bw.dcn.watermarks.d, 
>v20.dml_core_ctx);
//copy for deciding zstate use
context->bw_ctx.dml.vba.StutterPeriod = 
context->bw_ctx.dml2->v20.dml_core_ctx.mp.StutterPeriod;
+
+   cstate_enter_plus_exit_z8_ns = 
context->bw_ctx.bw.dcn.watermarks.a.cstate_pstate.cstate_enter_plus_exit_z8_ns;
+
+   if (context->bw_ctx.dml.vba.StutterPeriod < 
in_dc->debug.minimum_z8_residency_time &&
+   cstate_enter_plus_exit_z8_ns < 
in_dc->debug.minimum_z8_residency_time * 1000)
+   cstate_enter_plus_exit_z8_ns = 
in_dc->debug.minimum_z8_residency_time * 1000;
+
+   
context->bw_ctx.bw.dcn.watermarks.a.cstate_pstate.cstate_enter_plus_exit_z8_ns 
= cstate_enter_plus_exit_z8_ns;
}
 
return result;
-- 
2.34.1



[PATCH 14/22] drm/amd/display: Allow Z8 when stutter threshold is not met

2024-03-20 Thread Tom Chung
From: Bhawanpreet Lakha 

[Why]
Some panels don't meet the stutter threshold (4k etc), this leads to
power regressions. Allow z8 for panels that don't meet the threshold
but support PSR/replay

Reviewed-by: Nicholas Kazlauskas 
Acked-by: Tom Chung 
Signed-off-by: Bhawanpreet Lakha 
---
 drivers/gpu/drm/amd/display/dc/dml/dcn20/dcn20_fpu.c |  9 +++--
 drivers/gpu/drm/amd/display/dc/dml/dcn31/dcn31_fpu.c | 10 +-
 2 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/dml/dcn20/dcn20_fpu.c 
b/drivers/gpu/drm/amd/display/dc/dml/dcn20/dcn20_fpu.c
index 38ab9ad60ef8..9c93a9f1eb0c 100644
--- a/drivers/gpu/drm/amd/display/dc/dml/dcn20/dcn20_fpu.c
+++ b/drivers/gpu/drm/amd/display/dc/dml/dcn20/dcn20_fpu.c
@@ -1085,6 +1085,9 @@ static enum dcn_zstate_support_state  
decide_zstate_support(struct dc *dc, struc
int minmum_z8_residency = dc->debug.minimum_z8_residency_time > 
0 ? dc->debug.minimum_z8_residency_time : 1000;
bool allow_z8 = context->bw_ctx.dml.vba.StutterPeriod > 
(double)minmum_z8_residency;
bool is_pwrseq0 = link->link_index == 0;
+   bool is_psr = (link && (link->psr_settings.psr_version == 
DC_PSR_VERSION_1 ||
+   link->psr_settings.psr_version 
== DC_PSR_VERSION_SU_1) && !link->panel_config.psr.disable_psr);
+   bool is_replay = link && 
link->replay_settings.replay_feature_enabled;
 
/* Don't support multi-plane configurations */
if (stream_status->plane_count > 1)
@@ -1092,13 +1095,15 @@ static enum dcn_zstate_support_state  
decide_zstate_support(struct dc *dc, struc
 
if (is_pwrseq0 && context->bw_ctx.dml.vba.StutterPeriod > 
5000.0)
return DCN_ZSTATE_SUPPORT_ALLOW;
-   else if (is_pwrseq0 && link->psr_settings.psr_version == 
DC_PSR_VERSION_1 && !link->panel_config.psr.disable_psr)
-   return allow_z8 ? DCN_ZSTATE_SUPPORT_ALLOW_Z8_Z10_ONLY 
: DCN_ZSTATE_SUPPORT_ALLOW_Z10_ONLY;
+   else if (is_pwrseq0 && (is_psr || is_replay))
+   return DCN_ZSTATE_SUPPORT_ALLOW_Z8_Z10_ONLY;
else
return allow_z8 ? DCN_ZSTATE_SUPPORT_ALLOW_Z8_ONLY : 
DCN_ZSTATE_SUPPORT_DISALLOW;
} else {
return DCN_ZSTATE_SUPPORT_DISALLOW;
}
+
+   return DCN_ZSTATE_SUPPORT_DISALLOW;
 }
 
 static void dcn20_adjust_freesync_v_startup(
diff --git a/drivers/gpu/drm/amd/display/dc/dml/dcn31/dcn31_fpu.c 
b/drivers/gpu/drm/amd/display/dc/dml/dcn31/dcn31_fpu.c
index deb6d162a2d5..59a902313200 100644
--- a/drivers/gpu/drm/amd/display/dc/dml/dcn31/dcn31_fpu.c
+++ b/drivers/gpu/drm/amd/display/dc/dml/dcn31/dcn31_fpu.c
@@ -485,6 +485,7 @@ void dcn31_calculate_wm_and_dlg_fp(
 {
int i, pipe_idx, total_det = 0, active_hubp_count = 0;
double dcfclk = 
context->bw_ctx.dml.vba.DCFCLKState[vlevel][context->bw_ctx.dml.vba.maxMpcComb];
+   uint32_t cstate_enter_plus_exit_z8_ns;
 
dc_assert_fp_enabled();
 
@@ -504,6 +505,13 @@ void dcn31_calculate_wm_and_dlg_fp(
pipes[0].clks_cfg.dcfclk_mhz = dcfclk;
pipes[0].clks_cfg.socclk_mhz = 
context->bw_ctx.dml.soc.clock_limits[vlevel].socclk_mhz;
 
+   cstate_enter_plus_exit_z8_ns =
+   get_wm_z8_stutter_enter_exit(>bw_ctx.dml, pipes, 
pipe_cnt) * 1000;
+
+   if (get_stutter_period(>bw_ctx.dml, pipes, pipe_cnt) < 
dc->debug.minimum_z8_residency_time &&
+   cstate_enter_plus_exit_z8_ns < 
dc->debug.minimum_z8_residency_time * 1000)
+   cstate_enter_plus_exit_z8_ns = 
dc->debug.minimum_z8_residency_time * 1000;
+
/* Set A:
 * All clocks min required
 *
@@ -514,7 +522,7 @@ void dcn31_calculate_wm_and_dlg_fp(

context->bw_ctx.bw.dcn.watermarks.a.cstate_pstate.cstate_enter_plus_exit_ns = 
get_wm_stutter_enter_exit(>bw_ctx.dml, pipes, pipe_cnt) * 1000;
context->bw_ctx.bw.dcn.watermarks.a.cstate_pstate.cstate_exit_ns = 
get_wm_stutter_exit(>bw_ctx.dml, pipes, pipe_cnt) * 1000;
context->bw_ctx.bw.dcn.watermarks.a.cstate_pstate.pstate_change_ns = 
get_wm_dram_clock_change(>bw_ctx.dml, pipes, pipe_cnt) * 1000;
-   
context->bw_ctx.bw.dcn.watermarks.a.cstate_pstate.cstate_enter_plus_exit_z8_ns 
= get_wm_z8_stutter_enter_exit(>bw_ctx.dml, pipes, pipe_cnt) * 1000;
+   
context->bw_ctx.bw.dcn.watermarks.a.cstate_pstate.cstate_enter_plus_exit_z8_ns 
= cstate_enter_plus_exit_z8_ns;
context->bw_ctx.bw.dcn.watermarks.a.cstate_pstate.cstate_exit_z8_ns = 
get_wm_z8_stutter_exit(>bw_ctx.dml, pipes, pipe_cnt) * 1000;
context->bw_ctx.bw.dcn.watermarks.a.pte_meta_urgent_ns = 
get_wm_memory_trip(>bw_ctx.dml, pipes, pipe_cnt) * 1000;
context->bw_ctx.bw.dcn.watermarks.a.frac_urg_bw_nom = 
get_fraction_of_urgent_bandwidth(>bw_ctx.dml, pipes, pipe_cnt) * 1000;
-- 
2.34.1



[PATCH 13/22] drm/amd/display: Add new IPS config mode

2024-03-20 Thread Tom Chung
From: Nicholas Kazlauskas 

[Why]
We don't have a way to specify IPS2 for display off but RCG only for
static screen and local video playback.

[How]
Add a new setting that allows RCG only when displays are active but
IPS2 when all displays are off.

Reviewed-by: Ovidiu Bunea 
Acked-by: Tom Chung 
Signed-off-by: Nicholas Kazlauskas 
---
 drivers/gpu/drm/amd/display/dc/dc_dmub_srv.c  | 29 +++
 .../gpu/drm/amd/display/dmub/inc/dmub_cmd.h   |  1 +
 2 files changed, 30 insertions(+)

diff --git a/drivers/gpu/drm/amd/display/dc/dc_dmub_srv.c 
b/drivers/gpu/drm/amd/display/dc/dc_dmub_srv.c
index 12c142cae78b..9ae0e602e737 100644
--- a/drivers/gpu/drm/amd/display/dc/dc_dmub_srv.c
+++ b/drivers/gpu/drm/amd/display/dc/dc_dmub_srv.c
@@ -1201,6 +1201,20 @@ bool dc_dmub_srv_is_hw_pwr_up(struct dc_dmub_srv 
*dc_dmub_srv, bool wait)
return true;
 }
 
+static int count_active_streams(const struct dc *dc)
+{
+   int i, count = 0;
+
+   for (i = 0; i < dc->current_state->stream_count; ++i) {
+   struct dc_stream_state *stream = dc->current_state->streams[i];
+
+   if (stream && !stream->dpms_off)
+   count += 1;
+   }
+
+   return count;
+}
+
 static void dc_dmub_srv_notify_idle(const struct dc *dc, bool allow_idle)
 {
volatile const struct dmub_shared_state_ips_fw *ips_fw;
@@ -1255,6 +1269,21 @@ static void dc_dmub_srv_notify_idle(const struct dc *dc, 
bool allow_idle)
new_signals.bits.allow_pg = 1;
new_signals.bits.allow_ips1 = 1;
new_signals.bits.allow_ips2 = 1;
+   } else if (dc->config.disable_ips == 
DMUB_IPS_RCG_IN_ACTIVE_IPS2_IN_OFF) {
+   /* TODO: Move this logic out to hwseq */
+   if (count_active_streams(dc) == 0) {
+   /* IPS2 - Display off */
+   new_signals.bits.allow_pg = 1;
+   new_signals.bits.allow_ips1 = 1;
+   new_signals.bits.allow_ips2 = 1;
+   new_signals.bits.allow_z10 = 1;
+   } else {
+   /* RCG only */
+   new_signals.bits.allow_pg = 0;
+   new_signals.bits.allow_ips1 = 1;
+   new_signals.bits.allow_ips2 = 0;
+   new_signals.bits.allow_z10 = 0;
+   }
}
 
ips_driver->signals = new_signals;
diff --git a/drivers/gpu/drm/amd/display/dmub/inc/dmub_cmd.h 
b/drivers/gpu/drm/amd/display/dmub/inc/dmub_cmd.h
index 34cb25c6166a..995a54459335 100644
--- a/drivers/gpu/drm/amd/display/dmub/inc/dmub_cmd.h
+++ b/drivers/gpu/drm/amd/display/dmub/inc/dmub_cmd.h
@@ -624,6 +624,7 @@ enum dmub_ips_disable_type {
DMUB_IPS_DISABLE_IPS2 = 3,
DMUB_IPS_DISABLE_IPS2_Z10 = 4,
DMUB_IPS_DISABLE_DYNAMIC = 5,
+   DMUB_IPS_RCG_IN_ACTIVE_IPS2_IN_OFF = 6,
 };
 
 #define DMUB_IPS1_ALLOW_MASK 0x0001
-- 
2.34.1



[PATCH 12/22] drm/amd/display: Fix bounds check for dcn35 DcfClocks

2024-03-20 Thread Tom Chung
From: Roman Li 

[Why]
NumFclkLevelsEnabled is used for DcfClocks bounds check
instead of designated NumDcfClkLevelsEnabled.
That can cause array index out-of-bounds access.

[How]
Use designated variable for dcn35 DcfClocks bounds check.

Fixes: 6b8d9862159f ("drm/amd/display: Fix array-index-out-of-bounds in 
dcn35_clkmgr")

Cc: Mario Limonciello 
Cc: Alex Deucher 
Cc: sta...@vger.kernel.org
Reviewed-by: Sun peng Li 
Acked-by: Tom Chung 
Signed-off-by: Roman Li 
---
 drivers/gpu/drm/amd/display/dc/clk_mgr/dcn35/dcn35_clk_mgr.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn35/dcn35_clk_mgr.c 
b/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn35/dcn35_clk_mgr.c
index 928d61f9f858..8efde1cfb49a 100644
--- a/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn35/dcn35_clk_mgr.c
+++ b/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn35/dcn35_clk_mgr.c
@@ -715,7 +715,7 @@ static void dcn35_clk_mgr_helper_populate_bw_params(struct 
clk_mgr_internal *clk
clock_table->NumFclkLevelsEnabled;
max_fclk = find_max_clk_value(clock_table->FclkClocks_Freq, num_fclk);
 
-   num_dcfclk = (clock_table->NumFclkLevelsEnabled > 
NUM_DCFCLK_DPM_LEVELS) ? NUM_DCFCLK_DPM_LEVELS :
+   num_dcfclk = (clock_table->NumDcfClkLevelsEnabled > 
NUM_DCFCLK_DPM_LEVELS) ? NUM_DCFCLK_DPM_LEVELS :
clock_table->NumDcfClkLevelsEnabled;
for (i = 0; i < num_dcfclk; i++) {
int j;
-- 
2.34.1



[PATCH 10/22] drm/amd/display: build scaling params when a new plane is appended

2024-03-20 Thread Tom Chung
From: Wenjing Liu 

[why & how]
We are boundling changes in plane state and build scaling params
together. This is to simplify DML code so DML doesn't need to build
scaling params. We are also avoiding rebuilding scaling params for
planes without scaling changes.

Reviewed-by: Dillon Varone 
Acked-by: Tom Chung 
Signed-off-by: Wenjing Liu 
---
 drivers/gpu/drm/amd/display/dc/core/dc.c  |  5 -
 drivers/gpu/drm/amd/display/dc/core/dc_resource.c | 10 --
 2 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/core/dc.c 
b/drivers/gpu/drm/amd/display/dc/core/dc.c
index 726c82be16fa..5e93bd696d21 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc.c
@@ -3236,7 +3236,10 @@ static bool update_planes_and_stream_state(struct dc *dc,
for (i = 0; i < surface_count; i++) {
struct dc_plane_state *surface = srf_updates[i].surface;
 
-   if (update_type >= UPDATE_TYPE_MED) {
+   if (update_type != UPDATE_TYPE_MED)
+   continue;
+   if (surface->update_flags.bits.clip_size_change ||
+   surface->update_flags.bits.position_change) {
for (j = 0; j < dc->res_pool->pipe_count; j++) {
struct pipe_ctx *pipe_ctx = 
>res_ctx.pipe_ctx[j];
 
diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_resource.c 
b/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
index fa6e6184c437..2b01b4a3861d 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
@@ -2649,13 +2649,19 @@ bool resource_append_dpp_pipes_for_plane_composition(
struct pipe_ctx *otg_master_pipe,
struct dc_plane_state *plane_state)
 {
+   bool success;
if (otg_master_pipe->plane_state == NULL)
-   return add_plane_to_opp_head_pipes(otg_master_pipe,
+   success = add_plane_to_opp_head_pipes(otg_master_pipe,
plane_state, new_ctx);
else
-   return acquire_secondary_dpp_pipes_and_add_plane(
+   success = acquire_secondary_dpp_pipes_and_add_plane(
otg_master_pipe, plane_state, new_ctx,
cur_ctx, pool);
+   if (success)
+   /* when appending a plane mpc slice count changes from 0 to 1 */
+   success = update_pipe_params_after_mpc_slice_count_change(
+   plane_state, new_ctx, pool);
+   return success;
 }
 
 void resource_remove_dpp_pipes_for_plane_composition(
-- 
2.34.1



[PATCH 11/22] drm/amd/display: Remove MPC rate control logic from DCN30 and above

2024-03-20 Thread Tom Chung
From: George Shen 

[Why]
MPC flow rate control is not needed for DCN30 and above. Current logic
that uses it can result in underflow for certain edge cases (such as
DSC N422 + ODM combine + 422 left edge pixel).

[How]
Remove MPC flow rate control logic and programming for DCN30 and above.

Cc: Mario Limonciello 
Cc: Alex Deucher 
Cc: sta...@vger.kernel.org
Reviewed-by: Wenjing Liu 
Acked-by: Tom Chung 
Signed-off-by: George Shen 
---
 .../gpu/drm/amd/display/dc/dcn30/dcn30_mpc.c  | 54 +++
 .../gpu/drm/amd/display/dc/dcn30/dcn30_mpc.h  | 14 ++---
 .../gpu/drm/amd/display/dc/dcn32/dcn32_mpc.c  |  5 +-
 .../amd/display/dc/hwss/dcn314/dcn314_hwseq.c | 41 --
 .../amd/display/dc/hwss/dcn32/dcn32_hwseq.c   | 41 --
 .../amd/display/dc/hwss/dcn35/dcn35_hwseq.c   | 41 --
 6 files changed, 41 insertions(+), 155 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_mpc.c 
b/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_mpc.c
index bf3386cd444d..5ebb57303130 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_mpc.c
+++ b/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_mpc.c
@@ -44,6 +44,36 @@
 #define NUM_ELEMENTS(a) (sizeof(a) / sizeof((a)[0]))
 
 
+void mpc3_mpc_init(struct mpc *mpc)
+{
+   struct dcn30_mpc *mpc30 = TO_DCN30_MPC(mpc);
+   int opp_id;
+
+   mpc1_mpc_init(mpc);
+
+   for (opp_id = 0; opp_id < MAX_OPP; opp_id++) {
+   if (REG(MUX[opp_id]))
+   /* disable mpc out rate and flow control */
+   REG_UPDATE_2(MUX[opp_id], MPC_OUT_RATE_CONTROL_DISABLE,
+   1, MPC_OUT_FLOW_CONTROL_COUNT, 0);
+   }
+}
+
+void mpc3_mpc_init_single_inst(struct mpc *mpc, unsigned int mpcc_id)
+{
+   struct dcn30_mpc *mpc30 = TO_DCN30_MPC(mpc);
+
+   mpc1_mpc_init_single_inst(mpc, mpcc_id);
+
+   /* assuming mpc out mux is connected to opp with the same index at this
+* point in time (e.g. transitioning from vbios to driver)
+*/
+   if (mpcc_id < MAX_OPP && REG(MUX[mpcc_id]))
+   /* disable mpc out rate and flow control */
+   REG_UPDATE_2(MUX[mpcc_id], MPC_OUT_RATE_CONTROL_DISABLE,
+   1, MPC_OUT_FLOW_CONTROL_COUNT, 0);
+}
+
 bool mpc3_is_dwb_idle(
struct mpc *mpc,
int dwb_id)
@@ -80,25 +110,6 @@ void mpc3_disable_dwb_mux(
MPC_DWB0_MUX, 0xf);
 }
 
-void mpc3_set_out_rate_control(
-   struct mpc *mpc,
-   int opp_id,
-   bool enable,
-   bool rate_2x_mode,
-   struct mpc_dwb_flow_control *flow_control)
-{
-   struct dcn30_mpc *mpc30 = TO_DCN30_MPC(mpc);
-
-   REG_UPDATE_2(MUX[opp_id],
-   MPC_OUT_RATE_CONTROL_DISABLE, !enable,
-   MPC_OUT_RATE_CONTROL, rate_2x_mode);
-
-   if (flow_control)
-   REG_UPDATE_2(MUX[opp_id],
-   MPC_OUT_FLOW_CONTROL_MODE, flow_control->flow_ctrl_mode,
-   MPC_OUT_FLOW_CONTROL_COUNT, 
flow_control->flow_ctrl_cnt1);
-}
-
 enum dc_lut_mode mpc3_get_ogam_current(struct mpc *mpc, int mpcc_id)
 {
/*Contrary to DCN2 and DCN1 wherein a single status register field 
holds this info;
@@ -1490,8 +1501,8 @@ static const struct mpc_funcs dcn30_mpc_funcs = {
.read_mpcc_state = mpc3_read_mpcc_state,
.insert_plane = mpc1_insert_plane,
.remove_mpcc = mpc1_remove_mpcc,
-   .mpc_init = mpc1_mpc_init,
-   .mpc_init_single_inst = mpc1_mpc_init_single_inst,
+   .mpc_init = mpc3_mpc_init,
+   .mpc_init_single_inst = mpc3_mpc_init_single_inst,
.update_blending = mpc2_update_blending,
.cursor_lock = mpc1_cursor_lock,
.get_mpcc_for_dpp = mpc1_get_mpcc_for_dpp,
@@ -1508,7 +1519,6 @@ static const struct mpc_funcs dcn30_mpc_funcs = {
.set_dwb_mux = mpc3_set_dwb_mux,
.disable_dwb_mux = mpc3_disable_dwb_mux,
.is_dwb_idle = mpc3_is_dwb_idle,
-   .set_out_rate_control = mpc3_set_out_rate_control,
.set_gamut_remap = mpc3_set_gamut_remap,
.program_shaper = mpc3_program_shaper,
.acquire_rmu = mpcc3_acquire_rmu,
diff --git a/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_mpc.h 
b/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_mpc.h
index 9cb96ae95a2f..ce93003dae01 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_mpc.h
+++ b/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_mpc.h
@@ -1007,6 +1007,13 @@ void dcn30_mpc_construct(struct dcn30_mpc *mpc30,
int num_mpcc,
int num_rmu);
 
+void mpc3_mpc_init(
+   struct mpc *mpc);
+
+void mpc3_mpc_init_single_inst(
+   struct mpc *mpc,
+   unsigned int mpcc_id);
+
 bool mpc3_program_shaper(
struct mpc *mpc,
const struct pwl_params *params,
@@ -1078,13 +1085,6 @@ bool mpc3_is_dwb_idle(
struct mpc *mpc,
int dwb_id);
 
-void mpc3_set_out_rate_control(
-   struct mpc *mpc,
-   int opp_id,
-   

[PATCH 09/22] drm/amd/display: fix nonseamless transition from ODM + MPO to ODM + subvp

2024-03-20 Thread Tom Chung
From: Wenjing Liu 

[why]
 when ODM + MPO is used for all 4 available pipes. Pipe transition will
 be nonseamless. Phantom OTG master pipe reuses the secondary OPP head
 pipe. There is no possible seamless path to transit to the new
 state. The correct logic would be to reuse a secondary DPP pipe as the
 phantom OTG master pipe. This way we are able to first transit the
 minimal transtion state of new and then transit to new state seamlessly.

 current  New (nonseamless)
  
 | plane0  slice0  stream0|   | plane0  slice0  stream0|
 |DPP0OPP0OTG0|   |DPP0OPP0OTG0|
 | plane1 |   |   |   | plane0  slice1 |   |
 |DPP2|   |   |   |DPP2OPP2|   |
 | plane0  slice1 |   |   | plane0  slice0  stream1|
 |DPP1OPP1|   |   |DPP1OPP1OTG1|
 | plane1 |   |   | plane0  slice1 |   |
 |DPP3|   |   |DPP3OPP3|   |
 ||   ||

 New (seamless)   New (minimal transition)
  
 | plane0  slice0  stream0|   | plane0  slice0  stream0|
 |DPP0OPP0OTG0|   |DPP0OPP0OTG0|
 | plane0  slice1 |   |   | plane0  slice1 |   |
 |DPP1OPP1|   |   |DPP1OPP1|   |
 | plane0  slice0  stream1|   ||
 |DPP2OPP2OTG2|
 | plane0  slice1 |   |
 |DPP3OPP3|   |
 ||

[how]
Try to acquire free pipes used as secondary DPP pipes from current state
before try to acquire any free pipes for new OTG master pipe.

Reviewed-by: Alvin Lee 
Acked-by: Tom Chung 
Signed-off-by: Wenjing Liu 
---
 .../gpu/drm/amd/display/dc/core/dc_resource.c | 44 +++
 drivers/gpu/drm/amd/display/dc/inc/resource.h | 11 +
 2 files changed, 55 insertions(+)

diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_resource.c 
b/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
index ce56e47cfd0a..fa6e6184c437 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
@@ -1795,6 +1795,30 @@ int 
recource_find_free_pipe_used_as_otg_master_in_cur_res_ctx(
return free_pipe_idx;
 }
 
+int resource_find_free_pipe_used_as_cur_sec_dpp(
+   const struct resource_context *cur_res_ctx,
+   struct resource_context *new_res_ctx,
+   const struct resource_pool *pool)
+{
+   int free_pipe_idx = FREE_PIPE_INDEX_NOT_FOUND;
+   const struct pipe_ctx *new_pipe, *cur_pipe;
+   int i;
+
+   for (i = 0; i < pool->pipe_count; i++) {
+   cur_pipe = _res_ctx->pipe_ctx[i];
+   new_pipe = _res_ctx->pipe_ctx[i];
+
+   if (resource_is_pipe_type(cur_pipe, DPP_PIPE) &&
+   !resource_is_pipe_type(cur_pipe, OPP_HEAD) &&
+   resource_is_pipe_type(new_pipe, FREE_PIPE)) {
+   free_pipe_idx = i;
+   break;
+   }
+   }
+
+   return free_pipe_idx;
+}
+
 int resource_find_free_pipe_used_as_cur_sec_dpp_in_mpcc_combine(
const struct resource_context *cur_res_ctx,
struct resource_context *new_res_ctx,
@@ -3372,11 +3396,31 @@ static bool acquire_otg_master_pipe_for_stream(
 * any free pipes already used in current context as this could tear
 * down exiting ODM/MPC/MPO configuration unnecessarily.
 */
+
+   /*
+* Try to acquire the same OTG master already in use. This is not
+* optimal because resetting an enabled OTG master pipe for a new stream
+* requires an extra frame of wait. However there are test automation
+* and eDP assumptions that rely on reusing the same OTG master pipe
+* during mode change. We have to keep this logic as is for now.
+*/
pipe_idx = recource_find_free_pipe_used_as_otg_master_in_cur_res_ctx(
_ctx->res_ctx, _ctx->res_ctx, pool);
+   /*
+* Try to acquire a pipe not used in current resource context to avoid
+* pipe swapping.
+*/
if (pipe_idx == FREE_PIPE_INDEX_NOT_FOUND)
pipe_idx = recource_find_free_pipe_not_used_in_cur_res_ctx(
_ctx->res_ctx, _ctx->res_ctx, pool);
+   /*
+* If pipe swapping is unavoidable, try to acquire pipe used as
+* secondary DPP pipe in current state as we prioritize to support more
+* streams over supporting MPO planes.
+*/
+   if (pipe_idx == FREE_PIPE_INDEX_NOT_FOUND)
+   pipe_idx = resource_find_free_pipe_used_as_cur_sec_dpp(
+   _ctx->res_ctx, _ctx->res_ctx, pool);
if (pipe_idx == FREE_PIPE_INDEX_NOT_FOUND)
pipe_idx = 

[PATCH 07/22] drm/amd/display: Added missing null checks

2024-03-20 Thread Tom Chung
From: Sohaib Nadeem 

[why]
Add the missing null check before dereference for dc_stream_status*

Fixes: 06653fc22752 ("drm/amd/display: Implement update_planes_and_stream_v3 
sequence")
Reviewed-by: Josip Pavic 
Acked-by: Tom Chung 
Signed-off-by: Sohaib Nadeem 
---
 drivers/gpu/drm/amd/display/dc/core/dc.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/amd/display/dc/core/dc.c 
b/drivers/gpu/drm/amd/display/dc/core/dc.c
index cc4fd4e0deb6..dd9fe36646a3 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc.c
@@ -4291,6 +4291,9 @@ static int initialize_empty_surface_updates(
struct dc_stream_status *status = dc_stream_get_status(stream);
int i;
 
+   if (!status)
+   return 0;
+
for (i = 0; i < status->plane_count; i++)
srf_updates[i].surface = status->plane_states[i];
 
-- 
2.34.1



[PATCH 08/22] drm/amd/display: fix a dereference of a NULL pointer

2024-03-20 Thread Tom Chung
From: Wenjing Liu 

[why]
In some platform out_transfer_func may not be popualted. We need to check
for null before dereferencing it.

Fixes: 4b939f625e9b ("drm/amd/display: Generalize new minimal transition path")
Reviewed-by: Alvin Lee 
Acked-by: Tom Chung 
Signed-off-by: Wenjing Liu 
---
 drivers/gpu/drm/amd/display/dc/core/dc.c | 6 --
 1 file changed, 4 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 dd9fe36646a3..726c82be16fa 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc.c
@@ -3057,7 +3057,8 @@ static void backup_planes_and_stream_state(
scratch->blend_tf[i] = *status->plane_states[i]->blend_tf;
}
scratch->stream_state = *stream;
-   scratch->out_transfer_func = *stream->out_transfer_func;
+   if (stream->out_transfer_func)
+   scratch->out_transfer_func = *stream->out_transfer_func;
 }
 
 static void restore_planes_and_stream_state(
@@ -3079,7 +3080,8 @@ static void restore_planes_and_stream_state(
*status->plane_states[i]->blend_tf = scratch->blend_tf[i];
}
*stream = scratch->stream_state;
-   *stream->out_transfer_func = scratch->out_transfer_func;
+   if (stream->out_transfer_func)
+   *stream->out_transfer_func = scratch->out_transfer_func;
 }
 
 /**
-- 
2.34.1



[PATCH 06/22] drm/amd/display: Refactor DML2 interfaces

2024-03-20 Thread Tom Chung
From: Dillon Varone 

[Why}
Some interfaces needed changes to support future architectures.

Reviewed-by: Chaitanya Dhere 
Acked-by: Tom Chung 
Signed-off-by: Dillon Varone 
---
 .../gpu/drm/amd/display/dc/core/dc_state.c| 14 +--
 .../gpu/drm/amd/display/dc/core/dc_surface.c  |  2 +-
 drivers/gpu/drm/amd/display/dc/dc.h   |  1 +
 drivers/gpu/drm/amd/display/dc/dc_plane.h |  2 +-
 drivers/gpu/drm/amd/display/dc/dc_state.h |  4 ++--
 .../gpu/drm/amd/display/dc/dc_state_priv.h| 10 
 .../display/dc/dml2/dml2_dc_resource_mgmt.c   | 23 +++
 .../drm/amd/display/dc/dml2/dml2_wrapper.c|  2 +-
 .../drm/amd/display/dc/dml2/dml2_wrapper.h| 12 +-
 9 files changed, 37 insertions(+), 33 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_state.c 
b/drivers/gpu/drm/amd/display/dc/core/dc_state.c
index c5c078771b15..d546ea71026d 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc_state.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_state.c
@@ -340,7 +340,7 @@ void dc_state_release(struct dc_state *state)
  * dc_state_add_stream() - Add a new dc_stream_state to a dc_state.
  */
 enum dc_status dc_state_add_stream(
-   struct dc *dc,
+   const struct dc *dc,
struct dc_state *state,
struct dc_stream_state *stream)
 {
@@ -369,7 +369,7 @@ enum dc_status dc_state_add_stream(
  * dc_state_remove_stream() - Remove a stream from a dc_state.
  */
 enum dc_status dc_state_remove_stream(
-   struct dc *dc,
+   const struct dc *dc,
struct dc_state *state,
struct dc_stream_state *stream)
 {
@@ -679,7 +679,7 @@ void dc_state_release_phantom_stream(const struct dc *dc,
dc_stream_release(phantom_stream);
 }
 
-struct dc_plane_state *dc_state_create_phantom_plane(struct dc *dc,
+struct dc_plane_state *dc_state_create_phantom_plane(const struct dc *dc,
struct dc_state *state,
struct dc_plane_state *main_plane)
 {
@@ -715,7 +715,7 @@ void dc_state_release_phantom_plane(const struct dc *dc,
 }
 
 /* add phantom streams to context and generate correct meta inside dc_state */
-enum dc_status dc_state_add_phantom_stream(struct dc *dc,
+enum dc_status dc_state_add_phantom_stream(const struct dc *dc,
struct dc_state *state,
struct dc_stream_state *phantom_stream,
struct dc_stream_state *main_stream)
@@ -741,7 +741,7 @@ enum dc_status dc_state_add_phantom_stream(struct dc *dc,
return res;
 }
 
-enum dc_status dc_state_remove_phantom_stream(struct dc *dc,
+enum dc_status dc_state_remove_phantom_stream(const struct dc *dc,
struct dc_state *state,
struct dc_stream_state *phantom_stream)
 {
@@ -835,7 +835,7 @@ bool dc_state_add_all_phantom_planes_for_stream(
 }
 
 bool dc_state_remove_phantom_streams_and_planes(
-   struct dc *dc,
+   const struct dc *dc,
struct dc_state *state)
 {
int i;
@@ -857,7 +857,7 @@ bool dc_state_remove_phantom_streams_and_planes(
 }
 
 void dc_state_release_phantom_streams_and_planes(
-   struct dc *dc,
+   const struct dc *dc,
struct dc_state *state)
 {
int i;
diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_surface.c 
b/drivers/gpu/drm/amd/display/dc/core/dc_surface.c
index 19140fb65787..acc7a8baa169 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc_surface.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_surface.c
@@ -103,7 +103,7 @@ void enable_surface_flip_reporting(struct dc_plane_state 
*plane_state,
/*register_flip_interrupt(surface);*/
 }
 
-struct dc_plane_state *dc_create_plane_state(struct dc *dc)
+struct dc_plane_state *dc_create_plane_state(const struct dc *dc)
 {
struct dc_plane_state *plane_state = kvzalloc(sizeof(*plane_state),
GFP_KERNEL);
diff --git a/drivers/gpu/drm/amd/display/dc/dc.h 
b/drivers/gpu/drm/amd/display/dc/dc.h
index e0b44c43e959..52fd2ebdcdbb 100644
--- a/drivers/gpu/drm/amd/display/dc/dc.h
+++ b/drivers/gpu/drm/amd/display/dc/dc.h
@@ -996,6 +996,7 @@ struct dc_debug_options {
bool enable_idle_reg_checks;
unsigned int static_screen_wait_frames;
bool force_chroma_subsampling_1tap;
+   bool disable_422_left_edge_pixel;
 };
 
 struct gpu_info_soc_bounding_box_v1_0;
diff --git a/drivers/gpu/drm/amd/display/dc/dc_plane.h 
b/drivers/gpu/drm/amd/display/dc/dc_plane.h
index ef380cae816a..44afcd989224 100644
--- a/drivers/gpu/drm/amd/display/dc/dc_plane.h
+++ b/drivers/gpu/drm/amd/display/dc/dc_plane.h
@@ -29,7 +29,7 @@
 #include "dc.h"
 #include "dc_hw_types.h"
 
-struct dc_plane_state *dc_create_plane_state(struct dc *dc);
+struct dc_plane_state *dc_create_plane_state(const struct dc *dc);
 const struct dc_plane_status *dc_plane_get_status(
const struct dc_plane_state *plane_state);
 void 

[PATCH 05/22] drm/amd/display: Expand DML2 callbacks

2024-03-20 Thread Tom Chung
From: Dillon Varone 

[Why]
These additional callbacks to DC will be required for the DML2 wrapper. Also
consolidate common callbacks for projects to a single location for maintenance.

Reviewed-by: Chaitanya Dhere 
Acked-by: Tom Chung 
Signed-off-by: Dillon Varone 
---
 .../gpu/drm/amd/display/dc/core/dc_resource.c | 33 +++
 .../gpu/drm/amd/display/dc/core/dc_state.c| 17 +++-
 drivers/gpu/drm/amd/display/dc/dc_state.h |  2 +-
 .../gpu/drm/amd/display/dc/dc_state_priv.h|  2 +
 .../display/dc/dcn32/dcn32_resource_helpers.c | 23 ++-
 .../drm/amd/display/dc/dml2/dml2_wrapper.c|  2 +-
 .../drm/amd/display/dc/dml2/dml2_wrapper.h| 24 ++-
 .../amd/display/dc/hwss/dcn32/dcn32_hwseq.c   |  4 +-
 .../gpu/drm/amd/display/dc/inc/core_types.h   |  3 ++
 drivers/gpu/drm/amd/display/dc/inc/resource.h |  6 +++
 .../dc/resource/dcn32/dcn32_resource.c| 41 +--
 .../dc/resource/dcn32/dcn32_resource.h|  6 +--
 .../dc/resource/dcn321/dcn321_resource.c  | 25 ++-
 .../dc/resource/dcn35/dcn35_resource.c| 10 +
 .../dc/resource/dcn351/dcn351_resource.c  | 10 +
 15 files changed, 119 insertions(+), 89 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_resource.c 
b/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
index 96b4f68ec374..ce56e47cfd0a 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
@@ -4993,3 +4993,36 @@ bool check_subvp_sw_cursor_fallback_req(const struct dc 
*dc, struct dc_stream_st
 
return false;
 }
+
+void resource_init_common_dml2_callbacks(struct dc *dc, struct 
dml2_configuration_options *dml2_options)
+{
+   dml2_options->callbacks.dc = dc;
+   dml2_options->callbacks.build_scaling_params = 
_build_scaling_params;
+   dml2_options->callbacks.acquire_secondary_pipe_for_mpc_odm = 
_resource_acquire_secondary_pipe_for_mpc_odm_legacy;
+   dml2_options->callbacks.update_pipes_for_stream_with_slice_count = 
_update_pipes_for_stream_with_slice_count;
+   dml2_options->callbacks.update_pipes_for_plane_with_slice_count = 
_update_pipes_for_plane_with_slice_count;
+   dml2_options->callbacks.get_mpc_slice_index = 
_get_mpc_slice_index;
+   dml2_options->callbacks.get_odm_slice_index = 
_get_odm_slice_index;
+   dml2_options->callbacks.get_opp_head = _get_opp_head;
+   dml2_options->callbacks.get_otg_master_for_stream = 
_get_otg_master_for_stream;
+   dml2_options->callbacks.get_opp_heads_for_otg_master = 
_get_opp_heads_for_otg_master;
+   dml2_options->callbacks.get_dpp_pipes_for_plane = 
_get_dpp_pipes_for_plane;
+   dml2_options->callbacks.get_stream_status = _state_get_stream_status;
+   dml2_options->callbacks.get_stream_from_id = 
_state_get_stream_from_id;
+
+   dml2_options->svp_pstate.callbacks.dc = dc;
+   dml2_options->svp_pstate.callbacks.add_phantom_plane = 
_state_add_phantom_plane;
+   dml2_options->svp_pstate.callbacks.add_phantom_stream = 
_state_add_phantom_stream;
+   dml2_options->svp_pstate.callbacks.build_scaling_params = 
_build_scaling_params;
+   dml2_options->svp_pstate.callbacks.create_phantom_plane = 
_state_create_phantom_plane;
+   dml2_options->svp_pstate.callbacks.remove_phantom_plane = 
_state_remove_phantom_plane;
+   dml2_options->svp_pstate.callbacks.remove_phantom_stream = 
_state_remove_phantom_stream;
+   dml2_options->svp_pstate.callbacks.create_phantom_stream = 
_state_create_phantom_stream;
+   dml2_options->svp_pstate.callbacks.release_phantom_plane = 
_state_release_phantom_plane;
+   dml2_options->svp_pstate.callbacks.release_phantom_stream = 
_state_release_phantom_stream;
+   dml2_options->svp_pstate.callbacks.get_pipe_subvp_type = 
_state_get_pipe_subvp_type;
+   dml2_options->svp_pstate.callbacks.get_stream_subvp_type = 
_state_get_stream_subvp_type;
+   dml2_options->svp_pstate.callbacks.get_paired_subvp_stream = 
_state_get_paired_subvp_stream;
+   dml2_options->svp_pstate.callbacks.remove_phantom_streams_and_planes = 
_state_remove_phantom_streams_and_planes;
+   dml2_options->svp_pstate.callbacks.release_phantom_streams_and_planes = 
_state_release_phantom_streams_and_planes;
+}
diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_state.c 
b/drivers/gpu/drm/amd/display/dc/core/dc_state.c
index cce4e1c465b6..c5c078771b15 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc_state.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_state.c
@@ -585,7 +585,7 @@ bool dc_state_add_all_planes_for_stream(
  */
 struct dc_stream_status *dc_state_get_stream_status(
struct dc_state *state,
-   struct dc_stream_state *stream)
+   const struct dc_stream_state *stream)
 {
uint8_t i;
 
@@ -868,3 +868,18 @@ void dc_state_release_phantom_streams_and_planes(
for (i = 0; i < state->phantom_plane_count; i++)

[PATCH 04/22] drm/amd/display: Remove read/write to external register

2024-03-20 Thread Tom Chung
From: Sung Joon Kim 

[why]
We need to remove the reference to these registers to
prevent any usage in the future.

Reviewed-by: Nicholas Kazlauskas 
Acked-by: Tom Chung 
Signed-off-by: Sung Joon Kim 
---
 .../display/dc/clk_mgr/dcn35/dcn35_clk_mgr.c  | 38 ---
 .../amd/display/dc/clk_mgr/dcn35/dcn35_smu.c  | 21 --
 .../amd/display/dc/clk_mgr/dcn35/dcn35_smu.h  |  2 -
 .../amd/display/dc/hwss/dcn35/dcn35_hwseq.c   | 16 
 .../amd/display/dc/hwss/dcn35/dcn35_hwseq.h   |  3 --
 .../amd/display/dc/hwss/dcn35/dcn35_init.c|  2 -
 .../amd/display/dc/hwss/dcn351/dcn351_init.c  |  2 -
 .../drm/amd/display/dc/hwss/hw_sequencer.h|  2 -
 .../gpu/drm/amd/display/dc/inc/hw/clk_mgr.h   |  2 -
 9 files changed, 88 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn35/dcn35_clk_mgr.c 
b/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn35/dcn35_clk_mgr.c
index dafa07671d00..928d61f9f858 100644
--- a/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn35/dcn35_clk_mgr.c
+++ b/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn35/dcn35_clk_mgr.c
@@ -842,35 +842,6 @@ static void dcn35_set_low_power_state(struct clk_mgr 
*clk_mgr_base)
}
 }
 
-static void dcn35_set_ips_idle_state(struct clk_mgr *clk_mgr_base, bool 
allow_idle)
-{
-   struct clk_mgr_internal *clk_mgr = TO_CLK_MGR_INTERNAL(clk_mgr_base);
-   struct dc *dc = clk_mgr_base->ctx->dc;
-   uint32_t val = dcn35_smu_read_ips_scratch(clk_mgr);
-
-   if (dc->config.disable_ips == DMUB_IPS_ENABLE ||
-   dc->config.disable_ips == DMUB_IPS_DISABLE_DYNAMIC) {
-   val = val & ~DMUB_IPS1_ALLOW_MASK;
-   val = val & ~DMUB_IPS2_ALLOW_MASK;
-   } else if (dc->config.disable_ips == DMUB_IPS_DISABLE_IPS1) {
-   val |= DMUB_IPS1_ALLOW_MASK;
-   val |= DMUB_IPS2_ALLOW_MASK;
-   } else if (dc->config.disable_ips == DMUB_IPS_DISABLE_IPS2) {
-   val = val & ~DMUB_IPS1_ALLOW_MASK;
-   val |= DMUB_IPS2_ALLOW_MASK;
-   } else if (dc->config.disable_ips == DMUB_IPS_DISABLE_IPS2_Z10) {
-   val = val & ~DMUB_IPS1_ALLOW_MASK;
-   val = val & ~DMUB_IPS2_ALLOW_MASK;
-   }
-
-   if (!allow_idle) {
-   val |= DMUB_IPS1_ALLOW_MASK;
-   val |= DMUB_IPS2_ALLOW_MASK;
-   }
-
-   dcn35_smu_write_ips_scratch(clk_mgr, val);
-}
-
 static void dcn35_exit_low_power_state(struct clk_mgr *clk_mgr_base)
 {
struct clk_mgr_internal *clk_mgr = TO_CLK_MGR_INTERNAL(clk_mgr_base);
@@ -890,13 +861,6 @@ static bool dcn35_is_ips_supported(struct clk_mgr 
*clk_mgr_base)
return ips_supported;
 }
 
-static uint32_t dcn35_get_ips_idle_state(struct clk_mgr *clk_mgr_base)
-{
-   struct clk_mgr_internal *clk_mgr = TO_CLK_MGR_INTERNAL(clk_mgr_base);
-
-   return dcn35_smu_read_ips_scratch(clk_mgr);
-}
-
 static void dcn35_init_clocks_fpga(struct clk_mgr *clk_mgr)
 {
init_clk_states(clk_mgr);
@@ -984,8 +948,6 @@ static struct clk_mgr_funcs dcn35_funcs = {
.set_low_power_state = dcn35_set_low_power_state,
.exit_low_power_state = dcn35_exit_low_power_state,
.is_ips_supported = dcn35_is_ips_supported,
-   .set_idle_state = dcn35_set_ips_idle_state,
-   .get_idle_state = dcn35_get_ips_idle_state
 };
 
 struct clk_mgr_funcs dcn35_fpga_funcs = {
diff --git a/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn35/dcn35_smu.c 
b/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn35/dcn35_smu.c
index 9e588c56c570..1399b41dfd1c 100644
--- a/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn35/dcn35_smu.c
+++ b/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn35/dcn35_smu.c
@@ -487,24 +487,3 @@ int dcn35_smu_get_ips_supported(struct clk_mgr_internal 
*clk_mgr)
//smu_print("%s: VBIOSSMC_MSG_QueryIPS2Support return = %x\n", 
__func__, retv);
return retv;
 }
-
-void dcn35_smu_write_ips_scratch(struct clk_mgr_internal *clk_mgr, uint32_t 
param)
-{
-   if (!clk_mgr->smu_present)
-   return;
-
-   REG_WRITE(MP1_SMN_C2PMSG_71, param);
-   //smu_print("%s: write_ips_scratch = %x\n", __func__, param);
-}
-
-uint32_t dcn35_smu_read_ips_scratch(struct clk_mgr_internal *clk_mgr)
-{
-   uint32_t retv;
-
-   if (!clk_mgr->smu_present)
-   return 0;
-
-   retv = REG_READ(MP1_SMN_C2PMSG_71);
-   //smu_print("%s: dcn35_smu_read_ips_scratch = %x\n",  __func__, retv);
-   return retv;
-}
diff --git a/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn35/dcn35_smu.h 
b/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn35/dcn35_smu.h
index 2b8e6959a03d..06cd3cc6d36e 100644
--- a/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn35/dcn35_smu.h
+++ b/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn35/dcn35_smu.h
@@ -198,6 +198,4 @@ int dcn35_smu_exit_low_power_state(struct clk_mgr_internal 
*clk_mgr);
 int dcn35_smu_get_ips_supported(struct clk_mgr_internal *clk_mgr);
 int dcn35_smu_get_dtbclk(struct clk_mgr_internal *clk_mgr);
 int dcn35_smu_get_dprefclk(struct clk_mgr_internal 

[PATCH 02/22] drm/amd/display: Consolidate HPO enable/disable and restrict only to state transitions.

2024-03-20 Thread Tom Chung
From: Natanel Roizenman 

[WHY]
Previously, we'd disabled HPO whenever an HPO display was disconnected. This
caused other HPO displays to blank whenever one was unplugged.

[HOW]
This change restricts HPO enable/disable to dce110_apply_ctx_to_hw and adds a
helper function (dce110_is_hpo_enabled) that returns true if any HPO displays
are present in a context. We compare the current and previous dc ctx to check
whether HPO is transitioning from on to off or vice versa, and adjust the HPO
state accordingly.

Reviewed-by: Wenjing Liu 
Reviewed-by: Chris Park 
Acked-by: Tom Chung 
Signed-off-by: Natanel Roizenman 
---
 .../amd/display/dc/hwss/dce110/dce110_hwseq.c | 29 ---
 .../amd/display/dc/hwss/dcn20/dcn20_hwseq.c   |  5 
 2 files changed, 19 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/hwss/dce110/dce110_hwseq.c 
b/drivers/gpu/drm/amd/display/dc/hwss/dce110/dce110_hwseq.c
index 0ba1feaf96c0..de5542c20103 100644
--- a/drivers/gpu/drm/amd/display/dc/hwss/dce110/dce110_hwseq.c
+++ b/drivers/gpu/drm/amd/display/dc/hwss/dce110/dce110_hwseq.c
@@ -1192,16 +1192,6 @@ void dce110_disable_stream(struct pipe_ctx *pipe_ctx)
dccg->funcs->disable_symclk_se(dccg, 
stream_enc->stream_enc_inst,
   link_enc->transmitter - 
TRANSMITTER_UNIPHY_A);
}
-
-   if (dc->link_srv->dp_is_128b_132b_signal(pipe_ctx)) {
-   /* TODO: This looks like a bug to me as we are disabling HPO IO 
when
-* we are just disabling a single HPO stream. Shouldn't we 
disable HPO
-* HW control only when HPOs for all streams are disabled?
-*/
-   if 
(pipe_ctx->stream->ctx->dc->hwseq->funcs.setup_hpo_hw_control)
-   
pipe_ctx->stream->ctx->dc->hwseq->funcs.setup_hpo_hw_control(
-   pipe_ctx->stream->ctx->dc->hwseq, 
false);
-   }
 }
 
 void dce110_unblank_stream(struct pipe_ctx *pipe_ctx,
@@ -2288,6 +2278,19 @@ static void dce110_setup_audio_dto(
}
 }
 
+static bool dce110_is_hpo_enabled(struct dc_state *context)
+{
+   int i;
+
+   for (i = 0; i < MAX_HPO_DP2_ENCODERS; i++) {
+   if (context->res_ctx.is_hpo_dp_stream_enc_acquired[i]) {
+   return true;
+   }
+   }
+
+   return false;
+}
+
 enum dc_status dce110_apply_ctx_to_hw(
struct dc *dc,
struct dc_state *context)
@@ -2296,6 +2299,8 @@ enum dc_status dce110_apply_ctx_to_hw(
struct dc_bios *dcb = dc->ctx->dc_bios;
enum dc_status status;
int i;
+   bool was_hpo_enabled = dce110_is_hpo_enabled(dc->current_state);
+   bool is_hpo_enabled = dce110_is_hpo_enabled(context);
 
/* reset syncd pipes from disabled pipes */
if (dc->config.use_pipe_ctx_sync_logic)
@@ -2338,6 +2343,10 @@ enum dc_status dce110_apply_ctx_to_hw(
 
dce110_setup_audio_dto(dc, context);
 
+   if (dc->hwseq->funcs.setup_hpo_hw_control && was_hpo_enabled != 
is_hpo_enabled) {
+   dc->hwseq->funcs.setup_hpo_hw_control(dc->hwseq, 
is_hpo_enabled);
+   }
+
for (i = 0; i < dc->res_pool->pipe_count; i++) {
struct pipe_ctx *pipe_ctx_old =
>current_state->res_ctx.pipe_ctx[i];
diff --git a/drivers/gpu/drm/amd/display/dc/hwss/dcn20/dcn20_hwseq.c 
b/drivers/gpu/drm/amd/display/dc/hwss/dcn20/dcn20_hwseq.c
index 8b3536c380b8..eae71f448143 100644
--- a/drivers/gpu/drm/amd/display/dc/hwss/dcn20/dcn20_hwseq.c
+++ b/drivers/gpu/drm/amd/display/dc/hwss/dcn20/dcn20_hwseq.c
@@ -2892,11 +2892,6 @@ void dcn20_enable_stream(struct pipe_ctx *pipe_ctx)
struct link_encoder *link_enc = 
link_enc_cfg_get_link_enc(pipe_ctx->stream->link);
struct stream_encoder *stream_enc = pipe_ctx->stream_res.stream_enc;
 
-   if (dc->link_srv->dp_is_128b_132b_signal(pipe_ctx)) {
-   if (dc->hwseq->funcs.setup_hpo_hw_control)
-   dc->hwseq->funcs.setup_hpo_hw_control(dc->hwseq, true);
-   }
-
if (dc->link_srv->dp_is_128b_132b_signal(pipe_ctx)) {
dto_params.otg_inst = tg->inst;
dto_params.pixclk_khz = pipe_ctx->stream->timing.pix_clk_100hz 
/ 10;
-- 
2.34.1



[PATCH 03/22] drm/amd/display: Send DTBCLK disable message on first commit

2024-03-20 Thread Tom Chung
From: Taimur Hassan 

[Why]
Previous patch to allow DTBCLK disable didn't address boot case. Driver
thinks DTBCLK is disabled by default, so we don't send disable message to
PMFW. DTBCLK is then enabled at idle desktop on boot, burning power.

[How]
Set dtbclk_en to true on boot so that disable message is sent during first
commit.

Fixes: 27750e176a4f ("drm/amd/display: Allow DTBCLK disable for DCN35")
Reviewed-by: Charlene Liu 
Acked-by: Tom Chung 
Signed-off-by: Taimur Hassan 
---
 drivers/gpu/drm/amd/display/dc/clk_mgr/dcn35/dcn35_clk_mgr.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn35/dcn35_clk_mgr.c 
b/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn35/dcn35_clk_mgr.c
index c6030bed95a0..dafa07671d00 100644
--- a/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn35/dcn35_clk_mgr.c
+++ b/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn35/dcn35_clk_mgr.c
@@ -73,6 +73,8 @@
 #define CLK1_CLK2_BYPASS_CNTL__CLK2_BYPASS_SEL_MASK0x0007L
 #define CLK1_CLK2_BYPASS_CNTL__CLK2_BYPASS_DIV_MASK0x000FL
 
+#define SMU_VER_THRESHOLD 0x5D4A00 //93.74.0
+
 #define REG(reg_name) \
(ctx->clk_reg_offsets[reg ## reg_name ## _BASE_IDX] + reg ## reg_name)
 
@@ -412,9 +414,12 @@ static void dcn35_dump_clk_registers(struct 
clk_state_registers_and_bypass *regs
 
 static void init_clk_states(struct clk_mgr *clk_mgr)
 {
+   struct clk_mgr_internal *clk_mgr_int = TO_CLK_MGR_INTERNAL(clk_mgr);
uint32_t ref_dtbclk = clk_mgr->clks.ref_dtbclk_khz;
memset(&(clk_mgr->clks), 0, sizeof(struct dc_clocks));
 
+   if (clk_mgr_int->smu_ver >= SMU_VER_THRESHOLD)
+   clk_mgr->clks.dtbclk_en = true; // request DTBCLK disable on 
first commit
clk_mgr->clks.ref_dtbclk_khz = ref_dtbclk;  // restore ref_dtbclk
clk_mgr->clks.p_state_change_support = true;
clk_mgr->clks.prev_p_state_change_support = true;
-- 
2.34.1



[PATCH 01/22] drm/amd/display: Allow idle opts for no flip case on PSR panel

2024-03-20 Thread Tom Chung
From: Alvin Lee 

[Why & How]
There is a corner case where a single PSR panel
fails to enter idle optimizations if the panel
is not flipping (no planes or DPMS_OFF == true).
This is because the panel will not enter PSR if it's
not flipping, but this will prevent the FW idle opt
path from being executed. To handle this case we will
allow entry to idle opt from driver side even when a
PSR panel is connected under the following scenarios:
1. Only a single PSR panel is connected
2. PSR panel is not flipping

Reviewed-by: Samson Tam 
Acked-by: Tom Chung 
Signed-off-by: Alvin Lee 
---
 drivers/gpu/drm/amd/display/dc/hwss/dcn32/dcn32_hwseq.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/display/dc/hwss/dcn32/dcn32_hwseq.c 
b/drivers/gpu/drm/amd/display/dc/hwss/dcn32/dcn32_hwseq.c
index c0b526cf1786..a5e92389615f 100644
--- a/drivers/gpu/drm/amd/display/dc/hwss/dcn32/dcn32_hwseq.c
+++ b/drivers/gpu/drm/amd/display/dc/hwss/dcn32/dcn32_hwseq.c
@@ -261,7 +261,9 @@ bool dcn32_apply_idle_power_optimizations(struct dc *dc, 
bool enable)
for (i = 0; i < dc->current_state->stream_count; i++) {
/* MALL SS messaging is not supported with PSR at this time */
if (dc->current_state->streams[i] != NULL &&
-   
dc->current_state->streams[i]->link->psr_settings.psr_version != 
DC_PSR_VERSION_UNSUPPORTED)
+   
dc->current_state->streams[i]->link->psr_settings.psr_version != 
DC_PSR_VERSION_UNSUPPORTED &&
+   (dc->current_state->stream_count > 1 || 
(!dc->current_state->streams[i]->dpms_off &&
+   
dc->current_state->stream_status[i].plane_count > 0)))
return false;
}
 
-- 
2.34.1



[PATCH 00/22] DC Patches Mar 25 2024

2024-03-20 Thread Tom Chung
This DC patchset brings improvements in multiple areas. In summary, we have:

- Fix some bound and NULL check
- Fix nonseamless transition from ODM + MPO to ODM + subvp
- Allow Z8 when stutter threshold is not met
- Remove plane and stream pointers from dc scratch
- Remove read/write to external register
- Increase number of hpo dp link encoders
- Increase clock table size
- Add new IPS config mode
- Build scaling params when a new plane is appended
- Refactor DML2 interfaces
- Allow idle opts for no flip case on PSR panel

Cc: Daniel Wheeler 


Alvin Lee (2):
  drm/amd/display: Allow idle opts for no flip case on PSR panel
  drm/amd/display: Remove plane and stream pointers from dc scratch

Anthony Koo (1):
  drm/amd/display: [FW Promotion] Release 0.0.210.0

Aric Cyr (1):
  drm/amd/display: 3.2.278

Bhawanpreet Lakha (2):
  drm/amd/display: Allow Z8 when stutter threshold is not met
  drm/amd/display: Allow Z8 when stutter threshold is not met for dcn35

Dillon Varone (3):
  drm/amd/display: Expand DML2 callbacks
  drm/amd/display: Refactor DML2 interfaces
  drm/amd/display: Modify DHCUB waterwark structures and functions

George Shen (1):
  drm/amd/display: Remove MPC rate control logic from DCN30 and above

Muhammad Ahmed (1):
  drm/amd/display: Skip pipe if the pipe idx not set properly

Natanel Roizenman (1):
  drm/amd/display: Consolidate HPO enable/disable and restrict only to
state transitions.

Nicholas Kazlauskas (1):
  drm/amd/display: Add new IPS config mode

Roman Li (1):
  drm/amd/display: Fix bounds check for dcn35 DcfClocks

Sohaib Nadeem (1):
  drm/amd/display: Added missing null checks

Sridevi Arvindekar (1):
  drm/amd/display: Increase number of hpo dp link encoders

Sung Joon Kim (2):
  drm/amd/display: Remove read/write to external register
  drm/amd/display: Increase clock table size

Taimur Hassan (1):
  drm/amd/display: Send DTBCLK disable message on first commit

Wenjing Liu (3):
  drm/amd/display: fix a dereference of a NULL pointer
  drm/amd/display: fix nonseamless transition from ODM + MPO to ODM +
subvp
  drm/amd/display: build scaling params when a new plane is appended

 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c |  18 +--
 .../amd/display/amdgpu_dm/amdgpu_dm_color.c   |  42 +++---
 .../display/dc/clk_mgr/dcn35/dcn35_clk_mgr.c  |  45 +--
 .../amd/display/dc/clk_mgr/dcn35/dcn35_smu.c  |  21 ---
 .../amd/display/dc/clk_mgr/dcn35/dcn35_smu.h  |   2 -
 drivers/gpu/drm/amd/display/dc/core/dc.c  |  86 ++--
 .../drm/amd/display/dc/core/dc_hw_sequencer.c |   8 +-
 .../gpu/drm/amd/display/dc/core/dc_resource.c |  89 -
 .../gpu/drm/amd/display/dc/core/dc_state.c|  31 +++--
 .../gpu/drm/amd/display/dc/core/dc_stream.c   |  16 +--
 .../gpu/drm/amd/display/dc/core/dc_surface.c  |  49 ++-
 drivers/gpu/drm/amd/display/dc/dc.h   |  29 +++--
 drivers/gpu/drm/amd/display/dc/dc_dmub_srv.c  |  29 +
 drivers/gpu/drm/amd/display/dc/dc_plane.h |   2 +-
 drivers/gpu/drm/amd/display/dc/dc_state.h |   6 +-
 .../gpu/drm/amd/display/dc/dc_state_priv.h|  12 +-
 drivers/gpu/drm/amd/display/dc/dc_stream.h|   2 +-
 drivers/gpu/drm/amd/display/dc/dc_types.h |   2 +-
 .../drm/amd/display/dc/dcn10/dcn10_hubbub.c   |   8 +-
 .../drm/amd/display/dc/dcn10/dcn10_hubbub.h   |  10 +-
 .../drm/amd/display/dc/dcn20/dcn20_hubbub.c   |   2 +-
 .../drm/amd/display/dc/dcn20/dcn20_hubbub.h   |   2 +-
 .../drm/amd/display/dc/dcn201/dcn201_hubbub.c |   2 +-
 .../drm/amd/display/dc/dcn21/dcn21_hubbub.c   |   8 +-
 .../drm/amd/display/dc/dcn21/dcn21_hubbub.h   |   8 +-
 .../drm/amd/display/dc/dcn30/dcn30_hubbub.c   |   2 +-
 .../drm/amd/display/dc/dcn30/dcn30_hubbub.h   |   2 +-
 .../gpu/drm/amd/display/dc/dcn30/dcn30_mpc.c  |  54 
 .../gpu/drm/amd/display/dc/dcn30/dcn30_mpc.h  |  14 +-
 .../drm/amd/display/dc/dcn31/dcn31_hubbub.c   |   8 +-
 .../drm/amd/display/dc/dcn32/dcn32_hubbub.c   |  10 +-
 .../drm/amd/display/dc/dcn32/dcn32_hubbub.h   |   8 +-
 .../gpu/drm/amd/display/dc/dcn32/dcn32_mpc.c  |   5 +-
 .../display/dc/dcn32/dcn32_resource_helpers.c |  23 +---
 .../drm/amd/display/dc/dcn35/dcn35_hubbub.c   |   4 +-
 .../drm/amd/display/dc/dml/dcn20/dcn20_fpu.c  |   9 +-
 .../drm/amd/display/dc/dml/dcn31/dcn31_fpu.c  |  10 +-
 .../drm/amd/display/dc/dml/dcn35/dcn35_fpu.c  |   2 +-
 .../display/dc/dml2/dml2_dc_resource_mgmt.c   |  23 ++--
 .../display/dc/dml2/dml2_translation_helper.c |  14 +-
 .../gpu/drm/amd/display/dc/dml2/dml2_utils.c  |   5 +
 .../drm/amd/display/dc/dml2/dml2_wrapper.c|   9 ++
 .../drm/amd/display/dc/dml2/dml2_wrapper.h|  28 +++-
 .../drm/amd/display/dc/dpp/dcn20/dcn20_dpp.h  |   2 +-
 .../amd/display/dc/dpp/dcn20/dcn20_dpp_cm.c   |  10 +-
 .../drm/amd/display/dc/dpp/dcn30/dcn30_dpp.c  |  10 +-
 .../amd/display/dc/hwss/dce110/dce110_hwseq.c |  44 ---
 .../amd/display/dc/hwss/dcn10/dcn10_hwseq.c   |  24 ++--
 .../amd/display/dc/hwss/dcn20/dcn20_hwseq.c   |  84 ++--