* Re: [PATCH 3/4] OMAPDSS: panel-sharp-ls037v7dw01: add device tree support
From: Tony Lindgren @ 2014-05-15 18:21 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <537487AE.3060906@ti.com>
* Tomi Valkeinen <tomi.valkeinen@ti.com> [140515 02:24]:
> On 14/05/14 19:02, Tony Lindgren wrote:
>
> >> The video paths of the panels and encoders are connected using the v4l2
> >> style ports/endpoints. We can use those to see what display controller a
> >> panel is connected to, but only after the panel driver has already
> >> probed. We don't have control for the actual probing, as that happens
> >> with whatever the control bus is for the display component.
> >
> > OK. So with generic panels, you can just let the panels probe with
> > the right compatible flag then and let the ports/endpoints registration
> > to figure out if the panel is usable for the display controller in
> > question.
>
> I'm not sure what you mean here.
>
> Do you mean with the future common display framework? There's no need to
> figure out anything there, as supposedly the .dts has been written
> correctly and the panel and the display controller work together.
OK. Yes I meant for future use.
> If you mean with the current kernel, there's still nothing to figure
> out. We can have only single driver with a particular compatible string.
> And as our current drivers are omap specific, it makes sense to have
> their compatible string be something omap-ish. And if the .dts file
> connects the display controller and the panel, then they must be usable
> together.
Yup.
> >>> Well it seems at least the reset and enable pin standard from that
> >>> binding can be kept.
> >>
> >> Only enable gpio there. But even that's vague. Do you turn on the power
> >> before or after setting the enable gpio? How long delay should be
> >> between the power and the gpio? Different panels have different rules
> >> for the power-up.
> >
> > Sure, it's a complex problem. But for the enable gpio..
> >
> > Maybe the enable GPIO should be treated as a regulator? That would allow
> > specifying first the source regulator startup delay, and then the
> > panel has it's own startup delay.
>
> Well... I don't know. Sounds rather hacky to me. We have the option to
> have a specific driver for this panel, and that driver can handle all
> the delays and power-up sequences just right in a clean manner.
I guess we could have a generic startup-latency property for the GPIOs.
> >>>>> But I'm not really familiar with using extending current bindings, and
> >>>>> making new bindings compatible with old ones. Can you explain why it'd
> >>>>> be good to have the sharp panel bindings compatible with simple-panel?
> >>>>> In what kind of scenario would it be used?
> >>>
> >>> Ideally the panel binding just describes the panel and it should not
> >>> matter which display controller it is a child of.
> >>
> >> Yes, but that means the panel bindings need to have enough information
> >> so that all display controllers can use it. Simple-panel bindings do not
> >> have enough information. The simple-panel bindings do not have
> >> information about the video bus input, and it doesn't even have
> >> information about the resolution or bitdepth of the panel.
> >
> > Some of that you can hide into the panel driver based on the compatible
> > flag. So why not already do something like this in the .dts files
> > instead of the remapping:
> >
> > compatible = "sharp,ls037v7dw01-omap-dss", "sharp,ls037v7dw01";
> >
> > And drivers/video/fbdev/omap2/displays-new/panel-sharp-ls037v7dw01.c
> > would only claim to be compatible with "sharp,ls037v7dw01-omap-dss".
> >
> > Then when the common panel framework is available, you can stop
> > parsing sharp,ls037v7dw01-omap-dss but the .dts files don't need
> > to be changed and it's fine to keep "sharp,ls037v7dw01-omap-dss"
> > in the .dts files.
>
> Hmm, I don't see how this relates to the simple-panel bindings.
>
> But you're right, having "sharp,ls037v7dw01-omap-dss" in the .dts is an
> alternative for the compatible-string conversion we do now. I guess it's
> a matter of taste, but I rather hide it inside the kernel, in an
> internal omapdss file, than pollute the .dts files with those compatible
> strings.
Well it avoid you parsing through all the nodes during booting
and leaves out the function to do remapping. And removes the need
for maintaining a custom display mapping table. I'd say that's a
pretty good list of advantages right there :)
> >> So I'm still asking, if we create sharp bindings that use the same
> >> properties as the simple-panel bindings, and define that sharp panel is
> >> compatible with simple-panel, what kind of scenario in practice would it
> >> be used in?
> >
> > Well with the above example, just by dss with "sharp,ls037v7dw01-omap-dss"
> > until some other SoC notices it can use the GPIO parts of the panel
> > code at least :)
> >
> >> Would the scenario be some other OS, that doesn't have a driver for the
> >> sharp panel, but has a driver for the simple-panel? That would only work
> >> if the sharp panel hardware is setup so that only the enable gpio is
> >> needed, so that quite a narrow definition of "compatible".
> >
> > That's where we can use the compatible flags and just avoid parsing
> > the generic compatible flag unless some common framework is available.
>
> Hmm, sorry, I don't follow. My question was about the simple-panel
> bindings, not common display framework.
>
> You were saying that the sharp bindings should be compatible with
> simple-panel bindings. I'm still trying to understand what benefit does
> that give us.
Oh sorry, for that part I don't really have an opinion. If you think
the simple-panel binding is not going to be usable in the long
run, then I'm fine with whatever binding you display guys come up
with and prefer to use.
> As I see it, the sharp panel could be used with the simple-panel
> bindings only in certain special case, where all the mode pins and the
> reset are hardwired in the board hardware, and they are hardwired in a
> certain state (all hardwired low, probably), which matches what the
> simple-panel driver expects.
OK. Maybe take a stab at updating the binding for this as I don't
know what you want to do with it?
Regards,
Tony
^ permalink raw reply
* Re: [PATCH] OMAPDSS: move 'compatible' converter to omapdss driver
From: Tony Lindgren @ 2014-05-15 18:13 UTC (permalink / raw)
To: Tomi Valkeinen; +Cc: linux-omap, linux-fbdev, Archit Taneja
In-Reply-To: <1400141136-25358-1-git-send-email-tomi.valkeinen@ti.com>
* Tomi Valkeinen <tomi.valkeinen@ti.com> [140515 01:06]:
> Move the panel/encoder driver compatible-string converter from
> arch/arm/mach-omap2/display.c to omapdss driver. That is a more logical
> place for it, as it's really an omapdss internal hack.
Thanks, maybe also consider just using the compatible flags
instead as discussed in the other thread. Anyways for moving
this out of mach-omap2:
Acked-by: Tony Lindgren <tony@atomide.com>
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> ---
> arch/arm/mach-omap2/display.c | 56 -----------
> drivers/video/fbdev/omap2/Makefile | 2 +-
> drivers/video/fbdev/omap2/dss/Kconfig | 4 +
> drivers/video/fbdev/omap2/dss/Makefile | 1 +
> drivers/video/fbdev/omap2/dss/omapdss-boot-init.c | 112 ++++++++++++++++++++++
> 5 files changed, 118 insertions(+), 57 deletions(-)
> create mode 100644 drivers/video/fbdev/omap2/dss/omapdss-boot-init.c
>
> diff --git a/arch/arm/mach-omap2/display.c b/arch/arm/mach-omap2/display.c
> index 16d33d831287..519a20fc0432 100644
> --- a/arch/arm/mach-omap2/display.c
> +++ b/arch/arm/mach-omap2/display.c
> @@ -555,65 +555,9 @@ int omap_dss_reset(struct omap_hwmod *oh)
> return r;
> }
>
> -/* list of 'compatible' nodes to convert to omapdss specific */
> -static const char * const dss_compat_conv_list[] __initconst = {
> - "composite-connector",
> - "dvi-connector",
> - "hdmi-connector",
> - "panel-dpi",
> - "panel-dsi-cm",
> - "sony,acx565akm",
> - "svideo-connector",
> - "ti,tfp410",
> - "ti,tpd12s015",
> -};
> -
> -/* prepend compatible string with "omapdss," */
> -static __init void omapdss_omapify_node(struct device_node *node,
> - const char *compat)
> -{
> - char *new_compat;
> - struct property *prop;
> -
> - new_compat = kasprintf(GFP_KERNEL, "omapdss,%s", compat);
> -
> - prop = kzalloc(sizeof(*prop), GFP_KERNEL);
> -
> - if (!prop) {
> - pr_err("omapdss_omapify_node: kzalloc failed\n");
> - return;
> - }
> -
> - prop->name = "compatible";
> - prop->value = new_compat;
> - prop->length = strlen(new_compat) + 1;
> -
> - of_update_property(node, prop);
> -}
> -
> -/*
> - * As omapdss panel drivers are omapdss specific, but we want to define the
> - * DT-data in generic manner, we convert the compatible strings of the panel
> - * nodes from "panel-foo" to "omapdss,panel-foo". This way we can have both
> - * correct DT data and omapdss specific drivers.
> - *
> - * When we get generic panel drivers to the kernel, this will be removed.
> - */
> void __init omapdss_early_init_of(void)
> {
> - int i;
> -
> - for (i = 0; i < ARRAY_SIZE(dss_compat_conv_list); ++i) {
> - const char *compat = dss_compat_conv_list[i];
> - struct device_node *node = NULL;
> -
> - while ((node = of_find_compatible_node(node, NULL, compat))) {
> - if (!of_device_is_available(node))
> - continue;
>
> - omapdss_omapify_node(node, compat);
> - }
> - }
> }
>
> struct device_node * __init omapdss_find_dss_of_node(void)
> diff --git a/drivers/video/fbdev/omap2/Makefile b/drivers/video/fbdev/omap2/Makefile
> index bf8127df8c71..f8745ec369cc 100644
> --- a/drivers/video/fbdev/omap2/Makefile
> +++ b/drivers/video/fbdev/omap2/Makefile
> @@ -1,5 +1,5 @@
> obj-$(CONFIG_OMAP2_VRFB) += vrfb.o
>
> -obj-$(CONFIG_OMAP2_DSS) += dss/
> +obj-y += dss/
> obj-y += displays-new/
> obj-$(CONFIG_FB_OMAP2) += omapfb/
> diff --git a/drivers/video/fbdev/omap2/dss/Kconfig b/drivers/video/fbdev/omap2/dss/Kconfig
> index dde4281663b1..a28f3a39ab1b 100644
> --- a/drivers/video/fbdev/omap2/dss/Kconfig
> +++ b/drivers/video/fbdev/omap2/dss/Kconfig
> @@ -1,6 +1,10 @@
> +config OMAP2_DSS_INIT
> + bool
> +
> menuconfig OMAP2_DSS
> tristate "OMAP2+ Display Subsystem support"
> select VIDEOMODE_HELPERS
> + select OMAP2_DSS_INIT
> help
> OMAP2+ Display Subsystem support.
>
> diff --git a/drivers/video/fbdev/omap2/dss/Makefile b/drivers/video/fbdev/omap2/dss/Makefile
> index 8aec8bda27cc..3b79ad74f2e1 100644
> --- a/drivers/video/fbdev/omap2/dss/Makefile
> +++ b/drivers/video/fbdev/omap2/dss/Makefile
> @@ -1,3 +1,4 @@
> +obj-$(CONFIG_OMAP2_DSS_INIT) += omapdss-boot-init.o
> obj-$(CONFIG_OMAP2_DSS) += omapdss.o
> # Core DSS files
> omapdss-y := core.o dss.o dss_features.o dispc.o dispc_coefs.o display.o \
> diff --git a/drivers/video/fbdev/omap2/dss/omapdss-boot-init.c b/drivers/video/fbdev/omap2/dss/omapdss-boot-init.c
> new file mode 100644
> index 000000000000..468f6eff370c
> --- /dev/null
> +++ b/drivers/video/fbdev/omap2/dss/omapdss-boot-init.c
> @@ -0,0 +1,112 @@
> +/*
> + * Copyright (C) 2014 Texas Instruments
> + * Author: Tomi Valkeinen <tomi.valkeinen@nokia.com>
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License version 2 as published by
> + * the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
> + * more details.
> + *
> + * You should have received a copy of the GNU General Public License along with
> + * this program. If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/of.h>
> +#include <linux/slab.h>
> +
> +/* list of 'compatible' nodes to convert to omapdss specific */
> +static const char * const dss_compat_conv_list[] __initconst = {
> + "composite-connector",
> + "dvi-connector",
> + "hdmi-connector",
> + "panel-dpi",
> + "panel-dsi-cm",
> + "sony,acx565akm",
> + "svideo-connector",
> + "ti,tfp410",
> + "ti,tpd12s015",
> +};
> +
> +/* prepend compatible string with "omapdss," */
> +static __init void omapdss_omapify_node(struct device_node *node,
> + const char *compat)
> +{
> + char *new_compat;
> + struct property *prop;
> +
> + new_compat = kasprintf(GFP_KERNEL, "omapdss,%s", compat);
> +
> + prop = kzalloc(sizeof(*prop), GFP_KERNEL);
> +
> + if (!prop) {
> + pr_err("omapdss_omapify_node: kzalloc failed\n");
> + return;
> + }
> +
> + prop->name = "compatible";
> + prop->value = new_compat;
> + prop->length = strlen(new_compat) + 1;
> +
> + of_update_property(node, prop);
> +}
> +
> +static struct device_node * __init omapdss_find_dss_of_node(void)
> +{
> + struct device_node *node;
> +
> + node = of_find_compatible_node(NULL, NULL, "ti,omap2-dss");
> + if (node)
> + return node;
> +
> + node = of_find_compatible_node(NULL, NULL, "ti,omap3-dss");
> + if (node)
> + return node;
> +
> + node = of_find_compatible_node(NULL, NULL, "ti,omap4-dss");
> + if (node)
> + return node;
> +
> + node = of_find_compatible_node(NULL, NULL, "ti,omap5-dss");
> + if (node)
> + return node;
> +
> + return NULL;
> +}
> +
> +/*
> + * As omapdss panel drivers are omapdss specific, but we want to define the
> + * DT-data in generic manner, we convert the compatible strings of the panel
> + * nodes from "panel-foo" to "omapdss,panel-foo". This way we can have both
> + * correct DT data and omapdss specific drivers.
> + *
> + * When we get generic panel drivers to the kernel, this will be removed.
> + */
> +static int __init omap_dss_boot_init(void)
> +{
> + int i;
> +
> + /* do we have omapdss device? */
> + if (omapdss_find_dss_of_node() = NULL)
> + return 0;
> +
> + for (i = 0; i < ARRAY_SIZE(dss_compat_conv_list); ++i) {
> + const char *compat = dss_compat_conv_list[i];
> + struct device_node *node = NULL;
> +
> + while ((node = of_find_compatible_node(node, NULL, compat))) {
> + if (!of_device_is_available(node))
> + continue;
> +
> + omapdss_omapify_node(node, compat);
> + }
> + }
> +
> + return 0;
> +}
> +
> +subsys_initcall(omap_dss_boot_init);
> --
> 1.9.1
>
^ permalink raw reply
* Re: [PATCH 3/4] OMAPDSS: panel-sharp-ls037v7dw01: add device tree support
From: Tomi Valkeinen @ 2014-05-15 13:07 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140513212639.GA18001@atomide.com>
[-- Attachment #1: Type: text/plain, Size: 963 bytes --]
On 14/05/14 00:26, Tony Lindgren wrote:
> +static int sharp_ls_probe_of(struct platform_device *pdev)
> +{
> + struct panel_drv_data *ddata = platform_get_drvdata(pdev);
> + struct device_node *node = pdev->dev.of_node;
> + struct omap_dss_device *in;
> +
> + ddata->vcc = devm_regulator_get(&pdev->dev, "envdd");
> + if (IS_ERR(ddata->vcc)) {
> + dev_err(&pdev->dev, "failed to get regulator\n");
> + return PTR_ERR(ddata->vcc);
> + }
> +
> + /* lcd INI */
> + ddata->ini_gpio = sharp_ls_get_gpio_of(&pdev->dev, 0, 0, "enable");
> + if (PTR_ERR(ddata->ini_gpio) == -EPROBE_DEFER)
> + return -EPROBE_DEFER;
Hmm, the GPIOs are optional, but shouldn't we react somehow to real
errors? I guess we should do something like:
ddata->ini_gpio = sharp_ls_get_gpio_of(&pdev->dev, 0, 0, "enable");
if (IS_ERR(ddata->ini_gpio) {
int err = PTR_ERR(ddata->ini_gpio);
if (err == -EPROBE_DEFER || err != -ENOENT)
return err;
}
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* [PATCH] matroxfb: perform a dummy read of M_STATUS
From: Mikulas Patocka @ 2014-05-15 10:58 UTC (permalink / raw)
To: linux-fbdev
I had occasional screen corruption with the matrox framebuffer driver and
I found out that the reason for the corruption is that the hardware
blitter accesses the videoram while it is being written to.
The matrox driver has a macro WaitTillIdle() that should wait until the
blitter is idle, but it sometimes doesn't work. I added a dummy read
mga_inl(M_STATUS) to WaitTillIdle() to fix the problem. The dummy read
will flush the write buffer in the PCI chipset, and the next read of
M_STATUS will return the hardware status.
Since applying this patch, I had no screen corruption at all.
Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
Cc: stable@vger.kernel.org
---
drivers/video/matrox/matroxfb_base.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
Index: linux-3.14/drivers/video/matrox/matroxfb_base.h
=================================--- linux-3.14.orig/drivers/video/matrox/matroxfb_base.h 2014-04-08 00:35:19.460051657 +0200
+++ linux-3.14/drivers/video/matrox/matroxfb_base.h 2014-05-03 18:38:18.882396833 +0200
@@ -698,7 +698,7 @@ void matroxfb_unregister_driver(struct m
#define mga_fifo(n) do {} while ((mga_inl(M_FIFOSTATUS) & 0xFF) < (n))
-#define WaitTillIdle() do {} while (mga_inl(M_STATUS) & 0x10000)
+#define WaitTillIdle() do { mga_inl(M_STATUS); do {} while (mga_inl(M_STATUS) & 0x10000); } while (0)
/* code speedup */
#ifdef CONFIG_FB_MATROX_MILLENIUM
^ permalink raw reply
* Re: [PATCH] video: of: display_timing: remove two unsafe error messages
From: Tomi Valkeinen @ 2014-05-15 10:47 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1399985901-30756-1-git-send-email-l.stach@pengutronix.de>
[-- Attachment #1: Type: text/plain, Size: 463 bytes --]
On 13/05/14 15:58, Lucas Stach wrote:
> The error message would try to dereference the pointer that
> just has been tested to be NULL. As those messages don't
> really add any value without the info that the np could
> provide, just remove them.
>
> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> ---
> drivers/video/of_display_timing.c | 8 ++------
> 1 file changed, 2 insertions(+), 6 deletions(-)
Thanks, queued for 3.16.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* [PATCH 3/3] Doc/DT: Add binding doc for lgphilips,lb035q02.txt
From: Tomi Valkeinen @ 2014-05-15 10:10 UTC (permalink / raw)
To: linux-fbdev, linux-omap
Cc: Archit Taneja, Florian Vaussard, Tomi Valkeinen, devicetree
In-Reply-To: <1400148637-17726-1-git-send-email-tomi.valkeinen@ti.com>
Add DT bindings documentation for LG.Philips LB035Q02 LCD panel.
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: devicetree@vger.kernel.org
---
.../bindings/video/lgphilips,lb035q02.txt | 33 ++++++++++++++++++++++
1 file changed, 33 insertions(+)
create mode 100644 Documentation/devicetree/bindings/video/lgphilips,lb035q02.txt
diff --git a/Documentation/devicetree/bindings/video/lgphilips,lb035q02.txt b/Documentation/devicetree/bindings/video/lgphilips,lb035q02.txt
new file mode 100644
index 000000000000..1a1e653e5407
--- /dev/null
+++ b/Documentation/devicetree/bindings/video/lgphilips,lb035q02.txt
@@ -0,0 +1,33 @@
+LG.Philips LB035Q02 Panel
+============+
+Required properties:
+- compatible: "lgphilips,lb035q02"
+- enable-gpios: panel enable gpio
+
+Optional properties:
+- label: a symbolic name for the panel
+
+Required nodes:
+- Video port for DPI input
+
+Example
+-------
+
+lcd-panel: panel@0 {
+ compatible = "lgphilips,lb035q02";
+ reg = <0>;
+ spi-max-frequency = <100000>;
+ spi-cpol;
+ spi-cpha;
+
+ label = "lcd";
+
+ enable-gpios = <&gpio7 7 0>;
+
+ port {
+ lcd_in: endpoint {
+ remote-endpoint = <&dpi_out>;
+ };
+ };
+};
--
1.9.1
^ permalink raw reply related
* [PATCH 2/3] OMAPDSS: panel-lgphilips-lb035q02: Add DT support
From: Tomi Valkeinen @ 2014-05-15 10:10 UTC (permalink / raw)
To: linux-fbdev, linux-omap; +Cc: Archit Taneja, Florian Vaussard, Tomi Valkeinen
In-Reply-To: <1400148637-17726-1-git-send-email-tomi.valkeinen@ti.com>
Add DT support for panel-lgphilips-lb035q02.
We disable the use of the backlight_gpio as it should be handled via
backlight framework with DT boots.
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
.../omap2/displays-new/panel-lgphilips-lb035q02.c | 44 +++++++++++++++++++++-
1 file changed, 43 insertions(+), 1 deletion(-)
diff --git a/drivers/video/fbdev/omap2/displays-new/panel-lgphilips-lb035q02.c b/drivers/video/fbdev/omap2/displays-new/panel-lgphilips-lb035q02.c
index 6e6acd696a70..76c4fdc51c31 100644
--- a/drivers/video/fbdev/omap2/displays-new/panel-lgphilips-lb035q02.c
+++ b/drivers/video/fbdev/omap2/displays-new/panel-lgphilips-lb035q02.c
@@ -159,7 +159,8 @@ static int lb035q02_enable(struct omap_dss_device *dssdev)
if (omapdss_device_is_enabled(dssdev))
return 0;
- in->ops.dpi->set_data_lines(in, ddata->data_lines);
+ if (ddata->data_lines)
+ in->ops.dpi->set_data_lines(in, ddata->data_lines);
in->ops.dpi->set_timings(in, &ddata->videomode);
r = in->ops.dpi->enable(in);
@@ -277,6 +278,35 @@ err_gpio:
return r;
}
+static int lb035q02_probe_of(struct spi_device *spi)
+{
+ struct device_node *node = spi->dev.of_node;
+ struct panel_drv_data *ddata = dev_get_drvdata(&spi->dev);
+ struct omap_dss_device *in;
+ struct gpio_desc *gpio;
+
+ gpio = devm_gpiod_get(&spi->dev, "enable");
+ if (IS_ERR(gpio)) {
+ dev_err(&spi->dev, "failed to parse enable gpio\n");
+ return PTR_ERR(gpio);
+ } else {
+ gpiod_direction_output(gpio, 0);
+ ddata->enable_gpio = gpio;
+ }
+
+ ddata->backlight_gpio = -ENOENT;
+
+ in = omapdss_of_find_source_for_first_ep(node);
+ if (IS_ERR(in)) {
+ dev_err(&spi->dev, "failed to find video source\n");
+ return PTR_ERR(in);
+ }
+
+ ddata->in = in;
+
+ return 0;
+}
+
static int lb035q02_panel_spi_probe(struct spi_device *spi)
{
struct panel_drv_data *ddata;
@@ -295,6 +325,10 @@ static int lb035q02_panel_spi_probe(struct spi_device *spi)
r = lb035q02_probe_pdata(spi);
if (r)
return r;
+ } else if (spi->dev.of_node) {
+ r = lb035q02_probe_of(spi);
+ if (r)
+ return r;
} else {
return -ENODEV;
}
@@ -346,6 +380,13 @@ static int lb035q02_panel_spi_remove(struct spi_device *spi)
return 0;
}
+static const struct of_device_id lb035q02_of_match[] = {
+ { .compatible = "omapdss,lgphilips,lb035q02", },
+ {},
+};
+
+MODULE_DEVICE_TABLE(of, lb035q02_of_match);
+
static struct spi_driver lb035q02_spi_driver = {
.probe = lb035q02_panel_spi_probe,
.remove = lb035q02_panel_spi_remove,
@@ -357,6 +398,7 @@ static struct spi_driver lb035q02_spi_driver = {
module_spi_driver(lb035q02_spi_driver);
+MODULE_ALIAS("spi:lgphilips,lb035q02");
MODULE_AUTHOR("Tomi Valkeinen <tomi.valkeinen@ti.com>");
MODULE_DESCRIPTION("LG.Philips LB035Q02 LCD Panel driver");
MODULE_LICENSE("GPL");
--
1.9.1
^ permalink raw reply related
* [PATCH 1/3] OMAPDSS: panel-lgphilips-lb035q02: use gpiod for enable gpio
From: Tomi Valkeinen @ 2014-05-15 10:10 UTC (permalink / raw)
To: linux-fbdev, linux-omap; +Cc: Archit Taneja, Florian Vaussard, Tomi Valkeinen
In-Reply-To: <1400148637-17726-1-git-send-email-tomi.valkeinen@ti.com>
The new gpiod API supports automatic handling of active-high/active-low
with DT. To make it possible to use that when booting with DT, change
the driver's handling of the enable GPIO to use gpiod.
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
.../omap2/displays-new/panel-lgphilips-lb035q02.c | 32 ++++++++++++----------
1 file changed, 18 insertions(+), 14 deletions(-)
diff --git a/drivers/video/fbdev/omap2/displays-new/panel-lgphilips-lb035q02.c b/drivers/video/fbdev/omap2/displays-new/panel-lgphilips-lb035q02.c
index 2e6b513222d9..6e6acd696a70 100644
--- a/drivers/video/fbdev/omap2/displays-new/panel-lgphilips-lb035q02.c
+++ b/drivers/video/fbdev/omap2/displays-new/panel-lgphilips-lb035q02.c
@@ -50,9 +50,10 @@ struct panel_drv_data {
struct omap_video_timings videomode;
- int reset_gpio;
+ /* used for non-DT boot, to be removed */
int backlight_gpio;
- int enable_gpio;
+
+ struct gpio_desc *enable_gpio;
};
#define to_panel_data(p) container_of(p, struct panel_drv_data, dssdev)
@@ -165,8 +166,8 @@ static int lb035q02_enable(struct omap_dss_device *dssdev)
if (r)
return r;
- if (gpio_is_valid(ddata->enable_gpio))
- gpio_set_value_cansleep(ddata->enable_gpio, 1);
+ if (ddata->enable_gpio)
+ gpiod_set_value_cansleep(ddata->enable_gpio, 1);
if (gpio_is_valid(ddata->backlight_gpio))
gpio_set_value_cansleep(ddata->backlight_gpio, 1);
@@ -184,8 +185,8 @@ static void lb035q02_disable(struct omap_dss_device *dssdev)
if (!omapdss_device_is_enabled(dssdev))
return;
- if (gpio_is_valid(ddata->enable_gpio))
- gpio_set_value_cansleep(ddata->enable_gpio, 0);
+ if (ddata->enable_gpio)
+ gpiod_set_value_cansleep(ddata->enable_gpio, 0);
if (gpio_is_valid(ddata->backlight_gpio))
gpio_set_value_cansleep(ddata->backlight_gpio, 0);
@@ -243,6 +244,7 @@ static int lb035q02_probe_pdata(struct spi_device *spi)
const struct panel_lb035q02_platform_data *pdata;
struct panel_drv_data *ddata = dev_get_drvdata(&spi->dev);
struct omap_dss_device *dssdev, *in;
+ int r;
pdata = dev_get_platdata(&spi->dev);
@@ -260,10 +262,19 @@ static int lb035q02_probe_pdata(struct spi_device *spi)
dssdev = &ddata->dssdev;
dssdev->name = pdata->name;
- ddata->enable_gpio = pdata->enable_gpio;
+ r = devm_gpio_request_one(&spi->dev, pdata->enable_gpio,
+ GPIOF_OUT_INIT_LOW, "panel enable");
+ if (r)
+ goto err_gpio;
+
+ ddata->enable_gpio = gpio_to_desc(pdata->enable_gpio);
+
ddata->backlight_gpio = pdata->backlight_gpio;
return 0;
+err_gpio:
+ omap_dss_put_device(ddata->in);
+ return r;
}
static int lb035q02_panel_spi_probe(struct spi_device *spi)
@@ -288,13 +299,6 @@ static int lb035q02_panel_spi_probe(struct spi_device *spi)
return -ENODEV;
}
- if (gpio_is_valid(ddata->enable_gpio)) {
- r = devm_gpio_request_one(&spi->dev, ddata->enable_gpio,
- GPIOF_OUT_INIT_LOW, "panel enable");
- if (r)
- goto err_gpio;
- }
-
if (gpio_is_valid(ddata->backlight_gpio)) {
r = devm_gpio_request_one(&spi->dev, ddata->backlight_gpio,
GPIOF_OUT_INIT_LOW, "panel backlight");
--
1.9.1
^ permalink raw reply related
* [PATCH 0/3] OMAPDSS: DT support for panel-lgphilips-lb035q02
From: Tomi Valkeinen @ 2014-05-15 10:10 UTC (permalink / raw)
To: linux-fbdev, linux-omap; +Cc: Archit Taneja, Florian Vaussard, Tomi Valkeinen
Hi,
This adds DT support to panel-lgphilips-lb035q02. Compile tested only.
This panel is used on some Overo boards (alto35, if I'm not mistaken).
Tomi
Tomi Valkeinen (3):
OMAPDSS: panel-lgphilips-lb035q02: use gpiod for enable gpio
OMAPDSS: panel-lgphilips-lb035q02: Add DT support
Doc/DT: Add binding doc for lgphilips,lb035q02.txt
.../bindings/video/lgphilips,lb035q02.txt | 33 ++++++++++
.../omap2/displays-new/panel-lgphilips-lb035q02.c | 76 +++++++++++++++++-----
2 files changed, 94 insertions(+), 15 deletions(-)
create mode 100644 Documentation/devicetree/bindings/video/lgphilips,lb035q02.txt
--
1.9.1
^ permalink raw reply
* Re: [PATCH 3/4] OMAPDSS: panel-sharp-ls037v7dw01: add device tree support
From: Tomi Valkeinen @ 2014-05-15 9:23 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140514160216.GD18463@atomide.com>
[-- Attachment #1: Type: text/plain, Size: 5274 bytes --]
On 14/05/14 19:02, Tony Lindgren wrote:
>> The video paths of the panels and encoders are connected using the v4l2
>> style ports/endpoints. We can use those to see what display controller a
>> panel is connected to, but only after the panel driver has already
>> probed. We don't have control for the actual probing, as that happens
>> with whatever the control bus is for the display component.
>
> OK. So with generic panels, you can just let the panels probe with
> the right compatible flag then and let the ports/endpoints registration
> to figure out if the panel is usable for the display controller in
> question.
I'm not sure what you mean here.
Do you mean with the future common display framework? There's no need to
figure out anything there, as supposedly the .dts has been written
correctly and the panel and the display controller work together.
If you mean with the current kernel, there's still nothing to figure
out. We can have only single driver with a particular compatible string.
And as our current drivers are omap specific, it makes sense to have
their compatible string be something omap-ish. And if the .dts file
connects the display controller and the panel, then they must be usable
together.
>>> Well it seems at least the reset and enable pin standard from that
>>> binding can be kept.
>>
>> Only enable gpio there. But even that's vague. Do you turn on the power
>> before or after setting the enable gpio? How long delay should be
>> between the power and the gpio? Different panels have different rules
>> for the power-up.
>
> Sure, it's a complex problem. But for the enable gpio..
>
> Maybe the enable GPIO should be treated as a regulator? That would allow
> specifying first the source regulator startup delay, and then the
> panel has it's own startup delay.
Well... I don't know. Sounds rather hacky to me. We have the option to
have a specific driver for this panel, and that driver can handle all
the delays and power-up sequences just right in a clean manner.
>>>>> But I'm not really familiar with using extending current bindings, and
>>>>> making new bindings compatible with old ones. Can you explain why it'd
>>>>> be good to have the sharp panel bindings compatible with simple-panel?
>>>>> In what kind of scenario would it be used?
>>>
>>> Ideally the panel binding just describes the panel and it should not
>>> matter which display controller it is a child of.
>>
>> Yes, but that means the panel bindings need to have enough information
>> so that all display controllers can use it. Simple-panel bindings do not
>> have enough information. The simple-panel bindings do not have
>> information about the video bus input, and it doesn't even have
>> information about the resolution or bitdepth of the panel.
>
> Some of that you can hide into the panel driver based on the compatible
> flag. So why not already do something like this in the .dts files
> instead of the remapping:
>
> compatible = "sharp,ls037v7dw01-omap-dss", "sharp,ls037v7dw01";
>
> And drivers/video/fbdev/omap2/displays-new/panel-sharp-ls037v7dw01.c
> would only claim to be compatible with "sharp,ls037v7dw01-omap-dss".
>
> Then when the common panel framework is available, you can stop
> parsing sharp,ls037v7dw01-omap-dss but the .dts files don't need
> to be changed and it's fine to keep "sharp,ls037v7dw01-omap-dss"
> in the .dts files.
Hmm, I don't see how this relates to the simple-panel bindings.
But you're right, having "sharp,ls037v7dw01-omap-dss" in the .dts is an
alternative for the compatible-string conversion we do now. I guess it's
a matter of taste, but I rather hide it inside the kernel, in an
internal omapdss file, than pollute the .dts files with those compatible
strings.
>> So I'm still asking, if we create sharp bindings that use the same
>> properties as the simple-panel bindings, and define that sharp panel is
>> compatible with simple-panel, what kind of scenario in practice would it
>> be used in?
>
> Well with the above example, just by dss with "sharp,ls037v7dw01-omap-dss"
> until some other SoC notices it can use the GPIO parts of the panel
> code at least :)
>
>> Would the scenario be some other OS, that doesn't have a driver for the
>> sharp panel, but has a driver for the simple-panel? That would only work
>> if the sharp panel hardware is setup so that only the enable gpio is
>> needed, so that quite a narrow definition of "compatible".
>
> That's where we can use the compatible flags and just avoid parsing
> the generic compatible flag unless some common framework is available.
Hmm, sorry, I don't follow. My question was about the simple-panel
bindings, not common display framework.
You were saying that the sharp bindings should be compatible with
simple-panel bindings. I'm still trying to understand what benefit does
that give us.
As I see it, the sharp panel could be used with the simple-panel
bindings only in certain special case, where all the mode pins and the
reset are hardwired in the board hardware, and they are hardwired in a
certain state (all hardwired low, probably), which matches what the
simple-panel driver expects.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH 4/4] ARM: dts: Add LCD panel sharp ls037v7dw01 support for omap3-evm and ldp
From: Tomi Valkeinen @ 2014-05-15 8:57 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140513213205.GB18001@atomide.com>
[-- Attachment #1: Type: text/plain, Size: 1802 bytes --]
On 14/05/14 00:32, Tony Lindgren wrote:
> +&lcd0 {
> + enable-gpios = <&gpio5 24 GPIO_ACTIVE_HIGH>; /* gpio152, lcd INI */
> + reset-gpios = <&gpio5 27 GPIO_ACTIVE_HIGH>; /* gpio155, lcd RESB */
> + /*
> + * The LCD is sideways, so we want the VGA mode instead of
> + * QVGA mode. Probably also want to have omapfb.rotate=3
> + * in the kernel cmdline until there's a binding for that.
> + */
> + mode-gpios = <&gpio5 26 GPIO_ACTIVE_LOW /* gpio154, lcd MO */
> + &gpio1 2 GPIO_ACTIVE_HIGH /* gpio2, lcd LR */
> + &gpio1 3 GPIO_ACTIVE_HIGH>; /* gpio3, lcd UD */
I don't think that is correct. The panel bindings should define what the
first mode-gpio means. Looking at the panel spec, I think the definition
should be "enable QVGA mode". And in the board's dts above, the
GPIO_ACTIVE_x should tell which is one is correct polarity for QVGA
mode. Which is GPIO_ACTIVE_HIGH here.
If we want to tell the panel driver to use QVGA mode, we should do that
explicitly with a flag, not by hacking the GPIO polarities. It's the
panel driver's job to set the GPIO.
So in the previous mail I suggested the 'vga-mode' flag, but I think we
need actually two flags for each GPIO: one that's used to tell the
driver which mode we want, which is used if the panel driver has control
for the GPIO, and the other that tells which is the hardwired setting.
Then again, the two cases are exclusive, so maybe a single flag per mode
pin is ok. So, for the MO pin, we could have 'qvga-mode' flag in the
.dts, which means:
"If there is MO gpio, set MO high. If there's no MO gpio, presume MO pin
is pulled up"
Of course, one could argue that, in case MO is controlled with GPIO, the
'qvga-mode' flag is about SW level configuration, not hardware...
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH 3/4] OMAPDSS: panel-sharp-ls037v7dw01: add device tree support
From: Tomi Valkeinen @ 2014-05-15 8:41 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140513212639.GA18001@atomide.com>
[-- Attachment #1: Type: text/plain, Size: 957 bytes --]
On 14/05/14 00:26, Tony Lindgren wrote:
> + /* lcd MO */
> + ddata->mo_gpio = sharp_ls_get_gpio_of(&pdev->dev, 0, 1, "mode");
> + if (PTR_ERR(ddata->mo_gpio) == -EPROBE_DEFER)
> + return -EPROBE_DEFER;
> +
> + if (!IS_ERR(ddata->mo_gpio))
> + if (gpiod_get_raw_value_cansleep(ddata->mo_gpio))
> + ddata->flags |= SHARP_LS_QVGA;
Shouldn't there be an explicit flag in the DT data for this? If the
panel's MO pin is hardwired to, say, pull up, then the mode-gpios won't
have MO gpio, right? So something like:
mode-gpios = <0 /* high, lcd MO */
&gpio1 2 GPIO_ACTIVE_HIGH /* gpio2, lcd LR */
&gpio1 3 GPIO_ACTIVE_HIGH>; /* gpio3, lcd UD */
vga-mode; /* MO hardwired high */
Btw, the gpio.txt has each gpio inside <>:
chipsel-gpios = <&gpio1 12 0>,
<&gpio1 13 0>,
<0>, /* holes are permitted, means no GPIO 2 */
<&gpio2 2>;
Is that equivalent to having all gpios inside <>?
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* [PATCH] OMAPDSS: move 'compatible' converter to omapdss driver
From: Tomi Valkeinen @ 2014-05-15 8:05 UTC (permalink / raw)
To: Tony Lindgren; +Cc: linux-omap, linux-fbdev, Archit Taneja, Tomi Valkeinen
Move the panel/encoder driver compatible-string converter from
arch/arm/mach-omap2/display.c to omapdss driver. That is a more logical
place for it, as it's really an omapdss internal hack.
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
arch/arm/mach-omap2/display.c | 56 -----------
drivers/video/fbdev/omap2/Makefile | 2 +-
drivers/video/fbdev/omap2/dss/Kconfig | 4 +
drivers/video/fbdev/omap2/dss/Makefile | 1 +
drivers/video/fbdev/omap2/dss/omapdss-boot-init.c | 112 ++++++++++++++++++++++
5 files changed, 118 insertions(+), 57 deletions(-)
create mode 100644 drivers/video/fbdev/omap2/dss/omapdss-boot-init.c
diff --git a/arch/arm/mach-omap2/display.c b/arch/arm/mach-omap2/display.c
index 16d33d831287..519a20fc0432 100644
--- a/arch/arm/mach-omap2/display.c
+++ b/arch/arm/mach-omap2/display.c
@@ -555,65 +555,9 @@ int omap_dss_reset(struct omap_hwmod *oh)
return r;
}
-/* list of 'compatible' nodes to convert to omapdss specific */
-static const char * const dss_compat_conv_list[] __initconst = {
- "composite-connector",
- "dvi-connector",
- "hdmi-connector",
- "panel-dpi",
- "panel-dsi-cm",
- "sony,acx565akm",
- "svideo-connector",
- "ti,tfp410",
- "ti,tpd12s015",
-};
-
-/* prepend compatible string with "omapdss," */
-static __init void omapdss_omapify_node(struct device_node *node,
- const char *compat)
-{
- char *new_compat;
- struct property *prop;
-
- new_compat = kasprintf(GFP_KERNEL, "omapdss,%s", compat);
-
- prop = kzalloc(sizeof(*prop), GFP_KERNEL);
-
- if (!prop) {
- pr_err("omapdss_omapify_node: kzalloc failed\n");
- return;
- }
-
- prop->name = "compatible";
- prop->value = new_compat;
- prop->length = strlen(new_compat) + 1;
-
- of_update_property(node, prop);
-}
-
-/*
- * As omapdss panel drivers are omapdss specific, but we want to define the
- * DT-data in generic manner, we convert the compatible strings of the panel
- * nodes from "panel-foo" to "omapdss,panel-foo". This way we can have both
- * correct DT data and omapdss specific drivers.
- *
- * When we get generic panel drivers to the kernel, this will be removed.
- */
void __init omapdss_early_init_of(void)
{
- int i;
-
- for (i = 0; i < ARRAY_SIZE(dss_compat_conv_list); ++i) {
- const char *compat = dss_compat_conv_list[i];
- struct device_node *node = NULL;
-
- while ((node = of_find_compatible_node(node, NULL, compat))) {
- if (!of_device_is_available(node))
- continue;
- omapdss_omapify_node(node, compat);
- }
- }
}
struct device_node * __init omapdss_find_dss_of_node(void)
diff --git a/drivers/video/fbdev/omap2/Makefile b/drivers/video/fbdev/omap2/Makefile
index bf8127df8c71..f8745ec369cc 100644
--- a/drivers/video/fbdev/omap2/Makefile
+++ b/drivers/video/fbdev/omap2/Makefile
@@ -1,5 +1,5 @@
obj-$(CONFIG_OMAP2_VRFB) += vrfb.o
-obj-$(CONFIG_OMAP2_DSS) += dss/
+obj-y += dss/
obj-y += displays-new/
obj-$(CONFIG_FB_OMAP2) += omapfb/
diff --git a/drivers/video/fbdev/omap2/dss/Kconfig b/drivers/video/fbdev/omap2/dss/Kconfig
index dde4281663b1..a28f3a39ab1b 100644
--- a/drivers/video/fbdev/omap2/dss/Kconfig
+++ b/drivers/video/fbdev/omap2/dss/Kconfig
@@ -1,6 +1,10 @@
+config OMAP2_DSS_INIT
+ bool
+
menuconfig OMAP2_DSS
tristate "OMAP2+ Display Subsystem support"
select VIDEOMODE_HELPERS
+ select OMAP2_DSS_INIT
help
OMAP2+ Display Subsystem support.
diff --git a/drivers/video/fbdev/omap2/dss/Makefile b/drivers/video/fbdev/omap2/dss/Makefile
index 8aec8bda27cc..3b79ad74f2e1 100644
--- a/drivers/video/fbdev/omap2/dss/Makefile
+++ b/drivers/video/fbdev/omap2/dss/Makefile
@@ -1,3 +1,4 @@
+obj-$(CONFIG_OMAP2_DSS_INIT) += omapdss-boot-init.o
obj-$(CONFIG_OMAP2_DSS) += omapdss.o
# Core DSS files
omapdss-y := core.o dss.o dss_features.o dispc.o dispc_coefs.o display.o \
diff --git a/drivers/video/fbdev/omap2/dss/omapdss-boot-init.c b/drivers/video/fbdev/omap2/dss/omapdss-boot-init.c
new file mode 100644
index 000000000000..468f6eff370c
--- /dev/null
+++ b/drivers/video/fbdev/omap2/dss/omapdss-boot-init.c
@@ -0,0 +1,112 @@
+/*
+ * Copyright (C) 2014 Texas Instruments
+ * Author: Tomi Valkeinen <tomi.valkeinen@nokia.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published by
+ * the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <linux/kernel.h>
+#include <linux/of.h>
+#include <linux/slab.h>
+
+/* list of 'compatible' nodes to convert to omapdss specific */
+static const char * const dss_compat_conv_list[] __initconst = {
+ "composite-connector",
+ "dvi-connector",
+ "hdmi-connector",
+ "panel-dpi",
+ "panel-dsi-cm",
+ "sony,acx565akm",
+ "svideo-connector",
+ "ti,tfp410",
+ "ti,tpd12s015",
+};
+
+/* prepend compatible string with "omapdss," */
+static __init void omapdss_omapify_node(struct device_node *node,
+ const char *compat)
+{
+ char *new_compat;
+ struct property *prop;
+
+ new_compat = kasprintf(GFP_KERNEL, "omapdss,%s", compat);
+
+ prop = kzalloc(sizeof(*prop), GFP_KERNEL);
+
+ if (!prop) {
+ pr_err("omapdss_omapify_node: kzalloc failed\n");
+ return;
+ }
+
+ prop->name = "compatible";
+ prop->value = new_compat;
+ prop->length = strlen(new_compat) + 1;
+
+ of_update_property(node, prop);
+}
+
+static struct device_node * __init omapdss_find_dss_of_node(void)
+{
+ struct device_node *node;
+
+ node = of_find_compatible_node(NULL, NULL, "ti,omap2-dss");
+ if (node)
+ return node;
+
+ node = of_find_compatible_node(NULL, NULL, "ti,omap3-dss");
+ if (node)
+ return node;
+
+ node = of_find_compatible_node(NULL, NULL, "ti,omap4-dss");
+ if (node)
+ return node;
+
+ node = of_find_compatible_node(NULL, NULL, "ti,omap5-dss");
+ if (node)
+ return node;
+
+ return NULL;
+}
+
+/*
+ * As omapdss panel drivers are omapdss specific, but we want to define the
+ * DT-data in generic manner, we convert the compatible strings of the panel
+ * nodes from "panel-foo" to "omapdss,panel-foo". This way we can have both
+ * correct DT data and omapdss specific drivers.
+ *
+ * When we get generic panel drivers to the kernel, this will be removed.
+ */
+static int __init omap_dss_boot_init(void)
+{
+ int i;
+
+ /* do we have omapdss device? */
+ if (omapdss_find_dss_of_node() = NULL)
+ return 0;
+
+ for (i = 0; i < ARRAY_SIZE(dss_compat_conv_list); ++i) {
+ const char *compat = dss_compat_conv_list[i];
+ struct device_node *node = NULL;
+
+ while ((node = of_find_compatible_node(node, NULL, compat))) {
+ if (!of_device_is_available(node))
+ continue;
+
+ omapdss_omapify_node(node, compat);
+ }
+ }
+
+ return 0;
+}
+
+subsys_initcall(omap_dss_boot_init);
--
1.9.1
^ permalink raw reply related
* [PATCH] video : remove redundant error check
From: Daeseok Youn @ 2014-05-15 7:55 UTC (permalink / raw)
To: plagnioj
Cc: tomi.valkeinen, jg1.han, daeseok.youn, daniel.vetter,
Julia.Lawall, linux-fbdev, linux-kernel
From 060757f81f85babf393cc76714d49af25af7791d Mon Sep 17 00:00:00 2001
From: Daeseok Youn <daeseok.youn@gmail.com>
Date: Thu, 15 May 2014 16:51:35 +0900
Subject: [PATCH] video : remove redundant error check
It doesn't need to check "err" for printing info.
And also use pr_info instead of printk.
Signed-off-by: Daeseok Youn <daeseok.youn@gmail.com>
---
drivers/video/fbdev/i810/i810_main.c | 7 +++----
1 files changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/video/fbdev/i810/i810_main.c b/drivers/video/fbdev/i810/i810_main.c
index bb674e4..15cb397 100644
--- a/drivers/video/fbdev/i810/i810_main.c
+++ b/drivers/video/fbdev/i810/i810_main.c
@@ -1910,13 +1910,12 @@ static void i810fb_find_init_mode(struct fb_info *info)
for (i = 0; i < par->ddc_num + 1; i++) {
err = i810_probe_i2c_connector(info, &par->edid, i);
- if (!err)
+ if (!err) {
+ pr_info("i810fb_init_pci: DDC probe successful\n");
break;
+ }
}
- if (!err)
- printk("i810fb_init_pci: DDC probe successful\n");
-
fb_edid_to_monspecs(par->edid, specs);
if (specs->modedb = NULL)
--
1.7.1
^ permalink raw reply related
* Re: [PATCH] offb: Fix little-endian support
From: Takashi Iwai @ 2014-05-14 17:57 UTC (permalink / raw)
To: Cedric Le Goater
Cc: Dinar Valeev, Tomi Valkeinen, Jean-Christophe Plagniol-Villard,
linux-fbdev, linux-kernel
In-Reply-To: <5373A200.6080301@fr.ibm.com>
At Wed, 14 May 2014 19:04:00 +0200,
Cedric Le Goater wrote:
>
> On 05/14/2014 04:24 PM, Takashi Iwai wrote:
> > At Wed, 14 May 2014 16:01:17 +0200,
> > Cedric Le Goater wrote:
> >>
> >> Hi Iwai-san,
> >>
> >> On 05/14/2014 03:21 PM, Takashi Iwai wrote:
> >>> Although the color palette was corrected for little endian by the
> >>> commit [e1edf18b: offb: Add palette hack for little endian], the
> >>> graphics mode is still shown in psychedelic colors.
> >>
> >> Are you referring to the linux logo colors ? If so, could you please
> >> try the patch below, it should be a fix.
> >
> > Not only penguin logo but the whole X graphics got strange colors,
> > too, according to the bug report. I put the original reporter/tester
> > (Dinar Valeev) to Cc.
> > I'm merely a person who tries to fix this mess ;)
> >
> > BTW, did you try to run X on fbdev?
>
> Not at the time, I was working on the console only, BE and LE.
> I just tried fbdev and indeed this is a psychedelic mess :)
>
> Your fix has also issues on BE and console and the patch of mine
> below is of no use for fbdev. Damn, this is a nightmare.
Hm, so it actually regressed on BE?
It's strange because fb_math_be() should be true and the patch won't
change the values in that case...
Takashi
>
> C.
>
> >>> For fixing this
> >>> properly, we rather need to correct the RGB offsets depending on
> >>> endianess.
> >>>
> >>> Since the RGB base offsets are corrected, we don't need the hack for
> >>> pallette color entries. This patch reverts that, too.
> >>
> >> Are you testing using qemu -vga std -vnc :x ? If so, did you try changing
> >> the depth to 8,15,16,32 ?
> >
> > Yes, it was with qemu -vga std -vnc :x.
> > About different color depths, Dinar can test / clarify better, I
> > suppose.
> >
> >> I think the patch might be breaking big endian
> >> too.
> >
> > Big endian should work as is because my patch uses the original
> > offsets when fb_be_math() is true. It corrects the RGB offsets if
> > !fb_be_math().
> >
> > But, I'm also entirely not sure whether this is 100% correct, either.
> > Namely, if the RGB offsets were correct for some little endian
> > machines with offb, my patch would break it, of course. But, then
> > your previous fix must have already broken it as well, so I took the
> > same fb_be_math() check.
> >
> >
> > thanks,
> >
> > Takashi
> >
> >> Now, I am far from being an expert on frame buffers. It would be glad
> >> to have some insights on that topic.
> >>
> >> Thanks,
> >>
> >> C.
> >>
> >>
> >> [PATCH] fb: fix logo palette entries for little endian
> >>
> >> The offb_cmap_byteswap() routine helps byteswapping the color map
> >> entries when required. This patch externalizes and renames the helper
> >> routine to adjust the pseudo palette of the logo when running on
> >> little endian.
> >>
> >> Signed-off-by: Cédric Le Goater <clg@fr.ibm.com>
> >> ---
> >> drivers/video/fbmem.c | 6 ++++--
> >> drivers/video/offb.c | 11 +----------
> >> include/linux/fb.h | 8 ++++++++
> >> 3 files changed, 13 insertions(+), 12 deletions(-)
> >>
> >> Index: linux.git/drivers/video/fbmem.c
> >> =================================> >> --- linux.git.orig/drivers/video/fbmem.c
> >> +++ linux.git/drivers/video/fbmem.c
> >> @@ -252,7 +252,8 @@ static void fb_set_logo_truepalette(str
> >> blueshift = info->var.blue.offset - (8 - info->var.blue.length);
> >>
> >> for ( i = 0; i < logo->clutsize; i++) {
> >> - palette[i+32] = (safe_shift((clut[0] & redmask), redshift) |
> >> + palette[i+32] = fb_cmap_byteswap(info,
> >> + safe_shift((clut[0] & redmask), redshift) |
> >> safe_shift((clut[1] & greenmask), greenshift) |
> >> safe_shift((clut[2] & bluemask), blueshift));
> >> clut += 3;
> >> @@ -271,7 +272,8 @@ static void fb_set_logo_directpalette(st
> >> blueshift = info->var.blue.offset;
> >>
> >> for (i = 32; i < 32 + logo->clutsize; i++)
> >> - palette[i] = i << redshift | i << greenshift | i << blueshift;
> >> + palette[i] = fb_cmap_byteswap(info, i << redshift |
> >> + i << greenshift | i << blueshift);
> >> }
> >>
> >> static void fb_set_logo(struct fb_info *info,
> >> Index: linux.git/drivers/video/offb.c
> >> =================================> >> --- linux.git.orig/drivers/video/offb.c
> >> +++ linux.git/drivers/video/offb.c
> >> @@ -91,15 +91,6 @@ extern boot_infos_t *boot_infos;
> >> #define AVIVO_DC_LUTB_WHITE_OFFSET_GREEN 0x6cd4
> >> #define AVIVO_DC_LUTB_WHITE_OFFSET_RED 0x6cd8
> >>
> >> -#define FB_RIGHT_POS(p, bpp) (fb_be_math(p) ? 0 : (32 - (bpp)))
> >> -
> >> -static inline u32 offb_cmap_byteswap(struct fb_info *info, u32 value)
> >> -{
> >> - u32 bpp = info->var.bits_per_pixel;
> >> -
> >> - return cpu_to_be32(value) >> FB_RIGHT_POS(info, bpp);
> >> -}
> >> -
> >> /*
> >> * Set a single color register. The values supplied are already
> >> * rounded down to the hardware's capabilities (according to the
> >> @@ -129,7 +120,7 @@ static int offb_setcolreg(u_int regno, u
> >> mask <<= info->var.transp.offset;
> >> value |= mask;
> >> }
> >> - pal[regno] = offb_cmap_byteswap(info, value);
> >> + pal[regno] = fb_cmap_byteswap(info, value);
> >> return 0;
> >> }
> >>
> >> Index: linux.git/include/linux/fb.h
> >> =================================> >> --- linux.git.orig/include/linux/fb.h
> >> +++ linux.git/include/linux/fb.h
> >> @@ -582,6 +582,7 @@ static inline struct apertures_struct *a
> >>
> >> #endif
> >>
> >> +#define FB_RIGHT_POS(p, bpp) (fb_be_math(p) ? 0 : (32 - (bpp)))
> >> #define FB_LEFT_POS(p, bpp) (fb_be_math(p) ? (32 - (bpp)) : 0)
> >> #define FB_SHIFT_HIGH(p, val, bits) (fb_be_math(p) ? (val) >> (bits) : \
> >> (val) << (bits))
> >> @@ -681,6 +682,13 @@ static inline bool fb_be_math(struct fb_
> >> #endif /* CONFIG_FB_FOREIGN_ENDIAN */
> >> }
> >>
> >> +static inline u32 fb_cmap_byteswap(struct fb_info *info, u32 value)
> >> +{
> >> + u32 bpp = info->var.bits_per_pixel;
> >> +
> >> + return cpu_to_be32(value) >> FB_RIGHT_POS(info, bpp);
> >> +}
> >> +
> >> /* drivers/video/fbsysfs.c */
> >> extern struct fb_info *framebuffer_alloc(size_t size, struct device *dev);
> >> extern void framebuffer_release(struct fb_info *info);
> >>
> >
>
^ permalink raw reply
* Re: [PATCH] offb: Fix little-endian support
From: Cedric Le Goater @ 2014-05-14 17:04 UTC (permalink / raw)
To: Takashi Iwai
Cc: Dinar Valeev, Tomi Valkeinen, Jean-Christophe Plagniol-Villard,
linux-fbdev, linux-kernel
In-Reply-To: <s5hha4sh20d.wl%tiwai@suse.de>
On 05/14/2014 04:24 PM, Takashi Iwai wrote:
> At Wed, 14 May 2014 16:01:17 +0200,
> Cedric Le Goater wrote:
>>
>> Hi Iwai-san,
>>
>> On 05/14/2014 03:21 PM, Takashi Iwai wrote:
>>> Although the color palette was corrected for little endian by the
>>> commit [e1edf18b: offb: Add palette hack for little endian], the
>>> graphics mode is still shown in psychedelic colors.
>>
>> Are you referring to the linux logo colors ? If so, could you please
>> try the patch below, it should be a fix.
>
> Not only penguin logo but the whole X graphics got strange colors,
> too, according to the bug report. I put the original reporter/tester
> (Dinar Valeev) to Cc.
> I'm merely a person who tries to fix this mess ;)
>
> BTW, did you try to run X on fbdev?
Not at the time, I was working on the console only, BE and LE.
I just tried fbdev and indeed this is a psychedelic mess :)
Your fix has also issues on BE and console and the patch of mine
below is of no use for fbdev. Damn, this is a nightmare.
C.
>>> For fixing this
>>> properly, we rather need to correct the RGB offsets depending on
>>> endianess.
>>>
>>> Since the RGB base offsets are corrected, we don't need the hack for
>>> pallette color entries. This patch reverts that, too.
>>
>> Are you testing using qemu -vga std -vnc :x ? If so, did you try changing
>> the depth to 8,15,16,32 ?
>
> Yes, it was with qemu -vga std -vnc :x.
> About different color depths, Dinar can test / clarify better, I
> suppose.
>
>> I think the patch might be breaking big endian
>> too.
>
> Big endian should work as is because my patch uses the original
> offsets when fb_be_math() is true. It corrects the RGB offsets if
> !fb_be_math().
>
> But, I'm also entirely not sure whether this is 100% correct, either.
> Namely, if the RGB offsets were correct for some little endian
> machines with offb, my patch would break it, of course. But, then
> your previous fix must have already broken it as well, so I took the
> same fb_be_math() check.
>
>
> thanks,
>
> Takashi
>
>> Now, I am far from being an expert on frame buffers. It would be glad
>> to have some insights on that topic.
>>
>> Thanks,
>>
>> C.
>>
>>
>> [PATCH] fb: fix logo palette entries for little endian
>>
>> The offb_cmap_byteswap() routine helps byteswapping the color map
>> entries when required. This patch externalizes and renames the helper
>> routine to adjust the pseudo palette of the logo when running on
>> little endian.
>>
>> Signed-off-by: Cédric Le Goater <clg@fr.ibm.com>
>> ---
>> drivers/video/fbmem.c | 6 ++++--
>> drivers/video/offb.c | 11 +----------
>> include/linux/fb.h | 8 ++++++++
>> 3 files changed, 13 insertions(+), 12 deletions(-)
>>
>> Index: linux.git/drivers/video/fbmem.c
>> =================================>> --- linux.git.orig/drivers/video/fbmem.c
>> +++ linux.git/drivers/video/fbmem.c
>> @@ -252,7 +252,8 @@ static void fb_set_logo_truepalette(str
>> blueshift = info->var.blue.offset - (8 - info->var.blue.length);
>>
>> for ( i = 0; i < logo->clutsize; i++) {
>> - palette[i+32] = (safe_shift((clut[0] & redmask), redshift) |
>> + palette[i+32] = fb_cmap_byteswap(info,
>> + safe_shift((clut[0] & redmask), redshift) |
>> safe_shift((clut[1] & greenmask), greenshift) |
>> safe_shift((clut[2] & bluemask), blueshift));
>> clut += 3;
>> @@ -271,7 +272,8 @@ static void fb_set_logo_directpalette(st
>> blueshift = info->var.blue.offset;
>>
>> for (i = 32; i < 32 + logo->clutsize; i++)
>> - palette[i] = i << redshift | i << greenshift | i << blueshift;
>> + palette[i] = fb_cmap_byteswap(info, i << redshift |
>> + i << greenshift | i << blueshift);
>> }
>>
>> static void fb_set_logo(struct fb_info *info,
>> Index: linux.git/drivers/video/offb.c
>> =================================>> --- linux.git.orig/drivers/video/offb.c
>> +++ linux.git/drivers/video/offb.c
>> @@ -91,15 +91,6 @@ extern boot_infos_t *boot_infos;
>> #define AVIVO_DC_LUTB_WHITE_OFFSET_GREEN 0x6cd4
>> #define AVIVO_DC_LUTB_WHITE_OFFSET_RED 0x6cd8
>>
>> -#define FB_RIGHT_POS(p, bpp) (fb_be_math(p) ? 0 : (32 - (bpp)))
>> -
>> -static inline u32 offb_cmap_byteswap(struct fb_info *info, u32 value)
>> -{
>> - u32 bpp = info->var.bits_per_pixel;
>> -
>> - return cpu_to_be32(value) >> FB_RIGHT_POS(info, bpp);
>> -}
>> -
>> /*
>> * Set a single color register. The values supplied are already
>> * rounded down to the hardware's capabilities (according to the
>> @@ -129,7 +120,7 @@ static int offb_setcolreg(u_int regno, u
>> mask <<= info->var.transp.offset;
>> value |= mask;
>> }
>> - pal[regno] = offb_cmap_byteswap(info, value);
>> + pal[regno] = fb_cmap_byteswap(info, value);
>> return 0;
>> }
>>
>> Index: linux.git/include/linux/fb.h
>> =================================>> --- linux.git.orig/include/linux/fb.h
>> +++ linux.git/include/linux/fb.h
>> @@ -582,6 +582,7 @@ static inline struct apertures_struct *a
>>
>> #endif
>>
>> +#define FB_RIGHT_POS(p, bpp) (fb_be_math(p) ? 0 : (32 - (bpp)))
>> #define FB_LEFT_POS(p, bpp) (fb_be_math(p) ? (32 - (bpp)) : 0)
>> #define FB_SHIFT_HIGH(p, val, bits) (fb_be_math(p) ? (val) >> (bits) : \
>> (val) << (bits))
>> @@ -681,6 +682,13 @@ static inline bool fb_be_math(struct fb_
>> #endif /* CONFIG_FB_FOREIGN_ENDIAN */
>> }
>>
>> +static inline u32 fb_cmap_byteswap(struct fb_info *info, u32 value)
>> +{
>> + u32 bpp = info->var.bits_per_pixel;
>> +
>> + return cpu_to_be32(value) >> FB_RIGHT_POS(info, bpp);
>> +}
>> +
>> /* drivers/video/fbsysfs.c */
>> extern struct fb_info *framebuffer_alloc(size_t size, struct device *dev);
>> extern void framebuffer_release(struct fb_info *info);
>>
>
^ permalink raw reply
* Re: [PATCH] offb: Fix little-endian support
From: Geert Uytterhoeven @ 2014-05-14 16:56 UTC (permalink / raw)
To: Cedric Le Goater
Cc: Takashi Iwai, Tomi Valkeinen, Jean-Christophe Plagniol-Villard,
Linux Fbdev development list, linux-kernel@vger.kernel.org
In-Reply-To: <5373772D.9060807@fr.ibm.com>
On Wed, May 14, 2014 at 4:01 PM, Cedric Le Goater <clg@fr.ibm.com> wrote:
> --- linux.git.orig/drivers/video/fbmem.c
> +++ linux.git/drivers/video/fbmem.c
> @@ -252,7 +252,8 @@ static void fb_set_logo_truepalette(str
> blueshift = info->var.blue.offset - (8 - info->var.blue.length);
>
> for ( i = 0; i < logo->clutsize; i++) {
> - palette[i+32] = (safe_shift((clut[0] & redmask), redshift) |
> + palette[i+32] = fb_cmap_byteswap(info,
> + safe_shift((clut[0] & redmask), redshift) |
> safe_shift((clut[1] & greenmask), greenshift) |
> safe_shift((clut[2] & bluemask), blueshift));
> clut += 3;
> @@ -271,7 +272,8 @@ static void fb_set_logo_directpalette(st
> blueshift = info->var.blue.offset;
>
> for (i = 32; i < 32 + logo->clutsize; i++)
> - palette[i] = i << redshift | i << greenshift | i << blueshift;
> + palette[i] = fb_cmap_byteswap(info, i << redshift |
> + i << greenshift | i << blueshift);
I find it very strange you have to touch the generic code in such a way...
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: [alsa-devel] [PATCH 00/19] Rework OMAP4+ HDMI audio support
From: Joachim Eastwood @ 2014-05-14 16:25 UTC (permalink / raw)
To: Jyri Sarha
Cc: alsa-devel, linux-fbdev, devicetree, linux-omap, liam.r.girdwood,
detheridge, broonie, peter.ujfalusi, Tomi Valkeinen,
Benoit Cousson
In-Reply-To: <53733F2E.4030803@ti.com>
On 14 May 2014 12:02, Jyri Sarha <jsarha@ti.com> wrote:
> On 05/13/2014 12:13 AM, Joachim Eastwood wrote:
>>
>> On 12 May 2014 11:12, Jyri Sarha <jsarha@ti.com> wrote:
>
> ...
>
>>
>> hey, this worked straight away :)
>>
>> But there seems to be something wrong with the channel mapping.
>>
>> For stereo (speaker-test -c 2) the mapping is correct.
>>
>> But for -c 4 and -c 8 it gets weird:
>> speaker-test -c 4 -s X # where X is 1-4
>> 1: Front Left is Rear Left
>> 2: Front Right is Rear Right
>> 3: Rear Right is Front Right
>> 4: Rear Left is Front Left
>>
>> speaker-test -c 8 -s X # where X is 1-8
>> 1: Front Left is Rear Left
>> 2: Center is Rear Left
>> 3: Front Right is Rear Right
>> 4: Side Right is Front Right
>> 5: Rear Right is silent
>> 6: Rear Left is Center
>> 7: Side Left is Front Left
>> 8: LFE - Rear Right
>>
>> I think you need to check what channel order ALSA expects. I believe
>> speaker-test does the right thing on my HTPC normally connected to my
>> receiver.
>>
>
> I checked the implementation and there was indeed something weird there, but
> the implementation can not explain the FL and FR channels jumping around. FL
> and FL should always be the first two channels in all configurations and the
> implementation does not touch them.
>
> The implementation uses 8ch HDMI setup for anything above 2ch with "Audio
> InfoFrame Data Byte 4" set to 0x13. According to CEA-861 specs this means
> following channel order: FL, FR, LFE, FC, RL, RR, RLC, RRC
>
> This is a closest match to ALSA 8ch mapping (according to
> sound/core/pcm_lib.c) which is: FL, FR, FC, LFE, RL, RR, SL, SR
hm, okey. I haven't look at the code but it do seem strange. But with
speaker-test -c 4 the front and back are surely swapped here.
I'll do some more testing and also check with my HTPC. btw, I only
have a 5.1 setup over here so I can't test all the discrete channels.
> Current implementation has FLE and FC channels correctly swapped, but it
> shifts them to last two channels and RL, RR, SL, SR are shifted down to fill
> the place. This is all wrong and I'll try to come up with a fix for that.
> Unfortunately I can not test anything beyond 2 ch myself so I would need
> someone to volunteer to test my patch.
I have the dev kit setup up over here so I can test your patches.
regards
Joachim Eastwood
^ permalink raw reply
* Re: [PATCH 3/4] OMAPDSS: panel-sharp-ls037v7dw01: add device tree support
From: Tony Lindgren @ 2014-05-14 16:02 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <53730AFB.2090800@ti.com>
* Tomi Valkeinen <tomi.valkeinen@ti.com> [140513 23:20]:
> On 13/05/14 18:25, Tony Lindgren wrote:
>
> > Well ideally the revision info for a device would come from device
> > revision registers rather using the SoC revision. In the DSS case when
> > the SoC revision is needed by a device it maybe it can be deciphered
> > from a combination of compatible flag and the clock rate for example?
>
> I've been trying that. The HW guys didn't bother to update the DSS
> revision registers, so they are useless. And, for example, the OMAP3 ES
> difference is only about bitfield widths in two registers.
>
> I tried writing "too long" value to the register on the earlier ES
> version, hoping that the extra bits would be kept at zero, but that
> wasn't the case. So I just don't see a way to detect this from the DSS's
> point of view.
>
> >>> Do you object to the compatible string remapping as such, or just that
> >>> it's in arch/arm/mach-omap2?
> >
> > It's something I'd rather not have under mach-omap2 as that means that
> > I may need to deal with it too to some extent. And I don't think we
> > need to do such remapping, we should be able to use the panel compatible
> > strings as they are just fine. It should be possible to figure out from
> > the device tree properties what controller the panel belongs to. Or
> > for now, use the panel registration to figure out what display controller
> > it belongs to.
> >
> >>> I guess nothing prevents me from moving it to drivers/, and having some
> >>> early-ish initcall doing the job.
> >
> > /me likes this idea if you need it at all. Stuff like this tends to stay
> > and spread, so I'd rather not do the remapping of compatible strings at
> > all.
>
> Yep. I'll look to this. Thinking about it now, it kind of makes more
> sense to have it in the omapdss's directory.
OK thanks.
> >> So, since we can change the kernel later but not the DTS, I agree with
> >> you that the remapping is the least bad of our options.
> >
> > Yes the binding for the panel should just describe the panel so it can be
> > used with whatever display controller. But we do have quite a few buses
> > probing devices. How about set up the panel probing the same way?
>
> > For the panels on display controller, just do the usual
> > for_each_child_of_node(pdev->dev.of_node, child) and probe them?
> >
> > It seems the remapping of compatible strings is not needed in this
> > as we're only picking up panels that are children of the display
> > controller.
>
> The panels (or display encoders) are not (usually) children of the
> display controller. They are children of their respective control bus.
> Say, an i2c panel is a child of i2c bus. If there's no control bus, like
> is the case with the sharp panel, it's a platform device.
OK
> The video paths of the panels and encoders are connected using the v4l2
> style ports/endpoints. We can use those to see what display controller a
> panel is connected to, but only after the panel driver has already
> probed. We don't have control for the actual probing, as that happens
> with whatever the control bus is for the display component.
OK. So with generic panels, you can just let the panels probe with
the right compatible flag then and let the ports/endpoints registration
to figure out if the panel is usable for the display controller in
question.
> >>>>> I'm not sure what it would give us to try to be compatible with
> >>>>> simple-panel.txt. The simple-panel bindings won't probably be compatible
> >>>>> with the future common display drivers in any case, as the simple-panel
> >>>>> binding is just too limited and doesn't describe the hardware fully.
> >>>>
> >>>> Hmm what else does a panel need where the existing binding cannot be
> >>>> extended?
> >>>
> >>> The existing simple-panel binding doesn't describe the connections of
> >>> the panel, i.e. its video input. I guess it can be extended, but I don't
> >>> see what the benefit is of trying to create new panel bindings
> >>> compatible with the simple-panel bindings. As I see, the simple-panel
> >>> bindings work only with very limited use cases, where the drivers make
> >>> assumptions. Simple panel bindings cannot be used with omapdss, nor can
> >>> it be used with the future common display framework.
> >
> > Well it seems at least the reset and enable pin standard from that
> > binding can be kept.
>
> Only enable gpio there. But even that's vague. Do you turn on the power
> before or after setting the enable gpio? How long delay should be
> between the power and the gpio? Different panels have different rules
> for the power-up.
Sure, it's a complex problem. But for the enable gpio..
Maybe the enable GPIO should be treated as a regulator? That would allow
specifying first the source regulator startup delay, and then the
panel has it's own startup delay.
> >>> But I'm not really familiar with using extending current bindings, and
> >>> making new bindings compatible with old ones. Can you explain why it'd
> >>> be good to have the sharp panel bindings compatible with simple-panel?
> >>> In what kind of scenario would it be used?
> >
> > Ideally the panel binding just describes the panel and it should not
> > matter which display controller it is a child of.
>
> Yes, but that means the panel bindings need to have enough information
> so that all display controllers can use it. Simple-panel bindings do not
> have enough information. The simple-panel bindings do not have
> information about the video bus input, and it doesn't even have
> information about the resolution or bitdepth of the panel.
Some of that you can hide into the panel driver based on the compatible
flag. So why not already do something like this in the .dts files
instead of the remapping:
compatible = "sharp,ls037v7dw01-omap-dss", "sharp,ls037v7dw01";
And drivers/video/fbdev/omap2/displays-new/panel-sharp-ls037v7dw01.c
would only claim to be compatible with "sharp,ls037v7dw01-omap-dss".
Then when the common panel framework is available, you can stop
parsing sharp,ls037v7dw01-omap-dss but the .dts files don't need
to be changed and it's fine to keep "sharp,ls037v7dw01-omap-dss"
in the .dts files.
> So I'm still asking, if we create sharp bindings that use the same
> properties as the simple-panel bindings, and define that sharp panel is
> compatible with simple-panel, what kind of scenario in practice would it
> be used in?
Well with the above example, just by dss with "sharp,ls037v7dw01-omap-dss"
until some other SoC notices it can use the GPIO parts of the panel
code at least :)
> Would the scenario be some other OS, that doesn't have a driver for the
> sharp panel, but has a driver for the simple-panel? That would only work
> if the sharp panel hardware is setup so that only the enable gpio is
> needed, so that quite a narrow definition of "compatible".
That's where we can use the compatible flags and just avoid parsing
the generic compatible flag unless some common framework is available.
> Or is there some other scenario in which it could be used?
I guess other OS or other SoC with a different display controller
but the same panel.
Regards,
Tony
^ permalink raw reply
* Re: [PATCH] offb: Fix little-endian support
From: Takashi Iwai @ 2014-05-14 14:24 UTC (permalink / raw)
To: Cedric Le Goater
Cc: Dinar Valeev, Tomi Valkeinen, Jean-Christophe Plagniol-Villard,
linux-fbdev, linux-kernel
In-Reply-To: <5373772D.9060807@fr.ibm.com>
At Wed, 14 May 2014 16:01:17 +0200,
Cedric Le Goater wrote:
>
> Hi Iwai-san,
>
> On 05/14/2014 03:21 PM, Takashi Iwai wrote:
> > Although the color palette was corrected for little endian by the
> > commit [e1edf18b: offb: Add palette hack for little endian], the
> > graphics mode is still shown in psychedelic colors.
>
> Are you referring to the linux logo colors ? If so, could you please
> try the patch below, it should be a fix.
Not only penguin logo but the whole X graphics got strange colors,
too, according to the bug report. I put the original reporter/tester
(Dinar Valeev) to Cc.
I'm merely a person who tries to fix this mess ;)
BTW, did you try to run X on fbdev?
> > For fixing this
> > properly, we rather need to correct the RGB offsets depending on
> > endianess.
> >
> > Since the RGB base offsets are corrected, we don't need the hack for
> > pallette color entries. This patch reverts that, too.
>
> Are you testing using qemu -vga std -vnc :x ? If so, did you try changing
> the depth to 8,15,16,32 ?
Yes, it was with qemu -vga std -vnc :x.
About different color depths, Dinar can test / clarify better, I
suppose.
> I think the patch might be breaking big endian
> too.
Big endian should work as is because my patch uses the original
offsets when fb_be_math() is true. It corrects the RGB offsets if
!fb_be_math().
But, I'm also entirely not sure whether this is 100% correct, either.
Namely, if the RGB offsets were correct for some little endian
machines with offb, my patch would break it, of course. But, then
your previous fix must have already broken it as well, so I took the
same fb_be_math() check.
thanks,
Takashi
> Now, I am far from being an expert on frame buffers. It would be glad
> to have some insights on that topic.
>
> Thanks,
>
> C.
>
>
> [PATCH] fb: fix logo palette entries for little endian
>
> The offb_cmap_byteswap() routine helps byteswapping the color map
> entries when required. This patch externalizes and renames the helper
> routine to adjust the pseudo palette of the logo when running on
> little endian.
>
> Signed-off-by: Cédric Le Goater <clg@fr.ibm.com>
> ---
> drivers/video/fbmem.c | 6 ++++--
> drivers/video/offb.c | 11 +----------
> include/linux/fb.h | 8 ++++++++
> 3 files changed, 13 insertions(+), 12 deletions(-)
>
> Index: linux.git/drivers/video/fbmem.c
> =================================> --- linux.git.orig/drivers/video/fbmem.c
> +++ linux.git/drivers/video/fbmem.c
> @@ -252,7 +252,8 @@ static void fb_set_logo_truepalette(str
> blueshift = info->var.blue.offset - (8 - info->var.blue.length);
>
> for ( i = 0; i < logo->clutsize; i++) {
> - palette[i+32] = (safe_shift((clut[0] & redmask), redshift) |
> + palette[i+32] = fb_cmap_byteswap(info,
> + safe_shift((clut[0] & redmask), redshift) |
> safe_shift((clut[1] & greenmask), greenshift) |
> safe_shift((clut[2] & bluemask), blueshift));
> clut += 3;
> @@ -271,7 +272,8 @@ static void fb_set_logo_directpalette(st
> blueshift = info->var.blue.offset;
>
> for (i = 32; i < 32 + logo->clutsize; i++)
> - palette[i] = i << redshift | i << greenshift | i << blueshift;
> + palette[i] = fb_cmap_byteswap(info, i << redshift |
> + i << greenshift | i << blueshift);
> }
>
> static void fb_set_logo(struct fb_info *info,
> Index: linux.git/drivers/video/offb.c
> =================================> --- linux.git.orig/drivers/video/offb.c
> +++ linux.git/drivers/video/offb.c
> @@ -91,15 +91,6 @@ extern boot_infos_t *boot_infos;
> #define AVIVO_DC_LUTB_WHITE_OFFSET_GREEN 0x6cd4
> #define AVIVO_DC_LUTB_WHITE_OFFSET_RED 0x6cd8
>
> -#define FB_RIGHT_POS(p, bpp) (fb_be_math(p) ? 0 : (32 - (bpp)))
> -
> -static inline u32 offb_cmap_byteswap(struct fb_info *info, u32 value)
> -{
> - u32 bpp = info->var.bits_per_pixel;
> -
> - return cpu_to_be32(value) >> FB_RIGHT_POS(info, bpp);
> -}
> -
> /*
> * Set a single color register. The values supplied are already
> * rounded down to the hardware's capabilities (according to the
> @@ -129,7 +120,7 @@ static int offb_setcolreg(u_int regno, u
> mask <<= info->var.transp.offset;
> value |= mask;
> }
> - pal[regno] = offb_cmap_byteswap(info, value);
> + pal[regno] = fb_cmap_byteswap(info, value);
> return 0;
> }
>
> Index: linux.git/include/linux/fb.h
> =================================> --- linux.git.orig/include/linux/fb.h
> +++ linux.git/include/linux/fb.h
> @@ -582,6 +582,7 @@ static inline struct apertures_struct *a
>
> #endif
>
> +#define FB_RIGHT_POS(p, bpp) (fb_be_math(p) ? 0 : (32 - (bpp)))
> #define FB_LEFT_POS(p, bpp) (fb_be_math(p) ? (32 - (bpp)) : 0)
> #define FB_SHIFT_HIGH(p, val, bits) (fb_be_math(p) ? (val) >> (bits) : \
> (val) << (bits))
> @@ -681,6 +682,13 @@ static inline bool fb_be_math(struct fb_
> #endif /* CONFIG_FB_FOREIGN_ENDIAN */
> }
>
> +static inline u32 fb_cmap_byteswap(struct fb_info *info, u32 value)
> +{
> + u32 bpp = info->var.bits_per_pixel;
> +
> + return cpu_to_be32(value) >> FB_RIGHT_POS(info, bpp);
> +}
> +
> /* drivers/video/fbsysfs.c */
> extern struct fb_info *framebuffer_alloc(size_t size, struct device *dev);
> extern void framebuffer_release(struct fb_info *info);
>
^ permalink raw reply
* Re: [PATCH] offb: Fix little-endian support
From: Cedric Le Goater @ 2014-05-14 14:01 UTC (permalink / raw)
To: Takashi Iwai, Tomi Valkeinen
Cc: Jean-Christophe Plagniol-Villard, linux-fbdev, linux-kernel
In-Reply-To: <1400073709-15012-1-git-send-email-tiwai@suse.de>
Hi Iwai-san,
On 05/14/2014 03:21 PM, Takashi Iwai wrote:
> Although the color palette was corrected for little endian by the
> commit [e1edf18b: offb: Add palette hack for little endian], the
> graphics mode is still shown in psychedelic colors.
Are you referring to the linux logo colors ? If so, could you please
try the patch below, it should be a fix.
> For fixing this
> properly, we rather need to correct the RGB offsets depending on
> endianess.
>
> Since the RGB base offsets are corrected, we don't need the hack for
> pallette color entries. This patch reverts that, too.
Are you testing using qemu -vga std -vnc :x ? If so, did you try changing
the depth to 8,15,16,32 ? I think the patch might be breaking big endian
too.
Now, I am far from being an expert on frame buffers. It would be glad
to have some insights on that topic.
Thanks,
C.
[PATCH] fb: fix logo palette entries for little endian
The offb_cmap_byteswap() routine helps byteswapping the color map
entries when required. This patch externalizes and renames the helper
routine to adjust the pseudo palette of the logo when running on
little endian.
Signed-off-by: Cédric Le Goater <clg@fr.ibm.com>
---
drivers/video/fbmem.c | 6 ++++--
drivers/video/offb.c | 11 +----------
include/linux/fb.h | 8 ++++++++
3 files changed, 13 insertions(+), 12 deletions(-)
Index: linux.git/drivers/video/fbmem.c
=================================--- linux.git.orig/drivers/video/fbmem.c
+++ linux.git/drivers/video/fbmem.c
@@ -252,7 +252,8 @@ static void fb_set_logo_truepalette(str
blueshift = info->var.blue.offset - (8 - info->var.blue.length);
for ( i = 0; i < logo->clutsize; i++) {
- palette[i+32] = (safe_shift((clut[0] & redmask), redshift) |
+ palette[i+32] = fb_cmap_byteswap(info,
+ safe_shift((clut[0] & redmask), redshift) |
safe_shift((clut[1] & greenmask), greenshift) |
safe_shift((clut[2] & bluemask), blueshift));
clut += 3;
@@ -271,7 +272,8 @@ static void fb_set_logo_directpalette(st
blueshift = info->var.blue.offset;
for (i = 32; i < 32 + logo->clutsize; i++)
- palette[i] = i << redshift | i << greenshift | i << blueshift;
+ palette[i] = fb_cmap_byteswap(info, i << redshift |
+ i << greenshift | i << blueshift);
}
static void fb_set_logo(struct fb_info *info,
Index: linux.git/drivers/video/offb.c
=================================--- linux.git.orig/drivers/video/offb.c
+++ linux.git/drivers/video/offb.c
@@ -91,15 +91,6 @@ extern boot_infos_t *boot_infos;
#define AVIVO_DC_LUTB_WHITE_OFFSET_GREEN 0x6cd4
#define AVIVO_DC_LUTB_WHITE_OFFSET_RED 0x6cd8
-#define FB_RIGHT_POS(p, bpp) (fb_be_math(p) ? 0 : (32 - (bpp)))
-
-static inline u32 offb_cmap_byteswap(struct fb_info *info, u32 value)
-{
- u32 bpp = info->var.bits_per_pixel;
-
- return cpu_to_be32(value) >> FB_RIGHT_POS(info, bpp);
-}
-
/*
* Set a single color register. The values supplied are already
* rounded down to the hardware's capabilities (according to the
@@ -129,7 +120,7 @@ static int offb_setcolreg(u_int regno, u
mask <<= info->var.transp.offset;
value |= mask;
}
- pal[regno] = offb_cmap_byteswap(info, value);
+ pal[regno] = fb_cmap_byteswap(info, value);
return 0;
}
Index: linux.git/include/linux/fb.h
=================================--- linux.git.orig/include/linux/fb.h
+++ linux.git/include/linux/fb.h
@@ -582,6 +582,7 @@ static inline struct apertures_struct *a
#endif
+#define FB_RIGHT_POS(p, bpp) (fb_be_math(p) ? 0 : (32 - (bpp)))
#define FB_LEFT_POS(p, bpp) (fb_be_math(p) ? (32 - (bpp)) : 0)
#define FB_SHIFT_HIGH(p, val, bits) (fb_be_math(p) ? (val) >> (bits) : \
(val) << (bits))
@@ -681,6 +682,13 @@ static inline bool fb_be_math(struct fb_
#endif /* CONFIG_FB_FOREIGN_ENDIAN */
}
+static inline u32 fb_cmap_byteswap(struct fb_info *info, u32 value)
+{
+ u32 bpp = info->var.bits_per_pixel;
+
+ return cpu_to_be32(value) >> FB_RIGHT_POS(info, bpp);
+}
+
/* drivers/video/fbsysfs.c */
extern struct fb_info *framebuffer_alloc(size_t size, struct device *dev);
extern void framebuffer_release(struct fb_info *info);
^ permalink raw reply
* [PATCH] offb: Fix little-endian support
From: Takashi Iwai @ 2014-05-14 13:21 UTC (permalink / raw)
To: Tomi Valkeinen
Cc: Jean-Christophe Plagniol-Villard, Cedric Le Goater, linux-fbdev,
linux-kernel
Although the color palette was corrected for little endian by the
commit [e1edf18b: offb: Add palette hack for little endian], the
graphics mode is still shown in psychedelic colors. For fixing this
properly, we rather need to correct the RGB offsets depending on
endianess.
Since the RGB base offsets are corrected, we don't need the hack for
pallette color entries. This patch reverts that, too.
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
drivers/video/fbdev/offb.c | 51 +++++++++++++++++++++++++++++-----------------
1 file changed, 32 insertions(+), 19 deletions(-)
diff --git a/drivers/video/fbdev/offb.c b/drivers/video/fbdev/offb.c
index 7d44d669d5b6..0224a62aa49d 100644
--- a/drivers/video/fbdev/offb.c
+++ b/drivers/video/fbdev/offb.c
@@ -93,13 +93,6 @@ extern boot_infos_t *boot_infos;
#define FB_RIGHT_POS(p, bpp) (fb_be_math(p) ? 0 : (32 - (bpp)))
-static inline u32 offb_cmap_byteswap(struct fb_info *info, u32 value)
-{
- u32 bpp = info->var.bits_per_pixel;
-
- return cpu_to_be32(value) >> FB_RIGHT_POS(info, bpp);
-}
-
/*
* Set a single color register. The values supplied are already
* rounded down to the hardware's capabilities (according to the
@@ -129,7 +122,7 @@ static int offb_setcolreg(u_int regno, u_int red, u_int green, u_int blue,
mask <<= info->var.transp.offset;
value |= mask;
}
- pal[regno] = offb_cmap_byteswap(info, value);
+ pal[regno] = value;
return 0;
}
@@ -451,6 +444,8 @@ static void __init offb_init_fb(const char *name, const char *full_name,
else
fix->visual = FB_VISUAL_TRUECOLOR;
+ info->flags = FBINFO_DEFAULT | FBINFO_MISC_FIRMWARE | foreign_endian;
+
var->xoffset = var->yoffset = 0;
switch (depth) {
case 8:
@@ -466,35 +461,54 @@ static void __init offb_init_fb(const char *name, const char *full_name,
break;
case 15: /* RGB 555 */
var->bits_per_pixel = 16;
- var->red.offset = 10;
+ if (fb_be_math(info)) {
+ var->red.offset = 10;
+ var->green.offset = 5;
+ var->blue.offset = 0;
+ } else {
+ var->red.offset = 0;
+ var->green.offset = 5;
+ var->blue.offset = 10;
+ }
var->red.length = 5;
- var->green.offset = 5;
var->green.length = 5;
- var->blue.offset = 0;
var->blue.length = 5;
var->transp.offset = 0;
var->transp.length = 0;
break;
case 16: /* RGB 565 */
var->bits_per_pixel = 16;
- var->red.offset = 11;
+ if (fb_be_math(info)) {
+ var->red.offset = 11;
+ var->green.offset = 5;
+ var->blue.offset = 0;
+ } else {
+ var->red.offset = 0;
+ var->green.offset = 5;
+ var->blue.offset = 11;
+ }
var->red.length = 5;
- var->green.offset = 5;
var->green.length = 6;
- var->blue.offset = 0;
var->blue.length = 5;
var->transp.offset = 0;
var->transp.length = 0;
break;
case 32: /* RGB 888 */
var->bits_per_pixel = 32;
- var->red.offset = 16;
+ if (fb_be_math(info)) {
+ var->red.offset = 16;
+ var->green.offset = 8;
+ var->blue.offset = 0;
+ var->transp.offset = 24;
+ } else {
+ var->red.offset = 8;
+ var->green.offset = 16;
+ var->blue.offset = 24;
+ var->transp.offset = 0;
+ }
var->red.length = 8;
- var->green.offset = 8;
var->green.length = 8;
- var->blue.offset = 0;
var->blue.length = 8;
- var->transp.offset = 24;
var->transp.length = 8;
break;
}
@@ -521,7 +535,6 @@ static void __init offb_init_fb(const char *name, const char *full_name,
info->fbops = &offb_ops;
info->screen_base = ioremap(address, fix->smem_len);
info->pseudo_palette = (void *) (info + 1);
- info->flags = FBINFO_DEFAULT | FBINFO_MISC_FIRMWARE | foreign_endian;
fb_alloc_cmap(&info->cmap, 256, 0);
--
1.9.2
^ permalink raw reply related
* Re: [alsa-devel] [PATCH 00/19] Rework OMAP4+ HDMI audio support
From: Jyri Sarha @ 2014-05-14 10:02 UTC (permalink / raw)
To: Joachim Eastwood
Cc: alsa-devel, linux-fbdev, devicetree, linux-omap, liam.r.girdwood,
detheridge, broonie, peter.ujfalusi, Tomi Valkeinen,
Benoit Cousson
In-Reply-To: <CAGhQ9VzjndCBp+ZHS3XPPvS4ors6u9BfHf4NY_HNYN5ynyQOYQ@mail.gmail.com>
On 05/13/2014 12:13 AM, Joachim Eastwood wrote:
> On 12 May 2014 11:12, Jyri Sarha <jsarha@ti.com> wrote:
...
>
> hey, this worked straight away :)
>
> But there seems to be something wrong with the channel mapping.
>
> For stereo (speaker-test -c 2) the mapping is correct.
>
> But for -c 4 and -c 8 it gets weird:
> speaker-test -c 4 -s X # where X is 1-4
> 1: Front Left is Rear Left
> 2: Front Right is Rear Right
> 3: Rear Right is Front Right
> 4: Rear Left is Front Left
>
> speaker-test -c 8 -s X # where X is 1-8
> 1: Front Left is Rear Left
> 2: Center is Rear Left
> 3: Front Right is Rear Right
> 4: Side Right is Front Right
> 5: Rear Right is silent
> 6: Rear Left is Center
> 7: Side Left is Front Left
> 8: LFE - Rear Right
>
> I think you need to check what channel order ALSA expects. I believe
> speaker-test does the right thing on my HTPC normally connected to my
> receiver.
>
I checked the implementation and there was indeed something weird there,
but the implementation can not explain the FL and FR channels jumping
around. FL and FL should always be the first two channels in all
configurations and the implementation does not touch them.
The implementation uses 8ch HDMI setup for anything above 2ch with
"Audio InfoFrame Data Byte 4" set to 0x13. According to CEA-861 specs
this means following channel order: FL, FR, LFE, FC, RL, RR, RLC, RRC
This is a closest match to ALSA 8ch mapping (according to
sound/core/pcm_lib.c) which is: FL, FR, FC, LFE, RL, RR, SL, SR
Current implementation has FLE and FC channels correctly swapped, but it
shifts them to last two channels and RL, RR, SL, SR are shifted down to
fill the place. This is all wrong and I'll try to come up with a fix for
that. Unfortunately I can not test anything beyond 2 ch myself so I
would need someone to volunteer to test my patch.
Best regards,
Jyri
^ permalink raw reply
* Re: [PATCH 1/4] OMAPDSS: Fix DSS clock multiplier issue on 3703 and probably 3630
From: Tomi Valkeinen @ 2014-05-14 6:41 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140513152626.GB16837@atomide.com>
[-- Attachment #1: Type: text/plain, Size: 4221 bytes --]
On 13/05/14 18:26, Tony Lindgren wrote:
> * Tomi Valkeinen <tomi.valkeinen@ti.com> [140512 07:45]:
>> On 12/05/14 17:39, Tony Lindgren wrote:
>>> * Tomi Valkeinen <tomi.valkeinen@ti.com> [140512 04:36]:
>>>> On 09/05/14 17:37, Tony Lindgren wrote:
>>>>>
>>>>> This is just the 3730-evm with the Sharp VGA panel mentioned in
>>>>> this series.
>>>>
>>>> Hmm, well, those both look fine. The fck is well below the maximum,
>>>> which is somewhere around 170MHz-180MHz. The lck/pck ratio is higher
>>>> with this patch, but that should affect the GFX overlay.
>>>>
>>>> So you're just booting, and there are no applications that use the
>>>> framebuffer? And there is no rotation or such configured?
>>>
>>> Right. The rotation is set to 3 though.
>>
>> Hmm, that's probably the issue then. VRFB rotation is very heavy on the
>> memory bandwidth, and is generally a very easy way to get sync lost errors.
>
> I found another case without rotation where the error always triggers.
> By forcing 3730-evm LCD panel to QVGA mode it always seems to happen
> even without rotation.
>
> Without this patch with errors and no image:
>
> # cat /sys/kernel/debug/omapdss/clk
> [ 55.185729] DSS: dss_runtime_get
> [ 55.189422] DSS: dss_runtime_put
> [ 55.192810] DISPC: dispc_runtime_get
> [ 55.196685] DISPC: dispc_runtime_put
> - DSS -
> DSS_FCK (DSS1_ALWON_FCLK) = 27000000
> - DISPC -
> dispc fclk source = DSS_FCK (DSS1_ALWON_FCLK)
> fck 27000000
> - LCD -
> LCD clk source = DSS_FCK (DSS1_ALWON_FCLK)
> lck 27000000 lck div 1
> pck 5400000 pck div 5
>
> With this patch without errors and penguin showing:
>
> # cat /sys/kernel/debug/omapdss/clk
> [ 19.905792] DSS: dss_runtime_get
> [ 19.909545] DSS: dss_runtime_put
> [ 19.912933] DISPC: dispc_runtime_get
> [ 19.916778] DISPC: dispc_runtime_put
> - DSS -
> DSS_FCK (DSS1_ALWON_FCLK) = 108000000
> - DISPC -
> dispc fclk source = DSS_FCK (DSS1_ALWON_FCLK)
> fck 108000000
> - LCD -
> LCD clk source = DSS_FCK (DSS1_ALWON_FCLK)
> lck 108000000 lck div 1
> pck 5400000 pck div 20
>
> In this case the errors are different too:
>
> DISPC: channel 0 xres 240 yres 320
> DISPC: pck 5400000
> DISPC: hsw 3 hfp 3 hbp 39 vsw 1 vfp 2 vbp 7
> DISPC: vsync_level 1 hsync_level 1 data_pclk_edge 1 de_level 0 sync_pclk_edge 0
> DISPC: hsync 18947Hz, vsync 57Hz
> DISPC: lck = 27000000 (1)
> DISPC: pck = 5400000 (5)
> APPLY: DISPC IRQ: 0x60: GFX_FIFO_UNDERFLOW
> APPLY: DISPC IRQ: 0x4062: GFX_FIFO_UNDERFLOW SYNC_LOST
> DISPC: dispc_runtime_get
> omapdss APPLY error: FIFO UNDERFLOW on gfx, disabling the overlay
I'm quite out of ideas... The pixel clock is so low that underflow
shouldn't really happen. I was trying to find 3730 documentation that
would describe the PRCM's multipliers and dividers for DSS (i.e. the
ones you change in this patch), but I didn't find anything.
Maybe the DSS clock's fixed multiplier has been changed for 3730. But in
that case our .dts files related to the clocks are wrong too. The clock
nodes related to this are dpll4_m4x2_mul_ck, which is corresponds to the
"dss_fck_multiplier" value in omapdss, and dpll4_m4_ck, which
corresponds to the "fck_div_max" value.
One thing to try out is, without this patch, change
CONFIG_OMAP2_DSS_MIN_FCK_PER_PCK to force the dss func clock higher.
With QVGA mode, you could try, say, value of 19, which should make sure
the fck is at least 19 times higher than pck.
> Regarding rotation, it does look like that with VGA mode enabling
> rotation makes the error trigger quite often on 3730.
>
>> will handle it fine with the clock rates and bandwidth usage you have
>> for your use cases.
>
> I don't think it's all because of rotation. And rotating my head
No, probably not. The pixel clocks are so low that the rotation should
work fine. Except if there's something wrong the the VRFB or the memory
bus, which make the VRFB rotation not work even with low pixel clocks.
> makes my neck hurt!
You need to learn to hold the device a bit differently!
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH 3/4] OMAPDSS: panel-sharp-ls037v7dw01: add device tree support
From: Tomi Valkeinen @ 2014-05-14 6:19 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140513152518.GA16837@atomide.com>
[-- Attachment #1: Type: text/plain, Size: 5589 bytes --]
On 13/05/14 18:25, Tony Lindgren wrote:
> Well ideally the revision info for a device would come from device
> revision registers rather using the SoC revision. In the DSS case when
> the SoC revision is needed by a device it maybe it can be deciphered
> from a combination of compatible flag and the clock rate for example?
I've been trying that. The HW guys didn't bother to update the DSS
revision registers, so they are useless. And, for example, the OMAP3 ES
difference is only about bitfield widths in two registers.
I tried writing "too long" value to the register on the earlier ES
version, hoping that the extra bits would be kept at zero, but that
wasn't the case. So I just don't see a way to detect this from the DSS's
point of view.
>>> Do you object to the compatible string remapping as such, or just that
>>> it's in arch/arm/mach-omap2?
>
> It's something I'd rather not have under mach-omap2 as that means that
> I may need to deal with it too to some extent. And I don't think we
> need to do such remapping, we should be able to use the panel compatible
> strings as they are just fine. It should be possible to figure out from
> the device tree properties what controller the panel belongs to. Or
> for now, use the panel registration to figure out what display controller
> it belongs to.
>
>>> I guess nothing prevents me from moving it to drivers/, and having some
>>> early-ish initcall doing the job.
>
> /me likes this idea if you need it at all. Stuff like this tends to stay
> and spread, so I'd rather not do the remapping of compatible strings at
> all.
Yep. I'll look to this. Thinking about it now, it kind of makes more
sense to have it in the omapdss's directory.
>> So, since we can change the kernel later but not the DTS, I agree with
>> you that the remapping is the least bad of our options.
>
> Yes the binding for the panel should just describe the panel so it can be
> used with whatever display controller. But we do have quite a few buses
> probing devices. How about set up the panel probing the same way?
> For the panels on display controller, just do the usual
> for_each_child_of_node(pdev->dev.of_node, child) and probe them?
>
> It seems the remapping of compatible strings is not needed in this
> as we're only picking up panels that are children of the display
> controller.
The panels (or display encoders) are not (usually) children of the
display controller. They are children of their respective control bus.
Say, an i2c panel is a child of i2c bus. If there's no control bus, like
is the case with the sharp panel, it's a platform device.
The video paths of the panels and encoders are connected using the v4l2
style ports/endpoints. We can use those to see what display controller a
panel is connected to, but only after the panel driver has already
probed. We don't have control for the actual probing, as that happens
with whatever the control bus is for the display component.
>>>>> I'm not sure what it would give us to try to be compatible with
>>>>> simple-panel.txt. The simple-panel bindings won't probably be compatible
>>>>> with the future common display drivers in any case, as the simple-panel
>>>>> binding is just too limited and doesn't describe the hardware fully.
>>>>
>>>> Hmm what else does a panel need where the existing binding cannot be
>>>> extended?
>>>
>>> The existing simple-panel binding doesn't describe the connections of
>>> the panel, i.e. its video input. I guess it can be extended, but I don't
>>> see what the benefit is of trying to create new panel bindings
>>> compatible with the simple-panel bindings. As I see, the simple-panel
>>> bindings work only with very limited use cases, where the drivers make
>>> assumptions. Simple panel bindings cannot be used with omapdss, nor can
>>> it be used with the future common display framework.
>
> Well it seems at least the reset and enable pin standard from that
> binding can be kept.
Only enable gpio there. But even that's vague. Do you turn on the power
before or after setting the enable gpio? How long delay should be
between the power and the gpio? Different panels have different rules
for the power-up.
>>> But I'm not really familiar with using extending current bindings, and
>>> making new bindings compatible with old ones. Can you explain why it'd
>>> be good to have the sharp panel bindings compatible with simple-panel?
>>> In what kind of scenario would it be used?
>
> Ideally the panel binding just describes the panel and it should not
> matter which display controller it is a child of.
Yes, but that means the panel bindings need to have enough information
so that all display controllers can use it. Simple-panel bindings do not
have enough information. The simple-panel bindings do not have
information about the video bus input, and it doesn't even have
information about the resolution or bitdepth of the panel.
So I'm still asking, if we create sharp bindings that use the same
properties as the simple-panel bindings, and define that sharp panel is
compatible with simple-panel, what kind of scenario in practice would it
be used in?
Would the scenario be some other OS, that doesn't have a driver for the
sharp panel, but has a driver for the simple-panel? That would only work
if the sharp panel hardware is setup so that only the enable gpio is
needed, so that quite a narrow definition of "compatible".
Or is there some other scenario in which it could be used?
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 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