From patchwork Tue Aug 19 13:59:38 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Philipp Zabel X-Patchwork-Id: 4743191 Return-Path: X-Original-To: patchwork-dri-devel@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork1.web.kernel.org (Postfix) with ESMTP id E9E509F375 for ; Tue, 19 Aug 2014 13:59:58 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id C00E920136 for ; Tue, 19 Aug 2014 13:59:57 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by mail.kernel.org (Postfix) with ESMTP id A749D2012E for ; Tue, 19 Aug 2014 13:59:56 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 0BD196E5D8; Tue, 19 Aug 2014 06:59:55 -0700 (PDT) X-Original-To: dri-devel@lists.freedesktop.org Delivered-To: dri-devel@lists.freedesktop.org Received: from metis.ext.pengutronix.de (metis.ext.pengutronix.de [92.198.50.35]) by gabe.freedesktop.org (Postfix) with ESMTP id 17A336E5D8 for ; Tue, 19 Aug 2014 06:59:54 -0700 (PDT) Received: from paszta.hi.pengutronix.de ([2001:67c:670:100:96de:80ff:fec2:9969]) by metis.ext.pengutronix.de with esmtp (Exim 4.72) (envelope-from ) id 1XJjwu-00050C-NK; Tue, 19 Aug 2014 15:59:44 +0200 Message-ID: <1408456778.2899.6.camel@paszta.hi.pengutronix.de> Subject: Re: [PATCH 2/2] imx-drm: ipuv3-plane: fix kernel Oops in ipu_dp_put() From: Philipp Zabel To: Shawn Guo Date: Tue, 19 Aug 2014 15:59:38 +0200 In-Reply-To: <1408441795-24719-3-git-send-email-shawn.guo@freescale.com> References: <1408441795-24719-1-git-send-email-shawn.guo@freescale.com> <1408441795-24719-3-git-send-email-shawn.guo@freescale.com> X-Mailer: Evolution 3.12.2-1 Mime-Version: 1.0 X-SA-Exim-Connect-IP: 2001:67c:670:100:96de:80ff:fec2:9969 X-SA-Exim-Mail-From: p.zabel@pengutronix.de X-SA-Exim-Scanned: No (on metis.ext.pengutronix.de); SAEximRunCond expanded to false X-PTX-Original-Recipient: dri-devel@lists.freedesktop.org Cc: linux-arm-kernel@lists.infradead.org, Greg Kroah-Hartman , Russell King , dri-devel@lists.freedesktop.org, kernel@pengutronix.de X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" X-Spam-Status: No, score=-4.9 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_MED, RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP 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 [...] > 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 > --- > 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);