[Intel-gfx] [PATCH i-g-t v1 04/32] lib/igt_kms: Implement dynamic plane count support

Mika Kahola mika.kahola at intel.com
Mon Jan 23 11:00:33 UTC 2017


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

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);
-- 
Mika Kahola - Intel OTC



More information about the Intel-gfx mailing list