Re: [PATCH] drm/imx: switch to universal planes

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@freescale.com

[1] https://lkml.org/lkml/2015/11/4/107

Regards, Liu Ying

Signed-off-by: Philipp Zabel p.zabel@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