Re: [PATCH] drm/i915: Add extra plane information in debugfs.

[Robert Fekete <robert.fekete@linux.intel.com>]

On mån, 2015-10-26 at 17:40 +0200, Ville Syrjälä wrote:
> On Mon, Oct 26, 2015 at 03:56:33PM +0100, Robert Fekete wrote:
> > Hi, thanks for reviewing Maarten.
> > See comments inline...
> > 
> > On mån, 2015-10-26 at 09:23 +0100, Maarten Lankhorst wrote:
> > > Op 23-10-15 om 16:24 schreef Robert Fekete:
> > > > Extends i915_display_info so that for each active crtc also print
> > > > all planes associated with the pipe. This patch shows information
> > > > about each plane wrt format, size, position, rotation, and scaling.
> > > > This is very useful when debugging user space compositors that try
> > > > to utilize several planes for a commit.
> > > >
> > > > Signed-off-by: Robert Fekete <robert.fekete@linux.intel.com>
...

> > > I'm not sure that this is correct, and doing rotation = 180 + REFLECT_X + REFLECT_Y is allowed.
> > > 
> > > This would result in a plane with a normal orientation, but that would trigger a MISSING_CASE.
> > 
> > Yepp I misunderstood the value of DRM_ROTATE_xxx. It is a bit-position
> > intended to be used with BIT() macro and bitops. I'll rewrite this
> > function into printing the bits set to see if weird values may enter
> > into the rotation value. Above version is indeed wrong. Thanks for
> > noticing.
> > 
> > 
> > Although I may need an explanation in what is a valid rotation value.
> > This is how I get it.
> > 
> > DRM_ROTATE_0:    00000001 (1 << 0)
> > DRM_ROTATE_90:   00000010 (1 << 1)
> > DRM_ROTATE_180:  00000100 (1 << 2)
> > DRM_ROTATE_270:  00001000 (1 << 3)
> > DRM_REFLECT_X:   00010000 (1 << 4)
> > DRM_REFLECT_Y:   00100000 (1 << 5)
> > 
> > DRM_REFLECT_MASK 11110000
> > DRM_ROTATE_MASK  00001111
> > 
> > I assume 00010010 to be a valid rotation (ROTATE_90 and a REFLECT_X)
> > OTOH 00000110 (ROTATE_90 and ROTATE_180) is wrong right?
> 
> Yes, only one angle can be specified.

OK,

> > But it could
> > also be interpreted as ROTATE_270 if it is an accumulative rotation.
> > Also with this notion you will have two different rotation values for
> > the same actual rotation value. 
> > 
> > Also what confuses me with this setup (IIUC) is that different values
> > will in fact ultimately mean the same thing.
> > 00110001 (FLIP X and Y and ROTATE_0) is rot_180
> > 00000100 (ROTATE_180)
> > 
> > 00110100 (FLIP X and Y and ROTATE_180) is rot_0
> > 00000001 (ROTATE_0)
> > plus some more examples
> 
> Yes, all those are valid. drm_rotation_simplify() tries to use such
> identities to eliminate unsupported angles/reflections, but currently
> it's only meant as a helper for drivers.

Ok,

> > 
> > Regarding if rotation is CW or CCW? I cant figure that out in drm_crtc.h
> > where it is defined. OTOH in intel_display.c I can see a comment that
> > DRM_ROTATE_ is indeed CCW in order to be compatible with XRandr. I guess
> > that is one way to documentat things, or would perhaps a clockwise bit
> > be useful
> > DRM_ROTATE_CW:   01000000 (1 << 6)
> > AND a comment that default is CCW. Or at least a comment in drm_crtc.h
> > like: /* DRM_ROTATE_ is counter clockwise */
> > 
> > Finally for my better understanding....
> > If you for example do a REFLECT_X and a ROT_270 the order in which it is
> > applied matters. Which order is default? quite important for user-space
> > to know when manipulating these bits. I can't find any help in libdrm
> > either.
> > so what I mean.
> > (First REFLECT_X then ROTATE_270) != (First ROTATE_270 then REFLECT_X)
> 
> The rotation property documentation has the details, but repeating it
> next to the DRM_ROTATE bits probably wouldn't hurt.

Yepp, saw that under drm-kms-properties now. 

-- 
BR
/Robert Fekete
Intel Open Source Technology Center

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx