linux-arm-msm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/5] drm/msm/mdp5: atomic smp + hwpipe assignment
@ 2016-11-05 16:25 Rob Clark
  2016-11-05 16:25 ` [PATCH 1/5] drm/msm/mdp5: introduce mdp5_hw_pipe Rob Clark
                   ` (4 more replies)
  0 siblings, 5 replies; 12+ messages in thread
From: Rob Clark @ 2016-11-05 16:25 UTC (permalink / raw)
  To: dri-devel
  Cc: linux-arm-msm, freedreno, Archit Taneja, Daniel Vetter,
	Sean Paul, Rob Clark

In the process of getting kms fence fd (and related gpu egl fence fd
extension) working with real hardware (using android drm-hwc) I found
a lot of issues w/ plane support.  Perhaps that is to be expected the
first time getting a userspace that actually makes heavy use of overlay
planes working.  So some of the approaches here may be useful to other
drivers when they cross that bridge.

The mdp5 hw, in particular variants that have "SMP" (Shared Memory
Pool, ie. shared FIFO blocks that are allocated to the various hw
pipes) have a lot of constraints that a generic userspace is not
aware of.  Such as not being able to arbitrarily change scanout
width or pixel format on an active pipe.  In addition, not all pipes
can support all features (YUV, rotation, and in some cases, scaling).

Additionally, during the initial conversion to atomic, we did not
handle SMP block allocation in plane->atomic_check(), leading to cases
where an update could fail in plane->atomic_update() due to failure
to allocate SMP blocks.

To solve this:
 (1) dynamically assign hw pipes to planes at ->atomic_check() based
     on the required plane capabilities
 (2) handle SMP block allocation in ->atomic_check().  When SMP block
     allocation needs to change, re-assign a new hw pipe

To do this, we need to track the SMP block allocation, and hw pipe
assignment in a global atomic state object.  This way, we ensure that
multiple updates from different threads which touch shared state (the
hw pipe assignemnt or SMP block allocation) are properly serialized
(via state_lock), and atomic test-only updates, or commits that fail
in atomic_check() (or deadlock/backoff scenarios) are properly un-
wound by simply throwing away the new state objects.

The global state is handled in the same way as plane/crtc/etc state.
On mdp5_get_state(), the state_lock is acquired as part of the atomic
acquire_ctx, and the current state is copied.  At that point changes
to the copied state are permitted.  New plane state can reference
new hw pipes, since the assignment of hw pipes is tracked in the
global state.  At any point the new state can be thrown away (along
with the various new plane state objects that reference it as part of
the new drm_atomic_state).  And if the new state is committed, the
new global state is swapped with old state, in the same way per-plane/
crtc/connector state is handled, preserving consistency between global
and per-plane state.

Rob Clark (5):
  drm/msm/mdp5: introduce mdp5_hw_pipe
  drm/msm: subclass drm_atomic_state
  drm/msm/mdp5: add skeletal mdp5_state
  drm/msm/mdp5: dynamically assign hw pipes to planes
  drm/msm/mdp5: handle SMP block allocations "atomically"

 drivers/gpu/drm/msm/Makefile              |   1 +
 drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c  |   2 +-
 drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c   | 180 +++++++++++++++-----
 drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h   |  33 +++-
 drivers/gpu/drm/msm/mdp/mdp5/mdp5_pipe.c  | 132 +++++++++++++++
 drivers/gpu/drm/msm/mdp/mdp5/mdp5_pipe.h  |  54 ++++++
 drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c | 202 +++++++++-------------
 drivers/gpu/drm/msm/mdp/mdp5/mdp5_smp.c   | 270 ++++++++++--------------------
 drivers/gpu/drm/msm/mdp/mdp5/mdp5_smp.h   |  66 ++++++--
 drivers/gpu/drm/msm/msm_atomic.c          |  31 ++++
 drivers/gpu/drm/msm/msm_drv.c             |   3 +
 drivers/gpu/drm/msm/msm_drv.h             |   3 +
 drivers/gpu/drm/msm/msm_kms.h             |  14 ++
 13 files changed, 629 insertions(+), 362 deletions(-)
 create mode 100644 drivers/gpu/drm/msm/mdp/mdp5/mdp5_pipe.c
 create mode 100644 drivers/gpu/drm/msm/mdp/mdp5/mdp5_pipe.h

-- 
2.7.4

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 1/5] drm/msm/mdp5: introduce mdp5_hw_pipe
  2016-11-05 16:25 [PATCH 0/5] drm/msm/mdp5: atomic smp + hwpipe assignment Rob Clark
@ 2016-11-05 16:25 ` Rob Clark
  2016-11-07 10:38   ` Archit Taneja
  2016-11-05 16:25 ` [PATCH 2/5] drm/msm: subclass drm_atomic_state Rob Clark
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 12+ messages in thread
From: Rob Clark @ 2016-11-05 16:25 UTC (permalink / raw)
  To: dri-devel; +Cc: linux-arm-msm, freedreno

Split out the hardware pipe specifics from mdp5_plane.  To start, the hw
pipes are statically assigned to planes, but next step is to assign the
hw pipes during plane->atomic_check() based on requested caps (scaling,
YUV, etc).  And then hw pipe re-assignment if required if required SMP
blocks changes.

Signed-off-by: Rob Clark <robdclark@gmail.com>
---
 drivers/gpu/drm/msm/Makefile              |   1 +
 drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c   | 126 +++++++++++++++++++-----------
 drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h   |   7 +-
 drivers/gpu/drm/msm/mdp/mdp5/mdp5_pipe.c  |  43 ++++++++++
 drivers/gpu/drm/msm/mdp/mdp5/mdp5_pipe.h  |  39 +++++++++
 drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c |  66 +++++++---------
 6 files changed, 197 insertions(+), 85 deletions(-)
 create mode 100644 drivers/gpu/drm/msm/mdp/mdp5/mdp5_pipe.c
 create mode 100644 drivers/gpu/drm/msm/mdp/mdp5/mdp5_pipe.h

diff --git a/drivers/gpu/drm/msm/Makefile b/drivers/gpu/drm/msm/Makefile
index fb5be3e..90f66c4 100644
--- a/drivers/gpu/drm/msm/Makefile
+++ b/drivers/gpu/drm/msm/Makefile
@@ -37,6 +37,7 @@ msm-y := \
 	mdp/mdp5/mdp5_irq.o \
 	mdp/mdp5/mdp5_mdss.o \
 	mdp/mdp5/mdp5_kms.o \
+	mdp/mdp5/mdp5_pipe.o \
 	mdp/mdp5/mdp5_plane.o \
 	mdp/mdp5/mdp5_smp.o \
 	msm_atomic.o \
diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c
index f1288c7..d3d45ed 100644
--- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c
+++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c
@@ -119,6 +119,10 @@ static void mdp5_kms_destroy(struct msm_kms *kms)
 {
 	struct mdp5_kms *mdp5_kms = to_mdp5_kms(to_mdp_kms(kms));
 	struct msm_gem_address_space *aspace = mdp5_kms->aspace;
+	int i;
+
+	for (i = 0; i < mdp5_kms->num_hwpipes; i++)
+		mdp5_pipe_destroy(mdp5_kms->hwpipes[i]);
 
 	if (aspace) {
 		aspace->mmu->funcs->detach(aspace->mmu,
@@ -323,15 +327,6 @@ static int modeset_init_intf(struct mdp5_kms *mdp5_kms, int intf_num)
 
 static int modeset_init(struct mdp5_kms *mdp5_kms)
 {
-	static const enum mdp5_pipe rgb_planes[] = {
-			SSPP_RGB0, SSPP_RGB1, SSPP_RGB2, SSPP_RGB3,
-	};
-	static const enum mdp5_pipe vig_planes[] = {
-			SSPP_VIG0, SSPP_VIG1, SSPP_VIG2, SSPP_VIG3,
-	};
-	static const enum mdp5_pipe dma_planes[] = {
-			SSPP_DMA0, SSPP_DMA1,
-	};
 	struct drm_device *dev = mdp5_kms->dev;
 	struct msm_drm_private *priv = dev->dev_private;
 	const struct mdp5_cfg_hw *hw_cfg;
@@ -339,58 +334,34 @@ static int modeset_init(struct mdp5_kms *mdp5_kms)
 
 	hw_cfg = mdp5_cfg_get_hw_config(mdp5_kms->cfg);
 
-	/* construct CRTCs and their private planes: */
-	for (i = 0; i < hw_cfg->pipe_rgb.count; i++) {
+	/* Construct planes equaling the number of hw pipes, and CRTCs
+	 * for the N layer-mixers (LM).  The first N planes become primary
+	 * planes for the CRTCs, with the remainder as overlay planes:
+	 */
+	for (i = 0; i < mdp5_kms->num_hwpipes; i++) {
+		bool primary = i < mdp5_cfg->lm.count;
 		struct drm_plane *plane;
 		struct drm_crtc *crtc;
 
-		plane = mdp5_plane_init(dev, rgb_planes[i], true,
-			hw_cfg->pipe_rgb.base[i], hw_cfg->pipe_rgb.caps);
+		plane = mdp5_plane_init(dev, mdp5_kms->hwpipes[i], primary);
 		if (IS_ERR(plane)) {
 			ret = PTR_ERR(plane);
-			dev_err(dev->dev, "failed to construct plane for %s (%d)\n",
-					pipe2name(rgb_planes[i]), ret);
+			dev_err(dev->dev, "failed to construct plane %d (%d)\n", i, ret);
 			goto fail;
 		}
 
+		if (!primary)
+			continue;
+
 		crtc  = mdp5_crtc_init(dev, plane, i);
 		if (IS_ERR(crtc)) {
 			ret = PTR_ERR(crtc);
-			dev_err(dev->dev, "failed to construct crtc for %s (%d)\n",
-					pipe2name(rgb_planes[i]), ret);
+			dev_err(dev->dev, "failed to construct crtc %d (%d)\n", i, ret);
 			goto fail;
 		}
 		priv->crtcs[priv->num_crtcs++] = crtc;
 	}
 
-	/* Construct video planes: */
-	for (i = 0; i < hw_cfg->pipe_vig.count; i++) {
-		struct drm_plane *plane;
-
-		plane = mdp5_plane_init(dev, vig_planes[i], false,
-			hw_cfg->pipe_vig.base[i], hw_cfg->pipe_vig.caps);
-		if (IS_ERR(plane)) {
-			ret = PTR_ERR(plane);
-			dev_err(dev->dev, "failed to construct %s plane: %d\n",
-					pipe2name(vig_planes[i]), ret);
-			goto fail;
-		}
-	}
-
-	/* DMA planes */
-	for (i = 0; i < hw_cfg->pipe_dma.count; i++) {
-		struct drm_plane *plane;
-
-		plane = mdp5_plane_init(dev, dma_planes[i], false,
-				hw_cfg->pipe_dma.base[i], hw_cfg->pipe_dma.caps);
-		if (IS_ERR(plane)) {
-			ret = PTR_ERR(plane);
-			dev_err(dev->dev, "failed to construct %s plane: %d\n",
-					pipe2name(dma_planes[i]), ret);
-			goto fail;
-		}
-	}
-
 	/* Construct encoders and modeset initialize connector devices
 	 * for each external display interface.
 	 */
@@ -676,6 +647,67 @@ static void mdp5_destroy(struct platform_device *pdev)
 		pm_runtime_disable(&pdev->dev);
 }
 
+static int construct_pipes(struct mdp5_kms *mdp5_kms, int cnt,
+		const enum mdp5_pipe *pipes, const uint32_t *offsets,
+		uint32_t caps)
+{
+	struct drm_device *dev = mdp5_kms->dev;
+	int i, ret;
+
+	for (i = 0; i < cnt; i++) {
+		struct mdp5_hw_pipe *hwpipe;
+
+		hwpipe = mdp5_pipe_init(pipes[i], offsets[i], caps);
+		if (IS_ERR(hwpipe)) {
+			ret = PTR_ERR(hwpipe);
+			dev_err(dev->dev, "failed to construct pipe for %s (%d)\n",
+					pipe2name(pipes[i]), ret);
+			return ret;
+		}
+		hwpipe->idx = mdp5_kms->num_hwpipes;
+		mdp5_kms->hwpipes[mdp5_kms->num_hwpipes++] = hwpipe;
+	}
+
+	return 0;
+}
+
+static int hwpipe_init(struct mdp5_kms *mdp5_kms)
+{
+	static const enum mdp5_pipe rgb_planes[] = {
+			SSPP_RGB0, SSPP_RGB1, SSPP_RGB2, SSPP_RGB3,
+	};
+	static const enum mdp5_pipe vig_planes[] = {
+			SSPP_VIG0, SSPP_VIG1, SSPP_VIG2, SSPP_VIG3,
+	};
+	static const enum mdp5_pipe dma_planes[] = {
+			SSPP_DMA0, SSPP_DMA1,
+	};
+	const struct mdp5_cfg_hw *hw_cfg;
+	int ret;
+
+	hw_cfg = mdp5_cfg_get_hw_config(mdp5_kms->cfg);
+
+	/* Construct RGB pipes: */
+	ret = construct_pipes(mdp5_kms, hw_cfg->pipe_rgb.count, rgb_planes,
+			hw_cfg->pipe_rgb.base, hw_cfg->pipe_rgb.caps);
+	if (ret)
+		return ret;
+
+	/* Construct video (VIG) pipes: */
+	ret = construct_pipes(mdp5_kms, hw_cfg->pipe_vig.count, vig_planes,
+			hw_cfg->pipe_vig.base, hw_cfg->pipe_vig.caps);
+	if (ret)
+		return ret;
+
+	/* Construct DMA pipes: */
+	ret = construct_pipes(mdp5_kms, hw_cfg->pipe_dma.count, dma_planes,
+			hw_cfg->pipe_dma.base, hw_cfg->pipe_dma.caps);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
 static int mdp5_init(struct platform_device *pdev, struct drm_device *dev)
 {
 	struct msm_drm_private *priv = dev->dev_private;
@@ -765,6 +797,10 @@ static int mdp5_init(struct platform_device *pdev, struct drm_device *dev)
 		goto fail;
 	}
 
+	ret = hwpipe_init(mdp5_kms);
+	if (ret)
+		goto fail;
+
 	/* set uninit-ed kms */
 	priv->kms = &mdp5_kms->base.base;
 
diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h
index d5e40af..88120c5 100644
--- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h
+++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h
@@ -24,6 +24,7 @@
 #include "mdp5_cfg.h"	/* must be included before mdp5.xml.h */
 #include "mdp5.xml.h"
 #include "mdp5_ctl.h"
+#include "mdp5_pipe.h"
 #include "mdp5_smp.h"
 
 struct mdp5_kms {
@@ -33,6 +34,9 @@ struct mdp5_kms {
 
 	struct platform_device *pdev;
 
+	unsigned num_hwpipes;
+	struct mdp5_hw_pipe *hwpipes[16];
+
 	struct mdp5_cfg_handler *cfg;
 	uint32_t caps;	/* MDP capabilities (MDP_CAP_XXX bits) */
 
@@ -207,8 +211,7 @@ void mdp5_plane_complete_commit(struct drm_plane *plane,
 	struct drm_plane_state *state);
 enum mdp5_pipe mdp5_plane_pipe(struct drm_plane *plane);
 struct drm_plane *mdp5_plane_init(struct drm_device *dev,
-		enum mdp5_pipe pipe, bool private_plane,
-		uint32_t reg_offset, uint32_t caps);
+		struct mdp5_hw_pipe *hwpipe, bool primary);
 
 uint32_t mdp5_crtc_vblank(struct drm_crtc *crtc);
 
diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_pipe.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_pipe.c
new file mode 100644
index 0000000..7f3e8e50
--- /dev/null
+++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_pipe.c
@@ -0,0 +1,43 @@
+/*
+ * Copyright (C) 2016 Red Hat
+ * Author: Rob Clark <robdclark@gmail.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published by
+ * the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "mdp5_kms.h"
+
+void mdp5_pipe_destroy(struct mdp5_hw_pipe *hwpipe)
+{
+	kfree(hwpipe);
+}
+
+struct mdp5_hw_pipe *mdp5_pipe_init(enum mdp5_pipe pipe,
+		uint32_t reg_offset, uint32_t caps)
+{
+	struct mdp5_hw_pipe *hwpipe;
+
+	hwpipe = kzalloc(sizeof(*hwpipe), GFP_KERNEL);
+	if (!hwpipe)
+		return ERR_PTR(-ENOMEM);
+
+	hwpipe->name = pipe2name(pipe);
+	hwpipe->pipe = pipe;
+	hwpipe->reg_offset = reg_offset;
+	hwpipe->caps = caps;
+	hwpipe->flush_mask = mdp_ctl_flush_mask_pipe(pipe);
+
+	spin_lock_init(&hwpipe->pipe_lock);
+
+	return hwpipe;
+}
diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_pipe.h b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_pipe.h
new file mode 100644
index 0000000..dcfa0e0
--- /dev/null
+++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_pipe.h
@@ -0,0 +1,39 @@
+/*
+ * Copyright (C) 2016 Red Hat
+ * Author: Rob Clark <robdclark@gmail.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published by
+ * the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef __MDP5_PIPE_H__
+#define __MDP5_PIPE_H__
+
+/* represents a hw pipe, which is dynamically assigned to a plane */
+struct mdp5_hw_pipe {
+	int idx;
+
+	const char *name;
+	enum mdp5_pipe pipe;
+
+	spinlock_t pipe_lock;     /* protect REG_MDP5_PIPE_* registers */
+	uint32_t reg_offset;
+	uint32_t caps;
+
+	uint32_t flush_mask;      /* used to commit pipe registers */
+};
+
+struct mdp5_hw_pipe *mdp5_pipe_init(enum mdp5_pipe pipe,
+		uint32_t reg_offset, uint32_t caps);
+void mdp5_pipe_destroy(struct mdp5_hw_pipe *hwpipe);
+
+#endif /* __MDP5_PIPE_H__ */
diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
index 7b46cfb..e4ecfeb 100644
--- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
+++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
@@ -22,13 +22,7 @@
 struct mdp5_plane {
 	struct drm_plane base;
 
-	enum mdp5_pipe pipe;
-
-	spinlock_t pipe_lock;	/* protect REG_MDP5_PIPE_* registers */
-	uint32_t reg_offset;
-	uint32_t caps;
-
-	uint32_t flush_mask;	/* used to commit pipe registers */
+	struct mdp5_hw_pipe *hwpipe;
 
 	uint32_t nformats;
 	uint32_t formats[32];
@@ -71,8 +65,8 @@ static void mdp5_plane_install_rotation_property(struct drm_device *dev,
 {
 	struct mdp5_plane *mdp5_plane = to_mdp5_plane(plane);
 
-	if (!(mdp5_plane->caps & MDP_PIPE_CAP_HFLIP) &&
-		!(mdp5_plane->caps & MDP_PIPE_CAP_VFLIP))
+	if (!(mdp5_plane->hwpipe->caps & MDP_PIPE_CAP_HFLIP) &&
+		!(mdp5_plane->hwpipe->caps & MDP_PIPE_CAP_VFLIP))
 		return;
 
 	drm_plane_create_rotation_property(plane,
@@ -304,13 +298,13 @@ static int mdp5_plane_atomic_check(struct drm_plane *plane,
 
 		format = to_mdp_format(msm_framebuffer_format(state->fb));
 		if (MDP_FORMAT_IS_YUV(format) &&
-			!pipe_supports_yuv(mdp5_plane->caps)) {
+			!pipe_supports_yuv(mdp5_plane->hwpipe->caps)) {
 			DBG("Pipe doesn't support YUV\n");
 
 			return -EINVAL;
 		}
 
-		if (!(mdp5_plane->caps & MDP_PIPE_CAP_SCALE) &&
+		if (!(mdp5_plane->hwpipe->caps & MDP_PIPE_CAP_SCALE) &&
 			(((state->src_w >> 16) != state->crtc_w) ||
 			((state->src_h >> 16) != state->crtc_h))) {
 			DBG("Pipe doesn't support scaling (%dx%d -> %dx%d)\n",
@@ -324,11 +318,12 @@ static int mdp5_plane_atomic_check(struct drm_plane *plane,
 						 DRM_ROTATE_0 |
 						 DRM_REFLECT_X |
 						 DRM_REFLECT_Y);
+
 		hflip = !!(rotation & DRM_REFLECT_X);
 		vflip = !!(rotation & DRM_REFLECT_Y);
 
-		if ((vflip && !(mdp5_plane->caps & MDP_PIPE_CAP_VFLIP)) ||
-			(hflip && !(mdp5_plane->caps & MDP_PIPE_CAP_HFLIP))) {
+		if ((vflip && !(mdp5_plane->hwpipe->caps & MDP_PIPE_CAP_VFLIP)) ||
+			(hflip && !(mdp5_plane->hwpipe->caps & MDP_PIPE_CAP_HFLIP))) {
 			DBG("Pipe doesn't support flip\n");
 
 			return -EINVAL;
@@ -396,7 +391,7 @@ static void set_scanout_locked(struct drm_plane *plane,
 {
 	struct mdp5_plane *mdp5_plane = to_mdp5_plane(plane);
 	struct mdp5_kms *mdp5_kms = get_kms(plane);
-	enum mdp5_pipe pipe = mdp5_plane->pipe;
+	enum mdp5_pipe pipe = mdp5_plane->hwpipe->pipe;
 
 	mdp5_write(mdp5_kms, REG_MDP5_PIPE_SRC_STRIDE_A(pipe),
 			MDP5_PIPE_SRC_STRIDE_A_P0(fb->pitches[0]) |
@@ -678,12 +673,13 @@ static int mdp5_plane_mode_set(struct drm_plane *plane,
 {
 	struct mdp5_plane *mdp5_plane = to_mdp5_plane(plane);
 	struct drm_plane_state *pstate = plane->state;
+	struct mdp5_hw_pipe *hwpipe = mdp5_plane->hwpipe;
 	struct mdp5_kms *mdp5_kms = get_kms(plane);
-	enum mdp5_pipe pipe = mdp5_plane->pipe;
+	enum mdp5_pipe pipe = hwpipe->pipe;
 	const struct mdp_format *format;
 	uint32_t nplanes, config = 0;
 	uint32_t phasex_step[COMP_MAX] = {0,}, phasey_step[COMP_MAX] = {0,};
-	bool pe = mdp5_plane->caps & MDP_PIPE_CAP_SW_PIX_EXT;
+	bool pe = hwpipe->caps & MDP_PIPE_CAP_SW_PIX_EXT;
 	int pe_left[COMP_MAX], pe_right[COMP_MAX];
 	int pe_top[COMP_MAX], pe_bottom[COMP_MAX];
 	uint32_t hdecm = 0, vdecm = 0;
@@ -714,8 +710,8 @@ static int mdp5_plane_mode_set(struct drm_plane *plane,
 
 	/* Request some memory from the SMP: */
 	if (mdp5_kms->smp) {
-		ret = mdp5_smp_request(mdp5_kms->smp,
-				mdp5_plane->pipe, format, src_w, false);
+		ret = mdp5_smp_request(mdp5_kms->smp, pipe,
+				format, src_w, false);
 		if (ret)
 			return ret;
 	}
@@ -737,7 +733,7 @@ static int mdp5_plane_mode_set(struct drm_plane *plane,
 	if (ret)
 		return ret;
 
-	if (mdp5_plane->caps & MDP_PIPE_CAP_SW_PIX_EXT) {
+	if (hwpipe->caps & MDP_PIPE_CAP_SW_PIX_EXT) {
 		calc_pixel_ext(format, src_w, crtc_w, phasex_step,
 					 pe_left, pe_right, true);
 		calc_pixel_ext(format, src_h, crtc_h, phasey_step,
@@ -758,7 +754,7 @@ static int mdp5_plane_mode_set(struct drm_plane *plane,
 	hflip = !!(rotation & DRM_REFLECT_X);
 	vflip = !!(rotation & DRM_REFLECT_Y);
 
-	spin_lock_irqsave(&mdp5_plane->pipe_lock, flags);
+	spin_lock_irqsave(&hwpipe->pipe_lock, flags);
 
 	mdp5_write(mdp5_kms, REG_MDP5_PIPE_SRC_IMG_SIZE(pipe),
 			MDP5_PIPE_SRC_IMG_SIZE_WIDTH(min(fb->width, src_w)) |
@@ -807,12 +803,12 @@ static int mdp5_plane_mode_set(struct drm_plane *plane,
 	/* not using secure mode: */
 	mdp5_write(mdp5_kms, REG_MDP5_PIPE_SRC_ADDR_SW_STATUS(pipe), 0);
 
-	if (mdp5_plane->caps & MDP_PIPE_CAP_SW_PIX_EXT)
+	if (hwpipe->caps & MDP_PIPE_CAP_SW_PIX_EXT)
 		mdp5_write_pixel_ext(mdp5_kms, pipe, format,
 				src_w, pe_left, pe_right,
 				src_h, pe_top, pe_bottom);
 
-	if (mdp5_plane->caps & MDP_PIPE_CAP_SCALE) {
+	if (hwpipe->caps & MDP_PIPE_CAP_SCALE) {
 		mdp5_write(mdp5_kms, REG_MDP5_PIPE_SCALE_PHASE_STEP_X(pipe),
 				phasex_step[COMP_0]);
 		mdp5_write(mdp5_kms, REG_MDP5_PIPE_SCALE_PHASE_STEP_Y(pipe),
@@ -827,7 +823,7 @@ static int mdp5_plane_mode_set(struct drm_plane *plane,
 		mdp5_write(mdp5_kms, REG_MDP5_PIPE_SCALE_CONFIG(pipe), config);
 	}
 
-	if (mdp5_plane->caps & MDP_PIPE_CAP_CSC) {
+	if (hwpipe->caps & MDP_PIPE_CAP_CSC) {
 		if (MDP_FORMAT_IS_YUV(format))
 			csc_enable(mdp5_kms, pipe,
 					mdp_get_default_csc_cfg(CSC_YUV2RGB));
@@ -837,7 +833,7 @@ static int mdp5_plane_mode_set(struct drm_plane *plane,
 
 	set_scanout_locked(plane, fb);
 
-	spin_unlock_irqrestore(&mdp5_plane->pipe_lock, flags);
+	spin_unlock_irqrestore(&hwpipe->pipe_lock, flags);
 
 	return ret;
 }
@@ -845,14 +841,14 @@ static int mdp5_plane_mode_set(struct drm_plane *plane,
 enum mdp5_pipe mdp5_plane_pipe(struct drm_plane *plane)
 {
 	struct mdp5_plane *mdp5_plane = to_mdp5_plane(plane);
-	return mdp5_plane->pipe;
+	return mdp5_plane->hwpipe->pipe;
 }
 
 uint32_t mdp5_plane_get_flush(struct drm_plane *plane)
 {
 	struct mdp5_plane *mdp5_plane = to_mdp5_plane(plane);
 
-	return mdp5_plane->flush_mask;
+	return mdp5_plane->hwpipe->flush_mask;
 }
 
 /* called after vsync in thread context */
@@ -861,7 +857,7 @@ void mdp5_plane_complete_commit(struct drm_plane *plane,
 {
 	struct mdp5_kms *mdp5_kms = get_kms(plane);
 	struct mdp5_plane *mdp5_plane = to_mdp5_plane(plane);
-	enum mdp5_pipe pipe = mdp5_plane->pipe;
+	enum mdp5_pipe pipe = mdp5_plane->hwpipe->pipe;
 
 	if (mdp5_kms->smp) {
 		if (plane_enabled(plane->state)) {
@@ -878,8 +874,7 @@ void mdp5_plane_complete_commit(struct drm_plane *plane,
 
 /* initialize plane */
 struct drm_plane *mdp5_plane_init(struct drm_device *dev,
-		enum mdp5_pipe pipe, bool private_plane, uint32_t reg_offset,
-		uint32_t caps)
+		struct mdp5_hw_pipe *hwpipe, bool primary)
 {
 	struct drm_plane *plane = NULL;
 	struct mdp5_plane *mdp5_plane;
@@ -894,21 +889,16 @@ struct drm_plane *mdp5_plane_init(struct drm_device *dev,
 
 	plane = &mdp5_plane->base;
 
-	mdp5_plane->pipe = pipe;
-	mdp5_plane->caps = caps;
+	mdp5_plane->hwpipe = hwpipe;
 
 	mdp5_plane->nformats = mdp_get_formats(mdp5_plane->formats,
 		ARRAY_SIZE(mdp5_plane->formats),
-		!pipe_supports_yuv(mdp5_plane->caps));
-
-	mdp5_plane->flush_mask = mdp_ctl_flush_mask_pipe(pipe);
-	mdp5_plane->reg_offset = reg_offset;
-	spin_lock_init(&mdp5_plane->pipe_lock);
+		!pipe_supports_yuv(hwpipe->caps));
 
-	type = private_plane ? DRM_PLANE_TYPE_PRIMARY : DRM_PLANE_TYPE_OVERLAY;
+	type = primary ? DRM_PLANE_TYPE_PRIMARY : DRM_PLANE_TYPE_OVERLAY;
 	ret = drm_universal_plane_init(dev, plane, 0xff, &mdp5_plane_funcs,
 				 mdp5_plane->formats, mdp5_plane->nformats,
-				 type, "%s", pipe2name(pipe));
+				 type, "%s", hwpipe->name);
 	if (ret)
 		goto fail;
 
-- 
2.7.4

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

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 2/5] drm/msm: subclass drm_atomic_state
  2016-11-05 16:25 [PATCH 0/5] drm/msm/mdp5: atomic smp + hwpipe assignment Rob Clark
  2016-11-05 16:25 ` [PATCH 1/5] drm/msm/mdp5: introduce mdp5_hw_pipe Rob Clark
@ 2016-11-05 16:25 ` Rob Clark
       [not found] ` <1478363161-29293-1-git-send-email-robdclark-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 12+ messages in thread
From: Rob Clark @ 2016-11-05 16:25 UTC (permalink / raw)
  To: dri-devel
  Cc: linux-arm-msm, freedreno, Archit Taneja, Daniel Vetter,
	Sean Paul, Rob Clark

This will give the kms backends a slot to stash their own hw specific
global state.

Signed-off-by: Rob Clark <robdclark@gmail.com>
---
 drivers/gpu/drm/msm/msm_atomic.c | 31 +++++++++++++++++++++++++++++++
 drivers/gpu/drm/msm/msm_drv.c    |  3 +++
 drivers/gpu/drm/msm/msm_drv.h    |  3 +++
 drivers/gpu/drm/msm/msm_kms.h    | 14 ++++++++++++++
 4 files changed, 51 insertions(+)

diff --git a/drivers/gpu/drm/msm/msm_atomic.c b/drivers/gpu/drm/msm/msm_atomic.c
index 4e21e1d..30b5d23 100644
--- a/drivers/gpu/drm/msm/msm_atomic.c
+++ b/drivers/gpu/drm/msm/msm_atomic.c
@@ -241,6 +241,10 @@ int msm_atomic_commit(struct drm_device *dev,
 
 	drm_atomic_helper_swap_state(state, true);
 
+	/* swap driver private state while still holding state_lock */
+	if (to_kms_state(state)->state)
+		priv->kms->funcs->swap_state(priv->kms, state);
+
 	/*
 	 * Everything below can be run asynchronously without the need to grab
 	 * any modeset locks at all under one conditions: It must be guaranteed
@@ -271,3 +275,30 @@ error:
 	drm_atomic_helper_cleanup_planes(dev, state);
 	return ret;
 }
+
+struct drm_atomic_state *msm_atomic_state_alloc(struct drm_device *dev)
+{
+	struct msm_kms_state *state = kzalloc(sizeof(*state), GFP_KERNEL);
+
+	if (!state || drm_atomic_state_init(dev, &state->base) < 0) {
+		kfree(state);
+		return NULL;
+	}
+
+	return &state->base;
+}
+
+void msm_atomic_state_clear(struct drm_atomic_state *s)
+{
+	struct msm_kms_state *state = to_kms_state(s);
+	drm_atomic_state_default_clear(&state->base);
+	kfree(state->state);
+	state->state = NULL;
+}
+
+void msm_atomic_state_free(struct drm_atomic_state *state)
+{
+	kfree(to_kms_state(state)->state);
+	drm_atomic_state_default_release(state);
+	kfree(state);
+}
diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
index 0f4409d..57cba3f 100644
--- a/drivers/gpu/drm/msm/msm_drv.c
+++ b/drivers/gpu/drm/msm/msm_drv.c
@@ -46,6 +46,9 @@ static const struct drm_mode_config_funcs mode_config_funcs = {
 	.output_poll_changed = msm_fb_output_poll_changed,
 	.atomic_check = msm_atomic_check,
 	.atomic_commit = msm_atomic_commit,
+	.atomic_state_alloc = msm_atomic_state_alloc,
+	.atomic_state_clear = msm_atomic_state_clear,
+	.atomic_state_free = msm_atomic_state_free,
 };
 
 int msm_register_address_space(struct drm_device *dev,
diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h
index 03ce6a18..175b2bf 100644
--- a/drivers/gpu/drm/msm/msm_drv.h
+++ b/drivers/gpu/drm/msm/msm_drv.h
@@ -179,6 +179,9 @@ int msm_atomic_check(struct drm_device *dev,
 		     struct drm_atomic_state *state);
 int msm_atomic_commit(struct drm_device *dev,
 		struct drm_atomic_state *state, bool nonblock);
+struct drm_atomic_state *msm_atomic_state_alloc(struct drm_device *dev);
+void msm_atomic_state_clear(struct drm_atomic_state *state);
+void msm_atomic_state_free(struct drm_atomic_state *state);
 
 int msm_register_address_space(struct drm_device *dev,
 		struct msm_gem_address_space *aspace);
diff --git a/drivers/gpu/drm/msm/msm_kms.h b/drivers/gpu/drm/msm/msm_kms.h
index 40e41e5..cb9758b 100644
--- a/drivers/gpu/drm/msm/msm_kms.h
+++ b/drivers/gpu/drm/msm/msm_kms.h
@@ -40,6 +40,8 @@ struct msm_kms_funcs {
 	irqreturn_t (*irq)(struct msm_kms *kms);
 	int (*enable_vblank)(struct msm_kms *kms, struct drm_crtc *crtc);
 	void (*disable_vblank)(struct msm_kms *kms, struct drm_crtc *crtc);
+	/* swap global atomic state: */
+	void (*swap_state)(struct msm_kms *kms, struct drm_atomic_state *state);
 	/* modeset, bracketing atomic_commit(): */
 	void (*prepare_commit)(struct msm_kms *kms, struct drm_atomic_state *state);
 	void (*complete_commit)(struct msm_kms *kms, struct drm_atomic_state *state);
@@ -65,6 +67,18 @@ struct msm_kms {
 	int irq;
 };
 
+/**
+ * Subclass of drm_atomic_state, to allow kms backend to have driver
+ * private global state.  The kms backend can do whatever it wants
+ * with the ->state ptr.  On ->atomic_state_clear() the ->state ptr
+ * is kfree'd and set back to NULL.
+ */
+struct msm_kms_state {
+	struct drm_atomic_state base;
+	void *state;
+};
+#define to_kms_state(x) container_of(x, struct msm_kms_state, base)
+
 static inline void msm_kms_init(struct msm_kms *kms,
 		const struct msm_kms_funcs *funcs)
 {
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 3/5] drm/msm/mdp5: add skeletal mdp5_state
       [not found] ` <1478363161-29293-1-git-send-email-robdclark-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
@ 2016-11-05 16:25   ` Rob Clark
       [not found]     ` <1478363161-29293-4-git-send-email-robdclark-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
  0 siblings, 1 reply; 12+ messages in thread
From: Rob Clark @ 2016-11-05 16:25 UTC (permalink / raw)
  To: dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
  Cc: Archit Taneja, linux-arm-msm-u79uwXL29TY76Z2rM5mHXA, Rob Clark,
	Sean Paul, Daniel Vetter,
	freedreno-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW

Add basic state duplication/apply mechanism.  Following commits will
move actual global hw state into this.

The state_lock allows multiple concurrent updates to proceed as long as
they don't both try to alter global state.  The ww_mutex mechanism will
trigger backoff in case of deadlock between multiple threads trying to
update state.

Signed-off-by: Rob Clark <robdclark@gmail.com>
---
 drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c | 43 +++++++++++++++++++++++++++++++++
 drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h | 22 +++++++++++++++++
 2 files changed, 65 insertions(+)

diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c
index d3d45ed..ca6dfeb 100644
--- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c
+++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c
@@ -72,6 +72,39 @@ static int mdp5_hw_init(struct msm_kms *kms)
 	return 0;
 }
 
+struct mdp5_state *mdp5_get_state(struct drm_atomic_state *s)
+{
+	struct msm_drm_private *priv = s->dev->dev_private;
+	struct mdp5_kms *mdp5_kms = to_mdp5_kms(to_mdp_kms(priv->kms));
+	struct msm_kms_state *state = to_kms_state(s);
+	struct mdp5_state *new_state;
+	int ret;
+
+	if (state->state)
+		return state->state;
+
+	ret = drm_modeset_lock(&mdp5_kms->state_lock, s->acquire_ctx);
+	if (ret)
+		return ERR_PTR(ret);
+
+	new_state = kmalloc(sizeof(*mdp5_kms->state), GFP_KERNEL);
+	if (!new_state)
+		return ERR_PTR(-ENOMEM);
+
+	/* Copy state: */
+	/* TODO */
+
+	state->state = new_state;
+
+	return new_state;
+}
+
+static void mdp5_swap_state(struct msm_kms *kms, struct drm_atomic_state *state)
+{
+	struct mdp5_kms *mdp5_kms = to_mdp5_kms(to_mdp_kms(kms));
+	swap(to_kms_state(state)->state, mdp5_kms->state);
+}
+
 static void mdp5_prepare_commit(struct msm_kms *kms, struct drm_atomic_state *state)
 {
 	struct mdp5_kms *mdp5_kms = to_mdp5_kms(to_mdp_kms(kms));
@@ -140,6 +173,7 @@ static const struct mdp_kms_funcs kms_funcs = {
 		.irq             = mdp5_irq,
 		.enable_vblank   = mdp5_enable_vblank,
 		.disable_vblank  = mdp5_disable_vblank,
+		.swap_state      = mdp5_swap_state,
 		.prepare_commit  = mdp5_prepare_commit,
 		.complete_commit = mdp5_complete_commit,
 		.wait_for_crtc_commit_done = mdp5_wait_for_crtc_commit_done,
@@ -645,6 +679,8 @@ static void mdp5_destroy(struct platform_device *pdev)
 
 	if (mdp5_kms->rpm_enabled)
 		pm_runtime_disable(&pdev->dev);
+
+	kfree(mdp5_kms->state);
 }
 
 static int construct_pipes(struct mdp5_kms *mdp5_kms, int cnt,
@@ -729,6 +765,13 @@ static int mdp5_init(struct platform_device *pdev, struct drm_device *dev)
 	mdp5_kms->dev = dev;
 	mdp5_kms->pdev = pdev;
 
+	drm_modeset_lock_init(&mdp5_kms->state_lock);
+	mdp5_kms->state = kzalloc(sizeof(*mdp5_kms->state), GFP_KERNEL);
+	if (!mdp5_kms->state) {
+		ret = -ENOMEM;
+		goto fail;
+	}
+
 	mdp5_kms->mmio = msm_ioremap(pdev, "mdp_phys", "MDP5");
 	if (IS_ERR(mdp5_kms->mmio)) {
 		ret = PTR_ERR(mdp5_kms->mmio);
diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h
index 88120c5..52914ec 100644
--- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h
+++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h
@@ -27,6 +27,8 @@
 #include "mdp5_pipe.h"
 #include "mdp5_smp.h"
 
+struct mdp5_state;
+
 struct mdp5_kms {
 	struct mdp_kms base;
 
@@ -40,6 +42,11 @@ struct mdp5_kms {
 	struct mdp5_cfg_handler *cfg;
 	uint32_t caps;	/* MDP capabilities (MDP_CAP_XXX bits) */
 
+	/**
+	 * Global atomic state.  Do not access directly, use mdp5_get_state()
+	 */
+	struct mdp5_state *state;
+	struct drm_modeset_lock state_lock;
 
 	/* mapper-id used to request GEM buffer mapped for scanout: */
 	int id;
@@ -69,6 +76,21 @@ struct mdp5_kms {
 };
 #define to_mdp5_kms(x) container_of(x, struct mdp5_kms, base)
 
+/* Global atomic state for tracking resources that are shared across
+ * multiple kms objects (planes/crtcs/etc).
+ *
+ * For atomic updates which require modifying global state,
+ */
+struct mdp5_state {
+	uint32_t dummy;
+};
+
+struct mdp5_state *__must_check
+mdp5_get_state(struct drm_atomic_state *s);
+
+/* Atomic plane state.  Subclasses the base drm_plane_state in order to
+ * track assigned hwpipe and hw specific state.
+ */
 struct mdp5_plane_state {
 	struct drm_plane_state base;
 
-- 
2.7.4

_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 4/5] drm/msm/mdp5: dynamically assign hw pipes to planes
  2016-11-05 16:25 [PATCH 0/5] drm/msm/mdp5: atomic smp + hwpipe assignment Rob Clark
                   ` (2 preceding siblings ...)
       [not found] ` <1478363161-29293-1-git-send-email-robdclark-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
@ 2016-11-05 16:26 ` Rob Clark
  2016-11-07 10:29   ` Archit Taneja
  2016-11-05 16:26 ` [PATCH 5/5] drm/msm/mdp5: handle SMP block allocations "atomically" Rob Clark
  4 siblings, 1 reply; 12+ messages in thread
From: Rob Clark @ 2016-11-05 16:26 UTC (permalink / raw)
  To: dri-devel; +Cc: linux-arm-msm, freedreno

(re)assign the hw pipes to planes based on required caps, and to handle
situations where we could not modify an in-use plane (ie. SMP block
reallocation).

This means all planes advertise the superset of formats and properties.
Userspace must (as always) use atomic TEST_ONLY step for atomic updates,
as not all planes may be available for use on every frame.

The mapping of hwpipe to plane is stored in mdp5_state, so that state
updates are atomically committed in the same way that plane/etc state
updates are managed.  This is needed because the mdp5_plane_state keeps
a pointer to the hwpipe, and we don't want global state to become out
of sync with the plane state if an atomic update fails, we hit deadlock/
backoff scenario, etc.  The use of state_lock keeps multiple parallel
updates which both re-assign hwpipes properly serialized.

Signed-off-by: Rob Clark <robdclark@gmail.com>
---
 drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c  |   2 +-
 drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c   |   4 +-
 drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h   |   7 +-
 drivers/gpu/drm/msm/mdp/mdp5/mdp5_pipe.c  |  70 +++++++++++++
 drivers/gpu/drm/msm/mdp/mdp5/mdp5_pipe.h  |  10 ++
 drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c | 158 ++++++++++++++++--------------
 6 files changed, 171 insertions(+), 80 deletions(-)

diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c
index 6213f51..99958f7 100644
--- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c
+++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c
@@ -407,7 +407,7 @@ static int mdp5_crtc_atomic_check(struct drm_crtc *crtc,
 	for (i = 0; i < cnt; i++) {
 		pstates[i].state->stage = STAGE_BASE + i + base;
 		DBG("%s: assign pipe %s on stage=%d", crtc->name,
-				pipe2name(mdp5_plane_pipe(pstates[i].plane)),
+				pstates[i].plane->name,
 				pstates[i].state->stage);
 	}
 
diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c
index ca6dfeb..3542adf 100644
--- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c
+++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c
@@ -92,7 +92,7 @@ struct mdp5_state *mdp5_get_state(struct drm_atomic_state *s)
 		return ERR_PTR(-ENOMEM);
 
 	/* Copy state: */
-	/* TODO */
+	new_state->hwpipe = mdp5_kms->state->hwpipe;
 
 	state->state = new_state;
 
@@ -377,7 +377,7 @@ static int modeset_init(struct mdp5_kms *mdp5_kms)
 		struct drm_plane *plane;
 		struct drm_crtc *crtc;
 
-		plane = mdp5_plane_init(dev, mdp5_kms->hwpipes[i], primary);
+		plane = mdp5_plane_init(dev, primary);
 		if (IS_ERR(plane)) {
 			ret = PTR_ERR(plane);
 			dev_err(dev->dev, "failed to construct plane %d (%d)\n", i, ret);
diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h
index 52914ec..4475090 100644
--- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h
+++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h
@@ -82,7 +82,7 @@ struct mdp5_kms {
  * For atomic updates which require modifying global state,
  */
 struct mdp5_state {
-	uint32_t dummy;
+	struct mdp5_hw_pipe_state hwpipe;
 };
 
 struct mdp5_state *__must_check
@@ -94,6 +94,8 @@ mdp5_get_state(struct drm_atomic_state *s);
 struct mdp5_plane_state {
 	struct drm_plane_state base;
 
+	struct mdp5_hw_pipe *hwpipe;
+
 	/* aligned with property */
 	uint8_t premultiplied;
 	uint8_t zpos;
@@ -232,8 +234,7 @@ uint32_t mdp5_plane_get_flush(struct drm_plane *plane);
 void mdp5_plane_complete_commit(struct drm_plane *plane,
 	struct drm_plane_state *state);
 enum mdp5_pipe mdp5_plane_pipe(struct drm_plane *plane);
-struct drm_plane *mdp5_plane_init(struct drm_device *dev,
-		struct mdp5_hw_pipe *hwpipe, bool primary);
+struct drm_plane *mdp5_plane_init(struct drm_device *dev, bool primary);
 
 uint32_t mdp5_crtc_vblank(struct drm_crtc *crtc);
 
diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_pipe.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_pipe.c
index 7f3e8e50..115de7d 100644
--- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_pipe.c
+++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_pipe.c
@@ -17,6 +17,76 @@
 
 #include "mdp5_kms.h"
 
+struct mdp5_hw_pipe *mdp5_pipe_assign(struct drm_atomic_state *s,
+		struct drm_plane *plane, uint32_t caps)
+{
+	struct msm_drm_private *priv = s->dev->dev_private;
+	struct mdp5_kms *mdp5_kms = to_mdp5_kms(to_mdp_kms(priv->kms));
+	struct mdp5_state *state = mdp5_get_state(s);
+	struct mdp5_hw_pipe_state *old_state, *new_state;
+	struct mdp5_hw_pipe *hwpipe = NULL;
+	int i;
+
+	if (IS_ERR(state))
+		return ERR_CAST(state);
+
+	/* grab old_state after mdp5_get_state(), since now we hold lock: */
+	old_state = &mdp5_kms->state->hwpipe;
+	new_state = &state->hwpipe;
+
+	for (i = 0; i < mdp5_kms->num_hwpipes; i++) {
+		struct mdp5_hw_pipe *cur = mdp5_kms->hwpipes[i];
+
+		/* skip if already in-use.. check both new and old state,
+		 * since we cannot immediately re-use a pipe that is
+		 * released in the current update in some cases:
+		 *  (1) mdp5 has SMP (non-double-buffered)
+		 *  (2) hw pipe previously assigned to different CRTC
+		 *      (vblanks might not be aligned)
+		 */
+		if (new_state->hwpipe_to_plane[cur->idx] ||
+				old_state->hwpipe_to_plane[cur->idx])
+			continue;
+
+		/* skip if doesn't support some required caps: */
+		if (caps & ~cur->caps)
+			continue;
+
+		/* possible candidate, take the one with the
+		 * fewest unneeded caps bits set:
+		 */
+		if (!hwpipe || (hweight_long(cur->caps & ~caps) <
+				hweight_long(hwpipe->caps & ~caps)))
+			hwpipe = cur;
+	}
+
+	if (!hwpipe)
+		return ERR_PTR(-ENOMEM);
+
+	DBG("%s: assign to plane %s for caps %x",
+			hwpipe->name, plane->name, caps);
+	new_state->hwpipe_to_plane[hwpipe->idx] = plane;
+
+	return hwpipe;
+}
+
+void mdp5_pipe_release(struct drm_atomic_state *s, struct mdp5_hw_pipe *hwpipe)
+{
+	struct mdp5_state *state = mdp5_get_state(s);
+	struct mdp5_hw_pipe_state *new_state = &state->hwpipe;
+
+	if (!hwpipe)
+		return;
+
+	if (WARN_ON(!new_state->hwpipe_to_plane[hwpipe->idx]))
+		return;
+
+	DBG("%s: release from plane %s", hwpipe->name,
+		new_state->hwpipe_to_plane[hwpipe->idx]->name);
+
+	new_state->hwpipe_to_plane[hwpipe->idx] = NULL;
+}
+
 void mdp5_pipe_destroy(struct mdp5_hw_pipe *hwpipe)
 {
 	kfree(hwpipe);
diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_pipe.h b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_pipe.h
index dcfa0e0..bac5900 100644
--- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_pipe.h
+++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_pipe.h
@@ -32,6 +32,16 @@ struct mdp5_hw_pipe {
 	uint32_t flush_mask;      /* used to commit pipe registers */
 };
 
+/* global atomic state of assignment between pipes and planes: */
+struct mdp5_hw_pipe_state {
+	struct drm_plane *hwpipe_to_plane[16];
+};
+
+struct mdp5_hw_pipe *__must_check
+mdp5_pipe_assign(struct drm_atomic_state *s, struct drm_plane *plane,
+		uint32_t caps);
+void mdp5_pipe_release(struct drm_atomic_state *s, struct mdp5_hw_pipe *hwpipe);
+
 struct mdp5_hw_pipe *mdp5_pipe_init(enum mdp5_pipe pipe,
 		uint32_t reg_offset, uint32_t caps);
 void mdp5_pipe_destroy(struct mdp5_hw_pipe *hwpipe);
diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
index e4ecfeb..64e97ef 100644
--- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
+++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
@@ -22,8 +22,6 @@
 struct mdp5_plane {
 	struct drm_plane base;
 
-	struct mdp5_hw_pipe *hwpipe;
-
 	uint32_t nformats;
 	uint32_t formats[32];
 };
@@ -63,12 +61,6 @@ static void mdp5_plane_destroy(struct drm_plane *plane)
 static void mdp5_plane_install_rotation_property(struct drm_device *dev,
 		struct drm_plane *plane)
 {
-	struct mdp5_plane *mdp5_plane = to_mdp5_plane(plane);
-
-	if (!(mdp5_plane->hwpipe->caps & MDP_PIPE_CAP_HFLIP) &&
-		!(mdp5_plane->hwpipe->caps & MDP_PIPE_CAP_VFLIP))
-		return;
-
 	drm_plane_create_rotation_property(plane,
 					   DRM_ROTATE_0,
 					   DRM_ROTATE_0 |
@@ -184,6 +176,8 @@ mdp5_plane_atomic_print_state(struct drm_printer *p,
 {
 	struct mdp5_plane_state *pstate = to_mdp5_plane_state(state);
 
+	drm_printf(p, "\thwpipe=%s\n", pstate->hwpipe ?
+			pstate->hwpipe->name : "(null)");
 	drm_printf(p, "\tpremultiplied=%u\n", pstate->premultiplied);
 	drm_printf(p, "\tzpos=%u\n", pstate->zpos);
 	drm_printf(p, "\talpha=%u\n", pstate->alpha);
@@ -237,10 +231,12 @@ mdp5_plane_duplicate_state(struct drm_plane *plane)
 static void mdp5_plane_destroy_state(struct drm_plane *plane,
 		struct drm_plane_state *state)
 {
+	struct mdp5_plane_state *pstate = to_mdp5_plane_state(state);
+
 	if (state->fb)
 		drm_framebuffer_unreference(state->fb);
 
-	kfree(to_mdp5_plane_state(state));
+	kfree(pstate);
 }
 
 static const struct drm_plane_funcs mdp5_plane_funcs = {
@@ -285,70 +281,81 @@ static void mdp5_plane_cleanup_fb(struct drm_plane *plane,
 static int mdp5_plane_atomic_check(struct drm_plane *plane,
 		struct drm_plane_state *state)
 {
-	struct mdp5_plane *mdp5_plane = to_mdp5_plane(plane);
+	struct mdp5_plane_state *mdp5_state = to_mdp5_plane_state(state);
 	struct drm_plane_state *old_state = plane->state;
-	const struct mdp_format *format;
-	bool vflip, hflip;
+	bool new_hwpipe = false;
+	uint32_t caps = 0;
 
 	DBG("%s: check (%d -> %d)", plane->name,
 			plane_enabled(old_state), plane_enabled(state));
 
+	/* We don't allow faster-than-vblank updates.. if we did add this
+	 * some day, we would need to disallow in cases where hwpipe
+	 * changes
+	 */
+	if (WARN_ON(to_mdp5_plane_state(old_state)->pending))
+		return -EBUSY;
+
 	if (plane_enabled(state)) {
 		unsigned int rotation;
+		const struct mdp_format *format;
 
 		format = to_mdp_format(msm_framebuffer_format(state->fb));
-		if (MDP_FORMAT_IS_YUV(format) &&
-			!pipe_supports_yuv(mdp5_plane->hwpipe->caps)) {
-			DBG("Pipe doesn't support YUV\n");
-
-			return -EINVAL;
-		}
-
-		if (!(mdp5_plane->hwpipe->caps & MDP_PIPE_CAP_SCALE) &&
-			(((state->src_w >> 16) != state->crtc_w) ||
-			((state->src_h >> 16) != state->crtc_h))) {
-			DBG("Pipe doesn't support scaling (%dx%d -> %dx%d)\n",
-				state->src_w >> 16, state->src_h >> 16,
-				state->crtc_w, state->crtc_h);
+		if (MDP_FORMAT_IS_YUV(format))
+			caps |= MDP_PIPE_CAP_SCALE | MDP_PIPE_CAP_CSC;
 
-			return -EINVAL;
-		}
+		if (((state->src_w >> 16) != state->crtc_w) ||
+				((state->src_h >> 16) != state->crtc_h))
+			caps |= MDP_PIPE_CAP_SCALE;
 
 		rotation = drm_rotation_simplify(state->rotation,
 						 DRM_ROTATE_0 |
 						 DRM_REFLECT_X |
 						 DRM_REFLECT_Y);
 
-		hflip = !!(rotation & DRM_REFLECT_X);
-		vflip = !!(rotation & DRM_REFLECT_Y);
-
-		if ((vflip && !(mdp5_plane->hwpipe->caps & MDP_PIPE_CAP_VFLIP)) ||
-			(hflip && !(mdp5_plane->hwpipe->caps & MDP_PIPE_CAP_HFLIP))) {
-			DBG("Pipe doesn't support flip\n");
-
-			return -EINVAL;
+		if (rotation & DRM_REFLECT_X)
+			caps |= MDP_PIPE_CAP_HFLIP;
+
+		if (rotation & DRM_REFLECT_Y)
+			caps |= MDP_PIPE_CAP_VFLIP;
+
+		/* (re)allocate hw pipe if we don't have one or caps-mismatch: */
+		if (!mdp5_state->hwpipe || (caps & ~mdp5_state->hwpipe->caps))
+			new_hwpipe = true;
+
+		if (plane_enabled(old_state)) {
+			bool full_modeset = false;
+			if (state->fb->pixel_format != old_state->fb->pixel_format) {
+				DBG("%s: pixel_format change!", plane->name);
+				full_modeset = true;
+			}
+			if (state->src_w != old_state->src_w) {
+				DBG("%s: src_w change!", plane->name);
+				full_modeset = true;
+			}
+			if (full_modeset) {
+				/* cannot change SMP block allocation during
+				 * scanout:
+				 */
+				if (get_kms(plane)->smp)
+					new_hwpipe = true;
+			}
 		}
-	}
 
-	if (plane_enabled(state) && plane_enabled(old_state)) {
-		/* we cannot change SMP block configuration during scanout: */
-		bool full_modeset = false;
-		if (state->fb->pixel_format != old_state->fb->pixel_format) {
-			DBG("%s: pixel_format change!", plane->name);
-			full_modeset = true;
-		}
-		if (state->src_w != old_state->src_w) {
-			DBG("%s: src_w change!", plane->name);
-			full_modeset = true;
-		}
-		if (to_mdp5_plane_state(old_state)->pending) {
-			DBG("%s: still pending!", plane->name);
-			full_modeset = true;
-		}
-		if (full_modeset) {
-			struct drm_crtc_state *crtc_state =
-					drm_atomic_get_crtc_state(state->state, state->crtc);
-			crtc_state->mode_changed = true;
+		/* (re)assign hwpipe if needed, otherwise keep old one: */
+		if (new_hwpipe) {
+			/* TODO maybe we want to re-assign hwpipe sometimes
+			 * in cases when we no-longer need some caps to make
+			 * it available for other planes?
+			 */
+			struct mdp5_hw_pipe *hwpipe = mdp5_state->hwpipe;
+			mdp5_state->hwpipe =
+				mdp5_pipe_assign(state->state, plane, caps);
+			if (IS_ERR(mdp5_state->hwpipe)) {
+				DBG("%s: failed to assign hwpipe!", plane->name);
+				return PTR_ERR(mdp5_state->hwpipe);
+			}
+			mdp5_pipe_release(state->state, hwpipe);
 		}
 	}
 
@@ -389,9 +396,9 @@ static const struct drm_plane_helper_funcs mdp5_plane_helper_funcs = {
 static void set_scanout_locked(struct drm_plane *plane,
 		struct drm_framebuffer *fb)
 {
-	struct mdp5_plane *mdp5_plane = to_mdp5_plane(plane);
 	struct mdp5_kms *mdp5_kms = get_kms(plane);
-	enum mdp5_pipe pipe = mdp5_plane->hwpipe->pipe;
+	struct mdp5_hw_pipe *hwpipe = to_mdp5_plane_state(plane->state)->hwpipe;
+	enum mdp5_pipe pipe = hwpipe->pipe;
 
 	mdp5_write(mdp5_kms, REG_MDP5_PIPE_SRC_STRIDE_A(pipe),
 			MDP5_PIPE_SRC_STRIDE_A_P0(fb->pitches[0]) |
@@ -671,9 +678,8 @@ static int mdp5_plane_mode_set(struct drm_plane *plane,
 		uint32_t src_x, uint32_t src_y,
 		uint32_t src_w, uint32_t src_h)
 {
-	struct mdp5_plane *mdp5_plane = to_mdp5_plane(plane);
 	struct drm_plane_state *pstate = plane->state;
-	struct mdp5_hw_pipe *hwpipe = mdp5_plane->hwpipe;
+	struct mdp5_hw_pipe *hwpipe = to_mdp5_plane_state(pstate)->hwpipe;
 	struct mdp5_kms *mdp5_kms = get_kms(plane);
 	enum mdp5_pipe pipe = hwpipe->pipe;
 	const struct mdp_format *format;
@@ -840,15 +846,22 @@ static int mdp5_plane_mode_set(struct drm_plane *plane,
 
 enum mdp5_pipe mdp5_plane_pipe(struct drm_plane *plane)
 {
-	struct mdp5_plane *mdp5_plane = to_mdp5_plane(plane);
-	return mdp5_plane->hwpipe->pipe;
+	struct mdp5_plane_state *pstate = to_mdp5_plane_state(plane->state);
+
+	if (WARN_ON(!pstate->hwpipe))
+		return 0;
+
+	return pstate->hwpipe->pipe;
 }
 
 uint32_t mdp5_plane_get_flush(struct drm_plane *plane)
 {
-	struct mdp5_plane *mdp5_plane = to_mdp5_plane(plane);
+	struct mdp5_plane_state *pstate = to_mdp5_plane_state(plane->state);
+
+	if (WARN_ON(!pstate->hwpipe))
+		return 0;
 
-	return mdp5_plane->hwpipe->flush_mask;
+	return pstate->hwpipe->flush_mask;
 }
 
 /* called after vsync in thread context */
@@ -856,10 +869,11 @@ void mdp5_plane_complete_commit(struct drm_plane *plane,
 	struct drm_plane_state *state)
 {
 	struct mdp5_kms *mdp5_kms = get_kms(plane);
-	struct mdp5_plane *mdp5_plane = to_mdp5_plane(plane);
-	enum mdp5_pipe pipe = mdp5_plane->hwpipe->pipe;
+	struct mdp5_plane_state *pstate = to_mdp5_plane_state(plane->state);
+
+	if (mdp5_kms->smp && pstate->hwpipe) {
+		enum mdp5_pipe pipe = pstate->hwpipe->pipe;
 
-	if (mdp5_kms->smp) {
 		if (plane_enabled(plane->state)) {
 			DBG("%s: complete flip", plane->name);
 			mdp5_smp_commit(mdp5_kms->smp, pipe);
@@ -869,12 +883,11 @@ void mdp5_plane_complete_commit(struct drm_plane *plane,
 		}
 	}
 
-	to_mdp5_plane_state(plane->state)->pending = false;
+	pstate->pending = false;
 }
 
 /* initialize plane */
-struct drm_plane *mdp5_plane_init(struct drm_device *dev,
-		struct mdp5_hw_pipe *hwpipe, bool primary)
+struct drm_plane *mdp5_plane_init(struct drm_device *dev, bool primary)
 {
 	struct drm_plane *plane = NULL;
 	struct mdp5_plane *mdp5_plane;
@@ -889,16 +902,13 @@ struct drm_plane *mdp5_plane_init(struct drm_device *dev,
 
 	plane = &mdp5_plane->base;
 
-	mdp5_plane->hwpipe = hwpipe;
-
 	mdp5_plane->nformats = mdp_get_formats(mdp5_plane->formats,
-		ARRAY_SIZE(mdp5_plane->formats),
-		!pipe_supports_yuv(hwpipe->caps));
+		ARRAY_SIZE(mdp5_plane->formats), false);
 
 	type = primary ? DRM_PLANE_TYPE_PRIMARY : DRM_PLANE_TYPE_OVERLAY;
 	ret = drm_universal_plane_init(dev, plane, 0xff, &mdp5_plane_funcs,
 				 mdp5_plane->formats, mdp5_plane->nformats,
-				 type, "%s", hwpipe->name);
+				 type, NULL);
 	if (ret)
 		goto fail;
 
-- 
2.7.4

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

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 5/5] drm/msm/mdp5: handle SMP block allocations "atomically"
  2016-11-05 16:25 [PATCH 0/5] drm/msm/mdp5: atomic smp + hwpipe assignment Rob Clark
                   ` (3 preceding siblings ...)
  2016-11-05 16:26 ` [PATCH 4/5] drm/msm/mdp5: dynamically assign hw pipes to planes Rob Clark
@ 2016-11-05 16:26 ` Rob Clark
  4 siblings, 0 replies; 12+ messages in thread
From: Rob Clark @ 2016-11-05 16:26 UTC (permalink / raw)
  To: dri-devel; +Cc: linux-arm-msm, freedreno

Previously, SMP block allocation was not checked in the plane's
atomic_check() fxn, so we could fail allocation SMP block allocation at
atomic_update() time.  Re-work the block allocation to request blocks
during atomic_check(), but not update the hw until committing the atomic
update.

Since SMP blocks allocated at atomic_check() time, we need to manage the
SMP state as part of mdp5_state (global atomic state).  This actually
ends up significantly simplifying the SMP management, as the SMP module
does not need to manage the intermediate state between assigning new
blocks before setting flush bits and releasing old blocks after vblank.
(The SMP registers and SMP allocation is not double-buffered, so newly
allocated blocks need to be updated in kms->prepare_commit() released
blocks in kms->complete_commit().)

Signed-off-by: Rob Clark <robdclark@gmail.com>
---
 drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c   |  11 +-
 drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h   |   1 +
 drivers/gpu/drm/msm/mdp/mdp5/mdp5_pipe.c  |  21 ++-
 drivers/gpu/drm/msm/mdp/mdp5/mdp5_pipe.h  |   7 +-
 drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c |  62 ++-----
 drivers/gpu/drm/msm/mdp/mdp5/mdp5_smp.c   | 270 ++++++++++--------------------
 drivers/gpu/drm/msm/mdp/mdp5/mdp5_smp.h   |  66 ++++++--
 7 files changed, 193 insertions(+), 245 deletions(-)

diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c
index 3542adf..5f95947 100644
--- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c
+++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c
@@ -93,6 +93,8 @@ struct mdp5_state *mdp5_get_state(struct drm_atomic_state *s)
 
 	/* Copy state: */
 	new_state->hwpipe = mdp5_kms->state->hwpipe;
+	if (mdp5_kms->smp)
+		new_state->smp = mdp5_kms->state->smp;
 
 	state->state = new_state;
 
@@ -108,7 +110,11 @@ static void mdp5_swap_state(struct msm_kms *kms, struct drm_atomic_state *state)
 static void mdp5_prepare_commit(struct msm_kms *kms, struct drm_atomic_state *state)
 {
 	struct mdp5_kms *mdp5_kms = to_mdp5_kms(to_mdp_kms(kms));
+
 	mdp5_enable(mdp5_kms);
+
+	if (mdp5_kms->smp)
+		mdp5_smp_prepare_commit(mdp5_kms->smp, &mdp5_kms->state->smp);
 }
 
 static void mdp5_complete_commit(struct msm_kms *kms, struct drm_atomic_state *state)
@@ -121,6 +127,9 @@ static void mdp5_complete_commit(struct msm_kms *kms, struct drm_atomic_state *s
 	for_each_plane_in_state(state, plane, plane_state, i)
 		mdp5_plane_complete_commit(plane, plane_state);
 
+	if (mdp5_kms->smp)
+		mdp5_smp_complete_commit(mdp5_kms->smp, &mdp5_kms->state->smp);
+
 	mdp5_disable(mdp5_kms);
 }
 
@@ -825,7 +834,7 @@ static int mdp5_init(struct platform_device *pdev, struct drm_device *dev)
 	 * this section initializes the SMP:
 	 */
 	if (mdp5_kms->caps & MDP_CAP_SMP) {
-		mdp5_kms->smp = mdp5_smp_init(mdp5_kms->dev, &config->hw->smp);
+		mdp5_kms->smp = mdp5_smp_init(mdp5_kms, &config->hw->smp);
 		if (IS_ERR(mdp5_kms->smp)) {
 			ret = PTR_ERR(mdp5_kms->smp);
 			mdp5_kms->smp = NULL;
diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h
index 4475090..bc92f9f 100644
--- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h
+++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h
@@ -83,6 +83,7 @@ struct mdp5_kms {
  */
 struct mdp5_state {
 	struct mdp5_hw_pipe_state hwpipe;
+	struct mdp5_smp_state smp;
 };
 
 struct mdp5_state *__must_check
diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_pipe.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_pipe.c
index 115de7d..720f5f2 100644
--- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_pipe.c
+++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_pipe.c
@@ -18,7 +18,7 @@
 #include "mdp5_kms.h"
 
 struct mdp5_hw_pipe *mdp5_pipe_assign(struct drm_atomic_state *s,
-		struct drm_plane *plane, uint32_t caps)
+		struct drm_plane *plane, uint32_t caps, uint32_t blkcfg)
 {
 	struct msm_drm_private *priv = s->dev->dev_private;
 	struct mdp5_kms *mdp5_kms = to_mdp5_kms(to_mdp_kms(priv->kms));
@@ -63,6 +63,18 @@ struct mdp5_hw_pipe *mdp5_pipe_assign(struct drm_atomic_state *s,
 	if (!hwpipe)
 		return ERR_PTR(-ENOMEM);
 
+	if (mdp5_kms->smp) {
+		int ret;
+
+		DBG("%s: alloc SMP blocks", hwpipe->name);
+		ret = mdp5_smp_assign(mdp5_kms->smp, &state->smp,
+				hwpipe->pipe, blkcfg);
+		if (ret)
+			return ERR_PTR(-ENOMEM);
+
+		hwpipe->blkcfg = blkcfg;
+	}
+
 	DBG("%s: assign to plane %s for caps %x",
 			hwpipe->name, plane->name, caps);
 	new_state->hwpipe_to_plane[hwpipe->idx] = plane;
@@ -72,6 +84,8 @@ struct mdp5_hw_pipe *mdp5_pipe_assign(struct drm_atomic_state *s,
 
 void mdp5_pipe_release(struct drm_atomic_state *s, struct mdp5_hw_pipe *hwpipe)
 {
+	struct msm_drm_private *priv = s->dev->dev_private;
+	struct mdp5_kms *mdp5_kms = to_mdp5_kms(to_mdp_kms(priv->kms));
 	struct mdp5_state *state = mdp5_get_state(s);
 	struct mdp5_hw_pipe_state *new_state = &state->hwpipe;
 
@@ -84,6 +98,11 @@ void mdp5_pipe_release(struct drm_atomic_state *s, struct mdp5_hw_pipe *hwpipe)
 	DBG("%s: release from plane %s", hwpipe->name,
 		new_state->hwpipe_to_plane[hwpipe->idx]->name);
 
+	if (mdp5_kms->smp) {
+		DBG("%s: free SMP blocks", hwpipe->name);
+		mdp5_smp_release(mdp5_kms->smp, &state->smp, hwpipe->pipe);
+	}
+
 	new_state->hwpipe_to_plane[hwpipe->idx] = NULL;
 }
 
diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_pipe.h b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_pipe.h
index bac5900..26643ac 100644
--- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_pipe.h
+++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_pipe.h
@@ -30,6 +30,11 @@ struct mdp5_hw_pipe {
 	uint32_t caps;
 
 	uint32_t flush_mask;      /* used to commit pipe registers */
+
+	/* number of smp blocks per plane, ie:
+	 *   nblks_y | (nblks_u << 8) | (nblks_v << 16)
+	 */
+	uint32_t blkcfg;
 };
 
 /* global atomic state of assignment between pipes and planes: */
@@ -39,7 +44,7 @@ struct mdp5_hw_pipe_state {
 
 struct mdp5_hw_pipe *__must_check
 mdp5_pipe_assign(struct drm_atomic_state *s, struct drm_plane *plane,
-		uint32_t caps);
+		uint32_t caps, uint32_t blkcfg);
 void mdp5_pipe_release(struct drm_atomic_state *s, struct mdp5_hw_pipe *hwpipe);
 
 struct mdp5_hw_pipe *mdp5_pipe_init(enum mdp5_pipe pipe,
diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
index 64e97ef..49cb9f9 100644
--- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
+++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
@@ -299,6 +299,8 @@ static int mdp5_plane_atomic_check(struct drm_plane *plane,
 	if (plane_enabled(state)) {
 		unsigned int rotation;
 		const struct mdp_format *format;
+		struct mdp5_kms *mdp5_kms = get_kms(plane);
+		uint32_t blkcfg = 0;
 
 		format = to_mdp_format(msm_framebuffer_format(state->fb));
 		if (MDP_FORMAT_IS_YUV(format))
@@ -323,23 +325,15 @@ static int mdp5_plane_atomic_check(struct drm_plane *plane,
 		if (!mdp5_state->hwpipe || (caps & ~mdp5_state->hwpipe->caps))
 			new_hwpipe = true;
 
-		if (plane_enabled(old_state)) {
-			bool full_modeset = false;
-			if (state->fb->pixel_format != old_state->fb->pixel_format) {
-				DBG("%s: pixel_format change!", plane->name);
-				full_modeset = true;
-			}
-			if (state->src_w != old_state->src_w) {
-				DBG("%s: src_w change!", plane->name);
-				full_modeset = true;
-			}
-			if (full_modeset) {
-				/* cannot change SMP block allocation during
-				 * scanout:
-				 */
-				if (get_kms(plane)->smp)
-					new_hwpipe = true;
-			}
+		if (mdp5_kms->smp) {
+			const struct mdp_format *format =
+				to_mdp_format(msm_framebuffer_format(state->fb));
+
+			blkcfg = mdp5_smp_calculate(mdp5_kms->smp, format,
+					state->src_w >> 16, false);
+
+			if (mdp5_state->hwpipe && (mdp5_state->hwpipe->blkcfg != blkcfg))
+				new_hwpipe = true;
 		}
 
 		/* (re)assign hwpipe if needed, otherwise keep old one: */
@@ -349,8 +343,8 @@ static int mdp5_plane_atomic_check(struct drm_plane *plane,
 			 * it available for other planes?
 			 */
 			struct mdp5_hw_pipe *hwpipe = mdp5_state->hwpipe;
-			mdp5_state->hwpipe =
-				mdp5_pipe_assign(state->state, plane, caps);
+			mdp5_state->hwpipe = mdp5_pipe_assign(state->state,
+					plane, caps, blkcfg);
 			if (IS_ERR(mdp5_state->hwpipe)) {
 				DBG("%s: failed to assign hwpipe!", plane->name);
 				return PTR_ERR(mdp5_state->hwpipe);
@@ -714,23 +708,6 @@ static int mdp5_plane_mode_set(struct drm_plane *plane,
 			fb->base.id, src_x, src_y, src_w, src_h,
 			crtc->base.id, crtc_x, crtc_y, crtc_w, crtc_h);
 
-	/* Request some memory from the SMP: */
-	if (mdp5_kms->smp) {
-		ret = mdp5_smp_request(mdp5_kms->smp, pipe,
-				format, src_w, false);
-		if (ret)
-			return ret;
-	}
-
-	/*
-	 * Currently we update the hw for allocations/requests immediately,
-	 * but once atomic modeset/pageflip is in place, the allocation
-	 * would move into atomic->check_plane_state(), while updating the
-	 * hw would remain here:
-	 */
-	if (mdp5_kms->smp)
-		mdp5_smp_configure(mdp5_kms->smp, pipe);
-
 	ret = calc_scalex_steps(plane, pix_format, src_w, crtc_w, phasex_step);
 	if (ret)
 		return ret;
@@ -868,21 +845,8 @@ uint32_t mdp5_plane_get_flush(struct drm_plane *plane)
 void mdp5_plane_complete_commit(struct drm_plane *plane,
 	struct drm_plane_state *state)
 {
-	struct mdp5_kms *mdp5_kms = get_kms(plane);
 	struct mdp5_plane_state *pstate = to_mdp5_plane_state(plane->state);
 
-	if (mdp5_kms->smp && pstate->hwpipe) {
-		enum mdp5_pipe pipe = pstate->hwpipe->pipe;
-
-		if (plane_enabled(plane->state)) {
-			DBG("%s: complete flip", plane->name);
-			mdp5_smp_commit(mdp5_kms->smp, pipe);
-		} else {
-			DBG("%s: free SMP", plane->name);
-			mdp5_smp_release(mdp5_kms->smp, pipe);
-		}
-	}
-
 	pstate->pending = false;
 }
 
diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_smp.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_smp.c
index 27d7b55..ef1120a 100644
--- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_smp.c
+++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_smp.c
@@ -21,72 +21,6 @@
 #include "mdp5_smp.h"
 
 
-/* SMP - Shared Memory Pool
- *
- * These are shared between all the clients, where each plane in a
- * scanout buffer is a SMP client.  Ie. scanout of 3 plane I420 on
- * pipe VIG0 => 3 clients: VIG0_Y, VIG0_CB, VIG0_CR.
- *
- * Based on the size of the attached scanout buffer, a certain # of
- * blocks must be allocated to that client out of the shared pool.
- *
- * In some hw, some blocks are statically allocated for certain pipes
- * and CANNOT be re-allocated (eg: MMB0 and MMB1 both tied to RGB0).
- *
- * For each block that can be dynamically allocated, it can be either
- *     free:
- *     The block is free.
- *
- *     pending:
- *     The block is allocated to some client and not free.
- *
- *     configured:
- *     The block is allocated to some client, and assigned to that
- *     client in MDP5_SMP_ALLOC registers.
- *
- *     inuse:
- *     The block is being actively used by a client.
- *
- * The updates happen in the following steps:
- *
- *  1) mdp5_smp_request():
- *     When plane scanout is setup, calculate required number of
- *     blocks needed per client, and request. Blocks neither inuse nor
- *     configured nor pending by any other client are added to client's
- *     pending set.
- *     For shrinking, blocks in pending but not in configured can be freed
- *     directly, but those already in configured will be freed later by
- *     mdp5_smp_commit.
- *
- *  2) mdp5_smp_configure():
- *     As hw is programmed, before FLUSH, MDP5_SMP_ALLOC registers
- *     are configured for the union(pending, inuse)
- *     Current pending is copied to configured.
- *     It is assumed that mdp5_smp_request and mdp5_smp_configure not run
- *     concurrently for the same pipe.
- *
- *  3) mdp5_smp_commit():
- *     After next vblank, copy configured -> inuse.  Optionally update
- *     MDP5_SMP_ALLOC registers if there are newly unused blocks
- *
- *  4) mdp5_smp_release():
- *     Must be called after the pipe is disabled and no longer uses any SMB
- *
- * On the next vblank after changes have been committed to hw, the
- * client's pending blocks become it's in-use blocks (and no-longer
- * in-use blocks become available to other clients).
- *
- * btw, hurray for confusing overloaded acronyms!  :-/
- *
- * NOTE: for atomic modeset/pageflip NONBLOCK operations, step #1
- * should happen at (or before)? atomic->check().  And we'd need
- * an API to discard previous requests if update is aborted or
- * (test-only).
- *
- * TODO would perhaps be nice to have debugfs to dump out kernel
- * inuse and pending state of all clients..
- */
-
 struct mdp5_smp {
 	struct drm_device *dev;
 
@@ -94,16 +28,8 @@ struct mdp5_smp {
 
 	int blk_cnt;
 	int blk_size;
-
-	spinlock_t state_lock;
-	mdp5_smp_state_t state; /* to track smp allocation amongst pipes: */
-
-	struct mdp5_client_smp_state client_state[MAX_CLIENTS];
 };
 
-static void update_smp_state(struct mdp5_smp *smp,
-		u32 cid, mdp5_smp_state_t *assigned);
-
 static inline
 struct mdp5_kms *get_kms(struct mdp5_smp *smp)
 {
@@ -134,57 +60,38 @@ static inline u32 pipe2client(enum mdp5_pipe pipe, int plane)
 	return mdp5_cfg->smp.clients[pipe] + plane;
 }
 
-/* step #1: update # of blocks pending for the client: */
+/* allocate blocks for the specified request: */
 static int smp_request_block(struct mdp5_smp *smp,
+		struct mdp5_smp_state *state,
 		u32 cid, int nblks)
 {
-	struct mdp5_kms *mdp5_kms = get_kms(smp);
-	struct mdp5_client_smp_state *ps = &smp->client_state[cid];
-	int i, ret, avail, cur_nblks, cnt = smp->blk_cnt;
+	void *cs = state->client_state[cid];
+	int i, avail, cnt = smp->blk_cnt;
 	uint8_t reserved;
-	unsigned long flags;
 
-	reserved = smp->reserved[cid];
+	/* we shouldn't be requesting blocks for an in-use client: */
+	WARN_ON(bitmap_weight(cs, cnt) > 0);
 
-	spin_lock_irqsave(&smp->state_lock, flags);
+	reserved = smp->reserved[cid];
 
 	if (reserved) {
 		nblks = max(0, nblks - reserved);
 		DBG("%d MMBs allocated (%d reserved)", nblks, reserved);
 	}
 
-	avail = cnt - bitmap_weight(smp->state, cnt);
+	avail = cnt - bitmap_weight(state->state, cnt);
 	if (nblks > avail) {
-		dev_err(mdp5_kms->dev->dev, "out of blks (req=%d > avail=%d)\n",
+		dev_err(smp->dev->dev, "out of blks (req=%d > avail=%d)\n",
 				nblks, avail);
-		ret = -ENOSPC;
-		goto fail;
+		return -ENOSPC;
 	}
 
-	cur_nblks = bitmap_weight(ps->pending, cnt);
-	if (nblks > cur_nblks) {
-		/* grow the existing pending reservation: */
-		for (i = cur_nblks; i < nblks; i++) {
-			int blk = find_first_zero_bit(smp->state, cnt);
-			set_bit(blk, ps->pending);
-			set_bit(blk, smp->state);
-		}
-	} else {
-		/* shrink the existing pending reservation: */
-		for (i = cur_nblks; i > nblks; i--) {
-			int blk = find_first_bit(ps->pending, cnt);
-			clear_bit(blk, ps->pending);
-
-			/* clear in global smp_state if not in configured
-			 * otherwise until _commit()
-			 */
-			if (!test_bit(blk, ps->configured))
-				clear_bit(blk, smp->state);
-		}
+	for (i = 0; i < nblks; i++) {
+		int blk = find_first_zero_bit(state->state, cnt);
+		set_bit(blk, cs);
+		set_bit(blk, state->state);
 	}
 
-fail:
-	spin_unlock_irqrestore(&smp->state_lock, flags);
 	return 0;
 }
 
@@ -209,14 +116,15 @@ static void set_fifo_thresholds(struct mdp5_smp *smp,
  * decimated width.  Ie. SMP buffering sits downstream of decimation (which
  * presumably happens during the dma from scanout buffer).
  */
-int mdp5_smp_request(struct mdp5_smp *smp, enum mdp5_pipe pipe,
-		const struct mdp_format *format, u32 width, bool hdecim)
+uint32_t mdp5_smp_calculate(struct mdp5_smp *smp,
+		const struct mdp_format *format,
+		u32 width, bool hdecim)
 {
 	struct mdp5_kms *mdp5_kms = get_kms(smp);
-	struct drm_device *dev = mdp5_kms->dev;
 	int rev = mdp5_cfg_get_hw_rev(mdp5_kms->cfg);
-	int i, hsub, nplanes, nlines, nblks, ret;
+	int i, hsub, nplanes, nlines;
 	u32 fmt = format->base.pixel_format;
+	uint32_t blkcfg = 0;
 
 	nplanes = drm_format_num_planes(fmt);
 	hsub = drm_format_horz_chroma_subsampling(fmt);
@@ -239,7 +147,7 @@ int mdp5_smp_request(struct mdp5_smp *smp, enum mdp5_pipe pipe,
 			hsub = 1;
 	}
 
-	for (i = 0, nblks = 0; i < nplanes; i++) {
+	for (i = 0; i < nplanes; i++) {
 		int n, fetch_stride, cpp;
 
 		cpp = drm_format_plane_cpp(fmt, i);
@@ -251,60 +159,72 @@ int mdp5_smp_request(struct mdp5_smp *smp, enum mdp5_pipe pipe,
 		if (rev == 0)
 			n = roundup_pow_of_two(n);
 
+		blkcfg |= (n << (8 * i));
+	}
+
+	return blkcfg;
+}
+
+int mdp5_smp_assign(struct mdp5_smp *smp, struct mdp5_smp_state *state,
+		enum mdp5_pipe pipe, uint32_t blkcfg)
+{
+	struct mdp5_kms *mdp5_kms = get_kms(smp);
+	struct drm_device *dev = mdp5_kms->dev;
+	int i, ret;
+
+	for (i = 0; i < pipe2nclients(pipe); i++) {
+		u32 cid = pipe2client(pipe, i);
+		int n = blkcfg & 0xff;
+
+		if (!n)
+			continue;
+
 		DBG("%s[%d]: request %d SMP blocks", pipe2name(pipe), i, n);
-		ret = smp_request_block(smp, pipe2client(pipe, i), n);
+		ret = smp_request_block(smp, state, cid, n);
 		if (ret) {
 			dev_err(dev->dev, "Cannot allocate %d SMP blocks: %d\n",
 					n, ret);
 			return ret;
 		}
 
-		nblks += n;
+		blkcfg >>= 8;
 	}
 
-	set_fifo_thresholds(smp, pipe, nblks);
+	state->assigned |= (1 << pipe);
 
 	return 0;
 }
 
 /* Release SMP blocks for all clients of the pipe */
-void mdp5_smp_release(struct mdp5_smp *smp, enum mdp5_pipe pipe)
+void mdp5_smp_release(struct mdp5_smp *smp, struct mdp5_smp_state *state,
+		enum mdp5_pipe pipe)
 {
 	int i;
-	unsigned long flags;
 	int cnt = smp->blk_cnt;
 
 	for (i = 0; i < pipe2nclients(pipe); i++) {
-		mdp5_smp_state_t assigned;
 		u32 cid = pipe2client(pipe, i);
-		struct mdp5_client_smp_state *ps = &smp->client_state[cid];
-
-		spin_lock_irqsave(&smp->state_lock, flags);
-
-		/* clear hw assignment */
-		bitmap_or(assigned, ps->inuse, ps->configured, cnt);
-		update_smp_state(smp, CID_UNUSED, &assigned);
+		void *cs = state->client_state[cid];
 
-		/* free to global pool */
-		bitmap_andnot(smp->state, smp->state, ps->pending, cnt);
-		bitmap_andnot(smp->state, smp->state, assigned, cnt);
+		/* update global state: */
+		bitmap_andnot(state->state, state->state, cs, cnt);
 
-		/* clear client's infor */
-		bitmap_zero(ps->pending, cnt);
-		bitmap_zero(ps->configured, cnt);
-		bitmap_zero(ps->inuse, cnt);
-
-		spin_unlock_irqrestore(&smp->state_lock, flags);
+		/* clear client's state */
+		bitmap_zero(cs, cnt);
 	}
 
-	set_fifo_thresholds(smp, pipe, 0);
+	state->released |= (1 << pipe);
 }
 
-static void update_smp_state(struct mdp5_smp *smp,
+/* NOTE: SMP_ALLOC_* regs are *not* double buffered, so release has to
+ * happen after scanout completes.
+ */
+static unsigned update_smp_state(struct mdp5_smp *smp,
 		u32 cid, mdp5_smp_state_t *assigned)
 {
 	struct mdp5_kms *mdp5_kms = get_kms(smp);
 	int cnt = smp->blk_cnt;
+	unsigned nblks = 0;
 	u32 blk, val;
 
 	for_each_set_bit(blk, *assigned, cnt) {
@@ -330,62 +250,46 @@ static void update_smp_state(struct mdp5_smp *smp,
 
 		mdp5_write(mdp5_kms, REG_MDP5_SMP_ALLOC_W_REG(idx), val);
 		mdp5_write(mdp5_kms, REG_MDP5_SMP_ALLOC_R_REG(idx), val);
+
+		nblks++;
 	}
+
+	return nblks;
 }
 
-/* step #2: configure hw for union(pending, inuse): */
-void mdp5_smp_configure(struct mdp5_smp *smp, enum mdp5_pipe pipe)
+void mdp5_smp_prepare_commit(struct mdp5_smp *smp, struct mdp5_smp_state *state)
 {
-	int cnt = smp->blk_cnt;
-	mdp5_smp_state_t assigned;
-	int i;
-
-	for (i = 0; i < pipe2nclients(pipe); i++) {
-		u32 cid = pipe2client(pipe, i);
-		struct mdp5_client_smp_state *ps = &smp->client_state[cid];
+	enum mdp5_pipe pipe;
 
-		/*
-		 * if vblank has not happened since last smp_configure
-		 * skip the configure for now
-		 */
-		if (!bitmap_equal(ps->inuse, ps->configured, cnt))
-			continue;
+	for_each_set_bit(pipe, &state->assigned, sizeof(state->assigned) * 8) {
+		unsigned i, nblks = 0;
 
-		bitmap_copy(ps->configured, ps->pending, cnt);
-		bitmap_or(assigned, ps->inuse, ps->configured, cnt);
-		update_smp_state(smp, cid, &assigned);
-	}
-}
+		for (i = 0; i < pipe2nclients(pipe); i++) {
+			u32 cid = pipe2client(pipe, i);
+			void *cs = state->client_state[cid];
 
-/* step #3: after vblank, copy configured -> inuse: */
-void mdp5_smp_commit(struct mdp5_smp *smp, enum mdp5_pipe pipe)
-{
-	int cnt = smp->blk_cnt;
-	mdp5_smp_state_t released;
-	int i;
+			nblks += update_smp_state(smp, cid, cs);
 
-	for (i = 0; i < pipe2nclients(pipe); i++) {
-		u32 cid = pipe2client(pipe, i);
-		struct mdp5_client_smp_state *ps = &smp->client_state[cid];
+			DBG("assign %s:%u, %u blks",
+				pipe2name(pipe), i, nblks);
+		}
 
-		/*
-		 * Figure out if there are any blocks we where previously
-		 * using, which can be released and made available to other
-		 * clients:
-		 */
-		if (bitmap_andnot(released, ps->inuse, ps->configured, cnt)) {
-			unsigned long flags;
+		set_fifo_thresholds(smp, pipe, nblks);
+	}
 
-			spin_lock_irqsave(&smp->state_lock, flags);
-			/* clear released blocks: */
-			bitmap_andnot(smp->state, smp->state, released, cnt);
-			spin_unlock_irqrestore(&smp->state_lock, flags);
+	state->assigned = 0;
+}
 
-			update_smp_state(smp, CID_UNUSED, &released);
-		}
+void mdp5_smp_complete_commit(struct mdp5_smp *smp, struct mdp5_smp_state *state)
+{
+	enum mdp5_pipe pipe;
 
-		bitmap_copy(ps->inuse, ps->configured, cnt);
+	for_each_set_bit(pipe, &state->released, sizeof(state->released) * 8) {
+		DBG("release %s", pipe2name(pipe));
+		set_fifo_thresholds(smp, pipe, 0);
 	}
+
+	state->released = 0;
 }
 
 void mdp5_smp_destroy(struct mdp5_smp *smp)
@@ -393,8 +297,9 @@ void mdp5_smp_destroy(struct mdp5_smp *smp)
 	kfree(smp);
 }
 
-struct mdp5_smp *mdp5_smp_init(struct drm_device *dev, const struct mdp5_smp_block *cfg)
+struct mdp5_smp *mdp5_smp_init(struct mdp5_kms *mdp5_kms, const struct mdp5_smp_block *cfg)
 {
+	struct mdp5_smp_state *state = &mdp5_kms->state->smp;
 	struct mdp5_smp *smp = NULL;
 	int ret;
 
@@ -404,14 +309,13 @@ struct mdp5_smp *mdp5_smp_init(struct drm_device *dev, const struct mdp5_smp_blo
 		goto fail;
 	}
 
-	smp->dev = dev;
+	smp->dev = mdp5_kms->dev;
 	smp->blk_cnt = cfg->mmb_count;
 	smp->blk_size = cfg->mmb_size;
 
 	/* statically tied MMBs cannot be re-allocated: */
-	bitmap_copy(smp->state, cfg->reserved_state, smp->blk_cnt);
+	bitmap_copy(state->state, cfg->reserved_state, smp->blk_cnt);
 	memcpy(smp->reserved, cfg->reserved, sizeof(smp->reserved));
-	spin_lock_init(&smp->state_lock);
 
 	return smp;
 fail:
diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_smp.h b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_smp.h
index 20b87e8..10bdd9f 100644
--- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_smp.h
+++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_smp.h
@@ -21,10 +21,49 @@
 
 #include "msm_drv.h"
 
-struct mdp5_client_smp_state {
-	mdp5_smp_state_t inuse;
-	mdp5_smp_state_t configured;
-	mdp5_smp_state_t pending;
+/*
+ * SMP - Shared Memory Pool:
+ *
+ * SMP blocks are shared between all the clients, where each plane in
+ * a scanout buffer is a SMP client.  Ie. scanout of 3 plane I420 on
+ * pipe VIG0 => 3 clients: VIG0_Y, VIG0_CB, VIG0_CR.
+ *
+ * Based on the size of the attached scanout buffer, a certain # of
+ * blocks must be allocated to that client out of the shared pool.
+ *
+ * In some hw, some blocks are statically allocated for certain pipes
+ * and CANNOT be re-allocated (eg: MMB0 and MMB1 both tied to RGB0).
+ *
+ *
+ * Atomic SMP State:
+ *
+ * On atomic updates that modify SMP configuration, the state is cloned
+ * (copied) and modified.  For test-only, or in cases where atomic
+ * update fails (or if we hit ww_mutex deadlock/backoff condition) the
+ * new state is simply thrown away.
+ *
+ * Because the SMP registers are not double buffered, updates are a
+ * two step process:
+ *
+ * 1) in _prepare_commit() we configure things (via read-modify-write)
+ *    for the newly assigned pipes, so we don't take away blocks
+ *    assigned to pipes that are still scanning out
+ * 2) in _complete_commit(), after vblank/etc, we clear things for the
+ *    released clients, since at that point old pipes are no longer
+ *    scanning out.
+ */
+struct mdp5_smp_state {
+	/* global state of what blocks are in use: */
+	mdp5_smp_state_t state;
+
+	/* per client state of what blocks they are using: */
+	mdp5_smp_state_t client_state[MAX_CLIENTS];
+
+	/* assigned pipes (hw updated at _prepare_commit()): */
+	unsigned long assigned;
+
+	/* released pipes (hw updated at _complete_commit()): */
+	unsigned long released;
 };
 
 struct mdp5_kms;
@@ -36,13 +75,20 @@ struct mdp5_smp;
  * which is then used to call the other mdp5_smp_*(handler, ...) functions.
  */
 
-struct mdp5_smp *mdp5_smp_init(struct drm_device *dev, const struct mdp5_smp_block *cfg);
+struct mdp5_smp *mdp5_smp_init(struct mdp5_kms *mdp5_kms,
+		const struct mdp5_smp_block *cfg);
 void  mdp5_smp_destroy(struct mdp5_smp *smp);
 
-int  mdp5_smp_request(struct mdp5_smp *smp, enum mdp5_pipe pipe,
-		const struct mdp_format *format, u32 width, bool hdecim);
-void mdp5_smp_configure(struct mdp5_smp *smp, enum mdp5_pipe pipe);
-void mdp5_smp_commit(struct mdp5_smp *smp, enum mdp5_pipe pipe);
-void mdp5_smp_release(struct mdp5_smp *smp, enum mdp5_pipe pipe);
+uint32_t mdp5_smp_calculate(struct mdp5_smp *smp,
+		const struct mdp_format *format,
+		u32 width, bool hdecim);
+
+int mdp5_smp_assign(struct mdp5_smp *smp, struct mdp5_smp_state *state,
+		enum mdp5_pipe pipe, uint32_t blkcfg);
+void mdp5_smp_release(struct mdp5_smp *smp, struct mdp5_smp_state *state,
+		enum mdp5_pipe pipe);
+
+void mdp5_smp_prepare_commit(struct mdp5_smp *smp, struct mdp5_smp_state *state);
+void mdp5_smp_complete_commit(struct mdp5_smp *smp, struct mdp5_smp_state *state);
 
 #endif /* __MDP5_SMP_H__ */
-- 
2.7.4

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

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH 3/5] drm/msm/mdp5: add skeletal mdp5_state
       [not found]     ` <1478363161-29293-4-git-send-email-robdclark-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
@ 2016-11-07 10:29       ` Archit Taneja
  2016-11-07 11:31         ` Rob Clark
  0 siblings, 1 reply; 12+ messages in thread
From: Archit Taneja @ 2016-11-07 10:29 UTC (permalink / raw)
  To: Rob Clark, dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
  Cc: Sean Paul, linux-arm-msm-u79uwXL29TY76Z2rM5mHXA,
	freedreno-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Daniel Vetter



On 11/05/2016 09:55 PM, Rob Clark wrote:
> Add basic state duplication/apply mechanism.  Following commits will
> move actual global hw state into this.
>
> The state_lock allows multiple concurrent updates to proceed as long as
> they don't both try to alter global state.  The ww_mutex mechanism will
> trigger backoff in case of deadlock between multiple threads trying to
> update state.
>
> Signed-off-by: Rob Clark <robdclark@gmail.com>
> ---
>  drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c | 43 +++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h | 22 +++++++++++++++++
>  2 files changed, 65 insertions(+)
>
> diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c
> index d3d45ed..ca6dfeb 100644
> --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c
> +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c
> @@ -72,6 +72,39 @@ static int mdp5_hw_init(struct msm_kms *kms)
>  	return 0;
>  }
>
> +struct mdp5_state *mdp5_get_state(struct drm_atomic_state *s)
> +{
> +	struct msm_drm_private *priv = s->dev->dev_private;
> +	struct mdp5_kms *mdp5_kms = to_mdp5_kms(to_mdp_kms(priv->kms));
> +	struct msm_kms_state *state = to_kms_state(s);
> +	struct mdp5_state *new_state;
> +	int ret;
> +
> +	if (state->state)
> +		return state->state;
> +
> +	ret = drm_modeset_lock(&mdp5_kms->state_lock, s->acquire_ctx);
> +	if (ret)
> +		return ERR_PTR(ret);
> +
> +	new_state = kmalloc(sizeof(*mdp5_kms->state), GFP_KERNEL);
> +	if (!new_state)
> +		return ERR_PTR(-ENOMEM);
> +
> +	/* Copy state: */
> +	/* TODO */
> +
> +	state->state = new_state;
> +
> +	return new_state;
> +}
> +
> +static void mdp5_swap_state(struct msm_kms *kms, struct drm_atomic_state *state)
> +{
> +	struct mdp5_kms *mdp5_kms = to_mdp5_kms(to_mdp_kms(kms));
> +	swap(to_kms_state(state)->state, mdp5_kms->state);
> +}
> +
>  static void mdp5_prepare_commit(struct msm_kms *kms, struct drm_atomic_state *state)
>  {
>  	struct mdp5_kms *mdp5_kms = to_mdp5_kms(to_mdp_kms(kms));
> @@ -140,6 +173,7 @@ static const struct mdp_kms_funcs kms_funcs = {
>  		.irq             = mdp5_irq,
>  		.enable_vblank   = mdp5_enable_vblank,
>  		.disable_vblank  = mdp5_disable_vblank,
> +		.swap_state      = mdp5_swap_state,
>  		.prepare_commit  = mdp5_prepare_commit,
>  		.complete_commit = mdp5_complete_commit,
>  		.wait_for_crtc_commit_done = mdp5_wait_for_crtc_commit_done,
> @@ -645,6 +679,8 @@ static void mdp5_destroy(struct platform_device *pdev)
>
>  	if (mdp5_kms->rpm_enabled)
>  		pm_runtime_disable(&pdev->dev);
> +
> +	kfree(mdp5_kms->state);
>  }
>
>  static int construct_pipes(struct mdp5_kms *mdp5_kms, int cnt,
> @@ -729,6 +765,13 @@ static int mdp5_init(struct platform_device *pdev, struct drm_device *dev)
>  	mdp5_kms->dev = dev;
>  	mdp5_kms->pdev = pdev;
>
> +	drm_modeset_lock_init(&mdp5_kms->state_lock);
> +	mdp5_kms->state = kzalloc(sizeof(*mdp5_kms->state), GFP_KERNEL);
> +	if (!mdp5_kms->state) {
> +		ret = -ENOMEM;
> +		goto fail;
> +	}
> +

This would probably be better in mdp5_kms_init() since it's intializing kms stuff, and
not hw resources. Otherwise:

Reviewed-by: Archit Taneja <architt@codeaurora.org>


>  	mdp5_kms->mmio = msm_ioremap(pdev, "mdp_phys", "MDP5");
>  	if (IS_ERR(mdp5_kms->mmio)) {
>  		ret = PTR_ERR(mdp5_kms->mmio);
> diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h
> index 88120c5..52914ec 100644
> --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h
> +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h
> @@ -27,6 +27,8 @@
>  #include "mdp5_pipe.h"
>  #include "mdp5_smp.h"
>
> +struct mdp5_state;
> +
>  struct mdp5_kms {
>  	struct mdp_kms base;
>
> @@ -40,6 +42,11 @@ struct mdp5_kms {
>  	struct mdp5_cfg_handler *cfg;
>  	uint32_t caps;	/* MDP capabilities (MDP_CAP_XXX bits) */
>
> +	/**
> +	 * Global atomic state.  Do not access directly, use mdp5_get_state()
> +	 */
> +	struct mdp5_state *state;
> +	struct drm_modeset_lock state_lock;
>
>  	/* mapper-id used to request GEM buffer mapped for scanout: */
>  	int id;
> @@ -69,6 +76,21 @@ struct mdp5_kms {
>  };
>  #define to_mdp5_kms(x) container_of(x, struct mdp5_kms, base)
>
> +/* Global atomic state for tracking resources that are shared across
> + * multiple kms objects (planes/crtcs/etc).
> + *
> + * For atomic updates which require modifying global state,
> + */
> +struct mdp5_state {
> +	uint32_t dummy;
> +};
> +
> +struct mdp5_state *__must_check
> +mdp5_get_state(struct drm_atomic_state *s);
> +
> +/* Atomic plane state.  Subclasses the base drm_plane_state in order to
> + * track assigned hwpipe and hw specific state.
> + */
>  struct mdp5_plane_state {
>  	struct drm_plane_state base;
>
>

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 4/5] drm/msm/mdp5: dynamically assign hw pipes to planes
  2016-11-05 16:26 ` [PATCH 4/5] drm/msm/mdp5: dynamically assign hw pipes to planes Rob Clark
@ 2016-11-07 10:29   ` Archit Taneja
  0 siblings, 0 replies; 12+ messages in thread
From: Archit Taneja @ 2016-11-07 10:29 UTC (permalink / raw)
  To: Rob Clark, dri-devel; +Cc: linux-arm-msm, freedreno, Daniel Vetter, Sean Paul

Hi,

Minor comments below. LGTM otherwise.

On 11/05/2016 09:56 PM, Rob Clark wrote:
> (re)assign the hw pipes to planes based on required caps, and to handle
> situations where we could not modify an in-use plane (ie. SMP block
> reallocation).
>
> This means all planes advertise the superset of formats and properties.
> Userspace must (as always) use atomic TEST_ONLY step for atomic updates,
> as not all planes may be available for use on every frame.
>
> The mapping of hwpipe to plane is stored in mdp5_state, so that state
> updates are atomically committed in the same way that plane/etc state
> updates are managed.  This is needed because the mdp5_plane_state keeps
> a pointer to the hwpipe, and we don't want global state to become out
> of sync with the plane state if an atomic update fails, we hit deadlock/
> backoff scenario, etc.  The use of state_lock keeps multiple parallel
> updates which both re-assign hwpipes properly serialized.
>
> Signed-off-by: Rob Clark <robdclark@gmail.com>
> ---
>  drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c  |   2 +-
>  drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c   |   4 +-
>  drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h   |   7 +-
>  drivers/gpu/drm/msm/mdp/mdp5/mdp5_pipe.c  |  70 +++++++++++++
>  drivers/gpu/drm/msm/mdp/mdp5/mdp5_pipe.h  |  10 ++
>  drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c | 158 ++++++++++++++++--------------
>  6 files changed, 171 insertions(+), 80 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c
> index 6213f51..99958f7 100644
> --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c
> +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c
> @@ -407,7 +407,7 @@ static int mdp5_crtc_atomic_check(struct drm_crtc *crtc,
>  	for (i = 0; i < cnt; i++) {
>  		pstates[i].state->stage = STAGE_BASE + i + base;
>  		DBG("%s: assign pipe %s on stage=%d", crtc->name,
> -				pipe2name(mdp5_plane_pipe(pstates[i].plane)),
> +				pstates[i].plane->name,
>  				pstates[i].state->stage);
>  	}
>
> diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c
> index ca6dfeb..3542adf 100644
> --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c
> +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c
> @@ -92,7 +92,7 @@ struct mdp5_state *mdp5_get_state(struct drm_atomic_state *s)
>  		return ERR_PTR(-ENOMEM);
>
>  	/* Copy state: */
> -	/* TODO */
> +	new_state->hwpipe = mdp5_kms->state->hwpipe;
>
>  	state->state = new_state;
>
> @@ -377,7 +377,7 @@ static int modeset_init(struct mdp5_kms *mdp5_kms)
>  		struct drm_plane *plane;
>  		struct drm_crtc *crtc;
>
> -		plane = mdp5_plane_init(dev, mdp5_kms->hwpipes[i], primary);
> +		plane = mdp5_plane_init(dev, primary);
>  		if (IS_ERR(plane)) {
>  			ret = PTR_ERR(plane);
>  			dev_err(dev->dev, "failed to construct plane %d (%d)\n", i, ret);
> diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h
> index 52914ec..4475090 100644
> --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h
> +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h
> @@ -82,7 +82,7 @@ struct mdp5_kms {
>   * For atomic updates which require modifying global state,
>   */
>  struct mdp5_state {
> -	uint32_t dummy;
> +	struct mdp5_hw_pipe_state hwpipe;

I was wondering if we could rename the above as hwpipes/hwpipe_state/hwpipe_map,
because it gets a confusing since variables of the type mdp5_hw_pipe are
also named hwpipe.

>  };
>
>  struct mdp5_state *__must_check
> @@ -94,6 +94,8 @@ mdp5_get_state(struct drm_atomic_state *s);
>  struct mdp5_plane_state {
>  	struct drm_plane_state base;
>
> +	struct mdp5_hw_pipe *hwpipe;
> +
>  	/* aligned with property */
>  	uint8_t premultiplied;
>  	uint8_t zpos;
> @@ -232,8 +234,7 @@ uint32_t mdp5_plane_get_flush(struct drm_plane *plane);
>  void mdp5_plane_complete_commit(struct drm_plane *plane,
>  	struct drm_plane_state *state);
>  enum mdp5_pipe mdp5_plane_pipe(struct drm_plane *plane);
> -struct drm_plane *mdp5_plane_init(struct drm_device *dev,
> -		struct mdp5_hw_pipe *hwpipe, bool primary);
> +struct drm_plane *mdp5_plane_init(struct drm_device *dev, bool primary);
>
>  uint32_t mdp5_crtc_vblank(struct drm_crtc *crtc);
>
> diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_pipe.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_pipe.c
> index 7f3e8e50..115de7d 100644
> --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_pipe.c
> +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_pipe.c
> @@ -17,6 +17,76 @@
>
>  #include "mdp5_kms.h"
>
> +struct mdp5_hw_pipe *mdp5_pipe_assign(struct drm_atomic_state *s,
> +		struct drm_plane *plane, uint32_t caps)
> +{
> +	struct msm_drm_private *priv = s->dev->dev_private;
> +	struct mdp5_kms *mdp5_kms = to_mdp5_kms(to_mdp_kms(priv->kms));
> +	struct mdp5_state *state = mdp5_get_state(s);
> +	struct mdp5_hw_pipe_state *old_state, *new_state;
> +	struct mdp5_hw_pipe *hwpipe = NULL;
> +	int i;
> +

Can we assign "state = mdp5_get_state(s);" here instead for clarity? Since the func
does more than just getting the mdp5_state pointer?

> +	if (IS_ERR(state))
> +		return ERR_CAST(state);
> +
> +	/* grab old_state after mdp5_get_state(), since now we hold lock: */
> +	old_state = &mdp5_kms->state->hwpipe;
> +	new_state = &state->hwpipe;
> +
> +	for (i = 0; i < mdp5_kms->num_hwpipes; i++) {
> +		struct mdp5_hw_pipe *cur = mdp5_kms->hwpipes[i];
> +
> +		/* skip if already in-use.. check both new and old state,
> +		 * since we cannot immediately re-use a pipe that is
> +		 * released in the current update in some cases:
> +		 *  (1) mdp5 has SMP (non-double-buffered)
> +		 *  (2) hw pipe previously assigned to different CRTC
> +		 *      (vblanks might not be aligned)

For 1), we have non-SMP SoCs (a.k.a 8996), and for 2) we would have
the info whether the hwpipe is changing crtcs in old and new states.
I guess we could leave this as an optimization for the future.


> +		 */
> +		if (new_state->hwpipe_to_plane[cur->idx] ||
> +				old_state->hwpipe_to_plane[cur->idx])
> +			continue;
> +
> +		/* skip if doesn't support some required caps: */
> +		if (caps & ~cur->caps)
> +			continue;
> +
> +		/* possible candidate, take the one with the
> +		 * fewest unneeded caps bits set:
> +		 */
> +		if (!hwpipe || (hweight_long(cur->caps & ~caps) <
> +				hweight_long(hwpipe->caps & ~caps)))
> +			hwpipe = cur;
> +	}
> +
> +	if (!hwpipe)
> +		return ERR_PTR(-ENOMEM);
> +
> +	DBG("%s: assign to plane %s for caps %x",
> +			hwpipe->name, plane->name, caps);
> +	new_state->hwpipe_to_plane[hwpipe->idx] = plane;
> +
> +	return hwpipe;
> +}
> +
> +void mdp5_pipe_release(struct drm_atomic_state *s, struct mdp5_hw_pipe *hwpipe)
> +{
> +	struct mdp5_state *state = mdp5_get_state(s);
> +	struct mdp5_hw_pipe_state *new_state = &state->hwpipe;
> +
> +	if (!hwpipe)
> +		return;
> +
> +	if (WARN_ON(!new_state->hwpipe_to_plane[hwpipe->idx]))
> +		return;
> +
> +	DBG("%s: release from plane %s", hwpipe->name,
> +		new_state->hwpipe_to_plane[hwpipe->idx]->name);
> +
> +	new_state->hwpipe_to_plane[hwpipe->idx] = NULL;
> +}
> +
>  void mdp5_pipe_destroy(struct mdp5_hw_pipe *hwpipe)
>  {
>  	kfree(hwpipe);
> diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_pipe.h b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_pipe.h
> index dcfa0e0..bac5900 100644
> --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_pipe.h
> +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_pipe.h
> @@ -32,6 +32,16 @@ struct mdp5_hw_pipe {
>  	uint32_t flush_mask;      /* used to commit pipe registers */
>  };
>
> +/* global atomic state of assignment between pipes and planes: */
> +struct mdp5_hw_pipe_state {
> +	struct drm_plane *hwpipe_to_plane[16];

We could use SSPP_MAX here instead of 16. We would need to move its
definition from mdp5_crtc.c to mdp5_kms.h, though.

> +};
> +
> +struct mdp5_hw_pipe *__must_check
> +mdp5_pipe_assign(struct drm_atomic_state *s, struct drm_plane *plane,
> +		uint32_t caps);
> +void mdp5_pipe_release(struct drm_atomic_state *s, struct mdp5_hw_pipe *hwpipe);
> +
>  struct mdp5_hw_pipe *mdp5_pipe_init(enum mdp5_pipe pipe,
>  		uint32_t reg_offset, uint32_t caps);
>  void mdp5_pipe_destroy(struct mdp5_hw_pipe *hwpipe);
> diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
> index e4ecfeb..64e97ef 100644
> --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
> +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
> @@ -22,8 +22,6 @@
>  struct mdp5_plane {
>  	struct drm_plane base;
>
> -	struct mdp5_hw_pipe *hwpipe;
> -
>  	uint32_t nformats;
>  	uint32_t formats[32];
>  };
> @@ -63,12 +61,6 @@ static void mdp5_plane_destroy(struct drm_plane *plane)
>  static void mdp5_plane_install_rotation_property(struct drm_device *dev,
>  		struct drm_plane *plane)
>  {
> -	struct mdp5_plane *mdp5_plane = to_mdp5_plane(plane);
> -
> -	if (!(mdp5_plane->hwpipe->caps & MDP_PIPE_CAP_HFLIP) &&
> -		!(mdp5_plane->hwpipe->caps & MDP_PIPE_CAP_VFLIP))
> -		return;
> -
>  	drm_plane_create_rotation_property(plane,
>  					   DRM_ROTATE_0,
>  					   DRM_ROTATE_0 |
> @@ -184,6 +176,8 @@ mdp5_plane_atomic_print_state(struct drm_printer *p,
>  {
>  	struct mdp5_plane_state *pstate = to_mdp5_plane_state(state);
>
> +	drm_printf(p, "\thwpipe=%s\n", pstate->hwpipe ?
> +			pstate->hwpipe->name : "(null)");
>  	drm_printf(p, "\tpremultiplied=%u\n", pstate->premultiplied);
>  	drm_printf(p, "\tzpos=%u\n", pstate->zpos);
>  	drm_printf(p, "\talpha=%u\n", pstate->alpha);
> @@ -237,10 +231,12 @@ mdp5_plane_duplicate_state(struct drm_plane *plane)
>  static void mdp5_plane_destroy_state(struct drm_plane *plane,
>  		struct drm_plane_state *state)
>  {
> +	struct mdp5_plane_state *pstate = to_mdp5_plane_state(state);
> +
>  	if (state->fb)
>  		drm_framebuffer_unreference(state->fb);
>
> -	kfree(to_mdp5_plane_state(state));
> +	kfree(pstate);
>  }
>
>  static const struct drm_plane_funcs mdp5_plane_funcs = {
> @@ -285,70 +281,81 @@ static void mdp5_plane_cleanup_fb(struct drm_plane *plane,
>  static int mdp5_plane_atomic_check(struct drm_plane *plane,
>  		struct drm_plane_state *state)
>  {
> -	struct mdp5_plane *mdp5_plane = to_mdp5_plane(plane);
> +	struct mdp5_plane_state *mdp5_state = to_mdp5_plane_state(state);
>  	struct drm_plane_state *old_state = plane->state;
> -	const struct mdp_format *format;
> -	bool vflip, hflip;
> +	bool new_hwpipe = false;
> +	uint32_t caps = 0;
>
>  	DBG("%s: check (%d -> %d)", plane->name,
>  			plane_enabled(old_state), plane_enabled(state));
>
> +	/* We don't allow faster-than-vblank updates.. if we did add this
> +	 * some day, we would need to disallow in cases where hwpipe
> +	 * changes
> +	 */
> +	if (WARN_ON(to_mdp5_plane_state(old_state)->pending))
> +		return -EBUSY;
> +
>  	if (plane_enabled(state)) {
>  		unsigned int rotation;
> +		const struct mdp_format *format;
>
>  		format = to_mdp_format(msm_framebuffer_format(state->fb));
> -		if (MDP_FORMAT_IS_YUV(format) &&
> -			!pipe_supports_yuv(mdp5_plane->hwpipe->caps)) {
> -			DBG("Pipe doesn't support YUV\n");
> -
> -			return -EINVAL;
> -		}
> -
> -		if (!(mdp5_plane->hwpipe->caps & MDP_PIPE_CAP_SCALE) &&
> -			(((state->src_w >> 16) != state->crtc_w) ||
> -			((state->src_h >> 16) != state->crtc_h))) {
> -			DBG("Pipe doesn't support scaling (%dx%d -> %dx%d)\n",
> -				state->src_w >> 16, state->src_h >> 16,
> -				state->crtc_w, state->crtc_h);
> +		if (MDP_FORMAT_IS_YUV(format))
> +			caps |= MDP_PIPE_CAP_SCALE | MDP_PIPE_CAP_CSC;
>
> -			return -EINVAL;
> -		}
> +		if (((state->src_w >> 16) != state->crtc_w) ||
> +				((state->src_h >> 16) != state->crtc_h))
> +			caps |= MDP_PIPE_CAP_SCALE;
>
>  		rotation = drm_rotation_simplify(state->rotation,
>  						 DRM_ROTATE_0 |
>  						 DRM_REFLECT_X |
>  						 DRM_REFLECT_Y);
>
> -		hflip = !!(rotation & DRM_REFLECT_X);
> -		vflip = !!(rotation & DRM_REFLECT_Y);
> -
> -		if ((vflip && !(mdp5_plane->hwpipe->caps & MDP_PIPE_CAP_VFLIP)) ||
> -			(hflip && !(mdp5_plane->hwpipe->caps & MDP_PIPE_CAP_HFLIP))) {
> -			DBG("Pipe doesn't support flip\n");
> -
> -			return -EINVAL;
> +		if (rotation & DRM_REFLECT_X)
> +			caps |= MDP_PIPE_CAP_HFLIP;
> +
> +		if (rotation & DRM_REFLECT_Y)
> +			caps |= MDP_PIPE_CAP_VFLIP;
> +
> +		/* (re)allocate hw pipe if we don't have one or caps-mismatch: */
> +		if (!mdp5_state->hwpipe || (caps & ~mdp5_state->hwpipe->caps))
> +			new_hwpipe = true;
> +
> +		if (plane_enabled(old_state)) {
> +			bool full_modeset = false;
> +			if (state->fb->pixel_format != old_state->fb->pixel_format) {
> +				DBG("%s: pixel_format change!", plane->name);
> +				full_modeset = true;
> +			}
> +			if (state->src_w != old_state->src_w) {
> +				DBG("%s: src_w change!", plane->name);
> +				full_modeset = true;
> +			}
> +			if (full_modeset) {
> +				/* cannot change SMP block allocation during
> +				 * scanout:
> +				 */
> +				if (get_kms(plane)->smp)
> +					new_hwpipe = true;
> +			}
>  		}
> -	}
>
> -	if (plane_enabled(state) && plane_enabled(old_state)) {
> -		/* we cannot change SMP block configuration during scanout: */
> -		bool full_modeset = false;
> -		if (state->fb->pixel_format != old_state->fb->pixel_format) {
> -			DBG("%s: pixel_format change!", plane->name);
> -			full_modeset = true;
> -		}
> -		if (state->src_w != old_state->src_w) {
> -			DBG("%s: src_w change!", plane->name);
> -			full_modeset = true;
> -		}
> -		if (to_mdp5_plane_state(old_state)->pending) {
> -			DBG("%s: still pending!", plane->name);
> -			full_modeset = true;
> -		}
> -		if (full_modeset) {
> -			struct drm_crtc_state *crtc_state =
> -					drm_atomic_get_crtc_state(state->state, state->crtc);
> -			crtc_state->mode_changed = true;
> +		/* (re)assign hwpipe if needed, otherwise keep old one: */
> +		if (new_hwpipe) {
> +			/* TODO maybe we want to re-assign hwpipe sometimes
> +			 * in cases when we no-longer need some caps to make
> +			 * it available for other planes?
> +			 */
> +			struct mdp5_hw_pipe *hwpipe = mdp5_state->hwpipe;

Maybe hwpipe above could be curr_hwpipe or old_hwpipe?

> +			mdp5_state->hwpipe =
> +				mdp5_pipe_assign(state->state, plane, caps);
> +			if (IS_ERR(mdp5_state->hwpipe)) {
> +				DBG("%s: failed to assign hwpipe!", plane->name);
> +				return PTR_ERR(mdp5_state->hwpipe);
> +			}
> +			mdp5_pipe_release(state->state, hwpipe);
>  		}
>  	}

Thanks,
Archit

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/5] drm/msm/mdp5: introduce mdp5_hw_pipe
  2016-11-05 16:25 ` [PATCH 1/5] drm/msm/mdp5: introduce mdp5_hw_pipe Rob Clark
@ 2016-11-07 10:38   ` Archit Taneja
       [not found]     ` <1c36659b-753e-12d4-c958-79d0e8d2af88-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
  0 siblings, 1 reply; 12+ messages in thread
From: Archit Taneja @ 2016-11-07 10:38 UTC (permalink / raw)
  To: Rob Clark, dri-devel; +Cc: linux-arm-msm, freedreno



On 11/05/2016 09:55 PM, Rob Clark wrote:
> Split out the hardware pipe specifics from mdp5_plane.  To start, the hw
> pipes are statically assigned to planes, but next step is to assign the
> hw pipes during plane->atomic_check() based on requested caps (scaling,
> YUV, etc).  And then hw pipe re-assignment if required if required SMP
> blocks changes.
>
> Signed-off-by: Rob Clark <robdclark@gmail.com>
> ---
>  drivers/gpu/drm/msm/Makefile              |   1 +
>  drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c   | 126 +++++++++++++++++++-----------
>  drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h   |   7 +-
>  drivers/gpu/drm/msm/mdp/mdp5/mdp5_pipe.c  |  43 ++++++++++
>  drivers/gpu/drm/msm/mdp/mdp5/mdp5_pipe.h  |  39 +++++++++
>  drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c |  66 +++++++---------
>  6 files changed, 197 insertions(+), 85 deletions(-)
>  create mode 100644 drivers/gpu/drm/msm/mdp/mdp5/mdp5_pipe.c
>  create mode 100644 drivers/gpu/drm/msm/mdp/mdp5/mdp5_pipe.h
>
> diff --git a/drivers/gpu/drm/msm/Makefile b/drivers/gpu/drm/msm/Makefile
> index fb5be3e..90f66c4 100644
> --- a/drivers/gpu/drm/msm/Makefile
> +++ b/drivers/gpu/drm/msm/Makefile
> @@ -37,6 +37,7 @@ msm-y := \
>  	mdp/mdp5/mdp5_irq.o \
>  	mdp/mdp5/mdp5_mdss.o \
>  	mdp/mdp5/mdp5_kms.o \
> +	mdp/mdp5/mdp5_pipe.o \
>  	mdp/mdp5/mdp5_plane.o \
>  	mdp/mdp5/mdp5_smp.o \
>  	msm_atomic.o \
> diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c
> index f1288c7..d3d45ed 100644
> --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c
> +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c
> @@ -119,6 +119,10 @@ static void mdp5_kms_destroy(struct msm_kms *kms)
>  {
>  	struct mdp5_kms *mdp5_kms = to_mdp5_kms(to_mdp_kms(kms));
>  	struct msm_gem_address_space *aspace = mdp5_kms->aspace;
> +	int i;
> +
> +	for (i = 0; i < mdp5_kms->num_hwpipes; i++)
> +		mdp5_pipe_destroy(mdp5_kms->hwpipes[i]);
>
>  	if (aspace) {
>  		aspace->mmu->funcs->detach(aspace->mmu,
> @@ -323,15 +327,6 @@ static int modeset_init_intf(struct mdp5_kms *mdp5_kms, int intf_num)
>
>  static int modeset_init(struct mdp5_kms *mdp5_kms)
>  {
> -	static const enum mdp5_pipe rgb_planes[] = {
> -			SSPP_RGB0, SSPP_RGB1, SSPP_RGB2, SSPP_RGB3,
> -	};
> -	static const enum mdp5_pipe vig_planes[] = {
> -			SSPP_VIG0, SSPP_VIG1, SSPP_VIG2, SSPP_VIG3,
> -	};
> -	static const enum mdp5_pipe dma_planes[] = {
> -			SSPP_DMA0, SSPP_DMA1,
> -	};
>  	struct drm_device *dev = mdp5_kms->dev;
>  	struct msm_drm_private *priv = dev->dev_private;
>  	const struct mdp5_cfg_hw *hw_cfg;
> @@ -339,58 +334,34 @@ static int modeset_init(struct mdp5_kms *mdp5_kms)
>
>  	hw_cfg = mdp5_cfg_get_hw_config(mdp5_kms->cfg);
>
> -	/* construct CRTCs and their private planes: */
> -	for (i = 0; i < hw_cfg->pipe_rgb.count; i++) {
> +	/* Construct planes equaling the number of hw pipes, and CRTCs
> +	 * for the N layer-mixers (LM).  The first N planes become primary
> +	 * planes for the CRTCs, with the remainder as overlay planes:
> +	 */

Jfyi, we might need to change this a bit in the future. It'll be better to
get the max number of displays connected on our platform via parsing DT,
etc, and calculate CRTCs based on that, and not number of layermixers. Maybe
add a couple for writeback too. This way, we get the right number of
CRTCs, and we don't rely on #LMs, since we can have 2 per crtc
in the future.

Reviewed-by: Archit Taneja <architt@codeaurora.org>

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 3/5] drm/msm/mdp5: add skeletal mdp5_state
  2016-11-07 10:29       ` Archit Taneja
@ 2016-11-07 11:31         ` Rob Clark
  0 siblings, 0 replies; 12+ messages in thread
From: Rob Clark @ 2016-11-07 11:31 UTC (permalink / raw)
  To: Archit Taneja; +Cc: linux-arm-msm, freedreno, dri-devel

On Mon, Nov 7, 2016 at 5:29 AM, Archit Taneja <architt@codeaurora.org> wrote:
>
>
> On 11/05/2016 09:55 PM, Rob Clark wrote:
>>
>> Add basic state duplication/apply mechanism.  Following commits will
>> move actual global hw state into this.
>>
>> The state_lock allows multiple concurrent updates to proceed as long as
>> they don't both try to alter global state.  The ww_mutex mechanism will
>> trigger backoff in case of deadlock between multiple threads trying to
>> update state.
>>
>> Signed-off-by: Rob Clark <robdclark@gmail.com>
>> ---
>>  drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c | 43
>> +++++++++++++++++++++++++++++++++
>>  drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h | 22 +++++++++++++++++
>>  2 files changed, 65 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c
>> b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c
>> index d3d45ed..ca6dfeb 100644
>> --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c
>> +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c
>> @@ -72,6 +72,39 @@ static int mdp5_hw_init(struct msm_kms *kms)
>>         return 0;
>>  }
>>
>> +struct mdp5_state *mdp5_get_state(struct drm_atomic_state *s)
>> +{
>> +       struct msm_drm_private *priv = s->dev->dev_private;
>> +       struct mdp5_kms *mdp5_kms = to_mdp5_kms(to_mdp_kms(priv->kms));
>> +       struct msm_kms_state *state = to_kms_state(s);
>> +       struct mdp5_state *new_state;
>> +       int ret;
>> +
>> +       if (state->state)
>> +               return state->state;
>> +
>> +       ret = drm_modeset_lock(&mdp5_kms->state_lock, s->acquire_ctx);
>> +       if (ret)
>> +               return ERR_PTR(ret);
>> +
>> +       new_state = kmalloc(sizeof(*mdp5_kms->state), GFP_KERNEL);
>> +       if (!new_state)
>> +               return ERR_PTR(-ENOMEM);
>> +
>> +       /* Copy state: */
>> +       /* TODO */
>> +
>> +       state->state = new_state;
>> +
>> +       return new_state;
>> +}
>> +
>> +static void mdp5_swap_state(struct msm_kms *kms, struct drm_atomic_state
>> *state)
>> +{
>> +       struct mdp5_kms *mdp5_kms = to_mdp5_kms(to_mdp_kms(kms));
>> +       swap(to_kms_state(state)->state, mdp5_kms->state);
>> +}
>> +
>>  static void mdp5_prepare_commit(struct msm_kms *kms, struct
>> drm_atomic_state *state)
>>  {
>>         struct mdp5_kms *mdp5_kms = to_mdp5_kms(to_mdp_kms(kms));
>> @@ -140,6 +173,7 @@ static const struct mdp_kms_funcs kms_funcs = {
>>                 .irq             = mdp5_irq,
>>                 .enable_vblank   = mdp5_enable_vblank,
>>                 .disable_vblank  = mdp5_disable_vblank,
>> +               .swap_state      = mdp5_swap_state,
>>                 .prepare_commit  = mdp5_prepare_commit,
>>                 .complete_commit = mdp5_complete_commit,
>>                 .wait_for_crtc_commit_done =
>> mdp5_wait_for_crtc_commit_done,
>> @@ -645,6 +679,8 @@ static void mdp5_destroy(struct platform_device *pdev)
>>
>>         if (mdp5_kms->rpm_enabled)
>>                 pm_runtime_disable(&pdev->dev);
>> +
>> +       kfree(mdp5_kms->state);
>>  }
>>
>>  static int construct_pipes(struct mdp5_kms *mdp5_kms, int cnt,
>> @@ -729,6 +765,13 @@ static int mdp5_init(struct platform_device *pdev,
>> struct drm_device *dev)
>>         mdp5_kms->dev = dev;
>>         mdp5_kms->pdev = pdev;
>>
>> +       drm_modeset_lock_init(&mdp5_kms->state_lock);
>> +       mdp5_kms->state = kzalloc(sizeof(*mdp5_kms->state), GFP_KERNEL);
>> +       if (!mdp5_kms->state) {
>> +               ret = -ENOMEM;
>> +               goto fail;
>> +       }
>> +
>
>
> This would probably be better in mdp5_kms_init() since it's intializing kms
> stuff, and
> not hw resources. Otherwise:

it ends up needing to be before mdp5_smp_init() unfortunately, so
things can be setup properly for hw that has reserved blocks.

BR,
-R

> Reviewed-by: Archit Taneja <architt@codeaurora.org>
>
>
>
>>         mdp5_kms->mmio = msm_ioremap(pdev, "mdp_phys", "MDP5");
>>         if (IS_ERR(mdp5_kms->mmio)) {
>>                 ret = PTR_ERR(mdp5_kms->mmio);
>> diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h
>> b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h
>> index 88120c5..52914ec 100644
>> --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h
>> +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h
>> @@ -27,6 +27,8 @@
>>  #include "mdp5_pipe.h"
>>  #include "mdp5_smp.h"
>>
>> +struct mdp5_state;
>> +
>>  struct mdp5_kms {
>>         struct mdp_kms base;
>>
>> @@ -40,6 +42,11 @@ struct mdp5_kms {
>>         struct mdp5_cfg_handler *cfg;
>>         uint32_t caps;  /* MDP capabilities (MDP_CAP_XXX bits) */
>>
>> +       /**
>> +        * Global atomic state.  Do not access directly, use
>> mdp5_get_state()
>> +        */
>> +       struct mdp5_state *state;
>> +       struct drm_modeset_lock state_lock;
>>
>>         /* mapper-id used to request GEM buffer mapped for scanout: */
>>         int id;
>> @@ -69,6 +76,21 @@ struct mdp5_kms {
>>  };
>>  #define to_mdp5_kms(x) container_of(x, struct mdp5_kms, base)
>>
>> +/* Global atomic state for tracking resources that are shared across
>> + * multiple kms objects (planes/crtcs/etc).
>> + *
>> + * For atomic updates which require modifying global state,
>> + */
>> +struct mdp5_state {
>> +       uint32_t dummy;
>> +};
>> +
>> +struct mdp5_state *__must_check
>> +mdp5_get_state(struct drm_atomic_state *s);
>> +
>> +/* Atomic plane state.  Subclasses the base drm_plane_state in order to
>> + * track assigned hwpipe and hw specific state.
>> + */
>>  struct mdp5_plane_state {
>>         struct drm_plane_state base;
>>
>>
>
> --
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
> a Linux Foundation Collaborative Project
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/5] drm/msm/mdp5: introduce mdp5_hw_pipe
       [not found]     ` <1c36659b-753e-12d4-c958-79d0e8d2af88-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
@ 2016-11-07 14:48       ` Rob Clark
  2016-11-07 15:39         ` Archit Taneja
  0 siblings, 1 reply; 12+ messages in thread
From: Rob Clark @ 2016-11-07 14:48 UTC (permalink / raw)
  To: Archit Taneja
  Cc: Sean Paul, linux-arm-msm,
	freedreno-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Daniel Vetter,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW

On Mon, Nov 7, 2016 at 5:38 AM, Archit Taneja <architt@codeaurora.org> wrote:
>
>
> On 11/05/2016 09:55 PM, Rob Clark wrote:
>>
>> Split out the hardware pipe specifics from mdp5_plane.  To start, the hw
>> pipes are statically assigned to planes, but next step is to assign the
>> hw pipes during plane->atomic_check() based on requested caps (scaling,
>> YUV, etc).  And then hw pipe re-assignment if required if required SMP
>> blocks changes.
>>
>> Signed-off-by: Rob Clark <robdclark@gmail.com>
>> ---
>>  drivers/gpu/drm/msm/Makefile              |   1 +
>>  drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c   | 126
>> +++++++++++++++++++-----------
>>  drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h   |   7 +-
>>  drivers/gpu/drm/msm/mdp/mdp5/mdp5_pipe.c  |  43 ++++++++++
>>  drivers/gpu/drm/msm/mdp/mdp5/mdp5_pipe.h  |  39 +++++++++
>>  drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c |  66 +++++++---------
>>  6 files changed, 197 insertions(+), 85 deletions(-)
>>  create mode 100644 drivers/gpu/drm/msm/mdp/mdp5/mdp5_pipe.c
>>  create mode 100644 drivers/gpu/drm/msm/mdp/mdp5/mdp5_pipe.h
>>
>> diff --git a/drivers/gpu/drm/msm/Makefile b/drivers/gpu/drm/msm/Makefile
>> index fb5be3e..90f66c4 100644
>> --- a/drivers/gpu/drm/msm/Makefile
>> +++ b/drivers/gpu/drm/msm/Makefile
>> @@ -37,6 +37,7 @@ msm-y := \
>>         mdp/mdp5/mdp5_irq.o \
>>         mdp/mdp5/mdp5_mdss.o \
>>         mdp/mdp5/mdp5_kms.o \
>> +       mdp/mdp5/mdp5_pipe.o \
>>         mdp/mdp5/mdp5_plane.o \
>>         mdp/mdp5/mdp5_smp.o \
>>         msm_atomic.o \
>> diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c
>> b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c
>> index f1288c7..d3d45ed 100644
>> --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c
>> +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c
>> @@ -119,6 +119,10 @@ static void mdp5_kms_destroy(struct msm_kms *kms)
>>  {
>>         struct mdp5_kms *mdp5_kms = to_mdp5_kms(to_mdp_kms(kms));
>>         struct msm_gem_address_space *aspace = mdp5_kms->aspace;
>> +       int i;
>> +
>> +       for (i = 0; i < mdp5_kms->num_hwpipes; i++)
>> +               mdp5_pipe_destroy(mdp5_kms->hwpipes[i]);
>>
>>         if (aspace) {
>>                 aspace->mmu->funcs->detach(aspace->mmu,
>> @@ -323,15 +327,6 @@ static int modeset_init_intf(struct mdp5_kms
>> *mdp5_kms, int intf_num)
>>
>>  static int modeset_init(struct mdp5_kms *mdp5_kms)
>>  {
>> -       static const enum mdp5_pipe rgb_planes[] = {
>> -                       SSPP_RGB0, SSPP_RGB1, SSPP_RGB2, SSPP_RGB3,
>> -       };
>> -       static const enum mdp5_pipe vig_planes[] = {
>> -                       SSPP_VIG0, SSPP_VIG1, SSPP_VIG2, SSPP_VIG3,
>> -       };
>> -       static const enum mdp5_pipe dma_planes[] = {
>> -                       SSPP_DMA0, SSPP_DMA1,
>> -       };
>>         struct drm_device *dev = mdp5_kms->dev;
>>         struct msm_drm_private *priv = dev->dev_private;
>>         const struct mdp5_cfg_hw *hw_cfg;
>> @@ -339,58 +334,34 @@ static int modeset_init(struct mdp5_kms *mdp5_kms)
>>
>>         hw_cfg = mdp5_cfg_get_hw_config(mdp5_kms->cfg);
>>
>> -       /* construct CRTCs and their private planes: */
>> -       for (i = 0; i < hw_cfg->pipe_rgb.count; i++) {
>> +       /* Construct planes equaling the number of hw pipes, and CRTCs
>> +        * for the N layer-mixers (LM).  The first N planes become primary
>> +        * planes for the CRTCs, with the remainder as overlay planes:
>> +        */
>
>
> Jfyi, we might need to change this a bit in the future. It'll be better to
> get the max number of displays connected on our platform via parsing DT,
> etc, and calculate CRTCs based on that, and not number of layermixers. Maybe
> add a couple for writeback too. This way, we get the right number of
> CRTCs, and we don't rely on #LMs, since we can have 2 per crtc
> in the future.

I *guess* when we get to that stage, we'll dynamically assign LM's
too, in a similar way as hwpipe.  And I suppose we could also put a
cap on # of crtc's based on # of encoders?

BR,
-R

> Reviewed-by: Archit Taneja <architt@codeaurora.org>
>
> --
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
> a Linux Foundation Collaborative Project
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/5] drm/msm/mdp5: introduce mdp5_hw_pipe
  2016-11-07 14:48       ` Rob Clark
@ 2016-11-07 15:39         ` Archit Taneja
  0 siblings, 0 replies; 12+ messages in thread
From: Archit Taneja @ 2016-11-07 15:39 UTC (permalink / raw)
  To: Rob Clark; +Cc: linux-arm-msm, freedreno, dri-devel



On 11/7/2016 8:18 PM, Rob Clark wrote:
> On Mon, Nov 7, 2016 at 5:38 AM, Archit Taneja <architt@codeaurora.org> wrote:
>>
>>
>> On 11/05/2016 09:55 PM, Rob Clark wrote:
>>>
>>> Split out the hardware pipe specifics from mdp5_plane.  To start, the hw
>>> pipes are statically assigned to planes, but next step is to assign the
>>> hw pipes during plane->atomic_check() based on requested caps (scaling,
>>> YUV, etc).  And then hw pipe re-assignment if required if required SMP
>>> blocks changes.
>>>
>>> Signed-off-by: Rob Clark <robdclark@gmail.com>
>>> ---
>>>  drivers/gpu/drm/msm/Makefile              |   1 +
>>>  drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c   | 126
>>> +++++++++++++++++++-----------
>>>  drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h   |   7 +-
>>>  drivers/gpu/drm/msm/mdp/mdp5/mdp5_pipe.c  |  43 ++++++++++
>>>  drivers/gpu/drm/msm/mdp/mdp5/mdp5_pipe.h  |  39 +++++++++
>>>  drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c |  66 +++++++---------
>>>  6 files changed, 197 insertions(+), 85 deletions(-)
>>>  create mode 100644 drivers/gpu/drm/msm/mdp/mdp5/mdp5_pipe.c
>>>  create mode 100644 drivers/gpu/drm/msm/mdp/mdp5/mdp5_pipe.h
>>>
>>> diff --git a/drivers/gpu/drm/msm/Makefile b/drivers/gpu/drm/msm/Makefile
>>> index fb5be3e..90f66c4 100644
>>> --- a/drivers/gpu/drm/msm/Makefile
>>> +++ b/drivers/gpu/drm/msm/Makefile
>>> @@ -37,6 +37,7 @@ msm-y := \
>>>         mdp/mdp5/mdp5_irq.o \
>>>         mdp/mdp5/mdp5_mdss.o \
>>>         mdp/mdp5/mdp5_kms.o \
>>> +       mdp/mdp5/mdp5_pipe.o \
>>>         mdp/mdp5/mdp5_plane.o \
>>>         mdp/mdp5/mdp5_smp.o \
>>>         msm_atomic.o \
>>> diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c
>>> b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c
>>> index f1288c7..d3d45ed 100644
>>> --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c
>>> +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c
>>> @@ -119,6 +119,10 @@ static void mdp5_kms_destroy(struct msm_kms *kms)
>>>  {
>>>         struct mdp5_kms *mdp5_kms = to_mdp5_kms(to_mdp_kms(kms));
>>>         struct msm_gem_address_space *aspace = mdp5_kms->aspace;
>>> +       int i;
>>> +
>>> +       for (i = 0; i < mdp5_kms->num_hwpipes; i++)
>>> +               mdp5_pipe_destroy(mdp5_kms->hwpipes[i]);
>>>
>>>         if (aspace) {
>>>                 aspace->mmu->funcs->detach(aspace->mmu,
>>> @@ -323,15 +327,6 @@ static int modeset_init_intf(struct mdp5_kms
>>> *mdp5_kms, int intf_num)
>>>
>>>  static int modeset_init(struct mdp5_kms *mdp5_kms)
>>>  {
>>> -       static const enum mdp5_pipe rgb_planes[] = {
>>> -                       SSPP_RGB0, SSPP_RGB1, SSPP_RGB2, SSPP_RGB3,
>>> -       };
>>> -       static const enum mdp5_pipe vig_planes[] = {
>>> -                       SSPP_VIG0, SSPP_VIG1, SSPP_VIG2, SSPP_VIG3,
>>> -       };
>>> -       static const enum mdp5_pipe dma_planes[] = {
>>> -                       SSPP_DMA0, SSPP_DMA1,
>>> -       };
>>>         struct drm_device *dev = mdp5_kms->dev;
>>>         struct msm_drm_private *priv = dev->dev_private;
>>>         const struct mdp5_cfg_hw *hw_cfg;
>>> @@ -339,58 +334,34 @@ static int modeset_init(struct mdp5_kms *mdp5_kms)
>>>
>>>         hw_cfg = mdp5_cfg_get_hw_config(mdp5_kms->cfg);
>>>
>>> -       /* construct CRTCs and their private planes: */
>>> -       for (i = 0; i < hw_cfg->pipe_rgb.count; i++) {
>>> +       /* Construct planes equaling the number of hw pipes, and CRTCs
>>> +        * for the N layer-mixers (LM).  The first N planes become primary
>>> +        * planes for the CRTCs, with the remainder as overlay planes:
>>> +        */
>>
>>
>> Jfyi, we might need to change this a bit in the future. It'll be better to
>> get the max number of displays connected on our platform via parsing DT,
>> etc, and calculate CRTCs based on that, and not number of layermixers. Maybe
>> add a couple for writeback too. This way, we get the right number of
>> CRTCs, and we don't rely on #LMs, since we can have 2 per crtc
>> in the future.
>
> I *guess* when we get to that stage, we'll dynamically assign LM's
> too, in a similar way as hwpipe.  And I suppose we could also put a
> cap on # of crtc's based on # of encoders?

Yeah. Currently, we end up creating 2 encoders for each DSI instance,
i.e, 1 for command mode, and another for video mode. Only one can be
used at a time. If we adjust for that, then I guess # of crtcs should
equal to # of encoders.

Archit

>
> BR,
> -R
>
>> Reviewed-by: Archit Taneja <architt@codeaurora.org>
>>
>> --
>> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
>> a Linux Foundation Collaborative Project
> --
> To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2016-11-07 15:39 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-11-05 16:25 [PATCH 0/5] drm/msm/mdp5: atomic smp + hwpipe assignment Rob Clark
2016-11-05 16:25 ` [PATCH 1/5] drm/msm/mdp5: introduce mdp5_hw_pipe Rob Clark
2016-11-07 10:38   ` Archit Taneja
     [not found]     ` <1c36659b-753e-12d4-c958-79d0e8d2af88-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
2016-11-07 14:48       ` Rob Clark
2016-11-07 15:39         ` Archit Taneja
2016-11-05 16:25 ` [PATCH 2/5] drm/msm: subclass drm_atomic_state Rob Clark
     [not found] ` <1478363161-29293-1-git-send-email-robdclark-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2016-11-05 16:25   ` [PATCH 3/5] drm/msm/mdp5: add skeletal mdp5_state Rob Clark
     [not found]     ` <1478363161-29293-4-git-send-email-robdclark-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2016-11-07 10:29       ` Archit Taneja
2016-11-07 11:31         ` Rob Clark
2016-11-05 16:26 ` [PATCH 4/5] drm/msm/mdp5: dynamically assign hw pipes to planes Rob Clark
2016-11-07 10:29   ` Archit Taneja
2016-11-05 16:26 ` [PATCH 5/5] drm/msm/mdp5: handle SMP block allocations "atomically" Rob Clark

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).