Re: [igt-dev] [PATCH i-g-t v4 2/4] Moved common function in kms_color and kms_color_chamelium to kms_color_helper.c
From: Petri Latvala <hidden>
Date: 2020-01-24 10:08:23
On Thu, Jan 23, 2020 at 01:30:40PM +0530, Kunal Joshi wrote:
quoted hunk ↗ jump to hunk
kms_color and kms_color_chamelium shared common functions. Moved them to tests/kms_color_helper.c to avoid code duplication. (v4) Made a library kms_color_helper.c which is specific to kms_color and kms_color_chamelium. Signed-off-by: Kunal Joshi <redacted> Signed-off-by: Swati Sharma <redacted> Suggested-by: Uma Shankar <redacted> --- tests/Makefile.am | 3 + tests/Makefile.sources | 13 +- tests/kms_color.c | 383 +--------------------------------------------- tests/kms_color.h | 105 +++++++++++++ tests/kms_color_helper.c | 386 +++++++++++++++++++++++++++++++++++++++++++++++ tests/meson.build | 19 ++- 6 files changed, 525 insertions(+), 384 deletions(-) create mode 100644 tests/kms_color.h create mode 100644 tests/kms_color_helper.cdiff --git a/tests/Makefile.am b/tests/Makefile.am index fc30524..87ffec2 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am@@ -13,11 +13,14 @@ endif if HAVE_CHAMELIUM TESTS_progs += \ kms_chamelium \ + kms_color_chamelium \ $(NULL) endif TESTS_progs += testdisplay +TESTS_progs += kms_color + if BUILD_TESTS test-list.txt: Makefile @echo TESTLIST > $@diff --git a/tests/Makefile.sources b/tests/Makefile.sources index 806eb02..c9b7aea 100644 --- a/tests/Makefile.sources +++ b/tests/Makefile.sources@@ -35,7 +35,6 @@ TESTS_progs = \ kms_big_fb \ kms_busy \ kms_ccs \ - kms_color \ kms_concurrent \ kms_content_protection\ kms_crtc_background_color \@@ -590,6 +589,18 @@ testdisplay_SOURCES = \ testdisplay_hotplug.c \ $(NULL) +kms_color_SOURCES = \ + kms_color.c \ + kms_color.h \ + kms_color_helper.c \ + $(NULL) + +kms_color_chamelium_SOURCES = \ + kms_color_chamelium.c \ + kms_color.h \ + kms_color_helper.c \ + $(NULL) + check_SCRIPTS = igt_command_line.sh \ $(NULL)diff --git a/tests/kms_color.c b/tests/kms_color.c index b4b578a..864f42a 100644 --- a/tests/kms_color.c +++ b/tests/kms_color.c@@ -22,252 +22,10 @@ * */ -#include <math.h> -#include <unistd.h> - -#include "drm.h" -#include "drmtest.h" -#include "igt.h" +#include "kms_color.h" IGT_TEST_DESCRIPTION("Test Color Features at Pipe level"); -/* Internal */ -typedef struct { - double r, g, b; -} color_t; - -typedef struct { - int drm_fd; - uint32_t devid; - igt_display_t display; - igt_pipe_crc_t *pipe_crc; - - uint32_t color_depth; - uint64_t degamma_lut_size; - uint64_t gamma_lut_size; -} data_t; - -typedef struct { - int size; - double coeffs[]; -} gamma_lut_t; - -static void paint_gradient_rectangles(data_t *data, - drmModeModeInfo *mode, - color_t *colors, - struct igt_fb *fb) -{ - cairo_t *cr = igt_get_cairo_ctx(data->drm_fd, fb); - int i, l = mode->hdisplay / 3; - int rows_remaining = mode->hdisplay % 3; - - /* Paint 3 gradient rectangles with red/green/blue between 1.0 and - * 0.5. We want to avoid 0 so each max LUTs only affect their own - * rectangle. - */ - for (i = 0 ; i < 3; i++) { - igt_paint_color_gradient_range(cr, i * l, 0, l, mode->vdisplay, - colors[i].r != 0 ? 0.2 : 0, - colors[i].g != 0 ? 0.2 : 0, - colors[i].b != 0 ? 0.2 : 0, - colors[i].r, - colors[i].g, - colors[i].b); - } - - if (rows_remaining > 0) - igt_paint_color_gradient_range(cr, i * l, 0, rows_remaining, - mode->vdisplay, - colors[i-1].r != 0 ? 0.2 : 0, - colors[i-1].g != 0 ? 0.2 : 0, - colors[i-1].b != 0 ? 0.2 : 0, - colors[i-1].r, - colors[i-1].g, - colors[i-1].b); - - igt_put_cairo_ctx(data->drm_fd, fb, cr); -} - -static void paint_rectangles(data_t *data, - drmModeModeInfo *mode, - color_t *colors, - struct igt_fb *fb) -{ - cairo_t *cr = igt_get_cairo_ctx(data->drm_fd, fb); - int i, l = mode->hdisplay / 3; - int rows_remaining = mode->hdisplay % 3; - - /* Paint 3 solid rectangles. */ - for (i = 0 ; i < 3; i++) { - igt_paint_color(cr, i * l, 0, l, mode->vdisplay, - colors[i].r, colors[i].g, colors[i].b); - } - - if (rows_remaining > 0) - igt_paint_color(cr, i * l, 0, rows_remaining, mode->vdisplay, - colors[i-1].r, colors[i-1].g, colors[i-1].b); - - igt_put_cairo_ctx(data->drm_fd, fb, cr); -} - -static gamma_lut_t *alloc_lut(int lut_size) -{ - gamma_lut_t *gamma; - - igt_assert_lt(0, lut_size); - - gamma = malloc(sizeof(*gamma) + lut_size * sizeof(gamma->coeffs[0])); - igt_assert(gamma); - gamma->size = lut_size; - - return gamma; -} - -static void free_lut(gamma_lut_t *gamma) -{ - if (!gamma) - return; - - free(gamma); -} - -static gamma_lut_t *generate_table(int lut_size, double exp) -{ - gamma_lut_t *gamma = alloc_lut(lut_size); - int i; - - gamma->coeffs[0] = 0.0; - for (i = 1; i < lut_size; i++) - gamma->coeffs[i] = pow(i * 1.0 / (lut_size - 1), exp); - - return gamma; -} - -static gamma_lut_t *generate_table_max(int lut_size) -{ - gamma_lut_t *gamma = alloc_lut(lut_size); - int i; - - gamma->coeffs[0] = 0.0; - for (i = 1; i < lut_size; i++) - gamma->coeffs[i] = 1.0; - - return gamma; -} - -static gamma_lut_t *generate_table_zero(int lut_size) -{ - gamma_lut_t *gamma = alloc_lut(lut_size); - int i; - - for (i = 0; i < lut_size; i++) - gamma->coeffs[i] = 0.0; - - return gamma; -} - -static struct drm_color_lut *coeffs_to_lut(data_t *data, - const gamma_lut_t *gamma, - uint32_t color_depth, - int off) -{ - struct drm_color_lut *lut; - int i, lut_size = gamma->size; - uint32_t max_value = (1 << 16) - 1; - uint32_t mask; - - if (is_i915_device(data->drm_fd)) - mask = ((1 << color_depth) - 1) << 8; - else - mask = max_value; - - lut = malloc(sizeof(struct drm_color_lut) * lut_size); - - if (IS_CHERRYVIEW(data->devid)) - lut_size -= 1; - for (i = 0; i < lut_size; i++) { - uint32_t v = (gamma->coeffs[i] * max_value); - - /* - * Hardware might encode colors on a different number of bits - * than what is in our framebuffer (10 or 12bits for example). - * Mask the lower bits not provided by the framebuffer so we - * can do CRC comparisons. - */ - v &= mask; - - lut[i].red = v; - lut[i].green = v; - lut[i].blue = v; - } - - if (IS_CHERRYVIEW(data->devid)) - lut[lut_size].red = - lut[lut_size].green = - lut[lut_size].blue = lut[lut_size - 1].red; - - return lut; -} - -static void set_degamma(data_t *data, - igt_pipe_t *pipe, - const gamma_lut_t *gamma) -{ - size_t size = sizeof(struct drm_color_lut) * gamma->size; - struct drm_color_lut *lut = coeffs_to_lut(data, gamma, - data->color_depth, 0); - - igt_pipe_obj_replace_prop_blob(pipe, IGT_CRTC_DEGAMMA_LUT, lut, size); - - free(lut); -} - -static void set_gamma(data_t *data, - igt_pipe_t *pipe, - const gamma_lut_t *gamma) -{ - size_t size = sizeof(struct drm_color_lut) * gamma->size; - struct drm_color_lut *lut = coeffs_to_lut(data, gamma, - data->color_depth, 0); - - igt_pipe_obj_replace_prop_blob(pipe, IGT_CRTC_GAMMA_LUT, lut, size); - - free(lut); -} - -static void set_ctm(igt_pipe_t *pipe, const double *coefficients) -{ - struct drm_color_ctm ctm; - int i; - - for (i = 0; i < ARRAY_SIZE(ctm.matrix); i++) { - if (coefficients[i] < 0) { - ctm.matrix[i] = - (int64_t) (-coefficients[i] * ((int64_t) 1L << 32)); - ctm.matrix[i] |= 1ULL << 63; - } else - ctm.matrix[i] = - (int64_t) (coefficients[i] * ((int64_t) 1L << 32)); - } - - igt_pipe_obj_replace_prop_blob(pipe, IGT_CRTC_CTM, &ctm, sizeof(ctm)); -} - -static void disable_prop(igt_pipe_t *pipe, enum igt_atomic_crtc_properties prop) -{ - if (igt_pipe_obj_has_prop(pipe, prop)) - igt_pipe_obj_replace_prop_blob(pipe, prop, NULL, 0); -} - -#define disable_degamma(pipe) disable_prop(pipe, IGT_CRTC_DEGAMMA_LUT) -#define disable_gamma(pipe) disable_prop(pipe, IGT_CRTC_GAMMA_LUT) -#define disable_ctm(pipe) disable_prop(pipe, IGT_CRTC_CTM) - -/* - * Draw 3 gradient rectangles in red, green and blue, with a maxed out - * degamma LUT and verify we have the same CRC as drawing solid color - * rectangles with linear degamma LUT. - */ static void test_pipe_degamma(data_t *data, igt_plane_t *primary) {@@ -532,19 +290,6 @@ static void test_pipe_legacy_gamma(data_t *data, free(blue_lut); } -static drmModePropertyBlobPtr -get_blob(data_t *data, igt_pipe_t *pipe, enum igt_atomic_crtc_properties prop) -{ - uint64_t prop_value; - - prop_value = igt_pipe_obj_get_prop(pipe, prop); - - if (prop_value == 0) - return NULL; - - return drmModeGetPropertyBlob(data->drm_fd, prop_value); -} - /* * Verify that setting the legacy gamma LUT resets the gamma LUT set * through the GAMMA_LUT property.@@ -663,11 +408,6 @@ static void test_pipe_legacy_gamma_reset(data_t *data, free_lut(gamma_zero); } -static bool crc_equal(igt_crc_t *a, igt_crc_t *b) -{ - return memcmp(a->crc, b->crc, sizeof(a->crc[0]) * a->n_words) == 0; -} - /* * Draw 3 rectangles using before colors with the ctm matrix apply and verify * the CRC is equal to using after colors with an identify ctm matrix.@@ -1086,127 +826,6 @@ run_tests_for_pipe(data_t *data, enum pipe p) } } -static int -pipe_set_property_blob_id(igt_pipe_t *pipe, enum igt_atomic_crtc_properties prop, uint32_t blob_id) -{ - int ret; - - igt_pipe_obj_replace_prop_blob(pipe, prop, NULL, 0); - - igt_pipe_obj_set_prop_value(pipe, prop, blob_id); - - ret = igt_display_try_commit2(pipe->display, pipe->display->is_atomic ? COMMIT_ATOMIC : COMMIT_LEGACY); - - igt_pipe_obj_set_prop_value(pipe, prop, 0); - - return ret; -} - -static int -pipe_set_property_blob(igt_pipe_t *pipe, enum igt_atomic_crtc_properties prop, void *ptr, size_t length) -{ - igt_pipe_obj_replace_prop_blob(pipe, prop, ptr, length); - - return igt_display_try_commit2(pipe->display, pipe->display->is_atomic ? COMMIT_ATOMIC : COMMIT_LEGACY); -} - -static void -invalid_gamma_lut_sizes(data_t *data) -{ - igt_display_t *display = &data->display; - igt_pipe_t *pipe = &display->pipes[0]; - size_t gamma_lut_size = data->gamma_lut_size * sizeof(struct drm_color_lut); - struct drm_color_lut *gamma_lut; - - igt_require(igt_pipe_obj_has_prop(pipe, IGT_CRTC_GAMMA_LUT)); - - gamma_lut = malloc(gamma_lut_size * 2); - - igt_display_commit2(display, display->is_atomic ? COMMIT_ATOMIC : COMMIT_LEGACY); - - igt_assert_eq(pipe_set_property_blob(pipe, IGT_CRTC_GAMMA_LUT, - gamma_lut, 1), - -EINVAL); - igt_assert_eq(pipe_set_property_blob(pipe, IGT_CRTC_GAMMA_LUT, - gamma_lut, gamma_lut_size + 1), - -EINVAL); - igt_assert_eq(pipe_set_property_blob(pipe, IGT_CRTC_GAMMA_LUT, - gamma_lut, gamma_lut_size - 1), - -EINVAL); - igt_assert_eq(pipe_set_property_blob(pipe, IGT_CRTC_GAMMA_LUT, - gamma_lut, gamma_lut_size + sizeof(struct drm_color_lut)), - -EINVAL); - igt_assert_eq(pipe_set_property_blob_id(pipe, IGT_CRTC_GAMMA_LUT, pipe->crtc_id), - -EINVAL); - igt_assert_eq(pipe_set_property_blob_id(pipe, IGT_CRTC_GAMMA_LUT, 4096 * 4096), - -EINVAL); - - free(gamma_lut); -} - -static void -invalid_degamma_lut_sizes(data_t *data) -{ - igt_display_t *display = &data->display; - igt_pipe_t *pipe = &display->pipes[0]; - size_t degamma_lut_size = data->degamma_lut_size * sizeof(struct drm_color_lut); - struct drm_color_lut *degamma_lut; - - igt_require(igt_pipe_obj_has_prop(pipe, IGT_CRTC_DEGAMMA_LUT)); - - degamma_lut = malloc(degamma_lut_size * 2); - - igt_display_commit2(display, display->is_atomic ? COMMIT_ATOMIC : COMMIT_LEGACY); - - igt_assert_eq(pipe_set_property_blob(pipe, IGT_CRTC_DEGAMMA_LUT, - degamma_lut, 1), -EINVAL); - igt_assert_eq(pipe_set_property_blob(pipe, IGT_CRTC_DEGAMMA_LUT, - degamma_lut, degamma_lut_size + 1), - -EINVAL); - igt_assert_eq(pipe_set_property_blob(pipe, IGT_CRTC_DEGAMMA_LUT, - degamma_lut, degamma_lut_size - 1), - -EINVAL); - igt_assert_eq(pipe_set_property_blob(pipe, IGT_CRTC_DEGAMMA_LUT, - degamma_lut, degamma_lut_size + sizeof(struct drm_color_lut)), - -EINVAL); - igt_assert_eq(pipe_set_property_blob_id(pipe, IGT_CRTC_DEGAMMA_LUT, pipe->crtc_id), - -EINVAL); - igt_assert_eq(pipe_set_property_blob_id(pipe, IGT_CRTC_DEGAMMA_LUT, 4096 * 4096), - -EINVAL); - - free(degamma_lut); -} - -static void -invalid_ctm_matrix_sizes(data_t *data) -{ - igt_display_t *display = &data->display; - igt_pipe_t *pipe = &display->pipes[0]; - void *ptr; - - igt_require(igt_pipe_obj_has_prop(pipe, IGT_CRTC_CTM)); - - ptr = malloc(sizeof(struct drm_color_ctm) * 4); - - igt_assert_eq(pipe_set_property_blob(pipe, IGT_CRTC_CTM, ptr, 1), - -EINVAL); - igt_assert_eq(pipe_set_property_blob(pipe, IGT_CRTC_CTM, ptr, - sizeof(struct drm_color_ctm) + 1), - -EINVAL); - igt_assert_eq(pipe_set_property_blob(pipe, IGT_CRTC_CTM, ptr, - sizeof(struct drm_color_ctm) - 1), - -EINVAL); - igt_assert_eq(pipe_set_property_blob(pipe, IGT_CRTC_CTM, ptr, - sizeof(struct drm_color_ctm) * 2), - -EINVAL); - igt_assert_eq(pipe_set_property_blob_id(pipe, IGT_CRTC_CTM, pipe->crtc_id), - -EINVAL); - igt_assert_eq(pipe_set_property_blob_id(pipe, IGT_CRTC_CTM, 4096 * 4096), - -EINVAL); - - free(ptr); -} - igt_main { data_t data = {};diff --git a/tests/kms_color.h b/tests/kms_color.h new file mode 100644 index 0000000..cee3859 --- /dev/null +++ b/tests/kms_color.h
Hmm, this header should maybe be called kms_color_helper.h
quoted hunk ↗ jump to hunk
@@ -0,0 +1,105 @@ +/* + * Copyright © 2020 Intel Corporation
It's a new file but the licensed work inside isn't changed. This year should be the same as in old kms_color.c, 2015.
+ * 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. + * + */ + +#ifndef COLOR +#define COLOR
This is bound to collide at something at some point. IGT_KMS_COLOR_H instead. We could use a comment here that explains what this header file is for: Shared code for the two kms_color tests. With those fixed, Reviewed-by: Petri Latvala <redacted> _______________________________________________ igt-dev mailing list igt-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/igt-dev