Re: [PATCH] tests/kms_color:Color IGT

[Rob Bradford <robert.bradford@intel.com>]

On Fri, 2015-12-11 at 16:01 +0530, Dhanya Pillai wrote:
> From: Dhanya <dhanya.p.r@intel.com>
> 
> This patch will verify color correction capability of a display
> driver.
> Gamma/CSC/De-gamma for SKL/BXT supported.
> 
> Signed-off-by: Dhanya <dhanya.p.r@intel.com>
> ---
>  tests/.gitignore       |   1 +
>  tests/Makefile.sources |   1 +
>  tests/kms_color.c      | 684
> +++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 686 insertions(+)
>  create mode 100644 tests/kms_color.c
> 
> diff --git a/tests/.gitignore b/tests/.gitignore
> index 80af9a7..58c79e2 100644
> --- a/tests/.gitignore
> +++ b/tests/.gitignore
> @@ -127,6 +127,7 @@ gen7_forcewake_mt
>  kms_3d
>  kms_addfb_basic
>  kms_atomic
> +kms_color
>  kms_crtc_background_color
>  kms_cursor_crc
>  kms_draw_crc
> diff --git a/tests/Makefile.sources b/tests/Makefile.sources
> index 8fb2de8..906c14f 100644
> --- a/tests/Makefile.sources
> +++ b/tests/Makefile.sources
> @@ -64,6 +64,7 @@ TESTS_progs_M = \
>  	gem_write_read_ring_switch \
>  	kms_addfb_basic \
>  	kms_atomic \
> +	kms_color \
>  	kms_cursor_crc \
>  	kms_draw_crc \
>  	kms_fbc_crc \
> diff --git a/tests/kms_color.c b/tests/kms_color.c
> new file mode 100644
> index 0000000..b5d199b
> --- /dev/null
> +++ b/tests/kms_color.c
> @@ -0,0 +1,684 @@
> +/*
> + * Copyright © 2015 Intel Corporation
> + *
> +  * Permission is hereby granted, free of charge, to any person
> obtaining a
> + * copy of this software and associated documentation files (the
> "Software"),
> + * to deal in the Software without restriction, including without
> limitation
> + * the rights to use, copy, modify, merge, publish, distribute,
> sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom
> the
> + * Software is furnished to do so, subject to the following
> conditions:
> + *
> + * The above copyright notice and this permission notice (including
> the next
> + * paragraph) shall be included in all copies or substantial
> portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO
> EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES
> OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
> ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
> OTHER DEALINGS
> + * IN THE SOFTWARE.
> + *
> + */
> +
> +#include <math.h>
> +#include "drmtest.h"
> +#include "drm.h"
> +#include "igt_debugfs.h"
> +#include "igt_kms.h"
> +#include "igt_core.h"
> +#include "intel_io.h"
> +#include "intel_chipset.h"
> +#include "igt_aux.h"
> +#include<unistd.h>
> +#include<stdlib.h>

There are quite a few coding style issues found by checkpatch: https://
patchwork.freedesktop.org/series/989/ perhaps you could run
checkpatch.pl locally before sending the next version?

nit: spacing & perhaps system headers should be ahead of igt headers
(check other file coding style)

> +#include <sys/ioctl.h>
> +#include <linux/types.h>
> +
> +
> +IGT_TEST_DESCRIPTION("Test Color Features at Pipe level");
> +/*
> +This tool tests the following color features:
> +	1.csc-red
> +	2.csc-green
> +	3.csc-blue
> +	4.gamma-legacy
> +	5.gamma-8bit
> +	6.gamma-10bit
> +	7.gamma-12bit
> +	8.gamma-split
> +
> +Verification is done by CRC checks.
> +
> +*/
> +
> +#define CSC_MAX_VALS    9
> +#define GEN9_SPLITGAMMA_MAX_VALS                512
> +#define GEN9_8BIT_GAMMA_MAX_VALS                256
> +#define GEN9_10BIT_GAMMA_MAX_VALS               1024
> +#define GEN9_12BIT_GAMMA_MAX_VALS               513
> +#define GEN9_MAX_GAMMA                         ((1 << 24) - 1)
> +#define GEN9_MIN_GAMMA				0

GEN9? this functionality is currently enabled for BDW onwards (but I
think in theory could be enabled for HSW)

> +#define RED_CSC 0
> +#define GREEN_CSC 1
> +#define BLUE_CSC 2
> +#define RED_FB 0
> +#define GREEN_FB 1
> +#define BLUE_FB 2
> +
> +struct _drm_r32g32b32 {
> +	__u32 r32;
> +	__u32 g32;
> +	__u32 b32;
> +	__u32 reserved;
> +};
> +
> +struct _drm_palette {
> +	struct _drm_r32g32b32 lut[0];
> +};
> +
> +struct _drm_ctm {
> +	__s64 ctm_coeff[9];
> +};
> +

Surround these structure definitions with #ifndefs so that this
continues to work when librdrm gets updated.

> +float ctm_red[9] = {1, 1, 1, 0, 0, 0, 0, 0, 0};
> +float ctm_green[9] = {0, 0, 0, 1, 1, 1, 0, 0, 0};
> +float ctm_blue[9] = {0, 0, 0, 0, 0, 0, 1, 1, 1};
> +float ctm_unity[9] = {1, 0, 0, 0, 1, 0, 0, 0, 1};

These are only used in this file, and never changed: -> static const.

s/unity/identity/ (I think unity is the wrong terminology here.)

I'd like to see versions of the CTM tests that:

a.) Test with negative coefficients
b.) With fractional coeffecients (on BDW the the coefficients are
stored in a funky pseudo fp format. Testing some of those are essential
(testing all even better.)
c.) Swapping red with green would be a good test too.

> +struct framebuffer_color {
> +	int red;
> +	int green;
> +	int blue;
> +};
> +struct framebuffer_color fb_color = {0,0,0};

Maybe name this fb_color_black ?

> +
> +igt_crc_t crc_reference, crc_reference_black, crc_reference_white;
> +igt_crc_t crc_black, crc_white, crc_current;

static ^

> +struct data_t {
> +	int fb_initial;
> +	int drm_fd;
> +	int gen;
> +	int w, h;
> +	igt_display_t display;
> +	struct igt_fb fb_prep;
> +	struct igt_fb fb, fb1;
> +        igt_pipe_crc_t *pipe_crc;
> +	enum pipe pipe;
> +
> +};
> +
> +
> +static int create_blob(int fd, uint64_t *data, int length)
> +{
> +	struct drm_mode_create_blob blob;
> +	int ret = -1;
> +
> +	blob.data = (uint64_t)data;
> +	blob.length = length;
> +	blob.blob_id = -1;
> +	ret = ioctl(fd, DRM_IOCTL_MODE_CREATEPROPBLOB, &blob);
> +	if (!ret)
> +		return blob.blob_id;
> +	igt_fail(IGT_EXIT_FAILURE);
> +	return ret;
> +}
> +
> +static void prepare_crtc(struct data_t *data, igt_output_t *output,
> +			 enum pipe pipe1, igt_plane_t *plane,
> drmModeModeInfo *mode,
> +			 enum igt_commit_style s)
> +{
> +	igt_display_t *display = &data->display;
> +
> +	igt_output_set_pipe(output, pipe1);
> +	
> +	igt_pipe_crc_free(data->pipe_crc);
> +        data->pipe_crc = igt_pipe_crc_new(pipe1,
> INTEL_PIPE_CRC_SOURCE_AUTO);
> +	
> +	/* before allocating, free if any older fb */
> +	if (data->fb_initial) {
> +		igt_remove_fb(data->drm_fd, &data->fb_prep);
> +		data->fb_initial = 0;
> +	}
> +
> +	/* allocate fb for plane 1 */
> +	data->fb_initial = igt_create_color_fb(data->drm_fd,
> +					       mode->hdisplay, mode-
> >vdisplay,
> +					       DRM_FORMAT_XRGB8888,
> +					       LOCAL_I915_FORMAT_MOD
> _X_TILED, /* tiled */
> +					       fb_color.red,
> fb_color.green, fb_color.blue,
> +					       &data->fb_prep);
> +	igt_assert(data->fb_initial);
> +
> +	igt_plane_set_fb(plane, &data->fb_prep);
> +
> +	igt_display_commit2(display, s);
> +}
> +
> +static void cleanup_fb(struct data_t *data)
> +{
> +
> +	if (data->fb_initial) {
> +		igt_remove_fb(data->drm_fd, &data->fb_prep);
> +		data->fb_initial = 0;
> +	}
> +	if (data->fb.fb_id)
> +		igt_remove_fb(data->drm_fd, &data->fb);
> +
> +}
> +
> +static void cleanup_crtc(struct data_t *data, igt_output_t *output,
> +			 igt_plane_t *plane)
> +{
> +	igt_display_t *display = &data->display;
> +
> +	if (!plane->is_primary) {
> +		igt_plane_t *primary;
> +
> +		primary = igt_output_get_plane(output,
> IGT_PLANE_PRIMARY);
> +		igt_plane_set_fb(primary, NULL);
> +	}
> +
> +	igt_plane_set_fb(plane, NULL);
> +	igt_output_set_pipe(output, PIPE_ANY);
> +> 	igt_display_commit2(display, COMMIT_UNIVERSAL);
> +
> +}
> +
> +static int get_color_property(int drm_fd, int id, int object,const
> char *prop_name,
> +			      int blob_id)
> +{
> +	int i = 0, ret = 0;
> +	drmModeObjectPropertiesPtr props = NULL;
> +	igt_fail_on_f(id < 0 || ((object != DRM_MODE_OBJECT_CRTC) &&
> +		      (object != DRM_MODE_OBJECT_PLANE)),
> +		      "Invalid input to get color property %d", id);
> +
> +	props = drmModeObjectGetProperties(drm_fd, id, object);
> +	igt_fail_on_f(!props,
> +		     "\nNo property for object id=%d\n", id);
> +	for (i = 0; i < props->count_props; i++) {
> +		drmModePropertyPtr prop = drmModeGetProperty(drm_fd,
> +							     props-
> >props[i]);
> +		if (strcmp(prop->name, prop_name) == 0) {
> +			blob_id = props->prop_values[i];
> +			break;
> +		}
> +		drmModeFreeProperty(prop);
> +	}
> +
> +	ret = blob_id;
> +	drmModeFreeObjectProperties(props);
> +	igt_fail_on_f(i == props->count_props,
> +		      "%s No such property\n", prop_name);
> +	return ret;
> +}
> +
> +
> +static int set_color_property(int drm_fd, int id, int object, const
> char *prop_name, int blob_id)
> +{
> +	int i = 0, res;
> +
> +	drmModeObjectPropertiesPtr props = NULL;
> +	igt_fail_on_f(id < 0 || ((object != DRM_MODE_OBJECT_CRTC) &&
> +                      (object != DRM_MODE_OBJECT_PLANE)),
> +                      "Invalid input to get color property %d", id);
> +
> +
> +	props = drmModeObjectGetProperties(drm_fd, id, object);
> +        igt_fail_on_f(!props,
> +                     "\nNo property for object id=%d\n", id);
> +	for (i = 0; i < props->count_props; i++) {
> +		drmModePropertyPtr prop = drmModeGetProperty(drm_fd,
> +							     props-
> >props[i]);
> +		if (strcmp(prop->name, prop_name) == 0) {
> +			res = drmModeObjectSetProperty(drm_fd, id,
> object,
> +						       (uint32_t)pro
> p->prop_id, blob_id);
> +
> +			if (res) {
> +				drmModeFreeProperty(prop);
> +				drmModeFreeObjectProperties(props);
> +			} else {
> +				drmModeFreeProperty(prop);
> +				break;
> +			}
> +		}
> +
> +		drmModeFreeProperty(prop);
> +	}
> +	drmModeFreeObjectProperties(props);
> +        igt_fail_on_f(i == props->count_props,
> +                      "%s No such property\n", prop_name);
> +	return 0;
> +}
> +
> +static int64_t  convertFloatToBinary(double input)
> +{
> +
> +	int integer_part, count = 0;
> +	uint32_t temp_ip, frac_val = 0x00000000;
> +	uint64_t integer_val = 0x00000000;
> +	int64_t  value = 0x0000000000000000;
> +	float fractional_part, ip;
> +	integer_part = (int)input;
> +	fractional_part = input - integer_part;
> +	while (fractional_part != 0.000000) {
> +		ip = fractional_part * 16;
> +		temp_ip = (int)(fractional_part * 16);
> +		frac_val = frac_val | (temp_ip << (28 - count * 4));
> +		count++;
> +		fractional_part = ip - temp_ip;
> +	}
> +	integer_val = integer_part;
> +	value = value | (integer_val << 32);
> +	value = value | frac_val;
> +	return value;
> +

Given the truncation by the HW, I think you can replace this with input
* (1ull << 32). (I think...)

> +
> +}
> +
> +static void write_gamma_lut( uint32_t num_samples, struct
> _drm_r32g32b32 *gamma_ptr, int unit_gamma )
> +{
> +	unsigned short Red, Green, Blue;
> +	uint32_t r32, b32, g32;
> +	uint64_t i,  val;
> +
> +	for (i = 0; i < num_samples; i++) {
> +		if(unit_gamma == 0) {
> +		Blue = GEN9_MAX_GAMMA;
> +		Green = GEN9_MAX_GAMMA;
> +		Red = GEN9_MAX_GAMMA;
> +		} else {
> +		Blue = GEN9_MIN_GAMMA;
> +		Green = GEN9_MIN_GAMMA;
> +		Red = GEN9_MIN_GAMMA;
> +		}
> +		r32 = Red;
> +		g32 = Green;
> +		b32 = Blue;
> +		r32 <<=8;
> +		g32 <<=8;
> +		b32 <<=8;
> +		gamma_ptr[i].r32 = r32;
> +		gamma_ptr[i].g32 = g32;
> +		gamma_ptr[i].b32 = b32;
> +	}
> +}
> +
> +static uint64_t get_blob(int fd, int blob_id, int length)
> +{
> +	struct drm_mode_get_blob blob;
> +	int ret = 0;
> +
> +	blob.blob_id = blob_id;
> +	blob.length = length;
> +	blob.data =  (uint64_t)malloc(blob.length);
> +	ret = ioctl(fd, DRM_IOCTL_MODE_GETPROPBLOB, &blob);
> +
> +	if (ret)
> +		igt_info("GET BLOB Failed\n");
> +
> +	return blob.data;
> +}
> +
> +static void enable_plane(struct data_t *data, igt_display_t
> *display, igt_output_t *output , int pipe1)
> +{
> +	enum igt_commit_style commit;
> +	enum pipe pipe2;
> +        int width, height;
> +        uint32_t pixelformat = DRM_FORMAT_XRGB8888;
> +
> +	commit = COMMIT_UNIVERSAL;
> +	for_each_connected_output(display, output) {
> +		drmModeModeInfo *mode;
> +		igt_plane_t *plane;
> +		pipe2 = output->config.pipe;
> +
> +		if (pipe2 != pipe1)
> +			break;
> +		igt_output_set_pipe(output, pipe2);
> +		mode = igt_output_get_mode(output);
> +		/*Draw the initial primary plane*/
> +		plane = igt_output_get_plane(output,
> IGT_PLANE_PRIMARY);
> +
> +		prepare_crtc(data, output, pipe2, plane, mode,
> commit);
> +		/*Random Size  Buffer Creation */
> +		width = 600;
> +		height = 600;
> +
> +		plane = igt_output_get_plane(output, IGT_PLANE_2);
> +		igt_create_color_fb(data->drm_fd,
> +					    width, height,
> +					    pixelformat,
> +					    LOCAL_DRM_FORMAT_MOD_NON
> E,
> +					    fb_color.red,fb_color.gr
> een,fb_color.blue,
> +					    &data->fb);
> +		igt_plane_set_position(plane, 100, 100);
> +
> +		igt_plane_set_fb(plane, &data->fb);
> +		igt_display_commit2(display, commit);
> +	}
> +
> +}
> +/*
> +This function verify csc feature using both CRC check.
> +Steps followed are:
> +	1.Enable plane for capturing reference CRC
> +	2.Capture Reference CRC
> +	3.Apply CSC on Pipe
> +	4.Enable Plane .CSC will be applied on this planes.
> +	5.Capture CRC and compare with reference CRC
> +	6. Register Validation for CRC
> +*/
> +static void test_pipe_csc(struct data_t *data, igt_display_t
> *display, igt_output_t *output,
> +			  igt_plane_t *plane, bool enable, int
> value, int pipe1)
> +{
> +
> +	struct _drm_ctm *ctm_data = NULL;
> +	bool ret = false;
> +	int res, i, blob_id;
> +	float *ctm;
> +	uint64_t blob_address;
> +	ctm_data = (struct _drm_ctm *)
> +		malloc(sizeof(struct _drm_ctm));
> +	if (!enable) {
> +		ctm = ctm_unity;
> +	} else {
> +		if (value == RED_CSC) {
> +			ctm = ctm_red;
> +		} else if (value == GREEN_CSC) {
> +			ctm = ctm_green;
> +		} else if (value == BLUE_CSC) {
> +			ctm = ctm_blue;
> +		} else{
> +			ctm = ctm_unity;
> +		} }
> +	if(value == RED_FB) {
> +		fb_color.red = 1;
> +		fb_color.green =0;
> +		fb_color.blue = 0;
> +	} else  if(value == GREEN_FB) {
> +		fb_color.red = 0;
> +		fb_color.green = 1;
> +		fb_color.blue = 0;
> +	} else if(value == BLUE_FB) {
> +		fb_color.red = 0;
> +		fb_color.green = 0;
> +		fb_color.blue = 1;
> +	} else {
> +		fb_color.red = 1;
> +		fb_color.green = 1;
> +		fb_color.blue = 1;
> +	}
> +	/*Enable planes and capture reference crc*/
> +	enable_plane(data, display, output, pipe1);
> +	igt_pipe_crc_collect_crc(data->pipe_crc, &crc_reference);
> +
> +	fb_color.red = 1;
> +	fb_color.green = 1;
> +	fb_color.blue = 1;
> +
> +	enable_plane(data, display, output, pipe1);
> +
> +	for (i = 0; i < CSC_MAX_VALS; i++) {
> +		ctm_data->ctm_coeff[i] = convertFloatToBinary(*(ctm
> + i));
> +	}
> +
> +	blob_id = create_blob(display->drm_fd,
> +			(int *)(ctm_data), sizeof(struct _drm_ctm));
> +	igt_fail_on_f (blob_id < 0, "CTM:BLOB IOCTL Fail\n");
> +	res = set_color_property(display->drm_fd, output-
> >config.crtc->crtc_id,
> +			DRM_MODE_OBJECT_CRTC, "CTM", blob_id);
> +	igt_fail_on_f (res < 0, "CTM:Set Property Failed\n");
> +	res = get_color_property(display->drm_fd, output-
> >config.crtc->crtc_id, DRM_MODE_OBJECT_CRTC, "CTM", blob_id);
> +	igt_fail_on_f (res < 0, "CTM:Get Property Failed\n");
> +	
> +	blob_address = get_blob(display->drm_fd, res, sizeof(struct
> _drm_ctm));
> +	ctm_data = (struct _drm_ctm *) (intptr_t) blob_address;
> +
> +	enable_plane(data, display, output, pipe1);
> +	igt_pipe_crc_collect_crc(data->pipe_crc, &crc_current);
> +	igt_assert_crc_equal(&crc_reference, &crc_current);
> +	/*Restoring the Unit CSC*/
> +	ctm = ctm_unity;
> +	for (i = 0; i < CSC_MAX_VALS; i++) {
> +		ctm_data->ctm_coeff[i] = convertFloatToBinary(*(ctm
> + i));
> +	}
> +
> +	blob_id = create_blob(display->drm_fd,
> +			(int *)(ctm_data), sizeof(struct _drm_ctm));
> +        igt_fail_on_f (blob_id < 0, "CTM:BLOB IOCTL Fail\n");
> +
> +	res = set_color_property(display->drm_fd, output-
> >config.crtc->crtc_id,
> +			DRM_MODE_OBJECT_CRTC, "CTM", blob_id);
> +        igt_fail_on_f (res < 0, "CTM:Set Property Failed\n");
> +
> +	free(ctm_data);
> +
> +}
> +
> +
> +static void apply_gamma(struct data_t *data, igt_display_t *display,
> igt_output_t *output, int pipe1, uint32_t num_samples, int
> unit_gamma) 
> +{
> +	struct _drm_palette *gamma_data =  NULL;
> +	int  ret, res, blob_id, blob_length;
> +	bool status = false;
> +
> +
> +	if (num_samples == 0)
> +		blob_length = 1;
> +	else
> +		blob_length = (sizeof(struct _drm_palette) +
> (num_samples * sizeof(struct _drm_r32g32b32)));
> +
> +	gamma_data = malloc(blob_length);
> +	write_gamma_lut(num_samples, gamma_data->lut, unit_gamma);
> +	blob_id = create_blob(display->drm_fd, (uint64_t
> *)(gamma_data), blob_length);
> +	if (blob_id < 0)
> +		free(gamma_data);
> +
> +        igt_fail_on_f (blob_id < 0, "GAMMA:BLOB IOCTL Fail\n");
> +	/*Disabling degamma*/
> +	if(unit_gamma == 2 ) {
> +		res = set_color_property(display->drm_fd, output-
> >config.crtc->crtc_id,
> +                        DRM_MODE_OBJECT_CRTC, "PALETTE_BEFORE_CTM",
> blob_id);
> +                ret = get_color_property(display->drm_fd, output-
> >config.crtc->crtc_id,
> +                        DRM_MODE_OBJECT_CRTC, "PALETTE_BEFORE_CTM",
> blob_id);
> +                igt_fail_on_f (res < 0, "PALETTE_BEFORE_CTM:Set
> Property Failed\n");
> +                igt_fail_on_f (ret < 0, "PALETTE_BEFORE_CTM:Set
> Property Failed\n");
> +
> +	}
> +
> +	if (num_samples == GEN9_SPLITGAMMA_MAX_VALS) {
> +		res = set_color_property(display->drm_fd, output-
> >config.crtc->crtc_id,
> +                        DRM_MODE_OBJECT_CRTC, "PALETTE_BEFORE_CTM",
> blob_id);
> +		ret = get_color_property(display->drm_fd, output-
> >config.crtc->crtc_id,
> +                        DRM_MODE_OBJECT_CRTC, "PALETTE_BEFORE_CTM",
> blob_id);
> +		if (res < 0)
> +			free(gamma_data);
> +	        igt_fail_on_f (res < 0, "PALETTE_BEFORE_CTM:Set
> Property Failed\n");
> +                igt_fail_on_f (ret < 0, "PALETTE_BEFORE_CTM:Set
> Property Failed\n");
> +
> +	}
> +
> +	res = set_color_property(display->drm_fd, output-
> >config.crtc->crtc_id,
> +			DRM_MODE_OBJECT_CRTC, "PALETTE_AFTER_CTM",
> blob_id);
> +	if (res < 0) {
> +		free(gamma_data);
> +	}
> +        igt_fail_on_f (res < 0, "PALETTE_AFTER_CTM:Set Property
> Failed\n");
> +
> +	ret = get_color_property(display->drm_fd, output-
> >config.crtc->crtc_id,
> +			DRM_MODE_OBJECT_CRTC, "PALETTE_AFTER_CTM",
> blob_id);
> +	if (ret < 0) {
> +		free(gamma_data);
> +	}
> +        igt_fail_on_f (ret < 0, "PALETTE_BEFORE_CTM:Set Property
> Failed\n");
> +
> +	free(gamma_data);
> +
> +
> +}
> +/*
> +This function verify csc feature using both CRC check.
> +Steps followed are:
> +        1.Enable Black plane and capture CRC_REFERENCE_BLACK.
> +	2.Enable White plane and capture CRS_REFERENCE_WHITE
> +        3.Enable Red plane.
> +        4.Apply LOW Gamma.
> +        5.Capture CRC_Black.
> +        6.Apply High Gamma and capture CRC_White.
> +        7.CRC_Black should be equivalent to CRC_REFERENCE_BLACK
> +        8.CRC_White should be equivalent to CRC_REFERENCE_WHite.
> +
> +
> +*/
> +static void test_pipe_gamma(struct data_t *data, igt_display_t
> *display, igt_output_t *output,
> +		            uint32_t num_samples, int values, int
> pipe1)
> +{
> +	int unit_gamma, disable_gamma;
> +	igt_plane_t *plane;
> +
> +	/*Enable black planes and capture reference crc*/
> +
> +	fb_color.red = 0;
> +	fb_color.green =0;
> +	fb_color.blue = 0;
> +	enable_plane(data, display, output, pipe1);
> +	igt_pipe_crc_collect_crc(data->pipe_crc,
> &crc_reference_black);
> +	/*Enable white plane and capture refernce crc*/
> +	fb_color.red = 1;
> +	fb_color.green =1;
> +	fb_color.blue = 1;
> +	enable_plane(data, display, output, pipe1);
> +	igt_pipe_crc_collect_crc(data->pipe_crc,
> &crc_reference_white);
> +
> +	/*Enable red planes and apply unit gamma*/
> +	fb_color.red = 1;
> +	fb_color.green =0;
> +	fb_color.blue = 0;
> +	unit_gamma = 0; /*0 -> white 1->black*/

Take the CRC here too, as reference_red and check that crc_white !=
reference_red. (i.e. check its changed)

> +	apply_gamma(data, display, output, pipe1, num_samples,
> unit_gamma);
> +	enable_plane(data, display, output, pipe1);
> +	igt_pipe_crc_collect_crc(data->pipe_crc, &crc_white);
> +
> +	/* Apply 0x0 gamma */
> +	unit_gamma =  1;
> +
> +	apply_gamma(data, display, output, pipe1, num_samples,
> unit_gamma);
> +	enable_plane(data, display, output, pipe1);
> +	igt_pipe_crc_collect_crc(data->pipe_crc, &crc_black);
> +	igt_assert_crc_equal(&crc_reference_black, &crc_black);
> +	igt_assert_crc_equal(&crc_reference_white, &crc_white);
> +
> +	/*Disabling gamma*/
> +	if( num_samples == GEN9_SPLITGAMMA_MAX_VALS) {
> +		disable_gamma = 2;
> +		apply_gamma(data, display, output, pipe1, 0,
> disable_gamma);
> +	} else {
> +                apply_gamma(data, display, output, pipe1, 0,
> unit_gamma);
> +	}
> +	
> +
> +
> +
> +}
> +
> +static void test_legacy_gamma (struct data_t *data, igt_display_t
> *display, igt_output_t *output, int pipe1)
> +{
> +	int fd,ret,i;
> +
> +	uint32_t crtc;
> +	uint16_t *val = malloc(sizeof(uint16_t) * 256);
> +        igt_plane_t *plane;
> +
> +        fb_color.red = 1;
> +        fb_color.green =1;
> +        fb_color.blue = 1;
> +        enable_plane(data, display, output, pipe1);

nit: coding style (spacing)


> +	float gamma1 = 1.0;

Need to do more that test with a linear gamma, as that is default
initialised in the kernel.

> +	for(i = 0; i < 256 ; i++) {
> +		val[i] = 0xffff * pow((i/255), gamma1);
> +
> +	}
> +	for_each_connected_output (display, output) {
> +		ret = drmModeCrtcSetGamma (display->drm_fd, output-
> >config.crtc->crtc_id, 256, val, val, val);
> +	        igt_assert_lte(0, ret);

Also, need to do more that check return value. I recommend taking the
CRC with 0xff red FB. Then mapping 0xff red (last entry) to 0xf0 and
then comparing. And the same with other channels and more colours.


> +	}
> +}
> +
> +static void test_pipe_color(struct data_t *data, const  char
> *prop_name, int enable, int value)

Replace prop_name with an enum. Then you can replace all the strcmp()s
with a switch!

> +{
> +	igt_display_t *display = &data->display;
> +	igt_output_t *output;
> +	igt_plane_t *plane;
> +	enum pipe pipe1;
> +
> +	for_each_connected_output(display, output) {
> +		pipe1 = output->config.pipe;
> +		igt_output_set_pipe(output,pipe1);
> +		if ((strcmp(prop_name, "gamma_property_8bit") == 0)
> &&
> +				enable == true){
> +			test_pipe_gamma(data, display, output,
> +					GEN9_8BIT_GAMMA_MAX_VALS,
> value, pipe1);
> +		} else if ((strcmp(prop_name, "legacy_gamma") ==
> 0)){
> +			test_legacy_gamma(data, display, output,
> pipe1);
> +		} else  if ((strcmp(prop_name,
> "gamma_property_10bit") == 0) &&
> +				enable == true) {
> +			test_pipe_gamma(data, display, output,
> GEN9_10BIT_GAMMA_MAX_VALS, value, pipe1);
> +		} else  if ((strcmp(prop_name, "csc_property") ==
> 0)) {
> +			test_pipe_csc(data, display, output, plane,
> enable, value, pipe1);
> +		} else  if ((strcmp(prop_name,
> "gamma_property_split") == 0) &&
> +				enable == true) {
> +			test_pipe_gamma(data, display, output,
> +					GEN9_SPLITGAMMA_MAX_VALS,
> value, pipe1);
> +		} else  if ((strcmp(prop_name,
> "gamma_property_12bit") == 0) && enable == true) {
> +			test_pipe_gamma(data, display, output,
> +					GEN9_12BIT_GAMMA_MAX_VALS,
> value, pipe1);
> +		} else {
> +			igt_info("Invalid Test\n");
> +		}
> +		cleanup_fb(data);
> +		plane = igt_output_get_plane(output, IGT_PLANE_2);
> +		cleanup_crtc(data, output, plane);
> +	}
> +}
> +
> +igt_main
> +{
> +	struct data_t data = {};
> +	int values = 1;
> +	igt_skip_on_simulation();
> +	igt_fixture{
> +		//                data.drm_fd =
> drm_open_driver_master(DRIVER_INTEL);

Left over debugging comment?

> +		data.drm_fd = drm_open_any_master();
> +		kmstest_set_vt_graphics_mode();
> +		igt_display_init(&data.display, data.drm_fd);
> +		data.gen =
> intel_gen(intel_get_drm_devid(data.drm_fd));
> +		igt_require(data.gen >= 9);

9? This feature is enabled from BDW onwards atm, but is possible on
HSW. Can we instead check for the CTM property?

> +	}
> +	igt_subtest_f("csc-red")
> +		test_pipe_color(&data, "csc_property", true,
> RED_CSC);
> +	igt_subtest_f("csc-green")
> +		test_pipe_color(&data, "csc_property", true,
> GREEN_CSC);
> +	igt_subtest_f("csc-blue")
> +		test_pipe_color(&data, "csc_property", true,
> BLUE_CSC);
> +	igt_subtest_f("gamma-legacy")
> +                test_pipe_color(&data, "legacy_gamma", true,
> values);
> +
> +	igt_subtest_f("gamma-8bit")
> +		test_pipe_color(&data, "gamma_property_8bit", true,
> values);
> +	igt_subtest_f("gamma-10bit")
> +		test_pipe_color(&data, "gamma_property_10bit", true,
> values);
> +	igt_subtest_f("gamma-12bit")
> +		test_pipe_color(&data, "gamma_property_12bit",
> true,  values);
> +	igt_subtest_f("gamma-split")
> +		test_pipe_color(&data, "gamma_property_split",
> true,  values);
> +
> +	igt_fixture{
> +		igt_display_fini(&data.display);
> +	}
> +}
> +
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx