On Tue, Jul 26, 2016 at 1:34 PM, <ville.syrjala@xxxxxxxxxxxxxxx> wrote: > From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > > 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@xxxxxxxxxxxxxxxxxx> > Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> Reviewed-by: Sean Paul <seanpaul@xxxxxxxxxxxx> > --- > 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@xxxxxxxxxxxxxxxxxxxxx > https://lists.freedesktop.org/mailman/listinfo/intel-gfx _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel