diff mbox series

[i-g-t,07/14] tests/kms_color: New negative tests for plane level color mgmt

Message ID 20211115094759.520955-8-bhanuprakash.modem@intel.com (mailing list archive)
State New, archived
Headers show
Series Add IGT support for plane color management | expand

Commit Message

Modem, Bhanuprakash Nov. 15, 2021, 9:47 a.m. UTC 
Negative check for:
 * plane gamma lut sizes
 * plane degamma lut sizes
 * plane ctm matrix sizes

Cc: Harry Wentland <harry.wentland@amd.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
Cc: Uma Shankar <uma.shankar@intel.com>
Signed-off-by: Bhanuprakash Modem <bhanuprakash.modem@intel.com>
---
 tests/kms_color.c | 127 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 127 insertions(+)

Comments

Pekka Paalanen Nov. 18, 2021, 9:19 a.m. UTC | #1 
On Mon, 15 Nov 2021 15:17:52 +0530
Bhanuprakash Modem <bhanuprakash.modem@intel.com> wrote:

> Negative check for:
>  * plane gamma lut sizes
>  * plane degamma lut sizes
>  * plane ctm matrix sizes
> 
> Cc: Harry Wentland <harry.wentland@amd.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
> Cc: Uma Shankar <uma.shankar@intel.com>
> Signed-off-by: Bhanuprakash Modem <bhanuprakash.modem@intel.com>
> ---
>  tests/kms_color.c | 127 ++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 127 insertions(+)
> 
> diff --git a/tests/kms_color.c b/tests/kms_color.c
> index e14b37cb6f..d9fe417ba9 100644
> --- a/tests/kms_color.c
> +++ b/tests/kms_color.c
> @@ -736,6 +736,118 @@ static void test_pipe_limited_range_ctm(data_t *data,
>  }
>  #endif
>  
> +static bool invalid_plane_gamma_test(data_t *data, igt_plane_t *plane)
> +{
> +	igt_display_t *display = &data->display;
> +	drmModePropertyPtr gamma_mode = NULL;
> +	uint32_t i;
> +
> +	igt_info("Plane invalid gamma test is running on pipe-%s plane-%s(%s)\n",
> +			kmstest_pipe_name(plane->pipe->pipe),
> +			kmstest_plane_type_name(plane->type),
> +			is_hdr_plane(plane) ? "hdr":"sdr");
> +
> +	igt_require(igt_plane_has_prop(plane, IGT_PLANE_GAMMA_MODE));
> +	igt_require(igt_plane_has_prop(plane, IGT_PLANE_GAMMA_LUT));
> +
> +	gamma_mode = get_plane_gamma_degamma_mode(plane, IGT_PLANE_GAMMA_MODE);
> +
> +	/* Iterate all supported gamma modes. */
> +	for (i = 0; i < gamma_mode->count_enums; i++) {
> +		segment_data_t *segment_info = NULL;
> +		size_t lut_size = 0;
> +
> +		/* Ignore 'no gamma' from enum list. */
> +		if (!strcmp(gamma_mode->enums[i].name, "no gamma"))
> +			continue;
> +
> +		igt_info("Trying to use gamma mode: \'%s\'\n", gamma_mode->enums[i].name);
> +
> +		segment_info = get_segment_data(data, gamma_mode->enums[i].value,
> +				gamma_mode->enums[i].name);
> +		lut_size = sizeof(struct drm_color_lut_ext) * segment_info->entries_count;
> +
> +		igt_plane_set_prop_enum(plane, IGT_PLANE_GAMMA_MODE, gamma_mode->enums[i].name);
> +		invalid_plane_lut_sizes(display, plane,
> +					IGT_PLANE_GAMMA_LUT,
> +					lut_size);
> +
> +		clear_segment_data(segment_info);
> +
> +		/* One enum is enough. */
> +		break;
> +	}
> +
> +	drmModeFreeProperty(gamma_mode);
> +
> +	return true;
> +}
> +
> +static bool invalid_plane_degamma_test(data_t *data, igt_plane_t *plane)
> +{
> +	igt_display_t *display = &data->display;
> +	drmModePropertyPtr degamma_mode = NULL;
> +	uint32_t i;
> +
> +	igt_info("Plane invalid degamma test is running on pipe-%s plane-%s(%s)\n",
> +			kmstest_pipe_name(plane->pipe->pipe),
> +			kmstest_plane_type_name(plane->type),
> +			is_hdr_plane(plane) ? "hdr":"sdr");
> +
> +	igt_require(igt_plane_has_prop(plane, IGT_PLANE_DEGAMMA_MODE));
> +	igt_require(igt_plane_has_prop(plane, IGT_PLANE_DEGAMMA_LUT));
> +
> +	degamma_mode = get_plane_gamma_degamma_mode(plane, IGT_PLANE_DEGAMMA_MODE);
> +
> +	/* Iterate all supported degamma modes. */
> +	for (i = 0; i < degamma_mode->count_enums; i++) {
> +		segment_data_t *segment_info = NULL;
> +		size_t lut_size = 0;
> +
> +		/* Ignore 'no degamma' from enum list. */
> +		if (!strcmp(degamma_mode->enums[i].name, "no degamma"))
> +			continue;
> +
> +		igt_info("Trying to use degamma mode: \'%s\'\n", degamma_mode->enums[i].name);
> +
> +		segment_info = get_segment_data(data,
> +						degamma_mode->enums[i].value,
> +						degamma_mode->enums[i].name);
> +		lut_size = sizeof(struct drm_color_lut_ext) * segment_info->entries_count * 2;
> +
> +		igt_plane_set_prop_enum(plane,
> +					IGT_PLANE_DEGAMMA_MODE,
> +					degamma_mode->enums[i].name);
> +		invalid_plane_lut_sizes(display, plane,
> +					IGT_PLANE_DEGAMMA_LUT,
> +					lut_size);
> +
> +		clear_segment_data(segment_info);
> +
> +		/* One enum is enough. */
> +		break;

Why is one enum enough?

The same question for the other case in this patch.


> +	}
> +
> +	drmModeFreeProperty(degamma_mode);
> +
> +	return true;
> +}
> +
> +static bool invalid_plane_ctm_test(data_t *data, igt_plane_t *plane)
> +{
> +	igt_info("Plane invalid CTM test is running on pipe-%s plane-%s(%s)\n",
> +			kmstest_pipe_name(plane->pipe->pipe),
> +			kmstest_plane_type_name(plane->type),
> +			is_hdr_plane(plane) ? "hdr":"sdr");
> +
> +	igt_require(igt_plane_has_prop(plane, IGT_PLANE_CTM));
> +	invalid_plane_lut_sizes(&data->display, plane,
> +				IGT_PLANE_CTM,
> +				sizeof(struct drm_color_ctm));

The code says you're trying shove a LUT into a CTM blob. I understand
that mechanically this is test you want to do, feed a wrong sized blob,
and in this case the contents do not matter (unlike with actual LUTs),
but reading this code is completely misleading and does not make sense.
It takes a while to think about what you actually want to test here,
and then reverse-engineer the code to understand that that is what
actually happens, too. That is too much mental burden for the reader to
realize that this piece of code actually works.


Thanks,
pq

> +
> +	return true;
> +}
> +
>  static bool plane_gamma_test(data_t *data, igt_plane_t *plane)
>  {
>  	igt_output_t *output;
> @@ -1411,6 +1523,21 @@ static void run_tests_for_plane(data_t *data, enum pipe pipe)
>  					ctm_tests[i].iter);
>  		}
>  	}
> +
> +	igt_describe("Negative check for invalid plane gamma lut sizes");
> +	igt_subtest_f("pipe-%s-invalid-plane-gamma-lut-sizes",
> +			kmstest_pipe_name(pipe))
> +		run_plane_color_test(data, pipe, invalid_plane_gamma_test);
> +
> +	igt_describe("Negative check for invalid plane degamma lut sizes");
> +	igt_subtest_f("pipe-%s-invalid-plane-degamma-lut-sizes",
> +			kmstest_pipe_name(pipe))
> +		run_plane_color_test(data, pipe, invalid_plane_degamma_test);
> +
> +	igt_describe("Negative check for invalid plane ctm matrix sizes");
> +	igt_subtest_f("pipe-%s-invalid-plane-ctm-matrix-sizes",
> +			kmstest_pipe_name(pipe))
> +		run_plane_color_test(data, pipe, invalid_plane_ctm_test);
>  }
>  
>  igt_main
Harry Wentland Nov. 29, 2021, 2:56 p.m. UTC | #2 
On 2021-11-18 04:19, Pekka Paalanen wrote:
> On Mon, 15 Nov 2021 15:17:52 +0530
> Bhanuprakash Modem <bhanuprakash.modem@intel.com> wrote:
> 
>> Negative check for:
>>  * plane gamma lut sizes
>>  * plane degamma lut sizes
>>  * plane ctm matrix sizes
>>
>> Cc: Harry Wentland <harry.wentland@amd.com>
>> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> Cc: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
>> Cc: Uma Shankar <uma.shankar@intel.com>
>> Signed-off-by: Bhanuprakash Modem <bhanuprakash.modem@intel.com>
>> ---
>>  tests/kms_color.c | 127 ++++++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 127 insertions(+)
>>
>> diff --git a/tests/kms_color.c b/tests/kms_color.c
>> index e14b37cb6f..d9fe417ba9 100644
>> --- a/tests/kms_color.c
>> +++ b/tests/kms_color.c
>> @@ -736,6 +736,118 @@ static void test_pipe_limited_range_ctm(data_t *data,
>>  }
>>  #endif
>>  
>> +static bool invalid_plane_gamma_test(data_t *data, igt_plane_t *plane)
>> +{
>> +	igt_display_t *display = &data->display;
>> +	drmModePropertyPtr gamma_mode = NULL;
>> +	uint32_t i;
>> +
>> +	igt_info("Plane invalid gamma test is running on pipe-%s plane-%s(%s)\n",
>> +			kmstest_pipe_name(plane->pipe->pipe),
>> +			kmstest_plane_type_name(plane->type),
>> +			is_hdr_plane(plane) ? "hdr":"sdr");
>> +
>> +	igt_require(igt_plane_has_prop(plane, IGT_PLANE_GAMMA_MODE));
>> +	igt_require(igt_plane_has_prop(plane, IGT_PLANE_GAMMA_LUT));
>> +
>> +	gamma_mode = get_plane_gamma_degamma_mode(plane, IGT_PLANE_GAMMA_MODE);
>> +
>> +	/* Iterate all supported gamma modes. */
>> +	for (i = 0; i < gamma_mode->count_enums; i++) {
>> +		segment_data_t *segment_info = NULL;
>> +		size_t lut_size = 0;
>> +
>> +		/* Ignore 'no gamma' from enum list. */
>> +		if (!strcmp(gamma_mode->enums[i].name, "no gamma"))
>> +			continue;
>> +
>> +		igt_info("Trying to use gamma mode: \'%s\'\n", gamma_mode->enums[i].name);
>> +
>> +		segment_info = get_segment_data(data, gamma_mode->enums[i].value,
>> +				gamma_mode->enums[i].name);
>> +		lut_size = sizeof(struct drm_color_lut_ext) * segment_info->entries_count;
>> +
>> +		igt_plane_set_prop_enum(plane, IGT_PLANE_GAMMA_MODE, gamma_mode->enums[i].name);
>> +		invalid_plane_lut_sizes(display, plane,
>> +					IGT_PLANE_GAMMA_LUT,
>> +					lut_size);
>> +
>> +		clear_segment_data(segment_info);
>> +
>> +		/* One enum is enough. */
>> +		break;
>> +	}
>> +
>> +	drmModeFreeProperty(gamma_mode);
>> +
>> +	return true;
>> +}
>> +
>> +static bool invalid_plane_degamma_test(data_t *data, igt_plane_t *plane)
>> +{
>> +	igt_display_t *display = &data->display;
>> +	drmModePropertyPtr degamma_mode = NULL;
>> +	uint32_t i;
>> +
>> +	igt_info("Plane invalid degamma test is running on pipe-%s plane-%s(%s)\n",
>> +			kmstest_pipe_name(plane->pipe->pipe),
>> +			kmstest_plane_type_name(plane->type),
>> +			is_hdr_plane(plane) ? "hdr":"sdr");
>> +
>> +	igt_require(igt_plane_has_prop(plane, IGT_PLANE_DEGAMMA_MODE));
>> +	igt_require(igt_plane_has_prop(plane, IGT_PLANE_DEGAMMA_LUT));
>> +
>> +	degamma_mode = get_plane_gamma_degamma_mode(plane, IGT_PLANE_DEGAMMA_MODE);
>> +
>> +	/* Iterate all supported degamma modes. */
>> +	for (i = 0; i < degamma_mode->count_enums; i++) {
>> +		segment_data_t *segment_info = NULL;
>> +		size_t lut_size = 0;
>> +
>> +		/* Ignore 'no degamma' from enum list. */
>> +		if (!strcmp(degamma_mode->enums[i].name, "no degamma"))
>> +			continue;
>> +
>> +		igt_info("Trying to use degamma mode: \'%s\'\n", degamma_mode->enums[i].name);
>> +
>> +		segment_info = get_segment_data(data,
>> +						degamma_mode->enums[i].value,
>> +						degamma_mode->enums[i].name);
>> +		lut_size = sizeof(struct drm_color_lut_ext) * segment_info->entries_count * 2;
>> +
>> +		igt_plane_set_prop_enum(plane,
>> +					IGT_PLANE_DEGAMMA_MODE,
>> +					degamma_mode->enums[i].name);
>> +		invalid_plane_lut_sizes(display, plane,
>> +					IGT_PLANE_DEGAMMA_LUT,
>> +					lut_size);
>> +
>> +		clear_segment_data(segment_info);
>> +
>> +		/* One enum is enough. */
>> +		break;
> 
> Why is one enum enough?
> 

I think it's another intellism since Uma's patches only
report one enum.

Since the API is designed to allow for multiple enums the test
should just run on all of them. It'd be a trivial change to the
test.

Harry

> The same question for the other case in this patch.
> 
> 
>> +	}
>> +
>> +	drmModeFreeProperty(degamma_mode);
>> +
>> +	return true;
>> +}
>> +
>> +static bool invalid_plane_ctm_test(data_t *data, igt_plane_t *plane)
>> +{
>> +	igt_info("Plane invalid CTM test is running on pipe-%s plane-%s(%s)\n",
>> +			kmstest_pipe_name(plane->pipe->pipe),
>> +			kmstest_plane_type_name(plane->type),
>> +			is_hdr_plane(plane) ? "hdr":"sdr");
>> +
>> +	igt_require(igt_plane_has_prop(plane, IGT_PLANE_CTM));
>> +	invalid_plane_lut_sizes(&data->display, plane,
>> +				IGT_PLANE_CTM,
>> +				sizeof(struct drm_color_ctm));
> 
> The code says you're trying shove a LUT into a CTM blob. I understand
> that mechanically this is test you want to do, feed a wrong sized blob,
> and in this case the contents do not matter (unlike with actual LUTs),
> but reading this code is completely misleading and does not make sense.
> It takes a while to think about what you actually want to test here,
> and then reverse-engineer the code to understand that that is what
> actually happens, too. That is too much mental burden for the reader to
> realize that this piece of code actually works.
> 
> 
> Thanks,
> pq
> 
>> +
>> +	return true;
>> +}
>> +
>>  static bool plane_gamma_test(data_t *data, igt_plane_t *plane)
>>  {
>>  	igt_output_t *output;
>> @@ -1411,6 +1523,21 @@ static void run_tests_for_plane(data_t *data, enum pipe pipe)
>>  					ctm_tests[i].iter);
>>  		}
>>  	}
>> +
>> +	igt_describe("Negative check for invalid plane gamma lut sizes");
>> +	igt_subtest_f("pipe-%s-invalid-plane-gamma-lut-sizes",
>> +			kmstest_pipe_name(pipe))
>> +		run_plane_color_test(data, pipe, invalid_plane_gamma_test);
>> +
>> +	igt_describe("Negative check for invalid plane degamma lut sizes");
>> +	igt_subtest_f("pipe-%s-invalid-plane-degamma-lut-sizes",
>> +			kmstest_pipe_name(pipe))
>> +		run_plane_color_test(data, pipe, invalid_plane_degamma_test);
>> +
>> +	igt_describe("Negative check for invalid plane ctm matrix sizes");
>> +	igt_subtest_f("pipe-%s-invalid-plane-ctm-matrix-sizes",
>> +			kmstest_pipe_name(pipe))
>> +		run_plane_color_test(data, pipe, invalid_plane_ctm_test);
>>  }
>>  
>>  igt_main
>
Modem, Bhanuprakash Jan. 3, 2022, 4:05 a.m. UTC | #3 
> From: Pekka Paalanen <ppaalanen@gmail.com>
> Sent: Thursday, November 18, 2021 2:50 PM
> To: Modem, Bhanuprakash <bhanuprakash.modem@intel.com>
> Cc: igt-dev@lists.freedesktop.org; dri-devel@lists.freedesktop.org; Juha-Pekka
> Heikkila <juhapekka.heikkila@gmail.com>; Shankar, Uma <uma.shankar@intel.com>
> Subject: Re: [i-g-t 07/14] tests/kms_color: New negative tests for plane level
> color mgmt
> 
> On Mon, 15 Nov 2021 15:17:52 +0530
> Bhanuprakash Modem <bhanuprakash.modem@intel.com> wrote:
> 
> > Negative check for:
> >  * plane gamma lut sizes
> >  * plane degamma lut sizes
> >  * plane ctm matrix sizes
> >
> > Cc: Harry Wentland <harry.wentland@amd.com>
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Cc: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
> > Cc: Uma Shankar <uma.shankar@intel.com>
> > Signed-off-by: Bhanuprakash Modem <bhanuprakash.modem@intel.com>
> > ---
> >  tests/kms_color.c | 127 ++++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 127 insertions(+)
> >
> > diff --git a/tests/kms_color.c b/tests/kms_color.c
> > index e14b37cb6f..d9fe417ba9 100644
> > --- a/tests/kms_color.c
> > +++ b/tests/kms_color.c
> > @@ -736,6 +736,118 @@ static void test_pipe_limited_range_ctm(data_t *data,
> >  }
> >  #endif
> >
> > +static bool invalid_plane_gamma_test(data_t *data, igt_plane_t *plane)
> > +{
> > +	igt_display_t *display = &data->display;
> > +	drmModePropertyPtr gamma_mode = NULL;
> > +	uint32_t i;
> > +
> > +	igt_info("Plane invalid gamma test is running on pipe-%s plane-
> %s(%s)\n",
> > +			kmstest_pipe_name(plane->pipe->pipe),
> > +			kmstest_plane_type_name(plane->type),
> > +			is_hdr_plane(plane) ? "hdr":"sdr");
> > +
> > +	igt_require(igt_plane_has_prop(plane, IGT_PLANE_GAMMA_MODE));
> > +	igt_require(igt_plane_has_prop(plane, IGT_PLANE_GAMMA_LUT));
> > +
> > +	gamma_mode = get_plane_gamma_degamma_mode(plane, IGT_PLANE_GAMMA_MODE);
> > +
> > +	/* Iterate all supported gamma modes. */
> > +	for (i = 0; i < gamma_mode->count_enums; i++) {
> > +		segment_data_t *segment_info = NULL;
> > +		size_t lut_size = 0;
> > +
> > +		/* Ignore 'no gamma' from enum list. */
> > +		if (!strcmp(gamma_mode->enums[i].name, "no gamma"))
> > +			continue;
> > +
> > +		igt_info("Trying to use gamma mode: \'%s\'\n", gamma_mode-
> >enums[i].name);
> > +
> > +		segment_info = get_segment_data(data, gamma_mode->enums[i].value,
> > +				gamma_mode->enums[i].name);
> > +		lut_size = sizeof(struct drm_color_lut_ext) * segment_info-
> >entries_count;
> > +
> > +		igt_plane_set_prop_enum(plane, IGT_PLANE_GAMMA_MODE, gamma_mode-
> >enums[i].name);
> > +		invalid_plane_lut_sizes(display, plane,
> > +					IGT_PLANE_GAMMA_LUT,
> > +					lut_size);
> > +
> > +		clear_segment_data(segment_info);
> > +
> > +		/* One enum is enough. */
> > +		break;
> > +	}
> > +
> > +	drmModeFreeProperty(gamma_mode);
> > +
> > +	return true;
> > +}
> > +
> > +static bool invalid_plane_degamma_test(data_t *data, igt_plane_t *plane)
> > +{
> > +	igt_display_t *display = &data->display;
> > +	drmModePropertyPtr degamma_mode = NULL;
> > +	uint32_t i;
> > +
> > +	igt_info("Plane invalid degamma test is running on pipe-%s plane-
> %s(%s)\n",
> > +			kmstest_pipe_name(plane->pipe->pipe),
> > +			kmstest_plane_type_name(plane->type),
> > +			is_hdr_plane(plane) ? "hdr":"sdr");
> > +
> > +	igt_require(igt_plane_has_prop(plane, IGT_PLANE_DEGAMMA_MODE));
> > +	igt_require(igt_plane_has_prop(plane, IGT_PLANE_DEGAMMA_LUT));
> > +
> > +	degamma_mode = get_plane_gamma_degamma_mode(plane,
> IGT_PLANE_DEGAMMA_MODE);
> > +
> > +	/* Iterate all supported degamma modes. */
> > +	for (i = 0; i < degamma_mode->count_enums; i++) {
> > +		segment_data_t *segment_info = NULL;
> > +		size_t lut_size = 0;
> > +
> > +		/* Ignore 'no degamma' from enum list. */
> > +		if (!strcmp(degamma_mode->enums[i].name, "no degamma"))
> > +			continue;
> > +
> > +		igt_info("Trying to use degamma mode: \'%s\'\n", degamma_mode-
> >enums[i].name);
> > +
> > +		segment_info = get_segment_data(data,
> > +						degamma_mode->enums[i].value,
> > +						degamma_mode->enums[i].name);
> > +		lut_size = sizeof(struct drm_color_lut_ext) * segment_info-
> >entries_count * 2;
> > +
> > +		igt_plane_set_prop_enum(plane,
> > +					IGT_PLANE_DEGAMMA_MODE,
> > +					degamma_mode->enums[i].name);
> > +		invalid_plane_lut_sizes(display, plane,
> > +					IGT_PLANE_DEGAMMA_LUT,
> > +					lut_size);
> > +
> > +		clear_segment_data(segment_info);
> > +
> > +		/* One enum is enough. */
> > +		break;
> 
> Why is one enum enough?
> 
> The same question for the other case in this patch.

This is just for CI time optimization, seems we don't save much CI time
I'll remove this & float a new rev.

> 
> 
> > +	}
> > +
> > +	drmModeFreeProperty(degamma_mode);
> > +
> > +	return true;
> > +}
> > +
> > +static bool invalid_plane_ctm_test(data_t *data, igt_plane_t *plane)
> > +{
> > +	igt_info("Plane invalid CTM test is running on pipe-%s plane-%s(%s)\n",
> > +			kmstest_pipe_name(plane->pipe->pipe),
> > +			kmstest_plane_type_name(plane->type),
> > +			is_hdr_plane(plane) ? "hdr":"sdr");
> > +
> > +	igt_require(igt_plane_has_prop(plane, IGT_PLANE_CTM));
> > +	invalid_plane_lut_sizes(&data->display, plane,
> > +				IGT_PLANE_CTM,
> > +				sizeof(struct drm_color_ctm));
> 
> The code says you're trying shove a LUT into a CTM blob. I understand
> that mechanically this is test you want to do, feed a wrong sized blob,
> and in this case the contents do not matter (unlike with actual LUTs),
> but reading this code is completely misleading and does not make sense.
> It takes a while to think about what you actually want to test here,
> and then reverse-engineer the code to understand that that is what
> actually happens, too. That is too much mental burden for the reader to
> realize that this piece of code actually works.
> 

Sorry for the poor documentation, I'll try to add some comments.

- Bhanu

> 
> Thanks,
> pq
> 
> > +
> > +	return true;
> > +}
> > +
> >  static bool plane_gamma_test(data_t *data, igt_plane_t *plane)
> >  {
> >  	igt_output_t *output;
> > @@ -1411,6 +1523,21 @@ static void run_tests_for_plane(data_t *data, enum
> pipe pipe)
> >  					ctm_tests[i].iter);
> >  		}
> >  	}
> > +
> > +	igt_describe("Negative check for invalid plane gamma lut sizes");
> > +	igt_subtest_f("pipe-%s-invalid-plane-gamma-lut-sizes",
> > +			kmstest_pipe_name(pipe))
> > +		run_plane_color_test(data, pipe, invalid_plane_gamma_test);
> > +
> > +	igt_describe("Negative check for invalid plane degamma lut sizes");
> > +	igt_subtest_f("pipe-%s-invalid-plane-degamma-lut-sizes",
> > +			kmstest_pipe_name(pipe))
> > +		run_plane_color_test(data, pipe, invalid_plane_degamma_test);
> > +
> > +	igt_describe("Negative check for invalid plane ctm matrix sizes");
> > +	igt_subtest_f("pipe-%s-invalid-plane-ctm-matrix-sizes",
> > +			kmstest_pipe_name(pipe))
> > +		run_plane_color_test(data, pipe, invalid_plane_ctm_test);
> >  }
> >
> >  igt_main
diff mbox series

Patch

diff --git a/tests/kms_color.c b/tests/kms_color.c
index e14b37cb6f..d9fe417ba9 100644
--- a/tests/kms_color.c
+++ b/tests/kms_color.c
@@ -736,6 +736,118 @@  static void test_pipe_limited_range_ctm(data_t *data,
 }
 #endif
 
+static bool invalid_plane_gamma_test(data_t *data, igt_plane_t *plane)
+{
+	igt_display_t *display = &data->display;
+	drmModePropertyPtr gamma_mode = NULL;
+	uint32_t i;
+
+	igt_info("Plane invalid gamma test is running on pipe-%s plane-%s(%s)\n",
+			kmstest_pipe_name(plane->pipe->pipe),
+			kmstest_plane_type_name(plane->type),
+			is_hdr_plane(plane) ? "hdr":"sdr");
+
+	igt_require(igt_plane_has_prop(plane, IGT_PLANE_GAMMA_MODE));
+	igt_require(igt_plane_has_prop(plane, IGT_PLANE_GAMMA_LUT));
+
+	gamma_mode = get_plane_gamma_degamma_mode(plane, IGT_PLANE_GAMMA_MODE);
+
+	/* Iterate all supported gamma modes. */
+	for (i = 0; i < gamma_mode->count_enums; i++) {
+		segment_data_t *segment_info = NULL;
+		size_t lut_size = 0;
+
+		/* Ignore 'no gamma' from enum list. */
+		if (!strcmp(gamma_mode->enums[i].name, "no gamma"))
+			continue;
+
+		igt_info("Trying to use gamma mode: \'%s\'\n", gamma_mode->enums[i].name);
+
+		segment_info = get_segment_data(data, gamma_mode->enums[i].value,
+				gamma_mode->enums[i].name);
+		lut_size = sizeof(struct drm_color_lut_ext) * segment_info->entries_count;
+
+		igt_plane_set_prop_enum(plane, IGT_PLANE_GAMMA_MODE, gamma_mode->enums[i].name);
+		invalid_plane_lut_sizes(display, plane,
+					IGT_PLANE_GAMMA_LUT,
+					lut_size);
+
+		clear_segment_data(segment_info);
+
+		/* One enum is enough. */
+		break;
+	}
+
+	drmModeFreeProperty(gamma_mode);
+
+	return true;
+}
+
+static bool invalid_plane_degamma_test(data_t *data, igt_plane_t *plane)
+{
+	igt_display_t *display = &data->display;
+	drmModePropertyPtr degamma_mode = NULL;
+	uint32_t i;
+
+	igt_info("Plane invalid degamma test is running on pipe-%s plane-%s(%s)\n",
+			kmstest_pipe_name(plane->pipe->pipe),
+			kmstest_plane_type_name(plane->type),
+			is_hdr_plane(plane) ? "hdr":"sdr");
+
+	igt_require(igt_plane_has_prop(plane, IGT_PLANE_DEGAMMA_MODE));
+	igt_require(igt_plane_has_prop(plane, IGT_PLANE_DEGAMMA_LUT));
+
+	degamma_mode = get_plane_gamma_degamma_mode(plane, IGT_PLANE_DEGAMMA_MODE);
+
+	/* Iterate all supported degamma modes. */
+	for (i = 0; i < degamma_mode->count_enums; i++) {
+		segment_data_t *segment_info = NULL;
+		size_t lut_size = 0;
+
+		/* Ignore 'no degamma' from enum list. */
+		if (!strcmp(degamma_mode->enums[i].name, "no degamma"))
+			continue;
+
+		igt_info("Trying to use degamma mode: \'%s\'\n", degamma_mode->enums[i].name);
+
+		segment_info = get_segment_data(data,
+						degamma_mode->enums[i].value,
+						degamma_mode->enums[i].name);
+		lut_size = sizeof(struct drm_color_lut_ext) * segment_info->entries_count * 2;
+
+		igt_plane_set_prop_enum(plane,
+					IGT_PLANE_DEGAMMA_MODE,
+					degamma_mode->enums[i].name);
+		invalid_plane_lut_sizes(display, plane,
+					IGT_PLANE_DEGAMMA_LUT,
+					lut_size);
+
+		clear_segment_data(segment_info);
+
+		/* One enum is enough. */
+		break;
+	}
+
+	drmModeFreeProperty(degamma_mode);
+
+	return true;
+}
+
+static bool invalid_plane_ctm_test(data_t *data, igt_plane_t *plane)
+{
+	igt_info("Plane invalid CTM test is running on pipe-%s plane-%s(%s)\n",
+			kmstest_pipe_name(plane->pipe->pipe),
+			kmstest_plane_type_name(plane->type),
+			is_hdr_plane(plane) ? "hdr":"sdr");
+
+	igt_require(igt_plane_has_prop(plane, IGT_PLANE_CTM));
+	invalid_plane_lut_sizes(&data->display, plane,
+				IGT_PLANE_CTM,
+				sizeof(struct drm_color_ctm));
+
+	return true;
+}
+
 static bool plane_gamma_test(data_t *data, igt_plane_t *plane)
 {
 	igt_output_t *output;
@@ -1411,6 +1523,21 @@  static void run_tests_for_plane(data_t *data, enum pipe pipe)
 					ctm_tests[i].iter);
 		}
 	}
+
+	igt_describe("Negative check for invalid plane gamma lut sizes");
+	igt_subtest_f("pipe-%s-invalid-plane-gamma-lut-sizes",
+			kmstest_pipe_name(pipe))
+		run_plane_color_test(data, pipe, invalid_plane_gamma_test);
+
+	igt_describe("Negative check for invalid plane degamma lut sizes");
+	igt_subtest_f("pipe-%s-invalid-plane-degamma-lut-sizes",
+			kmstest_pipe_name(pipe))
+		run_plane_color_test(data, pipe, invalid_plane_degamma_test);
+
+	igt_describe("Negative check for invalid plane ctm matrix sizes");
+	igt_subtest_f("pipe-%s-invalid-plane-ctm-matrix-sizes",
+			kmstest_pipe_name(pipe))
+		run_plane_color_test(data, pipe, invalid_plane_ctm_test);
 }
 
 igt_main