Re: [PATCH i-g-t 1/2] Revert "lib/kms: Set display->has_plane_color_pipeline automatically"

On 7/21/2026 4:25 PM, Sharma, Swati2 wrote:
> Hi Chaitanya,
> 
> On 14-07-2026 08:15 pm, Chaitanya Kumar Borah wrote:
>> This reverts commit be4e98c3b79e8bb843f7bb762616742691fee72e.
>>
>> Enabling DRM_CLIENT_CAP_PLANE_COLOR_PIPELINE from the common
>> igt_display_require() path makes all IGT display clients opt into the
>> new plane color pipeline UAPI.
>>
>> With the color pipeline client cap enabled, legacy plane COLOR_ENCODING
>> and COLOR_RANGE properties are ignored during commit preparation.
>> The driver cannot distinguish this case, because enum drm_color_encoding
>> and enum drm_color_range do not provide an explicit uninitialized value.
>>
>> Revert the global opt-in and keep the cap enablement local to the
>> color-pipeline-aware tests. kms_properties colorop coverage shall be
>> preserved separately.
>>
>> Cc: John Harrison <[email protected]>
>> Cc: Kamil Konieczny <[email protected]>
>> Cc: Swati Sharma <[email protected]>
>> Signed-off-by: Chaitanya Kumar Borah <[email protected]>
>> ---
>>   lib/igt_kms.c              | 2 --
>>   tests/kms_color_pipeline.c | 5 +++++
>>   tests/kms_colorop.c        | 4 ++++
>>   3 files changed, 9 insertions(+), 2 deletions(-)
>>
>> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
>> index 2eefb773b..c56127c42 100644
>> --- a/lib/igt_kms.c
>> +++ b/lib/igt_kms.c
>> @@ -3201,8 +3201,6 @@ void igt_display_require(igt_display_t *display, 
>> int drm_fd)
>>       if (drmSetClientCap(drm_fd, 
>> LOCAL_DRM_CLIENT_CAP_CURSOR_PLANE_HOTSPOT, 1) == 0)
>>           display->has_virt_cursor_plane = 1;
>> -    if (drmSetClientCap(drm_fd, DRM_CLIENT_CAP_PLANE_COLOR_PIPELINE, 
>> 1) == 0)
>> -        display->has_plane_color_pipeline = 1;
>>       plane_resources = drmModeGetPlaneResources(display->drm_fd);
>>       igt_assert(plane_resources);
>> diff --git a/tests/kms_color_pipeline.c b/tests/kms_color_pipeline.c
>> index 6119a830f..78860a845 100644
>> --- a/tests/kms_color_pipeline.c
>> +++ b/tests/kms_color_pipeline.c
>> @@ -349,6 +349,7 @@ run_tests_for_plane(data_t *data)
>>   int igt_main()
>>   {
>> +    int has_plane_color_pipeline = 0;
>>       data_t data = {};
>>       igt_fixture() {
>> @@ -363,9 +364,13 @@ int igt_main()
>>           if (drmSetClientCap(data.drm_fd, DRM_CLIENT_CAP_ATOMIC, 1) 
>> == 0)
>>               data.display.is_atomic = 1;
>> +        if (drmSetClientCap(data.drm_fd, 
>> DRM_CLIENT_CAP_PLANE_COLOR_PIPELINE, 1) == 0)
>> +            has_plane_color_pipeline = 1;
>> +
>>           kmstest_set_vt_graphics_mode();
>>           igt_display_require(&data.display, data.drm_fd);
>> +        data.display.has_plane_color_pipeline = 
>> has_plane_color_pipeline;
>>           igt_require(data.display.is_atomic);
>>       }
>> diff --git a/tests/kms_colorop.c b/tests/kms_colorop.c
>> index 4db83470d..16db0b8ad 100644
>> --- a/tests/kms_colorop.c
>> +++ b/tests/kms_colorop.c
>> @@ -418,10 +418,14 @@ int igt_main_args("d", long_options, help_str, 
>> opt_handler, NULL)
>>           igt_require_f(!ret, "error setting 
>> DRM_CLIENT_CAP_WRITEBACK_CONNECTORS\n");
>>           igt_display_require(&display, display.drm_fd);
>> +        if (drmSetClientCap(display.drm_fd, 
>> DRM_CLIENT_CAP_PLANE_COLOR_PIPELINE, 1) == 0)
>> +            display.has_plane_color_pipeline = 1;
>>           kmstest_set_vt_graphics_mode();
>>           igt_display_require(&display, display.drm_fd);
>> +        if (drmSetClientCap(display.drm_fd, 
>> DRM_CLIENT_CAP_PLANE_COLOR_PIPELINE, 1) == 0)
>> +            display.has_plane_color_pipeline = 1;
> 
> Can't we follow same local variable pattern as kms_color_pipeline? That 
> will avoid calling drmSetClientCap() twice.
> 

We can but since this is a pure revert we can take it up in a separate 
patch.

I can float one if this gets approved.

==
Chaitanya

> 
>>           igt_require(display.is_atomic);
>>       }