[igt-dev] [PATCH i-g-t] tests/kms_available_modes_crc limit tested fb formats
Dhinakaran Pandiyan
dhinakaran.pandiyan at intel.com
Thu Feb 21 00:01:59 UTC 2019
On Mon, 2019-02-18 at 16:22 +0200, Juha-Pekka Heikkila wrote:
> This test is causing too much useless noise. Limit tested
> fb formats to DRM_FORMAT_C8 and DRM_FORMAT_XBGR2101010 for now.
> These two formats are currently not tested otherwise thus
> they're left here for now. DRM_FORMAT_XBGR2101010 need to be
> included into IGT supported formats and DRM_FORMAT_C8 test need
> to be moved elsewhere, maybe into kms_plane.
Thanks for doing this.
>
> Signed-off-by: Juha-Pekka Heikkila <juhapekka.heikkila at gmail.com>
> ---
> tests/kms_available_modes_crc.c | 217 +++++++++++++++---------------
> ----------
> 1 file changed, 83 insertions(+), 134 deletions(-)
>
> diff --git a/tests/kms_available_modes_crc.c
> b/tests/kms_available_modes_crc.c
> index 7ff385f..a51006b 100644
> --- a/tests/kms_available_modes_crc.c
> +++ b/tests/kms_available_modes_crc.c
> @@ -101,17 +101,17 @@ static void generate_comparison_crc_list(data_t
> *data, igt_output_t *output)
> igt_plane_set_fb(primary, &data->primary_fb);
> igt_display_commit2(&data->display, data->commit);
>
> + igt_pipe_crc_drain(data->pipe_crc);
> igt_pipe_crc_get_current(data->gfx_fd, data->pipe_crc, &data-
> >cursor_crc);
> igt_plane_set_fb(primary, NULL);
> igt_display_commit2(&data->display, data->commit);
>
> - intel_gen(intel_get_drm_devid(data->gfx_fd)) < 9 ?
> - igt_paint_color(cr, 0, 0, mode->hdisplay, mode-
> >vdisplay, 1.0, 1.0, 1.0) :
> - igt_paint_color_alpha(cr, 0, 0, mode->hdisplay, mode-
> >vdisplay, 1.0, 1.0, 1.0, 1.0);
> + igt_paint_color(cr, 0, 0, mode->hdisplay, mode->vdisplay, 1.0,
> 1.0, 1.0);
>
> igt_plane_set_fb(primary, &data->primary_fb);
> igt_display_commit2(&data->display, data->commit);
>
> + igt_pipe_crc_drain(data->pipe_crc);
> igt_pipe_crc_get_current(data->gfx_fd, data->pipe_crc, &data-
> >fullscreen_crc);
>
> cairo_destroy(cr);
> @@ -122,48 +122,11 @@ static const struct {
> uint32_t fourcc;
> char zeropadding;
> enum { BYTES_PP_1=1,
> - BYTES_PP_2=2,
> - BYTES_PP_4=4,
> - NV12,
> - P010,
> - SKIP4 } bpp;
> + BYTES_PP_4=4} bpp;
> uint32_t value;
> } fillers[] = {
> { DRM_FORMAT_C8, 0, BYTES_PP_1, 0xff},
> - { DRM_FORMAT_RGB565, 0, BYTES_PP_2, 0xffff},
> - { DRM_FORMAT_XRGB8888, 0, BYTES_PP_4, 0xffffffff},
> - { DRM_FORMAT_XBGR8888, 0, BYTES_PP_4, 0xffffffff},
> -
> - /*
> - * following two are skipped because blending seems to work
> - * incorrectly with exception of AR24 on cursor plane.
> - * Test still creates the planes, just filling plane
> - * and getting crc is skipped.
> - */
> - { DRM_FORMAT_ARGB8888, 0, SKIP4, 0xffffffff},
> - { DRM_FORMAT_ABGR8888, 0, SKIP4, 0x00ffffff},
> -
> - { DRM_FORMAT_XRGB2101010, 0, BYTES_PP_4, 0xffffffff},
> { DRM_FORMAT_XBGR2101010, 0, BYTES_PP_4, 0xffffffff},
> -
> - { DRM_FORMAT_YUYV, 0, BYTES_PP_4, 0x80eb80eb},
> - { DRM_FORMAT_YVYU, 0, BYTES_PP_4, 0x80eb80eb},
> - { DRM_FORMAT_VYUY, 0, BYTES_PP_4, 0xeb80eb80},
> - { DRM_FORMAT_UYVY, 0, BYTES_PP_4, 0xeb80eb80},
> -
> - /*
> - * (semi-)planar formats
> - */
> - { DRM_FORMAT_NV12, 0, NV12, 0x80eb},
> -#ifdef DRM_FORMAT_P010
> - { DRM_FORMAT_P010, 0, P010, 0x8000eb00},
> -#endif
> -#ifdef DRM_FORMAT_P012
> - { DRM_FORMAT_P012, 0, P010, 0x8000eb00},
> -#endif
> -#ifdef DRM_FORMAT_P016
> - { DRM_FORMAT_P016, 0, P010, 0x8000eb00},
> -#endif
> { 0, 0, 0, 0 }
> };
>
> @@ -175,10 +138,9 @@ static bool fill_in_fb(data_t *data,
> igt_output_t *output, igt_plane_t *plane,
> uint32_t format)
> {
> signed i, c, writesize;
> - unsigned short* ptemp_16_buf;
> unsigned int* ptemp_32_buf;
>
> - for( i = 0; fillers[i].fourcc != 0; i++ ) {
> + for( i = 0; i < ARRAY_SIZE(fillers)-1; i++ ) {
> if( fillers[i].fourcc == format )
> break;
> }
> @@ -190,53 +152,10 @@ static bool fill_in_fb(data_t *data,
> igt_output_t *output, igt_plane_t *plane,
> ptemp_32_buf[c] = fillers[i].value;
> writesize = data->size;
> break;
> - case BYTES_PP_2:
> - ptemp_16_buf = (unsigned short*)data->buf;
> - for (c = 0; c < data->size/2; c++)
> - ptemp_16_buf[c] = (unsigned
> short)fillers[i].value;
> - writesize = data->size;
> - break;
> case BYTES_PP_1:
> memset((void *)data->buf, fillers[i].value, data-
> >size);
> writesize = data->size;
> break;
> - case NV12:
> - memset((void *)data->buf, fillers[i].value&0xff,
> - data->fb.offsets[1]);
> -
> - memset((void *)(data->buf+data->fb.offsets[1]),
> - (fillers[i].value>>8)&0xff,
> - data->size - data->fb.offsets[1]);
> -
> - writesize = data->size;
> - break;
> - case P010:
> - ptemp_16_buf = (unsigned short*)data->buf;
> - for (c = 0; c < data->size/2; c++)
> - ptemp_16_buf[c] = (unsigned
> short)fillers[i].value&0xffff;
> -
> - ptemp_16_buf = (unsigned short*)(data->buf+data->size);
> - for (c = 0; c < data->size/2; c++)
> - ptemp_16_buf[c] = (unsigned
> short)(fillers[i].value>>16)&0xffff;
> -
> - writesize = data->size+data->size/2;
> - break;
> - case SKIP4:
> - if (fillers[i].fourcc == DRM_FORMAT_ARGB8888 &&
> - plane->type == DRM_PLANE_TYPE_CURSOR) {
> - /*
> - * special for cursor plane where blending works
> correctly.
> - */
> - ptemp_32_buf = (unsigned int*)data->buf;
> - for (c = 0; c < data->size/4; c++)
> - ptemp_32_buf[c] = fillers[i].value;
> - writesize = data->size;
> - break;
> - }
> - igt_info("Format %s CRC comparison skipped by
> design.\n",
> - (char*)&fillers[i].fourcc);
> -
> - return false;
> default:
> igt_info("Unsupported mode for test %s\n",
> (char*)&fillers[i].fourcc);
> @@ -271,26 +190,20 @@ static bool setup_fb(data_t *data, igt_output_t
> *output, igt_plane_t *plane,
> tiling = LOCAL_DRM_FORMAT_MOD_NONE;
> }
>
> - for (i = 0; fillers[i].fourcc != 0; i++) {
> - if (fillers[i].fourcc == format)
> + for( i = 0; i < ARRAY_SIZE(fillers)-1; i++ ) {
> + if( fillers[i].fourcc == format )
> break;
> }
>
> switch (fillers[i].bpp) {
> - case NV12:
> case BYTES_PP_1:
> bpp = 8;
> break;
> -
> - case P010:
> - case BYTES_PP_2:
> - bpp = 16;
> - break;
> -
> - case SKIP4:
> case BYTES_PP_4:
> bpp = 32;
> break;
> + default:
> + return false;
Hmm, we should never hit this condition. Mark this as a failure so that
the test gets fixed?
> }
>
> igt_get_fb_tile_size(data->gfx_fd, tiling, bpp,
> @@ -299,17 +212,6 @@ static bool setup_fb(data_t *data, igt_output_t
> *output, igt_plane_t *plane,
> data->fb.strides[0] = ALIGN(w * bpp / 8, tile_width);
> gemsize = data->size = data->fb.strides[0] * ALIGN(h,
> tile_height);
>
> - if (fillers[i].bpp == P010 || fillers[i].bpp == NV12) {
> - data->fb.offsets[1] = data->size;
> - data->fb.strides[1] = data->fb.strides[0];
> - gemsize = data->size * 2;
> -
> - if (fillers[i].bpp == NV12)
> - data->size += data->fb.strides[1] * ALIGN(h/2,
> tile_height);
> -
> - num_planes = 2;
> - }
> -
> data->gem_handle = gem_create(data->gfx_fd, gemsize);
> ret = __gem_set_tiling(data->gfx_fd, data->gem_handle,
> igt_fb_mod_to_tiling(tiling),
> @@ -386,12 +288,23 @@ static bool prepare_crtc(data_t *data,
> igt_output_t *output,
>
> static int
> test_one_mode(data_t* data, igt_output_t *output, igt_plane_t*
> plane,
> - int mode)
> + int mode, enum pipe pipe)
s/mode/format
Comment applies to other places too.
> {
> igt_crc_t current_crc;
> signed rVal = 0;
> bool do_crc;
> - char* crccompare[2];
> + int i;
> +
> + /*
> + * Limit tests only to those fb formats listed in fillers table
> + */
> + for( i = 0; i < ARRAY_SIZE(fillers)-1; i++ ) {
> + if( fillers[i].fourcc == mode )
> + break;
> + }
> +
> + if(fillers[i].bpp == 0)
> + return false;
.bpp is always valid for the two formats that are tested, isn't it? In
that case .bpp == 0 is a test failure.
>
> if (prepare_crtc(data, output, plane, mode)){
> /*
> @@ -412,29 +325,30 @@ test_one_mode(data_t* data, igt_output_t
> *output, igt_plane_t* plane,
> if (plane->type != DRM_PLANE_TYPE_CURSOR) {
> if (!igt_check_crc_equal(¤t_crc,
> &data->fullscreen_crc)) {
> - crccompare[0] =
> igt_crc_to_string(¤t_crc);
> - crccompare[1] =
> igt_crc_to_string(&data->fullscreen_crc);
> - igt_warn("crc mismatch. target
> %.8s, result %.8s.\n", crccompare[0], crccompare[1]);
> - free(crccompare[0]);
> - free(crccompare[1]);
> + igt_warn("crc mismatch.
> connector %s using pipe %s" \
> + " plane index %d mode
> %.4s\n",
> + igt_output_name(output)
> ,
> + kmstest_pipe_name(pipe)
> ,
> + plane->index,
> + (char*)&mode);
> rVal++;
> }
> } else {
> if (!igt_check_crc_equal(¤t_crc,
> &data->cursor_crc)) {
> - crccompare[0] =
> igt_crc_to_string(¤t_crc);
> - crccompare[1] =
> igt_crc_to_string(&data->cursor_crc);
> - igt_warn("crc mismatch. target
> %.8s, result %.8s.\n", crccompare[0], crccompare[1]);
> - free(crccompare[0]);
> - free(crccompare[1]);
> + igt_warn("crc mismatch.
> connector %s using pipe %s" \
> + " plane index %d mode
> %.4s\n",
> + igt_output_name(output)
> ,
> + kmstest_pipe_name(pipe)
> ,
> + plane->index,
> + (char*)&mode);
> rVal++;
> }
> }
> }
> - remove_fb(data, output, plane);
> - return rVal;
> }
> - return 1;
> + remove_fb(data, output, plane);
> + return rVal;
> }
>
>
> @@ -445,14 +359,44 @@ test_available_modes(data_t* data)
> igt_plane_t *plane;
> int modeindex;
> enum pipe pipe;
> - int invalids = 0;
> + int invalids = 0, i, lut_size;
> drmModePlane *modePlane;
> - char planetype[3][8] = {"OVERLAY\0", "PRIMARY\0", "CURSOR\0" };
> +
> + struct {
> + uint16_t red;
> + uint16_t green;
> + uint16_t blue;
> + uint16_t reserved;
> + } *lut = NULL;
>
> for_each_pipe_with_valid_output(&data->display, pipe, output) {
> igt_output_set_pipe(output, pipe);
> igt_display_commit2(&data->display, data->commit);
>
> + if (igt_pipe_obj_has_prop(&data->display.pipes[pipe],
> IGT_CRTC_GAMMA_LUT_SIZE)) {
> + lut_size = igt_pipe_get_prop(&data->display,
> pipe,
> + IGT_CRTC_GAMMA_LUT
> _SIZE);
> +
> + lut = calloc(sizeof(*lut), lut_size);
> +
> + for (i = 0; i < lut_size; i++) {
> + lut[i].red = (i * 0xffff / (lut_size -
> 1)) & 0x8000;
> + lut[i].green = (i * 0xffff / (lut_size
> - 1)) & 0x8000;
> + lut[i].blue = (i * 0xffff / (lut_size -
> 1)) & 0x8000;
> + }
> +
> + igt_pipe_replace_prop_blob(&data->display,
> pipe,
> + IGT_CRTC_GAMMA_LUT,
> + lut, sizeof(*lut) *
> lut_size);
> + igt_display_commit2(&data->display, data-
> >commit);
> +
> + for (i = 0; i < lut_size; i++) {
> + lut[i].red = i * 0xffff / (lut_size -
> 1);
> + lut[i].green = i * 0xffff / (lut_size -
> 1);
> + lut[i].blue = i * 0xffff / (lut_size -
> 1);
> + }
> + }
> +
> data->pipe_crc = igt_pipe_crc_new(data->gfx_fd, pipe,
> INTEL_PIPE_CRC_SOURCE
> _AUTO);
>
> @@ -467,29 +411,34 @@ test_available_modes(data_t* data)
> modePlane = drmModeGetPlane(data->gfx_fd,
> plane->drm_plane-
> >plane_id);
>
> + if (plane->type == DRM_PLANE_TYPE_CURSOR)
> + continue;
> +
> for (modeindex = 0;
> modeindex < modePlane->count_formats;
> modeindex++) {
> data->format.dword = modePlane-
> >formats[modeindex];
>
> - igt_info("Testing connector %s using
> pipe %s" \
> - " plane index %d type %s mode
> %s\n",
> - igt_output_name(output),
> - kmstest_pipe_name(pipe),
> - plane->index,
> - planetype[plane->type],
> - (char*)&data->format.name);
Not sure why this was removed, perhaps convert it to igt_debug()? It's
hard to tell what is being tested now even with the --debug option.
If you want to address the comments in a separate patch, feel free to
add
Reviewed-by: Dhinakaran Pandiyan <dhinakaran.pandiyan at intel.com>
> -
> invalids += test_one_mode(data, output,
> plane,
> - modePlane-
> >formats[modeindex]);
> + modePlane-
> >formats[modeindex],
> + pipe);
> }
> drmModeFreePlane(modePlane);
> }
> igt_pipe_crc_stop(data->pipe_crc);
> igt_pipe_crc_free(data->pipe_crc);
> - igt_display_commit2(&data->display, data->commit);
> +
> + if (lut != NULL) {
> + igt_pipe_replace_prop_blob(&data->display,
> pipe,
> + IGT_CRTC_GAMMA_LUT,
> + lut, sizeof(*lut) *
> lut_size);
> + free(lut);
> + lut = NULL;
> + }
> +
> igt_output_set_pipe(output, PIPE_NONE);
> + igt_display_commit2(&data->display, data->commit);
> }
> igt_assert(invalids == 0);
> }
More information about the igt-dev
mailing list