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