On Tue, Aug 25, 2015 at 03:35:58PM -0400, Rob Clark wrote:
> Add support for using atomic code-paths for restore_fbdev_mode().
>
> Signed-off-by: Rob Clark <robdclark at gmail.com>
> ---
> drivers/gpu/drm/drm_atomic_helper.c | 131
> +++++++++++++++++++++---------------
> drivers/gpu/drm/drm_fb_helper.c | 74 ++++++++++++++++++++
> include/drm/drm_atomic_helper.h | 6 ++
> include/drm/drm_fb_helper.h | 8 +++
> 4 files changed, 166 insertions(+), 53 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c
> b/drivers/gpu/drm/drm_atomic_helper.c
> index d432348..dbce582 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -1502,21 +1502,9 @@ retry:
> goto fail;
> }
>
> - ret = drm_atomic_set_crtc_for_plane(plane_state, NULL);
> + ret = __drm_atomic_helper_disable_plane(plane, plane_state);
> if (ret != 0)
> goto fail;
> - drm_atomic_set_fb_for_plane(plane_state, NULL);
> - plane_state->crtc_x = 0;
> - plane_state->crtc_y = 0;
> - plane_state->crtc_h = 0;
> - plane_state->crtc_w = 0;
> - plane_state->src_x = 0;
> - plane_state->src_y = 0;
> - plane_state->src_h = 0;
> - plane_state->src_w = 0;
> -
> - if (plane == plane->crtc->cursor)
> - state->legacy_cursor_update = true;
>
> ret = drm_atomic_commit(state);
> if (ret != 0)
> @@ -1546,6 +1534,32 @@ backoff:
> }
> EXPORT_SYMBOL(drm_atomic_helper_disable_plane);
>
> +/* just used from fb-helper and atomic-helper: */
> +int __drm_atomic_helper_disable_plane(struct drm_plane *plane,
> + struct drm_plane_state *plane_state)
> +{
> + int ret;
> +
> + ret = drm_atomic_set_crtc_for_plane(plane_state, NULL);
> + if (ret != 0)
> + return ret;
> +
> + drm_atomic_set_fb_for_plane(plane_state, NULL);
> + plane_state->crtc_x = 0;
> + plane_state->crtc_y = 0;
> + plane_state->crtc_h = 0;
> + plane_state->crtc_w = 0;
> + plane_state->src_x = 0;
> + plane_state->src_y = 0;
> + plane_state->src_h = 0;
> + plane_state->src_w = 0;
> +
> + if (plane->crtc && (plane == plane->crtc->cursor))
> + plane_state->state->legacy_cursor_update = true;
> +
> + return 0;
> +}
> +
> static int update_output_state(struct drm_atomic_state *state,
> struct drm_mode_set *set)
> {
> @@ -1629,8 +1643,6 @@ int drm_atomic_helper_set_config(struct drm_mode_set
> *set)
> {
> struct drm_atomic_state *state;
> struct drm_crtc *crtc = set->crtc;
> - struct drm_crtc_state *crtc_state;
> - struct drm_plane_state *primary_state;
> int ret = 0;
>
> state = drm_atomic_state_alloc(crtc->dev);
> @@ -1639,17 +1651,54 @@ int drm_atomic_helper_set_config(struct drm_mode_set
> *set)
>
> 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_set_config(set, state);
> + if (ret != 0)
> goto fail;
> - }
>
> - primary_state = drm_atomic_get_plane_state(state, crtc->primary);
> - if (IS_ERR(primary_state)) {
> - ret = PTR_ERR(primary_state);
> + ret = drm_atomic_commit(state);
> + if (ret != 0)
> goto fail;
> - }
> +
> + /* Driver takes ownership of state on successful commit. */
> + return 0;
> +fail:
> + if (ret == -EDEADLK)
> + goto backoff;
> +
> + drm_atomic_state_free(state);
> +
> + 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.
> + */
> + crtc->primary->old_fb = crtc->primary->fb;
> +
> + goto retry;
> +}
> +EXPORT_SYMBOL(drm_atomic_helper_set_config);
> +
> +/* just used from fb-helper and atomic-helper: */
> +int __drm_atomic_helper_set_config(struct drm_mode_set *set,
> + struct drm_atomic_state *state)
> +{
> + struct drm_crtc_state *crtc_state;
> + struct drm_plane_state *primary_state;
> + struct drm_crtc *crtc = set->crtc;
> + int ret;
> +
> + crtc_state = drm_atomic_get_crtc_state(state, crtc);
> + if (IS_ERR(crtc_state))
> + return PTR_ERR(crtc_state);
> +
> + primary_state = drm_atomic_get_plane_state(state, crtc->primary);
> + if (IS_ERR(primary_state))
> + return PTR_ERR(primary_state);
>
> if (!set->mode) {
> WARN_ON(set->fb);
> @@ -1657,13 +1706,13 @@ retry:
>
> ret = drm_atomic_set_mode_for_crtc(crtc_state, NULL);
> if (ret != 0)
> - goto fail;
> + return ret;
>
> crtc_state->active = false;
>
> ret = drm_atomic_set_crtc_for_plane(primary_state, NULL);
> if (ret != 0)
> - goto fail;
> + return ret;
>
> drm_atomic_set_fb_for_plane(primary_state, NULL);
>
> @@ -1675,13 +1724,14 @@ retry:
>
> ret = drm_atomic_set_mode_for_crtc(crtc_state, set->mode);
> if (ret != 0)
> - goto fail;
> + return ret;
>
> crtc_state->active = true;
>
> ret = drm_atomic_set_crtc_for_plane(primary_state, crtc);
> if (ret != 0)
> - goto fail;
> + return ret;
> +
> drm_atomic_set_fb_for_plane(primary_state, set->fb);
> primary_state->crtc_x = 0;
> primary_state->crtc_y = 0;
> @@ -1695,35 +1745,10 @@ retry:
> commit:
> ret = update_output_state(state, set);
> if (ret)
> - goto fail;
> -
> - ret = drm_atomic_commit(state);
> - if (ret != 0)
> - goto fail;
> + return ret;
>
> - /* Driver takes ownership of state on successful commit. */
> return 0;
> -fail:
> - if (ret == -EDEADLK)
> - goto backoff;
> -
> - drm_atomic_state_free(state);
> -
> - 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.
> - */
> - crtc->primary->old_fb = crtc->primary->fb;
> -
> - goto retry;
> }
> -EXPORT_SYMBOL(drm_atomic_helper_set_config);
>
> /**
> * drm_atomic_helper_crtc_set_property - helper for crtc properties
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index ba12f51..9741d79 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -38,6 +38,8 @@
> #include <drm/drm_crtc.h>
> #include <drm/drm_fb_helper.h>
> #include <drm/drm_crtc_helper.h>
> +#include <drm/drm_atomic.h>
> +#include <drm/drm_atomic_helper.h>
>
> static bool drm_fbdev_emulation = true;
> module_param_named(fbdev_emulation, drm_fbdev_emulation, bool, 0600);
> @@ -334,6 +336,73 @@ int drm_fb_helper_debug_leave(struct fb_info *info)
> }
> EXPORT_SYMBOL(drm_fb_helper_debug_leave);
>
> +static int restore_fbdev_mode_atomic(struct drm_fb_helper *fb_helper)
> +{
> + struct drm_device *dev = fb_helper->dev;
> + struct drm_plane *plane;
> + struct drm_atomic_state *state;
> + int i, ret;
> +
> + state = drm_atomic_state_alloc(dev);
> + if (!state)
> + return -ENOMEM;
> +
> + state->acquire_ctx = dev->mode_config.acquire_ctx;
> +retry:
> + drm_for_each_plane(plane, dev) {
> + struct drm_plane_state *plane_state;
> +
> + plane_state = drm_atomic_get_plane_state(state, plane);
> + if (IS_ERR(plane_state)) {
> + ret = PTR_ERR(plane_state);
> + goto fail;
> + }
> +
> + /* reset rotation: */
Comment is superflous imo. And we need to check for rotation_property here
like for the legacy path otherwise we'll blow up if that's not set up.
> + ret = drm_atomic_plane_set_property(plane, plane_state,
> + dev->mode_config.rotation_property,
> + BIT(DRM_ROTATE_0));
> + if (ret != 0)
> + goto fail;
> +
> + /* disable non-primary: */
> + if (plane->type == DRM_PLANE_TYPE_PRIMARY)
> + continue;
> +
> + ret = __drm_atomic_helper_disable_plane(plane, plane_state);
> + if (ret != 0)
> + goto fail;
> + }
> +
> + for(i = 0; i < fb_helper->crtc_count; i++) {
> + struct drm_mode_set *mode_set =
> &fb_helper->crtc_info[i].mode_set;
> +
> + ret = __drm_atomic_helper_set_config(mode_set, state);
> + if (ret != 0)
> + goto fail;
> + }
> +
> + ret = drm_atomic_commit(state);
> + if (ret != 0)
> + goto fail;
> +
> + return 0;
> +
> +fail:
> + if (ret == -EDEADLK)
> + goto backoff;
> +
> + drm_atomic_state_free(state);
> +
> + return ret;
> +
> +backoff:
> + drm_atomic_state_clear(state);
> + drm_atomic_legacy_backoff(state);
> +
> + goto retry;
> +}
> +
> static int restore_fbdev_mode(struct drm_fb_helper *fb_helper)
> {
> struct drm_device *dev = fb_helper->dev;
> @@ -342,6 +411,9 @@ static int restore_fbdev_mode(struct drm_fb_helper
> *fb_helper)
>
> drm_warn_on_modeset_not_all_locked(dev);
>
> + if (fb_helper->atomic)
> + return restore_fbdev_mode_atomic(fb_helper);
> +
> drm_for_each_plane(plane, dev) {
> if (plane->type != DRM_PLANE_TYPE_PRIMARY)
> drm_plane_force_disable(plane);
> @@ -644,6 +716,8 @@ int drm_fb_helper_init(struct drm_device *dev,
> i++;
> }
>
> + fb_helper->atomic = !!drm_core_check_feature(dev, DRIVER_ATOMIC);
> +
> return 0;
> out_free:
> drm_fb_helper_crtc_free(fb_helper);
> diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h
> index 11266d14..88c0ef3 100644
> --- a/include/drm/drm_atomic_helper.h
> +++ b/include/drm/drm_atomic_helper.h
> @@ -30,6 +30,8 @@
>
> #include <drm/drm_crtc.h>
>
> +struct drm_atomic_state;
> +
> int drm_atomic_helper_check_modeset(struct drm_device *dev,
> struct drm_atomic_state *state);
> int drm_atomic_helper_check_planes(struct drm_device *dev,
> @@ -72,7 +74,11 @@ int drm_atomic_helper_update_plane(struct drm_plane *plane,
> uint32_t src_x, uint32_t src_y,
> uint32_t src_w, uint32_t src_h);
> int drm_atomic_helper_disable_plane(struct drm_plane *plane);
> +int __drm_atomic_helper_disable_plane(struct drm_plane *plane,
> + struct drm_plane_state *plane_state);
> int drm_atomic_helper_set_config(struct drm_mode_set *set);
> +int __drm_atomic_helper_set_config(struct drm_mode_set *set,
> + struct drm_atomic_state *state);
>
> int drm_atomic_helper_crtc_set_property(struct drm_crtc *crtc,
> struct drm_property *property,
> diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h
> index 6254136..cc3d820 100644
> --- a/include/drm/drm_fb_helper.h
> +++ b/include/drm/drm_fb_helper.h
> @@ -117,6 +117,12 @@ struct drm_fb_helper_connector {
> * @pseudo_palette: fake palette of 16 colors
> * @kernel_fb_list: list_head in kernel_fb_helper_list
> * @delayed_hotplug: was there a hotplug while kms master active?
> + * @atomic: use atomic updates for restore_fbdev_mode(), etc. This
> + * defaults to true if driver has DRIVER_ATOMIC feature
> + * flag, but drivers can override it to true after
> + * drm_fb_helper_init() if they support atomic modeset
> + * but do not yet advertise DRIVER_ATOMIC (note that
> + * fb-helper does not require ASYNC commits).
New kerneldoc in 4.3 (already in linux-next and drm-intel-nightly) allows
you to push that right to the struct member, which is imo better for
longer explanations like this:
> */
> struct drm_fb_helper {
> struct drm_framebuffer *fb;
> @@ -134,6 +140,8 @@ struct drm_fb_helper {
> /* we got a hotplug but fbdev wasn't running the console
> delay until next set_par */
> bool delayed_hotplug;
/**
* @atomic:
*
* Use atomic updates for restore_fbdev_mode(), etc. This defaults to
* true if driver has DRIVER_ATOMIC feature flag, but drivers can
override
* it to true after drm_fb_helper_init() if they support atomic modeset
* but do not yet advertise DRIVER_ATOMIC (note that fb-helper does not
* require ASYNC commits).
*/
bool atomic;
Imo that looks prettier.
-Daniel
> };
>
> #ifdef CONFIG_DRM_FBDEV_EMULATION
> --
> 2.4.3
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
