Re: [PATCH 4/4] drm/tiny: st7586: drop driver owner assignment

[David Lechner]

On 3/27/24 12:48 PM, Krzysztof Kozlowski wrote:
> Core in spi_register_driver() already sets the .owner, so driver
> does not need to.
> 
> Signed-off-by: Krzysztof Kozlowski 
> ---


Acked-by: David Lechner 

Re: [PATCH 1/4] drm/tiny: ili9225: drop driver owner assignment

[David Lechner]

On 3/27/24 12:48 PM, Krzysztof Kozlowski wrote:
> Core in spi_register_driver() already sets the .owner, so driver
> does not need to.
>
> Signed-off-by: Krzysztof Kozlowski 
> ---

Acked-by: David Lechner 

Re: [PATCH 41/43] drm/tiny/st7735r: Use fbdev-dma

[David Lechner]

On 3/12/24 10:45 AM, Thomas Zimmermann wrote:
> Implement fbdev emulation with fbdev-dma. Fbdev-dma now supports
> damage handling, which is required by st7735r. Avoids the overhead of
> fbdev-generic's additional shadow buffering. No functional changes.
> 
> Signed-off-by: Thomas Zimmermann 
> Cc: David Lechner 
> ---

Acked-by: David Lechner 

Re: [PATCH 34/43] drm/tiny/ili9225: Use fbdev-dma

[David Lechner]

On 3/12/24 10:45 AM, Thomas Zimmermann wrote:
> Implement fbdev emulation with fbdev-dma. Fbdev-dma now supports
> damage handling, which is required by ili9225. Avoids the overhead of
> fbdev-generic's additional shadow buffering. No functional changes.
> 
> Signed-off-by: Thomas Zimmermann 
> Cc: David Lechner 
> ---

Acked-by: David Lechner 

Re: [PATCH 40/43] drm/tiny/st7586: Use fbdev-dma

[David Lechner]

On 3/12/24 10:45 AM, Thomas Zimmermann wrote:
> Implement fbdev emulation with fbdev-dma. Fbdev-dma now supports
> damage handling, which is required by st7586. Avoids the overhead of
> fbdev-generic's additional shadow buffering. No functional changes.
> 
> Signed-off-by: Thomas Zimmermann 
> Cc: David Lechner 
> ---

Acked-by: David Lechner 

Re: [PATCH] dt-bindings: display: remove backlight node from panel examples

[David Lechner]

On 10/11/23 5:47 AM, Luca Ceresoli wrote:

The examples for these panel drivers have a backlight node in addition to
the actual panel node. However the exact backlight is outside the scope of
this binding and should be dropped from the example.

Link: 
https://lore.kernel.org/linux-devicetree/20230724143152.ga3430423-r...@kernel.org/
Suggested-by: Rob Herring 
Signed-off-by: Luca Ceresoli 
---


Acked-by: David Lechner 

Re: [PATCH 0/2] Add st7735s drm driver and Winstar panel

[David Lechner]

On 9/6/23 11:22 AM, Stefan x Nilsson wrote:

Add a new driver for the Sitronix st7735s display controller
together with a 0.96" 80x160 color TFT display by Winstar.

The driver is very similar to the st7735r driver, but uses a
different pipe_enable sequence and also allows for an
optional regulator to be specified using devicetree.



Can this panel be used with the generic "panel-mipi-dbi-spi" driver?

more info: https://github.com/notro/panel-mipi-dbi/wiki

Re: [PATCH v3 1/2] drm/mipi-dbi: Lock SPI bus before setting D/C GPIO

[David Lechner]

On 7/24/23 1:56 AM, Otto Pflüger wrote:

Multiple displays may be connected to the same bus and share a D/C GPIO,
so the display driver needs exclusive access to the bus to ensure that
it can control the D/C GPIO safely.

Signed-off-by: Otto Pflüger 
Reviewed-by: Noralf Trønnes 
---


Acked-by: David Lechner 

Re: [PATCH v3 54/65] clk: da8xx: clk48: Switch to determine_rate

[David Lechner]

On 4/5/23 10:22 AM, Maxime Ripard wrote:

Hi David,

On Wed, Apr 05, 2023 at 10:03:24AM -0500, David Lechner wrote:

On 4/4/23 5:11 AM, Maxime Ripard wrote:

The TI DA8xx USB0 clk48 clocks implements a mux with a set_parent
hook, but doesn't provide a determine_rate implementation.

This is a bit odd, since set_parent() is there to, as its name implies,
change the parent of a clock. However, the most likely candidate to
trigger that parent change is a call to clk_set_rate(), with
determine_rate() figuring out which parent is the best suited for a
given rate.



As mentioned in my previous review, parent is selected by device
tree and should never be changed after init.


Great minds think alike then, because the driver implements exactly
that, either before or after that patch.

That patch makes the current behaviour explicit but doesn't change it in
any way.

So I guess that means that I can add your Acked-by on the three patches
you reviewed with the same message?

Maxime


Yes, preferably with a simplified commit message.

Acked-by: David Lechner 

Re: [PATCH v3 54/65] clk: da8xx: clk48: Switch to determine_rate

[David Lechner]

On 4/4/23 5:11 AM, Maxime Ripard wrote:

The TI DA8xx USB0 clk48 clocks implements a mux with a set_parent
hook, but doesn't provide a determine_rate implementation.

This is a bit odd, since set_parent() is there to, as its name implies,
change the parent of a clock. However, the most likely candidate to
trigger that parent change is a call to clk_set_rate(), with
determine_rate() figuring out which parent is the best suited for a
given rate.



As mentioned in my previous review, parent is selected by device
tree and should never be changed after init.

Re: [PATCH v3 21/65] clk: davinci: da8xx-cfgchip: Add a determine_rate hook

[David Lechner]

On 4/4/23 5:11 AM, Maxime Ripard wrote:

The Davinci DA8xxx cfgchip mux clock implements a mux with a set_parent
hook, but doesn't provide a determine_rate implementation.

This is a bit odd, since set_parent() is there to, as its name implies,
change the parent of a clock. However, the most likely candidate to
trigger that parent change is a call to clk_set_rate(), with
determine_rate() figuring out which parent is the best suited for a
given rate.



As mentioned in my previous review, parent is selected by device
tree and should never be changed after init.

Re: [PATCH v3 22/65] clk: davinci: da8xx-cfgchip: Add a determine_rate hook

[David Lechner]

On 4/4/23 5:11 AM, Maxime Ripard wrote:

The Davinci DA8xxx cfgchip "clk48" clock implements a mux with a
set_parent hook, but doesn't provide a determine_rate implementation.

This is a bit odd, since set_parent() is there to, as its name implies,
change the parent of a clock. However, the most likely candidate to
trigger that parent change is a call to clk_set_rate(), with
determine_rate() figuring out which parent is the best suited for a
given rate.



As mentioned in my previous review, parent is selected by device
tree and should never be changed after init.

Re: [PATCH v2 21/65] clk: davinci: da8xx-cfgchip: Add a determine_rate hook

[David Lechner]

On 11/7/22 6:06 AM, Maxime Ripard wrote:

Hi David,

On Fri, Nov 04, 2022 at 11:45:17AM -0500, David Lechner wrote:

On 11/4/22 8:17 AM, Maxime Ripard wrote:

The Davinci DA8xxx cfgchip mux clock implements a mux with a set_parent
hook, but doesn't provide a determine_rate implementation.

This is a bit odd, since set_parent() is there to, as its name implies,
change the parent of a clock. However, the most likely candidate to
trigger that parent change is a call to clk_set_rate(), with
determine_rate() figuring out which parent is the best suited for a
given rate.

The other trigger would be a call to clk_set_parent(), but it's far less
used, and it doesn't look like there's any obvious user for that clock.

So, the set_parent hook is effectively unused, possibly because of an
oversight. However, it could also be an explicit decision by the
original author to avoid any reparenting but through an explicit call to
clk_set_parent().



The parent is defined in the device tree and is not expected to change
at runtime, so if I am understanding the patch correctly, setting the
CLK_SET_RATE_NO_REPARENT flag seems correct.


Is that an acked-by/reviewed-by?

Thanks!
Maxime


The commit message could be updated to be more certain now, but sure...

Acked-by: David Lechner 

Re: [PATCH v2 22/65] clk: davinci: da8xx-cfgchip: Add a determine_rate hook

[David Lechner]

On 11/4/22 8:17 AM, Maxime Ripard wrote:

The Davinci DA8xxx cfgchip "clk48" clock implements a mux with a
set_parent hook, but doesn't provide a determine_rate implementation.

This is a bit odd, since set_parent() is there to, as its name implies,
change the parent of a clock. However, the most likely candidate to
trigger that parent change is a call to clk_set_rate(), with
determine_rate() figuring out which parent is the best suited for a
given rate.

The other trigger would be a call to clk_set_parent(), but it's far less
used, and it doesn't look like there's any obvious user for that clock.

So, the set_parent hook is effectively unused, possibly because of an
oversight. However, it could also be an explicit decision by the
original author to avoid any reparenting but through an explicit call to
clk_set_parent().

The latter case would be equivalent to setting the flag
CLK_SET_RATE_NO_REPARENT, together with setting our determine_rate hook
to __clk_mux_determine_rate(). Indeed, if no determine_rate
implementation is provided, clk_round_rate() (through
clk_core_round_rate_nolock()) will call itself on the parent if
CLK_SET_RATE_PARENT is set, and will not change the clock rate
otherwise. __clk_mux_determine_rate() has the exact same behavior when
CLK_SET_RATE_NO_REPARENT is set.

And if it was an oversight, then we are at least explicit about our
behavior now and it can be further refined down the line.


The parent is defined in the device tree and is not expected to change
at runtime, so if I am understanding the patch correctly, setting the
CLK_SET_RATE_NO_REPARENT flag seems correct.



Signed-off-by: Maxime Ripard 
---
  drivers/clk/davinci/da8xx-cfgchip.c | 2 ++
  1 file changed, 2 insertions(+)

diff --git a/drivers/clk/davinci/da8xx-cfgchip.c 
b/drivers/clk/davinci/da8xx-cfgchip.c
index c04276bc4051..4c1cc59bba53 100644
--- a/drivers/clk/davinci/da8xx-cfgchip.c
+++ b/drivers/clk/davinci/da8xx-cfgchip.c
@@ -565,6 +565,7 @@ static u8 da8xx_usb1_clk48_get_parent(struct clk_hw *hw)
  }
  
  static const struct clk_ops da8xx_usb1_clk48_ops = {

+   .determine_rate = __clk_mux_determine_rate,
.set_parent = da8xx_usb1_clk48_set_parent,
.get_parent = da8xx_usb1_clk48_get_parent,
  };
@@ -589,6 +590,7 @@ da8xx_cfgchip_register_usb1_clk48(struct device *dev,
  
  	init.name = "usb1_clk48";

init.ops = _usb1_clk48_ops;
+   init.flags = CLK_SET_RATE_NO_REPARENT;
init.parent_names = parent_names;
init.num_parents = 2;
  

Re: [PATCH v2 54/65] clk: da8xx: clk48: Switch to determine_rate

[David Lechner]

On 11/4/22 8:18 AM, Maxime Ripard wrote:

The TI DA8xx USB0 clk48 clocks implements a mux with a set_parent
hook, but doesn't provide a determine_rate implementation.

This is a bit odd, since set_parent() is there to, as its name implies,
change the parent of a clock. However, the most likely candidate to
trigger that parent change is a call to clk_set_rate(), with
determine_rate() figuring out which parent is the best suited for a
given rate.

The other trigger would be a call to clk_set_parent(), but it's far less
used, and it doesn't look like there's any obvious user for that clock.

So, the set_parent hook is effectively unused, possibly because of an
oversight. However, it could also be an explicit decision by the
original author to avoid any reparenting but through an explicit call to
clk_set_parent().

The driver does implement round_rate() though, which means that we can
change the rate of the clock, but we will never get to change the
parent.

However, It's hard to tell whether it's been done on purpose or not.

Since we'll start mandating a determine_rate() implementation, let's
convert the round_rate() implementation to a determine_rate(), which
will also make the current behavior explicit. And if it was an
oversight, the clock behaviour can be adjusted later on.


I think this one should be the same as the clk:davinci changes and
not allow re-parenting. Since this is a USB 48MHz PHY clock, a rate
change will never be requested.



Signed-off-by: Maxime Ripard 
---
  drivers/clk/davinci/da8xx-cfgchip.c | 10 ++
  1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/clk/davinci/da8xx-cfgchip.c 
b/drivers/clk/davinci/da8xx-cfgchip.c
index 4c1cc59bba53..f60c97091818 100644
--- a/drivers/clk/davinci/da8xx-cfgchip.c
+++ b/drivers/clk/davinci/da8xx-cfgchip.c
@@ -462,10 +462,12 @@ static unsigned long da8xx_usb0_clk48_recalc_rate(struct 
clk_hw *hw,
return 4800;
  }
  
-static long da8xx_usb0_clk48_round_rate(struct clk_hw *hw, unsigned long rate,

-   unsigned long *parent_rate)
+static int da8xx_usb0_clk48_determine_rate(struct clk_hw *hw,
+  struct clk_rate_request *req)
  {
-   return 4800;
+   req->rate = 4800;
+
+   return 0;
  }
  
  static int da8xx_usb0_clk48_set_parent(struct clk_hw *hw, u8 index)

@@ -494,7 +496,7 @@ static const struct clk_ops da8xx_usb0_clk48_ops = {
.disable= da8xx_usb0_clk48_disable,
.is_enabled = da8xx_usb0_clk48_is_enabled,
.recalc_rate= da8xx_usb0_clk48_recalc_rate,
-   .round_rate = da8xx_usb0_clk48_round_rate,
+   .determine_rate = da8xx_usb0_clk48_determine_rate,
.set_parent = da8xx_usb0_clk48_set_parent,
.get_parent = da8xx_usb0_clk48_get_parent,
  };

Re: [PATCH v2 21/65] clk: davinci: da8xx-cfgchip: Add a determine_rate hook

[David Lechner]

On 11/4/22 8:17 AM, Maxime Ripard wrote:

The Davinci DA8xxx cfgchip mux clock implements a mux with a set_parent
hook, but doesn't provide a determine_rate implementation.

This is a bit odd, since set_parent() is there to, as its name implies,
change the parent of a clock. However, the most likely candidate to
trigger that parent change is a call to clk_set_rate(), with
determine_rate() figuring out which parent is the best suited for a
given rate.

The other trigger would be a call to clk_set_parent(), but it's far less
used, and it doesn't look like there's any obvious user for that clock.

So, the set_parent hook is effectively unused, possibly because of an
oversight. However, it could also be an explicit decision by the
original author to avoid any reparenting but through an explicit call to
clk_set_parent().



The parent is defined in the device tree and is not expected to change
at runtime, so if I am understanding the patch correctly, setting the
CLK_SET_RATE_NO_REPARENT flag seems correct.



The latter case would be equivalent to setting the flag
CLK_SET_RATE_NO_REPARENT, together with setting our determine_rate hook
to __clk_mux_determine_rate(). Indeed, if no determine_rate
implementation is provided, clk_round_rate() (through
clk_core_round_rate_nolock()) will call itself on the parent if
CLK_SET_RATE_PARENT is set, and will not change the clock rate
otherwise. __clk_mux_determine_rate() has the exact same behavior when
CLK_SET_RATE_NO_REPARENT is set.

And if it was an oversight, then we are at least explicit about our
behavior now and it can be further refined down the line.

Signed-off-by: Maxime Ripard 
---
  drivers/clk/davinci/da8xx-cfgchip.c | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/davinci/da8xx-cfgchip.c 
b/drivers/clk/davinci/da8xx-cfgchip.c
index 4103d605e804..c04276bc4051 100644
--- a/drivers/clk/davinci/da8xx-cfgchip.c
+++ b/drivers/clk/davinci/da8xx-cfgchip.c
@@ -229,6 +229,7 @@ static u8 da8xx_cfgchip_mux_clk_get_parent(struct clk_hw 
*hw)
  }
  
  static const struct clk_ops da8xx_cfgchip_mux_clk_ops = {

+   .determine_rate = __clk_mux_determine_rate,
.set_parent = da8xx_cfgchip_mux_clk_set_parent,
.get_parent = da8xx_cfgchip_mux_clk_get_parent,
  };
@@ -251,7 +252,7 @@ da8xx_cfgchip_mux_clk_register(struct device *dev,
init.ops = _cfgchip_mux_clk_ops;
init.parent_names = parent_names;
init.num_parents = 2;
-   init.flags = 0;
+   init.flags = CLK_SET_RATE_NO_REPARENT;
  
  	mux->hw.init = 

mux->regmap = regmap;

Re: [PATCH] drm/st7735r: Fix module autoloading for Okaya RH128128T

[David Lechner]

On 5/20/22 4:16 AM, Javier Martinez Canillas wrote:

The SPI core always reports a "MODALIAS=spi:", even if the device was
registered via OF. This means that the st7735r.ko module won't autoload if
a DT has a node with a compatible "okaya,rh128128t" string.

In that case, kmod expects a "MODALIAS=of:N*T*Cokaya,rh128128t" uevent but
instead will get a "MODALIAS=spi:rh128128t", which is not present in the
list of aliases:

   $ modinfo drivers/gpu/drm/tiny/st7735r.ko | grep alias
   alias:  of:N*T*Cokaya,rh128128tC*
   alias:  of:N*T*Cokaya,rh128128t
   alias:  of:N*T*Cjianda,jd-t18003-t01C*
   alias:  of:N*T*Cjianda,jd-t18003-t01
   alias:  spi:jd-t18003-t01

To workaround this issue, add in the SPI table an entry for that device.

Fixes: d1d511d516f7 ("drm: tiny: st7735r: Add support for Okaya RH128128T")
Signed-off-by: Javier Martinez Canillas 
---


Acked-by: David Lechner 

Re: [PATCH v2 0/3] drm/panel: Add MIPI DBI compatible SPI driver

[David Lechner]

On 1/25/22 11:56 AM, Noralf Trønnes wrote:

Hi,

This patchset adds a driver that will work with most MIPI DBI compatible
SPI panels out there.

It's a follow up on 'drm/tiny/st7735r: Match up with staging/fbtft
driver'[1] which aimed at making the st7735r driver work with all panels
adding DT properties.

Maxime gave[2] a good overview of the situation with these displays and
proposed to make a driver that works with all MIPI DBI compatible
controllers and use a firmware file to provide the controller setup for
a particular panel.

Main change since previous version:
- Drop model property and use the compatible property instead (Rob)

Noralf.

[1] https://lore.kernel.org/dri-devel/20211124150757.17929-1-nor...@tronnes.org/
[2] https://lore.kernel.org/dri-devel/20211129093946.xhp22mvdut3m67sc@houat/


Noralf Trønnes (3):
   dt-bindings: display: add bindings for MIPI DBI compatible SPI panels
   drm/mipi-dbi: Add driver_private member to struct mipi_dbi_dev
   drm/panel: Add MIPI DBI compatible SPI driver

  .../display/panel/panel-mipi-dbi-spi.yaml |  59 +++
  MAINTAINERS   |   8 +
  drivers/gpu/drm/panel/Kconfig |  11 +
  drivers/gpu/drm/panel/Makefile|   1 +
  drivers/gpu/drm/panel/panel-mipi-dbi.c| 394 ++
  include/drm/drm_mipi_dbi.h|   2 +
  6 files changed, 475 insertions(+)
  create mode 100644 
Documentation/devicetree/bindings/display/panel/panel-mipi-dbi-spi.yaml
  create mode 100644 drivers/gpu/drm/panel/panel-mipi-dbi.c



It would be useful to also include a patch for a tool to create
these "firmware" files. For example a Python script that takes
a more human-readable input and generates a .bin file.

Re: [PATCH v2 3/3] drm/panel: Add MIPI DBI compatible SPI driver

[David Lechner]

On 1/25/22 11:57 AM, Noralf Trønnes wrote:

Add a driver that will work with most MIPI DBI compatible SPI panels.
This avoids adding a driver for every new MIPI DBI compatible controller
that is to be used by Linux. The 'compatible' Device Tree property with
a '.bin' suffix will be used to load a firmware file that contains the
controller configuration.

Example (driver will load sainsmart18.bin):

display@0 {
compatible = "sainsmart18", "panel-mipi-dbi-spi";
reg = <0>;
reset-gpios = < 25 0>;
dc-gpios = < 24 0>;
};



...


+static int panel_mipi_dbi_spi_probe(struct spi_device *spi)
+{
+   struct device *dev = >dev;
+   struct drm_display_mode mode;
+   struct mipi_dbi_dev *dbidev;
+   const struct firmware *fw;
+   const char *compatible;
+   struct drm_device *drm;
+   struct property *prop;
+   bool fw_found = false;
+   struct mipi_dbi *dbi;
+   struct gpio_desc *dc;
+   char fw_name[40];
+   int ret;
+
+   dbidev = devm_drm_dev_alloc(dev, _mipi_dbi_driver, struct 
mipi_dbi_dev, drm);
+   if (IS_ERR(dbidev))
+   return PTR_ERR(dbidev);
+
+   dbi = >dbi;
+   drm = >drm;
+
+   of_property_for_each_string(dev->of_node, "compatible", prop, 
compatible) {
+   snprintf(fw_name, sizeof(fw_name), "%s.bin", compatible);
+
+   ret = firmware_request_nowarn(, fw_name, dev);
+   if (ret) {
+   drm_dbg(drm, "No config file found for compatible: '%s' 
(error=%d)\n",
+   compatible, ret);
+   continue;
+   }
+


Should we add a directory prefix to the firmware file name to avoid the 
possibility of
file name clashes with unrelated firmwares?

Re: [PATCH 0/6] drm/tiny/st7735r: Match up with staging/fbtft driver

[David Lechner]

On 12/1/21 8:52 AM, Maxime Ripard wrote:

Hi Noralf,

On Tue, Nov 30, 2021 at 03:30:11PM +0100, Noralf Trønnes wrote:

Den 29.11.2021 10.39, skrev Maxime Ripard:

On Wed, Nov 24, 2021 at 04:03:07PM -0600, David Lechner wrote:

On 11/24/21 9:07 AM, Noralf Trønnes wrote:

I agree that it doesn't really fit in the DT either though. Noralf, what
kind of data do we need to setup a display in fbtft? The init sequence,
and maybe some enable/reset GPIO, plus some timing duration maybe?

There's one similar situation I can think of: wifi chips. Those also
need a few infos from the DT (like what bus it's connected to, enable
GPIO, etc) and a different sequence (firmware), sometimes different from
one board to the other.

Could we have a binding that would be something like:

panel@42 {
 compatible = "panel-spi";
 model = "panel-from-random-place-42";
 enable-gpios = <&...>;
}

And then, the driver would request the init sequence through the
firmware mechanism using a name generated from the model property.

It allows to support multiple devices in a given system, since the
firmware name wouldn't conflict, it makes a decent binding, and users
can adjust the init sequence easily (maybe with a bit of tooling)

Would that work?


I really like this idea. An added benefit is that one driver can handle
all MIPI DBI compatible controllers avoiding the need to do a patchset
like this for all the various MIPI DBI controllers. The firmware will
just contain numeric commands with parameters, so no need for different
controller drivers to handle the controller specific command names.

The following is a list of the MIPI DBI compatible controllers currently
in staging/fbtft: ili9341, hx8357d, st7735r, ili9163, ili9163, ili9163,
ili9163, ili9486, ili9481, tinylcd, s6d02a1, s6d02a1, hx8340bn, ili9340.

The compatible needs to be a bit more specific though since there are 2
major SPI protocols for these display: MIPI DBI and the one used by
ILI9325 and others.

The full binding would be something like this:

panel@42 {
compatible = "panel-mipi-dbi-spi";
model = "panel-from-random-place-42";

/* The MIPI DBI spec lists these powers supply pins */
vdd-supply = <&...>;
vddi-supply = <&...>;

/* Optional gpio to drive the RESX line */
reset-gpios = <&...>;

/*
 * D/CX: Data/Command, Command is active low
 * Abcense: Interface option 1 (D/C embedded in 9-bit word)
 * Precense: Interface option 3
 */
dc-gpios = <&...>;

/*
 * If set the driver won't try to read from the controller to see
 * if it's already configured by the bootloader or previously by
 * the driver. A readable controller avoids flicker and/or delay
 * enabling the pipeline.
 *
 * This property might not be necessary if we are guaranteed to
 * always read back all 1's or 0's when MISO is not connected.
 * I don't know if all setups can guarantee that.
 */
write-only;

/* Optional ref to backlight node */
backlight = <&...>;
}


It looks decent to me. We'll want Rob to give his opinion though, but it
looks in a much better shape compared to what we usually have :)


Many of these controllers also have a RGB interface option for the
pixels and only do configuration over SPI.
Maybe the compatible should reflect these 2 options somehow?


I think we'll want a "real" panel for RGB, with its own compatible
though. We have a few of these drivers in tree already, so it's better
to remain consistent.

Maxime



I'm on board with the idea of the init sequence as firmware as well.

It looks like Rob might have missed this thread, so maybe just apply
the acked patches and submit a v2 with the firmware implementation?

Re: [PATCH 3/6] dt-bindings: display: sitronix,st7735r: Remove spi-max-frequency limit

[David Lechner]

On 11/24/21 9:07 AM, Noralf Trønnes wrote:

The datasheet lists the minimum Serial clock cycle (Write) as 66ns which is


Is this supposed to say "maximum" rather than "minimum"?


15MHz. Mostly it can do much better than that and is in fact often run at
32MHz. With a clever driver that runs configuration commands at a low speed
and only the pixel data at the maximum speed the configuration can't be
messed up by transfer errors and the speed is only limited by the amount of
pixel glitches that one is able to tolerate.

Signed-off-by: Noralf Trønnes 
---

Acked-by: David Lechner 

Re: [PATCH 2/6] dt-bindings: display: sitronix,st7735r: Make reset-gpios optional

[David Lechner]

On 11/24/21 9:07 AM, Noralf Trønnes wrote:

There are other ways than using a gpio to reset the controller so make
this property optional.

Signed-off-by: Noralf Trønnes 
---

Acked-by: David Lechner 

Re: [PATCH 1/6] dt-bindings: display: sitronix, st7735r: Fix backlight in example

[David Lechner]

On 11/24/21 9:07 AM, Noralf Trønnes wrote:

The backlight property was lost during conversion to yaml in commit
abdd9e3705c8 ("dt-bindings: display: sitronix,st7735r: Convert to DT schema").
Put it back.

Fixes: abdd9e3705c8 ("dt-bindings: display: sitronix,st7735r: Convert to DT 
schema")
Signed-off-by: Noralf Trønnes 
---


Acked-by: David Lechner 

Re: [PATCH 0/6] drm/tiny/st7735r: Match up with staging/fbtft driver

[David Lechner]

On 11/24/21 9:07 AM, Noralf Trønnes wrote:

Hi,

This patchset adds a missing piece for decommissioning the
staging/fbtft/fb_st7735r.c driver namely a way to configure the
controller from Device Tree.

All fbtft drivers have builtin support for one display panel and all
other panels using that controller are configured using the Device Tree
'init' property. This property is supported by all fbtft drivers and
provides a generic way to set register values or issue commands
(depending on the type of controller).

It is common for these types of displays to have a datasheet listing the
necessary controller settings/commands or some example code doing the
same.

This is how the panel directly supported by the fb_st7735r staging
driver is described using Device Tree with that driver:

 width = <160>;
 height = <128>;

 init = <0x101
 0x296
 0x111
 0x2ff
 0x1B1 0x01 0x2C 0x2D
 0x1B4 0x07
 0x1C0 0xA2 0x02 0x84
 0x1C1 0xC5
 0x1C2 0x0A 0x00
 0x1C5 0x0E
 0x13a 0x55
 0x136 0x60
 0x1E0 0x0F 0x1A 0x0F 0x18 0x2F 0x28 0x20 0x22
   0x1F 0x1B 0x23 0x37 0x00 0x07 0x02 0x10
 0x1E1 0x0F 0x1B 0x0F 0x17 0x33 0x2C 0x29 0x2E
   0x30 0x30 0x39 0x3F 0x00 0x07 0x03 0x10
 0x129
 0x264>;


This is how the same panel is described using the st7735r drm driver and
this patchset:

 width = <160>;
 height = <128>;

 frmctr1 = [ 01 2C 2D ];
 invctr = [ 07 ];
 pwctr1 = [ A2 02 84 ];
 pwctr2 = [ C5 ];
 pwctr3 = [ 0A 00 ];
 vmctr1 = [ 0E ];
 madctl = [ 60 ];
 gamctrp1 = [ 0F 1A 0F 18 2F 28 20 22 1F 1B 23 37 00 07 02 10 ];
 gamctrn1 = [ 0F 1B 0F 17 33 2C 29 2E 30 30 39 3F 00 07 03 10 ];


Do these setting correspond to actual physical properties of the display?

What is the advantage of this compared to just adding a new compatible
string if a new display requires different settings? (Other than being
able to use a new display without compiling a new kernel/module.)

It is nice for the driver implementation to be able to use the byte
arrays from the binding directly, but it doesn't really make sense from
a "device tree describes the hardware" point of view.

For example, looking at the data sheet, frmctr1 looks like it is actually
multiple properties, the 1-line period, front porch and back porch.




Back when the fbtft drivers were added to staging I asked on the DT
mailinglist if it was OK to use the 'init' property but it was turned
down. In this patchset I'm trying the same approach used by the
solomon,ssd1307fb.yaml binding in describing the attached panel. That
binding prefixes the properties with the vendor name, not sure why that
is and I think it looks strange so I try without it.


Because [1] says so?

"DO use a vendor prefix on device-specific property names. Consider if
properties could be common among devices of the same class. Check other
existing bindings for similar devices."

Do all displays have "frmctr1" or only sitronix displays?


[1]: 
https://www.kernel.org/doc/html/latest/devicetree/bindings/writing-bindings.html



Noralf.


Noralf Trønnes (6):
   dt-bindings: display: sitronix,st7735r: Fix backlight in example
   dt-bindings: display: sitronix,st7735r: Make reset-gpios optional
   dt-bindings: display: sitronix,st7735r: Remove spi-max-frequency limit
   dt-bindings: display: sitronix,st7735r: Add initialization properties
   drm/mipi-dbi: Add device property functions
   drm: tiny: st7735r: Support DT initialization of controller

  .../bindings/display/sitronix,st7735r.yaml| 122 ++-
  drivers/gpu/drm/drm_mipi_dbi.c| 139 ++
  drivers/gpu/drm/tiny/st7735r.c|  87 +--
  include/drm/drm_mipi_dbi.h|   3 +
  4 files changed, 334 insertions(+), 17 deletions(-)

Re: [PATCH 7/7] drm/st7586: Use framebuffer dma-buf helpers

[David Lechner]

On 7/16/21 9:08 AM, Thomas Zimmermann wrote:

Replace dma_buf_begin_cpu_access() with drm_gem_fb_begin_cpu_access();
same for _end_cpu_access(). Remove some boiler-plate code. No functional
changes.

Signed-off-by: Thomas Zimmermann 
---


Acked-by: David Lechner 

Re: [PATCH 11/11] drm/tiny: drm_gem_simple_display_pipe_prepare_fb is the default

[David Lechner]

On 5/21/21 4:09 AM, Daniel Vetter wrote:

Goes through all the drivers and deletes the default hook since it's
the default now.


Acked-by: David Lechner 

Re: [PATCH v3 0/1] drm/tiny: add support for Waveshare 2inch LCD module

[David Lechner]

On 3/30/21 3:08 AM, Carlis wrote:

From: Xuezhi Zhang 

This adds a new module for the ST7789V controller with parameters for
the Waveshare 2inch LCD module.

Signed-off-by: Xuezhi Zhang 
---
v2:change compatible value.
v3:change author name.
---
  MAINTAINERS|   8 +
  drivers/gpu/drm/tiny/Kconfig   |  14 ++
  drivers/gpu/drm/tiny/Makefile  |   1 +
  drivers/gpu/drm/tiny/st7789v.c | 269 +
  4 files changed, 292 insertions(+)
  create mode 100644 drivers/gpu/drm/tiny/st7789v.c

diff --git a/MAINTAINERS b/MAINTAINERS
index d92f85ca831d..df25e8e0deb1 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -5769,6 +5769,14 @@ T:   git git://anongit.freedesktop.org/drm/drm-misc
  F:Documentation/devicetree/bindings/display/sitronix,st7735r.yaml
  F:drivers/gpu/drm/tiny/st7735r.c
  
+DRM DRIVER FOR SITRONIX ST7789V PANELS

+M: David Lechner 


I should not be added here. I don't have one of these displays.


+M: Xuezhi Zhang 
+S: Maintained
+T: git git://anongit.freedesktop.org/drm/drm-misc
+F: Documentation/devicetree/bindings/display/sitronix,st7789v-dbi.yaml
+F: drivers/gpu/drm/tiny/st7789v.c
+
  DRM DRIVER FOR SONY ACX424AKP PANELS
  M:Linus Walleij 
  S:Maintained

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 7/8] drm/mipi-dbi: Remove ->enabled

[David Lechner]

On 6/12/20 11:00 AM, Daniel Vetter wrote:

The atomic helpers try really hard to not lose track of things,
duplicating enabled tracking in the driver is at best confusing.
Double-enabling or disabling is a bug in atomic helpers.

In the fb_dirty function we can just assume that the fb always exists,
simple display pipe helpers guarantee that the crtc is only enabled
together with the output, so we always have a primary plane around.

Now in the update function we need to be a notch more careful, since
that can also get called when the crtc is off. And we don't want to
upload frames when that's the case, so filter that out too.

Signed-off-by: Daniel Vetter 
Cc: Maarten Lankhorst 
Cc: Maxime Ripard 
Cc: Thomas Zimmermann 
Cc: David Airlie 
Cc: Daniel Vetter 
Cc: David Lechner 
---


Acked-by: David Lechner 
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 18/44] drm/st7586: Use devm_drm_dev_alloc

[David Lechner]

On 4/3/20 8:58 AM, Daniel Vetter wrote:

Already using devm_drm_dev_init, so very simple replacment.

Signed-off-by: Daniel Vetter 
Cc: David Lechner 
---

Acked-by: David Lechner 
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 23/44] drm/ili9225: Use devm_drm_dev_alloc

[David Lechner]

On 4/3/20 8:58 AM, Daniel Vetter wrote:

Already using devm_drm_dev_init, so very simple replacment.

Signed-off-by: Daniel Vetter 
Cc: David Lechner 
---

Acked-by: David Lechner 
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 17/44] drm/st7735r: Use devm_drm_dev_alloc

[David Lechner]

On 4/3/20 8:58 AM, Daniel Vetter wrote:

Already using devm_drm_dev_init, so very simple replacment.

Aside: There was an oddity in the old code, we allocated priv but in
the error path we've freed priv->dbidev ...

Signed-off-by: Daniel Vetter 
Cc: David Lechner 
---


Acked-by: David Lechner 
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 22/44] drm/ili9341: Use devm_drm_dev_alloc

[David Lechner]

On 4/3/20 8:58 AM, Daniel Vetter wrote:

Already using devm_drm_dev_init, so very simple replacment.

Signed-off-by: Daniel Vetter 
Cc: "Noralf Trønnes" 
Cc: Sam Ravnborg 
Cc: Daniel Vetter 
Cc: Eric Anholt 
Cc: David Lechner 
---

Acked-by: David Lechner 
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 1/4] drm/tiny/repaper: Make driver OF-independent

[David Lechner]

On 2/5/20 3:31 PM, Sam Ravnborg wrote:

Hi David.

Are you planning to pick this series and apply it?
Unless I get any other info I plan to process it tomorrow.

Sam



I won't be able to do it before then, so please go ahead.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v1 2/4] drm/tiny/repaper: No need to set ->owner for spi_register_driver()

[David Lechner]

On 1/22/20 4:54 AM, Andy Shevchenko wrote:

The spi_register_driver() will set the ->owner member to THIS_MODULE.

Signed-off-by: Andy Shevchenko 
---

Reviewed-by: David Lechner 
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v1 4/4] drm/tiny/st7735r: No need to set ->owner for spi_register_driver()

[David Lechner]

On 1/22/20 4:54 AM, Andy Shevchenko wrote:

The spi_register_driver() will set the ->owner member to THIS_MODULE.

Signed-off-by: Andy Shevchenko 
---

Reviewed-by: David Lechner 

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v1 3/4] drm/tiny/st7735r: Make driver OF-independent

[David Lechner]

On 1/22/20 4:54 AM, Andy Shevchenko wrote:

There is one OF call in the driver that limits its area of use.
Replace it to generic device_get_match_data() and get rid of OF dependency.

Signed-off-by: Andy Shevchenko 
---

Acked-by: David Lechner 
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 1/5] dt-bindings: display: sitronix, st7735r: Convert to DT schema

[David Lechner]

On 1/15/20 6:45 AM, Geert Uytterhoeven wrote:

Convert the DT binding documentation for Sitronix ST7735R displays to DT
schema.

Add a reference to the Adafruit 1.8" LCD while at it.

Signed-off-by: Geert Uytterhoeven 
---
v2:
   - New.
---
  .../bindings/display/sitronix,st7735r.txt | 35 --
  .../bindings/display/sitronix,st7735r.yaml| 65 +++
  MAINTAINERS   |  2 +-
  3 files changed, 66 insertions(+), 36 deletions(-)
  delete mode 100644 
Documentation/devicetree/bindings/display/sitronix,st7735r.txt
  create mode 100644 
Documentation/devicetree/bindings/display/sitronix,st7735r.yaml

diff --git a/Documentation/devicetree/bindings/display/sitronix,st7735r.txt 
b/Documentation/devicetree/bindings/display/sitronix,st7735r.txt
deleted file mode 100644
index cd5c7186890a2be7..
--- a/Documentation/devicetree/bindings/display/sitronix,st7735r.txt
+++ /dev/null
@@ -1,35 +0,0 @@
-Sitronix ST7735R display panels
-
-This binding is for display panels using a Sitronix ST7735R controller in SPI
-mode.
-
-Required properties:
-- compatible:  "jianda,jd-t18003-t01", "sitronix,st7735r"
-- dc-gpios:Display data/command selection (D/CX)
-- reset-gpios: Reset signal (RSTX)
-
-The node for this driver must be a child node of a SPI controller, hence
-all mandatory properties described in ../spi/spi-bus.txt must be specified.
-
-Optional properties:
-- rotation:panel rotation in degrees counter clockwise (0,90,180,270)
-- backlight:   phandle of the backlight device attached to the panel
-
-Example:
-
-   backlight: backlight {
-   compatible = "gpio-backlight";
-   gpios = < 44 GPIO_ACTIVE_HIGH>;
-   };
-
-   ...
-
-   display@0{
-   compatible = "jianda,jd-t18003-t01", "sitronix,st7735r";
-   reg = <0>;
-   spi-max-frequency = <3200>;
-   dc-gpios = < 43 GPIO_ACTIVE_HIGH>;
-   reset-gpios = < 80 GPIO_ACTIVE_HIGH>;
-   rotation = <270>;
-   backlight = 
-   };
diff --git a/Documentation/devicetree/bindings/display/sitronix,st7735r.yaml 
b/Documentation/devicetree/bindings/display/sitronix,st7735r.yaml
new file mode 100644
index ..21bccc91f74255e1
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/sitronix,st7735r.yaml
@@ -0,0 +1,65 @@
+# SPDX-License-Identifier: GPL-2.0-only
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/sitronix,st7735r.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Sitronix ST7735R Display Panels Device Tree Bindings
+
+maintainers:
+  - David Lechner 
+
+description:
+  This binding is for display panels using a Sitronix ST7735R controller in
+  SPI mode.
+
+allOf:
+  - $ref: panel/panel-common.yaml#


not all of these properties are applicable.


+
+properties:
+  compatible:
+oneOf:
+  - description:
+  Adafruit 1.8" 160x128 Color TFT LCD (Product ID 358 or 618)
+items:
+  - enum:
+  - jianda,jd-t18003-t01
+  - const: sitronix,st7735r
+
+  spi-max-frequency:
+maximum: 3200
+
+  dc-gpios:
+maxItems: 1
+description: Display data/command selection (D/CX)
+
+required:
+  - compatible
+  - reg
+  - dc-gpios
+  - reset-gpios


Missing optional rotation and backlight properties.


+
+examples:
+  - |
+#include 
+
+backlight: backlight {
+compatible = "gpio-backlight";
+gpios = < 44 GPIO_ACTIVE_HIGH>;
+};
+
+spi {
+#address-cells = <1>;
+#size-cells = <0>;
+
+display@0{
+compatible = "jianda,jd-t18003-t01", "sitronix,st7735r";
+reg = <0>;
+spi-max-frequency = <3200>;
+dc-gpios = < 43 GPIO_ACTIVE_HIGH>;
+reset-gpios = < 80 GPIO_ACTIVE_HIGH>;
+rotation = <270>;
+    };
+    };
+
+...
diff --git a/MAINTAINERS b/MAINTAINERS
index ea8262509bdd21ac..3007f83bd504194a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -5382,7 +5382,7 @@ M:David Lechner 
  T:git git://anongit.freedesktop.org/drm/drm-misc
  S:Maintained
  F:drivers/gpu/drm/tiny/st7735r.c
-F: Documentation/devicetree/bindings/display/sitronix,st7735r.txt
+F: Documentation/devicetree/bindings/display/sitronix,st7735r.yaml
  
  DRM DRIVER FOR SONY ACX424AKP PANELS

  M:Linus Walleij 



___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 3/3] drm: tiny: st7735r: Add support for Okaya RH128128T

[David Lechner]

On 1/6/20 3:28 AM, Geert Uytterhoeven wrote:

Hi Sam,

On Sun, Jan 5, 2020 at 10:13 AM Sam Ravnborg  wrote:

Good to see we add more functionality to the smallest driver in DRM.
The patch triggered a few comments - see below.
Some comments relates to the original driver - and not your changes.


Thanks for your comments!


On Thu, Jan 02, 2020 at 03:12:46PM +0100, Geert Uytterhoeven wrote:

Add support for the Okaya RH128128T display to the st7735r driver.

The RH128128T is a 128x128 1.44" TFT display driven by a Sitronix
ST7715R TFT Controller/Driver.  The latter is very similar to the
ST7735R, and can be handled by the existing st7735r driver.


As a general comment - it would have eased review if this was split
in two patches.
One patch to introduce the infrastructure to deal with another set of
controller/display and one patch introducing the new combination.


I had thought about that, but didn't pursue as the new combination is
just 7 added lines.  If you prefer a split, I can do that.


--- a/drivers/gpu/drm/tiny/st7735r.c
+++ b/drivers/gpu/drm/tiny/st7735r.c
@@ -1,8 +1,9 @@
  // SPDX-License-Identifier: GPL-2.0+
  /*
- * DRM driver for Sitronix ST7735R panels
+ * DRM driver for Sitronix ST7715R/ST7735R panels


This comment could describe the situation a little better.
This is a sitronix st7735r controller with a jianda jd-t18003-t01
display.
Or a sitronix st7715r controller with a okaya rh128128t display.


Indeed. It is currently limited to two controller/display combos.
But I expect more combos to be added over time.
Hence does it make sense to describe all of that in the top comments?


@@ -37,12 +39,28 @@
  #define ST7735R_MY   BIT(7)
  #define ST7735R_MX   BIT(6)
  #define ST7735R_MV   BIT(5)
+#define ST7735R_RGB  BIT(3)
+
+struct st7735r_cfg {
+ const struct drm_display_mode mode;
+ unsigned int left_offset;
+ unsigned int top_offset;
+ unsigned int write_only:1;
+ unsigned int rgb:1; /* RGB (vs. BGR) */
+};
+
+struct st7735r_priv {
+ struct mipi_dbi_dev dbidev; /* Must be first for .release() */
+ unsigned int rgb:1;
+};


The structs here uses "st7735r" as the generic prefix.
But the rest of this file uses "jd_t18003_t01" as the generic prefix.

It would help readability if the same prefix is used for the common
stuff everywhere.


Agreed.
So I think it makes most sense to rename jd_t18003_t01_pipe_{enable,funcs}
to sh7735r_pipe_{enable,funcs}?
If needed, the display-specific parts (e.g. gamma parameters) could be
factored out in st7735r_cfg later, if neeeded.


IIRC, the original intention here is that functions/structs with the
jd_t18003_t01_ prefix are specific to the panel, not the controller.
E.g. things like power settings and gamma curves.

The idea is that it is much easier to write and understand the init sequence
as a function rather than trying to make a generic function that can parse
a any possible init sequence from a data structure.

This new panel really has all of the same settings as the existing one?

Having a separate pipe enable function for the new panel would also eliminate
the need for the extra private rgb data.

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/3] dt-bindings: display: sitronix, st7735r: Add Okaya rh128128t

[David Lechner]

On 1/2/20 8:46 AM, Sam Ravnborg wrote:

Hi Geert.

On Thu, Jan 02, 2020 at 03:12:44PM +0100, Geert Uytterhoeven wrote:

Document support for the Okaya RH128128T display, which is a 128x128
1.44" TFT display driven by a Sitronix ST7715R TFT Controller/Driver.

ST7715R and ST7735R are very similar.  Their major difference is that
the former is restricted to displays of up to 132x132 pixels, while the
latter supports displays up to 132x162 pixels.

Signed-off-by: Geert Uytterhoeven 
---
  .../devicetree/bindings/display/sitronix,st7735r.txt  | 4 +++-
  1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/display/sitronix,st7735r.txt 
b/Documentation/devicetree/bindings/display/sitronix,st7735r.txt
index cd5c7186890a2be7..87ebdcb294e29798 100644
--- a/Documentation/devicetree/bindings/display/sitronix,st7735r.txt
+++ b/Documentation/devicetree/bindings/display/sitronix,st7735r.txt

While touching the bindings file, can I convince you to convert it to
meta-schema format (.yaml)?



@@ -4,7 +4,9 @@ This binding is for display panels using a Sitronix ST7735R 
controller in SPI
  mode.
  
  Required properties:

-- compatible:  "jianda,jd-t18003-t01", "sitronix,st7735r"
+- compatible:  Must be one of the following combinations:
+ - "jianda,jd-t18003-t01", "sitronix,st7735r"
+ - "okaya,rh128128t", "sitronix,st7715r"


It would be nice if there was a "description" for each pair of
compatible that identified the actual panel.
In your case "Okaya RH128128T 1.44" 128x128 TFT display"
It can be looked up in git history - but better to have it in the
binding file.

Sam



It would be nice to have the Adafruit part name in here too while we
are at it. I had to dig really deep to find what the actual display
panel was.

https://www.adafruit.com/product/358
https://www.adafruit.com/product/618
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/tilcdc: Switch to using GPIO descriptors

[David Lechner]

On 12/3/19 7:09 AM, Linus Walleij wrote:

The TI LCDC picks a GPIO line from the device tree to use
for DPMS power on/off. We can switch this to use a GPIO
descriptor pretty easily. Make sure to request the GPIO
"as is" so that the DPMS state that we start (boot) in is
preserved.

Cc: Jyri Sarha 
Cc: Tomi Valkeinen 
Cc: David Lechner 
Signed-off-by: Linus Walleij 
---


Reviewed-by: David Lechner 


___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 4/5] drm/dsi: rename MIPI_DCS_SET_PARTIAL_AREA to MIPI_DCS_SET_PARTIAL_ROWS

[David Lechner]

On 10/22/19 5:09 AM, Jani Nikula wrote:

The DCS command has been named SET_PARTIAL_ROWS in the DCS spec since
v1.02, for more than a decade. Rename the enumeration to match the spec.

Cc: David Lechner 
Cc: Vandita Kulkarni 
Signed-off-by: Jani Nikula 
---


I guess all of my documents are old and say set_partial_area, but I will
take your word for it.

It could be helpful to leave a comment in the code about the renaming
so that if people with old docs search for SET_PARTIAL_AREA, they can
still find it.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/4] drm/tiny/ili9341: Move driver to drm/panel

[David Lechner]

On 8/1/19 8:52 AM, Noralf Trønnes wrote:

Move the driver to drm/panel and take advantage of the new panel support
in drm_mipi_dbi. Change the file name to match the naming standard in
drm/panel. The DRM driver name is kept since it is ABI.

Add missing MAINTAINERS entry.

Cc: David Lechner 
Signed-off-by: Noralf Trønnes 
---


Reviewed-by: David Lechner 

Although, I will say that the way the diff came out, it makes it a bit
hard to follow the patch, so more more details in the commit message about
the specific changes could be helpful.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 3/4] drm/tiny/mi0283qt: Move driver to panel-ilitek-ili9341

[David Lechner]

On 8/1/19 8:52 AM, Noralf Trønnes wrote:

The MI0283QT panels use a ILI9341 controller so it makes sense to merge
it with the other ili9341 code.

The DRM driver name is ABI, so that is retained.

Cc: David Lechner 
Signed-off-by: Noralf Trønnes 
---


...


@@ -216,6 +339,10 @@ static int ili9341_probe(struct spi_device *spi)
return PTR_ERR(dc);
}
  
+	ili->regulator = devm_regulator_get(dev, "power");

+   if (IS_ERR(ili->regulator))
+   return PTR_ERR(ili->regulator);
+
ili->backlight = devm_of_find_backlight(dev);
if (IS_ERR(ili->backlight))
return PTR_ERR(ili->backlight);
@@ -230,7 +357,12 @@ static int ili9341_probe(struct spi_device *spi)
ili->panel.dev = dev;
ili->panel.funcs = ili->conf->funcs;
  


Should probably add a comment here that this is for backwards
compatibility of the driver name so that no one is tempted to
to add more driver structs when adding new panels.


-   return drm_mipi_dbi_panel_register(>panel, >dbidev, 
_drm_driver,
+   if (ili->conf == _data)
+   driver = _drm_driver;
+   else
+   driver = _drm_driver;
+
+   return drm_mipi_dbi_panel_register(>panel, >dbidev, driver,
   ili->conf->mode, rotation);
  }
  

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [4/4] drm/panel/ili9341: Support DPI panels

[David Lechner]

On 8/1/19 8:52 AM, Noralf Trønnes wrote:

Add support for panels that use the DPI interface.
ILI9341 has onboard RAM so the assumption made here is that all such
panels support pixel upload over DBI.

The presence/absense of the Device Tree 'port' node decides which
interface is used for pixel transfer.

Signed-off-by: Noralf Trønnes 
---
  drivers/gpu/drm/panel/panel-ilitek-ili9341.c | 56 
  1 file changed, 45 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/panel/panel-ilitek-ili9341.c 
b/drivers/gpu/drm/panel/panel-ilitek-ili9341.c
index f6082fa2a389..7cbfd739c7fd 100644
--- a/drivers/gpu/drm/panel/panel-ilitek-ili9341.c
+++ b/drivers/gpu/drm/panel/panel-ilitek-ili9341.c
@@ -11,6 +11,7 @@
  #include 
  #include 
  #include 
+#include 
  #include 
  #include 
  #include 
@@ -53,11 +54,13 @@
  struct ili9341_config {
const struct drm_panel_funcs *funcs;
const struct drm_display_mode *mode;
+   bool no_dpi;


Can we invert this to use positive logic? i.e. use_dpi instead of
no_dpi.


  };
  
  struct ili9341 {

struct mipi_dbi_dev dbidev; /* This must be the first entry */
struct drm_panel panel;
+   bool use_dpi;
struct regulator *regulator;
struct backlight_device *backlight;
const struct ili9341_config *conf;
@@ -174,6 +177,7 @@ static const struct drm_display_mode yx240qv29_mode = {
  static const struct ili9341_config yx240qv29_data = {
.funcs = _funcs,
.mode = _mode,
+   .no_dpi = true,
  };
  
  static int mi0283qt_prepare(struct drm_panel *panel)

@@ -291,6 +295,7 @@ static const struct drm_display_mode mi0283qt_mode = {
  static const struct ili9341_config mi0283qt_data = {
.funcs = _drm_funcs,
.mode = _mode,
+   .no_dpi = true,
  };
  
  /* Legacy, DRM driver name is ABI */

@@ -303,6 +308,7 @@ static int ili9341_probe(struct spi_device *spi)
const struct spi_device_id *spi_id;
struct device *dev = >dev;
struct drm_driver *driver;
+   struct device_node *port;
struct mipi_dbi *dbi;
struct gpio_desc *dc;
struct ili9341 *ili;
@@ -357,21 +363,44 @@ static int ili9341_probe(struct spi_device *spi)
ili->panel.dev = dev;
ili->panel.funcs = ili->conf->funcs;
  
-	if (ili->conf == _data)

-   driver = _drm_driver;
-   else
-   driver = _drm_driver;
  
-	return drm_mipi_dbi_panel_register(>panel, >dbidev, driver,

-  ili->conf->mode, rotation);
+   port = of_get_child_by_name(dev->of_node, "port");
+   if (port) {
+   of_node_put(port);
+   ili->use_dpi = true;
+   }
+
+   if (ili->conf->no_dpi)
+   ili->use_dpi = false;


then this can just be ili->use_dpi = ili->conf->use_dpi.


+
+   if (ili->use_dpi) {
+   ret = drm_panel_add(>panel);
+   } else {
+   if (ili->conf == _data)
+   driver = _drm_driver;
+   else
+   driver = _drm_driver;
+
+   ret = drm_mipi_dbi_panel_register(>panel, >dbidev, 
driver,
+ ili->conf->mode, rotation);
+   }
+
+   return ret;
  }
  
  static int ili9341_remove(struct spi_device *spi)

  {
struct ili9341 *ili = spi_get_drvdata(spi);
  
-	drm_dev_unplug(>dbidev.drm);

-   drm_atomic_helper_shutdown(>dbidev.drm);
+   if (ili->use_dpi) {
+   drm_panel_remove(>panel);
+   drm_panel_disable(>panel);
+   drm_panel_unprepare(>panel);
+   kfree(ili);
+   } else {
+   drm_dev_unplug(>dbidev.drm);
+   drm_atomic_helper_shutdown(>dbidev.drm);
+   }
  
  	return 0;

  }
@@ -380,21 +409,26 @@ static void ili9341_shutdown(struct spi_device *spi)
  {
struct ili9341 *ili = spi_get_drvdata(spi);
  
-	drm_atomic_helper_shutdown(>dbidev.drm);

+   if (!ili->use_dpi)
+   drm_atomic_helper_shutdown(>dbidev.drm);
  }
  
  static int __maybe_unused ili9341_pm_suspend(struct device *dev)

  {
struct ili9341 *ili = dev_get_drvdata(dev);
  
-	return drm_mode_config_helper_suspend(>dbidev.drm);

+   if (!ili->use_dpi)
+   return drm_mode_config_helper_suspend(>dbidev.drm);
+
+   return 0;
  }
  
  static int __maybe_unused ili9341_pm_resume(struct device *dev)

  {
struct ili9341 *ili = dev_get_drvdata(dev);
  
-	drm_mode_config_helper_resume(>dbidev.drm);

+   if (!ili->use_dpi)
+   drm_mode_config_helper_resume(>dbidev.drm);
  
  	return 0;

  }



___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 06/10] drm/tinydrm: Move tinydrm_spi_transfer()

[David Lechner]

On 7/25/19 9:16 AM, Noralf Trønnes wrote:



Den 18.07.2019 14.14, skrev Noralf Trønnes:



Den 17.07.2019 21.48, skrev David Lechner:

On 7/17/19 6:58 AM, Noralf Trønnes wrote:

This is only used by mipi-dbi drivers so move it there.

The reason this isn't moved to the SPI subsystem is that it will in a
later patch pass a dummy rx buffer for SPI controllers that need this.
Low memory boards (64MB) can run into a problem allocating such a "large"
contiguous buffer on every transfer after a long up time.
This leaves a very specific use case, so we'll keep the function here.
mipi-dbi will first go through a refactoring though, before this will
be done.

Remove SPI todo entry now that we're done with the tinydrm.ko SPI code.

Additionally move the mipi_dbi_spi_init() declaration to the other SPI
functions.

Cc: David Lechner 
Signed-off-by: Noralf Trønnes 
---


Acked-by: : David Lechner 

I assume that the comments here might have something to do with the
issue[1] I raised a while back? Should I dust that patch off and resend
it after this series lands?

[1]:
https://lore.kernel.org/lkml/1519082461-32405-1-git-send-email-da...@lechnology.com/



Yep, that's the one. I want to refactor mipi-dbi first splitting struct
mipi_dbi into an interface and display pipeline part. The helper is
going to be moved to drivers/gpu/drm with the other helpers.
Please wait until that is done, I want to see what kind of coupling I
end up between the two structs and don't want another dependency to deal
with if I can avoid it.



I've applied the series now.

Do you have this problem only on the EV3 and with only one
display/driver? If so I'm wondering if there's a way to implement this
that doesn't affect the other drivers since you need a special use case
to be hit by this.

Noralf.



I've let this sit running for several days and I don't see the error
any more. So perhaps something was fixed in the DMA driver? Or maybe
I just haven't seen the error because it is sitting idle and not under
heavy memory usage? (I just ran some apt commands that crashed because
of OOM and nothing happened with the display driver.)

Interestingly, I was getting a similar allocation error from zram
in the 5.2 kernel, usually triggered by starting a SSH session (totally
unrelated to DRM).

So, unless I start seeing the problem again, I think we can leave it
alone for now.

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [1/3] RFT: drm/pl111: Support grayscale

[David Lechner]

On 7/23/19 8:37 AM, Linus Walleij wrote:

Migrating the TI nspire calculators to use the PL111 driver for
framebuffer requires grayscale support for the elder panel
which uses 8bit grayscale only.

DRM does not support 8bit grayscale framebuffers in memory,
but by defining the bus format to be MEDIA_BUS_FMT_Y8_1X8 we
can get the hardware to turn on a grayscaling feature and
convert the RGB framebuffer to grayscale for us.




What would it take to add proper grayscale framebuffer
support to DRM? I've been using the RGB to gray conversion
method for a while now with st7586 and it is OK but is
creates extra work if you want things to actually look
"good" instead of "OK" because you have to add code to
userspace programs to craft images in a certain way so
that they come out on the other side looking as intended
on the actual display.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 7/9] drm/tinydrm/mipi-dbi: Select DRM_KMS_HELPER

[David Lechner]

On 7/22/19 5:43 AM, Noralf Trønnes wrote:

mipi-dbi uses several KMS helper functions but that build dependency is
not expressed. Select DRM_KMS_HELPER to fix that.

Reported-by: kbuild test robot 
Signed-off-by: Noralf Trønnes 
---


Reviewed-by: David Lechner 



___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 5/9] drm/tinydrm/mipi-dbi: Remove CMA helper dependency

[David Lechner]

On 7/22/19 5:43 AM, Noralf Trønnes wrote:

mipi-dbi depends on the CMA helper through it's use of
drm_fb_cma_get_gem_obj(). This is an unnecessary dependency to drag in for
drivers that only want to use the MIPI DBI interface part.
Avoid this by open coding the function.

Signed-off-by: Noralf Trønnes 
---


Reviewed-by: David Lechner 

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 6/9] drm/tinydrm/Kconfig: drivers: Select BACKLIGHT_CLASS_DEVICE

[David Lechner]

On 7/22/19 5:43 AM, Noralf Trønnes wrote:

The mipi_dbi helper is missing a dependency on DRM_KMS_HELPER and putting
that in revealed this problem:

drivers/video/fbdev/Kconfig:12:error: recursive dependency detected!
drivers/video/fbdev/Kconfig:12: symbol FB is selected by DRM_KMS_FB_HELPER
drivers/gpu/drm/Kconfig:75: symbol DRM_KMS_FB_HELPER depends on 
DRM_KMS_HELPER
drivers/gpu/drm/Kconfig:69: symbol DRM_KMS_HELPER is selected by 
TINYDRM_MIPI_DBI
drivers/gpu/drm/tinydrm/Kconfig:11: symbol TINYDRM_MIPI_DBI is selected by 
TINYDRM_HX8357D
drivers/gpu/drm/tinydrm/Kconfig:15: symbol TINYDRM_HX8357D depends on 
BACKLIGHT_CLASS_DEVICE
drivers/video/backlight/Kconfig:144:symbol BACKLIGHT_CLASS_DEVICE is 
selected by FB_BACKLIGHT
drivers/video/fbdev/Kconfig:187:symbol FB_BACKLIGHT depends on FB

A symbol that selects DRM_KMS_HELPER can not depend on
BACKLIGHT_CLASS_DEVICE. The reason for this is that DRM_KMS_FB_HELPER
selects FB instead of depending on it.

The tinydrm drivers have somehow gotten away with depending on
BACKLIGHT_CLASS_DEVICE because DRM_TINYDRM selects DRM_KMS_HELPER and the
drivers depend on that symbol.

An audit shows that all DRM drivers that select DRM_KMS_HELPER and use
BACKLIGHT_CLASS_DEVICE, selects it:
   DRM_TILCDC, DRM_GMA500, DRM_SHMOBILE, DRM_NOUVEAU, DRM_FSL_DCU,
   DRM_I915, DRM_RADEON, DRM_AMDGPU, DRM_PARADE_PS8622

Documentation/kbuild/kconfig-language.txt has a note regarding select:
1. 'select should be used with care since it doesn't visit dependencies.'
This is not a problem since BACKLIGHT_CLASS_DEVICE doesn't have any
dependencies.
2. 'In general use select only for non-visible symbols'
BACKLIGHT_CLASS_DEVICE is user visible.

The real solution to this would be to have DRM_KMS_FB_HELPER depend on the
user visible symbol FB. That is a can of worms I'm not willing to tackle.
I fear that such a change will result in me handling difficult fallouts
for the next weeks. So I'm following DRM suite here.

Signed-off-by: Noralf Trønnes 
---


Reviewed-by: David Lechner 



___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 9/9] MAINTAINERS: Remove tinydrm entry

[David Lechner]

On 7/22/19 5:43 AM, Noralf Trønnes wrote:

tinydrm is just a collection of tiny drivers now.
Add T: drm-misc entry for tinydrm drivers that lacks it.

Cc: David Lechner 
Reviewed-by: Sam Ravnborg 
Signed-off-by: Noralf Trønnes 
---


Acked-by: David Lechner 


___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 8/9] drm/tinydrm: Move mipi-dbi

[David Lechner]

On 7/22/19 5:43 AM, Noralf Trønnes wrote:

This moves mipi-dbi to be a core helper with the name drm_mipi_dbi.

Fixup include's in drivers.
Move the docs entry and delete tinydrm.rst.
Delete the last tinydrm todo entry.

v2: Make DRM_MIPI_DBI tristate to enable it being built as a module.

Cc: Eric Anholt 
Cc: David Lechner 
Reviewed-by: Sam Ravnborg 
Signed-off-by: Noralf Trønnes 
---


Acked-by: David Lechner 


___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 4/9] drm/tinydrm: Split struct mipi_dbi in two

[David Lechner]

On 7/22/19 5:43 AM, Noralf Trønnes wrote:

Split struct mipi_dbi into an interface part and a display pipeline part.
The interface part can be used by drivers that need to initialize the
controller, but that won't upload the framebuffer over this interface.

MIPI DBI supports 3 interface types:
- A. Motorola 6800 type parallel bus
- B. Intel 8080 type parallel bus
- C. SPI type with 3 options:

I've embedded the SPI type specifics in the mipi_dbi struct to avoid
adding unnecessary complexity. If more interface types will be supported
in the future, the type specifics might have to be split out.

Rename functions to match the new struct mipi_dbi_dev:
- drm_to_mipi_dbi() -> drm_to_mipi_dbi_dev().
- mipi_dbi_init*() -> mipi_dbi_dev_init*().

Cc: Eric Anholt 
Cc: David Lechner 
Reviewed-by: Sam Ravnborg 
Signed-off-by: Noralf Trønnes 
---


Acked-by: David Lechner 


___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 3/9] drm/tinydrm: Rename remaining variable mipi -> dbidev

[David Lechner]

On 7/22/19 5:43 AM, Noralf Trønnes wrote:

struct mipi_dbi is going to be split into an interface part and a display
pipeline part. The interface part can be used by drivers that need to
initialize the controller, but that won't upload the framebuffer over
this interface.

tinydrm uses the variable name 'mipi' but this is not a good name since
MIPI refers to a lot of standards. This patch changes the variable name
to 'dbidev' where it refers to the pipeline part of struct mipi_dbi.

Cc: Eric Anholt 
Cc: David Lechner 
Reviewed-by: Sam Ravnborg 
Signed-off-by: Noralf Trønnes 
---


Acked-by: David Lechner 

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 2/9] drm/tinydrm: Rename variable mipi -> dbi

[David Lechner]

On 7/22/19 5:43 AM, Noralf Trønnes wrote:

struct mipi_dbi is going to be split into an interface part and a display
pipeline part. The interface part can be used by drivers that need to
initialize the controller, but that won't upload the framebuffer over
this interface.

tinydrm uses the variable name 'mipi' but this is not a good name since
MIPI refers to a lot of standards. This patch changes the variable name
to 'dbi' where it refers to the interface part of struct mipi_dbi.

Functions that use both future parts will have both variables temporarily
pointing to the same structure.

Cc: Eric Anholt 
Cc: David Lechner 
Reviewed-by: Sam Ravnborg 
Signed-off-by: Noralf Trønnes 
---


Acked-by: David Lechner 


___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 10/11] drm/tinydrm/mipi-dbi: Add mipi_dbi_init_with_formats()

[David Lechner]

On 7/19/19 10:59 AM, Noralf Trønnes wrote:

The MIPI DBI standard support more pixel formats than what this helper
supports. Add an init function that lets the driver use different
format(s). This avoids open coding mipi_dbi_init() in st7586.

st7586 sets preferred_depth but this is not necessary since it only
supports one format.

v2: Forgot to remove the mipi->rotation assignment in st7586,
 mipi_dbi_init_with_formats() handles it.

Cc: David Lechner 
Acked-by: David Lechner 
Signed-off-by: Noralf Trønnes 
---
  drivers/gpu/drm/tinydrm/mipi-dbi.c | 91 +-
  drivers/gpu/drm/tinydrm/st7586.c   | 33 ++-
  include/drm/tinydrm/mipi-dbi.h |  5 ++
  3 files changed, 74 insertions(+), 55 deletions(-)



Tested whole series on st7586. Seems to be still working.

Just putting Tested-by here since this patch is the only
one that changed st7586 significantly.

Tested-by: David Lechner 

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 10/10] drm/tinydrm: Move tinydrm_display_pipe_init() to mipi-dbi

[David Lechner]

On 7/17/19 6:58 AM, Noralf Trønnes wrote:

tinydrm_display_pipe_init() has only one user now, so move it to mipi-dbi.

Changes:
- Remove drm_connector_helper_funcs.detect, it's always connected.
- Store the connector and mode in mipi_dbi instead of it's own struct.

Otherwise remove some leftover tinydrm-helpers.h inclusions.


Were the uses of tinydrm-helpers.h removed in this series? If so, then th
#include should probably be removed at the same time. If not, removing the
#include lines could just be an independent patch from this series.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 09/10] drm/tinydrm/mipi-dbi: Add mipi_dbi_init_with_formats()

[David Lechner]

On 7/17/19 6:58 AM, Noralf Trønnes wrote:

The MIPI DBI standard support more pixel formats than what this helper
supports. Add an init function that lets the driver use different
format(s). This avoids open coding mipi_dbi_init() in st7586.

st7586 sets preferred_depth but this is not necessary since it only
supports one format.


Although that might not always be the case. FYI, we are finding that
having XRGB for a 2bpp grayscale display is not the greatest. When
we want to do direct drawing on the screen, we haven't found very good
support in existing graphics libraries for embedded systems for this
format. If I had more free time, I would like to look at adding
grayscale support to DRM.



Cc: David Lechner 
Signed-off-by: Noralf Trønnes 
---


Acked-by: David Lechner 

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 07/10] drm/tinydrm: Move tinydrm_machine_little_endian()

[David Lechner]

On 7/17/19 6:58 AM, Noralf Trønnes wrote:

The tinydrm helper is going away so move it into the only user mipi-dbi.

Signed-off-by: Noralf Trønnes 
---
  drivers/gpu/drm/tinydrm/mipi-dbi.c| 15 ---
  include/drm/tinydrm/tinydrm-helpers.h | 15 ---
  2 files changed, 12 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/tinydrm/mipi-dbi.c 
b/drivers/gpu/drm/tinydrm/mipi-dbi.c
index 6a8f2d66377f..73db287e5c52 100644
--- a/drivers/gpu/drm/tinydrm/mipi-dbi.c
+++ b/drivers/gpu/drm/tinydrm/mipi-dbi.c
@@ -628,6 +628,15 @@ u32 mipi_dbi_spi_cmd_max_speed(struct spi_device *spi, 
size_t len)
  }
  EXPORT_SYMBOL(mipi_dbi_spi_cmd_max_speed);
  
+static bool mipi_dbi_machine_little_endian(void)

+{
+#if defined(__LITTLE_ENDIAN)
+   return true;
+#else
+   return false;
+#endif
+}
+


I'm kind of surprised that there isn't something like this elsewhere
in the kernel already. The way this function is being used it kind of
seems like it should be static __always_inline (or a macro) so that
the compiler can do a better job optimizing the code (although it is
a very minor improvement).
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 06/10] drm/tinydrm: Move tinydrm_spi_transfer()

[David Lechner]

On 7/17/19 6:58 AM, Noralf Trønnes wrote:

This is only used by mipi-dbi drivers so move it there.

The reason this isn't moved to the SPI subsystem is that it will in a
later patch pass a dummy rx buffer for SPI controllers that need this.
Low memory boards (64MB) can run into a problem allocating such a "large"
contiguous buffer on every transfer after a long up time.
This leaves a very specific use case, so we'll keep the function here.
mipi-dbi will first go through a refactoring though, before this will
be done.

Remove SPI todo entry now that we're done with the tinydrm.ko SPI code.

Additionally move the mipi_dbi_spi_init() declaration to the other SPI
functions.

Cc: David Lechner 
Signed-off-by: Noralf Trønnes 
---


Acked-by: : David Lechner 

I assume that the comments here might have something to do with the
issue[1] I raised a while back? Should I dust that patch off and resend
it after this series lands?

[1]: 
https://lore.kernel.org/lkml/1519082461-32405-1-git-send-email-da...@lechnology.com/
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 01/10] drm: Add SPI connector type

[David Lechner]

On 7/17/19 6:58 AM, Noralf Trønnes wrote:

tinydrm drivers announce DRM_MODE_CONNECTOR_VIRTUAL for its SPI drivers.
Stop lying and add a SPI connector type

Cc: David Lechner 
Signed-off-by: Noralf Trønnes 
---


Acked-by: David Lechner 

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 05/10] drm/tinydrm: Clean up tinydrm_spi_transfer()

[David Lechner]

On 7/17/19 6:58 AM, Noralf Trønnes wrote:

Prep work before moving the function to mipi-dbi.

tinydrm_spi_transfer() was made to support one class of drivers in
drivers/staging/fbtft that has not been converted to DRM yet, so strip
away the unused functionality:
- Start byte (header) is not used.
- No driver relies on the automatic 16-bit byte swapping on little endian
   machines with SPI controllers only supporting 8 bits per word.

Other changes:
- No need to initialize ret
- No need for the WARN since mipi-dbi only uses 8 and 16 bpw.
- Use spi_message_init_with_transfers()

Cc: David Lechner 
Signed-off-by: Noralf Trønnes 
---


Acked-by: : David Lechner 
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v3 4/5] drm/tinydrm: Use damage helper for dirtyfb

[David Lechner]

On 1/14/19 10:36 PM, Noralf Trønnes wrote:

This switches to drm_atomic_helper_dirtyfb() as the framebuffer dirty
handler. All flushing will now happen in the pipe functions.

Also enable the damage plane property for all except repaper which can
only do full updates.

ili9225:
This change made ili9225_init() equal to mipi_dbi_init() so use it.

v3: Include vblank header (Sam)
 ili9225 and st7586 can't use mipi_dbi_enable_flush() (David)

v2: Remove fb check in mipi_dbi_enable_flush() it can't be NULL
 (kbuild test robot)

Cc: David Lechner 
Cc: Eric Anholt 
Signed-off-by: Noralf Trønnes 
Acked-by: Daniel Vetter 
Reviewed-by: Sam Ravnborg 
---


Tested-by: David Lechner 
Reviewed-by: David Lechner 

Tested st7586 on LEGO MINDSTORMS EV3 and ili9225 on BeagleBone
Green (requires "spi: omap2-mcspi: Fix DMA and FIFO event trigger
size mismatch" to work correctly because of recent bug).

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 4/5] drm/tinydrm: Use damage helper for dirtyfb

[David Lechner]

On 1/14/19 4:53 PM, Noralf Trønnes wrote:



Den 14.01.2019 23.33, skrev David Lechner:

On 1/14/19 3:50 PM, David Lechner wrote:

On 1/14/19 10:13 AM, Noralf Trønnes wrote:


I see that you have this call chain:
st7586_pipe_enable() -> mipi_dbi_enable_flush() -> mipi_dbi_fb_dirty().

That doesn't look safe. The st7586 driver allocates a tx_buf with size:
 size_t bufsize = (mode->vdisplay + 2) / 3 * mode->hdisplay;

whereas mipi_dbi_enable_flush() will trigger a copy to tx_buf with len:
fb->width * fb->height * 2

It looks like you're writing zeroes way past the end of the buffer.

Noralf.



Thanks! That does indeed seem to be the problem. I'll put together
a patch to fix this. I'm thinking it will be easier to make the
fix before applying this series so that it will be easier to
backport.



Well, now that I am looking into it more, I see that the problem
was not preexisting. This patch ("drm/tinydrm: Use damage helper
for dirtyfb") also changes mipi_dbi_enable_flush() from calling
tdev->fb_dirty() to mipi_dbi_fb_dirty().

Perhaps we should not be dropping tdev->fb_dirty()?


I want to get rid of tinydrm_device, to avoid tinydrm being like a
mid-layer. My goal is to make tinydrm just a collection of tiny regular
DRM drivers.



OK. Perhaps we should add a comment to mipi_dbi_enable_flush() to
say that it should not be used by drivers with a custom dirty function.

Or perhaps we could pass an optional parameter with the custom dirty
function to mipi_dbi_enable_flush() like this:

diff --git a/drivers/gpu/drm/tinydrm/hx8357d.c 
b/drivers/gpu/drm/tinydrm/hx8357d.c
index 8bbd0beafc6a..4b9981ffe5a4 100644
--- a/drivers/gpu/drm/tinydrm/hx8357d.c
+++ b/drivers/gpu/drm/tinydrm/hx8357d.c
@@ -170,7 +170,7 @@ static void yx240qv29_enable(struct drm_simple_display_pipe 
*pipe,
break;
}
mipi_dbi_command(mipi, MIPI_DCS_SET_ADDRESS_MODE, addr_mode);
-   mipi_dbi_enable_flush(mipi, crtc_state, plane_state);
+   mipi_dbi_enable_flush(mipi, crtc_state, plane_state, NULL);
 }

 static const struct drm_simple_display_pipe_funcs hx8357d_pipe_funcs = {
diff --git a/drivers/gpu/drm/tinydrm/ili9225.c 
b/drivers/gpu/drm/tinydrm/ili9225.c
index aa1376a22bda..bebfd0d01340 100644
--- a/drivers/gpu/drm/tinydrm/ili9225.c
+++ b/drivers/gpu/drm/tinydrm/ili9225.c
@@ -270,7 +270,7 @@ static void ili9225_pipe_enable(struct 
drm_simple_display_pipe *pipe,

ili9225_command(mipi, ILI9225_DISPLAY_CONTROL_1, 0x1017);

-   mipi_dbi_enable_flush(mipi, crtc_state, plane_state);
+   mipi_dbi_enable_flush(mipi, crtc_state, plane_state, ili9225_fb_dirty);
 }

 static void ili9225_pipe_disable(struct drm_simple_display_pipe *pipe)
diff --git a/drivers/gpu/drm/tinydrm/ili9341.c 
b/drivers/gpu/drm/tinydrm/ili9341.c
index 713bb2dd7e04..ecfb8ff02a40 100644
--- a/drivers/gpu/drm/tinydrm/ili9341.c
+++ b/drivers/gpu/drm/tinydrm/ili9341.c
@@ -126,7 +126,7 @@ static void yx240qv29_enable(struct drm_simple_display_pipe 
*pipe,
}
addr_mode |= ILI9341_MADCTL_BGR;
mipi_dbi_command(mipi, MIPI_DCS_SET_ADDRESS_MODE, addr_mode);
-   mipi_dbi_enable_flush(mipi, crtc_state, plane_state);
+   mipi_dbi_enable_flush(mipi, crtc_state, plane_state, NULL);
 }

 static const struct drm_simple_display_pipe_funcs ili9341_pipe_funcs = {
diff --git a/drivers/gpu/drm/tinydrm/mi0283qt.c 
b/drivers/gpu/drm/tinydrm/mi0283qt.c
index 82a92ec9ae3c..c81aa1fbc2ad 100644
--- a/drivers/gpu/drm/tinydrm/mi0283qt.c
+++ b/drivers/gpu/drm/tinydrm/mi0283qt.c
@@ -134,7 +134,7 @@ static void mi0283qt_enable(struct drm_simple_display_pipe 
*pipe,
}
addr_mode |= ILI9341_MADCTL_BGR;
mipi_dbi_command(mipi, MIPI_DCS_SET_ADDRESS_MODE, addr_mode);
-   mipi_dbi_enable_flush(mipi, crtc_state, plane_state);
+   mipi_dbi_enable_flush(mipi, crtc_state, plane_state, NULL);
 }

 static const struct drm_simple_display_pipe_funcs mi0283qt_pipe_funcs = {
diff --git a/drivers/gpu/drm/tinydrm/mipi-dbi.c 
b/drivers/gpu/drm/tinydrm/mipi-dbi.c
index 680692a83f94..399c66b3f0d7 100644
--- a/drivers/gpu/drm/tinydrm/mipi-dbi.c
+++ b/drivers/gpu/drm/tinydrm/mipi-dbi.c
@@ -287,6 +287,7 @@ EXPORT_SYMBOL(mipi_dbi_pipe_update);
  * @mipi: MIPI DBI structure
  * @crtc_state: CRTC state
  * @plane_state: Plane state
+ * @dirty: Optional custom dirty function
  *
  * This function sets _dbi->enabled, flushes the whole framebuffer and
  * enables the backlight. Drivers can use this in their
@@ -294,7 +295,9 @@ EXPORT_SYMBOL(mipi_dbi_pipe_update);
  */
 void mipi_dbi_enable_flush(struct mipi_dbi *mipi,
   struct drm_crtc_state *crtc_state,
-  struct drm_plane_state *plane_state)
+  struct drm_plane_state *plane_state,
+  void (*dirty)(struct drm_framebuffer *fb,
+struct drm_rect *rect))
 {
struct drm_framebuffer *fb = plane_st

Re: [PATCH v2 4/5] drm/tinydrm: Use damage helper for dirtyfb

[David Lechner]

On 1/14/19 4:43 PM, Noralf Trønnes wrote:



Den 14.01.2019 23.33, skrev David Lechner:

On 1/14/19 3:50 PM, David Lechner wrote:

On 1/14/19 10:13 AM, Noralf Trønnes wrote:


I see that you have this call chain:
st7586_pipe_enable() -> mipi_dbi_enable_flush() -> mipi_dbi_fb_dirty().

That doesn't look safe. The st7586 driver allocates a tx_buf with size:
 size_t bufsize = (mode->vdisplay + 2) / 3 * mode->hdisplay;

whereas mipi_dbi_enable_flush() will trigger a copy to tx_buf with len:
fb->width * fb->height * 2

It looks like you're writing zeroes way past the end of the buffer.

Noralf.



Thanks! That does indeed seem to be the problem. I'll put together
a patch to fix this. I'm thinking it will be easier to make the
fix before applying this series so that it will be easier to
backport.



Well, now that I am looking into it more, I see that the problem
was not preexisting. This patch ("drm/tinydrm: Use damage helper
for dirtyfb") also changes mipi_dbi_enable_flush() from calling
tdev->fb_dirty() to mipi_dbi_fb_dirty().

Perhaps we should not be dropping tdev->fb_dirty()?


Ah, now it makes sense. I thought it strange that you hadn't experienced
the corruption before.

How about this change?

diff --git a/drivers/gpu/drm/tinydrm/st7586.c
b/drivers/gpu/drm/tinydrm/st7586.c
index d39417b74e8b..01a8077954b3 100644
--- a/drivers/gpu/drm/tinydrm/st7586.c
+++ b/drivers/gpu/drm/tinydrm/st7586.c
@@ -177,6 +177,13 @@ static void st7586_pipe_enable(struct
drm_simple_display_pipe *pipe,
  {
 struct tinydrm_device *tdev = pipe_to_tinydrm(pipe);
 struct mipi_dbi *mipi = mipi_dbi_from_tinydrm(tdev);
+   struct drm_framebuffer *fb = plane_state->fb;
+   struct drm_rect rect = {
+   .x1 = 0,
+   .x2 = fb->width,
+   .y1 = 0,
+   .y2 = fb->height,
+   };
 int ret;
 u8 addr_mode;

@@ -233,9 +240,10 @@ static void st7586_pipe_enable(struct
drm_simple_display_pipe *pipe,

 msleep(100);

-   mipi_dbi_command(mipi, MIPI_DCS_SET_DISPLAY_ON);
+   mipi->enabled = true;
+   st7586_fb_dirty(fb, );

-   mipi_dbi_enable_flush(mipi, crtc_state, plane_state);
+   mipi_dbi_command(mipi, MIPI_DCS_SET_DISPLAY_ON);
  }

  static void st7586_pipe_disable(struct drm_simple_display_pipe *pipe)





That looks good. I also noticed that ili9225 has a custom dirty
function, so it will need a similar change as well.

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 4/5] drm/tinydrm: Use damage helper for dirtyfb

[David Lechner]

On 1/14/19 3:50 PM, David Lechner wrote:

On 1/14/19 10:13 AM, Noralf Trønnes wrote:


I see that you have this call chain:
st7586_pipe_enable() -> mipi_dbi_enable_flush() -> mipi_dbi_fb_dirty().

That doesn't look safe. The st7586 driver allocates a tx_buf with size:
size_t bufsize = (mode->vdisplay + 2) / 3 * mode->hdisplay;

whereas mipi_dbi_enable_flush() will trigger a copy to tx_buf with len:
fb->width * fb->height * 2

It looks like you're writing zeroes way past the end of the buffer.

Noralf.



Thanks! That does indeed seem to be the problem. I'll put together
a patch to fix this. I'm thinking it will be easier to make the
fix before applying this series so that it will be easier to
backport.



Well, now that I am looking into it more, I see that the problem
was not preexisting. This patch ("drm/tinydrm: Use damage helper
for dirtyfb") also changes mipi_dbi_enable_flush() from calling
tdev->fb_dirty() to mipi_dbi_fb_dirty().

Perhaps we should not be dropping tdev->fb_dirty()?
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 4/5] drm/tinydrm: Use damage helper for dirtyfb

[David Lechner]

On 1/11/19 2:12 PM, Noralf Trønnes wrote:

This switches to drm_atomic_helper_dirtyfb() as the framebuffer dirty
handler. All flushing will now happen in the pipe functions.

Also enable the damage plane property for all except repaper which can
only do full updates.

ili9225:
This change made ili9225_init() equal to mipi_dbi_init() so use it.

v2: Remove fb check in mipi_dbi_enable_flush() it can't be NULL
 (kbuild test robot)

Cc: David Lechner 
Cc: Eric Anholt 
Signed-off-by: Noralf Trønnes 
Acked-by: Sam Ravnborg 
Acked-by: Daniel Vetter 
---



...


diff --git a/drivers/gpu/drm/tinydrm/st7586.c b/drivers/gpu/drm/tinydrm/st7586.c
index a52bc3b02606..50e370ce5a59 100644
--- a/drivers/gpu/drm/tinydrm/st7586.c
+++ b/drivers/gpu/drm/tinydrm/st7586.c
@@ -17,6 +17,7 @@
  #include 
  #include 
  
+#include 

  #include 
  #include 
  #include 
@@ -112,23 +113,15 @@ static int st7586_buf_copy(void *dst, struct 
drm_framebuffer *fb,
return ret;
  }
  
-static int st7586_fb_dirty(struct drm_framebuffer *fb,

-  struct drm_file *file_priv, unsigned int flags,
-  unsigned int color, struct drm_clip_rect *clips,
-  unsigned int num_clips)
+static void st7586_fb_dirty(struct drm_framebuffer *fb, struct drm_rect *rect)
  {
struct tinydrm_device *tdev = fb->dev->dev_private;
struct mipi_dbi *mipi = mipi_dbi_from_tinydrm(tdev);
-   struct drm_rect clip;
-   struct drm_rect *rect = 


This function later modifies the fields of the struct pointed to by rect.
Are all callers of this function OK with having it modified or should we
make a local copy of rect and modify that instead?
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 4/5] drm/tinydrm: Use damage helper for dirtyfb

[David Lechner]

On 1/14/19 10:13 AM, Noralf Trønnes wrote:


I see that you have this call chain:
st7586_pipe_enable() -> mipi_dbi_enable_flush() -> mipi_dbi_fb_dirty().

That doesn't look safe. The st7586 driver allocates a tx_buf with size:
size_t bufsize = (mode->vdisplay + 2) / 3 * mode->hdisplay;

whereas mipi_dbi_enable_flush() will trigger a copy to tx_buf with len:
fb->width * fb->height * 2

It looks like you're writing zeroes way past the end of the buffer.

Noralf.



Thanks! That does indeed seem to be the problem. I'll put together
a patch to fix this. I'm thinking it will be easier to make the
fix before applying this series so that it will be easier to
backport.

FWIW, here is the change I made on top of this series.

diff --git a/drivers/gpu/drm/tinydrm/st7586.c b/drivers/gpu/drm/tinydrm/st7586.c
index 50e370ce5a59..a6cad8f1d2c9 100644
--- a/drivers/gpu/drm/tinydrm/st7586.c
+++ b/drivers/gpu/drm/tinydrm/st7586.c
@@ -176,6 +176,12 @@ static void st7586_pipe_enable(struct 
drm_simple_display_pipe *pipe,
 {
struct tinydrm_device *tdev = pipe_to_tinydrm(pipe);
struct mipi_dbi *mipi = mipi_dbi_from_tinydrm(tdev);
+   struct drm_rect rect = {
+   .x1 = 0,
+   .x2 = plane_state->fb->width,
+   .y1 = 0,
+   .y2 = plane_state->fb->height,
+   };
int ret;
u8 addr_mode;

@@ -234,7 +240,7 @@ static void st7586_pipe_enable(struct 
drm_simple_display_pipe *pipe,

mipi_dbi_command(mipi, MIPI_DCS_SET_DISPLAY_ON);

-   mipi_dbi_enable_flush(mipi, crtc_state, plane_state);
+   st7586_fb_dirty(plane_state->fb, );
 }

 static void st7586_pipe_disable(struct drm_simple_display_pipe *pipe)
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 4/5] drm/tinydrm: Use damage helper for dirtyfb

[David Lechner]

On 1/13/19 3:19 PM, David Lechner wrote:

On 1/11/19 2:12 PM, Noralf Trønnes wrote:

This switches to drm_atomic_helper_dirtyfb() as the framebuffer dirty
handler. All flushing will now happen in the pipe functions.

Also enable the damage plane property for all except repaper which can
only do full updates.

ili9225:
This change made ili9225_init() equal to mipi_dbi_init() so use it.

v2: Remove fb check in mipi_dbi_enable_flush() it can't be NULL
 (kbuild test robot)

Cc: David Lechner 
Cc: Eric Anholt 
Signed-off-by: Noralf Trønnes 
Acked-by: Sam Ravnborg 
Acked-by: Daniel Vetter 
---


Tested-by: David Lechner 
Reviewed-by: David Lechner 

Tested with ST7586 on LEGO MINDSTORMS EV3 and ILI9225 on
BeagleBone Green. (FWIW, I encounter other issues that had to
be fixed to make things work on both of these, but none were
actually related to this series.)



Actually, I have to take this back. It appears that the problems
that I had on LEGO MINDSTORMS EV3 are actually caused by this
series. I did a git bisect to be sure and ended up with this
patch as the bad commit. I'm guessing that the damage helper
must be causing some kind of memory corruption. Here is the
crash I am getting:

Unable to handle kernel NULL pointer dereference at virtual address 0004
pgd = (ptrval)
[0004] *pgd=
Internal error: Oops: 5 [#1] PREEMPT ARM
Modules linked in:
CPU: 0 PID: 1 Comm: swapper Not tainted 5.0.0-rc1-00125-ge9acadba5409 #1389
Hardware name: Generic DA850/OMAP-L138/AM18x
PC is at rb_insert_color+0x1c/0x1a4
LR is at kernfs_link_sibling+0x94/0xcc
pc : []lr : []psr: 6013
sp : c2831b38  ip :   fp : c06b762c
r10:   r9 : c06b835c  r8 : 
r7 : c2963f00  r6 : c066b028  r5 : c2016cc0  r4 : 
r3 :   r2 : c2983010  r1 : c2963f2c  r0 : c2016cd0
Flags: nZCv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment none
Control: 0005317f  Table: c0004000  DAC: 0053
Process swapper (pid: 1, stack limit = 0x(ptrval))
Stack: (0xc2831b38 to 0xc2832000)
1b20:    c2016cc0
1b40: c066b028 c014bfdc c2016cc0 c2963f00 c066b028 c014c768 c2963f00 c014c448
1b60:  c2016cc0 c2963f00 c066b028 c2963f00 c014c860  0001
1b80: c0589938 c2b4b408  c014ec70  c2b4b408  c04c4cb0
1ba0: 070f   9fd04dd9  c2b4b408 c066b028 
1bc0: c293ac98 c04b58f8 c059081c  c2b4b408 c066b028  
1be0:  c04b5d64 c2831c08 9fd04dd9 c2b4b3c0 c293ac80 c2bd16c0 c2b4b408
1c00: c00d650c c059081c c2bd16c0 c28e3a80 c2b4b3c0  c2b4b3c0 
1c20: c06b7634 0002 c06b835c c00d72f8  c00b0c24  0074
1c40: c0590728  c0590728 c2b4b3c0 0074 c0590728 0002 
1c60: c06b762c c00b0958  00380bc6  c06bbf1c c2bd9c00 c2bfe000
1c80: c06bbf14 000c   c2831e0c c00b0a90  
1ca0:   003b2580 c017d6e4    c2bfe000
1cc0: c2bd9c00  003b2580   c01971a0 1103 
1ce0: c2bd8400 0400 0400 9fd04dd9  000a 0002 
1d00:      0001 0a04 0032
1d20:  000c 0004 c00af550 c2bd9ecc 9fd04dd9 c2404480 c2bd9eb4
1d40: c2bd8400 c2404480 0002 0001   0001 
1d60:       0001 
1d80: 00380bc6  003b257f  0077  0200 0002
1da0: 0001   0401 c2bfe22c   
1dc0:   c2012400 c24be100 1000  c2404480 
1de0: 4003 9fd04dd9  c2bd9c00 c2404480 0083 c24044fc 8000
1e00:  0020 8000 c00e4d88 c2404480   c0190afc
1e20: c2bd0be0 c0692898 c0692898  0020 c0190b10 c0194f48 c2bd0be0
1e40: c066b370 c00e568c c29f5380 c01002d4 c29f5380 c2bd0be0 8000 c0100488
1e60: c0692898  c066b028 c2bd0be0 c2bd0bc0 c01033ec  ff00
1e80: 0a00 c0575ff4 c2bd0be0 c281a010 c2417098 c2bd0be0 000a 
1ea0: 000a 9fd04dd9 000a c2bd0be0 c2bd0bc0  c0575ff8 8000
1ec0: c066b028 8000 c0575ff4 c0104178   c2014000 c2014005
1ee0: c0575ff8 c3fb1280 c0652868 c062b1ec  c01013d8 c24c3558 
1f00:  6180 c24c3558 c00f17c4 e10c76ac 0b32 c2858015 c281a010
1f20: c2415da8 9fd04dd9  0002 c06ad310 c066b028 c066da68 
1f40:    c062b478  c0037200 00306b6c 9fd0
1f60: c0576098 9fd04dd9 0002 c06ad310 c0652878   
1f80:    c062b5e4    
1fa0: c04c7860 c04c7868  c00090e0    
1fc0:        
1fe0: 

Re: [PATCH v2 4/5] drm/tinydrm: Use damage helper for dirtyfb

[David Lechner]

On 1/11/19 2:12 PM, Noralf Trønnes wrote:

This switches to drm_atomic_helper_dirtyfb() as the framebuffer dirty
handler. All flushing will now happen in the pipe functions.

Also enable the damage plane property for all except repaper which can
only do full updates.

ili9225:
This change made ili9225_init() equal to mipi_dbi_init() so use it.

v2: Remove fb check in mipi_dbi_enable_flush() it can't be NULL
 (kbuild test robot)

Cc: David Lechner 
Cc: Eric Anholt 
Signed-off-by: Noralf Trønnes 
Acked-by: Sam Ravnborg 
Acked-by: Daniel Vetter 
---


Tested-by: David Lechner 
Reviewed-by: David Lechner 

Tested with ST7586 on LEGO MINDSTORMS EV3 and ILI9225 on
BeagleBone Green. (FWIW, I encounter other issues that had to
be fixed to make things work on both of these, but none were
actually related to this series.)

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 4/4] drm/tinydrm: Use damage helper for dirtyfb

[David Lechner]

On 1/9/19 11:49 AM, Noralf Trønnes wrote:

This switches to drm_atomic_helper_dirtyfb() as the framebuffer dirty
handler. All flushing will now happen in the pipe functions.

Also enable the damage plane property for all except repaper which can
only do full updates.

ili9225:
This change made ili9225_init() equal to mipi_dbi_init() so use it.

Cc: David Lechner 
Cc: Eric Anholt 
Signed-off-by: Noralf Trønnes 
---


...


+static void ili9225_pipe_update(struct drm_simple_display_pipe *pipe,
+   struct drm_plane_state *old_state)
+{
+   struct drm_plane_state *state = pipe->plane.state;
+   struct drm_crtc *crtc = >crtc;
+   struct drm_rect rect;
+
+   if (drm_atomic_helper_damage_merged(old_state, state, ))
+   ili9225_fb_dirty(state->fb, );
+
+   if (crtc->state->event) {
+   spin_lock_irq(>dev->event_lock);
+   drm_crtc_send_vblank_event(crtc, crtc->state->event);
+   spin_unlock_irq(>dev->event_lock);
+   crtc->state->event = NULL;
+   }
+}


It looks like this function body is repeated 4 times with the only
difference being the dirty function. Perhaps a helper function is
called for?

Also, I was going to test out this series the displays that I have,
but I guess I will wait until it is easier to apply the patches.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 13/14] drm/tinydrm: do not reply on drmP.h from drm_gem_cma_helper.h

[David Lechner]

On 12/30/18 11:48 AM, Sam Ravnborg wrote:

drmP.h was the only header file in the past and a lot
of files rely on that drmP.h defines everything.
The goal is to one day to delete drmP.h and
as a step towards this it will no longer be included in the
headers files in include/drm/

To prepare tinydrm/ for this add dependencies that
othwewise was pulled in by drmP.h from drm_gem_cma_helper.h

To avoid that tinydrm.h became "include everything",
push include files to the individual drivers.

Signed-off-by: Sam Ravnborg 
Cc: "Noralf Trønnes" 
Cc: David Airlie 
Cc: Eric Anholt 
Cc: David Lechner 
Cc: Daniel Vetter 
---


Acked-by: David Lechner 


___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v1 7/7] drm: remove include of drmP.h from drm_gem_cma_helper.h

[David Lechner]

On 12/26/18 3:03 PM, Sam Ravnborg wrote:

Fix fallout in various files/drivers.



What fallout is being fixed? It would be helpful if we received the full
patch series for context. It would also be nice to have a more detailed
description in this commit message.

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v5 5/5] drm/tinydrm: Switch from CMA to shmem buffers

[David Lechner]

On 10/26/2018 05:38 PM, Noralf Trønnes wrote:
> 
> Den 17.10.2018 15.04, skrev Noralf Trønnes:
>> This move makes tinydrm useful for more drivers. tinydrm doesn't need
>> continuous memory, but at the time it was convenient to use the CMA
>> library. The spi core can do dma on is_vmalloc() addresses making this
>> possible.
>>
>> Cc: David Lechner 
>> Signed-off-by: Noralf Trønnes 
>> Acked-by: David Lechner 
>> Tested-by: David Lechner 
>> ---
> 
> David,
> FYI This series is scratched.
> See the shmem helper patch thread for details.
> 

Yes, I saw that. Thank you.

I don't suppose there is a way to configure the DMA controller to do
the byte swapping?

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v3 1/2] drm: Add library for shmem backed GEM objects

[David Lechner]

On 09/11/2018 07:43 AM, Noralf Trønnes wrote:

This adds a library for shmem backed GEM objects with the necessary
drm_driver callbacks.

Signed-off-by: Noralf Trønnes 
---



...


+static int drm_gem_shmem_vmap_locked(struct drm_gem_shmem_object *shmem)
+{
+   struct drm_gem_object *obj = >base;
+   int ret;
+
+   if (shmem->vmap_use_count++ > 0)
+   return 0;
+
+   ret = drm_gem_shmem_get_pages(shmem);
+   if (ret)
+   goto err_zero_use;
+
+   if (obj->import_attach) {
+   shmem->vaddr = dma_buf_vmap(obj->import_attach->dmabuf);
+   } else {
+   pgprot_t prot;
+
+   switch (shmem->cache_mode) {
+   case DRM_GEM_SHMEM_BO_UNKNOWN:
+   DRM_DEBUG_KMS("Can't vmap cache mode is unknown\n");
+   ret = -EINVAL;
+   goto err_put_pages;
+
+   case DRM_GEM_SHMEM_BO_WRITECOMBINED:
+   prot = pgprot_writecombine(PAGE_KERNEL);
+   break;
+
+   case DRM_GEM_SHMEM_BO_UNCACHED:
+   prot = pgprot_noncached(PAGE_KERNEL);
+   break;
+
+   case DRM_GEM_SHMEM_BO_CACHED:
+   prot = PAGE_KERNEL;
+   break;
+   }
+
+   shmem->vaddr = vmap(shmem->pages, obj->size >> PAGE_SHIFT, 
VM_MAP, prot);


I get a gcc warning here:

/home/david/work/ev3dev2/ev3dev-kernel/drivers/gpu/drm/drm_gem_shmem_helper.c:220:18:
 warning: ‘prot’ may be used uninitialized in this function 
[-Wmaybe-uninitialized]
   shmem->vaddr = vmap(shmem->pages, obj->size >> PAGE_SHIFT, VM_MAP, prot);
  ^
/home/david/work/ev3dev2/ev3dev-kernel/drivers/gpu/drm/drm_gem_shmem_helper.c:199:12:
 note: ‘prot’ was declared here
   pgprot_t prot;
^~~~

---

And since I am making a comment anyway, it is not clear to me
what BO means in the enum names. I didn't see any hints in the
doc comments either.



+   }
+
+   if (!shmem->vaddr) {
+   DRM_DEBUG_KMS("Failed to vmap pages\n");
+   ret = -ENOMEM;
+   goto err_put_pages;
+   }
+
+   return 0;
+
+err_put_pages:
+   drm_gem_shmem_put_pages(shmem);
+err_zero_use:
+   shmem->vmap_use_count = 0;
+
+   return ret;
+}

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v3 2/2] drm/tinydrm: Switch from CMA to shmem buffers

[David Lechner]

On 09/11/2018 07:44 AM, Noralf Trønnes wrote:

This move makes tinydrm useful for more drivers. tinydrm doesn't need
continuous memory, but at the time it was convenient to use the CMA
library. The spi core can do dma on is_vmalloc() addresses making this
possible.

Cc: David Lechner 
Signed-off-by: Noralf Trønnes 
---


Acked-by: David Lechner 
Tested-by: David Lechner 

tested on st7586


___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/tinydrm: add backlight dependency for ili9341

[David Lechner]

On 07/09/2018 10:20 AM, Arnd Bergmann wrote:

This tinydrm driver fails to link without the backlight support:

drivers/gpu/drm/tinydrm/ili9341.o: In function `ili9341_probe':
ili9341.c:(.text+0x578): undefined reference to `devm_of_find_backlight'

Fixes: 3fa0e8f6f960 ("drm/tinydrm: new driver for ILI9341 display panels")
Signed-off-by: Arnd Bergmann 
---


applied to drm-misc-next.

thanks
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/tinydrm: Fix doc build warnings

[David Lechner]

On 07/10/2018 12:09 PM, Noralf Trønnes wrote:


Den 10.07.2018 18.40, skrev David Lechner:

On 07/10/2018 11:31 AM, Noralf Trønnes wrote:


Den 10.07.2018 18.18, skrev David Lechner:

On 07/10/2018 10:05 AM, Noralf Trønnes wrote:

include/drm/tinydrm/tinydrm.h:34: warning: Function parameter or member 
'fb_dirty' not described in 'tinydrm_device'
drivers/gpu/drm/tinydrm/mipi-dbi.c:272: warning: Function parameter or member 
'crtc_state' not described in 'mipi_dbi_enable_flush'
drivers/gpu/drm/tinydrm/mipi-dbi.c:272: warning: Function parameter or member 
'plane_state' not described in 'mipi_dbi_enable_flush'

Move struct member docs inline so it's not missed next time.

Cc: David Lechner 
Signed-off-by: Noralf Trønnes 
---
  drivers/gpu/drm/tinydrm/mipi-dbi.c |  2 ++
  include/drm/tinydrm/tinydrm.h  | 23 +++
  2 files changed, 21 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/tinydrm/mipi-dbi.c 
b/drivers/gpu/drm/tinydrm/mipi-dbi.c
index 4d1fb31a781f..cb3441e51d5f 100644
--- a/drivers/gpu/drm/tinydrm/mipi-dbi.c
+++ b/drivers/gpu/drm/tinydrm/mipi-dbi.c
@@ -260,6 +260,8 @@ static const struct drm_framebuffer_funcs mipi_dbi_fb_funcs 
= {
  /**
   * mipi_dbi_enable_flush - MIPI DBI enable helper
   * @mipi: MIPI DBI structure
+ * @crtc_state: CRTC state
+ * @plane_state: Plane state
   *
   * This function sets _dbi->enabled, flushes the whole framebuffer and
   * enables the backlight. Drivers can use this in their
diff --git a/include/drm/tinydrm/tinydrm.h b/include/drm/tinydrm/tinydrm.h
index 56e4a916b5e8..fe9827d0ca8a 100644
--- a/include/drm/tinydrm/tinydrm.h
+++ b/include/drm/tinydrm/tinydrm.h
@@ -16,16 +16,31 @@
    /**
   * struct tinydrm_device - tinydrm device
- * @drm: DRM device
- * @pipe: Display pipe structure
- * @dirty_lock: Serializes framebuffer flushing
- * @fb_funcs: Framebuffer functions used when creating framebuffers
   */
  struct tinydrm_device {
+    /**
+ * @drm: DRM device
+ */
  struct drm_device *drm;
+
+    /**
+ * @pipe: Display pipe structure
+ */
  struct drm_simple_display_pipe pipe;
+
+    /**
+ * @dirty_lock: Serializes framebuffer flushing
+ */
  struct mutex dirty_lock;
+
+    /**
+ * @fb_funcs: Framebuffer functions used when creating framebuffers
+ */
  const struct drm_framebuffer_funcs *fb_funcs;
+
+    /**
+ * @fb_dirty: Framebuffer dirty callback
+ */
  int (*fb_dirty)(struct drm_framebuffer *framebuffer,
  struct drm_file *file_priv, unsigned flags,
  unsigned color, struct drm_clip_rect *clips,



I assume the kerneldoc parser know how to handle this?



This? What are you referring to?

This is how I build the documentation: make DOCBOOKS="" htmldocs

Noralf.



I haven't seen struct fields with doc comments on each individual
field before. What I meant to say is that I'm assuming that when
you build the htmldocs that it picks up this documentation for each
field the same as if they were documented in the doc comment for
the struct itself. In other words, the resulting documentation
looks the same either way?



Yes it does.
https://www.kernel.org/doc/html/v4.17/doc-guide/kernel-doc.html#in-line-member-documentation-comments



Nifty.

Reviewed-by: David Lechner 

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/tinydrm: Fix doc build warnings

[David Lechner]

On 07/10/2018 11:31 AM, Noralf Trønnes wrote:


Den 10.07.2018 18.18, skrev David Lechner:

On 07/10/2018 10:05 AM, Noralf Trønnes wrote:

include/drm/tinydrm/tinydrm.h:34: warning: Function parameter or member 
'fb_dirty' not described in 'tinydrm_device'
drivers/gpu/drm/tinydrm/mipi-dbi.c:272: warning: Function parameter or member 
'crtc_state' not described in 'mipi_dbi_enable_flush'
drivers/gpu/drm/tinydrm/mipi-dbi.c:272: warning: Function parameter or member 
'plane_state' not described in 'mipi_dbi_enable_flush'

Move struct member docs inline so it's not missed next time.

Cc: David Lechner 
Signed-off-by: Noralf Trønnes 
---
  drivers/gpu/drm/tinydrm/mipi-dbi.c |  2 ++
  include/drm/tinydrm/tinydrm.h  | 23 +++
  2 files changed, 21 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/tinydrm/mipi-dbi.c 
b/drivers/gpu/drm/tinydrm/mipi-dbi.c
index 4d1fb31a781f..cb3441e51d5f 100644
--- a/drivers/gpu/drm/tinydrm/mipi-dbi.c
+++ b/drivers/gpu/drm/tinydrm/mipi-dbi.c
@@ -260,6 +260,8 @@ static const struct drm_framebuffer_funcs mipi_dbi_fb_funcs 
= {
  /**
   * mipi_dbi_enable_flush - MIPI DBI enable helper
   * @mipi: MIPI DBI structure
+ * @crtc_state: CRTC state
+ * @plane_state: Plane state
   *
   * This function sets _dbi->enabled, flushes the whole framebuffer and
   * enables the backlight. Drivers can use this in their
diff --git a/include/drm/tinydrm/tinydrm.h b/include/drm/tinydrm/tinydrm.h
index 56e4a916b5e8..fe9827d0ca8a 100644
--- a/include/drm/tinydrm/tinydrm.h
+++ b/include/drm/tinydrm/tinydrm.h
@@ -16,16 +16,31 @@
    /**
   * struct tinydrm_device - tinydrm device
- * @drm: DRM device
- * @pipe: Display pipe structure
- * @dirty_lock: Serializes framebuffer flushing
- * @fb_funcs: Framebuffer functions used when creating framebuffers
   */
  struct tinydrm_device {
+    /**
+ * @drm: DRM device
+ */
  struct drm_device *drm;
+
+    /**
+ * @pipe: Display pipe structure
+ */
  struct drm_simple_display_pipe pipe;
+
+    /**
+ * @dirty_lock: Serializes framebuffer flushing
+ */
  struct mutex dirty_lock;
+
+    /**
+ * @fb_funcs: Framebuffer functions used when creating framebuffers
+ */
  const struct drm_framebuffer_funcs *fb_funcs;
+
+    /**
+ * @fb_dirty: Framebuffer dirty callback
+ */
  int (*fb_dirty)(struct drm_framebuffer *framebuffer,
  struct drm_file *file_priv, unsigned flags,
  unsigned color, struct drm_clip_rect *clips,



I assume the kerneldoc parser know how to handle this?



This? What are you referring to?

This is how I build the documentation: make DOCBOOKS="" htmldocs

Noralf.



I haven't seen struct fields with doc comments on each individual
field before. What I meant to say is that I'm assuming that when
you build the htmldocs that it picks up this documentation for each
field the same as if they were documented in the doc comment for
the struct itself. In other words, the resulting documentation
looks the same either way?

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/tinydrm: Fix doc build warnings

[David Lechner]

On 07/10/2018 10:05 AM, Noralf Trønnes wrote:

include/drm/tinydrm/tinydrm.h:34: warning: Function parameter or member 
'fb_dirty' not described in 'tinydrm_device'
drivers/gpu/drm/tinydrm/mipi-dbi.c:272: warning: Function parameter or member 
'crtc_state' not described in 'mipi_dbi_enable_flush'
drivers/gpu/drm/tinydrm/mipi-dbi.c:272: warning: Function parameter or member 
'plane_state' not described in 'mipi_dbi_enable_flush'

Move struct member docs inline so it's not missed next time.

Cc: David Lechner 
Signed-off-by: Noralf Trønnes 
---
  drivers/gpu/drm/tinydrm/mipi-dbi.c |  2 ++
  include/drm/tinydrm/tinydrm.h  | 23 +++
  2 files changed, 21 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/tinydrm/mipi-dbi.c 
b/drivers/gpu/drm/tinydrm/mipi-dbi.c
index 4d1fb31a781f..cb3441e51d5f 100644
--- a/drivers/gpu/drm/tinydrm/mipi-dbi.c
+++ b/drivers/gpu/drm/tinydrm/mipi-dbi.c
@@ -260,6 +260,8 @@ static const struct drm_framebuffer_funcs mipi_dbi_fb_funcs 
= {
  /**
   * mipi_dbi_enable_flush - MIPI DBI enable helper
   * @mipi: MIPI DBI structure
+ * @crtc_state: CRTC state
+ * @plane_state: Plane state
   *
   * This function sets _dbi->enabled, flushes the whole framebuffer and
   * enables the backlight. Drivers can use this in their
diff --git a/include/drm/tinydrm/tinydrm.h b/include/drm/tinydrm/tinydrm.h
index 56e4a916b5e8..fe9827d0ca8a 100644
--- a/include/drm/tinydrm/tinydrm.h
+++ b/include/drm/tinydrm/tinydrm.h
@@ -16,16 +16,31 @@
  
  /**

   * struct tinydrm_device - tinydrm device
- * @drm: DRM device
- * @pipe: Display pipe structure
- * @dirty_lock: Serializes framebuffer flushing
- * @fb_funcs: Framebuffer functions used when creating framebuffers
   */
  struct tinydrm_device {
+   /**
+* @drm: DRM device
+*/
struct drm_device *drm;
+
+   /**
+* @pipe: Display pipe structure
+*/
struct drm_simple_display_pipe pipe;
+
+   /**
+* @dirty_lock: Serializes framebuffer flushing
+*/
struct mutex dirty_lock;
+
+   /**
+* @fb_funcs: Framebuffer functions used when creating framebuffers
+*/
const struct drm_framebuffer_funcs *fb_funcs;
+
+   /**
+* @fb_dirty: Framebuffer dirty callback
+*/
int (*fb_dirty)(struct drm_framebuffer *framebuffer,
struct drm_file *file_priv, unsigned flags,
unsigned color, struct drm_clip_rect *clips,



I assume the kerneldoc parser know how to handle this?

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/tinydrm: add backlight dependency for ili9341

[David Lechner]

On 07/10/2018 10:55 AM, Noralf Trønnes wrote:


Anyways, thanks for fixing this:
Acked-by: Noralf Trønnes 

David Lechner do you apply this?

Noralf.



Yes, I will pick it up.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v4 9/9] drm/cma-helper: Remove drm_fb_cma_fbdev_init_with_funcs()

[David Lechner]

On 07/02/2018 08:54 AM, Noralf Trønnes wrote:

Remove drm_fb_cma_fbdev_init_with_funcs(), its only user tinydrm has
moved to drm_fbdev_generic_setup().

Cc: Laurent Pinchart 
Signed-off-by: Noralf Trønnes 
---


Reviewed-by: David Lechner 


___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v4 8/9] drm/tinydrm: Use drm_fbdev_generic_setup()

[David Lechner]

On 07/02/2018 08:54 AM, Noralf Trønnes wrote:

Make full use of the generic fbdev client.

Cc: David Lechner 
Signed-off-by: Noralf Trønnes 
---


Reviewed-by: David Lechner 


___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 0/4] drm/tinydrm: new dirver for ILI9341 displays

[David Lechner]

On 06/27/2018 01:58 AM, Daniel Vetter wrote:

On Tue, Jun 26, 2018 at 8:16 PM, David Lechner  wrote:

On 06/20/2018 04:07 AM, Daniel Vetter wrote:


I acked and forwarded the account request, sorry for the delay. Next
time around, poking maintainers on irc helps if this stuff is stuck.
-Daniel



The account was supposedly setup, but I am not able to use it for some
reason. Are there any particular nicks I should ping on IRC?


#freedesktop on freenode is the channel where fd.o admins hang out.


Got that sorted out and pushed this series.

Hope I didn't break anything. :-o
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 0/4] drm/tinydrm: new dirver for ILI9341 displays

[David Lechner]

On 06/20/2018 04:07 AM, Daniel Vetter wrote:


I acked and forwarded the account request, sorry for the delay. Next
time around, poking maintainers on irc helps if this stuff is stuck.
-Daniel


The account was supposedly setup, but I am not able to use it for some
reason. Are there any particular nicks I should ping on IRC?

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 0/4] drm/tinydrm: new dirver for ILI9341 displays

[David Lechner]

On 6/3/18 5:00 PM, Noralf Trønnes wrote:


Den 25.05.2018 21.36, skrev David Lechner:
This series adds a new tinydrm driver for the Ilitek ILI9341 
controller and

a 2.4" display panel that uses this controller.


David, do you have commit rights now?


No. Opened a bug a while back to request access, but I guess the
right person didn't see it.

https://bugs.freedesktop.org/show_bug.cgi?id=105166



Noralf.


A few things to note here:
* The datasheet for this display[1] doesn't have a vendor mentioned on it
   anywhere, so I have used "adafruit" as the vendor prefix. If 
someone has a

   better suggestion, please speak up.
* The MAINTAINERS patch for ili9225 is included so we don't end up 
with a merge

   conflict later on.

v2 changes:
* change vendor prefix from "noname" to "adafruit"
* new patch for "adafruit" vendor prefix
* minor style changes
* drop regulator from driver (instead of adding to DT bindings)

[1]: 
https://cdn-learn.adafruit.com/assets/assets/000/046/879/original/SPEC-YX240QV29-T_Rev.A__1_.pdf 




David Lechner (4):
   MAINTAINERS: fix path to ilitek,ili9225 device tree bindings
   dt-bindings: Add vendor prefix for Adafruit
   dt-bindings: new binding for Ilitek ILI9341 display panels
   drm/tinydrm: new driver for ILI9341 display panels

  .../bindings/display/ilitek,ili9341.txt   |  27 ++
  .../devicetree/bindings/vendor-prefixes.txt   |   1 +
  MAINTAINERS   |   8 +-
  drivers/gpu/drm/tinydrm/Kconfig   |  10 +
  drivers/gpu/drm/tinydrm/Makefile  |   1 +
  drivers/gpu/drm/tinydrm/ili9341.c | 233 ++
  6 files changed, 279 insertions(+), 1 deletion(-)
  create mode 100644 
Documentation/devicetree/bindings/display/ilitek,ili9341.txt

  create mode 100644 drivers/gpu/drm/tinydrm/ili9341.c




___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 3/4] dt-bindings: new binding for Ilitek ILI9341 display panels

[David Lechner]

On 05/25/2018 02:36 PM, David Lechner wrote:

This adds a new binding for Ilitek ILI9341 display panels. It includes
a compatible string for one display (more can be added in the future).

The vendor prefix "noname" is used because the vendor is not known.


Looks like I forgot to update "noname" to "adafruit" in the commit message.
Patch is as intended though.


The YX240QV29-T panel[1] is found, for example, in an Adafruit breakout
board[2] and in Mindsensors' PiStorms[3].

[1]: 
https://cdn-learn.adafruit.com/assets/assets/000/046/879/original/SPEC-YX240QV29-T_Rev.A__1_.pdf
[2]: https://www.adafruit.com/product/2478
[3]: 
http://www.mindsensors.com/stem-with-robotics/13-pistorms-v2-base-kit-raspberry-pi-brain-for-lego-robot

Signed-off-by: David Lechner <da...@lechnology.com>
---
  .../bindings/display/ilitek,ili9341.txt   | 27 +++
  1 file changed, 27 insertions(+)
  create mode 100644 
Documentation/devicetree/bindings/display/ilitek,ili9341.txt

diff --git a/Documentation/devicetree/bindings/display/ilitek,ili9341.txt 
b/Documentation/devicetree/bindings/display/ilitek,ili9341.txt
new file mode 100644
index ..169b32e4ee4e
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/ilitek,ili9341.txt
@@ -0,0 +1,27 @@
+Ilitek ILI9341 display panels
+
+This binding is for display panels using an Ilitek ILI9341 controller in SPI
+mode.
+
+Required properties:
+- compatible:  "adafruit,yx240qv29", "ilitek,ili9341"
+- dc-gpios:D/C pin
+- reset-gpios: Reset pin
+
+The node for this driver must be a child node of a SPI controller, hence
+all mandatory properties described in ../spi/spi-bus.txt must be specified.
+
+Optional properties:
+- rotation:panel rotation in degrees counter clockwise (0,90,180,270)
+- backlight:   phandle of the backlight device attached to the panel
+
+Example:
+   display@0{
+   compatible = "adafruit,yx240qv29", "ilitek,ili9341";
+   reg = <0>;
+   spi-max-frequency = <3200>;
+   dc-gpios = < 9 GPIO_ACTIVE_HIGH>;
+   reset-gpios = < 8 GPIO_ACTIVE_HIGH>;
+   rotation = <270>;
+   backlight = <>;
+   };



___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 2/4] dt-bindings: Add vendor prefix for Adafruit

[David Lechner]

This adds a device tree vendor prefix for Adafruit Industries, LLC.

Signed-off-by: David Lechner <da...@lechnology.com>
---
 Documentation/devicetree/bindings/vendor-prefixes.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt 
b/Documentation/devicetree/bindings/vendor-prefixes.txt
index b5f978a4cac6..4d2ba7f52059 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.txt
+++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
@@ -8,6 +8,7 @@ abracon Abracon Corporation
 actionsActions Semiconductor Co., Ltd.
 active-semiActive-Semi International Inc
 ad Avionic Design GmbH
+adafruit   Adafruit Industries, LLC
 adapteva   Adapteva, Inc.
 adaptrum   Adaptrum, Inc.
 adhAD Holdings Plc.
-- 
2.17.0

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 3/4] dt-bindings: new binding for Ilitek ILI9341 display panels

[David Lechner]

This adds a new binding for Ilitek ILI9341 display panels. It includes
a compatible string for one display (more can be added in the future).

The vendor prefix "noname" is used because the vendor is not known.
The YX240QV29-T panel[1] is found, for example, in an Adafruit breakout
board[2] and in Mindsensors' PiStorms[3].

[1]: 
https://cdn-learn.adafruit.com/assets/assets/000/046/879/original/SPEC-YX240QV29-T_Rev.A__1_.pdf
[2]: https://www.adafruit.com/product/2478
[3]: 
http://www.mindsensors.com/stem-with-robotics/13-pistorms-v2-base-kit-raspberry-pi-brain-for-lego-robot

Signed-off-by: David Lechner <da...@lechnology.com>
---
 .../bindings/display/ilitek,ili9341.txt   | 27 +++
 1 file changed, 27 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/ilitek,ili9341.txt

diff --git a/Documentation/devicetree/bindings/display/ilitek,ili9341.txt 
b/Documentation/devicetree/bindings/display/ilitek,ili9341.txt
new file mode 100644
index ..169b32e4ee4e
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/ilitek,ili9341.txt
@@ -0,0 +1,27 @@
+Ilitek ILI9341 display panels
+
+This binding is for display panels using an Ilitek ILI9341 controller in SPI
+mode.
+
+Required properties:
+- compatible:  "adafruit,yx240qv29", "ilitek,ili9341"
+- dc-gpios:D/C pin
+- reset-gpios: Reset pin
+
+The node for this driver must be a child node of a SPI controller, hence
+all mandatory properties described in ../spi/spi-bus.txt must be specified.
+
+Optional properties:
+- rotation:panel rotation in degrees counter clockwise (0,90,180,270)
+- backlight:   phandle of the backlight device attached to the panel
+
+Example:
+   display@0{
+   compatible = "adafruit,yx240qv29", "ilitek,ili9341";
+   reg = <0>;
+   spi-max-frequency = <3200>;
+   dc-gpios = < 9 GPIO_ACTIVE_HIGH>;
+   reset-gpios = < 8 GPIO_ACTIVE_HIGH>;
+   rotation = <270>;
+   backlight = <>;
+   };
-- 
2.17.0

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 4/4] drm/tinydrm: new driver for ILI9341 display panels

[David Lechner]

This adds a new driver for display panels that use the Ilitek ILI9341
controller. It currently supports a single display panel, namely
the YX240QV29-T (e.g. Adafruit 2.4" TFT).

The init sequence is from the Adafruit Python library for the ILI9341
controller. https://github.com/adafruit/Adafruit_Python_ILI9341

Signed-off-by: David Lechner <da...@lechnology.com>
Reviewed-by: Andy Shevchenko <andy.shevche...@gmail.com>
Reviewed-by: Noralf Trønnes <nor...@tronnes.org>
---
 MAINTAINERS   |   6 +
 drivers/gpu/drm/tinydrm/Kconfig   |  10 ++
 drivers/gpu/drm/tinydrm/Makefile  |   1 +
 drivers/gpu/drm/tinydrm/ili9341.c | 233 ++
 4 files changed, 250 insertions(+)
 create mode 100644 drivers/gpu/drm/tinydrm/ili9341.c

diff --git a/MAINTAINERS b/MAINTAINERS
index bc219de9cbee..ffa099abbd79 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4480,6 +4480,12 @@ S:   Maintained
 F: drivers/gpu/drm/tinydrm/ili9225.c
 F: Documentation/devicetree/bindings/display/ilitek,ili9225.txt
 
+DRM DRIVER FOR ILITEK ILI9341 PANELS
+M: David Lechner <da...@lechnology.com>
+S: Maintained
+F: drivers/gpu/drm/tinydrm/ili9341.c
+F: Documentation/devicetree/bindings/display/ilitek,ili9341.txt
+
 DRM DRIVER FOR INTEL I810 VIDEO CARDS
 S: Orphan / Obsolete
 F: drivers/gpu/drm/i810/
diff --git a/drivers/gpu/drm/tinydrm/Kconfig b/drivers/gpu/drm/tinydrm/Kconfig
index 4592a5e3f20b..7a8008b0783f 100644
--- a/drivers/gpu/drm/tinydrm/Kconfig
+++ b/drivers/gpu/drm/tinydrm/Kconfig
@@ -20,6 +20,16 @@ config TINYDRM_ILI9225
 
  If M is selected the module will be called ili9225.
 
+config TINYDRM_ILI9341
+   tristate "DRM support for ILI9341 display panels"
+   depends on DRM_TINYDRM && SPI
+   select TINYDRM_MIPI_DBI
+   help
+ DRM driver for the following Ilitek ILI9341 panels:
+ * YX240QV29-T 2.4" 240x320 TFT (Adafruit 2.4")
+
+ If M is selected the module will be called ili9341.
+
 config TINYDRM_MI0283QT
tristate "DRM support for MI0283QT"
depends on DRM_TINYDRM && SPI
diff --git a/drivers/gpu/drm/tinydrm/Makefile b/drivers/gpu/drm/tinydrm/Makefile
index 49a111929724..14d99080665a 100644
--- a/drivers/gpu/drm/tinydrm/Makefile
+++ b/drivers/gpu/drm/tinydrm/Makefile
@@ -5,6 +5,7 @@ obj-$(CONFIG_TINYDRM_MIPI_DBI)  += mipi-dbi.o
 
 # Displays
 obj-$(CONFIG_TINYDRM_ILI9225)  += ili9225.o
+obj-$(CONFIG_TINYDRM_ILI9341)  += ili9341.o
 obj-$(CONFIG_TINYDRM_MI0283QT) += mi0283qt.o
 obj-$(CONFIG_TINYDRM_REPAPER)  += repaper.o
 obj-$(CONFIG_TINYDRM_ST7586)   += st7586.o
diff --git a/drivers/gpu/drm/tinydrm/ili9341.c 
b/drivers/gpu/drm/tinydrm/ili9341.c
new file mode 100644
index ..8864dcde6edc
--- /dev/null
+++ b/drivers/gpu/drm/tinydrm/ili9341.c
@@ -0,0 +1,233 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * DRM driver for Ilitek ILI9341 panels
+ *
+ * Copyright 2018 David Lechner <da...@lechnology.com>
+ *
+ * Based on mi0283qt.c:
+ * Copyright 2016 Noralf Trønnes
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define ILI9341_FRMCTR10xb1
+#define ILI9341_DISCTRL0xb6
+#define ILI9341_ETMOD  0xb7
+
+#define ILI9341_PWCTRL10xc0
+#define ILI9341_PWCTRL20xc1
+#define ILI9341_VMCTRL10xc5
+#define ILI9341_VMCTRL20xc7
+#define ILI9341_PWCTRLA0xcb
+#define ILI9341_PWCTRLB0xcf
+
+#define ILI9341_PGAMCTRL   0xe0
+#define ILI9341_NGAMCTRL   0xe1
+#define ILI9341_DTCTRLA0xe8
+#define ILI9341_DTCTRLB0xea
+#define ILI9341_PWRSEQ 0xed
+
+#define ILI9341_EN3GAM 0xf2
+#define ILI9341_PUMPCTRL   0xf7
+
+#define ILI9341_MADCTL_BGR BIT(3)
+#define ILI9341_MADCTL_MV  BIT(5)
+#define ILI9341_MADCTL_MX  BIT(6)
+#define ILI9341_MADCTL_MY  BIT(7)
+
+static void yx240qv29_enable(struct drm_simple_display_pipe *pipe,
+struct drm_crtc_state *crtc_state,
+struct drm_plane_state *plane_state)
+{
+   struct tinydrm_device *tdev = pipe_to_tinydrm(pipe);
+   struct mipi_dbi *mipi = mipi_dbi_from_tinydrm(tdev);
+   u8 addr_mode;
+   int ret;
+
+   DRM_DEBUG_KMS("\n");
+
+   ret = mipi_dbi_poweron_conditional_reset(mipi);
+   if (ret < 0)
+   return;
+   if (ret == 1)
+   goto out_enable;
+
+   mipi_dbi_command(mipi, MIPI_DCS_SET_DISPLAY_OFF);
+
+   mipi_dbi_command(mipi, ILI9341_PWCTRLB, 0x00, 0xc1, 0x30);
+   mipi_dbi_command(mipi, ILI9341_PWRSEQ, 0x64, 0x03, 0x12, 0x81);
+   mipi_dbi_command(mipi, ILI9341_DTCTRLA, 0x85, 0x00, 0x78);
+   mipi_d

[PATCH v2 1/4] MAINTAINERS: fix path to ilitek, ili9225 device tree bindings

[David Lechner]

This fixes the path to the ilitek,ili9225 device tree binding file.

Signed-off-by: David Lechner <da...@lechnology.com>
Reviewed-by: Noralf Trønnes <nor...@tronnes.org>
---
 MAINTAINERS | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 334a00350922..bc219de9cbee 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4478,7 +4478,7 @@ DRM DRIVER FOR ILITEK ILI9225 PANELS
 M: David Lechner <da...@lechnology.com>
 S: Maintained
 F: drivers/gpu/drm/tinydrm/ili9225.c
-F: Documentation/devicetree/bindings/display/ili9225.txt
+F: Documentation/devicetree/bindings/display/ilitek,ili9225.txt
 
 DRM DRIVER FOR INTEL I810 VIDEO CARDS
 S: Orphan / Obsolete
-- 
2.17.0

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 0/4] drm/tinydrm: new dirver for ILI9341 displays

[David Lechner]

This series adds a new tinydrm driver for the Ilitek ILI9341 controller and
a 2.4" display panel that uses this controller.

A few things to note here:
* The datasheet for this display[1] doesn't have a vendor mentioned on it
  anywhere, so I have used "adafruit" as the vendor prefix. If someone has a
  better suggestion, please speak up.
* The MAINTAINERS patch for ili9225 is included so we don't end up with a merge
  conflict later on.

v2 changes:
* change vendor prefix from "noname" to "adafruit"
* new patch for "adafruit" vendor prefix
* minor style changes
* drop regulator from driver (instead of adding to DT bindings)

[1]: 
https://cdn-learn.adafruit.com/assets/assets/000/046/879/original/SPEC-YX240QV29-T_Rev.A__1_.pdf


David Lechner (4):
  MAINTAINERS: fix path to ilitek,ili9225 device tree bindings
  dt-bindings: Add vendor prefix for Adafruit
  dt-bindings: new binding for Ilitek ILI9341 display panels
  drm/tinydrm: new driver for ILI9341 display panels

 .../bindings/display/ilitek,ili9341.txt   |  27 ++
 .../devicetree/bindings/vendor-prefixes.txt   |   1 +
 MAINTAINERS   |   8 +-
 drivers/gpu/drm/tinydrm/Kconfig   |  10 +
 drivers/gpu/drm/tinydrm/Makefile  |   1 +
 drivers/gpu/drm/tinydrm/ili9341.c | 233 ++
 6 files changed, 279 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/display/ilitek,ili9341.txt
 create mode 100644 drivers/gpu/drm/tinydrm/ili9341.c

-- 
2.17.0

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/3] dt-bindings: new binding for Ilitek ILI9341 display panels

[David Lechner]

On 05/19/2018 08:37 AM, Noralf Trønnes wrote:


Den 15.05.2018 03.43, skrev David Lechner:

This adds a new binding for Ilitek ILI9341 display panels. It includes
a compatible string for one display (more can be added in the future).

The vendor prefix "noname" is used because the vendor is not known.
The YX240QV29-T panel[1] is found, for example, in an Adafruit breakout
board[2] and in Mindsensors' PiStorms[3].

[1]: 
https://cdn-learn.adafruit.com/assets/assets/000/046/879/original/SPEC-YX240QV29-T_Rev.A__1_.pdf
[2]: https://www.adafruit.com/product/2478
[3]: 
http://www.mindsensors.com/stem-with-robotics/13-pistorms-v2-base-kit-raspberry-pi-brain-for-lego-robot

Signed-off-by: David Lechner <da...@lechnology.com>
---
  .../bindings/display/ilitek,ili9341.txt   | 27 +++
  1 file changed, 27 insertions(+)
  create mode 100644 
Documentation/devicetree/bindings/display/ilitek,ili9341.txt

diff --git a/Documentation/devicetree/bindings/display/ilitek,ili9341.txt 
b/Documentation/devicetree/bindings/display/ilitek,ili9341.txt
new file mode 100644
index ..0fc90b2dd732
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/ilitek,ili9341.txt
@@ -0,0 +1,27 @@
+Ilitek ILI9341 display panels
+
+This binding is for display panels using an Ilitek ILI9341 controller in SPI
+mode.
+
+Required properties:
+- compatible:    "noname,yx240qv29", "ilitek,ili9341"


Calling a vendor 'noname' looks a bit odd and AFAICT it isn't used by
any other binding. A google search always mentions Adafruit in
connection with this panel, so I suggest we use Adafruit as vendor.


I was hoping that someone might know the correct vendor, but barring that,
I agree that adafruit is probably the best choice.



I don't think we should use "ilitek,ili9341" as an option/fallback,
because panels varies in resolution (rarely) and initialization. A
generic ili9341 driver would probably not work with a random new panel.


I'm just following the precedent set in the other bindings for similar
displays that I have already done.

References:
* https://patchwork.freedesktop.org/patch/194648/
* https://patchwork.freedesktop.org/patch/195320/

I agree that it is probably not super-useful as a fallback. On the other
hand, the vendors and models for these displays are pretty obscure and
I think having the more well-known controller name _somewhere_ is useful.





+- dc-gpios:    D/C pin
+- reset-gpios:    Reset pin
+
+The node for this driver must be a child node of a SPI controller, hence
+all mandatory properties described in ../spi/spi-bus.txt must be specified.
+
+Optional properties:
+- rotation:    panel rotation in degrees counter clockwise (0,90,180,270)
+- backlight:    phandle of the backlight device attached to the panel


You forgot to mention the regulator that you support in the driver.

Noralf.


+
+Example:
+    display@0{
+    compatible = "noname,yx240qv29", "ilitek,ili9341";
+    reg = <0>;
+    spi-max-frequency = <3200>;
+    dc-gpios = < 9 GPIO_ACTIVE_HIGH>;
+    reset-gpios = < 8 GPIO_ACTIVE_HIGH>;
+    rotation = <270>;
+    backlight = <>;
+    };




___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 3/3] drm/tinydrm: new driver for ILI9341 display panels

[David Lechner]

This adds a new driver for display panels that use the Ilitek ILI9341
controller. It currently supports a single display panel, namely
the YX240QV29-T (e.g. Adafruit 2.4" TFT).

The init sequence is from the Adafruit Python library for the ILI9341
controller. https://github.com/adafruit/Adafruit_Python_ILI9341

Signed-off-by: David Lechner <da...@lechnology.com>
---
 MAINTAINERS   |   6 +
 drivers/gpu/drm/tinydrm/Kconfig   |  10 ++
 drivers/gpu/drm/tinydrm/Makefile  |   1 +
 drivers/gpu/drm/tinydrm/ili9341.c | 239 ++
 4 files changed, 256 insertions(+)
 create mode 100644 drivers/gpu/drm/tinydrm/ili9341.c

diff --git a/MAINTAINERS b/MAINTAINERS
index bc219de9cbee..ffa099abbd79 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4480,6 +4480,12 @@ S:   Maintained
 F: drivers/gpu/drm/tinydrm/ili9225.c
 F: Documentation/devicetree/bindings/display/ilitek,ili9225.txt
 
+DRM DRIVER FOR ILITEK ILI9341 PANELS
+M: David Lechner <da...@lechnology.com>
+S: Maintained
+F: drivers/gpu/drm/tinydrm/ili9341.c
+F: Documentation/devicetree/bindings/display/ilitek,ili9341.txt
+
 DRM DRIVER FOR INTEL I810 VIDEO CARDS
 S: Orphan / Obsolete
 F: drivers/gpu/drm/i810/
diff --git a/drivers/gpu/drm/tinydrm/Kconfig b/drivers/gpu/drm/tinydrm/Kconfig
index 4592a5e3f20b..7a8008b0783f 100644
--- a/drivers/gpu/drm/tinydrm/Kconfig
+++ b/drivers/gpu/drm/tinydrm/Kconfig
@@ -20,6 +20,16 @@ config TINYDRM_ILI9225
 
  If M is selected the module will be called ili9225.
 
+config TINYDRM_ILI9341
+   tristate "DRM support for ILI9341 display panels"
+   depends on DRM_TINYDRM && SPI
+   select TINYDRM_MIPI_DBI
+   help
+ DRM driver for the following Ilitek ILI9341 panels:
+ * YX240QV29-T 2.4" 240x320 TFT (Adafruit 2.4")
+
+ If M is selected the module will be called ili9341.
+
 config TINYDRM_MI0283QT
tristate "DRM support for MI0283QT"
depends on DRM_TINYDRM && SPI
diff --git a/drivers/gpu/drm/tinydrm/Makefile b/drivers/gpu/drm/tinydrm/Makefile
index 49a111929724..14d99080665a 100644
--- a/drivers/gpu/drm/tinydrm/Makefile
+++ b/drivers/gpu/drm/tinydrm/Makefile
@@ -5,6 +5,7 @@ obj-$(CONFIG_TINYDRM_MIPI_DBI)  += mipi-dbi.o
 
 # Displays
 obj-$(CONFIG_TINYDRM_ILI9225)  += ili9225.o
+obj-$(CONFIG_TINYDRM_ILI9341)  += ili9341.o
 obj-$(CONFIG_TINYDRM_MI0283QT) += mi0283qt.o
 obj-$(CONFIG_TINYDRM_REPAPER)  += repaper.o
 obj-$(CONFIG_TINYDRM_ST7586)   += st7586.o
diff --git a/drivers/gpu/drm/tinydrm/ili9341.c 
b/drivers/gpu/drm/tinydrm/ili9341.c
new file mode 100644
index ..2ce4244a68c3
--- /dev/null
+++ b/drivers/gpu/drm/tinydrm/ili9341.c
@@ -0,0 +1,239 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * DRM driver for Ilitek ILI9341 panels
+ *
+ * Copyright 2018 David Lechner <da...@lechnology.com>
+ *
+ * Based on mi0283qt.c:
+ * Copyright 2016 Noralf Trønnes
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define ILI9341_FRMCTR10xb1
+#define ILI9341_DISCTRL0xb6
+#define ILI9341_ETMOD  0xb7
+
+#define ILI9341_PWCTRL10xc0
+#define ILI9341_PWCTRL20xc1
+#define ILI9341_VMCTRL10xc5
+#define ILI9341_VMCTRL20xc7
+#define ILI9341_PWCTRLA0xcb
+#define ILI9341_PWCTRLB0xcf
+
+#define ILI9341_PGAMCTRL   0xe0
+#define ILI9341_NGAMCTRL   0xe1
+#define ILI9341_DTCTRLA0xe8
+#define ILI9341_DTCTRLB0xea
+#define ILI9341_PWRSEQ 0xed
+
+#define ILI9341_EN3GAM 0xf2
+#define ILI9341_PUMPCTRL   0xf7
+
+#define ILI9341_MADCTL_BGR BIT(3)
+#define ILI9341_MADCTL_MV  BIT(5)
+#define ILI9341_MADCTL_MX  BIT(6)
+#define ILI9341_MADCTL_MY  BIT(7)
+
+static void yx240qv29_enable(struct drm_simple_display_pipe *pipe,
+struct drm_crtc_state *crtc_state,
+struct drm_plane_state *plane_state)
+{
+   struct tinydrm_device *tdev = pipe_to_tinydrm(pipe);
+   struct mipi_dbi *mipi = mipi_dbi_from_tinydrm(tdev);
+   u8 addr_mode;
+   int ret;
+
+   DRM_DEBUG_KMS("\n");
+
+   ret = mipi_dbi_poweron_conditional_reset(mipi);
+   if (ret < 0)
+   return;
+   if (ret == 1)
+   goto out_enable;
+
+   mipi_dbi_command(mipi, MIPI_DCS_SET_DISPLAY_OFF);
+
+   mipi_dbi_command(mipi, ILI9341_PWCTRLB, 0x00, 0xc1, 0x30);
+   mipi_dbi_command(mipi, ILI9341_PWRSEQ, 0x64, 0x03, 0x12, 0x81);
+   mipi_dbi_command(mipi, ILI9341_DTCTRLA, 0x85, 0x00, 0x78);
+   mipi_dbi_command(mipi, ILI9341_PWCTRLA, 0x39, 0x2c, 0x00, 0x34, 0x02);
+   mipi_dbi_command(mipi, ILI9341_PUMPCTRL, 0x20

[PATCH 0/3] drm/tinydrm: new dirver for ILI9341 displays

[David Lechner]

This series adds a new tinydrm driver for the Ilitek ILI9341 controller and
a 2.4" display panel that uses this controller.

A few things to note here:
* The datasheet for this display[1] doesn't have a vendor mentioned on it
  anywhere, so I have used "noname" as the vendor prefix. If someone has a
  better suggestion, please speak up.
* The driver is basically a copy of mi0283qt.c with a new init sequence,
  a different physical panel size, fixed (as in corrected) rotation handling,
  and dropped PM (since I don't have a way to test it). Do we want to try to
  share code with these two drivers (it's not much)?
* The MAINTAINERS patch for ili9225 is included so we don't end up with a merge
  conflict later on.

[1]: 
https://cdn-learn.adafruit.com/assets/assets/000/046/879/original/SPEC-YX240QV29-T_Rev.A__1_.pdf

David Lechner (3):
  MAINTAINERS: fix path to ilitek,ili9225 device tree bindings
  dt-bindings: new binding for Ilitek ILI9341 display panels
  drm/tinydrm: new driver for ILI9341 display panels

 .../bindings/display/ilitek,ili9341.txt   |  27 ++
 MAINTAINERS   |   8 +-
 drivers/gpu/drm/tinydrm/Kconfig   |  10 +
 drivers/gpu/drm/tinydrm/Makefile  |   1 +
 drivers/gpu/drm/tinydrm/ili9341.c | 239 ++
 5 files changed, 284 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/display/ilitek,ili9341.txt
 create mode 100644 drivers/gpu/drm/tinydrm/ili9341.c

-- 
2.17.0

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 2/3] dt-bindings: new binding for Ilitek ILI9341 display panels

[David Lechner]

This adds a new binding for Ilitek ILI9341 display panels. It includes
a compatible string for one display (more can be added in the future).

The vendor prefix "noname" is used because the vendor is not known.
The YX240QV29-T panel[1] is found, for example, in an Adafruit breakout
board[2] and in Mindsensors' PiStorms[3].

[1]: 
https://cdn-learn.adafruit.com/assets/assets/000/046/879/original/SPEC-YX240QV29-T_Rev.A__1_.pdf
[2]: https://www.adafruit.com/product/2478
[3]: 
http://www.mindsensors.com/stem-with-robotics/13-pistorms-v2-base-kit-raspberry-pi-brain-for-lego-robot

Signed-off-by: David Lechner <da...@lechnology.com>
---
 .../bindings/display/ilitek,ili9341.txt   | 27 +++
 1 file changed, 27 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/ilitek,ili9341.txt

diff --git a/Documentation/devicetree/bindings/display/ilitek,ili9341.txt 
b/Documentation/devicetree/bindings/display/ilitek,ili9341.txt
new file mode 100644
index ..0fc90b2dd732
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/ilitek,ili9341.txt
@@ -0,0 +1,27 @@
+Ilitek ILI9341 display panels
+
+This binding is for display panels using an Ilitek ILI9341 controller in SPI
+mode.
+
+Required properties:
+- compatible:  "noname,yx240qv29", "ilitek,ili9341"
+- dc-gpios:D/C pin
+- reset-gpios: Reset pin
+
+The node for this driver must be a child node of a SPI controller, hence
+all mandatory properties described in ../spi/spi-bus.txt must be specified.
+
+Optional properties:
+- rotation:panel rotation in degrees counter clockwise (0,90,180,270)
+- backlight:   phandle of the backlight device attached to the panel
+
+Example:
+   display@0{
+   compatible = "noname,yx240qv29", "ilitek,ili9341";
+   reg = <0>;
+   spi-max-frequency = <3200>;
+   dc-gpios = < 9 GPIO_ACTIVE_HIGH>;
+   reset-gpios = < 8 GPIO_ACTIVE_HIGH>;
+   rotation = <270>;
+   backlight = <>;
+   };
-- 
2.17.0

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 1/3] MAINTAINERS: fix path to ilitek, ili9225 device tree bindings

[David Lechner]

This fixes the path to the ilitek,ili9225 device tree binding file.

Signed-off-by: David Lechner <da...@lechnology.com>
---
 MAINTAINERS | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 334a00350922..bc219de9cbee 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4478,7 +4478,7 @@ DRM DRIVER FOR ILITEK ILI9225 PANELS
 M: David Lechner <da...@lechnology.com>
 S: Maintained
 F: drivers/gpu/drm/tinydrm/ili9225.c
-F: Documentation/devicetree/bindings/display/ili9225.txt
+F: Documentation/devicetree/bindings/display/ilitek,ili9225.txt
 
 DRM DRIVER FOR INTEL I810 VIDEO CARDS
 S: Orphan / Obsolete
-- 
2.17.0

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/tilcdc: Fix setting clock divider for omap-l138

[David Lechner]

On 03/19/2018 04:05 AM, Jyri Sarha wrote:

Thanks,
I'll pick this for v4.18.


Ping. I don't see this in drm-misc-next yet.

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/9] drm: Move simple_display_pipe prepare_fb helper into gem fb helpers

[David Lechner]

On 04/05/2018 10:44 AM, Daniel Vetter wrote:

There's nothing tinydrm specific to this, and there's a few more
copies of the same in various other drivers.

Signed-off-by: Daniel Vetter <daniel.vet...@intel.com>
Cc: Gustavo Padovan <gust...@padovan.org>
Cc: Maarten Lankhorst <maarten.lankho...@linux.intel.com>
Cc: Sean Paul <seanp...@chromium.org>
Cc: David Airlie <airl...@linux.ie>
Cc: David Lechner <da...@lechnology.com>
Cc: "Noralf Trønnes" <nor...@tronnes.org>
Cc: Daniel Vetter <daniel.vet...@ffwll.ch>
Cc: Shawn Guo <shawn...@kernel.org>
Cc: Neil Armstrong <narmstr...@baylibre.com>
Cc: Daniel Stone <dani...@collabora.com>
Cc: Haneen Mohammed <hamohammed...@gmail.com>
Cc: Ben Widawsky <b...@bwidawsk.net>
Cc: "Ville Syrjälä" <ville.syrj...@linux.intel.com>
---


Acked-by: David Lechner <da...@lechnology.com>


___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/2] drm/simple-kms-helper: Plumb plane state to the enable hook

[David Lechner]

On 03/27/2018 05:07 AM, Ville Syrjälä wrote:

On Sat, Mar 24, 2018 at 12:26:32PM -0500, David Lechner wrote:

On 03/22/2018 03:27 PM, Ville Syrjala wrote:

From: Ville Syrjälä <ville.syrj...@linux.intel.com>

We'll need access to the plane state during .atomic_enable().



Some more details in the commit message would be useful. It is
not clear to me why this change is being made.


"tinydrm enable hook wants to play around with the new fb in
.atomic_enable(), thus we'll need access to the plane state."

Better? Worse?



Better.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/2] drm/simple-kms-helper: Plumb plane state to the enable hook

[David Lechner]

On 03/22/2018 03:27 PM, Ville Syrjala wrote:

From: Ville Syrjälä 

We'll need access to the plane state during .atomic_enable().



Some more details in the commit message would be useful. It is
not clear to me why this change is being made.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel