| Message ID | 1436788242-19788-1-git-send-email-zhjwpku@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Op 13-07-15 om 13:50 schreef John Hunter: > 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> Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
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
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.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,