drm/plane-helper: Add drm_plane_helper_check_update() (v3)

Pull the parameter checking from drm_primary_helper_update() out into
its own function; drivers that provide their own setplane()
implementations rather than using the helper may still want to share
this parameter checking logic.

A few of the checks here were also updated based on suggestions by
Ville Syrjälä.

v3:
 - s/primary_helper/plane_helper/ --- this checking logic may be useful
   for other types of planes as well.
 - Fix visibility check (need to dereference visibility pointer)
v2:
 - Pass src/dest/clip rects and min/max scaling down to helper to avoid
   duplication of effort between helper and drivers (suggested by
   Ville).
 - Allow caller to specify whether the primary plane should be
   updatable while the crtc is disabled.

Cc: dri-devel@lists.freedesktop.org
Reviewed-by: Chon Ming Lee <chon.ming.lee@intel.com>
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
Acked-by: Dave Airlie <airlied@gmail.com>
[danvet: Include header properly and fixup declaration mismatch to
make this compile.]
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

diff --git a/drivers/gpu/drm/drm_plane_helper.c b/drivers/gpu/drm/drm_plane_helper.c
index 37b9333cee49c6c5ef2359ea95e884e435d4798f..6d133149cc74873fd593aff0ae1aaa9362c29c31 100644 (file)
--- a/drivers/gpu/drm/drm_plane_helper.c
+++ b/drivers/gpu/drm/drm_plane_helper.c
@@ -25,6 +25,7 @@
 
 #include <linux/list.h>
 #include <drm/drmP.h>
+#include <drm/drm_plane_helper.h>
 #include <drm/drm_rect.h>
 #include <drm/drm_plane_helper.h>
 
@@ -73,6 +74,79 @@ static int get_connectors_for_crtc(struct drm_crtc *crtc,
        return count;
 }
 
+/**
+ * drm_plane_helper_check_update() - Check plane update for validity
+ * @plane: plane object to update
+ * @crtc: owning CRTC of owning plane
+ * @fb: framebuffer to flip onto plane
+ * @src: source coordinates in 16.16 fixed point
+ * @dest: integer destination coordinates
+ * @clip: integer clipping coordinates
+ * @min_scale: minimum @src:@dest scaling factor in 16.16 fixed point
+ * @max_scale: maximum @src:@dest scaling factor in 16.16 fixed point
+ * @can_position: is it legal to position the plane such that it
+ *                doesn't cover the entire crtc?  This will generally
+ *                only be false for primary planes.
+ * @can_update_disabled: can the plane be updated while the crtc
+ *                       is disabled?
+ * @visible: output parameter indicating whether plane is still visible after
+ *           clipping
+ *
+ * Checks that a desired plane update is valid.  Drivers that provide
+ * their own plane handling rather than helper-provided implementations may
+ * still wish to call this function to avoid duplication of error checking
+ * code.
+ *
+ * RETURNS:
+ * Zero if update appears valid, error code on failure
+ */
+int drm_plane_helper_check_update(struct drm_plane *plane,
+                                   struct drm_crtc *crtc,
+                                   struct drm_framebuffer *fb,
+                                   struct drm_rect *src,
+                                   struct drm_rect *dest,
+                                   const struct drm_rect *clip,
+                                   int min_scale,
+                                   int max_scale,
+                                   bool can_position,
+                                   bool can_update_disabled,
+                                   bool *visible)
+{
+       int hscale, vscale;
+
+       if (!crtc->enabled && !can_update_disabled) {
+               DRM_DEBUG_KMS("Cannot update plane of a disabled CRTC.\n");
+               return -EINVAL;
+       }
+
+       /* Check scaling */
+       hscale = drm_rect_calc_hscale(src, dest, min_scale, max_scale);
+       vscale = drm_rect_calc_vscale(src, dest, min_scale, max_scale);
+       if (hscale < 0 || vscale < 0) {
+               DRM_DEBUG_KMS("Invalid scaling of plane\n");
+               return -ERANGE;
+       }
+
+       *visible = drm_rect_clip_scaled(src, dest, clip, hscale, vscale);
+       if (!*visible)
+               /*
+                * Plane isn't visible; some drivers can handle this
+                * so we just return success here.  Drivers that can't
+                * (including those that use the primary plane helper's
+                * update function) will return an error from their
+                * update_plane handler.
+                */
+               return 0;
+
+       if (!can_position && !drm_rect_equals(dest, clip)) {
+               DRM_DEBUG_KMS("Plane must cover entire CRTC\n");
+               return -EINVAL;
+       }
+
+       return 0;
+}
+EXPORT_SYMBOL(drm_plane_helper_check_update);
+
 /**
  * drm_primary_helper_update() - Helper for primary plane update
  * @plane: plane object to update
@@ -121,51 +195,42 @@ int drm_primary_helper_update(struct drm_plane *plane, struct drm_crtc *crtc,
                .x = src_x >> 16,
                .y = src_y >> 16,
        };
+       struct drm_rect src = {
+               .x1 = src_x,
+               .y1 = src_y,
+               .x2 = src_x + src_w,
+               .y2 = src_y + src_h,
+       };
        struct drm_rect dest = {
                .x1 = crtc_x,
                .y1 = crtc_y,
                .x2 = crtc_x + crtc_w,
                .y2 = crtc_y + crtc_h,
        };
-       struct drm_rect clip = {
+       const struct drm_rect clip = {
                .x2 = crtc->mode.hdisplay,
                .y2 = crtc->mode.vdisplay,
        };
        struct drm_connector **connector_list;
        int num_connectors, ret;
+       bool visible;
 
-       if (!crtc->enabled) {
-               DRM_DEBUG_KMS("Cannot update primary plane of a disabled CRTC.\n");
-               return -EINVAL;
-       }
-
-       /* Disallow subpixel positioning */
-       if ((src_x | src_y | src_w | src_h) & SUBPIXEL_MASK) {
-               DRM_DEBUG_KMS("Primary plane does not support subpixel positioning\n");
-               return -EINVAL;
-       }
-
-       /* Disallow scaling */
-       src_w >>= 16;
-       src_h >>= 16;
-       if (crtc_w != src_w || crtc_h != src_h) {
-               DRM_DEBUG_KMS("Can't scale primary plane\n");
-               return -EINVAL;
-       }
-
-       /* Make sure primary plane covers entire CRTC */
-       drm_rect_intersect(&dest, &clip);
-       if (dest.x1 != 0 || dest.y1 != 0 ||
-           dest.x2 != crtc->mode.hdisplay || dest.y2 != crtc->mode.vdisplay) {
-               DRM_DEBUG_KMS("Primary plane must cover entire CRTC\n");
-               return -EINVAL;
-       }
-
-       /* Framebuffer must be big enough to cover entire plane */
-       ret = drm_crtc_check_viewport(crtc, crtc_x, crtc_y, &crtc->mode, fb);
+       ret = drm_plane_helper_check_update(plane, crtc, fb,
+                                           &src, &dest, &clip,
+                                           DRM_PLANE_HELPER_NO_SCALING,
+                                           DRM_PLANE_HELPER_NO_SCALING,
+                                           false, false, &visible);
        if (ret)
                return ret;
 
+       if (!visible)
+               /*
+                * Primary plane isn't visible.  Note that unless a driver
+                * provides their own disable function, this will just
+                * wind up returning -EINVAL to userspace.
+                */
+               return plane->funcs->disable_plane(plane);
+
        /* Find current connectors for CRTC */
        num_connectors = get_connectors_for_crtc(crtc, NULL, 0);
        BUG_ON(num_connectors == 0);

diff --git a/include/drm/drm_plane_helper.h b/include/drm/drm_plane_helper.h
index c5e7ab9503c822805469f396d348d175dea343b2..52e6870534b24ad90bec35526d23a4dda690765d 100644 (file)
--- a/include/drm/drm_plane_helper.h
+++ b/include/drm/drm_plane_helper.h
@@ -24,6 +24,17 @@
 #ifndef DRM_PLANE_HELPER_H
 #define DRM_PLANE_HELPER_H
 
+#include <drm/drm_rect.h>
+
+/*
+ * Drivers that don't allow primary plane scaling may pass this macro in place
+ * of the min/max scale parameters of the update checker function.
+ *
+ * Due to src being in 16.16 fixed point and dest being in integer pixels,
+ * 1<<16 represents no scaling.
+ */
+#define DRM_PLANE_HELPER_NO_SCALING (1<<16)
+
 /**
  * DOC: plane helpers
  *
@@ -31,6 +42,17 @@
  * planes.
  */
 
+extern int drm_plane_helper_check_update(struct drm_plane *plane,
+                                        struct drm_crtc *crtc,
+                                        struct drm_framebuffer *fb,
+                                        struct drm_rect *src,
+                                        struct drm_rect *dest,
+                                        const struct drm_rect *clip,
+                                        int min_scale,
+                                        int max_scale,
+                                        bool can_position,
+                                        bool can_update_disabled,
+                                        bool *visible);
 extern int drm_primary_helper_update(struct drm_plane *plane,
                                     struct drm_crtc *crtc,
                                     struct drm_framebuffer *fb,