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@xxxxxxxxxxxxxxx> --- 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 8b9c7c9234c6..c1a033992845 100644 --- a/lib/igt_kms.c +++ b/lib/igt_kms.c @@ -864,18 +864,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); @@ -935,8 +937,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; } /** @@ -1192,8 +1199,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, @@ -1204,19 +1210,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)); @@ -1254,10 +1260,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); } /* @@ -1319,15 +1325,37 @@ void igt_display_init(igt_display_t *display, int drm_fd) igt_plane_t *plane; int p = IGT_PLANE_2; int j, type; + 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]); @@ -1433,47 +1461,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); @@ -1538,7 +1536,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++) { @@ -1547,9 +1545,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", @@ -1558,25 +1553,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); } } @@ -1586,12 +1565,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 @@ -1621,6 +1599,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) @@ -1734,10 +1727,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; @@ -1756,13 +1749,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, @@ -1790,10 +1782,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, @@ -1834,11 +1825,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) { @@ -1846,9 +1837,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); @@ -1858,9 +1848,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); @@ -1876,9 +1865,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); @@ -1895,10 +1883,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; @@ -1913,7 +1902,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); @@ -1925,7 +1914,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); @@ -1939,9 +1928,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, @@ -1969,17 +1957,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); } } @@ -1993,31 +1981,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; @@ -2029,7 +2014,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); } @@ -2053,6 +2038,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); @@ -2118,6 +2106,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; @@ -2160,14 +2150,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); } @@ -2275,9 +2265,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)); @@ -2290,6 +2280,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 47b78e1a6b1f..a3505a1a6b25 100644 --- a/lib/igt_kms.h +++ b/lib/igt_kms.h @@ -328,17 +328,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++) \ -- 2.5.5 _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx