Commit cf020d49 authored by Nicholas Kazlauskas's avatar Nicholas Kazlauskas Committed by Alex Deucher

drm/amd/display: Rework CRTC color management

[Why]
To prepare for the upcoming DRM plane color management properties
we need to correct a lot of wrong behavior and assumptions made for
CRTC color management.

The documentation added by this commit in amdgpu_dm_color explains
how the HW color pipeline works and its limitations with the DRM
interface.

The current implementation does the following wrong:
- Implicit sRGB DGM when no CRTC DGM is set
- Implicit sRGB RGM when no CRTC RGM is set
- No way to specify a non-linear DGM matrix that produces correct output
- No way to specify a correct RGM when a linear DGM is used

We had workarounds for passing kms_color tests but not all of the
behavior we had wrong was covered by these tests (especially when
it comes to non-linear DGM). Testing both DGM and RGM at the same time
isn't something kms_color tests well either.

[How]
The specifics for how color management works in AMDGPU and the new
behavior can be found by reading the documentation added to
amdgpu_dm_color.c from this patch.

All of the incorrect cases from the old implementation have been
addressed for the atomic interface, but there still a few TODOs for
the legacy one.

Note: this does cause regressions for kms_color@pipe-a-ctm-* over HDMI.

The result looks correct from visual inspection but the CRC no longer
matches. For reference, the test was previously doing the following:

linear degamma -> CTM -> sRGB regamma -> RGB to YUV (709) -> ...

Now the test is doing:

linear degamma -> CTM -> linear regamma -> RGB to YUV (709) -> ...
Signed-off-by: default avatarNicholas Kazlauskas <nicholas.kazlauskas@amd.com>
Reviewed-by: default avatarSun peng Li <Sunpeng.Li@amd.com>
Acked-by: default avatarBhawanpreet Lakha <Bhawanpreet.Lakha@amd.com>
Signed-off-by: default avatarAlex Deucher <alexander.deucher@amd.com>
parent 043f5bb6
...@@ -2878,6 +2878,7 @@ static int fill_dc_plane_attributes(struct amdgpu_device *adev, ...@@ -2878,6 +2878,7 @@ static int fill_dc_plane_attributes(struct amdgpu_device *adev,
struct drm_plane_state *plane_state, struct drm_plane_state *plane_state,
struct drm_crtc_state *crtc_state) struct drm_crtc_state *crtc_state)
{ {
struct dm_crtc_state *dm_crtc_state = to_dm_crtc_state(crtc_state);
const struct amdgpu_framebuffer *amdgpu_fb = const struct amdgpu_framebuffer *amdgpu_fb =
to_amdgpu_framebuffer(plane_state->fb); to_amdgpu_framebuffer(plane_state->fb);
struct dc_scaling_info scaling_info; struct dc_scaling_info scaling_info;
...@@ -2922,13 +2923,11 @@ static int fill_dc_plane_attributes(struct amdgpu_device *adev, ...@@ -2922,13 +2923,11 @@ static int fill_dc_plane_attributes(struct amdgpu_device *adev,
* Always set input transfer function, since plane state is refreshed * Always set input transfer function, since plane state is refreshed
* every time. * every time.
*/ */
ret = amdgpu_dm_set_degamma_lut(crtc_state, dc_plane_state); ret = amdgpu_dm_update_plane_color_mgmt(dm_crtc_state, dc_plane_state);
if (ret) { if (ret)
dc_transfer_func_release(dc_plane_state->in_transfer_func); return ret;
dc_plane_state->in_transfer_func = NULL;
}
return ret; return 0;
} }
static void update_stream_scaling_settings(const struct drm_display_mode *mode, static void update_stream_scaling_settings(const struct drm_display_mode *mode,
...@@ -3517,6 +3516,8 @@ dm_crtc_duplicate_state(struct drm_crtc *crtc) ...@@ -3517,6 +3516,8 @@ dm_crtc_duplicate_state(struct drm_crtc *crtc)
state->vrr_supported = cur->vrr_supported; state->vrr_supported = cur->vrr_supported;
state->freesync_config = cur->freesync_config; state->freesync_config = cur->freesync_config;
state->crc_enabled = cur->crc_enabled; state->crc_enabled = cur->crc_enabled;
state->cm_has_degamma = cur->cm_has_degamma;
state->cm_is_degamma_srgb = cur->cm_is_degamma_srgb;
/* TODO Duplicate dc_stream after objects are stream object is flattened */ /* TODO Duplicate dc_stream after objects are stream object is flattened */
...@@ -5674,8 +5675,18 @@ static void amdgpu_dm_commit_planes(struct drm_atomic_state *state, ...@@ -5674,8 +5675,18 @@ static void amdgpu_dm_commit_planes(struct drm_atomic_state *state,
bundle->stream_update.dst = acrtc_state->stream->dst; bundle->stream_update.dst = acrtc_state->stream->dst;
} }
if (new_pcrtc_state->color_mgmt_changed) if (new_pcrtc_state->color_mgmt_changed) {
bundle->stream_update.out_transfer_func = acrtc_state->stream->out_transfer_func; /*
* TODO: This isn't fully correct since we've actually
* already modified the stream in place.
*/
bundle->stream_update.gamut_remap =
&acrtc_state->stream->gamut_remap_matrix;
bundle->stream_update.output_csc_transform =
&acrtc_state->stream->csc_color_matrix;
bundle->stream_update.out_transfer_func =
acrtc_state->stream->out_transfer_func;
}
acrtc_state->stream->abm_level = acrtc_state->abm_level; acrtc_state->stream->abm_level = acrtc_state->abm_level;
if (acrtc_state->abm_level != dm_old_crtc_state->abm_level) if (acrtc_state->abm_level != dm_old_crtc_state->abm_level)
...@@ -6525,10 +6536,9 @@ static int dm_update_crtc_state(struct amdgpu_display_manager *dm, ...@@ -6525,10 +6536,9 @@ static int dm_update_crtc_state(struct amdgpu_display_manager *dm,
*/ */
if (dm_new_crtc_state->base.color_mgmt_changed || if (dm_new_crtc_state->base.color_mgmt_changed ||
drm_atomic_crtc_needs_modeset(new_crtc_state)) { drm_atomic_crtc_needs_modeset(new_crtc_state)) {
ret = amdgpu_dm_set_regamma_lut(dm_new_crtc_state); ret = amdgpu_dm_update_crtc_color_mgmt(dm_new_crtc_state);
if (ret) if (ret)
goto fail; goto fail;
amdgpu_dm_set_ctm(dm_new_crtc_state);
} }
/* Update Freesync settings. */ /* Update Freesync settings. */
...@@ -6831,6 +6841,8 @@ dm_determine_update_type_for_commit(struct amdgpu_display_manager *dm, ...@@ -6831,6 +6841,8 @@ dm_determine_update_type_for_commit(struct amdgpu_display_manager *dm,
new_dm_plane_state->dc_state->in_transfer_func; new_dm_plane_state->dc_state->in_transfer_func;
stream_update.gamut_remap = stream_update.gamut_remap =
&new_dm_crtc_state->stream->gamut_remap_matrix; &new_dm_crtc_state->stream->gamut_remap_matrix;
stream_update.output_csc_transform =
&new_dm_crtc_state->stream->csc_color_matrix;
stream_update.out_transfer_func = stream_update.out_transfer_func =
new_dm_crtc_state->stream->out_transfer_func; new_dm_crtc_state->stream->out_transfer_func;
} }
......
...@@ -278,6 +278,9 @@ struct dm_crtc_state { ...@@ -278,6 +278,9 @@ struct dm_crtc_state {
struct drm_crtc_state base; struct drm_crtc_state base;
struct dc_stream_state *stream; struct dc_stream_state *stream;
bool cm_has_degamma;
bool cm_is_degamma_srgb;
int active_planes; int active_planes;
bool interrupts_enabled; bool interrupts_enabled;
...@@ -367,10 +370,9 @@ void amdgpu_dm_crtc_handle_crc_irq(struct drm_crtc *crtc); ...@@ -367,10 +370,9 @@ void amdgpu_dm_crtc_handle_crc_irq(struct drm_crtc *crtc);
#define MAX_COLOR_LEGACY_LUT_ENTRIES 256 #define MAX_COLOR_LEGACY_LUT_ENTRIES 256
void amdgpu_dm_init_color_mod(void); void amdgpu_dm_init_color_mod(void);
int amdgpu_dm_set_degamma_lut(struct drm_crtc_state *crtc_state, int amdgpu_dm_update_crtc_color_mgmt(struct dm_crtc_state *crtc);
struct dc_plane_state *dc_plane_state); int amdgpu_dm_update_plane_color_mgmt(struct dm_crtc_state *crtc,
void amdgpu_dm_set_ctm(struct dm_crtc_state *crtc); struct dc_plane_state *dc_plane_state);
int amdgpu_dm_set_regamma_lut(struct dm_crtc_state *crtc);
extern const struct drm_encoder_helper_funcs amdgpu_dm_encoder_helper_funcs; extern const struct drm_encoder_helper_funcs amdgpu_dm_encoder_helper_funcs;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment