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

[Alex Hung <alex.hung@amd.com>]

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?

>>           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);
>>       }