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 <robert.foss@xxxxxxxxxxxxx>
> > > ---
> > > 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
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
