[PATCH 4/5] drm/i915: add 'rotation' CRTC property
Marcus Lorentzon
marcus.xm.lorentzon at stericsson.com
Tue Mar 20 09:48:14 PDT 2012
Hi Paulo,
thanks for a quick response posting the patches.
In my use of CRTC rotation properties, I need the change to be
synchronized with modeset. Your implementation activate the property
change immediately instead of staging it for next modeset. Do you think
it is ok for different drivers to behave differently when properties are
set? I can't allow glitches in the output when changing the rotation in
one frame and flipping the framebuffer to one of correct size and
content in another. This would mean I will have to stage the setting and
apply it at next modeset (different behavior). Or do I just have to wait
until we have an atomic modeset that can also set properties (Jesse can
you post your latest proposal ...)?
Do you think you could export the call registering the rotation
property? That way other drivers can reuse it and we don't risk multiple
properties with different ranges etc for the same logical property (like
400 mil degrees or 0-3, 1-4, UR,UD,CCW,CW ...). So some sort of standard
property create.
Then it would also be nice to define some rules in KMS of what should be
modeset params and what should be properties. Modeset params seems hard
to add (struct change) and properties seems very loosely defined and
device specific. Having something in between would be nice (standard and
easy to add), to make it easier to add things like plane z-order
(instead of driver specific ioctls or properties).
Maybe a new "atomic" modeset with just a list of {object_id,
property_id, value} making all params into properties and modeset more
dynamic. It would also solve the atomic modset, even with device
specific properties and future features.
Sorry for multiple topics, but they are sort of related.
/BR
/Marcus
On 03/20/2012 03:48 PM, Paulo Zanoni wrote:
> From: Paulo Zanoni<paulo.r.zanoni at intel.com>
>
> This property is needed so we can inform the KVMr feature about our
> current rotation: whenever we change the rotation, we should change
> that property so that the KVMr knows that the screen is rotated.
>
> How to reproduce the problem:
> - on an AMT machine, enable KVM
> - boot the machine, use xrandr to rotate the display
> - use VNC to connect to the KVM
> - try to use the mouse
>
> v2: only create the property once
>
> Signed-off-by: Paulo Zanoni<paulo.r.zanoni at intel.com>
> ---
> drivers/gpu/drm/i915/i915_drv.h | 1 +
> drivers/gpu/drm/i915/i915_reg.h | 5 +++
> drivers/gpu/drm/i915/intel_display.c | 66 ++++++++++++++++++++++++++++++++++
> 3 files changed, 72 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index e7a00b7..7994c4f 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -780,6 +780,7 @@ typedef struct drm_i915_private {
>
> struct drm_property *broadcast_rgb_property;
> struct drm_property *force_audio_property;
> + struct drm_property *rotation_property;
> } drm_i915_private_t;
>
> enum hdmi_force_audio {
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 52a06be..79e8b12 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -2407,6 +2407,11 @@
> #define PIPECONF_INTERLACED_DBL_ILK (4<< 21) /* ilk/snb only */
> #define PIPECONF_PFIT_PF_INTERLACED_DBL_ILK (5<< 21) /* ilk/snb only */
> #define PIPECONF_CXSR_DOWNCLOCK (1<<16)
> +#define PIPECONF_ROTATION_MASK (3<< 14)
> +#define PIPECONF_ROTATION_0 (0<< 14)
> +#define PIPECONF_ROTATION_90 (1<< 14)
> +#define PIPECONF_ROTATION_180 (2<< 14)
> +#define PIPECONF_ROTATION_270 (3<< 14)
> #define PIPECONF_BPP_MASK (0x000000e0)
> #define PIPECONF_BPP_8 (0<<5)
> #define PIPECONF_BPP_10 (1<<5)
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 683002fb..4842de8 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -7625,6 +7625,50 @@ static void intel_crtc_reset(struct drm_crtc *crtc)
> intel_sanitize_modesetting(dev, intel_crtc->pipe, intel_crtc->plane);
> }
>
> +static void intel_crtc_set_rotation(struct drm_crtc *crtc,
> + uint64_t rotation)
> +{
> + struct drm_device *dev = crtc->dev;
> + struct drm_i915_private *dev_priv = dev->dev_private;
> + struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> + int reg = PIPECONF(intel_crtc->pipe);
> + u32 val = I915_READ(reg);
> +
> + val&= ~PIPECONF_ROTATION_MASK;
> +
> + switch (rotation) {
> + case 0:
> + val |= PIPECONF_ROTATION_0;
> + break;
> + case 90:
> + val |= PIPECONF_ROTATION_90;
> + break;
> + case 180:
> + val |= PIPECONF_ROTATION_180;
> + break;
> + case 270:
> + val |= PIPECONF_ROTATION_270;
> + break;
> + default:
> + DRM_ERROR("Unsupported rotation: %Lu\n", rotation);
> + val |= PIPECONF_ROTATION_0;
> + }
> +
> + I915_WRITE(reg, val);
> +}
> +
> +static int intel_crtc_set_property(struct drm_crtc *crtc,
> + struct drm_property *property,
> + uint64_t val)
> +{
> + struct drm_device *dev = crtc->dev;
> + struct drm_i915_private *dev_priv = dev->dev_private;
> +
> + if (property == dev_priv->rotation_property)
> + intel_crtc_set_rotation(crtc, val);
> + return 0;
> +}
> +
> static struct drm_crtc_helper_funcs intel_helper_funcs = {
> .dpms = intel_crtc_dpms,
> .mode_fixup = intel_crtc_mode_fixup,
> @@ -7643,8 +7687,27 @@ static const struct drm_crtc_funcs intel_crtc_funcs = {
> .set_config = drm_crtc_helper_set_config,
> .destroy = intel_crtc_destroy,
> .page_flip = intel_crtc_page_flip,
> + .set_property = intel_crtc_set_property,
> };
>
> +static void intel_attach_rotation_property(struct drm_crtc *crtc)
> +{
> + struct drm_device *dev = crtc->dev;
> + struct drm_i915_private *dev_priv = dev->dev_private;
> + struct drm_property *prop;
> +
> + prop = dev_priv->rotation_property;
> + if (prop == NULL) {
> + prop = drm_property_create_range(dev, 0, "rotation", 0, 359);
> + if (prop == NULL)
> + return;
> +
> + dev_priv->rotation_property = prop;
> + }
> +
> + drm_crtc_attach_property(crtc, prop, 0);
> +}
> +
> static void intel_crtc_init(struct drm_device *dev, int pipe)
> {
> drm_i915_private_t *dev_priv = dev->dev_private;
> @@ -7664,6 +7727,9 @@ static void intel_crtc_init(struct drm_device *dev, int pipe)
> intel_crtc->lut_b[i] = i;
> }
>
> + if (INTEL_INFO(dev)->gen>= 5)
> + intel_attach_rotation_property(&intel_crtc->base);
> +
> /* Swap pipes& planes for FBC on pre-965 */
> intel_crtc->pipe = pipe;
> intel_crtc->plane = pipe;
More information about the dri-devel
mailing list