On Mon, Jul 13, 2015 at 07:50:42PM +0800, John Hunter wrote:

From: Zhao Junwang zhjwpku@gmail.com

This is the equivalent helper to drm_plane_helper_check_update for legacy drivers, but using atomic state to check things.

Motivated by the atomic conversion of the bochs driver.

v2: according to Daniel's comment -polish the kerneldoc comment to match the signatures -crtc->mode is legacy state, we need to look at crtc_state instead

according to Maarten's comment
-there is no need for can_update_disabled in the atomic world,
 so, we can delete that parameter

v3: according to Daniel's comment -this function call can't fail, add a WARN_ON -use drm_atomic_get_existing_crtc_state

according to Maarten's comment
-kill off the plane parameter and rename state to plane_state
-do not handling NULL, i.e. no need to check plane_state in
 atomic.

Cc: Maarten Lankhorst maarten.lankhorst@linux.intel.com Cc: Daniel Vetter daniel.vetter@ffwll.ch Signed-off-by: Zhao Junwang zhjwpku@gmail.com

---

drivers/gpu/drm/drm_atomic_helper.c | 56 +++++++++++++++++++++++++++++++++++ drivers/gpu/drm/drm_plane_helper.c | 6 ++++ include/drm/drm_atomic_helper.h | 5 ++++ 3 files changed, 67 insertions(+)

diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index 536ae4d..eaef689 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -1336,6 +1336,62 @@ void drm_atomic_helper_swap_state(struct drm_device *dev, EXPORT_SYMBOL(drm_atomic_helper_swap_state);

/**

• • drm_atomic_helper_plane_check_update
• • @plane_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.
    

• • 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 atomic CRTC state in the atomic update
• • instead of legacy state directly embedded in struct &drm_crtc.
• • RETURNS:
• • Zero on success, error code on failure
• */

+int drm_atomic_helper_plane_check_update(struct drm_plane_state *plane_state,

• 			 int min_scale,
  

• 			 int max_scale,
  

• 			 bool can_position,
  

• 			 bool *visible)
  

+{

• struct drm_crtc_state *crtc_state;
• crtc_state = drm_atomic_get_existing_crtc_state(plane_state->state,

• 					plane_state->crtc);
  

This function only returns NULL if the object is there, not an ERR_PTR. Hence you only need to check for:

if (WARN_ON(!crtc_state)) return -EBUSY;

IS_ERR actually doesn't catch NULL pointers afaik, so this wouldn't catch the one case we want it to catch. But not sure about that (IS_ERR is a bit confusing to me). -Daniel

• if (WARN_ON(IS_ERR(crtc_state)))

• return PTR_ERR(crtc_state);
  

• struct drm_rect src = {

• .x1 = plane_state->src_x,
  

• .y1 = plane_state->src_y,
  

• .x2 = plane_state->src_x + plane_state->src_w,
  

• .y2 = plane_state->src_y + plane_state->src_h,
  

• };
• struct drm_rect dest = {

• .x1 = plane_state->crtc_x,
  

• .y1 = plane_state->crtc_y,
  

• .x2 = plane_state->crtc_x + plane_state->crtc_w,
  

• .y2 = plane_state->crtc_y + plane_state->crtc_h,
  

• };
• const struct drm_rect clip = {

• .x2 = crtc_state->mode.hdisplay,
  

• .y2 = crtc_state->mode.vdisplay,
  

• };
• return drm_plane_helper_check_update(plane_state->plane,

• 			     plane_state->crtc,
  

• 			     plane_state->fb,
  

• 			     &src, &dest, &clip,
  

• 			     min_scale, max_scale,
  

• 			     can_position, true, visible);
  

+} +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/drivers/gpu/drm/drm_plane_helper.c b/drivers/gpu/drm/drm_plane_helper.c index 10be2d2..e346fe2 100644 --- a/drivers/gpu/drm/drm_plane_helper.c +++ b/drivers/gpu/drm/drm_plane_helper.c @@ -125,6 +125,12 @@ static int get_connectors_for_crtc(struct drm_crtc *crtc,

• still wish to call this function to avoid duplication of error checking
• code.

• • 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 if update appears valid, error code on failure
  */

diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h index cc1fee8..eaa2544 100644 --- a/include/drm/drm_atomic_helper.h +++ b/include/drm/drm_atomic_helper.h @@ -64,6 +64,11 @@ 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_state *plane_state,

• 			 int min_scale,
  

• 			 int max_scale,
  

• 			 bool can_position,
  

• 			 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