Re: [PATCH i-g-t v3 6/6] tests/kms_color_pipeline: skip if not an intel device

On 8/18/2026 8:20 PM, Melissa Wen wrote:
> 
> 
> On 18/08/2026 16:30, Harry Wentland wrote:
>>
>> On 2026-08-18 09:54, Melissa Wen wrote:
>>>
>>> On 17/08/2026 17:39, Harry Wentland wrote:
>>>> On 2026-08-17 11:19, Melissa Wen wrote:
>>>>> On 14/08/2026 05:55, Alex Hung wrote:
>>>>>> On 8/11/26 08:23, Melissa Wen wrote:
>>>>>>>    From documentation, the test requires i915 or xe driver, so 
>>>>>>> make sure it
>>>>>>> only runs with intel devices. In addition, it's a CRC-based test, 
>>>>>>> making
>>>>>> Should this test be moved to tests/intel/?
>>>>> I kept it as a KMS test because kms_colorop relies on writeback 
>>>>> support, but writeback isn't present in many drivers AFAIK.
>>>>> So these CRC tests can be used by other drivers if they don't 
>>>>> support writeback but support colorop.
>>>>>
>>>> In that case the test shouldn't have an 
>>>> igt_require(is_intel_device...). It should
>>>> use a generic check to determine whether it runs or not.
>>> What would be this generic check? Something like "CRC support && ! 
>>> writeback"?
>>>
>> Something like that might make sense.
>>
>>> I took this `is_intel_device` path because on one hand I think the 
>>> test could be an
>>> alternative to other drivers that doesn't support writeback, on the 
>>> other hand I see
>>> that the current design is not generic enough and I can't ensure it 
>>> runs well in
>>> other drivers - for example, rn it doesn't meet AMD's requirements.
>>>
>>> My reasoning was: "the test with writeback support is preferable 
>>> (kms_colorop); if your
>>> driver doesn't support writeback, work on the kms_color_pipeline test 
>>> to meet the
>>> requirements of your driver".
>>>
>>> I can check how it goes with VKMS and what's needed to make it run on 
>>> AMD (probably
>>> the active primary plane is missing here too). I just don't think 
>>> those drivers need
>>> kms_color_pipeline since they support writeback and color tests work 
>>> better by
>>> checking writeback output instead of CRC.
>>>
>> I'm just tired of seeing is_intel_device in "generic" tests. I guess 
>> that's
>> why Alex suggested moving this to tests/intel. One could always move 
>> it back
>> to generic tests if another driver sees value in this test and 
>> actually makes
>> it generic.
>>
>> I don't think a CRC-based color test would work well on AMD, and it'll 
>> likely
>> have challenges with VKMS.
>>
>> Again, not a blocker, just a statement about "IGT hasn't been 'Intel
>> GPU Tools' for years", which is sort of true and sort of isn't. It's a 
>> bit
>> painful for parties that decided to use it that are not Intel.
> 
> Hi Harry,
> 
> I see what you mean.
> 
> I can add the generic check plus a debug message, but I agree with you that
> migrating to intel and only making it generic if there is interest from 
> other
> vendors is also a good path. So far, my reasoning about the relevance for
> drivers without writeback support is purely hypothetical.
> 
> Chaitanya, what do you think?

I think moving it to the intel folder makes the most sense.

Swati, do you see any issues for our CI or otherwise?

==
Chaitanya>
> Melissa
> 
>>
>> Harry
>>
>>> Melissa
>>>
>>>> Harry
>>>>
>>>>> Melissa
>>>>>
>>>>>>> kms_colorop with writeback more aligned with color testing and
>>>>>>> validation. Other drivers should work on adapting this test to their
>>>>>>> needs if they are looking for CRC validation.
>>>>>>>
>>>>>>> Reviewed-by: Chaitanya Kumar Borah <[email protected]>
>>>>>>> Signed-off-by: Melissa Wen <[email protected]>
>>>>>>> ---
>>>>>>>     tests/kms_color_pipeline.c | 1 +
>>>>>>>     1 file changed, 1 insertion(+)
>>>>>>>
>>>>>>> diff --git a/tests/kms_color_pipeline.c b/tests/kms_color_pipeline.c
>>>>>>> index 78860a845..3978745a5 100644
>>>>>>> --- a/tests/kms_color_pipeline.c
>>>>>>> +++ b/tests/kms_color_pipeline.c
>>>>>>> @@ -372,6 +372,7 @@ int igt_main()
>>>>>>>             igt_display_require(&data.display, data.drm_fd);
>>>>>>>             data.display.has_plane_color_pipeline = 
>>>>>>> has_plane_color_pipeline;
>>>>>>>             igt_require(data.display.is_atomic);
>>>>>>> +        igt_require(is_intel_device(data.drm_fd));
>>>>>>>         }
>>>>>>>           igt_subtest_group()
>