[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20151116160021.GX16848@phenom.ffwll.local>
Date: Mon, 16 Nov 2015 17:00:21 +0100
From: Daniel Vetter <daniel@...ll.ch>
To: Liu Ying <Ying.Liu@...escale.com>
Cc: dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/2] drm/imx: Remove the primary plane created by
create_primary_plane()
On Wed, Nov 04, 2015 at 06:15:58PM +0800, Liu Ying wrote:
> 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@...4'
> 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@...escale.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);
>
> return 0;
> diff --git a/drivers/gpu/drm/imx/ipuv3-crtc.c b/drivers/gpu/drm/imx/ipuv3-crtc.c
> index 8d68697..d27143f 100644
> --- a/drivers/gpu/drm/imx/ipuv3-crtc.c
> +++ b/drivers/gpu/drm/imx/ipuv3-crtc.c
> @@ -343,6 +343,11 @@ err_out:
> return ret;
> }
>
> +static inline void ipu_crtc_set_primary_plane(struct ipu_crtc *ipu_crtc)
> +{
> + ipu_crtc->base.primary = &ipu_crtc->plane[0]->base;
This is quite a hack. Better would be to reorg the code so that when you
call drm_crtc_init_with_planes the primary plane has been created already.
-Daniel
> +}
> +
> static int ipu_crtc_init(struct ipu_crtc *ipu_crtc,
> struct ipu_client_platformdata *pdata, struct drm_device *drm)
> {
> @@ -380,6 +385,7 @@ static int ipu_crtc_init(struct ipu_crtc *ipu_crtc,
> ret);
> goto err_remove_crtc;
> }
> + ipu_crtc_set_primary_plane(ipu_crtc);
>
> /* If this crtc is using the DP, add an overlay plane */
> if (pdata->dp >= 0 && pdata->dma[1] > 0) {
> --
> 2.5.0
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@...ts.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists