On Mon, Jan 23, 2017 at 10:47:02AM -0500, Robert Foss wrote: > In the subsequent patches I clear up all of the IGT_PLANE_* being missing > issues (which does mean that trunk does not build between the > enum being removed and that the individual test being fixed). > > I could fix this by moving the removal of the enum until after all of the > tests have had their dependency on the enum fixed.
This is how it's done. No deliberately broken commits please. -- Petri Latvala > > Another solution is just to collapse all of the test fixes into this patch. > Which does sound like the worse option to me. > > Rob. > > > > On Fri, 2017-01-20 at 12:45 -0500, Robert Foss wrote: > > > In upcoming drm-misc-next changes, the number of planes per pipe has > > > been increased as more than one primary plane can be associated with > > > a pipe. > > > > > > The simple fix for this would be to simply bump hardcoded value for > > > number of frames per pipe. > > > But a better solution would be to add support for dynamic number of > > > planes per pipe to i-g-t. > > > > > > Signed-off-by: Robert Foss <[email protected]> > > > --- > > > lib/igt_kms.c | 157 +++++++++++++++++++++++++++++++++++++++--------- > > > ---------- > > > lib/igt_kms.h | 32 ++++-------- > > > 2 files changed, 114 insertions(+), 75 deletions(-) > > > > > > diff --git a/lib/igt_kms.c b/lib/igt_kms.c > > > index 8fa40c28..58e62e2b 100644 > > > --- a/lib/igt_kms.c > > > +++ b/lib/igt_kms.c > > > @@ -347,24 +347,17 @@ int kmstest_pipe_to_index(char pipe) > > > * > > > * Returns: String represnting @pipe, e.g. "plane1". > > > */ > > > -const char *kmstest_plane_name(enum igt_plane plane) > > > +const char *kmstest_plane_type_name(int plane_type) > > > { > > > static const char *names[] = { > > > - [IGT_PLANE_1] = "plane1", > > > - [IGT_PLANE_2] = "plane2", > > > - [IGT_PLANE_3] = "plane3", > > > - [IGT_PLANE_4] = "plane4", > > > - [IGT_PLANE_5] = "plane5", > > > - [IGT_PLANE_6] = "plane6", > > > - [IGT_PLANE_7] = "plane7", > > > - [IGT_PLANE_8] = "plane8", > > > - [IGT_PLANE_9] = "plane9", > > > - [IGT_PLANE_CURSOR] = "cursor", > > > + [DRM_PLANE_TYPE_OVERLAY] = "overlay", > > > + [DRM_PLANE_TYPE_PRIMARY] = "primary", > > > + [DRM_PLANE_TYPE_CURSOR] = "cursor", > > > }; > > > > > > - igt_assert(plane < ARRAY_SIZE(names) && names[plane]); > > > + igt_assert(plane_type < ARRAY_SIZE(names) && > > > names[plane_type]); > > > > > > - return names[plane]; > > > + return names[plane_type]; > > > } > > > > > > static const char *mode_stereo_name(const drmModeModeInfo *mode) > > > @@ -1532,14 +1525,17 @@ void igt_display_init(igt_display_t *display, > > > int drm_fd) > > > for (i = 0; i < display->n_pipes; i++) { > > > igt_pipe_t *pipe = &display->pipes[i]; > > > igt_plane_t *plane; > > > - int p = IGT_PLANE_2; > > > + int p = 1; > > > int j, type; > > > - uint8_t n_planes = 0; > > > + uint8_t last_plane = 0, n_planes = 0; > > > uint64_t prop_value; > > > > > > pipe->crtc_id = resources->crtcs[i]; > > > pipe->display = display; > > > pipe->pipe = i; > > > + pipe->plane_cursor = -1; > > > + pipe->plane_primary = -1; > > > + pipe->planes = NULL; > > > > > > get_crtc_property(display->drm_fd, pipe->crtc_id, > > > "background_color", > > > @@ -1565,6 +1561,27 @@ void igt_display_init(igt_display_t *display, > > > int drm_fd) > > > > > > 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++) > > > { > > > + drmModePlane *drm_plane; > > > + > > > + drm_plane = drmModeGetPlane(display->drm_fd, > > > + plane_resources- > > > > planes[j]); > > > + igt_assert(drm_plane); > > > + > > > + if (!(drm_plane->possible_crtcs & (1 << i))) > > > { > > > + drmModeFreePlane(drm_plane); > > > + continue; > > > + } > > > + > > > + n_planes++; > > > + } > > > + > > > + igt_assert_lte(0, n_planes); > > > + pipe->planes = calloc(sizeof(igt_plane_t), > > > n_planes); > > > + igt_assert_f(pipe->planes, "Failed to allocate > > > memory for %d planes\n", n_planes); > > > + last_plane = n_planes - 1; > > > + > > > /* add the planes that can be used with that pipe */ > > > for (j = 0; j < plane_resources->count_planes; j++) > > > { > > > drmModePlane *drm_plane; > > > @@ -1582,21 +1599,24 @@ void igt_display_init(igt_display_t *display, > > > int drm_fd) > > > plane_resources- > > > > planes[j]); > > > switch (type) { > > > case DRM_PLANE_TYPE_PRIMARY: > > > - plane = &pipe- > > > > planes[IGT_PLANE_PRIMARY]; > > > - plane->is_primary = 1; > > > - plane->index = IGT_PLANE_PRIMARY; > > > + if (pipe->plane_primary == -1) { > > > + plane = &pipe->planes[0]; > > > + plane->index = 0; > > > + pipe->plane_primary = 0; > > > + } else { > > > + plane = &pipe->planes[p]; > > > + plane->index = p++; > > > + } > > > break; > > > case DRM_PLANE_TYPE_CURSOR: > > > - /* > > > - * Cursor should be the highest > > > index in our > > > - * internal list, but we don't know > > > what that > > > - * is yet. Just stick it in the > > > last slot > > > - * for now and we'll move it later, > > > if > > > - * necessary. > > > - */ > > > - plane = &pipe- > > > > planes[IGT_PLANE_CURSOR]; > > > - plane->is_cursor = 1; > > > - plane->index = IGT_PLANE_CURSOR; > > > + if (pipe->plane_cursor == -1) { > > > + plane = &pipe- > > > > planes[last_plane]; > > > + plane->index = last_plane; > > > + pipe->plane_cursor = > > > last_plane; > > > + } else { > > > + plane = &pipe->planes[p]; > > > + plane->index = p++; > > > + } > > > display->has_cursor_plane = true; > > > break; > > > default: > > > @@ -1605,7 +1625,7 @@ void igt_display_init(igt_display_t *display, > > > int drm_fd) > > > break; > > > } > > > > > > - n_planes++; > > > + plane->type = type; > > > plane->pipe = pipe; > > > plane->drm_plane = drm_plane; > > > > > > @@ -1626,7 +1646,7 @@ void igt_display_init(igt_display_t *display, > > > int drm_fd) > > > * At the bare minimum, we should expect to have a > > > primary > > > * plane > > > */ > > > - igt_assert(pipe- > > > > planes[IGT_PLANE_PRIMARY].drm_plane); > > > + igt_assert(pipe->planes[pipe- > > > > plane_primary].drm_plane); > > > > > > if (display->has_cursor_plane) { > > > /* > > > @@ -1634,11 +1654,11 @@ void igt_display_init(igt_display_t *display, > > > int drm_fd) > > > * only 1 sprite, that's the wrong slot and > > > we need to > > > * move it down. > > > */ > > > - if (p != IGT_PLANE_CURSOR) { > > > + if (p != last_plane) { > > > pipe->planes[p] = > > > - pipe- > > > > planes[IGT_PLANE_CURSOR]; > > > + pipe->planes[last_plane]; > > > pipe->planes[p].index = p; > > > - memset(&pipe- > > > > planes[IGT_PLANE_CURSOR], 0, > > > + memset(&pipe->planes[last_plane], 0, > > > sizeof *plane); > > > } > > > } else { > > > @@ -1646,7 +1666,7 @@ void igt_display_init(igt_display_t *display, > > > int drm_fd) > > > plane = &pipe->planes[p]; > > > plane->pipe = pipe; > > > plane->index = p; > > > - plane->is_cursor = true; > > > + plane->type = DRM_PLANE_TYPE_CURSOR; > > > } > > > > > > pipe->n_planes = n_planes; > > > @@ -1654,9 +1674,6 @@ void igt_display_init(igt_display_t *display, > > > int drm_fd) > > > for_each_plane_on_pipe(display, i, plane) > > > plane->fb_changed = true; > > > > > > - /* make sure we don't overflow the plane array */ > > > - igt_assert_lte(pipe->n_planes, IGT_MAX_PLANES); > > > - > > > pipe->mode_changed = true; > > > } > > > > > > @@ -1709,6 +1726,9 @@ static void igt_pipe_fini(igt_pipe_t *pipe) > > > plane->drm_plane = NULL; > > > } > > > } > > > + > > > + free(pipe->planes); > > > + pipe->planes = NULL; > > > } > > > > > > static void igt_output_fini(igt_output_t *output) > > > @@ -1797,20 +1817,41 @@ static igt_pipe_t > > > *igt_output_get_driving_pipe(igt_output_t *output) > > > return &display->pipes[pipe]; > > > } > > > > > > -static igt_plane_t *igt_pipe_get_plane(igt_pipe_t *pipe, enum > > > igt_plane plane) > > > +static igt_plane_t *igt_pipe_get_plane(igt_pipe_t *pipe, int > > > plane_idx) > > > { > > > - int idx; > > > + igt_assert_f(plane_idx >= 0 && plane_idx < pipe->n_planes, > > > + "Valid pipe->planes plane_idx not found, > > > plane_idx=%d n_planes=%d", > > > + plane_idx, pipe->n_planes); > > > > > > - /* Cursor plane is always the highest index */ > > > - if (plane == IGT_PLANE_CURSOR) > > > - idx = pipe->n_planes - 1; > > > - else { > > > - igt_assert_f(plane >= 0 && plane < (pipe->n_planes), > > > - "plane=%d\n", plane); > > > - idx = plane; > > > + return &pipe->planes[plane_idx]; > > > +} > > > + > > > + > > > +igt_plane_t *igt_pipe_get_plane_type(igt_pipe_t *pipe, int > > > plane_type) > > > +{ > > > + int i, plane_idx = -1; > > > + > > > + switch(plane_type) { > > > + case DRM_PLANE_TYPE_CURSOR: > > > + plane_idx = pipe->plane_cursor; > > > + break; > > > + case DRM_PLANE_TYPE_PRIMARY: > > > + plane_idx = pipe->plane_primary; > > > + break; > > > + case DRM_PLANE_TYPE_OVERLAY: > > > + for(i = 0; i < pipe->n_planes; i++) > > > + if (pipe->planes[i].type == > > > DRM_PLANE_TYPE_OVERLAY) > > > + plane_idx = i; > > > + break; > > > + default: > > > + break; > > > } > > > > > > - return &pipe->planes[idx]; > > > + igt_assert_f(plane_idx >= 0 && plane_idx < pipe->n_planes, > > > + "Valid pipe->planes idx not found. plane_idx=%d > > > plane_type=%d n_planes=%d\n", > > > + plane_idx, plane_type, pipe->n_planes); > > > + > > > + return &pipe->planes[plane_idx]; > > > } > > > > > > static igt_output_t *igt_pipe_get_output(igt_pipe_t *pipe) > > > @@ -2155,9 +2196,9 @@ static int igt_plane_commit(igt_plane_t *plane, > > > enum igt_commit_style s, > > > bool fail_on_error) > > > { > > > - if (plane->is_cursor && s == COMMIT_LEGACY) { > > > + if (plane->type == DRM_PLANE_TYPE_CURSOR && s == > > > COMMIT_LEGACY) { > > > return igt_cursor_commit_legacy(plane, pipe, > > > fail_on_error); > > > - } else if (plane->is_primary && s == COMMIT_LEGACY) { > > > + } else if (plane->type == DRM_PLANE_TYPE_PRIMARY && s == > > > COMMIT_LEGACY) { > > > return igt_primary_plane_commit_legacy(plane, pipe, > > > fail_on_error > > > ); > > > } else { > > > @@ -2374,7 +2415,9 @@ display_commit_changed(igt_display_t *display, > > > enum igt_commit_style s) > > > plane->position_changed = false; > > > plane->size_changed = false; > > > > > > - if (s != COMMIT_LEGACY || !(plane- > > > > is_primary || plane->is_cursor)) > > > + if (s != COMMIT_LEGACY || > > > + !(plane->type == DRM_PLANE_TYPE_PRIMARY > > > || > > > + plane->type == DRM_PLANE_TYPE_CURSOR)) > > > plane->rotation_changed = false; > > > } > > > } > > > @@ -2654,14 +2697,24 @@ void igt_output_set_scaling_mode(igt_output_t > > > *output, uint64_t scaling_mode) > > > igt_require(output- > > > > config.atomic_props_connector[IGT_CONNECTOR_SCALING_MODE]); > > > } > > > > > > -igt_plane_t *igt_output_get_plane(igt_output_t *output, enum > > > igt_plane plane) > > > +igt_plane_t *igt_output_get_plane(igt_output_t *output, int > > > plane_idx) > > > +{ > > > + igt_pipe_t *pipe; > > > + > > > + pipe = igt_output_get_driving_pipe(output); > > > + igt_assert(pipe); > > > + > > > + return igt_pipe_get_plane(pipe, plane_idx); > > > +} > > > + > > > +igt_plane_t *igt_output_get_plane_type(igt_output_t *output, int > > > plane_type) > > > { > > > igt_pipe_t *pipe; > > > > > > pipe = igt_output_get_driving_pipe(output); > > > igt_assert(pipe); > > > > > > - return igt_pipe_get_plane(pipe, plane); > > > + return igt_pipe_get_plane_type(pipe, plane_type); > > > } > > > > > > void igt_plane_set_fb(igt_plane_t *plane, struct igt_fb *fb) > > > diff --git a/lib/igt_kms.h b/lib/igt_kms.h > > > index 859c79aa..dc172ec9 100644 > > > --- a/lib/igt_kms.h > > > +++ b/lib/igt_kms.h > > > @@ -57,25 +57,7 @@ enum pipe { > > > I915_MAX_PIPES > > > }; > > > const char *kmstest_pipe_name(enum pipe pipe); > > > -int kmstest_pipe_to_index(char pipe); > > > - > > > -/* We namespace this enum to not conflict with the Android > > > i915_drm.h */ > > > -enum igt_plane { > > > - IGT_PLANE_1 = 0, > > > - IGT_PLANE_PRIMARY = IGT_PLANE_1, > > > - IGT_PLANE_2, > > > - IGT_PLANE_3, > > > - IGT_PLANE_4, > > > - IGT_PLANE_5, > > > - IGT_PLANE_6, > > > - IGT_PLANE_7, > > > - IGT_PLANE_8, > > > - IGT_PLANE_9, > > > - IGT_PLANE_CURSOR, /* IGT_PLANE_CURSOR is always the last > > > plane. */ > > > - IGT_MAX_PLANES, > > > -}; > > > - > > > -const char *kmstest_plane_name(enum igt_plane plane); > > > +const char *kmstest_plane_type_name(int plane_type); > > > > > > enum port { > > > PORT_A = 0, > > > @@ -257,8 +239,7 @@ typedef struct { > > > igt_pipe_t *pipe; > > > int index; > > > /* capabilities */ > > > - unsigned int is_primary : 1; > > > - unsigned int is_cursor : 1; > > > + int type; > > > /* state tracking */ > > > unsigned int fb_changed : 1; > > > unsigned int position_changed : 1; > > > @@ -293,8 +274,11 @@ struct igt_pipe { > > > igt_display_t *display; > > > enum pipe pipe; > > > bool enabled; > > > + > > > int n_planes; > > > - igt_plane_t planes[IGT_MAX_PLANES]; > > > + int plane_cursor; > > > + int plane_primary; > > > + igt_plane_t *planes; > > > > > > uint32_t atomic_props_crtc[IGT_NUM_CRTC_PROPS]; > > > > > > @@ -354,7 +338,9 @@ drmModeModeInfo *igt_output_get_mode(igt_output_t > > > *output); > > > void igt_output_override_mode(igt_output_t *output, drmModeModeInfo > > > *mode); > > > void igt_output_set_pipe(igt_output_t *output, enum pipe pipe); > > > void igt_output_set_scaling_mode(igt_output_t *output, uint64_t > > > scaling_mode); > > > -igt_plane_t *igt_output_get_plane(igt_output_t *output, enum > > > igt_plane plane); > > > +igt_plane_t *igt_output_get_plane(igt_output_t *output, int > > > plane_idx); > > > +igt_plane_t *igt_output_get_plane_type(igt_output_t *output, int > > > plane_type); > > > +igt_plane_t *igt_pipe_get_plane_type(igt_pipe_t *pipe, int > > > plane_type); > > > bool igt_pipe_get_property(igt_pipe_t *pipe, const char *name, > > > uint32_t *prop_id, uint64_t *value, > > > drmModePropertyPtr *prop); > _______________________________________________ > Intel-gfx mailing list > [email protected] > https://lists.freedesktop.org/mailman/listinfo/intel-gfx _______________________________________________ Intel-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/intel-gfx
