On Tue, Jul 26, 2016 at 05:33:29PM +0100, Chris Wilson wrote: > On Tue, Jul 26, 2016 at 07:06:58PM +0300, 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. > > > > Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > > --- > > drivers/gpu/drm/drm_plane_helper.c | 136 ++++++++++++++++++++++++++++--------- > > include/drm/drm_plane_helper.h | 5 ++ > > 2 files changed, 110 insertions(+), 31 deletions(-) > > > > diff --git a/drivers/gpu/drm/drm_plane_helper.c b/drivers/gpu/drm/drm_plane_helper.c > > index 16c4a7bd7465..1124eb827671 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,8 +118,6 @@ 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 > > * their own plane handling rather than helper-provided implementations may > > @@ -134,29 +127,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; > > It's a little surprising to me that the check writes fields into the > drm_plane_state. Care to at least mention the side-effects in the kernel > doc? That's a lot of what the atomic "check" functions do. A bit of a bad naming convention that. Not sure who came up with it. Should probably rename them all to "compute" or something. But I can at leat expand the docs here a bit in the meantime. -- Ville Syrjälä Intel OTC _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel