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

Message ID 20180208174855.55620-7-seanpaul@chromium.org
State New
Headers show
Series
  • Untitled series #27630
Related show

Commit Message

Sean Paul Feb. 8, 2018, 5:48 p.m.
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@vger.kernel.org
Cc: dri-devel@lists.freedesktop.org
Cc: linux-rockchip@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(+)

Comments

Enric Balletbo Serra Feb. 19, 2018, 2:34 p.m. | #1
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@vger.kernel.org
> Cc: dri-devel@lists.freedesktop.org
> Cc: linux-rockchip@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@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
Doug Anderson Feb. 26, 2018, 6:23 p.m. | #2
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@vger.kernel.org
> Cc: dri-devel@lists.freedesktop.org
> Cc: linux-rockchip@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
Ezequiel Garcia April 24, 2018, 2:31 p.m. | #3
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@vger.kernel.org
>> Cc: dri-devel@lists.freedesktop.org
>> Cc: linux-rockchip@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,
Stéphane Marchesin April 24, 2018, 11:02 p.m. | #4
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@vger.kernel.org
>>> Cc: dri-devel@lists.freedesktop.org
>>> Cc: linux-rockchip@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
Doug Anderson April 25, 2018, 4:29 a.m. | #5
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@vger.kernel.org
>>> Cc: dri-devel@lists.freedesktop.org
>>> Cc: linux-rockchip@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
Ezequiel Garcia April 25, 2018, 12:36 p.m. | #6
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@vger.kernel.org
>>>> Cc: dri-devel@lists.freedesktop.org
>>>> Cc: linux-rockchip@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@linaro-developer:/sys/class/i2c-adapter/i2c-10# cat name
DP-AUX
root@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,
Thierry Reding April 26, 2018, 12:05 p.m. | #7
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@vger.kernel.org
> > Cc: dri-devel@lists.freedesktop.org
> > Cc: linux-rockchip@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
Doug Anderson April 26, 2018, 3:29 p.m. | #8
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

Patch

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