public inbox for igt-dev@lists.freedesktop.org
 help / color / mirror / Atom feed
From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Daniel Vetter <daniel@ffwll.ch>
Cc: jani.nikula@intel.com, igt-dev@lists.freedesktop.org
Subject: Re: [igt-dev] [PATCH] tests/kms_color: Change in commit style from legacy to atomic
Date: Fri, 29 Mar 2019 12:34:33 +0200	[thread overview]
Message-ID: <20190329103433.GR3888@intel.com> (raw)
In-Reply-To: <20190329095453.GM2665@phenom.ffwll.local>

On Fri, Mar 29, 2019 at 10:54:53AM +0100, Daniel Vetter wrote:
> 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@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.

I have a test for that. It needs my vblank workers (still a bit wip) to
actually work (well, mostly work).

> 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

-- 
Ville Syrjälä
Intel
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

  parent reply	other threads:[~2019-03-29 10:34 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-03-29  6:50 [igt-dev] [PATCH] tests/kms_color: Change in commit style from legacy to atomic Swati Sharma
2019-03-29  7:47 ` [igt-dev] ✓ Fi.CI.BAT: success for " Patchwork
2019-03-29  9:54 ` [igt-dev] [PATCH] " Daniel Vetter
2019-03-29 10:02   ` Sharma, Swati2
2019-03-29 10:34   ` Ville Syrjälä [this message]
2019-04-01  9:19     ` Sharma, Swati2
2019-03-29 12:05 ` [igt-dev] ✓ Fi.CI.IGT: success for " Patchwork

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190329103433.GR3888@intel.com \
    --to=ville.syrjala@linux.intel.com \
    --cc=daniel@ffwll.ch \
    --cc=igt-dev@lists.freedesktop.org \
    --cc=jani.nikula@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox