From patchwork Tue Jul 30 18:46:42 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Markus Trippelsdorf X-Patchwork-Id: 2835873 Return-Path: X-Original-To: patchwork-dri-devel@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork2.web.kernel.org (Postfix) with ESMTP id 58213C0319 for ; Tue, 30 Jul 2013 18:48:36 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 32EA12037B for ; Tue, 30 Jul 2013 18:48:35 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by mail.kernel.org (Postfix) with ESMTP id 2E84D20379 for ; Tue, 30 Jul 2013 18:48:33 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 0FB67E7076 for ; Tue, 30 Jul 2013 11:48:33 -0700 (PDT) X-Original-To: dri-devel@lists.freedesktop.org Delivered-To: dri-devel@lists.freedesktop.org Received: from mail.ud10.udmedia.de (ud10.udmedia.de [194.117.254.50]) by gabe.freedesktop.org (Postfix) with ESMTP id D4B20E6FF2 for ; Tue, 30 Jul 2013 11:46:43 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=simple; d=mail.ud10.udmedia.de; h= date:from:to:cc:subject:message-id:references:mime-version :content-type:in-reply-to; s=beta; bh=KhNgW5vpSHzx+IfkMfMJSU+Pwo boizeIcvQgxk4Pws8=; b=RD3waDq1IERJAH9ybMr33/FkuJPbpn/BwGMZA9wOve QS5A22UNPcV05F08hc0byHtmKzcxQD2swl1rHqFUsTMzzxowM1zwLGK2r4PxnCr/ IWiVlHi6S3198UhuG6gGS4sumoO8PgLeeUp/gdkQHw2y6t1uRigYaJ6Cw8e58RA3 U= Received: (qmail 26324 invoked from network); 30 Jul 2013 20:46:42 +0200 Received: from unknown (HELO x4) (ud10?360p3@91.64.96.185) by mail.ud10.udmedia.de with ESMTPSA (DHE-RSA-AES256-SHA encrypted, authenticated); 30 Jul 2013 20:46:42 +0200 Date: Tue, 30 Jul 2013 20:46:42 +0200 From: Markus Trippelsdorf To: Alex Deucher Subject: Re: Commit f5d9b7f0f9 (fix r600_enable_sclk_control()) causes kexec issues Message-ID: <20130730184642.GA352@x4> References: <20130729075132.GA354@x4> <20130729140957.GA355@x4> <73ccbe50-7ff5-4b04-80b0-55fa89cce19e@email.android.com> <87k3k9z17s.fsf@xmission.com> <20130730112720.GA8608@x4> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: Cc: Alex Deucher , "Eric W. Biederman" , dri-devel@lists.freedesktop.org X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.13 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: dri-devel-bounces+patchwork-dri-devel=patchwork.kernel.org@lists.freedesktop.org Errors-To: dri-devel-bounces+patchwork-dri-devel=patchwork.kernel.org@lists.freedesktop.org X-Spam-Status: No, score=-5.6 required=5.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_MED,RP_MATCHES_RCVD,T_DKIM_INVALID,UNPARSEABLE_RELAY autolearn=ham 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 On 2013.07.30 at 10:53 -0400, Alex Deucher wrote: > On Tue, Jul 30, 2013 at 7:27 AM, Markus Trippelsdorf > wrote: > > On 2013.07.29 at 15:53 -0400, Alex Deucher wrote: > >> On Mon, Jul 29, 2013 at 2:10 PM, Eric W. Biederman > >> wrote: > >> > Alex Deucher writes: > >> > > >> >> On Mon, Jul 29, 2013 at 11:50 AM, Eric W. Biederman > >> >> wrote: > >> >>> > >> >>> > >> >>> Alex Deucher wrote: > >> >>>>On Mon, Jul 29, 2013 at 10:09 AM, Markus Trippelsdorf > >> >>>> wrote: > >> >>>>> On 2013.07.29 at 09:58 -0400, Alex Deucher wrote: > >> >>>>>> On Mon, Jul 29, 2013 at 3:51 AM, Markus Trippelsdorf > >> >>>>>> wrote: > >> >>>>>> > On my test machine Xorg doesn't start anymore when I kexec into a > >> >>>>>> > 3.11.0-rc3 kernel. > >> >>>>>> > >> >>>>>> With kexec, dpm doesn't get torn down properly which can result in a > >> >>>>>> bad hardware state when the driver loads again. Does the attached > >> >>>>>> patch help? It attempts to disable dpm at startup in case it wasn't > >> >>>>>> torn down properly previously. > >> >>>>> > >> >>>>> dpm initialization now works, but unfortunately GPU acceleration > >> >>>>still gets > >> >>>>> disabled: > >> >>>> > >> >>>>Stupid kexec complicates things. We need to make sure dpm is torn > >> >>>>down before we init the rest of the GPU, but dpm needs get initialized > >> >>>>later in the init process since it depends on certain other state from > >> >>>>the driver. I need to think about this for a bit. I'm not sure of a > >> >>>>good way to handle this. > >> >>> > >> >>> You might just want to implement a shutdown method that cleans things up properly. At least as a first pass until you worry about things like kexec on panic. > >> >>> > >> >>> Or can you not shutdown the graphics stack on reboot because of the need to display the kernels shutdown progress? > >> >> > >> >> Does kexec actually call this shutdown method? The driver implements > >> >> appropriate clean-up measures if it's shutdown properly. > >> > > >> > Absoltuely. All parts of the reboot path call ->shutdown. Including > >> > kexec. > >> > > >> > You don't get a device remove/hotunplug but unless this is a kexec on > >> > panic ->shutdown is most definitely called. Now I am talking about the > >> > device layer/pci layer shutdown method I don't know how gpu drivers are > >> > wired up. GPU land was a little strange last I looked. Hopefully it > >> > isn't so strange that there is a method named shutdown that is not wired > >> > up. > >> > >> It doesn't look like the drm infrastructure has a shutdown callback. > >> The drm drivers register a drm_driver callback struct that includes an > >> unload callback which takes care of all the device teardown (if you > >> unload the module for example). I don't know that it actually gets > >> called at kexec time however. I don't know enough about how kexec > >> works. > > > > BTW there is r100_restore_sanity() in drm/radeon/r100.c that explicitly > > handles the kexec case during init. So maybe an r600_restore_sanity() > > function is needed? > > > > (One of the advantages of using kexec (besides the much quicker boot > > time) is that the monitor is not switched off and then on during boot. > > I guess that advantage would be lost if the unload callback would be > > called.) > > r100_restore_sanity() is basically a set of hacks (that gets called at > driver startup) to work around the fact that with kexec the drm driver > is not torn down correctly. So we could add a bunch more asic > specific tear down sequences to deal with dpm (and all the other > engines on the GPU that may potentially cause problems if they are not > torn down properly), but that will just turn into a mess. All of > these hacks also add latency to the driver load. I think the best > solution would probably be to figure how to hook up the drm unload > callback to the shutdown method that kexec uses. FYI the following (ugly) hack works for me. (If I don't comment out radeon_fbdev_fini(rdev) kexec will hang.) diff --git a/drivers/gpu/drm/radeon/r100.c b/drivers/gpu/drm/radeon/r100.c index 75349cd..13e2988 100644 --- a/drivers/gpu/drm/radeon/r100.c +++ b/drivers/gpu/drm/radeon/r100.c @@ -3947,20 +3947,6 @@ void r100_fini(struct radeon_device *rdev) */ void r100_restore_sanity(struct radeon_device *rdev) { - u32 tmp; - - tmp = RREG32(RADEON_CP_CSQ_CNTL); - if (tmp) { - WREG32(RADEON_CP_CSQ_CNTL, 0); - } - tmp = RREG32(RADEON_CP_RB_CNTL); - if (tmp) { - WREG32(RADEON_CP_RB_CNTL, 0); - } - tmp = RREG32(RADEON_SCRATCH_UMSK); - if (tmp) { - WREG32(RADEON_SCRATCH_UMSK, 0); - } } int r100_init(struct radeon_device *rdev) diff --git a/drivers/gpu/drm/radeon/radeon_display.c b/drivers/gpu/drm/radeon/radeon_display.c index c2b67b4..79b38e2 100644 --- a/drivers/gpu/drm/radeon/radeon_display.c +++ b/drivers/gpu/drm/radeon/radeon_display.c @@ -1405,7 +1405,7 @@ int radeon_modeset_init(struct radeon_device *rdev) void radeon_modeset_fini(struct radeon_device *rdev) { - radeon_fbdev_fini(rdev); +// radeon_fbdev_fini(rdev); kfree(rdev->mode_info.bios_hardcoded_edid); radeon_pm_fini(rdev); diff --git a/drivers/gpu/drm/radeon/radeon_drv.c b/drivers/gpu/drm/radeon/radeon_drv.c index 29876b1..e705e8c 100644 --- a/drivers/gpu/drm/radeon/radeon_drv.c +++ b/drivers/gpu/drm/radeon/radeon_drv.c @@ -388,6 +388,15 @@ static const struct file_operations radeon_driver_kms_fops = { #endif }; + +static void +radeon_kexec_shutdown(struct pci_dev *pdev) +{ + struct drm_device *dev = pci_get_drvdata(pdev); + + radeon_driver_unload_kms(dev); +} + static struct drm_driver kms_driver = { .driver_features = DRIVER_USE_AGP | DRIVER_USE_MTRR | DRIVER_PCI_DMA | DRIVER_SG | @@ -463,6 +472,7 @@ static struct pci_driver radeon_kms_pci_driver = { .remove = radeon_pci_remove, .suspend = radeon_pci_suspend, .resume = radeon_pci_resume, + .shutdown = radeon_kexec_shutdown, }; static int __init radeon_init(void)