| Message ID | 1516707296-23667-1-git-send-email-hayashi.kunihiko@socionext.com |
|---|---|
| State | New |
| Headers | show |
| Series | fbdev: simplefb: add support for 'memory-region' property on DT node | expand |
Hi, On 23-01-18 12:34, Kunihiko Hayashi wrote: > Enables 'memory-region' property referring to the memory description on > the reserved-memory node in case of devicetree use. > If there is no 'reg' property that specifies the address and size of > the framebuffer, the address and size written in the memory description > on the reserved-memory node can be used for the framebuffer. > > Furthermore, the reserved-memory node needs to have "no-map" attributes > because simplefb driver maps the region by ioremap_wc(). > > Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com> Seems reasonable to me: Acked-by: Hans de Goede <hdegoede@redhat.com> Regards, Hans > --- > .../bindings/display/simple-framebuffer.txt | 3 ++ > drivers/video/fbdev/simplefb.c | 32 ++++++++++++++++++++++ > 2 files changed, 35 insertions(+) > > diff --git a/Documentation/devicetree/bindings/display/simple-framebuffer.txt b/Documentation/devicetree/bindings/display/simple-framebuffer.txt > index 5a9ce51..be5139f 100644 > --- a/Documentation/devicetree/bindings/display/simple-framebuffer.txt > +++ b/Documentation/devicetree/bindings/display/simple-framebuffer.txt > @@ -56,6 +56,9 @@ Optional properties: > framebuffer remains active. > > - display : phandle pointing to the primary display hardware node > +- memory-region: phandle to a node describing memory region as framebuffer > + memory instead of reg property. The node should include > + 'no-map'. > > Example: > > diff --git a/drivers/video/fbdev/simplefb.c b/drivers/video/fbdev/simplefb.c > index a3c44ec..aefc4b1 100644 > --- a/drivers/video/fbdev/simplefb.c > +++ b/drivers/video/fbdev/simplefb.c > @@ -29,6 +29,7 @@ > #include <linux/clk.h> > #include <linux/clk-provider.h> > #include <linux/of.h> > +#include <linux/of_address.h> > #include <linux/of_platform.h> > #include <linux/parser.h> > #include <linux/regulator/consumer.h> > @@ -294,6 +295,35 @@ static void simplefb_clocks_enable(struct simplefb_par *par, > static void simplefb_clocks_destroy(struct simplefb_par *par) { } > #endif > > +#if defined CONFIG_OF > +static struct resource *simplefb_parse_dt_reserved_mem(struct device *dev) > +{ > + static struct resource res; > + struct device_node *np; > + int ret; > + > + np = of_parse_phandle(dev->of_node, "memory-region", 0); > + if (!np) > + return NULL; > + > + ret = of_address_to_resource(np, 0, &res); > + if (ret < 0) > + return NULL; > + > + if (!of_find_property(np, "no-map", NULL)) { > + dev_err(dev, "Can't apply mapped reserved-memory\n"); > + return NULL; > + } > + > + return &res; > +} > +#else > +static struct resource *simplefb_parse_dt_reserved_mem(struct device *dev) > +{ > + return NULL; > +} > +#endif > + > #if defined CONFIG_OF && defined CONFIG_REGULATOR > > #define SUPPLY_SUFFIX "-supply" > @@ -428,6 +458,8 @@ static int simplefb_probe(struct platform_device *pdev) > return ret; > > mem = platform_get_resource(pdev, IORESOURCE_MEM, 0); > + if (!mem) > + mem = simplefb_parse_dt_reserved_mem(&pdev->dev); > if (!mem) { > dev_err(&pdev->dev, "No memory resource\n"); > return -EINVAL; > -- 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
On Thu, Feb 1, 2018 at 5:56 PM, Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com> wrote: > On Tuesday, January 23, 2018 08:34:56 PM Kunihiko Hayashi wrote: >> Enables 'memory-region' property referring to the memory description on >> the reserved-memory node in case of devicetree use. >> If there is no 'reg' property that specifies the address and size of >> the framebuffer, the address and size written in the memory description >> on the reserved-memory node can be used for the framebuffer. >> >> Furthermore, the reserved-memory node needs to have "no-map" attributes >> because simplefb driver maps the region by ioremap_wc(). >> >> Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com> >> +- memory-region: phandle to a node describing memory region as framebuffer >> + memory instead of reg property. The node should include >> + 'no-map'. >> mem = platform_get_resource(pdev, IORESOURCE_MEM, 0); >> + if (!mem) >> + mem = simplefb_parse_dt_reserved_mem(&pdev->dev); I'm not sure I understood why you need this entire function? Put your memory resource ('reg' property) as part of reserved memory with necessary flags. -- With Best Regards, Andy Shevchenko -- 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
Hi Bartlomiej, Rob, Mark, On Thu, 1 Feb 2018 16:56:08 +0100 <b.zolnierkie@samsung.com> wrote: > > Hi, > > On Tuesday, January 23, 2018 08:34:56 PM Kunihiko Hayashi wrote: > > Enables 'memory-region' property referring to the memory description on > > the reserved-memory node in case of devicetree use. > > If there is no 'reg' property that specifies the address and size of > > the framebuffer, the address and size written in the memory description > > on the reserved-memory node can be used for the framebuffer. > > > > Furthermore, the reserved-memory node needs to have "no-map" attributes > > because simplefb driver maps the region by ioremap_wc(). > > > > Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com> > > This needs an ACK from Rob or Mark (DT bindings Maintainers). Thanks for pointing out. Rob, Mark, would you please confirm the patch? This patch contains the addition to dt-bindings. --- Best Regards, Kunihiko Hayashi -- 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
Hi Andy, On Thu, 1 Feb 2018 21:03:30 +0200 <andy.shevchenko@gmail.com> wrote: > On Thu, Feb 1, 2018 at 5:56 PM, Bartlomiej Zolnierkiewicz > <b.zolnierkie@samsung.com> wrote: > > On Tuesday, January 23, 2018 08:34:56 PM Kunihiko Hayashi wrote: > >> Enables 'memory-region' property referring to the memory description on > >> the reserved-memory node in case of devicetree use. > >> If there is no 'reg' property that specifies the address and size of > >> the framebuffer, the address and size written in the memory description > >> on the reserved-memory node can be used for the framebuffer. > >> > >> Furthermore, the reserved-memory node needs to have "no-map" attributes > >> because simplefb driver maps the region by ioremap_wc(). > >> > >> Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com> > > >> +- memory-region: phandle to a node describing memory region as framebuffer > >> + memory instead of reg property. The node should include > >> + 'no-map'. > > >> mem = platform_get_resource(pdev, IORESOURCE_MEM, 0); > >> + if (!mem) > >> + mem = simplefb_parse_dt_reserved_mem(&pdev->dev); > > I'm not sure I understood why you need this entire function? > > Put your memory resource ('reg' property) as part of reserved memory > with necessary flags. Surely we prepare a memory resource as a part of reserved memory, for example: reserved-memory { fb_area: memory@0xa0000000 { reg = <0xa0000000 0x400000>; no-map; }; }; And we need to specify the address and size as a reg property in the framebuffer node. framebuffer { compatible = "simple-framebuffer"; reg = <0xa0000000 0x400000>; }; This function allows us to specify the area with phandle to the reserved memory instead of same address and size. framebuffer { compatible = "simple-framebuffer"; memory-region = <&fb_area>; }; If both reg and memory-region properties are specified in the framebuffer node, the reg propery will be applied. --- Best Regards, Kunihiko Hayashi -- 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
On Thu, Feb 01, 2018 at 04:56:08PM +0100, Bartlomiej Zolnierkiewicz wrote: > > Hi, > > On Tuesday, January 23, 2018 08:34:56 PM Kunihiko Hayashi wrote: > > Enables 'memory-region' property referring to the memory description on > > the reserved-memory node in case of devicetree use. > > If there is no 'reg' property that specifies the address and size of > > the framebuffer, the address and size written in the memory description > > on the reserved-memory node can be used for the framebuffer. > > > > Furthermore, the reserved-memory node needs to have "no-map" attributes > > because simplefb driver maps the region by ioremap_wc(). > > > > Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com> > > This needs an ACK from Rob or Mark (DT bindings Maintainers). > > > --- > > .../bindings/display/simple-framebuffer.txt | 3 ++ > > drivers/video/fbdev/simplefb.c | 32 ++++++++++++++++++++++ > > 2 files changed, 35 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/display/simple-framebuffer.txt b/Documentation/devicetree/bindings/display/simple-framebuffer.txt > > index 5a9ce51..be5139f 100644 > > --- a/Documentation/devicetree/bindings/display/simple-framebuffer.txt > > +++ b/Documentation/devicetree/bindings/display/simple-framebuffer.txt > > @@ -56,6 +56,9 @@ Optional properties: > > framebuffer remains active. > > > > - display : phandle pointing to the primary display hardware node > > +- memory-region: phandle to a node describing memory region as framebuffer > > + memory instead of reg property. The node should include > > + 'no-map'. This should also state when it's appropriate to use this instead of reg. The memory would only be reclaimed if reg is used. Though I'm wondering what keeps the simple fb memory from getting used by the OS if reserved memory is not always used. Rob -- 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
Hi Rob, On Mon, 5 Feb 2018 00:09:14 -0600 <robh@kernel.org> wrote: > On Thu, Feb 01, 2018 at 04:56:08PM +0100, Bartlomiej Zolnierkiewicz wrote: > > > > Hi, > > > > On Tuesday, January 23, 2018 08:34:56 PM Kunihiko Hayashi wrote: > > > Enables 'memory-region' property referring to the memory description on > > > the reserved-memory node in case of devicetree use. > > > If there is no 'reg' property that specifies the address and size of > > > the framebuffer, the address and size written in the memory description > > > on the reserved-memory node can be used for the framebuffer. > > > > > > Furthermore, the reserved-memory node needs to have "no-map" attributes > > > because simplefb driver maps the region by ioremap_wc(). > > > > > > Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com> > > > > This needs an ACK from Rob or Mark (DT bindings Maintainers). > > > > > --- > > > .../bindings/display/simple-framebuffer.txt | 3 ++ > > > drivers/video/fbdev/simplefb.c | 32 ++++++++++++++++++++++ > > > 2 files changed, 35 insertions(+) > > > > > > diff --git a/Documentation/devicetree/bindings/display/simple-framebuffer.txt b/Documentation/devicetree/bindings/display/simple-framebuffer.txt > > > index 5a9ce51..be5139f 100644 > > > --- a/Documentation/devicetree/bindings/display/simple-framebuffer.txt > > > +++ b/Documentation/devicetree/bindings/display/simple-framebuffer.txt > > > @@ -56,6 +56,9 @@ Optional properties: > > > framebuffer remains active. > > > > > > - display : phandle pointing to the primary display hardware node > > > +- memory-region: phandle to a node describing memory region as framebuffer > > > + memory instead of reg property. The node should include > > > + 'no-map'. > > This should also state when it's appropriate to use this instead of reg. > The memory would only be reclaimed if reg is used. > > Though I'm wondering what keeps the simple fb memory from getting used > by the OS if reserved memory is not always used. Sorry that my understanding might not be correct. Since a framebuffer defined by "reserved-memory" can't be reclaimed, I've assumed such the framebuffer was dedicated and had some restrictions by device or firmware. Even in that case, You mean that we describe only "reg" directly, not go through "reserved-memory". Is that correct? It might be better to define new driver handling the dedicated framebuffer rather than using simple-fb, though, I'm not sure. --- Best Regards, Kunihiko Hayashi -- 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 --git a/Documentation/devicetree/bindings/display/simple-framebuffer.txt b/Documentation/devicetree/bindings/display/simple-framebuffer.txt index 5a9ce51..be5139f 100644 --- a/Documentation/devicetree/bindings/display/simple-framebuffer.txt +++ b/Documentation/devicetree/bindings/display/simple-framebuffer.txt @@ -56,6 +56,9 @@ Optional properties: framebuffer remains active. - display : phandle pointing to the primary display hardware node +- memory-region: phandle to a node describing memory region as framebuffer + memory instead of reg property. The node should include + 'no-map'. Example: diff --git a/drivers/video/fbdev/simplefb.c b/drivers/video/fbdev/simplefb.c index a3c44ec..aefc4b1 100644 --- a/drivers/video/fbdev/simplefb.c +++ b/drivers/video/fbdev/simplefb.c @@ -29,6 +29,7 @@ #include <linux/clk.h> #include <linux/clk-provider.h> #include <linux/of.h> +#include <linux/of_address.h> #include <linux/of_platform.h> #include <linux/parser.h> #include <linux/regulator/consumer.h> @@ -294,6 +295,35 @@ static void simplefb_clocks_enable(struct simplefb_par *par, static void simplefb_clocks_destroy(struct simplefb_par *par) { } #endif +#if defined CONFIG_OF +static struct resource *simplefb_parse_dt_reserved_mem(struct device *dev) +{ + static struct resource res; + struct device_node *np; + int ret; + + np = of_parse_phandle(dev->of_node, "memory-region", 0); + if (!np) + return NULL; + + ret = of_address_to_resource(np, 0, &res); + if (ret < 0) + return NULL; + + if (!of_find_property(np, "no-map", NULL)) { + dev_err(dev, "Can't apply mapped reserved-memory\n"); + return NULL; + } + + return &res; +} +#else +static struct resource *simplefb_parse_dt_reserved_mem(struct device *dev) +{ + return NULL; +} +#endif + #if defined CONFIG_OF && defined CONFIG_REGULATOR #define SUPPLY_SUFFIX "-supply" @@ -428,6 +458,8 @@ static int simplefb_probe(struct platform_device *pdev) return ret; mem = platform_get_resource(pdev, IORESOURCE_MEM, 0); + if (!mem) + mem = simplefb_parse_dt_reserved_mem(&pdev->dev); if (!mem) { dev_err(&pdev->dev, "No memory resource\n"); return -EINVAL;
Enables 'memory-region' property referring to the memory description on the reserved-memory node in case of devicetree use. If there is no 'reg' property that specifies the address and size of the framebuffer, the address and size written in the memory description on the reserved-memory node can be used for the framebuffer. Furthermore, the reserved-memory node needs to have "no-map" attributes because simplefb driver maps the region by ioremap_wc(). Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com> --- .../bindings/display/simple-framebuffer.txt | 3 ++ drivers/video/fbdev/simplefb.c | 32 ++++++++++++++++++++++ 2 files changed, 35 insertions(+) -- 2.7.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