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

[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 --]

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 --]

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

[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 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>

[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