| Message ID | 1327d62b-3083-8761-dbed-7ae40004d1fb@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Maarten Sorry, my bad. This was a wrong push from my end. I have changed the tag to Not applicable. Apologies. Have sent out the NV12 series separately. Regards Vidya > -----Original Message----- > From: Maarten Lankhorst [mailto:maarten.lankhorst@linux.intel.com] > Sent: Tuesday, February 6, 2018 7:33 PM > To: Srinivas, Vidya <vidya.srinivas@intel.com>; intel- > gfx@lists.freedesktop.org > Cc: Kamath, Sunil <sunil.kamath@intel.com>; Sharma, Shashank > <shashank.sharma@intel.com>; Shankar, Uma <uma.shankar@intel.com>; > Konduru, Chandra <chandra.konduru@intel.com> > Subject: Re: [PATCH] drm/i915: Display WA #0827 for NV12 to RGB switch > > Hey, > > Op 06-02-18 om 12:06 schreef Vidya Srinivas: > > From: Chandra Konduru <chandra.konduru@intel.com> > > > > Display WA #0827: > > Switching the plane format from NV12 to RGB and leaving system idle > > results in display underrun and corruption. WA: Set the bit 15 & bit > > 19 to 1b in the CLKGATE_DIS_PSL register for the pipe in which NV12 plane > is enabled. > > > > Signed-off-by: Chandra Konduru <chandra.konduru@intel.com> > > Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com> > > Is it required to leave the workaround enabled all the time? And how can I > reproduce it? I tried to write a dumb testcase (below) to expose this issue, > but didn't have much luck.. > --- > diff --git a/tests/Makefile.sources b/tests/Makefile.sources index > 870c9093550b..f536db0fa433 100644 > --- a/tests/Makefile.sources > +++ b/tests/Makefile.sources > @@ -192,6 +192,7 @@ TESTS_progs = \ > kms_legacy_colorkey \ > kms_mmap_write_crc \ > kms_mmio_vs_cs_flip \ > + kms_nv12 \ > kms_panel_fitting \ > kms_pipe_b_c_ivb \ > kms_pipe_crc_basic \ > diff --git a/tests/kms_nv12.c b/tests/kms_nv12.c new file mode 100644 > index 000000000000..384a15afde52 > --- /dev/null > +++ b/tests/kms_nv12.c > @@ -0,0 +1,195 @@ > +/* > + * 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 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. > + * > + * Authors: > + * Maarten Lankhorst > + */ > +#include "config.h" > + > +#include "igt.h" > +#include > +#include > +#include > +#include > +#include > + > +typedef struct { > + igt_display_t display; > + > + igt_pipe_crc_t *pipe_crc; > + struct igt_fb fb[4]; > +} data_t; > + > +static bool plane_supports_format(igt_plane_t *plane, uint32_t format) > +{ > + int i; > + > + if (!igt_fb_supported_format(format)) > + return false; > + > + for (i = 0; i < plane->drm_plane->count_formats; i++) > + if (plane->drm_plane->formats[i] == format) > + return true; > + > + return false; > +} > + > +static bool pipe_supports_format(igt_display_t *display, enum pipe > +pipe, uint32_t format) { > + igt_plane_t *plane; > + > + for_each_plane_on_pipe(display, pipe, plane) > + if (plane_supports_format(plane, format)) > + return true; > + > + return false; > +} > + > +static void remove_fbs(data_t *data) > +{ > + int i; > + > + for (i = 0; i < ARRAY_SIZE(data->fb); i++) > + igt_remove_fb(data->display.drm_fd, &data->fb[i]); } > + > +static void prepare_crtc(data_t *data, enum pipe pipe, igt_output_t > +*output) { > + igt_display_t *display = &data->display; > + > + remove_fbs(data); > + igt_display_reset(display); > + igt_output_set_pipe(output, pipe); > + igt_display_commit2(display, COMMIT_ATOMIC); > + > + igt_pipe_crc_free(data->pipe_crc); > + data->pipe_crc = igt_pipe_crc_new(display->drm_fd, pipe, > +INTEL_PIPE_CRC_SOURCE_AUTO); } > + > +static void set_fb(igt_plane_t *plane, struct igt_fb *fb) { > + igt_plane_set_fb(plane, fb); > + > + if (fb && fb->tiling == LOCAL_I915_FORMAT_MOD_Y_TILED) { > + igt_plane_set_rotation(plane, IGT_ROTATION_90); > + igt_plane_set_size(plane, fb->height, fb->width); > + } else > + igt_plane_set_rotation(plane, IGT_ROTATION_0); } > + > +static void nv12_rgb_switch(data_t *data, enum pipe pipe, igt_output_t > +*output) { > + drmModeModeInfo *mode = igt_output_get_mode(output); > + igt_display_t *display = &data->display; > + igt_plane_t *plane; > + int i; > + igt_crc_t ref_crc[4], crc; > + > + prepare_crtc(data, pipe, output); > + > + igt_create_pattern_fb(display->drm_fd, mode->hdisplay, mode- > >vdisplay, > + DRM_FORMAT_NV12, > LOCAL_I915_FORMAT_MOD_X_TILED, > + &data->fb[0]); > + > + igt_create_pattern_fb(display->drm_fd, mode->vdisplay, mode- > >hdisplay, > + DRM_FORMAT_NV12, > LOCAL_I915_FORMAT_MOD_Y_TILED, > + &data->fb[1]); > + > + igt_create_pattern_fb(display->drm_fd, mode->hdisplay, mode- > >vdisplay, > + DRM_FORMAT_XRGB8888, > LOCAL_I915_FORMAT_MOD_X_TILED, > + &data->fb[2]); > + > + igt_create_pattern_fb(display->drm_fd, mode->vdisplay, mode- > >hdisplay, > + DRM_FORMAT_XRGB8888, > LOCAL_I915_FORMAT_MOD_Y_TILED, > + &data->fb[3]); > + > + for_each_plane_on_pipe(display, pipe, plane) > + if (plane->type != DRM_PLANE_TYPE_CURSOR) > + set_fb(plane, &data->fb[3]); > + > + for_each_plane_on_pipe(display, pipe, plane) { > + const int seq[] = { > + 2, 0, 2, 1, 3, 1, 3 > + }; > + > + if (!plane_supports_format(plane, DRM_FORMAT_NV12)) > + continue; > + > + /* Collect reference crc with toggle in between. */ > + for (i = 0; i < ARRAY_SIZE(ref_crc); i++) { > + set_fb(plane, &data->fb[i]); > + igt_display_commit2(display, COMMIT_ATOMIC); > + > + igt_pipe_crc_collect_crc(data->pipe_crc, > &ref_crc[i]); > + > + set_fb(plane, NULL); > + igt_display_commit2(display, COMMIT_ATOMIC); > + } > + > + for (i = 0; i < ARRAY_SIZE(seq); i++) { > + int j = seq[i]; > + > + set_fb(plane, &data->fb[j]); > + igt_display_commit2(display, COMMIT_ATOMIC); > + > + igt_pipe_crc_collect_crc(data->pipe_crc, &crc); > + igt_assert_crc_equal(&ref_crc[j], &crc); > + } > + } > +} > + > +static void run_tests_for_pipe(data_t *data, enum pipe pipe) { > + igt_output_t *output; > + igt_display_t *display = &data->display; > + > + igt_fixture { > + igt_display_require_output_on_pipe(display, pipe); > + igt_require(pipe_supports_format(display, pipe, > DRM_FORMAT_NV12)); > + } > + > + igt_subtest_f("pipe-%s-nv12-rgb-switch", > kmstest_pipe_name(pipe)) > + for_each_valid_output_on_pipe(display, pipe, output) > + nv12_rgb_switch(data, pipe, output); } > + > +igt_main > +{ > + data_t data = {}; > + enum pipe pipe; > + > + igt_skip_on_simulation(); > + > + igt_fixture { > + data.display.drm_fd = > drm_open_driver_master(DRIVER_ANY); > + > + kmstest_set_vt_graphics_mode(); > + igt_display_init(&data.display, data.display.drm_fd); > + igt_require(data.display.is_atomic); > + } > + > + for_each_pipe_static(pipe) > + igt_subtest_group > + run_tests_for_pipe(&data, pipe); > + > + igt_fixture { > + igt_display_fini(&data.display); > + close(data.display.drm_fd); > + } > +} > diff --git a/tests/meson.build b/tests/meson.build index > 521a4c425a68..cfcd476ccd10 100644 > --- a/tests/meson.build > +++ b/tests/meson.build > @@ -170,6 +170,7 @@ test_progs = [ > 'kms_legacy_colorkey', > 'kms_mmap_write_crc', > 'kms_mmio_vs_cs_flip', > + 'kms_nv12', > 'kms_panel_fitting', > 'kms_pipe_b_c_ivb', > 'kms_pipe_crc_basic',
diff --git a/tests/Makefile.sources b/tests/Makefile.sources index 870c9093550b..f536db0fa433 100644 --- a/tests/Makefile.sources +++ b/tests/Makefile.sources @@ -192,6 +192,7 @@ TESTS_progs = \ kms_legacy_colorkey \ kms_mmap_write_crc \ kms_mmio_vs_cs_flip \ + kms_nv12 \ kms_panel_fitting \ kms_pipe_b_c_ivb \ kms_pipe_crc_basic \ diff --git a/tests/kms_nv12.c b/tests/kms_nv12.c new file mode 100644 index 000000000000..384a15afde52 --- /dev/null +++ b/tests/kms_nv12.c @@ -0,0 +1,195 @@ +/* + * 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 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. + * + * Authors: + * Maarten Lankhorst + */ +#include "config.h" + +#include "igt.h" +#include +#include +#include +#include +#include + +typedef struct { + igt_display_t display; + + igt_pipe_crc_t *pipe_crc; + struct igt_fb fb[4]; +} data_t; + +static bool plane_supports_format(igt_plane_t *plane, uint32_t format) +{ + int i; + + if (!igt_fb_supported_format(format)) + return false; + + for (i = 0; i < plane->drm_plane->count_formats; i++) + if (plane->drm_plane->formats[i] == format) + return true; + + return false; +} + +static bool pipe_supports_format(igt_display_t *display, enum pipe pipe, uint32_t format) +{ + igt_plane_t *plane; + + for_each_plane_on_pipe(display, pipe, plane) + if (plane_supports_format(plane, format)) + return true; + + return false; +} + +static void remove_fbs(data_t *data) +{ + int i; + + for (i = 0; i < ARRAY_SIZE(data->fb); i++) + igt_remove_fb(data->display.drm_fd, &data->fb[i]); +} + +static void prepare_crtc(data_t *data, enum pipe pipe, igt_output_t *output) +{ + igt_display_t *display = &data->display; + + remove_fbs(data); + igt_display_reset(display); + igt_output_set_pipe(output, pipe); + igt_display_commit2(display, COMMIT_ATOMIC); + + igt_pipe_crc_free(data->pipe_crc); + data->pipe_crc = igt_pipe_crc_new(display->drm_fd, pipe, INTEL_PIPE_CRC_SOURCE_AUTO); +} + +static void set_fb(igt_plane_t *plane, struct igt_fb *fb) +{ + igt_plane_set_fb(plane, fb); + + if (fb && fb->tiling == LOCAL_I915_FORMAT_MOD_Y_TILED) { + igt_plane_set_rotation(plane, IGT_ROTATION_90); + igt_plane_set_size(plane, fb->height, fb->width); + } else + igt_plane_set_rotation(plane, IGT_ROTATION_0); +} + +static void nv12_rgb_switch(data_t *data, enum pipe pipe, igt_output_t *output) +{ + drmModeModeInfo *mode = igt_output_get_mode(output); + igt_display_t *display = &data->display; + igt_plane_t *plane; + int i; + igt_crc_t ref_crc[4], crc; + + prepare_crtc(data, pipe, output); + + igt_create_pattern_fb(display->drm_fd, mode->hdisplay, mode->vdisplay, + DRM_FORMAT_NV12, LOCAL_I915_FORMAT_MOD_X_TILED, + &data->fb[0]); + + igt_create_pattern_fb(display->drm_fd, mode->vdisplay, mode->hdisplay, + DRM_FORMAT_NV12, LOCAL_I915_FORMAT_MOD_Y_TILED, + &data->fb[1]); + + igt_create_pattern_fb(display->drm_fd, mode->hdisplay, mode->vdisplay, + DRM_FORMAT_XRGB8888, LOCAL_I915_FORMAT_MOD_X_TILED, + &data->fb[2]); + + igt_create_pattern_fb(display->drm_fd, mode->vdisplay, mode->hdisplay, + DRM_FORMAT_XRGB8888, LOCAL_I915_FORMAT_MOD_Y_TILED, + &data->fb[3]); + + for_each_plane_on_pipe(display, pipe, plane) + if (plane->type != DRM_PLANE_TYPE_CURSOR) + set_fb(plane, &data->fb[3]); + + for_each_plane_on_pipe(display, pipe, plane) { + const int seq[] = { + 2, 0, 2, 1, 3, 1, 3 + }; + + if (!plane_supports_format(plane, DRM_FORMAT_NV12)) + continue; + + /* Collect reference crc with toggle in between. */ + for (i = 0; i < ARRAY_SIZE(ref_crc); i++) { + set_fb(plane, &data->fb[i]); + igt_display_commit2(display, COMMIT_ATOMIC); + + igt_pipe_crc_collect_crc(data->pipe_crc, &ref_crc[i]); + + set_fb(plane, NULL); + igt_display_commit2(display, COMMIT_ATOMIC); + } + + for (i = 0; i < ARRAY_SIZE(seq); i++) { + int j = seq[i]; + + set_fb(plane, &data->fb[j]); + igt_display_commit2(display, COMMIT_ATOMIC); + + igt_pipe_crc_collect_crc(data->pipe_crc, &crc); + igt_assert_crc_equal(&ref_crc[j], &crc); + } + } +} + +static void run_tests_for_pipe(data_t *data, enum pipe pipe) +{ + igt_output_t *output; + igt_display_t *display = &data->display; + + igt_fixture { + igt_display_require_output_on_pipe(display, pipe); + igt_require(pipe_supports_format(display, pipe, DRM_FORMAT_NV12)); + } + + igt_subtest_f("pipe-%s-nv12-rgb-switch", kmstest_pipe_name(pipe)) + for_each_valid_output_on_pipe(display, pipe, output) + nv12_rgb_switch(data, pipe, output); +} + +igt_main +{ + data_t data = {}; + enum pipe pipe; + + igt_skip_on_simulation(); + + igt_fixture { + data.display.drm_fd = drm_open_driver_master(DRIVER_ANY); + + kmstest_set_vt_graphics_mode(); + igt_display_init(&data.display, data.display.drm_fd); + igt_require(data.display.is_atomic); + } + + for_each_pipe_static(pipe) + igt_subtest_group + run_tests_for_pipe(&data, pipe); + + igt_fixture { + igt_display_fini(&data.display); + close(data.display.drm_fd); + } +} diff --git a/tests/meson.build b/tests/meson.build index 521a4c425a68..cfcd476ccd10 100644 --- a/tests/meson.build +++ b/tests/meson.build @@ -170,6 +170,7 @@ test_progs = [ 'kms_legacy_colorkey', 'kms_mmap_write_crc', 'kms_mmio_vs_cs_flip', + 'kms_nv12', 'kms_panel_fitting', 'kms_pipe_b_c_ivb', 'kms_pipe_crc_basic',