| Message ID | 1483455998-26914-1-git-send-email-Andrey.Grodzovsky@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Jan 03, 2017 at 10:06:38AM -0500, Andrey Grodzovsky wrote: > Allows usage of the new page_flip_target hook for > drivers implementing the atomic path. > Provides default atomic helper for the new hook. > > Signed-off-by: Andrey Grodzovsky <Andrey.Grodzovsky@amd.com> Please keep a per-patch changelog so that it's easier for everyone to follow the evolution of this patch. > --- > drivers/gpu/drm/drm_atomic_helper.c | 146 ++++++++++++++++++++++++++++-------- > include/drm/drm_atomic_helper.h | 6 ++ > include/drm/drm_crtc.h | 3 + > 3 files changed, 125 insertions(+), 30 deletions(-) > > diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c > index 583f47f..5e76f90 100644 > --- a/drivers/gpu/drm/drm_atomic_helper.c > +++ b/drivers/gpu/drm/drm_atomic_helper.c > @@ -2733,6 +2733,59 @@ int drm_atomic_helper_resume(struct drm_device *dev, > } > EXPORT_SYMBOL(drm_atomic_helper_connector_set_property); > > +static int drm_atomic_helper_page_flip_create_state( I'd just call this page_flip_common without the long namespace prefix since it's static. And without the create_state name since this function also does some basic checks. > + struct drm_atomic_state *state, > + struct drm_crtc *crtc, > + struct drm_framebuffer *fb, > + struct drm_pending_vblank_event *event) > +{ > + struct drm_plane *plane = crtc->primary; > + struct drm_plane_state *plane_state; > + struct drm_crtc_state *crtc_state; > + int ret = 0; > + > + crtc_state = drm_atomic_get_crtc_state(state, crtc); > + if (IS_ERR(crtc_state)) > + return PTR_ERR(crtc_state); > + > + crtc_state->event = event; > + > + plane_state = drm_atomic_get_plane_state(state, plane); > + if (IS_ERR(plane_state)) > + return PTR_ERR(plane_state); > + > + > + ret = drm_atomic_set_crtc_for_plane(plane_state, crtc); > + if (ret != 0) > + return ret; > + drm_atomic_set_fb_for_plane(plane_state, fb); > + > + /* Make sure we don't accidentally do a full modeset. */ > + state->allow_modeset = false; > + if (!crtc_state->active) { > + DRM_DEBUG_ATOMIC("[CRTC:%d] disabled, rejecting legacy flip\n", > + crtc->base.id); > + return -EINVAL; > + } > + > + return ret; > +} > + > +static void drm_atomic_helper_page_flip_prepare_retry( > + struct drm_atomic_state *state, > + struct drm_plane *plane) > +{ > + drm_atomic_state_clear(state); > + drm_atomic_legacy_backoff(state); > + > + /* > + * Someone might have exchanged the framebuffer while we dropped locks > + * in the backoff code. We need to fix up the fb refcount tracking the > + * core does for us. > + */ > + plane->old_fb = plane->fb; There's a lot more places where we do this trick, please either refactor them all, or none. Personally I'd go with none since the function call is more verbose than the assignement, and you're hiding this rather important comment behind a function name that doesn't say what games are being played here. > +} > + > /** > * drm_atomic_helper_page_flip - execute a legacy page flip > * @crtc: DRM crtc > @@ -2756,8 +2809,6 @@ int drm_atomic_helper_page_flip(struct drm_crtc *crtc, > { > struct drm_plane *plane = crtc->primary; > struct drm_atomic_state *state; > - struct drm_plane_state *plane_state; > - struct drm_crtc_state *crtc_state; > int ret = 0; > > if (flags & DRM_MODE_PAGE_FLIP_ASYNC) > @@ -2768,35 +2819,79 @@ int drm_atomic_helper_page_flip(struct drm_crtc *crtc, > return -ENOMEM; > > state->acquire_ctx = drm_modeset_legacy_acquire_ctx(crtc); > + > retry: > - crtc_state = drm_atomic_get_crtc_state(state, crtc); > - if (IS_ERR(crtc_state)) { > - ret = PTR_ERR(crtc_state); > + ret = drm_atomic_helper_page_flip_create_state(state, crtc, fb, event); > + if (ret != 0) > goto fail; > - } > - crtc_state->event = event; > > - plane_state = drm_atomic_get_plane_state(state, plane); > - if (IS_ERR(plane_state)) { > - ret = PTR_ERR(plane_state); > - goto fail; > - } > + ret = drm_atomic_nonblocking_commit(state); > > - ret = drm_atomic_set_crtc_for_plane(plane_state, crtc); > +fail: > + if (ret == -EDEADLK) > + goto backoff; > + > + drm_atomic_state_put(state); > + return ret; > + > +backoff: > + drm_atomic_helper_page_flip_prepare_retry(state, plane); > + goto retry; > +} > +EXPORT_SYMBOL(drm_atomic_helper_page_flip); > + > +/** > + * drm_atomic_helper_page_flip - execute a legacy page flip > + * @crtc: DRM crtc > + * @fb: DRM framebuffer > + * @event: optional DRM event to signal upon completion > + * @flags: flip flags for non-vblank sync'ed updates > + * > + * Provides a default page flip on specific vblank implementation using > + * the atomic driver interface. > + * > + * Note that for now so called async page flips (i.e. updates which are not > + * synchronized to vblank) are not supported, since the atomic interfaces have > + * no provisions for this yet. > + * > + * Returns: > + * Returns 0 on success, negative errno numbers on failure. Kerneldoc isn't updated, just verbatim copypaste. Please also linke these 2 functions. And if you have time, would be good to sprinkle links to the vfunc hooks for each of these default helpers (they're unfortunately missing ...), e.g. here &drm_crtc_funcs.page_flip_target. And please build the docs per http://blog.ffwll.ch/2016/12/howto-docs.html and make sure it all looks pretty. > + */ > +int drm_atomic_helper_page_flip_on_target_vblank( Please name this helper to match the vhook it's supposed to slot into, i.e. drm_atomic_helper_page_flip_target. > + struct drm_crtc *crtc, > + struct drm_framebuffer *fb, > + struct drm_pending_vblank_event *event, > + uint32_t flags, > + uint32_t target) > +{ > + struct drm_plane *plane = crtc->primary; > + struct drm_atomic_state *state; > + struct drm_crtc_state *crtc_state; > + int ret = 0; > + > + if (flags & DRM_MODE_PAGE_FLIP_ASYNC) > + return -EINVAL; > + > + state = drm_atomic_state_alloc(plane->dev); > + if (!state) > + return -ENOMEM; > + > + state->acquire_ctx = drm_modeset_legacy_acquire_ctx(crtc); > + > +retry: > + ret = drm_atomic_helper_page_flip_create_state(state, crtc, fb, event); > if (ret != 0) > goto fail; > - drm_atomic_set_fb_for_plane(plane_state, fb); > > - /* Make sure we don't accidentally do a full modeset. */ > - state->allow_modeset = false; > - if (!crtc_state->active) { > - DRM_DEBUG_ATOMIC("[CRTC:%d] disabled, rejecting legacy flip\n", > - crtc->base.id); > + crtc_state = drm_atomic_get_existing_crtc_state(state, crtc); > + if (WARN_ON(!crtc_state)) { > ret = -EINVAL; > goto fail; > } > + crtc_state->target_vblank = target; > > ret = drm_atomic_nonblocking_commit(state); > + > fail: > if (ret == -EDEADLK) > goto backoff; > @@ -2805,19 +2900,10 @@ int drm_atomic_helper_page_flip(struct drm_crtc *crtc, > return ret; > > backoff: > - drm_atomic_state_clear(state); > - drm_atomic_legacy_backoff(state); > - > - /* > - * Someone might have exchanged the framebuffer while we dropped locks > - * in the backoff code. We need to fix up the fb refcount tracking the > - * core does for us. > - */ > - plane->old_fb = plane->fb; > - > + drm_atomic_helper_page_flip_prepare_retry(state, plane); > goto retry; > } > -EXPORT_SYMBOL(drm_atomic_helper_page_flip); > +EXPORT_SYMBOL(drm_atomic_helper_page_flip_on_target_vblank); > > /** > * drm_atomic_helper_connector_dpms() - connector dpms helper implementation > diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h > index 7ff92b0..e8cb6b7 100644 > --- a/include/drm/drm_atomic_helper.h > +++ b/include/drm/drm_atomic_helper.h > @@ -124,6 +124,12 @@ int drm_atomic_helper_page_flip(struct drm_crtc *crtc, > struct drm_framebuffer *fb, > struct drm_pending_vblank_event *event, > uint32_t flags); > +int drm_atomic_helper_page_flip_on_target_vblank( > + struct drm_crtc *crtc, > + struct drm_framebuffer *fb, > + struct drm_pending_vblank_event *event, > + uint32_t flags, > + uint32_t target); > int drm_atomic_helper_connector_dpms(struct drm_connector *connector, > int mode); > struct drm_encoder * > diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h > index 946672f..cc926dc 100644 > --- a/include/drm/drm_crtc.h > +++ b/include/drm/drm_crtc.h > @@ -103,6 +103,7 @@ static inline uint64_t I642U64(int64_t val) > * @ctm: Transformation matrix > * @gamma_lut: Lookup table for converting pixel data after the > * conversion matrix > + * @target_vblank: Target vblank count to flip > * @state: backpointer to global drm_atomic_state > * > * Note that the distinction between @enable and @active is rather subtile: > @@ -156,6 +157,8 @@ struct drm_crtc_state { > struct drm_property_blob *ctm; > struct drm_property_blob *gamma_lut; > > + u32 target_vblank; The in-line style is preferred for structures, since it allows to group comments with each member. There's also still the question of how drivers opt-in into target_vblank (only amdgpu, and then only in your private branch supports it). I think that should be documented in the kernel-doc. Thanks, Daniel > + > /** > * @event: > * > -- > 1.9.1 >
> -----Original Message----- > From: Daniel Vetter [mailto:daniel.vetter@ffwll.ch] On Behalf Of Daniel > Vetter > Sent: Wednesday, January 04, 2017 3:44 AM > To: Grodzovsky, Andrey > Cc: dri-devel@lists.freedesktop.org; daniel.vetter@ffwll.ch; Deucher, > Alexander; Daenzer, Michel; Wentland, Harry > Subject: Re: [PATCH] drm/atomic: Add target_vblank support in atomic > helpers. > > On Tue, Jan 03, 2017 at 10:06:38AM -0500, Andrey Grodzovsky wrote: > > Allows usage of the new page_flip_target hook for drivers implementing > > the atomic path. > > Provides default atomic helper for the new hook. > > > > Signed-off-by: Andrey Grodzovsky <Andrey.Grodzovsky@amd.com> > > Please keep a per-patch changelog so that it's easier for everyone to follow > the evolution of this patch. > > > --- > > drivers/gpu/drm/drm_atomic_helper.c | 146 > ++++++++++++++++++++++++++++-------- > > include/drm/drm_atomic_helper.h | 6 ++ > > include/drm/drm_crtc.h | 3 + > > 3 files changed, 125 insertions(+), 30 deletions(-) > > > > diff --git a/drivers/gpu/drm/drm_atomic_helper.c > > b/drivers/gpu/drm/drm_atomic_helper.c > > index 583f47f..5e76f90 100644 > > --- a/drivers/gpu/drm/drm_atomic_helper.c > > +++ b/drivers/gpu/drm/drm_atomic_helper.c > > @@ -2733,6 +2733,59 @@ int drm_atomic_helper_resume(struct > drm_device > > *dev, } EXPORT_SYMBOL(drm_atomic_helper_connector_set_property); > > > > +static int drm_atomic_helper_page_flip_create_state( > > I'd just call this page_flip_common without the long namespace prefix since > it's static. And without the create_state name since this function also does > some basic checks. > > > + struct drm_atomic_state *state, > > + struct drm_crtc *crtc, > > + struct drm_framebuffer *fb, > > + struct drm_pending_vblank_event *event) { > > + struct drm_plane *plane = crtc->primary; > > + struct drm_plane_state *plane_state; > > + struct drm_crtc_state *crtc_state; > > + int ret = 0; > > + > > + crtc_state = drm_atomic_get_crtc_state(state, crtc); > > + if (IS_ERR(crtc_state)) > > + return PTR_ERR(crtc_state); > > + > > + crtc_state->event = event; > > + > > + plane_state = drm_atomic_get_plane_state(state, plane); > > + if (IS_ERR(plane_state)) > > + return PTR_ERR(plane_state); > > + > > + > > + ret = drm_atomic_set_crtc_for_plane(plane_state, crtc); > > + if (ret != 0) > > + return ret; > > + drm_atomic_set_fb_for_plane(plane_state, fb); > > + > > + /* Make sure we don't accidentally do a full modeset. */ > > + state->allow_modeset = false; > > + if (!crtc_state->active) { > > + DRM_DEBUG_ATOMIC("[CRTC:%d] disabled, rejecting legacy > flip\n", > > + crtc->base.id); > > + return -EINVAL; > > + } > > + > > + return ret; > > +} > > + > > +static void drm_atomic_helper_page_flip_prepare_retry( > > + struct drm_atomic_state *state, > > + struct drm_plane *plane) > > +{ > > + drm_atomic_state_clear(state); > > + drm_atomic_legacy_backoff(state); > > + > > + /* > > + * Someone might have exchanged the framebuffer while we > dropped locks > > + * in the backoff code. We need to fix up the fb refcount tracking the > > + * core does for us. > > + */ > > + plane->old_fb = plane->fb; > > There's a lot more places where we do this trick, please either refactor them > all, or none. Personally I'd go with none since the function call is more > verbose than the assignement, and you're hiding this rather important > comment behind a function name that doesn't say what games are being > played here. > > > +} > > + > > /** > > * drm_atomic_helper_page_flip - execute a legacy page flip > > * @crtc: DRM crtc > > @@ -2756,8 +2809,6 @@ int drm_atomic_helper_page_flip(struct drm_crtc > > *crtc, { > > struct drm_plane *plane = crtc->primary; > > struct drm_atomic_state *state; > > - struct drm_plane_state *plane_state; > > - struct drm_crtc_state *crtc_state; > > int ret = 0; > > > > if (flags & DRM_MODE_PAGE_FLIP_ASYNC) @@ -2768,35 +2819,79 > @@ int > > drm_atomic_helper_page_flip(struct drm_crtc *crtc, > > return -ENOMEM; > > > > state->acquire_ctx = drm_modeset_legacy_acquire_ctx(crtc); > > + > > retry: > > - crtc_state = drm_atomic_get_crtc_state(state, crtc); > > - if (IS_ERR(crtc_state)) { > > - ret = PTR_ERR(crtc_state); > > + ret = drm_atomic_helper_page_flip_create_state(state, crtc, fb, > event); > > + if (ret != 0) > > goto fail; > > - } > > - crtc_state->event = event; > > > > - plane_state = drm_atomic_get_plane_state(state, plane); > > - if (IS_ERR(plane_state)) { > > - ret = PTR_ERR(plane_state); > > - goto fail; > > - } > > + ret = drm_atomic_nonblocking_commit(state); > > > > - ret = drm_atomic_set_crtc_for_plane(plane_state, crtc); > > +fail: > > + if (ret == -EDEADLK) > > + goto backoff; > > + > > + drm_atomic_state_put(state); > > + return ret; > > + > > +backoff: > > + drm_atomic_helper_page_flip_prepare_retry(state, plane); > > + goto retry; > > +} > > +EXPORT_SYMBOL(drm_atomic_helper_page_flip); > > + > > +/** > > + * drm_atomic_helper_page_flip - execute a legacy page flip > > + * @crtc: DRM crtc > > + * @fb: DRM framebuffer > > + * @event: optional DRM event to signal upon completion > > + * @flags: flip flags for non-vblank sync'ed updates > > + * > > + * Provides a default page flip on specific vblank implementation > > +using > > + * the atomic driver interface. > > + * > > + * Note that for now so called async page flips (i.e. updates which > > +are not > > + * synchronized to vblank) are not supported, since the atomic > > +interfaces have > > + * no provisions for this yet. > > + * > > + * Returns: > > + * Returns 0 on success, negative errno numbers on failure. > > Kerneldoc isn't updated, just verbatim copypaste. Please also linke these > 2 functions. And if you have time, would be good to sprinkle links to the Not sure here, do you mean kerneldoc is not generated for the new helper function ? If so , Is it because @tags are missing ? > vfunc hooks for each of these default helpers (they're unfortunately missing > ...), e.g. here &drm_crtc_funcs.page_flip_target. Do you mean adding kernedoc for the vfunc hook ? Thanks, Andrey > > And please build the docs per http://blog.ffwll.ch/2016/12/howto-docs.html > and make sure it all looks pretty. > > > + */ > > +int drm_atomic_helper_page_flip_on_target_vblank( > > Please name this helper to match the vhook it's supposed to slot into, i.e. > drm_atomic_helper_page_flip_target. > > > + struct drm_crtc *crtc, > > + struct drm_framebuffer *fb, > > + struct drm_pending_vblank_event *event, > > + uint32_t flags, > > + uint32_t target) > > +{ > > + struct drm_plane *plane = crtc->primary; > > + struct drm_atomic_state *state; > > + struct drm_crtc_state *crtc_state; > > + int ret = 0; > > + > > + if (flags & DRM_MODE_PAGE_FLIP_ASYNC) > > + return -EINVAL; > > + > > + state = drm_atomic_state_alloc(plane->dev); > > + if (!state) > > + return -ENOMEM; > > + > > + state->acquire_ctx = drm_modeset_legacy_acquire_ctx(crtc); > > + > > +retry: > > + ret = drm_atomic_helper_page_flip_create_state(state, crtc, fb, > > +event); > > if (ret != 0) > > goto fail; > > - drm_atomic_set_fb_for_plane(plane_state, fb); > > > > - /* Make sure we don't accidentally do a full modeset. */ > > - state->allow_modeset = false; > > - if (!crtc_state->active) { > > - DRM_DEBUG_ATOMIC("[CRTC:%d] disabled, rejecting legacy > flip\n", > > - crtc->base.id); > > + crtc_state = drm_atomic_get_existing_crtc_state(state, crtc); > > + if (WARN_ON(!crtc_state)) { > > ret = -EINVAL; > > goto fail; > > } > > + crtc_state->target_vblank = target; > > > > ret = drm_atomic_nonblocking_commit(state); > > + > > fail: > > if (ret == -EDEADLK) > > goto backoff; > > @@ -2805,19 +2900,10 @@ int drm_atomic_helper_page_flip(struct > drm_crtc *crtc, > > return ret; > > > > backoff: > > - drm_atomic_state_clear(state); > > - drm_atomic_legacy_backoff(state); > > - > > - /* > > - * Someone might have exchanged the framebuffer while we > dropped locks > > - * in the backoff code. We need to fix up the fb refcount tracking the > > - * core does for us. > > - */ > > - plane->old_fb = plane->fb; > > - > > + drm_atomic_helper_page_flip_prepare_retry(state, plane); > > goto retry; > > } > > -EXPORT_SYMBOL(drm_atomic_helper_page_flip); > > +EXPORT_SYMBOL(drm_atomic_helper_page_flip_on_target_vblank); > > > > /** > > * drm_atomic_helper_connector_dpms() - connector dpms helper > > implementation diff --git a/include/drm/drm_atomic_helper.h > > b/include/drm/drm_atomic_helper.h index 7ff92b0..e8cb6b7 100644 > > --- a/include/drm/drm_atomic_helper.h > > +++ b/include/drm/drm_atomic_helper.h > > @@ -124,6 +124,12 @@ int drm_atomic_helper_page_flip(struct drm_crtc > *crtc, > > struct drm_framebuffer *fb, > > struct drm_pending_vblank_event *event, > > uint32_t flags); > > +int drm_atomic_helper_page_flip_on_target_vblank( > > + struct drm_crtc *crtc, > > + struct drm_framebuffer *fb, > > + struct drm_pending_vblank_event *event, > > + uint32_t flags, > > + uint32_t target); > > int drm_atomic_helper_connector_dpms(struct drm_connector > *connector, > > int mode); > > struct drm_encoder * > > diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index > > 946672f..cc926dc 100644 > > --- a/include/drm/drm_crtc.h > > +++ b/include/drm/drm_crtc.h > > @@ -103,6 +103,7 @@ static inline uint64_t I642U64(int64_t val) > > * @ctm: Transformation matrix > > * @gamma_lut: Lookup table for converting pixel data after the > > * conversion matrix > > + * @target_vblank: Target vblank count to flip > > * @state: backpointer to global drm_atomic_state > > * > > * Note that the distinction between @enable and @active is rather > subtile: > > @@ -156,6 +157,8 @@ struct drm_crtc_state { > > struct drm_property_blob *ctm; > > struct drm_property_blob *gamma_lut; > > > > + u32 target_vblank; > > The in-line style is preferred for structures, since it allows to group comments > with each member. There's also still the question of how drivers opt-in into > target_vblank (only amdgpu, and then only in your private branch supports > it). I think that should be documented in the kernel-doc. > > Thanks, Daniel > > + > > /** > > * @event: > > * > > -- > > 1.9.1 > > > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch
On Wed, Jan 04, 2017 at 04:36:50PM +0000, Grodzovsky, Andrey wrote: > > > +/** > > > + * drm_atomic_helper_page_flip - execute a legacy page flip > > > + * @crtc: DRM crtc > > > + * @fb: DRM framebuffer > > > + * @event: optional DRM event to signal upon completion > > > + * @flags: flip flags for non-vblank sync'ed updates > > > + * > > > + * Provides a default page flip on specific vblank implementation > > > +using > > > + * the atomic driver interface. > > > + * > > > + * Note that for now so called async page flips (i.e. updates which > > > +are not > > > + * synchronized to vblank) are not supported, since the atomic > > > +interfaces have > > > + * no provisions for this yet. > > > + * > > > + * Returns: > > > + * Returns 0 on success, negative errno numbers on failure. > > > > Kerneldoc isn't updated, just verbatim copypaste. Please also linke these > > 2 functions. And if you have time, would be good to sprinkle links to the > > Not sure here, do you mean kerneldoc is not generated for the new helper function ? If so , > Is it because @tags are missing ? You should get a warning about the missing @tag, yes. But having the exact same text for both is pointless, so for similar functions it's good to highlight the differences (and mabye link between the two using the functions() syntax). > > > vfunc hooks for each of these default helpers (they're unfortunately missing > > ...), e.g. here &drm_crtc_funcs.page_flip_target. > > Do you mean adding kernedoc for the vfunc hook ? No, just linking to them. http://blog.ffwll.ch/2016/12/howto-docs.html and you can see the output, makes all this a bit easier to understand probably.
diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index 583f47f..5e76f90 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -2733,6 +2733,59 @@ int drm_atomic_helper_resume(struct drm_device *dev, } EXPORT_SYMBOL(drm_atomic_helper_connector_set_property); +static int drm_atomic_helper_page_flip_create_state( + struct drm_atomic_state *state, + struct drm_crtc *crtc, + struct drm_framebuffer *fb, + struct drm_pending_vblank_event *event) +{ + struct drm_plane *plane = crtc->primary; + struct drm_plane_state *plane_state; + struct drm_crtc_state *crtc_state; + int ret = 0; + + crtc_state = drm_atomic_get_crtc_state(state, crtc); + if (IS_ERR(crtc_state)) + return PTR_ERR(crtc_state); + + crtc_state->event = event; + + plane_state = drm_atomic_get_plane_state(state, plane); + if (IS_ERR(plane_state)) + return PTR_ERR(plane_state); + + + ret = drm_atomic_set_crtc_for_plane(plane_state, crtc); + if (ret != 0) + return ret; + drm_atomic_set_fb_for_plane(plane_state, fb); + + /* Make sure we don't accidentally do a full modeset. */ + state->allow_modeset = false; + if (!crtc_state->active) { + DRM_DEBUG_ATOMIC("[CRTC:%d] disabled, rejecting legacy flip\n", + crtc->base.id); + return -EINVAL; + } + + return ret; +} + +static void drm_atomic_helper_page_flip_prepare_retry( + struct drm_atomic_state *state, + struct drm_plane *plane) +{ + drm_atomic_state_clear(state); + drm_atomic_legacy_backoff(state); + + /* + * Someone might have exchanged the framebuffer while we dropped locks + * in the backoff code. We need to fix up the fb refcount tracking the + * core does for us. + */ + plane->old_fb = plane->fb; +} + /** * drm_atomic_helper_page_flip - execute a legacy page flip * @crtc: DRM crtc @@ -2756,8 +2809,6 @@ int drm_atomic_helper_page_flip(struct drm_crtc *crtc, { struct drm_plane *plane = crtc->primary; struct drm_atomic_state *state; - struct drm_plane_state *plane_state; - struct drm_crtc_state *crtc_state; int ret = 0; if (flags & DRM_MODE_PAGE_FLIP_ASYNC) @@ -2768,35 +2819,79 @@ int drm_atomic_helper_page_flip(struct drm_crtc *crtc, return -ENOMEM; state->acquire_ctx = drm_modeset_legacy_acquire_ctx(crtc); + retry: - crtc_state = drm_atomic_get_crtc_state(state, crtc); - if (IS_ERR(crtc_state)) { - ret = PTR_ERR(crtc_state); + ret = drm_atomic_helper_page_flip_create_state(state, crtc, fb, event); + if (ret != 0) goto fail; - } - crtc_state->event = event; - plane_state = drm_atomic_get_plane_state(state, plane); - if (IS_ERR(plane_state)) { - ret = PTR_ERR(plane_state); - goto fail; - } + ret = drm_atomic_nonblocking_commit(state); - ret = drm_atomic_set_crtc_for_plane(plane_state, crtc); +fail: + if (ret == -EDEADLK) + goto backoff; + + drm_atomic_state_put(state); + return ret; + +backoff: + drm_atomic_helper_page_flip_prepare_retry(state, plane); + goto retry; +} +EXPORT_SYMBOL(drm_atomic_helper_page_flip); + +/** + * drm_atomic_helper_page_flip - execute a legacy page flip + * @crtc: DRM crtc + * @fb: DRM framebuffer + * @event: optional DRM event to signal upon completion + * @flags: flip flags for non-vblank sync'ed updates + * + * Provides a default page flip on specific vblank implementation using + * the atomic driver interface. + * + * Note that for now so called async page flips (i.e. updates which are not + * synchronized to vblank) are not supported, since the atomic interfaces have + * no provisions for this yet. + * + * Returns: + * Returns 0 on success, negative errno numbers on failure. + */ +int drm_atomic_helper_page_flip_on_target_vblank( + struct drm_crtc *crtc, + struct drm_framebuffer *fb, + struct drm_pending_vblank_event *event, + uint32_t flags, + uint32_t target) +{ + struct drm_plane *plane = crtc->primary; + struct drm_atomic_state *state; + struct drm_crtc_state *crtc_state; + int ret = 0; + + if (flags & DRM_MODE_PAGE_FLIP_ASYNC) + return -EINVAL; + + state = drm_atomic_state_alloc(plane->dev); + if (!state) + return -ENOMEM; + + state->acquire_ctx = drm_modeset_legacy_acquire_ctx(crtc); + +retry: + ret = drm_atomic_helper_page_flip_create_state(state, crtc, fb, event); if (ret != 0) goto fail; - drm_atomic_set_fb_for_plane(plane_state, fb); - /* Make sure we don't accidentally do a full modeset. */ - state->allow_modeset = false; - if (!crtc_state->active) { - DRM_DEBUG_ATOMIC("[CRTC:%d] disabled, rejecting legacy flip\n", - crtc->base.id); + crtc_state = drm_atomic_get_existing_crtc_state(state, crtc); + if (WARN_ON(!crtc_state)) { ret = -EINVAL; goto fail; } + crtc_state->target_vblank = target; ret = drm_atomic_nonblocking_commit(state); + fail: if (ret == -EDEADLK) goto backoff; @@ -2805,19 +2900,10 @@ int drm_atomic_helper_page_flip(struct drm_crtc *crtc, return ret; backoff: - drm_atomic_state_clear(state); - drm_atomic_legacy_backoff(state); - - /* - * Someone might have exchanged the framebuffer while we dropped locks - * in the backoff code. We need to fix up the fb refcount tracking the - * core does for us. - */ - plane->old_fb = plane->fb; - + drm_atomic_helper_page_flip_prepare_retry(state, plane); goto retry; } -EXPORT_SYMBOL(drm_atomic_helper_page_flip); +EXPORT_SYMBOL(drm_atomic_helper_page_flip_on_target_vblank); /** * drm_atomic_helper_connector_dpms() - connector dpms helper implementation diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h index 7ff92b0..e8cb6b7 100644 --- a/include/drm/drm_atomic_helper.h +++ b/include/drm/drm_atomic_helper.h @@ -124,6 +124,12 @@ int drm_atomic_helper_page_flip(struct drm_crtc *crtc, struct drm_framebuffer *fb, struct drm_pending_vblank_event *event, uint32_t flags); +int drm_atomic_helper_page_flip_on_target_vblank( + struct drm_crtc *crtc, + struct drm_framebuffer *fb, + struct drm_pending_vblank_event *event, + uint32_t flags, + uint32_t target); int drm_atomic_helper_connector_dpms(struct drm_connector *connector, int mode); struct drm_encoder * diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index 946672f..cc926dc 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -103,6 +103,7 @@ static inline uint64_t I642U64(int64_t val) * @ctm: Transformation matrix * @gamma_lut: Lookup table for converting pixel data after the * conversion matrix + * @target_vblank: Target vblank count to flip * @state: backpointer to global drm_atomic_state * * Note that the distinction between @enable and @active is rather subtile: @@ -156,6 +157,8 @@ struct drm_crtc_state { struct drm_property_blob *ctm; struct drm_property_blob *gamma_lut; + u32 target_vblank; + /** * @event: *
Allows usage of the new page_flip_target hook for drivers implementing the atomic path. Provides default atomic helper for the new hook. Signed-off-by: Andrey Grodzovsky <Andrey.Grodzovsky@amd.com> --- drivers/gpu/drm/drm_atomic_helper.c | 146 ++++++++++++++++++++++++++++-------- include/drm/drm_atomic_helper.h | 6 ++ include/drm/drm_crtc.h | 3 + 3 files changed, 125 insertions(+), 30 deletions(-)