Hello Thomas,
Thanks for your positive feedback.
On Sun, Aug 14, 2005 at 09:38:45AM +0200, Thomas Hellstr?m wrote:
> >01_via_unichrome_pro
> > Trivial patch to make the VIA driver recognize my PCI id as
> > a Unichrome Pro chipset. This makes IRQ handling work properly
> > and avoids the kernel message
> > irq 201: nobody cared (try booting with the "irqpoll" option)
> I have the same problem on my K8M800/K8N800 computers, but when a stray
> interrupt occurs, nothing is seen in the Unichrome IRQ status register,
> and returning IRQ_HANDLED will make the unichrome issue about 300000
> interrupts per second. I've asked VIA about this and their first
> response is that the IRQ functionality was never verified on chipsets
> not having video capture functionality.
I was not aware of this, and unfortunately you seem to be right. I am
getting 300000 interrupts per second with this patch. Completely missed
that, although I did have the impression that my system was a bit
slow overall. Some of these interrupts are genuine VBLANKs, but I
can't figure out how to disable the spurious interrupts.
> Patch 3: I agree that there is checking needed for the returned index
> values, but can't this be solved using range checking on the kernel pointers,
> or perhaps store the pointers in a hash table, the content of which is
> verified before a memblock is freed?
> It seems very inefficient to loop through the block map (up to 5000
> blocks) on each allocation.
My solution is not less efficient than the old situation. The old mm code
can do fast mem_alloc() but will loop through all allocated blocks on
mem_free() (twice). My current solution will loop on mem_alloc() but has
a faster mem_free().
If you are concerned, I can use hashing to speed up mem_alloc(). I did
this originally but then thought it not worthwhile. A possible approach
is attached as 03a_via_mm_cleanup. This will of course turn slow again
when the number of allocated blocks passes a certain treshold.
> BTW How does the current solution interfere with x86-64?
> Here it works nicely except the kernel crashes when it tries to free
> unfreed memory blocks in final_context, but I haven't been able to track
> down the cause yet.
The old solution passes a kernel pointer to userspace as a unsigned long.
This should work in i386 as well as pure x86_64, but not in x86_64 with
32-bit compatibility. To be honest, I did not even test it; it simply
cannot work.
> Patch 4: An oversight in the initial code. It seems correct except for
> the DMA_INIT ioctl where the privilege checking is done within the IOCTL
> code.
Right. I missed the check in the ioctl.
Bye,
Joris.
diff -urN -U 5 drm.prev/shared-core/via_mm.c drm/shared-core/via_mm.c
--- drm.prev/shared-core/via_mm.c 2005-08-14 11:10:23.000000000 +0200
+++ drm/shared-core/via_mm.c 2005-08-14 11:19:24.000000000 +0200
@@ -23,52 +23,131 @@
*/
#include "drmP.h"
#include "via_drm.h"
#include "via_drv.h"
#include "via_ds.h"
-#include "via_mm.h"
#define MAX_CONTEXT 100
+#define MAX_MEMBLOCK_INDEX 5003
+
+/* Structure which maps indices to PMemBlock pointers.
+ index_base is used to make indices globally unique among
+ multiple mem_block_map_t structures, and to avoid ever using
+ the special value 0 as an index. */
+typedef struct {
+ unsigned int index_base;
+ PMemBlock m[MAX_MEMBLOCK_INDEX];
+} mem_block_map_t;
typedef struct {
int used;
int context;
- set_t *sets[2]; /* 0 for frame buffer, 1 for AGP , 2 for System
*/
+ mem_block_map_t *map[2]; /* 0 for frame buffer, 1 for AGP */
} via_context_t;
static via_context_t global_ppriv[MAX_CONTEXT];
static int via_agp_alloc(drm_via_mem_t * mem);
static int via_agp_free(drm_via_mem_t * mem);
static int via_fb_alloc(drm_via_mem_t * mem);
static int via_fb_free(drm_via_mem_t * mem);
-static int add_alloc_set(int context, int type, unsigned int val)
+/*
+ * Allocate a mem_block_map_t and initialize it to make all indices
+ * point to NULL. index_base must be non-zero
+ */
+static mem_block_map_t * via_mem_block_map_init(unsigned int index_base)
{
- int i, retval = 0;
+ mem_block_map_t *map;
+ int i;
- for (i = 0; i < MAX_CONTEXT; i++) {
- if (global_ppriv[i].used && global_ppriv[i].context == context)
{
- retval = via_setAdd(global_ppriv[i].sets[type], val);
- break;
- }
+ map = drm_alloc(sizeof(mem_block_map_t), DRM_MEM_DRIVER);
+ if (map) {
+ map->index_base = index_base;
+ for (i = 0; i < MAX_MEMBLOCK_INDEX; i++)
+ map->m[i] = NULL;
}
-
- return retval;
+ return map;
}
-static int del_alloc_set(int context, int type, unsigned int val)
+/*
+ * Destroy a mem_block_map_t and release allocated memory.
+ */
+static void via_mem_block_map_destroy(mem_block_map_t *map)
{
- int i, retval = 0;
+ drm_free(map, sizeof(mem_block_map_t), DRM_MEM_DRIVER);
+}
- for (i = 0; i < MAX_CONTEXT; i++)
- if (global_ppriv[i].used && global_ppriv[i].context == context)
{
- retval = via_setDel(global_ppriv[i].sets[type], val);
- break;
+
+/*
+ * Add a pointer to a mem_block_map_t and return the new index,
+ * or return 0 if all slots are in use.
+ */
+static unsigned int via_mem_block_map_add(mem_block_map_t *map, PMemBlock
block)
+{
+ int h = (((unsigned long)block) >> 4) %
+ ((unsigned long)MAX_MEMBLOCK_INDEX);
+ int i;
+ for (i = h; i < MAX_MEMBLOCK_INDEX; i++)
+ if (!map->m[i]) {
+ map->m[i] = block;
+ return i + map->index_base;
}
+ for (i = 0; i < h; i++)
+ if (!map->m[i]) {
+ map->m[i] = block;
+ return i + map->index_base;
+ }
+ return 0;
+}
- return retval;
+/*
+ * Remove a pointer from a given index in a mem_block_map_t and return
+ * the removed pointer, or return NULL if no pointer was present.
+ */
+static PMemBlock via_mem_block_map_remove(mem_block_map_t *map,
+ unsigned int index)
+{
+ PMemBlock block;
+ if (index < map->index_base)
+ return NULL;
+ index -= map->index_base;
+ if (index >= MAX_MEMBLOCK_INDEX)
+ return NULL;
+ block = map->m[index];
+ map->m[index] = NULL;
+ return block;
+}
+
+/*
+ * Add this block pointer to the allocation map of the specified context
+ * and return the block index, or return 0 for failure.
+ */
+static unsigned int add_alloc_map(int context, int type, PMemBlock block)
+{
+ int i;
+ for (i = 0; i < MAX_CONTEXT; i++) {
+ via_context_t *ctx = &global_ppriv[i];
+ if (ctx->used && ctx->context == context)
+ return via_mem_block_map_add(ctx->map[type], block);
+ }
+ return 0;
+}
+
+/*
+ * Lookup and remove the specified block index from the context allocation map,
+ * and return the corresponding block pointer, or return NULL for not found.
+ */
+static PMemBlock del_alloc_map(int context, int type, unsigned int index)
+{
+ int i;
+ for (i = 0; i < MAX_CONTEXT; i++) {
+ via_context_t *ctx = &global_ppriv[i];
+ if (ctx->used && ctx->context == context)
+ return via_mem_block_map_remove(ctx->map[type], index);
+ }
+ return NULL;
}
/* agp memory management */
static memHeap_t *AgpHeap = NULL;
@@ -107,32 +186,37 @@
int i;
for (i = 0; i < MAX_CONTEXT; i++)
if (global_ppriv[i].used &&
(global_ppriv[i].context == context))
- break;
+ return 1;
- if (i >= MAX_CONTEXT) {
- for (i = 0; i < MAX_CONTEXT; i++) {
- if (!global_ppriv[i].used) {
- global_ppriv[i].context = context;
- global_ppriv[i].used = 1;
- global_ppriv[i].sets[0] = via_setInit();
- global_ppriv[i].sets[1] = via_setInit();
- DRM_DEBUG("init allocation set, socket=%d,"
- " context = %d\n", i, context);
- break;
+ for (i = 0; i < MAX_CONTEXT; i++) {
+ if (!global_ppriv[i].used) {
+ global_ppriv[i].map[0] =
via_mem_block_map_init(2*i*MAX_MEMBLOCK_INDEX + 1);
+ global_ppriv[i].map[1] =
via_mem_block_map_init((2*i+1)*MAX_MEMBLOCK_INDEX + 1);
+ if ( global_ppriv[i].map[0] == NULL ||
+ global_ppriv[i].map[1] == NULL ) {
+ if (global_ppriv[i].map[0] != NULL)
+
via_mem_block_map_destroy(global_ppriv[i].map[0]);
+ if (global_ppriv[i].map[1] != NULL)
+
via_mem_block_map_destroy(global_ppriv[i].map[1]);
+ DRM_DEBUG("init allocation set failed,"
+ " no memory, context=%d\n", context);
+ return 0;
}
- }
-
- if ((i >= MAX_CONTEXT) || (global_ppriv[i].sets[0] == NULL) ||
- (global_ppriv[i].sets[1] == NULL)) {
- return 0;
+ global_ppriv[i].context = context;
+ global_ppriv[i].used = 1;
+ DRM_DEBUG("init allocation set, socket=%d,"
+ " context = %d\n", i, context);
+ return 1;
}
}
- return 1;
+ DRM_DEBUG("init allocation set failed, no free socket, context=%d\n",
+ context);
+ return 0;
}
int via_final_context(struct drm_device *dev, int context)
{
int i;
@@ -142,35 +226,36 @@
if (global_ppriv[i].used &&
(global_ppriv[i].context == context))
break;
if (i < MAX_CONTEXT) {
- set_t *set;
- ITEM_TYPE item;
- int retval;
+ mem_block_map_t *map;
+ int index;
DRM_DEBUG("find socket %d, context = %d\n", i, context);
/* Video Memory */
- set = global_ppriv[i].sets[0];
- retval = via_setFirst(set, &item);
- while (retval) {
- DRM_DEBUG("free video memory 0x%lx\n", item);
- via_mmFreeMem((PMemBlock) item);
- retval = via_setNext(set, &item);
+ map = global_ppriv[i].map[0];
+ for (index = 0; index < MAX_MEMBLOCK_INDEX; index++) {
+ PMemBlock item = map->m[index];
+ if (item) {
+ DRM_DEBUG("free video memory %p\n", item);
+ via_mmFreeMem(item);
+ }
}
- via_setDestroy(set);
+ via_mem_block_map_destroy(map);
/* AGP Memory */
- set = global_ppriv[i].sets[1];
- retval = via_setFirst(set, &item);
- while (retval) {
- DRM_DEBUG("free agp memory 0x%lx\n", item);
- via_mmFreeMem((PMemBlock) item);
- retval = via_setNext(set, &item);
+ map = global_ppriv[i].map[1];
+ for (index = 0; index < MAX_MEMBLOCK_INDEX; index++) {
+ PMemBlock item = map->m[index];
+ if (item) {
+ DRM_DEBUG("free agp memory %p\n", item);
+ via_mmFreeMem(item);
+ }
}
- via_setDestroy(set);
+ via_mem_block_map_destroy(map);
global_ppriv[i].used = 0;
}
via_release_futex(dev_priv, context);
@@ -215,77 +300,67 @@
return -EFAULT;
}
static int via_fb_alloc(drm_via_mem_t * mem)
{
- drm_via_mm_t fb;
PMemBlock block;
int retval = 0;
if (!FBHeap)
return -1;
- fb.size = mem->size;
- fb.context = mem->context;
-
- block = via_mmAllocMem(FBHeap, fb.size, 5, 0);
+ block = via_mmAllocMem(FBHeap, mem->size, 5, 0);
if (block) {
- fb.offset = block->ofs;
- fb.free = (unsigned long)block;
- if (!add_alloc_set(fb.context, VIDEO, fb.free)) {
+ int index = add_alloc_map(mem->context, VIDEO, block);
+ if (!index) {
DRM_DEBUG("adding to allocation set fails\n");
- via_mmFreeMem((PMemBlock) fb.free);
+ via_mmFreeMem(block);
retval = -1;
+ } else {
+ mem->offset = block->ofs;
+ mem->index = index;
}
} else {
- fb.offset = 0;
- fb.size = 0;
- fb.free = 0;
+ mem->offset = 0;
+ mem->index = 0;
retval = -1;
}
- mem->offset = fb.offset;
- mem->index = fb.free;
-
- DRM_DEBUG("alloc fb, size = %d, offset = %d\n", fb.size,
- (int)fb.offset);
+ DRM_DEBUG("alloc fb, size = %d, offset = %d\n",
+ mem->size, (unsigned int)mem->offset);
return retval;
}
static int via_agp_alloc(drm_via_mem_t * mem)
{
- drm_via_mm_t agp;
PMemBlock block;
int retval = 0;
if (!AgpHeap)
return -1;
- agp.size = mem->size;
- agp.context = mem->context;
-
- block = via_mmAllocMem(AgpHeap, agp.size, 5, 0);
+ block = via_mmAllocMem(AgpHeap, mem->size, 5, 0);
if (block) {
- agp.offset = block->ofs;
- agp.free = (unsigned long)block;
- if (!add_alloc_set(agp.context, AGP, agp.free)) {
+ int index = add_alloc_map(mem->context, AGP, block);
+ if (!index) {
DRM_DEBUG("adding to allocation set fails\n");
- via_mmFreeMem((PMemBlock) agp.free);
+ via_mmFreeMem(block);
retval = -1;
+ } else {
+ mem->offset = block->ofs;
+ mem->index = index;
}
} else {
- agp.offset = 0;
- agp.size = 0;
- agp.free = 0;
+ mem->offset = 0;
+ mem->index = 0;
+ /* should we set retval = -1 ?? */
}
- mem->offset = agp.offset;
- mem->index = agp.free;
+ DRM_DEBUG("alloc agp, size = %d, offset = %d\n",
+ mem->size, (unsigned int)mem->offset);
- DRM_DEBUG("alloc agp, size = %d, offset = %d\n", agp.size,
- (unsigned int)agp.offset);
return retval;
}
int via_mem_free(DRM_IOCTL_ARGS)
{
@@ -309,53 +384,48 @@
return -EFAULT;
}
static int via_fb_free(drm_via_mem_t * mem)
{
- drm_via_mm_t fb;
+ PMemBlock block;
int retval = 0;
if (!FBHeap) {
return -1;
}
- fb.free = mem->index;
- fb.context = mem->context;
-
- if (!fb.free) {
+ if (!mem->index) {
return -1;
}
- via_mmFreeMem((PMemBlock) fb.free);
-
- if (!del_alloc_set(fb.context, VIDEO, fb.free)) {
+ block = del_alloc_map(mem->context, VIDEO, mem->index);
+ if (!block) {
retval = -1;
+ } else {
+ via_mmFreeMem(block);
}
- DRM_DEBUG("free fb, free = %ld\n", fb.free);
+ DRM_DEBUG("free fb, index = %ld\n", mem->index);
return retval;
}
static int via_agp_free(drm_via_mem_t * mem)
{
- drm_via_mm_t agp;
-
+ PMemBlock block;
int retval = 0;
- agp.free = mem->index;
- agp.context = mem->context;
-
- if (!agp.free)
+ if (!mem->index)
return -1;
- via_mmFreeMem((PMemBlock) agp.free);
-
- if (!del_alloc_set(agp.context, AGP, agp.free)) {
+ block = del_alloc_map(mem->context, AGP, mem->index);
+ if (!block) {
retval = -1;
+ } else {
+ via_mmFreeMem(block);
}
- DRM_DEBUG("free agp, free = %ld\n", agp.free);
+ DRM_DEBUG("free agp, index = %ld\n", mem->index);
return retval;
}
diff -urN -U 5 drm.prev/shared-core/via_mm.h drm/shared-core/via_mm.h
--- drm.prev/shared-core/via_mm.h 2005-08-14 11:10:23.000000000 +0200
+++ drm/shared-core/via_mm.h 1970-01-01 01:00:00.000000000 +0100
@@ -1,40 +0,0 @@
-/*
- * Copyright 1998-2003 VIA Technologies, Inc. All Rights Reserved.
- * Copyright 2001-2003 S3 Graphics, Inc. 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, sub license,
- * 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 NON-INFRINGEMENT. IN NO EVENT SHALL
- * VIA, S3 GRAPHICS, 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.
- */
-#ifndef _via_drm_mm_h_
-#define _via_drm_mm_h_
-
-typedef struct {
- unsigned int context;
- unsigned int size;
- unsigned long offset;
- unsigned long free;
-} drm_via_mm_t;
-
-typedef struct {
- unsigned int size;
- unsigned long handle;
- void *virtual;
-} drm_via_dma_t;
-
-#endif
