[PATCH 50/72] imx-drm: Fix separate primary plane objects
Steve Longerbeam
slongerbeam at gmail.com
Fri Oct 31 15:54:33 PDT 2014
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 at 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
More information about the dri-devel
mailing list