Re: [PATCH] staging: drm/omap: add rotation properties
On Fri, Jun 29, 2012 at 07:17:23AM -0500, Rob Clark wrote: On Fri, Jun 29, 2012 at 5:46 AM, Tomi Valkeinen tomi.valkei...@ti.com wrote: On Wed, 2012-06-27 at 09:06 -0500, Rob Clark wrote: From: Rob Clark r...@ti.com Use tiled buffers for rotated/reflected scanout, with CRTC and plane properties as the interface for userspace to configure rotation. Signed-off-by: Rob Clark r...@ti.com +/* this should probably be in drm-core to standardize amongst drivers */ +#define DRM_ROTATE_0 0 +#define DRM_ROTATE_90 1 +#define DRM_ROTATE_180 2 +#define DRM_ROTATE_270 3 +#define DRM_REFLECT_X 4 +#define DRM_REFLECT_Y 5 Are both reflect X and Y needed? You can get all the possible orientations with just one of the reflects. And I think the word mirror represents nicely what the reflect does, i.e. if you look at the mirror, the image you see is flipped horizontally. fwiw these values are aligned with what is used in userspace xrandr.. an earlier version of this patch used just 3 bits, which where aligned with what the omap hw uses and can describe all possible combinations of mirroring and isomorphic rotation (x-invert, y-invert, and xy-flip). But the advantage of the xrandr approach is you can more easily leave out bits for rotation/mirroring modes that your hw does not support.. for example if some hw supports only certain rotations or does not support mirror/reflect. When I hear mirror I always think of it as the happening after rotation, but that's not how xrandr does things. I suppose reflection has more or less the same connotation for me, but since it's already used by xrandr, I know I have to do the necessary mental gymnastics. If you think about it in terms of matrix multiplication, xrandr does things like this: x' = Rot * Ref * x, whereas for me the more natural order (for whatever reason) would be x' = Ref * Rot * x, where x is the source coordinates, and x' the destination coordinates. IIRC in dss mirroring was specified in the post-rotation way, and the direction of the rotation was also opposite to xrandr (cw in dss, ccw in xrandr). So FWIW, my vote goes to reflect if we want to match xrandr semantics. Also I agree on the number of bits, six is better than three since it allows you to describe the hw capabilities. -- Ville Syrjälä Intel OTC -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] staging: drm/omap: add rotation properties
On Wed, 2012-06-27 at 09:06 -0500, Rob Clark wrote: From: Rob Clark r...@ti.com Use tiled buffers for rotated/reflected scanout, with CRTC and plane properties as the interface for userspace to configure rotation. Signed-off-by: Rob Clark r...@ti.com +/* this should probably be in drm-core to standardize amongst drivers */ +#define DRM_ROTATE_0 0 +#define DRM_ROTATE_901 +#define DRM_ROTATE_180 2 +#define DRM_ROTATE_270 3 +#define DRM_REFLECT_X4 +#define DRM_REFLECT_Y5 Are both reflect X and Y needed? You can get all the possible orientations with just one of the reflects. And I think the word mirror represents nicely what the reflect does, i.e. if you look at the mirror, the image you see is flipped horizontally. Tomi signature.asc Description: This is a digitally signed message part
Re: [PATCH] staging: drm/omap: add rotation properties
On Fri, Jun 29, 2012 at 5:46 AM, Tomi Valkeinen tomi.valkei...@ti.com wrote: On Wed, 2012-06-27 at 09:06 -0500, Rob Clark wrote: From: Rob Clark r...@ti.com Use tiled buffers for rotated/reflected scanout, with CRTC and plane properties as the interface for userspace to configure rotation. Signed-off-by: Rob Clark r...@ti.com +/* this should probably be in drm-core to standardize amongst drivers */ +#define DRM_ROTATE_0 0 +#define DRM_ROTATE_90 1 +#define DRM_ROTATE_180 2 +#define DRM_ROTATE_270 3 +#define DRM_REFLECT_X 4 +#define DRM_REFLECT_Y 5 Are both reflect X and Y needed? You can get all the possible orientations with just one of the reflects. And I think the word mirror represents nicely what the reflect does, i.e. if you look at the mirror, the image you see is flipped horizontally. fwiw these values are aligned with what is used in userspace xrandr.. an earlier version of this patch used just 3 bits, which where aligned with what the omap hw uses and can describe all possible combinations of mirroring and isomorphic rotation (x-invert, y-invert, and xy-flip). But the advantage of the xrandr approach is you can more easily leave out bits for rotation/mirroring modes that your hw does not support.. for example if some hw supports only certain rotations or does not support mirror/reflect. BR, -R Tomi -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] staging: drm/omap: add rotation properties
On Wed, Jun 27, 2012 at 9:06 AM, Rob Clark rob.cl...@linaro.org wrote: From: Rob Clark r...@ti.com Use tiled buffers for rotated/reflected scanout, with CRTC and plane properties as the interface for userspace to configure rotation. btw, I assume the potential controversial part of the patch would be the property names/values exposed to userspace, because we probably should try to standardize property names/values wherever possible. There is one difference, AFAIU, from the rotation properties proposed for the intel driver, in that in the omap case these properties are actually instructing the hw to do the rotation, rather than just informing the driver that rotation was done via a shadow buffer in userspace. BR, -R Signed-off-by: Rob Clark r...@ti.com --- drivers/staging/omapdrm/omap_crtc.c | 17 + drivers/staging/omapdrm/omap_dmm_tiler.c | 47 -- drivers/staging/omapdrm/omap_dmm_tiler.h | 17 - drivers/staging/omapdrm/omap_drv.c | 17 + drivers/staging/omapdrm/omap_drv.h | 32 +- drivers/staging/omapdrm/omap_fb.c | 99 +- drivers/staging/omapdrm/omap_gem.c | 43 - drivers/staging/omapdrm/omap_plane.c | 92 --- 8 files changed, 318 insertions(+), 46 deletions(-) diff --git a/drivers/staging/omapdrm/omap_crtc.c b/drivers/staging/omapdrm/omap_crtc.c index 8b864af..7479dcb 100644 --- a/drivers/staging/omapdrm/omap_crtc.c +++ b/drivers/staging/omapdrm/omap_crtc.c @@ -191,10 +191,25 @@ static int omap_crtc_page_flip_locked(struct drm_crtc *crtc, return 0; } +static int omap_crtc_set_property(struct drm_crtc *crtc, + struct drm_property *property, uint64_t val) +{ + struct omap_crtc *omap_crtc = to_omap_crtc(crtc); + struct omap_drm_private *priv = crtc-dev-dev_private; + + if (property == priv-rotation_prop) { + crtc-invert_dimensions = + !!(val ((1LL DRM_ROTATE_90) | (1LL DRM_ROTATE_270))); + } + + return omap_plane_set_property(omap_crtc-plane, property, val); +} + static const struct drm_crtc_funcs omap_crtc_funcs = { .set_config = drm_crtc_helper_set_config, .destroy = omap_crtc_destroy, .page_flip = omap_crtc_page_flip_locked, + .set_property = omap_crtc_set_property, }; static const struct drm_crtc_helper_funcs omap_crtc_helper_funcs = { @@ -231,6 +246,8 @@ struct drm_crtc *omap_crtc_init(struct drm_device *dev, drm_crtc_init(dev, crtc, omap_crtc_funcs); drm_crtc_helper_add(crtc, omap_crtc_helper_funcs); + omap_plane_install_properties(omap_crtc-plane, crtc-base); + return crtc; fail: diff --git a/drivers/staging/omapdrm/omap_dmm_tiler.c b/drivers/staging/omapdrm/omap_dmm_tiler.c index 9d83060..68da290 100644 --- a/drivers/staging/omapdrm/omap_dmm_tiler.c +++ b/drivers/staging/omapdrm/omap_dmm_tiler.c @@ -401,8 +401,26 @@ int tiler_release(struct tiler_block *block) * Utils */ -/* calculate the tiler space address of a pixel in a view orientation */ -static u32 tiler_get_address(u32 orient, enum tiler_fmt fmt, u32 x, u32 y) +/* calculate the tiler space address of a pixel in a view orientation... + * below description copied from the display subsystem section of TRM: + * + * When the TILER is addressed, the bits: + * [28:27] = 0x0 for 8-bit tiled + * 0x1 for 16-bit tiled + * 0x2 for 32-bit tiled + * 0x3 for page mode + * [31:29] = 0x0 for 0-degree view + * 0x1 for 180-degree view + mirroring + * 0x2 for 0-degree view + mirroring + * 0x3 for 180-degree view + * 0x4 for 270-degree view + mirroring + * 0x5 for 270-degree view + * 0x6 for 90-degree view + * 0x7 for 90-degree view + mirroring + * Otherwise the bits indicated the corresponding bit address to access + * the SDRAM. + */ +static u32 tiler_get_address(enum tiler_fmt fmt, u32 orient, u32 x, u32 y) { u32 x_bits, y_bits, tmp, x_mask, y_mask, alignment; @@ -414,8 +432,11 @@ static u32 tiler_get_address(u32 orient, enum tiler_fmt fmt, u32 x, u32 y) x_mask = MASK(x_bits); y_mask = MASK(y_bits); - if (x 0 || x x_mask || y 0 || y y_mask) + if (x 0 || x x_mask || y 0 || y y_mask) { + DBG(invalid coords: %u 0 || %u %u || %u 0 || %u %u, + x, x, x_mask, y, y, y_mask); return 0; + } /* account for mirroring */ if (orient MASK_X_INVERT) @@ -436,11 +457,22 @@ dma_addr_t tiler_ssptr(struct tiler_block *block) { BUG_ON(!validfmt(block-fmt)); - return TILVIEW_8BIT + tiler_get_address(0, block-fmt, + return TILVIEW_8BIT + tiler_get_address(block-fmt, 0,
[PATCH] staging: drm/omap: add rotation properties
From: Rob Clark r...@ti.com Use tiled buffers for rotated/reflected scanout, with CRTC and plane properties as the interface for userspace to configure rotation. Signed-off-by: Rob Clark r...@ti.com --- drivers/staging/omapdrm/omap_crtc.c | 17 + drivers/staging/omapdrm/omap_dmm_tiler.c | 47 -- drivers/staging/omapdrm/omap_dmm_tiler.h | 17 - drivers/staging/omapdrm/omap_drv.c | 17 + drivers/staging/omapdrm/omap_drv.h | 32 +- drivers/staging/omapdrm/omap_fb.c| 99 +- drivers/staging/omapdrm/omap_gem.c | 43 - drivers/staging/omapdrm/omap_plane.c | 92 --- 8 files changed, 318 insertions(+), 46 deletions(-) diff --git a/drivers/staging/omapdrm/omap_crtc.c b/drivers/staging/omapdrm/omap_crtc.c index 8b864af..7479dcb 100644 --- a/drivers/staging/omapdrm/omap_crtc.c +++ b/drivers/staging/omapdrm/omap_crtc.c @@ -191,10 +191,25 @@ static int omap_crtc_page_flip_locked(struct drm_crtc *crtc, return 0; } +static int omap_crtc_set_property(struct drm_crtc *crtc, + struct drm_property *property, uint64_t val) +{ + struct omap_crtc *omap_crtc = to_omap_crtc(crtc); + struct omap_drm_private *priv = crtc-dev-dev_private; + + if (property == priv-rotation_prop) { + crtc-invert_dimensions = + !!(val ((1LL DRM_ROTATE_90) | (1LL DRM_ROTATE_270))); + } + + return omap_plane_set_property(omap_crtc-plane, property, val); +} + static const struct drm_crtc_funcs omap_crtc_funcs = { .set_config = drm_crtc_helper_set_config, .destroy = omap_crtc_destroy, .page_flip = omap_crtc_page_flip_locked, + .set_property = omap_crtc_set_property, }; static const struct drm_crtc_helper_funcs omap_crtc_helper_funcs = { @@ -231,6 +246,8 @@ struct drm_crtc *omap_crtc_init(struct drm_device *dev, drm_crtc_init(dev, crtc, omap_crtc_funcs); drm_crtc_helper_add(crtc, omap_crtc_helper_funcs); + omap_plane_install_properties(omap_crtc-plane, crtc-base); + return crtc; fail: diff --git a/drivers/staging/omapdrm/omap_dmm_tiler.c b/drivers/staging/omapdrm/omap_dmm_tiler.c index 9d83060..68da290 100644 --- a/drivers/staging/omapdrm/omap_dmm_tiler.c +++ b/drivers/staging/omapdrm/omap_dmm_tiler.c @@ -401,8 +401,26 @@ int tiler_release(struct tiler_block *block) * Utils */ -/* calculate the tiler space address of a pixel in a view orientation */ -static u32 tiler_get_address(u32 orient, enum tiler_fmt fmt, u32 x, u32 y) +/* calculate the tiler space address of a pixel in a view orientation... + * below description copied from the display subsystem section of TRM: + * + * When the TILER is addressed, the bits: + * [28:27] = 0x0 for 8-bit tiled + * 0x1 for 16-bit tiled + * 0x2 for 32-bit tiled + * 0x3 for page mode + * [31:29] = 0x0 for 0-degree view + * 0x1 for 180-degree view + mirroring + * 0x2 for 0-degree view + mirroring + * 0x3 for 180-degree view + * 0x4 for 270-degree view + mirroring + * 0x5 for 270-degree view + * 0x6 for 90-degree view + * 0x7 for 90-degree view + mirroring + * Otherwise the bits indicated the corresponding bit address to access + * the SDRAM. + */ +static u32 tiler_get_address(enum tiler_fmt fmt, u32 orient, u32 x, u32 y) { u32 x_bits, y_bits, tmp, x_mask, y_mask, alignment; @@ -414,8 +432,11 @@ static u32 tiler_get_address(u32 orient, enum tiler_fmt fmt, u32 x, u32 y) x_mask = MASK(x_bits); y_mask = MASK(y_bits); - if (x 0 || x x_mask || y 0 || y y_mask) + if (x 0 || x x_mask || y 0 || y y_mask) { + DBG(invalid coords: %u 0 || %u %u || %u 0 || %u %u, + x, x, x_mask, y, y, y_mask); return 0; + } /* account for mirroring */ if (orient MASK_X_INVERT) @@ -436,11 +457,22 @@ dma_addr_t tiler_ssptr(struct tiler_block *block) { BUG_ON(!validfmt(block-fmt)); - return TILVIEW_8BIT + tiler_get_address(0, block-fmt, + return TILVIEW_8BIT + tiler_get_address(block-fmt, 0, block-area.p0.x * geom[block-fmt].slot_w, block-area.p0.y * geom[block-fmt].slot_h); } +dma_addr_t tiler_tsptr(struct tiler_block *block, uint32_t orient, + uint32_t x, uint32_t y) +{ + struct tcm_pt *p = block-area.p0; + BUG_ON(!validfmt(block-fmt)); + + return tiler_get_address(block-fmt, orient, + (p-x * geom[block-fmt].slot_w) + x, + (p-y * geom[block-fmt].slot_h) + y); +} + void tiler_align(enum tiler_fmt fmt, uint16_t *w, uint16_t *h) { BUG_ON(!validfmt(fmt)); @@ -448,11 +480,14 @@ void tiler_align(enum tiler_fmt fmt,
Re: [PATCH] staging: drm/omap: add rotation properties
On Wed, Jun 27, 2012 at 09:06:26AM -0500, Rob Clark wrote: From: Rob Clark r...@ti.com Use tiled buffers for rotated/reflected scanout, with CRTC and plane properties as the interface for userspace to configure rotation. Signed-off-by: Rob Clark r...@ti.com --- drivers/staging/omapdrm/omap_crtc.c | 17 + drivers/staging/omapdrm/omap_dmm_tiler.c | 47 -- drivers/staging/omapdrm/omap_dmm_tiler.h | 17 - drivers/staging/omapdrm/omap_drv.c | 17 + drivers/staging/omapdrm/omap_drv.h | 32 +- drivers/staging/omapdrm/omap_fb.c| 99 +- drivers/staging/omapdrm/omap_gem.c | 43 - drivers/staging/omapdrm/omap_plane.c | 92 --- 8 files changed, 318 insertions(+), 46 deletions(-) That's great you are adding new features, but how goes the progress to get this driver out of the drivers/staging/ area and moved to the real part of the kernel? I don't want to keep taking new features without seeing some progress on getting the code cleaned up and moved out first. thanks, greg k-h -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] staging: drm/omap: add rotation properties
On Wed, Jun 27, 2012 at 2:40 PM, Greg KH g...@kroah.com wrote: On Wed, Jun 27, 2012 at 09:06:26AM -0500, Rob Clark wrote: From: Rob Clark r...@ti.com Use tiled buffers for rotated/reflected scanout, with CRTC and plane properties as the interface for userspace to configure rotation. Signed-off-by: Rob Clark r...@ti.com --- drivers/staging/omapdrm/omap_crtc.c | 17 + drivers/staging/omapdrm/omap_dmm_tiler.c | 47 -- drivers/staging/omapdrm/omap_dmm_tiler.h | 17 - drivers/staging/omapdrm/omap_drv.c | 17 + drivers/staging/omapdrm/omap_drv.h | 32 +- drivers/staging/omapdrm/omap_fb.c | 99 +- drivers/staging/omapdrm/omap_gem.c | 43 - drivers/staging/omapdrm/omap_plane.c | 92 --- 8 files changed, 318 insertions(+), 46 deletions(-) That's great you are adding new features, but how goes the progress to get this driver out of the drivers/staging/ area and moved to the real part of the kernel? I don't want to keep taking new features without seeing some progress on getting the code cleaned up and moved out first. Oh, heh, well I suppose the first thing to do is send a patch to clean up the TODO file.. I just noticed that I'd been forgetting to update it. BR, -R thanks, greg k-h -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html