Re: [PATCH i-g-t 6/6] test/kms_pipe_color: add test to verify legacy ioctl resets GAMMA_LUT

[Matt Roper <matthew.d.roper@intel.com>]

On Thu, Mar 10, 2016 at 12:47:05PM +0000, Lionel Landwerlin wrote:
> The GAMMA_LUT property must be updated when the legacy ioctl is
> triggered to ensure the new property does not impact older userspace
> code.
> 
> Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
> Cc: Matt Roper <matthew.d.roper@intel.com>
> ---
>  tests/kms_pipe_color.c | 101 +++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 101 insertions(+)
> 
> diff --git a/tests/kms_pipe_color.c b/tests/kms_pipe_color.c
> index 0241cf2..d17bf7f 100644
> --- a/tests/kms_pipe_color.c
> +++ b/tests/kms_pipe_color.c
> @@ -516,6 +516,104 @@ static void test_pipe_legacy_gamma(data_t *data,
>  	free(blue_lut);
>  }
>  
> +static drmModePropertyBlobPtr
> +get_gamma_lut_blob(data_t *data, igt_pipe_t *pipe)
> +{
> +	uint64_t prop_value;
> +	drmModePropertyPtr prop;
> +	drmModePropertyBlobPtr blob;
> +
> +	igt_assert(igt_pipe_get_property(pipe, "GAMMA_LUT",
> +					 NULL, &prop_value, &prop));
> +	igt_assert(prop->flags & DRM_MODE_PROP_BLOB);
> +
> +	blob = drmModeGetPropertyBlob(data->drm_fd, prop_value);
> +
> +	drmModeFreeProperty(prop);
> +
> +	return blob;
> +}
> +
> +/*
> + * Verify that setting the legacy gamma LUT resets the gamma LUT set
> + * through the GAMMA_LUT property.
> + */
> +static void test_pipe_legacy_gamma_reset(data_t *data,
> +					 igt_plane_t *primary)
> +{
> +	drmModeCrtc *kms_crtc;
> +	double *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;
> +
> +	gamma_zero = malloc(sizeof(double) * data->gamma_lut_size);
> +	for (i = 0; i < data->gamma_lut_size; i++)
> +		gamma_zero[i] = 0.0;
> +
> +	for_each_connected_output(&data->display, output) {
> +		igt_output_set_pipe(output, primary->pipe->pipe);
> +
> +		/* Set a gamma LUT using the property and verify the
> +		 * content of the property. */
> +		set_gamma(data, primary->pipe, gamma_zero);
> +		igt_display_commit(&data->display);
> +
> +		blob = get_gamma_lut_blob(data, primary->pipe);
> +		igt_assert(blob &&
> +			   blob->length == (sizeof(struct _drm_color_lut) *
> +					    data->gamma_lut_size));
> +
> +		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 &&
> +				   lut[i].blue == 0);
> +
> +		drmModeFreePropertyBlob(blob);
> +
> +		/* Set a gamma LUT using the legacy ioctl and verify
> +		 * the content of the GAMMA_LUT property is
> +		 * changed. */
> +		kms_crtc = drmModeGetCrtc(data->drm_fd, primary->pipe->crtc_id);
> +		legacy_lut_size = kms_crtc->gamma_size;
> +		drmModeFreeCrtc(kms_crtc);
> +
> +		red_lut = malloc(sizeof(uint16_t) * legacy_lut_size);
> +		green_lut = malloc(sizeof(uint16_t) * legacy_lut_size);
> +		blue_lut = malloc(sizeof(uint16_t) * legacy_lut_size);
> +
> +		for (i = 0; i < legacy_lut_size; i++)
> +			red_lut[i] = green_lut[i] = blue_lut[i] = 0xffff;
> +
> +		igt_assert_eq(drmModeCrtcSetGamma(data->drm_fd,
> +						  primary->pipe->crtc_id,
> +						  legacy_lut_size,
> +						  red_lut, green_lut, blue_lut),
> +			      0);
> +		igt_display_commit(&data->display);
> +
> +		blob = get_gamma_lut_blob(data, primary->pipe);
> +		igt_assert(blob &&
> +			   blob->length == (sizeof(struct _drm_color_lut) *
> +					    legacy_lut_size));
> +
> +		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 &&
> +				   lut[i].blue == 0xffff);
> +
> +		drmModeFreePropertyBlob(blob);

I think it might also be worth grabbing the degamma and CTM properties
here and making sure they've been turned off by invoking the legacy
gamma.

Aside from that, this looks good (and the other patches looked good when
I reviewed them before), so with that final check added you can consider
the test series

Reviewed-by: Matt Roper <matthew.d.roper@intel.com>



> +
> +		igt_output_set_pipe(output, PIPE_ANY);
> +	}
> +
> +	free(gamma_zero);
> +}
> +
>  /*
>   * Draw 3 rectangles using before colors with the ctm matrix apply and verify
>   * the CRC is equal to using after colors with an identify ctm matrix.
> @@ -894,6 +992,9 @@ run_tests_for_pipe(data_t *data, enum pipe p)
>  	igt_subtest_f("legacy-gamma-pipe%d", p)
>  		test_pipe_legacy_gamma(data, primary);
>  
> +	igt_subtest_f("legacy-gamma-reset-pipe%d", p)
> +		test_pipe_legacy_gamma_reset(data, primary);
> +
>  	igt_fixture {
>  		for_each_connected_output(&data->display, output)
>  			output_set_property_enum(output, "Broadcast RGB", "Full");
> -- 
> 2.7.0
> 

-- 
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx