On Tue, Mar 18, 2014 at 05:22:48PM -0700, Matt Roper wrote:
When we expose non-overlay planes to userspace, they will become accessible via standard userspace plane API's. We should be able to handle the standard plane operations against primary planes in a generic way via the page flip handler and modeset handler.
Drivers that can program primary planes more efficiently, that want to use their own primary plane structure to track additional information, or that don't have the limitations assumed by the helpers are free to provide their own implementation of some or all of these handlers.
Signed-off-by: Matt Roper matthew.d.roper@intel.com
One more below ...
+/**
- drm_primary_helper_disable() - Helper for primary plane disable
- @plane: plane to disable
- Provides a default plane disable handler for primary planes. This is handler
- is called in response to a userspace SetPlane operation on the plane with a
- NULL framebuffer parameter. We call the driver's modeset handler with a NULL
- framebuffer to disable the CRTC.
- Note that some hardware may be able to disable the primary plane without
- disabling the whole CRTC. Drivers for such hardware should provide their
- own disable handler that disables just the primary plane (and they'll likely
- need to provide their own update handler as well to properly re-enable a
- disabled primary plane).
- RETURNS:
- Zero on success, error code on failure
- */
+int drm_primary_helper_disable(struct drm_plane *plane) +{
- struct drm_mode_set set = {
.crtc = plane->crtc,.fb = NULL,- };
- if (plane->crtc == NULL || plane->fb == NULL)
/* Already disabled */return 0;
I think we should have a check here if any other plane is enabled (including the cursor plane), and fail the plane disabling with -EBUSY. Otherwise new userspace has no way to figure out whether the driver is updated already or not.
- return plane->crtc->funcs->set_config(&set);
Again I think you need to use set_config_internal to have correct fb refcounting. -Daniel