Re: [PATCH 7/15] dt-bindings: display: sun4i: Add allwinner,tcon-channel property

Re: [PATCH 7/15] dt-bindings: display: sun4i: Add allwinner,tcon-channel property

[Icenowy Zheng]

CjIwMTflubQz5pyIMjfml6Ug5LiK5Y2INToxMeS6jiBNYXhpbWUgUmlwYXJkIDxtYXhpbWUucmlw
YXJkQGZyZWUtZWxlY3Ryb25zLmNvbT7lhpnpgZPvvJoKPgo+IE9uIEZyaSwgTWFyIDE3LCAyMDE3
IGF0IDExOjM0OjQ1QU0gKzA4MDAsIENoZW4tWXUgVHNhaSB3cm90ZTogCj4gPiBPbiBUaHUsIE1h
ciAxNiwgMjAxNyBhdCAxOjM3IEFNLCBSb2IgSGVycmluZyA8cm9iaEBrZXJuZWwub3JnPiB3cm90
ZTogCj4gPiA+IE9uIFR1ZSwgTWFyIDA3LCAyMDE3IGF0IDA5OjU2OjI2QU0gKzAxMDAsIE1heGlt
ZSBSaXBhcmQgd3JvdGU6IAo+ID4gPj4gVGhlIEFsbHdpbm5lciBUaW1pbmdzIENvbnRyb2xsZXIg
aGFzIHR3bywgbXV0dWFsbHkgZXhjbHVzaXZlLCBjaGFubmVscy4gCj4gPiA+PiBXaGVuIHRoZSBi
aW5kaW5nIGhhcyBiZWVuIGludHJvZHVjZWQsIGl0IHdhcyBhc3N1bWVkIHRoYXQgdGhlcmUgd291
bGQgYmUgCj4gPiA+PiBvbmx5IGEgc2luZ2xlIHVzZXIgcGVyIGNoYW5uZWwgaW4gdGhlIHN5c3Rl
bS4gCj4gPiA+PiAKPiA+ID4+IFdoaWxlIHRoaXMgaXMgbGlrZWx5IGZvciB0aGUgY2hhbm5lbCAw
IHdoaWNoIG9ubHkgY29ubmVjdHMgdG8gTENEIGRpc3BsYXlzLCAKPiA+ID4+IGl0IHR1cm5zIG91
dCB0aGF0IHRoZSBjaGFubmVsIDEgY2FuIGJlIGNvbm5lY3RlZCB0byBtdWx0aXBsZSBjb250cm9s
bGVycyBpbiAKPiA+ID4+IHRoZSBTb0MgKEhETUkgYW5kIFRWIGVuY29kZXJzIGZvciBleGFtcGxl
KS4gQW5kIHdoaWxlIHRoZSBzaW11bHRhbmVvdXMgdXNlIAo+ID4gPj4gb2YgSERNSSBhbmQgVFYg
b3V0cHV0cyBjYW5ub3QgYmUgYWNoaWV2ZWQsIHN3aXRjaGluZyBmcm9tIG9uZSB0byB0aGUgb3Ro
ZXIgCj4gPiA+PiBhdCBydW50aW1lIGRlZmluaXRlbHkgc291bmRzIHBsYXVzaWJsZS4gCj4gPiA+
PiAKPiA+ID4+IEFkZCBhbiBleHRyYSBwcm9wZXJ0eSwgYWxsd2lubmVyLHRjb24tY2hhbm5lbCwg
dG8gc3BlY2lmeSBmb3IgYSBnaXZlbiAKPiA+ID4+IGVuZHBvaW50IHdoaWNoIFRDT04gY2hhbm5l
bCBpdCBpcyBjb25uZWN0ZWQgdG8sIHdoaWxlIGZhbGxpbmcgYmFjayB0byB0aGUgCj4gPiA+PiBw
cmV2aW91cyBtZWNoYW5pc20gaWYgdGhhdCBwcm9wZXJ0eSBpcyBtaXNzaW5nLiAKPiA+ID4gCj4g
PiA+IEkgdGhpbmsgcGVyaGFwcyBUQ09OIGNoYW5uZWxzIHNob3VsZCBoYXZlIGJlZW4gcG9ydHMg
cmF0aGVyIHRoYW4gCj4gPiA+IGVuZHBvaW50cy4gVGhlIGZhY3QgdGhhdCB0aGUgY2hhbm5lbHMg
YXJlIG11dHVhbGx5IGV4Y2x1c2l2ZSBjYW4gYmUgCj4gPiA+IGhhbmRsZWQgaW4gdGhlIGRyaXZl
ciBhbmQgZG9lc24ndCByZWFsbHkgbWF0dGVyIGluIHRoZSBiaW5kaW5nLiBIb3cgCj4gPiA+IHBh
aW5mdWwgd291bGQgaXQgYmUgdG8gcmV3b3JrIHRoaW5ncyB0byBtb3ZlIFRDT04gY2hhbm5lbCAx
IGZyb20gcG9ydCAwLCAKPiA+ID4gZW5kcG9pbnQgMSB0byBwb3J0IDE/IAo+ID4gCj4gPiBIYXZp
bmcgYSBzZXBhcmF0ZSBvdXRwdXQgcG9ydCBmb3IgY2hhbm5lbCAxIHdhcyBvbmUgb3B0aW9uIHdl
IGRpc2N1c3NlZC4gCj4gPiBIb3dldmVyIGl0IHdvdWxkbid0IHdvcmsgd2VsbCB3aXRoIHRoZSBr
ZXJuZWwncyBvZl9ncmFwaCBiYXNlZCBjcnRjIAo+ID4gZGV0ZWN0aW9uIChkcm1fb2ZfZmluZF9w
b3NzaWJsZV9jcnRjcyAvIGRybV9vZl9maW5kX3Bvc3NpYmxlX2NydGNzKSwgCj4gPiB3aGljaCBo
YXMgdGhlIGNydGNzIGJvdW5kIHZpYSB0aGUgb3V0cHV0IHBvcnQuIEFzIHRoZSBsb2dpYyBpcyB1
c2VkIAo+ID4gYnkgbXVsdGlwbGUgZHJpdmVycywgSSdtIG5vdCBzdXJlIGl0J3MgZWFzeSB0byBy
ZXdvcmsgb3IgdGVzdC4gCj4KPiBDYW4ndCB3ZSB1c2UgYSBkaWZmZXJlbnQgbG9naWMgdGhhbiBk
cm1fb2ZfZmluZF9wb3NzaWJsZV9jcnRjcyB0byBmaWxsIAo+IHRoZSBzYW1lIHJvbGU/IAo+Cj4g
PiBBbHNvLCB3ZSBzdGlsbCBoYXZlIHRvIHN1cHBvcnQgb2xkIGRldmljZSB0cmVlcyB1c2luZyBj
aGFubmVsIDEgZnJvbSAKPiA+IG91dHB1dCBwb3J0IDAgZW5kcG9pbnQgMS4gVGhpcyBpcyB0aGUg
VFYgZW5jb2RlciBvbiBzdW41aSAoQTEwcy9BMTMvUjgpLiAKCkFuZCBmcm9tIEE4M1Qgb24gd2Ug
d2lsbCBmYWNlIGNoYW5uZWwtMSBvbmx5IFRDT05zLiwgd2hpY2ggZG8gbm90CmhhdmUgY2hhbm5l
bCAwIGF0IGFsbCBpbiBoYXJkd2FyZS4KCj4KPiBXZSBjb3VsZCBwcm9iYWJseSB3b3JrIHNvbWV0
aGluZyBvdXQgaWYgd2UgZ28gdGhhdCB3YXkgdG8gZGVhbCB3aXRoIAo+IG9sZCBEVHMgdGhvdWdo
LiAKPgo+IE1heGltZSAKPgo+IC0tIAo+IE1heGltZSBSaXBhcmQsIEZyZWUgRWxlY3Ryb25zIAo+
IEVtYmVkZGVkIExpbnV4IGFuZCBLZXJuZWwgZW5naW5lZXJpbmcgCj4gaHR0cDovL2ZyZWUtZWxl
Y3Ryb25zLmNvbQo+Cj4gX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f
X19fX18gCj4gbGludXgtYXJtLWtlcm5lbCBtYWlsaW5nIGxpc3QgCj4gbGludXgtYXJtLWtlcm5l
bEBsaXN0cy5pbmZyYWRlYWQub3JnIAo+IGh0dHA6Ly9saXN0cy5pbmZyYWRlYWQub3JnL21haWxt
YW4vbGlzdGluZm8vbGludXgtYXJtLWtlcm5lbCAK

Re: [PATCH 7/15] dt-bindings: display: sun4i: Add allwinner,tcon-channel property

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 2600 bytes --]

On Tue, Mar 28, 2017 at 06:05:08PM +0800, Icenowy Zheng wrote:
> 
> 2017年3月27日 上午5:11于 Maxime Ripard <maxime.ripard@free-electrons.com>写道：
> >
> > On Fri, Mar 17, 2017 at 11:34:45AM +0800, Chen-Yu Tsai wrote: 
> > > On Thu, Mar 16, 2017 at 1:37 AM, Rob Herring <robh@kernel.org> wrote: 
> > > > On Tue, Mar 07, 2017 at 09:56:26AM +0100, Maxime Ripard wrote: 
> > > >> The Allwinner Timings Controller has two, mutually exclusive, channels. 
> > > >> When the binding has been introduced, it was assumed that there would be 
> > > >> only a single user per channel in the system. 
> > > >> 
> > > >> While this is likely for the channel 0 which only connects to LCD displays, 
> > > >> it turns out that the channel 1 can be connected to multiple controllers in 
> > > >> the SoC (HDMI and TV encoders for example). And while the simultaneous use 
> > > >> of HDMI and TV outputs cannot be achieved, switching from one to the other 
> > > >> at runtime definitely sounds plausible. 
> > > >> 
> > > >> Add an extra property, allwinner,tcon-channel, to specify for a given 
> > > >> endpoint which TCON channel it is connected to, while falling back to the 
> > > >> previous mechanism if that property is missing. 
> > > > 
> > > > I think perhaps TCON channels should have been ports rather than 
> > > > endpoints. The fact that the channels are mutually exclusive can be 
> > > > handled in the driver and doesn't really matter in the binding. How 
> > > > painful would it be to rework things to move TCON channel 1 from port 0, 
> > > > endpoint 1 to port 1? 
> > > 
> > > Having a separate output port for channel 1 was one option we discussed. 
> > > However it wouldn't work well with the kernel's of_graph based crtc 
> > > detection (drm_of_find_possible_crtcs / drm_of_find_possible_crtcs), 
> > > which has the crtcs bound via the output port. As the logic is used 
> > > by multiple drivers, I'm not sure it's easy to rework or test. 
> >
> > Can't we use a different logic than drm_of_find_possible_crtcs to fill 
> > the same role? 
> >
> > > Also, we still have to support old device trees using channel 1 from 
> > > output port 0 endpoint 1. This is the TV encoder on sun5i (A10s/A13/R8). 
> 
> And from A83T on we will face channel-1 only TCONs., which do not
> have channel 0 at all in hardware.

Yes, but those patches have not been merged yet, so we don't have to
care about the backward compatibility.

Maxime

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

[PATCH 0/15] drm: sun4i: Add support for the HDMI controller

[Maxime Ripard]

Hi,

Here is an attempt at getting the HDMI controller running.

This HDMI controller is found on a number of old Allwinner SoCs (A10, A10s,
A20, A31).

This driver only supports for now the A10s because it was an easy target,
being very close to the A13 that is already supported by our DRM driver.

There's nothing out of the extraordinary there, except maybe the clock
setup. All the internal clocks (TMDS, DDC) have been modeled using the
common clock framework, the TMDS clock being the parent of the DDC one.

While this might sound overkill, other SoC have a different, external
source for the DDC clock, which will be easier to support through the clock
framework.

It's still a bit rough around the edges, as it doesn't work for all the
modes. This will need to be fixed before being merged obviously.

The IP also supports audio (through an already supported i2s controller,
and some missing configuration in the HDMI controller) and CEC. Both will
come eventually.

Let me know what you think!
Maxime

Maxime Ripard (15):
  clk: divider: Make divider_round_rate take the parent clock
  clk: sunxi-ng: Pass the parent and a pointer to the clocks round rate
  clk: sunxi-ng: div: Switch to divider_round_rate
  clk: sunxi-ng: mux: Don't just rely on the parent for CLK_SET_RATE_PARENT
  clk: sunxi-ng: sun5i: Export video PLLs
  dt-bindings: display: sun4i: Add HDMI display bindings
  dt-bindings: display: sun4i: Add allwinner,tcon-channel property
  drm/sun4i: tcon: Add channel debug
  drm/sun4i: tcon: Pass the encoder to the mode set functions
  drm/sun4i: tcon: Switch mux on only for composite
  drm/sun4i: tcon: Fix tcon channel 1 backporch calculation
  drm/sun4i: tcon: multiply the vtotal when not in interlace
  drm/sun4i: Add HDMI support
  ARM: sun5i: a10s: Add the HDMI controller node
  ARM: sun5i: a10s-olinuxino: Enable HDMI

 Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt |  32 +-
 arch/arm/boot/dts/sun5i-a10s-olinuxino-micro.dts              |  12 +-
 arch/arm/boot/dts/sun5i-a10s.dtsi                             |  34 +-
 arch/arm/boot/dts/sun5i.dtsi                                  |   1 +-
 drivers/clk/clk-divider.c                                     |  18 +-
 drivers/clk/hisilicon/clkdivider-hi6220.c                     |   5 +-
 drivers/clk/meson/clk-cpu.c                                   |   5 +-
 drivers/clk/nxp/clk-lpc32xx.c                                 |   5 +-
 drivers/clk/qcom/clk-alpha-pll.c                              |   5 +-
 drivers/clk/qcom/clk-regmap-divider.c                         |   3 +-
 drivers/clk/sunxi-ng/ccu-sun5i.h                              |   6 +-
 drivers/clk/sunxi-ng/ccu_div.c                                |  28 +-
 drivers/clk/sunxi-ng/ccu_mp.c                                 |   7 +-
 drivers/clk/sunxi-ng/ccu_mult.c                               |  11 +-
 drivers/clk/sunxi-ng/ccu_mux.c                                |  22 +-
 drivers/clk/sunxi-ng/ccu_mux.h                                |   3 +-
 drivers/clk/sunxi-ng/ccu_nkm.c                                |   7 +-
 drivers/gpu/drm/sun4i/Makefile                                |   5 +-
 drivers/gpu/drm/sun4i/sun4i_hdmi.h                            | 124 ++-
 drivers/gpu/drm/sun4i/sun4i_hdmi_ddc_clk.c                    | 128 ++-
 drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c                        | 449 +++++++-
 drivers/gpu/drm/sun4i/sun4i_hdmi_tmds_clk.c                   | 236 ++++-
 drivers/gpu/drm/sun4i/sun4i_rgb.c                             |   2 +-
 drivers/gpu/drm/sun4i/sun4i_tcon.c                            |  25 +-
 drivers/gpu/drm/sun4i/sun4i_tcon.h                            |   4 +-
 drivers/gpu/drm/sun4i/sun4i_tv.c                              |   2 +-
 drivers/rtc/rtc-ac100.c                                       |   6 +-
 include/dt-bindings/clock/sun5i-ccu.h                         |   3 +-
 include/linux/clk-provider.h                                  |   5 +-
 29 files changed, 1103 insertions(+), 90 deletions(-)
 create mode 100644 drivers/gpu/drm/sun4i/sun4i_hdmi.h
 create mode 100644 drivers/gpu/drm/sun4i/sun4i_hdmi_ddc_clk.c
 create mode 100644 drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c
 create mode 100644 drivers/gpu/drm/sun4i/sun4i_hdmi_tmds_clk.c

base-commit: d1bee31b9da7222c6be3248d1f3b087e8cc9004c
-- 
git-series 0.8.11

[PATCH 7/15] dt-bindings: display: sun4i: Add allwinner,tcon-channel property

[Maxime Ripard]

The Allwinner Timings Controller has two, mutually exclusive, channels.
When the binding has been introduced, it was assumed that there would be
only a single user per channel in the system.

While this is likely for the channel 0 which only connects to LCD displays,
it turns out that the channel 1 can be connected to multiple controllers in
the SoC (HDMI and TV encoders for example). And while the simultaneous use
of HDMI and TV outputs cannot be achieved, switching from one to the other
at runtime definitely sounds plausible.

Add an extra property, allwinner,tcon-channel, to specify for a given
endpoint which TCON channel it is connected to, while falling back to the
previous mechanism if that property is missing.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt | 11 ++++---
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt b/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt
index 4b280672658e..18d445723c3e 100644
--- a/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt
+++ b/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt
@@ -68,10 +68,13 @@ Required properties:
   Documentation/devicetree/bindings/media/video-interfaces.txt. The
   first port should be the input endpoint, the second one the output
 
-  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)
+  The output may have multiple endpoints. The TCON has two channels,
+  usually with the first channel being used for the panels interfaces
+  (RGB, LVDS, etc.), and the second being used for the outputs that
+  require another controller (TV Encoder, HDMI, etc.). The endpoints
+  will take an extra property, allwinner,tcon-channel, to specify the
+  channel the endpoint is associated to. If that property is not
+  present, the endpoint number will be used as the channel number.
 
 On SoCs other than the A33, there is one more clock required:
    - 'tcon-ch1': The clock driving the TCON channel 1
-- 
git-series 0.8.11

Re: [PATCH 7/15] dt-bindings: display: sun4i: Add allwinner,tcon-channel property

[Chen-Yu Tsai]

On Tue, Mar 7, 2017 at 4:56 PM, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:
> The Allwinner Timings Controller has two, mutually exclusive, channels.
> When the binding has been introduced, it was assumed that there would be
> only a single user per channel in the system.
>
> While this is likely for the channel 0 which only connects to LCD displays,
> it turns out that the channel 1 can be connected to multiple controllers in
> the SoC (HDMI and TV encoders for example). And while the simultaneous use
> of HDMI and TV outputs cannot be achieved, switching from one to the other
> at runtime definitely sounds plausible.
>
> Add an extra property, allwinner,tcon-channel, to specify for a given
> endpoint which TCON channel it is connected to, while falling back to the
> previous mechanism if that property is missing.
>
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>

Acked-by: Chen-Yu Tsai <wens@csie.org>

Re: [PATCH 7/15] dt-bindings: display: sun4i: Add allwinner,tcon-channel property

[Rob Herring]

On Tue, Mar 07, 2017 at 09:56:26AM +0100, Maxime Ripard wrote:
> The Allwinner Timings Controller has two, mutually exclusive, channels.
> When the binding has been introduced, it was assumed that there would be
> only a single user per channel in the system.
> 
> While this is likely for the channel 0 which only connects to LCD displays,
> it turns out that the channel 1 can be connected to multiple controllers in
> the SoC (HDMI and TV encoders for example). And while the simultaneous use
> of HDMI and TV outputs cannot be achieved, switching from one to the other
> at runtime definitely sounds plausible.
> 
> Add an extra property, allwinner,tcon-channel, to specify for a given
> endpoint which TCON channel it is connected to, while falling back to the
> previous mechanism if that property is missing.

I think perhaps TCON channels should have been ports rather than 
endpoints. The fact that the channels are mutually exclusive can be 
handled in the driver and doesn't really matter in the binding. How 
painful would it be to rework things to move TCON channel 1 from port 0, 
endpoint 1 to port 1? 

Rob

Re: [PATCH 7/15] dt-bindings: display: sun4i: Add allwinner,tcon-channel property

[Chen-Yu Tsai]

On Thu, Mar 16, 2017 at 1:37 AM, Rob Herring <robh@kernel.org> wrote:
> On Tue, Mar 07, 2017 at 09:56:26AM +0100, Maxime Ripard wrote:
>> The Allwinner Timings Controller has two, mutually exclusive, channels.
>> When the binding has been introduced, it was assumed that there would be
>> only a single user per channel in the system.
>>
>> While this is likely for the channel 0 which only connects to LCD displays,
>> it turns out that the channel 1 can be connected to multiple controllers in
>> the SoC (HDMI and TV encoders for example). And while the simultaneous use
>> of HDMI and TV outputs cannot be achieved, switching from one to the other
>> at runtime definitely sounds plausible.
>>
>> Add an extra property, allwinner,tcon-channel, to specify for a given
>> endpoint which TCON channel it is connected to, while falling back to the
>> previous mechanism if that property is missing.
>
> I think perhaps TCON channels should have been ports rather than
> endpoints. The fact that the channels are mutually exclusive can be
> handled in the driver and doesn't really matter in the binding. How
> painful would it be to rework things to move TCON channel 1 from port 0,
> endpoint 1 to port 1?

Having a separate output port for channel 1 was one option we discussed.
However it wouldn't work well with the kernel's of_graph based crtc
detection (drm_of_find_possible_crtcs / drm_of_find_possible_crtcs),
which has the crtcs bound via the output port. As the logic is used
by multiple drivers, I'm not sure it's easy to rework or test.

Also, we still have to support old device trees using channel 1 from
output port 0 endpoint 1. This is the TV encoder on sun5i (A10s/A13/R8).

Regards
ChenYu

Re: [PATCH 7/15] dt-bindings: display: sun4i: Add allwinner,tcon-channel property

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 2135 bytes --]

On Fri, Mar 17, 2017 at 11:34:45AM +0800, Chen-Yu Tsai wrote:
> On Thu, Mar 16, 2017 at 1:37 AM, Rob Herring <robh@kernel.org> wrote:
> > On Tue, Mar 07, 2017 at 09:56:26AM +0100, Maxime Ripard wrote:
> >> The Allwinner Timings Controller has two, mutually exclusive, channels.
> >> When the binding has been introduced, it was assumed that there would be
> >> only a single user per channel in the system.
> >>
> >> While this is likely for the channel 0 which only connects to LCD displays,
> >> it turns out that the channel 1 can be connected to multiple controllers in
> >> the SoC (HDMI and TV encoders for example). And while the simultaneous use
> >> of HDMI and TV outputs cannot be achieved, switching from one to the other
> >> at runtime definitely sounds plausible.
> >>
> >> Add an extra property, allwinner,tcon-channel, to specify for a given
> >> endpoint which TCON channel it is connected to, while falling back to the
> >> previous mechanism if that property is missing.
> >
> > I think perhaps TCON channels should have been ports rather than
> > endpoints. The fact that the channels are mutually exclusive can be
> > handled in the driver and doesn't really matter in the binding. How
> > painful would it be to rework things to move TCON channel 1 from port 0,
> > endpoint 1 to port 1?
> 
> Having a separate output port for channel 1 was one option we discussed.
> However it wouldn't work well with the kernel's of_graph based crtc
> detection (drm_of_find_possible_crtcs / drm_of_find_possible_crtcs),
> which has the crtcs bound via the output port. As the logic is used
> by multiple drivers, I'm not sure it's easy to rework or test.

Can't we use a different logic than drm_of_find_possible_crtcs to fill
the same role?

> Also, we still have to support old device trees using channel 1 from
> output port 0 endpoint 1. This is the TV encoder on sun5i (A10s/A13/R8).

We could probably work something out if we go that way to deal with
old DTs though.

Maxime

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]