On Tue, Feb 21, 2017 at 02:51:42PM +0100, Maarten Lankhorst wrote:
> This introduces a slight behavioral change to rmfb. Instead of
> disabling a crtc when the primary plane is disabled, we try to
> preserve it.
>
> Apart from old versions of the vmwgfx xorg driver, there is
> nothing depending on rmfb disabling a crtc. Since vmwgfx is
> a legacy driver we can safely only disable the plane with atomic.
> In the conversion to atomic the driver will reject the config
> without primary plane.
>
> If this commit is rejected by the driver then we will still fall
> back to the old behavior and turn off the crtc.
>
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx>
> ---
> drivers/gpu/drm/drm_atomic.c | 22 ++++++++++++++++++++--
> 1 file changed, 20 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 285e1c23e8c9..0d5b0065208e 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -2070,7 +2070,7 @@ int drm_atomic_remove_fb(struct drm_framebuffer *fb)
> struct drm_connector *conn;
> struct drm_connector_state *conn_state;
> int i, ret = 0;
> - unsigned plane_mask;
> + unsigned plane_mask, disable_crtcs = false;
>
> state = drm_atomic_state_alloc(dev);
> if (!state)
> @@ -2097,7 +2097,13 @@ int drm_atomic_remove_fb(struct drm_framebuffer *fb)
> goto unlock;
> }
>
> - if (plane_state->crtc->primary == plane) {
> + /*
> + * Some drivers do not support keeping crtc active with the
> + * primary plane disabled. If we fail to commit with -EINVAL
> + * then we will try to perform the same commit but with all
> + * crtc's disabled for primary planes as well.
> + */
> + if (disable_crtcs && plane_state->crtc->primary == plane) {
> struct drm_crtc_state *crtc_state;
>
> crtc_state = drm_atomic_get_existing_crtc_state(state, plane_state->crtc);
> @@ -2122,6 +2128,7 @@ int drm_atomic_remove_fb(struct drm_framebuffer *fb)
> plane->old_fb = plane->fb;
> }
>
> + /* This list is only not empty when disable_crtcs is set. */
> for_each_connector_in_state(state, conn, conn_state, i) {
> ret = drm_atomic_set_crtc_for_connector(conn_state, NULL);
>
> @@ -2143,6 +2150,17 @@ int drm_atomic_remove_fb(struct drm_framebuffer *fb)
>
> drm_atomic_state_put(state);
>
> + if (ret == -EINVAL && !disable_crtcs) {
> + disable_crtcs = true;
> +
> + state = drm_atomic_state_alloc(dev);
> + if (state) {
> + state->acquire_ctx = &ctx;
> + goto retry;
Re-using the retry label is a little fishy here. You need to re-allocate state,
but we don't drop the locks, which makes things a little unbalanced (I realize
modeset_lock will s/EALREADY/0/).
Perhaps something like this above would be more clear:
+ drm_modeset_acquire_init(&ctx, 0);
+retry:
+ plane_mask = 0;
+ ret = drm_modeset_lock_all_ctx(dev, &ctx);
+ if (ret)
+ goto unlock;
+
+retry_disable:
+ state = drm_atomic_state_alloc(dev);
+ if (!state)
+ return -ENOMEM;
+ state->acquire_ctx = &ctx;
+
Then you can do:
+ if (ret == -EINVAL && !disable_crtcs) {
+ disable_crtcs = true;
+ goto retry_disable;
+ }
(you would also need to ensure you moved the state_put above the EDEADLK
handling).
Thoughts?
Sean
> + }
> + ret = -ENOMEM;
> + }
> +
> drm_modeset_drop_locks(&ctx);
> drm_modeset_acquire_fini(&ctx);
>
> --
> 2.7.4
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Sean Paul, Software Engineer, Google / Chromium OS
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
