linux-fbdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Maxime Ripard <maxime.ripard@free-electrons.com>
To: "Thomas Niederprüm" <niederp@physik.uni-kl.de>
Cc: linux-fbdev@vger.kernel.org, plagnioj@jcrosoft.com,
	tomi.valkeinen@ti.com, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 4/8] fbdev: ssd1307fb: Use vmalloc to allocate video memory.
Date: Thu, 12 Feb 2015 15:11:21 +0000	[thread overview]
Message-ID: <20150212151121.GJ2079@lukather> (raw)
In-Reply-To: <20150207163541.30047a33@maestro.intranet>

[-- Attachment #1: Type: text/plain, Size: 2879 bytes --]

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

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

  reply	other threads:[~2015-02-12 15:11 UTC|newest]

Thread overview: 138+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-02-06 22:28 [PATCH 0/8] Cleanup and add support for SSD1305 niederp
2015-02-06 22:28 ` [PATCH 1/8] Documentation: dts: add missing Solomon Systech vendor prefix niederp
2015-02-06 22:28 ` [PATCH 2/8] fbdev: ssd1307fb: Unify init code and make controller configurable from device tree niederp
2015-02-07 10:42   ` Maxime Ripard
2015-02-07 14:59     ` Thomas Niederprüm
2015-02-07 15:19       ` Noralf Trønnes
2015-02-09 10:37         ` Thomas Niederprüm
2015-02-12 16:58         ` Maxime Ripard
2015-02-12 16:41       ` Maxime Ripard
2015-02-14 16:12         ` Thomas Niederprüm
2015-02-23  9:43           ` Maxime Ripard
2015-02-06 22:28 ` [PATCH 3/8] fbdev: ssd1307fb: Add support for SSD1305 niederp
2015-02-06 22:28 ` [PATCH 4/8] fbdev: ssd1307fb: Use vmalloc to allocate video memory niederp
2015-02-07 11:18   ` Maxime Ripard
2015-02-07 15:35     ` Thomas Niederprüm
2015-02-12 15:11       ` Maxime Ripard [this message]
2015-02-14 14:22         ` Thomas Niederprüm
2015-02-14 15:36           ` Maxime Ripard
2015-03-10 11:28           ` Tomi Valkeinen
2015-03-13 21:31             ` Thomas Niederprüm
2015-03-14 22:02               ` Geert Uytterhoeven
2015-03-20 11:37                 ` Tomi Valkeinen
2015-03-20 12:12                   ` Geert Uytterhoeven
2015-03-20 14:47                   ` Maxime Ripard
2015-03-20 15:24                     ` Geert Uytterhoeven
2015-03-20 20:27                       ` Thomas Niederprüm
2015-03-20 15:25                     ` Tomi Valkeinen
2015-03-20 20:36                       ` Thomas Niederprüm
2015-02-06 22:28 ` [PATCH 5/8] fbdev: ssd1307fb: Add module parameter bitsperpixel niederp
2015-02-07 11:20   ` Maxime Ripard
2015-02-07 16:05     ` Thomas Niederprüm
2015-02-14 15:54       ` Maxime Ripard
2015-03-10 10:45         ` Tomi Valkeinen
2015-03-13 19:25           ` Thomas Niederprüm
2015-03-25 10:56           ` Olliver Schinagl
2015-03-25 16:02             ` Maxime Ripard
2015-02-06 22:28 ` [PATCH 6/8] fbdev: ssd1307fb: Add module parameter to set update delay of the deffered io niederp
2015-02-07 11:26   ` Maxime Ripard
2015-02-07 16:12     ` Thomas Niederprüm
2015-02-09  9:03       ` Maxime Ripard
2015-02-06 22:28 ` [PATCH 7/8] fbdev: ssd1307fb: Add sysfs handles to expose contrast and dim setting to userspace niederp
2015-02-07 11:43   ` Maxime Ripard
2015-02-07 16:42     ` Thomas Niederprüm
2015-02-09  8:52       ` Maxime Ripard
2015-03-10 10:49         ` Tomi Valkeinen
2015-03-13 19:21           ` Thomas Niederprüm
2015-02-06 22:28 ` [PATCH 8/8] fbdev: ssd1307fb: Turn off display on driver unload niederp
2015-02-07 11:45   ` Maxime Ripard
2015-02-07 16:49     ` Thomas Niederprüm
2015-03-01 22:27 ` [PATCHv2 00/10] fbdev: ssd1307fb: Cleanup and add support for SSD1305 Thomas Niederprüm
2015-03-01 22:27   ` [PATCHv2 01/10] fbdev: ssd1307fb: fix memory address smem_start Thomas Niederprüm
2015-03-03  7:17     ` Maxime Ripard
2015-03-01 22:27   ` [PATCHv2 02/10] fbdev: ssd1307fb: Use vmalloc to allocate video memory Thomas Niederprüm
2015-03-03  8:52     ` Maxime Ripard
2015-03-03 19:04       ` Thomas Niederprüm
2015-03-01 22:27   ` [PATCHv2 03/10] Documentation: dts: add missing Solomon Systech vendor prefix Thomas Niederprüm
2015-03-01 22:27   ` [PATCHv2 04/10] fbdev: ssd1307fb: Unify init code and obtain hw specific bits from DT Thomas Niederprüm
2015-03-05 21:49     ` Maxime Ripard
2015-03-01 22:27   ` [PATCHv2 05/10] fbdev: ssd1307fb: fix in tree users of ssd1306 Thomas Niederprüm
2015-03-03  9:31     ` Maxime Ripard
2015-03-01 22:27   ` [PATCHv2 06/10] fbdev: ssd1307fb: Add support for SSD1305 Thomas Niederprüm
2015-03-01 22:28   ` [PATCHv2 07/10] fbdev: ssd1307fb: Add module parameter to set refresh rate of the display Thomas Niederprüm
2015-03-05 22:12     ` Maxime Ripard
2015-03-01 22:28   ` [PATCHv2 08/10] fbdev: ssd1307fb: Add module parameter bitsperpixel Thomas Niederprüm
2015-03-01 22:28   ` [PATCHv2 09/10] fbdev: ssd1307fb: Add sysfs handles to expose contrast and dim setting to userspace Thomas Niederprüm
2015-03-01 22:28   ` [PATCHv2 10/10] fbdev: ssd1307fb: Turn off display on driver unload Thomas Niederprüm
2015-03-03  9:40     ` Maxime Ripard
2015-03-09 22:22 ` [PATCHv3 00/10] fbdev: ssd1307fb: Cleanup and add support for SSD1305 Thomas Niederprüm
2015-03-09 22:22   ` [PATCHv3 01/10] fbdev: ssd1307fb: fix memory address smem_start Thomas Niederprüm
2015-03-09 22:22   ` [PATCHv3 02/10] fbdev: ssd1307fb: Use vmalloc to allocate video memory Thomas Niederprüm
2015-03-09 22:22   ` [PATCHv3 03/10] of: Add Solomon Systech vendor prefix Thomas Niederprüm
2015-03-09 22:22   ` [PATCHv3 04/10] fbdev: ssd1307fb: Unify init code and obtain hw specific bits from DT Thomas Niederprüm
2015-03-09 22:22   ` [PATCHv3 05/10] ARM: mxs: fix in tree users of ssd1306 Thomas Niederprüm
2015-03-09 22:22   ` [PATCHv3 06/10] fbdev: ssd1307fb: Add support for SSD1305 Thomas Niederprüm
2015-03-09 22:22   ` [PATCHv3 07/10] fbdev: ssd1307fb: Add module parameter to set refresh rate of the display Thomas Niederprüm
2015-03-09 22:22   ` [PATCHv3 08/10] fbdev: ssd1307fb: Add module parameter bitsperpixel Thomas Niederprüm
2015-03-09 22:22   ` [PATCHv3 09/10] fbdev: ssd1307fb: Add sysfs handles to expose contrast and dim setting to userspace Thomas Niederprüm
2015-03-09 22:22   ` [PATCHv3 10/10] fbdev: ssd1307fb: Turn off display on driver unload Thomas Niederprüm
2015-03-16 17:11 ` [PATCHv4 00/10] Cleanup and add support for SSD1305 Thomas Niederprüm
2015-03-16 17:11   ` [PATCHv4 01/10] fbdev: ssd1307fb: fix memory address smem_start Thomas Niederprüm
2015-03-16 17:11   ` [PATCHv4 02/10] fbdev: ssd1307fb: Allocate page aligned video memory Thomas Niederprüm
2015-03-18 17:38     ` Maxime Ripard
2015-03-16 17:11   ` [PATCHv4 03/10] of: Add Solomon Systech vendor prefix Thomas Niederprüm
2015-03-16 17:11   ` [PATCHv4 04/10] fbdev: ssd1307fb: Unify init code and obtain hw specific bits from DT Thomas Niederprüm
2015-03-18 19:27     ` Maxime Ripard
2015-03-20 21:12       ` Thomas Niederprüm
2015-03-24 15:24         ` Maxime Ripard
2015-03-16 17:11   ` [PATCHv4 05/10] ARM: mxs: fix in tree users of ssd1306 Thomas Niederprüm
2015-03-16 17:11   ` [PATCHv4 06/10] fbdev: ssd1307fb: Add support for SSD1305 Thomas Niederprüm
2015-03-16 17:11   ` [PATCHv4 07/10] fbdev: ssd1307fb: Add a module parameter to set the refresh rate Thomas Niederprüm
2015-03-19 13:18     ` Maxime Ripard
2015-03-20 21:16       ` Thomas Niederprüm
2015-03-16 17:11   ` [PATCHv4 08/10] fbdev: ssd1307fb: Turn off display on driver unload Thomas Niederprüm
2015-03-16 17:11   ` [PATCHv4 09/10] fbdev: ssd1307fb: add backlight controls for setting the contrast Thomas Niederprüm
2015-03-19 13:22     ` Maxime Ripard
2015-03-16 17:11   ` [PATCHv4 10/10] fbdev: ssd1307fb: Add blank mode Thomas Niederprüm
2015-03-24 21:23 ` [PATCHv5 00/11] Cleanup and add support for SSD1305 Thomas Niederprüm
2015-03-24 21:23   ` [PATCHv5 01/11] fbdev: ssd1307fb: fix memory address smem_start Thomas Niederprüm
2015-03-24 21:23   ` [PATCHv5 02/11] fbdev: ssd1307fb: Allocate page aligned video memory Thomas Niederprüm
2015-03-24 21:23   ` [PATCHv5 03/11] of: Add Solomon Systech vendor prefix Thomas Niederprüm
2015-03-24 21:23   ` [PATCHv5 04/11] fbdev: ssd1307fb: Unify init code and obtain hw specific bits from DT Thomas Niederprüm
2015-03-24 22:14     ` Maxime Ripard
2015-03-25 20:03       ` Thomas Niederprüm
2015-03-25 15:49     ` Olliver Schinagl
2015-03-25 22:14       ` Thomas Niederprüm
2015-03-24 21:23   ` [PATCHv5 05/11] ARM: mxs: fix in tree users of ssd1306 Thomas Niederprüm
2015-03-24 21:23   ` [PATCHv5 06/11] fbdev: ssd1307fb: Add support for SSD1305 Thomas Niederprüm
2015-03-24 21:23   ` [PATCHv5 07/11] fbdev: ssd1307fb: Add a module parameter to set the refresh rate Thomas Niederprüm
2015-03-24 21:54     ` Maxime Ripard
2015-03-24 21:23   ` [PATCHv5 08/11] fbdev: ssd1307fb: Turn off display on driver unload Thomas Niederprüm
2015-03-24 21:23   ` [PATCHv5 09/11] fbdev: ssd1307fb: Add module parameter to set the initial contrast Thomas Niederprüm
2015-03-24 22:16     ` Maxime Ripard
2015-03-25 20:10       ` Thomas Niederprüm
2015-03-24 21:23   ` [PATCHv5 10/11] fbdev: ssd1307fb: add backlight controls for setting the contrast Thomas Niederprüm
2015-03-24 21:23   ` [PATCHv5 11/11] fbdev: ssd1307fb: Add blank mode Thomas Niederprüm
2015-03-25 15:50     ` Olliver Schinagl
2015-03-25 20:33       ` Thomas Niederprüm
2015-03-25 22:00       ` Maxime Ripard
2015-03-31 18:27 ` [PATCHv6 00/10] Cleanup and add support for SSD1305 Thomas Niederprüm
2015-03-31 18:27   ` [PATCHv6 01/10] fbdev: ssd1307fb: fix memory address smem_start Thomas Niederprüm
2015-03-31 18:27   ` [PATCHv6 02/10] fbdev: ssd1307fb: Allocate page aligned video memory Thomas Niederprüm
2015-03-31 18:27   ` [PATCHv6 03/10] of: Add Solomon Systech vendor prefix Thomas Niederprüm
2015-04-01 20:33     ` Rob Herring
2015-03-31 18:27   ` [PATCHv6 04/10] fbdev: ssd1307fb: Unify init code and obtain hw specific bits from DT Thomas Niederprüm
2015-04-01 16:00     ` Maxime Ripard
2015-03-31 18:27   ` [PATCHv6 05/10] ARM: mxs: fix in tree users of ssd1306 Thomas Niederprüm
2015-05-07 10:55     ` Tomi Valkeinen
2015-05-07 11:28       ` Shawn Guo
2015-05-08 13:31         ` Maxime Ripard
2015-05-26  7:08           ` Tomi Valkeinen
2015-05-27  3:03             ` Shawn Guo
2015-03-31 18:27   ` [PATCHv6 06/10] fbdev: ssd1307fb: Add support for SSD1305 Thomas Niederprüm
2015-04-01 16:01     ` Maxime Ripard
2015-03-31 18:27   ` [PATCHv6 07/10] fbdev: ssd1307fb: Add a module parameter to set the refresh rate Thomas Niederprüm
2015-03-31 18:27   ` [PATCHv6 08/10] fbdev: ssd1307fb: Turn off display on driver unload Thomas Niederprüm
2015-03-31 18:27   ` [PATCHv6 09/10] fbdev: ssd1307fb: add backlight controls for setting the contrast Thomas Niederprüm
2015-03-31 18:27   ` [PATCHv6 10/10] fbdev: ssd1307fb: Add blank mode Thomas Niederprüm
2015-05-27 10:15   ` [PATCHv6 00/10] Cleanup and add support for SSD1305 Tomi Valkeinen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20150212151121.GJ2079@lukather \
    --to=maxime.ripard@free-electrons.com \
    --cc=linux-fbdev@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=niederp@physik.uni-kl.de \
    --cc=plagnioj@jcrosoft.com \
    --cc=tomi.valkeinen@ti.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).