[RFC PATCH 00/11] Support for H3 Composite Output support

[RFC PATCH 00/11] Support for H3 Composite Output support

[Icenowy Zheng]

This patchset depends on the DE2 patchset, version 8 of that patchset
is available at [1].

Allwinner H3 SoC features a TV Encoder like the one in Allwinner A13,
which can only output TV Composite signal.

The display pipeline of H3 is also special -- it has two mixers and
two TCONs, of which the connection can be swapped. The TCONs do not
have channel 0 (as they are all connected to internal bridges, TVE
and HDMI TX).

Add support for the display pipeline and the TVE in H3, in order to
make it possible to display something with mainline kernel with H3.

The image quality of TVE is bad, so HDMI is a better output -- this
patchset also prepared the mixers and TCONs for HDMI output, and
the HDMI controller driver is already done by Jernej Skrabec.

Currently the jack detection feature of the TVE is still not so
clear -- so it's not implemented in this version. Thus the TV
output shouldn't be defaultly enabled now.

[1] http://lists.infradead.org/pipermail/linux-arm-kernel/2017-May/506806.html

Icenowy Zheng (11):
  dt-bindings: update the binding for Allwinner H3 TVE support
  drm: sun4i: add support for H3 mixers
  drm: sun4i: ignore swapped mixer<->tcon connection for DE2
  drm: sun4i: add support for H3's TCON0/1
  drm: sun4i: add compatible for H3 display engine
  drm: sun4i: add color space correction support for DE2 mixer
  drm: sun4i: add support for the TV encoder in H3 SoC
  clk: sunxi-ng: allow CLK_DE to set CLK_PLL_DE for H3
  clk: sunxi-ng: export CLK_PLL_DE for H3
  ARM: sun8i: h3: add display engine pipeline for TVE
  [DO NOT MERGE] ARM: sun8i: h3: enable TV output on Orange Pi PC

 .../bindings/display/sunxi/sun4i-drm.txt           |  47 ++++-
 arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts         |  12 ++
 arch/arm/boot/dts/sun8i-h3.dtsi                    | 189 +++++++++++++++++++++
 drivers/clk/sunxi-ng/ccu-sun8i-h3.c                |   2 +-
 drivers/clk/sunxi-ng/ccu-sun8i-h3.h                |   3 +-
 drivers/gpu/drm/sun4i/sun4i_drv.c                  |  28 +++
 drivers/gpu/drm/sun4i/sun4i_tcon.c                 | 117 +++++++++----
 drivers/gpu/drm/sun4i/sun4i_tcon.h                 |   5 +
 drivers/gpu/drm/sun4i/sun4i_tv.c                   |  65 ++++++-
 drivers/gpu/drm/sun4i/sun8i_mixer.c                |  53 ++++++
 drivers/gpu/drm/sun4i/sun8i_mixer.h                |   6 +-
 include/dt-bindings/clock/sun8i-h3-ccu.h           |   2 +
 12 files changed, 488 insertions(+), 41 deletions(-)

-- 
2.12.2

[RFC PATCH 01/11] dt-bindings: update the binding for Allwinner H3 TVE support

[Icenowy Zheng]

Allwinner H3 features a "DE2.0" and a TV Encoder.

Add device tree bindings for the following parts:
- H3 TCONs
- H3 Mixers
- The connection between H3 TCONs and H3 Mixers
- H3 TV Encoder
- H3 Display engine

Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
---
 .../bindings/display/sunxi/sun4i-drm.txt           | 47 ++++++++++++++++++++--
 1 file changed, 43 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt b/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt
index 66b85a195ef2..52781943713b 100644
--- a/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt
+++ b/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt
@@ -21,7 +21,9 @@ The TV Encoder supports the composite and VGA output. It is one end of
 the pipeline.
 
 Required properties:
- - compatible: value should be "allwinner,sun4i-a10-tv-encoder".
+ - compatible: value must be either:
+    * allwinner,sun4i-a10-tv-encoder
+    * allwinner,sun8i-h3-tv-encoder
  - reg: base address and size of memory-mapped region
  - clocks: the clocks driving the TV encoder
  - resets: phandle to the reset controller driving the encoder
@@ -30,6 +32,13 @@ Required properties:
   Documentation/devicetree/bindings/media/video-interfaces.txt. The
   first port should be the input endpoint.
 
+For "allwinner,sun4i-a10-tv-encoder", there is only one clock required,
+and it's not named.
+
+For "allwinner,sun8i-h3-tv-encoder", these clocks are needed:
+    - 'bus': the AHB bus clock of TVE
+    - 'mod': the mod clock of TVE
+
 TCON
 ----
 
@@ -41,29 +50,51 @@ Required properties:
    * allwinner,sun6i-a31-tcon
    * allwinner,sun6i-a31s-tcon
    * allwinner,sun8i-a33-tcon
+   * allwinner,sun8i-h3-tcon0
+   * allwinner,sun8i-h3-tcon1
    * allwinner,sun8i-v3s-tcon
  - reg: base address and size of memory-mapped region
  - interrupts: interrupt associated to this IP
  - clocks: phandles to the clocks feeding the TCON. Three are needed:
    - 'ahb': the interface clocks
-   - 'tcon-ch0': The clock driving the TCON channel 0
  - resets: phandles to the reset controllers driving the encoder
    - "lcd": the reset line for the TCON channel 0
 
  - clock-names: the clock names mentioned above
  - reset-names: the reset names mentioned above
- - clock-output-names: Name of the pixel clock created
 
 - ports: A ports node with endpoint definitions as defined in
   Documentation/devicetree/bindings/media/video-interfaces.txt. The
   first port should be the input endpoint, the second one the output
 
+  In the situation of Display Engine 2.0 that the connection between
+  the mixer and the TCON can be swapped, the input should have two
+  endpoints. The first is the default mixer connected to the TCON,
+  the second the mixer which will be connected to the TCON if the
+  swap bit is set.
+
   The output should have two endpoints. The first is the block
   connected to the TCON channel 0 (usually a panel or a bridge), the
   second the block connected to the TCON channel 1 (usually the TV
   encoder)
 
-On SoCs other than the A33 and V3s, there is one more clock required:
+For the following compatibles:
+   * allwinner,sun5i-a13-tcon
+   * allwinner,sun6i-a31-tcon
+   * allwinner,sun6i-a31s-tcon
+   * allwinner,sun8i-a33-tcon
+   * allwinner,sun8i-v3s-tcon
+there is one more clock and one more property required:
+ - clocks:
+   - 'tcon-ch0': The clock driving the TCON channel 0
+ - clock-output-names: Name of the pixel clock created
+
+For the following compatibles:
+   * allwinner,sun5i-a13-tcon
+   * allwinner,sun6i-a31-tcon
+   * allwinner,sun6i-a31s-tcon
+   * allwinner,sun8i-h3-tcon0
+there is one more clock required:
    - 'tcon-ch1': The clock driving the TCON channel 1
 
 DRC
@@ -158,6 +189,8 @@ supported.
 Required properties:
   - compatible: value must be one of:
     * allwinner,sun8i-v3s-de2-mixer
+    * allwinner,sun8i-h3-de2-mixer0
+    * allwinner,sun8i-h3-de2-mixer1
   - reg: base address and size of the memory-mapped region.
   - clocks: phandles to the clocks feeding the mixer
     * bus: the mixer interface clock
@@ -169,6 +202,11 @@ Required properties:
   Documentation/devicetree/bindings/media/video-interfaces.txt. The
   first port should be the input endpoints, the second one the output
 
+  In the situation of Display Engine 2.0 that the connection between
+  the mixer and the TCON can be swapped, the output should have two
+  endpoints. The first is the default TCON connected to the mixer,
+  the second the TCON which will be connected to the mixer if the
+  swap bit is set.
 
 Display Engine Pipeline
 -----------------------
@@ -183,6 +221,7 @@ Required properties:
     * allwinner,sun6i-a31-display-engine
     * allwinner,sun6i-a31s-display-engine
     * allwinner,sun8i-a33-display-engine
+    * allwinner,sun8i-h3-display-engine
     * allwinner,sun8i-v3s-display-engine
 
   - allwinner,pipelines: list of phandle to the display engine
-- 
2.12.2

[RFC PATCH 01/11] dt-bindings: update the binding for Allwinner H3 TVE support

[Maxime Ripard]

On Thu, May 18, 2017 at 12:43:44AM +0800, Icenowy Zheng wrote:
> -On SoCs other than the A33 and V3s, there is one more clock required:
> +For the following compatibles:
> +   * allwinner,sun5i-a13-tcon
> +   * allwinner,sun6i-a31-tcon
> +   * allwinner,sun6i-a31s-tcon
> +   * allwinner,sun8i-a33-tcon
> +   * allwinner,sun8i-v3s-tcon
> +there is one more clock and one more property required:
> + - clocks:
> +   - 'tcon-ch0': The clock driving the TCON channel 0
> + - clock-output-names: Name of the pixel clock created
> +
> +For the following compatibles:
> +   * allwinner,sun5i-a13-tcon
> +   * allwinner,sun6i-a31-tcon
> +   * allwinner,sun6i-a31s-tcon
> +   * allwinner,sun8i-h3-tcon0
> +there is one more clock required:
>     - 'tcon-ch1': The clock driving the TCON channel 1

Putting ID's in the compatible name is usually a bad idea. What is the
difference between the two? Only that the second one doesn't have a
clock?

That seems highly unlikely. How does it generate the pixel clock
frequency?

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20170519/6b634b74/attachment.sig>

[RFC PATCH 01/11] dt-bindings: update the binding for Allwinner H3 TVE support

[Icenowy Zheng]

? 2017?5?20? GMT+08:00 ??2:02:15, Maxime Ripard <maxime.ripard@free-electrons.com> ??:
>On Thu, May 18, 2017 at 12:43:44AM +0800, Icenowy Zheng wrote:
>> -On SoCs other than the A33 and V3s, there is one more clock
>required:
>> +For the following compatibles:
>> +   * allwinner,sun5i-a13-tcon
>> +   * allwinner,sun6i-a31-tcon
>> +   * allwinner,sun6i-a31s-tcon
>> +   * allwinner,sun8i-a33-tcon
>> +   * allwinner,sun8i-v3s-tcon
>> +there is one more clock and one more property required:
>> + - clocks:
>> +   - 'tcon-ch0': The clock driving the TCON channel 0
>> + - clock-output-names: Name of the pixel clock created
>> +
>> +For the following compatibles:
>> +   * allwinner,sun5i-a13-tcon
>> +   * allwinner,sun6i-a31-tcon
>> +   * allwinner,sun6i-a31s-tcon
>> +   * allwinner,sun8i-h3-tcon0
>> +there is one more clock required:
>>     - 'tcon-ch1': The clock driving the TCON channel 1
>
>Putting ID's in the compatible name is usually a bad idea. What is the
>difference between the two? Only that the second one doesn't have a
>clock?

Yes.

>
>That seems highly unlikely. How does it generate the pixel clock
>frequency?

Yes it seems impossible, but it's also the fact.

There's only one CLK_TCON in H3/5, which is for TCON0.

It's possible that lcd-ch1 clk is CLK_TVE, but it's still a weird situation --
Although we have a lcd-ch1 clock, we cannot touch it, otherwise
the TVE will refuse to work (the TVE can only work under 216MHz).

>
>Maxime

[linux-sunxi] Re: [RFC PATCH 01/11] dt-bindings: update the binding for Allwinner H3 TVE support

[Chen-Yu Tsai]

On Sat, May 20, 2017 at 2:06 AM, Icenowy Zheng <icenowy@aosc.io> wrote:
>
>
> ? 2017?5?20? GMT+08:00 ??2:02:15, Maxime Ripard <maxime.ripard@free-electrons.com> ??:
>>On Thu, May 18, 2017 at 12:43:44AM +0800, Icenowy Zheng wrote:
>>> -On SoCs other than the A33 and V3s, there is one more clock
>>required:
>>> +For the following compatibles:
>>> +   * allwinner,sun5i-a13-tcon
>>> +   * allwinner,sun6i-a31-tcon
>>> +   * allwinner,sun6i-a31s-tcon
>>> +   * allwinner,sun8i-a33-tcon
>>> +   * allwinner,sun8i-v3s-tcon
>>> +there is one more clock and one more property required:
>>> + - clocks:
>>> +   - 'tcon-ch0': The clock driving the TCON channel 0
>>> + - clock-output-names: Name of the pixel clock created
>>> +
>>> +For the following compatibles:
>>> +   * allwinner,sun5i-a13-tcon
>>> +   * allwinner,sun6i-a31-tcon
>>> +   * allwinner,sun6i-a31s-tcon
>>> +   * allwinner,sun8i-h3-tcon0
>>> +there is one more clock required:
>>>     - 'tcon-ch1': The clock driving the TCON channel 1
>>
>>Putting ID's in the compatible name is usually a bad idea. What is the
>>difference between the two? Only that the second one doesn't have a
>>clock?
>
> Yes.
>
>>
>>That seems highly unlikely. How does it generate the pixel clock
>>frequency?
>
> Yes it seems impossible, but it's also the fact.
>
> There's only one CLK_TCON in H3/5, which is for TCON0.
>
> It's possible that lcd-ch1 clk is CLK_TVE, but it's still a weird situation --
> Although we have a lcd-ch1 clock, we cannot touch it, otherwise
> the TVE will refuse to work (the TVE can only work under 216MHz).

Assuming the TV encoder is like the old one, then it never had a
separate module clock. Instead its timing signals are fed from the
TCON. So CLK_TVE is likely the clock for TCON1 here.

ChenYu

[RFC PATCH 02/11] drm: sun4i: add support for H3 mixers

[Icenowy Zheng]

From: Icenowy Zheng <icenowy@aosc.xyz>

Allwinner H3 SoC has two mixers, one has 1 VI channel and 3 UI channels,
and the other has 1 VI and 1 UI.

Add support for these two variants.

Signed-off-by: Icenowy Zheng <icenowy@aosc.xyz>
---
 drivers/gpu/drm/sun4i/sun8i_mixer.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.c b/drivers/gpu/drm/sun4i/sun8i_mixer.c
index cb193c5f1686..d658a3a8159a 100644
--- a/drivers/gpu/drm/sun4i/sun8i_mixer.c
+++ b/drivers/gpu/drm/sun4i/sun8i_mixer.c
@@ -390,11 +390,29 @@ static const struct sun8i_mixer_cfg sun8i_v3s_mixer_cfg = {
 	.ui_num = 1,
 };
 
+static const struct sun8i_mixer_cfg sun8i_h3_mixer0_cfg = {
+	.vi_num = 1,
+	.ui_num = 3,
+};
+
+static const struct sun8i_mixer_cfg sun8i_h3_mixer1_cfg = {
+	.vi_num = 1,
+	.ui_num = 1,
+};
+
 static const struct of_device_id sun8i_mixer_of_table[] = {
 	{
 		.compatible = "allwinner,sun8i-v3s-de2-mixer",
 		.data = &sun8i_v3s_mixer_cfg,
 	},
+	{
+		.compatible = "allwinner,sun8i-h3-de2-mixer0",
+		.data = &sun8i_h3_mixer0_cfg
+	},
+	{
+		.compatible = "allwinner,sun8i-h3-de2-mixer1",
+		.data = &sun8i_h3_mixer1_cfg
+	},
 	{ }
 };
 MODULE_DEVICE_TABLE(of, sun8i_mixer_of_table);
-- 
2.12.2

[RFC PATCH 02/11] drm: sun4i: add support for H3 mixers

[Maxime Ripard]

On Thu, May 18, 2017 at 12:43:45AM +0800, Icenowy Zheng wrote:
> From: Icenowy Zheng <icenowy@aosc.xyz>
> 
> Allwinner H3 SoC has two mixers, one has 1 VI channel and 3 UI channels,
> and the other has 1 VI and 1 UI.
> 
> Add support for these two variants.
> 
> Signed-off-by: Icenowy Zheng <icenowy@aosc.xyz>
> ---
>  drivers/gpu/drm/sun4i/sun8i_mixer.c | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.c b/drivers/gpu/drm/sun4i/sun8i_mixer.c
> index cb193c5f1686..d658a3a8159a 100644
> --- a/drivers/gpu/drm/sun4i/sun8i_mixer.c
> +++ b/drivers/gpu/drm/sun4i/sun8i_mixer.c
> @@ -390,11 +390,29 @@ static const struct sun8i_mixer_cfg sun8i_v3s_mixer_cfg = {
>  	.ui_num = 1,
>  };
>  
> +static const struct sun8i_mixer_cfg sun8i_h3_mixer0_cfg = {
> +	.vi_num = 1,
> +	.ui_num = 3,
> +};
> +
> +static const struct sun8i_mixer_cfg sun8i_h3_mixer1_cfg = {
> +	.vi_num = 1,
> +	.ui_num = 1,
> +};
> +
>  static const struct of_device_id sun8i_mixer_of_table[] = {
>  	{
>  		.compatible = "allwinner,sun8i-v3s-de2-mixer",
>  		.data = &sun8i_v3s_mixer_cfg,
>  	},
> +	{
> +		.compatible = "allwinner,sun8i-h3-de2-mixer0",
> +		.data = &sun8i_h3_mixer0_cfg
> +	},
> +	{
> +		.compatible = "allwinner,sun8i-h3-de2-mixer1",
> +		.data = &sun8i_h3_mixer1_cfg
> +	},

So the only difference between the two is the number of ui planes?

Why not create a property to give the number then, instead of a
compatible?

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20170519/faff51d3/attachment.sig>

[linux-sunxi] Re: [RFC PATCH 02/11] drm: sun4i: add support for H3 mixers

[Icenowy Zheng]

? 2017?5?20? GMT+08:00 ??1:47:29, Maxime Ripard <maxime.ripard@free-electrons.com> ??:
>On Thu, May 18, 2017 at 12:43:45AM +0800, Icenowy Zheng wrote:
>> From: Icenowy Zheng <icenowy@aosc.xyz>
>> 
>> Allwinner H3 SoC has two mixers, one has 1 VI channel and 3 UI
>channels,
>> and the other has 1 VI and 1 UI.
>> 
>> Add support for these two variants.
>> 
>> Signed-off-by: Icenowy Zheng <icenowy@aosc.xyz>
>> ---
>>  drivers/gpu/drm/sun4i/sun8i_mixer.c | 18 ++++++++++++++++++
>>  1 file changed, 18 insertions(+)
>> 
>> diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.c
>b/drivers/gpu/drm/sun4i/sun8i_mixer.c
>> index cb193c5f1686..d658a3a8159a 100644
>> --- a/drivers/gpu/drm/sun4i/sun8i_mixer.c
>> +++ b/drivers/gpu/drm/sun4i/sun8i_mixer.c
>> @@ -390,11 +390,29 @@ static const struct sun8i_mixer_cfg
>sun8i_v3s_mixer_cfg = {
>>  	.ui_num = 1,
>>  };
>>  
>> +static const struct sun8i_mixer_cfg sun8i_h3_mixer0_cfg = {
>> +	.vi_num = 1,
>> +	.ui_num = 3,
>> +};
>> +
>> +static const struct sun8i_mixer_cfg sun8i_h3_mixer1_cfg = {
>> +	.vi_num = 1,
>> +	.ui_num = 1,
>> +};
>> +
>>  static const struct of_device_id sun8i_mixer_of_table[] = {
>>  	{
>>  		.compatible = "allwinner,sun8i-v3s-de2-mixer",
>>  		.data = &sun8i_v3s_mixer_cfg,
>>  	},
>> +	{
>> +		.compatible = "allwinner,sun8i-h3-de2-mixer0",
>> +		.data = &sun8i_h3_mixer0_cfg
>> +	},
>> +	{
>> +		.compatible = "allwinner,sun8i-h3-de2-mixer1",
>> +		.data = &sun8i_h3_mixer1_cfg
>> +	},
>
>So the only difference between the two is the number of ui planes?

Not only., but currently we only implemented this.

More functions differ, but we still don't support them...

>
>Why not create a property to give the number then, instead of a
>compatible?
>
>Maxime

[linux-sunxi] Re: [RFC PATCH 02/11] drm: sun4i: add support for H3 mixers

[Jernej Škrabec]

Hi!

Dne petek, 19. maj 2017 ob 19:49:58 CEST je Icenowy Zheng napisal(a):
> ? 2017?5?20? GMT+08:00 ??1:47:29, Maxime Ripard <maxime.ripard@free-
electrons.com> ??:
> >On Thu, May 18, 2017 at 12:43:45AM +0800, Icenowy Zheng wrote:
> >> From: Icenowy Zheng <icenowy@aosc.xyz>
> >> 
> >> Allwinner H3 SoC has two mixers, one has 1 VI channel and 3 UI
> >
> >channels,
> >
> >> and the other has 1 VI and 1 UI.
> >> 
> >> Add support for these two variants.
> >> 
> >> Signed-off-by: Icenowy Zheng <icenowy@aosc.xyz>
> >> ---
> >> 
> >>  drivers/gpu/drm/sun4i/sun8i_mixer.c | 18 ++++++++++++++++++
> >>  1 file changed, 18 insertions(+)
> >> 
> >> diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.c
> >
> >b/drivers/gpu/drm/sun4i/sun8i_mixer.c
> >
> >> index cb193c5f1686..d658a3a8159a 100644
> >> --- a/drivers/gpu/drm/sun4i/sun8i_mixer.c
> >> +++ b/drivers/gpu/drm/sun4i/sun8i_mixer.c
> >> @@ -390,11 +390,29 @@ static const struct sun8i_mixer_cfg
> >
> >sun8i_v3s_mixer_cfg = {
> >
> >>  	.ui_num = 1,
> >>  
> >>  };
> >> 
> >> +static const struct sun8i_mixer_cfg sun8i_h3_mixer0_cfg = {
> >> +	.vi_num = 1,
> >> +	.ui_num = 3,
> >> +};
> >> +
> >> +static const struct sun8i_mixer_cfg sun8i_h3_mixer1_cfg = {
> >> +	.vi_num = 1,
> >> +	.ui_num = 1,
> >> +};
> >> +
> >> 
> >>  static const struct of_device_id sun8i_mixer_of_table[] = {
> >>  
> >>  	{
> >>  	
> >>  		.compatible = "allwinner,sun8i-v3s-de2-mixer",
> >>  		.data = &sun8i_v3s_mixer_cfg,
> >>  	
> >>  	},
> >> 
> >> +	{
> >> +		.compatible = "allwinner,sun8i-h3-de2-mixer0",
> >> +		.data = &sun8i_h3_mixer0_cfg
> >> +	},
> >> +	{
> >> +		.compatible = "allwinner,sun8i-h3-de2-mixer1",
> >> +		.data = &sun8i_h3_mixer1_cfg
> >> +	},
> >
> >So the only difference between the two is the number of ui planes?
> 
> Not only., but currently we only implemented this.
> 
> More functions differ, but we still don't support them...
> 

As far as I can tell, they only differ in ui & vi number of planes and between 
different SoCs, max plane size.

Icenowy,
Do you know any other property they differ? I think everything else is based 
mostly on ui & vi number of planes.

Best regards,
Jernej

> >Why not create a property to give the number then, instead of a
> >compatible?
> >
> >Maxime
> 
> --
> You received this message because you are subscribed to the Google Groups
> "linux-sunxi" group. To unsubscribe from this group and stop receiving
> emails from it, send an email to linux-sunxi+unsubscribe at googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.

[RFC PATCH 03/11] drm: sun4i: ignore swapped mixer<->tcon connection for DE2

[Icenowy Zheng]

Some SoC's DE2 has two mixers. Defaultly the mixer0 is connected to
tcon0 and mixer1 is connected to tcon1; however by setting a bit
the connection can be swapped.

As we now hardcode the default connection, ignore the bonus endpoint for
the mixer's output and the TCON's input, as they stands for the swapped
connection.

Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
---
 drivers/gpu/drm/sun4i/sun4i_drv.c  | 27 ++++++++++++++++++++++++++
 drivers/gpu/drm/sun4i/sun4i_tcon.c | 39 +++++++++++++++++++++++++++++---------
 drivers/gpu/drm/sun4i/sun4i_tcon.h |  2 ++
 3 files changed, 59 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun4i_drv.c b/drivers/gpu/drm/sun4i/sun4i_drv.c
index 1dd1948025d2..29bf1325ded6 100644
--- a/drivers/gpu/drm/sun4i/sun4i_drv.c
+++ b/drivers/gpu/drm/sun4i/sun4i_drv.c
@@ -173,6 +173,13 @@ static bool sun4i_drv_node_is_frontend(struct device_node *node)
 		of_device_is_compatible(node, "allwinner,sun8i-a33-display-frontend");
 }
 
+static bool sun4i_drv_node_is_swappable_de2_mixer(struct device_node *node)
+{
+	/* The V3s has only one mixer-tcon pair, so it's not listed here. */
+	return of_device_is_compatible(node, "allwinner,sun8i-h3-de2-mixer0") ||
+		of_device_is_compatible(node, "allwinner,sun8i-h3-de2-mixer1");
+}
+
 static bool sun4i_drv_node_is_tcon(struct device_node *node)
 {
 	return of_device_is_compatible(node, "allwinner,sun5i-a13-tcon") ||
@@ -249,6 +256,26 @@ static int sun4i_drv_add_endpoints(struct device *dev,
 			}
 		}
 
+		/*
+		 * The second endpoint of the output of a swappable DE2 mixer
+		 * is the TCON after connection swapping.
+		 * Ignore it now, as we now hardcode mixer0->tcon0,
+		 * mixer1->tcon1 connection.
+		 */
+		if (sun4i_drv_node_is_swappable_de2_mixer(node)) {
+			struct of_endpoint endpoint;
+
+			if (of_graph_parse_endpoint(ep, &endpoint)) {
+				DRM_DEBUG_DRIVER("Couldn't parse endpoint\n");
+				continue;
+			}
+
+			if (endpoint.id) {
+				DRM_DEBUG_DRIVER("Endpoint is an unused connection for DE2 mixer... skipping\n");
+				continue;
+			}
+		}
+
 		/* Walk down our tree */
 		count += sun4i_drv_add_endpoints(dev, match, remote);
 
diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c
index f44a37a5993d..89a215ff2370 100644
--- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
+++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
@@ -425,7 +425,8 @@ static int sun4i_tcon_init_regmap(struct device *dev,
  * requested via the get_id function of the engine.
  */
 static struct sunxi_engine *sun4i_tcon_find_engine(struct sun4i_drv *drv,
-						   struct device_node *node)
+						   struct device_node *node,
+						   bool skip_bonus_ep)
 {
 	struct device_node *port, *ep, *remote;
 	struct sunxi_engine *engine;
@@ -439,6 +440,20 @@ static struct sunxi_engine *sun4i_tcon_find_engine(struct sun4i_drv *drv,
 		if (!remote)
 			continue;
 
+		if (skip_bonus_ep) {
+			struct of_endpoint endpoint;
+
+			if (of_graph_parse_endpoint(ep, &endpoint)) {
+				DRM_DEBUG_DRIVER("Couldn't parse endpoint\n");
+				continue;
+			}
+
+			if (endpoint.id) {
+				DRM_DEBUG_DRIVER("Skipping bonus mixer->TCON connection when searching engine\n");
+				continue;
+			}
+		}
+
 		/* does this node match any registered engines? */
 		list_for_each_entry(engine, &drv->engine_list, list) {
 			if (remote == engine->node) {
@@ -449,7 +464,7 @@ static struct sunxi_engine *sun4i_tcon_find_engine(struct sun4i_drv *drv,
 		}
 
 		/* keep looking through upstream ports */
-		engine = sun4i_tcon_find_engine(drv, remote);
+		engine = sun4i_tcon_find_engine(drv, remote, skip_bonus_ep);
 		if (!IS_ERR(engine)) {
 			of_node_put(remote);
 			of_node_put(port);
@@ -469,21 +484,27 @@ static int sun4i_tcon_bind(struct device *dev, struct device *master,
 	struct sun4i_tcon *tcon;
 	int ret;
 
-	engine = sun4i_tcon_find_engine(drv, dev->of_node);
-	if (IS_ERR(engine)) {
-		dev_err(dev, "Couldn't find matching engine\n");
-		return -EPROBE_DEFER;
-	}
-
 	tcon = devm_kzalloc(dev, sizeof(*tcon), GFP_KERNEL);
 	if (!tcon)
 		return -ENOMEM;
 	dev_set_drvdata(dev, tcon);
 	tcon->drm = drm;
 	tcon->dev = dev;
-	tcon->id = engine->id;
 	tcon->quirks = of_device_get_match_data(dev);
 
+	/*
+	 * As we keep the connection between DE2 mixer and TCON not swapped,
+	 * skip the bonus endpoints (which stand for swapped connection)
+	 * when finding the correspoing engine.
+	 */
+	engine = sun4i_tcon_find_engine(drv, dev->of_node,
+					tcon->quirks->swappable_input);
+	if (IS_ERR(engine)) {
+		dev_err(dev, "Couldn't find matching engine\n");
+		return -EPROBE_DEFER;
+	}
+	tcon->id = engine->id;
+
 	tcon->lcd_rst = devm_reset_control_get(dev, "lcd");
 	if (IS_ERR(tcon->lcd_rst)) {
 		dev_err(dev, "Couldn't get our reset line\n");
diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.h b/drivers/gpu/drm/sun4i/sun4i_tcon.h
index d37e1e2ed60e..568dc736238a 100644
--- a/drivers/gpu/drm/sun4i/sun4i_tcon.h
+++ b/drivers/gpu/drm/sun4i/sun4i_tcon.h
@@ -146,6 +146,8 @@
 struct sun4i_tcon_quirks {
 	bool	has_unknown_mux; /* sun5i has undocumented mux */
 	bool	has_channel_1;	/* a33 does not have channel 1 */
+	/* Some DE2 can swap the mixer<->TCON connection */
+	bool	swappable_input;
 };
 
 struct sun4i_tcon {
-- 
2.12.2

[RFC PATCH 03/11] drm: sun4i: ignore swapped mixer<->tcon connection for DE2

[Maxime Ripard]

On Thu, May 18, 2017 at 12:43:46AM +0800, Icenowy Zheng wrote:
> Some SoC's DE2 has two mixers. Defaultly the mixer0 is connected to
> tcon0 and mixer1 is connected to tcon1; however by setting a bit
> the connection can be swapped.
> 
> As we now hardcode the default connection, ignore the bonus endpoint for
> the mixer's output and the TCON's input, as they stands for the swapped
> connection.
> 
> Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
> ---
>  drivers/gpu/drm/sun4i/sun4i_drv.c  | 27 ++++++++++++++++++++++++++
>  drivers/gpu/drm/sun4i/sun4i_tcon.c | 39 +++++++++++++++++++++++++++++---------
>  drivers/gpu/drm/sun4i/sun4i_tcon.h |  2 ++
>  3 files changed, 59 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/sun4i/sun4i_drv.c b/drivers/gpu/drm/sun4i/sun4i_drv.c
> index 1dd1948025d2..29bf1325ded6 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_drv.c
> +++ b/drivers/gpu/drm/sun4i/sun4i_drv.c
> @@ -173,6 +173,13 @@ static bool sun4i_drv_node_is_frontend(struct device_node *node)
>  		of_device_is_compatible(node, "allwinner,sun8i-a33-display-frontend");
>  }
>  
> +static bool sun4i_drv_node_is_swappable_de2_mixer(struct device_node *node)
> +{
> +	/* The V3s has only one mixer-tcon pair, so it's not listed here. */
> +	return of_device_is_compatible(node, "allwinner,sun8i-h3-de2-mixer0") ||
> +		of_device_is_compatible(node, "allwinner,sun8i-h3-de2-mixer1");
> +}
> +
>  static bool sun4i_drv_node_is_tcon(struct device_node *node)
>  {
>  	return of_device_is_compatible(node, "allwinner,sun5i-a13-tcon") ||
> @@ -249,6 +256,26 @@ static int sun4i_drv_add_endpoints(struct device *dev,
>  			}
>  		}
>  
> +		/*
> +		 * The second endpoint of the output of a swappable DE2 mixer
> +		 * is the TCON after connection swapping.
> +		 * Ignore it now, as we now hardcode mixer0->tcon0,
> +		 * mixer1->tcon1 connection.
> +		 */
> +		if (sun4i_drv_node_is_swappable_de2_mixer(node)) {
> +			struct of_endpoint endpoint;
> +
> +			if (of_graph_parse_endpoint(ep, &endpoint)) {
> +				DRM_DEBUG_DRIVER("Couldn't parse endpoint\n");
> +				continue;
> +			}
> +
> +			if (endpoint.id) {
> +				DRM_DEBUG_DRIVER("Endpoint is an unused connection for DE2 mixer... skipping\n");
> +				continue;
> +			}
> +		}
> +
>  		/* Walk down our tree */
>  		count += sun4i_drv_add_endpoints(dev, match, remote);
>  
> diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> index f44a37a5993d..89a215ff2370 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
> +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> @@ -425,7 +425,8 @@ static int sun4i_tcon_init_regmap(struct device *dev,
>   * requested via the get_id function of the engine.
>   */
>  static struct sunxi_engine *sun4i_tcon_find_engine(struct sun4i_drv *drv,
> -						   struct device_node *node)
> +						   struct device_node *node,
> +						   bool skip_bonus_ep)
>  {
>  	struct device_node *port, *ep, *remote;
>  	struct sunxi_engine *engine;
> @@ -439,6 +440,20 @@ static struct sunxi_engine *sun4i_tcon_find_engine(struct sun4i_drv *drv,
>  		if (!remote)
>  			continue;
>  
> +		if (skip_bonus_ep) {
> +			struct of_endpoint endpoint;
> +
> +			if (of_graph_parse_endpoint(ep, &endpoint)) {
> +				DRM_DEBUG_DRIVER("Couldn't parse endpoint\n");
> +				continue;
> +			}
> +
> +			if (endpoint.id) {
> +				DRM_DEBUG_DRIVER("Skipping bonus mixer->TCON connection when searching engine\n");
> +				continue;
> +			}
> +		}
> +

You don't list the mixers in the tcon's output, why do you need that
exactly?

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20170519/eb1f9469/attachment.sig>

[RFC PATCH 03/11] drm: sun4i: ignore swapped mixer<->tcon connection for DE2

[Icenowy Zheng]

? 2017?5?20? GMT+08:00 ??1:57:53, Maxime Ripard <maxime.ripard@free-electrons.com> ??:
>On Thu, May 18, 2017 at 12:43:46AM +0800, Icenowy Zheng wrote:
>> Some SoC's DE2 has two mixers. Defaultly the mixer0 is connected to
>> tcon0 and mixer1 is connected to tcon1; however by setting a bit
>> the connection can be swapped.
>> 
>> As we now hardcode the default connection, ignore the bonus endpoint
>for
>> the mixer's output and the TCON's input, as they stands for the
>swapped
>> connection.
>> 
>> Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
>> ---
>>  drivers/gpu/drm/sun4i/sun4i_drv.c  | 27 ++++++++++++++++++++++++++
>>  drivers/gpu/drm/sun4i/sun4i_tcon.c | 39
>+++++++++++++++++++++++++++++---------
>>  drivers/gpu/drm/sun4i/sun4i_tcon.h |  2 ++
>>  3 files changed, 59 insertions(+), 9 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/sun4i/sun4i_drv.c
>b/drivers/gpu/drm/sun4i/sun4i_drv.c
>> index 1dd1948025d2..29bf1325ded6 100644
>> --- a/drivers/gpu/drm/sun4i/sun4i_drv.c
>> +++ b/drivers/gpu/drm/sun4i/sun4i_drv.c
>> @@ -173,6 +173,13 @@ static bool sun4i_drv_node_is_frontend(struct
>device_node *node)
>>  		of_device_is_compatible(node,
>"allwinner,sun8i-a33-display-frontend");
>>  }
>>  
>> +static bool sun4i_drv_node_is_swappable_de2_mixer(struct device_node
>*node)
>> +{
>> +	/* The V3s has only one mixer-tcon pair, so it's not listed here.
>*/
>> +	return of_device_is_compatible(node,
>"allwinner,sun8i-h3-de2-mixer0") ||
>> +		of_device_is_compatible(node, "allwinner,sun8i-h3-de2-mixer1");
>> +}
>> +
>>  static bool sun4i_drv_node_is_tcon(struct device_node *node)
>>  {
>>  	return of_device_is_compatible(node, "allwinner,sun5i-a13-tcon") ||
>> @@ -249,6 +256,26 @@ static int sun4i_drv_add_endpoints(struct device
>*dev,
>>  			}
>>  		}
>>  
>> +		/*
>> +		 * The second endpoint of the output of a swappable DE2 mixer
>> +		 * is the TCON after connection swapping.
>> +		 * Ignore it now, as we now hardcode mixer0->tcon0,
>> +		 * mixer1->tcon1 connection.
>> +		 */
>> +		if (sun4i_drv_node_is_swappable_de2_mixer(node)) {
>> +			struct of_endpoint endpoint;
>> +
>> +			if (of_graph_parse_endpoint(ep, &endpoint)) {
>> +				DRM_DEBUG_DRIVER("Couldn't parse endpoint\n");
>> +				continue;
>> +			}
>> +
>> +			if (endpoint.id) {
>> +				DRM_DEBUG_DRIVER("Endpoint is an unused connection for DE2
>mixer... skipping\n");
>> +				continue;
>> +			}
>> +		}
>> +
>>  		/* Walk down our tree */
>>  		count += sun4i_drv_add_endpoints(dev, match, remote);
>>  
>> diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c
>b/drivers/gpu/drm/sun4i/sun4i_tcon.c
>> index f44a37a5993d..89a215ff2370 100644
>> --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
>> +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
>> @@ -425,7 +425,8 @@ static int sun4i_tcon_init_regmap(struct device
>*dev,
>>   * requested via the get_id function of the engine.
>>   */
>>  static struct sunxi_engine *sun4i_tcon_find_engine(struct sun4i_drv
>*drv,
>> -						   struct device_node *node)
>> +						   struct device_node *node,
>> +						   bool skip_bonus_ep)
>>  {
>>  	struct device_node *port, *ep, *remote;
>>  	struct sunxi_engine *engine;
>> @@ -439,6 +440,20 @@ static struct sunxi_engine
>*sun4i_tcon_find_engine(struct sun4i_drv *drv,
>>  		if (!remote)
>>  			continue;
>>  
>> +		if (skip_bonus_ep) {
>> +			struct of_endpoint endpoint;
>> +
>> +			if (of_graph_parse_endpoint(ep, &endpoint)) {
>> +				DRM_DEBUG_DRIVER("Couldn't parse endpoint\n");
>> +				continue;
>> +			}
>> +
>> +			if (endpoint.id) {
>> +				DRM_DEBUG_DRIVER("Skipping bonus mixer->TCON connection when
>searching engine\n");
>> +				continue;
>> +			}
>> +		}
>> +
>
>You don't list the mixers in the tcon's output, why do you need that
>exactly?

Mixers are TCONs' input, not output...

>
>Maxime

[RFC PATCH 03/11] drm: sun4i: ignore swapped mixer<->tcon connection for DE2

[Maxime Ripard]

On Sat, May 20, 2017 at 02:00:22AM +0800, Icenowy Zheng wrote:
> 
> 
> ? 2017?5?20? GMT+08:00 ??1:57:53, Maxime Ripard <maxime.ripard@free-electrons.com> ??:
> >On Thu, May 18, 2017 at 12:43:46AM +0800, Icenowy Zheng wrote:
> >> Some SoC's DE2 has two mixers. Defaultly the mixer0 is connected to
> >> tcon0 and mixer1 is connected to tcon1; however by setting a bit
> >> the connection can be swapped.
> >> 
> >> As we now hardcode the default connection, ignore the bonus endpoint
> >for
> >> the mixer's output and the TCON's input, as they stands for the
> >swapped
> >> connection.
> >> 
> >> Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
> >> ---
> >>  drivers/gpu/drm/sun4i/sun4i_drv.c  | 27 ++++++++++++++++++++++++++
> >>  drivers/gpu/drm/sun4i/sun4i_tcon.c | 39
> >+++++++++++++++++++++++++++++---------
> >>  drivers/gpu/drm/sun4i/sun4i_tcon.h |  2 ++
> >>  3 files changed, 59 insertions(+), 9 deletions(-)
> >> 
> >> diff --git a/drivers/gpu/drm/sun4i/sun4i_drv.c
> >b/drivers/gpu/drm/sun4i/sun4i_drv.c
> >> index 1dd1948025d2..29bf1325ded6 100644
> >> --- a/drivers/gpu/drm/sun4i/sun4i_drv.c
> >> +++ b/drivers/gpu/drm/sun4i/sun4i_drv.c
> >> @@ -173,6 +173,13 @@ static bool sun4i_drv_node_is_frontend(struct
> >device_node *node)
> >>  		of_device_is_compatible(node,
> >"allwinner,sun8i-a33-display-frontend");
> >>  }
> >>  
> >> +static bool sun4i_drv_node_is_swappable_de2_mixer(struct device_node
> >*node)
> >> +{
> >> +	/* The V3s has only one mixer-tcon pair, so it's not listed here.
> >*/
> >> +	return of_device_is_compatible(node,
> >"allwinner,sun8i-h3-de2-mixer0") ||
> >> +		of_device_is_compatible(node, "allwinner,sun8i-h3-de2-mixer1");
> >> +}
> >> +
> >>  static bool sun4i_drv_node_is_tcon(struct device_node *node)
> >>  {
> >>  	return of_device_is_compatible(node, "allwinner,sun5i-a13-tcon") ||
> >> @@ -249,6 +256,26 @@ static int sun4i_drv_add_endpoints(struct device
> >*dev,
> >>  			}
> >>  		}
> >>  
> >> +		/*
> >> +		 * The second endpoint of the output of a swappable DE2 mixer
> >> +		 * is the TCON after connection swapping.
> >> +		 * Ignore it now, as we now hardcode mixer0->tcon0,
> >> +		 * mixer1->tcon1 connection.
> >> +		 */
> >> +		if (sun4i_drv_node_is_swappable_de2_mixer(node)) {
> >> +			struct of_endpoint endpoint;
> >> +
> >> +			if (of_graph_parse_endpoint(ep, &endpoint)) {
> >> +				DRM_DEBUG_DRIVER("Couldn't parse endpoint\n");
> >> +				continue;
> >> +			}
> >> +
> >> +			if (endpoint.id) {
> >> +				DRM_DEBUG_DRIVER("Endpoint is an unused connection for DE2
> >mixer... skipping\n");
> >> +				continue;
> >> +			}
> >> +		}
> >> +
> >>  		/* Walk down our tree */
> >>  		count += sun4i_drv_add_endpoints(dev, match, remote);
> >>  
> >> diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c
> >b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> >> index f44a37a5993d..89a215ff2370 100644
> >> --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
> >> +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> >> @@ -425,7 +425,8 @@ static int sun4i_tcon_init_regmap(struct device
> >*dev,
> >>   * requested via the get_id function of the engine.
> >>   */
> >>  static struct sunxi_engine *sun4i_tcon_find_engine(struct sun4i_drv
> >*drv,
> >> -						   struct device_node *node)
> >> +						   struct device_node *node,
> >> +						   bool skip_bonus_ep)
> >>  {
> >>  	struct device_node *port, *ep, *remote;
> >>  	struct sunxi_engine *engine;
> >> @@ -439,6 +440,20 @@ static struct sunxi_engine
> >*sun4i_tcon_find_engine(struct sun4i_drv *drv,
> >>  		if (!remote)
> >>  			continue;
> >>  
> >> +		if (skip_bonus_ep) {
> >> +			struct of_endpoint endpoint;
> >> +
> >> +			if (of_graph_parse_endpoint(ep, &endpoint)) {
> >> +				DRM_DEBUG_DRIVER("Couldn't parse endpoint\n");
> >> +				continue;
> >> +			}
> >> +
> >> +			if (endpoint.id) {
> >> +				DRM_DEBUG_DRIVER("Skipping bonus mixer->TCON connection when
> >searching engine\n");
> >> +				continue;
> >> +			}
> >> +		}
> >> +
> >
> >You don't list the mixers in the tcon's output, why do you need that
> >exactly?
> 
> Mixers are TCONs' input, not output...

Then why are they even parsed? The whole parsing logic in the driver
only searches for output nodes.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20170524/d4efe716/attachment.sig>

[RFC PATCH 03/11] drm: sun4i: ignore swapped mixer<->tcon connection for DE2

[icenowy at aosc.io]

? 2017-05-24 16:14?Maxime Ripard ???
> On Sat, May 20, 2017 at 02:00:22AM +0800, Icenowy Zheng wrote:
>> 
>> 
>> ? 2017?5?20? GMT+08:00 ??1:57:53, Maxime Ripard 
>> <maxime.ripard@free-electrons.com> ??:
>> >On Thu, May 18, 2017 at 12:43:46AM +0800, Icenowy Zheng wrote:
>> >> Some SoC's DE2 has two mixers. Defaultly the mixer0 is connected to
>> >> tcon0 and mixer1 is connected to tcon1; however by setting a bit
>> >> the connection can be swapped.
>> >>
>> >> As we now hardcode the default connection, ignore the bonus endpoint
>> >for
>> >> the mixer's output and the TCON's input, as they stands for the
>> >swapped
>> >> connection.
>> >>
>> >> Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
>> >> ---
>> >>  drivers/gpu/drm/sun4i/sun4i_drv.c  | 27 ++++++++++++++++++++++++++
>> >>  drivers/gpu/drm/sun4i/sun4i_tcon.c | 39
>> >+++++++++++++++++++++++++++++---------
>> >>  drivers/gpu/drm/sun4i/sun4i_tcon.h |  2 ++
>> >>  3 files changed, 59 insertions(+), 9 deletions(-)
>> >>
>> >> diff --git a/drivers/gpu/drm/sun4i/sun4i_drv.c
>> >b/drivers/gpu/drm/sun4i/sun4i_drv.c
>> >> index 1dd1948025d2..29bf1325ded6 100644
>> >> --- a/drivers/gpu/drm/sun4i/sun4i_drv.c
>> >> +++ b/drivers/gpu/drm/sun4i/sun4i_drv.c
>> >> @@ -173,6 +173,13 @@ static bool sun4i_drv_node_is_frontend(struct
>> >device_node *node)
>> >>  		of_device_is_compatible(node,
>> >"allwinner,sun8i-a33-display-frontend");
>> >>  }
>> >>
>> >> +static bool sun4i_drv_node_is_swappable_de2_mixer(struct device_node
>> >*node)
>> >> +{
>> >> +	/* The V3s has only one mixer-tcon pair, so it's not listed here.
>> >*/
>> >> +	return of_device_is_compatible(node,
>> >"allwinner,sun8i-h3-de2-mixer0") ||
>> >> +		of_device_is_compatible(node, "allwinner,sun8i-h3-de2-mixer1");
>> >> +}
>> >> +
>> >>  static bool sun4i_drv_node_is_tcon(struct device_node *node)
>> >>  {
>> >>  	return of_device_is_compatible(node, "allwinner,sun5i-a13-tcon") ||
>> >> @@ -249,6 +256,26 @@ static int sun4i_drv_add_endpoints(struct device
>> >*dev,
>> >>  			}
>> >>  		}
>> >>
>> >> +		/*
>> >> +		 * The second endpoint of the output of a swappable DE2 mixer
>> >> +		 * is the TCON after connection swapping.
>> >> +		 * Ignore it now, as we now hardcode mixer0->tcon0,
>> >> +		 * mixer1->tcon1 connection.
>> >> +		 */
>> >> +		if (sun4i_drv_node_is_swappable_de2_mixer(node)) {
>> >> +			struct of_endpoint endpoint;
>> >> +
>> >> +			if (of_graph_parse_endpoint(ep, &endpoint)) {
>> >> +				DRM_DEBUG_DRIVER("Couldn't parse endpoint\n");
>> >> +				continue;
>> >> +			}
>> >> +
>> >> +			if (endpoint.id) {
>> >> +				DRM_DEBUG_DRIVER("Endpoint is an unused connection for DE2
>> >mixer... skipping\n");
>> >> +				continue;
>> >> +			}
>> >> +		}
>> >> +
>> >>  		/* Walk down our tree */
>> >>  		count += sun4i_drv_add_endpoints(dev, match, remote);
>> >>
>> >> diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c
>> >b/drivers/gpu/drm/sun4i/sun4i_tcon.c
>> >> index f44a37a5993d..89a215ff2370 100644
>> >> --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
>> >> +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
>> >> @@ -425,7 +425,8 @@ static int sun4i_tcon_init_regmap(struct device
>> >*dev,
>> >>   * requested via the get_id function of the engine.
>> >>   */
>> >>  static struct sunxi_engine *sun4i_tcon_find_engine(struct sun4i_drv
>> >*drv,
>> >> -						   struct device_node *node)
>> >> +						   struct device_node *node,
>> >> +						   bool skip_bonus_ep)
>> >>  {
>> >>  	struct device_node *port, *ep, *remote;
>> >>  	struct sunxi_engine *engine;
>> >> @@ -439,6 +440,20 @@ static struct sunxi_engine
>> >*sun4i_tcon_find_engine(struct sun4i_drv *drv,
>> >>  		if (!remote)
>> >>  			continue;
>> >>
>> >> +		if (skip_bonus_ep) {
>> >> +			struct of_endpoint endpoint;
>> >> +
>> >> +			if (of_graph_parse_endpoint(ep, &endpoint)) {
>> >> +				DRM_DEBUG_DRIVER("Couldn't parse endpoint\n");
>> >> +				continue;
>> >> +			}
>> >> +
>> >> +			if (endpoint.id) {
>> >> +				DRM_DEBUG_DRIVER("Skipping bonus mixer->TCON connection when
>> >searching engine\n");
>> >> +				continue;
>> >> +			}
>> >> +		}
>> >> +
>> >
>> >You don't list the mixers in the tcon's output, why do you need that
>> >exactly?
>> 
>> Mixers are TCONs' input, not output...
> 
> Then why are they even parsed? The whole parsing logic in the driver
> only searches for output nodes.

Here it's really parsing the input node, for searching for a connected
engine.

In the situation of DE1 TCON has only one input endpoint and it must be
the connected backend.

But in the situation of DE2 it has now two input endpoints, and it's
now needed to figure out which connection is used.

> 
> Maxime

[RFC PATCH 04/11] drm: sun4i: add support for H3's TCON0/1

[Icenowy Zheng]

From: Icenowy Zheng <icenowy@aosc.xyz>

Allwinner H3 has two special TCONs, both come without channel0. And the
TCON1 of H3 has no special clocks even for the channel1.

Add support for these kinds of TCON.

Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
---
 drivers/gpu/drm/sun4i/sun4i_tcon.c | 78 ++++++++++++++++++++++++++++----------
 drivers/gpu/drm/sun4i/sun4i_tcon.h |  3 ++
 2 files changed, 61 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c
index 89a215ff2370..7009292f99e4 100644
--- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
+++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
@@ -57,6 +57,7 @@ void sun4i_tcon_channel_disable(struct sun4i_tcon *tcon, int channel)
 {
 	/* Disable the TCON's channel */
 	if (channel == 0) {
+		WARN_ON(!tcon->quirks->has_channel_0);
 		regmap_update_bits(tcon->regs, SUN4I_TCON0_CTL_REG,
 				   SUN4I_TCON0_CTL_TCON_ENABLE, 0);
 		clk_disable_unprepare(tcon->dclk);
@@ -66,7 +67,8 @@ void sun4i_tcon_channel_disable(struct sun4i_tcon *tcon, int channel)
 	WARN_ON(!tcon->quirks->has_channel_1);
 	regmap_update_bits(tcon->regs, SUN4I_TCON1_CTL_REG,
 			   SUN4I_TCON1_CTL_TCON_ENABLE, 0);
-	clk_disable_unprepare(tcon->sclk1);
+	if (tcon->quirks->has_channel_1_clk)
+		clk_disable_unprepare(tcon->sclk1);
 }
 EXPORT_SYMBOL(sun4i_tcon_channel_disable);
 
@@ -74,6 +76,7 @@ void sun4i_tcon_channel_enable(struct sun4i_tcon *tcon, int channel)
 {
 	/* Enable the TCON's channel */
 	if (channel == 0) {
+		WARN_ON(!tcon->quirks->has_channel_0);
 		regmap_update_bits(tcon->regs, SUN4I_TCON0_CTL_REG,
 				   SUN4I_TCON0_CTL_TCON_ENABLE,
 				   SUN4I_TCON0_CTL_TCON_ENABLE);
@@ -85,7 +88,8 @@ void sun4i_tcon_channel_enable(struct sun4i_tcon *tcon, int channel)
 	regmap_update_bits(tcon->regs, SUN4I_TCON1_CTL_REG,
 			   SUN4I_TCON1_CTL_TCON_ENABLE,
 			   SUN4I_TCON1_CTL_TCON_ENABLE);
-	clk_prepare_enable(tcon->sclk1);
+	if (tcon->quirks->has_channel_1_clk)
+		clk_prepare_enable(tcon->sclk1);
 }
 EXPORT_SYMBOL(sun4i_tcon_channel_enable);
 
@@ -132,6 +136,7 @@ void sun4i_tcon0_mode_set(struct sun4i_tcon *tcon,
 
 	/* Configure the dot clock */
 	clk_set_rate(tcon->dclk, mode->crtc_clock * 1000);
+	WARN_ON(!tcon->quirks->has_channel_0);
 
 	/* Adjust clock delay */
 	clk_delay = sun4i_tcon_get_clk_delay(mode, 0);
@@ -209,7 +214,8 @@ void sun4i_tcon1_mode_set(struct sun4i_tcon *tcon,
 	WARN_ON(!tcon->quirks->has_channel_1);
 
 	/* Configure the dot clock */
-	clk_set_rate(tcon->sclk1, mode->crtc_clock * 1000);
+	if (tcon->quirks->has_channel_1_clk)
+		clk_set_rate(tcon->sclk1, mode->crtc_clock * 1000);
 
 	/* Adjust clock delay */
 	clk_delay = sun4i_tcon_get_clk_delay(mode, 1);
@@ -327,13 +333,15 @@ static int sun4i_tcon_init_clocks(struct device *dev,
 	}
 	clk_prepare_enable(tcon->clk);
 
-	tcon->sclk0 = devm_clk_get(dev, "tcon-ch0");
-	if (IS_ERR(tcon->sclk0)) {
-		dev_err(dev, "Couldn't get the TCON channel 0 clock\n");
-		return PTR_ERR(tcon->sclk0);
+	if (tcon->quirks->has_channel_0) {
+		tcon->sclk0 = devm_clk_get(dev, "tcon-ch0");
+		if (IS_ERR(tcon->sclk0)) {
+			dev_err(dev, "Couldn't get the TCON channel 0 clock\n");
+			return PTR_ERR(tcon->sclk0);
+		}
 	}
 
-	if (tcon->quirks->has_channel_1) {
+	if (tcon->quirks->has_channel_1 && tcon->quirks->has_channel_1_clk) {
 		tcon->sclk1 = devm_clk_get(dev, "tcon-ch1");
 		if (IS_ERR(tcon->sclk1)) {
 			dev_err(dev, "Couldn't get the TCON channel 1 clock\n");
@@ -533,10 +541,12 @@ static int sun4i_tcon_bind(struct device *dev, struct device *master,
 		goto err_free_clocks;
 	}
 
-	ret = sun4i_dclk_create(dev, tcon);
-	if (ret) {
-		dev_err(dev, "Couldn't create our TCON dot clock\n");
-		goto err_free_clocks;
+	if (tcon->quirks->has_channel_0) {
+		ret = sun4i_dclk_create(dev, tcon);
+		if (ret) {
+			dev_err(dev, "Couldn't create our TCON dot clock\n");
+			goto err_free_clocks;
+		}
 	}
 
 	ret = sun4i_tcon_init_irq(dev, tcon);
@@ -561,7 +571,8 @@ static int sun4i_tcon_bind(struct device *dev, struct device *master,
 	return 0;
 
 err_free_dotclock:
-	sun4i_dclk_free(tcon);
+	if (tcon->quirks->has_channel_0)
+		sun4i_dclk_free(tcon);
 err_free_clocks:
 	sun4i_tcon_free_clocks(tcon);
 err_assert_reset:
@@ -575,7 +586,9 @@ static void sun4i_tcon_unbind(struct device *dev, struct device *master,
 	struct sun4i_tcon *tcon = dev_get_drvdata(dev);
 
 	list_del(&tcon->list);
-	sun4i_dclk_free(tcon);
+
+	if (tcon->quirks->has_channel_0)
+		sun4i_dclk_free(tcon);
 	sun4i_tcon_free_clocks(tcon);
 }
 
@@ -606,24 +619,41 @@ static int sun4i_tcon_remove(struct platform_device *pdev)
 }
 
 static const struct sun4i_tcon_quirks sun5i_a13_quirks = {
-	.has_unknown_mux = true,
-	.has_channel_1	= true,
+	.has_unknown_mux	= true,
+	.has_channel_0		= true,
+	.has_channel_1		= true,
+	.has_channel_1_clk	= true,
 };
 
 static const struct sun4i_tcon_quirks sun6i_a31_quirks = {
-	.has_channel_1	= true,
+	.has_channel_0		= true,
+	.has_channel_1		= true,
+	.has_channel_1_clk	= true,
 };
 
 static const struct sun4i_tcon_quirks sun6i_a31s_quirks = {
-	.has_channel_1	= true,
+	.has_channel_0		= true,
+	.has_channel_1		= true,
+	.has_channel_1_clk	= true,
 };
 
 static const struct sun4i_tcon_quirks sun8i_a33_quirks = {
-	/* nothing is supported */
+	.has_channel_0	= true,
 };
 
 static const struct sun4i_tcon_quirks sun8i_v3s_quirks = {
-	/* nothing is supported */
+	.has_channel_0	= true,
+};
+
+static const struct sun4i_tcon_quirks sun8i_h3_tcon0_quirks = {
+	.has_channel_1		= true,
+	.has_channel_1_clk	= true,
+	.swappable_input	= true,
+};
+
+static const struct sun4i_tcon_quirks sun8i_h3_tcon1_quirks = {
+	.has_channel_1		= true,
+	.swappable_input	= true,
 };
 
 static const struct of_device_id sun4i_tcon_of_table[] = {
@@ -632,6 +662,14 @@ static const struct of_device_id sun4i_tcon_of_table[] = {
 	{ .compatible = "allwinner,sun6i-a31s-tcon", .data = &sun6i_a31s_quirks },
 	{ .compatible = "allwinner,sun8i-a33-tcon", .data = &sun8i_a33_quirks },
 	{ .compatible = "allwinner,sun8i-v3s-tcon", .data = &sun8i_v3s_quirks },
+	{
+		.compatible = "allwinner,sun8i-h3-tcon0",
+		.data = &sun8i_h3_tcon0_quirks
+	},
+	{
+		.compatible = "allwinner,sun8i-h3-tcon1",
+		.data = &sun8i_h3_tcon1_quirks
+	},
 	{ }
 };
 MODULE_DEVICE_TABLE(of, sun4i_tcon_of_table);
diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.h b/drivers/gpu/drm/sun4i/sun4i_tcon.h
index 568dc736238a..6aeb2d68b523 100644
--- a/drivers/gpu/drm/sun4i/sun4i_tcon.h
+++ b/drivers/gpu/drm/sun4i/sun4i_tcon.h
@@ -145,7 +145,10 @@
 
 struct sun4i_tcon_quirks {
 	bool	has_unknown_mux; /* sun5i has undocumented mux */
+	bool	has_channel_0;	/* some A83T+ TCONs don't have channel 0*/
 	bool	has_channel_1;	/* a33 does not have channel 1 */
+	/* H3 TCON1 doesn't have channel 1 sclk */
+	bool	has_channel_1_clk;
 	/* Some DE2 can swap the mixer<->TCON connection */
 	bool	swappable_input;
 };
-- 
2.12.2

[RFC PATCH 05/11] drm: sun4i: add compatible for H3 display engine

[Icenowy Zheng]

Add a compatible string for H3 display engine in sun4i_drv code.

Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
---
 drivers/gpu/drm/sun4i/sun4i_drv.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/sun4i/sun4i_drv.c b/drivers/gpu/drm/sun4i/sun4i_drv.c
index 29bf1325ded6..c0de0741c923 100644
--- a/drivers/gpu/drm/sun4i/sun4i_drv.c
+++ b/drivers/gpu/drm/sun4i/sun4i_drv.c
@@ -324,6 +324,7 @@ static const struct of_device_id sun4i_drv_of_table[] = {
 	{ .compatible = "allwinner,sun6i-a31-display-engine" },
 	{ .compatible = "allwinner,sun6i-a31s-display-engine" },
 	{ .compatible = "allwinner,sun8i-a33-display-engine" },
+	{ .compatible = "allwinner,sun8i-h3-display-engine" },
 	{ .compatible = "allwinner,sun8i-v3s-display-engine" },
 	{ }
 };
-- 
2.12.2

[RFC PATCH 06/11] drm: sun4i: add color space correction support for DE2 mixer

[Icenowy Zheng]

The DE2 mixer can do color space correction needed by TV Encoder with
its DCSC sub-engine.

Add support for it.

Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
---
 drivers/gpu/drm/sun4i/sun8i_mixer.c | 35 +++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/sun4i/sun8i_mixer.h |  6 +++++-
 2 files changed, 40 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.c b/drivers/gpu/drm/sun4i/sun8i_mixer.c
index d658a3a8159a..65f86641eca3 100644
--- a/drivers/gpu/drm/sun4i/sun8i_mixer.c
+++ b/drivers/gpu/drm/sun4i/sun8i_mixer.c
@@ -29,6 +29,14 @@
 #include "sun8i_layer.h"
 #include "sunxi_engine.h"
 
+static const u32 sun8i_rgb2yuv_coef[12] = {
+	0x00000107, 0x00000204, 0x00000064, 0x00004200,
+	0x00001f68, 0x00001ed6, 0x000001c2, 0x00020200,
+	0x000001c2, 0x00001e87, 0x00001fb7, 0x00020200,
+};
+
+static const u32 sun8i_rgb2yuv_dcsc_alpha = 0x00020200;
+
 static void sun8i_mixer_commit(struct sunxi_engine *engine)
 {
 	DRM_DEBUG_DRIVER("Committing changes\n");
@@ -37,6 +45,31 @@ static void sun8i_mixer_commit(struct sunxi_engine *engine)
 		     SUN8I_MIXER_GLOBAL_DBUFF_ENABLE);
 }
 
+static void sun8i_mixer_apply_color_correction(struct sunxi_engine *engine)
+{
+	int i;
+
+	DRM_DEBUG_DRIVER("Applying RGB to YUV color correction\n");
+
+	/* Set color correction */
+	regmap_write(engine->regs, SUN8I_MIXER_DCSC_EN, 1);
+
+	for (i = 0; i < 12; i++)
+		regmap_write(engine->regs, SUN8I_MIXER_DCSC_COEF_REG(i),
+			     sun8i_rgb2yuv_coef[i]);
+
+	regmap_write(engine->regs, SUN8I_MIXER_DCSC_COEF_ALPHA,
+		     sun8i_rgb2yuv_dcsc_alpha);
+}
+
+static void sun8i_mixer_disable_color_correction(struct sunxi_engine *engine)
+{
+	DRM_DEBUG_DRIVER("Disabling color correction\n");
+
+	/* Disable color correction */
+	regmap_write(engine->regs, SUN8I_MIXER_DCSC_EN, 0);
+}
+
 void sun8i_mixer_layer_enable(struct sun8i_mixer *mixer,
 				int layer, bool enable)
 {
@@ -229,6 +262,8 @@ int sun8i_mixer_update_layer_buffer(struct sun8i_mixer *mixer,
 static const struct sunxi_engine_ops sun8i_engine_ops = {
 	.commit		= sun8i_mixer_commit,
 	.layers_init	= sun8i_layers_init,
+	.apply_color_correction		= sun8i_mixer_apply_color_correction,
+	.disable_color_correction	= sun8i_mixer_disable_color_correction,
 };
 
 static struct regmap_config sun8i_mixer_regmap_config = {
diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.h b/drivers/gpu/drm/sun4i/sun8i_mixer.h
index 4785ac090b8c..d7f7513898b6 100644
--- a/drivers/gpu/drm/sun4i/sun8i_mixer.h
+++ b/drivers/gpu/drm/sun4i/sun8i_mixer.h
@@ -88,6 +88,11 @@
 #define SUN8I_MIXER_CHAN_UI_LAYER_ATTR_FBFMT_RGB888	(8 << 8)
 #define SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA_DEF	(0xff << 24)
 
+/* The DCSC sub-engine is used to do color space conversation */
+#define SUN8I_MIXER_DCSC_EN			0xb0000
+#define SUN8I_MIXER_DCSC_COEF_REG(x)		(0xb0010 + 0x4 * x)
+#define SUN8I_MIXER_DCSC_COEF_ALPHA		0xb0040
+
 /*
  * These sub-engines are still unknown now, the EN registers are here only to
  * be used to disable these sub-engines.
@@ -102,7 +107,6 @@
 #define SUN8I_MIXER_PEAK_EN			0xa6000
 #define SUN8I_MIXER_ASE_EN			0xa8000
 #define SUN8I_MIXER_FCC_EN			0xaa000
-#define SUN8I_MIXER_DCSC_EN			0xb0000
 
 struct sun8i_mixer_cfg {
 	int		vi_num;
-- 
2.12.2

[linux-sunxi] [RFC PATCH 06/11] drm: sun4i: add color space correction support for DE2 mixer

[Jernej Škrabec]

Hi,

Dne sreda, 17. maj 2017 ob 18:43:49 CEST je Icenowy Zheng napisal(a):
> The DE2 mixer can do color space correction needed by TV Encoder with
> its DCSC sub-engine.
> 
> Add support for it.
> 
> Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
> ---
>  drivers/gpu/drm/sun4i/sun8i_mixer.c | 35
> +++++++++++++++++++++++++++++++++++ drivers/gpu/drm/sun4i/sun8i_mixer.h | 
> 6 +++++-
>  2 files changed, 40 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.c
> b/drivers/gpu/drm/sun4i/sun8i_mixer.c index d658a3a8159a..65f86641eca3
> 100644
> --- a/drivers/gpu/drm/sun4i/sun8i_mixer.c
> +++ b/drivers/gpu/drm/sun4i/sun8i_mixer.c
> @@ -29,6 +29,14 @@
>  #include "sun8i_layer.h"
>  #include "sunxi_engine.h"
> 
> +static const u32 sun8i_rgb2yuv_coef[12] = {
> +	0x00000107, 0x00000204, 0x00000064, 0x00004200,
> +	0x00001f68, 0x00001ed6, 0x000001c2, 0x00020200,
> +	0x000001c2, 0x00001e87, 0x00001fb7, 0x00020200,
> +};
> +
> +static const u32 sun8i_rgb2yuv_dcsc_alpha = 0x00020200;
> +

There is no need to set/use alpha. BSP code doesn't set it and 0x00020200 
value is default.

Best regards,
Jernej

>  static void sun8i_mixer_commit(struct sunxi_engine *engine)
>  {
>  	DRM_DEBUG_DRIVER("Committing changes\n");
> @@ -37,6 +45,31 @@ static void sun8i_mixer_commit(struct sunxi_engine
> *engine) SUN8I_MIXER_GLOBAL_DBUFF_ENABLE);
>  }
> 
> +static void sun8i_mixer_apply_color_correction(struct sunxi_engine *engine)
> +{
> +	int i;
> +
> +	DRM_DEBUG_DRIVER("Applying RGB to YUV color correction\n");
> +
> +	/* Set color correction */
> +	regmap_write(engine->regs, SUN8I_MIXER_DCSC_EN, 1);
> +
> +	for (i = 0; i < 12; i++)
> +		regmap_write(engine->regs, SUN8I_MIXER_DCSC_COEF_REG(i),
> +			     sun8i_rgb2yuv_coef[i]);
> +
> +	regmap_write(engine->regs, SUN8I_MIXER_DCSC_COEF_ALPHA,
> +		     sun8i_rgb2yuv_dcsc_alpha);
> +}
> +
> +static void sun8i_mixer_disable_color_correction(struct sunxi_engine
> *engine) +{
> +	DRM_DEBUG_DRIVER("Disabling color correction\n");
> +
> +	/* Disable color correction */
> +	regmap_write(engine->regs, SUN8I_MIXER_DCSC_EN, 0);
> +}
> +
>  void sun8i_mixer_layer_enable(struct sun8i_mixer *mixer,
>  				int layer, bool enable)
>  {
> @@ -229,6 +262,8 @@ int sun8i_mixer_update_layer_buffer(struct sun8i_mixer
> *mixer, static const struct sunxi_engine_ops sun8i_engine_ops = {
>  	.commit		= sun8i_mixer_commit,
>  	.layers_init	= sun8i_layers_init,
> +	.apply_color_correction		= sun8i_mixer_apply_color_correction,
> +	.disable_color_correction	= sun8i_mixer_disable_color_correction,
>  };
> 
>  static struct regmap_config sun8i_mixer_regmap_config = {
> diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.h
> b/drivers/gpu/drm/sun4i/sun8i_mixer.h index 4785ac090b8c..d7f7513898b6
> 100644
> --- a/drivers/gpu/drm/sun4i/sun8i_mixer.h
> +++ b/drivers/gpu/drm/sun4i/sun8i_mixer.h
> @@ -88,6 +88,11 @@
>  #define SUN8I_MIXER_CHAN_UI_LAYER_ATTR_FBFMT_RGB888	(8 << 8)
>  #define SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA_DEF	(0xff << 24)
> 
> +/* The DCSC sub-engine is used to do color space conversation */
> +#define SUN8I_MIXER_DCSC_EN			0xb0000
> +#define SUN8I_MIXER_DCSC_COEF_REG(x)		(0xb0010 + 0x4 * x)
> +#define SUN8I_MIXER_DCSC_COEF_ALPHA		0xb0040
> +
>  /*
>   * These sub-engines are still unknown now, the EN registers are here only
> to * be used to disable these sub-engines.
> @@ -102,7 +107,6 @@
>  #define SUN8I_MIXER_PEAK_EN			0xa6000
>  #define SUN8I_MIXER_ASE_EN			0xa8000
>  #define SUN8I_MIXER_FCC_EN			0xaa000
> -#define SUN8I_MIXER_DCSC_EN			0xb0000
> 
>  struct sun8i_mixer_cfg {
>  	int		vi_num;
> --
> 2.12.2
> 
> --
> You received this message because you are subscribed to the Google Groups
> "linux-sunxi" group. To unsubscribe from this group and stop receiving
> emails from it, send an email to linux-sunxi+unsubscribe at googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.

[RFC PATCH 07/11] drm: sun4i: add support for the TV encoder in H3 SoC

[Icenowy Zheng]

Allwinner H3 features a TV encoder similar to the one in earlier SoCs,
but with some different points about clocks:
- It has a mod clock and a bus clock.
- The mod clock must be at a fixed rate to generate signal.

Add support for it.

Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
---
 drivers/gpu/drm/sun4i/sun4i_tv.c | 65 +++++++++++++++++++++++++++++++++++++---
 1 file changed, 61 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun4i_tv.c b/drivers/gpu/drm/sun4i/sun4i_tv.c
index a9cad00d4ee8..c9943103f499 100644
--- a/drivers/gpu/drm/sun4i/sun4i_tv.c
+++ b/drivers/gpu/drm/sun4i/sun4i_tv.c
@@ -13,6 +13,7 @@
 #include <linux/clk.h>
 #include <linux/component.h>
 #include <linux/of_address.h>
+#include <linux/of_device.h>
 #include <linux/regmap.h>
 #include <linux/reset.h>
 
@@ -169,14 +170,23 @@ struct tv_mode {
 	const struct resync_parameters	*resync_params;
 };
 
+struct sun4i_tv_quirks {
+	bool has_mod_clk;
+	bool fixed_clock;
+	unsigned long fixed_clock_rate;
+};
+
 struct sun4i_tv {
 	struct drm_connector	connector;
 	struct drm_encoder	encoder;
 
 	struct clk		*clk;
+	struct clk		*mod_clk;
 	struct regmap		*regs;
 	struct reset_control	*reset;
 
+	const struct sun4i_tv_quirks *quirks;
+
 	struct sun4i_drv	*drv;
 };
 
@@ -578,6 +588,10 @@ static int sun4i_tv_bind(struct device *dev, struct device *master,
 	tv->drv = drv;
 	dev_set_drvdata(dev, tv);
 
+	tv->quirks = of_device_get_match_data(dev);
+	if (!tv->quirks)
+		return -EINVAL;
+
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	regs = devm_ioremap_resource(dev, res);
 	if (IS_ERR(regs)) {
@@ -604,7 +618,10 @@ static int sun4i_tv_bind(struct device *dev, struct device *master,
 		return ret;
 	}
 
-	tv->clk = devm_clk_get(dev, NULL);
+	if (tv->quirks->has_mod_clk)
+		tv->clk = devm_clk_get(dev, "bus");
+	else
+		tv->clk = devm_clk_get(dev, NULL);
 	if (IS_ERR(tv->clk)) {
 		dev_err(dev, "Couldn't get the TV encoder clock\n");
 		ret = PTR_ERR(tv->clk);
@@ -612,6 +629,26 @@ static int sun4i_tv_bind(struct device *dev, struct device *master,
 	}
 	clk_prepare_enable(tv->clk);
 
+	if (tv->quirks->has_mod_clk) {
+		tv->mod_clk = devm_clk_get(dev, "mod");
+		if (IS_ERR(tv->mod_clk)) {
+			dev_err(dev, "Couldn't get the TV encoder mod clock\n");
+			ret = PTR_ERR(tv->mod_clk);
+			goto err_disable_clk;
+		};
+
+		if (tv->quirks->fixed_clock) {
+			ret = clk_set_rate(tv->mod_clk,
+					   tv->quirks->fixed_clock_rate);
+			if (ret) {
+				dev_err(dev, "Couldn't set TV encoder mod clock rate\n");
+				goto err_disable_clk;
+			}
+		}
+
+		clk_prepare_enable(tv->mod_clk);
+	}
+
 	drm_encoder_helper_add(&tv->encoder,
 			       &sun4i_tv_helper_funcs);
 	ret = drm_encoder_init(drm,
@@ -621,14 +658,14 @@ static int sun4i_tv_bind(struct device *dev, struct device *master,
 			       NULL);
 	if (ret) {
 		dev_err(dev, "Couldn't initialise the TV encoder\n");
-		goto err_disable_clk;
+		goto err_disable_mod_clk;
 	}
 
 	tv->encoder.possible_crtcs = drm_of_find_possible_crtcs(drm,
 								dev->of_node);
 	if (!tv->encoder.possible_crtcs) {
 		ret = -EPROBE_DEFER;
-		goto err_disable_clk;
+		goto err_disable_mod_clk;
 	}
 
 	drm_connector_helper_add(&tv->connector,
@@ -649,6 +686,9 @@ static int sun4i_tv_bind(struct device *dev, struct device *master,
 
 err_cleanup_connector:
 	drm_encoder_cleanup(&tv->encoder);
+err_disable_mod_clk:
+	if (tv->quirks->has_mod_clk)
+		clk_disable_unprepare(tv->mod_clk);
 err_disable_clk:
 	clk_disable_unprepare(tv->clk);
 err_assert_reset:
@@ -683,8 +723,25 @@ static int sun4i_tv_remove(struct platform_device *pdev)
 	return 0;
 }
 
+static const struct sun4i_tv_quirks sun4i_a10_tv_quirks = {
+	/* Nothing special */
+};
+
+static const struct sun4i_tv_quirks sun8i_h3_tv_quirks = {
+	.has_mod_clk = true,
+	.fixed_clock = true,
+	.fixed_clock_rate = 216000000UL,
+};
+
 static const struct of_device_id sun4i_tv_of_table[] = {
-	{ .compatible = "allwinner,sun4i-a10-tv-encoder" },
+	{
+		.compatible = "allwinner,sun4i-a10-tv-encoder",
+		.data = &sun4i_a10_tv_quirks,
+	},
+	{
+		.compatible = "allwinner,sun8i-h3-tv-encoder",
+		.data = &sun8i_h3_tv_quirks,
+	},
 	{ }
 };
 MODULE_DEVICE_TABLE(of, sun4i_tv_of_table);
-- 
2.12.2

[RFC PATCH 07/11] drm: sun4i: add support for the TV encoder in H3 SoC

[Maxime Ripard]

On Thu, May 18, 2017 at 12:43:50AM +0800, Icenowy Zheng wrote:
> Allwinner H3 features a TV encoder similar to the one in earlier SoCs,
> but with some different points about clocks:
> - It has a mod clock and a bus clock.
> - The mod clock must be at a fixed rate to generate signal.

Why?

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20170519/f6a71f40/attachment.sig>

[RFC PATCH 07/11] drm: sun4i: add support for the TV encoder in H3 SoC

[Icenowy Zheng]

? 2017?5?20? GMT+08:00 ??2:03:30, Maxime Ripard <maxime.ripard@free-electrons.com> ??:
>On Thu, May 18, 2017 at 12:43:50AM +0800, Icenowy Zheng wrote:
>> Allwinner H3 features a TV encoder similar to the one in earlier
>SoCs,
>> but with some different points about clocks:
>> - It has a mod clock and a bus clock.
>> - The mod clock must be at a fixed rate to generate signal.
>
>Why?

It's experiment result by Jernej.

The clock rates in BSP kernel is also specially designed
(PLL_DE at 432MHz) in order to be able to feed the TVE.

>
>Maxime

[linux-sunxi] Re: [RFC PATCH 07/11] drm: sun4i: add support for the TV encoder in H3 SoC

[Jernej Škrabec]

Hi,

Dne petek, 19. maj 2017 ob 20:08:18 CEST je Icenowy Zheng napisal(a):
> ? 2017?5?20? GMT+08:00 ??2:03:30, Maxime Ripard <maxime.ripard@free-
electrons.com> ??:
> >On Thu, May 18, 2017 at 12:43:50AM +0800, Icenowy Zheng wrote:
> >> Allwinner H3 features a TV encoder similar to the one in earlier
> >
> >SoCs,
> >
> >> but with some different points about clocks:
> >> - It has a mod clock and a bus clock.
> >> - The mod clock must be at a fixed rate to generate signal.
> >
> >Why?
> 
> It's experiment result by Jernej.
> 
> The clock rates in BSP kernel is also specially designed
> (PLL_DE at 432MHz) in order to be able to feed the TVE.

My experiments and search through BSP code showed that TVE seems to have 
additional fixed predivider 8. So if you want to generate 27 MHz clock, unit 
has to be feed with 216 MHz. 

TVE has only one PLL source PLL_DE. And since 216 MHz is a bit low for DE2, 
BSP defaults to 432 MHz for PLL_DE and use divider 2 to generate 216 MHz. This 
clock is then divided by 8 internaly to get final 27 MHz.

Please note that I don't have any hard evidence to support that, only 
experimental data. However, only that explanation make sense to me.

BTW, BSP H3/H5 TV driver supports only PAL and NTSC which both use 27 MHz base 
clock. Further experiments are needed to check if there is any possibility to 
have other resolutions by manipulating clocks and give other proper settings. 
I plan to do that, but not in very near future.

Best regards,
Jernej

[linux-sunxi] Re: [RFC PATCH 07/11] drm: sun4i: add support for the TV encoder in H3 SoC

[Chen-Yu Tsai]

On Sat, May 20, 2017 at 2:23 AM, Jernej ?krabec <jernej.skrabec@siol.net> wrote:
> Hi,
>
> Dne petek, 19. maj 2017 ob 20:08:18 CEST je Icenowy Zheng napisal(a):
>> ? 2017?5?20? GMT+08:00 ??2:03:30, Maxime Ripard <maxime.ripard@free-
> electrons.com> ??:
>> >On Thu, May 18, 2017 at 12:43:50AM +0800, Icenowy Zheng wrote:
>> >> Allwinner H3 features a TV encoder similar to the one in earlier
>> >
>> >SoCs,
>> >
>> >> but with some different points about clocks:
>> >> - It has a mod clock and a bus clock.
>> >> - The mod clock must be at a fixed rate to generate signal.
>> >
>> >Why?
>>
>> It's experiment result by Jernej.
>>
>> The clock rates in BSP kernel is also specially designed
>> (PLL_DE at 432MHz) in order to be able to feed the TVE.
>
> My experiments and search through BSP code showed that TVE seems to have
> additional fixed predivider 8. So if you want to generate 27 MHz clock, unit
> has to be feed with 216 MHz.
>
> TVE has only one PLL source PLL_DE. And since 216 MHz is a bit low for DE2,
> BSP defaults to 432 MHz for PLL_DE and use divider 2 to generate 216 MHz. This
> clock is then divided by 8 internaly to get final 27 MHz.
>
> Please note that I don't have any hard evidence to support that, only
> experimental data. However, only that explanation make sense to me.
>
> BTW, BSP H3/H5 TV driver supports only PAL and NTSC which both use 27 MHz base
> clock. Further experiments are needed to check if there is any possibility to
> have other resolutions by manipulating clocks and give other proper settings.
> I plan to do that, but not in very near future.

You only have composite video output, and those are the only 2 standard
resolutions that make any sense.

ChenYu

[linux-sunxi] Re: [RFC PATCH 07/11] drm: sun4i: add support for the TV encoder in H3 SoC

[Jernej Škrabec]

Hi,

Dne sobota, 20. maj 2017 ob 03:37:53 CEST je Chen-Yu Tsai napisal(a):
> On Sat, May 20, 2017 at 2:23 AM, Jernej ?krabec <jernej.skrabec@siol.net> 
wrote:
> > Hi,
> > 
> > Dne petek, 19. maj 2017 ob 20:08:18 CEST je Icenowy Zheng napisal(a):
> >> ? 2017?5?20? GMT+08:00 ??2:03:30, Maxime Ripard <maxime.ripard@free-
> > 
> > electrons.com> ??:
> >> >On Thu, May 18, 2017 at 12:43:50AM +0800, Icenowy Zheng wrote:
> >> >> Allwinner H3 features a TV encoder similar to the one in earlier
> >> >
> >> >SoCs,
> >> >
> >> >> but with some different points about clocks:
> >> >> - It has a mod clock and a bus clock.
> >> >> - The mod clock must be at a fixed rate to generate signal.
> >> >
> >> >Why?
> >> 
> >> It's experiment result by Jernej.
> >> 
> >> The clock rates in BSP kernel is also specially designed
> >> (PLL_DE at 432MHz) in order to be able to feed the TVE.
> > 
> > My experiments and search through BSP code showed that TVE seems to have
> > additional fixed predivider 8. So if you want to generate 27 MHz clock,
> > unit has to be feed with 216 MHz.
> > 
> > TVE has only one PLL source PLL_DE. And since 216 MHz is a bit low for
> > DE2,
> > BSP defaults to 432 MHz for PLL_DE and use divider 2 to generate 216 MHz.
> > This clock is then divided by 8 internaly to get final 27 MHz.
> > 
> > Please note that I don't have any hard evidence to support that, only
> > experimental data. However, only that explanation make sense to me.
> > 
> > BTW, BSP H3/H5 TV driver supports only PAL and NTSC which both use 27 MHz
> > base clock. Further experiments are needed to check if there is any
> > possibility to have other resolutions by manipulating clocks and give
> > other proper settings. I plan to do that, but not in very near future.
> 
> You only have composite video output, and those are the only 2 standard
> resolutions that make any sense.

Right, other resolutions are for VGA.

Anyway, I did some more digging in A10 and R40 datasheets. I think that H3 TVE 
unit is something in between. R40 TVE has a setting to select "up sample". 
Possible settings are 27 MHz, 54 MHz, 108 MHz and 216 MHz. BSP driver on R40 
has this setting enabled only for PAL and NTSC and it is always 216 MHz. I 
think that H3 may have this hardwired to 216 MHz and this would be the reason 
why 216 MHz is needed.

Has anyone else any better explanation?

Best regards,
Jernej

[linux-sunxi] Re: [RFC PATCH 07/11] drm: sun4i: add support for the TV encoder in H3 SoC

[Maxime Ripard]

On Mon, May 22, 2017 at 07:55:56PM +0200, Jernej ?krabec wrote:
> Hi,
> 
> Dne sobota, 20. maj 2017 ob 03:37:53 CEST je Chen-Yu Tsai napisal(a):
> > On Sat, May 20, 2017 at 2:23 AM, Jernej ?krabec <jernej.skrabec@siol.net> 
> wrote:
> > > Hi,
> > > 
> > > Dne petek, 19. maj 2017 ob 20:08:18 CEST je Icenowy Zheng napisal(a):
> > >> ? 2017?5?20? GMT+08:00 ??2:03:30, Maxime Ripard <maxime.ripard@free-
> > > 
> > > electrons.com> ??:
> > >> >On Thu, May 18, 2017 at 12:43:50AM +0800, Icenowy Zheng wrote:
> > >> >> Allwinner H3 features a TV encoder similar to the one in earlier
> > >> >
> > >> >SoCs,
> > >> >
> > >> >> but with some different points about clocks:
> > >> >> - It has a mod clock and a bus clock.
> > >> >> - The mod clock must be at a fixed rate to generate signal.
> > >> >
> > >> >Why?
> > >> 
> > >> It's experiment result by Jernej.
> > >> 
> > >> The clock rates in BSP kernel is also specially designed
> > >> (PLL_DE at 432MHz) in order to be able to feed the TVE.
> > > 
> > > My experiments and search through BSP code showed that TVE seems to have
> > > additional fixed predivider 8. So if you want to generate 27 MHz clock,
> > > unit has to be feed with 216 MHz.
> > > 
> > > TVE has only one PLL source PLL_DE. And since 216 MHz is a bit low for
> > > DE2,
> > > BSP defaults to 432 MHz for PLL_DE and use divider 2 to generate 216 MHz.
> > > This clock is then divided by 8 internaly to get final 27 MHz.
> > > 
> > > Please note that I don't have any hard evidence to support that, only
> > > experimental data. However, only that explanation make sense to me.
> > > 
> > > BTW, BSP H3/H5 TV driver supports only PAL and NTSC which both use 27 MHz
> > > base clock. Further experiments are needed to check if there is any
> > > possibility to have other resolutions by manipulating clocks and give
> > > other proper settings. I plan to do that, but not in very near future.
> > 
> > You only have composite video output, and those are the only 2 standard
> > resolutions that make any sense.
> 
> Right, other resolutions are for VGA.
> 
> Anyway, I did some more digging in A10 and R40 datasheets. I think that H3 TVE 
> unit is something in between. R40 TVE has a setting to select "up sample".

That might be just another translation of oversampling :)

I didn't know it could be applied to composite signals though, but I
guess this is just another analog signal after all.

> Possible settings are 27 MHz, 54 MHz, 108 MHz and 216 MHz. BSP driver on R40 
> has this setting enabled only for PAL and NTSC and it is always 216 MHz. I 
> think that H3 may have this hardwired to 216 MHz and this would be the reason 
> why 216 MHz is needed.
> 
> Has anyone else any better explanation?

That's already a pretty good one.

Either way, wether this is upsampling, oversampling or just a
pre-divider, this can and should be dealt with in the mode_set
callback, and not in the probe.

Thanks!
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20170523/9b264e48/attachment.sig>

[linux-sunxi] Re: [RFC PATCH 07/11] drm: sun4i: add support for the TV encoder in H3 SoC

[Icenowy Zheng]

? 2017?5?23? GMT+08:00 ??8:53:21, Maxime Ripard <maxime.ripard@free-electrons.com> ??:
>On Mon, May 22, 2017 at 07:55:56PM +0200, Jernej ?krabec wrote:
>> Hi,
>> 
>> Dne sobota, 20. maj 2017 ob 03:37:53 CEST je Chen-Yu Tsai napisal(a):
>> > On Sat, May 20, 2017 at 2:23 AM, Jernej ?krabec
><jernej.skrabec@siol.net> 
>> wrote:
>> > > Hi,
>> > > 
>> > > Dne petek, 19. maj 2017 ob 20:08:18 CEST je Icenowy Zheng
>napisal(a):
>> > >> ? 2017?5?20? GMT+08:00 ??2:03:30, Maxime Ripard
><maxime.ripard@free-
>> > > 
>> > > electrons.com> ??:
>> > >> >On Thu, May 18, 2017 at 12:43:50AM +0800, Icenowy Zheng wrote:
>> > >> >> Allwinner H3 features a TV encoder similar to the one in
>earlier
>> > >> >
>> > >> >SoCs,
>> > >> >
>> > >> >> but with some different points about clocks:
>> > >> >> - It has a mod clock and a bus clock.
>> > >> >> - The mod clock must be at a fixed rate to generate signal.
>> > >> >
>> > >> >Why?
>> > >> 
>> > >> It's experiment result by Jernej.
>> > >> 
>> > >> The clock rates in BSP kernel is also specially designed
>> > >> (PLL_DE at 432MHz) in order to be able to feed the TVE.
>> > > 
>> > > My experiments and search through BSP code showed that TVE seems
>to have
>> > > additional fixed predivider 8. So if you want to generate 27 MHz
>clock,
>> > > unit has to be feed with 216 MHz.
>> > > 
>> > > TVE has only one PLL source PLL_DE. And since 216 MHz is a bit
>low for
>> > > DE2,
>> > > BSP defaults to 432 MHz for PLL_DE and use divider 2 to generate
>216 MHz.
>> > > This clock is then divided by 8 internaly to get final 27 MHz.
>> > > 
>> > > Please note that I don't have any hard evidence to support that,
>only
>> > > experimental data. However, only that explanation make sense to
>me.
>> > > 
>> > > BTW, BSP H3/H5 TV driver supports only PAL and NTSC which both
>use 27 MHz
>> > > base clock. Further experiments are needed to check if there is
>any
>> > > possibility to have other resolutions by manipulating clocks and
>give
>> > > other proper settings. I plan to do that, but not in very near
>future.
>> > 
>> > You only have composite video output, and those are the only 2
>standard
>> > resolutions that make any sense.
>> 
>> Right, other resolutions are for VGA.
>> 
>> Anyway, I did some more digging in A10 and R40 datasheets. I think
>that H3 TVE 
>> unit is something in between. R40 TVE has a setting to select "up
>sample".
>
>That might be just another translation of oversampling :)
>
>I didn't know it could be applied to composite signals though, but I
>guess this is just another analog signal after all.
>
>> Possible settings are 27 MHz, 54 MHz, 108 MHz and 216 MHz. BSP driver
>on R40 
>> has this setting enabled only for PAL and NTSC and it is always 216
>MHz. I 
>> think that H3 may have this hardwired to 216 MHz and this would be
>the reason 
>> why 216 MHz is needed.
>> 
>> Has anyone else any better explanation?
>
>That's already a pretty good one.
>
>Either way, wether this is upsampling, oversampling or just a
>pre-divider, this can and should be dealt with in the mode_set
>callback, and not in the probe.

What should we do for this?

Add a hook in TCON driver and let TVE driver affect the clock value (*16, as the dotclock is halfed)?

>
>Thanks!
>Maxime

[linux-sunxi] Re: [RFC PATCH 07/11] drm: sun4i: add support for the TV encoder in H3 SoC

[icenowy at aosc.io]

? 2017-05-23 20:53?Maxime Ripard ???
> On Mon, May 22, 2017 at 07:55:56PM +0200, Jernej ?krabec wrote:
>> Hi,
>> 
>> Dne sobota, 20. maj 2017 ob 03:37:53 CEST je Chen-Yu Tsai napisal(a):
>> > On Sat, May 20, 2017 at 2:23 AM, Jernej ?krabec <jernej.skrabec@siol.net>
>> wrote:
>> > > Hi,
>> > >
>> > > Dne petek, 19. maj 2017 ob 20:08:18 CEST je Icenowy Zheng napisal(a):
>> > >> ? 2017?5?20? GMT+08:00 ??2:03:30, Maxime Ripard <maxime.ripard@free-
>> > >
>> > > electrons.com> ??:
>> > >> >On Thu, May 18, 2017 at 12:43:50AM +0800, Icenowy Zheng wrote:
>> > >> >> Allwinner H3 features a TV encoder similar to the one in earlier
>> > >> >
>> > >> >SoCs,
>> > >> >
>> > >> >> but with some different points about clocks:
>> > >> >> - It has a mod clock and a bus clock.
>> > >> >> - The mod clock must be at a fixed rate to generate signal.
>> > >> >
>> > >> >Why?
>> > >>
>> > >> It's experiment result by Jernej.
>> > >>
>> > >> The clock rates in BSP kernel is also specially designed
>> > >> (PLL_DE at 432MHz) in order to be able to feed the TVE.
>> > >
>> > > My experiments and search through BSP code showed that TVE seems to have
>> > > additional fixed predivider 8. So if you want to generate 27 MHz clock,
>> > > unit has to be feed with 216 MHz.
>> > >
>> > > TVE has only one PLL source PLL_DE. And since 216 MHz is a bit low for
>> > > DE2,
>> > > BSP defaults to 432 MHz for PLL_DE and use divider 2 to generate 216 MHz.
>> > > This clock is then divided by 8 internaly to get final 27 MHz.
>> > >
>> > > Please note that I don't have any hard evidence to support that, only
>> > > experimental data. However, only that explanation make sense to me.
>> > >
>> > > BTW, BSP H3/H5 TV driver supports only PAL and NTSC which both use 27 MHz
>> > > base clock. Further experiments are needed to check if there is any
>> > > possibility to have other resolutions by manipulating clocks and give
>> > > other proper settings. I plan to do that, but not in very near future.
>> >
>> > You only have composite video output, and those are the only 2 standard
>> > resolutions that make any sense.
>> 
>> Right, other resolutions are for VGA.
>> 
>> Anyway, I did some more digging in A10 and R40 datasheets. I think 
>> that H3 TVE
>> unit is something in between. R40 TVE has a setting to select "up 
>> sample".
> 
> That might be just another translation of oversampling :)
> 
> I didn't know it could be applied to composite signals though, but I
> guess this is just another analog signal after all.
> 
>> Possible settings are 27 MHz, 54 MHz, 108 MHz and 216 MHz. BSP driver 
>> on R40
>> has this setting enabled only for PAL and NTSC and it is always 216 
>> MHz. I
>> think that H3 may have this hardwired to 216 MHz and this would be the 
>> reason
>> why 216 MHz is needed.
>> 
>> Has anyone else any better explanation?
> 
> That's already a pretty good one.
> 
> Either way, wether this is upsampling, oversampling or just a
> pre-divider, this can and should be dealt with in the mode_set
> callback, and not in the probe.

I got a better idea -- let TVE driver have the CLK_TVE as an
input and create a subclock output with divider 16, and feed this
subclock to TCON lcd-ch1.

This is a model of the real hardware -- the clock divider is in
TVE, not TCON.

> 
> Thanks!
> Maxime
> 
> --
> Maxime Ripard, Free Electrons
> Embedded Linux and Kernel engineering
> http://free-electrons.com

[linux-sunxi] Re: [RFC PATCH 07/11] drm: sun4i: add support for the TV encoder in H3 SoC

[Maxime Ripard]

On Tue, May 23, 2017 at 09:00:59PM +0800, icenowy at aosc.io wrote:
> ? 2017-05-23 20:53?Maxime Ripard ???
> > On Mon, May 22, 2017 at 07:55:56PM +0200, Jernej ?krabec wrote:
> > > Hi,
> > > 
> > > Dne sobota, 20. maj 2017 ob 03:37:53 CEST je Chen-Yu Tsai napisal(a):
> > > > On Sat, May 20, 2017 at 2:23 AM, Jernej ?krabec <jernej.skrabec@siol.net>
> > > wrote:
> > > > > Hi,
> > > > >
> > > > > Dne petek, 19. maj 2017 ob 20:08:18 CEST je Icenowy Zheng napisal(a):
> > > > >> ? 2017?5?20? GMT+08:00 ??2:03:30, Maxime Ripard <maxime.ripard@free-
> > > > >
> > > > > electrons.com> ??:
> > > > >> >On Thu, May 18, 2017 at 12:43:50AM +0800, Icenowy Zheng wrote:
> > > > >> >> Allwinner H3 features a TV encoder similar to the one in earlier
> > > > >> >
> > > > >> >SoCs,
> > > > >> >
> > > > >> >> but with some different points about clocks:
> > > > >> >> - It has a mod clock and a bus clock.
> > > > >> >> - The mod clock must be at a fixed rate to generate signal.
> > > > >> >
> > > > >> >Why?
> > > > >>
> > > > >> It's experiment result by Jernej.
> > > > >>
> > > > >> The clock rates in BSP kernel is also specially designed
> > > > >> (PLL_DE at 432MHz) in order to be able to feed the TVE.
> > > > >
> > > > > My experiments and search through BSP code showed that TVE seems to have
> > > > > additional fixed predivider 8. So if you want to generate 27 MHz clock,
> > > > > unit has to be feed with 216 MHz.
> > > > >
> > > > > TVE has only one PLL source PLL_DE. And since 216 MHz is a bit low for
> > > > > DE2,
> > > > > BSP defaults to 432 MHz for PLL_DE and use divider 2 to generate 216 MHz.
> > > > > This clock is then divided by 8 internaly to get final 27 MHz.
> > > > >
> > > > > Please note that I don't have any hard evidence to support that, only
> > > > > experimental data. However, only that explanation make sense to me.
> > > > >
> > > > > BTW, BSP H3/H5 TV driver supports only PAL and NTSC which both use 27 MHz
> > > > > base clock. Further experiments are needed to check if there is any
> > > > > possibility to have other resolutions by manipulating clocks and give
> > > > > other proper settings. I plan to do that, but not in very near future.
> > > >
> > > > You only have composite video output, and those are the only 2 standard
> > > > resolutions that make any sense.
> > > 
> > > Right, other resolutions are for VGA.
> > > 
> > > Anyway, I did some more digging in A10 and R40 datasheets. I think
> > > that H3 TVE
> > > unit is something in between. R40 TVE has a setting to select "up
> > > sample".
> > 
> > That might be just another translation of oversampling :)
> > 
> > I didn't know it could be applied to composite signals though, but I
> > guess this is just another analog signal after all.
> > 
> > > Possible settings are 27 MHz, 54 MHz, 108 MHz and 216 MHz. BSP
> > > driver on R40
> > > has this setting enabled only for PAL and NTSC and it is always 216
> > > MHz. I
> > > think that H3 may have this hardwired to 216 MHz and this would be
> > > the reason
> > > why 216 MHz is needed.
> > > 
> > > Has anyone else any better explanation?
> > 
> > That's already a pretty good one.
> > 
> > Either way, wether this is upsampling, oversampling or just a
> > pre-divider, this can and should be dealt with in the mode_set
> > callback, and not in the probe.
> 
> I got a better idea -- let TVE driver have the CLK_TVE as an
> input and create a subclock output with divider 16, and feed this
> subclock to TCON lcd-ch1.
> 
> This is a model of the real hardware -- the clock divider is in
> TVE, not TCON.

That's definitely not a good representation of the hardware. There's
one clock, it goes to the TCON, period.

However, the TV encoder has a constraint on that clock rate. This can
be easily implemented using a custom encoder state where you'd set the
multiplier to set on that clock, and the TCON will use it.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20170524/76babc64/attachment.sig>

[linux-sunxi] Re: [RFC PATCH 07/11] drm: sun4i: add support for the TV encoder in H3 SoC

[Icenowy Zheng]

? 2017?5?24? GMT+08:00 ??3:30:19, Maxime Ripard <maxime.ripard@free-electrons.com> ??:
>On Tue, May 23, 2017 at 09:00:59PM +0800, icenowy at aosc.io wrote:
>> ? 2017-05-23 20:53?Maxime Ripard ???
>> > On Mon, May 22, 2017 at 07:55:56PM +0200, Jernej ?krabec wrote:
>> > > Hi,
>> > > 
>> > > Dne sobota, 20. maj 2017 ob 03:37:53 CEST je Chen-Yu Tsai
>napisal(a):
>> > > > On Sat, May 20, 2017 at 2:23 AM, Jernej ?krabec
><jernej.skrabec@siol.net>
>> > > wrote:
>> > > > > Hi,
>> > > > >
>> > > > > Dne petek, 19. maj 2017 ob 20:08:18 CEST je Icenowy Zheng
>napisal(a):
>> > > > >> ? 2017?5?20? GMT+08:00 ??2:03:30, Maxime Ripard
><maxime.ripard@free-
>> > > > >
>> > > > > electrons.com> ??:
>> > > > >> >On Thu, May 18, 2017 at 12:43:50AM +0800, Icenowy Zheng
>wrote:
>> > > > >> >> Allwinner H3 features a TV encoder similar to the one in
>earlier
>> > > > >> >
>> > > > >> >SoCs,
>> > > > >> >
>> > > > >> >> but with some different points about clocks:
>> > > > >> >> - It has a mod clock and a bus clock.
>> > > > >> >> - The mod clock must be at a fixed rate to generate
>signal.
>> > > > >> >
>> > > > >> >Why?
>> > > > >>
>> > > > >> It's experiment result by Jernej.
>> > > > >>
>> > > > >> The clock rates in BSP kernel is also specially designed
>> > > > >> (PLL_DE at 432MHz) in order to be able to feed the TVE.
>> > > > >
>> > > > > My experiments and search through BSP code showed that TVE
>seems to have
>> > > > > additional fixed predivider 8. So if you want to generate 27
>MHz clock,
>> > > > > unit has to be feed with 216 MHz.
>> > > > >
>> > > > > TVE has only one PLL source PLL_DE. And since 216 MHz is a
>bit low for
>> > > > > DE2,
>> > > > > BSP defaults to 432 MHz for PLL_DE and use divider 2 to
>generate 216 MHz.
>> > > > > This clock is then divided by 8 internaly to get final 27
>MHz.
>> > > > >
>> > > > > Please note that I don't have any hard evidence to support
>that, only
>> > > > > experimental data. However, only that explanation make sense
>to me.
>> > > > >
>> > > > > BTW, BSP H3/H5 TV driver supports only PAL and NTSC which
>both use 27 MHz
>> > > > > base clock. Further experiments are needed to check if there
>is any
>> > > > > possibility to have other resolutions by manipulating clocks
>and give
>> > > > > other proper settings. I plan to do that, but not in very
>near future.
>> > > >
>> > > > You only have composite video output, and those are the only 2
>standard
>> > > > resolutions that make any sense.
>> > > 
>> > > Right, other resolutions are for VGA.
>> > > 
>> > > Anyway, I did some more digging in A10 and R40 datasheets. I
>think
>> > > that H3 TVE
>> > > unit is something in between. R40 TVE has a setting to select "up
>> > > sample".
>> > 
>> > That might be just another translation of oversampling :)
>> > 
>> > I didn't know it could be applied to composite signals though, but
>I
>> > guess this is just another analog signal after all.
>> > 
>> > > Possible settings are 27 MHz, 54 MHz, 108 MHz and 216 MHz. BSP
>> > > driver on R40
>> > > has this setting enabled only for PAL and NTSC and it is always
>216
>> > > MHz. I
>> > > think that H3 may have this hardwired to 216 MHz and this would
>be
>> > > the reason
>> > > why 216 MHz is needed.
>> > > 
>> > > Has anyone else any better explanation?
>> > 
>> > That's already a pretty good one.
>> > 
>> > Either way, wether this is upsampling, oversampling or just a
>> > pre-divider, this can and should be dealt with in the mode_set
>> > callback, and not in the probe.
>> 
>> I got a better idea -- let TVE driver have the CLK_TVE as an
>> input and create a subclock output with divider 16, and feed this
>> subclock to TCON lcd-ch1.
>> 
>> This is a model of the real hardware -- the clock divider is in
>> TVE, not TCON.
>
>That's definitely not a good representation of the hardware. There's
>one clock, it goes to the TCON, period.

No, I still think it goes to the TVE as:

1. it's named TVE in datasheet.
2. Generating signal with such a low resolution but such
a high dotclock is not a good situation.

>
>However, the TV encoder has a constraint on that clock rate. This can
>be easily implemented using a custom encoder state where you'd set the
>multiplier to set on that clock, and the TCON will use it.
>
>Maxime

[linux-sunxi] Re: [RFC PATCH 07/11] drm: sun4i: add support for the TV encoder in H3 SoC

[Jernej Škrabec]

Hi,

Dne sreda, 24. maj 2017 ob 10:25:46 CEST je Icenowy Zheng napisal(a):
> ? 2017?5?24? GMT+08:00 ??3:30:19, Maxime Ripard <maxime.ripard@free-
electrons.com> ??:
> >On Tue, May 23, 2017 at 09:00:59PM +0800, icenowy at aosc.io wrote:
> >> ? 2017-05-23 20:53?Maxime Ripard ???
> >> 
> >> > On Mon, May 22, 2017 at 07:55:56PM +0200, Jernej ?krabec wrote:
> >> > > Hi,
> >> > > 
> >> > > Dne sobota, 20. maj 2017 ob 03:37:53 CEST je Chen-Yu Tsai
> >
> >napisal(a):
> >> > > > On Sat, May 20, 2017 at 2:23 AM, Jernej ?krabec
> >
> ><jernej.skrabec@siol.net>
> >
> >> > > wrote:
> >> > > > > Hi,
> >> > > > > 
> >> > > > > Dne petek, 19. maj 2017 ob 20:08:18 CEST je Icenowy Zheng
> >
> >napisal(a):
> >> > > > >> ? 2017?5?20? GMT+08:00 ??2:03:30, Maxime Ripard
> >
> ><maxime.ripard@free-
> >
> >> > > > > electrons.com> ??:
> >> > > > >> >On Thu, May 18, 2017 at 12:43:50AM +0800, Icenowy Zheng
> >
> >wrote:
> >> > > > >> >> Allwinner H3 features a TV encoder similar to the one in
> >
> >earlier
> >
> >> > > > >> >SoCs,
> >> > > > >> >
> >> > > > >> >> but with some different points about clocks:
> >> > > > >> >> - It has a mod clock and a bus clock.
> >> > > > >> >> - The mod clock must be at a fixed rate to generate
> >
> >signal.
> >
> >> > > > >> >Why?
> >> > > > >> 
> >> > > > >> It's experiment result by Jernej.
> >> > > > >> 
> >> > > > >> The clock rates in BSP kernel is also specially designed
> >> > > > >> (PLL_DE at 432MHz) in order to be able to feed the TVE.
> >> > > > > 
> >> > > > > My experiments and search through BSP code showed that TVE
> >
> >seems to have
> >
> >> > > > > additional fixed predivider 8. So if you want to generate 27
> >
> >MHz clock,
> >
> >> > > > > unit has to be feed with 216 MHz.
> >> > > > > 
> >> > > > > TVE has only one PLL source PLL_DE. And since 216 MHz is a
> >
> >bit low for
> >
> >> > > > > DE2,
> >> > > > > BSP defaults to 432 MHz for PLL_DE and use divider 2 to
> >
> >generate 216 MHz.
> >
> >> > > > > This clock is then divided by 8 internaly to get final 27
> >
> >MHz.
> >
> >> > > > > Please note that I don't have any hard evidence to support
> >
> >that, only
> >
> >> > > > > experimental data. However, only that explanation make sense
> >
> >to me.
> >
> >> > > > > BTW, BSP H3/H5 TV driver supports only PAL and NTSC which
> >
> >both use 27 MHz
> >
> >> > > > > base clock. Further experiments are needed to check if there
> >
> >is any
> >
> >> > > > > possibility to have other resolutions by manipulating clocks
> >
> >and give
> >
> >> > > > > other proper settings. I plan to do that, but not in very
> >
> >near future.
> >
> >> > > > You only have composite video output, and those are the only 2
> >
> >standard
> >
> >> > > > resolutions that make any sense.
> >> > > 
> >> > > Right, other resolutions are for VGA.
> >> > > 
> >> > > Anyway, I did some more digging in A10 and R40 datasheets. I
> >
> >think
> >
> >> > > that H3 TVE
> >> > > unit is something in between. R40 TVE has a setting to select "up
> >> > > sample".
> >> > 
> >> > That might be just another translation of oversampling :)
> >> > 
> >> > I didn't know it could be applied to composite signals though, but
> >
> >I
> >
> >> > guess this is just another analog signal after all.
> >> > 
> >> > > Possible settings are 27 MHz, 54 MHz, 108 MHz and 216 MHz. BSP
> >> > > driver on R40
> >> > > has this setting enabled only for PAL and NTSC and it is always
> >
> >216
> >
> >> > > MHz. I
> >> > > think that H3 may have this hardwired to 216 MHz and this would
> >
> >be
> >
> >> > > the reason
> >> > > why 216 MHz is needed.
> >> > > 
> >> > > Has anyone else any better explanation?
> >> > 
> >> > That's already a pretty good one.
> >> > 
> >> > Either way, wether this is upsampling, oversampling or just a
> >> > pre-divider, this can and should be dealt with in the mode_set
> >> > callback, and not in the probe.
> >> 
> >> I got a better idea -- let TVE driver have the CLK_TVE as an
> >> input and create a subclock output with divider 16, and feed this
> >> subclock to TCON lcd-ch1.
> >> 
> >> This is a model of the real hardware -- the clock divider is in
> >> TVE, not TCON.

If we are talking about HW divider, it is 8 (216 / 27 = 8).

Slightly offtopic, reason why DE2 is hardcoded to 432 might be that for 4K 
resolution you need at least 297 MHz. So next dividable frequency is taken 
(432 MHz). That way you can have 4K HDMI display and composite TV connected at 
the same time, although this sounds a bit weird.

Best regards,
Jernej

> >
> >That's definitely not a good representation of the hardware. There's
> >one clock, it goes to the TCON, period.
> 
> No, I still think it goes to the TVE as:
> 
> 1. it's named TVE in datasheet.
> 2. Generating signal with such a low resolution but such
> a high dotclock is not a good situation.
> 
> >However, the TV encoder has a constraint on that clock rate. This can
> >be easily implemented using a custom encoder state where you'd set the
> >multiplier to set on that clock, and the TCON will use it.
> >
> >Maxime

[linux-sunxi] Re: [RFC PATCH 07/11] drm: sun4i: add support for the TV encoder in H3 SoC

[Maxime Ripard]

On Wed, May 24, 2017 at 04:25:46PM +0800, Icenowy Zheng wrote:
> 
> 
> ? 2017?5?24? GMT+08:00 ??3:30:19, Maxime Ripard <maxime.ripard@free-electrons.com> ??:
> >On Tue, May 23, 2017 at 09:00:59PM +0800, icenowy at aosc.io wrote:
> >> ? 2017-05-23 20:53?Maxime Ripard ???
> >> > On Mon, May 22, 2017 at 07:55:56PM +0200, Jernej ?krabec wrote:
> >> > > Hi,
> >> > > 
> >> > > Dne sobota, 20. maj 2017 ob 03:37:53 CEST je Chen-Yu Tsai
> >napisal(a):
> >> > > > On Sat, May 20, 2017 at 2:23 AM, Jernej ?krabec
> ><jernej.skrabec@siol.net>
> >> > > wrote:
> >> > > > > Hi,
> >> > > > >
> >> > > > > Dne petek, 19. maj 2017 ob 20:08:18 CEST je Icenowy Zheng
> >napisal(a):
> >> > > > >> ? 2017?5?20? GMT+08:00 ??2:03:30, Maxime Ripard
> ><maxime.ripard@free-
> >> > > > >
> >> > > > > electrons.com> ??:
> >> > > > >> >On Thu, May 18, 2017 at 12:43:50AM +0800, Icenowy Zheng
> >wrote:
> >> > > > >> >> Allwinner H3 features a TV encoder similar to the one in
> >earlier
> >> > > > >> >
> >> > > > >> >SoCs,
> >> > > > >> >
> >> > > > >> >> but with some different points about clocks:
> >> > > > >> >> - It has a mod clock and a bus clock.
> >> > > > >> >> - The mod clock must be at a fixed rate to generate
> >signal.
> >> > > > >> >
> >> > > > >> >Why?
> >> > > > >>
> >> > > > >> It's experiment result by Jernej.
> >> > > > >>
> >> > > > >> The clock rates in BSP kernel is also specially designed
> >> > > > >> (PLL_DE at 432MHz) in order to be able to feed the TVE.
> >> > > > >
> >> > > > > My experiments and search through BSP code showed that TVE
> >seems to have
> >> > > > > additional fixed predivider 8. So if you want to generate 27
> >MHz clock,
> >> > > > > unit has to be feed with 216 MHz.
> >> > > > >
> >> > > > > TVE has only one PLL source PLL_DE. And since 216 MHz is a
> >bit low for
> >> > > > > DE2,
> >> > > > > BSP defaults to 432 MHz for PLL_DE and use divider 2 to
> >generate 216 MHz.
> >> > > > > This clock is then divided by 8 internaly to get final 27
> >MHz.
> >> > > > >
> >> > > > > Please note that I don't have any hard evidence to support
> >that, only
> >> > > > > experimental data. However, only that explanation make sense
> >to me.
> >> > > > >
> >> > > > > BTW, BSP H3/H5 TV driver supports only PAL and NTSC which
> >both use 27 MHz
> >> > > > > base clock. Further experiments are needed to check if there
> >is any
> >> > > > > possibility to have other resolutions by manipulating clocks
> >and give
> >> > > > > other proper settings. I plan to do that, but not in very
> >near future.
> >> > > >
> >> > > > You only have composite video output, and those are the only 2
> >standard
> >> > > > resolutions that make any sense.
> >> > > 
> >> > > Right, other resolutions are for VGA.
> >> > > 
> >> > > Anyway, I did some more digging in A10 and R40 datasheets. I
> >think
> >> > > that H3 TVE
> >> > > unit is something in between. R40 TVE has a setting to select "up
> >> > > sample".
> >> > 
> >> > That might be just another translation of oversampling :)
> >> > 
> >> > I didn't know it could be applied to composite signals though, but
> >I
> >> > guess this is just another analog signal after all.
> >> > 
> >> > > Possible settings are 27 MHz, 54 MHz, 108 MHz and 216 MHz. BSP
> >> > > driver on R40
> >> > > has this setting enabled only for PAL and NTSC and it is always
> >216
> >> > > MHz. I
> >> > > think that H3 may have this hardwired to 216 MHz and this would
> >be
> >> > > the reason
> >> > > why 216 MHz is needed.
> >> > > 
> >> > > Has anyone else any better explanation?
> >> > 
> >> > That's already a pretty good one.
> >> > 
> >> > Either way, wether this is upsampling, oversampling or just a
> >> > pre-divider, this can and should be dealt with in the mode_set
> >> > callback, and not in the probe.
> >> 
> >> I got a better idea -- let TVE driver have the CLK_TVE as an
> >> input and create a subclock output with divider 16, and feed this
> >> subclock to TCON lcd-ch1.
> >> 
> >> This is a model of the real hardware -- the clock divider is in
> >> TVE, not TCON.
> >
> >That's definitely not a good representation of the hardware. There's
> >one clock, it goes to the TCON, period.
> 
> No, I still think it goes to the TVE as:
> 
> 1. it's named TVE in datasheet.

Feel free to come up with a better, more sensible explanation?

> 2. Generating signal with such a low resolution but such
> a high dotclock is not a good situation.

What? What do you mean? The pixel clock is 27MHz, I'm pretty sure we
agree on that.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20170531/242eb47e/attachment.sig>

[linux-sunxi] Re: [RFC PATCH 07/11] drm: sun4i: add support for the TV encoder in H3 SoC

[icenowy at aosc.io]

? 2017-06-01 02:43?Maxime Ripard ???
> On Wed, May 24, 2017 at 04:25:46PM +0800, Icenowy Zheng wrote:
>> 
>> 
>> ? 2017?5?24? GMT+08:00 ??3:30:19, Maxime Ripard 
>> <maxime.ripard@free-electrons.com> ??:
>> >On Tue, May 23, 2017 at 09:00:59PM +0800, icenowy at aosc.io wrote:
>> >> ? 2017-05-23 20:53?Maxime Ripard ???
>> >> > On Mon, May 22, 2017 at 07:55:56PM +0200, Jernej ?krabec wrote:
>> >> > > Hi,
>> >> > >
>> >> > > Dne sobota, 20. maj 2017 ob 03:37:53 CEST je Chen-Yu Tsai
>> >napisal(a):
>> >> > > > On Sat, May 20, 2017 at 2:23 AM, Jernej ?krabec
>> ><jernej.skrabec@siol.net>
>> >> > > wrote:
>> >> > > > > Hi,
>> >> > > > >
>> >> > > > > Dne petek, 19. maj 2017 ob 20:08:18 CEST je Icenowy Zheng
>> >napisal(a):
>> >> > > > >> ? 2017?5?20? GMT+08:00 ??2:03:30, Maxime Ripard
>> ><maxime.ripard@free-
>> >> > > > >
>> >> > > > > electrons.com> ??:
>> >> > > > >> >On Thu, May 18, 2017 at 12:43:50AM +0800, Icenowy Zheng
>> >wrote:
>> >> > > > >> >> Allwinner H3 features a TV encoder similar to the one in
>> >earlier
>> >> > > > >> >
>> >> > > > >> >SoCs,
>> >> > > > >> >
>> >> > > > >> >> but with some different points about clocks:
>> >> > > > >> >> - It has a mod clock and a bus clock.
>> >> > > > >> >> - The mod clock must be at a fixed rate to generate
>> >signal.
>> >> > > > >> >
>> >> > > > >> >Why?
>> >> > > > >>
>> >> > > > >> It's experiment result by Jernej.
>> >> > > > >>
>> >> > > > >> The clock rates in BSP kernel is also specially designed
>> >> > > > >> (PLL_DE at 432MHz) in order to be able to feed the TVE.
>> >> > > > >
>> >> > > > > My experiments and search through BSP code showed that TVE
>> >seems to have
>> >> > > > > additional fixed predivider 8. So if you want to generate 27
>> >MHz clock,
>> >> > > > > unit has to be feed with 216 MHz.
>> >> > > > >
>> >> > > > > TVE has only one PLL source PLL_DE. And since 216 MHz is a
>> >bit low for
>> >> > > > > DE2,
>> >> > > > > BSP defaults to 432 MHz for PLL_DE and use divider 2 to
>> >generate 216 MHz.
>> >> > > > > This clock is then divided by 8 internaly to get final 27
>> >MHz.
>> >> > > > >
>> >> > > > > Please note that I don't have any hard evidence to support
>> >that, only
>> >> > > > > experimental data. However, only that explanation make sense
>> >to me.
>> >> > > > >
>> >> > > > > BTW, BSP H3/H5 TV driver supports only PAL and NTSC which
>> >both use 27 MHz
>> >> > > > > base clock. Further experiments are needed to check if there
>> >is any
>> >> > > > > possibility to have other resolutions by manipulating clocks
>> >and give
>> >> > > > > other proper settings. I plan to do that, but not in very
>> >near future.
>> >> > > >
>> >> > > > You only have composite video output, and those are the only 2
>> >standard
>> >> > > > resolutions that make any sense.
>> >> > >
>> >> > > Right, other resolutions are for VGA.
>> >> > >
>> >> > > Anyway, I did some more digging in A10 and R40 datasheets. I
>> >think
>> >> > > that H3 TVE
>> >> > > unit is something in between. R40 TVE has a setting to select "up
>> >> > > sample".
>> >> >
>> >> > That might be just another translation of oversampling :)
>> >> >
>> >> > I didn't know it could be applied to composite signals though, but
>> >I
>> >> > guess this is just another analog signal after all.
>> >> >
>> >> > > Possible settings are 27 MHz, 54 MHz, 108 MHz and 216 MHz. BSP
>> >> > > driver on R40
>> >> > > has this setting enabled only for PAL and NTSC and it is always
>> >216
>> >> > > MHz. I
>> >> > > think that H3 may have this hardwired to 216 MHz and this would
>> >be
>> >> > > the reason
>> >> > > why 216 MHz is needed.
>> >> > >
>> >> > > Has anyone else any better explanation?
>> >> >
>> >> > That's already a pretty good one.
>> >> >
>> >> > Either way, wether this is upsampling, oversampling or just a
>> >> > pre-divider, this can and should be dealt with in the mode_set
>> >> > callback, and not in the probe.
>> >>
>> >> I got a better idea -- let TVE driver have the CLK_TVE as an
>> >> input and create a subclock output with divider 16, and feed this
>> >> subclock to TCON lcd-ch1.
>> >>
>> >> This is a model of the real hardware -- the clock divider is in
>> >> TVE, not TCON.
>> >
>> >That's definitely not a good representation of the hardware. There's
>> >one clock, it goes to the TCON, period.
>> 
>> No, I still think it goes to the TVE as:
>> 
>> 1. it's named TVE in datasheet.
> 
> Feel free to come up with a better, more sensible explanation?
> 
>> 2. Generating signal with such a low resolution but such
>> a high dotclock is not a good situation.
> 
> What? What do you mean? The pixel clock is 27MHz, I'm pretty sure we
> agree on that.

Yes, so I don't think the TCON code should set the clock to any value
rather than 27MHz.

So we should create a clock with divider in TVE, and feed it to TCON.

> 
> Maxime
> 
> --
> Maxime Ripard, Free Electrons
> Embedded Linux and Kernel engineering
> http://free-electrons.com

[linux-sunxi] Re: [RFC PATCH 07/11] drm: sun4i: add support for the TV encoder in H3 SoC

[Maxime Ripard]

On Thu, Jun 01, 2017 at 10:11:14PM +0800, icenowy at aosc.io wrote:
> ? 2017-06-01 02:43?Maxime Ripard ???
> > On Wed, May 24, 2017 at 04:25:46PM +0800, Icenowy Zheng wrote:
> > > 
> > > 
> > > ? 2017?5?24? GMT+08:00 ??3:30:19, Maxime Ripard
> > > <maxime.ripard@free-electrons.com> ??:
> > > >On Tue, May 23, 2017 at 09:00:59PM +0800, icenowy at aosc.io wrote:
> > > >> ? 2017-05-23 20:53?Maxime Ripard ???
> > > >> > On Mon, May 22, 2017 at 07:55:56PM +0200, Jernej ?krabec wrote:
> > > >> > > Hi,
> > > >> > >
> > > >> > > Dne sobota, 20. maj 2017 ob 03:37:53 CEST je Chen-Yu Tsai
> > > >napisal(a):
> > > >> > > > On Sat, May 20, 2017 at 2:23 AM, Jernej ?krabec
> > > ><jernej.skrabec@siol.net>
> > > >> > > wrote:
> > > >> > > > > Hi,
> > > >> > > > >
> > > >> > > > > Dne petek, 19. maj 2017 ob 20:08:18 CEST je Icenowy Zheng
> > > >napisal(a):
> > > >> > > > >> ? 2017?5?20? GMT+08:00 ??2:03:30, Maxime Ripard
> > > ><maxime.ripard@free-
> > > >> > > > >
> > > >> > > > > electrons.com> ??:
> > > >> > > > >> >On Thu, May 18, 2017 at 12:43:50AM +0800, Icenowy Zheng
> > > >wrote:
> > > >> > > > >> >> Allwinner H3 features a TV encoder similar to the one in
> > > >earlier
> > > >> > > > >> >
> > > >> > > > >> >SoCs,
> > > >> > > > >> >
> > > >> > > > >> >> but with some different points about clocks:
> > > >> > > > >> >> - It has a mod clock and a bus clock.
> > > >> > > > >> >> - The mod clock must be at a fixed rate to generate
> > > >signal.
> > > >> > > > >> >
> > > >> > > > >> >Why?
> > > >> > > > >>
> > > >> > > > >> It's experiment result by Jernej.
> > > >> > > > >>
> > > >> > > > >> The clock rates in BSP kernel is also specially designed
> > > >> > > > >> (PLL_DE at 432MHz) in order to be able to feed the TVE.
> > > >> > > > >
> > > >> > > > > My experiments and search through BSP code showed that TVE
> > > >seems to have
> > > >> > > > > additional fixed predivider 8. So if you want to generate 27
> > > >MHz clock,
> > > >> > > > > unit has to be feed with 216 MHz.
> > > >> > > > >
> > > >> > > > > TVE has only one PLL source PLL_DE. And since 216 MHz is a
> > > >bit low for
> > > >> > > > > DE2,
> > > >> > > > > BSP defaults to 432 MHz for PLL_DE and use divider 2 to
> > > >generate 216 MHz.
> > > >> > > > > This clock is then divided by 8 internaly to get final 27
> > > >MHz.
> > > >> > > > >
> > > >> > > > > Please note that I don't have any hard evidence to support
> > > >that, only
> > > >> > > > > experimental data. However, only that explanation make sense
> > > >to me.
> > > >> > > > >
> > > >> > > > > BTW, BSP H3/H5 TV driver supports only PAL and NTSC which
> > > >both use 27 MHz
> > > >> > > > > base clock. Further experiments are needed to check if there
> > > >is any
> > > >> > > > > possibility to have other resolutions by manipulating clocks
> > > >and give
> > > >> > > > > other proper settings. I plan to do that, but not in very
> > > >near future.
> > > >> > > >
> > > >> > > > You only have composite video output, and those are the only 2
> > > >standard
> > > >> > > > resolutions that make any sense.
> > > >> > >
> > > >> > > Right, other resolutions are for VGA.
> > > >> > >
> > > >> > > Anyway, I did some more digging in A10 and R40 datasheets. I
> > > >think
> > > >> > > that H3 TVE
> > > >> > > unit is something in between. R40 TVE has a setting to select "up
> > > >> > > sample".
> > > >> >
> > > >> > That might be just another translation of oversampling :)
> > > >> >
> > > >> > I didn't know it could be applied to composite signals though, but
> > > >I
> > > >> > guess this is just another analog signal after all.
> > > >> >
> > > >> > > Possible settings are 27 MHz, 54 MHz, 108 MHz and 216 MHz. BSP
> > > >> > > driver on R40
> > > >> > > has this setting enabled only for PAL and NTSC and it is always
> > > >216
> > > >> > > MHz. I
> > > >> > > think that H3 may have this hardwired to 216 MHz and this would
> > > >be
> > > >> > > the reason
> > > >> > > why 216 MHz is needed.
> > > >> > >
> > > >> > > Has anyone else any better explanation?
> > > >> >
> > > >> > That's already a pretty good one.
> > > >> >
> > > >> > Either way, wether this is upsampling, oversampling or just a
> > > >> > pre-divider, this can and should be dealt with in the mode_set
> > > >> > callback, and not in the probe.
> > > >>
> > > >> I got a better idea -- let TVE driver have the CLK_TVE as an
> > > >> input and create a subclock output with divider 16, and feed this
> > > >> subclock to TCON lcd-ch1.
> > > >>
> > > >> This is a model of the real hardware -- the clock divider is in
> > > >> TVE, not TCON.
> > > >
> > > >That's definitely not a good representation of the hardware. There's
> > > >one clock, it goes to the TCON, period.
> > > 
> > > No, I still think it goes to the TVE as:
> > > 
> > > 1. it's named TVE in datasheet.
> > 
> > Feel free to come up with a better, more sensible explanation?
> > 
> > > 2. Generating signal with such a low resolution but such
> > > a high dotclock is not a good situation.
> > 
> > What? What do you mean? The pixel clock is 27MHz, I'm pretty sure we
> > agree on that.
> 
> Yes, so I don't think the TCON code should set the clock to any value
> rather than 27MHz.

The pixel clock that matters is what is output on the TV
connector. The intermediate pixel clock don't matter, and in this
case, it seems to be clearly the case that the TCON needs a higher
clock, then let's give it a higher clock.

> So we should create a clock with divider in TVE, and feed it to TCON.

I'm sorry, but this still doesn't make much sense. The clock is
provided by the TCON, and there's a divider in the TVE, it's as simple
as that, unless you can provide some other meaningful explanation.

That the TV encoder feeds a clock that would drive the TCON is already
suspicious, and all the precedent designs indicate that this is not
what is going on. Both the explanation and the design is much simpler
that way.

Sometimes the Occam's razor is a better justification than a label on
a datasheet.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20170603/addc1010/attachment.sig>

[linux-sunxi] Re: [RFC PATCH 07/11] drm: sun4i: add support for the TV encoder in H3 SoC

[icenowy at aosc.io]

? 2017-05-24 15:30?Maxime Ripard ???
> On Tue, May 23, 2017 at 09:00:59PM +0800, icenowy at aosc.io wrote:
>> ? 2017-05-23 20:53?Maxime Ripard ???
>> > On Mon, May 22, 2017 at 07:55:56PM +0200, Jernej ?krabec wrote:
>> > > Hi,
>> > >
>> > > Dne sobota, 20. maj 2017 ob 03:37:53 CEST je Chen-Yu Tsai napisal(a):
>> > > > On Sat, May 20, 2017 at 2:23 AM, Jernej ?krabec <jernej.skrabec@siol.net>
>> > > wrote:
>> > > > > Hi,
>> > > > >
>> > > > > Dne petek, 19. maj 2017 ob 20:08:18 CEST je Icenowy Zheng napisal(a):
>> > > > >> ? 2017?5?20? GMT+08:00 ??2:03:30, Maxime Ripard <maxime.ripard@free-
>> > > > >
>> > > > > electrons.com> ??:
>> > > > >> >On Thu, May 18, 2017 at 12:43:50AM +0800, Icenowy Zheng wrote:
>> > > > >> >> Allwinner H3 features a TV encoder similar to the one in earlier
>> > > > >> >
>> > > > >> >SoCs,
>> > > > >> >
>> > > > >> >> but with some different points about clocks:
>> > > > >> >> - It has a mod clock and a bus clock.
>> > > > >> >> - The mod clock must be at a fixed rate to generate signal.
>> > > > >> >
>> > > > >> >Why?
>> > > > >>
>> > > > >> It's experiment result by Jernej.
>> > > > >>
>> > > > >> The clock rates in BSP kernel is also specially designed
>> > > > >> (PLL_DE at 432MHz) in order to be able to feed the TVE.
>> > > > >
>> > > > > My experiments and search through BSP code showed that TVE seems to have
>> > > > > additional fixed predivider 8. So if you want to generate 27 MHz clock,
>> > > > > unit has to be feed with 216 MHz.
>> > > > >
>> > > > > TVE has only one PLL source PLL_DE. And since 216 MHz is a bit low for
>> > > > > DE2,
>> > > > > BSP defaults to 432 MHz for PLL_DE and use divider 2 to generate 216 MHz.
>> > > > > This clock is then divided by 8 internaly to get final 27 MHz.
>> > > > >
>> > > > > Please note that I don't have any hard evidence to support that, only
>> > > > > experimental data. However, only that explanation make sense to me.
>> > > > >
>> > > > > BTW, BSP H3/H5 TV driver supports only PAL and NTSC which both use 27 MHz
>> > > > > base clock. Further experiments are needed to check if there is any
>> > > > > possibility to have other resolutions by manipulating clocks and give
>> > > > > other proper settings. I plan to do that, but not in very near future.
>> > > >
>> > > > You only have composite video output, and those are the only 2 standard
>> > > > resolutions that make any sense.
>> > >
>> > > Right, other resolutions are for VGA.
>> > >
>> > > Anyway, I did some more digging in A10 and R40 datasheets. I think
>> > > that H3 TVE
>> > > unit is something in between. R40 TVE has a setting to select "up
>> > > sample".
>> >
>> > That might be just another translation of oversampling :)
>> >
>> > I didn't know it could be applied to composite signals though, but I
>> > guess this is just another analog signal after all.
>> >
>> > > Possible settings are 27 MHz, 54 MHz, 108 MHz and 216 MHz. BSP
>> > > driver on R40
>> > > has this setting enabled only for PAL and NTSC and it is always 216
>> > > MHz. I
>> > > think that H3 may have this hardwired to 216 MHz and this would be
>> > > the reason
>> > > why 216 MHz is needed.
>> > >
>> > > Has anyone else any better explanation?
>> >
>> > That's already a pretty good one.
>> >
>> > Either way, wether this is upsampling, oversampling or just a
>> > pre-divider, this can and should be dealt with in the mode_set
>> > callback, and not in the probe.
>> 
>> I got a better idea -- let TVE driver have the CLK_TVE as an
>> input and create a subclock output with divider 16, and feed this
>> subclock to TCON lcd-ch1.
>> 
>> This is a model of the real hardware -- the clock divider is in
>> TVE, not TCON.
> 
> That's definitely not a good representation of the hardware. There's
> one clock, it goes to the TCON, period.
> 
> However, the TV encoder has a constraint on that clock rate. This can
> be easily implemented using a custom encoder state where you'd set the
> multiplier to set on that clock, and the TCON will use it.

P.S. how to do such a custom state?

Should I do the multiplying in sun4i_tv_mode_to_drm_mode?

> 
> Maxime
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[linux-sunxi] Re: [RFC PATCH 07/11] drm: sun4i: add support for the TV encoder in H3 SoC

[Maxime Ripard]

On Sun, Jun 04, 2017 at 10:29:29PM +0800, icenowy at aosc.io wrote:
> ? 2017-05-24 15:30?Maxime Ripard ???
> > On Tue, May 23, 2017 at 09:00:59PM +0800, icenowy at aosc.io wrote:
> > > ? 2017-05-23 20:53?Maxime Ripard ???
> > > > On Mon, May 22, 2017 at 07:55:56PM +0200, Jernej ?krabec wrote:
> > > > > Hi,
> > > > >
> > > > > Dne sobota, 20. maj 2017 ob 03:37:53 CEST je Chen-Yu Tsai napisal(a):
> > > > > > On Sat, May 20, 2017 at 2:23 AM, Jernej ?krabec <jernej.skrabec@siol.net>
> > > > > wrote:
> > > > > > > Hi,
> > > > > > >
> > > > > > > Dne petek, 19. maj 2017 ob 20:08:18 CEST je Icenowy Zheng napisal(a):
> > > > > > >> ? 2017?5?20? GMT+08:00 ??2:03:30, Maxime Ripard <maxime.ripard@free-
> > > > > > >
> > > > > > > electrons.com> ??:
> > > > > > >> >On Thu, May 18, 2017 at 12:43:50AM +0800, Icenowy Zheng wrote:
> > > > > > >> >> Allwinner H3 features a TV encoder similar to the one in earlier
> > > > > > >> >
> > > > > > >> >SoCs,
> > > > > > >> >
> > > > > > >> >> but with some different points about clocks:
> > > > > > >> >> - It has a mod clock and a bus clock.
> > > > > > >> >> - The mod clock must be at a fixed rate to generate signal.
> > > > > > >> >
> > > > > > >> >Why?
> > > > > > >>
> > > > > > >> It's experiment result by Jernej.
> > > > > > >>
> > > > > > >> The clock rates in BSP kernel is also specially designed
> > > > > > >> (PLL_DE at 432MHz) in order to be able to feed the TVE.
> > > > > > >
> > > > > > > My experiments and search through BSP code showed that TVE seems to have
> > > > > > > additional fixed predivider 8. So if you want to generate 27 MHz clock,
> > > > > > > unit has to be feed with 216 MHz.
> > > > > > >
> > > > > > > TVE has only one PLL source PLL_DE. And since 216 MHz is a bit low for
> > > > > > > DE2,
> > > > > > > BSP defaults to 432 MHz for PLL_DE and use divider 2 to generate 216 MHz.
> > > > > > > This clock is then divided by 8 internaly to get final 27 MHz.
> > > > > > >
> > > > > > > Please note that I don't have any hard evidence to support that, only
> > > > > > > experimental data. However, only that explanation make sense to me.
> > > > > > >
> > > > > > > BTW, BSP H3/H5 TV driver supports only PAL and NTSC which both use 27 MHz
> > > > > > > base clock. Further experiments are needed to check if there is any
> > > > > > > possibility to have other resolutions by manipulating clocks and give
> > > > > > > other proper settings. I plan to do that, but not in very near future.
> > > > > >
> > > > > > You only have composite video output, and those are the only 2 standard
> > > > > > resolutions that make any sense.
> > > > >
> > > > > Right, other resolutions are for VGA.
> > > > >
> > > > > Anyway, I did some more digging in A10 and R40 datasheets. I think
> > > > > that H3 TVE
> > > > > unit is something in between. R40 TVE has a setting to select "up
> > > > > sample".
> > > >
> > > > That might be just another translation of oversampling :)
> > > >
> > > > I didn't know it could be applied to composite signals though, but I
> > > > guess this is just another analog signal after all.
> > > >
> > > > > Possible settings are 27 MHz, 54 MHz, 108 MHz and 216 MHz. BSP
> > > > > driver on R40
> > > > > has this setting enabled only for PAL and NTSC and it is always 216
> > > > > MHz. I
> > > > > think that H3 may have this hardwired to 216 MHz and this would be
> > > > > the reason
> > > > > why 216 MHz is needed.
> > > > >
> > > > > Has anyone else any better explanation?
> > > >
> > > > That's already a pretty good one.
> > > >
> > > > Either way, wether this is upsampling, oversampling or just a
> > > > pre-divider, this can and should be dealt with in the mode_set
> > > > callback, and not in the probe.
> > > 
> > > I got a better idea -- let TVE driver have the CLK_TVE as an
> > > input and create a subclock output with divider 16, and feed this
> > > subclock to TCON lcd-ch1.
> > > 
> > > This is a model of the real hardware -- the clock divider is in
> > > TVE, not TCON.
> > 
> > That's definitely not a good representation of the hardware. There's
> > one clock, it goes to the TCON, period.
> > 
> > However, the TV encoder has a constraint on that clock rate. This can
> > be easily implemented using a custom encoder state where you'd set the
> > multiplier to set on that clock, and the TCON will use it.
> 
> P.S. how to do such a custom state?
> 
> Should I do the multiplying in sun4i_tv_mode_to_drm_mode?

You have an example of that in ebd14afe8fa7, but it basically boils
down to:
  - Create a structure subclassing drm_connector_state
  - Adding your own reset and atomic_duplicate_state callback
  - In the atomic_check, change the state to set the pre-divider, and
    then use it in TCON's mode_set.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20170607/c0bd0814/attachment-0001.sig>

[RFC PATCH 08/11] clk: sunxi-ng: allow CLK_DE to set CLK_PLL_DE for H3

[Icenowy Zheng]

Allwinner H3 features a PLL named CLK_PLL_DE, and a mod clock for the
"Display Engine 2.0" named CLK_DE. As the name indicated, the CLK_PLL_DE
is a PLL for CLK_DE.

Only CLK_DE and CLK_TVE have a parent of CLK_PLL_DE, and CLK_TVE is also
one part of the display clocks.

So allow CLK_DE to set CLK_PLL_DE (add CLK_SET_RATE_PARENT to it).

Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
---
 drivers/clk/sunxi-ng/ccu-sun8i-h3.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/sunxi-ng/ccu-sun8i-h3.c b/drivers/clk/sunxi-ng/ccu-sun8i-h3.c
index 4cbc1b701b7c..6e39ba7cb173 100644
--- a/drivers/clk/sunxi-ng/ccu-sun8i-h3.c
+++ b/drivers/clk/sunxi-ng/ccu-sun8i-h3.c
@@ -439,7 +439,7 @@ static SUNXI_CCU_GATE(dram_ts_clk,	"dram-ts",	"dram",
 
 static const char * const de_parents[] = { "pll-periph0-2x", "pll-de" };
 static SUNXI_CCU_M_WITH_MUX_GATE(de_clk, "de", de_parents,
-				 0x104, 0, 4, 24, 3, BIT(31), 0);
+				 0x104, 0, 4, 24, 3, BIT(31), CLK_SET_RATE_PARENT);
 
 static const char * const tcon_parents[] = { "pll-video" };
 static SUNXI_CCU_M_WITH_MUX_GATE(tcon_clk, "tcon", tcon_parents,
-- 
2.12.2

[RFC PATCH 09/11] clk: sunxi-ng: export CLK_PLL_DE for H3

[Icenowy Zheng]

The CLK_PLL_DE is needed to be referenced in device tree for H3, for
both forcing the parent of PLL_DE.

So export it to the device tree binding header.

Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
---
 drivers/clk/sunxi-ng/ccu-sun8i-h3.h      | 3 +--
 include/dt-bindings/clock/sun8i-h3-ccu.h | 2 ++
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/sunxi-ng/ccu-sun8i-h3.h b/drivers/clk/sunxi-ng/ccu-sun8i-h3.h
index 85973d1e8165..7029091a6c9f 100644
--- a/drivers/clk/sunxi-ng/ccu-sun8i-h3.h
+++ b/drivers/clk/sunxi-ng/ccu-sun8i-h3.h
@@ -33,9 +33,8 @@
 #define CLK_PLL_PERIPH0_2X	10
 #define CLK_PLL_GPU		11
 #define CLK_PLL_PERIPH1		12
-#define CLK_PLL_DE		13
 
-/* The CPUX clock is exported */
+/* The PLL_DE and CPUX clocks is exported */
 
 #define CLK_AXI			15
 #define CLK_AHB1		16
diff --git a/include/dt-bindings/clock/sun8i-h3-ccu.h b/include/dt-bindings/clock/sun8i-h3-ccu.h
index c2afc41d6964..82496a57efd4 100644
--- a/include/dt-bindings/clock/sun8i-h3-ccu.h
+++ b/include/dt-bindings/clock/sun8i-h3-ccu.h
@@ -43,6 +43,8 @@
 #ifndef _DT_BINDINGS_CLK_SUN8I_H3_H_
 #define _DT_BINDINGS_CLK_SUN8I_H3_H_
 
+#define CLK_PLL_DE		13
+
 #define CLK_CPUX		14
 
 #define CLK_BUS_CE		20
-- 
2.12.2

[RFC PATCH 10/11] ARM: sun8i: h3: add display engine pipeline for TVE

[Icenowy Zheng]

As we have already the support for the TV encoder on Allwinner H3, add
the display engine pipeline device tree nodes to its DTSI file.

The H5 pipeline has some differences and will be enabled later.

The currently-unused mixer0 and tcon0 are also needed, for the
completement of the pipeline.

Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
---
 arch/arm/boot/dts/sun8i-h3.dtsi | 189 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 189 insertions(+)

diff --git a/arch/arm/boot/dts/sun8i-h3.dtsi b/arch/arm/boot/dts/sun8i-h3.dtsi
index b36f9f423c39..20172ef92415 100644
--- a/arch/arm/boot/dts/sun8i-h3.dtsi
+++ b/arch/arm/boot/dts/sun8i-h3.dtsi
@@ -41,6 +41,8 @@
  */
 
 #include "sunxi-h3-h5.dtsi"
+#include <dt-bindings/clock/sun8i-de2.h>
+#include <dt-bindings/reset/sun8i-de2.h>
 
 / {
 	cpus {
@@ -72,6 +74,193 @@
 		};
 	};
 
+	de: display-engine {
+		compatible = "allwinner,sun8i-h3-display-engine";
+		allwinner,pipelines = <&mixer0>,
+				      <&mixer1>;
+		status = "disabled";
+	};
+
+	soc {
+		display_clocks: clock at 1000000 {
+			compatible = "allwinner,sun8i-a83t-de2-clk";
+			reg = <0x01000000 0x100000>;
+			clocks = <&ccu CLK_BUS_DE>,
+				 <&ccu CLK_DE>;
+			clock-names = "bus",
+				      "mod";
+			resets = <&ccu RST_BUS_DE>;
+			#clock-cells = <1>;
+			#reset-cells = <1>;
+			assigned-clocks = <&ccu CLK_DE>;
+			assigned-clock-parents = <&ccu CLK_PLL_DE>;
+			assigned-clock-rates = <432000000>;
+		};
+
+		mixer0: mixer at 1100000 {
+			compatible = "allwinner,sun8i-h3-de2-mixer0";
+			reg = <0x01100000 0x100000>;
+			clocks = <&display_clocks CLK_BUS_MIXER0>,
+				 <&display_clocks CLK_MIXER0>;
+			clock-names = "bus",
+				      "mod";
+			resets = <&display_clocks RST_MIXER0>;
+			status = "disabled";
+
+			ports {
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				mixer0_out: port at 1 {
+					#address-cells = <1>;
+					#size-cells = <0>;
+					reg = <1>;
+
+					mixer0_out_tcon0: endpoint at 0 {
+						reg = <0>;
+						remote-endpoint = <&tcon0_in_mixer0>;
+					};
+
+					mixer0_out_tcon1: endpoint at 1 {
+						reg = <1>;
+						remote-endpoint = <&tcon1_in_mixer0>;
+					};
+				};
+			};
+		};
+
+		mixer1: mixer at 1200000 {
+			compatible = "allwinner,sun8i-h3-de2-mixer1";
+			reg = <0x01200000 0x100000>;
+			clocks = <&display_clocks CLK_BUS_MIXER1>,
+				 <&display_clocks CLK_MIXER1>;
+			clock-names = "bus",
+				      "mod";
+			resets = <&display_clocks RST_WB>;
+
+			ports {
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				mixer1_out: port at 1 {
+					#address-cells = <1>;
+					#size-cells = <0>;
+					reg = <1>;
+
+					mixer1_out_tcon1: endpoint at 0 {
+						reg = <0>;
+						remote-endpoint = <&tcon1_in_mixer1>;
+					};
+
+					mixer1_out_tcon0: endpoint at 1 {
+						reg = <1>;
+						remote-endpoint = <&tcon0_in_mixer1>;
+					};
+				};
+			};
+		};
+
+		tcon0: lcd-controller at 1c0c000 {
+			compatible = "allwinner,sun8i-h3-tcon0";
+			reg = <0x01c0c000 0x1000>;
+			interrupts = <GIC_SPI 86 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&ccu CLK_BUS_TCON0>,
+				 <&ccu CLK_TCON0>;
+			clock-names = "ahb",
+				      "tcon-ch1";
+			resets = <&ccu RST_BUS_TCON0>;
+			reset-names = "lcd";
+			status = "disabled";
+
+			ports {
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				tcon0_in: port at 0 {
+					#address-cells = <1>;
+					#size-cells = <0>;
+					reg = <0>;
+
+					tcon0_in_mixer0: endpoint at 0 {
+						reg = <0>;
+						remote-endpoint = <&mixer0_out_tcon0>;
+					};
+
+					tcon0_in_mixer1: endpoint at 1 {
+						reg = <1>;
+						remote-endpoint = <&mixer1_out_tcon0>;
+					};
+				};
+			};
+		};
+
+		tcon1: lcd-controller at 1c0d000 {
+			compatible = "allwinner,sun8i-h3-tcon1";
+			reg = <0x01c0d000 0x1000>;
+			interrupts = <GIC_SPI 87 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&ccu CLK_BUS_TCON1>;
+			clock-names = "ahb";
+			resets = <&ccu RST_BUS_TCON1>;
+			reset-names = "lcd";
+			status = "disabled";
+
+			ports {
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				tcon1_in: port at 0 {
+					#address-cells = <1>;
+					#size-cells = <0>;
+					reg = <0>;
+
+					tcon1_in_mixer1: endpoint at 0 {
+						reg = <0>;
+						remote-endpoint = <&mixer1_out_tcon1>;
+					};
+
+					tcon1_in_mixer0: endpoint at 1 {
+						reg = <1>;
+						remote-endpoint = <&mixer0_out_tcon1>;
+					};
+				};
+
+				tcon1_out: port at 1 {
+					#address-cells = <1>;
+					#size-cells = <0>;
+					reg = <1>;
+
+					tcon1_out_tve0: endpoint at 1 {
+						reg = <1>;
+						remote-endpoint = <&tve0_in_tcon1>;
+					};
+				};
+			};
+		};
+
+		tve0: tv-encoder at 1e00000 {
+			compatible = "allwinner,sun8i-h3-tv-encoder";
+			reg = <0x01e00000 0x1000>;
+			clocks = <&ccu CLK_BUS_TVE>, <&ccu CLK_TVE>;
+			clock-names = "bus", "mod";
+			resets = <&ccu RST_BUS_TVE>;
+			status = "disabled";
+
+			assigned-clocks = <&ccu CLK_TVE>;
+			assigned-clock-parents = <&ccu CLK_PLL_DE>;
+			assigned-clock-rates = <216000000>;
+
+			port {
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				tve0_in_tcon1: endpoint at 0 {
+					reg = <0>;
+					remote-endpoint = <&tcon1_out_tve0>;
+				};
+			};
+		};
+	};
+
 	timer {
 		compatible = "arm,armv7-timer";
 		interrupts = <GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>,
-- 
2.12.2

[linux-sunxi] [RFC PATCH 10/11] ARM: sun8i: h3: add display engine pipeline for TVE

[Jernej Škrabec]

Hi,

Dne sreda, 17. maj 2017 ob 18:43:53 CEST je Icenowy Zheng napisal(a):
> As we have already the support for the TV encoder on Allwinner H3, add
> the display engine pipeline device tree nodes to its DTSI file.
> 
> The H5 pipeline has some differences and will be enabled later.
> 
> The currently-unused mixer0 and tcon0 are also needed, for the
> completement of the pipeline.
> 
> Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
> ---
>  arch/arm/boot/dts/sun8i-h3.dtsi | 189
> ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 189 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/sun8i-h3.dtsi
> b/arch/arm/boot/dts/sun8i-h3.dtsi index b36f9f423c39..20172ef92415 100644
> --- a/arch/arm/boot/dts/sun8i-h3.dtsi
> +++ b/arch/arm/boot/dts/sun8i-h3.dtsi
> @@ -41,6 +41,8 @@
>   */
> 
>  #include "sunxi-h3-h5.dtsi"
> +#include <dt-bindings/clock/sun8i-de2.h>
> +#include <dt-bindings/reset/sun8i-de2.h>
> 
>  / {
>  	cpus {
> @@ -72,6 +74,193 @@
>  		};
>  	};
> 
> +	de: display-engine {
> +		compatible = "allwinner,sun8i-h3-display-engine";
> +		allwinner,pipelines = <&mixer0>,
> +				      <&mixer1>;
> +		status = "disabled";
> +	};
> +
> +	soc {
> +		display_clocks: clock at 1000000 {
> +			compatible = "allwinner,sun8i-a83t-de2-clk";
> +			reg = <0x01000000 0x100000>;
> +			clocks = <&ccu CLK_BUS_DE>,
> +				 <&ccu CLK_DE>;
> +			clock-names = "bus",
> +				      "mod";
> +			resets = <&ccu RST_BUS_DE>;
> +			#clock-cells = <1>;
> +			#reset-cells = <1>;
> +			assigned-clocks = <&ccu CLK_DE>;
> +			assigned-clock-parents = <&ccu CLK_PLL_DE>;
> +			assigned-clock-rates = <432000000>;
> +		};
> +
> +		mixer0: mixer at 1100000 {
> +			compatible = "allwinner,sun8i-h3-de2-mixer0";
> +			reg = <0x01100000 0x100000>;
> +			clocks = <&display_clocks CLK_BUS_MIXER0>,
> +				 <&display_clocks CLK_MIXER0>;
> +			clock-names = "bus",
> +				      "mod";
> +			resets = <&display_clocks RST_MIXER0>;
> +			status = "disabled";
> +
> +			ports {
> +				#address-cells = <1>;
> +				#size-cells = <0>;
> +
> +				mixer0_out: port at 1 {
> +					#address-cells = <1>;
> +					#size-cells = <0>;
> +					reg = <1>;
> +
> +					mixer0_out_tcon0: endpoint at 0 {
> +						reg = <0>;
> +						remote-endpoint = <&tcon0_in_mixer0>;
> +					};
> +
> +					mixer0_out_tcon1: endpoint at 1 {
> +						reg = <1>;
> +						remote-endpoint = <&tcon1_in_mixer0>;
> +					};
> +				};
> +			};
> +		};
> +
> +		mixer1: mixer at 1200000 {
> +			compatible = "allwinner,sun8i-h3-de2-mixer1";
> +			reg = <0x01200000 0x100000>;
> +			clocks = <&display_clocks CLK_BUS_MIXER1>,
> +				 <&display_clocks CLK_MIXER1>;
> +			clock-names = "bus",
> +				      "mod";
> +			resets = <&display_clocks RST_WB>;
> +
> +			ports {
> +				#address-cells = <1>;
> +				#size-cells = <0>;
> +
> +				mixer1_out: port at 1 {
> +					#address-cells = <1>;
> +					#size-cells = <0>;
> +					reg = <1>;
> +
> +					mixer1_out_tcon1: endpoint at 0 {
> +						reg = <0>;
> +						remote-endpoint = <&tcon1_in_mixer1>;
> +					};
> +
> +					mixer1_out_tcon0: endpoint at 1 {
> +						reg = <1>;
> +						remote-endpoint = <&tcon0_in_mixer1>;
> +					};
> +				};
> +			};
> +		};
> +
> +		tcon0: lcd-controller at 1c0c000 {
> +			compatible = "allwinner,sun8i-h3-tcon0";
> +			reg = <0x01c0c000 0x1000>;
> +			interrupts = <GIC_SPI 86 IRQ_TYPE_LEVEL_HIGH>;
> +			clocks = <&ccu CLK_BUS_TCON0>,
> +				 <&ccu CLK_TCON0>;
> +			clock-names = "ahb",
> +				      "tcon-ch1";
> +			resets = <&ccu RST_BUS_TCON0>;
> +			reset-names = "lcd";
> +			status = "disabled";
> +
> +			ports {
> +				#address-cells = <1>;
> +				#size-cells = <0>;
> +
> +				tcon0_in: port at 0 {
> +					#address-cells = <1>;
> +					#size-cells = <0>;
> +					reg = <0>;
> +
> +					tcon0_in_mixer0: endpoint at 0 {
> +						reg = <0>;
> +						remote-endpoint = <&mixer0_out_tcon0>;
> +					};
> +
> +					tcon0_in_mixer1: endpoint at 1 {
> +						reg = <1>;
> +						remote-endpoint = <&mixer1_out_tcon0>;
> +					};
> +				};
> +			};
> +		};
> +
> +		tcon1: lcd-controller at 1c0d000 {
> +			compatible = "allwinner,sun8i-h3-tcon1";
> +			reg = <0x01c0d000 0x1000>;
> +			interrupts = <GIC_SPI 87 IRQ_TYPE_LEVEL_HIGH>;
> +			clocks = <&ccu CLK_BUS_TCON1>;
> +			clock-names = "ahb";
> +			resets = <&ccu RST_BUS_TCON1>;
> +			reset-names = "lcd";
> +			status = "disabled";
> +
> +			ports {
> +				#address-cells = <1>;
> +				#size-cells = <0>;
> +
> +				tcon1_in: port at 0 {
> +					#address-cells = <1>;
> +					#size-cells = <0>;
> +					reg = <0>;
> +
> +					tcon1_in_mixer1: endpoint at 0 {
> +						reg = <0>;
> +						remote-endpoint = <&mixer1_out_tcon1>;
> +					};
> +
> +					tcon1_in_mixer0: endpoint at 1 {
> +						reg = <1>;
> +						remote-endpoint = <&mixer0_out_tcon1>;
> +					};
> +				};
> +
> +				tcon1_out: port at 1 {
> +					#address-cells = <1>;
> +					#size-cells = <0>;
> +					reg = <1>;
> +
> +					tcon1_out_tve0: endpoint at 1 {
> +						reg = <1>;
> +						remote-endpoint = <&tve0_in_tcon1>;
> +					};
> +				};
> +			};
> +		};
> +
> +		tve0: tv-encoder at 1e00000 {
> +			compatible = "allwinner,sun8i-h3-tv-encoder";
> +			reg = <0x01e00000 0x1000>;
> +			clocks = <&ccu CLK_BUS_TVE>, <&ccu CLK_TVE>;
> +			clock-names = "bus", "mod";
> +			resets = <&ccu RST_BUS_TVE>;
> +			status = "disabled";
> +
> +			assigned-clocks = <&ccu CLK_TVE>;
> +			assigned-clock-parents = <&ccu CLK_PLL_DE>;
> +			assigned-clock-rates = <216000000>;
> +
> +			port {
> +				#address-cells = <1>;
> +				#size-cells = <0>;
> +
> +				tve0_in_tcon1: endpoint at 0 {
> +					reg = <0>;
> +					remote-endpoint = <&tcon1_out_tve0>;
> +				};

I think there should be out endpoint to composite connector (compatible: 
"composite-video-connector").

Best regards,
Jernej

> +			};
> +		};
> +	};
> +
>  	timer {
>  		compatible = "arm,armv7-timer";
>  		interrupts = <GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(4) | 
IRQ_TYPE_LEVEL_LOW)>,
> --
> 2.12.2
> 
> --
> You received this message because you are subscribed to the Google Groups
> "linux-sunxi" group. To unsubscribe from this group and stop receiving
> emails from it, send an email to linux-sunxi+unsubscribe at googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.

[RFC PATCH 10/11] ARM: sun8i: h3: add display engine pipeline for TVE

[Maxime Ripard]

On Thu, May 18, 2017 at 12:43:53AM +0800, Icenowy Zheng wrote:
> As we have already the support for the TV encoder on Allwinner H3, add
> the display engine pipeline device tree nodes to its DTSI file.
> 
> The H5 pipeline has some differences and will be enabled later.
> 
> The currently-unused mixer0 and tcon0 are also needed, for the
> completement of the pipeline.
> 
> Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
> ---
>  arch/arm/boot/dts/sun8i-h3.dtsi | 189 ++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 189 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/sun8i-h3.dtsi b/arch/arm/boot/dts/sun8i-h3.dtsi
> index b36f9f423c39..20172ef92415 100644
> --- a/arch/arm/boot/dts/sun8i-h3.dtsi
> +++ b/arch/arm/boot/dts/sun8i-h3.dtsi
> @@ -41,6 +41,8 @@
>   */
>  
>  #include "sunxi-h3-h5.dtsi"
> +#include <dt-bindings/clock/sun8i-de2.h>
> +#include <dt-bindings/reset/sun8i-de2.h>
>  
>  / {
>  	cpus {
> @@ -72,6 +74,193 @@
>  		};
>  	};
>  
> +	de: display-engine {
> +		compatible = "allwinner,sun8i-h3-display-engine";
> +		allwinner,pipelines = <&mixer0>,
> +				      <&mixer1>;
> +		status = "disabled";
> +	};
> +
> +	soc {
> +		display_clocks: clock at 1000000 {
> +			compatible = "allwinner,sun8i-a83t-de2-clk";
> +			reg = <0x01000000 0x100000>;
> +			clocks = <&ccu CLK_BUS_DE>,
> +				 <&ccu CLK_DE>;
> +			clock-names = "bus",
> +				      "mod";
> +			resets = <&ccu RST_BUS_DE>;
> +			#clock-cells = <1>;
> +			#reset-cells = <1>;
> +			assigned-clocks = <&ccu CLK_DE>;
> +			assigned-clock-parents = <&ccu CLK_PLL_DE>;
> +			assigned-clock-rates = <432000000>;

This shouldn't be set in the DT, but evaluated at runtime when calling
clk_set_rate.

> +		tve0: tv-encoder at 1e00000 {
> +			compatible = "allwinner,sun8i-h3-tv-encoder";
> +			reg = <0x01e00000 0x1000>;
> +			clocks = <&ccu CLK_BUS_TVE>, <&ccu CLK_TVE>;
> +			clock-names = "bus", "mod";
> +			resets = <&ccu RST_BUS_TVE>;
> +			status = "disabled";
> +
> +			assigned-clocks = <&ccu CLK_TVE>;
> +			assigned-clock-parents = <&ccu CLK_PLL_DE>;

Same thing here. clk_set_rate should just do the right thing.

> +			assigned-clock-rates = <216000000>;

And why are you setting it in the driver and in the DT?

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20170519/c57dec29/attachment.sig>

[linux-sunxi] Re: [RFC PATCH 10/11] ARM: sun8i: h3: add display engine pipeline for TVE

[Icenowy Zheng]

? 2017?5?20? GMT+08:00 ??2:06:16, Maxime Ripard <maxime.ripard@free-electrons.com> ??:
>On Thu, May 18, 2017 at 12:43:53AM +0800, Icenowy Zheng wrote:
>> As we have already the support for the TV encoder on Allwinner H3,
>add
>> the display engine pipeline device tree nodes to its DTSI file.
>> 
>> The H5 pipeline has some differences and will be enabled later.
>> 
>> The currently-unused mixer0 and tcon0 are also needed, for the
>> completement of the pipeline.
>> 
>> Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
>> ---
>>  arch/arm/boot/dts/sun8i-h3.dtsi | 189
>++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 189 insertions(+)
>> 
>> diff --git a/arch/arm/boot/dts/sun8i-h3.dtsi
>b/arch/arm/boot/dts/sun8i-h3.dtsi
>> index b36f9f423c39..20172ef92415 100644
>> --- a/arch/arm/boot/dts/sun8i-h3.dtsi
>> +++ b/arch/arm/boot/dts/sun8i-h3.dtsi
>> @@ -41,6 +41,8 @@
>>   */
>>  
>>  #include "sunxi-h3-h5.dtsi"
>> +#include <dt-bindings/clock/sun8i-de2.h>
>> +#include <dt-bindings/reset/sun8i-de2.h>
>>  
>>  / {
>>  	cpus {
>> @@ -72,6 +74,193 @@
>>  		};
>>  	};
>>  
>> +	de: display-engine {
>> +		compatible = "allwinner,sun8i-h3-display-engine";
>> +		allwinner,pipelines = <&mixer0>,
>> +				      <&mixer1>;
>> +		status = "disabled";
>> +	};
>> +
>> +	soc {
>> +		display_clocks: clock at 1000000 {
>> +			compatible = "allwinner,sun8i-a83t-de2-clk";
>> +			reg = <0x01000000 0x100000>;
>> +			clocks = <&ccu CLK_BUS_DE>,
>> +				 <&ccu CLK_DE>;
>> +			clock-names = "bus",
>> +				      "mod";
>> +			resets = <&ccu RST_BUS_DE>;
>> +			#clock-cells = <1>;
>> +			#reset-cells = <1>;
>> +			assigned-clocks = <&ccu CLK_DE>;
>> +			assigned-clock-parents = <&ccu CLK_PLL_DE>;
>> +			assigned-clock-rates = <432000000>;
>
>This shouldn't be set in the DT, but evaluated at runtime when calling
>clk_set_rate.

Nope, DE2 clock doesn't need evalution, as the clock is decoupled with
DE2 mixers' output signal. (Although it seems that SoCs with larger
plane size will use higher clock.)

And setting it to 432MHz is also needed for properly 216MHz clock to TVE.

>
>> +		tve0: tv-encoder at 1e00000 {
>> +			compatible = "allwinner,sun8i-h3-tv-encoder";
>> +			reg = <0x01e00000 0x1000>;
>> +			clocks = <&ccu CLK_BUS_TVE>, <&ccu CLK_TVE>;
>> +			clock-names = "bus", "mod";
>> +			resets = <&ccu RST_BUS_TVE>;
>> +			status = "disabled";
>> +
>> +			assigned-clocks = <&ccu CLK_TVE>;
>> +			assigned-clock-parents = <&ccu CLK_PLL_DE>;
>
>Same thing here. clk_set_rate should just do the right thing.
>
>> +			assigned-clock-rates = <216000000>;
>
>And why are you setting it in the driver and in the DT?
>
>Maxime

[linux-sunxi] Re: [RFC PATCH 10/11] ARM: sun8i: h3: add display engine pipeline for TVE

[Maxime Ripard]

On Sat, May 20, 2017 at 02:10:35AM +0800, Icenowy Zheng wrote:
> 
> 
> ? 2017?5?20? GMT+08:00 ??2:06:16, Maxime Ripard <maxime.ripard@free-electrons.com> ??:
> >On Thu, May 18, 2017 at 12:43:53AM +0800, Icenowy Zheng wrote:
> >> As we have already the support for the TV encoder on Allwinner H3,
> >add
> >> the display engine pipeline device tree nodes to its DTSI file.
> >> 
> >> The H5 pipeline has some differences and will be enabled later.
> >> 
> >> The currently-unused mixer0 and tcon0 are also needed, for the
> >> completement of the pipeline.
> >> 
> >> Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
> >> ---
> >>  arch/arm/boot/dts/sun8i-h3.dtsi | 189
> >++++++++++++++++++++++++++++++++++++++++
> >>  1 file changed, 189 insertions(+)
> >> 
> >> diff --git a/arch/arm/boot/dts/sun8i-h3.dtsi
> >b/arch/arm/boot/dts/sun8i-h3.dtsi
> >> index b36f9f423c39..20172ef92415 100644
> >> --- a/arch/arm/boot/dts/sun8i-h3.dtsi
> >> +++ b/arch/arm/boot/dts/sun8i-h3.dtsi
> >> @@ -41,6 +41,8 @@
> >>   */
> >>  
> >>  #include "sunxi-h3-h5.dtsi"
> >> +#include <dt-bindings/clock/sun8i-de2.h>
> >> +#include <dt-bindings/reset/sun8i-de2.h>
> >>  
> >>  / {
> >>  	cpus {
> >> @@ -72,6 +74,193 @@
> >>  		};
> >>  	};
> >>  
> >> +	de: display-engine {
> >> +		compatible = "allwinner,sun8i-h3-display-engine";
> >> +		allwinner,pipelines = <&mixer0>,
> >> +				      <&mixer1>;
> >> +		status = "disabled";
> >> +	};
> >> +
> >> +	soc {
> >> +		display_clocks: clock at 1000000 {
> >> +			compatible = "allwinner,sun8i-a83t-de2-clk";
> >> +			reg = <0x01000000 0x100000>;
> >> +			clocks = <&ccu CLK_BUS_DE>,
> >> +				 <&ccu CLK_DE>;
> >> +			clock-names = "bus",
> >> +				      "mod";
> >> +			resets = <&ccu RST_BUS_DE>;
> >> +			#clock-cells = <1>;
> >> +			#reset-cells = <1>;
> >> +			assigned-clocks = <&ccu CLK_DE>;
> >> +			assigned-clock-parents = <&ccu CLK_PLL_DE>;
> >> +			assigned-clock-rates = <432000000>;
> >
> >This shouldn't be set in the DT, but evaluated at runtime when calling
> >clk_set_rate.
> 
> Nope, DE2 clock doesn't need evalution, as the clock is decoupled with
> DE2 mixers' output signal. (Although it seems that SoCs with larger
> plane size will use higher clock.)

So it's the display engine that needs that clock to operate properly?
This is the wrong DT node to set that value then. It should be in the
mixer node, or even better in the mixers' driver.

> And setting it to 432MHz is also needed for properly 216MHz clock to
> TVE.

Just like the parenthood, this can and should be evaluated at runtime.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20170524/68b8e86b/attachment.sig>

[linux-sunxi] [RFC PATCH 10/11] ARM: sun8i: h3: add display engine pipeline for TVE

[Chen-Yu Tsai]

On Thu, May 18, 2017 at 12:43 AM, Icenowy Zheng <icenowy@aosc.io> wrote:
> As we have already the support for the TV encoder on Allwinner H3, add
> the display engine pipeline device tree nodes to its DTSI file.
>
> The H5 pipeline has some differences and will be enabled later.
>
> The currently-unused mixer0 and tcon0 are also needed, for the
> completement of the pipeline.
>
> Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
> ---
>  arch/arm/boot/dts/sun8i-h3.dtsi | 189 ++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 189 insertions(+)
>
> diff --git a/arch/arm/boot/dts/sun8i-h3.dtsi b/arch/arm/boot/dts/sun8i-h3.dtsi
> index b36f9f423c39..20172ef92415 100644
> --- a/arch/arm/boot/dts/sun8i-h3.dtsi
> +++ b/arch/arm/boot/dts/sun8i-h3.dtsi
> @@ -41,6 +41,8 @@
>   */
>
>  #include "sunxi-h3-h5.dtsi"
> +#include <dt-bindings/clock/sun8i-de2.h>
> +#include <dt-bindings/reset/sun8i-de2.h>
>
>  / {
>         cpus {
> @@ -72,6 +74,193 @@
>                 };
>         };
>
> +       de: display-engine {
> +               compatible = "allwinner,sun8i-h3-display-engine";
> +               allwinner,pipelines = <&mixer0>,
> +                                     <&mixer1>;
> +               status = "disabled";
> +       };
> +
> +       soc {
> +               display_clocks: clock at 1000000 {
> +                       compatible = "allwinner,sun8i-a83t-de2-clk";
> +                       reg = <0x01000000 0x100000>;
> +                       clocks = <&ccu CLK_BUS_DE>,
> +                                <&ccu CLK_DE>;
> +                       clock-names = "bus",
> +                                     "mod";
> +                       resets = <&ccu RST_BUS_DE>;
> +                       #clock-cells = <1>;
> +                       #reset-cells = <1>;
> +                       assigned-clocks = <&ccu CLK_DE>;
> +                       assigned-clock-parents = <&ccu CLK_PLL_DE>;
> +                       assigned-clock-rates = <432000000>;
> +               };
> +
> +               mixer0: mixer at 1100000 {
> +                       compatible = "allwinner,sun8i-h3-de2-mixer0";
> +                       reg = <0x01100000 0x100000>;
> +                       clocks = <&display_clocks CLK_BUS_MIXER0>,
> +                                <&display_clocks CLK_MIXER0>;
> +                       clock-names = "bus",
> +                                     "mod";
> +                       resets = <&display_clocks RST_MIXER0>;
> +                       status = "disabled";
> +
> +                       ports {
> +                               #address-cells = <1>;
> +                               #size-cells = <0>;
> +
> +                               mixer0_out: port at 1 {
> +                                       #address-cells = <1>;
> +                                       #size-cells = <0>;
> +                                       reg = <1>;
> +
> +                                       mixer0_out_tcon0: endpoint at 0 {
> +                                               reg = <0>;
> +                                               remote-endpoint = <&tcon0_in_mixer0>;
> +                                       };
> +
> +                                       mixer0_out_tcon1: endpoint at 1 {
> +                                               reg = <1>;
> +                                               remote-endpoint = <&tcon1_in_mixer0>;
> +                                       };
> +                               };
> +                       };
> +               };
> +
> +               mixer1: mixer at 1200000 {
> +                       compatible = "allwinner,sun8i-h3-de2-mixer1";
> +                       reg = <0x01200000 0x100000>;
> +                       clocks = <&display_clocks CLK_BUS_MIXER1>,
> +                                <&display_clocks CLK_MIXER1>;
> +                       clock-names = "bus",
> +                                     "mod";
> +                       resets = <&display_clocks RST_WB>;
> +
> +                       ports {
> +                               #address-cells = <1>;
> +                               #size-cells = <0>;
> +
> +                               mixer1_out: port at 1 {
> +                                       #address-cells = <1>;
> +                                       #size-cells = <0>;
> +                                       reg = <1>;
> +
> +                                       mixer1_out_tcon1: endpoint at 0 {
> +                                               reg = <0>;

I would prefer if you could stick to the numbering scheme we're using for
Display Engine 1.0, as in endpoint 0 links to component 0 of whatever type.

We're probably going to stick to that for the R40's incredibly complicated
pipeline. I don't want to have any outliers unless absolutely necessary.

ChenYu

> +                                               remote-endpoint = <&tcon1_in_mixer1>;
> +                                       };
> +
> +                                       mixer1_out_tcon0: endpoint at 1 {
> +                                               reg = <1>;
> +                                               remote-endpoint = <&tcon0_in_mixer1>;
> +                                       };
> +                               };
> +                       };
> +               };
> +
> +               tcon0: lcd-controller at 1c0c000 {
> +                       compatible = "allwinner,sun8i-h3-tcon0";
> +                       reg = <0x01c0c000 0x1000>;
> +                       interrupts = <GIC_SPI 86 IRQ_TYPE_LEVEL_HIGH>;
> +                       clocks = <&ccu CLK_BUS_TCON0>,
> +                                <&ccu CLK_TCON0>;
> +                       clock-names = "ahb",
> +                                     "tcon-ch1";
> +                       resets = <&ccu RST_BUS_TCON0>;
> +                       reset-names = "lcd";
> +                       status = "disabled";
> +
> +                       ports {
> +                               #address-cells = <1>;
> +                               #size-cells = <0>;
> +
> +                               tcon0_in: port at 0 {
> +                                       #address-cells = <1>;
> +                                       #size-cells = <0>;
> +                                       reg = <0>;
> +
> +                                       tcon0_in_mixer0: endpoint at 0 {
> +                                               reg = <0>;
> +                                               remote-endpoint = <&mixer0_out_tcon0>;
> +                                       };
> +
> +                                       tcon0_in_mixer1: endpoint at 1 {
> +                                               reg = <1>;
> +                                               remote-endpoint = <&mixer1_out_tcon0>;
> +                                       };
> +                               };
> +                       };
> +               };
> +
> +               tcon1: lcd-controller at 1c0d000 {
> +                       compatible = "allwinner,sun8i-h3-tcon1";
> +                       reg = <0x01c0d000 0x1000>;
> +                       interrupts = <GIC_SPI 87 IRQ_TYPE_LEVEL_HIGH>;
> +                       clocks = <&ccu CLK_BUS_TCON1>;
> +                       clock-names = "ahb";
> +                       resets = <&ccu RST_BUS_TCON1>;
> +                       reset-names = "lcd";
> +                       status = "disabled";
> +
> +                       ports {
> +                               #address-cells = <1>;
> +                               #size-cells = <0>;
> +
> +                               tcon1_in: port at 0 {
> +                                       #address-cells = <1>;
> +                                       #size-cells = <0>;
> +                                       reg = <0>;
> +
> +                                       tcon1_in_mixer1: endpoint at 0 {
> +                                               reg = <0>;
> +                                               remote-endpoint = <&mixer1_out_tcon1>;
> +                                       };
> +
> +                                       tcon1_in_mixer0: endpoint at 1 {
> +                                               reg = <1>;
> +                                               remote-endpoint = <&mixer0_out_tcon1>;
> +                                       };
> +                               };
> +
> +                               tcon1_out: port at 1 {
> +                                       #address-cells = <1>;
> +                                       #size-cells = <0>;
> +                                       reg = <1>;
> +
> +                                       tcon1_out_tve0: endpoint at 1 {
> +                                               reg = <1>;
> +                                               remote-endpoint = <&tve0_in_tcon1>;
> +                                       };
> +                               };
> +                       };
> +               };
> +
> +               tve0: tv-encoder at 1e00000 {
> +                       compatible = "allwinner,sun8i-h3-tv-encoder";
> +                       reg = <0x01e00000 0x1000>;
> +                       clocks = <&ccu CLK_BUS_TVE>, <&ccu CLK_TVE>;
> +                       clock-names = "bus", "mod";
> +                       resets = <&ccu RST_BUS_TVE>;
> +                       status = "disabled";
> +
> +                       assigned-clocks = <&ccu CLK_TVE>;
> +                       assigned-clock-parents = <&ccu CLK_PLL_DE>;
> +                       assigned-clock-rates = <216000000>;
> +
> +                       port {
> +                               #address-cells = <1>;
> +                               #size-cells = <0>;
> +
> +                               tve0_in_tcon1: endpoint at 0 {
> +                                       reg = <0>;
> +                                       remote-endpoint = <&tcon1_out_tve0>;
> +                               };
> +                       };
> +               };
> +       };
> +
>         timer {
>                 compatible = "arm,armv7-timer";
>                 interrupts = <GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>,
> --
> 2.12.2
>
> --
> You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe at googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.

[linux-sunxi] [RFC PATCH 10/11] ARM: sun8i: h3: add display engine pipeline for TVE

[Icenowy Zheng]

? 2017?5?24? GMT+08:00 ??1:24:29, Chen-Yu Tsai <wens@csie.org> ??:
>On Thu, May 18, 2017 at 12:43 AM, Icenowy Zheng <icenowy@aosc.io>
>wrote:
>> As we have already the support for the TV encoder on Allwinner H3,
>add
>> the display engine pipeline device tree nodes to its DTSI file.
>>
>> The H5 pipeline has some differences and will be enabled later.
>>
>> The currently-unused mixer0 and tcon0 are also needed, for the
>> completement of the pipeline.
>>
>> Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
>> ---
>>  arch/arm/boot/dts/sun8i-h3.dtsi | 189
>++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 189 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/sun8i-h3.dtsi
>b/arch/arm/boot/dts/sun8i-h3.dtsi
>> index b36f9f423c39..20172ef92415 100644
>> --- a/arch/arm/boot/dts/sun8i-h3.dtsi
>> +++ b/arch/arm/boot/dts/sun8i-h3.dtsi
>> @@ -41,6 +41,8 @@
>>   */
>>
>>  #include "sunxi-h3-h5.dtsi"
>> +#include <dt-bindings/clock/sun8i-de2.h>
>> +#include <dt-bindings/reset/sun8i-de2.h>
>>
>>  / {
>>         cpus {
>> @@ -72,6 +74,193 @@
>>                 };
>>         };
>>
>> +       de: display-engine {
>> +               compatible = "allwinner,sun8i-h3-display-engine";
>> +               allwinner,pipelines = <&mixer0>,
>> +                                     <&mixer1>;
>> +               status = "disabled";
>> +       };
>> +
>> +       soc {
>> +               display_clocks: clock at 1000000 {
>> +                       compatible = "allwinner,sun8i-a83t-de2-clk";
>> +                       reg = <0x01000000 0x100000>;
>> +                       clocks = <&ccu CLK_BUS_DE>,
>> +                                <&ccu CLK_DE>;
>> +                       clock-names = "bus",
>> +                                     "mod";
>> +                       resets = <&ccu RST_BUS_DE>;
>> +                       #clock-cells = <1>;
>> +                       #reset-cells = <1>;
>> +                       assigned-clocks = <&ccu CLK_DE>;
>> +                       assigned-clock-parents = <&ccu CLK_PLL_DE>;
>> +                       assigned-clock-rates = <432000000>;
>> +               };
>> +
>> +               mixer0: mixer at 1100000 {
>> +                       compatible = "allwinner,sun8i-h3-de2-mixer0";
>> +                       reg = <0x01100000 0x100000>;
>> +                       clocks = <&display_clocks CLK_BUS_MIXER0>,
>> +                                <&display_clocks CLK_MIXER0>;
>> +                       clock-names = "bus",
>> +                                     "mod";
>> +                       resets = <&display_clocks RST_MIXER0>;
>> +                       status = "disabled";
>> +
>> +                       ports {
>> +                               #address-cells = <1>;
>> +                               #size-cells = <0>;
>> +
>> +                               mixer0_out: port at 1 {
>> +                                       #address-cells = <1>;
>> +                                       #size-cells = <0>;
>> +                                       reg = <1>;
>> +
>> +                                       mixer0_out_tcon0: endpoint at 0
>{
>> +                                               reg = <0>;
>> +                                               remote-endpoint =
><&tcon0_in_mixer0>;
>> +                                       };
>> +
>> +                                       mixer0_out_tcon1: endpoint at 1
>{
>> +                                               reg = <1>;
>> +                                               remote-endpoint =
><&tcon1_in_mixer0>;
>> +                                       };
>> +                               };
>> +                       };
>> +               };
>> +
>> +               mixer1: mixer at 1200000 {
>> +                       compatible = "allwinner,sun8i-h3-de2-mixer1";
>> +                       reg = <0x01200000 0x100000>;
>> +                       clocks = <&display_clocks CLK_BUS_MIXER1>,
>> +                                <&display_clocks CLK_MIXER1>;
>> +                       clock-names = "bus",
>> +                                     "mod";
>> +                       resets = <&display_clocks RST_WB>;
>> +
>> +                       ports {
>> +                               #address-cells = <1>;
>> +                               #size-cells = <0>;
>> +
>> +                               mixer1_out: port at 1 {
>> +                                       #address-cells = <1>;
>> +                                       #size-cells = <0>;
>> +                                       reg = <1>;
>> +
>> +                                       mixer1_out_tcon1: endpoint at 0
>{
>> +                                               reg = <0>;
>
>I would prefer if you could stick to the numbering scheme we're using
>for
>Display Engine 1.0, as in endpoint 0 links to component 0 of whatever
>type.

If we keep this we will need a ugly id property in mixer node,
otherwise we cannot know which TCON to be bind.

>
>We're probably going to stick to that for the R40's incredibly
>complicated
>pipeline. I don't want to have any outliers unless absolutely
>necessary.
>
>ChenYu
>
>> +                                               remote-endpoint =
><&tcon1_in_mixer1>;
>> +                                       };
>> +
>> +                                       mixer1_out_tcon0: endpoint at 1
>{
>> +                                               reg = <1>;
>> +                                               remote-endpoint =
><&tcon0_in_mixer1>;
>> +                                       };
>> +                               };
>> +                       };
>> +               };
>> +
>> +               tcon0: lcd-controller at 1c0c000 {
>> +                       compatible = "allwinner,sun8i-h3-tcon0";
>> +                       reg = <0x01c0c000 0x1000>;
>> +                       interrupts = <GIC_SPI 86
>IRQ_TYPE_LEVEL_HIGH>;
>> +                       clocks = <&ccu CLK_BUS_TCON0>,
>> +                                <&ccu CLK_TCON0>;
>> +                       clock-names = "ahb",
>> +                                     "tcon-ch1";
>> +                       resets = <&ccu RST_BUS_TCON0>;
>> +                       reset-names = "lcd";
>> +                       status = "disabled";
>> +
>> +                       ports {
>> +                               #address-cells = <1>;
>> +                               #size-cells = <0>;
>> +
>> +                               tcon0_in: port at 0 {
>> +                                       #address-cells = <1>;
>> +                                       #size-cells = <0>;
>> +                                       reg = <0>;
>> +
>> +                                       tcon0_in_mixer0: endpoint at 0 {
>> +                                               reg = <0>;
>> +                                               remote-endpoint =
><&mixer0_out_tcon0>;
>> +                                       };
>> +
>> +                                       tcon0_in_mixer1: endpoint at 1 {
>> +                                               reg = <1>;
>> +                                               remote-endpoint =
><&mixer1_out_tcon0>;
>> +                                       };
>> +                               };
>> +                       };
>> +               };
>> +
>> +               tcon1: lcd-controller at 1c0d000 {
>> +                       compatible = "allwinner,sun8i-h3-tcon1";
>> +                       reg = <0x01c0d000 0x1000>;
>> +                       interrupts = <GIC_SPI 87
>IRQ_TYPE_LEVEL_HIGH>;
>> +                       clocks = <&ccu CLK_BUS_TCON1>;
>> +                       clock-names = "ahb";
>> +                       resets = <&ccu RST_BUS_TCON1>;
>> +                       reset-names = "lcd";
>> +                       status = "disabled";
>> +
>> +                       ports {
>> +                               #address-cells = <1>;
>> +                               #size-cells = <0>;
>> +
>> +                               tcon1_in: port at 0 {
>> +                                       #address-cells = <1>;
>> +                                       #size-cells = <0>;
>> +                                       reg = <0>;
>> +
>> +                                       tcon1_in_mixer1: endpoint at 0 {
>> +                                               reg = <0>;
>> +                                               remote-endpoint =
><&mixer1_out_tcon1>;
>> +                                       };
>> +
>> +                                       tcon1_in_mixer0: endpoint at 1 {
>> +                                               reg = <1>;
>> +                                               remote-endpoint =
><&mixer0_out_tcon1>;
>> +                                       };
>> +                               };
>> +
>> +                               tcon1_out: port at 1 {
>> +                                       #address-cells = <1>;
>> +                                       #size-cells = <0>;
>> +                                       reg = <1>;
>> +
>> +                                       tcon1_out_tve0: endpoint at 1 {
>> +                                               reg = <1>;
>> +                                               remote-endpoint =
><&tve0_in_tcon1>;
>> +                                       };
>> +                               };
>> +                       };
>> +               };
>> +
>> +               tve0: tv-encoder at 1e00000 {
>> +                       compatible = "allwinner,sun8i-h3-tv-encoder";
>> +                       reg = <0x01e00000 0x1000>;
>> +                       clocks = <&ccu CLK_BUS_TVE>, <&ccu CLK_TVE>;
>> +                       clock-names = "bus", "mod";
>> +                       resets = <&ccu RST_BUS_TVE>;
>> +                       status = "disabled";
>> +
>> +                       assigned-clocks = <&ccu CLK_TVE>;
>> +                       assigned-clock-parents = <&ccu CLK_PLL_DE>;
>> +                       assigned-clock-rates = <216000000>;
>> +
>> +                       port {
>> +                               #address-cells = <1>;
>> +                               #size-cells = <0>;
>> +
>> +                               tve0_in_tcon1: endpoint at 0 {
>> +                                       reg = <0>;
>> +                                       remote-endpoint =
><&tcon1_out_tve0>;
>> +                               };
>> +                       };
>> +               };
>> +       };
>> +
>>         timer {
>>                 compatible = "arm,armv7-timer";
>>                 interrupts = <GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(4) |
>IRQ_TYPE_LEVEL_LOW)>,
>> --
>> 2.12.2
>>
>> --
>> You received this message because you are subscribed to the Google
>Groups "linux-sunxi" group.
>> To unsubscribe from this group and stop receiving emails from it,
>send an email to linux-sunxi+unsubscribe at googlegroups.com.
>> For more options, visit https://groups.google.com/d/optout.

[linux-sunxi] [RFC PATCH 10/11] ARM: sun8i: h3: add display engine pipeline for TVE

[Chen-Yu Tsai]

On Wed, May 24, 2017 at 1:28 PM, Icenowy Zheng <icenowy@aosc.io> wrote:
>
>
> ? 2017?5?24? GMT+08:00 ??1:24:29, Chen-Yu Tsai <wens@csie.org> ??:
>>On Thu, May 18, 2017 at 12:43 AM, Icenowy Zheng <icenowy@aosc.io>
>>wrote:
>>> As we have already the support for the TV encoder on Allwinner H3,
>>add
>>> the display engine pipeline device tree nodes to its DTSI file.
>>>
>>> The H5 pipeline has some differences and will be enabled later.
>>>
>>> The currently-unused mixer0 and tcon0 are also needed, for the
>>> completement of the pipeline.
>>>
>>> Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
>>> ---
>>>  arch/arm/boot/dts/sun8i-h3.dtsi | 189
>>++++++++++++++++++++++++++++++++++++++++
>>>  1 file changed, 189 insertions(+)
>>>
>>> diff --git a/arch/arm/boot/dts/sun8i-h3.dtsi
>>b/arch/arm/boot/dts/sun8i-h3.dtsi
>>> index b36f9f423c39..20172ef92415 100644
>>> --- a/arch/arm/boot/dts/sun8i-h3.dtsi
>>> +++ b/arch/arm/boot/dts/sun8i-h3.dtsi
>>> @@ -41,6 +41,8 @@
>>>   */
>>>
>>>  #include "sunxi-h3-h5.dtsi"
>>> +#include <dt-bindings/clock/sun8i-de2.h>
>>> +#include <dt-bindings/reset/sun8i-de2.h>
>>>
>>>  / {
>>>         cpus {
>>> @@ -72,6 +74,193 @@
>>>                 };
>>>         };
>>>
>>> +       de: display-engine {
>>> +               compatible = "allwinner,sun8i-h3-display-engine";
>>> +               allwinner,pipelines = <&mixer0>,
>>> +                                     <&mixer1>;
>>> +               status = "disabled";
>>> +       };
>>> +
>>> +       soc {
>>> +               display_clocks: clock at 1000000 {
>>> +                       compatible = "allwinner,sun8i-a83t-de2-clk";
>>> +                       reg = <0x01000000 0x100000>;
>>> +                       clocks = <&ccu CLK_BUS_DE>,
>>> +                                <&ccu CLK_DE>;
>>> +                       clock-names = "bus",
>>> +                                     "mod";
>>> +                       resets = <&ccu RST_BUS_DE>;
>>> +                       #clock-cells = <1>;
>>> +                       #reset-cells = <1>;
>>> +                       assigned-clocks = <&ccu CLK_DE>;
>>> +                       assigned-clock-parents = <&ccu CLK_PLL_DE>;
>>> +                       assigned-clock-rates = <432000000>;
>>> +               };
>>> +
>>> +               mixer0: mixer at 1100000 {
>>> +                       compatible = "allwinner,sun8i-h3-de2-mixer0";
>>> +                       reg = <0x01100000 0x100000>;
>>> +                       clocks = <&display_clocks CLK_BUS_MIXER0>,
>>> +                                <&display_clocks CLK_MIXER0>;
>>> +                       clock-names = "bus",
>>> +                                     "mod";
>>> +                       resets = <&display_clocks RST_MIXER0>;
>>> +                       status = "disabled";
>>> +
>>> +                       ports {
>>> +                               #address-cells = <1>;
>>> +                               #size-cells = <0>;
>>> +
>>> +                               mixer0_out: port at 1 {
>>> +                                       #address-cells = <1>;
>>> +                                       #size-cells = <0>;
>>> +                                       reg = <1>;
>>> +
>>> +                                       mixer0_out_tcon0: endpoint at 0
>>{
>>> +                                               reg = <0>;
>>> +                                               remote-endpoint =
>><&tcon0_in_mixer0>;
>>> +                                       };
>>> +
>>> +                                       mixer0_out_tcon1: endpoint at 1
>>{
>>> +                                               reg = <1>;
>>> +                                               remote-endpoint =
>><&tcon1_in_mixer0>;
>>> +                                       };
>>> +                               };
>>> +                       };
>>> +               };
>>> +
>>> +               mixer1: mixer at 1200000 {
>>> +                       compatible = "allwinner,sun8i-h3-de2-mixer1";
>>> +                       reg = <0x01200000 0x100000>;
>>> +                       clocks = <&display_clocks CLK_BUS_MIXER1>,
>>> +                                <&display_clocks CLK_MIXER1>;
>>> +                       clock-names = "bus",
>>> +                                     "mod";
>>> +                       resets = <&display_clocks RST_WB>;
>>> +
>>> +                       ports {
>>> +                               #address-cells = <1>;
>>> +                               #size-cells = <0>;
>>> +
>>> +                               mixer1_out: port at 1 {
>>> +                                       #address-cells = <1>;
>>> +                                       #size-cells = <0>;
>>> +                                       reg = <1>;
>>> +
>>> +                                       mixer1_out_tcon1: endpoint at 0
>>{
>>> +                                               reg = <0>;
>>
>>I would prefer if you could stick to the numbering scheme we're using
>>for
>>Display Engine 1.0, as in endpoint 0 links to component 0 of whatever
>>type.
>
> If we keep this we will need a ugly id property in mixer node,
> otherwise we cannot know which TCON to be bind.

Why? You can simply change the logic in your driver from:

    if (remote_endpoint.id) { continue; }

to:

    if (local_endpoint.id != remote_endpoint.id) { continue; }

I don't see the need for any ID property in the mixer node in this case.
The ID is already encoded into the endpoint IDs.

ChenYu

>>
>>We're probably going to stick to that for the R40's incredibly
>>complicated
>>pipeline. I don't want to have any outliers unless absolutely
>>necessary.
>>
>>ChenYu
>>
>>> +                                               remote-endpoint =
>><&tcon1_in_mixer1>;
>>> +                                       };
>>> +
>>> +                                       mixer1_out_tcon0: endpoint at 1
>>{
>>> +                                               reg = <1>;
>>> +                                               remote-endpoint =
>><&tcon0_in_mixer1>;
>>> +                                       };
>>> +                               };
>>> +                       };
>>> +               };
>>> +
>>> +               tcon0: lcd-controller at 1c0c000 {
>>> +                       compatible = "allwinner,sun8i-h3-tcon0";
>>> +                       reg = <0x01c0c000 0x1000>;
>>> +                       interrupts = <GIC_SPI 86
>>IRQ_TYPE_LEVEL_HIGH>;
>>> +                       clocks = <&ccu CLK_BUS_TCON0>,
>>> +                                <&ccu CLK_TCON0>;
>>> +                       clock-names = "ahb",
>>> +                                     "tcon-ch1";
>>> +                       resets = <&ccu RST_BUS_TCON0>;
>>> +                       reset-names = "lcd";
>>> +                       status = "disabled";
>>> +
>>> +                       ports {
>>> +                               #address-cells = <1>;
>>> +                               #size-cells = <0>;
>>> +
>>> +                               tcon0_in: port at 0 {
>>> +                                       #address-cells = <1>;
>>> +                                       #size-cells = <0>;
>>> +                                       reg = <0>;
>>> +
>>> +                                       tcon0_in_mixer0: endpoint at 0 {
>>> +                                               reg = <0>;
>>> +                                               remote-endpoint =
>><&mixer0_out_tcon0>;
>>> +                                       };
>>> +
>>> +                                       tcon0_in_mixer1: endpoint at 1 {
>>> +                                               reg = <1>;
>>> +                                               remote-endpoint =
>><&mixer1_out_tcon0>;
>>> +                                       };
>>> +                               };
>>> +                       };
>>> +               };
>>> +
>>> +               tcon1: lcd-controller at 1c0d000 {
>>> +                       compatible = "allwinner,sun8i-h3-tcon1";
>>> +                       reg = <0x01c0d000 0x1000>;
>>> +                       interrupts = <GIC_SPI 87
>>IRQ_TYPE_LEVEL_HIGH>;
>>> +                       clocks = <&ccu CLK_BUS_TCON1>;
>>> +                       clock-names = "ahb";
>>> +                       resets = <&ccu RST_BUS_TCON1>;
>>> +                       reset-names = "lcd";
>>> +                       status = "disabled";
>>> +
>>> +                       ports {
>>> +                               #address-cells = <1>;
>>> +                               #size-cells = <0>;
>>> +
>>> +                               tcon1_in: port at 0 {
>>> +                                       #address-cells = <1>;
>>> +                                       #size-cells = <0>;
>>> +                                       reg = <0>;
>>> +
>>> +                                       tcon1_in_mixer1: endpoint at 0 {
>>> +                                               reg = <0>;
>>> +                                               remote-endpoint =
>><&mixer1_out_tcon1>;
>>> +                                       };
>>> +
>>> +                                       tcon1_in_mixer0: endpoint at 1 {
>>> +                                               reg = <1>;
>>> +                                               remote-endpoint =
>><&mixer0_out_tcon1>;
>>> +                                       };
>>> +                               };
>>> +
>>> +                               tcon1_out: port at 1 {
>>> +                                       #address-cells = <1>;
>>> +                                       #size-cells = <0>;
>>> +                                       reg = <1>;
>>> +
>>> +                                       tcon1_out_tve0: endpoint at 1 {
>>> +                                               reg = <1>;
>>> +                                               remote-endpoint =
>><&tve0_in_tcon1>;
>>> +                                       };
>>> +                               };
>>> +                       };
>>> +               };
>>> +
>>> +               tve0: tv-encoder at 1e00000 {
>>> +                       compatible = "allwinner,sun8i-h3-tv-encoder";
>>> +                       reg = <0x01e00000 0x1000>;
>>> +                       clocks = <&ccu CLK_BUS_TVE>, <&ccu CLK_TVE>;
>>> +                       clock-names = "bus", "mod";
>>> +                       resets = <&ccu RST_BUS_TVE>;
>>> +                       status = "disabled";
>>> +
>>> +                       assigned-clocks = <&ccu CLK_TVE>;
>>> +                       assigned-clock-parents = <&ccu CLK_PLL_DE>;
>>> +                       assigned-clock-rates = <216000000>;
>>> +
>>> +                       port {
>>> +                               #address-cells = <1>;
>>> +                               #size-cells = <0>;
>>> +
>>> +                               tve0_in_tcon1: endpoint at 0 {
>>> +                                       reg = <0>;
>>> +                                       remote-endpoint =
>><&tcon1_out_tve0>;
>>> +                               };
>>> +                       };
>>> +               };
>>> +       };
>>> +
>>>         timer {
>>>                 compatible = "arm,armv7-timer";
>>>                 interrupts = <GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(4) |
>>IRQ_TYPE_LEVEL_LOW)>,
>>> --
>>> 2.12.2
>>>
>>> --
>>> You received this message because you are subscribed to the Google
>>Groups "linux-sunxi" group.
>>> To unsubscribe from this group and stop receiving emails from it,
>>send an email to linux-sunxi+unsubscribe at googlegroups.com.
>>> For more options, visit https://groups.google.com/d/optout.
>
> --
> You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe at googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.

[linux-sunxi] [RFC PATCH 10/11] ARM: sun8i: h3: add display engine pipeline for TVE

[Icenowy Zheng]

? 2017?5?24? GMT+08:00 ??1:34:58, Chen-Yu Tsai <wens@csie.org> ??:
>On Wed, May 24, 2017 at 1:28 PM, Icenowy Zheng <icenowy@aosc.io> wrote:
>>
>>
>> ? 2017?5?24? GMT+08:00 ??1:24:29, Chen-Yu Tsai <wens@csie.org> ??:
>>>On Thu, May 18, 2017 at 12:43 AM, Icenowy Zheng <icenowy@aosc.io>
>>>wrote:
>>>> As we have already the support for the TV encoder on Allwinner H3,
>>>add
>>>> the display engine pipeline device tree nodes to its DTSI file.
>>>>
>>>> The H5 pipeline has some differences and will be enabled later.
>>>>
>>>> The currently-unused mixer0 and tcon0 are also needed, for the
>>>> completement of the pipeline.
>>>>
>>>> Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
>>>> ---
>>>>  arch/arm/boot/dts/sun8i-h3.dtsi | 189
>>>++++++++++++++++++++++++++++++++++++++++
>>>>  1 file changed, 189 insertions(+)
>>>>
>>>> diff --git a/arch/arm/boot/dts/sun8i-h3.dtsi
>>>b/arch/arm/boot/dts/sun8i-h3.dtsi
>>>> index b36f9f423c39..20172ef92415 100644
>>>> --- a/arch/arm/boot/dts/sun8i-h3.dtsi
>>>> +++ b/arch/arm/boot/dts/sun8i-h3.dtsi
>>>> @@ -41,6 +41,8 @@
>>>>   */
>>>>
>>>>  #include "sunxi-h3-h5.dtsi"
>>>> +#include <dt-bindings/clock/sun8i-de2.h>
>>>> +#include <dt-bindings/reset/sun8i-de2.h>
>>>>
>>>>  / {
>>>>         cpus {
>>>> @@ -72,6 +74,193 @@
>>>>                 };
>>>>         };
>>>>
>>>> +       de: display-engine {
>>>> +               compatible = "allwinner,sun8i-h3-display-engine";
>>>> +               allwinner,pipelines = <&mixer0>,
>>>> +                                     <&mixer1>;
>>>> +               status = "disabled";
>>>> +       };
>>>> +
>>>> +       soc {
>>>> +               display_clocks: clock at 1000000 {
>>>> +                       compatible =
>"allwinner,sun8i-a83t-de2-clk";
>>>> +                       reg = <0x01000000 0x100000>;
>>>> +                       clocks = <&ccu CLK_BUS_DE>,
>>>> +                                <&ccu CLK_DE>;
>>>> +                       clock-names = "bus",
>>>> +                                     "mod";
>>>> +                       resets = <&ccu RST_BUS_DE>;
>>>> +                       #clock-cells = <1>;
>>>> +                       #reset-cells = <1>;
>>>> +                       assigned-clocks = <&ccu CLK_DE>;
>>>> +                       assigned-clock-parents = <&ccu CLK_PLL_DE>;
>>>> +                       assigned-clock-rates = <432000000>;
>>>> +               };
>>>> +
>>>> +               mixer0: mixer at 1100000 {
>>>> +                       compatible =
>"allwinner,sun8i-h3-de2-mixer0";
>>>> +                       reg = <0x01100000 0x100000>;
>>>> +                       clocks = <&display_clocks CLK_BUS_MIXER0>,
>>>> +                                <&display_clocks CLK_MIXER0>;
>>>> +                       clock-names = "bus",
>>>> +                                     "mod";
>>>> +                       resets = <&display_clocks RST_MIXER0>;
>>>> +                       status = "disabled";
>>>> +
>>>> +                       ports {
>>>> +                               #address-cells = <1>;
>>>> +                               #size-cells = <0>;
>>>> +
>>>> +                               mixer0_out: port at 1 {
>>>> +                                       #address-cells = <1>;
>>>> +                                       #size-cells = <0>;
>>>> +                                       reg = <1>;
>>>> +
>>>> +                                       mixer0_out_tcon0:
>endpoint at 0
>>>{
>>>> +                                               reg = <0>;
>>>> +                                               remote-endpoint =
>>><&tcon0_in_mixer0>;
>>>> +                                       };
>>>> +
>>>> +                                       mixer0_out_tcon1:
>endpoint at 1
>>>{
>>>> +                                               reg = <1>;
>>>> +                                               remote-endpoint =
>>><&tcon1_in_mixer0>;
>>>> +                                       };
>>>> +                               };
>>>> +                       };
>>>> +               };
>>>> +
>>>> +               mixer1: mixer at 1200000 {
>>>> +                       compatible =
>"allwinner,sun8i-h3-de2-mixer1";
>>>> +                       reg = <0x01200000 0x100000>;
>>>> +                       clocks = <&display_clocks CLK_BUS_MIXER1>,
>>>> +                                <&display_clocks CLK_MIXER1>;
>>>> +                       clock-names = "bus",
>>>> +                                     "mod";
>>>> +                       resets = <&display_clocks RST_WB>;
>>>> +
>>>> +                       ports {
>>>> +                               #address-cells = <1>;
>>>> +                               #size-cells = <0>;
>>>> +
>>>> +                               mixer1_out: port at 1 {
>>>> +                                       #address-cells = <1>;
>>>> +                                       #size-cells = <0>;
>>>> +                                       reg = <1>;
>>>> +
>>>> +                                       mixer1_out_tcon1:
>endpoint at 0
>>>{
>>>> +                                               reg = <0>;
>>>
>>>I would prefer if you could stick to the numbering scheme we're using
>>>for
>>>Display Engine 1.0, as in endpoint 0 links to component 0 of whatever
>>>type.
>>
>> If we keep this we will need a ugly id property in mixer node,
>> otherwise we cannot know which TCON to be bind.
>
>Why? You can simply change the logic in your driver from:
>
>    if (remote_endpoint.id) { continue; }
>
>to:
>
>    if (local_endpoint.id != remote_endpoint.id) { continue; }

Thanks, I forgot that there's two endpoint ids...

So silly I am.

>
>I don't see the need for any ID property in the mixer node in this
>case.
>The ID is already encoded into the endpoint IDs.
>
>ChenYu
>
>>>
>>>We're probably going to stick to that for the R40's incredibly
>>>complicated
>>>pipeline. I don't want to have any outliers unless absolutely
>>>necessary.
>>>
>>>ChenYu
>>>
>>>> +                                               remote-endpoint =
>>><&tcon1_in_mixer1>;
>>>> +                                       };
>>>> +
>>>> +                                       mixer1_out_tcon0:
>endpoint at 1
>>>{
>>>> +                                               reg = <1>;
>>>> +                                               remote-endpoint =
>>><&tcon0_in_mixer1>;
>>>> +                                       };
>>>> +                               };
>>>> +                       };
>>>> +               };
>>>> +
>>>> +               tcon0: lcd-controller at 1c0c000 {
>>>> +                       compatible = "allwinner,sun8i-h3-tcon0";
>>>> +                       reg = <0x01c0c000 0x1000>;
>>>> +                       interrupts = <GIC_SPI 86
>>>IRQ_TYPE_LEVEL_HIGH>;
>>>> +                       clocks = <&ccu CLK_BUS_TCON0>,
>>>> +                                <&ccu CLK_TCON0>;
>>>> +                       clock-names = "ahb",
>>>> +                                     "tcon-ch1";
>>>> +                       resets = <&ccu RST_BUS_TCON0>;
>>>> +                       reset-names = "lcd";
>>>> +                       status = "disabled";
>>>> +
>>>> +                       ports {
>>>> +                               #address-cells = <1>;
>>>> +                               #size-cells = <0>;
>>>> +
>>>> +                               tcon0_in: port at 0 {
>>>> +                                       #address-cells = <1>;
>>>> +                                       #size-cells = <0>;
>>>> +                                       reg = <0>;
>>>> +
>>>> +                                       tcon0_in_mixer0: endpoint at 0
>{
>>>> +                                               reg = <0>;
>>>> +                                               remote-endpoint =
>>><&mixer0_out_tcon0>;
>>>> +                                       };
>>>> +
>>>> +                                       tcon0_in_mixer1: endpoint at 1
>{
>>>> +                                               reg = <1>;
>>>> +                                               remote-endpoint =
>>><&mixer1_out_tcon0>;
>>>> +                                       };
>>>> +                               };
>>>> +                       };
>>>> +               };
>>>> +
>>>> +               tcon1: lcd-controller at 1c0d000 {
>>>> +                       compatible = "allwinner,sun8i-h3-tcon1";
>>>> +                       reg = <0x01c0d000 0x1000>;
>>>> +                       interrupts = <GIC_SPI 87
>>>IRQ_TYPE_LEVEL_HIGH>;
>>>> +                       clocks = <&ccu CLK_BUS_TCON1>;
>>>> +                       clock-names = "ahb";
>>>> +                       resets = <&ccu RST_BUS_TCON1>;
>>>> +                       reset-names = "lcd";
>>>> +                       status = "disabled";
>>>> +
>>>> +                       ports {
>>>> +                               #address-cells = <1>;
>>>> +                               #size-cells = <0>;
>>>> +
>>>> +                               tcon1_in: port at 0 {
>>>> +                                       #address-cells = <1>;
>>>> +                                       #size-cells = <0>;
>>>> +                                       reg = <0>;
>>>> +
>>>> +                                       tcon1_in_mixer1: endpoint at 0
>{
>>>> +                                               reg = <0>;
>>>> +                                               remote-endpoint =
>>><&mixer1_out_tcon1>;
>>>> +                                       };
>>>> +
>>>> +                                       tcon1_in_mixer0: endpoint at 1
>{
>>>> +                                               reg = <1>;
>>>> +                                               remote-endpoint =
>>><&mixer0_out_tcon1>;
>>>> +                                       };
>>>> +                               };
>>>> +
>>>> +                               tcon1_out: port at 1 {
>>>> +                                       #address-cells = <1>;
>>>> +                                       #size-cells = <0>;
>>>> +                                       reg = <1>;
>>>> +
>>>> +                                       tcon1_out_tve0: endpoint at 1
>{
>>>> +                                               reg = <1>;
>>>> +                                               remote-endpoint =
>>><&tve0_in_tcon1>;
>>>> +                                       };
>>>> +                               };
>>>> +                       };
>>>> +               };
>>>> +
>>>> +               tve0: tv-encoder at 1e00000 {
>>>> +                       compatible =
>"allwinner,sun8i-h3-tv-encoder";
>>>> +                       reg = <0x01e00000 0x1000>;
>>>> +                       clocks = <&ccu CLK_BUS_TVE>, <&ccu
>CLK_TVE>;
>>>> +                       clock-names = "bus", "mod";
>>>> +                       resets = <&ccu RST_BUS_TVE>;
>>>> +                       status = "disabled";
>>>> +
>>>> +                       assigned-clocks = <&ccu CLK_TVE>;
>>>> +                       assigned-clock-parents = <&ccu CLK_PLL_DE>;
>>>> +                       assigned-clock-rates = <216000000>;
>>>> +
>>>> +                       port {
>>>> +                               #address-cells = <1>;
>>>> +                               #size-cells = <0>;
>>>> +
>>>> +                               tve0_in_tcon1: endpoint at 0 {
>>>> +                                       reg = <0>;
>>>> +                                       remote-endpoint =
>>><&tcon1_out_tve0>;
>>>> +                               };
>>>> +                       };
>>>> +               };
>>>> +       };
>>>> +
>>>>         timer {
>>>>                 compatible = "arm,armv7-timer";
>>>>                 interrupts = <GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(4) |
>>>IRQ_TYPE_LEVEL_LOW)>,
>>>> --
>>>> 2.12.2
>>>>
>>>> --
>>>> You received this message because you are subscribed to the Google
>>>Groups "linux-sunxi" group.
>>>> To unsubscribe from this group and stop receiving emails from it,
>>>send an email to linux-sunxi+unsubscribe at googlegroups.com.
>>>> For more options, visit https://groups.google.com/d/optout.
>>
>> --
>> You received this message because you are subscribed to the Google
>Groups "linux-sunxi" group.
>> To unsubscribe from this group and stop receiving emails from it,
>send an email to linux-sunxi+unsubscribe at googlegroups.com.
>> For more options, visit https://groups.google.com/d/optout.

[RFC PATCH 11/11] [DO NOT MERGE] ARM: sun8i: h3: enable TV output on Orange Pi PC

[Icenowy Zheng]

Orange Pi PC features a 3.5mm jack with TV output in it.

Enable the TV output.

As it currently do not have jack detection feature, do not merge this
patch.

Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
---
 arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts b/arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts
index 1a044b17d6c6..9c50ac3e82f3 100644
--- a/arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts
+++ b/arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts
@@ -97,6 +97,10 @@
 	status = "okay";
 };
 
+&de {
+	status = "okay";
+};
+
 &ehci0 {
 	status = "okay";
 };
@@ -169,6 +173,14 @@
 	status = "okay";
 };
 
+&tcon1 {
+	status = "okay";
+};
+
+&tve0 {
+	status = "okay";
+};
+
 &uart0 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&uart0_pins_a>;
-- 
2.12.2