Re: [git pull] drm: previous pull req + 1.

[Linus Torvalds]

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,
 };
 
 /**
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/