Re: [PATCH i-g-t 0/5] New pipe level color management tests V5

Lionel Landwerlin
2016-03-08T11:50:27Z

[-- Attachment #1.1: Type: text/plain, Size: 4832 bytes --]

On 08/03/16 10:42, Lionel Landwerlin wrote:
> On 08/03/16 02:46, Matt Roper wrote:
>> On Thu, Feb 25, 2016 at 05:16:08PM +0000, Lionel Landwerlin wrote:
>>> Hi,
>>>
>>> This series enables testing pipe level color management using kernel patches
>>> from this serie :
>>>
>>> https://patchwork.freedesktop.org/series/2720/
>>>
>>> Most of the tests use pipe CRCs to check the results by comparing the output
>>> with the expected output drawn using cairo.
>> These tests look good. The only thing that might be worth adding is a
>> test that intermixes the legacy gamma ioctl with full color management
>> use. I believe invoking the legacy ioctl should clear the degamma and
>> CTM, right? It might be worth checking that that actually happens.
> That's right. We have drm_atomic_helper_legacy_gamma_set helper() to
> do this (https://patchwork.freedesktop.org/patch/75106/).
> I'll add a test exercising that back and forth.
>
>> Also, there was one misplaced patch hunk that I called out on patch 2/3.
> Fixing.
>
>> Aside from the final IGT tweaks, I think we're pretty close to merging.
>> Here are the final opens I see:
>>
>> * I expect we'll just merge this whole thing through the Intel tree,
>> even though patch #3 adds the properties, structures, and helpers to
>> the DRM core, so we need to get an okay from airlied. Dave, do you
>> have any concerns about this?
>>
>> * I know the ChromeOS guys have been reviewing this series
>> independently on their end and putting together the userspace (open
>> source) which will be using this. We should get someone to formally
>> Ack this on their behalf; I think Rob Bradford has been working with
>> them; adding him to the Cc list to see if he can provide the Ack.
>>
>> * I'm not sure if I ever saw a CI report for your final iteration. I
>> may have just missed the email, or it may have never come out due to
>> the temporary FDO downtime or the CI outage last week. Could you
>> provide a pointer to the results (with false positives justified with
>> bugzilla entries), or resubmit again for CI if necessary?
>
> It's attached to the patchwork series :
> https://patchwork.freedesktop.org/series/2720/
>
> The usual flaky tests are : suspend-read-crc-pipe-* and
> basic-flip-vs-modeset.
> The force-load-detect and error-state-basic were failing for other
> people at the time I posted the v10 series.
>
>> * I'm still a little bit uncomfortable that we (the general DRM
>> community) never really came to a consensus on the whole
>> DRM_MODE_PROP_ATOMIC vs not question (both for this patchset and as a
>> general guideline going forward with similar features). Emil had a
>> good argument that if we make something atomic-only to start with,
>> it's trivial to change in the future if we decide we actually want to
>> make use of it with a non-atomic userspace; on the other hand, if we
>> make it unrestricted on day one, that's a change we can't reverse
>> down the line since it becomes part of the ABI. However nobody
>> seemed to really care very strongly either way during the discussion
>> on the color management series, so I'm just putting this here as a
>> final warning in case anyone really wants to see it changed before
>> merging.
> I don't mind switching to atomic only.

Actually, talking with Rob, it might be a bit tricky for ChromeOS.
Most of the userspace modifications have been reviewed with the
assumption that we didn't need atomic.
Switching the userspace as atomic aware might have unintended consequences.

>
>> Thanks!
>>
>>
>> Matt
>>
>>
>>> Cheers,
>>>
>>> Lionel
>>>
>>> Lionel Landwerlin (5):
>>> lib: kms: add crtc_id to igt_pipe_t
>>> lib: kms: add helpers for color management properties on pipes
>>> lib: fb: add igt_paint_color_gradient_range
>>> lib: add crc comparison function without an assert
>>> tests/kms_color: New test for pipe level color management
>>>
>>> lib/igt_debugfs.c | 17 +
>>> lib/igt_debugfs.h | 1 +
>>> lib/igt_fb.c | 34 ++
>>> lib/igt_fb.h | 3 +
>>> lib/igt_kms.c | 75 ++++
>>> lib/igt_kms.h | 18 +-
>>> tests/Makefile.sources | 1 +
>>> tests/kms_pipe_color.c | 1045 ++++++++++++++++++++++++++++++++++++++++++++++++
>>> 8 files changed, 1193 insertions(+), 1 deletion(-)
>>> create mode 100644 tests/kms_pipe_color.c
>>>
>>> --
>>> 2.7.0
>>> _______________________________________________
>>> Intel-gfx mailing list
>>> Intel-gfx@lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
>
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[-- Attachment #1.2: Type: text/html, Size: 7066 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]