| Message ID | 1487685102-31991-3-git-send-email-maarten.lankhorst@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Feb 21, 2017 at 02:51:41PM +0100, Maarten Lankhorst wrote: > Instead of trying to do everything in 1 go, just do a basic safe > conversion first. We've been bitten by too many regressions in the > past. > > This patch only converts drm_framebuffer_remove to atomic. The > regression sensitive part is split out to a separate patch. > > v2: > - Remove plane->fb assignment, done by drm_atomic_clean_old_fb. > - Add WARN_ON when atomic_remove_fb fails. > - Always call drm_atomic_state_put. > v3: > - Use drm_drv_uses_atomic_modeset > - Handle the case where the first plane-disable-only commit fails > with -EINVAL. Some drivers do not support this, fall back to > disabling all crtc's in this case. > v4: > - Solve vmwgfx compatibility issue in their driver, was fixed in this > patch by v3. > - Move only disabling primary to a separate patch. > Reviewed-by: Sean Paul <seanpaul@chromium.org> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> > Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > --- > drivers/gpu/drm/drm_atomic.c | 88 +++++++++++++++++++++++++++++++++++++ > drivers/gpu/drm/drm_crtc_internal.h | 1 + > drivers/gpu/drm/drm_framebuffer.c | 7 +++ > 3 files changed, 96 insertions(+) > > diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c > index afec53832145..285e1c23e8c9 100644 > --- a/drivers/gpu/drm/drm_atomic.c > +++ b/drivers/gpu/drm/drm_atomic.c > @@ -2061,6 +2061,94 @@ static void complete_crtc_signaling(struct drm_device *dev, > kfree(fence_state); > } > > +int drm_atomic_remove_fb(struct drm_framebuffer *fb) > +{ > + struct drm_modeset_acquire_ctx ctx; > + struct drm_device *dev = fb->dev; > + struct drm_atomic_state *state; > + struct drm_plane *plane; > + struct drm_connector *conn; > + struct drm_connector_state *conn_state; > + int i, ret = 0; > + unsigned plane_mask; > + > + state = drm_atomic_state_alloc(dev); > + if (!state) > + return -ENOMEM; > + > + drm_modeset_acquire_init(&ctx, 0); > + state->acquire_ctx = &ctx; > + > +retry: > + plane_mask = 0; > + ret = drm_modeset_lock_all_ctx(dev, &ctx); > + if (ret) > + goto unlock; > + > + drm_for_each_plane(plane, dev) { > + struct drm_plane_state *plane_state; > + > + if (plane->state->fb != fb) > + continue; > + > + plane_state = drm_atomic_get_plane_state(state, plane); > + if (IS_ERR(plane_state)) { > + ret = PTR_ERR(plane_state); > + goto unlock; > + } > + > + if (plane_state->crtc->primary == plane) { > + struct drm_crtc_state *crtc_state; > + > + crtc_state = drm_atomic_get_existing_crtc_state(state, plane_state->crtc); > + > + ret = drm_atomic_add_affected_connectors(state, plane_state->crtc); > + if (ret) > + goto unlock; > + > + crtc_state->active = false; > + ret = drm_atomic_set_mode_for_crtc(crtc_state, NULL); > + if (ret) > + goto unlock; > + } > + > + drm_atomic_set_fb_for_plane(plane_state, NULL); > + ret = drm_atomic_set_crtc_for_plane(plane_state, NULL); > + if (ret) > + goto unlock; > + > + plane_mask |= BIT(drm_plane_index(plane)); > + > + plane->old_fb = plane->fb; > + } > + > + for_each_connector_in_state(state, conn, conn_state, i) { > + ret = drm_atomic_set_crtc_for_connector(conn_state, NULL); > + > + if (ret) > + goto unlock; > + } > + > + if (plane_mask) > + ret = drm_atomic_commit(state); > + > +unlock: > + if (plane_mask) > + drm_atomic_clean_old_fb(dev, plane_mask, ret); > + > + if (ret == -EDEADLK) { > + drm_modeset_backoff(&ctx); > + goto retry; > + } > + > + drm_atomic_state_put(state); > + > + drm_modeset_drop_locks(&ctx); > + drm_modeset_acquire_fini(&ctx); > + > + return ret; > +} > + > int drm_mode_atomic_ioctl(struct drm_device *dev, > void *data, struct drm_file *file_priv) > { > diff --git a/drivers/gpu/drm/drm_crtc_internal.h b/drivers/gpu/drm/drm_crtc_internal.h > index 955c5690bf64..e0678f8a51cf 100644 > --- a/drivers/gpu/drm/drm_crtc_internal.h > +++ b/drivers/gpu/drm/drm_crtc_internal.h > @@ -183,6 +183,7 @@ int drm_atomic_get_property(struct drm_mode_object *obj, > struct drm_property *property, uint64_t *val); > int drm_mode_atomic_ioctl(struct drm_device *dev, > void *data, struct drm_file *file_priv); > +int drm_atomic_remove_fb(struct drm_framebuffer *fb); > > > /* drm_plane.c */ > diff --git a/drivers/gpu/drm/drm_framebuffer.c b/drivers/gpu/drm/drm_framebuffer.c > index 28a0108a1ab8..c0e593a7f9b4 100644 > --- a/drivers/gpu/drm/drm_framebuffer.c > +++ b/drivers/gpu/drm/drm_framebuffer.c > @@ -773,6 +773,12 @@ void drm_framebuffer_remove(struct drm_framebuffer *fb) > * in this manner. > */ > if (drm_framebuffer_read_refcount(fb) > 1) { > + if (drm_drv_uses_atomic_modeset(dev)) { > + int ret = drm_atomic_remove_fb(fb); > + WARN(ret, "atomic remove_fb failed with %i\n", ret); > + goto out; > + } > + > drm_modeset_lock_all(dev); > /* remove from any CRTC */ > drm_for_each_crtc(crtc, dev) { > @@ -790,6 +796,7 @@ void drm_framebuffer_remove(struct drm_framebuffer *fb) > drm_modeset_unlock_all(dev); > } > > +out: > drm_framebuffer_unreference(fb); > } > EXPORT_SYMBOL(drm_framebuffer_remove); > -- > 2.7.4 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
On Thu, Feb 23, 2017 at 10:09:27AM -0500, Sean Paul wrote: > On Tue, Feb 21, 2017 at 02:51:41PM +0100, Maarten Lankhorst wrote: > > Instead of trying to do everything in 1 go, just do a basic safe > > conversion first. We've been bitten by too many regressions in the > > past. > > > > This patch only converts drm_framebuffer_remove to atomic. The > > regression sensitive part is split out to a separate patch. > > > > v2: > > - Remove plane->fb assignment, done by drm_atomic_clean_old_fb. > > - Add WARN_ON when atomic_remove_fb fails. > > - Always call drm_atomic_state_put. > > v3: > > - Use drm_drv_uses_atomic_modeset > > - Handle the case where the first plane-disable-only commit fails > > with -EINVAL. Some drivers do not support this, fall back to > > disabling all crtc's in this case. > > v4: > > - Solve vmwgfx compatibility issue in their driver, was fixed in this > > patch by v3. > > - Move only disabling primary to a separate patch. > > > > Reviewed-by: Sean Paul <seanpaul@chromium.org> Ok, merged the first 2 from this series to drm-misc-next. Thanks, Daniel > > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> > > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> > > Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > > --- > > drivers/gpu/drm/drm_atomic.c | 88 +++++++++++++++++++++++++++++++++++++ > > drivers/gpu/drm/drm_crtc_internal.h | 1 + > > drivers/gpu/drm/drm_framebuffer.c | 7 +++ > > 3 files changed, 96 insertions(+) > > > > diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c > > index afec53832145..285e1c23e8c9 100644 > > --- a/drivers/gpu/drm/drm_atomic.c > > +++ b/drivers/gpu/drm/drm_atomic.c > > @@ -2061,6 +2061,94 @@ static void complete_crtc_signaling(struct drm_device *dev, > > kfree(fence_state); > > } > > > > +int drm_atomic_remove_fb(struct drm_framebuffer *fb) > > +{ > > + struct drm_modeset_acquire_ctx ctx; > > + struct drm_device *dev = fb->dev; > > + struct drm_atomic_state *state; > > + struct drm_plane *plane; > > + struct drm_connector *conn; > > + struct drm_connector_state *conn_state; > > + int i, ret = 0; > > + unsigned plane_mask; > > + > > + state = drm_atomic_state_alloc(dev); > > + if (!state) > > + return -ENOMEM; > > + > > + drm_modeset_acquire_init(&ctx, 0); > > + state->acquire_ctx = &ctx; > > + > > +retry: > > + plane_mask = 0; > > + ret = drm_modeset_lock_all_ctx(dev, &ctx); > > + if (ret) > > + goto unlock; > > + > > + drm_for_each_plane(plane, dev) { > > + struct drm_plane_state *plane_state; > > + > > + if (plane->state->fb != fb) > > + continue; > > + > > + plane_state = drm_atomic_get_plane_state(state, plane); > > + if (IS_ERR(plane_state)) { > > + ret = PTR_ERR(plane_state); > > + goto unlock; > > + } > > + > > + if (plane_state->crtc->primary == plane) { > > + struct drm_crtc_state *crtc_state; > > + > > + crtc_state = drm_atomic_get_existing_crtc_state(state, plane_state->crtc); > > + > > + ret = drm_atomic_add_affected_connectors(state, plane_state->crtc); > > + if (ret) > > + goto unlock; > > + > > + crtc_state->active = false; > > + ret = drm_atomic_set_mode_for_crtc(crtc_state, NULL); > > + if (ret) > > + goto unlock; > > + } > > + > > + drm_atomic_set_fb_for_plane(plane_state, NULL); > > + ret = drm_atomic_set_crtc_for_plane(plane_state, NULL); > > + if (ret) > > + goto unlock; > > + > > + plane_mask |= BIT(drm_plane_index(plane)); > > + > > + plane->old_fb = plane->fb; > > + } > > + > > + for_each_connector_in_state(state, conn, conn_state, i) { > > + ret = drm_atomic_set_crtc_for_connector(conn_state, NULL); > > + > > + if (ret) > > + goto unlock; > > + } > > + > > + if (plane_mask) > > + ret = drm_atomic_commit(state); > > + > > +unlock: > > + if (plane_mask) > > + drm_atomic_clean_old_fb(dev, plane_mask, ret); > > + > > + if (ret == -EDEADLK) { > > + drm_modeset_backoff(&ctx); > > + goto retry; > > + } > > + > > + drm_atomic_state_put(state); > > + > > + drm_modeset_drop_locks(&ctx); > > + drm_modeset_acquire_fini(&ctx); > > + > > + return ret; > > +} > > + > > int drm_mode_atomic_ioctl(struct drm_device *dev, > > void *data, struct drm_file *file_priv) > > { > > diff --git a/drivers/gpu/drm/drm_crtc_internal.h b/drivers/gpu/drm/drm_crtc_internal.h > > index 955c5690bf64..e0678f8a51cf 100644 > > --- a/drivers/gpu/drm/drm_crtc_internal.h > > +++ b/drivers/gpu/drm/drm_crtc_internal.h > > @@ -183,6 +183,7 @@ int drm_atomic_get_property(struct drm_mode_object *obj, > > struct drm_property *property, uint64_t *val); > > int drm_mode_atomic_ioctl(struct drm_device *dev, > > void *data, struct drm_file *file_priv); > > +int drm_atomic_remove_fb(struct drm_framebuffer *fb); > > > > > > /* drm_plane.c */ > > diff --git a/drivers/gpu/drm/drm_framebuffer.c b/drivers/gpu/drm/drm_framebuffer.c > > index 28a0108a1ab8..c0e593a7f9b4 100644 > > --- a/drivers/gpu/drm/drm_framebuffer.c > > +++ b/drivers/gpu/drm/drm_framebuffer.c > > @@ -773,6 +773,12 @@ void drm_framebuffer_remove(struct drm_framebuffer *fb) > > * in this manner. > > */ > > if (drm_framebuffer_read_refcount(fb) > 1) { > > + if (drm_drv_uses_atomic_modeset(dev)) { > > + int ret = drm_atomic_remove_fb(fb); > > + WARN(ret, "atomic remove_fb failed with %i\n", ret); > > + goto out; > > + } > > + > > drm_modeset_lock_all(dev); > > /* remove from any CRTC */ > > drm_for_each_crtc(crtc, dev) { > > @@ -790,6 +796,7 @@ void drm_framebuffer_remove(struct drm_framebuffer *fb) > > drm_modeset_unlock_all(dev); > > } > > > > +out: > > drm_framebuffer_unreference(fb); > > } > > EXPORT_SYMBOL(drm_framebuffer_remove); > > -- > > 2.7.4 > > > > _______________________________________________ > > dri-devel mailing list > > dri-devel@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/dri-devel > > -- > Sean Paul, Software Engineer, Google / Chromium OS
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c index afec53832145..285e1c23e8c9 100644 --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -2061,6 +2061,94 @@ static void complete_crtc_signaling(struct drm_device *dev, kfree(fence_state); } +int drm_atomic_remove_fb(struct drm_framebuffer *fb) +{ + struct drm_modeset_acquire_ctx ctx; + struct drm_device *dev = fb->dev; + struct drm_atomic_state *state; + struct drm_plane *plane; + struct drm_connector *conn; + struct drm_connector_state *conn_state; + int i, ret = 0; + unsigned plane_mask; + + state = drm_atomic_state_alloc(dev); + if (!state) + return -ENOMEM; + + drm_modeset_acquire_init(&ctx, 0); + state->acquire_ctx = &ctx; + +retry: + plane_mask = 0; + ret = drm_modeset_lock_all_ctx(dev, &ctx); + if (ret) + goto unlock; + + drm_for_each_plane(plane, dev) { + struct drm_plane_state *plane_state; + + if (plane->state->fb != fb) + continue; + + plane_state = drm_atomic_get_plane_state(state, plane); + if (IS_ERR(plane_state)) { + ret = PTR_ERR(plane_state); + goto unlock; + } + + if (plane_state->crtc->primary == plane) { + struct drm_crtc_state *crtc_state; + + crtc_state = drm_atomic_get_existing_crtc_state(state, plane_state->crtc); + + ret = drm_atomic_add_affected_connectors(state, plane_state->crtc); + if (ret) + goto unlock; + + crtc_state->active = false; + ret = drm_atomic_set_mode_for_crtc(crtc_state, NULL); + if (ret) + goto unlock; + } + + drm_atomic_set_fb_for_plane(plane_state, NULL); + ret = drm_atomic_set_crtc_for_plane(plane_state, NULL); + if (ret) + goto unlock; + + plane_mask |= BIT(drm_plane_index(plane)); + + plane->old_fb = plane->fb; + } + + for_each_connector_in_state(state, conn, conn_state, i) { + ret = drm_atomic_set_crtc_for_connector(conn_state, NULL); + + if (ret) + goto unlock; + } + + if (plane_mask) + ret = drm_atomic_commit(state); + +unlock: + if (plane_mask) + drm_atomic_clean_old_fb(dev, plane_mask, ret); + + if (ret == -EDEADLK) { + drm_modeset_backoff(&ctx); + goto retry; + } + + drm_atomic_state_put(state); + + drm_modeset_drop_locks(&ctx); + drm_modeset_acquire_fini(&ctx); + + return ret; +} + int drm_mode_atomic_ioctl(struct drm_device *dev, void *data, struct drm_file *file_priv) { diff --git a/drivers/gpu/drm/drm_crtc_internal.h b/drivers/gpu/drm/drm_crtc_internal.h index 955c5690bf64..e0678f8a51cf 100644 --- a/drivers/gpu/drm/drm_crtc_internal.h +++ b/drivers/gpu/drm/drm_crtc_internal.h @@ -183,6 +183,7 @@ int drm_atomic_get_property(struct drm_mode_object *obj, struct drm_property *property, uint64_t *val); int drm_mode_atomic_ioctl(struct drm_device *dev, void *data, struct drm_file *file_priv); +int drm_atomic_remove_fb(struct drm_framebuffer *fb); /* drm_plane.c */ diff --git a/drivers/gpu/drm/drm_framebuffer.c b/drivers/gpu/drm/drm_framebuffer.c index 28a0108a1ab8..c0e593a7f9b4 100644 --- a/drivers/gpu/drm/drm_framebuffer.c +++ b/drivers/gpu/drm/drm_framebuffer.c @@ -773,6 +773,12 @@ void drm_framebuffer_remove(struct drm_framebuffer *fb) * in this manner. */ if (drm_framebuffer_read_refcount(fb) > 1) { + if (drm_drv_uses_atomic_modeset(dev)) { + int ret = drm_atomic_remove_fb(fb); + WARN(ret, "atomic remove_fb failed with %i\n", ret); + goto out; + } + drm_modeset_lock_all(dev); /* remove from any CRTC */ drm_for_each_crtc(crtc, dev) { @@ -790,6 +796,7 @@ void drm_framebuffer_remove(struct drm_framebuffer *fb) drm_modeset_unlock_all(dev); } +out: drm_framebuffer_unreference(fb); } EXPORT_SYMBOL(drm_framebuffer_remove);