* [git pull] drm: previous pull req + 1. @ 2009-06-20 5:23 Dave Airlie 2009-06-21 0:04 ` Maxim Levitsky 2009-06-21 3:41 ` Andy Lutomirski 0 siblings, 2 replies; 40+ messages in thread From: Dave Airlie @ 2009-06-20 5:23 UTC (permalink / raw) To: torvalds; +Cc: dri-devel, linux-kernel [-- Attachment #1: Type: TEXT/PLAIN, Size: 15440 bytes --] Hi Linus, Please pull the 'drm-linus' branch from ssh://master.kernel.org/pub/scm/linux/kernel/git/airlied/drm-2.6.git drm-linus This is the same tree from the previous pull request + the fix from Pierre that actually makes the PAE/GEM combination on i965 work. \o/ Dave. drivers/char/agp/agp.h | 12 +- drivers/char/agp/ali-agp.c | 28 +- drivers/char/agp/amd-k7-agp.c | 2 +- drivers/char/agp/amd64-agp.c | 2 +- drivers/char/agp/ati-agp.c | 23 +- drivers/char/agp/backend.c | 8 +- drivers/char/agp/efficeon-agp.c | 5 +- drivers/char/agp/generic.c | 69 ++--- drivers/char/agp/hp-agp.c | 9 +- drivers/char/agp/i460-agp.c | 47 ++-- drivers/char/agp/intel-agp.c | 49 ++-- drivers/char/agp/nvidia-agp.c | 2 +- drivers/char/agp/parisc-agp.c | 20 +- drivers/char/agp/sgi-agp.c | 9 +- drivers/char/agp/sworks-agp.c | 2 +- drivers/char/agp/uninorth-agp.c | 30 ++- drivers/gpu/drm/drm_agpsupport.c | 14 +- drivers/gpu/drm/drm_auth.c | 4 +- drivers/gpu/drm/drm_bufs.c | 140 ++++------ drivers/gpu/drm/drm_context.c | 4 +- drivers/gpu/drm/drm_debugfs.c | 9 +- drivers/gpu/drm/drm_dma.c | 31 +-- drivers/gpu/drm/drm_drawable.c | 25 +- drivers/gpu/drm/drm_drv.c | 18 +- drivers/gpu/drm/drm_edid.c | 100 ++++--- drivers/gpu/drm/drm_fops.c | 8 +- drivers/gpu/drm/drm_gem.c | 8 +- drivers/gpu/drm/drm_hashtab.c | 6 +- drivers/gpu/drm/drm_ioctl.c | 14 +- drivers/gpu/drm/drm_irq.c | 44 +-- drivers/gpu/drm/drm_memory.c | 33 +-- drivers/gpu/drm/drm_mm.c | 48 +--- drivers/gpu/drm/drm_pci.c | 53 +---- drivers/gpu/drm/drm_proc.c | 8 +- drivers/gpu/drm/drm_scatter.c | 33 +-- drivers/gpu/drm/drm_sman.c | 29 +- drivers/gpu/drm/drm_stub.c | 19 +- drivers/gpu/drm/drm_vm.c | 12 +- drivers/gpu/drm/i810/i810_dma.c | 6 +- drivers/gpu/drm/i830/i830_dma.c | 6 +- drivers/gpu/drm/i915/i915_dma.c | 45 +-- drivers/gpu/drm/i915/i915_drv.h | 2 + drivers/gpu/drm/i915/i915_gem.c | 86 ++++-- drivers/gpu/drm/i915/i915_gem_debugfs.c | 4 +- drivers/gpu/drm/i915/i915_gem_tiling.c | 67 ++++- drivers/gpu/drm/i915/i915_mem.c | 24 +- drivers/gpu/drm/i915/intel_bios.c | 6 +- drivers/gpu/drm/i915/intel_display.c | 20 +- drivers/gpu/drm/i915/intel_fb.c | 6 +- drivers/gpu/drm/i915/intel_tv.c | 11 +- drivers/gpu/drm/mga/mga_dma.c | 14 +- drivers/gpu/drm/r128/r128_cce.c | 12 +- drivers/gpu/drm/r128/r128_state.c | 84 +++--- drivers/gpu/drm/radeon/r100.c | 85 +++--- drivers/gpu/drm/radeon/r300.c | 478 ++++++++++++++++++++++++++++-- drivers/gpu/drm/radeon/r300.h | 36 +++ drivers/gpu/drm/radeon/radeon.h | 9 +- drivers/gpu/drm/radeon/radeon_asic.h | 15 +- drivers/gpu/drm/radeon/radeon_atombios.c | 2 - drivers/gpu/drm/radeon/radeon_combios.c | 9 + drivers/gpu/drm/radeon/radeon_cp.c | 9 +- drivers/gpu/drm/radeon/radeon_device.c | 4 + drivers/gpu/drm/radeon/radeon_display.c | 2 +- drivers/gpu/drm/radeon/radeon_drv.c | 2 +- drivers/gpu/drm/radeon/radeon_i2c.c | 6 +- drivers/gpu/drm/radeon/radeon_kms.c | 4 +- drivers/gpu/drm/radeon/radeon_mem.c | 24 +- drivers/gpu/drm/radeon/radeon_reg.h | 1 + drivers/gpu/drm/radeon/radeon_state.c | 16 +- drivers/gpu/drm/radeon/radeon_ttm.c | 8 +- drivers/gpu/drm/radeon/rv515.c | 58 ++++ drivers/gpu/drm/savage/savage_bci.c | 21 +- drivers/gpu/drm/savage/savage_state.c | 17 +- drivers/gpu/drm/sis/sis_drv.c | 6 +- drivers/gpu/drm/ttm/ttm_agp_backend.c | 3 +- drivers/gpu/drm/ttm/ttm_bo.c | 11 +- drivers/gpu/drm/ttm/ttm_tt.c | 11 +- drivers/gpu/drm/via/via_map.c | 8 +- include/drm/drmP.h | 52 ---- include/drm/drm_edid.h | 92 +++--- include/drm/drm_memory_debug.h | 309 ------------------- include/drm/drm_mm.h | 21 +- include/linux/agp_backend.h | 2 +- 83 files changed, 1391 insertions(+), 1300 deletions(-) create mode 100644 drivers/gpu/drm/radeon/r300.h delete mode 100644 include/drm/drm_memory_debug.h commit 0b7af262aba912f52bc6ef76f1bc0960b01b8502 Author: Pierre Willenbrock <pierre@pirsoft.de> Date: Fri Jun 19 18:31:47 2009 +0200 agp/intel: Make intel_i965_mask_memory use dma_addr_t for physical addresses Otherwise, the high bits to be stuffed in the unused lower bits of the page address are lost. Signed-off-by: Pierre Willenbrock <pierre@pirsoft.de> Signed-off-by: Dave Airlie <airlied@redhat.com> commit a95fe463e73b8c7b2d97606ac86ce261f1270726 Author: Dave Airlie <airlied@redhat.com> Date: Fri Jun 19 10:52:57 2009 +1000 agp: add user mapping support to ATI AGP bridge. This should fix TTM/KMS on some of the original ATI IGP chipsets. (rs100/rs200) Signed-off-by: Dave Airlie <airlied@redhat.com> commit 95934f939c46ea2b37f3c91a4f8c82e003727761 Author: Dave Airlie <airlied@redhat.com> Date: Fri Jun 19 10:29:20 2009 +1000 drm/i915: enable GEM on PAE. In theory now that the AGP subsystem is using struct page, we should have on problems enabling GEM on PAE systems. Signed-off-by: Dave Airlie <airlied@redhat.com> commit fc436d9d3720b382566e03bef2d7391a58714999 Author: Dave Airlie <airlied@redhat.com> Date: Fri Jun 19 10:22:21 2009 +1000 drm/radeon: fix unused variables warning just remove i variable left over from previous code. Signed-off-by: Dave Airlie <airlied@redhat.com> commit 07613ba2f464f59949266f4337b75b91eb610795 Author: Dave Airlie <airlied@redhat.com> Date: Fri Jun 12 14:11:41 2009 +1000 agp: switch AGP to use page array instead of unsigned long array This switches AGP to use an array of pages for tracking the pages allocated to the GART. This should enable GEM on PAE to work a lot better as we can pass highmem pages to the PAT code and it will do the right thing with them. Signed-off-by: Dave Airlie <airlied@redhat.com> commit 2908826d045a89805714e0a3055a99dc40565d41 Author: Ondrej Zary <linux@rainbow-software.org> Date: Wed Jun 10 12:41:11 2009 -0700 agpgart: detected ALi M???? chipset with M1621 Add M1621 chipset name to ali-agp, preventing "Detected ALi M???? chipset" message. Signed-off-by: Ondrej Zary <linux@rainbow-software.org> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Dave Airlie <airlied@redhat.com> commit 068a117ca38f27c9641db7642f24fe9270d9424e Author: Jerome Glisse <glisse@freedesktop.org> Date: Wed Jun 17 13:28:30 2009 +0200 drm/radeon: command stream checker for r3xx-r5xx hardware For security purpose we want to make sure the userspace process doesn't access memory beyond buffer it owns. To achieve this we need to check states the userspace program. For color buffer and zbuffer we check that the clipping register will discard access beyond buffers set as color or zbuffer. For vertex buffer we check that no vertex fetch will happen beyond buffer end. For texture we check various texture states (number of mipmap level, texture size, texture depth, ...) to compute the amount of memory the texture fetcher might access. The command stream checking impact the performances so far quick benchmark shows an average of 3% decrease in fps of various applications. It can be optimized a bit more by caching result of checking and thus avoid a full recheck if no states changed since last check. Note that this patch is still incomplete on checking side as it doesn't check 2d rendering states. Signed-off-by: Jerome Glisse <jglisse@redhat.com> Signed-off-by: Dave Airlie <airlied@redhat.com> commit 8b5c744485b75d940ccb1c83c9a358b20eb91346 Author: Michel Dänzer <daenzer@vmware.com> Date: Wed Jun 17 18:28:38 2009 +0200 drm/radeon: Fully initialize LVDS info also when we can't get it from the ROM. This makes the panel on my PowerBook light up. Signed-off-by: Dave Airlie <airlied@redhat.com> commit 4e484e7dc5856ff5086b6329d82e36d4adaf1f02 Author: Michel Dänzer <daenzer@vmware.com> Date: Tue Jun 16 17:29:06 2009 +0200 radeon: Fix CP byte order on big endian architectures with KMS. Signed-off-by: Dave Airlie <airlied@redhat.com> commit 62369028c7e2039b821799b3db52f0d622f0e8b5 Author: Michel Dänzer <daenzer@vmware.com> Date: Mon Jun 15 16:56:15 2009 +0200 agp/uninorth: Handle user memory types. This adds support for TTM to the uninorth AGP bridge. Signed-off-by: Dave Airlie <airlied@redhat.com> commit 46f4b3eab73e621bc239bfa62ebdc44dcc0a877a Author: Michel Dänzer <daenzer@vmware.com> Date: Mon Jun 15 16:56:13 2009 +0200 drm/ttm: Add some powerpc cache flush code. Optimise the powerpc flushing path for TTM. Signed-off-by: Dave Airlie <airlied@redhat.com> commit 919f32f1df228723f66bf5c5aed23e0ab076b1a1 Author: Michel Dänzer <daenzer@vmware.com> Date: Mon Jun 15 16:56:09 2009 +0200 radeon: Enable modesetting on non-x86. Signed-off-by: Dave Airlie <airlied@redhat.com> commit 55c93278ec03c349af7b01933655b31c7f740df4 Author: Michel Dänzer <daenzer@vmware.com> Date: Mon Jun 15 16:56:11 2009 +0200 drm/radeon: Respect AGP cant_use_aperture flag. Some AGP devices can't map the aperture, radeon needs to tell TTM this. Signed-off-by: Dave Airlie <airlied@redhat.com> commit 0454beab0f6bc6d350860abd549b86959d2f6f40 Author: Michel Dänzer <daenzer@vmware.com> Date: Mon Jun 15 16:56:07 2009 +0200 drm: EDID endianness fixes. Mostly replacing bitfields with explicit masks and shifts. Signed-off-by: Dave Airlie <airlied@redhat.com> commit 00fa28ae29f70c9f26023f9922c4d2e1ca1297e3 Author: Dave Airlie <airlied@itt42.(none)> Date: Thu Jun 18 18:08:33 2009 +1000 drm/radeon: this VRAM vs aperture test is wrong, just remove it. Its quite valid to have VRAM < aperture size. Signed-off-by: Dave Airlie <airlied@redhat.com> commit 87ef92092fd092936535ba057ee19b97bb6a709a Author: Thomas Hellstrom <thellstrom@vmware.com> Date: Wed Jun 17 12:29:57 2009 +0200 drm/ttm: fix an error path to exit function correctly Just a goto instead of a direct exit. Signed-off-by: Thomas Hellstrom <thellstrom@vmware.com> Signed-off-by: Dave Airlie <airlied@redhat.com> commit 89579f778266d5a4d08d0c64c46b1565218de9f9 Author: Thomas Hellstrom <thellstrom@vmware.com> Date: Wed Jun 17 12:29:56 2009 +0200 drm: Apply "Memory fragmentation from lost alignment blocks" also for the atomic path by using a common code-path. Signed-off-by: Thomas Hellstrom <thellstrom@vmware.com> Signed-off-by: Dave Airlie <airlied@redhat.com> commit 78ecf091aa592a9e160ebbbfa5873c2bb2e2d0f8 Author: Thomas Hellstrom <thellstrom@vmware.com> Date: Wed Jun 17 12:29:55 2009 +0200 ttm: Return -ERESTART when a signal interrupts bo eviction. A bug caused the ttm code to just terminate the wait when a signal was received while waiting for the GPU to release a buffer object that was to be evicted. Signed-off-by: Thomas Hellstrom <thellstrom@vmware.com> Signed-off-by: Dave Airlie <airlied@redhat.com> commit 9a298b2acd771d8a5c0004d8f8e4156c65b11f6b Author: Eric Anholt <eric@anholt.net> Date: Tue Mar 24 12:23:04 2009 -0700 drm: Remove memory debugging infrastructure. It hasn't been used in ages, and having the user tell your how much memory is being freed at free time is a recipe for disaster even if it was ever used. Signed-off-by: Eric Anholt <eric@anholt.net> commit 52dc7d32b88156248167864f77a9026abe27b432 Author: Chris Wilson <chris@chris-wilson.co.uk> Date: Sat Jun 6 09:46:01 2009 +0100 drm/i915: Clear fence register on tiling stride change. The fence register value also depends upon the stride of the object, so we need to clear the fence if that is changed as well. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> [anholt: Added 8xx and 965 paths, and renamed the confusing i915_gem_object_tiling_ok function to i915_gem_object_fence_offset_ok] Signed-off-by: Eric Anholt <eric@anholt.net> commit 8c4b8c3f34de4e2da20df042bba173fe557f8b45 Author: Chris Wilson <chris@chris-wilson.co.uk> Date: Wed Jun 17 22:08:52 2009 +0100 drm/i915: Install fence register for tiled scanout on i915 With the work by Jesse Barnes to eliminate allocation of fences during execbuffer, it becomes possible to write to the scan-out buffer with it never acquiring a fence (simply by only ever writing to the object using tiled GPU commands and never writing to it via the GTT). So for pre-i965 chipsets which require fenced access for tiled scan-out buffers, we need to obtain a fence register. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Signed-off-by: Eric Anholt <eric@anholt.net> commit d78b47b9a527bf46cb6081555847facd6efd5f81 Author: Chris Wilson <chris@chris-wilson.co.uk> Date: Wed Jun 17 21:52:49 2009 +0100 drm/i915: detach/attach get/put pages symmetry After performing an operation over the page list for a buffer retrieved by i915_gem_object_get_pages() the pages need to be returned with i915_gem_object_put_pages(). This was not being observed for the phys objects which were thus leaking references to their backing pages. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> CC: Dave Airlie <airlied@gmail.com> Signed-off-by: Eric Anholt <eric@anholt.net> commit 76cff81ad1cfa3bd8b52b5e4510702ce2ed28335 Author: Ben Gamari <bgamari.foss@gmail.com> Date: Wed Jun 10 18:26:20 2009 -0400 drm/i915: A few debugfs formatting fixes Signed-Off-By: Ben Gamari <bgamari.foss@gmail.com> Signed-off-by: Eric Anholt <eric@anholt.net> commit 049b77cb2ad8bd36308a4a424ca4f2eb4d65d2af Author: Ben Gamari <bgamari.foss@gmail.com> Date: Thu Jun 11 00:44:26 2009 -0400 drm/i915: Warn when inteldrmfb fails to restore its framebuffer config While sifting through the inteldrmfb code trying to solve #22040 I found that the fb restore path doesn't check the return value of drm_crtc_helper_set_config(), which seems to have all sorts of potential failure modes. We should warn someone if one of these is triggered. Signed-Off-By: Ben Gamari <bgamari.foss@gmail.com> Acked-by: Jesse Barnes <jbarnes@virtuousgeek.org> [anholt: hand-applied, failures are mine] Signed-off-by: Eric Anholt <eric@anholt.net> ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [git pull] drm: previous pull req + 1. 2009-06-20 5:23 [git pull] drm: previous pull req + 1 Dave Airlie @ 2009-06-21 0:04 ` Maxim Levitsky 2009-06-21 0:42 ` Linus Torvalds 2009-06-21 1:33 ` Dave Airlie 2009-06-21 3:41 ` Andy Lutomirski 1 sibling, 2 replies; 40+ messages in thread From: Maxim Levitsky @ 2009-06-21 0:04 UTC (permalink / raw) To: Dave Airlie; +Cc: torvalds, dri-devel, linux-kernel On Sat, 2009-06-20 at 06:23 +0100, Dave Airlie wrote: > Hi Linus, > > Please pull the 'drm-linus' branch from > ssh://master.kernel.org/pub/scm/linux/kernel/git/airlied/drm-2.6.git drm-linus > > This is the same tree from the previous pull request + the fix from Pierre > that actually makes the PAE/GEM combination on i965 work. \o/ Something from this tree breaks my i965. Using -git just before this was pulled, a552f0af753eb4b5bbbe9eff205fe874b04c4583 works, but using latest git makes google earth stall, it doesn't update its main window. It appears that openining and closing its menu, allows it to progress frame after frame. No crashes hangs however. Used latest -git of all graphic components (mesa, libdrm, xserver, intel driver) KMS is used, as well as UXA and DRI2 (this is enabled by default now) Compiz used as well, but this happens without it as well. Best regards, Maxim Levitsky ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [git pull] drm: previous pull req + 1. 2009-06-21 0:04 ` Maxim Levitsky @ 2009-06-21 0:42 ` Linus Torvalds 2009-06-21 14:47 ` Maxim Levitsky 2009-06-21 1:33 ` Dave Airlie 1 sibling, 1 reply; 40+ messages in thread From: Linus Torvalds @ 2009-06-21 0:42 UTC (permalink / raw) To: Maxim Levitsky; +Cc: Dave Airlie, dri-devel, linux-kernel On Sun, 21 Jun 2009, Maxim Levitsky wrote: > > Something from this tree breaks my i965. > Using -git just before this was pulled, > > a552f0af753eb4b5bbbe9eff205fe874b04c4583 works, but using latest git > > makes google earth stall, it doesn't update its main window. It appears > that openining and closing its menu, allows it to progress frame after > frame. No crashes hangs however. Can you bisect? There's not a tons of commit there, so it shouldn't be more than a couple of recompiles/reboots, and you'd be able to pinpoint the exact commit that breaks. That will help people figure it out, or at worst just pinpoint what we need to revert. Linus ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [git pull] drm: previous pull req + 1. 2009-06-21 0:42 ` Linus Torvalds @ 2009-06-21 14:47 ` Maxim Levitsky 2009-06-21 21:24 ` Chris Wilson 0 siblings, 1 reply; 40+ messages in thread From: Maxim Levitsky @ 2009-06-21 14:47 UTC (permalink / raw) To: Linus Torvalds; +Cc: Maxim Levitsky, Dave Airlie, dri-devel, linux-kernel On Sat, 2009-06-20 at 17:42 -0700, Linus Torvalds wrote: > > On Sun, 21 Jun 2009, Maxim Levitsky wrote: > > > > Something from this tree breaks my i965. > > Using -git just before this was pulled, > > > > a552f0af753eb4b5bbbe9eff205fe874b04c4583 works, but using latest git > > > > makes google earth stall, it doesn't update its main window. It appears > > that openining and closing its menu, allows it to progress frame after > > frame. No crashes hangs however. > > Can you bisect? There's not a tons of commit there, so it shouldn't be > more than a couple of recompiles/reboots, and you'd be able to pinpoint > the exact commit that breaks. That will help people figure it out, or at > worst just pinpoint what we need to revert. > > Linus Here the result: > 52dc7d32b88156248167864f77a9026abe27b432 is first bad commit > commit 52dc7d32b88156248167864f77a9026abe27b432 > Author: Chris Wilson <chris@chris-wilson.co.uk> > Date: Sat Jun 6 09:46:01 2009 +0100 > > drm/i915: Clear fence register on tiling stride change. > > The fence register value also depends upon the stride of the object, so we > need to clear the fence if that is changed as well. > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > [anholt: Added 8xx and 965 paths, and renamed the confusing > i915_gem_object_tiling_ok function to i915_gem_object_fence_offset_ok] > Signed-off-by: Eric Anholt <eric@anholt.net> > However I can't reproduce the situation I have earlier, maybe I have changed some settings, don't know. Now, the bad behavior (and I reproduced it many times, is that GE shows incorrect textures (like they are divided in tiny interlaced rows, one row ok, other contain image from other part of world), only few textures are such it seems logical that this is related to tiling. Also, if I maximize it, it hangs. This seems to be a separate bug introduced by these series. commit 43813f399c72aa22e01a680559c1cb5274bf2140 both textures and maximize broken commit 52dc7d32b88156248167864f77a9026abe27b432, shows this incorect textures, but doesn't hang the system on maximize commit 8c4b8c3f34de4e2da20df042bba173fe557f8b45 works just fine Unfortunaly I have no time now to do another bisect now, I try to do such as soon as possible. Thanks, Maxim Levitsky ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [git pull] drm: previous pull req + 1. 2009-06-21 14:47 ` Maxim Levitsky @ 2009-06-21 21:24 ` Chris Wilson 2009-06-22 18:09 ` Maxim Levitsky 0 siblings, 1 reply; 40+ messages in thread From: Chris Wilson @ 2009-06-21 21:24 UTC (permalink / raw) To: Maxim Levitsky; +Cc: Linus Torvalds, Dave Airlie, linux-kernel, dri-devel On Sun, 2009-06-21 at 17:47 +0300, Maxim Levitsky wrote: > > 52dc7d32b88156248167864f77a9026abe27b432 is first bad commit > > commit 52dc7d32b88156248167864f77a9026abe27b432 > > Author: Chris Wilson <chris@chris-wilson.co.uk> > > Date: Sat Jun 6 09:46:01 2009 +0100 The error here seems to be my presumption that only the i915 was using fences for GPU access. (In hindsight, it seems obvious that we do not know why the fence was allocated for the object and so if it has outstanding rendering, we must assume that it is using a fence for a rendering op.) To confirm, please can you try: diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index fd2b8bd..0735518 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -2347,7 +2347,7 @@ i915_gem_object_put_fence_reg(struct drm_gem_object *obj) * therefore we must wait for any outstanding access to complete * before clearing the fence. */ - if (!IS_I965G(dev)) { + if (1) { int ret; i915_gem_object_flush_gpu_write_domain(obj); (I'm travelling tomorrow, so if this does turn out to be the fault, I'd appreciate it if someone wrote it up as a complete patch.) > However I can't reproduce the situation I have earlier, maybe I have changed some settings, don't know. > Now, the bad behavior (and I reproduced it many times, is that GE shows incorrect textures > (like they are divided in tiny interlaced rows, one row ok, other contain image from other part of world), only few textures are such > it seems logical that this is related to tiling. What you describe is exactly the artefact you would see when you access a tiled texture using linear addressing. Pretty conclusive evidence that we do need to flush outstanding GPU access. > Also, if I maximize it, it hangs. This seems to be a separate bug introduced by these series. It does seem like a separate bug. Hopefully, we can get a better handle on it after we fix this obnoxious tiling issue. -ickle ^ permalink raw reply related [flat|nested] 40+ messages in thread
* Re: [git pull] drm: previous pull req + 1. 2009-06-21 21:24 ` Chris Wilson @ 2009-06-22 18:09 ` Maxim Levitsky 2009-06-29 7:57 ` Chris Wilson 0 siblings, 1 reply; 40+ messages in thread From: Maxim Levitsky @ 2009-06-22 18:09 UTC (permalink / raw) To: Chris Wilson Cc: Maxim Levitsky, Linus Torvalds, Dave Airlie, linux-kernel, dri-devel On Sun, 2009-06-21 at 22:24 +0100, Chris Wilson wrote: > On Sun, 2009-06-21 at 17:47 +0300, Maxim Levitsky wrote: > > > 52dc7d32b88156248167864f77a9026abe27b432 is first bad commit > > > commit 52dc7d32b88156248167864f77a9026abe27b432 > > > Author: Chris Wilson <chris@chris-wilson.co.uk> > > > Date: Sat Jun 6 09:46:01 2009 +0100 > > The error here seems to be my presumption that only the i915 was using > fences for GPU access. (In hindsight, it seems obvious that we do not > know why the fence was allocated for the object and so if it has > outstanding rendering, we must assume that it is using a fence for a > rendering op.) > > To confirm, please can you try: > > diff --git a/drivers/gpu/drm/i915/i915_gem.c > b/drivers/gpu/drm/i915/i915_gem.c > index fd2b8bd..0735518 100644 > --- a/drivers/gpu/drm/i915/i915_gem.c > +++ b/drivers/gpu/drm/i915/i915_gem.c > @@ -2347,7 +2347,7 @@ i915_gem_object_put_fence_reg(struct > drm_gem_object *obj) > * therefore we must wait for any outstanding access to complete > * before clearing the fence. > */ > - if (!IS_I965G(dev)) { > + if (1) { > int ret; > > i915_gem_object_flush_gpu_write_domain(obj); Nope, same thing. I use commit 87ef92092fd092936535ba057ee19b97bb6a709a + this patch Note that GE doesn't hang the system when maximizing it. It is for sure tiled textures accessed incorrectly, same behavior observed in many games (they still run though) Best regards, Maxim Levitsky ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [git pull] drm: previous pull req + 1. 2009-06-22 18:09 ` Maxim Levitsky @ 2009-06-29 7:57 ` Chris Wilson 2009-06-30 9:49 ` Chris Wilson 2009-07-09 23:11 ` Eric Anholt 0 siblings, 2 replies; 40+ messages in thread From: Chris Wilson @ 2009-06-29 7:57 UTC (permalink / raw) To: Maxim Levitsky Cc: Linus Torvalds, Dave Airlie, linux-kernel, dri-devel, Eric Anholt On Mon, 2009-06-22 at 21:09 +0300, Maxim Levitsky wrote: > Nope, same thing. > > I use commit 87ef92092fd092936535ba057ee19b97bb6a709a + this patch > Note that GE doesn't hang the system when maximizing it. > > It is for sure tiled textures accessed incorrectly, same behavior > observed in many games (they still run though) Sorry for the delay, I was travelling last week and was sure I had nailed the problem. Aside from the missing flush on i965 when a batch buffer might be using fenced commands, the only other issue I found was that we did not zap the mapping range on clear - which could also cause tiling errors if textures were being written via a GTT mmap. So please can you try this patch: >From 20b7c9322914213bb589035afbbc03faf1ae3bf0 Mon Sep 17 00:00:00 2001 From: Chris Wilson <chris@chris-wilson.co.uk> Date: Mon, 29 Jun 2009 08:45:31 +0100 Subject: [PATCH] drm/i915: Remove mappings on clearing fence register As the fence register is not locked whilst the user has mmaped the buffer through the GTT, in order for the buffer to reacquire a fence register we need to cause a fresh page-fault on the next user access. In order to cause the page fault, we zap the current mapping on clearing the register. We also ensure that all potential outstanding access via the fence register is flushed before release as well. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> --- drivers/gpu/drm/i915/i915_gem.c | 41 ++++++++++++++++++-------------------- 1 files changed, 19 insertions(+), 22 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 685a876..6dc74c8 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -1946,12 +1946,6 @@ i915_gem_object_unbind(struct drm_gem_object *obj) obj_priv->agp_mem = NULL; } - - /* blow away mappings if mapped through GTT */ - if (obj_priv->mmap_offset && dev->dev_mapping) - unmap_mapping_range(dev->dev_mapping, - obj_priv->mmap_offset, obj->size, 1); - i915_gem_object_put_pages(obj); BUG_ON(obj_priv->pages); @@ -2350,8 +2344,7 @@ try_again: if (old_obj_priv->pin_count) continue; - /* i915 uses fences for GPU access to tiled buffers */ - if (IS_I965G(dev) || !old_obj_priv->active) + if (!old_obj_priv->active) break; /* find the seqno of the first available fence */ @@ -2440,6 +2433,8 @@ i915_gem_clear_fence_reg(struct drm_gem_object *obj) obj_priv->gtt_offset, obj->size); #endif + BUG_ON(obj_priv->active); + if (IS_I965G(dev)) I915_WRITE64(FENCE_REG_965_0 + (obj_priv->fence_reg * 8), 0); else { @@ -2456,6 +2451,11 @@ i915_gem_clear_fence_reg(struct drm_gem_object *obj) dev_priv->fence_regs[obj_priv->fence_reg].obj = NULL; obj_priv->fence_reg = I915_FENCE_REG_NONE; + + /* blow away mappings if mapped through GTT */ + if (obj_priv->mmap_offset && dev->dev_mapping) + unmap_mapping_range(dev->dev_mapping, + obj_priv->mmap_offset, obj->size, 1); } /** @@ -2469,27 +2469,24 @@ i915_gem_clear_fence_reg(struct drm_gem_object *obj) int i915_gem_object_put_fence_reg(struct drm_gem_object *obj) { - struct drm_device *dev = obj->dev; struct drm_i915_gem_object *obj_priv = obj->driver_private; + int ret; if (obj_priv->fence_reg == I915_FENCE_REG_NONE) return 0; - /* On the i915, GPU access to tiled buffers is via a fence, - * therefore we must wait for any outstanding access to complete - * before clearing the fence. + /* If there is outstanding activity on the buffer whilst it holds + * a fence register we must assume that it requires that fence for + * correct operation. Therefore we must wait for any outstanding + * access to complete before clearing the fence. */ - if (!IS_I965G(dev)) { - int ret; - - i915_gem_object_flush_gpu_write_domain(obj); - i915_gem_object_flush_gtt_write_domain(obj); - ret = i915_gem_object_wait_rendering(obj); - if (ret != 0) - return ret; - } + i915_gem_object_flush_gpu_write_domain(obj); + i915_gem_object_flush_gtt_write_domain(obj); + ret = i915_gem_object_wait_rendering(obj); + if (ret != 0) + return ret; - i915_gem_clear_fence_reg (obj); + i915_gem_clear_fence_reg(obj); return 0; } -- 1.6.3.1 ^ permalink raw reply related [flat|nested] 40+ messages in thread
* Re: [git pull] drm: previous pull req + 1. 2009-06-29 7:57 ` Chris Wilson @ 2009-06-30 9:49 ` Chris Wilson 2009-07-09 23:11 ` Eric Anholt 1 sibling, 0 replies; 40+ messages in thread From: Chris Wilson @ 2009-06-30 9:49 UTC (permalink / raw) To: Maxim Levitsky Cc: Dave Airlie, Linus Torvalds, linux-kernel, dri-devel, Eric Anholt Revised patch, unmap_mapping_range() on unbind and clear register. >From 8f13b6389ee0c8a39a2073279928a3a228bd27dc Mon Sep 17 00:00:00 2001 From: Chris Wilson <chris@chris-wilson.co.uk> Date: Mon, 29 Jun 2009 08:45:31 +0100 Subject: [PATCH] drm/i915: Remove mappings on clearing fence register As the fence register is not locked whilst the user has mmaped the buffer through the GTT, in order for the buffer to reacquire a fence register we need to cause a fresh page-fault on the next user access. In order to cause the page fault, we zap the current mapping on clearing the register. We also ensure that all potential outstanding access via the fence register is flushed before release as well. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> --- drivers/gpu/drm/i915/i915_gem.c | 35 +++++++++++++++++++---------------- 1 files changed, 19 insertions(+), 16 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 685a876..7fb636b 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -1946,8 +1946,7 @@ i915_gem_object_unbind(struct drm_gem_object *obj) obj_priv->agp_mem = NULL; } - - /* blow away mappings if mapped through GTT */ + /* Force the next mmap access to trigger a fault and rebind */ if (obj_priv->mmap_offset && dev->dev_mapping) unmap_mapping_range(dev->dev_mapping, obj_priv->mmap_offset, obj->size, 1); @@ -2350,8 +2349,7 @@ try_again: if (old_obj_priv->pin_count) continue; - /* i915 uses fences for GPU access to tiled buffers */ - if (IS_I965G(dev) || !old_obj_priv->active) + if (!old_obj_priv->active) break; /* find the seqno of the first available fence */ @@ -2440,6 +2438,8 @@ i915_gem_clear_fence_reg(struct drm_gem_object *obj) obj_priv->gtt_offset, obj->size); #endif + BUG_ON(obj_priv->active); + if (IS_I965G(dev)) I915_WRITE64(FENCE_REG_965_0 + (obj_priv->fence_reg * 8), 0); else { @@ -2471,25 +2471,28 @@ i915_gem_object_put_fence_reg(struct drm_gem_object *obj) { struct drm_device *dev = obj->dev; struct drm_i915_gem_object *obj_priv = obj->driver_private; + int ret; if (obj_priv->fence_reg == I915_FENCE_REG_NONE) return 0; - /* On the i915, GPU access to tiled buffers is via a fence, - * therefore we must wait for any outstanding access to complete - * before clearing the fence. + /* If there is outstanding activity on the buffer whilst it holds + * a fence register we must assume that it requires that fence for + * correct operation. Therefore we must wait for any outstanding + * access to complete before clearing the fence. */ - if (!IS_I965G(dev)) { - int ret; + i915_gem_object_flush_gpu_write_domain(obj); + i915_gem_object_flush_gtt_write_domain(obj); + ret = i915_gem_object_wait_rendering(obj); + if (ret != 0) + return ret; - i915_gem_object_flush_gpu_write_domain(obj); - i915_gem_object_flush_gtt_write_domain(obj); - ret = i915_gem_object_wait_rendering(obj); - if (ret != 0) - return ret; - } + i915_gem_clear_fence_reg(obj); - i915_gem_clear_fence_reg (obj); + /* Reacquire fence register on next mmap access (via page fault) */ + if (obj_priv->mmap_offset) + unmap_mapping_range(dev->dev_mapping, + obj_priv->mmap_offset, obj->size, 1); return 0; } -- 1.6.3.3 ^ permalink raw reply related [flat|nested] 40+ messages in thread
* Re: [git pull] drm: previous pull req + 1. 2009-06-29 7:57 ` Chris Wilson 2009-06-30 9:49 ` Chris Wilson @ 2009-07-09 23:11 ` Eric Anholt 1 sibling, 0 replies; 40+ messages in thread From: Eric Anholt @ 2009-07-09 23:11 UTC (permalink / raw) To: Chris Wilson Cc: Maxim Levitsky, Linus Torvalds, Dave Airlie, linux-kernel, dri-devel [-- Attachment #1: Type: text/plain, Size: 4299 bytes --] On Mon, 2009-06-29 at 08:57 +0100, Chris Wilson wrote: > On Mon, 2009-06-22 at 21:09 +0300, Maxim Levitsky wrote: > > Nope, same thing. > > > > I use commit 87ef92092fd092936535ba057ee19b97bb6a709a + this patch > > Note that GE doesn't hang the system when maximizing it. > > > > It is for sure tiled textures accessed incorrectly, same behavior > > observed in many games (they still run though) > > Sorry for the delay, I was travelling last week and was sure I had > nailed the problem. Aside from the missing flush on i965 when a batch > buffer might be using fenced commands, the only other issue I found was > that we did not zap the mapping range on clear - which could also cause > tiling errors if textures were being written via a GTT mmap. > > So please can you try this patch: > > >From 20b7c9322914213bb589035afbbc03faf1ae3bf0 Mon Sep 17 00:00:00 2001 > From: Chris Wilson <chris@chris-wilson.co.uk> > Date: Mon, 29 Jun 2009 08:45:31 +0100 > Subject: [PATCH] drm/i915: Remove mappings on clearing fence register > > As the fence register is not locked whilst the user has mmaped the buffer > through the GTT, in order for the buffer to reacquire a fence register we > need to cause a fresh page-fault on the next user access. In order to > cause the page fault, we zap the current mapping on clearing the register. > We also ensure that all potential outstanding access via the fence > register is flushed before release as well. > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > --- > drivers/gpu/drm/i915/i915_gem.c | 41 ++++++++++++++++++-------------------- > 1 files changed, 19 insertions(+), 22 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c > index 685a876..6dc74c8 100644 > --- a/drivers/gpu/drm/i915/i915_gem.c > +++ b/drivers/gpu/drm/i915/i915_gem.c > @@ -1946,12 +1946,6 @@ i915_gem_object_unbind(struct drm_gem_object *obj) > obj_priv->agp_mem = NULL; > } > > - > - /* blow away mappings if mapped through GTT */ > - if (obj_priv->mmap_offset && dev->dev_mapping) > - unmap_mapping_range(dev->dev_mapping, > - obj_priv->mmap_offset, obj->size, 1); > - Err, so now untiled things wouldn't fault to rebind? Seems wrong. > @@ -2456,6 +2451,11 @@ i915_gem_clear_fence_reg(struct drm_gem_object *obj) > > dev_priv->fence_regs[obj_priv->fence_reg].obj = NULL; > obj_priv->fence_reg = I915_FENCE_REG_NONE; > + > + /* blow away mappings if mapped through GTT */ > + if (obj_priv->mmap_offset && dev->dev_mapping) > + unmap_mapping_range(dev->dev_mapping, > + obj_priv->mmap_offset, obj->size, 1); > } This part seems good. > > /** > @@ -2469,27 +2469,24 @@ i915_gem_clear_fence_reg(struct drm_gem_object *obj) > int > i915_gem_object_put_fence_reg(struct drm_gem_object *obj) > { > - struct drm_device *dev = obj->dev; > struct drm_i915_gem_object *obj_priv = obj->driver_private; > + int ret; > > if (obj_priv->fence_reg == I915_FENCE_REG_NONE) > return 0; > > - /* On the i915, GPU access to tiled buffers is via a fence, > - * therefore we must wait for any outstanding access to complete > - * before clearing the fence. > + /* If there is outstanding activity on the buffer whilst it holds > + * a fence register we must assume that it requires that fence for > + * correct operation. Therefore we must wait for any outstanding > + * access to complete before clearing the fence. > */ > - if (!IS_I965G(dev)) { > - int ret; > - > - i915_gem_object_flush_gpu_write_domain(obj); > - i915_gem_object_flush_gtt_write_domain(obj); > - ret = i915_gem_object_wait_rendering(obj); > - if (ret != 0) > - return ret; > - } > + i915_gem_object_flush_gpu_write_domain(obj); > + i915_gem_object_flush_gtt_write_domain(obj); > + ret = i915_gem_object_wait_rendering(obj); > + if (ret != 0) > + return ret; > > - i915_gem_clear_fence_reg (obj); > + i915_gem_clear_fence_reg(obj); > > return 0; > } This part doesn't make sense to me. There should be nothing in the 965 rendering path that's using fences. Did you identify something that was? -- Eric Anholt eric@anholt.net eric.anholt@intel.com [-- Attachment #2: This is a digitally signed message part --] [-- Type: application/pgp-signature, Size: 197 bytes --] ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [git pull] drm: previous pull req + 1. 2009-06-21 0:04 ` Maxim Levitsky 2009-06-21 0:42 ` Linus Torvalds @ 2009-06-21 1:33 ` Dave Airlie 1 sibling, 0 replies; 40+ messages in thread From: Dave Airlie @ 2009-06-21 1:33 UTC (permalink / raw) To: Maxim Levitsky; +Cc: torvalds, dri-devel, linux-kernel > > > > Please pull the 'drm-linus' branch from > > ssh://master.kernel.org/pub/scm/linux/kernel/git/airlied/drm-2.6.git drm-linus > > > > This is the same tree from the previous pull request + the fix from Pierre > > that actually makes the PAE/GEM combination on i965 work. \o/ > > Something from this tree breaks my i965. > Using -git just before this was pulled, > > a552f0af753eb4b5bbbe9eff205fe874b04c4583 works, but using latest git > > makes google earth stall, it doesn't update its main window. It appears > that openining and closing its menu, allows it to progress frame after > frame. No crashes hangs however. > > Used latest -git of all graphic components (mesa, libdrm, xserver, intel > driver) > > KMS is used, as well as UXA and DRI2 (this is enabled by default now) Wierd I'm not seeing anything in there that would just break something like googleearth, and still leave compiz going, I think a git bisect on the drivers/gpu/drm/i915 subdir should work. Were you running a PAE kernel before now? Dave. > > > Compiz used as well, but this happens without it as well. > > > Best regards, > Maxim Levitsky > > ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [git pull] drm: previous pull req + 1. 2009-06-20 5:23 [git pull] drm: previous pull req + 1 Dave Airlie 2009-06-21 0:04 ` Maxim Levitsky @ 2009-06-21 3:41 ` Andy Lutomirski 2009-06-21 5:16 ` Dave Airlie 1 sibling, 1 reply; 40+ messages in thread From: Andy Lutomirski @ 2009-06-21 3:41 UTC (permalink / raw) To: Dave Airlie; +Cc: torvalds, dri-devel, linux-kernel [-- Attachment #1: Type: text/plain, Size: 1118 bytes --] Dave Airlie wrote: > Hi Linus, > > Please pull the 'drm-linus' branch from > ssh://master.kernel.org/pub/scm/linux/kernel/git/airlied/drm-2.6.git drm-linus > > This is the same tree from the previous pull request + the fix from Pierre > that actually makes the PAE/GEM combination on i965 work. \o/ > I tried this tree (specifically, a merge of Linus' fb20871 this tree) on Fedora 11 with modesetting enabled on an integrated Radeon 2100, and plymouthd crashes immediately with a corrupt page table. Photo attached. After the crash, bootup stops, although ctrl-alt-del works. This is on a Gigabyte GA-MA74GM-S2 (RS690 chipset) with a somewhat old BIOS which has never had reliable graphics on X with any kernel and userspace I've tried, modesetting or otherwise, so this isn't a regression per se. But on F11's kernel, I can at least boot to a console with modesetting enabled and nothing dies until I try to start X, and, with modesetting off on older kernels I can boot just fine (again, until I try to use X, at which point things break). I'd be happy to test things to help debug. Thanks, Andy [-- Attachment #2: radeon_modeset_crash.jpg --] [-- Type: image/jpeg, Size: 278259 bytes --] ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [git pull] drm: previous pull req + 1. 2009-06-21 3:41 ` Andy Lutomirski @ 2009-06-21 5:16 ` Dave Airlie 2009-06-21 12:06 ` Andrew Lutomirski 2009-06-21 16:46 ` Linus Torvalds 0 siblings, 2 replies; 40+ messages in thread From: Dave Airlie @ 2009-06-21 5:16 UTC (permalink / raw) To: Andy Lutomirski; +Cc: torvalds, dri-devel, linux-kernel > > I tried this tree (specifically, a merge of Linus' fb20871 this tree) on > Fedora 11 with modesetting enabled on an integrated Radeon 2100, and plymouthd > crashes immediately with a corrupt page table. Photo attached. After the > crash, bootup stops, although ctrl-alt-del works. You need a different userspace, -ati from koji in F11 should do it. However we have some outstanding rs690 issues that I'm trying to track down as we speak, Dave. > > This is on a Gigabyte GA-MA74GM-S2 (RS690 chipset) with a somewhat old BIOS > which has never had reliable graphics on X with any kernel and userspace I've > tried, modesetting or otherwise, so this isn't a regression per se. But on > F11's kernel, I can at least boot to a console with modesetting enabled and > nothing dies until I try to start X, and, with modesetting off on older > kernels I can boot just fine (again, until I try to use X, at which point > things break). > > I'd be happy to test things to help debug. > > Thanks, > Andy > ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [git pull] drm: previous pull req + 1. 2009-06-21 5:16 ` Dave Airlie @ 2009-06-21 12:06 ` Andrew Lutomirski 2009-06-21 16:46 ` Linus Torvalds 1 sibling, 0 replies; 40+ messages in thread From: Andrew Lutomirski @ 2009-06-21 12:06 UTC (permalink / raw) To: Dave Airlie; +Cc: torvalds, dri-devel, linux-kernel On Sun, Jun 21, 2009 at 1:16 AM, Dave Airlie<airlied@linux.ie> wrote: >> >> I tried this tree (specifically, a merge of Linus' fb20871 this tree) on >> Fedora 11 with modesetting enabled on an integrated Radeon 2100, and plymouthd >> crashes immediately with a corrupt page table. Photo attached. After the >> crash, bootup stops, although ctrl-alt-del works. > > You need a different userspace, -ati from koji in F11 should do it. I'm already running that (xorg-x11-drv-ati-6.12.2-17.fc11.x86_64, which is the latest in koji). But the oops is in *plymouth*, which as far as I can tell uses the framebuffer interface, so it shouldn't matter what xorg driver I have installed. Shouldn't it be impossible for a buggy framebuffer client to cause an oops? > > However we have some outstanding rs690 issues that I'm trying to track > down as we speak, Great! I'll keep on testing. --Andy > > Dave. > >> >> This is on a Gigabyte GA-MA74GM-S2 (RS690 chipset) with a somewhat old BIOS >> which has never had reliable graphics on X with any kernel and userspace I've >> tried, modesetting or otherwise, so this isn't a regression per se. But on >> F11's kernel, I can at least boot to a console with modesetting enabled and >> nothing dies until I try to start X, and, with modesetting off on older >> kernels I can boot just fine (again, until I try to use X, at which point >> things break). >> >> I'd be happy to test things to help debug. >> >> Thanks, >> Andy >> > ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [git pull] drm: previous pull req + 1. 2009-06-21 5:16 ` Dave Airlie 2009-06-21 12:06 ` Andrew Lutomirski @ 2009-06-21 16:46 ` Linus Torvalds 2009-06-21 17:13 ` Linus Torvalds 2009-06-21 22:41 ` Dave Airlie 1 sibling, 2 replies; 40+ messages in thread From: Linus Torvalds @ 2009-06-21 16:46 UTC (permalink / raw) To: Dave Airlie; +Cc: Andy Lutomirski, dri-devel, linux-kernel On Sun, 21 Jun 2009, Dave Airlie wrote: > > > > I tried this tree (specifically, a merge of Linus' fb20871 this tree) on > > Fedora 11 with modesetting enabled on an integrated Radeon 2100, and plymouthd > > crashes immediately with a corrupt page table. Photo attached. After the > > crash, bootup stops, although ctrl-alt-del works. > > You need a different userspace, -ati from koji in F11 should do it. Dave - no amount of userspace differences make a corrupted page table acceptable. This needs to be fixed. No excuses. Kernel crashes are never an issue of "you used the wrong user space". Linus ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [git pull] drm: previous pull req + 1. 2009-06-21 16:46 ` Linus Torvalds @ 2009-06-21 17:13 ` Linus Torvalds 2009-06-21 18:50 ` Andrew Lutomirski 2009-06-21 22:41 ` Dave Airlie 1 sibling, 1 reply; 40+ messages in thread From: Linus Torvalds @ 2009-06-21 17:13 UTC (permalink / raw) To: Dave Airlie; +Cc: Andy Lutomirski, dri-devel, linux-kernel On Sun, 21 Jun 2009, Linus Torvalds wrote: > > Dave - no amount of userspace differences make a corrupted page table > acceptable. > > This needs to be fixed. No excuses. Kernel crashes are never an issue of > "you used the wrong user space". So "corrupted page table" means that one of the reserved bits was set, and we get a page fault with the PF_RSVD bit on in the error code. Looking at the debug output, it says PGD 12148a067 PUD 12148b067 PMD 121496067 PTE ffffc90011780237 where the top-level entries look fine, but the PTE is total crap. It looks like it has filled in the page frame number with a virtual address rather than with an actual page The PTE _should_ look like this: - bit 63: NX - bits 62-52: zero (available to sw, but I don't think we use them) - bits 51-47: zero (reserved) - bits 46-12: page frame - bits 11-0: protection and PAT bits etc (bits 8-7 are also reserved) and that PTE clearly does not match. Strictly speaking, that "47-bit" physical address is purely theoretical. I think existing CPU's are limited to 40 bits or so, so there are even more reserved bits. Anyway, here's a totally UNTESTED patch that hopefully gives a warning on where exactly we set the invalid bits. Andy, mind trying it out? You should get the warnign much earlier, and it should have a much more useful back-trace. (But this is _untested_, so maybe I screwed up and it doesn't compile or work. The BAD_PTE_BITS mask could also be improved upon, but that mask should be "good enough" - it doesn't include _all_ the bits it could, but it certainly has enough bits set to trigger that obviously bad case). Linus --- arch/x86/include/asm/pgtable_64.h | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-) diff --git a/arch/x86/include/asm/pgtable_64.h b/arch/x86/include/asm/pgtable_64.h index c57a301..b95828e 100644 --- a/arch/x86/include/asm/pgtable_64.h +++ b/arch/x86/include/asm/pgtable_64.h @@ -49,8 +49,11 @@ static inline void native_pte_clear(struct mm_struct *mm, unsigned long addr, *ptep = native_make_pte(0); } +#define BAD_PTE_BITS (_PAGE_NX - (1ul << __PHYSICAL_MASK_SHIFT)) + static inline void native_set_pte(pte_t *ptep, pte_t pte) { + WARN_ON_ONCE(pte.pte & BAD_PTE_BITS); *ptep = pte; } ^ permalink raw reply related [flat|nested] 40+ messages in thread
* Re: [git pull] drm: previous pull req + 1. 2009-06-21 17:13 ` Linus Torvalds @ 2009-06-21 18:50 ` Andrew Lutomirski 2009-06-21 19:47 ` Linus Torvalds 0 siblings, 1 reply; 40+ messages in thread From: Andrew Lutomirski @ 2009-06-21 18:50 UTC (permalink / raw) To: Linus Torvalds; +Cc: Dave Airlie, dri-devel, linux-kernel [-- Attachment #1: Type: text/plain, Size: 1636 bytes --] On Sun, Jun 21, 2009 at 1:13 PM, Linus Torvalds<torvalds@linux-foundation.org> wrote: > > > On Sun, 21 Jun 2009, Linus Torvalds wrote: >> >> Dave - no amount of userspace differences make a corrupted page table >> acceptable. >> >> This needs to be fixed. No excuses. Kernel crashes are never an issue of >> "you used the wrong user space". > > So "corrupted page table" means that one of the reserved bits was set, and > we get a page fault with the PF_RSVD bit on in the error code. > > Looking at the debug output, it says > > PGD 12148a067 > PUD 12148b067 > PMD 121496067 > PTE ffffc90011780237 > > where the top-level entries look fine, but the PTE is total crap. It looks > like it has filled in the page frame number with a virtual address rather > than with an actual page > > The PTE _should_ look like this: > > - bit 63: NX > - bits 62-52: zero (available to sw, but I don't think we use them) > - bits 51-47: zero (reserved) > - bits 46-12: page frame > - bits 11-0: protection and PAT bits etc (bits 8-7 are also reserved) > > and that PTE clearly does not match. > > Strictly speaking, that "47-bit" physical address is purely theoretical. I > think existing CPU's are limited to 40 bits or so, so there are even more > reserved bits. > > Anyway, here's a totally UNTESTED patch that hopefully gives a warning on > where exactly we set the invalid bits. Andy, mind trying it out? You > should get the warnign much earlier, and it should have a much more useful > back-trace. Your patch worked. Photo attached. --Andy [-- Attachment #2: radeon_modeset_crash_2.jpg --] [-- Type: image/jpeg, Size: 287242 bytes --] ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [git pull] drm: previous pull req + 1. 2009-06-21 18:50 ` Andrew Lutomirski @ 2009-06-21 19:47 ` Linus Torvalds 2009-06-21 21:14 ` Andrew Lutomirski ` (2 more replies) 0 siblings, 3 replies; 40+ messages in thread From: Linus Torvalds @ 2009-06-21 19:47 UTC (permalink / raw) To: Andrew Lutomirski Cc: Dave Airlie, dri-devel, Linux Kernel Mailing List, Jerome Glisse, Alex Deucher On Sun, 21 Jun 2009, Andrew Lutomirski wrote: > > > > Anyway, here's a totally UNTESTED patch that hopefully gives a warning on > > where exactly we set the invalid bits. Andy, mind trying it out? You > > should get the warnign much earlier, and it should have a much more useful > > back-trace. > > Your patch worked. Photo attached. Ok. So it's fb_mmap() that uses an invalid page frame number when it does the "io_remap_pfn_range()" thing. And the way it gets that page frame number is basically - Offset (in bytes) from start of mapping: off = vma->vm_pgoff << PAGE_SHIFT; .. - frame buffer start address: /* frame buffer memory */ start = info->fix.smem_start; len = PAGE_ALIGN((start & ~PAGE_MASK) + info->fix.smem_len); .. off += start; - do the remap: io_remap_pfn_range(vma, vma->vm_start, off >> PAGE_SHIFT, .. and there has been no changes to this logic in drivers/video/fbmem.c lately. What *has* changed is that we have a newradeon driver, and it looks like that new radeon driver is crap, and does this: info->fix.smem_start = (unsigned long)fbptr; which is totally screwed up. It assigns a _virtual_ address to that "smem_start" thing, even though it should be a physical one. I don't know the radeon driver, so I don't know where to find the physical address. It's also possible that there is no good single physical address, and the radeon driver should implement a "fb_mmap" function. Does this patch make the warning and the oops at least go away? Obviously it won't result in a working frame buffer, but that's a separate issue Linus --- drivers/gpu/drm/radeon/radeon_fb.c | 9 +++++++++ 1 files changed, 9 insertions(+), 0 deletions(-) diff --git a/drivers/gpu/drm/radeon/radeon_fb.c b/drivers/gpu/drm/radeon/radeon_fb.c index fa86d39..4aa151e 100644 --- a/drivers/gpu/drm/radeon/radeon_fb.c +++ b/drivers/gpu/drm/radeon/radeon_fb.c @@ -380,6 +380,14 @@ int radeonfb_blank(int blank, struct fb_info *info) return 0; } +/* + * Not yet implemented. The fix.smem_start is crap. + */ +static int radeonfb_mmap(struct fb_info *info, struct vm_area_struct *vma) +{ + return -EINVAL; +} + static struct fb_ops radeonfb_ops = { .owner = THIS_MODULE, .fb_check_var = radeonfb_check_var, @@ -390,6 +398,7 @@ static struct fb_ops radeonfb_ops = { .fb_imageblit = cfb_imageblit, .fb_pan_display = radeonfb_pan_display, .fb_blank = radeonfb_blank, + .fb_mmap = radeonfb_mmap, }; /** ^ permalink raw reply related [flat|nested] 40+ messages in thread
* Re: [git pull] drm: previous pull req + 1. 2009-06-21 19:47 ` Linus Torvalds @ 2009-06-21 21:14 ` Andrew Lutomirski 2009-06-22 0:05 ` Andrew Lutomirski 2009-06-21 22:40 ` Dave Airlie 2009-06-22 23:57 ` Benjamin Herrenschmidt 2 siblings, 1 reply; 40+ messages in thread From: Andrew Lutomirski @ 2009-06-21 21:14 UTC (permalink / raw) To: Linus Torvalds Cc: Dave Airlie, dri-devel, Linux Kernel Mailing List, Jerome Glisse, Alex Deucher On Sun, Jun 21, 2009 at 3:47 PM, Linus Torvalds<torvalds@linux-foundation.org> wrote: > > > What *has* changed is that we have a newradeon driver, and it looks like > that new radeon driver is crap, and does this: > > info->fix.smem_start = (unsigned long)fbptr; > > which is totally screwed up. It assigns a _virtual_ address to that > "smem_start" thing, even though it should be a physical one. > > I don't know the radeon driver, so I don't know where to find the physical > address. It's also possible that there is no good single physical > address, and the radeon driver should implement a "fb_mmap" function. > > Does this patch make the warning and the oops at least go away? Obviously > it won't result in a working frame buffer, but that's a separate issue > I haven't tried your patch, but I hacked up the one below instead, which also fixes the oops. It still doesn't boot, though -- plymouth hangs (or otherwise dies), preventing my initramfs from finishing. The same kernel image boots fine with radeon.modeset=0. I wouldn't apply this without someone more familiar with the code's ack -- I have no idea if the lifetime of the framebuffer object is right, and there may be any number of other bugs lurking in here. Not to mention a missing "static." Signed-off-by: Andy Lutomirski <luto@mit.edu> diff --git a/drivers/gpu/drm/radeon/radeon_fb.c b/drivers/gpu/drm/radeon/radeon_fb.c index fa86d39..cbb330d 100644 --- a/drivers/gpu/drm/radeon/radeon_fb.c +++ b/drivers/gpu/drm/radeon/radeon_fb.c @@ -380,6 +380,12 @@ int radeonfb_blank(int blank, struct fb_info *info) return 0; } +int radeonfb_mmap(struct fb_info *info, struct vm_area_struct *vma) +{ + struct radeon_fb_device *rfbdev = info->par; + return radeon_object_fbdev_mmap(rfbdev->rdev->fbdev_robj, vma); +} + static struct fb_ops radeonfb_ops = { .owner = THIS_MODULE, .fb_check_var = radeonfb_check_var, @@ -390,6 +396,7 @@ static struct fb_ops radeonfb_ops = { .fb_imageblit = cfb_imageblit, .fb_pan_display = radeonfb_pan_display, .fb_blank = radeonfb_blank, + .fb_mmap = radeonfb_mmap, }; /** @@ -499,7 +506,7 @@ int radeonfb_create(struct radeon_device *rdev, ret = radeon_gem_object_create(rdev, aligned_size, 0, RADEON_GEM_DOMAIN_VRAM, - false, ttm_bo_type_kernel, + false, ttm_bo_type_device, false, &gobj); if (ret) { printk(KERN_ERR "failed to allocate framebuffer\n"); @@ -547,8 +554,6 @@ int radeonfb_create(struct radeon_device *rdev, info->fbops = &radeonfb_ops; info->fix.line_length = fb->pitch; info->screen_base = fbptr; - info->fix.smem_start = (unsigned long)fbptr; - info->fix.smem_len = size; info->screen_base = fbptr; info->screen_size = size; info->pseudo_palette = fb->pseudo_palette; diff --git a/drivers/gpu/drm/radeon/radeon_object.c b/drivers/gpu/drm/radeon/radeon_object.c index 983e8df..433e52e 100644 --- a/drivers/gpu/drm/radeon/radeon_object.c +++ b/drivers/gpu/drm/radeon/radeon_object.c @@ -267,7 +267,6 @@ int radeon_object_pin(struct radeon_object *robj, uint32_t domain, } if (!r) { robj->rdev->fbdev_info->screen_base = fbptr; - robj->rdev->fbdev_info->fix.smem_start = (unsigned long)fbptr; } mutex_unlock(&robj->rdev->fbdev_info->lock); } @@ -316,7 +315,6 @@ void radeon_object_unpin(struct radeon_object *robj) } if (!r) { robj->rdev->fbdev_info->screen_base = fbptr; - robj->rdev->fbdev_info->fix.smem_start = (unsigned long)fbptr; } mutex_unlock(&robj->rdev->fbdev_info->lock); } ^ permalink raw reply related [flat|nested] 40+ messages in thread
* Re: [git pull] drm: previous pull req + 1. 2009-06-21 21:14 ` Andrew Lutomirski @ 2009-06-22 0:05 ` Andrew Lutomirski 2009-06-22 19:20 ` Arnaldo Carvalho de Melo 0 siblings, 1 reply; 40+ messages in thread From: Andrew Lutomirski @ 2009-06-22 0:05 UTC (permalink / raw) To: Linus Torvalds Cc: Dave Airlie, dri-devel, Linux Kernel Mailing List, Jerome Glisse, Alex Deucher On Sun, Jun 21, 2009 at 5:14 PM, Andrew Lutomirski<luto@mit.edu> wrote: > On Sun, Jun 21, 2009 at 3:47 PM, Linus > Torvalds<torvalds@linux-foundation.org> wrote: >> >> >> What *has* changed is that we have a newradeon driver, and it looks like >> that new radeon driver is crap, and does this: >> >> info->fix.smem_start = (unsigned long)fbptr; >> >> which is totally screwed up. It assigns a _virtual_ address to that >> "smem_start" thing, even though it should be a physical one. >> >> I don't know the radeon driver, so I don't know where to find the physical >> address. It's also possible that there is no good single physical >> address, and the radeon driver should implement a "fb_mmap" function. >> >> Does this patch make the warning and the oops at least go away? Obviously >> it won't result in a working frame buffer, but that's a separate issue >> > > I haven't tried your patch, but I hacked up the one below instead, > which also fixes the oops. It still doesn't boot, though -- plymouth > hangs (or otherwise dies), preventing my initramfs from finishing. > The same kernel image boots fine with radeon.modeset=0. My patch is no good -- plymouthd just dies more silently with it. Returning -EINVAL from radeonfb_mmap prevents plymouthd's splash screen from working (of course) but lets the system boot. (Not only does the system boot, but this is the first modesetting kernel I've ever seen work on this hardware.) Linus's patch works and even lets me start X. David -- there are still plenty of bugs. The kernel gets my monitor's resolution wrong (X reads it off DDC correctly but I have to use xrandr to force the resolution). And glxgears triggers the kernel's command verification and dies. But this is still a huge improvement over modesetting on the F11 kernel, which is unusable for me (image is garbled beyond recognition once I start X). --Andy ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [git pull] drm: previous pull req + 1. 2009-06-22 0:05 ` Andrew Lutomirski @ 2009-06-22 19:20 ` Arnaldo Carvalho de Melo 0 siblings, 0 replies; 40+ messages in thread From: Arnaldo Carvalho de Melo @ 2009-06-22 19:20 UTC (permalink / raw) To: Andrew Lutomirski Cc: Linus Torvalds, Dave Airlie, dri-devel, Linux Kernel Mailing List, Jerome Glisse, Alex Deucher Em Sun, Jun 21, 2009 at 08:05:36PM -0400, Andrew Lutomirski escreveu: > > David -- there are still plenty of bugs. The kernel gets my monitor's > resolution wrong (X reads it off DDC correctly but I have to use > xrandr to force the resolution). No crashes here, but I was able to use an external Samsung SyncMaster 226BW at 1680x1050 and now this resolution doesn't even appear as one of the possible resolutions on xrandr :-( F11, uptodate: xorg-x11-drv-intel-2.7.0-7.fc11.x86_64 Thinkpad T60W 00:02.0 VGA compatible controller: Intel Corporation Mobile 945GM/GMS, 943/940GML Express Integrated Graphics Controller (rev 03) 00:02.1 Display controller: Intel Corporation Mobile 945GM/GMS/GME, 943/940GML Express Integrated Graphics Controller (rev 03) dmesg lines related: That DAC-6 line should be setting it to 1680x1050 as has been till I switched to this new kernel: ACPI: Video Device [VID] (multi-head: yes rom: no post: no) [drm] Initialized drm 1.1.0 20060810 i915 0000:00:02.0: power state changed by ACPI to D0 i915 0000:00:02.0: PCI INT A -> GSI 16 (level, low) -> IRQ 16 i915 0000:00:02.0: setting latency timer to 64 <SNIP> allocated 1680x1050 fb: 0x007df000, bo ffff880037c3a780 Console: switching to colour frame buffer device 160x64 [drm] DAC-6: set mode 1280x1024 b <==================================== <SNIP> [drm] LVDS-8: set mode 1680x1050 21 fb0: inteldrmfb frame buffer device registered panic notifier [drm] Initialized i915 1.6.0 20080730 for 0000:00:02.0 on minor 0 - Arnaldo ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [git pull] drm: previous pull req + 1. 2009-06-21 19:47 ` Linus Torvalds 2009-06-21 21:14 ` Andrew Lutomirski @ 2009-06-21 22:40 ` Dave Airlie 2009-06-22 8:18 ` Thomas Hellström 2009-06-22 23:57 ` Benjamin Herrenschmidt 2 siblings, 1 reply; 40+ messages in thread From: Dave Airlie @ 2009-06-21 22:40 UTC (permalink / raw) To: Linus Torvalds Cc: Andrew Lutomirski, Alex Deucher, Jerome Glisse, Linux Kernel Mailing List, dri-devel > > > On Sun, 21 Jun 2009, Andrew Lutomirski wrote: > > > > > > Anyway, here's a totally UNTESTED patch that hopefully gives a warning on > > > where exactly we set the invalid bits. Andy, mind trying it out? You > > > should get the warnign much earlier, and it should have a much more useful > > > back-trace. > > > > Your patch worked. Photo attached. > > Ok. > > So it's fb_mmap() that uses an invalid page frame number when it does the > "io_remap_pfn_range()" thing. > > And the way it gets that page frame number is basically > > - Offset (in bytes) from start of mapping: > > off = vma->vm_pgoff << PAGE_SHIFT; > .. > > - frame buffer start address: > > /* frame buffer memory */ > start = info->fix.smem_start; > len = PAGE_ALIGN((start & ~PAGE_MASK) + info->fix.smem_len); > .. > off += start; > > - do the remap: > > io_remap_pfn_range(vma, vma->vm_start, off >> PAGE_SHIFT, .. > > and there has been no changes to this logic in drivers/video/fbmem.c > lately. > > What *has* changed is that we have a newradeon driver, and it looks like > that new radeon driver is crap, and does this: > > info->fix.smem_start = (unsigned long)fbptr; > > which is totally screwed up. It assigns a _virtual_ address to that > "smem_start" thing, even though it should be a physical one. > > I don't know the radeon driver, so I don't know where to find the physical > address. It's also possible that there is no good single physical > address, and the radeon driver should implement a "fb_mmap" function. > > Does this patch make the warning and the oops at least go away? Obviously > it won't result in a working frame buffer, but that's a separate issue > > Linus I noticed the same bogus line myself last night, I'll get Jerome to look at it since its his code, he was trying to be smart about how the radeon fbdev emulation should work, but fbdev isn't smart enough to do what he wants, so I've asked him to go back to the dumb pin the fbcon in VRAM until we can fix fbdev to do some sort of prepare/commit type hooks around blocks of reads/writes. With the safe method we end up with an 8MB pinned fbcon on 32MB in some scenarios, which is still totally unacceptable from a user pov. Btw this driver is under staging for a good reason, nobody claimed it was perfect, we just said we felt it was a good baseline to build the final driver on top off. Maybe I can add CONFIG_DONT_CC_LINUS_ON_STAGING_DRIVERS. Dave. > > --- > drivers/gpu/drm/radeon/radeon_fb.c | 9 +++++++++ > 1 files changed, 9 insertions(+), 0 deletions(-) > > diff --git a/drivers/gpu/drm/radeon/radeon_fb.c b/drivers/gpu/drm/radeon/radeon_fb.c > index fa86d39..4aa151e 100644 > --- a/drivers/gpu/drm/radeon/radeon_fb.c > +++ b/drivers/gpu/drm/radeon/radeon_fb.c > @@ -380,6 +380,14 @@ int radeonfb_blank(int blank, struct fb_info *info) > return 0; > } > > +/* > + * Not yet implemented. The fix.smem_start is crap. > + */ > +static int radeonfb_mmap(struct fb_info *info, struct vm_area_struct *vma) > +{ > + return -EINVAL; > +} > + > static struct fb_ops radeonfb_ops = { > .owner = THIS_MODULE, > .fb_check_var = radeonfb_check_var, > @@ -390,6 +398,7 @@ static struct fb_ops radeonfb_ops = { > .fb_imageblit = cfb_imageblit, > .fb_pan_display = radeonfb_pan_display, > .fb_blank = radeonfb_blank, > + .fb_mmap = radeonfb_mmap, > }; > > /** > > ------------------------------------------------------------------------------ > Are you an open source citizen? Join us for the Open Source Bridge conference! > Portland, OR, June 17-19. Two days of sessions, one day of unconference: $250. > Need another reason to go? 24-hour hacker lounge. Register today! > http://ad.doubleclick.net/clk;215844324;13503038;v?http://opensourcebridge.org > -- > _______________________________________________ > Dri-devel mailing list > Dri-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/dri-devel > > ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [git pull] drm: previous pull req + 1. 2009-06-21 22:40 ` Dave Airlie @ 2009-06-22 8:18 ` Thomas Hellström 2009-06-22 8:30 ` Dave Airlie ` (2 more replies) 0 siblings, 3 replies; 40+ messages in thread From: Thomas Hellström @ 2009-06-22 8:18 UTC (permalink / raw) To: Dave Airlie Cc: Linus Torvalds, Alex Deucher, Andrew Lutomirski, dri-devel, Jerome Glisse, Linux Kernel Mailing List Dave Airlie wrote: >> On Sun, 21 Jun 2009, Andrew Lutomirski wrote: >> >>>> Anyway, here's a totally UNTESTED patch that hopefully gives a warning on >>>> where exactly we set the invalid bits. Andy, mind trying it out? You >>>> should get the warnign much earlier, and it should have a much more useful >>>> back-trace. >>>> >>> Your patch worked. Photo attached. >>> >> Ok. >> >> So it's fb_mmap() that uses an invalid page frame number when it does the >> "io_remap_pfn_range()" thing. >> >> And the way it gets that page frame number is basically >> >> - Offset (in bytes) from start of mapping: >> >> off = vma->vm_pgoff << PAGE_SHIFT; >> .. >> >> - frame buffer start address: >> >> /* frame buffer memory */ >> start = info->fix.smem_start; >> len = PAGE_ALIGN((start & ~PAGE_MASK) + info->fix.smem_len); >> .. >> off += start; >> >> - do the remap: >> >> io_remap_pfn_range(vma, vma->vm_start, off >> PAGE_SHIFT, .. >> >> and there has been no changes to this logic in drivers/video/fbmem.c >> lately. >> >> What *has* changed is that we have a newradeon driver, and it looks like >> that new radeon driver is crap, and does this: >> >> info->fix.smem_start = (unsigned long)fbptr; >> >> which is totally screwed up. It assigns a _virtual_ address to that >> "smem_start" thing, even though it should be a physical one. >> >> I don't know the radeon driver, so I don't know where to find the physical >> address. It's also possible that there is no good single physical >> address, and the radeon driver should implement a "fb_mmap" function. >> >> Does this patch make the warning and the oops at least go away? Obviously >> it won't result in a working frame buffer, but that's a separate issue >> >> Linus >> > > I noticed the same bogus line myself last night, I'll get Jerome to look > at it since its his code, he was trying to be smart about how the > radeon fbdev emulation should work, but fbdev isn't smart enough to do > what he wants, so I've asked him to go back to the dumb pin the fbcon in > VRAM until we can fix fbdev to do some sort of prepare/commit type hooks > around blocks of reads/writes. > > With the safe method we end up with an 8MB pinned fbcon on 32MB in some > scenarios, which is still totally unacceptable from a user pov. > > There is a ttm_fbdev_mmap() function in TTM that may help in this situation. As with the standard ttm mmap it's using fault() which means it's possible to move out the backing buffer object if you first reserve it and then call unmap_mapping_range() on the relevant fbdev address space to kill existing user-space mappings. We've experimented a little with this on the Poulsbo / Moorestown KMS driver (we threw out the fbcon buffer when the X server was switched in) but the problem turned out to be that fbdev always expects an always present _kernel_ mapping and it seemed a bit too hard to block accesses to that mapping while it was torn down and set up again to point to the new location. In particular, the fbdev acceleration hooks seems to be called from atomic context. It would be very helpful if we could introduce an fbdev mutex that protects fbdev accesses to the kernel map and to the fbdev acceleration functions. /Thomas ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [git pull] drm: previous pull req + 1. 2009-06-22 8:18 ` Thomas Hellström @ 2009-06-22 8:30 ` Dave Airlie 2009-06-22 18:22 ` Linus Torvalds 2009-06-23 0:00 ` Benjamin Herrenschmidt 2 siblings, 0 replies; 40+ messages in thread From: Dave Airlie @ 2009-06-22 8:30 UTC (permalink / raw) To: Thomas Hellström Cc: Linus Torvalds, Alex Deucher, Andrew Lutomirski, dri-devel, Jerome Glisse, Linux Kernel Mailing List > > > There is a ttm_fbdev_mmap() function in TTM that may help in this situation. > As with the standard ttm mmap it's using fault() which means it's possible to > move out the backing buffer object if you first reserve it and then call > unmap_mapping_range() on the relevant fbdev address space to kill existing > user-space mappings. Yup I've looked at this from the fbdev pov, however I hit the same problem with the fbcon writes happening pretty much whenever they wanted to. It might be possible to move the fbcon around by updating screen_base, but you'd need to have some sort of lock around the read/write functions, I think locking on each individual read/write might be a lot of overhead. Ideallly something analogous to the X server Prepare/Finish access hooks. Dave. ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [git pull] drm: previous pull req + 1. 2009-06-22 8:18 ` Thomas Hellström 2009-06-22 8:30 ` Dave Airlie @ 2009-06-22 18:22 ` Linus Torvalds 2009-06-22 18:59 ` Andrew Lutomirski 2009-06-23 0:01 ` Benjamin Herrenschmidt 2009-06-23 0:00 ` Benjamin Herrenschmidt 2 siblings, 2 replies; 40+ messages in thread From: Linus Torvalds @ 2009-06-22 18:22 UTC (permalink / raw) To: Thomas Hellström Cc: Dave Airlie, Alex Deucher, Andrew Lutomirski, dri-devel, Jerome Glisse, Linux Kernel Mailing List On Mon, 22 Jun 2009, Thomas Hellström wrote: > > It would be very helpful if we could introduce an fbdev mutex that protects > fbdev accesses to the kernel map and to the fbdev acceleration functions. Not going to happen. Why? 'printk'. If you can't handle printk, then you're basically useless. And printk absolutely -has- to work in bad situations (the most important messages could happen in any context). Linus ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [git pull] drm: previous pull req + 1. 2009-06-22 18:22 ` Linus Torvalds @ 2009-06-22 18:59 ` Andrew Lutomirski 2009-06-22 19:43 ` Linus Torvalds 2009-06-23 0:01 ` Benjamin Herrenschmidt 1 sibling, 1 reply; 40+ messages in thread From: Andrew Lutomirski @ 2009-06-22 18:59 UTC (permalink / raw) To: Linus Torvalds Cc: Thomas Hellström, Dave Airlie, Alex Deucher, dri-devel, Jerome Glisse, Linux Kernel Mailing List 2009/6/22 Linus Torvalds <torvalds@linux-foundation.org>: > > > On Mon, 22 Jun 2009, Thomas Hellström wrote: >> >> It would be very helpful if we could introduce an fbdev mutex that protects >> fbdev accesses to the kernel map and to the fbdev acceleration functions. > > Not going to happen. > > Why? 'printk'. > > If you can't handle printk, then you're basically useless. And printk > absolutely -has- to work in bad situations (the most important messages > could happen in any context). What if we only guaranteed that the framebuffer is mapped when it's showing on the screen? printk doesn't need to write to the framebuffer immediately when X isn't running (since the framebuffer isn't shown) and presumably the framebuffer needs to be pinned somewhere when it's being displayed anyway. This would involve fbcon knowing how to buffer text to be shown later so that printk still works in interrupt context. --Andy ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [git pull] drm: previous pull req + 1. 2009-06-22 18:59 ` Andrew Lutomirski @ 2009-06-22 19:43 ` Linus Torvalds 0 siblings, 0 replies; 40+ messages in thread From: Linus Torvalds @ 2009-06-22 19:43 UTC (permalink / raw) To: Andrew Lutomirski Cc: Thomas Hellström, Dave Airlie, Alex Deucher, dri-devel, Jerome Glisse, Linux Kernel Mailing List On Mon, 22 Jun 2009, Andrew Lutomirski wrote: > > What if we only guaranteed that the framebuffer is mapped when it's > showing on the screen? I think that works ok. We only care about printk being immediate in that case, and if it gets buffered I don't think we care. > printk doesn't need to write to the framebuffer immediately when X > isn't running (since the framebuffer isn't shown) and presumably the > framebuffer needs to be pinned somewhere when it's being displayed > anyway. This would involve fbcon knowing how to buffer text to be > shown later so that printk still works in interrupt context. But doesn't fbcon do that _anyway_ for VC switching? (I've tried to stay out of fbcon, and have traditionally personally always preferred just regular VGA text mode, so I really have no clue about the internals). Linus ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [git pull] drm: previous pull req + 1. 2009-06-22 18:22 ` Linus Torvalds 2009-06-22 18:59 ` Andrew Lutomirski @ 2009-06-23 0:01 ` Benjamin Herrenschmidt 1 sibling, 0 replies; 40+ messages in thread From: Benjamin Herrenschmidt @ 2009-06-23 0:01 UTC (permalink / raw) To: Linus Torvalds Cc: Thomas Hellström, Dave Airlie, Alex Deucher, Andrew Lutomirski, dri-devel, Jerome Glisse, Linux Kernel Mailing List On Mon, 2009-06-22 at 11:22 -0700, Linus Torvalds wrote: > Not going to happen. > > Why? 'printk'. > > If you can't handle printk, then you're basically useless. And printk > absolutely -has- to work in bad situations (the most important > messages could happen in any context). Well... yes and no. If X is frontmost, printk is not going to be printed, ie, I'm talking about today, when the console is !KD_TEXT. There -is- a mechanism to deal with these things today, and the console semaphore does take care of accesses to the fb. (That doesn't exclude having the ability to force-switch back to kernel fb for printing things like oops btw, which KMS could do, but for basic access control, it makes sense). Cheers, Ben. ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [git pull] drm: previous pull req + 1. 2009-06-22 8:18 ` Thomas Hellström 2009-06-22 8:30 ` Dave Airlie 2009-06-22 18:22 ` Linus Torvalds @ 2009-06-23 0:00 ` Benjamin Herrenschmidt 2009-06-23 0:24 ` Linus Torvalds 2 siblings, 1 reply; 40+ messages in thread From: Benjamin Herrenschmidt @ 2009-06-23 0:00 UTC (permalink / raw) To: Thomas Hellström Cc: Dave Airlie, Linus Torvalds, Alex Deucher, Andrew Lutomirski, dri-devel, Jerome Glisse, Linux Kernel Mailing List On Mon, 2009-06-22 at 10:18 +0200, Thomas Hellström wrote: > It would be very helpful if we could introduce an fbdev mutex that > protects fbdev accesses to the kernel map and to the fbdev acceleration > functions. As far as I can remember, all fbdev operations are done under the console semaphore. (I know I did fix a bunch of that crap ages ago). Cheers, Ben. ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [git pull] drm: previous pull req + 1. 2009-06-23 0:00 ` Benjamin Herrenschmidt @ 2009-06-23 0:24 ` Linus Torvalds 2009-06-23 1:04 ` Benjamin Herrenschmidt 0 siblings, 1 reply; 40+ messages in thread From: Linus Torvalds @ 2009-06-23 0:24 UTC (permalink / raw) To: Benjamin Herrenschmidt Cc: Thomas Hellström, Dave Airlie, Alex Deucher, Andrew Lutomirski, dri-devel, Jerome Glisse, Linux Kernel Mailing List On Tue, 23 Jun 2009, Benjamin Herrenschmidt wrote: > > As far as I can remember, all fbdev operations are done under the > console semaphore. Yeah, and some of them are horribly broken (ie copying data from user space while doing it - causing horrible things like VC switching latencies and invisible printk's if an oops happens during the op). Or maybe that got fixed. Linus ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [git pull] drm: previous pull req + 1. 2009-06-23 0:24 ` Linus Torvalds @ 2009-06-23 1:04 ` Benjamin Herrenschmidt 2009-06-23 1:18 ` Jesse Barnes 0 siblings, 1 reply; 40+ messages in thread From: Benjamin Herrenschmidt @ 2009-06-23 1:04 UTC (permalink / raw) To: Linus Torvalds Cc: Thomas Hellström, Dave Airlie, Alex Deucher, Andrew Lutomirski, dri-devel, Jerome Glisse, Linux Kernel Mailing List On Mon, 2009-06-22 at 17:24 -0700, Linus Torvalds wrote: > > On Tue, 23 Jun 2009, Benjamin Herrenschmidt wrote: > > > > As far as I can remember, all fbdev operations are done under the > > console semaphore. > > Yeah, and some of them are horribly broken (ie copying data from user > space while doing it - causing horrible things like VC switching latencies > and invisible printk's if an oops happens during the op). > > Or maybe that got fixed. Well, it does rely on userspace behaving.. ie, no accel ops are done by the kernel in KD_GRAPHICS and userspace is -supposed- to switch to KD_GRAPHICS before touching the fb. In fact, nowdays, we do have the infrastructure to be smart and enforce that. IE. Instead of using a boring remap_page_ranges() in fb_mmap() we could use a fault handler. When in KD_TEXT, we fail them, when in KD_GRAPHICS, we service them, and we unmap_mapping_range() when switching. Something like that... Dunno how that interacts with the new DRM thingy though. Cheers, Ben. ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [git pull] drm: previous pull req + 1. 2009-06-23 1:04 ` Benjamin Herrenschmidt @ 2009-06-23 1:18 ` Jesse Barnes 2009-06-23 1:58 ` Benjamin Herrenschmidt 2009-06-23 7:48 ` Michel Dänzer 0 siblings, 2 replies; 40+ messages in thread From: Jesse Barnes @ 2009-06-23 1:18 UTC (permalink / raw) To: Benjamin Herrenschmidt Cc: Linus Torvalds, Thomas Hellström, Dave Airlie, Alex Deucher, Andrew Lutomirski, dri-devel, Jerome Glisse, Linux Kernel Mailing List On Tue, 23 Jun 2009 11:04:39 +1000 Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote: > On Mon, 2009-06-22 at 17:24 -0700, Linus Torvalds wrote: > > > > On Tue, 23 Jun 2009, Benjamin Herrenschmidt wrote: > > > > > > As far as I can remember, all fbdev operations are done under the > > > console semaphore. > > > > Yeah, and some of them are horribly broken (ie copying data from > > user space while doing it - causing horrible things like VC > > switching latencies and invisible printk's if an oops happens > > during the op). > > > > Or maybe that got fixed. > > Well, it does rely on userspace behaving.. ie, no accel ops are done > by the kernel in KD_GRAPHICS and userspace is -supposed- to switch to > KD_GRAPHICS before touching the fb. > > In fact, nowdays, we do have the infrastructure to be smart and > enforce that. IE. Instead of using a boring remap_page_ranges() in > fb_mmap() we could use a fault handler. When in KD_TEXT, we fail > them, when in KD_GRAPHICS, we service them, and we > unmap_mapping_range() when switching. Something like that... > > Dunno how that interacts with the new DRM thingy though. I think it could work, but ideally we'd keep the kernel fbcon object pinned, and keep printing into it even while some other gfx app is running. That way we don't have to dump the whole queue into it when a panic occurs, we can just switch buffers (something like this would also be handy for dual head debugging; one head running your desktop and the other a debug console printing all the messages). That's slightly more invasive surgery though... I should have a chance to do something like that as part of the kdb/kms work I'll be doing with Jason. -- Jesse Barnes, Intel Open Source Technology Center ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [git pull] drm: previous pull req + 1. 2009-06-23 1:18 ` Jesse Barnes @ 2009-06-23 1:58 ` Benjamin Herrenschmidt 2009-06-23 2:07 ` Jesse Barnes 2009-06-23 7:48 ` Michel Dänzer 1 sibling, 1 reply; 40+ messages in thread From: Benjamin Herrenschmidt @ 2009-06-23 1:58 UTC (permalink / raw) To: Jesse Barnes Cc: Linus Torvalds, Thomas Hellström, Dave Airlie, Alex Deucher, Andrew Lutomirski, dri-devel, Jerome Glisse, Linux Kernel Mailing List On Mon, 2009-06-22 at 18:18 -0700, Jesse Barnes wrote: > I think it could work, but ideally we'd keep the kernel fbcon object > pinned, and keep printing into it even while some other gfx app is > running. That way we don't have to dump the whole queue into it when > a > panic occurs, we can just switch buffers (something like this would > also be handy for dual head debugging; one head running your desktop > and the other a debug console printing all the messages). That's > slightly more invasive surgery though... I should have a chance to do > something like that as part of the kdb/kms work I'll be doing with > Jason. > Do we really need that ? We can easily repaint (ie, regenerate the fb content from the pseudo vgacon image kept by the console layer). So if we want the kernel to "take" over, it's reasonably easy to make it also repaint the content of the fb. How, of course, kicking out usespace with unmap_mapping_ranges() isn't going to work well from an oops or something at interrupt time, we do need to have a reasonably safe path for these things, which is why I believe that sort of emergency printing should be done without any acceleration, just basic manual painting in the front buffer... Should we even bother changing the mode ? Not sure... Cheers, Ben. ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [git pull] drm: previous pull req + 1. 2009-06-23 1:58 ` Benjamin Herrenschmidt @ 2009-06-23 2:07 ` Jesse Barnes 2009-06-23 2:26 ` Benjamin Herrenschmidt 0 siblings, 1 reply; 40+ messages in thread From: Jesse Barnes @ 2009-06-23 2:07 UTC (permalink / raw) To: Benjamin Herrenschmidt Cc: Linus Torvalds, Thomas Hellström, Dave Airlie, Alex Deucher, Andrew Lutomirski, dri-devel, Jerome Glisse, Linux Kernel Mailing List On Tue, 23 Jun 2009 11:58:44 +1000 Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote: > On Mon, 2009-06-22 at 18:18 -0700, Jesse Barnes wrote: > > I think it could work, but ideally we'd keep the kernel fbcon object > > pinned, and keep printing into it even while some other gfx app is > > running. That way we don't have to dump the whole queue into it > > when a > > panic occurs, we can just switch buffers (something like this would > > also be handy for dual head debugging; one head running your desktop > > and the other a debug console printing all the messages). That's > > slightly more invasive surgery though... I should have a chance to > > do something like that as part of the kdb/kms work I'll be doing > > with Jason. > > > Do we really need that ? > > We can easily repaint (ie, regenerate the fb content from the pseudo > vgacon image kept by the console layer). > > So if we want the kernel to "take" over, it's reasonably easy to > make it also repaint the content of the fb. For just repainting panic messages, probably not. For the dual head case I talked about though it would sure be nice... > How, of course, kicking out usespace with unmap_mapping_ranges() isn't > going to work well from an oops or something at interrupt time, we > do need to have a reasonably safe path for these things, which is why > I believe that sort of emergency printing should be done without > any acceleration, just basic manual painting in the front buffer... > > Should we even bother changing the mode ? Not sure... Yeah I don't think we should try to change the mode, unless we really have to for whatever reason. fbcon should generally be able to paint to whatever we have up as long as we set it up properly. -- Jesse Barnes, Intel Open Source Technology Center ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [git pull] drm: previous pull req + 1. 2009-06-23 2:07 ` Jesse Barnes @ 2009-06-23 2:26 ` Benjamin Herrenschmidt 2009-06-23 15:40 ` Jesse Barnes 0 siblings, 1 reply; 40+ messages in thread From: Benjamin Herrenschmidt @ 2009-06-23 2:26 UTC (permalink / raw) To: Jesse Barnes Cc: Linus Torvalds, Thomas Hellström, Dave Airlie, Alex Deucher, Andrew Lutomirski, dri-devel, Jerome Glisse, Linux Kernel Mailing List On Mon, 2009-06-22 at 19:07 -0700, Jesse Barnes wrote: > Yeah I don't think we should try to change the mode, unless we really > have to for whatever reason. fbcon should generally be able to paint > to whatever we have up as long as we set it up properly. Well... it may not. IE. The text consoles can be using a different mode than X, and so fbcon will be all setup for that ... (including how many lines/columns etc...). So it's not totally trivial... but on the other hands, it would be useful to avoid as much as possible complicated things like mode switches when hitting an oops. Cheers, Ben. ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [git pull] drm: previous pull req + 1. 2009-06-23 2:26 ` Benjamin Herrenschmidt @ 2009-06-23 15:40 ` Jesse Barnes 0 siblings, 0 replies; 40+ messages in thread From: Jesse Barnes @ 2009-06-23 15:40 UTC (permalink / raw) To: Benjamin Herrenschmidt Cc: Linus Torvalds, Thomas Hellström, Dave Airlie, Alex Deucher, Andrew Lutomirski, dri-devel, Jerome Glisse, Linux Kernel Mailing List On Tue, 23 Jun 2009 12:26:15 +1000 Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote: > On Mon, 2009-06-22 at 19:07 -0700, Jesse Barnes wrote: > > > Yeah I don't think we should try to change the mode, unless we > > really have to for whatever reason. fbcon should generally be able > > to paint to whatever we have up as long as we set it up properly. > > Well... it may not. IE. The text consoles can be using a different > mode than X, and so fbcon will be all setup for that ... (including > how many lines/columns etc...). Yeah, I was suggesting we change fbcon's view of the world at panic time, rather than trying to change the mode. It's a bit of surgery though... -- Jesse Barnes, Intel Open Source Technology Center ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [git pull] drm: previous pull req + 1. 2009-06-23 1:18 ` Jesse Barnes 2009-06-23 1:58 ` Benjamin Herrenschmidt @ 2009-06-23 7:48 ` Michel Dänzer 2009-06-23 15:39 ` Jesse Barnes 1 sibling, 1 reply; 40+ messages in thread From: Michel Dänzer @ 2009-06-23 7:48 UTC (permalink / raw) To: Jesse Barnes Cc: Benjamin Herrenschmidt, Thomas, Dave Airlie, Andrew Lutomirski, Linux Kernel Mailing List, Jerome Glisse, dri-devel, Alex Deucher, Linus Torvalds On Mon, 2009-06-22 at 18:18 -0700, Jesse Barnes wrote: > On Tue, 23 Jun 2009 11:04:39 +1000 > Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote: > > > On Mon, 2009-06-22 at 17:24 -0700, Linus Torvalds wrote: > > > > > > On Tue, 23 Jun 2009, Benjamin Herrenschmidt wrote: > > > > > > > > As far as I can remember, all fbdev operations are done under the > > > > console semaphore. > > > > > > Yeah, and some of them are horribly broken (ie copying data from > > > user space while doing it - causing horrible things like VC > > > switching latencies and invisible printk's if an oops happens > > > during the op). > > > > > > Or maybe that got fixed. > > > > Well, it does rely on userspace behaving.. ie, no accel ops are done > > by the kernel in KD_GRAPHICS and userspace is -supposed- to switch to > > KD_GRAPHICS before touching the fb. > > > > In fact, nowdays, we do have the infrastructure to be smart and > > enforce that. IE. Instead of using a boring remap_page_ranges() in > > fb_mmap() we could use a fault handler. When in KD_TEXT, we fail > > them, when in KD_GRAPHICS, we service them, and we > > unmap_mapping_range() when switching. Something like that... > > > > Dunno how that interacts with the new DRM thingy though. > > I think it could work, but ideally we'd keep the kernel fbcon object > pinned, and keep printing into it even while some other gfx app is > running. It doesn't need to be pinned for that, does it? I think in the long run it's a bad idea to have it pinned all the time, think of machines with only 8 MB of VRAM... > (something like this would also be handy for dual head debugging; one > head running your desktop and the other a debug console printing all > the messages). On a side note, I did precisely that about ten years ago on my Amiga. :) Granted, that was using two separate framebuffer devices (X glint driver on top of pm2fb, debug messages on amifb), but I think even that case isn't possible ATM. I agree it would be nice, though realistically there's hardly a way around a second machine for graphics driver development anyway. -- Earthling Michel Dänzer | http://www.vmware.com Libre software enthusiast | Debian, X and DRI developer ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [git pull] drm: previous pull req + 1. 2009-06-23 7:48 ` Michel Dänzer @ 2009-06-23 15:39 ` Jesse Barnes 2009-06-23 16:28 ` Jesse Barnes 0 siblings, 1 reply; 40+ messages in thread From: Jesse Barnes @ 2009-06-23 15:39 UTC (permalink / raw) To: Michel Dänzer Cc: Benjamin Herrenschmidt, Thomas, Dave Airlie, Andrew Lutomirski, Linux Kernel Mailing List, Jerome Glisse, dri-devel, Alex Deucher, Linus Torvalds On Tue, 23 Jun 2009 09:48:00 +0200 Michel Dänzer <michel@daenzer.net> wrote: > > > In fact, nowdays, we do have the infrastructure to be smart and > > > enforce that. IE. Instead of using a boring remap_page_ranges() in > > > fb_mmap() we could use a fault handler. When in KD_TEXT, we fail > > > them, when in KD_GRAPHICS, we service them, and we > > > unmap_mapping_range() when switching. Something like that... > > > > > > Dunno how that interacts with the new DRM thingy though. > > > > I think it could work, but ideally we'd keep the kernel fbcon object > > pinned, and keep printing into it even while some other gfx app is > > running. > > It doesn't need to be pinned for that, does it? I think in the long > run it's a bad idea to have it pinned all the time, think of machines > with only 8 MB of VRAM... Yeah, in the general case we shouldn't have to pin it... > > (something like this would also be handy for dual head debugging; > > one head running your desktop and the other a debug console > > printing all the messages). > > On a side note, I did precisely that about ten years ago on my > Amiga. :) Granted, that was using two separate framebuffer devices (X > glint driver on top of pm2fb, debug messages on amifb), but I think > even that case isn't possible ATM. I agree it would be nice, though > realistically there's hardly a way around a second machine for > graphics driver development anyway. Oh I know, most operating systems have reasonable debugging facilities, and have had for a long time. Linux is the one lagging here. I agree it probably won't be that helpful for low level gfx debugging, but I'm trying to think beyond that too. :) -- Jesse Barnes, Intel Open Source Technology Center ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [git pull] drm: previous pull req + 1. 2009-06-23 15:39 ` Jesse Barnes @ 2009-06-23 16:28 ` Jesse Barnes 0 siblings, 0 replies; 40+ messages in thread From: Jesse Barnes @ 2009-06-23 16:28 UTC (permalink / raw) Cc: Michel Dänzer, Benjamin Herrenschmidt, Thomas, Dave Airlie, Andrew Lutomirski, Linux Kernel Mailing List, Jerome Glisse, dri-devel, Alex Deucher, Linus Torvalds On Tue, 23 Jun 2009 08:39:44 -0700 Jesse Barnes <jbarnes@virtuousgeek.org> wrote: > On Tue, 23 Jun 2009 09:48:00 +0200 > Michel Dänzer <michel@daenzer.net> wrote: > > On a side note, I did precisely that about ten years ago on my > > Amiga. :) Granted, that was using two separate framebuffer devices > > (X glint driver on top of pm2fb, debug messages on amifb), but I > > think even that case isn't possible ATM. I agree it would be nice, > > though realistically there's hardly a way around a second machine > > for graphics driver development anyway. > > Oh I know, most operating systems have reasonable debugging > facilities, and have had for a long time. Linux is the one lagging > here. > Heh, I saw "amiga" and stopped reading. So it seems Linux on x86 is the main laggard here... I know PPC has had interesting debug features for awhile now, and most arches have had kgdb for a long time. Anyway, there's plenty of work to do in this area to give Linux a decent system level debugger... -- Jesse Barnes, Intel Open Source Technology Center ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [git pull] drm: previous pull req + 1. 2009-06-21 19:47 ` Linus Torvalds 2009-06-21 21:14 ` Andrew Lutomirski 2009-06-21 22:40 ` Dave Airlie @ 2009-06-22 23:57 ` Benjamin Herrenschmidt 2 siblings, 0 replies; 40+ messages in thread From: Benjamin Herrenschmidt @ 2009-06-22 23:57 UTC (permalink / raw) To: Linus Torvalds Cc: Andrew Lutomirski, Dave Airlie, dri-devel, Linux Kernel Mailing List, Jerome Glisse, Alex Deucher On Sun, 2009-06-21 at 12:47 -0700, Linus Torvalds wrote: > > What *has* changed is that we have a newradeon driver, and it looks > like > that new radeon driver is crap, and does this: > > info->fix.smem_start = (unsigned long)fbptr; > > which is totally screwed up. It assigns a _virtual_ address to that > "smem_start" thing, even though it should be a physical one. BTW. While we are at it that there's an additional problem (old fbdev design issue) with that which is smem_start is an unsigned long, which is too small to hold a physical address on a variety of 32-bit platforms. Dave, what about we split those structures into a "sane" in-kernel one and a "user visible" one ? At that point, we can create new saner variants of the user ones using separate ioctls ... > I don't know the radeon driver, so I don't know where to find the > physical address. It's also possible that there is no good single > physical address, and the radeon driver should implement a "fb_mmap" > function. > > Does this patch make the warning and the oops at least go away? > Obviously it won't result in a working frame buffer, but that's a > separate issue There should be a physical address but it's nasty to let userspace map it directly without using the DRM for access control, not sure what David plans here, but it may need surface control set properly etc... Dave ? Should the fbdev mmap go down the DRM object map path or similar ? Cheers, Ben. ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [git pull] drm: previous pull req + 1. 2009-06-21 16:46 ` Linus Torvalds 2009-06-21 17:13 ` Linus Torvalds @ 2009-06-21 22:41 ` Dave Airlie 1 sibling, 0 replies; 40+ messages in thread From: Dave Airlie @ 2009-06-21 22:41 UTC (permalink / raw) To: Linus Torvalds; +Cc: Andy Lutomirski, linux-kernel, dri-devel > > > On Sun, 21 Jun 2009, Dave Airlie wrote: > > > > > > I tried this tree (specifically, a merge of Linus' fb20871 this tree) on > > > Fedora 11 with modesetting enabled on an integrated Radeon 2100, and plymouthd > > > crashes immediately with a corrupt page table. Photo attached. After the > > > crash, bootup stops, although ctrl-alt-del works. > > > > You need a different userspace, -ati from koji in F11 should do it. > > Dave - no amount of userspace differences make a corrupted page table > acceptable. I was original responding to a line I saw in his dmesg (which was about an ioctl not being available, I only got to parsing the crash later. > This needs to be fixed. No excuses. Kernel crashes are never an issue of > "you used the wrong user space". Agreed, and we block old userspaces running on KMS kernels (at least trying to use DRI) and I thought that was what he was seeing. Dave. > > Linus > > ------------------------------------------------------------------------------ > Are you an open source citizen? Join us for the Open Source Bridge conference! > Portland, OR, June 17-19. Two days of sessions, one day of unconference: $250. > Need another reason to go? 24-hour hacker lounge. Register today! > http://ad.doubleclick.net/clk;215844324;13503038;v?http://opensourcebridge.org > -- > _______________________________________________ > Dri-devel mailing list > Dri-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/dri-devel > > ^ permalink raw reply [flat|nested] 40+ messages in thread
end of thread, other threads:[~2009-07-10 18:03 UTC | newest] Thread overview: 40+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2009-06-20 5:23 [git pull] drm: previous pull req + 1 Dave Airlie 2009-06-21 0:04 ` Maxim Levitsky 2009-06-21 0:42 ` Linus Torvalds 2009-06-21 14:47 ` Maxim Levitsky 2009-06-21 21:24 ` Chris Wilson 2009-06-22 18:09 ` Maxim Levitsky 2009-06-29 7:57 ` Chris Wilson 2009-06-30 9:49 ` Chris Wilson 2009-07-09 23:11 ` Eric Anholt 2009-06-21 1:33 ` Dave Airlie 2009-06-21 3:41 ` Andy Lutomirski 2009-06-21 5:16 ` Dave Airlie 2009-06-21 12:06 ` Andrew Lutomirski 2009-06-21 16:46 ` Linus Torvalds 2009-06-21 17:13 ` Linus Torvalds 2009-06-21 18:50 ` Andrew Lutomirski 2009-06-21 19:47 ` Linus Torvalds 2009-06-21 21:14 ` Andrew Lutomirski 2009-06-22 0:05 ` Andrew Lutomirski 2009-06-22 19:20 ` Arnaldo Carvalho de Melo 2009-06-21 22:40 ` Dave Airlie 2009-06-22 8:18 ` Thomas Hellström 2009-06-22 8:30 ` Dave Airlie 2009-06-22 18:22 ` Linus Torvalds 2009-06-22 18:59 ` Andrew Lutomirski 2009-06-22 19:43 ` Linus Torvalds 2009-06-23 0:01 ` Benjamin Herrenschmidt 2009-06-23 0:00 ` Benjamin Herrenschmidt 2009-06-23 0:24 ` Linus Torvalds 2009-06-23 1:04 ` Benjamin Herrenschmidt 2009-06-23 1:18 ` Jesse Barnes 2009-06-23 1:58 ` Benjamin Herrenschmidt 2009-06-23 2:07 ` Jesse Barnes 2009-06-23 2:26 ` Benjamin Herrenschmidt 2009-06-23 15:40 ` Jesse Barnes 2009-06-23 7:48 ` Michel Dänzer 2009-06-23 15:39 ` Jesse Barnes 2009-06-23 16:28 ` Jesse Barnes 2009-06-22 23:57 ` Benjamin Herrenschmidt 2009-06-21 22:41 ` Dave Airlie
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.