[PATCH i-g-t v2 06/15] igt_kms: Change PIPE_ANY behavior to mean unassigned

Maarten Lankhorst
2016-07-06T09:56:02Z

None of the tests requires that a output bound to PIPE_ANY is assigned,
so don't do it. Fix the display commit to iterate over crtc's instead
of outputs to properly disable pipes without outputs.

This also means that output->valid is only set after connecting a
output to a pipe, so no longer depend on it in for_each_connected_output
and similar macros.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
lib/igt_kms.c | 246 ++++++++++++++++++++++++++++------------------------------
lib/igt_kms.h | 18 ++++-
2 files changed, 135 insertions(+), 129 deletions(-)

diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index ce8aa2455348..88cae7d51787 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -869,18 +869,20 @@ static bool _kmstest_connector_config(int drm_fd, uint32_t connector_id,
*/
_kmstest_connector_config_crtc_mask(drm_fd, connector, config);

+ if (!kmstest_get_connector_default_mode(drm_fd, connector,
+ &config->default_mode))
+ goto err3;
+
+ config->connector = connector;
+
crtc_idx_mask &= config->valid_crtc_idx_mask;
if (!crtc_idx_mask)
- goto err3;
+ /* Keep config->connector */
+ goto err2;

config->pipe = ffs(crtc_idx_mask) - 1;

- if (!kmstest_get_connector_default_mode(drm_fd, connector,
- &config->default_mode))
- goto err3;
-
config->encoder = _kmstest_connector_config_find_encoder(drm_fd, connector, config->pipe);
- config->connector = connector;
config->crtc = drmModeGetCrtc(drm_fd, resources->crtcs[config->pipe]);

drmModeFreeResources(resources);
@@ -940,8 +942,13 @@ bool kmstest_probe_connector_config(int drm_fd, uint32_t connector_id,
void kmstest_free_connector_config(struct kmstest_connector_config *config)
{
drmModeFreeCrtc(config->crtc);
+ config->crtc = NULL;
+
drmModeFreeEncoder(config->encoder);
+ config->encoder = NULL;
+
drmModeFreeConnector(config->connector);
+ config->connector = NULL;
}

/**
@@ -1197,8 +1204,7 @@ static void igt_output_refresh(igt_output_t *output)
/* we mask out the pipes already in use */
crtc_idx_mask = output->pending_crtc_idx_mask & ~display->pipes_in_use;

- if (output->valid)
- kmstest_free_connector_config(&output->config);
+ kmstest_free_connector_config(&output->config);

ret = kmstest_get_connector_config(display->drm_fd,
output->id,
@@ -1209,19 +1215,19 @@ static void igt_output_refresh(igt_output_t *output)
else
output->valid = false;

- if (!output->valid)
- return;
-
- if (output->use_override_mode)
- output->config.default_mode = output->override_mode;
-
- if (!output->name) {
+ if (!output->name && output->config.connector) {
drmModeConnector *c = output->config.connector;

igt_assert_neq(asprintf(&output->name, "%s-%d", kmstest_connector_type_str(c->connector_type), c->connector_type_id),
-1);
}

+ if (!output->valid)
+ return;
+
+ if (output->use_override_mode)
+ output->config.default_mode = output->override_mode;
+
LOG(display, "%s: Selecting pipe %s\n", output->name,
kmstest_pipe_name(output->config.pipe));

@@ -1259,10 +1265,10 @@ get_crtc_property(int drm_fd, uint32_t crtc_id, const char *name,
}

static void
-igt_crtc_set_property(igt_output_t *output, uint32_t prop_id, uint64_t value)
+igt_crtc_set_property(igt_pipe_t *pipe, uint32_t prop_id, uint64_t value)
{
- drmModeObjectSetProperty(output->display->drm_fd,
- output->config.crtc->crtc_id, DRM_MODE_OBJECT_CRTC, prop_id, value);
+ drmModeObjectSetProperty(pipe->display->drm_fd,
+ pipe->crtc_id, DRM_MODE_OBJECT_CRTC, prop_id, value);
}

/*
@@ -1325,15 +1331,37 @@ void igt_display_init(igt_display_t *display, int drm_fd)
int p = IGT_PLANE_2;
int j, type;
uint8_t n_planes = 0;
+ uint64_t prop_value;

pipe->crtc_id = resources->crtcs[i];
pipe->display = display;
pipe->pipe = i;

+ get_crtc_property(display->drm_fd, pipe->crtc_id,
+ "background_color",
+ &pipe->background_property,
+ &prop_value,
+ 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);
+
/* add the planes that can be used with that pipe */
for (j = 0; j < plane_resources->count_planes; j++) {
drmModePlane *drm_plane;
- uint64_t prop_value;

drm_plane = drmModeGetPlane(display->drm_fd,
plane_resources->planes[j]);
@@ -1440,47 +1468,17 @@ void igt_display_init(igt_display_t *display, int drm_fd)
igt_assert(display->outputs);

for (i = 0; i < display->n_outputs; i++) {
- int j;
igt_output_t *output = &display->outputs[i];

/*
- * We're free to select any pipe to drive that output until
- * a constraint is set with igt_output_set_pipe().
+ * We don't assign each output a pipe unless
+ * a pipe is set with igt_output_set_pipe().
*/
- output->pending_crtc_idx_mask = -1UL;
+ output->pending_crtc_idx_mask = 0;
output->id = resources->connectors[i];
output->display = display;

igt_output_refresh(output);
-
- for (j = 0; j < display->n_pipes; j++) {
- uint64_t prop_value;
- igt_pipe_t *pipe = &display->pipes[j];
-
- if (output->config.crtc) {
- get_crtc_property(display->drm_fd, output->config.crtc->crtc_id,
- "background_color",
- &pipe->background_property,
- &prop_value,
- NULL);
- pipe->background = (uint32_t)prop_value;
- get_crtc_property(display->drm_fd, output->config.crtc->crtc_id,
- "DEGAMMA_LUT",
- &pipe->degamma_property,
- NULL,
- NULL);
- get_crtc_property(display->drm_fd, output->config.crtc->crtc_id,
- "CTM",
- &pipe->ctm_property,
- NULL,
- NULL);
- get_crtc_property(display->drm_fd, output->config.crtc->crtc_id,
- "GAMMA_LUT",
- &pipe->gamma_property,
- NULL,
- NULL);
- }
- }
}

drmModeFreePlaneResources(plane_resources);
@@ -1545,7 +1543,7 @@ static void igt_display_refresh(igt_display_t *display)
for (i = 0; i < display->n_outputs; i++) {
igt_output_t *a = &display->outputs[i];

- if (a->pending_crtc_idx_mask == -1UL)
+ if (!a->pending_crtc_idx_mask)
continue;

for (j = 0; j < display->n_outputs; j++) {
@@ -1554,9 +1552,6 @@ static void igt_display_refresh(igt_display_t *display)
if (i == j)
continue;

- if (b->pending_crtc_idx_mask == -1UL)
- continue;
-
igt_assert_f(a->pending_crtc_idx_mask !=
b->pending_crtc_idx_mask,
"%s and %s are both trying to use pipe %s\n",
@@ -1565,25 +1560,9 @@ static void igt_display_refresh(igt_display_t *display)
}
}

- /*
- * The pipe allocation has to be done in two phases:
- * - first, try to satisfy the outputs where a pipe has been specified
- * - then, allocate the outputs with PIPE_ANY
- */
for (i = 0; i < display->n_outputs; i++) {
igt_output_t *output = &display->outputs[i];

- if (output->pending_crtc_idx_mask == -1UL)
- continue;
-
- igt_output_refresh(output);
- }
- for (i = 0; i < display->n_outputs; i++) {
- igt_output_t *output = &display->outputs[i];
-
- if (output->pending_crtc_idx_mask != -1UL)
- continue;
-
igt_output_refresh(output);
}
}
@@ -1593,12 +1572,11 @@ static igt_pipe_t *igt_output_get_driving_pipe(igt_output_t *output)
igt_display_t *display = output->display;
enum pipe pipe;

- if (output->pending_crtc_idx_mask == -1UL) {
+ if (!output->pending_crtc_idx_mask) {
/*
- * The user hasn't specified a pipe to use, take the one
- * configured by the last refresh()
+ * The user hasn't specified a pipe to use, return none.
*/
- pipe = output->config.pipe;
+ return NULL;
} else {
/*
* Otherwise, return the pending pipe (ie the pipe that should
@@ -1628,6 +1606,21 @@ static igt_plane_t *igt_pipe_get_plane(igt_pipe_t *pipe, enum igt_plane plane)
return &pipe->planes[idx];
}

+static igt_output_t *igt_pipe_get_output(igt_pipe_t *pipe)
+{
+ igt_display_t *display = pipe->display;
+ int i;
+
+ for (i = 0; i < display->n_outputs; i++) {
+ igt_output_t *output = &display->outputs[i];
+
+ if (output->pending_crtc_idx_mask == (1 << pipe->pipe))
+ return output;
+ }
+
+ return NULL;
+}
+
bool igt_pipe_get_property(igt_pipe_t *pipe, const char *name,
uint32_t *prop_id, uint64_t *value,
drmModePropertyPtr *prop)
@@ -1741,10 +1734,10 @@ igt_atomic_prepare_plane_commit(igt_plane_t *plane, igt_output_t *output,
* tests that expect a specific error code).
*/
static int igt_drm_plane_commit(igt_plane_t *plane,
- igt_output_t *output,
+ igt_pipe_t *pipe,
bool fail_on_error)
{
- igt_display_t *display = output->display;
+ igt_display_t *display = pipe->display;
uint32_t fb_id, crtc_id;
int ret;
uint32_t src_x;
@@ -1763,13 +1756,12 @@ static int igt_drm_plane_commit(igt_plane_t *plane,
!plane->rotation_changed);

fb_id = igt_plane_get_fb_id(plane);
- crtc_id = output->config.crtc->crtc_id;
+ crtc_id = pipe->crtc_id;

if ((plane->fb_changed || plane->size_changed) && fb_id == 0) {
LOG(display,
- "%s: SetPlane pipe %s, plane %d, disabling\n",
- igt_output_name(output),
- kmstest_pipe_name(output->config.pipe),
+ "SetPlane pipe %s, plane %d, disabling\n",
+ kmstest_pipe_name(pipe->pipe),
plane->index);

ret = drmModeSetPlane(display->drm_fd,
@@ -1797,10 +1789,9 @@ static int igt_drm_plane_commit(igt_plane_t *plane,
crtc_h = plane->crtc_h;

LOG(display,
- "%s: SetPlane %s.%d, fb %u, src = (%d, %d) "
+ "SetPlane %s.%d, fb %u, src = (%d, %d) "
"%ux%u dst = (%u, %u) %ux%u\n",
- igt_output_name(output),
- kmstest_pipe_name(output->config.pipe),
+ kmstest_pipe_name(pipe->pipe),
plane->index,
fb_id,
src_x >> 16, src_y >> 16, src_w >> 16, src_h >> 16,
@@ -1841,11 +1832,11 @@ static int igt_drm_plane_commit(igt_plane_t *plane,
* code).
*/
static int igt_cursor_commit_legacy(igt_plane_t *cursor,
- igt_output_t *output,
+ igt_pipe_t *pipe,
bool fail_on_error)
{
- igt_display_t *display = output->display;
- uint32_t crtc_id = output->config.crtc->crtc_id;
+ igt_display_t *display = pipe->display;
+ uint32_t crtc_id = pipe->crtc_id;
int ret;

if (cursor->fb_changed) {
@@ -1853,9 +1844,8 @@ static int igt_cursor_commit_legacy(igt_plane_t *cursor,

if (gem_handle) {
LOG(display,
- "%s: SetCursor pipe %s, fb %u %dx%d\n",
- igt_output_name(output),
- kmstest_pipe_name(output->config.pipe),
+ "SetCursor pipe %s, fb %u %dx%d\n",
+ kmstest_pipe_name(pipe->pipe),
gem_handle,
cursor->crtc_w, cursor->crtc_h);

@@ -1865,9 +1855,8 @@ static int igt_cursor_commit_legacy(igt_plane_t *cursor,
cursor->crtc_h);
} else {
LOG(display,
- "%s: SetCursor pipe %s, disabling\n",
- igt_output_name(output),
- kmstest_pipe_name(output->config.pipe));
+ "SetCursor pipe %s, disabling\n",
+ kmstest_pipe_name(pipe->pipe));

ret = drmModeSetCursor(display->drm_fd, crtc_id,
0, 0, 0);
@@ -1883,9 +1872,8 @@ static int igt_cursor_commit_legacy(igt_plane_t *cursor,
int y = cursor->crtc_y;

LOG(display,
- "%s: MoveCursor pipe %s, (%d, %d)\n",
- igt_output_name(output),
- kmstest_pipe_name(output->config.pipe),
+ "MoveCursor pipe %s, (%d, %d)\n",
+ kmstest_pipe_name(pipe->pipe),
x, y);

ret = drmModeMoveCursor(display->drm_fd, crtc_id, x, y);
@@ -1902,10 +1890,11 @@ static int igt_cursor_commit_legacy(igt_plane_t *cursor,
* (setmode).
*/
static int igt_primary_plane_commit_legacy(igt_plane_t *primary,
- igt_output_t *output,
+ igt_pipe_t *pipe,
bool fail_on_error)
{
struct igt_display *display = primary->pipe->display;
+ igt_output_t *output = igt_pipe_get_output(pipe);
drmModeModeInfo *mode;
uint32_t fb_id, crtc_id;
int ret;
@@ -1920,7 +1909,7 @@ static int igt_primary_plane_commit_legacy(igt_plane_t *primary,
!primary->size_changed && !primary->panning_changed)
return 0;

- crtc_id = output->config.crtc->crtc_id;
+ crtc_id = pipe->crtc_id;
fb_id = igt_plane_get_fb_id(primary);
if (fb_id)
mode = igt_output_get_mode(output);
@@ -1932,7 +1921,7 @@ static int igt_primary_plane_commit_legacy(igt_plane_t *primary,
"%s: SetCrtc pipe %s, fb %u, panning (%d, %d), "
"mode %dx%d\n",
igt_output_name(output),
- kmstest_pipe_name(output->config.pipe),
+ kmstest_pipe_name(pipe->pipe),
fb_id,
primary->pan_x, primary->pan_y,
mode->hdisplay, mode->vdisplay);
@@ -1946,9 +1935,8 @@ static int igt_primary_plane_commit_legacy(igt_plane_t *primary,
mode);
} else {
LOG(display,
- "%s: SetCrtc pipe %s, disabling\n",
- igt_output_name(output),
- kmstest_pipe_name(output->config.pipe));
+ "SetCrtc pipe %s, disabling\n",
+ kmstest_pipe_name(pipe->pipe));

ret = drmModeSetCrtc(display->drm_fd,
crtc_id,
@@ -1976,17 +1964,17 @@ static int igt_primary_plane_commit_legacy(igt_plane_t *primary,
* which API is used to do the programming.
*/
static int igt_plane_commit(igt_plane_t *plane,
- igt_output_t *output,
+ igt_pipe_t *pipe,
enum igt_commit_style s,
bool fail_on_error)
{
if (plane->is_cursor && s == COMMIT_LEGACY) {
- return igt_cursor_commit_legacy(plane, output, fail_on_error);
+ return igt_cursor_commit_legacy(plane, pipe, fail_on_error);
} else if (plane->is_primary && s == COMMIT_LEGACY) {
- return igt_primary_plane_commit_legacy(plane, output,
+ return igt_primary_plane_commit_legacy(plane, pipe,
fail_on_error);
} else {
- return igt_drm_plane_commit(plane, output, fail_on_error);
+ return igt_drm_plane_commit(plane, pipe, fail_on_error);
}
}

@@ -2000,31 +1988,28 @@ static int igt_plane_commit(igt_plane_t *plane,
* further programming will take place, which may result in some changes
* taking effect and others not taking effect.
*/
-static int igt_output_commit(igt_output_t *output,
- enum igt_commit_style s,
- bool fail_on_error)
+static int igt_pipe_commit(igt_pipe_t *pipe,
+ enum igt_commit_style s,
+ bool fail_on_error)
{
- igt_display_t *display = output->display;
- igt_pipe_t *pipe;
+ igt_display_t *display = pipe->display;
int i;
int ret;
bool need_wait_for_vblank = false;

- pipe = igt_output_get_driving_pipe(output);
-
if (pipe->background_changed) {
- igt_crtc_set_property(output, pipe->background_property,
+ igt_crtc_set_property(pipe, pipe->background_property,
pipe->background);

pipe->background_changed = false;
}

if (pipe->color_mgmt_changed) {
- igt_crtc_set_property(output, pipe->degamma_property,
+ igt_crtc_set_property(pipe, pipe->degamma_property,
pipe->degamma_blob);
- igt_crtc_set_property(output, pipe->ctm_property,
+ igt_crtc_set_property(pipe, pipe->ctm_property,
pipe->ctm_blob);
- igt_crtc_set_property(output, pipe->gamma_property,
+ igt_crtc_set_property(pipe, pipe->gamma_property,
pipe->gamma_blob);

pipe->color_mgmt_changed = false;
@@ -2036,7 +2021,7 @@ static int igt_output_commit(igt_output_t *output,
if (plane->fb_changed || plane->position_changed || plane->size_changed)
need_wait_for_vblank = true;

- ret = igt_plane_commit(plane, output, s, fail_on_error);
+ ret = igt_plane_commit(plane, pipe, s, fail_on_error);
CHECK_RETURN(ret, fail_on_error);
}

@@ -2060,6 +2045,9 @@ static void igt_atomic_prepare_crtc_commit(igt_output_t *output, drmModeAtomicRe

igt_pipe_t *pipe_obj = igt_output_get_driving_pipe(output);

+ if (!pipe_obj)
+ return;
+
if (pipe_obj->background_changed)
igt_atomic_populate_crtc_req(req, output, IGT_CRTC_BACKGROUND, pipe_obj->background);

@@ -2125,6 +2113,8 @@ static int igt_atomic_commit(igt_display_t *display)

pipe_obj = igt_output_get_driving_pipe(output);
+ if (!pipe_obj)
+ continue;

pipe = pipe_obj->pipe;

@@ -2167,14 +2157,14 @@ static int do_display_commit(igt_display_t *display,

}

- for (i = 0; i < display->n_outputs; i++) {
- igt_output_t *output = &display->outputs[i];
+ for_each_pipe(display, i) {
+ igt_pipe_t *pipe_obj = &display->pipes[i];
+ igt_output_t *output = igt_pipe_get_output(pipe_obj);

- if (!output->valid)
- continue;
+ if (output && output->valid)
+ valid_outs++;

- valid_outs++;
- ret = igt_output_commit(output, s, fail_on_error);
+ ret = igt_pipe_commit(pipe_obj, s, fail_on_error);
CHECK_RETURN(ret, fail_on_error);
}

@@ -2282,9 +2272,9 @@ void igt_output_set_pipe(igt_output_t *output, enum pipe pipe)
{
igt_display_t *display = output->display;

- if (pipe == PIPE_ANY) {
+ if (pipe == PIPE_NONE) {
LOG(display, "%s: set_pipe(any)\n", igt_output_name(output));
- output->pending_crtc_idx_mask = -1UL;
+ output->pending_crtc_idx_mask = 0;
} else {
LOG(display, "%s: set_pipe(%s)\n", igt_output_name(output),
kmstest_pipe_name(pipe));
@@ -2297,6 +2287,8 @@ igt_plane_t *igt_output_get_plane(igt_output_t *output, enum igt_plane plane)
igt_pipe_t *pipe;

pipe = igt_output_get_driving_pipe(output);
+ igt_assert(pipe);
+
return igt_pipe_get_plane(pipe, plane);
}

diff --git a/lib/igt_kms.h b/lib/igt_kms.h
index dc6be5e53b74..3531dc20b6e0 100644
--- a/lib/igt_kms.h
+++ b/lib/igt_kms.h
@@ -334,17 +334,31 @@ void igt_fb_set_size(struct igt_fb *fb, igt_plane_t *plane,

void igt_wait_for_vblank(int drm_fd, enum pipe pipe);

+static inline bool igt_output_is_connected(igt_output_t *output)
+{
+ /* Something went wrong during probe? */
+ if (!output->config.connector)
+ return false;
+
+ if (output->config.connector->connection == DRM_MODE_CONNECTED)
+ return true;
+
+ return false;
+}
+
static inline bool igt_pipe_connector_valid(enum pipe pipe,
igt_output_t *output)
{
- return output->valid && (output->config.valid_crtc_idx_mask & (1 << pipe));
+ return igt_output_is_connected(output) &&
+ (output->config.valid_crtc_idx_mask & (1 << pipe));
}

#define for_each_if(condition) if (!(condition)) {} else

#define for_each_connected_output(display, output) \
for (int i__ = 0; i__ < (display)->n_outputs; i__++) \
- for_each_if (((output = &(display)->outputs[i__]), output->valid))
+ for_each_if (((output = &(display)->outputs[i__]), \
+ igt_output_is_connected(output)))

#define for_each_pipe(display, pipe) \
for (pipe = 0; pipe < igt_display_get_n_pipes(display); pipe++) \