On 2017-01-23 06:00 AM, Mika Kahola wrote:
With this patch applied, I received couple of issues during the
compilation

igt_kms.c:332:5: warning: no previous prototype for
‘kmstest_pipe_to_index’ [-Wmissing-prototypes]
 int kmstest_pipe_to_index(char pipe)

and

kms_fence_pin_leak.c:118:41: error: ‘IGT_PLANE_PRIMARY’ undeclared
(first use in this function)
  primary = igt_output_get_plane(output, IGT_PLANE_PRIMARY);

This would require that we move all IGT_PLANE_PRIMARY references to
DRM_PLANE_TYPE_PRIMARY

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.

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.f...@collabora.com>
---
 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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to