[Intel-gfx] [PATCH] tests/kms_color:Color IGT

R, Dhanya p dhanya.p.r at intel.com
Sun Dec 13 23:43:48 PST 2015


HI Rob,

Thanks for the feedback.
Please find my comments inline.

Regards,
Dhanya
On 12/11/2015 9:05 PM, Rob Bradford wrote:
> On Fri, 2015-12-11 at 16:01 +0530, Dhanya Pillai wrote:
>> From: Dhanya <dhanya.p.r at 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 at 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)
Will fix these.
>> +#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)
Will change GEN9 -> BDW.
>
>> +#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.
Will fix this.
>> +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.
Will add these tests.
>> +struct framebuffer_color {
>> +	int red;
>> +	int green;
>> +	int blue;
>> +};
>> +struct framebuffer_color fb_color = {0,0,0};
> Maybe name this fb_color_black ?
This is just the initialization of the fb.
>> +
>> +igt_crc_t crc_reference, crc_reference_black, crc_reference_white;
>> +igt_crc_t crc_black, crc_white, crc_current;
> static ^
Will fix this.
>> +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...)
Can you clarify more.
>> +
>> +}
>> +
>> +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)
As suggested by Daniel , not adding  this check here.
>
>> +	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)
Will fix this.
>
>> +	float gamma1 = 1.0;
> Need to do more that test with a linear gamma, as that is default
> initialised in the kernel.
We are doing only negative testing for legacy gamma.
This patch is only meant for Color Manager 3.0 . If required , will add 
another patch which
does testing for legacy.
>> +	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!
Will add this.
>> +{
>> +	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?
My bad. WIll remove.
>> +		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?
Will remove this check and add for 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 at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



More information about the Intel-gfx mailing list