* [RFC PATCH v2 0/5] drm/amd/display: improve DTN color state log
@ 2023-09-13 16:43 Melissa Wen
2023-09-13 16:43 ` [RFC PATCH v2 1/5] drm/amd/display: detach color state from hw state logging Melissa Wen
` (6 more replies)
0 siblings, 7 replies; 23+ messages in thread
From: Melissa Wen @ 2023-09-13 16:43 UTC (permalink / raw)
To: Harry Wentland, Rodrigo Siqueira, sunpeng.li, airlied,
alexander.deucher, christian.koenig, daniel, Xinhui.Pan
Cc: Krunoslav Kovac, Shashank Sharma, dri-devel, amd-gfx, kernel-dev,
Nicholas Kazlauskas, sungjoon.kim
Hi,
This is an update of previous RFC [0] improving the data collection of
Gamma Correction and Blend Gamma color blocks.
As I mentioned in the last version, I'm updating the color state part of
DTN log to match DCN3.0 HW better. Currently, the DTN log considers the
DCN10 color pipeline, which is useless for DCN3.0 because of all the
differences in color caps between DCN versions. In addition to new color
blocks and caps, some semantic differences made the DCN10 output not fit
DCN30.
In this RFC, the first patch adds new color state elements to DPP and
implements the reading of registers according to HW blocks. Similarly to
MPC, the second patch also creates a DCN3-specific function to read the
MPC state and add the MPC color state logging to it. With DPP and MPC
color-register reading, I detach DCN10 color state logging from the HW
log and create a `.log_color_state` hook for logging color state
according to HW color blocks with DCN30 as the first use case. Finally,
the last patch adds DPP and MPC color caps output to facilitate
understanding of the color state log.
This version works well with the driver-specific color properties[1] and
steamdeck/gamescope[2] together, where we can see color state changing
from default values.
Here is a before vs. after example:
Without this series:
===================
DPP: IGAM format IGAM mode DGAM mode RGAM mode GAMUT mode C11 C12 C13 C14 C21 C22 C23 C24 C31 C32 C33 C34
[ 0]: 0h BypassFixed Bypass Bypass 0 00000000h 00000000h 00000000h 00000000h 00000000h 00000000h
[ 3]: 0h BypassFixed Bypass Bypass 0 00000000h 00000000h 00000000h 00000000h 00000000h 00000000h
MPCC: OPP DPP MPCCBOT MODE ALPHA_MODE PREMULT OVERLAP_ONLY IDLE
[ 0]: 0h 0h 3h 3 2 0 0 0
[ 3]: 0h 3h fh 2 2 0 0 0
With this series (Steamdeck/Gamescope):
======================================
DPP: DGAM ROM DGAM ROM type DGAM LUT SHAPER mode 3DLUT mode 3DLUT bit depth 3DLUT size RGAM mode GAMUT mode C11 C12 C13 C14 C21 C22 C23 C24 C31 C32 C33 C34
[ 0]: 1 sRGB Bypass RAM A RAM B 12-bit 17x17x17 RAM A 0 00000000h 00000000h 00000000h 00000000h 00000000h 00000000h
[ 1]: 1 sRGB Bypass RAM B RAM A 12-bit 17x17x17 RAM A 0 00000000h 00000000h 00000000h 00000000h 00000000h 00000000h
[ 2]: 1 sRGB Bypass RAM B RAM A 12-bit 17x17x17 RAM A 0 00000000h 00000000h 00000000h 00000000h 00000000h 00000000h
[ 3]: 1 sRGB Bypass RAM A RAM B 12-bit 17x17x17 RAM A 0 00000000h 00000000h 00000000h 00000000h 00000000h 00000000h
DPP Color Caps: input_lut_shared:0 icsc:1 dgam_ram:0 dgam_rom: srgb:1,bt2020:1,gamma2_2:1,pq:1,hlg:1 post_csc:1 gamcor:1 dgam_rom_for_yuv:0 3d_lut:1 blnd_lut:1 oscs:0
MPCC: OPP DPP MPCCBOT MODE ALPHA_MODE PREMULT OVERLAP_ONLY IDLE SHAPER mode 3DLUT_mode 3DLUT bit-depth 3DLUT size OGAM mode OGAM LUT GAMUT mode C11 C12 C33 C34
[ 0]: 0h 0h 2h 3 0 1 0 0 Bypass Bypass 12-bit 17x17x17 RAM A 0 00000000h 00000000h
[ 1]: 0h 1h fh 2 2 0 0 0 Bypass Bypass 12-bit 17x17x17 Bypass A 0 00000000h 00000000h
[ 2]: 0h 2h 3h 3 0 1 0 0 Bypass Bypass 12-bit 17x17x17 Bypass A 0 00000000h 00000000h
[ 3]: 0h 3h 1h 3 2 0 0 0 Bypass Bypass 12-bit 17x17x17 Bypass A 0 00000000h 00000000h
MPC Color Caps: gamut_remap:1, 3dlut:2, ogam_ram:1, ocsc:1
With this series (Steamdeck/KDE):
================================
DPP: DGAM ROM DGAM ROM type DGAM LUT SHAPER mode 3DLUT mode 3DLUT bit depth 3DLUT size RGAM mode GAMUT mode C11 C12 C13 C14 C21 C22 C23 C24 C31 C32 C33 C34
[ 0]: 0 sRGB Bypass Bypass Bypass 12-bit 9x9x9 Bypass 0 00000000h 00000000h 00000000h 00000000h 00000000h 00000000h
[ 3]: 0 sRGB Bypass Bypass Bypass 12-bit 9x9x9 Bypass 0 00000000h 00000000h 00000000h 00000000h 00000000h 00000000h
DPP Color Caps: input_lut_shared:0 icsc:1 dgam_ram:0 dgam_rom: srgb:1,bt2020:1,gamma2_2:1,pq:1,hlg:1 post_csc:1 gamcor:1 dgam_rom_for_yuv:0 3d_lut:1 blnd_lut:1 oscs:0
MPCC: OPP DPP MPCCBOT MODE ALPHA_MODE PREMULT OVERLAP_ONLY IDLE SHAPER mode 3DLUT_mode 3DLUT bit-depth 3DLUT size OGAM mode OGAM LUT GAMUT mode C11 C12 C33 C34
[ 0]: 0h 0h 3h 3 2 0 0 0 Bypass Bypass 12-bit 17x17x17 RAM A 1 00002000h 00002000h
[ 3]: 0h 3h fh 2 2 0 0 0 Bypass Bypass 12-bit 17x17x17 Bypass A 0 00000000h 00000000h
MPC Color Caps: gamut_remap:1, 3dlut:2, ogam_ram:1, ocsc:1
Before extending it to other DCN families, I have some doubts.
- Does this approach of the `.log_color_state` hook make sense for you?
- Is there any conflict between logging color state by HW version and
DTN log usage?
- Is there a template/style for DTN log output that I should follow or
information that I should include?
Let me know your thoughts.
Thanks,
Melissa
[0] https://lore.kernel.org/amd-gfx/20230905142545.451153-1-mwen@igalia.com/
[1] https://lore.kernel.org/amd-gfx/20230810160314.48225-1-mwen@igalia.com/
[2] https://github.com/ValveSoftware/gamescope
Melissa Wen (5):
drm/amd/display: detach color state from hw state logging
drm/amd/display: fill up DCN3 DPP color state
drm/amd/display: create DCN3-specific log for MPC state
drm/amd/display: hook DCN30 color state logging to DTN log
drm/amd/display: add DPP and MPC color caps to DTN log
.../amd/display/dc/dcn10/dcn10_hw_sequencer.c | 53 +++++++--
.../gpu/drm/amd/display/dc/dcn30/dcn30_dpp.c | 45 ++++++-
.../drm/amd/display/dc/dcn30/dcn30_hwseq.c | 111 ++++++++++++++++++
.../drm/amd/display/dc/dcn30/dcn30_hwseq.h | 3 +
.../gpu/drm/amd/display/dc/dcn30/dcn30_init.c | 1 +
.../gpu/drm/amd/display/dc/dcn30/dcn30_mpc.c | 55 ++++++++-
.../drm/amd/display/dc/dcn301/dcn301_init.c | 1 +
drivers/gpu/drm/amd/display/dc/inc/hw/dpp.h | 8 ++
drivers/gpu/drm/amd/display/dc/inc/hw/mpc.h | 13 ++
.../gpu/drm/amd/display/dc/inc/hw_sequencer.h | 2 +
10 files changed, 280 insertions(+), 12 deletions(-)
--
2.40.1
^ permalink raw reply [flat|nested] 23+ messages in thread
* [RFC PATCH v2 1/5] drm/amd/display: detach color state from hw state logging
2023-09-13 16:43 [RFC PATCH v2 0/5] drm/amd/display: improve DTN color state log Melissa Wen
@ 2023-09-13 16:43 ` Melissa Wen
2023-09-13 22:41 ` Rodrigo Siqueira Jordao
2023-09-13 16:43 ` [RFC PATCH v2 2/5] drm/amd/display: fill up DCN3 DPP color state Melissa Wen
` (5 subsequent siblings)
6 siblings, 1 reply; 23+ messages in thread
From: Melissa Wen @ 2023-09-13 16:43 UTC (permalink / raw)
To: Harry Wentland, Rodrigo Siqueira, sunpeng.li, alexander.deucher,
christian.koenig, Xinhui.Pan, airlied, daniel
Cc: Krunoslav Kovac, Shashank Sharma, dri-devel, amd-gfx, kernel-dev,
Nicholas Kazlauskas, sungjoon.kim
Prepare to hook color state logging according to DCN version.
Signed-off-by: Melissa Wen <mwen@igalia.com>
---
.../amd/display/dc/dcn10/dcn10_hw_sequencer.c | 27 +++++++++++++------
1 file changed, 19 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c
index 79befa17bb03..a0c489ed218c 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c
+++ b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c
@@ -279,19 +279,14 @@ static void dcn10_log_hubp_states(struct dc *dc, void *log_ctx)
DTN_INFO("\n");
}
-void dcn10_log_hw_state(struct dc *dc,
- struct dc_log_buffer_ctx *log_ctx)
+static void
+dcn10_log_color_state(struct dc *dc,
+ struct dc_log_buffer_ctx *log_ctx)
{
struct dc_context *dc_ctx = dc->ctx;
struct resource_pool *pool = dc->res_pool;
int i;
- DTN_INFO_BEGIN();
-
- dcn10_log_hubbub_state(dc, log_ctx);
-
- dcn10_log_hubp_states(dc, log_ctx);
-
DTN_INFO("DPP: IGAM format IGAM mode DGAM mode RGAM mode"
" GAMUT mode C11 C12 C13 C14 C21 C22 C23 C24 "
"C31 C32 C33 C34\n");
@@ -348,6 +343,22 @@ void dcn10_log_hw_state(struct dc *dc,
s.idle);
}
DTN_INFO("\n");
+}
+
+void dcn10_log_hw_state(struct dc *dc,
+ struct dc_log_buffer_ctx *log_ctx)
+{
+ struct dc_context *dc_ctx = dc->ctx;
+ struct resource_pool *pool = dc->res_pool;
+ int i;
+
+ DTN_INFO_BEGIN();
+
+ dcn10_log_hubbub_state(dc, log_ctx);
+
+ dcn10_log_hubp_states(dc, log_ctx);
+
+ dcn10_log_color_state(dc, log_ctx);
DTN_INFO("OTG: v_bs v_be v_ss v_se vpol vmax vmin vmax_sel vmin_sel h_bs h_be h_ss h_se hpol htot vtot underflow blank_en\n");
--
2.40.1
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [RFC PATCH v2 2/5] drm/amd/display: fill up DCN3 DPP color state
2023-09-13 16:43 [RFC PATCH v2 0/5] drm/amd/display: improve DTN color state log Melissa Wen
2023-09-13 16:43 ` [RFC PATCH v2 1/5] drm/amd/display: detach color state from hw state logging Melissa Wen
@ 2023-09-13 16:43 ` Melissa Wen
2023-09-13 22:50 ` Rodrigo Siqueira Jordao
2023-09-13 16:43 ` [RFC PATCH v2 3/5] drm/amd/display: create DCN3-specific log for MPC state Melissa Wen
` (4 subsequent siblings)
6 siblings, 1 reply; 23+ messages in thread
From: Melissa Wen @ 2023-09-13 16:43 UTC (permalink / raw)
To: Harry Wentland, Rodrigo Siqueira, sunpeng.li, alexander.deucher,
christian.koenig, Xinhui.Pan, airlied, daniel
Cc: Krunoslav Kovac, Shashank Sharma, dri-devel, amd-gfx, kernel-dev,
Nicholas Kazlauskas, sungjoon.kim
DCN3 DPP color state was uncollected and some state elements from DCN1
doesn't fit DCN3. Create new elements according to DCN3 color caps and
fill them up for DTN log output.
rfc-v2:
- fix reading of gamcor and blnd gamma states
Signed-off-by: Melissa Wen <mwen@igalia.com>
---
.../gpu/drm/amd/display/dc/dcn30/dcn30_dpp.c | 45 +++++++++++++++++--
drivers/gpu/drm/amd/display/dc/inc/hw/dpp.h | 8 ++++
2 files changed, 50 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_dpp.c b/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_dpp.c
index 50dc83404644..a26b33c84ae0 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_dpp.c
+++ b/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_dpp.c
@@ -44,11 +44,50 @@
void dpp30_read_state(struct dpp *dpp_base, struct dcn_dpp_state *s)
{
struct dcn3_dpp *dpp = TO_DCN30_DPP(dpp_base);
+ uint32_t gamcor_lut_mode, rgam_lut_mode;
REG_GET(DPP_CONTROL,
- DPP_CLOCK_ENABLE, &s->is_enabled);
-
- // TODO: Implement for DCN3
+ DPP_CLOCK_ENABLE, &s->is_enabled);
+ // Pre-degamma (ROM)
+ REG_GET_2(PRE_DEGAM,
+ PRE_DEGAM_MODE, &s->pre_dgam_mode,
+ PRE_DEGAM_SELECT, &s->pre_dgam_select);
+ // Gamma Correction (RAM)
+ REG_GET(CM_GAMCOR_CONTROL,
+ CM_GAMCOR_MODE_CURRENT, &s->gamcor_mode);
+ if (s->gamcor_mode) {
+ REG_GET(CM_GAMCOR_CONTROL, CM_GAMCOR_SELECT_CURRENT, &gamcor_lut_mode);
+ if (!gamcor_lut_mode)
+ s->gamcor_mode = LUT_RAM_A; // Otherwise, LUT_RAM_B
+ }
+ // Shaper LUT (RAM), 3D LUT (mode, bit-depth, size)
+ REG_GET(CM_SHAPER_CONTROL,
+ CM_SHAPER_LUT_MODE, &s->shaper_lut_mode);
+ REG_GET(CM_3DLUT_MODE,
+ CM_3DLUT_MODE_CURRENT, &s->lut3d_mode);
+ REG_GET(CM_3DLUT_READ_WRITE_CONTROL,
+ CM_3DLUT_30BIT_EN, &s->lut3d_bit_depth);
+ REG_GET(CM_3DLUT_MODE,
+ CM_3DLUT_SIZE, &s->lut3d_size);
+ // Gamut Remap Matrix (3x4)
+ REG_GET(CM_GAMUT_REMAP_CONTROL,
+ CM_GAMUT_REMAP_MODE, &s->gamut_remap_mode);
+ if (s->gamut_remap_mode) {
+ s->gamut_remap_c11_c12 = REG_READ(CM_GAMUT_REMAP_C11_C12);
+ s->gamut_remap_c13_c14 = REG_READ(CM_GAMUT_REMAP_C13_C14);
+ s->gamut_remap_c21_c22 = REG_READ(CM_GAMUT_REMAP_C21_C22);
+ s->gamut_remap_c23_c24 = REG_READ(CM_GAMUT_REMAP_C23_C24);
+ s->gamut_remap_c31_c32 = REG_READ(CM_GAMUT_REMAP_C31_C32);
+ s->gamut_remap_c33_c34 = REG_READ(CM_GAMUT_REMAP_C33_C34);
+ }
+ // Blend/Out Gamma (RAM)
+ REG_GET(CM_BLNDGAM_CONTROL,
+ CM_BLNDGAM_MODE_CURRENT, &s->rgam_lut_mode);
+ if (s->rgam_lut_mode){
+ REG_GET(CM_BLNDGAM_CONTROL, CM_BLNDGAM_SELECT_CURRENT, &rgam_lut_mode);
+ if (!rgam_lut_mode)
+ s->rgam_lut_mode = LUT_RAM_A; // Otherwise, LUT_RAM_B
+ }
}
/*program post scaler scs block in dpp CM*/
void dpp3_program_post_csc(
diff --git a/drivers/gpu/drm/amd/display/dc/inc/hw/dpp.h b/drivers/gpu/drm/amd/display/dc/inc/hw/dpp.h
index f4aa76e02518..1dfe08dc4364 100644
--- a/drivers/gpu/drm/amd/display/dc/inc/hw/dpp.h
+++ b/drivers/gpu/drm/amd/display/dc/inc/hw/dpp.h
@@ -148,6 +148,14 @@ struct dcn_dpp_state {
uint32_t gamut_remap_c23_c24;
uint32_t gamut_remap_c31_c32;
uint32_t gamut_remap_c33_c34;
+ uint32_t shaper_lut_mode;
+ uint32_t lut3d_mode;
+ uint32_t lut3d_bit_depth;
+ uint32_t lut3d_size;
+ uint32_t blnd_lut_mode;
+ uint32_t pre_dgam_mode;
+ uint32_t pre_dgam_select;
+ uint32_t gamcor_mode;
};
struct CM_bias_params {
--
2.40.1
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [RFC PATCH v2 3/5] drm/amd/display: create DCN3-specific log for MPC state
2023-09-13 16:43 [RFC PATCH v2 0/5] drm/amd/display: improve DTN color state log Melissa Wen
2023-09-13 16:43 ` [RFC PATCH v2 1/5] drm/amd/display: detach color state from hw state logging Melissa Wen
2023-09-13 16:43 ` [RFC PATCH v2 2/5] drm/amd/display: fill up DCN3 DPP color state Melissa Wen
@ 2023-09-13 16:43 ` Melissa Wen
2023-09-25 16:03 ` Harry Wentland
2023-09-13 16:43 ` [RFC PATCH v2 4/5] drm/amd/display: hook DCN30 color state logging to DTN log Melissa Wen
` (3 subsequent siblings)
6 siblings, 1 reply; 23+ messages in thread
From: Melissa Wen @ 2023-09-13 16:43 UTC (permalink / raw)
To: Harry Wentland, Rodrigo Siqueira, sunpeng.li, alexander.deucher,
christian.koenig, Xinhui.Pan, airlied, daniel
Cc: Krunoslav Kovac, Shashank Sharma, dri-devel, amd-gfx, kernel-dev,
Nicholas Kazlauskas, sungjoon.kim
Logging DCN3 MPC state was following DCN1 implementation that doesn't
consider new DCN3 MPC color blocks. Create new elements according to
DCN3 MPC color caps and a new DCN3-specific function for reading MPC
data.
Signed-off-by: Melissa Wen <mwen@igalia.com>
---
.../gpu/drm/amd/display/dc/dcn30/dcn30_mpc.c | 55 ++++++++++++++++++-
drivers/gpu/drm/amd/display/dc/inc/hw/mpc.h | 13 +++++
2 files changed, 67 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_mpc.c b/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_mpc.c
index d1500b223858..d164fbf89212 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_mpc.c
+++ b/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_mpc.c
@@ -1382,8 +1382,61 @@ static void mpc3_set_mpc_mem_lp_mode(struct mpc *mpc)
}
}
+static void mpc3_read_mpcc_state(
+ struct mpc *mpc,
+ int mpcc_inst,
+ struct mpcc_state *s)
+{
+ struct dcn30_mpc *mpc30 = TO_DCN30_MPC(mpc);
+ uint32_t rmu_status = 0xf;
+
+ REG_GET(MPCC_OPP_ID[mpcc_inst], MPCC_OPP_ID, &s->opp_id);
+ REG_GET(MPCC_TOP_SEL[mpcc_inst], MPCC_TOP_SEL, &s->dpp_id);
+ REG_GET(MPCC_BOT_SEL[mpcc_inst], MPCC_BOT_SEL, &s->bot_mpcc_id);
+ REG_GET_4(MPCC_CONTROL[mpcc_inst], MPCC_MODE, &s->mode,
+ MPCC_ALPHA_BLND_MODE, &s->alpha_mode,
+ MPCC_ALPHA_MULTIPLIED_MODE, &s->pre_multiplied_alpha,
+ MPCC_BLND_ACTIVE_OVERLAP_ONLY, &s->overlap_only);
+ REG_GET_2(MPCC_STATUS[mpcc_inst], MPCC_IDLE, &s->idle,
+ MPCC_BUSY, &s->busy);
+
+ /* Color blocks state */
+ REG_GET(MPC_RMU_CONTROL, MPC_RMU0_MUX_STATUS, &rmu_status);
+ if (rmu_status == mpcc_inst) {
+ REG_GET(SHAPER_CONTROL[0],
+ MPC_RMU_SHAPER_LUT_MODE_CURRENT, &s->shaper_lut_mode);
+ REG_GET(RMU_3DLUT_MODE[0],
+ MPC_RMU_3DLUT_MODE_CURRENT, &s->lut3d_mode);
+ REG_GET(RMU_3DLUT_READ_WRITE_CONTROL[0],
+ MPC_RMU_3DLUT_30BIT_EN, &s->lut3d_bit_depth);
+ REG_GET(RMU_3DLUT_MODE[0],
+ MPC_RMU_3DLUT_SIZE, &s->lut3d_size);
+ } else {
+ REG_GET(SHAPER_CONTROL[1],
+ MPC_RMU_SHAPER_LUT_MODE_CURRENT, &s->shaper_lut_mode);
+ REG_GET(RMU_3DLUT_MODE[1],
+ MPC_RMU_3DLUT_MODE_CURRENT, &s->lut3d_mode);
+ REG_GET(RMU_3DLUT_READ_WRITE_CONTROL[1],
+ MPC_RMU_3DLUT_30BIT_EN, &s->lut3d_bit_depth);
+ REG_GET(RMU_3DLUT_MODE[1],
+ MPC_RMU_3DLUT_SIZE, &s->lut3d_size);
+ }
+ REG_GET_2(MPCC_OGAM_CONTROL[mpcc_inst],
+ MPCC_OGAM_MODE_CURRENT, &s->rgam_mode,
+ MPCC_OGAM_SELECT_CURRENT, &s->rgam_lut);
+ REG_GET(MPCC_GAMUT_REMAP_MODE[mpcc_inst],
+ MPCC_GAMUT_REMAP_MODE_CURRENT, &s->gamut_remap_mode);
+ if (s->gamut_remap_mode == 1) {
+ s->gamut_remap_c11_c12 = REG_READ(MPC_GAMUT_REMAP_C11_C12_A[mpcc_inst]);
+ s->gamut_remap_c33_c34 = REG_READ(MPC_GAMUT_REMAP_C33_C34_A[mpcc_inst]);
+ } else if (s->gamut_remap_mode == 2) {
+ s->gamut_remap_c11_c12 = REG_READ(MPC_GAMUT_REMAP_C11_C12_B[mpcc_inst]);
+ s->gamut_remap_c33_c34 = REG_READ(MPC_GAMUT_REMAP_C33_C34_B[mpcc_inst]);
+ }
+}
+
static const struct mpc_funcs dcn30_mpc_funcs = {
- .read_mpcc_state = mpc1_read_mpcc_state,
+ .read_mpcc_state = mpc3_read_mpcc_state,
.insert_plane = mpc1_insert_plane,
.remove_mpcc = mpc1_remove_mpcc,
.mpc_init = mpc1_mpc_init,
diff --git a/drivers/gpu/drm/amd/display/dc/inc/hw/mpc.h b/drivers/gpu/drm/amd/display/dc/inc/hw/mpc.h
index 8d86159d9de0..e60b3503605b 100644
--- a/drivers/gpu/drm/amd/display/dc/inc/hw/mpc.h
+++ b/drivers/gpu/drm/amd/display/dc/inc/hw/mpc.h
@@ -193,6 +193,19 @@ struct mpcc_state {
uint32_t overlap_only;
uint32_t idle;
uint32_t busy;
+ uint32_t shaper_lut_mode;
+ uint32_t lut3d_mode;
+ uint32_t lut3d_bit_depth;
+ uint32_t lut3d_size;
+ uint32_t rgam_mode;
+ uint32_t rgam_lut;
+ uint32_t gamut_remap_mode;
+ uint32_t gamut_remap_c11_c12;
+ uint32_t gamut_remap_c13_c14;
+ uint32_t gamut_remap_c21_c22;
+ uint32_t gamut_remap_c23_c24;
+ uint32_t gamut_remap_c31_c32;
+ uint32_t gamut_remap_c33_c34;
};
/**
--
2.40.1
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [RFC PATCH v2 4/5] drm/amd/display: hook DCN30 color state logging to DTN log
2023-09-13 16:43 [RFC PATCH v2 0/5] drm/amd/display: improve DTN color state log Melissa Wen
` (2 preceding siblings ...)
2023-09-13 16:43 ` [RFC PATCH v2 3/5] drm/amd/display: create DCN3-specific log for MPC state Melissa Wen
@ 2023-09-13 16:43 ` Melissa Wen
2023-09-25 16:03 ` Harry Wentland
2023-09-13 16:43 ` [RFC PATCH v2 5/5] drm/amd/display: add DPP and MPC color caps " Melissa Wen
` (2 subsequent siblings)
6 siblings, 1 reply; 23+ messages in thread
From: Melissa Wen @ 2023-09-13 16:43 UTC (permalink / raw)
To: Harry Wentland, Rodrigo Siqueira, sunpeng.li, alexander.deucher,
christian.koenig, Xinhui.Pan, airlied, daniel
Cc: Krunoslav Kovac, Shashank Sharma, dri-devel, amd-gfx, kernel-dev,
Nicholas Kazlauskas, sungjoon.kim
Color caps changed between HW versions which caused DCN10 color state
sections on DTN log no longer fit DCN3.0 versions. Create a
DCN3.0-specific color state logging and hook it to drivers of DCN3.0
family.
rfc-v2:
- detail RAM mode for gamcor and blnd gamma blocks
Signed-off-by: Melissa Wen <mwen@igalia.com>
---
.../amd/display/dc/dcn10/dcn10_hw_sequencer.c | 5 +-
.../drm/amd/display/dc/dcn30/dcn30_hwseq.c | 88 +++++++++++++++++++
.../drm/amd/display/dc/dcn30/dcn30_hwseq.h | 3 +
.../gpu/drm/amd/display/dc/dcn30/dcn30_init.c | 1 +
.../drm/amd/display/dc/dcn301/dcn301_init.c | 1 +
.../gpu/drm/amd/display/dc/inc/hw_sequencer.h | 2 +
6 files changed, 99 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c
index a0c489ed218c..9255425ef794 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c
+++ b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c
@@ -358,7 +358,10 @@ void dcn10_log_hw_state(struct dc *dc,
dcn10_log_hubp_states(dc, log_ctx);
- dcn10_log_color_state(dc, log_ctx);
+ if (dc->hwss.log_color_state)
+ dc->hwss.log_color_state(dc, log_ctx);
+ else
+ dcn10_log_color_state(dc, log_ctx);
DTN_INFO("OTG: v_bs v_be v_ss v_se vpol vmax vmin vmax_sel vmin_sel h_bs h_be h_ss h_se hpol htot vtot underflow blank_en\n");
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 255713ec29bb..47119ae80e98 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_hwseq.c
+++ b/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_hwseq.c
@@ -69,6 +69,94 @@
#define FN(reg_name, field_name) \
hws->shifts->field_name, hws->masks->field_name
+void
+dcn30_log_color_state(struct dc *dc,
+ struct dc_log_buffer_ctx *log_ctx)
+{
+ struct dc_context *dc_ctx = dc->ctx;
+ struct resource_pool *pool = dc->res_pool;
+ int i;
+
+ DTN_INFO("DPP: DGAM ROM DGAM ROM type DGAM LUT SHAPER mode"
+ " 3DLUT mode 3DLUT bit depth 3DLUT size RGAM mode"
+ " GAMUT mode "
+ "C11 C12 C13 C14 C21 C22 C23 C24 C31 C32 C33 C34\n");
+
+ for (i = 0; i < pool->pipe_count; i++) {
+ struct dpp *dpp = pool->dpps[i];
+ struct dcn_dpp_state s = {0};
+
+ dpp->funcs->dpp_read_state(dpp, &s);
+
+ if (!s.is_enabled)
+ continue;
+
+ DTN_INFO("[%2d]: %7x %13s %8s %11s %10s %15s %10s %9s"
+ " %10x %08xh %08xh %08xh %08xh %08xh %08xh",
+ dpp->inst,
+ s.pre_dgam_mode,
+ (s.pre_dgam_select == 0) ? "sRGB" :
+ ((s.pre_dgam_select == 1) ? "Gamma 2.2" :
+ ((s.pre_dgam_select == 2) ? "Gamma 2.4" :
+ ((s.pre_dgam_select == 3) ? "Gamma 2.6" :
+ ((s.pre_dgam_select == 4) ? "BT.709" :
+ ((s.pre_dgam_select == 5) ? "PQ" :
+ ((s.pre_dgam_select == 6) ? "HLG" :
+ "Unknown")))))),
+ (s.gamcor_mode == 0) ? "Bypass" :
+ ((s.gamcor_mode == 1) ? "RAM A" :
+ "RAM B"),
+ (s.shaper_lut_mode == 1) ? "RAM A" :
+ ((s.shaper_lut_mode == 2) ? "RAM B" :
+ "Bypass"),
+ (s.lut3d_mode == 1) ? "RAM A" :
+ ((s.lut3d_mode == 2) ? "RAM B" :
+ "Bypass"),
+ (s.lut3d_bit_depth <= 0) ? "12-bit" : "10-bit",
+ (s.lut3d_size == 0) ? "17x17x17" : "9x9x9",
+ (s.rgam_lut_mode == 0) ? "Bypass" :
+ ((s.rgam_lut_mode == 1) ? "RAM A" :
+ "RAM B"),
+ s.gamut_remap_mode,
+ s.gamut_remap_c11_c12, s.gamut_remap_c13_c14,
+ s.gamut_remap_c21_c22, s.gamut_remap_c23_c24,
+ s.gamut_remap_c31_c32, s.gamut_remap_c33_c34);
+ DTN_INFO("\n");
+ }
+ DTN_INFO("\n");
+
+ DTN_INFO("MPCC: OPP DPP MPCCBOT MODE ALPHA_MODE PREMULT OVERLAP_ONLY IDLE"
+ " SHAPER mode 3DLUT mode 3DLUT bit-depth 3DLUT size OGAM mode OGAM LUT"
+ " GAMUT mode C11 C12 C33 C34\n");
+
+ for (i = 0; i < pool->pipe_count; i++) {
+ struct mpcc_state s = {0};
+
+ pool->mpc->funcs->read_mpcc_state(pool->mpc, i, &s);
+ if (s.opp_id != 0xf)
+ DTN_INFO("[%2d]: %2xh %2xh %6xh %4d %10d %7d %12d %4d %11s %11s %16s %11s %10s %9s"
+ " %10x %08xh %08xh\n",
+ i, s.opp_id, s.dpp_id, s.bot_mpcc_id,
+ s.mode, s.alpha_mode, s.pre_multiplied_alpha, s.overlap_only,
+ s.idle,
+ (s.shaper_lut_mode == 1) ? "RAM A" :
+ ((s.shaper_lut_mode == 2) ? "RAM B" :
+ "Bypass"),
+ (s.lut3d_mode == 1) ? "RAM A" :
+ ((s.lut3d_mode == 2) ? "RAM B" :
+ "Bypass"),
+ (s.lut3d_bit_depth <= 0) ? "12-bit" : "10-bit",
+ (s.lut3d_size == 0) ? "17x17x17" : "9x9x9",
+ (s.rgam_mode == 0) ? "Bypass" :
+ ((s.rgam_mode == 2) ? "RAM" :
+ "Unknown"),
+ (s.rgam_mode == 1) ? "B" : "A",
+ s.gamut_remap_mode,
+ s.gamut_remap_c11_c12, s.gamut_remap_c33_c34);
+ }
+ DTN_INFO("\n");
+}
+
bool dcn30_set_blend_lut(
struct pipe_ctx *pipe_ctx, const struct dc_plane_state *plane_state)
{
diff --git a/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_hwseq.h b/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_hwseq.h
index ce19c54097f8..cfb3646d6740 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_hwseq.h
+++ b/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_hwseq.h
@@ -52,6 +52,9 @@ bool dcn30_mmhubbub_warmup(
unsigned int num_dwb,
struct dc_writeback_info *wb_info);
+void dcn30_log_color_state(struct dc *dc,
+ struct dc_log_buffer_ctx *log_ctx);
+
bool dcn30_set_blend_lut(struct pipe_ctx *pipe_ctx,
const struct dc_plane_state *plane_state);
diff --git a/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_init.c b/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_init.c
index 0de8b2783cf6..58e4d7e1753b 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_init.c
+++ b/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_init.c
@@ -68,6 +68,7 @@ static const struct hw_sequencer_funcs dcn30_funcs = {
.setup_stereo = dcn10_setup_stereo,
.set_avmute = dcn30_set_avmute,
.log_hw_state = dcn10_log_hw_state,
+ .log_color_state = dcn30_log_color_state,
.get_hw_state = dcn10_get_hw_state,
.clear_status_bits = dcn10_clear_status_bits,
.wait_for_mpcc_disconnect = dcn10_wait_for_mpcc_disconnect,
diff --git a/drivers/gpu/drm/amd/display/dc/dcn301/dcn301_init.c b/drivers/gpu/drm/amd/display/dc/dcn301/dcn301_init.c
index 61205cdbe2d5..227e611f25b8 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn301/dcn301_init.c
+++ b/drivers/gpu/drm/amd/display/dc/dcn301/dcn301_init.c
@@ -72,6 +72,7 @@ static const struct hw_sequencer_funcs dcn301_funcs = {
.setup_stereo = dcn10_setup_stereo,
.set_avmute = dcn30_set_avmute,
.log_hw_state = dcn10_log_hw_state,
+ .log_color_state = dcn30_log_color_state,
.get_hw_state = dcn10_get_hw_state,
.clear_status_bits = dcn10_clear_status_bits,
.wait_for_mpcc_disconnect = dcn10_wait_for_mpcc_disconnect,
diff --git a/drivers/gpu/drm/amd/display/dc/inc/hw_sequencer.h b/drivers/gpu/drm/amd/display/dc/inc/hw_sequencer.h
index e0dd56182841..a480c1092486 100644
--- a/drivers/gpu/drm/amd/display/dc/inc/hw_sequencer.h
+++ b/drivers/gpu/drm/amd/display/dc/inc/hw_sequencer.h
@@ -343,6 +343,8 @@ struct hw_sequencer_funcs {
/* HW State Logging Related */
void (*log_hw_state)(struct dc *dc, struct dc_log_buffer_ctx *log_ctx);
+ void (*log_color_state)(struct dc *dc,
+ struct dc_log_buffer_ctx *log_ctx);
void (*get_hw_state)(struct dc *dc, char *pBuf,
unsigned int bufSize, unsigned int mask);
void (*clear_status_bits)(struct dc *dc, unsigned int mask);
--
2.40.1
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [RFC PATCH v2 5/5] drm/amd/display: add DPP and MPC color caps to DTN log
2023-09-13 16:43 [RFC PATCH v2 0/5] drm/amd/display: improve DTN color state log Melissa Wen
` (3 preceding siblings ...)
2023-09-13 16:43 ` [RFC PATCH v2 4/5] drm/amd/display: hook DCN30 color state logging to DTN log Melissa Wen
@ 2023-09-13 16:43 ` Melissa Wen
2023-09-13 23:02 ` Rodrigo Siqueira Jordao
2023-09-25 16:03 ` Harry Wentland
6 siblings, 0 replies; 23+ messages in thread
From: Melissa Wen @ 2023-09-13 16:43 UTC (permalink / raw)
To: Harry Wentland, Rodrigo Siqueira, sunpeng.li, alexander.deucher,
christian.koenig, Xinhui.Pan, airlied, daniel
Cc: Krunoslav Kovac, Shashank Sharma, dri-devel, amd-gfx, kernel-dev,
Nicholas Kazlauskas, sungjoon.kim
Add color caps information for DPP and MPC block to show HW color caps.
Signed-off-by: Melissa Wen <mwen@igalia.com>
---
.../amd/display/dc/dcn10/dcn10_hw_sequencer.c | 23 +++++++++++++++++++
.../drm/amd/display/dc/dcn30/dcn30_hwseq.c | 23 +++++++++++++++++++
2 files changed, 46 insertions(+)
diff --git a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c
index 9255425ef794..49285f0a146a 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c
+++ b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c
@@ -330,6 +330,24 @@ dcn10_log_color_state(struct dc *dc,
DTN_INFO("\n");
}
DTN_INFO("\n");
+ DTN_INFO("DPP Color Caps: input_lut_shared:%d icsc:%d"
+ " dgam_ram:%d dgam_rom: srgb:%d,bt2020:%d,gamma2_2:%d,pq:%d,hlg:%d"
+ " post_csc:%d gamcor:%d dgam_rom_for_yuv:%d 3d_lut:%d"
+ " blnd_lut:%d oscs:%d\n\n",
+ dc->caps.color.dpp.input_lut_shared,
+ dc->caps.color.dpp.icsc,
+ dc->caps.color.dpp.dgam_ram,
+ dc->caps.color.dpp.dgam_rom_caps.srgb,
+ dc->caps.color.dpp.dgam_rom_caps.bt2020,
+ dc->caps.color.dpp.dgam_rom_caps.gamma2_2,
+ dc->caps.color.dpp.dgam_rom_caps.pq,
+ dc->caps.color.dpp.dgam_rom_caps.hlg,
+ dc->caps.color.dpp.post_csc,
+ dc->caps.color.dpp.gamma_corr,
+ dc->caps.color.dpp.dgam_rom_for_yuv,
+ dc->caps.color.dpp.hw_3d_lut,
+ dc->caps.color.dpp.ogam_ram,
+ dc->caps.color.dpp.ocsc);
DTN_INFO("MPCC: OPP DPP MPCCBOT MODE ALPHA_MODE PREMULT OVERLAP_ONLY IDLE\n");
for (i = 0; i < pool->pipe_count; i++) {
@@ -343,6 +361,11 @@ dcn10_log_color_state(struct dc *dc,
s.idle);
}
DTN_INFO("\n");
+ DTN_INFO("MPC Color Caps: gamut_remap:%d, 3dlut:%d, ogam_ram:%d, ocsc:%d\n\n",
+ dc->caps.color.mpc.gamut_remap,
+ dc->caps.color.mpc.num_3dluts,
+ dc->caps.color.mpc.ogam_ram,
+ dc->caps.color.mpc.ocsc);
}
void dcn10_log_hw_state(struct dc *dc,
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 47119ae80e98..f604c684c853 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_hwseq.c
+++ b/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_hwseq.c
@@ -124,6 +124,24 @@ dcn30_log_color_state(struct dc *dc,
DTN_INFO("\n");
}
DTN_INFO("\n");
+ DTN_INFO("DPP Color Caps: input_lut_shared:%d icsc:%d"
+ " dgam_ram:%d dgam_rom: srgb:%d,bt2020:%d,gamma2_2:%d,pq:%d,hlg:%d"
+ " post_csc:%d gamcor:%d dgam_rom_for_yuv:%d 3d_lut:%d"
+ " blnd_lut:%d oscs:%d\n\n",
+ dc->caps.color.dpp.input_lut_shared,
+ dc->caps.color.dpp.icsc,
+ dc->caps.color.dpp.dgam_ram,
+ dc->caps.color.dpp.dgam_rom_caps.srgb,
+ dc->caps.color.dpp.dgam_rom_caps.bt2020,
+ dc->caps.color.dpp.dgam_rom_caps.gamma2_2,
+ dc->caps.color.dpp.dgam_rom_caps.pq,
+ dc->caps.color.dpp.dgam_rom_caps.hlg,
+ dc->caps.color.dpp.post_csc,
+ dc->caps.color.dpp.gamma_corr,
+ dc->caps.color.dpp.dgam_rom_for_yuv,
+ dc->caps.color.dpp.hw_3d_lut,
+ dc->caps.color.dpp.ogam_ram,
+ dc->caps.color.dpp.ocsc);
DTN_INFO("MPCC: OPP DPP MPCCBOT MODE ALPHA_MODE PREMULT OVERLAP_ONLY IDLE"
" SHAPER mode 3DLUT mode 3DLUT bit-depth 3DLUT size OGAM mode OGAM LUT"
@@ -155,6 +173,11 @@ dcn30_log_color_state(struct dc *dc,
s.gamut_remap_c11_c12, s.gamut_remap_c33_c34);
}
DTN_INFO("\n");
+ DTN_INFO("MPC Color Caps: gamut_remap:%d, 3dlut:%d, ogam_ram:%d, ocsc:%d\n\n",
+ dc->caps.color.mpc.gamut_remap,
+ dc->caps.color.mpc.num_3dluts,
+ dc->caps.color.mpc.ogam_ram,
+ dc->caps.color.mpc.ocsc);
}
bool dcn30_set_blend_lut(
--
2.40.1
^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [RFC PATCH v2 1/5] drm/amd/display: detach color state from hw state logging
2023-09-13 16:43 ` [RFC PATCH v2 1/5] drm/amd/display: detach color state from hw state logging Melissa Wen
@ 2023-09-13 22:41 ` Rodrigo Siqueira Jordao
0 siblings, 0 replies; 23+ messages in thread
From: Rodrigo Siqueira Jordao @ 2023-09-13 22:41 UTC (permalink / raw)
To: Melissa Wen, Harry Wentland, sunpeng.li, alexander.deucher
Cc: Krunoslav Kovac, Shashank Sharma, Xinhui.Pan, amd-gfx,
christian.koenig, dri-devel, kernel-dev, Nicholas Kazlauskas,
sungjoon.kim
On 9/13/23 10:43, Melissa Wen wrote:
> Prepare to hook color state logging according to DCN version.
>
> Signed-off-by: Melissa Wen <mwen@igalia.com>
> ---
> .../amd/display/dc/dcn10/dcn10_hw_sequencer.c | 27 +++++++++++++------
> 1 file changed, 19 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c
> index 79befa17bb03..a0c489ed218c 100644
> --- a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c
> +++ b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c
> @@ -279,19 +279,14 @@ static void dcn10_log_hubp_states(struct dc *dc, void *log_ctx)
> DTN_INFO("\n");
> }
>
> -void dcn10_log_hw_state(struct dc *dc,
> - struct dc_log_buffer_ctx *log_ctx)
> +static void
> +dcn10_log_color_state(struct dc *dc,
> + struct dc_log_buffer_ctx *log_ctx)
nitpick:
put the function definition in a single line.
> {
> struct dc_context *dc_ctx = dc->ctx;
> struct resource_pool *pool = dc->res_pool;
> int i;
>
> - DTN_INFO_BEGIN();
> -
> - dcn10_log_hubbub_state(dc, log_ctx);
> -
> - dcn10_log_hubp_states(dc, log_ctx);
> -
> DTN_INFO("DPP: IGAM format IGAM mode DGAM mode RGAM mode"
> " GAMUT mode C11 C12 C13 C14 C21 C22 C23 C24 "
> "C31 C32 C33 C34\n");
> @@ -348,6 +343,22 @@ void dcn10_log_hw_state(struct dc *dc,
> s.idle);
> }
> DTN_INFO("\n");
> +}
> +
> +void dcn10_log_hw_state(struct dc *dc,
> + struct dc_log_buffer_ctx *log_ctx)
ditto
> +{
> + struct dc_context *dc_ctx = dc->ctx;
> + struct resource_pool *pool = dc->res_pool;
> + int i;
> +
> + DTN_INFO_BEGIN();
> +
> + dcn10_log_hubbub_state(dc, log_ctx);
> +
> + dcn10_log_hubp_states(dc, log_ctx);
> +
> + dcn10_log_color_state(dc, log_ctx);
>
> DTN_INFO("OTG: v_bs v_be v_ss v_se vpol vmax vmin vmax_sel vmin_sel h_bs h_be h_ss h_se hpol htot vtot underflow blank_en\n");
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [RFC PATCH v2 1/5] drm/amd/display: detach color state from hw state logging
@ 2023-09-13 22:41 ` Rodrigo Siqueira Jordao
0 siblings, 0 replies; 23+ messages in thread
From: Rodrigo Siqueira Jordao @ 2023-09-13 22:41 UTC (permalink / raw)
To: Melissa Wen, Harry Wentland, sunpeng.li, alexander.deucher
Cc: Krunoslav Kovac, daniel, Shashank Sharma, Xinhui.Pan, amd-gfx,
christian.koenig, dri-devel, kernel-dev, airlied,
Nicholas Kazlauskas, sungjoon.kim
On 9/13/23 10:43, Melissa Wen wrote:
> Prepare to hook color state logging according to DCN version.
>
> Signed-off-by: Melissa Wen <mwen@igalia.com>
> ---
> .../amd/display/dc/dcn10/dcn10_hw_sequencer.c | 27 +++++++++++++------
> 1 file changed, 19 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c
> index 79befa17bb03..a0c489ed218c 100644
> --- a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c
> +++ b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c
> @@ -279,19 +279,14 @@ static void dcn10_log_hubp_states(struct dc *dc, void *log_ctx)
> DTN_INFO("\n");
> }
>
> -void dcn10_log_hw_state(struct dc *dc,
> - struct dc_log_buffer_ctx *log_ctx)
> +static void
> +dcn10_log_color_state(struct dc *dc,
> + struct dc_log_buffer_ctx *log_ctx)
nitpick:
put the function definition in a single line.
> {
> struct dc_context *dc_ctx = dc->ctx;
> struct resource_pool *pool = dc->res_pool;
> int i;
>
> - DTN_INFO_BEGIN();
> -
> - dcn10_log_hubbub_state(dc, log_ctx);
> -
> - dcn10_log_hubp_states(dc, log_ctx);
> -
> DTN_INFO("DPP: IGAM format IGAM mode DGAM mode RGAM mode"
> " GAMUT mode C11 C12 C13 C14 C21 C22 C23 C24 "
> "C31 C32 C33 C34\n");
> @@ -348,6 +343,22 @@ void dcn10_log_hw_state(struct dc *dc,
> s.idle);
> }
> DTN_INFO("\n");
> +}
> +
> +void dcn10_log_hw_state(struct dc *dc,
> + struct dc_log_buffer_ctx *log_ctx)
ditto
> +{
> + struct dc_context *dc_ctx = dc->ctx;
> + struct resource_pool *pool = dc->res_pool;
> + int i;
> +
> + DTN_INFO_BEGIN();
> +
> + dcn10_log_hubbub_state(dc, log_ctx);
> +
> + dcn10_log_hubp_states(dc, log_ctx);
> +
> + dcn10_log_color_state(dc, log_ctx);
>
> DTN_INFO("OTG: v_bs v_be v_ss v_se vpol vmax vmin vmax_sel vmin_sel h_bs h_be h_ss h_se hpol htot vtot underflow blank_en\n");
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [RFC PATCH v2 2/5] drm/amd/display: fill up DCN3 DPP color state
2023-09-13 16:43 ` [RFC PATCH v2 2/5] drm/amd/display: fill up DCN3 DPP color state Melissa Wen
@ 2023-09-13 22:50 ` Rodrigo Siqueira Jordao
0 siblings, 0 replies; 23+ messages in thread
From: Rodrigo Siqueira Jordao @ 2023-09-13 22:50 UTC (permalink / raw)
To: Melissa Wen, Harry Wentland, sunpeng.li, alexander.deucher, Xinhui.Pan
Cc: Krunoslav Kovac, Shashank Sharma, amd-gfx, christian.koenig,
dri-devel, kernel-dev, Nicholas Kazlauskas, sungjoon.kim
On 9/13/23 10:43, Melissa Wen wrote:
> DCN3 DPP color state was uncollected and some state elements from DCN1
> doesn't fit DCN3. Create new elements according to DCN3 color caps and
> fill them up for DTN log output.
>
> rfc-v2:
> - fix reading of gamcor and blnd gamma states
>
> Signed-off-by: Melissa Wen <mwen@igalia.com>
> ---
> .../gpu/drm/amd/display/dc/dcn30/dcn30_dpp.c | 45 +++++++++++++++++--
> drivers/gpu/drm/amd/display/dc/inc/hw/dpp.h | 8 ++++
> 2 files changed, 50 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_dpp.c b/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_dpp.c
> index 50dc83404644..a26b33c84ae0 100644
> --- a/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_dpp.c
> +++ b/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_dpp.c
> @@ -44,11 +44,50 @@
> void dpp30_read_state(struct dpp *dpp_base, struct dcn_dpp_state *s)
> {
> struct dcn3_dpp *dpp = TO_DCN30_DPP(dpp_base);
> + uint32_t gamcor_lut_mode, rgam_lut_mode;
>
> REG_GET(DPP_CONTROL,
> - DPP_CLOCK_ENABLE, &s->is_enabled);
> -
> - // TODO: Implement for DCN3
> + DPP_CLOCK_ENABLE, &s->is_enabled);
> + // Pre-degamma (ROM)
> + REG_GET_2(PRE_DEGAM,
> + PRE_DEGAM_MODE, &s->pre_dgam_mode,
> + PRE_DEGAM_SELECT, &s->pre_dgam_select);
nitpick:
Add a new line before each comment in this function.
> + // Gamma Correction (RAM)
> + REG_GET(CM_GAMCOR_CONTROL,
> + CM_GAMCOR_MODE_CURRENT, &s->gamcor_mode);
> + if (s->gamcor_mode) {
> + REG_GET(CM_GAMCOR_CONTROL, CM_GAMCOR_SELECT_CURRENT, &gamcor_lut_mode);
> + if (!gamcor_lut_mode)
> + s->gamcor_mode = LUT_RAM_A; // Otherwise, LUT_RAM_B
> + }
> + // Shaper LUT (RAM), 3D LUT (mode, bit-depth, size)
> + REG_GET(CM_SHAPER_CONTROL,
> + CM_SHAPER_LUT_MODE, &s->shaper_lut_mode);
> + REG_GET(CM_3DLUT_MODE,
> + CM_3DLUT_MODE_CURRENT, &s->lut3d_mode);
> + REG_GET(CM_3DLUT_READ_WRITE_CONTROL,
> + CM_3DLUT_30BIT_EN, &s->lut3d_bit_depth);
> + REG_GET(CM_3DLUT_MODE,
> + CM_3DLUT_SIZE, &s->lut3d_size);
> + // Gamut Remap Matrix (3x4)
> + REG_GET(CM_GAMUT_REMAP_CONTROL,
> + CM_GAMUT_REMAP_MODE, &s->gamut_remap_mode);
> + if (s->gamut_remap_mode) {
> + s->gamut_remap_c11_c12 = REG_READ(CM_GAMUT_REMAP_C11_C12);
> + s->gamut_remap_c13_c14 = REG_READ(CM_GAMUT_REMAP_C13_C14);
> + s->gamut_remap_c21_c22 = REG_READ(CM_GAMUT_REMAP_C21_C22);
> + s->gamut_remap_c23_c24 = REG_READ(CM_GAMUT_REMAP_C23_C24);
> + s->gamut_remap_c31_c32 = REG_READ(CM_GAMUT_REMAP_C31_C32);
> + s->gamut_remap_c33_c34 = REG_READ(CM_GAMUT_REMAP_C33_C34);
> + }
> + // Blend/Out Gamma (RAM)
> + REG_GET(CM_BLNDGAM_CONTROL,
> + CM_BLNDGAM_MODE_CURRENT, &s->rgam_lut_mode);
> + if (s->rgam_lut_mode){
> + REG_GET(CM_BLNDGAM_CONTROL, CM_BLNDGAM_SELECT_CURRENT, &rgam_lut_mode);
> + if (!rgam_lut_mode)
> + s->rgam_lut_mode = LUT_RAM_A; // Otherwise, LUT_RAM_B
> + }
> }
> /*program post scaler scs block in dpp CM*/
> void dpp3_program_post_csc(
> diff --git a/drivers/gpu/drm/amd/display/dc/inc/hw/dpp.h b/drivers/gpu/drm/amd/display/dc/inc/hw/dpp.h
> index f4aa76e02518..1dfe08dc4364 100644
> --- a/drivers/gpu/drm/amd/display/dc/inc/hw/dpp.h
> +++ b/drivers/gpu/drm/amd/display/dc/inc/hw/dpp.h
> @@ -148,6 +148,14 @@ struct dcn_dpp_state {
> uint32_t gamut_remap_c23_c24;
> uint32_t gamut_remap_c31_c32;
> uint32_t gamut_remap_c33_c34;
> + uint32_t shaper_lut_mode;
> + uint32_t lut3d_mode;
> + uint32_t lut3d_bit_depth;
> + uint32_t lut3d_size;
> + uint32_t blnd_lut_mode;
> + uint32_t pre_dgam_mode;
> + uint32_t pre_dgam_select;
> + uint32_t gamcor_mode;
> };
>
> struct CM_bias_params {
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [RFC PATCH v2 2/5] drm/amd/display: fill up DCN3 DPP color state
@ 2023-09-13 22:50 ` Rodrigo Siqueira Jordao
0 siblings, 0 replies; 23+ messages in thread
From: Rodrigo Siqueira Jordao @ 2023-09-13 22:50 UTC (permalink / raw)
To: Melissa Wen, Harry Wentland, sunpeng.li, alexander.deucher, Xinhui.Pan
Cc: Krunoslav Kovac, daniel, Shashank Sharma, amd-gfx,
christian.koenig, dri-devel, kernel-dev, airlied,
Nicholas Kazlauskas, sungjoon.kim
On 9/13/23 10:43, Melissa Wen wrote:
> DCN3 DPP color state was uncollected and some state elements from DCN1
> doesn't fit DCN3. Create new elements according to DCN3 color caps and
> fill them up for DTN log output.
>
> rfc-v2:
> - fix reading of gamcor and blnd gamma states
>
> Signed-off-by: Melissa Wen <mwen@igalia.com>
> ---
> .../gpu/drm/amd/display/dc/dcn30/dcn30_dpp.c | 45 +++++++++++++++++--
> drivers/gpu/drm/amd/display/dc/inc/hw/dpp.h | 8 ++++
> 2 files changed, 50 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_dpp.c b/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_dpp.c
> index 50dc83404644..a26b33c84ae0 100644
> --- a/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_dpp.c
> +++ b/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_dpp.c
> @@ -44,11 +44,50 @@
> void dpp30_read_state(struct dpp *dpp_base, struct dcn_dpp_state *s)
> {
> struct dcn3_dpp *dpp = TO_DCN30_DPP(dpp_base);
> + uint32_t gamcor_lut_mode, rgam_lut_mode;
>
> REG_GET(DPP_CONTROL,
> - DPP_CLOCK_ENABLE, &s->is_enabled);
> -
> - // TODO: Implement for DCN3
> + DPP_CLOCK_ENABLE, &s->is_enabled);
> + // Pre-degamma (ROM)
> + REG_GET_2(PRE_DEGAM,
> + PRE_DEGAM_MODE, &s->pre_dgam_mode,
> + PRE_DEGAM_SELECT, &s->pre_dgam_select);
nitpick:
Add a new line before each comment in this function.
> + // Gamma Correction (RAM)
> + REG_GET(CM_GAMCOR_CONTROL,
> + CM_GAMCOR_MODE_CURRENT, &s->gamcor_mode);
> + if (s->gamcor_mode) {
> + REG_GET(CM_GAMCOR_CONTROL, CM_GAMCOR_SELECT_CURRENT, &gamcor_lut_mode);
> + if (!gamcor_lut_mode)
> + s->gamcor_mode = LUT_RAM_A; // Otherwise, LUT_RAM_B
> + }
> + // Shaper LUT (RAM), 3D LUT (mode, bit-depth, size)
> + REG_GET(CM_SHAPER_CONTROL,
> + CM_SHAPER_LUT_MODE, &s->shaper_lut_mode);
> + REG_GET(CM_3DLUT_MODE,
> + CM_3DLUT_MODE_CURRENT, &s->lut3d_mode);
> + REG_GET(CM_3DLUT_READ_WRITE_CONTROL,
> + CM_3DLUT_30BIT_EN, &s->lut3d_bit_depth);
> + REG_GET(CM_3DLUT_MODE,
> + CM_3DLUT_SIZE, &s->lut3d_size);
> + // Gamut Remap Matrix (3x4)
> + REG_GET(CM_GAMUT_REMAP_CONTROL,
> + CM_GAMUT_REMAP_MODE, &s->gamut_remap_mode);
> + if (s->gamut_remap_mode) {
> + s->gamut_remap_c11_c12 = REG_READ(CM_GAMUT_REMAP_C11_C12);
> + s->gamut_remap_c13_c14 = REG_READ(CM_GAMUT_REMAP_C13_C14);
> + s->gamut_remap_c21_c22 = REG_READ(CM_GAMUT_REMAP_C21_C22);
> + s->gamut_remap_c23_c24 = REG_READ(CM_GAMUT_REMAP_C23_C24);
> + s->gamut_remap_c31_c32 = REG_READ(CM_GAMUT_REMAP_C31_C32);
> + s->gamut_remap_c33_c34 = REG_READ(CM_GAMUT_REMAP_C33_C34);
> + }
> + // Blend/Out Gamma (RAM)
> + REG_GET(CM_BLNDGAM_CONTROL,
> + CM_BLNDGAM_MODE_CURRENT, &s->rgam_lut_mode);
> + if (s->rgam_lut_mode){
> + REG_GET(CM_BLNDGAM_CONTROL, CM_BLNDGAM_SELECT_CURRENT, &rgam_lut_mode);
> + if (!rgam_lut_mode)
> + s->rgam_lut_mode = LUT_RAM_A; // Otherwise, LUT_RAM_B
> + }
> }
> /*program post scaler scs block in dpp CM*/
> void dpp3_program_post_csc(
> diff --git a/drivers/gpu/drm/amd/display/dc/inc/hw/dpp.h b/drivers/gpu/drm/amd/display/dc/inc/hw/dpp.h
> index f4aa76e02518..1dfe08dc4364 100644
> --- a/drivers/gpu/drm/amd/display/dc/inc/hw/dpp.h
> +++ b/drivers/gpu/drm/amd/display/dc/inc/hw/dpp.h
> @@ -148,6 +148,14 @@ struct dcn_dpp_state {
> uint32_t gamut_remap_c23_c24;
> uint32_t gamut_remap_c31_c32;
> uint32_t gamut_remap_c33_c34;
> + uint32_t shaper_lut_mode;
> + uint32_t lut3d_mode;
> + uint32_t lut3d_bit_depth;
> + uint32_t lut3d_size;
> + uint32_t blnd_lut_mode;
> + uint32_t pre_dgam_mode;
> + uint32_t pre_dgam_select;
> + uint32_t gamcor_mode;
> };
>
> struct CM_bias_params {
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [RFC PATCH v2 0/5] drm/amd/display: improve DTN color state log
2023-09-13 16:43 [RFC PATCH v2 0/5] drm/amd/display: improve DTN color state log Melissa Wen
@ 2023-09-13 23:02 ` Rodrigo Siqueira Jordao
2023-09-13 16:43 ` [RFC PATCH v2 2/5] drm/amd/display: fill up DCN3 DPP color state Melissa Wen
` (5 subsequent siblings)
6 siblings, 0 replies; 23+ messages in thread
From: Rodrigo Siqueira Jordao @ 2023-09-13 23:02 UTC (permalink / raw)
To: Melissa Wen, Harry Wentland, sunpeng.li, alexander.deucher
Cc: Krunoslav Kovac, Shashank Sharma, Xinhui.Pan, amd-gfx,
christian.koenig, dri-devel, kernel-dev, Nicholas Kazlauskas,
sungjoon.kim
On 9/13/23 10:43, Melissa Wen wrote:
> Hi,
>
> This is an update of previous RFC [0] improving the data collection of
> Gamma Correction and Blend Gamma color blocks.
>
> As I mentioned in the last version, I'm updating the color state part of
> DTN log to match DCN3.0 HW better. Currently, the DTN log considers the
> DCN10 color pipeline, which is useless for DCN3.0 because of all the
> differences in color caps between DCN versions. In addition to new color
> blocks and caps, some semantic differences made the DCN10 output not fit
> DCN30.
>
> In this RFC, the first patch adds new color state elements to DPP and
> implements the reading of registers according to HW blocks. Similarly to
> MPC, the second patch also creates a DCN3-specific function to read the
> MPC state and add the MPC color state logging to it. With DPP and MPC
> color-register reading, I detach DCN10 color state logging from the HW
> log and create a `.log_color_state` hook for logging color state
> according to HW color blocks with DCN30 as the first use case. Finally,
> the last patch adds DPP and MPC color caps output to facilitate
> understanding of the color state log.
>
> This version works well with the driver-specific color properties[1] and
> steamdeck/gamescope[2] together, where we can see color state changing
> from default values.
>
> Here is a before vs. after example:
>
> Without this series:
> ===================
> DPP: IGAM format IGAM mode DGAM mode RGAM mode GAMUT mode C11 C12 C13 C14 C21 C22 C23 C24 C31 C32 C33 C34
> [ 0]: 0h BypassFixed Bypass Bypass 0 00000000h 00000000h 00000000h 00000000h 00000000h 00000000h
> [ 3]: 0h BypassFixed Bypass Bypass 0 00000000h 00000000h 00000000h 00000000h 00000000h 00000000h
>
> MPCC: OPP DPP MPCCBOT MODE ALPHA_MODE PREMULT OVERLAP_ONLY IDLE
> [ 0]: 0h 0h 3h 3 2 0 0 0
> [ 3]: 0h 3h fh 2 2 0 0 0
>
> With this series (Steamdeck/Gamescope):
> ======================================
>
> DPP: DGAM ROM DGAM ROM type DGAM LUT SHAPER mode 3DLUT mode 3DLUT bit depth 3DLUT size RGAM mode GAMUT mode C11 C12 C13 C14 C21 C22 C23 C24 C31 C32 C33 C34
> [ 0]: 1 sRGB Bypass RAM A RAM B 12-bit 17x17x17 RAM A 0 00000000h 00000000h 00000000h 00000000h 00000000h 00000000h
> [ 1]: 1 sRGB Bypass RAM B RAM A 12-bit 17x17x17 RAM A 0 00000000h 00000000h 00000000h 00000000h 00000000h 00000000h
> [ 2]: 1 sRGB Bypass RAM B RAM A 12-bit 17x17x17 RAM A 0 00000000h 00000000h 00000000h 00000000h 00000000h 00000000h
> [ 3]: 1 sRGB Bypass RAM A RAM B 12-bit 17x17x17 RAM A 0 00000000h 00000000h 00000000h 00000000h 00000000h 00000000h
>
> DPP Color Caps: input_lut_shared:0 icsc:1 dgam_ram:0 dgam_rom: srgb:1,bt2020:1,gamma2_2:1,pq:1,hlg:1 post_csc:1 gamcor:1 dgam_rom_for_yuv:0 3d_lut:1 blnd_lut:1 oscs:0
>
> MPCC: OPP DPP MPCCBOT MODE ALPHA_MODE PREMULT OVERLAP_ONLY IDLE SHAPER mode 3DLUT_mode 3DLUT bit-depth 3DLUT size OGAM mode OGAM LUT GAMUT mode C11 C12 C33 C34
> [ 0]: 0h 0h 2h 3 0 1 0 0 Bypass Bypass 12-bit 17x17x17 RAM A 0 00000000h 00000000h
> [ 1]: 0h 1h fh 2 2 0 0 0 Bypass Bypass 12-bit 17x17x17 Bypass A 0 00000000h 00000000h
> [ 2]: 0h 2h 3h 3 0 1 0 0 Bypass Bypass 12-bit 17x17x17 Bypass A 0 00000000h 00000000h
> [ 3]: 0h 3h 1h 3 2 0 0 0 Bypass Bypass 12-bit 17x17x17 Bypass A 0 00000000h 00000000h
>
> MPC Color Caps: gamut_remap:1, 3dlut:2, ogam_ram:1, ocsc:1
I liked this new visualization. At some point, we need to document this
information as a kernel-doc to make it easy to understand this DTN LOG.
> With this series (Steamdeck/KDE):
> ================================
>
> DPP: DGAM ROM DGAM ROM type DGAM LUT SHAPER mode 3DLUT mode 3DLUT bit depth 3DLUT size RGAM mode GAMUT mode C11 C12 C13 C14 C21 C22 C23 C24 C31 C32 C33 C34
> [ 0]: 0 sRGB Bypass Bypass Bypass 12-bit 9x9x9 Bypass 0 00000000h 00000000h 00000000h 00000000h 00000000h 00000000h
> [ 3]: 0 sRGB Bypass Bypass Bypass 12-bit 9x9x9 Bypass 0 00000000h 00000000h 00000000h 00000000h 00000000h 00000000h
>
> DPP Color Caps: input_lut_shared:0 icsc:1 dgam_ram:0 dgam_rom: srgb:1,bt2020:1,gamma2_2:1,pq:1,hlg:1 post_csc:1 gamcor:1 dgam_rom_for_yuv:0 3d_lut:1 blnd_lut:1 oscs:0
>
> MPCC: OPP DPP MPCCBOT MODE ALPHA_MODE PREMULT OVERLAP_ONLY IDLE SHAPER mode 3DLUT_mode 3DLUT bit-depth 3DLUT size OGAM mode OGAM LUT GAMUT mode C11 C12 C33 C34
> [ 0]: 0h 0h 3h 3 2 0 0 0 Bypass Bypass 12-bit 17x17x17 RAM A 1 00002000h 00002000h
> [ 3]: 0h 3h fh 2 2 0 0 0 Bypass Bypass 12-bit 17x17x17 Bypass A 0 00000000h 00000000h
>
> MPC Color Caps: gamut_remap:1, 3dlut:2, ogam_ram:1, ocsc:1
>
> Before extending it to other DCN families, I have some doubts.
> - Does this approach of the `.log_color_state` hook make sense for you?
It does; it follows the code organization. Additionally, this might be
different between ASICs.
> - Is there any conflict between logging color state by HW version and
> DTN log usage?
> - Is there a template/style for DTN log output that I should follow or
> information that I should include?
afaik, we don't have any template or style.
Anyway, this series looks really nice, and I pass through all the
patches. Aside from code style changes, I have nothing else to add.
Thanks
Siqueira
> Let me know your thoughts.
>
> Thanks,
>
> Melissa
>
> [0] https://lore.kernel.org/amd-gfx/20230905142545.451153-1-mwen@igalia.com/
> [1] https://lore.kernel.org/amd-gfx/20230810160314.48225-1-mwen@igalia.com/
> [2] https://github.com/ValveSoftware/gamescope
>
> Melissa Wen (5):
> drm/amd/display: detach color state from hw state logging
> drm/amd/display: fill up DCN3 DPP color state
> drm/amd/display: create DCN3-specific log for MPC state
> drm/amd/display: hook DCN30 color state logging to DTN log
> drm/amd/display: add DPP and MPC color caps to DTN log
>
> .../amd/display/dc/dcn10/dcn10_hw_sequencer.c | 53 +++++++--
> .../gpu/drm/amd/display/dc/dcn30/dcn30_dpp.c | 45 ++++++-
> .../drm/amd/display/dc/dcn30/dcn30_hwseq.c | 111 ++++++++++++++++++
> .../drm/amd/display/dc/dcn30/dcn30_hwseq.h | 3 +
> .../gpu/drm/amd/display/dc/dcn30/dcn30_init.c | 1 +
> .../gpu/drm/amd/display/dc/dcn30/dcn30_mpc.c | 55 ++++++++-
> .../drm/amd/display/dc/dcn301/dcn301_init.c | 1 +
> drivers/gpu/drm/amd/display/dc/inc/hw/dpp.h | 8 ++
> drivers/gpu/drm/amd/display/dc/inc/hw/mpc.h | 13 ++
> .../gpu/drm/amd/display/dc/inc/hw_sequencer.h | 2 +
> 10 files changed, 280 insertions(+), 12 deletions(-)
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [RFC PATCH v2 0/5] drm/amd/display: improve DTN color state log
@ 2023-09-13 23:02 ` Rodrigo Siqueira Jordao
0 siblings, 0 replies; 23+ messages in thread
From: Rodrigo Siqueira Jordao @ 2023-09-13 23:02 UTC (permalink / raw)
To: Melissa Wen, Harry Wentland, sunpeng.li, alexander.deucher
Cc: Krunoslav Kovac, daniel, Shashank Sharma, Xinhui.Pan, amd-gfx,
christian.koenig, dri-devel, kernel-dev, airlied,
Nicholas Kazlauskas, sungjoon.kim
On 9/13/23 10:43, Melissa Wen wrote:
> Hi,
>
> This is an update of previous RFC [0] improving the data collection of
> Gamma Correction and Blend Gamma color blocks.
>
> As I mentioned in the last version, I'm updating the color state part of
> DTN log to match DCN3.0 HW better. Currently, the DTN log considers the
> DCN10 color pipeline, which is useless for DCN3.0 because of all the
> differences in color caps between DCN versions. In addition to new color
> blocks and caps, some semantic differences made the DCN10 output not fit
> DCN30.
>
> In this RFC, the first patch adds new color state elements to DPP and
> implements the reading of registers according to HW blocks. Similarly to
> MPC, the second patch also creates a DCN3-specific function to read the
> MPC state and add the MPC color state logging to it. With DPP and MPC
> color-register reading, I detach DCN10 color state logging from the HW
> log and create a `.log_color_state` hook for logging color state
> according to HW color blocks with DCN30 as the first use case. Finally,
> the last patch adds DPP and MPC color caps output to facilitate
> understanding of the color state log.
>
> This version works well with the driver-specific color properties[1] and
> steamdeck/gamescope[2] together, where we can see color state changing
> from default values.
>
> Here is a before vs. after example:
>
> Without this series:
> ===================
> DPP: IGAM format IGAM mode DGAM mode RGAM mode GAMUT mode C11 C12 C13 C14 C21 C22 C23 C24 C31 C32 C33 C34
> [ 0]: 0h BypassFixed Bypass Bypass 0 00000000h 00000000h 00000000h 00000000h 00000000h 00000000h
> [ 3]: 0h BypassFixed Bypass Bypass 0 00000000h 00000000h 00000000h 00000000h 00000000h 00000000h
>
> MPCC: OPP DPP MPCCBOT MODE ALPHA_MODE PREMULT OVERLAP_ONLY IDLE
> [ 0]: 0h 0h 3h 3 2 0 0 0
> [ 3]: 0h 3h fh 2 2 0 0 0
>
> With this series (Steamdeck/Gamescope):
> ======================================
>
> DPP: DGAM ROM DGAM ROM type DGAM LUT SHAPER mode 3DLUT mode 3DLUT bit depth 3DLUT size RGAM mode GAMUT mode C11 C12 C13 C14 C21 C22 C23 C24 C31 C32 C33 C34
> [ 0]: 1 sRGB Bypass RAM A RAM B 12-bit 17x17x17 RAM A 0 00000000h 00000000h 00000000h 00000000h 00000000h 00000000h
> [ 1]: 1 sRGB Bypass RAM B RAM A 12-bit 17x17x17 RAM A 0 00000000h 00000000h 00000000h 00000000h 00000000h 00000000h
> [ 2]: 1 sRGB Bypass RAM B RAM A 12-bit 17x17x17 RAM A 0 00000000h 00000000h 00000000h 00000000h 00000000h 00000000h
> [ 3]: 1 sRGB Bypass RAM A RAM B 12-bit 17x17x17 RAM A 0 00000000h 00000000h 00000000h 00000000h 00000000h 00000000h
>
> DPP Color Caps: input_lut_shared:0 icsc:1 dgam_ram:0 dgam_rom: srgb:1,bt2020:1,gamma2_2:1,pq:1,hlg:1 post_csc:1 gamcor:1 dgam_rom_for_yuv:0 3d_lut:1 blnd_lut:1 oscs:0
>
> MPCC: OPP DPP MPCCBOT MODE ALPHA_MODE PREMULT OVERLAP_ONLY IDLE SHAPER mode 3DLUT_mode 3DLUT bit-depth 3DLUT size OGAM mode OGAM LUT GAMUT mode C11 C12 C33 C34
> [ 0]: 0h 0h 2h 3 0 1 0 0 Bypass Bypass 12-bit 17x17x17 RAM A 0 00000000h 00000000h
> [ 1]: 0h 1h fh 2 2 0 0 0 Bypass Bypass 12-bit 17x17x17 Bypass A 0 00000000h 00000000h
> [ 2]: 0h 2h 3h 3 0 1 0 0 Bypass Bypass 12-bit 17x17x17 Bypass A 0 00000000h 00000000h
> [ 3]: 0h 3h 1h 3 2 0 0 0 Bypass Bypass 12-bit 17x17x17 Bypass A 0 00000000h 00000000h
>
> MPC Color Caps: gamut_remap:1, 3dlut:2, ogam_ram:1, ocsc:1
I liked this new visualization. At some point, we need to document this
information as a kernel-doc to make it easy to understand this DTN LOG.
> With this series (Steamdeck/KDE):
> ================================
>
> DPP: DGAM ROM DGAM ROM type DGAM LUT SHAPER mode 3DLUT mode 3DLUT bit depth 3DLUT size RGAM mode GAMUT mode C11 C12 C13 C14 C21 C22 C23 C24 C31 C32 C33 C34
> [ 0]: 0 sRGB Bypass Bypass Bypass 12-bit 9x9x9 Bypass 0 00000000h 00000000h 00000000h 00000000h 00000000h 00000000h
> [ 3]: 0 sRGB Bypass Bypass Bypass 12-bit 9x9x9 Bypass 0 00000000h 00000000h 00000000h 00000000h 00000000h 00000000h
>
> DPP Color Caps: input_lut_shared:0 icsc:1 dgam_ram:0 dgam_rom: srgb:1,bt2020:1,gamma2_2:1,pq:1,hlg:1 post_csc:1 gamcor:1 dgam_rom_for_yuv:0 3d_lut:1 blnd_lut:1 oscs:0
>
> MPCC: OPP DPP MPCCBOT MODE ALPHA_MODE PREMULT OVERLAP_ONLY IDLE SHAPER mode 3DLUT_mode 3DLUT bit-depth 3DLUT size OGAM mode OGAM LUT GAMUT mode C11 C12 C33 C34
> [ 0]: 0h 0h 3h 3 2 0 0 0 Bypass Bypass 12-bit 17x17x17 RAM A 1 00002000h 00002000h
> [ 3]: 0h 3h fh 2 2 0 0 0 Bypass Bypass 12-bit 17x17x17 Bypass A 0 00000000h 00000000h
>
> MPC Color Caps: gamut_remap:1, 3dlut:2, ogam_ram:1, ocsc:1
>
> Before extending it to other DCN families, I have some doubts.
> - Does this approach of the `.log_color_state` hook make sense for you?
It does; it follows the code organization. Additionally, this might be
different between ASICs.
> - Is there any conflict between logging color state by HW version and
> DTN log usage?
> - Is there a template/style for DTN log output that I should follow or
> information that I should include?
afaik, we don't have any template or style.
Anyway, this series looks really nice, and I pass through all the
patches. Aside from code style changes, I have nothing else to add.
Thanks
Siqueira
> Let me know your thoughts.
>
> Thanks,
>
> Melissa
>
> [0] https://lore.kernel.org/amd-gfx/20230905142545.451153-1-mwen@igalia.com/
> [1] https://lore.kernel.org/amd-gfx/20230810160314.48225-1-mwen@igalia.com/
> [2] https://github.com/ValveSoftware/gamescope
>
> Melissa Wen (5):
> drm/amd/display: detach color state from hw state logging
> drm/amd/display: fill up DCN3 DPP color state
> drm/amd/display: create DCN3-specific log for MPC state
> drm/amd/display: hook DCN30 color state logging to DTN log
> drm/amd/display: add DPP and MPC color caps to DTN log
>
> .../amd/display/dc/dcn10/dcn10_hw_sequencer.c | 53 +++++++--
> .../gpu/drm/amd/display/dc/dcn30/dcn30_dpp.c | 45 ++++++-
> .../drm/amd/display/dc/dcn30/dcn30_hwseq.c | 111 ++++++++++++++++++
> .../drm/amd/display/dc/dcn30/dcn30_hwseq.h | 3 +
> .../gpu/drm/amd/display/dc/dcn30/dcn30_init.c | 1 +
> .../gpu/drm/amd/display/dc/dcn30/dcn30_mpc.c | 55 ++++++++-
> .../drm/amd/display/dc/dcn301/dcn301_init.c | 1 +
> drivers/gpu/drm/amd/display/dc/inc/hw/dpp.h | 8 ++
> drivers/gpu/drm/amd/display/dc/inc/hw/mpc.h | 13 ++
> .../gpu/drm/amd/display/dc/inc/hw_sequencer.h | 2 +
> 10 files changed, 280 insertions(+), 12 deletions(-)
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [RFC PATCH v2 3/5] drm/amd/display: create DCN3-specific log for MPC state
2023-09-13 16:43 ` [RFC PATCH v2 3/5] drm/amd/display: create DCN3-specific log for MPC state Melissa Wen
@ 2023-09-25 16:03 ` Harry Wentland
2023-09-26 12:38 ` Melissa Wen
0 siblings, 1 reply; 23+ messages in thread
From: Harry Wentland @ 2023-09-25 16:03 UTC (permalink / raw)
To: Melissa Wen, Rodrigo Siqueira, sunpeng.li, alexander.deucher,
christian.koenig, Xinhui.Pan, airlied, daniel
Cc: Krunoslav Kovac, Shashank Sharma, dri-devel, amd-gfx, kernel-dev,
Nicholas Kazlauskas, sungjoon.kim
On 2023-09-13 12:43, Melissa Wen wrote:
> Logging DCN3 MPC state was following DCN1 implementation that doesn't
> consider new DCN3 MPC color blocks. Create new elements according to
> DCN3 MPC color caps and a new DCN3-specific function for reading MPC
> data.
>
> Signed-off-by: Melissa Wen <mwen@igalia.com>
> ---
> .../gpu/drm/amd/display/dc/dcn30/dcn30_mpc.c | 55 ++++++++++++++++++-
> drivers/gpu/drm/amd/display/dc/inc/hw/mpc.h | 13 +++++
> 2 files changed, 67 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_mpc.c b/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_mpc.c
> index d1500b223858..d164fbf89212 100644
> --- a/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_mpc.c
> +++ b/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_mpc.c
> @@ -1382,8 +1382,61 @@ static void mpc3_set_mpc_mem_lp_mode(struct mpc *mpc)
> }
> }
>
> +static void mpc3_read_mpcc_state(
> + struct mpc *mpc,
> + int mpcc_inst,
> + struct mpcc_state *s)
> +{
> + struct dcn30_mpc *mpc30 = TO_DCN30_MPC(mpc);
> + uint32_t rmu_status = 0xf;
> +
> + REG_GET(MPCC_OPP_ID[mpcc_inst], MPCC_OPP_ID, &s->opp_id);
> + REG_GET(MPCC_TOP_SEL[mpcc_inst], MPCC_TOP_SEL, &s->dpp_id);
> + REG_GET(MPCC_BOT_SEL[mpcc_inst], MPCC_BOT_SEL, &s->bot_mpcc_id);
> + REG_GET_4(MPCC_CONTROL[mpcc_inst], MPCC_MODE, &s->mode,
> + MPCC_ALPHA_BLND_MODE, &s->alpha_mode,
> + MPCC_ALPHA_MULTIPLIED_MODE, &s->pre_multiplied_alpha,
> + MPCC_BLND_ACTIVE_OVERLAP_ONLY, &s->overlap_only);
> + REG_GET_2(MPCC_STATUS[mpcc_inst], MPCC_IDLE, &s->idle,
> + MPCC_BUSY, &s->busy);
> +
> + /* Color blocks state */
> + REG_GET(MPC_RMU_CONTROL, MPC_RMU0_MUX_STATUS, &rmu_status);
> + if (rmu_status == mpcc_inst) {
> + REG_GET(SHAPER_CONTROL[0],
> + MPC_RMU_SHAPER_LUT_MODE_CURRENT, &s->shaper_lut_mode);
> + REG_GET(RMU_3DLUT_MODE[0],
> + MPC_RMU_3DLUT_MODE_CURRENT, &s->lut3d_mode);
> + REG_GET(RMU_3DLUT_READ_WRITE_CONTROL[0],
> + MPC_RMU_3DLUT_30BIT_EN, &s->lut3d_bit_depth);
> + REG_GET(RMU_3DLUT_MODE[0],
> + MPC_RMU_3DLUT_SIZE, &s->lut3d_size);
> + } else {
> + REG_GET(SHAPER_CONTROL[1],
> + MPC_RMU_SHAPER_LUT_MODE_CURRENT, &s->shaper_lut_mode);
> + REG_GET(RMU_3DLUT_MODE[1],
> + MPC_RMU_3DLUT_MODE_CURRENT, &s->lut3d_mode);
> + REG_GET(RMU_3DLUT_READ_WRITE_CONTROL[1],
> + MPC_RMU_3DLUT_30BIT_EN, &s->lut3d_bit_depth);
> + REG_GET(RMU_3DLUT_MODE[1],
> + MPC_RMU_3DLUT_SIZE, &s->lut3d_size);
> + }
> + REG_GET_2(MPCC_OGAM_CONTROL[mpcc_inst],
> + MPCC_OGAM_MODE_CURRENT, &s->rgam_mode,
> + MPCC_OGAM_SELECT_CURRENT, &s->rgam_lut);
> + REG_GET(MPCC_GAMUT_REMAP_MODE[mpcc_inst],
> + MPCC_GAMUT_REMAP_MODE_CURRENT, &s->gamut_remap_mode);
> + if (s->gamut_remap_mode == 1) {
> + s->gamut_remap_c11_c12 = REG_READ(MPC_GAMUT_REMAP_C11_C12_A[mpcc_inst]);
> + s->gamut_remap_c33_c34 = REG_READ(MPC_GAMUT_REMAP_C33_C34_A[mpcc_inst]);
> + } else if (s->gamut_remap_mode == 2) {
> + s->gamut_remap_c11_c12 = REG_READ(MPC_GAMUT_REMAP_C11_C12_B[mpcc_inst]);
> + s->gamut_remap_c33_c34 = REG_READ(MPC_GAMUT_REMAP_C33_C34_B[mpcc_inst]);
Any reason we're getting (and printing) only the first and last
coefficients? Is it to avoid printing lines that are too wide?
Harry
> + }
> +}
> +
> static const struct mpc_funcs dcn30_mpc_funcs = {
> - .read_mpcc_state = mpc1_read_mpcc_state,
> + .read_mpcc_state = mpc3_read_mpcc_state,
> .insert_plane = mpc1_insert_plane,
> .remove_mpcc = mpc1_remove_mpcc,
> .mpc_init = mpc1_mpc_init,
> diff --git a/drivers/gpu/drm/amd/display/dc/inc/hw/mpc.h b/drivers/gpu/drm/amd/display/dc/inc/hw/mpc.h
> index 8d86159d9de0..e60b3503605b 100644
> --- a/drivers/gpu/drm/amd/display/dc/inc/hw/mpc.h
> +++ b/drivers/gpu/drm/amd/display/dc/inc/hw/mpc.h
> @@ -193,6 +193,19 @@ struct mpcc_state {
> uint32_t overlap_only;
> uint32_t idle;
> uint32_t busy;
> + uint32_t shaper_lut_mode;
> + uint32_t lut3d_mode;
> + uint32_t lut3d_bit_depth;
> + uint32_t lut3d_size;
> + uint32_t rgam_mode;
> + uint32_t rgam_lut;
> + uint32_t gamut_remap_mode;
> + uint32_t gamut_remap_c11_c12;
> + uint32_t gamut_remap_c13_c14;
> + uint32_t gamut_remap_c21_c22;
> + uint32_t gamut_remap_c23_c24;
> + uint32_t gamut_remap_c31_c32;
> + uint32_t gamut_remap_c33_c34;
> };
>
> /**
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [RFC PATCH v2 4/5] drm/amd/display: hook DCN30 color state logging to DTN log
2023-09-13 16:43 ` [RFC PATCH v2 4/5] drm/amd/display: hook DCN30 color state logging to DTN log Melissa Wen
@ 2023-09-25 16:03 ` Harry Wentland
2023-09-26 12:43 ` Melissa Wen
0 siblings, 1 reply; 23+ messages in thread
From: Harry Wentland @ 2023-09-25 16:03 UTC (permalink / raw)
To: Melissa Wen, Rodrigo Siqueira, sunpeng.li, alexander.deucher,
christian.koenig, Xinhui.Pan, airlied, daniel
Cc: Krunoslav Kovac, Shashank Sharma, dri-devel, amd-gfx, kernel-dev,
Nicholas Kazlauskas, sungjoon.kim
On 2023-09-13 12:43, Melissa Wen wrote:
> Color caps changed between HW versions which caused DCN10 color state
> sections on DTN log no longer fit DCN3.0 versions. Create a
> DCN3.0-specific color state logging and hook it to drivers of DCN3.0
> family.
>
> rfc-v2:
> - detail RAM mode for gamcor and blnd gamma blocks
>
> Signed-off-by: Melissa Wen <mwen@igalia.com>
> ---
> .../amd/display/dc/dcn10/dcn10_hw_sequencer.c | 5 +-
> .../drm/amd/display/dc/dcn30/dcn30_hwseq.c | 88 +++++++++++++++++++
> .../drm/amd/display/dc/dcn30/dcn30_hwseq.h | 3 +
> .../gpu/drm/amd/display/dc/dcn30/dcn30_init.c | 1 +
> .../drm/amd/display/dc/dcn301/dcn301_init.c | 1 +
> .../gpu/drm/amd/display/dc/inc/hw_sequencer.h | 2 +
> 6 files changed, 99 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c
> index a0c489ed218c..9255425ef794 100644
> --- a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c
> +++ b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c
> @@ -358,7 +358,10 @@ void dcn10_log_hw_state(struct dc *dc,
>
> dcn10_log_hubp_states(dc, log_ctx);
>
> - dcn10_log_color_state(dc, log_ctx);
> + if (dc->hwss.log_color_state)
> + dc->hwss.log_color_state(dc, log_ctx);
> + else
> + dcn10_log_color_state(dc, log_ctx);
>
> DTN_INFO("OTG: v_bs v_be v_ss v_se vpol vmax vmin vmax_sel vmin_sel h_bs h_be h_ss h_se hpol htot vtot underflow blank_en\n");
>
> 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 255713ec29bb..47119ae80e98 100644
> --- a/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_hwseq.c
> +++ b/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_hwseq.c
> @@ -69,6 +69,94 @@
> #define FN(reg_name, field_name) \
> hws->shifts->field_name, hws->masks->field_name
>
> +void
> +dcn30_log_color_state(struct dc *dc,
> + struct dc_log_buffer_ctx *log_ctx)
> +{
> + struct dc_context *dc_ctx = dc->ctx;
> + struct resource_pool *pool = dc->res_pool;
> + int i;
> +
> + DTN_INFO("DPP: DGAM ROM DGAM ROM type DGAM LUT SHAPER mode"
> + " 3DLUT mode 3DLUT bit depth 3DLUT size RGAM mode"
> + " GAMUT mode "
> + "C11 C12 C13 C14 C21 C22 C23 C24 C31 C32 C33 C34\n");
> +
> + for (i = 0; i < pool->pipe_count; i++) {
> + struct dpp *dpp = pool->dpps[i];
> + struct dcn_dpp_state s = {0};
> +
> + dpp->funcs->dpp_read_state(dpp, &s);
> +
> + if (!s.is_enabled)
> + continue;
> +
> + DTN_INFO("[%2d]: %7x %13s %8s %11s %10s %15s %10s %9s"
> + " %10x %08xh %08xh %08xh %08xh %08xh %08xh",
> + dpp->inst,
> + s.pre_dgam_mode,
> + (s.pre_dgam_select == 0) ? "sRGB" :
> + ((s.pre_dgam_select == 1) ? "Gamma 2.2" :
> + ((s.pre_dgam_select == 2) ? "Gamma 2.4" :
> + ((s.pre_dgam_select == 3) ? "Gamma 2.6" :
> + ((s.pre_dgam_select == 4) ? "BT.709" :
> + ((s.pre_dgam_select == 5) ? "PQ" :
> + ((s.pre_dgam_select == 6) ? "HLG" :
> + "Unknown")))))),
> + (s.gamcor_mode == 0) ? "Bypass" :
> + ((s.gamcor_mode == 1) ? "RAM A" :
> + "RAM B"),
> + (s.shaper_lut_mode == 1) ? "RAM A" :
> + ((s.shaper_lut_mode == 2) ? "RAM B" :
> + "Bypass"),
> + (s.lut3d_mode == 1) ? "RAM A" :
> + ((s.lut3d_mode == 2) ? "RAM B" :
> + "Bypass"),
> + (s.lut3d_bit_depth <= 0) ? "12-bit" : "10-bit",
> + (s.lut3d_size == 0) ? "17x17x17" : "9x9x9",
> + (s.rgam_lut_mode == 0) ? "Bypass" :
> + ((s.rgam_lut_mode == 1) ? "RAM A" :
> + "RAM B"),
> + s.gamut_remap_mode,
> + s.gamut_remap_c11_c12, s.gamut_remap_c13_c14,
> + s.gamut_remap_c21_c22, s.gamut_remap_c23_c24,
> + s.gamut_remap_c31_c32, s.gamut_remap_c33_c34);
> + DTN_INFO("\n");
> + }
> + DTN_INFO("\n");
> +
> + DTN_INFO("MPCC: OPP DPP MPCCBOT MODE ALPHA_MODE PREMULT OVERLAP_ONLY IDLE"
> + " SHAPER mode 3DLUT mode 3DLUT bit-depth 3DLUT size OGAM mode OGAM LUT"
> + " GAMUT mode C11 C12 C33 C34\n");
> +
> + for (i = 0; i < pool->pipe_count; i++) {
> + struct mpcc_state s = {0};
> +
> + pool->mpc->funcs->read_mpcc_state(pool->mpc, i, &s);
> + if (s.opp_id != 0xf)
> + DTN_INFO("[%2d]: %2xh %2xh %6xh %4d %10d %7d %12d %4d %11s %11s %16s %11s %10s %9s"
> + " %10x %08xh %08xh\n",
> + i, s.opp_id, s.dpp_id, s.bot_mpcc_id,
> + s.mode, s.alpha_mode, s.pre_multiplied_alpha, s.overlap_only,
> + s.idle,
> + (s.shaper_lut_mode == 1) ? "RAM A" :
> + ((s.shaper_lut_mode == 2) ? "RAM B" :
> + "Bypass"),
> + (s.lut3d_mode == 1) ? "RAM A" :
> + ((s.lut3d_mode == 2) ? "RAM B" :
> + "Bypass"),
> + (s.lut3d_bit_depth <= 0) ? "12-bit" : "10-bit",
> + (s.lut3d_size == 0) ? "17x17x17" : "9x9x9",
> + (s.rgam_mode == 0) ? "Bypass" :
> + ((s.rgam_mode == 2) ? "RAM" :
> + "Unknown"),
> + (s.rgam_mode == 1) ? "B" : "A",
> + s.gamut_remap_mode,
> + s.gamut_remap_c11_c12, s.gamut_remap_c33_c34);
> + }
> + DTN_INFO("\n");
> +}
> +
> bool dcn30_set_blend_lut(
> struct pipe_ctx *pipe_ctx, const struct dc_plane_state *plane_state)
> {
> diff --git a/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_hwseq.h b/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_hwseq.h
> index ce19c54097f8..cfb3646d6740 100644
> --- a/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_hwseq.h
> +++ b/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_hwseq.h
> @@ -52,6 +52,9 @@ bool dcn30_mmhubbub_warmup(
> unsigned int num_dwb,
> struct dc_writeback_info *wb_info);
>
> +void dcn30_log_color_state(struct dc *dc,
> + struct dc_log_buffer_ctx *log_ctx);
> +
> bool dcn30_set_blend_lut(struct pipe_ctx *pipe_ctx,
> const struct dc_plane_state *plane_state);
>
> diff --git a/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_init.c b/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_init.c
> index 0de8b2783cf6..58e4d7e1753b 100644
> --- a/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_init.c
> +++ b/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_init.c
> @@ -68,6 +68,7 @@ static const struct hw_sequencer_funcs dcn30_funcs = {
> .setup_stereo = dcn10_setup_stereo,
> .set_avmute = dcn30_set_avmute,
> .log_hw_state = dcn10_log_hw_state,
> + .log_color_state = dcn30_log_color_state,
> .get_hw_state = dcn10_get_hw_state,
> .clear_status_bits = dcn10_clear_status_bits,
> .wait_for_mpcc_disconnect = dcn10_wait_for_mpcc_disconnect,
> diff --git a/drivers/gpu/drm/amd/display/dc/dcn301/dcn301_init.c b/drivers/gpu/drm/amd/display/dc/dcn301/dcn301_init.c
> index 61205cdbe2d5..227e611f25b8 100644
> --- a/drivers/gpu/drm/amd/display/dc/dcn301/dcn301_init.c
> +++ b/drivers/gpu/drm/amd/display/dc/dcn301/dcn301_init.c
> @@ -72,6 +72,7 @@ static const struct hw_sequencer_funcs dcn301_funcs = {
> .setup_stereo = dcn10_setup_stereo,
> .set_avmute = dcn30_set_avmute,
> .log_hw_state = dcn10_log_hw_state,
> + .log_color_state = dcn30_log_color_state,
Can we do this for all dcn3+ versions?
Harry
> .get_hw_state = dcn10_get_hw_state,
> .clear_status_bits = dcn10_clear_status_bits,
> .wait_for_mpcc_disconnect = dcn10_wait_for_mpcc_disconnect,
> diff --git a/drivers/gpu/drm/amd/display/dc/inc/hw_sequencer.h b/drivers/gpu/drm/amd/display/dc/inc/hw_sequencer.h
> index e0dd56182841..a480c1092486 100644
> --- a/drivers/gpu/drm/amd/display/dc/inc/hw_sequencer.h
> +++ b/drivers/gpu/drm/amd/display/dc/inc/hw_sequencer.h
> @@ -343,6 +343,8 @@ struct hw_sequencer_funcs {
>
> /* HW State Logging Related */
> void (*log_hw_state)(struct dc *dc, struct dc_log_buffer_ctx *log_ctx);
> + void (*log_color_state)(struct dc *dc,
> + struct dc_log_buffer_ctx *log_ctx);
> void (*get_hw_state)(struct dc *dc, char *pBuf,
> unsigned int bufSize, unsigned int mask);
> void (*clear_status_bits)(struct dc *dc, unsigned int mask);
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [RFC PATCH v2 0/5] drm/amd/display: improve DTN color state log
2023-09-13 16:43 [RFC PATCH v2 0/5] drm/amd/display: improve DTN color state log Melissa Wen
` (5 preceding siblings ...)
2023-09-13 23:02 ` Rodrigo Siqueira Jordao
@ 2023-09-25 16:03 ` Harry Wentland
2023-09-26 12:48 ` Melissa Wen
6 siblings, 1 reply; 23+ messages in thread
From: Harry Wentland @ 2023-09-25 16:03 UTC (permalink / raw)
To: Melissa Wen, Rodrigo Siqueira, sunpeng.li, airlied,
alexander.deucher, christian.koenig, daniel, Xinhui.Pan
Cc: Krunoslav Kovac, Shashank Sharma, dri-devel, amd-gfx, kernel-dev,
Nicholas Kazlauskas, sungjoon.kim
On 2023-09-13 12:43, Melissa Wen wrote:
> Hi,
>
> This is an update of previous RFC [0] improving the data collection of
> Gamma Correction and Blend Gamma color blocks.
>
> As I mentioned in the last version, I'm updating the color state part of
> DTN log to match DCN3.0 HW better. Currently, the DTN log considers the
> DCN10 color pipeline, which is useless for DCN3.0 because of all the
> differences in color caps between DCN versions. In addition to new color
> blocks and caps, some semantic differences made the DCN10 output not fit
> DCN30.
>
> In this RFC, the first patch adds new color state elements to DPP and
> implements the reading of registers according to HW blocks. Similarly to
> MPC, the second patch also creates a DCN3-specific function to read the
> MPC state and add the MPC color state logging to it. With DPP and MPC
> color-register reading, I detach DCN10 color state logging from the HW
> log and create a `.log_color_state` hook for logging color state
> according to HW color blocks with DCN30 as the first use case. Finally,
> the last patch adds DPP and MPC color caps output to facilitate
> understanding of the color state log.
>
> This version works well with the driver-specific color properties[1] and
> steamdeck/gamescope[2] together, where we can see color state changing
> from default values.
>
> Here is a before vs. after example:
>
> Without this series:
> ===================
> DPP: IGAM format IGAM mode DGAM mode RGAM mode GAMUT mode C11 C12 C13 C14 C21 C22 C23 C24 C31 C32 C33 C34
> [ 0]: 0h BypassFixed Bypass Bypass 0 00000000h 00000000h 00000000h 00000000h 00000000h 00000000h
> [ 3]: 0h BypassFixed Bypass Bypass 0 00000000h 00000000h 00000000h 00000000h 00000000h 00000000h
>
> MPCC: OPP DPP MPCCBOT MODE ALPHA_MODE PREMULT OVERLAP_ONLY IDLE
> [ 0]: 0h 0h 3h 3 2 0 0 0
> [ 3]: 0h 3h fh 2 2 0 0 0
>
> With this series (Steamdeck/Gamescope):
> ======================================
>
> DPP: DGAM ROM DGAM ROM type DGAM LUT SHAPER mode 3DLUT mode 3DLUT bit depth 3DLUT size RGAM mode GAMUT mode C11 C12 C13 C14 C21 C22 C23 C24 C31 C32 C33 C34
> [ 0]: 1 sRGB Bypass RAM A RAM B 12-bit 17x17x17 RAM A 0 00000000h 00000000h 00000000h 00000000h 00000000h 00000000h
> [ 1]: 1 sRGB Bypass RAM B RAM A 12-bit 17x17x17 RAM A 0 00000000h 00000000h 00000000h 00000000h 00000000h 00000000h
> [ 2]: 1 sRGB Bypass RAM B RAM A 12-bit 17x17x17 RAM A 0 00000000h 00000000h 00000000h 00000000h 00000000h 00000000h
> [ 3]: 1 sRGB Bypass RAM A RAM B 12-bit 17x17x17 RAM A 0 00000000h 00000000h 00000000h 00000000h 00000000h 00000000h
>
> DPP Color Caps: input_lut_shared:0 icsc:1 dgam_ram:0 dgam_rom: srgb:1,bt2020:1,gamma2_2:1,pq:1,hlg:1 post_csc:1 gamcor:1 dgam_rom_for_yuv:0 3d_lut:1 blnd_lut:1 oscs:0
>
> MPCC: OPP DPP MPCCBOT MODE ALPHA_MODE PREMULT OVERLAP_ONLY IDLE SHAPER mode 3DLUT_mode 3DLUT bit-depth 3DLUT size OGAM mode OGAM LUT GAMUT mode C11 C12 C33 C34
> [ 0]: 0h 0h 2h 3 0 1 0 0 Bypass Bypass 12-bit 17x17x17 RAM A 0 00000000h 00000000h
> [ 1]: 0h 1h fh 2 2 0 0 0 Bypass Bypass 12-bit 17x17x17 Bypass A 0 00000000h 00000000h
> [ 2]: 0h 2h 3h 3 0 1 0 0 Bypass Bypass 12-bit 17x17x17 Bypass A 0 00000000h 00000000h
> [ 3]: 0h 3h 1h 3 2 0 0 0 Bypass Bypass 12-bit 17x17x17 Bypass A 0 00000000h 00000000h
>
> MPC Color Caps: gamut_remap:1, 3dlut:2, ogam_ram:1, ocsc:1
>
> With this series (Steamdeck/KDE):
> ================================
>
> DPP: DGAM ROM DGAM ROM type DGAM LUT SHAPER mode 3DLUT mode 3DLUT bit depth 3DLUT size RGAM mode GAMUT mode C11 C12 C13 C14 C21 C22 C23 C24 C31 C32 C33 C34
> [ 0]: 0 sRGB Bypass Bypass Bypass 12-bit 9x9x9 Bypass 0 00000000h 00000000h 00000000h 00000000h 00000000h 00000000h
> [ 3]: 0 sRGB Bypass Bypass Bypass 12-bit 9x9x9 Bypass 0 00000000h 00000000h 00000000h 00000000h 00000000h 00000000h
>
> DPP Color Caps: input_lut_shared:0 icsc:1 dgam_ram:0 dgam_rom: srgb:1,bt2020:1,gamma2_2:1,pq:1,hlg:1 post_csc:1 gamcor:1 dgam_rom_for_yuv:0 3d_lut:1 blnd_lut:1 oscs:0
>
> MPCC: OPP DPP MPCCBOT MODE ALPHA_MODE PREMULT OVERLAP_ONLY IDLE SHAPER mode 3DLUT_mode 3DLUT bit-depth 3DLUT size OGAM mode OGAM LUT GAMUT mode C11 C12 C33 C34
> [ 0]: 0h 0h 3h 3 2 0 0 0 Bypass Bypass 12-bit 17x17x17 RAM A 1 00002000h 00002000h
> [ 3]: 0h 3h fh 2 2 0 0 0 Bypass Bypass 12-bit 17x17x17 Bypass A 0 00000000h 00000000h
>
> MPC Color Caps: gamut_remap:1, 3dlut:2, ogam_ram:1, ocsc:1
>
> Before extending it to other DCN families, I have some doubts.
> - Does this approach of the `.log_color_state` hook make sense for you?
Yes
> - Is there any conflict between logging color state by HW version and
> DTN log usage?
> - Is there a template/style for DTN log output that I should follow or
> information that I should include?
>
At this point it looks like we only use the DTN log for debug purposes,
so no conflict and no need to follow a specific format, as long as the
output is human-parseable (which yours is).
At one point in the past these were used by automated tests on other
platforms but that's no longer the case.
Harry
> Let me know your thoughts.
>
> Thanks,
>
> Melissa
>
> [0] https://lore.kernel.org/amd-gfx/20230905142545.451153-1-mwen@igalia.com/
> [1] https://lore.kernel.org/amd-gfx/20230810160314.48225-1-mwen@igalia.com/
> [2] https://github.com/ValveSoftware/gamescope
>
> Melissa Wen (5):
> drm/amd/display: detach color state from hw state logging
> drm/amd/display: fill up DCN3 DPP color state
> drm/amd/display: create DCN3-specific log for MPC state
> drm/amd/display: hook DCN30 color state logging to DTN log
> drm/amd/display: add DPP and MPC color caps to DTN log
>
> .../amd/display/dc/dcn10/dcn10_hw_sequencer.c | 53 +++++++--
> .../gpu/drm/amd/display/dc/dcn30/dcn30_dpp.c | 45 ++++++-
> .../drm/amd/display/dc/dcn30/dcn30_hwseq.c | 111 ++++++++++++++++++
> .../drm/amd/display/dc/dcn30/dcn30_hwseq.h | 3 +
> .../gpu/drm/amd/display/dc/dcn30/dcn30_init.c | 1 +
> .../gpu/drm/amd/display/dc/dcn30/dcn30_mpc.c | 55 ++++++++-
> .../drm/amd/display/dc/dcn301/dcn301_init.c | 1 +
> drivers/gpu/drm/amd/display/dc/inc/hw/dpp.h | 8 ++
> drivers/gpu/drm/amd/display/dc/inc/hw/mpc.h | 13 ++
> .../gpu/drm/amd/display/dc/inc/hw_sequencer.h | 2 +
> 10 files changed, 280 insertions(+), 12 deletions(-)
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [RFC PATCH v2 3/5] drm/amd/display: create DCN3-specific log for MPC state
2023-09-25 16:03 ` Harry Wentland
@ 2023-09-26 12:38 ` Melissa Wen
0 siblings, 0 replies; 23+ messages in thread
From: Melissa Wen @ 2023-09-26 12:38 UTC (permalink / raw)
To: Harry Wentland
Cc: Krunoslav Kovac, kernel-dev, Shashank Sharma, sunpeng.li,
Xinhui.Pan, Rodrigo Siqueira, amd-gfx, Nicholas Kazlauskas,
dri-devel, alexander.deucher, christian.koenig, sungjoon.kim
On 09/25, Harry Wentland wrote:
>
>
> On 2023-09-13 12:43, Melissa Wen wrote:
> > Logging DCN3 MPC state was following DCN1 implementation that doesn't
> > consider new DCN3 MPC color blocks. Create new elements according to
> > DCN3 MPC color caps and a new DCN3-specific function for reading MPC
> > data.
> >
> > Signed-off-by: Melissa Wen <mwen@igalia.com>
> > ---
> > .../gpu/drm/amd/display/dc/dcn30/dcn30_mpc.c | 55 ++++++++++++++++++-
> > drivers/gpu/drm/amd/display/dc/inc/hw/mpc.h | 13 +++++
> > 2 files changed, 67 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_mpc.c b/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_mpc.c
> > index d1500b223858..d164fbf89212 100644
> > --- a/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_mpc.c
> > +++ b/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_mpc.c
> > @@ -1382,8 +1382,61 @@ static void mpc3_set_mpc_mem_lp_mode(struct mpc *mpc)
> > }
> > }
> >
> > +static void mpc3_read_mpcc_state(
> > + struct mpc *mpc,
> > + int mpcc_inst,
> > + struct mpcc_state *s)
> > +{
> > + struct dcn30_mpc *mpc30 = TO_DCN30_MPC(mpc);
> > + uint32_t rmu_status = 0xf;
> > +
> > + REG_GET(MPCC_OPP_ID[mpcc_inst], MPCC_OPP_ID, &s->opp_id);
> > + REG_GET(MPCC_TOP_SEL[mpcc_inst], MPCC_TOP_SEL, &s->dpp_id);
> > + REG_GET(MPCC_BOT_SEL[mpcc_inst], MPCC_BOT_SEL, &s->bot_mpcc_id);
> > + REG_GET_4(MPCC_CONTROL[mpcc_inst], MPCC_MODE, &s->mode,
> > + MPCC_ALPHA_BLND_MODE, &s->alpha_mode,
> > + MPCC_ALPHA_MULTIPLIED_MODE, &s->pre_multiplied_alpha,
> > + MPCC_BLND_ACTIVE_OVERLAP_ONLY, &s->overlap_only);
> > + REG_GET_2(MPCC_STATUS[mpcc_inst], MPCC_IDLE, &s->idle,
> > + MPCC_BUSY, &s->busy);
> > +
> > + /* Color blocks state */
> > + REG_GET(MPC_RMU_CONTROL, MPC_RMU0_MUX_STATUS, &rmu_status);
> > + if (rmu_status == mpcc_inst) {
> > + REG_GET(SHAPER_CONTROL[0],
> > + MPC_RMU_SHAPER_LUT_MODE_CURRENT, &s->shaper_lut_mode);
> > + REG_GET(RMU_3DLUT_MODE[0],
> > + MPC_RMU_3DLUT_MODE_CURRENT, &s->lut3d_mode);
> > + REG_GET(RMU_3DLUT_READ_WRITE_CONTROL[0],
> > + MPC_RMU_3DLUT_30BIT_EN, &s->lut3d_bit_depth);
> > + REG_GET(RMU_3DLUT_MODE[0],
> > + MPC_RMU_3DLUT_SIZE, &s->lut3d_size);
> > + } else {
> > + REG_GET(SHAPER_CONTROL[1],
> > + MPC_RMU_SHAPER_LUT_MODE_CURRENT, &s->shaper_lut_mode);
> > + REG_GET(RMU_3DLUT_MODE[1],
> > + MPC_RMU_3DLUT_MODE_CURRENT, &s->lut3d_mode);
> > + REG_GET(RMU_3DLUT_READ_WRITE_CONTROL[1],
> > + MPC_RMU_3DLUT_30BIT_EN, &s->lut3d_bit_depth);
> > + REG_GET(RMU_3DLUT_MODE[1],
> > + MPC_RMU_3DLUT_SIZE, &s->lut3d_size);
> > + }
> > + REG_GET_2(MPCC_OGAM_CONTROL[mpcc_inst],
> > + MPCC_OGAM_MODE_CURRENT, &s->rgam_mode,
> > + MPCC_OGAM_SELECT_CURRENT, &s->rgam_lut);
> > + REG_GET(MPCC_GAMUT_REMAP_MODE[mpcc_inst],
> > + MPCC_GAMUT_REMAP_MODE_CURRENT, &s->gamut_remap_mode);
> > + if (s->gamut_remap_mode == 1) {
> > + s->gamut_remap_c11_c12 = REG_READ(MPC_GAMUT_REMAP_C11_C12_A[mpcc_inst]);
> > + s->gamut_remap_c33_c34 = REG_READ(MPC_GAMUT_REMAP_C33_C34_A[mpcc_inst]);
> > + } else if (s->gamut_remap_mode == 2) {
> > + s->gamut_remap_c11_c12 = REG_READ(MPC_GAMUT_REMAP_C11_C12_B[mpcc_inst]);
> > + s->gamut_remap_c33_c34 = REG_READ(MPC_GAMUT_REMAP_C33_C34_B[mpcc_inst]);
>
> Any reason we're getting (and printing) only the first and last
> coefficients? Is it to avoid printing lines that are too wide?
I'm unable to reach the other coefficients through this
`MPC_GAMUT_REMAP` register prefix. But I'm probably missing something
since I can see the registers using UMR. I'll try to find the right path
and update it.
Melissa
>
> Harry
>
> > + }
> > +}
> > +
> > static const struct mpc_funcs dcn30_mpc_funcs = {
> > - .read_mpcc_state = mpc1_read_mpcc_state,
> > + .read_mpcc_state = mpc3_read_mpcc_state,
> > .insert_plane = mpc1_insert_plane,
> > .remove_mpcc = mpc1_remove_mpcc,
> > .mpc_init = mpc1_mpc_init,
> > diff --git a/drivers/gpu/drm/amd/display/dc/inc/hw/mpc.h b/drivers/gpu/drm/amd/display/dc/inc/hw/mpc.h
> > index 8d86159d9de0..e60b3503605b 100644
> > --- a/drivers/gpu/drm/amd/display/dc/inc/hw/mpc.h
> > +++ b/drivers/gpu/drm/amd/display/dc/inc/hw/mpc.h
> > @@ -193,6 +193,19 @@ struct mpcc_state {
> > uint32_t overlap_only;
> > uint32_t idle;
> > uint32_t busy;
> > + uint32_t shaper_lut_mode;
> > + uint32_t lut3d_mode;
> > + uint32_t lut3d_bit_depth;
> > + uint32_t lut3d_size;
> > + uint32_t rgam_mode;
> > + uint32_t rgam_lut;
> > + uint32_t gamut_remap_mode;
> > + uint32_t gamut_remap_c11_c12;
> > + uint32_t gamut_remap_c13_c14;
> > + uint32_t gamut_remap_c21_c22;
> > + uint32_t gamut_remap_c23_c24;
> > + uint32_t gamut_remap_c31_c32;
> > + uint32_t gamut_remap_c33_c34;
> > };
> >
> > /**
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [RFC PATCH v2 3/5] drm/amd/display: create DCN3-specific log for MPC state
@ 2023-09-26 12:38 ` Melissa Wen
0 siblings, 0 replies; 23+ messages in thread
From: Melissa Wen @ 2023-09-26 12:38 UTC (permalink / raw)
To: Harry Wentland
Cc: Krunoslav Kovac, kernel-dev, Shashank Sharma, sunpeng.li,
Xinhui.Pan, Rodrigo Siqueira, amd-gfx, Nicholas Kazlauskas,
dri-devel, daniel, alexander.deucher, airlied, christian.koenig,
sungjoon.kim
On 09/25, Harry Wentland wrote:
>
>
> On 2023-09-13 12:43, Melissa Wen wrote:
> > Logging DCN3 MPC state was following DCN1 implementation that doesn't
> > consider new DCN3 MPC color blocks. Create new elements according to
> > DCN3 MPC color caps and a new DCN3-specific function for reading MPC
> > data.
> >
> > Signed-off-by: Melissa Wen <mwen@igalia.com>
> > ---
> > .../gpu/drm/amd/display/dc/dcn30/dcn30_mpc.c | 55 ++++++++++++++++++-
> > drivers/gpu/drm/amd/display/dc/inc/hw/mpc.h | 13 +++++
> > 2 files changed, 67 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_mpc.c b/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_mpc.c
> > index d1500b223858..d164fbf89212 100644
> > --- a/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_mpc.c
> > +++ b/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_mpc.c
> > @@ -1382,8 +1382,61 @@ static void mpc3_set_mpc_mem_lp_mode(struct mpc *mpc)
> > }
> > }
> >
> > +static void mpc3_read_mpcc_state(
> > + struct mpc *mpc,
> > + int mpcc_inst,
> > + struct mpcc_state *s)
> > +{
> > + struct dcn30_mpc *mpc30 = TO_DCN30_MPC(mpc);
> > + uint32_t rmu_status = 0xf;
> > +
> > + REG_GET(MPCC_OPP_ID[mpcc_inst], MPCC_OPP_ID, &s->opp_id);
> > + REG_GET(MPCC_TOP_SEL[mpcc_inst], MPCC_TOP_SEL, &s->dpp_id);
> > + REG_GET(MPCC_BOT_SEL[mpcc_inst], MPCC_BOT_SEL, &s->bot_mpcc_id);
> > + REG_GET_4(MPCC_CONTROL[mpcc_inst], MPCC_MODE, &s->mode,
> > + MPCC_ALPHA_BLND_MODE, &s->alpha_mode,
> > + MPCC_ALPHA_MULTIPLIED_MODE, &s->pre_multiplied_alpha,
> > + MPCC_BLND_ACTIVE_OVERLAP_ONLY, &s->overlap_only);
> > + REG_GET_2(MPCC_STATUS[mpcc_inst], MPCC_IDLE, &s->idle,
> > + MPCC_BUSY, &s->busy);
> > +
> > + /* Color blocks state */
> > + REG_GET(MPC_RMU_CONTROL, MPC_RMU0_MUX_STATUS, &rmu_status);
> > + if (rmu_status == mpcc_inst) {
> > + REG_GET(SHAPER_CONTROL[0],
> > + MPC_RMU_SHAPER_LUT_MODE_CURRENT, &s->shaper_lut_mode);
> > + REG_GET(RMU_3DLUT_MODE[0],
> > + MPC_RMU_3DLUT_MODE_CURRENT, &s->lut3d_mode);
> > + REG_GET(RMU_3DLUT_READ_WRITE_CONTROL[0],
> > + MPC_RMU_3DLUT_30BIT_EN, &s->lut3d_bit_depth);
> > + REG_GET(RMU_3DLUT_MODE[0],
> > + MPC_RMU_3DLUT_SIZE, &s->lut3d_size);
> > + } else {
> > + REG_GET(SHAPER_CONTROL[1],
> > + MPC_RMU_SHAPER_LUT_MODE_CURRENT, &s->shaper_lut_mode);
> > + REG_GET(RMU_3DLUT_MODE[1],
> > + MPC_RMU_3DLUT_MODE_CURRENT, &s->lut3d_mode);
> > + REG_GET(RMU_3DLUT_READ_WRITE_CONTROL[1],
> > + MPC_RMU_3DLUT_30BIT_EN, &s->lut3d_bit_depth);
> > + REG_GET(RMU_3DLUT_MODE[1],
> > + MPC_RMU_3DLUT_SIZE, &s->lut3d_size);
> > + }
> > + REG_GET_2(MPCC_OGAM_CONTROL[mpcc_inst],
> > + MPCC_OGAM_MODE_CURRENT, &s->rgam_mode,
> > + MPCC_OGAM_SELECT_CURRENT, &s->rgam_lut);
> > + REG_GET(MPCC_GAMUT_REMAP_MODE[mpcc_inst],
> > + MPCC_GAMUT_REMAP_MODE_CURRENT, &s->gamut_remap_mode);
> > + if (s->gamut_remap_mode == 1) {
> > + s->gamut_remap_c11_c12 = REG_READ(MPC_GAMUT_REMAP_C11_C12_A[mpcc_inst]);
> > + s->gamut_remap_c33_c34 = REG_READ(MPC_GAMUT_REMAP_C33_C34_A[mpcc_inst]);
> > + } else if (s->gamut_remap_mode == 2) {
> > + s->gamut_remap_c11_c12 = REG_READ(MPC_GAMUT_REMAP_C11_C12_B[mpcc_inst]);
> > + s->gamut_remap_c33_c34 = REG_READ(MPC_GAMUT_REMAP_C33_C34_B[mpcc_inst]);
>
> Any reason we're getting (and printing) only the first and last
> coefficients? Is it to avoid printing lines that are too wide?
I'm unable to reach the other coefficients through this
`MPC_GAMUT_REMAP` register prefix. But I'm probably missing something
since I can see the registers using UMR. I'll try to find the right path
and update it.
Melissa
>
> Harry
>
> > + }
> > +}
> > +
> > static const struct mpc_funcs dcn30_mpc_funcs = {
> > - .read_mpcc_state = mpc1_read_mpcc_state,
> > + .read_mpcc_state = mpc3_read_mpcc_state,
> > .insert_plane = mpc1_insert_plane,
> > .remove_mpcc = mpc1_remove_mpcc,
> > .mpc_init = mpc1_mpc_init,
> > diff --git a/drivers/gpu/drm/amd/display/dc/inc/hw/mpc.h b/drivers/gpu/drm/amd/display/dc/inc/hw/mpc.h
> > index 8d86159d9de0..e60b3503605b 100644
> > --- a/drivers/gpu/drm/amd/display/dc/inc/hw/mpc.h
> > +++ b/drivers/gpu/drm/amd/display/dc/inc/hw/mpc.h
> > @@ -193,6 +193,19 @@ struct mpcc_state {
> > uint32_t overlap_only;
> > uint32_t idle;
> > uint32_t busy;
> > + uint32_t shaper_lut_mode;
> > + uint32_t lut3d_mode;
> > + uint32_t lut3d_bit_depth;
> > + uint32_t lut3d_size;
> > + uint32_t rgam_mode;
> > + uint32_t rgam_lut;
> > + uint32_t gamut_remap_mode;
> > + uint32_t gamut_remap_c11_c12;
> > + uint32_t gamut_remap_c13_c14;
> > + uint32_t gamut_remap_c21_c22;
> > + uint32_t gamut_remap_c23_c24;
> > + uint32_t gamut_remap_c31_c32;
> > + uint32_t gamut_remap_c33_c34;
> > };
> >
> > /**
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [RFC PATCH v2 4/5] drm/amd/display: hook DCN30 color state logging to DTN log
2023-09-25 16:03 ` Harry Wentland
@ 2023-09-26 12:43 ` Melissa Wen
0 siblings, 0 replies; 23+ messages in thread
From: Melissa Wen @ 2023-09-26 12:43 UTC (permalink / raw)
To: Harry Wentland
Cc: Krunoslav Kovac, kernel-dev, Shashank Sharma, sunpeng.li,
Xinhui.Pan, Rodrigo Siqueira, dri-devel, Nicholas Kazlauskas,
amd-gfx, alexander.deucher, christian.koenig, sungjoon.kim
On 09/25, Harry Wentland wrote:
>
>
> On 2023-09-13 12:43, Melissa Wen wrote:
> > Color caps changed between HW versions which caused DCN10 color state
> > sections on DTN log no longer fit DCN3.0 versions. Create a
> > DCN3.0-specific color state logging and hook it to drivers of DCN3.0
> > family.
> >
> > rfc-v2:
> > - detail RAM mode for gamcor and blnd gamma blocks
> >
> > Signed-off-by: Melissa Wen <mwen@igalia.com>
> > ---
> > .../amd/display/dc/dcn10/dcn10_hw_sequencer.c | 5 +-
> > .../drm/amd/display/dc/dcn30/dcn30_hwseq.c | 88 +++++++++++++++++++
> > .../drm/amd/display/dc/dcn30/dcn30_hwseq.h | 3 +
> > .../gpu/drm/amd/display/dc/dcn30/dcn30_init.c | 1 +
> > .../drm/amd/display/dc/dcn301/dcn301_init.c | 1 +
> > .../gpu/drm/amd/display/dc/inc/hw_sequencer.h | 2 +
> > 6 files changed, 99 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c
> > index a0c489ed218c..9255425ef794 100644
> > --- a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c
> > +++ b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c
> > @@ -358,7 +358,10 @@ void dcn10_log_hw_state(struct dc *dc,
> >
> > dcn10_log_hubp_states(dc, log_ctx);
> >
> > - dcn10_log_color_state(dc, log_ctx);
> > + if (dc->hwss.log_color_state)
> > + dc->hwss.log_color_state(dc, log_ctx);
> > + else
> > + dcn10_log_color_state(dc, log_ctx);
> >
> > DTN_INFO("OTG: v_bs v_be v_ss v_se vpol vmax vmin vmax_sel vmin_sel h_bs h_be h_ss h_se hpol htot vtot underflow blank_en\n");
> >
> > 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 255713ec29bb..47119ae80e98 100644
> > --- a/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_hwseq.c
> > +++ b/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_hwseq.c
> > @@ -69,6 +69,94 @@
> > #define FN(reg_name, field_name) \
> > hws->shifts->field_name, hws->masks->field_name
> >
> > +void
> > +dcn30_log_color_state(struct dc *dc,
> > + struct dc_log_buffer_ctx *log_ctx)
> > +{
> > + struct dc_context *dc_ctx = dc->ctx;
> > + struct resource_pool *pool = dc->res_pool;
> > + int i;
> > +
> > + DTN_INFO("DPP: DGAM ROM DGAM ROM type DGAM LUT SHAPER mode"
> > + " 3DLUT mode 3DLUT bit depth 3DLUT size RGAM mode"
> > + " GAMUT mode "
> > + "C11 C12 C13 C14 C21 C22 C23 C24 C31 C32 C33 C34\n");
> > +
> > + for (i = 0; i < pool->pipe_count; i++) {
> > + struct dpp *dpp = pool->dpps[i];
> > + struct dcn_dpp_state s = {0};
> > +
> > + dpp->funcs->dpp_read_state(dpp, &s);
> > +
> > + if (!s.is_enabled)
> > + continue;
> > +
> > + DTN_INFO("[%2d]: %7x %13s %8s %11s %10s %15s %10s %9s"
> > + " %10x %08xh %08xh %08xh %08xh %08xh %08xh",
> > + dpp->inst,
> > + s.pre_dgam_mode,
> > + (s.pre_dgam_select == 0) ? "sRGB" :
> > + ((s.pre_dgam_select == 1) ? "Gamma 2.2" :
> > + ((s.pre_dgam_select == 2) ? "Gamma 2.4" :
> > + ((s.pre_dgam_select == 3) ? "Gamma 2.6" :
> > + ((s.pre_dgam_select == 4) ? "BT.709" :
> > + ((s.pre_dgam_select == 5) ? "PQ" :
> > + ((s.pre_dgam_select == 6) ? "HLG" :
> > + "Unknown")))))),
> > + (s.gamcor_mode == 0) ? "Bypass" :
> > + ((s.gamcor_mode == 1) ? "RAM A" :
> > + "RAM B"),
> > + (s.shaper_lut_mode == 1) ? "RAM A" :
> > + ((s.shaper_lut_mode == 2) ? "RAM B" :
> > + "Bypass"),
> > + (s.lut3d_mode == 1) ? "RAM A" :
> > + ((s.lut3d_mode == 2) ? "RAM B" :
> > + "Bypass"),
> > + (s.lut3d_bit_depth <= 0) ? "12-bit" : "10-bit",
> > + (s.lut3d_size == 0) ? "17x17x17" : "9x9x9",
> > + (s.rgam_lut_mode == 0) ? "Bypass" :
> > + ((s.rgam_lut_mode == 1) ? "RAM A" :
> > + "RAM B"),
> > + s.gamut_remap_mode,
> > + s.gamut_remap_c11_c12, s.gamut_remap_c13_c14,
> > + s.gamut_remap_c21_c22, s.gamut_remap_c23_c24,
> > + s.gamut_remap_c31_c32, s.gamut_remap_c33_c34);
> > + DTN_INFO("\n");
> > + }
> > + DTN_INFO("\n");
> > +
> > + DTN_INFO("MPCC: OPP DPP MPCCBOT MODE ALPHA_MODE PREMULT OVERLAP_ONLY IDLE"
> > + " SHAPER mode 3DLUT mode 3DLUT bit-depth 3DLUT size OGAM mode OGAM LUT"
> > + " GAMUT mode C11 C12 C33 C34\n");
> > +
> > + for (i = 0; i < pool->pipe_count; i++) {
> > + struct mpcc_state s = {0};
> > +
> > + pool->mpc->funcs->read_mpcc_state(pool->mpc, i, &s);
> > + if (s.opp_id != 0xf)
> > + DTN_INFO("[%2d]: %2xh %2xh %6xh %4d %10d %7d %12d %4d %11s %11s %16s %11s %10s %9s"
> > + " %10x %08xh %08xh\n",
> > + i, s.opp_id, s.dpp_id, s.bot_mpcc_id,
> > + s.mode, s.alpha_mode, s.pre_multiplied_alpha, s.overlap_only,
> > + s.idle,
> > + (s.shaper_lut_mode == 1) ? "RAM A" :
> > + ((s.shaper_lut_mode == 2) ? "RAM B" :
> > + "Bypass"),
> > + (s.lut3d_mode == 1) ? "RAM A" :
> > + ((s.lut3d_mode == 2) ? "RAM B" :
> > + "Bypass"),
> > + (s.lut3d_bit_depth <= 0) ? "12-bit" : "10-bit",
> > + (s.lut3d_size == 0) ? "17x17x17" : "9x9x9",
> > + (s.rgam_mode == 0) ? "Bypass" :
> > + ((s.rgam_mode == 2) ? "RAM" :
> > + "Unknown"),
> > + (s.rgam_mode == 1) ? "B" : "A",
> > + s.gamut_remap_mode,
> > + s.gamut_remap_c11_c12, s.gamut_remap_c33_c34);
> > + }
> > + DTN_INFO("\n");
> > +}
> > +
> > bool dcn30_set_blend_lut(
> > struct pipe_ctx *pipe_ctx, const struct dc_plane_state *plane_state)
> > {
> > diff --git a/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_hwseq.h b/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_hwseq.h
> > index ce19c54097f8..cfb3646d6740 100644
> > --- a/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_hwseq.h
> > +++ b/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_hwseq.h
> > @@ -52,6 +52,9 @@ bool dcn30_mmhubbub_warmup(
> > unsigned int num_dwb,
> > struct dc_writeback_info *wb_info);
> >
> > +void dcn30_log_color_state(struct dc *dc,
> > + struct dc_log_buffer_ctx *log_ctx);
> > +
> > bool dcn30_set_blend_lut(struct pipe_ctx *pipe_ctx,
> > const struct dc_plane_state *plane_state);
> >
> > diff --git a/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_init.c b/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_init.c
> > index 0de8b2783cf6..58e4d7e1753b 100644
> > --- a/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_init.c
> > +++ b/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_init.c
> > @@ -68,6 +68,7 @@ static const struct hw_sequencer_funcs dcn30_funcs = {
> > .setup_stereo = dcn10_setup_stereo,
> > .set_avmute = dcn30_set_avmute,
> > .log_hw_state = dcn10_log_hw_state,
> > + .log_color_state = dcn30_log_color_state,
> > .get_hw_state = dcn10_get_hw_state,
> > .clear_status_bits = dcn10_clear_status_bits,
> > .wait_for_mpcc_disconnect = dcn10_wait_for_mpcc_disconnect,
> > diff --git a/drivers/gpu/drm/amd/display/dc/dcn301/dcn301_init.c b/drivers/gpu/drm/amd/display/dc/dcn301/dcn301_init.c
> > index 61205cdbe2d5..227e611f25b8 100644
> > --- a/drivers/gpu/drm/amd/display/dc/dcn301/dcn301_init.c
> > +++ b/drivers/gpu/drm/amd/display/dc/dcn301/dcn301_init.c
> > @@ -72,6 +72,7 @@ static const struct hw_sequencer_funcs dcn301_funcs = {
> > .setup_stereo = dcn10_setup_stereo,
> > .set_avmute = dcn30_set_avmute,
> > .log_hw_state = dcn10_log_hw_state,
> > + .log_color_state = dcn30_log_color_state,
>
> Can we do this for all dcn3+ versions?
Sure, I'll include in the next version. I want to update the color state
log for DCN2+ too, since it also has some specific outputs.
>
> Harry
>
> > .get_hw_state = dcn10_get_hw_state,
> > .clear_status_bits = dcn10_clear_status_bits,
> > .wait_for_mpcc_disconnect = dcn10_wait_for_mpcc_disconnect,
> > diff --git a/drivers/gpu/drm/amd/display/dc/inc/hw_sequencer.h b/drivers/gpu/drm/amd/display/dc/inc/hw_sequencer.h
> > index e0dd56182841..a480c1092486 100644
> > --- a/drivers/gpu/drm/amd/display/dc/inc/hw_sequencer.h
> > +++ b/drivers/gpu/drm/amd/display/dc/inc/hw_sequencer.h
> > @@ -343,6 +343,8 @@ struct hw_sequencer_funcs {
> >
> > /* HW State Logging Related */
> > void (*log_hw_state)(struct dc *dc, struct dc_log_buffer_ctx *log_ctx);
> > + void (*log_color_state)(struct dc *dc,
> > + struct dc_log_buffer_ctx *log_ctx);
> > void (*get_hw_state)(struct dc *dc, char *pBuf,
> > unsigned int bufSize, unsigned int mask);
> > void (*clear_status_bits)(struct dc *dc, unsigned int mask);
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [RFC PATCH v2 4/5] drm/amd/display: hook DCN30 color state logging to DTN log
@ 2023-09-26 12:43 ` Melissa Wen
0 siblings, 0 replies; 23+ messages in thread
From: Melissa Wen @ 2023-09-26 12:43 UTC (permalink / raw)
To: Harry Wentland
Cc: Krunoslav Kovac, kernel-dev, Shashank Sharma, sunpeng.li,
Xinhui.Pan, Rodrigo Siqueira, dri-devel, Nicholas Kazlauskas,
amd-gfx, daniel, alexander.deucher, airlied, christian.koenig,
sungjoon.kim
On 09/25, Harry Wentland wrote:
>
>
> On 2023-09-13 12:43, Melissa Wen wrote:
> > Color caps changed between HW versions which caused DCN10 color state
> > sections on DTN log no longer fit DCN3.0 versions. Create a
> > DCN3.0-specific color state logging and hook it to drivers of DCN3.0
> > family.
> >
> > rfc-v2:
> > - detail RAM mode for gamcor and blnd gamma blocks
> >
> > Signed-off-by: Melissa Wen <mwen@igalia.com>
> > ---
> > .../amd/display/dc/dcn10/dcn10_hw_sequencer.c | 5 +-
> > .../drm/amd/display/dc/dcn30/dcn30_hwseq.c | 88 +++++++++++++++++++
> > .../drm/amd/display/dc/dcn30/dcn30_hwseq.h | 3 +
> > .../gpu/drm/amd/display/dc/dcn30/dcn30_init.c | 1 +
> > .../drm/amd/display/dc/dcn301/dcn301_init.c | 1 +
> > .../gpu/drm/amd/display/dc/inc/hw_sequencer.h | 2 +
> > 6 files changed, 99 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c
> > index a0c489ed218c..9255425ef794 100644
> > --- a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c
> > +++ b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c
> > @@ -358,7 +358,10 @@ void dcn10_log_hw_state(struct dc *dc,
> >
> > dcn10_log_hubp_states(dc, log_ctx);
> >
> > - dcn10_log_color_state(dc, log_ctx);
> > + if (dc->hwss.log_color_state)
> > + dc->hwss.log_color_state(dc, log_ctx);
> > + else
> > + dcn10_log_color_state(dc, log_ctx);
> >
> > DTN_INFO("OTG: v_bs v_be v_ss v_se vpol vmax vmin vmax_sel vmin_sel h_bs h_be h_ss h_se hpol htot vtot underflow blank_en\n");
> >
> > 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 255713ec29bb..47119ae80e98 100644
> > --- a/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_hwseq.c
> > +++ b/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_hwseq.c
> > @@ -69,6 +69,94 @@
> > #define FN(reg_name, field_name) \
> > hws->shifts->field_name, hws->masks->field_name
> >
> > +void
> > +dcn30_log_color_state(struct dc *dc,
> > + struct dc_log_buffer_ctx *log_ctx)
> > +{
> > + struct dc_context *dc_ctx = dc->ctx;
> > + struct resource_pool *pool = dc->res_pool;
> > + int i;
> > +
> > + DTN_INFO("DPP: DGAM ROM DGAM ROM type DGAM LUT SHAPER mode"
> > + " 3DLUT mode 3DLUT bit depth 3DLUT size RGAM mode"
> > + " GAMUT mode "
> > + "C11 C12 C13 C14 C21 C22 C23 C24 C31 C32 C33 C34\n");
> > +
> > + for (i = 0; i < pool->pipe_count; i++) {
> > + struct dpp *dpp = pool->dpps[i];
> > + struct dcn_dpp_state s = {0};
> > +
> > + dpp->funcs->dpp_read_state(dpp, &s);
> > +
> > + if (!s.is_enabled)
> > + continue;
> > +
> > + DTN_INFO("[%2d]: %7x %13s %8s %11s %10s %15s %10s %9s"
> > + " %10x %08xh %08xh %08xh %08xh %08xh %08xh",
> > + dpp->inst,
> > + s.pre_dgam_mode,
> > + (s.pre_dgam_select == 0) ? "sRGB" :
> > + ((s.pre_dgam_select == 1) ? "Gamma 2.2" :
> > + ((s.pre_dgam_select == 2) ? "Gamma 2.4" :
> > + ((s.pre_dgam_select == 3) ? "Gamma 2.6" :
> > + ((s.pre_dgam_select == 4) ? "BT.709" :
> > + ((s.pre_dgam_select == 5) ? "PQ" :
> > + ((s.pre_dgam_select == 6) ? "HLG" :
> > + "Unknown")))))),
> > + (s.gamcor_mode == 0) ? "Bypass" :
> > + ((s.gamcor_mode == 1) ? "RAM A" :
> > + "RAM B"),
> > + (s.shaper_lut_mode == 1) ? "RAM A" :
> > + ((s.shaper_lut_mode == 2) ? "RAM B" :
> > + "Bypass"),
> > + (s.lut3d_mode == 1) ? "RAM A" :
> > + ((s.lut3d_mode == 2) ? "RAM B" :
> > + "Bypass"),
> > + (s.lut3d_bit_depth <= 0) ? "12-bit" : "10-bit",
> > + (s.lut3d_size == 0) ? "17x17x17" : "9x9x9",
> > + (s.rgam_lut_mode == 0) ? "Bypass" :
> > + ((s.rgam_lut_mode == 1) ? "RAM A" :
> > + "RAM B"),
> > + s.gamut_remap_mode,
> > + s.gamut_remap_c11_c12, s.gamut_remap_c13_c14,
> > + s.gamut_remap_c21_c22, s.gamut_remap_c23_c24,
> > + s.gamut_remap_c31_c32, s.gamut_remap_c33_c34);
> > + DTN_INFO("\n");
> > + }
> > + DTN_INFO("\n");
> > +
> > + DTN_INFO("MPCC: OPP DPP MPCCBOT MODE ALPHA_MODE PREMULT OVERLAP_ONLY IDLE"
> > + " SHAPER mode 3DLUT mode 3DLUT bit-depth 3DLUT size OGAM mode OGAM LUT"
> > + " GAMUT mode C11 C12 C33 C34\n");
> > +
> > + for (i = 0; i < pool->pipe_count; i++) {
> > + struct mpcc_state s = {0};
> > +
> > + pool->mpc->funcs->read_mpcc_state(pool->mpc, i, &s);
> > + if (s.opp_id != 0xf)
> > + DTN_INFO("[%2d]: %2xh %2xh %6xh %4d %10d %7d %12d %4d %11s %11s %16s %11s %10s %9s"
> > + " %10x %08xh %08xh\n",
> > + i, s.opp_id, s.dpp_id, s.bot_mpcc_id,
> > + s.mode, s.alpha_mode, s.pre_multiplied_alpha, s.overlap_only,
> > + s.idle,
> > + (s.shaper_lut_mode == 1) ? "RAM A" :
> > + ((s.shaper_lut_mode == 2) ? "RAM B" :
> > + "Bypass"),
> > + (s.lut3d_mode == 1) ? "RAM A" :
> > + ((s.lut3d_mode == 2) ? "RAM B" :
> > + "Bypass"),
> > + (s.lut3d_bit_depth <= 0) ? "12-bit" : "10-bit",
> > + (s.lut3d_size == 0) ? "17x17x17" : "9x9x9",
> > + (s.rgam_mode == 0) ? "Bypass" :
> > + ((s.rgam_mode == 2) ? "RAM" :
> > + "Unknown"),
> > + (s.rgam_mode == 1) ? "B" : "A",
> > + s.gamut_remap_mode,
> > + s.gamut_remap_c11_c12, s.gamut_remap_c33_c34);
> > + }
> > + DTN_INFO("\n");
> > +}
> > +
> > bool dcn30_set_blend_lut(
> > struct pipe_ctx *pipe_ctx, const struct dc_plane_state *plane_state)
> > {
> > diff --git a/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_hwseq.h b/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_hwseq.h
> > index ce19c54097f8..cfb3646d6740 100644
> > --- a/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_hwseq.h
> > +++ b/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_hwseq.h
> > @@ -52,6 +52,9 @@ bool dcn30_mmhubbub_warmup(
> > unsigned int num_dwb,
> > struct dc_writeback_info *wb_info);
> >
> > +void dcn30_log_color_state(struct dc *dc,
> > + struct dc_log_buffer_ctx *log_ctx);
> > +
> > bool dcn30_set_blend_lut(struct pipe_ctx *pipe_ctx,
> > const struct dc_plane_state *plane_state);
> >
> > diff --git a/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_init.c b/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_init.c
> > index 0de8b2783cf6..58e4d7e1753b 100644
> > --- a/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_init.c
> > +++ b/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_init.c
> > @@ -68,6 +68,7 @@ static const struct hw_sequencer_funcs dcn30_funcs = {
> > .setup_stereo = dcn10_setup_stereo,
> > .set_avmute = dcn30_set_avmute,
> > .log_hw_state = dcn10_log_hw_state,
> > + .log_color_state = dcn30_log_color_state,
> > .get_hw_state = dcn10_get_hw_state,
> > .clear_status_bits = dcn10_clear_status_bits,
> > .wait_for_mpcc_disconnect = dcn10_wait_for_mpcc_disconnect,
> > diff --git a/drivers/gpu/drm/amd/display/dc/dcn301/dcn301_init.c b/drivers/gpu/drm/amd/display/dc/dcn301/dcn301_init.c
> > index 61205cdbe2d5..227e611f25b8 100644
> > --- a/drivers/gpu/drm/amd/display/dc/dcn301/dcn301_init.c
> > +++ b/drivers/gpu/drm/amd/display/dc/dcn301/dcn301_init.c
> > @@ -72,6 +72,7 @@ static const struct hw_sequencer_funcs dcn301_funcs = {
> > .setup_stereo = dcn10_setup_stereo,
> > .set_avmute = dcn30_set_avmute,
> > .log_hw_state = dcn10_log_hw_state,
> > + .log_color_state = dcn30_log_color_state,
>
> Can we do this for all dcn3+ versions?
Sure, I'll include in the next version. I want to update the color state
log for DCN2+ too, since it also has some specific outputs.
>
> Harry
>
> > .get_hw_state = dcn10_get_hw_state,
> > .clear_status_bits = dcn10_clear_status_bits,
> > .wait_for_mpcc_disconnect = dcn10_wait_for_mpcc_disconnect,
> > diff --git a/drivers/gpu/drm/amd/display/dc/inc/hw_sequencer.h b/drivers/gpu/drm/amd/display/dc/inc/hw_sequencer.h
> > index e0dd56182841..a480c1092486 100644
> > --- a/drivers/gpu/drm/amd/display/dc/inc/hw_sequencer.h
> > +++ b/drivers/gpu/drm/amd/display/dc/inc/hw_sequencer.h
> > @@ -343,6 +343,8 @@ struct hw_sequencer_funcs {
> >
> > /* HW State Logging Related */
> > void (*log_hw_state)(struct dc *dc, struct dc_log_buffer_ctx *log_ctx);
> > + void (*log_color_state)(struct dc *dc,
> > + struct dc_log_buffer_ctx *log_ctx);
> > void (*get_hw_state)(struct dc *dc, char *pBuf,
> > unsigned int bufSize, unsigned int mask);
> > void (*clear_status_bits)(struct dc *dc, unsigned int mask);
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [RFC PATCH v2 0/5] drm/amd/display: improve DTN color state log
2023-09-25 16:03 ` Harry Wentland
@ 2023-09-26 12:48 ` Melissa Wen
0 siblings, 0 replies; 23+ messages in thread
From: Melissa Wen @ 2023-09-26 12:48 UTC (permalink / raw)
To: Harry Wentland
Cc: Krunoslav Kovac, kernel-dev, Shashank Sharma, sunpeng.li,
Xinhui.Pan, Rodrigo Siqueira, dri-devel, Nicholas Kazlauskas,
amd-gfx, daniel, alexander.deucher, airlied, christian.koenig,
sungjoon.kim
On 09/25, Harry Wentland wrote:
>
>
> On 2023-09-13 12:43, Melissa Wen wrote:
> > Hi,
> >
> > This is an update of previous RFC [0] improving the data collection of
> > Gamma Correction and Blend Gamma color blocks.
> >
> > As I mentioned in the last version, I'm updating the color state part of
> > DTN log to match DCN3.0 HW better. Currently, the DTN log considers the
> > DCN10 color pipeline, which is useless for DCN3.0 because of all the
> > differences in color caps between DCN versions. In addition to new color
> > blocks and caps, some semantic differences made the DCN10 output not fit
> > DCN30.
> >
> > In this RFC, the first patch adds new color state elements to DPP and
> > implements the reading of registers according to HW blocks. Similarly to
> > MPC, the second patch also creates a DCN3-specific function to read the
> > MPC state and add the MPC color state logging to it. With DPP and MPC
> > color-register reading, I detach DCN10 color state logging from the HW
> > log and create a `.log_color_state` hook for logging color state
> > according to HW color blocks with DCN30 as the first use case. Finally,
> > the last patch adds DPP and MPC color caps output to facilitate
> > understanding of the color state log.
> >
> > This version works well with the driver-specific color properties[1] and
> > steamdeck/gamescope[2] together, where we can see color state changing
> > from default values.
> >
> > Here is a before vs. after example:
> >
> > Without this series:
> > ===================
> > DPP: IGAM format IGAM mode DGAM mode RGAM mode GAMUT mode C11 C12 C13 C14 C21 C22 C23 C24 C31 C32 C33 C34
> > [ 0]: 0h BypassFixed Bypass Bypass 0 00000000h 00000000h 00000000h 00000000h 00000000h 00000000h
> > [ 3]: 0h BypassFixed Bypass Bypass 0 00000000h 00000000h 00000000h 00000000h 00000000h 00000000h
> >
> > MPCC: OPP DPP MPCCBOT MODE ALPHA_MODE PREMULT OVERLAP_ONLY IDLE
> > [ 0]: 0h 0h 3h 3 2 0 0 0
> > [ 3]: 0h 3h fh 2 2 0 0 0
> >
> > With this series (Steamdeck/Gamescope):
> > ======================================
> >
> > DPP: DGAM ROM DGAM ROM type DGAM LUT SHAPER mode 3DLUT mode 3DLUT bit depth 3DLUT size RGAM mode GAMUT mode C11 C12 C13 C14 C21 C22 C23 C24 C31 C32 C33 C34
> > [ 0]: 1 sRGB Bypass RAM A RAM B 12-bit 17x17x17 RAM A 0 00000000h 00000000h 00000000h 00000000h 00000000h 00000000h
> > [ 1]: 1 sRGB Bypass RAM B RAM A 12-bit 17x17x17 RAM A 0 00000000h 00000000h 00000000h 00000000h 00000000h 00000000h
> > [ 2]: 1 sRGB Bypass RAM B RAM A 12-bit 17x17x17 RAM A 0 00000000h 00000000h 00000000h 00000000h 00000000h 00000000h
> > [ 3]: 1 sRGB Bypass RAM A RAM B 12-bit 17x17x17 RAM A 0 00000000h 00000000h 00000000h 00000000h 00000000h 00000000h
> >
> > DPP Color Caps: input_lut_shared:0 icsc:1 dgam_ram:0 dgam_rom: srgb:1,bt2020:1,gamma2_2:1,pq:1,hlg:1 post_csc:1 gamcor:1 dgam_rom_for_yuv:0 3d_lut:1 blnd_lut:1 oscs:0
> >
> > MPCC: OPP DPP MPCCBOT MODE ALPHA_MODE PREMULT OVERLAP_ONLY IDLE SHAPER mode 3DLUT_mode 3DLUT bit-depth 3DLUT size OGAM mode OGAM LUT GAMUT mode C11 C12 C33 C34
> > [ 0]: 0h 0h 2h 3 0 1 0 0 Bypass Bypass 12-bit 17x17x17 RAM A 0 00000000h 00000000h
> > [ 1]: 0h 1h fh 2 2 0 0 0 Bypass Bypass 12-bit 17x17x17 Bypass A 0 00000000h 00000000h
> > [ 2]: 0h 2h 3h 3 0 1 0 0 Bypass Bypass 12-bit 17x17x17 Bypass A 0 00000000h 00000000h
> > [ 3]: 0h 3h 1h 3 2 0 0 0 Bypass Bypass 12-bit 17x17x17 Bypass A 0 00000000h 00000000h
> >
> > MPC Color Caps: gamut_remap:1, 3dlut:2, ogam_ram:1, ocsc:1
> >
> > With this series (Steamdeck/KDE):
> > ================================
> >
> > DPP: DGAM ROM DGAM ROM type DGAM LUT SHAPER mode 3DLUT mode 3DLUT bit depth 3DLUT size RGAM mode GAMUT mode C11 C12 C13 C14 C21 C22 C23 C24 C31 C32 C33 C34
> > [ 0]: 0 sRGB Bypass Bypass Bypass 12-bit 9x9x9 Bypass 0 00000000h 00000000h 00000000h 00000000h 00000000h 00000000h
> > [ 3]: 0 sRGB Bypass Bypass Bypass 12-bit 9x9x9 Bypass 0 00000000h 00000000h 00000000h 00000000h 00000000h 00000000h
> >
> > DPP Color Caps: input_lut_shared:0 icsc:1 dgam_ram:0 dgam_rom: srgb:1,bt2020:1,gamma2_2:1,pq:1,hlg:1 post_csc:1 gamcor:1 dgam_rom_for_yuv:0 3d_lut:1 blnd_lut:1 oscs:0
> >
> > MPCC: OPP DPP MPCCBOT MODE ALPHA_MODE PREMULT OVERLAP_ONLY IDLE SHAPER mode 3DLUT_mode 3DLUT bit-depth 3DLUT size OGAM mode OGAM LUT GAMUT mode C11 C12 C33 C34
> > [ 0]: 0h 0h 3h 3 2 0 0 0 Bypass Bypass 12-bit 17x17x17 RAM A 1 00002000h 00002000h
> > [ 3]: 0h 3h fh 2 2 0 0 0 Bypass Bypass 12-bit 17x17x17 Bypass A 0 00000000h 00000000h
> >
> > MPC Color Caps: gamut_remap:1, 3dlut:2, ogam_ram:1, ocsc:1
> >
> > Before extending it to other DCN families, I have some doubts.
> > - Does this approach of the `.log_color_state` hook make sense for you?
>
> Yes
>
> > - Is there any conflict between logging color state by HW version and
> > DTN log usage?
> > - Is there a template/style for DTN log output that I should follow or
> > information that I should include?
> >
>
> At this point it looks like we only use the DTN log for debug purposes,
> so no conflict and no need to follow a specific format, as long as the
> output is human-parseable (which yours is).
>
> At one point in the past these were used by automated tests on other
> platforms but that's no longer the case.
Great! I'll prepare a next version from your suggestions and also
addressing Siqueira's review.
Thanks for the guidance.
Melissa
>
> Harry
>
> > Let me know your thoughts.
> >
> > Thanks,
> >
> > Melissa
> >
> > [0] https://lore.kernel.org/amd-gfx/20230905142545.451153-1-mwen@igalia.com/
> > [1] https://lore.kernel.org/amd-gfx/20230810160314.48225-1-mwen@igalia.com/
> > [2] https://github.com/ValveSoftware/gamescope
> >
> > Melissa Wen (5):
> > drm/amd/display: detach color state from hw state logging
> > drm/amd/display: fill up DCN3 DPP color state
> > drm/amd/display: create DCN3-specific log for MPC state
> > drm/amd/display: hook DCN30 color state logging to DTN log
> > drm/amd/display: add DPP and MPC color caps to DTN log
> >
> > .../amd/display/dc/dcn10/dcn10_hw_sequencer.c | 53 +++++++--
> > .../gpu/drm/amd/display/dc/dcn30/dcn30_dpp.c | 45 ++++++-
> > .../drm/amd/display/dc/dcn30/dcn30_hwseq.c | 111 ++++++++++++++++++
> > .../drm/amd/display/dc/dcn30/dcn30_hwseq.h | 3 +
> > .../gpu/drm/amd/display/dc/dcn30/dcn30_init.c | 1 +
> > .../gpu/drm/amd/display/dc/dcn30/dcn30_mpc.c | 55 ++++++++-
> > .../drm/amd/display/dc/dcn301/dcn301_init.c | 1 +
> > drivers/gpu/drm/amd/display/dc/inc/hw/dpp.h | 8 ++
> > drivers/gpu/drm/amd/display/dc/inc/hw/mpc.h | 13 ++
> > .../gpu/drm/amd/display/dc/inc/hw_sequencer.h | 2 +
> > 10 files changed, 280 insertions(+), 12 deletions(-)
> >
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [RFC PATCH v2 0/5] drm/amd/display: improve DTN color state log
@ 2023-09-26 12:48 ` Melissa Wen
0 siblings, 0 replies; 23+ messages in thread
From: Melissa Wen @ 2023-09-26 12:48 UTC (permalink / raw)
To: Harry Wentland
Cc: Krunoslav Kovac, kernel-dev, Shashank Sharma, sunpeng.li,
Xinhui.Pan, Rodrigo Siqueira, dri-devel, Nicholas Kazlauskas,
amd-gfx, alexander.deucher, christian.koenig, sungjoon.kim
On 09/25, Harry Wentland wrote:
>
>
> On 2023-09-13 12:43, Melissa Wen wrote:
> > Hi,
> >
> > This is an update of previous RFC [0] improving the data collection of
> > Gamma Correction and Blend Gamma color blocks.
> >
> > As I mentioned in the last version, I'm updating the color state part of
> > DTN log to match DCN3.0 HW better. Currently, the DTN log considers the
> > DCN10 color pipeline, which is useless for DCN3.0 because of all the
> > differences in color caps between DCN versions. In addition to new color
> > blocks and caps, some semantic differences made the DCN10 output not fit
> > DCN30.
> >
> > In this RFC, the first patch adds new color state elements to DPP and
> > implements the reading of registers according to HW blocks. Similarly to
> > MPC, the second patch also creates a DCN3-specific function to read the
> > MPC state and add the MPC color state logging to it. With DPP and MPC
> > color-register reading, I detach DCN10 color state logging from the HW
> > log and create a `.log_color_state` hook for logging color state
> > according to HW color blocks with DCN30 as the first use case. Finally,
> > the last patch adds DPP and MPC color caps output to facilitate
> > understanding of the color state log.
> >
> > This version works well with the driver-specific color properties[1] and
> > steamdeck/gamescope[2] together, where we can see color state changing
> > from default values.
> >
> > Here is a before vs. after example:
> >
> > Without this series:
> > ===================
> > DPP: IGAM format IGAM mode DGAM mode RGAM mode GAMUT mode C11 C12 C13 C14 C21 C22 C23 C24 C31 C32 C33 C34
> > [ 0]: 0h BypassFixed Bypass Bypass 0 00000000h 00000000h 00000000h 00000000h 00000000h 00000000h
> > [ 3]: 0h BypassFixed Bypass Bypass 0 00000000h 00000000h 00000000h 00000000h 00000000h 00000000h
> >
> > MPCC: OPP DPP MPCCBOT MODE ALPHA_MODE PREMULT OVERLAP_ONLY IDLE
> > [ 0]: 0h 0h 3h 3 2 0 0 0
> > [ 3]: 0h 3h fh 2 2 0 0 0
> >
> > With this series (Steamdeck/Gamescope):
> > ======================================
> >
> > DPP: DGAM ROM DGAM ROM type DGAM LUT SHAPER mode 3DLUT mode 3DLUT bit depth 3DLUT size RGAM mode GAMUT mode C11 C12 C13 C14 C21 C22 C23 C24 C31 C32 C33 C34
> > [ 0]: 1 sRGB Bypass RAM A RAM B 12-bit 17x17x17 RAM A 0 00000000h 00000000h 00000000h 00000000h 00000000h 00000000h
> > [ 1]: 1 sRGB Bypass RAM B RAM A 12-bit 17x17x17 RAM A 0 00000000h 00000000h 00000000h 00000000h 00000000h 00000000h
> > [ 2]: 1 sRGB Bypass RAM B RAM A 12-bit 17x17x17 RAM A 0 00000000h 00000000h 00000000h 00000000h 00000000h 00000000h
> > [ 3]: 1 sRGB Bypass RAM A RAM B 12-bit 17x17x17 RAM A 0 00000000h 00000000h 00000000h 00000000h 00000000h 00000000h
> >
> > DPP Color Caps: input_lut_shared:0 icsc:1 dgam_ram:0 dgam_rom: srgb:1,bt2020:1,gamma2_2:1,pq:1,hlg:1 post_csc:1 gamcor:1 dgam_rom_for_yuv:0 3d_lut:1 blnd_lut:1 oscs:0
> >
> > MPCC: OPP DPP MPCCBOT MODE ALPHA_MODE PREMULT OVERLAP_ONLY IDLE SHAPER mode 3DLUT_mode 3DLUT bit-depth 3DLUT size OGAM mode OGAM LUT GAMUT mode C11 C12 C33 C34
> > [ 0]: 0h 0h 2h 3 0 1 0 0 Bypass Bypass 12-bit 17x17x17 RAM A 0 00000000h 00000000h
> > [ 1]: 0h 1h fh 2 2 0 0 0 Bypass Bypass 12-bit 17x17x17 Bypass A 0 00000000h 00000000h
> > [ 2]: 0h 2h 3h 3 0 1 0 0 Bypass Bypass 12-bit 17x17x17 Bypass A 0 00000000h 00000000h
> > [ 3]: 0h 3h 1h 3 2 0 0 0 Bypass Bypass 12-bit 17x17x17 Bypass A 0 00000000h 00000000h
> >
> > MPC Color Caps: gamut_remap:1, 3dlut:2, ogam_ram:1, ocsc:1
> >
> > With this series (Steamdeck/KDE):
> > ================================
> >
> > DPP: DGAM ROM DGAM ROM type DGAM LUT SHAPER mode 3DLUT mode 3DLUT bit depth 3DLUT size RGAM mode GAMUT mode C11 C12 C13 C14 C21 C22 C23 C24 C31 C32 C33 C34
> > [ 0]: 0 sRGB Bypass Bypass Bypass 12-bit 9x9x9 Bypass 0 00000000h 00000000h 00000000h 00000000h 00000000h 00000000h
> > [ 3]: 0 sRGB Bypass Bypass Bypass 12-bit 9x9x9 Bypass 0 00000000h 00000000h 00000000h 00000000h 00000000h 00000000h
> >
> > DPP Color Caps: input_lut_shared:0 icsc:1 dgam_ram:0 dgam_rom: srgb:1,bt2020:1,gamma2_2:1,pq:1,hlg:1 post_csc:1 gamcor:1 dgam_rom_for_yuv:0 3d_lut:1 blnd_lut:1 oscs:0
> >
> > MPCC: OPP DPP MPCCBOT MODE ALPHA_MODE PREMULT OVERLAP_ONLY IDLE SHAPER mode 3DLUT_mode 3DLUT bit-depth 3DLUT size OGAM mode OGAM LUT GAMUT mode C11 C12 C33 C34
> > [ 0]: 0h 0h 3h 3 2 0 0 0 Bypass Bypass 12-bit 17x17x17 RAM A 1 00002000h 00002000h
> > [ 3]: 0h 3h fh 2 2 0 0 0 Bypass Bypass 12-bit 17x17x17 Bypass A 0 00000000h 00000000h
> >
> > MPC Color Caps: gamut_remap:1, 3dlut:2, ogam_ram:1, ocsc:1
> >
> > Before extending it to other DCN families, I have some doubts.
> > - Does this approach of the `.log_color_state` hook make sense for you?
>
> Yes
>
> > - Is there any conflict between logging color state by HW version and
> > DTN log usage?
> > - Is there a template/style for DTN log output that I should follow or
> > information that I should include?
> >
>
> At this point it looks like we only use the DTN log for debug purposes,
> so no conflict and no need to follow a specific format, as long as the
> output is human-parseable (which yours is).
>
> At one point in the past these were used by automated tests on other
> platforms but that's no longer the case.
Great! I'll prepare a next version from your suggestions and also
addressing Siqueira's review.
Thanks for the guidance.
Melissa
>
> Harry
>
> > Let me know your thoughts.
> >
> > Thanks,
> >
> > Melissa
> >
> > [0] https://lore.kernel.org/amd-gfx/20230905142545.451153-1-mwen@igalia.com/
> > [1] https://lore.kernel.org/amd-gfx/20230810160314.48225-1-mwen@igalia.com/
> > [2] https://github.com/ValveSoftware/gamescope
> >
> > Melissa Wen (5):
> > drm/amd/display: detach color state from hw state logging
> > drm/amd/display: fill up DCN3 DPP color state
> > drm/amd/display: create DCN3-specific log for MPC state
> > drm/amd/display: hook DCN30 color state logging to DTN log
> > drm/amd/display: add DPP and MPC color caps to DTN log
> >
> > .../amd/display/dc/dcn10/dcn10_hw_sequencer.c | 53 +++++++--
> > .../gpu/drm/amd/display/dc/dcn30/dcn30_dpp.c | 45 ++++++-
> > .../drm/amd/display/dc/dcn30/dcn30_hwseq.c | 111 ++++++++++++++++++
> > .../drm/amd/display/dc/dcn30/dcn30_hwseq.h | 3 +
> > .../gpu/drm/amd/display/dc/dcn30/dcn30_init.c | 1 +
> > .../gpu/drm/amd/display/dc/dcn30/dcn30_mpc.c | 55 ++++++++-
> > .../drm/amd/display/dc/dcn301/dcn301_init.c | 1 +
> > drivers/gpu/drm/amd/display/dc/inc/hw/dpp.h | 8 ++
> > drivers/gpu/drm/amd/display/dc/inc/hw/mpc.h | 13 ++
> > .../gpu/drm/amd/display/dc/inc/hw_sequencer.h | 2 +
> > 10 files changed, 280 insertions(+), 12 deletions(-)
> >
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [RFC PATCH v2 3/5] drm/amd/display: create DCN3-specific log for MPC state
2023-09-26 12:38 ` Melissa Wen
@ 2023-09-26 13:00 ` Harry Wentland
-1 siblings, 0 replies; 23+ messages in thread
From: Harry Wentland @ 2023-09-26 13:00 UTC (permalink / raw)
To: Melissa Wen
Cc: Krunoslav Kovac, kernel-dev, Shashank Sharma, sunpeng.li,
Xinhui.Pan, Rodrigo Siqueira, amd-gfx, Nicholas Kazlauskas,
dri-devel, alexander.deucher, christian.koenig, sungjoon.kim
On 2023-09-26 08:38, Melissa Wen wrote:
> On 09/25, Harry Wentland wrote:
>>
>>
>> On 2023-09-13 12:43, Melissa Wen wrote:
>>> Logging DCN3 MPC state was following DCN1 implementation that doesn't
>>> consider new DCN3 MPC color blocks. Create new elements according to
>>> DCN3 MPC color caps and a new DCN3-specific function for reading MPC
>>> data.
>>>
>>> Signed-off-by: Melissa Wen <mwen@igalia.com>
>>> ---
>>> .../gpu/drm/amd/display/dc/dcn30/dcn30_mpc.c | 55 ++++++++++++++++++-
>>> drivers/gpu/drm/amd/display/dc/inc/hw/mpc.h | 13 +++++
>>> 2 files changed, 67 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_mpc.c b/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_mpc.c
>>> index d1500b223858..d164fbf89212 100644
>>> --- a/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_mpc.c
>>> +++ b/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_mpc.c
>>> @@ -1382,8 +1382,61 @@ static void mpc3_set_mpc_mem_lp_mode(struct mpc *mpc)
>>> }
>>> }
>>>
>>> +static void mpc3_read_mpcc_state(
>>> + struct mpc *mpc,
>>> + int mpcc_inst,
>>> + struct mpcc_state *s)
>>> +{
>>> + struct dcn30_mpc *mpc30 = TO_DCN30_MPC(mpc);
>>> + uint32_t rmu_status = 0xf;
>>> +
>>> + REG_GET(MPCC_OPP_ID[mpcc_inst], MPCC_OPP_ID, &s->opp_id);
>>> + REG_GET(MPCC_TOP_SEL[mpcc_inst], MPCC_TOP_SEL, &s->dpp_id);
>>> + REG_GET(MPCC_BOT_SEL[mpcc_inst], MPCC_BOT_SEL, &s->bot_mpcc_id);
>>> + REG_GET_4(MPCC_CONTROL[mpcc_inst], MPCC_MODE, &s->mode,
>>> + MPCC_ALPHA_BLND_MODE, &s->alpha_mode,
>>> + MPCC_ALPHA_MULTIPLIED_MODE, &s->pre_multiplied_alpha,
>>> + MPCC_BLND_ACTIVE_OVERLAP_ONLY, &s->overlap_only);
>>> + REG_GET_2(MPCC_STATUS[mpcc_inst], MPCC_IDLE, &s->idle,
>>> + MPCC_BUSY, &s->busy);
>>> +
>>> + /* Color blocks state */
>>> + REG_GET(MPC_RMU_CONTROL, MPC_RMU0_MUX_STATUS, &rmu_status);
>>> + if (rmu_status == mpcc_inst) {
>>> + REG_GET(SHAPER_CONTROL[0],
>>> + MPC_RMU_SHAPER_LUT_MODE_CURRENT, &s->shaper_lut_mode);
>>> + REG_GET(RMU_3DLUT_MODE[0],
>>> + MPC_RMU_3DLUT_MODE_CURRENT, &s->lut3d_mode);
>>> + REG_GET(RMU_3DLUT_READ_WRITE_CONTROL[0],
>>> + MPC_RMU_3DLUT_30BIT_EN, &s->lut3d_bit_depth);
>>> + REG_GET(RMU_3DLUT_MODE[0],
>>> + MPC_RMU_3DLUT_SIZE, &s->lut3d_size);
>>> + } else {
>>> + REG_GET(SHAPER_CONTROL[1],
>>> + MPC_RMU_SHAPER_LUT_MODE_CURRENT, &s->shaper_lut_mode);
>>> + REG_GET(RMU_3DLUT_MODE[1],
>>> + MPC_RMU_3DLUT_MODE_CURRENT, &s->lut3d_mode);
>>> + REG_GET(RMU_3DLUT_READ_WRITE_CONTROL[1],
>>> + MPC_RMU_3DLUT_30BIT_EN, &s->lut3d_bit_depth);
>>> + REG_GET(RMU_3DLUT_MODE[1],
>>> + MPC_RMU_3DLUT_SIZE, &s->lut3d_size);
>>> + }
>>> + REG_GET_2(MPCC_OGAM_CONTROL[mpcc_inst],
>>> + MPCC_OGAM_MODE_CURRENT, &s->rgam_mode,
>>> + MPCC_OGAM_SELECT_CURRENT, &s->rgam_lut);
>>> + REG_GET(MPCC_GAMUT_REMAP_MODE[mpcc_inst],
>>> + MPCC_GAMUT_REMAP_MODE_CURRENT, &s->gamut_remap_mode);
>>> + if (s->gamut_remap_mode == 1) {
>>> + s->gamut_remap_c11_c12 = REG_READ(MPC_GAMUT_REMAP_C11_C12_A[mpcc_inst]);
>>> + s->gamut_remap_c33_c34 = REG_READ(MPC_GAMUT_REMAP_C33_C34_A[mpcc_inst]);
>>> + } else if (s->gamut_remap_mode == 2) {
>>> + s->gamut_remap_c11_c12 = REG_READ(MPC_GAMUT_REMAP_C11_C12_B[mpcc_inst]);
>>> + s->gamut_remap_c33_c34 = REG_READ(MPC_GAMUT_REMAP_C33_C34_B[mpcc_inst]);
>>
>> Any reason we're getting (and printing) only the first and last
>> coefficients? Is it to avoid printing lines that are too wide?
>
> I'm unable to reach the other coefficients through this
> `MPC_GAMUT_REMAP` register prefix. But I'm probably missing something
> since I can see the registers using UMR. I'll try to find the right path
> and update it.
>
From dcn3_0_1_offset.h the registers are there:
> #define mmMPCC_OGAM0_MPC_GAMUT_REMAP_C11_C12_A 0x014c
> #define mmMPCC_OGAM0_MPC_GAMUT_REMAP_C11_C12_A_BASE_IDX 3
> #define mmMPCC_OGAM0_MPC_GAMUT_REMAP_C13_C14_A 0x014d
> #define mmMPCC_OGAM0_MPC_GAMUT_REMAP_C13_C14_A_BASE_IDX 3
> #define mmMPCC_OGAM0_MPC_GAMUT_REMAP_C21_C22_A 0x014e
> #define mmMPCC_OGAM0_MPC_GAMUT_REMAP_C21_C22_A_BASE_IDX 3
> #define mmMPCC_OGAM0_MPC_GAMUT_REMAP_C23_C24_A 0x014f
> #define mmMPCC_OGAM0_MPC_GAMUT_REMAP_C23_C24_A_BASE_IDX 3
> #define mmMPCC_OGAM0_MPC_GAMUT_REMAP_C31_C32_A 0x0150
> #define mmMPCC_OGAM0_MPC_GAMUT_REMAP_C31_C32_A_BASE_IDX 3
> #define mmMPCC_OGAM0_MPC_GAMUT_REMAP_C33_C34_A 0x0151
> #define mmMPCC_OGAM0_MPC_GAMUT_REMAP_C33_C34_A_BASE_IDX 3
> #define mmMPCC_OGAM0_MPC_GAMUT_REMAP_C11_C12_B 0x0152
> #define mmMPCC_OGAM0_MPC_GAMUT_REMAP_C11_C12_B_BASE_IDX 3
> #define mmMPCC_OGAM0_MPC_GAMUT_REMAP_C13_C14_B 0x0153
> #define mmMPCC_OGAM0_MPC_GAMUT_REMAP_C13_C14_B_BASE_IDX 3
> #define mmMPCC_OGAM0_MPC_GAMUT_REMAP_C21_C22_B 0x0154
> #define mmMPCC_OGAM0_MPC_GAMUT_REMAP_C21_C22_B_BASE_IDX 3
> #define mmMPCC_OGAM0_MPC_GAMUT_REMAP_C23_C24_B 0x0155
> #define mmMPCC_OGAM0_MPC_GAMUT_REMAP_C23_C24_B_BASE_IDX 3
> #define mmMPCC_OGAM0_MPC_GAMUT_REMAP_C31_C32_B 0x0156
> #define mmMPCC_OGAM0_MPC_GAMUT_REMAP_C31_C32_B_BASE_IDX 3
> #define mmMPCC_OGAM0_MPC_GAMUT_REMAP_C33_C34_B 0x0157
> #define mmMPCC_OGAM0_MPC_GAMUT_REMAP_C33_C34_B_BASE_IDX 3
But in dcn30_mpc.h we only define the first and last:
> #define MPC_REG_LIST_DCN3_0(inst)\
...
> SRII(MPC_GAMUT_REMAP_C11_C12_A, MPCC_OGAM, inst),\
> SRII(MPC_GAMUT_REMAP_C33_C34_A, MPCC_OGAM, inst),\
> SRII(MPC_GAMUT_REMAP_C11_C12_B, MPCC_OGAM, inst),\
> SRII(MPC_GAMUT_REMAP_C33_C34_B, MPCC_OGAM, inst),\
If you add the othes it should work.
The reason for the MPC_REG_LIST_DCN3_0 (and others like it) is to
(a) abstract the _offset.h definitions away and give us common code
for register access for all generations, no matter if the offsets
change, and
(b) to give us compile errors if a register definition is missing.
Harry
> Melissa
>
>>
>> Harry
>>
>>> + }
>>> +}
>>> +
>>> static const struct mpc_funcs dcn30_mpc_funcs = {
>>> - .read_mpcc_state = mpc1_read_mpcc_state,
>>> + .read_mpcc_state = mpc3_read_mpcc_state,
>>> .insert_plane = mpc1_insert_plane,
>>> .remove_mpcc = mpc1_remove_mpcc,
>>> .mpc_init = mpc1_mpc_init,
>>> diff --git a/drivers/gpu/drm/amd/display/dc/inc/hw/mpc.h b/drivers/gpu/drm/amd/display/dc/inc/hw/mpc.h
>>> index 8d86159d9de0..e60b3503605b 100644
>>> --- a/drivers/gpu/drm/amd/display/dc/inc/hw/mpc.h
>>> +++ b/drivers/gpu/drm/amd/display/dc/inc/hw/mpc.h
>>> @@ -193,6 +193,19 @@ struct mpcc_state {
>>> uint32_t overlap_only;
>>> uint32_t idle;
>>> uint32_t busy;
>>> + uint32_t shaper_lut_mode;
>>> + uint32_t lut3d_mode;
>>> + uint32_t lut3d_bit_depth;
>>> + uint32_t lut3d_size;
>>> + uint32_t rgam_mode;
>>> + uint32_t rgam_lut;
>>> + uint32_t gamut_remap_mode;
>>> + uint32_t gamut_remap_c11_c12;
>>> + uint32_t gamut_remap_c13_c14;
>>> + uint32_t gamut_remap_c21_c22;
>>> + uint32_t gamut_remap_c23_c24;
>>> + uint32_t gamut_remap_c31_c32;
>>> + uint32_t gamut_remap_c33_c34;
>>> };
>>>
>>> /**
>>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [RFC PATCH v2 3/5] drm/amd/display: create DCN3-specific log for MPC state
@ 2023-09-26 13:00 ` Harry Wentland
0 siblings, 0 replies; 23+ messages in thread
From: Harry Wentland @ 2023-09-26 13:00 UTC (permalink / raw)
To: Melissa Wen
Cc: Krunoslav Kovac, kernel-dev, Shashank Sharma, sunpeng.li,
Xinhui.Pan, Rodrigo Siqueira, amd-gfx, Nicholas Kazlauskas,
dri-devel, daniel, alexander.deucher, airlied, christian.koenig,
sungjoon.kim
On 2023-09-26 08:38, Melissa Wen wrote:
> On 09/25, Harry Wentland wrote:
>>
>>
>> On 2023-09-13 12:43, Melissa Wen wrote:
>>> Logging DCN3 MPC state was following DCN1 implementation that doesn't
>>> consider new DCN3 MPC color blocks. Create new elements according to
>>> DCN3 MPC color caps and a new DCN3-specific function for reading MPC
>>> data.
>>>
>>> Signed-off-by: Melissa Wen <mwen@igalia.com>
>>> ---
>>> .../gpu/drm/amd/display/dc/dcn30/dcn30_mpc.c | 55 ++++++++++++++++++-
>>> drivers/gpu/drm/amd/display/dc/inc/hw/mpc.h | 13 +++++
>>> 2 files changed, 67 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_mpc.c b/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_mpc.c
>>> index d1500b223858..d164fbf89212 100644
>>> --- a/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_mpc.c
>>> +++ b/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_mpc.c
>>> @@ -1382,8 +1382,61 @@ static void mpc3_set_mpc_mem_lp_mode(struct mpc *mpc)
>>> }
>>> }
>>>
>>> +static void mpc3_read_mpcc_state(
>>> + struct mpc *mpc,
>>> + int mpcc_inst,
>>> + struct mpcc_state *s)
>>> +{
>>> + struct dcn30_mpc *mpc30 = TO_DCN30_MPC(mpc);
>>> + uint32_t rmu_status = 0xf;
>>> +
>>> + REG_GET(MPCC_OPP_ID[mpcc_inst], MPCC_OPP_ID, &s->opp_id);
>>> + REG_GET(MPCC_TOP_SEL[mpcc_inst], MPCC_TOP_SEL, &s->dpp_id);
>>> + REG_GET(MPCC_BOT_SEL[mpcc_inst], MPCC_BOT_SEL, &s->bot_mpcc_id);
>>> + REG_GET_4(MPCC_CONTROL[mpcc_inst], MPCC_MODE, &s->mode,
>>> + MPCC_ALPHA_BLND_MODE, &s->alpha_mode,
>>> + MPCC_ALPHA_MULTIPLIED_MODE, &s->pre_multiplied_alpha,
>>> + MPCC_BLND_ACTIVE_OVERLAP_ONLY, &s->overlap_only);
>>> + REG_GET_2(MPCC_STATUS[mpcc_inst], MPCC_IDLE, &s->idle,
>>> + MPCC_BUSY, &s->busy);
>>> +
>>> + /* Color blocks state */
>>> + REG_GET(MPC_RMU_CONTROL, MPC_RMU0_MUX_STATUS, &rmu_status);
>>> + if (rmu_status == mpcc_inst) {
>>> + REG_GET(SHAPER_CONTROL[0],
>>> + MPC_RMU_SHAPER_LUT_MODE_CURRENT, &s->shaper_lut_mode);
>>> + REG_GET(RMU_3DLUT_MODE[0],
>>> + MPC_RMU_3DLUT_MODE_CURRENT, &s->lut3d_mode);
>>> + REG_GET(RMU_3DLUT_READ_WRITE_CONTROL[0],
>>> + MPC_RMU_3DLUT_30BIT_EN, &s->lut3d_bit_depth);
>>> + REG_GET(RMU_3DLUT_MODE[0],
>>> + MPC_RMU_3DLUT_SIZE, &s->lut3d_size);
>>> + } else {
>>> + REG_GET(SHAPER_CONTROL[1],
>>> + MPC_RMU_SHAPER_LUT_MODE_CURRENT, &s->shaper_lut_mode);
>>> + REG_GET(RMU_3DLUT_MODE[1],
>>> + MPC_RMU_3DLUT_MODE_CURRENT, &s->lut3d_mode);
>>> + REG_GET(RMU_3DLUT_READ_WRITE_CONTROL[1],
>>> + MPC_RMU_3DLUT_30BIT_EN, &s->lut3d_bit_depth);
>>> + REG_GET(RMU_3DLUT_MODE[1],
>>> + MPC_RMU_3DLUT_SIZE, &s->lut3d_size);
>>> + }
>>> + REG_GET_2(MPCC_OGAM_CONTROL[mpcc_inst],
>>> + MPCC_OGAM_MODE_CURRENT, &s->rgam_mode,
>>> + MPCC_OGAM_SELECT_CURRENT, &s->rgam_lut);
>>> + REG_GET(MPCC_GAMUT_REMAP_MODE[mpcc_inst],
>>> + MPCC_GAMUT_REMAP_MODE_CURRENT, &s->gamut_remap_mode);
>>> + if (s->gamut_remap_mode == 1) {
>>> + s->gamut_remap_c11_c12 = REG_READ(MPC_GAMUT_REMAP_C11_C12_A[mpcc_inst]);
>>> + s->gamut_remap_c33_c34 = REG_READ(MPC_GAMUT_REMAP_C33_C34_A[mpcc_inst]);
>>> + } else if (s->gamut_remap_mode == 2) {
>>> + s->gamut_remap_c11_c12 = REG_READ(MPC_GAMUT_REMAP_C11_C12_B[mpcc_inst]);
>>> + s->gamut_remap_c33_c34 = REG_READ(MPC_GAMUT_REMAP_C33_C34_B[mpcc_inst]);
>>
>> Any reason we're getting (and printing) only the first and last
>> coefficients? Is it to avoid printing lines that are too wide?
>
> I'm unable to reach the other coefficients through this
> `MPC_GAMUT_REMAP` register prefix. But I'm probably missing something
> since I can see the registers using UMR. I'll try to find the right path
> and update it.
>
From dcn3_0_1_offset.h the registers are there:
> #define mmMPCC_OGAM0_MPC_GAMUT_REMAP_C11_C12_A 0x014c
> #define mmMPCC_OGAM0_MPC_GAMUT_REMAP_C11_C12_A_BASE_IDX 3
> #define mmMPCC_OGAM0_MPC_GAMUT_REMAP_C13_C14_A 0x014d
> #define mmMPCC_OGAM0_MPC_GAMUT_REMAP_C13_C14_A_BASE_IDX 3
> #define mmMPCC_OGAM0_MPC_GAMUT_REMAP_C21_C22_A 0x014e
> #define mmMPCC_OGAM0_MPC_GAMUT_REMAP_C21_C22_A_BASE_IDX 3
> #define mmMPCC_OGAM0_MPC_GAMUT_REMAP_C23_C24_A 0x014f
> #define mmMPCC_OGAM0_MPC_GAMUT_REMAP_C23_C24_A_BASE_IDX 3
> #define mmMPCC_OGAM0_MPC_GAMUT_REMAP_C31_C32_A 0x0150
> #define mmMPCC_OGAM0_MPC_GAMUT_REMAP_C31_C32_A_BASE_IDX 3
> #define mmMPCC_OGAM0_MPC_GAMUT_REMAP_C33_C34_A 0x0151
> #define mmMPCC_OGAM0_MPC_GAMUT_REMAP_C33_C34_A_BASE_IDX 3
> #define mmMPCC_OGAM0_MPC_GAMUT_REMAP_C11_C12_B 0x0152
> #define mmMPCC_OGAM0_MPC_GAMUT_REMAP_C11_C12_B_BASE_IDX 3
> #define mmMPCC_OGAM0_MPC_GAMUT_REMAP_C13_C14_B 0x0153
> #define mmMPCC_OGAM0_MPC_GAMUT_REMAP_C13_C14_B_BASE_IDX 3
> #define mmMPCC_OGAM0_MPC_GAMUT_REMAP_C21_C22_B 0x0154
> #define mmMPCC_OGAM0_MPC_GAMUT_REMAP_C21_C22_B_BASE_IDX 3
> #define mmMPCC_OGAM0_MPC_GAMUT_REMAP_C23_C24_B 0x0155
> #define mmMPCC_OGAM0_MPC_GAMUT_REMAP_C23_C24_B_BASE_IDX 3
> #define mmMPCC_OGAM0_MPC_GAMUT_REMAP_C31_C32_B 0x0156
> #define mmMPCC_OGAM0_MPC_GAMUT_REMAP_C31_C32_B_BASE_IDX 3
> #define mmMPCC_OGAM0_MPC_GAMUT_REMAP_C33_C34_B 0x0157
> #define mmMPCC_OGAM0_MPC_GAMUT_REMAP_C33_C34_B_BASE_IDX 3
But in dcn30_mpc.h we only define the first and last:
> #define MPC_REG_LIST_DCN3_0(inst)\
...
> SRII(MPC_GAMUT_REMAP_C11_C12_A, MPCC_OGAM, inst),\
> SRII(MPC_GAMUT_REMAP_C33_C34_A, MPCC_OGAM, inst),\
> SRII(MPC_GAMUT_REMAP_C11_C12_B, MPCC_OGAM, inst),\
> SRII(MPC_GAMUT_REMAP_C33_C34_B, MPCC_OGAM, inst),\
If you add the othes it should work.
The reason for the MPC_REG_LIST_DCN3_0 (and others like it) is to
(a) abstract the _offset.h definitions away and give us common code
for register access for all generations, no matter if the offsets
change, and
(b) to give us compile errors if a register definition is missing.
Harry
> Melissa
>
>>
>> Harry
>>
>>> + }
>>> +}
>>> +
>>> static const struct mpc_funcs dcn30_mpc_funcs = {
>>> - .read_mpcc_state = mpc1_read_mpcc_state,
>>> + .read_mpcc_state = mpc3_read_mpcc_state,
>>> .insert_plane = mpc1_insert_plane,
>>> .remove_mpcc = mpc1_remove_mpcc,
>>> .mpc_init = mpc1_mpc_init,
>>> diff --git a/drivers/gpu/drm/amd/display/dc/inc/hw/mpc.h b/drivers/gpu/drm/amd/display/dc/inc/hw/mpc.h
>>> index 8d86159d9de0..e60b3503605b 100644
>>> --- a/drivers/gpu/drm/amd/display/dc/inc/hw/mpc.h
>>> +++ b/drivers/gpu/drm/amd/display/dc/inc/hw/mpc.h
>>> @@ -193,6 +193,19 @@ struct mpcc_state {
>>> uint32_t overlap_only;
>>> uint32_t idle;
>>> uint32_t busy;
>>> + uint32_t shaper_lut_mode;
>>> + uint32_t lut3d_mode;
>>> + uint32_t lut3d_bit_depth;
>>> + uint32_t lut3d_size;
>>> + uint32_t rgam_mode;
>>> + uint32_t rgam_lut;
>>> + uint32_t gamut_remap_mode;
>>> + uint32_t gamut_remap_c11_c12;
>>> + uint32_t gamut_remap_c13_c14;
>>> + uint32_t gamut_remap_c21_c22;
>>> + uint32_t gamut_remap_c23_c24;
>>> + uint32_t gamut_remap_c31_c32;
>>> + uint32_t gamut_remap_c33_c34;
>>> };
>>>
>>> /**
>>
^ permalink raw reply [flat|nested] 23+ messages in thread
end of thread, other threads:[~2023-09-26 13:00 UTC | newest]
Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-09-13 16:43 [RFC PATCH v2 0/5] drm/amd/display: improve DTN color state log Melissa Wen
2023-09-13 16:43 ` [RFC PATCH v2 1/5] drm/amd/display: detach color state from hw state logging Melissa Wen
2023-09-13 22:41 ` Rodrigo Siqueira Jordao
2023-09-13 22:41 ` Rodrigo Siqueira Jordao
2023-09-13 16:43 ` [RFC PATCH v2 2/5] drm/amd/display: fill up DCN3 DPP color state Melissa Wen
2023-09-13 22:50 ` Rodrigo Siqueira Jordao
2023-09-13 22:50 ` Rodrigo Siqueira Jordao
2023-09-13 16:43 ` [RFC PATCH v2 3/5] drm/amd/display: create DCN3-specific log for MPC state Melissa Wen
2023-09-25 16:03 ` Harry Wentland
2023-09-26 12:38 ` Melissa Wen
2023-09-26 12:38 ` Melissa Wen
2023-09-26 13:00 ` Harry Wentland
2023-09-26 13:00 ` Harry Wentland
2023-09-13 16:43 ` [RFC PATCH v2 4/5] drm/amd/display: hook DCN30 color state logging to DTN log Melissa Wen
2023-09-25 16:03 ` Harry Wentland
2023-09-26 12:43 ` Melissa Wen
2023-09-26 12:43 ` Melissa Wen
2023-09-13 16:43 ` [RFC PATCH v2 5/5] drm/amd/display: add DPP and MPC color caps " Melissa Wen
2023-09-13 23:02 ` [RFC PATCH v2 0/5] drm/amd/display: improve DTN color state log Rodrigo Siqueira Jordao
2023-09-13 23:02 ` Rodrigo Siqueira Jordao
2023-09-25 16:03 ` Harry Wentland
2023-09-26 12:48 ` Melissa Wen
2023-09-26 12:48 ` Melissa Wen
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.