In the future I want to allow tests to commit more properties,
but for this to work I have to fix all properties to work better
with atomic commit. Instead of special casing each
property make a bitmask for all property changed flags, and try to
commit all properties.
This has been the most involved one, since legacy pipe commit still
handles a lot of the properties differently from the rest.
Changes since v1:
- Dump all changed properties on commit.
- Fix bug in igt_pipe_refresh().
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx>
---
lib/igt_kms.c | 213 ++++++++++++++++++++------------------
lib/igt_kms.h | 77 ++++++--------
tests/kms_atomic_interruptible.c | 4 +-
tests/kms_atomic_transition.c | 2 +-
tests/kms_crtc_background_color.c | 2 +-
5 files changed, 144 insertions(+), 154 deletions(-)
diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index 3181ee605195..23bb543efd61 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -259,8 +259,8 @@ igt_atomic_fill_connector_props(igt_display_t *display, igt_output_t *output,
}
static void
-igt_atomic_fill_pipe_props(igt_display_t *display, igt_pipe_t *pipe,
- int num_crtc_props, const char **crtc_prop_names)
+igt_fill_pipe_props(igt_display_t *display, igt_pipe_t *pipe,
+ int num_crtc_props, const char **crtc_prop_names)
{
drmModeObjectPropertiesPtr props;
int i, j, fd;
@@ -278,7 +278,7 @@ igt_atomic_fill_pipe_props(igt_display_t *display, igt_pipe_t *pipe,
if (strcmp(prop->name, crtc_prop_names[j]) != 0)
continue;
- pipe->atomic_props_crtc[j] = props->props[i];
+ pipe->props[j] = props->props[i];
break;
}
@@ -1690,7 +1690,6 @@ void igt_display_init(igt_display_t *display, int drm_fd)
int p = 1;
int j, type;
uint8_t last_plane = 0, n_planes = 0;
- uint64_t prop_value;
pipe->crtc_id = resources->crtcs[i];
pipe->display = display;
@@ -1700,29 +1699,12 @@ void igt_display_init(igt_display_t *display, int drm_fd)
pipe->planes = NULL;
pipe->out_fence_fd = -1;
+ igt_fill_pipe_props(display, pipe, IGT_NUM_CRTC_PROPS, igt_crtc_prop_names);
+
get_crtc_property(display->drm_fd, pipe->crtc_id,
- "background_color",
- &pipe->background_property,
- &prop_value,
+ "background_color", NULL,
+ &pipe->values[IGT_CRTC_BACKGROUND],
NULL);
- pipe->background = (uint32_t)prop_value;
- get_crtc_property(display->drm_fd, pipe->crtc_id,
- "DEGAMMA_LUT",
- &pipe->degamma_property,
- NULL,
- NULL);
- get_crtc_property(display->drm_fd, pipe->crtc_id,
- "CTM",
- &pipe->ctm_property,
- NULL,
- NULL);
- get_crtc_property(display->drm_fd, pipe->crtc_id,
- "GAMMA_LUT",
- &pipe->gamma_property,
- NULL,
- NULL);
-
- igt_atomic_fill_pipe_props(display, pipe, IGT_NUM_CRTC_PROPS, igt_crtc_prop_names);
/* count number of valid planes */
for (j = 0; j < plane_resources->count_planes; j++) {
@@ -1807,7 +1789,7 @@ void igt_display_init(igt_display_t *display, int drm_fd)
igt_plane_set_prop_changed(plane, IGT_PLANE_CRTC_ID);
}
- pipe->mode_changed = true;
+ igt_pipe_refresh(display, i, false);
}
/*
@@ -2284,7 +2266,7 @@ static int igt_primary_plane_commit_legacy(igt_plane_t *primary,
if (!igt_plane_is_prop_changed(primary, IGT_PLANE_FB_ID) &&
!(primary->changed & IGT_PLANE_COORD_CHANGED_MASK) &&
- !primary->pipe->mode_changed)
+ !igt_pipe_obj_is_prop_changed(primary->pipe, IGT_CRTC_MODE_ID))
return 0;
crtc_id = pipe->crtc_id;
@@ -2353,6 +2335,16 @@ static int igt_plane_commit(igt_plane_t *plane,
}
}
+static bool is_atomic_prop(enum igt_atomic_crtc_properties prop)
+{
+ if (prop == IGT_CRTC_MODE_ID ||
+ prop == IGT_CRTC_ACTIVE ||
+ prop == IGT_CRTC_OUT_FENCE_PTR)
+ return true;
+
+ return false;
+}
+
/*
* Commit all plane changes to an output. Note that if @s is COMMIT_LEGACY,
* enabling/disabling the primary plane will also enable/disable the CRTC.
@@ -2370,19 +2362,9 @@ static int igt_pipe_commit(igt_pipe_t *pipe,
int i;
int ret;
- if (pipe->background_changed) {
- igt_crtc_set_property(pipe, pipe->background_property,
- pipe->background);
- }
-
- if (pipe->color_mgmt_changed) {
- igt_crtc_set_property(pipe, pipe->degamma_property,
- pipe->degamma_blob);
- igt_crtc_set_property(pipe, pipe->ctm_property,
- pipe->ctm_blob);
- igt_crtc_set_property(pipe, pipe->gamma_property,
- pipe->gamma_blob);
- }
+ for (i = 0; i < IGT_NUM_CRTC_PROPS; i++)
+ if (!is_atomic_prop(i))
+ igt_crtc_set_property(pipe, pipe->props[i], pipe->values[i]);
for (i = 0; i < pipe->n_planes; i++) {
igt_plane_t *plane = &pipe->planes[i];
@@ -2395,9 +2377,10 @@ static int igt_pipe_commit(igt_pipe_t *pipe,
}
static void
-igt_pipe_replace_blob(igt_pipe_t *pipe, uint64_t *blob, void *ptr, size_t length)
+igt_pipe_replace_blob(igt_pipe_t *pipe, enum igt_atomic_crtc_properties prop, void *ptr, size_t length)
{
igt_display_t *display = pipe->display;
+ uint64_t *blob = &pipe->values[prop];
uint32_t blob_id = 0;
if (*blob != 0)
@@ -2409,6 +2392,7 @@ igt_pipe_replace_blob(igt_pipe_t *pipe, uint64_t *blob, void *ptr, size_t length
ptr, length, &blob_id) == 0);
*blob = blob_id;
+ igt_pipe_obj_set_prop_changed(pipe, prop);
}
/*
@@ -2416,51 +2400,23 @@ igt_pipe_replace_blob(igt_pipe_t *pipe, uint64_t *blob, void *ptr, size_t length
*/
static void igt_atomic_prepare_crtc_commit(igt_pipe_t *pipe_obj, drmModeAtomicReq *req)
{
- if (pipe_obj->background_changed)
- igt_atomic_populate_crtc_req(req, pipe_obj, IGT_CRTC_BACKGROUND, pipe_obj->background);
-
- if (pipe_obj->color_mgmt_changed) {
- igt_atomic_populate_crtc_req(req, pipe_obj, IGT_CRTC_DEGAMMA_LUT, pipe_obj->degamma_blob);
- igt_atomic_populate_crtc_req(req, pipe_obj, IGT_CRTC_CTM, pipe_obj->ctm_blob);
- igt_atomic_populate_crtc_req(req, pipe_obj, IGT_CRTC_GAMMA_LUT, pipe_obj->gamma_blob);
- }
-
- if (pipe_obj->mode_changed) {
- igt_output_t *output = igt_pipe_get_output(pipe_obj);
-
- if (!output) {
- igt_pipe_replace_blob(pipe_obj, &pipe_obj->mode_blob, NULL, 0);
-
- LOG(pipe_obj->display, "%s: Setting NULL mode\n",
- kmstest_pipe_name(pipe_obj->pipe));
- } else {
- drmModeModeInfo *mode = igt_output_get_mode(output);
+ int i;
- igt_pipe_replace_blob(pipe_obj, &pipe_obj->mode_blob, mode, sizeof(*mode));
+ for (i = 0; i < IGT_NUM_CRTC_PROPS; i++) {
+ if (!igt_pipe_obj_is_prop_changed(pipe_obj, i))
+ continue;
- LOG(pipe_obj->display, "%s: Setting mode %s from %s\n",
- kmstest_pipe_name(pipe_obj->pipe),
- mode->name, igt_output_name(output));
- }
+ igt_debug("Pipe %s: Setting property \"%s\" to 0x%"PRIx64"/%"PRIi64"\n",
+ kmstest_pipe_name(pipe_obj->pipe), igt_crtc_prop_names[i],
+ pipe_obj->values[i], pipe_obj->values[i]);
- igt_atomic_populate_crtc_req(req, pipe_obj, IGT_CRTC_MODE_ID, pipe_obj->mode_blob);
- igt_atomic_populate_crtc_req(req, pipe_obj, IGT_CRTC_ACTIVE, !!output);
+ igt_assert_lt(0, drmModeAtomicAddProperty(req, pipe_obj->crtc_id, pipe_obj->props[i], pipe_obj->values[i]));
}
if (pipe_obj->out_fence_fd != -1) {
close(pipe_obj->out_fence_fd);
pipe_obj->out_fence_fd = -1;
}
-
- if (pipe_obj->out_fence_requested)
- {
- igt_atomic_populate_crtc_req(req, pipe_obj, IGT_CRTC_OUT_FENCE_PTR,
- (uint64_t)(uintptr_t) &pipe_obj->out_fence_fd);
- }
-
- /*
- * TODO: Add all crtc level properties here
- */
}
/*
@@ -2551,15 +2507,21 @@ display_commit_changed(igt_display_t *display, enum igt_commit_style s)
igt_pipe_t *pipe_obj = &display->pipes[pipe];
igt_plane_t *plane;
- pipe_obj->color_mgmt_changed = false;
- pipe_obj->background_changed = false;
-
- if (s != COMMIT_UNIVERSAL)
- pipe_obj->mode_changed = false;
+ if (s == COMMIT_ATOMIC) {
+ if (igt_pipe_obj_is_prop_changed(pipe_obj, IGT_CRTC_OUT_FENCE_PTR))
+ igt_assert(pipe_obj->out_fence_fd >= 0);
- if (s == COMMIT_ATOMIC && pipe_obj->out_fence_requested) {
- pipe_obj->out_fence_requested = false;
- igt_assert(pipe_obj->out_fence_fd >= 0);
+ pipe_obj->changed = 0;
+ } else {
+ igt_pipe_obj_clear_prop_changed(pipe_obj, IGT_CRTC_BACKGROUND);
+ igt_pipe_obj_clear_prop_changed(pipe_obj, IGT_CRTC_CTM);
+ igt_pipe_obj_clear_prop_changed(pipe_obj, IGT_CRTC_DEGAMMA_LUT);
+ igt_pipe_obj_clear_prop_changed(pipe_obj, IGT_CRTC_GAMMA_LUT);
+
+ if (s != COMMIT_UNIVERSAL) {
+ igt_pipe_obj_clear_prop_changed(pipe_obj, IGT_CRTC_MODE_ID);
+ igt_pipe_obj_clear_prop_changed(pipe_obj, IGT_CRTC_ACTIVE);
+ }
}
for_each_plane_on_pipe(display, pipe, plane) {
@@ -2813,33 +2775,83 @@ void igt_output_override_mode(igt_output_t *output, drmModeModeInfo *mode)
output->use_override_mode = !!mode;
- if (pipe)
- pipe->mode_changed = true;
+ if (pipe) {
+ if (output->display->is_atomic)
+ igt_pipe_replace_blob(pipe, IGT_CRTC_MODE_ID, igt_output_get_mode(output), sizeof(*mode));
+ else
+ igt_pipe_obj_set_prop_changed(pipe, IGT_CRTC_MODE_ID);
+ }
}
void igt_output_set_pipe(igt_output_t *output, enum pipe pipe)
{
igt_display_t *display = output->display;
- igt_pipe_t *old_pipe;
+ igt_pipe_t *old_pipe = NULL, *pipe_obj = NULL;;
igt_assert(output->name);
- if (output->pending_pipe != PIPE_NONE) {
+ if (output->pending_pipe != PIPE_NONE)
old_pipe = igt_output_get_driving_pipe(output);
- old_pipe->mode_changed = true;
- }
+ if (pipe != PIPE_NONE)
+ pipe_obj = &display->pipes[pipe];
LOG(display, "%s: set_pipe(%s)\n", igt_output_name(output),
kmstest_pipe_name(pipe));
output->pending_pipe = pipe;
- if (pipe != PIPE_NONE)
- display->pipes[pipe].mode_changed = true;
+ if (old_pipe) {
+ igt_output_t *old_output;
+
+ old_output = igt_pipe_get_output(old_pipe);
+ if (!old_output) {
+ if (display->is_atomic)
+ igt_pipe_replace_blob(old_pipe, IGT_CRTC_MODE_ID, NULL, 0);
+ else
+ igt_pipe_obj_set_prop_changed(old_pipe, IGT_CRTC_MODE_ID);
+
+ igt_pipe_obj_set_prop_value(old_pipe, IGT_CRTC_ACTIVE, 0);
+ }
+ }
igt_output_set_prop_value(output, IGT_CONNECTOR_CRTC_ID, pipe == PIPE_NONE ? 0 : display->pipes[pipe].crtc_id);
igt_output_refresh(output);
+
+ if (pipe_obj) {
+ if (display->is_atomic)
+ igt_pipe_replace_blob(pipe_obj, IGT_CRTC_MODE_ID, igt_output_get_mode(output), sizeof(drmModeModeInfo));
+ else
+ igt_pipe_obj_set_prop_changed(pipe_obj, IGT_CRTC_MODE_ID);
+
+ igt_pipe_obj_set_prop_value(pipe_obj, IGT_CRTC_ACTIVE, 1);
+ }
+}
+
+/*
+ * igt_pipe_refresh:
+ * @display: a pointer to an #igt_display_t structure
+ * @pipe: Pipe to refresh
+ * @force: Should be set to true if mode_blob is no longer considered
+ * to be valid, for example after doing an atomic commit during fork or closing display fd.
+ *
+ * Requests the pipe to be part of the state on next update.
+ * This is useful when state may have been out of sync after
+ * a fork, or we just want to be sure the pipe is included
+ * in the next commit.
+ */
+void igt_pipe_refresh(igt_display_t *display, enum pipe pipe, bool force)
+{
+ igt_pipe_t *pipe_obj = &display->pipes[pipe];
+
+ if (force && display->is_atomic) {
+ igt_output_t *output = igt_pipe_get_output(pipe_obj);
+
+ pipe_obj->values[IGT_CRTC_MODE_ID] = 0;
+ if (output)
+ igt_pipe_replace_blob(pipe_obj, IGT_CRTC_MODE_ID, igt_output_get_mode(output), sizeof(drmModeModeInfo));
+ } else
+ igt_pipe_obj_set_prop_changed(pipe_obj, IGT_CRTC_MODE_ID);
}
void igt_output_set_scaling_mode(igt_output_t *output, uint64_t scaling_mode)
@@ -3047,28 +3059,25 @@ void igt_plane_set_rotation(igt_plane_t *plane, igt_rotation_t rotation)
*/
void igt_pipe_request_out_fence(igt_pipe_t *pipe)
{
- pipe->out_fence_requested = true;
+ igt_pipe_obj_set_prop_value(pipe, IGT_CRTC_OUT_FENCE_PTR, (ptrdiff_t)&pipe->out_fence_fd);
}
void
igt_pipe_set_degamma_lut(igt_pipe_t *pipe, void *ptr, size_t length)
{
- igt_pipe_replace_blob(pipe, &pipe->degamma_blob, ptr, length);
- pipe->color_mgmt_changed = 1;
+ igt_pipe_replace_blob(pipe, IGT_CRTC_DEGAMMA_LUT, ptr, length);
}
void
igt_pipe_set_ctm_matrix(igt_pipe_t *pipe, void *ptr, size_t length)
{
- igt_pipe_replace_blob(pipe, &pipe->ctm_blob, ptr, length);
- pipe->color_mgmt_changed = 1;
+ igt_pipe_replace_blob(pipe, IGT_CRTC_CTM, ptr, length);
}
void
igt_pipe_set_gamma_lut(igt_pipe_t *pipe, void *ptr, size_t length)
{
- igt_pipe_replace_blob(pipe, &pipe->gamma_blob, ptr, length);
- pipe->color_mgmt_changed = 1;
+ igt_pipe_replace_blob(pipe, IGT_CRTC_GAMMA_LUT, ptr, length);
}
/**
@@ -3089,9 +3098,7 @@ void igt_crtc_set_background(igt_pipe_t *pipe, uint64_t background)
kmstest_pipe_name(pipe->pipe),
pipe->pipe, background);
- pipe->background = background;
-
- pipe->background_changed = true;
+ igt_pipe_obj_set_prop_value(pipe, IGT_CRTC_BACKGROUND, background);
}
void igt_wait_for_vblank_count(int drm_fd, enum pipe pipe, int count)
diff --git a/lib/igt_kms.h b/lib/igt_kms.h
index f87f8be31421..b53127ffef5f 100644
--- a/lib/igt_kms.h
+++ b/lib/igt_kms.h
@@ -313,27 +313,13 @@ struct igt_pipe {
int plane_primary;
igt_plane_t *planes;
- uint32_t atomic_props_crtc[IGT_NUM_CRTC_PROPS];
-
- uint64_t background; /* Background color MSB BGR 16bpc LSB */
- uint32_t background_changed : 1;
- uint32_t background_property;
-
- uint64_t degamma_blob;
- uint32_t degamma_property;
- uint64_t ctm_blob;
- uint32_t ctm_property;
- uint64_t gamma_blob;
- uint32_t gamma_property;
- uint32_t color_mgmt_changed : 1;
+ uint64_t changed;
+ uint32_t props[IGT_NUM_CRTC_PROPS];
+ uint64_t values[IGT_NUM_CRTC_PROPS];
uint32_t crtc_id;
- uint64_t mode_blob;
- bool mode_changed;
-
int32_t out_fence_fd;
- bool out_fence_requested;
};
typedef struct {
@@ -527,17 +513,6 @@ static inline bool igt_output_is_connected(igt_output_t *output)
igt_plane_set_prop_changed(plane, prop); \
} while (0)
-/**
- * igt_atomic_populate_crtc_req:
- * @req: A pointer to drmModeAtomicReq
- * @pipe: A pointer igt_pipe_t
- * @prop: one of igt_atomic_crtc_properties
- * @value: the value to add
- */
-#define igt_atomic_populate_crtc_req(req, pipe, prop, value) \
- igt_assert_lt(0, drmModeAtomicAddProperty(req, pipe->crtc_id,\
- pipe->atomic_props_crtc[prop], value))
-
#define igt_output_is_prop_changed(output, prop) \
(!!((output)->changed & (1 << (prop))))
#define igt_output_set_prop_changed(output, prop) \
@@ -552,26 +527,34 @@ static inline bool igt_output_is_connected(igt_output_t *output)
igt_output_set_prop_changed(output, prop); \
} while (0)
-/*
- * igt_pipe_refresh:
- * @display: a pointer to an #igt_display_t structure
- * @pipe: Pipe to refresh
- * @force: Should be set to true if mode_blob is no longer considered
- * to be valid, for example after doing an atomic commit during fork or closing display fd.
- *
- * Requests the pipe to be part of the state on next update.
- * This is useful when state may have been out of sync after
- * a fork, or we just want to be sure the pipe is included
- * in the next commit.
- */
-static inline void
-igt_pipe_refresh(igt_display_t *display, enum pipe pipe, bool force)
-{
- if (force)
- display->pipes[pipe].mode_blob = 0;
+#define igt_pipe_obj_is_prop_changed(pipe_obj, prop) \
+ (!!((pipe_obj)->changed & (1 << (prop))))
- display->pipes[pipe].mode_changed = true;
-}
+#define igt_pipe_is_prop_changed(display, pipe, prop) \
+ igt_pipe_obj_is_prop_changed(&(display)->pipes[(pipe)], prop)
+
+#define igt_pipe_obj_set_prop_changed(pipe_obj, prop) \
+ (pipe_obj)->changed |= 1 << (prop)
+
+#define igt_pipe_set_prop_changed(display, pipe, prop) \
+ igt_pipe_obj_set_prop_changed(&(display)->pipes[(pipe)], prop)
+
+#define igt_pipe_obj_clear_prop_changed(pipe_obj, prop) \
+ (pipe_obj)->changed &= ~(1 << (prop))
+
+#define igt_pipe_clear_prop_changed(display, pipe, prop) \
+ igt_pipe_obj_clear_prop_changed(&(display)->pipes[(pipe)], prop)
+
+#define igt_pipe_obj_set_prop_value(pipe_obj, prop, value) \
+ do { \
+ (pipe_obj)->values[prop] = (value); \
+ igt_pipe_obj_set_prop_changed(pipe_obj, prop); \
+ } while (0)
+
+#define igt_pipe_set_prop_value(display, pipe, prop, value) \
+ igt_pipe_obj_set_prop_value(&(display)->pipes[(pipe)], prop, value)
+
+void igt_pipe_refresh(igt_display_t *display, enum pipe pipe, bool force);
void igt_enable_connectors(void);
void igt_reset_connectors(void);
diff --git a/tests/kms_atomic_interruptible.c b/tests/kms_atomic_interruptible.c
index 5570854390ea..dd753b6e5db3 100644
--- a/tests/kms_atomic_interruptible.c
+++ b/tests/kms_atomic_interruptible.c
@@ -156,8 +156,8 @@ static void run_plane_test(igt_display_t *display, enum pipe pipe, igt_output_t
uint32_t count_props[3] = { 2, 1, 6 };
uint32_t props[] = {
/* crtc: 2 props */
- plane->pipe->atomic_props_crtc[IGT_CRTC_MODE_ID],
- plane->pipe->atomic_props_crtc[IGT_CRTC_ACTIVE],
+ plane->pipe->props[IGT_CRTC_MODE_ID],
+ plane->pipe->props[IGT_CRTC_ACTIVE],
/* connector: 1 prop */
output->props[IGT_CONNECTOR_CRTC_ID],
/* plane: remainder props */
diff --git a/tests/kms_atomic_transition.c b/tests/kms_atomic_transition.c
index 2ae75f2d6630..7ddb65cea183 100644
--- a/tests/kms_atomic_transition.c
+++ b/tests/kms_atomic_transition.c
@@ -633,7 +633,7 @@ static unsigned set_combinations(igt_display_t *display, unsigned mask, struct i
drmModeModeInfo *mode = NULL;
if (!(mask & (1 << pipe))) {
- if (display->pipes[pipe].mode_blob) {
+ if (igt_pipe_is_prop_changed(display, pipe, IGT_CRTC_ACTIVE)) {
event_mask |= 1 << pipe;
igt_plane_set_fb(plane, NULL);
}
diff --git a/tests/kms_crtc_background_color.c b/tests/kms_crtc_background_color.c
index e12e163449f8..659a30b90219 100644
--- a/tests/kms_crtc_background_color.c
+++ b/tests/kms_crtc_background_color.c
@@ -137,7 +137,7 @@ static void test_crtc_background(data_t *data)
igt_output_set_pipe(output, pipe);
plane = igt_output_get_plane_type(output, DRM_PLANE_TYPE_PRIMARY);
- igt_require(plane->pipe->background_property);
+ igt_require(plane->pipe->props[IGT_CRTC_BACKGROUND]);
prepare_crtc(data, output, pipe, plane, 1, PURPLE, BLACK64);
--
2.14.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
