All of lore.kernel.org
 help / color / mirror / Atom feed
From: Harry Wentland <harry.wentland@amd.com>
To: Melissa Wen <mwen@igalia.com>, Alex Hung <alex.hung@amd.com>,
	Petri Latvala <adrinael@adrinael.net>,
	Arkadiusz Hiler <arek@hiler.eu>,
	Kamil Konieczny <kamil.konieczny@linux.intel.com>,
	Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>,
	Bhanuprakash Modem <bhanuprakash.modem@gmail.com>,
	Ashutosh Dixit <ashutosh.dixit@intel.com>,
	Karthik B S <karthik.b.s@intel.com>
Cc: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>,
	igt-dev@lists.freedesktop.org, kernel-dev@igalia.com,
	Swati Sharma <swati2.sharma@intel.com>,
	John Harrison <John.Harrison@Igalia.com>,
	Rodrigo Siqueira <siqueira@igalia.com>,
	Simon Ser <contact@emersion.fr>, Xaver Hugl <xaver.hugl@kde.org>,
	Uma Shankar <uma.shankar@intel.com>
Subject: Re: [PATCH i-g-t v3 6/6] tests/kms_color_pipeline: skip if not an intel device
Date: Tue, 18 Aug 2026 10:30:25 -0400	[thread overview]
Message-ID: <2216cc39-92a4-4472-93c9-96ac89b8d6d7@amd.com> (raw)
In-Reply-To: <4a5e0dea-1a44-4e5a-97c5-530e61a92ee3@igalia.com>



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.

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 <chaitanya.kumar.borah@intel.com>
>>>>> Signed-off-by: Melissa Wen <mwen@igalia.com>
>>>>> ---
>>>>>    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()
> 


  reply	other threads:[~2026-08-18 14:31 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-11 14:23 [PATCH i-g-t v3 0/6] test/kms_colorop_helper: don't request colorop updates indefinitely Melissa Wen
2026-08-11 14:23 ` [PATCH i-g-t v3 1/6] lib/igt_kms: clear colorop-changed flag after commit Melissa Wen
2026-08-11 16:37   ` Jani Nikula
2026-08-14  3:39   ` Alex Hung
2026-08-11 14:23 ` [PATCH i-g-t v3 2/6] test/kms_colorop_helper: only check if a given enum value exist Melissa Wen
2026-08-14  3:41   ` Alex Hung
2026-08-11 14:23 ` [PATCH i-g-t v3 3/6] tests/kms_properties: don't check colorop if no plane color pipeline prop Melissa Wen
2026-08-14  3:44   ` Alex Hung
2026-08-11 14:23 ` [PATCH i-g-t v3 4/6] tests/kms_properties: keep primary plane enabled for each CRTC Melissa Wen
2026-08-14  4:05   ` Alex Hung
2026-08-14  6:20     ` Borah, Chaitanya Kumar
2026-08-17 15:16       ` Melissa Wen
2026-08-11 14:23 ` [PATCH i-g-t v3 6/6] tests/kms_color_pipeline: skip if not an intel device Melissa Wen
2026-08-14  3:55   ` Alex Hung
2026-08-17 15:19     ` Melissa Wen
2026-08-17 15:39       ` Harry Wentland
2026-08-17 16:08         ` Harry Wentland
2026-08-18 13:54         ` Melissa Wen
2026-08-18 14:30           ` Harry Wentland [this message]
2026-08-18 14:50             ` Melissa Wen
2026-08-19  5:24               ` Borah, Chaitanya Kumar
2026-08-21 11:53                 ` Sharma, Swati2
2026-08-11 16:43 ` ✓ Xe.CI.BAT: success for test/kms_colorop_helper: don't request colorop updates indefinitely (rev2) Patchwork
2026-08-11 16:59 ` ✓ i915.CI.BAT: " Patchwork
2026-08-11 18:43 ` ✓ Xe.CI.FULL: " Patchwork
2026-08-11 19:21 ` ✗ i915.CI.Full: failure " Patchwork

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=2216cc39-92a4-4472-93c9-96ac89b8d6d7@amd.com \
    --to=harry.wentland@amd.com \
    --cc=John.Harrison@Igalia.com \
    --cc=adrinael@adrinael.net \
    --cc=alex.hung@amd.com \
    --cc=arek@hiler.eu \
    --cc=ashutosh.dixit@intel.com \
    --cc=bhanuprakash.modem@gmail.com \
    --cc=chaitanya.kumar.borah@intel.com \
    --cc=contact@emersion.fr \
    --cc=igt-dev@lists.freedesktop.org \
    --cc=juhapekka.heikkila@gmail.com \
    --cc=kamil.konieczny@linux.intel.com \
    --cc=karthik.b.s@intel.com \
    --cc=kernel-dev@igalia.com \
    --cc=mwen@igalia.com \
    --cc=siqueira@igalia.com \
    --cc=swati2.sharma@intel.com \
    --cc=uma.shankar@intel.com \
    --cc=xaver.hugl@kde.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.