[igt-dev] [PATCH i-g-t v2 2/3] tests/kms_plane: Test all YCbCr encodings/ranges
Kazlauskas, Nicholas
Nicholas.Kazlauskas at amd.com
Wed Jul 3 18:01:01 UTC 2019
On 7/3/19 1:12 PM, Ville Syrjälä wrote:
> On Tue, Jul 02, 2019 at 06:07:24PM +0000, Kazlauskas, Nicholas wrote:
>> On 7/2/19 1:57 PM, Ville Syrjala wrote:
>>> From: Ville Syrjälä <ville.syrjala at linux.intel.com>
>>>
>>> Test all support YCbCr encoding/range combinations. So far
>>> we've only been testing one particular combo.
>>>
>>> v2: Use igt_create_fb_with_bo_size() and hand roll the
>>> solid fills
>>>
>>> Cc: Uma Shankar <uma.shankar at intel.com>
>>> Reviewed-by: Nicholas Kazlauskas <nicholas.kazlauskas at amd.com> #v1
>>> Reviewed-by: Uma Shankar <uma.shankar at intel.com> #v1
>>> Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
>>> ---
>>> tests/kms_plane.c | 164 ++++++++++++++++++++++++++++++++++++----------
>>> 1 file changed, 129 insertions(+), 35 deletions(-)
>>>
>>> diff --git a/tests/kms_plane.c b/tests/kms_plane.c
>>> index 59d5f1e80cd2..ce4bf9e1c21d 100644
>>> --- a/tests/kms_plane.c
>>> +++ b/tests/kms_plane.c
>>> @@ -427,27 +427,36 @@ static void test_format_plane_color(data_t *data, enum pipe pipe,
>>> igt_plane_t *plane,
>>> uint32_t format, uint64_t modifier,
>>> int width, int height,
>>> + enum igt_color_encoding color_encoding,
>>> + enum igt_color_range color_range,
>>> int color, igt_crc_t *crc, struct igt_fb *fb)
>>> {
>>> const color_t *c = &colors[color];
>>> struct igt_fb old_fb = *fb;
>>> + cairo_t *cr;
>>>
>>> if (data->crop == 0 || format == DRM_FORMAT_XRGB8888) {
>>> - igt_create_color_fb(data->drm_fd, width, height,
>>> - format, modifier,
>>> - c->red, c->green, c->blue, fb);
>>> + igt_create_fb_with_bo_size(data->drm_fd, width, height,
>>> + format, modifier, color_encoding,
>>> + color_range, fb, 0, 0);
>>> +
>>> + cr = igt_get_cairo_ctx(data->drm_fd, fb);
>>> +
>>> + igt_paint_color(cr, 0, 0, width, height,
>>> + c->red, c->green, c->blue);
>>> +
>>> + igt_put_cairo_ctx(data->drm_fd, fb, cr);
>>> } else {
>>> + igt_create_fb_with_bo_size(data->drm_fd,
>>> + width + data->crop * 2,
>>> + height + data->crop * 2,
>>> + format, modifier, color_encoding,
>>> + color_range, fb, 0, 0);
>>> +
>>> /*
>>> * paint border in inverted color, then visible area in middle
>>> * with correct color for clamping test
>>> */
>>> - cairo_t *cr;
>>> -
>>> - igt_create_fb(data->drm_fd,
>>> - width + data->crop * 2,
>>> - height + data->crop * 2,
>>> - format, modifier, fb);
>>> -
>>> cr = igt_get_cairo_ctx(data->drm_fd, fb);
>>>
>>> igt_paint_color(cr, 0, 0,
>>> @@ -500,6 +509,104 @@ static int num_unique_crcs(const igt_crc_t crc[], int num_crc)
>>> return num_unique_crc;
>>> }
>>>
>>> +static bool test_format_plane_colors(data_t *data, enum pipe pipe,
>>> + igt_plane_t *plane,
>>> + uint32_t format, uint64_t modifier,
>>> + int width, int height,
>>> + enum igt_color_encoding encoding,
>>> + enum igt_color_range range,
>>> + igt_crc_t ref_crc[ARRAY_SIZE(colors)],
>>> + struct igt_fb *fb)
>>> +{
>>> + int crc_mismatch_count = 0;
>>> + unsigned int crc_mismatch_mask = 0;
>>> + bool result = true;
>>> +
>>> + for (int i = 0; i < ARRAY_SIZE(colors); i++) {
>>> + igt_crc_t crc;
>>> +
>>> + test_format_plane_color(data, pipe, plane,
>>> + format, modifier,
>>> + width, height,
>>> + encoding, range,
>>> + i, &crc, fb);
>>> +
>>> + if (!igt_check_crc_equal(&crc, &ref_crc[i])) {
>>> + crc_mismatch_count++;
>>> + crc_mismatch_mask |= (1 << i);
>>> + result = false;
>>> + }
>>> + }
>>> +
>>> + if (crc_mismatch_count)
>>> + igt_warn("CRC mismatches with format " IGT_FORMAT_FMT " on %s.%u with %d/%d solid colors tested (0x%X)\n",
>>> + IGT_FORMAT_ARGS(format), kmstest_pipe_name(pipe),
>>> + plane->index, crc_mismatch_count, (int)ARRAY_SIZE(colors), crc_mismatch_mask);
>>> +
>>> + return result;
>>> +}
>>> +
>>> +static bool test_format_plane_rgb(data_t *data, enum pipe pipe,
>>> + igt_plane_t *plane,
>>> + uint32_t format, uint64_t modifier,
>>> + int width, int height,
>>> + igt_crc_t ref_crc[ARRAY_SIZE(colors)],
>>> + struct igt_fb *fb)
>>> +{
>>> + igt_info("Testing format " IGT_FORMAT_FMT " / modifier 0x%" PRIx64 " on %s.%u\n",
>>> + IGT_FORMAT_ARGS(format), modifier,
>>> + kmstest_pipe_name(pipe), plane->index);
>>> +
>>> + return test_format_plane_colors(data, pipe, plane,
>>> + format, modifier,
>>> + width, height,
>>> + IGT_COLOR_YCBCR_BT601,
>>> + IGT_COLOR_YCBCR_LIMITED_RANGE,
>>> + ref_crc, fb);
>>> +}
>>> +
>>> +static bool test_format_plane_yuv(data_t *data, enum pipe pipe,
>>> + igt_plane_t *plane,
>>> + uint32_t format, uint64_t modifier,
>>> + int width, int height,
>>> + igt_crc_t ref_crc[ARRAY_SIZE(colors)],
>>> + struct igt_fb *fb)
>>> +{
>>> + bool result = true;
>>> +
>>> + if (!igt_plane_has_prop(plane, IGT_PLANE_COLOR_ENCODING))
>>> + return true;
>>> + if (!igt_plane_has_prop(plane, IGT_PLANE_COLOR_RANGE))
>>> + return true;
>>
>> Skimmed over this in my first pass.
>>
>> Do we really want to be returning true here if the driver doesn't
>> support encoding/range? It's probably not a good idea to really expose
>> YUV formats with them so it might be best to just assert here instead.
>
> I suspect there are still quite a few drivers that don't expose
> these props. Asserting/skipping here would mean skipping the entire
> subtest which is not so great. So IMO better to just skip the YUV
> formats.
That's a fair point. It's still a silent skip but it's probably not a
big deal. This series looks fine to me:
Reviewed-by: Nicholas Kazlauskas <nicholas.kazluaskas at amd.com>
>
>>
>> The rest of P1+P2 lgtm. I don't see P3 but that still only needs the
>> title fix.
>>
>> Nicholas Kazlauskas
>>
>>> +
>>> + for (enum igt_color_encoding e = 0; e < IGT_NUM_COLOR_ENCODINGS; e++) {
>>> + if (!igt_plane_try_prop_enum(plane,
>>> + IGT_PLANE_COLOR_ENCODING,
>>> + igt_color_encoding_to_str(e)))
>>> + continue;
>>> +
>>> + for (enum igt_color_range r = 0; r < IGT_NUM_COLOR_RANGES; r++) {
>>> + if (!igt_plane_try_prop_enum(plane,
>>> + IGT_PLANE_COLOR_RANGE,
>>> + igt_color_range_to_str(r)))
>>> + continue;
>>> +
>>> + igt_info("Testing format " IGT_FORMAT_FMT " / modifier 0x%" PRIx64 " (%s, %s) on %s.%u\n",
>>> + IGT_FORMAT_ARGS(format), modifier,
>>> + igt_color_encoding_to_str(e),
>>> + igt_color_range_to_str(r),
>>> + kmstest_pipe_name(pipe), plane->index);
>>> +
>>> + result &= test_format_plane_colors(data, pipe, plane,
>>> + format, modifier,
>>> + width, height,
>>> + e, r, ref_crc, fb);
>>> + }
>>> + }
>>> +
>>> + return result;
>>> +}
>>> +
>>> static bool test_format_plane(data_t *data, enum pipe pipe,
>>> igt_output_t *output, igt_plane_t *plane)
>>> {
>>> @@ -569,6 +676,8 @@ static bool test_format_plane(data_t *data, enum pipe pipe,
>>> test_format_plane_color(data, pipe, plane,
>>> format, modifier,
>>> width, height,
>>> + IGT_COLOR_YCBCR_BT709,
>>> + IGT_COLOR_YCBCR_LIMITED_RANGE,
>>> i, &ref_crc[i], &fb);
>>> }
>>>
>>> @@ -580,10 +689,6 @@ static bool test_format_plane(data_t *data, enum pipe pipe,
>>> igt_require(num_unique_crcs(ref_crc, ARRAY_SIZE(colors)) > 1);
>>>
>>> for (int i = 0; i < plane->format_mod_count; i++) {
>>> - int crc_mismatch_count = 0;
>>> - int crc_mismatch_mask = 0;
>>> - igt_crc_t crc;
>>> -
>>> format = plane->formats[i];
>>> modifier = plane->modifiers[i];
>>>
>>> @@ -599,30 +704,19 @@ static bool test_format_plane(data_t *data, enum pipe pipe,
>>> continue;
>>> }
>>>
>>> - igt_info("Testing format " IGT_FORMAT_FMT " / modifier 0x%" PRIx64 " on %s.%u\n",
>>> - IGT_FORMAT_ARGS(format), modifier,
>>> - kmstest_pipe_name(pipe), plane->index);
>>> -
>>> - for (int j = 0; j < ARRAY_SIZE(colors); j++) {
>>> - test_format_plane_color(data, pipe, plane,
>>> - format, modifier,
>>> - width, height,
>>> - j, &crc, &fb);
>>> -
>>> - if (!igt_check_crc_equal(&crc, &ref_crc[j])) {
>>> - crc_mismatch_count++;
>>> - crc_mismatch_mask |= (1 << j);
>>> - result = false;
>>> - }
>>> - }
>>> + if (igt_format_is_yuv(format))
>>> + result &= test_format_plane_yuv(data, pipe, plane,
>>> + format, modifier,
>>> + width, height,
>>> + ref_crc, &fb);
>>> + else
>>> + result &= test_format_plane_rgb(data, pipe, plane,
>>> + format, modifier,
>>> + width, height,
>>> + ref_crc, &fb);
>>>
>>> if (format == DRM_FORMAT_C8)
>>> set_legacy_lut(data, pipe, 0xfc00);
>>> -
>>> - if (crc_mismatch_count)
>>> - igt_warn("CRC mismatches with format " IGT_FORMAT_FMT " on %s.%u with %d/%d solid colors tested (0x%X)\n",
>>> - IGT_FORMAT_ARGS(format), kmstest_pipe_name(pipe),
>>> - plane->index, crc_mismatch_count, (int)ARRAY_SIZE(colors), crc_mismatch_mask);
>>> }
>>>
>>> igt_pipe_crc_stop(data->pipe_crc);
>>>
>>
>
More information about the igt-dev
mailing list