[PATCH v3 6/6] arm64: dts: rockchip: Specify override mode for kevin panel

[PATCH v3 6/6] arm64: dts: rockchip: Specify override mode for kevin panel

[Sean Paul]

This patch adds an override mode for kevin devices. The mode increases
both back porches to allow a pixel clock of 26666kHz as opposed to the
'typical' value of 252750kHz. This is needed to avoid interference with
the touch digitizer on these laptops.

Changes in v2:
 - Wrap the timing in display-timings node to match binding (Rob/Thierry)
Changes in v3:
 - Unwrap the timing from display-timings and rename panel-timing (Rob)

Cc: Doug Anderson <dianders@chromium.org>
Cc: Eric Anholt <eric@anholt.net>
Cc: Heiko Stuebner <heiko@sntech.de>
Cc: Jeffy Chen <jeffy.chen@rock-chips.com>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: St?phane Marchesin <marcheu@chromium.org>
Cc: Thierry Reding <thierry.reding@gmail.com>
Cc: devicetree at vger.kernel.org
Cc: dri-devel at lists.freedesktop.org
Cc: linux-rockchip at lists.infradead.org
Signed-off-by: Sean Paul <seanpaul@chromium.org>
---
 arch/arm64/boot/dts/rockchip/rk3399-gru-kevin.dts | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3399-gru-kevin.dts b/arch/arm64/boot/dts/rockchip/rk3399-gru-kevin.dts
index 191a6bcb1704..658411ce37ea 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399-gru-kevin.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3399-gru-kevin.dts
@@ -98,6 +98,20 @@
 		backlight = <&backlight>;
 		power-supply = <&pp3300_disp>;
 
+		panel-timing {
+			clock-frequency = <266604720>;
+			hactive = <2400>;
+			hfront-porch = <48>;
+			hback-porch = <84>;
+			hsync-len = <32>;
+			hsync-active = <0>;
+			vactive = <1600>;
+			vfront-porch = <3>;
+			vback-porch = <120>;
+			vsync-len = <10>;
+			vsync-active = <0>;
+		};
+
 		ports {
 			panel_in_edp: endpoint {
 				remote-endpoint = <&edp_out_panel>;
-- 
2.16.0.rc1.238.g530d649a79-goog

[PATCH v3 6/6] arm64: dts: rockchip: Specify override mode for kevin panel

[Enric Balletbo Serra]

2018-02-08 18:48 GMT+01:00 Sean Paul <seanpaul@chromium.org>:
> This patch adds an override mode for kevin devices. The mode increases
> both back porches to allow a pixel clock of 26666kHz as opposed to the
> 'typical' value of 252750kHz. This is needed to avoid interference with
> the touch digitizer on these laptops.
>
> Changes in v2:
>  - Wrap the timing in display-timings node to match binding (Rob/Thierry)
> Changes in v3:
>  - Unwrap the timing from display-timings and rename panel-timing (Rob)
>
> Cc: Doug Anderson <dianders@chromium.org>
> Cc: Eric Anholt <eric@anholt.net>
> Cc: Heiko Stuebner <heiko@sntech.de>
> Cc: Jeffy Chen <jeffy.chen@rock-chips.com>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: St?phane Marchesin <marcheu@chromium.org>
> Cc: Thierry Reding <thierry.reding@gmail.com>
> Cc: devicetree at vger.kernel.org
> Cc: dri-devel at lists.freedesktop.org
> Cc: linux-rockchip at lists.infradead.org
> Signed-off-by: Sean Paul <seanpaul@chromium.org>
> ---
>  arch/arm64/boot/dts/rockchip/rk3399-gru-kevin.dts | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/rockchip/rk3399-gru-kevin.dts b/arch/arm64/boot/dts/rockchip/rk3399-gru-kevin.dts
> index 191a6bcb1704..658411ce37ea 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3399-gru-kevin.dts
> +++ b/arch/arm64/boot/dts/rockchip/rk3399-gru-kevin.dts
> @@ -98,6 +98,20 @@
>                 backlight = <&backlight>;
>                 power-supply = <&pp3300_disp>;
>
> +               panel-timing {
> +                       clock-frequency = <266604720>;
> +                       hactive = <2400>;
> +                       hfront-porch = <48>;
> +                       hback-porch = <84>;
> +                       hsync-len = <32>;
> +                       hsync-active = <0>;
> +                       vactive = <1600>;
> +                       vfront-porch = <3>;
> +                       vback-porch = <120>;
> +                       vsync-len = <10>;
> +                       vsync-active = <0>;
> +               };
> +
>                 ports {
>                         panel_in_edp: endpoint {
>                                 remote-endpoint = <&edp_out_panel>;
> --


Tested on top of linux-next on a Samsung Chromebook Plus.

Tested-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>


> 2.16.0.rc1.238.g530d649a79-goog
>
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v3 6/6] arm64: dts: rockchip: Specify override mode for kevin panel

[Doug Anderson]

Hi,

On Thu, Feb 8, 2018 at 9:48 AM, Sean Paul <seanpaul@chromium.org> wrote:
> This patch adds an override mode for kevin devices. The mode increases
> both back porches to allow a pixel clock of 26666kHz as opposed to the
> 'typical' value of 252750kHz. This is needed to avoid interference with
> the touch digitizer on these laptops.
>
> Changes in v2:
>  - Wrap the timing in display-timings node to match binding (Rob/Thierry)
> Changes in v3:
>  - Unwrap the timing from display-timings and rename panel-timing (Rob)
>
> Cc: Doug Anderson <dianders@chromium.org>
> Cc: Eric Anholt <eric@anholt.net>
> Cc: Heiko Stuebner <heiko@sntech.de>
> Cc: Jeffy Chen <jeffy.chen@rock-chips.com>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: St?phane Marchesin <marcheu@chromium.org>
> Cc: Thierry Reding <thierry.reding@gmail.com>
> Cc: devicetree at vger.kernel.org
> Cc: dri-devel at lists.freedesktop.org
> Cc: linux-rockchip at lists.infradead.org
> Signed-off-by: Sean Paul <seanpaul@chromium.org>
> ---
>  arch/arm64/boot/dts/rockchip/rk3399-gru-kevin.dts | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/rockchip/rk3399-gru-kevin.dts b/arch/arm64/boot/dts/rockchip/rk3399-gru-kevin.dts
> index 191a6bcb1704..658411ce37ea 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3399-gru-kevin.dts
> +++ b/arch/arm64/boot/dts/rockchip/rk3399-gru-kevin.dts
> @@ -98,6 +98,20 @@
>                 backlight = <&backlight>;
>                 power-supply = <&pp3300_disp>;
>
> +               panel-timing {
> +                       clock-frequency = <266604720>;
> +                       hactive = <2400>;
> +                       hfront-porch = <48>;
> +                       hback-porch = <84>;
> +                       hsync-len = <32>;
> +                       hsync-active = <0>;
> +                       vactive = <1600>;
> +                       vfront-porch = <3>;
> +                       vback-porch = <120>;
> +                       vsync-len = <10>;
> +                       vsync-active = <0>;
> +               };
> +
>                 ports {
>                         panel_in_edp: endpoint {
>                                 remote-endpoint = <&edp_out_panel>;

Kristian brought an old bug to my attention
<https://bugs.chromium.org/p/chromium/issues/detail?id=750354> and it
made me think.  Should we somehow adjust the bindings here to account
for the fact that a board may source several different panels?

AKA: on some boards an ODM may want to second source (or third source,
or ...) the panel.  They'll randomly connect several different panels
to the board and ship the boards out.  The panels are all compatible
electrically (same power sequencing) but might need slightly different
timings.  In this particular case there's no board-level strappings
for the different panels--it's assumed that the EDID on the panels can
be used to distinguish them.

In that case it seems like it would be nice to allow specifying more
than one "panel-timing" nodes.  Maybe keyed off some type of ID that's
present in the EDID?


Is that something we'd want to account for before we land this series?
 It seems like it would just be adding an extra level of nodes?


-Doug

[PATCH v3 6/6] arm64: dts: rockchip: Specify override mode for kevin panel

[Ezequiel Garcia]

Hi Doug, Sean:

I would like to move this forward.

On 26 February 2018 at 15:23, Doug Anderson <dianders@chromium.org> wrote:
> Hi,
>
> On Thu, Feb 8, 2018 at 9:48 AM, Sean Paul <seanpaul@chromium.org> wrote:
>> This patch adds an override mode for kevin devices. The mode increases
>> both back porches to allow a pixel clock of 26666kHz as opposed to the
>> 'typical' value of 252750kHz. This is needed to avoid interference with
>> the touch digitizer on these laptops.
>>
>> Changes in v2:
>>  - Wrap the timing in display-timings node to match binding (Rob/Thierry)
>> Changes in v3:
>>  - Unwrap the timing from display-timings and rename panel-timing (Rob)
>>
>> Cc: Doug Anderson <dianders@chromium.org>
>> Cc: Eric Anholt <eric@anholt.net>
>> Cc: Heiko Stuebner <heiko@sntech.de>
>> Cc: Jeffy Chen <jeffy.chen@rock-chips.com>
>> Cc: Rob Herring <robh+dt@kernel.org>
>> Cc: St?phane Marchesin <marcheu@chromium.org>
>> Cc: Thierry Reding <thierry.reding@gmail.com>
>> Cc: devicetree at vger.kernel.org
>> Cc: dri-devel at lists.freedesktop.org
>> Cc: linux-rockchip at lists.infradead.org
>> Signed-off-by: Sean Paul <seanpaul@chromium.org>
>> ---
>>  arch/arm64/boot/dts/rockchip/rk3399-gru-kevin.dts | 14 ++++++++++++++
>>  1 file changed, 14 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/rockchip/rk3399-gru-kevin.dts b/arch/arm64/boot/dts/rockchip/rk3399-gru-kevin.dts
>> index 191a6bcb1704..658411ce37ea 100644
>> --- a/arch/arm64/boot/dts/rockchip/rk3399-gru-kevin.dts
>> +++ b/arch/arm64/boot/dts/rockchip/rk3399-gru-kevin.dts
>> @@ -98,6 +98,20 @@
>>                 backlight = <&backlight>;
>>                 power-supply = <&pp3300_disp>;
>>
>> +               panel-timing {
>> +                       clock-frequency = <266604720>;
>> +                       hactive = <2400>;
>> +                       hfront-porch = <48>;
>> +                       hback-porch = <84>;
>> +                       hsync-len = <32>;
>> +                       hsync-active = <0>;
>> +                       vactive = <1600>;
>> +                       vfront-porch = <3>;
>> +                       vback-porch = <120>;
>> +                       vsync-len = <10>;
>> +                       vsync-active = <0>;
>> +               };
>> +
>>                 ports {
>>                         panel_in_edp: endpoint {
>>                                 remote-endpoint = <&edp_out_panel>;
>
> Kristian brought an old bug to my attention
> <https://bugs.chromium.org/p/chromium/issues/detail?id=750354> and it
> made me think.  Should we somehow adjust the bindings here to account
> for the fact that a board may source several different panels?
>
> AKA: on some boards an ODM may want to second source (or third source,
> or ...) the panel.  They'll randomly connect several different panels
> to the board and ship the boards out.  The panels are all compatible
> electrically (same power sequencing) but might need slightly different
> timings.  In this particular case there's no board-level strappings
> for the different panels--it's assumed that the EDID on the panels can
> be used to distinguish them.
>
> In that case it seems like it would be nice to allow specifying more
> than one "panel-timing" nodes.  Maybe keyed off some type of ID that's
> present in the EDID?
>
>
> Is that something we'd want to account for before we land this series?
>  It seems like it would just be adding an extra level of nodes?
>

AFAICS, there is no EDID without a DDC bus, which we don't
seem to have on gru platforms, do we?

Regards,
-- 
Ezequiel Garc?a, VanguardiaSur
www.vanguardiasur.com.ar

[PATCH v3 6/6] arm64: dts: rockchip: Specify override mode for kevin panel

[Stéphane Marchesin]

On Tue, Apr 24, 2018 at 7:31 AM, Ezequiel Garcia
<ezequiel@vanguardiasur.com.ar> wrote:
> Hi Doug, Sean:
>
> I would like to move this forward.
>
> On 26 February 2018 at 15:23, Doug Anderson <dianders@chromium.org> wrote:
>> Hi,
>>
>> On Thu, Feb 8, 2018 at 9:48 AM, Sean Paul <seanpaul@chromium.org> wrote:
>>> This patch adds an override mode for kevin devices. The mode increases
>>> both back porches to allow a pixel clock of 26666kHz as opposed to the
>>> 'typical' value of 252750kHz. This is needed to avoid interference with
>>> the touch digitizer on these laptops.
>>>
>>> Changes in v2:
>>>  - Wrap the timing in display-timings node to match binding (Rob/Thierry)
>>> Changes in v3:
>>>  - Unwrap the timing from display-timings and rename panel-timing (Rob)
>>>
>>> Cc: Doug Anderson <dianders@chromium.org>
>>> Cc: Eric Anholt <eric@anholt.net>
>>> Cc: Heiko Stuebner <heiko@sntech.de>
>>> Cc: Jeffy Chen <jeffy.chen@rock-chips.com>
>>> Cc: Rob Herring <robh+dt@kernel.org>
>>> Cc: St?phane Marchesin <marcheu@chromium.org>
>>> Cc: Thierry Reding <thierry.reding@gmail.com>
>>> Cc: devicetree at vger.kernel.org
>>> Cc: dri-devel at lists.freedesktop.org
>>> Cc: linux-rockchip at lists.infradead.org
>>> Signed-off-by: Sean Paul <seanpaul@chromium.org>
>>> ---
>>>  arch/arm64/boot/dts/rockchip/rk3399-gru-kevin.dts | 14 ++++++++++++++
>>>  1 file changed, 14 insertions(+)
>>>
>>> diff --git a/arch/arm64/boot/dts/rockchip/rk3399-gru-kevin.dts b/arch/arm64/boot/dts/rockchip/rk3399-gru-kevin.dts
>>> index 191a6bcb1704..658411ce37ea 100644
>>> --- a/arch/arm64/boot/dts/rockchip/rk3399-gru-kevin.dts
>>> +++ b/arch/arm64/boot/dts/rockchip/rk3399-gru-kevin.dts
>>> @@ -98,6 +98,20 @@
>>>                 backlight = <&backlight>;
>>>                 power-supply = <&pp3300_disp>;
>>>
>>> +               panel-timing {
>>> +                       clock-frequency = <266604720>;
>>> +                       hactive = <2400>;
>>> +                       hfront-porch = <48>;
>>> +                       hback-porch = <84>;
>>> +                       hsync-len = <32>;
>>> +                       hsync-active = <0>;
>>> +                       vactive = <1600>;
>>> +                       vfront-porch = <3>;
>>> +                       vback-porch = <120>;
>>> +                       vsync-len = <10>;
>>> +                       vsync-active = <0>;
>>> +               };
>>> +
>>>                 ports {
>>>                         panel_in_edp: endpoint {
>>>                                 remote-endpoint = <&edp_out_panel>;
>>
>> Kristian brought an old bug to my attention
>> <https://bugs.chromium.org/p/chromium/issues/detail?id=750354> and it
>> made me think.  Should we somehow adjust the bindings here to account
>> for the fact that a board may source several different panels?
>>
>> AKA: on some boards an ODM may want to second source (or third source,
>> or ...) the panel.  They'll randomly connect several different panels
>> to the board and ship the boards out.  The panels are all compatible
>> electrically (same power sequencing) but might need slightly different
>> timings.  In this particular case there's no board-level strappings
>> for the different panels--it's assumed that the EDID on the panels can
>> be used to distinguish them.
>>
>> In that case it seems like it would be nice to allow specifying more
>> than one "panel-timing" nodes.  Maybe keyed off some type of ID that's
>> present in the EDID?
>>
>>
>> Is that something we'd want to account for before we land this series?
>>  It seems like it would just be adding an extra level of nodes?
>>
>
> AFAICS, there is no EDID without a DDC bus, which we don't
> seem to have on gru platforms, do we?

IIRC historically we've only done second sourcing when
doable because either:
1. the timings between all the panels we use are compatible, or
2. we have a working DDC to figure it out for us.

In other words, we haven't handled the case where timings are not
compatible and we can't address this by reading EDIDs.

St?phane

>
> Regards,
> --
> Ezequiel Garc?a, VanguardiaSur
> www.vanguardiasur.com.ar

[PATCH v3 6/6] arm64: dts: rockchip: Specify override mode for kevin panel

[Doug Anderson]

Hi,

On Tue, Apr 24, 2018 at 7:31 AM, Ezequiel Garcia
<ezequiel@vanguardiasur.com.ar> wrote:
> Hi Doug, Sean:
>
> I would like to move this forward.
>
> On 26 February 2018 at 15:23, Doug Anderson <dianders@chromium.org> wrote:
>> Hi,
>>
>> On Thu, Feb 8, 2018 at 9:48 AM, Sean Paul <seanpaul@chromium.org> wrote:
>>> This patch adds an override mode for kevin devices. The mode increases
>>> both back porches to allow a pixel clock of 26666kHz as opposed to the
>>> 'typical' value of 252750kHz. This is needed to avoid interference with
>>> the touch digitizer on these laptops.
>>>
>>> Changes in v2:
>>>  - Wrap the timing in display-timings node to match binding (Rob/Thierry)
>>> Changes in v3:
>>>  - Unwrap the timing from display-timings and rename panel-timing (Rob)
>>>
>>> Cc: Doug Anderson <dianders@chromium.org>
>>> Cc: Eric Anholt <eric@anholt.net>
>>> Cc: Heiko Stuebner <heiko@sntech.de>
>>> Cc: Jeffy Chen <jeffy.chen@rock-chips.com>
>>> Cc: Rob Herring <robh+dt@kernel.org>
>>> Cc: St?phane Marchesin <marcheu@chromium.org>
>>> Cc: Thierry Reding <thierry.reding@gmail.com>
>>> Cc: devicetree at vger.kernel.org
>>> Cc: dri-devel at lists.freedesktop.org
>>> Cc: linux-rockchip at lists.infradead.org
>>> Signed-off-by: Sean Paul <seanpaul@chromium.org>
>>> ---
>>>  arch/arm64/boot/dts/rockchip/rk3399-gru-kevin.dts | 14 ++++++++++++++
>>>  1 file changed, 14 insertions(+)
>>>
>>> diff --git a/arch/arm64/boot/dts/rockchip/rk3399-gru-kevin.dts b/arch/arm64/boot/dts/rockchip/rk3399-gru-kevin.dts
>>> index 191a6bcb1704..658411ce37ea 100644
>>> --- a/arch/arm64/boot/dts/rockchip/rk3399-gru-kevin.dts
>>> +++ b/arch/arm64/boot/dts/rockchip/rk3399-gru-kevin.dts
>>> @@ -98,6 +98,20 @@
>>>                 backlight = <&backlight>;
>>>                 power-supply = <&pp3300_disp>;
>>>
>>> +               panel-timing {
>>> +                       clock-frequency = <266604720>;
>>> +                       hactive = <2400>;
>>> +                       hfront-porch = <48>;
>>> +                       hback-porch = <84>;
>>> +                       hsync-len = <32>;
>>> +                       hsync-active = <0>;
>>> +                       vactive = <1600>;
>>> +                       vfront-porch = <3>;
>>> +                       vback-porch = <120>;
>>> +                       vsync-len = <10>;
>>> +                       vsync-active = <0>;
>>> +               };
>>> +
>>>                 ports {
>>>                         panel_in_edp: endpoint {
>>>                                 remote-endpoint = <&edp_out_panel>;
>>
>> Kristian brought an old bug to my attention
>> <https://bugs.chromium.org/p/chromium/issues/detail?id=750354> and it
>> made me think.  Should we somehow adjust the bindings here to account
>> for the fact that a board may source several different panels?
>>
>> AKA: on some boards an ODM may want to second source (or third source,
>> or ...) the panel.  They'll randomly connect several different panels
>> to the board and ship the boards out.  The panels are all compatible
>> electrically (same power sequencing) but might need slightly different
>> timings.  In this particular case there's no board-level strappings
>> for the different panels--it's assumed that the EDID on the panels can
>> be used to distinguish them.
>>
>> In that case it seems like it would be nice to allow specifying more
>> than one "panel-timing" nodes.  Maybe keyed off some type of ID that's
>> present in the EDID?
>>
>>
>> Is that something we'd want to account for before we land this series?
>>  It seems like it would just be adding an extra level of nodes?
>>
>
> AFAICS, there is no EDID without a DDC bus, which we don't
> seem to have on gru platforms, do we?

I'm fairly certain we can read the EDID on gru devices.  I see the
"aux" channel connected on the schematics.  That'll do it, right?
...and I could have sworn that at some point in time I could read the
EDID in the software.  My kevin devices are not connected for
debugging so I can't check right now, but I'm pretty sure I was able
to read the EDID in the past...

-Doug

[PATCH v3 6/6] arm64: dts: rockchip: Specify override mode for kevin panel

[Ezequiel Garcia]

On 25 April 2018 at 01:29, Doug Anderson <dianders@chromium.org> wrote:
> Hi,
>
> On Tue, Apr 24, 2018 at 7:31 AM, Ezequiel Garcia
> <ezequiel@vanguardiasur.com.ar> wrote:
>> Hi Doug, Sean:
>>
>> I would like to move this forward.
>>
>> On 26 February 2018 at 15:23, Doug Anderson <dianders@chromium.org> wrote:
>>> Hi,
>>>
>>> On Thu, Feb 8, 2018 at 9:48 AM, Sean Paul <seanpaul@chromium.org> wrote:
>>>> This patch adds an override mode for kevin devices. The mode increases
>>>> both back porches to allow a pixel clock of 26666kHz as opposed to the
>>>> 'typical' value of 252750kHz. This is needed to avoid interference with
>>>> the touch digitizer on these laptops.
>>>>
>>>> Changes in v2:
>>>>  - Wrap the timing in display-timings node to match binding (Rob/Thierry)
>>>> Changes in v3:
>>>>  - Unwrap the timing from display-timings and rename panel-timing (Rob)
>>>>
>>>> Cc: Doug Anderson <dianders@chromium.org>
>>>> Cc: Eric Anholt <eric@anholt.net>
>>>> Cc: Heiko Stuebner <heiko@sntech.de>
>>>> Cc: Jeffy Chen <jeffy.chen@rock-chips.com>
>>>> Cc: Rob Herring <robh+dt@kernel.org>
>>>> Cc: St?phane Marchesin <marcheu@chromium.org>
>>>> Cc: Thierry Reding <thierry.reding@gmail.com>
>>>> Cc: devicetree at vger.kernel.org
>>>> Cc: dri-devel at lists.freedesktop.org
>>>> Cc: linux-rockchip at lists.infradead.org
>>>> Signed-off-by: Sean Paul <seanpaul@chromium.org>
>>>> ---
>>>>  arch/arm64/boot/dts/rockchip/rk3399-gru-kevin.dts | 14 ++++++++++++++
>>>>  1 file changed, 14 insertions(+)
>>>>
>>>> diff --git a/arch/arm64/boot/dts/rockchip/rk3399-gru-kevin.dts b/arch/arm64/boot/dts/rockchip/rk3399-gru-kevin.dts
>>>> index 191a6bcb1704..658411ce37ea 100644
>>>> --- a/arch/arm64/boot/dts/rockchip/rk3399-gru-kevin.dts
>>>> +++ b/arch/arm64/boot/dts/rockchip/rk3399-gru-kevin.dts
>>>> @@ -98,6 +98,20 @@
>>>>                 backlight = <&backlight>;
>>>>                 power-supply = <&pp3300_disp>;
>>>>
>>>> +               panel-timing {
>>>> +                       clock-frequency = <266604720>;
>>>> +                       hactive = <2400>;
>>>> +                       hfront-porch = <48>;
>>>> +                       hback-porch = <84>;
>>>> +                       hsync-len = <32>;
>>>> +                       hsync-active = <0>;
>>>> +                       vactive = <1600>;
>>>> +                       vfront-porch = <3>;
>>>> +                       vback-porch = <120>;
>>>> +                       vsync-len = <10>;
>>>> +                       vsync-active = <0>;
>>>> +               };
>>>> +
>>>>                 ports {
>>>>                         panel_in_edp: endpoint {
>>>>                                 remote-endpoint = <&edp_out_panel>;
>>>
>>> Kristian brought an old bug to my attention
>>> <https://bugs.chromium.org/p/chromium/issues/detail?id=750354> and it
>>> made me think.  Should we somehow adjust the bindings here to account
>>> for the fact that a board may source several different panels?
>>>
>>> AKA: on some boards an ODM may want to second source (or third source,
>>> or ...) the panel.  They'll randomly connect several different panels
>>> to the board and ship the boards out.  The panels are all compatible
>>> electrically (same power sequencing) but might need slightly different
>>> timings.  In this particular case there's no board-level strappings
>>> for the different panels--it's assumed that the EDID on the panels can
>>> be used to distinguish them.
>>>
>>> In that case it seems like it would be nice to allow specifying more
>>> than one "panel-timing" nodes.  Maybe keyed off some type of ID that's
>>> present in the EDID?
>>>
>>>
>>> Is that something we'd want to account for before we land this series?
>>>  It seems like it would just be adding an extra level of nodes?
>>>
>>
>> AFAICS, there is no EDID without a DDC bus, which we don't
>> seem to have on gru platforms, do we?
>
> I'm fairly certain we can read the EDID on gru devices.  I see the
> "aux" channel connected on the schematics.  That'll do it, right?
> ...and I could have sworn that at some point in time I could read the
> EDID in the software.  My kevin devices are not connected for
> debugging so I can't check right now, but I'm pretty sure I was able
> to read the EDID in the past...
>

Right.

Well, I've been trying to get some EDID without much success.
Perhaps I am missing something? The I2C aux bus seems empty:

root at linaro-developer:/sys/class/i2c-adapter/i2c-10# cat name
DP-AUX
root at linaro-developer:/sys/class/i2c-adapter/i2c-10# i2cdetect -y 10
     0  1  2  3  4  5  6  7  8  9  a  b  c  d  e  f
00:          -- -- -- -- -- -- -- -- -- -- -- -- --
10: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- --
20: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- --
30: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- --
40: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- --
50: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- --
60: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- --
70: -- -- -- -- -- -- -- --

In any case, perhaps it's better to not stall this series
upstream - and work on second sourcing as follow up
patches?

Thanks,
-- 
Ezequiel Garc?a, VanguardiaSur
www.vanguardiasur.com.ar

[PATCH v3 6/6] arm64: dts: rockchip: Specify override mode for kevin panel

[Thierry Reding]

On Mon, Feb 26, 2018 at 10:23:00AM -0800, Doug Anderson wrote:
> Hi,
> 
> On Thu, Feb 8, 2018 at 9:48 AM, Sean Paul <seanpaul@chromium.org> wrote:
> > This patch adds an override mode for kevin devices. The mode increases
> > both back porches to allow a pixel clock of 26666kHz as opposed to the
> > 'typical' value of 252750kHz. This is needed to avoid interference with
> > the touch digitizer on these laptops.
> >
> > Changes in v2:
> >  - Wrap the timing in display-timings node to match binding (Rob/Thierry)
> > Changes in v3:
> >  - Unwrap the timing from display-timings and rename panel-timing (Rob)
> >
> > Cc: Doug Anderson <dianders@chromium.org>
> > Cc: Eric Anholt <eric@anholt.net>
> > Cc: Heiko Stuebner <heiko@sntech.de>
> > Cc: Jeffy Chen <jeffy.chen@rock-chips.com>
> > Cc: Rob Herring <robh+dt@kernel.org>
> > Cc: St?phane Marchesin <marcheu@chromium.org>
> > Cc: Thierry Reding <thierry.reding@gmail.com>
> > Cc: devicetree at vger.kernel.org
> > Cc: dri-devel at lists.freedesktop.org
> > Cc: linux-rockchip at lists.infradead.org
> > Signed-off-by: Sean Paul <seanpaul@chromium.org>
> > ---
> >  arch/arm64/boot/dts/rockchip/rk3399-gru-kevin.dts | 14 ++++++++++++++
> >  1 file changed, 14 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/rockchip/rk3399-gru-kevin.dts b/arch/arm64/boot/dts/rockchip/rk3399-gru-kevin.dts
> > index 191a6bcb1704..658411ce37ea 100644
> > --- a/arch/arm64/boot/dts/rockchip/rk3399-gru-kevin.dts
> > +++ b/arch/arm64/boot/dts/rockchip/rk3399-gru-kevin.dts
> > @@ -98,6 +98,20 @@
> >                 backlight = <&backlight>;
> >                 power-supply = <&pp3300_disp>;
> >
> > +               panel-timing {
> > +                       clock-frequency = <266604720>;
> > +                       hactive = <2400>;
> > +                       hfront-porch = <48>;
> > +                       hback-porch = <84>;
> > +                       hsync-len = <32>;
> > +                       hsync-active = <0>;
> > +                       vactive = <1600>;
> > +                       vfront-porch = <3>;
> > +                       vback-porch = <120>;
> > +                       vsync-len = <10>;
> > +                       vsync-active = <0>;
> > +               };
> > +
> >                 ports {
> >                         panel_in_edp: endpoint {
> >                                 remote-endpoint = <&edp_out_panel>;
> 
> Kristian brought an old bug to my attention
> <https://bugs.chromium.org/p/chromium/issues/detail?id=750354> and it
> made me think.  Should we somehow adjust the bindings here to account
> for the fact that a board may source several different panels?
> 
> AKA: on some boards an ODM may want to second source (or third source,
> or ...) the panel.  They'll randomly connect several different panels
> to the board and ship the boards out.  The panels are all compatible
> electrically (same power sequencing) but might need slightly different
> timings.  In this particular case there's no board-level strappings
> for the different panels--it's assumed that the EDID on the panels can
> be used to distinguish them.
> 
> In that case it seems like it would be nice to allow specifying more
> than one "panel-timing" nodes.  Maybe keyed off some type of ID that's
> present in the EDID?

If you've got an EDID you should be relying on the EDID to provide the
timings. No need to have any timings in the DT in that case.

Thierry
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180426/439f5bec/attachment.sig>

[PATCH v3 6/6] arm64: dts: rockchip: Specify override mode for kevin panel

[Doug Anderson]

Hi,

On Thu, Apr 26, 2018 at 5:05 AM, Thierry Reding
<thierry.reding@gmail.com> wrote:
> If you've got an EDID you should be relying on the EDID to provide the
> timings. No need to have any timings in the DT in that case.

The problem that's specifically trying to be solved by Sean's series
is when we have to use timings other than the one suggested by the
EDID.  Specifically:

* On many Rockchip SoCs there is only one "extra" PLL available.  This
extra PLL can only be used by one of the two displays (eDP for
internal panel or HDMI/DP for external display).  The other display
has to use one of the "shared" PLLs in the system.  These PLLs have
their rate set at boot and aren't changed.

* In order to provide maximum flexibility to connect external
displays, we always want the "extra" PLL dedicated to the external
display port.  Then we can make lots of pixel clocks.

* We work with device and display manufacturers to figure out a pixel
clock that is achievable with the shared PLLs and that has valid
timings.  This is the the pixel clock that was used when testing EMI,
etc.  It is the one that should be used.

* Panel manufacturers agreed that this pixel clock was good to use,
but we didn't get an updated EDID that suggested this mode.  It's my
understanding that panels were already available and it didn't really
make sense to program in an EDID to work around a certain board.



-Doug