On Sat, Mar 31, 2012 at 4:09 PM, Ville Syrjälä syrjala@sci.fi wrote:
On Fri, Mar 30, 2012 at 05:59:32PM -0500, Rob Clark wrote:
From: Rob Clark rob@ti.com
For drivers that can support rotated scanout, the extra parameter checking in drm-core, while nice, tends to get confused. To solve this drivers can set the crtc or plane invert_dimensions field so that the dimension checking takes into account the rotation that the driver is performing.
Note: RFC still mainly because I've only tested the CRTC rotation so far.. still need to write some test code for plane rotation.
drivers/gpu/drm/drm_crtc.c | 50 +++++++++++++++++++++++++++++-------------- include/drm/drm_crtc.h | 9 ++++++++ 2 files changed, 43 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index 0dff444..261c9bd 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -375,6 +375,7 @@ int drm_crtc_init(struct drm_device *dev, struct drm_crtc *crtc,
crtc->dev = dev; crtc->funcs = funcs;
- crtc->invert_dimensions = false;
mutex_lock(&dev->mode_config.mutex);
@@ -609,6 +610,7 @@ int drm_plane_init(struct drm_device *dev, struct drm_plane *plane, plane->base.properties = &plane->properties; plane->dev = dev; plane->funcs = funcs;
- plane->invert_dimensions = false;
plane->format_types = kmalloc(sizeof(uint32_t) * format_count, GFP_KERNEL); if (!plane->format_types) { @@ -1758,6 +1760,9 @@ int drm_mode_setplane(struct drm_device *dev, void *data, fb_width = fb->width << 16; fb_height = fb->height << 16;
- if (plane->invert_dimensions)
- swap(fb_width, fb_height);
In my opinion the only sane way to specify this stuff is that the source coordinates are not transformed in any way.
fwiw, it might be a bit odd that in one case I swapped fb dimensions, and in the other crtc dimensions.. although it was just laziness (there were fewer param's to swap that way ;-))
So you fetch the data from the fb based on the source coordinates, then apply all plane transformations (if any), and then apply all CRTC transformations (if any).
fwiw, in my case planes and CRTCs are rotated independently.. ie. rotating the CRTC doesn't automatically rotate the planes. But I include invert_dimensions flag in both drm_crtc and drm_plane so drivers for hw that works differently can do whatever is appropriate
/* Make sure source coordinates are inside the fb. */ if (plane_req->src_w > fb_width || plane_req->src_x > fb_width - plane_req->src_w || @@ -1856,6 +1861,7 @@ int drm_mode_setcrtc(struct drm_device *dev, void *data, DRM_DEBUG_KMS("[CRTC:%d]\n", crtc->base.id);
if (crtc_req->mode_valid) {
- int hdisplay, vdisplay;
/* If we have a mode we need a framebuffer. */ /* If we pass -1, set the mode with the currently bound fb */ if (crtc_req->fb_id == -1) { @@ -1891,14 +1897,20 @@ int drm_mode_setcrtc(struct drm_device *dev, void *data,
drm_mode_set_crtcinfo(mode, CRTC_INTERLACE_HALVE_V);
- if (mode->hdisplay > fb->width ||
- mode->vdisplay > fb->height ||
- crtc_req->x > fb->width - mode->hdisplay ||
- crtc_req->y > fb->height - mode->vdisplay) {
- DRM_DEBUG_KMS("Invalid CRTC viewport %ux%u+%u+%u for fb size %ux%u.\n",
- mode->hdisplay, mode->vdisplay,
- crtc_req->x, crtc_req->y,
- fb->width, fb->height);
- hdisplay = mode->hdisplay;
- vdisplay = mode->vdisplay;
- if (crtc->invert_dimensions)
- swap(hdisplay, vdisplay);
- if (hdisplay > fb->width ||
- vdisplay > fb->height ||
- crtc_req->x > fb->width - hdisplay ||
- crtc_req->y > fb->height - vdisplay) {
- DRM_DEBUG_KMS("Invalid fb size %ux%u for CRTC viewport %ux%u+%d+%d%s.\n",
- fb->width, fb->height,
- hdisplay, vdisplay, crtc_req->x, crtc_req->y,
- crtc->invert_dimensions ? " (inverted)" : "");
I would perhaps just stick more details about the transformations into drm_crtc, but we will anyway require a better mode setting API. So perhaps this is an OK intermediate solution.
I was trying to avoid putting full matrix transformation in the kernel ;-)
but at least the hw I'm familiar with is only doing simple isometric transformations in scanout (ie. multiples of 90 degress and/or horiz/vert mirroring). Anything more complex would require a shadow buffer and gpu, which is (I think) better to do in userspace. But I don't know if this is sufficient for other drivers or not.. part of the reason for the RFC ;-)
BR, -R
ret = -ENOSPC; goto out; }
-- Ville Syrjälä syrjala@sci.fi http://www.sci.fi/~syrjala/ _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel