From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1758478AbcEFOyW (ORCPT ); Fri, 6 May 2016 10:54:22 -0400 Received: from mail-wm0-f45.google.com ([74.125.82.45]:37997 "EHLO mail-wm0-f45.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1758125AbcEFOyV (ORCPT ); Fri, 6 May 2016 10:54:21 -0400 Date: Fri, 6 May 2016 16:54:17 +0200 From: Thierry Reding To: Robin Murphy Cc: airlied@linux.ie, liviu.dudau@arm.com, dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org Subject: Re: [PATCH 1/2] drm: hdlcd: Skip PM callbacks if unbound Message-ID: <20160506145417.GB15076@ulmo.ba.sec> References: MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="p4qYPpj5QlsIQJ0K" Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.6.1 (2016-04-27) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org --p4qYPpj5QlsIQJ0K Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Thu, May 05, 2016 at 05:13:37PM +0100, Robin Murphy wrote: > Until an encoder component is available to trigger the HDLCD's component > master bind callback, the drm_device we keep in drvdata isn't allocated. > Since it is quite possible to, say, hibernate the system without having > loaded the encoder driver, the PM callbacks need to check for a valid > drm_device before trying to take the lock and suspend/resume the crtcs, > otherwise this happens: >=20 > [ 51.779092] Unable to handle kernel NULL pointer dereference at virtua= l address 00000200 > [ 51.787143] pgd =3D ffff800974462000 > [ 51.790514] [00000200] *pgd=3D0000000000000000 > [ 51.794768] Internal error: Oops: 96000004 [#1] PREEMPT SMP > [ 51.800291] Modules linked in: > [ 51.803325] CPU: 4 PID: 2098 Comm: systemd-sleep Not tainted 4.6.0-rc6= + #4 > [ 51.810140] Hardware name: ARM Juno development board (r1) (DT) > [ 51.816008] task: ffff800974430000 ti: ffff80097380c000 task.ti: ffff8= 0097380c000 > [ 51.823433] PC is at mutex_lock+0x14/0x60 > [ 51.827411] LR is at drm_modeset_lock_all+0x3c/0xe0 > [ 51.832246] pc : [] lr : [] pstate= : 40000145 > [ 51.839577] sp : ffff80097380faf0 > [ 51.842859] x29: ffff80097380faf0 x28: 0000000000000000 > [ 51.848132] x27: 0000000000000002 x26: ffff000008e01000 > [ 51.853405] x25: ffff0000084cc000 x24: ffff000008dcb408 > [ 51.858678] x23: ffff000008e7c000 x22: ffff8009760e8870 > [ 51.863962] x21: 0000000000000200 x20: 0000000000000000 > [ 51.869244] x19: 0000000000000200 x18: ffff800079357dd0 > [ 51.874527] x17: 0000ffff8a00ab88 x16: 0000000000000018 > [ 51.879809] x15: 0000000000000018 x14: 0000000000000000 > [ 51.885091] x13: 0000000000000002 x12: 0000000000000000 > [ 51.890374] x11: ffff0000087ff904 x10: 0000000000000840 > [ 51.895656] x9 : 0000000000000000 x8 : ffff800973821680 > [ 51.900938] x7 : 0000000000000000 x6 : 000000000000003f > [ 51.906220] x5 : 0000000000000040 x4 : 0000000000000000 > [ 51.911502] x3 : 0000000000000004 x2 : 0000000000000000 > [ 51.916784] x1 : 0000000000000001 x0 : 0000000000000200 > ... > [ 52.339406] [] mutex_lock+0x14/0x60 > [ 52.344425] [] drm_modeset_lock_all+0x3c/0xe0 > [ 52.350305] [] hdlcd_pm_suspend+0x2c/0x90 > [ 52.355842] [] platform_pm_suspend+0x24/0x58 > [ 52.361638] [] dpm_run_callback.isra.4+0x20/0x68 > [ 52.367778] [] __device_suspend+0xe4/0x238 > [ 52.373400] [] dpm_suspend+0x10c/0x240 > [ 52.378679] [] dpm_suspend_start+0x70/0x80 > [ 52.384302] [] suspend_devices_and_enter+0x9c/0x480 > [ 52.390699] [] pm_suspend+0x1d0/0x240 > [ 52.395890] [] state_store+0x80/0x100 > [ 52.401084] [] kobj_attr_store+0x14/0x28 > [ 52.406535] [] sysfs_kf_write+0x48/0x58 > [ 52.411899] [] kernfs_fop_write+0xbc/0x188 > [ 52.417521] [] __vfs_write+0x1c/0xe0 > [ 52.422626] [] vfs_write+0x8c/0x1a8 > [ 52.427644] [] SyS_write+0x44/0xa0 > [ 52.432578] [] __sys_trace_return+0x0/0x4 > [ 52.438114] Code: 910003fd f9000bf3 aa0003f3 f9800011 (885ffc01) > [ 52.444229] ---[ end trace 34b87a5fb9453f65 ]--- >=20 > Acked-by: Liviu Dudau > Signed-off-by: Robin Murphy > --- > drivers/gpu/drm/arm/hdlcd_drv.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) >=20 > diff --git a/drivers/gpu/drm/arm/hdlcd_drv.c b/drivers/gpu/drm/arm/hdlcd_= drv.c > index 734899c4e4bb..86a5eea4cf53 100644 > --- a/drivers/gpu/drm/arm/hdlcd_drv.c > +++ b/drivers/gpu/drm/arm/hdlcd_drv.c > @@ -498,7 +498,7 @@ static int __maybe_unused hdlcd_pm_suspend(struct dev= ice *dev) > struct drm_device *drm =3D dev_get_drvdata(dev); > struct drm_crtc *crtc; > =20 > - if (pm_runtime_suspended(dev)) > + if (pm_runtime_suspended(dev) || !drm) > return 0; > =20 > drm_modeset_lock_all(drm); > @@ -513,7 +513,7 @@ static int __maybe_unused hdlcd_pm_resume(struct devi= ce *dev) > struct drm_device *drm =3D dev_get_drvdata(dev); > struct drm_crtc *crtc; > =20 > - if (!pm_runtime_suspended(dev)) > + if (!pm_runtime_suspended(dev) || !drm) > return 0; Perhaps rather than this workaround you should only enable runtime PM on this after the master has been bound? Thierry --p4qYPpj5QlsIQJ0K Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAABCAAGBQJXLLAXAAoJEN0jrNd/PrOh9q8P/i71pPnggSidGyge8ILeLtpt im38GDx90OoQQWagNhpVQ26D9BfMvsPgjD/Y4T2gQ2ouaEgW4omNO8phokcCEUSW p7R6X0ldbwK6eqyblIopCE9hBdd+b1Uq6lG91wXv2X6W3y1sUMXs/N9OYYaUgwhK x5yXPHnHljI/NP42mYmZGhCWsxAwnoruZpTSwh9DWBivVfRRizkCn+Er4+F1ztd7 MvzWXoUANEM0LkPFs+Su3kynFrNixtrbASHkpsrFv/ttkujHW+g7qEN6/AK5Ud30 jsk55iYfQNOO0JQgQUHBC2goQ5m+fWNDbj9JcLw5GKF/IqJwdW74obRDsNacQ6sT BuBsBWLNfsle8BVMnMeQub0vvFfC6dGiPxODNqqALkfapjL3iTxxehAZT4ygtov6 tVhJgUAGzBAF0dgJ3BzvS66jxKD02Ms7sfPoFyWdUOddtPEnzb0lzM6SPyGWCx9B 7QwDtMGVQ3SdG3fljV5NjTKC4lfHW4Gr9QdataWHHovPxhURsYjOWxKztBayUGn6 UXFlKKepY3ZVFLPd8Mjn0RUvlTA1T42FPkTHpKDHoIsuy9/cEKJ9WD+Y/wGW/mZX 6Kv1Nq17kvks5HV+adRjJlRRmWN2DZLs2W5vAoxWvj0/alJmsWDKh47/Cchb4R4W sFaaPn+BHqMX+DlW8JHL =jbbF -----END PGP SIGNATURE----- --p4qYPpj5QlsIQJ0K--