[PATCH] drm/imx: switch to universal planes

Liu Ying Ying.Liu at freescale.com
Fri Nov 6 05:58:08 PST 2015


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
> 


More information about the dri-devel mailing list