drm: Refactor setplane to allow internal use (v3)

Refactor DRM setplane code into a new setplane_internal() function that
takes DRM objects directly as parameters rather than looking them up by
ID.  We'll use this in a future patch when we implement legacy cursor
ioctls on top of the universal plane interface.

v3:
 - Move integer overflow checking from setplane_internal to setplane
   ioctl.  The upcoming legacy cursor support via universal planes needs
   to maintain current cursor ioctl semantics and not return error for
   these extreme values (found via intel-gpu-tools kms_cursor_crc test).
v2:
 - Allow planes to be disabled without a valid crtc again (and add
   mention of this to setplane's kerneldoc, since it doesn't seem to be
   mentioned anywhere else).
 - Reformat some parameter line wrap

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
Reviewed-by: Pallavi G<pallavi.g@intel.com>
Acked-by: Dave Airlie <airlied@linux.ie>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 5a88267fa8dd6597b307c550612f38a682ea9b52..27eae03bd38eea96bb46c5e39bd36159a6efeb1e 100644 (file)
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -2122,45 +2122,32 @@ out:
        return ret;
 }
 
-/**
- * drm_mode_setplane - configure a plane's configuration
- * @dev: DRM device
- * @data: ioctl data*
- * @file_priv: DRM file info
+/*
+ * setplane_internal - setplane handler for internal callers
  *
- * Set plane configuration, including placement, fb, scaling, and other factors.
- * Or pass a NULL fb to disable.
+ * Note that we assume an extra reference has already been taken on fb.  If the
+ * update fails, this reference will be dropped before return; if it succeeds,
+ * the previous framebuffer (if any) will be unreferenced instead.
  *
- * Returns:
- * Zero on success, errno on failure.
+ * src_{x,y,w,h} are provided in 16.16 fixed point format
  */
-int drm_mode_setplane(struct drm_device *dev, void *data,
-                     struct drm_file *file_priv)
+static int setplane_internal(struct drm_crtc *crtc,
+                            struct drm_plane *plane,
+                            struct drm_framebuffer *fb,
+                            int32_t crtc_x, int32_t crtc_y,
+                            uint32_t crtc_w, uint32_t crtc_h,
+                            /* src_{x,y,w,h} values are 16.16 fixed point */
+                            uint32_t src_x, uint32_t src_y,
+                            uint32_t src_w, uint32_t src_h)
 {
-       struct drm_mode_set_plane *plane_req = data;
-       struct drm_plane *plane;
-       struct drm_crtc *crtc;
-       struct drm_framebuffer *fb = NULL, *old_fb = NULL;
+       struct drm_device *dev = crtc->dev;
+       struct drm_framebuffer *old_fb = NULL;
        int ret = 0;
        unsigned int fb_width, fb_height;
        int i;
 
-       if (!drm_core_check_feature(dev, DRIVER_MODESET))
-               return -EINVAL;
-
-       /*
-        * First, find the plane, crtc, and fb objects.  If not available,
-        * we don't bother to call the driver.
-        */
-       plane = drm_plane_find(dev, plane_req->plane_id);
-       if (!plane) {
-               DRM_DEBUG_KMS("Unknown plane ID %d\n",
-                             plane_req->plane_id);
-               return -ENOENT;
-       }
-
        /* No fb means shut it down */
-       if (!plane_req->fb_id) {
+       if (!fb) {
                drm_modeset_lock_all(dev);
                old_fb = plane->fb;
                ret = plane->funcs->disable_plane(plane);
@@ -2174,14 +2161,6 @@ int drm_mode_setplane(struct drm_device *dev, void *data,
                goto out;
        }
 
-       crtc = drm_crtc_find(dev, plane_req->crtc_id);
-       if (!crtc) {
-               DRM_DEBUG_KMS("Unknown crtc ID %d\n",
-                             plane_req->crtc_id);
-               ret = -ENOENT;
-               goto out;
-       }
-
        /* Check whether this plane is usable on this CRTC */
        if (!(plane->possible_crtcs & drm_crtc_mask(crtc))) {
                DRM_DEBUG_KMS("Invalid crtc for plane\n");
@@ -2189,14 +2168,6 @@ int drm_mode_setplane(struct drm_device *dev, void *data,
                goto out;
        }
 
-       fb = drm_framebuffer_lookup(dev, plane_req->fb_id);
-       if (!fb) {
-               DRM_DEBUG_KMS("Unknown framebuffer ID %d\n",
-                             plane_req->fb_id);
-               ret = -ENOENT;
-               goto out;
-       }
-
        /* Check whether this plane supports the fb pixel format. */
        for (i = 0; i < plane->format_count; i++)
                if (fb->pixel_format == plane->format_types[i])
@@ -2212,43 +2183,25 @@ int drm_mode_setplane(struct drm_device *dev, void *data,
        fb_height = fb->height << 16;
 
        /* 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 ||
-           plane_req->src_h > fb_height ||
-           plane_req->src_y > fb_height - plane_req->src_h) {
+       if (src_w > fb_width ||
+           src_x > fb_width - src_w ||
+           src_h > fb_height ||
+           src_y > fb_height - src_h) {
                DRM_DEBUG_KMS("Invalid source coordinates "
                              "%u.%06ux%u.%06u+%u.%06u+%u.%06u\n",
-                             plane_req->src_w >> 16,
-                             ((plane_req->src_w & 0xffff) * 15625) >> 10,
-                             plane_req->src_h >> 16,
-                             ((plane_req->src_h & 0xffff) * 15625) >> 10,
-                             plane_req->src_x >> 16,
-                             ((plane_req->src_x & 0xffff) * 15625) >> 10,
-                             plane_req->src_y >> 16,
-                             ((plane_req->src_y & 0xffff) * 15625) >> 10);
+                             src_w >> 16, ((src_w & 0xffff) * 15625) >> 10,
+                             src_h >> 16, ((src_h & 0xffff) * 15625) >> 10,
+                             src_x >> 16, ((src_x & 0xffff) * 15625) >> 10,
+                             src_y >> 16, ((src_y & 0xffff) * 15625) >> 10);
                ret = -ENOSPC;
                goto out;
        }
 
-       /* Give drivers some help against integer overflows */
-       if (plane_req->crtc_w > INT_MAX ||
-           plane_req->crtc_x > INT_MAX - (int32_t) plane_req->crtc_w ||
-           plane_req->crtc_h > INT_MAX ||
-           plane_req->crtc_y > INT_MAX - (int32_t) plane_req->crtc_h) {
-               DRM_DEBUG_KMS("Invalid CRTC coordinates %ux%u+%d+%d\n",
-                             plane_req->crtc_w, plane_req->crtc_h,
-                             plane_req->crtc_x, plane_req->crtc_y);
-               ret = -ERANGE;
-               goto out;
-       }
-
        drm_modeset_lock_all(dev);
        old_fb = plane->fb;
        ret = plane->funcs->update_plane(plane, crtc, fb,
-                                        plane_req->crtc_x, plane_req->crtc_y,
-                                        plane_req->crtc_w, plane_req->crtc_h,
-                                        plane_req->src_x, plane_req->src_y,
-                                        plane_req->src_w, plane_req->src_h);
+                                        crtc_x, crtc_y, crtc_w, crtc_h,
+                                        src_x, src_y, src_w, src_h);
        if (!ret) {
                plane->crtc = crtc;
                plane->fb = fb;
@@ -2265,6 +2218,81 @@ out:
                drm_framebuffer_unreference(old_fb);
 
        return ret;
+
+}
+
+/**
+ * drm_mode_setplane - configure a plane's configuration
+ * @dev: DRM device
+ * @data: ioctl data*
+ * @file_priv: DRM file info
+ *
+ * Set plane configuration, including placement, fb, scaling, and other factors.
+ * Or pass a NULL fb to disable (planes may be disabled without providing a
+ * valid crtc).
+ *
+ * Returns:
+ * Zero on success, errno on failure.
+ */
+int drm_mode_setplane(struct drm_device *dev, void *data,
+                     struct drm_file *file_priv)
+{
+       struct drm_mode_set_plane *plane_req = data;
+       struct drm_mode_object *obj;
+       struct drm_plane *plane;
+       struct drm_crtc *crtc = NULL;
+       struct drm_framebuffer *fb = NULL;
+
+       if (!drm_core_check_feature(dev, DRIVER_MODESET))
+               return -EINVAL;
+
+       /* Give drivers some help against integer overflows */
+       if (plane_req->crtc_w > INT_MAX ||
+           plane_req->crtc_x > INT_MAX - (int32_t) plane_req->crtc_w ||
+           plane_req->crtc_h > INT_MAX ||
+           plane_req->crtc_y > INT_MAX - (int32_t) plane_req->crtc_h) {
+               DRM_DEBUG_KMS("Invalid CRTC coordinates %ux%u+%d+%d\n",
+                             plane_req->crtc_w, plane_req->crtc_h,
+                             plane_req->crtc_x, plane_req->crtc_y);
+               return -ERANGE;
+       }
+
+       /*
+        * First, find the plane, crtc, and fb objects.  If not available,
+        * we don't bother to call the driver.
+        */
+       obj = drm_mode_object_find(dev, plane_req->plane_id,
+                                  DRM_MODE_OBJECT_PLANE);
+       if (!obj) {
+               DRM_DEBUG_KMS("Unknown plane ID %d\n",
+                             plane_req->plane_id);
+               return -ENOENT;
+       }
+       plane = obj_to_plane(obj);
+
+       if (plane_req->fb_id) {
+               fb = drm_framebuffer_lookup(dev, plane_req->fb_id);
+               if (!fb) {
+                       DRM_DEBUG_KMS("Unknown framebuffer ID %d\n",
+                                     plane_req->fb_id);
+                       return -ENOENT;
+               }
+
+               obj = drm_mode_object_find(dev, plane_req->crtc_id,
+                                          DRM_MODE_OBJECT_CRTC);
+               if (!obj) {
+                       DRM_DEBUG_KMS("Unknown crtc ID %d\n",
+                                     plane_req->crtc_id);
+                       return -ENOENT;
+               }
+               crtc = obj_to_crtc(obj);
+       }
+
+       return setplane_internal(crtc, plane, fb,
+                                plane_req->crtc_x, plane_req->crtc_y,
+                                plane_req->crtc_w, plane_req->crtc_h,
+                                plane_req->src_x, plane_req->src_y,
+                                plane_req->src_w, plane_req->src_h);
 }
 
 /**