| Message ID | 000C66961D35964B9714611E548C10AD0C0E9815@BGSMSX104.gar.corp.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Oct 07, 2014 at 08:43:46AM +0000, Jindal, Sonika wrote: > Hi, > > Did anybody get a chance to look at this patch? > > Thanks, > Sonika Looks like we waited a bit too long and the codebase has evolved, so I needed to make some tweaks to your patches to get them to apply cleanly on the latest di-nightly. However the overall changes here look good to me, and we don't seem to be missing any details from the bspec, so Reviewed-by: Matt Roper <matthew.d.roper@intel.com> After tweaking your patches to apply against the latest di-nightly, your i-g-t test runs properly for me on IVB, as well as another simple test I wrote myself, so you can also put Tested-by: Matt Roper <matthew.d.roper@intel.com> Let me know if you want the rebased copies of the patches I used for testing. Matt > > -----Original Message----- > From: Jindal, Sonika > Sent: Monday, September 15, 2014 1:14 PM > To: intel-gfx@lists.freedesktop.org > Cc: Ville Syrjälä; Kamble, Sagar A; Jindal, Sonika > Subject: [PATCH] drm/i915: Add rotation support for cursor plane > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > The cursor plane also supports 180 degree rotation. Add a new "cursor-rotation" property on the crtc which controls this. > > Unlike sprites, the cursor has a fixed size, so if you have a small cursor image with the rest of the bo filled by transparent pixels, simply flipping the rotation property will cause the visible part of the cursor to shift. This is something to keep in mind when using cursor rotation. > > v2: Fix gen4/vlv by offsetting the base address appropriately > > v3: Removing cursor-rotation property and using rotation property on cursor plane. > v4: Changing the author name back to Ville. > > Testcase: kms_rotation_crc > > Cc: Sagar Kamble <sagar.a.kamble@intel.com> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > Signed-off-by: Sonika Jindal <sonika.jindal@intel.com> > --- > drivers/gpu/drm/i915/i915_reg.h | 1 + > drivers/gpu/drm/i915/intel_display.c | 26 +++++++++++++++++++++++++- > 2 files changed, 26 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 15c0eaa..5a9fab9 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -4162,6 +4162,7 @@ enum punit_power_well { > #define MCURSOR_PIPE_A 0x00 > #define MCURSOR_PIPE_B (1 << 28) > #define MCURSOR_GAMMA_ENABLE (1 << 26) > +#define CURSOR_ROTATE_180 (1<<15) > #define CURSOR_TRICKLE_FEED_DISABLE (1 << 14) > #define _CURABASE 0x70084 > #define _CURAPOS 0x70088 > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 122ac6e..8c83bcc 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -8247,6 +8247,9 @@ static void i9xx_update_cursor(struct drm_crtc *crtc, u32 base) > if (IS_HASWELL(dev) || IS_BROADWELL(dev)) > cntl |= CURSOR_PIPE_CSC_ENABLE; > > + if (to_intel_plane(crtc->cursor)->rotation == BIT(DRM_ROTATE_180)) > + cntl |= CURSOR_ROTATE_180; > + > if (intel_crtc->cursor_cntl != cntl) { > I915_WRITE(CURCNTR(pipe), cntl); > POSTING_READ(CURCNTR(pipe)); > @@ -8302,6 +8305,13 @@ static void intel_crtc_update_cursor(struct drm_crtc *crtc, > > I915_WRITE(CURPOS(pipe), pos); > > + /* ILK+ do this automagically */ Minor note, but it might be nice to clarify this with a little more detail. Something like "Gen4 and Valley View expect the address to point to the lower right corner for 180-rotated cursors. Other platforms expect the address to point to the top-left corner regardless of rotation." > + if (HAS_GMCH_DISPLAY(dev) && > + to_intel_plane(crtc->cursor)->rotation == BIT(DRM_ROTATE_180)) { > + base += (intel_crtc->cursor_height * > + intel_crtc->cursor_width - 1) * 4; > + } > + > if (IS_845G(dev) || IS_I865G(dev)) > i845_update_cursor(crtc, base); > else > @@ -8453,7 +8463,6 @@ static int intel_crtc_cursor_set_obj(struct drm_crtc *crtc, > mutex_unlock(&dev->struct_mutex); > > old_width = intel_crtc->cursor_width; > - Unintentional whitespace change? > intel_crtc->cursor_addr = addr; > intel_crtc->cursor_bo = obj; > intel_crtc->cursor_width = width; > @@ -12074,6 +12083,7 @@ static const struct drm_plane_funcs intel_cursor_plane_funcs = { > .update_plane = intel_cursor_plane_update, > .disable_plane = intel_cursor_plane_disable, > .destroy = intel_plane_destroy, > + .set_property = intel_plane_set_property, > }; > > static struct drm_plane *intel_cursor_plane_create(struct drm_device *dev, @@ -12089,12 +12099,26 @@ static struct drm_plane *intel_cursor_plane_create(struct drm_device *dev, > cursor->max_downscale = 1; > cursor->pipe = pipe; > cursor->plane = pipe; > + cursor->rotation = BIT(DRM_ROTATE_0); > > drm_universal_plane_init(dev, &cursor->base, 0, > &intel_cursor_plane_funcs, > intel_cursor_formats, > ARRAY_SIZE(intel_cursor_formats), > DRM_PLANE_TYPE_CURSOR); > + > + 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, > + cursor->rotation); > + } > + > return &cursor->base; > } > > -- > 1.7.10.4 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Wed, Oct 22, 2014 at 05:29:24PM -0700, Matt Roper wrote: > On Tue, Oct 07, 2014 at 08:43:46AM +0000, Jindal, Sonika wrote: > > Hi, > > > > Did anybody get a chance to look at this patch? > > > > Thanks, > > Sonika > > Looks like we waited a bit too long and the codebase has evolved, so I > needed to make some tweaks to your patches to get them to apply cleanly > on the latest di-nightly. However the overall changes here look good to > me, and we don't seem to be missing any details from the bspec, so > > Reviewed-by: Matt Roper <matthew.d.roper@intel.com> > > After tweaking your patches to apply against the latest di-nightly, your > i-g-t test runs properly for me on IVB, as well as another simple test I > wrote myself, so you can also put > > Tested-by: Matt Roper <matthew.d.roper@intel.com> > > Let me know if you want the rebased copies of the patches I used for > testing. Lazy maintainer here. That would be awesome ;-) Since you've frobbed the patches a bit pls also put your sob onto them. Thanks, Daniel
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 15c0eaa..5a9fab9 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -4162,6 +4162,7 @@ enum punit_power_well { #define MCURSOR_PIPE_A 0x00 #define MCURSOR_PIPE_B (1 << 28) #define MCURSOR_GAMMA_ENABLE (1 << 26) +#define CURSOR_ROTATE_180 (1<<15) #define CURSOR_TRICKLE_FEED_DISABLE (1 << 14) #define _CURABASE 0x70084 #define _CURAPOS 0x70088 diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 122ac6e..8c83bcc 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -8247,6 +8247,9 @@ static void i9xx_update_cursor(struct drm_crtc *crtc, u32 base) if (IS_HASWELL(dev) || IS_BROADWELL(dev)) cntl |= CURSOR_PIPE_CSC_ENABLE; + if (to_intel_plane(crtc->cursor)->rotation == BIT(DRM_ROTATE_180)) + cntl |= CURSOR_ROTATE_180; + if (intel_crtc->cursor_cntl != cntl) { I915_WRITE(CURCNTR(pipe), cntl); POSTING_READ(CURCNTR(pipe)); @@ -8302,6 +8305,13 @@ static void intel_crtc_update_cursor(struct drm_crtc *crtc, I915_WRITE(CURPOS(pipe), pos); + /* ILK+ do this automagically */ + if (HAS_GMCH_DISPLAY(dev) && + to_intel_plane(crtc->cursor)->rotation == BIT(DRM_ROTATE_180)) { + base += (intel_crtc->cursor_height * + intel_crtc->cursor_width - 1) * 4; + } + if (IS_845G(dev) || IS_I865G(dev)) i845_update_cursor(crtc, base); else @@ -8453,7 +8463,6 @@ static int intel_crtc_cursor_set_obj(struct drm_crtc *crtc, mutex_unlock(&dev->struct_mutex); old_width = intel_crtc->cursor_width; - intel_crtc->cursor_addr = addr; intel_crtc->cursor_bo = obj; intel_crtc->cursor_width = width; @@ -12074,6 +12083,7 @@ static const struct drm_plane_funcs intel_cursor_plane_funcs = { .update_plane = intel_cursor_plane_update, .disable_plane = intel_cursor_plane_disable, .destroy = intel_plane_destroy, + .set_property = intel_plane_set_property, }; static struct drm_plane *intel_cursor_plane_create(struct drm_device *dev, @@ -12089,12 +12099,26 @@ static struct drm_plane *intel_cursor_plane_create(struct drm_device *dev,