lkml.org 
[lkml]   [2015]   [Feb]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 4/8] fbdev: ssd1307fb: Use vmalloc to allocate video memory.
On Sat, Feb 07, 2015 at 04:35:41PM +0100, Thomas Niederprüm wrote:
> Am Sat, 7 Feb 2015 12:18:21 +0100
> schrieb Maxime Ripard <maxime.ripard@free-electrons.com>:
>
> > Hi,
> >
> > On Fri, Feb 06, 2015 at 11:28:10PM +0100, niederp@physik.uni-kl.de
> > wrote:
> > > From: Thomas Niederprüm <niederp@physik.uni-kl.de>
> > >
> > > It makes sense to use vmalloc to allocate the video buffer since it
> > > has to be page aligned memory for using it with mmap.
> >
> > Please wrap your commit log at 80 chars.
>
> I'll try to do so in future, sorry for that.
>
> >
> > It looks like there's numerous fbdev drivers using this (especially
> > since you copy pasted that code, without mentionning it).
>
> Yes, I should have mentioned that in the commit message. As
> implicitly indicated in the cover letter the rvmalloc() and rvfree() are
> copy pasted from the vfb driver. Honestly, I didn't give this one too
> much thought. It seemed a viable solution to the mmap problem. For a
> bit more history on that, see my comment below.
>
> >
> > That should be turned into an allocator so that drivers all get this
> > right.
> >
> > > Also deffered io seems buggy in combination with kmalloc'ed memory
> > > (crash on unloading the module).
> >
> > And maybe that's the real issue to fix.
>
> The problem is solved by using vmalloc ;)

Yep, but why do you need to mark the reserved pages?

...

> > > @@ -570,7 +608,7 @@ static int ssd1307fb_probe(struct i2c_client
> > > *client, info->var.blue.offset = 0;
> > >
> > > info->screen_base = (u8 __force __iomem *)vmem;
> > > - info->fix.smem_start = (unsigned long)vmem;
> > > + info->fix.smem_start = __pa(vmem);
> >
> > Why are you changing from virtual to physical address here?
>
> Oh, good catch! This is still a residual of my attempts to get this
> working with kmalloc'ed memory. In the current state the driver is
> presenting a completely wrong memory address upon mmap. As reported in
> [0] info->fix.smem_start has to hold the physical address of the video
> memory if it was allocated using kmalloc. Correcting this let me run
> into the problem that the kmalloc'ed memory was not page aligned but,
> the memory address handed to userspace mmap was aligned to the next
> full page, resulting in an inaccessable display region. At that point I
> just copied the vmalloc approach from the vfb driver.
>
> [0] http://stackoverflow.com/questions/22285151/kernel-panic-using-deferred-io-on-kmalloced-buffer

And the documentation of fb_fix_screeninfo indeed state that this
should be the physical address:
http://lxr.free-electrons.com/source/include/uapi/linux/fb.h#L158

Could you make that a separate patch?

Thanks,
Maxime

--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
[unhandled content-type:application/pgp-signature]
\
 
 \ /
  Last update: 2015-02-12 16:21    [W:0.157 / U:2.664 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site