[PATCH 0/5] drm/sti: do not sync legacy IOCTL on vblank if not ATOMIC

[PATCH 0/5] drm/sti: do not sync legacy IOCTL on vblank if not ATOMIC

[Fabien Dessenne]

These patches allow a legacy framework (eg non-atomic Weston) to call
several SETPLANE within the same Vsync cycle.
- [PATCH 1/5] drm/sti: use atomic_helper for commit
- [PATCH 2/5] drm/sti: add drm_file to sti_private
- [PATCH 3/5] drm/sti: do not sync SETPLANE on vblank if not ATOMIC
- [PATCH 4/5] drm/sti: do not sync SETPROPERTY on vblank if not ATOMIC
- [PATCH 5/5] drm/sti: do not check hw scaling if mode is not set

Fabien

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 1/5] drm/sti: use atomic_helper for commit

[Fabien Dessenne]

Since nonblocking atomic commits are now supported, the driver can
now use drm_atomic_helper_commit().

Change-Id: I3e49872b0dc9e79ca652bec7e5cd29d912c86382
Signed-off-by: Fabien Dessenne <fabien.dessenne@st.com>
---
 drivers/gpu/drm/sti/sti_drv.c | 83 +------------------------------------------
 drivers/gpu/drm/sti/sti_drv.h |  6 ----
 2 files changed, 1 insertion(+), 88 deletions(-)

diff --git a/drivers/gpu/drm/sti/sti_drv.c b/drivers/gpu/drm/sti/sti_drv.c
index ff71e25..9a9b9a2 100644
--- a/drivers/gpu/drm/sti/sti_drv.c
+++ b/drivers/gpu/drm/sti/sti_drv.c
@@ -150,52 +150,6 @@ static void sti_drm_dbg_cleanup(struct drm_minor *minor)
 				 1, minor);
 }
 
-static void sti_atomic_schedule(struct sti_private *private,
-				struct drm_atomic_state *state)
-{
-	private->commit.state = state;
-	schedule_work(&private->commit.work);
-}
-
-static void sti_atomic_complete(struct sti_private *private,
-				struct drm_atomic_state *state)
-{
-	struct drm_device *drm = private->drm_dev;
-
-	/*
-	 * Everything below can be run asynchronously without the need to grab
-	 * any modeset locks at all under one condition: It must be guaranteed
-	 * that the asynchronous work has either been cancelled (if the driver
-	 * supports it, which at least requires that the framebuffers get
-	 * cleaned up with drm_atomic_helper_cleanup_planes()) or completed
-	 * before the new state gets committed on the software side with
-	 * drm_atomic_helper_swap_state().
-	 *
-	 * This scheme allows new atomic state updates to be prepared and
-	 * checked in parallel to the asynchronous completion of the previous
-	 * update. Which is important since compositors need to figure out the
-	 * composition of the next frame right after having submitted the
-	 * current layout.
-	 */
-
-	drm_atomic_helper_commit_modeset_disables(drm, state);
-	drm_atomic_helper_commit_planes(drm, state, 0);
-	drm_atomic_helper_commit_modeset_enables(drm, state);
-
-	drm_atomic_helper_wait_for_vblanks(drm, state);
-
-	drm_atomic_helper_cleanup_planes(drm, state);
-	drm_atomic_state_put(state);
-}
-
-static void sti_atomic_work(struct work_struct *work)
-{
-	struct sti_private *private = container_of(work,
-			struct sti_private, commit.work);
-
-	sti_atomic_complete(private, private->commit.state);
-}
-
 static int sti_atomic_check(struct drm_device *dev,
 			    struct drm_atomic_state *state)
 {
@@ -216,38 +170,6 @@ static int sti_atomic_check(struct drm_device *dev,
 	return ret;
 }
 
-static int sti_atomic_commit(struct drm_device *drm,
-			     struct drm_atomic_state *state, bool nonblock)
-{
-	struct sti_private *private = drm->dev_private;
-	int err;
-
-	err = drm_atomic_helper_prepare_planes(drm, state);
-	if (err)
-		return err;
-
-	/* serialize outstanding nonblocking commits */
-	mutex_lock(&private->commit.lock);
-	flush_work(&private->commit.work);
-
-	/*
-	 * This is the point of no return - everything below never fails except
-	 * when the hw goes bonghits. Which means we can commit the new state on
-	 * the software side now.
-	 */
-
-	drm_atomic_helper_swap_state(state, true);
-
-	drm_atomic_state_get(state);
-	if (nonblock)
-		sti_atomic_schedule(private, state);
-	else
-		sti_atomic_complete(private, state);
-
-	mutex_unlock(&private->commit.lock);
-	return 0;
-}
-
 static void sti_output_poll_changed(struct drm_device *ddev)
 {
 	struct sti_private *private = ddev->dev_private;
@@ -271,7 +193,7 @@ static const struct drm_mode_config_funcs sti_mode_config_funcs = {
 	.fb_create = drm_fb_cma_create,
 	.output_poll_changed = sti_output_poll_changed,
 	.atomic_check = sti_atomic_check,
-	.atomic_commit = sti_atomic_commit,
+	.atomic_commit = drm_atomic_helper_commit,
 };
 
 static void sti_mode_config_init(struct drm_device *dev)
@@ -352,9 +274,6 @@ static int sti_init(struct drm_device *ddev)
 	dev_set_drvdata(ddev->dev, ddev);
 	private->drm_dev = ddev;
 
-	mutex_init(&private->commit.lock);
-	INIT_WORK(&private->commit.work, sti_atomic_work);
-
 	drm_mode_config_init(ddev);
 
 	sti_mode_config_init(ddev);
diff --git a/drivers/gpu/drm/sti/sti_drv.h b/drivers/gpu/drm/sti/sti_drv.h
index 78ebe5e..f9276cd 100644
--- a/drivers/gpu/drm/sti/sti_drv.h
+++ b/drivers/gpu/drm/sti/sti_drv.h
@@ -25,12 +25,6 @@ struct sti_private {
 	struct drm_property *plane_zorder_property;
 	struct drm_device *drm_dev;
 	struct drm_fbdev_cma *fbdev;
-
-	struct {
-		struct drm_atomic_state *state;
-		struct work_struct work;
-		struct mutex lock;
-	} commit;
 };
 
 extern struct platform_driver sti_tvout_driver;
-- 
2.7.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 2/5] drm/sti: add drm_file to sti_private

[Fabien Dessenne]

Store the drm_file *filp in sti_private, so the driver can access more
configuration information like the client capabilities.

Change-Id: Ib8f305f1a41b4fdfe56f80294cd79e5dc44433ee
Signed-off-by: Fabien Dessenne <fabien.dessenne@st.com>
---
 drivers/gpu/drm/sti/sti_drv.c | 10 ++++++++++
 drivers/gpu/drm/sti/sti_drv.h |  3 +++
 2 files changed, 13 insertions(+)

diff --git a/drivers/gpu/drm/sti/sti_drv.c b/drivers/gpu/drm/sti/sti_drv.c
index 9a9b9a2..63febf8 100644
--- a/drivers/gpu/drm/sti/sti_drv.c
+++ b/drivers/gpu/drm/sti/sti_drv.c
@@ -170,6 +170,15 @@ static int sti_atomic_check(struct drm_device *dev,
 	return ret;
 }
 
+static int sti_drm_open(struct drm_device *ddev, struct drm_file *filp)
+{
+	struct sti_private *private = ddev->dev_private;
+
+	private->filp = filp;
+
+	return 0;
+}
+
 static void sti_output_poll_changed(struct drm_device *ddev)
 {
 	struct sti_private *private = ddev->dev_private;
@@ -226,6 +235,7 @@ static const struct file_operations sti_driver_fops = {
 static struct drm_driver sti_driver = {
 	.driver_features = DRIVER_MODESET |
 	    DRIVER_GEM | DRIVER_PRIME | DRIVER_ATOMIC,
+	.open = sti_drm_open,
 	.gem_free_object_unlocked = drm_gem_cma_free_object,
 	.gem_vm_ops = &drm_gem_cma_vm_ops,
 	.dumb_create = drm_gem_cma_dumb_create,
diff --git a/drivers/gpu/drm/sti/sti_drv.h b/drivers/gpu/drm/sti/sti_drv.h
index f9276cd..de22b0d 100644
--- a/drivers/gpu/drm/sti/sti_drv.h
+++ b/drivers/gpu/drm/sti/sti_drv.h
@@ -19,12 +19,15 @@ struct sti_tvout;
  * @compo:                 compositor
  * @plane_zorder_property: z-order property for CRTC planes
  * @drm_dev:               drm device
+ * @fbdev:                 framebuffer dev cma struct
+ * @drm_file:              drm file private data
  */
 struct sti_private {
 	struct sti_compositor *compo;
 	struct drm_property *plane_zorder_property;
 	struct drm_device *drm_dev;
 	struct drm_fbdev_cma *fbdev;
+	struct drm_file *filp;
 };
 
 extern struct platform_driver sti_tvout_driver;
-- 
2.7.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 3/5] drm/sti: do not sync SETPLANE on vblank if not ATOMIC

[Fabien Dessenne]

If the client does not set the ATOMIC capability, do not wait for vblank
before returning an DRM_IOCTL_MODE_SETPLANE call.

In this way, a legacy framework (eg non-atomic Weston) can call several
SETPLANE within the same Vsync cycle.

This is implemented by setting the legacy_cursor_update flag, to behave
the same way as DRM_IOCTL_MODE_CURSOR (not vblank synced).

Change-Id: Ia241b6c88411c675bf589c17d4a44db6d02f669f
Signed-off-by: Fabien Dessenne <fabien.dessenne@st.com>
---
 drivers/gpu/drm/sti/sti_cursor.c |   4 +-
 drivers/gpu/drm/sti/sti_gdp.c    |   4 +-
 drivers/gpu/drm/sti/sti_hqvdp.c  |   4 +-
 drivers/gpu/drm/sti/sti_plane.c  | 144 +++++++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/sti/sti_plane.h  |   8 +++
 5 files changed, 158 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/sti/sti_cursor.c b/drivers/gpu/drm/sti/sti_cursor.c
index cca75bd..ea0dbae 100644
--- a/drivers/gpu/drm/sti/sti_cursor.c
+++ b/drivers/gpu/drm/sti/sti_cursor.c
@@ -346,8 +346,8 @@ static int sti_cursor_late_register(struct drm_plane *drm_plane)
 }
 
 static const struct drm_plane_funcs sti_cursor_plane_helpers_funcs = {
-	.update_plane = drm_atomic_helper_update_plane,
-	.disable_plane = drm_atomic_helper_disable_plane,
+	.update_plane = sti_plane_update_plane,
+	.disable_plane = sti_plane_disable_plane,
 	.destroy = sti_cursor_destroy,
 	.set_property = drm_atomic_helper_plane_set_property,
 	.reset = sti_plane_reset,
diff --git a/drivers/gpu/drm/sti/sti_gdp.c b/drivers/gpu/drm/sti/sti_gdp.c
index 81df309..a379bbe 100644
--- a/drivers/gpu/drm/sti/sti_gdp.c
+++ b/drivers/gpu/drm/sti/sti_gdp.c
@@ -882,8 +882,8 @@ static int sti_gdp_late_register(struct drm_plane *drm_plane)
 }
 
 static const struct drm_plane_funcs sti_gdp_plane_helpers_funcs = {
-	.update_plane = drm_atomic_helper_update_plane,
-	.disable_plane = drm_atomic_helper_disable_plane,
+	.update_plane = sti_plane_update_plane,
+	.disable_plane = sti_plane_disable_plane,
 	.destroy = sti_gdp_destroy,
 	.set_property = drm_atomic_helper_plane_set_property,
 	.reset = sti_plane_reset,
diff --git a/drivers/gpu/drm/sti/sti_hqvdp.c b/drivers/gpu/drm/sti/sti_hqvdp.c
index f88130f..65ca43f 100644
--- a/drivers/gpu/drm/sti/sti_hqvdp.c
+++ b/drivers/gpu/drm/sti/sti_hqvdp.c
@@ -1252,8 +1252,8 @@ static int sti_hqvdp_late_register(struct drm_plane *drm_plane)
 }
 
 static const struct drm_plane_funcs sti_hqvdp_plane_helpers_funcs = {
-	.update_plane = drm_atomic_helper_update_plane,
-	.disable_plane = drm_atomic_helper_disable_plane,
+	.update_plane = sti_plane_update_plane,
+	.disable_plane = sti_plane_disable_plane,
 	.destroy = sti_hqvdp_destroy,
 	.set_property = drm_atomic_helper_plane_set_property,
 	.reset = sti_plane_reset,
diff --git a/drivers/gpu/drm/sti/sti_plane.c b/drivers/gpu/drm/sti/sti_plane.c
index ca4b371..22cf30d 100644
--- a/drivers/gpu/drm/sti/sti_plane.c
+++ b/drivers/gpu/drm/sti/sti_plane.c
@@ -7,6 +7,7 @@
  */
 
 #include <drm/drmP.h>
+#include <drm/drm_atomic.h>
 #include <drm/drm_fb_cma_helper.h>
 #include <drm/drm_gem_cma_helper.h>
 
@@ -130,3 +131,146 @@ void sti_plane_init_property(struct sti_plane *plane,
 	DRM_DEBUG_DRIVER("drm plane:%d mapped to %s\n",
 			 plane->drm_plane.base.id, sti_plane_to_str(plane));
 }
+
+int sti_plane_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
+			   struct drm_framebuffer *fb,
+			   int crtc_x, int crtc_y,
+			   unsigned int crtc_w, unsigned int crtc_h,
+			   uint32_t src_x, uint32_t src_y,
+			   uint32_t src_w, uint32_t src_h)
+{
+	/*
+	 * Forked from drm_atomic_helper_update_plane().
+	 * Here we do not wait for vblank if the client is not atomic, so
+	 * DRM_IOCTL_MODE_SETPLANE returns before vblank.
+	 */
+
+	struct drm_atomic_state *state;
+	struct drm_plane_state *plane_state;
+	struct sti_private *private = plane->dev->dev_private;
+	int ret = 0;
+
+	state = drm_atomic_state_alloc(plane->dev);
+	if (!state)
+		return -ENOMEM;
+
+	state->acquire_ctx = drm_modeset_legacy_acquire_ctx(crtc);
+retry:
+	plane_state = drm_atomic_get_plane_state(state, plane);
+	if (IS_ERR(plane_state)) {
+		ret = PTR_ERR(plane_state);
+		goto fail;
+	}
+
+	ret = drm_atomic_set_crtc_for_plane(plane_state, crtc);
+	if (ret != 0)
+		goto fail;
+	drm_atomic_set_fb_for_plane(plane_state, fb);
+	plane_state->crtc_x = crtc_x;
+	plane_state->crtc_y = crtc_y;
+	plane_state->crtc_w = crtc_w;
+	plane_state->crtc_h = crtc_h;
+	plane_state->src_x = src_x;
+	plane_state->src_y = src_y;
+	plane_state->src_w = src_w;
+	plane_state->src_h = src_h;
+
+	if ((plane == crtc->cursor) || !private->filp->atomic)
+		state->legacy_cursor_update = true;
+
+	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.
+	 */
+	plane->old_fb = plane->fb;
+
+	goto retry;
+}
+
+int sti_plane_disable_plane(struct drm_plane *plane)
+{
+	/*
+	 * Forked from drm_atomic_helper_disable_plane().
+	 * Here we do not wait for vblank if the client is not atomic, so
+	 * DRM_IOCTL_MODE_SETPLANE returns before vblank.
+	 */
+	struct drm_atomic_state *state;
+	struct drm_plane_state *plane_state;
+	struct sti_private *private = plane->dev->dev_private;
+	int ret = 0;
+
+	/*
+	 * FIXME: Without plane->crtc set we can't get at the implicit legacy
+	 * acquire context. The real fix will be to wire the acquire ctx through
+	 * everywhere we need it, but meanwhile prevent chaos by just skipping
+	 * this noop. The critical case is the cursor ioctls which a) only grab
+	 * crtc/cursor-plane locks (so we need the crtc to get at the right
+	 * acquire context) and b) can try to disable the plane multiple times.
+	 */
+	if (!plane->crtc)
+		return 0;
+
+	state = drm_atomic_state_alloc(plane->dev);
+	if (!state)
+		return -ENOMEM;
+
+	state->acquire_ctx = drm_modeset_legacy_acquire_ctx(plane->crtc);
+retry:
+	plane_state = drm_atomic_get_plane_state(state, plane);
+	if (IS_ERR(plane_state)) {
+		ret = PTR_ERR(plane_state);
+		goto fail;
+	}
+
+	if ((plane_state->crtc && (plane == plane->crtc->cursor)) ||
+	    !private->filp->atomic)
+		plane_state->state->legacy_cursor_update = true;
+
+	ret = __drm_atomic_helper_disable_plane(plane, plane_state);
+	if (ret != 0)
+		goto fail;
+
+	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.
+	 */
+	plane->old_fb = plane->fb;
+
+	goto retry;
+}
diff --git a/drivers/gpu/drm/sti/sti_plane.h b/drivers/gpu/drm/sti/sti_plane.h
index ce3e8d6..1372b9c 100644
--- a/drivers/gpu/drm/sti/sti_plane.h
+++ b/drivers/gpu/drm/sti/sti_plane.h
@@ -83,4 +83,12 @@ void sti_plane_update_fps(struct sti_plane *plane,
 void sti_plane_init_property(struct sti_plane *plane,
 			     enum drm_plane_type type);
 void sti_plane_reset(struct drm_plane *plane);
+
+int sti_plane_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
+			   struct drm_framebuffer *fb,
+			   int crtc_x, int crtc_y,
+			   unsigned int crtc_w, unsigned int crtc_h,
+			   uint32_t src_x, uint32_t src_y,
+			   uint32_t src_w, uint32_t src_h);
+int sti_plane_disable_plane(struct drm_plane *plane);
 #endif
-- 
2.7.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 4/5] drm/sti: do not sync SETPROPERTY on vblank if not ATOMIC

[Fabien Dessenne]

If the client does not set the ATOMIC capability, do not wait for vblank
before returning an DRM_IOCTL_MODE_OBJ_SETPROPERTY call.

In this way, a legacy framework (eg non-atomic Weston) can call several
SETPROPERTY within the same Vsync cycle.

This is implemented by setting the legacy_cursor_update flag, to behave
the same way as DRM_IOCTL_MODE_CURSOR (not vblank synced).

Change-Id: I6b6134eca57eca399bdda006ab1cb8280d4002d4
Signed-off-by: Fabien Dessenne <fabien.dessenne@st.com>
---
 drivers/gpu/drm/sti/sti_cursor.c |  2 +-
 drivers/gpu/drm/sti/sti_gdp.c    |  2 +-
 drivers/gpu/drm/sti/sti_hqvdp.c  |  2 +-
 drivers/gpu/drm/sti/sti_plane.c  | 54 ++++++++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/sti/sti_plane.h  |  2 ++
 5 files changed, 59 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/sti/sti_cursor.c b/drivers/gpu/drm/sti/sti_cursor.c
index ea0dbae..d7e9f8a 100644
--- a/drivers/gpu/drm/sti/sti_cursor.c
+++ b/drivers/gpu/drm/sti/sti_cursor.c
@@ -349,7 +349,7 @@ static const struct drm_plane_funcs sti_cursor_plane_helpers_funcs = {
 	.update_plane = sti_plane_update_plane,
 	.disable_plane = sti_plane_disable_plane,
 	.destroy = sti_cursor_destroy,
-	.set_property = drm_atomic_helper_plane_set_property,
+	.set_property = sti_plane_set_property,
 	.reset = sti_plane_reset,
 	.atomic_duplicate_state = drm_atomic_helper_plane_duplicate_state,
 	.atomic_destroy_state = drm_atomic_helper_plane_destroy_state,
diff --git a/drivers/gpu/drm/sti/sti_gdp.c b/drivers/gpu/drm/sti/sti_gdp.c
index a379bbe..6fa5042 100644
--- a/drivers/gpu/drm/sti/sti_gdp.c
+++ b/drivers/gpu/drm/sti/sti_gdp.c
@@ -885,7 +885,7 @@ static const struct drm_plane_funcs sti_gdp_plane_helpers_funcs = {
 	.update_plane = sti_plane_update_plane,
 	.disable_plane = sti_plane_disable_plane,
 	.destroy = sti_gdp_destroy,
-	.set_property = drm_atomic_helper_plane_set_property,
+	.set_property = sti_plane_set_property,
 	.reset = sti_plane_reset,
 	.atomic_duplicate_state = drm_atomic_helper_plane_duplicate_state,
 	.atomic_destroy_state = drm_atomic_helper_plane_destroy_state,
diff --git a/drivers/gpu/drm/sti/sti_hqvdp.c b/drivers/gpu/drm/sti/sti_hqvdp.c
index 65ca43f..3fd6f3a 100644
--- a/drivers/gpu/drm/sti/sti_hqvdp.c
+++ b/drivers/gpu/drm/sti/sti_hqvdp.c
@@ -1255,7 +1255,7 @@ static const struct drm_plane_funcs sti_hqvdp_plane_helpers_funcs = {
 	.update_plane = sti_plane_update_plane,
 	.disable_plane = sti_plane_disable_plane,
 	.destroy = sti_hqvdp_destroy,
-	.set_property = drm_atomic_helper_plane_set_property,
+	.set_property = sti_plane_set_property,
 	.reset = sti_plane_reset,
 	.atomic_duplicate_state = drm_atomic_helper_plane_duplicate_state,
 	.atomic_destroy_state = drm_atomic_helper_plane_destroy_state,
diff --git a/drivers/gpu/drm/sti/sti_plane.c b/drivers/gpu/drm/sti/sti_plane.c
index 22cf30d..c58fe1b 100644
--- a/drivers/gpu/drm/sti/sti_plane.c
+++ b/drivers/gpu/drm/sti/sti_plane.c
@@ -132,6 +132,60 @@ void sti_plane_init_property(struct sti_plane *plane,
 			 plane->drm_plane.base.id, sti_plane_to_str(plane));
 }
 
+int sti_plane_set_property(struct drm_plane *plane,
+			   struct drm_property *property, uint64_t val)
+{
+	/*
+	 * Forked from drm_atomic_helper_plane_set_property().
+	 * Here we do not wait for vblank if the client is not atomic, so
+	 * DRM_IOCTL_MODE_OBJ_SETPROPERTY returns before vblank.
+	 */
+	struct drm_atomic_state *state;
+	struct drm_plane_state *plane_state;
+	struct sti_private *private = plane->dev->dev_private;
+	int ret = 0;
+
+	state = drm_atomic_state_alloc(plane->dev);
+	if (!state)
+		return -ENOMEM;
+
+	/* ->set_property is always called with all locks held. */
+	state->acquire_ctx = plane->dev->mode_config.acquire_ctx;
+retry:
+	plane_state = drm_atomic_get_plane_state(state, plane);
+	if (IS_ERR(plane_state)) {
+		ret = PTR_ERR(plane_state);
+		goto fail;
+	}
+
+	ret = drm_atomic_plane_set_property(plane, plane_state,
+					    property, val);
+	if (ret)
+		goto fail;
+
+	if (!private->filp->atomic)
+		state->legacy_cursor_update = true;
+
+	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);
+
+	goto retry;
+}
+
 int sti_plane_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
 			   struct drm_framebuffer *fb,
 			   int crtc_x, int crtc_y,
diff --git a/drivers/gpu/drm/sti/sti_plane.h b/drivers/gpu/drm/sti/sti_plane.h
index 1372b9c..aa1b6ce 100644
--- a/drivers/gpu/drm/sti/sti_plane.h
+++ b/drivers/gpu/drm/sti/sti_plane.h
@@ -82,6 +82,8 @@ void sti_plane_update_fps(struct sti_plane *plane,
 
 void sti_plane_init_property(struct sti_plane *plane,
 			     enum drm_plane_type type);
+int sti_plane_set_property(struct drm_plane *plane,
+			   struct drm_property *property, uint64_t val);
 void sti_plane_reset(struct drm_plane *plane);
 
 int sti_plane_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
-- 
2.7.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 5/5] drm/sti: do not check hw scaling if mode is not set

[Fabien Dessenne]

Fix a division by 0 case : in some cases, when the HQVDP plane is being
disabled atomic_check() is called with "mode->clock = 0".
In that case, do not check for scaling capabilities.

Change-Id: I7fb752ab394211c3deafa149f52cfb2bca244e84
Signed-off-by: Fabien Dessenne <fabien.dessenne@st.com>
---
 drivers/gpu/drm/sti/sti_hqvdp.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/sti/sti_hqvdp.c b/drivers/gpu/drm/sti/sti_hqvdp.c
index 3fd6f3a..fe36e0f 100644
--- a/drivers/gpu/drm/sti/sti_hqvdp.c
+++ b/drivers/gpu/drm/sti/sti_hqvdp.c
@@ -1035,9 +1035,9 @@ static int sti_hqvdp_atomic_check(struct drm_plane *drm_plane,
 	src_w = state->src_w >> 16;
 	src_h = state->src_h >> 16;
 
-	if (!sti_hqvdp_check_hw_scaling(hqvdp, mode,
-					src_w, src_h,
-					dst_w, dst_h)) {
+	if (mode->clock && !sti_hqvdp_check_hw_scaling(hqvdp, mode,
+						       src_w, src_h,
+						       dst_w, dst_h)) {
 		DRM_ERROR("Scaling beyond HW capabilities\n");
 		return -EINVAL;
 	}
-- 
2.7.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 0/5] drm/sti: do not sync legacy IOCTL on vblank if not ATOMIC

[Daniel Vetter]

On Tue, Jan 03, 2017 at 05:56:47PM +0100, Fabien Dessenne wrote:
> These patches allow a legacy framework (eg non-atomic Weston) to call
> several SETPLANE within the same Vsync cycle.
> - [PATCH 1/5] drm/sti: use atomic_helper for commit
> - [PATCH 2/5] drm/sti: add drm_file to sti_private
> - [PATCH 3/5] drm/sti: do not sync SETPLANE on vblank if not ATOMIC
> - [PATCH 4/5] drm/sti: do not sync SETPROPERTY on vblank if not ATOMIC
> - [PATCH 5/5] drm/sti: do not check hw scaling if mode is not set

Upstream weston never really enabled plane support, why exactly do you
need this? Also, if this really is required, I think we should implement
something like this (aka async plane flips) in general for everyone. sti
is by far not the only driver playing around with these games.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 0/5] drm/sti: do not sync legacy IOCTL on vblank if not ATOMIC

[Fabien DESSENNE]

Hi Daniel


I come to the conclusion that (only) Atomic Weston will solve all of my 
troubles, so let's forget these patches (and work for atomic weston).

By the way, is the expected behavior (Vblank - sync'ed or not) of 
drmModeSetPlane() described anywhere? The last time I browsed the 
related documentation I could not find the answer. Maybe something that 
needs to be clarified ?

I will also re-send patch 1 & 5 out of this abandoned series since they 
make sense independently of this (a)sync stuff.


BR.


Fabien


On 04/01/17 10:18, Daniel Vetter wrote:
> On Tue, Jan 03, 2017 at 05:56:47PM +0100, Fabien Dessenne wrote:
>> These patches allow a legacy framework (eg non-atomic Weston) to call
>> several SETPLANE within the same Vsync cycle.
>> - [PATCH 1/5] drm/sti: use atomic_helper for commit
>> - [PATCH 2/5] drm/sti: add drm_file to sti_private
>> - [PATCH 3/5] drm/sti: do not sync SETPLANE on vblank if not ATOMIC
>> - [PATCH 4/5] drm/sti: do not sync SETPROPERTY on vblank if not ATOMIC
>> - [PATCH 5/5] drm/sti: do not check hw scaling if mode is not set
> Upstream weston never really enabled plane support, why exactly do you
> need this? Also, if this really is required, I think we should implement
> something like this (aka async plane flips) in general for everyone. sti
> is by far not the only driver playing around with these games.
> -Daniel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 0/5] drm/sti: do not sync legacy IOCTL on vblank if not ATOMIC

[Daniel Vetter]

On Thu, Jan 05, 2017 at 03:23:24PM +0000, Fabien DESSENNE wrote:
> Hi Daniel
> 
> 
> I come to the conclusion that (only) Atomic Weston will solve all of my 
> troubles, so let's forget these patches (and work for atomic weston).
> 
> By the way, is the expected behavior (Vblank - sync'ed or not) of 
> drmModeSetPlane() described anywhere? The last time I browsed the 
> related documentation I could not find the answer. Maybe something that 
> needs to be clarified ?

It's undefined and depends upon driver and platform and kernel version.
And all the work is done already, we've taken all the lessons learned from
legacy planes and made sure atomic sucks less ;-) There's no point in
changing/clarifying legacy semantics, at worst you'll end up breaking more
existing userspace. And we're not going to get uniform behaviour between
all drivers anyway for the legacy interfaces. Imo not worth the trouble,
let's just use atomic.
-Daniel


> I will also re-send patch 1 & 5 out of this abandoned series since they 
> make sense independently of this (a)sync stuff.
> 
> 
> BR.
> 
> 
> Fabien
> 
> 
> On 04/01/17 10:18, Daniel Vetter wrote:
> > On Tue, Jan 03, 2017 at 05:56:47PM +0100, Fabien Dessenne wrote:
> >> These patches allow a legacy framework (eg non-atomic Weston) to call
> >> several SETPLANE within the same Vsync cycle.
> >> - [PATCH 1/5] drm/sti: use atomic_helper for commit
> >> - [PATCH 2/5] drm/sti: add drm_file to sti_private
> >> - [PATCH 3/5] drm/sti: do not sync SETPLANE on vblank if not ATOMIC
> >> - [PATCH 4/5] drm/sti: do not sync SETPROPERTY on vblank if not ATOMIC
> >> - [PATCH 5/5] drm/sti: do not check hw scaling if mode is not set
> > Upstream weston never really enabled plane support, why exactly do you
> > need this? Also, if this really is required, I think we should implement
> > something like this (aka async plane flips) in general for everyone. sti
> > is by far not the only driver playing around with these games.
> > -Daniel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel