On Wed, 2009-08-05 at 12:06 +0200, Thomas Hellström wrote:
> Michel Dänzer wrote:
> > On Wed, 2009-08-05 at 11:18 +0200, Thomas Hellström wrote:
> >
> >> Aargh. Wait. I remember now.
> >>
> >> The fbcon bo is exported through the fbdev address space at offset 0.
> >> The vm_node is for the drm device address space only. So it is perfectly
> >> legal and actually correct for it not to have a vm_node, unless it's
> >> going to be accessible from the drm device. Does it need to for KMS?
> >>
> >
> > I don't think so.
> >
> >
> >> I'm a bit unsure whether it's OK to export a bo through two different
> >> address spaces. In particular, unmap_mapping_range() on the drm device
> >> will not kill the fbdev user-space mappings.
> >>
> >
> > Hmm, so that would mean that if an fbdev mapping is created while the BO
> > is in VRAM, it would still access VRAM after the BO has been evicted? Is
> > there a solution for this?
> >
> >
> Yes, You need to call unmap_mapping_range() on the fbdev address space.
> See how that is done in ttm_bo_unmap_virtual() for the drm address
> space. Actually, I think you need to set up a bo_driver hook in
> ttm_bo_unmap_virtual() to do this every time the bo is moved or swapped out.
The attached patches should hopefully address all the feedback I've
received here and on IRC. Let me know what you think.
--
Earthling Michel Dänzer | http://www.vmware.com
Libre software enthusiast | Debian, X and DRI developer
From bc1e3c5d116f194ac972f6f69b79b69bc64d1789 Mon Sep 17 00:00:00 2001
From: =?utf-8?q?Michel=20D=C3=A4nzer?= <daen...@vmware.com>
Date: Mon, 10 Aug 2009 23:41:47 +0200
Subject: [PATCH 1/2] ttm: fbdev fixes.
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 8bit
* Fix ttm_bo_fbdev_io() and export it.
* Add a BO driver hook for unmapping virtual memory ranges backed by a BO.
Signed-off-by: Michel Dänzer <daen...@vmware.com>
---
drivers/gpu/drm/ttm/ttm_bo.c | 4 ++++
drivers/gpu/drm/ttm/ttm_bo_vm.c | 29 ++++++++++++++++++++---------
include/drm/ttm/ttm_bo_api.h | 15 +++++++++++++++
include/drm/ttm/ttm_bo_driver.h | 15 +++++++++++++++
4 files changed, 54 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index b98a02d..2c363e1 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -1443,6 +1443,10 @@ void ttm_bo_unmap_virtual(struct ttm_buffer_object *bo)
loff_t offset = (loff_t) bo->addr_space_offset;
loff_t holelen = ((loff_t) bo->mem.num_pages) << PAGE_SHIFT;
+ if (bdev->driver->unmap_mapping_range &&
+ bdev->driver->unmap_mapping_range(bo))
+ return;
+
if (!bdev->dev_mapping)
return;
diff --git a/drivers/gpu/drm/ttm/ttm_bo_vm.c b/drivers/gpu/drm/ttm/ttm_bo_vm.c
index 27b146c..bdd1bed 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_vm.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_vm.c
@@ -129,10 +129,15 @@ static int ttm_bo_vm_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
is_iomem = (bus_size != 0);
- page_offset = ((address - vma->vm_start) >> PAGE_SHIFT) +
- bo->vm_node->start - vma->vm_pgoff;
- page_last = ((vma->vm_end - vma->vm_start) >> PAGE_SHIFT) +
- bo->vm_node->start - vma->vm_pgoff;
+ page_offset = ((address - vma->vm_start) >> PAGE_SHIFT) -
+ vma->vm_pgoff;
+ page_last = ((vma->vm_end - vma->vm_start) >> PAGE_SHIFT) -
+ vma->vm_pgoff;
+
+ if (likely(bo->vm_node)) {
+ page_offset += bo->vm_node->start;
+ page_last += bo->vm_node->start;
+ }
if (unlikely(page_offset >= bo->num_pages)) {
retval = VM_FAULT_SIGBUS;
@@ -414,23 +419,25 @@ ssize_t ttm_bo_fbdev_io(struct ttm_buffer_object *bo, const char __user *wbuf,
kmap_end = (*f_pos + count - 1) >> PAGE_SHIFT;
kmap_num = kmap_end - kmap_offset + 1;
+ ttm_bo_reference(bo);
ret = ttm_bo_reserve(bo, true, no_wait, false, 0);
switch (ret) {
case 0:
break;
case -ERESTART:
- return -EINTR;
+ ret = -EINTR;
+ goto out_unreference;
case -EBUSY:
- return -EAGAIN;
+ ret = -EAGAIN;
+ goto out_unreference;
default:
- return ret;
+ goto out_unreference;
}
ret = ttm_bo_kmap(bo, kmap_offset, kmap_num, &map);
if (unlikely(ret != 0)) {
- ttm_bo_unreserve(bo);
- return ret;
+ goto out_unreserve;
}
virtual = ttm_kmap_obj_virtual(&map, &dummy);
@@ -442,7 +449,10 @@ ssize_t ttm_bo_fbdev_io(struct ttm_buffer_object *bo, const char __user *wbuf,
ret = copy_to_user(rbuf, virtual, io_size);
ttm_bo_kunmap(&map);
+
+out_unreserve:
ttm_bo_unreserve(bo);
+out_unreference:
ttm_bo_unref(&bo);
if (unlikely(ret != 0))
@@ -452,3 +462,4 @@ ssize_t ttm_bo_fbdev_io(struct ttm_buffer_object *bo, const char __user *wbuf,
return io_size;
}
+EXPORT_SYMBOL(ttm_bo_fbdev_io);
diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h
index cd22ab4..94cc624 100644
--- a/include/drm/ttm/ttm_bo_api.h
+++ b/include/drm/ttm/ttm_bo_api.h
@@ -575,6 +575,21 @@ extern int ttm_fbdev_mmap(struct vm_area_struct *vma,
struct ttm_buffer_object *bo);
/**
+ * ttm_bo_fbdev_io - Handle fbdev I/O on a ttm buffer object.
+ *
+ * @bo: The bo backing the address space. The address space will
+ * have the same size as the bo, and start at offset 0.
+ *
+ * This function is intended to be called by the fbdev fb_read/write
+ * methods if the fbdev address space is to be backed by a bo.
+ */
+
+extern ssize_t ttm_bo_fbdev_io(struct ttm_buffer_object *bo,
+ const char __user *wbuf,
+ char __user *rbuf, size_t count,
+ loff_t *f_pos, bool write);
+
+/**
* ttm_bo_mmap - mmap out of the ttm device address space.
*
* @filp: filp as input from the mmap method.
diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h
index ea83dd2..99f3248 100644
--- a/include/drm/ttm/ttm_bo_driver.h
+++ b/include/drm/ttm/ttm_bo_driver.h
@@ -354,6 +354,21 @@ struct ttm_bo_driver {
int (*sync_obj_flush) (void *sync_obj, void *sync_arg);
void (*sync_obj_unref) (void **sync_obj);
void *(*sync_obj_ref) (void *sync_obj);
+
+ /**
+ * struct ttm_bo_driver_member unmap_mapping_range
+ *
+ * @bo: Pointer to a buffer object.
+ *
+ * Called for unmapping virtual memory ranges backed by the buffer
+ * object.
+ * This member may be set to NULL, in which case the caller behaves the
+ * same as if the member exists but returns false.
+ * This function should return true if it umapped all virtual memory
+ * ranges backed by the buffer object, false if the caller should take
+ * care of the standard buffer object device address space.
+ */
+ bool (*unmap_mapping_range)(struct ttm_buffer_object *bo);
};
#define TTM_NUM_MEM_TYPES 8
--
1.6.3.3
From ae75cc72a5cff09768a2e126efaf11684d76309d Mon Sep 17 00:00:00 2001
From: =?utf-8?q?Michel=20D=C3=A4nzer?= <daen...@vmware.com>
Date: Mon, 10 Aug 2009 23:41:47 +0200
Subject: [PATCH 2/2] drm/radeon: Don't always keep the fbcon BO pinned in VRAM.
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 8bit
Use the fb_mmap hook and new TTM unmap_mapping_range hook for userspace mappings
and re-create the kernel mapping whenever the fbcon BO moves back into VRAM. KMS
will pin it while it's scanned out. This way we aren't wasting precious VRAM
when we're e.g. in X.
Signed-off-by: Michel Dänzer <daen...@vmware.com>
---
drivers/gpu/drm/radeon/radeon.h | 2 +-
drivers/gpu/drm/radeon/radeon_device.c | 4 +-
drivers/gpu/drm/radeon/radeon_fb.c | 99 +++++++++++++++++++++++++++-----
drivers/gpu/drm/radeon/radeon_gem.c | 2 -
drivers/gpu/drm/radeon/radeon_mode.h | 8 +++
drivers/gpu/drm/radeon/radeon_ttm.c | 42 +++++++++++++-
6 files changed, 135 insertions(+), 22 deletions(-)
diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
index 3cd43ce..f56b6d2 100644
--- a/drivers/gpu/drm/radeon/radeon.h
+++ b/drivers/gpu/drm/radeon/radeon.h
@@ -601,7 +601,7 @@ struct radeon_device {
struct radeon_object *stollen_vga_memory;
struct fb_info *fbdev_info;
struct radeon_object *fbdev_robj;
- struct radeon_framebuffer *fbdev_rfb;
+ struct address_space *fbdev_mapping;
/* Register mmio */
unsigned long rmmio_base;
unsigned long rmmio_size;
diff --git a/drivers/gpu/drm/radeon/radeon_device.c b/drivers/gpu/drm/radeon/radeon_device.c
index a03eebd..a2418d4 100644
--- a/drivers/gpu/drm/radeon/radeon_device.c
+++ b/drivers/gpu/drm/radeon/radeon_device.c
@@ -705,9 +705,7 @@ int radeon_suspend_kms(struct drm_device *dev, pm_message_t state)
continue;
}
robj = rfb->obj->driver_private;
- if (robj != rdev->fbdev_robj) {
- radeon_object_unpin(robj);
- }
+ radeon_object_unpin(robj);
}
/* evict vram memory */
radeon_object_evict_vram(rdev);
diff --git a/drivers/gpu/drm/radeon/radeon_fb.c b/drivers/gpu/drm/radeon/radeon_fb.c
index 260870a..d4ff495 100644
--- a/drivers/gpu/drm/radeon/radeon_fb.c
+++ b/drivers/gpu/drm/radeon/radeon_fb.c
@@ -404,6 +404,84 @@ int radeonfb_blank(int blank, struct fb_info *info)
return 0;
}
+static int radeonfb_mmap(struct fb_info *info, struct vm_area_struct *vma)
+{
+ struct radeon_fb_device *rfbdev = info->par;
+ struct radeon_object *robj = rfbdev->rfb->obj->driver_private;
+
+ if (!rfbdev->rdev->fbdev_mapping)
+ rfbdev->rdev->fbdev_mapping = vma->vm_file->f_mapping;
+
+ return radeon_object_fbdev_mmap(robj, vma);
+}
+
+ssize_t radeonfb_read(struct fb_info *info, char __user *buf, size_t count,
+ loff_t *ppos)
+{
+ struct radeon_fb_device *rfbdev = info->par;
+
+ return ttm_bo_fbdev_io(rfbdev->rfb->obj->driver_private, NULL, buf,
+ count, ppos, false);
+}
+
+ssize_t radeonfb_write(struct fb_info *info, const char __user *buf,
+ size_t count, loff_t *ppos)
+{
+ struct radeon_fb_device *rfbdev = info->par;
+
+ return ttm_bo_fbdev_io(rfbdev->rfb->obj->driver_private, buf, NULL,
+ count, ppos, true);
+}
+
+/* It looks like fbcon likes to access the framebuffer just before it's being
+ * displayed. To cope with that (and to protect against fbcon bugs causing it to
+ * access the framebuffer when it's not being (or going to be) displayed), just
+ * move the BO to VRAM if fbcon attempts to access the framebuffer while it's
+ * somewhere else.
+ */
+static bool radeonfb_remap(struct fb_info *info)
+{
+ if (info->screen_base == (void*)~0) {
+ struct radeon_fb_device *rfbdev = info->par;
+ struct radeon_object *robj = rfbdev->rfb->obj->driver_private;
+ uint64_t gpu_addr;
+
+ if (radeon_object_pin(robj, RADEON_GEM_DOMAIN_VRAM, &gpu_addr))
+ return false;
+ radeon_object_unpin(robj);
+ }
+
+ info->fbops->fb_fillrect = cfb_fillrect;
+ info->fbops->fb_copyarea = cfb_copyarea;
+ info->fbops->fb_imageblit = cfb_imageblit;
+
+ return true;
+}
+
+void radeonfb_fillrect(struct fb_info *info, const struct fb_fillrect *rect)
+{
+ if (!radeonfb_remap(info))
+ return;
+
+ cfb_fillrect(info, rect);
+}
+
+void radeonfb_copyarea(struct fb_info *info, const struct fb_copyarea *region)
+{
+ if (!radeonfb_remap(info))
+ return;
+
+ cfb_copyarea(info, region);
+}
+
+void radeonfb_imageblit(struct fb_info *info, const struct fb_image *image)
+{
+ if (!radeonfb_remap(info))
+ return;
+
+ cfb_imageblit(info, image);
+}
+
static struct fb_ops radeonfb_ops = {
.owner = THIS_MODULE,
.fb_check_var = radeonfb_check_var,
@@ -414,6 +492,9 @@ static struct fb_ops radeonfb_ops = {
.fb_imageblit = cfb_imageblit,
.fb_pan_display = radeonfb_pan_display,
.fb_blank = radeonfb_blank,
+ .fb_mmap = radeonfb_mmap,
+ .fb_read = radeonfb_read,
+ .fb_write = radeonfb_write,
};
/**
@@ -509,9 +590,7 @@ int radeonfb_create(struct radeon_device *rdev,
struct radeon_object *robj = NULL;
struct device *device = &rdev->pdev->dev;
int size, aligned_size, ret;
- u64 fb_gpuaddr;
void *fbptr = NULL;
- unsigned long tmp;
mode_cmd.width = surface_width;
mode_cmd.height = surface_height;
@@ -524,9 +603,8 @@ int radeonfb_create(struct radeon_device *rdev,
aligned_size = ALIGN(size, PAGE_SIZE);
ret = radeon_gem_object_create(rdev, aligned_size, 0,
- RADEON_GEM_DOMAIN_VRAM,
- false, ttm_bo_type_kernel,
- false, &gobj);
+ RADEON_GEM_DOMAIN_VRAM,
+ false, true, false, &gobj);
if (ret) {
printk(KERN_ERR "failed to allocate framebuffer (%d %d)\n",
surface_width, surface_height);
@@ -542,18 +620,11 @@ int radeonfb_create(struct radeon_device *rdev,
ret = -ENOMEM;
goto out_unref;
}
- ret = radeon_object_pin(robj, RADEON_GEM_DOMAIN_VRAM, &fb_gpuaddr);
- if (ret) {
- printk(KERN_ERR "failed to pin framebuffer\n");
- ret = -ENOMEM;
- goto out_unref;
- }
list_add(&fb->filp_head, &rdev->ddev->mode_config.fb_kernel_list);
rfb = to_radeon_framebuffer(fb);
*rfb_p = rfb;
- rdev->fbdev_rfb = rfb;
rdev->fbdev_robj = robj;
info = framebuffer_alloc(sizeof(struct radeon_fb_device), device);
@@ -580,8 +651,7 @@ int radeonfb_create(struct radeon_device *rdev,
info->flags = FBINFO_DEFAULT;
info->fbops = &radeonfb_ops;
info->fix.line_length = fb->pitch;
- tmp = fb_gpuaddr - rdev->mc.vram_location;
- info->fix.smem_start = rdev->mc.aper_base + tmp;
+ info->fix.smem_start = 0;
info->fix.smem_len = size;
info->screen_base = fbptr;
info->screen_size = size;
@@ -870,7 +940,6 @@ int radeonfb_remove(struct drm_device *dev, struct drm_framebuffer *fb)
robj = rfb->obj->driver_private;
unregister_framebuffer(info);
radeon_object_kunmap(robj);
- radeon_object_unpin(robj);
framebuffer_release(info);
}
diff --git a/drivers/gpu/drm/radeon/radeon_gem.c b/drivers/gpu/drm/radeon/radeon_gem.c
index c3dd0fc..c36d603 100644
--- a/drivers/gpu/drm/radeon/radeon_gem.c
+++ b/drivers/gpu/drm/radeon/radeon_gem.c
@@ -163,8 +163,6 @@ int radeon_gem_info_ioctl(struct drm_device *dev, void *data,
args->vram_visible = rdev->mc.vram_size;
if (rdev->stollen_vga_memory)
args->vram_visible -= radeon_object_size(rdev->stollen_vga_memory);
- if (rdev->fbdev_robj)
- args->vram_visible -= radeon_object_size(rdev->fbdev_robj);
list_for_each_entry(crtc, &rdev->ddev->mode_config.crtc_list, head) {
if (drm_helper_crtc_in_use(crtc)) {
struct radeon_crtc *radeon_crtc = to_radeon_crtc(crtc);
diff --git a/drivers/gpu/drm/radeon/radeon_mode.h b/drivers/gpu/drm/radeon/radeon_mode.h
index 99a35b1..2bd5627 100644
--- a/drivers/gpu/drm/radeon/radeon_mode.h
+++ b/drivers/gpu/drm/radeon/radeon_mode.h
@@ -362,6 +362,14 @@ struct drm_framebuffer *radeon_framebuffer_create(struct drm_device *dev,
struct drm_mode_fb_cmd *mode_cmd,
struct drm_gem_object *obj);
+ssize_t radeonfb_read(struct fb_info *info, char __user *buf, size_t count,
+ loff_t *ppos);
+ssize_t radeonfb_write(struct fb_info *info, const char __user *buf,
+ size_t count, loff_t *ppos);
+void radeonfb_fillrect(struct fb_info *info, const struct fb_fillrect *rect);
+void radeonfb_copyarea(struct fb_info *info, const struct fb_copyarea *region);
+void radeonfb_imageblit(struct fb_info *info, const struct fb_image *image);
+
int radeonfb_probe(struct drm_device *dev);
int radeonfb_remove(struct drm_device *dev, struct drm_framebuffer *fb);
diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c
index 8a3015c..8a0c6f1 100644
--- a/drivers/gpu/drm/radeon/radeon_ttm.c
+++ b/drivers/gpu/drm/radeon/radeon_ttm.c
@@ -350,7 +350,8 @@ static int radeon_bo_move(struct ttm_buffer_object *bo,
new_mem->mem_type == TTM_PL_TT)) {
/* bind is enought */
radeon_move_null(bo, new_mem);
- return 0;
+ r = 0;
+ goto remap;
}
if (!rdev->cp.ready) {
/* use memcpy */
@@ -375,6 +376,30 @@ memcpy:
r = ttm_bo_move_memcpy(bo, evict, no_wait, new_mem);
}
+remap:
+ if (likely(!r) && unlikely(bo == (struct ttm_buffer_object*)rdev->fbdev_robj)) {
+ struct fb_ops *fbops = rdev->fbdev_info->fbops;
+
+ if (rdev->fbdev_info->screen_base != (void*)~0) {
+ radeon_object_kunmap(rdev->fbdev_robj);
+ rdev->fbdev_info->screen_base = (void*)~0;
+ }
+
+ if (ttm_mem_reg_is_pci(bo->bdev, new_mem)) {
+ r = radeon_object_kmap(rdev->fbdev_robj,
+ (void**)&rdev->fbdev_info->screen_base);
+
+ if (!r) {
+ fbops->fb_fillrect = cfb_fillrect;
+ fbops->fb_copyarea = cfb_copyarea;
+ fbops->fb_imageblit = cfb_imageblit;
+ }
+ } else {
+ fbops->fb_fillrect = radeonfb_fillrect;
+ fbops->fb_copyarea = radeonfb_copyarea;
+ fbops->fb_imageblit = radeonfb_imageblit;
+ }
+ }
return r;
}
@@ -416,6 +441,20 @@ static bool radeon_sync_obj_signaled(void *sync_obj, void *sync_arg)
return radeon_fence_signaled((struct radeon_fence *)sync_obj);
}
+static bool radeon_unmap_mapping_range(struct ttm_buffer_object *bo)
+{
+ struct radeon_device *rdev = radeon_get_rdev(bo->bdev);
+
+ if (rdev->fbdev_mapping && bo == (struct ttm_buffer_object*)rdev->fbdev_robj) {
+ unmap_mapping_range(rdev->fbdev_mapping, 0,
+ ((loff_t) bo->mem.num_pages) << PAGE_SHIFT,
+ 1);
+ return true;
+ }
+
+ return false;
+}
+
static struct ttm_bo_driver radeon_bo_driver = {
.mem_type_prio = radeon_mem_prios,
.mem_busy_prio = radeon_busy_prios,
@@ -432,6 +471,7 @@ static struct ttm_bo_driver radeon_bo_driver = {
.sync_obj_flush = &radeon_sync_obj_flush,
.sync_obj_unref = &radeon_sync_obj_unref,
.sync_obj_ref = &radeon_sync_obj_ref,
+ .unmap_mapping_range = &radeon_unmap_mapping_range,
};
int radeon_ttm_init(struct radeon_device *rdev)
--
1.6.3.3
------------------------------------------------------------------------------
Let Crystal Reports handle the reporting - Free Crystal Reports 2008 30-Day
trial. Simplify your report design, integration and deployment - and focus on
what you do best, core application coding. Discover what's new with
Crystal Reports now. http://p.sf.net/sfu/bobj-july
--
_______________________________________________
Dri-devel mailing list
Dri-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/dri-devel
