NetBSD Problem Report #49645

From www@NetBSD.org  Thu Feb  5 18:12:50 2015
Return-Path: <www@NetBSD.org>
Received: from mail.netbsd.org (mail.netbsd.org [149.20.53.66])
	(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
	(Client CN "mail.netbsd.org", Issuer "Postmaster NetBSD.org" (verified OK))
	by mollari.NetBSD.org (Postfix) with ESMTPS id 9DAAFA650D
	for <gnats-bugs@gnats.NetBSD.org>; Thu,  5 Feb 2015 18:12:50 +0000 (UTC)
Message-Id: <20150205181249.853A7A654C@mollari.NetBSD.org>
Date: Thu,  5 Feb 2015 18:12:49 +0000 (UTC)
From: armihu@gmail.com
Reply-To: armihu@gmail.com
To: gnats-bugs@NetBSD.org
Subject: radeon pmf hooks
X-Send-Pr-Version: www-1.0

>Number:         49645
>Category:       kern
>Synopsis:       radeon pmf hooks
>Confidential:   no
>Severity:       serious
>Priority:       high
>Responsible:    mrg
>State:          closed
>Class:          change-request
>Submitter-Id:   net
>Arrival-Date:   Thu Feb 05 18:15:00 +0000 2015
>Closed-Date:    Thu Apr 23 08:25:35 +0000 2015
>Last-Modified:  Thu Apr 23 08:25:35 +0000 2015
>Originator:     Arto Huusko
>Release:        7.99.4
>Organization:
>Environment:
7.99.4 amd64 GENERIC
>Description:
The radeon kms driver does not support suspend/resume.
>How-To-Repeat:
sysctl -w hw.acpi.sleep.state=3
...
the following drivers do not support: radeon
...
>Fix:
The following patch worked for me with vendor 1002, product 9612, or
drm: initializing kernel modesetting (RS780 0x1002:0x9612 0x1025:0x0148)

Did not try with X11 yet, but suspend/resume worked from console.

--- sys/external/bsd/drm2/radeon/radeon_pci.c
+++ sys/external/bsd/drm2/radeon/radeon_pci.c
@@ -100,6 +100,9 @@ static void	radeon_attach(device_t, device_t, void *);
 static void	radeon_attach_real(device_t);
 static int	radeon_detach(device_t, int);

+static bool	radeon_pmf_suspend(device_t, const pmf_qual_t *);
+static bool	radeon_pmf_resume(device_t, const pmf_qual_t *);
+
 static void	radeon_task_work(struct work *, void *);

 CFATTACH_DECL_NEW(radeon, sizeof(struct radeon_softc),
@@ -157,6 +160,9 @@ radeon_attach(device_t parent, device_t self, void *aux)

 	pci_aprint_devinfo(pa, NULL);

+	if (!pmf_device_register(self, &radeon_pmf_suspend, &radeon_pmf_resume))
+		aprint_error_dev(self, "unable to establish power handler\n");
+
 	/*
 	 * Trivial initialization first; the rest will come after we
 	 * have mounted the root file system and can load firmware
@@ -228,14 +234,14 @@ radeon_detach(device_t self, int flags)
 		return error;

 	if (sc->sc_task_state == RADEON_TASK_ATTACH)
-		return 0;
+        goto out;
 	if (sc->sc_task_u.workqueue != NULL) {
 		workqueue_destroy(sc->sc_task_u.workqueue);
 		sc->sc_task_u.workqueue = NULL;
 	}

 	if (sc->sc_drm_dev == NULL)
-		return 0;
+		goto out;
 	/* XXX errno Linux->NetBSD */
 	error = -drm_pci_detach(sc->sc_drm_dev, flags);
 	if (error)
@@ -243,9 +249,45 @@ radeon_detach(device_t self, int flags)
 		return error;
 	sc->sc_drm_dev = NULL;

+out:	pmf_device_deregister(self);
+
 	return 0;
 }

+static bool
+radeon_pmf_suspend(device_t self, const pmf_qual_t *qual)
+{
+	struct radeon_softc *const sc = device_private(self);
+	struct drm_device *const dev = sc->sc_drm_dev;
+	int ret;
+
+	if (dev == NULL)
+		return true;
+
+	ret = radeon_suspend_kms(dev, true, true);
+	if (ret)
+		return false;
+
+	return true;
+}
+
+static bool
+radeon_pmf_resume(device_t self, const pmf_qual_t *qual)
+{
+	struct radeon_softc *const sc = device_private(self);
+	struct drm_device *const dev = sc->sc_drm_dev;
+	int ret;
+
+	if (dev == NULL)
+		return true;
+
+	ret = radeon_resume_kms(dev, false, true);
+	if (ret)
+		return false;
+
+	return true;
+}
+
 static void
 radeon_task_work(struct work *work, void *cookie __unused)
 {

>Release-Note:

>Audit-Trail:

Responsible-Changed-From-To: kern-bug-people->mrg
Responsible-Changed-By: mrg@NetBSD.org
Responsible-Changed-When: Sun, 19 Apr 2015 00:16:57 +0000
Responsible-Changed-Why:
i commited a similar patch, this one has at least one bug fix, and one
minor change that may make it work better than mine.


State-Changed-From-To: open->pending-pullups
State-Changed-By: mrg@NetBSD.org
State-Changed-When: Sun, 19 Apr 2015 01:10:25 +0000
State-Changed-Why:
i commited remaining changes; they'll need to go to netbsd-7.


From: "matthew green" <mrg@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/49645 CVS commit: src/sys/external/bsd/drm2/radeon
Date: Sun, 19 Apr 2015 01:08:56 +0000

 Module Name:	src
 Committed By:	mrg
 Date:		Sun Apr 19 01:08:56 UTC 2015

 Modified Files:
 	src/sys/external/bsd/drm2/radeon: radeon_pci.c

 Log Message:
 apply some of the additional changes from Arto Huusko in PR#49645:
 - call pmf_device_deregister on detach.

 i've kept the "resume = true" for radeon_resume_kms() call as it
 seems to work for me (indeed, code inspection shows it is unused
 on netbsd :-)

 my old nforce4 box that can resume old drm (or could, last i tried
 several years ago) while X and GL apps were running, can at least
 survive a resume if X hasn't started.  my one attempt so far with
 X exited, but having run, did not work.


 To generate a diff of this commit:
 cvs rdiff -u -r1.8 -r1.9 src/sys/external/bsd/drm2/radeon/radeon_pci.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

From: "Soren Jacobsen" <snj@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/49645 CVS commit: [netbsd-7] src/sys
Date: Thu, 23 Apr 2015 07:31:18 +0000

 Module Name:	src
 Committed By:	snj
 Date:		Thu Apr 23 07:31:17 UTC 2015

 Modified Files:
 	src/sys/arch/x86/include [netbsd-7]: pmap.h
 	src/sys/arch/x86/x86 [netbsd-7]: pmap.c
 	src/sys/dev/pci [netbsd-7]: agp_amd64.c agp_i810.c
 	src/sys/external/bsd/drm2/dist/drm/i915 [netbsd-7]: i915_dma.c
 	    i915_gem.c
 	src/sys/external/bsd/drm2/dist/drm/nouveau [netbsd-7]: nouveau_agp.c
 	    nouveau_ttm.c
 	src/sys/external/bsd/drm2/dist/drm/radeon [netbsd-7]: atombios_crtc.c
 	    radeon_agp.c radeon_display.c radeon_legacy_crtc.c radeon_object.c
 	    radeon_ttm.c
 	src/sys/external/bsd/drm2/dist/drm/ttm [netbsd-7]: ttm_bo.c
 	    ttm_bo_util.c
 	src/sys/external/bsd/drm2/i915drm [netbsd-7]: intelfb.c
 	src/sys/external/bsd/drm2/include/drm [netbsd-7]: drm_wait_netbsd.h
 	src/sys/external/bsd/drm2/include/linux [netbsd-7]: mm.h pci.h
 	src/sys/external/bsd/drm2/nouveau [netbsd-7]: nouveaufb.c
 	src/sys/external/bsd/drm2/radeon [netbsd-7]: radeon_pci.c
 	src/sys/uvm [netbsd-7]: uvm_init.c

 Log Message:
 Pull up following revision(s) (requested by mrg in ticket #718):
 	sys/arch/x86/include/pmap.h: revision 1.56
 	sys/arch/x86/x86/pmap.c: revision 1.188
 	sys/dev/pci/agp_amd64.c: revision 1.8
 	sys/dev/pci/agp_i810.c: revision 1.118
 	sys/external/bsd/drm2/dist/drm/i915/i915_dma.c: revision 1.16
 	sys/external/bsd/drm2/dist/drm/i915/i915_gem.c: revision 1.29
 	sys/external/bsd/drm2/dist/drm/nouveau/nouveau_agp.c: revision 1.3
 	sys/external/bsd/drm2/dist/drm/nouveau/nouveau_ttm.c: revision 1.4
 	sys/external/bsd/drm2/dist/drm/radeon/atombios_crtc.c: revision 1.3
 	sys/external/bsd/drm2/dist/drm/radeon/radeon_agp.c: revision 1.3
 	sys/external/bsd/drm2/dist/drm/radeon/radeon_display.c: revision 1.3
 	sys/external/bsd/drm2/dist/drm/radeon/radeon_legacy_crtc.c: revision 1.2
 	sys/external/bsd/drm2/dist/drm/radeon/radeon_object.c: revision 1.3
 	sys/external/bsd/drm2/dist/drm/radeon/radeon_ttm.c: revision 1.7
 	sys/external/bsd/drm2/dist/drm/ttm/ttm_bo.c: revisions 1.7-1.10
 	sys/external/bsd/drm2/dist/drm/ttm/ttm_bo_util.c: revision 1.5
 	sys/external/bsd/drm2/i915drm/intelfb.c: revision 1.13
 	sys/external/bsd/drm2/include/drm/drm_wait_netbsd.h: revisions 1.12, 1.13
 	sys/external/bsd/drm2/include/linux/mm.h: revision 1.5
 	sys/external/bsd/drm2/include/linux/pci.h: revisions 1.16, 1.17
 	sys/external/bsd/drm2/nouveau/nouveaufb.c: revision 1.2
 	sys/external/bsd/drm2/radeon/radeon_pci.c: revisions 1.8, 1.9
 	sys/uvm/uvm_init.c: revision 1.46
 Hack against the blank console problem:
 Leave the CLUT alone on ancient cards. At least this leaves us with a
 semi working console (red and blue are flipped). Leave an example of what
 seems to be happening but disable it because colors are better than 444 bit
 greyscale.
 --
 Initialize P->V tracking for unmanaged device pages in uvm_init.

 Conditional on __HAVE_PMAP_PV_TRACK until we add it to all pmaps.

 MI part of pmap_pv(9) change proposed on tech-kern:

 https://mail-index.netbsd.org/tech-kern/2015/03/26/msg018561.html
 --
 Implement pmap_pv(9) for x86 for P->V tracking of unmanaged pages.

 Proposed on tech-kern with no objections:

 https://mail-index.netbsd.org/tech-kern/2015/03/26/msg018561.html
 --
 Use pmap_pv(9) to remove mappings of Intel graphics aperture pages.

 Proposed on tech-kern with no objections:

 https://mail-index.netbsd.org/tech-kern/2015/03/26/msg018561.html

 Further background at:

 https://mail-index.netbsd.org/tech-kern/2014/07/23/msg017392.html
 --
 Use pmap_pv(9) to remove mappings of device pages in TTM.

 Adapt nouveau and radeon to do pmap_pv_track for their device pages.

 Proposed on tech-kern with no objections:

 https://mail-index.netbsd.org/tech-kern/2015/03/26/msg018561.html

 Further background at:

 https://mail-index.netbsd.org/tech-kern/2014/07/23/msg017392.html
 --
 Fix error branches in agp_amd64.c.

 - agp_generic_detach always.
 - Free asc if it was allocated.  (Found by Brainy, noted by maxv@.)
 - Free the GATT if it was allocated.
 --
 pmf_device_register returns false on failure, not true
 --
 In DRM_SPIN_WAIT_ON, don't stop after waiting only one tick.

 Continue the loop to recheck the condition and count the whole
 duration.
 --
 Don't use the video BIOS memory as an i915 flush page!
 --
 Don't let anyone else allocate the video BIOS either.
 --
 Missed a zero: it's 0x100000, not 0x10000.
 --
 Don't reserve if atomic -- caller must have pre-pinned the buffer.
 --
 Don't reserve if atomic -- caller must have pre-pinned the buffer.
 --
 almost add radeondrmkms suspend/resume support.  it unfortunately doesn't work.
 --
 Need the page's uvm object lock to do pmap_page_protect.
 --
 Use KASSERTMSG to show bad base/offset.
 --
 KASSERT about page-alignment on initialization too.
 --
 Don't break when hardclock_ticks wraps around.

 Since we now only count time spent in wait, rather than determining
 the end time and checking whether we've passed it, timeouts might be
 marginally longer in effect.  Unlikely to be an issue.
 --
 Remove broken drm2 vm_mmap stub.  Can't possibly have ever worked.
 --
 apply some of the additional changes from Arto Huusko in PR#49645:
 - call pmf_device_deregister on detach.

 i've kept the "resume = true" for radeon_resume_kms() call as it
 seems to work for me (indeed, code inspection shows it is unused
 on netbsd :-)

 my old nforce4 box that can resume old drm (or could, last i tried
 several years ago) while X and GL apps were running, can at least
 survive a resume if X hasn't started.  my one attempt so far with
 X exited, but having run, did not work.
 --
 First attempt to make ttm_buffer_object_transfer less bogus.
 --
 Make sure mem.bus.is_iomem is initialized.  PR 49833


 To generate a diff of this commit:
 cvs rdiff -u -r1.55 -r1.55.4.1 src/sys/arch/x86/include/pmap.h
 cvs rdiff -u -r1.183.2.1 -r1.183.2.2 src/sys/arch/x86/x86/pmap.c
 cvs rdiff -u -r1.7 -r1.7.14.1 src/sys/dev/pci/agp_amd64.c
 cvs rdiff -u -r1.112.2.2 -r1.112.2.3 src/sys/dev/pci/agp_i810.c
 cvs rdiff -u -r1.10.2.3 -r1.10.2.4 \
     src/sys/external/bsd/drm2/dist/drm/i915/i915_dma.c
 cvs rdiff -u -r1.14.2.7 -r1.14.2.8 \
     src/sys/external/bsd/drm2/dist/drm/i915/i915_gem.c
 cvs rdiff -u -r1.2 -r1.2.4.1 \
     src/sys/external/bsd/drm2/dist/drm/nouveau/nouveau_agp.c
 cvs rdiff -u -r1.2.4.1 -r1.2.4.2 \
     src/sys/external/bsd/drm2/dist/drm/nouveau/nouveau_ttm.c
 cvs rdiff -u -r1.2 -r1.2.4.1 \
     src/sys/external/bsd/drm2/dist/drm/radeon/atombios_crtc.c \
     src/sys/external/bsd/drm2/dist/drm/radeon/radeon_agp.c \
     src/sys/external/bsd/drm2/dist/drm/radeon/radeon_display.c \
     src/sys/external/bsd/drm2/dist/drm/radeon/radeon_object.c
 cvs rdiff -u -r1.1.1.1 -r1.1.1.1.4.1 \
     src/sys/external/bsd/drm2/dist/drm/radeon/radeon_legacy_crtc.c
 cvs rdiff -u -r1.5.4.1 -r1.5.4.2 \
     src/sys/external/bsd/drm2/dist/drm/radeon/radeon_ttm.c
 cvs rdiff -u -r1.4.2.1 -r1.4.2.2 \
     src/sys/external/bsd/drm2/dist/drm/ttm/ttm_bo.c
 cvs rdiff -u -r1.4 -r1.4.2.1 \
     src/sys/external/bsd/drm2/dist/drm/ttm/ttm_bo_util.c
 cvs rdiff -u -r1.9.4.2 -r1.9.4.3 src/sys/external/bsd/drm2/i915drm/intelfb.c
 cvs rdiff -u -r1.4.2.3 -r1.4.2.4 \
     src/sys/external/bsd/drm2/include/drm/drm_wait_netbsd.h
 cvs rdiff -u -r1.3.2.1 -r1.3.2.2 src/sys/external/bsd/drm2/include/linux/mm.h
 cvs rdiff -u -r1.7.2.5 -r1.7.2.6 \
     src/sys/external/bsd/drm2/include/linux/pci.h
 cvs rdiff -u -r1.1.2.2 -r1.1.2.3 \
     src/sys/external/bsd/drm2/nouveau/nouveaufb.c
 cvs rdiff -u -r1.4.4.1 -r1.4.4.2 \
     src/sys/external/bsd/drm2/radeon/radeon_pci.c
 cvs rdiff -u -r1.45 -r1.45.12.1 src/sys/uvm/uvm_init.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

State-Changed-From-To: pending-pullups->closed
State-Changed-By: mrg@NetBSD.org
State-Changed-When: Thu, 23 Apr 2015 08:25:35 +0000
State-Changed-Why:
all done.


>Unformatted:

Home	PR Database Search