All of lore.kernel.org
 help / color / mirror / Atom feed
From: Liu Ying <Ying.Liu@freescale.com>
To: <dri-devel@lists.freedesktop.org>
Cc: <linux-kernel@vger.kernel.org>, <p.zabel@pengutronix.de>,
	<airlied@linux.ie>
Subject: [PATCH 2/2] drm/imx: Remove the primary plane created by create_primary_plane()
Date: Wed, 4 Nov 2015 18:15:58 +0800	[thread overview]
Message-ID: <1446632158-19998-2-git-send-email-Ying.Liu@freescale.com> (raw)
In-Reply-To: <1446632158-19998-1-git-send-email-Ying.Liu@freescale.com>

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);
 
 	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;
+}
+
 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


WARNING: multiple messages have this Message-ID (diff)
From: Liu Ying <Ying.Liu@freescale.com>
To: dri-devel@lists.freedesktop.org
Cc: linux-kernel@vger.kernel.org, p.zabel@pengutronix.de, airlied@linux.ie
Subject: [PATCH 2/2] drm/imx: Remove the primary plane created by create_primary_plane()
Date: Wed, 4 Nov 2015 18:15:58 +0800	[thread overview]
Message-ID: <1446632158-19998-2-git-send-email-Ying.Liu@freescale.com> (raw)
In-Reply-To: <1446632158-19998-1-git-send-email-Ying.Liu@freescale.com>

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);
 
 	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;
+}
+
 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

  reply	other threads:[~2015-11-04 10:10 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-11-04 10:15 [PATCH 1/2] drm/imx: ipuv3-crtc: Return error if ipu_plane_init() fails for primary plane Liu Ying
2015-11-04 10:15 ` Liu Ying
2015-11-04 10:15 ` Liu Ying [this message]
2015-11-04 10:15   ` [PATCH 2/2] drm/imx: Remove the primary plane created by create_primary_plane() Liu Ying
2015-11-06 10:05   ` Philipp Zabel
2015-11-06 10:05     ` Philipp Zabel
2015-11-06 10:38     ` Liu Ying
2015-11-06 10:38       ` Liu Ying
2015-11-16 16:00   ` Daniel Vetter
2015-11-16 16:00     ` Daniel Vetter
2015-11-17  2:10     ` Liu Ying
2015-11-17  2:10       ` Liu Ying
2015-11-06 10:05 ` [PATCH 1/2] drm/imx: ipuv3-crtc: Return error if ipu_plane_init() fails for primary plane Philipp Zabel
2015-11-06 10:05   ` Philipp Zabel
2015-11-06 10:25   ` Liu Ying
2015-11-06 10:25     ` Liu Ying

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1446632158-19998-2-git-send-email-Ying.Liu@freescale.com \
    --to=ying.liu@freescale.com \
    --cc=airlied@linux.ie \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=p.zabel@pengutronix.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.