Re: [PATCH] Revert "drm/amd/display: avoid large on-stack structures"
[AMD Official Use Only - AMD Internal Distribution Only]
Thanks for the heads up!
--
Regards,
Jay
From: Mahfooz, Hamza
Sent: Tuesday, June 4, 2024 1:50 PM
To: Pillai, Aurabindo ; amd-gfx@lists.freedesktop.org
Cc: a...@arndb.de ; Deucher, Alexander
; Wentland, Harry ;
Siqueira, Rodrigo ; Zuo, Jerry
Subject: Re: [PATCH] Revert "drm/amd/display: avoid large on-stack structures"
On 6/4/24 13:45, Aurabindo Pillai wrote:
> This reverts commit 44069f0f9b1fe577c5d4f05fa9eb02db8c618adc since
> the code path is called from FPU context, and triggers error like:
>
> [ 26.924055] BUG: sleeping function called from invalid context at
> include/linux/sched/mm.h:306
> [ 26.924060] in_atomic(): 1, irqs_disabled(): 0, non_block: 0, pid: 1022,
> name: modprobe
> [ 26.924063] preempt_count: 2, expected: 0
> [ 26.924064] RCU nest depth: 0, expected: 0
> [ 26.924066] Preemption disabled at:
> [ 26.924067] [] dc_fpu_begin+0x30/0xd0 [amdgpu]
> [ 26.924322] CPU: 9 PID: 1022 Comm: modprobe Not tainted 6.8.0+ #20
> [ 26.924325] Hardware name: System manufacturer System Product
> Name/CROSSHAIR VI HERO, BIOS 6302 10/23/2018
> [ 26.924326] Call Trace:
> [ 26.924327]
> [ 26.924329] dump_stack_lvl+0x37/0x50
> [ 26.924333] ? dc_fpu_begin+0x30/0xd0 [amdgpu]
> [ 26.924589] dump_stack+0x10/0x20
> [ 26.924592] __might_resched+0x16a/0x1c0
> [ 26.924596] __might_sleep+0x42/0x70
> [ 26.924598] __kmalloc_node_track_caller+0x2ad/0x4b0
> [ 26.924601] ? dm_helpers_allocate_gpu_mem+0x12/0x20 [amdgpu]
> [ 26.924855] ? dcn401_update_bw_bounding_box+0x2a/0xf0 [amdgpu]
> [ 26.925122] kmemdup+0x20/0x50
> [ 26.925124] ? kernel_fpu_begin_mask+0x6b/0xe0
> [ 26.925127] ? kmemdup+0x20/0x50
> [ 26.925129] dcn401_update_bw_bounding_box+0x2a/0xf0 [amdgpu]
> [ 26.925393] dc_create+0x311/0x670 [amdgpu]
> [ 26.925649] amdgpu_dm_init+0x2aa/0x1fa0 [amdgpu]
> [ 26.925903] ? irq_work_queue+0x38/0x50
> [ 26.925907] ? vprintk_emit+0x1e7/0x270
> [ 26.925910] ? dev_printk_emit+0x83/0xb0
> [ 26.925914] ? amdgpu_device_rreg+0x17/0x20 [amdgpu]
> [ 26.926133] dm_hw_init+0x14/0x30 [amdgpu]
> ---
> drivers/gpu/drm/amd/display/dc/core/dc_state.c | 16 +---
> .../display/dc/resource/dcn401/dcn401_resource.c | 16 +---
> 2 files changed, 10 insertions(+), 22 deletions(-)
You probably want something like
https://patchwork.freedesktop.org/patch/597044/ instead.
>
> diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_state.c
> b/drivers/gpu/drm/amd/display/dc/core/dc_state.c
> index 8ea9391c60b7..70928223b642 100644
> --- a/drivers/gpu/drm/amd/display/dc/core/dc_state.c
> +++ b/drivers/gpu/drm/amd/display/dc/core/dc_state.c
> @@ -193,11 +193,7 @@ static void init_state(struct dc *dc, struct dc_state
> *state)
> struct dc_state *dc_state_create(struct dc *dc, struct
> dc_state_create_params *params)
> {
> #ifdef CONFIG_DRM_AMD_DC_FP
> - struct dml2_configuration_options *dml2_opt;
> -
> - dml2_opt = kmemdup(>dml2_options, sizeof(*dml2_opt), GFP_KERNEL);
> - if (!dml2_opt)
> - return NULL;
> + struct dml2_configuration_options dml2_opt = dc->dml2_options;
> #endif
>struct dc_state *state = kvzalloc(sizeof(struct dc_state),
>GFP_KERNEL);
> @@ -211,14 +207,12 @@ struct dc_state *dc_state_create(struct dc *dc, struct
> dc_state_create_params *p
>
> #ifdef CONFIG_DRM_AMD_DC_FP
>if (dc->debug.using_dml2) {
> - dml2_opt->use_clock_dc_limits = false;
> - dml2_create(dc, dml2_opt, >bw_ctx.dml2);
> + dml2_opt.use_clock_dc_limits = false;
> + dml2_create(dc, _opt, >bw_ctx.dml2);
>
> - dml2_opt->use_clock_dc_limits = true;
> - dml2_create(dc, dml2_opt, >bw_ctx.dml2_dc_power_source);
> + dml2_opt.use_clock_dc_limits = true;
> + dml2_create(dc, _opt, >bw_ctx.dml2_dc_power_source);
>}
> -
> - kfree(dml2_opt);
> #endif
>
>kref_init(>refcount);
> diff --git a/drivers/gpu/drm/amd/display/dc/resource/dcn401/dcn401_resource.c
> b/drivers/gpu/drm/amd/display/dc/resource/dcn401/dcn401_resource.c
> index 8dfb0a3d21cb..247bac177d1b 100644
> --- a/drivers/gpu/drm/amd/display/dc/resource/dcn401/dcn401_resource.c
> +++ b/drivers/gpu/drm/amd/display/dc/resource/dcn401/dcn401_resource.c
> @@ -1581,27 +1581,21 @@ static struct dc_cap_funcs cap_funcs = {
>
> static void dcn401_update_bw_bounding_box(struct dc *dc, struct
> clk_bw_params *bw_params)
> {
> - struct dml2_configuration_options *dml2_opt;
> -
> -
Re: [PATCH] drm/amd/display: use pre-allocated temp structure for bounding box
[AMD Official Use Only - AMD Internal Distribution Only]
Hi Alex,
I'll a hunk for fixing DCN401 as well to this and resend it later today.
--
Regards,
Jay
From: amd-gfx on behalf of Zhang,
George
Sent: Tuesday, June 4, 2024 12:49 PM
To: Deucher, Alexander ;
amd-gfx@lists.freedesktop.org
Cc: Mahfooz, Hamza ; Arnd Bergmann ;
Wentland, Harry ; Li, Sun peng (Leo)
; Siqueira, Rodrigo
Subject: RE: [PATCH] drm/amd/display: use pre-allocated temp structure for
bounding box
[AMD Official Use Only - AMD Internal Distribution Only]
[AMD Official Use Only - AMD Internal Distribution Only]
Tested-by: George Zhang
Thanks,
George
-Original Message-
From: Deucher, Alexander
Sent: Tuesday, June 4, 2024 11:50 AM
To: amd-gfx@lists.freedesktop.org
Cc: Deucher, Alexander ; Mahfooz, Hamza
; Zhang, George ; Arnd Bergmann
; Wentland, Harry ; Li, Sun peng (Leo)
; Siqueira, Rodrigo
Subject: [PATCH] drm/amd/display: use pre-allocated temp structure for bounding
box
This mirrors what the driver does for older DCN generations.
Should fix:
BUG: sleeping function called from invalid context at
include/linux/sched/mm.h:306
in_atomic(): 1, irqs_disabled(): 0, non_block: 0, pid: 449, name: kworker/u64:8
preempt_count: 2, expected: 0
RCU nest depth: 0, expected: 0
Preemption disabled at:
c0ce1580>] dc_fpu_begin+0x30/0xd0 [amdgpu]
CPU: 5 PID: 449 Comm: kworker/u64:8 Tainted: GW 6.8.0+ #35
Hardware name: System manufacturer System Product Name/ROG STRIX X570-E GAMING
WIFI II, BIOS 4204 02/24/2022
Workqueue: events_unbound async_run_entry_fn
Fixes: 88c61827cedc ("drm/amd/display: dynamically allocate
dml2_configuration_options structures")
Suggested-by: Hamza Mahfooz
Signed-off-by: Alex Deucher
Cc: George Zhang
Cc: Arnd Bergmann
Cc: harry.wentl...@amd.com
Cc: sunpeng...@amd.com
Cc: rodrigo.sique...@amd.com
---
drivers/gpu/drm/amd/display/dc/dc.h | 1 +
.../drm/amd/display/dc/resource/dcn32/dcn32_resource.c| 8 +++-
.../drm/amd/display/dc/resource/dcn321/dcn321_resource.c | 8 +++-
3 files changed, 7 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/dc/dc.h
b/drivers/gpu/drm/amd/display/dc/dc.h
index d0ed01ac460d..d843933ad731 100644
--- a/drivers/gpu/drm/amd/display/dc/dc.h
+++ b/drivers/gpu/drm/amd/display/dc/dc.h
@@ -1444,6 +1444,7 @@ struct dc {
} scratch;
struct dml2_configuration_options dml2_options;
+ struct dml2_configuration_options dml2_tmp;
enum dc_acpi_cm_power_state power_state;
};
diff --git a/drivers/gpu/drm/amd/display/dc/resource/dcn32/dcn32_resource.c
b/drivers/gpu/drm/amd/display/dc/resource/dcn32/dcn32_resource.c
index 0f11d7c8791c..7a8aa9396dea 100644
--- a/drivers/gpu/drm/amd/display/dc/resource/dcn32/dcn32_resource.c
+++ b/drivers/gpu/drm/amd/display/dc/resource/dcn32/dcn32_resource.c
@@ -2007,11 +2007,9 @@ void dcn32_calculate_wm_and_dlg(struct dc *dc, struct
dc_state *context,
static void dcn32_update_bw_bounding_box(struct dc *dc, struct clk_bw_params
*bw_params)
{
- struct dml2_configuration_options *dml2_opt;
+ struct dml2_configuration_options *dml2_opt = >dml2_tmp;
- dml2_opt = kmemdup(>dml2_options, sizeof(dc->dml2_options),
GFP_KERNEL);
- if (!dml2_opt)
- return;
+ memcpy(dml2_opt, >dml2_options, sizeof(dc->dml2_options));
DC_FP_START();
@@ -2027,7 +2025,7 @@ static void dcn32_update_bw_bounding_box(struct dc *dc,
struct clk_bw_params *bw
DC_FP_END();
- kfree(dml2_opt);
+ memcpy(>dml2_options, dml2_opt, sizeof(dc->dml2_options));
}
static struct resource_funcs dcn32_res_pool_funcs = {
diff --git a/drivers/gpu/drm/amd/display/dc/resource/dcn321/dcn321_resource.c
b/drivers/gpu/drm/amd/display/dc/resource/dcn321/dcn321_resource.c
index 07ca6f58447d..ef30e8632607 100644
--- a/drivers/gpu/drm/amd/display/dc/resource/dcn321/dcn321_resource.c
+++ b/drivers/gpu/drm/amd/display/dc/resource/dcn321/dcn321_resource.c
@@ -1581,11 +1581,9 @@ static struct dc_cap_funcs cap_funcs = {
static void dcn321_update_bw_bounding_box(struct dc *dc, struct clk_bw_params
*bw_params)
{
- struct dml2_configuration_options *dml2_opt;
+ struct dml2_configuration_options *dml2_opt = >dml2_tmp;
- dml2_opt = kmemdup(>dml2_options, sizeof(dc->dml2_options),
GFP_KERNEL);
- if (!dml2_opt)
- return;
+ memcpy(dml2_opt, >dml2_options, sizeof(dc->dml2_options));
DC_FP_START();
@@ -1601,7 +1599,7 @@ static void dcn321_update_bw_bounding_box(struct dc *dc,
struct clk_bw_params *b
DC_FP_END();
- kfree(dml2_opt);
+ memcpy(>dml2_options, dml2_opt, sizeof(dc->dml2_options));
}
static struct resource_funcs dcn321_res_pool_funcs = {
--
2.45.1
Re: [PATCH v2] drm/amd/display: Remove redundant NULL check in dcn10_set_input_transfer_func
[AMD Official Use Only - General]
Reviewed-by: Aurabindo Pillai
--
Regards,
Jay
From: SHANMUGAM, SRINIVASAN
Sent: Tuesday, April 23, 2024 9:34 PM
To: Siqueira, Rodrigo ; Pillai, Aurabindo
Cc: amd-gfx@lists.freedesktop.org ; SHANMUGAM,
SRINIVASAN ; Liu, Wenjing ;
Chung, ChiaHsuan (Tom) ; Lee, Alvin
; Li, Roman ; Wu, Hersen
; Hung, Alex ; Wentland, Harry
; Dan Carpenter
Subject: [PATCH v2] drm/amd/display: Remove redundant NULL check in
dcn10_set_input_transfer_func
This commit removes an unnecessary NULL check in the
`dcn10_set_input_transfer_func` function in the `dcn10_hwseq.c` file.
The variable `tf` is assigned the address of
`plane_state->in_transfer_func` unconditionally, so it can never be
`NULL`. Therefore, the check `if (tf == NULL)` is unnecessary and has
been removed.
Fixes the below smatch warning:
drivers/gpu/drm/amd/amdgpu/../display/dc/hwss/dcn10/dcn10_hwseq.c:1839
dcn10_set_input_transfer_func() warn: address of
'plane_state->in_transfer_func' is non-NULL
Fixes: 285a7054bf81 ("drm/amd/display: Remove plane and stream pointers from dc
scratch")
Cc: Wenjing Liu
Cc: Tom Chung
Cc: Alvin Lee
Cc: Rodrigo Siqueira
Cc: Roman Li
Cc: Hersen Wu
Cc: Alex Hung
Cc: Aurabindo Pillai
Cc: Harry Wentland
Suggested-by: Dan Carpenter
Signed-off-by: Srinivasan Shanmugam
---
v2:
- s/dcn20/dcn10 in commit title
drivers/gpu/drm/amd/display/dc/hwss/dcn10/dcn10_hwseq.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/dc/hwss/dcn10/dcn10_hwseq.c
b/drivers/gpu/drm/amd/display/dc/hwss/dcn10/dcn10_hwseq.c
index 32a07ab74c51..f258914a4838 100644
--- a/drivers/gpu/drm/amd/display/dc/hwss/dcn10/dcn10_hwseq.c
+++ b/drivers/gpu/drm/amd/display/dc/hwss/dcn10/dcn10_hwseq.c
@@ -1837,9 +1837,7 @@ bool dcn10_set_input_transfer_func(struct dc *dc, struct
pipe_ctx *pipe_ctx,
&& dce_use_lut(plane_state->format))
dpp_base->funcs->dpp_program_input_lut(dpp_base,
_state->gamma_correction);
- if (tf == NULL)
- dpp_base->funcs->dpp_set_degamma(dpp_base,
IPP_DEGAMMA_MODE_BYPASS);
- else if (tf->type == TF_TYPE_PREDEFINED) {
+ if (tf->type == TF_TYPE_PREDEFINED) {
switch (tf->tf) {
case TRANSFER_FUNCTION_SRGB:
dpp_base->funcs->dpp_set_degamma(dpp_base,
IPP_DEGAMMA_MODE_HW_sRGB);
--
2.34.1
Re: [PATCH] drm/amd/display: Override DCN410 IP version
[AMD Official Use Only - General] Thanks for the review! Just sent a v2 From: Wentland, Harry Sent: Tuesday, April 30, 2024 9:36 AM To: Pillai, Aurabindo ; amd-gfx@lists.freedesktop.org Cc: Siqueira, Rodrigo ; Deucher, Alexander Subject: Re: [PATCH] drm/amd/display: Override DCN410 IP version On 2024-04-30 09:23, Aurabindo Pillai wrote: > Override DCN IP version to 4.0.1 from 4.1.0 temporarily until change is > made in DC codebase to use 4.1.0 > > Signed-off-by: Aurabindo Pillai > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c | 5 + > 1 file changed, 5 insertions(+) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c > index 2aad1ba0ab9d..87a2f15c8a64 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c > @@ -1965,6 +1965,11 @@ static int > amdgpu_discovery_set_display_ip_blocks(struct amdgpu_device *adev) >case IP_VERSION(3, 2, 1): >case IP_VERSION(3, 5, 0): >case IP_VERSION(3, 5, 1): > + case IP_VERSION(4, 1, 0): Doesn't this do more than override? Doesn't this enable IP creation for DCN 4.1.0 in the first place? It might make sense to split this into a separate patch as it sounds like we'll want to revert the override at some point in the future but not the IP creation. Harry > + /* TODO: Fix IP version. DC code expects version 4.0.1 > */ > + if (adev->ip_versions[DCE_HWIP][0] == IP_VERSION(4, 1, > 0)) > + adev->ip_versions[DCE_HWIP][0] = > IP_VERSION(4, 0, 1); > + >if (amdgpu_sriov_vf(adev)) >amdgpu_discovery_set_sriov_display(adev); >else
Re: [PATCH 2/2] drm/amd/display: Fix CFLAGS for dml2_core_dcn4_calcs.o
[AMD Official Use Only - General]
Thanks for the fix.
Reviewed-by: Aurabindo Pillai
--
Regards,
Jay
From: Nathan Chancellor
Sent: Wednesday, April 24, 2024 2:19 PM
To: Wentland, Harry ; Li, Sun peng (Leo)
; Siqueira, Rodrigo ; Deucher,
Alexander ; Koenig, Christian
; Pan, Xinhui
Cc: Pillai, Aurabindo ; amd-gfx@lists.freedesktop.org
; dri-de...@lists.freedesktop.org
; l...@lists.linux.dev ;
patc...@lists.linux.dev ; Nathan Chancellor
Subject: [PATCH 2/2] drm/amd/display: Fix CFLAGS for dml2_core_dcn4_calcs.o
-Wframe-larger-than=2048 is a part of both CFLAGS and CFLAGS_REMOVE for
dml2_core_dcn4_calcs.o, which means that it ultimately gets removed
altogether for 64-bit targets, as 2048 is the default FRAME_WARN value
for 64-bit platforms, resulting in no -Wframe-larger-than coverage for
this file.
Remove -Wframe-larger-than from CFLAGS_REMOVE_dml2_core_dcn4_calcs.o and
move to $(frame_warn_flag) for CFLAGS_dml2_core_dcn4_calcs.o, as that
accounts for the fact that -Wframe-larger-than may need to be larger
than 2048 in certain situations, such as when the sanitizers are
enabled.
Fixes: d546a39c6b10 ("drm/amd/display: Add misc DC changes for DCN401")
Signed-off-by: Nathan Chancellor
---
drivers/gpu/drm/amd/display/dc/dml2/Makefile | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/dc/dml2/Makefile
b/drivers/gpu/drm/amd/display/dc/dml2/Makefile
index c35212a4a968..904a2d419638 100644
--- a/drivers/gpu/drm/amd/display/dc/dml2/Makefile
+++ b/drivers/gpu/drm/amd/display/dc/dml2/Makefile
@@ -111,7 +111,7 @@ CFLAGS_$(AMDDALPATH)/dc/dml2/dml21/src/dml2_top/dml_top.o
:= $(dml2_ccflags)
CFLAGS_$(AMDDALPATH)/dc/dml2/dml21/src/dml2_top/dml_top_mcache.o :=
$(dml2_ccflags)
CFLAGS_$(AMDDALPATH)/dc/dml2/dml21/src/dml2_top/dml2_top_optimization :=
$(dml2_ccflags)
CFLAGS_$(AMDDALPATH)/dc/dml2/dml21/src/dml2_core/dml2_core_dcn4.o :=
$(dml2_ccflags)
-CFLAGS_$(AMDDALPATH)/dc/dml2/dml21/src/dml2_core/dml2_core_dcn4_calcs.o :=
$(dml2_ccflags) -Wframe-larger-than=2048
+CFLAGS_$(AMDDALPATH)/dc/dml2/dml21/src/dml2_core/dml2_core_dcn4_calcs.o :=
$(dml2_ccflags) $(frame_warn_flag)
CFLAGS_$(AMDDALPATH)/dc/dml2/dml21/src/dml2_core/dml2_core_factory.o :=
$(dml2_ccflags)
CFLAGS_$(AMDDALPATH)/dc/dml2/dml21/src/dml2_core/dml2_core_shared.o :=
$(dml2_ccflags) $(frame_warn_flag)
CFLAGS_$(AMDDALPATH)/dc/dml2/dml21/src/dml2_dpmm/dml2_dpmm_dcn4.o :=
$(dml2_ccflags)
@@ -134,7 +134,7 @@
CFLAGS_REMOVE_$(AMDDALPATH)/dc/dml2/dml21/src/dml2_top/dml_top.o := $(dml2_rcfla
CFLAGS_REMOVE_$(AMDDALPATH)/dc/dml2/dml21/src/dml2_top/dml_top_mcache.o :=
$(dml2_rcflags)
CFLAGS_REMOVE_$(AMDDALPATH)/dc/dml2/dml21/src/dml2_top/dml2_top_optimization.o
:= $(dml2_rcflags)
CFLAGS_REMOVE_$(AMDDALPATH)/dc/dml2/dml21/src/dml2_core/dml2_core_dcn4.o :=
$(dml2_rcflags)
-CFLAGS_REMOVE_$(AMDDALPATH)/dc/dml2/dml21/src/dml2_core/dml2_core_dcn4_calcs.o
:= $(dml2_rcflags) -Wframe-larger-than=2048
+CFLAGS_REMOVE_$(AMDDALPATH)/dc/dml2/dml21/src/dml2_core/dml2_core_dcn4_calcs.o
:= $(dml2_rcflags)
CFLAGS_REMOVE_$(AMDDALPATH)/dc/dml2/dml21/src/dml2_core/dml2_core_factory.o :=
$(dml2_rcflags)
CFLAGS_REMOVE_$(AMDDALPATH)/dc/dml2/dml21/src/dml2_core/dml2_core_shared.o :=
$(dml2_rcflags)
CFLAGS_REMOVE_$(AMDDALPATH)/dc/dml2/dml21/src/dml2_dpmm/dml2_dpmm_dcn4.o :=
$(dml2_rcflags)
--
2.44.0
Re: [PATCH 33/43] drm/amd/display: Prevent crash on bring-up
[AMD Official Use Only - General]
Might want to avoid bringup in the commit description
--
Regards,
Jay
From: Wayne Lin
Sent: Tuesday, March 12, 2024 5:20 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 ; Park, Chris ; Limonciello,
Mario ; Deucher, Alexander
; sta...@vger.kernel.org ;
Liu, Charlene
Subject: [PATCH 33/43] drm/amd/display: Prevent crash on bring-up
From: Chris Park
[Why]
Disabling stream encoder invokes a function that no longer exists
in bring-up.
[How]
Check if the function declaration is NULL in disable stream encoder.
Cc: Mario Limonciello
Cc: Alex Deucher
Cc: sta...@vger.kernel.org
Reviewed-by: Charlene Liu
Acked-by: Wayne Lin
Signed-off-by: Chris Park
---
drivers/gpu/drm/amd/display/dc/hwss/dce110/dce110_hwseq.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/display/dc/hwss/dce110/dce110_hwseq.c
b/drivers/gpu/drm/amd/display/dc/hwss/dce110/dce110_hwseq.c
index 9d5df4c0da59..0ba1feaf96c0 100644
--- a/drivers/gpu/drm/amd/display/dc/hwss/dce110/dce110_hwseq.c
+++ b/drivers/gpu/drm/amd/display/dc/hwss/dce110/dce110_hwseq.c
@@ -1185,7 +1185,8 @@ void dce110_disable_stream(struct pipe_ctx *pipe_ctx)
if (dccg) {
dccg->funcs->disable_symclk32_se(dccg, dp_hpo_inst);
dccg->funcs->set_dpstreamclk(dccg, REFCLK, tg->inst,
dp_hpo_inst);
- dccg->funcs->set_dtbclk_dto(dccg, _params);
+ if (dccg && dccg->funcs->set_dtbclk_dto)
+ dccg->funcs->set_dtbclk_dto(dccg, _params);
}
} else if (dccg && dccg->funcs->disable_symclk_se) {
dccg->funcs->disable_symclk_se(dccg,
stream_enc->stream_enc_inst,
--
2.37.3
Re: [PATCH v2] drm/amdgpu/display: Initialize gamma correction mode variable in dcn30_get_gamcor_current()
[AMD Official Use Only - General]
Reviewed-by: Aurabindo Pillai
--
Regards,
Jay
From: SHANMUGAM, SRINIVASAN
Sent: Monday, February 12, 2024 10:33 AM
To: Siqueira, Rodrigo ; Pillai, Aurabindo
Cc: amd-gfx@lists.freedesktop.org ; SHANMUGAM,
SRINIVASAN ; Lakha, Bhawanpreet
; Chung, ChiaHsuan (Tom) ;
Li, Roman
Subject: [PATCH v2] drm/amdgpu/display: Initialize gamma correction mode
variable in dcn30_get_gamcor_current()
The dcn30_get_gamcor_current() function is responsible for determining
the current gamma correction mode used by the display controller.
However, the 'mode' variable, which stores the gamma correction mode,
was not initialized before its first usage, leading to an uninitialized
symbol error.
Thus initializes the 'mode' variable with a default value of LUT_BYPASS
before the conditional statements in the function, improves code clarity
and stability, ensuring correct behavior of the
dcn30_get_gamcor_current() function in determining the gamma correction
mode.
Fixes the below:
drivers/gpu/drm/amd/amdgpu/../display/dc/dcn30/dcn30_dpp_cm.c:77
dpp30_get_gamcor_current() error: uninitialized symbol 'mode'.
Fixes: 03f54d7d3448 ("drm/amd/display: Add DCN3 DPP")
Cc: Bhawanpreet Lakha
Cc: Rodrigo Siqueira
Cc: Aurabindo Pillai
Cc: Tom Chung
Signed-off-by: Srinivasan Shanmugam
Suggested-by: Roman Li
---
v2:
- removed the below redundant code (Roman)
if (state_mode == 0)
mode = LUT_BYPASS;
drivers/gpu/drm/amd/display/dc/dcn30/dcn30_dpp_cm.c | 5 +
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_dpp_cm.c
b/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_dpp_cm.c
index 54ec144f7b81..2f5b3fbd3507 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_dpp_cm.c
+++ b/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_dpp_cm.c
@@ -56,16 +56,13 @@ static void dpp3_enable_cm_block(
static enum dc_lut_mode dpp30_get_gamcor_current(struct dpp *dpp_base)
{
- enum dc_lut_mode mode;
+ enum dc_lut_mode mode = LUT_BYPASS;
uint32_t state_mode;
uint32_t lut_mode;
struct dcn3_dpp *dpp = TO_DCN30_DPP(dpp_base);
REG_GET(CM_GAMCOR_CONTROL, CM_GAMCOR_MODE_CURRENT, _mode);
- if (state_mode == 0)
- mode = LUT_BYPASS;
-
if (state_mode == 2) {//Programmable RAM LUT
REG_GET(CM_GAMCOR_CONTROL, CM_GAMCOR_SELECT_CURRENT,
_mode);
if (lut_mode == 0)
--
2.34.1
Re: [PATCH] drm/amd/display: Add NULL check for kzalloc in 'amdgpu_dm_atomic_commit_tail()'
[AMD Official Use Only - General]
Prefer drm_err instead of DRM_ERR:
https://elixir.bootlin.com/linux/latest/source/include/drm/drm_print.h#L468
With or without that fixed, patch is
Reviewed-by: Aurabindo Pillai
--
Regards,
Jay
From: SHANMUGAM, SRINIVASAN
Sent: Tuesday, January 30, 2024 4:45 AM
To: Siqueira, Rodrigo ; Pillai, Aurabindo
Cc: amd-gfx@lists.freedesktop.org ; Julia Lawall
; Hung, Alex ; Deucher, Alexander
; Chung, ChiaHsuan (Tom)
Subject: Re: [PATCH] drm/amd/display: Add NULL check for kzalloc in
'amdgpu_dm_atomic_commit_tail()'
+ Cc: Tom Chung
On 1/30/2024 2:11 PM, SHANMUGAM, SRINIVASAN wrote:
> Add a NULL check for the kzalloc call that allocates memory for
> dummy_updates in the amdgpu_dm_atomic_commit_tail function. Previously,
> if kzalloc failed to allocate memory and returned NULL, the code would
> attempt to use the NULL pointer.
>
> The fix is to check if kzalloc returns NULL, and if so, log an error
> message and skip the rest of the current loop iteration with the
> continue statement. This prevents the code from attempting to use the
> NULL pointer.
>
> Cc: Julia Lawall
> Cc: Aurabindo Pillai
> Cc: Rodrigo Siqueira
> Cc: Alex Hung
> Cc: Alex Deucher
> Signed-off-by: Srinivasan Shanmugam
> ---
> drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 4
> 1 file changed, 4 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 0bf1bc7ced7d..8590c9f1dda6 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -9236,6 +9236,10 @@ static void amdgpu_dm_atomic_commit_tail(struct
> drm_atomic_state *state)
> * To fix this, DC should permit updating only stream
> properties.
> */
>dummy_updates = kzalloc(sizeof(struct dc_surface_update) *
> MAX_SURFACES, GFP_ATOMIC);
> + if (!dummy_updates) {
> + DRM_ERROR("Failed to allocate memory for
> dummy_updates.\n");
> + continue;
> + }
>for (j = 0; j < status->plane_count; j++)
>dummy_updates[j].surface = status->plane_states[0];
>
Re: [PATCH] drm/amd/include: Add missing registers/mask for DCN316 and 350
[AMD Official Use Only - General] Reviewed-by: Aurabindo Pillai -- Regards, Jay From: Siqueira, Rodrigo Sent: Thursday, January 25, 2024 1:37 PM To: amd-gfx@lists.freedesktop.org Cc: Siqueira, Rodrigo ; Lei, Jun ; Pillai, Aurabindo ; Mahfooz, Hamza ; Wentland, Harry ; Deucher, Alexander Subject: [PATCH] drm/amd/include: Add missing registers/mask for DCN316 and 350 Cc: Jun Lei Cc: Aurabindo Pillai Cc: Hamza Mahfooz Cc: Harry Wentland Cc: Alex Deucher Signed-off-by: Rodrigo Siqueira --- .../include/asic_reg/dcn/dcn_3_1_6_offset.h | 4 ++ .../include/asic_reg/dcn/dcn_3_1_6_sh_mask.h | 10 +++ .../include/asic_reg/dcn/dcn_3_5_0_offset.h | 24 +++ .../include/asic_reg/dcn/dcn_3_5_0_sh_mask.h | 65 +++ 4 files changed, 103 insertions(+) diff --git a/drivers/gpu/drm/amd/include/asic_reg/dcn/dcn_3_1_6_offset.h b/drivers/gpu/drm/amd/include/asic_reg/dcn/dcn_3_1_6_offset.h index 222fa8d13269..a05bf8e4f58d 100644 --- a/drivers/gpu/drm/amd/include/asic_reg/dcn/dcn_3_1_6_offset.h +++ b/drivers/gpu/drm/amd/include/asic_reg/dcn/dcn_3_1_6_offset.h @@ -626,6 +626,8 @@ #define regDTBCLK_DTO2_MODULO_BASE_IDX 2 #define regDTBCLK_DTO3_MODULO 0x0022 #define regDTBCLK_DTO3_MODULO_BASE_IDX 2 +#define regHDMICHARCLK0_CLOCK_CNTL 0x004a +#define regHDMICHARCLK0_CLOCK_CNTL_BASE_IDX 2 #define regPHYASYMCLK_CLOCK_CNTL 0x0052 #define regPHYASYMCLK_CLOCK_CNTL_BASE_IDX 2 #define regPHYBSYMCLK_CLOCK_CNTL 0x0053 @@ -638,6 +640,8 @@ #define regPHYESYMCLK_CLOCK_CNTL_BASE_IDX 2 #define regPHYFSYMCLK_CLOCK_CNTL 0x0057 #define regPHYFSYMCLK_CLOCK_CNTL_BASE_IDX 2 +#define regHDMISTREAMCLK_CNTL 0x0059 +#define regHDMISTREAMCLK_CNTL_BASE_IDX 2 #define regDCCG_GATE_DISABLE_CNTL3 0x005a #define regDCCG_GATE_DISABLE_CNTL3_BASE_IDX 2 #define regHDMISTREAMCLK0_DTO_PARAM 0x005b diff --git a/drivers/gpu/drm/amd/include/asic_reg/dcn/dcn_3_1_6_sh_mask.h b/drivers/gpu/drm/amd/include/asic_reg/dcn/dcn_3_1_6_sh_mask.h index 8ddb03a1dc39..df84941bbe5b 100644 --- a/drivers/gpu/drm/amd/include/asic_reg/dcn/dcn_3_1_6_sh_mask.h +++ b/drivers/gpu/drm/amd/include/asic_reg/dcn/dcn_3_1_6_sh_mask.h @@ -1933,6 +1933,11 @@ //DTBCLK_DTO3_MODULO #define DTBCLK_DTO3_MODULO__DTBCLK_DTO3_MODULO__SHIFT 0x0 #define DTBCLK_DTO3_MODULO__DTBCLK_DTO3_MODULO_MASK 0xL +//HDMICHARCLK0_CLOCK_CNTL +#define HDMICHARCLK0_CLOCK_CNTL__HDMICHARCLK0_EN__SHIFT 0x0 +#define HDMICHARCLK0_CLOCK_CNTL__HDMICHARCLK0_SRC_SEL__SHIFT 0x4 +#define HDMICHARCLK0_CLOCK_CNTL__HDMICHARCLK0_EN_MASK 0x0001L +#define HDMICHARCLK0_CLOCK_CNTL__HDMICHARCLK0_SRC_SEL_MASK 0x0070L //PHYASYMCLK_CLOCK_CNTL #define PHYASYMCLK_CLOCK_CNTL__PHYASYMCLK_FORCE_EN__SHIFT 0x0 #define PHYASYMCLK_CLOCK_CNTL__PHYASYMCLK_FORCE_SRC_SEL__SHIFT 0x4 @@ -1967,6 +1972,11 @@ #define PHYFSYMCLK_CLOCK_CNTL__PHYFSYMCLK_FORCE_SRC_SEL__SHIFT 0x4 #define PHYFSYMCLK_CLOCK_CNTL__PHYFSYMCLK_FORCE_EN_MASK 0x0001L #define PHYFSYMCLK_CLOCK_CNTL__PHYFSYMCLK_FORCE_SRC_SEL_MASK 0x0030L +//HDMISTREAMCLK_CNTL +#define HDMISTREAMCLK_CNTL__HDMISTREAMCLK0_SRC_SEL__SHIFT 0x0 +#define HDMISTREAMCLK_CNTL__HDMISTREAMCLK0_DTO_FORCE_DIS__SHIFT 0x10 +#define HDMISTREAMCLK_CNTL__HDMISTREAMCLK0_SRC_SEL_MASK
Re: [PATCH] drm/amd/display: fix ABM disablement
[AMD Official Use Only - General]
Does amdgpu_dm_connector_funcs_reset() get called on wakeup from suspend ?
Users would want the system to have the same brightness level before suspending.
--
Regards,
Jay
From: Mahfooz, Hamza
Sent: Wednesday, November 22, 2023 5:24 PM
To: amd-gfx@lists.freedesktop.org
Cc: Wentland, Harry ; Li, Sun peng (Leo)
; Siqueira, Rodrigo ; Koenig,
Christian ; Hung, Alex ; Pillai,
Aurabindo ; Wu, Hersen ; Lin,
Wayne ; Mahfooz, Hamza
Subject: [PATCH] drm/amd/display: fix ABM disablement
On recent versions of DMUB firmware, if we want to completely disable
ABM we have to pass ABM_LEVEL_IMMEDIATE_DISABLE as the requested ABM
level to DMUB. Otherwise, LCD eDP displays are unable to reach their
maximum brightness levels. So, to fix this whenever the user requests an
ABM level of 0 pass ABM_LEVEL_IMMEDIATE_DISABLE to DMUB instead. Also,
to keep the user's experience consistent map ABM_LEVEL_IMMEDIATE_DISABLE
to 0 when a user tries to read the requested ABM level.
Signed-off-by: Hamza Mahfooz
---
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 8 +---
1 file changed, 5 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 5d9496db0ecb..8cb92d941cd9 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -6230,7 +6230,7 @@ int amdgpu_dm_connector_atomic_set_property(struct
drm_connector *connector,
dm_new_state->underscan_enable = val;
ret = 0;
} else if (property == adev->mode_info.abm_level_property) {
- dm_new_state->abm_level = val;
+ dm_new_state->abm_level = val ?: ABM_LEVEL_IMMEDIATE_DISABLE;
ret = 0;
}
@@ -6275,7 +6275,8 @@ int amdgpu_dm_connector_atomic_get_property(struct
drm_connector *connector,
*val = dm_state->underscan_enable;
ret = 0;
} else if (property == adev->mode_info.abm_level_property) {
- *val = dm_state->abm_level;
+ *val = (dm_state->abm_level != ABM_LEVEL_IMMEDIATE_DISABLE) ?
+ dm_state->abm_level : 0;
ret = 0;
}
@@ -6348,7 +6349,8 @@ void amdgpu_dm_connector_funcs_reset(struct drm_connector
*connector)
state->pbn = 0;
if (connector->connector_type == DRM_MODE_CONNECTOR_eDP)
- state->abm_level = amdgpu_dm_abm_level;
+ state->abm_level = amdgpu_dm_abm_level ?:
+ ABM_LEVEL_IMMEDIATE_DISABLE;
__drm_atomic_helper_connector_reset(connector, >base);
}
--
2.42.1
Re: [PATCH 13/15] drm/amd/display: Remove freesync video mode amdgpu parameter
[AMD Official Use Only - General] Hi Stylon, Sorry, the title is misleading. This doesn't touch display. Could you please remove display from the title and make it drm/amd: -- Regards, Jay From: Wang, Chao-kai (Stylon) Sent: Wednesday, August 9, 2023 11:05 AM 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 ; Pillai, Aurabindo Subject: [PATCH 13/15] drm/amd/display: Remove freesync video mode amdgpu parameter From: Aurabindo Pillai [Why] Freesync Video mode was enabled by default. Hence no need for the module parameter, so remove it completely Acked-by: Stylon Wang Signed-off-by: Aurabindo Pillai Reviewed-by: Stylon Wang --- drivers/gpu/drm/amd/amdgpu/amdgpu.h | 1 - drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 27 - 2 files changed, 28 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h index 2e3c7c15cb8e..4de074243c4d 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h @@ -193,7 +193,6 @@ extern int amdgpu_emu_mode; extern uint amdgpu_smu_memory_pool_size; extern int amdgpu_smu_pptable_id; extern uint amdgpu_dc_feature_mask; -extern uint amdgpu_freesync_vid_mode; extern uint amdgpu_dc_debug_mask; extern uint amdgpu_dc_visual_confirm; extern uint amdgpu_dm_abm_level; diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c index 0fec81d6a7df..8f7d0f8e57ae 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c @@ -187,7 +187,6 @@ int amdgpu_mes_kiq; int amdgpu_noretry = -1; int amdgpu_force_asic_type = -1; int amdgpu_tmz = -1; /* auto */ -uint amdgpu_freesync_vid_mode; int amdgpu_reset_method = -1; /* auto */ int amdgpu_num_kcq = -1; int amdgpu_smartshift_bias; @@ -885,32 +884,6 @@ module_param_named(backlight, amdgpu_backlight, bint, 0444); MODULE_PARM_DESC(tmz, "Enable TMZ feature (-1 = auto (default), 0 = off, 1 = on)"); module_param_named(tmz, amdgpu_tmz, int, 0444); -/** - * DOC: freesync_video (uint) - * Enable the optimization to adjust front porch timing to achieve seamless - * mode change experience when setting a freesync supported mode for which full - * modeset is not needed. - * - * The Display Core will add a set of modes derived from the base FreeSync - * video mode into the corresponding connector's mode list based on commonly - * used refresh rates and VRR range of the connected display, when users enable - * this feature. From the userspace perspective, they can see a seamless mode - * change experience when the change between different refresh rates under the - * same resolution. Additionally, userspace applications such as Video playback - * can read this modeset list and change the refresh rate based on the video - * frame rate. Finally, the userspace can also derive an appropriate mode for a - * particular refresh rate based on the FreeSync Mode and add it to the - * connector's mode list. - * - * Note: This is an experimental feature. - * - * The default value: 0 (off). - */ -MODULE_PARM_DESC( - freesync_video, - "Enable freesync modesetting optimization feature (0 = off (default), 1 = on)"); -module_param_named(freesync_video, amdgpu_freesync_vid_mode, uint, 0444); - /** * DOC: reset_method (int) * GPU reset method (-1 = auto (default), 0 = legacy, 1 = mode0, 2 = mode1, 3 = mode2, 4 = baco) -- 2.41.0
Re: [PATCH v2] drm/amd/display: Use root connector's colorspace property for MST
[Public]
Reviewed-by: Aurabindo Pillai
--
Regards,
Jay
From: amd-gfx on behalf of Harry
Wentland
Sent: Thursday, July 13, 2023 3:58 PM
To: amd-gfx@lists.freedesktop.org
Cc: Wentland, Harry
Subject: [PATCH v2] drm/amd/display: Use root connector's colorspace property
for MST
After driver init we shouldn't create new properties. Doing so
will lead to a warning storm from __drm_mode_object_add.
We don't really need to create the property for MST connectors.
Re-using the mst_root connector's property is fine.
v2: Add curly braces to avoid possibly 'else' confusion
Signed-off-by: Harry Wentland
---
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 10 --
1 file changed, 8 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 28f8ac6007fb..f6dab6226b29 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -7357,8 +7357,14 @@ void amdgpu_dm_connector_init_helper(struct
amdgpu_display_manager *dm,
drm_connector_attach_colorspace_property(>base);
} else if (connector_type == DRM_MODE_CONNECTOR_DisplayPort ||
connector_type == DRM_MODE_CONNECTOR_eDP) {
- if (!drm_mode_create_dp_colorspace_property(>base,
supported_colorspaces))
-
drm_connector_attach_colorspace_property(>base);
+ if (!aconnector->mst_root) {
+ if
(!drm_mode_create_dp_colorspace_property(>base,
supported_colorspaces))
+
drm_connector_attach_colorspace_property(>base);
+ } else { /* use root connector's property */
+ if (aconnector->mst_root->base.colorspace_property)
+
drm_connector_attach_colorspace_property(>mst_root->base);
+ }
+
}
if (connector_type == DRM_MODE_CONNECTOR_HDMIA ||
--
2.41.0
Re: [PATCH] Revert "drm/amd/display: Program OTG vtotal min/max selectors unconditionally for DCN1+"
[Public]
Hi Guilherme,
Sorry there was one more patch which I missed to attach. Please add this 3rd
patch and retry.
Reverting that patch would cause high power consumption on Navi2x GPU also
cause hangs on certain multi monitor configurations. With these 3 patches,
you're getting the same effect as reverting the aforementioned patches, but it
makes the reverted sequence available only for Steam deck hardware.
--
Regards,
Jay
From: Guilherme G. Piccoli
Sent: Tuesday, July 11, 2023 7:15 PM
To: Pillai, Aurabindo ; Deucher, Alexander
Cc: amd-gfx@lists.freedesktop.org ; Koenig,
Christian ; Pan, Xinhui ;
dri-de...@lists.freedesktop.org ;
kernel-...@igalia.com ; cristian.ciocal...@collabora.com
; André Almeida ;
Melissa Wen ; Siqueira, Rodrigo
Subject: Re: [PATCH] Revert "drm/amd/display: Program OTG vtotal min/max
selectors unconditionally for DCN1+"
On 11/07/2023 15:22, Aurabindo Pillai wrote:
> [...]
> Hi,
>
> Sorry for the delayed response, this patch went unnoticed. This revert would
> break asics. Could you try the attached patch without reverting this one ?
Hi Aurabindo, thanks for your response!
I've tried kernel 6.5-rc1, and it seems the issue is present, due to the
patch being merged on Linus tree [as 1598fc576420 ("drm/amd/display:
Program OTG vtotal min/max selectors unconditionally for DCN1+")].
Then, I tried both your attached patches on top of that, and
unfortunately, the behavior is the same: Steam Deck doesn't boot with
graphics, and we can see the single error "amdgpu :04:00.0: [drm]
*ERROR* [CRTC:67:crtc-0] flip_done timed out" on dmesg.
Do you / Alex think we could get this revert for 6.5-rc2, so at least we
could boot mainline there while the issue is handled? It would be an
intermediate fix. You mentioned it breaks some asics, but did they work
until now, without your patch?
Thanks,
Guilherme
0001-drm-amd-display-switch-to-DCN301-specific-TG-init.patch
Description: 0001-drm-amd-display-switch-to-DCN301-specific-TG-init.patch
Re: [PATCH 1/2] drm/amd/display: Do not set drr on pipe commit
[Public] >From the log you provided, I extracted the panel id so I expect FAMS to get >disabled on your monitor. If it doesn't work, let me know. -- Regards, Jay From: Michel Dänzer Sent: Thursday, July 6, 2023 10:11 AM To: Pillai, Aurabindo ; amd-gfx@lists.freedesktop.org Cc: Chalmers, Wesley ; Siqueira, Rodrigo ; Wheeler, Daniel ; Mahfooz, Hamza ; Deucher, Alexander ; Wentland, Harry Subject: Re: [PATCH 1/2] drm/amd/display: Do not set drr on pipe commit On 7/6/23 15:55, Pillai, Aurabindo wrote: > > Hi Michel, > > You confirmed in another thread the monitor specific quirk for disabling FAMS > fixed your hang. That's not quite accurate. I confirmed that hard-coding FAMS to be disabled works around the hangs I reported, and you asked me to test an EDID quirk patch, which I reported not to have any effect for my monitor. Then I didn't hear back anything until this patch series today, so I haven't tested the de1da2f7fe25 commit. (I expect it to work, but I can't confirm it yet) -- Earthling Michel Dänzer| https://redhat.com Libre software enthusiast | Mesa and Xwayland developer
Re: [PATCH 1/2] drm/amd/display: Do not set drr on pipe commit
[Public]
Hi Michel,
You confirmed in another thread the monitor specific quirk for disabling FAMS
fixed your hang. Fixes references the commit which adds such quirk:
de1da2f7fe25 drm/amd/display: Add monitor specific edid quirk
The intention is that these two patches should only be merged into stable trees
after applying the monitor specific quirk.
--
Regards,
Jay
From: Michel Dänzer
Sent: Thursday, July 6, 2023 6:05 AM
To: Pillai, Aurabindo ; amd-gfx@lists.freedesktop.org
Cc: Chalmers, Wesley ; Siqueira, Rodrigo
; Wheeler, Daniel ; Mahfooz,
Hamza ; Deucher, Alexander ;
Wentland, Harry
Subject: Re: [PATCH 1/2] drm/amd/display: Do not set drr on pipe commit
On 7/5/23 20:07, Aurabindo Pillai wrote:
> From: Wesley Chalmers
>
> [WHY]
> Writing to DRR registers such as OTG_V_TOTAL_MIN on the same frame as a
> pipe commit can cause underflow.
>
> [HOW]
> Move DMUB p-state delegate into optimze_bandwidth; enabling FAMS sets
> optimized_required.
>
> This change expects that Freesync requests are blocked when
> optimized_required is true.
>
> Fixes: de1da2f7fe25 ("drm/amd/display: Add monitor specific edid quirk")
Seems like inappropriate use of Fixes:, or how does this commit "fix" the
referenced commit?
(Also a bit surprised to learn about that one only now, and that it landed
without my confirming it actually works)
--
Earthling Michel Dänzer| https://redhat.com
Libre software enthusiast | Mesa and Xwayland developer
Re: [PATCH] drm/amdgpu: Add dcdebugmask option to enable DPIA trace
[Public]
Reviewed-by: Aurabindo Pillai
--
Regards,
Jay
From: Wang, Chao-kai (Stylon)
Sent: Thursday, July 6, 2023 3:00 AM
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] drm/amdgpu: Add dcdebugmask option to enable DPIA trace
[Why & How]
It's useful to be able to enable DPIA trace with dcdebugmask
option, especially to debug DPIA issues involved in transition
of system power states.
This patch adds an option to amdgpu.dcdebugmask to be picked up
by amdgpu DM to enable DPIA trace.
Signed-off-by: Stylon Wang
---
drivers/gpu/drm/amd/include/amd_shared.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/amd/include/amd_shared.h
b/drivers/gpu/drm/amd/include/amd_shared.h
index f175e65b853a..abe829bbd54a 100644
--- a/drivers/gpu/drm/amd/include/amd_shared.h
+++ b/drivers/gpu/drm/amd/include/amd_shared.h
@@ -250,6 +250,7 @@ enum DC_DEBUG_MASK {
DC_DISABLE_PSR = 0x10,
DC_FORCE_SUBVP_MCLK_SWITCH = 0x20,
DC_DISABLE_MPO = 0x40,
+ DC_ENABLE_DPIA_TRACE = 0x80,
};
enum amd_dpm_forced_level;
--
2.40.1
Re: [PATCH 10/66] drm/amd/display: Do not set drr on pipe commit
[Public]
Hi Michel,
I want to double check if we're identifying the correct monitor for applying
the workaround. Could you please try the attached patch and let me know the
panel id ?
--
Regards,
Jay
From: Michel Dänzer
Sent: Thursday, June 8, 2023 11:18 AM
To: Pillai, Aurabindo ; Zhuo, Qingqing (Lillian)
; Chalmers, Wesley
Cc: Wang, Chao-kai (Stylon) ; Li, Sun peng (Leo)
; Wentland, Harry ; Siqueira,
Rodrigo ; Li, Roman ;
amd-gfx@lists.freedesktop.org ; Chiu, Solomon
; Lin, Wayne ; Lakha, Bhawanpreet
; Gutierrez, Agustin ;
Kotarac, Pavle
Subject: Re: [PATCH 10/66] drm/amd/display: Do not set drr on pipe commit
On 6/8/23 16:31, Pillai, Aurabindo wrote:
>
> Thanks Michel,
>
> I reached out to windows driver team, and they have a monitor specific quirk
> to disable FAMS on this model. I suspect the issue is only present on certain
> fw revisions on the monitor which is why we cant see your issue.
>
> Unfortunately, having the patches in question reverted causes hangs with 3
> monitor setups. So I will push that monitor specific quirk and bring back the
> reverted patches.
Sounds good, thanks.
--
Earthling Michel Dänzer| https://redhat.com
Libre software enthusiast | Mesa and Xwayland developer
From 3dfcb5e60ec9fc9ec6c573231e5b6aa4edca2ed6 Mon Sep 17 00:00:00 2001
From: Aurabindo Pillai
Date: Mon, 12 Jun 2023 12:44:00 -0400
Subject: [PATCH] drm/amd/display: Add monitor specific edid quirk
Disable FAMS on a Samsung Odyssey G9 monitor. Experiments show that this
monitor does not work well under some use cases, and is likely
implementation specific bug on some revisions of the device.
Signed-off-by: Aurabindo Pillai
---
.../amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 24 +++
1 file changed, 24 insertions(+)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
index cd20cfc04996..e7e545665007 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
@@ -44,6 +44,28 @@
#include "dm_helpers.h"
#include "ddc_service_types.h"
+static u32 edid_extract_panel_id(struct edid *edid)
+{
+ return (u32)edid->mfg_id[0] << 24 |
+ (u32)edid->mfg_id[1] << 16 |
+ (u32)EDID_PRODUCT_ID(edid);
+}
+
+static void apply_edid_quirks(struct edid *edid, struct dc_edid_caps *edid_caps) {
+ uint32_t panel_id = edid_extract_panel_id(edid);
+
+ switch (panel_id) {
+ case drm_edid_encode_panel_id('S', 'A', 'M', 0x0E5E):
+ case drm_edid_encode_panel_id('S', 'A', 'M', 0x7053):
+ pr_err("### Applying any edid quirk for panel %x\n", panel_id);
+ edid_caps->panel_patch.disable_fams = true;
+ break;
+ default:
+ pr_err("### Not applying any edid quirk for panel %x\n", panel_id);
+ return;
+ }
+}
+
/* dm_helpers_parse_edid_caps
*
* Parse edid caps
@@ -115,6 +137,8 @@ enum dc_edid_status dm_helpers_parse_edid_caps(
else
edid_caps->speaker_flags = DEFAULT_SPEAKER_LOCATION;
+ apply_edid_quirks(edid_buf, edid_caps);
+
kfree(sads);
kfree(sadb);
--
2.40.1
Re: [PATCH 10/66] drm/amd/display: Do not set drr on pipe commit
[Public] Thanks Michel, I reached out to windows driver team, and they have a monitor specific quirk to disable FAMS on this model. I suspect the issue is only present on certain fw revisions on the monitor which is why we cant see your issue. Unfortunately, having the patches in question reverted causes hangs with 3 monitor setups. So I will push that monitor specific quirk and bring back the reverted patches. -- Regards, Jay From: Michel Dänzer Sent: Thursday, June 8, 2023 10:20 AM To: Pillai, Aurabindo ; Zhuo, Qingqing (Lillian) ; Chalmers, Wesley Cc: Wang, Chao-kai (Stylon) ; Li, Sun peng (Leo) ; Lakha, Bhawanpreet ; Siqueira, Rodrigo ; Li, Roman ; amd-gfx@lists.freedesktop.org ; Chiu, Solomon ; Lin, Wayne ; Wentland, Harry ; Gutierrez, Agustin ; Kotarac, Pavle Subject: Re: [PATCH 10/66] drm/amd/display: Do not set drr on pipe commit On 6/7/23 19:35, Pillai, Aurabindo wrote: > > Do you see the issue if you force disable FAMS? Neither hang occurs with FAMS disabled. -- Earthling Michel Dänzer| https://redhat.com Libre software enthusiast | Mesa and Xwayland developer
Re: [PATCH 10/66] drm/amd/display: Do not set drr on pipe commit
[Public]
Thanks Michel.
Do you see the issue if you force disable FAMS? The following diff should do:
diff --git a/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_resource.c
b/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_resource.c
index f4ee4b3df596..475c16aab518 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_resource.c
+++ b/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_resource.c
@@ -725,7 +725,8 @@ static const struct dc_debug_options debug_defaults_drv = {
.dwb_fi_phase = -1, // -1 = disable,
.dmub_command_table = true,
.use_max_lb = true,
- .exit_idle_opt_for_cursor_updates = true
+ .exit_idle_opt_for_cursor_updates = true,
+ .disable_fams=true
};
static const struct dc_panel_config panel_config_defaults = {
--
Regards,
Jay
From: Michel Dänzer
Sent: Wednesday, June 7, 2023 1:00 PM
To: Pillai, Aurabindo ; Zhuo, Qingqing (Lillian)
; Chalmers, Wesley
Cc: Wang, Chao-kai (Stylon) ; Li, Sun peng (Leo)
; Wentland, Harry ; Siqueira,
Rodrigo ; Li, Roman ; Chiu, Solomon
; Lin, Wayne ; Lakha, Bhawanpreet
; Gutierrez, Agustin ;
Kotarac, Pavle ; amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH 10/66] drm/amd/display: Do not set drr on pipe commit
On 6/6/23 20:01, Pillai, Aurabindo wrote:
>
> I'm attaching another DMCUB firmware which has the bug fix for the hang we
> saw at our end and some added tracing enabled.
Still runs into the newer hang when starting a KDE Plasma Wayland session.
Should I try this for starting the game without the program OTG patch as well?
> Could you please grab the dmesg with the following added to the kernel
> cmdline: "drm.debug=0x156 log_buf_len=20M" using stock gnome/kde when you
> have all 3 patches merged ?
>
> Also attach the contents of /sys/kernel/debug/dri/0/amdgpu_dm_dmub_tracebuffer
Both files attached.
--
Earthling Michel Dänzer| https://redhat.com
Libre software enthusiast | Mesa and Xwayland developer
Re: [PATCH 10/66] drm/amd/display: Do not set drr on pipe commit
[Public] Hi Michel, AMD driver package also contains various firmware that could make a difference. Even though I do not expect any major deltas for Navi21 at this point, its an extra variable in the equation that could potentially create a different behaviour. We tried upstream stack (without any AMD packaged driver) on Ubuntu 22.04 on the same display that you reported the issue on, but couldn't reproduce the hang you're seeing (using a different steam game that uses the same framework). Maybe your custom gnome build could have affected the results too. Could you provide instructions for setting up your userspace environment ? Other than the game, is there any other workload that could trigger the hang? We have a set of IGT tests you could try: Repo: https://gitlab.freedesktop.org/drm/igt-gpu-tools Build & install: meson build # Compile IGT ninja -C build # Download Piglit ./scripts/run-tests.sh -d Run tests ./scripts/run-tests.sh -s -T /path/to/navi21_postsubmission.testlist -- Regards, Jay From: Michel Dänzer Sent: Thursday, June 1, 2023 11:53 AM To: Pillai, Aurabindo ; Zhuo, Qingqing (Lillian) ; amd-gfx@lists.freedesktop.org ; Chalmers, Wesley Cc: Wang, Chao-kai (Stylon) ; Li, Sun peng (Leo) ; Lakha, Bhawanpreet ; Siqueira, Rodrigo ; Li, Roman ; Chiu, Solomon ; Lin, Wayne ; Wentland, Harry ; Gutierrez, Agustin ; Kotarac, Pavle Subject: Re: [PATCH 10/66] drm/amd/display: Do not set drr on pipe commit On 6/1/23 17:45, Pillai, Aurabindo wrote: > > I see, thanks for the info. I'll try repro'ing it locally. Thanks. Note that I'm using a GNOME Wayland session, which doesn't support VRR upstream yet (I'm building mutter with https://gitlab.gnome.org/GNOME/mutter/-/merge_requests/1154 for that). I don't know if it's reproducible with Xorg. > But do you have the open userspace stack from AMD's packaged driver installed > ? If not, could you please try downloading from > https://www.amd.com/en/support/linux-drivers > <https://www.amd.com/en/support/linux-drivers> and install just the open > components? I don't, and I'd rather not unless it's absolutely necessary. I'm not sure how the user-space drivers could affect this. I'll happily test further patches though. -- Earthling Michel Dänzer| https://redhat.com Libre software enthusiast | Mesa and Xwayland developer navi21_postsubmission.testlist Description: navi21_postsubmission.testlist
Re: [PATCH 10/66] drm/amd/display: Do not set drr on pipe commit
[Public] I see, thanks for the info. I'll try repro'ing it locally. But do you have the open userspace stack from AMD's packaged driver installed ? If not, could you please try downloading from https://www.amd.com/en/support/linux-drivers and install just the open components? You can run: sudo amdgpu-install --use-case=graphics --no-dkms -- Regards, Jay From: amd-gfx on behalf of Michel Dänzer Sent: Thursday, June 1, 2023 10:59 AM To: Pillai, Aurabindo ; Zhuo, Qingqing (Lillian) ; amd-gfx@lists.freedesktop.org ; Chalmers, Wesley Cc: Wang, Chao-kai (Stylon) ; Li, Sun peng (Leo) ; Wentland, Harry ; Siqueira, Rodrigo ; Li, Roman ; Chiu, Solomon ; Lin, Wayne ; Lakha, Bhawanpreet ; Gutierrez, Agustin ; Kotarac, Pavle Subject: Re: [PATCH 10/66] drm/amd/display: Do not set drr on pipe commit On 5/31/23 22:14, Aurabindo Pillai wrote: > On 5/11/23 03:06, Michel Dänzer wrote: >> On 5/10/23 22:54, Aurabindo Pillai wrote: >>> On 5/10/23 09:20, Michel Dänzer wrote: >>>> On 5/9/23 23:07, Pillai, Aurabindo wrote: >>>>> >>>>> Sorry - the firmware in the previous message is for DCN32. For Navi2x, >>>>> please use the firmware attached here. >>>> >>>> Same problem (contents of /sys/kernel/debug/dri/0/amdgpu_firmware_info >>>> below). >>>> >>>> Even if it did work with newer FW, the kernel must keep working with older >>>> FW, so in that case the new behaviour would need to be guarded by the FW >>>> version. >>>> >>> >>> Agreed. Were you able to repro the hang on any other modes/monitors? >> >> Haven't tried specifically, and this is the only system I have with VRR. >> >> > Hi Michel, > > I've fixed a related issue on Navi21. Could you please try the attached DMCUB > along with the patches to be applied on top of amd-staging-drm-next and check > if the hang/corruption is gone? Thanks, though I'm afraid that made it kind of worse: Now it already hangs when Steam starts up in Big Picture mode. Same with the new DMCUB firmware or older one. This time, only amdgpu :0c:00.0: [drm] *ERROR* [CRTC:82:crtc-0] flip_done timed out appears in dmesg. -- Earthling Michel Dänzer| https://redhat.com Libre software enthusiast | Mesa and Xwayland developer
Re: [PATCH 2/2] Revert "drm/amd/display: Do not set drr on pipe commit"
[AMD Official Use Only - General]
Yep, we shall, thanks Alex.
--
Regards,
Jay
From: Alex Deucher
Sent: Tuesday, May 23, 2023 11:47 AM
To: Pillai, Aurabindo ; Mahfooz, Hamza
Cc: Michel Dänzer ; Deucher, Alexander
; Chalmers, Wesley ; Li,
Sun peng (Leo) ; Zhuo, Qingqing (Lillian)
; Siqueira, Rodrigo ;
amd-gfx@lists.freedesktop.org ; Wentland, Harry
Subject: Re: [PATCH 2/2] Revert "drm/amd/display: Do not set drr on pipe commit"
Acked-by: Alex Deucher
for the series.
Jay, I assume you or Hamza will pick these up?
Thanks,
Alex
On Tue, May 23, 2023 at 11:31 AM Aurabindo Pillai
wrote:
>
> Reviewed-by: Aurabindo Pillai
>
> On 5/22/23 09:08, Michel Dänzer wrote:
> > From: Michel Dänzer
> >
> > This reverts commit 474f01015ffdb74e01c2eb3584a2822c64e7b2be.
> >
> > Caused a regression:
> >
> > Samsung Odyssey Neo G9, running at 5120x1440@240/VRR, connected to Navi
> > 21 via DisplayPort, blanks and the GPU hangs while starting the Steam
> > game Assetto Corsa Competizione (via Proton 7.0).
> >
> > Example dmesg excerpt:
> >
> > amdgpu :0c:00.0: [drm] ERROR [CRTC:82:crtc-0] flip_done timed out
> > NMI watchdog: Watchdog detected hard LOCKUP on cpu 6
> > [...]
> > RIP: 0010:amdgpu_device_rreg.part.0+0x2f/0xf0 [amdgpu]
> > Code: 41 54 44 8d 24 b5 00 00 00 00 55 89 f5 53 48 89 fb 4c 3b a7 60 0b 00
> > 00 73 6a 83 e2 02 74 29 4c 03 a3 68 0b 00 00 45 8b 24 24 <48> 8b 43 08 0f
> > b7 70 3e 66 90 44 89 e0 5b 5d 41 5c 31 d2 31 c9 31
> > RSP: :b39a119dfb88 EFLAGS: 0086
> > RAX: c0eb96a0 RBX: 9e7963dc RCX: 7fff
> > RDX: RSI: 4ff6 RDI: 9e7963dc
> > RBP: 4ff6 R08: b39a119dfc40 R09: 0010
> > R10: b39a119dfc40 R11: b39a119dfc44 R12: 000e05ae
> > R13: R14: 9e7963dc0010 R15:
> > FS: 1012f6c0() GS:9e805eb8()
> > knlGS:7fd4
> > CS: 0010 DS: ES: CR0: 80050033
> > CR2: 461ca000 CR3: 0002a8a2 CR4: 00350ee0
> > Call Trace:
> >
> > dm_read_reg_func+0x37/0xc0 [amdgpu]
> > generic_reg_get2+0x22/0x60 [amdgpu]
> > optc1_get_crtc_scanoutpos+0x6a/0xc0 [amdgpu]
> > dc_stream_get_scanoutpos+0x74/0x90 [amdgpu]
> > dm_crtc_get_scanoutpos+0x82/0xf0 [amdgpu]
> > amdgpu_display_get_crtc_scanoutpos+0x91/0x190 [amdgpu]
> > ? dm_read_reg_func+0x37/0xc0 [amdgpu]
> > amdgpu_get_vblank_counter_kms+0xb4/0x1a0 [amdgpu]
> > dm_pflip_high_irq+0x213/0x2f0 [amdgpu]
> > amdgpu_dm_irq_handler+0x8a/0x200 [amdgpu]
> > amdgpu_irq_dispatch+0xd4/0x220 [amdgpu]
> > amdgpu_ih_process+0x7f/0x110 [amdgpu]
> > amdgpu_irq_handler+0x1f/0x70 [amdgpu]
> > __handle_irq_event_percpu+0x46/0x1b0
> > handle_irq_event+0x34/0x80
> > handle_edge_irq+0x9f/0x240
> > __common_interrupt+0x66/0x110
> > common_interrupt+0x5c/0xd0
> > asm_common_interrupt+0x22/0x40
> >
> > Signed-off-by: Michel Dänzer
> > ---
> > drivers/gpu/drm/amd/display/dc/dcn20/dcn20_hwseq.c | 6 --
> > drivers/gpu/drm/amd/display/dc/dcn30/dcn30_hwseq.c | 7 ---
> > 2 files changed, 13 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_hwseq.c
> > b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_hwseq.c
> > index 6ce10fd4bb1a..5403e9399a46 100644
> > --- a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_hwseq.c
> > +++ b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_hwseq.c
> > @@ -2113,12 +2113,6 @@ void dcn20_optimize_bandwidth(
> > if (hubbub->funcs->program_compbuf_size)
> > hubbub->funcs->program_compbuf_size(hubbub,
> > context->bw_ctx.bw.dcn.compbuf_size_kb, true);
> >
> > - if (context->bw_ctx.bw.dcn.clk.fw_based_mclk_switching) {
> > - dc_dmub_srv_p_state_delegate(dc,
> > - true, context);
> > - context->bw_ctx.bw.dcn.clk.p_state_change_support = true;
> > - }
> > -
> > dc->clk_mgr->funcs->update_clocks(
> > dc->clk_mgr,
> > context,
> > diff --git a/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_hwseq.c
> > b/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_hwseq.c
> > index 0411867654dd..0e071fbc9154 100644
> > --- a/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_hwseq.c
> > +++ b/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_hwseq.c
> > @@ -985,18 +985,11 @@ void dcn30_set_disp_patte
Re: [PATCH] Revert "drm/amd/display: enable DPG when disabling plane for phantom pipe"
[AMD Official Use Only - General]
Reviewed-by: Aurabindo Pillai
--
Regards,
Aurabindo
From: Zhuo, Qingqing (Lillian)
Sent: Wednesday, February 15, 2023 2:37 AM
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] Revert "drm/amd/display: enable DPG when disabling plane for
phantom pipe"
This reverts commit b73cf50bd1d0008027cc1b41881b671d9c9054b9.
regression detected by the change. Revert until
fix is available.
Signed-off-by: Qingqing Zhuo
---
drivers/gpu/drm/amd/display/dc/core/dc.c | 47 +---
1 file changed, 1 insertion(+), 46 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/dc/core/dc.c
b/drivers/gpu/drm/amd/display/dc/core/dc.c
index 510661d303e8..1c218c526650 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc.c
@@ -74,8 +74,6 @@
#include "dc_trace.h"
-#include "hw_sequencer_private.h"
-
#include "dce/dmub_outbox.h"
#define CTX \
@@ -1059,44 +1057,6 @@ static void apply_ctx_interdependent_lock(struct dc *dc,
struct dc_state *contex
}
}
-static void phantom_pipe_blank(
- struct dc *dc,
- struct timing_generator *tg,
- int width,
- int height)
-{
- struct dce_hwseq *hws = dc->hwseq;
- enum dc_color_space color_space;
- struct tg_color black_color = {0};
- struct output_pixel_processor *opp = NULL;
- uint32_t num_opps, opp_id_src0, opp_id_src1;
- uint32_t otg_active_width, otg_active_height;
-
- /* program opp dpg blank color */
- color_space = COLOR_SPACE_SRGB;
- color_space_to_black_color(dc, color_space, _color);
-
- otg_active_width = width;
- otg_active_height = height;
-
- /* get the OPTC source */
- tg->funcs->get_optc_source(tg, _opps, _id_src0, _id_src1);
- ASSERT(opp_id_src0 < dc->res_pool->res_cap->num_opp);
- opp = dc->res_pool->opps[opp_id_src0];
-
- opp->funcs->opp_set_disp_pattern_generator(
- opp,
- CONTROLLER_DP_TEST_PATTERN_SOLID_COLOR,
- CONTROLLER_DP_COLOR_SPACE_UDEFINED,
- COLOR_DEPTH_UNDEFINED,
- _color,
- otg_active_width,
- otg_active_height,
- 0);
-
- hws->funcs.wait_for_blank_complete(opp);
-}
-
static void disable_dangling_plane(struct dc *dc, struct dc_state *context)
{
int i, j;
@@ -1155,13 +1115,8 @@ static void disable_dangling_plane(struct dc *dc, struct
dc_state *context)
* again for different use.
*/
if (old_stream->mall_stream_config.type ==
SUBVP_PHANTOM) {
- if (tg->funcs->enable_crtc) {
- int main_pipe_width, main_pipe_height;
- main_pipe_width =
old_stream->mall_stream_config.paired_stream->dst.width;
- main_pipe_height =
old_stream->mall_stream_config.paired_stream->dst.height;
- phantom_pipe_blank(dc, tg,
main_pipe_width, main_pipe_height);
+ if (tg->funcs->enable_crtc)
tg->funcs->enable_crtc(tg);
- }
}
dc_rem_all_planes_for_stream(dc, old_stream,
dangling_context);
disable_all_writeback_pipes_for_stream(dc, old_stream,
dangling_context);
--
2.34.1
Re: [PATCH] drm/amd/display: Properly handle additional cases where DCN is not supported
[AMD Official Use Only - General]
Reviewed-by: Aurabindo Pillai
--
Regards,
Aurabindo
From: Deucher, Alexander
Sent: Friday, January 27, 2023 1:05 PM
To: amd-gfx@lists.freedesktop.org
Cc: Deucher, Alexander ; Pillai, Aurabindo
Subject: [PATCH] drm/amd/display: Properly handle additional cases where DCN is
not supported
There could be boards with DCN listed in IP discovery, but no
display hardware actually wired up. In this case the vbios
display table will not be populated. Detect this case and
skip loading DM when we detect it.
v2: Mark DCN as harvested as well so other display checks
elsewhere in the driver are handled properly.
Cc: Aurabindo Pillai
Signed-off-by: Alex Deucher
---
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 11 +++
1 file changed, 11 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 42d99bf4bbc9..fe66b7aec248 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -4563,6 +4563,17 @@ static int dm_init_microcode(struct amdgpu_device *adev)
static int dm_early_init(void *handle)
{
struct amdgpu_device *adev = (struct amdgpu_device *)handle;
+ struct amdgpu_mode_info *mode_info = >mode_info;
+ struct atom_context *ctx = mode_info->atom_context;
+ int index = GetIndexIntoMasterTable(DATA, Object_Header);
+ u16 data_offset;
+
+ /* if there is no object header, skip DM */
+ if (!amdgpu_atom_parse_data_header(ctx, index, NULL, NULL, NULL,
_offset)) {
+ adev->harvest_ip_mask |= AMD_HARVEST_IP_DMU_MASK;
+ dev_info(adev->dev, "No object header, skipping DM\n");
+ return -ENOENT;
+ }
switch (adev->asic_type) {
#if defined(CONFIG_DRM_AMD_DC_SI)
--
2.39.1
Re: [PATCH] drm/amd/display: fix array-bounds error in dc_stream_remove_writeback()
[AMD Official Use Only - General]
Reviewed-by: Aurabindo Pillai
--
Regards,
Jay
From: Mahfooz, Hamza
Sent: Tuesday, September 27, 2022 3:12 PM
To: linux-ker...@vger.kernel.org
Cc: Mahfooz, Hamza ; Wentland, Harry
; Li, Sun peng (Leo) ; Siqueira,
Rodrigo ; Deucher, Alexander
; Koenig, Christian ; Pan,
Xinhui ; David Airlie ; Daniel Vetter
; Lee, Alvin ; Hung, Alex
; Kotarac, Pavle ; Wang, Chao-kai
(Stylon) ; Pillai, Aurabindo ;
Ma, Leo ; Wu, Hersen ; Po-Yu Hsieh
Paul ; Jimmy Kizito ;
amd-gfx@lists.freedesktop.org ;
dri-de...@lists.freedesktop.org
Subject: [PATCH] drm/amd/display: fix array-bounds error in
dc_stream_remove_writeback()
Address the following error:
drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_stream.c: In function
‘dc_stream_remove_writeback’:
drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_stream.c:527:55: error: array
subscript [0, 0] is outside array bounds of ‘struct dc_writeback_info[1]’
[-Werror=array-bounds]
527 | stream->writeback_info[j] =
stream->writeback_info[i];
| ~~^~~
In file included from ./drivers/gpu/drm/amd/amdgpu/../display/dc/dc.h:1269,
from
./drivers/gpu/drm/amd/amdgpu/../display/dc/inc/core_types.h:29,
from
./drivers/gpu/drm/amd/amdgpu/../display/dc/basics/dc_common.h:29,
from
drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_stream.c:27:
./drivers/gpu/drm/amd/amdgpu/../display/dc/dc_stream.h:241:34: note: while
referencing ‘writeback_info’
241 | struct dc_writeback_info writeback_info[MAX_DWB_PIPES];
|
Currently, we aren't checking to see if j remains within
writeback_info[]'s bounds. So, add a check to make sure that we aren't
overflowing the buffer.
Signed-off-by: Hamza Mahfooz
---
drivers/gpu/drm/amd/display/dc/core/dc_stream.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_stream.c
b/drivers/gpu/drm/amd/display/dc/core/dc_stream.c
index 3ca1592ce7ac..ae13887756bf 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc_stream.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_stream.c
@@ -520,7 +520,7 @@ bool dc_stream_remove_writeback(struct dc *dc,
}
/* remove writeback info for disabled writeback pipes from stream */
- for (i = 0, j = 0; i < stream->num_wb_info; i++) {
+ for (i = 0, j = 0; i < stream->num_wb_info && j < MAX_DWB_PIPES; i++) {
if (stream->writeback_info[i].wb_enabled) {
if (i != j)
/* trim the array */
--
2.37.2
Re: [PATCH 23/31] Add debug option for exiting idle optimizations on cursor updates
[AMD Official Use Only - General]
Hi Jas,
Please add drm/amd/display prefix to the patch title
--
Regards,
Jay
From: Dhillon, Jasdeep
Sent: Wednesday, September 21, 2022 8:18 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 ;
Syu, Brandon ; Cyr, Aric ; Dhillon,
Jasdeep
Subject: [PATCH 23/31] Add debug option for exiting idle optimizations on
cursor updates
From: Brandon Syu
[Description]
- Have option to exit idle opt on cursor updates
for debug and optimizations purposes
Reviewed-by: Aric Cyr
Acked-by: Jasdeep Dhillon
Signed-off-by: Brandon Syu
---
drivers/gpu/drm/amd/display/dc/dcn30/dcn30_resource.c | 3 ++-
drivers/gpu/drm/amd/display/dc/dcn301/dcn301_resource.c | 1 +
drivers/gpu/drm/amd/display/dc/dcn302/dcn302_resource.c | 3 ++-
drivers/gpu/drm/amd/display/dc/dcn303/dcn303_resource.c | 1 +
4 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_resource.c
b/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_resource.c
index f6f3878c99b8..3a3b2ac791c7 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_resource.c
+++ b/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_resource.c
@@ -724,7 +724,8 @@ static const struct dc_debug_options debug_defaults_drv = {
.dwb_fi_phase = -1, // -1 = disable,
.dmub_command_table = true,
.disable_psr = false,
- .use_max_lb = true
+ .use_max_lb = true,
+ .exit_idle_opt_for_cursor_updates = true
};
static const struct dc_debug_options debug_defaults_diags = {
diff --git a/drivers/gpu/drm/amd/display/dc/dcn301/dcn301_resource.c
b/drivers/gpu/drm/amd/display/dc/dcn301/dcn301_resource.c
index 0c2b15a0f3a7..559e563d5bc1 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn301/dcn301_resource.c
+++ b/drivers/gpu/drm/amd/display/dc/dcn301/dcn301_resource.c
@@ -700,6 +700,7 @@ static const struct dc_debug_options debug_defaults_drv = {
.dwb_fi_phase = -1, // -1 = disable
.dmub_command_table = true,
.use_max_lb = false,
+ .exit_idle_opt_for_cursor_updates = true
};
static const struct dc_debug_options debug_defaults_diags = {
diff --git a/drivers/gpu/drm/amd/display/dc/dcn302/dcn302_resource.c
b/drivers/gpu/drm/amd/display/dc/dcn302/dcn302_resource.c
index 4fab537e822f..b925b6ddde5a 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn302/dcn302_resource.c
+++ b/drivers/gpu/drm/amd/display/dc/dcn302/dcn302_resource.c
@@ -93,7 +93,8 @@ static const struct dc_debug_options debug_defaults_drv = {
.underflow_assert_delay_us = 0x,
.dwb_fi_phase = -1, // -1 = disable,
.dmub_command_table = true,
- .use_max_lb = true
+ .use_max_lb = true,
+ .exit_idle_opt_for_cursor_updates = true
};
static const struct dc_debug_options debug_defaults_diags = {
diff --git a/drivers/gpu/drm/amd/display/dc/dcn303/dcn303_resource.c
b/drivers/gpu/drm/amd/display/dc/dcn303/dcn303_resource.c
index d97076648acb..527d5c902878 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn303/dcn303_resource.c
+++ b/drivers/gpu/drm/amd/display/dc/dcn303/dcn303_resource.c
@@ -77,6 +77,7 @@ static const struct dc_debug_options debug_defaults_drv = {
.underflow_assert_delay_us = 0x,
.dwb_fi_phase = -1, // -1 = disable,
.dmub_command_table = true,
+ .exit_idle_opt_for_cursor_updates = true,
.disable_idle_power_optimizations = false,
};
--
2.25.1
Re: [PATCH 21/31] Add ABM control to panel_config struct.
[AMD Official Use Only - General]
Hi Jas,
Please add drm/amd/display prefix to the patch title.
--
Regards,
Jay
From: Dhillon, Jasdeep
Sent: Wednesday, September 21, 2022 8:18 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 ;
Chen, Ian ; Pavic, Josip ; Dhillon,
Jasdeep
Subject: [PATCH 21/31] Add ABM control to panel_config struct.
From: Ian Chen
Reviewed-by: Josip Pavic
Acked-by: Jasdeep Dhillon
Signed-off-by: Ian Chen
---
drivers/gpu/drm/amd/display/dc/dc_link.h | 6 ++
1 file changed, 6 insertions(+)
diff --git a/drivers/gpu/drm/amd/display/dc/dc_link.h
b/drivers/gpu/drm/amd/display/dc/dc_link.h
index 6e49ec262487..bf5f9e2773bc 100644
--- a/drivers/gpu/drm/amd/display/dc/dc_link.h
+++ b/drivers/gpu/drm/amd/display/dc/dc_link.h
@@ -127,6 +127,12 @@ struct dc_panel_config {
unsigned int extra_t12_ms;
unsigned int extra_post_OUI_ms;
} pps;
+ // ABM
+ struct varib {
+ unsigned int varibright_feature_enable;
+ unsigned int def_varibright_level;
+ unsigned int abm_config_setting;
+ } varib;
// edp DSC
struct dsc {
bool disable_dsc_edp;
--
2.25.1
Re: [PATCH] drm/amd/display: Fix register definitions for DCN32/321
[AMD Official Use Only - General]
Thank you Siqueira.
--
Regards,
Jay
From: Siqueira, Rodrigo
Sent: Tuesday, September 6, 2022 11:56 AM
To: Pillai, Aurabindo ; amd-gfx@lists.freedesktop.org
Cc: Wentland, Harry ; Deucher, Alexander
Subject: Re: [PATCH] drm/amd/display: Fix register definitions for DCN32/321
On 2022-09-01 15:27, Aurabindo Pillai wrote:
> [Why & How]
> Fix the instatiation sequence for MPC registers and add a few other
> missing register definitions that were ommited erroneously when copying
> them over to enable runtime initialization of reigster offsets for
> DCN32/321
>
> Signed-off-by: Aurabindo Pillai
> ---
> .../drm/amd/display/dc/dcn32/dcn32_resource.c | 27 +--
> .../drm/amd/display/dc/dcn32/dcn32_resource.h | 216 --
> .../amd/display/dc/dcn321/dcn321_resource.c | 24 +-
> 3 files changed, 166 insertions(+), 101 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/display/dc/dcn32/dcn32_resource.c
> b/drivers/gpu/drm/amd/display/dc/dcn32/dcn32_resource.c
> index ef0a6d468a10..9d3b8568351e 100644
> --- a/drivers/gpu/drm/amd/display/dc/dcn32/dcn32_resource.c
> +++ b/drivers/gpu/drm/amd/display/dc/dcn32/dcn32_resource.c
> @@ -461,22 +461,17 @@ static const struct dcn20_dsc_mask dsc_mask = {
> };
>
> static struct dcn30_mpc_registers mpc_regs;
> -#define dcn_mpc_regs_init()\
> - ( \
> - MPC_REG_LIST_DCN3_0_RI(0),\
> - MPC_REG_LIST_DCN3_0_RI(1),\
> - MPC_REG_LIST_DCN3_0_RI(2),\
> - MPC_REG_LIST_DCN3_0_RI(3),\
> - MPC_OUT_MUX_REG_LIST_DCN3_0_RI(0),\
> - MPC_OUT_MUX_REG_LIST_DCN3_0_RI(1),\
> - MPC_OUT_MUX_REG_LIST_DCN3_0_RI(2),\
> - MPC_OUT_MUX_REG_LIST_DCN3_0_RI(3),\
> - MPC_MCM_REG_LIST_DCN32_RI(0),\
> - MPC_MCM_REG_LIST_DCN32_RI(1),\
> - MPC_MCM_REG_LIST_DCN32_RI(2),\
> - MPC_MCM_REG_LIST_DCN32_RI(3),\
> - MPC_DWB_MUX_REG_LIST_DCN3_0_RI(0)\
> - )
> +
> +#define dcn_mpc_regs_init() \
> + MPC_REG_LIST_DCN3_2_RI(0),\
> + MPC_REG_LIST_DCN3_2_RI(1),\
> + MPC_REG_LIST_DCN3_2_RI(2),\
> + MPC_REG_LIST_DCN3_2_RI(3),\
> + MPC_OUT_MUX_REG_LIST_DCN3_0_RI(0),\
> + MPC_OUT_MUX_REG_LIST_DCN3_0_RI(1),\
> + MPC_OUT_MUX_REG_LIST_DCN3_0_RI(2),\
> + MPC_OUT_MUX_REG_LIST_DCN3_0_RI(3),\
> + MPC_DWB_MUX_REG_LIST_DCN3_0_RI(0)
>
> static const struct dcn30_mpc_shift mpc_shift = {
>MPC_COMMON_MASK_SH_LIST_DCN32(__SHIFT)
> diff --git a/drivers/gpu/drm/amd/display/dc/dcn32/dcn32_resource.h
> b/drivers/gpu/drm/amd/display/dc/dcn32/dcn32_resource.h
> index 60d8fad16eee..4c931905223d 100644
> --- a/drivers/gpu/drm/amd/display/dc/dcn32/dcn32_resource.h
> +++ b/drivers/gpu/drm/amd/display/dc/dcn32/dcn32_resource.h
> @@ -222,7 +222,8 @@ void dcn32_determine_det_override(struct dc_state
> *context, display_e2e_pipe_par
> SRI_ARR(DP_MSA_TIMING_PARAM4, DP, id),
>\
> SRI_ARR(DP_MSE_RATE_CNTL, DP, id), SRI_ARR(DP_MSE_RATE_UPDATE, DP,
> id), \
> SRI_ARR(DP_PIXEL_FORMAT, DP, id), SRI_ARR(DP_SEC_CNTL, DP, id),
>\
> - SRI_ARR(DP_SEC_CNTL2, DP, id), SRI_ARR(DP_SEC_CNTL6, DP, id),
> \
> + SRI_ARR(DP_SEC_CNTL1, DP, id), SRI_ARR(DP_SEC_CNTL2, DP, id),
> \
> + SRI_ARR(DP_SEC_CNTL5, DP, id), SRI_ARR(DP_SEC_CNTL6, DP, id),
> \
> SRI_ARR(DP_STEER_FIFO, DP, id), SRI_ARR(DP_VID_M, DP, id),
>\
> SRI_ARR(DP_VID_N, DP, id), SRI_ARR(DP_VID_STREAM_CNTL, DP, id),
>\
> SRI_ARR(DP_VID_TIMING, DP, id), SRI_ARR(DP_SEC_AUD_N, DP, id),
>\
> @@ -735,75 +736,6 @@ void dcn32_determine_det_override(struct dc_state
> *context, display_e2e_pipe_par
> #define MPC_DWB_MUX_REG_LIST_DCN3_0_RI(inst)
>\
> SRII_DWB(DWB_MUX, MUX, MPC_DWB, inst)
>
> -#define MPC_MCM_REG_LIST_DCN32_RI(inst)
> \
> - ( \
> - SRII(MPCC_MCM_SHAPER_CONTROL, MPCC_MCM, inst),
> \
> - SRII(MPCC_MCM_SHAPER_OFFSET_R, MPCC_MCM, inst),
> \
> - SRII(MPCC_MCM_SHAPER_OFFSET_G, MPCC_MCM, inst),
> \
> - SRII(MPCC_MCM_SHAPER_OFFSET_B, MPCC_MCM, inst),
> \
> - SRII(MPCC_MCM_SHAPER_SCALE_R, MPCC_MCM, inst),
> \
> - SRII(MPCC_MCM_SHAPER_SCALE_G_B, MPCC_MCM, inst),
> \
> - SRII(MPCC_MCM_SHAPER_LUT_INDEX, MPCC_MCM, inst),
> \
>
Re: [PATCH] drm/amd/display: remove stale debug setting
[Public] Thanks, will merge with Cc and fixes tag -- Regards, Jay From: Deucher, Alexander Sent: Wednesday, July 6, 2022 11:33 AM To: Pillai, Aurabindo ; amd-gfx@lists.freedesktop.org Cc: Siqueira, Rodrigo ; Wentland, Harry Subject: Re: [PATCH] drm/amd/display: remove stale debug setting [Public] Acked-by: Alex Deucher From: amd-gfx on behalf of Aurabindo Pillai Sent: Wednesday, July 6, 2022 11:25 AM To: amd-gfx@lists.freedesktop.org Cc: Siqueira, Rodrigo ; Pillai, Aurabindo ; Wentland, Harry Subject: [PATCH] drm/amd/display: remove stale debug setting [Why] The debug option to disable idle power optimization can be dropped Signed-off-by: Aurabindo Pillai --- drivers/gpu/drm/amd/display/dc/dcn32/dcn32_hwseq.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/gpu/drm/amd/display/dc/dcn32/dcn32_hwseq.c b/drivers/gpu/drm/amd/display/dc/dcn32/dcn32_hwseq.c index baaeaaff23df..1cc1296aed9c 100644 --- a/drivers/gpu/drm/amd/display/dc/dcn32/dcn32_hwseq.c +++ b/drivers/gpu/drm/amd/display/dc/dcn32/dcn32_hwseq.c @@ -736,7 +736,6 @@ void dcn32_init_hw(struct dc *dc) int edp_num; uint32_t backlight = MAX_BACKLIGHT_LEVEL; - dc->debug.disable_idle_power_optimizations = true; if (dc->clk_mgr && dc->clk_mgr->funcs->init_clocks) dc->clk_mgr->funcs->init_clocks(dc->clk_mgr); -- 2.37.0
Re: [PATCH] drm/amd/display: ignore modifiers when checking for format support
[AMD Official Use Only - General]
Thanks for noticing, will fix it in a separate patch since I already merged
this.
--
Regards,
Jay
From: Chen, Guchun
Sent: Thursday, June 9, 2022 9:28 PM
To: Pillai, Aurabindo ; Olsak, Marek
; amd-gfx@lists.freedesktop.org
Cc: Li, Sun peng (Leo) ; Siqueira, Rodrigo
; Li, Roman ; Qiao, Ken
; Pillai, Aurabindo ; Deucher,
Alexander ; Wentland, Harry
Subject: RE: [PATCH] drm/amd/display: ignore modifiers when checking for format
support
+ return true;
+ break;
Possibly a coding style problem, 'break' after 'return' looks redundant.
Regards,
Guchun
-Original Message-
From: amd-gfx On Behalf Of Aurabindo
Pillai
Sent: Thursday, June 9, 2022 10:27 PM
To: Olsak, Marek ; amd-gfx@lists.freedesktop.org
Cc: Li, Sun peng (Leo) ; Siqueira, Rodrigo
; Li, Roman ; Qiao, Ken
; Pillai, Aurabindo ; Deucher,
Alexander ; Wentland, Harry
Subject: [PATCH] drm/amd/display: ignore modifiers when checking for format
support
[Why]
There are cases where swizzle modes are set but modifiers arent. For such a
userspace, we need not check modifiers while checking compatibilty in the drm
hook for checking plane format.
Ignore checking modifiers but check the DCN generation for the supported
swizzle mode.
Signed-off-by: Aurabindo Pillai
---
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 51 +--
1 file changed, 46 insertions(+), 5 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 2023baf41b7e..1322df491736 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -4938,6 +4938,7 @@ static bool dm_plane_format_mod_supported(struct
drm_plane *plane, {
struct amdgpu_device *adev = drm_to_adev(plane->dev);
const struct drm_format_info *info = drm_format_info(format);
+ struct hw_asic_id asic_id = adev->dm.dc->ctx->asic_id;
int i;
enum dm_micro_swizzle microtile = modifier_gfx9_swizzle_mode(modifier)
& 3; @@ -4955,13 +4956,53 @@ static bool dm_plane_format_mod_supported(struct
drm_plane *plane,
return true;
}
- /* Check that the modifier is on the list of the plane's supported
modifiers. */
- for (i = 0; i < plane->modifier_count; i++) {
- if (modifier == plane->modifiers[i])
+ /* check if swizzle mode is supported by this version of DCN */
+ switch (asic_id.chip_family) {
+ case FAMILY_SI:
+ case FAMILY_CI:
+ case FAMILY_KV:
+ case FAMILY_CZ:
+ case FAMILY_VI:
+ /* AI and earlier asics does not have modifier support
*/
+ return false;
+ break;
+ case FAMILY_AI:
+ case FAMILY_RV:
+ case FAMILY_NV:
+ case FAMILY_VGH:
+ case FAMILY_YELLOW_CARP:
+ case AMDGPU_FAMILY_GC_10_3_6:
+ case AMDGPU_FAMILY_GC_10_3_7:
+ switch (AMD_FMT_MOD_GET(TILE, modifier)) {
+ case AMD_FMT_MOD_TILE_GFX9_64K_R_X:
+ case AMD_FMT_MOD_TILE_GFX9_64K_D_X:
+ case AMD_FMT_MOD_TILE_GFX9_64K_S_X:
+ case AMD_FMT_MOD_TILE_GFX9_64K_D:
+ return true;
+ break;
+ default:
+ return false;
+ break;
+ }
+ break;
+ case AMDGPU_FAMILY_GC_11_0_0:
+ 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:
+ case AMD_FMT_MOD_TILE_GFX9_64K_D_X:
+ case AMD_FMT_MOD_TILE_GFX9_64K_S_X:
+ case AMD_FMT_MOD_TILE_GFX9_64K_D:
+ return true;
+ break;
+ default:
+ return false;
+ break;
+ }
+ break;
+ default:
+ ASSERT(0); /* Unknown asic */
break;
}
- if (i == plane->modifier_count)
- return false;
/*
* For D swizzle the canonical modifier depends on the bpp, so check
--
2.36.1
Re: [PATCH] drm/amd/display: Use pre-allocated temp struct for bounding box update
[AMD Official Use Only - General]
Isnt it cleaner to revert the original patch which removed the temporary
variable and then instead allocate the clock_limits array on heap, and later
freed at the end of the function?
--
Regards,
Jay
From: Li, Sun peng (Leo)
Sent: Wednesday, June 8, 2022 12:48 PM
To: amd-gfx@lists.freedesktop.org
Cc: Wentland, Harry ; Pillai, Aurabindo
; Siqueira, Rodrigo ; Li,
Sun peng (Leo)
Subject: [PATCH] drm/amd/display: Use pre-allocated temp struct for bounding
box update
From: Leo Li
[Why]
There is a theoretical problem in prior patches for reducing the stack
size of *update_bw_bounding_box() functions.
By modifying the soc.clock_limits[n] struct directly, this can cause
unintended behavior as the for loop attempts to swap rows in
clock_limits[n]. A temporary struct is still required to make sure we
stay functinoally equivalent.
[How]
Add a temporary clock_limits table to the SOC struct, and use it when
swapping rows.
Signed-off-by: Leo Li
---
.../drm/amd/display/dc/dml/dcn20/dcn20_fpu.c | 33 +-
.../amd/display/dc/dml/dcn301/dcn301_fpu.c| 36 ++-
.../drm/amd/display/dc/dml/dcn31/dcn31_fpu.c | 64 +++
.../amd/display/dc/dml/display_mode_structs.h | 5 ++
4 files changed, 82 insertions(+), 56 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/dc/dml/dcn20/dcn20_fpu.c
b/drivers/gpu/drm/amd/display/dc/dml/dcn20/dcn20_fpu.c
index c2fec0d85da4..e247b2270b1d 100644
--- a/drivers/gpu/drm/amd/display/dc/dml/dcn20/dcn20_fpu.c
+++ b/drivers/gpu/drm/amd/display/dc/dml/dcn20/dcn20_fpu.c
@@ -2015,9 +2015,8 @@ void dcn21_update_bw_bounding_box(struct dc *dc, struct
clk_bw_params *bw_params
ASSERT(clk_table->num_entries);
/* Copy dcn2_1_soc.clock_limits to clock_limits to avoid copying over
null states later */
- for (i = 0; i < dcn2_1_soc.num_states + 1; i++) {
- dcn2_1_soc.clock_limits[i] = dcn2_1_soc.clock_limits[i];
- }
+ memcpy(_1_soc._clock_tmp, _1_soc.clock_limits,
+ sizeof(dcn2_1_soc.clock_limits));
for (i = 0; i < clk_table->num_entries; i++) {
/* loop backwards*/
@@ -2032,22 +2031,26 @@ void dcn21_update_bw_bounding_box(struct dc *dc, struct
clk_bw_params *bw_params
if (i == 1)
k++;
- dcn2_1_soc.clock_limits[k].state = k;
- dcn2_1_soc.clock_limits[k].dcfclk_mhz =
clk_table->entries[i].dcfclk_mhz;
- dcn2_1_soc.clock_limits[k].fabricclk_mhz =
clk_table->entries[i].fclk_mhz;
- dcn2_1_soc.clock_limits[k].socclk_mhz =
clk_table->entries[i].socclk_mhz;
- dcn2_1_soc.clock_limits[k].dram_speed_mts =
clk_table->entries[i].memclk_mhz * 2;
+ dcn2_1_soc._clock_tmp[k].state = k;
+ dcn2_1_soc._clock_tmp[k].dcfclk_mhz =
clk_table->entries[i].dcfclk_mhz;
+ dcn2_1_soc._clock_tmp[k].fabricclk_mhz =
clk_table->entries[i].fclk_mhz;
+ dcn2_1_soc._clock_tmp[k].socclk_mhz =
clk_table->entries[i].socclk_mhz;
+ dcn2_1_soc._clock_tmp[k].dram_speed_mts =
clk_table->entries[i].memclk_mhz * 2;
- dcn2_1_soc.clock_limits[k].dispclk_mhz =
dcn2_1_soc.clock_limits[closest_clk_lvl].dispclk_mhz;
- dcn2_1_soc.clock_limits[k].dppclk_mhz =
dcn2_1_soc.clock_limits[closest_clk_lvl].dppclk_mhz;
- dcn2_1_soc.clock_limits[k].dram_bw_per_chan_gbps =
dcn2_1_soc.clock_limits[closest_clk_lvl].dram_bw_per_chan_gbps;
- dcn2_1_soc.clock_limits[k].dscclk_mhz =
dcn2_1_soc.clock_limits[closest_clk_lvl].dscclk_mhz;
- dcn2_1_soc.clock_limits[k].dtbclk_mhz =
dcn2_1_soc.clock_limits[closest_clk_lvl].dtbclk_mhz;
- dcn2_1_soc.clock_limits[k].phyclk_d18_mhz =
dcn2_1_soc.clock_limits[closest_clk_lvl].phyclk_d18_mhz;
- dcn2_1_soc.clock_limits[k].phyclk_mhz =
dcn2_1_soc.clock_limits[closest_clk_lvl].phyclk_mhz;
+ dcn2_1_soc._clock_tmp[k].dispclk_mhz =
dcn2_1_soc.clock_limits[closest_clk_lvl].dispclk_mhz;
+ dcn2_1_soc._clock_tmp[k].dppclk_mhz =
dcn2_1_soc.clock_limits[closest_clk_lvl].dppclk_mhz;
+ dcn2_1_soc._clock_tmp[k].dram_bw_per_chan_gbps =
dcn2_1_soc.clock_limits[closest_clk_lvl].dram_bw_per_chan_gbps;
+ dcn2_1_soc._clock_tmp[k].dscclk_mhz =
dcn2_1_soc.clock_limits[closest_clk_lvl].dscclk_mhz;
+ dcn2_1_soc._clock_tmp[k].dtbclk_mhz =
dcn2_1_soc.clock_limits[closest_clk_lvl].dtbclk_mhz;
+ dcn2_1_soc._clock_tmp[k].phyclk_d18_mhz =
dcn2_1_soc.clock_limits[closest_clk_lvl].phyclk_d18_mhz;
+ dcn2_1_soc._clock_tmp[k].phyclk_mhz =
dcn2_1_soc.clock_limits[closest_clk_lvl].phyclk_mhz;
k++;
}
+
+ memcpy(_1_soc.clock_limits, _1_soc._clock_tmp,
+ sizeof(dcn2_1_soc.clock_limits));
+
Re: [PATCH] drm/amd/display: fix a missing prototype warning in DCN303
[AMD Official Use Only] Hi Siqueira, Thanks for the heads up, lets apply Isabella's patch instead. -- Regards, Jay From: Siqueira, Rodrigo Sent: Wednesday, December 8, 2021 10:17 AM To: Pillai, Aurabindo ; amd-gfx@lists.freedesktop.org ; Isabella Basso Cc: Wentland, Harry ; Siqueira, Rodrigo ; kernel test robot Subject: Re: [PATCH] drm/amd/display: fix a missing prototype warning in DCN303 On 2021-12-07 2:18 p.m., Aurabindo Pillai wrote: > [Why] > A missing include statement triggered a warning when running > a build with W=1: > >>> drivers/gpu/drm/amd/amdgpu/../display/dc/dcn303/dcn303_init.c:30:6: >>> warning: no previous prototype for 'dcn303_hw_sequencer_construct' >>> [-Wmissing-prototypes] >30 | void dcn303_hw_sequencer_construct(struct dc *dc) > | ^ > > Fixes: cd6d421e3d1a (drm/amd/display: Initial DC support for Beige Goby) > Reported-by: kernel test robot > Signed-off-by: Aurabindo Pillai > --- > drivers/gpu/drm/amd/display/dc/dcn303/dcn303_init.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/gpu/drm/amd/display/dc/dcn303/dcn303_init.c > b/drivers/gpu/drm/amd/display/dc/dcn303/dcn303_init.c > index aa5dbbade2bd..d59b24a972bc 100644 > --- a/drivers/gpu/drm/amd/display/dc/dcn303/dcn303_init.c > +++ b/drivers/gpu/drm/amd/display/dc/dcn303/dcn303_init.c > @@ -7,6 +7,7 @@ > > #include "dcn303_hwseq.h" > #include "dcn30/dcn30_init.h" > +#include "dcn303_init.h" > #include "dc.h" > > void dcn303_hw_sequencer_construct(struct dc *dc) > Hi Jay, Isabella made the below patch that fixes this issue in multiple files: https://patchwork.freedesktop.org/patch/465821/?series=97701=1 Maybe we can go ahead with Isabella's patch? Thanks Siqueira
Re: [PATCH 14/19] drm/amd/display: fix HDCP drm prop update for MST
[AMD Official Use Only - Internal Distribution Only]
This patch is a related change for the MST null pointer deref regression, so
will be dropped.
--
Thanks & Regards,
Aurabindo Pillai
From: Aurabindo Pillai
Sent: Friday, April 16, 2021 10:34 AM
To: amd-gfx@lists.freedesktop.org
Cc: Wentland, Harry ; Li, Sun peng (Leo)
; Lakha, Bhawanpreet ; Siqueira,
Rodrigo ; Pillai, Aurabindo
; Zhuo, Qingqing ; Brol, Eryk
; R, Bindu ; Jacob, Anson
; Zhang, Dingchen (David) ; Zhang,
Dingchen (David)
Subject: [PATCH 14/19] drm/amd/display: fix HDCP drm prop update for MST
From: "Dingchen (David) Zhang"
[why]
For MST topology with 1 physical link and multiple connectors (>=2),
e.g. daisy cahined MST + SST, or 1-to-multi MST hub, if userspace
set to enable the HDCP simultaneously on all connected outputs, the
commit tail iteratively call the hdcp_update_display() for each
display (connector). However, the hdcp workqueue data structure for
each link has only one DM connector and encryption status members,
which means the work queue of property_validate/update() would only
be triggered for the last connector within this physical link, and
therefore the HDCP property value of other connectors would stay on
DESIRED instead of switching to ENABLED, which is NOT as expected.
[how]
Use array of MAX_NUM_OF_DISPLAY for both aconnector and encryption
status in hdcp workqueue data structure for each physical link.
For property validate/update work queue, we iterates over the array
and do similar operation/check for each connected display.
Signed-off-by: Dingchen (David) Zhang
Reviewed-by: Dingchen Zhang
Acked-by: Aurabindo Pillai
---
.../amd/display/amdgpu_dm/amdgpu_dm_hdcp.c| 109 +-
.../amd/display/amdgpu_dm/amdgpu_dm_hdcp.h| 6 +-
2 files changed, 81 insertions(+), 34 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_hdcp.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_hdcp.c
index 50f6b3a86931..2ec076af9e89 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_hdcp.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_hdcp.c
@@ -171,9 +171,10 @@ void hdcp_update_display(struct hdcp_workqueue *hdcp_work,
struct mod_hdcp_display *display = _work[link_index].display;
struct mod_hdcp_link *link = _work[link_index].link;
struct mod_hdcp_display_query query;
+ unsigned int conn_index = aconnector->base.index;
mutex_lock(_w->mutex);
- hdcp_w->aconnector = aconnector;
+ hdcp_w->aconnector[conn_index] = aconnector;
query.display = NULL;
mod_hdcp_query_display(_w->hdcp, aconnector->base.index, );
@@ -205,7 +206,7 @@ void hdcp_update_display(struct hdcp_workqueue *hdcp_work,
msecs_to_jiffies(DRM_HDCP_CHECK_PERIOD_MS));
} else {
display->adjust.disable =
MOD_HDCP_DISPLAY_DISABLE_AUTHENTICATION;
- hdcp_w->encryption_status =
MOD_HDCP_ENCRYPTION_STATUS_HDCP_OFF;
+ hdcp_w->encryption_status[conn_index] =
MOD_HDCP_ENCRYPTION_STATUS_HDCP_OFF;
cancel_delayed_work(_w->property_validate_dwork);
}
@@ -224,9 +225,10 @@ static void hdcp_remove_display(struct hdcp_workqueue
*hdcp_work,
{
struct hdcp_workqueue *hdcp_w = _work[link_index];
struct drm_connector_state *conn_state = aconnector->base.state;
+ unsigned int conn_index = aconnector->base.index;
mutex_lock(_w->mutex);
- hdcp_w->aconnector = aconnector;
+ hdcp_w->aconnector[conn_index] = aconnector;
/* the removal of display will invoke auth reset -> hdcp destroy and
* we'd expect the CP prop changed back to DESIRED if at the time
ENABLED.
@@ -247,13 +249,16 @@ static void hdcp_remove_display(struct hdcp_workqueue
*hdcp_work,
void hdcp_reset_display(struct hdcp_workqueue *hdcp_work, unsigned int
link_index)
{
struct hdcp_workqueue *hdcp_w = _work[link_index];
+ unsigned int conn_index;
mutex_lock(_w->mutex);
mod_hdcp_reset_connection(_w->hdcp, _w->output);
cancel_delayed_work(_w->property_validate_dwork);
- hdcp_w->encryption_status = MOD_HDCP_ENCRYPTION_STATUS_HDCP_OFF;
+
+ for (conn_index = 0; conn_index < MAX_NUM_OF_DISPLAYS; ++conn_index)
+ hdcp_w->encryption_status[conn_index] =
MOD_HDCP_ENCRYPTION_STATUS_HDCP_OFF;
process_output(hdcp_w);
@@ -290,38 +295,67 @@ static void event_callback(struct work_struct *work)
}
+
+static struct amdgpu_dm_connector *find_first_connected_output(struct
hdcp_workqueue *hdcp_work)
+{
+ unsigned int conn_index;
+
+ for (conn_index = 0; conn_index < MAX_NUM_OF_DISPLAYS; ++conn_index) {
+ if (hdcp_work
Re: [PATCH 13/19] drm/amd/display: force CP to DESIRED when removing display.
[AMD Official Use Only - Internal Distribution Only]
This patch introduces a null pointer deref on MST hotplug, so this shall be
dropped.
--
Thanks & Regards,
Aurabindo Pillai
From: Aurabindo Pillai
Sent: Friday, April 16, 2021 10:34 AM
To: amd-gfx@lists.freedesktop.org
Cc: Wentland, Harry ; Li, Sun peng (Leo)
; Lakha, Bhawanpreet ; Siqueira,
Rodrigo ; Pillai, Aurabindo
; Zhuo, Qingqing ; Brol, Eryk
; R, Bindu ; Jacob, Anson
; Zhang, Dingchen (David) ; Zhang,
Dingchen (David)
Subject: [PATCH 13/19] drm/amd/display: force CP to DESIRED when removing
display.
From: "Dingchen (David) Zhang"
[why]
It is possible that the commit from userspace to cause link stream
disable and hdcp auth reset when the HDCP has been enabled at the
moment. We'd expect the CP prop back to DESIRED from ENABLED.
[how]
In the helper of hdcp display removal, we check and change the CP prop
to DESIRED if at the moment CP is ENABLED before the auth reset and
removal of linked list element.
Signed-off-by: Dingchen (David) Zhang
Reviewed-by: Dingchen Zhang
Acked-by: Aurabindo Pillai
---
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_hdcp.c | 13 +
1 file changed, 13 insertions(+)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_hdcp.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_hdcp.c
index 616f5b1ea3a8..50f6b3a86931 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_hdcp.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_hdcp.c
@@ -160,6 +160,7 @@ static void link_lock(struct hdcp_workqueue *work, bool
lock)
mutex_unlock([i].mutex);
}
}
+
void hdcp_update_display(struct hdcp_workqueue *hdcp_work,
unsigned int link_index,
struct amdgpu_dm_connector *aconnector,
@@ -222,10 +223,22 @@ static void hdcp_remove_display(struct hdcp_workqueue
*hdcp_work,
struct amdgpu_dm_connector *aconnector)
{
struct hdcp_workqueue *hdcp_w = _work[link_index];
+ struct drm_connector_state *conn_state = aconnector->base.state;
mutex_lock(_w->mutex);
hdcp_w->aconnector = aconnector;
+ /* the removal of display will invoke auth reset -> hdcp destroy and
+* we'd expect the CP prop changed back to DESIRED if at the time
ENABLED.
+* the CP prop change should occur before the element removed from
linked list.
+*/
+ if (conn_state && conn_state->content_protection ==
DRM_MODE_CONTENT_PROTECTION_ENABLED) {
+ conn_state->content_protection =
DRM_MODE_CONTENT_PROTECTION_DESIRED;
+
+ pr_debug("[HDCP_DM] display %d, CP 2 -> 1, type %u, DPMS %u\n",
+aconnector->base.index, conn_state->hdcp_content_type,
aconnector->base.dpms);
+ }
+
mod_hdcp_remove_display(_w->hdcp, aconnector->base.index,
_w->output);
process_output(hdcp_w);
--
2.31.1
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amd/display: Update DCN302 SR Exit Latency
[AMD Official Use Only - Internal Distribution Only]
Hi Bindu,
The value is recommended by the hardware team. Since a non-recommended value
was being used, that itself is the root cause. I will add this to commit
message.
--
Thanks & Regards,
Aurabindo Pillai
From: R, Bindu
Sent: Thursday, April 8, 2021 11:55 AM
To: Pillai, Aurabindo ; amd-gfx@lists.freedesktop.org
Cc: Aberback, Joshua
Subject: Re: [PATCH] drm/amd/display: Update DCN302 SR Exit Latency
[AMD Official Use Only - Internal Distribution Only]
Hi Jay,
Could you please add few details on the root cause, in the [Why/How] section?
Thanks,
Bindu
From: Aurabindo Pillai
Sent: Thursday, April 8, 2021 11:48 AM
To: amd-gfx@lists.freedesktop.org
Cc: Pillai, Aurabindo ; R, Bindu ;
Aberback, Joshua
Subject: [PATCH] drm/amd/display: Update DCN302 SR Exit Latency
From: Joshua Aberback
[Why]
Update SR Exit Latency to fix screen flickering caused due to OTG
underflow
Signed-off-by: Joshua Aberback
Acked-by: Aurabindo Pillai
---
drivers/gpu/drm/amd/display/dc/dcn302/dcn302_resource.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/display/dc/dcn302/dcn302_resource.c
b/drivers/gpu/drm/amd/display/dc/dcn302/dcn302_resource.c
index a928c1d9a557..fc2dea243d1b 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn302/dcn302_resource.c
+++ b/drivers/gpu/drm/amd/display/dc/dcn302/dcn302_resource.c
@@ -164,7 +164,7 @@ struct _vcs_dpi_soc_bounding_box_st dcn3_02_soc = {
.min_dcfclk = 500.0, /* TODO: set this to actual min DCFCLK */
.num_states = 1,
- .sr_exit_time_us = 12,
+ .sr_exit_time_us = 15.5,
.sr_enter_plus_exit_time_us = 20,
.urgent_latency_us = 4.0,
.urgent_latency_pixel_data_only_us = 4.0,
--
2.31.1
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH v3 1/3] drm/amd/display: Add module parameter for freesync video mode
[AMD Official Use Only - Internal Distribution Only] Hi Daniel, Could you please be more specific about the _unsafe API options you mentioned ? -- Thanks & Regards, Aurabindo Pillai From: Daniel Vetter Sent: Tuesday, January 19, 2021 8:11 AM To: Pekka Paalanen Cc: Pillai, Aurabindo ; amd-gfx list ; dri-devel ; Kazlauskas, Nicholas ; Wang, Chao-kai (Stylon) ; Thai, Thong ; Sharma, Shashank ; Lin, Wayne ; Deucher, Alexander ; Koenig, Christian Subject: Re: [PATCH v3 1/3] drm/amd/display: Add module parameter for freesync video mode On Tue, Jan 19, 2021 at 9:35 AM Pekka Paalanen wrote: > > On Mon, 18 Jan 2021 09:36:47 -0500 > Aurabindo Pillai wrote: > > > On Thu, 2021-01-14 at 11:14 +0200, Pekka Paalanen wrote: > > > > > > Hi, > > > > > > please document somewhere that ends up in git history (commit > > > message, > > > code comments, description of the parameter would be the best but > > > maybe > > > there isn't enough space?) what Christian König explained in > > > > > > > > > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Farchives%2Fdri-devel%2F2020-December%2F291254.htmldata=04%7C01%7Caurabindo.pillai%40amd.com%7C56ba07934c5c48e7ad7b08d8bc7bb4a9%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637466586800649481%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000sdata=GM0ZEM9JeFM5os13E1zlVy8Bn3D8Kxmo%2FajSG02WsGI%3Dreserved=0 > > > > > > that this is a stop-gap feature intended to be removed as soon as > > > possible (when a better solution comes up, which could be years). > > > > > > So far I have not seen a single mention of this intention in your > > > patch > > > submissions, and I think it is very important to make known. > > > > Hi, > > > > Thanks for the headsup, I shall add the relevant info in the next > > verison. > > > > > > > > I also did not see an explanation of why this instead of > > > manufacturing > > > these video modes in userspace (an idea mentioned by Christian in the > > > referenced email). I think that too should be part of a commit > > > message. > > > > This is an opt-in feature, which shall be superseded by a better > > solution. We also add a set of common modes for scaling similarly. > > Userspace can still add whatever mode they want. So I dont see a reason > > why this cant be in the kernel. > > Hi, > > sorry, I think that kind of thinking is backwards. There needs to be a > reason to put something in the kernel, and if there is no reason, then > it remains in userspace. So what's the reason to put this in the kernel? > > One example reason why this should not be in the kernel is that the set > of video modes to manufacture is a kind of policy, which modes to add > and which not. Userspace knows what modes it needs, and establishing > the modes in the kernel instead is second-guessing what the userspace > would want. So if userspace needs to manufacture modes in userspace > anyway as some modes might be missed by the kernel, then why bother in > the kernel to begin with? Why should the kernel play catch-up with what > modes userspace wants when we already have everything userspace needs > to make its own modes, even to add them to the kernel mode list? > > Does manufacturing these extra video modes to achieve fast timing > changes require AMD hardware-specific knowledge, as opposed to the > general VRR approach of simply adjusting the front porch? > > Something like this should also be documented in a commit message. Or > if you insist that "no reason to not put this in the kernel" is reason > enough, then write that down, because it does not seem obvious to me or > others that this feature needs to be in the kernel. One reason might be debugging, if a feature is known to cause issues. But imo in that case the knob should be using the _unsafe variants so it taints the kernel, since otherwise we get stuck in this very cozy place where kernel maintainers don't have to care much for bugs "because it's off by default", but also not really care about polishing the feature "since users can just enable it if they want it". Just a slightly different flavour of what you're explaining above already. -Daniel > Thanks, > pq -- Daniel Vetter Software Engineer, Intel Corporation https://nam11.safelinks.protection.outlook.com/?url=http%3A%2F%2Fblog.ffwll.ch%2Fdata=04%7C01%7Caurabindo.pillai%40amd.com%7C56ba07934c5c48e7ad7b08d8bc7bb4a9%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637466586800649481%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000sdata=2isCpwa3V92TnO4njhe9cQjdWVdsV1GQMo7WP7buVZI%3Dreserved=0 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH v3] drm/amd/display: turn DPMS off on mst connector unplug
[AMD Official Use Only - Internal Distribution Only]
Thank you very much for the review!
--
Thanks & Regards,
Aurabindo Pillai
From: Kazlauskas, Nicholas
Sent: Friday, November 27, 2020 10:44 AM
To: Pillai, Aurabindo ; amd-gfx@lists.freedesktop.org
Cc: Wentland, Harry ; Lakha, Bhawanpreet
; Brol, Eryk
Subject: Re: [PATCH v3] drm/amd/display: turn DPMS off on mst connector unplug
On 2020-11-26 7:18 p.m., Aurabindo Pillai wrote:
> [Why]
>
> Set dpms off on the MST connector that was unplugged, for the side effect of
> releasing some references held through deallocation of mst payload.
Applies to non-MST now too, so the description and title should be updated.
>
> Signed-off-by: Aurabindo Pillai
> Signed-off-by: Eryk Brol
> ---
> .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 31 ++-
> drivers/gpu/drm/amd/display/dc/core/dc.c | 13
> drivers/gpu/drm/amd/display/dc/dc_stream.h| 1 +
> 3 files changed, 44 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 e213246e3f04..9966679d29e7 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -1984,6 +1984,32 @@ static void dm_gpureset_commit_state(struct dc_state
> *dc_state,
>return;
> }
>
> +static void dm_set_dpms_off(struct dc_link *link)
> +{
> + struct dc_stream_state *stream_state;
> + struct amdgpu_dm_connector *aconnector = link->priv;
> + struct amdgpu_device *adev = drm_to_adev(aconnector->base.dev);
> + struct dc_stream_update stream_update = {0};
Please use a memset instead of a zero initializer here. Some compilers
complain about that.
> + bool dpms_off = true;
> +
> + stream_update.dpms_off = _off;
> +
> + mutex_lock(>dm.dc_lock);
> + stream_state = dc_stream_find_from_link(link);
> +
> + if (stream_state == NULL) {
> + dm_error("Error finding stream state associated with link!\n");
This shouldn't be using a dm_error print here. a DRM_DEBUG_DRIVER would
be better suited.
With these three items fixed the v4 of this patch will be:
Reviewed-by: Nicholas Kazlauskas
Regards,
Nicholas Kazlauskas
> + mutex_unlock(>dm.dc_lock);
> + return;
> + }
> +
> + stream_update.stream = stream_state;
> + dc_commit_updates_for_stream(stream_state->ctx->dc, NULL, 0,
> + stream_state, _update,
> + stream_state->ctx->dc->current_state);
> + mutex_unlock(>dm.dc_lock);
> +}
> +
> static int dm_resume(void *handle)
> {
>struct amdgpu_device *adev = handle;
> @@ -2434,8 +2460,11 @@ static void handle_hpd_irq(void *param)
>drm_kms_helper_hotplug_event(dev);
>
>} else if (dc_link_detect(aconnector->dc_link, DETECT_REASON_HPD)) {
> - amdgpu_dm_update_connector_after_detect(aconnector);
> + if (new_connection_type == dc_connection_none &&
> + aconnector->dc_link->type == dc_connection_none)
> + dm_set_dpms_off(aconnector->dc_link);
>
> + amdgpu_dm_update_connector_after_detect(aconnector);
>
>drm_modeset_lock_all(dev);
>dm_restore_drm_connector_state(dev, connector);
> diff --git a/drivers/gpu/drm/amd/display/dc/core/dc.c
> b/drivers/gpu/drm/amd/display/dc/core/dc.c
> index 903353389edb..58eb0d69873a 100644
> --- a/drivers/gpu/drm/amd/display/dc/core/dc.c
> +++ b/drivers/gpu/drm/amd/display/dc/core/dc.c
> @@ -2798,6 +2798,19 @@ struct dc_stream_state *dc_get_stream_at_index(struct
> dc *dc, uint8_t i)
>return NULL;
> }
>
> +struct dc_stream_state *dc_stream_find_from_link(const struct dc_link *link)
> +{
> + uint8_t i;
> + struct dc_context *ctx = link->ctx;
> +
> + for (i = 0; i < ctx->dc->current_state->stream_count; i++) {
> + if (ctx->dc->current_state->streams[i]->link == link)
> + return ctx->dc->current_state->streams[i];
> + }
> +
> + return NULL;
> +}
> +
> enum dc_irq_source dc_interrupt_to_irq_source(
>struct dc *dc,
>uint32_t src_id,
> diff --git a/drivers/gpu/drm/amd/display/dc/dc_stream.h
> b/drivers/gpu/drm/amd/display/dc/dc_stream.h
> index bf090afc2f70..b7910976b81a 100644
> --- a/drivers/gpu/drm/amd/display/dc/dc_stream.h
> +++ b/drivers/gpu/drm/amd/display/dc/dc_stream.h
> @@ -292,6 +292,7 @@ void dc_stre
