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.

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