<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
</head>
<body text="#000000" bgcolor="#FFFFFF">
<div class="moz-cite-prefix">On 29-Mar-19 4:04 PM, Ville Syrjälä
wrote:<br>
</div>
<blockquote type="cite" cite="mid:20190329103433.GR3888@intel.com">
<pre class="moz-quote-pre" wrap="">On Fri, Mar 29, 2019 at 10:54:53AM +0100, Daniel Vetter wrote:
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">On Fri, Mar 29, 2019 at 12:20:14PM +0530, Swati Sharma wrote:
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">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 <a class="moz-txt-link-rfc2396E" href="mailto:swati2.sharma@intel.com"><swati2.sharma@intel.com></a>
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
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.
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
I have a test for that. It needs my vblank workers (still a bit wip) to
actually work (well, mostly work).
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">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</pre>
</blockquote>
</blockquote>
<pre>Hi Daniel,
I think there is some misunderstanding from my end. When I first sent this patch as
an RFC <a href="https://patchwork.freedesktop.org/patch/254752/">https://patchwork.freedesktop.org/patch/254752/</a> there I had given provision
for both legacy and atomic.
That time feedback which I received was:
Imo just switch over to atomic. There's no upstream driver supporting
color management props that doesn't support atomic.
-Daniel
That's y I switched to atomic completely.
I still feel provision for both legacy and atomic should be given. Though this change is
not adding atomic test(now which is already Ville is doing) but like other IGTs
it should allow commit through both the paths.
Please do let me know what you think. Thanks in advance.
</pre>
<blockquote type="cite" cite="mid:20190329103433.GR3888@intel.com">
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">---
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
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
--
Daniel Vetter
Software Engineer, Intel Corporation
<a class="moz-txt-link-freetext" href="http://blog.ffwll.ch">http://blog.ffwll.ch</a>
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
</pre>
</blockquote>
<br>
<br>
<pre class="moz-signature" cols="72">--
~Swati Sharma
</pre>
</body>
</html>