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

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

[Anatolij Gustschin]

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 <ben@simtec.co.uk>
Cc: Simtec Linux Team <linux@simtec.co.uk>
---
 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

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

[Ben Dooks]

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?

> Signed-off-by: Anatolij Gustschin <agust@denx.de>
> Cc: Ben Dooks <ben@simtec.co.uk>
> Cc: Simtec Linux Team <linux@simtec.co.uk>
> ---
>  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

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

[Anatolij Gustschin]

On Wed, 26 May 2010 20:13:16 +0900
Ben Dooks <ben@simtec.co.uk> wrote:

> 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?

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?

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

[Andrew Morton]

On Wed, 26 May 2010 15:50:28 +0200
Anatolij Gustschin <agust@denx.de> wrote:

> On Wed, 26 May 2010 20:13:16 +0900
> Ben Dooks <ben@simtec.co.uk> wrote:
> 
> > 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?
> 
> 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.

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

[Ben Dooks]

On 29/05/10 08:48, Andrew Morton wrote:
> On Wed, 26 May 2010 15:50:28 +0200
> Anatolij Gustschin <agust@denx.de> wrote:
> 
>> On Wed, 26 May 2010 20:13:16 +0900
>> Ben Dooks <ben@simtec.co.uk> wrote:
>>
>>> 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?
>>
>> 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.

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