On 6/18/2014 10:32 PM, Damien Lespiau wrote:
On Wed, Jun 18, 2014 at 02:27:26PM +0530, sonika.jindal@xxxxxxxxx wrote:
From: Sonika Jindal <sonika.jindal@xxxxxxxxx>
Primary planes support 180 degree rotation. Expose the feature
through rotation drm property.
v2: Calculating linear/tiled offsets based on pipe source width and
height. Added 180 degree rotation support in ironlake_update_plane.
v3: Checking if CRTC is active before issueing update_plane. Added
wait for vblank to make sure we dont overtake page flips. Disabling
FBC since it does not work with rotated planes.
v4: Updated rotation checks for pending flips, fbc disable. Creating
rotation property only for Gen4 onwards. Property resetting as part
of lastclose.
v5: Resetting property in i915_driver_lastclose properly for planes
and crtcs. Fixed linear offset calculation that was off by 1 w.r.t
width in i9xx_update_plane and ironlake_update_plane. Removed tab
based indentation and unnecessary braces in intel_crtc_set_property
and intel_update_fbc. FBC and flip related checks should be done only
for valid crtcs.
v6: Minor nits in FBC disable checks for comments in intel_crtc_set_property
and positioning the disable code in intel_update_fbc.
v7: In case rotation property on inactive crtc is updated, we return
successfully printing debug log as crtc is inactive and only property change
is preserved.
v8: update_plane is changed to update_primary_plane, crtc->fb is changed to
crtc->primary->fb and return value of update_primary_plane is ignored.
v9: added rotation property to primary plane instead of crtc.
Testcase: igt/kms_rotation_crc
Cc: Daniel Vetter <daniel.vetter at ffwll.ch>
Cc: Jani Nikula <jani.nikula at linux.intel.com>
Cc: David Airlie <airlied at linux.ie>
Cc: dri-devel at lists.freedesktop.org
Cc: linux-kernel at vger.kernel.org
Cc: vijay.a.purushothaman at intel.com
Signed-off-by: Uma Shankar <uma.shankar at intel.com>
Signed-off-by: Sagar Kamble <sagar.a.kamble at intel.com>
Some checkpatch.pl warnings:
ERROR: code indent should use tabs where possible
#145: FILE: drivers/gpu/drm/i915/intel_display.c:2488:
+ (intel_crtc->config.pipe_src_w - 1) * pixel_size;$
WARNING: please, no spaces at the start of a line
#145: FILE: drivers/gpu/drm/i915/intel_display.c:2488:
+ (intel_crtc->config.pipe_src_w - 1) * pixel_size;$
---
drivers/gpu/drm/i915/i915_dma.c | 17 +++++++
drivers/gpu/drm/i915/i915_reg.h | 1 +
drivers/gpu/drm/i915/intel_display.c | 91 ++++++++++++++++++++++++++++++++--
drivers/gpu/drm/i915/intel_pm.c | 8 +++
4 files changed, 113 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 5e583a1..4c91fbc 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1939,6 +1939,8 @@ int i915_driver_open(struct drm_device *dev, struct drm_file *file)
void i915_driver_lastclose(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev->dev_private;
+ struct intel_crtc *crtc;
+ struct intel_plane *plane;
/* On gen6+ we refuse to init without kms enabled, but then the drm core
* goes right around and calls lastclose. Check for this and don't clean
@@ -1946,6 +1948,21 @@ void i915_driver_lastclose(struct drm_device *dev)
if (!dev_priv)
return;
+ if (dev_priv->rotation_property) {
+ list_for_each_entry(crtc, &dev->mode_config.crtc_list, base.head) {
+ to_intel_plane(crtc->base.primary)->rotation = BIT(DRM_ROTATE_0);
+ drm_object_property_set_value(&crtc->base.base,
+ dev_priv->rotation_property,
+ to_intel_plane(crtc->base.primary)->rotation);
+ }
+ 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,
+ dev_priv->rotation_property,
+ plane->rotation);
+ }
+ }
+
The purpose of this seems to be restoring rotation to 0 and rely on the next
modeset to re-program the register. This code, is orthogonal to the pure
primary plane rotation enabling, spans through both sprite and primary planes
and may actually be a bit tricky to get right.
-> This shouldn't be part of the same commit as the primary plane rotation.
Please span a new commit for it.
Sure I will add another patch.
Now, we also need something like this when switching VT, and probably for
kernel panic and debug handling as well, so lastclose() is not enough (and we
can probably do better).
One idea would be for the core DRM code to know about this property, and make
sure we put the rotation back to 0 in restore_fbdev_mode(), we do something
similar for the for the sprite planes in there already. Another idea would be
to add a vfunc to execute driver specific code there in restore_fbdev_mode().
There is probably a better way to do it, I have to say I'm not super familiar
with this part of the driver.
I see that in omap driver too it is done in lastclose of the driver.
Also, from driver fbdev_restore is only called during lastclose.
Again I don't have more knowledge on this.
Can we keep it here in this lastclose function to comply with omap driver?
if (drm_core_check_feature(dev, DRIVER_MODESET)) {
intel_fbdev_restore_mode(dev);
vga_switcheroo_process_delayed_switch();
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index c70c804..c600d3b 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4087,6 +4087,7 @@ enum punit_power_well {
#define DISPPLANE_NO_LINE_DOUBLE 0
#define DISPPLANE_STEREO_POLARITY_FIRST 0
#define DISPPLANE_STEREO_POLARITY_SECOND (1<<18)
+#define DISPPLANE_ROTATE_180 (1<<15)
#define DISPPLANE_TRICKLE_FEED_DISABLE (1<<14) /* Ironlake */
#define DISPPLANE_TILED (1<<10)
#define _DSPAADDR 0x70184
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 5e8e711..1dc8b68 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2414,7 +2414,9 @@ static void i9xx_update_primary_plane(struct drm_crtc *crtc,
unsigned long linear_offset;
u32 dspcntr;
u32 reg;
+ int pixel_size;
+ pixel_size = drm_format_plane_cpp(fb->pixel_format, 0);
intel_fb = to_intel_framebuffer(fb);
obj = intel_fb->obj;
@@ -2422,6 +2424,8 @@ static void i9xx_update_primary_plane(struct drm_crtc *crtc,
dspcntr = I915_READ(reg);
/* Mask out pixel format bits in case we change it */
dspcntr &= ~DISPPLANE_PIXFORMAT_MASK;
+ dspcntr &= ~DISPPLANE_ROTATE_180;
+
switch (fb->pixel_format) {
case DRM_FORMAT_C8:
dspcntr |= DISPPLANE_8BPP;
@@ -2463,8 +2467,6 @@ static void i9xx_update_primary_plane(struct drm_crtc *crtc,
if (IS_G4X(dev))
dspcntr |= DISPPLANE_TRICKLE_FEED_DISABLE;
- I915_WRITE(reg, dspcntr);
-
linear_offset = y * fb->pitches[0] + x * (fb->bits_per_pixel / 8);
if (INTEL_INFO(dev)->gen >= 4) {
@@ -2477,6 +2479,17 @@ static void i9xx_update_primary_plane(struct drm_crtc *crtc,
intel_crtc->dspaddr_offset = linear_offset;
}
+ if (to_intel_plane(crtc->primary)->rotation == BIT(DRM_ROTATE_180)) {
+ dspcntr |= DISPPLANE_ROTATE_180;
+
+ x += (intel_crtc->config.pipe_src_w - 1);
+ y += (intel_crtc->config.pipe_src_h - 1);
+ linear_offset += (intel_crtc->config.pipe_src_h - 1) * fb->pitches[0] +
+ (intel_crtc->config.pipe_src_w - 1) * pixel_size;
+ }
+
+ I915_WRITE(reg, dspcntr);
+
DRM_DEBUG_KMS("Writing base %08lX %08lX %d %d %d\n",
i915_gem_obj_ggtt_offset(obj), linear_offset, x, y,
fb->pitches[0]);
@@ -2504,7 +2517,9 @@ static void ironlake_update_primary_plane(struct drm_crtc *crtc,
unsigned long linear_offset;
u32 dspcntr;
u32 reg;
+ int pixel_size;
+ pixel_size = drm_format_plane_cpp(fb->pixel_format, 0);
intel_fb = to_intel_framebuffer(fb);
obj = intel_fb->obj;
@@ -2512,6 +2527,8 @@ static void ironlake_update_primary_plane(struct drm_crtc *crtc,
dspcntr = I915_READ(reg);
/* Mask out pixel format bits in case we change it */
dspcntr &= ~DISPPLANE_PIXFORMAT_MASK;
+ dspcntr &= ~DISPPLANE_ROTATE_180;
+
switch (fb->pixel_format) {
case DRM_FORMAT_C8:
dspcntr |= DISPPLANE_8BPP;
@@ -2549,8 +2566,6 @@ static void ironlake_update_primary_plane(struct drm_crtc *crtc,
else
dspcntr |= DISPPLANE_TRICKLE_FEED_DISABLE;
- I915_WRITE(reg, dspcntr);
-
linear_offset = y * fb->pitches[0] + x * (fb->bits_per_pixel / 8);
intel_crtc->dspaddr_offset =
intel_gen4_compute_page_offset(&x, &y, obj->tiling_mode,
@@ -2558,6 +2573,19 @@ static void ironlake_update_primary_plane(struct drm_crtc *crtc,
fb->pitches[0]);
linear_offset -= intel_crtc->dspaddr_offset;
+ if (to_intel_plane(crtc->primary)->rotation == BIT(DRM_ROTATE_180)) {
+ dspcntr |= DISPPLANE_ROTATE_180;
+
+ if (!IS_HASWELL(dev) && !IS_BROADWELL(dev)) {
+ x += (intel_crtc->config.pipe_src_w - 1);
+ y += (intel_crtc->config.pipe_src_h - 1);
+ linear_offset += (intel_crtc->config.pipe_src_h - 1) * fb->pitches[0] +
+ (intel_crtc->config.pipe_src_w - 1) * pixel_size;
+ }
+ }
+
+ I915_WRITE(reg, dspcntr);
+
DRM_DEBUG_KMS("Writing base %08lX %08lX %d %d %d\n",
i915_gem_obj_ggtt_offset(obj), linear_offset, x, y,
fb->pitches[0]);
@@ -11324,10 +11352,51 @@ static void intel_plane_destroy(struct drm_plane *plane)
kfree(intel_plane);
}
+static int intel_primary_plane_set_property(struct drm_plane *plane,
+ struct drm_property *prop,
+ uint64_t val)
+{
+ struct drm_device *dev = plane->dev;
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ struct intel_plane *intel_plane = to_intel_plane(plane);
+ struct intel_crtc *intel_crtc = to_intel_crtc(plane->crtc);
+ struct drm_crtc *crtc = &intel_crtc->base;
+ uint64_t old_val;
+ int ret = -ENOENT;
+
+ if (prop == dev_priv->rotation_property) {
+ /* exactly one rotation angle please */
+ if (hweight32(val & 0xf) != 1)
+ return -EINVAL;
+
+ old_val = intel_plane->rotation;
This value is set but never used again?
This was a miss from the last version of the patch. Will remove it.
+ intel_plane->rotation = val;
+
+ if (intel_crtc->active && intel_crtc->primary_enabled) {
+ intel_crtc_wait_for_pending_flips(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_plane->rotation != BIT(DRM_ROTATE_0))
+ intel_disable_fbc(dev);
+
+ dev_priv->display.update_primary_plane(crtc, crtc->primary->fb, 0, 0);
+ } else {
+ DRM_DEBUG_KMS("[CRTC:%d] is not active. Only rotation property is updated\n",
+ crtc->base.id);
+ ret = 0;
+ }
+ }
+
+ return ret;
+}
+
static const struct drm_plane_funcs intel_primary_plane_funcs = {
.update_plane = intel_primary_plane_setplane,
.disable_plane = intel_primary_plane_disable,
.destroy = intel_plane_destroy,
+ .set_property = intel_primary_plane_set_property
};
static struct drm_plane *intel_primary_plane_create(struct drm_device *dev,
@@ -11335,6 +11404,7 @@ static struct drm_plane *intel_primary_plane_create(struct drm_device *dev,
{
struct intel_plane *primary;
const uint32_t *intel_primary_formats;
+ struct drm_i915_private *dev_priv = dev->dev_private;
int num_formats;
primary = kzalloc(sizeof(*primary), GFP_KERNEL);
@@ -11345,6 +11415,7 @@ static struct drm_plane *intel_primary_plane_create(struct drm_device *dev,
primary->max_downscale = 1;
primary->pipe = pipe;
primary->plane = pipe;
+ primary->rotation = BIT(DRM_ROTATE_0);
if (HAS_FBC(dev) && INTEL_INFO(dev)->gen < 4)
primary->plane = !pipe;
@@ -11360,6 +11431,18 @@ static struct drm_plane *intel_primary_plane_create(struct drm_device *dev,
&intel_primary_plane_funcs,
intel_primary_formats, num_formats,
DRM_PLANE_TYPE_PRIMARY);
+ if (INTEL_INFO(dev)->gen >= 4) {
+ if (!dev_priv->rotation_property)
+ dev_priv->rotation_property =
+ drm_mode_create_rotation_property(dev,
+ BIT(DRM_ROTATE_0) |
+ BIT(DRM_ROTATE_180));
+ if (dev_priv->rotation_property)
+ drm_object_attach_property(&primary->base.base,
+ dev_priv->rotation_property,
+ primary->rotation);
+ }
+
return &primary->base;
}
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 2043c4b..cabccfb 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -562,6 +562,14 @@ void intel_update_fbc(struct drm_device *dev)
goto out_disable;
}
+ if (INTEL_INFO(dev)->gen <= 4 && !IS_G4X(dev) &&
+ to_intel_plane(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");
This debug message isn't helpful. You need to add that's because of the
rotation.
Sure, will add.
+ goto out_disable;
+ }
+
/* If the kernel debugger is active, always disable compression */
if (in_dbg_master())
goto out_disable;
--
1.7.10.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
