[PATCH 2/2] sm501fb.c: support mmap on PPC440SPe/PPC440EPx

Add driver specific mmap function to be able to mmap
frame buffer on PPC440SPe/PPC440EPx platforms. This
is needed because mmaping of the 36-bit physical
address of the frame buffer or MMIO is not supported
in generic fb_mmap().

Signed-off-by: Anatolij Gustschin <agust@denx.de>
Cc: Ben Dooks <redacted>
Cc: Simtec Linux Team <redacted>
---
 drivers/video/sm501fb.c |   52 +++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 52 insertions(+), 0 deletions(-)

diff --git a/drivers/video/sm501fb.c b/drivers/video/sm501fb.c
index 0b59112..9cdc298 100644
--- a/drivers/video/sm501fb.c
+++ b/drivers/video/sm501fb.c

@@ -1426,6 +1426,52 @@ static void sm501fb_fillrect(struct fb_info *info, const struct fb_fillrect *rec
 	writel(0x800100cc, fbi->regs2d + SM501_2D_CONTROL);
 }
 
+#if defined(CONFIG_440EPX) || defined(CONFIG_440SPe)
+static int sm501fb_mmap(struct fb_info *info,
+			struct vm_area_struct *vma)
+{
+	struct sm501fb_par *par = info->par;
+	struct sm501fb_info *fbi = par->info;
+	unsigned long long start;
+	unsigned long len, off, map_off;
+
+	off = vma->vm_pgoff << PAGE_SHIFT;
+	/*
+	 * Get 36-bit base address of the frame buffer memory.
+	 * Since it was previously truncated while assigning to
+	 * fix.smem_start, we retrieve correct address here.
+	 */
+	start = (fbi->fbmem_res->start & 0xf00000000ULL) +
+		info->fix.smem_start;
+	len = PAGE_ALIGN((start & ~PAGE_MASK) + info->fix.smem_len);
+
+	if (off >= len) {
+		/* memory mapped io */
+		off -= len;
+		if (info->var.accel_flags)
+			return -EINVAL;
+		start = fbi->regs_res->start;
+		len = PAGE_ALIGN((start & ~PAGE_MASK) +
+				 resource_size(fbi->regs_res));
+	}
+
+	start &= PAGE_MASK;
+	if ((vma->vm_end - vma->vm_start + off) > len)
+		return -EINVAL;
+
+	map_off = (unsigned long)((off + start) >> PAGE_SHIFT);
+	vma->vm_pgoff = map_off;
+
+	vma->vm_flags |= VM_IO | VM_RESERVED;
+	vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot);
+	if (io_remap_pfn_range(vma, vma->vm_start, map_off,
+			       vma->vm_end - vma->vm_start,
+			       vma->vm_page_prot))
+		return -EAGAIN;
+
+	return 0;
+}
+#endif
 
 static struct fb_ops sm501fb_ops_crt = {
 	.owner		= THIS_MODULE,

@@ -1439,6 +1485,9 @@ static struct fb_ops sm501fb_ops_crt = {
 	.fb_copyarea	= sm501fb_copyarea,
 	.fb_imageblit	= cfb_imageblit,
 	.fb_sync	= sm501fb_sync,
+#if defined(CONFIG_440EPX) || defined(CONFIG_440SPe)
+	.fb_mmap	= sm501fb_mmap,
+#endif
 };
 
 static struct fb_ops sm501fb_ops_pnl = {

@@ -1453,6 +1502,9 @@ static struct fb_ops sm501fb_ops_pnl = {
 	.fb_copyarea	= sm501fb_copyarea,
 	.fb_imageblit	= cfb_imageblit,
 	.fb_sync	= sm501fb_sync,
+#if defined(CONFIG_440EPX) || defined(CONFIG_440SPe)
+	.fb_mmap	= sm501fb_mmap,
+#endif
 };
 
 /* sm501_init_cursor

-- 
1.6.2.5

On 26/05/10 18:57, Anatolij Gustschin wrote:

Add driver specific mmap function to be able to mmap
frame buffer on PPC440SPe/PPC440EPx platforms. This
is needed because mmaping of the 36-bit physical
address of the frame buffer or MMIO is not supported
in generic fb_mmap().

Surely this is something we should be fixing in the
main fb layer?

On Wed, 26 May 2010 20:13:16 +0900
Ben Dooks [off-list ref] wrote:

On 26/05/10 18:57, Anatolij Gustschin wrote:

quoted

Add driver specific mmap function to be able to mmap
frame buffer on PPC440SPe/PPC440EPx platforms. This
is needed because mmaping of the 36-bit physical
address of the frame buffer or MMIO is not supported
in generic fb_mmap().

Surely this is something we should be fixing in the
main fb layer?

We need to store phys addresses > 32-bit somewhere. Changing
smem_start and mmio_start of the struct fb_fix_screeninfo to
unsigned long long would break user space compatibility.
How about adding smem_start_high and mmio_start_high to the
struct fb_fix_screeninfo for the purpose of storing upper
address bits?

On Wed, 26 May 2010 15:50:28 +0200
Anatolij Gustschin [off-list ref] wrote:

On Wed, 26 May 2010 20:13:16 +0900
Ben Dooks [off-list ref] wrote:

quoted

On 26/05/10 18:57, Anatolij Gustschin wrote:

quoted

Add driver specific mmap function to be able to mmap
frame buffer on PPC440SPe/PPC440EPx platforms. This
is needed because mmaping of the 36-bit physical
address of the frame buffer or MMIO is not supported
in generic fb_mmap().

Surely this is something we should be fixing in the
main fb layer?

We need to store phys addresses > 32-bit somewhere. Changing
smem_start and mmio_start of the struct fb_fix_screeninfo to
unsigned long long would break user space compatibility.

gaaah, that was a big screwup.  What are we doing passing these
addresses to and from userspace?

How about adding smem_start_high and mmio_start_high to the
struct fb_fix_screeninfo for the purpose of storing upper
address bits?

Bit ugly.  fb_fix_screeninfo32 is presently treated as "thing for
communicating with userspace" as well as "thing for kernel runtime
use".  We could separate these functions, and treat fb_fix_screeninfo32
as purely a userspace communication format.  Copy the fields in and out
when we talk to userspace.  Obviously the simpler implementation would
be to create a new fb_fix_screeninfo32_user.  And change the
fb_fix_screeninfo32 fields to dma_addr_t or whatever.

What a pita.

On 29/05/10 08:48, Andrew Morton wrote:

On Wed, 26 May 2010 15:50:28 +0200
Anatolij Gustschin [off-list ref] wrote:

quoted

On Wed, 26 May 2010 20:13:16 +0900
Ben Dooks [off-list ref] wrote:

quoted

On 26/05/10 18:57, Anatolij Gustschin wrote:

quoted

Add driver specific mmap function to be able to mmap
frame buffer on PPC440SPe/PPC440EPx platforms. This
is needed because mmaping of the 36-bit physical
address of the frame buffer or MMIO is not supported
in generic fb_mmap().

Surely this is something we should be fixing in the
main fb layer?

We need to store phys addresses > 32-bit somewhere. Changing
smem_start and mmio_start of the struct fb_fix_screeninfo to
unsigned long long would break user space compatibility.

gaaah, that was a big screwup.  What are we doing passing these
addresses to and from userspace?

quoted

How about adding smem_start_high and mmio_start_high to the
struct fb_fix_screeninfo for the purpose of storing upper
address bits?

Bit ugly.  fb_fix_screeninfo32 is presently treated as "thing for
communicating with userspace" as well as "thing for kernel runtime
use".  We could separate these functions, and treat fb_fix_screeninfo32
as purely a userspace communication format.  Copy the fields in and out
when we talk to userspace.  Obviously the simpler implementation would
be to create a new fb_fix_screeninfo32_user.  And change the
fb_fix_screeninfo32 fields to dma_addr_t or whatever.

Something like that, we could add a call to get an u64
smem_start if people really need that in userspace.

I'd rather see a solution that works for all drivers
than fixing up individual drivers.

If people don't want to spend too much time fixing
drivers, we could simply deprecate smem_start and
mmio_start from the in-kernel one and move it outside
to avoid a pile of selective copying of stuff across
ioctl() calls.

-- 
Ben