[Intel-gfx] [PATCH i-g-t v1 04/32] lib/igt_kms: Implement dynamic plane count support
Robert Foss
robert.foss at collabora.com
Mon Jan 23 15:47:02 UTC 2017
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.foss at 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);
More information about the Intel-gfx
mailing list