On ke, 2016-07-20 at 16:18 +0300, ville.syrjala@linux.intel.com wrote:

From: Ville Syrjälä ville.syrjala@linux.intel.com

We have intel_rotation_90_or_270() in i915 to check if the rotation is 90 or 270 degrees. Similar checks are elsewhere in drm, so let's move the helper into a central place and use it everwhere.

Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com

Reviewed-by: Joonas Lahtinen joonas.lahtinen@linux.intel.com

---

drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c |  4 ++--  drivers/gpu/drm/drm_atomic_helper.c             |  2 +-  drivers/gpu/drm/drm_crtc.c                      |  3 +--  drivers/gpu/drm/i915/intel_atomic_plane.c       |  2 +-  drivers/gpu/drm/i915/intel_display.c            | 10 +++++-----  drivers/gpu/drm/i915/intel_drv.h                |  6 ------  drivers/gpu/drm/i915/intel_fbc.c                |  2 +-  drivers/gpu/drm/i915/intel_pm.c                 | 12 ++++++------  drivers/gpu/drm/i915/intel_sprite.c             |  2 +-  drivers/gpu/drm/omapdrm/omap_plane.c            |  8 ++------  include/drm/drm_crtc.h                          |  5 +++++  11 files changed, 25 insertions(+), 31 deletions(-)

diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c index 016c191221f3..f3350c80704d 100644 --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c @@ -393,7 +393,7 @@ static void atmel_hlcdc_plane_update_format(struct atmel_hlcdc_plane *plane,     if ((state->base.fb->pixel_format == DRM_FORMAT_YUV422 ||        state->base.fb->pixel_format == DRM_FORMAT_NV61) &&

• (state->base.rotation & (BIT(DRM_ROTATE_90) | BIT(DRM_ROTATE_270))))

• drm_rotation_90_or_270(state->base.rotation))

cfg |= ATMEL_HLCDC_YUV422ROT;     atmel_hlcdc_layer_update_cfg(&plane->layer, @@ -628,7 +628,7 @@ static int atmel_hlcdc_plane_atomic_check(struct drm_plane *p,   /*    * Swap width and size in case of 90 or 270 degrees rotation    */

• if (state->base.rotation & (BIT(DRM_ROTATE_90) | BIT(DRM_ROTATE_270))) {

• if (drm_rotation_90_or_270(state->base.rotation)) {

tmp = state->crtc_w;   state->crtc_w = state->crtc_h;   state->crtc_h = tmp; diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index de7fddce3cef..3e12e6e646ad 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -2348,7 +2348,7 @@ int __drm_atomic_helper_set_config(struct drm_mode_set *set,   primary_state->crtc_h = vdisplay;   primary_state->src_x = set->x << 16;   primary_state->src_y = set->y << 16;

• if (primary_state->rotation & (BIT(DRM_ROTATE_90) | BIT(DRM_ROTATE_270))) {

• if (drm_rotation_90_or_270(primary_state->rotation)) {

primary_state->src_w = vdisplay << 16;   primary_state->src_h = hdisplay << 16;   } else { diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index f1d9f0569d7f..986de180e6c2 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -2804,8 +2804,7 @@ int drm_crtc_check_viewport(const struct drm_crtc *crtc,   drm_crtc_get_hv_timing(mode, &hdisplay, &vdisplay);     if (crtc->state &&

• crtc->primary->state->rotation & (BIT(DRM_ROTATE_90) |

• 			      BIT(DRM_ROTATE_270)))
  

• drm_rotation_90_or_270(crtc->primary->state->rotation))

swap(hdisplay, vdisplay);     return check_src_coords(x << 16, y << 16, diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c b/drivers/gpu/drm/i915/intel_atomic_plane.c index 7de7721f65bc..c19eb9a0cd4a 100644 --- a/drivers/gpu/drm/i915/intel_atomic_plane.c +++ b/drivers/gpu/drm/i915/intel_atomic_plane.c @@ -156,7 +156,7 @@ static int intel_plane_atomic_check(struct drm_plane *plane,   intel_state->clip.y2 =   crtc_state->base.enable ? crtc_state->pipe_src_h : 0;

• if (state->fb && intel_rotation_90_or_270(state->rotation)) {

• if (state->fb && drm_rotation_90_or_270(state->rotation)) {

if (!(state->fb->modifier[0] == I915_FORMAT_MOD_Y_TILED ||   state->fb->modifier[0] == I915_FORMAT_MOD_Yf_TILED)) {   DRM_DEBUG_KMS("Y/Yf tiling required for 90/270!\n"); diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 51fbca756cdb..08b9f9a19df0 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -2138,7 +2138,7 @@ intel_fill_fb_ggtt_view(struct i915_ggtt_view *view,   const struct drm_framebuffer *fb,   unsigned int rotation)  {

• if (intel_rotation_90_or_270(rotation)) {

• if (drm_rotation_90_or_270(rotation)) {

*view = i915_ggtt_view_rotated;   view->params.rotated = to_intel_framebuffer(fb)->rot_info;   } else { @@ -2352,7 +2352,7 @@ u32 intel_compute_tile_offset(int *x, int *y,   intel_tile_dims(dev_priv, &tile_width, &tile_height,   fb_modifier, cpp);

• if (intel_rotation_90_or_270(rotation)) {
  

• if (drm_rotation_90_or_270(rotation)) {
  

pitch_tiles = pitch / tile_height;   swap(tile_width, tile_height);   } else { @@ -3011,7 +3011,7 @@ static void skylake_update_primary_plane(struct drm_plane *plane,     WARN_ON(drm_rect_width(&plane_state->src) == 0);

• if (intel_rotation_90_or_270(rotation)) {

• if (drm_rotation_90_or_270(rotation)) {

int cpp = drm_format_plane_cpp(fb->pixel_format, 0);     /* stride = Surface height in tiles */ @@ -4179,7 +4179,7 @@ skl_update_scaler(struct intel_crtc_state *crtc_state, bool force_detach,   to_intel_crtc(crtc_state->base.crtc);   int need_scaling;

• need_scaling = intel_rotation_90_or_270(rotation) ?

• need_scaling = drm_rotation_90_or_270(rotation) ?

(src_h != dst_w || src_w != dst_h):   (src_w != dst_w || src_h != dst_h);   @@ -11414,7 +11414,7 @@ static void skl_do_mmio_flip(struct intel_crtc *intel_crtc,    * The stride is either expressed as a multiple of 64 bytes chunks for    * linear buffers or in number of tiles for tiled buffers.    */

• if (intel_rotation_90_or_270(rotation)) {

• if (drm_rotation_90_or_270(rotation)) {

/* stride = Surface height in tiles */   tile_height = intel_tile_height(dev_priv, fb->modifier[0], 0);   stride = DIV_ROUND_UP(fb->height, tile_height); diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index e74d851868c5..907a72cfdad3 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -1255,12 +1255,6 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state,  unsigned int intel_tile_height(const struct drm_i915_private *dev_priv,          uint64_t fb_modifier, unsigned int cpp);   -static inline bool -intel_rotation_90_or_270(unsigned int rotation) -{

• return rotation & (BIT(DRM_ROTATE_90) | BIT(DRM_ROTATE_270));

-}

void intel_create_rotation_property(struct drm_device *dev,   struct intel_plane *plane);   diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c index 781e2f5f7cd8..f3f3322139e0 100644 --- a/drivers/gpu/drm/i915/intel_fbc.c +++ b/drivers/gpu/drm/i915/intel_fbc.c @@ -84,7 +84,7 @@ static void intel_fbc_get_plane_source_size(struct intel_fbc_state_cache *cache,  {   int w, h;

• if (intel_rotation_90_or_270(cache->plane.rotation)) {

• if (drm_rotation_90_or_270(cache->plane.rotation)) {

w = cache->plane.src_h;   h = cache->plane.src_w;   } else { diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index a1bf5f8fbb1c..53116cdc43a6 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -3004,7 +3004,7 @@ skl_plane_downscale_amount(const struct intel_plane_state *pstate)   src_h = drm_rect_height(&pstate->src);   dst_w = drm_rect_width(&pstate->dst);   dst_h = drm_rect_height(&pstate->dst);

• if (intel_rotation_90_or_270(pstate->base.rotation))

• if (drm_rotation_90_or_270(pstate->base.rotation))

swap(dst_w, dst_h);     downscale_h = max(src_h / dst_h, (uint32_t)DRM_PLANE_HELPER_NO_SCALING); @@ -3035,7 +3035,7 @@ skl_plane_relative_data_rate(const struct intel_crtc_state *cstate,   width = drm_rect_width(&intel_pstate->src) >> 16;   height = drm_rect_height(&intel_pstate->src) >> 16;

• if (intel_rotation_90_or_270(pstate->rotation))

• if (drm_rotation_90_or_270(pstate->rotation))

swap(width, height);     /* for planar format */ @@ -3137,7 +3137,7 @@ skl_ddb_min_alloc(const struct drm_plane_state *pstate,   src_w = drm_rect_width(&intel_pstate->src) >> 16;   src_h = drm_rect_height(&intel_pstate->src) >> 16;

• if (intel_rotation_90_or_270(pstate->rotation))

• if (drm_rotation_90_or_270(pstate->rotation))

swap(src_w, src_h);     /* Halve UV plane width and height for NV12 */ @@ -3151,7 +3151,7 @@ skl_ddb_min_alloc(const struct drm_plane_state *pstate,   else   plane_bpp = drm_format_plane_cpp(fb->pixel_format, 0);

• if (intel_rotation_90_or_270(pstate->rotation)) {

• if (drm_rotation_90_or_270(pstate->rotation)) {

switch (plane_bpp) {   case 1:   min_scanlines = 32; @@ -3407,7 +3407,7 @@ static int skl_compute_plane_wm(const struct drm_i915_private *dev_priv,   width = drm_rect_width(&intel_pstate->src) >> 16;   height = drm_rect_height(&intel_pstate->src) >> 16;

• if (intel_rotation_90_or_270(pstate->rotation))

• if (drm_rotation_90_or_270(pstate->rotation))

swap(width, height);     cpp = drm_format_plane_cpp(fb->pixel_format, 0); @@ -3428,7 +3428,7 @@ static int skl_compute_plane_wm(const struct drm_i915_private *dev_priv,       fb->modifier[0] == I915_FORMAT_MOD_Yf_TILED) {   uint32_t min_scanlines = 4;   uint32_t y_tile_minimum;

• if (intel_rotation_90_or_270(pstate->rotation)) {
  

• if (drm_rotation_90_or_270(pstate->rotation)) {
  

int cpp = (fb->pixel_format == DRM_FORMAT_NV12) ?   drm_format_plane_cpp(fb->pixel_format, 1) :   drm_format_plane_cpp(fb->pixel_format, 0); diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c index 0de935ad01c2..cc6af1410d67 100644 --- a/drivers/gpu/drm/i915/intel_sprite.c +++ b/drivers/gpu/drm/i915/intel_sprite.c @@ -251,7 +251,7 @@ skl_update_plane(struct drm_plane *drm_plane,     surf_addr = intel_plane_obj_offset(intel_plane, obj, 0);

• if (intel_rotation_90_or_270(rotation)) {

• if (drm_rotation_90_or_270(rotation)) {

int cpp = drm_format_plane_cpp(fb->pixel_format, 0);     /* stride: Surface height in tiles */ diff --git a/drivers/gpu/drm/omapdrm/omap_plane.c b/drivers/gpu/drm/omapdrm/omap_plane.c index 5252ab720e70..7f0d567b8d67 100644 --- a/drivers/gpu/drm/omapdrm/omap_plane.c +++ b/drivers/gpu/drm/omapdrm/omap_plane.c @@ -108,16 +108,12 @@ static void omap_plane_atomic_update(struct drm_plane *plane,   win.src_x = state->src_x >> 16;   win.src_y = state->src_y >> 16;

• switch (state->rotation & DRM_ROTATE_MASK) {
• case BIT(DRM_ROTATE_90):
• case BIT(DRM_ROTATE_270):

• if (drm_rotation_90_or_270(state->rotation)) {

win.src_w = state->src_h >> 16;   win.src_h = state->src_w >> 16;

• break;
  

• default:

• } else {

win.src_w = state->src_w >> 16;   win.src_h = state->src_h >> 16;

• break;
  

}     /* update scanout: */ diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index 3edeaf88ebc0..b7ac7dcf52db 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -92,6 +92,11 @@ static inline uint64_t I642U64(int64_t val)  #define DRM_REFLECT_X 4  #define DRM_REFLECT_Y 5   +static inline bool drm_rotation_90_or_270(unsigned int rotation) +{

• return rotation & (BIT(DRM_ROTATE_90) | BIT(DRM_ROTATE_270));

+}

enum drm_connector_force {   DRM_FORCE_UNSPECIFIED,   DRM_FORCE_OFF,

On Wed, Jul 20, 2016 at 04:51:57PM +0300, Joonas Lahtinen wrote:

On ke, 2016-07-20 at 16:18 +0300, ville.syrjala@linux.intel.com wrote:

...

From: Ville Syrjälä ville.syrjala@linux.intel.com

Not all planes on the ssytem may support the same rotations/reflections,

s/ssytem/system/

...

so make it possible to create a separate property for each plane. This way userspace gets told exactly which rotations/reflections are possible for each plane.

Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com

drivers/gpu/drm/drm_atomic.c    | 6 ++++--  drivers/gpu/drm/drm_fb_helper.c | 6 +++++-  include/drm/drm_crtc.h          | 2 ++  3 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c index 5f0002897862..38b023da6f09 100644 --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -710,7 +710,8 @@ int drm_atomic_plane_set_property(struct drm_plane *plane,   state->src_w = val;   } else if (property == config->prop_src_h) {   state->src_h = val;

• } else if (property == config->rotation_property) {

• } else if (property == config->rotation_property ||

•    property == plane->rotation_property) {
  

if (!is_power_of_2(val & DRM_ROTATE_MASK))   return -EINVAL;   state->rotation = val; @@ -768,7 +769,8 @@ drm_atomic_plane_get_property(struct drm_plane *plane,   *val = state->src_w;   } else if (property == config->prop_src_h) {   *val = state->src_h;

• } else if (property == config->rotation_property) {

• } else if (property == config->rotation_property ||

•    property == plane->rotation_property) {
  

*val = state->rotation;   } else if (plane->funcs->atomic_get_property) {   return plane->funcs->atomic_get_property(plane, state, property, val); diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c index ce54e985d91b..ce536c0553e5 100644 --- a/drivers/gpu/drm/drm_fb_helper.c +++ b/drivers/gpu/drm/drm_fb_helper.c @@ -392,7 +392,11 @@ static int restore_fbdev_mode(struct drm_fb_helper *fb_helper)   if (plane->type != DRM_PLANE_TYPE_PRIMARY)   drm_plane_force_disable(plane);

• if (dev->mode_config.rotation_property) {
  

• if (plane->rotation_property) {
  

• 	drm_mode_plane_set_obj_prop(plane,
  

• 				    plane->rotation_property,
  

• 				    BIT(DRM_ROTATE_0));
  

• } else if (dev->mode_config.rotation_property) {
  

Why not clear the mode_config.rotation_property too, still?

I would be cleared if any plane uses it. Though I'm not sure mixing per-plane and global rotation props in one device makes any real sense.

Or maybe I should just nuke the global property?

Regards, Joonas

...

drm_mode_plane_set_obj_prop(plane,       dev->mode_config.rotation_property,       BIT(DRM_ROTATE_0)); diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index b7ac7dcf52db..8d65e85d6d91 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -1737,6 +1737,8 @@ struct drm_plane {   const struct drm_plane_helper_funcs *helper_private;     struct drm_plane_state *state;

• struct drm_property *rotation_property;

};    /**

-- Joonas Lahtinen Open Source Technology Center Intel Corporation

On ke, 2016-07-20 at 16:18 +0300, ville.syrjala@linux.intel.com wrote:

From: Ville Syrjälä ville.syrjala@linux.intel.com

On certain platforms not all planes support the same set of rotations/reflections, so let's use the per-plane property for this.

This is already a problem on SKL when we use the legay cursor plane as it only supports 0|180 whereas the universal planes support 0|90|180|270, and it will be a problem on CHV soon.

Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com

drivers/gpu/drm/i915/intel_display.c | 51 +++++++++++++++++++-----------------  drivers/gpu/drm/i915/intel_drv.h     |  4 +--  drivers/gpu/drm/i915/intel_sprite.c  | 12 ++++++++-  3 files changed, 40 insertions(+), 27 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 08b9f9a19df0..93ecb259c5ce 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -14217,6 +14217,7 @@ static struct drm_plane *intel_primary_plane_create(struct drm_device *dev,   struct intel_plane *primary = NULL;   struct intel_plane_state *state = NULL;   const uint32_t *intel_primary_formats;

• unsigned int supported_rotations;

unsigned int num_formats;   int ret;   @@ -14289,8 +14290,19 @@ static struct drm_plane *intel_primary_plane_create(struct drm_device *dev,   if (ret)   goto fail;

• if (INTEL_INFO(dev)->gen >= 9) {

• supported_rotations =
  

• 	BIT(DRM_ROTATE_0) | BIT(DRM_ROTATE_90) |
  

• 	BIT(DRM_ROTATE_180) | BIT(DRM_ROTATE_270);
  

• } else if (INTEL_INFO(dev)->gen >= 4) {

• supported_rotations =
  

• 	BIT(DRM_ROTATE_0) | BIT(DRM_ROTATE_180);
  

• } else {

• supported_rotations = BIT(DRM_ROTATE_0);
  

• }

Might this be more informative if it was;

supported_rotations = BIT(DRM_ROTATE_0);

if (gen >= 4) supported_rotations |= BIT(DRM_ROTATE_180);

Then you could exclude too, for special platforms.

Also, use INTEL_GEN()

if (INTEL_INFO(dev)->gen >= 4)

• intel_create_rotation_property(dev, primary);
  

• intel_create_rotation_property(primary, supported_rotations);
  

drm_plane_helper_add(&primary->base, &intel_plane_helper_funcs);   @@ -14303,22 +14315,20 @@ fail:   return NULL;  }   -void intel_create_rotation_property(struct drm_device *dev, struct intel_plane *plane) +void intel_create_rotation_property(struct intel_plane *plane,

• 		    unsigned int supported_rotations)
  

{

• if (!dev->mode_config.rotation_property) {

• unsigned long flags = BIT(DRM_ROTATE_0) |
  

• 	BIT(DRM_ROTATE_180);
  

• struct drm_device *dev = plane->base.dev;

• if (INTEL_INFO(dev)->gen >= 9)
  

• 	flags |= BIT(DRM_ROTATE_90) | BIT(DRM_ROTATE_270);
  

• if (!plane->base.rotation_property)

• plane->base.rotation_property =
  

• 	drm_mode_create_rotation_property(dev,
  

• 					  supported_rotations);
  

• dev->mode_config.rotation_property =
  

• 	drm_mode_create_rotation_property(dev, flags);
  

• }
• if (dev->mode_config.rotation_property)

• if (plane->base.rotation_property)

drm_object_attach_property(&plane->base.base,

• 		dev->mode_config.rotation_property,
  

• 		plane->base.state->rotation);
  

• 			   plane->base.rotation_property,
  

• 			   plane->base.state->rotation);
  

}    static int @@ -14449,17 +14459,10 @@ static struct drm_plane *intel_cursor_plane_create(struct drm_device *dev,   if (ret)   goto fail;

• if (INTEL_INFO(dev)->gen >= 4) {

• if (!dev->mode_config.rotation_property)
  

• 	dev->mode_config.rotation_property =
  

• 		drm_mode_create_rotation_property(dev,
  

• 					BIT(DRM_ROTATE_0) |
  

• 					BIT(DRM_ROTATE_180));
  

• if (dev->mode_config.rotation_property)
  

• 	drm_object_attach_property(&cursor->base.base,
  

• 		dev->mode_config.rotation_property,
  

• 		state->base.rotation);
  

• }

• if (INTEL_INFO(dev)->gen >= 4)

INTEL_GEN() while touching it.

• intel_create_rotation_property(cursor,
  

• 			       BIT(DRM_ROTATE_0) |
  

• 			       BIT(DRM_ROTATE_180));
  

if (INTEL_INFO(dev)->gen >=9)   state->scaler_id = -1; diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 907a72cfdad3..2715aec979fc 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -1255,8 +1255,8 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state,  unsigned int intel_tile_height(const struct drm_i915_private *dev_priv,          uint64_t fb_modifier, unsigned int cpp);   -void intel_create_rotation_property(struct drm_device *dev,

• 			struct intel_plane *plane);
  

+void intel_create_rotation_property(struct intel_plane *plane,

• 		    unsigned int supported_rotations);
  

void assert_pch_transcoder_disabled(struct drm_i915_private *dev_priv,       enum pipe pipe); diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c index cc6af1410d67..bc41c1bba04c 100644 --- a/drivers/gpu/drm/i915/intel_sprite.c +++ b/drivers/gpu/drm/i915/intel_sprite.c @@ -1046,6 +1046,7 @@ intel_plane_init(struct drm_device *dev, enum pipe pipe, int plane)   struct intel_plane_state *state = NULL;   unsigned long possible_crtcs;   const uint32_t *plane_formats;

• unsigned int supported_rotations;

int num_plane_formats;   int ret;   @@ -1121,6 +1122,15 @@ intel_plane_init(struct drm_device *dev, enum pipe pipe, int plane)   goto fail;   }

• if (INTEL_INFO(dev)->gen >= 9) {

• supported_rotations =
  

• 	BIT(DRM_ROTATE_0) | BIT(DRM_ROTATE_90) |
  

• 	BIT(DRM_ROTATE_180) | BIT(DRM_ROTATE_270);
  

• } else {

• supported_rotations =
  

• 	BIT(DRM_ROTATE_0) | BIT(DRM_ROTATE_180);
  

• }

Same here, I'd put the |='s in order so that new gens add features, making easier to get a picture of what was added at which point.

Regards, Joonas

intel_plane->pipe = pipe;   intel_plane->plane = plane;   intel_plane->frontbuffer_bit = INTEL_FRONTBUFFER_SPRITE(pipe, plane); @@ -1143,7 +1153,7 @@ intel_plane_init(struct drm_device *dev, enum pipe pipe, int plane)   if (ret)   goto fail;

• intel_create_rotation_property(dev, intel_plane);

• intel_create_rotation_property(intel_plane, supported_rotations);

drm_plane_helper_add(&intel_plane->base, &intel_plane_helper_funcs);

On ke, 2016-07-20 at 16:18 +0300, ville.syrjala@linux.intel.com wrote:

From: Ville Syrjälä ville.syrjala@linux.intel.com

Using == to check for 180 degree rotation only works as long as the reflection bits aren't set. That will change soon enough for CHV, so let's stop doing things the wrong way.

Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com

Reviewed-by: Joonas Lahtinen joonas.lahtinen@linux.intel.com

Reading through the patch makes me wanna do the change;

#define DRM_ROTATE_180 BIT(x)

even more. Maybe with coccinelle.

Regards, Joonas

---

drivers/gpu/drm/i915/intel_display.c | 9 +++++----  drivers/gpu/drm/i915/intel_sprite.c  | 6 +++---  2 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 93ecb259c5ce..79c1a8b89d1d 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -2688,7 +2688,7 @@ static void i9xx_update_primary_plane(struct drm_plane *primary,   intel_crtc->dspaddr_offset = linear_offset;   }

• if (rotation == BIT(DRM_ROTATE_180)) {

• if (rotation & BIT(DRM_ROTATE_180)) {

dspcntr |= DISPPLANE_ROTATE_180;     x += (crtc_state->pipe_src_w - 1); @@ -2791,7 +2791,8 @@ static void ironlake_update_primary_plane(struct drm_plane *primary,   intel_compute_tile_offset(&x, &y, fb, 0,     fb->pitches[0], rotation);   linear_offset -= intel_crtc->dspaddr_offset;

• if (rotation == BIT(DRM_ROTATE_180)) {

• if (rotation & BIT(DRM_ROTATE_180)) {

dspcntr |= DISPPLANE_ROTATE_180;     if (!IS_HASWELL(dev) && !IS_BROADWELL(dev)) { @@ -10263,7 +10264,7 @@ static void i9xx_update_cursor(struct drm_crtc *crtc, u32 base,   if (HAS_DDI(dev))   cntl |= CURSOR_PIPE_CSC_ENABLE;

• if (plane_state->base.rotation == BIT(DRM_ROTATE_180))
  

• if (plane_state->base.rotation & BIT(DRM_ROTATE_180))
  

cntl |= CURSOR_ROTATE_180;   }   @@ -10309,7 +10310,7 @@ static void intel_crtc_update_cursor(struct drm_crtc *crtc,     /* ILK+ do this automagically */   if (HAS_GMCH_DISPLAY(dev) &&

•     plane_state->base.rotation == BIT(DRM_ROTATE_180)) {
  

•     plane_state->base.rotation & BIT(DRM_ROTATE_180)) {
  

base += (plane_state->base.crtc_h *    plane_state->base.crtc_w - 1) * 4;   } diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c index bc41c1bba04c..6b815d57d75a 100644 --- a/drivers/gpu/drm/i915/intel_sprite.c +++ b/drivers/gpu/drm/i915/intel_sprite.c @@ -444,7 +444,7 @@ vlv_update_plane(struct drm_plane *dplane,      fb->pitches[0], rotation);   linear_offset -= sprsurf_offset;

• if (rotation == BIT(DRM_ROTATE_180)) {

• if (rotation & BIT(DRM_ROTATE_180)) {

sprctl |= SP_ROTATE_180;     x += src_w; @@ -577,7 +577,7 @@ ivb_update_plane(struct drm_plane *plane,      fb->pitches[0], rotation);   linear_offset -= sprsurf_offset;

• if (rotation == BIT(DRM_ROTATE_180)) {

• if (rotation & BIT(DRM_ROTATE_180)) {

sprctl |= SPRITE_ROTATE_180;     /* HSW and BDW does this automagically in hardware */ @@ -714,7 +714,7 @@ ilk_update_plane(struct drm_plane *plane,      fb->pitches[0], rotation);   linear_offset -= dvssurf_offset;

• if (rotation == BIT(DRM_ROTATE_180)) {

• if (rotation & BIT(DRM_ROTATE_180)) {

dvscntr |= DVS_ROTATE_180;     x += src_w;

On ke, 2016-07-20 at 16:18 +0300, ville.syrjala@linux.intel.com wrote:

From: Ville Syrjälä ville.syrjala@linux.intel.com

Move the plane control register rotation setup away from the coordinate munging code. This will result in neater looking code once we add reflection support for CHV.

Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com

Looks cleaner,

Reviewed-by: Joonas Lahtinen joonas.lahtinen@linux.intel.com

---

drivers/gpu/drm/i915/intel_display.c | 28 ++++++++++++++--------------  drivers/gpu/drm/i915/intel_sprite.c  | 28 +++++++++++++++-------------  2 files changed, 29 insertions(+), 27 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 79c1a8b89d1d..88a7c4173715 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -2674,6 +2674,9 @@ static void i9xx_update_primary_plane(struct drm_plane *primary,       obj->tiling_mode != I915_TILING_NONE)   dspcntr |= DISPPLANE_TILED;

• if (rotation & BIT(DRM_ROTATE_180))

• dspcntr |= DISPPLANE_ROTATE_180;
  

if (IS_G4X(dev))   dspcntr |= DISPPLANE_TRICKLE_FEED_DISABLE;   @@ -2689,8 +2692,6 @@ static void i9xx_update_primary_plane(struct drm_plane *primary,   }     if (rotation & BIT(DRM_ROTATE_180)) {

• dspcntr |= DISPPLANE_ROTATE_180;
  

x += (crtc_state->pipe_src_w - 1);   y += (crtc_state->pipe_src_h - 1);   @@ -2783,6 +2784,9 @@ static void ironlake_update_primary_plane(struct drm_plane *primary,   if (obj->tiling_mode != I915_TILING_NONE)   dspcntr |= DISPPLANE_TILED;

• if (rotation & BIT(DRM_ROTATE_180))

• dspcntr |= DISPPLANE_ROTATE_180;
  

if (!IS_HASWELL(dev) && !IS_BROADWELL(dev))   dspcntr |= DISPPLANE_TRICKLE_FEED_DISABLE;   @@ -2792,19 +2796,15 @@ static void ironlake_update_primary_plane(struct drm_plane *primary,     fb->pitches[0], rotation);   linear_offset -= intel_crtc->dspaddr_offset;

• if (rotation & BIT(DRM_ROTATE_180)) {

• dspcntr |= DISPPLANE_ROTATE_180;
  

• if (!IS_HASWELL(dev) && !IS_BROADWELL(dev)) {
  

• 	x += (crtc_state->pipe_src_w - 1);
  

• 	y += (crtc_state->pipe_src_h - 1);
  

• /* HSW and BDW does this automagically in hardware */
• if (!IS_HASWELL(dev) && !IS_BROADWELL(dev) &&
• rotation & BIT(DRM_ROTATE_180)) {

• x += (crtc_state->pipe_src_w - 1);
  

• y += (crtc_state->pipe_src_h - 1);
  

• 	/* Finding the last pixel of the last line of the display
  

• 	data and adding to linear_offset*/
  

• 	linear_offset +=
  

• 		(crtc_state->pipe_src_h - 1) * fb->pitches[0] +
  

• 		(crtc_state->pipe_src_w - 1) * cpp;
  

• }
  

• linear_offset +=
  

• 	(crtc_state->pipe_src_h - 1) * fb->pitches[0] +
  

• 	(crtc_state->pipe_src_w - 1) * cpp;
  

}     intel_crtc->adjusted_x = x; diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c index 6b815d57d75a..14173f53f520 100644 --- a/drivers/gpu/drm/i915/intel_sprite.c +++ b/drivers/gpu/drm/i915/intel_sprite.c @@ -433,6 +433,9 @@ vlv_update_plane(struct drm_plane *dplane,   if (obj->tiling_mode != I915_TILING_NONE)   sprctl |= SP_TILED;

• if (rotation & BIT(DRM_ROTATE_180))

• sprctl |= SP_ROTATE_180;
  

/* Sizes are 0 based */   src_w--;   src_h--; @@ -445,8 +448,6 @@ vlv_update_plane(struct drm_plane *dplane,   linear_offset -= sprsurf_offset;     if (rotation & BIT(DRM_ROTATE_180)) {

• sprctl |= SP_ROTATE_180;
  

x += src_w;   y += src_h;   linear_offset += src_h * fb->pitches[0] + src_w * cpp; @@ -555,6 +556,9 @@ ivb_update_plane(struct drm_plane *plane,   if (obj->tiling_mode != I915_TILING_NONE)   sprctl |= SPRITE_TILED;

• if (rotation & BIT(DRM_ROTATE_180))

• sprctl |= SPRITE_ROTATE_180;
  

if (IS_HASWELL(dev) || IS_BROADWELL(dev))   sprctl &= ~SPRITE_TRICKLE_FEED_DISABLE;   else @@ -577,15 +581,12 @@ ivb_update_plane(struct drm_plane *plane,      fb->pitches[0], rotation);   linear_offset -= sprsurf_offset;

• if (rotation & BIT(DRM_ROTATE_180)) {

• sprctl |= SPRITE_ROTATE_180;
  

• /* HSW and BDW does this automagically in hardware */
  

• if (!IS_HASWELL(dev) && !IS_BROADWELL(dev)) {
  

• 	x += src_w;
  

• 	y += src_h;
  

• 	linear_offset += src_h * fb->pitches[0] + src_w * cpp;
  

• }
  

• /* HSW and BDW does this automagically in hardware */
• if (!IS_HASWELL(dev) && !IS_BROADWELL(dev) &&
• rotation & BIT(DRM_ROTATE_180)) {

• x += src_w;
  

• y += src_h;
  

• linear_offset += src_h * fb->pitches[0] + src_w * cpp;
  

}     if (key->flags) { @@ -696,6 +697,9 @@ ilk_update_plane(struct drm_plane *plane,   if (obj->tiling_mode != I915_TILING_NONE)   dvscntr |= DVS_TILED;

• if (rotation & BIT(DRM_ROTATE_180))

• dvscntr |= DVS_ROTATE_180;
  

if (IS_GEN6(dev))   dvscntr |= DVS_TRICKLE_FEED_DISABLE; /* must disable */   @@ -715,8 +719,6 @@ ilk_update_plane(struct drm_plane *plane,   linear_offset -= dvssurf_offset;     if (rotation & BIT(DRM_ROTATE_180)) {

• dvscntr |= DVS_ROTATE_180;
  

x += src_w;   y += src_h;   linear_offset += src_h * fb->pitches[0] + src_w * cpp;

On ke, 2016-07-20 at 16:18 +0300, ville.syrjala@linux.intel.com wrote:

From: Ville Syrjälä ville.syrjala@linux.intel.com

The primary and sprite planes on CHV pipe B support horizontal mirroring. Expose it to the world.

Sadly the hardware ignores the mirror bit when the rotate bit is set, so we'll have to reject the 180+X case.

Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com

Seems to match spec. Maybe add your own T-b, as you have obviously tested on hardware.

Reviewed-by: Joonas Lahtinen joonas.lahtinen@linux.intel.com

Regards, Joonas

---

drivers/gpu/drm/i915/intel_atomic_plane.c |  8 ++++++++  drivers/gpu/drm/i915/intel_display.c      | 10 ++++++++++  drivers/gpu/drm/i915/intel_sprite.c       | 10 ++++++++++  3 files changed, 28 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c b/drivers/gpu/drm/i915/intel_atomic_plane.c index c19eb9a0cd4a..0a019eacfede 100644 --- a/drivers/gpu/drm/i915/intel_atomic_plane.c +++ b/drivers/gpu/drm/i915/intel_atomic_plane.c @@ -180,6 +180,14 @@ static int intel_plane_atomic_check(struct drm_plane *plane,   }   }

• /* CHV ignores the mirror bit when the rotate bit is set :( */
• if (IS_CHERRYVIEW(plane->dev) &&
• state->rotation & BIT(DRM_ROTATE_180) &&
• state->rotation & BIT(DRM_REFLECT_X)) {

• DRM_DEBUG_KMS("Cannot rotate and reflect at the same time\n");
  

• return -EINVAL;
  

• }

intel_state->visible = false;   ret = intel_plane->check_plane(plane, crtc_state, intel_state);   if (ret) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 88a7c4173715..edb1809a642d 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -2677,6 +2677,9 @@ static void i9xx_update_primary_plane(struct drm_plane *primary,   if (rotation & BIT(DRM_ROTATE_180))   dspcntr |= DISPPLANE_ROTATE_180;

• if (rotation & BIT(DRM_REFLECT_X))

• dspcntr |= DISPPLANE_MIRROR;
  

if (IS_G4X(dev))   dspcntr |= DISPPLANE_TRICKLE_FEED_DISABLE;   @@ -2700,6 +2703,9 @@ static void i9xx_update_primary_plane(struct drm_plane *primary,   linear_offset +=   (crtc_state->pipe_src_h - 1) * fb->pitches[0] +   (crtc_state->pipe_src_w - 1) * cpp;

• } else if (rotation & BIT(DRM_REFLECT_X)) {

• x += (crtc_state->pipe_src_w - 1);
  

• linear_offset += (crtc_state->pipe_src_w - 1) * cpp;
  

}     intel_crtc->adjusted_x = x; @@ -14295,6 +14301,10 @@ static struct drm_plane *intel_primary_plane_create(struct drm_device *dev,   supported_rotations =   BIT(DRM_ROTATE_0) | BIT(DRM_ROTATE_90) |   BIT(DRM_ROTATE_180) | BIT(DRM_ROTATE_270);

• } else if (IS_CHERRYVIEW(dev) && pipe == PIPE_B) {

• supported_rotations =
  

• 	BIT(DRM_ROTATE_0) | BIT(DRM_ROTATE_180) |
  

• 	BIT(DRM_REFLECT_X);
  

} else if (INTEL_INFO(dev)->gen >= 4) {   supported_rotations =   BIT(DRM_ROTATE_0) | BIT(DRM_ROTATE_180); diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c index 14173f53f520..4d6cd1a02e34 100644 --- a/drivers/gpu/drm/i915/intel_sprite.c +++ b/drivers/gpu/drm/i915/intel_sprite.c @@ -436,6 +436,9 @@ vlv_update_plane(struct drm_plane *dplane,   if (rotation & BIT(DRM_ROTATE_180))   sprctl |= SP_ROTATE_180;

• if (rotation & BIT(DRM_REFLECT_X))

• sprctl |= SP_MIRROR;
  

/* Sizes are 0 based */   src_w--;   src_h--; @@ -451,6 +454,9 @@ vlv_update_plane(struct drm_plane *dplane,   x += src_w;   y += src_h;   linear_offset += src_h * fb->pitches[0] + src_w * cpp;

• } else if (rotation & BIT(DRM_REFLECT_X)) {

• x += src_w;
  

• linear_offset += src_w * cpp;
  

}     if (key->flags) { @@ -1128,6 +1134,10 @@ intel_plane_init(struct drm_device *dev, enum pipe pipe, int plane)   supported_rotations =   BIT(DRM_ROTATE_0) | BIT(DRM_ROTATE_90) |   BIT(DRM_ROTATE_180) | BIT(DRM_ROTATE_270);

• } else if (IS_CHERRYVIEW(dev) && pipe == PIPE_B) {

• supported_rotations =
  

• 	BIT(DRM_ROTATE_0) | BIT(DRM_ROTATE_180) |
  

• 	BIT(DRM_REFLECT_X);
  

} else {   supported_rotations =   BIT(DRM_ROTATE_0) | BIT(DRM_ROTATE_180);

Was not this implemented once and then dropped for some reason?

On ke, 2016-07-20 at 16:18 +0300, ville.syrjala@linux.intel.com wrote:

From: Ville Syrjälä ville.syrjala@linux.intel.com

Add "bad-rotation" subtest to make sure the kernel rejects some invalid rotation values (0 and specifying multiple angles at one).

Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com

tests/kms_rotation_crc.c | 33 +++++++++++++++++++++++++++++++++  1 file changed, 33 insertions(+)

diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c index 6cc15337fff9..e10a0a770437 100644 --- a/tests/kms_rotation_crc.c +++ b/tests/kms_rotation_crc.c @@ -237,6 +237,31 @@ static void wait_for_pageflip(int fd)   igt_assert(drmHandleEvent(fd, &evctx) == 0);  }   +static void test_bad_prop_value(data_t *data) +{

• igt_display_t *display = &data->display;
• int valid_tests = 0;
• enum pipe pipe;
• igt_plane_t *plane;
• int ret;
• for_each_pipe(display, pipe)  {

• for_each_plane_on_pipe(display, pipe, plane) {
  

• 	igt_require(igt_plane_supports_rotation(plane));
  

• 	ret = drmModeObjectSetProperty(display->drm_fd,
  

• 				       plane->drm_plane->plane_id,
  

• 				       DRM_MODE_OBJECT_PLANE,
  

• 				       plane->rotation_property,
  

• 				       data->rotation);
  

• 	igt_assert_eq(ret, -EINVAL);
  

• 	valid_tests++;
  

• }
  

• }
• igt_require_f(valid_tests, "no valid crtc/connector combinations found\n");

+}

static void test_plane_rotation(data_t *data, enum igt_plane plane_type)  {   igt_display_t *display = &data->display; @@ -508,6 +533,14 @@ igt_main     igt_display_init(&data.display, data.gfx_fd);   }

• igt_subtest_f("bad-rotation") {

• data.rotation = 0;
  

• test_bad_prop_value(&data);
  

• data.rotation = IGT_ROTATION_0 | IGT_ROTATION_180;
  

• test_bad_prop_value(&data);
  

• }

igt_subtest_f("primary-rotation-180") {   data.rotation = IGT_ROTATION_180;   test_plane_rotation(&data, IGT_PLANE_PRIMARY);