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.lankho...@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++) \
-- 
2.5.5

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to