Re: [PATCH i-g-t 05/43] igt_kms: Add support for setting plane rotation

[Daniel Vetter <daniel@ffwll.ch>]

On Thu, Jul 10, 2014 at 07:00:06PM +0100, Damien Lespiau wrote:
> Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
> ---
>  lib/igt_kms.c | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  lib/igt_kms.h | 11 +++++++++++
>  2 files changed, 65 insertions(+)
> 
> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> index 87f5109..69f9977 100644
> --- a/lib/igt_kms.c
> +++ b/lib/igt_kms.c
> @@ -537,6 +537,16 @@ get_plane_property(igt_display_t *display, uint32_t plane_id, const char *name,
>  			    name, prop_id, value);
>  }
>  
> +static void
> +igt_plane_set_property(igt_plane_t *plane, uint32_t prop_id, uint64_t value)
> +{
> +	igt_pipe_t *pipe = plane->pipe;
> +	igt_display_t *display = pipe->display;
> +
> +	drmModeObjectSetProperty(display->drm_fd, plane->drm_plane->plane_id,
> +				 DRM_MODE_OBJECT_PLANE, prop_id, value);
> +}

I wonder whether we shouldn't have interfaces using the property names and
maybe for enum properties also the strings and let igt do it's thing. But
as long as this is just an internal function it should be good enough.
-Daniel

> +
>  /*
>   * Walk a plane's property list to determine its type.  If we don't
>   * find a type property, then the kernel doesn't support universal
> @@ -882,6 +892,10 @@ static int igt_drm_plane_commit(igt_plane_t *plane,
>  
>  	igt_assert(plane->drm_plane);
>  
> +	/* it's an error to try an unsupported feature */
> +	igt_assert(igt_plane_supports_rotation(plane) ||
> +		   !plane->rotation_changed);
> +
>  	fb_id = igt_plane_get_fb_id(plane);
>  	crtc_id = output->config.crtc->crtc_id;
>  
> @@ -931,6 +945,14 @@ static int igt_drm_plane_commit(igt_plane_t *plane,
>  
>  	plane->fb_changed = false;
>  	plane->position_changed = false;
> +
> +	if (plane->rotation_changed) {
> +		igt_plane_set_property(plane, plane->rotation_property,
> +				       plane->rotation);
> +
> +		plane->rotation_changed = false;
> +	}
> +
>  	return 0;
>  }
>  
> @@ -1013,6 +1035,9 @@ static int igt_primary_plane_commit_legacy(igt_plane_t *primary,
>  	/* Primary planes can't be windowed when using a legacy commit */
>  	igt_assert((primary->crtc_x == 0 && primary->crtc_y == 0));
>  
> +	/* nor rotated */
> +	igt_assert(!primary->rotation_changed);
> +
>  	if (!primary->fb_changed && !primary->position_changed &&
>  	    !primary->panning_changed)
>  		return 0;
> @@ -1304,6 +1329,35 @@ void igt_plane_set_panning(igt_plane_t *plane, int x, int y)
>  	plane->panning_changed = true;
>  }
>  
> +static const char *rotation_name(igt_rotation_t rotation)
> +{
> +	switch (rotation) {
> +	case IGT_ROTATION_0:
> +		return "0°";
> +	case IGT_ROTATION_90:
> +		return "90°";
> +	case IGT_ROTATION_180:
> +		return "180°";
> +	case IGT_ROTATION_270:
> +		return "270°";
> +	default:
> +		igt_assert(0);
> +	}
> +}
> +
> +void igt_plane_set_rotation(igt_plane_t *plane, igt_rotation_t rotation)
> +{
> +	igt_pipe_t *pipe = plane->pipe;
> +	igt_display_t *display = pipe->display;
> +
> +	LOG(display, "%c.%d: plane_set_rotation(%s)\n", pipe_name(pipe->pipe),
> +	    plane->index, rotation_name(rotation));
> +
> +	plane->rotation = rotation;
> +
> +	plane->rotation_changed = true;
> +}
> +
>  void igt_wait_for_vblank(int drm_fd, enum pipe pipe)
>  {
>  	drmVBlank wait_vbl;
> diff --git a/lib/igt_kms.h b/lib/igt_kms.h
> index 4f3474e..d34bcee 100644
> --- a/lib/igt_kms.h
> +++ b/lib/igt_kms.h
> @@ -71,6 +71,14 @@ enum igt_commit_style {
>  	/* We'll add atomic here eventually. */
>  };
>  
> +typedef enum {
> +	/* this maps to the kernel API */
> +	IGT_ROTATION_0   = 1 << 0,
> +	IGT_ROTATION_90  = 1 << 1,
> +	IGT_ROTATION_180 = 1 << 2,
> +	IGT_ROTATION_270 = 1 << 3,
> +} igt_rotation_t;
> +
>  #include "igt_fb.h"
>  
>  struct kmstest_connector_config {
> @@ -116,6 +124,7 @@ typedef struct {
>  	unsigned int fb_changed       : 1;
>  	unsigned int position_changed : 1;
>  	unsigned int panning_changed  : 1;
> +	unsigned int rotation_changed : 1;
>  	/*
>  	 * drm_plane can be NULL for primary and cursor planes (when not
>  	 * using the atomic modeset API)
> @@ -129,6 +138,7 @@ typedef struct {
>  	int crtc_x, crtc_y;
>  	/* panning offset within the fb */
>  	unsigned int pan_x, pan_y;
> +	igt_rotation_t rotation;
>  } igt_plane_t;
>  
>  struct igt_pipe {
> @@ -184,6 +194,7 @@ static inline bool igt_plane_supports_rotation(igt_plane_t *plane)
>  void igt_plane_set_fb(igt_plane_t *plane, struct igt_fb *fb);
>  void igt_plane_set_position(igt_plane_t *plane, int x, int y);
>  void igt_plane_set_panning(igt_plane_t *plane, int x, int y);
> +void igt_plane_set_rotation(igt_plane_t *plane, igt_rotation_t rotation);
>  
>  void igt_wait_for_vblank(int drm_fd, enum pipe pipe);
>  
> -- 
> 1.8.3.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch