* Re: [PATCH] video: vgacon: Don't build on arm64
From: Geert Uytterhoeven @ 2014-01-08 14:34 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1387323421-26126-1-git-send-email-broonie@kernel.org>
On Wed, Jan 8, 2014 at 2:48 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Wednesday 08 January 2014 15:43:20 Tomi Valkeinen wrote:
>> On 2013-12-18 01:37, Mark Brown wrote:
>> > From: Mark Brown <broonie@linaro.org>
>> >
>> > arm64 is unlikely to have a VGA console and does not export screen_info
>> > causing build failures if the driver is build, for example in all*config.
>> > Add a dependency on !ARM64 to prevent this.
>> >
>> > This list is getting quite long, it may be easier to depend on a symbol
>> > which architectures that do support the driver can select.
>>
>> I agree, that depends on looks horrible =).
>
> I've suggested creating a "CONFIG_PC_IO" symbol before that could used
> to simplify this one and a couple of other similar Kconfig statements.
> It is unfortunately a bit tricky because out of the dozen drivers that
> are similar to this one, each one has a slightly different list of
> architectures, and it's not clear which of the differences are intentional
> rather than mistakes.
VGA is special, in that it uses "ISA memory space". This is not a subset of
"PCI memory space", but something different. Some PCI host bridges
(IIRC, e.g. on Mac) do not allow access to this space.
Most other "PC I/O" use ISA I/O space, which is a subset of PCI I/O space.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: [PATCH] video: imxfb: Use regulator API with LCD class for powering
From: Shawn Guo @ 2014-01-08 14:45 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <52CD5361.50205@ti.com>
On Wed, Jan 08, 2014 at 03:32:17PM +0200, Tomi Valkeinen wrote:
> On 2013-12-23 10:28, Shawn Guo wrote:
> > On Mon, Dec 23, 2013 at 12:24:13PM +0400, Alexander Shiyan wrote:
> >>> On Sat, Dec 21, 2013 at 03:08:00PM +0400, Alexander Shiyan wrote:
> >>>> This patch replaces custom lcd_power() callback with
> >>>> regulator API over LCD class.
> >>>
> >>> FYI. Denis' effort on this already goes to v13.
> >>>
> >>> http://thread.gmane.org/gmane.linux.ports.arm.kernel/285326
> >>
> >> Hmm, OK, but having LCD class could resolve our problems with contrast control.
> >> https://www.mail-archive.com/devicetree@vger.kernel.org/msg07660.html
> >
> > I just want to make sure you two are aware of each other's work.
>
> So, should I ignore this patch, or...?
No, no, that's not what I meant. Please go ahead to review the patch
and merge it if it looks good to you.
Shawn
^ permalink raw reply
* Re: [PATCH] video: vgacon: Don't build on arm64
From: Arnd Bergmann @ 2014-01-08 14:48 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1387323421-26126-1-git-send-email-broonie@kernel.org>
On Wednesday 08 January 2014, Geert Uytterhoeven wrote:
> VGA is special, in that it uses "ISA memory space". This is not a subset of
> "PCI memory space", but something different. Some PCI host bridges
> (IIRC, e.g. on Mac) do not allow access to this space.
> Most other "PC I/O" use ISA I/O space, which is a subset of PCI I/O space.
Right, but they often go together, and I think vgacon actually requires
both, doesn't it? I'm not aware of anything else requiring access to the
0xa0000-0xfffff or the 0xf00000-0xffffff ISA memory windows except VGA,
but I could be missing some less common devices. These are often not
available on non-x86 systems, which prevents VGA from working even if
low I/O space addresses are routed to PCI.
The CONFIG_PC_IO symbol would mostly be used for stuff like PC-style
floppy, dma, rtc, pic, parport, uart, etc. I think it does make sense
to include VGA in that list, but we may want to add a few machines
that explicitly support VGA on PCI without supporting other PC-style
components.
Arnd
^ permalink raw reply
* Re: [PATCH] video: vgacon: Don't build on arm64
From: Geert Uytterhoeven @ 2014-01-08 15:05 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1387323421-26126-1-git-send-email-broonie@kernel.org>
On Wed, Jan 8, 2014 at 3:48 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Wednesday 08 January 2014, Geert Uytterhoeven wrote:
>> VGA is special, in that it uses "ISA memory space". This is not a subset of
>> "PCI memory space", but something different. Some PCI host bridges
>> (IIRC, e.g. on Mac) do not allow access to this space.
>> Most other "PC I/O" use ISA I/O space, which is a subset of PCI I/O space.
>
> Right, but they often go together, and I think vgacon actually requires
> both, doesn't it? I'm not aware of anything else requiring access to the
Yes, VGA uses both.
> 0xa0000-0xfffff or the 0xf00000-0xffffff ISA memory windows except VGA,
> but I could be missing some less common devices. These are often not
> available on non-x86 systems, which prevents VGA from working even if
> low I/O space addresses are routed to PCI.
And MDA, for mdacon.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: [PATCH 1/2] drm/panel: Add support for Samsung LTN101NT05 panel
From: Thierry Reding @ 2014-01-08 15:24 UTC (permalink / raw)
To: Marc Dietrich
Cc: linux-fbdev, dri-devel, linux-tegra, Thierry Reding,
linux-arm-kernel
In-Reply-To: <f03a644deb858f10affb7b039f9982bac81b02c9.1387656959.git.marvin24@gmx.de>
[-- Attachment #1: Type: text/plain, Size: 818 bytes --]
On Sat, Dec 21, 2013 at 09:38:12PM +0100, Marc Dietrich wrote:
> The Samsung LNT101NT05 10.1" WXVGA panel can be supported by the simple panel
> driver.
>
> Cc: linux-fbdev@vger.kernel.org
> Cc: dri-devel@lists.freedesktop.org
> Cc: David Airlie <airlied@linux.ie>
> Signed-off-by: Marc Dietrich <marvin24@gmx.de>
> ---
> This isn't strickly required to get the panel up, but Thierry suggested on IRC to
> include it anyway, in case someone else has some use for it.
For the record: the reason this isn't strictly needed is because there's
an EDID that can be probed to get the video timings. But the panel still
can be supported using the timings from the datasheet. One example where
this would be useful is when the EDID isn't connected or simply broken.
That said: applied, thanks!
Thierry
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* RE: [PATCH 1/2] ARM: omapfb: add coherent dma memory support
From: Hiremath, Vaibhav @ 2014-01-09 5:06 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <52CD5D13.9090806@ti.com>
> -----Original Message-----
> From: Valkeinen, Tomi
> Sent: Wednesday, January 08, 2014 7:44 PM
> To: Tony Lindgren; Ivaylo Dimitrov
> Cc: Hiremath, Vaibhav; linux-omap@vger.kernel.org; linux-arm-
> kernel@lists.infradead.org; linux-fbdev@vger.kernel.org
> Subject: Re: [PATCH 1/2] ARM: omapfb: add coherent dma memory support
>
> On 2014-01-08 01:59, Tony Lindgren wrote:
> > * Tomi Valkeinen <tomi.valkeinen@ti.com> [131230 05:21]:
> >> The omapfb driver uses dma_alloc to reserve memory for the framebuffers.
> >> However, on some use cases, even when CMA is in use, it's quite
> >> probable that omapfb fails to allocate the fb, either due to not
> >> enough free dma memory, fragmented dma memory, or CMA failing to make
> >> enough contiguous space.
> >>
> >> This patch adds a kernel cmdline parameter 'omapfb_vram' which can be
> >> used to give the size of a memory area reserved exclusively for
> >> omapfb, and optionally a physical address where the memory area is
> reserved.
> >>
> >> The memory area is reserved with memblock, and assigned to omapfb
> >> with dma_declare_coherent_memory. The dma_alloc function will first
> >> try to allocate the fb from the coherent memory area, and if that
> >> fails, it'll use the normal method of allocation.
> >>
> >> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> >> Cc: Ivaylo Dimitrov <freemangordon@abv.bg>
> >
> > Feel free to queue this along with the DSS patches:
> >
> > Acked-by: Tony Lindgren <tony@atomide.com>
>
> Thanks.
>
> This introduces new kernel boot parameter, and I haven't really had time to test
> and think about this. If Ivaylo doesn't insist on this to be merged for 3.14, I'd
> rather leave this for 3.15 as adding new parameter that we need to support
> "forever" should be thought a bit more.
>
Tomi,
I am seeing underflow issue on AM43x device if I use omapfb_vram argument.
Did you see this on OMAP?
I am using "omapfb_vram\x10M@0xA0000000", and I believe it is correct way of usage.
Thanks,
Vaibhav
> Tomi
>
^ permalink raw reply
* [PATCH] video: mmp: add device tree support
From: Zhou Zhu @ 2014-01-09 5:13 UTC (permalink / raw)
To: linux-fbdev
add device tree support for mmp fb/controller
the description at Documentation/devicetree/bindings/fb/mmp-disp.txt
Signed-off-by: Zhou Zhu <zzhu3@marvell.com>
---
Documentation/devicetree/bindings/fb/mmp-disp.txt | 71 ++++++++++++
drivers/video/mmp/fb/mmpfb.c | 71 ++++++++----
drivers/video/mmp/hw/mmp_ctrl.c | 120 ++++++++++++++++-----
3 files changed, 217 insertions(+), 45 deletions(-)
create mode 100644 Documentation/devicetree/bindings/fb/mmp-disp.txt
diff --git a/Documentation/devicetree/bindings/fb/mmp-disp.txt b/Documentation/devicetree/bindings/fb/mmp-disp.txt
new file mode 100644
index 0000000..3cf2903
--- /dev/null
+++ b/Documentation/devicetree/bindings/fb/mmp-disp.txt
@@ -0,0 +1,71 @@
+* Marvell MMP Display (MMP_DISP)
+
+To config mmp display, 3 parts are required to be set in dts:
+1. mmp fb
+Required properties:
+- compatible: Should be "marvell,mmp-fb".
+- marvell,fb-name: Should be the name of this fb.
+- marvell,path-name: Should be the name of path this fb connecting to.
+- marvell,overlay-id: Should be the id of overlay this fb is on.
+- marvell,dmafetch-id: Should be the dma fetch id this fb using.
+- marvell,default-pixfmt: Should be the default pixel format when this fb is
+turned on.
+
+2. mmp controller
+Required properties:
+- compatible: Should be "marvell,mmp-disp".
+- reg: Should be address and length of the register set for this controller.
+- interrupts: Should be interrupt of this controller.
+- marvell,disp-name: Should be name of this controller
+- marvell,path-num: Should be path number exists in this controller.
+- marvell,clk-name: Should be name of clock this controller using.
+
+Required sub-node:
+- path:
+Required properties in this sub-node:
+-- marvell,path-name: Should be name of this path, fb/panel uses this name to
+connect to this path.
+-- marvell,overlay_num: Should be number of overlay this path has.
+-- marvell,output-type: Should be output-type settings
+-- marvell,path-config: Should be path-config settings
+-- marvell,link-config: Should be link-config settings
+-- marvell,rbswap: Should be rbswap settings
+
+3. panel
+Required properties:
+- marvell,path-name: Should be path name that this panel connected to.
+- other properties each panel has.
+
+Examples:
+
+fb: fb {
+ compatible = "marvell,mmp-fb";
+ marvell,fb-name = "mmp_fb";
+ marvell,path-name = "mmp_pnpath";
+ marvell,overlay-id = <0>;
+ marvell,dmafetch-id = <1>;
+ marvell,default-pixfmt = <0x108>;
+};
+
+disp: disp@d420b000 {
+ compatible = "marvell,mmp-disp";
+ reg = <0xd420b000 0x1fc>;
+ interrupts = <0 41 0x4>;
+ marvell,disp-name = "mmp_disp";
+ marvell,path-num = <1>;
+ marvell,clk-name = "LCDCIHCLK";
+ path1 {
+ marvell,path-name = "mmp_pnpath";
+ marvell,overlay-num = <2>;
+ marvell,output-type = <0>;
+ marvell,path-config = <0x20000000>;
+ marvell,link-config = <0x60000001>;
+ marvell,rbswap = <0>;
+ };
+};
+
+panel: <panel-name> {
+ ...
+ marvell,path-name = "mmp_pnpath";
+ ...
+};
diff --git a/drivers/video/mmp/fb/mmpfb.c b/drivers/video/mmp/fb/mmpfb.c
index 7ab31eb..e84a411 100644
--- a/drivers/video/mmp/fb/mmpfb.c
+++ b/drivers/video/mmp/fb/mmpfb.c
@@ -22,6 +22,8 @@
#include <linux/module.h>
#include <linux/dma-mapping.h>
#include <linux/platform_device.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
#include "mmpfb.h"
static int var_to_pixfmt(struct fb_var_screeninfo *var)
@@ -551,56 +553,86 @@ static void fb_info_clear(struct fb_info *info)
fb_dealloc_cmap(&info->cmap);
}
+#ifdef CONFIG_OF
+static const struct of_device_id mmp_fb_dt_match[] = {
+ { .compatible = "marvell,mmp-fb" },
+ {},
+};
+#endif
+
static int mmpfb_probe(struct platform_device *pdev)
{
+#ifdef CONFIG_OF
+ struct device_node *np;
+#else
struct mmp_buffer_driver_mach_info *mi;
+#endif
struct fb_info *info = 0;
struct mmpfb_info *fbi = 0;
- int ret, modes_num;
-
- mi = pdev->dev.platform_data;
- if (mi = NULL) {
- dev_err(&pdev->dev, "no platform data defined\n");
- return -EINVAL;
- }
+ int ret = -EINVAL, modes_num;
+ int overlay_id, dmafetch_id;
+ const char *path_name;
/* initialize fb */
info = framebuffer_alloc(sizeof(struct mmpfb_info), &pdev->dev);
if (info = NULL)
return -ENOMEM;
fbi = info->par;
- if (!fbi) {
- ret = -EINVAL;
+ if (!fbi)
+ goto failed;
+
+#ifdef CONFIG_OF
+ np = pdev->dev.of_node;
+
+ if (!np || of_property_read_string(np,
+ "marvell,fb-name", &fbi->name) ||
+ of_property_read_string(np,
+ "marvell,path-name", &path_name) ||
+ of_property_read_u32(np,
+ "marvell,overlay-id", &overlay_id) ||
+ of_property_read_u32(np,
+ "marvell,dmafetch-id", &dmafetch_id) ||
+ of_property_read_u32(np,
+ "marvell,default-pixfmt", &fbi->pix_fmt)) {
+ dev_err(&pdev->dev, "unable to get fb setting from dt\n");
goto failed;
}
+#else
+ mi = pdev->dev.platform_data;
+ if (mi = NULL) {
+ dev_err(&pdev->dev, "no platform data defined\n");
+ goto failed;
+ }
+ fbi->name = mi->name;
+ path_name = mi->path_name;
+ overlay_id = mi->overlay_id;
+ dmafetch_id = mi->dmafetch_id;
+ fbi->pix_fmt = mi->default_pixfmt;
+#endif
/* init fb */
fbi->fb_info = info;
platform_set_drvdata(pdev, fbi);
fbi->dev = &pdev->dev;
- fbi->name = mi->name;
- fbi->pix_fmt = mi->default_pixfmt;
pixfmt_to_var(&info->var, fbi->pix_fmt);
mutex_init(&fbi->access_ok);
/* get display path by name */
- fbi->path = mmp_get_path(mi->path_name);
+ fbi->path = mmp_get_path(path_name);
if (!fbi->path) {
- dev_err(&pdev->dev, "can't get the path %s\n", mi->path_name);
- ret = -EINVAL;
+ dev_err(&pdev->dev, "can't get the path %s\n", path_name);
goto failed_destroy_mutex;
}
dev_info(fbi->dev, "path %s get\n", fbi->path->name);
/* get overlay */
- fbi->overlay = mmp_path_get_overlay(fbi->path, mi->overlay_id);
- if (!fbi->overlay) {
- ret = -EINVAL;
+ fbi->overlay = mmp_path_get_overlay(fbi->path, overlay_id);
+ if (!fbi->overlay)
goto failed_destroy_mutex;
- }
+
/* set fetch used */
- mmp_overlay_set_fetch(fbi->overlay, mi->dmafetch_id);
+ mmp_overlay_set_fetch(fbi->overlay, dmafetch_id);
modes_num = modes_setup(fbi);
if (modes_num < 0) {
@@ -679,6 +711,7 @@ static struct platform_driver mmpfb_driver = {
.driver = {
.name = "mmp-fb",
.owner = THIS_MODULE,
+ .of_match_table = of_match_ptr(mmp_fb_dt_match),
},
.probe = mmpfb_probe,
};
diff --git a/drivers/video/mmp/hw/mmp_ctrl.c b/drivers/video/mmp/hw/mmp_ctrl.c
index 8621a9f..19d68bc 100644
--- a/drivers/video/mmp/hw/mmp_ctrl.c
+++ b/drivers/video/mmp/hw/mmp_ctrl.c
@@ -37,6 +37,8 @@
#include <linux/uaccess.h>
#include <linux/kthread.h>
#include <linux/io.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
#include "mmp_ctrl.h"
@@ -396,26 +398,57 @@ static void path_set_default(struct mmp_path *path)
writel_relaxed(tmp, ctrl_regs(path) + dma_ctrl(0, path->id));
}
-static int path_init(struct mmphw_path_plat *path_plat,
- struct mmp_mach_path_config *config)
+static int path_init(struct mmphw_path_plat *path_plat, void *arg)
{
struct mmphw_ctrl *ctrl = path_plat->ctrl;
struct mmp_path_info *path_info;
struct mmp_path *path = NULL;
-
- dev_info(ctrl->dev, "%s: %s\n", __func__, config->name);
+#ifdef CONFIG_OF
+ struct device_node *path_np = arg;
+#else
+ struct mmp_mach_path_config *config = arg;
+#endif
/* init driver data */
path_info = kzalloc(sizeof(struct mmp_path_info), GFP_KERNEL);
if (!path_info) {
- dev_err(ctrl->dev, "%s: unable to alloc path_info for %s\n",
- __func__, config->name);
- return 0;
+ dev_err(ctrl->dev, "%s: unable to alloc path_info\n",
+ __func__);
+ return -ENOMEM;
+ }
+
+#ifdef CONFIG_OF
+ if (!path_np || of_property_read_string(path_np, "marvell,path-name",
+ &path_info->name) ||
+ of_property_read_u32(path_np, "marvell,overlay-num",
+ &path_info->output_type) ||
+ of_property_read_u32(path_np, "marvell,output-type",
+ &path_info->overlay_num)) {
+ dev_err(ctrl->dev, "%s: unable to get path setting from dt\n",
+ __func__);
+ kfree(path_info);
+ return -EINVAL;
}
+ /* allow these settings not set */
+ of_property_read_u32(path_np, "marvell,path-config",
+ &path_plat->path_config);
+ of_property_read_u32(path_np, "marvell,link-config",
+ &path_plat->link_config);
+ of_property_read_u32(path_np, "marvell,rbswap",
+ &path_plat->dsi_rbswap);
+#else
path_info->name = config->name;
+ path_info->overlay_num = config->overlay_num;
+ path_info->output_type = config->output_type;
+ path_plat->path_config = config->path_config;
+ path_plat->link_config = config->link_config;
+ path_plat->dsi_rbswap = config->dsi_rbswap;
+#endif
+
+ dev_info(ctrl->dev, "%s: %s\n", __func__, path_info->name);
+
path_info->id = path_plat->id;
path_info->dev = ctrl->dev;
- path_info->overlay_num = config->overlay_num;
path_info->overlay_ops = &mmphw_overlay_ops;
path_info->set_mode = path_set_mode;
path_info->plat_data = path_plat;
@@ -424,16 +457,13 @@ static int path_init(struct mmphw_path_plat *path_plat,
path = mmp_register_path(path_info);
if (!path) {
kfree(path_info);
- return 0;
+ return -EINVAL;
}
path_plat->path = path;
- path_plat->path_config = config->path_config;
- path_plat->link_config = config->link_config;
- path_plat->dsi_rbswap = config->dsi_rbswap;
path_set_default(path);
kfree(path_info);
- return 1;
+ return 0;
}
static void path_deinit(struct mmphw_path_plat *path_plat)
@@ -445,13 +475,25 @@ static void path_deinit(struct mmphw_path_plat *path_plat)
mmp_unregister_path(path_plat->path);
}
+#ifdef CONFIG_OF
+static const struct of_device_id mmp_disp_dt_match[] = {
+ { .compatible = "marvell,mmp-disp" },
+ {},
+};
+#endif
+
static int mmphw_probe(struct platform_device *pdev)
{
- struct mmp_mach_plat_info *mi;
struct resource *res;
- int ret, i, size, irq;
+ int ret, i, size, irq, path_num;
+ const char *clk_name, *disp_name;
struct mmphw_path_plat *path_plat;
struct mmphw_ctrl *ctrl = NULL;
+#ifdef CONFIG_OF
+ struct device_node *np, *path_np = NULL;
+#else
+ struct mmp_mach_plat_info *mi;
+#endif
/* get resources from platform data */
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
@@ -468,6 +510,22 @@ static int mmphw_probe(struct platform_device *pdev)
goto failed;
}
+#ifdef CONFIG_OF
+ np = pdev->dev.of_node;
+
+ if (!np || of_property_read_u32(np,
+ "marvell,path-num", &path_num) ||
+ of_property_read_string(np,
+ "marvell,disp-name", &disp_name) ||
+ of_property_read_string(np,
+ "marvell,clk-name", &clk_name) ||
+ of_get_child_count(np) != ctrl->path_num) {
+ dev_err(&pdev->dev, "%s: failed to get settings from dt\n",
+ __func__);
+ ret = -EINVAL;
+ goto failed;
+ }
+#else
/* get configs from platform data */
mi = pdev->dev.platform_data;
if (mi = NULL || !mi->path_num || !mi->paths) {
@@ -476,17 +534,21 @@ static int mmphw_probe(struct platform_device *pdev)
goto failed;
}
- /* allocate */
+ disp_name = mi->name;
+ path_num = mi->path_num;
+ clk_name = mi->clk_name;
+#endif
+
+ /* allocate ctrl */
size = sizeof(struct mmphw_ctrl) + sizeof(struct mmphw_path_plat) *
- mi->path_num;
+ path_num;
ctrl = devm_kzalloc(&pdev->dev, size, GFP_KERNEL);
if (!ctrl) {
ret = -ENOMEM;
goto failed;
}
-
- ctrl->name = mi->name;
- ctrl->path_num = mi->path_num;
+ ctrl->path_num = path_num;
+ ctrl->name = disp_name;
ctrl->dev = &pdev->dev;
ctrl->irq = irq;
platform_set_drvdata(pdev, ctrl);
@@ -521,9 +583,9 @@ static int mmphw_probe(struct platform_device *pdev)
}
/* get clock */
- ctrl->clk = devm_clk_get(ctrl->dev, mi->clk_name);
+ ctrl->clk = devm_clk_get(ctrl->dev, clk_name);
if (IS_ERR(ctrl->clk)) {
- dev_err(ctrl->dev, "unable to get clk %s\n", mi->clk_name);
+ dev_err(ctrl->dev, "unable to get clk %s\n", clk_name);
ret = -ENOENT;
goto failed;
}
@@ -539,11 +601,16 @@ static int mmphw_probe(struct platform_device *pdev)
path_plat->id = i;
path_plat->ctrl = ctrl;
- /* path init */
- if (!path_init(path_plat, &mi->paths[i])) {
- ret = -EINVAL;
+ /* path init from mach info or dt */
+#ifdef CONFIG_OF
+ path_np = of_get_next_child(np, path_np);
+ ret = path_init(path_plat, path_np);
+#else
+ ret = path_init(path_plat, &mi->paths[i]);
+#endif
+
+ if (ret)
goto failed_path_init;
- }
}
#ifdef CONFIG_MMP_DISP_SPI
@@ -573,6 +640,7 @@ static struct platform_driver mmphw_driver = {
.driver = {
.name = "mmp-disp",
.owner = THIS_MODULE,
+ .of_match_table = of_match_ptr(mmp_disp_dt_match),
},
.probe = mmphw_probe,
};
--
1.7.9.5
^ permalink raw reply related
* Re: [PATCH] video: mmp: add device tree support
From: Zhou Zhu @ 2014-01-09 6:05 UTC (permalink / raw)
To: Zhou Zhu, devicetree-u79uwXL29TY76Z2rM5mHXA
Cc: linux-fbdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Tomi Valkeinen, Jean-Christophe Plagniol-Villard, Haojian Zhuang,
Chao Xie, Guoqing Li
In-Reply-To: <1389244394-10779-1-git-send-email-zzhu3-eYqpPyKDWXRBDgjK7y7TUQ@public.gmane.org>
Add devicetree mail list.
On 01/09/2014 01:13 PM, Zhou Zhu wrote:
> add device tree support for mmp fb/controller
> the description at Documentation/devicetree/bindings/fb/mmp-disp.txt
>
> Signed-off-by: Zhou Zhu <zzhu3@marvell.com>
> ---
> Documentation/devicetree/bindings/fb/mmp-disp.txt | 71 ++++++++++++
> drivers/video/mmp/fb/mmpfb.c | 71 ++++++++----
> drivers/video/mmp/hw/mmp_ctrl.c | 120 ++++++++++++++++-----
> 3 files changed, 217 insertions(+), 45 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/fb/mmp-disp.txt
>
> diff --git a/Documentation/devicetree/bindings/fb/mmp-disp.txt b/Documentation/devicetree/bindings/fb/mmp-disp.txt
> new file mode 100644
> index 0000000..3cf2903
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/fb/mmp-disp.txt
> @@ -0,0 +1,71 @@
> +* Marvell MMP Display (MMP_DISP)
> +
> +To config mmp display, 3 parts are required to be set in dts:
> +1. mmp fb
> +Required properties:
> +- compatible: Should be "marvell,mmp-fb".
> +- marvell,fb-name: Should be the name of this fb.
> +- marvell,path-name: Should be the name of path this fb connecting to.
> +- marvell,overlay-id: Should be the id of overlay this fb is on.
> +- marvell,dmafetch-id: Should be the dma fetch id this fb using.
> +- marvell,default-pixfmt: Should be the default pixel format when this fb is
> +turned on.
> +
> +2. mmp controller
> +Required properties:
> +- compatible: Should be "marvell,mmp-disp".
> +- reg: Should be address and length of the register set for this controller.
> +- interrupts: Should be interrupt of this controller.
> +- marvell,disp-name: Should be name of this controller
> +- marvell,path-num: Should be path number exists in this controller.
> +- marvell,clk-name: Should be name of clock this controller using.
> +
> +Required sub-node:
> +- path:
> +Required properties in this sub-node:
> +-- marvell,path-name: Should be name of this path, fb/panel uses this name to
> +connect to this path.
> +-- marvell,overlay_num: Should be number of overlay this path has.
> +-- marvell,output-type: Should be output-type settings
> +-- marvell,path-config: Should be path-config settings
> +-- marvell,link-config: Should be link-config settings
> +-- marvell,rbswap: Should be rbswap settings
> +
> +3. panel
> +Required properties:
> +- marvell,path-name: Should be path name that this panel connected to.
> +- other properties each panel has.
> +
> +Examples:
> +
> +fb: fb {
> + compatible = "marvell,mmp-fb";
> + marvell,fb-name = "mmp_fb";
> + marvell,path-name = "mmp_pnpath";
> + marvell,overlay-id = <0>;
> + marvell,dmafetch-id = <1>;
> + marvell,default-pixfmt = <0x108>;
> +};
> +
> +disp: disp@d420b000 {
> + compatible = "marvell,mmp-disp";
> + reg = <0xd420b000 0x1fc>;
> + interrupts = <0 41 0x4>;
> + marvell,disp-name = "mmp_disp";
> + marvell,path-num = <1>;
> + marvell,clk-name = "LCDCIHCLK";
> + path1 {
> + marvell,path-name = "mmp_pnpath";
> + marvell,overlay-num = <2>;
> + marvell,output-type = <0>;
> + marvell,path-config = <0x20000000>;
> + marvell,link-config = <0x60000001>;
> + marvell,rbswap = <0>;
> + };
> +};
> +
> +panel: <panel-name> {
> + ...
> + marvell,path-name = "mmp_pnpath";
> + ...
> +};
> diff --git a/drivers/video/mmp/fb/mmpfb.c b/drivers/video/mmp/fb/mmpfb.c
> index 7ab31eb..e84a411 100644
> --- a/drivers/video/mmp/fb/mmpfb.c
> +++ b/drivers/video/mmp/fb/mmpfb.c
> @@ -22,6 +22,8 @@
> #include <linux/module.h>
> #include <linux/dma-mapping.h>
> #include <linux/platform_device.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> #include "mmpfb.h"
>
> static int var_to_pixfmt(struct fb_var_screeninfo *var)
> @@ -551,56 +553,86 @@ static void fb_info_clear(struct fb_info *info)
> fb_dealloc_cmap(&info->cmap);
> }
>
> +#ifdef CONFIG_OF
> +static const struct of_device_id mmp_fb_dt_match[] = {
> + { .compatible = "marvell,mmp-fb" },
> + {},
> +};
> +#endif
> +
> static int mmpfb_probe(struct platform_device *pdev)
> {
> +#ifdef CONFIG_OF
> + struct device_node *np;
> +#else
> struct mmp_buffer_driver_mach_info *mi;
> +#endif
> struct fb_info *info = 0;
> struct mmpfb_info *fbi = 0;
> - int ret, modes_num;
> -
> - mi = pdev->dev.platform_data;
> - if (mi = NULL) {
> - dev_err(&pdev->dev, "no platform data defined\n");
> - return -EINVAL;
> - }
> + int ret = -EINVAL, modes_num;
> + int overlay_id, dmafetch_id;
> + const char *path_name;
>
> /* initialize fb */
> info = framebuffer_alloc(sizeof(struct mmpfb_info), &pdev->dev);
> if (info = NULL)
> return -ENOMEM;
> fbi = info->par;
> - if (!fbi) {
> - ret = -EINVAL;
> + if (!fbi)
> + goto failed;
> +
> +#ifdef CONFIG_OF
> + np = pdev->dev.of_node;
> +
> + if (!np || of_property_read_string(np,
> + "marvell,fb-name", &fbi->name) ||
> + of_property_read_string(np,
> + "marvell,path-name", &path_name) ||
> + of_property_read_u32(np,
> + "marvell,overlay-id", &overlay_id) ||
> + of_property_read_u32(np,
> + "marvell,dmafetch-id", &dmafetch_id) ||
> + of_property_read_u32(np,
> + "marvell,default-pixfmt", &fbi->pix_fmt)) {
> + dev_err(&pdev->dev, "unable to get fb setting from dt\n");
> goto failed;
> }
> +#else
> + mi = pdev->dev.platform_data;
> + if (mi = NULL) {
> + dev_err(&pdev->dev, "no platform data defined\n");
> + goto failed;
> + }
> + fbi->name = mi->name;
> + path_name = mi->path_name;
> + overlay_id = mi->overlay_id;
> + dmafetch_id = mi->dmafetch_id;
> + fbi->pix_fmt = mi->default_pixfmt;
> +#endif
>
> /* init fb */
> fbi->fb_info = info;
> platform_set_drvdata(pdev, fbi);
> fbi->dev = &pdev->dev;
> - fbi->name = mi->name;
> - fbi->pix_fmt = mi->default_pixfmt;
> pixfmt_to_var(&info->var, fbi->pix_fmt);
> mutex_init(&fbi->access_ok);
>
> /* get display path by name */
> - fbi->path = mmp_get_path(mi->path_name);
> + fbi->path = mmp_get_path(path_name);
> if (!fbi->path) {
> - dev_err(&pdev->dev, "can't get the path %s\n", mi->path_name);
> - ret = -EINVAL;
> + dev_err(&pdev->dev, "can't get the path %s\n", path_name);
> goto failed_destroy_mutex;
> }
>
> dev_info(fbi->dev, "path %s get\n", fbi->path->name);
>
> /* get overlay */
> - fbi->overlay = mmp_path_get_overlay(fbi->path, mi->overlay_id);
> - if (!fbi->overlay) {
> - ret = -EINVAL;
> + fbi->overlay = mmp_path_get_overlay(fbi->path, overlay_id);
> + if (!fbi->overlay)
> goto failed_destroy_mutex;
> - }
> +
> /* set fetch used */
> - mmp_overlay_set_fetch(fbi->overlay, mi->dmafetch_id);
> + mmp_overlay_set_fetch(fbi->overlay, dmafetch_id);
>
> modes_num = modes_setup(fbi);
> if (modes_num < 0) {
> @@ -679,6 +711,7 @@ static struct platform_driver mmpfb_driver = {
> .driver = {
> .name = "mmp-fb",
> .owner = THIS_MODULE,
> + .of_match_table = of_match_ptr(mmp_fb_dt_match),
> },
> .probe = mmpfb_probe,
> };
> diff --git a/drivers/video/mmp/hw/mmp_ctrl.c b/drivers/video/mmp/hw/mmp_ctrl.c
> index 8621a9f..19d68bc 100644
> --- a/drivers/video/mmp/hw/mmp_ctrl.c
> +++ b/drivers/video/mmp/hw/mmp_ctrl.c
> @@ -37,6 +37,8 @@
> #include <linux/uaccess.h>
> #include <linux/kthread.h>
> #include <linux/io.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
>
> #include "mmp_ctrl.h"
>
> @@ -396,26 +398,57 @@ static void path_set_default(struct mmp_path *path)
> writel_relaxed(tmp, ctrl_regs(path) + dma_ctrl(0, path->id));
> }
>
> -static int path_init(struct mmphw_path_plat *path_plat,
> - struct mmp_mach_path_config *config)
> +static int path_init(struct mmphw_path_plat *path_plat, void *arg)
> {
> struct mmphw_ctrl *ctrl = path_plat->ctrl;
> struct mmp_path_info *path_info;
> struct mmp_path *path = NULL;
> -
> - dev_info(ctrl->dev, "%s: %s\n", __func__, config->name);
> +#ifdef CONFIG_OF
> + struct device_node *path_np = arg;
> +#else
> + struct mmp_mach_path_config *config = arg;
> +#endif
>
> /* init driver data */
> path_info = kzalloc(sizeof(struct mmp_path_info), GFP_KERNEL);
> if (!path_info) {
> - dev_err(ctrl->dev, "%s: unable to alloc path_info for %s\n",
> - __func__, config->name);
> - return 0;
> + dev_err(ctrl->dev, "%s: unable to alloc path_info\n",
> + __func__);
> + return -ENOMEM;
> + }
> +
> +#ifdef CONFIG_OF
> + if (!path_np || of_property_read_string(path_np, "marvell,path-name",
> + &path_info->name) ||
> + of_property_read_u32(path_np, "marvell,overlay-num",
> + &path_info->output_type) ||
> + of_property_read_u32(path_np, "marvell,output-type",
> + &path_info->overlay_num)) {
> + dev_err(ctrl->dev, "%s: unable to get path setting from dt\n",
> + __func__);
> + kfree(path_info);
> + return -EINVAL;
> }
> + /* allow these settings not set */
> + of_property_read_u32(path_np, "marvell,path-config",
> + &path_plat->path_config);
> + of_property_read_u32(path_np, "marvell,link-config",
> + &path_plat->link_config);
> + of_property_read_u32(path_np, "marvell,rbswap",
> + &path_plat->dsi_rbswap);
> +#else
> path_info->name = config->name;
> + path_info->overlay_num = config->overlay_num;
> + path_info->output_type = config->output_type;
> + path_plat->path_config = config->path_config;
> + path_plat->link_config = config->link_config;
> + path_plat->dsi_rbswap = config->dsi_rbswap;
> +#endif
> +
> + dev_info(ctrl->dev, "%s: %s\n", __func__, path_info->name);
> +
> path_info->id = path_plat->id;
> path_info->dev = ctrl->dev;
> - path_info->overlay_num = config->overlay_num;
> path_info->overlay_ops = &mmphw_overlay_ops;
> path_info->set_mode = path_set_mode;
> path_info->plat_data = path_plat;
> @@ -424,16 +457,13 @@ static int path_init(struct mmphw_path_plat *path_plat,
> path = mmp_register_path(path_info);
> if (!path) {
> kfree(path_info);
> - return 0;
> + return -EINVAL;
> }
> path_plat->path = path;
> - path_plat->path_config = config->path_config;
> - path_plat->link_config = config->link_config;
> - path_plat->dsi_rbswap = config->dsi_rbswap;
> path_set_default(path);
>
> kfree(path_info);
> - return 1;
> + return 0;
> }
>
> static void path_deinit(struct mmphw_path_plat *path_plat)
> @@ -445,13 +475,25 @@ static void path_deinit(struct mmphw_path_plat *path_plat)
> mmp_unregister_path(path_plat->path);
> }
>
> +#ifdef CONFIG_OF
> +static const struct of_device_id mmp_disp_dt_match[] = {
> + { .compatible = "marvell,mmp-disp" },
> + {},
> +};
> +#endif
> +
> static int mmphw_probe(struct platform_device *pdev)
> {
> - struct mmp_mach_plat_info *mi;
> struct resource *res;
> - int ret, i, size, irq;
> + int ret, i, size, irq, path_num;
> + const char *clk_name, *disp_name;
> struct mmphw_path_plat *path_plat;
> struct mmphw_ctrl *ctrl = NULL;
> +#ifdef CONFIG_OF
> + struct device_node *np, *path_np = NULL;
> +#else
> + struct mmp_mach_plat_info *mi;
> +#endif
>
> /* get resources from platform data */
> res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> @@ -468,6 +510,22 @@ static int mmphw_probe(struct platform_device *pdev)
> goto failed;
> }
>
> +#ifdef CONFIG_OF
> + np = pdev->dev.of_node;
> +
> + if (!np || of_property_read_u32(np,
> + "marvell,path-num", &path_num) ||
> + of_property_read_string(np,
> + "marvell,disp-name", &disp_name) ||
> + of_property_read_string(np,
> + "marvell,clk-name", &clk_name) ||
> + of_get_child_count(np) != ctrl->path_num) {
> + dev_err(&pdev->dev, "%s: failed to get settings from dt\n",
> + __func__);
> + ret = -EINVAL;
> + goto failed;
> + }
> +#else
> /* get configs from platform data */
> mi = pdev->dev.platform_data;
> if (mi = NULL || !mi->path_num || !mi->paths) {
> @@ -476,17 +534,21 @@ static int mmphw_probe(struct platform_device *pdev)
> goto failed;
> }
>
> - /* allocate */
> + disp_name = mi->name;
> + path_num = mi->path_num;
> + clk_name = mi->clk_name;
> +#endif
> +
> + /* allocate ctrl */
> size = sizeof(struct mmphw_ctrl) + sizeof(struct mmphw_path_plat) *
> - mi->path_num;
> + path_num;
> ctrl = devm_kzalloc(&pdev->dev, size, GFP_KERNEL);
> if (!ctrl) {
> ret = -ENOMEM;
> goto failed;
> }
> -
> - ctrl->name = mi->name;
> - ctrl->path_num = mi->path_num;
> + ctrl->path_num = path_num;
> + ctrl->name = disp_name;
> ctrl->dev = &pdev->dev;
> ctrl->irq = irq;
> platform_set_drvdata(pdev, ctrl);
> @@ -521,9 +583,9 @@ static int mmphw_probe(struct platform_device *pdev)
> }
>
> /* get clock */
> - ctrl->clk = devm_clk_get(ctrl->dev, mi->clk_name);
> + ctrl->clk = devm_clk_get(ctrl->dev, clk_name);
> if (IS_ERR(ctrl->clk)) {
> - dev_err(ctrl->dev, "unable to get clk %s\n", mi->clk_name);
> + dev_err(ctrl->dev, "unable to get clk %s\n", clk_name);
> ret = -ENOENT;
> goto failed;
> }
> @@ -539,11 +601,16 @@ static int mmphw_probe(struct platform_device *pdev)
> path_plat->id = i;
> path_plat->ctrl = ctrl;
>
> - /* path init */
> - if (!path_init(path_plat, &mi->paths[i])) {
> - ret = -EINVAL;
> + /* path init from mach info or dt */
> +#ifdef CONFIG_OF
> + path_np = of_get_next_child(np, path_np);
> + ret = path_init(path_plat, path_np);
> +#else
> + ret = path_init(path_plat, &mi->paths[i]);
> +#endif
> +
> + if (ret)
> goto failed_path_init;
> - }
> }
>
> #ifdef CONFIG_MMP_DISP_SPI
> @@ -573,6 +640,7 @@ static struct platform_driver mmphw_driver = {
> .driver = {
> .name = "mmp-disp",
> .owner = THIS_MODULE,
> + .of_match_table = of_match_ptr(mmp_disp_dt_match),
> },
> .probe = mmphw_probe,
> };
>
--
Thanks, -Zhou
^ permalink raw reply
* Re: [PATCH] video: mmp: add device tree support
From: Sascha Hauer @ 2014-01-09 7:31 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1389244394-10779-1-git-send-email-zzhu3@marvell.com>
On Thu, Jan 09, 2014 at 01:13:14PM +0800, Zhou Zhu wrote:
> add device tree support for mmp fb/controller
> the description at Documentation/devicetree/bindings/fb/mmp-disp.txt
>
> Signed-off-by: Zhou Zhu <zzhu3@marvell.com>
> ---
> Documentation/devicetree/bindings/fb/mmp-disp.txt | 71 ++++++++++++
> drivers/video/mmp/fb/mmpfb.c | 71 ++++++++----
> drivers/video/mmp/hw/mmp_ctrl.c | 120 ++++++++++++++++-----
> 3 files changed, 217 insertions(+), 45 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/fb/mmp-disp.txt
>
> diff --git a/Documentation/devicetree/bindings/fb/mmp-disp.txt b/Documentation/devicetree/bindings/fb/mmp-disp.txt
> new file mode 100644
> index 0000000..3cf2903
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/fb/mmp-disp.txt
> @@ -0,0 +1,71 @@
> +* Marvell MMP Display (MMP_DISP)
> +
> +To config mmp display, 3 parts are required to be set in dts:
> +1. mmp fb
> +Required properties:
> +- compatible: Should be "marvell,mmp-fb".
> +- marvell,fb-name: Should be the name of this fb.
> +- marvell,path-name: Should be the name of path this fb connecting to.
> +- marvell,overlay-id: Should be the id of overlay this fb is on.
> +- marvell,dmafetch-id: Should be the dma fetch id this fb using.
> +- marvell,default-pixfmt: Should be the default pixel format when this fb is
> +turned on.
> +
> +2. mmp controller
> +Required properties:
> +- compatible: Should be "marvell,mmp-disp".
> +- reg: Should be address and length of the register set for this controller.
> +- interrupts: Should be interrupt of this controller.
> +- marvell,disp-name: Should be name of this controller
> +- marvell,path-num: Should be path number exists in this controller.
> +- marvell,clk-name: Should be name of clock this controller using.
> +
> +Required sub-node:
> +- path:
> +Required properties in this sub-node:
> +-- marvell,path-name: Should be name of this path, fb/panel uses this name to
> +connect to this path.
> +-- marvell,overlay_num: Should be number of overlay this path has.
> +-- marvell,output-type: Should be output-type settings
> +-- marvell,path-config: Should be path-config settings
> +-- marvell,link-config: Should be link-config settings
> +-- marvell,rbswap: Should be rbswap settings
> +
> +3. panel
> +Required properties:
> +- marvell,path-name: Should be path name that this panel connected to.
> +- other properties each panel has.
> +
> +Examples:
> +
> +fb: fb {
> + compatible = "marvell,mmp-fb";
This compatible should have the specific SoC name in it, not just
'mmp'. Otherwise you can't properly distinguish between this version and
future versions of the mmp core.
> + marvell,fb-name = "mmp_fb";
> + marvell,path-name = "mmp_pnpath";
You're not going to use this string to reference to another node, do
you? We have phandles for this.
> + marvell,overlay-id = <0>;
> + marvell,dmafetch-id = <1>;
> + marvell,default-pixfmt = <0x108>;
> +};
> +
> +disp: disp@d420b000 {
> + compatible = "marvell,mmp-disp";
> + reg = <0xd420b000 0x1fc>;
> + interrupts = <0 41 0x4>;
> + marvell,disp-name = "mmp_disp";
> + marvell,path-num = <1>;
> + marvell,clk-name = "LCDCIHCLK";
Don't pass clk names like this. We have a documented clock binding, use
it.
> +#ifdef CONFIG_OF
> + struct device_node *np;
> +#else
> struct mmp_buffer_driver_mach_info *mi;
> +#endif
> struct fb_info *info = 0;
> struct mmpfb_info *fbi = 0;
> - int ret, modes_num;
> -
> - mi = pdev->dev.platform_data;
> - if (mi = NULL) {
> - dev_err(&pdev->dev, "no platform data defined\n");
> - return -EINVAL;
> - }
> + int ret = -EINVAL, modes_num;
> + int overlay_id, dmafetch_id;
> + const char *path_name;
>
> /* initialize fb */
> info = framebuffer_alloc(sizeof(struct mmpfb_info), &pdev->dev);
> if (info = NULL)
> return -ENOMEM;
> fbi = info->par;
> - if (!fbi) {
> - ret = -EINVAL;
> + if (!fbi)
> + goto failed;
> +
> +#ifdef CONFIG_OF
Just because your kernel build does have CONFIG_OF enabled doesn't mean
it's actually started with a devicetree. You need to make a runtime
decision, not compile time.
Sascha
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply
* Re: RE: [PATCH 1/2] ARM: omapfb: add coherent dma memory support
From: Ivaylo Dimitrov @ 2014-01-09 7:34 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <79CD15C6BA57404B839C016229A409A83EDB11F5@DBDE04.ent.ti.com>
On 09.01.2014 07:06, Hiremath, Vaibhav wrote:
> Tomi,
>
> I am seeing underflow issue on AM43x device if I use omapfb_vram argument.
> Did you see this on OMAP?
>
> I am using "omapfb_vram\x10M@0xA0000000", and I believe it is correct way of usage.
>
> Thanks,
> Vaibhav
>
AFAIK underflow interrupts could come from badly calculated DSS core
clock or bad HW resizer setup and should be unrelated to the memory
allocation. It might be something similar to the problem I have on N900
- see https://lkml.org/lkml/2014/1/6/173
Is it possible to upload the video you have problems with, so me to test
it on N900? So far I didn't see any underflow issues on it (N900 is
OMAP3, in case you're not aware), no matter the resolution of the videos
I played(up to 720p), however I didn't test the part that allocates the
memory on a pre-defined address. Though I don't think that should matter.
Ivo
^ permalink raw reply
* Re: [PATCH] video: mmp: add device tree support
From: Jingoo Han @ 2014-01-09 7:43 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1389244394-10779-1-git-send-email-zzhu3@marvell.com>
On Thursday, January 09, 2014 4:32 PM, Sascha Hauer wrote:
> On Thu, Jan 09, 2014 at 01:13:14PM +0800, Zhou Zhu wrote:
> > add device tree support for mmp fb/controller
> > the description at Documentation/devicetree/bindings/fb/mmp-disp.txt
> >
> > Signed-off-by: Zhou Zhu <zzhu3@marvell.com>
> > ---
> > Documentation/devicetree/bindings/fb/mmp-disp.txt | 71 ++++++++++++
> > drivers/video/mmp/fb/mmpfb.c | 71 ++++++++----
> > drivers/video/mmp/hw/mmp_ctrl.c | 120 ++++++++++++++++-----
> > 3 files changed, 217 insertions(+), 45 deletions(-)
> > create mode 100644 Documentation/devicetree/bindings/fb/mmp-disp.txt
> >
> > diff --git a/Documentation/devicetree/bindings/fb/mmp-disp.txt
> b/Documentation/devicetree/bindings/fb/mmp-disp.txt
> > new file mode 100644
> > index 0000000..3cf2903
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/fb/mmp-disp.txt
[.....]
>
> > +#ifdef CONFIG_OF
> > + struct device_node *np;
> > +#else
> > struct mmp_buffer_driver_mach_info *mi;
> > +#endif
> > struct fb_info *info = 0;
> > struct mmpfb_info *fbi = 0;
> > - int ret, modes_num;
> > -
> > - mi = pdev->dev.platform_data;
> > - if (mi = NULL) {
> > - dev_err(&pdev->dev, "no platform data defined\n");
> > - return -EINVAL;
> > - }
> > + int ret = -EINVAL, modes_num;
> > + int overlay_id, dmafetch_id;
> > + const char *path_name;
> >
> > /* initialize fb */
> > info = framebuffer_alloc(sizeof(struct mmpfb_info), &pdev->dev);
> > if (info = NULL)
> > return -ENOMEM;
> > fbi = info->par;
> > - if (!fbi) {
> > - ret = -EINVAL;
> > + if (!fbi)
> > + goto failed;
> > +
> > +#ifdef CONFIG_OF
>
> Just because your kernel build does have CONFIG_OF enabled doesn't mean
> it's actually started with a devicetree. You need to make a runtime
> decision, not compile time.
Yes, right.
As Sascha Hauer said, you need to make a runtime decision,
instead of compile time. Please keep the same binary for
both cases (CONFIG_OF is 'enabled' and 'disabled').
For example,
if (pdev->dev.of_node) {
// DT code
} else {
// Non-DT code
}
Best regards,
Jingoo Han
^ permalink raw reply
* RE: RE: [PATCH 1/2] ARM: omapfb: add coherent dma memory support
From: Hiremath, Vaibhav @ 2014-01-09 8:08 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <52CE5123.9050702@gmail.com>
> -----Original Message-----
> From: linux-omap-owner@vger.kernel.org [mailto:linux-omap-
> owner@vger.kernel.org] On Behalf Of Ivaylo Dimitrov
> Sent: Thursday, January 09, 2014 1:05 PM
> To: Hiremath, Vaibhav; Valkeinen, Tomi; Tony Lindgren; Ivaylo Dimitrov
> Cc: linux-omap@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-
> fbdev@vger.kernel.org
> Subject: Re: RE: [PATCH 1/2] ARM: omapfb: add coherent dma memory support
>
>
> On 09.01.2014 07:06, Hiremath, Vaibhav wrote:
> > Tomi,
> >
> > I am seeing underflow issue on AM43x device if I use omapfb_vram argument.
> > Did you see this on OMAP?
> >
> > I am using "omapfb_vram\x10M@0xA0000000", and I believe it is correct way
> of usage.
> >
> > Thanks,
> > Vaibhav
> >
>
> AFAIK underflow interrupts could come from badly calculated DSS core clock or
> bad HW resizer setup and should be unrelated to the memory allocation. It
> might be something similar to the problem I have on N900
> - see https://lkml.org/lkml/2014/1/6/173
>
I can see the difference when I really "omapfb_vram" command line argument.
Without omapfb_vram in bootargs
-------------------------------------------
bootargs=console=ttyO0,115200n8 root=/dev/mmcblk0p2 rw rootfstype=ext3 rootwait mem\x128M
consoleblank=0 clocksource=gp_timer consoleblank=0 earlyprintk omapfb.debug=y omapdss.debug=y
I do not get UNDERFLOW during boot.
With omapfb_vram in the bootargs
---------------------------------------------
bootargs=console=ttyO0,115200n8 root=/dev/mmcblk0p2 rw rootfstype=ext3 rootwait mem\x128M
consoleblank=0 clocksource=gp_timer consoleblank=0 earlyprintk omapfb_vram\x10M@0xA0000000
omapfb.debug=y omapdss.debug=y
I always get UNDERFLOW during boot itself.
> Is it possible to upload the video you have problems with, so me to test it on
> N900? So far I didn't see any underflow issues on it (N900 is OMAP3, in case
> you're not aware), no matter the resolution of the videos I played(up to 720p),
> however I didn't test the part that allocates the memory on a pre-defined
> address. Though I don't think that should matter.
No, that's what is causing issue to me. Can you try predefined address flow?
Just to highlight, I get UNDERFLOW during boot itself, immediately when it gets mapped to userspace.
Boot LOG:
[ 1.443021] OMAPFB: omapfb_probe
[ 1.446137] OMAPFB: create 3 framebuffers
[ 1.446178] OMAPFB: fb_infos allocated
[ 1.446198] OMAPFB: allocating 1536000 bytes for fb 0
[ 1.451044] OMAPFB: allocated VRAM paddr a0000000, vaddr ca000000
[ 1.451069] OMAPFB: region0 phys a0000000 virt ca000000 size\x1536000
[ 1.451086] OMAPFB: region1 phys 00000000 virt (null) size=0
[ 1.451100] OMAPFB: region2 phys 00000000 virt (null) size=0
[ 1.451109] OMAPFB: fbmems allocated
[ 1.451363] OMAPFB: check_fb_var 0
[ 1.451386] OMAPFB: max frame size 1536000, line size 3200
[ 1.451401] OMAPFB: xres = 800, yres = 480, vxres = 800, vyres = 480
[ 1.451414] OMAPFB: set_fb_fix
[ 1.460278] OMAPFB: fb_infos initialized
[ 1.465325] OMAPFB: set_par(0)
[ 1.465384] OMAPFB: set_fb_fix
[ 1.465393] OMAPFB: apply_changes, fb 0, ovl 0
[ 1.465443] OMAPFB: setup_overlay 0, posx 0, posy 0, outw 800, outh 480
[ 1.465450] OMAPFB: paddr a0000000
[ 1.465592] OMAPFB: pan_display(0)
[ 1.465600] OMAPFB: setcmap
[ 1.465607] OMAPFB: setcmap
[ 1.474504] Console: switching to colour frame buffer device 100x30
[ 1.474528] OMAPFB: pan_display(0)
[ 1.474534] OMAPFB: setcmap
[ 1.482185] OMAPFB: setcmap
[ 1.484808] OMAPFB: framebuffers registered
[ 1.484839] OMAPFB: apply_changes, fb 0, ovl 0
[ 1.484857] OMAPFB: setup_overlay 0, posx 0, posy 0, outw 800, outh 480
[ 1.484870] OMAPFB: paddr a0000000
[ 1.484919] OMAPFB: apply_changes, fb 1, ovl 1
[ 1.485010] OMAPFB: apply_changes, fb 2, ovl 2
[ 1.485111] OMAPFB: create_framebuffers done
[ 1.485128] OMAPFB: mgr->apply'ed
[ 1.489793] OMAPFB: create sysfs for fbs
[ 1.489816] OMAPFB: create sysfs for fbs
....
[ 4.822549] Freeing unused kernel memory: 440K (c0919000 - c0987000)
[ 5.276615] OMAPFB: pan_display(0)
[ 5.276625] OMAPFB: setcmap
[ 5.276635] OMAPFB: setcmap
[ 5.293518] OMAPFB: user mmap region start a0000000, len 1536000, off 0
[ 5.300171] omapdss APPLY error: FIFO UNDERFLOW on gfx, disabling the overlay
...
[ 20.499076] OMAPFB: pan_display(0)
[ 20.499085] OMAPFB: setcmap
[ 20.499093] OMAPFB: setcmap
[ 20.544419] OMAPFB: check_var(0)
[ 20.544631] OMAPFB: check_fb_var 0
[ 20.544644] OMAPFB: max frame size 1536000, line size 3200
[ 20.544651] OMAPFB: xres = 800, yres = 480, vxres = 800, vyres = 480
[ 20.544699] OMAPFB: set_par(0)
[ 20.544706] OMAPFB: set_fb_fix
[ 20.544712] OMAPFB: apply_changes, fb 0, ovl 0
[ 20.544762] OMAPFB: setup_overlay 0, posx 0, posy 0, outw 800, outh 480
[ 20.544767] OMAPFB: paddr a0000000
[ 20.544798] OMAPFB: pan_display(0)
[ 20.544802] OMAPFB: setcmap
[ 20.544859] OMAPFB: pan_display(0)
[ 20.544865] OMAPFB: setcmap
[ 20.544872] OMAPFB: setcmap
[ 20.553841] OMAPFB: setcmap
[ 23.002160] OMAPFB: user mmap region start a0000000, len 1536000, off 0
Thanks,
Vaibhav
^ permalink raw reply
* Re: [PATCH 1/2] ARM: omapfb: add coherent dma memory support
From: Tomi Valkeinen @ 2014-01-09 8:21 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <79CD15C6BA57404B839C016229A409A83EDB11F5@DBDE04.ent.ti.com>
[-- Attachment #1: Type: text/plain, Size: 1271 bytes --]
On 2014-01-09 07:06, Hiremath, Vaibhav wrote:
> I am seeing underflow issue on AM43x device if I use omapfb_vram argument.
> Did you see this on OMAP?
>
> I am using "omapfb_vram=10M@0xA0000000", and I believe it is correct way of usage.
Hmm ok... The AM4x seems to have issues anyway, as we're seeing
underflows easily in other situations also.
Well, there's a small difference in the allocation. The normal dma alloc
uses dma_alloc_attrs() and passes DMA_ATTR_WRITE_COMBINE as a flag,
whereas allocating from the absolute address just uses the piece of
memory. I couldn't find how to set write-combine for the abs memory area.
Then again, that's for CPU caching, so I don't see why it would affect
DSS as such (but that's still something we should measure, cpu
read/write perf for normal and abs allocation).
The only thought I have is that somehow the reserved memory area is
missing some configuration that is done for the rest of the memory. But
that's purely a guess, this is totally out of my area of expertise...
Vaibhav, just to be sure, can you run both with normal dma_alloc and
with the reserve, and verify that the dispc register dumps are the same?
I don't see how they could be different, but just to be sure.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]
^ permalink raw reply
* Re: [PATCH 1/2] ARM: omapfb: add coherent dma memory support
From: Tomi Valkeinen @ 2014-01-09 8:27 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <79CD15C6BA57404B839C016229A409A83EDB141E@DBDE04.ent.ti.com>
[-- Attachment #1: Type: text/plain, Size: 1042 bytes --]
On 2014-01-09 10:08, Hiremath, Vaibhav wrote:
> No, that's what is causing issue to me. Can you try predefined address flow?
> Just to highlight, I get UNDERFLOW during boot itself, immediately when it gets mapped to userspace.
>
> Boot LOG:
>
> [ 4.822549] Freeing unused kernel memory: 440K (c0919000 - c0987000)
> [ 5.276615] OMAPFB: pan_display(0)
> [ 5.276625] OMAPFB: setcmap
> [ 5.276635] OMAPFB: setcmap
> [ 5.293518] OMAPFB: user mmap region start a0000000, len 1536000, off 0
> [ 5.300171] omapdss APPLY error: FIFO UNDERFLOW on gfx, disabling the overlay
Hmm that's interesting... So you have some tool that's ran early, which
draws something on the screen? Or maybe that's X starting?
Ah... I think I understand now. As I mentioned, AM4x has issues already.
I think the CPU/others can block DSS when accessing memory. So, if we
have bad caching for the mapped framebuffer, the CPU will use more
bandwidth when reading/writing to it, and that will cause DSS to underflow.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]
^ permalink raw reply
* RE: [PATCH 1/2] ARM: omapfb: add coherent dma memory support
From: Hiremath, Vaibhav @ 2014-01-09 8:31 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <52CE5C1A.3070603@ti.com>
> -----Original Message-----
> From: Valkeinen, Tomi
> Sent: Thursday, January 09, 2014 1:52 PM
> To: Hiremath, Vaibhav; Ivaylo Dimitrov
> Cc: Tony Lindgren; linux-omap@vger.kernel.org; linux-arm-
> kernel@lists.infradead.org; linux-fbdev@vger.kernel.org
> Subject: Re: [PATCH 1/2] ARM: omapfb: add coherent dma memory support
>
> On 2014-01-09 07:06, Hiremath, Vaibhav wrote:
>
> > I am seeing underflow issue on AM43x device if I use omapfb_vram argument.
> > Did you see this on OMAP?
> >
> > I am using "omapfb_vram\x10M@0xA0000000", and I believe it is correct way
> of usage.
>
> Hmm ok... The AM4x seems to have issues anyway, as we're seeing underflows
> easily in other situations also.
>
> Well, there's a small difference in the allocation. The normal dma alloc uses
> dma_alloc_attrs() and passes DMA_ATTR_WRITE_COMBINE as a flag, whereas
> allocating from the absolute address just uses the piece of memory. I couldn't
> find how to set write-combine for the abs memory area.
>
> Then again, that's for CPU caching, so I don't see why it would affect DSS as
> such (but that's still something we should measure, cpu read/write perf for
> normal and abs allocation).
>
> The only thought I have is that somehow the reserved memory area is missing
> some configuration that is done for the rest of the memory. But that's purely a
> guess, this is totally out of my area of expertise...
>
> Vaibhav, just to be sure, can you run both with normal dma_alloc and with the
> reserve, and verify that the dispc register dumps are the same?
> I don't see how they could be different, but just to be sure.
>
Will check and update you shortly.
Thanks,
Vaibhav
^ permalink raw reply
* RE: [PATCH 1/2] ARM: omapfb: add coherent dma memory support
From: Hiremath, Vaibhav @ 2014-01-09 8:31 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <52CE5D6F.3050700@ti.com>
> -----Original Message-----
> From: Valkeinen, Tomi
> Sent: Thursday, January 09, 2014 1:57 PM
> To: Hiremath, Vaibhav; Ivaylo Dimitrov; Ivaylo Dimitrov
> Cc: Tony Lindgren; linux-omap@vger.kernel.org; linux-arm-
> kernel@lists.infradead.org; linux-fbdev@vger.kernel.org
> Subject: Re: [PATCH 1/2] ARM: omapfb: add coherent dma memory support
>
> On 2014-01-09 10:08, Hiremath, Vaibhav wrote:
>
> > No, that's what is causing issue to me. Can you try predefined address flow?
> > Just to highlight, I get UNDERFLOW during boot itself, immediately when it
> gets mapped to userspace.
> >
> > Boot LOG:
> >
>
> > [ 4.822549] Freeing unused kernel memory: 440K (c0919000 - c0987000)
> > [ 5.276615] OMAPFB: pan_display(0)
> > [ 5.276625] OMAPFB: setcmap
> > [ 5.276635] OMAPFB: setcmap
> > [ 5.293518] OMAPFB: user mmap region start a0000000, len 1536000, off 0
> > [ 5.300171] omapdss APPLY error: FIFO UNDERFLOW on gfx, disabling the
> overlay
>
> Hmm that's interesting... So you have some tool that's ran early, which draws
> something on the screen? Or maybe that's X starting?
>
It's initial demo, not sure whether you heard of MATRIX demo. I am running Matrix demo
As part of init script.
> Ah... I think I understand now. As I mentioned, AM4x has issues already.
> I think the CPU/others can block DSS when accessing memory. So, if we have
> bad caching for the mapped framebuffer, the CPU will use more bandwidth
> when reading/writing to it, and that will cause DSS to underflow.
>
Yeah, AM4x already has some issues but I am comparing normal dma_alloc and reserved
Memory and I see different behavior and it could be due to bad caching for the mapped buffers.
Thanks,
Vaibhav
^ permalink raw reply
* Re: [PATCH] video: Replace local macro with PCI standard macro
From: Tomi Valkeinen @ 2014-01-09 12:05 UTC (permalink / raw)
To: Yijing Wang
Cc: Jean-Christophe Plagniol-Villard, linux-fbdev, linux-kernel,
Hanjun Guo
In-Reply-To: <1386242718-24372-1-git-send-email-wangyijing@huawei.com>
[-- Attachment #1: Type: text/plain, Size: 327 bytes --]
On 2013-12-05 13:25, Yijing Wang wrote:
> Replace local macro TGA_BUS_PCI with PCI standard
> marco dev_is_pci().
>
> Signed-off-by: Yijing Wang <wangyijing@huawei.com>
> ---
> drivers/video/tgafb.c | 16 +++++-----------
> 1 files changed, 5 insertions(+), 11 deletions(-)
Thanks, queued for 3.14.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]
^ permalink raw reply
* Re: [PATCH] video: mmp: add device tree support
From: Zhou Zhu @ 2014-01-09 12:09 UTC (permalink / raw)
To: Jingoo Han, 'Sascha Hauer'
Cc: linux-fbdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
'Tomi Valkeinen',
'Jean-Christophe Plagniol-Villard',
'Haojian Zhuang', Chao Xie, Guoqing Li,
devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <000301cf0d0e$91f29090$b5d7b1b0$%han-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
Sascha/Jingoo,
Thank you for your review!
On 01/09/2014 03:43 PM, Jingoo Han wrote:
> On Thursday, January 09, 2014 4:32 PM, Sascha Hauer wrote:
>> On Thu, Jan 09, 2014 at 01:13:14PM +0800, Zhou Zhu wrote:
>>> add device tree support for mmp fb/controller
>>> the description at Documentation/devicetree/bindings/fb/mmp-disp.txt
>>>
>>> Signed-off-by: Zhou Zhu <zzhu3@marvell.com>
>>> ---
>>> Documentation/devicetree/bindings/fb/mmp-disp.txt | 71 ++++++++++++
>>> drivers/video/mmp/fb/mmpfb.c | 71 ++++++++----
>>> drivers/video/mmp/hw/mmp_ctrl.c | 120 ++++++++++++++++-----
>>> 3 files changed, 217 insertions(+), 45 deletions(-)
>>> create mode 100644 Documentation/devicetree/bindings/fb/mmp-disp.txt
>>>
[...]
>> +fb: fb {
>> + compatible = "marvell,mmp-fb";
>
> This compatible should have the specific SoC name in it, not just
> 'mmp'. Otherwise you can't properly distinguish between this version and
> future versions of the mmp core.
>
We are using a same display IP for all mmp serial SoCs, and there would
be inside register to get version. So I am planning put same compatible
here for all SoCs using this IP.
Would it be ok if I update compatible to "marvell,mmpdcx-fb"? "mmpdcx"
is the IP name.
>
>> + marvell,fb-name = "mmp_fb";
>> + marvell,path-name = "mmp_pnpath";
>
> You're not going to use this string to reference to another node, do
> you? We have phandles for this.
>
I will update it in v2.
>> + marvell,overlay-id = <0>;
>> + marvell,dmafetch-id = <1>;
>> + marvell,default-pixfmt = <0x108>;
>> +};
>> +
>> +disp: disp@d420b000 {
>> + compatible = "marvell,mmp-disp";
>> + reg = <0xd420b000 0x1fc>;
>> + interrupts = <0 41 0x4>;
>> + marvell,disp-name = "mmp_disp";
>> + marvell,path-num = <1>;
>> + marvell,clk-name = "LCDCIHCLK";
>
> Don't pass clk names like this. We have a documented clock binding, use
> it.
>
The patches to add dt support in common clk in our platforms are not
upstreamed yet. As there's only one clock in this device, could I remove
clock name related codes and direct use: devm_clk_get(dev, NULL)?
>
>>
>>> +#ifdef CONFIG_OF
>>> + struct device_node *np;
>>> +#else
>>> struct mmp_buffer_driver_mach_info *mi;
>>> +#endif
>>> struct fb_info *info = 0;
>>> struct mmpfb_info *fbi = 0;
>>> - int ret, modes_num;
>>> -
>>> - mi = pdev->dev.platform_data;
>>> - if (mi = NULL) {
>>> - dev_err(&pdev->dev, "no platform data defined\n");
>>> - return -EINVAL;
>>> - }
>>> + int ret = -EINVAL, modes_num;
>>> + int overlay_id, dmafetch_id;
>>> + const char *path_name;
>>>
>>> /* initialize fb */
>>> info = framebuffer_alloc(sizeof(struct mmpfb_info), &pdev->dev);
>>> if (info = NULL)
>>> return -ENOMEM;
>>> fbi = info->par;
>>> - if (!fbi) {
>>> - ret = -EINVAL;
>>> + if (!fbi)
>>> + goto failed;
>>> +
>>> +#ifdef CONFIG_OF
>>
>> Just because your kernel build does have CONFIG_OF enabled doesn't mean
>> it's actually started with a devicetree. You need to make a runtime
>> decision, not compile time.
>
> Yes, right.
> As Sascha Hauer said, you need to make a runtime decision,
> instead of compile time. Please keep the same binary for
> both cases (CONFIG_OF is 'enabled' and 'disabled').
>
> For example,
>
> if (pdev->dev.of_node) {
> // DT code
> } else {
> // Non-DT code
> }
>
Thank you for your suggestion. I will update the code to dynamic in v2.
--
Thanks, -Zhou
^ permalink raw reply
* Re: [PATCH RESEND] drivers: video: metronomefb: avoid out-of-bounds array access
From: Tomi Valkeinen @ 2014-01-09 12:21 UTC (permalink / raw)
To: Michal Nazarewicz
Cc: Jean-Christophe Plagniol-Villard, linux-fbdev, linux-kernel
In-Reply-To: <1385743903-9079-1-git-send-email-mpn@google.com>
[-- Attachment #1: Type: text/plain, Size: 2135 bytes --]
On 2013-11-29 18:51, Michal Nazarewicz wrote:
> From: Michal Nazarewicz <mina86@mina86.com>
>
> load_waveform function checks whether padding bytes in stuff2a
> and stuff2b are all zero, but does so by treating those arrays
> as a single longer array. Since the structure is packed, and
> the size sum matches, it all works, but creates some confusion
> in the code.
>
> This commit changes the stuff2a and stuff2b arrays into pad1 and
> pad2 fields such that they cover the same bytes as the arrays
> covered, and changes the check in the load_waveform function so
> that the fields are read instead of iterating over an arary.
>
> It also renames the other “stuff” fields to “ignore*” fields to
> give them more semantic meaning.
>
> Signed-off-by: Michal Nazarewicz <mina86@mina86.com>
> ---
> drivers/video/metronomefb.c | 17 ++++++++---------
> 1 file changed, 8 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/video/metronomefb.c b/drivers/video/metronomefb.c
> index 195cc2d..4f36a2b 100644
> --- a/drivers/video/metronomefb.c
> +++ b/drivers/video/metronomefb.c
> @@ -126,7 +126,7 @@ static struct fb_var_screeninfo metronomefb_var = {
>
> /* the waveform structure that is coming from userspace firmware */
> struct waveform_hdr {
> - u8 stuff[32];
> + u8 ignore1[32];
>
> u8 wmta[3];
> u8 fvsn;
> @@ -134,13 +134,14 @@ struct waveform_hdr {
> u8 luts;
> u8 mc;
> u8 trc;
> - u8 stuff3;
> + u8 ignore2;
>
> u8 endb;
> u8 swtb;
> - u8 stuff2a[2];
> + u32 pad1; /* u16 halfof(pad1) */
>
> - u8 stuff2b[3];
> + /* u16 halfof(pad1) */
> + u8 pad2;
I don't quite follow what those comments mean...
I know nothing of the hw in question, but I guess there's some reason
the stuff2a and stuff2b has been separate, and there's even a blank line
between. If they pad a particular block in the data structure, doesn't
your change make it confusing?
It wouldn't be too pad to have those 5 fields as separate ones. One u16
for the stuff2a, and u16 + u8 for the stuff2b. It'd be simple to check
that those are zero.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]
^ permalink raw reply
* RE: [PATCH 1/2] ARM: omapfb: add coherent dma memory support
From: Hiremath, Vaibhav @ 2014-01-09 13:02 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <52CE5C1A.3070603@ti.com>
> -----Original Message-----
> From: Hiremath, Vaibhav
> Sent: Thursday, January 09, 2014 2:01 PM
> To: Valkeinen, Tomi; Ivaylo Dimitrov
> Cc: Tony Lindgren; linux-omap@vger.kernel.org; linux-arm-
> kernel@lists.infradead.org; linux-fbdev@vger.kernel.org
> Subject: RE: [PATCH 1/2] ARM: omapfb: add coherent dma memory support
>
>
> > -----Original Message-----
> > From: Valkeinen, Tomi
> > Sent: Thursday, January 09, 2014 1:52 PM
> > To: Hiremath, Vaibhav; Ivaylo Dimitrov
> > Cc: Tony Lindgren; linux-omap@vger.kernel.org; linux-arm-
> > kernel@lists.infradead.org; linux-fbdev@vger.kernel.org
> > Subject: Re: [PATCH 1/2] ARM: omapfb: add coherent dma memory support
> >
> > On 2014-01-09 07:06, Hiremath, Vaibhav wrote:
> >
> > > I am seeing underflow issue on AM43x device if I use omapfb_vram
> argument.
> > > Did you see this on OMAP?
> > >
> > > I am using "omapfb_vram\x10M@0xA0000000", and I believe it is correct
> > > way
> > of usage.
> >
> > Hmm ok... The AM4x seems to have issues anyway, as we're seeing
> > underflows easily in other situations also.
> >
> > Well, there's a small difference in the allocation. The normal dma
> > alloc uses
> > dma_alloc_attrs() and passes DMA_ATTR_WRITE_COMBINE as a flag, whereas
> > allocating from the absolute address just uses the piece of memory. I
> > couldn't find how to set write-combine for the abs memory area.
> >
> > Then again, that's for CPU caching, so I don't see why it would affect
> > DSS as such (but that's still something we should measure, cpu
> > read/write perf for normal and abs allocation).
> >
> > The only thought I have is that somehow the reserved memory area is
> > missing some configuration that is done for the rest of the memory.
> > But that's purely a guess, this is totally out of my area of expertise...
> >
> > Vaibhav, just to be sure, can you run both with normal dma_alloc and
> > with the reserve, and verify that the dispc register dumps are the same?
> > I don't see how they could be different, but just to be sure.
> >
>
> Will check and update you shortly.
>
I checked both the DSS configuration in both scenarios and they look same to me.
With normal dma_alloc:
=========root@am43xx-epos-evm:~#
root@am43xx-epos-evm:~#
root@am43xx-epos-evm:~# cat /proc/cmdline
console=ttyO0,115200n8 root=/dev/mmcblk0p2 rw rootfstype=ext3 rootwait mem\x128M consoleblank=0 clocksource=gp_timer consoleblank=0 earlyprintk
root@am43xx-epos-evm:~#
root@am43xx-epos-evm:~#
root@am43xx-epos-evm:~#
root@am43xx-epos-evm:~# cat /sys/kernel/debug/omapdss/dss
DSS_REVISION 00000020
DSS_SYSCONFIG 00000001
DSS_SYSSTATUS 00000001
DSS_CONTROL 00000018
root@am43xx-epos-evm:~#
root@am43xx-epos-evm:~# cat /sys/kernel/debug/omapdss/clk
- DSS -
DSS_FCK (DSS_FCLK1) = 198000000
- DISPC -
dispc fclk source = DSS_FCK (DSS_FCLK1)
fck 198000000
- LCD -
LCD clk source = DSS_FCK (DSS_FCLK1)
lck 198000000 lck div 1
pck 33000000 pck div 6
root@am43xx-epos-evm:~#
root@am43xx-epos-evm:~# cat /sys/kernel/debug/omapdss/dispc_irq
period 44780 ms
irqs 1
FRAMEDONE 0
VSYNC 1
EVSYNC_EVEN 1
EVSYNC_ODD 1
ACBIAS_COUNT_STAT 0
PROG_LINE_NUM 1
GFX_FIFO_UNDERFLOW 0
GFX_END_WIN 1
PAL_GAMMA_MASK 0
OCP_ERR 0
VID1_FIFO_UNDERFLOW 0
VID1_END_WIN 0
VID2_FIFO_UNDERFLOW 0
VID2_END_WIN 0
SYNC_LOST 0
SYNC_LOST_DIGIT 0
WAKEUP 0
root@am43xx-epos-evm:~#
root@am43xx-epos-evm:~# cat /sys/kernel/debug/omapdss/dispc
DISPC_REVISION 00000030
DISPC_SYSCONFIG 00002011
DISPC_SYSSTATUS 00000001
DISPC_IRQSTATUS 000000ae
DISPC_IRQENABLE 0000d640
DISPC_CONTROL 00018309
DISPC_CONFIG 00000204
DISPC_CAPABLE 000003ff
DISPC_LINE_STATUS 000000b1
DISPC_LINE_NUMBER 00000000
DISPC_GLOBAL_ALPHA 000000ff
DISPC_DEFAULT_COLOR(LCD) 00000000
DISPC_TRANS_COLOR(LCD) 00000000
DISPC_SIZE_MGR(LCD) 01df031f
DISPC_DEFAULT_COLOR(LCD) 00000000
DISPC_TRANS_COLOR(LCD) 00000000
DISPC_TIMING_H(LCD) 00f0d11d
DISPC_TIMING_V(LCD) 00a0160c
DISPC_POL_FREQ(LCD) 00003000
DISPC_DIVISORo(LCD) 00010006
DISPC_SIZE_MGR(LCD) 01df031f
DISPC_DATA_CYCLE1(LCD) 00000000
DISPC_DATA_CYCLE2(LCD) 00000000
DISPC_DATA_CYCLE3(LCD) 00000000
DISPC_CPR_COEF_R(LCD) 00000000
DISPC_CPR_COEF_G(LCD) 00000000
DISPC_CPR_COEF_B(LCD) 00000000
DISPC_OVL_BA0(GFX) 86900000
DISPC_OVL_BA1(GFX) 86900000
DISPC_OVL_POSITION(GFX) 00000000
DISPC_OVL_SIZE(GFX) 01df031f
DISPC_OVL_ATTRIBUTES(GFX) 00000091
DISPC_OVL_FIFO_THRESHOLD(GFX) 03ff03c0
DISPC_OVL_FIFO_SIZE_STATUS(GFX) 00000400
DISPC_OVL_ROW_INC(GFX) 00000001
DISPC_OVL_PIXEL_INC(GFX) 00000001
DISPC_OVL_PRELOAD(GFX) 00000100
DISPC_OVL_WINDOW_SKIP(GFX) 00000000
DISPC_OVL_TABLE_BA(GFX) 00000000
DISPC_OVL_BA0(VID1) 00000000
DISPC_OVL_BA1(VID1) 00000000
DISPC_OVL_POSITION(VID1) 00000000
DISPC_OVL_SIZE(VID1) 00000000
DISPC_OVL_ATTRIBUTES(VID1) 00008000
DISPC_OVL_FIFO_THRESHOLD(VID1) 03ff03c0
DISPC_OVL_FIFO_SIZE_STATUS(VID1) 00000400
DISPC_OVL_ROW_INC(VID1) 00000001
DISPC_OVL_PIXEL_INC(VID1) 00000001
DISPC_OVL_PRELOAD(VID1) 00000100
DISPC_OVL_FIR(VID1) 00000000
DISPC_OVL_PICTURE_SIZE(VID1) 00000000
DISPC_OVL_ACCU0(VID1) 00000000
DISPC_OVL_ACCU1(VID1) 00000000
DISPC_OVL_PRELOAD(VID1) 00000100
DISPC_OVL_BA0(VID2) 00000000
DISPC_OVL_BA1(VID2) 00000000
DISPC_OVL_POSITION(VID2) 00000000
DISPC_OVL_SIZE(VID2) 00000000
DISPC_OVL_ATTRIBUTES(VID2) 00008000
DISPC_OVL_FIFO_THRESHOLD(VID2) 03ff03c0
DISPC_OVL_FIFO_SIZE_STATUS(VID2) 00000400
DISPC_OVL_ROW_INC(VID2) 00000001
DISPC_OVL_PIXEL_INC(VID2) 00000001
DISPC_OVL_PRELOAD(VID2) 00000100
DISPC_OVL_FIR(VID2) 00000000
DISPC_OVL_PICTURE_SIZE(VID2) 00000000
DISPC_OVL_ACCU0(VID2) 00000000
DISPC_OVL_ACCU1(VID2) 00000000
DISPC_OVL_PRELOAD(VID2) 00000100
DISPC_OVL_FIR_COEF_H_0(VID1) 00000000
DISPC_OVL_FIR_COEF_H_1(VID1) 00000000
DISPC_OVL_FIR_COEF_H_2(VID1) 00000000
DISPC_OVL_FIR_COEF_H_3(VID1) 00000000
DISPC_OVL_FIR_COEF_H_4(VID1) 00000000
DISPC_OVL_FIR_COEF_H_5(VID1) 00000000
DISPC_OVL_FIR_COEF_H_6(VID1) 00000000
DISPC_OVL_FIR_COEF_H_7(VID1) 00000000
DISPC_OVL_FIR_COEF_HV_0(VID1) 00000000
DISPC_OVL_FIR_COEF_HV_1(VID1) 00000000
DISPC_OVL_FIR_COEF_HV_2(VID1) 00000000
DISPC_OVL_FIR_COEF_HV_3(VID1) 00000000
DISPC_OVL_FIR_COEF_HV_4(VID1) 00000000
DISPC_OVL_FIR_COEF_HV_5(VID1) 00000000
DISPC_OVL_FIR_COEF_HV_6(VID1) 00000000
DISPC_OVL_FIR_COEF_HV_7(VID1) 00000000
DISPC_OVL_CONV_COEF_0(VID1) 0199012a
DISPC_OVL_CONV_COEF_1(VID1) 012a0000
DISPC_OVL_CONV_COEF_2(VID1) 079c0730
DISPC_OVL_CONV_COEF_3(VID1) 0000012a
DISPC_OVL_CONV_COEF_4(VID1) 00000205
DISPC_OVL_FIR_COEF_V_0(VID1) 00000000
DISPC_OVL_FIR_COEF_V_1(VID1) 00000000
DISPC_OVL_FIR_COEF_V_2(VID1) 00000000
DISPC_OVL_FIR_COEF_V_3(VID1) 00000000
DISPC_OVL_FIR_COEF_V_4(VID1) 00000000
DISPC_OVL_FIR_COEF_V_5(VID1) 00000000
DISPC_OVL_FIR_COEF_V_6(VID1) 00000000
DISPC_OVL_FIR_COEF_V_7(VID1) 00000000
DISPC_OVL_FIR_COEF_H_0(VID2) 00000000
DISPC_OVL_FIR_COEF_H_1(VID2) 00000000
DISPC_OVL_FIR_COEF_H_2(VID2) 00000000
DISPC_OVL_FIR_COEF_H_3(VID2) 00000000
DISPC_OVL_FIR_COEF_H_4(VID2) 00000000
DISPC_OVL_FIR_COEF_H_5(VID2) 00000000
DISPC_OVL_FIR_COEF_H_6(VID2) 00000000
DISPC_OVL_FIR_COEF_H_7(VID2) 00000000
DISPC_OVL_FIR_COEF_HV_0(VID2) 00000000
DISPC_OVL_FIR_COEF_HV_1(VID2) 00000000
DISPC_OVL_FIR_COEF_HV_2(VID2) 00000000
DISPC_OVL_FIR_COEF_HV_3(VID2) 00000000
DISPC_OVL_FIR_COEF_HV_4(VID2) 00000000
DISPC_OVL_FIR_COEF_HV_5(VID2) 00000000
DISPC_OVL_FIR_COEF_HV_6(VID2) 00000000
DISPC_OVL_FIR_COEF_HV_7(VID2) 00000000
DISPC_OVL_CONV_COEF_0(VID2) 0199012a
DISPC_OVL_CONV_COEF_1(VID2) 012a0000
DISPC_OVL_CONV_COEF_2(VID2) 079c0730
DISPC_OVL_CONV_COEF_3(VID2) 0000012a
DISPC_OVL_CONV_COEF_4(VID2) 00000205
DISPC_OVL_FIR_COEF_V_0(VID2) 00000000
DISPC_OVL_FIR_COEF_V_1(VID2) 00000000
DISPC_OVL_FIR_COEF_V_2(VID2) 00000000
DISPC_OVL_FIR_COEF_V_3(VID2) 00000000
DISPC_OVL_FIR_COEF_V_4(VID2) 00000000
DISPC_OVL_FIR_COEF_V_5(VID2) 00000000
DISPC_OVL_FIR_COEF_V_6(VID2) 00000000
DISPC_OVL_FIR_COEF_V_7(VID2) 00000000
root@am43xx-epos-evm:~#
With Reserved memory for FB:
==============
root@am43xx-epos-evm:~# cat /proc/cmdline
console=ttyO0,115200n8 root=/dev/mmcblk0p2 rw rootfstype=ext3 rootwait mem\x128M consoleblank=0 clocksource=gp_timer consoleblank=0 earlyprintk omapfb_vram\x10M@0xA0000000
root@am43xx-epos-evm:~#
root@am43xx-epos-evm:~#
root@am43xx-epos-evm:~#
root@am43xx-epos-evm:~# cat /sys/kernel/debug/omapdss/dss
DSS_REVISION 00000020
DSS_SYSCONFIG 00000001
DSS_SYSSTATUS 00000001
DSS_CONTROL 00000018
root@am43xx-epos-evm:~#
root@am43xx-epos-evm:~# cat /sys/kernel/debug/omapdss/clk
- DSS -
DSS_FCK (DSS_FCLK1) = 198000000
- DISPC -
dispc fclk source = DSS_FCK (DSS_FCLK1)
fck 198000000
- LCD -
LCD clk source = DSS_FCK (DSS_FCLK1)
lck 198000000 lck div 1
pck 33000000 pck div 6
root@am43xx-epos-evm:~#
root@am43xx-epos-evm:~# cat /sys/kernel/debug/omapdss/dispc_irq
period 48010 ms
irqs 2
FRAMEDONE 0
VSYNC 2
EVSYNC_EVEN 2
EVSYNC_ODD 2
ACBIAS_COUNT_STAT 0
PROG_LINE_NUM 2
GFX_FIFO_UNDERFLOW 2
GFX_END_WIN 2
PAL_GAMMA_MASK 0
OCP_ERR 0
VID1_FIFO_UNDERFLOW 0
VID1_END_WIN 0
VID2_FIFO_UNDERFLOW 0
VID2_END_WIN 0
SYNC_LOST 0
SYNC_LOST_DIGIT 0
WAKEUP 0
root@am43xx-epos-evm:~#
root@am43xx-epos-evm:~#
root@am43xx-epos-evm:~#
root@am43xx-epos-evm:~# cat /sys/kernel/debug/omapdss/dispc
DISPC_REVISION 00000030
DISPC_SYSCONFIG 00002011
DISPC_SYSSTATUS 00000001
DISPC_IRQSTATUS 0000002e
DISPC_IRQENABLE 0000d640
DISPC_CONTROL 00018309
DISPC_CONFIG 00000204
DISPC_CAPABLE 000003ff
DISPC_LINE_STATUS 0000011d
DISPC_LINE_NUMBER 00000000
DISPC_GLOBAL_ALPHA 000000ff
DISPC_DEFAULT_COLOR(LCD) 00000000
DISPC_TRANS_COLOR(LCD) 00000000
DISPC_SIZE_MGR(LCD) 01df031f
DISPC_DEFAULT_COLOR(LCD) 00000000
DISPC_TRANS_COLOR(LCD) 00000000
DISPC_TIMING_H(LCD) 00f0d11d
DISPC_TIMING_V(LCD) 00a0160c
DISPC_POL_FREQ(LCD) 00003000
DISPC_DIVISORo(LCD) 00010006
DISPC_SIZE_MGR(LCD) 01df031f
DISPC_DATA_CYCLE1(LCD) 00000000
DISPC_DATA_CYCLE2(LCD) 00000000
DISPC_DATA_CYCLE3(LCD) 00000000
DISPC_CPR_COEF_R(LCD) 00000000
DISPC_CPR_COEF_G(LCD) 00000000
DISPC_CPR_COEF_B(LCD) 00000000
DISPC_OVL_BA0(GFX) a0000000
DISPC_OVL_BA1(GFX) a0000000
DISPC_OVL_POSITION(GFX) 00000000
DISPC_OVL_SIZE(GFX) 01df031f
DISPC_OVL_ATTRIBUTES(GFX) 00000090
DISPC_OVL_FIFO_THRESHOLD(GFX) 03ff03c0
DISPC_OVL_FIFO_SIZE_STATUS(GFX) 00000400
DISPC_OVL_ROW_INC(GFX) 00000001
DISPC_OVL_PIXEL_INC(GFX) 00000001
DISPC_OVL_PRELOAD(GFX) 00000100
DISPC_OVL_WINDOW_SKIP(GFX) 00000000
DISPC_OVL_TABLE_BA(GFX) 00000000
DISPC_OVL_BA0(VID1) 00000000
DISPC_OVL_BA1(VID1) 00000000
DISPC_OVL_POSITION(VID1) 00000000
DISPC_OVL_SIZE(VID1) 00000000
DISPC_OVL_ATTRIBUTES(VID1) 00008000
DISPC_OVL_FIFO_THRESHOLD(VID1) 03ff03c0
DISPC_OVL_FIFO_SIZE_STATUS(VID1) 00000400
DISPC_OVL_ROW_INC(VID1) 00000001
DISPC_OVL_PIXEL_INC(VID1) 00000001
DISPC_OVL_PRELOAD(VID1) 00000100
DISPC_OVL_FIR(VID1) 00000000
DISPC_OVL_PICTURE_SIZE(VID1) 00000000
DISPC_OVL_ACCU0(VID1) 00000000
DISPC_OVL_ACCU1(VID1) 00000000
DISPC_OVL_PRELOAD(VID1) 00000100
DISPC_OVL_BA0(VID2) 00000000
DISPC_OVL_BA1(VID2) 00000000
DISPC_OVL_POSITION(VID2) 00000000
DISPC_OVL_SIZE(VID2) 00000000
DISPC_OVL_ATTRIBUTES(VID2) 00008000
DISPC_OVL_FIFO_THRESHOLD(VID2) 03ff03c0
DISPC_OVL_FIFO_SIZE_STATUS(VID2) 00000400
DISPC_OVL_ROW_INC(VID2) 00000001
DISPC_OVL_PIXEL_INC(VID2) 00000001
DISPC_OVL_PRELOAD(VID2) 00000100
DISPC_OVL_FIR(VID2) 00000000
DISPC_OVL_PICTURE_SIZE(VID2) 00000000
DISPC_OVL_ACCU0(VID2) 00000000
DISPC_OVL_ACCU1(VID2) 00000000
DISPC_OVL_PRELOAD(VID2) 00000100
DISPC_OVL_FIR_COEF_H_0(VID1) 00000000
DISPC_OVL_FIR_COEF_H_1(VID1) 00000000
DISPC_OVL_FIR_COEF_H_2(VID1) 00000000
DISPC_OVL_FIR_COEF_H_3(VID1) 00000000
DISPC_OVL_FIR_COEF_H_4(VID1) 00000000
DISPC_OVL_FIR_COEF_H_5(VID1) 00000000
DISPC_OVL_FIR_COEF_H_6(VID1) 00000000
DISPC_OVL_FIR_COEF_H_7(VID1) 00000000
DISPC_OVL_FIR_COEF_HV_0(VID1) 00000000
DISPC_OVL_FIR_COEF_HV_1(VID1) 00000000
DISPC_OVL_FIR_COEF_HV_2(VID1) 00000000
DISPC_OVL_FIR_COEF_HV_3(VID1) 00000000
DISPC_OVL_FIR_COEF_HV_4(VID1) 00000000
DISPC_OVL_FIR_COEF_HV_5(VID1) 00000000
DISPC_OVL_FIR_COEF_HV_6(VID1) 00000000
DISPC_OVL_FIR_COEF_HV_7(VID1) 00000000
DISPC_OVL_CONV_COEF_0(VID1) 0199012a
DISPC_OVL_CONV_COEF_1(VID1) 012a0000
DISPC_OVL_CONV_COEF_2(VID1) 079c0730
DISPC_OVL_CONV_COEF_3(VID1) 0000012a
DISPC_OVL_CONV_COEF_4(VID1) 00000205
DISPC_OVL_FIR_COEF_V_0(VID1) 00000000
DISPC_OVL_FIR_COEF_V_1(VID1) 00000000
DISPC_OVL_FIR_COEF_V_2(VID1) 00000000
DISPC_OVL_FIR_COEF_V_3(VID1) 00000000
DISPC_OVL_FIR_COEF_V_4(VID1) 00000000
DISPC_OVL_FIR_COEF_V_5(VID1) 00000000
DISPC_OVL_FIR_COEF_V_6(VID1) 00000000
DISPC_OVL_FIR_COEF_V_7(VID1) 00000000
DISPC_OVL_FIR_COEF_H_0(VID2) 00000000
DISPC_OVL_FIR_COEF_H_1(VID2) 00000000
DISPC_OVL_FIR_COEF_H_2(VID2) 00000000
DISPC_OVL_FIR_COEF_H_3(VID2) 00000000
DISPC_OVL_FIR_COEF_H_4(VID2) 00000000
DISPC_OVL_FIR_COEF_H_5(VID2) 00000000
DISPC_OVL_FIR_COEF_H_6(VID2) 00000000
DISPC_OVL_FIR_COEF_H_7(VID2) 00000000
DISPC_OVL_FIR_COEF_HV_0(VID2) 00000000
DISPC_OVL_FIR_COEF_HV_1(VID2) 00000000
DISPC_OVL_FIR_COEF_HV_2(VID2) 00000000
DISPC_OVL_FIR_COEF_HV_3(VID2) 00000000
DISPC_OVL_FIR_COEF_HV_4(VID2) 00000000
DISPC_OVL_FIR_COEF_HV_5(VID2) 00000000
DISPC_OVL_FIR_COEF_HV_6(VID2) 00000000
DISPC_OVL_FIR_COEF_HV_7(VID2) 00000000
DISPC_OVL_CONV_COEF_0(VID2) 0199012a
DISPC_OVL_CONV_COEF_1(VID2) 012a0000
DISPC_OVL_CONV_COEF_2(VID2) 079c0730
DISPC_OVL_CONV_COEF_3(VID2) 0000012a
DISPC_OVL_CONV_COEF_4(VID2) 00000205
DISPC_OVL_FIR_COEF_V_0(VID2) 00000000
DISPC_OVL_FIR_COEF_V_1(VID2) 00000000
DISPC_OVL_FIR_COEF_V_2(VID2) 00000000
DISPC_OVL_FIR_COEF_V_3(VID2) 00000000
DISPC_OVL_FIR_COEF_V_4(VID2) 00000000
DISPC_OVL_FIR_COEF_V_5(VID2) 00000000
DISPC_OVL_FIR_COEF_V_6(VID2) 00000000
DISPC_OVL_FIR_COEF_V_7(VID2) 00000000
root@am43xx-epos-evm:~#
^ permalink raw reply
* Re: [PATCH] fbdev: sh_mipi_dsi/sh_mobile_hdmi: clk_round_rate() can return a zero upon error
From: Tomi Valkeinen @ 2014-01-09 13:03 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <alpine.DEB.2.02.1311261651300.11450@tamien>
[-- Attachment #1: Type: text/plain, Size: 1221 bytes --]
On 2013-11-27 02:53, Paul Walmsley wrote:
>
> Treat both negative and zero return values from clk_round_rate() as
> errors. This is needed since subsequent patches will convert
> clk_round_rate()'s return value to be an unsigned type, rather than a
> signed type, since some clock sources can generate rates higher than
> (2^31)-1 Hz.
>
> Eventually, when calling clk_round_rate(), only a return value of zero
> will be considered a error. All other values will be considered valid
> rates. The comparison against values less than 0 is kept to preserve
> the correct behavior in the meantime.
>
> Signed-off-by: Paul Walmsley <pwalmsley@nvidia.com>
> Cc: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> Cc: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
> Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> ---
>
> Applies on v3.13-rc1. See also:
I'm not able to apply this, I'm just getting "patch doesn't apply". I
tried both the patch saved from my email client, and the patch from
patchworks...
Can you rebase to, say, rc6, and resend?
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]
^ permalink raw reply
* Re: [PATCH] video: mmp: add device tree support
From: 'Sascha Hauer' @ 2014-01-10 9:07 UTC (permalink / raw)
To: Zhou Zhu
Cc: Jingoo Han, linux-fbdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
'Tomi Valkeinen',
'Jean-Christophe Plagniol-Villard',
'Haojian Zhuang', Chao Xie, Guoqing Li,
devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <52CE9165.7050002-eYqpPyKDWXRBDgjK7y7TUQ@public.gmane.org>
On Thu, Jan 09, 2014 at 08:09:09PM +0800, Zhou Zhu wrote:
> Sascha/Jingoo,
> Thank you for your review!
>
> On 01/09/2014 03:43 PM, Jingoo Han wrote:
> >On Thursday, January 09, 2014 4:32 PM, Sascha Hauer wrote:
> >>On Thu, Jan 09, 2014 at 01:13:14PM +0800, Zhou Zhu wrote:
> >>>add device tree support for mmp fb/controller
> >>>the description at Documentation/devicetree/bindings/fb/mmp-disp.txt
> >>>
> >>>Signed-off-by: Zhou Zhu <zzhu3@marvell.com>
> >>>---
> >>> Documentation/devicetree/bindings/fb/mmp-disp.txt | 71 ++++++++++++
> >>> drivers/video/mmp/fb/mmpfb.c | 71 ++++++++----
> >>> drivers/video/mmp/hw/mmp_ctrl.c | 120 ++++++++++++++++-----
> >>> 3 files changed, 217 insertions(+), 45 deletions(-)
> >>> create mode 100644 Documentation/devicetree/bindings/fb/mmp-disp.txt
> >>>
>
> [...]
>
> >>+fb: fb {
> >>+ compatible = "marvell,mmp-fb";
> >
> >This compatible should have the specific SoC name in it, not just
> >'mmp'. Otherwise you can't properly distinguish between this version and
> >future versions of the mmp core.
> >
> We are using a same display IP for all mmp serial SoCs, and there
> would be inside register to get version. So I am planning put same
> compatible here for all SoCs using this IP.
> Would it be ok if I update compatible to "marvell,mmpdcx-fb"?
> "mmpdcx" is the IP name.
Using the SoC name is really good practice. This way you can add SoC
specific data to the compatible entry in the driver. Most bindings I've
seen which bind to some more generic string tend to add some additional
properties to the binding like marvell,has-new-feature or
marvell,needs-workaround-for-xy which you don't need at all if you bind
to the specific SoC in the first place.
If you insist on a generic binding you can still do a
compatible = "marvell,soc-xy-fb", "marvell,mmp-fb";
This way you at least have the SoC information around (even in old
devicetrees) should you need it in the future.
BTW I've seen often enough that the version register is at a different
location on newer generations which makes it pretty useless.
> >> + interrupts = <0 41 0x4>;
> >> + marvell,disp-name = "mmp_disp";
> >> + marvell,path-num = <1>;
> >> + marvell,clk-name = "LCDCIHCLK";
> >
> > Don't pass clk names like this. We have a documented clock binding, use
> > it.
> >
> The patches to add dt support in common clk in our platforms are not
> upstreamed yet. As there's only one clock in this device, could I
> remove clock name related codes and direct use: devm_clk_get(dev,
> NULL)?
The clock name is defined by the device you are implementing, *not*
the name of the clock in the SoC. So if you have:
----------------------
--------- LCDCIHCLK | MMP |
| OSC/PLL |-----------------------|>pixclk |
---------- | |
----------------------
You have to do clk_get(dev, "pixclk") because you want to get the
clock associated to the pixclk input. Doing clk_get(dev, "LCDCIHCLK")
is wrong because that may change in future SoCs.
If you do this correctly there is no need to put the clk name into the
devicetree. You only have to put the information "which clk is connected
to the pixclk input of the MMP" into the devicetree.
Sascha
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply
* Re: [PATCH v2] ARM: OMAPFB: panel-sony-acx565akm: fix missing mutex unlocks
From: Tomi Valkeinen @ 2014-01-10 10:56 UTC (permalink / raw)
To: Ivaylo Dimitrov
Cc: plagnioj, pali.rohar, pavel, linux-omap, linux-fbdev,
linux-kernel, Ivaylo Dimitrov, Aaro Koskinen
In-Reply-To: <1388927585-5640-1-git-send-email-ivo.g.dimitrov.75@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 3767 bytes --]
On 2014-01-05 15:13, Ivaylo Dimitrov wrote:
> From: Ivaylo Dimitrov <freemangordon@abv.bg>
>
> Commit c37dd677988ca50bc8bc60ab5ab053720583c168 fixes the unbalanced
> unlock in acx565akm_enable but introduces another problem - if
> acx565akm_panel_power_on exits early, the mutex is not unlocked. Fix
> that by unlocking the mutex on early return. Also add mutex protection in
> acx565akm_panel_power_off and remove an unused variable
>
I think this is just getting more messy. How about we more or less revert the c37dd677988ca50bc8bc60ab5ab053720583c168 and fix it like this:
diff --git a/drivers/video/omap2/displays-new/panel-sony-acx565akm.c b/drivers/video/omap2/displays-new/panel-sony-acx565akm.c
index 714ee92dfb9f..8e97d06921ff 100644
--- a/drivers/video/omap2/displays-new/panel-sony-acx565akm.c
+++ b/drivers/video/omap2/displays-new/panel-sony-acx565akm.c
@@ -346,28 +346,22 @@ static int acx565akm_get_actual_brightness(struct panel_drv_data *ddata)
static int acx565akm_bl_update_status(struct backlight_device *dev)
{
struct panel_drv_data *ddata = dev_get_drvdata(&dev->dev);
- int r;
int level;
dev_dbg(&ddata->spi->dev, "%s\n", __func__);
- mutex_lock(&ddata->mutex);
-
if (dev->props.fb_blank == FB_BLANK_UNBLANK &&
dev->props.power == FB_BLANK_UNBLANK)
level = dev->props.brightness;
else
level = 0;
- r = 0;
if (ddata->has_bc)
acx565akm_set_brightness(ddata, level);
else
- r = -ENODEV;
-
- mutex_unlock(&ddata->mutex);
+ return -ENODEV;
- return r;
+ return 0;
}
static int acx565akm_bl_get_intensity(struct backlight_device *dev)
@@ -390,9 +384,33 @@ static int acx565akm_bl_get_intensity(struct backlight_device *dev)
return 0;
}
+static int acx565akm_bl_update_status_locked(struct backlight_device *dev)
+{
+ struct panel_drv_data *ddata = dev_get_drvdata(&dev->dev);
+ int r;
+
+ mutex_lock(&ddata->mutex);
+ r = acx565akm_bl_update_status(dev);
+ mutex_unlock(&ddata->mutex);
+
+ return r;
+}
+
+static int acx565akm_bl_get_intensity_locked(struct backlight_device *dev)
+{
+ struct panel_drv_data *ddata = dev_get_drvdata(&dev->dev);
+ int r;
+
+ mutex_lock(&ddata->mutex);
+ r = acx565akm_bl_get_intensity(dev);
+ mutex_unlock(&ddata->mutex);
+
+ return r;
+}
+
static const struct backlight_ops acx565akm_bl_ops = {
- .get_brightness = acx565akm_bl_get_intensity,
- .update_status = acx565akm_bl_update_status,
+ .get_brightness = acx565akm_bl_get_intensity_locked,
+ .update_status = acx565akm_bl_update_status_locked,
};
/*--------------------Auto Brightness control via Sysfs---------------------*/
@@ -526,8 +544,6 @@ static int acx565akm_panel_power_on(struct omap_dss_device *dssdev)
struct omap_dss_device *in = ddata->in;
int r;
- mutex_lock(&ddata->mutex);
-
dev_dbg(&ddata->spi->dev, "%s\n", __func__);
in->ops.sdi->set_timings(in, &ddata->videomode);
@@ -568,8 +584,6 @@ static int acx565akm_panel_power_on(struct omap_dss_device *dssdev)
set_display_state(ddata, 1);
set_cabc_mode(ddata, ddata->cabc_mode);
- mutex_unlock(&ddata->mutex);
-
return acx565akm_bl_update_status(ddata->bl_dev);
}
@@ -605,6 +619,7 @@ static void acx565akm_panel_power_off(struct omap_dss_device *dssdev)
static int acx565akm_enable(struct omap_dss_device *dssdev)
{
+ struct panel_drv_data *ddata = to_panel_data(dssdev);
int r;
dev_dbg(dssdev->dev, "%s\n", __func__);
@@ -615,7 +630,9 @@ static int acx565akm_enable(struct omap_dss_device *dssdev)
if (omapdss_device_is_enabled(dssdev))
return 0;
+ mutex_lock(&ddata->mutex);
r = acx565akm_panel_power_on(dssdev);
+ mutex_unlock(&ddata->mutex);
if (r)
return r;
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]
^ permalink raw reply related
* Re: [PATCH] fbcon: Avoid corrupting active workqueue entries
From: Tomi Valkeinen @ 2014-01-10 11:24 UTC (permalink / raw)
To: Chris Wilson, intel-gfx
Cc: Jean-Christophe Plagniol-Villard, linux-fbdev, dri-devel, stable
In-Reply-To: <1387290459-26440-1-git-send-email-chris@chris-wilson.co.uk>
[-- Attachment #1: Type: text/plain, Size: 670 bytes --]
Hi,
On 2013-12-17 16:27, Chris Wilson wrote:
> We attempt to reschedule an active work which can itself corrupt the
> workqueue lists, and we may then free the work item whilst the task is
> still pending. Both of these may lead to a system deadlock and an
> unresponsive machine without any outputs for a panic to even be shown.
This gives the following warning:
drivers/video/console/fbcon.c:407:20: warning: unused variable ‘ops’
[-Wunused-variable]
I can fix that. Or maybe it's better if you send a v2 with that fixed,
so that this gets properly to stable-kernel.
However, I wouldn't mind an acked-by or reviewed-by from someone.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]
^ permalink raw reply
* Re: [PATCH] video: amba-clcd: Make CLCD driver available on more platforms
From: Tomi Valkeinen @ 2014-01-10 11:27 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1386868390-12231-1-git-send-email-broonie@kernel.org>
[-- Attachment #1: Type: text/plain, Size: 1068 bytes --]
On 2013-12-12 19:13, Mark Brown wrote:
> From: Mark Brown <broonie@linaro.org>
>
> The CLCD driver is used on ARM reference models for ARMv8 so add ARM64
> to the list of dependencies. The driver also has no build time dependencies
> on ARM (stubs are provided for ARM-specific DMA functions in the code) so
> make it available with COMPILE_TEST in order to maximise build coverage.
>
> Signed-off-by: Mark Brown <broonie@linaro.org>
> ---
> drivers/video/Kconfig | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
> index 4f2e1b35eb38..e6c7fb1a389b 100644
> --- a/drivers/video/Kconfig
> +++ b/drivers/video/Kconfig
> @@ -312,7 +312,8 @@ config FB_PM2_FIFO_DISCONNECT
>
> config FB_ARMCLCD
> tristate "ARM PrimeCell PL110 support"
> - depends on FB && ARM && ARM_AMBA
> + depends on ARM || ARM64 || COMPILE_TEST
> + depends on FB && ARM_AMBA
> select FB_CFB_FILLRECT
> select FB_CFB_COPYAREA
> select FB_CFB_IMAGEBLIT
>
Queued for 3.14.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox