[igt-dev] [PATCH i-g-t 3/3] tests/kms_yuv: Add yuv specific tests, v4.
Maarten Lankhorst
maarten.lankhorst at linux.intel.com
Fri Mar 29 09:10:08 UTC 2019
Op 28-03-2019 om 20:25 schreef Ville Syrjälä:
> On Thu, Mar 28, 2019 at 07:08:45PM +0100, Maarten Lankhorst wrote:
>> Add tests excercising switching between various scaled/unscaled
>> transitions to and from various nv12 formats, since some of the
>> workarounds mention this may fail.
>>
>> We also add NV12 specific clipping/scaling tests, to make sure
>> that YUV src coordinates are always programmed as a multiple of 2
>> correctly, and verify scaling/clipping works with CRC tests.
>>
>> crop-scale-bug shows a corruption when scaling and cropping at the
>> same time.
>>
>> The subpixel clip test rotates and clips a rectangle on each side,
>> which should produce the same CRC as when that part of the rectangle
>> is hidden by a cursor and a whole 2x2 pixel is hidden.
>>
>> Changes since v1:
>> - Reset plane position to 0,0 at the start of yuv_valid_width_plane().
>> - Handle Swati's feedback.
>> Changes since v2:
>> - Skip 90°/270° rotation in tests when formats don't support it.
>> - Add all new HDR formats to test.
>> - Use igt_plane_has_format_mod.
>> - Support tests on !i915 by checking if X/Y tiling modifiers are supported.
>> - Do not enable untested planes to prevent exhausing memory bandwidth
>> and available NV12 Y planes in the rgb-switch-scaled test.
>> Changes since v3:
>> - Add a sanity test to sanity test YUV conversion paths, on all planes and pipes.
>> - Dynamically generate list of YUV tests through igt_format_array_fill.
>>
>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
>> ---
>> tests/Makefile.sources | 1 +
>> tests/kms_yuv.c | 942 +++++++++++++++++++++++++++++++++++++++++
>> tests/meson.build | 1 +
>> 3 files changed, 944 insertions(+)
>> create mode 100644 tests/kms_yuv.c
>>
>> diff --git a/tests/Makefile.sources b/tests/Makefile.sources
>> index 71ccf00af256..5eb46087e362 100644
>> --- a/tests/Makefile.sources
>> +++ b/tests/Makefile.sources
>> @@ -74,6 +74,7 @@ TESTS_progs = \
>> kms_universal_plane \
>> kms_vblank \
>> kms_vrr \
>> + kms_yuv \
>> meta_test \
>> perf \
>> perf_pmu \
>> diff --git a/tests/kms_yuv.c b/tests/kms_yuv.c
>> new file mode 100644
>> index 000000000000..0077a07a73f3
>> --- /dev/null
>> +++ b/tests/kms_yuv.c
>> @@ -0,0 +1,942 @@
>> +/*
>> + * Copyright © 2019 Intel Corporation
>> + *
>> + * Permission is hereby granted, free of charge, to any person obtaining a
>> + * copy of this software and associated documentation files (the "Software"),
>> + * to deal in the Software without restriction, including without limitation
>> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
>> + * and/or sell copies of the Software, and to permit persons to whom the
>> + * Software is furnished to do so, subject to the following conditions:
>> + *
>> + * The above copyright notice and this permission notice shall be included in
>> + * all copies or substantial portions of the Software.
>> + *
>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
>> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
>> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
>> + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
>> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
>> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
>> + * IN THE SOFTWARE.
>> + *
>> + * Authors:
>> + * Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
>> + */
>> +#include "config.h"
>> +
>> +#include "igt.h"
>> +#include <cairo.h>
>> +#include <errno.h>
>> +#include <stdint.h>
>> +#include <unistd.h>
>> +#include <sys/time.h>
>> +
>> +typedef struct {
>> + igt_display_t display;
>> +
>> + igt_pipe_crc_t *pipe_crc;
>> + struct igt_fb fb[6];
>> +} data_t;
>> +
>> +static uint64_t x_modifier = LOCAL_DRM_FORMAT_MOD_NONE;
>> +static uint64_t y_modifier = LOCAL_DRM_FORMAT_MOD_NONE;
>> +static uint32_t *formats, count_formats;
>> +
>> +static bool pipe_supports_format(igt_display_t *display, enum pipe pipe,
>> + uint32_t format, uint64_t tiling)
>> +{
>> + igt_plane_t *plane;
>> +
>> + for_each_plane_on_pipe(display, pipe, plane)
>> + if (igt_plane_has_format_mod(plane, format, tiling))
>> + return true;
>> +
>> + return false;
>> +}
>> +
>> +
>> +static void remove_fbs(data_t *data)
>> +{
>> + int i;
>> +
>> + for (i = 0; i < ARRAY_SIZE(data->fb); i++)
>> + igt_remove_fb(data->display.drm_fd, &data->fb[i]);
>> +}
>> +
>> +static void prepare_crtc(data_t *data, enum pipe pipe, igt_output_t *output)
>> +{
>> + igt_display_t *display = &data->display;
>> +
>> + remove_fbs(data);
>> + igt_display_reset(display);
>> + igt_output_set_pipe(output, pipe);
>> + igt_display_commit2(display, COMMIT_ATOMIC);
>> +
>> + igt_pipe_crc_free(data->pipe_crc);
>> + data->pipe_crc = igt_pipe_crc_new(display->drm_fd, pipe,
>> + INTEL_PIPE_CRC_SOURCE_AUTO);
>> +}
>> +
>> +static void fudge_lut(data_t *data, enum pipe pipe, uint16_t mask)
>> +{
>> + igt_pipe_t *pipe_obj = &data->display.pipes[pipe];
>> + uint16_t *lut;
>> + int i, lut_size;
>> +
>> + if (!igt_pipe_obj_has_prop(pipe_obj, IGT_CRTC_GAMMA_LUT_SIZE))
>> + return;
>> +
>> + lut_size = igt_pipe_obj_get_prop(pipe_obj, IGT_CRTC_GAMMA_LUT_SIZE);
> I would suggest hardcoding 256 for i915 so that we'll get the legacy
> LUT and get consistent behaviour on all platforms. It has more than
> enough bits for us anyway.
Where's the fun in not using the extra bits? O:)
> Also for platforms that don't expose the LUT prop we'd probably want to
> fallback to the legacy API. Although I will soonish post some patches to
> expose the GAMMA_LUT stuff on all our platforms, so not a big deal for
> i915 I suppose. Other drivers might care more about that.
Why don't we? Patch should be 2 lines in the kernel..
What prevents us from doing this?
diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
index 467fd1a1630c..250ac1765f7f 100644
--- a/drivers/gpu/drm/i915/intel_color.c
+++ b/drivers/gpu/drm/i915/intel_color.c
@@ -873,7 +873,7 @@ void intel_color_init(struct intel_crtc *crtc)
{
struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
- drm_mode_crtc_set_gamma_size(&crtc->base, 256);
+ drm_mode_crtc_set_gamma_size(&crtc->base, LEGACY_LUT_LENGTH);
if (HAS_GMCH(dev_priv)) {
if (IS_CHERRYVIEW(dev_priv))
@@ -907,4 +907,6 @@ void intel_color_init(struct intel_crtc *crtc)
INTEL_INFO(dev_priv)->color.degamma_lut_size,
true,
INTEL_INFO(dev_priv)->color.gamma_lut_size);
+ else
+ drm_crtc_enable_color_mgmt(&crtc->base, 0, false, LEGACY_LUT_LENGTH);
}
Can send as separate patch, would be nice to have this in-kernel.
>
>> +
>> + lut = malloc(sizeof(uint16_t) * lut_size);
>> + for (i = 0; i < lut_size; i++)
>> + lut[i] = (i * 0xffff / (lut_size - 1)) & mask;
>> +
>> + igt_pipe_obj_replace_prop_blob(pipe_obj, IGT_CRTC_GAMMA_LUT, lut, lut_size * sizeof(uint16_t));
>> + free(lut);
>> +
>> + if (igt_pipe_obj_has_prop(pipe_obj, IGT_CRTC_DEGAMMA_LUT))
>> + igt_pipe_obj_replace_prop_blob(pipe_obj, IGT_CRTC_DEGAMMA_LUT, NULL, 0);
>> +
>> + if (igt_pipe_obj_has_prop(pipe_obj, IGT_CRTC_CTM))
>> + igt_pipe_obj_replace_prop_blob(pipe_obj, IGT_CRTC_CTM, NULL, 0);
> Shouldn't those be cleared already by some display_reset() type thing?
We don't currently sanitize many properties. Although it seems that from the changes to
igt_plane_reset() to deal with fallout from previous tests we probably should start setting sane defaults on everything..
>> +}
>> +
>> +static void unfudge_lut(data_t *data, enum pipe pipe)
>> +{
>> + igt_pipe_t *pipe_obj = &data->display.pipes[pipe];
>> +
>> + if (igt_pipe_obj_has_prop(pipe_obj, IGT_CRTC_GAMMA_LUT))
>> + igt_pipe_obj_replace_prop_blob(pipe_obj, IGT_CRTC_GAMMA_LUT, NULL, 0);
>> +}
>> +
>> +/*
>> + * Test that we initialize a YUV fb as black correctly,
>> + * that when we write a black FB it will get the same CRC, and that
>> + * all colors we test will give the same CRC on all formats.
>> + */
>> +static void yuv_sanity(data_t *data, enum pipe pipe, igt_output_t *output)
> I wonder if this should just be part of the normal pixel format test?
I wrote these tests specifically to expose bugs in our YUV conversion routines. I
could start capturing the reference CRC's with XRGB8888 like the normal pixel
format test.
Whenever
~Maarten
More information about the igt-dev
mailing list