| Message ID | 1392239704-21776-3-git-send-email-ville.syrjala@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, 2014-02-12 at 23:15 +0200, ville.syrjala@linux.intel.com wrote: > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > I'd prefer have the crtc "rotation" property rotate the entire crtc > (planes and all). So for that reason we'd need to come up with some > other name for the "rotate the primary plane only" property. > > Originally I had though that omapdrm had already made the decision for > us, but after another look, it looks like it never attaches the > "rotation" property to the crtc. So we can still change the name > without any ABI breakage. > > Suggestions for better naming scheme are also welcome.... I would suggest name to be "primary-rotation" or "primary_rotation". It seems more aligned to member variable primary_rotation as well. > > TODO: squash into "drm/i915: Add 180 degree primary plane rotation support"? > > Cc: Sagar Kamble <sagar.a.kamble@intel.com> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > --- > drivers/gpu/drm/i915/i915_dma.c | 11 +++++++---- > drivers/gpu/drm/i915/i915_drv.h | 3 ++- > drivers/gpu/drm/i915/intel_display.c | 28 ++++++++++++++-------------- > drivers/gpu/drm/i915/intel_drv.h | 2 +- > drivers/gpu/drm/i915/intel_pm.c | 2 +- > 5 files changed, 25 insertions(+), 21 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c > index 9232fdf..4a3ef34 100644 > --- a/drivers/gpu/drm/i915/i915_dma.c > +++ b/drivers/gpu/drm/i915/i915_dma.c > @@ -1898,13 +1898,16 @@ void i915_driver_lastclose(struct drm_device * dev) > if (!dev_priv) > return; > > - if (dev_priv->rotation_property) { > + if (dev_priv->plane_rotation_property) { > list_for_each_entry(crtc, &dev->mode_config.crtc_list, base.head) { > - crtc->rotation = BIT(DRM_ROTATE_0); > + crtc->primary_rotation = BIT(DRM_ROTATE_0); > drm_object_property_set_value(&crtc->base.base, > - dev_priv->rotation_property, > - crtc->rotation); > + dev_priv->plane_rotation_property, > + crtc->primary_rotation); > } > + } > + > + if (dev_priv->rotation_property) { > list_for_each_entry(plane, &dev->mode_config.plane_list, base.head) { > plane->rotation = BIT(DRM_ROTATE_0); > drm_object_property_set_value(&plane->base.base, > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 5a46788..6af78ee 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -1560,7 +1560,8 @@ typedef struct drm_i915_private { > > struct drm_property *broadcast_rgb_property; > struct drm_property *force_audio_property; > - struct drm_property *rotation_property; > + struct drm_property *rotation_property; /* "rotation" */ > + struct drm_property *plane_rotation_property; /* "plane-rotation" */ > > uint32_t hw_context_size; > struct list_head context_list; > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index bab17fd..37b23d1 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -2133,7 +2133,7 @@ static int i9xx_update_plane(struct drm_crtc *crtc, struct drm_framebuffer *fb, > intel_crtc->dspaddr_offset = linear_offset; > } > > - if (intel_crtc->rotation == BIT(DRM_ROTATE_180)) { > + if (intel_crtc->primary_rotation == BIT(DRM_ROTATE_180)) { > dspcntr |= DISPPLANE_ROTATE_180; > > x += (intel_crtc->config.pipe_src_w - 1); > @@ -2238,7 +2238,7 @@ static int ironlake_update_plane(struct drm_crtc *crtc, > fb->pitches[0]); > linear_offset -= intel_crtc->dspaddr_offset; > > - if (intel_crtc->rotation == BIT(DRM_ROTATE_180)) { > + if (intel_crtc->primary_rotation == BIT(DRM_ROTATE_180)) { > dspcntr |= DISPPLANE_ROTATE_180; > > if (!IS_HASWELL(dev) && !IS_BROADWELL(dev)) { > @@ -8820,13 +8820,13 @@ static int intel_crtc_set_property(struct drm_crtc *crtc, > uint64_t old_val; > int ret = -ENOENT; > > - if (prop == dev_priv->rotation_property) { > + if (prop == dev_priv->plane_rotation_property) { > /* exactly one rotation angle please */ > if (hweight32(val & 0xf) != 1) > return -EINVAL; > > - old_val = intel_crtc->rotation; > - intel_crtc->rotation = val; > + old_val = intel_crtc->primary_rotation; > + intel_crtc->primary_rotation = val; > > if (intel_crtc->active) { > intel_crtc_wait_for_pending_flips(crtc); > @@ -8834,12 +8834,12 @@ static int intel_crtc_set_property(struct drm_crtc *crtc, > /* FBC does not work on some platforms for rotated planes */ > if (dev_priv->fbc.plane == intel_crtc->plane && > INTEL_INFO(dev)->gen <= 4 && !IS_G4X(dev) && > - intel_crtc->rotation != BIT(DRM_ROTATE_0)) > + intel_crtc->primary_rotation != BIT(DRM_ROTATE_0)) > intel_disable_fbc(dev); > > ret = dev_priv->display.update_plane(crtc, crtc->fb, 0, 0); > if (ret) > - intel_crtc->rotation = old_val; > + intel_crtc->primary_rotation = old_val; > } > } > > @@ -10387,7 +10387,7 @@ static void intel_crtc_init(struct drm_device *dev, int pipe) > */ > intel_crtc->pipe = pipe; > intel_crtc->plane = pipe; > - intel_crtc->rotation = BIT(DRM_ROTATE_0); > + intel_crtc->primary_rotation = BIT(DRM_ROTATE_0); > if (HAS_FBC(dev) && INTEL_INFO(dev)->gen < 4) { > DRM_DEBUG_KMS("swapping pipes & planes for FBC\n"); > intel_crtc->plane = !pipe; > @@ -10399,15 +10399,15 @@ static void intel_crtc_init(struct drm_device *dev, int pipe) > dev_priv->pipe_to_crtc_mapping[intel_crtc->pipe] = &intel_crtc->base; > > if (INTEL_INFO(dev)->gen >= 4) { > - if (!dev_priv->rotation_property) > - dev_priv->rotation_property = > - drm_mode_create_rotation_property(dev, "rotation", > + if (!dev_priv->plane_rotation_property) > + dev_priv->plane_rotation_property = > + drm_mode_create_rotation_property(dev, "plane-rotation", > BIT(DRM_ROTATE_0) | > BIT(DRM_ROTATE_180)); > - if (dev_priv->rotation_property) > + if (dev_priv->plane_rotation_property) > drm_object_attach_property(&intel_crtc->base.base, > - dev_priv->rotation_property, > - intel_crtc->rotation); > + dev_priv->plane_rotation_property, > + intel_crtc->primary_rotation); > } > > drm_crtc_helper_add(&intel_crtc->base, &intel_helper_funcs); > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > index 82f0f9a..8c17a82 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -331,7 +331,7 @@ struct intel_crtc { > struct drm_crtc base; > enum pipe pipe; > enum plane plane; > - unsigned int rotation; > + unsigned int primary_rotation; /* primary plane in relation to the pipe */ > > u8 lut_r[256], lut_g[256], lut_b[256]; > /* > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > index 9c9ddfe..5ebeb78 100644 > --- a/drivers/gpu/drm/i915/intel_pm.c > +++ b/drivers/gpu/drm/i915/intel_pm.c > @@ -558,7 +558,7 @@ void intel_update_fbc(struct drm_device *dev) > } > > if (INTEL_INFO(dev)->gen <= 4 && !IS_G4X(dev) && > - intel_crtc->rotation != BIT(DRM_ROTATE_0)) { > + intel_crtc->primary_rotation != BIT(DRM_ROTATE_0)) { > if (set_no_fbc_reason(dev_priv, FBC_UNSUPPORTED_MODE)) > DRM_DEBUG_KMS("mode incompatible with compression, " > "disabling\n");
On Thu, Feb 13, 2014 at 06:07:27PM +0530, Sagar Arun Kamble wrote: > On Wed, 2014-02-12 at 23:15 +0200, ville.syrjala@linux.intel.com wrote: > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > I'd prefer have the crtc "rotation" property rotate the entire crtc > > (planes and all). So for that reason we'd need to come up with some > > other name for the "rotate the primary plane only" property. > > > > Originally I had though that omapdrm had already made the decision for > > us, but after another look, it looks like it never attaches the > > "rotation" property to the crtc. So we can still change the name > > without any ABI breakage. > > > > Suggestions for better naming scheme are also welcome.... > I would suggest name to be "primary-rotation" or "primary_rotation". It > seems more aligned to member variable primary_rotation as well. Well, "primary plane" is an Intel term, so I don't know if other people would find it sensible. But I guess you can consider any plane "primary" if it's assigned to act as the crtc scanout engine... > > > > TODO: squash into "drm/i915: Add 180 degree primary plane rotation support"? > > > > Cc: Sagar Kamble <sagar.a.kamble@intel.com> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > --- > > drivers/gpu/drm/i915/i915_dma.c | 11 +++++++---- > > drivers/gpu/drm/i915/i915_drv.h | 3 ++- > > drivers/gpu/drm/i915/intel_display.c | 28 ++++++++++++++-------------- > > drivers/gpu/drm/i915/intel_drv.h | 2 +- > > drivers/gpu/drm/i915/intel_pm.c | 2 +- > > 5 files changed, 25 insertions(+), 21 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c > > index 9232fdf..4a3ef34 100644 > > --- a/drivers/gpu/drm/i915/i915_dma.c > > +++ b/drivers/gpu/drm/i915/i915_dma.c > > @@ -1898,13 +1898,16 @@ void i915_driver_lastclose(struct drm_device * dev) > > if (!dev_priv) > > return; > > > > - if (dev_priv->rotation_property) { > > + if (dev_priv->plane_rotation_property) { > > list_for_each_entry(crtc, &dev->mode_config.crtc_list, base.head) { > > - crtc->rotation = BIT(DRM_ROTATE_0); > > + crtc->primary_rotation = BIT(DRM_ROTATE_0); > > drm_object_property_set_value(&crtc->base.base, > > - dev_priv->rotation_property, > > - crtc->rotation); > > + dev_priv->plane_rotation_property, > > + crtc->primary_rotation); > > } > > + } > > + > > + if (dev_priv->rotation_property) { > > list_for_each_entry(plane, &dev->mode_config.plane_list, base.head) { > > plane->rotation = BIT(DRM_ROTATE_0); > > drm_object_property_set_value(&plane->base.base, > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > > index 5a46788..6af78ee 100644 > > --- a/drivers/gpu/drm/i915/i915_drv.h > > +++ b/drivers/gpu/drm/i915/i915_drv.h > > @@ -1560,7 +1560,8 @@ typedef struct drm_i915_private { > > > > struct drm_property *broadcast_rgb_property; > > struct drm_property *force_audio_property; > > - struct drm_property *rotation_property; > > + struct drm_property *rotation_property; /* "rotation" */ > > + struct drm_property *plane_rotation_property; /* "plane-rotation" */ > > > > uint32_t hw_context_size; > > struct list_head context_list; > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > > index bab17fd..37b23d1 100644 > > --- a/drivers/gpu/drm/i915/intel_display.c > > +++ b/drivers/gpu/drm/i915/intel_display.c > > @@ -2133,7 +2133,7 @@ static int i9xx_update_plane(struct drm_crtc *crtc, struct drm_framebuffer *fb, > > intel_crtc->dspaddr_offset = linear_offset; > > } > > > > - if (intel_crtc->rotation == BIT(DRM_ROTATE_180)) { > > + if (intel_crtc->primary_rotation == BIT(DRM_ROTATE_180)) { > > dspcntr |= DISPPLANE_ROTATE_180; > > > > x += (intel_crtc->config.pipe_src_w - 1); > > @@ -2238,7 +2238,7 @@ static int ironlake_update_plane(struct drm_crtc *crtc, > > fb->pitches[0]); > > linear_offset -= intel_crtc->dspaddr_offset; > > > > - if (intel_crtc->rotation == BIT(DRM_ROTATE_180)) { > > + if (intel_crtc->primary_rotation == BIT(DRM_ROTATE_180)) { > > dspcntr |= DISPPLANE_ROTATE_180; > > > > if (!IS_HASWELL(dev) && !IS_BROADWELL(dev)) { > > @@ -8820,13 +8820,13 @@ static int intel_crtc_set_property(struct drm_crtc *crtc, > > uint64_t old_val; > > int ret = -ENOENT; > > > > - if (prop == dev_priv->rotation_property) { > > + if (prop == dev_priv->plane_rotation_property) { > > /* exactly one rotation angle please */ > > if (hweight32(val & 0xf) != 1) > > return -EINVAL; > > > > - old_val = intel_crtc->rotation; > > - intel_crtc->rotation = val; > > + old_val = intel_crtc->primary_rotation; > > + intel_crtc->primary_rotation = val; > > > > if (intel_crtc->active) { > > intel_crtc_wait_for_pending_flips(crtc); > > @@ -8834,12 +8834,12 @@ static int intel_crtc_set_property(struct drm_crtc *crtc, > > /* FBC does not work on some platforms for rotated planes */ > > if (dev_priv->fbc.plane == intel_crtc->plane && > > INTEL_INFO(dev)->gen <= 4 && !IS_G4X(dev) && > > - intel_crtc->rotation != BIT(DRM_ROTATE_0)) > > + intel_crtc->primary_rotation != BIT(DRM_ROTATE_0)) > > intel_disable_fbc(dev); > > > > ret = dev_priv->display.update_plane(crtc, crtc->fb, 0, 0); > > if (ret) > > - intel_crtc->rotation = old_val; > > + intel_crtc->primary_rotation = old_val; > > } > > } > > > > @@ -10387,7 +10387,7 @@ static void intel_crtc_init(struct drm_device *dev, int pipe) > > */ > > intel_crtc->pipe = pipe; > > intel_crtc->plane = pipe; > > - intel_crtc->rotation = BIT(DRM_ROTATE_0); > > + intel_crtc->primary_rotation = BIT(DRM_ROTATE_0); > > if (HAS_FBC(dev) && INTEL_INFO(dev)->gen < 4) { > > DRM_DEBUG_KMS("swapping pipes & planes for FBC\n"); > > intel_crtc->plane = !pipe; > > @@ -10399,15 +10399,15 @@ static void intel_crtc_init(struct drm_device *dev, int pipe) > > dev_priv->pipe_to_crtc_mapping[intel_crtc->pipe] = &intel_crtc->base; > > > > if (INTEL_INFO(dev)->gen >= 4) { > > - if (!dev_priv->rotation_property) > > - dev_priv->rotation_property = > > - drm_mode_create_rotation_property(dev, "rotation", > > + if (!dev_priv->plane_rotation_property) > > + dev_priv->plane_rotation_property = > > + drm_mode_create_rotation_property(dev, "plane-rotation", > > BIT(DRM_ROTATE_0) | > > BIT(DRM_ROTATE_180)); > > - if (dev_priv->rotation_property) > > + if (dev_priv->plane_rotation_property) > > drm_object_attach_property(&intel_crtc->base.base, > > - dev_priv->rotation_property, > > - intel_crtc->rotation); > > + dev_priv->plane_rotation_property, > > + intel_crtc->primary_rotation); > > } > > > > drm_crtc_helper_add(&intel_crtc->base, &intel_helper_funcs); > > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > > index 82f0f9a..8c17a82 100644 > > --- a/drivers/gpu/drm/i915/intel_drv.h > > +++ b/drivers/gpu/drm/i915/intel_drv.h > > @@ -331,7 +331,7 @@ struct intel_crtc { > > struct drm_crtc base; > > enum pipe pipe; > > enum plane plane; > > - unsigned int rotation; > > + unsigned int primary_rotation; /* primary plane in relation to the pipe */ > > > > u8 lut_r[256], lut_g[256], lut_b[256]; > > /* > > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > > index 9c9ddfe..5ebeb78 100644 > > --- a/drivers/gpu/drm/i915/intel_pm.c > > +++ b/drivers/gpu/drm/i915/intel_pm.c > > @@ -558,7 +558,7 @@ void intel_update_fbc(struct drm_device *dev) > > } > > > > if (INTEL_INFO(dev)->gen <= 4 && !IS_G4X(dev) && > > - intel_crtc->rotation != BIT(DRM_ROTATE_0)) { > > + intel_crtc->primary_rotation != BIT(DRM_ROTATE_0)) { > > if (set_no_fbc_reason(dev_priv, FBC_UNSUPPORTED_MODE)) > > DRM_DEBUG_KMS("mode incompatible with compression, " > > "disabling\n"); >
On Thu, Feb 13, 2014 at 03:46:39PM +0200, Ville Syrjälä wrote: > On Thu, Feb 13, 2014 at 06:07:27PM +0530, Sagar Arun Kamble wrote: > > On Wed, 2014-02-12 at 23:15 +0200, ville.syrjala@linux.intel.com wrote: > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > > > I'd prefer have the crtc "rotation" property rotate the entire crtc > > > (planes and all). So for that reason we'd need to come up with some > > > other name for the "rotate the primary plane only" property. > > > > > > Originally I had though that omapdrm had already made the decision for > > > us, but after another look, it looks like it never attaches the > > > "rotation" property to the crtc. So we can still change the name > > > without any ABI breakage. > > > > > > Suggestions for better naming scheme are also welcome.... > > I would suggest name to be "primary-rotation" or "primary_rotation". It > > seems more aligned to member variable primary_rotation as well. > > Well, "primary plane" is an Intel term, so I don't know if other people > would find it sensible. But I guess you can consider any plane "primary" > if it's assigned to act as the crtc scanout engine... It seems I need to scrap this plan actually. Rob hit me with the clue bat on irc, and omapdrm does in fact install the "rotation" prop on the crtc. So I guess I need to rename the "rotation" prop to something else "crtc-rotation" maybe? Anyone have a good name up their sleeve? > > > > > > > TODO: squash into "drm/i915: Add 180 degree primary plane rotation support"? > > > > > > Cc: Sagar Kamble <sagar.a.kamble@intel.com> > > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > --- > > > drivers/gpu/drm/i915/i915_dma.c | 11 +++++++---- > > > drivers/gpu/drm/i915/i915_drv.h | 3 ++- > > > drivers/gpu/drm/i915/intel_display.c | 28 ++++++++++++++-------------- > > > drivers/gpu/drm/i915/intel_drv.h | 2 +- > > > drivers/gpu/drm/i915/intel_pm.c | 2 +- > > > 5 files changed, 25 insertions(+), 21 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c > > > index 9232fdf..4a3ef34 100644 > > > --- a/drivers/gpu/drm/i915/i915_dma.c > > > +++ b/drivers/gpu/drm/i915/i915_dma.c > > > @@ -1898,13 +1898,16 @@ void i915_driver_lastclose(struct drm_device * dev) > > > if (!dev_priv) > > > return; > > > > > > - if (dev_priv->rotation_property) { > > > + if (dev_priv->plane_rotation_property) { > > > list_for_each_entry(crtc, &dev->mode_config.crtc_list, base.head) { > > > - crtc->rotation = BIT(DRM_ROTATE_0); > > > + crtc->primary_rotation = BIT(DRM_ROTATE_0); > > > drm_object_property_set_value(&crtc->base.base, > > > - dev_priv->rotation_property, > > > - crtc->rotation); > > > + dev_priv->plane_rotation_property, > > > + crtc->primary_rotation); > > > } > > > + } > > > + > > > + if (dev_priv->rotation_property) { > > > list_for_each_entry(plane, &dev->mode_config.plane_list, base.head) { > > > plane->rotation = BIT(DRM_ROTATE_0); > > > drm_object_property_set_value(&plane->base.base, > > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > > > index 5a46788..6af78ee 100644 > > > --- a/drivers/gpu/drm/i915/i915_drv.h > > > +++ b/drivers/gpu/drm/i915/i915_drv.h > > > @@ -1560,7 +1560,8 @@ typedef struct drm_i915_private { > > > > > > struct drm_property *broadcast_rgb_property; > > > struct drm_property *force_audio_property; > > > - struct drm_property *rotation_property; > > > + struct drm_property *rotation_property; /* "rotation" */ > > > + struct drm_property *plane_rotation_property; /* "plane-rotation" */ > > > > > > uint32_t hw_context_size; > > > struct list_head context_list; > > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > > > index bab17fd..37b23d1 100644 > > > --- a/drivers/gpu/drm/i915/intel_display.c > > > +++ b/drivers/gpu/drm/i915/intel_display.c > > > @@ -2133,7 +2133,7 @@ static int i9xx_update_plane(struct drm_crtc *crtc, struct drm_framebuffer *fb, > > > intel_crtc->dspaddr_offset = linear_offset; > > > } > > > > > > - if (intel_crtc->rotation == BIT(DRM_ROTATE_180)) { > > > + if (intel_crtc->primary_rotation == BIT(DRM_ROTATE_180)) { > > > dspcntr |= DISPPLANE_ROTATE_180; > > > > > > x += (intel_crtc->config.pipe_src_w - 1); > > > @@ -2238,7 +2238,7 @@ static int ironlake_update_plane(struct drm_crtc *crtc, > > > fb->pitches[0]); > > > linear_offset -= intel_crtc->dspaddr_offset; > > > > > > - if (intel_crtc->rotation == BIT(DRM_ROTATE_180)) { > > > + if (intel_crtc->primary_rotation == BIT(DRM_ROTATE_180)) { > > > dspcntr |= DISPPLANE_ROTATE_180; > > > > > > if (!IS_HASWELL(dev) && !IS_BROADWELL(dev)) { > > > @@ -8820,13 +8820,13 @@ static int intel_crtc_set_property(struct drm_crtc *crtc, > > > uint64_t old_val; > > > int ret = -ENOENT; > > > > > > - if (prop == dev_priv->rotation_property) { > > > + if (prop == dev_priv->plane_rotation_property) { > > > /* exactly one rotation angle please */ > > > if (hweight32(val & 0xf) != 1) > > > return -EINVAL; > > > > > > - old_val = intel_crtc->rotation; > > > - intel_crtc->rotation = val; > > > + old_val = intel_crtc->primary_rotation; > > > + intel_crtc->primary_rotation = val; > > > > > > if (intel_crtc->active) { > > > intel_crtc_wait_for_pending_flips(crtc); > > > @@ -8834,12 +8834,12 @@ static int intel_crtc_set_property(struct drm_crtc *crtc, > > > /* FBC does not work on some platforms for rotated planes */ > > > if (dev_priv->fbc.plane == intel_crtc->plane && > > > INTEL_INFO(dev)->gen <= 4 && !IS_G4X(dev) && > > > - intel_crtc->rotation != BIT(DRM_ROTATE_0)) > > > + intel_crtc->primary_rotation != BIT(DRM_ROTATE_0)) > > > intel_disable_fbc(dev); > > > > > > ret = dev_priv->display.update_plane(crtc, crtc->fb, 0, 0); > > > if (ret) > > > - intel_crtc->rotation = old_val; > > > + intel_crtc->primary_rotation = old_val; > > > } > > > } > > > > > > @@ -10387,7 +10387,7 @@ static void intel_crtc_init(struct drm_device *dev, int pipe) > > > */ > > > intel_crtc->pipe = pipe; > > > intel_crtc->plane = pipe; > > > - intel_crtc->rotation = BIT(DRM_ROTATE_0); > > > + intel_crtc->primary_rotation = BIT(DRM_ROTATE_0); > > > if (HAS_FBC(dev) && INTEL_INFO(dev)->gen < 4) { > > > DRM_DEBUG_KMS("swapping pipes & planes for FBC\n"); > > > intel_crtc->plane = !pipe; > > > @@ -10399,15 +10399,15 @@ static void intel_crtc_init(struct drm_device *dev, int pipe) > > > dev_priv->pipe_to_crtc_mapping[intel_crtc->pipe] = &intel_crtc->base; > > > > > > if (INTEL_INFO(dev)->gen >= 4) { > > > - if (!dev_priv->rotation_property) > > > - dev_priv->rotation_property = > > > - drm_mode_create_rotation_property(dev, "rotation", > > > + if (!dev_priv->plane_rotation_property) > > > + dev_priv->plane_rotation_property = > > > + drm_mode_create_rotation_property(dev, "plane-rotation", > > > BIT(DRM_ROTATE_0) | > > > BIT(DRM_ROTATE_180)); > > > - if (dev_priv->rotation_property) > > > + if (dev_priv->plane_rotation_property) > > > drm_object_attach_property(&intel_crtc->base.base, > > > - dev_priv->rotation_property, > > > - intel_crtc->rotation); > > > + dev_priv->plane_rotation_property, > > > + intel_crtc->primary_rotation); > > > } > > > > > > drm_crtc_helper_add(&intel_crtc->base, &intel_helper_funcs); > > > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > > > index 82f0f9a..8c17a82 100644 > > > --- a/drivers/gpu/drm/i915/intel_drv.h > > > +++ b/drivers/gpu/drm/i915/intel_drv.h > > > @@ -331,7 +331,7 @@ struct intel_crtc { > > > struct drm_crtc base; > > > enum pipe pipe; > > > enum plane plane; > > > - unsigned int rotation; > > > + unsigned int primary_rotation; /* primary plane in relation to the pipe */ > > > > > > u8 lut_r[256], lut_g[256], lut_b[256]; > > > /* > > > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > > > index 9c9ddfe..5ebeb78 100644 > > > --- a/drivers/gpu/drm/i915/intel_pm.c > > > +++ b/drivers/gpu/drm/i915/intel_pm.c > > > @@ -558,7 +558,7 @@ void intel_update_fbc(struct drm_device *dev) > > > } > > > > > > if (INTEL_INFO(dev)->gen <= 4 && !IS_G4X(dev) && > > > - intel_crtc->rotation != BIT(DRM_ROTATE_0)) { > > > + intel_crtc->primary_rotation != BIT(DRM_ROTATE_0)) { > > > if (set_no_fbc_reason(dev_priv, FBC_UNSUPPORTED_MODE)) > > > DRM_DEBUG_KMS("mode incompatible with compression, " > > > "disabling\n"); > > > > -- > Ville Syrjälä > Intel OTC > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Thu, Feb 13, 2014 at 04:06:31PM +0200, Ville Syrjälä wrote: > On Thu, Feb 13, 2014 at 03:46:39PM +0200, Ville Syrjälä wrote: > > On Thu, Feb 13, 2014 at 06:07:27PM +0530, Sagar Arun Kamble wrote: > > > On Wed, 2014-02-12 at 23:15 +0200, ville.syrjala@linux.intel.com wrote: > > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > > > > > I'd prefer have the crtc "rotation" property rotate the entire crtc > > > > (planes and all). So for that reason we'd need to come up with some > > > > other name for the "rotate the primary plane only" property. > > > > > > > > Originally I had though that omapdrm had already made the decision for > > > > us, but after another look, it looks like it never attaches the > > > > "rotation" property to the crtc. So we can still change the name > > > > without any ABI breakage. > > > > > > > > Suggestions for better naming scheme are also welcome.... > > > I would suggest name to be "primary-rotation" or "primary_rotation". It > > > seems more aligned to member variable primary_rotation as well. > > > > Well, "primary plane" is an Intel term, so I don't know if other people > > would find it sensible. But I guess you can consider any plane "primary" > > if it's assigned to act as the crtc scanout engine... > > It seems I need to scrap this plan actually. Rob hit me with the clue > bat on irc, and omapdrm does in fact install the "rotation" prop on > the crtc. So I guess I need to rename the "rotation" prop to something > else "crtc-rotation" maybe? Anyone have a good name up their sleeve? To recap, you mean that the CRTC rotation property is to be the control over the rotation of the primary plane, and that we need a new property name for "rotate the world"? In which case, I'd suggest "rotate-all" since it seems akin to the action that you take upon setting it (as opposed to the state of the planes). -Chris
On Thu, Feb 13, 2014 at 02:20:57PM +0000, Chris Wilson wrote: > On Thu, Feb 13, 2014 at 04:06:31PM +0200, Ville Syrjälä wrote: > > On Thu, Feb 13, 2014 at 03:46:39PM +0200, Ville Syrjälä wrote: > > > On Thu, Feb 13, 2014 at 06:07:27PM +0530, Sagar Arun Kamble wrote: > > > > On Wed, 2014-02-12 at 23:15 +0200, ville.syrjala@linux.intel.com wrote: > > > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > > > > > > > I'd prefer have the crtc "rotation" property rotate the entire crtc > > > > > (planes and all). So for that reason we'd need to come up with some > > > > > other name for the "rotate the primary plane only" property. > > > > > > > > > > Originally I had though that omapdrm had already made the decision for > > > > > us, but after another look, it looks like it never attaches the > > > > > "rotation" property to the crtc. So we can still change the name > > > > > without any ABI breakage. > > > > > > > > > > Suggestions for better naming scheme are also welcome.... > > > > I would suggest name to be "primary-rotation" or "primary_rotation". It > > > > seems more aligned to member variable primary_rotation as well. > > > > > > Well, "primary plane" is an Intel term, so I don't know if other people > > > would find it sensible. But I guess you can consider any plane "primary" > > > if it's assigned to act as the crtc scanout engine... > > > > It seems I need to scrap this plan actually. Rob hit me with the clue > > bat on irc, and omapdrm does in fact install the "rotation" prop on > > the crtc. So I guess I need to rename the "rotation" prop to something > > else "crtc-rotation" maybe? Anyone have a good name up their sleeve? > > To recap, you mean that the CRTC rotation property is to be the control > over the rotation of the primary plane, and that we need a new property > name for "rotate the world"? In which case, I'd suggest "rotate-all" > since it seems akin to the action that you take upon setting it (as > opposed to the state of the planes). Yeah something like that. "rotation" vs "rotate-all" isn't super consistent though, but I guess the only real rule about property names is inconsistency, so I'm fine with that idea.
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c index 9232fdf..4a3ef34 100644 --- a/drivers/gpu/drm/i915/i915_dma.c +++ b/drivers/gpu/drm/i915/i915_dma.c @@ -1898,13 +1898,16 @@ void i915_driver_lastclose(struct drm_device * dev) if (!dev_priv) return; - if (dev_priv->rotation_property) { + if (dev_priv->plane_rotation_property) { list_for_each_entry(crtc, &dev->mode_config.crtc_list, base.head) { - crtc->rotation = BIT(DRM_ROTATE_0); + crtc->primary_rotation = BIT(DRM_ROTATE_0); drm_object_property_set_value(&crtc->base.base, - dev_priv->rotation_property, - crtc->rotation); + dev_priv->plane_rotation_property, + crtc->primary_rotation); } + } + + if (dev_priv->rotation_property) { list_for_each_entry(plane, &dev->mode_config.plane_list, base.head) { plane->rotation = BIT(DRM_ROTATE_0); drm_object_property_set_value(&plane->base.base, diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 5a46788..6af78ee 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1560,7 +1560,8 @@ typedef struct drm_i915_private { struct drm_property *broadcast_rgb_property; struct drm_property *force_audio_property; - struct drm_property *rotation_property; + struct drm_property *rotation_property; /* "rotation" */ + struct drm_property *plane_rotation_property; /* "plane-rotation" */ uint32_t hw_context_size; struct list_head context_list; diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index bab17fd..37b23d1 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -2133,7 +2133,7 @@ static int i9xx_update_plane(struct drm_crtc *crtc, struct drm_framebuffer *fb, intel_crtc->dspaddr_offset = linear_offset; } - if (intel_crtc->rotation == BIT(DRM_ROTATE_180)) { + if (intel_crtc->primary_rotation == BIT(DRM_ROTATE_180)) { dspcntr |= DISPPLANE_ROTATE_180; x += (intel_crtc->config.pipe_src_w - 1); @@ -2238,7 +2238,7 @@ static int ironlake_update_plane(struct drm_crtc *crtc, fb->pitches[0]); linear_offset -= intel_crtc->dspaddr_offset; - if (intel_crtc->rotation == BIT(DRM_ROTATE_180)) { + if (intel_crtc->primary_rotation == BIT(DRM_ROTATE_180)) { dspcntr |= DISPPLANE_ROTATE_180; if (!IS_HASWELL(dev) && !IS_BROADWELL(dev)) { @@ -8820,13 +8820,13 @@ static int intel_crtc_set_property(struct drm_crtc *crtc, uint64_t old_val; int ret = -ENOENT; - if (prop == dev_priv->rotation_property) { + if (prop == dev_priv->plane_rotation_property) { /* exactly one rotation angle please */ if (hweight32(val & 0xf) != 1) return -EINVAL; - old_val = intel_crtc->rotation; - intel_crtc->rotation = val; + old_val = intel_crtc->primary_rotation; + intel_crtc->primary_rotation = val; if (intel_crtc->active) { intel_crtc_wait_for_pending_flips(crtc); @@ -8834,12 +8834,12 @@ static int intel_crtc_set_property(struct drm_crtc *crtc, /* FBC does not work on some platforms for rotated planes */ if (dev_priv->fbc.plane == intel_crtc->plane && INTEL_INFO(dev)->gen <= 4 && !IS_G4X(dev) && - intel_crtc->rotation != BIT(DRM_ROTATE_0)) + intel_crtc->primary_rotation != BIT(DRM_ROTATE_0)) intel_disable_fbc(dev); ret = dev_priv->display.update_plane(crtc, crtc->fb, 0, 0); if (ret) - intel_crtc->rotation = old_val; + intel_crtc->primary_rotation = old_val; } } @@ -10387,7 +10387,7 @@ static void intel_crtc_init(struct drm_device *dev, int pipe) */ intel_crtc->pipe = pipe; intel_crtc->plane = pipe; - intel_crtc->rotation = BIT(DRM_ROTATE_0); + intel_crtc->primary_rotation = BIT(DRM_ROTATE_0); if (HAS_FBC(dev) && INTEL_INFO(dev)->gen < 4) { DRM_DEBUG_KMS("swapping pipes & planes for FBC\n"); intel_crtc->plane = !pipe; @@ -10399,15 +10399,15 @@ static void intel_crtc_init(struct drm_device *dev, int pipe) dev_priv->pipe_to_crtc_mapping[intel_crtc->pipe] = &intel_crtc->base; if (INTEL_INFO(dev)->gen >= 4) { - if (!dev_priv->rotation_property) - dev_priv->rotation_property = - drm_mode_create_rotation_property(dev, "rotation", + if (!dev_priv->plane_rotation_property) + dev_priv->plane_rotation_property = + drm_mode_create_rotation_property(dev, "plane-rotation", BIT(DRM_ROTATE_0) | BIT(DRM_ROTATE_180)); - if (dev_priv->rotation_property) + if (dev_priv->plane_rotation_property) drm_object_attach_property(&intel_crtc->base.base, - dev_priv->rotation_property, - intel_crtc->rotation); + dev_priv->plane_rotation_property, + intel_crtc->primary_rotation); } drm_crtc_helper_add(&intel_crtc->base, &intel_helper_funcs); diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 82f0f9a..8c17a82 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -331,7 +331,7 @@ struct intel_crtc { struct drm_crtc base; enum pipe pipe; enum plane plane; - unsigned int rotation; + unsigned int primary_rotation; /* primary plane in relation to the pipe */ u8 lut_r[256], lut_g[256], lut_b[256]; /* diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 9c9ddfe..5ebeb78 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -558,7 +558,7 @@ void intel_update_fbc(struct drm_device *dev) } if (INTEL_INFO(dev)->gen <= 4 && !IS_G4X(dev) && - intel_crtc->rotation != BIT(DRM_ROTATE_0)) { + intel_crtc->primary_rotation != BIT(DRM_ROTATE_0)) { if (set_no_fbc_reason(dev_priv, FBC_UNSUPPORTED_MODE)) DRM_DEBUG_KMS("mode incompatible with compression, " "disabling\n");