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