On Mon, Jul 13, 2015 at 08:21:32AM +0800, John Hunter wrote: > From: Zhao Junwang <zhjwpku@xxxxxxxxx> A bit of text to motivate this would be good - i.e. why is this useful, how did it blow up in the bochs conversion? > Cc: Daniel Vetter <daniel.vetter@xxxxxxxx> > Signed-off-by: Zhao Junwang <zhjwpku@xxxxxxxxx> > --- > drivers/gpu/drm/drm_atomic_helper.c | 55 +++++++++++++++++++++++++++++++++++ > include/drm/drm_atomic_helper.h | 7 +++++ > 2 files changed, 62 insertions(+) > > diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c > index 536ae4d..3d94ff8 100644 > --- a/drivers/gpu/drm/drm_atomic_helper.c > +++ b/drivers/gpu/drm/drm_atomic_helper.c > @@ -1336,6 +1336,61 @@ void drm_atomic_helper_swap_state(struct drm_device *dev, > EXPORT_SYMBOL(drm_atomic_helper_swap_state); > > /** > + * drm_atomic_helper_plane_check_update > + * @plane: plane object to update > + * @state: drm plane state > + * @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? > + * > + * Provide a default plane check update handler It's not a default handler since the signatures don't match. Also I think we should mention the legacy plane helper function here too. What about This provides a helper to be used in a driver's plane atomic_check callback. It is the atomic equivalent of drm_plane_helper_check_update() and has the exact same semantics, except that it looks at the atomit CRTC state in the atomic update instead of legacy state directly embedded in struct &drm_crtc. Also adding a comment to drm_plane_helper_check_update's kerneldoc would be good, like this: Note: When converting over to atomic drivers you need to switch over to using drm_atomic_helper_plane_check_update() since only that correctly checks atomic state - this function here only looks at legacy state and hence will check against stale values in atomic updates. > + * > + * RETURNS: > + * Zero on success, error code on failure > + */ > +int drm_atomic_helper_plane_check_update(struct drm_plane *plane, > + struct drm_plane_state *state, > + int min_scale, > + int max_scale, > + bool can_position, > + bool can_update_disabled, > + bool *visible) > +{ > + struct drm_crtc *crtc = state->crtc; > + struct drm_framebuffer *fb = state->fb; > + > + if (!fb) > + return 0; > + > + struct drm_rect src = { > + .x1 = state->src_x, > + .y1 = state->src_y, > + .x2 = state->src_x + state->src_w, > + .y2 = state->src_y + state->src_h, > + }; > + struct drm_rect dest = { > + .x1 = state->crtc_x, > + .y1 = state->crtc_y, > + .x2 = state->crtc_x + state->crtc_w, > + .y2 = state->crtc_y + state->crtc_h, > + }; > + const struct drm_rect clip = { > + .x2 = crtc->mode.hdisplay, > + .y2 = crtc->mode.vdisplay, crtc->mode is legacy state - we need to look at crtc_state here instead. > + }; > + > + return drm_plane_helper_check_update(plane, crtc, fb, > + &src, &dest, &clip, > + min_scale, max_scale, > + can_position, can_update_disabled, visible); This looks at crtc->enabled (which is also legacy state) for can_update_disabled. If we want to reuse this function we need to check can_updated_disabled here and it unconditionally to true when calling the plane helpers. Cheers, Daniel > +} > +EXPORT_SYMBOL(drm_atomic_helper_plane_check_update); > + > +/** > * drm_atomic_helper_update_plane - Helper for primary plane update using atomic > * @plane: plane object to update > * @crtc: owning CRTC of owning plane > diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h > index cc1fee8..5305a01 100644 > --- a/include/drm/drm_atomic_helper.h > +++ b/include/drm/drm_atomic_helper.h > @@ -64,6 +64,13 @@ void drm_atomic_helper_swap_state(struct drm_device *dev, > struct drm_atomic_state *state); > > /* implementations for legacy interfaces */ > +int drm_atomic_helper_plane_check_update(struct drm_plane *plane, > + struct drm_plane_state *state, > + int min_scale, > + int max_scale, > + bool can_position, > + bool can_update_disabled, > + bool *visible); > int drm_atomic_helper_update_plane(struct drm_plane *plane, > struct drm_crtc *crtc, > struct drm_framebuffer *fb, > -- > 1.7.10.4 > > > _______________________________________________ > dri-devel mailing list > dri-devel@xxxxxxxxxxxxxxxxxxxxx > http://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel