[Intel-gfx] [PATCH v2 3/9] drm/plane-helper: Add drm_plane_helper_check_state()
Sean Paul
seanpaul at chromium.org
Mon Aug 1 15:10:08 UTC 2016
On Tue, Jul 26, 2016 at 1:34 PM, <ville.syrjala at linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala at linux.intel.com>
>
> Add a version of drm_plane_helper_check_update() which takes a plane
> state instead of having the caller pass in everything.
>
> And to reduce code duplication, let's reimplement
> drm_plane_helper_check_update() in terms of the new function, by
> having a tempororary plane state on the stack.
>
> v2: Add a note that the functions modifies the state (Chris)
>
> Cc: Chris Wilson <chris at chris-wilson.co.uk>
> Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
Reviewed-by: Sean Paul <seanpaul at chromium.org>
> ---
> drivers/gpu/drm/drm_plane_helper.c | 139 ++++++++++++++++++++++++++++---------
> include/drm/drm_plane_helper.h | 5 ++
> 2 files changed, 112 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_plane_helper.c b/drivers/gpu/drm/drm_plane_helper.c
> index 16c4a7bd7465..ca02997b1c36 100644
> --- a/drivers/gpu/drm/drm_plane_helper.c
> +++ b/drivers/gpu/drm/drm_plane_helper.c
> @@ -108,14 +108,9 @@ static int get_connectors_for_crtc(struct drm_crtc *crtc,
> }
>
> /**
> - * 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
> + * drm_plane_helper_check_state() - Check plane state for validity
> + * @state: plane state to check
> * @clip: integer clipping coordinates
> - * @rotation: plane rotation
> * @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
> @@ -123,10 +118,9 @@ static int get_connectors_for_crtc(struct drm_crtc *crtc,
> * 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
> + * Checks that a desired plane update is valid, and updates various
> + * bits of derived state (clipped coordinates etc.). 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.
> @@ -134,29 +128,38 @@ static int get_connectors_for_crtc(struct drm_crtc *crtc,
> * 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,
> - unsigned int rotation,
> - int min_scale,
> - int max_scale,
> - bool can_position,
> - bool can_update_disabled,
> - bool *visible)
> +int drm_plane_helper_check_state(struct drm_plane_state *state,
> + const struct drm_rect *clip,
> + int min_scale,
> + int max_scale,
> + bool can_position,
> + bool can_update_disabled)
> {
> + struct drm_crtc *crtc = state->crtc;
> + struct drm_framebuffer *fb = state->fb;
> + struct drm_rect *src = &state->src;
> + struct drm_rect *dst = &state->dst;
> + unsigned int rotation = state->rotation;
> int hscale, vscale;
>
> + src->x1 = state->src_x;
> + src->y1 = state->src_y;
> + src->x2 = state->src_x + state->src_w;
> + src->y2 = state->src_y + state->src_h;
> +
> + dst->x1 = state->crtc_x;
> + dst->y1 = state->crtc_y;
> + dst->x2 = state->crtc_x + state->crtc_w;
> + dst->y2 = state->crtc_y + state->crtc_h;
> +
> if (!fb) {
> - *visible = false;
> + state->visible = false;
> return 0;
> }
>
> /* crtc should only be NULL when disabling (i.e., !fb) */
> if (WARN_ON(!crtc)) {
> - *visible = false;
> + state->visible = false;
> return 0;
> }
>
> @@ -168,20 +171,20 @@ int drm_plane_helper_check_update(struct drm_plane *plane,
> drm_rect_rotate(src, fb->width << 16, fb->height << 16, rotation);
>
> /* Check scaling */
> - hscale = drm_rect_calc_hscale(src, dest, min_scale, max_scale);
> - vscale = drm_rect_calc_vscale(src, dest, min_scale, max_scale);
> + hscale = drm_rect_calc_hscale(src, dst, min_scale, max_scale);
> + vscale = drm_rect_calc_vscale(src, dst, min_scale, max_scale);
> if (hscale < 0 || vscale < 0) {
> DRM_DEBUG_KMS("Invalid scaling of plane\n");
> - drm_rect_debug_print("src: ", src, true);
> - drm_rect_debug_print("dst: ", dest, false);
> + drm_rect_debug_print("src: ", &state->src, true);
> + drm_rect_debug_print("dst: ", &state->dst, false);
> return -ERANGE;
> }
>
> - *visible = drm_rect_clip_scaled(src, dest, clip, hscale, vscale);
> + state->visible = drm_rect_clip_scaled(src, dst, clip, hscale, vscale);
>
> drm_rect_rotate_inv(src, fb->width << 16, fb->height << 16, rotation);
>
> - if (!*visible)
> + if (!state->visible)
> /*
> * Plane isn't visible; some drivers can handle this
> * so we just return success here. Drivers that can't
> @@ -191,15 +194,87 @@ int drm_plane_helper_check_update(struct drm_plane *plane,
> */
> return 0;
>
> - if (!can_position && !drm_rect_equals(dest, clip)) {
> + if (!can_position && !drm_rect_equals(dst, clip)) {
> DRM_DEBUG_KMS("Plane must cover entire CRTC\n");
> - drm_rect_debug_print("dst: ", dest, false);
> + drm_rect_debug_print("dst: ", dst, false);
> drm_rect_debug_print("clip: ", clip, false);
> return -EINVAL;
> }
>
> return 0;
> }
> +EXPORT_SYMBOL(drm_plane_helper_check_state);
> +
> +/**
> + * 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
> + * @rotation: plane rotation
> + * @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 *dst,
> + const struct drm_rect *clip,
> + unsigned int rotation,
> + int min_scale,
> + int max_scale,
> + bool can_position,
> + bool can_update_disabled,
> + bool *visible)
> +{
> + struct drm_plane_state state = {
> + .plane = plane,
> + .crtc = crtc,
> + .fb = fb,
> + .src_x = src->x1,
> + .src_y = src->x2,
> + .src_w = drm_rect_width(src),
> + .src_h = drm_rect_height(src),
> + .crtc_x = dst->x1,
> + .crtc_y = dst->x2,
> + .crtc_w = drm_rect_width(dst),
> + .crtc_h = drm_rect_height(dst),
> + .rotation = rotation,
> + .visible = *visible,
> + };
> + int ret;
> +
> + ret = drm_plane_helper_check_state(&state, clip,
> + min_scale, max_scale,
> + can_position,
> + can_update_disabled);
> + if (ret)
> + return ret;
> +
> + *src = state.src;
> + *dst = state.dst;
> + *visible = state.visible;
> +
> + return 0;
> +}
> EXPORT_SYMBOL(drm_plane_helper_check_update);
>
> /**
> diff --git a/include/drm/drm_plane_helper.h b/include/drm/drm_plane_helper.h
> index 0e0c3573cce0..fbc8ecb3e5e8 100644
> --- a/include/drm/drm_plane_helper.h
> +++ b/include/drm/drm_plane_helper.h
> @@ -40,6 +40,11 @@
> int drm_crtc_init(struct drm_device *dev, struct drm_crtc *crtc,
> const struct drm_crtc_funcs *funcs);
>
> +int drm_plane_helper_check_state(struct drm_plane_state *state,
> + const struct drm_rect *clip,
> + int min_scale, int max_scale,
> + bool can_position,
> + bool can_update_disabled);
> int drm_plane_helper_check_update(struct drm_plane *plane,
> struct drm_crtc *crtc,
> struct drm_framebuffer *fb,
> --
> 2.7.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
More information about the Intel-gfx
mailing list