RE: [PATCH 28/46] drm/amd/display: Enable RCO for PHYSYMCLK in DCN35

[Li, Roman]

[Public]

> -Original Message-
> From: Wayne Lin 
> Sent: Wednesday, April 24, 2024 4:49 AM
> To: amd-gfx@lists.freedesktop.org
> Cc: Wentland, Harry ; Li, Sun peng (Leo)
> ; Siqueira, Rodrigo ;
> Pillai, Aurabindo ; Li, Roman
> ; Lin, Wayne ; Gutierrez,
> Agustin ; Chung, ChiaHsuan (Tom)
> ; Wu, Hersen ; Zuo,
> Jerry ; Miess, Daniel ; Liu,
> Charlene 
> Subject: [PATCH 28/46] drm/amd/display: Enable RCO for PHYSYMCLK in
> DCN35
>
> From: Daniel Miess 
>
> [Why & How]
> Enable root clock optimization for PHYSYMCLK and only disable it when it's
> actively being used
>
> Reviewed-by: Charlene Liu 
> Acked-by: Wayne Lin 
> Signed-off-by: Daniel Miess 
> ---
>  drivers/gpu/drm/amd/display/dc/dc.h   |  1 +
>  .../gpu/drm/amd/display/dc/dcn35/dcn35_dccg.c | 45 ---
>  .../amd/display/dc/hwss/dcn35/dcn35_hwseq.c   | 32 +
>  .../amd/display/dc/hwss/dcn35/dcn35_hwseq.h   |  2 +
>  .../amd/display/dc/hwss/dcn35/dcn35_init.c|  1 +
>  .../amd/display/dc/hwss/dcn351/dcn351_init.c  |  1 +
>  .../display/dc/hwss/hw_sequencer_private.h|  4 ++
>  7 files changed, 41 insertions(+), 45 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/display/dc/dc.h
> b/drivers/gpu/drm/amd/display/dc/dc.h
> index 3048d5a0e87d..dd8940c2a4bf 100644
> --- a/drivers/gpu/drm/amd/display/dc/dc.h
> +++ b/drivers/gpu/drm/amd/display/dc/dc.h
> @@ -724,6 +724,7 @@ enum pg_hw_pipe_resources {
>   PG_OPTC,
>   PG_DPSTREAM,
>   PG_HDMISTREAM,
> + PG_PHYSYMCLK,
>   PG_HW_PIPE_RESOURCES_NUM_ELEMENT
>  };
>
> diff --git a/drivers/gpu/drm/amd/display/dc/dcn35/dcn35_dccg.c
> b/drivers/gpu/drm/amd/display/dc/dcn35/dcn35_dccg.c
> index 4b282b7e0996..795320a25fd2 100644
> --- a/drivers/gpu/drm/amd/display/dc/dcn35/dcn35_dccg.c
> +++ b/drivers/gpu/drm/amd/display/dc/dcn35/dcn35_dccg.c
> @@ -461,32 +461,22 @@ static void
> dccg35_set_physymclk_root_clock_gating(
>   case 0:
>   REG_UPDATE(DCCG_GATE_DISABLE_CNTL2,
>   PHYASYMCLK_ROOT_GATE_DISABLE, enable ?
> 1 : 0);
> -//   REG_UPDATE(DCCG_GATE_DISABLE_CNTL4,
> -//   PHYA_REFCLK_ROOT_GATE_DISABLE, enable ?
> 1 : 0);
>   break;
>   case 1:
>   REG_UPDATE(DCCG_GATE_DISABLE_CNTL2,
>   PHYBSYMCLK_ROOT_GATE_DISABLE, enable ?
> 1 : 0);
> -//   REG_UPDATE(DCCG_GATE_DISABLE_CNTL4,
> -//   PHYB_REFCLK_ROOT_GATE_DISABLE, enable ?
> 1 : 0);
>   break;
>   case 2:
>   REG_UPDATE(DCCG_GATE_DISABLE_CNTL2,
>   PHYCSYMCLK_ROOT_GATE_DISABLE, enable ?
> 1 : 0);
> -//   REG_UPDATE(DCCG_GATE_DISABLE_CNTL4,
> -//   PHYC_REFCLK_ROOT_GATE_DISABLE, enable ?
> 1 : 0);
>   break;
>   case 3:
>   REG_UPDATE(DCCG_GATE_DISABLE_CNTL2,
>   PHYDSYMCLK_ROOT_GATE_DISABLE, enable ?
> 1 : 0);
> -//   REG_UPDATE(DCCG_GATE_DISABLE_CNTL4,
> -//   PHYD_REFCLK_ROOT_GATE_DISABLE, enable ?
> 1 : 0);
>   break;
>   case 4:
>   REG_UPDATE(DCCG_GATE_DISABLE_CNTL2,
>   PHYESYMCLK_ROOT_GATE_DISABLE, enable ?
> 1 : 0);
> -//   REG_UPDATE(DCCG_GATE_DISABLE_CNTL4,
> -//   PHYE_REFCLK_ROOT_GATE_DISABLE, enable ?
> 1 : 0);
>   break;
>   default:
>   BREAK_TO_DEBUGGER();
> @@ -509,16 +499,10 @@ static void dccg35_set_physymclk(
>   REG_UPDATE_2(PHYASYMCLK_CLOCK_CNTL,
>   PHYASYMCLK_EN, 1,
>   PHYASYMCLK_SRC_SEL, clk_src);
> -//   if (dccg->ctx->dc-
> >debug.root_clock_optimization.bits.physymclk)
> -//   REG_UPDATE(DCCG_GATE_DISABLE_CNTL4,
> -//
>   PHYA_REFCLK_ROOT_GATE_DISABLE, 0);
>   } else {
>   REG_UPDATE_2(PHYASYMCLK_CLOCK_CNTL,
>   PHYASYMCLK_EN, 0,
>   PHYASYMCLK_SRC_SEL, 0);
> -//   if (dccg->ctx->dc-
> >debug.root_clock_optimization.bits.physymclk)
> -//   REG_UPDATE(DCCG_GATE_DISABLE_CNTL4,
> -//
>   PHYA_REFCLK_ROOT_GATE_DISABLE, 1);
>   }
>   break;
>   case 1:
> @@ -526,16 +510,10 @@ static void dccg35_set_physymclk(
>   REG_UPDATE_2(PHYBSYMCLK_CLOCK_CNTL,
> 

RE: [PATCH v3] drm/amd/display: Prevent potential buffer overflow in map_hw_resources

[Li, Roman]

Reviewed-by: Roman Li 

> -Original Message-
> From: SHANMUGAM, SRINIVASAN 
> Sent: Wednesday, February 21, 2024 11:37 PM
> To: Siqueira, Rodrigo ; Pillai, Aurabindo
> ; Chung, ChiaHsuan (Tom)
> ; Li, Roman 
> Cc: amd-gfx@lists.freedesktop.org; SHANMUGAM, SRINIVASAN
> ; Zhuo, Lillian
> 
> Subject: [PATCH v3] drm/amd/display: Prevent potential buffer overflow in
> map_hw_resources
> 
> Adds a check in the map_hw_resources function to prevent a potential buffer
> overflow. The function was accessing arrays using an index that could
> potentially be greater than the size of the arrays, leading to a buffer 
> overflow.
> 
> Adds a check to ensure that the index is within the bounds of the arrays. If 
> the
> index is out of bounds, an error message is printed and break it will continue
> execution with just ignoring extra data early to prevent the buffer overflow.
> 
> Reported by smatch:
> drivers/gpu/drm/amd/amdgpu/../display/dc/dml2/dml2_wrapper.c:79
> map_hw_resources() error: buffer overflow 'dml2-
> >v20.scratch.dml_to_dc_pipe_mapping.disp_cfg_to_stream_id' 6 <= 7
> drivers/gpu/drm/amd/amdgpu/../display/dc/dml2/dml2_wrapper.c:81
> map_hw_resources() error: buffer overflow 'dml2-
> >v20.scratch.dml_to_dc_pipe_mapping.disp_cfg_to_plane_id' 6 <= 7
> 
> Fixes: 482ce89eec1b ("drm/amd/display: Introduce DML2")
> Cc: Rodrigo Siqueira 
> Cc: Roman Li 
> Cc: Qingqing Zhuo 
> Cc: Aurabindo Pillai 
> Cc: Tom Chung 
> Signed-off-by: Srinivasan Shanmugam 
> ---
> v3:
>  - s/return/break as return may leave the system in a bad state
> 
>  drivers/gpu/drm/amd/display/dc/dml2/dml2_wrapper.c | 5 +
>  1 file changed, 5 insertions(+)
> 
> 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 26307e599614..2a58a7687bdb 100644
> --- a/drivers/gpu/drm/amd/display/dc/dml2/dml2_wrapper.c
> +++ b/drivers/gpu/drm/amd/display/dc/dml2/dml2_wrapper.c
> @@ -76,6 +76,11 @@ static void map_hw_resources(struct dml2_context
> *dml2,
>   in_out_display_cfg->hw.DLGRefClkFreqMHz = 50;
>   }
>   for (j = 0; j < mode_support_info->DPPPerSurface[i]; j++) {
> + if (i >=
> __DML2_WRAPPER_MAX_STREAMS_PLANES__) {
> + dml_print("DML::%s: Index out of bounds:
> i=%d, __DML2_WRAPPER_MAX_STREAMS_PLANES__=%d\n",
> +   __func__, i,
> __DML2_WRAPPER_MAX_STREAMS_PLANES__);
> + break;
> + }
>   dml2-
> >v20.scratch.dml_to_dc_pipe_mapping.dml_pipe_idx_to_stream_id[num_pip
> es] = dml2->v20.scratch.dml_to_dc_pipe_mapping.disp_cfg_to_stream_id[i];
>   dml2-
> >v20.scratch.dml_to_dc_pipe_mapping.dml_pipe_idx_to_stream_id_valid[nu
> m_pipes] = true;
>   dml2-
> >v20.scratch.dml_to_dc_pipe_mapping.dml_pipe_idx_to_plane_id[num_pipe
> s] = dml2->v20.scratch.dml_to_dc_pipe_mapping.disp_cfg_to_plane_id[i];
> --
> 2.34.1

RE: [PATCH] drm/amd/display: Fix && vs || in 'edp_set_replay_allow_active()'

[Li, Roman]

[AMD Official Use Only - General]

Reviewed-by: Roman Li 

> -Original Message-
> From: amd-gfx  On Behalf Of
> Srinivasan Shanmugam
> Sent: Friday, February 9, 2024 9:53 AM
> To: Siqueira, Rodrigo ; Pillai, Aurabindo
> 
> Cc: amd-gfx@lists.freedesktop.org; SHANMUGAM, SRINIVASAN
> ; Lakha, Bhawanpreet
> ; Wentland, Harry
> 
> Subject: [PATCH] drm/amd/display: Fix && vs || in
> 'edp_set_replay_allow_active()'
>
> AND should be OR or it will lead to a NULL dereference.
>
> Fixes the below:
> drivers/gpu/drm/amd/amdgpu/../display/dc/link/protocols/link_edp_panel_
> control.c:895 edp_set_replay_allow_active() error: we previously assumed
> 'replay' could be null (see line 887)
>
> Fixes: c7ddc0a800bc ("drm/amd/display: Add Functions to enable Freesync
> Panel Replay")
> Cc: Bhawanpreet Lakha 
> Cc: Harry Wentland 
> Cc: Rodrigo Siqueira 
> Cc: Aurabindo Pillai 
> Signed-off-by: Srinivasan Shanmugam 
> ---
>  .../drm/amd/display/dc/link/protocols/link_edp_panel_control.c  | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git
> a/drivers/gpu/drm/amd/display/dc/link/protocols/link_edp_panel_control.c
> b/drivers/gpu/drm/amd/display/dc/link/protocols/link_edp_panel_control.c
> index 443215b96308..77648228ec60 100644
> ---
> a/drivers/gpu/drm/amd/display/dc/link/protocols/link_edp_panel_control.c
> +++
> b/drivers/gpu/drm/amd/display/dc/link/protocols/link_edp_panel_control.c
> @@ -884,7 +884,7 @@ bool edp_set_replay_allow_active(struct dc_link
> *link, const bool *allow_active,
>   struct dmub_replay *replay = dc->res_pool->replay;
>   unsigned int panel_inst;
>
> - if (replay == NULL && force_static)
> + if (!replay || force_static)
>   return false;
>
>   if (!dc_get_edp_link_panel_inst(dc, link, _inst))
> --
> 2.34.1

RE: [PATCH] drm/amd/display: Fix possible NULL dereference on device remove/driver unload

[Li, Roman]

[AMD Official Use Only - General]

Reviewed-by: Roman Li 

> -Original Message-
> From: amd-gfx  On Behalf Of
> Srinivasan Shanmugam
> Sent: Tuesday, February 6, 2024 11:55 PM
> To: Siqueira, Rodrigo ; Pillai, Aurabindo
> 
> Cc: amd-gfx@lists.freedesktop.org; SHANMUGAM, SRINIVASAN
> ; Andrey Grodzovsky
> ; Wentland, Harry
> 
> Subject: [PATCH] drm/amd/display: Fix possible NULL dereference on device
> remove/driver unload
>
> As part of a cleanup amdgpu_dm_fini() function, which is typically called when
> a device is being shut down or a driver is being unloaded
>
> The below error message suggests that there is a potential null pointer
> dereference issue with adev->dm.dc.
>
> In the below, line of code where adev->dm.dc is used without a preceding null
> check:
>
> for (i = 0; i < adev->dm.dc->caps.max_links; i++) {
>
> To fix this issue, add a null check for adev->dm.dc before this line.
>
> Reported by smatch:
> drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c:1959
> amdgpu_dm_fini() error: we previously assumed 'adev->dm.dc' could be null
> (see line 1943)
>
> Fixes: 006c26a0f1c8 ("drm/amd/display: Fix crash on device remove/driver
> unload")
> Cc: Andrey Grodzovsky 
> Cc: Harry Wentland 
> Cc: Rodrigo Siqueira 
> Cc: Aurabindo Pillai 
> Signed-off-by: Srinivasan Shanmugam 
> ---
>  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> index b3a5e730be24..d4c1415f4562 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -1956,7 +1956,7 @@ static void amdgpu_dm_fini(struct amdgpu_device
> *adev)
> >dm.dmub_bo_gpu_addr,
> >dm.dmub_bo_cpu_addr);
>
> - if (adev->dm.hpd_rx_offload_wq) {
> + if (adev->dm.hpd_rx_offload_wq && adev->dm.dc) {
>   for (i = 0; i < adev->dm.dc->caps.max_links; i++) {
>   if (adev->dm.hpd_rx_offload_wq[i].wq) {
>   destroy_workqueue(adev-
> >dm.hpd_rx_offload_wq[i].wq);
> --
> 2.34.1

RE: [PATCH] drm/amd/display: Fix possible use of uninitialized 'max_chunks_fbc_mode' in 'calculate_bandwidth()'

[Li, Roman]

[Public]

Reviewed-by: Roman Li 

> -Original Message-
> From: amd-gfx  On Behalf Of
> Srinivasan Shanmugam
> Sent: Tuesday, February 6, 2024 11:55 PM
> To: Siqueira, Rodrigo ; Pillai, Aurabindo
> 
> Cc: amd-gfx@lists.freedesktop.org; SHANMUGAM, SRINIVASAN
> ; Wentland, Harry
> ; Deucher, Alexander
> 
> Subject: [PATCH] drm/amd/display: Fix possible use of uninitialized
> 'max_chunks_fbc_mode' in 'calculate_bandwidth()'
>
> 'max_chunks_fbc_mode' is only declared and assigned a value under a specific
> condition in the following lines:
>
> if (data->fbc_en[i] == 1) {
>   max_chunks_fbc_mode = 128 - dmif_chunk_buff_margin; }
>
> If 'data->fbc_en[i]' is not equal to 1 for any i, max_chunks_fbc_mode will not
> be initialized if it's used outside of this for loop.
>
> Ensure that 'max_chunks_fbc_mode' is properly initialized before it's used.
> Initialize it to a default value right after its declaration to ensure that 
> it gets a
> value assigned under all possible control flow paths.
>
> Thus fixing the below:
> drivers/gpu/drm/amd/amdgpu/../display/dc/basics/dce_calcs.c:914
> calculate_bandwidth() error: uninitialized symbol 'max_chunks_fbc_mode'.
> drivers/gpu/drm/amd/amdgpu/../display/dc/basics/dce_calcs.c:917
> calculate_bandwidth() error: uninitialized symbol 'max_chunks_fbc_mode'.
>
> Fixes: 4562236b3bc0 ("drm/amd/dc: Add dc display driver (v2)")
> Cc: Harry Wentland 
> Cc: Alex Deucher 
> Cc: Rodrigo Siqueira 
> Cc: Aurabindo Pillai 
> Signed-off-by: Srinivasan Shanmugam 
> ---
>  drivers/gpu/drm/amd/display/dc/basics/dce_calcs.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/display/dc/basics/dce_calcs.c
> b/drivers/gpu/drm/amd/display/dc/basics/dce_calcs.c
> index f2dfa96f9ef5..39530b2ea495 100644
> --- a/drivers/gpu/drm/amd/display/dc/basics/dce_calcs.c
> +++ b/drivers/gpu/drm/amd/display/dc/basics/dce_calcs.c
> @@ -94,7 +94,7 @@ static void calculate_bandwidth(
>   const uint32_t s_high = 7;
>   const uint32_t dmif_chunk_buff_margin = 1;
>
> - uint32_t max_chunks_fbc_mode;
> + uint32_t max_chunks_fbc_mode = 0;
>   int32_t num_cursor_lines;
>
>   int32_t i, j, k;
> --
> 2.34.1

RE: [PATCH] drm/amd/display: Initialize 'wait_time_microsec' variable in link_dp_training_dpia.c

[Li, Roman]

[Public]

Reviewed-by: Roman Li 

> -Original Message-
> From: amd-gfx  On Behalf Of
> Srinivasan Shanmugam
> Sent: Tuesday, February 6, 2024 11:55 PM
> To: Siqueira, Rodrigo ; Pillai, Aurabindo
> 
> Cc: amd-gfx@lists.freedesktop.org; SHANMUGAM, SRINIVASAN
> ; Liu, Wenjing
> 
> Subject: [PATCH] drm/amd/display: Initialize 'wait_time_microsec' variable in
> link_dp_training_dpia.c
>
> wait_time_microsec = max(wait_time_microsec, (uint32_t)
> DPIA_CLK_SYNC_DELAY);
>
> Above line is trying to assign the maximum value between
> 'wait_time_microsec' and 'DPIA_CLK_SYNC_DELAY' to wait_time_microsec.
> However, 'wait_time_microsec' has not been assigned a value before this line,
> initialize 'wait_time_microsec' at the point of declaration.
>
> Fixes the below:
> drivers/gpu/drm/amd/amdgpu/../display/dc/link/protocols/link_dp_training
> _dpia.c:697 dpia_training_eq_non_transparent() error: uninitialized symbol
> 'wait_time_microsec'.
>
> Fixes: 630168a97314 ("drm/amd/display: move dp link training logic to
> link_dp_training")
> Cc: Wenjing Liu 
> Cc: Rodrigo Siqueira 
> Cc: Aurabindo Pillai 
> Signed-off-by: Srinivasan Shanmugam 
> ---
>  .../drm/amd/display/dc/link/protocols/link_dp_training_dpia.c   | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git
> a/drivers/gpu/drm/amd/display/dc/link/protocols/link_dp_training_dpia.c
> b/drivers/gpu/drm/amd/display/dc/link/protocols/link_dp_training_dpia.c
> index e8dda44b23cb..5d36bab0029c 100644
> --- a/drivers/gpu/drm/amd/display/dc/link/protocols/link_dp_training_dpia.c
> +++ b/drivers/gpu/drm/amd/display/dc/link/protocols/link_dp_training_dpi
> +++ a.c
> @@ -619,7 +619,7 @@ static enum link_training_result
> dpia_training_eq_non_transparent(
>   uint32_t retries_eq = 0;
>   enum dc_status status;
>   enum dc_dp_training_pattern tr_pattern;
> - uint32_t wait_time_microsec;
> + uint32_t wait_time_microsec = 0;
>   enum dc_lane_count lane_count = lt_settings-
> >link_settings.lane_count;
>   union lane_align_status_updated dpcd_lane_status_updated = {0};
>   union lane_status dpcd_lane_status[LANE_COUNT_DP_MAX] = {0};
> --
> 2.34.1

RE: [PATCH] drm/amd/display: Fix possible buffer overflow in 'find_dcfclk_for_voltage()'

[Li, Roman]

[Public]

> -Original Message-
> From: SHANMUGAM, SRINIVASAN 
> Sent: Tuesday, February 6, 2024 11:55 PM
> To: Siqueira, Rodrigo ; Pillai, Aurabindo
> 
> Cc: amd-gfx@lists.freedesktop.org; SHANMUGAM, SRINIVASAN
> ; Li, Roman 
> Subject: [PATCH] drm/amd/display: Fix possible buffer overflow in
> 'find_dcfclk_for_voltage()'
>
> when 'find_dcfclk_for_voltage()' function is looping over
> VG_NUM_SOC_VOLTAGE_LEVELS (which is 8), but the size of the DcfClocks
> array is VG_NUM_DCFCLK_DPM_LEVELS (which is 7).
>
> When the loop variable i reaches 7, the function tries to access clock_table-
> >DcfClocks[7]. However, since the size of the DcfClocks array is 7, the valid
> indices are 0 to 6. Index 7 is beyond the size of the array, leading to a 
> buffer
> overflow.
>
> Fixes the below:
> drivers/gpu/drm/amd/amdgpu/../display/dc/clk_mgr/dcn301/vg_clk_mgr.c:
> 550 find_dcfclk_for_voltage() error: buffer overflow 'clock_table->DcfClocks' 
> 7
> <= 7

I recommend mentioning that this is a static analysis tool error.
With that:
Reviewed-by: Roman Li 

>
> Fixes: 3a83e4e64bb1 ("drm/amd/display: Add dcn3.01 support to DC (v2)")
> Cc: Roman Li 
> Cc: Rodrigo Siqueira 
> Cc: Aurabindo Pillai 
> Signed-off-by: Srinivasan Shanmugam 
> ---
>  drivers/gpu/drm/amd/display/dc/clk_mgr/dcn301/vg_clk_mgr.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn301/vg_clk_mgr.c
> b/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn301/vg_clk_mgr.c
> index a5489fe6875f..aa9fd1dc550a 100644
> --- a/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn301/vg_clk_mgr.c
> +++ b/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn301/vg_clk_mgr.c
> @@ -546,6 +546,8 @@ static unsigned int find_dcfclk_for_voltage(const
> struct vg_dpm_clocks *clock_ta
>   int i;
>
>   for (i = 0; i < VG_NUM_SOC_VOLTAGE_LEVELS; i++) {
> + if (i >= VG_NUM_DCFCLK_DPM_LEVELS)
> + break;
>   if (clock_table->SocVoltage[i] == voltage)
>   return clock_table->DcfClocks[i];
>   }
> --
> 2.34.1

RE: [PATCH v2] drm/amd/display: Implement bounds check for stream encoder creation in DCN301

[Li, Roman]

[Public]

Inline.

> -Original Message-
> From: SHANMUGAM, SRINIVASAN 
> Sent: Monday, February 5, 2024 10:47 PM
> To: Li, Roman ; Siqueira, Rodrigo
> ; Pillai, Aurabindo 
> Cc: amd-gfx@lists.freedesktop.org; SHANMUGAM, SRINIVASAN
> 
> Subject: [PATCH v2] drm/amd/display: Implement bounds check for stream
> encoder creation in DCN301
>
> 'stream_enc_regs' array is an array of dcn10_stream_enc_registers structures.
> The array is initialized with four elements, corresponding to the four calls 
> to
> stream_enc_regs() in the array initializer. This means that valid indices for 
> this
> array are 0, 1, 2, and 3.
>
> The error message 'stream_enc_regs' 4 <= 5 below, is indicating that there is 
> an
> attempt to access this array with an index of 5, which is out of bounds. This
> could lead to undefined behavior
>
> Here, eng_id is used as an index to access the stream_enc_regs array. If 
> eng_id
> is 5, this would result in an out-of-bounds access on the stream_enc_regs
> array.
>
> Thus fixing Buffer overflow error in dcn301_stream_encoder_create reported
> by Smatch:
> drivers/gpu/drm/amd/amdgpu/../display/dc/resource/dcn301/dcn301_reso
> urce.c:1011 dcn301_stream_encoder_create() error: buffer overflow
> 'stream_enc_regs' 4 <= 5
>
> Fixes: 3a83e4e64bb1 ("drm/amd/display: Add dcn3.01 support to DC (v2)")
> Cc: Roman Li 
> Cc: Rodrigo Siqueira 
> Cc: Aurabindo Pillai 
> Signed-off-by: Srinivasan Shanmugam 
> ---
>  .../drm/amd/display/dc/resource/dcn301/dcn301_resource.c | 9 -
>  1 file changed, 4 insertions(+), 5 deletions(-)
>
> diff --git
> a/drivers/gpu/drm/amd/display/dc/resource/dcn301/dcn301_resource.c
> b/drivers/gpu/drm/amd/display/dc/resource/dcn301/dcn301_resource.c
> index 511ff6b5b985..4a475a723191 100644
> --- a/drivers/gpu/drm/amd/display/dc/resource/dcn301/dcn301_resource.c
> +++
> b/drivers/gpu/drm/amd/display/dc/resource/dcn301/dcn301_resource.c
> @@ -999,7 +999,7 @@ static struct stream_encoder
> *dcn301_stream_encoder_create(enum engine_id eng_id
>   vpg = dcn301_vpg_create(ctx, vpg_inst);
>   afmt = dcn301_afmt_create(ctx, afmt_inst);
>
> - if (!enc1 || !vpg || !afmt) {
> + if (!enc1 || !vpg || !afmt || eng_id >= ARRAY_SIZE(stream_enc_regs))
> {
>   kfree(enc1);
>   kfree(vpg);
>   kfree(afmt);

Reviewed-by: Roman Li 
I don't think the part below is necessary.

> @@ -1007,10 +1007,9 @@ static struct stream_encoder
> *dcn301_stream_encoder_create(enum engine_id eng_id
>   }
>
>   dcn30_dio_stream_encoder_construct(enc1, ctx, ctx->dc_bios,
> - eng_id, vpg, afmt,
> - _enc_regs[eng_id],
> - _shift, _mask);
> -
> +eng_id, vpg, afmt,
> +_enc_regs[eng_id],
> +_shift, _mask);
>   return >base;
>  }
>
> --
> 2.34.1

RE: [PATCH] drm/amd/display: Implement bounds check for stream encoder creation in DCN301

[Li, Roman]

[Public]

Inline.

> -Original Message-
> From: SHANMUGAM, SRINIVASAN 
> Sent: Sunday, February 4, 2024 9:35 PM
> To: Siqueira, Rodrigo ; Pillai, Aurabindo
> 
> Cc: amd-gfx@lists.freedesktop.org; SHANMUGAM, SRINIVASAN
> ; Li, Roman 
> Subject: [PATCH] drm/amd/display: Implement bounds check for stream
> encoder creation in DCN301
>
> 'stream_enc_regs' array is an array of dcn10_stream_enc_registers structures.
> The array is initialized with four elements, corresponding to the four calls 
> to
> stream_enc_regs() in the array initializer. This means that valid indices for 
> this
> array are 0, 1, 2, and 3.
>
> The error message 'stream_enc_regs' 4 <= 5 below, is indicating that there is 
> an
> attempt to access this array with an index of 5, which is out of bounds. This
> could lead to undefined behavior
>
> Here, eng_id is used as an index to access the stream_enc_regs array. If 
> eng_id
> is 5, this would result in an out-of-bounds access.
>
> Fixes the below:
> drivers/gpu/drm/amd/amdgpu/../display/dc/resource/dcn301/dcn301_reso
> urce.c:1011 dcn301_stream_encoder_create() error: buffer overflow
> 'stream_enc_regs' 4 <= 5

Please mention that this is Smatch warning.
In current implementation this function is called with eng_id  limited by 
num_stream_encoder = 4 for dcn301.

>
> Fixes: 3a83e4e64bb1 ("drm/amd/display: Add dcn3.01 support to DC (v2)")
> Cc: Roman Li 
> Cc: Rodrigo Siqueira 
> Cc: Aurabindo Pillai 
> Signed-off-by: Srinivasan Shanmugam 
> ---
>  .../display/dc/resource/dcn301/dcn301_resource.c | 16 
>  1 file changed, 12 insertions(+), 4 deletions(-)
>
> diff --git
> a/drivers/gpu/drm/amd/display/dc/resource/dcn301/dcn301_resource.c
> b/drivers/gpu/drm/amd/display/dc/resource/dcn301/dcn301_resource.c
> index 511ff6b5b985..f915d7c3980e 100644
> --- a/drivers/gpu/drm/amd/display/dc/resource/dcn301/dcn301_resource.c
> +++
> b/drivers/gpu/drm/amd/display/dc/resource/dcn301/dcn301_resource.c
> @@ -1006,10 +1006,18 @@ static struct stream_encoder
> *dcn301_stream_encoder_create(enum engine_id eng_id
>   return NULL;
>   }

>
> - dcn30_dio_stream_encoder_construct(enc1, ctx, ctx->dc_bios,
> - eng_id, vpg, afmt,
> - _enc_regs[eng_id],
> - _shift, _mask);
> + if (eng_id < ARRAY_SIZE(stream_enc_regs)) {
> + dcn30_dio_stream_encoder_construct(enc1, ctx, ctx-
> >dc_bios,
> +eng_id, vpg, afmt,
> +_enc_regs[eng_id],
> +_shift, _mask);
> + } else {
> + DRM_ERROR("Invalid engine id: %d\n", eng_id);
> + kfree(enc1);
> + kfree(vpg);
> + kfree(afmt);
> + return NULL;
> + }

Can you just extend the existing null checks instead?
e.g.
if (!enc1 || !vpg || !afmt || (eng_id  >= ARRAY_SIZE(stream_enc_regs))

>
>   return >base;
>  }
> --
> 2.34.1

RE: [PATCH] drm/amd/display: Remove redundant DRM device struct in amdgpu_dm_, mst_types.c

[Li, Roman]

[AMD Official Use Only - General]

Reviewed-by: Roman Li 

> -Original Message-
> From: SHANMUGAM, SRINIVASAN 
> Sent: Saturday, November 11, 2023 11:01 PM
> To: Pillai, Aurabindo ; Siqueira, Rodrigo
> ; Li, Roman ; Deucher,
> Alexander ; Wentland, Harry
> ; Mahfooz, Hamza
> 
> Cc: amd-gfx@lists.freedesktop.org; SHANMUGAM, SRINIVASAN
> 
> Subject: [PATCH] drm/amd/display: Remove redundant DRM device struct in
> amdgpu_dm_, mst_types.c
>
> Declaration of 'struct drm_device *dev' is redundant, as 'connector->dev' &
> 'dev_get_drvdata(kdev)' can be directly passed to 'drm_to_adev', without
> intermediate DRM device 'dev' variable
>
> Cc: Roman Li 
> Cc: Hamza Mahfooz 
> Cc: Harry Wentland 
> Cc: Rodrigo Siqueira 
> Cc: Aurabindo Pillai 
> Cc: Alex Deucher 
> Signed-off-by: Srinivasan Shanmugam 
> ---
>  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c| 9 +++--
>  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c  | 3 +--
>  2 files changed, 4 insertions(+), 8 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 adbeb2c897b5..39c73e5321c3 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -893,8 +893,7 @@ static int dm_early_init(void *handle);
>  /* Allocate memory for FBC compressed data  */  static void
> amdgpu_dm_fbc_init(struct drm_connector *connector)  {
> - struct drm_device *dev = connector->dev;
> - struct amdgpu_device *adev = drm_to_adev(dev);
> + struct amdgpu_device *adev = drm_to_adev(connector->dev);
>   struct dm_compressor_info *compressor = >dm.compressor;
>   struct amdgpu_dm_connector *aconn =
> to_amdgpu_dm_connector(connector);
>   struct drm_display_mode *mode;
> @@ -988,8 +987,7 @@ static int
> amdgpu_dm_audio_component_bind(struct device *kdev,  static void
> amdgpu_dm_audio_component_unbind(struct device *kdev,
> struct device *hda_kdev, void *data)
> {
> - struct drm_device *dev = dev_get_drvdata(kdev);
> - struct amdgpu_device *adev = drm_to_adev(dev);
> + struct amdgpu_device *adev = drm_to_adev(dev_get_drvdata(kdev));
>   struct drm_audio_component *acomp = data;
>
>   acomp->ops = NULL;
> @@ -10766,8 +10764,7 @@ void amdgpu_dm_update_freesync_caps(struct
> drm_connector *connector,
>   struct dm_connector_state *dm_con_state = NULL;
>   struct dc_sink *sink;
>
> - struct drm_device *dev = connector->dev;
> - struct amdgpu_device *adev = drm_to_adev(dev);
> + struct amdgpu_device *adev = drm_to_adev(connector->dev);
>   struct amdgpu_hdmi_vsdb_info vsdb_info = {0};
>   bool freesync_capable = false;
>   enum adaptive_sync_type as_type = ADAPTIVE_SYNC_TYPE_NONE;
> diff --git
> a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
> index d3b13d362eda..e4dd0037bcb7 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
> @@ -424,8 +424,7 @@ dm_mst_atomic_best_encoder(struct
> drm_connector *connector,  {
>   struct drm_connector_state *connector_state =
> drm_atomic_get_new_connector_state(state,
>
>connector);
> - struct drm_device *dev = connector->dev;
> - struct amdgpu_device *adev = drm_to_adev(dev);
> + struct amdgpu_device *adev = drm_to_adev(connector->dev);
>   struct amdgpu_crtc *acrtc = to_amdgpu_crtc(connector_state->crtc);
>
>   return >dm.mst_encoders[acrtc->crtc_id].base;
> --
> 2.34.1

RE: [PATCH v2] drm/amd/display: Drop unused DCN_BASE variable in dcn314_resource.c

[Li, Roman]

[Public]

Reviewed-by: Roman Li 

> -Original Message-
> From: SHANMUGAM, SRINIVASAN 
> Sent: Wednesday, May 31, 2023 1:46 PM
> To: Pillai, Aurabindo ; Mahfooz, Hamza
> ; Siqueira, Rodrigo
> ; Wentland, Harry
> ; Li, Roman 
> Cc: amd-gfx@lists.freedesktop.org; SHANMUGAM, SRINIVASAN
> 
> Subject: [PATCH v2] drm/amd/display: Drop unused DCN_BASE variable in
> dcn314_resource.c
>
> Fixes the following W=1 kernel build warning:
>
> drivers/gpu/drm/amd/amdgpu/../display/dc/dcn314/dcn314_resource.c:12
> 8:29: warning: ‘DCN_BASE’ defined but not used [-Wunused-const-variable=]
>   128 | static const struct IP_BASE DCN_BASE = { { { { 0x0012,
> 0x00C0, 0x34C0, 0x9000, 0x02403C00, 0, 0, 0 } },
>   | ^~~~
>
> Suggested-by: Roman Li 
> Cc: Hamza Mahfooz 
> Cc: Rodrigo Siqueira 
> Cc: Harry Wentland 
> Cc: Aurabindo Pillai 
> Signed-off-by: Srinivasan Shanmugam 
> ---
>
> v2:
>  - Remove even unused IP_BASE_INSTANCE and IP_BASE struct definitions
>altogether (Roman)
>
>  .../drm/amd/display/dc/dcn314/dcn314_resource.c | 17 -
>  1 file changed, 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/display/dc/dcn314/dcn314_resource.c
> b/drivers/gpu/drm/amd/display/dc/dcn314/dcn314_resource.c
> index 3592efcc7fae..837884c4f03a 100644
> --- a/drivers/gpu/drm/amd/display/dc/dcn314/dcn314_resource.c
> +++ b/drivers/gpu/drm/amd/display/dc/dcn314/dcn314_resource.c
> @@ -117,23 +117,6 @@
>  #define regBIF_BX2_BIOS_SCRATCH_60x003e
>  #define regBIF_BX2_BIOS_SCRATCH_6_BASE_IDX   1
>
> -struct IP_BASE_INSTANCE {
> - unsigned int segment[MAX_SEGMENT];
> -};
> -
> -struct IP_BASE {
> - struct IP_BASE_INSTANCE instance[MAX_INSTANCE];
> -};
> -
> -static const struct IP_BASE DCN_BASE = { { { { 0x0012, 0x00C0,
> 0x34C0, 0x9000, 0x02403C00, 0, 0, 0 } },
> - { { 0, 0, 0, 0, 0, 0, 0, 0 } },
> - { { 0, 0, 0, 0, 0, 0, 0, 0 } },
> - { { 0, 0, 0, 0, 0, 0, 0, 0 } },
> - { { 0, 0, 0, 0, 0, 0, 0, 0 } },
> - { { 0, 0, 0, 0, 0, 0, 0, 0 } },
> - { { 0, 0, 0, 0, 0, 0, 0, 0 } } } };
> -
> -
>  #define DC_LOGGER_INIT(logger)
>
>  enum dcn31_clk_src_array_id {
> --
> 2.25.1

RE: [PATCH v3] drm/amd/display: fix flickering caused by S/G mode

[Li, Roman]

[Public]

Reviewed-by: Roman Li 

> -Original Message-
> From: amd-gfx  On Behalf Of Hamza
> Mahfooz
> Sent: Thursday, April 20, 2023 9:44 AM
> To: amd-gfx@lists.freedesktop.org
> Cc: Wang, Chao-kai (Stylon) ; Tuikov, Luben
> ; dri-de...@lists.freedesktop.org; Li, Sun peng (Leo)
> ; David Airlie ; Zhuo, Qingqing
> (Lillian) ; Pan, Xinhui ;
> Siqueira, Rodrigo ; linux-ker...@vger.kernel.org;
> sta...@vger.kernel.org; Hans de Goede ; Pillai,
> Aurabindo ; Wu, Hersen
> ; Mahfooz, Hamza ;
> Daniel Vetter ; Deucher, Alexander
> ; Wentland, Harry
> ; Koenig, Christian 
> Subject: [PATCH v3] drm/amd/display: fix flickering caused by S/G mode
>
> Currently, on a handful of ASICs. We allow the framebuffer for a given plane 
> to
> exist in either VRAM or GTT. However, if the plane's new framebuffer is in a
> different memory domain than it's previous framebuffer, flipping between
> them can cause the screen to flicker. So, to fix this, don't perform an
> immediate flip in the aforementioned case.
>
> Cc: sta...@vger.kernel.org
> Link: https://gitlab.freedesktop.org/drm/amd/-/issues/2354
> Fixes: 81d0bcf99009 ("drm/amdgpu: make display pinning more flexible (v2)")
> Signed-off-by: Hamza Mahfooz 
> ---
> v2: make a number of clarifications to the commit message and drop
> locking.
> v3: use a stronger check
> ---
>  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c| 16 ++-
> -
>  1 file changed, 14 insertions(+), 2 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 dfcb9815b5a8..875111340203 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -7900,6 +7900,13 @@ static void amdgpu_dm_commit_cursors(struct
> drm_atomic_state *state)
>   amdgpu_dm_plane_handle_cursor_update(plane,
> old_plane_state);  }
>
> +static inline uint32_t get_mem_type(struct drm_framebuffer *fb) {
> + struct amdgpu_bo *abo = gem_to_amdgpu_bo(fb->obj[0]);
> +
> + return abo->tbo.resource ? abo->tbo.resource->mem_type : 0; }
> +
>  static void amdgpu_dm_commit_planes(struct drm_atomic_state *state,
>   struct dc_state *dc_state,
>   struct drm_device *dev,
> @@ -7919,6 +7926,7 @@ static void amdgpu_dm_commit_planes(struct
> drm_atomic_state *state,
>
>   to_dm_crtc_state(drm_atomic_get_old_crtc_state(state, pcrtc));
>   int planes_count = 0, vpos, hpos;
>   unsigned long flags;
> + uint32_t mem_type;
>   u32 target_vblank, last_flip_vblank;
>   bool vrr_active = amdgpu_dm_crtc_vrr_active(acrtc_state);
>   bool cursor_update = false;
> @@ -8040,13 +8048,17 @@ static void amdgpu_dm_commit_planes(struct
> drm_atomic_state *state,
>   }
>   }
>
> + mem_type = get_mem_type(old_plane_state->fb);
> +
>   /*
>* Only allow immediate flips for fast updates that don't
> -  * change FB pitch, DCC state, rotation or mirroing.
> +  * change memory domain, FB pitch, DCC state, rotation or
> +  * mirroring.
>*/
>   bundle->flip_addrs[planes_count].flip_immediate =
>   crtc->state->async_flip &&
> - acrtc_state->update_type == UPDATE_TYPE_FAST;
> + acrtc_state->update_type == UPDATE_TYPE_FAST &&
> + mem_type && get_mem_type(fb) == mem_type;
>
>   timestamp_ns = ktime_get_ns();
>   bundle->flip_addrs[planes_count].flip_timestamp_in_us =
> div_u64(timestamp_ns, 1000);
> --
> 2.40.0

RE: [PATCH 12/16] drm/amd/display: Add Function delaration in dc_link

[Li, Roman]

[AMD Official Use Only - General]

Please fix typo in commit title.

> -Original Message-
> From: Hung, Alex 
> Sent: Wednesday, January 25, 2023 7:32 PM
> To: amd-gfx@lists.freedesktop.org
> Cc: Wentland, Harry ; Li, Sun peng (Leo)
> ; Lakha, Bhawanpreet
> ; Siqueira, Rodrigo
> ; Pillai, Aurabindo
> ; Zhuo, Qingqing (Lillian)
> ; Li, Roman ; Lin, Wayne
> ; Wang, Chao-kai (Stylon)
> ; Chiu, Solomon ;
> Kotarac, Pavle ; Gutierrez, Agustin
> ; Ghaddar, Mustapha
> ; Lei, Jun ; Hung, Alex
> 
> Subject: [PATCH 12/16] drm/amd/display: Add Function delaration in dc_link
>
> From: Mustapha Ghaddar 
>
> [WHY]
> Housekeeping cleaning and adding declaration for function to be called from
> DM layer
>
> [HOW]
> Adding public functions to dc_link.h
>
> Reviewed-by: Jun Lei 
> Acked-by: Alex Hung 
> Signed-off-by: Mustapha Ghaddar 
> ---
>  drivers/gpu/drm/amd/display/dc/dc_link.h  | 27 +++
>  .../dc/link/protocols/link_dp_dpia_bw.h   | 24 -
>  2 files changed, 27 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/display/dc/dc_link.h
> b/drivers/gpu/drm/amd/display/dc/dc_link.h
> index 1927eacbfa71..85b57848f5cb 100644
> --- a/drivers/gpu/drm/amd/display/dc/dc_link.h
> +++ b/drivers/gpu/drm/amd/display/dc/dc_link.h
> @@ -627,4 +627,31 @@ struct fixed31_32
> calculate_sst_avg_time_slots_per_mtp(
>  void setup_dp_hpo_stream(struct pipe_ctx *pipe_ctx, bool enable);  void
> dp_source_sequence_trace(struct dc_link *link, uint8_t dp_test_mode);
>
> +/*
> + *  USB4 DPIA BW ALLOCATION PUBLIC FUNCTIONS  */
> +/*
> + * Send a request from DP-Tx requesting to allocate BW remotely after
> + * allocating it locally. This will get processed by CM and a CB
> +function
> + * will be called.
> + *
> + * @link: pointer to the dc_link struct instance
> + * @req_bw: The requested bw in Kbyte to allocated
> + *
> + * return: none
> + */
> +void dc_link_set_usb4_req_bw_req(struct dc_link *link, int req_bw);
> +
> +/*
> + * CB function for when the status of the Req above is complete. We
> +will
> + * find out the result of allocating on CM and update structs
> +accordingly
> + *
> + * @link: pointer to the dc_link struct instance
> + * @bw: Allocated or Estimated BW depending on the result
> + * @result: Response type
> + *
> + * return: none
> + */
> +void dc_link_get_usb4_req_bw_resp(struct dc_link *link, uint8_t bw,
> +uint8_t result);
> +
>  #endif /* DC_LINK_H_ */
> diff --git
> a/drivers/gpu/drm/amd/display/dc/link/protocols/link_dp_dpia_bw.h
> b/drivers/gpu/drm/amd/display/dc/link/protocols/link_dp_dpia_bw.h
> index 58eb7b581093..832a6dd2c5fa 100644
> --- a/drivers/gpu/drm/amd/display/dc/link/protocols/link_dp_dpia_bw.h
> +++ b/drivers/gpu/drm/amd/display/dc/link/protocols/link_dp_dpia_bw.h
> @@ -44,30 +44,6 @@ enum bw_type {
>   */
>  bool set_dptx_usb4_bw_alloc_support(struct dc_link *link);
>
> -/*
> - * Send a request from DP-Tx requesting to allocate BW remotely after
> - * allocating it locally. This will get processed by CM and a CB function
> - * will be called.
> - *
> - * @link: pointer to the dc_link struct instance
> - * @req_bw: The requested bw in Kbyte to allocated
> - *
> - * return: none
> - */
> -void set_usb4_req_bw_req(struct dc_link *link, int req_bw);
> -
> -/*
> - * CB function for when the status of the Req above is complete. We will
> - * find out the result of allocating on CM and update structs accordingly
> - *
> - * @link: pointer to the dc_link struct instance
> - * @bw: Allocated or Estimated BW depending on the result
> - * @result: Response type
> - *
> - * return: none
> - */
> -void get_usb4_req_bw_resp(struct dc_link *link, uint8_t bw, uint8_t
> result);
> -
>  /*
>   * Return the response_ready flag from dc_link struct
>   *
> --
> 2.39.1

RE: [PATCH 2/2] drm/amd/display: disable S/G display on DCN 3.1.4

[Li, Roman]

[Public]

Reviewed-by: Roman Li 

> -Original Message-
> From: Deucher, Alexander 
> Sent: Wednesday, January 18, 2023 9:26 AM
> To: amd-gfx@lists.freedesktop.org
> Cc: Deucher, Alexander ; Li, Roman
> ; Zhang, Yifan 
> Subject: [PATCH 2/2] drm/amd/display: disable S/G display on DCN 3.1.4
>
> Causes flickering or white screens in some configurations.
> Disable it for now until we can fix the issue.
>
> Cc: roman...@amd.com
> Cc: yifan1.zh...@amd.com
> Signed-off-by: Alex Deucher 
> ---
>  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> index 7167a7b63f8c..b8ae51345668 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -1502,7 +1502,6 @@ static int amdgpu_dm_init(struct amdgpu_device
> *adev)
>   case IP_VERSION(3, 0, 1):
>   case IP_VERSION(3, 1, 2):
>   case IP_VERSION(3, 1, 3):
> - case IP_VERSION(3, 1, 4):
>   case IP_VERSION(3, 1, 6):
>   init_data.flags.gpu_vm_support = true;
>   break;
> --
> 2.39.0

RE: [PATCH] drm/amd/display: Align dcn314_smu logging with other DCNs

[Li, Roman]

Hi Mario,

Thanks for your comments.
I replied  inline.

> -Original Message-
> From: Limonciello, Mario 
> Sent: Monday, November 14, 2022 4:16 PM
> To: Li, Roman ; amd-gfx@lists.freedesktop.org;
> Deucher, Alexander ; Wentland, Harry
> ; Rizvi, Saaem 
> Cc: Li, Roman 
> Subject: RE: [PATCH] drm/amd/display: Align dcn314_smu logging with other
> DCNs
> 
> [Public]
> 
> Conceptually makes sense to me, but please see below comments:
> 
> > -Original Message-
> > From: roman...@amd.com 
> > Sent: Monday, November 14, 2022 15:07
> > To: amd-gfx@lists.freedesktop.org; Deucher, Alexander
> > ; Wentland, Harry
> ;
> > Limonciello, Mario ; Rizvi, Saaem
> > 
> > Cc: Li, Roman 
> > Subject: [PATCH] drm/amd/display: Align dcn314_smu logging with other
> > DCNs
> >
> > From: Roman Li 
> >
> > [Why]
> > Assert on non-OK response from SMU is unnecessary.
> > It was replaced with respective log message on other asics in the past
> > with commit:
> > "drm/amd/display: Removing assert statements for Linux"
> >
> > [How]
> > Remove asert and add dbg logging as on other DCNs.

I will fix "assert" spelling before merging.

> >
> > CC: Saaem Rizvi 
> > Signed-off-by: Roman Li 
> > ---
> >  .../drm/amd/display/dc/clk_mgr/dcn314/dcn314_smu.c| 11 +-
> -
> >  1 file changed, 9 insertions(+), 2 deletions(-)
> >
> > diff --git
> > a/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn314/dcn314_smu.c
> > b/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn314/dcn314_smu.c
> > index ef0795b14a1f..2db595672a46 100644
> > --- a/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn314/dcn314_smu.c
> > +++ b/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn314/dcn314_smu.c
> > @@ -123,9 +123,10 @@ static int
> > dcn314_smu_send_msg_with_param(struct clk_mgr_internal *clk_mgr,
> > uint32_t result;
> >
> > result = dcn314_smu_wait_for_response(clk_mgr, 10, 20);
> > -   ASSERT(result == VBIOSSMC_Result_OK);
> 
> Does this flow actually happen still?  I thought the assertion should have
> gone away as a result of 83293f7f3d15fc56e86bd5067a2c88b6b233ac3a.
> 
Happens or not, we don't  assert here on other asics.
I don't try to fix any bugs with this patch, just align the dcn314 logging/bug 
reporting  with other asics.

> Maybe we want to also undo the REG_WRITE() call there if pulling this in.
> 
> >
> > -   smu_print("SMU response after wait: %d\n", result);
> > +   if (result != VBIOSSMC_Result_OK)
> > +   smu_print("SMU Response was not OK. SMU response after
> > wait received is: %d\n",
> > +   result);
> >
> > if (result == VBIOSSMC_Status_BUSY)
> > return -1;
> 
> I think what is missing to clean up recent asserts is actually a little bit 
> further
> in the code than this.  It should be part of the error flow introduced by
> 03ad3093c7c069d6ab4403730009ebafeea9ee37

03ad3093c7c069d6a is for dcn3.1 initially. 
If there's an issue with it (which I didn't experience) it should be addressed 
on all dcn3x, that reuse it,  in a  separate patch.


> 
> > @@ -216,6 +217,12 @@ int dcn314_smu_set_hard_min_dcfclk(struct
> > clk_mgr_internal *clk_mgr, int request
> > VBIOSSMC_MSG_SetHardMinDcfclkByFreq,
> > khz_to_mhz_ceil(requested_dcfclk_khz));
> >
> > +#ifdef DBG
> > +   smu_print("actual_dcfclk_set_mhz %d is set to : %d\n",
> > +   actual_dcfclk_set_mhz,
> > +   actual_dcfclk_set_mhz * 1000);
> > +#endif
> > +
> > return actual_dcfclk_set_mhz * 1000;  }
> >
> > --
> > 2.17.1

Thanks,
Roman

RE: [PATCH 07/21] drm/amd/display: Enable dlg and vba compilation for dcn314

[Li, Roman]

[Public]

Reviewed-by: Roman Li 

> -Original Message-
> From: Kotarac, Pavle 
> Sent: Wednesday, September 7, 2022 2:12 PM
> To: amd-gfx@lists.freedesktop.org
> Cc: Wentland, Harry ; Li, Sun peng (Leo)
> ; Lakha, Bhawanpreet
> ; Siqueira, Rodrigo
> ; Pillai, Aurabindo
> ; Zhuo, Qingqing (Lillian)
> ; Li, Roman ; Lin, Wayne
> ; Wang, Chao-kai (Stylon)
> ; Chiu, Solomon ;
> Kotarac, Pavle ; Gutierrez, Agustin
> 
> Subject: [PATCH 07/21] drm/amd/display: Enable dlg and vba compilation for
> dcn314
>
> From: Rodrigo Siqueira 
>
> We were not using the VBA and DLG files for DCN314, but the next sequence
> of changes for DCN314 will require those files. This commit adds the
> necessary files to the Makefile.
>
> Cc: Roman Li 
> Signed-off-by: Rodrigo Siqueira 
> ---
>  drivers/gpu/drm/amd/display/dc/dml/Makefile | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/display/dc/dml/Makefile
> b/drivers/gpu/drm/amd/display/dc/dml/Makefile
> index 86a3b5bfd699..cb81ed2fbd53 100644
> --- a/drivers/gpu/drm/amd/display/dc/dml/Makefile
> +++ b/drivers/gpu/drm/amd/display/dc/dml/Makefile
> @@ -70,6 +70,8 @@
> CFLAGS_$(AMDDALPATH)/dc/dml/dcn30/display_mode_vba_30.o :=
> $(dml_ccflags) $(fram
> CFLAGS_$(AMDDALPATH)/dc/dml/dcn30/display_rq_dlg_calc_30.o :=
> $(dml_ccflags)
> CFLAGS_$(AMDDALPATH)/dc/dml/dcn31/display_mode_vba_31.o :=
> $(dml_ccflags) $(frame_warn_flag)
> CFLAGS_$(AMDDALPATH)/dc/dml/dcn31/display_rq_dlg_calc_31.o :=
> $(dml_ccflags)
> +CFLAGS_$(AMDDALPATH)/dc/dml/dcn314/display_mode_vba_314.o :=
> +$(dml_ccflags) $(frame_warn_flag)
> +CFLAGS_$(AMDDALPATH)/dc/dml/dcn314/display_rq_dlg_calc_314.o :=
> +$(dml_ccflags)
>  CFLAGS_$(AMDDALPATH)/dc/dml/dcn314/dcn314_fpu.o := $(dml_ccflags)
> CFLAGS_$(AMDDALPATH)/dc/dml/dcn30/dcn30_fpu.o := $(dml_ccflags)
> CFLAGS_$(AMDDALPATH)/dc/dml/dcn32/dcn32_fpu.o := $(dml_ccflags) @@
> -123,6 +125,7 @@ DML += dcn20/display_rq_dlg_calc_20v2.o
> dcn20/display_mode_vba_20v2.o  DML += dcn21/display_rq_dlg_calc_21.o
> dcn21/display_mode_vba_21.o  DML += dcn30/dcn30_fpu.o
> dcn30/display_mode_vba_30.o dcn30/display_rq_dlg_calc_30.o  DML +=
> dcn31/display_mode_vba_31.o dcn31/display_rq_dlg_calc_31.o
> +DML += dcn314/display_mode_vba_314.o
> dcn314/display_rq_dlg_calc_314.o
>  DML += dcn32/display_mode_vba_32.o dcn32/display_rq_dlg_calc_32.o
> dcn32/display_mode_vba_util_32.o  DML += dcn31/dcn31_fpu.o  DML +=
> dcn32/dcn32_fpu.o
> --
> 2.34.1

RE: [PATCH] drm/amd/display: change family id name for DCN314

[Li, Roman]

[AMD Official Use Only - General]

Reviewed-by: Roman Li 

> -Original Message-
> From: Zhang, Yifan 
> Sent: Thursday, August 4, 2022 4:49 AM
> To: amd-gfx@lists.freedesktop.org
> Cc: Li, Roman ; Huang, Tim ;
> Deucher, Alexander ; Du, Xiaojian
> ; Zhang, Yifan 
> Subject: [PATCH] drm/amd/display: change family id name for DCN314
>
> GC version is 11.0.1 rather than 11.0.2
>
> Signed-off-by: Yifan Zhang 
> ---
>  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c | 4 ++--
>  drivers/gpu/drm/amd/display/dc/clk_mgr/clk_mgr.c| 4 ++--
>  drivers/gpu/drm/amd/display/dc/core/dc.c| 2 +-
>  drivers/gpu/drm/amd/display/dc/core/dc_link.c   | 2 +-
>  drivers/gpu/drm/amd/display/dc/core/dc_resource.c   | 2 +-
>  drivers/gpu/drm/amd/display/dc/dcn31/dcn31_resource.c   | 2 +-
>  drivers/gpu/drm/amd/display/include/dal_asic_id.h   | 2 +-
>  7 files changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
> index 516739bc0a96..3b4bf131bab1 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
> @@ -660,7 +660,7 @@ static int get_plane_modifiers(struct amdgpu_device
> *adev, unsigned int plane_ty
>   add_gfx10_1_modifiers(adev, mods, ,
> );
>   break;
>   case AMDGPU_FAMILY_GC_11_0_0:
> - case AMDGPU_FAMILY_GC_11_0_2:
> + case AMDGPU_FAMILY_GC_11_0_1:
>   add_gfx11_modifiers(adev, mods, , );
>   break;
>   }
> @@ -1408,7 +1408,7 @@ static bool
> dm_plane_format_mod_supported(struct drm_plane *plane,
>   }
>   break;
>   case AMDGPU_FAMILY_GC_11_0_0:
> - case AMDGPU_FAMILY_GC_11_0_2:
> + case AMDGPU_FAMILY_GC_11_0_1:
>   switch (AMD_FMT_MOD_GET(TILE, modifier)) {
>   case AMD_FMT_MOD_TILE_GFX11_256K_R_X:
>   case AMD_FMT_MOD_TILE_GFX9_64K_R_X:
> diff --git a/drivers/gpu/drm/amd/display/dc/clk_mgr/clk_mgr.c
> b/drivers/gpu/drm/amd/display/dc/clk_mgr/clk_mgr.c
> index 4c76091fd1f2..f276abb63bcd 100644
> --- a/drivers/gpu/drm/amd/display/dc/clk_mgr/clk_mgr.c
> +++ b/drivers/gpu/drm/amd/display/dc/clk_mgr/clk_mgr.c
> @@ -337,7 +337,7 @@ struct clk_mgr *dc_clk_mgr_create(struct dc_context
> *ctx, struct pp_smu_funcs *p
>   break;
>   }
>
> - case AMDGPU_FAMILY_GC_11_0_2: {
> + case AMDGPU_FAMILY_GC_11_0_1: {
>   struct clk_mgr_dcn314 *clk_mgr = kzalloc(sizeof(*clk_mgr),
> GFP_KERNEL);
>
>   if (clk_mgr == NULL) {
> @@ -397,7 +397,7 @@ void dc_destroy_clk_mgr(struct clk_mgr
> *clk_mgr_base)
>   dcn32_clk_mgr_destroy(clk_mgr);
>   break;
>
> - case AMDGPU_FAMILY_GC_11_0_2:
> + case AMDGPU_FAMILY_GC_11_0_1:
>   dcn314_clk_mgr_destroy(clk_mgr);
>   break;
>
> diff --git a/drivers/gpu/drm/amd/display/dc/core/dc.c
> b/drivers/gpu/drm/amd/display/dc/core/dc.c
> index e42f44fc1c08..5d84adb6ea52 100644
> --- a/drivers/gpu/drm/amd/display/dc/core/dc.c
> +++ b/drivers/gpu/drm/amd/display/dc/core/dc.c
> @@ -4292,7 +4292,7 @@ bool dc_is_dmub_outbox_supported(struct dc
> *dc)
>   !dc->debug.dpia_debug.bits.disable_dpia)
>   return true;
>
> - if (dc->ctx->asic_id.chip_family == AMDGPU_FAMILY_GC_11_0_2
> &&
> + if (dc->ctx->asic_id.chip_family == AMDGPU_FAMILY_GC_11_0_1
> &&
>   !dc->debug.dpia_debug.bits.disable_dpia)
>   return true;
>
> diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_link.c
> b/drivers/gpu/drm/amd/display/dc/core/dc_link.c
> index 9e51338441d0..66d2ae7aacf5 100644
> --- a/drivers/gpu/drm/amd/display/dc/core/dc_link.c
> +++ b/drivers/gpu/drm/amd/display/dc/core/dc_link.c
> @@ -3372,7 +3372,7 @@ bool dc_link_setup_psr(struct dc_link *link,
>   switch(link->ctx->asic_id.chip_family) {
>   case FAMILY_YELLOW_CARP:
>   case AMDGPU_FAMILY_GC_10_3_6:
> - case AMDGPU_FAMILY_GC_11_0_2:
> + case AMDGPU_FAMILY_GC_11_0_1:
>   if(!dc->debug.disable_z10)
>   psr_context-
> >psr_level.bits.SKIP_CRTC_DISABLE = false;
>   break;
> 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 ffc0f1c0ea93..7dbab15bfa68 100644
> --- a/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
> +++ b/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
&

RE: [PATCH 07/16] drm/amd/display: Cap OLED brightness per max frame-average luminance

[Li, Roman]

[AMD Official Use Only - General]

Hi Aaron,

Yes, the panel brightness should reach max after we send max-fall value as 
backlight current peak.

Thanks,
Roman

> -Original Message-
> From: Aaron Ma 
> Sent: Wednesday, June 8, 2022 6:02 AM
> To: Mahfooz, Hamza ; Li, Roman
> 
> Cc: Pillai, Aurabindo ; Lakha, Bhawanpreet
> ; Wentland, Harry
> ; Siqueira, Rodrigo
> ; Li, Sun peng (Leo) ;
> Gutierrez, Agustin ; amd-
> g...@lists.freedesktop.org; Zuo, Jerry ; Kotarac, Pavle
> ; Zhuo, Qingqing (Lillian)
> ; Chiu, Solomon ;
> Wang, Chao-kai (Stylon) ; Lin, Wayne
> 
> Subject: Re: [PATCH 07/16] drm/amd/display: Cap OLED brightness per max
> frame-average luminance
>
> Hi Roman:
>
> Can the panel achieve the max peak luminance if it is limited in frame-average
> luminance?
>
> Regards,
> Aaron

RE: [PATCH] drm/amd: add dc feature mask flags for PSR allow smu and multi-display optimizations

[Li, Roman]

[Public]

Reviewed-by: Roman Li 

> -Original Message-
> From: Zhang, Dingchen (David) 
> Sent: Monday, April 25, 2022 3:25 PM
> To: amd-gfx@lists.freedesktop.org
> Cc: Wentland, Harry ; Li, Sun peng (Leo)
> ; Lakha, Bhawanpreet
> ; Siqueira, Rodrigo
> ; Pillai, Aurabindo ;
> Zhuo, Qingqing (Lillian) ; Li, Roman
> ; Lin, Wayne ; Wang, Chao-kai
> (Stylon) ; Chiu, Solomon ;
> Kotarac, Pavle ; Gutierrez, Agustin
> ; Zuo, Jerry 
> Subject: [PATCH] drm/amd: add dc feature mask flags for PSR allow smu and
> multi-display optimizations
>
> [Why]
> Allow for PSR SMU optimization and PSR multiple display optimization.
>
> [How]
> Add feature flags of PSR smu optimization and PSR multiple display
> optimiztaion, and set them during init sequence. By default, flags are 
> disabled.
>
> Signed-off-by: David Zhang 
> ---
>  drivers/gpu/drm/amd/include/amd_shared.h | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/include/amd_shared.h
> b/drivers/gpu/drm/amd/include/amd_shared.h
> index 741dae17562a..06f21e9008c6 100644
> --- a/drivers/gpu/drm/amd/include/amd_shared.h
> +++ b/drivers/gpu/drm/amd/include/amd_shared.h
> @@ -234,6 +234,8 @@ enum DC_FEATURE_MASK {
>   DC_EDP_NO_POWER_SEQUENCING = (1 << 4), //0x10, disabled by
> default
>   DC_DISABLE_LTTPR_DP1_4A = (1 << 5), //0x20, disabled by default
>   DC_DISABLE_LTTPR_DP2_0 = (1 << 6), //0x40, disabled by default
> + DC_PSR_ALLOW_SMU_OPT = (1 << 7), //0x80, disabled by default
> + DC_PSR_ALLOW_MULTI_DISP_OPT = (1 << 8), //0x100, disabled by
> default
>  };
>
>  enum DC_DEBUG_MASK {
> --
> 2.25.1

RE: [PATCH] drm/amd/display: Don't fill up the logs

[Li, Roman]

[AMD Official Use Only]

Definitely overflowed log makes no sense.
"ONCE" is better than nothing for debugging.
With that said:

Reviewed-by: Roman Li 

> -Original Message-
> From: Tuikov, Luben 
> Sent: Thursday, February 24, 2022 5:50 PM
> To: Alex Deucher 
> Cc: amd-gfx list ; Kuehling, Felix
> ; Li, Roman ; Wu, Hersen
> ; Wheeler, Daniel ;
> Deucher, Alexander 
> Subject: Re: [PATCH] drm/amd/display: Don't fill up the logs
>
> I could, but if they enable KMS debug, this prints 100s of times a second.
> It literally overflows the log. It needs to be printed "ONCE" in whichever 
> mode.
> Either that, or not print it at all--it is "DEBUG" after all.
>
> Regards,
> Luben
>
> On 2022-02-24 17:21, Alex Deucher wrote:
> > On Thu, Feb 24, 2022 at 4:46 PM Luben Tuikov 
> wrote:
> >>
> >> Don't fill up the logs with:
> >>
> >> [253557.859575] [drm:amdgpu_dm_atomic_check [amdgpu]] DSC
> precompute is not needed.
> >> [253557.892966] [drm:amdgpu_dm_atomic_check [amdgpu]] DSC
> precompute is not needed.
> >> [253557.926070] [drm:amdgpu_dm_atomic_check [amdgpu]] DSC
> precompute is not needed.
> >> [253557.959344] [drm:amdgpu_dm_atomic_check [amdgpu]] DSC
> precompute is not needed.
> >>
> >> which prints many times a second, when the kernel is run with
> >> drm.debug=2.
> >>
> >> Instead of DRM_DEBUG_DRIVER(), make it DRM_INFO_ONCE().
> >
> > Maybe convert to DRM_DEBUG_KMS() for consistency with the rest of the
> > display code?
> >
> > Alex
> >
> >>
> >> Cc: Alex Deucher 
> >> Cc: Roman Li 
> >> Cc: Felix Kuehling 
> >> Cc: Hersen Wu 
> >> Cc: Daniel Wheeler 
> >> Fixes: 5898243ba7acdb ("drm/amd/display: Add dsc pre-validation in
> >> atomic check")
> >> Signed-off-by: Luben Tuikov 
> >> ---
> >>  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c   | 8 -
> ---
> >>  1 file changed, 4 insertions(+), 4 deletions(-)
> >>
> >> diff --git
> >> a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
> >> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
> >> index 05573f073b21f7..0542034530b1b0 100644
> >> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
> >> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
> >> @@ -1198,11 +1198,11 @@ void pre_validate_dsc(struct drm_atomic_state
> *state,
> >> struct dc_state *local_dc_state = NULL;
> >>
> >> if (!is_dsc_precompute_needed(state)) {
> >> -   DRM_DEBUG_DRIVER("DSC precompute is not needed.\n");
> >> +   DRM_INFO_ONCE("DSC precompute is not needed.\n");
> >> return;
> >> }
> >> if (dm_atomic_get_state(state, dm_state_ptr)) {
> >> -   DRM_DEBUG_DRIVER("dm_atomic_get_state() failed\n");
> >> +   DRM_INFO_ONCE("dm_atomic_get_state() failed\n");
> >> return;
> >> }
> >> dm_state = *dm_state_ptr;
> >> @@ -1245,7 +1245,7 @@ void pre_validate_dsc(struct drm_atomic_state
> *state,
> >> }
> >>
> >> if (!pre_compute_mst_dsc_configs_for_state(state, local_dc_state,
> vars)) {
> >> -
> DRM_DEBUG_DRIVER("pre_compute_mst_dsc_configs_for_state() failed\n");
> >> +
> >> + DRM_INFO_ONCE("pre_compute_mst_dsc_configs_for_state() failed\n");
> >> goto clean_exit;
> >> }
> >>
> >> @@ -1258,7 +1258,7 @@ void pre_validate_dsc(struct drm_atomic_state
> >> *state,
> >>
> >> if (local_dc_state->streams[i] &&
> >> is_timing_changed(stream, local_dc_state->streams[i])) 
> >> {
> >> -   DRM_DEBUG_DRIVER("crtc[%d] needs mode_changed\n", 
> >> i);
> >> +   DRM_INFO_ONCE("crtc[%d] needs
> >> + mode_changed\n", i);
> >> } else {
> >> int ind =
> >> find_crtc_index_in_state_by_stream(state, stream);
> >>
> >>
> >> base-commit: bff980c7632ef3b0099ba230cf7d5c864db3e1a3
> >> --
> >> 2.35.1.129.gb80121027d
> >>
>
> Regards,
> --
> Luben

RE: Backlight control broken on UM325 (OLED) on 5.15 (bisected)

[Li, Roman]

[Public]

Hi Samuel,

Can you please try: https://patchwork.freedesktop.org/patch/463485/ ?

Thanks,
Roman

> -Original Message-
> From: Samuel Čavoj 
> Sent: Tuesday, November 16, 2021 8:33 AM
> To: Alex Deucher 
> Cc: Deucher, Alexander ; Li, Sun peng (Leo)
> ; Li, Roman ; Maling list - DRI
> developers ; LKML  ker...@vger.kernel.org>; amd-gfx list 
> Subject: Re: Backlight control broken on UM325 (OLED) on 5.15 (bisected)
>
> Hi Alex,
>
> thank you for your response.
>
> On 15.11.2021 10:43, Alex Deucher wrote:
> > [...]
> >
> > That patch adds support for systems with multiple backlights.  Do you
> > have multiple backlight devices now?  If so, does the other one work?
>
> No, there is still only one backlight device -- amdgpu_bl0.
> >
> > Can you also try this patch?
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
> > index 4811b0faafd9..67163c9d49e6 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
> > @@ -854,8 +854,8 @@ int amdgpu_acpi_init(struct amdgpu_device *adev)
> > if (amdgpu_device_has_dc_support(adev)) {  #if
> > defined(CONFIG_DRM_AMD_DC)
> > struct amdgpu_display_manager *dm = >dm;
> > -   if (dm->backlight_dev[0])
> > -   atif->bd = dm->backlight_dev[0];
> > +   if (dm->backlight_dev[1])
> > +   atif->bd = dm->backlight_dev[1];
> >  #endif
> > } else {
> > struct drm_encoder *tmp;
> >
>
> There is no difference in behaviour after applying the patch.
>
> Samuel
>
> >
> > Alex
> >
> > >
> > > Regards,
> > > Samuel Čavoj
> > >
> > > [0]:
> > > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fww
> > >
> w.reddit.com%2Fr%2FAMDLaptops%2Fcomments%2Fqst0fm%2Fafter_updating
> _t
> > >
> o_linux_515_my_brightness%2Fdata=04%7C01%7CRoman.Li%40amd.co
> m%7
> > >
> Ce1c766a2f7014cdb664308d9a9059cc6%7C3dd8961fe4884e608e11a82d994e1
> 83d
> > >
> %7C0%7C0%7C637726663861883494%7CUnknown%7CTWFpbGZsb3d8eyJWIjoi
> MC4wLj
> > >
> AwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000s
> dat
> > >
> a=hfsaEzng9%2FjAI2F%2BKg87Tv2Mu%2FfPurCQELr62%2B%2FVF%2BQ%3D
> mp;res
> > > erved=0

RE: [PATCH v3] drm/amd/display: Look at firmware version to determine using dmub on dcn21

[Li, Roman]

[Public]

Reviewed-by: Roman Li 

> -Original Message-
> From: Alex Deucher 
> Sent: Tuesday, November 2, 2021 11:58 AM
> To: Limonciello, Mario 
> Cc: amd-gfx list ; Li, Roman
> 
> Subject: Re: [PATCH v3] drm/amd/display: Look at firmware version to
> determine using dmub on dcn21
>
> On Tue, Nov 2, 2021 at 11:04 AM Mario Limonciello
>  wrote:
> >
> > commit b1c61212d8dc ("drm/amd/display: Fully switch to dmub for all
> > dcn21
> > asics") switched over to using dmub on Renoir to fix Gitlab 1735, but
> > this implied a new dependency on newer firmware which might not be met
> > on older kernel versions.
> >
> > Since sw_init runs before hw_init, there is an opportunity to
> > determine whether or not the firmware version is new to adjust the behavior.
> >
> > Cc: roman...@amd.com
> > BugLink:
> > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitl
> > ab.freedesktop.org%2Fdrm%2Famd%2F-
> %2Fissues%2F1772data=04%7C01%7C
> >
> Roman.Li%40amd.com%7C29e4fd818a2446e0cf5208d99e198897%7C3dd8961f
> e4884e
> >
> 608e11a82d994e183d%7C0%7C0%7C637714654774196023%7CUnknown%7CT
> WFpbGZsb3
> >
> d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3
> D%7
> >
> C1000sdata=NrV1aszBAZkJ%2FQELpzxx7NCKXjWKPEvgk6rEAwfhWYk%3D
> r
> > eserved=0
> > BugLink:
> > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitl
> > ab.freedesktop.org%2Fdrm%2Famd%2F-
> %2Fissues%2F1735data=04%7C01%7C
> >
> Roman.Li%40amd.com%7C29e4fd818a2446e0cf5208d99e198897%7C3dd8961f
> e4884e
> >
> 608e11a82d994e183d%7C0%7C0%7C637714654774196023%7CUnknown%7CT
> WFpbGZsb3
> >
> d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3
> D%7
> >
> C1000sdata=EqjhSIwtkgo%2BtJFwUkf%2FDTY4RiSNVSh8DjXLu7LAszM%3
> D
> > ;reserved=0
> > Fixes: b1c61212d8dc ("drm/amd/display: Fully switch to dmub for all
> > dcn21 asics")
> > Signed-off-by: Mario Limonciello 
>
> Acked-by: Alex Deucher 
>
> > ---
> > Changes from v2->v3:
> >  * Don't special case Green Sardine / Renoir anymore
> >  * Special case the two Renoir firmware that have this problem
> >(that weren't released for green sardine)
> >  * Special case development version of ucode version
> > drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 8 
> >  1 file changed, 8 insertions(+)
> >
> > 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 b48f3cb5ddeb..dcc97ef28c50 100644
> > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> > @@ -1410,7 +1410,15 @@ static int amdgpu_dm_init(struct amdgpu_device
> *adev)
> > switch (adev->ip_versions[DCE_HWIP][0]) {
> > case IP_VERSION(2, 1, 0):
> > init_data.flags.gpu_vm_support = true;
> > +   switch (adev->dm.dmcub_fw_version) {
> > +   case 0: /* development */
> > +   case 0x1: /* linux-firmware.git hash 6d9f399 */
> > +   case 0x0100: /* linux-firmware.git hash 9a0b0f4 
> > */
> > +   init_data.flags.disable_dmcu = false;
> > +   break;
> > +   default:
> > init_data.flags.disable_dmcu = true;
> > +   }
> > break;
> > case IP_VERSION(1, 0, 0):
> > case IP_VERSION(1, 0, 1):
> > --
> > 2.25.1
> >

RE: [PATCH v2] drm/amd/display: Look at firmware version to determine using dmub on dcn21

[Li, Roman]

[Public]

> -Original Message-
> From: Alex Deucher 
> Sent: Friday, October 29, 2021 4:34 PM
> To: Limonciello, Mario 
> Cc: amd-gfx list ; Li, Roman
> 
> Subject: Re: [PATCH v2] drm/amd/display: Look at firmware version to
> determine using dmub on dcn21
>
> On Fri, Oct 29, 2021 at 4:33 PM Mario Limonciello
>  wrote:
> >
> > commit b1c61212d8dc ("drm/amd/display: Fully switch to dmub for all
> > dcn21
> > asics") switched over to using dmub on Renoir to fix Gitlab 1735, but
> > this implied a new dependency on newer firmware which might not be met
> > on older kernel versions.
> >
> > Since sw_init runs before hw_init, there is an opportunity to
> > determine whether or not the firmware version is new to adjust the behavior.
> >
> > Cc: roman...@amd.com
> > BugLink:
> > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitl
> > ab.freedesktop.org%2Fdrm%2Famd%2F-
> %2Fissues%2F1772data=04%7C01%7C
> >
> Roman.Li%40amd.com%7C4e27c983112e4ffdd36008d99b1b860a%7C3dd8961f
> e4884e
> >
> 608e11a82d994e183d%7C0%7C0%7C637711364793611804%7CUnknown%7CT
> WFpbGZsb3
> >
> d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3
> D%7
> >
> C1000sdata=cFwunb4aAJbFkCe6lIFMY4oWfbkCVWAGshe8lB0rg0U%3D&
> amp;res
> > erved=0
> > BugLink:
> > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitl
> > ab.freedesktop.org%2Fdrm%2Famd%2F-
> %2Fissues%2F1735data=04%7C01%7C
> >
> Roman.Li%40amd.com%7C4e27c983112e4ffdd36008d99b1b860a%7C3dd8961f
> e4884e
> >
> 608e11a82d994e183d%7C0%7C0%7C637711364793611804%7CUnknown%7CT
> WFpbGZsb3
> >
> d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3
> D%7
> >
> C1000sdata=%2F9sfVD5PN2tsl0bXkctPzkrHpJKQoZDex8xCDt1bVPg%3D&
> amp;r
> > eserved=0
> > Fixes: b1c61212d8dc ("drm/amd/display: Fully switch to dmub for all
> > dcn21 asics")
> > Signed-off-by: Mario Limonciello 
>
> Acked-by: Alex Deucher 
>
> > ---
> >  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > 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 6dd6262f2769..e7ff8ad4c5a7 100644
> > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> > @@ -1410,7 +1410,10 @@ static int amdgpu_dm_init(struct amdgpu_device
> *adev)
> > switch (adev->ip_versions[DCE_HWIP][0]) {
> > case IP_VERSION(2, 1, 0):
> > init_data.flags.gpu_vm_support = true;
> > +   if
> > + (ASICREV_IS_GREEN_SARDINE(adev->external_rev_id))
> > init_data.flags.disable_dmcu = true;
> > +   else
> > +   init_data.flags.disable_dmcu =
> > + adev->dm.dmcub_fw_version > 0x0100;

Since this is the only fw version that needs dmcu, it's safer to use equal 
condition here.

> > break;
> > case IP_VERSION(1, 0, 0):
> > case IP_VERSION(1, 0, 1):
> > --
> > 2.25.1
> >

RE: [PATCH] drm/amd/display: Fully switch to dmub for all dcn21 asics

[Li, Roman]

[Public]

> -Original Message-
> From: Wentland, Harry 
> Sent: Monday, October 18, 2021 9:57 AM
> To: Limonciello, Mario ; Li, Roman
> ; amd-gfx@lists.freedesktop.org; Deucher, Alexander
> ; Siqueira, Rodrigo
> 
> Cc: sta...@vger.kernel.org
> Subject: Re: [PATCH] drm/amd/display: Fully switch to dmub for all dcn21 asics
>
>
>
> On 2021-10-18 09:41, Limonciello, Mario wrote:
> > On 10/15/2021 17:31, roman...@amd.com wrote:
> >> From: Roman Li 
> >>
> >> [Why]
> >> On renoir usb-c port stops functioning on resume after f/w update.
> >> New dmub firmware caused regression due to conflict with dmcu.
> >> With new dmub f/w dmcu is superseded and should be disabled.
> >>
> >> [How]
> >> - Disable dmcu for all dcn21.
> >>
> >> Check dmesg for dmub f/w version.
> >> The old firmware (before regression):
> >> [drm] DMUB hardware initialized: version=0x0001 All other
> >> versions require that patch for renoir.
> >>
> >> Bug: https://gitlab.freedesktop.org/drm/amd/-/issues/1735>>> Cc:
> >> sta...@vger.kernel.org
> >
> > This won't backport cleanly to stable 5.15 and earlier don't use IP version 
> > to
> detect the chip.
> >
> > Also - a question: *should* this go to stable?  If a user has the older FW 
> > what
> happens with this change?
> >
>
> Good point. Might be better of we drop Cc: stable from this patch
>
> Harry

I will drop  Cc: sta...@vger.kernel.org, thank you Mario and Harry.
- Roman

> >> Signed-off-by: Roman Li 
> >> ---
> >>   drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 3 +--
> >>   1 file changed, 1 insertion(+), 2 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 ff54550..e56f73e 100644
> >> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> >> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> >> @@ -1356,8 +1356,7 @@ static int amdgpu_dm_init(struct amdgpu_device
> >> *adev)
> >>   switch (adev->ip_versions[DCE_HWIP][0]) {
> >>   case IP_VERSION(2, 1, 0):
> >>   init_data.flags.gpu_vm_support = true;
> >> -if (ASICREV_IS_GREEN_SARDINE(adev->external_rev_id))
> >> -init_data.flags.disable_dmcu = true;
> >> +init_data.flags.disable_dmcu = true;
> >>   break;
> >>   case IP_VERSION(1, 0, 0):
> >>   case IP_VERSION(1, 0, 1):
> >>
> >

RE: [PATCH V2] drm/amdgpu/display: add support for multiple backlights

[Li, Roman]

[Public]

Reviewed-by: Roman Li 

-Original Message-
From: amd-gfx  On Behalf Of Alex Deucher
Sent: Wednesday, July 14, 2021 3:00 PM
To: amd-gfx@lists.freedesktop.org
Cc: Deucher, Alexander ; kernel test robot 

Subject: [PATCH V2] drm/amdgpu/display: add support for multiple backlights

On platforms that support multiple backlights, register each one separately.  
This lets us manage them independently rather than registering a single 
backlight and applying the same settings to both.

v2: fix typo:
Reported-by: kernel test robot 

Signed-off-by: Alex Deucher 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c  |   4 +-
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 147 ++
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h |   4 +-
 3 files changed, 84 insertions(+), 71 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
index 59373c697b00..9122dcf42c46 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
@@ -854,8 +854,8 @@ int amdgpu_acpi_init(struct amdgpu_device *adev)
if (amdgpu_device_has_dc_support(adev)) {  #if 
defined(CONFIG_DRM_AMD_DC)
struct amdgpu_display_manager *dm = >dm;
-   if (dm->backlight_dev)
-   atif->bd = dm->backlight_dev;
+   if (dm->backlight_dev[0])
+   atif->bd = dm->backlight_dev[0];
 #endif
} else {
struct drm_encoder *tmp;
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 5cdb0e75d36b..2d48bb09645f 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -2412,6 +2412,7 @@ static void update_connector_ext_caps(struct 
amdgpu_dm_connector *aconnector)
static const u8 pre_computed_values[] = {
50, 51, 52, 53, 55, 56, 57, 58, 59, 61, 62, 63, 65, 66, 68, 69,
71, 72, 74, 75, 77, 79, 81, 82, 84, 86, 88, 90, 92, 94, 96, 98};
+   int i;

if (!aconnector || !aconnector->dc_link)
return;
@@ -2423,7 +2424,13 @@ static void update_connector_ext_caps(struct 
amdgpu_dm_connector *aconnector)
conn_base = >base;
adev = drm_to_adev(conn_base->dev);
dm = >dm;
-   caps = >backlight_caps;
+   for (i = 0; i < dm->num_of_edps; i++) {
+   if (link == dm->backlight_link[i])
+   break;
+   }
+   if (i >= dm->num_of_edps)
+   return;
+   caps = >backlight_caps[i];
caps->ext_caps = >dc_link->dpcd_sink_ext_caps;
caps->aux_support = false;
max_cll = conn_base->hdr_sink_metadata.hdmi_type1.max_cll;
@@ -3423,35 +3430,36 @@ static int amdgpu_dm_mode_config_init(struct 
amdgpu_device *adev)  #if defined(CONFIG_BACKLIGHT_CLASS_DEVICE) ||\
defined(CONFIG_BACKLIGHT_CLASS_DEVICE_MODULE)

-static void amdgpu_dm_update_backlight_caps(struct amdgpu_display_manager *dm)
+static void amdgpu_dm_update_backlight_caps(struct amdgpu_display_manager *dm,
+   int bl_idx)
 {
 #if defined(CONFIG_ACPI)
struct amdgpu_dm_backlight_caps caps;

memset(, 0, sizeof(caps));

-   if (dm->backlight_caps.caps_valid)
+   if (dm->backlight_caps[bl_idx].caps_valid)
return;

amdgpu_acpi_get_backlight_caps();
if (caps.caps_valid) {
-   dm->backlight_caps.caps_valid = true;
+   dm->backlight_caps[bl_idx].caps_valid = true;
if (caps.aux_support)
return;
-   dm->backlight_caps.min_input_signal = caps.min_input_signal;
-   dm->backlight_caps.max_input_signal = caps.max_input_signal;
+   dm->backlight_caps[bl_idx].min_input_signal = 
caps.min_input_signal;
+   dm->backlight_caps[bl_idx].max_input_signal = 
caps.max_input_signal;
} else {
-   dm->backlight_caps.min_input_signal =
+   dm->backlight_caps[bl_idx].min_input_signal =
AMDGPU_DM_DEFAULT_MIN_BACKLIGHT;
-   dm->backlight_caps.max_input_signal =
+   dm->backlight_caps[bl_idx].max_input_signal =
AMDGPU_DM_DEFAULT_MAX_BACKLIGHT;
}
 #else
-   if (dm->backlight_caps.aux_support)
+   if (dm->backlight_caps[bl_idx].aux_support)
return;

-   dm->backlight_caps.min_input_signal = AMDGPU_DM_DEFAULT_MIN_BACKLIGHT;
-   dm->backlight_caps.max_input_signal = AMDGPU_DM_DEFAULT_MAX_BACKLIGHT;
+   dm->backlight_caps[bl_idx].min_input_signal = 
AMDGPU_DM_DEFAULT_MIN_BACKLIGHT;
+   dm->backlight_caps[bl_idx].max_input_signal =
+AMDGPU_DM_DEFAULT_MAX_BACKLIGHT;
 #endif
 }

@@ -3502,41 +3510,31 @@ static u32 

RE: [PATCH] drm/amdgpu/display: only enable aux backlight control for OLED panels

[Li, Roman]

[Public]

Reviewed-by: Roman Li 

-Original Message-
From: amd-gfx  On Behalf Of Alex Deucher
Sent: Wednesday, July 21, 2021 6:18 PM
To: amd-gfx@lists.freedesktop.org
Cc: Deucher, Alexander 
Subject: [PATCH] drm/amdgpu/display: only enable aux backlight control for OLED 
panels

We've gotten a number of reports about backlight control not working on panels 
which indicate that they use aux backlight control.  A recent patch:

commit 2d73eabe2984a435737498ab39bb1500a9ffe9a9
Author: Camille Cho 
Date:   Thu Jul 8 18:28:37 2021 +0800

drm/amd/display: Only set default brightness for OLED

[Why]
We used to unconditionally set backlight path as AUX for panels capable
of backlight adjustment via DPCD in set default brightness.

[How]
This should be limited to OLED panel only since we control backlight via
PWM path for SDR mode in LCD HDR panel.

Reviewed-by: Krunoslav Kovac 
Acked-by: Rodrigo Siqueira 
Signed-off-by: Camille Cho 
Signed-off-by: Alex Deucher 

Changes some other code to only use aux for backlight control on OLED panels.  
The commit message seems to indicate that PWM should be used for SDR mode on 
HDR panels.  Do something similar for backlight control in general.  This may 
need to be revisited if and when HDR started to get used.

Bug: 
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitlab.freedesktop.org%2Fdrm%2Famd%2F-%2Fissues%2F1438data=04%7C01%7Croman.li%40amd.com%7Caf093dc8782244c1a61008d94c9579a5%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637625027141336218%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000sdata=lSPNjXlohpn%2FZ4kRFp0Y7Ir4qY8pNBSbHiGlccvBSRY%3Dreserved=0
Bug: 
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.kernel.org%2Fshow_bug.cgi%3Fid%3D213715data=04%7C01%7Croman.li%40amd.com%7Caf093dc8782244c1a61008d94c9579a5%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637625027141336218%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000sdata=Km4sba6yF0UFMd8mTCamMJ2SaZrFQSkc7PNDLmptsO4%3Dreserved=0
Signed-off-by: Alex Deucher 
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 4 ++--
 1 file changed, 2 insertions(+), 2 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 12db13d2bce9..986c9d29d686 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -2436,9 +2436,9 @@ static void update_connector_ext_caps(struct 
amdgpu_dm_connector *aconnector)
max_cll = conn_base->hdr_sink_metadata.hdmi_type1.max_cll;
min_cll = conn_base->hdr_sink_metadata.hdmi_type1.min_cll;

-   if (caps->ext_caps->bits.oled == 1 ||
+   if (caps->ext_caps->bits.oled == 1 /*||
caps->ext_caps->bits.sdr_aux_backlight_control == 1 ||
-   caps->ext_caps->bits.hdr_aux_backlight_control == 1)
+   caps->ext_caps->bits.hdr_aux_backlight_control == 1*/)
caps->aux_support = true;

if (amdgpu_backlight == 0)
--
2.31.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfxdata=04%7C01%7Croman.li%40amd.com%7Caf093dc8782244c1a61008d94c9579a5%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637625027141336218%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000sdata=ttvmpBk9TOJvgr3HBPIW0VFFHvxiAZ4nvNx2pJfpz08%3Dreserved=0
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

RE: [PATCH] drm/amd/display: Allow bandwidth validation for 0 streams.

[Li, Roman]

[Public]

Reviewed-by: Roman Li 

-Original Message-
From: Bindu Ramamurthy 
Sent: Thursday, May 20, 2021 5:04 PM
To: amd-gfx@lists.freedesktop.org
Cc: Li, Roman ; R, Bindu 
Subject: [PATCH] drm/amd/display: Allow bandwidth validation for 0 streams.

[Why]
Bandwidth calculations are triggered for non zero streams, and
in case of 0 streams these calculations were skipped with
pstate status not being updated.

[How]
As the pstate status is applicable for non zero streams, check
added for allowing 0 streams inline with dcn internal bandwidth
validations.

Signed-off-by: Bindu Ramamurthy 
---
 drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c 
b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c
index 6a56a03cfba3..0b49420e7d64 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c
+++ b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c
@@ -3236,7 +3236,7 @@ static noinline bool dcn20_validate_bandwidth_fp(struct 
dc *dc,
voltage_supported = dcn20_validate_bandwidth_internal(dc, context, 
false);
dummy_pstate_supported = 
context->bw_ctx.bw.dcn.clk.p_state_change_support;

-   if (voltage_supported && dummy_pstate_supported) {
+   if (voltage_supported && (dummy_pstate_supported || 
!(context->stream_count))) {
context->bw_ctx.bw.dcn.clk.p_state_change_support = false;
goto restore_dml_state;
}
--
2.25.1

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

RE: [PATCH] drm/amd/display: disable dcn10 pipe split by default

[Li, Roman]

[AMD Public Use]


Reviewed-by: Roman Li mailto:roman...@amd.com>>

From: Wu, Hersen 
Sent: Wednesday, December 30, 2020 12:12 PM
To: amd-gfx@lists.freedesktop.org; Deucher, Alexander 
; Li, Roman 
Subject: [PATCH] drm/amd/display: disable dcn10 pipe split by default


[Why] the initial purpose of dcn10 pipe split is to support
some high bandwidth mode which requires dispclk greater
than max dispclk. By initial bring up power measurement
data, it showed power consumption is less with pipe split
for dcn block. This could be reason for enable pipe split
by default. By battery life measurement of some Chromebooks,
result shows battery life is longer with pipe split disabled.

[How] disable pipe split by default. Pipe split could be
still enabled when required dispclk is greater than max dispclk.

Signed-off-by: hersen wu mailto:hersenxs...@amd.com>>
---
 drivers/gpu/drm/amd/display/dc/dcn10/dcn10_resource.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_resource.c 
b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_resource.c
index bdc37831535e..17eafe209946 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_resource.c
+++ b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_resource.c
@@ -608,8 +608,8 @@ static const struct dc_debug_options debug_defaults_drv = {
  .disable_pplib_clock_request = false,
  .disable_pplib_wm_range = false,
  .pplib_wm_report_mode = WM_REPORT_DEFAULT,
- .pipe_split_policy = MPC_SPLIT_DYNAMIC,
- .force_single_disp_pipe_split = true,
+ .pipe_split_policy = MPC_SPLIT_AVOID,
+ .force_single_disp_pipe_split = false,
  .disable_dcc = DCC_ENABLE,
  .voltage_align_fclk = true,
  .disable_stereo_support = true,
--
2.17.1

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

RE: [PATCH] drm/amd/display: Fix the display corruption issue on Navi10

[Li, Roman]

[AMD Public Use]

Reviewed-by: Roman Li 

-Original Message-
From: Yifan Zhang  
Sent: Wednesday, October 21, 2020 10:24 AM
To: amd-gfx@lists.freedesktop.org
Cc: Li, Sun peng (Leo) ; Wentland, Harry 
; Li, Roman ; Zhang, Yifan1 
; Deucher, Alexander 
Subject: [PATCH] drm/amd/display: Fix the display corruption issue on Navi10

[Why]
Screen corruption on Navi10 card

[How]
Set system context in DCN only on Renoir

Acked-by: Alex Deucher 
Signed-off-by: Yifan Zhang 
Signed-off-by: Roman Li 
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 10 +++---
 1 file changed, 7 insertions(+), 3 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 71ecd963759e..ed0e7680b420 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -1061,10 +1061,14 @@ static int amdgpu_dm_init(struct amdgpu_device *adev)
 
dc_hardware_init(adev->dm.dc);
 
-   mmhub_read_system_context(adev, _config);
+#if defined(CONFIG_DRM_AMD_DC_DCN)
+   if (adev->asic_type == CHIP_RENOIR) {
+   mmhub_read_system_context(adev, _config);
 
-   // Call the DC init_memory func
-   dc_setup_system_context(adev->dm.dc, _config);
+   // Call the DC init_memory func
+   dc_setup_system_context(adev->dm.dc, _config);
+   }
+#endif
 
adev->dm.freesync_module = mod_freesync_create(adev->dm.dc);
if (!adev->dm.freesync_module) {
-- 
2.25.1
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

RE: [PATCH] drm/amd/display: Only revalidate bandwidth on medium and fast updates

[Li, Roman]

[AMD Public Use]

Reviewed-by: Roman Li 

-Original Message-
From: amd-gfx  On Behalf Of Nicholas 
Kazlauskas
Sent: Monday, June 29, 2020 4:49 PM
To: amd-gfx@lists.freedesktop.org
Cc: Lakha, Bhawanpreet ; Siqueira, Rodrigo 
; Wu, Hersen ; Kazlauskas, 
Nicholas 
Subject: [PATCH] drm/amd/display: Only revalidate bandwidth on medium and fast 
updates

[Why]
Changes that are fast don't require updating DLG parameters making this call 
unnecessary. Considering this is an expensive call it should not be done on 
every flip.

DML touches clocks, p-state support, DLG params and a few other DC internal 
flags and these aren't expected during fast. A hang has been reported with this 
change when called on every flip which suggests that modifying these fields is 
not recommended behavior on fast updates.

[How]
Guard the validation to only happen if update type isn't FAST.

Bug: 
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitlab.freedesktop.org%2Fdrm%2Famd%2F-%2Fissues%2F1191data=02%7C01%7Croman.li%40amd.com%7Cc98c2d3c363643a4fef208d81c6dfa68%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637290606647861029sdata=%2B3rZlvLgef1c2FOYTKx37U%2Fbr%2BWOxI9GXVI2K7HjH3E%3Dreserved=0
Fixes: e1995f0909e3 ("drm/amd/display: Revalidate bandwidth before commiting DC 
updates")
Cc: Hersen Wu 
Cc: Bhawanpreet Lakha 
Cc: Rodrigo Siqueira 
Signed-off-by: Nicholas Kazlauskas 
---
 drivers/gpu/drm/amd/display/dc/core/dc.c | 10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/core/dc.c 
b/drivers/gpu/drm/amd/display/dc/core/dc.c
index 67402d75e67e..942ceb0f6383 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc.c
@@ -2607,10 +2607,12 @@ void dc_commit_updates_for_stream(struct dc *dc,
 
copy_stream_update_to_stream(dc, context, stream, stream_update);
 
-   if (!dc->res_pool->funcs->validate_bandwidth(dc, context, false)) {
-   DC_ERROR("Mode validation failed for stream update!\n");
-   dc_release_state(context);
-   return;
+   if (update_type > UPDATE_TYPE_FAST) {
+   if (!dc->res_pool->funcs->validate_bandwidth(dc, context, 
false)) {
+   DC_ERROR("Mode validation failed for stream update!\n");
+   dc_release_state(context);
+   return;
+   }
}
 
commit_planes_for_stream(
--
2.25.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfxdata=02%7C01%7Croman.li%40amd.com%7Cc98c2d3c363643a4fef208d81c6dfa68%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637290606647861029sdata=EPY6DwDOG88a944QieZj%2F8NSeBJTot2z1WLghTM2Iuw%3Dreserved=0
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

RE: [PATCH 01/11] drm/amdgpu: Add ucode support for DMCUB

[Li, Roman]

On Behalf  Of: Hersen Wu
Series is:
Reviewed-by: Hersen Wu 

-Original Message-
From: amd-gfx  On Behalf Of Nicholas 
Kazlauskas
Sent: Monday, October 28, 2019 10:08 AM
To: amd-gfx@lists.freedesktop.org
Cc: Kazlauskas, Nicholas 
Subject: [PATCH 01/11] drm/amdgpu: Add ucode support for DMCUB

The DMCUB is a secondary DMCU (Display MicroController Unit) that has its own 
separate firmware. It's required for DMCU support on Renoir.

Signed-off-by: Nicholas Kazlauskas 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c | 11 ++-  
drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.h |  9 +
 2 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c
index 833fc4b68940..9ef312428231 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c
@@ -447,6 +447,7 @@ static int amdgpu_ucode_init_single_fw(struct amdgpu_device 
*adev,
const struct common_firmware_header *header = NULL;
const struct gfx_firmware_header_v1_0 *cp_hdr = NULL;
const struct dmcu_firmware_header_v1_0 *dmcu_hdr = NULL;
+   const struct dmcub_firmware_header_v1_0 *dmcub_hdr = NULL;
 
if (NULL == ucode->fw)
return 0;
@@ -460,6 +461,7 @@ static int amdgpu_ucode_init_single_fw(struct amdgpu_device 
*adev,
header = (const struct common_firmware_header *)ucode->fw->data;
cp_hdr = (const struct gfx_firmware_header_v1_0 *)ucode->fw->data;
dmcu_hdr = (const struct dmcu_firmware_header_v1_0 *)ucode->fw->data;
+   dmcub_hdr = (const struct dmcub_firmware_header_v1_0 
+*)ucode->fw->data;
 
if (adev->firmware.load_type != AMDGPU_FW_LOAD_PSP ||
(ucode->ucode_id != AMDGPU_UCODE_ID_CP_MEC1 && @@ -470,7 +472,8 @@ 
static int amdgpu_ucode_init_single_fw(struct amdgpu_device *adev,
 ucode->ucode_id != AMDGPU_UCODE_ID_RLC_RESTORE_LIST_GPM_MEM &&
 ucode->ucode_id != AMDGPU_UCODE_ID_RLC_RESTORE_LIST_SRM_MEM &&
 ucode->ucode_id != AMDGPU_UCODE_ID_DMCU_ERAM &&
-ucode->ucode_id != AMDGPU_UCODE_ID_DMCU_INTV)) {
+ucode->ucode_id != AMDGPU_UCODE_ID_DMCU_INTV &&
+ucode->ucode_id != AMDGPU_UCODE_ID_DMCUB)) {
ucode->ucode_size = le32_to_cpu(header->ucode_size_bytes);
 
memcpy(ucode->kaddr, (void *)((uint8_t *)ucode->fw->data + @@ 
-506,6 +509,12 @@ static int amdgpu_ucode_init_single_fw(struct amdgpu_device 
*adev,
  
le32_to_cpu(header->ucode_array_offset_bytes) +
  
le32_to_cpu(dmcu_hdr->intv_offset_bytes)),
   ucode->ucode_size);
+   } else if (ucode->ucode_id == AMDGPU_UCODE_ID_DMCUB) {
+   ucode->ucode_size = le32_to_cpu(dmcub_hdr->inst_const_bytes);
+   memcpy(ucode->kaddr,
+  (void *)((uint8_t *)ucode->fw->data +
+   le32_to_cpu(header->ucode_array_offset_bytes)),
+  ucode->ucode_size);
} else if (ucode->ucode_id == AMDGPU_UCODE_ID_RLC_RESTORE_LIST_CNTL) {
ucode->ucode_size = 
adev->gfx.rlc.save_restore_list_cntl_size_bytes;
memcpy(ucode->kaddr, adev->gfx.rlc.save_restore_list_cntl,
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.h
index 410587b950f3..eaf2d5b9c92f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.h
@@ -251,6 +251,13 @@ struct dmcu_firmware_header_v1_0 {
uint32_t intv_size_bytes;  /* size of interrupt vectors, in bytes */  };
 
+/* version_major=1, version_minor=0 */
+struct dmcub_firmware_header_v1_0 {
+   struct common_firmware_header header;
+   uint32_t inst_const_bytes; /* size of instruction region, in bytes */
+   uint32_t bss_data_bytes; /* size of bss/data region, in bytes */ };
+
 /* header is fixed size */
 union amdgpu_firmware_header {
struct common_firmware_header common;
@@ -268,6 +275,7 @@ union amdgpu_firmware_header {
struct sdma_firmware_header_v1_1 sdma_v1_1;
struct gpu_info_firmware_header_v1_0 gpu_info;
struct dmcu_firmware_header_v1_0 dmcu;
+   struct dmcub_firmware_header_v1_0 dmcub;
uint8_t raw[0x100];
 };
 
@@ -307,6 +315,7 @@ enum AMDGPU_UCODE_ID {
AMDGPU_UCODE_ID_DMCU_INTV,
AMDGPU_UCODE_ID_VCN0_RAM,
AMDGPU_UCODE_ID_VCN1_RAM,
+   AMDGPU_UCODE_ID_DMCUB,
AMDGPU_UCODE_ID_MAXIMUM,
 };
 
--
2.20.1

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

RE: [PATCH] drm/amdgpu/display: add dc feature mask for psr enablement

[Li, Roman]

> Any reason why we're skipping a flag here going from 0x2 to 0x8?

0x4 is reserved for fractional pwm  mask:
https://patchwork.freedesktop.org/patch/336923/

Thank you Nicholas for the review.

-Original Message-
From: Kazlauskas, Nicholas  
Sent: Tuesday, October 22, 2019 8:39 AM
To: Li, Roman ; amd-gfx@lists.freedesktop.org; Deucher, 
Alexander 
Cc: Wentland, Harry ; Lakha, Bhawanpreet 
; Li, Sun peng (Leo) 
Subject: Re: [PATCH] drm/amdgpu/display: add dc feature mask for psr enablement

On 2019-10-21 5:45 p.m., roman...@amd.com wrote:
> From: Roman Li 
> 
> [Why]
> Adding psr mask to dc features allows selectively disable/enable psr.
> Current psr implementation may not work with non-pageflipping application.
> Until resolved it should be disabled by default.
> 
> [How]
> Add dcfeaturemask for psr enablement. Disable by default.
> To enable set amdgpu.dcfeaturemask=0x8 in grub kernel command line.
> 
> Signed-off-by: Roman Li 
> ---
>   drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 3 ++-
>   drivers/gpu/drm/amd/include/amd_shared.h  | 1 +
>   2 files changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> index 1cf4beb..0f08879 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -2424,7 +2424,8 @@ static int amdgpu_dm_initialize_drm_device(struct 
> amdgpu_device *adev)
>   } else if (dc_link_detect(link, DETECT_REASON_BOOT)) {
>   amdgpu_dm_update_connector_after_detect(aconnector);
>   register_backlight_device(dm, link);
> - amdgpu_dm_set_psr_caps(link);
> + if (amdgpu_dc_feature_mask & DC_PSR_MASK)
> + amdgpu_dm_set_psr_caps(link);
>   }
>   
>   
> diff --git a/drivers/gpu/drm/amd/include/amd_shared.h 
> b/drivers/gpu/drm/amd/include/amd_shared.h
> index 8889aac..1daa221 100644
> --- a/drivers/gpu/drm/amd/include/amd_shared.h
> +++ b/drivers/gpu/drm/amd/include/amd_shared.h
> @@ -143,6 +143,7 @@ enum PP_FEATURE_MASK {
>   enum DC_FEATURE_MASK {
>   DC_FBC_MASK = 0x1,
>   DC_MULTI_MON_PP_MCLK_SWITCH_MASK = 0x2,
> + DC_PSR_MASK = 0x8,

Can this just be 0x4 instead? Any reason why we're skipping a flag here going 
from 0x2 to 0x8?

You can still have my:

Reviewed-by: Nicholas Kazlauskas 

but my preference would be on fixing this up to a 0x4 first in the commit 
message / DC_FEATURE_MASK.

Nicholas Kazlauskas

>   };
>   
>   enum amd_dpm_forced_level;
> 

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

RE: [PATCH 01/29] drm/amd/display: update register field access mechanism

[Li, Roman]

Series is
Reviewed-by: Roman Li 

-Original Message-
From: amd-gfx  On Behalf Of Bhawanpreet 
Lakha
Sent: Friday, October 11, 2019 3:53 PM
To: amd-gfx@lists.freedesktop.org
Cc: Berthe, Abdoulaye 
Subject: [PATCH 01/29] drm/amd/display: update register field access mechanism

From: abdoulaye berthe 

1-add timeout length and multiplier fields to aux_control1 register 2-update 
access mechanism from macro constructed name to uint32_t defined addresses.
3-define registers and field per asic family

Signed-off-by: abdoulaye berthe 
Acked-by: Bhawanpreet Lakha 
---
 drivers/gpu/drm/amd/display/dc/dce/dce_aux.c  |  11 +-  
drivers/gpu/drm/amd/display/dc/dce/dce_aux.h  | 175 +-
 .../amd/display/dc/dce100/dce100_resource.c   |  12 +-
 .../amd/display/dc/dce110/dce110_resource.c   |  12 +-
 .../amd/display/dc/dce112/dce112_resource.c   |  12 +-
 .../amd/display/dc/dce120/dce120_resource.c   |  12 +-
 .../drm/amd/display/dc/dce80/dce80_resource.c |  12 +-  
.../drm/amd/display/dc/dcn10/dcn10_resource.c |  12 +-  
.../drm/amd/display/dc/dcn20/dcn20_resource.c |  13 +-  
.../drm/amd/display/dc/dcn21/dcn21_resource.c |  12 +-
 10 files changed, 271 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/dce/dce_aux.c 
b/drivers/gpu/drm/amd/display/dc/dce/dce_aux.c
index 48a0e4ae80c2..de233270e3d5 100644
--- a/drivers/gpu/drm/amd/display/dc/dce/dce_aux.c
+++ b/drivers/gpu/drm/amd/display/dc/dce/dce_aux.c
@@ -42,6 +42,10 @@
 
 #include "reg_helper.h"
 
+#undef FN
+#define FN(reg_name, field_name) \
+   aux110->shift->field_name, aux110->mask->field_name
+
 #define FROM_AUX_ENGINE(ptr) \
container_of((ptr), struct aux_engine_dce110, base)
 
@@ -414,11 +418,14 @@ void dce110_engine_destroy(struct dce_aux **engine)
*engine = NULL;
 
 }
+
 struct dce_aux *dce110_aux_engine_construct(struct aux_engine_dce110 
*aux_engine110,
struct dc_context *ctx,
uint32_t inst,
uint32_t timeout_period,
-   const struct dce110_aux_registers *regs)
+   const struct dce110_aux_registers *regs,
+   const struct dce110_aux_registers_mask *mask,
+   const struct dce110_aux_registers_shift *shift)
 {
aux_engine110->base.ddc = NULL;
aux_engine110->base.ctx = ctx;
@@ -428,6 +435,8 @@ struct dce_aux *dce110_aux_engine_construct(struct 
aux_engine_dce110 *aux_engine
aux_engine110->timeout_period = timeout_period;
aux_engine110->regs = regs;
 
+   aux_engine110->mask = mask;
+   aux_engine110->shift = shift;
return _engine110->base;
 }
 
diff --git a/drivers/gpu/drm/amd/display/dc/dce/dce_aux.h 
b/drivers/gpu/drm/amd/display/dc/dce/dce_aux.h
index ed7fec8fe253..717378502e9d 100644
--- a/drivers/gpu/drm/amd/display/dc/dce/dce_aux.h
+++ b/drivers/gpu/drm/amd/display/dc/dce/dce_aux.h
@@ -29,6 +29,7 @@
 #include "i2caux_interface.h"
 #include "inc/hw/aux_engine.h"
 
+
 #ifdef CONFIG_DRM_AMD_DC_DCN2_0
 #define AUX_COMMON_REG_LIST0(id)\
SRI(AUX_CONTROL, DP_AUX, id), \
@@ -36,6 +37,7 @@
SRI(AUX_SW_DATA, DP_AUX, id), \
SRI(AUX_SW_CONTROL, DP_AUX, id), \
SRI(AUX_INTERRUPT_CONTROL, DP_AUX, id), \
+   SRI(AUX_DPHY_RX_CONTROL1, DP_AUX, id), \
SRI(AUX_SW_STATUS, DP_AUX, id)
 #endif
 
@@ -55,6 +57,7 @@ struct dce110_aux_registers {
uint32_t AUX_SW_DATA;
uint32_t AUX_SW_CONTROL;
uint32_t AUX_INTERRUPT_CONTROL;
+   uint32_t AUX_DPHY_RX_CONTROL1;
uint32_t AUX_SW_STATUS;
uint32_t AUXN_IMPCAL;
uint32_t AUXP_IMPCAL;
@@ -62,6 +65,156 @@ struct dce110_aux_registers {
uint32_t AUX_RESET_MASK;
 };
 
+#define DCE_AUX_REG_FIELD_LIST(type)\
+   type AUX_EN;\
+   type AUX_RESET;\
+   type AUX_RESET_DONE;\
+   type AUX_REG_RW_CNTL_STATUS;\
+   type AUX_SW_USE_AUX_REG_REQ;\
+   type AUX_SW_DONE_USING_AUX_REG;\
+   type AUX_SW_AUTOINCREMENT_DISABLE;\
+   type AUX_SW_DATA_RW;\
+   type AUX_SW_INDEX;\
+   type AUX_SW_GO;\
+   type AUX_SW_DATA;\
+   type AUX_SW_REPLY_BYTE_COUNT;\
+   type AUX_SW_DONE;\
+   type AUX_SW_DONE_ACK;\
+   type AUXN_IMPCAL_ENABLE;\
+   type AUXP_IMPCAL_ENABLE;\
+   type AUXN_IMPCAL_OVERRIDE_ENABLE;\
+   type AUXP_IMPCAL_OVERRIDE_ENABLE;\
+   type AUX_RX_TIMEOUT_LEN;\
+   type AUX_RX_TIMEOUT_LEN_MUL;\
+   type AUXN_CALOUT_ERROR_AK;\
+   type AUXP_CALOUT_ERROR_AK;\
+   type AUX_SW_START_DELAY;\
+   type AUX_SW_WR_BYTES
+
+#define DCE10_AUX_MASK_SH_LIST(mask_sh)\
+   AUX_SF(AUX_CONTROL, AUX_EN, mask_sh),\
+   AUX_SF(AUX_ARB_CONTROL, AUX_REG_RW_CNTL_STATUS, mask_sh),\
+   AUX_SF(AUX_ARB_CONTROL, AUX_SW_USE_AUX_REG_REQ, mask_sh),\
+   AUX_SF(AUX_ARB_CONTROL, AUX_SW_DONE_USING_AUX_REG, mask_sh),\
+   AUX_SF(AUX_SW_CONTROL, AUX_SW_START_DELAY, mask_sh),\
+   AUX_SF(AUX_SW_CONTROL, AUX_SW_WR_BYTES, mask_sh),\
+   AUX_SF(AUX_SW_CONTROL, 

RE: [PATCH 2/2] drm/amd/display: update renoir_ip_offset.h

[Li, Roman]

Acked-by: Roman Li 

-Original Message-
From: Liu, Aaron  
Sent: Wednesday, September 4, 2019 2:07 AM
To: amd-gfx@lists.freedesktop.org
Cc: Deucher, Alexander ; Huang, Ray 
; Li, Roman ; Liu, Aaron 

Subject: [PATCH 2/2] drm/amd/display: update renoir_ip_offset.h

This patch updates MP1_BASE in renoir_ip_offset.h

Signed-off-by: Aaron Liu 
---
 drivers/gpu/drm/amd/include/renoir_ip_offset.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/include/renoir_ip_offset.h 
b/drivers/gpu/drm/amd/include/renoir_ip_offset.h
index 554714c..094648c 100644
--- a/drivers/gpu/drm/amd/include/renoir_ip_offset.h
+++ b/drivers/gpu/drm/amd/include/renoir_ip_offset.h
@@ -155,7 +155,7 @@ static const struct IP_BASE MP0_BASE ={ { { { 0x00016000, 
0x0243FC00, 0x00DC
 { { 0, 0, 0, 0, 0 } },
 { { 0, 0, 0, 0, 0 } },
 { { 0, 0, 0, 0, 0 } } } }; -static 
const struct IP_BASE MP1_BASE ={ { { { 0x00016200, 0x02400400, 0x00E8, 
0x00EC, 0x00F0 } },
+static const struct IP_BASE MP1_BASE ={ { { { 0x00016000, 0x02400400, 
+0x00E8, 0x00EC, 0x00F0 } },
 { { 0, 0, 0, 0, 0 } },
 { { 0, 0, 0, 0, 0 } },
 { { 0, 0, 0, 0, 0 } },
--
2.7.4

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

RE: [PATCH] drm/amd/display: Use context parameters to enable FBC

[Li, Roman]

Reviewed-by: Roman Li 

-Original Message-
From: amd-gfx  On Behalf Of S, Shirish
Sent: Monday, February 4, 2019 4:20 AM
To: Li, Sun peng (Leo) ; Wu, Hersen ; 
Wentland, Harry 
Cc: amd-gfx@lists.freedesktop.org; S, Shirish 
Subject: [PATCH] drm/amd/display: Use context parameters to enable FBC

[What]
FBC fails to get enabled when switched between LINEAR(console/VT) and 
non-LINEAR(GUI) based rendering due to default value of tiling info stored in 
the current_state which is used for deciding whether or not to turn FBC on or 
off.

[How]
Use context structure's tiling information which is coherant with the screen 
updates.

Signed-off-by: Shirish S 
---
 drivers/gpu/drm/amd/display/dc/dce110/dce110_hw_sequencer.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/display/dc/dce110/dce110_hw_sequencer.c 
b/drivers/gpu/drm/amd/display/dc/dce110/dce110_hw_sequencer.c
index db0ef41..fd7cd5b 100644
--- a/drivers/gpu/drm/amd/display/dc/dce110/dce110_hw_sequencer.c
+++ b/drivers/gpu/drm/amd/display/dc/dce110/dce110_hw_sequencer.c
@@ -2535,7 +2535,7 @@ static void dce110_apply_ctx_for_surface(
}
 
if (dc->fbc_compressor)
-   enable_fbc(dc, dc->current_state);
+   enable_fbc(dc, context);
 }
 
 static void dce110_power_down_fe(struct dc *dc, struct pipe_ctx *pipe_ctx)
--
2.7.4

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

RE: [PATCH 09/16] drm/amd/display: fbc state could not reach while enable fbc

[Li, Roman]

Unfortunately, not. I sent this patch to the reporter to try and it  didn't 
work.

- Roman

From: amd-gfx  On Behalf Of Deucher, 
Alexander
Sent: Thursday, November 29, 2018 11:27 AM
To: Li, Sun peng (Leo) ; amd-gfx@lists.freedesktop.org
Cc: Wu, Hersen 
Subject: Re: [PATCH 09/16] drm/amd/display: fbc state could not reach while 
enable fbc


Do you think this will fix this bug?

https://bugs.freedesktop.org/show_bug.cgi?id=108577

If so, we can re-enable fbc.



Alex


From: amd-gfx 
mailto:amd-gfx-boun...@lists.freedesktop.org>>
 on behalf of sunpeng...@amd.com 
mailto:sunpeng...@amd.com>>
Sent: Thursday, November 29, 2018 10:52:16 AM
To: amd-gfx@lists.freedesktop.org
Cc: Wu, Hersen
Subject: [PATCH 09/16] drm/amd/display: fbc state could not reach while enable 
fbc

From: hersen wu mailto:hersenxs...@amd.com>>

   [WHY] fbc is within the data path from memory to dce. while
   re-configure mc dmif, fbc should be enabled. otherwise, fbc
   may not be enabled properly.

   [HOW] before re-configure mc dmif, disable fbc, only after
   dmif re-configuration fully done, enable fbc again.

Signed-off-by: hersen wu mailto:hersenxs...@amd.com>>
Reviewed-by: Roman Li mailto:roman...@amd.com>>
Acked-by: Leo Li mailto:sunpeng...@amd.com>>
---
 .../drm/amd/display/dc/dce110/dce110_compressor.c  | 91 --
 .../amd/display/dc/dce110/dce110_hw_sequencer.c| 57 --
 drivers/gpu/drm/amd/display/dc/inc/compressor.h|  1 +
 3 files changed, 66 insertions(+), 83 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/dce110/dce110_compressor.c 
b/drivers/gpu/drm/amd/display/dc/dce110/dce110_compressor.c
index 1f7f250..52d50e2 100644
--- a/drivers/gpu/drm/amd/display/dc/dce110/dce110_compressor.c
+++ b/drivers/gpu/drm/amd/display/dc/dce110/dce110_compressor.c
@@ -64,65 +64,37 @@ static const struct dce110_compressor_reg_offsets 
reg_offsets[] = {

 static const uint32_t dce11_one_lpt_channel_max_resolution = 2560 * 1600;

-enum fbc_idle_force {
-   /* Bit 0 - Display registers updated */
-   FBC_IDLE_FORCE_DISPLAY_REGISTER_UPDATE = 0x0001,
-
-   /* Bit 2 - FBC_GRPH_COMP_EN register updated */
-   FBC_IDLE_FORCE_GRPH_COMP_EN = 0x0002,
-   /* Bit 3 - FBC_SRC_SEL register updated */
-   FBC_IDLE_FORCE_SRC_SEL_CHANGE = 0x0004,
-   /* Bit 4 - FBC_MIN_COMPRESSION register updated */
-   FBC_IDLE_FORCE_MIN_COMPRESSION_CHANGE = 0x0008,
-   /* Bit 5 - FBC_ALPHA_COMP_EN register updated */
-   FBC_IDLE_FORCE_ALPHA_COMP_EN = 0x0010,
-   /* Bit 6 - FBC_ZERO_ALPHA_CHUNK_SKIP_EN register updated */
-   FBC_IDLE_FORCE_ZERO_ALPHA_CHUNK_SKIP_EN = 0x0020,
-   /* Bit 7 - FBC_FORCE_COPY_TO_COMP_BUF register updated */
-   FBC_IDLE_FORCE_FORCE_COPY_TO_COMP_BUF = 0x0040,
-
-   /* Bit 24 - Memory write to region 0 defined by MC registers. */
-   FBC_IDLE_FORCE_MEMORY_WRITE_TO_REGION0 = 0x0100,
-   /* Bit 25 - Memory write to region 1 defined by MC registers */
-   FBC_IDLE_FORCE_MEMORY_WRITE_TO_REGION1 = 0x0200,
-   /* Bit 26 - Memory write to region 2 defined by MC registers */
-   FBC_IDLE_FORCE_MEMORY_WRITE_TO_REGION2 = 0x0400,
-   /* Bit 27 - Memory write to region 3 defined by MC registers. */
-   FBC_IDLE_FORCE_MEMORY_WRITE_TO_REGION3 = 0x0800,
-
-   /* Bit 28 - Memory write from any client other than MCIF */
-   FBC_IDLE_FORCE_MEMORY_WRITE_OTHER_THAN_MCIF = 0x1000,
-   /* Bit 29 - CG statics screen signal is inactive */
-   FBC_IDLE_FORCE_CG_STATIC_SCREEN_IS_INACTIVE = 0x2000,
-};
-
-
 static uint32_t align_to_chunks_number_per_line(uint32_t pixels)
 {
 return 256 * ((pixels + 255) / 256);
 }

-static void reset_lb_on_vblank(struct dc_context *ctx)
+static void reset_lb_on_vblank(struct compressor *compressor, uint32_t 
crtc_inst)
 {
-   uint32_t value, frame_count;
+   uint32_t value;
+   uint32_t frame_count;
+   uint32_t status_pos;
 uint32_t retry = 0;
-   uint32_t status_pos =
-   dm_read_reg(ctx, mmCRTC_STATUS_POSITION);
+   struct dce110_compressor *cp110 = TO_DCE110_COMPRESSOR(compressor);
+
+   cp110->offsets = reg_offsets[crtc_inst];
+
+   status_pos = dm_read_reg(compressor->ctx, 
DCP_REG(mmCRTC_STATUS_POSITION));


 /* Only if CRTC is enabled and counter is moving we wait for one 
frame. */
-   if (status_pos != dm_read_reg(ctx, mmCRTC_STATUS_POSITION)) {
+   if (status_pos != dm_read_reg(compressor->ctx, 
DCP_REG(mmCRTC_STATUS_POSITION))) {
 /* Resetting LB on VBlank */
-   value = dm_read_reg(ctx, mmLB_SYNC_RESET_SEL);
+   value = dm_read_reg(compressor->ctx, 
DCP_REG(mmLB_SYNC_RESET_SEL));
 set_reg_field_value(value, 3, LB_SYNC_RESET_SEL, 
LB_SYNC_RESET_SEL);
 set_reg_field_value(value, 

RE: linux-4.16-rc5/drivers/gpu/drm/amd/display/dc/dce110/dce110_hw_sequencer.c:1723]: (warning) Possible null pointer dereference: pipe_ctx

[Li, Roman]

There is a check just before for-loop that should ensure pipe_ctx is not null:

/* Only supports single display */
if (context->stream_count != 1)
return false;

To remove the subject warning - we can rather add  an assert:
assert(pipe_ctx);

Thanks,
Roman

-Original Message-
From: Grodzovsky, Andrey 
Sent: Monday, March 12, 2018 6:23 PM
To: David Binderman <dcb...@hotmail.com>; Deucher, Alexander 
<alexander.deuc...@amd.com>; Koenig, Christian <christian.koe...@amd.com>; 
Zhou, David(ChunMing) <david1.z...@amd.com>; airl...@linux.ie; 
amd-gfx@lists.freedesktop.org; dri-de...@lists.freedesktop.org; 
linux-ker...@vger.kernel.org; Wentland, Harry <harry.wentl...@amd.com>; Li, 
Roman <roman...@amd.com>
Subject: Re: 
linux-4.16-rc5/drivers/gpu/drm/amd/display/dc/dce110/dce110_hw_sequencer.c:1723]:
 (warning) Possible null pointer dereference: pipe_ctx



On 03/12/2018 06:22 AM, David Binderman wrote:
> hello there,
>
> Source code is
>
>
>      for (i = 0; i < dc->res_pool->pipe_count; i++) {
>      if (res_ctx->pipe_ctx[i].stream) {
>      pipe_ctx = _ctx->pipe_ctx[i];
>      *pipe_idx = i;
>      break;
>      }
>      }

Indeed something like

if (!pipe_ctx)
   return false;

seems the right thing here.

Harry, Roman , your opinion ?

Andrey

>
>      /* Only supports eDP */
>      if (pipe_ctx->stream->sink->link->connector_signal != 
> SIGNAL_TYPE_EDP)
>      return false;
>
> Suggest add some code to deal with the case that the for loop doesn't 
> find what it is looking for and so pipe_ctx is NULL.
>
> Regards
>
> David Binderman
>  
> ___
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx

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