Skip to content

Commit 51cb93a

Browse files
melissawenalexdeucher
authored andcommitted
drm/amd/display: change dc stream color settings only in atomic commit
Don't update DC stream color components during atomic check. The driver will continue validating the new CRTC color state but will not change DC stream color components. The DC stream color state will only be programmed at commit time in the `atomic_setup_commit` stage. It fixes gamma LUT loss reported by KDE users when changing brightness quickly or changing Display settings (such as overscan) with nightlight on and HDR. As KWin can do a test commit with color settings different from those that should be applied in a non-test-only commit, if the driver changes DC stream color state in atomic check, this state can be eventually HW programmed in commit tail, instead of the respective state set by the non-blocking commit. Closes: https://gitlab.freedesktop.org/drm/amd/-/issues/4444 Reported-by: Xaver Hugl <xaver.hugl@gmail.com> Signed-off-by: Melissa Wen <mwen@igalia.com> Reviewed-by: Harry Wentland <harry.wentland@amd.com> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
1 parent 2330437 commit 51cb93a

3 files changed

Lines changed: 66 additions & 24 deletions

File tree

drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11210,7 +11210,7 @@ static int dm_update_crtc_state(struct amdgpu_display_manager *dm,
1121011210
if (dm_new_crtc_state->base.color_mgmt_changed ||
1121111211
dm_old_crtc_state->regamma_tf != dm_new_crtc_state->regamma_tf ||
1121211212
drm_atomic_crtc_needs_modeset(new_crtc_state)) {
11213-
ret = amdgpu_dm_update_crtc_color_mgmt(dm_new_crtc_state);
11213+
ret = amdgpu_dm_check_crtc_color_mgmt(dm_new_crtc_state, true);
1121411214
if (ret)
1121511215
goto fail;
1121611216
}

drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1054,6 +1054,8 @@ void amdgpu_dm_init_color_mod(void);
10541054
int amdgpu_dm_create_color_properties(struct amdgpu_device *adev);
10551055
int amdgpu_dm_verify_lut_sizes(const struct drm_crtc_state *crtc_state);
10561056
int amdgpu_dm_update_crtc_color_mgmt(struct dm_crtc_state *crtc);
1057+
int amdgpu_dm_check_crtc_color_mgmt(struct dm_crtc_state *crtc,
1058+
bool check_only);
10571059
int amdgpu_dm_update_plane_color_mgmt(struct dm_crtc_state *crtc,
10581060
struct drm_plane_state *plane_state,
10591061
struct dc_plane_state *dc_plane_state);

drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c

Lines changed: 63 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -566,12 +566,11 @@ static int __set_output_tf(struct dc_transfer_func *func,
566566
return res ? 0 : -ENOMEM;
567567
}
568568

569-
static int amdgpu_dm_set_atomic_regamma(struct dc_stream_state *stream,
569+
static int amdgpu_dm_set_atomic_regamma(struct dc_transfer_func *out_tf,
570570
const struct drm_color_lut *regamma_lut,
571571
uint32_t regamma_size, bool has_rom,
572572
enum dc_transfer_func_predefined tf)
573573
{
574-
struct dc_transfer_func *out_tf = &stream->out_transfer_func;
575574
int ret = 0;
576575

577576
if (regamma_size || tf != TRANSFER_FUNCTION_LINEAR) {
@@ -885,33 +884,33 @@ int amdgpu_dm_verify_lut_sizes(const struct drm_crtc_state *crtc_state)
885884
}
886885

887886
/**
888-
* amdgpu_dm_update_crtc_color_mgmt: Maps DRM color management to DC stream.
887+
* amdgpu_dm_check_crtc_color_mgmt: Check if DRM color props are programmable by DC.
889888
* @crtc: amdgpu_dm crtc state
889+
* @check_only: only check color state without update dc stream
890890
*
891-
* With no plane level color management properties we're free to use any
892-
* of the HW blocks as long as the CRTC CTM always comes before the
893-
* CRTC RGM and after the CRTC DGM.
894-
*
895-
* - The CRTC RGM block will be placed in the RGM LUT block if it is non-linear.
896-
* - The CRTC DGM block will be placed in the DGM LUT block if it is non-linear.
897-
* - The CRTC CTM will be placed in the gamut remap block if it is non-linear.
891+
* This function just verifies CRTC LUT sizes, if there is enough space for
892+
* output transfer function and if its parameters can be calculated by AMD
893+
* color module. It also adjusts some settings for programming CRTC degamma at
894+
* plane stage, using plane DGM block.
898895
*
899896
* The RGM block is typically more fully featured and accurate across
900897
* all ASICs - DCE can't support a custom non-linear CRTC DGM.
901898
*
902899
* For supporting both plane level color management and CRTC level color
903-
* management at once we have to either restrict the usage of CRTC properties
904-
* or blend adjustments together.
900+
* management at once we have to either restrict the usage of some CRTC
901+
* properties or blend adjustments together.
905902
*
906903
* Returns:
907-
* 0 on success. Error code if setup fails.
904+
* 0 on success. Error code if validation fails.
908905
*/
909-
int amdgpu_dm_update_crtc_color_mgmt(struct dm_crtc_state *crtc)
906+
907+
int amdgpu_dm_check_crtc_color_mgmt(struct dm_crtc_state *crtc,
908+
bool check_only)
910909
{
911910
struct dc_stream_state *stream = crtc->stream;
912911
struct amdgpu_device *adev = drm_to_adev(crtc->base.state->dev);
913912
bool has_rom = adev->asic_type <= CHIP_RAVEN;
914-
struct drm_color_ctm *ctm = NULL;
913+
struct dc_transfer_func *out_tf;
915914
const struct drm_color_lut *degamma_lut, *regamma_lut;
916915
uint32_t degamma_size, regamma_size;
917916
bool has_regamma, has_degamma;
@@ -940,6 +939,14 @@ int amdgpu_dm_update_crtc_color_mgmt(struct dm_crtc_state *crtc)
940939
crtc->cm_has_degamma = false;
941940
crtc->cm_is_degamma_srgb = false;
942941

942+
if (check_only) {
943+
out_tf = kvzalloc(sizeof(*out_tf), GFP_KERNEL);
944+
if (!out_tf)
945+
return -ENOMEM;
946+
} else {
947+
out_tf = &stream->out_transfer_func;
948+
}
949+
943950
/* Setup regamma and degamma. */
944951
if (is_legacy) {
945952
/*
@@ -954,25 +961,21 @@ int amdgpu_dm_update_crtc_color_mgmt(struct dm_crtc_state *crtc)
954961
* inverse color ramp in legacy userspace.
955962
*/
956963
crtc->cm_is_degamma_srgb = true;
957-
stream->out_transfer_func.type = TF_TYPE_DISTRIBUTED_POINTS;
958-
stream->out_transfer_func.tf = TRANSFER_FUNCTION_SRGB;
964+
out_tf->type = TF_TYPE_DISTRIBUTED_POINTS;
965+
out_tf->tf = TRANSFER_FUNCTION_SRGB;
959966
/*
960967
* Note: although we pass has_rom as parameter here, we never
961968
* actually use ROM because the color module only takes the ROM
962969
* path if transfer_func->type == PREDEFINED.
963970
*
964971
* See more in mod_color_calculate_regamma_params()
965972
*/
966-
r = __set_legacy_tf(&stream->out_transfer_func, regamma_lut,
973+
r = __set_legacy_tf(out_tf, regamma_lut,
967974
regamma_size, has_rom);
968-
if (r)
969-
return r;
970975
} else {
971976
regamma_size = has_regamma ? regamma_size : 0;
972-
r = amdgpu_dm_set_atomic_regamma(stream, regamma_lut,
977+
r = amdgpu_dm_set_atomic_regamma(out_tf, regamma_lut,
973978
regamma_size, has_rom, tf);
974-
if (r)
975-
return r;
976979
}
977980

978981
/*
@@ -981,6 +984,43 @@ int amdgpu_dm_update_crtc_color_mgmt(struct dm_crtc_state *crtc)
981984
* have to place the CTM in the OCSC in that case.
982985
*/
983986
crtc->cm_has_degamma = has_degamma;
987+
if (check_only)
988+
kvfree(out_tf);
989+
990+
return r;
991+
}
992+
993+
/**
994+
* amdgpu_dm_update_crtc_color_mgmt: Maps DRM color management to DC stream.
995+
* @crtc: amdgpu_dm crtc state
996+
*
997+
* With no plane level color management properties we're free to use any
998+
* of the HW blocks as long as the CRTC CTM always comes before the
999+
* CRTC RGM and after the CRTC DGM.
1000+
*
1001+
* - The CRTC RGM block will be placed in the RGM LUT block if it is non-linear.
1002+
* - The CRTC DGM block will be placed in the DGM LUT block if it is non-linear.
1003+
* - The CRTC CTM will be placed in the gamut remap block if it is non-linear.
1004+
*
1005+
* The RGM block is typically more fully featured and accurate across
1006+
* all ASICs - DCE can't support a custom non-linear CRTC DGM.
1007+
*
1008+
* For supporting both plane level color management and CRTC level color
1009+
* management at once we have to either restrict the usage of CRTC properties
1010+
* or blend adjustments together.
1011+
*
1012+
* Returns:
1013+
* 0 on success. Error code if setup fails.
1014+
*/
1015+
int amdgpu_dm_update_crtc_color_mgmt(struct dm_crtc_state *crtc)
1016+
{
1017+
struct dc_stream_state *stream = crtc->stream;
1018+
struct drm_color_ctm *ctm = NULL;
1019+
int ret;
1020+
1021+
ret = amdgpu_dm_check_crtc_color_mgmt(crtc, false);
1022+
if (ret)
1023+
return ret;
9841024

9851025
/* Setup CRTC CTM. */
9861026
if (crtc->base.ctm) {

0 commit comments

Comments
 (0)