diff mbox

[BUG] simplefb not showing any output

Message ID CANq1E4RAG+sJWbMShs5te89NAEQ6d=JRRry5NxG690VJTc4zmQ@mail.gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

David Herrmann Sept. 7, 2013, 12:25 p.m. UTC
Hi Tom

On Sat, Sep 7, 2013 at 1:47 PM, Tom Gundersen <teg@jklm.no> wrote:
> On Sat, Sep 7, 2013 at 1:18 PM, David Herrmann <dh.herrmann@gmail.com> wrote:
>> On Sat, Sep 7, 2013 at 11:26 AM, Tom Gundersen <teg@jklm.no> wrote:
>>> On Fri, Sep 6, 2013 at 6:11 PM, Tom Gundersen <teg@jklm.no> wrote:
>>>> The driver seems to load ok, but sadly it does not give any output. If
>>>> I boot via the gummiboot menu the screen remains black, and if I don't
>>>> enter the gummiboot menu the screen remains grey (it is a Mac).
>>>
>>> My apologies, this was a config error (I somehow ended up with
>>> FRAMEBUFFER_CONSOLE=m, which works fine with inteldrmfb, but obviously
>>> not with any of the compiled in ones).
>>
>> Good, I almost went crazy looking for an error. Does that mean
>> simplefb works for you?
>
> Sorry about that! Yes, it now works.

No worries. Going crazy is part of working on the kernel, I guess.

>> I guess the x86-sysfb patch is still needed?
>
> Yes, that is still needed.

Ok, I will keep watching it then.

>> Now I only wonder why the i915 issue showed up. Is it also solved with
>> a built-in fbcon?
>
> Sadly, no, that problem persists.

I will let Daniel know on IRC.

> A related question: is it expected that simplefb should be
> significantly slower than efifb, or is that something worth looking
> into? My boot with simplefb is roughly five seconds slower than with
> efifb. Coincidentally, I notice the same (or similar slowdown) with
> inteldrmfb when I see the oops (but not otherwise).

That is probably related to the missing write-combine tag in ioremap.
Stephen, any objections to this attached patch?
Tom, if this solves the speed-issues, I will send it out once I get home.

Thanks
David

(Patch also attached in case of new-lines issues)

From dbfb8e12166d494cd60823cbe84134d5d1a73ec8 Mon Sep 17 00:00:00 2001
From: David Herrmann <dh.herrmann@gmail.com>
Date: Sat, 7 Sep 2013 14:22:01 +0200
Subject: [PATCH] devm/simplefb: introduce and use devm_ioremap_wc()

We want to use devm_ioremap_nocache() or even devm_ioremap_wc() to speed
up fbdev writes _a lot_. As devm_ioremap_wc() doesn't exist, yet,
introduce it along the way. Note that ioremap_wc() is aliases to
ioremap_nocache() in asm-generic/{io,iomem}.h so we can safely expect all
architectures to either provide it or use the same alias.

Reported-by: Tom Gundersen <teg@jklm.no>
Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
Cc: Stephen Warren <swarren@wwwdotorg.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
---
 drivers/video/simplefb.c |  4 ++--
 include/linux/io.h       |  2 ++
 lib/devres.c             | 28 ++++++++++++++++++++++++++++
 3 files changed, 32 insertions(+), 2 deletions(-)

  * @addr: Address to unmap

Comments

Tom Gundersen Sept. 7, 2013, 1:02 p.m. UTC | #1
On Sat, Sep 7, 2013 at 2:25 PM, David Herrmann <dh.herrmann@gmail.com> wrote:
> On Sat, Sep 7, 2013 at 1:47 PM, Tom Gundersen <teg@jklm.no> wrote:
>> A related question: is it expected that simplefb should be
>> significantly slower than efifb, or is that something worth looking
>> into? My boot with simplefb is roughly five seconds slower than with
>> efifb. Coincidentally, I notice the same (or similar slowdown) with
>> inteldrmfb when I see the oops (but not otherwise).
>
> That is probably related to the missing write-combine tag in ioremap.
> Stephen, any objections to this attached patch?
> Tom, if this solves the speed-issues, I will send it out once I get home.

This solves the speed issues. Thanks!

Feel free to add my tested-by.

Cheers,

Tom

> From dbfb8e12166d494cd60823cbe84134d5d1a73ec8 Mon Sep 17 00:00:00 2001
> From: David Herrmann <dh.herrmann@gmail.com>
> Date: Sat, 7 Sep 2013 14:22:01 +0200
> Subject: [PATCH] devm/simplefb: introduce and use devm_ioremap_wc()
>
> We want to use devm_ioremap_nocache() or even devm_ioremap_wc() to speed
> up fbdev writes _a lot_. As devm_ioremap_wc() doesn't exist, yet,
> introduce it along the way. Note that ioremap_wc() is aliases to
> ioremap_nocache() in asm-generic/{io,iomem}.h so we can safely expect all
> architectures to either provide it or use the same alias.
>
> Reported-by: Tom Gundersen <teg@jklm.no>
> Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
> Cc: Stephen Warren <swarren@wwwdotorg.org>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> ---
>  drivers/video/simplefb.c |  4 ++--
>  include/linux/io.h       |  2 ++
>  lib/devres.c             | 28 ++++++++++++++++++++++++++++
>  3 files changed, 32 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/video/simplefb.c b/drivers/video/simplefb.c
> index 8d78106..a29f1c4 100644
> --- a/drivers/video/simplefb.c
> +++ b/drivers/video/simplefb.c
> @@ -212,8 +212,8 @@ static int simplefb_probe(struct platform_device *pdev)
>
>   info->fbops = &simplefb_ops;
>   info->flags = FBINFO_DEFAULT | FBINFO_MISC_FIRMWARE;
> - info->screen_base = devm_ioremap(&pdev->dev, info->fix.smem_start,
> - info->fix.smem_len);
> + info->screen_base = devm_ioremap_wc(&pdev->dev, info->fix.smem_start,
> +    info->fix.smem_len);
>   if (!info->screen_base) {
>   framebuffer_release(info);
>   return -ENODEV;
> diff --git a/include/linux/io.h b/include/linux/io.h
> index f4f42fa..c529410 100644
> --- a/include/linux/io.h
> +++ b/include/linux/io.h
> @@ -62,6 +62,8 @@ void __iomem *devm_ioremap(struct device *dev,
> resource_size_t offset,
>      unsigned long size);
>  void __iomem *devm_ioremap_nocache(struct device *dev, resource_size_t offset,
>      unsigned long size);
> +void __iomem *devm_ioremap_wc(struct device *dev, resource_size_t offset,
> +      unsigned long size);
>  void devm_iounmap(struct device *dev, void __iomem *addr);
>  int check_signature(const volatile void __iomem *io_addr,
>   const unsigned char *signature, int length);
> diff --git a/lib/devres.c b/lib/devres.c
> index 8235331..34af7a9 100644
> --- a/lib/devres.c
> +++ b/lib/devres.c
> @@ -72,6 +72,34 @@ void __iomem *devm_ioremap_nocache(struct device
> *dev, resource_size_t offset,
>  EXPORT_SYMBOL(devm_ioremap_nocache);
>
>  /**
> + * devm_ioremap_wc - Managed ioremap_wc()
> + * @dev: Generic device to remap IO address for
> + * @offset: BUS offset to map
> + * @size: Size of map
> + *
> + * Managed ioremap_wc().  Map is automatically unmapped on driver detach.
> + */
> +void __iomem *devm_ioremap_wc(struct device *dev, resource_size_t offset,
> +      unsigned long size)
> +{
> + void __iomem **ptr, *addr;
> +
> + ptr = devres_alloc(devm_ioremap_release, sizeof(*ptr), GFP_KERNEL);
> + if (!ptr)
> + return NULL;
> +
> + addr = ioremap_wc(offset, size);
> + if (addr) {
> + *ptr = addr;
> + devres_add(dev, ptr);
> + } else
> + devres_free(ptr);
> +
> + return addr;
> +}
> +EXPORT_SYMBOL(devm_ioremap_wc);
> +
> +/**
>   * devm_iounmap - Managed iounmap()
>   * @dev: Generic device to unmap for
>   * @addr: Address to unmap
> --
> 1.8.4
--
To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Alexandre Courbot Sept. 9, 2013, 2:36 a.m. UTC | #2
On Sat, Sep 7, 2013 at 9:25 PM, David Herrmann <dh.herrmann@gmail.com> wrote:
> From dbfb8e12166d494cd60823cbe84134d5d1a73ec8 Mon Sep 17 00:00:00 2001
> From: David Herrmann <dh.herrmann@gmail.com>
> Date: Sat, 7 Sep 2013 14:22:01 +0200
> Subject: [PATCH] devm/simplefb: introduce and use devm_ioremap_wc()
>
> We want to use devm_ioremap_nocache() or even devm_ioremap_wc() to speed
> up fbdev writes _a lot_. As devm_ioremap_wc() doesn't exist, yet,
> introduce it along the way. Note that ioremap_wc() is aliases to
> ioremap_nocache() in asm-generic/{io,iomem}.h so we can safely expect all
> architectures to either provide it or use the same alias.
>
> Reported-by: Tom Gundersen <teg@jklm.no>
> Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
> Cc: Stephen Warren <swarren@wwwdotorg.org>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Andrew Morton <akpm@linux-foundation.org>

Tried this on SHIELD, and it dramatically increases performance.

Tested-by: Alexandre Courbot <acourbot@nvidia.com>

Alex.
--
To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Alexandre Courbot Sept. 9, 2013, 2:40 a.m. UTC | #3
On Mon, Sep 9, 2013 at 11:36 AM, Alexandre Courbot <gnurou@gmail.com> wrote:
> On Sat, Sep 7, 2013 at 9:25 PM, David Herrmann <dh.herrmann@gmail.com> wrote:
>> From dbfb8e12166d494cd60823cbe84134d5d1a73ec8 Mon Sep 17 00:00:00 2001
>> From: David Herrmann <dh.herrmann@gmail.com>
>> Date: Sat, 7 Sep 2013 14:22:01 +0200
>> Subject: [PATCH] devm/simplefb: introduce and use devm_ioremap_wc()
>>
>> We want to use devm_ioremap_nocache() or even devm_ioremap_wc() to speed
>> up fbdev writes _a lot_. As devm_ioremap_wc() doesn't exist, yet,
>> introduce it along the way. Note that ioremap_wc() is aliases to
>> ioremap_nocache() in asm-generic/{io,iomem}.h so we can safely expect all
>> architectures to either provide it or use the same alias.
>>
>> Reported-by: Tom Gundersen <teg@jklm.no>
>> Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
>> Cc: Stephen Warren <swarren@wwwdotorg.org>
>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>> Cc: Andrew Morton <akpm@linux-foundation.org>
>
> Tried this on SHIELD, and it dramatically increases performance.
>
> Tested-by: Alexandre Courbot <acourbot@nvidia.com>

Maybe the patch should be split into two though, one that adds
devm_ioremap_wc() and another that takes advantage of it for simplefb.

Alex.
--
To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Herrmann Sept. 9, 2013, 9:46 a.m. UTC | #4
Hi

On Mon, Sep 9, 2013 at 4:40 AM, Alexandre Courbot <gnurou@gmail.com> wrote:
> On Mon, Sep 9, 2013 at 11:36 AM, Alexandre Courbot <gnurou@gmail.com> wrote:
>> On Sat, Sep 7, 2013 at 9:25 PM, David Herrmann <dh.herrmann@gmail.com> wrote:
>>> From dbfb8e12166d494cd60823cbe84134d5d1a73ec8 Mon Sep 17 00:00:00 2001
>>> From: David Herrmann <dh.herrmann@gmail.com>
>>> Date: Sat, 7 Sep 2013 14:22:01 +0200
>>> Subject: [PATCH] devm/simplefb: introduce and use devm_ioremap_wc()
>>>
>>> We want to use devm_ioremap_nocache() or even devm_ioremap_wc() to speed
>>> up fbdev writes _a lot_. As devm_ioremap_wc() doesn't exist, yet,
>>> introduce it along the way. Note that ioremap_wc() is aliases to
>>> ioremap_nocache() in asm-generic/{io,iomem}.h so we can safely expect all
>>> architectures to either provide it or use the same alias.
>>>
>>> Reported-by: Tom Gundersen <teg@jklm.no>
>>> Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
>>> Cc: Stephen Warren <swarren@wwwdotorg.org>
>>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>>> Cc: Andrew Morton <akpm@linux-foundation.org>
>>
>> Tried this on SHIELD, and it dramatically increases performance.
>>
>> Tested-by: Alexandre Courbot <acourbot@nvidia.com>
>
> Maybe the patch should be split into two though, one that adds
> devm_ioremap_wc() and another that takes advantage of it for simplefb.

Thanks for testing! I added your tested-by. I also changed the patches
slightly to no longer use devm_*. The *_wc patch is now split off,
too. I will send it later today.

Thanks
David
--
To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Stephen Warren Sept. 9, 2013, 3:47 p.m. UTC | #5
On 09/07/2013 06:25 AM, David Herrmann wrote:
> On Sat, Sep 7, 2013 at 1:47 PM, Tom Gundersen <teg@jklm.no> wrote:
...
>> A related question: is it expected that simplefb should be
>> significantly slower than efifb, or is that something worth looking
>> into? My boot with simplefb is roughly five seconds slower than with
>> efifb. Coincidentally, I notice the same (or similar slowdown) with
>> inteldrmfb when I see the oops (but not otherwise).
> 
> That is probably related to the missing write-combine tag in ioremap.
> Stephen, any objections to this attached patch?
> Tom, if this solves the speed-issues, I will send it out once I get home.
> 
> Thanks
> David
> 
> (Patch also attached in case of new-lines issues)
> 
> From dbfb8e12166d494cd60823cbe84134d5d1a73ec8 Mon Sep 17 00:00:00 2001
> From: David Herrmann <dh.herrmann@gmail.com>
> Date: Sat, 7 Sep 2013 14:22:01 +0200
> Subject: [PATCH] devm/simplefb: introduce and use devm_ioremap_wc()
> 
> We want to use devm_ioremap_nocache() or even devm_ioremap_wc() to speed
> up fbdev writes _a lot_. As devm_ioremap_wc() doesn't exist, yet,
> introduce it along the way. Note that ioremap_wc() is aliases to
> ioremap_nocache() in asm-generic/{io,iomem}.h so we can safely expect all
> architectures to either provide it or use the same alias.

I'm fine with this so long as wc mappings are always possible, or
automatically fall back to uc if not. This certainly works on Tegra, and
I'll try to remember to test it on Raspberry Pi tonight.
--
To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Stephen Warren Sept. 10, 2013, 2:34 a.m. UTC | #6
On 09/07/2013 06:25 AM, David Herrmann wrote:
> On Sat, Sep 7, 2013 at 1:47 PM, Tom Gundersen <teg@jklm.no> wrote:
...
>> A related question: is it expected that simplefb should be
>> significantly slower than efifb, or is that something worth looking
>> into? My boot with simplefb is roughly five seconds slower than with
>> efifb. Coincidentally, I notice the same (or similar slowdown) with
>> inteldrmfb when I see the oops (but not otherwise).
> 
> That is probably related to the missing write-combine tag in ioremap.
> Stephen, any objections to this attached patch?
> Tom, if this solves the speed-issues, I will send it out once I get home.
> 
> Thanks
> David
> 
> (Patch also attached in case of new-lines issues)
> 
> From dbfb8e12166d494cd60823cbe84134d5d1a73ec8 Mon Sep 17 00:00:00 2001
> From: David Herrmann <dh.herrmann@gmail.com>
> Date: Sat, 7 Sep 2013 14:22:01 +0200
> Subject: [PATCH] devm/simplefb: introduce and use devm_ioremap_wc()
> 
> We want to use devm_ioremap_nocache() or even devm_ioremap_wc() to speed
> up fbdev writes _a lot_. As devm_ioremap_wc() doesn't exist, yet,
> introduce it along the way. Note that ioremap_wc() is aliases to
> ioremap_nocache() in asm-generic/{io,iomem}.h so we can safely expect all
> architectures to either provide it or use the same alias.

OK, that works fine on the Raspberry Pi too, so,
Tested-by: Stephen Warren <swarren@wwwdotorg.org>

--
To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Herrmann Oct. 2, 2013, 3:01 p.m. UTC | #7
Hi

On Tue, Sep 10, 2013 at 4:34 AM, Stephen Warren <swarren@wwwdotorg.org> wrote:
> On 09/07/2013 06:25 AM, David Herrmann wrote:
>> On Sat, Sep 7, 2013 at 1:47 PM, Tom Gundersen <teg@jklm.no> wrote:
> ...
>>> A related question: is it expected that simplefb should be
>>> significantly slower than efifb, or is that something worth looking
>>> into? My boot with simplefb is roughly five seconds slower than with
>>> efifb. Coincidentally, I notice the same (or similar slowdown) with
>>> inteldrmfb when I see the oops (but not otherwise).
>>
>> That is probably related to the missing write-combine tag in ioremap.
>> Stephen, any objections to this attached patch?
>> Tom, if this solves the speed-issues, I will send it out once I get home.
>>
>> Thanks
>> David
>>
>> (Patch also attached in case of new-lines issues)
>>
>> From dbfb8e12166d494cd60823cbe84134d5d1a73ec8 Mon Sep 17 00:00:00 2001
>> From: David Herrmann <dh.herrmann@gmail.com>
>> Date: Sat, 7 Sep 2013 14:22:01 +0200
>> Subject: [PATCH] devm/simplefb: introduce and use devm_ioremap_wc()
>>
>> We want to use devm_ioremap_nocache() or even devm_ioremap_wc() to speed
>> up fbdev writes _a lot_. As devm_ioremap_wc() doesn't exist, yet,
>> introduce it along the way. Note that ioremap_wc() is aliases to
>> ioremap_nocache() in asm-generic/{io,iomem}.h so we can safely expect all
>> architectures to either provide it or use the same alias.
>
> OK, that works fine on the Raspberry Pi too, so,
> Tested-by: Stephen Warren <swarren@wwwdotorg.org>

Back from the US and all patches sent out. Sorry for the delay. If I
missed something, please let me know.

Thanks for your feedback!
David
--
To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/video/simplefb.c b/drivers/video/simplefb.c
index 8d78106..a29f1c4 100644
--- a/drivers/video/simplefb.c
+++ b/drivers/video/simplefb.c
@@ -212,8 +212,8 @@  static int simplefb_probe(struct platform_device *pdev)

  info->fbops = &simplefb_ops;
  info->flags = FBINFO_DEFAULT | FBINFO_MISC_FIRMWARE;
- info->screen_base = devm_ioremap(&pdev->dev, info->fix.smem_start,
- info->fix.smem_len);
+ info->screen_base = devm_ioremap_wc(&pdev->dev, info->fix.smem_start,
+    info->fix.smem_len);
  if (!info->screen_base) {
  framebuffer_release(info);
  return -ENODEV;
diff --git a/include/linux/io.h b/include/linux/io.h
index f4f42fa..c529410 100644
--- a/include/linux/io.h
+++ b/include/linux/io.h
@@ -62,6 +62,8 @@  void __iomem *devm_ioremap(struct device *dev,
resource_size_t offset,
     unsigned long size);
 void __iomem *devm_ioremap_nocache(struct device *dev, resource_size_t offset,
     unsigned long size);
+void __iomem *devm_ioremap_wc(struct device *dev, resource_size_t offset,
+      unsigned long size);
 void devm_iounmap(struct device *dev, void __iomem *addr);
 int check_signature(const volatile void __iomem *io_addr,
  const unsigned char *signature, int length);
diff --git a/lib/devres.c b/lib/devres.c
index 8235331..34af7a9 100644
--- a/lib/devres.c
+++ b/lib/devres.c
@@ -72,6 +72,34 @@  void __iomem *devm_ioremap_nocache(struct device
*dev, resource_size_t offset,
 EXPORT_SYMBOL(devm_ioremap_nocache);

 /**
+ * devm_ioremap_wc - Managed ioremap_wc()
+ * @dev: Generic device to remap IO address for
+ * @offset: BUS offset to map
+ * @size: Size of map
+ *
+ * Managed ioremap_wc().  Map is automatically unmapped on driver detach.
+ */
+void __iomem *devm_ioremap_wc(struct device *dev, resource_size_t offset,
+      unsigned long size)
+{
+ void __iomem **ptr, *addr;
+
+ ptr = devres_alloc(devm_ioremap_release, sizeof(*ptr), GFP_KERNEL);
+ if (!ptr)
+ return NULL;
+
+ addr = ioremap_wc(offset, size);
+ if (addr) {
+ *ptr = addr;
+ devres_add(dev, ptr);
+ } else
+ devres_free(ptr);
+
+ return addr;
+}
+EXPORT_SYMBOL(devm_ioremap_wc);
+
+/**
  * devm_iounmap - Managed iounmap()
  * @dev: Generic device to unmap for