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@xxxxxxxxx>
> ---
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
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel
