[igt-dev] [PATCH i-g-t 1/2] tests/kms_plane: optimize pixel format tests
Ville Syrjälä
ville.syrjala at linux.intel.com
Tue Dec 1 16:29:55 UTC 2020
On Sat, Nov 28, 2020 at 12:40:29AM +0200, Juha-Pekka Heikkila wrote:
> On packed formats there's no need to show separate colors on separate fbs.
> Here augmented path with packed color handling which check colors just on
> one fb.
>
> This cannot be directly applied to planar yuv formats because of scaler
> use with those formats.
>
> On my ICL this drop test execution time from 44.91s to 21.98s
>
> v2 (vsyrjala): paint entire screen instead of lines. small refinement.
>
> Signed-off-by: Juha-Pekka Heikkila <juhapekka.heikkila at gmail.com>
> ---
> tests/kms_plane.c | 159 ++++++++++++++++++++++++++++++----------------
> 1 file changed, 106 insertions(+), 53 deletions(-)
>
> diff --git a/tests/kms_plane.c b/tests/kms_plane.c
> index 4ce859fba..e61a9b9f5 100644
> --- a/tests/kms_plane.c
> +++ b/tests/kms_plane.c
> @@ -460,61 +460,72 @@ static bool set_c8_legacy_lut(data_t *data, enum pipe pipe,
> return true;
> }
>
> +static void draw_entire_color_array(data_t *data, cairo_t *cr, uint32_t format,
> + struct igt_fb *fb)
> +{
> + const int color_amount = ARRAY_SIZE(colors_extended);
> + const int x = format == DRM_FORMAT_XRGB8888 ? 0 : data->crop;
> +
> + for (int n = 0; n < color_amount; n++) {
> + int y = (fb->height - x * 2) * n / color_amount + x;
> +
> + igt_paint_color(cr, x, y,
> + fb->width - x * 2,
> + (fb->height - x * 2) / color_amount,
> + colors_extended[n].red,
> + colors_extended[n].green,
> + colors_extended[n].blue);
> + }
> +}
> +
> static void prepare_format_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,
> - const color_t *c, struct igt_fb *fb)
> + const color_t *c, struct igt_fb *fb,
> + bool packed)
> {
> cairo_t *cr;
> + const int localcrop = format == DRM_FORMAT_XRGB8888 ? 0 : data->crop;
>
> - if (data->crop == 0 || format == DRM_FORMAT_XRGB8888) {
> - 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(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);
> + igt_create_fb_with_bo_size(data->drm_fd,
> + width + localcrop * 2,
> + height + localcrop * 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
> - */
> - cr = igt_get_cairo_ctx(data->drm_fd, fb);
> + cr = igt_get_cairo_ctx(data->drm_fd, fb);
>
> + /*
> + * paint border in inverted color, then visible area in middle
> + * with correct color for clamping test
> + */
> + if (localcrop)
> igt_paint_color(cr, 0, 0,
> - width + data->crop * 2,
> - height + data->crop * 2,
> + width + localcrop * 2,
> + height + localcrop * 2,
Doing this localcrop refactoring as a separate patch would have
made this patch a bit less busy.
> 1.0f - c->red,
> 1.0f - c->green,
> 1.0f - c->blue);
>
> - igt_paint_color(cr, data->crop, data->crop,
> +
> + if (packed)
> + draw_entire_color_array(data, cr, format, fb);
> + else
> + igt_paint_color(cr, localcrop, localcrop,
> width, height,
> c->red, c->green, c->blue);
>
> - igt_put_cairo_ctx(cr);
> - }
> -
> + igt_put_cairo_ctx(cr);
> igt_plane_set_fb(plane, fb);
>
> /*
> - * if clamping test. DRM_FORMAT_XRGB8888 is used for reference color.
> + * if clamping test.
> */
> - if (data->crop != 0 && format != DRM_FORMAT_XRGB8888) {
> - igt_fb_set_position(fb, plane, data->crop, data->crop);
> + if (localcrop) {
> + igt_fb_set_position(fb, plane, localcrop, localcrop);
> igt_fb_set_size(fb, plane, width, height);
> igt_plane_set_size(plane, width, height);
> }
> @@ -548,13 +559,35 @@ static void capture_crc(data_t *data, unsigned int vblank, igt_crc_t *crc)
> crc->frame, vblank);
> }
>
> -static void capture_format_crcs(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 crc[], struct igt_fb *fb)
> +static void capture_format_crcs_packed(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 crc[], struct igt_fb *fb)
> +{
> + struct igt_fb old_fb = *fb;
> + const color_t black = { 0.0f, 0.0f, 0.0f };
> +
> + prepare_format_color(data, pipe, plane, format, modifier,
> + width, height, encoding, range, &black, fb, true);
> +
> + igt_display_commit_atomic(&data->display, DRM_MODE_ATOMIC_ALLOW_MODESET,
> + NULL);
> +
> + igt_remove_fb(data->drm_fd, &old_fb);
> +
> + igt_pipe_crc_get_current(data->drm_fd, data->pipe_crc, &crc[0]);
> +}
> +
> +static void capture_format_crcs_planar(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 crc[], struct igt_fb *fb)
> {
> unsigned int vblank[ARRAY_SIZE(colors_extended)];
> struct drm_event_vblank ev;
> @@ -567,7 +600,8 @@ restart_round:
> int ret;
>
> prepare_format_color(data, pipe, plane, format, modifier,
> - width, height, encoding, range, c, fb);
> + width, height, encoding, range, c, fb,
> + false);
>
> if (data->display.is_atomic && i >= 1) {
> igt_assert(read(data->drm_fd, &ev, sizeof(ev)) == sizeof(ev));
> @@ -683,12 +717,20 @@ static bool test_format_plane_colors(data_t *data, enum pipe pipe,
> unsigned int crc_mismatch_mask = 0;
> int crc_mismatch_count = 0;
> bool result = true;
> - int i;
> -
> - capture_format_crcs(data, pipe, plane, format, modifier,
> - width, height, encoding, range, crc, fb);
> -
> - for (i = 0; i < data->num_colors; i++) {
> + int i, total_crcs = 1;
> + bool planar = igt_format_is_yuv_semiplanar(format);
> +
> + if (planar) {
> + capture_format_crcs_planar(data, pipe, plane, format, modifier,
> + width, height, encoding, range, crc,
> + fb);
> + total_crcs = data->num_colors;
> + } else
> + capture_format_crcs_packed(data, pipe, plane, format, modifier,
> + width, height, encoding, range, crc,
> + fb);
> +
> + for (i = 0; i < total_crcs; i++) {
> if (!igt_check_crc_equal(&crc[i], &ref_crc[i])) {
> crc_mismatch_count++;
> crc_mismatch_mask |= (1 << i);
> @@ -776,6 +818,7 @@ static bool test_format_plane_yuv(data_t *data, enum pipe pipe,
> return result;
> }
>
> +enum crc_set {PACKED_CRC_SET = 0, PLANAR_CRC_SET = 1, MAX_CRC_SET = 2};
No point in the =0 & co.
Could use some newlines around these parts.
Apart from those seems good.
Reviewed-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> static bool test_format_plane(data_t *data, enum pipe pipe,
> igt_output_t *output, igt_plane_t *plane)
> {
> @@ -784,7 +827,8 @@ static bool test_format_plane(data_t *data, enum pipe pipe,
> uint32_t format, ref_format;
> uint64_t modifier, ref_modifier;
> uint64_t width, height;
> - igt_crc_t ref_crc[ARRAY_SIZE(colors_extended)];
> + igt_crc_t ref_crc[MAX_CRC_SET][ARRAY_SIZE(colors_extended)];
> + igt_crc_t* crcset;
> bool result = true;
>
> /*
> @@ -843,16 +887,22 @@ static bool test_format_plane(data_t *data, enum pipe pipe,
> igt_remove_fb(data->drm_fd, &test_fb);
> }
>
> - capture_format_crcs(data, pipe, plane, format, modifier,
> - width, height, IGT_COLOR_YCBCR_BT709,
> - IGT_COLOR_YCBCR_LIMITED_RANGE, ref_crc, &fb);
> + capture_format_crcs_packed(data, pipe, plane, format, modifier,
> + width, height, IGT_COLOR_YCBCR_BT709,
> + IGT_COLOR_YCBCR_LIMITED_RANGE,
> + ref_crc[PACKED_CRC_SET], &fb);
> +
> + capture_format_crcs_planar(data, pipe, plane, format, modifier,
> + width, height, IGT_COLOR_YCBCR_BT709,
> + IGT_COLOR_YCBCR_LIMITED_RANGE,
> + ref_crc[PLANAR_CRC_SET], &fb);
>
> /*
> * Make sure we have some difference between the colors. This
> * at least avoids claiming success when everything is just
> * black all the time (eg. if the plane is never even on).
> */
> - igt_require(num_unique_crcs(ref_crc, data->num_colors) > 1);
> + igt_require(num_unique_crcs(ref_crc[PLANAR_CRC_SET], data->num_colors) > 1);
>
> for (int i = 0; i < plane->format_mod_count; i++) {
> format = plane->formats[i];
> @@ -870,16 +920,19 @@ static bool test_format_plane(data_t *data, enum pipe pipe,
> continue;
> }
>
> + crcset = ref_crc[(igt_format_is_yuv_semiplanar(format)
> + ? PLANAR_CRC_SET : PACKED_CRC_SET)];
> +
> if (igt_format_is_yuv(format))
> result &= test_format_plane_yuv(data, pipe, plane,
> format, modifier,
> width, height,
> - ref_crc, &fb);
> + crcset, &fb);
> else
> result &= test_format_plane_rgb(data, pipe, plane,
> format, modifier,
> width, height,
> - ref_crc, &fb);
> + crcset, &fb);
>
> if (format == DRM_FORMAT_C8)
> set_legacy_lut(data, pipe, LUT_MASK);
> --
> 2.28.0
--
Ville Syrjälä
Intel
More information about the igt-dev
mailing list