* [PATCH 1/5] drm: Pass name to drm_rotation_property_create()
2014-02-12 21:14 [PATCH 0/5] drm/i915: Full pipe rotation & rotation property name bikeshedding ville.syrjala
@ 2014-02-12 21:15 ` ville.syrjala
2014-02-13 10:42 ` Sagar Arun Kamble
2014-02-12 21:15 ` [PATCH 2/5] drm/i915: Rename primary plane rotation property to "plane-rotation" ville.syrjala
` (5 subsequent siblings)
6 siblings, 1 reply; 24+ messages in thread
From: ville.syrjala @ 2014-02-12 21:15 UTC (permalink / raw)
To: intel-gfx; +Cc: Sagar Kamble, dri-devel
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
Allow rotation properties to have custom names.
TODO: maybe squash into "drm: Add drm_mode_create_rotation_property()"
Cc: Sagar Kamble <sagar.a.kamble@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/drm_crtc.c | 3 ++-
drivers/gpu/drm/i915/intel_display.c | 2 +-
drivers/gpu/drm/i915/intel_sprite.c | 2 +-
drivers/gpu/drm/omapdrm/omap_plane.c | 2 +-
include/drm/drm_crtc.h | 1 +
5 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index fe04889..7077676 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -4179,6 +4179,7 @@ void drm_mode_config_cleanup(struct drm_device *dev)
EXPORT_SYMBOL(drm_mode_config_cleanup);
struct drm_property *drm_mode_create_rotation_property(struct drm_device *dev,
+ const char *name,
unsigned int supported_rotations)
{
static const struct drm_prop_enum_list props[] = {
@@ -4190,7 +4191,7 @@ struct drm_property *drm_mode_create_rotation_property(struct drm_device *dev,
{ DRM_REFLECT_Y, "reflect-y" },
};
- return drm_property_create_bitmask(dev, 0, "rotation",
+ return drm_property_create_bitmask(dev, 0, name,
props, ARRAY_SIZE(props),
supported_rotations);
}
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 050b249..bab17fd 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -10401,7 +10401,7 @@ static void intel_crtc_init(struct drm_device *dev, int pipe)
if (INTEL_INFO(dev)->gen >= 4) {
if (!dev_priv->rotation_property)
dev_priv->rotation_property =
- drm_mode_create_rotation_property(dev,
+ drm_mode_create_rotation_property(dev, "rotation",
BIT(DRM_ROTATE_0) |
BIT(DRM_ROTATE_180));
if (dev_priv->rotation_property)
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 7dcce4e..2936007 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -1216,7 +1216,7 @@ intel_plane_init(struct drm_device *dev, enum pipe pipe, int plane)
if (!dev_priv->rotation_property)
dev_priv->rotation_property =
- drm_mode_create_rotation_property(dev,
+ drm_mode_create_rotation_property(dev, "rotation",
BIT(DRM_ROTATE_0) |
BIT(DRM_ROTATE_180));
diff --git a/drivers/gpu/drm/omapdrm/omap_plane.c b/drivers/gpu/drm/omapdrm/omap_plane.c
index e4a3fd1..72b9dc7 100644
--- a/drivers/gpu/drm/omapdrm/omap_plane.c
+++ b/drivers/gpu/drm/omapdrm/omap_plane.c
@@ -300,7 +300,7 @@ void omap_plane_install_properties(struct drm_plane *plane,
if (priv->has_dmm) {
prop = priv->rotation_prop;
if (!prop) {
- prop = drm_mode_create_rotation_property(dev,
+ prop = drm_mode_create_rotation_property(dev, "rotation",
BIT(DRM_ROTATE_0) | BIT(DRM_ROTATE_90) |
BIT(DRM_ROTATE_180) | BIT(DRM_ROTATE_270) |
BIT(DRM_REFLECT_X) | BIT(DRM_REFLECT_Y));
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index e1c0aba..ee84a4a 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -1185,6 +1185,7 @@ extern int drm_format_horz_chroma_subsampling(uint32_t format);
extern int drm_format_vert_chroma_subsampling(uint32_t format);
extern const char *drm_get_format_name(uint32_t format);
extern struct drm_property *drm_mode_create_rotation_property(struct drm_device *dev,
+ const char *name,
unsigned int supported_rotations);
extern unsigned int drm_rotation_simplify(unsigned int rotation,
unsigned int supported_rotations);
--
1.8.3.2
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [PATCH 1/5] drm: Pass name to drm_rotation_property_create()
2014-02-12 21:15 ` [PATCH 1/5] drm: Pass name to drm_rotation_property_create() ville.syrjala
@ 2014-02-13 10:42 ` Sagar Arun Kamble
0 siblings, 0 replies; 24+ messages in thread
From: Sagar Arun Kamble @ 2014-02-13 10:42 UTC (permalink / raw)
To: ville.syrjala; +Cc: intel-gfx, dri-devel
Reviewed-by: Sagar Kamble <sagar.a.kamble@intel.com>
On Wed, 2014-02-12 at 23:15 +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Allow rotation properties to have custom names.
>
> TODO: maybe squash into "drm: Add drm_mode_create_rotation_property()"
>
> Cc: Sagar Kamble <sagar.a.kamble@intel.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
> drivers/gpu/drm/drm_crtc.c | 3 ++-
> drivers/gpu/drm/i915/intel_display.c | 2 +-
> drivers/gpu/drm/i915/intel_sprite.c | 2 +-
> drivers/gpu/drm/omapdrm/omap_plane.c | 2 +-
> include/drm/drm_crtc.h | 1 +
> 5 files changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index fe04889..7077676 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -4179,6 +4179,7 @@ void drm_mode_config_cleanup(struct drm_device *dev)
> EXPORT_SYMBOL(drm_mode_config_cleanup);
>
> struct drm_property *drm_mode_create_rotation_property(struct drm_device *dev,
> + const char *name,
> unsigned int supported_rotations)
> {
> static const struct drm_prop_enum_list props[] = {
> @@ -4190,7 +4191,7 @@ struct drm_property *drm_mode_create_rotation_property(struct drm_device *dev,
> { DRM_REFLECT_Y, "reflect-y" },
> };
>
> - return drm_property_create_bitmask(dev, 0, "rotation",
> + return drm_property_create_bitmask(dev, 0, name,
> props, ARRAY_SIZE(props),
> supported_rotations);
> }
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 050b249..bab17fd 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -10401,7 +10401,7 @@ static void intel_crtc_init(struct drm_device *dev, int pipe)
> if (INTEL_INFO(dev)->gen >= 4) {
> if (!dev_priv->rotation_property)
> dev_priv->rotation_property =
> - drm_mode_create_rotation_property(dev,
> + drm_mode_create_rotation_property(dev, "rotation",
> BIT(DRM_ROTATE_0) |
> BIT(DRM_ROTATE_180));
> if (dev_priv->rotation_property)
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> index 7dcce4e..2936007 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -1216,7 +1216,7 @@ intel_plane_init(struct drm_device *dev, enum pipe pipe, int plane)
>
> if (!dev_priv->rotation_property)
> dev_priv->rotation_property =
> - drm_mode_create_rotation_property(dev,
> + drm_mode_create_rotation_property(dev, "rotation",
> BIT(DRM_ROTATE_0) |
> BIT(DRM_ROTATE_180));
>
> diff --git a/drivers/gpu/drm/omapdrm/omap_plane.c b/drivers/gpu/drm/omapdrm/omap_plane.c
> index e4a3fd1..72b9dc7 100644
> --- a/drivers/gpu/drm/omapdrm/omap_plane.c
> +++ b/drivers/gpu/drm/omapdrm/omap_plane.c
> @@ -300,7 +300,7 @@ void omap_plane_install_properties(struct drm_plane *plane,
> if (priv->has_dmm) {
> prop = priv->rotation_prop;
> if (!prop) {
> - prop = drm_mode_create_rotation_property(dev,
> + prop = drm_mode_create_rotation_property(dev, "rotation",
> BIT(DRM_ROTATE_0) | BIT(DRM_ROTATE_90) |
> BIT(DRM_ROTATE_180) | BIT(DRM_ROTATE_270) |
> BIT(DRM_REFLECT_X) | BIT(DRM_REFLECT_Y));
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index e1c0aba..ee84a4a 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -1185,6 +1185,7 @@ extern int drm_format_horz_chroma_subsampling(uint32_t format);
> extern int drm_format_vert_chroma_subsampling(uint32_t format);
> extern const char *drm_get_format_name(uint32_t format);
> extern struct drm_property *drm_mode_create_rotation_property(struct drm_device *dev,
> + const char *name,
> unsigned int supported_rotations);
> extern unsigned int drm_rotation_simplify(unsigned int rotation,
> unsigned int supported_rotations);
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 2/5] drm/i915: Rename primary plane rotation property to "plane-rotation"
2014-02-12 21:14 [PATCH 0/5] drm/i915: Full pipe rotation & rotation property name bikeshedding ville.syrjala
2014-02-12 21:15 ` [PATCH 1/5] drm: Pass name to drm_rotation_property_create() ville.syrjala
@ 2014-02-12 21:15 ` ville.syrjala
2014-02-13 12:37 ` Sagar Arun Kamble
2014-02-12 21:15 ` [PATCH 3/5] drm: Add drm_rotation_chain() ville.syrjala
` (4 subsequent siblings)
6 siblings, 1 reply; 24+ messages in thread
From: ville.syrjala @ 2014-02-12 21:15 UTC (permalink / raw)
To: intel-gfx; +Cc: Sagar Kamble, dri-devel
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....
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");
--
1.8.3.2
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [PATCH 2/5] drm/i915: Rename primary plane rotation property to "plane-rotation"
2014-02-12 21:15 ` [PATCH 2/5] drm/i915: Rename primary plane rotation property to "plane-rotation" ville.syrjala
@ 2014-02-13 12:37 ` Sagar Arun Kamble
2014-02-13 13:46 ` Ville Syrjälä
0 siblings, 1 reply; 24+ messages in thread
From: Sagar Arun Kamble @ 2014-02-13 12:37 UTC (permalink / raw)
To: ville.syrjala; +Cc: intel-gfx, dri-devel
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");
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH 2/5] drm/i915: Rename primary plane rotation property to "plane-rotation"
2014-02-13 12:37 ` Sagar Arun Kamble
@ 2014-02-13 13:46 ` Ville Syrjälä
2014-02-13 14:06 ` Ville Syrjälä
0 siblings, 1 reply; 24+ messages in thread
From: Ville Syrjälä @ 2014-02-13 13:46 UTC (permalink / raw)
To: Sagar Arun Kamble; +Cc: intel-gfx, dri-devel
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");
>
--
Ville Syrjälä
Intel OTC
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH 2/5] drm/i915: Rename primary plane rotation property to "plane-rotation"
2014-02-13 13:46 ` Ville Syrjälä
@ 2014-02-13 14:06 ` Ville Syrjälä
2014-02-13 14:20 ` [Intel-gfx] " Chris Wilson
0 siblings, 1 reply; 24+ messages in thread
From: Ville Syrjälä @ 2014-02-13 14:06 UTC (permalink / raw)
To: Sagar Arun Kamble; +Cc: intel-gfx, dri-devel
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
--
Ville Syrjälä
Intel OTC
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [Intel-gfx] [PATCH 2/5] drm/i915: Rename primary plane rotation property to "plane-rotation"
2014-02-13 14:06 ` Ville Syrjälä
@ 2014-02-13 14:20 ` Chris Wilson
2014-02-13 14:40 ` Ville Syrjälä
0 siblings, 1 reply; 24+ messages in thread
From: Chris Wilson @ 2014-02-13 14:20 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: intel-gfx, Sagar Arun Kamble, dri-devel
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
--
Chris Wilson, Intel Open Source Technology Centre
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Intel-gfx] [PATCH 2/5] drm/i915: Rename primary plane rotation property to "plane-rotation"
2014-02-13 14:20 ` [Intel-gfx] " Chris Wilson
@ 2014-02-13 14:40 ` Ville Syrjälä
0 siblings, 0 replies; 24+ messages in thread
From: Ville Syrjälä @ 2014-02-13 14:40 UTC (permalink / raw)
To: Chris Wilson, Sagar Arun Kamble, intel-gfx, dri-devel
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.
--
Ville Syrjälä
Intel OTC
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 3/5] drm: Add drm_rotation_chain()
2014-02-12 21:14 [PATCH 0/5] drm/i915: Full pipe rotation & rotation property name bikeshedding ville.syrjala
2014-02-12 21:15 ` [PATCH 1/5] drm: Pass name to drm_rotation_property_create() ville.syrjala
2014-02-12 21:15 ` [PATCH 2/5] drm/i915: Rename primary plane rotation property to "plane-rotation" ville.syrjala
@ 2014-02-12 21:15 ` ville.syrjala
2014-02-12 21:15 ` [PATCH 4/5] drm/i915: Add rotation support for the cursor plane ville.syrjala
` (3 subsequent siblings)
6 siblings, 0 replies; 24+ messages in thread
From: ville.syrjala @ 2014-02-12 21:15 UTC (permalink / raw)
To: intel-gfx; +Cc: Sagar Kamble, dri-devel
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
drm_rotation_chain() can be used to combine the plane and crtc rotations
to calculate the total rotation for a specific plane.
Cc: Sagar Kamble <sagar.a.kamble@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/drm_crtc.c | 42 ++++++++++++++++++++++++++++++++++++++++++
include/drm/drm_crtc.h | 2 ++
2 files changed, 44 insertions(+)
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 7077676..71d366f 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -4065,6 +4065,48 @@ unsigned int drm_rotation_simplify(unsigned int rotation,
EXPORT_SYMBOL(drm_rotation_simplify);
/**
+ * drm_rotation_chain() - Chain plane and crtc rotations together
+ * @crtc_rotation: rotation for the entire crtc
+ * @plane_rotation: rotation for the plane in relation to the crtc
+ *
+ * Given @crtc_rotation and @plane_rotation, this function will
+ * comptute the total rotation for the plane in relation to 0
+ * degree rotated crtc. This can be used eg. to implement full crtc
+ * rotation using just plane rotation in hardware. Obviously that
+ * requires that all planes must support rotation since the crtc
+ * itself won't rotate the composed scene.
+ */
+unsigned int drm_rotation_chain(unsigned int crtc_rotation,
+ unsigned int plane_rotation)
+{
+ unsigned int rotation;
+
+ /* add up the rotation angles */
+ rotation = 1 << ((ffs(plane_rotation & 0xf) + ffs(crtc_rotation & 0xf) - 2) % 4);
+
+ /* plane reflection before plane rotation */
+ rotation |= plane_rotation & (BIT(DRM_REFLECT_X) | BIT(DRM_REFLECT_Y));
+
+ /* crtc reflection after plane rotation and before crtc rotation */
+ switch (plane_rotation & 0xf) {
+ case BIT(DRM_ROTATE_0):
+ case BIT(DRM_ROTATE_180):
+ rotation ^= crtc_rotation & (BIT(DRM_REFLECT_X) | BIT(DRM_REFLECT_Y));
+ break;
+ case BIT(DRM_ROTATE_90):
+ case BIT(DRM_ROTATE_270):
+ if (crtc_rotation & BIT(DRM_REFLECT_X))
+ rotation ^= BIT(DRM_REFLECT_Y);
+ if (crtc_rotation & BIT(DRM_REFLECT_Y))
+ rotation ^= BIT(DRM_REFLECT_X);
+ break;
+ }
+
+ return rotation;
+}
+EXPORT_SYMBOL(drm_rotation_chain);
+
+/**
* drm_mode_config_init - initialize DRM mode_configuration structure
* @dev: DRM device
*
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index ee84a4a..ebb9bf4 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -1189,6 +1189,8 @@ extern struct drm_property *drm_mode_create_rotation_property(struct drm_device
unsigned int supported_rotations);
extern unsigned int drm_rotation_simplify(unsigned int rotation,
unsigned int supported_rotations);
+extern unsigned int drm_rotation_chain(unsigned int crtc_rotation,
+ unsigned int plane_rotation);
/* Helpers */
static inline struct drm_crtc *drm_crtc_find(struct drm_device *dev,
--
1.8.3.2
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH 4/5] drm/i915: Add rotation support for the cursor plane
2014-02-12 21:14 [PATCH 0/5] drm/i915: Full pipe rotation & rotation property name bikeshedding ville.syrjala
` (2 preceding siblings ...)
2014-02-12 21:15 ` [PATCH 3/5] drm: Add drm_rotation_chain() ville.syrjala
@ 2014-02-12 21:15 ` ville.syrjala
2014-02-14 11:01 ` Sagar Arun Kamble
2014-02-12 21:15 ` [PATCH 5/5] drm/i915: Add full pipe rotation ville.syrjala
` (2 subsequent siblings)
6 siblings, 1 reply; 24+ messages in thread
From: ville.syrjala @ 2014-02-12 21:15 UTC (permalink / raw)
To: intel-gfx; +Cc: Sagar Kamble, dri-devel
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.
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 | 9 ++++++
drivers/gpu/drm/i915/i915_drv.h | 1 +
drivers/gpu/drm/i915/i915_reg.h | 1 +
drivers/gpu/drm/i915/intel_display.c | 59 ++++++++++++++++++++++++------------
drivers/gpu/drm/i915/intel_drv.h | 1 +
5 files changed, 51 insertions(+), 20 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 4a3ef34..3dd9abb 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1916,6 +1916,15 @@ void i915_driver_lastclose(struct drm_device * dev)
}
}
+ if (dev_priv->cursor_rotation_property) {
+ list_for_each_entry(crtc, &dev->mode_config.crtc_list, base.head) {
+ crtc->cursor_rotation = BIT(DRM_ROTATE_0);
+ drm_object_property_set_value(&crtc->base.base,
+ dev_priv->cursor_rotation_property,
+ crtc->cursor_rotation);
+ }
+ }
+
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_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 6af78ee..8ecd3bb 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1562,6 +1562,7 @@ typedef struct drm_i915_private {
struct drm_property *force_audio_property;
struct drm_property *rotation_property; /* "rotation" */
struct drm_property *plane_rotation_property; /* "plane-rotation" */
+ struct drm_property *cursor_rotation_property; /* "cursor-rotation" */
uint32_t hw_context_size;
struct list_head context_list;
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index ab1aeb82..fee068a 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -3534,6 +3534,7 @@
#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 (dev_priv->info.display_mmio_offset + 0x70084)
#define _CURAPOS (dev_priv->info.display_mmio_offset + 0x70088)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 37b23d1..e94167b 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -42,7 +42,7 @@
#include <linux/dma_remapping.h>
static void intel_increase_pllclock(struct drm_crtc *crtc);
-static void intel_crtc_update_cursor(struct drm_crtc *crtc, bool on);
+static void intel_crtc_update_cursor(struct drm_crtc *crtc, bool on, bool force);
static void i9xx_crtc_clock_get(struct intel_crtc *crtc,
struct intel_crtc_config *pipe_config);
@@ -3640,7 +3640,7 @@ static void ironlake_crtc_enable(struct drm_crtc *crtc)
intel_enable_pipe(intel_crtc);
intel_enable_primary_plane(dev_priv, plane, pipe);
intel_enable_planes(crtc);
- intel_crtc_update_cursor(crtc, true);
+ intel_crtc_update_cursor(crtc, true, false);
if (intel_crtc->config.has_pch_encoder)
ironlake_pch_enable(crtc);
@@ -3682,7 +3682,7 @@ static void haswell_crtc_enable_planes(struct drm_crtc *crtc)
intel_enable_primary_plane(dev_priv, plane, pipe);
intel_enable_planes(crtc);
- intel_crtc_update_cursor(crtc, true);
+ intel_crtc_update_cursor(crtc, true, false);
hsw_enable_ips(intel_crtc);
@@ -3708,7 +3708,7 @@ static void haswell_crtc_disable_planes(struct drm_crtc *crtc)
hsw_disable_ips(intel_crtc);
- intel_crtc_update_cursor(crtc, false);
+ intel_crtc_update_cursor(crtc, false, false);
intel_disable_planes(crtc);
intel_disable_primary_plane(dev_priv, plane, pipe);
}
@@ -3836,7 +3836,7 @@ static void ironlake_crtc_disable(struct drm_crtc *crtc)
if (dev_priv->fbc.plane == plane)
intel_disable_fbc(dev);
- intel_crtc_update_cursor(crtc, false);
+ intel_crtc_update_cursor(crtc, false, false);
intel_disable_planes(crtc);
intel_disable_primary_plane(dev_priv, plane, pipe);
@@ -4211,7 +4211,7 @@ static void valleyview_crtc_enable(struct drm_crtc *crtc)
intel_set_cpu_fifo_underrun_reporting(dev, pipe, true);
intel_enable_primary_plane(dev_priv, plane, pipe);
intel_enable_planes(crtc);
- intel_crtc_update_cursor(crtc, true);
+ intel_crtc_update_cursor(crtc, true, false);
intel_update_fbc(dev);
@@ -4253,7 +4253,7 @@ static void i9xx_crtc_enable(struct drm_crtc *crtc)
/* The fixup needs to happen before cursor is enabled */
if (IS_G4X(dev))
g4x_fixup_plane(dev_priv, pipe);
- intel_crtc_update_cursor(crtc, true);
+ intel_crtc_update_cursor(crtc, true, false);
/* Give the overlay scaler a chance to enable if it's on this pipe */
intel_crtc_dpms_overlay(intel_crtc, true);
@@ -4302,7 +4302,7 @@ static void i9xx_crtc_disable(struct drm_crtc *crtc)
intel_disable_fbc(dev);
intel_crtc_dpms_overlay(intel_crtc, false);
- intel_crtc_update_cursor(crtc, false);
+ intel_crtc_update_cursor(crtc, false, false);
intel_disable_planes(crtc);
intel_disable_primary_plane(dev_priv, plane, pipe);
@@ -7429,7 +7429,7 @@ void intel_write_eld(struct drm_encoder *encoder,
dev_priv->display.write_eld(connector, crtc, mode);
}
-static void i845_update_cursor(struct drm_crtc *crtc, u32 base)
+static void i845_update_cursor(struct drm_crtc *crtc, u32 base, bool force)
{
struct drm_device *dev = crtc->dev;
struct drm_i915_private *dev_priv = dev->dev_private;
@@ -7437,7 +7437,7 @@ static void i845_update_cursor(struct drm_crtc *crtc, u32 base)
bool visible = base != 0;
u32 cntl;
- if (intel_crtc->cursor_visible == visible)
+ if (!force && intel_crtc->cursor_visible == visible)
return;
cntl = I915_READ(_CURACNTR);
@@ -7459,7 +7459,7 @@ static void i845_update_cursor(struct drm_crtc *crtc, u32 base)
intel_crtc->cursor_visible = visible;
}
-static void i9xx_update_cursor(struct drm_crtc *crtc, u32 base)
+static void i9xx_update_cursor(struct drm_crtc *crtc, u32 base, bool force)
{
struct drm_device *dev = crtc->dev;
struct drm_i915_private *dev_priv = dev->dev_private;
@@ -7467,7 +7467,7 @@ static void i9xx_update_cursor(struct drm_crtc *crtc, u32 base)
int pipe = intel_crtc->pipe;
bool visible = base != 0;
- if (intel_crtc->cursor_visible != visible) {
+ if (force || intel_crtc->cursor_visible != visible) {
uint32_t cntl = I915_READ(CURCNTR(pipe));
if (base) {
cntl &= ~(CURSOR_MODE | MCURSOR_PIPE_SELECT);
@@ -7477,6 +7477,10 @@ static void i9xx_update_cursor(struct drm_crtc *crtc, u32 base)
cntl &= ~(CURSOR_MODE | MCURSOR_GAMMA_ENABLE);
cntl |= CURSOR_MODE_DISABLE;
}
+ if (intel_crtc->cursor_rotation == BIT(DRM_ROTATE_180))
+ cntl |= CURSOR_ROTATE_180;
+ else
+ cntl &= ~CURSOR_ROTATE_180;
I915_WRITE(CURCNTR(pipe), cntl);
intel_crtc->cursor_visible = visible;
@@ -7487,7 +7491,7 @@ static void i9xx_update_cursor(struct drm_crtc *crtc, u32 base)
POSTING_READ(CURBASE(pipe));
}
-static void ivb_update_cursor(struct drm_crtc *crtc, u32 base)
+static void ivb_update_cursor(struct drm_crtc *crtc, u32 base, bool force)
{
struct drm_device *dev = crtc->dev;
struct drm_i915_private *dev_priv = dev->dev_private;
@@ -7495,7 +7499,7 @@ static void ivb_update_cursor(struct drm_crtc *crtc, u32 base)
int pipe = intel_crtc->pipe;
bool visible = base != 0;
- if (intel_crtc->cursor_visible != visible) {
+ if (force || intel_crtc->cursor_visible != visible) {
uint32_t cntl = I915_READ(CURCNTR_IVB(pipe));
if (base) {
cntl &= ~CURSOR_MODE;
@@ -7508,6 +7512,10 @@ static void ivb_update_cursor(struct drm_crtc *crtc, u32 base)
cntl |= CURSOR_PIPE_CSC_ENABLE;
cntl &= ~CURSOR_TRICKLE_FEED_DISABLE;
}
+ if (intel_crtc->cursor_rotation == BIT(DRM_ROTATE_180))
+ cntl |= CURSOR_ROTATE_180;
+ else
+ cntl &= ~CURSOR_ROTATE_180;
I915_WRITE(CURCNTR_IVB(pipe), cntl);
intel_crtc->cursor_visible = visible;
@@ -7520,7 +7528,7 @@ static void ivb_update_cursor(struct drm_crtc *crtc, u32 base)
/* If no-part of the cursor is visible on the framebuffer, then the GPU may hang... */
static void intel_crtc_update_cursor(struct drm_crtc *crtc,
- bool on)
+ bool on, bool force)
{
struct drm_device *dev = crtc->dev;
struct drm_i915_private *dev_priv = dev->dev_private;
@@ -7564,13 +7572,13 @@ static void intel_crtc_update_cursor(struct drm_crtc *crtc,
if (IS_IVYBRIDGE(dev) || IS_HASWELL(dev) || IS_BROADWELL(dev)) {
I915_WRITE(CURPOS_IVB(pipe), pos);
- ivb_update_cursor(crtc, base);
+ ivb_update_cursor(crtc, base, force);
} else {
I915_WRITE(CURPOS(pipe), pos);
if (IS_845G(dev) || IS_I865G(dev))
- i845_update_cursor(crtc, base);
+ i845_update_cursor(crtc, base, force);
else
- i9xx_update_cursor(crtc, base);
+ i9xx_update_cursor(crtc, base, force);
}
}
@@ -7677,7 +7685,7 @@ static int intel_crtc_cursor_set(struct drm_crtc *crtc,
intel_crtc->cursor_height = height;
if (intel_crtc->active)
- intel_crtc_update_cursor(crtc, intel_crtc->cursor_bo != NULL);
+ intel_crtc_update_cursor(crtc, intel_crtc->cursor_bo != NULL, false);
return 0;
fail_unpin:
@@ -7697,7 +7705,7 @@ static int intel_crtc_cursor_move(struct drm_crtc *crtc, int x, int y)
intel_crtc->cursor_y = clamp_t(int, y, SHRT_MIN, SHRT_MAX);
if (intel_crtc->active)
- intel_crtc_update_cursor(crtc, intel_crtc->cursor_bo != NULL);
+ intel_crtc_update_cursor(crtc, intel_crtc->cursor_bo != NULL, false);
return 0;
}
@@ -10388,6 +10396,7 @@ static void intel_crtc_init(struct drm_device *dev, int pipe)
intel_crtc->pipe = pipe;
intel_crtc->plane = pipe;
intel_crtc->primary_rotation = BIT(DRM_ROTATE_0);
+ intel_crtc->cursor_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;
@@ -10408,6 +10417,16 @@ static void intel_crtc_init(struct drm_device *dev, int pipe)
drm_object_attach_property(&intel_crtc->base.base,
dev_priv->plane_rotation_property,
intel_crtc->primary_rotation);
+
+ if (!dev_priv->cursor_rotation_property)
+ dev_priv->cursor_rotation_property =
+ drm_mode_create_rotation_property(dev, "cursor-rotation",
+ BIT(DRM_ROTATE_0) |
+ BIT(DRM_ROTATE_180));
+ if (dev_priv->cursor_rotation_property)
+ drm_object_attach_property(&intel_crtc->base.base,
+ dev_priv->cursor_rotation_property,
+ intel_crtc->cursor_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 8c17a82..4a7f4f1 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -332,6 +332,7 @@ struct intel_crtc {
enum pipe pipe;
enum plane plane;
unsigned int primary_rotation; /* primary plane in relation to the pipe */
+ unsigned int cursor_rotation; /* cursor plane in relation to the pipe */
u8 lut_r[256], lut_g[256], lut_b[256];
/*
--
1.8.3.2
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [PATCH 4/5] drm/i915: Add rotation support for the cursor plane
2014-02-12 21:15 ` [PATCH 4/5] drm/i915: Add rotation support for the cursor plane ville.syrjala
@ 2014-02-14 11:01 ` Sagar Arun Kamble
2014-02-14 11:39 ` Ville Syrjälä
0 siblings, 1 reply; 24+ messages in thread
From: Sagar Arun Kamble @ 2014-02-14 11:01 UTC (permalink / raw)
To: ville.syrjala; +Cc: intel-gfx, dri-devel
On Wed, 2014-02-12 at 23:15 +0200, ville.syrjala@linux.intel.com wrote:
> 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.
By flipping you meant setting 180 degree rotation?
Don't we have to adjust the cursor base as well to the lower right
corner apart from setting the control bit?
>
> 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 | 9 ++++++
> drivers/gpu/drm/i915/i915_drv.h | 1 +
> drivers/gpu/drm/i915/i915_reg.h | 1 +
> drivers/gpu/drm/i915/intel_display.c | 59 ++++++++++++++++++++++++------------
> drivers/gpu/drm/i915/intel_drv.h | 1 +
> 5 files changed, 51 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> index 4a3ef34..3dd9abb 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -1916,6 +1916,15 @@ void i915_driver_lastclose(struct drm_device * dev)
> }
> }
>
> + if (dev_priv->cursor_rotation_property) {
> + list_for_each_entry(crtc, &dev->mode_config.crtc_list, base.head) {
> + crtc->cursor_rotation = BIT(DRM_ROTATE_0);
> + drm_object_property_set_value(&crtc->base.base,
> + dev_priv->cursor_rotation_property,
> + crtc->cursor_rotation);
> + }
> + }
> +
> 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_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 6af78ee..8ecd3bb 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1562,6 +1562,7 @@ typedef struct drm_i915_private {
> struct drm_property *force_audio_property;
> struct drm_property *rotation_property; /* "rotation" */
> struct drm_property *plane_rotation_property; /* "plane-rotation" */
> + struct drm_property *cursor_rotation_property; /* "cursor-rotation" */
>
> uint32_t hw_context_size;
> struct list_head context_list;
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index ab1aeb82..fee068a 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -3534,6 +3534,7 @@
> #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 (dev_priv->info.display_mmio_offset + 0x70084)
> #define _CURAPOS (dev_priv->info.display_mmio_offset + 0x70088)
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 37b23d1..e94167b 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -42,7 +42,7 @@
> #include <linux/dma_remapping.h>
>
> static void intel_increase_pllclock(struct drm_crtc *crtc);
> -static void intel_crtc_update_cursor(struct drm_crtc *crtc, bool on);
> +static void intel_crtc_update_cursor(struct drm_crtc *crtc, bool on, bool force);
>
> static void i9xx_crtc_clock_get(struct intel_crtc *crtc,
> struct intel_crtc_config *pipe_config);
> @@ -3640,7 +3640,7 @@ static void ironlake_crtc_enable(struct drm_crtc *crtc)
> intel_enable_pipe(intel_crtc);
> intel_enable_primary_plane(dev_priv, plane, pipe);
> intel_enable_planes(crtc);
> - intel_crtc_update_cursor(crtc, true);
> + intel_crtc_update_cursor(crtc, true, false);
>
> if (intel_crtc->config.has_pch_encoder)
> ironlake_pch_enable(crtc);
> @@ -3682,7 +3682,7 @@ static void haswell_crtc_enable_planes(struct drm_crtc *crtc)
>
> intel_enable_primary_plane(dev_priv, plane, pipe);
> intel_enable_planes(crtc);
> - intel_crtc_update_cursor(crtc, true);
> + intel_crtc_update_cursor(crtc, true, false);
>
> hsw_enable_ips(intel_crtc);
>
> @@ -3708,7 +3708,7 @@ static void haswell_crtc_disable_planes(struct drm_crtc *crtc)
>
> hsw_disable_ips(intel_crtc);
>
> - intel_crtc_update_cursor(crtc, false);
> + intel_crtc_update_cursor(crtc, false, false);
> intel_disable_planes(crtc);
> intel_disable_primary_plane(dev_priv, plane, pipe);
> }
> @@ -3836,7 +3836,7 @@ static void ironlake_crtc_disable(struct drm_crtc *crtc)
> if (dev_priv->fbc.plane == plane)
> intel_disable_fbc(dev);
>
> - intel_crtc_update_cursor(crtc, false);
> + intel_crtc_update_cursor(crtc, false, false);
> intel_disable_planes(crtc);
> intel_disable_primary_plane(dev_priv, plane, pipe);
>
> @@ -4211,7 +4211,7 @@ static void valleyview_crtc_enable(struct drm_crtc *crtc)
> intel_set_cpu_fifo_underrun_reporting(dev, pipe, true);
> intel_enable_primary_plane(dev_priv, plane, pipe);
> intel_enable_planes(crtc);
> - intel_crtc_update_cursor(crtc, true);
> + intel_crtc_update_cursor(crtc, true, false);
>
> intel_update_fbc(dev);
>
> @@ -4253,7 +4253,7 @@ static void i9xx_crtc_enable(struct drm_crtc *crtc)
> /* The fixup needs to happen before cursor is enabled */
> if (IS_G4X(dev))
> g4x_fixup_plane(dev_priv, pipe);
> - intel_crtc_update_cursor(crtc, true);
> + intel_crtc_update_cursor(crtc, true, false);
>
> /* Give the overlay scaler a chance to enable if it's on this pipe */
> intel_crtc_dpms_overlay(intel_crtc, true);
> @@ -4302,7 +4302,7 @@ static void i9xx_crtc_disable(struct drm_crtc *crtc)
> intel_disable_fbc(dev);
>
> intel_crtc_dpms_overlay(intel_crtc, false);
> - intel_crtc_update_cursor(crtc, false);
> + intel_crtc_update_cursor(crtc, false, false);
> intel_disable_planes(crtc);
> intel_disable_primary_plane(dev_priv, plane, pipe);
>
> @@ -7429,7 +7429,7 @@ void intel_write_eld(struct drm_encoder *encoder,
> dev_priv->display.write_eld(connector, crtc, mode);
> }
>
> -static void i845_update_cursor(struct drm_crtc *crtc, u32 base)
> +static void i845_update_cursor(struct drm_crtc *crtc, u32 base, bool force)
> {
> struct drm_device *dev = crtc->dev;
> struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -7437,7 +7437,7 @@ static void i845_update_cursor(struct drm_crtc *crtc, u32 base)
> bool visible = base != 0;
> u32 cntl;
>
> - if (intel_crtc->cursor_visible == visible)
> + if (!force && intel_crtc->cursor_visible == visible)
> return;
>
> cntl = I915_READ(_CURACNTR);
> @@ -7459,7 +7459,7 @@ static void i845_update_cursor(struct drm_crtc *crtc, u32 base)
> intel_crtc->cursor_visible = visible;
> }
>
> -static void i9xx_update_cursor(struct drm_crtc *crtc, u32 base)
> +static void i9xx_update_cursor(struct drm_crtc *crtc, u32 base, bool force)
> {
> struct drm_device *dev = crtc->dev;
> struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -7467,7 +7467,7 @@ static void i9xx_update_cursor(struct drm_crtc *crtc, u32 base)
> int pipe = intel_crtc->pipe;
> bool visible = base != 0;
>
> - if (intel_crtc->cursor_visible != visible) {
> + if (force || intel_crtc->cursor_visible != visible) {
> uint32_t cntl = I915_READ(CURCNTR(pipe));
> if (base) {
> cntl &= ~(CURSOR_MODE | MCURSOR_PIPE_SELECT);
> @@ -7477,6 +7477,10 @@ static void i9xx_update_cursor(struct drm_crtc *crtc, u32 base)
> cntl &= ~(CURSOR_MODE | MCURSOR_GAMMA_ENABLE);
> cntl |= CURSOR_MODE_DISABLE;
> }
> + if (intel_crtc->cursor_rotation == BIT(DRM_ROTATE_180))
> + cntl |= CURSOR_ROTATE_180;
> + else
> + cntl &= ~CURSOR_ROTATE_180;
> I915_WRITE(CURCNTR(pipe), cntl);
>
> intel_crtc->cursor_visible = visible;
> @@ -7487,7 +7491,7 @@ static void i9xx_update_cursor(struct drm_crtc *crtc, u32 base)
> POSTING_READ(CURBASE(pipe));
> }
>
> -static void ivb_update_cursor(struct drm_crtc *crtc, u32 base)
> +static void ivb_update_cursor(struct drm_crtc *crtc, u32 base, bool force)
> {
> struct drm_device *dev = crtc->dev;
> struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -7495,7 +7499,7 @@ static void ivb_update_cursor(struct drm_crtc *crtc, u32 base)
> int pipe = intel_crtc->pipe;
> bool visible = base != 0;
>
> - if (intel_crtc->cursor_visible != visible) {
> + if (force || intel_crtc->cursor_visible != visible) {
> uint32_t cntl = I915_READ(CURCNTR_IVB(pipe));
> if (base) {
> cntl &= ~CURSOR_MODE;
> @@ -7508,6 +7512,10 @@ static void ivb_update_cursor(struct drm_crtc *crtc, u32 base)
> cntl |= CURSOR_PIPE_CSC_ENABLE;
> cntl &= ~CURSOR_TRICKLE_FEED_DISABLE;
> }
> + if (intel_crtc->cursor_rotation == BIT(DRM_ROTATE_180))
> + cntl |= CURSOR_ROTATE_180;
> + else
> + cntl &= ~CURSOR_ROTATE_180;
> I915_WRITE(CURCNTR_IVB(pipe), cntl);
>
> intel_crtc->cursor_visible = visible;
> @@ -7520,7 +7528,7 @@ static void ivb_update_cursor(struct drm_crtc *crtc, u32 base)
>
> /* If no-part of the cursor is visible on the framebuffer, then the GPU may hang... */
> static void intel_crtc_update_cursor(struct drm_crtc *crtc,
> - bool on)
> + bool on, bool force)
> {
> struct drm_device *dev = crtc->dev;
> struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -7564,13 +7572,13 @@ static void intel_crtc_update_cursor(struct drm_crtc *crtc,
>
> if (IS_IVYBRIDGE(dev) || IS_HASWELL(dev) || IS_BROADWELL(dev)) {
> I915_WRITE(CURPOS_IVB(pipe), pos);
> - ivb_update_cursor(crtc, base);
> + ivb_update_cursor(crtc, base, force);
> } else {
> I915_WRITE(CURPOS(pipe), pos);
> if (IS_845G(dev) || IS_I865G(dev))
> - i845_update_cursor(crtc, base);
> + i845_update_cursor(crtc, base, force);
> else
> - i9xx_update_cursor(crtc, base);
> + i9xx_update_cursor(crtc, base, force);
> }
> }
>
> @@ -7677,7 +7685,7 @@ static int intel_crtc_cursor_set(struct drm_crtc *crtc,
> intel_crtc->cursor_height = height;
>
> if (intel_crtc->active)
> - intel_crtc_update_cursor(crtc, intel_crtc->cursor_bo != NULL);
> + intel_crtc_update_cursor(crtc, intel_crtc->cursor_bo != NULL, false);
>
> return 0;
> fail_unpin:
> @@ -7697,7 +7705,7 @@ static int intel_crtc_cursor_move(struct drm_crtc *crtc, int x, int y)
> intel_crtc->cursor_y = clamp_t(int, y, SHRT_MIN, SHRT_MAX);
>
> if (intel_crtc->active)
> - intel_crtc_update_cursor(crtc, intel_crtc->cursor_bo != NULL);
> + intel_crtc_update_cursor(crtc, intel_crtc->cursor_bo != NULL, false);
>
> return 0;
> }
> @@ -10388,6 +10396,7 @@ static void intel_crtc_init(struct drm_device *dev, int pipe)
> intel_crtc->pipe = pipe;
> intel_crtc->plane = pipe;
> intel_crtc->primary_rotation = BIT(DRM_ROTATE_0);
> + intel_crtc->cursor_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;
> @@ -10408,6 +10417,16 @@ static void intel_crtc_init(struct drm_device *dev, int pipe)
> drm_object_attach_property(&intel_crtc->base.base,
> dev_priv->plane_rotation_property,
> intel_crtc->primary_rotation);
> +
> + if (!dev_priv->cursor_rotation_property)
> + dev_priv->cursor_rotation_property =
> + drm_mode_create_rotation_property(dev, "cursor-rotation",
> + BIT(DRM_ROTATE_0) |
> + BIT(DRM_ROTATE_180));
> + if (dev_priv->cursor_rotation_property)
> + drm_object_attach_property(&intel_crtc->base.base,
> + dev_priv->cursor_rotation_property,
> + intel_crtc->cursor_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 8c17a82..4a7f4f1 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -332,6 +332,7 @@ struct intel_crtc {
> enum pipe pipe;
> enum plane plane;
> unsigned int primary_rotation; /* primary plane in relation to the pipe */
> + unsigned int cursor_rotation; /* cursor plane in relation to the pipe */
>
> u8 lut_r[256], lut_g[256], lut_b[256];
> /*
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH 4/5] drm/i915: Add rotation support for the cursor plane
2014-02-14 11:01 ` Sagar Arun Kamble
@ 2014-02-14 11:39 ` Ville Syrjälä
2014-02-17 17:23 ` Sagar Arun Kamble
0 siblings, 1 reply; 24+ messages in thread
From: Ville Syrjälä @ 2014-02-14 11:39 UTC (permalink / raw)
To: Sagar Arun Kamble; +Cc: intel-gfx, dri-devel
On Fri, Feb 14, 2014 at 04:31:17PM +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>
> >
> > 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.
> By flipping you meant setting 180 degree rotation?
Yes.
> Don't we have to adjust the cursor base as well to the lower right
> corner apart from setting the control bit?
No, the hardware does that automagically. Hmm. Except on gen4
apparently. Looks like I need to test on gen4, and fix it if it's
really the case.
--
Ville Syrjälä
Intel OTC
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 4/5] drm/i915: Add rotation support for the cursor plane
2014-02-14 11:39 ` Ville Syrjälä
@ 2014-02-17 17:23 ` Sagar Arun Kamble
2014-02-17 17:51 ` Ville Syrjälä
0 siblings, 1 reply; 24+ messages in thread
From: Sagar Arun Kamble @ 2014-02-17 17:23 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: intel-gfx, dri-devel
On Fri, 2014-02-14 at 13:39 +0200, Ville Syrjälä wrote:
> On Fri, Feb 14, 2014 at 04:31:17PM +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>
> > >
> > > 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.
> > By flipping you meant setting 180 degree rotation?
>
> Yes.
>
> > Don't we have to adjust the cursor base as well to the lower right
> > corner apart from setting the control bit?
>
> No, the hardware does that automagically. Hmm. Except on gen4
> apparently. Looks like I need to test on gen4, and fix it if it's
> really the case.
I tried on BYT system and 180 rotation on cursor plane is showing
garbage data in cursor plane. We might need to adjust the cursor base.
Another thing, pipe rotation somehow did not work for me when I do this:
echo 0x4 > /sys/kernel/debug/dri/0/i915_pipe_rotation
Only cursor plane had impact. Need to debug this as well.
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 4/5] drm/i915: Add rotation support for the cursor plane
2014-02-17 17:23 ` Sagar Arun Kamble
@ 2014-02-17 17:51 ` Ville Syrjälä
2014-02-18 7:49 ` Sagar Arun Kamble
0 siblings, 1 reply; 24+ messages in thread
From: Ville Syrjälä @ 2014-02-17 17:51 UTC (permalink / raw)
To: Sagar Arun Kamble; +Cc: intel-gfx, dri-devel
On Mon, Feb 17, 2014 at 10:53:50PM +0530, Sagar Arun Kamble wrote:
> On Fri, 2014-02-14 at 13:39 +0200, Ville Syrjälä wrote:
> > On Fri, Feb 14, 2014 at 04:31:17PM +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>
> > > >
> > > > 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.
> > > By flipping you meant setting 180 degree rotation?
> >
> > Yes.
> >
> > > Don't we have to adjust the cursor base as well to the lower right
> > > corner apart from setting the control bit?
> >
> > No, the hardware does that automagically. Hmm. Except on gen4
> > apparently. Looks like I need to test on gen4, and fix it if it's
> > really the case.
> I tried on BYT system and 180 rotation on cursor plane is showing
> garbage data in cursor plane. We might need to adjust the cursor base.
Yeah it's the same on gen4. I already have a fixed patch, but didn't
repost it yet.
> Another thing, pipe rotation somehow did not work for me when I do this:
> echo 0x4 > /sys/kernel/debug/dri/0/i915_pipe_rotation
> Only cursor plane had impact. Need to debug this as well.
That's expected. It doesn't actually call the set_property codepath,
instead it just sets the value directly and excpects a subsequent
modeset to do the actual work. It was anyway just a hack to try things
out a bit, so I didn't implement it properly. But it should be trivial
to make it work correctly, so I might as well do it...
--
Ville Syrjälä
Intel OTC
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 4/5] drm/i915: Add rotation support for the cursor plane
2014-02-17 17:51 ` Ville Syrjälä
@ 2014-02-18 7:49 ` Sagar Arun Kamble
2014-02-18 9:23 ` Ville Syrjälä
0 siblings, 1 reply; 24+ messages in thread
From: Sagar Arun Kamble @ 2014-02-18 7:49 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: intel-gfx, dri-devel
On Mon, 2014-02-17 at 19:51 +0200, Ville Syrjälä wrote:
> On Mon, Feb 17, 2014 at 10:53:50PM +0530, Sagar Arun Kamble wrote:
> > On Fri, 2014-02-14 at 13:39 +0200, Ville Syrjälä wrote:
> > > On Fri, Feb 14, 2014 at 04:31:17PM +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>
> > > > >
> > > > > 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.
> > > > By flipping you meant setting 180 degree rotation?
> > >
> > > Yes.
> > >
> > > > Don't we have to adjust the cursor base as well to the lower right
> > > > corner apart from setting the control bit?
> > >
> > > No, the hardware does that automagically. Hmm. Except on gen4
> > > apparently. Looks like I need to test on gen4, and fix it if it's
> > > really the case.
> > I tried on BYT system and 180 rotation on cursor plane is showing
> > garbage data in cursor plane. We might need to adjust the cursor base.
>
> Yeah it's the same on gen4. I already have a fixed patch, but didn't
> repost it yet.
>
> > Another thing, pipe rotation somehow did not work for me when I do this:
> > echo 0x4 > /sys/kernel/debug/dri/0/i915_pipe_rotation
> > Only cursor plane had impact. Need to debug this as well.
>
> That's expected. It doesn't actually call the set_property codepath,
> instead it just sets the value directly and excpects a subsequent
> modeset to do the actual work. It was anyway just a hack to try things
> out a bit, so I didn't implement it properly. But it should be trivial
> to make it work correctly, so I might as well do it...
Yeah. Tried doing modeset and it works perfectly.
For Cursor rotation we might need to add check for 32bpp cursors as
well.
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 4/5] drm/i915: Add rotation support for the cursor plane
2014-02-18 7:49 ` Sagar Arun Kamble
@ 2014-02-18 9:23 ` Ville Syrjälä
2014-02-18 10:09 ` Sagar Arun Kamble
0 siblings, 1 reply; 24+ messages in thread
From: Ville Syrjälä @ 2014-02-18 9:23 UTC (permalink / raw)
To: Sagar Arun Kamble; +Cc: intel-gfx, dri-devel
On Tue, Feb 18, 2014 at 01:19:05PM +0530, Sagar Arun Kamble wrote:
> On Mon, 2014-02-17 at 19:51 +0200, Ville Syrjälä wrote:
> > On Mon, Feb 17, 2014 at 10:53:50PM +0530, Sagar Arun Kamble wrote:
> > > On Fri, 2014-02-14 at 13:39 +0200, Ville Syrjälä wrote:
> > > > On Fri, Feb 14, 2014 at 04:31:17PM +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>
> > > > > >
> > > > > > 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.
> > > > > By flipping you meant setting 180 degree rotation?
> > > >
> > > > Yes.
> > > >
> > > > > Don't we have to adjust the cursor base as well to the lower right
> > > > > corner apart from setting the control bit?
> > > >
> > > > No, the hardware does that automagically. Hmm. Except on gen4
> > > > apparently. Looks like I need to test on gen4, and fix it if it's
> > > > really the case.
> > > I tried on BYT system and 180 rotation on cursor plane is showing
> > > garbage data in cursor plane. We might need to adjust the cursor base.
> >
> > Yeah it's the same on gen4. I already have a fixed patch, but didn't
> > repost it yet.
> >
> > > Another thing, pipe rotation somehow did not work for me when I do this:
> > > echo 0x4 > /sys/kernel/debug/dri/0/i915_pipe_rotation
> > > Only cursor plane had impact. Need to debug this as well.
> >
> > That's expected. It doesn't actually call the set_property codepath,
> > instead it just sets the value directly and excpects a subsequent
> > modeset to do the actual work. It was anyway just a hack to try things
> > out a bit, so I didn't implement it properly. But it should be trivial
> > to make it work correctly, so I might as well do it...
> Yeah. Tried doing modeset and it works perfectly.
> For Cursor rotation we might need to add check for 32bpp cursors as
> well.
We don't support anything else at the moment. And I don't think there's
much point in adding support for any legacy cursor formats. The one
thing we want to do is add support for larger cursor sizes. But I think
that can wait until we expose the cursor as a drm_plane.
--
Ville Syrjälä
Intel OTC
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 4/5] drm/i915: Add rotation support for the cursor plane
2014-02-18 9:23 ` Ville Syrjälä
@ 2014-02-18 10:09 ` Sagar Arun Kamble
0 siblings, 0 replies; 24+ messages in thread
From: Sagar Arun Kamble @ 2014-02-18 10:09 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: intel-gfx, dri-devel
On Tue, 2014-02-18 at 11:23 +0200, Ville Syrjälä wrote:
> On Tue, Feb 18, 2014 at 01:19:05PM +0530, Sagar Arun Kamble wrote:
> > On Mon, 2014-02-17 at 19:51 +0200, Ville Syrjälä wrote:
> > > On Mon, Feb 17, 2014 at 10:53:50PM +0530, Sagar Arun Kamble wrote:
> > > > On Fri, 2014-02-14 at 13:39 +0200, Ville Syrjälä wrote:
> > > > > On Fri, Feb 14, 2014 at 04:31:17PM +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>
> > > > > > >
> > > > > > > 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.
> > > > > > By flipping you meant setting 180 degree rotation?
> > > > >
> > > > > Yes.
> > > > >
> > > > > > Don't we have to adjust the cursor base as well to the lower right
> > > > > > corner apart from setting the control bit?
> > > > >
> > > > > No, the hardware does that automagically. Hmm. Except on gen4
> > > > > apparently. Looks like I need to test on gen4, and fix it if it's
> > > > > really the case.
> > > > I tried on BYT system and 180 rotation on cursor plane is showing
> > > > garbage data in cursor plane. We might need to adjust the cursor base.
> > >
> > > Yeah it's the same on gen4. I already have a fixed patch, but didn't
> > > repost it yet.
> > >
> > > > Another thing, pipe rotation somehow did not work for me when I do this:
> > > > echo 0x4 > /sys/kernel/debug/dri/0/i915_pipe_rotation
> > > > Only cursor plane had impact. Need to debug this as well.
> > >
> > > That's expected. It doesn't actually call the set_property codepath,
> > > instead it just sets the value directly and excpects a subsequent
> > > modeset to do the actual work. It was anyway just a hack to try things
> > > out a bit, so I didn't implement it properly. But it should be trivial
> > > to make it work correctly, so I might as well do it...
> > Yeah. Tried doing modeset and it works perfectly.
> > For Cursor rotation we might need to add check for 32bpp cursors as
> > well.
>
> We don't support anything else at the moment. And I don't think there's
> much point in adding support for any legacy cursor formats. The one
> thing we want to do is add support for larger cursor sizes. But I think
> that can wait until we expose the cursor as a drm_plane.
Actually I am working on enabling larger cursor sizes now. Will share
patch now.
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 5/5] drm/i915: Add full pipe rotation
2014-02-12 21:14 [PATCH 0/5] drm/i915: Full pipe rotation & rotation property name bikeshedding ville.syrjala
` (3 preceding siblings ...)
2014-02-12 21:15 ` [PATCH 4/5] drm/i915: Add rotation support for the cursor plane ville.syrjala
@ 2014-02-12 21:15 ` ville.syrjala
2014-02-19 10:25 ` Sagar Arun Kamble
2014-02-12 23:17 ` [PATCH 0/5] drm/i915: Full pipe rotation & rotation property name bikeshedding Chris Wilson
2014-02-13 11:20 ` Chris Wilson
6 siblings, 1 reply; 24+ messages in thread
From: ville.syrjala @ 2014-02-12 21:15 UTC (permalink / raw)
To: intel-gfx; +Cc: Sagar Kamble, dri-devel
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
We can pretend that we can rotate the entire pipe by rotating all the
planes and adjusting their positions appropriately. Add a "rotation"
property on the crtc which will do this.
The main upshot of doing the full pipe rotation instead of rotating all
the planes individually is that the plane positions turn out correct
automagically. So userspace doesn't need to do anything except toggle
the property and continue as if nothing had changed.
The actual implementation is pretty much trivial thanks to drm_rect
and drm_rotation_chain() ;)
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 | 6 ++
drivers/gpu/drm/i915/intel_display.c | 154 +++++++++++++++++++++++++++++++----
drivers/gpu/drm/i915/intel_drv.h | 1 +
drivers/gpu/drm/i915/intel_pm.c | 6 +-
drivers/gpu/drm/i915/intel_sprite.c | 21 +++--
5 files changed, 164 insertions(+), 24 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 3dd9abb..b59bff1 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1914,6 +1914,12 @@ void i915_driver_lastclose(struct drm_device * dev)
dev_priv->rotation_property,
plane->rotation);
}
+ list_for_each_entry(crtc, &dev->mode_config.crtc_list, base.head) {
+ crtc->pipe_rotation = BIT(DRM_ROTATE_0);
+ drm_object_property_set_value(&crtc->base.base,
+ dev_priv->rotation_property,
+ crtc->pipe_rotation);
+ }
}
if (dev_priv->cursor_rotation_property) {
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index e94167b..1b74d24 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -39,6 +39,7 @@
#include "i915_trace.h"
#include <drm/drm_dp_helper.h>
#include <drm/drm_crtc_helper.h>
+#include <drm/drm_rect.h>
#include <linux/dma_remapping.h>
static void intel_increase_pllclock(struct drm_crtc *crtc);
@@ -2060,6 +2061,8 @@ static int i9xx_update_plane(struct drm_crtc *crtc, struct drm_framebuffer *fb,
u32 dspcntr;
u32 reg;
int pixel_size;
+ unsigned int rotation = drm_rotation_chain(intel_crtc->pipe_rotation,
+ intel_crtc->primary_rotation);
switch (plane) {
case 0:
@@ -2133,7 +2136,7 @@ static int i9xx_update_plane(struct drm_crtc *crtc, struct drm_framebuffer *fb,
intel_crtc->dspaddr_offset = linear_offset;
}
- if (intel_crtc->primary_rotation == BIT(DRM_ROTATE_180)) {
+ if (rotation == BIT(DRM_ROTATE_180)) {
dspcntr |= DISPPLANE_ROTATE_180;
x += (intel_crtc->config.pipe_src_w - 1);
@@ -2173,6 +2176,8 @@ static int ironlake_update_plane(struct drm_crtc *crtc,
u32 dspcntr;
u32 reg;
int pixel_size;
+ unsigned int rotation = drm_rotation_chain(intel_crtc->pipe_rotation,
+ intel_crtc->primary_rotation);
switch (plane) {
case 0:
@@ -2238,7 +2243,7 @@ static int ironlake_update_plane(struct drm_crtc *crtc,
fb->pitches[0]);
linear_offset -= intel_crtc->dspaddr_offset;
- if (intel_crtc->primary_rotation == BIT(DRM_ROTATE_180)) {
+ if (rotation == BIT(DRM_ROTATE_180)) {
dspcntr |= DISPPLANE_ROTATE_180;
if (!IS_HASWELL(dev) && !IS_BROADWELL(dev)) {
@@ -7468,6 +7473,8 @@ static void i9xx_update_cursor(struct drm_crtc *crtc, u32 base, bool force)
bool visible = base != 0;
if (force || intel_crtc->cursor_visible != visible) {
+ unsigned int rotation = drm_rotation_chain(intel_crtc->pipe_rotation,
+ intel_crtc->cursor_rotation);
uint32_t cntl = I915_READ(CURCNTR(pipe));
if (base) {
cntl &= ~(CURSOR_MODE | MCURSOR_PIPE_SELECT);
@@ -7477,7 +7484,7 @@ static void i9xx_update_cursor(struct drm_crtc *crtc, u32 base, bool force)
cntl &= ~(CURSOR_MODE | MCURSOR_GAMMA_ENABLE);
cntl |= CURSOR_MODE_DISABLE;
}
- if (intel_crtc->cursor_rotation == BIT(DRM_ROTATE_180))
+ if (rotation == BIT(DRM_ROTATE_180))
cntl |= CURSOR_ROTATE_180;
else
cntl &= ~CURSOR_ROTATE_180;
@@ -7500,6 +7507,8 @@ static void ivb_update_cursor(struct drm_crtc *crtc, u32 base, bool force)
bool visible = base != 0;
if (force || intel_crtc->cursor_visible != visible) {
+ unsigned int rotation = drm_rotation_chain(intel_crtc->pipe_rotation,
+ intel_crtc->cursor_rotation);
uint32_t cntl = I915_READ(CURCNTR_IVB(pipe));
if (base) {
cntl &= ~CURSOR_MODE;
@@ -7512,7 +7521,7 @@ static void ivb_update_cursor(struct drm_crtc *crtc, u32 base, bool force)
cntl |= CURSOR_PIPE_CSC_ENABLE;
cntl &= ~CURSOR_TRICKLE_FEED_DISABLE;
}
- if (intel_crtc->cursor_rotation == BIT(DRM_ROTATE_180))
+ if (rotation == BIT(DRM_ROTATE_180))
cntl |= CURSOR_ROTATE_180;
else
cntl &= ~CURSOR_ROTATE_180;
@@ -7538,10 +7547,24 @@ static void intel_crtc_update_cursor(struct drm_crtc *crtc,
int y = intel_crtc->cursor_y;
u32 base = 0, pos = 0;
bool visible;
+ struct drm_rect r = {
+ .x1 = x,
+ .x2 = x + intel_crtc->cursor_width,
+ .y1 = y,
+ .y2 = y + intel_crtc->cursor_height,
+ };
if (on)
base = intel_crtc->cursor_addr;
+ drm_rect_rotate(&r,
+ intel_crtc->config.pipe_src_w,
+ intel_crtc->config.pipe_src_h,
+ intel_crtc->pipe_rotation);
+
+ x = r.x1;
+ y = r.y1;
+
if (x >= intel_crtc->config.pipe_src_w)
base = 0;
@@ -8818,6 +8841,66 @@ free_work:
return ret;
}
+static int intel_set_primary_plane_rotation(struct intel_crtc *crtc,
+ unsigned int rotation)
+{
+ struct drm_device *dev = crtc->base.dev;
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ unsigned int old_rotation;
+ int ret = 0;
+
+ old_rotation = crtc->primary_rotation;
+ crtc->primary_rotation = rotation;
+
+ if (!crtc->active)
+ return 0;
+
+ rotation = drm_rotation_chain(crtc->pipe_rotation,
+ crtc->primary_rotation);
+
+ intel_crtc_wait_for_pending_flips(&crtc->base);
+
+ /* FBC does not work on some platforms for rotated planes */
+ if (dev_priv->fbc.plane == crtc->plane &&
+ INTEL_INFO(dev)->gen <= 4 && !IS_G4X(dev) &&
+ rotation != BIT(DRM_ROTATE_0))
+ intel_disable_fbc(dev);
+
+ ret = dev_priv->display.update_plane(&crtc->base, crtc->base.fb, 0, 0);
+ if (ret)
+ crtc->primary_rotation = old_rotation;
+
+ return ret;
+}
+
+static void intel_set_cursor_plane_rotation(struct intel_crtc *crtc,
+ unsigned int rotation)
+{
+ crtc->cursor_rotation = rotation;
+
+ if (crtc->active)
+ intel_crtc_update_cursor(&crtc->base, true, true);
+}
+
+static int intel_update_planes(struct intel_crtc *crtc)
+{
+ struct drm_device *dev = crtc->base.dev;
+ struct intel_plane *plane;
+
+ list_for_each_entry(plane, &dev->mode_config.plane_list, base.head) {
+ int ret;
+
+ if (plane->pipe != crtc->pipe)
+ continue;
+
+ ret = intel_plane_restore(&plane->base);
+ if (ret)
+ return ret;
+ }
+
+ return 0;
+}
+
static int intel_crtc_set_property(struct drm_crtc *crtc,
struct drm_property *prop,
uint64_t val)
@@ -8828,27 +8911,51 @@ static int intel_crtc_set_property(struct drm_crtc *crtc,
uint64_t old_val;
int ret = -ENOENT;
- if (prop == dev_priv->plane_rotation_property) {
+ if (prop == dev_priv->rotation_property) {
/* exactly one rotation angle please */
if (hweight32(val & 0xf) != 1)
return -EINVAL;
- old_val = intel_crtc->primary_rotation;
- intel_crtc->primary_rotation = val;
+ old_val = intel_crtc->pipe_rotation;
+ intel_crtc->pipe_rotation = val;
- if (intel_crtc->active) {
- 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_crtc->primary_rotation != BIT(DRM_ROTATE_0))
- intel_disable_fbc(dev);
+ ret = intel_set_primary_plane_rotation(intel_crtc,
+ intel_crtc->primary_rotation);
+ if (ret) {
+ intel_crtc->pipe_rotation = old_val;
+ return ret;
+ }
- ret = dev_priv->display.update_plane(crtc, crtc->fb, 0, 0);
- if (ret)
- intel_crtc->primary_rotation = old_val;
+ ret = intel_update_planes(intel_crtc);
+ if (ret) {
+ intel_crtc->pipe_rotation = old_val;
+
+ if (intel_set_primary_plane_rotation(intel_crtc,
+ intel_crtc->primary_rotation))
+ DRM_ERROR("failed to restore primary plane rotation\n");
+ if (intel_update_planes(intel_crtc))
+ DRM_ERROR("failed to restore sprite plane rotation\n");
+ return ret;
}
+
+ intel_set_cursor_plane_rotation(intel_crtc,
+ intel_crtc->cursor_rotation);
+
+ return 0;
+ } else if (prop == dev_priv->cursor_rotation_property) {
+ /* exactly one rotation angle please */
+ if (hweight32(val & 0xf) != 1)
+ return -EINVAL;
+
+ intel_set_cursor_plane_rotation(intel_crtc, val);
+
+ return 0;
+ } else if (prop == dev_priv->plane_rotation_property) {
+ /* exactly one rotation angle please */
+ if (hweight32(val & 0xf) != 1)
+ return -EINVAL;
+
+ return intel_set_primary_plane_rotation(intel_crtc, val);
}
return ret;
@@ -10397,6 +10504,7 @@ static void intel_crtc_init(struct drm_device *dev, int pipe)
intel_crtc->plane = pipe;
intel_crtc->primary_rotation = BIT(DRM_ROTATE_0);
intel_crtc->cursor_rotation = BIT(DRM_ROTATE_0);
+ intel_crtc->pipe_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;
@@ -10427,6 +10535,16 @@ static void intel_crtc_init(struct drm_device *dev, int pipe)
drm_object_attach_property(&intel_crtc->base.base,
dev_priv->cursor_rotation_property,
intel_crtc->cursor_rotation);
+
+ if (!dev_priv->rotation_property)
+ dev_priv->rotation_property =
+ drm_mode_create_rotation_property(dev, "rotation",
+ BIT(DRM_ROTATE_0) |
+ BIT(DRM_ROTATE_180));
+ if (dev_priv->rotation_property)
+ drm_object_attach_property(&intel_crtc->base.base,
+ dev_priv->rotation_property,
+ intel_crtc->pipe_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 4a7f4f1..f967abf 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -333,6 +333,7 @@ struct intel_crtc {
enum plane plane;
unsigned int primary_rotation; /* primary plane in relation to the pipe */
unsigned int cursor_rotation; /* cursor plane in relation to the pipe */
+ unsigned int pipe_rotation; /* entire 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 5ebeb78..3735815 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -463,6 +463,7 @@ void intel_update_fbc(struct drm_device *dev)
struct drm_i915_gem_object *obj;
const struct drm_display_mode *adjusted_mode;
unsigned int max_width, max_height;
+ unsigned int rotation;
if (!HAS_FBC(dev)) {
set_no_fbc_reason(dev_priv, FBC_UNSUPPORTED);
@@ -557,8 +558,11 @@ void intel_update_fbc(struct drm_device *dev)
goto out_disable;
}
+ rotation = drm_rotation_chain(intel_crtc->pipe_rotation,
+ intel_crtc->primary_rotation);
+
if (INTEL_INFO(dev)->gen <= 4 && !IS_G4X(dev) &&
- intel_crtc->primary_rotation != BIT(DRM_ROTATE_0)) {
+ rotation != BIT(DRM_ROTATE_0)) {
if (set_no_fbc_reason(dev_priv, FBC_UNSUPPORTED_MODE))
DRM_DEBUG_KMS("mode incompatible with compression, "
"disabling\n");
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 2936007..e1d593c 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -53,6 +53,8 @@ vlv_update_plane(struct drm_plane *dplane, struct drm_crtc *crtc,
u32 sprctl;
unsigned long sprsurf_offset, linear_offset;
int pixel_size = drm_format_plane_cpp(fb->pixel_format, 0);
+ unsigned int rotation = drm_rotation_chain(to_intel_crtc(crtc)->pipe_rotation,
+ intel_plane->rotation);
sprctl = I915_READ(SPCNTR(pipe, plane));
@@ -132,7 +134,7 @@ vlv_update_plane(struct drm_plane *dplane, struct drm_crtc *crtc,
fb->pitches[0]);
linear_offset -= sprsurf_offset;
- if (intel_plane->rotation == BIT(DRM_ROTATE_180)) {
+ if (rotation == BIT(DRM_ROTATE_180)) {
sprctl |= SP_ROTATE_180;
x += src_w;
@@ -239,6 +241,8 @@ ivb_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
u32 sprctl, sprscale = 0;
unsigned long sprsurf_offset, linear_offset;
int pixel_size = drm_format_plane_cpp(fb->pixel_format, 0);
+ unsigned int rotation = drm_rotation_chain(to_intel_crtc(crtc)->pipe_rotation,
+ intel_plane->rotation);
sprctl = I915_READ(SPRCTL(pipe));
@@ -309,7 +313,7 @@ ivb_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
pixel_size, fb->pitches[0]);
linear_offset -= sprsurf_offset;
- if (intel_plane->rotation == BIT(DRM_ROTATE_180)) {
+ if (rotation == BIT(DRM_ROTATE_180)) {
sprctl |= SPRITE_ROTATE_180;
/* HSW and BDW does this automagically in hardware */
@@ -435,6 +439,8 @@ ilk_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
unsigned long dvssurf_offset, linear_offset;
u32 dvscntr, dvsscale;
int pixel_size = drm_format_plane_cpp(fb->pixel_format, 0);
+ unsigned int rotation = drm_rotation_chain(to_intel_crtc(crtc)->pipe_rotation,
+ intel_plane->rotation);
dvscntr = I915_READ(DVSCNTR(pipe));
@@ -500,7 +506,7 @@ ilk_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
pixel_size, fb->pitches[0]);
linear_offset -= dvssurf_offset;
- if (intel_plane->rotation == BIT(DRM_ROTATE_180)) {
+ if (rotation == BIT(DRM_ROTATE_180)) {
dvscntr |= DVS_ROTATE_180;
x += src_w;
@@ -738,6 +744,8 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
.src_w = src_w,
.src_h = src_h,
};
+ unsigned int rotation = drm_rotation_chain(intel_crtc->pipe_rotation,
+ intel_plane->rotation);
/* Don't modify another pipe's plane */
if (intel_plane->pipe != intel_crtc->pipe) {
@@ -769,8 +777,11 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
max_scale = intel_plane->max_downscale << 16;
min_scale = intel_plane->can_scale ? 1 : (1 << 16);
+ drm_rect_rotate(&dst, drm_rect_width(&clip), drm_rect_height(&clip),
+ intel_crtc->pipe_rotation);
+
drm_rect_rotate(&src, fb->width << 16, fb->height << 16,
- intel_plane->rotation);
+ rotation);
hscale = drm_rect_calc_hscale_relaxed(&src, &dst, min_scale, max_scale);
BUG_ON(hscale < 0);
@@ -811,7 +822,7 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
drm_rect_height(&dst) * vscale - drm_rect_height(&src));
drm_rect_rotate_inv(&src, fb->width << 16, fb->height << 16,
- intel_plane->rotation);
+ rotation);
/* sanity check to make sure the src viewport wasn't enlarged */
WARN_ON(src.x1 < (int) src_x ||
--
1.8.3.2
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [PATCH 5/5] drm/i915: Add full pipe rotation
2014-02-12 21:15 ` [PATCH 5/5] drm/i915: Add full pipe rotation ville.syrjala
@ 2014-02-19 10:25 ` Sagar Arun Kamble
0 siblings, 0 replies; 24+ messages in thread
From: Sagar Arun Kamble @ 2014-02-19 10:25 UTC (permalink / raw)
To: ville.syrjala; +Cc: intel-gfx, dri-devel
Reviewed-by: Sagar Kamble <sagar.a.kamble@intel.com>
On Wed, 2014-02-12 at 23:15 +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> We can pretend that we can rotate the entire pipe by rotating all the
> planes and adjusting their positions appropriately. Add a "rotation"
> property on the crtc which will do this.
>
> The main upshot of doing the full pipe rotation instead of rotating all
> the planes individually is that the plane positions turn out correct
> automagically. So userspace doesn't need to do anything except toggle
> the property and continue as if nothing had changed.
>
> The actual implementation is pretty much trivial thanks to drm_rect
> and drm_rotation_chain() ;)
>
> 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 | 6 ++
> drivers/gpu/drm/i915/intel_display.c | 154 +++++++++++++++++++++++++++++++----
> drivers/gpu/drm/i915/intel_drv.h | 1 +
> drivers/gpu/drm/i915/intel_pm.c | 6 +-
> drivers/gpu/drm/i915/intel_sprite.c | 21 +++--
> 5 files changed, 164 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> index 3dd9abb..b59bff1 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -1914,6 +1914,12 @@ void i915_driver_lastclose(struct drm_device * dev)
> dev_priv->rotation_property,
> plane->rotation);
> }
> + list_for_each_entry(crtc, &dev->mode_config.crtc_list, base.head) {
> + crtc->pipe_rotation = BIT(DRM_ROTATE_0);
> + drm_object_property_set_value(&crtc->base.base,
> + dev_priv->rotation_property,
> + crtc->pipe_rotation);
> + }
> }
>
> if (dev_priv->cursor_rotation_property) {
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index e94167b..1b74d24 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -39,6 +39,7 @@
> #include "i915_trace.h"
> #include <drm/drm_dp_helper.h>
> #include <drm/drm_crtc_helper.h>
> +#include <drm/drm_rect.h>
> #include <linux/dma_remapping.h>
>
> static void intel_increase_pllclock(struct drm_crtc *crtc);
> @@ -2060,6 +2061,8 @@ static int i9xx_update_plane(struct drm_crtc *crtc, struct drm_framebuffer *fb,
> u32 dspcntr;
> u32 reg;
> int pixel_size;
> + unsigned int rotation = drm_rotation_chain(intel_crtc->pipe_rotation,
> + intel_crtc->primary_rotation);
>
> switch (plane) {
> case 0:
> @@ -2133,7 +2136,7 @@ static int i9xx_update_plane(struct drm_crtc *crtc, struct drm_framebuffer *fb,
> intel_crtc->dspaddr_offset = linear_offset;
> }
>
> - if (intel_crtc->primary_rotation == BIT(DRM_ROTATE_180)) {
> + if (rotation == BIT(DRM_ROTATE_180)) {
> dspcntr |= DISPPLANE_ROTATE_180;
>
> x += (intel_crtc->config.pipe_src_w - 1);
> @@ -2173,6 +2176,8 @@ static int ironlake_update_plane(struct drm_crtc *crtc,
> u32 dspcntr;
> u32 reg;
> int pixel_size;
> + unsigned int rotation = drm_rotation_chain(intel_crtc->pipe_rotation,
> + intel_crtc->primary_rotation);
>
> switch (plane) {
> case 0:
> @@ -2238,7 +2243,7 @@ static int ironlake_update_plane(struct drm_crtc *crtc,
> fb->pitches[0]);
> linear_offset -= intel_crtc->dspaddr_offset;
>
> - if (intel_crtc->primary_rotation == BIT(DRM_ROTATE_180)) {
> + if (rotation == BIT(DRM_ROTATE_180)) {
> dspcntr |= DISPPLANE_ROTATE_180;
>
> if (!IS_HASWELL(dev) && !IS_BROADWELL(dev)) {
> @@ -7468,6 +7473,8 @@ static void i9xx_update_cursor(struct drm_crtc *crtc, u32 base, bool force)
> bool visible = base != 0;
>
> if (force || intel_crtc->cursor_visible != visible) {
> + unsigned int rotation = drm_rotation_chain(intel_crtc->pipe_rotation,
> + intel_crtc->cursor_rotation);
> uint32_t cntl = I915_READ(CURCNTR(pipe));
> if (base) {
> cntl &= ~(CURSOR_MODE | MCURSOR_PIPE_SELECT);
> @@ -7477,7 +7484,7 @@ static void i9xx_update_cursor(struct drm_crtc *crtc, u32 base, bool force)
> cntl &= ~(CURSOR_MODE | MCURSOR_GAMMA_ENABLE);
> cntl |= CURSOR_MODE_DISABLE;
> }
> - if (intel_crtc->cursor_rotation == BIT(DRM_ROTATE_180))
> + if (rotation == BIT(DRM_ROTATE_180))
> cntl |= CURSOR_ROTATE_180;
> else
> cntl &= ~CURSOR_ROTATE_180;
> @@ -7500,6 +7507,8 @@ static void ivb_update_cursor(struct drm_crtc *crtc, u32 base, bool force)
> bool visible = base != 0;
>
> if (force || intel_crtc->cursor_visible != visible) {
> + unsigned int rotation = drm_rotation_chain(intel_crtc->pipe_rotation,
> + intel_crtc->cursor_rotation);
> uint32_t cntl = I915_READ(CURCNTR_IVB(pipe));
> if (base) {
> cntl &= ~CURSOR_MODE;
> @@ -7512,7 +7521,7 @@ static void ivb_update_cursor(struct drm_crtc *crtc, u32 base, bool force)
> cntl |= CURSOR_PIPE_CSC_ENABLE;
> cntl &= ~CURSOR_TRICKLE_FEED_DISABLE;
> }
> - if (intel_crtc->cursor_rotation == BIT(DRM_ROTATE_180))
> + if (rotation == BIT(DRM_ROTATE_180))
> cntl |= CURSOR_ROTATE_180;
> else
> cntl &= ~CURSOR_ROTATE_180;
> @@ -7538,10 +7547,24 @@ static void intel_crtc_update_cursor(struct drm_crtc *crtc,
> int y = intel_crtc->cursor_y;
> u32 base = 0, pos = 0;
> bool visible;
> + struct drm_rect r = {
> + .x1 = x,
> + .x2 = x + intel_crtc->cursor_width,
> + .y1 = y,
> + .y2 = y + intel_crtc->cursor_height,
> + };
>
> if (on)
> base = intel_crtc->cursor_addr;
>
> + drm_rect_rotate(&r,
> + intel_crtc->config.pipe_src_w,
> + intel_crtc->config.pipe_src_h,
> + intel_crtc->pipe_rotation);
> +
> + x = r.x1;
> + y = r.y1;
> +
> if (x >= intel_crtc->config.pipe_src_w)
> base = 0;
>
> @@ -8818,6 +8841,66 @@ free_work:
> return ret;
> }
>
> +static int intel_set_primary_plane_rotation(struct intel_crtc *crtc,
> + unsigned int rotation)
> +{
> + struct drm_device *dev = crtc->base.dev;
> + struct drm_i915_private *dev_priv = dev->dev_private;
> + unsigned int old_rotation;
> + int ret = 0;
> +
> + old_rotation = crtc->primary_rotation;
> + crtc->primary_rotation = rotation;
> +
> + if (!crtc->active)
> + return 0;
> +
> + rotation = drm_rotation_chain(crtc->pipe_rotation,
> + crtc->primary_rotation);
> +
> + intel_crtc_wait_for_pending_flips(&crtc->base);
> +
> + /* FBC does not work on some platforms for rotated planes */
> + if (dev_priv->fbc.plane == crtc->plane &&
> + INTEL_INFO(dev)->gen <= 4 && !IS_G4X(dev) &&
> + rotation != BIT(DRM_ROTATE_0))
> + intel_disable_fbc(dev);
> +
> + ret = dev_priv->display.update_plane(&crtc->base, crtc->base.fb, 0, 0);
> + if (ret)
> + crtc->primary_rotation = old_rotation;
> +
> + return ret;
> +}
> +
> +static void intel_set_cursor_plane_rotation(struct intel_crtc *crtc,
> + unsigned int rotation)
> +{
> + crtc->cursor_rotation = rotation;
> +
> + if (crtc->active)
> + intel_crtc_update_cursor(&crtc->base, true, true);
> +}
> +
> +static int intel_update_planes(struct intel_crtc *crtc)
> +{
> + struct drm_device *dev = crtc->base.dev;
> + struct intel_plane *plane;
> +
> + list_for_each_entry(plane, &dev->mode_config.plane_list, base.head) {
> + int ret;
> +
> + if (plane->pipe != crtc->pipe)
> + continue;
> +
> + ret = intel_plane_restore(&plane->base);
> + if (ret)
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> static int intel_crtc_set_property(struct drm_crtc *crtc,
> struct drm_property *prop,
> uint64_t val)
> @@ -8828,27 +8911,51 @@ static int intel_crtc_set_property(struct drm_crtc *crtc,
> uint64_t old_val;
> int ret = -ENOENT;
>
> - if (prop == dev_priv->plane_rotation_property) {
> + if (prop == dev_priv->rotation_property) {
> /* exactly one rotation angle please */
> if (hweight32(val & 0xf) != 1)
> return -EINVAL;
>
> - old_val = intel_crtc->primary_rotation;
> - intel_crtc->primary_rotation = val;
> + old_val = intel_crtc->pipe_rotation;
> + intel_crtc->pipe_rotation = val;
>
> - if (intel_crtc->active) {
> - 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_crtc->primary_rotation != BIT(DRM_ROTATE_0))
> - intel_disable_fbc(dev);
> + ret = intel_set_primary_plane_rotation(intel_crtc,
> + intel_crtc->primary_rotation);
> + if (ret) {
> + intel_crtc->pipe_rotation = old_val;
> + return ret;
> + }
>
> - ret = dev_priv->display.update_plane(crtc, crtc->fb, 0, 0);
> - if (ret)
> - intel_crtc->primary_rotation = old_val;
> + ret = intel_update_planes(intel_crtc);
> + if (ret) {
> + intel_crtc->pipe_rotation = old_val;
> +
> + if (intel_set_primary_plane_rotation(intel_crtc,
> + intel_crtc->primary_rotation))
> + DRM_ERROR("failed to restore primary plane rotation\n");
> + if (intel_update_planes(intel_crtc))
> + DRM_ERROR("failed to restore sprite plane rotation\n");
> + return ret;
> }
> +
> + intel_set_cursor_plane_rotation(intel_crtc,
> + intel_crtc->cursor_rotation);
> +
> + return 0;
> + } else if (prop == dev_priv->cursor_rotation_property) {
> + /* exactly one rotation angle please */
> + if (hweight32(val & 0xf) != 1)
> + return -EINVAL;
> +
> + intel_set_cursor_plane_rotation(intel_crtc, val);
> +
> + return 0;
> + } else if (prop == dev_priv->plane_rotation_property) {
> + /* exactly one rotation angle please */
> + if (hweight32(val & 0xf) != 1)
> + return -EINVAL;
> +
> + return intel_set_primary_plane_rotation(intel_crtc, val);
> }
>
> return ret;
> @@ -10397,6 +10504,7 @@ static void intel_crtc_init(struct drm_device *dev, int pipe)
> intel_crtc->plane = pipe;
> intel_crtc->primary_rotation = BIT(DRM_ROTATE_0);
> intel_crtc->cursor_rotation = BIT(DRM_ROTATE_0);
> + intel_crtc->pipe_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;
> @@ -10427,6 +10535,16 @@ static void intel_crtc_init(struct drm_device *dev, int pipe)
> drm_object_attach_property(&intel_crtc->base.base,
> dev_priv->cursor_rotation_property,
> intel_crtc->cursor_rotation);
> +
> + if (!dev_priv->rotation_property)
> + dev_priv->rotation_property =
> + drm_mode_create_rotation_property(dev, "rotation",
> + BIT(DRM_ROTATE_0) |
> + BIT(DRM_ROTATE_180));
> + if (dev_priv->rotation_property)
> + drm_object_attach_property(&intel_crtc->base.base,
> + dev_priv->rotation_property,
> + intel_crtc->pipe_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 4a7f4f1..f967abf 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -333,6 +333,7 @@ struct intel_crtc {
> enum plane plane;
> unsigned int primary_rotation; /* primary plane in relation to the pipe */
> unsigned int cursor_rotation; /* cursor plane in relation to the pipe */
> + unsigned int pipe_rotation; /* entire 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 5ebeb78..3735815 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -463,6 +463,7 @@ void intel_update_fbc(struct drm_device *dev)
> struct drm_i915_gem_object *obj;
> const struct drm_display_mode *adjusted_mode;
> unsigned int max_width, max_height;
> + unsigned int rotation;
>
> if (!HAS_FBC(dev)) {
> set_no_fbc_reason(dev_priv, FBC_UNSUPPORTED);
> @@ -557,8 +558,11 @@ void intel_update_fbc(struct drm_device *dev)
> goto out_disable;
> }
>
> + rotation = drm_rotation_chain(intel_crtc->pipe_rotation,
> + intel_crtc->primary_rotation);
> +
> if (INTEL_INFO(dev)->gen <= 4 && !IS_G4X(dev) &&
> - intel_crtc->primary_rotation != BIT(DRM_ROTATE_0)) {
> + rotation != BIT(DRM_ROTATE_0)) {
> if (set_no_fbc_reason(dev_priv, FBC_UNSUPPORTED_MODE))
> DRM_DEBUG_KMS("mode incompatible with compression, "
> "disabling\n");
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> index 2936007..e1d593c 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -53,6 +53,8 @@ vlv_update_plane(struct drm_plane *dplane, struct drm_crtc *crtc,
> u32 sprctl;
> unsigned long sprsurf_offset, linear_offset;
> int pixel_size = drm_format_plane_cpp(fb->pixel_format, 0);
> + unsigned int rotation = drm_rotation_chain(to_intel_crtc(crtc)->pipe_rotation,
> + intel_plane->rotation);
>
> sprctl = I915_READ(SPCNTR(pipe, plane));
>
> @@ -132,7 +134,7 @@ vlv_update_plane(struct drm_plane *dplane, struct drm_crtc *crtc,
> fb->pitches[0]);
> linear_offset -= sprsurf_offset;
>
> - if (intel_plane->rotation == BIT(DRM_ROTATE_180)) {
> + if (rotation == BIT(DRM_ROTATE_180)) {
> sprctl |= SP_ROTATE_180;
>
> x += src_w;
> @@ -239,6 +241,8 @@ ivb_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
> u32 sprctl, sprscale = 0;
> unsigned long sprsurf_offset, linear_offset;
> int pixel_size = drm_format_plane_cpp(fb->pixel_format, 0);
> + unsigned int rotation = drm_rotation_chain(to_intel_crtc(crtc)->pipe_rotation,
> + intel_plane->rotation);
>
> sprctl = I915_READ(SPRCTL(pipe));
>
> @@ -309,7 +313,7 @@ ivb_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
> pixel_size, fb->pitches[0]);
> linear_offset -= sprsurf_offset;
>
> - if (intel_plane->rotation == BIT(DRM_ROTATE_180)) {
> + if (rotation == BIT(DRM_ROTATE_180)) {
> sprctl |= SPRITE_ROTATE_180;
>
> /* HSW and BDW does this automagically in hardware */
> @@ -435,6 +439,8 @@ ilk_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
> unsigned long dvssurf_offset, linear_offset;
> u32 dvscntr, dvsscale;
> int pixel_size = drm_format_plane_cpp(fb->pixel_format, 0);
> + unsigned int rotation = drm_rotation_chain(to_intel_crtc(crtc)->pipe_rotation,
> + intel_plane->rotation);
>
> dvscntr = I915_READ(DVSCNTR(pipe));
>
> @@ -500,7 +506,7 @@ ilk_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
> pixel_size, fb->pitches[0]);
> linear_offset -= dvssurf_offset;
>
> - if (intel_plane->rotation == BIT(DRM_ROTATE_180)) {
> + if (rotation == BIT(DRM_ROTATE_180)) {
> dvscntr |= DVS_ROTATE_180;
>
> x += src_w;
> @@ -738,6 +744,8 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
> .src_w = src_w,
> .src_h = src_h,
> };
> + unsigned int rotation = drm_rotation_chain(intel_crtc->pipe_rotation,
> + intel_plane->rotation);
>
> /* Don't modify another pipe's plane */
> if (intel_plane->pipe != intel_crtc->pipe) {
> @@ -769,8 +777,11 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
> max_scale = intel_plane->max_downscale << 16;
> min_scale = intel_plane->can_scale ? 1 : (1 << 16);
>
> + drm_rect_rotate(&dst, drm_rect_width(&clip), drm_rect_height(&clip),
> + intel_crtc->pipe_rotation);
> +
> drm_rect_rotate(&src, fb->width << 16, fb->height << 16,
> - intel_plane->rotation);
> + rotation);
>
> hscale = drm_rect_calc_hscale_relaxed(&src, &dst, min_scale, max_scale);
> BUG_ON(hscale < 0);
> @@ -811,7 +822,7 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
> drm_rect_height(&dst) * vscale - drm_rect_height(&src));
>
> drm_rect_rotate_inv(&src, fb->width << 16, fb->height << 16,
> - intel_plane->rotation);
> + rotation);
>
> /* sanity check to make sure the src viewport wasn't enlarged */
> WARN_ON(src.x1 < (int) src_x ||
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 0/5] drm/i915: Full pipe rotation & rotation property name bikeshedding
2014-02-12 21:14 [PATCH 0/5] drm/i915: Full pipe rotation & rotation property name bikeshedding ville.syrjala
` (4 preceding siblings ...)
2014-02-12 21:15 ` [PATCH 5/5] drm/i915: Add full pipe rotation ville.syrjala
@ 2014-02-12 23:17 ` Chris Wilson
2014-02-13 8:51 ` Ville Syrjälä
2014-02-13 11:20 ` Chris Wilson
6 siblings, 1 reply; 24+ messages in thread
From: Chris Wilson @ 2014-02-12 23:17 UTC (permalink / raw)
To: ville.syrjala; +Cc: intel-gfx, Sagar Kamble, dri-devel
On Wed, Feb 12, 2014 at 11:14:59PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> After playing around Sagar's primary plane rotation a bit, I decided that
> extending that to full pipe rotation would be nice. Chris also seemed to
> want that, but I'm not sure he does anymore :) But then I decided it's so
> easy to implement that I can't leave it hanging. So here it is.
It would make the video sprite easier, as it would just work with a very
minimal change. And it would make using the sprite easier for DRI window
flips. I need to dig a bit as to find out how to fixup cursors I think.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH 0/5] drm/i915: Full pipe rotation & rotation property name bikeshedding
2014-02-12 23:17 ` [PATCH 0/5] drm/i915: Full pipe rotation & rotation property name bikeshedding Chris Wilson
@ 2014-02-13 8:51 ` Ville Syrjälä
0 siblings, 0 replies; 24+ messages in thread
From: Ville Syrjälä @ 2014-02-13 8:51 UTC (permalink / raw)
To: Chris Wilson, intel-gfx, Sagar Kamble, dri-devel
On Wed, Feb 12, 2014 at 11:17:27PM +0000, Chris Wilson wrote:
> On Wed, Feb 12, 2014 at 11:14:59PM +0200, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > After playing around Sagar's primary plane rotation a bit, I decided that
> > extending that to full pipe rotation would be nice. Chris also seemed to
> > want that, but I'm not sure he does anymore :) But then I decided it's so
> > easy to implement that I can't leave it hanging. So here it is.
>
> It would make the video sprite easier, as it would just work with a very
> minimal change. And it would make using the sprite easier for DRI window
> flips. I need to dig a bit as to find out how to fixup cursors I think.
Cursors should just work (tm) with full pipe rotation. You get into a
bit more trouble if you rotate just the cursor plane since then you get
to fix up the dst coordinates if your visible cursor is smaller than the
fixed cursor size. Or did you mean something else?
BTW if you want to try it, I pushed the lot to:
git://gitorious.org/vsyrjala/linux.git rotate_latest_2
--
Ville Syrjälä
Intel OTC
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 0/5] drm/i915: Full pipe rotation & rotation property name bikeshedding
2014-02-12 21:14 [PATCH 0/5] drm/i915: Full pipe rotation & rotation property name bikeshedding ville.syrjala
` (5 preceding siblings ...)
2014-02-12 23:17 ` [PATCH 0/5] drm/i915: Full pipe rotation & rotation property name bikeshedding Chris Wilson
@ 2014-02-13 11:20 ` Chris Wilson
2014-02-13 11:58 ` Ville Syrjälä
6 siblings, 1 reply; 24+ messages in thread
From: Chris Wilson @ 2014-02-13 11:20 UTC (permalink / raw)
To: ville.syrjala; +Cc: intel-gfx, Sagar Kamble, dri-devel
On Wed, Feb 12, 2014 at 11:14:59PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> After playing around Sagar's primary plane rotation a bit, I decided that
> extending that to full pipe rotation would be nice. Chris also seemed to
> want that, but I'm not sure he does anymore :) But then I decided it's so
> easy to implement that I can't leave it hanging. So here it is.
Ok, another fun issue is that I still have to reset all plane rotations
when switching VT to X anyway, so having a property that chains the pipe
on top of the plane rotations, just increases the amount of work I have
to do. I may as well just set each plane rotation.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH 0/5] drm/i915: Full pipe rotation & rotation property name bikeshedding
2014-02-13 11:20 ` Chris Wilson
@ 2014-02-13 11:58 ` Ville Syrjälä
0 siblings, 0 replies; 24+ messages in thread
From: Ville Syrjälä @ 2014-02-13 11:58 UTC (permalink / raw)
To: Chris Wilson, intel-gfx, Sagar Kamble, dri-devel
On Thu, Feb 13, 2014 at 11:20:49AM +0000, Chris Wilson wrote:
> On Wed, Feb 12, 2014 at 11:14:59PM +0200, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > After playing around Sagar's primary plane rotation a bit, I decided that
> > extending that to full pipe rotation would be nice. Chris also seemed to
> > want that, but I'm not sure he does anymore :) But then I decided it's so
> > easy to implement that I can't leave it hanging. So here it is.
>
> Ok, another fun issue is that I still have to reset all plane rotations
> when switching VT to X anyway, so having a property that chains the pipe
> on top of the plane rotations, just increases the amount of work I have
> to do. I may as well just set each plane rotation.
Yeah if we had atomic modeset + per-master state, and fbdev would be
like a master in this case, the correct state would get magically
blasted into the hardware on vt switch. One can dream, right? ;)
Even without that, I'd still vote for adding the full crtc rotation
property. It frees the client from having to adjust the coordinates
of individual planes when rotating the world. I guess for you that
would just mean resetting it to DRM_ROTATE_0 always.
--
Ville Syrjälä
Intel OTC
^ permalink raw reply [flat|nested] 24+ messages in thread