Re: [PATCH] drm: add a drm_atomic_helper_plane_check_update

On Mon, Jul 13, 2015 at 08:21:32AM +0800, John Hunter wrote:

From: Zhao Junwang zhjwpku@gmail.com

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@ffwll.ch Signed-off-by: Zhao Junwang zhjwpku@gmail.com

---

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@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel