On 2026-07-03 06:40, Robert Mader wrote:
> Hi,
>
> On 23.06.26 18:48, Harry Wentland wrote:
>> When the fixed matrix colorop is bypassed, the color_space was set to
>> COLOR_SPACE_UNKNOWN (0). In DC's DPP setup (dpp1_cnv_setup), the logic
>> 'input_color_space ? input_color_space : color_space' treats 0 as
>> 'not provided', causing it to fall back to the format-based default of
>> COLOR_SPACE_YCBCR709 for YUV framebuffers. This results in an implicit
>> YUV-to-RGB conversion via ICSC even when a color pipeline is active and
>> the fixed matrix is bypassed.
>
> shouldn't a commit with YUV framebuffer and fixed matrix set to bypass always
> fail, like it does in the VKMS implementation?
>
> If this commit is only needed for RGB formats, somehow getting a YUV-to-RGB
> conversion applied to them otherwise, then I think the commit message should
> be clarified accordingly, no?
I talked about this at Hackfest (with Xaver and Sebastian, I think) and we
noted that since this is simply a fixed matrix colorop there is no reason to
reject YUV + bypass or RGB + matrix. For all other colorops we assume
that userspace knows what they're doing.
Thanks for spotting the inconsistencies between VKMS and amdgpu. IMO VKMS
shouldn't reject that commit and simply do a no-op (or identity matrix).
For the RGB + fixed_matrix scenario both drivers already allow that.
If no objection I'll make that change for v4.
Harry
>
>>
>> Fix this by setting COLOR_SPACE_SRGB (1) instead. This is non-zero, so
>> it overrides the format default. The SRGB entry in dpp_input_csc_matrix
>> is an identity matrix, so ICSC performs no actual conversion, which is
>> the correct behavior when the fixed matrix colorop is bypassed.
>>
>> Cc: [email protected]
>> Fixes: 93d922f4833b ("drm/amd/display: Implement fixed matrix colorop color
>> space mapping")
>> Assisted-by: Copilot:claude-opus-4.6
>> Signed-off-by: Harry Wentland <[email protected]>
>> ---
>> drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c
>> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c
>> index 561ee9a2e749..984bbfcf23f0 100644
>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c
>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c
>> @@ -1534,7 +1534,7 @@ __set_dm_plane_colorop_fixed_matrix(struct
>> drm_plane_state *plane_state,
>> return -EINVAL;
>> if (colorop_state->bypass) {
>> - dc_plane_state->color_space = COLOR_SPACE_UNKNOWN;
>> + dc_plane_state->color_space = COLOR_SPACE_SRGB;
>> return 0;
>> }
>>
>
