* Some HDMI deep color output fixes for DC on DCE 6-11
@ 2021-01-21 6:17 ` Mario Kleiner
0 siblings, 0 replies; 14+ messages in thread
From: Mario Kleiner @ 2021-01-21 6:17 UTC (permalink / raw)
To: dri-devel, amd-gfx; +Cc: alexander.deucher
Hi,
these two patches fix non-working HDMI deep color output on DCE-8.3,
AMD Mullins when amdgpu-kms is used with Displaycore force-enabled,
ie. for radeon.cik_support=0 amdgpu.cik_support=1 amdgpu.dc=1:
I suspect they might fix similar problems on other older asics of
DCE-11.0 and earlier.
Patch 1/2 is a fix for some oddity i found while hunting for the
HDMI deep color bug. It fixes what looks like an obvious mistake,
but the fix did not improve or degrade anything, so maybe the hw
doesn't care all too much about the wrong clamping/truncation mask?
Anyway, it makes the code less confusing.
Patch 2/2 fixes HDMI deep color output at 10 bpc or 12 bpc output
on AMD Mullins, DCE-8.3, where at output_bpc 10 or 12 the display
would be scrambled. With the patch, the display looks correct, and
photometer measurements on a HDR-10 monitor suggest we probably get
the correct output signal. I found the fix by comparing DC against
the classic amdgpu-kms and radeon-kms modesetting path, readding
missing stuff.
Given other encoder/pixelclock setup functions than the ones used
on DCE-8.3 showed the same omissions, i added missing code there as
well, but i couldn't test it due to lack of hw, so i hope that fixes
instead of breaks things on asic's other than DCE-8.3.
I also created a similar patch for DCE-11.2 and later, not included
here, but during testing on a Raven Ridge DCN-1, the patch neither
helped nor hurt. Output was correct without the patch, and adding the
patch didn't change or break anything on DCN-1. Looking at disassembled
AtomBios tables for DCN-1 and a DCE-11.2, i think AtomBios may not do
much with the info that was missing, which would explain why the
current upstream code seems to work fine without it? At least as
verified on DCN-1. I can't test on DCE-11.2 or DCE-12 due to lack
of hw with actual HDMI output. But it would be interesting for me to
know what changed wrt. Atombios in later asic versions to make some
of this setup apparently redundant in DC?
Do you test DC wrt. HDMI deep color starting at a specific DCE
revision, given that the bug went unnoticed in DCE-8.3, but things
seem to be fine on at least DCN-1?
Thanks,
-mario
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 14+ messages in thread
* Some HDMI deep color output fixes for DC on DCE 6-11
@ 2021-01-21 6:17 ` Mario Kleiner
0 siblings, 0 replies; 14+ messages in thread
From: Mario Kleiner @ 2021-01-21 6:17 UTC (permalink / raw)
To: dri-devel, amd-gfx; +Cc: alexander.deucher, mario.kleiner.de, harry.wentland
Hi,
these two patches fix non-working HDMI deep color output on DCE-8.3,
AMD Mullins when amdgpu-kms is used with Displaycore force-enabled,
ie. for radeon.cik_support=0 amdgpu.cik_support=1 amdgpu.dc=1:
I suspect they might fix similar problems on other older asics of
DCE-11.0 and earlier.
Patch 1/2 is a fix for some oddity i found while hunting for the
HDMI deep color bug. It fixes what looks like an obvious mistake,
but the fix did not improve or degrade anything, so maybe the hw
doesn't care all too much about the wrong clamping/truncation mask?
Anyway, it makes the code less confusing.
Patch 2/2 fixes HDMI deep color output at 10 bpc or 12 bpc output
on AMD Mullins, DCE-8.3, where at output_bpc 10 or 12 the display
would be scrambled. With the patch, the display looks correct, and
photometer measurements on a HDR-10 monitor suggest we probably get
the correct output signal. I found the fix by comparing DC against
the classic amdgpu-kms and radeon-kms modesetting path, readding
missing stuff.
Given other encoder/pixelclock setup functions than the ones used
on DCE-8.3 showed the same omissions, i added missing code there as
well, but i couldn't test it due to lack of hw, so i hope that fixes
instead of breaks things on asic's other than DCE-8.3.
I also created a similar patch for DCE-11.2 and later, not included
here, but during testing on a Raven Ridge DCN-1, the patch neither
helped nor hurt. Output was correct without the patch, and adding the
patch didn't change or break anything on DCN-1. Looking at disassembled
AtomBios tables for DCN-1 and a DCE-11.2, i think AtomBios may not do
much with the info that was missing, which would explain why the
current upstream code seems to work fine without it? At least as
verified on DCN-1. I can't test on DCE-11.2 or DCE-12 due to lack
of hw with actual HDMI output. But it would be interesting for me to
know what changed wrt. Atombios in later asic versions to make some
of this setup apparently redundant in DC?
Do you test DC wrt. HDMI deep color starting at a specific DCE
revision, given that the bug went unnoticed in DCE-8.3, but things
seem to be fine on at least DCN-1?
Thanks,
-mario
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 1/2] drm/amd/display: Fix 10/12 bpc setup in DCE output bit depth reduction.
2021-01-21 6:17 ` Mario Kleiner
@ 2021-01-21 6:17 ` Mario Kleiner
-1 siblings, 0 replies; 14+ messages in thread
From: Mario Kleiner @ 2021-01-21 6:17 UTC (permalink / raw)
To: dri-devel, amd-gfx; +Cc: alexander.deucher
In set_clamp(), the comments and definitions for the COLOR_DEPTH_101010
and COLOR_DEPTH_121212 cases directly contradict the code comment which
explains how this should work, whereas the COLOR_DEPTH_888 case
is consistent with the code comments. Comment says the bitmask should
be chosen to align to the top-most 10 or 12 MSB's on a 14 bit bus, but
the implementation contradicts that: 10 bit case sets a mask for 12 bpc
clamping, whereas 12 bit case sets a mask for 14 bpc clamping.
Note that during my limited testing on DCE-8.3 (HDMI deep color)
and DCE-11.2 (DP deep color), this didn't have any obvious ill
effects, neither did fixing it change anything obvious for the
better, so this fix may be inconsequential on DCE, and just
reduce the confusion of innocent bystanders when reading the code
and trying to investigate problems with 10 bpc+ output.
Fixes: 4562236b3bc0 ("drm/amd/dc: Add dc display driver (v2)")
Signed-off-by: Mario Kleiner <mario.kleiner.de@gmail.com>
Cc: Harry Wentland <harry.wentland@amd.com>
---
drivers/gpu/drm/amd/display/dc/dce/dce_transform.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/dc/dce/dce_transform.c b/drivers/gpu/drm/amd/display/dc/dce/dce_transform.c
index 130a0a0c8332..68028ec995e7 100644
--- a/drivers/gpu/drm/amd/display/dc/dce/dce_transform.c
+++ b/drivers/gpu/drm/amd/display/dc/dce/dce_transform.c
@@ -601,12 +601,12 @@ static void set_clamp(
clamp_max = 0x3FC0;
break;
case COLOR_DEPTH_101010:
- /* 10bit MSB aligned on 14 bit bus '11 1111 1111 1100' */
- clamp_max = 0x3FFC;
+ /* 10bit MSB aligned on 14 bit bus '11 1111 1111 0000' */
+ clamp_max = 0x3FF0;
break;
case COLOR_DEPTH_121212:
- /* 12bit MSB aligned on 14 bit bus '11 1111 1111 1111' */
- clamp_max = 0x3FFF;
+ /* 12bit MSB aligned on 14 bit bus '11 1111 1111 1100' */
+ clamp_max = 0x3FFC;
break;
default:
clamp_max = 0x3FC0;
--
2.25.1
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 1/2] drm/amd/display: Fix 10/12 bpc setup in DCE output bit depth reduction.
@ 2021-01-21 6:17 ` Mario Kleiner
0 siblings, 0 replies; 14+ messages in thread
From: Mario Kleiner @ 2021-01-21 6:17 UTC (permalink / raw)
To: dri-devel, amd-gfx; +Cc: alexander.deucher, mario.kleiner.de, harry.wentland
In set_clamp(), the comments and definitions for the COLOR_DEPTH_101010
and COLOR_DEPTH_121212 cases directly contradict the code comment which
explains how this should work, whereas the COLOR_DEPTH_888 case
is consistent with the code comments. Comment says the bitmask should
be chosen to align to the top-most 10 or 12 MSB's on a 14 bit bus, but
the implementation contradicts that: 10 bit case sets a mask for 12 bpc
clamping, whereas 12 bit case sets a mask for 14 bpc clamping.
Note that during my limited testing on DCE-8.3 (HDMI deep color)
and DCE-11.2 (DP deep color), this didn't have any obvious ill
effects, neither did fixing it change anything obvious for the
better, so this fix may be inconsequential on DCE, and just
reduce the confusion of innocent bystanders when reading the code
and trying to investigate problems with 10 bpc+ output.
Fixes: 4562236b3bc0 ("drm/amd/dc: Add dc display driver (v2)")
Signed-off-by: Mario Kleiner <mario.kleiner.de@gmail.com>
Cc: Harry Wentland <harry.wentland@amd.com>
---
drivers/gpu/drm/amd/display/dc/dce/dce_transform.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/dc/dce/dce_transform.c b/drivers/gpu/drm/amd/display/dc/dce/dce_transform.c
index 130a0a0c8332..68028ec995e7 100644
--- a/drivers/gpu/drm/amd/display/dc/dce/dce_transform.c
+++ b/drivers/gpu/drm/amd/display/dc/dce/dce_transform.c
@@ -601,12 +601,12 @@ static void set_clamp(
clamp_max = 0x3FC0;
break;
case COLOR_DEPTH_101010:
- /* 10bit MSB aligned on 14 bit bus '11 1111 1111 1100' */
- clamp_max = 0x3FFC;
+ /* 10bit MSB aligned on 14 bit bus '11 1111 1111 0000' */
+ clamp_max = 0x3FF0;
break;
case COLOR_DEPTH_121212:
- /* 12bit MSB aligned on 14 bit bus '11 1111 1111 1111' */
- clamp_max = 0x3FFF;
+ /* 12bit MSB aligned on 14 bit bus '11 1111 1111 1100' */
+ clamp_max = 0x3FFC;
break;
default:
clamp_max = 0x3FC0;
--
2.25.1
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 2/2] drm/amd/display: Fix HDMI deep color output for DCE 6-11.
2021-01-21 6:17 ` Mario Kleiner
@ 2021-01-21 6:17 ` Mario Kleiner
-1 siblings, 0 replies; 14+ messages in thread
From: Mario Kleiner @ 2021-01-21 6:17 UTC (permalink / raw)
To: dri-devel, amd-gfx; +Cc: alexander.deucher
This fixes corrupted display output in HDMI deep color
10/12 bpc mode at least as observed on AMD Mullins, DCE-8.3.
It will hopefully also provide fixes for other DCE's up to
DCE-11, assuming those will need similar fixes, but i could
not test that for HDMI due to lack of suitable hw, so viewer
discretion is advised.
dce110_stream_encoder_hdmi_set_stream_attribute() is used for
HDMI setup on all DCE's and is missing color_depth assignment.
dce110_program_pix_clk() is used for pixel clock setup on HDMI
for DCE 6-11, and is missing color_depth assignment.
Additionally some of the underlying Atombios specific encoder
and pixelclock setup functions are missing code which is in
the classic amdgpu kms modesetting path and the in the radeon
kms driver for DCE6/DCE8.
encoder_control_digx_v3() - Was missing setup code wrt. amdgpu
and radeon kms classic drivers. Added here, but untested due to
lack of suitable test hw.
encoder_control_digx_v4() - Added missing setup code.
Successfully tested on AMD mullins / DCE-8.3 with HDMI deep color
output at 10 bpc and 12 bpc.
Note that encoder_control_digx_v5() has proper setup code in place
and is used, e.g., by DCE-11.2, but this code wasn't used for deep
color setup due to the missing cntl.color_depth setup in the calling
function for HDMI.
set_pixel_clock_v5() - Missing setup code wrt. classic amdgpu/radeon
kms. Added here, but untested due to lack of hw.
set_pixel_clock_v6() - Missing setup code added. Successfully tested
on AMD mullins DCE-8.3. This fixes corrupted display output at HDMI
deep color output with 10 bpc or 12 bpc.
Fixes: 4562236b3bc0 ("drm/amd/dc: Add dc display driver (v2)")
Signed-off-by: Mario Kleiner <mario.kleiner.de@gmail.com>
Cc: Harry Wentland <harry.wentland@amd.com>
---
.../drm/amd/display/dc/bios/command_table.c | 61 +++++++++++++++++++
.../drm/amd/display/dc/dce/dce_clock_source.c | 14 +++++
.../amd/display/dc/dce/dce_stream_encoder.c | 1 +
3 files changed, 76 insertions(+)
diff --git a/drivers/gpu/drm/amd/display/dc/bios/command_table.c b/drivers/gpu/drm/amd/display/dc/bios/command_table.c
index 070459e3e407..afc10b954ffa 100644
--- a/drivers/gpu/drm/amd/display/dc/bios/command_table.c
+++ b/drivers/gpu/drm/amd/display/dc/bios/command_table.c
@@ -245,6 +245,23 @@ static enum bp_result encoder_control_digx_v3(
cntl->enable_dp_audio);
params.ucLaneNum = (uint8_t)(cntl->lanes_number);
+ switch (cntl->color_depth) {
+ case COLOR_DEPTH_888:
+ params.ucBitPerColor = PANEL_8BIT_PER_COLOR;
+ break;
+ case COLOR_DEPTH_101010:
+ params.ucBitPerColor = PANEL_10BIT_PER_COLOR;
+ break;
+ case COLOR_DEPTH_121212:
+ params.ucBitPerColor = PANEL_12BIT_PER_COLOR;
+ break;
+ case COLOR_DEPTH_161616:
+ params.ucBitPerColor = PANEL_16BIT_PER_COLOR;
+ break;
+ default:
+ break;
+ }
+
if (EXEC_BIOS_CMD_TABLE(DIGxEncoderControl, params))
result = BP_RESULT_OK;
@@ -274,6 +291,23 @@ static enum bp_result encoder_control_digx_v4(
cntl->enable_dp_audio));
params.ucLaneNum = (uint8_t)(cntl->lanes_number);
+ switch (cntl->color_depth) {
+ case COLOR_DEPTH_888:
+ params.ucBitPerColor = PANEL_8BIT_PER_COLOR;
+ break;
+ case COLOR_DEPTH_101010:
+ params.ucBitPerColor = PANEL_10BIT_PER_COLOR;
+ break;
+ case COLOR_DEPTH_121212:
+ params.ucBitPerColor = PANEL_12BIT_PER_COLOR;
+ break;
+ case COLOR_DEPTH_161616:
+ params.ucBitPerColor = PANEL_16BIT_PER_COLOR;
+ break;
+ default:
+ break;
+ }
+
if (EXEC_BIOS_CMD_TABLE(DIGxEncoderControl, params))
result = BP_RESULT_OK;
@@ -1057,6 +1091,19 @@ static enum bp_result set_pixel_clock_v5(
* driver choose program it itself, i.e. here we program it
* to 888 by default.
*/
+ if (bp_params->signal_type == SIGNAL_TYPE_HDMI_TYPE_A)
+ switch (bp_params->color_depth) {
+ case TRANSMITTER_COLOR_DEPTH_30:
+ /* yes this is correct, the atom define is wrong */
+ clk.sPCLKInput.ucMiscInfo |= PIXEL_CLOCK_V5_MISC_HDMI_32BPP;
+ break;
+ case TRANSMITTER_COLOR_DEPTH_36:
+ /* yes this is correct, the atom define is wrong */
+ clk.sPCLKInput.ucMiscInfo |= PIXEL_CLOCK_V5_MISC_HDMI_30BPP;
+ break;
+ default:
+ break;
+ }
if (EXEC_BIOS_CMD_TABLE(SetPixelClock, clk))
result = BP_RESULT_OK;
@@ -1135,6 +1182,20 @@ static enum bp_result set_pixel_clock_v6(
* driver choose program it itself, i.e. here we pass required
* target rate that includes deep color.
*/
+ if (bp_params->signal_type == SIGNAL_TYPE_HDMI_TYPE_A)
+ switch (bp_params->color_depth) {
+ case TRANSMITTER_COLOR_DEPTH_30:
+ clk.sPCLKInput.ucMiscInfo |= PIXEL_CLOCK_V6_MISC_HDMI_30BPP_V6;
+ break;
+ case TRANSMITTER_COLOR_DEPTH_36:
+ clk.sPCLKInput.ucMiscInfo |= PIXEL_CLOCK_V6_MISC_HDMI_36BPP_V6;
+ break;
+ case TRANSMITTER_COLOR_DEPTH_48:
+ clk.sPCLKInput.ucMiscInfo |= PIXEL_CLOCK_V6_MISC_HDMI_48BPP;
+ break;
+ default:
+ break;
+ }
if (EXEC_BIOS_CMD_TABLE(SetPixelClock, clk))
result = BP_RESULT_OK;
diff --git a/drivers/gpu/drm/amd/display/dc/dce/dce_clock_source.c b/drivers/gpu/drm/amd/display/dc/dce/dce_clock_source.c
index fb733f573715..466f8f5803c9 100644
--- a/drivers/gpu/drm/amd/display/dc/dce/dce_clock_source.c
+++ b/drivers/gpu/drm/amd/display/dc/dce/dce_clock_source.c
@@ -871,6 +871,20 @@ static bool dce110_program_pix_clk(
bp_pc_params.flags.SET_EXTERNAL_REF_DIV_SRC =
pll_settings->use_external_clk;
+ switch (pix_clk_params->color_depth) {
+ case COLOR_DEPTH_101010:
+ bp_pc_params.color_depth = TRANSMITTER_COLOR_DEPTH_30;
+ break;
+ case COLOR_DEPTH_121212:
+ bp_pc_params.color_depth = TRANSMITTER_COLOR_DEPTH_36;
+ break;
+ case COLOR_DEPTH_161616:
+ bp_pc_params.color_depth = TRANSMITTER_COLOR_DEPTH_48;
+ break;
+ default:
+ break;
+ }
+
if (clk_src->bios->funcs->set_pixel_clock(
clk_src->bios, &bp_pc_params) != BP_RESULT_OK)
return false;
diff --git a/drivers/gpu/drm/amd/display/dc/dce/dce_stream_encoder.c b/drivers/gpu/drm/amd/display/dc/dce/dce_stream_encoder.c
index ada57f745fd7..19e380e0a330 100644
--- a/drivers/gpu/drm/amd/display/dc/dce/dce_stream_encoder.c
+++ b/drivers/gpu/drm/amd/display/dc/dce/dce_stream_encoder.c
@@ -564,6 +564,7 @@ static void dce110_stream_encoder_hdmi_set_stream_attribute(
cntl.enable_dp_audio = enable_audio;
cntl.pixel_clock = actual_pix_clk_khz;
cntl.lanes_number = LANE_COUNT_FOUR;
+ cntl.color_depth = crtc_timing->display_color_depth;
if (enc110->base.bp->funcs->encoder_control(
enc110->base.bp, &cntl) != BP_RESULT_OK)
--
2.25.1
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 2/2] drm/amd/display: Fix HDMI deep color output for DCE 6-11.
@ 2021-01-21 6:17 ` Mario Kleiner
0 siblings, 0 replies; 14+ messages in thread
From: Mario Kleiner @ 2021-01-21 6:17 UTC (permalink / raw)
To: dri-devel, amd-gfx; +Cc: alexander.deucher, mario.kleiner.de, harry.wentland
This fixes corrupted display output in HDMI deep color
10/12 bpc mode at least as observed on AMD Mullins, DCE-8.3.
It will hopefully also provide fixes for other DCE's up to
DCE-11, assuming those will need similar fixes, but i could
not test that for HDMI due to lack of suitable hw, so viewer
discretion is advised.
dce110_stream_encoder_hdmi_set_stream_attribute() is used for
HDMI setup on all DCE's and is missing color_depth assignment.
dce110_program_pix_clk() is used for pixel clock setup on HDMI
for DCE 6-11, and is missing color_depth assignment.
Additionally some of the underlying Atombios specific encoder
and pixelclock setup functions are missing code which is in
the classic amdgpu kms modesetting path and the in the radeon
kms driver for DCE6/DCE8.
encoder_control_digx_v3() - Was missing setup code wrt. amdgpu
and radeon kms classic drivers. Added here, but untested due to
lack of suitable test hw.
encoder_control_digx_v4() - Added missing setup code.
Successfully tested on AMD mullins / DCE-8.3 with HDMI deep color
output at 10 bpc and 12 bpc.
Note that encoder_control_digx_v5() has proper setup code in place
and is used, e.g., by DCE-11.2, but this code wasn't used for deep
color setup due to the missing cntl.color_depth setup in the calling
function for HDMI.
set_pixel_clock_v5() - Missing setup code wrt. classic amdgpu/radeon
kms. Added here, but untested due to lack of hw.
set_pixel_clock_v6() - Missing setup code added. Successfully tested
on AMD mullins DCE-8.3. This fixes corrupted display output at HDMI
deep color output with 10 bpc or 12 bpc.
Fixes: 4562236b3bc0 ("drm/amd/dc: Add dc display driver (v2)")
Signed-off-by: Mario Kleiner <mario.kleiner.de@gmail.com>
Cc: Harry Wentland <harry.wentland@amd.com>
---
.../drm/amd/display/dc/bios/command_table.c | 61 +++++++++++++++++++
.../drm/amd/display/dc/dce/dce_clock_source.c | 14 +++++
.../amd/display/dc/dce/dce_stream_encoder.c | 1 +
3 files changed, 76 insertions(+)
diff --git a/drivers/gpu/drm/amd/display/dc/bios/command_table.c b/drivers/gpu/drm/amd/display/dc/bios/command_table.c
index 070459e3e407..afc10b954ffa 100644
--- a/drivers/gpu/drm/amd/display/dc/bios/command_table.c
+++ b/drivers/gpu/drm/amd/display/dc/bios/command_table.c
@@ -245,6 +245,23 @@ static enum bp_result encoder_control_digx_v3(
cntl->enable_dp_audio);
params.ucLaneNum = (uint8_t)(cntl->lanes_number);
+ switch (cntl->color_depth) {
+ case COLOR_DEPTH_888:
+ params.ucBitPerColor = PANEL_8BIT_PER_COLOR;
+ break;
+ case COLOR_DEPTH_101010:
+ params.ucBitPerColor = PANEL_10BIT_PER_COLOR;
+ break;
+ case COLOR_DEPTH_121212:
+ params.ucBitPerColor = PANEL_12BIT_PER_COLOR;
+ break;
+ case COLOR_DEPTH_161616:
+ params.ucBitPerColor = PANEL_16BIT_PER_COLOR;
+ break;
+ default:
+ break;
+ }
+
if (EXEC_BIOS_CMD_TABLE(DIGxEncoderControl, params))
result = BP_RESULT_OK;
@@ -274,6 +291,23 @@ static enum bp_result encoder_control_digx_v4(
cntl->enable_dp_audio));
params.ucLaneNum = (uint8_t)(cntl->lanes_number);
+ switch (cntl->color_depth) {
+ case COLOR_DEPTH_888:
+ params.ucBitPerColor = PANEL_8BIT_PER_COLOR;
+ break;
+ case COLOR_DEPTH_101010:
+ params.ucBitPerColor = PANEL_10BIT_PER_COLOR;
+ break;
+ case COLOR_DEPTH_121212:
+ params.ucBitPerColor = PANEL_12BIT_PER_COLOR;
+ break;
+ case COLOR_DEPTH_161616:
+ params.ucBitPerColor = PANEL_16BIT_PER_COLOR;
+ break;
+ default:
+ break;
+ }
+
if (EXEC_BIOS_CMD_TABLE(DIGxEncoderControl, params))
result = BP_RESULT_OK;
@@ -1057,6 +1091,19 @@ static enum bp_result set_pixel_clock_v5(
* driver choose program it itself, i.e. here we program it
* to 888 by default.
*/
+ if (bp_params->signal_type == SIGNAL_TYPE_HDMI_TYPE_A)
+ switch (bp_params->color_depth) {
+ case TRANSMITTER_COLOR_DEPTH_30:
+ /* yes this is correct, the atom define is wrong */
+ clk.sPCLKInput.ucMiscInfo |= PIXEL_CLOCK_V5_MISC_HDMI_32BPP;
+ break;
+ case TRANSMITTER_COLOR_DEPTH_36:
+ /* yes this is correct, the atom define is wrong */
+ clk.sPCLKInput.ucMiscInfo |= PIXEL_CLOCK_V5_MISC_HDMI_30BPP;
+ break;
+ default:
+ break;
+ }
if (EXEC_BIOS_CMD_TABLE(SetPixelClock, clk))
result = BP_RESULT_OK;
@@ -1135,6 +1182,20 @@ static enum bp_result set_pixel_clock_v6(
* driver choose program it itself, i.e. here we pass required
* target rate that includes deep color.
*/
+ if (bp_params->signal_type == SIGNAL_TYPE_HDMI_TYPE_A)
+ switch (bp_params->color_depth) {
+ case TRANSMITTER_COLOR_DEPTH_30:
+ clk.sPCLKInput.ucMiscInfo |= PIXEL_CLOCK_V6_MISC_HDMI_30BPP_V6;
+ break;
+ case TRANSMITTER_COLOR_DEPTH_36:
+ clk.sPCLKInput.ucMiscInfo |= PIXEL_CLOCK_V6_MISC_HDMI_36BPP_V6;
+ break;
+ case TRANSMITTER_COLOR_DEPTH_48:
+ clk.sPCLKInput.ucMiscInfo |= PIXEL_CLOCK_V6_MISC_HDMI_48BPP;
+ break;
+ default:
+ break;
+ }
if (EXEC_BIOS_CMD_TABLE(SetPixelClock, clk))
result = BP_RESULT_OK;
diff --git a/drivers/gpu/drm/amd/display/dc/dce/dce_clock_source.c b/drivers/gpu/drm/amd/display/dc/dce/dce_clock_source.c
index fb733f573715..466f8f5803c9 100644
--- a/drivers/gpu/drm/amd/display/dc/dce/dce_clock_source.c
+++ b/drivers/gpu/drm/amd/display/dc/dce/dce_clock_source.c
@@ -871,6 +871,20 @@ static bool dce110_program_pix_clk(
bp_pc_params.flags.SET_EXTERNAL_REF_DIV_SRC =
pll_settings->use_external_clk;
+ switch (pix_clk_params->color_depth) {
+ case COLOR_DEPTH_101010:
+ bp_pc_params.color_depth = TRANSMITTER_COLOR_DEPTH_30;
+ break;
+ case COLOR_DEPTH_121212:
+ bp_pc_params.color_depth = TRANSMITTER_COLOR_DEPTH_36;
+ break;
+ case COLOR_DEPTH_161616:
+ bp_pc_params.color_depth = TRANSMITTER_COLOR_DEPTH_48;
+ break;
+ default:
+ break;
+ }
+
if (clk_src->bios->funcs->set_pixel_clock(
clk_src->bios, &bp_pc_params) != BP_RESULT_OK)
return false;
diff --git a/drivers/gpu/drm/amd/display/dc/dce/dce_stream_encoder.c b/drivers/gpu/drm/amd/display/dc/dce/dce_stream_encoder.c
index ada57f745fd7..19e380e0a330 100644
--- a/drivers/gpu/drm/amd/display/dc/dce/dce_stream_encoder.c
+++ b/drivers/gpu/drm/amd/display/dc/dce/dce_stream_encoder.c
@@ -564,6 +564,7 @@ static void dce110_stream_encoder_hdmi_set_stream_attribute(
cntl.enable_dp_audio = enable_audio;
cntl.pixel_clock = actual_pix_clk_khz;
cntl.lanes_number = LANE_COUNT_FOUR;
+ cntl.color_depth = crtc_timing->display_color_depth;
if (enc110->base.bp->funcs->encoder_control(
enc110->base.bp, &cntl) != BP_RESULT_OK)
--
2.25.1
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] drm/amd/display: Fix HDMI deep color output for DCE 6-11.
2021-01-21 6:17 ` Mario Kleiner
@ 2021-01-25 17:57 ` Alex Deucher
-1 siblings, 0 replies; 14+ messages in thread
From: Alex Deucher @ 2021-01-25 17:57 UTC (permalink / raw)
To: Mario Kleiner
Cc: Deucher, Alexander, amd-gfx list, Maling list - DRI developers
On Thu, Jan 21, 2021 at 1:17 AM Mario Kleiner
<mario.kleiner.de@gmail.com> wrote:
>
> This fixes corrupted display output in HDMI deep color
> 10/12 bpc mode at least as observed on AMD Mullins, DCE-8.3.
>
> It will hopefully also provide fixes for other DCE's up to
> DCE-11, assuming those will need similar fixes, but i could
> not test that for HDMI due to lack of suitable hw, so viewer
> discretion is advised.
>
> dce110_stream_encoder_hdmi_set_stream_attribute() is used for
> HDMI setup on all DCE's and is missing color_depth assignment.
>
> dce110_program_pix_clk() is used for pixel clock setup on HDMI
> for DCE 6-11, and is missing color_depth assignment.
>
> Additionally some of the underlying Atombios specific encoder
> and pixelclock setup functions are missing code which is in
> the classic amdgpu kms modesetting path and the in the radeon
> kms driver for DCE6/DCE8.
>
> encoder_control_digx_v3() - Was missing setup code wrt. amdgpu
> and radeon kms classic drivers. Added here, but untested due to
> lack of suitable test hw.
>
> encoder_control_digx_v4() - Added missing setup code.
> Successfully tested on AMD mullins / DCE-8.3 with HDMI deep color
> output at 10 bpc and 12 bpc.
>
> Note that encoder_control_digx_v5() has proper setup code in place
> and is used, e.g., by DCE-11.2, but this code wasn't used for deep
> color setup due to the missing cntl.color_depth setup in the calling
> function for HDMI.
>
> set_pixel_clock_v5() - Missing setup code wrt. classic amdgpu/radeon
> kms. Added here, but untested due to lack of hw.
>
> set_pixel_clock_v6() - Missing setup code added. Successfully tested
> on AMD mullins DCE-8.3. This fixes corrupted display output at HDMI
> deep color output with 10 bpc or 12 bpc.
>
> Fixes: 4562236b3bc0 ("drm/amd/dc: Add dc display driver (v2)")
>
> Signed-off-by: Mario Kleiner <mario.kleiner.de@gmail.com>
> Cc: Harry Wentland <harry.wentland@amd.com>
These make sense. I've applied the series. I'll let the display guys
gauge the other points in your cover letter.
Alex
> ---
> .../drm/amd/display/dc/bios/command_table.c | 61 +++++++++++++++++++
> .../drm/amd/display/dc/dce/dce_clock_source.c | 14 +++++
> .../amd/display/dc/dce/dce_stream_encoder.c | 1 +
> 3 files changed, 76 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/display/dc/bios/command_table.c b/drivers/gpu/drm/amd/display/dc/bios/command_table.c
> index 070459e3e407..afc10b954ffa 100644
> --- a/drivers/gpu/drm/amd/display/dc/bios/command_table.c
> +++ b/drivers/gpu/drm/amd/display/dc/bios/command_table.c
> @@ -245,6 +245,23 @@ static enum bp_result encoder_control_digx_v3(
> cntl->enable_dp_audio);
> params.ucLaneNum = (uint8_t)(cntl->lanes_number);
>
> + switch (cntl->color_depth) {
> + case COLOR_DEPTH_888:
> + params.ucBitPerColor = PANEL_8BIT_PER_COLOR;
> + break;
> + case COLOR_DEPTH_101010:
> + params.ucBitPerColor = PANEL_10BIT_PER_COLOR;
> + break;
> + case COLOR_DEPTH_121212:
> + params.ucBitPerColor = PANEL_12BIT_PER_COLOR;
> + break;
> + case COLOR_DEPTH_161616:
> + params.ucBitPerColor = PANEL_16BIT_PER_COLOR;
> + break;
> + default:
> + break;
> + }
> +
> if (EXEC_BIOS_CMD_TABLE(DIGxEncoderControl, params))
> result = BP_RESULT_OK;
>
> @@ -274,6 +291,23 @@ static enum bp_result encoder_control_digx_v4(
> cntl->enable_dp_audio));
> params.ucLaneNum = (uint8_t)(cntl->lanes_number);
>
> + switch (cntl->color_depth) {
> + case COLOR_DEPTH_888:
> + params.ucBitPerColor = PANEL_8BIT_PER_COLOR;
> + break;
> + case COLOR_DEPTH_101010:
> + params.ucBitPerColor = PANEL_10BIT_PER_COLOR;
> + break;
> + case COLOR_DEPTH_121212:
> + params.ucBitPerColor = PANEL_12BIT_PER_COLOR;
> + break;
> + case COLOR_DEPTH_161616:
> + params.ucBitPerColor = PANEL_16BIT_PER_COLOR;
> + break;
> + default:
> + break;
> + }
> +
> if (EXEC_BIOS_CMD_TABLE(DIGxEncoderControl, params))
> result = BP_RESULT_OK;
>
> @@ -1057,6 +1091,19 @@ static enum bp_result set_pixel_clock_v5(
> * driver choose program it itself, i.e. here we program it
> * to 888 by default.
> */
> + if (bp_params->signal_type == SIGNAL_TYPE_HDMI_TYPE_A)
> + switch (bp_params->color_depth) {
> + case TRANSMITTER_COLOR_DEPTH_30:
> + /* yes this is correct, the atom define is wrong */
> + clk.sPCLKInput.ucMiscInfo |= PIXEL_CLOCK_V5_MISC_HDMI_32BPP;
> + break;
> + case TRANSMITTER_COLOR_DEPTH_36:
> + /* yes this is correct, the atom define is wrong */
> + clk.sPCLKInput.ucMiscInfo |= PIXEL_CLOCK_V5_MISC_HDMI_30BPP;
> + break;
> + default:
> + break;
> + }
>
> if (EXEC_BIOS_CMD_TABLE(SetPixelClock, clk))
> result = BP_RESULT_OK;
> @@ -1135,6 +1182,20 @@ static enum bp_result set_pixel_clock_v6(
> * driver choose program it itself, i.e. here we pass required
> * target rate that includes deep color.
> */
> + if (bp_params->signal_type == SIGNAL_TYPE_HDMI_TYPE_A)
> + switch (bp_params->color_depth) {
> + case TRANSMITTER_COLOR_DEPTH_30:
> + clk.sPCLKInput.ucMiscInfo |= PIXEL_CLOCK_V6_MISC_HDMI_30BPP_V6;
> + break;
> + case TRANSMITTER_COLOR_DEPTH_36:
> + clk.sPCLKInput.ucMiscInfo |= PIXEL_CLOCK_V6_MISC_HDMI_36BPP_V6;
> + break;
> + case TRANSMITTER_COLOR_DEPTH_48:
> + clk.sPCLKInput.ucMiscInfo |= PIXEL_CLOCK_V6_MISC_HDMI_48BPP;
> + break;
> + default:
> + break;
> + }
>
> if (EXEC_BIOS_CMD_TABLE(SetPixelClock, clk))
> result = BP_RESULT_OK;
> diff --git a/drivers/gpu/drm/amd/display/dc/dce/dce_clock_source.c b/drivers/gpu/drm/amd/display/dc/dce/dce_clock_source.c
> index fb733f573715..466f8f5803c9 100644
> --- a/drivers/gpu/drm/amd/display/dc/dce/dce_clock_source.c
> +++ b/drivers/gpu/drm/amd/display/dc/dce/dce_clock_source.c
> @@ -871,6 +871,20 @@ static bool dce110_program_pix_clk(
> bp_pc_params.flags.SET_EXTERNAL_REF_DIV_SRC =
> pll_settings->use_external_clk;
>
> + switch (pix_clk_params->color_depth) {
> + case COLOR_DEPTH_101010:
> + bp_pc_params.color_depth = TRANSMITTER_COLOR_DEPTH_30;
> + break;
> + case COLOR_DEPTH_121212:
> + bp_pc_params.color_depth = TRANSMITTER_COLOR_DEPTH_36;
> + break;
> + case COLOR_DEPTH_161616:
> + bp_pc_params.color_depth = TRANSMITTER_COLOR_DEPTH_48;
> + break;
> + default:
> + break;
> + }
> +
> if (clk_src->bios->funcs->set_pixel_clock(
> clk_src->bios, &bp_pc_params) != BP_RESULT_OK)
> return false;
> diff --git a/drivers/gpu/drm/amd/display/dc/dce/dce_stream_encoder.c b/drivers/gpu/drm/amd/display/dc/dce/dce_stream_encoder.c
> index ada57f745fd7..19e380e0a330 100644
> --- a/drivers/gpu/drm/amd/display/dc/dce/dce_stream_encoder.c
> +++ b/drivers/gpu/drm/amd/display/dc/dce/dce_stream_encoder.c
> @@ -564,6 +564,7 @@ static void dce110_stream_encoder_hdmi_set_stream_attribute(
> cntl.enable_dp_audio = enable_audio;
> cntl.pixel_clock = actual_pix_clk_khz;
> cntl.lanes_number = LANE_COUNT_FOUR;
> + cntl.color_depth = crtc_timing->display_color_depth;
>
> if (enc110->base.bp->funcs->encoder_control(
> enc110->base.bp, &cntl) != BP_RESULT_OK)
> --
> 2.25.1
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] drm/amd/display: Fix HDMI deep color output for DCE 6-11.
@ 2021-01-25 17:57 ` Alex Deucher
0 siblings, 0 replies; 14+ messages in thread
From: Alex Deucher @ 2021-01-25 17:57 UTC (permalink / raw)
To: Mario Kleiner
Cc: Deucher, Alexander, amd-gfx list, Maling list - DRI developers
On Thu, Jan 21, 2021 at 1:17 AM Mario Kleiner
<mario.kleiner.de@gmail.com> wrote:
>
> This fixes corrupted display output in HDMI deep color
> 10/12 bpc mode at least as observed on AMD Mullins, DCE-8.3.
>
> It will hopefully also provide fixes for other DCE's up to
> DCE-11, assuming those will need similar fixes, but i could
> not test that for HDMI due to lack of suitable hw, so viewer
> discretion is advised.
>
> dce110_stream_encoder_hdmi_set_stream_attribute() is used for
> HDMI setup on all DCE's and is missing color_depth assignment.
>
> dce110_program_pix_clk() is used for pixel clock setup on HDMI
> for DCE 6-11, and is missing color_depth assignment.
>
> Additionally some of the underlying Atombios specific encoder
> and pixelclock setup functions are missing code which is in
> the classic amdgpu kms modesetting path and the in the radeon
> kms driver for DCE6/DCE8.
>
> encoder_control_digx_v3() - Was missing setup code wrt. amdgpu
> and radeon kms classic drivers. Added here, but untested due to
> lack of suitable test hw.
>
> encoder_control_digx_v4() - Added missing setup code.
> Successfully tested on AMD mullins / DCE-8.3 with HDMI deep color
> output at 10 bpc and 12 bpc.
>
> Note that encoder_control_digx_v5() has proper setup code in place
> and is used, e.g., by DCE-11.2, but this code wasn't used for deep
> color setup due to the missing cntl.color_depth setup in the calling
> function for HDMI.
>
> set_pixel_clock_v5() - Missing setup code wrt. classic amdgpu/radeon
> kms. Added here, but untested due to lack of hw.
>
> set_pixel_clock_v6() - Missing setup code added. Successfully tested
> on AMD mullins DCE-8.3. This fixes corrupted display output at HDMI
> deep color output with 10 bpc or 12 bpc.
>
> Fixes: 4562236b3bc0 ("drm/amd/dc: Add dc display driver (v2)")
>
> Signed-off-by: Mario Kleiner <mario.kleiner.de@gmail.com>
> Cc: Harry Wentland <harry.wentland@amd.com>
These make sense. I've applied the series. I'll let the display guys
gauge the other points in your cover letter.
Alex
> ---
> .../drm/amd/display/dc/bios/command_table.c | 61 +++++++++++++++++++
> .../drm/amd/display/dc/dce/dce_clock_source.c | 14 +++++
> .../amd/display/dc/dce/dce_stream_encoder.c | 1 +
> 3 files changed, 76 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/display/dc/bios/command_table.c b/drivers/gpu/drm/amd/display/dc/bios/command_table.c
> index 070459e3e407..afc10b954ffa 100644
> --- a/drivers/gpu/drm/amd/display/dc/bios/command_table.c
> +++ b/drivers/gpu/drm/amd/display/dc/bios/command_table.c
> @@ -245,6 +245,23 @@ static enum bp_result encoder_control_digx_v3(
> cntl->enable_dp_audio);
> params.ucLaneNum = (uint8_t)(cntl->lanes_number);
>
> + switch (cntl->color_depth) {
> + case COLOR_DEPTH_888:
> + params.ucBitPerColor = PANEL_8BIT_PER_COLOR;
> + break;
> + case COLOR_DEPTH_101010:
> + params.ucBitPerColor = PANEL_10BIT_PER_COLOR;
> + break;
> + case COLOR_DEPTH_121212:
> + params.ucBitPerColor = PANEL_12BIT_PER_COLOR;
> + break;
> + case COLOR_DEPTH_161616:
> + params.ucBitPerColor = PANEL_16BIT_PER_COLOR;
> + break;
> + default:
> + break;
> + }
> +
> if (EXEC_BIOS_CMD_TABLE(DIGxEncoderControl, params))
> result = BP_RESULT_OK;
>
> @@ -274,6 +291,23 @@ static enum bp_result encoder_control_digx_v4(
> cntl->enable_dp_audio));
> params.ucLaneNum = (uint8_t)(cntl->lanes_number);
>
> + switch (cntl->color_depth) {
> + case COLOR_DEPTH_888:
> + params.ucBitPerColor = PANEL_8BIT_PER_COLOR;
> + break;
> + case COLOR_DEPTH_101010:
> + params.ucBitPerColor = PANEL_10BIT_PER_COLOR;
> + break;
> + case COLOR_DEPTH_121212:
> + params.ucBitPerColor = PANEL_12BIT_PER_COLOR;
> + break;
> + case COLOR_DEPTH_161616:
> + params.ucBitPerColor = PANEL_16BIT_PER_COLOR;
> + break;
> + default:
> + break;
> + }
> +
> if (EXEC_BIOS_CMD_TABLE(DIGxEncoderControl, params))
> result = BP_RESULT_OK;
>
> @@ -1057,6 +1091,19 @@ static enum bp_result set_pixel_clock_v5(
> * driver choose program it itself, i.e. here we program it
> * to 888 by default.
> */
> + if (bp_params->signal_type == SIGNAL_TYPE_HDMI_TYPE_A)
> + switch (bp_params->color_depth) {
> + case TRANSMITTER_COLOR_DEPTH_30:
> + /* yes this is correct, the atom define is wrong */
> + clk.sPCLKInput.ucMiscInfo |= PIXEL_CLOCK_V5_MISC_HDMI_32BPP;
> + break;
> + case TRANSMITTER_COLOR_DEPTH_36:
> + /* yes this is correct, the atom define is wrong */
> + clk.sPCLKInput.ucMiscInfo |= PIXEL_CLOCK_V5_MISC_HDMI_30BPP;
> + break;
> + default:
> + break;
> + }
>
> if (EXEC_BIOS_CMD_TABLE(SetPixelClock, clk))
> result = BP_RESULT_OK;
> @@ -1135,6 +1182,20 @@ static enum bp_result set_pixel_clock_v6(
> * driver choose program it itself, i.e. here we pass required
> * target rate that includes deep color.
> */
> + if (bp_params->signal_type == SIGNAL_TYPE_HDMI_TYPE_A)
> + switch (bp_params->color_depth) {
> + case TRANSMITTER_COLOR_DEPTH_30:
> + clk.sPCLKInput.ucMiscInfo |= PIXEL_CLOCK_V6_MISC_HDMI_30BPP_V6;
> + break;
> + case TRANSMITTER_COLOR_DEPTH_36:
> + clk.sPCLKInput.ucMiscInfo |= PIXEL_CLOCK_V6_MISC_HDMI_36BPP_V6;
> + break;
> + case TRANSMITTER_COLOR_DEPTH_48:
> + clk.sPCLKInput.ucMiscInfo |= PIXEL_CLOCK_V6_MISC_HDMI_48BPP;
> + break;
> + default:
> + break;
> + }
>
> if (EXEC_BIOS_CMD_TABLE(SetPixelClock, clk))
> result = BP_RESULT_OK;
> diff --git a/drivers/gpu/drm/amd/display/dc/dce/dce_clock_source.c b/drivers/gpu/drm/amd/display/dc/dce/dce_clock_source.c
> index fb733f573715..466f8f5803c9 100644
> --- a/drivers/gpu/drm/amd/display/dc/dce/dce_clock_source.c
> +++ b/drivers/gpu/drm/amd/display/dc/dce/dce_clock_source.c
> @@ -871,6 +871,20 @@ static bool dce110_program_pix_clk(
> bp_pc_params.flags.SET_EXTERNAL_REF_DIV_SRC =
> pll_settings->use_external_clk;
>
> + switch (pix_clk_params->color_depth) {
> + case COLOR_DEPTH_101010:
> + bp_pc_params.color_depth = TRANSMITTER_COLOR_DEPTH_30;
> + break;
> + case COLOR_DEPTH_121212:
> + bp_pc_params.color_depth = TRANSMITTER_COLOR_DEPTH_36;
> + break;
> + case COLOR_DEPTH_161616:
> + bp_pc_params.color_depth = TRANSMITTER_COLOR_DEPTH_48;
> + break;
> + default:
> + break;
> + }
> +
> if (clk_src->bios->funcs->set_pixel_clock(
> clk_src->bios, &bp_pc_params) != BP_RESULT_OK)
> return false;
> diff --git a/drivers/gpu/drm/amd/display/dc/dce/dce_stream_encoder.c b/drivers/gpu/drm/amd/display/dc/dce/dce_stream_encoder.c
> index ada57f745fd7..19e380e0a330 100644
> --- a/drivers/gpu/drm/amd/display/dc/dce/dce_stream_encoder.c
> +++ b/drivers/gpu/drm/amd/display/dc/dce/dce_stream_encoder.c
> @@ -564,6 +564,7 @@ static void dce110_stream_encoder_hdmi_set_stream_attribute(
> cntl.enable_dp_audio = enable_audio;
> cntl.pixel_clock = actual_pix_clk_khz;
> cntl.lanes_number = LANE_COUNT_FOUR;
> + cntl.color_depth = crtc_timing->display_color_depth;
>
> if (enc110->base.bp->funcs->encoder_control(
> enc110->base.bp, &cntl) != BP_RESULT_OK)
> --
> 2.25.1
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] drm/amd/display: Fix HDMI deep color output for DCE 6-11.
2021-01-25 17:57 ` Alex Deucher
@ 2021-01-25 19:16 ` Kazlauskas, Nicholas
-1 siblings, 0 replies; 14+ messages in thread
From: Kazlauskas, Nicholas @ 2021-01-25 19:16 UTC (permalink / raw)
To: Alex Deucher, Mario Kleiner
Cc: Deucher, Alexander, Maling list - DRI developers, amd-gfx list
On 2021-01-25 12:57 p.m., Alex Deucher wrote:
> On Thu, Jan 21, 2021 at 1:17 AM Mario Kleiner
> <mario.kleiner.de@gmail.com> wrote:
>>
>> This fixes corrupted display output in HDMI deep color
>> 10/12 bpc mode at least as observed on AMD Mullins, DCE-8.3.
>>
>> It will hopefully also provide fixes for other DCE's up to
>> DCE-11, assuming those will need similar fixes, but i could
>> not test that for HDMI due to lack of suitable hw, so viewer
>> discretion is advised.
>>
>> dce110_stream_encoder_hdmi_set_stream_attribute() is used for
>> HDMI setup on all DCE's and is missing color_depth assignment.
>>
>> dce110_program_pix_clk() is used for pixel clock setup on HDMI
>> for DCE 6-11, and is missing color_depth assignment.
>>
>> Additionally some of the underlying Atombios specific encoder
>> and pixelclock setup functions are missing code which is in
>> the classic amdgpu kms modesetting path and the in the radeon
>> kms driver for DCE6/DCE8.
>>
>> encoder_control_digx_v3() - Was missing setup code wrt. amdgpu
>> and radeon kms classic drivers. Added here, but untested due to
>> lack of suitable test hw.
>>
>> encoder_control_digx_v4() - Added missing setup code.
>> Successfully tested on AMD mullins / DCE-8.3 with HDMI deep color
>> output at 10 bpc and 12 bpc.
>>
>> Note that encoder_control_digx_v5() has proper setup code in place
>> and is used, e.g., by DCE-11.2, but this code wasn't used for deep
>> color setup due to the missing cntl.color_depth setup in the calling
>> function for HDMI.
>>
>> set_pixel_clock_v5() - Missing setup code wrt. classic amdgpu/radeon
>> kms. Added here, but untested due to lack of hw.
>>
>> set_pixel_clock_v6() - Missing setup code added. Successfully tested
>> on AMD mullins DCE-8.3. This fixes corrupted display output at HDMI
>> deep color output with 10 bpc or 12 bpc.
>>
>> Fixes: 4562236b3bc0 ("drm/amd/dc: Add dc display driver (v2)")
>>
>> Signed-off-by: Mario Kleiner <mario.kleiner.de@gmail.com>
>> Cc: Harry Wentland <harry.wentland@amd.com>
>
> These make sense. I've applied the series. I'll let the display guys
> gauge the other points in your cover letter.
>
> Alex
I don't have any concerns with this patch.
Even though it's already applied feel free to have my:
Reviewed-by: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>
Regards,
Nicholas Kazlauskas
>
>
>> ---
>> .../drm/amd/display/dc/bios/command_table.c | 61 +++++++++++++++++++
>> .../drm/amd/display/dc/dce/dce_clock_source.c | 14 +++++
>> .../amd/display/dc/dce/dce_stream_encoder.c | 1 +
>> 3 files changed, 76 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/amd/display/dc/bios/command_table.c b/drivers/gpu/drm/amd/display/dc/bios/command_table.c
>> index 070459e3e407..afc10b954ffa 100644
>> --- a/drivers/gpu/drm/amd/display/dc/bios/command_table.c
>> +++ b/drivers/gpu/drm/amd/display/dc/bios/command_table.c
>> @@ -245,6 +245,23 @@ static enum bp_result encoder_control_digx_v3(
>> cntl->enable_dp_audio);
>> params.ucLaneNum = (uint8_t)(cntl->lanes_number);
>>
>> + switch (cntl->color_depth) {
>> + case COLOR_DEPTH_888:
>> + params.ucBitPerColor = PANEL_8BIT_PER_COLOR;
>> + break;
>> + case COLOR_DEPTH_101010:
>> + params.ucBitPerColor = PANEL_10BIT_PER_COLOR;
>> + break;
>> + case COLOR_DEPTH_121212:
>> + params.ucBitPerColor = PANEL_12BIT_PER_COLOR;
>> + break;
>> + case COLOR_DEPTH_161616:
>> + params.ucBitPerColor = PANEL_16BIT_PER_COLOR;
>> + break;
>> + default:
>> + break;
>> + }
>> +
>> if (EXEC_BIOS_CMD_TABLE(DIGxEncoderControl, params))
>> result = BP_RESULT_OK;
>>
>> @@ -274,6 +291,23 @@ static enum bp_result encoder_control_digx_v4(
>> cntl->enable_dp_audio));
>> params.ucLaneNum = (uint8_t)(cntl->lanes_number);
>>
>> + switch (cntl->color_depth) {
>> + case COLOR_DEPTH_888:
>> + params.ucBitPerColor = PANEL_8BIT_PER_COLOR;
>> + break;
>> + case COLOR_DEPTH_101010:
>> + params.ucBitPerColor = PANEL_10BIT_PER_COLOR;
>> + break;
>> + case COLOR_DEPTH_121212:
>> + params.ucBitPerColor = PANEL_12BIT_PER_COLOR;
>> + break;
>> + case COLOR_DEPTH_161616:
>> + params.ucBitPerColor = PANEL_16BIT_PER_COLOR;
>> + break;
>> + default:
>> + break;
>> + }
>> +
>> if (EXEC_BIOS_CMD_TABLE(DIGxEncoderControl, params))
>> result = BP_RESULT_OK;
>>
>> @@ -1057,6 +1091,19 @@ static enum bp_result set_pixel_clock_v5(
>> * driver choose program it itself, i.e. here we program it
>> * to 888 by default.
>> */
>> + if (bp_params->signal_type == SIGNAL_TYPE_HDMI_TYPE_A)
>> + switch (bp_params->color_depth) {
>> + case TRANSMITTER_COLOR_DEPTH_30:
>> + /* yes this is correct, the atom define is wrong */
>> + clk.sPCLKInput.ucMiscInfo |= PIXEL_CLOCK_V5_MISC_HDMI_32BPP;
>> + break;
>> + case TRANSMITTER_COLOR_DEPTH_36:
>> + /* yes this is correct, the atom define is wrong */
>> + clk.sPCLKInput.ucMiscInfo |= PIXEL_CLOCK_V5_MISC_HDMI_30BPP;
>> + break;
>> + default:
>> + break;
>> + }
>>
>> if (EXEC_BIOS_CMD_TABLE(SetPixelClock, clk))
>> result = BP_RESULT_OK;
>> @@ -1135,6 +1182,20 @@ static enum bp_result set_pixel_clock_v6(
>> * driver choose program it itself, i.e. here we pass required
>> * target rate that includes deep color.
>> */
>> + if (bp_params->signal_type == SIGNAL_TYPE_HDMI_TYPE_A)
>> + switch (bp_params->color_depth) {
>> + case TRANSMITTER_COLOR_DEPTH_30:
>> + clk.sPCLKInput.ucMiscInfo |= PIXEL_CLOCK_V6_MISC_HDMI_30BPP_V6;
>> + break;
>> + case TRANSMITTER_COLOR_DEPTH_36:
>> + clk.sPCLKInput.ucMiscInfo |= PIXEL_CLOCK_V6_MISC_HDMI_36BPP_V6;
>> + break;
>> + case TRANSMITTER_COLOR_DEPTH_48:
>> + clk.sPCLKInput.ucMiscInfo |= PIXEL_CLOCK_V6_MISC_HDMI_48BPP;
>> + break;
>> + default:
>> + break;
>> + }
>>
>> if (EXEC_BIOS_CMD_TABLE(SetPixelClock, clk))
>> result = BP_RESULT_OK;
>> diff --git a/drivers/gpu/drm/amd/display/dc/dce/dce_clock_source.c b/drivers/gpu/drm/amd/display/dc/dce/dce_clock_source.c
>> index fb733f573715..466f8f5803c9 100644
>> --- a/drivers/gpu/drm/amd/display/dc/dce/dce_clock_source.c
>> +++ b/drivers/gpu/drm/amd/display/dc/dce/dce_clock_source.c
>> @@ -871,6 +871,20 @@ static bool dce110_program_pix_clk(
>> bp_pc_params.flags.SET_EXTERNAL_REF_DIV_SRC =
>> pll_settings->use_external_clk;
>>
>> + switch (pix_clk_params->color_depth) {
>> + case COLOR_DEPTH_101010:
>> + bp_pc_params.color_depth = TRANSMITTER_COLOR_DEPTH_30;
>> + break;
>> + case COLOR_DEPTH_121212:
>> + bp_pc_params.color_depth = TRANSMITTER_COLOR_DEPTH_36;
>> + break;
>> + case COLOR_DEPTH_161616:
>> + bp_pc_params.color_depth = TRANSMITTER_COLOR_DEPTH_48;
>> + break;
>> + default:
>> + break;
>> + }
>> +
>> if (clk_src->bios->funcs->set_pixel_clock(
>> clk_src->bios, &bp_pc_params) != BP_RESULT_OK)
>> return false;
>> diff --git a/drivers/gpu/drm/amd/display/dc/dce/dce_stream_encoder.c b/drivers/gpu/drm/amd/display/dc/dce/dce_stream_encoder.c
>> index ada57f745fd7..19e380e0a330 100644
>> --- a/drivers/gpu/drm/amd/display/dc/dce/dce_stream_encoder.c
>> +++ b/drivers/gpu/drm/amd/display/dc/dce/dce_stream_encoder.c
>> @@ -564,6 +564,7 @@ static void dce110_stream_encoder_hdmi_set_stream_attribute(
>> cntl.enable_dp_audio = enable_audio;
>> cntl.pixel_clock = actual_pix_clk_khz;
>> cntl.lanes_number = LANE_COUNT_FOUR;
>> + cntl.color_depth = crtc_timing->display_color_depth;
>>
>> if (enc110->base.bp->funcs->encoder_control(
>> enc110->base.bp, &cntl) != BP_RESULT_OK)
>> --
>> 2.25.1
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Fdri-devel&data=04%7C01%7Cnicholas.kazlauskas%40amd.com%7C598b2b5e841940b8c27708d8c15aa80d%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637471942408643835%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=b9C3Xi%2F2RmNsGyGUN5oBF8U%2BuGyt2w4jUZ2dK8NM4AY%3D&reserved=0
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Fdri-devel&data=04%7C01%7Cnicholas.kazlauskas%40amd.com%7C598b2b5e841940b8c27708d8c15aa80d%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637471942408643835%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=b9C3Xi%2F2RmNsGyGUN5oBF8U%2BuGyt2w4jUZ2dK8NM4AY%3D&reserved=0
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] drm/amd/display: Fix HDMI deep color output for DCE 6-11.
@ 2021-01-25 19:16 ` Kazlauskas, Nicholas
0 siblings, 0 replies; 14+ messages in thread
From: Kazlauskas, Nicholas @ 2021-01-25 19:16 UTC (permalink / raw)
To: Alex Deucher, Mario Kleiner
Cc: Deucher, Alexander, Maling list - DRI developers, amd-gfx list
On 2021-01-25 12:57 p.m., Alex Deucher wrote:
> On Thu, Jan 21, 2021 at 1:17 AM Mario Kleiner
> <mario.kleiner.de@gmail.com> wrote:
>>
>> This fixes corrupted display output in HDMI deep color
>> 10/12 bpc mode at least as observed on AMD Mullins, DCE-8.3.
>>
>> It will hopefully also provide fixes for other DCE's up to
>> DCE-11, assuming those will need similar fixes, but i could
>> not test that for HDMI due to lack of suitable hw, so viewer
>> discretion is advised.
>>
>> dce110_stream_encoder_hdmi_set_stream_attribute() is used for
>> HDMI setup on all DCE's and is missing color_depth assignment.
>>
>> dce110_program_pix_clk() is used for pixel clock setup on HDMI
>> for DCE 6-11, and is missing color_depth assignment.
>>
>> Additionally some of the underlying Atombios specific encoder
>> and pixelclock setup functions are missing code which is in
>> the classic amdgpu kms modesetting path and the in the radeon
>> kms driver for DCE6/DCE8.
>>
>> encoder_control_digx_v3() - Was missing setup code wrt. amdgpu
>> and radeon kms classic drivers. Added here, but untested due to
>> lack of suitable test hw.
>>
>> encoder_control_digx_v4() - Added missing setup code.
>> Successfully tested on AMD mullins / DCE-8.3 with HDMI deep color
>> output at 10 bpc and 12 bpc.
>>
>> Note that encoder_control_digx_v5() has proper setup code in place
>> and is used, e.g., by DCE-11.2, but this code wasn't used for deep
>> color setup due to the missing cntl.color_depth setup in the calling
>> function for HDMI.
>>
>> set_pixel_clock_v5() - Missing setup code wrt. classic amdgpu/radeon
>> kms. Added here, but untested due to lack of hw.
>>
>> set_pixel_clock_v6() - Missing setup code added. Successfully tested
>> on AMD mullins DCE-8.3. This fixes corrupted display output at HDMI
>> deep color output with 10 bpc or 12 bpc.
>>
>> Fixes: 4562236b3bc0 ("drm/amd/dc: Add dc display driver (v2)")
>>
>> Signed-off-by: Mario Kleiner <mario.kleiner.de@gmail.com>
>> Cc: Harry Wentland <harry.wentland@amd.com>
>
> These make sense. I've applied the series. I'll let the display guys
> gauge the other points in your cover letter.
>
> Alex
I don't have any concerns with this patch.
Even though it's already applied feel free to have my:
Reviewed-by: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>
Regards,
Nicholas Kazlauskas
>
>
>> ---
>> .../drm/amd/display/dc/bios/command_table.c | 61 +++++++++++++++++++
>> .../drm/amd/display/dc/dce/dce_clock_source.c | 14 +++++
>> .../amd/display/dc/dce/dce_stream_encoder.c | 1 +
>> 3 files changed, 76 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/amd/display/dc/bios/command_table.c b/drivers/gpu/drm/amd/display/dc/bios/command_table.c
>> index 070459e3e407..afc10b954ffa 100644
>> --- a/drivers/gpu/drm/amd/display/dc/bios/command_table.c
>> +++ b/drivers/gpu/drm/amd/display/dc/bios/command_table.c
>> @@ -245,6 +245,23 @@ static enum bp_result encoder_control_digx_v3(
>> cntl->enable_dp_audio);
>> params.ucLaneNum = (uint8_t)(cntl->lanes_number);
>>
>> + switch (cntl->color_depth) {
>> + case COLOR_DEPTH_888:
>> + params.ucBitPerColor = PANEL_8BIT_PER_COLOR;
>> + break;
>> + case COLOR_DEPTH_101010:
>> + params.ucBitPerColor = PANEL_10BIT_PER_COLOR;
>> + break;
>> + case COLOR_DEPTH_121212:
>> + params.ucBitPerColor = PANEL_12BIT_PER_COLOR;
>> + break;
>> + case COLOR_DEPTH_161616:
>> + params.ucBitPerColor = PANEL_16BIT_PER_COLOR;
>> + break;
>> + default:
>> + break;
>> + }
>> +
>> if (EXEC_BIOS_CMD_TABLE(DIGxEncoderControl, params))
>> result = BP_RESULT_OK;
>>
>> @@ -274,6 +291,23 @@ static enum bp_result encoder_control_digx_v4(
>> cntl->enable_dp_audio));
>> params.ucLaneNum = (uint8_t)(cntl->lanes_number);
>>
>> + switch (cntl->color_depth) {
>> + case COLOR_DEPTH_888:
>> + params.ucBitPerColor = PANEL_8BIT_PER_COLOR;
>> + break;
>> + case COLOR_DEPTH_101010:
>> + params.ucBitPerColor = PANEL_10BIT_PER_COLOR;
>> + break;
>> + case COLOR_DEPTH_121212:
>> + params.ucBitPerColor = PANEL_12BIT_PER_COLOR;
>> + break;
>> + case COLOR_DEPTH_161616:
>> + params.ucBitPerColor = PANEL_16BIT_PER_COLOR;
>> + break;
>> + default:
>> + break;
>> + }
>> +
>> if (EXEC_BIOS_CMD_TABLE(DIGxEncoderControl, params))
>> result = BP_RESULT_OK;
>>
>> @@ -1057,6 +1091,19 @@ static enum bp_result set_pixel_clock_v5(
>> * driver choose program it itself, i.e. here we program it
>> * to 888 by default.
>> */
>> + if (bp_params->signal_type == SIGNAL_TYPE_HDMI_TYPE_A)
>> + switch (bp_params->color_depth) {
>> + case TRANSMITTER_COLOR_DEPTH_30:
>> + /* yes this is correct, the atom define is wrong */
>> + clk.sPCLKInput.ucMiscInfo |= PIXEL_CLOCK_V5_MISC_HDMI_32BPP;
>> + break;
>> + case TRANSMITTER_COLOR_DEPTH_36:
>> + /* yes this is correct, the atom define is wrong */
>> + clk.sPCLKInput.ucMiscInfo |= PIXEL_CLOCK_V5_MISC_HDMI_30BPP;
>> + break;
>> + default:
>> + break;
>> + }
>>
>> if (EXEC_BIOS_CMD_TABLE(SetPixelClock, clk))
>> result = BP_RESULT_OK;
>> @@ -1135,6 +1182,20 @@ static enum bp_result set_pixel_clock_v6(
>> * driver choose program it itself, i.e. here we pass required
>> * target rate that includes deep color.
>> */
>> + if (bp_params->signal_type == SIGNAL_TYPE_HDMI_TYPE_A)
>> + switch (bp_params->color_depth) {
>> + case TRANSMITTER_COLOR_DEPTH_30:
>> + clk.sPCLKInput.ucMiscInfo |= PIXEL_CLOCK_V6_MISC_HDMI_30BPP_V6;
>> + break;
>> + case TRANSMITTER_COLOR_DEPTH_36:
>> + clk.sPCLKInput.ucMiscInfo |= PIXEL_CLOCK_V6_MISC_HDMI_36BPP_V6;
>> + break;
>> + case TRANSMITTER_COLOR_DEPTH_48:
>> + clk.sPCLKInput.ucMiscInfo |= PIXEL_CLOCK_V6_MISC_HDMI_48BPP;
>> + break;
>> + default:
>> + break;
>> + }
>>
>> if (EXEC_BIOS_CMD_TABLE(SetPixelClock, clk))
>> result = BP_RESULT_OK;
>> diff --git a/drivers/gpu/drm/amd/display/dc/dce/dce_clock_source.c b/drivers/gpu/drm/amd/display/dc/dce/dce_clock_source.c
>> index fb733f573715..466f8f5803c9 100644
>> --- a/drivers/gpu/drm/amd/display/dc/dce/dce_clock_source.c
>> +++ b/drivers/gpu/drm/amd/display/dc/dce/dce_clock_source.c
>> @@ -871,6 +871,20 @@ static bool dce110_program_pix_clk(
>> bp_pc_params.flags.SET_EXTERNAL_REF_DIV_SRC =
>> pll_settings->use_external_clk;
>>
>> + switch (pix_clk_params->color_depth) {
>> + case COLOR_DEPTH_101010:
>> + bp_pc_params.color_depth = TRANSMITTER_COLOR_DEPTH_30;
>> + break;
>> + case COLOR_DEPTH_121212:
>> + bp_pc_params.color_depth = TRANSMITTER_COLOR_DEPTH_36;
>> + break;
>> + case COLOR_DEPTH_161616:
>> + bp_pc_params.color_depth = TRANSMITTER_COLOR_DEPTH_48;
>> + break;
>> + default:
>> + break;
>> + }
>> +
>> if (clk_src->bios->funcs->set_pixel_clock(
>> clk_src->bios, &bp_pc_params) != BP_RESULT_OK)
>> return false;
>> diff --git a/drivers/gpu/drm/amd/display/dc/dce/dce_stream_encoder.c b/drivers/gpu/drm/amd/display/dc/dce/dce_stream_encoder.c
>> index ada57f745fd7..19e380e0a330 100644
>> --- a/drivers/gpu/drm/amd/display/dc/dce/dce_stream_encoder.c
>> +++ b/drivers/gpu/drm/amd/display/dc/dce/dce_stream_encoder.c
>> @@ -564,6 +564,7 @@ static void dce110_stream_encoder_hdmi_set_stream_attribute(
>> cntl.enable_dp_audio = enable_audio;
>> cntl.pixel_clock = actual_pix_clk_khz;
>> cntl.lanes_number = LANE_COUNT_FOUR;
>> + cntl.color_depth = crtc_timing->display_color_depth;
>>
>> if (enc110->base.bp->funcs->encoder_control(
>> enc110->base.bp, &cntl) != BP_RESULT_OK)
>> --
>> 2.25.1
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Fdri-devel&data=04%7C01%7Cnicholas.kazlauskas%40amd.com%7C598b2b5e841940b8c27708d8c15aa80d%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637471942408643835%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=b9C3Xi%2F2RmNsGyGUN5oBF8U%2BuGyt2w4jUZ2dK8NM4AY%3D&reserved=0
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Fdri-devel&data=04%7C01%7Cnicholas.kazlauskas%40amd.com%7C598b2b5e841940b8c27708d8c15aa80d%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637471942408643835%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=b9C3Xi%2F2RmNsGyGUN5oBF8U%2BuGyt2w4jUZ2dK8NM4AY%3D&reserved=0
>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] drm/amd/display: Fix HDMI deep color output for DCE 6-11.
2021-01-25 19:16 ` Kazlauskas, Nicholas
@ 2021-01-25 19:24 ` Mario Kleiner
-1 siblings, 0 replies; 14+ messages in thread
From: Mario Kleiner @ 2021-01-25 19:24 UTC (permalink / raw)
To: Kazlauskas, Nicholas
Cc: Deucher, Alexander, Maling list - DRI developers, amd-gfx list
[-- Attachment #1.1: Type: text/plain, Size: 11607 bytes --]
Thanks Alex and Nicholas! Brings quite a bit of extra shiny to those older
asics :)
Nicholas, any thoughts on my cover-letter wrt. why a similar patch (that I
wrote and tested to no good or bad effect) not seem to be needed on DCN,
and probably not DCE-11.2+ either? Is what is left in DC for those asic's
just dead code? My Atombios disassembly sort of pointed into that
direction, but reading disassembly is not easy on the brain, and my brain
was getting quite mushy towards the end of digging through all the code. So
some official statement would add peace of mind on my side. Is there a
certain DCE version at which your team starts validating output precision /
HDR etc. on hw?
Thanks,
-mario
On Mon, Jan 25, 2021 at 8:16 PM Kazlauskas, Nicholas <
nicholas.kazlauskas@amd.com> wrote:
> On 2021-01-25 12:57 p.m., Alex Deucher wrote:
> > On Thu, Jan 21, 2021 at 1:17 AM Mario Kleiner
> > <mario.kleiner.de@gmail.com> wrote:
> >>
> >> This fixes corrupted display output in HDMI deep color
> >> 10/12 bpc mode at least as observed on AMD Mullins, DCE-8.3.
> >>
> >> It will hopefully also provide fixes for other DCE's up to
> >> DCE-11, assuming those will need similar fixes, but i could
> >> not test that for HDMI due to lack of suitable hw, so viewer
> >> discretion is advised.
> >>
> >> dce110_stream_encoder_hdmi_set_stream_attribute() is used for
> >> HDMI setup on all DCE's and is missing color_depth assignment.
> >>
> >> dce110_program_pix_clk() is used for pixel clock setup on HDMI
> >> for DCE 6-11, and is missing color_depth assignment.
> >>
> >> Additionally some of the underlying Atombios specific encoder
> >> and pixelclock setup functions are missing code which is in
> >> the classic amdgpu kms modesetting path and the in the radeon
> >> kms driver for DCE6/DCE8.
> >>
> >> encoder_control_digx_v3() - Was missing setup code wrt. amdgpu
> >> and radeon kms classic drivers. Added here, but untested due to
> >> lack of suitable test hw.
> >>
> >> encoder_control_digx_v4() - Added missing setup code.
> >> Successfully tested on AMD mullins / DCE-8.3 with HDMI deep color
> >> output at 10 bpc and 12 bpc.
> >>
> >> Note that encoder_control_digx_v5() has proper setup code in place
> >> and is used, e.g., by DCE-11.2, but this code wasn't used for deep
> >> color setup due to the missing cntl.color_depth setup in the calling
> >> function for HDMI.
> >>
> >> set_pixel_clock_v5() - Missing setup code wrt. classic amdgpu/radeon
> >> kms. Added here, but untested due to lack of hw.
> >>
> >> set_pixel_clock_v6() - Missing setup code added. Successfully tested
> >> on AMD mullins DCE-8.3. This fixes corrupted display output at HDMI
> >> deep color output with 10 bpc or 12 bpc.
> >>
> >> Fixes: 4562236b3bc0 ("drm/amd/dc: Add dc display driver (v2)")
> >>
> >> Signed-off-by: Mario Kleiner <mario.kleiner.de@gmail.com>
> >> Cc: Harry Wentland <harry.wentland@amd.com>
> >
> > These make sense. I've applied the series. I'll let the display guys
> > gauge the other points in your cover letter.
> >
> > Alex
>
> I don't have any concerns with this patch.
>
> Even though it's already applied feel free to have my:
>
> Reviewed-by: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>
>
> Regards,
> Nicholas Kazlauskas
>
> >
> >
> >> ---
> >> .../drm/amd/display/dc/bios/command_table.c | 61 +++++++++++++++++++
> >> .../drm/amd/display/dc/dce/dce_clock_source.c | 14 +++++
> >> .../amd/display/dc/dce/dce_stream_encoder.c | 1 +
> >> 3 files changed, 76 insertions(+)
> >>
> >> diff --git a/drivers/gpu/drm/amd/display/dc/bios/command_table.c
> b/drivers/gpu/drm/amd/display/dc/bios/command_table.c
> >> index 070459e3e407..afc10b954ffa 100644
> >> --- a/drivers/gpu/drm/amd/display/dc/bios/command_table.c
> >> +++ b/drivers/gpu/drm/amd/display/dc/bios/command_table.c
> >> @@ -245,6 +245,23 @@ static enum bp_result encoder_control_digx_v3(
> >> cntl->enable_dp_audio);
> >> params.ucLaneNum = (uint8_t)(cntl->lanes_number);
> >>
> >> + switch (cntl->color_depth) {
> >> + case COLOR_DEPTH_888:
> >> + params.ucBitPerColor = PANEL_8BIT_PER_COLOR;
> >> + break;
> >> + case COLOR_DEPTH_101010:
> >> + params.ucBitPerColor = PANEL_10BIT_PER_COLOR;
> >> + break;
> >> + case COLOR_DEPTH_121212:
> >> + params.ucBitPerColor = PANEL_12BIT_PER_COLOR;
> >> + break;
> >> + case COLOR_DEPTH_161616:
> >> + params.ucBitPerColor = PANEL_16BIT_PER_COLOR;
> >> + break;
> >> + default:
> >> + break;
> >> + }
> >> +
> >> if (EXEC_BIOS_CMD_TABLE(DIGxEncoderControl, params))
> >> result = BP_RESULT_OK;
> >>
> >> @@ -274,6 +291,23 @@ static enum bp_result encoder_control_digx_v4(
> >> cntl->enable_dp_audio));
> >> params.ucLaneNum = (uint8_t)(cntl->lanes_number);
> >>
> >> + switch (cntl->color_depth) {
> >> + case COLOR_DEPTH_888:
> >> + params.ucBitPerColor = PANEL_8BIT_PER_COLOR;
> >> + break;
> >> + case COLOR_DEPTH_101010:
> >> + params.ucBitPerColor = PANEL_10BIT_PER_COLOR;
> >> + break;
> >> + case COLOR_DEPTH_121212:
> >> + params.ucBitPerColor = PANEL_12BIT_PER_COLOR;
> >> + break;
> >> + case COLOR_DEPTH_161616:
> >> + params.ucBitPerColor = PANEL_16BIT_PER_COLOR;
> >> + break;
> >> + default:
> >> + break;
> >> + }
> >> +
> >> if (EXEC_BIOS_CMD_TABLE(DIGxEncoderControl, params))
> >> result = BP_RESULT_OK;
> >>
> >> @@ -1057,6 +1091,19 @@ static enum bp_result set_pixel_clock_v5(
> >> * driver choose program it itself, i.e. here we
> program it
> >> * to 888 by default.
> >> */
> >> + if (bp_params->signal_type == SIGNAL_TYPE_HDMI_TYPE_A)
> >> + switch (bp_params->color_depth) {
> >> + case TRANSMITTER_COLOR_DEPTH_30:
> >> + /* yes this is correct, the atom define
> is wrong */
> >> + clk.sPCLKInput.ucMiscInfo |=
> PIXEL_CLOCK_V5_MISC_HDMI_32BPP;
> >> + break;
> >> + case TRANSMITTER_COLOR_DEPTH_36:
> >> + /* yes this is correct, the atom define
> is wrong */
> >> + clk.sPCLKInput.ucMiscInfo |=
> PIXEL_CLOCK_V5_MISC_HDMI_30BPP;
> >> + break;
> >> + default:
> >> + break;
> >> + }
> >>
> >> if (EXEC_BIOS_CMD_TABLE(SetPixelClock, clk))
> >> result = BP_RESULT_OK;
> >> @@ -1135,6 +1182,20 @@ static enum bp_result set_pixel_clock_v6(
> >> * driver choose program it itself, i.e. here we pass
> required
> >> * target rate that includes deep color.
> >> */
> >> + if (bp_params->signal_type == SIGNAL_TYPE_HDMI_TYPE_A)
> >> + switch (bp_params->color_depth) {
> >> + case TRANSMITTER_COLOR_DEPTH_30:
> >> + clk.sPCLKInput.ucMiscInfo |=
> PIXEL_CLOCK_V6_MISC_HDMI_30BPP_V6;
> >> + break;
> >> + case TRANSMITTER_COLOR_DEPTH_36:
> >> + clk.sPCLKInput.ucMiscInfo |=
> PIXEL_CLOCK_V6_MISC_HDMI_36BPP_V6;
> >> + break;
> >> + case TRANSMITTER_COLOR_DEPTH_48:
> >> + clk.sPCLKInput.ucMiscInfo |=
> PIXEL_CLOCK_V6_MISC_HDMI_48BPP;
> >> + break;
> >> + default:
> >> + break;
> >> + }
> >>
> >> if (EXEC_BIOS_CMD_TABLE(SetPixelClock, clk))
> >> result = BP_RESULT_OK;
> >> diff --git a/drivers/gpu/drm/amd/display/dc/dce/dce_clock_source.c
> b/drivers/gpu/drm/amd/display/dc/dce/dce_clock_source.c
> >> index fb733f573715..466f8f5803c9 100644
> >> --- a/drivers/gpu/drm/amd/display/dc/dce/dce_clock_source.c
> >> +++ b/drivers/gpu/drm/amd/display/dc/dce/dce_clock_source.c
> >> @@ -871,6 +871,20 @@ static bool dce110_program_pix_clk(
> >> bp_pc_params.flags.SET_EXTERNAL_REF_DIV_SRC =
> >> pll_settings->use_external_clk;
> >>
> >> + switch (pix_clk_params->color_depth) {
> >> + case COLOR_DEPTH_101010:
> >> + bp_pc_params.color_depth = TRANSMITTER_COLOR_DEPTH_30;
> >> + break;
> >> + case COLOR_DEPTH_121212:
> >> + bp_pc_params.color_depth = TRANSMITTER_COLOR_DEPTH_36;
> >> + break;
> >> + case COLOR_DEPTH_161616:
> >> + bp_pc_params.color_depth = TRANSMITTER_COLOR_DEPTH_48;
> >> + break;
> >> + default:
> >> + break;
> >> + }
> >> +
> >> if (clk_src->bios->funcs->set_pixel_clock(
> >> clk_src->bios, &bp_pc_params) != BP_RESULT_OK)
> >> return false;
> >> diff --git a/drivers/gpu/drm/amd/display/dc/dce/dce_stream_encoder.c
> b/drivers/gpu/drm/amd/display/dc/dce/dce_stream_encoder.c
> >> index ada57f745fd7..19e380e0a330 100644
> >> --- a/drivers/gpu/drm/amd/display/dc/dce/dce_stream_encoder.c
> >> +++ b/drivers/gpu/drm/amd/display/dc/dce/dce_stream_encoder.c
> >> @@ -564,6 +564,7 @@ static void
> dce110_stream_encoder_hdmi_set_stream_attribute(
> >> cntl.enable_dp_audio = enable_audio;
> >> cntl.pixel_clock = actual_pix_clk_khz;
> >> cntl.lanes_number = LANE_COUNT_FOUR;
> >> + cntl.color_depth = crtc_timing->display_color_depth;
> >>
> >> if (enc110->base.bp->funcs->encoder_control(
> >> enc110->base.bp, &cntl) != BP_RESULT_OK)
> >> --
> >> 2.25.1
> >>
> >> _______________________________________________
> >> dri-devel mailing list
> >> dri-devel@lists.freedesktop.org
> >>
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Fdri-devel&data=04%7C01%7Cnicholas.kazlauskas%40amd.com%7C598b2b5e841940b8c27708d8c15aa80d%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637471942408643835%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=b9C3Xi%2F2RmNsGyGUN5oBF8U%2BuGyt2w4jUZ2dK8NM4AY%3D&reserved=0
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> >
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Fdri-devel&data=04%7C01%7Cnicholas.kazlauskas%40amd.com%7C598b2b5e841940b8c27708d8c15aa80d%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637471942408643835%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=b9C3Xi%2F2RmNsGyGUN5oBF8U%2BuGyt2w4jUZ2dK8NM4AY%3D&reserved=0
> >
>
>
[-- Attachment #1.2: Type: text/html, Size: 16009 bytes --]
[-- Attachment #2: Type: text/plain, Size: 160 bytes --]
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] drm/amd/display: Fix HDMI deep color output for DCE 6-11.
@ 2021-01-25 19:24 ` Mario Kleiner
0 siblings, 0 replies; 14+ messages in thread
From: Mario Kleiner @ 2021-01-25 19:24 UTC (permalink / raw)
To: Kazlauskas, Nicholas
Cc: Alex Deucher, Deucher, Alexander, Maling list - DRI developers,
amd-gfx list
[-- Attachment #1.1: Type: text/plain, Size: 11607 bytes --]
Thanks Alex and Nicholas! Brings quite a bit of extra shiny to those older
asics :)
Nicholas, any thoughts on my cover-letter wrt. why a similar patch (that I
wrote and tested to no good or bad effect) not seem to be needed on DCN,
and probably not DCE-11.2+ either? Is what is left in DC for those asic's
just dead code? My Atombios disassembly sort of pointed into that
direction, but reading disassembly is not easy on the brain, and my brain
was getting quite mushy towards the end of digging through all the code. So
some official statement would add peace of mind on my side. Is there a
certain DCE version at which your team starts validating output precision /
HDR etc. on hw?
Thanks,
-mario
On Mon, Jan 25, 2021 at 8:16 PM Kazlauskas, Nicholas <
nicholas.kazlauskas@amd.com> wrote:
> On 2021-01-25 12:57 p.m., Alex Deucher wrote:
> > On Thu, Jan 21, 2021 at 1:17 AM Mario Kleiner
> > <mario.kleiner.de@gmail.com> wrote:
> >>
> >> This fixes corrupted display output in HDMI deep color
> >> 10/12 bpc mode at least as observed on AMD Mullins, DCE-8.3.
> >>
> >> It will hopefully also provide fixes for other DCE's up to
> >> DCE-11, assuming those will need similar fixes, but i could
> >> not test that for HDMI due to lack of suitable hw, so viewer
> >> discretion is advised.
> >>
> >> dce110_stream_encoder_hdmi_set_stream_attribute() is used for
> >> HDMI setup on all DCE's and is missing color_depth assignment.
> >>
> >> dce110_program_pix_clk() is used for pixel clock setup on HDMI
> >> for DCE 6-11, and is missing color_depth assignment.
> >>
> >> Additionally some of the underlying Atombios specific encoder
> >> and pixelclock setup functions are missing code which is in
> >> the classic amdgpu kms modesetting path and the in the radeon
> >> kms driver for DCE6/DCE8.
> >>
> >> encoder_control_digx_v3() - Was missing setup code wrt. amdgpu
> >> and radeon kms classic drivers. Added here, but untested due to
> >> lack of suitable test hw.
> >>
> >> encoder_control_digx_v4() - Added missing setup code.
> >> Successfully tested on AMD mullins / DCE-8.3 with HDMI deep color
> >> output at 10 bpc and 12 bpc.
> >>
> >> Note that encoder_control_digx_v5() has proper setup code in place
> >> and is used, e.g., by DCE-11.2, but this code wasn't used for deep
> >> color setup due to the missing cntl.color_depth setup in the calling
> >> function for HDMI.
> >>
> >> set_pixel_clock_v5() - Missing setup code wrt. classic amdgpu/radeon
> >> kms. Added here, but untested due to lack of hw.
> >>
> >> set_pixel_clock_v6() - Missing setup code added. Successfully tested
> >> on AMD mullins DCE-8.3. This fixes corrupted display output at HDMI
> >> deep color output with 10 bpc or 12 bpc.
> >>
> >> Fixes: 4562236b3bc0 ("drm/amd/dc: Add dc display driver (v2)")
> >>
> >> Signed-off-by: Mario Kleiner <mario.kleiner.de@gmail.com>
> >> Cc: Harry Wentland <harry.wentland@amd.com>
> >
> > These make sense. I've applied the series. I'll let the display guys
> > gauge the other points in your cover letter.
> >
> > Alex
>
> I don't have any concerns with this patch.
>
> Even though it's already applied feel free to have my:
>
> Reviewed-by: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>
>
> Regards,
> Nicholas Kazlauskas
>
> >
> >
> >> ---
> >> .../drm/amd/display/dc/bios/command_table.c | 61 +++++++++++++++++++
> >> .../drm/amd/display/dc/dce/dce_clock_source.c | 14 +++++
> >> .../amd/display/dc/dce/dce_stream_encoder.c | 1 +
> >> 3 files changed, 76 insertions(+)
> >>
> >> diff --git a/drivers/gpu/drm/amd/display/dc/bios/command_table.c
> b/drivers/gpu/drm/amd/display/dc/bios/command_table.c
> >> index 070459e3e407..afc10b954ffa 100644
> >> --- a/drivers/gpu/drm/amd/display/dc/bios/command_table.c
> >> +++ b/drivers/gpu/drm/amd/display/dc/bios/command_table.c
> >> @@ -245,6 +245,23 @@ static enum bp_result encoder_control_digx_v3(
> >> cntl->enable_dp_audio);
> >> params.ucLaneNum = (uint8_t)(cntl->lanes_number);
> >>
> >> + switch (cntl->color_depth) {
> >> + case COLOR_DEPTH_888:
> >> + params.ucBitPerColor = PANEL_8BIT_PER_COLOR;
> >> + break;
> >> + case COLOR_DEPTH_101010:
> >> + params.ucBitPerColor = PANEL_10BIT_PER_COLOR;
> >> + break;
> >> + case COLOR_DEPTH_121212:
> >> + params.ucBitPerColor = PANEL_12BIT_PER_COLOR;
> >> + break;
> >> + case COLOR_DEPTH_161616:
> >> + params.ucBitPerColor = PANEL_16BIT_PER_COLOR;
> >> + break;
> >> + default:
> >> + break;
> >> + }
> >> +
> >> if (EXEC_BIOS_CMD_TABLE(DIGxEncoderControl, params))
> >> result = BP_RESULT_OK;
> >>
> >> @@ -274,6 +291,23 @@ static enum bp_result encoder_control_digx_v4(
> >> cntl->enable_dp_audio));
> >> params.ucLaneNum = (uint8_t)(cntl->lanes_number);
> >>
> >> + switch (cntl->color_depth) {
> >> + case COLOR_DEPTH_888:
> >> + params.ucBitPerColor = PANEL_8BIT_PER_COLOR;
> >> + break;
> >> + case COLOR_DEPTH_101010:
> >> + params.ucBitPerColor = PANEL_10BIT_PER_COLOR;
> >> + break;
> >> + case COLOR_DEPTH_121212:
> >> + params.ucBitPerColor = PANEL_12BIT_PER_COLOR;
> >> + break;
> >> + case COLOR_DEPTH_161616:
> >> + params.ucBitPerColor = PANEL_16BIT_PER_COLOR;
> >> + break;
> >> + default:
> >> + break;
> >> + }
> >> +
> >> if (EXEC_BIOS_CMD_TABLE(DIGxEncoderControl, params))
> >> result = BP_RESULT_OK;
> >>
> >> @@ -1057,6 +1091,19 @@ static enum bp_result set_pixel_clock_v5(
> >> * driver choose program it itself, i.e. here we
> program it
> >> * to 888 by default.
> >> */
> >> + if (bp_params->signal_type == SIGNAL_TYPE_HDMI_TYPE_A)
> >> + switch (bp_params->color_depth) {
> >> + case TRANSMITTER_COLOR_DEPTH_30:
> >> + /* yes this is correct, the atom define
> is wrong */
> >> + clk.sPCLKInput.ucMiscInfo |=
> PIXEL_CLOCK_V5_MISC_HDMI_32BPP;
> >> + break;
> >> + case TRANSMITTER_COLOR_DEPTH_36:
> >> + /* yes this is correct, the atom define
> is wrong */
> >> + clk.sPCLKInput.ucMiscInfo |=
> PIXEL_CLOCK_V5_MISC_HDMI_30BPP;
> >> + break;
> >> + default:
> >> + break;
> >> + }
> >>
> >> if (EXEC_BIOS_CMD_TABLE(SetPixelClock, clk))
> >> result = BP_RESULT_OK;
> >> @@ -1135,6 +1182,20 @@ static enum bp_result set_pixel_clock_v6(
> >> * driver choose program it itself, i.e. here we pass
> required
> >> * target rate that includes deep color.
> >> */
> >> + if (bp_params->signal_type == SIGNAL_TYPE_HDMI_TYPE_A)
> >> + switch (bp_params->color_depth) {
> >> + case TRANSMITTER_COLOR_DEPTH_30:
> >> + clk.sPCLKInput.ucMiscInfo |=
> PIXEL_CLOCK_V6_MISC_HDMI_30BPP_V6;
> >> + break;
> >> + case TRANSMITTER_COLOR_DEPTH_36:
> >> + clk.sPCLKInput.ucMiscInfo |=
> PIXEL_CLOCK_V6_MISC_HDMI_36BPP_V6;
> >> + break;
> >> + case TRANSMITTER_COLOR_DEPTH_48:
> >> + clk.sPCLKInput.ucMiscInfo |=
> PIXEL_CLOCK_V6_MISC_HDMI_48BPP;
> >> + break;
> >> + default:
> >> + break;
> >> + }
> >>
> >> if (EXEC_BIOS_CMD_TABLE(SetPixelClock, clk))
> >> result = BP_RESULT_OK;
> >> diff --git a/drivers/gpu/drm/amd/display/dc/dce/dce_clock_source.c
> b/drivers/gpu/drm/amd/display/dc/dce/dce_clock_source.c
> >> index fb733f573715..466f8f5803c9 100644
> >> --- a/drivers/gpu/drm/amd/display/dc/dce/dce_clock_source.c
> >> +++ b/drivers/gpu/drm/amd/display/dc/dce/dce_clock_source.c
> >> @@ -871,6 +871,20 @@ static bool dce110_program_pix_clk(
> >> bp_pc_params.flags.SET_EXTERNAL_REF_DIV_SRC =
> >> pll_settings->use_external_clk;
> >>
> >> + switch (pix_clk_params->color_depth) {
> >> + case COLOR_DEPTH_101010:
> >> + bp_pc_params.color_depth = TRANSMITTER_COLOR_DEPTH_30;
> >> + break;
> >> + case COLOR_DEPTH_121212:
> >> + bp_pc_params.color_depth = TRANSMITTER_COLOR_DEPTH_36;
> >> + break;
> >> + case COLOR_DEPTH_161616:
> >> + bp_pc_params.color_depth = TRANSMITTER_COLOR_DEPTH_48;
> >> + break;
> >> + default:
> >> + break;
> >> + }
> >> +
> >> if (clk_src->bios->funcs->set_pixel_clock(
> >> clk_src->bios, &bp_pc_params) != BP_RESULT_OK)
> >> return false;
> >> diff --git a/drivers/gpu/drm/amd/display/dc/dce/dce_stream_encoder.c
> b/drivers/gpu/drm/amd/display/dc/dce/dce_stream_encoder.c
> >> index ada57f745fd7..19e380e0a330 100644
> >> --- a/drivers/gpu/drm/amd/display/dc/dce/dce_stream_encoder.c
> >> +++ b/drivers/gpu/drm/amd/display/dc/dce/dce_stream_encoder.c
> >> @@ -564,6 +564,7 @@ static void
> dce110_stream_encoder_hdmi_set_stream_attribute(
> >> cntl.enable_dp_audio = enable_audio;
> >> cntl.pixel_clock = actual_pix_clk_khz;
> >> cntl.lanes_number = LANE_COUNT_FOUR;
> >> + cntl.color_depth = crtc_timing->display_color_depth;
> >>
> >> if (enc110->base.bp->funcs->encoder_control(
> >> enc110->base.bp, &cntl) != BP_RESULT_OK)
> >> --
> >> 2.25.1
> >>
> >> _______________________________________________
> >> dri-devel mailing list
> >> dri-devel@lists.freedesktop.org
> >>
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Fdri-devel&data=04%7C01%7Cnicholas.kazlauskas%40amd.com%7C598b2b5e841940b8c27708d8c15aa80d%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637471942408643835%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=b9C3Xi%2F2RmNsGyGUN5oBF8U%2BuGyt2w4jUZ2dK8NM4AY%3D&reserved=0
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> >
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Fdri-devel&data=04%7C01%7Cnicholas.kazlauskas%40amd.com%7C598b2b5e841940b8c27708d8c15aa80d%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637471942408643835%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=b9C3Xi%2F2RmNsGyGUN5oBF8U%2BuGyt2w4jUZ2dK8NM4AY%3D&reserved=0
> >
>
>
[-- Attachment #1.2: Type: text/html, Size: 16009 bytes --]
[-- Attachment #2: Type: text/plain, Size: 154 bytes --]
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] drm/amd/display: Fix HDMI deep color output for DCE 6-11.
2021-01-25 19:24 ` Mario Kleiner
@ 2021-02-10 21:06 ` Mario Kleiner
-1 siblings, 0 replies; 14+ messages in thread
From: Mario Kleiner @ 2021-02-10 21:06 UTC (permalink / raw)
To: Kazlauskas, Nicholas
Cc: Deucher, Alexander, Maling list - DRI developers, amd-gfx list
[-- Attachment #1.1: Type: text/plain, Size: 11999 bytes --]
Ping. Any bit of info appreciated wrt. the DCE-11.2+ situation.
-mario
On Mon, Jan 25, 2021 at 8:24 PM Mario Kleiner <mario.kleiner.de@gmail.com>
wrote:
> Thanks Alex and Nicholas! Brings quite a bit of extra shiny to those older
> asics :)
>
> Nicholas, any thoughts on my cover-letter wrt. why a similar patch (that I
> wrote and tested to no good or bad effect) not seem to be needed on DCN,
> and probably not DCE-11.2+ either? Is what is left in DC for those asic's
> just dead code? My Atombios disassembly sort of pointed into that
> direction, but reading disassembly is not easy on the brain, and my brain
> was getting quite mushy towards the end of digging through all the code. So
> some official statement would add peace of mind on my side. Is there a
> certain DCE version at which your team starts validating output precision /
> HDR etc. on hw?
>
> Thanks,
> -mario
>
>
> On Mon, Jan 25, 2021 at 8:16 PM Kazlauskas, Nicholas <
> nicholas.kazlauskas@amd.com> wrote:
>
>> On 2021-01-25 12:57 p.m., Alex Deucher wrote:
>> > On Thu, Jan 21, 2021 at 1:17 AM Mario Kleiner
>> > <mario.kleiner.de@gmail.com> wrote:
>> >>
>> >> This fixes corrupted display output in HDMI deep color
>> >> 10/12 bpc mode at least as observed on AMD Mullins, DCE-8.3.
>> >>
>> >> It will hopefully also provide fixes for other DCE's up to
>> >> DCE-11, assuming those will need similar fixes, but i could
>> >> not test that for HDMI due to lack of suitable hw, so viewer
>> >> discretion is advised.
>> >>
>> >> dce110_stream_encoder_hdmi_set_stream_attribute() is used for
>> >> HDMI setup on all DCE's and is missing color_depth assignment.
>> >>
>> >> dce110_program_pix_clk() is used for pixel clock setup on HDMI
>> >> for DCE 6-11, and is missing color_depth assignment.
>> >>
>> >> Additionally some of the underlying Atombios specific encoder
>> >> and pixelclock setup functions are missing code which is in
>> >> the classic amdgpu kms modesetting path and the in the radeon
>> >> kms driver for DCE6/DCE8.
>> >>
>> >> encoder_control_digx_v3() - Was missing setup code wrt. amdgpu
>> >> and radeon kms classic drivers. Added here, but untested due to
>> >> lack of suitable test hw.
>> >>
>> >> encoder_control_digx_v4() - Added missing setup code.
>> >> Successfully tested on AMD mullins / DCE-8.3 with HDMI deep color
>> >> output at 10 bpc and 12 bpc.
>> >>
>> >> Note that encoder_control_digx_v5() has proper setup code in place
>> >> and is used, e.g., by DCE-11.2, but this code wasn't used for deep
>> >> color setup due to the missing cntl.color_depth setup in the calling
>> >> function for HDMI.
>> >>
>> >> set_pixel_clock_v5() - Missing setup code wrt. classic amdgpu/radeon
>> >> kms. Added here, but untested due to lack of hw.
>> >>
>> >> set_pixel_clock_v6() - Missing setup code added. Successfully tested
>> >> on AMD mullins DCE-8.3. This fixes corrupted display output at HDMI
>> >> deep color output with 10 bpc or 12 bpc.
>> >>
>> >> Fixes: 4562236b3bc0 ("drm/amd/dc: Add dc display driver (v2)")
>> >>
>> >> Signed-off-by: Mario Kleiner <mario.kleiner.de@gmail.com>
>> >> Cc: Harry Wentland <harry.wentland@amd.com>
>> >
>> > These make sense. I've applied the series. I'll let the display guys
>> > gauge the other points in your cover letter.
>> >
>> > Alex
>>
>> I don't have any concerns with this patch.
>>
>> Even though it's already applied feel free to have my:
>>
>> Reviewed-by: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>
>>
>> Regards,
>> Nicholas Kazlauskas
>>
>> >
>> >
>> >> ---
>> >> .../drm/amd/display/dc/bios/command_table.c | 61
>> +++++++++++++++++++
>> >> .../drm/amd/display/dc/dce/dce_clock_source.c | 14 +++++
>> >> .../amd/display/dc/dce/dce_stream_encoder.c | 1 +
>> >> 3 files changed, 76 insertions(+)
>> >>
>> >> diff --git a/drivers/gpu/drm/amd/display/dc/bios/command_table.c
>> b/drivers/gpu/drm/amd/display/dc/bios/command_table.c
>> >> index 070459e3e407..afc10b954ffa 100644
>> >> --- a/drivers/gpu/drm/amd/display/dc/bios/command_table.c
>> >> +++ b/drivers/gpu/drm/amd/display/dc/bios/command_table.c
>> >> @@ -245,6 +245,23 @@ static enum bp_result encoder_control_digx_v3(
>> >> cntl->enable_dp_audio);
>> >> params.ucLaneNum = (uint8_t)(cntl->lanes_number);
>> >>
>> >> + switch (cntl->color_depth) {
>> >> + case COLOR_DEPTH_888:
>> >> + params.ucBitPerColor = PANEL_8BIT_PER_COLOR;
>> >> + break;
>> >> + case COLOR_DEPTH_101010:
>> >> + params.ucBitPerColor = PANEL_10BIT_PER_COLOR;
>> >> + break;
>> >> + case COLOR_DEPTH_121212:
>> >> + params.ucBitPerColor = PANEL_12BIT_PER_COLOR;
>> >> + break;
>> >> + case COLOR_DEPTH_161616:
>> >> + params.ucBitPerColor = PANEL_16BIT_PER_COLOR;
>> >> + break;
>> >> + default:
>> >> + break;
>> >> + }
>> >> +
>> >> if (EXEC_BIOS_CMD_TABLE(DIGxEncoderControl, params))
>> >> result = BP_RESULT_OK;
>> >>
>> >> @@ -274,6 +291,23 @@ static enum bp_result encoder_control_digx_v4(
>> >> cntl->enable_dp_audio));
>> >> params.ucLaneNum = (uint8_t)(cntl->lanes_number);
>> >>
>> >> + switch (cntl->color_depth) {
>> >> + case COLOR_DEPTH_888:
>> >> + params.ucBitPerColor = PANEL_8BIT_PER_COLOR;
>> >> + break;
>> >> + case COLOR_DEPTH_101010:
>> >> + params.ucBitPerColor = PANEL_10BIT_PER_COLOR;
>> >> + break;
>> >> + case COLOR_DEPTH_121212:
>> >> + params.ucBitPerColor = PANEL_12BIT_PER_COLOR;
>> >> + break;
>> >> + case COLOR_DEPTH_161616:
>> >> + params.ucBitPerColor = PANEL_16BIT_PER_COLOR;
>> >> + break;
>> >> + default:
>> >> + break;
>> >> + }
>> >> +
>> >> if (EXEC_BIOS_CMD_TABLE(DIGxEncoderControl, params))
>> >> result = BP_RESULT_OK;
>> >>
>> >> @@ -1057,6 +1091,19 @@ static enum bp_result set_pixel_clock_v5(
>> >> * driver choose program it itself, i.e. here we
>> program it
>> >> * to 888 by default.
>> >> */
>> >> + if (bp_params->signal_type == SIGNAL_TYPE_HDMI_TYPE_A)
>> >> + switch (bp_params->color_depth) {
>> >> + case TRANSMITTER_COLOR_DEPTH_30:
>> >> + /* yes this is correct, the atom
>> define is wrong */
>> >> + clk.sPCLKInput.ucMiscInfo |=
>> PIXEL_CLOCK_V5_MISC_HDMI_32BPP;
>> >> + break;
>> >> + case TRANSMITTER_COLOR_DEPTH_36:
>> >> + /* yes this is correct, the atom
>> define is wrong */
>> >> + clk.sPCLKInput.ucMiscInfo |=
>> PIXEL_CLOCK_V5_MISC_HDMI_30BPP;
>> >> + break;
>> >> + default:
>> >> + break;
>> >> + }
>> >>
>> >> if (EXEC_BIOS_CMD_TABLE(SetPixelClock, clk))
>> >> result = BP_RESULT_OK;
>> >> @@ -1135,6 +1182,20 @@ static enum bp_result set_pixel_clock_v6(
>> >> * driver choose program it itself, i.e. here we pass
>> required
>> >> * target rate that includes deep color.
>> >> */
>> >> + if (bp_params->signal_type == SIGNAL_TYPE_HDMI_TYPE_A)
>> >> + switch (bp_params->color_depth) {
>> >> + case TRANSMITTER_COLOR_DEPTH_30:
>> >> + clk.sPCLKInput.ucMiscInfo |=
>> PIXEL_CLOCK_V6_MISC_HDMI_30BPP_V6;
>> >> + break;
>> >> + case TRANSMITTER_COLOR_DEPTH_36:
>> >> + clk.sPCLKInput.ucMiscInfo |=
>> PIXEL_CLOCK_V6_MISC_HDMI_36BPP_V6;
>> >> + break;
>> >> + case TRANSMITTER_COLOR_DEPTH_48:
>> >> + clk.sPCLKInput.ucMiscInfo |=
>> PIXEL_CLOCK_V6_MISC_HDMI_48BPP;
>> >> + break;
>> >> + default:
>> >> + break;
>> >> + }
>> >>
>> >> if (EXEC_BIOS_CMD_TABLE(SetPixelClock, clk))
>> >> result = BP_RESULT_OK;
>> >> diff --git a/drivers/gpu/drm/amd/display/dc/dce/dce_clock_source.c
>> b/drivers/gpu/drm/amd/display/dc/dce/dce_clock_source.c
>> >> index fb733f573715..466f8f5803c9 100644
>> >> --- a/drivers/gpu/drm/amd/display/dc/dce/dce_clock_source.c
>> >> +++ b/drivers/gpu/drm/amd/display/dc/dce/dce_clock_source.c
>> >> @@ -871,6 +871,20 @@ static bool dce110_program_pix_clk(
>> >> bp_pc_params.flags.SET_EXTERNAL_REF_DIV_SRC =
>> >>
>> pll_settings->use_external_clk;
>> >>
>> >> + switch (pix_clk_params->color_depth) {
>> >> + case COLOR_DEPTH_101010:
>> >> + bp_pc_params.color_depth = TRANSMITTER_COLOR_DEPTH_30;
>> >> + break;
>> >> + case COLOR_DEPTH_121212:
>> >> + bp_pc_params.color_depth = TRANSMITTER_COLOR_DEPTH_36;
>> >> + break;
>> >> + case COLOR_DEPTH_161616:
>> >> + bp_pc_params.color_depth = TRANSMITTER_COLOR_DEPTH_48;
>> >> + break;
>> >> + default:
>> >> + break;
>> >> + }
>> >> +
>> >> if (clk_src->bios->funcs->set_pixel_clock(
>> >> clk_src->bios, &bp_pc_params) != BP_RESULT_OK)
>> >> return false;
>> >> diff --git a/drivers/gpu/drm/amd/display/dc/dce/dce_stream_encoder.c
>> b/drivers/gpu/drm/amd/display/dc/dce/dce_stream_encoder.c
>> >> index ada57f745fd7..19e380e0a330 100644
>> >> --- a/drivers/gpu/drm/amd/display/dc/dce/dce_stream_encoder.c
>> >> +++ b/drivers/gpu/drm/amd/display/dc/dce/dce_stream_encoder.c
>> >> @@ -564,6 +564,7 @@ static void
>> dce110_stream_encoder_hdmi_set_stream_attribute(
>> >> cntl.enable_dp_audio = enable_audio;
>> >> cntl.pixel_clock = actual_pix_clk_khz;
>> >> cntl.lanes_number = LANE_COUNT_FOUR;
>> >> + cntl.color_depth = crtc_timing->display_color_depth;
>> >>
>> >> if (enc110->base.bp->funcs->encoder_control(
>> >> enc110->base.bp, &cntl) != BP_RESULT_OK)
>> >> --
>> >> 2.25.1
>> >>
>> >> _______________________________________________
>> >> dri-devel mailing list
>> >> dri-devel@lists.freedesktop.org
>> >>
>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Fdri-devel&data=04%7C01%7Cnicholas.kazlauskas%40amd.com%7C598b2b5e841940b8c27708d8c15aa80d%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637471942408643835%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=b9C3Xi%2F2RmNsGyGUN5oBF8U%2BuGyt2w4jUZ2dK8NM4AY%3D&reserved=0
>> > _______________________________________________
>> > dri-devel mailing list
>> > dri-devel@lists.freedesktop.org
>> >
>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Fdri-devel&data=04%7C01%7Cnicholas.kazlauskas%40amd.com%7C598b2b5e841940b8c27708d8c15aa80d%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637471942408643835%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=b9C3Xi%2F2RmNsGyGUN5oBF8U%2BuGyt2w4jUZ2dK8NM4AY%3D&reserved=0
>> >
>>
>>
[-- Attachment #1.2: Type: text/html, Size: 16476 bytes --]
[-- Attachment #2: Type: text/plain, Size: 160 bytes --]
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] drm/amd/display: Fix HDMI deep color output for DCE 6-11.
@ 2021-02-10 21:06 ` Mario Kleiner
0 siblings, 0 replies; 14+ messages in thread
From: Mario Kleiner @ 2021-02-10 21:06 UTC (permalink / raw)
To: Kazlauskas, Nicholas
Cc: Alex Deucher, Deucher, Alexander, Maling list - DRI developers,
amd-gfx list
[-- Attachment #1.1: Type: text/plain, Size: 11999 bytes --]
Ping. Any bit of info appreciated wrt. the DCE-11.2+ situation.
-mario
On Mon, Jan 25, 2021 at 8:24 PM Mario Kleiner <mario.kleiner.de@gmail.com>
wrote:
> Thanks Alex and Nicholas! Brings quite a bit of extra shiny to those older
> asics :)
>
> Nicholas, any thoughts on my cover-letter wrt. why a similar patch (that I
> wrote and tested to no good or bad effect) not seem to be needed on DCN,
> and probably not DCE-11.2+ either? Is what is left in DC for those asic's
> just dead code? My Atombios disassembly sort of pointed into that
> direction, but reading disassembly is not easy on the brain, and my brain
> was getting quite mushy towards the end of digging through all the code. So
> some official statement would add peace of mind on my side. Is there a
> certain DCE version at which your team starts validating output precision /
> HDR etc. on hw?
>
> Thanks,
> -mario
>
>
> On Mon, Jan 25, 2021 at 8:16 PM Kazlauskas, Nicholas <
> nicholas.kazlauskas@amd.com> wrote:
>
>> On 2021-01-25 12:57 p.m., Alex Deucher wrote:
>> > On Thu, Jan 21, 2021 at 1:17 AM Mario Kleiner
>> > <mario.kleiner.de@gmail.com> wrote:
>> >>
>> >> This fixes corrupted display output in HDMI deep color
>> >> 10/12 bpc mode at least as observed on AMD Mullins, DCE-8.3.
>> >>
>> >> It will hopefully also provide fixes for other DCE's up to
>> >> DCE-11, assuming those will need similar fixes, but i could
>> >> not test that for HDMI due to lack of suitable hw, so viewer
>> >> discretion is advised.
>> >>
>> >> dce110_stream_encoder_hdmi_set_stream_attribute() is used for
>> >> HDMI setup on all DCE's and is missing color_depth assignment.
>> >>
>> >> dce110_program_pix_clk() is used for pixel clock setup on HDMI
>> >> for DCE 6-11, and is missing color_depth assignment.
>> >>
>> >> Additionally some of the underlying Atombios specific encoder
>> >> and pixelclock setup functions are missing code which is in
>> >> the classic amdgpu kms modesetting path and the in the radeon
>> >> kms driver for DCE6/DCE8.
>> >>
>> >> encoder_control_digx_v3() - Was missing setup code wrt. amdgpu
>> >> and radeon kms classic drivers. Added here, but untested due to
>> >> lack of suitable test hw.
>> >>
>> >> encoder_control_digx_v4() - Added missing setup code.
>> >> Successfully tested on AMD mullins / DCE-8.3 with HDMI deep color
>> >> output at 10 bpc and 12 bpc.
>> >>
>> >> Note that encoder_control_digx_v5() has proper setup code in place
>> >> and is used, e.g., by DCE-11.2, but this code wasn't used for deep
>> >> color setup due to the missing cntl.color_depth setup in the calling
>> >> function for HDMI.
>> >>
>> >> set_pixel_clock_v5() - Missing setup code wrt. classic amdgpu/radeon
>> >> kms. Added here, but untested due to lack of hw.
>> >>
>> >> set_pixel_clock_v6() - Missing setup code added. Successfully tested
>> >> on AMD mullins DCE-8.3. This fixes corrupted display output at HDMI
>> >> deep color output with 10 bpc or 12 bpc.
>> >>
>> >> Fixes: 4562236b3bc0 ("drm/amd/dc: Add dc display driver (v2)")
>> >>
>> >> Signed-off-by: Mario Kleiner <mario.kleiner.de@gmail.com>
>> >> Cc: Harry Wentland <harry.wentland@amd.com>
>> >
>> > These make sense. I've applied the series. I'll let the display guys
>> > gauge the other points in your cover letter.
>> >
>> > Alex
>>
>> I don't have any concerns with this patch.
>>
>> Even though it's already applied feel free to have my:
>>
>> Reviewed-by: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>
>>
>> Regards,
>> Nicholas Kazlauskas
>>
>> >
>> >
>> >> ---
>> >> .../drm/amd/display/dc/bios/command_table.c | 61
>> +++++++++++++++++++
>> >> .../drm/amd/display/dc/dce/dce_clock_source.c | 14 +++++
>> >> .../amd/display/dc/dce/dce_stream_encoder.c | 1 +
>> >> 3 files changed, 76 insertions(+)
>> >>
>> >> diff --git a/drivers/gpu/drm/amd/display/dc/bios/command_table.c
>> b/drivers/gpu/drm/amd/display/dc/bios/command_table.c
>> >> index 070459e3e407..afc10b954ffa 100644
>> >> --- a/drivers/gpu/drm/amd/display/dc/bios/command_table.c
>> >> +++ b/drivers/gpu/drm/amd/display/dc/bios/command_table.c
>> >> @@ -245,6 +245,23 @@ static enum bp_result encoder_control_digx_v3(
>> >> cntl->enable_dp_audio);
>> >> params.ucLaneNum = (uint8_t)(cntl->lanes_number);
>> >>
>> >> + switch (cntl->color_depth) {
>> >> + case COLOR_DEPTH_888:
>> >> + params.ucBitPerColor = PANEL_8BIT_PER_COLOR;
>> >> + break;
>> >> + case COLOR_DEPTH_101010:
>> >> + params.ucBitPerColor = PANEL_10BIT_PER_COLOR;
>> >> + break;
>> >> + case COLOR_DEPTH_121212:
>> >> + params.ucBitPerColor = PANEL_12BIT_PER_COLOR;
>> >> + break;
>> >> + case COLOR_DEPTH_161616:
>> >> + params.ucBitPerColor = PANEL_16BIT_PER_COLOR;
>> >> + break;
>> >> + default:
>> >> + break;
>> >> + }
>> >> +
>> >> if (EXEC_BIOS_CMD_TABLE(DIGxEncoderControl, params))
>> >> result = BP_RESULT_OK;
>> >>
>> >> @@ -274,6 +291,23 @@ static enum bp_result encoder_control_digx_v4(
>> >> cntl->enable_dp_audio));
>> >> params.ucLaneNum = (uint8_t)(cntl->lanes_number);
>> >>
>> >> + switch (cntl->color_depth) {
>> >> + case COLOR_DEPTH_888:
>> >> + params.ucBitPerColor = PANEL_8BIT_PER_COLOR;
>> >> + break;
>> >> + case COLOR_DEPTH_101010:
>> >> + params.ucBitPerColor = PANEL_10BIT_PER_COLOR;
>> >> + break;
>> >> + case COLOR_DEPTH_121212:
>> >> + params.ucBitPerColor = PANEL_12BIT_PER_COLOR;
>> >> + break;
>> >> + case COLOR_DEPTH_161616:
>> >> + params.ucBitPerColor = PANEL_16BIT_PER_COLOR;
>> >> + break;
>> >> + default:
>> >> + break;
>> >> + }
>> >> +
>> >> if (EXEC_BIOS_CMD_TABLE(DIGxEncoderControl, params))
>> >> result = BP_RESULT_OK;
>> >>
>> >> @@ -1057,6 +1091,19 @@ static enum bp_result set_pixel_clock_v5(
>> >> * driver choose program it itself, i.e. here we
>> program it
>> >> * to 888 by default.
>> >> */
>> >> + if (bp_params->signal_type == SIGNAL_TYPE_HDMI_TYPE_A)
>> >> + switch (bp_params->color_depth) {
>> >> + case TRANSMITTER_COLOR_DEPTH_30:
>> >> + /* yes this is correct, the atom
>> define is wrong */
>> >> + clk.sPCLKInput.ucMiscInfo |=
>> PIXEL_CLOCK_V5_MISC_HDMI_32BPP;
>> >> + break;
>> >> + case TRANSMITTER_COLOR_DEPTH_36:
>> >> + /* yes this is correct, the atom
>> define is wrong */
>> >> + clk.sPCLKInput.ucMiscInfo |=
>> PIXEL_CLOCK_V5_MISC_HDMI_30BPP;
>> >> + break;
>> >> + default:
>> >> + break;
>> >> + }
>> >>
>> >> if (EXEC_BIOS_CMD_TABLE(SetPixelClock, clk))
>> >> result = BP_RESULT_OK;
>> >> @@ -1135,6 +1182,20 @@ static enum bp_result set_pixel_clock_v6(
>> >> * driver choose program it itself, i.e. here we pass
>> required
>> >> * target rate that includes deep color.
>> >> */
>> >> + if (bp_params->signal_type == SIGNAL_TYPE_HDMI_TYPE_A)
>> >> + switch (bp_params->color_depth) {
>> >> + case TRANSMITTER_COLOR_DEPTH_30:
>> >> + clk.sPCLKInput.ucMiscInfo |=
>> PIXEL_CLOCK_V6_MISC_HDMI_30BPP_V6;
>> >> + break;
>> >> + case TRANSMITTER_COLOR_DEPTH_36:
>> >> + clk.sPCLKInput.ucMiscInfo |=
>> PIXEL_CLOCK_V6_MISC_HDMI_36BPP_V6;
>> >> + break;
>> >> + case TRANSMITTER_COLOR_DEPTH_48:
>> >> + clk.sPCLKInput.ucMiscInfo |=
>> PIXEL_CLOCK_V6_MISC_HDMI_48BPP;
>> >> + break;
>> >> + default:
>> >> + break;
>> >> + }
>> >>
>> >> if (EXEC_BIOS_CMD_TABLE(SetPixelClock, clk))
>> >> result = BP_RESULT_OK;
>> >> diff --git a/drivers/gpu/drm/amd/display/dc/dce/dce_clock_source.c
>> b/drivers/gpu/drm/amd/display/dc/dce/dce_clock_source.c
>> >> index fb733f573715..466f8f5803c9 100644
>> >> --- a/drivers/gpu/drm/amd/display/dc/dce/dce_clock_source.c
>> >> +++ b/drivers/gpu/drm/amd/display/dc/dce/dce_clock_source.c
>> >> @@ -871,6 +871,20 @@ static bool dce110_program_pix_clk(
>> >> bp_pc_params.flags.SET_EXTERNAL_REF_DIV_SRC =
>> >>
>> pll_settings->use_external_clk;
>> >>
>> >> + switch (pix_clk_params->color_depth) {
>> >> + case COLOR_DEPTH_101010:
>> >> + bp_pc_params.color_depth = TRANSMITTER_COLOR_DEPTH_30;
>> >> + break;
>> >> + case COLOR_DEPTH_121212:
>> >> + bp_pc_params.color_depth = TRANSMITTER_COLOR_DEPTH_36;
>> >> + break;
>> >> + case COLOR_DEPTH_161616:
>> >> + bp_pc_params.color_depth = TRANSMITTER_COLOR_DEPTH_48;
>> >> + break;
>> >> + default:
>> >> + break;
>> >> + }
>> >> +
>> >> if (clk_src->bios->funcs->set_pixel_clock(
>> >> clk_src->bios, &bp_pc_params) != BP_RESULT_OK)
>> >> return false;
>> >> diff --git a/drivers/gpu/drm/amd/display/dc/dce/dce_stream_encoder.c
>> b/drivers/gpu/drm/amd/display/dc/dce/dce_stream_encoder.c
>> >> index ada57f745fd7..19e380e0a330 100644
>> >> --- a/drivers/gpu/drm/amd/display/dc/dce/dce_stream_encoder.c
>> >> +++ b/drivers/gpu/drm/amd/display/dc/dce/dce_stream_encoder.c
>> >> @@ -564,6 +564,7 @@ static void
>> dce110_stream_encoder_hdmi_set_stream_attribute(
>> >> cntl.enable_dp_audio = enable_audio;
>> >> cntl.pixel_clock = actual_pix_clk_khz;
>> >> cntl.lanes_number = LANE_COUNT_FOUR;
>> >> + cntl.color_depth = crtc_timing->display_color_depth;
>> >>
>> >> if (enc110->base.bp->funcs->encoder_control(
>> >> enc110->base.bp, &cntl) != BP_RESULT_OK)
>> >> --
>> >> 2.25.1
>> >>
>> >> _______________________________________________
>> >> dri-devel mailing list
>> >> dri-devel@lists.freedesktop.org
>> >>
>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Fdri-devel&data=04%7C01%7Cnicholas.kazlauskas%40amd.com%7C598b2b5e841940b8c27708d8c15aa80d%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637471942408643835%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=b9C3Xi%2F2RmNsGyGUN5oBF8U%2BuGyt2w4jUZ2dK8NM4AY%3D&reserved=0
>> > _______________________________________________
>> > dri-devel mailing list
>> > dri-devel@lists.freedesktop.org
>> >
>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Fdri-devel&data=04%7C01%7Cnicholas.kazlauskas%40amd.com%7C598b2b5e841940b8c27708d8c15aa80d%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637471942408643835%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=b9C3Xi%2F2RmNsGyGUN5oBF8U%2BuGyt2w4jUZ2dK8NM4AY%3D&reserved=0
>> >
>>
>>
[-- Attachment #1.2: Type: text/html, Size: 16476 bytes --]
[-- Attachment #2: Type: text/plain, Size: 154 bytes --]
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2021-02-10 21:06 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-21 6:17 Some HDMI deep color output fixes for DC on DCE 6-11 Mario Kleiner
2021-01-21 6:17 ` Mario Kleiner
2021-01-21 6:17 ` [PATCH 1/2] drm/amd/display: Fix 10/12 bpc setup in DCE output bit depth reduction Mario Kleiner
2021-01-21 6:17 ` Mario Kleiner
2021-01-21 6:17 ` [PATCH 2/2] drm/amd/display: Fix HDMI deep color output for DCE 6-11 Mario Kleiner
2021-01-21 6:17 ` Mario Kleiner
2021-01-25 17:57 ` Alex Deucher
2021-01-25 17:57 ` Alex Deucher
2021-01-25 19:16 ` Kazlauskas, Nicholas
2021-01-25 19:16 ` Kazlauskas, Nicholas
2021-01-25 19:24 ` Mario Kleiner
2021-01-25 19:24 ` Mario Kleiner
2021-02-10 21:06 ` Mario Kleiner
2021-02-10 21:06 ` Mario Kleiner
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.