All of lore.kernel.org
 help / color / mirror / Atom feed
* [igt-dev] [PATCH i-g-t 1/6] tests/kms_color: Nuke local struct definitions
@ 2019-04-02 16:33 Ville Syrjala
  2019-04-02 16:33 ` [igt-dev] [PATCH i-g-t 2/6] tests/kms_color: Wrap LUTs in a gamma_lut_t struct Ville Syrjala
                   ` (7 more replies)
  0 siblings, 8 replies; 16+ messages in thread
From: Ville Syrjala @ 2019-04-02 16:33 UTC (permalink / raw)
  To: igt-dev

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

I think we can assume fresh enough headers by now, so remove
the local _drm_color_ctm and _drm_color_lut structs definitions.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 tests/kms_color.c | 74 +++++++++++++++++++----------------------------
 1 file changed, 29 insertions(+), 45 deletions(-)

diff --git a/tests/kms_color.c b/tests/kms_color.c
index decf3c2a680e..1e5a5d329bea 100644
--- a/tests/kms_color.c
+++ b/tests/kms_color.c
@@ -31,22 +31,6 @@
 
 IGT_TEST_DESCRIPTION("Test Color Features at Pipe level");
 
-/* Data structures for gamma/degamma ramps & ctm matrix. */
-struct _drm_color_ctm {
-	/* Transformation matrix in S31.32 format. */
-	__s64 matrix[9];
-};
-
-struct _drm_color_lut {
-	/*
-	 * Data is U0.16 fixed point format.
-	 */
-	__u16 red;
-	__u16 green;
-	__u16 blue;
-	__u16 reserved;
-};
-
 /* Internal */
 typedef struct {
 	double r, g, b;
@@ -163,13 +147,13 @@ static double *generate_table_zero(uint32_t lut_size)
 	return coeffs;
 }
 
-static struct _drm_color_lut *coeffs_to_lut(data_t *data,
-					    const double *coefficients,
-					    uint32_t lut_size,
-					    uint32_t color_depth,
-					    int off)
+static struct drm_color_lut *coeffs_to_lut(data_t *data,
+					   const double *coefficients,
+					   uint32_t lut_size,
+					   uint32_t color_depth,
+					   int off)
 {
-	struct _drm_color_lut *lut;
+	struct drm_color_lut *lut;
 	uint32_t i;
 	uint32_t max_value = (1 << 16) - 1;
 	uint32_t mask;
@@ -179,7 +163,7 @@ static struct _drm_color_lut *coeffs_to_lut(data_t *data,
 	else
 		mask = max_value;
 
-	lut = malloc(sizeof(struct _drm_color_lut) * lut_size);
+	lut = malloc(sizeof(struct drm_color_lut) * lut_size);
 
 	if (IS_CHERRYVIEW(data->devid))
 		lut_size -= 1;
@@ -211,8 +195,8 @@ static void set_degamma(data_t *data,
 			igt_pipe_t *pipe,
 			const double *coefficients)
 {
-	size_t size = sizeof(struct _drm_color_lut) * data->degamma_lut_size;
-	struct _drm_color_lut *lut = coeffs_to_lut(data,
+	size_t size = sizeof(struct drm_color_lut) * data->degamma_lut_size;
+	struct drm_color_lut *lut = coeffs_to_lut(data,
 						   coefficients,
 						   data->degamma_lut_size,
 						   data->color_depth, 0);
@@ -226,8 +210,8 @@ static void set_gamma(data_t *data,
 		      igt_pipe_t *pipe,
 		      const double *coefficients)
 {
-	size_t size = sizeof(struct _drm_color_lut) * data->gamma_lut_size;
-	struct _drm_color_lut *lut = coeffs_to_lut(data,
+	size_t size = sizeof(struct drm_color_lut) * data->gamma_lut_size;
+	struct drm_color_lut *lut = coeffs_to_lut(data,
 						   coefficients,
 						   data->gamma_lut_size,
 						   data->color_depth, 0);
@@ -239,7 +223,7 @@ static void set_gamma(data_t *data,
 
 static void set_ctm(igt_pipe_t *pipe, const double *coefficients)
 {
-	struct _drm_color_ctm ctm;
+	struct drm_color_ctm ctm;
 	int i;
 
 	for (i = 0; i < ARRAY_SIZE(ctm.matrix); i++) {
@@ -552,7 +536,7 @@ static void test_pipe_legacy_gamma_reset(data_t *data,
 	double *degamma_linear, *gamma_zero;
 	uint32_t i, legacy_lut_size;
 	uint16_t *red_lut, *green_lut, *blue_lut;
-	struct _drm_color_lut *lut;
+	struct drm_color_lut *lut;
 	drmModePropertyBlobPtr blob;
 	igt_output_t *output;
 
@@ -578,20 +562,20 @@ static void test_pipe_legacy_gamma_reset(data_t *data,
 
 		blob = get_blob(data, primary->pipe, IGT_CRTC_DEGAMMA_LUT);
 		igt_assert(blob &&
-			   blob->length == (sizeof(struct _drm_color_lut) *
+			   blob->length == (sizeof(struct drm_color_lut) *
 					    data->degamma_lut_size));
 		drmModeFreePropertyBlob(blob);
 
 		blob = get_blob(data, primary->pipe, IGT_CRTC_CTM);
 		igt_assert(blob &&
-			   blob->length == sizeof(struct _drm_color_ctm));
+			   blob->length == sizeof(struct drm_color_ctm));
 		drmModeFreePropertyBlob(blob);
 
 		blob = get_blob(data, primary->pipe, IGT_CRTC_GAMMA_LUT);
 		igt_assert(blob &&
-			   blob->length == (sizeof(struct _drm_color_lut) *
+			   blob->length == (sizeof(struct drm_color_lut) *
 					    data->gamma_lut_size));
-		lut = (struct _drm_color_lut *) blob->data;
+		lut = (struct drm_color_lut *) blob->data;
 		for (i = 0; i < data->gamma_lut_size; i++)
 			igt_assert(lut[i].red == 0 &&
 				   lut[i].green == 0 &&
@@ -625,9 +609,9 @@ static void test_pipe_legacy_gamma_reset(data_t *data,
 
 		blob = get_blob(data, primary->pipe, IGT_CRTC_GAMMA_LUT);
 		igt_assert(blob &&
-			   blob->length == (sizeof(struct _drm_color_lut) *
+			   blob->length == (sizeof(struct drm_color_lut) *
 					    legacy_lut_size));
-		lut = (struct _drm_color_lut *) blob->data;
+		lut = (struct drm_color_lut *) blob->data;
 		for (i = 0; i < legacy_lut_size; i++)
 			igt_assert(lut[i].red == 0xffff &&
 				   lut[i].green == 0xffff &&
@@ -1079,11 +1063,11 @@ invalid_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);
-	size_t gamma_lut_size = data->gamma_lut_size * sizeof(struct _drm_color_lut);
+	size_t degamma_lut_size = data->degamma_lut_size * sizeof(struct drm_color_lut);
+	size_t gamma_lut_size = data->gamma_lut_size * sizeof(struct drm_color_lut);
 
-	struct _drm_color_lut *degamma_lut = malloc(data->degamma_lut_size * sizeof(struct _drm_color_lut) * 2);
-	struct _drm_color_lut *gamma_lut = malloc(data->gamma_lut_size * sizeof(struct _drm_color_lut) * 2);
+	struct drm_color_lut *degamma_lut = malloc(data->degamma_lut_size * sizeof(struct drm_color_lut) * 2);
+	struct drm_color_lut *gamma_lut = malloc(data->gamma_lut_size * sizeof(struct drm_color_lut) * 2);
 
 	igt_display_commit2(display, display->is_atomic ? COMMIT_ATOMIC : COMMIT_LEGACY);
 
@@ -1097,7 +1081,7 @@ invalid_lut_sizes(data_t *data)
 						     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)),
+						     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);
@@ -1116,7 +1100,7 @@ invalid_lut_sizes(data_t *data)
 						     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)),
+						     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);
@@ -1138,18 +1122,18 @@ invalid_ctm_matrix_sizes(data_t *data)
 	if (!igt_pipe_obj_has_prop(pipe, IGT_CRTC_CTM))
 		return;
 
-	ptr = malloc(sizeof(struct _drm_color_ctm) * 4);
+	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),
+					     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),
+					     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),
+					     sizeof(struct drm_color_ctm) * 2),
 		      -EINVAL);
 	igt_assert_eq(pipe_set_property_blob_id(pipe, IGT_CRTC_CTM, pipe->crtc_id),
 		      -EINVAL);
-- 
2.19.2

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [igt-dev] [PATCH i-g-t 2/6] tests/kms_color: Wrap LUTs in a gamma_lut_t struct
  2019-04-02 16:33 [igt-dev] [PATCH i-g-t 1/6] tests/kms_color: Nuke local struct definitions Ville Syrjala
@ 2019-04-02 16:33 ` Ville Syrjala
  2019-04-03 10:50   ` Daniel Vetter
  2019-04-02 16:33 ` [igt-dev] [PATCH i-g-t 3/6] tests/kms_color: Reuse some already compute values Ville Syrjala
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 16+ messages in thread
From: Ville Syrjala @ 2019-04-02 16:33 UTC (permalink / raw)
  To: igt-dev

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

To make life easier let's wrap the LUTs in a small struct.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 tests/kms_color.c | 119 +++++++++++++++++++++++++---------------------
 1 file changed, 66 insertions(+), 53 deletions(-)

diff --git a/tests/kms_color.c b/tests/kms_color.c
index 1e5a5d329bea..43e59dd42b0b 100644
--- a/tests/kms_color.c
+++ b/tests/kms_color.c
@@ -47,6 +47,10 @@ typedef struct {
 	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,
@@ -106,55 +110,68 @@ static void paint_rectangles(data_t *data,
 	igt_put_cairo_ctx(data->drm_fd, fb, cr);
 }
 
-static double *generate_table(uint32_t lut_size, double exp)
+static gamma_lut_t *alloc_lut(int lut_size)
 {
-	double *coeffs;
-	uint32_t i;
+	gamma_lut_t *gamma;
 
 	igt_assert_lt(0, lut_size);
 
-	coeffs = malloc(sizeof(double) * lut_size);
+	gamma = malloc(sizeof(*gamma) + lut_size * sizeof(gamma->coeffs[0]));
+	gamma->size = lut_size;
 
-	for (i = 0; i < lut_size; i++)
-		coeffs[i] = powf((double) i * 1.0 / (double) (lut_size - 1), exp);
+	return gamma;
+}
+
+static void free_lut(gamma_lut_t *gamma)
+{
+	if (!gamma)
+		return;
 
-	return coeffs;
+	free(gamma);
 }
 
-static double *generate_table_max(uint32_t lut_size)
+static gamma_lut_t *generate_table(int lut_size, double exp)
 {
-	double *coeffs;
-	uint32_t i;
+	gamma_lut_t *gamma = alloc_lut(lut_size);
+	int i;
 
-	igt_assert_lt(0, lut_size);
+	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;
+}
 
-	coeffs = malloc(sizeof(double) * lut_size);
-	coeffs[0] = 0.0;
+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++)
-		coeffs[i] = 1.0;
+		gamma->coeffs[i] = 1.0;
 
-	return coeffs;
+	return gamma;
 }
 
-static double *generate_table_zero(uint32_t lut_size)
+static gamma_lut_t *generate_table_zero(int lut_size)
 {
-	double *coeffs = malloc(sizeof(double) * lut_size);
-	uint32_t i;
+	gamma_lut_t *gamma = alloc_lut(lut_size);
+	int i;
 
 	for (i = 0; i < lut_size; i++)
-		coeffs[i] = 0.0;
+		gamma->coeffs[i] = 0.0;
 
-	return coeffs;
+	return gamma;
 }
 
 static struct drm_color_lut *coeffs_to_lut(data_t *data,
-					   const double *coefficients,
-					   uint32_t lut_size,
+					   const gamma_lut_t *gamma,
 					   uint32_t color_depth,
 					   int off)
 {
 	struct drm_color_lut *lut;
-	uint32_t i;
+	int i, lut_size = gamma->size;
 	uint32_t max_value = (1 << 16) - 1;
 	uint32_t mask;
 
@@ -168,7 +185,7 @@ static struct drm_color_lut *coeffs_to_lut(data_t *data,
 	if (IS_CHERRYVIEW(data->devid))
 		lut_size -= 1;
 	for (i = 0; i < lut_size; i++) {
-		uint32_t v = (coefficients[i] * max_value);
+		uint32_t v = (gamma->coeffs[i] * max_value);
 
 		/*
 		 * Hardware might encode colors on a different number of bits
@@ -193,13 +210,11 @@ static struct drm_color_lut *coeffs_to_lut(data_t *data,
 
 static void set_degamma(data_t *data,
 			igt_pipe_t *pipe,
-			const double *coefficients)
+			const gamma_lut_t *gamma)
 {
-	size_t size = sizeof(struct drm_color_lut) * data->degamma_lut_size;
-	struct drm_color_lut *lut = coeffs_to_lut(data,
-						   coefficients,
-						   data->degamma_lut_size,
-						   data->color_depth, 0);
+	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);
 
@@ -208,15 +223,13 @@ static void set_degamma(data_t *data,
 
 static void set_gamma(data_t *data,
 		      igt_pipe_t *pipe,
-		      const double *coefficients)
+		      const gamma_lut_t *gamma)
 {
-	size_t size = sizeof(struct drm_color_lut) * data->gamma_lut_size;
-	struct drm_color_lut *lut = coeffs_to_lut(data,
-						   coefficients,
-						   data->gamma_lut_size,
-						   data->color_depth, 0);
+	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);
+	igt_pipe_obj_replace_prop_blob(pipe, IGT_CRTC_GAMMA_LUT, lut, size);
 
 	free(lut);
 }
@@ -252,8 +265,8 @@ static void test_pipe_degamma(data_t *data,
 			      igt_plane_t *primary)
 {
 	igt_output_t *output;
-	double *degamma_linear, *degamma_full;
-	double *gamma_linear;
+	gamma_lut_t *degamma_linear, *degamma_full;
+	gamma_lut_t *gamma_linear;
 	color_t red_green_blue[] = {
 		{ 1.0, 0.0, 0.0 },
 		{ 0.0, 1.0, 0.0 },
@@ -323,9 +336,9 @@ static void test_pipe_degamma(data_t *data,
 		igt_output_set_pipe(output, PIPE_NONE);
 	}
 
-	free(degamma_linear);
-	free(degamma_full);
-	free(gamma_linear);
+	free_lut(degamma_linear);
+	free_lut(degamma_full);
+	free_lut(gamma_linear);
 }
 
 /*
@@ -336,7 +349,7 @@ static void test_pipe_gamma(data_t *data,
 			    igt_plane_t *primary)
 {
 	igt_output_t *output;
-	double *gamma_full;
+	gamma_lut_t *gamma_full;
 	color_t red_green_blue[] = {
 		{ 1.0, 0.0, 0.0 },
 		{ 0.0, 1.0, 0.0 },
@@ -402,7 +415,7 @@ static void test_pipe_gamma(data_t *data,
 		igt_output_set_pipe(output, PIPE_NONE);
 	}
 
-	free(gamma_full);
+	free_lut(gamma_full);
 }
 
 /*
@@ -533,7 +546,7 @@ static void test_pipe_legacy_gamma_reset(data_t *data,
 		0.0, 0.0, 1.0
 	};
 	drmModeCrtc *kms_crtc;
-	double *degamma_linear, *gamma_zero;
+	gamma_lut_t *degamma_linear, *gamma_zero;
 	uint32_t i, legacy_lut_size;
 	uint16_t *red_lut, *green_lut, *blue_lut;
 	struct drm_color_lut *lut;
@@ -622,8 +635,8 @@ static void test_pipe_legacy_gamma_reset(data_t *data,
 		igt_output_set_pipe(output, PIPE_NONE);
 	}
 
-	free(degamma_linear);
-	free(gamma_zero);
+	free_lut(degamma_linear);
+	free_lut(gamma_zero);
 }
 
 static bool crc_equal(igt_crc_t *a, igt_crc_t *b)
@@ -646,7 +659,7 @@ static bool test_pipe_ctm(data_t *data,
 		0.0, 1.0, 0.0,
 		0.0, 0.0, 1.0
 	};
-	double *degamma_linear, *gamma_linear;
+	gamma_lut_t *degamma_linear, *gamma_linear;
 	igt_output_t *output;
 	bool ret = true;
 
@@ -709,8 +722,8 @@ static bool test_pipe_ctm(data_t *data,
 		igt_output_set_pipe(output, PIPE_NONE);
 	}
 
-	free(degamma_linear);
-	free(gamma_linear);
+	free_lut(degamma_linear);
+	free_lut(gamma_linear);
 
 	return ret;
 }
@@ -745,7 +758,7 @@ static void test_pipe_limited_range_ctm(data_t *data,
 	double ctm[] = { 1.0, 0.0, 0.0,
 			0.0, 1.0, 0.0,
 			0.0, 0.0, 1.0 };
-	double *degamma_linear, *gamma_linear;
+	gamma_lut_t *degamma_linear, *gamma_linear;
 	igt_output_t *output;
 	bool has_broadcast_rgb_output = false;
 
@@ -814,8 +827,8 @@ static void test_pipe_limited_range_ctm(data_t *data,
 		igt_assert_crc_equal(&crc_full, &crc_limited);
 	}
 
-	free(gamma_linear);
-	free(degamma_linear);
+	free_lut(gamma_linear);
+	free_lut(degamma_linear);
 
 	igt_require(has_broadcast_rgb_output);
 }
-- 
2.19.2

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [igt-dev] [PATCH i-g-t 3/6] tests/kms_color: Reuse some already compute values
  2019-04-02 16:33 [igt-dev] [PATCH i-g-t 1/6] tests/kms_color: Nuke local struct definitions Ville Syrjala
  2019-04-02 16:33 ` [igt-dev] [PATCH i-g-t 2/6] tests/kms_color: Wrap LUTs in a gamma_lut_t struct Ville Syrjala
@ 2019-04-02 16:33 ` Ville Syrjala
  2019-04-03 10:51   ` Daniel Vetter
  2019-04-02 16:33 ` [igt-dev] [PATCH i-g-t 4/6] tests/kms_color: Allow most subtests to run with a partial color pipeline Ville Syrjala
                   ` (5 subsequent siblings)
  7 siblings, 1 reply; 16+ messages in thread
From: Ville Syrjala @ 2019-04-02 16:33 UTC (permalink / raw)
  To: igt-dev

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

We already compute the lut_size*entry_size so let's reuse those
when allocating the LUTs.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 tests/kms_color.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/kms_color.c b/tests/kms_color.c
index 43e59dd42b0b..c65bb88cd7dc 100644
--- a/tests/kms_color.c
+++ b/tests/kms_color.c
@@ -1079,8 +1079,8 @@ invalid_lut_sizes(data_t *data)
 	size_t degamma_lut_size = data->degamma_lut_size * sizeof(struct drm_color_lut);
 	size_t gamma_lut_size = data->gamma_lut_size * sizeof(struct drm_color_lut);
 
-	struct drm_color_lut *degamma_lut = malloc(data->degamma_lut_size * sizeof(struct drm_color_lut) * 2);
-	struct drm_color_lut *gamma_lut = malloc(data->gamma_lut_size * sizeof(struct drm_color_lut) * 2);
+	struct drm_color_lut *degamma_lut = malloc(degamma_lut_size * 2);
+	struct drm_color_lut *gamma_lut = malloc(gamma_lut_size * 2);
 
 	igt_display_commit2(display, display->is_atomic ? COMMIT_ATOMIC : COMMIT_LEGACY);
 
-- 
2.19.2

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [igt-dev] [PATCH i-g-t 4/6] tests/kms_color: Allow most subtests to run with a partial color pipeline
  2019-04-02 16:33 [igt-dev] [PATCH i-g-t 1/6] tests/kms_color: Nuke local struct definitions Ville Syrjala
  2019-04-02 16:33 ` [igt-dev] [PATCH i-g-t 2/6] tests/kms_color: Wrap LUTs in a gamma_lut_t struct Ville Syrjala
  2019-04-02 16:33 ` [igt-dev] [PATCH i-g-t 3/6] tests/kms_color: Reuse some already compute values Ville Syrjala
@ 2019-04-02 16:33 ` Ville Syrjala
  2019-04-03 13:50   ` Daniel Vetter
  2019-04-02 16:33 ` [igt-dev] [PATCH i-g-t 5/6] tests/kms_color: Split invalid_lut_sizes() into gamma vs. degamma versions Ville Syrjala
                   ` (4 subsequent siblings)
  7 siblings, 1 reply; 16+ messages in thread
From: Ville Syrjala @ 2019-04-02 16:33 UTC (permalink / raw)
  To: igt-dev

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Requiring a full color pipeline when we're just testing eg. the gamma
LUT is silly. Make the requirements more sensible.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 tests/kms_color.c | 47 +++++++++++++++++++++++++++++++++--------------
 1 file changed, 33 insertions(+), 14 deletions(-)

diff --git a/tests/kms_color.c b/tests/kms_color.c
index c65bb88cd7dc..554356a04f75 100644
--- a/tests/kms_color.c
+++ b/tests/kms_color.c
@@ -252,9 +252,15 @@ static void set_ctm(igt_pipe_t *pipe, const double *coefficients)
 	igt_pipe_obj_replace_prop_blob(pipe, IGT_CRTC_CTM, &ctm, sizeof(ctm));
 }
 
-#define disable_degamma(pipe) igt_pipe_obj_replace_prop_blob(pipe, IGT_CRTC_DEGAMMA_LUT, NULL, 0)
-#define disable_gamma(pipe) igt_pipe_obj_replace_prop_blob(pipe, IGT_CRTC_GAMMA_LUT, NULL, 0)
-#define disable_ctm(pipe) igt_pipe_obj_replace_prop_blob(pipe, IGT_CRTC_CTM, NULL, 0)
+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
@@ -273,6 +279,9 @@ static void test_pipe_degamma(data_t *data,
 		{ 0.0, 0.0, 1.0 }
 	};
 
+	igt_require(igt_pipe_obj_has_prop(primary->pipe, IGT_CRTC_DEGAMMA_LUT));
+	igt_require(igt_pipe_obj_has_prop(primary->pipe, IGT_CRTC_GAMMA_LUT));
+
 	degamma_linear = generate_table(data->degamma_lut_size, 1.0);
 	degamma_full = generate_table_max(data->degamma_lut_size);
 
@@ -356,6 +365,8 @@ static void test_pipe_gamma(data_t *data,
 		{ 0.0, 0.0, 1.0 }
 	};
 
+	igt_require(igt_pipe_obj_has_prop(primary->pipe, IGT_CRTC_GAMMA_LUT));
+
 	gamma_full = generate_table_max(data->gamma_lut_size);
 
 	for_each_valid_output_on_pipe(&data->display, primary->pipe->pipe, output) {
@@ -553,6 +564,11 @@ static void test_pipe_legacy_gamma_reset(data_t *data,
 	drmModePropertyBlobPtr blob;
 	igt_output_t *output;
 
+	/* FIXME accept any one of these */
+	igt_require(igt_pipe_obj_has_prop(primary->pipe, IGT_CRTC_DEGAMMA_LUT));
+	igt_require(igt_pipe_obj_has_prop(primary->pipe, IGT_CRTC_GAMMA_LUT));
+	igt_require(igt_pipe_obj_has_prop(primary->pipe, IGT_CRTC_CTM));
+
 	degamma_linear = generate_table(data->degamma_lut_size, 1.0);
 	gamma_zero = generate_table_zero(data->gamma_lut_size);
 
@@ -663,6 +679,8 @@ static bool test_pipe_ctm(data_t *data,
 	igt_output_t *output;
 	bool ret = true;
 
+	igt_require(igt_pipe_obj_has_prop(primary->pipe, IGT_CRTC_CTM));
+
 	degamma_linear = generate_table(data->degamma_lut_size, 1.0);
 	gamma_linear = generate_table(data->gamma_lut_size, 1.0);
 
@@ -861,19 +879,20 @@ run_tests_for_pipe(data_t *data, enum pipe p)
 						  primary->pipe->pipe,
 						  INTEL_PIPE_CRC_SOURCE_AUTO);
 
-		igt_require(igt_pipe_obj_has_prop(&data->display.pipes[p], IGT_CRTC_DEGAMMA_LUT_SIZE));
-		igt_require(igt_pipe_obj_has_prop(&data->display.pipes[p], IGT_CRTC_GAMMA_LUT_SIZE));
-
-		data->degamma_lut_size =
-			igt_pipe_obj_get_prop(&data->display.pipes[p],
-					      IGT_CRTC_DEGAMMA_LUT_SIZE);
+		if (igt_pipe_obj_has_prop(&data->display.pipes[p], IGT_CRTC_DEGAMMA_LUT_SIZE)) {
+			data->degamma_lut_size =
+				igt_pipe_obj_get_prop(&data->display.pipes[p],
+						      IGT_CRTC_DEGAMMA_LUT_SIZE);
+			igt_assert_lt(0, data->degamma_lut_size);
+		}
 
-		data->gamma_lut_size =
-			igt_pipe_obj_get_prop(&data->display.pipes[p],
-					      IGT_CRTC_GAMMA_LUT_SIZE);
+		if (igt_pipe_obj_has_prop(&data->display.pipes[p], IGT_CRTC_GAMMA_LUT_SIZE)) {
+			data->gamma_lut_size =
+				igt_pipe_obj_get_prop(&data->display.pipes[p],
+						      IGT_CRTC_GAMMA_LUT_SIZE);
+			igt_assert_lt(0, data->gamma_lut_size);
+		}
 
-		igt_assert_lt(0, data->degamma_lut_size);
-		igt_assert_lt(0, data->gamma_lut_size);
 		igt_display_require_output_on_pipe(&data->display, p);
 	}
 
-- 
2.19.2

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [igt-dev] [PATCH i-g-t 5/6] tests/kms_color: Split invalid_lut_sizes() into gamma vs. degamma versions
  2019-04-02 16:33 [igt-dev] [PATCH i-g-t 1/6] tests/kms_color: Nuke local struct definitions Ville Syrjala
                   ` (2 preceding siblings ...)
  2019-04-02 16:33 ` [igt-dev] [PATCH i-g-t 4/6] tests/kms_color: Allow most subtests to run with a partial color pipeline Ville Syrjala
@ 2019-04-02 16:33 ` Ville Syrjala
  2019-04-03 13:52   ` Daniel Vetter
  2019-04-02 16:33 ` [igt-dev] [PATCH i-g-t 6/6] tests/kms_color: Make legacy-gamma-reset work with a partial color pipeline Ville Syrjala
                   ` (3 subsequent siblings)
  7 siblings, 1 reply; 16+ messages in thread
From: Ville Syrjala @ 2019-04-02 16:33 UTC (permalink / raw)
  To: igt-dev

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Split the invalid-lut-sizes test into separate gamma and degamma tests.
This way we can report SKIP for the thing we don't have. Also make the
CTM invalid sizes test report a skip too.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 tests/kms_color.c | 103 ++++++++++++++++++++++++++--------------------
 1 file changed, 59 insertions(+), 44 deletions(-)

diff --git a/tests/kms_color.c b/tests/kms_color.c
index 554356a04f75..fd4c9a6dd812 100644
--- a/tests/kms_color.c
+++ b/tests/kms_color.c
@@ -1091,57 +1091,70 @@ pipe_set_property_blob(igt_pipe_t *pipe, enum igt_atomic_crtc_properties prop, v
 }
 
 static void
-invalid_lut_sizes(data_t *data)
+invalid_gamma_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);
 	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));
 
-	struct drm_color_lut *degamma_lut = malloc(degamma_lut_size * 2);
-	struct drm_color_lut *gamma_lut = malloc(gamma_lut_size * 2);
+	gamma_lut = malloc(gamma_lut_size * 2);
 
 	igt_display_commit2(display, display->is_atomic ? COMMIT_ATOMIC : COMMIT_LEGACY);
 
-	if (igt_pipe_obj_has_prop(pipe, IGT_CRTC_DEGAMMA_LUT)) {
-		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);
-	}
+	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);
 
-	if (igt_pipe_obj_has_prop(pipe, IGT_CRTC_GAMMA_LUT)) {
-		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);
-	free(gamma_lut);
 }
 
 static void
@@ -1151,8 +1164,7 @@ invalid_ctm_matrix_sizes(data_t *data)
 	igt_pipe_t *pipe = &display->pipes[0];
 	void *ptr;
 
-	if (!igt_pipe_obj_has_prop(pipe, IGT_CRTC_CTM))
-		return;
+	igt_require(igt_pipe_obj_has_prop(pipe, IGT_CRTC_CTM));
 
 	ptr = malloc(sizeof(struct drm_color_ctm) * 4);
 
@@ -1195,8 +1207,11 @@ igt_main
 		igt_subtest_group
 			run_tests_for_pipe(&data, pipe);
 
-	igt_subtest_f("pipe-invalid-lut-sizes")
-		invalid_lut_sizes(&data);
+	igt_subtest_f("pipe-invalid-gamma-lut-sizes")
+		invalid_gamma_lut_sizes(&data);
+
+	igt_subtest_f("pipe-invalid-degamma-lut-sizes")
+		invalid_degamma_lut_sizes(&data);
 
 	igt_subtest_f("pipe-invalid-ctm-matrix-sizes")
 		invalid_ctm_matrix_sizes(&data);
-- 
2.19.2

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [igt-dev] [PATCH i-g-t 6/6] tests/kms_color: Make legacy-gamma-reset work with a partial color pipeline
  2019-04-02 16:33 [igt-dev] [PATCH i-g-t 1/6] tests/kms_color: Nuke local struct definitions Ville Syrjala
                   ` (3 preceding siblings ...)
  2019-04-02 16:33 ` [igt-dev] [PATCH i-g-t 5/6] tests/kms_color: Split invalid_lut_sizes() into gamma vs. degamma versions Ville Syrjala
@ 2019-04-02 16:33 ` Ville Syrjala
  2019-04-03 13:53   ` Daniel Vetter
  2019-04-02 17:37 ` [igt-dev] ✓ Fi.CI.BAT: success for series starting with [i-g-t,1/6] tests/kms_color: Nuke local struct definitions Patchwork
                   ` (2 subsequent siblings)
  7 siblings, 1 reply; 16+ messages in thread
From: Ville Syrjala @ 2019-04-02 16:33 UTC (permalink / raw)
  To: igt-dev

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

No reason why we shouldn't be able to execute the legacy-gamma-reset
test with a partial color pipeline.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 tests/kms_color.c | 45 ++++++++++++++++++++++++++-------------------
 1 file changed, 26 insertions(+), 19 deletions(-)

diff --git a/tests/kms_color.c b/tests/kms_color.c
index fd4c9a6dd812..43126c79079d 100644
--- a/tests/kms_color.c
+++ b/tests/kms_color.c
@@ -557,19 +557,17 @@ static void test_pipe_legacy_gamma_reset(data_t *data,
 		0.0, 0.0, 1.0
 	};
 	drmModeCrtc *kms_crtc;
-	gamma_lut_t *degamma_linear, *gamma_zero;
+	gamma_lut_t *degamma_linear = NULL, *gamma_zero;
 	uint32_t i, legacy_lut_size;
 	uint16_t *red_lut, *green_lut, *blue_lut;
 	struct drm_color_lut *lut;
 	drmModePropertyBlobPtr blob;
 	igt_output_t *output;
 
-	/* FIXME accept any one of these */
-	igt_require(igt_pipe_obj_has_prop(primary->pipe, IGT_CRTC_DEGAMMA_LUT));
 	igt_require(igt_pipe_obj_has_prop(primary->pipe, IGT_CRTC_GAMMA_LUT));
-	igt_require(igt_pipe_obj_has_prop(primary->pipe, IGT_CRTC_CTM));
 
-	degamma_linear = generate_table(data->degamma_lut_size, 1.0);
+	if (igt_pipe_obj_has_prop(primary->pipe, IGT_CRTC_DEGAMMA_LUT))
+		degamma_linear = generate_table(data->degamma_lut_size, 1.0);
 	gamma_zero = generate_table_zero(data->gamma_lut_size);
 
 	for_each_valid_output_on_pipe(&data->display, primary->pipe->pipe, output) {
@@ -584,21 +582,27 @@ static void test_pipe_legacy_gamma_reset(data_t *data,
 		/* Set a degama & gamma LUT and a CTM using the
 		 * properties and verify the content of the
 		 * properties. */
-		set_degamma(data, primary->pipe, degamma_linear);
-		set_ctm(primary->pipe, ctm_identity);
+		if (igt_pipe_obj_has_prop(primary->pipe, IGT_CRTC_DEGAMMA_LUT))
+			set_degamma(data, primary->pipe, degamma_linear);
+		if (igt_pipe_obj_has_prop(primary->pipe, IGT_CRTC_CTM))
+			set_ctm(primary->pipe, ctm_identity);
 		set_gamma(data, primary->pipe, gamma_zero);
 		igt_display_commit(&data->display);
 
-		blob = get_blob(data, primary->pipe, IGT_CRTC_DEGAMMA_LUT);
-		igt_assert(blob &&
-			   blob->length == (sizeof(struct drm_color_lut) *
-					    data->degamma_lut_size));
-		drmModeFreePropertyBlob(blob);
+		if (igt_pipe_obj_has_prop(primary->pipe, IGT_CRTC_DEGAMMA_LUT)) {
+			blob = get_blob(data, primary->pipe, IGT_CRTC_DEGAMMA_LUT);
+			igt_assert(blob &&
+				   blob->length == (sizeof(struct drm_color_lut) *
+						    data->degamma_lut_size));
+			drmModeFreePropertyBlob(blob);
+		}
 
-		blob = get_blob(data, primary->pipe, IGT_CRTC_CTM);
-		igt_assert(blob &&
-			   blob->length == sizeof(struct drm_color_ctm));
-		drmModeFreePropertyBlob(blob);
+		if (igt_pipe_obj_has_prop(primary->pipe, IGT_CRTC_CTM)) {
+			blob = get_blob(data, primary->pipe, IGT_CRTC_CTM);
+			igt_assert(blob &&
+				   blob->length == sizeof(struct drm_color_ctm));
+			drmModeFreePropertyBlob(blob);
+		}
 
 		blob = get_blob(data, primary->pipe, IGT_CRTC_GAMMA_LUT);
 		igt_assert(blob &&
@@ -632,9 +636,12 @@ static void test_pipe_legacy_gamma_reset(data_t *data,
 			      0);
 		igt_display_commit(&data->display);
 
-		igt_assert(get_blob(data, primary->pipe,
-				    IGT_CRTC_DEGAMMA_LUT) == NULL);
-		igt_assert(get_blob(data, primary->pipe, IGT_CRTC_CTM) == NULL);
+		if (igt_pipe_obj_has_prop(primary->pipe, IGT_CRTC_DEGAMMA_LUT))
+			igt_assert(get_blob(data, primary->pipe,
+					    IGT_CRTC_DEGAMMA_LUT) == NULL);
+
+		if (igt_pipe_obj_has_prop(primary->pipe, IGT_CRTC_CTM))
+			igt_assert(get_blob(data, primary->pipe, IGT_CRTC_CTM) == NULL);
 
 		blob = get_blob(data, primary->pipe, IGT_CRTC_GAMMA_LUT);
 		igt_assert(blob &&
-- 
2.19.2

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [igt-dev] ✓ Fi.CI.BAT: success for series starting with [i-g-t,1/6] tests/kms_color: Nuke local struct definitions
  2019-04-02 16:33 [igt-dev] [PATCH i-g-t 1/6] tests/kms_color: Nuke local struct definitions Ville Syrjala
                   ` (4 preceding siblings ...)
  2019-04-02 16:33 ` [igt-dev] [PATCH i-g-t 6/6] tests/kms_color: Make legacy-gamma-reset work with a partial color pipeline Ville Syrjala
@ 2019-04-02 17:37 ` Patchwork
  2019-04-03  6:33 ` [igt-dev] ✓ Fi.CI.IGT: " Patchwork
  2019-04-03  8:40 ` [igt-dev] [PATCH i-g-t 1/6] " Daniel Vetter
  7 siblings, 0 replies; 16+ messages in thread
From: Patchwork @ 2019-04-02 17:37 UTC (permalink / raw)
  To: Ville Syrjala; +Cc: igt-dev

== Series Details ==

Series: series starting with [i-g-t,1/6] tests/kms_color: Nuke local struct definitions
URL   : https://patchwork.freedesktop.org/series/58879/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5856 -> IGTPW_2763
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/58879/revisions/1/mbox/

Known issues
------------

  Here are the changes found in IGTPW_2763 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@amdgpu/amd_cs_nop@fork-gfx0:
    - fi-icl-u2:          NOTRUN -> SKIP [fdo#109315] +17

  * igt@gem_exec_basic@basic-bsd2:
    - fi-kbl-7500u:       NOTRUN -> SKIP [fdo#109271] +9

  * igt@gem_exec_basic@readonly-bsd1:
    - fi-icl-u2:          NOTRUN -> SKIP [fdo#109276] +7

  * igt@gem_exec_parse@basic-allowed:
    - fi-icl-u2:          NOTRUN -> SKIP [fdo#109289] +1

  * igt@gem_exec_suspend@basic-s4-devices:
    - fi-icl-u2:          NOTRUN -> DMESG-WARN [fdo#109638]

  * igt@i915_pm_rpm@basic-pci-d3-state:
    - fi-hsw-4770:        PASS -> SKIP [fdo#109271] +3

  * igt@i915_selftest@live_contexts:
    - fi-icl-u2:          NOTRUN -> DMESG-FAIL [fdo#108569]

  * igt@i915_selftest@live_uncore:
    - fi-ivb-3770:        PASS -> DMESG-FAIL [fdo#110210]

  * igt@kms_chamelium@dp-crc-fast:
    - fi-kbl-7500u:       NOTRUN -> DMESG-WARN [fdo#103841]

  * igt@kms_chamelium@dp-edid-read:
    - fi-icl-u2:          NOTRUN -> SKIP [fdo#109316] +2

  * igt@kms_chamelium@vga-hpd-fast:
    - fi-icl-u2:          NOTRUN -> SKIP [fdo#109309] +1

  * igt@kms_force_connector_basic@prune-stale-modes:
    - fi-icl-u2:          NOTRUN -> SKIP [fdo#109285] +3

  * igt@kms_frontbuffer_tracking@basic:
    - fi-icl-u2:          NOTRUN -> FAIL [fdo#103167]

  * igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a-frame-sequence:
    - fi-byt-clapper:     PASS -> FAIL [fdo#103191] / [fdo#107362]

  * igt@kms_psr@primary_mmap_gtt:
    - fi-byt-clapper:     NOTRUN -> SKIP [fdo#109271] +23

  * igt@kms_psr@primary_page_flip:
    - fi-apl-guc:         NOTRUN -> SKIP [fdo#109271] +50

  * igt@runner@aborted:
    - fi-kbl-7500u:       NOTRUN -> FAIL [fdo#103841]

  
#### Possible fixes ####

  * igt@i915_selftest@live_contexts:
    - fi-bdw-gvtdvm:      DMESG-FAIL [fdo#110235 ] -> PASS
    - fi-skl-gvtdvm:      DMESG-FAIL [fdo#110235 ] -> PASS

  * igt@kms_pipe_crc_basic@nonblocking-crc-pipe-b:
    - fi-byt-clapper:     FAIL [fdo#107362] -> PASS

  * igt@kms_pipe_crc_basic@read-crc-pipe-b-frame-sequence:
    - fi-byt-clapper:     FAIL [fdo#103191] / [fdo#107362] -> PASS

  * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
    - fi-byt-clapper:     INCOMPLETE [fdo#102657] -> PASS

  
  [fdo#102657]: https://bugs.freedesktop.org/show_bug.cgi?id=102657
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191
  [fdo#103841]: https://bugs.freedesktop.org/show_bug.cgi?id=103841
  [fdo#107362]: https://bugs.freedesktop.org/show_bug.cgi?id=107362
  [fdo#108569]: https://bugs.freedesktop.org/show_bug.cgi?id=108569
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109276]: https://bugs.freedesktop.org/show_bug.cgi?id=109276
  [fdo#109285]: https://bugs.freedesktop.org/show_bug.cgi?id=109285
  [fdo#109289]: https://bugs.freedesktop.org/show_bug.cgi?id=109289
  [fdo#109309]: https://bugs.freedesktop.org/show_bug.cgi?id=109309
  [fdo#109315]: https://bugs.freedesktop.org/show_bug.cgi?id=109315
  [fdo#109316]: https://bugs.freedesktop.org/show_bug.cgi?id=109316
  [fdo#109638]: https://bugs.freedesktop.org/show_bug.cgi?id=109638
  [fdo#110210]: https://bugs.freedesktop.org/show_bug.cgi?id=110210
  [fdo#110235 ]: https://bugs.freedesktop.org/show_bug.cgi?id=110235 


Participating hosts (43 -> 40)
------------------------------

  Additional (3): fi-icl-u2 fi-apl-guc fi-kbl-7500u 
  Missing    (6): fi-ilk-m540 fi-byt-squawks fi-bwr-2160 fi-skl-6260u fi-icl-u3 fi-bdw-samus 


Build changes
-------------

    * IGT: IGT_4922 -> IGTPW_2763

  CI_DRM_5856: 55074bd825098a71779cf65a69786547f0eccbe9 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_2763: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2763/
  IGT_4922: e941e4a29438c7130554492e4daf52afbc99ffdf @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools



== Testlist changes ==

+igt@kms_color@pipe-invalid-degamma-lut-sizes
+igt@kms_color@pipe-invalid-gamma-lut-sizes
-igt@kms_color@pipe-invalid-lut-sizes

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2763/
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [igt-dev] ✓ Fi.CI.IGT: success for series starting with [i-g-t,1/6] tests/kms_color: Nuke local struct definitions
  2019-04-02 16:33 [igt-dev] [PATCH i-g-t 1/6] tests/kms_color: Nuke local struct definitions Ville Syrjala
                   ` (5 preceding siblings ...)
  2019-04-02 17:37 ` [igt-dev] ✓ Fi.CI.BAT: success for series starting with [i-g-t,1/6] tests/kms_color: Nuke local struct definitions Patchwork
@ 2019-04-03  6:33 ` Patchwork
  2019-04-03  8:40 ` [igt-dev] [PATCH i-g-t 1/6] " Daniel Vetter
  7 siblings, 0 replies; 16+ messages in thread
From: Patchwork @ 2019-04-03  6:33 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: igt-dev

== Series Details ==

Series: series starting with [i-g-t,1/6] tests/kms_color: Nuke local struct definitions
URL   : https://patchwork.freedesktop.org/series/58879/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5856_full -> IGTPW_2763_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/58879/revisions/1/mbox/

New tests
---------

  New tests have been introduced between CI_DRM_5856_full and IGTPW_2763_full:

### New IGT tests (2) ###

  * igt@kms_color@pipe-invalid-degamma-lut-sizes:
    - Statuses : 3 pass(s) 2 skip(s)
    - Exec time: [0.0, 0.00] s

  * igt@kms_color@pipe-invalid-gamma-lut-sizes:
    - Statuses : 3 pass(s) 2 skip(s)
    - Exec time: [0.0, 0.00] s

  

Known issues
------------

  Here are the changes found in IGTPW_2763_full that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_exec_store@cachelines-bsd2:
    - shard-glk:          NOTRUN -> SKIP [fdo#109271] +18

  * igt@kms_atomic_transition@1x-modeset-transitions-nonblocking:
    - shard-apl:          PASS -> FAIL [fdo#109660]
    - shard-kbl:          PASS -> FAIL [fdo#109660]

  * igt@kms_atomic_transition@3x-modeset-transitions:
    - shard-hsw:          NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +3

  * igt@kms_available_modes_crc@available_mode_test_crc:
    - shard-snb:          NOTRUN -> FAIL [fdo#106641]

  * igt@kms_busy@basic-modeset-e:
    - shard-kbl:          NOTRUN -> SKIP [fdo#109271] / [fdo#109278]

  * igt@kms_busy@extended-modeset-hang-newfb-render-a:
    - shard-hsw:          PASS -> DMESG-WARN [fdo#110222] +2

  * igt@kms_busy@extended-modeset-hang-newfb-with-reset-render-c:
    - shard-kbl:          PASS -> DMESG-WARN [fdo#110222]

  * igt@kms_chamelium@common-hpd-after-suspend:
    - shard-hsw:          NOTRUN -> SKIP [fdo#109271] +24

  * igt@kms_color@pipe-invalid-ctm-matrix-sizes:
    - shard-snb:          PASS -> SKIP [fdo#109271]

  * igt@kms_flip@2x-wf_vblank-ts-check:
    - shard-snb:          NOTRUN -> SKIP [fdo#109271] +71

  * igt@kms_flip@dpms-vs-vblank-race:
    - shard-glk:          PASS -> FAIL [fdo#103060]

  * igt@kms_flip@flip-vs-expired-vblank:
    - shard-apl:          PASS -> FAIL [fdo#102887] / [fdo#105363]

  * igt@kms_frontbuffer_tracking@fbcpsr-2p-primscrn-cur-indfb-draw-mmap-wc:
    - shard-kbl:          NOTRUN -> SKIP [fdo#109271] +24

  * igt@kms_lease@atomic_implicit_crtc:
    - shard-kbl:          NOTRUN -> FAIL [fdo#110279]

  * igt@kms_plane_alpha_blend@pipe-a-alpha-7efc:
    - shard-kbl:          NOTRUN -> FAIL [fdo#108145] / [fdo#108590]

  * igt@kms_plane_scaling@pipe-a-scaler-with-clipping-clamping:
    - shard-glk:          PASS -> SKIP [fdo#109271] / [fdo#109278] +1

  * igt@kms_rotation_crc@multiplane-rotation-cropping-top:
    - shard-kbl:          PASS -> FAIL [fdo#109016]

  * igt@kms_setmode@basic:
    - shard-apl:          PASS -> FAIL [fdo#99912]
    - shard-kbl:          PASS -> FAIL [fdo#99912]

  * igt@kms_universal_plane@disable-primary-vs-flip-pipe-d:
    - shard-glk:          NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +1

  * igt@kms_vblank@pipe-a-ts-continuation-dpms-rpm:
    - shard-snb:          NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +9

  * igt@kms_vblank@pipe-c-ts-continuation-modeset-rpm:
    - shard-kbl:          PASS -> FAIL [fdo#104894] +1
    - shard-apl:          PASS -> FAIL [fdo#104894] +1

  
#### Possible fixes ####

  * igt@gem_create@create-clear:
    - shard-snb:          INCOMPLETE [fdo#105411] -> PASS

  * igt@gem_tiled_swapping@non-threaded:
    - shard-hsw:          FAIL [fdo#108686] -> PASS

  * igt@kms_busy@extended-modeset-hang-newfb-with-reset-render-a:
    - shard-snb:          DMESG-WARN [fdo#110222] -> PASS
    - shard-hsw:          DMESG-WARN [fdo#110222] -> PASS
    - shard-kbl:          DMESG-WARN [fdo#110222] -> PASS

  * igt@kms_color@pipe-a-legacy-gamma:
    - shard-snb:          SKIP [fdo#109271] -> PASS

  * igt@kms_color@pipe-b-legacy-gamma:
    - shard-hsw:          SKIP [fdo#109271] -> PASS +1

  * igt@kms_cursor_crc@cursor-256x256-random:
    - shard-glk:          FAIL [fdo#103232] -> PASS

  * igt@kms_cursor_legacy@2x-long-nonblocking-modeset-vs-cursor-atomic:
    - shard-glk:          FAIL [fdo#106509] / [fdo#107409] -> PASS

  * igt@kms_vblank@pipe-a-ts-continuation-dpms-suspend:
    - shard-kbl:          INCOMPLETE [fdo#103665] -> PASS

  * igt@kms_vblank@pipe-c-ts-continuation-dpms-suspend:
    - shard-apl:          FAIL [fdo#104894] -> PASS

  
#### Warnings ####

  * igt@kms_frontbuffer_tracking@psr-2p-primscrn-pri-shrfb-draw-render:
    - shard-hsw:          INCOMPLETE [fdo#103540] -> SKIP [fdo#109271]

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#102887]: https://bugs.freedesktop.org/show_bug.cgi?id=102887
  [fdo#103060]: https://bugs.freedesktop.org/show_bug.cgi?id=103060
  [fdo#103232]: https://bugs.freedesktop.org/show_bug.cgi?id=103232
  [fdo#103540]: https://bugs.freedesktop.org/show_bug.cgi?id=103540
  [fdo#103665]: https://bugs.freedesktop.org/show_bug.cgi?id=103665
  [fdo#104894]: https://bugs.freedesktop.org/show_bug.cgi?id=104894
  [fdo#105363]: https://bugs.freedesktop.org/show_bug.cgi?id=105363
  [fdo#105411]: https://bugs.freedesktop.org/show_bug.cgi?id=105411
  [fdo#106509]: https://bugs.freedesktop.org/show_bug.cgi?id=106509
  [fdo#106641]: https://bugs.freedesktop.org/show_bug.cgi?id=106641
  [fdo#107409]: https://bugs.freedesktop.org/show_bug.cgi?id=107409
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#108590]: https://bugs.freedesktop.org/show_bug.cgi?id=108590
  [fdo#108686]: https://bugs.freedesktop.org/show_bug.cgi?id=108686
  [fdo#109016]: https://bugs.freedesktop.org/show_bug.cgi?id=109016
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
  [fdo#109660]: https://bugs.freedesktop.org/show_bug.cgi?id=109660
  [fdo#110222]: https://bugs.freedesktop.org/show_bug.cgi?id=110222
  [fdo#110279]: https://bugs.freedesktop.org/show_bug.cgi?id=110279
  [fdo#99912]: https://bugs.freedesktop.org/show_bug.cgi?id=99912


Participating hosts (10 -> 5)
------------------------------

  Missing    (5): shard-skl pig-hsw-4770r pig-glk-j5005 shard-iclb pig-skl-6260u 


Build changes
-------------

    * IGT: IGT_4922 -> IGTPW_2763
    * Piglit: piglit_4509 -> None

  CI_DRM_5856: 55074bd825098a71779cf65a69786547f0eccbe9 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_2763: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2763/
  IGT_4922: e941e4a29438c7130554492e4daf52afbc99ffdf @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2763/
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [igt-dev] [PATCH i-g-t 1/6] tests/kms_color: Nuke local struct definitions
  2019-04-02 16:33 [igt-dev] [PATCH i-g-t 1/6] tests/kms_color: Nuke local struct definitions Ville Syrjala
                   ` (6 preceding siblings ...)
  2019-04-03  6:33 ` [igt-dev] ✓ Fi.CI.IGT: " Patchwork
@ 2019-04-03  8:40 ` Daniel Vetter
  7 siblings, 0 replies; 16+ messages in thread
From: Daniel Vetter @ 2019-04-03  8:40 UTC (permalink / raw)
  To: Ville Syrjala; +Cc: igt-dev

On Tue, Apr 02, 2019 at 07:33:43PM +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> I think we can assume fresh enough headers by now, so remove
> the local _drm_color_ctm and _drm_color_lut structs definitions.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> ---
>  tests/kms_color.c | 74 +++++++++++++++++++----------------------------
>  1 file changed, 29 insertions(+), 45 deletions(-)
> 
> diff --git a/tests/kms_color.c b/tests/kms_color.c
> index decf3c2a680e..1e5a5d329bea 100644
> --- a/tests/kms_color.c
> +++ b/tests/kms_color.c
> @@ -31,22 +31,6 @@
>  
>  IGT_TEST_DESCRIPTION("Test Color Features at Pipe level");
>  
> -/* Data structures for gamma/degamma ramps & ctm matrix. */
> -struct _drm_color_ctm {
> -	/* Transformation matrix in S31.32 format. */
> -	__s64 matrix[9];
> -};
> -
> -struct _drm_color_lut {
> -	/*
> -	 * Data is U0.16 fixed point format.
> -	 */
> -	__u16 red;
> -	__u16 green;
> -	__u16 blue;
> -	__u16 reserved;
> -};
> -
>  /* Internal */
>  typedef struct {
>  	double r, g, b;
> @@ -163,13 +147,13 @@ static double *generate_table_zero(uint32_t lut_size)
>  	return coeffs;
>  }
>  
> -static struct _drm_color_lut *coeffs_to_lut(data_t *data,
> -					    const double *coefficients,
> -					    uint32_t lut_size,
> -					    uint32_t color_depth,
> -					    int off)
> +static struct drm_color_lut *coeffs_to_lut(data_t *data,
> +					   const double *coefficients,
> +					   uint32_t lut_size,
> +					   uint32_t color_depth,
> +					   int off)
>  {
> -	struct _drm_color_lut *lut;
> +	struct drm_color_lut *lut;
>  	uint32_t i;
>  	uint32_t max_value = (1 << 16) - 1;
>  	uint32_t mask;
> @@ -179,7 +163,7 @@ static struct _drm_color_lut *coeffs_to_lut(data_t *data,
>  	else
>  		mask = max_value;
>  
> -	lut = malloc(sizeof(struct _drm_color_lut) * lut_size);
> +	lut = malloc(sizeof(struct drm_color_lut) * lut_size);
>  
>  	if (IS_CHERRYVIEW(data->devid))
>  		lut_size -= 1;
> @@ -211,8 +195,8 @@ static void set_degamma(data_t *data,
>  			igt_pipe_t *pipe,
>  			const double *coefficients)
>  {
> -	size_t size = sizeof(struct _drm_color_lut) * data->degamma_lut_size;
> -	struct _drm_color_lut *lut = coeffs_to_lut(data,
> +	size_t size = sizeof(struct drm_color_lut) * data->degamma_lut_size;
> +	struct drm_color_lut *lut = coeffs_to_lut(data,
>  						   coefficients,
>  						   data->degamma_lut_size,
>  						   data->color_depth, 0);
> @@ -226,8 +210,8 @@ static void set_gamma(data_t *data,
>  		      igt_pipe_t *pipe,
>  		      const double *coefficients)
>  {
> -	size_t size = sizeof(struct _drm_color_lut) * data->gamma_lut_size;
> -	struct _drm_color_lut *lut = coeffs_to_lut(data,
> +	size_t size = sizeof(struct drm_color_lut) * data->gamma_lut_size;
> +	struct drm_color_lut *lut = coeffs_to_lut(data,
>  						   coefficients,
>  						   data->gamma_lut_size,
>  						   data->color_depth, 0);
> @@ -239,7 +223,7 @@ static void set_gamma(data_t *data,
>  
>  static void set_ctm(igt_pipe_t *pipe, const double *coefficients)
>  {
> -	struct _drm_color_ctm ctm;
> +	struct drm_color_ctm ctm;
>  	int i;
>  
>  	for (i = 0; i < ARRAY_SIZE(ctm.matrix); i++) {
> @@ -552,7 +536,7 @@ static void test_pipe_legacy_gamma_reset(data_t *data,
>  	double *degamma_linear, *gamma_zero;
>  	uint32_t i, legacy_lut_size;
>  	uint16_t *red_lut, *green_lut, *blue_lut;
> -	struct _drm_color_lut *lut;
> +	struct drm_color_lut *lut;
>  	drmModePropertyBlobPtr blob;
>  	igt_output_t *output;
>  
> @@ -578,20 +562,20 @@ static void test_pipe_legacy_gamma_reset(data_t *data,
>  
>  		blob = get_blob(data, primary->pipe, IGT_CRTC_DEGAMMA_LUT);
>  		igt_assert(blob &&
> -			   blob->length == (sizeof(struct _drm_color_lut) *
> +			   blob->length == (sizeof(struct drm_color_lut) *
>  					    data->degamma_lut_size));
>  		drmModeFreePropertyBlob(blob);
>  
>  		blob = get_blob(data, primary->pipe, IGT_CRTC_CTM);
>  		igt_assert(blob &&
> -			   blob->length == sizeof(struct _drm_color_ctm));
> +			   blob->length == sizeof(struct drm_color_ctm));
>  		drmModeFreePropertyBlob(blob);
>  
>  		blob = get_blob(data, primary->pipe, IGT_CRTC_GAMMA_LUT);
>  		igt_assert(blob &&
> -			   blob->length == (sizeof(struct _drm_color_lut) *
> +			   blob->length == (sizeof(struct drm_color_lut) *
>  					    data->gamma_lut_size));
> -		lut = (struct _drm_color_lut *) blob->data;
> +		lut = (struct drm_color_lut *) blob->data;
>  		for (i = 0; i < data->gamma_lut_size; i++)
>  			igt_assert(lut[i].red == 0 &&
>  				   lut[i].green == 0 &&
> @@ -625,9 +609,9 @@ static void test_pipe_legacy_gamma_reset(data_t *data,
>  
>  		blob = get_blob(data, primary->pipe, IGT_CRTC_GAMMA_LUT);
>  		igt_assert(blob &&
> -			   blob->length == (sizeof(struct _drm_color_lut) *
> +			   blob->length == (sizeof(struct drm_color_lut) *
>  					    legacy_lut_size));
> -		lut = (struct _drm_color_lut *) blob->data;
> +		lut = (struct drm_color_lut *) blob->data;
>  		for (i = 0; i < legacy_lut_size; i++)
>  			igt_assert(lut[i].red == 0xffff &&
>  				   lut[i].green == 0xffff &&
> @@ -1079,11 +1063,11 @@ invalid_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);
> -	size_t gamma_lut_size = data->gamma_lut_size * sizeof(struct _drm_color_lut);
> +	size_t degamma_lut_size = data->degamma_lut_size * sizeof(struct drm_color_lut);
> +	size_t gamma_lut_size = data->gamma_lut_size * sizeof(struct drm_color_lut);
>  
> -	struct _drm_color_lut *degamma_lut = malloc(data->degamma_lut_size * sizeof(struct _drm_color_lut) * 2);
> -	struct _drm_color_lut *gamma_lut = malloc(data->gamma_lut_size * sizeof(struct _drm_color_lut) * 2);
> +	struct drm_color_lut *degamma_lut = malloc(data->degamma_lut_size * sizeof(struct drm_color_lut) * 2);
> +	struct drm_color_lut *gamma_lut = malloc(data->gamma_lut_size * sizeof(struct drm_color_lut) * 2);
>  
>  	igt_display_commit2(display, display->is_atomic ? COMMIT_ATOMIC : COMMIT_LEGACY);
>  
> @@ -1097,7 +1081,7 @@ invalid_lut_sizes(data_t *data)
>  						     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)),
> +						     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);
> @@ -1116,7 +1100,7 @@ invalid_lut_sizes(data_t *data)
>  						     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)),
> +						     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);
> @@ -1138,18 +1122,18 @@ invalid_ctm_matrix_sizes(data_t *data)
>  	if (!igt_pipe_obj_has_prop(pipe, IGT_CRTC_CTM))
>  		return;
>  
> -	ptr = malloc(sizeof(struct _drm_color_ctm) * 4);
> +	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),
> +					     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),
> +					     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),
> +					     sizeof(struct drm_color_ctm) * 2),
>  		      -EINVAL);
>  	igt_assert_eq(pipe_set_property_blob_id(pipe, IGT_CRTC_CTM, pipe->crtc_id),
>  		      -EINVAL);
> -- 
> 2.19.2
> 
> _______________________________________________
> igt-dev mailing list
> igt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [igt-dev] [PATCH i-g-t 2/6] tests/kms_color: Wrap LUTs in a gamma_lut_t struct
  2019-04-02 16:33 ` [igt-dev] [PATCH i-g-t 2/6] tests/kms_color: Wrap LUTs in a gamma_lut_t struct Ville Syrjala
@ 2019-04-03 10:50   ` Daniel Vetter
  0 siblings, 0 replies; 16+ messages in thread
From: Daniel Vetter @ 2019-04-03 10:50 UTC (permalink / raw)
  To: Ville Syrjala; +Cc: igt-dev

On Tue, Apr 02, 2019 at 07:33:44PM +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> To make life easier let's wrap the LUTs in a small struct.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  tests/kms_color.c | 119 +++++++++++++++++++++++++---------------------
>  1 file changed, 66 insertions(+), 53 deletions(-)
> 
> diff --git a/tests/kms_color.c b/tests/kms_color.c
> index 1e5a5d329bea..43e59dd42b0b 100644
> --- a/tests/kms_color.c
> +++ b/tests/kms_color.c
> @@ -47,6 +47,10 @@ typedef struct {
>  	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,
> @@ -106,55 +110,68 @@ static void paint_rectangles(data_t *data,
>  	igt_put_cairo_ctx(data->drm_fd, fb, cr);
>  }
>  
> -static double *generate_table(uint32_t lut_size, double exp)
> +static gamma_lut_t *alloc_lut(int lut_size)
>  {
> -	double *coeffs;
> -	uint32_t i;
> +	gamma_lut_t *gamma;
>  
>  	igt_assert_lt(0, lut_size);
>  
> -	coeffs = malloc(sizeof(double) * lut_size);
> +	gamma = malloc(sizeof(*gamma) + lut_size * sizeof(gamma->coeffs[0]));

igt_assert(gamma); is missing here I think.

> +	gamma->size = lut_size;
>  
> -	for (i = 0; i < lut_size; i++)
> -		coeffs[i] = powf((double) i * 1.0 / (double) (lut_size - 1), exp);
> +	return gamma;
> +}
> +
> +static void free_lut(gamma_lut_t *gamma)
> +{
> +	if (!gamma)
> +		return;
>  
> -	return coeffs;
> +	free(gamma);
>  }
>  
> -static double *generate_table_max(uint32_t lut_size)
> +static gamma_lut_t *generate_table(int lut_size, double exp)
>  {
> -	double *coeffs;
> -	uint32_t i;
> +	gamma_lut_t *gamma = alloc_lut(lut_size);
> +	int i;
>  
> -	igt_assert_lt(0, lut_size);
> +	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;
> +}
>  
> -	coeffs = malloc(sizeof(double) * lut_size);
> -	coeffs[0] = 0.0;
> +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++)
> -		coeffs[i] = 1.0;
> +		gamma->coeffs[i] = 1.0;
>  
> -	return coeffs;
> +	return gamma;
>  }
>  
> -static double *generate_table_zero(uint32_t lut_size)
> +static gamma_lut_t *generate_table_zero(int lut_size)
>  {
> -	double *coeffs = malloc(sizeof(double) * lut_size);
> -	uint32_t i;
> +	gamma_lut_t *gamma = alloc_lut(lut_size);
> +	int i;
>  
>  	for (i = 0; i < lut_size; i++)
> -		coeffs[i] = 0.0;
> +		gamma->coeffs[i] = 0.0;
>  
> -	return coeffs;
> +	return gamma;
>  }
>  
>  static struct drm_color_lut *coeffs_to_lut(data_t *data,
> -					   const double *coefficients,
> -					   uint32_t lut_size,
> +					   const gamma_lut_t *gamma,
>  					   uint32_t color_depth,
>  					   int off)
>  {
>  	struct drm_color_lut *lut;
> -	uint32_t i;
> +	int i, lut_size = gamma->size;
>  	uint32_t max_value = (1 << 16) - 1;
>  	uint32_t mask;
>  
> @@ -168,7 +185,7 @@ static struct drm_color_lut *coeffs_to_lut(data_t *data,
>  	if (IS_CHERRYVIEW(data->devid))
>  		lut_size -= 1;
>  	for (i = 0; i < lut_size; i++) {
> -		uint32_t v = (coefficients[i] * max_value);
> +		uint32_t v = (gamma->coeffs[i] * max_value);
>  
>  		/*
>  		 * Hardware might encode colors on a different number of bits
> @@ -193,13 +210,11 @@ static struct drm_color_lut *coeffs_to_lut(data_t *data,
>  
>  static void set_degamma(data_t *data,
>  			igt_pipe_t *pipe,
> -			const double *coefficients)
> +			const gamma_lut_t *gamma)
>  {
> -	size_t size = sizeof(struct drm_color_lut) * data->degamma_lut_size;
> -	struct drm_color_lut *lut = coeffs_to_lut(data,
> -						   coefficients,
> -						   data->degamma_lut_size,
> -						   data->color_depth, 0);
> +	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);
>  
> @@ -208,15 +223,13 @@ static void set_degamma(data_t *data,
>  
>  static void set_gamma(data_t *data,
>  		      igt_pipe_t *pipe,
> -		      const double *coefficients)
> +		      const gamma_lut_t *gamma)
>  {
> -	size_t size = sizeof(struct drm_color_lut) * data->gamma_lut_size;
> -	struct drm_color_lut *lut = coeffs_to_lut(data,
> -						   coefficients,
> -						   data->gamma_lut_size,
> -						   data->color_depth, 0);
> +	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);
> +	igt_pipe_obj_replace_prop_blob(pipe, IGT_CRTC_GAMMA_LUT, lut, size);
>  
>  	free(lut);
>  }
> @@ -252,8 +265,8 @@ static void test_pipe_degamma(data_t *data,
>  			      igt_plane_t *primary)
>  {
>  	igt_output_t *output;
> -	double *degamma_linear, *degamma_full;
> -	double *gamma_linear;
> +	gamma_lut_t *degamma_linear, *degamma_full;
> +	gamma_lut_t *gamma_linear;
>  	color_t red_green_blue[] = {
>  		{ 1.0, 0.0, 0.0 },
>  		{ 0.0, 1.0, 0.0 },
> @@ -323,9 +336,9 @@ static void test_pipe_degamma(data_t *data,
>  		igt_output_set_pipe(output, PIPE_NONE);
>  	}
>  
> -	free(degamma_linear);
> -	free(degamma_full);
> -	free(gamma_linear);
> +	free_lut(degamma_linear);
> +	free_lut(degamma_full);
> +	free_lut(gamma_linear);
>  }
>  
>  /*
> @@ -336,7 +349,7 @@ static void test_pipe_gamma(data_t *data,
>  			    igt_plane_t *primary)
>  {
>  	igt_output_t *output;
> -	double *gamma_full;
> +	gamma_lut_t *gamma_full;
>  	color_t red_green_blue[] = {
>  		{ 1.0, 0.0, 0.0 },
>  		{ 0.0, 1.0, 0.0 },
> @@ -402,7 +415,7 @@ static void test_pipe_gamma(data_t *data,
>  		igt_output_set_pipe(output, PIPE_NONE);
>  	}
>  
> -	free(gamma_full);
> +	free_lut(gamma_full);
>  }
>  
>  /*
> @@ -533,7 +546,7 @@ static void test_pipe_legacy_gamma_reset(data_t *data,
>  		0.0, 0.0, 1.0
>  	};
>  	drmModeCrtc *kms_crtc;
> -	double *degamma_linear, *gamma_zero;
> +	gamma_lut_t *degamma_linear, *gamma_zero;
>  	uint32_t i, legacy_lut_size;
>  	uint16_t *red_lut, *green_lut, *blue_lut;
>  	struct drm_color_lut *lut;
> @@ -622,8 +635,8 @@ static void test_pipe_legacy_gamma_reset(data_t *data,
>  		igt_output_set_pipe(output, PIPE_NONE);
>  	}
>  
> -	free(degamma_linear);
> -	free(gamma_zero);
> +	free_lut(degamma_linear);
> +	free_lut(gamma_zero);
>  }
>  
>  static bool crc_equal(igt_crc_t *a, igt_crc_t *b)
> @@ -646,7 +659,7 @@ static bool test_pipe_ctm(data_t *data,
>  		0.0, 1.0, 0.0,
>  		0.0, 0.0, 1.0
>  	};
> -	double *degamma_linear, *gamma_linear;
> +	gamma_lut_t *degamma_linear, *gamma_linear;
>  	igt_output_t *output;
>  	bool ret = true;
>  
> @@ -709,8 +722,8 @@ static bool test_pipe_ctm(data_t *data,
>  		igt_output_set_pipe(output, PIPE_NONE);
>  	}
>  
> -	free(degamma_linear);
> -	free(gamma_linear);
> +	free_lut(degamma_linear);
> +	free_lut(gamma_linear);
>  
>  	return ret;
>  }
> @@ -745,7 +758,7 @@ static void test_pipe_limited_range_ctm(data_t *data,
>  	double ctm[] = { 1.0, 0.0, 0.0,
>  			0.0, 1.0, 0.0,
>  			0.0, 0.0, 1.0 };
> -	double *degamma_linear, *gamma_linear;
> +	gamma_lut_t *degamma_linear, *gamma_linear;
>  	igt_output_t *output;
>  	bool has_broadcast_rgb_output = false;
>  
> @@ -814,8 +827,8 @@ static void test_pipe_limited_range_ctm(data_t *data,
>  		igt_assert_crc_equal(&crc_full, &crc_limited);
>  	}
>  
> -	free(gamma_linear);
> -	free(degamma_linear);
> +	free_lut(gamma_linear);
> +	free_lut(degamma_linear);
>  
>  	igt_require(has_broadcast_rgb_output);
>  }

With the nit addressed:

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> -- 
> 2.19.2
> 
> _______________________________________________
> igt-dev mailing list
> igt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [igt-dev] [PATCH i-g-t 3/6] tests/kms_color: Reuse some already compute values
  2019-04-02 16:33 ` [igt-dev] [PATCH i-g-t 3/6] tests/kms_color: Reuse some already compute values Ville Syrjala
@ 2019-04-03 10:51   ` Daniel Vetter
  0 siblings, 0 replies; 16+ messages in thread
From: Daniel Vetter @ 2019-04-03 10:51 UTC (permalink / raw)
  To: Ville Syrjala; +Cc: igt-dev

On Tue, Apr 02, 2019 at 07:33:45PM +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> We already compute the lut_size*entry_size so let's reuse those
> when allocating the LUTs.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  tests/kms_color.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/kms_color.c b/tests/kms_color.c
> index 43e59dd42b0b..c65bb88cd7dc 100644
> --- a/tests/kms_color.c
> +++ b/tests/kms_color.c
> @@ -1079,8 +1079,8 @@ invalid_lut_sizes(data_t *data)
>  	size_t degamma_lut_size = data->degamma_lut_size * sizeof(struct drm_color_lut);
>  	size_t gamma_lut_size = data->gamma_lut_size * sizeof(struct drm_color_lut);
>  
> -	struct drm_color_lut *degamma_lut = malloc(data->degamma_lut_size * sizeof(struct drm_color_lut) * 2);
> -	struct drm_color_lut *gamma_lut = malloc(data->gamma_lut_size * sizeof(struct drm_color_lut) * 2);
> +	struct drm_color_lut *degamma_lut = malloc(degamma_lut_size * 2);
> +	struct drm_color_lut *gamma_lut = malloc(gamma_lut_size * 2);

Could also bikeshed to calloc, but this isn't kernel code, so who cares
about overflows.

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

>  
>  	igt_display_commit2(display, display->is_atomic ? COMMIT_ATOMIC : COMMIT_LEGACY);
>  
> -- 
> 2.19.2
> 
> _______________________________________________
> igt-dev mailing list
> igt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [igt-dev] [PATCH i-g-t 4/6] tests/kms_color: Allow most subtests to run with a partial color pipeline
  2019-04-02 16:33 ` [igt-dev] [PATCH i-g-t 4/6] tests/kms_color: Allow most subtests to run with a partial color pipeline Ville Syrjala
@ 2019-04-03 13:50   ` Daniel Vetter
  2019-04-03 14:22     ` Ville Syrjälä
  0 siblings, 1 reply; 16+ messages in thread
From: Daniel Vetter @ 2019-04-03 13:50 UTC (permalink / raw)
  To: Ville Syrjala; +Cc: igt-dev

On Tue, Apr 02, 2019 at 07:33:46PM +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Requiring a full color pipeline when we're just testing eg. the gamma
> LUT is silly. Make the requirements more sensible.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  tests/kms_color.c | 47 +++++++++++++++++++++++++++++++++--------------
>  1 file changed, 33 insertions(+), 14 deletions(-)
> 
> diff --git a/tests/kms_color.c b/tests/kms_color.c
> index c65bb88cd7dc..554356a04f75 100644
> --- a/tests/kms_color.c
> +++ b/tests/kms_color.c
> @@ -252,9 +252,15 @@ static void set_ctm(igt_pipe_t *pipe, const double *coefficients)
>  	igt_pipe_obj_replace_prop_blob(pipe, IGT_CRTC_CTM, &ctm, sizeof(ctm));
>  }
>  
> -#define disable_degamma(pipe) igt_pipe_obj_replace_prop_blob(pipe, IGT_CRTC_DEGAMMA_LUT, NULL, 0)
> -#define disable_gamma(pipe) igt_pipe_obj_replace_prop_blob(pipe, IGT_CRTC_GAMMA_LUT, NULL, 0)
> -#define disable_ctm(pipe) igt_pipe_obj_replace_prop_blob(pipe, IGT_CRTC_CTM, NULL, 0)
> +static void disable_prop(igt_pipe_t *pipe, enum igt_atomic_crtc_properties prop)

Hm, could even be an igt_kms helper function I think. For another patch.

> +{
> +	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
> @@ -273,6 +279,9 @@ static void test_pipe_degamma(data_t *data,
>  		{ 0.0, 0.0, 1.0 }
>  	};
>  
> +	igt_require(igt_pipe_obj_has_prop(primary->pipe, IGT_CRTC_DEGAMMA_LUT));
> +	igt_require(igt_pipe_obj_has_prop(primary->pipe, IGT_CRTC_GAMMA_LUT));
> +
>  	degamma_linear = generate_table(data->degamma_lut_size, 1.0);
>  	degamma_full = generate_table_max(data->degamma_lut_size);
>  
> @@ -356,6 +365,8 @@ static void test_pipe_gamma(data_t *data,
>  		{ 0.0, 0.0, 1.0 }
>  	};
>  
> +	igt_require(igt_pipe_obj_has_prop(primary->pipe, IGT_CRTC_GAMMA_LUT));
> +
>  	gamma_full = generate_table_max(data->gamma_lut_size);
>  
>  	for_each_valid_output_on_pipe(&data->display, primary->pipe->pipe, output) {
> @@ -553,6 +564,11 @@ static void test_pipe_legacy_gamma_reset(data_t *data,
>  	drmModePropertyBlobPtr blob;
>  	igt_output_t *output;
>  
> +	/* FIXME accept any one of these */
> +	igt_require(igt_pipe_obj_has_prop(primary->pipe, IGT_CRTC_DEGAMMA_LUT));
> +	igt_require(igt_pipe_obj_has_prop(primary->pipe, IGT_CRTC_GAMMA_LUT));
> +	igt_require(igt_pipe_obj_has_prop(primary->pipe, IGT_CRTC_CTM));
> +
>  	degamma_linear = generate_table(data->degamma_lut_size, 1.0);
>  	gamma_zero = generate_table_zero(data->gamma_lut_size);
>  
> @@ -663,6 +679,8 @@ static bool test_pipe_ctm(data_t *data,
>  	igt_output_t *output;
>  	bool ret = true;
>  
> +	igt_require(igt_pipe_obj_has_prop(primary->pipe, IGT_CRTC_CTM));

Hm, am I blind or did we not have any igt require for the CTM beforehand?
Would be good to mention that in the commit message this your patch here
fixes that.

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>


> +
>  	degamma_linear = generate_table(data->degamma_lut_size, 1.0);
>  	gamma_linear = generate_table(data->gamma_lut_size, 1.0);
>  
> @@ -861,19 +879,20 @@ run_tests_for_pipe(data_t *data, enum pipe p)
>  						  primary->pipe->pipe,
>  						  INTEL_PIPE_CRC_SOURCE_AUTO);
>  
> -		igt_require(igt_pipe_obj_has_prop(&data->display.pipes[p], IGT_CRTC_DEGAMMA_LUT_SIZE));
> -		igt_require(igt_pipe_obj_has_prop(&data->display.pipes[p], IGT_CRTC_GAMMA_LUT_SIZE));
> -
> -		data->degamma_lut_size =
> -			igt_pipe_obj_get_prop(&data->display.pipes[p],
> -					      IGT_CRTC_DEGAMMA_LUT_SIZE);
> +		if (igt_pipe_obj_has_prop(&data->display.pipes[p], IGT_CRTC_DEGAMMA_LUT_SIZE)) {
> +			data->degamma_lut_size =
> +				igt_pipe_obj_get_prop(&data->display.pipes[p],
> +						      IGT_CRTC_DEGAMMA_LUT_SIZE);
> +			igt_assert_lt(0, data->degamma_lut_size);
> +		}
>  
> -		data->gamma_lut_size =
> -			igt_pipe_obj_get_prop(&data->display.pipes[p],
> -					      IGT_CRTC_GAMMA_LUT_SIZE);
> +		if (igt_pipe_obj_has_prop(&data->display.pipes[p], IGT_CRTC_GAMMA_LUT_SIZE)) {
> +			data->gamma_lut_size =
> +				igt_pipe_obj_get_prop(&data->display.pipes[p],
> +						      IGT_CRTC_GAMMA_LUT_SIZE);
> +			igt_assert_lt(0, data->gamma_lut_size);
> +		}
>  
> -		igt_assert_lt(0, data->degamma_lut_size);
> -		igt_assert_lt(0, data->gamma_lut_size);
>  		igt_display_require_output_on_pipe(&data->display, p);
>  	}
>  
> -- 
> 2.19.2
> 
> _______________________________________________
> igt-dev mailing list
> igt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [igt-dev] [PATCH i-g-t 5/6] tests/kms_color: Split invalid_lut_sizes() into gamma vs. degamma versions
  2019-04-02 16:33 ` [igt-dev] [PATCH i-g-t 5/6] tests/kms_color: Split invalid_lut_sizes() into gamma vs. degamma versions Ville Syrjala
@ 2019-04-03 13:52   ` Daniel Vetter
  2019-04-03 14:20     ` Ville Syrjälä
  0 siblings, 1 reply; 16+ messages in thread
From: Daniel Vetter @ 2019-04-03 13:52 UTC (permalink / raw)
  To: Ville Syrjala; +Cc: igt-dev

On Tue, Apr 02, 2019 at 07:33:47PM +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Split the invalid-lut-sizes test into separate gamma and degamma tests.
> This way we can report SKIP for the thing we don't have. Also make the
> CTM invalid sizes test report a skip too.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  tests/kms_color.c | 103 ++++++++++++++++++++++++++--------------------
>  1 file changed, 59 insertions(+), 44 deletions(-)
> 
> diff --git a/tests/kms_color.c b/tests/kms_color.c
> index 554356a04f75..fd4c9a6dd812 100644
> --- a/tests/kms_color.c
> +++ b/tests/kms_color.c
> @@ -1091,57 +1091,70 @@ pipe_set_property_blob(igt_pipe_t *pipe, enum igt_atomic_crtc_properties prop, v
>  }
>  
>  static void
> -invalid_lut_sizes(data_t *data)
> +invalid_gamma_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);
>  	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));
>  
> -	struct drm_color_lut *degamma_lut = malloc(degamma_lut_size * 2);
> -	struct drm_color_lut *gamma_lut = malloc(gamma_lut_size * 2);
> +	gamma_lut = malloc(gamma_lut_size * 2);
>  
>  	igt_display_commit2(display, display->is_atomic ? COMMIT_ATOMIC : COMMIT_LEGACY);
>  
> -	if (igt_pipe_obj_has_prop(pipe, IGT_CRTC_DEGAMMA_LUT)) {
> -		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);
> -	}
> +	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);
>  
> -	if (igt_pipe_obj_has_prop(pipe, IGT_CRTC_GAMMA_LUT)) {
> -		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);
> -	free(gamma_lut);
>  }
>  
>  static void
> @@ -1151,8 +1164,7 @@ invalid_ctm_matrix_sizes(data_t *data)
>  	igt_pipe_t *pipe = &display->pipes[0];
>  	void *ptr;
>  
> -	if (!igt_pipe_obj_has_prop(pipe, IGT_CRTC_CTM))
> -		return;
> +	igt_require(igt_pipe_obj_has_prop(pipe, IGT_CRTC_CTM));

Ah, you fixed that one too. I guess I should have kept reviewing before
typing that quick patch of mine :-)

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

>  
>  	ptr = malloc(sizeof(struct drm_color_ctm) * 4);
>  
> @@ -1195,8 +1207,11 @@ igt_main
>  		igt_subtest_group
>  			run_tests_for_pipe(&data, pipe);
>  
> -	igt_subtest_f("pipe-invalid-lut-sizes")
> -		invalid_lut_sizes(&data);
> +	igt_subtest_f("pipe-invalid-gamma-lut-sizes")
> +		invalid_gamma_lut_sizes(&data);
> +
> +	igt_subtest_f("pipe-invalid-degamma-lut-sizes")
> +		invalid_degamma_lut_sizes(&data);
>  
>  	igt_subtest_f("pipe-invalid-ctm-matrix-sizes")
>  		invalid_ctm_matrix_sizes(&data);
> -- 
> 2.19.2
> 
> _______________________________________________
> igt-dev mailing list
> igt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [igt-dev] [PATCH i-g-t 6/6] tests/kms_color: Make legacy-gamma-reset work with a partial color pipeline
  2019-04-02 16:33 ` [igt-dev] [PATCH i-g-t 6/6] tests/kms_color: Make legacy-gamma-reset work with a partial color pipeline Ville Syrjala
@ 2019-04-03 13:53   ` Daniel Vetter
  0 siblings, 0 replies; 16+ messages in thread
From: Daniel Vetter @ 2019-04-03 13:53 UTC (permalink / raw)
  To: Ville Syrjala; +Cc: igt-dev

On Tue, Apr 02, 2019 at 07:33:48PM +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> No reason why we shouldn't be able to execute the legacy-gamma-reset
> test with a partial color pipeline.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Yay for more robust tests!

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  tests/kms_color.c | 45 ++++++++++++++++++++++++++-------------------
>  1 file changed, 26 insertions(+), 19 deletions(-)
> 
> diff --git a/tests/kms_color.c b/tests/kms_color.c
> index fd4c9a6dd812..43126c79079d 100644
> --- a/tests/kms_color.c
> +++ b/tests/kms_color.c
> @@ -557,19 +557,17 @@ static void test_pipe_legacy_gamma_reset(data_t *data,
>  		0.0, 0.0, 1.0
>  	};
>  	drmModeCrtc *kms_crtc;
> -	gamma_lut_t *degamma_linear, *gamma_zero;
> +	gamma_lut_t *degamma_linear = NULL, *gamma_zero;
>  	uint32_t i, legacy_lut_size;
>  	uint16_t *red_lut, *green_lut, *blue_lut;
>  	struct drm_color_lut *lut;
>  	drmModePropertyBlobPtr blob;
>  	igt_output_t *output;
>  
> -	/* FIXME accept any one of these */
> -	igt_require(igt_pipe_obj_has_prop(primary->pipe, IGT_CRTC_DEGAMMA_LUT));
>  	igt_require(igt_pipe_obj_has_prop(primary->pipe, IGT_CRTC_GAMMA_LUT));
> -	igt_require(igt_pipe_obj_has_prop(primary->pipe, IGT_CRTC_CTM));
>  
> -	degamma_linear = generate_table(data->degamma_lut_size, 1.0);
> +	if (igt_pipe_obj_has_prop(primary->pipe, IGT_CRTC_DEGAMMA_LUT))
> +		degamma_linear = generate_table(data->degamma_lut_size, 1.0);
>  	gamma_zero = generate_table_zero(data->gamma_lut_size);
>  
>  	for_each_valid_output_on_pipe(&data->display, primary->pipe->pipe, output) {
> @@ -584,21 +582,27 @@ static void test_pipe_legacy_gamma_reset(data_t *data,
>  		/* Set a degama & gamma LUT and a CTM using the
>  		 * properties and verify the content of the
>  		 * properties. */
> -		set_degamma(data, primary->pipe, degamma_linear);
> -		set_ctm(primary->pipe, ctm_identity);
> +		if (igt_pipe_obj_has_prop(primary->pipe, IGT_CRTC_DEGAMMA_LUT))
> +			set_degamma(data, primary->pipe, degamma_linear);
> +		if (igt_pipe_obj_has_prop(primary->pipe, IGT_CRTC_CTM))
> +			set_ctm(primary->pipe, ctm_identity);
>  		set_gamma(data, primary->pipe, gamma_zero);
>  		igt_display_commit(&data->display);
>  
> -		blob = get_blob(data, primary->pipe, IGT_CRTC_DEGAMMA_LUT);
> -		igt_assert(blob &&
> -			   blob->length == (sizeof(struct drm_color_lut) *
> -					    data->degamma_lut_size));
> -		drmModeFreePropertyBlob(blob);
> +		if (igt_pipe_obj_has_prop(primary->pipe, IGT_CRTC_DEGAMMA_LUT)) {
> +			blob = get_blob(data, primary->pipe, IGT_CRTC_DEGAMMA_LUT);
> +			igt_assert(blob &&
> +				   blob->length == (sizeof(struct drm_color_lut) *
> +						    data->degamma_lut_size));
> +			drmModeFreePropertyBlob(blob);
> +		}
>  
> -		blob = get_blob(data, primary->pipe, IGT_CRTC_CTM);
> -		igt_assert(blob &&
> -			   blob->length == sizeof(struct drm_color_ctm));
> -		drmModeFreePropertyBlob(blob);
> +		if (igt_pipe_obj_has_prop(primary->pipe, IGT_CRTC_CTM)) {
> +			blob = get_blob(data, primary->pipe, IGT_CRTC_CTM);
> +			igt_assert(blob &&
> +				   blob->length == sizeof(struct drm_color_ctm));
> +			drmModeFreePropertyBlob(blob);
> +		}
>  
>  		blob = get_blob(data, primary->pipe, IGT_CRTC_GAMMA_LUT);
>  		igt_assert(blob &&
> @@ -632,9 +636,12 @@ static void test_pipe_legacy_gamma_reset(data_t *data,
>  			      0);
>  		igt_display_commit(&data->display);
>  
> -		igt_assert(get_blob(data, primary->pipe,
> -				    IGT_CRTC_DEGAMMA_LUT) == NULL);
> -		igt_assert(get_blob(data, primary->pipe, IGT_CRTC_CTM) == NULL);
> +		if (igt_pipe_obj_has_prop(primary->pipe, IGT_CRTC_DEGAMMA_LUT))
> +			igt_assert(get_blob(data, primary->pipe,
> +					    IGT_CRTC_DEGAMMA_LUT) == NULL);
> +
> +		if (igt_pipe_obj_has_prop(primary->pipe, IGT_CRTC_CTM))
> +			igt_assert(get_blob(data, primary->pipe, IGT_CRTC_CTM) == NULL);
>  
>  		blob = get_blob(data, primary->pipe, IGT_CRTC_GAMMA_LUT);
>  		igt_assert(blob &&
> -- 
> 2.19.2
> 
> _______________________________________________
> igt-dev mailing list
> igt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [igt-dev] [PATCH i-g-t 5/6] tests/kms_color: Split invalid_lut_sizes() into gamma vs. degamma versions
  2019-04-03 13:52   ` Daniel Vetter
@ 2019-04-03 14:20     ` Ville Syrjälä
  0 siblings, 0 replies; 16+ messages in thread
From: Ville Syrjälä @ 2019-04-03 14:20 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: igt-dev

On Wed, Apr 03, 2019 at 03:52:08PM +0200, Daniel Vetter wrote:
> On Tue, Apr 02, 2019 at 07:33:47PM +0300, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Split the invalid-lut-sizes test into separate gamma and degamma tests.
> > This way we can report SKIP for the thing we don't have. Also make the
> > CTM invalid sizes test report a skip too.
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  tests/kms_color.c | 103 ++++++++++++++++++++++++++--------------------
> >  1 file changed, 59 insertions(+), 44 deletions(-)
> > 
> > diff --git a/tests/kms_color.c b/tests/kms_color.c
> > index 554356a04f75..fd4c9a6dd812 100644
> > --- a/tests/kms_color.c
> > +++ b/tests/kms_color.c
> > @@ -1091,57 +1091,70 @@ pipe_set_property_blob(igt_pipe_t *pipe, enum igt_atomic_crtc_properties prop, v
> >  }
> >  
> >  static void
> > -invalid_lut_sizes(data_t *data)
> > +invalid_gamma_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);
> >  	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));
> >  
> > -	struct drm_color_lut *degamma_lut = malloc(degamma_lut_size * 2);
> > -	struct drm_color_lut *gamma_lut = malloc(gamma_lut_size * 2);
> > +	gamma_lut = malloc(gamma_lut_size * 2);
> >  
> >  	igt_display_commit2(display, display->is_atomic ? COMMIT_ATOMIC : COMMIT_LEGACY);
> >  
> > -	if (igt_pipe_obj_has_prop(pipe, IGT_CRTC_DEGAMMA_LUT)) {
> > -		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);
> > -	}
> > +	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);
> >  
> > -	if (igt_pipe_obj_has_prop(pipe, IGT_CRTC_GAMMA_LUT)) {
> > -		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);
> > -	free(gamma_lut);
> >  }
> >  
> >  static void
> > @@ -1151,8 +1164,7 @@ invalid_ctm_matrix_sizes(data_t *data)
> >  	igt_pipe_t *pipe = &display->pipes[0];
> >  	void *ptr;
> >  
> > -	if (!igt_pipe_obj_has_prop(pipe, IGT_CRTC_CTM))
> > -		return;
> > +	igt_require(igt_pipe_obj_has_prop(pipe, IGT_CRTC_CTM));
> 
> Ah, you fixed that one too. I guess I should have kept reviewing before
> typing that quick patch of mine :-)

Yeah, well I probably shouldn't have hidden it in this patch. But
lazyness won out for whatever reason and I didn't split it out.

> 
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> 
> >  
> >  	ptr = malloc(sizeof(struct drm_color_ctm) * 4);
> >  
> > @@ -1195,8 +1207,11 @@ igt_main
> >  		igt_subtest_group
> >  			run_tests_for_pipe(&data, pipe);
> >  
> > -	igt_subtest_f("pipe-invalid-lut-sizes")
> > -		invalid_lut_sizes(&data);
> > +	igt_subtest_f("pipe-invalid-gamma-lut-sizes")
> > +		invalid_gamma_lut_sizes(&data);
> > +
> > +	igt_subtest_f("pipe-invalid-degamma-lut-sizes")
> > +		invalid_degamma_lut_sizes(&data);
> >  
> >  	igt_subtest_f("pipe-invalid-ctm-matrix-sizes")
> >  		invalid_ctm_matrix_sizes(&data);
> > -- 
> > 2.19.2
> > 
> > _______________________________________________
> > igt-dev mailing list
> > igt-dev@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/igt-dev
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch

-- 
Ville Syrjälä
Intel
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [igt-dev] [PATCH i-g-t 4/6] tests/kms_color: Allow most subtests to run with a partial color pipeline
  2019-04-03 13:50   ` Daniel Vetter
@ 2019-04-03 14:22     ` Ville Syrjälä
  0 siblings, 0 replies; 16+ messages in thread
From: Ville Syrjälä @ 2019-04-03 14:22 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: igt-dev

On Wed, Apr 03, 2019 at 03:50:54PM +0200, Daniel Vetter wrote:
> On Tue, Apr 02, 2019 at 07:33:46PM +0300, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Requiring a full color pipeline when we're just testing eg. the gamma
> > LUT is silly. Make the requirements more sensible.
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  tests/kms_color.c | 47 +++++++++++++++++++++++++++++++++--------------
> >  1 file changed, 33 insertions(+), 14 deletions(-)
> > 
> > diff --git a/tests/kms_color.c b/tests/kms_color.c
> > index c65bb88cd7dc..554356a04f75 100644
> > --- a/tests/kms_color.c
> > +++ b/tests/kms_color.c
> > @@ -252,9 +252,15 @@ static void set_ctm(igt_pipe_t *pipe, const double *coefficients)
> >  	igt_pipe_obj_replace_prop_blob(pipe, IGT_CRTC_CTM, &ctm, sizeof(ctm));
> >  }
> >  
> > -#define disable_degamma(pipe) igt_pipe_obj_replace_prop_blob(pipe, IGT_CRTC_DEGAMMA_LUT, NULL, 0)
> > -#define disable_gamma(pipe) igt_pipe_obj_replace_prop_blob(pipe, IGT_CRTC_GAMMA_LUT, NULL, 0)
> > -#define disable_ctm(pipe) igt_pipe_obj_replace_prop_blob(pipe, IGT_CRTC_CTM, NULL, 0)
> > +static void disable_prop(igt_pipe_t *pipe, enum igt_atomic_crtc_properties prop)
> 
> Hm, could even be an igt_kms helper function I think. For another patch.
> 
> > +{
> > +	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
> > @@ -273,6 +279,9 @@ static void test_pipe_degamma(data_t *data,
> >  		{ 0.0, 0.0, 1.0 }
> >  	};
> >  
> > +	igt_require(igt_pipe_obj_has_prop(primary->pipe, IGT_CRTC_DEGAMMA_LUT));
> > +	igt_require(igt_pipe_obj_has_prop(primary->pipe, IGT_CRTC_GAMMA_LUT));
> > +
> >  	degamma_linear = generate_table(data->degamma_lut_size, 1.0);
> >  	degamma_full = generate_table_max(data->degamma_lut_size);
> >  
> > @@ -356,6 +365,8 @@ static void test_pipe_gamma(data_t *data,
> >  		{ 0.0, 0.0, 1.0 }
> >  	};
> >  
> > +	igt_require(igt_pipe_obj_has_prop(primary->pipe, IGT_CRTC_GAMMA_LUT));
> > +
> >  	gamma_full = generate_table_max(data->gamma_lut_size);
> >  
> >  	for_each_valid_output_on_pipe(&data->display, primary->pipe->pipe, output) {
> > @@ -553,6 +564,11 @@ static void test_pipe_legacy_gamma_reset(data_t *data,
> >  	drmModePropertyBlobPtr blob;
> >  	igt_output_t *output;
> >  
> > +	/* FIXME accept any one of these */
> > +	igt_require(igt_pipe_obj_has_prop(primary->pipe, IGT_CRTC_DEGAMMA_LUT));
> > +	igt_require(igt_pipe_obj_has_prop(primary->pipe, IGT_CRTC_GAMMA_LUT));
> > +	igt_require(igt_pipe_obj_has_prop(primary->pipe, IGT_CRTC_CTM));
> > +
> >  	degamma_linear = generate_table(data->degamma_lut_size, 1.0);
> >  	gamma_zero = generate_table_zero(data->gamma_lut_size);
> >  
> > @@ -663,6 +679,8 @@ static bool test_pipe_ctm(data_t *data,
> >  	igt_output_t *output;
> >  	bool ret = true;
> >  
> > +	igt_require(igt_pipe_obj_has_prop(primary->pipe, IGT_CRTC_CTM));
> 
> Hm, am I blind or did we not have any igt require for the CTM beforehand?

Looks like we did not.

> Would be good to mention that in the commit message this your patch here
> fixes that.

Sure, I'll amend the message a bit.

> 
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> 
> 
> > +
> >  	degamma_linear = generate_table(data->degamma_lut_size, 1.0);
> >  	gamma_linear = generate_table(data->gamma_lut_size, 1.0);
> >  
> > @@ -861,19 +879,20 @@ run_tests_for_pipe(data_t *data, enum pipe p)
> >  						  primary->pipe->pipe,
> >  						  INTEL_PIPE_CRC_SOURCE_AUTO);
> >  
> > -		igt_require(igt_pipe_obj_has_prop(&data->display.pipes[p], IGT_CRTC_DEGAMMA_LUT_SIZE));
> > -		igt_require(igt_pipe_obj_has_prop(&data->display.pipes[p], IGT_CRTC_GAMMA_LUT_SIZE));
> > -
> > -		data->degamma_lut_size =
> > -			igt_pipe_obj_get_prop(&data->display.pipes[p],
> > -					      IGT_CRTC_DEGAMMA_LUT_SIZE);
> > +		if (igt_pipe_obj_has_prop(&data->display.pipes[p], IGT_CRTC_DEGAMMA_LUT_SIZE)) {
> > +			data->degamma_lut_size =
> > +				igt_pipe_obj_get_prop(&data->display.pipes[p],
> > +						      IGT_CRTC_DEGAMMA_LUT_SIZE);
> > +			igt_assert_lt(0, data->degamma_lut_size);
> > +		}
> >  
> > -		data->gamma_lut_size =
> > -			igt_pipe_obj_get_prop(&data->display.pipes[p],
> > -					      IGT_CRTC_GAMMA_LUT_SIZE);
> > +		if (igt_pipe_obj_has_prop(&data->display.pipes[p], IGT_CRTC_GAMMA_LUT_SIZE)) {
> > +			data->gamma_lut_size =
> > +				igt_pipe_obj_get_prop(&data->display.pipes[p],
> > +						      IGT_CRTC_GAMMA_LUT_SIZE);
> > +			igt_assert_lt(0, data->gamma_lut_size);
> > +		}
> >  
> > -		igt_assert_lt(0, data->degamma_lut_size);
> > -		igt_assert_lt(0, data->gamma_lut_size);
> >  		igt_display_require_output_on_pipe(&data->display, p);
> >  	}
> >  
> > -- 
> > 2.19.2
> > 
> > _______________________________________________
> > igt-dev mailing list
> > igt-dev@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/igt-dev
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch

-- 
Ville Syrjälä
Intel
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2019-04-03 14:22 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-02 16:33 [igt-dev] [PATCH i-g-t 1/6] tests/kms_color: Nuke local struct definitions Ville Syrjala
2019-04-02 16:33 ` [igt-dev] [PATCH i-g-t 2/6] tests/kms_color: Wrap LUTs in a gamma_lut_t struct Ville Syrjala
2019-04-03 10:50   ` Daniel Vetter
2019-04-02 16:33 ` [igt-dev] [PATCH i-g-t 3/6] tests/kms_color: Reuse some already compute values Ville Syrjala
2019-04-03 10:51   ` Daniel Vetter
2019-04-02 16:33 ` [igt-dev] [PATCH i-g-t 4/6] tests/kms_color: Allow most subtests to run with a partial color pipeline Ville Syrjala
2019-04-03 13:50   ` Daniel Vetter
2019-04-03 14:22     ` Ville Syrjälä
2019-04-02 16:33 ` [igt-dev] [PATCH i-g-t 5/6] tests/kms_color: Split invalid_lut_sizes() into gamma vs. degamma versions Ville Syrjala
2019-04-03 13:52   ` Daniel Vetter
2019-04-03 14:20     ` Ville Syrjälä
2019-04-02 16:33 ` [igt-dev] [PATCH i-g-t 6/6] tests/kms_color: Make legacy-gamma-reset work with a partial color pipeline Ville Syrjala
2019-04-03 13:53   ` Daniel Vetter
2019-04-02 17:37 ` [igt-dev] ✓ Fi.CI.BAT: success for series starting with [i-g-t,1/6] tests/kms_color: Nuke local struct definitions Patchwork
2019-04-03  6:33 ` [igt-dev] ✓ Fi.CI.IGT: " Patchwork
2019-04-03  8:40 ` [igt-dev] [PATCH i-g-t 1/6] " Daniel Vetter

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.