From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by gabe.freedesktop.org (Postfix) with ESMTPS id 499536E830 for ; Tue, 1 Dec 2020 16:30:01 +0000 (UTC) Date: Tue, 1 Dec 2020 18:29:55 +0200 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Message-ID: <20201201162955.GY6112@intel.com> References: <20201127224030.8912-1-juhapekka.heikkila@gmail.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20201127224030.8912-1-juhapekka.heikkila@gmail.com> Subject: Re: [igt-dev] [PATCH i-g-t 1/2] tests/kms_plane: optimize pixel format tests List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Errors-To: igt-dev-bounces@lists.freedesktop.org Sender: "igt-dev" To: Juha-Pekka Heikkila Cc: igt-dev@lists.freedesktop.org List-ID: 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 > --- > 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 pi= pe 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 =3D ARRAY_SIZE(colors_extended); > + const int x =3D format =3D=3D DRM_FORMAT_XRGB8888 ? 0 : data->crop; > + > + for (int n =3D 0; n < color_amount; n++) { > + int y =3D (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 =3D format =3D=3D DRM_FORMAT_XRGB8888 ? 0 : data->c= rop; > = > - if (data->crop =3D=3D 0 || format =3D=3D DRM_FORMAT_XRGB8888) { > - igt_create_fb_with_bo_size(data->drm_fd, width, height, > - format, modifier, color_encoding, > - color_range, fb, 0, 0); > - > - cr =3D 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 =3D igt_get_cairo_ctx(data->drm_fd, fb); > + cr =3D 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 !=3D 0 && format !=3D 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 =3D *fb; > + const color_t black =3D { 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 >=3D 1) { > igt_assert(read(data->drm_fd, &ev, sizeof(ev)) =3D=3D sizeof(ev)); > @@ -683,12 +717,20 @@ static bool test_format_plane_colors(data_t *data, = enum pipe pipe, > unsigned int crc_mismatch_mask =3D 0; > int crc_mismatch_count =3D 0; > bool result =3D true; > - int i; > - > - capture_format_crcs(data, pipe, plane, format, modifier, > - width, height, encoding, range, crc, fb); > - > - for (i =3D 0; i < data->num_colors; i++) { > + int i, total_crcs =3D 1; > + bool planar =3D 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 =3D data->num_colors; > + } else > + capture_format_crcs_packed(data, pipe, plane, format, modifier, > + width, height, encoding, range, crc, > + fb); > + > + for (i =3D 0; i < total_crcs; i++) { > if (!igt_check_crc_equal(&crc[i], &ref_crc[i])) { > crc_mismatch_count++; > crc_mismatch_mask |=3D (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 =3D 0, PLANAR_CRC_SET =3D 1, MAX_CRC_SET = =3D 2}; No point in the =3D0 & co. Could use some newlines around these parts. Apart from those seems good. Reviewed-by: Ville Syrj=E4l=E4 > 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 =3D true; > = > /* > @@ -843,16 +887,22 @@ static bool test_format_plane(data_t *data, enum pi= pe 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 =3D 0; i < plane->format_mod_count; i++) { > format =3D plane->formats[i]; > @@ -870,16 +920,19 @@ static bool test_format_plane(data_t *data, enum pi= pe pipe, > continue; > } > = > + crcset =3D ref_crc[(igt_format_is_yuv_semiplanar(format) > + ? PLANAR_CRC_SET : PACKED_CRC_SET)]; > + > if (igt_format_is_yuv(format)) > result &=3D test_format_plane_yuv(data, pipe, plane, > format, modifier, > width, height, > - ref_crc, &fb); > + crcset, &fb); > else > result &=3D test_format_plane_rgb(data, pipe, plane, > format, modifier, > width, height, > - ref_crc, &fb); > + crcset, &fb); > = > if (format =3D=3D DRM_FORMAT_C8) > set_legacy_lut(data, pipe, LUT_MASK); > -- = > 2.28.0 -- = Ville Syrj=E4l=E4 Intel _______________________________________________ igt-dev mailing list igt-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/igt-dev