[PATCH 00/12] kill-with-fire, resurrected 2nd part

List overview All Threads
Download

newer

older

[PATCH] vgaarb: call...

[PATCH] drm/sis: fixup sis_mm...

Daniel Vetter

14 Jun 2012 14 Jun '12

10:24 a.m.

Hi all,

Blind me has finally found the bug that killed via - dropping locks again in the early exit path is highly advised ;-)

The big part of this series is cleaning up buffer handling in the teardown/lastclose path: All special-cases only used by 1-2 drivers are moved into the drivers respective preclose functions (newly created in most cases), the remaining stuff is streamlined. The only things left in the common code which is not used by all drivers is used by at least a few of them.

So after this patch series the drm_release/lastclose stuff is now fairly readable and understandable. Originally I wanted to fix up a few of the resource leaks that are possible at module unload time, but got stuck at understanding the release/lastclose stuff ;-)

4 drivers needed special treatement, unfortunately I couldn't get a tested by for sis. I've tried to ebay one of these old cards and found one that works with the sisfb and sis ddx driver, but unfortunately the sis mesa driver only supports one specific generations. But since drm/sis is more or less a 1:1 copy of drm/via I'm fairly sure that this will work, too.

The last two patches just kill stuff that I've noticed while crawling around in the close/release paths.

As usual, comments, flames, reviews (and especially from a sis users) tested-bys highly welcome.

Yours, Daniel

Daniel Vetter (12): drm/via: clean up reclaim_buffers drm/sis: clean up reclaim_buffers drm: kill reclaim_buffers_idlelocked functions Revert "Revert "drm/i810: cleanup reclaim_buffers"" drm: kill reclaim_buffers_locked drm/savage: clean up reclaim_buffers drm: kill reclaim_buffers callback drm: ditch strange DRIVER_DMA_QUEUE only error bail-out drm: kill dma queue support drm: unconditionally clean up dma buffers of closing clients drm: kill i915/i830 ids from drm_pciids.h drm: kill procfs support

-- 1.7.10

Show replies by date

Daniel Vetter

14 Jun 14 Jun

10:24 a.m.

New subject: [PATCH 01/12] drm/via: clean up reclaim_buffers

A few things - kill reclaim_buffers, it's never ever called because via does not set DRIVER_HAVE_DMA - inline the idlelock dance into the buffer reclaim logic and make it a simple preclose cleanup function - directly call the the dma_quiescent function and kill the needless if check.

v2: Actually drop the idlelock when we take it. Reported by James Simmons.

v3: Rebased onto latest drm-next.

v4: Fixup the refactor.

v5: More fixup the refactor - I've accidentally changed the check for any master to checking whether the closing fd is the master.

v6: Don't forget to drop the idlelock in the early return path, too.

Tested-by: Jakob Bornecrantz jakob@vmware.com Signed-off-by: Daniel Vetter daniel.vetter@ffwll.ch --- drivers/gpu/drm/via/via_drv.c | 4 +--- drivers/gpu/drm/via/via_mm.c | 13 +++++++++++-- 2 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/via/via_drv.c b/drivers/gpu/drm/via/via_drv.c index 02661f3..e927b4c 100644 --- a/drivers/gpu/drm/via/via_drv.c +++ b/drivers/gpu/drm/via/via_drv.c @@ -75,6 +75,7 @@ static struct drm_driver driver = { .load = via_driver_load, .unload = via_driver_unload, .open = via_driver_open, + .preclose = via_reclaim_buffers_locked, .postclose = via_driver_postclose, .context_dtor = via_final_context, .get_vblank_counter = via_get_vblank_counter, @@ -85,9 +86,6 @@ static struct drm_driver driver = { .irq_uninstall = via_driver_irq_uninstall, .irq_handler = via_driver_irq_handler, .dma_quiescent = via_driver_dma_quiescent, - .reclaim_buffers = drm_core_reclaim_buffers, - .reclaim_buffers_locked = NULL, - .reclaim_buffers_idlelocked = via_reclaim_buffers_locked, .lastclose = via_lastclose, .ioctls = via_ioctls, .fops = &via_driver_fops, diff --git a/drivers/gpu/drm/via/via_mm.c b/drivers/gpu/drm/via/via_mm.c index a3574d0..acfcb35 100644 --- a/drivers/gpu/drm/via/via_mm.c +++ b/drivers/gpu/drm/via/via_mm.c @@ -215,14 +215,20 @@ void via_reclaim_buffers_locked(struct drm_device *dev, struct via_file_private *file_priv = file->driver_priv; struct via_memblock *entry, *next;

+ if (!(file->minor->master && file->master->lock.hw_lock)) + return; + + drm_idlelock_take(&file->master->lock); + mutex_lock(&dev->struct_mutex); if (list_empty(&file_priv->obj_list)) { mutex_unlock(&dev->struct_mutex); + drm_idlelock_release(&file->master->lock); + return; }

- if (dev->driver->dma_quiescent) - dev->driver->dma_quiescent(dev); + via_driver_dma_quiescent(dev);

list_for_each_entry_safe(entry, next, &file_priv->obj_list, owner_list) { @@ -231,5 +237,8 @@ void via_reclaim_buffers_locked(struct drm_device *dev, kfree(entry); } mutex_unlock(&dev->struct_mutex); + + drm_idlelock_release(&file->master->lock); + return; }

-- 1.7.10

Daniel Vetter

10:24 a.m.

New subject: [PATCH 02/12] drm/sis: clean up reclaim_buffers

Like for via.

v2: Actually drop the idlelock again if taken.

v3: Fixup.

v4: Fixup the "has master" vs. "is master" confusion the refactor introduced.

v5: Drop the idlelock in the early return path.

Signed-off-by: Daniel Vetter daniel.vetter@ffwll.ch --- drivers/gpu/drm/sis/sis_drv.c | 3 +-- drivers/gpu/drm/sis/sis_mm.c | 13 +++++++++++-- 2 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/sis/sis_drv.c b/drivers/gpu/drm/sis/sis_drv.c index c7263a4..614029d 100644 --- a/drivers/gpu/drm/sis/sis_drv.c +++ b/drivers/gpu/drm/sis/sis_drv.c @@ -105,10 +105,9 @@ static struct drm_driver driver = { .load = sis_driver_load, .unload = sis_driver_unload, .open = sis_driver_open, + .preclose = sis_reclaim_buffers_locked, .postclose = sis_driver_postclose, .dma_quiescent = sis_idle, - .reclaim_buffers = NULL, - .reclaim_buffers_idlelocked = sis_reclaim_buffers_locked, .lastclose = sis_lastclose, .ioctls = sis_ioctls, .fops = &sis_driver_fops, diff --git a/drivers/gpu/drm/sis/sis_mm.c b/drivers/gpu/drm/sis/sis_mm.c index dd4a316..5acc396 100644 --- a/drivers/gpu/drm/sis/sis_mm.c +++ b/drivers/gpu/drm/sis/sis_mm.c @@ -321,14 +321,20 @@ void sis_reclaim_buffers_locked(struct drm_device *dev, struct sis_file_private *file_priv = file->driver_priv; struct sis_memblock *entry, *next;

- if (dev->driver->dma_quiescent) - dev->driver->dma_quiescent(dev); + sis_idle(dev);

list_for_each_entry_safe(entry, next, &file_priv->obj_list, @@ -343,6 +349,9 @@ void sis_reclaim_buffers_locked(struct drm_device *dev, kfree(entry); } mutex_unlock(&dev->struct_mutex); + + drm_idlelock_release(&file->master->lock); + return; }

-- 1.7.10

Daniel Vetter

24 Jun 24 Jun

6:09 p.m.

New subject: [PATCH 02/12] drm/sis: clean up reclaim_buffers

On Thu, Jun 14, 2012 at 12:24:57PM +0200, Daniel Vetter wrote:

...

Like for via.

v2: Actually drop the idlelock again if taken.

v3: Fixup.

v4: Fixup the "has master" vs. "is master" confusion the refactor introduced.

v5: Drop the idlelock in the early return path.

Signed-off-by: Daniel Vetter daniel.vetter@ffwll.ch

After a few trials of hunting around in the ebay dungeons I've managed to scrap together a sis system which actually works on mesa 7.11. Hence this (and the previous sis reworks) are now tested both with and without sis_fb (different codepaths and memory managers) on a 64bit SMP machine. Card is a 305 AGP sis:

5:00.0 VGA compatible controller [0300]: Silicon Integrated Systems [SiS] 300/305 PCI/AGP VGA Display Adapter [1039:0300] (rev 90)

Cheers, Daniel

...

drivers/gpu/drm/sis/sis_drv.c | 3 +-- drivers/gpu/drm/sis/sis_mm.c | 13 +++++++++++-- 2 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/sis/sis_drv.c b/drivers/gpu/drm/sis/sis_drv.c index c7263a4..614029d 100644 --- a/drivers/gpu/drm/sis/sis_drv.c +++ b/drivers/gpu/drm/sis/sis_drv.c @@ -105,10 +105,9 @@ static struct drm_driver driver = { .load = sis_driver_load, .unload = sis_driver_unload, .open = sis_driver_open,

.preclose = sis_reclaim_buffers_locked, .postclose = sis_driver_postclose, .dma_quiescent = sis_idle,

.reclaim_buffers = NULL,

.reclaim_buffers_idlelocked = sis_reclaim_buffers_locked, .lastclose = sis_lastclose, .ioctls = sis_ioctls, .fops = &sis_driver_fops,

diff --git a/drivers/gpu/drm/sis/sis_mm.c b/drivers/gpu/drm/sis/sis_mm.c index dd4a316..5acc396 100644 --- a/drivers/gpu/drm/sis/sis_mm.c +++ b/drivers/gpu/drm/sis/sis_mm.c @@ -321,14 +321,20 @@ void sis_reclaim_buffers_locked(struct drm_device *dev, struct sis_file_private *file_priv = file->driver_priv; struct sis_memblock *entry, *next;
if (!(file->minor->master && file->master->lock.hw_lock))
return;
drm_idlelock_take(&file->master->lock);

mutex_lock(&dev->struct_mutex); if (list_empty(&file_priv->obj_list)) { mutex_unlock(&dev->struct_mutex);
drm_idlelock_release(&file->master->lock);
return; }
if (dev->driver->dma_quiescent)
dev->driver->dma_quiescent(dev);
sis_idle(dev);

list_for_each_entry_safe(entry, next, &file_priv->obj_list,

@@ -343,6 +349,9 @@ void sis_reclaim_buffers_locked(struct drm_device *dev, kfree(entry); } mutex_unlock(&dev->struct_mutex);

drm_idlelock_release(&file->master->lock);

return;

}

-- 1.7.10

-- Daniel Vetter Mail: daniel@ffwll.ch Mobile: +41 (0)79 365 57 48

Daniel Vetter

14 Jun 14 Jun

10:24 a.m.

New subject: [PATCH 03/12] drm: kill reclaim_buffers_idlelocked functions

The only two users are now folded into the drivers preclose functions, so this is unused.

Signed-off-by: Daniel Vetter daniel.vetter@ffwll.ch --- drivers/gpu/drm/drm_fops.c | 8 -------- include/drm/drmP.h | 2 -- 2 files changed, 10 deletions(-)

diff --git a/drivers/gpu/drm/drm_fops.c b/drivers/gpu/drm/drm_fops.c index 123de28..f6231f02 100644 --- a/drivers/gpu/drm/drm_fops.c +++ b/drivers/gpu/drm/drm_fops.c @@ -417,14 +417,6 @@ static void drm_master_release(struct drm_device *dev, struct file *filp) file_priv->master->lock.hw_lock) drm_reclaim_locked_buffers(dev, filp);

- if (dev->driver->reclaim_buffers_idlelocked && - file_priv->master->lock.hw_lock) { - drm_idlelock_take(&file_priv->master->lock); - dev->driver->reclaim_buffers_idlelocked(dev, file_priv); - drm_idlelock_release(&file_priv->master->lock); - } - - if (drm_i_have_hw_lock(dev, file_priv)) { DRM_DEBUG("File %p released, freeing lock for context %d\n", filp, _DRM_LOCKING_CONTEXT(file_priv->master->lock.hw_lock->lock)); diff --git a/include/drm/drmP.h b/include/drm/drmP.h index 31ad880..005af91 100644 --- a/include/drm/drmP.h +++ b/include/drm/drmP.h @@ -880,8 +880,6 @@ struct drm_driver { struct drm_file * file_priv); void (*reclaim_buffers_locked) (struct drm_device *dev, struct drm_file *file_priv); - void (*reclaim_buffers_idlelocked) (struct drm_device *dev, - struct drm_file *file_priv); void (*set_version) (struct drm_device *dev, struct drm_set_version *sv);

-- 1.7.10

Daniel Vetter

10:24 a.m.

New subject: [PATCH 04/12] Revert "Revert "drm/i810: cleanup reclaim_buffers""

This reverts commit 6e877b576ddf7cde5db2e9a6dcb56fef0ea77e64, reinstating the original commit:

commit 87499ffdcb1c70f66988cd8febc4ead0ba2f9118 Author: Daniel Vetter daniel.vetter@ffwll.ch Date: Tue Oct 25 23:51:24 2011 +0200

drm/i810: cleanup reclaim_buffers

My dear old i815 always hits the deadlocked on reclaim_buffers warning. Switch over to the idlelock duct-tape on hope that works better. I've fired up my i815 and now closing glxgears doesn't take 5 seconds anymore. \o/

The original problem with that was that I've moved it ahead in the series so that it could be included despite some patches not being ready quite yet. The little problem is that this patch required some of the previous rework to work correctly.

Now that everything is in the right order again, this actually works on my i810 and does speed up closing gl apps as the original commit claimed. Without hanging the machine, as the revert says.

Signed-Off-by: Daniel Vetter daniel.vetter@ffwll.ch --- drivers/gpu/drm/i810/i810_dma.c | 17 +++++++++++------ drivers/gpu/drm/i810/i810_drv.c | 1 - drivers/gpu/drm/i810/i810_drv.h | 6 ++---- 3 files changed, 13 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i810/i810_dma.c b/drivers/gpu/drm/i810/i810_dma.c index fa94391..57d892e 100644 --- a/drivers/gpu/drm/i810/i810_dma.c +++ b/drivers/gpu/drm/i810/i810_dma.c @@ -881,7 +881,7 @@ static int i810_flush_queue(struct drm_device *dev) }

/* Must be called with the lock held */ -static void i810_reclaim_buffers(struct drm_device *dev, +void i810_driver_reclaim_buffers(struct drm_device *dev, struct drm_file *file_priv) { struct drm_device_dma *dma = dev->dma; @@ -1220,12 +1220,17 @@ void i810_driver_preclose(struct drm_device *dev, struct drm_file *file_priv) if (dev_priv->page_flipping) i810_do_cleanup_pageflip(dev); } -}

-void i810_driver_reclaim_buffers_locked(struct drm_device *dev, - struct drm_file *file_priv) -{ - i810_reclaim_buffers(dev, file_priv); + if (file_priv->master && file_priv->master->lock.hw_lock) { + drm_idlelock_take(&file_priv->master->lock); + i810_driver_reclaim_buffers(dev, file_priv); + drm_idlelock_release(&file_priv->master->lock); + } else { + /* master disappeared, clean up stuff anyway and hope nothing + * goes wrong */ + i810_driver_reclaim_buffers(dev, file_priv); + } + }

int i810_driver_dma_quiescent(struct drm_device *dev) diff --git a/drivers/gpu/drm/i810/i810_drv.c b/drivers/gpu/drm/i810/i810_drv.c index ec12f7d..053f1ee 100644 --- a/drivers/gpu/drm/i810/i810_drv.c +++ b/drivers/gpu/drm/i810/i810_drv.c @@ -63,7 +63,6 @@ static struct drm_driver driver = { .lastclose = i810_driver_lastclose, .preclose = i810_driver_preclose, .device_is_agp = i810_driver_device_is_agp, - .reclaim_buffers_locked = i810_driver_reclaim_buffers_locked, .dma_quiescent = i810_driver_dma_quiescent, .ioctls = i810_ioctls, .fops = &i810_driver_fops, diff --git a/drivers/gpu/drm/i810/i810_drv.h b/drivers/gpu/drm/i810/i810_drv.h index c9339f4..6e0acad 100644 --- a/drivers/gpu/drm/i810/i810_drv.h +++ b/drivers/gpu/drm/i810/i810_drv.h @@ -116,14 +116,12 @@ typedef struct drm_i810_private {

/* i810_dma.c */ extern int i810_driver_dma_quiescent(struct drm_device *dev); -extern void i810_driver_reclaim_buffers_locked(struct drm_device *dev, - struct drm_file *file_priv); +void i810_driver_reclaim_buffers(struct drm_device *dev, + struct drm_file *file_priv); extern int i810_driver_load(struct drm_device *, unsigned long flags); extern void i810_driver_lastclose(struct drm_device *dev); extern void i810_driver_preclose(struct drm_device *dev, struct drm_file *file_priv); -extern void i810_driver_reclaim_buffers_locked(struct drm_device *dev, - struct drm_file *file_priv); extern int i810_driver_device_is_agp(struct drm_device *dev);

extern long i810_ioctl(struct file *file, unsigned int cmd, unsigned long arg);

-- 1.7.10

Daniel Vetter

10:25 a.m.

New subject: [PATCH 05/12] drm: kill reclaim_buffers_locked

i810 was the last user of this code, with that gone, kill it.

Signed-off-by: Daniel Vetter daniel.vetter@ffwll.ch --- drivers/gpu/drm/drm_fops.c | 46 +---------------------------------- drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 1 - include/drm/drmP.h | 2 -- 3 files changed, 1 insertion(+), 48 deletions(-)

diff --git a/drivers/gpu/drm/drm_fops.c b/drivers/gpu/drm/drm_fops.c index f6231f02..6cf9369 100644 --- a/drivers/gpu/drm/drm_fops.c +++ b/drivers/gpu/drm/drm_fops.c @@ -370,53 +370,10 @@ int drm_fasync(int fd, struct file *filp, int on) } EXPORT_SYMBOL(drm_fasync);

-/* - * Reclaim locked buffers; note that this may be a bad idea if the current - * context doesn't have the hw lock... - */ -static void drm_reclaim_locked_buffers(struct drm_device *dev, struct file *f) -{ - struct drm_file *file_priv = f->private_data; - - if (drm_i_have_hw_lock(dev, file_priv)) { - dev->driver->reclaim_buffers_locked(dev, file_priv); - } else { - unsigned long _end = jiffies + 3 * DRM_HZ; - int locked = 0; - - drm_idlelock_take(&file_priv->master->lock); - - /* - * Wait for a while. - */ - do { - spin_lock_bh(&file_priv->master->lock.spinlock); - locked = file_priv->master->lock.idle_has_lock; - spin_unlock_bh(&file_priv->master->lock.spinlock); - if (locked) - break; - schedule(); - } while (!time_after_eq(jiffies, _end)); - - if (!locked) { - DRM_ERROR("reclaim_buffers_locked() deadlock. Please rework this\n" - "\tdriver to use reclaim_buffers_idlelocked() instead.\n" - "\tI will go on reclaiming the buffers anyway.\n"); - } - - dev->driver->reclaim_buffers_locked(dev, file_priv); - drm_idlelock_release(&file_priv->master->lock); - } -} - static void drm_master_release(struct drm_device *dev, struct file *filp) { struct drm_file *file_priv = filp->private_data;

- if (dev->driver->reclaim_buffers_locked && - file_priv->master->lock.hw_lock) - drm_reclaim_locked_buffers(dev, filp); - if (drm_i_have_hw_lock(dev, file_priv)) { DRM_DEBUG("File %p released, freeing lock for context %d\n", filp, _DRM_LOCKING_CONTEXT(file_priv->master->lock.hw_lock->lock)); @@ -424,8 +381,7 @@ static void drm_master_release(struct drm_device *dev, struct file *filp) _DRM_LOCKING_CONTEXT(file_priv->master->lock.hw_lock->lock)); }

- if (drm_core_check_feature(dev, DRIVER_HAVE_DMA) && - !dev->driver->reclaim_buffers_locked) { + if (drm_core_check_feature(dev, DRIVER_HAVE_DMA)) { dev->driver->reclaim_buffers(dev, file_priv); } } diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c index ee24d21..5d5632f 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c @@ -1147,7 +1147,6 @@ static struct drm_driver driver = { .get_vblank_counter = vmw_get_vblank_counter, .enable_vblank = vmw_enable_vblank, .disable_vblank = vmw_disable_vblank, - .reclaim_buffers_locked = NULL, .ioctls = vmw_ioctls, .num_ioctls = DRM_ARRAY_SIZE(vmw_ioctls), .dma_quiescent = NULL, /*vmw_dma_quiescent, */ diff --git a/include/drm/drmP.h b/include/drm/drmP.h index 005af91..8ee41e6 100644 --- a/include/drm/drmP.h +++ b/include/drm/drmP.h @@ -878,8 +878,6 @@ struct drm_driver { void (*irq_uninstall) (struct drm_device *dev); void (*reclaim_buffers) (struct drm_device *dev, struct drm_file * file_priv); - void (*reclaim_buffers_locked) (struct drm_device *dev, - struct drm_file *file_priv); void (*set_version) (struct drm_device *dev, struct drm_set_version *sv);

-- 1.7.10

Daniel Vetter

10:25 a.m.

New subject: [PATCH 06/12] drm/savage: clean up reclaim_buffers

The reclaim_buffers function of the savage driver actually wants to run with the hw_lock held - at least there are printks in the call-chain to that effect. But the drm core only calls reclaim_buffers as used by savage _after_ forcefully dropping the hwlock (in case it's still hold by the closing fd).

So do the same idlelock dance as for the other dma drivers and hope that papers over any issues.

v2: Don't let the idlelock linger around.

Signed-off-by: Daniel Vetter daniel.vetter@ffwll.ch Tested-by: Tormod Volden debian.tormod@gmail.com --- drivers/gpu/drm/savage/savage_bci.c | 9 +++++++-- drivers/gpu/drm/savage/savage_drv.c | 2 +- 2 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/savage/savage_bci.c b/drivers/gpu/drm/savage/savage_bci.c index 6eb507a..1efbb90 100644 --- a/drivers/gpu/drm/savage/savage_bci.c +++ b/drivers/gpu/drm/savage/savage_bci.c @@ -1050,6 +1050,7 @@ void savage_reclaim_buffers(struct drm_device *dev, struct drm_file *file_priv) { struct drm_device_dma *dma = dev->dma; drm_savage_private_t *dev_priv = dev->dev_private; + int release_idlelock = 0; int i;

if (!dma) @@ -1059,7 +1060,10 @@ void savage_reclaim_buffers(struct drm_device *dev, struct drm_file *file_priv) if (!dma->buflist) return;

- /*i830_flush_queue(dev); */ + if (file_priv->master && file_priv->master->lock.hw_lock) { + drm_idlelock_take(&file_priv->master->lock); + release_idlelock = 1; + }

for (i = 0; i < dma->buf_count; i++) { struct drm_buf *buf = dma->buflist[i]; @@ -1075,7 +1079,8 @@ void savage_reclaim_buffers(struct drm_device *dev, struct drm_file *file_priv) } }

- drm_core_reclaim_buffers(dev, file_priv); + if (release_idlelock) + drm_idlelock_release(&file_priv->master->lock); }

struct drm_ioctl_desc savage_ioctls[] = { diff --git a/drivers/gpu/drm/savage/savage_drv.c b/drivers/gpu/drm/savage/savage_drv.c index 89afe0b..d31d4cc 100644 --- a/drivers/gpu/drm/savage/savage_drv.c +++ b/drivers/gpu/drm/savage/savage_drv.c @@ -52,9 +52,9 @@ static struct drm_driver driver = { .dev_priv_size = sizeof(drm_savage_buf_priv_t), .load = savage_driver_load, .firstopen = savage_driver_firstopen, + .preclose = savage_reclaim_buffers, .lastclose = savage_driver_lastclose, .unload = savage_driver_unload, - .reclaim_buffers = savage_reclaim_buffers, .ioctls = savage_ioctls, .dma_ioctl = savage_bci_buffers, .fops = &savage_driver_fops,

-- 1.7.10

Daniel Vetter

10:25 a.m.

New subject: [PATCH 07/12] drm: kill reclaim_buffers callback

All leftover users either haven't set DRIVER_HAVE_DMA, in which case this will never be called, or use the drm_core implementation.

Call that directly in the only callsite.

Signed-off-by: Daniel Vetter daniel.vetter@ffwll.ch --- drivers/gpu/drm/drm_fops.c | 5 ++--- drivers/gpu/drm/gma500/psb_drv.c | 1 - drivers/gpu/drm/i915/i915_drv.c | 1 - drivers/gpu/drm/mga/mga_drv.c | 1 - drivers/gpu/drm/nouveau/nouveau_drv.c | 1 - drivers/gpu/drm/r128/r128_drv.c | 1 - drivers/gpu/drm/radeon/radeon_drv.c | 2 -- drivers/gpu/drm/tdfx/tdfx_drv.c | 1 - include/drm/drmP.h | 2 -- 9 files changed, 2 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/drm_fops.c b/drivers/gpu/drm/drm_fops.c index 6cf9369..96b8c8f 100644 --- a/drivers/gpu/drm/drm_fops.c +++ b/drivers/gpu/drm/drm_fops.c @@ -381,9 +381,8 @@ static void drm_master_release(struct drm_device *dev, struct file *filp) _DRM_LOCKING_CONTEXT(file_priv->master->lock.hw_lock->lock)); }

- if (drm_core_check_feature(dev, DRIVER_HAVE_DMA)) { - dev->driver->reclaim_buffers(dev, file_priv); - } + if (drm_core_check_feature(dev, DRIVER_HAVE_DMA)) + drm_core_reclaim_buffers(dev, file_priv); }

static void drm_events_release(struct drm_file *file_priv) diff --git a/drivers/gpu/drm/gma500/psb_drv.c b/drivers/gpu/drm/gma500/psb_drv.c index caba6e0..cd1dd1b 100644 --- a/drivers/gpu/drm/gma500/psb_drv.c +++ b/drivers/gpu/drm/gma500/psb_drv.c @@ -632,7 +632,6 @@ static struct drm_driver driver = { .open = psb_driver_open, .preclose = psb_driver_preclose, .postclose = psb_driver_close, - .reclaim_buffers = drm_core_reclaim_buffers,

.gem_init_object = psb_gem_init_object, .gem_free_object = psb_gem_free_object, diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 9fe9ebe..a196eca 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -1058,7 +1058,6 @@ static struct drm_driver driver = { .resume = i915_resume,

.device_is_agp = i915_driver_device_is_agp, - .reclaim_buffers = drm_core_reclaim_buffers, .master_create = i915_master_create, .master_destroy = i915_master_destroy, #if defined(CONFIG_DEBUG_FS) diff --git a/drivers/gpu/drm/mga/mga_drv.c b/drivers/gpu/drm/mga/mga_drv.c index f9a925d..b1bb46d 100644 --- a/drivers/gpu/drm/mga/mga_drv.c +++ b/drivers/gpu/drm/mga/mga_drv.c @@ -75,7 +75,6 @@ static struct drm_driver driver = { .irq_postinstall = mga_driver_irq_postinstall, .irq_uninstall = mga_driver_irq_uninstall, .irq_handler = mga_driver_irq_handler, - .reclaim_buffers = drm_core_reclaim_buffers, .ioctls = mga_ioctls, .dma_ioctl = mga_dma_buffers, .fops = &mga_driver_fops, diff --git a/drivers/gpu/drm/nouveau/nouveau_drv.c b/drivers/gpu/drm/nouveau/nouveau_drv.c index cad254c..b4d1b4a 100644 --- a/drivers/gpu/drm/nouveau/nouveau_drv.c +++ b/drivers/gpu/drm/nouveau/nouveau_drv.c @@ -422,7 +422,6 @@ static struct drm_driver driver = { .get_vblank_counter = drm_vblank_count, .enable_vblank = nouveau_vblank_enable, .disable_vblank = nouveau_vblank_disable, - .reclaim_buffers = drm_core_reclaim_buffers, .ioctls = nouveau_ioctls, .fops = &nouveau_driver_fops,

diff --git a/drivers/gpu/drm/r128/r128_drv.c b/drivers/gpu/drm/r128/r128_drv.c index 88718fa..2666a53 100644 --- a/drivers/gpu/drm/r128/r128_drv.c +++ b/drivers/gpu/drm/r128/r128_drv.c @@ -71,7 +71,6 @@ static struct drm_driver driver = { .irq_postinstall = r128_driver_irq_postinstall, .irq_uninstall = r128_driver_irq_uninstall, .irq_handler = r128_driver_irq_handler, - .reclaim_buffers = drm_core_reclaim_buffers, .ioctls = r128_ioctls, .dma_ioctl = r128_cce_buffers, .fops = &r128_driver_fops, diff --git a/drivers/gpu/drm/radeon/radeon_drv.c b/drivers/gpu/drm/radeon/radeon_drv.c index f0bb2b5..040a8e7 100644 --- a/drivers/gpu/drm/radeon/radeon_drv.c +++ b/drivers/gpu/drm/radeon/radeon_drv.c @@ -260,7 +260,6 @@ static struct drm_driver driver_old = { .irq_postinstall = radeon_driver_irq_postinstall, .irq_uninstall = radeon_driver_irq_uninstall, .irq_handler = radeon_driver_irq_handler, - .reclaim_buffers = drm_core_reclaim_buffers, .ioctls = radeon_ioctls, .dma_ioctl = radeon_cp_buffers, .fops = &radeon_driver_old_fops, @@ -363,7 +362,6 @@ static struct drm_driver kms_driver = { .irq_postinstall = radeon_driver_irq_postinstall_kms, .irq_uninstall = radeon_driver_irq_uninstall_kms, .irq_handler = radeon_driver_irq_handler_kms, - .reclaim_buffers = drm_core_reclaim_buffers, .ioctls = radeon_ioctls_kms, .gem_init_object = radeon_gem_object_init, .gem_free_object = radeon_gem_object_free, diff --git a/drivers/gpu/drm/tdfx/tdfx_drv.c b/drivers/gpu/drm/tdfx/tdfx_drv.c index 1613c78..90f6b13 100644 --- a/drivers/gpu/drm/tdfx/tdfx_drv.c +++ b/drivers/gpu/drm/tdfx/tdfx_drv.c @@ -54,7 +54,6 @@ static const struct file_operations tdfx_driver_fops = {

static struct drm_driver driver = { .driver_features = DRIVER_USE_MTRR, - .reclaim_buffers = drm_core_reclaim_buffers, .fops = &tdfx_driver_fops, .name = DRIVER_NAME, .desc = DRIVER_DESC, diff --git a/include/drm/drmP.h b/include/drm/drmP.h index 8ee41e6..b55c4d4 100644 --- a/include/drm/drmP.h +++ b/include/drm/drmP.h @@ -876,8 +876,6 @@ struct drm_driver { void (*irq_preinstall) (struct drm_device *dev); int (*irq_postinstall) (struct drm_device *dev); void (*irq_uninstall) (struct drm_device *dev); - void (*reclaim_buffers) (struct drm_device *dev, - struct drm_file * file_priv); void (*set_version) (struct drm_device *dev, struct drm_set_version *sv);

-- 1.7.10

Daniel Vetter

10:25 a.m.

New subject: [PATCH 08/12] drm: ditch strange DRIVER_DMA_QUEUE only error bail-out

Only one driver (i810) even sets that flag. Now the actual locking code uncoditionally promotes lock->context to an unsigned int.

Closer inspection of the userspace reveals that the drm lock context is defined as an unsigned int (at least on linux). I suspect we just have a strange case of signedness confusion going on.

Tested on my i815, doesn't seem to break anything.

Signed-off-by: Daniel Vetter daniel.vetter@ffwll.ch --- drivers/gpu/drm/drm_lock.c | 4 ---- 1 file changed, 4 deletions(-)

diff --git a/drivers/gpu/drm/drm_lock.c b/drivers/gpu/drm/drm_lock.c index 5211520..3203955 100644 --- a/drivers/gpu/drm/drm_lock.c +++ b/drivers/gpu/drm/drm_lock.c @@ -70,10 +70,6 @@ int drm_lock(struct drm_device *dev, void *data, struct drm_file *file_priv) lock->context, task_pid_nr(current), master->lock.hw_lock->lock, lock->flags);

- if (drm_core_check_feature(dev, DRIVER_DMA_QUEUE)) - if (lock->context < 0) - return -EINVAL; - add_wait_queue(&master->lock.lock_queue, &entry); spin_lock_bh(&master->lock.spinlock); master->lock.user_waiters++;

-- 1.7.10

Daniel Vetter

10:25 a.m.

New subject: [PATCH 09/12] drm: kill dma queue support

Absolutely unused. All the values are only ever initialized and then used at most in some debug printout functions.

Signed-off-by: Daniel Vetter daniel.vetter@ffwll.ch --- drivers/gpu/drm/drm_bufs.c | 16 ++-------------- drivers/gpu/drm/drm_debugfs.c | 1 - drivers/gpu/drm/drm_dma.c | 5 ----- drivers/gpu/drm/drm_drv.c | 11 ----------- drivers/gpu/drm/drm_fops.c | 4 ---- drivers/gpu/drm/drm_info.c | 36 ------------------------------------ drivers/gpu/drm/drm_proc.c | 1 - drivers/gpu/drm/i810/i810_drv.c | 2 +- include/drm/drmP.h | 8 +------- 9 files changed, 4 insertions(+), 80 deletions(-)

diff --git a/drivers/gpu/drm/drm_bufs.c b/drivers/gpu/drm/drm_bufs.c index 348b367..b356c71 100644 --- a/drivers/gpu/drm/drm_bufs.c +++ b/drivers/gpu/drm/drm_bufs.c @@ -641,8 +641,6 @@ int drm_addbufs_agp(struct drm_device * dev, struct drm_buf_desc * request)

if (order < DRM_MIN_ORDER || order > DRM_MAX_ORDER) return -EINVAL; - if (dev->queue_count) - return -EBUSY; /* Not while in use */

/* Make sure buffers are located in AGP memory that we own */ valid = 0; @@ -704,7 +702,6 @@ int drm_addbufs_agp(struct drm_device * dev, struct drm_buf_desc * request) buf->next = NULL; buf->waiting = 0; buf->pending = 0; - init_waitqueue_head(&buf->dma_wait); buf->file_priv = NULL;

buf->dev_priv_size = dev->driver->dev_priv_size; @@ -796,13 +793,11 @@ int drm_addbufs_pci(struct drm_device * dev, struct drm_buf_desc * request) order = drm_order(request->size); size = 1 << order;

- DRM_DEBUG("count=%d, size=%d (%d), order=%d, queue_count=%d\n", - request->count, request->size, size, order, dev->queue_count); + DRM_DEBUG("count=%d, size=%d (%d), order=%d\n", + request->count, request->size, size, order);

if (order < DRM_MIN_ORDER || order > DRM_MAX_ORDER) return -EINVAL; - if (dev->queue_count) - return -EBUSY; /* Not while in use */

alignment = (request->flags & _DRM_PAGE_ALIGN) ? PAGE_ALIGN(size) : size; @@ -904,7 +899,6 @@ int drm_addbufs_pci(struct drm_device * dev, struct drm_buf_desc * request) buf->next = NULL; buf->waiting = 0; buf->pending = 0; - init_waitqueue_head(&buf->dma_wait); buf->file_priv = NULL;

buf->dev_priv_size = dev->driver->dev_priv_size; @@ -1019,8 +1013,6 @@ static int drm_addbufs_sg(struct drm_device * dev, struct drm_buf_desc * request

if (order < DRM_MIN_ORDER || order > DRM_MAX_ORDER) return -EINVAL; - if (dev->queue_count) - return -EBUSY; /* Not while in use */

spin_lock(&dev->count_lock); if (dev->buf_use) { @@ -1071,7 +1063,6 @@ static int drm_addbufs_sg(struct drm_device * dev, struct drm_buf_desc * request buf->next = NULL; buf->waiting = 0; buf->pending = 0; - init_waitqueue_head(&buf->dma_wait); buf->file_priv = NULL;

buf->dev_priv_size = dev->driver->dev_priv_size; @@ -1177,8 +1168,6 @@ static int drm_addbufs_fb(struct drm_device * dev, struct drm_buf_desc * request

if (order < DRM_MIN_ORDER || order > DRM_MAX_ORDER) return -EINVAL; - if (dev->queue_count) - return -EBUSY; /* Not while in use */

spin_lock(&dev->count_lock); if (dev->buf_use) { @@ -1228,7 +1217,6 @@ static int drm_addbufs_fb(struct drm_device * dev, struct drm_buf_desc * request buf->next = NULL; buf->waiting = 0; buf->pending = 0; - init_waitqueue_head(&buf->dma_wait); buf->file_priv = NULL;

buf->dev_priv_size = dev->driver->dev_priv_size; diff --git a/drivers/gpu/drm/drm_debugfs.c b/drivers/gpu/drm/drm_debugfs.c index 1c7a1c0..70b13fc 100644 --- a/drivers/gpu/drm/drm_debugfs.c +++ b/drivers/gpu/drm/drm_debugfs.c @@ -46,7 +46,6 @@ static struct drm_info_list drm_debugfs_list[] = { {"name", drm_name_info, 0}, {"vm", drm_vm_info, 0}, {"clients", drm_clients_info, 0}, - {"queues", drm_queues_info, 0}, {"bufs", drm_bufs_info, 0}, {"gem_names", drm_gem_name_info, DRIVER_GEM}, #if DRM_DEBUG_CODE diff --git a/drivers/gpu/drm/drm_dma.c b/drivers/gpu/drm/drm_dma.c index cfb4e33..08f5e53 100644 --- a/drivers/gpu/drm/drm_dma.c +++ b/drivers/gpu/drm/drm_dma.c @@ -120,11 +120,6 @@ void drm_free_buffer(struct drm_device *dev, struct drm_buf * buf) buf->pending = 0; buf->file_priv = NULL; buf->used = 0; - - if (drm_core_check_feature(dev, DRIVER_DMA_QUEUE) - && waitqueue_active(&buf->dma_wait)) { - wake_up_interruptible(&buf->dma_wait); - } }

/** diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c index 8a9d079..bbabc05 100644 --- a/drivers/gpu/drm/drm_drv.c +++ b/drivers/gpu/drm/drm_drv.c @@ -182,7 +182,6 @@ static struct drm_ioctl_desc drm_ioctls[] = { int drm_lastclose(struct drm_device * dev) { struct drm_vma_entry *vma, *vma_temp; - int i;

DRM_DEBUG("\n");

@@ -228,16 +227,6 @@ int drm_lastclose(struct drm_device * dev) kfree(vma); }

- if (drm_core_check_feature(dev, DRIVER_DMA_QUEUE) && dev->queuelist) { - for (i = 0; i < dev->queue_count; i++) { - kfree(dev->queuelist[i]); - dev->queuelist[i] = NULL; - } - kfree(dev->queuelist); - dev->queuelist = NULL; - } - dev->queue_count = 0; - if (drm_core_check_feature(dev, DRIVER_HAVE_DMA) && !drm_core_check_feature(dev, DRIVER_MODESET)) drm_dma_takedown(dev); diff --git a/drivers/gpu/drm/drm_fops.c b/drivers/gpu/drm/drm_fops.c index 96b8c8f..c6f5f89 100644 --- a/drivers/gpu/drm/drm_fops.c +++ b/drivers/gpu/drm/drm_fops.c @@ -75,10 +75,6 @@ static int drm_setup(struct drm_device * dev)

dev->sigdata.lock = NULL;

- dev->queue_count = 0; - dev->queue_reserved = 0; - dev->queue_slots = 0; - dev->queuelist = NULL; dev->context_flag = 0; dev->interrupt_flag = 0; dev->dma_flag = 0; diff --git a/drivers/gpu/drm/drm_info.c b/drivers/gpu/drm/drm_info.c index ab1162d..e46ef04 100644 --- a/drivers/gpu/drm/drm_info.c +++ b/drivers/gpu/drm/drm_info.c @@ -110,42 +110,6 @@ int drm_vm_info(struct seq_file *m, void *data) }

/** - * Called when "/proc/dri/.../queues" is read. - */ -int drm_queues_info(struct seq_file *m, void *data) -{ - struct drm_info_node *node = (struct drm_info_node *) m->private; - struct drm_device *dev = node->minor->dev; - int i; - struct drm_queue *q; - - mutex_lock(&dev->struct_mutex); - seq_printf(m, " ctx/flags use fin" - " blk/rw/rwf wait flushed queued" - " locks\n\n"); - for (i = 0; i < dev->queue_count; i++) { - q = dev->queuelist[i]; - atomic_inc(&q->use_count); - seq_printf(m, "%5d/0x%03x %5d %5d" - " %5d/%c%c/%c%c%c %5Zd\n", - i, - q->flags, - atomic_read(&q->use_count), - atomic_read(&q->finalization), - atomic_read(&q->block_count), - atomic_read(&q->block_read) ? 'r' : '-', - atomic_read(&q->block_write) ? 'w' : '-', - waitqueue_active(&q->read_queue) ? 'r' : '-', - waitqueue_active(&q->write_queue) ? 'w' : '-', - waitqueue_active(&q->flush_queue) ? 'f' : '-', - DRM_BUFCOUNT(&q->waitlist)); - atomic_dec(&q->use_count); - } - mutex_unlock(&dev->struct_mutex); - return 0; -} - -/** * Called when "/proc/dri/.../bufs" is read. */ int drm_bufs_info(struct seq_file *m, void *data) diff --git a/drivers/gpu/drm/drm_proc.c b/drivers/gpu/drm/drm_proc.c index fff8722..371c695 100644 --- a/drivers/gpu/drm/drm_proc.c +++ b/drivers/gpu/drm/drm_proc.c @@ -53,7 +53,6 @@ static struct drm_info_list drm_proc_list[] = { {"name", drm_name_info, 0}, {"vm", drm_vm_info, 0}, {"clients", drm_clients_info, 0}, - {"queues", drm_queues_info, 0}, {"bufs", drm_bufs_info, 0}, {"gem_names", drm_gem_name_info, DRIVER_GEM}, #if DRM_DEBUG_CODE diff --git a/drivers/gpu/drm/i810/i810_drv.c b/drivers/gpu/drm/i810/i810_drv.c index 053f1ee..f9924ad 100644 --- a/drivers/gpu/drm/i810/i810_drv.c +++ b/drivers/gpu/drm/i810/i810_drv.c @@ -57,7 +57,7 @@ static const struct file_operations i810_driver_fops = { static struct drm_driver driver = { .driver_features = DRIVER_USE_AGP | DRIVER_REQUIRE_AGP | DRIVER_USE_MTRR | - DRIVER_HAVE_DMA | DRIVER_DMA_QUEUE, + DRIVER_HAVE_DMA, .dev_priv_size = sizeof(drm_i810_buf_priv_t), .load = i810_driver_load, .lastclose = i810_driver_lastclose, diff --git a/include/drm/drmP.h b/include/drm/drmP.h index b55c4d4..df21016 100644 --- a/include/drm/drmP.h +++ b/include/drm/drmP.h @@ -348,7 +348,6 @@ struct drm_buf { struct drm_buf *next; /**< Kernel-only: used for free list */ __volatile__ int waiting; /**< On kernel DMA queue */ __volatile__ int pending; /**< On hardware DMA queue */ - wait_queue_head_t dma_wait; /**< Processes waiting */ struct drm_file *file_priv; /**< Private of holding file descr */ int context; /**< Kernel queue for this buffer */ int while_locked; /**< Dispatch this buffer while locked */ @@ -1102,12 +1101,8 @@ struct drm_device {

/*@} */

- /** \name DMA queues (contexts) */ + /** \name DMA support */ /*@{ */ - int queue_count; /**< Number of active DMA queues */ - int queue_reserved; /**< Number of reserved DMA queues */ - int queue_slots; /**< Actual length of queuelist */ - struct drm_queue **queuelist; /**< Vector of pointers to DMA queues */ struct drm_device_dma *dma; /**< Optional pointer for DMA support */ /*@} */

@@ -1534,7 +1529,6 @@ extern int drm_debugfs_cleanup(struct drm_minor *minor); /* Info file support */ extern int drm_name_info(struct seq_file *m, void *data); extern int drm_vm_info(struct seq_file *m, void *data); -extern int drm_queues_info(struct seq_file *m, void *data); extern int drm_bufs_info(struct seq_file *m, void *data); extern int drm_vblank_info(struct seq_file *m, void *data); extern int drm_clients_info(struct seq_file *m, void* data);

-- 1.7.10

Daniel Vetter

10:25 a.m.

New subject: [PATCH 10/12] drm: unconditionally clean up dma buffers of closing clients

With the last patch to ditch DMA_QUEUE support, we should be able to call the dma cleanup uncoditionally, even when the master has disappeared.

Do so because it just makes more sense.

Signed-off-by: Daniel Vetter daniel.vetter@ffwll.ch --- drivers/gpu/drm/drm_fops.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/drm_fops.c b/drivers/gpu/drm/drm_fops.c index c6f5f89..d25a617 100644 --- a/drivers/gpu/drm/drm_fops.c +++ b/drivers/gpu/drm/drm_fops.c @@ -376,9 +376,6 @@ static void drm_master_release(struct drm_device *dev, struct file *filp) drm_lock_free(&file_priv->master->lock, _DRM_LOCKING_CONTEXT(file_priv->master->lock.hw_lock->lock)); } - - if (drm_core_check_feature(dev, DRIVER_HAVE_DMA)) - drm_core_reclaim_buffers(dev, file_priv); }

static void drm_events_release(struct drm_file *file_priv) @@ -448,6 +445,9 @@ int drm_release(struct inode *inode, struct file *filp) if (file_priv->minor->master) drm_master_release(dev, filp);

+ if (drm_core_check_feature(dev, DRIVER_HAVE_DMA)) + drm_core_reclaim_buffers(dev, file_priv); + drm_events_release(file_priv);

if (dev->driver->driver_features & DRIVER_MODESET)

-- 1.7.10

Daniel Vetter

10:25 a.m.

New subject: [PATCH 11/12] drm: kill i915/i830 ids from drm_pciids.h

Signed-off-by: Daniel Vetter daniel.vetter@ffwll.ch --- include/drm/drm_pciids.h | 42 ------------------------------------------ 1 file changed, 42 deletions(-)

diff --git a/include/drm/drm_pciids.h b/include/drm/drm_pciids.h index 81368ab..20c46ec 100644 --- a/include/drm/drm_pciids.h +++ b/include/drm/drm_pciids.h @@ -690,14 +690,6 @@ {0x8086, 0x1132, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0}, \ {0, 0, 0}

-#define i830_PCI_IDS \ - {0x8086, 0x3577, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0}, \ - {0x8086, 0x2562, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0}, \ - {0x8086, 0x3582, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0}, \ - {0x8086, 0x2572, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0}, \ - {0x8086, 0x358e, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0}, \ - {0, 0, 0} - #define gamma_PCI_IDS \ {0x3d3d, 0x0008, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0}, \ {0, 0, 0} @@ -730,37 +722,3 @@

#define ffb_PCI_IDS \ {0, 0, 0} - -#define i915_PCI_IDS \ - {0x8086, 0x3577, PCI_ANY_ID, PCI_ANY_ID, PCI_CLASS_DISPLAY_VGA << 8, 0xffff00, 0}, \ - {0x8086, 0x2562, PCI_ANY_ID, PCI_ANY_ID, PCI_CLASS_DISPLAY_VGA << 8, 0xffff00, 0}, \ - {0x8086, 0x3582, PCI_ANY_ID, PCI_ANY_ID, PCI_CLASS_DISPLAY_VGA << 8, 0xffff00, 0}, \ - {0x8086, 0x2572, PCI_ANY_ID, PCI_ANY_ID, PCI_CLASS_DISPLAY_VGA << 8, 0xffff00, 0}, \ - {0x8086, 0x2582, PCI_ANY_ID, PCI_ANY_ID, PCI_CLASS_DISPLAY_VGA << 8, 0xffff00, 0}, \ - {0x8086, 0x258a, PCI_ANY_ID, PCI_ANY_ID, PCI_CLASS_DISPLAY_VGA << 8, 0xffff00, 0}, \ - {0x8086, 0x2592, PCI_ANY_ID, PCI_ANY_ID, PCI_CLASS_DISPLAY_VGA << 8, 0xffff00, 0}, \ - {0x8086, 0x2772, PCI_ANY_ID, PCI_ANY_ID, PCI_CLASS_DISPLAY_VGA << 8, 0xffff00, 0}, \ - {0x8086, 0x27a2, PCI_ANY_ID, PCI_ANY_ID, PCI_CLASS_DISPLAY_VGA << 8, 0xffff00, 0}, \ - {0x8086, 0x27ae, PCI_ANY_ID, PCI_ANY_ID, PCI_CLASS_DISPLAY_VGA << 8, 0xffff00, 0}, \ - {0x8086, 0x2972, PCI_ANY_ID, PCI_ANY_ID, PCI_CLASS_DISPLAY_VGA << 8, 0xffff00, 0}, \ - {0x8086, 0x2982, PCI_ANY_ID, PCI_ANY_ID, PCI_CLASS_DISPLAY_VGA << 8, 0xffff00, 0}, \ - {0x8086, 0x2992, PCI_ANY_ID, PCI_ANY_ID, PCI_CLASS_DISPLAY_VGA << 8, 0xffff00, 0}, \ - {0x8086, 0x29a2, PCI_ANY_ID, PCI_ANY_ID, PCI_CLASS_DISPLAY_VGA << 8, 0xffff00, 0}, \ - {0x8086, 0x29b2, PCI_ANY_ID, PCI_ANY_ID, PCI_CLASS_DISPLAY_VGA << 8, 0xffff00, 0}, \ - {0x8086, 0x29c2, PCI_ANY_ID, PCI_ANY_ID, PCI_CLASS_DISPLAY_VGA << 8, 0xffff00, 0}, \ - {0x8086, 0x29d2, PCI_ANY_ID, PCI_ANY_ID, PCI_CLASS_DISPLAY_VGA << 8, 0xffff00, 0}, \ - {0x8086, 0x2a02, PCI_ANY_ID, PCI_ANY_ID, PCI_CLASS_DISPLAY_VGA << 8, 0xffff00, 0}, \ - {0x8086, 0x2a12, PCI_ANY_ID, PCI_ANY_ID, PCI_CLASS_DISPLAY_VGA << 8, 0xffff00, 0}, \ - {0x8086, 0x2a42, PCI_ANY_ID, PCI_ANY_ID, PCI_CLASS_DISPLAY_VGA << 8, 0xffff00, 0}, \ - {0x8086, 0x2e02, PCI_ANY_ID, PCI_ANY_ID, PCI_CLASS_DISPLAY_VGA << 8, 0xffff00, 0}, \ - {0x8086, 0x2e12, PCI_ANY_ID, PCI_ANY_ID, PCI_CLASS_DISPLAY_VGA << 8, 0xffff00, 0}, \ - {0x8086, 0x2e22, PCI_ANY_ID, PCI_ANY_ID, PCI_CLASS_DISPLAY_VGA << 8, 0xffff00, 0}, \ - {0x8086, 0x2e32, PCI_ANY_ID, PCI_ANY_ID, PCI_CLASS_DISPLAY_VGA << 8, 0xffff00, 0}, \ - {0x8086, 0x2e42, PCI_ANY_ID, PCI_ANY_ID, PCI_CLASS_DISPLAY_VGA << 8, 0xffff00, 0}, \ - {0x8086, 0xa001, PCI_ANY_ID, PCI_ANY_ID, PCI_CLASS_DISPLAY_VGA << 8, 0xffff00, 0}, \ - {0x8086, 0xa011, PCI_ANY_ID, PCI_ANY_ID, PCI_CLASS_DISPLAY_VGA << 8, 0xffff00, 0}, \ - {0x8086, 0x35e8, PCI_ANY_ID, PCI_ANY_ID, PCI_CLASS_DISPLAY_VGA << 8, 0xffff00, 0}, \ - {0x8086, 0x0042, PCI_ANY_ID, PCI_ANY_ID, PCI_CLASS_DISPLAY_VGA << 8, 0xffff00, 0}, \ - {0x8086, 0x0046, PCI_ANY_ID, PCI_ANY_ID, PCI_CLASS_DISPLAY_VGA << 8, 0xffff00, 0}, \ - {0x8086, 0x0102, PCI_ANY_ID, PCI_ANY_ID, PCI_CLASS_DISPLAY_VGA << 8, 0xffff00, 0}, \ - {0, 0, 0}

-- 1.7.10

Daniel Vetter

10:25 a.m.

New subject: [PATCH 12/12] drm: kill procfs support

We have debugfs and sysfs for these things now!

Digging through the dungeons of old code and wading through countless result pages on google indeed turned up one user of this:

libdrm before 2001 checks via the presence of /proc/dri/0 whether the kernel drm is present and has a module successfully loaded.

Frankly I think that demanding from users that they upgrade to a more modern libdrm is in order.

Signed-off-by: Daniel Vetter daniel.vetter@ffwll.ch --- drivers/gpu/drm/Makefile | 2 +- drivers/gpu/drm/drm_drv.c | 8 -- drivers/gpu/drm/drm_proc.c | 220 -------------------------------------------- drivers/gpu/drm/drm_stub.c | 25 ++--- include/drm/drmP.h | 9 -- 5 files changed, 8 insertions(+), 256 deletions(-) delete mode 100644 drivers/gpu/drm/drm_proc.c

diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile index f65f65e..36b20b1 100644 --- a/drivers/gpu/drm/Makefile +++ b/drivers/gpu/drm/Makefile @@ -7,7 +7,7 @@ ccflags-y := -Iinclude/drm drm-y := drm_auth.o drm_buffer.o drm_bufs.o drm_cache.o \ drm_context.o drm_dma.o \ drm_drv.o drm_fops.o drm_gem.o drm_ioctl.o drm_irq.o \ - drm_lock.o drm_memory.o drm_proc.o drm_stub.o drm_vm.o \ + drm_lock.o drm_memory.o drm_stub.o drm_vm.o \ drm_agpsupport.o drm_scatter.o ati_pcigart.o drm_pci.o \ drm_platform.o drm_sysfs.o drm_hashtab.o drm_mm.o \ drm_crtc.o drm_modes.o drm_edid.o \ diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c index bbabc05..0bb4b84 100644 --- a/drivers/gpu/drm/drm_drv.c +++ b/drivers/gpu/drm/drm_drv.c @@ -262,13 +262,6 @@ static int __init drm_core_init(void) goto err_p2; }

- drm_proc_root = proc_mkdir("dri", NULL); - if (!drm_proc_root) { - DRM_ERROR("Cannot create /proc/dri\n"); - ret = -1; - goto err_p3; - } - drm_debugfs_root = debugfs_create_dir("dri", NULL); if (!drm_debugfs_root) { DRM_ERROR("Cannot create /sys/kernel/debug/dri\n"); @@ -291,7 +284,6 @@ err_p1:

static void __exit drm_core_exit(void) { - remove_proc_entry("dri", NULL); debugfs_remove(drm_debugfs_root); drm_sysfs_destroy();

diff --git a/drivers/gpu/drm/drm_proc.c b/drivers/gpu/drm/drm_proc.c deleted file mode 100644 index 371c695..0000000 --- a/drivers/gpu/drm/drm_proc.c +++ /dev/null @@ -1,220 +0,0 @@ -/** - * \file drm_proc.c - * /proc support for DRM - * - * \author Rickard E. (Rik) Faith faith@valinux.com - * \author Gareth Hughes gareth@valinux.com - * - * \par Acknowledgements: - * Matthew J Sottek matthew.j.sottek@intel.com sent in a patch to fix - * the problem with the proc files not outputting all their information. - */ - -/* - * Created: Mon Jan 11 09:48:47 1999 by faith@valinux.com - * - * Copyright 1999 Precision Insight, Inc., Cedar Park, Texas. - * Copyright 2000 VA Linux Systems, Inc., Sunnyvale, California. - * All Rights Reserved. - * - * Permission is hereby granted, free of charge, to any person obtaining a - * copy of this software and associated documentation files (the "Software"), - * to deal in the Software without restriction, including without limitation - * the rights to use, copy, modify, merge, publish, distribute, sublicense, - * and/or sell copies of the Software, and to permit persons to whom the - * Software is furnished to do so, subject to the following conditions: - * - * The above copyright notice and this permission notice (including the next - * paragraph) shall be included in all copies or substantial portions of the - * Software. - * - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, - * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL - * VA LINUX SYSTEMS AND/OR ITS SUPPLIERS BE LIABLE FOR ANY CLAIM, DAMAGES OR - * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, - * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR - * OTHER DEALINGS IN THE SOFTWARE. - */ - -#include <linux/seq_file.h> -#include <linux/slab.h> -#include <linux/export.h> -#include "drmP.h" - -/*************************************************** - * Initialization, etc. - **************************************************/ - -/** - * Proc file list. - */ -static struct drm_info_list drm_proc_list[] = { - {"name", drm_name_info, 0}, - {"vm", drm_vm_info, 0}, - {"clients", drm_clients_info, 0}, - {"bufs", drm_bufs_info, 0}, - {"gem_names", drm_gem_name_info, DRIVER_GEM}, -#if DRM_DEBUG_CODE - {"vma", drm_vma_info, 0}, -#endif -}; -#define DRM_PROC_ENTRIES ARRAY_SIZE(drm_proc_list) - -static int drm_proc_open(struct inode *inode, struct file *file) -{ - struct drm_info_node* node = PDE(inode)->data; - - return single_open(file, node->info_ent->show, node); -} - -static const struct file_operations drm_proc_fops = { - .owner = THIS_MODULE, - .open = drm_proc_open, - .read = seq_read, - .llseek = seq_lseek, - .release = single_release, -}; - - -/** - * Initialize a given set of proc files for a device - * - * \param files The array of files to create - * \param count The number of files given - * \param root DRI proc dir entry. - * \param minor device minor number - * \return Zero on success, non-zero on failure - * - * Create a given set of proc files represented by an array of - * gdm_proc_lists in the given root directory. - */ -int drm_proc_create_files(struct drm_info_list *files, int count, - struct proc_dir_entry *root, struct drm_minor *minor) -{ - struct drm_device *dev = minor->dev; - struct proc_dir_entry *ent; - struct drm_info_node *tmp; - int i, ret; - - for (i = 0; i < count; i++) { - u32 features = files[i].driver_features; - - if (features != 0 && - (dev->driver->driver_features & features) != features) - continue; - - tmp = kmalloc(sizeof(struct drm_info_node), GFP_KERNEL); - if (tmp == NULL) { - ret = -1; - goto fail; - } - tmp->minor = minor; - tmp->info_ent = &files[i]; - list_add(&tmp->list, &minor->proc_nodes.list); - - ent = proc_create_data(files[i].name, S_IRUGO, root, - &drm_proc_fops, tmp); - if (!ent) { - DRM_ERROR("Cannot create /proc/dri/%s/%s\n", - root->name, files[i].name); - list_del(&tmp->list); - kfree(tmp); - ret = -1; - goto fail; - } - - } - return 0; - -fail: - for (i = 0; i < count; i++) - remove_proc_entry(drm_proc_list[i].name, minor->proc_root); - return ret; -} - -/** - * Initialize the DRI proc filesystem for a device - * - * \param dev DRM device - * \param minor device minor number - * \param root DRI proc dir entry. - * \param dev_root resulting DRI device proc dir entry. - * \return root entry pointer on success, or NULL on failure. - * - * Create the DRI proc root entry "/proc/dri", the device proc root entry - * "/proc/dri/%minor%/", and each entry in proc_list as - * "/proc/dri/%minor%/%name%". - */ -int drm_proc_init(struct drm_minor *minor, int minor_id, - struct proc_dir_entry *root) -{ - char name[64]; - int ret; - - INIT_LIST_HEAD(&minor->proc_nodes.list); - sprintf(name, "%d", minor_id); - minor->proc_root = proc_mkdir(name, root); - if (!minor->proc_root) { - DRM_ERROR("Cannot create /proc/dri/%s\n", name); - return -1; - } - - ret = drm_proc_create_files(drm_proc_list, DRM_PROC_ENTRIES, - minor->proc_root, minor); - if (ret) { - remove_proc_entry(name, root); - minor->proc_root = NULL; - DRM_ERROR("Failed to create core drm proc files\n"); - return ret; - } - - return 0; -} - -int drm_proc_remove_files(struct drm_info_list *files, int count, - struct drm_minor *minor) -{ - struct list_head *pos, *q; - struct drm_info_node *tmp; - int i; - - for (i = 0; i < count; i++) { - list_for_each_safe(pos, q, &minor->proc_nodes.list) { - tmp = list_entry(pos, struct drm_info_node, list); - if (tmp->info_ent == &files[i]) { - remove_proc_entry(files[i].name, - minor->proc_root); - list_del(pos); - kfree(tmp); - } - } - } - return 0; -} - -/** - * Cleanup the proc filesystem resources. - * - * \param minor device minor number. - * \param root DRI proc dir entry. - * \param dev_root DRI device proc dir entry. - * \return always zero. - * - * Remove all proc entries created by proc_init(). - */ -int drm_proc_cleanup(struct drm_minor *minor, struct proc_dir_entry *root) -{ - char name[64]; - - if (!root || !minor->proc_root) - return 0; - - drm_proc_remove_files(drm_proc_list, DRM_PROC_ENTRIES, minor); - - sprintf(name, "%d", minor->index); - remove_proc_entry(name, root); - - return 0; -} - diff --git a/drivers/gpu/drm/drm_stub.c b/drivers/gpu/drm/drm_stub.c index 21bcd4a..d8e6927 100644 --- a/drivers/gpu/drm/drm_stub.c +++ b/drivers/gpu/drm/drm_stub.c @@ -60,7 +60,6 @@ module_param_named(timestamp_precision_usec, drm_timestamp_precision, int, 0600) struct idr drm_minors_idr;

struct class *drm_class; -struct proc_dir_entry *drm_proc_root; struct dentry *drm_debugfs_root;

int drm_err(const char *func, const char *format, ...) @@ -329,7 +328,7 @@ EXPORT_SYMBOL(drm_fill_in_dev); * \return negative number on failure. * * Search an empty entry and initialize it to the given parameters, and - * create the proc init entry via proc_init(). This routines assigns + * create the sysfs/debugfs init entry. This routines assigns * minor numbers to secondary heads of multi-headed cards */ int drm_get_minor(struct drm_device *dev, struct drm_minor **minor, int type) @@ -358,20 +357,11 @@ int drm_get_minor(struct drm_device *dev, struct drm_minor **minor, int type)

idr_replace(&drm_minors_idr, new_minor, minor_id);

- if (type == DRM_MINOR_LEGACY) { - ret = drm_proc_init(new_minor, minor_id, drm_proc_root); - if (ret) { - DRM_ERROR("DRM: Failed to initialize /proc/dri.\n"); - goto err_mem; - } - } else - new_minor->proc_root = NULL; - #if defined(CONFIG_DEBUG_FS) ret = drm_debugfs_init(new_minor, minor_id, drm_debugfs_root); if (ret) { DRM_ERROR("DRM: Failed to initialize /sys/kernel/debug/dri.\n"); - goto err_g2; + goto err_mem; } #endif

@@ -379,7 +369,7 @@ int drm_get_minor(struct drm_device *dev, struct drm_minor **minor, int type) if (ret) { printk(KERN_ERR "DRM: Error sysfs_device_add.\n"); - goto err_g2; + goto err_debugfs; } *minor = new_minor;

@@ -387,9 +377,10 @@ int drm_get_minor(struct drm_device *dev, struct drm_minor **minor, int type) return 0;

-err_g2: - if (new_minor->type == DRM_MINOR_LEGACY) - drm_proc_cleanup(new_minor, drm_proc_root); +err_debugfs: +#if defined(CONFIG_DEBUG_FS) + drm_debugfs_cleanup(new_minor); +#endif err_mem: kfree(new_minor); err_idr: @@ -415,8 +406,6 @@ int drm_put_minor(struct drm_minor **minor_p)

DRM_DEBUG("release secondary minor %d\n", minor->index);

- if (minor->type == DRM_MINOR_LEGACY) - drm_proc_cleanup(minor, drm_proc_root); #if defined(CONFIG_DEBUG_FS) drm_debugfs_cleanup(minor); #endif diff --git a/include/drm/drmP.h b/include/drm/drmP.h index df21016..f217ecc 100644 --- a/include/drm/drmP.h +++ b/include/drm/drmP.h @@ -45,7 +45,6 @@ #include <linux/kernel.h> #include <linux/miscdevice.h> #include <linux/fs.h> -#include <linux/proc_fs.h> #include <linux/init.h> #include <linux/file.h> #include <linux/platform_device.h> @@ -1016,8 +1015,6 @@ struct drm_minor { struct device kdev; /**< Linux device */ struct drm_device *dev;

- struct proc_dir_entry *proc_root; /**< proc directory entry */ - struct drm_info_node proc_nodes; struct dentry *debugfs_root;

struct list_head debugfs_list; @@ -1503,18 +1500,12 @@ extern unsigned int drm_vblank_offdelay; extern unsigned int drm_timestamp_precision;

extern struct class *drm_class; -extern struct proc_dir_entry *drm_proc_root; extern struct dentry *drm_debugfs_root;

extern struct idr drm_minors_idr;

extern struct drm_local_map *drm_getsarea(struct drm_device *dev);

- /* Proc support (drm_proc.h) */ -extern int drm_proc_init(struct drm_minor *minor, int minor_id, - struct proc_dir_entry *root); -extern int drm_proc_cleanup(struct drm_minor *minor, struct proc_dir_entry *root); - /* Debugfs support */ #if defined(CONFIG_DEBUG_FS) extern int drm_debugfs_init(struct drm_minor *minor, int minor_id,

-- 1.7.10

Daniel Vetter

18 Jul 18 Jul

3:58 p.m.

On Thu, Jun 14, 2012 at 12:24:55PM +0200, Daniel Vetter wrote:

...

Hi all,

Blind me has finally found the bug that killed via - dropping locks again in the early exit path is highly advised ;-)

The big part of this series is cleaning up buffer handling in the teardown/lastclose path: All special-cases only used by 1-2 drivers are moved into the drivers respective preclose functions (newly created in most cases), the remaining stuff is streamlined. The only things left in the common code which is not used by all drivers is used by at least a few of them.

So after this patch series the drm_release/lastclose stuff is now fairly readable and understandable. Originally I wanted to fix up a few of the resource leaks that are possible at module unload time, but got stuck at understanding the release/lastclose stuff ;-)

4 drivers needed special treatement, unfortunately I couldn't get a tested by for sis. I've tried to ebay one of these old cards and found one that works with the sisfb and sis ddx driver, but unfortunately the sis mesa driver only supports one specific generations. But since drm/sis is more or less a 1:1 copy of drm/via I'm fairly sure that this will work, too.

The last two patches just kill stuff that I've noticed while crawling around in the close/release paths.

As usual, comments, flames, reviews (and especially from a sis users) tested-bys highly welcome.

Yours, Daniel

Reprod for inclusion into 3.6. Has been tested on all affected drivers by some random people (and me), and imo really nicely shoves some horrible stuff into driver-private close callbacks. Heck, I like this so much that I've ebayed myself through a few sis cards until I've found one that actually worked. And then fixed the sis driver for 64bit ;-)

Note that the procfs patch is bogus, after some prodding by you on irc we've dug up a few scripts that rely on this. And some strange tools in libdrm.git to read out the procfs debug files and cat them together. So don't include that one - it's smells like a mild form of abi breakage.

Cheers, Daniel

...

Daniel Vetter (12): drm/via: clean up reclaim_buffers drm/sis: clean up reclaim_buffers drm: kill reclaim_buffers_idlelocked functions Revert "Revert "drm/i810: cleanup reclaim_buffers"" drm: kill reclaim_buffers_locked drm/savage: clean up reclaim_buffers drm: kill reclaim_buffers callback drm: ditch strange DRIVER_DMA_QUEUE only error bail-out drm: kill dma queue support drm: unconditionally clean up dma buffers of closing clients drm: kill i915/i830 ids from drm_pciids.h drm: kill procfs support

drivers/gpu/drm/Makefile | 2 +- drivers/gpu/drm/drm_bufs.c | 16 +-- drivers/gpu/drm/drm_debugfs.c | 1 - drivers/gpu/drm/drm_dma.c | 5 - drivers/gpu/drm/drm_drv.c | 19 --- drivers/gpu/drm/drm_fops.c | 63 +--------- drivers/gpu/drm/drm_info.c | 36 ------ drivers/gpu/drm/drm_lock.c | 4 - drivers/gpu/drm/drm_proc.c | 221 --------------------------------- drivers/gpu/drm/drm_stub.c | 25 ++-- drivers/gpu/drm/gma500/psb_drv.c | 1 - drivers/gpu/drm/i810/i810_dma.c | 17 ++- drivers/gpu/drm/i810/i810_drv.c | 3 +- drivers/gpu/drm/i810/i810_drv.h | 6 +- drivers/gpu/drm/i915/i915_drv.c | 1 - drivers/gpu/drm/mga/mga_drv.c | 1 - drivers/gpu/drm/nouveau/nouveau_drv.c | 1 - drivers/gpu/drm/r128/r128_drv.c | 1 - drivers/gpu/drm/radeon/radeon_drv.c | 2 - drivers/gpu/drm/savage/savage_bci.c | 9 +- drivers/gpu/drm/savage/savage_drv.c | 2 +- drivers/gpu/drm/sis/sis_drv.c | 3 +- drivers/gpu/drm/sis/sis_mm.c | 13 +- drivers/gpu/drm/tdfx/tdfx_drv.c | 1 - drivers/gpu/drm/via/via_drv.c | 4 +- drivers/gpu/drm/via/via_mm.c | 13 +- drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 1 - include/drm/drmP.h | 23 +--- include/drm/drm_pciids.h | 42 ------- 29 files changed, 60 insertions(+), 476 deletions(-) delete mode 100644 drivers/gpu/drm/drm_proc.c

-- 1.7.10

-- Daniel Vetter Mail: daniel@ffwll.ch Mobile: +41 (0)79 365 57 48

4686

Age (days ago)

4720

Last active (days ago)

dri-devel@lists.freedesktop.org

14 comments

2 participants

tags (0)

participants (2)

Daniel Vetter
Daniel Vetter