* Re: [PATCH 11/11] ARM: versatile: move CLCD configuration to device tree
From: Russell King - ARM Linux @ 2016-02-23 12:01 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAFEAcA9U3V04o2_93bS5G7+k=UT7kTWt4=v7t1Ju0X9W=5oo9A@mail.gmail.com>
On Tue, Feb 23, 2016 at 11:56:34AM +0000, Peter Maydell wrote:
> On 23 February 2016 at 09:58, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> > I'm not quite sure how it works if, as in versatile display case, there
> > are multiple DT overlays of which one has to be enabled. I hope there's
> > support to choose which one to use via kernel cmdline or similar.
> >
> > I would personally like it much more if the bootloader would either
> > compose a final dtb from DT fragments and pass it to the kernel, or
> > alternatively the bootloader would pass the base dtb image and a bunch
> > of DT overlays to the kernel, and the kernel would deal with the DT
> > overlays.
>
> Speaking as somebody who's written the "bootloader" code that's
> used for what I guess are the majority of versatile kernel boots,
> i.e. the one in QEMU, I think that requiring the bootloader to do this
> would be a significant worsening from the current state.
>
> Right now the bootloader doesn't need to do much at all with device
> trees, except pass the kernel the DT that the user gave us, which
> is just the kernel's own data structures in a separate file for
> convenience. You need to do some very minor tweaks to the /chosen
> node, but these can be handled the same way for any board and aren't
> hardware specific. There's no need to worry about dt fragments
> either for the bootloader or for the user. Imposing a new requirement
> for the bootloader to have to probe hardware which it otherwise
> has no need to even care about, and then edit and update the DT
> in a board-specific manner, or have board-specific DT fragments,
> seems like a totally unnecessary imposition on both bootloader
> authors and end-users, and of course it would break booting newer
> kernels on the great mass of already existing boot loaders and
> QEMU installs.
>
> The kernel is in a position to probe the display hardware and determine
> what is there, and do the right thing, and that's exactly what it
> does today. The kernel should continue to do this.
> The advantage of DT is that it allows moving information about
> non-probeable hardware that was previously hardwired in the kernel
> C sources into a separate data structure, but the versatile displays
> are not non-probeable. I can see no benefit at all from hardwiring into
> the dt something which the kernel has previously been successfully
> dynamically getting right without any bootloader intervention -- it just
> makes the kernel less flexible and less user-friendly.
+1.
--
RMK's Patch system: http://www.arm.linux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
^ permalink raw reply
* Re: [PATCH 11/11] ARM: versatile: move CLCD configuration to device tree
From: Peter Maydell @ 2016-02-23 11:56 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <56CC2D39.80909@ti.com>
On 23 February 2016 at 09:58, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> I'm not quite sure how it works if, as in versatile display case, there
> are multiple DT overlays of which one has to be enabled. I hope there's
> support to choose which one to use via kernel cmdline or similar.
>
> I would personally like it much more if the bootloader would either
> compose a final dtb from DT fragments and pass it to the kernel, or
> alternatively the bootloader would pass the base dtb image and a bunch
> of DT overlays to the kernel, and the kernel would deal with the DT
> overlays.
Speaking as somebody who's written the "bootloader" code that's
used for what I guess are the majority of versatile kernel boots,
i.e. the one in QEMU, I think that requiring the bootloader to do this
would be a significant worsening from the current state.
Right now the bootloader doesn't need to do much at all with device
trees, except pass the kernel the DT that the user gave us, which
is just the kernel's own data structures in a separate file for
convenience. You need to do some very minor tweaks to the /chosen
node, but these can be handled the same way for any board and aren't
hardware specific. There's no need to worry about dt fragments
either for the bootloader or for the user. Imposing a new requirement
for the bootloader to have to probe hardware which it otherwise
has no need to even care about, and then edit and update the DT
in a board-specific manner, or have board-specific DT fragments,
seems like a totally unnecessary imposition on both bootloader
authors and end-users, and of course it would break booting newer
kernels on the great mass of already existing boot loaders and
QEMU installs.
The kernel is in a position to probe the display hardware and determine
what is there, and do the right thing, and that's exactly what it
does today. The kernel should continue to do this.
The advantage of DT is that it allows moving information about
non-probeable hardware that was previously hardwired in the kernel
C sources into a separate data structure, but the versatile displays
are not non-probeable. I can see no benefit at all from hardwiring into
the dt something which the kernel has previously been successfully
dynamically getting right without any bootloader intervention -- it just
makes the kernel less flexible and less user-friendly.
thanks
-- PMM
^ permalink raw reply
* Re: [PATCH 11/11] ARM: versatile: move CLCD configuration to device tree
From: Arnd Bergmann @ 2016-02-23 11:22 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CACRpkdZjfjOCFUucF1U5yVZBxSwa41GKEMv-HHQE=ymF2NZ21g@mail.gmail.com>
On Tuesday 23 February 2016 11:10:33 Linus Walleij wrote:
>
> ERROR (duplicate_label): ERROR (duplicate_label): Duplicate label
> 'clcd_panel' on /panel@1/port/endpoint and /panel@0/port/endpoint
> Duplicate label 'clcd_panel' on /panel@1/port/endpoint and
> /panel@0/port/endpoint
>
> So the dtc does not allow duplicate node labels for the phandle
> even if the node it resides in is disabled. Tough luck...
Ah, too bad. I guess we could still do it by also setting the
remote-endpoint property to node->phandle, but then it's
not as simple any more.
Arnd
^ permalink raw reply
* Re: [PATCH 11/11] ARM: versatile: move CLCD configuration to device tree
From: Pantelis Antoniou @ 2016-02-23 10:59 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAHCN7xLo7q=Ts3WLvx_GzSBWt5r1SiU7vr+W==kUAaAvmwXJYA@mail.gmail.com>
Hi Adam,
> On Feb 23, 2016, at 12:32 , Adam Ford <aford173@gmail.com> wrote:
>
> On Tue, Feb 23, 2016 at 3:58 AM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
>> (cc'ing a few more people as this is going into generic direction)
>>
>> On 23/02/16 11:08, Linus Walleij wrote:
>>> On Thu, Feb 4, 2016 at 3:04 PM, Linus Walleij <linus.walleij@linaro.org> wrote:
>>>
>>>> This moves the versatile CLCD configuration to the device tree by:
>>>
>>> As this Schrödinger's cat approach seems controversial, as well
>>> as the alternative to manipulate the DT in memory at run-time,
>>> I will respin the series without the full Versatile support, adding
>>> plug-in for the other ARM boards and and half-baked support
>>> for the Versatile supporting just VGA (like the other reference
>>> designs from ARM).
>>>
>>> The pluggable displays prove yet again to be a problem to the
>>> world, sigh.
>>>
>>> I will think of a better solution, if any, for this for v4.6, but will
>>> put forward something that handles the Nomadik and all the
>>> other ARM reference designs for now.
>>
>> I had a chat with Tom Rini and Pantelis Antoniou yesterday.
>>
>> Panto is about to send a new series for DT overlay management soon. I
>> haven't had time to test that yet, but what it would give us is that you
>> could build DT overlay binaries as "firmware" into the kernel image, and
>> thus the panel DT data would be there even before rootfs is mounted. The
>> DT overlays can be loaded from the rootfs or initramfs too, if it's not
>> critical to get the display up early.
>>
>> I'm not quite sure how it works if, as in versatile display case, there
>> are multiple DT overlays of which one has to be enabled. I hope there's
>> support to choose which one to use via kernel cmdline or similar.
>>
>> I would personally like it much more if the bootloader would either
>> compose a final dtb from DT fragments and pass it to the kernel, or
>> alternatively the bootloader would pass the base dtb image and a bunch
>> of DT overlays to the kernel, and the kernel would deal with the DT
>> overlays.
>>
> If there was a nice way to merge the device trees, I would love to
> create device tree 'modules' that could be loaded at will and merged
> just before the time of boot. I could see this being useful beyond
> just plugable displays.
>
Looks like you would be happy by having a way to boot using a device tree
blob + a number of device tree overlay blobs to be applied in sequence.
You could append the dtbo’s to the device tree blob (where-ever that may be)
and it should work.
The good thing about this scheme is that it’s independent of the bootloader.
An advanced bootloader (like u-boot) with DT smarts can create the appended
DT blob itself, while a dumb one can just be given the appended blob as
it was created outside of the device.
>> In any case, I think the firmware solution is a good step forward, and
>> will probably work fine for many users. Then again, I haven't tested it
>> yet so I don't know the details, and it's not in the mainline.
>>
> I am willing to test it, but I'll need the patch and some instructions
> from Panto.
>
The appended device tree blob thing does not exist yet, but it’s not such a big
problem to add. I’d give it a shot this week.
The standard device tree overlays are in my overlay branch at
https://github.com/pantoniou/linux-beagle-track-mainline/tree/bbb-overlays
The capemgr has options for parsing a kernel command line to apply an overlay:
https://github.com/pantoniou/linux-beagle-track-mainline/blob/bbb-overlays/drivers/misc/bone_capemgr.c
The command line options are enable_partno & disable_partno where you supply
the part numbers of the capes you want enabled/disabled at boot.
The capemgr is complicated due to the eeprom detection and the horrible kludges done with the
variants (beaglebone white/beaglebone black), but I think you can figure out what’s going
on with the command line options.
>> Tomi
>>
Regards
— Pantelis
^ permalink raw reply
* Re: [PATCH 4/7 v2] video: ARM CLCD: support pads connected in reverse order
From: Russell King - ARM Linux @ 2016-02-23 10:35 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1456221704-5792-5-git-send-email-linus.walleij@linaro.org>
On Tue, Feb 23, 2016 at 11:01:41AM +0100, Linus Walleij wrote:
> - if (var->red.offset = 0)
> - val &= ~CNTL_BGR;
> - else
> - val |= CNTL_BGR;
> + if (!fb->panel->bgr_connection) {
> + if (var->red.offset = 0)
> + val &= ~CNTL_BGR;
> + else
> + val |= CNTL_BGR;
> + } else {
> + if (var->blue.offset = 0)
> + val &= ~CNTL_BGR;
> + else
> + val |= CNTL_BGR;
> + }
if (fb->panel->bgr_connection)
val ^= CNTL_BGR;
is a shorter way to write the above, and probably easier on the compiler
too.
--
RMK's Patch system: http://www.arm.linux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
^ permalink raw reply
* Re: [PATCH 11/11] ARM: versatile: move CLCD configuration to device tree
From: Adam Ford @ 2016-02-23 10:32 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <56CC2D39.80909@ti.com>
On Tue, Feb 23, 2016 at 3:58 AM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> (cc'ing a few more people as this is going into generic direction)
>
> On 23/02/16 11:08, Linus Walleij wrote:
>> On Thu, Feb 4, 2016 at 3:04 PM, Linus Walleij <linus.walleij@linaro.org> wrote:
>>
>>> This moves the versatile CLCD configuration to the device tree by:
>>
>> As this Schrödinger's cat approach seems controversial, as well
>> as the alternative to manipulate the DT in memory at run-time,
>> I will respin the series without the full Versatile support, adding
>> plug-in for the other ARM boards and and half-baked support
>> for the Versatile supporting just VGA (like the other reference
>> designs from ARM).
>>
>> The pluggable displays prove yet again to be a problem to the
>> world, sigh.
>>
>> I will think of a better solution, if any, for this for v4.6, but will
>> put forward something that handles the Nomadik and all the
>> other ARM reference designs for now.
>
> I had a chat with Tom Rini and Pantelis Antoniou yesterday.
>
> Panto is about to send a new series for DT overlay management soon. I
> haven't had time to test that yet, but what it would give us is that you
> could build DT overlay binaries as "firmware" into the kernel image, and
> thus the panel DT data would be there even before rootfs is mounted. The
> DT overlays can be loaded from the rootfs or initramfs too, if it's not
> critical to get the display up early.
>
> I'm not quite sure how it works if, as in versatile display case, there
> are multiple DT overlays of which one has to be enabled. I hope there's
> support to choose which one to use via kernel cmdline or similar.
>
> I would personally like it much more if the bootloader would either
> compose a final dtb from DT fragments and pass it to the kernel, or
> alternatively the bootloader would pass the base dtb image and a bunch
> of DT overlays to the kernel, and the kernel would deal with the DT
> overlays.
>
If there was a nice way to merge the device trees, I would love to
create device tree 'modules' that could be loaded at will and merged
just before the time of boot. I could see this being useful beyond
just plugable displays.
> In any case, I think the firmware solution is a good step forward, and
> will probably work fine for many users. Then again, I haven't tested it
> yet so I don't know the details, and it's not in the mainline.
>
I am willing to test it, but I'll need the patch and some instructions
from Panto.
> Tomi
>
^ permalink raw reply
* Re: [PATCH 11/11] ARM: versatile: move CLCD configuration to device tree
From: Linus Walleij @ 2016-02-23 10:10 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <3695074.sKSIyOjpjD@wuerfel>
On Tue, Feb 23, 2016 at 10:34 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Tuesday 23 February 2016 10:08:05 Linus Walleij wrote:
>> I will think of a better solution, if any, for this for v4.6, but will
>> put forward something that handles the Nomadik and all the
>> other ARM reference designs for now.
>>
>
> How about still describing all known panels in the DT marked 'status="disabled"',
> but then having a minimal piece of board specific code that just enables
> whichever one gets detected at boot time?
Close but no cigar :D
It means that all panels (enabled and all disabled) have to have the same
endpoint in the of_graph.
I tried it:
panel@0 {
compatible = "panel-dpi";
port {
clcd_panel: endpoint {
remote-endpoint = <&clcd_pads>;
};
};
(...)
panel@1 {
compatible = "panel-dpi";
port {
clcd_panel: endpoint {
remote-endpoint = <&clcd_pads>;
};
};
(...)
display@10120000 {
(...)
port {
(...)
clcd_pads: endpoint@0 {
reg = <0>;
remote-endpoint = <&clcd_panel>;
arm,pl11x,tft-r0g0b0-pads = <1 7 13>;
};
};
Building the device tree:
ERROR (duplicate_label): ERROR (duplicate_label): Duplicate label
'clcd_panel' on /panel@1/port/endpoint and /panel@0/port/endpoint
Duplicate label 'clcd_panel' on /panel@1/port/endpoint and
/panel@0/port/endpoint
So the dtc does not allow duplicate node labels for the phandle
even if the node it resides in is disabled. Tough luck...
Yours,
Linus Walleij
^ permalink raw reply
* [PATCH 7/7 v2] video: ARM CLCD: add special panel hook for Versatiles
From: Linus Walleij @ 2016-02-23 10:01 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1456221704-5792-1-git-send-email-linus.walleij@linaro.org>
This adds a special panel init hook for the ARM reference designs
Integrator (IM-PD1), Versatile and RealView, so we can configure
a DPI panel from device tree and have it working without
boardfiles for these machines.
Basically this is the same code as from the board files, just
moved over to look up the syscon DT node and manipulate the
special CLCD register from their regmap.
Tested on RealView PB11MPcore.
Cc: Pawel Moll <pawel.moll@arm.com>
Cc: Rob Herring <robh@kernel.org>
Cc: Russell King <linux@arm.linux.org.uk>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
ChangeLog v1->v2:
- For Versatile: just detect the panel and look up and compare
the expected compatible string to what is provided by the device
tree. If the device tree does not match the hardware-detected
display, print a warning and continue anyways.
- Make no attempt at actually switching the panel at runtime or
to modify the device tree at runtime.
---
drivers/video/fbdev/Kconfig | 2 +
drivers/video/fbdev/amba-clcd-versatile.c | 391 ++++++++++++++++++++++++++++++
drivers/video/fbdev/amba-clcd-versatile.h | 17 ++
drivers/video/fbdev/amba-clcd.c | 4 +-
4 files changed, 413 insertions(+), 1 deletion(-)
create mode 100644 drivers/video/fbdev/amba-clcd-versatile.h
diff --git a/drivers/video/fbdev/Kconfig b/drivers/video/fbdev/Kconfig
index 6dcd91237551..a3fdc4cd1837 100644
--- a/drivers/video/fbdev/Kconfig
+++ b/drivers/video/fbdev/Kconfig
@@ -307,6 +307,8 @@ config PLAT_VERSATILE_CLCD
def_bool ARCH_VERSATILE || ARCH_REALVIEW || ARCH_VEXPRESS || ARCH_INTEGRATOR
depends on ARM
depends on FB_ARMCLCD && FB=y
+ select REGMAP
+ select MFD_SYSCON
config FB_ACORN
bool "Acorn VIDC support"
diff --git a/drivers/video/fbdev/amba-clcd-versatile.c b/drivers/video/fbdev/amba-clcd-versatile.c
index 7a8afcd4573e..892bbf6c6a07 100644
--- a/drivers/video/fbdev/amba-clcd-versatile.c
+++ b/drivers/video/fbdev/amba-clcd-versatile.c
@@ -3,6 +3,12 @@
#include <linux/amba/bus.h>
#include <linux/amba/clcd.h>
#include <linux/platform_data/video-clcd-versatile.h>
+#include <linux/of.h>
+#include <linux/of_graph.h>
+#include <linux/regmap.h>
+#include <linux/mfd/syscon.h>
+#include <linux/bitops.h>
+#include "amba-clcd-versatile.h"
static struct clcd_panel vga = {
.mode = {
@@ -180,3 +186,388 @@ void versatile_clcd_remove_dma(struct clcd_fb *fb)
dma_free_writecombine(&fb->dev->dev, fb->fb.fix.smem_len,
fb->fb.screen_base, fb->fb.fix.smem_start);
}
+
+#ifdef CONFIG_OF
+
+static struct regmap *versatile_syscon_map;
+static struct regmap *versatile_ib2_map;
+
+/*
+ * We detect the different syscon types from the compatible strings.
+ */
+enum versatile_clcd {
+ INTEGRATOR_CLCD_CM,
+ VERSATILE_CLCD,
+ REALVIEW_CLCD_EB,
+ REALVIEW_CLCD_PB1176,
+ REALVIEW_CLCD_PB11MP,
+ REALVIEW_CLCD_PBA8,
+ REALVIEW_CLCD_PBX,
+};
+
+static const struct of_device_id versatile_clcd_of_match[] = {
+ {
+ .compatible = "arm,core-module-integrator",
+ .data = (void *)INTEGRATOR_CLCD_CM,
+ },
+ {
+ .compatible = "arm,versatile-sysreg",
+ .data = (void *)VERSATILE_CLCD,
+ },
+ {
+ .compatible = "arm,realview-eb-syscon",
+ .data = (void *)REALVIEW_CLCD_EB,
+ },
+ {
+ .compatible = "arm,realview-pb1176-syscon",
+ .data = (void *)REALVIEW_CLCD_PB1176,
+ },
+ {
+ .compatible = "arm,realview-pb11mp-syscon",
+ .data = (void *)REALVIEW_CLCD_PB11MP,
+ },
+ {
+ .compatible = "arm,realview-pba8-syscon",
+ .data = (void *)REALVIEW_CLCD_PBA8,
+ },
+ {
+ .compatible = "arm,realview-pbx-syscon",
+ .data = (void *)REALVIEW_CLCD_PBX,
+ },
+ {},
+};
+
+/*
+ * Core module CLCD control on the Integrator/CP, bits
+ * 8 thru 19 of the CM_CONTROL register controls a bunch
+ * of CLCD settings.
+ */
+#define INTEGRATOR_HDR_CTRL_OFFSET 0x0C
+#define INTEGRATOR_CLCD_LCDBIASEN BIT(8)
+#define INTEGRATOR_CLCD_LCDBIASUP BIT(9)
+#define INTEGRATOR_CLCD_LCDBIASDN BIT(10)
+/* Bits 11,12,13 controls the LCD type */
+#define INTEGRATOR_CLCD_LCDMUX_MASK (BIT(11)|BIT(12)|BIT(13))
+#define INTEGRATOR_CLCD_LCDMUX_LCD24 BIT(11)
+#define INTEGRATOR_CLCD_LCDMUX_VGA565 BIT(12)
+#define INTEGRATOR_CLCD_LCDMUX_SHARP (BIT(11)|BIT(12))
+#define INTEGRATOR_CLCD_LCDMUX_VGA555 BIT(13)
+#define INTEGRATOR_CLCD_LCDMUX_VGA24 (BIT(11)|BIT(12)|BIT(13))
+#define INTEGRATOR_CLCD_LCD0_EN BIT(14)
+#define INTEGRATOR_CLCD_LCD1_EN BIT(15)
+/* R/L flip on Sharp */
+#define INTEGRATOR_CLCD_LCD_STATIC1 BIT(16)
+/* U/D flip on Sharp */
+#define INTEGRATOR_CLCD_LCD_STATIC2 BIT(17)
+/* No connection on Sharp */
+#define INTEGRATOR_CLCD_LCD_STATIC BIT(18)
+/* 0 = 24bit VGA, 1 = 18bit VGA */
+#define INTEGRATOR_CLCD_LCD_N24BITEN BIT(19)
+
+#define INTEGRATOR_CLCD_MASK (INTEGRATOR_CLCD_LCDMUX_MASK | \
+ INTEGRATOR_CLCD_LCD0_EN | \
+ INTEGRATOR_CLCD_LCD1_EN | \
+ INTEGRATOR_CLCD_LCD_STATIC1 | \
+ INTEGRATOR_CLCD_LCD_STATIC2 | \
+ INTEGRATOR_CLCD_LCD_STATIC | \
+ INTEGRATOR_CLCD_LCD_N24BITEN)
+
+static void integrator_clcd_enable(struct clcd_fb *fb)
+{
+ struct fb_var_screeninfo *var = &fb->fb.var;
+ u32 val;
+
+ dev_info(&fb->dev->dev, "enable Integrator CLCD connectors\n");
+
+ val = INTEGRATOR_CLCD_LCD_STATIC1 | INTEGRATOR_CLCD_LCD_STATIC2 |
+ INTEGRATOR_CLCD_LCD0_EN | INTEGRATOR_CLCD_LCD1_EN;
+ if (var->bits_per_pixel <= 8 ||
+ (var->bits_per_pixel = 16 && var->green.length = 5))
+ /* Pseudocolor, RGB555, BGR555 */
+ val |= INTEGRATOR_CLCD_LCDMUX_VGA555;
+ else if (fb->fb.var.bits_per_pixel <= 16)
+ /* truecolor RGB565 */
+ val |= INTEGRATOR_CLCD_LCDMUX_VGA565;
+ else
+ val = 0; /* no idea for this, don't trust the docs */
+
+ regmap_update_bits(versatile_syscon_map,
+ INTEGRATOR_HDR_CTRL_OFFSET,
+ 0,
+ INTEGRATOR_CLCD_MASK);
+}
+
+/*
+ * This configuration register in the Versatile and RealView
+ * family is uniformly present but appears more and more
+ * unutilized starting with the RealView series.
+ */
+#define SYS_CLCD 0x50
+#define SYS_CLCD_MODE_MASK (BIT(0)|BIT(1))
+#define SYS_CLCD_MODE_888 0
+#define SYS_CLCD_MODE_5551 BIT(0)
+#define SYS_CLCD_MODE_565_R_LSB BIT(1)
+#define SYS_CLCD_MODE_565_B_LSB (BIT(0)|BIT(1))
+#define SYS_CLCD_CONNECTOR_MASK (BIT(2)|BIT(3)|BIT(4)|BIT(5))
+#define SYS_CLCD_NLCDIOON BIT(2)
+#define SYS_CLCD_VDDPOSSWITCH BIT(3)
+#define SYS_CLCD_PWR3V5SWITCH BIT(4)
+#define SYS_CLCD_VDDNEGSWITCH BIT(5)
+#define SYS_CLCD_TSNSS BIT(6) /* touchscreen enable */
+#define SYS_CLCD_SSPEXP BIT(7) /* SSP expansion enable */
+
+/* The Versatile can detect the connected panel type */
+#define SYS_CLCD_CLCDID_MASK (BIT(8)|BIT(9)|BIT(10)|BIT(11)|BIT(12))
+#define SYS_CLCD_ID_SANYO_3_8 (0x00 << 8)
+#define SYS_CLCD_ID_SHARP_8_4 (0x01 << 8)
+#define SYS_CLCD_ID_EPSON_2_2 (0x02 << 8)
+#define SYS_CLCD_ID_SANYO_2_5 (0x07 << 8)
+#define SYS_CLCD_ID_VGA (0x1f << 8)
+
+#define SYS_CLCD_TSNDAV BIT(13) /* data ready from TS */
+
+/* IB2 control register for the Versatile daughterboard */
+#define IB2_CTRL 0x00
+#define IB2_CTRL_LCD_SD BIT(1) /* 1 = shut down LCD */
+#define IB2_CTRL_LCD_BL_ON BIT(0)
+#define IB2_CTRL_LCD_MASK (BIT(0)|BIT(1))
+
+static void versatile_clcd_disable(struct clcd_fb *fb)
+{
+ dev_info(&fb->dev->dev, "disable Versatile CLCD connectors\n");
+ regmap_update_bits(versatile_syscon_map,
+ SYS_CLCD,
+ SYS_CLCD_CONNECTOR_MASK,
+ 0);
+
+ /* If we're on an IB2 daughterboard, turn off display */
+ if (versatile_ib2_map) {
+ dev_info(&fb->dev->dev, "disable IB2 display\n");
+ regmap_update_bits(versatile_ib2_map,
+ IB2_CTRL,
+ IB2_CTRL_LCD_MASK,
+ IB2_CTRL_LCD_SD);
+ }
+}
+
+static void versatile_clcd_enable(struct clcd_fb *fb)
+{
+ struct fb_var_screeninfo *var = &fb->fb.var;
+ u32 val = 0;
+
+ dev_info(&fb->dev->dev, "enable Versatile CLCD connectors\n");
+ switch (var->green.length) {
+ case 5:
+ val |= SYS_CLCD_MODE_5551;
+ break;
+ case 6:
+ if (var->red.offset = 0)
+ val |= SYS_CLCD_MODE_565_R_LSB;
+ else
+ val |= SYS_CLCD_MODE_565_B_LSB;
+ break;
+ case 8:
+ val |= SYS_CLCD_MODE_888;
+ break;
+ }
+
+ /* Set up the MUX */
+ regmap_update_bits(versatile_syscon_map,
+ SYS_CLCD,
+ SYS_CLCD_MODE_MASK,
+ val);
+
+ /* Then enable the display */
+ regmap_update_bits(versatile_syscon_map,
+ SYS_CLCD,
+ SYS_CLCD_CONNECTOR_MASK,
+ SYS_CLCD_NLCDIOON | SYS_CLCD_PWR3V5SWITCH);
+
+ /* If we're on an IB2 daughterboard, turn on display */
+ if (versatile_ib2_map) {
+ dev_info(&fb->dev->dev, "enable IB2 display\n");
+ regmap_update_bits(versatile_ib2_map,
+ IB2_CTRL,
+ IB2_CTRL_LCD_MASK,
+ IB2_CTRL_LCD_BL_ON);
+ }
+}
+
+static void versatile_clcd_decode(struct clcd_fb *fb, struct clcd_regs *regs)
+{
+ clcdfb_decode(fb, regs);
+
+ /* Always clear BGR for RGB565: we do the routing externally */
+ if (fb->fb.var.green.length = 6)
+ regs->cntl &= ~CNTL_BGR;
+}
+
+static void realview_clcd_disable(struct clcd_fb *fb)
+{
+ dev_info(&fb->dev->dev, "disable RealView CLCD connectors\n");
+ regmap_update_bits(versatile_syscon_map,
+ SYS_CLCD,
+ SYS_CLCD_CONNECTOR_MASK,
+ 0);
+}
+
+static void realview_clcd_enable(struct clcd_fb *fb)
+{
+ dev_info(&fb->dev->dev, "enable RealView CLCD connectors\n");
+ regmap_update_bits(versatile_syscon_map,
+ SYS_CLCD,
+ SYS_CLCD_CONNECTOR_MASK,
+ SYS_CLCD_NLCDIOON | SYS_CLCD_PWR3V5SWITCH);
+}
+
+struct versatile_panel {
+ u32 id;
+ char *compatible;
+ bool ib2;
+};
+
+static const struct versatile_panel versatile_panels[] = {
+ {
+ .id = SYS_CLCD_ID_VGA,
+ .compatible = "VGA",
+ },
+ {
+ .id = SYS_CLCD_ID_SANYO_3_8,
+ .compatible = "sanyo,tm38qv67a02a",
+ },
+ {
+ .id = SYS_CLCD_ID_SHARP_8_4,
+ .compatible = "sharp,lq084v1dg21",
+ },
+ {
+ .id = SYS_CLCD_ID_EPSON_2_2,
+ .compatible = "epson,l2f50113t00",
+ },
+ {
+ .id = SYS_CLCD_ID_SANYO_2_5,
+ .compatible = "sanyo,alr252rgt",
+ .ib2 = true,
+ },
+};
+
+static void versatile_panel_probe(struct device *dev,
+ struct device_node *endpoint)
+{
+ struct versatile_panel const *vpanel = NULL;
+ struct device_node *panel = NULL;
+ u32 val;
+ int ret;
+ int i;
+
+ /*
+ * The Versatile CLCD has a panel auto-detection mechanism.
+ * We use this and look for the compatible panel in the
+ * device tree.
+ */
+ ret = regmap_read(versatile_syscon_map, SYS_CLCD, &val);
+ if (ret) {
+ dev_err(dev, "cannot read CLCD syscon register\n");
+ return;
+ }
+ val &= SYS_CLCD_CLCDID_MASK;
+
+ /* First find corresponding panel information */
+ for (i = 0; i < ARRAY_SIZE(versatile_panels); i++) {
+ vpanel = &versatile_panels[i];
+
+ if (val = vpanel->id) {
+ dev_err(dev, "autodetected panel \"%s\"\n",
+ vpanel->compatible);
+ break;
+ }
+ }
+ if (i = ARRAY_SIZE(versatile_panels)) {
+ dev_err(dev, "could not auto-detect panel\n");
+ return;
+ }
+
+ panel = of_graph_get_remote_port_parent(endpoint);
+ if (!panel) {
+ dev_err(dev, "could not locate panel in DT\n");
+ return;
+ }
+ if (!of_device_is_compatible(panel, vpanel->compatible))
+ dev_err(dev, "panel in DT is not compatible with the "
+ "auto-detected panel, continuing anyway\n");
+
+ /*
+ * If we have a Sanyo 2.5" port
+ * that we're running on an IB2 and proceed to look for the
+ * IB2 syscon regmap.
+ */
+ if (!vpanel->ib2)
+ return;
+
+ versatile_ib2_map = syscon_regmap_lookup_by_compatible(
+ "arm,versatile-ib2-syscon");
+ if (IS_ERR(versatile_ib2_map)) {
+ dev_err(dev, "could not locate IB2 control register\n");
+ versatile_ib2_map = NULL;
+ return;
+ }
+}
+
+int versatile_clcd_init_panel(struct clcd_fb *fb,
+ struct device_node *endpoint)
+{
+ const struct of_device_id *clcd_id;
+ enum versatile_clcd versatile_clcd_type;
+ struct device_node *np;
+ struct regmap *map;
+ struct device *dev = &fb->dev->dev;
+
+ np = of_find_matching_node_and_match(NULL, versatile_clcd_of_match,
+ &clcd_id);
+ if (!np) {
+ dev_err(dev, "no Versatile syscon node\n");
+ return -ENODEV;
+ }
+ versatile_clcd_type = (enum versatile_clcd)clcd_id->data;
+
+ map = syscon_node_to_regmap(np);
+ if (!map) {
+ dev_err(dev, "no Versatile syscon regmap\n");
+ return -ENODEV;
+ }
+
+ switch (versatile_clcd_type) {
+ case INTEGRATOR_CLCD_CM:
+ versatile_syscon_map = map;
+ fb->board->enable = integrator_clcd_enable;
+ /* Override the caps, we have only these */
+ fb->board->caps = CLCD_CAP_5551 | CLCD_CAP_RGB565 |
+ CLCD_CAP_888;
+ dev_info(dev, "set up callbacks for Integrator PL110\n");
+ break;
+ case VERSATILE_CLCD:
+ versatile_syscon_map = map;
+ fb->board->enable = versatile_clcd_enable;
+ fb->board->disable = versatile_clcd_disable;
+ fb->board->decode = versatile_clcd_decode;
+ versatile_panel_probe(dev, endpoint);
+ dev_info(dev, "set up callbacks for Versatile\n");
+ break;
+ case REALVIEW_CLCD_EB:
+ case REALVIEW_CLCD_PB1176:
+ case REALVIEW_CLCD_PB11MP:
+ case REALVIEW_CLCD_PBA8:
+ case REALVIEW_CLCD_PBX:
+ versatile_syscon_map = map;
+ fb->board->enable = realview_clcd_enable;
+ fb->board->disable = realview_clcd_disable;
+ dev_info(dev, "set up callbacks for RealView PL111\n");
+ break;
+ default:
+ dev_info(dev, "unknown Versatile system controller\n");
+ break;
+ }
+
+ return 0;
+}
+
+#endif
diff --git a/drivers/video/fbdev/amba-clcd-versatile.h b/drivers/video/fbdev/amba-clcd-versatile.h
new file mode 100644
index 000000000000..1b14359c2cf6
--- /dev/null
+++ b/drivers/video/fbdev/amba-clcd-versatile.h
@@ -0,0 +1,17 @@
+/*
+ * Special local versatile callbacks
+ */
+#include <linux/of.h>
+#include <linux/amba/bus.h>
+#include <linux/platform_data/video-clcd-versatile.h>
+
+#if defined(CONFIG_PLAT_VERSATILE_CLCD) && defined(CONFIG_OF)
+int versatile_clcd_init_panel(struct clcd_fb *fb,
+ struct device_node *endpoint);
+#else
+static inline int versatile_clcd_init_panel(struct clcd_fb *fb,
+ struct device_node *endpoint)
+{
+ return 0;
+}
+#endif
diff --git a/drivers/video/fbdev/amba-clcd.c b/drivers/video/fbdev/amba-clcd.c
index 2541b237e46e..9a0e7ebeaa33 100644
--- a/drivers/video/fbdev/amba-clcd.c
+++ b/drivers/video/fbdev/amba-clcd.c
@@ -38,6 +38,7 @@
#include <asm/sizes.h>
#include "amba-clcd-nomadik.h"
+#include "amba-clcd-versatile.h"
#define to_clcd(info) container_of(info, struct clcd_fb, fb)
@@ -1042,7 +1043,8 @@ static int clcdfb_remove(struct amba_device *dev)
}
static struct clcd_vendor_data vendor_arm = {
- /* No special business */
+ /* Sets up the versatile board displays */
+ .init_panel = versatile_clcd_init_panel,
};
static struct clcd_vendor_data vendor_nomadik = {
--
2.4.3
^ permalink raw reply related
* [PATCH 6/7 v2] video: ARM CLCD: add special board and panel hooks for Nomadik
From: Linus Walleij @ 2016-02-23 10:01 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1456221704-5792-1-git-send-email-linus.walleij@linaro.org>
In the .board_init() callback will set up a mux register in
the Nomadik system controller. It so happens that the platform
has two display output engines, and we have to poke a bit in
a special register to make sure the right engine is muxed in
as they are mutually exclusive.
The Nomadik CLCD variant is instantiated on a platform where
it is combined with a 800x480 TPO WVGA display. In the
.panel_init() hook we will detect this display from the
compatible string and set it up. We also add .enable() and
.disable() callbacks for it as the sleep state is software
controlled.
The display is connected with a special 3-wire serial bus
(this is sadly neither I2C or SPI) using three GPIO lines that
we bitbang to detect the display and enable/disable sleep
state.
Cc: Pawel Moll <pawel.moll@arm.com>
Cc: Rob Herring <robh@kernel.org>
Cc: Russell King <linux@arm.linux.org.uk>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
ChangeLog v1->v2:
- No changes. Just reposting.
---
drivers/video/fbdev/Makefile | 1 +
drivers/video/fbdev/amba-clcd-nomadik.c | 257 ++++++++++++++++++++++++++++++++
drivers/video/fbdev/amba-clcd-nomadik.h | 24 +++
drivers/video/fbdev/amba-clcd.c | 6 +-
4 files changed, 287 insertions(+), 1 deletion(-)
create mode 100644 drivers/video/fbdev/amba-clcd-nomadik.c
create mode 100644 drivers/video/fbdev/amba-clcd-nomadik.h
diff --git a/drivers/video/fbdev/Makefile b/drivers/video/fbdev/Makefile
index 65fb15075c8f..f0aec3aa213c 100644
--- a/drivers/video/fbdev/Makefile
+++ b/drivers/video/fbdev/Makefile
@@ -79,6 +79,7 @@ obj-$(CONFIG_FB_ATMEL) += atmel_lcdfb.o
obj-$(CONFIG_FB_PVR2) += pvr2fb.o
obj-$(CONFIG_FB_VOODOO1) += sstfb.o
obj-$(CONFIG_FB_ARMCLCD) += amba-clcd.o
+obj-$(CONFIG_ARCH_NOMADIK) += amba-clcd-nomadik.o
obj-$(CONFIG_PLAT_VERSATILE_CLCD) += amba-clcd-versatile.o
obj-$(CONFIG_FB_GOLDFISH) += goldfishfb.o
obj-$(CONFIG_FB_68328) += 68328fb.o
diff --git a/drivers/video/fbdev/amba-clcd-nomadik.c b/drivers/video/fbdev/amba-clcd-nomadik.c
new file mode 100644
index 000000000000..243167c8ee15
--- /dev/null
+++ b/drivers/video/fbdev/amba-clcd-nomadik.c
@@ -0,0 +1,257 @@
+#include <linux/amba/bus.h>
+#include <linux/amba/clcd.h>
+#include <linux/gpio/consumer.h>
+#include <linux/of.h>
+#include <linux/of_graph.h>
+#include <linux/delay.h>
+#include <linux/bitops.h>
+#include <linux/mfd/syscon.h>
+#include <linux/regmap.h>
+
+#include "amba-clcd-nomadik.h"
+
+static struct gpio_desc *grestb;
+static struct gpio_desc *scen;
+static struct gpio_desc *scl;
+static struct gpio_desc *sda;
+
+static u8 tpg110_readwrite_reg(bool write, u8 address, u8 outval)
+{
+ int i;
+ u8 inval = 0;
+
+ /* Assert SCEN */
+ gpiod_set_value_cansleep(scen, 1);
+ ndelay(150);
+ /* Hammer out the address */
+ for (i = 5; i >= 0; i--) {
+ if (address & BIT(i))
+ gpiod_set_value_cansleep(sda, 1);
+ else
+ gpiod_set_value_cansleep(sda, 0);
+ ndelay(150);
+ /* Send an SCL pulse */
+ gpiod_set_value_cansleep(scl, 1);
+ ndelay(160);
+ gpiod_set_value_cansleep(scl, 0);
+ ndelay(160);
+ }
+
+ if (write) {
+ /* WRITE */
+ gpiod_set_value_cansleep(sda, 0);
+ } else {
+ /* READ */
+ gpiod_set_value_cansleep(sda, 1);
+ }
+ ndelay(150);
+ /* Send an SCL pulse */
+ gpiod_set_value_cansleep(scl, 1);
+ ndelay(160);
+ gpiod_set_value_cansleep(scl, 0);
+ ndelay(160);
+
+ if (!write)
+ /* HiZ turn-around cycle */
+ gpiod_direction_input(sda);
+ ndelay(150);
+ /* Send an SCL pulse */
+ gpiod_set_value_cansleep(scl, 1);
+ ndelay(160);
+ gpiod_set_value_cansleep(scl, 0);
+ ndelay(160);
+
+ /* Hammer in/out the data */
+ for (i = 7; i >= 0; i--) {
+ int value;
+
+ if (write) {
+ value = !!(outval & BIT(i));
+ gpiod_set_value_cansleep(sda, value);
+ } else {
+ value = gpiod_get_value(sda);
+ if (value)
+ inval |= BIT(i);
+ }
+ ndelay(150);
+ /* Send an SCL pulse */
+ gpiod_set_value_cansleep(scl, 1);
+ ndelay(160);
+ gpiod_set_value_cansleep(scl, 0);
+ ndelay(160);
+ }
+
+ gpiod_direction_output(sda, 0);
+ /* Deassert SCEN */
+ gpiod_set_value_cansleep(scen, 0);
+ /* Satisfies SCEN pulse width */
+ udelay(1);
+
+ return inval;
+}
+
+static u8 tpg110_read_reg(u8 address)
+{
+ return tpg110_readwrite_reg(false, address, 0);
+}
+
+static void tpg110_write_reg(u8 address, u8 outval)
+{
+ tpg110_readwrite_reg(true, address, outval);
+}
+
+static void tpg110_startup(struct device *dev)
+{
+ u8 val;
+
+ dev_info(dev, "TPG110 display enable\n");
+ /* De-assert the reset signal */
+ gpiod_set_value_cansleep(grestb, 0);
+ mdelay(1);
+ dev_info(dev, "de-asserted GRESTB\n");
+
+ /* Test display communication */
+ tpg110_write_reg(0x00, 0x55);
+ val = tpg110_read_reg(0x00);
+ if (val = 0x55)
+ dev_info(dev, "passed communication test\n");
+ val = tpg110_read_reg(0x01);
+ dev_info(dev, "TPG110 chip ID: %d version: %d\n",
+ val>>4, val&0x0f);
+
+ /* Show display resolution */
+ val = tpg110_read_reg(0x02);
+ val &= 7;
+ switch (val) {
+ case 0x0:
+ dev_info(dev, "IN 400x240 RGB -> OUT 800x480 RGB (dual scan)");
+ break;
+ case 0x1:
+ dev_info(dev, "IN 480x272 RGB -> OUT 800x480 RGB (dual scan)");
+ break;
+ case 0x4:
+ dev_info(dev, "480x640 RGB");
+ break;
+ case 0x5:
+ dev_info(dev, "480x272 RGB");
+ break;
+ case 0x6:
+ dev_info(dev, "640x480 RGB");
+ break;
+ case 0x7:
+ dev_info(dev, "800x480 RGB");
+ break;
+ default:
+ dev_info(dev, "ILLEGAL RESOLUTION");
+ break;
+ }
+
+ val = tpg110_read_reg(0x03);
+ dev_info(dev, "resolution is controlled by %s\n",
+ (val & BIT(7)) ? "software" : "hardware");
+}
+
+static void tpg110_enable(struct clcd_fb *fb)
+{
+ struct device *dev = &fb->dev->dev;
+ static bool startup;
+ u8 val;
+
+ if (!startup) {
+ tpg110_startup(dev);
+ startup = true;
+ }
+
+ /* Take chip out of standby */
+ val = tpg110_read_reg(0x03);
+ val |= BIT(0);
+ tpg110_write_reg(0x03, val);
+}
+
+static void tpg110_disable(struct clcd_fb *fb)
+{
+ u8 val;
+
+ dev_info(&fb->dev->dev, "TPG110 display disable\n");
+ val = tpg110_read_reg(0x03);
+ /* Put into standby */
+ val &= ~BIT(0);
+ tpg110_write_reg(0x03, val);
+}
+
+static void tpg110_init(struct device *dev, struct device_node *np,
+ struct clcd_board *board)
+{
+ dev_info(dev, "TPG110 display init\n");
+
+ grestb = devm_get_gpiod_from_child(dev, "grestb", &np->fwnode);
+ if (IS_ERR(grestb)) {
+ dev_err(dev, "no GRESTB GPIO\n");
+ return;
+ }
+ /* This asserts the GRESTB signal, putting the display into reset */
+ gpiod_direction_output(grestb, 1);
+
+ scen = devm_get_gpiod_from_child(dev, "scen", &np->fwnode);
+ if (IS_ERR(scen)) {
+ dev_err(dev, "no SCEN GPIO\n");
+ return;
+ }
+ gpiod_direction_output(scen, 0);
+ scl = devm_get_gpiod_from_child(dev, "scl", &np->fwnode);
+ if (IS_ERR(scl)) {
+ dev_err(dev, "no SCL GPIO\n");
+ return;
+ }
+ gpiod_direction_output(scl, 0);
+ sda = devm_get_gpiod_from_child(dev, "sda", &np->fwnode);
+ if (IS_ERR(sda)) {
+ dev_err(dev, "no SDA GPIO\n");
+ return;
+ }
+ gpiod_direction_output(sda, 0);
+ board->enable = tpg110_enable;
+ board->disable = tpg110_disable;
+}
+
+int nomadik_clcd_init_panel(struct clcd_fb *fb,
+ struct device_node *endpoint)
+{
+ struct device_node *panel;
+
+ panel = of_graph_get_remote_port_parent(endpoint);
+ if (!panel)
+ return -ENODEV;
+
+ if (of_device_is_compatible(panel, "tpo,tpg110"))
+ tpg110_init(&fb->dev->dev, panel, fb->board);
+ else
+ dev_info(&fb->dev->dev, "unknown panel\n");
+
+ /* Unknown panel, fall through */
+ return 0;
+}
+
+#define PMU_CTRL_OFFSET 0x0000
+#define PMU_CTRL_LCDNDIF BIT(26)
+
+int nomadik_clcd_init_board(struct amba_device *adev,
+ struct clcd_board *board)
+{
+ struct regmap *pmu_regmap;
+
+ dev_info(&adev->dev, "Nomadik CLCD board init\n");
+ pmu_regmap + syscon_regmap_lookup_by_compatible("stericsson,nomadik-pmu");
+ if (IS_ERR(pmu_regmap)) {
+ dev_err(&adev->dev, "could not find PMU syscon regmap\n");
+ return PTR_ERR(pmu_regmap);
+ }
+ regmap_update_bits(pmu_regmap,
+ PMU_CTRL_OFFSET,
+ PMU_CTRL_LCDNDIF,
+ 0);
+ dev_info(&adev->dev, "set PMU mux to CLCD mode\n");
+
+ return 0;
+}
diff --git a/drivers/video/fbdev/amba-clcd-nomadik.h b/drivers/video/fbdev/amba-clcd-nomadik.h
new file mode 100644
index 000000000000..50aa9bda69fd
--- /dev/null
+++ b/drivers/video/fbdev/amba-clcd-nomadik.h
@@ -0,0 +1,24 @@
+#ifndef _AMBA_CLCD_NOMADIK_H
+#define _AMBA_CLCD_NOMADIK_H
+
+#include <linux/amba/bus.h>
+
+#ifdef CONFIG_ARCH_NOMADIK
+int nomadik_clcd_init_board(struct amba_device *adev,
+ struct clcd_board *board);
+int nomadik_clcd_init_panel(struct clcd_fb *fb,
+ struct device_node *endpoint);
+#else
+static inline int nomadik_clcd_init_board(struct amba_device *adev,
+ struct clcd_board *board)
+{
+ return 0;
+}
+static inline int nomadik_clcd_init_panel(struct clcd_fb *fb,
+ struct device_node *endpoint)
+{
+ return 0;
+}
+#endif
+
+#endif /* inclusion guard */
diff --git a/drivers/video/fbdev/amba-clcd.c b/drivers/video/fbdev/amba-clcd.c
index 24f4b596d502..2541b237e46e 100644
--- a/drivers/video/fbdev/amba-clcd.c
+++ b/drivers/video/fbdev/amba-clcd.c
@@ -37,6 +37,8 @@
#include <asm/sizes.h>
+#include "amba-clcd-nomadik.h"
+
#define to_clcd(info) container_of(info, struct clcd_fb, fb)
/* This is limited to 16 characters when displayed by X startup */
@@ -1003,7 +1005,7 @@ static int clcdfb_probe(struct amba_device *dev, const struct amba_id *id)
if (ret)
goto free_fb;
- ret = clcdfb_register(fb);
+ ret = clcdfb_register(fb);
if (ret = 0) {
amba_set_drvdata(dev, fb);
goto out;
@@ -1047,6 +1049,8 @@ static struct clcd_vendor_data vendor_nomadik = {
.clock_timregs = true,
.packed_24_bit_pixels = true,
.st_bitmux_control = true,
+ .init_board = nomadik_clcd_init_board,
+ .init_panel = nomadik_clcd_init_panel,
};
static struct amba_id clcdfb_id_table[] = {
--
2.4.3
^ permalink raw reply related
* [PATCH 5/7 v2] video: ARM CLCD: support Nomadik variant
From: Linus Walleij @ 2016-02-23 10:01 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1456221704-5792-1-git-send-email-linus.walleij@linaro.org>
The Nomadik variant has a few special quirks that need to be respected
to make the driver work:
- The block need to be clocked during writing of the TIMn registers
or the bus will stall.
- Special bits in the control register select how many of the output
display lines get activated.
- Special bits in the control register select how to manage the
different 565 and 5551 modes.
- There is a packed 24bit graphics mode, i.e 888 pixels can be stored
in memory is three consecutive bytes, not evenly aligned to a 32bit
word.
This patch uses the vendor data pointer from the AMBA matching mechanism
to track the quirks for this variant, and adds two hooks that variants
can use to initialize boards and panels during start-up. These will
later be used to adopt a Nomadik board profile.
Cc: Pawel Moll <pawel.moll@arm.com>
Cc: Rob Herring <robh@kernel.org>
Cc: Russell King <linux@arm.linux.org.uk>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
ChangeLog v1->v2:
- No changes. Just reposting.
---
drivers/video/fbdev/amba-clcd.c | 106 ++++++++++++++++++++++++++++++++++++----
include/linux/amba/clcd.h | 42 ++++++++++++++++
2 files changed, 139 insertions(+), 9 deletions(-)
diff --git a/drivers/video/fbdev/amba-clcd.c b/drivers/video/fbdev/amba-clcd.c
index 9a631a8e2c04..24f4b596d502 100644
--- a/drivers/video/fbdev/amba-clcd.c
+++ b/drivers/video/fbdev/amba-clcd.c
@@ -227,6 +227,15 @@ clcdfb_set_bitfields(struct clcd_fb *fb, struct fb_var_screeninfo *var)
var->blue.length = 4;
}
break;
+ case 24:
+ if (fb->vendor->packed_24_bit_pixels) {
+ var->red.length = 8;
+ var->green.length = 8;
+ var->blue.length = 8;
+ } else {
+ ret = -EINVAL;
+ }
+ break;
case 32:
/* If we can't do 888, reject */
caps &= CLCD_CAP_888;
@@ -313,6 +322,12 @@ static int clcdfb_set_par(struct fb_info *info)
clcdfb_disable(fb);
+ /* Some variants must be clocked here */
+ if (fb->vendor->clock_timregs && !fb->clk_enabled) {
+ fb->clk_enabled = true;
+ clk_enable(fb->clk);
+ }
+
writel(regs.tim0, fb->regs + CLCD_TIM0);
writel(regs.tim1, fb->regs + CLCD_TIM1);
writel(regs.tim2, fb->regs + CLCD_TIM2);
@@ -716,6 +731,42 @@ static int clcdfb_of_init_tft_panel(struct clcd_fb *fb, u32 r0, u32 g0, u32 b0)
if (r0 != 0 && b0 = 0)
fb->panel->bgr_connection = true;
+ if (fb->panel->caps && fb->vendor->st_bitmux_control) {
+ /*
+ * Set up the special bits for the Nomadik control register
+ * (other platforms tend to do this through an external
+ * register).
+ */
+
+ /* Offset of the highest used color */
+ int maxoff = max3(r0, g0, b0);
+ /* Most significant bit out, highest used bit */
+ int msb = 0;
+
+ if (fb->panel->caps & CLCD_CAP_888) {
+ msb = maxoff + 8 - 1;
+ } else if (fb->panel->caps & CLCD_CAP_565) {
+ msb = maxoff + 5 - 1;
+ fb->panel->cntl |= CNTL_ST_1XBPP_565;
+ } else if (fb->panel->caps & CLCD_CAP_5551) {
+ msb = maxoff + 5 - 1;
+ fb->panel->cntl |= CNTL_ST_1XBPP_5551;
+ } else if (fb->panel->caps & CLCD_CAP_444) {
+ msb = maxoff + 4 - 1;
+ fb->panel->cntl |= CNTL_ST_1XBPP_444;
+ }
+
+ /* Send out as many bits as we need */
+ if (msb > 17)
+ fb->panel->cntl |= CNTL_ST_CDWID_24;
+ else if (msb > 15)
+ fb->panel->cntl |= CNTL_ST_CDWID_18;
+ else if (msb > 11)
+ fb->panel->cntl |= CNTL_ST_CDWID_16;
+ else
+ fb->panel->cntl |= CNTL_ST_CDWID_12;
+ }
+
return fb->panel->caps ? 0 : -EINVAL;
}
@@ -731,9 +782,21 @@ static int clcdfb_of_init_display(struct clcd_fb *fb)
if (!fb->panel)
return -ENOMEM;
- endpoint = of_graph_get_next_endpoint(fb->dev->dev.of_node, NULL);
- if (!endpoint)
- return -ENODEV;
+ /*
+ * Fetch the panel endpoint.
+ */
+ if (!endpoint) {
+ endpoint = of_graph_get_next_endpoint(fb->dev->dev.of_node,
+ NULL);
+ if (!endpoint)
+ return -ENODEV;
+ }
+
+ if (fb->vendor->init_panel) {
+ err = fb->vendor->init_panel(fb, endpoint);
+ if (err)
+ return err;
+ }
err = clcdfb_of_get_backlight(endpoint, fb->panel);
if (err)
@@ -770,11 +833,11 @@ static int clcdfb_of_init_display(struct clcd_fb *fb)
if (of_property_read_u32_array(endpoint,
"arm,pl11x,tft-r0g0b0-pads",
- tft_r0b0g0, ARRAY_SIZE(tft_r0b0g0)) = 0)
- return clcdfb_of_init_tft_panel(fb, tft_r0b0g0[0],
- tft_r0b0g0[1], tft_r0b0g0[2]);
+ tft_r0b0g0, ARRAY_SIZE(tft_r0b0g0)) != 0)
+ return -ENOENT;
- return -ENOENT;
+ return clcdfb_of_init_tft_panel(fb, tft_r0b0g0[0],
+ tft_r0b0g0[1], tft_r0b0g0[2]);
}
static int clcdfb_of_vram_setup(struct clcd_fb *fb)
@@ -895,6 +958,7 @@ static struct clcd_board *clcdfb_of_get_board(struct amba_device *dev)
static int clcdfb_probe(struct amba_device *dev, const struct amba_id *id)
{
struct clcd_board *board = dev_get_platdata(&dev->dev);
+ struct clcd_vendor_data *vendor = id->data;
struct clcd_fb *fb;
int ret;
@@ -904,6 +968,12 @@ static int clcdfb_probe(struct amba_device *dev, const struct amba_id *id)
if (!board)
return -EINVAL;
+ if (vendor->init_board) {
+ ret = vendor->init_board(dev, board);
+ if (ret)
+ return ret;
+ }
+
ret = dma_set_mask_and_coherent(&dev->dev, DMA_BIT_MASK(32));
if (ret)
goto out;
@@ -922,10 +992,11 @@ static int clcdfb_probe(struct amba_device *dev, const struct amba_id *id)
}
fb->dev = dev;
+ fb->vendor = vendor;
fb->board = board;
- dev_info(&fb->dev->dev, "PL%03x rev%u at 0x%08llx\n",
- amba_part(dev), amba_rev(dev),
+ dev_info(&fb->dev->dev, "PL%03x designer %02x rev%u at 0x%08llx\n",
+ amba_part(dev), amba_manf(dev), amba_rev(dev),
(unsigned long long)dev->res.start);
ret = fb->board->setup(fb);
@@ -968,10 +1039,27 @@ static int clcdfb_remove(struct amba_device *dev)
return 0;
}
+static struct clcd_vendor_data vendor_arm = {
+ /* No special business */
+};
+
+static struct clcd_vendor_data vendor_nomadik = {
+ .clock_timregs = true,
+ .packed_24_bit_pixels = true,
+ .st_bitmux_control = true,
+};
+
static struct amba_id clcdfb_id_table[] = {
{
.id = 0x00041110,
.mask = 0x000ffffe,
+ .data = &vendor_arm,
+ },
+ /* ST Electronics Nomadik variant */
+ {
+ .id = 0x00180110,
+ .mask = 0x00fffffe,
+ .data = &vendor_nomadik,
},
{ 0, 0 },
};
diff --git a/include/linux/amba/clcd.h b/include/linux/amba/clcd.h
index c60f32e23a83..7d95633250a4 100644
--- a/include/linux/amba/clcd.h
+++ b/include/linux/amba/clcd.h
@@ -67,6 +67,17 @@
#define CNTL_LDMAFIFOTIME (1 << 15)
#define CNTL_WATERMARK (1 << 16)
+/* ST Microelectronics variant bits */
+#define CNTL_ST_1XBPP_444 0x0
+#define CNTL_ST_1XBPP_5551 (1 << 17)
+#define CNTL_ST_1XBPP_565 (1 << 18)
+#define CNTL_ST_CDWID_12 0x0
+#define CNTL_ST_CDWID_16 (1 << 19)
+#define CNTL_ST_CDWID_18 (1 << 20)
+#define CNTL_ST_CDWID_24 ((1 << 19)|(1 << 20))
+#define CNTL_ST_CEAEN (1 << 21)
+#define CNTL_ST_LCDBPP24_PACKED (6 << 1)
+
enum {
/* individual formats */
CLCD_CAP_RGB444 = (1 << 0),
@@ -179,11 +190,38 @@ struct clcd_board {
struct amba_device;
struct clk;
+/**
+ * struct clcd_vendor_data - holds hardware (IP-block) vendor-specific
+ * variant information
+ *
+ * @clock_timregs: the CLCD needs to be clocked when accessing the
+ * timer registers, or the hardware will hang.
+ * @packed_24_bit_pixels: this variant supports 24bit packed pixel data,
+ * so that RGB accesses 3 bytes at a time, not just on even 32bit
+ * boundaries, packing the pixel data in memory. ST Microelectronics
+ * have this.
+ * @st_bitmux_control: ST Microelectronics have implemented output
+ * bit line multiplexing into the CLCD control register. This indicates
+ * that we need to use this.
+ * @init_board: custom board init function for this variant
+ * @init_panel: custom panel init function for this variant
+ */
+struct clcd_vendor_data {
+ bool clock_timregs;
+ bool packed_24_bit_pixels;
+ bool st_bitmux_control;
+ int (*init_board)(struct amba_device *adev,
+ struct clcd_board *board);
+ int (*init_panel)(struct clcd_fb *fb,
+ struct device_node *panel);
+};
+
/* this data structure describes each frame buffer device we find */
struct clcd_fb {
struct fb_info fb;
struct amba_device *dev;
struct clk *clk;
+ struct clcd_vendor_data *vendor;
struct clcd_panel *panel;
struct clcd_board *board;
void *board_data;
@@ -290,6 +328,10 @@ static inline void clcdfb_decode(struct clcd_fb *fb, struct clcd_regs *regs)
else
val |= CNTL_LCDBPP16_444;
break;
+ case 24:
+ /* Modified variant supporting 24 bit packed pixels */
+ val |= CNTL_ST_LCDBPP24_PACKED;
+ break;
case 32:
val |= CNTL_LCDBPP24;
break;
--
2.4.3
^ permalink raw reply related
* [PATCH 4/7 v2] video: ARM CLCD: support pads connected in reverse order
From: Linus Walleij @ 2016-02-23 10:01 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1456221704-5792-1-git-send-email-linus.walleij@linaro.org>
There are CLCDs connected with the pads in BGR rather than RGB
order. It really doesn't matter since the CLCD has a flag and
a bit to switch the position of the RGB and BGR components.
This is needed to put something logical into the
arm,pl11x,tft-r0g0b0-pads property of the device tree on the
Nomadik which will then be <16 8 0>.
Cc: Pawel Moll <pawel.moll@arm.com>
Cc: Rob Herring <robh@kernel.org>
Cc: Russell King <linux@arm.linux.org.uk>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
ChangeLog v1->v2:
- No changes. Just reposting.
---
drivers/video/fbdev/amba-clcd.c | 8 ++++++++
include/linux/amba/clcd.h | 31 ++++++++++++++++++++++++-------
2 files changed, 32 insertions(+), 7 deletions(-)
diff --git a/drivers/video/fbdev/amba-clcd.c b/drivers/video/fbdev/amba-clcd.c
index 8903a42c4122..9a631a8e2c04 100644
--- a/drivers/video/fbdev/amba-clcd.c
+++ b/drivers/video/fbdev/amba-clcd.c
@@ -681,6 +681,7 @@ static int clcdfb_of_init_tft_panel(struct clcd_fb *fb, u32 r0, u32 g0, u32 b0)
} panels[] = {
{ 0x110, 1, 7, 13, CLCD_CAP_5551 },
{ 0x110, 0, 8, 16, CLCD_CAP_888 },
+ { 0x110, 16, 8, 0, CLCD_CAP_888 },
{ 0x111, 4, 14, 20, CLCD_CAP_444 },
{ 0x111, 3, 11, 19, CLCD_CAP_444 | CLCD_CAP_5551 },
{ 0x111, 3, 10, 19, CLCD_CAP_444 | CLCD_CAP_5551 |
@@ -708,6 +709,13 @@ static int clcdfb_of_init_tft_panel(struct clcd_fb *fb, u32 r0, u32 g0, u32 b0)
fb->panel->caps = panels[i].caps;
}
+ /*
+ * If we actually physically connected the R lines to B and
+ * vice versa
+ */
+ if (r0 != 0 && b0 = 0)
+ fb->panel->bgr_connection = true;
+
return fb->panel->caps ? 0 : -EINVAL;
}
diff --git a/include/linux/amba/clcd.h b/include/linux/amba/clcd.h
index e64c1ccebb76..c60f32e23a83 100644
--- a/include/linux/amba/clcd.h
+++ b/include/linux/amba/clcd.h
@@ -108,6 +108,12 @@ struct clcd_panel {
grayscale:1;
unsigned int connector;
struct backlight_device *backlight;
+ /*
+ * If the B/R lines are switched between the CLCD
+ * and the panel we need to know this and not try to
+ * compensate with the BGR bit in the control register.
+ */
+ bool bgr_connection;
};
struct clcd_regs {
@@ -234,16 +240,27 @@ static inline void clcdfb_decode(struct clcd_fb *fb, struct clcd_regs *regs)
if (var->grayscale)
val |= CNTL_LCDBW;
- if (fb->panel->caps && fb->board->caps &&
- var->bits_per_pixel >= 16) {
+ if (fb->panel->caps && fb->board->caps && var->bits_per_pixel >= 16) {
/*
* if board and panel supply capabilities, we can support
- * changing BGR/RGB depending on supplied parameters
+ * changing BGR/RGB depending on supplied parameters. Here
+ * we switch to what the framebuffer is providing if need
+ * be, so if the framebuffer is BGR but the display connection
+ * is RGB (first case) we switch it around. Vice versa mutatis
+ * mutandis if the framebuffer is RGB but the display connection
+ * is BGR, we flip it around.
*/
- if (var->red.offset = 0)
- val &= ~CNTL_BGR;
- else
- val |= CNTL_BGR;
+ if (!fb->panel->bgr_connection) {
+ if (var->red.offset = 0)
+ val &= ~CNTL_BGR;
+ else
+ val |= CNTL_BGR;
+ } else {
+ if (var->blue.offset = 0)
+ val &= ~CNTL_BGR;
+ else
+ val |= CNTL_BGR;
+ }
}
switch (var->bits_per_pixel) {
--
2.4.3
^ permalink raw reply related
* [PATCH 3/7 v2] video: ARM CLCD: support DT signal inversion flags
From: Linus Walleij @ 2016-02-23 10:01 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1456221704-5792-1-git-send-email-linus.walleij@linaro.org>
The device tree bindings from display-timing.txt allows us to
specify if data enable, hsync, vsync or the pixed clock should be
inverted on the way to the display. The driver does not currently
handle this so add support for those flags as it is needed for
the Versatile Sanyo LCD display.
Note that the previous behaviour was to invert the pixel clock
for all displays, so unless the pixel clock polarity is
explicitly defined in the device tree (i.e. the timings node
has the "pixelclk-active" property) we fall back to inverting
the pixel clock. This needs some extra compatibility code.
Since the timing flags have to be set up inside the struct
clcd_panel, we need to refactor the code a bit to pass around
the panel rather than just the mode.
Cc: Pawel Moll <pawel.moll@arm.com>
Cc: Rob Herring <robh@kernel.org>
Cc: Russell King <linux@arm.linux.org.uk>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
ChangeLog v1->v2:
- No changes. Just reposting.
---
drivers/video/fbdev/amba-clcd.c | 41 ++++++++++++++++++++++++++++++++++-------
1 file changed, 34 insertions(+), 7 deletions(-)
diff --git a/drivers/video/fbdev/amba-clcd.c b/drivers/video/fbdev/amba-clcd.c
index c5d1e9ca81ab..8903a42c4122 100644
--- a/drivers/video/fbdev/amba-clcd.c
+++ b/drivers/video/fbdev/amba-clcd.c
@@ -567,10 +567,11 @@ static int clcdfb_register(struct clcd_fb *fb)
#ifdef CONFIG_OF
static int clcdfb_of_get_dpi_panel_mode(struct device_node *node,
- struct fb_videomode *mode)
+ struct clcd_panel *clcd_panel)
{
int err;
struct display_timing timing;
+ struct device_node *timnp;
struct videomode video;
err = of_get_display_timing(node, "panel-timing", &timing);
@@ -579,10 +580,34 @@ static int clcdfb_of_get_dpi_panel_mode(struct device_node *node,
videomode_from_timing(&timing, &video);
- err = fb_videomode_from_videomode(&video, mode);
+ err = fb_videomode_from_videomode(&video, &clcd_panel->mode);
if (err)
return err;
+ /* Set up some inversion flags */
+ timnp = of_get_child_by_name(node, "panel-timing");
+ if (timnp && of_property_read_bool(timnp, "pixelclk-active")) {
+ if (timing.flags & DISPLAY_FLAGS_PIXDATA_NEGEDGE)
+ clcd_panel->tim2 |= TIM2_IPC;
+ } else {
+ /*
+ * To preserve backwards compatibility, the IPC (inverted
+ * pixel clock) flag needs to be set on any display that
+ * doesn't explicitly specify that the pixel clock is
+ * active on the negative edge.
+ */
+ clcd_panel->tim2 |= TIM2_IPC;
+ }
+
+ if (timing.flags & DISPLAY_FLAGS_HSYNC_LOW)
+ clcd_panel->tim2 |= TIM2_IHS;
+
+ if (timing.flags & DISPLAY_FLAGS_VSYNC_LOW)
+ clcd_panel->tim2 |= TIM2_IVS;
+
+ if (timing.flags & DISPLAY_FLAGS_DE_LOW)
+ clcd_panel->tim2 |= TIM2_IOE;
+
return 0;
}
@@ -615,10 +640,11 @@ static int clcdfb_of_get_backlight(struct device_node *endpoint,
}
static int clcdfb_of_get_mode(struct device *dev, struct device_node *endpoint,
- struct fb_videomode *mode)
+ struct clcd_panel *clcd_panel)
{
int err;
struct device_node *panel;
+ struct fb_videomode *mode;
char *name;
int len;
@@ -628,11 +654,12 @@ static int clcdfb_of_get_mode(struct device *dev, struct device_node *endpoint,
/* Only directly connected DPI panels supported for now */
if (of_device_is_compatible(panel, "panel-dpi"))
- err = clcdfb_of_get_dpi_panel_mode(panel, mode);
+ err = clcdfb_of_get_dpi_panel_mode(panel, clcd_panel);
else
err = -ENOENT;
if (err)
return err;
+ mode = &clcd_panel->mode;
len = clcdfb_snprintf_mode(NULL, 0, mode);
name = devm_kzalloc(dev, len + 1, GFP_KERNEL);
@@ -663,8 +690,8 @@ static int clcdfb_of_init_tft_panel(struct clcd_fb *fb, u32 r0, u32 g0, u32 b0)
};
int i;
- /* Bypass pixel clock divider, data output on the falling edge */
- fb->panel->tim2 = TIM2_BCD | TIM2_IPC;
+ /* Bypass pixel clock divider */
+ fb->panel->tim2 |= TIM2_BCD;
/* TFT display, vert. comp. interrupt at the start of the back porch */
fb->panel->cntl |= CNTL_LCDTFT | CNTL_LCDVCOMP(1);
@@ -704,7 +731,7 @@ static int clcdfb_of_init_display(struct clcd_fb *fb)
if (err)
return err;
- err = clcdfb_of_get_mode(&fb->dev->dev, endpoint, &fb->panel->mode);
+ err = clcdfb_of_get_mode(&fb->dev->dev, endpoint, fb->panel);
if (err)
return err;
--
2.4.3
^ permalink raw reply related
* [PATCH 2/7 v2] video: ARM CLCD: backlight support for OF
From: Linus Walleij @ 2016-02-23 10:01 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1456221704-5792-1-git-send-email-linus.walleij@linaro.org>
If the device is probed from device tree, we can support
backlight. This is used with some systems such as the
ST Microelectronics Nomadik.
We have to add HAS_IOMEM to the dependencies of CLCD since
the backlight class device will now be selected, and if it
gets selected on an arch that does not have IOMEM,
compilation will fail.
Cc: Pawel Moll <pawel.moll@arm.com>
Cc: Rob Herring <robh@kernel.org>
Cc: Russell King <linux@arm.linux.org.uk>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
ChangeLog v1->v2:
- No changes. Just reposting.
---
drivers/video/fbdev/Kconfig | 4 +++-
drivers/video/fbdev/amba-clcd.c | 40 ++++++++++++++++++++++++++++++++++++++++
include/linux/amba/clcd.h | 3 +++
3 files changed, 46 insertions(+), 1 deletion(-)
diff --git a/drivers/video/fbdev/Kconfig b/drivers/video/fbdev/Kconfig
index 8ea45a5cd806..6dcd91237551 100644
--- a/drivers/video/fbdev/Kconfig
+++ b/drivers/video/fbdev/Kconfig
@@ -284,12 +284,14 @@ config FB_PM2_FIFO_DISCONNECT
config FB_ARMCLCD
tristate "ARM PrimeCell PL110 support"
depends on ARM || ARM64 || COMPILE_TEST
- depends on FB && ARM_AMBA
+ depends on FB && ARM_AMBA && HAS_IOMEM
select FB_CFB_FILLRECT
select FB_CFB_COPYAREA
select FB_CFB_IMAGEBLIT
select FB_MODE_HELPERS if OF
select VIDEOMODE_HELPERS if OF
+ select BACKLIGHT_LCD_SUPPORT if OF
+ select BACKLIGHT_CLASS_DEVICE if OF
help
This framebuffer device driver is for the ARM PrimeCell PL110
Colour LCD controller. ARM PrimeCells provide the building
diff --git a/drivers/video/fbdev/amba-clcd.c b/drivers/video/fbdev/amba-clcd.c
index f9ef06d0cd48..c5d1e9ca81ab 100644
--- a/drivers/video/fbdev/amba-clcd.c
+++ b/drivers/video/fbdev/amba-clcd.c
@@ -30,6 +30,7 @@
#include <linux/of.h>
#include <linux/of_address.h>
#include <linux/of_graph.h>
+#include <linux/backlight.h>
#include <video/display_timing.h>
#include <video/of_display_timing.h>
#include <video/videomode.h>
@@ -73,6 +74,11 @@ static void clcdfb_disable(struct clcd_fb *fb)
if (fb->board->disable)
fb->board->disable(fb);
+ if (fb->panel->backlight) {
+ fb->panel->backlight->props.power = FB_BLANK_POWERDOWN;
+ backlight_update_status(fb->panel->backlight);
+ }
+
val = readl(fb->regs + fb->off_cntl);
if (val & CNTL_LCDPWR) {
val &= ~CNTL_LCDPWR;
@@ -119,6 +125,14 @@ static void clcdfb_enable(struct clcd_fb *fb, u32 cntl)
writel(cntl, fb->regs + fb->off_cntl);
/*
+ * Turn on backlight
+ */
+ if (fb->panel->backlight) {
+ fb->panel->backlight->props.power = FB_BLANK_UNBLANK;
+ backlight_update_status(fb->panel->backlight);
+ }
+
+ /*
* finally, enable the interface.
*/
if (fb->board->enable)
@@ -578,6 +592,28 @@ static int clcdfb_snprintf_mode(char *buf, int size, struct fb_videomode *mode)
mode->refresh);
}
+static int clcdfb_of_get_backlight(struct device_node *endpoint,
+ struct clcd_panel *clcd_panel)
+{
+ struct device_node *panel;
+ struct device_node *backlight;
+
+ panel = of_graph_get_remote_port_parent(endpoint);
+ if (!panel)
+ return -ENODEV;
+
+ /* Look up the optional backlight phandle */
+ backlight = of_parse_phandle(panel, "backlight", 0);
+ if (backlight) {
+ clcd_panel->backlight = of_find_backlight_by_node(backlight);
+ of_node_put(backlight);
+
+ if (!clcd_panel->backlight)
+ return -EPROBE_DEFER;
+ }
+ return 0;
+}
+
static int clcdfb_of_get_mode(struct device *dev, struct device_node *endpoint,
struct fb_videomode *mode)
{
@@ -664,6 +700,10 @@ static int clcdfb_of_init_display(struct clcd_fb *fb)
if (!endpoint)
return -ENODEV;
+ err = clcdfb_of_get_backlight(endpoint, fb->panel);
+ if (err)
+ return err;
+
err = clcdfb_of_get_mode(&fb->dev->dev, endpoint, &fb->panel->mode);
if (err)
return err;
diff --git a/include/linux/amba/clcd.h b/include/linux/amba/clcd.h
index e82e3ee2c54a..e64c1ccebb76 100644
--- a/include/linux/amba/clcd.h
+++ b/include/linux/amba/clcd.h
@@ -93,6 +93,8 @@ enum {
CLCD_CAP_ALL = CLCD_CAP_BGR | CLCD_CAP_RGB,
};
+struct backlight_device;
+
struct clcd_panel {
struct fb_videomode mode;
signed short width; /* width in mm */
@@ -105,6 +107,7 @@ struct clcd_panel {
fixedtimings:1,
grayscale:1;
unsigned int connector;
+ struct backlight_device *backlight;
};
struct clcd_regs {
--
2.4.3
^ permalink raw reply related
* [PATCH 1/7 RESEND] video: ARM CLCD: runtime check for Versatile
From: Linus Walleij @ 2016-02-23 10:01 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1456221704-5792-1-git-send-email-linus.walleij@linaro.org>
The current compile-time check for inversed IENB/CNTL does not
work in multiplatform boots: as soon as versatile is included
in the build, the IENB/CNTL is switched and breaks graphics.
Convert this to a runtime switch.
Cc: stable@vger.kernel.org
Cc: Rob Herring <robh@kernel.org>
Cc: Russell King <linux@arm.linux.org.uk>
Fixes: a29da136de34 ("ARM: versatile: convert to multi-platform")
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
drivers/video/fbdev/amba-clcd.c | 15 ++++++++-------
1 file changed, 8 insertions(+), 7 deletions(-)
diff --git a/drivers/video/fbdev/amba-clcd.c b/drivers/video/fbdev/amba-clcd.c
index 9362424c2340..f9ef06d0cd48 100644
--- a/drivers/video/fbdev/amba-clcd.c
+++ b/drivers/video/fbdev/amba-clcd.c
@@ -440,13 +440,14 @@ static int clcdfb_register(struct clcd_fb *fb)
fb->off_ienb = CLCD_PL111_IENB;
fb->off_cntl = CLCD_PL111_CNTL;
} else {
-#ifdef CONFIG_ARCH_VERSATILE
- fb->off_ienb = CLCD_PL111_IENB;
- fb->off_cntl = CLCD_PL111_CNTL;
-#else
- fb->off_ienb = CLCD_PL110_IENB;
- fb->off_cntl = CLCD_PL110_CNTL;
-#endif
+ if (of_machine_is_compatible("arm,versatile-ab") ||
+ of_machine_is_compatible("arm,versatile-pb")) {
+ fb->off_ienb = CLCD_PL111_IENB;
+ fb->off_cntl = CLCD_PL111_CNTL;
+ } else {
+ fb->off_ienb = CLCD_PL110_IENB;
+ fb->off_cntl = CLCD_PL110_CNTL;
+ }
}
fb->clk = clk_get(&fb->dev->dev, NULL);
--
2.4.3
^ permalink raw reply related
* [PATCH 0/7] video: ARM CLCD: non-controversial parts
From: Linus Walleij @ 2016-02-23 10:01 UTC (permalink / raw)
To: linux-arm-kernel
This is a redux version of the previous patch set that strips out
any attempts to auto-switch or rewrite the device tree to match
the CLCD Versatile panel.
Instead, it does detect it and COMPARE it to what was found in
the device tree, prints a warning if those do not match, and
continue anyway.
This patch series brings support for device tree-only boot with
graphics to the Nomadik and all RealView platforms (so we can boot
RealView with only device tree) so it is very useful still even
if the Versatile is not yet fully supported.
Please consider applying this for v4.6 if these parts look
uncontroversial. Or applying partly from low to high patch
numbers so I'm not sitting with too much out-of-tree code.
Also resending a fix which has not yet landed upstream (1/7)
I guess it was lost.
Linus Walleij (7):
video: ARM CLCD: runtime check for Versatile
video: ARM CLCD: backlight support for OF
video: ARM CLCD: support DT signal inversion flags
video: ARM CLCD: support pads connected in reverse order
video: ARM CLCD: support Nomadik variant
video: ARM CLCD: add special board and panel hooks for Nomadik
video: ARM CLCD: add special panel hook for Versatiles
drivers/video/fbdev/Kconfig | 6 +-
drivers/video/fbdev/Makefile | 1 +
drivers/video/fbdev/amba-clcd-nomadik.c | 257 ++++++++++++++++++++
drivers/video/fbdev/amba-clcd-nomadik.h | 24 ++
drivers/video/fbdev/amba-clcd-versatile.c | 391 ++++++++++++++++++++++++++++++
drivers/video/fbdev/amba-clcd-versatile.h | 17 ++
drivers/video/fbdev/amba-clcd.c | 218 +++++++++++++++--
include/linux/amba/clcd.h | 76 +++++-
8 files changed, 958 insertions(+), 32 deletions(-)
create mode 100644 drivers/video/fbdev/amba-clcd-nomadik.c
create mode 100644 drivers/video/fbdev/amba-clcd-nomadik.h
create mode 100644 drivers/video/fbdev/amba-clcd-versatile.h
--
2.4.3
^ permalink raw reply
* Re: [PATCH 11/11] ARM: versatile: move CLCD configuration to device tree
From: Tomi Valkeinen @ 2016-02-23 9:58 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CACRpkdbnLgVsrSAu87wxSwktK3ftJzbQLXjU8y65tq=9Ynxicw@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 2094 bytes --]
(cc'ing a few more people as this is going into generic direction)
On 23/02/16 11:08, Linus Walleij wrote:
> On Thu, Feb 4, 2016 at 3:04 PM, Linus Walleij <linus.walleij@linaro.org> wrote:
>
>> This moves the versatile CLCD configuration to the device tree by:
>
> As this Schrödinger's cat approach seems controversial, as well
> as the alternative to manipulate the DT in memory at run-time,
> I will respin the series without the full Versatile support, adding
> plug-in for the other ARM boards and and half-baked support
> for the Versatile supporting just VGA (like the other reference
> designs from ARM).
>
> The pluggable displays prove yet again to be a problem to the
> world, sigh.
>
> I will think of a better solution, if any, for this for v4.6, but will
> put forward something that handles the Nomadik and all the
> other ARM reference designs for now.
I had a chat with Tom Rini and Pantelis Antoniou yesterday.
Panto is about to send a new series for DT overlay management soon. I
haven't had time to test that yet, but what it would give us is that you
could build DT overlay binaries as "firmware" into the kernel image, and
thus the panel DT data would be there even before rootfs is mounted. The
DT overlays can be loaded from the rootfs or initramfs too, if it's not
critical to get the display up early.
I'm not quite sure how it works if, as in versatile display case, there
are multiple DT overlays of which one has to be enabled. I hope there's
support to choose which one to use via kernel cmdline or similar.
I would personally like it much more if the bootloader would either
compose a final dtb from DT fragments and pass it to the kernel, or
alternatively the bootloader would pass the base dtb image and a bunch
of DT overlays to the kernel, and the kernel would deal with the DT
overlays.
In any case, I think the firmware solution is a good step forward, and
will probably work fine for many users. Then again, I haven't tested it
yet so I don't know the details, and it's not in the mainline.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH 11/11] ARM: versatile: move CLCD configuration to device tree
From: Arnd Bergmann @ 2016-02-23 9:34 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CACRpkdbnLgVsrSAu87wxSwktK3ftJzbQLXjU8y65tq=9Ynxicw@mail.gmail.com>
On Tuesday 23 February 2016 10:08:05 Linus Walleij wrote:
> I will think of a better solution, if any, for this for v4.6, but will
> put forward something that handles the Nomadik and all the
> other ARM reference designs for now.
>
How about still describing all known panels in the DT marked 'status="disabled"',
but then having a minimal piece of board specific code that just enables
whichever one gets detected at boot time?
Arnd
^ permalink raw reply
* Re: [PATCH 11/11] ARM: versatile: move CLCD configuration to device tree
From: Linus Walleij @ 2016-02-23 9:08 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1454594660-7532-12-git-send-email-linus.walleij@linaro.org>
On Thu, Feb 4, 2016 at 3:04 PM, Linus Walleij <linus.walleij@linaro.org> wrote:
> This moves the versatile CLCD configuration to the device tree by:
As this Schrödinger's cat approach seems controversial, as well
as the alternative to manipulate the DT in memory at run-time,
I will respin the series without the full Versatile support, adding
plug-in for the other ARM boards and and half-baked support
for the Versatile supporting just VGA (like the other reference
designs from ARM).
The pluggable displays prove yet again to be a problem to the
world, sigh.
I will think of a better solution, if any, for this for v4.6, but will
put forward something that handles the Nomadik and all the
other ARM reference designs for now.
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH] fbdev: sh_mobile_lcdc: Use ARCH_RENESAS
From: Laurent Pinchart @ 2016-02-23 7:41 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1456106391-10353-1-git-send-email-horms+renesas@verge.net.au>
Hi Simon,
On Tuesday 23 February 2016 09:11:03 Simon Horman wrote:
> On Mon, Feb 22, 2016 at 03:05:58PM +0200, Laurent Pinchart wrote:
> > On Monday 22 February 2016 13:39:37 Geert Uytterhoeven wrote:
> >> On Mon, Feb 22, 2016 at 1:24 PM, Laurent Pinchart wrote:
> >>> On Monday 22 February 2016 10:59:51 Simon Horman wrote:
> >>>> Make use of ARCH_RENESAS in place of ARCH_SHMOBILE.
> >>>>
> >>>> This is part of an ongoing process to migrate from ARCH_SHMOBILE to
> >>>> ARCH_RENESAS the motivation for which being that RENESAS seems to be
> >>>> a more appropriate name than SHMOBILE for the majority of Renesas ARM
> >>>> based SoCs.
> >>>>
> >>>> Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
> >>>
> >>> Wouldn't it make sense to drop the driver instead ? We have a DRM
> >>> driver that replaces it.
> >>
> >> Does the DRM driver work on all hardware supported by the fbdev driver?
> >> It's not only used on r8a7740/armadillo (through staging/board due to
> >> lack of DT support), but also on many SH boards.
> >
> > It's supposed to be a replacement (lacking support for SYS panels though),
> > but has obviously not been tested on SH boards.
>
> From my point of view it would be overreach to remove the driver as we
> aren't in a position to test the SH boards.
I'd be surprised if the driver still worked on those boards, but I like good
surprises :-)
> We could stop using it on the Renesas ARM SoCs and in turn remove
> ARCH_SHMOBILE/ARCH_RENESAS. Am I right in thinking that would only
> effect the r8a7740/armadillo at this time?
That's correct.
--
Regards,
Laurent Pinchart
^ permalink raw reply
* Re: [PATCH v3 00/12] pwm: add support for atomic update
From: Doug Anderson @ 2016-02-23 3:03 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20160222212438.GG18327@sirena.org.uk>
Mark,
On Mon, Feb 22, 2016 at 1:24 PM, Mark Brown <broonie@kernel.org> wrote:
> On Mon, Feb 22, 2016 at 11:15:09AM -0800, Doug Anderson wrote:
>
>> Note that historically I remember that Linus Torvalds has stated that
>> there is no stable API within the Linux kernel and that forcing the
>> in-kernel API to never change was bad for software development. I
>> tracked down my memory and found
>> <http://lwn.net/1999/0211/a/lt-binary.html>. Linus is rabid about not
>> breaking userspace, but in general there's no strong requirement to
>> never change the driver API inside the kernel. That being said,
>> changing the driver API causes a lot of churn, so presumably changing
>> it in a backward compatible way (like adding to the API instead of
>> changing it) will make things happier.
>
> You do need to fix the users though, change is fine but you can't cause
> people's systems to break.
Yes, of course! :) Thanks for clarifying.
-Doug
^ permalink raw reply
* Re: [PATCH] fbdev: sh_mobile_lcdc: Use ARCH_RENESAS
From: Simon Horman @ 2016-02-23 0:11 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1456106391-10353-1-git-send-email-horms+renesas@verge.net.au>
On Mon, Feb 22, 2016 at 03:05:58PM +0200, Laurent Pinchart wrote:
> Hi Geert,
>
> On Monday 22 February 2016 13:39:37 Geert Uytterhoeven wrote:
> > On Mon, Feb 22, 2016 at 1:24 PM, Laurent Pinchart wrote:
> > > On Monday 22 February 2016 10:59:51 Simon Horman wrote:
> > >> Make use of ARCH_RENESAS in place of ARCH_SHMOBILE.
> > >>
> > >> This is part of an ongoing process to migrate from ARCH_SHMOBILE to
> > >> ARCH_RENESAS the motivation for which being that RENESAS seems to be a
> > >> more appropriate name than SHMOBILE for the majority of Renesas ARM based
> > >> SoCs.
> > >>
> > >> Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
> > >
> > > Wouldn't it make sense to drop the driver instead ? We have a DRM driver
> > > that replaces it.
> >
> > Does the DRM driver work on all hardware supported by the fbdev driver?
> > It's not only used on r8a7740/armadillo (through staging/board due to lack
> > of DT support), but also on many SH boards.
>
> It's supposed to be a replacement (lacking support for SYS panels though), but
> has obviously not been tested on SH boards.
From my point of view it would be overreach to remove the driver as we
aren't in a position to test the SH boards.
We could stop using it on the Renesas ARM SoCs and in turn remove
ARCH_SHMOBILE/ARCH_RENESAS. Am I right in thinking that would only
effect the r8a7740/armadillo at this time?
^ permalink raw reply
* Re: [PATCH v3 00/12] pwm: add support for atomic update
From: Mark Brown @ 2016-02-22 21:24 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAD=FV=UCErW3EjYRtwt49_ztNR7cyY4U0o4R_dxdomer-Ac2Dg@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 802 bytes --]
On Mon, Feb 22, 2016 at 11:15:09AM -0800, Doug Anderson wrote:
> Note that historically I remember that Linus Torvalds has stated that
> there is no stable API within the Linux kernel and that forcing the
> in-kernel API to never change was bad for software development. I
> tracked down my memory and found
> <http://lwn.net/1999/0211/a/lt-binary.html>. Linus is rabid about not
> breaking userspace, but in general there's no strong requirement to
> never change the driver API inside the kernel. That being said,
> changing the driver API causes a lot of churn, so presumably changing
> it in a backward compatible way (like adding to the API instead of
> changing it) will make things happier.
You do need to fix the users though, change is fine but you can't cause
people's systems to break.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply
* [PATCH 9/9] simplefb: Remove impossible check for of_clk_get_parent_count() < 0
From: Stephen Boyd @ 2016-02-22 20:54 UTC (permalink / raw)
To: Michael Turquette, Stephen Boyd
Cc: linux-kernel, linux-clk, Hans de Goede,
Jean-Christophe Plagniol-Villard, Tomi Valkeinen, linux-fbdev
In-Reply-To: <1456174487-28397-1-git-send-email-sboyd@codeaurora.org>
The check for < 0 is impossible now that
of_clk_get_parent_count() returns an unsigned int. Simplify the
code and update the types.
Cc: Hans de Goede <hdegoede@redhat.com>
Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: <linux-fbdev@vger.kernel.org>
Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
---
Please ack so this can go through clk tree along with patch 1.
drivers/video/fbdev/simplefb.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/video/fbdev/simplefb.c b/drivers/video/fbdev/simplefb.c
index 48ccf6db62a2..e9cf19977285 100644
--- a/drivers/video/fbdev/simplefb.c
+++ b/drivers/video/fbdev/simplefb.c
@@ -174,7 +174,7 @@ static int simplefb_parse_pd(struct platform_device *pdev,
struct simplefb_par {
u32 palette[PSEUDO_PALETTE_SIZE];
#if defined CONFIG_OF && defined CONFIG_COMMON_CLK
- int clk_count;
+ unsigned int clk_count;
struct clk **clks;
#endif
#if defined CONFIG_OF && defined CONFIG_REGULATOR
@@ -213,7 +213,7 @@ static int simplefb_clocks_init(struct simplefb_par *par,
return 0;
par->clk_count = of_clk_get_parent_count(np);
- if (par->clk_count <= 0)
+ if (!par->clk_count)
return 0;
par->clks = kcalloc(par->clk_count, sizeof(struct clk *), GFP_KERNEL);
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply related
* [PATCH 0/9] Change return type of of_clk_get_parent_count() to unsigned
From: Stephen Boyd @ 2016-02-22 20:54 UTC (permalink / raw)
To: Michael Turquette, Stephen Boyd
Cc: linux-kernel, linux-clk, Boris Brezillon, Yoshinori Sato,
uclinux-h8-devel, Gabriel Fernandez, Maxime Ripard, Chen-Yu Tsai,
Tero Kristo, Felipe Balbi, linux-usb, Hans de Goede,
Jean-Christophe Plagniol-Villard, Tomi Valkeinen, linux-fbdev
Russell King recently pointed out that of_clk_get_parent_count() can
return a negative number if the "clocks" property isn't present in a
node. Upon investigating all the callers of this API, none of the them
seem to care about that case, and furthermore some callers are assigning
the return value to an unsigned int which could cause very large numbers
of parents to be used.
This series changes the API to return an unsigned int from 0 (no parents)
to N, thereby avoiding any problems with negative return values.
Cc: Boris Brezillon <boris.brezillon@free-electrons.com>
Cc: Yoshinori Sato <ysato@users.sourceforge.jp>
Cc: <uclinux-h8-devel@lists.sourceforge.jp>
Cc: Gabriel Fernandez <gabriel.fernandez@st.com>
Cc: Maxime Ripard <maxime.ripard@free-electrons.com>
Cc: Chen-Yu Tsai <wens@csie.org>
Cc: Tero Kristo <t-kristo@ti.com>
Cc: Felipe Balbi <balbi@ti.com>
Cc: <linux-usb@vger.kernel.org>
Cc: Hans de Goede <hdegoede@redhat.com>
Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: <linux-fbdev@vger.kernel.org>
Stephen Boyd (9):
clk: Make of_clk_get_parent_count() return unsigned ints
clk: at91: Remove impossible checks for of_clk_get_parent_count()
clk: gpio: Remove impossible check for of_clk_get_parent_count() < 0
clk: h8300: Remove impossible check for of_clk_get_parent_count()
clk: st: Remove impossible check for of_clk_get_parent_count() < 0
clk: sunxi: Use proper type for of_clk_get_parent_count() return value
clk: ti: Update for of_clk_get_parent_count() returning unsigned int
usb: dwc3: Remove impossible check for of_clk_get_parent_count() < 0
simplefb: Remove impossible check for of_clk_get_parent_count() < 0
drivers/clk/at91/clk-generated.c | 4 ++--
drivers/clk/at91/clk-main.c | 4 ++--
drivers/clk/at91/clk-master.c | 4 ++--
drivers/clk/at91/clk-programmable.c | 4 ++--
drivers/clk/at91/clk-slow.c | 6 +++---
drivers/clk/at91/clk-smd.c | 4 ++--
drivers/clk/at91/clk-usb.c | 4 ++--
drivers/clk/clk-gpio.c | 6 ++----
drivers/clk/clk.c | 16 ++++++++++++++--
drivers/clk/h8300/clk-div.c | 4 ++--
drivers/clk/h8300/clk-h8s2678.c | 4 ++--
drivers/clk/st/clk-flexgen.c | 4 ++--
drivers/clk/st/clkgen-mux.c | 4 ++--
drivers/clk/sunxi/clk-sun6i-ar100.c | 2 +-
drivers/clk/ti/clockdomain.c | 2 +-
drivers/clk/ti/composite.c | 8 ++++----
drivers/clk/ti/dpll.c | 2 +-
drivers/clk/ti/mux.c | 4 ++--
drivers/usb/dwc3/dwc3-of-simple.c | 9 +++++----
drivers/video/fbdev/simplefb.c | 4 ++--
include/linux/clk-provider.h | 2 +-
21 files changed, 56 insertions(+), 45 deletions(-)
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply
* Re: [PATCH v3 00/12] pwm: add support for atomic update
From: Doug Anderson @ 2016-02-22 19:15 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20160222175929.GA23899@ulmo>
Thierry,
On Mon, Feb 22, 2016 at 9:59 AM, Thierry Reding
<thierry.reding@gmail.com> wrote:
>> This is because only some drivers would be able to read the hardware
>> state? I'm not sure how we can get away from that. In all proposals
>> we've talked about (including what you propose below, right?) the PWM
>> regulator will need a PWM driver that can read hardware state. Only
>> PWM drivers that have been upgraded to support reading hardware state
>> can use the PWM regulator (or at least only those drivers will be able
>> to use the PWM regulator glitch-free).
>
> Yes, the key here is glitch-free. There's no reason whatsoever that the
> rugaltor-pwm driver should be limited to usage with a hardware readout-
> capable PWM driver. If you don't care about glitches, likely because no
> critical components depend on the regulator, you can simply force what
> state you choose on boot.
>
> As a matter of fact, I think that's how regulators work already. If the
> current output voltage doesn't match the specified constraints, then a
> valid value will be forced by the regulator core. If the voltage lies
> within the constraints the core won't touch the regulator. Is this not
> going to "just work" with the PWM regulator?
>
> The problem is somewhat simplified if that's the case. An implementation
> could then fail the regulator_get_voltage() if hardware readout is not
> supported and return the current voltage when readout is possible.
Based on looking at the current code, I believe it just returns 0V
until you call regulator_set_voltage() today. I don't think that was
always the case. Back before it was made continuous I think it
returned the voltage that matched with 0% duty cycle.
I haven't dug into what the regulator framework does in the current
system nor what happens if regulator_get_voltage() returns an error.
Perhaps Boris can dig / comment?
>> When we add a new feature then it's expected that only updated drivers
>> will support that feature.
>>
>> We need to make sure that we don't regress / negatively change the
>> behavior for anyone running non-updated drivers. ...and we should
>> strive to require as few changes to drivers as possible. ...but if
>> the best we can do requires changes to the PWM driver API then we will
>> certainly have differences depending on the PWM driver.
>
> How so? Drivers should behave consistently, irrespective of the API. Of
> course if you need to change behaviour of the user driver depending on
> the availability of a certain feature, that's perfectly fine.
>
> Furthermore it's out of the question that changes to the API will be
> required. That's precisely the reason why the atomic PWM proposal came
> about. It's an attempt to solve the shortcomings of the current API for
> cases such as Rockchip.
I _think_ we're on the same page here. If there are shortcomings with
the current API that make it impossible to implement a feature, we've
got to change and/or add to the existing API. ...but we don't want to
break existing users / drivers.
Note that historically I remember that Linus Torvalds has stated that
there is no stable API within the Linux kernel and that forcing the
in-kernel API to never change was bad for software development. I
tracked down my memory and found
<http://lwn.net/1999/0211/a/lt-binary.html>. Linus is rabid about not
breaking userspace, but in general there's no strong requirement to
never change the driver API inside the kernel. That being said,
changing the driver API causes a lot of churn, so presumably changing
it in a backward compatible way (like adding to the API instead of
changing it) will make things happier.
>> That means that if you call pwm_get_period() right away at boot time
>> you're not getting the current period of the hardware but the period
>> that was specified in the device tree.
>
> Yes.
>
>> So all we need is a new API call that lets you read the hardware
>> values and make sure that the PWM regulator calls that before anyone
>> calls pwm_config(). That's roughly B) above.
>
> Yes. I'm thinking that we should have a pwm_get_state() which retrieves
> the current state of the PWM. For drivers that support hardware readout
> this state should match the hardware state. For other drivers it should
> reflect whatever was specified in DT; essentially what pwm_get_period()
> and friends return today.
Excellent, so pwm_get_period() gets the period as specified in the
device tree (or other board config) and pwm_get_state() returns the
hardware state. SGTM.
> That way if you want to get the current voltage in the regulator-pwm
> driver you'd simply do a pwm_get_state() and compute the voltage from
> the period and duty cycle. If the PWM driver that you happen to use
> doesn't support hardware readout, you'll get an initial output voltage
> of 0, which is as good as any, really.
Sounds fine to me. PWM regulator is in charge of calling
pwm_get_state(), which can return 0 (or an error?) if driver (or
underlying hardware) doesn't support hardware readout. PWM regulator
is in charge of using the resulting period / duty cycle to calculate a
percentage.
>> Sure. ...but you agree that somehow you need a new API call for this,
>> right? Somehow the PWM regulator needs to be able to say that it
>> wants the hardware state, not the initial state as specified in the
>> device tree.
>
> The atomic PWM API is this new API. I'm not arguing that we don't need
> new API. What's being discussed is what the API needs to look like to
> support this new use-case and at the same time be maximally compatible
> with existing users.
>
> I think perhaps one question that needs answering to do that is whether
> or not the regulator-pwm driver can guess the correct period. The issue
> that was initially being discussed is what to do with hardware state vs
> initial state (i.e. what's defined in DT). Mark commented in another
> subthread that DT should simply leave out data that doesn't make sense
> to be specified.
>
> However I don't think there's any particularly reasonable period for the
> regulator-pwm use-case, so specifying the period within DT would still
> be necessary. What works for one board may not be the correct (or
> optimal) value for another. Similarly one board may need to invert the
> PWM signal to generate the proper voltage, or require other parameters
> that we haven't even defined yet.
>
> There's also the issue of a voltage table that you need to define in the
> DT for the regulator-pwm device. Does that consist of only duty-cycle
> values, or does it have corresponding period values as well. I'd guess
> that it really only needs the duty cycle given any period. So something
> like this is what I'd expect:
>
> regulator {
> compatible = "regulator-pwm";
>
> pwms = <&pwm0 5000>;
>
> voltages = <0 0>, <1000000 1000>, ..., <5000000 5000>;
> };
>
> I think the pwm-regulator binding defines the duty cycle in percent. I
> guess that would work equally well.
>
> And now we've come full circle because, again, we need to differentiate
> between the current hardware state and the initial state. Or, as was
> also discussed previously, alternatively, ignore the period specified in
> DT and just go with what hardware readout defined. In case hardware
> readout isn't supported, the "hardware state" will simply be the
> "initial state".
>
> The objection to the latter alternative was that we shouldn't trust the
> firmware to have set up the regulator correctly. But if it didn't, how
> can we trust that the duty cycle to period ratio is correct? Or the
> other way around: if we trust the duty cycle to period ratio to have
> been setup correctly, why don't we trust the period?
To answer:
* No, PWM regulator can't guess the correct period.
* PWM regulator API is already fine as is. Period specified in PWM
specifier and table is specified in percentages.
* Firmware may have voltage setup correctly but may not have the
"ideal" period. Often firmware configures things in a way that works
but is sub-optimal. Basically the kernel should be able to tell what
voltage the firmware set things up at (so we know not to go lower on
accident), but we shouldn't assume that the firmware values are
perfect. Said another way: obviously the firmware made values that
were good enough to boot us to where we are (or we wouldn't be even
executing code), but we might want to configure things to reduce noise
on the lines (make HDMI work better?) or optimize power consumption.
--
I _think_ the end result of all this is just:
1. Introduce pwm_get_state() that gets hardware state. Up for debate
if this returns 0 or ERROR if a driver doesn't implement this.
2. PWM regulator calls pwm_get_state at probe time to get hardware
state, calculates a percentage (and voltage) with this.
3. PWM regulator does nothing else until it is asked to set the
voltage, but uses the voltage calculated from #2 to satisfy any "get
voltage" calls.
4. When asked to set the voltage, PWM regulator uses pwm_get_period()
and calculates a duty cycle based on that, just like it does today.
This uses pwm_config() which includes a duty cycle and period and is
thus "atomic".
Said another way: the only action item from this point of view is just
that we introduce a new pwm_get_state().
Boris: I think that's everything you need, right?
--
Historically there was also a necessity that we were very careful with
the PWM clock because the clock would end up getting disabled
temporarily at bootup (after the PWM probe time but before the PWM
regulator finished probing). Presumably that's either been solved
already or can be debated totally separately.
-Doug
^ 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;
as well as URLs for NNTP newsgroup(s).