[igt-dev] [PATCH i-g-t] tests/kms_flip_scaled_crc: add flip to downscaled subtests
Juha-Pekka Heikkila
juhapekka.heikkila at gmail.com
Mon Sep 14 11:58:34 UTC 2020
On 4.9.2020 17.31, Shankar, Uma wrote:
>
>
>> -----Original Message-----
>> From: igt-dev <igt-dev-bounces at lists.freedesktop.org> On Behalf Of Juha-Pekka
>> Heikkila
>> Sent: Wednesday, September 2, 2020 2:42 PM
>> To: igt-dev at lists.freedesktop.org
>> Subject: [igt-dev] [PATCH i-g-t] tests/kms_flip_scaled_crc: add flip to downscaled
>> subtests
>
> I think this would sound better "Add Subtests for Flip with fb downscaling"
>
>>
>> attempt to cause cdclk changes and stress scalers with flipping to different size fb
>> with different modifiers. Verify results with crc.
>
> This is failing on some platforms like in GLK:
> https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4945/shard-glk9/igt@kms_flip_scaled_crc@flip-32bpp-ytile-to-32bpp-ytileccs.html
>
> Can you check why it could be happening, I can see you have the modifier check. But still its rejecting the combination in driver.
> May be something to fix.
This I suspect is something problematic on i915 side, what is returned
there is -EINVAL from flip. Those GLK dmesgs generally look somehow
really messy, it is for some reason all the time trying to do something
with pipe b even when it's supposing to be disabled.
>
> Overall this is really a good plan to have this combination tested, something we have lacked.
There was regression on drm-tip on these which was just recently fixed
hence this test. If for example would have run some videoplayer which
uses hw downscaling result would've been just black screen ;)
>
>> Signed-off-by: Juha-Pekka Heikkila <juhapekka.heikkila at gmail.com>
>> ---
>> tests/Makefile.sources | 1 +
>> tests/kms_flip_scaled_crc.c | 255 ++++++++++++++++++++++++++++++++++++
>> tests/meson.build | 1 +
>> 3 files changed, 257 insertions(+)
>> create mode 100644 tests/kms_flip_scaled_crc.c
>>
>> diff --git a/tests/Makefile.sources b/tests/Makefile.sources index
>> 269b506d..fe9fb7ed 100644
>> --- a/tests/Makefile.sources
>> +++ b/tests/Makefile.sources
>> @@ -53,6 +53,7 @@ TESTS_progs = \
>> kms_fence_pin_leak \
>> kms_flip \
>> kms_flip_event_leak \
>> + kms_flip_scaled_crc \
>> kms_flip_tiling \
>> kms_force_connector_basic \
>> kms_frontbuffer_tracking \
>> diff --git a/tests/kms_flip_scaled_crc.c b/tests/kms_flip_scaled_crc.c new file
>> mode 100644 index 00000000..9daa2f92
>> --- /dev/null
>> +++ b/tests/kms_flip_scaled_crc.c
>> @@ -0,0 +1,255 @@
>> +/*
>> + * Copyright © 2020 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 (including the
>> +next
>> + * paragraph) 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.
>> + *
>> + */
>> +
>> +#include "igt.h"
>> +
>> +IGT_TEST_DESCRIPTION("Test flipping between scaled/nonscaled
>> +framebuffers");
>> +
>> +typedef struct {
>> + int drm_fd;
>> + igt_display_t display;
>> + igt_output_t *output;
>> + enum pipe pipe;
>> + uint32_t gen;
>> + struct igt_fb small_fb;
>> + struct igt_fb big_fb;
>> + igt_pipe_crc_t *pipe_crc;
>> +} data_t;
>> +
>> +const struct {
>> + const char * const name;
>> + const char * const describe;
>> + const uint64_t firstmodifier;
>> + const uint32_t firstformat;
>> + const uint64_t secondmodifier;
>> + const uint32_t secondformat;
>> +} flip_scenario_test[] = {
>> + {
>> + "flip-32bpp-ytile-to-64bpp-ytile",
>> + "Try to flip from 32bpp non scaled fb to 64bpp downscaled fb in
>> attempt to stress cdclk reprogramming.",
>
> We can trim this description, Like: "Flip from 32bpp non scaled fb to 64bpp downscaled fb to stress CD clock programming"
> Trim this for all combinations.
>
>> + LOCAL_I915_FORMAT_MOD_Y_TILED, DRM_FORMAT_XRGB8888,
>> + LOCAL_I915_FORMAT_MOD_Y_TILED,
>> DRM_FORMAT_XRGB16161616F
>> + },
>> + {
>> + "flip-64bpp-ytile-to-32bpp-ytile",
>> + "Try to flip from 64bpp non scaled fb to 32bpp downscaled fb.",
>> + LOCAL_I915_FORMAT_MOD_Y_TILED,
>> DRM_FORMAT_XRGB16161616F,
>> + LOCAL_I915_FORMAT_MOD_Y_TILED, DRM_FORMAT_XRGB8888
>> + },
>> + {
>> + "flip-64bpp-ytile-to-16bpp-ytile",
>> + "Try to flip from 64bpp non scaled fb to 16bpp downscaled fb.",
>> + LOCAL_I915_FORMAT_MOD_Y_TILED,
>> DRM_FORMAT_XRGB16161616F,
>> + LOCAL_I915_FORMAT_MOD_Y_TILED, DRM_FORMAT_RGB565
>> + },
>> + {
>> + "flip-32bpp-ytileccs-to-64bpp-ytile",
>> + "Try to flip from 32bpp ccs non scaled fb to 64bpp downscaled fb
>> ytile.",
>> + LOCAL_I915_FORMAT_MOD_Y_TILED_CCS,
>> DRM_FORMAT_XRGB8888,
>> + LOCAL_I915_FORMAT_MOD_Y_TILED,
>> DRM_FORMAT_XRGB16161616F
>> + },
>> + {
>> + "flip-32bpp-ytile-to-32bpp-ytilegen12rcccs",
>> + "Try to flip from 32bpp non scaled fb to 32bpp downscaled gen12
>> rc ccs fb in attempt to stress cdclk reprogramming.",
>> + LOCAL_I915_FORMAT_MOD_Y_TILED, DRM_FORMAT_XRGB8888,
>> + LOCAL_I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS,
>> DRM_FORMAT_XRGB8888
>> + },
>> + {
>> + "flip-32bpp-ytile-to-32bpp-ytileccs",
>> + "Try to flip from 32bpp non scaled ytiled fb to 32bpp downscaled
>> ytiled ccs fb.",
>> + LOCAL_I915_FORMAT_MOD_Y_TILED, DRM_FORMAT_XRGB8888,
>> + LOCAL_I915_FORMAT_MOD_Y_TILED_CCS,
>> DRM_FORMAT_XRGB8888
>> + },
>> + {
>> + "flip-64bpp-ytile-to-32bpp-ytilercccs",
>> + "Try to flip from 64bpp non scaled ytiled fb to 32bpp downscaled
>> gen12 rc ccs fb.",
>> + LOCAL_I915_FORMAT_MOD_Y_TILED,
>> DRM_FORMAT_XRGB16161616F,
>> + LOCAL_I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS,
>> DRM_FORMAT_XRGB8888
>> + },
>> +};
>> +
>> +static void setup_fb(data_t *data, struct igt_fb *newfb, uint32_t width,
>> + uint32_t height, uint64_t format, uint64_t modifier) {
>> + struct drm_mode_fb_cmd2 f = {0};
>> + cairo_t *cr;
>> +
>> + igt_require(igt_display_has_format_mod(&data->display, format,
>> + modifier));
>> +
>> + igt_create_bo_for_fb(data->drm_fd, width, height, format, modifier,
>> + newfb);
>> + igt_assert(newfb->gem_handle > 0);
>> +
>> + f.width = newfb->width;
>> + f.height = newfb->height;
>> + f.pixel_format = newfb->drm_format;
>> + f.flags = LOCAL_DRM_MODE_FB_MODIFIERS;
>> +
>> + for (int n = 0; n < newfb->num_planes; n++) {
>> + f.handles[n] = newfb->gem_handle;
>> + f.modifier[n] = newfb->modifier;
>> + f.pitches[n] = newfb->strides[n];
>> + f.offsets[n] = newfb->offsets[n];
>> + }
>> +
>> + cr = igt_get_cairo_ctx(data->drm_fd, newfb);
>> + igt_paint_color(cr, 0, 0, newfb->width, newfb->height, 0, 1, 0);
>> + igt_put_cairo_ctx(cr);
>> +
>> + igt_assert(drmIoctl(data->drm_fd, LOCAL_DRM_IOCTL_MODE_ADDFB2,
>> &f) == 0);
>> + newfb->fb_id = f.fb_id;
>> +}
>> +
>> +static void free_fbs(data_t *data)
>> +{
>> + igt_remove_fb(data->drm_fd, &data->small_fb);
>> + igt_remove_fb(data->drm_fd, &data->big_fb); }
>> +
>> +static void test_flip_to_scaled(data_t *data, uint32_t index, enum pipe pipe,
>> + igt_output_t *output)
>> +{
>> + igt_plane_t *primary;
>> + igt_crc_t small_crc, big_crc;
>> +
>> + drmModeModeInfo *mode;
>> + struct drm_event_vblank ev;
>> + int ret;
>> +
>> + igt_display_reset(&data->display);
>> +
>> + igt_debug("running on output %s pipe %s\n", output->name,
>> + kmstest_pipe_name(pipe));
>> + if (data->big_fb.fb_id == 0) {
>> + mode = igt_output_get_mode(output);
>> +
>> + // big fb will be 4k unless running on older than ICL
>> + if (data->gen < 11) {
>> + setup_fb(data, &data->small_fb,
>> + min(mode->hdisplay, 640),
>> + min(mode->vdisplay, 480),
>> + flip_scenario_test[index].firstformat,
>> + flip_scenario_test[index].firstmodifier);
>> +
>> + setup_fb(data, &data->big_fb,
>> + 1280, 960,
>
> Not sure, but just something to check. Whether this downscaling will trigger a cd clock change for all platforms.
> We should somehow check that as well if cd clock change really happened. CD clock steps are not uniform across platforms.
> Also if driver hardcodes a higher cd clock this test may pass without its intended objective.
I cannot enforce cdclk changes but what happen here with downscaling is
it always go 4:1 I figure there's minimum four times more samples needed
with filtering. As much as I verified those dmesgs from CI there was
always display clock reprogrammed on all platforms but it is just what
happen on today's drm-tip. If would run this on i915 from year ago there
would be no such change thus on description it just says 'attempt to
stress cdclk'.
As a flipping test I think final intended objective is flipping between
framebuffers succeeded and it is verified with crc below.
>
>> + flip_scenario_test[index].secondformat,
>> + flip_scenario_test[index].secondmodifier);
>> + igt_debug("small fb %dx%d\n", data->small_fb.width,
>> + data->small_fb.height);
>> + igt_debug("big fb %dx%d\n", data->big_fb.width,
>> + data->big_fb.height);
>> + } else {
>> + setup_fb(data, &data->small_fb,
>> + min(mode->hdisplay, 1920),
>> + min(mode->vdisplay, 1080),
>> + flip_scenario_test[index].firstformat,
>> + flip_scenario_test[index].firstmodifier);
>> +
>> + setup_fb(data, &data->big_fb,
>> + 3840, 2160,
>> + flip_scenario_test[index].secondformat,
>> + flip_scenario_test[index].secondmodifier);
>> + igt_debug("small fb %dx%d\n", data->small_fb.width,
>> + data->small_fb.height);
>> + igt_debug("big fb %dx%d\n", data->big_fb.width,
>> + data->big_fb.height);
>> + }
>> + }
>> +
>> + igt_output_set_pipe(output, pipe);
>> +
>> + primary = igt_output_get_plane_type(output,
>> DRM_PLANE_TYPE_PRIMARY);
>> + igt_display_commit_atomic(&data->display,
>> DRM_MODE_ATOMIC_ALLOW_MODESET,
>> + NULL);
>> + data->pipe_crc = igt_pipe_crc_new(data->drm_fd, pipe,
>> + INTEL_PIPE_CRC_SOURCE_AUTO);
>> + igt_pipe_crc_start(data->pipe_crc);
>> +
>> + igt_plane_set_position(primary, 0, 0);
>> + igt_plane_set_fb(primary, &data->small_fb);
>> +
>> + igt_display_commit_atomic(&data->display,
>> + DRM_MODE_ATOMIC_ALLOW_MODESET, NULL);
>> + igt_pipe_crc_get_current(data->drm_fd, data->pipe_crc, &small_crc);
>> +
>> + igt_plane_set_fb(primary, &data->big_fb);
>> + igt_plane_set_size(primary, data->small_fb.width,
>> + data->small_fb.height);
>> + ret = igt_display_try_commit_atomic(&data->display,
>> +
>> DRM_MODE_ATOMIC_ALLOW_MODESET |
>> + DRM_MODE_PAGE_FLIP_EVENT, NULL);
>> +
>> + igt_require_f(ret != -ERANGE,
>> + "Platform scaling limits exceeded, skipping.");
>> + igt_assert_eq(ret, 0);
>> +
>> + igt_assert(read(data->drm_fd, &ev, sizeof(ev)) == sizeof(ev));
>> +
>> + igt_pipe_crc_get_current(data->drm_fd, data->pipe_crc, &big_crc);
>> + igt_assert(igt_check_crc_equal(&small_crc, &big_crc));
>
> With this downscaling, are we able to match crc. Scalar may do some rounding, so not sure if this will
> match in all cases. Good to check this.
We cannot really do crc checking with scalers but I have idea here
maximum solid green would stay maximum solid green regardless of the
scaler, all components for scaler are maximum solid green. With these
starting values if scaler couldn't produce maximum solid green I'd call
scaler broken.
>
>> +
>> + igt_pipe_crc_stop(data->pipe_crc);
>> + igt_pipe_crc_free(data->pipe_crc);
>> + data->pipe_crc = NULL;
>> +}
>> +
>> +igt_main
>> +{
>> + enum pipe pipe;
>> + data_t data = {};
>> + igt_output_t *output;
>> +
>> + igt_fixture {
>> + data.drm_fd = drm_open_driver_master(DRIVER_INTEL);
>> + data.gen = intel_gen(intel_get_drm_devid(data.drm_fd));
>> + igt_require(data.gen >= 9);
>> + igt_display_require(&data.display, data.drm_fd);
>> + igt_require(data.display.is_atomic);
>> + igt_require_pipe_crc(data.drm_fd);
>> + kmstest_set_vt_graphics_mode();
>> + }
>> +
>> + for (int index = 0; index < ARRAY_SIZE(flip_scenario_test); index++) {
>> + igt_describe(flip_scenario_test[index].describe);
>> + igt_subtest(flip_scenario_test[index].name) {
>> + for_each_pipe_with_single_output(&data.display, pipe,
>> + output)
>> + test_flip_to_scaled(&data, index, pipe, output);
>> +
>> + free_fbs(&data);
>> + }
>> + }
>> + igt_fixture {
>> + free_fbs(&data);
>> + if (data.pipe_crc) {
>> + igt_pipe_crc_stop(data.pipe_crc);
>> + igt_pipe_crc_free(data.pipe_crc);
>> + data.pipe_crc = NULL;
>> + }
>> + kmstest_set_vt_text_mode();
>> + igt_display_fini(&data.display);
>> + }
>> +}
>> diff --git a/tests/meson.build b/tests/meson.build index 684de043..8e17a6ae
>> 100644
>> --- a/tests/meson.build
>> +++ b/tests/meson.build
>> @@ -37,6 +37,7 @@ test_progs = [
>> 'kms_fence_pin_leak',
>> 'kms_flip',
>> 'kms_flip_event_leak',
>> + 'kms_flip_scaled_crc',
>> 'kms_flip_tiling',
>> 'kms_force_connector_basic',
>> 'kms_frontbuffer_tracking',
>> --
>> 2.26.0
>>
>> _______________________________________________
>> igt-dev mailing list
>> igt-dev at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/igt-dev
More information about the igt-dev
mailing list