Re: [PATCH RFC v2 04/11] ARM: dts: omap4: Add device tree entry for SGX GPU
Hi, On Mon, 15 Jan 2024 09:55:00 +0100 "H. Nikolaus Schaller" wrote: > > There's no reason to disable it in the DT: the hardware block would > > still be there and it's rendering to memory so it still could be useful. > > Well, if you know that the board does not have a dm3730 but a dm3725 without > GPU it is better to disable the GPU completely instead of loading the driver > and make it detect by some internal bits that it has no GPU on the SoC. > That is at least some valid reason. > > If there's no display on the board and you really don't want the GPU > > driver, then you can disable the driver or block the module loading, but > > it should be a distro / package / user decision, not a DT / kernel one > > still. > > The same holds for aes: dss: dsi: hdmi: etc. If they are not used by some > board file, they don't change a single bit of the DTB [1] which IMHO would > be of reasonable concern to question additional labels. There is some difference here, some hardware can just not be used without wired external pins, the gpu can be used even if no display is connected either to accelerate some remote access or you could use the gpu for something completely else... Maybe mining bitcoins if temperate gets too low to warm you pocket ;-) But as these labels do not harm, I have no strong opinion against it. Regards, Andreas
Re: [PATCH RFC 01/10] dt-bindings: gpu: Add PowerVR Series5 SGX GPUs
On Tue, 5 Dec 2023 09:45:44 +0100 Krzysztof Kozlowski wrote: > > Sure the clock nodes can be there for the child IP, but they won't do > > anything. And still need to be managed separately by the device driver if > > added. > > So if OS does not have runtime PM, the bindings are wrong? Bindings > should not depend on some particular feature of some particular OS. Any user of the devicetree sees that there is a parent and the parent needs to be enabled by some mechanism. E.g. I2c devices do not specify the clocks of the parent (the i2c master) Maybe it is just more fine-grained on omap. look e.g. at ti/omap/omap4-l4.dtsi there are target-module@ with the devices as a child and a clock in the parent. Regards, Andreas
Re: [PATCH RFC 01/10] dt-bindings: gpu: Add PowerVR Series5 SGX GPUs
On Tue, 5 Dec 2023 10:27:56 +0100 Krzysztof Kozlowski wrote: > On 05/12/2023 10:02, Andreas Kemnade wrote: > > On Tue, 5 Dec 2023 09:45:44 +0100 > > Krzysztof Kozlowski wrote: > > > >>> Sure the clock nodes can be there for the child IP, but they won't do > >>> anything. And still need to be managed separately by the device driver if > >>> added. > >> > >> So if OS does not have runtime PM, the bindings are wrong? Bindings > >> should not depend on some particular feature of some particular OS. > > > > Any user of the devicetree sees that there is a parent and the parent needs > > to be enabled by some mechanism. > > E.g. I2c devices do not specify the clocks of the parent (the i2c master) > > If you use this analogy, then compare it with an I2C device which has > these clock inputs. Such device must have clocks in the bindings. > I would see target-module = i2c master. Well, if there is a variant of the i2c device which does not require external clocks and a variant which requires it, then clock can be optional. > > > > Maybe it is just more fine-grained on omap. > > > > look e.g. at ti/omap/omap4-l4.dtsi > > there are target-module@ > > with the devices as a child and a clock in the parent. > > Not related to runtime PM... > Well, runtime PM is just the linux-specific mechanism to enable the resources needed by the parent, so yes, it is not related... As said, another OS can have another mechanism. But anyways, the target module specifies resources which are required. Regards, Andreas
Re: [RFC PATCH] drm: omapdrm: dsi: add refsel also for omap4
Am Wed, 13 Sep 2023 15:58:11 +0300 schrieb Tomi Valkeinen : > On 13/09/2023 15:48, Tony Lindgren wrote: > > * Tomi Valkeinen [230913 12:11]: > >> I'm somewhat sure that the upstream driver used to work on omap4 > >> sdp, which has two DSI panels. But I can't even remember what > >> omap4 version it had. > > > > I think those were both dsi command mode panels though, not video > > mode? > > Yes, true. If the PLL is totally wrong due to refsel, I'm sure a > command mode panel would also fail. But it's true that video mode > panels are more sensitive to the clock rate. > hmm, still analyzing: What works: OMAP5 + Pyra (Videomode display requiring some init commands) some command mode stuff with OMAP4 (droid4) What does not work: OMAP4 with some dsi videomode to something else (LVDS/DPI) converter if init commands are sent through dsi, then these commands fail with bta sync problems. So sending init commands to video mode displays seems not to be a principal problem. But looking deeper at the drivers, there seem to be commands sent to the converters to configure lanes on that side, e.g. tc358762_write(ctx, DSI_LANEENABLE, LANEENABLE_L0EN | LANEENABLE_CLEN); There might be trouble if these are not sent in low power mode. So probably the next analyzing step would be to check if things are really sent in low power mode. Regards, Andreas
Re: [RFC PATCH] drm: omapdrm: dsi: add refsel also for omap4
On Wed, 13 Sep 2023 15:11:08 +0300 Tomi Valkeinen wrote: > On 13/09/2023 09:59, Andreas Kemnade wrote: > > Some 3.0 source has it set behind a if (omap4). > > Maybe it is helpful maybe not, at least in the omap4460 > > trm these bits are marked as reserved. > > But maybe some dsi video mode panel starts magically working. > > Sorry, what does this mean? That this fixes something, or you are > just guessing? > just diffing registers between good and bad. It does not fix anything, just reducing the diff. > I'm somewhat sure that the upstream driver used to work on omap4 sdp, > which has two DSI panels. But I can't even remember what omap4 > version it had. > after we are using displays from gpu/drm/displays? Regards Andreas
[RFC PATCH] drm: omapdrm: dsi: add refsel also for omap4
Some 3.0 source has it set behind a if (omap4). Maybe it is helpful maybe not, at least in the omap4460 trm these bits are marked as reserved. But maybe some dsi video mode panel starts magically working. Signed-off-by: Andreas Kemnade --- drivers/gpu/drm/omapdrm/dss/dsi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/omapdrm/dss/dsi.c b/drivers/gpu/drm/omapdrm/dss/dsi.c index 60189a23506a1..e2f576cd9f63c 100644 --- a/drivers/gpu/drm/omapdrm/dss/dsi.c +++ b/drivers/gpu/drm/omapdrm/dss/dsi.c @@ -4505,7 +4505,7 @@ static const struct dss_pll_hw dss_omap4_dsi_pll_hw = { .has_stopmode = true, .has_freqsel = false, .has_selfreqdco = false, - .has_refsel = false, + .has_refsel = true, }; static const struct dss_pll_hw dss_omap5_dsi_pll_hw = { -- 2.39.2
Re: [RFC PATCH 00/16] drm/rockchip: Rockchip EBC ("E-Book Controller") display driver
On Wed, 13 Apr 2022 17:19:00 -0500 Samuel Holland wrote: [...] > Waveform Selection From Userspace > = > EPDs use different waveforms for different purposes: high-quality > grayscale vs. monochrome text vs. dithered monochrome video. How can > userspace select which waveform to use? Should this be a plane property? > Or does userspace rather select a QoS, like low-latency vs. high quality. Or this will not change for a longer time: like doing full refreshes. > It is also likely that userspace will want to use different waveforms at > the same time for different parts of the screen, for example a fast > monochrome waveform for the drawing area of a note-taking app, but a > grayscale waveform for surrounding UI and window manager. > > I believe the i.MX6 EPDC supports multiple planes, each with their own > waveform choice. That seems like a good abstraction, but the EBC only > supports one plane in hardware. So using this abstraction with the EBC > would require blending pixels and doing waveform lookups in software. > The iMX6 EPDC has one working buffer containing the old+new state of the pixel. That is 16bpp. Then for each update you can specify a rectangle in an independant 8bpp buffer as a source. For now I am just using a single buffer. But yes, that construction could be used to do some multi plane stuff. > Blitting/Blending in Software > = > There are multiple layers to this topic (pun slightly intended): > 1) Today's userspace does not expect a grayscale framebuffer. > Currently, the driver advertises XRGB and converts to Y4 > in software. This seems to match other drivers (e.g. repaper). > > 2) Ignoring what userspace "wants", the closest existing format is > DRM_FORMAT_R8. Geert sent a series[4] adding DRM_FORMAT_R1 through > DRM_FORMAT_R4 (patch 9), which I believe are the "correct" formats > to use. > hmm R=red? That sounds strange. I am unsure whether doing things with lower bit depths actually really helps. > 3) The RK356x SoCs have an "RGA" hardware block that can do the > RGB-to-grayscale conversion, and also RGB-to-dithered-monochrome > which is needed for animation/video. Currently this is exposed with > a V4L2 platform driver. Can this be inserted into the pipeline in a > way that is transparent to userspace? Or must some userspace library > be responsible for setting up the RGA => EBC pipeline? hmm, we have other drivers with some hardware block doing rotation, but in that cases it is not exposed as v4l2 mem2mem device. On IMX6 there is also the PXP doing RGB-to-grayscale and rotation but exposed as v4l2 device. But it can also be used to do undocumented stuff writing to the 16bpp working buffer. So basically it is similar. But I would do thoso things in a second step and just have the basic stuff upstreamed Regards, Andreas
Re: [RFC PATCH 02/16] dt-bindings: display: rockchip: Add EBC binding
On Thu, 14 Apr 2022 22:00:09 -0500 Samuel Holland wrote: > Hi Andreas, > > Thanks for the comments. > > On 4/14/22 3:15 AM, Andreas Kemnade wrote: > > Hi Samuel, > > > > for comparison, here is my submission for the IMX EPDC bindings: > > > > https://lore.kernel.org/linux-devicetree/20220206080016.796556-2-andr...@kemnade.info/ > > > > On Wed, 13 Apr 2022 17:19:02 -0500 > > Samuel Holland wrote: > > > > [...] > > we have sy7636a driver in kernel which should be suitable for powering a EPD > > and temperature measurement. So I would expect that to be > >> + io-channels: > >> +maxItems: 1 > >> +description: I/O channel for panel temperature measurement > >> + > > so how would I reference the hwmon/thermal(-zone) of the sy7636a here? > > It seems the consensus is to use a thermal zone for panel temperature, so I > will > need to change this. > I am open to anything here as long as it fits together. > I think it's best to reference the thermal zone by phandle, not by name, even > if > it requires extending the thermal zone API to support this. > maybe referencing the hwmon might be interesting, or we add a hwmon_iio adaptor. The other way round it is there. The thermal zone stuff is only needed because hwmon cannot referenced directly. Regards, Andreas
Re: [RFC PATCH 02/16] dt-bindings: display: rockchip: Add EBC binding
Hi Samuel, for comparison, here is my submission for the IMX EPDC bindings: https://lore.kernel.org/linux-devicetree/20220206080016.796556-2-andr...@kemnade.info/ On Wed, 13 Apr 2022 17:19:02 -0500 Samuel Holland wrote: [...] we have sy7636a driver in kernel which should be suitable for powering a EPD and temperature measurement. So I would expect that to be > + io-channels: > +maxItems: 1 > +description: I/O channel for panel temperature measurement > + so how would I reference the hwmon/thermal(-zone) of the sy7636a here? > + panel-supply: > +description: Regulator supplying the panel's logic voltage > + > + power-domains: > +maxItems: 1 > + > + vcom-supply: > +description: Regulator supplying the panel's compensation voltage > + > + vdrive-supply: > +description: Regulator supplying the panel's gate and source drivers > + SY7636a has only one logical regulator in kernel for for the latter two. If we have a separate panel node, than maybe these regulators should go there as they belong to the panel as they are powering the panel and not the EBC. > + port: > +$ref: /schemas/graph.yaml#/properties/port > +description: OF graph port for the attached display panel > + In my approach for the IMX EPDC, (I will send a better commented one soon) I have no separate subnode to avoid messing with additional display parameters. Not sure what is really better here. > +required: > + - compatible > + - reg > + - interrupts > + - clocks > + - clock-names > + - resets > + - reset-names > + - power-domains > + - panel-supply > + - vcom-supply > + - vdrive-supply If things differ how the different phyiscally existing regulators are mapped into logical ones (even the vdrive supply is still a bunch of physical regulators mapped into one logical one), then not everything can be required. Regards, Andreas
Re: [RFC PATCH 1/6] dt-bindings: display: imx: Add EPDC
On Sat, 12 Mar 2022 20:23:48 +0100 Jonathan Neuschäfer wrote: > Hello Andreas, > > Sorry for the delay, I finally got around to having a look at the > patchset. > > Some comments from skimming the patches below, and in my other replies. > > > On Sun, Feb 06, 2022 at 09:00:11AM +0100, Andreas Kemnade wrote: > > Add a binding for the Electrophoretic Display Controller found at least > > in the i.MX6. > > The timing subnode is directly here to avoid having display parameters > > spread all over the plate. > > > > Supplies are organized the same way as in the fbdev driver in the > > NXP/Freescale kernel forks. The regulators used for that purpose, > > like the TPS65185, the SY7636A and MAX17135 have typically a single bit to > > start a bunch of regulators of higher or negative voltage with a > > well-defined timing. VCOM can be handled separately, but can also be > > incorporated into that single bit. > > > > Signed-off-by: Andreas Kemnade > > --- > > .../bindings/display/imx/fsl,mxc-epdc.yaml| 159 ++ > > 1 file changed, 159 insertions(+) > > create mode 100644 > > Documentation/devicetree/bindings/display/imx/fsl,mxc-epdc.yaml > > > > diff --git > > a/Documentation/devicetree/bindings/display/imx/fsl,mxc-epdc.yaml > > b/Documentation/devicetree/bindings/display/imx/fsl,mxc-epdc.yaml > > new file mode 100644 > > index ..7e0795cc3f70 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/display/imx/fsl,mxc-epdc.yaml > > @@ -0,0 +1,159 @@ > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > +%YAML 1.2 > > +--- > [...] > > + - vscan-holdoff > > + - sdoed-width > > + - sdoed-delay > > + - sdoez-width > > + - sdoez-delay > > + - gdclk-hp-offs > > + - gdsp-offs > > + - gdoe-offs > > + - gdclk-offs > > + - num-ce > > These parameters should perhaps have sane defaults in the driver, and be > optional in the DT. > First of all I think I should document them better (as said in an earlier review mail) I doubt there are sane defaults, in vendor kernels, there is typically a definition of these parameters and a video mode per display. > > > + > > +additionalProperties: false > > + > > +examples: > > + - | > > +#include > > +#include > > + > > +epdc: epdc@20f4000 { > [...] > > + > > +timing { > > +clock-frequency = <8000>; > > +hactive = <1448>; > > +hback-porch = <16>; > > +hfront-porch = <102>; > > +hsync-len = <28>; > > +vactive = <1072>; > > +vback-porch = <4>; > > +vfront-porch = <4>; > > +vsync-len = <2>; > > +}; > > +}; > > The way you did it here, the timing parameters are directly under the > EPDC node in the DT, but I wonder if it would be better to have a > separate node for the display panel, which can then provide the timing > parameters either in the DT or in the panel driver (selected by compatible > string of the panel). > IMHO it makes sense to store these timing parameters together with the timing parameters from above. If that all somehow comes from a panel driver, we need to design an interface for it. So for simplicity I added the stuff just to the EPDC node. Vendor kernel has this: struct imx_epdc_fb_mode { struct fb_videomode *vmode; int vscan_holdoff; int sdoed_width; int sdoed_delay; int sdoez_width; int sdoez_delay; int gdclk_hp_offs; int gdsp_offs; int gdoe_offs; int gdclk_offs; int num_ce; }; So things are basically combined here. Regards, Andreas
Re: [RFC PATCH 1/6] dt-bindings: display: imx: Add EPDC
On Thu, 17 Feb 2022 10:21:15 +0100 Krzysztof Kozlowski wrote: > On 06/02/2022 09:00, Andreas Kemnade wrote: > > Add a binding for the Electrophoretic Display Controller found at least > > in the i.MX6. > > The timing subnode is directly here to avoid having display parameters > > spread all over the plate. > > > > Supplies are organized the same way as in the fbdev driver in the > > NXP/Freescale kernel forks. The regulators used for that purpose, > > like the TPS65185, the SY7636A and MAX17135 have typically a single bit to > > start a bunch of regulators of higher or negative voltage with a > > well-defined timing. VCOM can be handled separately, but can also be > > incorporated into that single bit. > > > > Signed-off-by: Andreas Kemnade > > --- > > .../bindings/display/imx/fsl,mxc-epdc.yaml| 159 ++ > > 1 file changed, 159 insertions(+) > > create mode 100644 > > Documentation/devicetree/bindings/display/imx/fsl,mxc-epdc.yaml > > [..] > > + > > + DISPLAY-supply: > > +description: > > + A couple of +/- voltages automatically powered on in a defintive > > order > > Typo, definitive? > yes, of course. > > + > > + VCOM-supply: > > +description: compensation voltage > > + > > + V3P3-supply: > > All of supplies names - lowercase. > > > +description: V3P3 supply > > + > > + epd-thermal-zone: > > +description: > > + Zone to get temperature of the EPD from, practically ambient > > temperature. > > Is it a phandle? > a string used in of_property_read_string(priv->drm.dev->of_node, "epd-thermal-zone", ); if (thermal) { priv->thermal = thermal_zone_get_zone_by_name(thermal); if (IS_ERR(priv->thermal)) return dev_err_probe(priv->drm.dev, PTR_ERR(priv->thermal), "unable to get thermal"); } [...] > > +examples: > > + - | > > +#include > > +#include > > + > > +epdc: epdc@20f4000 { > > Generic node name, e.g. display-controller > hmm, does IHMO not make too much sense here. E.g. in the imx6sll.dtsi we have lcd-controller next to it. So having epd-controller? But that is exactly what epdc stands for. Regards, Andreas
Re: [RFC PATCH 1/6] dt-bindings: display: imx: Add EPDC
Hi Rob, On Fri, 11 Feb 2022 09:46:27 -0600 Rob Herring wrote: > On Sun, Feb 06, 2022 at 09:00:11AM +0100, Andreas Kemnade wrote: > > Add a binding for the Electrophoretic Display Controller found at least > > in the i.MX6. > > The first version was in i.MX50 (I helped design the register > interface). Is that version compatible? > it has some differences, but that could be detected by EPDC_VERSION register. I do not own such a device, so I cannot fully check. I have not seen any driver with devicetree for IMX5. For now I am rejecting anything which has a EPDC version which I cannot check. > > The timing subnode is directly here to avoid having display parameters > > spread all over the plate. > > > > Supplies are organized the same way as in the fbdev driver in the > > NXP/Freescale kernel forks. The regulators used for that purpose, > > like the TPS65185, the SY7636A and MAX17135 have typically a single bit to > > start a bunch of regulators of higher or negative voltage with a > > well-defined timing. VCOM can be handled separately, but can also be > > incorporated into that single bit. > > > > Signed-off-by: Andreas Kemnade > > --- > > .../bindings/display/imx/fsl,mxc-epdc.yaml| 159 ++ > > 1 file changed, 159 insertions(+) > > create mode 100644 > > Documentation/devicetree/bindings/display/imx/fsl,mxc-epdc.yaml > > > > diff --git > > a/Documentation/devicetree/bindings/display/imx/fsl,mxc-epdc.yaml > > b/Documentation/devicetree/bindings/display/imx/fsl,mxc-epdc.yaml > > new file mode 100644 > > index ..7e0795cc3f70 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/display/imx/fsl,mxc-epdc.yaml > > @@ -0,0 +1,159 @@ > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > +%YAML 1.2 > > +--- > > +$id: http://devicetree.org/schemas/display/imx/fsl,mxc-epdc.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: Freescale i.MX6 EPDC > > + > > +maintainers: > > + - Andreas Kemnade > > + > > +description: | > > + The EPDC is a controller for handling electronic paper displays found in > > + i.MX6 SoCs. > > + > > +properties: > > + compatible: > > +enum: > > + - fsl,imx6sl-epdc > > + - fsl,imx6sll-epdc > > Not compatible with each other? > differences are detectable by EPDC_VERSION register, so probably so problem. NXP/Freescale kernel uses fsl,imx6dl-epdc and fsl,imx7d-epdc (used also by imx6 devices with EPDC_VERSION = 3.0) in their drivers. fsl,imx6dl-epdc fsl,imx6sl-epdc fsl,imx6sll-epdc fsl,imx7d-epdc in their dtsis. But the general rule is to use as less as possible compatible strings if differences can be probed properly, so only one should be sufficient? Which one? Regards, Andreas
[RFC PATCH 1/6] dt-bindings: display: imx: Add EPDC
Add a binding for the Electrophoretic Display Controller found at least in the i.MX6. The timing subnode is directly here to avoid having display parameters spread all over the plate. Supplies are organized the same way as in the fbdev driver in the NXP/Freescale kernel forks. The regulators used for that purpose, like the TPS65185, the SY7636A and MAX17135 have typically a single bit to start a bunch of regulators of higher or negative voltage with a well-defined timing. VCOM can be handled separately, but can also be incorporated into that single bit. Signed-off-by: Andreas Kemnade --- .../bindings/display/imx/fsl,mxc-epdc.yaml| 159 ++ 1 file changed, 159 insertions(+) create mode 100644 Documentation/devicetree/bindings/display/imx/fsl,mxc-epdc.yaml diff --git a/Documentation/devicetree/bindings/display/imx/fsl,mxc-epdc.yaml b/Documentation/devicetree/bindings/display/imx/fsl,mxc-epdc.yaml new file mode 100644 index ..7e0795cc3f70 --- /dev/null +++ b/Documentation/devicetree/bindings/display/imx/fsl,mxc-epdc.yaml @@ -0,0 +1,159 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/display/imx/fsl,mxc-epdc.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Freescale i.MX6 EPDC + +maintainers: + - Andreas Kemnade + +description: | + The EPDC is a controller for handling electronic paper displays found in + i.MX6 SoCs. + +properties: + compatible: +enum: + - fsl,imx6sl-epdc + - fsl,imx6sll-epdc + + reg: +maxItems: 1 + + clocks: +items: + - description: Bus clock + - description: Pixel clock + + clock-names: +items: + - const: axi + - const: pix + + interrupts: +maxItems: 1 + + vscan-holdoff: +$ref: /schemas/types.yaml#/definitions/uint32 +maxItems: 1 + + sdoed-width: +$ref: /schemas/types.yaml#/definitions/uint32 +maxItems: 1 + + sdoed-delay: +$ref: /schemas/types.yaml#/definitions/uint32 +maxItems: 1 + + sdoez-width: +$ref: /schemas/types.yaml#/definitions/uint32 +maxItems: 1 + + sdoez-delay: +$ref: /schemas/types.yaml#/definitions/uint32 +maxItems: 1 + + gdclk-hp-offs: +$ref: /schemas/types.yaml#/definitions/uint32 +maxItems: 1 + + gdsp-offs: +$ref: /schemas/types.yaml#/definitions/uint32 +maxItems: 1 + + gdoe-offs: +$ref: /schemas/types.yaml#/definitions/uint32 +maxItems: 1 + + gdclk-offs: +$ref: /schemas/types.yaml#/definitions/uint32 +maxItems: 1 + + num-ce: +$ref: /schemas/types.yaml#/definitions/uint32 +maxItems: 1 + + timing: +$ref: /display/panel/panel-timing.yaml# + + DISPLAY-supply: +description: + A couple of +/- voltages automatically powered on in a defintive order + + VCOM-supply: +description: compensation voltage + + V3P3-supply: +description: V3P3 supply + + epd-thermal-zone: +description: + Zone to get temperature of the EPD from, practically ambient temperature. + + + +required: + - compatible + - reg + - clocks + - clock-names + - interrupts + - vscan-holdoff + - sdoed-width + - sdoed-delay + - sdoez-width + - sdoez-delay + - gdclk-hp-offs + - gdsp-offs + - gdoe-offs + - gdclk-offs + - num-ce + +additionalProperties: false + +examples: + - | +#include +#include + +epdc: epdc@20f4000 { +compatible = "fsl,imx6sl-epdc"; +reg = <0x020f4000 0x4000>; +interrupts = <0 97 IRQ_TYPE_LEVEL_HIGH>; +clocks = < IMX6SL_CLK_EPDC_AXI>, < IMX6SL_CLK_EPDC_PIX>; +clock-names = "axi", "pix"; + +pinctrl-names = "default"; +pinctrl-0 = <_epdc0>; +V3P3-supply = <_reg>; +VCOM-supply = <_reg>; +DISPLAY-supply = <_reg>; +epd-thermal-zone = "epd-thermal"; + +vscan-holdoff = <4>; +sdoed-width = <10>; +sdoed-delay = <20>; +sdoez-width = <10>; +sdoez-delay = <20>; +gdclk-hp-offs = <562>; +gdsp-offs = <662>; +gdoe-offs = <0>; +gdclk-offs = <225>; +num-ce = <3>; +status = "okay"; + +timing { +clock-frequency = <8000>; +hactive = <1448>; +hback-porch = <16>; +hfront-porch = <102>; +hsync-len = <28>; +vactive = <1072>; +vback-porch = <4>; +vfront-porch = <4>; +vsync-len = <2>; +}; +}; +... -- 2.30.2
[RFC PATCH 6/6] arm: dts: imx6sl: Add EPDC
Extend definition of EPDC. Signed-off-by: Andreas Kemnade --- arch/arm/boot/dts/imx6sl.dtsi | 3 +++ 1 file changed, 3 insertions(+) diff --git a/arch/arm/boot/dts/imx6sl.dtsi b/arch/arm/boot/dts/imx6sl.dtsi index c7d907c5c352..919e86e4fc74 100644 --- a/arch/arm/boot/dts/imx6sl.dtsi +++ b/arch/arm/boot/dts/imx6sl.dtsi @@ -765,8 +765,11 @@ pxp: pxp@20f { }; epdc: epdc@20f4000 { + compatible = "fsl,imx6sl-epdc"; reg = <0x020f4000 0x4000>; interrupts = <0 97 IRQ_TYPE_LEVEL_HIGH>; + clocks = < IMX6SL_CLK_EPDC_AXI>, < IMX6SL_CLK_EPDC_PIX>; + clock-names = "axi", "pix"; }; lcdif: lcdif@20f8000 { -- 2.30.2
[RFC PATCH 3/6] drm: mxc-epdc: Add display and waveform initialisation
Adds display parameter initialisation, display power up/down and waveform loading Signed-off-by: Andreas Kemnade --- drivers/gpu/drm/mxc-epdc/Makefile| 2 +- drivers/gpu/drm/mxc-epdc/epdc_hw.c | 495 +++ drivers/gpu/drm/mxc-epdc/epdc_hw.h | 8 + drivers/gpu/drm/mxc-epdc/epdc_waveform.c | 189 + drivers/gpu/drm/mxc-epdc/epdc_waveform.h | 7 + drivers/gpu/drm/mxc-epdc/mxc_epdc.h | 81 drivers/gpu/drm/mxc-epdc/mxc_epdc_drv.c | 94 + 7 files changed, 875 insertions(+), 1 deletion(-) create mode 100644 drivers/gpu/drm/mxc-epdc/epdc_hw.c create mode 100644 drivers/gpu/drm/mxc-epdc/epdc_hw.h create mode 100644 drivers/gpu/drm/mxc-epdc/epdc_waveform.c create mode 100644 drivers/gpu/drm/mxc-epdc/epdc_waveform.h diff --git a/drivers/gpu/drm/mxc-epdc/Makefile b/drivers/gpu/drm/mxc-epdc/Makefile index a47ced72b7f6..0263ef2bf0db 100644 --- a/drivers/gpu/drm/mxc-epdc/Makefile +++ b/drivers/gpu/drm/mxc-epdc/Makefile @@ -1,5 +1,5 @@ # SPDX-License-Identifier: GPL-2.0 -mxc_epdc_drm-y := mxc_epdc_drv.o +mxc_epdc_drm-y := mxc_epdc_drv.o epdc_hw.o epdc_waveform.o obj-$(CONFIG_DRM_MXC_EPDC) += mxc_epdc_drm.o diff --git a/drivers/gpu/drm/mxc-epdc/epdc_hw.c b/drivers/gpu/drm/mxc-epdc/epdc_hw.c new file mode 100644 index ..a74cbd237e0d --- /dev/null +++ b/drivers/gpu/drm/mxc-epdc/epdc_hw.c @@ -0,0 +1,495 @@ +// SPDX-License-Identifier: GPL-2.0+ +// Copyright (C) 2022 Andreas Kemnade +// +/* + * based on the EPDC framebuffer driver + * Copyright (C) 2010-2016 Freescale Semiconductor, Inc. + * Copyright 2017 NXP + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include "mxc_epdc.h" +#include "epdc_regs.h" +#include "epdc_hw.h" +#include "epdc_waveform.h" + +void mxc_epdc_powerup(struct mxc_epdc *priv) +{ + int ret = 0; + + mutex_lock(>power_mutex); + + /* +* If power down request is pending, clear +* powering_down to cancel the request. +*/ + if (priv->powering_down) + priv->powering_down = false; + + if (priv->powered) { + mutex_unlock(>power_mutex); + return; + } + + dev_dbg(priv->drm.dev, "EPDC Powerup\n"); + + priv->updates_active = true; + + /* Enable the v3p3 regulator */ + ret = regulator_enable(priv->v3p3_regulator); + if (IS_ERR((void *)ret)) { + dev_err(priv->drm.dev, + "Unable to enable V3P3 regulator. err = 0x%x\n", + ret); + mutex_unlock(>power_mutex); + return; + } + + usleep_range(1000, 2000); + + pm_runtime_get_sync(priv->drm.dev); + + /* Enable clocks to EPDC */ + clk_prepare_enable(priv->epdc_clk_axi); + clk_prepare_enable(priv->epdc_clk_pix); + + epdc_write(priv, EPDC_CTRL_CLEAR, EPDC_CTRL_CLKGATE); + + /* Enable power to the EPD panel */ + ret = regulator_enable(priv->display_regulator); + if (IS_ERR((void *)ret)) { + dev_err(priv->drm.dev, + "Unable to enable DISPLAY regulator. err = 0x%x\n", + ret); + mutex_unlock(>power_mutex); + return; + } + + ret = regulator_enable(priv->vcom_regulator); + if (IS_ERR((void *)ret)) { + dev_err(priv->drm.dev, + "Unable to enable VCOM regulator. err = 0x%x\n", + ret); + mutex_unlock(>power_mutex); + return; + } + + priv->powered = true; + + mutex_unlock(>power_mutex); +} + +void mxc_epdc_powerdown(struct mxc_epdc *priv) +{ + mutex_lock(>power_mutex); + + /* If powering_down has been cleared, a powerup +* request is pre-empting this powerdown request. +*/ + if (!priv->powering_down + || (!priv->powered)) { + mutex_unlock(>power_mutex); + return; + } + + dev_dbg(priv->drm.dev, "EPDC Powerdown\n"); + + /* Disable power to the EPD panel */ + regulator_disable(priv->vcom_regulator); + regulator_disable(priv->display_regulator); + + /* Disable clocks to EPDC */ + epdc_write(priv, EPDC_CTRL_SET, EPDC_CTRL_CLKGATE); + clk_disable_unprepare(priv->epdc_clk_pix); + clk_disable_unprepare(priv->epdc_clk_axi); + + pm_runtime_put_sync_suspend(priv->drm.dev); + + /* turn off the V3p3 */ + regulator_disable(priv->v3p3_regulator); + + priv->powered = false; + priv->powering_down = false; + + if (priv->wait_for_powerdown) { + priv->wait_for_powerdown = false; + complete(>po
[RFC PATCH 2/6] drm: Add skeleton for EPDC driver
This driver is for the EPD controller in the i.MX SoCs. Add a skeleton and basic things for the driver Signed-off-by: Andreas Kemnade --- drivers/gpu/drm/Kconfig | 2 + drivers/gpu/drm/Makefile| 1 + drivers/gpu/drm/mxc-epdc/Kconfig| 15 + drivers/gpu/drm/mxc-epdc/Makefile | 5 + drivers/gpu/drm/mxc-epdc/epdc_regs.h| 442 drivers/gpu/drm/mxc-epdc/mxc_epdc.h | 20 ++ drivers/gpu/drm/mxc-epdc/mxc_epdc_drv.c | 248 + 7 files changed, 733 insertions(+) create mode 100644 drivers/gpu/drm/mxc-epdc/Kconfig create mode 100644 drivers/gpu/drm/mxc-epdc/Makefile create mode 100644 drivers/gpu/drm/mxc-epdc/epdc_regs.h create mode 100644 drivers/gpu/drm/mxc-epdc/mxc_epdc.h create mode 100644 drivers/gpu/drm/mxc-epdc/mxc_epdc_drv.c diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig index b1f22e457fd0..6b6b44ff7556 100644 --- a/drivers/gpu/drm/Kconfig +++ b/drivers/gpu/drm/Kconfig @@ -390,6 +390,8 @@ source "drivers/gpu/drm/gud/Kconfig" source "drivers/gpu/drm/sprd/Kconfig" +source "drivers/gpu/drm/mxc-epdc/Kconfig" + config DRM_HYPERV tristate "DRM Support for Hyper-V synthetic video device" depends on DRM && PCI && MMU && HYPERV diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile index 301a44dc18e3..e5eb9815cf9a 100644 --- a/drivers/gpu/drm/Makefile +++ b/drivers/gpu/drm/Makefile @@ -131,6 +131,7 @@ obj-$(CONFIG_DRM_PANFROST) += panfrost/ obj-$(CONFIG_DRM_ASPEED_GFX) += aspeed/ obj-$(CONFIG_DRM_MCDE) += mcde/ obj-$(CONFIG_DRM_TIDSS) += tidss/ +obj-$(CONFIG_DRM_MXC_EPDC) += mxc-epdc/ obj-y += xlnx/ obj-y += gud/ obj-$(CONFIG_DRM_HYPERV) += hyperv/ diff --git a/drivers/gpu/drm/mxc-epdc/Kconfig b/drivers/gpu/drm/mxc-epdc/Kconfig new file mode 100644 index ..3f5744161cff --- /dev/null +++ b/drivers/gpu/drm/mxc-epdc/Kconfig @@ -0,0 +1,15 @@ +# SPDX-License-Identifier: GPL-2.0 +config DRM_MXC_EPDC + tristate "i.MX EPD Controller" + depends on DRM && OF + depends on (COMPILE_TEST || ARCH_MXC) + select DRM_KMS_HELPER + select DRM_KMS_CMA_HELPER + select DMA_CMA if HAVE_DMA_CONTIGUOUS + select CMA if HAVE_DMA_CONTIGUOUS + help + Choose this option if you have an i.MX system with an EPDC. + It enables the usage of E-paper displays. A waveform is expected + to be present in /lib/firmware/imx/epdc/epdc.fw + + If M is selected this module will be called mxc_epdc_drm. diff --git a/drivers/gpu/drm/mxc-epdc/Makefile b/drivers/gpu/drm/mxc-epdc/Makefile new file mode 100644 index ..a47ced72b7f6 --- /dev/null +++ b/drivers/gpu/drm/mxc-epdc/Makefile @@ -0,0 +1,5 @@ +# SPDX-License-Identifier: GPL-2.0 +mxc_epdc_drm-y := mxc_epdc_drv.o + +obj-$(CONFIG_DRM_MXC_EPDC) += mxc_epdc_drm.o + diff --git a/drivers/gpu/drm/mxc-epdc/epdc_regs.h b/drivers/gpu/drm/mxc-epdc/epdc_regs.h new file mode 100644 index ..83445c56d911 --- /dev/null +++ b/drivers/gpu/drm/mxc-epdc/epdc_regs.h @@ -0,0 +1,442 @@ +/* SPDX-License-Identifier: GPL-2.0+ */ +/* Copyright (C) 2010-2013 Freescale Semiconductor, Inc. */ + +#ifndef __EPDC_REGS_INCLUDED__ +#define __EPDC_REGS_INCLUDED__ + +/* + * Register addresses + **/ + +#define EPDC_CTRL 0x000 +#define EPDC_CTRL_SET 0x004 +#define EPDC_CTRL_CLEAR0x008 +#define EPDC_CTRL_TOGGLE 0x00C +#define EPDC_WB_ADDR_TCE_V30x010 +#define EPDC_WVADDR0x020 +#define EPDC_WB_ADDR 0x030 +#define EPDC_RES 0x040 +#define EPDC_FORMAT0x050 +#define EPDC_FORMAT_SET0x054 +#define EPDC_FORMAT_CLEAR 0x058 +#define EPDC_FORMAT_TOGGLE 0x05C +#define EPDC_WB_FIELD0 0x060 +#define EPDC_WB_FIELD0_SET 0x064 +#define EPDC_WB_FIELD0_CLEAR 0x068 +#define EPDC_WB_FIELD0_TOGGLE 0x06C +#define EPDC_WB_FIELD1 0x070 +#define EPDC_WB_FIELD1_SET 0x074 +#define EPDC_WB_FIELD1_CLEAR 0x078 +#define EPDC_WB_FIELD1_TOGGLE 0x07C +#define EPDC_WB_FIELD2 0x080 +#define EPDC_WB_FIELD2_SET 0x084 +#define EPDC_WB_FIELD2_CLEAR 0x088 +#define EPDC_WB_FIELD2_TOGGLE 0x08C +#define EPDC_WB_FIELD3 0x090 +#define EPDC_WB_FIELD3_SET 0x094 +#define EPDC_WB_FIELD3_CLEAR 0x098 +#define EPDC_WB_FIELD3_TOGGLE 0x09C +#define EPDC_FIFOCTRL 0x0A0 +#define EPDC_FIFOCTRL_SET 0x0A4 +#define EPDC_FIFOCTRL_CLEAR0x0A8 +#define EPDC_FIFOCTRL_TOGGLE 0x0AC +#define EPDC_UPD_ADDR
[RFC PATCH 0/6] drm: EPDC driver for i.MX6
Add a driver for the Electrophoretic Display Controller found in the i.MX6 SoCs. In combination with a driver for an EPD PMIC (like the TPS65185 or the SY7636A), it works with the EPDC found in i.MX6SLL based devices and the EPDC found in i.MX6SL devices. Support for waveforms might be limited, there was no 4bit waveform found which works with the 6SLL but it works with the vendor waveforms of the Kobo Clara HD (6SLL), the Tolino Shine 2/3 (6SL). On the 6SL devices, also the epdc_E060SCM.fw works but not as brilliant as the vendor one. It does not involve the PXP yet. The NXP/Freescale kernel fork uses that for rotation and mysterious waveform handling. That is not planed to be upstreamed in the first step. Also it does not provide any special userspace API to fine-tune updates. That is also IMHO something for a second step. Andreas Kemnade (6): dt-bindings: display: imx: Add EPDC drm: Add skeleton for EPDC driver drm: mxc-epdc: Add display and waveform initialisation drm: mxc-epdc: Add update management ARM: dts: imx6sll: add EPDC arm: dts: imx6sl: Add EPDC .../bindings/display/imx/fsl,mxc-epdc.yaml| 159 +++ arch/arm/boot/dts/imx6sl.dtsi |3 + arch/arm/boot/dts/imx6sll.dtsi|9 + drivers/gpu/drm/Kconfig |2 + drivers/gpu/drm/Makefile |1 + drivers/gpu/drm/mxc-epdc/Kconfig | 15 + drivers/gpu/drm/mxc-epdc/Makefile |5 + drivers/gpu/drm/mxc-epdc/epdc_hw.c| 497 +++ drivers/gpu/drm/mxc-epdc/epdc_hw.h|8 + drivers/gpu/drm/mxc-epdc/epdc_regs.h | 442 ++ drivers/gpu/drm/mxc-epdc/epdc_update.c| 1210 + drivers/gpu/drm/mxc-epdc/epdc_update.h|9 + drivers/gpu/drm/mxc-epdc/epdc_waveform.c | 189 +++ drivers/gpu/drm/mxc-epdc/epdc_waveform.h |7 + drivers/gpu/drm/mxc-epdc/mxc_epdc.h | 151 ++ drivers/gpu/drm/mxc-epdc/mxc_epdc_drv.c | 373 + 16 files changed, 3080 insertions(+) create mode 100644 Documentation/devicetree/bindings/display/imx/fsl,mxc-epdc.yaml create mode 100644 drivers/gpu/drm/mxc-epdc/Kconfig create mode 100644 drivers/gpu/drm/mxc-epdc/Makefile create mode 100644 drivers/gpu/drm/mxc-epdc/epdc_hw.c create mode 100644 drivers/gpu/drm/mxc-epdc/epdc_hw.h create mode 100644 drivers/gpu/drm/mxc-epdc/epdc_regs.h create mode 100644 drivers/gpu/drm/mxc-epdc/epdc_update.c create mode 100644 drivers/gpu/drm/mxc-epdc/epdc_update.h create mode 100644 drivers/gpu/drm/mxc-epdc/epdc_waveform.c create mode 100644 drivers/gpu/drm/mxc-epdc/epdc_waveform.h create mode 100644 drivers/gpu/drm/mxc-epdc/mxc_epdc.h create mode 100644 drivers/gpu/drm/mxc-epdc/mxc_epdc_drv.c -- 2.30.2
[RFC PATCH 4/6] drm: mxc-epdc: Add update management
The EPDC can process some dirty rectangles at a time, pick them up and forward them to the controller. Only processes not involving PXP are supported at the moment. Due to that and to work with more waveforms, there is some masking/shifting done. It was tested with the factory waveforms of Kobo Clara HD, Tolino Shine 3, and Tolino Shine 2HD. Also the waveform called epdc_E060SCM.fw from NXP BSP works with the i.MX6SL devices. Signed-off-by: Andreas Kemnade --- drivers/gpu/drm/mxc-epdc/Makefile |2 +- drivers/gpu/drm/mxc-epdc/epdc_hw.c |2 + drivers/gpu/drm/mxc-epdc/epdc_update.c | 1210 +++ drivers/gpu/drm/mxc-epdc/epdc_update.h |9 + drivers/gpu/drm/mxc-epdc/mxc_epdc.h | 50 + drivers/gpu/drm/mxc-epdc/mxc_epdc_drv.c | 33 +- 6 files changed, 1304 insertions(+), 2 deletions(-) create mode 100644 drivers/gpu/drm/mxc-epdc/epdc_update.c create mode 100644 drivers/gpu/drm/mxc-epdc/epdc_update.h diff --git a/drivers/gpu/drm/mxc-epdc/Makefile b/drivers/gpu/drm/mxc-epdc/Makefile index 0263ef2bf0db..a55e2bfe824a 100644 --- a/drivers/gpu/drm/mxc-epdc/Makefile +++ b/drivers/gpu/drm/mxc-epdc/Makefile @@ -1,5 +1,5 @@ # SPDX-License-Identifier: GPL-2.0 -mxc_epdc_drm-y := mxc_epdc_drv.o epdc_hw.o epdc_waveform.o +mxc_epdc_drm-y := mxc_epdc_drv.o epdc_hw.o epdc_update.o epdc_waveform.o obj-$(CONFIG_DRM_MXC_EPDC) += mxc_epdc_drm.o diff --git a/drivers/gpu/drm/mxc-epdc/epdc_hw.c b/drivers/gpu/drm/mxc-epdc/epdc_hw.c index a74cbd237e0d..22a065ac6992 100644 --- a/drivers/gpu/drm/mxc-epdc/epdc_hw.c +++ b/drivers/gpu/drm/mxc-epdc/epdc_hw.c @@ -20,6 +20,7 @@ #include "mxc_epdc.h" #include "epdc_regs.h" #include "epdc_hw.h" +#include "epdc_update.h" #include "epdc_waveform.h" void mxc_epdc_powerup(struct mxc_epdc *priv) @@ -410,6 +411,7 @@ void mxc_epdc_init_sequence(struct mxc_epdc *priv, struct drm_display_mode *m) priv->in_init = true; mxc_epdc_powerup(priv); + mxc_epdc_draw_mode0(priv); /* Force power down event */ priv->powering_down = true; mxc_epdc_powerdown(priv); diff --git a/drivers/gpu/drm/mxc-epdc/epdc_update.c b/drivers/gpu/drm/mxc-epdc/epdc_update.c new file mode 100644 index ..0c061982aa0b --- /dev/null +++ b/drivers/gpu/drm/mxc-epdc/epdc_update.c @@ -0,0 +1,1210 @@ +// SPDX-License-Identifier: GPL-2.0+ +// Copyright (C) 2022 Andreas Kemnade +// +/* + * based on the EPDC framebuffer driver + * Copyright (C) 2010-2016 Freescale Semiconductor, Inc. + * Copyright 2017 NXP + */ + +#include +#include +#include +#include "mxc_epdc.h" +#include "epdc_hw.h" +#include "epdc_regs.h" +#include "epdc_waveform.h" + +#define EPDC_V2_NUM_LUTS 64 +#define EPDC_V2_MAX_NUM_UPDATES 64 +#define INVALID_LUT (-1) +#define DRY_RUN_NO_LUT 100 + +#define MERGE_OK 0 +#define MERGE_FAIL 1 +#define MERGE_BLOCK 2 + +struct update_desc_list { + struct list_head list; + struct mxcfb_update_data upd_data;/* Update parameters */ + u32 update_order; /* Numeric ordering value for update */ +}; + +/* This structure represents a list node containing both + * a memory region allocated as an output buffer for the PxP + * update processing task, and the update description (mode, region, etc.) + */ +struct update_data_list { + struct list_head list; + struct update_desc_list *update_desc; + int lut_num;/* Assigned before update is processed into working buffer */ + u64 collision_mask; /* Set when update creates collision */ + /* Mask of the LUTs the update collides with */ +}; + +/ + * Start Low-Level EPDC Functions + / + +static inline void epdc_lut_complete_intr(struct mxc_epdc *priv, u32 lut_num, bool enable) +{ + if (enable) { + if (lut_num < 32) + epdc_write(priv, EPDC_IRQ_MASK1_SET, BIT(lut_num)); + else + epdc_write(priv, EPDC_IRQ_MASK2_SET, BIT(lut_num - 32)); + } else { + if (lut_num < 32) + epdc_write(priv, EPDC_IRQ_MASK1_CLEAR, BIT(lut_num)); + else + epdc_write(priv, EPDC_IRQ_MASK2_CLEAR, BIT(lut_num - 32)); + } +} + +static inline void epdc_working_buf_intr(struct mxc_epdc *priv, bool enable) +{ + if (enable) + epdc_write(priv, EPDC_IRQ_MASK_SET, EPDC_IRQ_WB_CMPLT_IRQ); + else + epdc_write(priv, EPDC_IRQ_MASK_CLEAR, EPDC_IRQ_WB_CMPLT_IRQ); +} + +static inline void epdc_clear_working_buf_irq(struct mxc_epdc *priv) +{ + epdc_write(priv, EPDC_IRQ_CLEAR, + EPDC_IRQ_WB_CMPLT_IRQ | EPDC_IRQ_LUT_COL_IRQ); +} + +static inline void epdc_eof_intr(struct
[RFC PATCH 5/6] ARM: dts: imx6sll: add EPDC
The commercial variant has a controller for e-Paper displays. Signed-off-by: Andreas Kemnade --- arch/arm/boot/dts/imx6sll.dtsi | 9 + 1 file changed, 9 insertions(+) diff --git a/arch/arm/boot/dts/imx6sll.dtsi b/arch/arm/boot/dts/imx6sll.dtsi index d4a000c3dde7..042e8a391b2f 100644 --- a/arch/arm/boot/dts/imx6sll.dtsi +++ b/arch/arm/boot/dts/imx6sll.dtsi @@ -643,6 +643,15 @@ pxp: pxp@20f { clock-names = "axi"; }; + epdc: epdc@20f4000 { + compatible = "fsl,imx6sll-epdc"; + reg = <0x020f4000 0x4000>; + interrupts = ; + clocks = < IMX6SLL_CLK_EPDC_AXI>, < IMX6SLL_CLK_EPDC_PIX>; + clock-names = "axi", "pix"; + status = "disabled"; + }; + lcdif: lcd-controller@20f8000 { compatible = "fsl,imx6sll-lcdif", "fsl,imx28-lcdif"; reg = <0x020f8000 0x4000>; -- 2.30.2
Re: [Letux-kernel] [PATCH 07/14] MIPS: DTS: CI20: fix PMU definitions for ACT8600
On Tue, 11 Feb 2020 22:41:24 +0100 "H. Nikolaus Schaller" wrote: > There is a ACT8600 on the CI20 board and the bindings of the > ACT8865 driver have changed without updating the CI20 device > tree. Therefore the PMU can not be probed successfully and > is running in power-on reset state. > > Fix DT to match the latest act8865-regulator bindings. > > Signed-off-by: H. Nikolaus Schaller hmm, what about a Fixes: tag here? Sounds like a regression. Regards, Andreas pgplvTYDTZnXy.pgp Description: OpenPGP digital signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFCv1 33/42] drm/omap: dsi: use atomic helper for dirtyfb
On Tue, 19 Nov 2019 17:55:57 +0200 Tomi Valkeinen wrote: > On 19/11/2019 17:06, Tony Lindgren wrote: > > >> The userspace apps need to do this. If they're using single-buffering, then > >> using the dirtyfb ioctl should do the trick, after the SGX has finished > >> drawing. > > > > Sounds like that's not happening. > > > > But why would the userspace app need to know this might be needed for > > a DSI command mode display and that it's not needed for HDMI? > > When double-buffering, the userspace doesn't need to care, as the > page-flip ioctl explicitly tells that the buffer is ready. > > When single buffering, either the userspace has to tell that the buffer > is now ready, or the kernel has to guess based on something. But afaics, > the latter is always a guess, and may not be a good guess. > > The kernel could trigger a delayed update based on, say, page fault if > drawing with CPU. But how much delayed... And with this scenario, we > would somehow need to find a way to catch the writes from any IP to the > buffer, and afaik there's no such thing. > > >> It's probably somewhat difficult if EGL is controlling the display, as, > >> afaik, SGX EGL is closed source, and that's probably where it should be > >> done. > >> > >> But adding back the hacky omap gem sync stuff, and then somehow guessing > >> from the sync finish that some display needs to be updated... It just does > >> not sound right to me. > > > > Right it's ugly. Still sounds like we need something in the kernel > > that knows "this is a DSI command mode LCD and needs to be updated". > well, if we look broader around and not only at the remaining displays to be converted from omapdrm to generic and look at more displays, there are also EPDs. > I think one option is to refresh the command mode display all the time. > Either using a timer, or trigger updates based on the previous update > being finished. > > Of course, that's kind of against the whole point of manual update > display, but then it should effectively behave like a conventional > always-updating display (except your HW is more expensive and consumes > more power than a conventional display). > And again as an extreme example about power consumption: EPDs. So I guess we need a generic interface for userspace-triggered refreshes anyways. And in that case that are only partly refreshes. Regards, Andreas ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4 1/2] dt-bindings: backlight: lm3630a: add enable_gpios
Hi, On Mon, 28 Oct 2019 18:25:56 -0500 Rob Herring wrote: > On Thu, Sep 12, 2019 at 4:33 PM Andreas Kemnade wrote: > > > > add enable-gpios to describe HWEN pin > > > > Signed-off-by: Andreas Kemnade > > Acked-by: Daniel Thompson > > This breaking linux-next now... > oops, sorry. > > --- > > changes in v2: added example > > changes in v3: added Acked-by > > changes in v4: moved enable-gpios to the right position > > in the example > > .../bindings/leds/backlight/lm3630a-backlight.yaml | 5 + > > 1 file changed, 5 insertions(+) > > > > diff --git > > a/Documentation/devicetree/bindings/leds/backlight/lm3630a-backlight.yaml > > b/Documentation/devicetree/bindings/leds/backlight/lm3630a-backlight.yaml > > index dc129d9a329e..c8470628fe02 100644 > > --- > > a/Documentation/devicetree/bindings/leds/backlight/lm3630a-backlight.yaml > > +++ > > b/Documentation/devicetree/bindings/leds/backlight/lm3630a-backlight.yaml > > @@ -29,6 +29,10 @@ properties: > >'#size-cells': > > const: 0 > > > > + enable-gpios: > > +description: GPIO to use to enable/disable the backlight (HWEN pin). > > +maxItems: 1 > > + > > required: > >- compatible > >- reg > > @@ -96,6 +100,7 @@ examples: > > led-controller@38 { > > compatible = "ti,lm3630a"; > > reg = <0x38>; > > +enable-gpios = < 5 GPIO_ACTIVE_HIGH>; > > Error: > Documentation/devicetree/bindings/leds/backlight/lm3630a-backlight.example.dts:24.46-47 > syntax error > FATAL ERROR: Unable to parse input tree > make[1]: *** > [Documentation/devicetree/bindings/leds/backlight/lm3630a-backlight.example.dt.yaml] > Error 1 > scripts/Makefile.lib:314: recipe for target > 'Documentation/devicetree/bindings/leds/backlight/lm3630a-backlight.example.dt.yaml' > failed > > You need the include for the define. gpio/gpio.h is missing. Yes, was not aware of that these things will be compiled and the automatic check did not work on my system at all. So I decided not to fix that for just this simple thing which was a not so good idea. Will send a fixup. Regards, Andreas pgpK_PlxUAPL4.pgp Description: OpenPGP digital signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH next] dt-bindings: backlight: lm3630a: fix missing include
example failed to compile due to undefined GPIO_ACTIVE_HIGH fix that by adding the needed #include to the exammple Fixes: ae92365cdd75 ("dt-bindings: backlight: lm3630a: Add enable-gpios to describe HWEN pin") Signed-off-by: Andreas Kemnade --- .../devicetree/bindings/leds/backlight/lm3630a-backlight.yaml| 1 + 1 file changed, 1 insertion(+) diff --git a/Documentation/devicetree/bindings/leds/backlight/lm3630a-backlight.yaml b/Documentation/devicetree/bindings/leds/backlight/lm3630a-backlight.yaml index c8470628fe02..08fe5cf8614a 100644 --- a/Documentation/devicetree/bindings/leds/backlight/lm3630a-backlight.yaml +++ b/Documentation/devicetree/bindings/leds/backlight/lm3630a-backlight.yaml @@ -93,6 +93,7 @@ additionalProperties: false examples: - | +#include i2c { #address-cells = <1>; #size-cells = <0>; -- 2.20.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Letux-kernel] [PATCH 4/5] drm/panel: tpo-td028ttec1: Fix SPI alias
On Mon, 7 Oct 2019 20:08:00 +0300 Laurent Pinchart wrote: > The panel-tpo-td028ttec1 driver incorrectly includes the OF vendor > prefix in its SPI alias. Fix it. > > Fixes: 415b8dd08711 ("drm/panel: Add driver for the Toppoly TD028TTEC1 panel") > Reported-by: H. Nikolaus Schaller > Signed-off-by: Laurent Pinchart Tested-by: Andreas Kemnade > --- > drivers/gpu/drm/panel/panel-tpo-td028ttec1.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/panel/panel-tpo-td028ttec1.c > b/drivers/gpu/drm/panel/panel-tpo-td028ttec1.c > index d7b2e34626ef..f2baff827f50 100644 > --- a/drivers/gpu/drm/panel/panel-tpo-td028ttec1.c > +++ b/drivers/gpu/drm/panel/panel-tpo-td028ttec1.c > @@ -375,8 +375,7 @@ static const struct of_device_id td028ttec1_of_match[] = { > MODULE_DEVICE_TABLE(of, td028ttec1_of_match); > > static const struct spi_device_id td028ttec1_ids[] = { > - { "tpo,td028ttec1", 0}, > - { "toppoly,td028ttec1", 0 }, > + { "td028ttec1", 0 }, > { /* sentinel */ } > }; > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm: panels: fix spi aliases of former omap panels
On Mon, 7 Oct 2019 19:04:46 +0200 Sebastian Reichel wrote: > Hi, > > On Mon, Oct 07, 2019 at 06:41:30PM +0200, Andreas Kemnade wrote: > > When the panels were moved from omap/displays/ to panel/ > > omapdss prefix was stripped, which cause spi modalias > > to not contain the vendor-prefix anymore. > > > > so we had e.g. in former times: > > compatible=omapdss,tpo,td028ttec1 -> modalias=spi:tpo,td028ttec1 > > now: > > compatible=tpo,td028ttec1 -> modalias=spi:td028ttec1 > > > > This is consistent with other drivers. Tested the td028ttec.c > > only, but the pattern looks the same for the other ones. > > > > Fixes: 45f16c82db7e8 ("drm/omap: displays: Remove unused panel drivers") > > Signed-off-by: Andreas Kemnade > > --- > > Patch looks good to me, but you have one false positive. > > > [...] > > > > diff --git a/drivers/gpu/drm/panel/panel-sharp-ls037v7dw01.c > > b/drivers/gpu/drm/panel/panel-sharp-ls037v7dw01.c > > index 46cd9a2501298..838d39a263f53 100644 > > --- a/drivers/gpu/drm/panel/panel-sharp-ls037v7dw01.c > > +++ b/drivers/gpu/drm/panel/panel-sharp-ls037v7dw01.c > > @@ -204,7 +204,7 @@ static int ls037v7dw01_remove(struct platform_device > > *pdev) > > } > > > > static const struct of_device_id ls037v7dw01_of_match[] = { > > - { .compatible = "sharp,ls037v7dw01", }, > > + { .compatible = "ls037v7dw01", }, > > { /* sentinel */ }, > > }; > > > > The DT compatible should have a vendor prefix. > oops, sorry, but I it seems that Laurent already has submitted a fix. Regards, Andreas ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm: panels: fix spi aliases of former omap panels
When the panels were moved from omap/displays/ to panel/ omapdss prefix was stripped, which cause spi modalias to not contain the vendor-prefix anymore. so we had e.g. in former times: compatible=omapdss,tpo,td028ttec1 -> modalias=spi:tpo,td028ttec1 now: compatible=tpo,td028ttec1 -> modalias=spi:td028ttec1 This is consistent with other drivers. Tested the td028ttec.c only, but the pattern looks the same for the other ones. Fixes: 45f16c82db7e8 ("drm/omap: displays: Remove unused panel drivers") Signed-off-by: Andreas Kemnade --- drivers/gpu/drm/panel/panel-lg-lb035q02.c | 2 +- drivers/gpu/drm/panel/panel-nec-nl8048hl11.c| 2 +- drivers/gpu/drm/panel/panel-sharp-ls037v7dw01.c | 2 +- drivers/gpu/drm/panel/panel-sony-acx565akm.c| 2 +- drivers/gpu/drm/panel/panel-tpo-td028ttec1.c| 3 +-- drivers/gpu/drm/panel/panel-tpo-td043mtea1.c| 2 +- 6 files changed, 6 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/panel/panel-lg-lb035q02.c b/drivers/gpu/drm/panel/panel-lg-lb035q02.c index fc82a525b071b..8423684a0557e 100644 --- a/drivers/gpu/drm/panel/panel-lg-lb035q02.c +++ b/drivers/gpu/drm/panel/panel-lg-lb035q02.c @@ -231,7 +231,7 @@ static struct spi_driver lb035q02_driver = { module_spi_driver(lb035q02_driver); -MODULE_ALIAS("spi:lgphilips,lb035q02"); +MODULE_ALIAS("spi:lb035q02"); MODULE_AUTHOR("Tomi Valkeinen "); MODULE_DESCRIPTION("LG.Philips LB035Q02 LCD Panel driver"); MODULE_LICENSE("GPL"); diff --git a/drivers/gpu/drm/panel/panel-nec-nl8048hl11.c b/drivers/gpu/drm/panel/panel-nec-nl8048hl11.c index 299b217c83e18..71b07ef2a62dd 100644 --- a/drivers/gpu/drm/panel/panel-nec-nl8048hl11.c +++ b/drivers/gpu/drm/panel/panel-nec-nl8048hl11.c @@ -242,7 +242,7 @@ static struct spi_driver nl8048_driver = { module_spi_driver(nl8048_driver); -MODULE_ALIAS("spi:nec,nl8048hl11"); +MODULE_ALIAS("spi:nl8048hl11"); MODULE_AUTHOR("Erik Gilling "); MODULE_DESCRIPTION("NEC-NL8048HL11 Driver"); MODULE_LICENSE("GPL"); diff --git a/drivers/gpu/drm/panel/panel-sharp-ls037v7dw01.c b/drivers/gpu/drm/panel/panel-sharp-ls037v7dw01.c index 46cd9a2501298..838d39a263f53 100644 --- a/drivers/gpu/drm/panel/panel-sharp-ls037v7dw01.c +++ b/drivers/gpu/drm/panel/panel-sharp-ls037v7dw01.c @@ -204,7 +204,7 @@ static int ls037v7dw01_remove(struct platform_device *pdev) } static const struct of_device_id ls037v7dw01_of_match[] = { - { .compatible = "sharp,ls037v7dw01", }, + { .compatible = "ls037v7dw01", }, { /* sentinel */ }, }; diff --git a/drivers/gpu/drm/panel/panel-sony-acx565akm.c b/drivers/gpu/drm/panel/panel-sony-acx565akm.c index 305259b587670..a8af9340f89ee 100644 --- a/drivers/gpu/drm/panel/panel-sony-acx565akm.c +++ b/drivers/gpu/drm/panel/panel-sony-acx565akm.c @@ -695,7 +695,7 @@ static struct spi_driver acx565akm_driver = { module_spi_driver(acx565akm_driver); -MODULE_ALIAS("spi:sony,acx565akm"); +MODULE_ALIAS("spi:acx565akm"); MODULE_AUTHOR("Nokia Corporation"); MODULE_DESCRIPTION("Sony ACX565AKM LCD Panel Driver"); MODULE_LICENSE("GPL"); diff --git a/drivers/gpu/drm/panel/panel-tpo-td028ttec1.c b/drivers/gpu/drm/panel/panel-tpo-td028ttec1.c index d7b2e34626efe..3ab66b4d3ea27 100644 --- a/drivers/gpu/drm/panel/panel-tpo-td028ttec1.c +++ b/drivers/gpu/drm/panel/panel-tpo-td028ttec1.c @@ -375,8 +375,7 @@ static const struct of_device_id td028ttec1_of_match[] = { MODULE_DEVICE_TABLE(of, td028ttec1_of_match); static const struct spi_device_id td028ttec1_ids[] = { - { "tpo,td028ttec1", 0}, - { "toppoly,td028ttec1", 0 }, + { "td028ttec1", 0}, { /* sentinel */ } }; diff --git a/drivers/gpu/drm/panel/panel-tpo-td043mtea1.c b/drivers/gpu/drm/panel/panel-tpo-td043mtea1.c index 84370562910ff..7e6cf0890600c 100644 --- a/drivers/gpu/drm/panel/panel-tpo-td043mtea1.c +++ b/drivers/gpu/drm/panel/panel-tpo-td043mtea1.c @@ -503,7 +503,7 @@ static struct spi_driver td043mtea1_driver = { module_spi_driver(td043mtea1_driver); -MODULE_ALIAS("spi:tpo,td043mtea1"); +MODULE_ALIAS("spi:td043mtea1"); MODULE_AUTHOR("Gražvydas Ignotas "); MODULE_DESCRIPTION("TPO TD043MTEA1 Panel Driver"); MODULE_LICENSE("GPL"); -- 2.20.1
Re: [PATCH] backlight: lm3630a: fix module aliases
Hi, hmm, this bugfix has not arrived in -next or any -fixes branch I am aware of yet. I hope it is not forgotten... so a friendly ping. Regards, Andreas On Wed, 11 Sep 2019 11:51:48 +0100 Daniel Thompson wrote: > On Tue, Sep 10, 2019 at 05:23:59PM +0200, Andreas Kemnade wrote: > > Devicetree aliases are missing, so that module autoloading > > does not work properly. > > > > Signed-off-by: Andreas Kemnade > > Reviewed-by: Daniel Thompson > > > > --- > > drivers/video/backlight/lm3630a_bl.c | 4 +++- > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/video/backlight/lm3630a_bl.c > > b/drivers/video/backlight/lm3630a_bl.c > > index 3b45a1733198..9d67c07db2f2 100644 > > --- a/drivers/video/backlight/lm3630a_bl.c > > +++ b/drivers/video/backlight/lm3630a_bl.c > > @@ -617,12 +617,14 @@ static const struct i2c_device_id lm3630a_id[] = { > > {} > > }; > > > > +MODULE_DEVICE_TABLE(i2c, lm3630a_id); > > + > > static const struct of_device_id lm3630a_match_table[] = { > > { .compatible = "ti,lm3630a", }, > > { }, > > }; > > > > -MODULE_DEVICE_TABLE(i2c, lm3630a_id); > > +MODULE_DEVICE_TABLE(of, lm3630a_match_table); > > > > static struct i2c_driver lm3630a_i2c_driver = { > > .driver = { > > -- > > 2.11.0 > > >
Re: [PATCH 1/2] backlight: lm3630a: add an enable gpio for the HWEN pin
Hi, On Sun, 15 Sep 2019 18:52:04 +0200 Pavel Machek wrote: > Hi! > > > > > > Is this needed? > > > > > > > > > > This is a remove path, not a power management path, and we have no > > > > > idea > > > > > what the original status of the pin was anyway? > > > > > > > > > > > > > Looking at Ishdn on page 5 of the datasheet, switching it off everytime > > > > possible seems not needed. We would need to call chip_init() everytime > > > > we enable the gpio or live with default values. > > > > Therefore I did decide to not put it into any power management path. > > > > But switching it on and not switching it off feels so unbalanced. > > > > > > Either the power consumed by the controller when strings aren't lit up > > > matters, in which case the driver should implement proper power > > > management or it doesn't matter and changing the pin state isn't needed. > > > > > > I'm happy with either of the above but this looks like a third way, > > > where eager users could hack in a bit of extra power management by > > > forcing drivers to unbind. > > > > > I think I will take the simple way. I am quite sure that the power > > consumption with HWEN on and leds off does not matter. If someone > > later comes up and finds out that I misread the datasheet, things > > are prepared to be improved. > > Dunno.. if the power consumption does not matter, why does the chip have the > enable > pin in the first place, and why do we bother supporting it? We could hardcode > the > pin to enabled as well.. Well, I agree having the pin and no power saving seems not to make sense. Two points here: I think it is a good idea to properly describe the hardware in the devicetree. What to do with that information is another thing. A problem is that at the moment I cannot easily measure consumption of the chip. Hmm, even testing a solution which disables the pin while the chip is not in use, is not so easy. But wait... I could use a wrong gpio but one that I can easily monitor to check if the pin is toggled. And set the real pin to high by some other means. And then use the real gpio to check if timings are correct (waiting enough after enabling the chip, e.g. Regards, Andreas
[PATCH v4 2/2] backlight: lm3630a: add an enable gpio for the HWEN pin
For now just enable it in the probe function to allow i2c access. Disabling also means resetting the register values to default and according to the datasheet does not give power savings. Tested on Kobo Clara HD. Signed-off-by: Andreas Kemnade Reviewed-by: Dan Murphy Reviewed-by: Daniel Thompson --- changes in v2: - simplification - correct gpio direction initialisation changes in v3: - removed legacy include drivers/video/backlight/lm3630a_bl.c | 9 + 1 file changed, 9 insertions(+) diff --git a/drivers/video/backlight/lm3630a_bl.c b/drivers/video/backlight/lm3630a_bl.c index 8f84f3684f04..d9e67b9b2571 100644 --- a/drivers/video/backlight/lm3630a_bl.c +++ b/drivers/video/backlight/lm3630a_bl.c @@ -12,6 +12,7 @@ #include #include #include +#include #include #include @@ -48,6 +49,7 @@ struct lm3630a_chip { struct lm3630a_platform_data *pdata; struct backlight_device *bleda; struct backlight_device *bledb; + struct gpio_desc *enable_gpio; struct regmap *regmap; struct pwm_device *pwmd; }; @@ -535,6 +537,13 @@ static int lm3630a_probe(struct i2c_client *client, } pchip->pdata = pdata; + pchip->enable_gpio = devm_gpiod_get_optional(>dev, "enable", + GPIOD_OUT_HIGH); + if (IS_ERR(pchip->enable_gpio)) { + rval = PTR_ERR(pchip->enable_gpio); + return rval; + } + /* chip initialize */ rval = lm3630a_chip_init(pchip); if (rval < 0) { -- 2.20.1
[PATCH v4 0/2] backlight_lm3630a: add enable_gpios property
To be able to handle the HWEN pin of the lm3630a, add an enable gpio to the driver and a property. Tested on Kobo Clara HD. Changes in v2: simplification and reordering Changes in v3: added acked-by removed legacy include Changes in v4: added reviewed-by moved gpio to the right position in the bindings example Andreas Kemnade (2): dt-bindings: backlight: lm3630a: add enable_gpios backlight: lm3630a: add an enable gpio for the HWEN pin .../bindings/leds/backlight/lm3630a-backlight.yaml | 5 + drivers/video/backlight/lm3630a_bl.c | 9 + 2 files changed, 14 insertions(+) -- 2.20.1
[PATCH v4 1/2] dt-bindings: backlight: lm3630a: add enable_gpios
add enable-gpios to describe HWEN pin Signed-off-by: Andreas Kemnade Acked-by: Daniel Thompson --- changes in v2: added example changes in v3: added Acked-by changes in v4: moved enable-gpios to the right position in the example .../bindings/leds/backlight/lm3630a-backlight.yaml | 5 + 1 file changed, 5 insertions(+) diff --git a/Documentation/devicetree/bindings/leds/backlight/lm3630a-backlight.yaml b/Documentation/devicetree/bindings/leds/backlight/lm3630a-backlight.yaml index dc129d9a329e..c8470628fe02 100644 --- a/Documentation/devicetree/bindings/leds/backlight/lm3630a-backlight.yaml +++ b/Documentation/devicetree/bindings/leds/backlight/lm3630a-backlight.yaml @@ -29,6 +29,10 @@ properties: '#size-cells': const: 0 + enable-gpios: +description: GPIO to use to enable/disable the backlight (HWEN pin). +maxItems: 1 + required: - compatible - reg @@ -96,6 +100,7 @@ examples: led-controller@38 { compatible = "ti,lm3630a"; reg = <0x38>; +enable-gpios = < 5 GPIO_ACTIVE_HIGH>; #address-cells = <1>; #size-cells = <0>; -- 2.20.1
Re: [PATCH v3 1/2] dt-bindings: backlight: lm3630a: add enable_gpios
On Thu, 12 Sep 2019 06:39:50 -0500 Dan Murphy wrote: > Andreas > > On 9/11/19 12:21 PM, Andreas Kemnade wrote: > > add enable-gpios to describe HWEN pin > > > > Signed-off-by: Andreas Kemnade > > Acked-by: Daniel Thompson > > --- > > changes in v2: added example > > changes in v3: added Acked-by > > .../bindings/leds/backlight/lm3630a-backlight.yaml | 5 + > > 1 file changed, 5 insertions(+) > > > > diff --git > > a/Documentation/devicetree/bindings/leds/backlight/lm3630a-backlight.yaml > > b/Documentation/devicetree/bindings/leds/backlight/lm3630a-backlight.yaml > > index dc129d9a329e..1fa83feffe16 100644 > > --- > > a/Documentation/devicetree/bindings/leds/backlight/lm3630a-backlight.yaml > > +++ > > b/Documentation/devicetree/bindings/leds/backlight/lm3630a-backlight.yaml > > @@ -29,6 +29,10 @@ properties: > > '#size-cells': > > const: 0 > > > > + enable-gpios: > > +description: GPIO to use to enable/disable the backlight (HWEN pin). > > +maxItems: 1 > > + > > required: > > - compatible > > - reg > > @@ -92,6 +96,7 @@ examples: > > i2c { > > #address-cells = <1>; > > #size-cells = <0>; > > +enable-gpios = < 5 GPIO_ACTIVE_HIGH>; > > > > led-controller@38 { > > compatible = "ti,lm3630a"; > > Looks good to me > well, the enable-gpios is still at the same place as in v2. This was sent before your comments to v2 have been arrived. Regards, Andreas pgpCpMwJJpC6G.pgp Description: OpenPGP digital signature
Re: [PATCH v2 2/2] backlight: lm3630a: add an enable gpio for the HWEN pin
On Wed, 11 Sep 2019 13:48:36 -0500 Dan Murphy wrote: > >> @@ -535,6 +538,13 @@ static int lm3630a_probe(struct i2c_client *client, > >>} > >>pchip->pdata = pdata; > >> > >> + pchip->enable_gpio = devm_gpiod_get_optional(>dev, "enable", > >> + GPIOD_OUT_HIGH); > >> + if (IS_ERR(pchip->enable_gpio)) { > >> + rval = PTR_ERR(pchip->enable_gpio); > >> + return rval; > > the enable gpio is optional so if it fails you log the error and move on > well, if the gpio is not there, then it returns NULL. It might return e.g. -EDEFER. So I need to check for errors here. > Also on driver removal did you want to set the GPIO to low to disable > the device to save power? > page 5 of the datasheet says: Ishdn = Typ. 1µA max. 4µA. For HWEN=Vin, I2c shutdown (I guess this means outputs powered off) ond for HWEN=GND. So are we really saving something here? Regards, Andreas
[PATCH v3 2/2] backlight: lm3630a: add an enable gpio for the HWEN pin
For now just enable it in the probe function to allow i2c access. Disabling also means resetting the register values to default and according to the datasheet does not give power savings. Tested on Kobo Clara HD. Signed-off-by: Andreas Kemnade --- changes in v2: - simplification - correct gpio direction initialisation changes in v3: - removed legacy include drivers/video/backlight/lm3630a_bl.c | 9 + 1 file changed, 9 insertions(+) diff --git a/drivers/video/backlight/lm3630a_bl.c b/drivers/video/backlight/lm3630a_bl.c index 8f84f3684f04..d9e67b9b2571 100644 --- a/drivers/video/backlight/lm3630a_bl.c +++ b/drivers/video/backlight/lm3630a_bl.c @@ -12,6 +12,7 @@ #include #include #include +#include #include #include @@ -48,6 +49,7 @@ struct lm3630a_chip { struct lm3630a_platform_data *pdata; struct backlight_device *bleda; struct backlight_device *bledb; + struct gpio_desc *enable_gpio; struct regmap *regmap; struct pwm_device *pwmd; }; @@ -535,6 +537,13 @@ static int lm3630a_probe(struct i2c_client *client, } pchip->pdata = pdata; + pchip->enable_gpio = devm_gpiod_get_optional(>dev, "enable", + GPIOD_OUT_HIGH); + if (IS_ERR(pchip->enable_gpio)) { + rval = PTR_ERR(pchip->enable_gpio); + return rval; + } + /* chip initialize */ rval = lm3630a_chip_init(pchip); if (rval < 0) { -- 2.20.1
[PATCH v3 1/2] dt-bindings: backlight: lm3630a: add enable_gpios
add enable-gpios to describe HWEN pin Signed-off-by: Andreas Kemnade Acked-by: Daniel Thompson --- changes in v2: added example changes in v3: added Acked-by .../bindings/leds/backlight/lm3630a-backlight.yaml | 5 + 1 file changed, 5 insertions(+) diff --git a/Documentation/devicetree/bindings/leds/backlight/lm3630a-backlight.yaml b/Documentation/devicetree/bindings/leds/backlight/lm3630a-backlight.yaml index dc129d9a329e..1fa83feffe16 100644 --- a/Documentation/devicetree/bindings/leds/backlight/lm3630a-backlight.yaml +++ b/Documentation/devicetree/bindings/leds/backlight/lm3630a-backlight.yaml @@ -29,6 +29,10 @@ properties: '#size-cells': const: 0 + enable-gpios: +description: GPIO to use to enable/disable the backlight (HWEN pin). +maxItems: 1 + required: - compatible - reg @@ -92,6 +96,7 @@ examples: i2c { #address-cells = <1>; #size-cells = <0>; +enable-gpios = < 5 GPIO_ACTIVE_HIGH>; led-controller@38 { compatible = "ti,lm3630a"; -- 2.20.1
[PATCH v3 0/2] backlight_lm3630a: add enable_gpios property
To be able to handle the HWEN pin of the lm3630a, add an enable gpio to the driver and a property. Tested on Kobo Clara HD. Changes in v2: simplification and reordering Changes in v3: added acked-by removed legacy include Andreas Kemnade (2): dt-bindings: backlight: lm3630a: add enable_gpios backlight: lm3630a: add an enable gpio for the HWEN pin .../bindings/leds/backlight/lm3630a-backlight.yaml | 5 + drivers/video/backlight/lm3630a_bl.c | 9 + 2 files changed, 14 insertions(+) -- 2.20.1
[PATCH v2 2/2] backlight: lm3630a: add an enable gpio for the HWEN pin
For now just enable it in the probe function to allow i2c access. Disabling also means resetting the register values to default and according to the datasheet does not give power savings Tested on Kobo Clara HD. Signed-off-by: Andreas Kemnade --- changes in v2: - simplification - correct gpio direction initialisation drivers/video/backlight/lm3630a_bl.c | 10 ++ 1 file changed, 10 insertions(+) diff --git a/drivers/video/backlight/lm3630a_bl.c b/drivers/video/backlight/lm3630a_bl.c index 8f84f3684f04..9d0639d4202d 100644 --- a/drivers/video/backlight/lm3630a_bl.c +++ b/drivers/video/backlight/lm3630a_bl.c @@ -12,6 +12,8 @@ #include #include #include +#include +#include #include #include @@ -48,6 +50,7 @@ struct lm3630a_chip { struct lm3630a_platform_data *pdata; struct backlight_device *bleda; struct backlight_device *bledb; + struct gpio_desc *enable_gpio; struct regmap *regmap; struct pwm_device *pwmd; }; @@ -535,6 +538,13 @@ static int lm3630a_probe(struct i2c_client *client, } pchip->pdata = pdata; + pchip->enable_gpio = devm_gpiod_get_optional(>dev, "enable", + GPIOD_OUT_HIGH); + if (IS_ERR(pchip->enable_gpio)) { + rval = PTR_ERR(pchip->enable_gpio); + return rval; + } + /* chip initialize */ rval = lm3630a_chip_init(pchip); if (rval < 0) { -- 2.20.1
[PATCH v2 0/2] backlight_lm3630a: add enable_gpios property
To be able to handle the HWEN pin of the lm3630a, add an enable gpio to the driver and a property. Tested on Kobo Clara HD. Changes in v2: simplification and reordering Andreas Kemnade (2): dt-bindings: backlight: lm3630a: add enable_gpios backlight: lm3630a: add an enable gpio for the HWEN pin .../bindings/leds/backlight/lm3630a-backlight.yaml | 5 + drivers/video/backlight/lm3630a_bl.c | 10 ++ 2 files changed, 15 insertions(+) -- 2.20.1
[PATCH v2 1/2] dt-bindings: backlight: lm3630a: add enable_gpios
add enable-gpios to describe HWEN pin Signed-off-by: Andreas Kemnade --- changes in v2: add example .../bindings/leds/backlight/lm3630a-backlight.yaml | 5 + 1 file changed, 5 insertions(+) diff --git a/Documentation/devicetree/bindings/leds/backlight/lm3630a-backlight.yaml b/Documentation/devicetree/bindings/leds/backlight/lm3630a-backlight.yaml index dc129d9a329e..1fa83feffe16 100644 --- a/Documentation/devicetree/bindings/leds/backlight/lm3630a-backlight.yaml +++ b/Documentation/devicetree/bindings/leds/backlight/lm3630a-backlight.yaml @@ -29,6 +29,10 @@ properties: '#size-cells': const: 0 + enable-gpios: +description: GPIO to use to enable/disable the backlight (HWEN pin). +maxItems: 1 + required: - compatible - reg @@ -92,6 +96,7 @@ examples: i2c { #address-cells = <1>; #size-cells = <0>; +enable-gpios = < 5 GPIO_ACTIVE_HIGH>; led-controller@38 { compatible = "ti,lm3630a"; -- 2.20.1
Re: [PATCH 1/2] backlight: lm3630a: add an enable gpio for the HWEN pin
On Tue, 10 Sep 2019 11:21:56 +0100 Daniel Thompson wrote: [...] > > > Is this needed? > > > > > > This is a remove path, not a power management path, and we have no idea > > > what the original status of the pin was anyway? > > > > > > > Looking at Ishdn on page 5 of the datasheet, switching it off everytime > > possible seems not needed. We would need to call chip_init() everytime > > we enable the gpio or live with default values. > > Therefore I did decide to not put it into any power management path. > > But switching it on and not switching it off feels so unbalanced. > > Either the power consumed by the controller when strings aren't lit up > matters, in which case the driver should implement proper power > management or it doesn't matter and changing the pin state isn't needed. > > I'm happy with either of the above but this looks like a third way, > where eager users could hack in a bit of extra power management by > forcing drivers to unbind. > I think I will take the simple way. I am quite sure that the power consumption with HWEN on and leds off does not matter. If someone later comes up and finds out that I misread the datasheet, things are prepared to be improved. At least the hardware can be properly described in the devicetree. Regards, Andreas pgpvJw6rr7Ej0.pgp Description: OpenPGP digital signature
[PATCH] backlight: lm3630a: fix module aliases
Devicetree aliases are missing, so that module autoloading does not work properly. Signed-off-by: Andreas Kemnade --- drivers/video/backlight/lm3630a_bl.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/video/backlight/lm3630a_bl.c b/drivers/video/backlight/lm3630a_bl.c index 3b45a1733198..9d67c07db2f2 100644 --- a/drivers/video/backlight/lm3630a_bl.c +++ b/drivers/video/backlight/lm3630a_bl.c @@ -617,12 +617,14 @@ static const struct i2c_device_id lm3630a_id[] = { {} }; +MODULE_DEVICE_TABLE(i2c, lm3630a_id); + static const struct of_device_id lm3630a_match_table[] = { { .compatible = "ti,lm3630a", }, { }, }; -MODULE_DEVICE_TABLE(i2c, lm3630a_id); +MODULE_DEVICE_TABLE(of, lm3630a_match_table); static struct i2c_driver lm3630a_i2c_driver = { .driver = { -- 2.11.0
Re: [PATCH 1/2] backlight: lm3630a: add an enable gpio for the HWEN pin
On Mon, 9 Sep 2019 11:57:29 +0100 Daniel Thompson wrote: > On Sun, Sep 08, 2019 at 10:37:03PM +0200, Andreas Kemnade wrote: > > For now just enable it in the probe function to allow i2c > > access and disable it on remove. Disabling also means resetting > > the register values to default. > > > > Tested on Kobo Clara HD. > > > > Signed-off-by: Andreas Kemnade > > --- > > drivers/video/backlight/lm3630a_bl.c | 18 ++ > > 1 file changed, 18 insertions(+) > > > > diff --git a/drivers/video/backlight/lm3630a_bl.c > > b/drivers/video/backlight/lm3630a_bl.c > > index b04b35d007a2..3b45a1733198 100644 > > --- a/drivers/video/backlight/lm3630a_bl.c > > +++ b/drivers/video/backlight/lm3630a_bl.c > > @@ -12,6 +12,8 @@ > > #include > > #include > > #include > > +#include > > +#include > > #include > > #include > > > > @@ -48,6 +50,7 @@ struct lm3630a_chip { > > struct lm3630a_platform_data *pdata; > > struct backlight_device *bleda; > > struct backlight_device *bledb; > > + struct gpio_desc *enable_gpio; > > struct regmap *regmap; > > struct pwm_device *pwmd; > > }; > > @@ -506,6 +509,14 @@ static int lm3630a_probe(struct i2c_client *client, > > return -ENOMEM; > > pchip->dev = >dev; > > > > + pchip->enable_gpio = devm_gpiod_get_optional(>dev, "enable", > > + GPIOD_ASIS); > > Initializing GPIOD_ASIS doesn't look right to me. > > If you initialize ASIS then the driver must configure the pin as an > output... far easier just to set GPIOD_OUT_HIGH during the get. > > Note also that the call to this function should also be moved *below* > the calls parse the DT. > oops, must have forgotten that, and had good luck here. > > > + if (IS_ERR(pchip->enable_gpio)) { > > + rval = PTR_ERR(pchip->enable_gpio); > > + return rval; > > + } > > + > > + > > pchip->regmap = devm_regmap_init_i2c(client, _regmap); > > if (IS_ERR(pchip->regmap)) { > > rval = PTR_ERR(pchip->regmap); > > @@ -535,6 +546,10 @@ static int lm3630a_probe(struct i2c_client *client, > > } > > pchip->pdata = pdata; > > > > + if (pchip->enable_gpio) { > > + gpiod_set_value_cansleep(pchip->enable_gpio, 1); > > Not needed, use GPIOD_OUT_HIGH instead. > > > > + usleep_range(1000, 2000); > > Not needed, this sleep is already part of lm3630a_chip_init(). > you are right. > > > + } > > /* chip initialize */ > > rval = lm3630a_chip_init(pchip); > > if (rval < 0) { > > @@ -586,6 +601,9 @@ static int lm3630a_remove(struct i2c_client *client) > > if (rval < 0) > > dev_err(pchip->dev, "i2c failed to access register\n"); > > > > + if (pchip->enable_gpio) > > + gpiod_set_value_cansleep(pchip->enable_gpio, 0); > > + > > Is this needed? > > This is a remove path, not a power management path, and we have no idea > what the original status of the pin was anyway? > Looking at Ishdn on page 5 of the datasheet, switching it off everytime possible seems not needed. We would need to call chip_init() everytime we enable the gpio or live with default values. Therefore I did decide to not put it into any power management path. But switching it on and not switching it off feels so unbalanced. Regards, Andreas
[PATCH 0/2] backlight_lm3630a: add enable_gpios property
To be able to handle the HWEN pin of the lm3630a, add an enable gpio to the driver and a property. Tested on Kobo Clara HD Andreas Kemnade (2): backlight: lm3630a: add an enable gpio for the HWEN pin dt-bindings: backlight: lm3630a: add enable_gpios .../leds/backlight/lm3630a-backlight.yaml | 4 drivers/video/backlight/lm3630a_bl.c | 18 ++ 2 files changed, 22 insertions(+) -- 2.20.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/2] dt-bindings: backlight: lm3630a: add enable_gpios
add enable-gpios to describe HWEN pin Signed-off-by: Andreas Kemnade --- .../devicetree/bindings/leds/backlight/lm3630a-backlight.yaml | 4 1 file changed, 4 insertions(+) diff --git a/Documentation/devicetree/bindings/leds/backlight/lm3630a-backlight.yaml b/Documentation/devicetree/bindings/leds/backlight/lm3630a-backlight.yaml index dc129d9a329e..a9656d72b668 100644 --- a/Documentation/devicetree/bindings/leds/backlight/lm3630a-backlight.yaml +++ b/Documentation/devicetree/bindings/leds/backlight/lm3630a-backlight.yaml @@ -29,6 +29,10 @@ properties: '#size-cells': const: 0 + enable-gpios: +description: GPIO to use to enable/disable the backlight (HWEN pin). +maxItems: 1 + required: - compatible - reg -- 2.20.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/2] backlight: lm3630a: add an enable gpio for the HWEN pin
For now just enable it in the probe function to allow i2c access and disable it on remove. Disabling also means resetting the register values to default. Tested on Kobo Clara HD. Signed-off-by: Andreas Kemnade --- drivers/video/backlight/lm3630a_bl.c | 18 ++ 1 file changed, 18 insertions(+) diff --git a/drivers/video/backlight/lm3630a_bl.c b/drivers/video/backlight/lm3630a_bl.c index b04b35d007a2..3b45a1733198 100644 --- a/drivers/video/backlight/lm3630a_bl.c +++ b/drivers/video/backlight/lm3630a_bl.c @@ -12,6 +12,8 @@ #include #include #include +#include +#include #include #include @@ -48,6 +50,7 @@ struct lm3630a_chip { struct lm3630a_platform_data *pdata; struct backlight_device *bleda; struct backlight_device *bledb; + struct gpio_desc *enable_gpio; struct regmap *regmap; struct pwm_device *pwmd; }; @@ -506,6 +509,14 @@ static int lm3630a_probe(struct i2c_client *client, return -ENOMEM; pchip->dev = >dev; + pchip->enable_gpio = devm_gpiod_get_optional(>dev, "enable", + GPIOD_ASIS); + if (IS_ERR(pchip->enable_gpio)) { + rval = PTR_ERR(pchip->enable_gpio); + return rval; + } + + pchip->regmap = devm_regmap_init_i2c(client, _regmap); if (IS_ERR(pchip->regmap)) { rval = PTR_ERR(pchip->regmap); @@ -535,6 +546,10 @@ static int lm3630a_probe(struct i2c_client *client, } pchip->pdata = pdata; + if (pchip->enable_gpio) { + gpiod_set_value_cansleep(pchip->enable_gpio, 1); + usleep_range(1000, 2000); + } /* chip initialize */ rval = lm3630a_chip_init(pchip); if (rval < 0) { @@ -586,6 +601,9 @@ static int lm3630a_remove(struct i2c_client *client) if (rval < 0) dev_err(pchip->dev, "i2c failed to access register\n"); + if (pchip->enable_gpio) + gpiod_set_value_cansleep(pchip->enable_gpio, 0); + if (pchip->irq) { free_irq(pchip->irq, pchip); flush_workqueue(pchip->irqthread); -- 2.20.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 0/2] drm/omap: panel-tpo-td028ttec1: add backlight support
Hi, On Fri, 8 Feb 2019 11:13:33 +0200 Tomi Valkeinen wrote: > On 05/02/2019 08:38, Andreas Kemnade wrote: > > This panel has a backlight, so add a property describing that and > > add the code to use that. > > This makes things like xset dpms force off > > also turn off the backlight, so we do not need to rely on additional > > userspace programs to do that. > > > > Andreas Kemnade (2): > > drm/omap: panel-tpo-td028ttec1: add backlight support > > dt-bindings: panel: td028ttec1: add backlight property > > > > .../devicetree/bindings/display/panel/tpo,td028ttec1.txt | 2 ++ > > drivers/gpu/drm/omapdrm/displays/panel-tpo-td028ttec1.c| 10 > > ++ > > 2 files changed, 12 insertions(+) > > > > Thanks, I'll pick these up. > hmm, that sounds like: patch accepted, will appear in linux-next soon. I have not seen it there, or is it just in some branch not merged into linux-next. Any new obstacles? Regards, Andreas pgplXfeyaYtKV.pgp Description: OpenPGP digital signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 2/2] dt-bindings: panel: td028ttec1: add backlight property
This adds an additional backlight property as described in panel-common.txt Signed-off-by: Andreas Kemnade --- Documentation/devicetree/bindings/display/panel/tpo,td028ttec1.txt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Documentation/devicetree/bindings/display/panel/tpo,td028ttec1.txt b/Documentation/devicetree/bindings/display/panel/tpo,td028ttec1.txt index ed34253d9fb1..898e06ecf4ef 100644 --- a/Documentation/devicetree/bindings/display/panel/tpo,td028ttec1.txt +++ b/Documentation/devicetree/bindings/display/panel/tpo,td028ttec1.txt @@ -6,6 +6,7 @@ Required properties: Optional properties: - label: a symbolic name for the panel +- backlight: phandle of the backlight device Required nodes: - Video port for DPI input @@ -21,6 +22,7 @@ lcd-panel: td028ttec1@0 { spi-cpha; label = "lcd"; + backlight = <>; port { lcd_in: endpoint { remote-endpoint = <_out>; -- 2.11.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 1/2] drm/omap: panel-tpo-td028ttec1: add backlight support
This panel has a backlight, so fetch it from devicetree using the corresponding property as documented in panel-common.txt. It is implemented the same way as in panel-dpi.c This ensures the backlight is also disabled when the display is turned off like when doing xset dpms force off. Signed-off-by: Andreas Kemnade --- Changes in v2: - do not reorder initialisation - fix commit message drivers/gpu/drm/omapdrm/displays/panel-tpo-td028ttec1.c | 10 ++ 1 file changed, 10 insertions(+) diff --git a/drivers/gpu/drm/omapdrm/displays/panel-tpo-td028ttec1.c b/drivers/gpu/drm/omapdrm/displays/panel-tpo-td028ttec1.c index 7ddc8c574a61..1db8740f3c21 100644 --- a/drivers/gpu/drm/omapdrm/displays/panel-tpo-td028ttec1.c +++ b/drivers/gpu/drm/omapdrm/displays/panel-tpo-td028ttec1.c @@ -35,6 +35,8 @@ struct panel_drv_data { struct videomode vm; + struct backlight_device *backlight; + struct spi_device *spi_dev; }; @@ -268,6 +270,8 @@ static int td028ttec1_panel_enable(struct omap_dss_device *dssdev) r |= jbt_ret_write_0(ddata, JBT_REG_DISPLAY_ON); + backlight_enable(ddata->backlight); + dssdev->state = OMAP_DSS_DISPLAY_ACTIVE; transfer_err: @@ -283,6 +287,8 @@ static void td028ttec1_panel_disable(struct omap_dss_device *dssdev) if (!omapdss_device_is_enabled(dssdev)) return; + backlight_disable(ddata->backlight); + dev_dbg(dssdev->dev, "td028ttec1_panel_disable()\n"); jbt_ret_write_0(ddata, JBT_REG_DISPLAY_OFF); @@ -334,6 +340,10 @@ static int td028ttec1_panel_probe(struct spi_device *spi) if (ddata == NULL) return -ENOMEM; + ddata->backlight = devm_of_find_backlight(>dev); + if (IS_ERR(ddata->backlight)) + return PTR_ERR(ddata->backlight); + dev_set_drvdata(>dev, ddata); ddata->spi_dev = spi; -- 2.11.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 0/2] drm/omap: panel-tpo-td028ttec1: add backlight support
This panel has a backlight, so add a property describing that and add the code to use that. This makes things like xset dpms force off also turn off the backlight, so we do not need to rely on additional userspace programs to do that. Andreas Kemnade (2): drm/omap: panel-tpo-td028ttec1: add backlight support dt-bindings: panel: td028ttec1: add backlight property .../devicetree/bindings/display/panel/tpo,td028ttec1.txt | 2 ++ drivers/gpu/drm/omapdrm/displays/panel-tpo-td028ttec1.c| 10 ++ 2 files changed, 12 insertions(+) -- 2.11.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/2] drm/omap: panel-tpo-td028ttec1: add backlight support
Hi, On Mon, 4 Feb 2019 10:13:46 +0200 Tomi Valkeinen wrote: > Hi, > > On 19/01/2019 20:21, Andreas Kemnade wrote: > > This panel has a backlight, so fetch it from devicetree using the > > as documented in panel-common.txt. It is implemented the same way as in > > Extra words above, or maybe some are missing... > oops, This panel has a backlight, so fetch it from devicetree using the properties as documented in panel-common.txt. It is implemented the same way as in panel-dpi.c > > panel-dpi.c > > This ensures the backlight is also disabled when the display is > > turned off like when doing xset dpms force off. > > > > Signed-off-by: Andreas Kemnade > > --- > > .../gpu/drm/omapdrm/displays/panel-tpo-td028ttec1.c| 18 > > +++--- > > 1 file changed, 15 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/gpu/drm/omapdrm/displays/panel-tpo-td028ttec1.c > > b/drivers/gpu/drm/omapdrm/displays/panel-tpo-td028ttec1.c > > index 7ddc8c574a61..f326ba9dcf62 100644 > > --- a/drivers/gpu/drm/omapdrm/displays/panel-tpo-td028ttec1.c > > +++ b/drivers/gpu/drm/omapdrm/displays/panel-tpo-td028ttec1.c > > @@ -35,6 +35,8 @@ struct panel_drv_data { > > > > struct videomode vm; > > > > + struct backlight_device *backlight; > > + > > struct spi_device *spi_dev; > > }; > > > > @@ -268,6 +270,8 @@ static int td028ttec1_panel_enable(struct > > omap_dss_device *dssdev) > > > > r |= jbt_ret_write_0(ddata, JBT_REG_DISPLAY_ON); > > > > + backlight_enable(ddata->backlight); > > + > > dssdev->state = OMAP_DSS_DISPLAY_ACTIVE; > > > > transfer_err: > > @@ -283,6 +287,8 @@ static void td028ttec1_panel_disable(struct > > omap_dss_device *dssdev) > > if (!omapdss_device_is_enabled(dssdev)) > > return; > > > > + backlight_disable(ddata->backlight); > > + > > dev_dbg(dssdev->dev, "td028ttec1_panel_disable()\n"); > > > > jbt_ret_write_0(ddata, JBT_REG_DISPLAY_OFF); > > @@ -321,6 +327,15 @@ static int td028ttec1_panel_probe(struct spi_device > > *spi) > > > > dev_dbg(>dev, "%s\n", __func__); > > > > + ddata = devm_kzalloc(>dev, sizeof(*ddata), GFP_KERNEL); > > + if (ddata == NULL) > > + return -ENOMEM; > > + > > + ddata->backlight = devm_of_find_backlight(>dev); > > + > > + if (IS_ERR(ddata->backlight)) > > + return PTR_ERR(ddata->backlight); > > + > > Is there a reason for moving the ddata alloc here, instead of keeping it > where it was? > Well, I was just unsure if the spi_setup needs to be undone on error, so I moved things around. But the kzalloc() error check would face the same problem and other error checks further on, too. So I can rather keep it as is. I will send a v2. Regards, Andreas pgpLq6UsdbS9e.pgp Description: OpenPGP digital signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 0/2] drm/omap: panel-tpo-td028ttec1: add backlight support
ping On Sat, 19 Jan 2019 19:21:29 +0100 Andreas Kemnade wrote: > This panel has a backlight, so add a property describing that and > add the code to use that. > This makes things like xset dpms force off > also turn off the backlight, so we do not need to rely on additional > userspace programs to do that. > > Andreas Kemnade (2): > drm/omap: panel-tpo-td028ttec1: add backlight support > dt-bindings: panel: td028ttec1: add backlight property > > .../bindings/display/panel/tpo,td028ttec1.txt | 2 ++ > .../gpu/drm/omapdrm/displays/panel-tpo-td028ttec1.c| 18 > +++--- > 2 files changed, 17 insertions(+), 3 deletions(-) > > -- > 2.11.0 > > pgpA68rhGri6m.pgp Description: OpenPGP digital signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/2] drm/omap: panel-tpo-td028ttec1: add backlight support
This panel has a backlight, so fetch it from devicetree using the as documented in panel-common.txt. It is implemented the same way as in panel-dpi.c This ensures the backlight is also disabled when the display is turned off like when doing xset dpms force off. Signed-off-by: Andreas Kemnade --- .../gpu/drm/omapdrm/displays/panel-tpo-td028ttec1.c| 18 +++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/omapdrm/displays/panel-tpo-td028ttec1.c b/drivers/gpu/drm/omapdrm/displays/panel-tpo-td028ttec1.c index 7ddc8c574a61..f326ba9dcf62 100644 --- a/drivers/gpu/drm/omapdrm/displays/panel-tpo-td028ttec1.c +++ b/drivers/gpu/drm/omapdrm/displays/panel-tpo-td028ttec1.c @@ -35,6 +35,8 @@ struct panel_drv_data { struct videomode vm; + struct backlight_device *backlight; + struct spi_device *spi_dev; }; @@ -268,6 +270,8 @@ static int td028ttec1_panel_enable(struct omap_dss_device *dssdev) r |= jbt_ret_write_0(ddata, JBT_REG_DISPLAY_ON); + backlight_enable(ddata->backlight); + dssdev->state = OMAP_DSS_DISPLAY_ACTIVE; transfer_err: @@ -283,6 +287,8 @@ static void td028ttec1_panel_disable(struct omap_dss_device *dssdev) if (!omapdss_device_is_enabled(dssdev)) return; + backlight_disable(ddata->backlight); + dev_dbg(dssdev->dev, "td028ttec1_panel_disable()\n"); jbt_ret_write_0(ddata, JBT_REG_DISPLAY_OFF); @@ -321,6 +327,15 @@ static int td028ttec1_panel_probe(struct spi_device *spi) dev_dbg(>dev, "%s\n", __func__); + ddata = devm_kzalloc(>dev, sizeof(*ddata), GFP_KERNEL); + if (ddata == NULL) + return -ENOMEM; + + ddata->backlight = devm_of_find_backlight(>dev); + + if (IS_ERR(ddata->backlight)) + return PTR_ERR(ddata->backlight); + spi->bits_per_word = 9; spi->mode = SPI_MODE_3; @@ -330,9 +345,6 @@ static int td028ttec1_panel_probe(struct spi_device *spi) return r; } - ddata = devm_kzalloc(>dev, sizeof(*ddata), GFP_KERNEL); - if (ddata == NULL) - return -ENOMEM; dev_set_drvdata(>dev, ddata); -- 2.11.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 0/2] drm/omap: panel-tpo-td028ttec1: add backlight support
This panel has a backlight, so add a property describing that and add the code to use that. This makes things like xset dpms force off also turn off the backlight, so we do not need to rely on additional userspace programs to do that. Andreas Kemnade (2): drm/omap: panel-tpo-td028ttec1: add backlight support dt-bindings: panel: td028ttec1: add backlight property .../bindings/display/panel/tpo,td028ttec1.txt | 2 ++ .../gpu/drm/omapdrm/displays/panel-tpo-td028ttec1.c| 18 +++--- 2 files changed, 17 insertions(+), 3 deletions(-) -- 2.11.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/2] dt-bindings: panel: td028ttec1: add backlight property
This add an additional backlight property as described in panel-common.txt Signed-off-by: Andreas Kemnade --- Documentation/devicetree/bindings/display/panel/tpo,td028ttec1.txt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Documentation/devicetree/bindings/display/panel/tpo,td028ttec1.txt b/Documentation/devicetree/bindings/display/panel/tpo,td028ttec1.txt index ed34253d9fb1..898e06ecf4ef 100644 --- a/Documentation/devicetree/bindings/display/panel/tpo,td028ttec1.txt +++ b/Documentation/devicetree/bindings/display/panel/tpo,td028ttec1.txt @@ -6,6 +6,7 @@ Required properties: Optional properties: - label: a symbolic name for the panel +- backlight: phandle of the backlight device Required nodes: - Video port for DPI input @@ -21,6 +22,7 @@ lcd-panel: td028ttec1@0 { spi-cpha; label = "lcd"; + backlight = <>; port { lcd_in: endpoint { remote-endpoint = <_out>; -- 2.11.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel