Re: [igt-dev] [PATCH i-g-t 4/6] tests/kms_color: Allow most subtests to run with a partial color pipeline

[Daniel Vetter <daniel@ffwll.ch>]

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