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 Zanonipaulo.r.zanoni@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 Zanonipaulo.r.zanoni@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;