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

Hi Chaitanya

On 21-07-2026 04:30 pm, Borah, Chaitanya Kumar wrote:
>
>
> 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.

Sure, lets get this fixed as well as part of a new patch.

Above changes LGTM

Reviewed-by: Swati Sharma <[email protected]>

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