Re: out-of-bounds array index
On Thursday, February 07, 2008 10:56 am Jens Axboe wrote:
Hi,
Just saw this from gcc:
drivers/char/drm/i915_drv.c: In function ?i915_suspend?:
drivers/char/drm/i915_drv.c:173: warning: array subscript is above array
bounds
CC [M] drivers/char/drm/i915_dma.o
drivers/char/drm/i915_drv.c: In function ?i915_resume?:
drivers/char/drm/i915_drv.c:220: warning: array subscript is above array
bounds
It's this code:
dev_priv-saveGR[0x18] =
i915_read_indexed(VGA_GR_INDEX, VGA_GR_DATA, 0x18);
which looks legit, since saveGR is
u8 saveGR[24];
It has been introduced by commit
ba8bbcf6ff4650712f64c0ef61139c73898e2165, which seems to be you Jesse.
Just a silly off by one, don't know why I didn't catch it earlier. I'll push
the fix to the drm tree. Linus, you may want to take it in parallel.
Jesse
Make sure we have enough room for all the GR registers or we'll end up
clobbering the AR index register (which should actually be harmless unless
the BIOS is making an assumption about it).
Signed-off-by: Jesse Barnes [EMAIL PROTECTED]
diff --git a/drivers/char/drm/i915_drv.h b/drivers/char/drm/i915_drv.h
index 37bbf67..f8308bf 100644
--- a/drivers/char/drm/i915_drv.h
+++ b/drivers/char/drm/i915_drv.h
@@ -187,7 +187,7 @@ typedef struct drm_i915_private {
u32 saveSWF2[3];
u8 saveMSR;
u8 saveSR[8];
- u8 saveGR[24];
+ u8 saveGR[25];
u8 saveAR_INDEX;
u8 saveAR[20];
u8 saveDACMASK;
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [git pull] drm patches for 2.6.25
On Wednesday, February 06, 2008 9:37 pm Dave Airlie wrote: > Hi Linus, > > Please pull the 'drm-patches' branch from > ssh://master.kernel.org/pub/scm/linux/kernel/git/airlied/drm-2.6.git > drm-patches > > Sorry this is so late, after much talking at LCA we decided to pull TTM > from this round and I had to re-order a lot of things, so highlights of > this bunch are > > intel driver suspend/resume support - can bring back text mode now. Just FYI, given some of the weirdness we've seen in 8xx chipsets in the 2D driver, this new suspend/resume code may still need a few tweaks. Please test it out and report any bugs you find asap (preferably w/o X running as that makes things easier to debug). Also, if you find that your outputs don't come back, I've got an experimental patch at https://bugs.freedesktop.org/show_bug.cgi?id=14249 that may work for you (this one in particular is the one I'm worried may break 855). Thanks, Jesse -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] x86: introduce /dev/mem restrictions with a config option
On Thursday, January 31, 2008 9:42 am H. Peter Anvin wrote: > Arjan van de Ven wrote: > > On Thu, 31 Jan 2008 17:53:04 +0100 (CET) > > > > Jan Engelhardt <[EMAIL PROTECTED]> wrote: > >> On Jan 30 2008 12:48, Arjan van de Ven wrote: > >>> Subject: [PATCH] x86: introduce /dev/mem restrictions with a > >>> config option > >>> > >>> This patch introduces a restriction on /dev/mem: Only non-memory > >>> can be read or written unless the newly introduced config option > >>> is set. > >> > >> Would not it be nicer to add a /dev/pcimem that implements the > >> given restrictive semantics? > >> > >> Maybe it's just wishful thinking, but I am dreaming of an > >> unprivileged X, and /dev/pcimem (owned by an 'x11' user or so) > >> would be a step in that direction. > > > > /dev/pcimem is wrong; X can use the exact bar in sysfs already. > > This is more for compatibility with legacy X > > Legacy X, and non-BAR X memory (originally ISA, of course; now > probably more often "stolen system memory"). For legacy memory, we actually have /sys/bus/pci//legacy_mem (though ia64 may be the only supported platform). It's actually required on some arches due to the way this space is allocated across the system. Jesse -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: e1000 full-duplex TCP performance well below wire speed
Bill Fink wrote: > a 2.6.15.4 kernel. The GigE NICs are Intel PRO/1000 > 82546EB_QUAD_COPPER, > on a 64-bit/133-MHz PCI-X bus, using version 6.1.16-k2 of the e1000 > driver, and running with 9000-byte jumbo frames. The TCP congestion > control is BIC. Bill, FYI, there was a known issue with e1000 (fixed in 7.0.38-k2) and socket charge due to truesize that kept one end or the other from opening its window. The result is not so great performance, and you must upgrade the driver at both ends to fix it. it was fixed in commit 9e2feace1acd38d7a3b1275f7f9f8a397d09040e That commit itself needed a couple of follow on bug fixes, but the point is that you could download 7.3.20 from sourceforge (which would compile on your kernel) and compare the performance with it if you were interested in a further experiment. Jesse -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: e1000 full-duplex TCP performance well below wire speed
Bill Fink wrote: a 2.6.15.4 kernel. The GigE NICs are Intel PRO/1000 82546EB_QUAD_COPPER, on a 64-bit/133-MHz PCI-X bus, using version 6.1.16-k2 of the e1000 driver, and running with 9000-byte jumbo frames. The TCP congestion control is BIC. Bill, FYI, there was a known issue with e1000 (fixed in 7.0.38-k2) and socket charge due to truesize that kept one end or the other from opening its window. The result is not so great performance, and you must upgrade the driver at both ends to fix it. it was fixed in commit 9e2feace1acd38d7a3b1275f7f9f8a397d09040e That commit itself needed a couple of follow on bug fixes, but the point is that you could download 7.3.20 from sourceforge (which would compile on your kernel) and compare the performance with it if you were interested in a further experiment. Jesse -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] x86: introduce /dev/mem restrictions with a config option
On Thursday, January 31, 2008 9:42 am H. Peter Anvin wrote: Arjan van de Ven wrote: On Thu, 31 Jan 2008 17:53:04 +0100 (CET) Jan Engelhardt [EMAIL PROTECTED] wrote: On Jan 30 2008 12:48, Arjan van de Ven wrote: Subject: [PATCH] x86: introduce /dev/mem restrictions with a config option This patch introduces a restriction on /dev/mem: Only non-memory can be read or written unless the newly introduced config option is set. Would not it be nicer to add a /dev/pcimem that implements the given restrictive semantics? Maybe it's just wishful thinking, but I am dreaming of an unprivileged X, and /dev/pcimem (owned by an 'x11' user or so) would be a step in that direction. /dev/pcimem is wrong; X can use the exact bar in sysfs already. This is more for compatibility with legacy X Legacy X, and non-BAR X memory (originally ISA, of course; now probably more often stolen system memory). For legacy memory, we actually have /sys/bus/pci/busno/legacy_mem (though ia64 may be the only supported platform). It's actually required on some arches due to the way this space is allocated across the system. Jesse -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Patch v2] Make PCI extended config space (MMCONFIG) a driver opt-in
On Tuesday 29 January 2008 05:19:55 am Greg KH wrote: > Hahahaha, oh, that's a good one... > > But what about the thousands of implementations out there that are > buggy? > > I'm with Arjan here, I'm very skeptical. Ugg, let's look at the actual data (again); I'm really not sure why people are jumping to such dire conclusions about the current state of things. AIUI we only have 3 issues so far (remember mmconfig has been enabled in -mm for a long time): 1) host bridge decode problems (disabling decode to avoid overlaps can cause some bridges to stop decoding RAM addrs, but we have a fix for that) 2) config space retry on ATI (I think willy already debunked this one?) 3) some FUD about SMM or other firmware interrupts coming in during BAR sizing while decode is disabled (this one is just pure FUD; if we want to solve it properly we need a new platform hook to disable SMM/NMI/etc. around PCI probing) What else was there? What reason do we have to think that things are so disastrous? So I really prefer willy's approach to Arjan's alternative... Jesse -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: Mostly revert "e1000/e1000e: Move PCI-Express device IDs over to e1000e"
Frans Pop wrote: > There is one thing I don't understand, but that may well be just me... > > From Linus' original patch: >> +++ b/drivers/net/e1000/e1000_main.c >> + INTEL_E1000_ETHERNET_DEVICE(0x108C), > > So, apparently support for 8086:108c was removed from the e1000 > driver. When it was enabled to be supported by e1000e. > From my lspci: > $ lspci -nn | grep Ether > 01:00.0 Ethernet controller [0200]: Intel Corporation 82573E Gigabit > Ethernet Controller (Copper) [8086:108c] (rev 03) > > But when I look at where that card is sitting: > $ readlink pci/devices/\:01\:00.0/driver > ../../../../bus/pci/drivers/e1000 > > So, it's on the PCI bus, not on the PCI-Express bus (which I also > have, but > which has no devices on it). 82573E/L are PCIe devices only, don't let the use of "PCI configuration space" confuse you. All PCIe devices support PCI configuration space. This allows systems with PCIe to work right (or mostly right) with all the PCI supporting software like Linux. > Or does the e1000e driver also support cards on the PCI bus? E1000e is targeted at the PCIe devices only. > If that's the case then the original changelog entry "Move PCI-Express > device IDs over to e1000e" is misleading as it's not only PCI-Express > devices... Unfortunate bit of confusion over terminology. > Hmmm. Or does which driver is loaded decide on which bus the device > ends up? Hope this helped, Jesse -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: Mostly revert e1000/e1000e: Move PCI-Express device IDs over to e1000e
Frans Pop wrote: There is one thing I don't understand, but that may well be just me... From Linus' original patch: +++ b/drivers/net/e1000/e1000_main.c + INTEL_E1000_ETHERNET_DEVICE(0x108C), So, apparently support for 8086:108c was removed from the e1000 driver. When it was enabled to be supported by e1000e. From my lspci: $ lspci -nn | grep Ether 01:00.0 Ethernet controller [0200]: Intel Corporation 82573E Gigabit Ethernet Controller (Copper) [8086:108c] (rev 03) But when I look at where that card is sitting: $ readlink pci/devices/\:01\:00.0/driver ../../../../bus/pci/drivers/e1000 So, it's on the PCI bus, not on the PCI-Express bus (which I also have, but which has no devices on it). 82573E/L are PCIe devices only, don't let the use of PCI configuration space confuse you. All PCIe devices support PCI configuration space. This allows systems with PCIe to work right (or mostly right) with all the PCI supporting software like Linux. Or does the e1000e driver also support cards on the PCI bus? E1000e is targeted at the PCIe devices only. If that's the case then the original changelog entry Move PCI-Express device IDs over to e1000e is misleading as it's not only PCI-Express devices... Unfortunate bit of confusion over terminology. Hmmm. Or does which driver is loaded decide on which bus the device ends up? Hope this helped, Jesse -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Patch v2] Make PCI extended config space (MMCONFIG) a driver opt-in
On Tuesday 29 January 2008 05:19:55 am Greg KH wrote: Hahahaha, oh, that's a good one... But what about the thousands of implementations out there that are buggy? I'm with Arjan here, I'm very skeptical. Ugg, let's look at the actual data (again); I'm really not sure why people are jumping to such dire conclusions about the current state of things. AIUI we only have 3 issues so far (remember mmconfig has been enabled in -mm for a long time): 1) host bridge decode problems (disabling decode to avoid overlaps can cause some bridges to stop decoding RAM addrs, but we have a fix for that) 2) config space retry on ATI (I think willy already debunked this one?) 3) some FUD about SMM or other firmware interrupts coming in during BAR sizing while decode is disabled (this one is just pure FUD; if we want to solve it properly we need a new platform hook to disable SMM/NMI/etc. around PCI probing) What else was there? What reason do we have to think that things are so disastrous? So I really prefer willy's approach to Arjan's alternative... Jesse -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] x86_32: trim memory by updating e820 v2
On Sunday, January 20, 2008 10:56 pm Yinghai Lu wrote: > [PATCH] x86_32: trim memory by updating e820 v2 > > when mtrr is not covering all e820 table, need to trim the ram, need to > update e820 > > reuse some code for x86_64 > > here need to add early_identify_cpu for x86_32, and move mtrr_bp_init early > > compiled test only, need someone test it I like this approach too. So as long as the E820 modification method works (i.e. we have some testers, maybe Justin can give it a try), you can add Signed-off-by: Jesse Barnes <[EMAIL PROTECTED]> or Acked-by: Jesse Barnes <[EMAIL PROTECTED]> as appropriate too. Thanks, Jesse -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] x86_32: trim memory by updating e820 v2
On Sunday, January 20, 2008 10:56 pm Yinghai Lu wrote: [PATCH] x86_32: trim memory by updating e820 v2 when mtrr is not covering all e820 table, need to trim the ram, need to update e820 reuse some code for x86_64 here need to add early_identify_cpu for x86_32, and move mtrr_bp_init early compiled test only, need someone test it I like this approach too. So as long as the E820 modification method works (i.e. we have some testers, maybe Justin can give it a try), you can add Signed-off-by: Jesse Barnes [EMAIL PROTECTED] or Acked-by: Jesse Barnes [EMAIL PROTECTED] as appropriate too. Thanks, Jesse -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [REGRESSION] 2.6.24-rc7: e1000: Detected Tx Unit Hang
David Miller wrote: > From: Robert Olsson <[EMAIL PROTECTED]> > Date: Fri, 18 Jan 2008 14:00:57 +0100 > >> I don't understand the idea with semaphore for enabling/disabling >> irq's either the overall logic must safer/better without it. > > They must have had code paths where they didn't know if IRQs were > enabled or not already, so they tried to create something which > approximates the: > > local_irq_save(flags); > local_irq_restore(flags); > > constructs we have for CPU interrupts, so they could go: > > e1000_irq_disable(); > /* ... */ > e1000_irq_enable(); > > and this would work even if the caller was running > with e1000 interrupts disabled already. > > Or, something like that... it is indeed confusing. > > Anyways, yes it's totally bogus and should be removed. I agree, bogus, and in fact I've already removed it from our development version of ixgbe. Right now I wanted to report I can't remove e1000 at all on 2.6.24-rc8+git I continually get the kernel: unregister_netdevice: waiting for eth2 to become free. Usage count = 1 Where 2.6.24-rc5 e1000 is okay still. Seems like maybe we are still missing a netif_rx_complete or a napi_disable somewhere. I don't think this problem has anything to do with the irq_sem right now. Something is still badly broken. I am just using the interface regularly (no heavy load) and I can't unload the module. Jesse -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [REGRESSION] 2.6.24-rc7: e1000: Detected Tx Unit Hang
David Miller wrote: From: Robert Olsson [EMAIL PROTECTED] Date: Fri, 18 Jan 2008 14:00:57 +0100 I don't understand the idea with semaphore for enabling/disabling irq's either the overall logic must safer/better without it. They must have had code paths where they didn't know if IRQs were enabled or not already, so they tried to create something which approximates the: local_irq_save(flags); local_irq_restore(flags); constructs we have for CPU interrupts, so they could go: e1000_irq_disable(); /* ... */ e1000_irq_enable(); and this would work even if the caller was running with e1000 interrupts disabled already. Or, something like that... it is indeed confusing. Anyways, yes it's totally bogus and should be removed. I agree, bogus, and in fact I've already removed it from our development version of ixgbe. Right now I wanted to report I can't remove e1000 at all on 2.6.24-rc8+git I continually get the kernel: unregister_netdevice: waiting for eth2 to become free. Usage count = 1 Where 2.6.24-rc5 e1000 is okay still. Seems like maybe we are still missing a netif_rx_complete or a napi_disable somewhere. I don't think this problem has anything to do with the irq_sem right now. Something is still badly broken. I am just using the interface regularly (no heavy load) and I can't unload the module. Jesse -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 02/11] PAT x86: Map only usable memory in x86_64 identity map and kernel text
On Friday, January 18, 2008 10:12 am Andi Kleen wrote: > I looked back then when I had bisected it down and I admit I didn't spot > the problem from source review. I think it came from the reordering so > blacklisting AMD alone wouldn't have helped. Might have been some > subtle race (e.g. long ago we had such races in the MTRR code > triggered by the first HT CPUs) > > Anyways I just test booted latest git-x86 with your patches included on > the QC system and it booted now. However it has both more RAM and newer > CPUs (the original ones were pre-production, that is why I also didn't send > you logs[1] ..) then when I tested originally. So this means either the > problem was somewhere else or the different configuration hides it. > > I guess you will hear about it if it's still broken on other machines. > > Currently it looks good. > > I think it should be enabled on AMD too though. If the reordering breaks > it then blacklisting won't help anyways. > > -Andi > > [1] but I checked the known errata and there was nothing related to MTRR. Ah, ok, that explains your reticence earlier. Thanks for testing again, I guess the patch is good to go. Jesse -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 02/11] PAT x86: Map only usable memory in x86_64 identity map and kernel text
On Friday, January 18, 2008 5:12 am Andi Kleen wrote: > > (AMD machines apparently don't need it > > That's not true -- we had AMD systems in the past with broken MTRRs for > large memory configurations too, Mostly it was pre revE though. It should be easy enough to enable it for AMD as well, and it would also be good to track down the one failure you found... I don't *think* the re-ordering of MTRR initialization should affect AMDs anymore than it does Intel, but someone familiar with the boot code would have to do a quick audit to be sure. Jesse -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 02/11] PAT x86: Map only usable memory in x86_64 identity map and kernel text
On Friday, January 18, 2008 5:12 am Andi Kleen wrote: (AMD machines apparently don't need it That's not true -- we had AMD systems in the past with broken MTRRs for large memory configurations too, Mostly it was pre revE though. It should be easy enough to enable it for AMD as well, and it would also be good to track down the one failure you found... I don't *think* the re-ordering of MTRR initialization should affect AMDs anymore than it does Intel, but someone familiar with the boot code would have to do a quick audit to be sure. Jesse -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 02/11] PAT x86: Map only usable memory in x86_64 identity map and kernel text
On Friday, January 18, 2008 10:12 am Andi Kleen wrote: I looked back then when I had bisected it down and I admit I didn't spot the problem from source review. I think it came from the reordering so blacklisting AMD alone wouldn't have helped. Might have been some subtle race (e.g. long ago we had such races in the MTRR code triggered by the first HT CPUs) Anyways I just test booted latest git-x86 with your patches included on the QC system and it booted now. However it has both more RAM and newer CPUs (the original ones were pre-production, that is why I also didn't send you logs[1] ..) then when I tested originally. So this means either the problem was somewhere else or the different configuration hides it. I guess you will hear about it if it's still broken on other machines. Currently it looks good. I think it should be enabled on AMD too though. If the reordering breaks it then blacklisting won't help anyways. -Andi [1] but I checked the known errata and there was nothing related to MTRR. Ah, ok, that explains your reticence earlier. Thanks for testing again, I guess the patch is good to go. Jesse -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [E1000-devel] 2.6.24-rc8-mm1
Andrew Morton wrote: > On Thu, 17 Jan 2008 11:22:19 -0800 "Pallipadi, Venkatesh" > <[EMAIL PROTECTED]> wrote: > >> >> The problem is modprobe:2584 conflicting cache attribute 5000-50001000 uncached<->default >> >> Some address range here is being mapped with conflicting types. >> Somewhere the range was mapped with default (write-back). Later >> pci_iomap() is mapping that region as uncacheable which is basically >> aliasing. PAT code detects the aliasing and fails the second >> uncacheable request which leads in the failure. its probably the e100 screaming interrupt disable quirk code doing the mapping? > It sounds to me like you need considerably more runtime debugging and > reporting support in that code. Ensure that it generates enough > output > both during regular operation and during failures for you to be able > to > diagnose things in a single iteration. > > We can always take it out later. FWIW (nothing) I agree. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [E1000-devel] 2.6.24-rc8-mm1
Andrew Morton wrote: On Thu, 17 Jan 2008 11:22:19 -0800 Pallipadi, Venkatesh [EMAIL PROTECTED] wrote: The problem is modprobe:2584 conflicting cache attribute 5000-50001000 uncached-default Some address range here is being mapped with conflicting types. Somewhere the range was mapped with default (write-back). Later pci_iomap() is mapping that region as uncacheable which is basically aliasing. PAT code detects the aliasing and fails the second uncacheable request which leads in the failure. its probably the e100 screaming interrupt disable quirk code doing the mapping? It sounds to me like you need considerably more runtime debugging and reporting support in that code. Ensure that it generates enough output both during regular operation and during failures for you to be able to diagnose things in a single iteration. We can always take it out later. FWIW (nothing) I agree. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [REGRESSION] 2.6.24-rc7: e1000: Detected Tx Unit Hang
David Miller wrote: > From: "Brandeburg, Jesse" <[EMAIL PROTECTED]> > Date: Tue, 15 Jan 2008 13:53:43 -0800 > >> The tx code has an "early exit" that tries to limit the amount of tx >> packets handled in a single poll loop and requires napi or interrupt >> rescheduling based on the return value from e1000_clean_tx_irq. > > That explains everything, thanks Jesse. > > Ok, here is the patch I'll propose to fix this. The goal is to make > it as simple as possible without regressing the thing we were trying > to fix. We spent Wednesday trying to reproduce (without the patch) these issues without much luck, and have applied the patch cleanly and will continue testing it. Given the simplicity of the changes, and the community testing, I'll give my ack and we will continue testing. I think we should fix Robert's (unrelated, but in this thread) reported issue before 2.6.24 final if we can, and I'll look at that tonight and tomorrow. Thanks for your work on this Dave, Jesse Acked-by: Jesse Brandeburg <[EMAIL PROTECTED]> -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [REGRESSION] 2.6.24-rc7: e1000: Detected Tx Unit Hang
David Miller wrote: From: Brandeburg, Jesse [EMAIL PROTECTED] Date: Tue, 15 Jan 2008 13:53:43 -0800 The tx code has an early exit that tries to limit the amount of tx packets handled in a single poll loop and requires napi or interrupt rescheduling based on the return value from e1000_clean_tx_irq. That explains everything, thanks Jesse. Ok, here is the patch I'll propose to fix this. The goal is to make it as simple as possible without regressing the thing we were trying to fix. We spent Wednesday trying to reproduce (without the patch) these issues without much luck, and have applied the patch cleanly and will continue testing it. Given the simplicity of the changes, and the community testing, I'll give my ack and we will continue testing. I think we should fix Robert's (unrelated, but in this thread) reported issue before 2.6.24 final if we can, and I'll look at that tonight and tomorrow. Thanks for your work on this Dave, Jesse Acked-by: Jesse Brandeburg [EMAIL PROTECTED] -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [REGRESSION] 2.6.24-rc7: e1000: Detected Tx Unit Hang
[EMAIL PROTECTED] wrote: > Quoting Frans Pop <[EMAIL PROTECTED]>: >>> (Note this isn't the final correct patch we should apply. There is >>> no reason why this revert back to the older ->poll() logic here >>> should have any effect on the TX hang triggering...) >> >> s/no reason/no obvious reason/ ? ;-) The tx code has an "early exit" that tries to limit the amount of tx packets handled in a single poll loop and requires napi or interrupt rescheduling based on the return value from e1000_clean_tx_irq. see this code in e1000_clean_tx_irq 4005 #ifdef CONFIG_E1000_NAPI 4006 #define E1000_TX_WEIGHT 64 4007 > > /* weight of a sort for tx, to avoid endless transmit cleanup */ 4008 > > if (count++ == E1000_TX_WEIGHT) break; 4009 #endif I think that is probably related. For a test you could apply the original patch, and remove this "break" just by commenting out line 4008. This would guarantee all tx work is cleaned at every e1000_clean Jesse -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [REGRESSION] 2.6.24-rc7: e1000: Detected Tx Unit Hang
[EMAIL PROTECTED] wrote: Quoting Frans Pop [EMAIL PROTECTED]: (Note this isn't the final correct patch we should apply. There is no reason why this revert back to the older -poll() logic here should have any effect on the TX hang triggering...) s/no reason/no obvious reason/ ? ;-) The tx code has an early exit that tries to limit the amount of tx packets handled in a single poll loop and requires napi or interrupt rescheduling based on the return value from e1000_clean_tx_irq. see this code in e1000_clean_tx_irq 4005 #ifdef CONFIG_E1000_NAPI 4006 #define E1000_TX_WEIGHT 64 4007 /* weight of a sort for tx, to avoid endless transmit cleanup */ 4008 if (count++ == E1000_TX_WEIGHT) break; 4009 #endif I think that is probably related. For a test you could apply the original patch, and remove this break just by commenting out line 4008. This would guarantee all tx work is cleaned at every e1000_clean Jesse -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Patch v2] Make PCI extended config space (MMCONFIG) a driver opt-in
On Friday, January 11, 2008 3:58 Ivan Kokshaysky wrote: > On Fri, Jan 11, 2008 at 02:38:03PM -0700, Matthew Wilcox wrote: > > On Fri, Jan 11, 2008 at 01:28:30PM -0800, Linus Torvalds wrote: > > > Hmm. Were all those reports root-caused to just that BAR probing? > > > If so, we may be in better shape than I worried. > > > > I believe so. > > Ditto. > > One typical problem is that on "Intel(r) 3 Series Experss Chipset > Family" MMCONFIG probing of the BAR #2 (frame buffer address) of > integrated graphics device locks up the machine (depending on BIOS > settings, of course). This happens because the frame buffer of IGD > has higher decode priority than MMCONFIG range, as stated in Intel > docs... Yeah, I'm only aware of 3: - the BAR overlapping w/MMCONFIG problem described above - ATI chipset config space retry bug - VIA (?) chipset host bridges don't respond well to having decode disabled (they stop decoding RAM addresses as well) That's it afaik, so I've never really known where Linus' paranoia comes from. OTOH I haven't been too keen to challenge it either; MMCONFIG space is only just beginning to be tested widely with the deployment of Vista, so we'll doubtless see more problems on older chipsets if we enable it by default. Jesse -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: MCFG ACPI patch in git-x86 causes boot regression
On Tuesday, January 08, 2008 8:27 Ingo Molnar wrote: > * Andi Kleen <[EMAIL PROTECTED]> wrote: > > > > http://www.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/ > > > >2.6.24-rc6/2.6.24-rc6-mm1/broken-out/pci-disable-decoding-during > > > >-sizing-of-bars.patch > > > > > > > > Without this change a bunch of machines fail to boot even > > > > without the MCFG change (since their MCFG was E820 reserved and > > > > so the old validation check passed). > > > > > > Andi, can you confirm that your box boots fine with that patch > > > applied? > > > > Yes it does. > > thanks for testing it. (and thanks for finding & reporting the > problem) I've added that patch to x86.git, right before: > > Subject: x86: validate against ACPI motherboard resources > > this should be the only dependency AFAICS. Yeah, that should work afaik, thanks for sorting this out. Jesse -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: MCFG ACPI patch in git-x86 causes boot regression
On Tuesday, January 08, 2008 8:27 Ingo Molnar wrote: * Andi Kleen [EMAIL PROTECTED] wrote: http://www.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/ 2.6.24-rc6/2.6.24-rc6-mm1/broken-out/pci-disable-decoding-during -sizing-of-bars.patch Without this change a bunch of machines fail to boot even without the MCFG change (since their MCFG was E820 reserved and so the old validation check passed). Andi, can you confirm that your box boots fine with that patch applied? Yes it does. thanks for testing it. (and thanks for finding reporting the problem) I've added that patch to x86.git, right before: Subject: x86: validate against ACPI motherboard resources this should be the only dependency AFAICS. Yeah, that should work afaik, thanks for sorting this out. Jesse -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: broken suspend, sometimes (drm related) [Was: 2.6.24-rc5-mm1]
> next suspend/resume try: > BLE drm_addmap_core a: map 81007c2d9b00, handle > BLE drm_addmap_core c: map 81007c2d9b00, handle c20010092000 > BLE drm_rmmap_locked b: map 81007c2d9b00, handle c20010092000 > BLE drm_addmap_core a: map 81007c2d9b00, handle c20010092000 > BLE drm_addmap_core c: map 81007c2d9b00, handle c20010092000 > BLE drm_rmmap_locked b: map 81007c2d9b00, handle c20010092000 > BLE drm_addmap_core a: map 81007c2d9b00, handle c20010092000 > BLE drm_addmap_core c: map 81007c2d9b00, handle c20010092000 > BLE drm_addmap_core a: map 81007c2d90c0, handle 8000 > BLE drm_addmap_ioctl a: map 81007c2d96c0, handle > BLE drm_addmap_core a: map 81007c2d90c0, handle > BLE drm_addmap_ioctl a: map 81007c2d96c0, handle > BLE drm_addmap_core a: map 81007c2d9800, handle > BLE drm_addmap_core b: map 81007c2d9800, handle c2001038 > BLE drm_addmap_ioctl a: map 81007c2d96c0, handle > BLE drm_addmap_core a: map 81007c2d9e80, handle 81007c2d9d40 > BLE drm_addmap_ioctl a: map 81007c2d96c0, handle > BLE drm_core_ioremap: map 81007d0ab050, handle c2001024 > set status page addr 0x00033000 > BLE drm_core_ioremap: map 81007d0ab098, handle c20010096000 > BLE drm_addmap_core a: map 81007bd7d440, handle 6632785c63766632 > BLE drm_addmap_ioctl a: map 81007bd7d080, handle > BLE drm_addmap_core a: map 81007bd7d700, handle 6632785c63766632 > BLE drm_addmap_ioctl a: map 81007bd7d080, handle > BLE drm_addmap_core a: map 81007c16f840, handle 81007c16f680 > BLE drm_addmap_ioctl a: map 81007bd7d080, handle > BLE drm_addmap_core a: map 81007c16f2c0, handle 81007cc33e10 > BLE drm_addmap_ioctl a: map 81007c16f640, handle > BLE drm_core_ioremapfree a: map 81007d0ab050, handle c2001024 > BLE drm_core_ioremapfree b: map 81007d0ab050, handle c2001024 > BLE drm_core_ioremapfree a: map 81007d0ab098, handle c20010096000 > BLE drm_core_ioremapfree b: map 81007d0ab098, handle c20010096000 > BLE drm_rmmap_locked a: map 81007c2d9800, handle c2001038 > BLE drm_rmmap_locked b: map 81007c2d9b00, handle c20010092000 > PM: Syncing filesystems ... done. > PM: Preparing system for mem sleep > Freezing user space processes ... (elapsed 0.00 seconds) done. > Freezing remaining freezable tasks ... (elapsed 0.00 seconds) done. > PM: Entering mem sleep > sd 2:0:0:0: [sdc] Synchronizing SCSI cache > sd 2:0:0:0: [sdc] Stopping disk > sd 1:0:0:0: [sdb] Synchronizing SCSI cache > sd 1:0:0:0: [sdb] Stopping disk > sd 0:0:0:0: [sda] Synchronizing SCSI cache > sd 0:0:0:0: [sda] Stopping disk > drm_sysfs_suspend > BAD BAD BAD 81007c2d9800 > suspend_device(): drm_sysfs_suspend+0x0/0x40() returns -5 > Could not suspend device card0: error -5 > sd 0:0:0:0: [sda] Starting disk > > I;m out of ideas, please give me a clue. This sounds a lot like a problem we had recently. The driver wasn't preserving its mappings across X startup/shutdown (drm open/close) and so you'd see crashes like this. It should be fixed already in DRM git. Jesse -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: broken suspend, sometimes (drm related) [Was: 2.6.24-rc5-mm1]
next suspend/resume try: BLE drm_addmap_core a: map 81007c2d9b00, handle BLE drm_addmap_core c: map 81007c2d9b00, handle c20010092000 BLE drm_rmmap_locked b: map 81007c2d9b00, handle c20010092000 BLE drm_addmap_core a: map 81007c2d9b00, handle c20010092000 BLE drm_addmap_core c: map 81007c2d9b00, handle c20010092000 BLE drm_rmmap_locked b: map 81007c2d9b00, handle c20010092000 BLE drm_addmap_core a: map 81007c2d9b00, handle c20010092000 BLE drm_addmap_core c: map 81007c2d9b00, handle c20010092000 BLE drm_addmap_core a: map 81007c2d90c0, handle 8000 BLE drm_addmap_ioctl a: map 81007c2d96c0, handle BLE drm_addmap_core a: map 81007c2d90c0, handle BLE drm_addmap_ioctl a: map 81007c2d96c0, handle BLE drm_addmap_core a: map 81007c2d9800, handle BLE drm_addmap_core b: map 81007c2d9800, handle c2001038 BLE drm_addmap_ioctl a: map 81007c2d96c0, handle BLE drm_addmap_core a: map 81007c2d9e80, handle 81007c2d9d40 BLE drm_addmap_ioctl a: map 81007c2d96c0, handle BLE drm_core_ioremap: map 81007d0ab050, handle c2001024 set status page addr 0x00033000 BLE drm_core_ioremap: map 81007d0ab098, handle c20010096000 BLE drm_addmap_core a: map 81007bd7d440, handle 6632785c63766632 BLE drm_addmap_ioctl a: map 81007bd7d080, handle BLE drm_addmap_core a: map 81007bd7d700, handle 6632785c63766632 BLE drm_addmap_ioctl a: map 81007bd7d080, handle BLE drm_addmap_core a: map 81007c16f840, handle 81007c16f680 BLE drm_addmap_ioctl a: map 81007bd7d080, handle BLE drm_addmap_core a: map 81007c16f2c0, handle 81007cc33e10 BLE drm_addmap_ioctl a: map 81007c16f640, handle BLE drm_core_ioremapfree a: map 81007d0ab050, handle c2001024 BLE drm_core_ioremapfree b: map 81007d0ab050, handle c2001024 BLE drm_core_ioremapfree a: map 81007d0ab098, handle c20010096000 BLE drm_core_ioremapfree b: map 81007d0ab098, handle c20010096000 BLE drm_rmmap_locked a: map 81007c2d9800, handle c2001038 BLE drm_rmmap_locked b: map 81007c2d9b00, handle c20010092000 PM: Syncing filesystems ... done. PM: Preparing system for mem sleep Freezing user space processes ... (elapsed 0.00 seconds) done. Freezing remaining freezable tasks ... (elapsed 0.00 seconds) done. PM: Entering mem sleep sd 2:0:0:0: [sdc] Synchronizing SCSI cache sd 2:0:0:0: [sdc] Stopping disk sd 1:0:0:0: [sdb] Synchronizing SCSI cache sd 1:0:0:0: [sdb] Stopping disk sd 0:0:0:0: [sda] Synchronizing SCSI cache sd 0:0:0:0: [sda] Stopping disk drm_sysfs_suspend BAD BAD BAD 81007c2d9800 suspend_device(): drm_sysfs_suspend+0x0/0x40() returns -5 Could not suspend device card0: error -5 sd 0:0:0:0: [sda] Starting disk I;m out of ideas, please give me a clue. This sounds a lot like a problem we had recently. The driver wasn't preserving its mappings across X startup/shutdown (drm open/close) and so you'd see crashes like this. It should be fixed already in DRM git. Jesse -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH 08/12] PAT 64b: coherent mmap and sysfs bin ioctl
On Thursday, December 13, 2007 3:55 pm [EMAIL PROTECTED] wrote: > Forward port of coherent-mmap.patch and sysfs-bin-ioctl.patch to x86 tree. > > TBD: Do we need the ioctl interface to sysfs or get the type attribute > through a different sysfs file. And then actually specify the attribute > while doing pci_mmap_page_range ;-) > > And when this interface is in place, X server has to use this interface for > WC mapping. I remember talking with people about using madvise and/or mmap flags to set the attributes correctly, but I don't remember whether it was a good idea or not... The advantage of a specific post-mmap call is that it would make setting the attribute types a little easier, so either ioctl or madvise seems preferable to mmapping over and over with different flags until you get the mapping. Jesse -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Possible issue with dangling PCI BARs
On Thursday, December 13, 2007 3:20 Benjamin Herrenschmidt wrote: > > > Supporting pci_enable_device_io / pci_enable_device_mmio / > > > pci_iomap_io / pci_iomap_mmio seems to cover pretty much all the > > > use cases we have. > > > > > > The users we have right now that are: > > > > > > - pata_cs5520 (can be dealt with easily) > > > - old IDE (with the new resource handling for > > > legacy IDE can use pci_enable_device_io I think, ditto > > > pci/cs5520) > > > - scx200_acb(looks like a simple substitution works) > > > - lpfc pci_enable_device_mmio > > > - qla2xxx pci_enable_device ? (enables IO and MMIO) > > I may have not fully undestood you in my previous reply. You are > proposing replacing pci_enable_device_bars() with a pair of > pci_enable_device_io/mem ? > > I think that would be a good idea indeed. Yeah, that seems like a reasonable compromise. Though in practice I'd expect the full disable decode approach to work fairly well too. I mean, if we really end up failing to allocate space for the device with the root drive on it, there are probably bigger issues than just failing to get a few bytes of I/O space for it... OTOH like Robert said, many devices really only need either MMIO or IO space enabled, not both, so having separate enable/disable routines for them makes a lot of sense. Jesse -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Possible issue with dangling PCI BARs
On Thursday, December 13, 2007 3:20 Benjamin Herrenschmidt wrote: Supporting pci_enable_device_io / pci_enable_device_mmio / pci_iomap_io / pci_iomap_mmio seems to cover pretty much all the use cases we have. The users we have right now that are: - pata_cs5520 (can be dealt with easily) - old IDE (with the new resource handling for legacy IDE can use pci_enable_device_io I think, ditto pci/cs5520) - scx200_acb(looks like a simple substitution works) - lpfc pci_enable_device_mmio - qla2xxx pci_enable_device ? (enables IO and MMIO) I may have not fully undestood you in my previous reply. You are proposing replacing pci_enable_device_bars() with a pair of pci_enable_device_io/mem ? I think that would be a good idea indeed. Yeah, that seems like a reasonable compromise. Though in practice I'd expect the full disable decode approach to work fairly well too. I mean, if we really end up failing to allocate space for the device with the root drive on it, there are probably bigger issues than just failing to get a few bytes of I/O space for it... OTOH like Robert said, many devices really only need either MMIO or IO space enabled, not both, so having separate enable/disable routines for them makes a lot of sense. Jesse -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH 08/12] PAT 64b: coherent mmap and sysfs bin ioctl
On Thursday, December 13, 2007 3:55 pm [EMAIL PROTECTED] wrote: Forward port of coherent-mmap.patch and sysfs-bin-ioctl.patch to x86 tree. TBD: Do we need the ioctl interface to sysfs or get the type attribute through a different sysfs file. And then actually specify the attribute while doing pci_mmap_page_range ;-) And when this interface is in place, X server has to use this interface for WC mapping. I remember talking with people about using madvise and/or mmap flags to set the attributes correctly, but I don't remember whether it was a good idea or not... The advantage of a specific post-mmap call is that it would make setting the attribute types a little easier, so either ioctl or madvise seems preferable to mmapping over and over with different flags until you get the mapping. Jesse -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH] [NET]: Fix Ooops of napi net_rx_action.
Joonwoo Park wrote: > 2007/12/12, Brandeburg, Jesse <[EMAIL PROTECTED]>: >> >> all drivers using NAPI in 2.6.24+ (NNAPI??) must return zero here, >> after calling netif_rx_complete. netif_rx_complete plus work_done >> != 0 causes a bug. >> > > Brandeburg, > Don't we need to return non-zero work_done after netif_rx_complete if > work_done != weight? Actually we just need to make sure we don't return work_done == weight as it is checked on line 2210 of dev.c. I should also note that I was wrong above, and that the requirement is that we MUST not return work_done == weight when indicating netif_rx_complete. So, returning 0 when we actually did some work, but are being removed from the poll list because something like !netif_running, is probably okay, but I'm sure someone will disagree with me. Maybe returning like this would be better diff --git a/drivers/net/e1000/e1000_main.c b/drivers/net/e1000/e1000_main.c index 724f067..76d5e3b 100644 --- a/drivers/net/e1000/e1000_main.c +++ b/drivers/net/e1000/e1000_main.c @@ -3933,6 +3933,10 @@ quit_polling: e1000_set_itr(adapter); netif_rx_complete(poll_dev, napi); e1000_irq_enable(adapter); + if (work_done == weight) + return work_done - 1; + else + return work_done; } return work_done; Jesse -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH] [NET]: Fix Ooops of napi net_rx_action.
Joonwoo Park wrote:
> /* If no Tx and not enough Rx work done, exit the polling mode */
> if ((!tx_cleaned && (work_done == 0)) ||
>!netif_running(poll_dev)) {
> quit_polling:
> if (likely(adapter->itr_setting & 3))
> e1000_set_itr(adapter);
> netif_rx_complete(poll_dev, napi);
> e1000_irq_enable(adapter);
all drivers using NAPI in 2.6.24+ (NNAPI??) must return zero here, after
calling netif_rx_complete. netif_rx_complete plus work_done != 0 causes
a bug.
> }
>
> return work_done;
> }
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH] [NET]: Fix Ooops of napi net_rx_action.
Joonwoo Park wrote:
/* If no Tx and not enough Rx work done, exit the polling mode */
if ((!tx_cleaned (work_done == 0)) ||
!netif_running(poll_dev)) {
quit_polling:
if (likely(adapter-itr_setting 3))
e1000_set_itr(adapter);
netif_rx_complete(poll_dev, napi);
e1000_irq_enable(adapter);
all drivers using NAPI in 2.6.24+ (NNAPI??) must return zero here, after
calling netif_rx_complete. netif_rx_complete plus work_done != 0 causes
a bug.
}
return work_done;
}
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH] [NET]: Fix Ooops of napi net_rx_action.
Joonwoo Park wrote: 2007/12/12, Brandeburg, Jesse [EMAIL PROTECTED]: all drivers using NAPI in 2.6.24+ (NNAPI??) must return zero here, after calling netif_rx_complete. netif_rx_complete plus work_done != 0 causes a bug. Brandeburg, Don't we need to return non-zero work_done after netif_rx_complete if work_done != weight? Actually we just need to make sure we don't return work_done == weight as it is checked on line 2210 of dev.c. I should also note that I was wrong above, and that the requirement is that we MUST not return work_done == weight when indicating netif_rx_complete. So, returning 0 when we actually did some work, but are being removed from the poll list because something like !netif_running, is probably okay, but I'm sure someone will disagree with me. Maybe returning like this would be better diff --git a/drivers/net/e1000/e1000_main.c b/drivers/net/e1000/e1000_main.c index 724f067..76d5e3b 100644 --- a/drivers/net/e1000/e1000_main.c +++ b/drivers/net/e1000/e1000_main.c @@ -3933,6 +3933,10 @@ quit_polling: e1000_set_itr(adapter); netif_rx_complete(poll_dev, napi); e1000_irq_enable(adapter); + if (work_done == weight) + return work_done - 1; + else + return work_done; } return work_done; Jesse -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [E1000-devel] [PATCH] e100: free IRQ to remove warning whenrebooting
Ian Wienand wrote: > Hi, > > When rebooting today I got > > Will now restart. > ACPI: PCI interrupt for device :00:03.0 disabled > GSI 20 (level, low) -> CPU 1 (0x0100) vector 53 unregistered > Destroying IRQ53 without calling free_irq > WARNING: at > /home/insecure/ianw/programs/git-kernel/linux-2.6/kernel/irq/chip.c:76 > dynamic_irq_cleanup() > > Call Trace: > [] show_stack+0x40/0xa0 > sp=e0407c927b40 > bsp=e0407c920eb8 [] dump_stack+0x30/0x60 > sp=e0407c927d10 > bsp=e0407c920ea0 [] > dynamic_irq_cleanup+0x160/0x1e0 > sp=e0407c927d10 bsp=e0407c920e70 [] > destroy_and_reserve_irq+0x30/0xc0 > sp=e0407c927d10 bsp=e0407c920e40 [] > iosapic_unregister_intr+0x5b0/0x5e0 > sp=e0407c927d10 bsp=e0407c920dd8 [] > acpi_unregister_gsi+0x30/0x60 > sp=e0407c927d10 bsp=e0407c920db8 [] > acpi_pci_irq_disable+0x140/0x160 > sp=e0407c927d10 bsp=e0407c920d88 [] > pcibios_disable_device+0xa0/0xc0 > sp=e0407c927d20 bsp=e0407c920d68 [] > pci_disable_device+0x130/0x160 > sp=e0407c927d20 bsp=e0407c920d38 [] > e100_shutdown+0x1c0/0x220 > sp=e0407c927d30 bsp=e0407c920d08 [] > pci_device_shutdown+0x80/0xc0 > sp=e0407c927d30 bsp=e0407c920ce8 [] > device_shutdown+0xf0/0x180 > sp=e0407c927d30 bsp=e0407c920cc8 [] > kernel_restart+0x60/0x120 > sp=e0407c927d30 bsp=e0407c920ca8 [] > sys_reboot+0x3b0/0x480 > sp=e0407c927d30 bsp=e0407c920c30 [] > ia64_ret_from_syscall+0x0/0x20 > sp=e0407c927e30 bsp=e0407c920c30 [] > ia64_ivt+0x00010620/0x400 > sp=e0407c928000 bsp=e0407c920c30 > Restarting system. > > I think the solution might be to free the IRQ before the > pci_device_shutdown I believe you are correct. Our other drivers appear to do this correctly. > Signed-off-by: Ian Wienand <[EMAIL PROTECTED]> Auke will add this and push it next week. Thanks! > > --- > > e100.c |1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/net/e100.c b/drivers/net/e100.c > index 3dbaec6..8ae5ac3 100644 > --- a/drivers/net/e100.c > +++ b/drivers/net/e100.c > @@ -2782,6 +2782,7 @@ static void e100_shutdown(struct pci_dev *pdev) > pci_enable_wake(pdev, PCI_D3cold, 0); > } > > + free_irq(pdev->irq, netdev); > pci_disable_device(pdev); > pci_set_power_state(pdev, PCI_D3hot); > } - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [E1000-devel] [PATCH] e100: free IRQ to remove warning whenrebooting
forwarding whole message for netdev to review Ian Wienand wrote: Hi, When rebooting today I got Will now restart. ACPI: PCI interrupt for device :00:03.0 disabled GSI 20 (level, low) - CPU 1 (0x0100) vector 53 unregistered Destroying IRQ53 without calling free_irq WARNING: at /home/insecure/ianw/programs/git-kernel/linux-2.6/kernel/irq/chip.c:76 dynamic_irq_cleanup() Call Trace: [a00100014340] show_stack+0x40/0xa0 sp=e0407c927b40 bsp=e0407c920eb8 [a001000143d0] dump_stack+0x30/0x60 sp=e0407c927d10 bsp=e0407c920ea0 [a001000e58e0] dynamic_irq_cleanup+0x160/0x1e0 sp=e0407c927d10 bsp=e0407c920e70 [a001000106b0] destroy_and_reserve_irq+0x30/0xc0 sp=e0407c927d10 bsp=e0407c920e40 [a001000508f0] iosapic_unregister_intr+0x5b0/0x5e0 sp=e0407c927d10 bsp=e0407c920dd8 [a001aa70] acpi_unregister_gsi+0x30/0x60 sp=e0407c927d10 bsp=e0407c920db8 [a0010042e300] acpi_pci_irq_disable+0x140/0x160 sp=e0407c927d10 bsp=e0407c920d88 [a00100774200] pcibios_disable_device+0xa0/0xc0 sp=e0407c927d20 bsp=e0407c920d68 [a001003778d0] pci_disable_device+0x130/0x160 sp=e0407c927d20 bsp=e0407c920d38 [a00100525d20] e100_shutdown+0x1c0/0x220 sp=e0407c927d30 bsp=e0407c920d08 [a0010037d0e0] pci_device_shutdown+0x80/0xc0 sp=e0407c927d30 bsp=e0407c920ce8 [a001004ecb70] device_shutdown+0xf0/0x180 sp=e0407c927d30 bsp=e0407c920cc8 [a001000ac4e0] kernel_restart+0x60/0x120 sp=e0407c927d30 bsp=e0407c920ca8 [a001000ac990] sys_reboot+0x3b0/0x480 sp=e0407c927d30 bsp=e0407c920c30 [a001b4e0] ia64_ret_from_syscall+0x0/0x20 sp=e0407c927e30 bsp=e0407c920c30 [a0010620] ia64_ivt+0x00010620/0x400 sp=e0407c928000 bsp=e0407c920c30 Restarting system. I think the solution might be to free the IRQ before the pci_device_shutdown I believe you are correct. Our other drivers appear to do this correctly. Signed-off-by: Ian Wienand [EMAIL PROTECTED] Auke will add this and push it next week. Thanks! --- e100.c |1 + 1 file changed, 1 insertion(+) diff --git a/drivers/net/e100.c b/drivers/net/e100.c index 3dbaec6..8ae5ac3 100644 --- a/drivers/net/e100.c +++ b/drivers/net/e100.c @@ -2782,6 +2782,7 @@ static void e100_shutdown(struct pci_dev *pdev) pci_enable_wake(pdev, PCI_D3cold, 0); } + free_irq(pdev-irq, netdev); pci_disable_device(pdev); pci_set_power_state(pdev, PCI_D3hot); } - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: x86_64 ten times slower than i386
On Monday, November 05, 2007 4:26 Andi Kleen wrote: > On Mon, Nov 05, 2007 at 08:32:24AM -0800, Ray Lee wrote: > > (Don't trim cc:s.) > > > > On Nov 5, 2007 8:00 AM, Bo Brantén <[EMAIL PROTECTED]> wrote: > > >> Intel Core 2 Quad > > >> and I noticed that the 64-bit versions was at least 10 times > > >> slower than the 32-bit versions, > > > > > > After I uppgraded the BIOS the mtrr looks like below, and now it > > > works if I boot with mem=4736M so I can use all memory but it > > > still doesn't work without the mem parameter then it will run as > > > slow as before. > > Then the BIOS is still broken Comapl in to your motherboard vendor. > > > > reg00: base=0x ( 0MB), size=2048MB: write-back, count=1 > > > reg01: base=0x8000 (2048MB), size=1024MB: write-back, count=1 > > > reg02: base=0xc000 (3072MB), size= 256MB: write-back, count=1 > > > reg03: base=0xcf80 (3320MB), size= 8MB: uncachable, count=1 > > > reg04: base=0xcf70 (3319MB), size= 1MB: uncachable, count=1 > > > reg05: base=0x1 (4096MB), size= 512MB: write-back, > > > count=1 reg06: base=0x12000 (4608MB), size= 128MB: > > > write-back, count=1 > > > > Jesse Barnes (cc:d) wrote a patch to address this, I think (x86: > > trim memory not covered by WB MTRRs), but as far as I can tell it > > hasn't been merged yet. System is Intel, 4gb of RAM. > > It wasn't merged because it broke booting on some systems. > Besides the memory would be still lost -- all it did was to automate > the "mem=" line. Andi, do you have any details on which system broke and how? I haven't heard back from you on my last message on the subject... the patch was in -mm for awhile with no complaints. Ultimately, this is a broken BIOS issue, but still, it would be nice if the kernel handled it better. Thanks, Jesse - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: x86_64 ten times slower than i386
On Monday, November 05, 2007 4:26 Andi Kleen wrote: On Mon, Nov 05, 2007 at 08:32:24AM -0800, Ray Lee wrote: (Don't trim cc:s.) On Nov 5, 2007 8:00 AM, Bo Brantén [EMAIL PROTECTED] wrote: Intel Core 2 Quad and I noticed that the 64-bit versions was at least 10 times slower than the 32-bit versions, After I uppgraded the BIOS the mtrr looks like below, and now it works if I boot with mem=4736M so I can use all memory but it still doesn't work without the mem parameter then it will run as slow as before. Then the BIOS is still broken Comapl in to your motherboard vendor. reg00: base=0x ( 0MB), size=2048MB: write-back, count=1 reg01: base=0x8000 (2048MB), size=1024MB: write-back, count=1 reg02: base=0xc000 (3072MB), size= 256MB: write-back, count=1 reg03: base=0xcf80 (3320MB), size= 8MB: uncachable, count=1 reg04: base=0xcf70 (3319MB), size= 1MB: uncachable, count=1 reg05: base=0x1 (4096MB), size= 512MB: write-back, count=1 reg06: base=0x12000 (4608MB), size= 128MB: write-back, count=1 Jesse Barnes (cc:d) wrote a patch to address this, I think (x86: trim memory not covered by WB MTRRs), but as far as I can tell it hasn't been merged yet. System is Intel, 4gb of RAM. It wasn't merged because it broke booting on some systems. Besides the memory would be still lost -- all it did was to automate the mem= line. Andi, do you have any details on which system broke and how? I haven't heard back from you on my last message on the subject... the patch was in -mm for awhile with no complaints. Ultimately, this is a broken BIOS issue, but still, it would be nice if the kernel handled it better. Thanks, Jesse - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: pci-disable-decode-of-io-memory-during-bar-sizing.patch
On Tuesday, October 30, 2007 4:59 pm Linus Torvalds wrote: > Also, there are several devices that don't show up in the MMCFG > things, or just otherwise get it wrong. > > So just take a look at arch/x86/pci/mmconfig-shared.c and look for > "conf1". > > Really. Damn, I'm nervous taking any MMCFG patches that has you as an > author, if you aren't even aware of these kinds of fundamnetal > issues. You probably read the standards about how things are > "supposed" to work, and then just believed them? We have a few bits of code in there to look at the actual MMCONFIG base register, which is good since we can sanity check it against the firmware. However, by itself it's not enough since we may end up using it even though the BIOS didn't tell us about an overlap that really does exist. Robert recognized this and added checks to make sure the values read from the base regs actually matched what the firmware was telling us in the ACPI tables (at least iirc, it's been awhile since I looked at the patch). So don't be too nervous. :) Jesse - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: pci-disable-decode-of-io-memory-during-bar-sizing.patch
On Tuesday, October 30, 2007 3:38 pm Linus Torvalds wrote: > On Tue, 30 Oct 2007, Linus Torvalds wrote: > > No. You really don't see the big picture. There's been tons of > > problems with MMCONFIG. Like the fact that other devices have their > > IO regions registered on top of it, because the MMCONFIG thing was > > done as a hidden resource. Or the fact that the area claimed was > > too small. Or too large. Or not listed at all. > > Actually, I guess the bad case wasn't "not listed at all", but > incorrectly listed - so the probing would go to the wrong address, > not find any devices, and then promptly result in an unusable machine > with no hardware attached. Yeah, busted BIOS (and I agree a poor design decision). > I _think_ (and hope) those machines were never released. But even > now, on my main machine, I get "MCFG area at f000 is not > E820-reserved", and probably the only reason the PCI layer doesn't > overwrite it is because it does show up as a PnP region, and I have > pnp support enabled. Unfortunately I think some such machines *were* released, only because the release engineers figured no one actually uses MMCONFIG yet (Windows == whole world of users), so why worry about that particular bug? The "not E820-reserved" message is actually bogus. I'll bet MMCONFIG works fine on your machine with Robert's patch and the disable decode stuff applied. Jesse - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: pci-disable-decode-of-io-memory-during-bar-sizing.patch
On Tuesday, October 30, 2007 3:22 pm Linus Torvalds wrote: > On Tue, 30 Oct 2007, Jesse Barnes wrote: > > The per-device flag is fine with me, but I should make something > > clear: > > > > MMCONFIG IS NOT BROKEN! > > Trust me, it is. > > The particular problem _you_ had with it is only a small small part > of the bugs we have had. > > > What's broken is our PCI probing with certain address space layouts > > that include MMCONFIG space. > > No. You really don't see the big picture. There's been tons of > problems with MMCONFIG. Like the fact that other devices have their > IO regions registered on top of it, because the MMCONFIG thing was > done as a hidden resource. Or the fact that the area claimed was too > small. Or too large. Or not listed at all. Yeah, that's definitely a problem, and would be a firmware bug. There's no doubt that firmwares have had trouble with this in the past, but given that Vista now relies on this stuff working, it's a lot more likely to be reliable in current and future systems. > The whole thing is a total disaster. I told Intel engineers literally > *years* ago to not do that idiotic "hidden IO resources that are > described by firmware that then inevitably gets things wrong", and > yet what happens? Every single time. I don't disagree there. I'm just saying the actual mechanism is fine (as illustrated by the numerous non-PC ports of Linux), and this particular problem, at least, isn't really specific to how MMCONFIG is described or configured by the firmware and OS, it's simply a Linux problem. But like I said, the per-device flag Arjan suggested is fine with me... Jesse - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: pci-disable-decode-of-io-memory-during-bar-sizing.patch
On Tuesday, October 30, 2007 10:07 am Linus Torvalds wrote: > (Where "screwed up badly" is the usual "left it to firmware people" > thing, of course. Dammit, Intel *could* have just made it a real PCI > BAR in the Northbridge, and specified it as such, and we wouldn't > have these problems! But no, it had to be another idiotic "firmware > tells where it is" thing) The per-device flag is fine with me, but I should make something clear: MMCONFIG IS NOT BROKEN! Nor is finding mmconfig space. We know how to do that too. What's broken is our PCI probing with certain address space layouts that include MMCONFIG space. Since MMCONFIG is in MMIO space (by definition) there will always be the potential for problems when we use MMCONFIG and don't disable decode while sizing BARs (probably a latent bug on many non-PC Linux platforms). So we can either use I/O ports for sizing and only switch to MMCONFIG later, or we can just use it on an as-needed basis, or we can make our PCI probing safe if MMCONFIG is in use. Jesse - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: pci-disable-decode-of-io-memory-during-bar-sizing.patch
On Tuesday, October 30, 2007 10:07 am Linus Torvalds wrote: (Where screwed up badly is the usual left it to firmware people thing, of course. Dammit, Intel *could* have just made it a real PCI BAR in the Northbridge, and specified it as such, and we wouldn't have these problems! But no, it had to be another idiotic firmware tells where it is thing) The per-device flag is fine with me, but I should make something clear: MMCONFIG IS NOT BROKEN! Nor is finding mmconfig space. We know how to do that too. What's broken is our PCI probing with certain address space layouts that include MMCONFIG space. Since MMCONFIG is in MMIO space (by definition) there will always be the potential for problems when we use MMCONFIG and don't disable decode while sizing BARs (probably a latent bug on many non-PC Linux platforms). So we can either use I/O ports for sizing and only switch to MMCONFIG later, or we can just use it on an as-needed basis, or we can make our PCI probing safe if MMCONFIG is in use. Jesse - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: pci-disable-decode-of-io-memory-during-bar-sizing.patch
On Tuesday, October 30, 2007 3:22 pm Linus Torvalds wrote: On Tue, 30 Oct 2007, Jesse Barnes wrote: The per-device flag is fine with me, but I should make something clear: MMCONFIG IS NOT BROKEN! Trust me, it is. The particular problem _you_ had with it is only a small small part of the bugs we have had. What's broken is our PCI probing with certain address space layouts that include MMCONFIG space. No. You really don't see the big picture. There's been tons of problems with MMCONFIG. Like the fact that other devices have their IO regions registered on top of it, because the MMCONFIG thing was done as a hidden resource. Or the fact that the area claimed was too small. Or too large. Or not listed at all. Yeah, that's definitely a problem, and would be a firmware bug. There's no doubt that firmwares have had trouble with this in the past, but given that Vista now relies on this stuff working, it's a lot more likely to be reliable in current and future systems. The whole thing is a total disaster. I told Intel engineers literally *years* ago to not do that idiotic hidden IO resources that are described by firmware that then inevitably gets things wrong, and yet what happens? Every single time. I don't disagree there. I'm just saying the actual mechanism is fine (as illustrated by the numerous non-PC ports of Linux), and this particular problem, at least, isn't really specific to how MMCONFIG is described or configured by the firmware and OS, it's simply a Linux problem. But like I said, the per-device flag Arjan suggested is fine with me... Jesse - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: pci-disable-decode-of-io-memory-during-bar-sizing.patch
On Tuesday, October 30, 2007 3:38 pm Linus Torvalds wrote: On Tue, 30 Oct 2007, Linus Torvalds wrote: No. You really don't see the big picture. There's been tons of problems with MMCONFIG. Like the fact that other devices have their IO regions registered on top of it, because the MMCONFIG thing was done as a hidden resource. Or the fact that the area claimed was too small. Or too large. Or not listed at all. Actually, I guess the bad case wasn't not listed at all, but incorrectly listed - so the probing would go to the wrong address, not find any devices, and then promptly result in an unusable machine with no hardware attached. Yeah, busted BIOS (and I agree a poor design decision). I _think_ (and hope) those machines were never released. But even now, on my main machine, I get MCFG area at f000 is not E820-reserved, and probably the only reason the PCI layer doesn't overwrite it is because it does show up as a PnP region, and I have pnp support enabled. Unfortunately I think some such machines *were* released, only because the release engineers figured no one actually uses MMCONFIG yet (Windows == whole world of users), so why worry about that particular bug? The not E820-reserved message is actually bogus. I'll bet MMCONFIG works fine on your machine with Robert's patch and the disable decode stuff applied. Jesse - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: pci-disable-decode-of-io-memory-during-bar-sizing.patch
On Tuesday, October 30, 2007 4:59 pm Linus Torvalds wrote: Also, there are several devices that don't show up in the MMCFG things, or just otherwise get it wrong. So just take a look at arch/x86/pci/mmconfig-shared.c and look for conf1. Really. Damn, I'm nervous taking any MMCFG patches that has you as an author, if you aren't even aware of these kinds of fundamnetal issues. You probably read the standards about how things are supposed to work, and then just believed them? We have a few bits of code in there to look at the actual MMCONFIG base register, which is good since we can sanity check it against the firmware. However, by itself it's not enough since we may end up using it even though the BIOS didn't tell us about an overlap that really does exist. Robert recognized this and added checks to make sure the values read from the base regs actually matched what the firmware was telling us in the ACPI tables (at least iirc, it's been awhile since I looked at the patch). So don't be too nervous. :) Jesse - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC] AGP initial support for chipset flushing..
On Monday, October 29, 2007 1:12 pm Keith Packard wrote: > On Mon, 2007-10-29 at 12:47 -0700, Jesse Barnes wrote: > > In this case, we're performing basically a > > dma_sync*(...DMA_TO_DEVICE) right? > > But this is just for the GPU; every other DMA device in the system is > cache-coherent. Right. > > Can we be sure that a single flush is sufficient? Is there any > > window between when we flush and when we start accessing memory > > with the device that we could get into more caching trouble? > > An uncached write to this page will not complete until the buffers > are completely flushed. Yeah, so we should be safe. > > Looks reasonable, I'm not sure we can do much better. The only > > concern I have is that allocating some more PCI space like that may > > end up clobbering some *other* hidden BIOS mapping, but there's not > > a whole lot we can do about that. > > This isn't a hidden mapping; the i965 doesn't allocate space for it > in the BIOS. I know, that's what I'm worried about. If the BIOS is broken enough to not allocate MMIO space for the flush page, it may also be broken enough that our hand crafted MMIO space allocation will end up conflicting with some unreported BIOS area, which would be bad. Jesse - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC] AGP initial support for chipset flushing..
On Monday, October 29, 2007 12:52 pm Dave Airlie wrote: > > In this case, we're performing basically a > > dma_sync*(...DMA_TO_DEVICE) right? Can we be sure that a single > > flush is sufficient? Is there any window between when we flush and > > when we start accessing memory with the device that we could get > > into more caching trouble? > > Not that I can think off, but I don't work for the company who > screwed up the coherency :-), and I don't have the docs, so please > investigate for me ;-) It *looks* like it'll be enough. I assume Keith has talked to the chipset guys to confirm this. > > Looks reasonable, I'm not sure we can do much better. The only > > concern I have is that allocating some more PCI space like that may > > end up clobbering some *other* hidden BIOS mapping, but there's not > > a whole lot we can do about that. > > Again I'm trying to workaround broken BIOS.. nothing I can do. Right, BIOSes are so much fun to deal with. One other thing: it looks like the flush mmio space has to be allocated above the top of DRAM but below 4G. I wonder if there's an easy way to guarantee this with the pci_bus* routines... Jesse - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC] AGP initial support for chipset flushing..
On Monday, October 29, 2007 1:15 am Dave Airlie wrote: > Hi, > > We've uncovered a need when using the new memory manager to flush the > chipset global write buffers on certain intel chipset due to a lack > of coherency.. > > The attached patches add a new AGP interface for this purpose and > implements this in the Intel AGP driver. This stuff is based of some > guesswork in the 915 case from comments in the documentation :). In this case, we're performing basically a dma_sync*(...DMA_TO_DEVICE) right? Can we be sure that a single flush is sufficient? Is there any window between when we flush and when we start accessing memory with the device that we could get into more caching trouble? > Unfortuantely the 965 BIOS doesn't set this stuff up properly and it > doesn't use a standard BAR address, so I have to do it by hand, I'd > appreciate any commentary particularly in the setting up of the > resource stuff. Looks reasonable, I'm not sure we can do much better. The only concern I have is that allocating some more PCI space like that may end up clobbering some *other* hidden BIOS mapping, but there's not a whole lot we can do about that. Jesse - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC] AGP initial support for chipset flushing..
On Monday, October 29, 2007 1:12 pm Keith Packard wrote: On Mon, 2007-10-29 at 12:47 -0700, Jesse Barnes wrote: In this case, we're performing basically a dma_sync*(...DMA_TO_DEVICE) right? But this is just for the GPU; every other DMA device in the system is cache-coherent. Right. Can we be sure that a single flush is sufficient? Is there any window between when we flush and when we start accessing memory with the device that we could get into more caching trouble? An uncached write to this page will not complete until the buffers are completely flushed. Yeah, so we should be safe. Looks reasonable, I'm not sure we can do much better. The only concern I have is that allocating some more PCI space like that may end up clobbering some *other* hidden BIOS mapping, but there's not a whole lot we can do about that. This isn't a hidden mapping; the i965 doesn't allocate space for it in the BIOS. I know, that's what I'm worried about. If the BIOS is broken enough to not allocate MMIO space for the flush page, it may also be broken enough that our hand crafted MMIO space allocation will end up conflicting with some unreported BIOS area, which would be bad. Jesse - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC] AGP initial support for chipset flushing..
On Monday, October 29, 2007 12:52 pm Dave Airlie wrote: In this case, we're performing basically a dma_sync*(...DMA_TO_DEVICE) right? Can we be sure that a single flush is sufficient? Is there any window between when we flush and when we start accessing memory with the device that we could get into more caching trouble? Not that I can think off, but I don't work for the company who screwed up the coherency :-), and I don't have the docs, so please investigate for me ;-) It *looks* like it'll be enough. I assume Keith has talked to the chipset guys to confirm this. Looks reasonable, I'm not sure we can do much better. The only concern I have is that allocating some more PCI space like that may end up clobbering some *other* hidden BIOS mapping, but there's not a whole lot we can do about that. Again I'm trying to workaround broken BIOS.. nothing I can do. Right, BIOSes are so much fun to deal with. One other thing: it looks like the flush mmio space has to be allocated above the top of DRAM but below 4G. I wonder if there's an easy way to guarantee this with the pci_bus* routines... Jesse - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC] AGP initial support for chipset flushing..
On Monday, October 29, 2007 1:15 am Dave Airlie wrote: Hi, We've uncovered a need when using the new memory manager to flush the chipset global write buffers on certain intel chipset due to a lack of coherency.. The attached patches add a new AGP interface for this purpose and implements this in the Intel AGP driver. This stuff is based of some guesswork in the 915 case from comments in the documentation :). In this case, we're performing basically a dma_sync*(...DMA_TO_DEVICE) right? Can we be sure that a single flush is sufficient? Is there any window between when we flush and when we start accessing memory with the device that we could get into more caching trouble? Unfortuantely the 965 BIOS doesn't set this stuff up properly and it doesn't use a standard BAR address, so I have to do it by hand, I'd appreciate any commentary particularly in the setting up of the resource stuff. Looks reasonable, I'm not sure we can do much better. The only concern I have is that allocating some more PCI space like that may end up clobbering some *other* hidden BIOS mapping, but there's not a whole lot we can do about that. Jesse - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: fixing up DRM device model usage
On Friday, October 26, 2007 12:08 pm Kay Sievers wrote:
> > How does this conversion look?
>
> Seems fine, at a first look. You moved the device structure into the
> object where it belongs, instead of allocating one, and saving the
> pointer. You should really considering changing the core to do the
> free()ing of your object with the embedded devices release function,
> that is called when the last reference to the object is gone, instead
> of "hoping the best". :) But if the drm core does that properly, it
> might work, sure.
Yeah, but that'll require other changes to the DRM. Maybe I can tackle
that as part of the refactoring I'm doing for some other work.
> The open coded: device_create_file(>dev, _attrs[i])
> should probably replaced by passing the array to the class, and the
> core will do that for you.
You mean just set drm_class->dev_attrs = device_attrs? I didn't see in
the core device model code where that would create the files...
> Do you assign the dev_t: MKDEV(DRM_MAJOR, head->minor) somewhere? You
> need to put it in dev->devt, if you want a device node created by
> userspace.
Yeah, that's part of the drm_head structure. I'll go ahead and assign
it to dev->devt too.
> > It retains directory compatibility with
> > the old scheme, but I'm not sure about the dev->dev.parent value.
>
> You should use the same value as the old code:
> &(head->dev->pdev)->dev
> and assign it as the parent, seems right..
>
> > I'm using the PCI device corresponding to the DRM device as the
> > parent, but maybe I don't need one at all?
>
> Keep it, you want to express the relationship in sysfs, so that a
> "device" link is created, or that the device directory lives as a
> child below the parent device. Seems fine so far.
Ok, sounds good.
>
> > Dave, the drm_head stuff is a bit funky; it seems like a partially
> > implemented feature? I wonder if we should rip that out too, just
> > to keep things simple...
>
> Hehe, that's always a solution. :)
Yeah, removing code is nearly always a win. :)
Thanks,
Jesse
diff --git a/linux-core/drmP.h b/linux-core/drmP.h
index d0ab2c9..82a3a23 100644
--- a/linux-core/drmP.h
+++ b/linux-core/drmP.h
@@ -619,6 +619,8 @@ struct drm_driver {
void (*postclose) (struct drm_device *, struct drm_file *);
void (*lastclose) (struct drm_device *);
int (*unload) (struct drm_device *);
+ int (*suspend) (struct drm_device *);
+ int (*resume) (struct drm_device *);
int (*dma_ioctl) (struct drm_device *dev, void *data, struct drm_file
*file_priv);
void (*dma_ready) (struct drm_device *);
int (*dma_quiescent) (struct drm_device *);
@@ -697,6 +699,7 @@ struct drm_head {
* may contain multiple heads.
*/
struct drm_device {
+ struct device dev; /**< Linux device */
char *unique; /**< Unique identifier: e.g., busid */
int unique_len; /**< Length of unique field */
char *devname; /**< For /proc/interrupts */
@@ -1163,10 +1166,9 @@ extern void drm_pci_free(struct drm_device *dev,
drm_dma_handle_t *dmah);
/* sysfs support (drm_sysfs.c) */
struct drm_sysfs_class;
extern struct class *drm_sysfs_create(struct module *owner, char *name);
-extern void drm_sysfs_destroy(struct class *cs);
-extern struct class_device *drm_sysfs_device_add(struct class *cs,
-struct drm_head * head);
-extern void drm_sysfs_device_remove(struct class_device *class_dev);
+extern void drm_sysfs_destroy(void);
+extern int drm_sysfs_device_add(struct drm_device *dev, struct drm_head *
head);
+extern void drm_sysfs_device_remove(struct drm_device *dev);
/*
* Basic memory manager support (drm_mm.c)
diff --git a/linux-core/drm_drv.c b/linux-core/drm_drv.c
index fe2b120..47d1765 100644
--- a/linux-core/drm_drv.c
+++ b/linux-core/drm_drv.c
@@ -519,7 +519,7 @@ static int __init drm_core_init(void)
CORE_MAJOR, CORE_MINOR, CORE_PATCHLEVEL, CORE_DATE);
return 0;
err_p3:
- drm_sysfs_destroy(drm_class);
+ drm_sysfs_destroy();
err_p2:
unregister_chrdev(DRM_MAJOR, "drm");
drm_free(drm_heads, sizeof(*drm_heads) * drm_cards_limit, DRM_MEM_STUB);
@@ -530,7 +530,7 @@ err_p1:
static void __exit drm_core_exit(void)
{
remove_proc_entry("dri", NULL);
- drm_sysfs_destroy(drm_class);
+ drm_sysfs_destroy();
unregister_chrdev(DRM_MAJOR, "drm");
diff --git a/linux-core/drm_stub.c b/linux-core/drm_stub.c
index 9e140ac..1d88d37 100644
--- a/linux-core/drm_stub.c
+++ b/linux-core/drm_stub.c
@@ -183,11 +183,10 @@ static int drm_get_head(struct drm_device * dev, struct
dr
Re: fixing up DRM device model usage
On Friday, October 26, 2007 10:10 am Kay Sievers wrote:
> On 10/26/07, Jesse Barnes <[EMAIL PROTECTED]> wrote:
> > On Thursday, October 25, 2007 9:59 pm Greg KH wrote:
> > > On Thu, Oct 25, 2007 at 04:53:18PM -0700, Jesse Barnes wrote:
> > > > Ok, here's yet another version that uses the device model for
> > > > the suspend/resume, rather than pci hooks.
> > > >
> > > > Greg, DRM desperately needs review of its device model usage,
> > > > can you take a look at this patch and the current drm_sysfs.c
> > > > code? Right now, we're mixing class_devices and regular devices
> > > > (the latter seem to be required for suspend/resume to work
> > > > correctly), but this seems wrong. Any ideas? Should we just
> > > > rip out the class_device stuff and create full-on DRM device
> > > > nodes?
> > >
> > > The class_device stuff is already ripped out in the latest -mm
> > > trees and I will be forwarding that change on for 2.6.25 after
> > > 2.6.24 is out. So yes, it should be taken away :)
> > >
> > > But converting from class_device to struct device does not mean
> > > you use a "device node". But you could if you want to :)
> >
> > Yeah, bad choice of words. :)
> >
> > To retain compatibility, we need to have directories under the DRM
> > class dir (/sys/class/drm) for each card (e.g. card0) that contains
> > a file describing which graphics driver is bound to the device.
> > For class devices, we could just add an attributes structure to the
> > device. Can we do the same with regular, non-class devices?
>
> The conversion is already queued in Greg's tree, and in -mm:
> http://git.kernel.org/?p=linux/kernel/git/gregkh/patches.git;a=blob;f
>=driver/drm-convert-from-class_device-to-device-in-drivers-char-drm.pa
>tch;h=f993183d1cb017f981cc2232d17930af40459bd8;hb=HEAD
>
> /sys/class/drm will look the same as with the class_device's, only if
> !CONFIG_SYSFS_DEPRECATED, there will be symlinks instead of
> directories, otherwise the same pathes, like for all other
> (converted) classes too.
How does this conversion look? It retains directory compatibility with
the old scheme, but I'm not sure about the dev->dev.parent value. I'm
using the PCI device corresponding to the DRM device as the parent, but
maybe I don't need one at all?
Dave, the drm_head stuff is a bit funky; it seems like a partially
implemented feature? I wonder if we should rip that out too, just to
keep things simple...
Thanks,
Jesse
diff --git a/linux-core/Kconfig b/linux-core/Kconfig
diff --git a/linux-core/drmP.h b/linux-core/drmP.h
index d0ab2c9..82a3a23 100644
--- a/linux-core/drmP.h
+++ b/linux-core/drmP.h
@@ -619,6 +619,8 @@ struct drm_driver {
void (*postclose) (struct drm_device *, struct drm_file *);
void (*lastclose) (struct drm_device *);
int (*unload) (struct drm_device *);
+ int (*suspend) (struct drm_device *);
+ int (*resume) (struct drm_device *);
int (*dma_ioctl) (struct drm_device *dev, void *data, struct drm_file
*file_priv);
void (*dma_ready) (struct drm_device *);
int (*dma_quiescent) (struct drm_device *);
@@ -697,6 +699,7 @@ struct drm_head {
* may contain multiple heads.
*/
struct drm_device {
+ struct device dev; /**< Linux device */
char *unique; /**< Unique identifier: e.g., busid */
int unique_len; /**< Length of unique field */
char *devname; /**< For /proc/interrupts */
@@ -1163,10 +1166,9 @@ extern void drm_pci_free(struct drm_device *dev,
drm_dma_handle_t *dmah);
/* sysfs support (drm_sysfs.c) */
struct drm_sysfs_class;
extern struct class *drm_sysfs_create(struct module *owner, char *name);
-extern void drm_sysfs_destroy(struct class *cs);
-extern struct class_device *drm_sysfs_device_add(struct class *cs,
-struct drm_head * head);
-extern void drm_sysfs_device_remove(struct class_device *class_dev);
+extern void drm_sysfs_destroy(void);
+extern int drm_sysfs_device_add(struct drm_device *dev, struct drm_head *
head);
+extern void drm_sysfs_device_remove(struct drm_device *dev);
/*
* Basic memory manager support (drm_mm.c)
diff --git a/linux-core/drm_drv.c b/linux-core/drm_drv.c
index fe2b120..47d1765 100644
--- a/linux-core/drm_drv.c
+++ b/linux-core/drm_drv.c
@@ -519,7 +519,7 @@ static int __init drm_core_init(void)
CORE_MAJOR, CORE_MINOR, CORE_PATCHLEVEL, CORE_DATE);
return 0;
err_p3:
- drm_sysfs_destroy(drm_class);
+ drm_sysfs_destroy();
err_p2:
unregister_chrdev(DRM_MAJOR, "drm");
drm
Re: [RFC] full suspend/resume support for i915 DRM driver
On Friday, October 26, 2007 10:10 am Kay Sievers wrote: > The conversion is already queued in Greg's tree, and in -mm: > http://git.kernel.org/?p=linux/kernel/git/gregkh/patches.git;a=blob;f >=driver/drm-convert-from-class_device-to-device-in-drivers-char-drm.pa >tch;h=f993183d1cb017f981cc2232d17930af40459bd8;hb=HEAD Hm, I've done it slightly differently, by adding an actual device to the drm_device structure... I'll post it once I've cleaned it up a little more. > /sys/class/drm will look the same as with the class_device's, only if > !CONFIG_SYSFS_DEPRECATED, there will be symlinks instead of > directories, otherwise the same pathes, like for all other > (converted) classes too. Ok, thanks. Jesse - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: pci-disable-decode-of-io-memory-during-bar-sizing.patch
On Thursday, October 25, 2007 7:54 pm Greg KH wrote: > On Thu, Oct 25, 2007 at 04:22:35PM -0700, Jesse Barnes wrote: > > I think Greg doesn't like it, even though we don't have an > > alternative at this point... > > Yes, I didn't like it, Ivan didn't like it, and I got reports that it > wasn't even needed at all once you upgraded your BIOS to the latest > version. > > So, is this still needed? And if so, can you try to implement what > Ivan suggested to do here instead? Yes, it's still needed. Auke rescinded his "BIOS upgrade makes it work" message, so something like this is still necessary. Jesse - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: - mmconfig-validate-against-acpi-motherboard-resources.patch removed from -mm tree
On Thursday, October 25, 2007 10:27 pm Greg KH wrote: > On Thu, Oct 25, 2007 at 11:03:51PM -0600, Robert Hancock wrote: > > Greg KH wrote: > >> On Thu, Oct 25, 2007 at 04:22:35PM -0700, Jesse Barnes wrote: > >>> I think Greg doesn't like it, even though we don't have an > >>> alternative at this point... > >> > >> Yes, I didn't like it, Ivan didn't like it, and I got reports that > >> it wasn't even needed at all once you upgraded your BIOS to the > >> latest version. > >> So, is this still needed? And if so, can you try to implement > >> what Ivan suggested to do here instead? > > > > Aren't you guys referring to > > pci-disable-decode-of-io-memory-during-bar-sizing.patch? This is > > another one entirely, though related. > > I have no idea, there have been a lot of conflicting patches in this > area... Oh sorry, my confusion. I don't know what happened to the mmconfig vs. ACPI resources patch. Jesse - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC] full suspend/resume support for i915 DRM driver
On Thursday, October 25, 2007 9:59 pm Greg KH wrote: > On Thu, Oct 25, 2007 at 04:53:18PM -0700, Jesse Barnes wrote: > > Ok, here's yet another version that uses the device model for the > > suspend/resume, rather than pci hooks. > > > > Greg, DRM desperately needs review of its device model usage, can > > you take a look at this patch and the current drm_sysfs.c code? > > Right now, we're mixing class_devices and regular devices (the > > latter seem to be required for suspend/resume to work correctly), > > but this seems wrong. Any ideas? Should we just rip out the > > class_device stuff and create full-on DRM device nodes? > > The class_device stuff is already ripped out in the latest -mm trees > and I will be forwarding that change on for 2.6.25 after 2.6.24 is > out. So yes, it should be taken away :) > > But converting from class_device to struct device does not mean you > use a "device node". But you could if you want to :) Yeah, bad choice of words. :) To retain compatibility, we need to have directories under the DRM class dir (/sys/class/drm) for each card (e.g. card0) that contains a file describing which graphics driver is bound to the device. For class devices, we could just add an attributes structure to the device. Can we do the same with regular, non-class devices? Thanks, Jesse - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC] full suspend/resume support for i915 DRM driver
On Thursday, October 25, 2007 9:59 pm Greg KH wrote: On Thu, Oct 25, 2007 at 04:53:18PM -0700, Jesse Barnes wrote: Ok, here's yet another version that uses the device model for the suspend/resume, rather than pci hooks. Greg, DRM desperately needs review of its device model usage, can you take a look at this patch and the current drm_sysfs.c code? Right now, we're mixing class_devices and regular devices (the latter seem to be required for suspend/resume to work correctly), but this seems wrong. Any ideas? Should we just rip out the class_device stuff and create full-on DRM device nodes? The class_device stuff is already ripped out in the latest -mm trees and I will be forwarding that change on for 2.6.25 after 2.6.24 is out. So yes, it should be taken away :) But converting from class_device to struct device does not mean you use a device node. But you could if you want to :) Yeah, bad choice of words. :) To retain compatibility, we need to have directories under the DRM class dir (/sys/class/drm) for each card (e.g. card0) that contains a file describing which graphics driver is bound to the device. For class devices, we could just add an attributes structure to the device. Can we do the same with regular, non-class devices? Thanks, Jesse - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: pci-disable-decode-of-io-memory-during-bar-sizing.patch
On Thursday, October 25, 2007 7:54 pm Greg KH wrote: On Thu, Oct 25, 2007 at 04:22:35PM -0700, Jesse Barnes wrote: I think Greg doesn't like it, even though we don't have an alternative at this point... Yes, I didn't like it, Ivan didn't like it, and I got reports that it wasn't even needed at all once you upgraded your BIOS to the latest version. So, is this still needed? And if so, can you try to implement what Ivan suggested to do here instead? Yes, it's still needed. Auke rescinded his BIOS upgrade makes it work message, so something like this is still necessary. Jesse - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: - mmconfig-validate-against-acpi-motherboard-resources.patch removed from -mm tree
On Thursday, October 25, 2007 10:27 pm Greg KH wrote: On Thu, Oct 25, 2007 at 11:03:51PM -0600, Robert Hancock wrote: Greg KH wrote: On Thu, Oct 25, 2007 at 04:22:35PM -0700, Jesse Barnes wrote: I think Greg doesn't like it, even though we don't have an alternative at this point... Yes, I didn't like it, Ivan didn't like it, and I got reports that it wasn't even needed at all once you upgraded your BIOS to the latest version. So, is this still needed? And if so, can you try to implement what Ivan suggested to do here instead? Aren't you guys referring to pci-disable-decode-of-io-memory-during-bar-sizing.patch? This is another one entirely, though related. I have no idea, there have been a lot of conflicting patches in this area... Oh sorry, my confusion. I don't know what happened to the mmconfig vs. ACPI resources patch. Jesse - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC] full suspend/resume support for i915 DRM driver
On Friday, October 26, 2007 10:10 am Kay Sievers wrote: The conversion is already queued in Greg's tree, and in -mm: http://git.kernel.org/?p=linux/kernel/git/gregkh/patches.git;a=blob;f =driver/drm-convert-from-class_device-to-device-in-drivers-char-drm.pa tch;h=f993183d1cb017f981cc2232d17930af40459bd8;hb=HEAD Hm, I've done it slightly differently, by adding an actual device to the drm_device structure... I'll post it once I've cleaned it up a little more. /sys/class/drm will look the same as with the class_device's, only if !CONFIG_SYSFS_DEPRECATED, there will be symlinks instead of directories, otherwise the same pathes, like for all other (converted) classes too. Ok, thanks. Jesse - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: fixing up DRM device model usage
On Friday, October 26, 2007 10:10 am Kay Sievers wrote:
On 10/26/07, Jesse Barnes [EMAIL PROTECTED] wrote:
On Thursday, October 25, 2007 9:59 pm Greg KH wrote:
On Thu, Oct 25, 2007 at 04:53:18PM -0700, Jesse Barnes wrote:
Ok, here's yet another version that uses the device model for
the suspend/resume, rather than pci hooks.
Greg, DRM desperately needs review of its device model usage,
can you take a look at this patch and the current drm_sysfs.c
code? Right now, we're mixing class_devices and regular devices
(the latter seem to be required for suspend/resume to work
correctly), but this seems wrong. Any ideas? Should we just
rip out the class_device stuff and create full-on DRM device
nodes?
The class_device stuff is already ripped out in the latest -mm
trees and I will be forwarding that change on for 2.6.25 after
2.6.24 is out. So yes, it should be taken away :)
But converting from class_device to struct device does not mean
you use a device node. But you could if you want to :)
Yeah, bad choice of words. :)
To retain compatibility, we need to have directories under the DRM
class dir (/sys/class/drm) for each card (e.g. card0) that contains
a file describing which graphics driver is bound to the device.
For class devices, we could just add an attributes structure to the
device. Can we do the same with regular, non-class devices?
The conversion is already queued in Greg's tree, and in -mm:
http://git.kernel.org/?p=linux/kernel/git/gregkh/patches.git;a=blob;f
=driver/drm-convert-from-class_device-to-device-in-drivers-char-drm.pa
tch;h=f993183d1cb017f981cc2232d17930af40459bd8;hb=HEAD
/sys/class/drm will look the same as with the class_device's, only if
!CONFIG_SYSFS_DEPRECATED, there will be symlinks instead of
directories, otherwise the same pathes, like for all other
(converted) classes too.
How does this conversion look? It retains directory compatibility with
the old scheme, but I'm not sure about the dev-dev.parent value. I'm
using the PCI device corresponding to the DRM device as the parent, but
maybe I don't need one at all?
Dave, the drm_head stuff is a bit funky; it seems like a partially
implemented feature? I wonder if we should rip that out too, just to
keep things simple...
Thanks,
Jesse
diff --git a/linux-core/Kconfig b/linux-core/Kconfig
diff --git a/linux-core/drmP.h b/linux-core/drmP.h
index d0ab2c9..82a3a23 100644
--- a/linux-core/drmP.h
+++ b/linux-core/drmP.h
@@ -619,6 +619,8 @@ struct drm_driver {
void (*postclose) (struct drm_device *, struct drm_file *);
void (*lastclose) (struct drm_device *);
int (*unload) (struct drm_device *);
+ int (*suspend) (struct drm_device *);
+ int (*resume) (struct drm_device *);
int (*dma_ioctl) (struct drm_device *dev, void *data, struct drm_file
*file_priv);
void (*dma_ready) (struct drm_device *);
int (*dma_quiescent) (struct drm_device *);
@@ -697,6 +699,7 @@ struct drm_head {
* may contain multiple heads.
*/
struct drm_device {
+ struct device dev; /** Linux device */
char *unique; /** Unique identifier: e.g., busid */
int unique_len; /** Length of unique field */
char *devname; /** For /proc/interrupts */
@@ -1163,10 +1166,9 @@ extern void drm_pci_free(struct drm_device *dev,
drm_dma_handle_t *dmah);
/* sysfs support (drm_sysfs.c) */
struct drm_sysfs_class;
extern struct class *drm_sysfs_create(struct module *owner, char *name);
-extern void drm_sysfs_destroy(struct class *cs);
-extern struct class_device *drm_sysfs_device_add(struct class *cs,
-struct drm_head * head);
-extern void drm_sysfs_device_remove(struct class_device *class_dev);
+extern void drm_sysfs_destroy(void);
+extern int drm_sysfs_device_add(struct drm_device *dev, struct drm_head *
head);
+extern void drm_sysfs_device_remove(struct drm_device *dev);
/*
* Basic memory manager support (drm_mm.c)
diff --git a/linux-core/drm_drv.c b/linux-core/drm_drv.c
index fe2b120..47d1765 100644
--- a/linux-core/drm_drv.c
+++ b/linux-core/drm_drv.c
@@ -519,7 +519,7 @@ static int __init drm_core_init(void)
CORE_MAJOR, CORE_MINOR, CORE_PATCHLEVEL, CORE_DATE);
return 0;
err_p3:
- drm_sysfs_destroy(drm_class);
+ drm_sysfs_destroy();
err_p2:
unregister_chrdev(DRM_MAJOR, drm);
drm_free(drm_heads, sizeof(*drm_heads) * drm_cards_limit, DRM_MEM_STUB);
@@ -530,7 +530,7 @@ err_p1:
static void __exit drm_core_exit(void)
{
remove_proc_entry(dri, NULL);
- drm_sysfs_destroy(drm_class);
+ drm_sysfs_destroy();
unregister_chrdev(DRM_MAJOR, drm);
diff --git a/linux-core/drm_stub.c b/linux-core/drm_stub.c
index 9e140ac..1d88d37 100644
--- a/linux-core/drm_stub.c
+++ b/linux-core
Re: fixing up DRM device model usage
On Friday, October 26, 2007 12:08 pm Kay Sievers wrote:
How does this conversion look?
Seems fine, at a first look. You moved the device structure into the
object where it belongs, instead of allocating one, and saving the
pointer. You should really considering changing the core to do the
free()ing of your object with the embedded devices release function,
that is called when the last reference to the object is gone, instead
of hoping the best. :) But if the drm core does that properly, it
might work, sure.
Yeah, but that'll require other changes to the DRM. Maybe I can tackle
that as part of the refactoring I'm doing for some other work.
The open coded: device_create_file(dev-dev, device_attrs[i])
should probably replaced by passing the array to the class, and the
core will do that for you.
You mean just set drm_class-dev_attrs = device_attrs? I didn't see in
the core device model code where that would create the files...
Do you assign the dev_t: MKDEV(DRM_MAJOR, head-minor) somewhere? You
need to put it in dev-devt, if you want a device node created by
userspace.
Yeah, that's part of the drm_head structure. I'll go ahead and assign
it to dev-devt too.
It retains directory compatibility with
the old scheme, but I'm not sure about the dev-dev.parent value.
You should use the same value as the old code:
(head-dev-pdev)-dev
and assign it as the parent, seems right..
I'm using the PCI device corresponding to the DRM device as the
parent, but maybe I don't need one at all?
Keep it, you want to express the relationship in sysfs, so that a
device link is created, or that the device directory lives as a
child below the parent device. Seems fine so far.
Ok, sounds good.
Dave, the drm_head stuff is a bit funky; it seems like a partially
implemented feature? I wonder if we should rip that out too, just
to keep things simple...
Hehe, that's always a solution. :)
Yeah, removing code is nearly always a win. :)
Thanks,
Jesse
diff --git a/linux-core/drmP.h b/linux-core/drmP.h
index d0ab2c9..82a3a23 100644
--- a/linux-core/drmP.h
+++ b/linux-core/drmP.h
@@ -619,6 +619,8 @@ struct drm_driver {
void (*postclose) (struct drm_device *, struct drm_file *);
void (*lastclose) (struct drm_device *);
int (*unload) (struct drm_device *);
+ int (*suspend) (struct drm_device *);
+ int (*resume) (struct drm_device *);
int (*dma_ioctl) (struct drm_device *dev, void *data, struct drm_file
*file_priv);
void (*dma_ready) (struct drm_device *);
int (*dma_quiescent) (struct drm_device *);
@@ -697,6 +699,7 @@ struct drm_head {
* may contain multiple heads.
*/
struct drm_device {
+ struct device dev; /** Linux device */
char *unique; /** Unique identifier: e.g., busid */
int unique_len; /** Length of unique field */
char *devname; /** For /proc/interrupts */
@@ -1163,10 +1166,9 @@ extern void drm_pci_free(struct drm_device *dev,
drm_dma_handle_t *dmah);
/* sysfs support (drm_sysfs.c) */
struct drm_sysfs_class;
extern struct class *drm_sysfs_create(struct module *owner, char *name);
-extern void drm_sysfs_destroy(struct class *cs);
-extern struct class_device *drm_sysfs_device_add(struct class *cs,
-struct drm_head * head);
-extern void drm_sysfs_device_remove(struct class_device *class_dev);
+extern void drm_sysfs_destroy(void);
+extern int drm_sysfs_device_add(struct drm_device *dev, struct drm_head *
head);
+extern void drm_sysfs_device_remove(struct drm_device *dev);
/*
* Basic memory manager support (drm_mm.c)
diff --git a/linux-core/drm_drv.c b/linux-core/drm_drv.c
index fe2b120..47d1765 100644
--- a/linux-core/drm_drv.c
+++ b/linux-core/drm_drv.c
@@ -519,7 +519,7 @@ static int __init drm_core_init(void)
CORE_MAJOR, CORE_MINOR, CORE_PATCHLEVEL, CORE_DATE);
return 0;
err_p3:
- drm_sysfs_destroy(drm_class);
+ drm_sysfs_destroy();
err_p2:
unregister_chrdev(DRM_MAJOR, drm);
drm_free(drm_heads, sizeof(*drm_heads) * drm_cards_limit, DRM_MEM_STUB);
@@ -530,7 +530,7 @@ err_p1:
static void __exit drm_core_exit(void)
{
remove_proc_entry(dri, NULL);
- drm_sysfs_destroy(drm_class);
+ drm_sysfs_destroy();
unregister_chrdev(DRM_MAJOR, drm);
diff --git a/linux-core/drm_stub.c b/linux-core/drm_stub.c
index 9e140ac..1d88d37 100644
--- a/linux-core/drm_stub.c
+++ b/linux-core/drm_stub.c
@@ -183,11 +183,10 @@ static int drm_get_head(struct drm_device * dev, struct
drm_head * head)
goto err_g1;
}
- head-dev_class = drm_sysfs_device_add(drm_class, head);
- if (IS_ERR(head-dev_class)) {
+ ret = drm_sysfs_device_add(dev, head
Re: [RFC] full suspend/resume support for i915 DRM driver
Ok, here's yet another version that uses the device model for the
suspend/resume, rather than pci hooks.
Greg, DRM desperately needs review of its device model usage, can you
take a look at this patch and the current drm_sysfs.c code? Right now,
we're mixing class_devices and regular devices (the latter seem to be
required for suspend/resume to work correctly), but this seems wrong.
Any ideas? Should we just rip out the class_device stuff and create
full-on DRM device nodes?
This one also includes the feedback from Pavel & Rafael, along with
some fixes that were causing crashes during module unload or X startup.
Thanks,
Jesse
diff --git a/linux-core/Kconfig b/linux-core/Kconfig
diff --git a/linux-core/drmP.h b/linux-core/drmP.h
index d0ab2c9..39fce95 100644
--- a/linux-core/drmP.h
+++ b/linux-core/drmP.h
@@ -619,6 +619,8 @@ struct drm_driver {
void (*postclose) (struct drm_device *, struct drm_file *);
void (*lastclose) (struct drm_device *);
int (*unload) (struct drm_device *);
+ int (*suspend) (struct drm_device *);
+ int (*resume) (struct drm_device *);
int (*dma_ioctl) (struct drm_device *dev, void *data, struct drm_file
*file_priv);
void (*dma_ready) (struct drm_device *);
int (*dma_quiescent) (struct drm_device *);
@@ -697,6 +699,7 @@ struct drm_head {
* may contain multiple heads.
*/
struct drm_device {
+ struct device dev; /**< Linux device */
char *unique; /**< Unique identifier: e.g., busid */
int unique_len; /**< Length of unique field */
char *devname; /**< For /proc/interrupts */
@@ -1164,7 +1167,8 @@ extern void drm_pci_free(struct drm_device *dev,
drm_dma_handle_t *dmah);
struct drm_sysfs_class;
extern struct class *drm_sysfs_create(struct module *owner, char *name);
extern void drm_sysfs_destroy(struct class *cs);
-extern struct class_device *drm_sysfs_device_add(struct class *cs,
+extern struct class_device *drm_sysfs_device_add(struct drm_device *dev,
+struct class *cs,
struct drm_head * head);
extern void drm_sysfs_device_remove(struct class_device *class_dev);
diff --git a/linux-core/drm_drv.c b/linux-core/drm_drv.c
diff --git a/linux-core/drm_stub.c b/linux-core/drm_stub.c
index 9e140ac..b0b1001 100644
--- a/linux-core/drm_stub.c
+++ b/linux-core/drm_stub.c
@@ -183,7 +183,8 @@ static int drm_get_head(struct drm_device * dev, struct
drm_head * head)
goto err_g1;
}
- head->dev_class = drm_sysfs_device_add(drm_class, head);
+ head->dev_class = drm_sysfs_device_add(dev, drm_class,
+ head);
if (IS_ERR(head->dev_class)) {
printk(KERN_ERR
"DRM: Error sysfs_device_add.\n");
diff --git a/linux-core/drm_sysfs.c b/linux-core/drm_sysfs.c
index cf4349b..ce83c24 100644
--- a/linux-core/drm_sysfs.c
+++ b/linux-core/drm_sysfs.c
@@ -19,6 +19,30 @@
#include "drm_core.h"
#include "drmP.h"
+#define to_drm_device(d) container_of(d, struct drm_device, dev)
+
+static int drm_sysfs_suspend(struct device *dev, pm_message_t state)
+{
+ struct drm_device *drm_dev = to_drm_device(dev);
+
+ printk(KERN_ERR "%s\n", __FUNCTION__);
+
+ if (drm_dev->driver->suspend)
+ return drm_dev->driver->suspend(drm_dev);
+
+ return 0;
+}
+
+static int drm_sysfs_resume(struct device *dev)
+{
+ struct drm_device *drm_dev = to_drm_device(dev);
+
+ if (drm_dev->driver->resume)
+ return drm_dev->driver->resume(drm_dev);
+
+ return 0;
+}
+
/* Display the version of drm_core. This doesn't work right in current design
*/
static ssize_t version_show(struct class *dev, char *buf)
{
@@ -50,6 +74,9 @@ struct class *drm_sysfs_create(struct module *owner, char
*name)
goto err_out;
}
+ class->suspend = drm_sysfs_suspend;
+ class->resume = drm_sysfs_resume;
+
err = class_create_file(class, _attr_version);
if (err)
goto err_out_class;
@@ -73,7 +100,6 @@ void drm_sysfs_destroy(struct class *class)
{
if ((class == NULL) || (IS_ERR(class)))
return;
-
class_remove_file(class, _attr_version);
class_destroy(class);
}
@@ -104,10 +130,14 @@ static struct class_device_attribute class_device_attrs[]
= {
* Note: the struct class passed to this function must have previously been
* created with a call to drm_sysfs_create().
*/
-struct class_device *drm_sysfs_device_add(struct class *cs, struct drm_head
*head)
+struct class_device *drm_sysf
Re: - mmconfig-validate-against-acpi-motherboard-resources.patch removed from -mm tree
I think Greg doesn't like it, even though we don't have an alternative at this point... Jesse On Thursday, October 25, 2007 4:20 pm Robert Hancock wrote: > Where did this patch go? I didn't get notified that anyone dropped > it, but I don't see it in current -git.. > > [EMAIL PROTECTED] wrote: > > The patch titled > > MMCONFIG: validate against ACPI motherboard resources > > has been removed from the -mm tree. Its filename was > > mmconfig-validate-against-acpi-motherboard-resources.patch > > > > This patch was dropped because it was merged into mainline or a > > subsystem tree > > > > -- > > Subject: MMCONFIG: validate against ACPI motherboard resources > > From: Robert Hancock <[EMAIL PROTECTED]> > > > > This path adds validation of the MMCONFIG table against the ACPI > > reserved motherboard resources. If the MMCONFIG table is found to > > be reserved in ACPI, we don't bother checking the E820 table. The > > PCI Express firmware spec apparently tells BIOS developers that > > reservation in ACPI is required and E820 reservation is optional, > > so checking against ACPI first makes sense. Many BIOSes don't > > reserve the MMCONFIG region in E820 even though it is perfectly > > functional, the existing check needlessly disables MMCONFIG in > > these cases. > > > > In order to do this, MMCONFIG setup has been split into two phases. > > If PCI configuration type 1 is not available then MMCONFIG is > > enabled early as before. Otherwise, it is enabled later after the > > ACPI interpreter is enabled, since we need to be able to execute > > control methods in order to check the ACPI reserved resources. > > Presently this is just triggered off the end of ACPI interpreter > > initialization. > > > > There are a few other behavioral changes here: > > > > - Validate all MMCONFIG configurations provided, not just the first > > one. > > > > - Validate the entire required length of each configuration > > according to the provided ending bus number is reserved, not just > > the minimum required allocation. > > > > - Validate that the area is reserved even if we read it from the > > chipset directly and not from the MCFG table. This catches the > > case where the BIOS didn't set the location properly in the chipset > > and has mapped it over other things it shouldn't have. > > > > This also cleans up the MMCONFIG initialization functions so that > > they simply do nothing if MMCONFIG is not compiled in. > > > > Based on an original patch by Rajesh Shah from Intel. > > > > [EMAIL PROTECTED]: many fixes and cleanups] > > Signed-off-by: Robert Hancock <[EMAIL PROTECTED]> > > Cc: Rajesh Shah <[EMAIL PROTECTED]> > > Cc: Jesse Barnes <[EMAIL PROTECTED]> > > Acked-by: Linus Torvalds <[EMAIL PROTECTED]> > > Cc: Andi Kleen <[EMAIL PROTECTED]> > > Cc: Greg KH <[EMAIL PROTECTED]> > > Signed-off-by: Andrew Morton <[EMAIL PROTECTED]> > > --- > > > > arch/i386/pci/init.c|4 > > arch/i386/pci/mmconfig-shared.c | 151 > > ++ arch/i386/pci/pci.h | > > 1 > > drivers/acpi/bus.c |2 > > include/linux/pci.h |8 + > > 5 files changed, 144 insertions(+), 22 deletions(-) > > > > diff -puN > > arch/i386/pci/init.c~mmconfig-validate-against-acpi-motherboard-res > >ources arch/i386/pci/init.c --- > > a/arch/i386/pci/init.c~mmconfig-validate-against-acpi-motherboard-r > >esources +++ a/arch/i386/pci/init.c > > @@ -11,9 +11,7 @@ static __init int pci_access_init(void) > > #ifdef CONFIG_PCI_DIRECT > > type = pci_direct_probe(); > > #endif > > -#ifdef CONFIG_PCI_MMCONFIG > > - pci_mmcfg_init(type); > > -#endif > > + pci_mmcfg_early_init(type); > > if (raw_pci_ops) > > return 0; > > #ifdef CONFIG_PCI_BIOS > > diff -puN > > arch/i386/pci/mmconfig-shared.c~mmconfig-validate-against-acpi-moth > >erboard-resources arch/i386/pci/mmconfig-shared.c --- > > a/arch/i386/pci/mmconfig-shared.c~mmconfig-validate-against-acpi-mo > >therboard-resources +++ a/arch/i386/pci/mmconfig-shared.c > > @@ -206,9 +206,78 @@ static void __init pci_mmcfg_insert_reso > > pci_mmcfg_resources_inserted = 1; > > } > > > > -static void __init pci_mmcfg_reject_broken(int type) > > +static acpi_status __init check_mcfg_resource(struct acpi_resou
Re: - mmconfig-validate-against-acpi-motherboard-resources.patch removed from -mm tree
I think Greg doesn't like it, even though we don't have an alternative
at this point...
Jesse
On Thursday, October 25, 2007 4:20 pm Robert Hancock wrote:
Where did this patch go? I didn't get notified that anyone dropped
it, but I don't see it in current -git..
[EMAIL PROTECTED] wrote:
The patch titled
MMCONFIG: validate against ACPI motherboard resources
has been removed from the -mm tree. Its filename was
mmconfig-validate-against-acpi-motherboard-resources.patch
This patch was dropped because it was merged into mainline or a
subsystem tree
--
Subject: MMCONFIG: validate against ACPI motherboard resources
From: Robert Hancock [EMAIL PROTECTED]
This path adds validation of the MMCONFIG table against the ACPI
reserved motherboard resources. If the MMCONFIG table is found to
be reserved in ACPI, we don't bother checking the E820 table. The
PCI Express firmware spec apparently tells BIOS developers that
reservation in ACPI is required and E820 reservation is optional,
so checking against ACPI first makes sense. Many BIOSes don't
reserve the MMCONFIG region in E820 even though it is perfectly
functional, the existing check needlessly disables MMCONFIG in
these cases.
In order to do this, MMCONFIG setup has been split into two phases.
If PCI configuration type 1 is not available then MMCONFIG is
enabled early as before. Otherwise, it is enabled later after the
ACPI interpreter is enabled, since we need to be able to execute
control methods in order to check the ACPI reserved resources.
Presently this is just triggered off the end of ACPI interpreter
initialization.
There are a few other behavioral changes here:
- Validate all MMCONFIG configurations provided, not just the first
one.
- Validate the entire required length of each configuration
according to the provided ending bus number is reserved, not just
the minimum required allocation.
- Validate that the area is reserved even if we read it from the
chipset directly and not from the MCFG table. This catches the
case where the BIOS didn't set the location properly in the chipset
and has mapped it over other things it shouldn't have.
This also cleans up the MMCONFIG initialization functions so that
they simply do nothing if MMCONFIG is not compiled in.
Based on an original patch by Rajesh Shah from Intel.
[EMAIL PROTECTED]: many fixes and cleanups]
Signed-off-by: Robert Hancock [EMAIL PROTECTED]
Cc: Rajesh Shah [EMAIL PROTECTED]
Cc: Jesse Barnes [EMAIL PROTECTED]
Acked-by: Linus Torvalds [EMAIL PROTECTED]
Cc: Andi Kleen [EMAIL PROTECTED]
Cc: Greg KH [EMAIL PROTECTED]
Signed-off-by: Andrew Morton [EMAIL PROTECTED]
---
arch/i386/pci/init.c|4
arch/i386/pci/mmconfig-shared.c | 151
++ arch/i386/pci/pci.h |
1
drivers/acpi/bus.c |2
include/linux/pci.h |8 +
5 files changed, 144 insertions(+), 22 deletions(-)
diff -puN
arch/i386/pci/init.c~mmconfig-validate-against-acpi-motherboard-res
ources arch/i386/pci/init.c ---
a/arch/i386/pci/init.c~mmconfig-validate-against-acpi-motherboard-r
esources +++ a/arch/i386/pci/init.c
@@ -11,9 +11,7 @@ static __init int pci_access_init(void)
#ifdef CONFIG_PCI_DIRECT
type = pci_direct_probe();
#endif
-#ifdef CONFIG_PCI_MMCONFIG
- pci_mmcfg_init(type);
-#endif
+ pci_mmcfg_early_init(type);
if (raw_pci_ops)
return 0;
#ifdef CONFIG_PCI_BIOS
diff -puN
arch/i386/pci/mmconfig-shared.c~mmconfig-validate-against-acpi-moth
erboard-resources arch/i386/pci/mmconfig-shared.c ---
a/arch/i386/pci/mmconfig-shared.c~mmconfig-validate-against-acpi-mo
therboard-resources +++ a/arch/i386/pci/mmconfig-shared.c
@@ -206,9 +206,78 @@ static void __init pci_mmcfg_insert_reso
pci_mmcfg_resources_inserted = 1;
}
-static void __init pci_mmcfg_reject_broken(int type)
+static acpi_status __init check_mcfg_resource(struct acpi_resource
*res, + void *data)
+{
+ struct resource *mcfg_res = data;
+ struct acpi_resource_address64 address;
+ acpi_status status;
+
+ if (res-type == ACPI_RESOURCE_TYPE_FIXED_MEMORY32) {
+ struct acpi_resource_fixed_memory32 *fixmem32 =
+ res-data.fixed_memory32;
+ if (!fixmem32)
+ return AE_OK;
+ if ((mcfg_res-start = fixmem32-address)
+ (mcfg_res-end (fixmem32-address +
+ fixmem32-address_length))) {
+ mcfg_res-flags = 1;
+ return AE_CTRL_TERMINATE;
+ }
+ }
+ if ((res-type != ACPI_RESOURCE_TYPE_ADDRESS32)
+ (res-type != ACPI_RESOURCE_TYPE_ADDRESS64))
+ return AE_OK;
+
+ status
Re: [RFC] full suspend/resume support for i915 DRM driver
Ok, here's yet another version that uses the device model for the
suspend/resume, rather than pci hooks.
Greg, DRM desperately needs review of its device model usage, can you
take a look at this patch and the current drm_sysfs.c code? Right now,
we're mixing class_devices and regular devices (the latter seem to be
required for suspend/resume to work correctly), but this seems wrong.
Any ideas? Should we just rip out the class_device stuff and create
full-on DRM device nodes?
This one also includes the feedback from Pavel Rafael, along with
some fixes that were causing crashes during module unload or X startup.
Thanks,
Jesse
diff --git a/linux-core/Kconfig b/linux-core/Kconfig
diff --git a/linux-core/drmP.h b/linux-core/drmP.h
index d0ab2c9..39fce95 100644
--- a/linux-core/drmP.h
+++ b/linux-core/drmP.h
@@ -619,6 +619,8 @@ struct drm_driver {
void (*postclose) (struct drm_device *, struct drm_file *);
void (*lastclose) (struct drm_device *);
int (*unload) (struct drm_device *);
+ int (*suspend) (struct drm_device *);
+ int (*resume) (struct drm_device *);
int (*dma_ioctl) (struct drm_device *dev, void *data, struct drm_file
*file_priv);
void (*dma_ready) (struct drm_device *);
int (*dma_quiescent) (struct drm_device *);
@@ -697,6 +699,7 @@ struct drm_head {
* may contain multiple heads.
*/
struct drm_device {
+ struct device dev; /** Linux device */
char *unique; /** Unique identifier: e.g., busid */
int unique_len; /** Length of unique field */
char *devname; /** For /proc/interrupts */
@@ -1164,7 +1167,8 @@ extern void drm_pci_free(struct drm_device *dev,
drm_dma_handle_t *dmah);
struct drm_sysfs_class;
extern struct class *drm_sysfs_create(struct module *owner, char *name);
extern void drm_sysfs_destroy(struct class *cs);
-extern struct class_device *drm_sysfs_device_add(struct class *cs,
+extern struct class_device *drm_sysfs_device_add(struct drm_device *dev,
+struct class *cs,
struct drm_head * head);
extern void drm_sysfs_device_remove(struct class_device *class_dev);
diff --git a/linux-core/drm_drv.c b/linux-core/drm_drv.c
diff --git a/linux-core/drm_stub.c b/linux-core/drm_stub.c
index 9e140ac..b0b1001 100644
--- a/linux-core/drm_stub.c
+++ b/linux-core/drm_stub.c
@@ -183,7 +183,8 @@ static int drm_get_head(struct drm_device * dev, struct
drm_head * head)
goto err_g1;
}
- head-dev_class = drm_sysfs_device_add(drm_class, head);
+ head-dev_class = drm_sysfs_device_add(dev, drm_class,
+ head);
if (IS_ERR(head-dev_class)) {
printk(KERN_ERR
DRM: Error sysfs_device_add.\n);
diff --git a/linux-core/drm_sysfs.c b/linux-core/drm_sysfs.c
index cf4349b..ce83c24 100644
--- a/linux-core/drm_sysfs.c
+++ b/linux-core/drm_sysfs.c
@@ -19,6 +19,30 @@
#include drm_core.h
#include drmP.h
+#define to_drm_device(d) container_of(d, struct drm_device, dev)
+
+static int drm_sysfs_suspend(struct device *dev, pm_message_t state)
+{
+ struct drm_device *drm_dev = to_drm_device(dev);
+
+ printk(KERN_ERR %s\n, __FUNCTION__);
+
+ if (drm_dev-driver-suspend)
+ return drm_dev-driver-suspend(drm_dev);
+
+ return 0;
+}
+
+static int drm_sysfs_resume(struct device *dev)
+{
+ struct drm_device *drm_dev = to_drm_device(dev);
+
+ if (drm_dev-driver-resume)
+ return drm_dev-driver-resume(drm_dev);
+
+ return 0;
+}
+
/* Display the version of drm_core. This doesn't work right in current design
*/
static ssize_t version_show(struct class *dev, char *buf)
{
@@ -50,6 +74,9 @@ struct class *drm_sysfs_create(struct module *owner, char
*name)
goto err_out;
}
+ class-suspend = drm_sysfs_suspend;
+ class-resume = drm_sysfs_resume;
+
err = class_create_file(class, class_attr_version);
if (err)
goto err_out_class;
@@ -73,7 +100,6 @@ void drm_sysfs_destroy(struct class *class)
{
if ((class == NULL) || (IS_ERR(class)))
return;
-
class_remove_file(class, class_attr_version);
class_destroy(class);
}
@@ -104,10 +130,14 @@ static struct class_device_attribute class_device_attrs[]
= {
* Note: the struct class passed to this function must have previously been
* created with a call to drm_sysfs_create().
*/
-struct class_device *drm_sysfs_device_add(struct class *cs, struct drm_head
*head)
+struct class_device *drm_sysfs_device_add(struct drm_device *dev,
+ struct class *cs
Re: [RFC] full suspend/resume support for i915 DRM driver
On Wednesday, October 24, 2007 1:17 pm Adrian Bunk wrote: > > diff --git a/linux-core/Kconfig b/linux-core/Kconfig > > index 2d02c76..5e73fc7 100644 > > --- a/linux-core/Kconfig > > +++ b/linux-core/Kconfig > > @@ -50,7 +50,7 @@ config DRM_I810 > > > > choice > > prompt "Intel 830M, 845G, 852GM, 855GM, 865G" > > - depends on DRM && AGP && AGP_INTEL > > + depends on DRM && AGP && AGP_INTEL && !FB_INTEL > > optional > >... > > This sounds like a bad regression. Well it's really documenting existing behavior. Unless you're very careful, running custom applications, the Intel FB and DRM drivers will conflict. IMO this is a long overdue change, but Dave has some custom stuff that requires both drivers so he'd rather not see it go in, so I'm fine with dropping this part. Jesse - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC] full suspend/resume support for i915 DRM driver
On Wednesday, October 24, 2007 6:35:14 am Pavel Machek wrote: > Hi! > > > We seem to see a lot of bug reports along the lines of, "my machine > > resumes but I can't see X" or, "I can see X but only with a bright > > flashlight", etc. These sorts of problems are due to the fact that > > the X server isn't designed to do full state save/restore, and none > > of the available kernel drivers do it on its behalf. > > > > Since intelfb and the rest of the Intel drivers are fairly incompatible, > > this patch makes the DRM bind to the PCI device so it can register real > > suspend/resume handlers. Those handlers take care of saving and > > restoring enough state for X to come back reliably on at least one of my > > problematic test machines, but text mode still has trouble (still > > debugging VGA state save/restore, including trying to save/restore > > actual VRAM contents for possible hibernate support). > > > > How does this approach look? Is a new DRM driver flag a good thing for > > similar situations with other drivers? Thoughts? > > Looks okay to me... from very quick look. > > > + if (!i915_pipe_enabled(dev, pipe)) > > + return; > > + > > + if (pipe == PIPE_A) > > + array = dev_priv->savePaletteA; > > coding style, we probably want save_palette_A. Yeah, I tried not to pull over uglies from the X code but I guess I forgot this bit. I should also update the copyright. > > + unsigned long reg = pipe == PIPE_A ? PALETTE_A : PALETTE_B; > > Uff. Mixing = and == and ? in one expression is evil. I could put parens around it if you think that would help, or just move it to a new line... > I think I seen some #if 0 code just remove that. Oh I left some in there? Yeah I'll remove it. Thanks, Jesse - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC] full suspend/resume support for i915 DRM driver
On Wednesday, October 24, 2007 6:35:14 am Pavel Machek wrote: Hi! We seem to see a lot of bug reports along the lines of, my machine resumes but I can't see X or, I can see X but only with a bright flashlight, etc. These sorts of problems are due to the fact that the X server isn't designed to do full state save/restore, and none of the available kernel drivers do it on its behalf. Since intelfb and the rest of the Intel drivers are fairly incompatible, this patch makes the DRM bind to the PCI device so it can register real suspend/resume handlers. Those handlers take care of saving and restoring enough state for X to come back reliably on at least one of my problematic test machines, but text mode still has trouble (still debugging VGA state save/restore, including trying to save/restore actual VRAM contents for possible hibernate support). How does this approach look? Is a new DRM driver flag a good thing for similar situations with other drivers? Thoughts? Looks okay to me... from very quick look. + if (!i915_pipe_enabled(dev, pipe)) + return; + + if (pipe == PIPE_A) + array = dev_priv-savePaletteA; coding style, we probably want save_palette_A. Yeah, I tried not to pull over uglies from the X code but I guess I forgot this bit. I should also update the copyright. + unsigned long reg = pipe == PIPE_A ? PALETTE_A : PALETTE_B; Uff. Mixing = and == and ? in one expression is evil. I could put parens around it if you think that would help, or just move it to a new line... I think I seen some #if 0 code just remove that. Oh I left some in there? Yeah I'll remove it. Thanks, Jesse - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC] full suspend/resume support for i915 DRM driver
On Wednesday, October 24, 2007 1:17 pm Adrian Bunk wrote: diff --git a/linux-core/Kconfig b/linux-core/Kconfig index 2d02c76..5e73fc7 100644 --- a/linux-core/Kconfig +++ b/linux-core/Kconfig @@ -50,7 +50,7 @@ config DRM_I810 choice prompt Intel 830M, 845G, 852GM, 855GM, 865G - depends on DRM AGP AGP_INTEL + depends on DRM AGP AGP_INTEL !FB_INTEL optional ... This sounds like a bad regression. Well it's really documenting existing behavior. Unless you're very careful, running custom applications, the Intel FB and DRM drivers will conflict. IMO this is a long overdue change, but Dave has some custom stuff that requires both drivers so he'd rather not see it go in, so I'm fine with dropping this part. Jesse - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC] full suspend/resume support for i915 DRM driver
On Friday, October 19, 2007, Jesse Barnes wrote:
> Dave can you take a look at the new flag and also see what you think
> about supporting suspend/resume in the event X hasn't started yet?
> There's some #if 0'd code to support that case, but I haven't tested
> it.
Ok Dave, this one's been updated with support for suspend/resume with
or without X running (iow I removed the requirement that X initialize
the driver, which DRM drivers usually have).
Hope it looks alright, it could definitely use testing on more
machines though...
Jesse
diff --git a/linux-core/Kconfig b/linux-core/Kconfig
index 2d02c76..5e73fc7 100644
--- a/linux-core/Kconfig
+++ b/linux-core/Kconfig
@@ -50,7 +50,7 @@ config DRM_I810
choice
prompt "Intel 830M, 845G, 852GM, 855GM, 865G"
- depends on DRM && AGP && AGP_INTEL
+ depends on DRM && AGP && AGP_INTEL && !FB_INTEL
optional
config DRM_I915
@@ -60,6 +60,9 @@ config DRM_I915
852GM, 855GM, 865G, 915G, 915GM, 945G, 945GM and 965G integrated
graphics. If M is selected, the module will be called i915.
AGP support is required for this driver to work.
+
+ Note that this driver is incompatible with the Intel framebuffer
+ driver.
endchoice
diff --git a/linux-core/drmP.h b/linux-core/drmP.h
index f8ca3f4..36ce266 100644
--- a/linux-core/drmP.h
+++ b/linux-core/drmP.h
@@ -106,6 +106,7 @@ struct drm_file;
#define DRIVER_DMA_QUEUE 0x200
#define DRIVER_FB_DMA 0x400
#define DRIVER_IRQ_VBL20x800
+#define DRIVER_BIND_PCI0x1000
/[EMAIL PROTECTED]/
diff --git a/linux-core/drm_drv.c b/linux-core/drm_drv.c
index a09fa96..3d14fb4 100644
--- a/linux-core/drm_drv.c
+++ b/linux-core/drm_drv.c
@@ -340,9 +340,10 @@ int drm_init(struct drm_driver *driver,
}
}
- if (!drm_fb_loaded)
+ if (!drm_fb_loaded || (driver->driver_features & DRIVER_BIND_PCI)) {
+ printk(KERN_INFO "DRM binding to PCI device\n");
return pci_register_driver(>pci_driver);
- else {
+ } else {
for (i = 0; pciidlist[i].vendor != 0; i++) {
pid = [i];
@@ -394,7 +395,7 @@ static void drm_cleanup(struct drm_device * dev)
drm_mm_takedown(>offset_manager);
drm_ht_remove(>object_hash);
- if (!drm_fb_loaded)
+ if (!drm_fb_loaded || (dev->driver->driver_features & DRIVER_BIND_PCI))
pci_disable_device(dev->pdev);
drm_ctxbitmap_cleanup(dev);
@@ -427,7 +428,7 @@ void drm_exit(struct drm_driver *driver)
struct drm_head *head;
DRM_DEBUG("\n");
- if (drm_fb_loaded) {
+ if (drm_fb_loaded && !(driver->driver_features & DRIVER_BIND_PCI)) {
for (i = 0; i < drm_cards_limit; i++) {
head = drm_heads[i];
if (!head)
diff --git a/linux-core/drm_stub.c b/linux-core/drm_stub.c
index 07ea91e..e504865 100644
--- a/linux-core/drm_stub.c
+++ b/linux-core/drm_stub.c
@@ -230,7 +230,7 @@ int drm_get_dev(struct pci_dev *pdev, const struct
pci_device_id *ent,
if (!dev)
return -ENOMEM;
- if (!drm_fb_loaded) {
+ if (!drm_fb_loaded || (driver->driver_features & DRIVER_BIND_PCI)) {
pci_set_drvdata(pdev, dev);
ret = pci_request_regions(pdev, driver->pci_driver.name);
if (ret)
@@ -256,13 +256,13 @@ int drm_get_dev(struct pci_dev *pdev, const struct
pci_device_id *ent,
return 0;
err_g3:
- if (!drm_fb_loaded)
+ if (!drm_fb_loaded || (driver->driver_features & DRIVER_BIND_PCI))
pci_disable_device(pdev);
err_g2:
- if (!drm_fb_loaded)
+ if (!drm_fb_loaded || (driver->driver_features & DRIVER_BIND_PCI))
pci_release_regions(pdev);
err_g1:
- if (!drm_fb_loaded)
+ if (!drm_fb_loaded || (driver->driver_features & DRIVER_BIND_PCI))
pci_set_drvdata(pdev, NULL);
drm_free(dev, sizeof(*dev), DRM_MEM_STUB);
diff --git a/linux-core/i915_drv.c b/linux-core/i915_drv.c
index e337e1d..5b8b74f 100644
--- a/linux-core/i915_drv.c
+++ b/linux-core/i915_drv.c
@@ -69,6 +69,460 @@ static struct drm_bo_driver i915_bo_driver = {
};
#endif
+enum pipe {
+PIPE_A = 0,
+PIPE_B,
+};
+
+static bool i915_pipe_enabled(struct drm_device *dev, enum pipe pipe)
+{
+ struct drm_i915_private *dev_priv = dev->dev_private;
+
+ if (pipe == PIPE_A)
+ return (I915_READ(DPLL_A) & DPLL_VCO_ENABLE);
+ else
+ return (I915_READ(DPLL_B) & DPLL_VCO_ENABLE);
+}
+
+static void i915_save_palette(struct drm_device *dev, enum pipe pipe)
+{
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ unsigned long reg
Re: [RFC] full suspend/resume support for i915 DRM driver
On Friday, October 19, 2007, Jesse Barnes wrote:
Dave can you take a look at the new flag and also see what you think
about supporting suspend/resume in the event X hasn't started yet?
There's some #if 0'd code to support that case, but I haven't tested
it.
Ok Dave, this one's been updated with support for suspend/resume with
or without X running (iow I removed the requirement that X initialize
the driver, which DRM drivers usually have).
Hope it looks alright, it could definitely use testing on more
machines though...
Jesse
diff --git a/linux-core/Kconfig b/linux-core/Kconfig
index 2d02c76..5e73fc7 100644
--- a/linux-core/Kconfig
+++ b/linux-core/Kconfig
@@ -50,7 +50,7 @@ config DRM_I810
choice
prompt Intel 830M, 845G, 852GM, 855GM, 865G
- depends on DRM AGP AGP_INTEL
+ depends on DRM AGP AGP_INTEL !FB_INTEL
optional
config DRM_I915
@@ -60,6 +60,9 @@ config DRM_I915
852GM, 855GM, 865G, 915G, 915GM, 945G, 945GM and 965G integrated
graphics. If M is selected, the module will be called i915.
AGP support is required for this driver to work.
+
+ Note that this driver is incompatible with the Intel framebuffer
+ driver.
endchoice
diff --git a/linux-core/drmP.h b/linux-core/drmP.h
index f8ca3f4..36ce266 100644
--- a/linux-core/drmP.h
+++ b/linux-core/drmP.h
@@ -106,6 +106,7 @@ struct drm_file;
#define DRIVER_DMA_QUEUE 0x200
#define DRIVER_FB_DMA 0x400
#define DRIVER_IRQ_VBL20x800
+#define DRIVER_BIND_PCI0x1000
/[EMAIL PROTECTED]/
diff --git a/linux-core/drm_drv.c b/linux-core/drm_drv.c
index a09fa96..3d14fb4 100644
--- a/linux-core/drm_drv.c
+++ b/linux-core/drm_drv.c
@@ -340,9 +340,10 @@ int drm_init(struct drm_driver *driver,
}
}
- if (!drm_fb_loaded)
+ if (!drm_fb_loaded || (driver-driver_features DRIVER_BIND_PCI)) {
+ printk(KERN_INFO DRM binding to PCI device\n);
return pci_register_driver(driver-pci_driver);
- else {
+ } else {
for (i = 0; pciidlist[i].vendor != 0; i++) {
pid = pciidlist[i];
@@ -394,7 +395,7 @@ static void drm_cleanup(struct drm_device * dev)
drm_mm_takedown(dev-offset_manager);
drm_ht_remove(dev-object_hash);
- if (!drm_fb_loaded)
+ if (!drm_fb_loaded || (dev-driver-driver_features DRIVER_BIND_PCI))
pci_disable_device(dev-pdev);
drm_ctxbitmap_cleanup(dev);
@@ -427,7 +428,7 @@ void drm_exit(struct drm_driver *driver)
struct drm_head *head;
DRM_DEBUG(\n);
- if (drm_fb_loaded) {
+ if (drm_fb_loaded !(driver-driver_features DRIVER_BIND_PCI)) {
for (i = 0; i drm_cards_limit; i++) {
head = drm_heads[i];
if (!head)
diff --git a/linux-core/drm_stub.c b/linux-core/drm_stub.c
index 07ea91e..e504865 100644
--- a/linux-core/drm_stub.c
+++ b/linux-core/drm_stub.c
@@ -230,7 +230,7 @@ int drm_get_dev(struct pci_dev *pdev, const struct
pci_device_id *ent,
if (!dev)
return -ENOMEM;
- if (!drm_fb_loaded) {
+ if (!drm_fb_loaded || (driver-driver_features DRIVER_BIND_PCI)) {
pci_set_drvdata(pdev, dev);
ret = pci_request_regions(pdev, driver-pci_driver.name);
if (ret)
@@ -256,13 +256,13 @@ int drm_get_dev(struct pci_dev *pdev, const struct
pci_device_id *ent,
return 0;
err_g3:
- if (!drm_fb_loaded)
+ if (!drm_fb_loaded || (driver-driver_features DRIVER_BIND_PCI))
pci_disable_device(pdev);
err_g2:
- if (!drm_fb_loaded)
+ if (!drm_fb_loaded || (driver-driver_features DRIVER_BIND_PCI))
pci_release_regions(pdev);
err_g1:
- if (!drm_fb_loaded)
+ if (!drm_fb_loaded || (driver-driver_features DRIVER_BIND_PCI))
pci_set_drvdata(pdev, NULL);
drm_free(dev, sizeof(*dev), DRM_MEM_STUB);
diff --git a/linux-core/i915_drv.c b/linux-core/i915_drv.c
index e337e1d..5b8b74f 100644
--- a/linux-core/i915_drv.c
+++ b/linux-core/i915_drv.c
@@ -69,6 +69,460 @@ static struct drm_bo_driver i915_bo_driver = {
};
#endif
+enum pipe {
+PIPE_A = 0,
+PIPE_B,
+};
+
+static bool i915_pipe_enabled(struct drm_device *dev, enum pipe pipe)
+{
+ struct drm_i915_private *dev_priv = dev-dev_private;
+
+ if (pipe == PIPE_A)
+ return (I915_READ(DPLL_A) DPLL_VCO_ENABLE);
+ else
+ return (I915_READ(DPLL_B) DPLL_VCO_ENABLE);
+}
+
+static void i915_save_palette(struct drm_device *dev, enum pipe pipe)
+{
+ struct drm_i915_private *dev_priv = dev-dev_private;
+ unsigned long reg = pipe == PIPE_A ? PALETTE_A : PALETTE_B;
+ u32 *array;
+ int i;
+
+ if (!i915_pipe_enabled(dev, pipe))
+ return;
+
+ if (pipe == PIPE_A)
+ array = dev_priv
Re: [RFC] full suspend/resume support for i915 DRM driver
On Thursday, October 18, 2007 2:01 pm Jesse Barnes wrote:
> We seem to see a lot of bug reports along the lines of, "my machine
> resumes but I can't see X" or, "I can see X but only with a bright
> flashlight", etc. These sorts of problems are due to the fact that
> the X server isn't designed to do full state save/restore, and none
> of the available kernel drivers do it on its behalf.
>
> Since intelfb and the rest of the Intel drivers are fairly
> incompatible, this patch makes the DRM bind to the PCI device so it
> can register real suspend/resume handlers. Those handlers take care
> of saving and restoring enough state for X to come back reliably on
> at least one of my problematic test machines, but text mode still has
> trouble (still debugging VGA state save/restore, including trying to
> save/restore actual VRAM contents for possible hibernate support).
>
> How does this approach look? Is a new DRM driver flag a good thing
> for similar situations with other drivers? Thoughts?
Here's an updated one that actually works fully on my 965 based test
platform. It should also work on other 9xx platforms, but note that it
lacks TV out and SDVO state save/restore, so I don't expect those to
work. I also haven't tested on 8xx platforms, but a similar approach
should work, so I'd welcome additions to support those chipsets.
Dave can you take a look at the new flag and also see what you think
about supporting suspend/resume in the event X hasn't started yet?
There's some #if 0'd code to support that case, but I haven't tested
it.
Thanks,
Jesse
diff --git a/linux-core/Kconfig b/linux-core/Kconfig
index 2d02c76..5e73fc7 100644
--- a/linux-core/Kconfig
+++ b/linux-core/Kconfig
@@ -50,7 +50,7 @@ config DRM_I810
choice
prompt "Intel 830M, 845G, 852GM, 855GM, 865G"
- depends on DRM && AGP && AGP_INTEL
+ depends on DRM && AGP && AGP_INTEL && !FB_INTEL
optional
config DRM_I915
@@ -60,6 +60,9 @@ config DRM_I915
852GM, 855GM, 865G, 915G, 915GM, 945G, 945GM and 965G integrated
graphics. If M is selected, the module will be called i915.
AGP support is required for this driver to work.
+
+ Note that this driver is incompatible with the Intel framebuffer
+ driver.
endchoice
diff --git a/linux-core/drmP.h b/linux-core/drmP.h
index f8ca3f4..36ce266 100644
--- a/linux-core/drmP.h
+++ b/linux-core/drmP.h
@@ -106,6 +106,7 @@ struct drm_file;
#define DRIVER_DMA_QUEUE 0x200
#define DRIVER_FB_DMA 0x400
#define DRIVER_IRQ_VBL20x800
+#define DRIVER_BIND_PCI0x1000
/[EMAIL PROTECTED]/
diff --git a/linux-core/drm_drv.c b/linux-core/drm_drv.c
index a09fa96..4d11707 100644
--- a/linux-core/drm_drv.c
+++ b/linux-core/drm_drv.c
@@ -340,9 +340,10 @@ int drm_init(struct drm_driver *driver,
}
}
- if (!drm_fb_loaded)
+ if (!drm_fb_loaded || (driver->driver_features & DRIVER_BIND_PCI)) {
+ printk(KERN_ERR "DRM binding to PCI device\n");
return pci_register_driver(>pci_driver);
- else {
+ } else {
for (i = 0; pciidlist[i].vendor != 0; i++) {
pid = [i];
diff --git a/linux-core/i915_drv.c b/linux-core/i915_drv.c
index e337e1d..809ef0d 100644
--- a/linux-core/i915_drv.c
+++ b/linux-core/i915_drv.c
@@ -69,6 +69,501 @@ static struct drm_bo_driver i915_bo_driver = {
};
#endif
+enum pipe {
+PIPE_A = 0,
+PIPE_B,
+};
+
+static bool i915_pipe_enabled(struct drm_device *dev, enum pipe pipe)
+{
+ struct drm_i915_private *dev_priv = dev->dev_private;
+
+ if (pipe == PIPE_A)
+ return (I915_READ(DPLL_A) & DPLL_VCO_ENABLE);
+ else
+ return (I915_READ(DPLL_B) & DPLL_VCO_ENABLE);
+}
+
+static void i915_save_palette(struct drm_device *dev, enum pipe pipe)
+{
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ unsigned long reg = pipe == PIPE_A ? PALETTE_A : PALETTE_B;
+ u32 *array;
+ int i;
+
+ if (!i915_pipe_enabled(dev, pipe))
+ return;
+
+ if (pipe == PIPE_A)
+ array = dev_priv->savePaletteA;
+ else
+ array = dev_priv->savePaletteB;
+
+ for(i = 0; i < 256; i++)
+ array[i] = I915_READ(reg + (i << 2));
+}
+
+static void i915_restore_palette(struct drm_device *dev, enum pipe pipe)
+{
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ unsigned long reg = pipe == PIPE_A ? PALETTE_A : PALETTE_B;
+ u32 *array;
+ int i;
+
+ if (!i915_pipe_enabled(dev, pipe))
+ return;
+
+ if (pipe == PIPE_A)
+ array = dev_priv->savePaletteA;
+ else
+ array = dev_priv->savePaletteB;
+
+ for(i =
Re: [RFC] full suspend/resume support for i915 DRM driver
On Thursday, October 18, 2007 2:01 pm Jesse Barnes wrote:
We seem to see a lot of bug reports along the lines of, my machine
resumes but I can't see X or, I can see X but only with a bright
flashlight, etc. These sorts of problems are due to the fact that
the X server isn't designed to do full state save/restore, and none
of the available kernel drivers do it on its behalf.
Since intelfb and the rest of the Intel drivers are fairly
incompatible, this patch makes the DRM bind to the PCI device so it
can register real suspend/resume handlers. Those handlers take care
of saving and restoring enough state for X to come back reliably on
at least one of my problematic test machines, but text mode still has
trouble (still debugging VGA state save/restore, including trying to
save/restore actual VRAM contents for possible hibernate support).
How does this approach look? Is a new DRM driver flag a good thing
for similar situations with other drivers? Thoughts?
Here's an updated one that actually works fully on my 965 based test
platform. It should also work on other 9xx platforms, but note that it
lacks TV out and SDVO state save/restore, so I don't expect those to
work. I also haven't tested on 8xx platforms, but a similar approach
should work, so I'd welcome additions to support those chipsets.
Dave can you take a look at the new flag and also see what you think
about supporting suspend/resume in the event X hasn't started yet?
There's some #if 0'd code to support that case, but I haven't tested
it.
Thanks,
Jesse
diff --git a/linux-core/Kconfig b/linux-core/Kconfig
index 2d02c76..5e73fc7 100644
--- a/linux-core/Kconfig
+++ b/linux-core/Kconfig
@@ -50,7 +50,7 @@ config DRM_I810
choice
prompt Intel 830M, 845G, 852GM, 855GM, 865G
- depends on DRM AGP AGP_INTEL
+ depends on DRM AGP AGP_INTEL !FB_INTEL
optional
config DRM_I915
@@ -60,6 +60,9 @@ config DRM_I915
852GM, 855GM, 865G, 915G, 915GM, 945G, 945GM and 965G integrated
graphics. If M is selected, the module will be called i915.
AGP support is required for this driver to work.
+
+ Note that this driver is incompatible with the Intel framebuffer
+ driver.
endchoice
diff --git a/linux-core/drmP.h b/linux-core/drmP.h
index f8ca3f4..36ce266 100644
--- a/linux-core/drmP.h
+++ b/linux-core/drmP.h
@@ -106,6 +106,7 @@ struct drm_file;
#define DRIVER_DMA_QUEUE 0x200
#define DRIVER_FB_DMA 0x400
#define DRIVER_IRQ_VBL20x800
+#define DRIVER_BIND_PCI0x1000
/[EMAIL PROTECTED]/
diff --git a/linux-core/drm_drv.c b/linux-core/drm_drv.c
index a09fa96..4d11707 100644
--- a/linux-core/drm_drv.c
+++ b/linux-core/drm_drv.c
@@ -340,9 +340,10 @@ int drm_init(struct drm_driver *driver,
}
}
- if (!drm_fb_loaded)
+ if (!drm_fb_loaded || (driver-driver_features DRIVER_BIND_PCI)) {
+ printk(KERN_ERR DRM binding to PCI device\n);
return pci_register_driver(driver-pci_driver);
- else {
+ } else {
for (i = 0; pciidlist[i].vendor != 0; i++) {
pid = pciidlist[i];
diff --git a/linux-core/i915_drv.c b/linux-core/i915_drv.c
index e337e1d..809ef0d 100644
--- a/linux-core/i915_drv.c
+++ b/linux-core/i915_drv.c
@@ -69,6 +69,501 @@ static struct drm_bo_driver i915_bo_driver = {
};
#endif
+enum pipe {
+PIPE_A = 0,
+PIPE_B,
+};
+
+static bool i915_pipe_enabled(struct drm_device *dev, enum pipe pipe)
+{
+ struct drm_i915_private *dev_priv = dev-dev_private;
+
+ if (pipe == PIPE_A)
+ return (I915_READ(DPLL_A) DPLL_VCO_ENABLE);
+ else
+ return (I915_READ(DPLL_B) DPLL_VCO_ENABLE);
+}
+
+static void i915_save_palette(struct drm_device *dev, enum pipe pipe)
+{
+ struct drm_i915_private *dev_priv = dev-dev_private;
+ unsigned long reg = pipe == PIPE_A ? PALETTE_A : PALETTE_B;
+ u32 *array;
+ int i;
+
+ if (!i915_pipe_enabled(dev, pipe))
+ return;
+
+ if (pipe == PIPE_A)
+ array = dev_priv-savePaletteA;
+ else
+ array = dev_priv-savePaletteB;
+
+ for(i = 0; i 256; i++)
+ array[i] = I915_READ(reg + (i 2));
+}
+
+static void i915_restore_palette(struct drm_device *dev, enum pipe pipe)
+{
+ struct drm_i915_private *dev_priv = dev-dev_private;
+ unsigned long reg = pipe == PIPE_A ? PALETTE_A : PALETTE_B;
+ u32 *array;
+ int i;
+
+ if (!i915_pipe_enabled(dev, pipe))
+ return;
+
+ if (pipe == PIPE_A)
+ array = dev_priv-savePaletteA;
+ else
+ array = dev_priv-savePaletteB;
+
+ for(i = 0; i 256; i++)
+ I915_WRITE(reg + (i 2), array[i]);
+}
+
+static u8 i915_read_indexed(u16 index_port, u16 data_port, u8 reg)
+{
+ outb(reg, index_port);
+ return inb(data_port);
+}
+
+static
[RFC] full suspend/resume support for i915 DRM driver
We seem to see a lot of bug reports along the lines of, "my machine
resumes but I can't see X" or, "I can see X but only with a bright
flashlight", etc. These sorts of problems are due to the fact that
the X server isn't designed to do full state save/restore, and none
of the available kernel drivers do it on its behalf.
Since intelfb and the rest of the Intel drivers are fairly incompatible,
this patch makes the DRM bind to the PCI device so it can register real
suspend/resume handlers. Those handlers take care of saving and
restoring enough state for X to come back reliably on at least one of my
problematic test machines, but text mode still has trouble (still
debugging VGA state save/restore, including trying to save/restore
actual VRAM contents for possible hibernate support).
How does this approach look? Is a new DRM driver flag a good thing for
similar situations with other drivers? Thoughts?
Thanks,
Jesse
diff --git a/linux-core/Kconfig b/linux-core/Kconfig
index 2d02c76..5e73fc7 100644
--- a/linux-core/Kconfig
+++ b/linux-core/Kconfig
@@ -50,7 +50,7 @@ config DRM_I810
choice
prompt "Intel 830M, 845G, 852GM, 855GM, 865G"
- depends on DRM && AGP && AGP_INTEL
+ depends on DRM && AGP && AGP_INTEL && !FB_INTEL
optional
config DRM_I915
@@ -60,6 +60,9 @@ config DRM_I915
852GM, 855GM, 865G, 915G, 915GM, 945G, 945GM and 965G integrated
graphics. If M is selected, the module will be called i915.
AGP support is required for this driver to work.
+
+ Note that this driver is incompatible with the Intel framebuffer
+ driver.
endchoice
diff --git a/linux-core/drmP.h b/linux-core/drmP.h
index f8ca3f4..36ce266 100644
--- a/linux-core/drmP.h
+++ b/linux-core/drmP.h
@@ -106,6 +106,7 @@ struct drm_file;
#define DRIVER_DMA_QUEUE 0x200
#define DRIVER_FB_DMA 0x400
#define DRIVER_IRQ_VBL20x800
+#define DRIVER_BIND_PCI0x1000
/[EMAIL PROTECTED]/
diff --git a/linux-core/drm_drv.c b/linux-core/drm_drv.c
index a09fa96..4d11707 100644
--- a/linux-core/drm_drv.c
+++ b/linux-core/drm_drv.c
@@ -340,9 +340,10 @@ int drm_init(struct drm_driver *driver,
}
}
- if (!drm_fb_loaded)
+ if (!drm_fb_loaded || (driver->driver_features & DRIVER_BIND_PCI)) {
+ printk(KERN_ERR "DRM binding to PCI device\n");
return pci_register_driver(>pci_driver);
- else {
+ } else {
for (i = 0; pciidlist[i].vendor != 0; i++) {
pid = [i];
diff --git a/linux-core/i915_drv.c b/linux-core/i915_drv.c
index e337e1d..1f99944 100644
--- a/linux-core/i915_drv.c
+++ b/linux-core/i915_drv.c
@@ -69,6 +69,621 @@ static struct drm_bo_driver i915_bo_driver = {
};
#endif
+enum pipe {
+PIPE_A = 0,
+PIPE_B,
+};
+
+static bool i915_pipe_enabled(struct drm_device *dev, enum pipe pipe)
+{
+ struct drm_i915_private *dev_priv = dev->dev_private;
+
+ if (pipe == PIPE_A)
+ return (I915_READ(DPLL_A) & DPLL_VCO_ENABLE);
+ else
+ return (I915_READ(DPLL_B) & DPLL_VCO_ENABLE);
+}
+
+static void i915_save_palette(struct drm_device *dev, enum pipe pipe)
+{
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ unsigned long reg = pipe == PIPE_A ? PALETTE_A : PALETTE_B;
+ u32 *array;
+ int i;
+
+ if (!i915_pipe_enabled(dev, pipe))
+ return;
+
+ if (pipe == PIPE_A)
+ array = dev_priv->savePaletteA;
+ else
+ array = dev_priv->savePaletteB;
+
+ for(i = 0; i < 256; i++)
+ array[i] = I915_READ(reg + (i << 2));
+}
+
+static void i915_restore_palette(struct drm_device *dev, enum pipe pipe)
+{
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ unsigned long reg = pipe == PIPE_A ? PALETTE_A : PALETTE_B;
+ u32 *array;
+ int i;
+
+ if (!i915_pipe_enabled(dev, pipe))
+ return;
+
+ if (pipe == PIPE_A)
+ array = dev_priv->savePaletteA;
+ else
+ array = dev_priv->savePaletteB;
+
+ for(i = 0; i < 256; i++)
+ I915_WRITE(reg + (i << 2), array[i]);
+}
+
+static u8 i915_read_indexed(u16 index_port, u16 data_port, u8 reg)
+{
+ outb(reg, index_port);
+ return inb(data_port);
+}
+
+static void i915_write_indexed(u16 index_port, u16 data_port, u8 reg, u8 val)
+{
+ outb(reg, index_port);
+ outb(val, data_port);
+}
+
+static void i915_save_vga(struct drm_device *dev)
+{
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ unsigned long mem_mode, mem_offset = 0, mem_size = 0;
+ u8 *mapped_offset;
+ int i;
+ u16 cr_index, cr_data, st01;
+ u8 msr;
+
+ /* MSR bits */
+
[RFC] full suspend/resume support for i915 DRM driver
We seem to see a lot of bug reports along the lines of, my machine
resumes but I can't see X or, I can see X but only with a bright
flashlight, etc. These sorts of problems are due to the fact that
the X server isn't designed to do full state save/restore, and none
of the available kernel drivers do it on its behalf.
Since intelfb and the rest of the Intel drivers are fairly incompatible,
this patch makes the DRM bind to the PCI device so it can register real
suspend/resume handlers. Those handlers take care of saving and
restoring enough state for X to come back reliably on at least one of my
problematic test machines, but text mode still has trouble (still
debugging VGA state save/restore, including trying to save/restore
actual VRAM contents for possible hibernate support).
How does this approach look? Is a new DRM driver flag a good thing for
similar situations with other drivers? Thoughts?
Thanks,
Jesse
diff --git a/linux-core/Kconfig b/linux-core/Kconfig
index 2d02c76..5e73fc7 100644
--- a/linux-core/Kconfig
+++ b/linux-core/Kconfig
@@ -50,7 +50,7 @@ config DRM_I810
choice
prompt Intel 830M, 845G, 852GM, 855GM, 865G
- depends on DRM AGP AGP_INTEL
+ depends on DRM AGP AGP_INTEL !FB_INTEL
optional
config DRM_I915
@@ -60,6 +60,9 @@ config DRM_I915
852GM, 855GM, 865G, 915G, 915GM, 945G, 945GM and 965G integrated
graphics. If M is selected, the module will be called i915.
AGP support is required for this driver to work.
+
+ Note that this driver is incompatible with the Intel framebuffer
+ driver.
endchoice
diff --git a/linux-core/drmP.h b/linux-core/drmP.h
index f8ca3f4..36ce266 100644
--- a/linux-core/drmP.h
+++ b/linux-core/drmP.h
@@ -106,6 +106,7 @@ struct drm_file;
#define DRIVER_DMA_QUEUE 0x200
#define DRIVER_FB_DMA 0x400
#define DRIVER_IRQ_VBL20x800
+#define DRIVER_BIND_PCI0x1000
/[EMAIL PROTECTED]/
diff --git a/linux-core/drm_drv.c b/linux-core/drm_drv.c
index a09fa96..4d11707 100644
--- a/linux-core/drm_drv.c
+++ b/linux-core/drm_drv.c
@@ -340,9 +340,10 @@ int drm_init(struct drm_driver *driver,
}
}
- if (!drm_fb_loaded)
+ if (!drm_fb_loaded || (driver-driver_features DRIVER_BIND_PCI)) {
+ printk(KERN_ERR DRM binding to PCI device\n);
return pci_register_driver(driver-pci_driver);
- else {
+ } else {
for (i = 0; pciidlist[i].vendor != 0; i++) {
pid = pciidlist[i];
diff --git a/linux-core/i915_drv.c b/linux-core/i915_drv.c
index e337e1d..1f99944 100644
--- a/linux-core/i915_drv.c
+++ b/linux-core/i915_drv.c
@@ -69,6 +69,621 @@ static struct drm_bo_driver i915_bo_driver = {
};
#endif
+enum pipe {
+PIPE_A = 0,
+PIPE_B,
+};
+
+static bool i915_pipe_enabled(struct drm_device *dev, enum pipe pipe)
+{
+ struct drm_i915_private *dev_priv = dev-dev_private;
+
+ if (pipe == PIPE_A)
+ return (I915_READ(DPLL_A) DPLL_VCO_ENABLE);
+ else
+ return (I915_READ(DPLL_B) DPLL_VCO_ENABLE);
+}
+
+static void i915_save_palette(struct drm_device *dev, enum pipe pipe)
+{
+ struct drm_i915_private *dev_priv = dev-dev_private;
+ unsigned long reg = pipe == PIPE_A ? PALETTE_A : PALETTE_B;
+ u32 *array;
+ int i;
+
+ if (!i915_pipe_enabled(dev, pipe))
+ return;
+
+ if (pipe == PIPE_A)
+ array = dev_priv-savePaletteA;
+ else
+ array = dev_priv-savePaletteB;
+
+ for(i = 0; i 256; i++)
+ array[i] = I915_READ(reg + (i 2));
+}
+
+static void i915_restore_palette(struct drm_device *dev, enum pipe pipe)
+{
+ struct drm_i915_private *dev_priv = dev-dev_private;
+ unsigned long reg = pipe == PIPE_A ? PALETTE_A : PALETTE_B;
+ u32 *array;
+ int i;
+
+ if (!i915_pipe_enabled(dev, pipe))
+ return;
+
+ if (pipe == PIPE_A)
+ array = dev_priv-savePaletteA;
+ else
+ array = dev_priv-savePaletteB;
+
+ for(i = 0; i 256; i++)
+ I915_WRITE(reg + (i 2), array[i]);
+}
+
+static u8 i915_read_indexed(u16 index_port, u16 data_port, u8 reg)
+{
+ outb(reg, index_port);
+ return inb(data_port);
+}
+
+static void i915_write_indexed(u16 index_port, u16 data_port, u8 reg, u8 val)
+{
+ outb(reg, index_port);
+ outb(val, data_port);
+}
+
+static void i915_save_vga(struct drm_device *dev)
+{
+ struct drm_i915_private *dev_priv = dev-dev_private;
+ unsigned long mem_mode, mem_offset = 0, mem_size = 0;
+ u8 *mapped_offset;
+ int i;
+ u16 cr_index, cr_data, st01;
+ u8 msr;
+
+ /* MSR bits */
+ msr = dev_priv-saveMSR = inb(VGA_MSR_READ);
+ if (dev_priv-saveMSR VGA_MSR_CGA_MODE) {
+ cr_index = VGA_CR_INDEX_CGA;
+ cr_data = VGA_CR_DATA_CGA
Re: [PATCH] Map volume and brightness events on thinkpads
On Tuesday, October 16, 2007 11:25 pm Henrique de Moraes Holschuh wrote: > On Tue, 16 Oct 2007, Jesse Barnes wrote: > > > Last time the issue was brought up (and I do believe it was > > > because of thinkpad-acpi :-) ), he made it clear that any events > > > you are to act upon are fine in input, but events that are just > > > notifications (i.e. the firmware already did the action) are not. > > > > Ah yeah, I agree with that. Regular events should be uevents or > > something, not input events. Actual keyboard keys though (whether > > they generate firmware event messages or actual scancodes) should > > probably go through the input layer. I thought that's what > > Jeremy's patch was doing, maybe I didn't look closely enough. > > The patch adds keycodes for "keys" that are acually notifications on > many thinkpads. And that is the problem, please refer to the rest of > the thread for more details... Yeah, just read through it. It really doesn't seem like it matters that much whether "keypress+notify" events are delivered via the input layer or via some sort of uevent. However, since we already have userspace code in place handling the input layer case, why not just go with it? It's fairly straightforward and already works in some distros. Jesse - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Map volume and brightness events on thinkpads
On Tuesday, October 16, 2007 11:25 pm Henrique de Moraes Holschuh wrote: On Tue, 16 Oct 2007, Jesse Barnes wrote: Last time the issue was brought up (and I do believe it was because of thinkpad-acpi :-) ), he made it clear that any events you are to act upon are fine in input, but events that are just notifications (i.e. the firmware already did the action) are not. Ah yeah, I agree with that. Regular events should be uevents or something, not input events. Actual keyboard keys though (whether they generate firmware event messages or actual scancodes) should probably go through the input layer. I thought that's what Jeremy's patch was doing, maybe I didn't look closely enough. The patch adds keycodes for keys that are acually notifications on many thinkpads. And that is the problem, please refer to the rest of the thread for more details... Yeah, just read through it. It really doesn't seem like it matters that much whether keypress+notify events are delivered via the input layer or via some sort of uevent. However, since we already have userspace code in place handling the input layer case, why not just go with it? It's fairly straightforward and already works in some distros. Jesse - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Map volume and brightness events on thinkpads
On Tuesday, October 16, 2007 2:18 am Henrique de Moraes Holschuh wrote: > On Tue, 16 Oct 2007, Jesse Barnes wrote: > > On Tuesday, October 16, 2007 1:36 am Henrique de Moraes Holschuh wrote: > > > You want ACPI video to just pass the messages to userspace when > > > X.org is driving the backlight? Fine with me. That *still* > > > doesn't make it right to get these messages as hot key presses > > > over the input layer through the thinkpad-acpi driver. So the > > > NAK stands. Any changes should be done to the ACPI video driver > > > in this case. > > > > So is this really the direction that input is going? Last time I > > talked with Dmitry, he seemed ok with adding input events for ACPI > > and other firmware hotkeys... > > Last time the issue was brought up (and I do believe it was because > of thinkpad-acpi :-) ), he made it clear that any events you are to > act upon are fine in input, but events that are just notifications > (i.e. the firmware already did the action) are not. Ah yeah, I agree with that. Regular events should be uevents or something, not input events. Actual keyboard keys though (whether they generate firmware event messages or actual scancodes) should probably go through the input layer. I thought that's what Jeremy's patch was doing, maybe I didn't look closely enough. Jesse - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Map volume and brightness events on thinkpads
On Tuesday, October 16, 2007 1:36 am Henrique de Moraes Holschuh wrote: > > No, on Lenovo (and in general actually) the firmware should *not* > > touch the backlight. Otherwise if another driver touches it the > > driver and > > This is not an option on IBM ThinkPads, unless you patch the DSDT on > non-ancient ACPI-based models, and unless you patch the BIOS (and > maybe even the EC control program) itself on the really ancient > models. It is that simple. > Yeah, unfortunately, "should not" doesn't mean "does not" in this case. In cases where we can't control the firmware, we have to use it to control the backlight exclusively, or we'll definitely get into trouble. > You want ACPI video to just pass the messages to userspace when X.org > is driving the backlight? Fine with me. That *still* doesn't make > it right to get these messages as hot key presses over the input > layer through the thinkpad-acpi driver. So the NAK stands. Any > changes should be done to the ACPI video driver in this case. So is this really the direction that input is going? Last time I talked with Dmitry, he seemed ok with adding input events for ACPI and other firmware hotkeys... > Good luck, and please interface it properly to the backlight class > while at it. There's no reason to make the waters mudier :-) Yeah, the backlight class needs some changes: the backlight dirs should somehow identify the display they control and only *one* backlight driver should be registered for a given display. Unfortunately we don't have enough info in the kernel to identify displays, so those changes may have to wait until the DRM grows such knowledge. Jesse - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Map volume and brightness events on thinkpads
On Tuesday, October 16, 2007 1:36 am Henrique de Moraes Holschuh wrote: No, on Lenovo (and in general actually) the firmware should *not* touch the backlight. Otherwise if another driver touches it the driver and This is not an option on IBM ThinkPads, unless you patch the DSDT on non-ancient ACPI-based models, and unless you patch the BIOS (and maybe even the EC control program) itself on the really ancient models. It is that simple. Yeah, unfortunately, should not doesn't mean does not in this case. In cases where we can't control the firmware, we have to use it to control the backlight exclusively, or we'll definitely get into trouble. You want ACPI video to just pass the messages to userspace when X.org is driving the backlight? Fine with me. That *still* doesn't make it right to get these messages as hot key presses over the input layer through the thinkpad-acpi driver. So the NAK stands. Any changes should be done to the ACPI video driver in this case. So is this really the direction that input is going? Last time I talked with Dmitry, he seemed ok with adding input events for ACPI and other firmware hotkeys... Good luck, and please interface it properly to the backlight class while at it. There's no reason to make the waters mudier :-) Yeah, the backlight class needs some changes: the backlight dirs should somehow identify the display they control and only *one* backlight driver should be registered for a given display. Unfortunately we don't have enough info in the kernel to identify displays, so those changes may have to wait until the DRM grows such knowledge. Jesse - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Map volume and brightness events on thinkpads
On Tuesday, October 16, 2007 2:18 am Henrique de Moraes Holschuh wrote: On Tue, 16 Oct 2007, Jesse Barnes wrote: On Tuesday, October 16, 2007 1:36 am Henrique de Moraes Holschuh wrote: You want ACPI video to just pass the messages to userspace when X.org is driving the backlight? Fine with me. That *still* doesn't make it right to get these messages as hot key presses over the input layer through the thinkpad-acpi driver. So the NAK stands. Any changes should be done to the ACPI video driver in this case. So is this really the direction that input is going? Last time I talked with Dmitry, he seemed ok with adding input events for ACPI and other firmware hotkeys... Last time the issue was brought up (and I do believe it was because of thinkpad-acpi :-) ), he made it clear that any events you are to act upon are fine in input, but events that are just notifications (i.e. the firmware already did the action) are not. Ah yeah, I agree with that. Regular events should be uevents or something, not input events. Actual keyboard keys though (whether they generate firmware event messages or actual scancodes) should probably go through the input layer. I thought that's what Jeremy's patch was doing, maybe I didn't look closely enough. Jesse - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Map volume and brightness events on thinkpads
On Monday, October 15, 2007 2:07 pm Henrique de Moraes Holschuh wrote: > We should fix the backlight class to be more useful and support > poll() or somesuch, for userspace to track the backlight level in a > resource-friendly way for OSD (the only sane thing to do on an IBM > thinkpad with such events). And an ALSA mixer to provide a proper > path to the thinkpad-acpi volume functionality is also in my schedule > for 2.6.25. > > As for Lenovo thinkpads, brightness control is to be processed by the > ACPI video module, so brightness hot keys are not to be reported by > default there either. I am not so sure about the volume keys, but > your patch touches the IBM keymap *and* you provide no testing > information for the various Lenovo models, so I have to NAK it as > well until more information is available. No, on Lenovo (and in general actually) the firmware should *not* touch the backlight. Otherwise if another driver touches it the driver and firmware will be out of sync, causing unexpected and undesirable behavior. We intend to fix this for the Intel driver at least (requiring both ACPI video driver and gfx driver updates), others will probably follow eventually. Jesse - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Map volume and brightness events on thinkpads
On Monday, October 15, 2007 2:07 pm Henrique de Moraes Holschuh wrote: We should fix the backlight class to be more useful and support poll() or somesuch, for userspace to track the backlight level in a resource-friendly way for OSD (the only sane thing to do on an IBM thinkpad with such events). And an ALSA mixer to provide a proper path to the thinkpad-acpi volume functionality is also in my schedule for 2.6.25. As for Lenovo thinkpads, brightness control is to be processed by the ACPI video module, so brightness hot keys are not to be reported by default there either. I am not so sure about the volume keys, but your patch touches the IBM keymap *and* you provide no testing information for the various Lenovo models, so I have to NAK it as well until more information is available. No, on Lenovo (and in general actually) the firmware should *not* touch the backlight. Otherwise if another driver touches it the driver and firmware will be out of sync, causing unexpected and undesirable behavior. We intend to fix this for the Intel driver at least (requiring both ACPI video driver and gfx driver updates), others will probably follow eventually. Jesse - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 0/4] allow drivers to flush in-flight DMA v2
On Tuesday, October 2, 2007 6:04 pm [EMAIL PROTECTED] wrote: > On Fri, Sep 28, 2007 at 03:43:39PM -0700, David Miller wrote: > > > > My only beef with this patch set is that it seems > > a bit much to create a totally new function name every > > time we want to set some kind of new attribute on some > > DMA object. Why not add a "dma_set_flags()" or similar > > that can be used later on to set other kinds of aspects > > we'd like to change? > > > > You can make the arguments "u32 flags" and "int dir". > > Actually you should probably use the dma direction > > enumaration instead of 'int'. > > OK, this will be in the next version along with the > coding style changes you mentioned. I can't find it now, but IIRC Dave, you vehemently opposed adding a flags argument awhile back (~2000), so SGI has been reluctant to push it again. :) Glad you're finally coming around, I think a flags argument makes sense, as long as we're careful about adding new ones. Jesse - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 0/4] allow drivers to flush in-flight DMA v2
On Tuesday, October 2, 2007 6:04 pm [EMAIL PROTECTED] wrote: On Fri, Sep 28, 2007 at 03:43:39PM -0700, David Miller wrote: My only beef with this patch set is that it seems a bit much to create a totally new function name every time we want to set some kind of new attribute on some DMA object. Why not add a dma_set_flags() or similar that can be used later on to set other kinds of aspects we'd like to change? You can make the arguments u32 flags and int dir. Actually you should probably use the dma direction enumaration instead of 'int'. OK, this will be in the next version along with the coding style changes you mentioned. I can't find it now, but IIRC Dave, you vehemently opposed adding a flags argument awhile back (~2000), so SGI has been reluctant to push it again. :) Glad you're finally coming around, I think a flags argument makes sense, as long as we're careful about adding new ones. Jesse - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] i915: make vbl interrupts work properly on i965g/gm
On Friday, September 28, 2007 3:23 pm Roland Dreier wrote: > > I don't have a 945 to test with, but Dave might... > > Actually I'm not looking for testing (I can test fine on my own > laptop :) I was just hoping someone with docs could tell me "MSI is > documented to be broken on 945GM" or "Oh yeah, 945GM requires you to > set SECRET_MSI_ENABLE_BIT in register FOO before it generates MSIs." > > As I said, it's not a big deal but I'd sort of like to know whether > my MSI enable hack at least has a chance at working before I try too > hard to get it working. I don't see anything in the docs (either public or private) that would indicate that MSI is broken on those chips, so I'd expect it to work... Jesse - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] i915: make vbl interrupts work properly on i965g/gm
On Friday, September 28, 2007 1:02 pm Roland Dreier wrote: > Sorry to hijack this thread, but I have a question about i915 > interrupts (945GM specifically) that I haven't gotten an answer to > yet, and this thread reminded me of it so I thought I'd ask again: > I hacked the kernel drm stuff on my laptop with 945GM graphics to do > pci_enable_msi() on the graphics device, and I don't see any > interrupts occur. > > So can someone with access to details of the 945GM tell me if MSI > interrupts should work, and if so if there's some internal registers > that need to be poked to enable it (beyond what pci_enable_msi() > does). > > It's not super-urgent-- I just like using shiny things like MSI, and > I already have ahci, iwl3945 and e1000 working with MSI on my laptop, > so the graphics are the only thing left with an MSI capability that's > not working. I don't have a 945 to test with, but Dave might... Jesse - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] i915: make vbl interrupts work properly on i965g/gm
On Friday, September 28, 2007 1:02 pm Roland Dreier wrote: Sorry to hijack this thread, but I have a question about i915 interrupts (945GM specifically) that I haven't gotten an answer to yet, and this thread reminded me of it so I thought I'd ask again: I hacked the kernel drm stuff on my laptop with 945GM graphics to do pci_enable_msi() on the graphics device, and I don't see any interrupts occur. So can someone with access to details of the 945GM tell me if MSI interrupts should work, and if so if there's some internal registers that need to be poked to enable it (beyond what pci_enable_msi() does). It's not super-urgent-- I just like using shiny things like MSI, and I already have ahci, iwl3945 and e1000 working with MSI on my laptop, so the graphics are the only thing left with an MSI capability that's not working. I don't have a 945 to test with, but Dave might... Jesse - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] i915: make vbl interrupts work properly on i965g/gm
On Friday, September 28, 2007 3:23 pm Roland Dreier wrote: I don't have a 945 to test with, but Dave might... Actually I'm not looking for testing (I can test fine on my own laptop :) I was just hoping someone with docs could tell me MSI is documented to be broken on 945GM or Oh yeah, 945GM requires you to set SECRET_MSI_ENABLE_BIT in register FOO before it generates MSIs. As I said, it's not a big deal but I'd sort of like to know whether my MSI enable hack at least has a chance at working before I try too hard to get it working. I don't see anything in the docs (either public or private) that would indicate that MSI is broken on those chips, so I'd expect it to work... Jesse - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] i915: make vbl interrupts work properly on i965g/gm
On Thursday, September 27, 2007 7:05:31 pm Dave Airlie wrote: > Hi Linus, > > The attached patch is to fix a bug reported on 965gm chipsets (lots of new > laptops), I think distros will all have to patch this in to fix it, so can > we get it into the 2.6.23 final? > > (Otherwise I'll wait until stable..) Without this patch, my 965GM drops vblank interrupts, so I'd really like to see it upstream ASAP too. Acked-by: Jesse Barnes <[EMAIL PROTECTED]> - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] i915: make vbl interrupts work properly on i965g/gm
On Thursday, September 27, 2007 7:05:31 pm Dave Airlie wrote: Hi Linus, The attached patch is to fix a bug reported on 965gm chipsets (lots of new laptops), I think distros will all have to patch this in to fix it, so can we get it into the 2.6.23 final? (Otherwise I'll wait until stable..) Without this patch, my 965GM drops vblank interrupts, so I'd really like to see it upstream ASAP too. Acked-by: Jesse Barnes [EMAIL PROTECTED] - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: PCI: Fix boot-time hang on G31/G33 PC
On Wednesday, September 26, 2007 2:56 pm Greg KH wrote: > On Wed, Sep 26, 2007 at 02:55:55PM -0700, Jesse Barnes wrote: > > On Wednesday, September 26, 2007 2:18 pm Greg KH wrote: > > > Due to the issues surrounding this patch, I'm dropping it from my > > > repo. > > > > What issues? Is it causing problems for people? > > I thought this was the patch that Ivan objected to. Yeah, Ivan objected to this, but incorrectly I think. Ivan, your concern is about disabling things like interrupt controllers and power management chips during probe right? You're right that doing that could cause problems if we get and interrupt or PMU event at just the wrong time, but that could just as easily happen if decode was still enabled but the BAR had a bogus address programmed (as it would during probing). Ultimately, I don't care much one way or another as long as we can get the desktop platforms fixed somehow. I think disabling decode is the most correct way of doing this, but I'm open to other solutions (this is the only patch I've seen though that's been tested to solve the problem). Jesse - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: PCI: Fix boot-time hang on G31/G33 PC
On Wednesday, September 26, 2007 2:18 pm Greg KH wrote: > Due to the issues surrounding this patch, I'm dropping it from my > repo. What issues? Is it causing problems for people? Jesse - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 0/4] allow drivers to flush in-flight DMA
On Tuesday, September 25, 2007 11:49:50 pm Grant Grundler wrote: > Upon reading the "2) Platforms that permit DMA reordering", I think I > have been confusing coherency with ordering. I think I have because DMA > is leaving the "PCI domain", crossing an "unordered domain" (NUMA, > interconnect), and then finally hitting the cache coherency "domain" > when it reaches a "far away" memory controller. That's why I've > been thinking of this as a coherency problem. > > The description and API uses the word "flush" (which is ok I guess) instead > of describing this in terms of enforcing DMA ordering. Any DMA write to > the "strongly ordered" region will cause _all_ inflight DMA to be visible > to cache coherency, thus preserving the illusion of strong DMA ordering. > > Does that sound right/better to you too? > I don't have chipset docs and some of this is just trying to rephrase > what I've heard before from former SGI employees. I definitely wouldn't describe this as a coherency issue--the lines involved in the DMA writes are fully coherent. It's really an ordering problem, and the new API is setting a "barrier" bit in the DMA address that indicates to the bridge that any outstanding DMA should be written before the barriered data. So calling it set_flush or set_barrier is fine with me... Jesse - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
