On ke, 2016-07-20 at 16:18 +0300, ville.syrjala at linux.intel.com wrote: > From: Ville Syrjälä <ville.syrjala at 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 at 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); > Â -- Joonas Lahtinen Open Source Technology Center Intel Corporation