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

2014-07-11 Thread Daniel Vetter 
On Thu, Jul 10, 2014 at 07:00:06PM +0100, Damien Lespiau wrote:
 Signed-off-by: Damien Lespiau damien.lesp...@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

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

2014-07-11 Thread Daniel Vetter 
On Thu, Jul 10, 2014 at 07:00:06PM +0100, Damien Lespiau wrote:
 +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;

Should we also add the flip X/Y bits, even if we currently don't support
this in the kernel?
-Daniel

 +
  #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
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

2014-07-11 Thread Chris Wilson 
On Fri, Jul 11, 2014 at 08:40:29AM +0200, Daniel Vetter wrote:
 On Thu, Jul 10, 2014 at 07:00:06PM +0100, Damien Lespiau wrote:
  +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;
 
 Should we also add the flip X/Y bits, even if we currently don't support
 this in the kernel?

Presumably there are tests in here to check the kernel rejects
unsupported combination of rotations and unknown reflections? In which,
yes.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

2014-07-11 Thread Daniel Vetter 
On Fri, Jul 11, 2014 at 07:42:56AM +0100, Chris Wilson wrote:
 On Fri, Jul 11, 2014 at 08:40:29AM +0200, Daniel Vetter wrote:
  On Thu, Jul 10, 2014 at 07:00:06PM +0100, Damien Lespiau wrote:
   +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;
  
  Should we also add the flip X/Y bits, even if we currently don't support
  this in the kernel?
 
 Presumably there are tests in here to check the kernel rejects
 unsupported combination of rotations and unknown reflections? In which,
 yes.

I guess we should try everything and check whether it either gets rejected
or works properly ...
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

2014-07-11 Thread Damien Lespiau 
On Fri, Jul 11, 2014 at 08:37:42AM +0200, Daniel Vetter wrote:
 On Thu, Jul 10, 2014 at 07:00:06PM +0100, Damien Lespiau wrote:
  Signed-off-by: Damien Lespiau damien.lesp...@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.

That'd be handy. the only issue is that you'd have to do a lookup of the
property id each time or create a prop_name - prop_id map in the
different objects. Also, we can provide a bit more typing than a
uint64_t for the value. Trade-offs, trade-offs, ...

-- 
Damien
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx