From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Subject: Re: [PATCH 2/5] drm/i915: Rename primary plane rotation property to "plane-rotation" Date: Thu, 13 Feb 2014 16:06:31 +0200 Message-ID: <20140213140631.GC3852@intel.com> References: <1392239704-21776-1-git-send-email-ville.syrjala@linux.intel.com> <1392239704-21776-3-git-send-email-ville.syrjala@linux.intel.com> <1392295047.28501.37.camel@sagar-desktop> <20140213134638.GB3852@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Content-Disposition: inline In-Reply-To: <20140213134638.GB3852@intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces@lists.freedesktop.org Errors-To: intel-gfx-bounces@lists.freedesktop.org To: Sagar Arun Kamble Cc: intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org List-Id: dri-devel@lists.freedesktop.org On Thu, Feb 13, 2014 at 03:46:39PM +0200, Ville Syrj=E4l=E4 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=E4l=E4 > > > = > > > 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 su= pport"? > > > = > > > Cc: Sagar Kamble > > > Signed-off-by: Ville Syrj=E4l=E4 > > > --- > > > 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/i= 915_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 =3D BIT(DRM_ROTATE_0); > > > + crtc->primary_rotation =3D 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 =3D 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/i= 915_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/i= 915/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 *c= rtc, struct drm_framebuffer *fb, > > > intel_crtc->dspaddr_offset =3D linear_offset; > > > } > > > = > > > - if (intel_crtc->rotation =3D=3D BIT(DRM_ROTATE_180)) { > > > + if (intel_crtc->primary_rotation =3D=3D BIT(DRM_ROTATE_180)) { > > > dspcntr |=3D DISPPLANE_ROTATE_180; > > > = > > > x +=3D (intel_crtc->config.pipe_src_w - 1); > > > @@ -2238,7 +2238,7 @@ static int ironlake_update_plane(struct drm_crt= c *crtc, > > > fb->pitches[0]); > > > linear_offset -=3D intel_crtc->dspaddr_offset; > > > = > > > - if (intel_crtc->rotation =3D=3D BIT(DRM_ROTATE_180)) { > > > + if (intel_crtc->primary_rotation =3D=3D BIT(DRM_ROTATE_180)) { > > > dspcntr |=3D 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 =3D -ENOENT; > > > = > > > - if (prop =3D=3D dev_priv->rotation_property) { > > > + if (prop =3D=3D dev_priv->plane_rotation_property) { > > > /* exactly one rotation angle please */ > > > if (hweight32(val & 0xf) !=3D 1) > > > return -EINVAL; > > > = > > > - old_val =3D intel_crtc->rotation; > > > - intel_crtc->rotation =3D val; > > > + old_val =3D intel_crtc->primary_rotation; > > > + intel_crtc->primary_rotation =3D 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 =3D=3D intel_crtc->plane && > > > INTEL_INFO(dev)->gen <=3D 4 && !IS_G4X(dev) && > > > - intel_crtc->rotation !=3D BIT(DRM_ROTATE_0)) > > > + intel_crtc->primary_rotation !=3D BIT(DRM_ROTATE_0)) > > > intel_disable_fbc(dev); > > > = > > > ret =3D dev_priv->display.update_plane(crtc, crtc->fb, 0, 0); > > > if (ret) > > > - intel_crtc->rotation =3D old_val; > > > + intel_crtc->primary_rotation =3D old_val; > > > } > > > } > > > = > > > @@ -10387,7 +10387,7 @@ static void intel_crtc_init(struct drm_device= *dev, int pipe) > > > */ > > > intel_crtc->pipe =3D pipe; > > > intel_crtc->plane =3D pipe; > > > - intel_crtc->rotation =3D BIT(DRM_ROTATE_0); > > > + intel_crtc->primary_rotation =3D 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 =3D !pipe; > > > @@ -10399,15 +10399,15 @@ static void intel_crtc_init(struct drm_devi= ce *dev, int pipe) > > > dev_priv->pipe_to_crtc_mapping[intel_crtc->pipe] =3D &intel_crtc->b= ase; > > > = > > > if (INTEL_INFO(dev)->gen >=3D 4) { > > > - if (!dev_priv->rotation_property) > > > - dev_priv->rotation_property =3D > > > - drm_mode_create_rotation_property(dev, "rotation", > > > + if (!dev_priv->plane_rotation_property) > > > + dev_priv->plane_rotation_property =3D > > > + 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/i= ntel_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 <=3D 4 && !IS_G4X(dev) && > > > - intel_crtc->rotation !=3D BIT(DRM_ROTATE_0)) { > > > + intel_crtc->primary_rotation !=3D 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=E4l=E4 > Intel OTC > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- = Ville Syrj=E4l=E4 Intel OTC