[PATCH v2] drm: add a drm_atomic_helper_plane_check_update
From: Zhao Junwang
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
Cc: Daniel Vetter
Signed-off-by: Zhao Junwang
---
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);
+ 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
[PATCH v2] drm: add a drm_atomic_helper_plane_check_update
On Mon, Jul 13, 2015 at 07:50:42PM +0800, John Hunter wrote:
> From: Zhao Junwang
>
> 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
> Cc: Daniel Vetter
> Signed-off-by: Zhao Junwang
> ---
> 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
[PATCH v2] drm: add a drm_atomic_helper_plane_check_update
Op 13-07-15 om 13:50 schreef John Hunter: > From: Zhao Junwang > > 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 > Cc: Daniel Vetter > Signed-off-by: Zhao Junwang Reviewed-by: Maarten Lankhorst
