Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1333283 > unrolled thread

Re: [PATCH] ARM: omapfb: Add early framebuffer memory allocator

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH] ARM: omapfb: Add early framebuffer memory allocator Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com> - 2016-02-13 08:30 +0100
    Re: [PATCH] ARM: omapfb: Add early framebuffer memory allocator Tomi Valkeinen <tomi.valkeinen@ti.com> - 2016-02-16 15:00 +0100
      Re: [PATCH] ARM: omapfb: Add early framebuffer memory allocator Pali Rohár <pali.rohar@gmail.com> - 2016-02-16 15:10 +0100
      Re: [PATCH] ARM: omapfb: Add early framebuffer memory allocator Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com> - 2016-02-17 08:40 +0100

#1333283 — Re: [PATCH] ARM: omapfb: Add early framebuffer memory allocator

Hi Tomi,

On 11.01.2016 20:34, Tomi Valkeinen wrote:
>
> So, I'm not very enthusiastic about adding this feature as an omapfb
> specific boot parameter.
>

What about something like (not properly formatted, just want your 
opinion on the idea):

diff --git a/arch/arm/mach-omap2/fb.c b/arch/arm/mach-omap2/fb.c
index 1f1ecf8..0d109d8 100644
--- a/arch/arm/mach-omap2/fb.c
+++ b/arch/arm/mach-omap2/fb.c
@@ -28,6 +28,7 @@
  #include <linux/io.h>
  #include <linux/omapfb.h>
  #include <linux/dma-mapping.h>
+#include <linux/of_reserved_mem.h>

  #include <asm/mach/map.h>

@@ -110,6 +111,49 @@ int __init omap_init_fb(void)
  {
         return platform_device_register(&omap_fb_device);
  }
+
+static int rmem_omapfb_device_init(struct reserved_mem *rmem, struct 
device *dev)
+{
+       int dma;
+
+       if (rmem->priv)
+               return 0;
+
+       dma = dma_declare_coherent_memory(&omap_fb_device.dev, rmem->base,
+                                         rmem->base, rmem->size,
+                                         DMA_MEMORY_MAP |
+                                         DMA_MEMORY_EXCLUSIVE);
+
+       if (!(dma & DMA_MEMORY_MAP)) {
+               pr_err("omapfb: dma_declare_coherent_memory failed\n");
+               return -ENOMEM;
+       }
+       else
+               rmem->priv = omap_fb_device.dev.dma_mem;
+
+       return 0;
+}
+
+static void rmem_omapfb_device_release(struct reserved_mem *rmem,
+                                      struct device *dev)
+{
+       dma_release_declared_memory(&omap_fb_device.dev);
+}
+
+static const struct reserved_mem_ops rmem_omapfb_ops = {
+       .device_init    = rmem_omapfb_device_init,
+       .device_release = rmem_omapfb_device_release,
+};
+
+static int __init rmem_omapfb_setup(struct reserved_mem *rmem)
+{
+       rmem->ops = &rmem_omapfb_ops;
+       pr_info("omapfb: reserved %d bytes at %pa\n", rmem->size, 
&rmem->base);
+
+       return 0;
+}
+
+RESERVEDMEM_OF_DECLARE(dss, "ti,omapfb-memsize", rmem_omapfb_setup);
  #else
  int __init omap_init_fb(void) { return 0; }
  #endif

diff --git a/arch/arm/mach-omap2/display.c b/arch/arm/mach-omap2/display.c
index 6ab13d1..6f0ba03 100644
--- a/arch/arm/mach-omap2/display.c
+++ b/arch/arm/mach-omap2/display.c
@@ -28,6 +28,7 @@
  #include <linux/slab.h>
  #include <linux/mfd/syscon.h>
  #include <linux/regmap.h>
+#include <linux/of_reserved_mem.h>

  #include <video/omapdss.h>
  #include "omap_hwmod.h"
@@ -640,6 +641,7 @@ int __init omapdss_init_of(void)
         omap_display_device.dev.platform_data = &board_data;

         r = platform_device_register(&omap_display_device);
+
         if (r < 0) {
                 pr_err("Unable to register omapdss device\n");
                 return r;
@@ -666,6 +668,9 @@ int __init omapdss_init_of(void)
                 return r;
         }

+       /* Init fb reserved memory, there may be none so ignore the 
result */
+       of_reserved_mem_device_init(&pdev->dev);
+
         /* create V4L2 display device */
         r = omap_init_vout();
         if (r < 0) {


Regards,
Ivo

[toc] | [next] | [standalone]

#1335403

[Multipart message — attachments visible in raw view] — view raw

On 13/02/16 09:25, Ivaylo Dimitrov wrote:
> Hi Tomi,
> 
> On 11.01.2016 20:34, Tomi Valkeinen wrote:
>>
>> So, I'm not very enthusiastic about adding this feature as an omapfb
>> specific boot parameter.
>>
> 
> What about something like (not properly formatted, just want your
> opinion on the idea):
> 
> diff --git a/arch/arm/mach-omap2/fb.c b/arch/arm/mach-omap2/fb.c
> index 1f1ecf8..0d109d8 100644
> --- a/arch/arm/mach-omap2/fb.c
> +++ b/arch/arm/mach-omap2/fb.c
> @@ -28,6 +28,7 @@
>  #include <linux/io.h>
>  #include <linux/omapfb.h>
>  #include <linux/dma-mapping.h>
> +#include <linux/of_reserved_mem.h>
> 
>  #include <asm/mach/map.h>
> 
> @@ -110,6 +111,49 @@ int __init omap_init_fb(void)
>  {
>         return platform_device_register(&omap_fb_device);
>  }
> +
> +static int rmem_omapfb_device_init(struct reserved_mem *rmem, struct
> device *dev)
> +{
> +       int dma;
> +
> +       if (rmem->priv)
> +               return 0;
> +
> +       dma = dma_declare_coherent_memory(&omap_fb_device.dev, rmem->base,
> +                                         rmem->base, rmem->size,
> +                                         DMA_MEMORY_MAP |
> +                                         DMA_MEMORY_EXCLUSIVE);
> +
> +       if (!(dma & DMA_MEMORY_MAP)) {
> +               pr_err("omapfb: dma_declare_coherent_memory failed\n");
> +               return -ENOMEM;
> +       }
> +       else
> +               rmem->priv = omap_fb_device.dev.dma_mem;
> +
> +       return 0;
> +}
> +
> +static void rmem_omapfb_device_release(struct reserved_mem *rmem,
> +                                      struct device *dev)
> +{
> +       dma_release_declared_memory(&omap_fb_device.dev);
> +}
> +
> +static const struct reserved_mem_ops rmem_omapfb_ops = {
> +       .device_init    = rmem_omapfb_device_init,
> +       .device_release = rmem_omapfb_device_release,
> +};
> +
> +static int __init rmem_omapfb_setup(struct reserved_mem *rmem)
> +{
> +       rmem->ops = &rmem_omapfb_ops;
> +       pr_info("omapfb: reserved %d bytes at %pa\n", rmem->size,
> &rmem->base);
> +
> +       return 0;
> +}
> +
> +RESERVEDMEM_OF_DECLARE(dss, "ti,omapfb-memsize", rmem_omapfb_setup);
>  #else
>  int __init omap_init_fb(void) { return 0; }
>  #endif
> 
> diff --git a/arch/arm/mach-omap2/display.c b/arch/arm/mach-omap2/display.c
> index 6ab13d1..6f0ba03 100644
> --- a/arch/arm/mach-omap2/display.c
> +++ b/arch/arm/mach-omap2/display.c
> @@ -28,6 +28,7 @@
>  #include <linux/slab.h>
>  #include <linux/mfd/syscon.h>
>  #include <linux/regmap.h>
> +#include <linux/of_reserved_mem.h>
> 
>  #include <video/omapdss.h>
>  #include "omap_hwmod.h"
> @@ -640,6 +641,7 @@ int __init omapdss_init_of(void)
>         omap_display_device.dev.platform_data = &board_data;
> 
>         r = platform_device_register(&omap_display_device);
> +
>         if (r < 0) {
>                 pr_err("Unable to register omapdss device\n");
>                 return r;
> @@ -666,6 +668,9 @@ int __init omapdss_init_of(void)
>                 return r;
>         }
> 
> +       /* Init fb reserved memory, there may be none so ignore the
> result */
> +       of_reserved_mem_device_init(&pdev->dev);
> +

Does it work for you? I haven't used DT reserved-memory, do you have an
example .dts change?

Now, having to support DT bindings is not any better than supporting
cmdline options. But with a quick read of reserved-memory.txt I like the
idea. However we should have "reserved memory for display", not for
omapfb, so that the same reserved area could be used by omapdrm too.

Another thing, with v4.5, omapfb has moved into maintenance mode. I
don't want to merge new features there. Are you planning to move to
omapdrm, and if not, why? I'd rather see all this done for omapdrm only.

 Tomi

[toc] | [prev] | [next] | [standalone]

#1335415

On Tuesday 16 February 2016 15:51:34 Tomi Valkeinen wrote:
> Another thing, with v4.5, omapfb has moved into maintenance mode. I
> don't want to merge new features there. Are you planning to move to
> omapdrm, and if not, why? I'd rather see all this done for omapdrm only.

Hi! We are using (patched) Maemo and for OpenGL ES support we need SGX
PVR graphics kernel driver which depends on omapfb :-( Because Maemo
needs OpenGL ES we needs omapfb driver. Kernel part of SGX PVR driver is
open source (GPL) but there is no manpower to port it to omapdrm...

So if we want to use (patched) Maemo (which I really want) we need to
stay on omapfb for now...

And there are no full open source drivers for SGX PVR graphic chip which
would work on TI omap3 Nokia N900 device...

-- 
Pali Rohár
pali.rohar@gmail.com

[toc] | [prev] | [next] | [standalone]

#1336111

Hi,

On 16.02.2016 15:51, Tomi Valkeinen wrote:
>
> Does it work for you? I haven't used DT reserved-memory, do you have an
> example .dts change?
>

Yes, it does work, I tested it on n900:

diff --git a/arch/arm/boot/dts/omap3-n900.dts 
b/arch/arm/boot/dts/omap3-n900.dts
index 1e94237..863d547 100644
--- a/arch/arm/boot/dts/omap3-n900.dts
+++ b/arch/arm/boot/dts/omap3-n900.dts
@@ -59,6 +59,18 @@
                 reg = <0x80000000 0x10000000>; /* 256 MB */
         };

+       reserved-memory {
+               #address-cells = <1>;
+               #size-cells = <1>;
+               ranges;
+
+               omapfb_reserved: omapfb {
+                       size = <0x700000>;
+                       alignment = <0x100000>;
+                       compatible = "ti,omapfb-memsize";
+               };
+       };
+
         gpio_keys {
                 compatible = "gpio-keys";

@@ -1083,6 +1095,8 @@

         vdds_sdi-supply = <&vaux1>;

+       memory-region = <&omapfb_reserved>;
+
         ports {
                 #address-cells = <1>;
                 #size-cells = <0>;

> Now, having to support DT bindings is not any better than supporting
> cmdline options. But with a quick read of reserved-memory.txt I like the
> idea. However we should have "reserved memory for display", not for
> omapfb, so that the same reserved area could be used by omapdrm too.

Sounds reasonable and I don't really care how it is to be called or who 
does the actual reservation, as long as there is some reserved memory we 
can use for omapfb :)

Keep in mind that the changes I did were just a quick-n-dirty hack to 
see if it will work and if you will accept something like that. A better 
approach is maybe to move RESERVEDMEM_OF_DECLARE() and co to display.c 
and pass base and size to whoever needs them (be it omapfb or omapdrm). 
Also, compatible could be called "ti,dss-memsize" or the like, but those 
are cosmetics IMO.

>
> Another thing, with v4.5, omapfb has moved into maintenance mode. I
> don't want to merge new features there. Are you planning to move to
> omapdrm, and if not, why? I'd rather see all this done for omapdrm only.

I don't see a reason to not merge a small change like that in omapfb if 
there is reserved display memory used by omapdrm, but still, I am not 
the maintainer.

Pali already explained the situation with PVR driver we use to boot 
maemo UI. Honestly, I have no idea what it takes to move from omapfb to 
omapdrm. Any hints?

Ivo

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web