Hi Phillip, On Fri, Nov 06, 2015 at 11:12:20AM +0100, Philipp Zabel wrote: > Use drm_universal_plane_init to create the planes, create the primary > plane first and use drm_crtc_init_with_planes to associate it with > the crtc.
It's better to mention in the commit message that this patch removes the safe plane which is created by create_primary_plane(). Or, at least, it fixes the NULL pointer dereference issue triggered by the drm modetest I mentioned in my patch[1]. Otherwise, Acked-by: Liu Ying <Ying.Liu at freescale.com> [1] https://lkml.org/lkml/2015/11/4/107 Regards, Liu Ying > > Signed-off-by: Philipp Zabel <p.zabel at pengutronix.de> > --- > drivers/gpu/drm/imx/imx-drm-core.c | 4 ++-- > drivers/gpu/drm/imx/imx-drm.h | 3 ++- > drivers/gpu/drm/imx/ipuv3-crtc.c | 20 ++++++++++---------- > drivers/gpu/drm/imx/ipuv3-plane.c | 9 ++++----- > drivers/gpu/drm/imx/ipuv3-plane.h | 2 +- > 5 files changed, 19 insertions(+), 19 deletions(-) > > diff --git a/drivers/gpu/drm/imx/imx-drm-core.c > b/drivers/gpu/drm/imx/imx-drm-core.c > index ee7981b..4140caa 100644 > --- a/drivers/gpu/drm/imx/imx-drm-core.c > +++ b/drivers/gpu/drm/imx/imx-drm-core.c > @@ -340,7 +340,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 imx_drm_crtc **new_crtc, > + struct imx_drm_crtc **new_crtc, struct drm_plane *primary_plane, > const struct imx_drm_crtc_helper_funcs *imx_drm_helper_funcs, > struct device_node *port) > { > @@ -379,7 +379,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_plane, NULL, > imx_drm_crtc->imx_drm_helper_funcs.crtc_funcs); > > return 0; > diff --git a/drivers/gpu/drm/imx/imx-drm.h b/drivers/gpu/drm/imx/imx-drm.h > index 10ed4e1..71380b9 100644 > --- a/drivers/gpu/drm/imx/imx-drm.h > +++ b/drivers/gpu/drm/imx/imx-drm.h > @@ -9,6 +9,7 @@ struct drm_display_mode; > struct drm_encoder; > struct drm_fbdev_cma; > struct drm_framebuffer; > +struct drm_plane; > struct imx_drm_crtc; > struct platform_device; > > @@ -24,7 +25,7 @@ struct imx_drm_crtc_helper_funcs { > }; > > int imx_drm_add_crtc(struct drm_device *drm, struct drm_crtc *crtc, > - struct imx_drm_crtc **new_crtc, > + struct imx_drm_crtc **new_crtc, struct drm_plane *primary_plane, > const struct imx_drm_crtc_helper_funcs *imx_helper_funcs, > struct device_node *port); > int imx_drm_remove_crtc(struct imx_drm_crtc *); > diff --git a/drivers/gpu/drm/imx/ipuv3-crtc.c > b/drivers/gpu/drm/imx/ipuv3-crtc.c > index 7bc8301..872183a 100644 > --- a/drivers/gpu/drm/imx/ipuv3-crtc.c > +++ b/drivers/gpu/drm/imx/ipuv3-crtc.c > @@ -349,7 +349,6 @@ static int ipu_crtc_init(struct ipu_crtc *ipu_crtc, > struct ipu_soc *ipu = dev_get_drvdata(ipu_crtc->dev->parent); > int dp = -EINVAL; > int ret; > - int id; > > ret = ipu_get_resources(ipu_crtc, pdata); > if (ret) { > @@ -358,18 +357,19 @@ static int ipu_crtc_init(struct ipu_crtc *ipu_crtc, > return ret; > } > > + if (pdata->dp >= 0) > + dp = IPU_DP_FLOW_SYNC_BG; > + ipu_crtc->plane[0] = ipu_plane_init(drm, ipu, pdata->dma[0], dp, 0, > + DRM_PLANE_TYPE_PRIMARY); > + > ret = imx_drm_add_crtc(drm, &ipu_crtc->base, &ipu_crtc->imx_crtc, > - &ipu_crtc_helper_funcs, ipu_crtc->dev->of_node); > + &ipu_crtc->plane[0]->base, &ipu_crtc_helper_funcs, > + ipu_crtc->dev->of_node); > if (ret) { > dev_err(ipu_crtc->dev, "adding crtc failed with %d.\n", ret); > goto err_put_resources; > } > > - if (pdata->dp >= 0) > - dp = IPU_DP_FLOW_SYNC_BG; > - id = imx_drm_crtc_id(ipu_crtc->imx_crtc); > - ipu_crtc->plane[0] = ipu_plane_init(ipu_crtc->base.dev, ipu, > - pdata->dma[0], dp, BIT(id), true); > ret = ipu_plane_get_resources(ipu_crtc->plane[0]); > if (ret) { > dev_err(ipu_crtc->dev, "getting plane 0 resources failed with > %d.\n", > @@ -379,10 +379,10 @@ static int ipu_crtc_init(struct ipu_crtc *ipu_crtc, > > /* If this crtc is using the DP, add an overlay plane */ > if (pdata->dp >= 0 && pdata->dma[1] > 0) { > - ipu_crtc->plane[1] = ipu_plane_init(ipu_crtc->base.dev, ipu, > - pdata->dma[1], > + ipu_crtc->plane[1] = ipu_plane_init(drm, ipu, pdata->dma[1], > IPU_DP_FLOW_SYNC_FG, > - BIT(id), false); > + > drm_crtc_mask(&ipu_crtc->base), > + DRM_PLANE_TYPE_OVERLAY); > if (IS_ERR(ipu_crtc->plane[1])) > ipu_crtc->plane[1] = NULL; > } > diff --git a/drivers/gpu/drm/imx/ipuv3-plane.c > b/drivers/gpu/drm/imx/ipuv3-plane.c > index 0c5c4d9..d2ca451 100644 > --- a/drivers/gpu/drm/imx/ipuv3-plane.c > +++ b/drivers/gpu/drm/imx/ipuv3-plane.c > @@ -453,7 +453,7 @@ static struct drm_plane_funcs ipu_plane_funcs = { > > struct ipu_plane *ipu_plane_init(struct drm_device *dev, struct ipu_soc *ipu, > int dma, int dp, unsigned int possible_crtcs, > - bool priv) > + enum drm_plane_type type) > { > struct ipu_plane *ipu_plane; > int ret; > @@ -471,10 +471,9 @@ struct ipu_plane *ipu_plane_init(struct drm_device *dev, > struct ipu_soc *ipu, > ipu_plane->dma = dma; > ipu_plane->dp_flow = dp; > > - ret = drm_plane_init(dev, &ipu_plane->base, possible_crtcs, > - &ipu_plane_funcs, ipu_plane_formats, > - ARRAY_SIZE(ipu_plane_formats), > - priv); > + ret = drm_universal_plane_init(dev, &ipu_plane->base, possible_crtcs, > + &ipu_plane_funcs, ipu_plane_formats, > + ARRAY_SIZE(ipu_plane_formats), type); > if (ret) { > DRM_ERROR("failed to initialize plane\n"); > kfree(ipu_plane); > diff --git a/drivers/gpu/drm/imx/ipuv3-plane.h > b/drivers/gpu/drm/imx/ipuv3-plane.h > index c8913ed..d02558f 100644 > --- a/drivers/gpu/drm/imx/ipuv3-plane.h > +++ b/drivers/gpu/drm/imx/ipuv3-plane.h > @@ -36,7 +36,7 @@ struct ipu_plane { > > struct ipu_plane *ipu_plane_init(struct drm_device *dev, struct ipu_soc *ipu, > int dma, int dp, unsigned int possible_crtcs, > - bool priv); > + enum drm_plane_type type); > > /* Init IDMAC, DMFC, DP */ > int ipu_plane_mode_set(struct ipu_plane *plane, struct drm_crtc *crtc, > -- > 2.6.1 >
