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

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?

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()