Thread (6 messages) flat view 6 messages, 2 authors, 2014-08-19
STALE4379d

Revision v1 of 4 in this series.

Revisions (4)
  1. v1 [diff vs current]
  2. v1 current
  3. v1 [diff vs current]
  4. v2 [diff vs current]

[PATCH 2/2] imx-drm: ipuv3-plane: fix kernel Oops in ipu_dp_put()

From: p.zabel@pengutronix.de (Philipp Zabel)
Date: 2014-08-19 13:59:38
Also in: dri-devel
Subsystem: staging subsystem, the rest · Maintainers: Greg Kroah-Hartman, Linus Torvalds

Hi Shawn,

Am Dienstag, den 19.08.2014, 17:49 +0800 schrieb Shawn Guo:
Unbinding imx-hdmi or imx-ldb driver triggers a kernel Oops in function
ipu_dp_put() as below.

$ echo 120000.hdmi > /sys/devices/soc0/soc/2000000.aips-bus/120000.hdmi/driver/unbind
[...]
quoted hunk ↗ jump to hunk
From the call stack, the issue is caused by that ipu_dp_put() is called
from ipu_plane_dpms() with ipu_plane->dp being NULL.  Rather than
being called unconditionally, ipu_dp_put() should be called only when
ipu_plane->dp is valid, just like what ipu_plane_put_resources() does.
Acutally, all those ipu_*_put() calls in ipu_plane_dpms() are
unnecessary, because the only occurrence of ipu_plane_dpms() with 'mode'
not being DRM_MODE_DPMS_ON is from function ipu_disable_plane(), which
already has a ipu_plane_put_resources() call right after
ipu_plane_dpms().

The patch fixes above kernel Oops by removing those buggy and
unnecessary ipu_*_put() calls from ipu_plane_dpms().

Signed-off-by: Shawn Guo <redacted>
---
 drivers/staging/imx-drm/ipuv3-plane.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)
diff --git a/drivers/staging/imx-drm/ipuv3-plane.c b/drivers/staging/imx-drm/ipuv3-plane.c
index 6f393a11f44d..a79ae7731a03 100644
--- a/drivers/staging/imx-drm/ipuv3-plane.c
+++ b/drivers/staging/imx-drm/ipuv3-plane.c
@@ -274,15 +274,10 @@ static void ipu_plane_dpms(struct ipu_plane *ipu_plane, int mode)
 	if (enable == ipu_plane->enabled)
 		return;
 
-	if (enable) {
+	if (enable)
 		ipu_plane_enable(ipu_plane);
-	} else {
+	else
 		ipu_plane_disable(ipu_plane);
-
-		ipu_idmac_put(ipu_plane->ipu_ch);
-		ipu_dmfc_put(ipu_plane->dmfc);
-		ipu_dp_put(ipu_plane->dp);
-	}
 }
 
 /*
we could then remove ipu_plane_dpms completely:

---
 drivers/staging/imx-drm/ipuv3-plane.c | 23 ++++-------------------
 1 file changed, 4 insertions(+), 19 deletions(-)
diff --git a/drivers/staging/imx-drm/ipuv3-plane.c b/drivers/staging/imx-drm/ipuv3-plane.c
index b2d1bb2..6987e16 100644
--- a/drivers/staging/imx-drm/ipuv3-plane.c
+++ b/drivers/staging/imx-drm/ipuv3-plane.c
@@ -290,23 +290,6 @@ void ipu_plane_disable(struct ipu_plane *ipu_plane)
 		ipu_dp_disable(ipu_plane->ipu);
 }
 
-static void ipu_plane_dpms(struct ipu_plane *ipu_plane, int mode)
-{
-	bool enable;
-
-	DRM_DEBUG_KMS("mode = %d", mode);
-
-	enable = (mode == DRM_MODE_DPMS_ON);
-
-	if (enable == ipu_plane->enabled)
-		return;
-
-	if (enable)
-		ipu_plane_enable(ipu_plane);
-	else
-		ipu_plane_disable(ipu_plane);
-}
-
 /*
  * drm_plane API
  */
@@ -340,7 +323,8 @@ static int ipu_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
 				plane->crtc, crtc);
 	plane->crtc = crtc;
 
-	ipu_plane_dpms(ipu_plane, DRM_MODE_DPMS_ON);
+	if (!ipu_plane->enabled)
+		ipu_plane_enable(ipu_plane);
 
 	return 0;
 }
@@ -351,7 +335,8 @@ static int ipu_disable_plane(struct drm_plane *plane)
 
 	DRM_DEBUG_KMS("[%d] %s\n", __LINE__, __func__);
 
-	ipu_plane_dpms(ipu_plane, DRM_MODE_DPMS_OFF);
+	if (ipu_plane->enabled)
+		ipu_plane_disable(ipu_plane);
 
 	ipu_plane_put_resources(ipu_plane);
 
-- 
2.1.0.rc1

regards
Philipp
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help