On Mon, May 19, 2014 at 04:58:39PM -0700, Matt Roper wrote:
> If drivers support universal planes and have registered a cursor plane
> with the DRM core, we should use that universal plane support when
> handling legacy cursor ioctls. Drivers that transition to universal
> planes won't have to maintain separate legacy ioctl handling; drivers
> that don't transition to universal planes will continue to operate
> without any change to behavior.
>
> Note that there's a bit of a mismatch between the legacy cursor ioctls
> and the universal plane API's --- legacy ioctl's use driver buffer
> handles directly whereas the universal plane API takes drm_framebuffers.
> Since there's no way to recover the driver handle from a
> drm_framebuffer, we can implement legacy ioctl's in terms of universal
> plane interfaces, but cannot implement universal plane interfaces in
> terms of legacy ioctls. Specifically, there's no way to create a
> general cursor helper in the way we previously created a primary plane
> helper.
>
> It's important to land this patch before any patches that add universal
> cursor support to individual drivers so that drivers don't have to worry
> about juggling two different styles of reference counting for cursor
> buffers when userspace mixes and matches legacy and universal cursor
> calls. With this patch, a driver that switches to universal cursor
> support may assume that all cursor buffers are wrapped in a
> drm_framebuffer and can rely on framebuffer reference counting for all
> cursor operations.
>
> v3:
> - Drop drm_mode_rmfb() call that is no longer needed now that we're
> using setplane_internal(), which takes care of deref'ing the
> appropriate framebuffer.
> v2:
> - Use new add_framebuffer_internal() function to create framebuffer
> rather than trying to call directly into the ioctl interface and
> look up the handle returned.
> - Use new setplane_internal() function to update the cursor plane
> rather than calling through the ioctl interface. Note that since
> we're no longer looking up an fb_id, no extra reference will be
> taken here.
> - Grab extra reference to fb under lock in !BO case to avoid issues
> where racing userspace could cause the fb to be destroyed out from
> under us after we grab the fb pointer.
>
> Signed-off-by: Matt Roper <matthew.d.roper@xxxxxxxxx>
Reviewed-by: Daniel Vetter <daniel.vetter@xxxxxxxx>
Maybe add a comment that setplane_internal will eat the reference, but
optional.
-Daniel
> ---
> drivers/gpu/drm/drm_crtc.c | 99 ++++++++++++++++++++++++++++++++++++++++++++++
> include/drm/drm_crtc.h | 4 ++
> 2 files changed, 103 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index 3e9dae0..a3ebc45 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -2516,6 +2516,98 @@ out:
> return ret;
> }
>
> +/**
> + * drm_mode_cursor_universal - translate legacy cursor ioctl call into a
> + * universal plane handler call
> + * @crtc: crtc to update cursor for
> + * @req: data pointer for the ioctl
> + * @file_priv: drm file for the ioctl call
> + *
> + * Legacy cursor ioctl's work directly with driver buffer handles. To
> + * translate legacy ioctl calls into universal plane handler calls, we need to
> + * wrap the native buffer handle in a drm_framebuffer.
> + *
> + * Note that we assume any handle passed to the legacy ioctls was a 32-bit ARGB
> + * buffer with a pitch of 4*width; the universal plane interface should be used
> + * directly in cases where the hardware can support other buffer settings and
> + * userspace wants to make use of these capabilities.
> + *
> + * Returns:
> + * Zero on success, errno on failure.
> + */
> +static int drm_mode_cursor_universal(struct drm_crtc *crtc,
> + struct drm_mode_cursor2 *req,
> + struct drm_file *file_priv)
> +{
> + struct drm_device *dev = crtc->dev;
> + struct drm_framebuffer *fb = NULL;
> + struct drm_mode_fb_cmd2 fbreq = {
> + .width = req->width,
> + .height = req->height,
> + .pixel_format = DRM_FORMAT_ARGB8888,
> + .pitches = { req->width * 4 },
> + .handles = { req->handle },
> + };
> + int32_t crtc_x, crtc_y;
> + uint32_t crtc_w = 0, crtc_h = 0;
> + uint32_t src_w = 0, src_h = 0;
> + int ret = 0;
> +
> + BUG_ON(!crtc->cursor);
> +
> + /*
> + * Obtain fb we'll be using (either new or existing) and take an extra
> + * reference to it if fb != null. setplane will take care of dropping
> + * the reference if the plane update fails.
> + */
> + if (req->flags & DRM_MODE_CURSOR_BO) {
> + if (req->handle) {
> + fb = add_framebuffer_internal(dev, &fbreq, file_priv);
> + if (IS_ERR(fb)) {
> + DRM_DEBUG_KMS("failed to wrap cursor buffer in drm framebuffer\n");
> + return PTR_ERR(fb);
> + }
> +
> + drm_framebuffer_reference(fb);
> + } else {
> + fb = NULL;
> + }
> + } else {
> + mutex_lock(&dev->mode_config.mutex);
> + fb = crtc->cursor->fb;
> + if (fb)
> + drm_framebuffer_reference(fb);
> + mutex_unlock(&dev->mode_config.mutex);
> + }
> +
> + if (req->flags & DRM_MODE_CURSOR_MOVE) {
> + crtc_x = req->x;
> + crtc_y = req->y;
> + } else {
> + crtc_x = crtc->cursor_x;
> + crtc_y = crtc->cursor_y;
> + }
> +
> + if (fb) {
> + crtc_w = fb->width;
> + crtc_h = fb->height;
> + src_w = fb->width << 16;
> + src_h = fb->height << 16;
> + }
> +
> + ret = setplane_internal(crtc, crtc->cursor, fb,
> + crtc_x, crtc_y, crtc_w, crtc_h,
> + 0, 0, src_w, src_h);
> +
> + /* Update successful; save new cursor position, if necessary */
> + if (ret == 0 && req->flags & DRM_MODE_CURSOR_MOVE) {
> + crtc->cursor_x = req->x;
> + crtc->cursor_y = req->y;
> + }
> +
> + return ret;
> +}
> +
> static int drm_mode_cursor_common(struct drm_device *dev,
> struct drm_mode_cursor2 *req,
> struct drm_file *file_priv)
> @@ -2537,6 +2629,13 @@ static int drm_mode_cursor_common(struct drm_device *dev,
> }
> crtc = obj_to_crtc(obj);
>
> + /*
> + * If this crtc has a universal cursor plane, call that plane's update
> + * handler rather than using legacy cursor handlers.
> + */
> + if (crtc->cursor)
> + return drm_mode_cursor_universal(crtc, req, file_priv);
> +
> mutex_lock(&crtc->mutex);
> if (req->flags & DRM_MODE_CURSOR_BO) {
> if (!crtc->funcs->cursor_set && !crtc->funcs->cursor_set2) {
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index 5c1c31c..9ed4d1f 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -311,6 +311,10 @@ struct drm_crtc {
> struct drm_plane *primary;
> struct drm_plane *cursor;
>
> + /* position of cursor plane on crtc */
> + int cursor_x;
> + int cursor_y;
> +
> /* Temporary tracking of the old fb while a modeset is ongoing. Used
> * by drm_mode_set_config_internal to implement correct refcounting. */
> struct drm_framebuffer *old_fb;
> --
> 1.8.5.1
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
--
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
