[PATCH 50/72] imx-drm: Fix separate primary plane objects

[Steve Longerbeam <slongerbeam@gmail.com>]

drm_crtc_init() will create a primary plane object, while imx-drm also
creates its own, thus two primary planes are separately created, and
can cause difficult bugs to track down in the future.

Fix by using drm_crtc_init_with_planes() instead of drm_crtc_init(),
so that we can hand drm our own primary plane object, thus crtc->primary
and ipu_crtc->plane[0].base now are the same object.

To do this we need to statically declare the primary and overlay
planes in the crtc driver, to avoid a chicken-before-the-egg problem,
the primary plane must exist when imx_drm_add_crtc() is called but
before ipu_plane_init().

Signed-off-by: Steve Longerbeam <steve_longerbeam@mentor.com>
---
 drivers/staging/imx-drm/imx-drm-core.c |    3 +-
 drivers/staging/imx-drm/imx-drm.h      |    1 +
 drivers/staging/imx-drm/ipuv3-crtc.c   |   54 ++++++++++++++++++--------------
 drivers/staging/imx-drm/ipuv3-plane.c  |   32 +++++--------------
 drivers/staging/imx-drm/ipuv3-plane.h  |    6 ++--
 5 files changed, 45 insertions(+), 51 deletions(-)

diff --git a/drivers/staging/imx-drm/imx-drm-core.c b/drivers/staging/imx-drm/imx-drm-core.c
index e5ec010..59200ff 100644
--- a/drivers/staging/imx-drm/imx-drm-core.c
+++ b/drivers/staging/imx-drm/imx-drm-core.c
@@ -342,6 +342,7 @@ err_kms:
  * imx_drm_add_crtc - add a new crtc
  */
 int imx_drm_add_crtc(struct drm_device *drm, struct drm_crtc *crtc,
+		struct drm_plane *primary,
 		struct imx_drm_crtc **new_crtc,
 		const struct imx_drm_crtc_helper_funcs *imx_drm_helper_funcs,
 		struct device_node *port)
@@ -380,7 +381,7 @@ int imx_drm_add_crtc(struct drm_device *drm, struct drm_crtc *crtc,
 	drm_crtc_helper_add(crtc,
 			imx_drm_crtc->imx_drm_helper_funcs.crtc_helper_funcs);
 
-	drm_crtc_init(drm, crtc,
+	drm_crtc_init_with_planes(drm, crtc, primary, NULL,
 			imx_drm_crtc->imx_drm_helper_funcs.crtc_funcs);
 
 	return 0;
diff --git a/drivers/staging/imx-drm/imx-drm.h b/drivers/staging/imx-drm/imx-drm.h
index 7453ae0..3a30f16 100644
--- a/drivers/staging/imx-drm/imx-drm.h
+++ b/drivers/staging/imx-drm/imx-drm.h
@@ -24,6 +24,7 @@ struct imx_drm_crtc_helper_funcs {
 };
 
 int imx_drm_add_crtc(struct drm_device *drm, struct drm_crtc *crtc,
+		struct drm_plane *primary,
 		struct imx_drm_crtc **new_crtc,
 		const struct imx_drm_crtc_helper_funcs *imx_helper_funcs,
 		struct device_node *port);
diff --git a/drivers/staging/imx-drm/ipuv3-crtc.c b/drivers/staging/imx-drm/ipuv3-crtc.c
index 5a60017..593261f 100644
--- a/drivers/staging/imx-drm/ipuv3-crtc.c
+++ b/drivers/staging/imx-drm/ipuv3-crtc.c
@@ -84,7 +84,8 @@ struct ipu_crtc {
 	const struct ipu_channels *ch;
 
 	/* plane[0] is the full plane, plane[1] is the partial plane */
-	struct ipu_plane	*plane[2];
+	struct ipu_plane	plane[2];
+	bool			have_overlay; /* we have a partial plane */
 
 	struct ipu_dc		*dc;
 	struct ipu_di		*di;
@@ -106,7 +107,7 @@ static void ipu_fb_enable(struct ipu_crtc *ipu_crtc)
 		return;
 
 	ipu_dc_enable(ipu_crtc->dc);
-	ipu_plane_enable(ipu_crtc->plane[0]);
+	ipu_plane_enable(&ipu_crtc->plane[0]);
 	/* Start DC channel and DI after IDMAC */
 	ipu_dc_enable_channel(ipu_crtc->dc);
 	ipu_di_enable(ipu_crtc->di);
@@ -123,7 +124,7 @@ static void ipu_fb_disable(struct ipu_crtc *ipu_crtc)
 	/* Stop DC channel and DI before IDMAC */
 	ipu_dc_disable_channel(ipu_crtc->dc);
 	ipu_di_disable(ipu_crtc->di);
-	ipu_plane_disable(ipu_crtc->plane[0]);
+	ipu_plane_disable(&ipu_crtc->plane[0]);
 	ipu_dc_disable(ipu_crtc->dc);
 	ipu_di_disable_clock(ipu_crtc->di);
 
@@ -241,7 +242,7 @@ static int ipu_crtc_mode_set(struct drm_crtc *crtc,
 		return ret;
 	}
 
-	return ipu_plane_mode_set(ipu_crtc->plane[0], crtc, mode,
+	return ipu_plane_mode_set(&ipu_crtc->plane[0], crtc, mode,
 				  crtc->primary->fb,
 				  0, 0, mode->hdisplay, mode->vdisplay,
 				  x, y, mode->hdisplay, mode->vdisplay);
@@ -267,7 +268,7 @@ static irqreturn_t ipu_irq_handler(int irq, void *dev_id)
 	imx_drm_handle_vblank(ipu_crtc->imx_crtc);
 
 	if (ipu_crtc->newfb) {
-		struct ipu_plane *plane = ipu_crtc->plane[0];
+		struct ipu_plane *plane = &ipu_crtc->plane[0];
 
 		ipu_crtc->newfb = NULL;
 		ipu_plane_set_base(plane, ipu_crtc->base.primary->fb,
@@ -474,20 +475,27 @@ static int ipu_crtc_init(struct ipu_crtc *ipu_crtc,
 		return ret;
 	}
 
-	ret = imx_drm_add_crtc(drm, &ipu_crtc->base, &ipu_crtc->imx_crtc,
-			       &ipu_crtc_helper_funcs, ipu_crtc->port);
+	ret = imx_drm_add_crtc(drm, &ipu_crtc->base, &ipu_crtc->plane[0].base,
+			       &ipu_crtc->imx_crtc, &ipu_crtc_helper_funcs,
+			       ipu_crtc->port);
 	if (ret) {
 		dev_err(ipu_crtc->dev, "adding crtc failed with %d.\n", ret);
 		goto err_put_resources;
 	}
 
 	id = imx_drm_crtc_id(ipu_crtc->imx_crtc);
-	ipu_crtc->plane[0] = ipu_plane_init(ipu_crtc->base.dev,
-					    ipu_crtc->ipu,
-					    ipu_crtc->ch->dma[0],
-					    ipu_crtc->ch->dp[0],
-					    BIT(id), true);
-	ret = ipu_plane_get_resources(ipu_crtc->plane[0]);
+	ret = ipu_plane_init(&ipu_crtc->plane[0], drm,
+			     ipu_crtc->ipu,
+			     ipu_crtc->ch->dma[0],
+			     ipu_crtc->ch->dp[0],
+			     BIT(id), true);
+	if (ret) {
+		dev_err(ipu_crtc->dev, "init primary plane failed with %d\n",
+			ret);
+		goto err_remove_crtc;
+	}
+
+	ret = ipu_plane_get_resources(&ipu_crtc->plane[0]);
 	if (ret) {
 		dev_err(ipu_crtc->dev, "getting plane 0 resources failed with %d.\n",
 			ret);
@@ -496,16 +504,16 @@ static int ipu_crtc_init(struct ipu_crtc *ipu_crtc,
 
 	/* If this crtc is using the DP, add an overlay plane */
 	if (ipu_crtc->ch->dp[1] >= 0) {
-		ipu_crtc->plane[1] = ipu_plane_init(ipu_crtc->base.dev,
-						    ipu_crtc->ipu,
-						    ipu_crtc->ch->dma[1],
-						    ipu_crtc->ch->dp[1],
-						    BIT(id), false);
-		if (IS_ERR(ipu_crtc->plane[1]))
-			ipu_crtc->plane[1] = NULL;
+		ret = ipu_plane_init(&ipu_crtc->plane[1], drm,
+				     ipu_crtc->ipu,
+				     ipu_crtc->ch->dma[1],
+				     ipu_crtc->ch->dp[1],
+				     BIT(id), false);
+
+		ipu_crtc->have_overlay = ret ? false : true;
 	}
 
-	ipu_crtc->irq = ipu_plane_irq(ipu_crtc->plane[0]);
+	ipu_crtc->irq = ipu_plane_irq(&ipu_crtc->plane[0]);
 	ret = devm_request_irq(ipu_crtc->dev, ipu_crtc->irq, ipu_irq_handler, 0,
 			"imx_drm", ipu_crtc);
 	if (ret < 0) {
@@ -516,7 +524,7 @@ static int ipu_crtc_init(struct ipu_crtc *ipu_crtc,
 	return 0;
 
 err_put_plane_res:
-	ipu_plane_put_resources(ipu_crtc->plane[0]);
+	ipu_plane_put_resources(&ipu_crtc->plane[0]);
 err_remove_crtc:
 	imx_drm_remove_crtc(ipu_crtc->imx_crtc);
 err_put_resources:
@@ -553,7 +561,7 @@ static void ipu_drm_unbind(struct device *dev, struct device *master,
 
 	imx_drm_remove_crtc(ipu_crtc->imx_crtc);
 
-	ipu_plane_put_resources(ipu_crtc->plane[0]);
+	ipu_plane_put_resources(&ipu_crtc->plane[0]);
 	ipu_put_resources(ipu_crtc);
 }
 
diff --git a/drivers/staging/imx-drm/ipuv3-plane.c b/drivers/staging/imx-drm/ipuv3-plane.c
index 1572c80..51a257a 100644
--- a/drivers/staging/imx-drm/ipuv3-plane.c
+++ b/drivers/staging/imx-drm/ipuv3-plane.c
@@ -338,13 +338,10 @@ static int ipu_disable_plane(struct drm_plane *plane)
 
 static void ipu_plane_destroy(struct drm_plane *plane)
 {
-	struct ipu_plane *ipu_plane = to_ipu_plane(plane);
-
 	DRM_DEBUG_KMS("[%d] %s\n", __LINE__, __func__);
 
 	ipu_disable_plane(plane);
 	drm_plane_cleanup(plane);
-	kfree(ipu_plane);
 }
 
 static int ipu_plane_set_global_alpha(struct ipu_plane *ipu_plane,
@@ -426,22 +423,15 @@ static struct drm_plane_funcs ipu_plane_funcs = {
 	.set_property	= ipu_plane_set_property,
 };
 
-struct ipu_plane *ipu_plane_init(struct drm_device *drm, struct ipu_soc *ipu,
-				 int dma, int dp, unsigned int possible_crtcs,
-				 bool priv)
+int ipu_plane_init(struct ipu_plane *ipu_plane, struct drm_device *drm,
+		   struct ipu_soc *ipu, int dma, int dp,
+		   unsigned int possible_crtcs, bool priv)
 {
-	struct ipu_plane *ipu_plane;
 	int ret;
 
 	DRM_DEBUG_KMS("channel %d, dp flow %d, possible_crtcs=0x%x\n",
 		      dma, dp, possible_crtcs);
 
-	ipu_plane = kzalloc(sizeof(*ipu_plane), GFP_KERNEL);
-	if (!ipu_plane) {
-		DRM_ERROR("failed to allocate plane\n");
-		return ERR_PTR(-ENOMEM);
-	}
-
 	ipu_plane->ipu = ipu;
 	ipu_plane->dma = dma;
 	ipu_plane->dp_flow = dp;
@@ -452,7 +442,7 @@ struct ipu_plane *ipu_plane_init(struct drm_device *drm, struct ipu_soc *ipu,
 			     priv);
 	if (ret) {
 		DRM_ERROR("failed to initialize plane\n");
-		goto err_free;
+		return ret;
 	}
 
 	/* default global alpha is enabled and completely opaque */
@@ -461,7 +451,7 @@ struct ipu_plane *ipu_plane_init(struct drm_device *drm, struct ipu_soc *ipu,
 
 	/* for private planes, skip setting up properties */
 	if (priv)
-		return ipu_plane;
+		return 0;
 
 	/*
 	 * global alpha range is 0 - 255. Bit 8 is used as a
@@ -472,8 +462,7 @@ struct ipu_plane *ipu_plane_init(struct drm_device *drm, struct ipu_soc *ipu,
 								 0, 0x1ff);
 	if (!ipu_plane->global_alpha_prop) {
 		DRM_ERROR("failed to create global alpha property\n");
-		ret = -ENOMEM;
-		goto err_free;
+		return -ENOMEM;
 	}
 
 	/*
@@ -485,8 +474,7 @@ struct ipu_plane *ipu_plane_init(struct drm_device *drm, struct ipu_soc *ipu,
 							     0, 0x01ffffff);
 	if (!ipu_plane->colorkey_prop) {
 		DRM_ERROR("failed to create colorkey property\n");
-		ret = -ENOMEM;
-		goto err_free;
+		return -ENOMEM;
 	}
 
 	drm_object_attach_property(&ipu_plane->base.base,
@@ -495,9 +483,5 @@ struct ipu_plane *ipu_plane_init(struct drm_device *drm, struct ipu_soc *ipu,
 	drm_object_attach_property(&ipu_plane->base.base,
 				   ipu_plane->colorkey_prop,
 				   0);
-	return ipu_plane;
-
-err_free:
-	kfree(ipu_plane);
-	return ERR_PTR(ret);
+	return 0;
 }
diff --git a/drivers/staging/imx-drm/ipuv3-plane.h b/drivers/staging/imx-drm/ipuv3-plane.h
index b4daee5..1487a08 100644
--- a/drivers/staging/imx-drm/ipuv3-plane.h
+++ b/drivers/staging/imx-drm/ipuv3-plane.h
@@ -37,9 +37,9 @@ struct ipu_plane {
 	bool			enabled;
 };
 
-struct ipu_plane *ipu_plane_init(struct drm_device *dev, struct ipu_soc *ipu,
-				 int dma, int dp, unsigned int possible_crtcs,
-				 bool priv);
+int ipu_plane_init(struct ipu_plane *ipu_plane, struct drm_device *drm,
+		   struct ipu_soc *ipu, int dma, int dp,
+		   unsigned int possible_crtcs, bool priv);
 
 /* Init IDMAC, DMFC, DP */
 int ipu_plane_mode_set(struct ipu_plane *plane, struct drm_crtc *crtc,
-- 
1.7.9.5

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