[igt-dev] [PATCH] tests/kms_color: Change in commit style from legacy to atomic

Sharma, Swati2 swati2.sharma at intel.com
Fri Mar 29 10:02:21 UTC 2019


Sorry I was of the perception that this is required too since we are having only legacy commit in this IGT.

Thanks and Regards,
Swati

-----Original Message-----
From: Daniel Vetter <daniel.vetter at ffwll.ch> On Behalf Of Daniel Vetter
Sent: Friday, March 29, 2019 3:25 PM
To: Sharma, Swati2 <swati2.sharma at intel.com>
Cc: igt-dev at lists.freedesktop.org; maarten.lankhorst at linux.intel.com; daniel.vetter at ffwll.ch; Nautiyal, Ankit K <ankit.k.nautiyal at intel.com>; Shankar, Uma <uma.shankar at intel.com>; ville.syrjala at linux.intel.com; Nikula, Jani <jani.nikula at intel.com>
Subject: Re: [PATCH] tests/kms_color: Change in commit style from legacy to atomic

On Fri, Mar 29, 2019 at 12:20:14PM +0530, Swati Sharma wrote:
> Existing kms_color i-g-t, commit style by default is legacy for all 
> the ctm/gamma/degamma subtests.
> 
> In this patch, legacy commit is changed to atomic (since i915 no 
> longer supports legacy commit)
> 
> v1: As per Daniel's comments switching over to atomic.
> v2: As per Maarten's comments did COMMIT_ATOMIC, and added
>     igt_require(display.is_atomic)
> v3: Fixed stupid mistake, now test cases are not getting skipped
> 
> Need Feedback?
> Still there are few functions like pipe_set_property_blob_id, 
> pipe_set_property_blob, invalid_lut_sizes where provision of both 
> legacy and atomic is there. Should I change them too to atomic only 
> like I did for other func?
> 
> Also, this patch is floating since ages, it will be nice if I can have 
> a closure(=>rb) on this and move forward :)
> 
> Signed-off-by: Swati Sharma <swati2.sharma at intel.com>

Typed this on jira already months ago, replicating here: The idea is to add an atomic test (with crc checks every frame and all that) to make sure lut updates are atomic on screen. Switching to atomic commit style for everything else just means we can't run this on older drivers anymore (not that there are many of those left), which seems a bit pointless.
-Daniel

> ---
>  tests/kms_color.c | 42 +++++++++++++++++++++---------------------
>  1 file changed, 21 insertions(+), 21 deletions(-)
> 
> diff --git a/tests/kms_color.c b/tests/kms_color.c index 
> decf3c2..92f5c12 100644
> --- a/tests/kms_color.c
> +++ b/tests/kms_color.c
> @@ -63,7 +63,6 @@ typedef struct {
>  	uint64_t gamma_lut_size;
>  } data_t;
>  
> -
>  static void paint_gradient_rectangles(data_t *data,
>  				      drmModeModeInfo *mode,
>  				      color_t *colors,
> @@ -311,12 +310,12 @@ static void test_pipe_degamma(data_t *data,
>  		disable_ctm(primary->pipe);
>  		disable_degamma(primary->pipe);
>  		set_gamma(data, primary->pipe, gamma_linear);
> -		igt_display_commit(&data->display);
> +		igt_display_commit2(&data->display, COMMIT_ATOMIC);
>  
>  		/* Draw solid colors with no degamma transformation. */
>  		paint_rectangles(data, mode, red_green_blue, &fb);
>  		igt_plane_set_fb(primary, &fb);
> -		igt_display_commit(&data->display);
> +		igt_display_commit2(&data->display, COMMIT_ATOMIC);
>  		igt_wait_for_vblank(data->drm_fd, primary->pipe->pipe);
>  		igt_pipe_crc_collect_crc(data->pipe_crc, &crc_fullcolors);
>  
> @@ -326,7 +325,7 @@ static void test_pipe_degamma(data_t *data,
>  		paint_gradient_rectangles(data, mode, red_green_blue, &fb);
>  		igt_plane_set_fb(primary, &fb);
>  		set_degamma(data, primary->pipe, degamma_full);
> -		igt_display_commit(&data->display);
> +		igt_display_commit2(&data->display, COMMIT_ATOMIC);
>  		igt_wait_for_vblank(data->drm_fd, primary->pipe->pipe);
>  		igt_pipe_crc_collect_crc(data->pipe_crc, &crc_fullgamma);
>  
> @@ -391,12 +390,12 @@ static void test_pipe_gamma(data_t *data,
>  		disable_ctm(primary->pipe);
>  		disable_degamma(primary->pipe);
>  		set_gamma(data, primary->pipe, gamma_full);
> -		igt_display_commit(&data->display);
> +		igt_display_commit2(&data->display, COMMIT_ATOMIC);
>  
>  		/* Draw solid colors with no gamma transformation. */
>  		paint_rectangles(data, mode, red_green_blue, &fb);
>  		igt_plane_set_fb(primary, &fb);
> -		igt_display_commit(&data->display);
> +		igt_display_commit2(&data->display, COMMIT_ATOMIC);
>  		igt_wait_for_vblank(data->drm_fd, primary->pipe->pipe);
>  		igt_pipe_crc_collect_crc(data->pipe_crc, &crc_fullcolors);
>  
> @@ -405,7 +404,7 @@ static void test_pipe_gamma(data_t *data,
>  		 */
>  		paint_gradient_rectangles(data, mode, red_green_blue, &fb);
>  		igt_plane_set_fb(primary, &fb);
> -		igt_display_commit(&data->display);
> +		igt_display_commit2(&data->display, COMMIT_ATOMIC);
>  		igt_wait_for_vblank(data->drm_fd, primary->pipe->pipe);
>  		igt_pipe_crc_collect_crc(data->pipe_crc, &crc_fullgamma);
>  
> @@ -477,12 +476,12 @@ static void test_pipe_legacy_gamma(data_t *data,
>  		disable_degamma(primary->pipe);
>  		disable_gamma(primary->pipe);
>  		disable_ctm(primary->pipe);
> -		igt_display_commit(&data->display);
> +		igt_display_commit2(&data->display, COMMIT_ATOMIC);
>  
>  		/* Draw solid colors with no gamma transformation. */
>  		paint_rectangles(data, mode, red_green_blue, &fb);
>  		igt_plane_set_fb(primary, &fb);
> -		igt_display_commit(&data->display);
> +		igt_display_commit2(&data->display, COMMIT_ATOMIC);
>  		igt_wait_for_vblank(data->drm_fd, primary->pipe->pipe);
>  		igt_pipe_crc_collect_crc(data->pipe_crc, &crc_fullcolors);
>  
> @@ -497,7 +496,7 @@ static void test_pipe_legacy_gamma(data_t *data,
>  			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);
> +		igt_display_commit2(&data->display, COMMIT_ATOMIC);
>  		igt_wait_for_vblank(data->drm_fd, primary->pipe->pipe);
>  		igt_pipe_crc_collect_crc(data->pipe_crc, &crc_fullgamma);
>  
> @@ -512,7 +511,7 @@ static void test_pipe_legacy_gamma(data_t *data,
>  
>  		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);
> +		igt_display_commit2(&data->display, COMMIT_ATOMIC);
>  
>  		igt_plane_set_fb(primary, NULL);
>  		igt_output_set_pipe(output, PIPE_NONE); @@ -566,7 +565,7 @@ static 
> void test_pipe_legacy_gamma_reset(data_t *data,
>  		disable_degamma(primary->pipe);
>  		disable_ctm(primary->pipe);
>  		disable_gamma(primary->pipe);
> -		igt_display_commit(&data->display);
> +		igt_display_commit2(&data->display, COMMIT_ATOMIC);
>  
>  		/* Set a degama & gamma LUT and a CTM using the
>  		 * properties and verify the content of the @@ -574,7 +573,7 @@ 
> static void test_pipe_legacy_gamma_reset(data_t *data,
>  		set_degamma(data, primary->pipe, degamma_linear);
>  		set_ctm(primary->pipe, ctm_identity);
>  		set_gamma(data, primary->pipe, gamma_zero);
> -		igt_display_commit(&data->display);
> +		igt_display_commit2(&data->display, COMMIT_ATOMIC);
>  
>  		blob = get_blob(data, primary->pipe, IGT_CRTC_DEGAMMA_LUT);
>  		igt_assert(blob &&
> @@ -617,7 +616,7 @@ static void test_pipe_legacy_gamma_reset(data_t *data,
>  						  legacy_lut_size,
>  						  red_lut, green_lut, blue_lut),
>  			      0);
> -		igt_display_commit(&data->display);
> +		igt_display_commit2(&data->display, COMMIT_ATOMIC);
>  
>  		igt_assert(get_blob(data, primary->pipe,
>  				    IGT_CRTC_DEGAMMA_LUT) == NULL); @@ -699,12 +698,12 @@ static 
> bool test_pipe_ctm(data_t *data,
>  		set_degamma(data, primary->pipe, degamma_linear);
>  		set_gamma(data, primary->pipe, gamma_linear);
>  		disable_ctm(primary->pipe);
> -		igt_display_commit(&data->display);
> +		igt_display_commit2(&data->display, COMMIT_ATOMIC);
>  
>  		paint_rectangles(data, mode, after, &fb);
>  		igt_plane_set_fb(primary, &fb);
>  		set_ctm(primary->pipe, ctm_identity);
> -		igt_display_commit(&data->display);
> +		igt_display_commit2(&data->display, COMMIT_ATOMIC);
>  		igt_wait_for_vblank(data->drm_fd, primary->pipe->pipe);
>  		igt_pipe_crc_collect_crc(data->pipe_crc, &crc_software);
>  
> @@ -712,7 +711,7 @@ static bool test_pipe_ctm(data_t *data,
>  		paint_rectangles(data, mode, before, &fb);
>  		igt_plane_set_fb(primary, &fb);
>  		set_ctm(primary->pipe, ctm_matrix);
> -		igt_display_commit(&data->display);
> +		igt_display_commit2(&data->display, COMMIT_ATOMIC);
>  		igt_wait_for_vblank(data->drm_fd, primary->pipe->pipe);
>  		igt_pipe_crc_collect_crc(data->pipe_crc, &crc_hardware);
>  
> @@ -807,7 +806,7 @@ static void test_pipe_limited_range_ctm(data_t *data,
>  		igt_output_set_prop_value(output, IGT_CONNECTOR_BROADCAST_RGB, BROADCAST_RGB_FULL);
>  		paint_rectangles(data, mode, red_green_blue_limited, &fb);
>  		igt_plane_set_fb(primary, &fb);
> -		igt_display_commit(&data->display);
> +		igt_display_commit2(&data->display, COMMIT_ATOMIC);
>  		igt_wait_for_vblank(data->drm_fd, primary->pipe->pipe);
>  		igt_pipe_crc_collect_crc(data->pipe_crc, &crc_full);
>  
> @@ -815,7 +814,7 @@ static void test_pipe_limited_range_ctm(data_t *data,
>  		igt_output_set_prop_value(output, IGT_CONNECTOR_BROADCAST_RGB, BROADCAST_RGB_16_235);
>  		paint_rectangles(data, mode, red_green_blue_full, &fb);
>  		igt_plane_set_fb(primary, &fb);
> -		igt_display_commit(&data->display);
> +		igt_display_commit2(&data->display, COMMIT_ATOMIC);
>  		igt_wait_for_vblank(data->drm_fd, primary->pipe->pipe);
>  		igt_pipe_crc_collect_crc(data->pipe_crc, &crc_limited);
>  
> @@ -1043,7 +1042,7 @@ run_tests_for_pipe(data_t *data, enum pipe p)
>  		disable_degamma(primary->pipe);
>  		disable_gamma(primary->pipe);
>  		disable_ctm(primary->pipe);
> -		igt_display_commit(&data->display);
> +		igt_display_commit2(&data->display, COMMIT_ATOMIC);
>  
>  		igt_pipe_crc_free(data->pipe_crc);
>  		data->pipe_crc = NULL;
> @@ -1085,7 +1084,7 @@ invalid_lut_sizes(data_t *data)
>  	struct _drm_color_lut *degamma_lut = malloc(data->degamma_lut_size * sizeof(struct _drm_color_lut) * 2);
>  	struct _drm_color_lut *gamma_lut = malloc(data->gamma_lut_size * 
> sizeof(struct _drm_color_lut) * 2);
>  
> -	igt_display_commit2(display, display->is_atomic ? COMMIT_ATOMIC : COMMIT_LEGACY);
> +	igt_display_commit2(display, COMMIT_ATOMIC);
>  
>  	if (igt_pipe_obj_has_prop(pipe, IGT_CRTC_DEGAMMA_LUT)) {
>  		igt_assert_eq(pipe_set_property_blob(pipe, IGT_CRTC_DEGAMMA_LUT, @@ 
> -1173,6 +1172,7 @@ igt_main
>  		kmstest_set_vt_graphics_mode();
>  
>  		igt_display_require(&data.display, data.drm_fd);
> +		igt_require(data.display.is_atomic);
>  	}
>  
>  	for_each_pipe_static(pipe)
> --
> 1.9.1
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch


More information about the igt-dev mailing list