[Intel-gfx] [PATCH 2/3] drm/plane-helper: Add drm_primary_helper_check_update()
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ä.
Cc: Ville Syrjälä ville.syrj...@linux.intel.com
Cc: dri-de...@lists.freedesktop.org
Signed-off-by: Matt Roper matthew.d.ro...@intel.com
---
drivers/gpu/drm/drm_plane_helper.c | 148 +
include/drm/drm_plane_helper.h | 9 +++
2 files changed, 128 insertions(+), 29 deletions(-)
diff --git a/drivers/gpu/drm/drm_plane_helper.c
b/drivers/gpu/drm/drm_plane_helper.c
index 65c4a00..9bbbdf2 100644
--- a/drivers/gpu/drm/drm_plane_helper.c
+++ b/drivers/gpu/drm/drm_plane_helper.c
@@ -66,6 +66,102 @@ static int get_connectors_for_crtc(struct drm_crtc *crtc,
}
/**
+ * drm_primary_helper_check_update() - Check primary plane update for validity
+ * @plane: plane object to update
+ * @crtc: owning CRTC of owning plane
+ * @fb: framebuffer to flip onto plane
+ * @crtc_x: x offset of primary plane on crtc
+ * @crtc_y: y offset of primary plane on crtc
+ * @crtc_w: width of primary plane rectangle on crtc
+ * @crtc_h: height of primary plane rectangle on crtc
+ * @src_x: x offset of @fb for panning
+ * @src_y: y offset of @fb for panning
+ * @src_w: width of source rectangle in @fb
+ * @src_h: height of source rectangle in @fb
+ * @can_scale: is primary plane scaling legal?
+ * @can_position: is it legal to position the primary plane such that it
+ *doesn't cover the entire crtc?
+ *
+ * Checks that a desired primary plane update is valid. Drivers that provide
+ * their own primary plane handling 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_primary_helper_check_update(struct drm_plane *plane,
+ struct drm_crtc *crtc,
+ struct drm_framebuffer *fb,
+ int crtc_x, int crtc_y,
+ unsigned int crtc_w, unsigned int crtc_h,
+ uint32_t src_x, uint32_t src_y,
+ uint32_t src_w, uint32_t src_h,
+ bool can_scale,
+ bool can_position)
+{
+ struct drm_rect dest = {
+ .x1 = crtc_x,
+ .y1 = crtc_y,
+ .x2 = crtc_x + crtc_w,
+ .y2 = crtc_y + crtc_h,
+ };
+ struct drm_rect src = {
+ .x1 = src_x 16,
+ .y1 = src_y 16,
+ .x2 = (src_x + src_w) 16,
+ .y2 = (src_y + src_h) 16,
+ };
+ const struct drm_rect clip = {
+ .x2 = crtc-mode.hdisplay,
+ .y2 = crtc-mode.vdisplay,
+ };
+ int hscale, vscale;
+ int visible;
+
+ if (!crtc-enabled) {
+ DRM_DEBUG_KMS(Cannot update primary plane of a disabled
CRTC.\n);
+ return -EINVAL;
+ }
+
+ /* setplane API takes shifted source rectangle values; unshift them */
+ src_x = 16;
+ src_y = 16;
+ src_w = 16;
+ src_h = 16;
+
+ /* check scaling */
+ if (!can_scale (crtc_w != src_w || crtc_h != src_h)) {
+ DRM_DEBUG_KMS(Can't scale primary plane\n);
+ return -EINVAL;
+ }
+
+ /*
+* Drivers that can scale should perform their own min/max scale
+* factor checks.
+*/
+ hscale = drm_rect_calc_hscale(src, dest, 0, INT_MAX);
+ vscale = drm_rect_calc_vscale(src, dest, 0, INT_MAX);
+ visible = drm_rect_clip_scaled(src, dest, clip, hscale, vscale);
+ if (!visible)
+ /*
+* Primary 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(Primary plane must cover entire CRTC\n);
+ return -EINVAL;
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL(drm_primary_helper_check_update);
+
+/**
* drm_primary_helper_update() - Helper for primary plane update
* @plane: plane object to update
* @crtc: owning CRTC of owning plane
@@ -113,6 +209,12 @@ 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 16,
+ .y1 = src_y 16,
+
Re: [Intel-gfx] [PATCH 2/3] drm/plane-helper: Add drm_primary_helper_check_update()
On Wed, Apr 23, 2014 at 10:04:00AM -0700, Matt Roper wrote:
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ä.
Cc: Ville Syrjälä ville.syrj...@linux.intel.com
Cc: dri-de...@lists.freedesktop.org
Signed-off-by: Matt Roper matthew.d.ro...@intel.com
---
drivers/gpu/drm/drm_plane_helper.c | 148
+
include/drm/drm_plane_helper.h | 9 +++
2 files changed, 128 insertions(+), 29 deletions(-)
diff --git a/drivers/gpu/drm/drm_plane_helper.c
b/drivers/gpu/drm/drm_plane_helper.c
index 65c4a00..9bbbdf2 100644
--- a/drivers/gpu/drm/drm_plane_helper.c
+++ b/drivers/gpu/drm/drm_plane_helper.c
@@ -66,6 +66,102 @@ static int get_connectors_for_crtc(struct drm_crtc *crtc,
}
/**
+ * drm_primary_helper_check_update() - Check primary plane update for
validity
+ * @plane: plane object to update
+ * @crtc: owning CRTC of owning plane
+ * @fb: framebuffer to flip onto plane
+ * @crtc_x: x offset of primary plane on crtc
+ * @crtc_y: y offset of primary plane on crtc
+ * @crtc_w: width of primary plane rectangle on crtc
+ * @crtc_h: height of primary plane rectangle on crtc
+ * @src_x: x offset of @fb for panning
+ * @src_y: y offset of @fb for panning
+ * @src_w: width of source rectangle in @fb
+ * @src_h: height of source rectangle in @fb
+ * @can_scale: is primary plane scaling legal?
+ * @can_position: is it legal to position the primary plane such that it
+ *doesn't cover the entire crtc?
+ *
+ * Checks that a desired primary plane update is valid. Drivers that provide
+ * their own primary plane handling 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_primary_helper_check_update(struct drm_plane *plane,
+ struct drm_crtc *crtc,
+ struct drm_framebuffer *fb,
+ int crtc_x, int crtc_y,
+ unsigned int crtc_w, unsigned int crtc_h,
+ uint32_t src_x, uint32_t src_y,
+ uint32_t src_w, uint32_t src_h,
+ bool can_scale,
+ bool can_position)
+{
+ struct drm_rect dest = {
+ .x1 = crtc_x,
+ .y1 = crtc_y,
+ .x2 = crtc_x + crtc_w,
+ .y2 = crtc_y + crtc_h,
+ };
+ struct drm_rect src = {
+ .x1 = src_x 16,
+ .y1 = src_y 16,
+ .x2 = (src_x + src_w) 16,
+ .y2 = (src_y + src_h) 16,
I think you want '(x16) + (y16)' instead. Otherwise you may end up
increasing the source width/height.
+ };
+ const struct drm_rect clip = {
+ .x2 = crtc-mode.hdisplay,
+ .y2 = crtc-mode.vdisplay,
+ };
+ int hscale, vscale;
+ int visible;
+
+ if (!crtc-enabled) {
+ DRM_DEBUG_KMS(Cannot update primary plane of a disabled
CRTC.\n);
+ return -EINVAL;
+ }
We allow this for sprites, so I'd allow it for everything. I'd be fine
with leaving this restriction in drm_primary_helper_update() simply
because I have no interst in auditing every other driver.
Although I think we still overwrite the primary plane configure during
setcrtc. We should really change that so that the user can pre-configure
all the planes and then watch them pop into view during a modeset as
previously configured.
+
+ /* setplane API takes shifted source rectangle values; unshift them */
+ src_x = 16;
+ src_y = 16;
+ src_w = 16;
+ src_h = 16;
+
+ /* check scaling */
+ if (!can_scale (crtc_w != src_w || crtc_h != src_h)) {
+ DRM_DEBUG_KMS(Can't scale primary plane\n);
+ return -EINVAL;
+ }
+
+ /*
+ * Drivers that can scale should perform their own min/max scale
+ * factor checks.
+ */
+ hscale = drm_rect_calc_hscale(src, dest, 0, INT_MAX);
+ vscale = drm_rect_calc_vscale(src, dest, 0, INT_MAX);
+ visible = drm_rect_clip_scaled(src, dest, clip, hscale, vscale);
w/o sub-pixel coordinates the scaling factors will be truncated
to integers, which makes the clipping rather bogus if the plane can
actually scale. I think I'd just make this code assume that scaling
isn't supported, and if the driver supports scaling it can just
implent the appropriate code itself. We can try to refactor some of
the scaling aware code from intel_sprite later if warranted.
But I'm starting to question the usefulness of this function. We
anyway have to
