On Tue, Jun 21, 2011 at 6:21 AM, Rob Clark robdclark@gmail.com wrote:
On Mon, Jun 20, 2011 at 3:11 PM, Jesse Barnes jbarnes@virtuousgeek.org wrote:
This version adds both source and dest rect params to the set_plane ioctl, and makes the source fixed point to support hardware that needs it.
I haven't changed the name of the SNB implementation yet (per Chris's suggestions) but will before it gets upstream.
I'd be interested to see whether these interfaces will work for other hardware, so please take a close look at them and ideally implement them on your hardware to make sure (see my userspace example code from earlier posts if you want something to crib from).
Cool, thanks for this
I'm just thinking through how I'd implement the driver part in omap drm driver.. so please bear with me if I'm misunderstanding..
In particular I'm thinking about being able to use a given video pipe (basically like a dma channel) as either framebuffer layer or overlay at various points in time, depending on how many displays are attached. Is the idea to use drm_plane *only* for overlay layer, and still use crtc->fb for the normal framebuffer layer?
Or would/could a drm_plane also be used for main layer instead of crtc->fb? In this case, either userspace would have to know (which doesn't seem like a good idea for things like plymouth which use drm interface a bit generically). Or the driver would have to internally automatically hook up a drm_plane if it sees that userspace hasn't done this.
I was thinking perhaps that if we let userspace DRM_IOCTL_MODE_SETCRTC pass in -1 for fb_id, followed by one or more DRM_IOCTL_MODE_SETPLANE's, to set things up the "new" way (explicitly specify the drm_plane's). Or if _SETCRTC passes in a valid fb_id, we know it is the old way, and driver automatically picks a drm_plane.
just thinking through this a bit more.. the idea is that for drivers w/ DRM_SUPPORTS_PLANES[1], there is no fb hooked directly to crtc. And support for userspace that is not aware of planes could be, I think, mostly in the kms core, although I'm just thinking through how disruptive that would be..
I think if we add a crtc->funcs->get_primary_plane() to be implemented by drivers that use planes in cases where userspace does not know about planes. It would be used to get (and if necessary, assign) a drm_plane to a drm_crtc, then we can do things like:
handle _SETCRTC from userspace that doesn't understand planes: --------- @@ -1580,7 +1580,14 @@ int drm_mode_setcrtc(struct drm_device *dev, void *data, set.mode = mode; set.connectors = connector_set; set.num_connectors = crtc_req->count_connectors; - set.fb = fb; + if (fb & (dev->driver->driver_features & DRM_SUPPORTS_PLANES)) { + /* pass fb to the primary plane for compatibility */ + struct drm_plane *plane = + crtc->funcs->get_primary_plane(crtc); + set.plane = plane; + plane->funcs->set_config(&set); + } else { + set.fb = fb; + } ret = crtc->funcs->set_config(&set);
out: ---------
or page flip ioctl from userspace that doesn't understand planes:
--------- @@ -2645,7 +2652,14 @@ int drm_mode_page_flip_ioctl(struct drm_device *dev, (void (*) (struct drm_pending_event *)) kfree; }
- ret = crtc->funcs->page_flip(crtc, fb, e); + if (dev->driver->driver_features & DRM_SUPPORTS_PLANES) { + struct drm_plane *plane = + crtc->funcs->get_primary_plane(crtc); + ret = plane->funcs->page_flip(plane, fb, e); + } else { + ret = crtc->funcs->page_flip(crtc, fb, e); + } + if (ret) { spin_lock_irqsave(&dev->event_lock, flags); file_priv->event_space += sizeof e->event; ---------
It doesn't look like there are *that* many places where crtc->fb is reference, but I've not gone through all of them.. I'm not sure if this sort of change would be too intrusive.
BR, -R
-- [1] or maybe DRM_MANDATORY_PLANES if there are some drivers that would use a hybrid model of fb either direct to drm_crtc, or indirect via a drm_plane
BR, -R