Message-ID: <1446804348.4569.1.camel@pengutronix.de>
Subject: Re: [PATCH 2/2] drm/imx: Remove the primary plane created by
 create_primary_plane()
From: Philipp Zabel <p.zabel@pengutronix.de>
To: Liu Ying <Ying.Liu@freescale.com>
Cc: dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org,
        airlied@linux.ie
Date: Fri, 06 Nov 2015 11:05:48 +0100
In-Reply-To: <1446632158-19998-2-git-send-email-Ying.Liu@freescale.com>
References: <1446632158-19998-1-git-send-email-Ying.Liu@freescale.com>
	 <1446632158-19998-2-git-send-email-Ying.Liu@freescale.com>
Content-Type: text/plain; charset="UTF-8"
Mime-Version: 1.0
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 2293
Lines: 49

Am Mittwoch, den 04.11.2015, 18:15 +0800 schrieb Liu Ying:
> Since we are using ipu_plane_init() to add one primary plane for each
> IPU CRTC, it's unnecessary to create the safe one by using the helper
> create_primary_plane().
> 
> Furthermore, the safe one is attached to crtc->primary, which actually
> carries a framebuffer(crtc->primary->fb) created by the fbdev helper to
> build up the fbcon.  Instead, the one created by ipu_plane_init() is
> dangling, but it is the one actually does ipu_plane_mode_set() for the
> fbcon.  This may causes the mismatch bewteen ipu_plane->enabled(true) and
> ipu_plane->base.fb(NULL).  Thus, it brings a NULL pointer dereference
> issue in ipu_plane_mode_set() when we try to additionally touch the
> IDMAC channel of the ipu_plane.  This issue could be reproduced by
> running the drm modetest with command line 'modetest -P 19:1024x768@XR24'
> on the i.MX6Q SabreSD platform(single LVDS display).  This patch binds
> the plane created by ipu_plane_init() with crtc->primary and removes the
> safe one to address this issue.
> 
> Signed-off-by: Liu Ying <Ying.Liu@freescale.com>
> ---
>  drivers/gpu/drm/imx/imx-drm-core.c | 3 ++-
>  drivers/gpu/drm/imx/ipuv3-crtc.c   | 6 ++++++
>  2 files changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/imx/imx-drm-core.c b/drivers/gpu/drm/imx/imx-drm-core.c
> index 6faa735..08eceeb 100644
> --- a/drivers/gpu/drm/imx/imx-drm-core.c
> +++ b/drivers/gpu/drm/imx/imx-drm-core.c
> @@ -373,7 +373,8 @@ 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,
> +	/* The related primary plane will be created in ipu_plane_init(). */
> +	drm_crtc_init_with_planes(drm, crtc, NULL, NULL,
>  			imx_drm_crtc->imx_drm_helper_funcs.crtc_funcs);

We rather should generate the plane first and add the primary_plane
parameter to imx_drm_add_crtc than calling drm_crtc_init_with_planes
without planes.

regards
Philipp

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/