Re: [i-g-t] tests/kms_colorop: Use local variable for color pipeline cap

["Borah, Chaitanya Kumar" <chaitanya.kumar.borah@intel.com>]

On 8/28/2026 5:45 AM, Alex Hung wrote:
> 
> 
> On 8/26/26 23:31, Sharma, Swati2 wrote:
>> Hi Chaitanya
>>
>> Patch LGTM
>>
>> Reviewed-by: Swati Sharma <swati2.sharma@intel.com>
>>
>> Also, it seems we have duplicate igt_display_require(&display, 
>> display.drm_fd);
>>
>> It is required once. Anyways, its a separate patch.
>>
>> Alex,
>>
>> Can you please test on AMD driver with above change?
> 
> yes it ran fine. Some inline comments below.
> 
>>
>> On 03-08-2026 01:33 pm, Chaitanya Kumar Borah wrote:
>>> Following the pattern in kms_color_pipeline, set
>>> DRM_CLIENT_CAP_PLANE_COLOR_PIPELINE once and stash the result in a
>>> local variable, then propagate it to display->has_plane_color_pipeline
>>> after the final igt_display_require() (which reinitializes the display
>>> state). This avoids calling drmSetClientCap() twice.
>>>
>>> Cc: Swati Sharma <swati2.sharma@intel.com>
>>> Cc: Harry Wentland <harry.wentland@amd.com>
>>> Cc: Alex Hung <alex.hung@amd.com>
>>> Signed-off-by: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>
>>> ---
>>>   tests/kms_colorop.c | 9 +++++----
>>>   1 file changed, 5 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/tests/kms_colorop.c b/tests/kms_colorop.c
>>> index 16db0b8ad..d573d2ceb 100644
>>> --- a/tests/kms_colorop.c
>>> +++ b/tests/kms_colorop.c
>>> @@ -406,6 +406,7 @@ int igt_main_args("d", long_options, help_str, 
>>> opt_handler, NULL)
>>>       igt_display_t display;
>>>       int i, j, ret;
>>> +    int has_plane_color_pipeline = 0;
> can this be bool?
> 
>>>       igt_fixture() {
>>>           display.drm_fd = drm_open_driver_master(DRIVER_ANY);
>>> @@ -417,15 +418,15 @@ 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;
>>> +            has_plane_color_pipeline = 1;
>>> +
>>> +        igt_display_require(&display, display.drm_fd);
> is this igt_display_require() necessary?
> 

Swati flagged this too and this is probably unnecessary. However, I 
thought of handling it in a separate patch. What do you think?

>>>           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;
>>> +        display.has_plane_color_pipeline = has_plane_color_pipeline;
>>>           igt_require(display.is_atomic);
>>>       }
>