diff mbox series

[v5,6/7] ARM: dts: rockchip: Specify rk3288-veyron-chromebook's display timings

Message ID 20190401171724.215780-7-dianders@chromium.org (mailing list archive)
State New, archived
Headers show
Series drm/panel: simple: Add mode support to devicetree | expand

Commit Message

Doug Anderson April 1, 2019, 5:17 p.m. UTC
Let's document the display timings that most veyron chromebooks (like
jaq, jerry, mighty, speedy) have been using out in the field.  This
uses the standard blankings but a slightly slower clock rate, thus
getting a refresh rate 58.3 Hz.

NOTE: this won't really do anything except cause DRM to properly
report the refresh rate since vop_crtc_mode_fixup() was rounding the
pixel clock to 74.25 MHz anyway.  Apparently the adjusted rate isn't
exposed to userspace so it's important that the rate we're trying to
achieve is mostly right.

For the downstream kernel change related to this see See
https://crrev.com/c/324558.

NOTE: minnie uses a different panel will be fixed up in a future
patch, so for now we'll just delete the panel timings there.

Signed-off-by: Douglas Anderson <dianders@chromium.org>
---

Changes in v5:
- It's not just jerry, it's most rk3288 Chromebooks (Heiko)

Changes in v4:
- rk3288-veyron-jerry patch new for v4.

Changes in v3: None
Changes in v2: None

 arch/arm/boot/dts/rk3288-veyron-chromebook.dtsi | 14 ++++++++++++++
 arch/arm/boot/dts/rk3288-veyron-minnie.dts      |  2 ++
 2 files changed, 16 insertions(+)

Comments

Urja Rannikko April 7, 2019, 1:15 a.m. UTC | #1
Hi,

On Mon, Apr 1, 2019 at 5:18 PM Douglas Anderson <dianders@chromium.org> wrote:
>
> Let's document the display timings that most veyron chromebooks (like
> jaq, jerry, mighty, speedy) have been using out in the field.  This
> uses the standard blankings but a slightly slower clock rate, thus
> getting a refresh rate 58.3 Hz.
>
> NOTE: this won't really do anything except cause DRM to properly
> report the refresh rate since vop_crtc_mode_fixup() was rounding the
> pixel clock to 74.25 MHz anyway.  Apparently the adjusted rate isn't
> exposed to userspace so it's important that the rate we're trying to
> achieve is mostly right.

I just thought it would be worth mentioning that I have picked &
tested a close to 60Hz mode on two veyron speedys of mine, but thought
it too much effort to try and upstream, especially as it was done as a
change to the actual panel info:
https://github.com/urjaman/linux/commit/23d46278911e18df138b7adde1bebc23f606baae

The difference would be in this format just setting hfront-porch = 87
and vback-porch = 14.
Anyways the point is: I support moving this mode info into the dts,
and I'd like to know how if at all should i go about getting a
60Hz(ish) mode upstreamed?
Doug Anderson April 8, 2019, 3:21 p.m. UTC | #2
Hi,

On Sat, Apr 6, 2019 at 6:16 PM Urja Rannikko <urjaman@gmail.com> wrote:
>
> Hi,
>
> On Mon, Apr 1, 2019 at 5:18 PM Douglas Anderson <dianders@chromium.org> wrote:
> >
> > Let's document the display timings that most veyron chromebooks (like
> > jaq, jerry, mighty, speedy) have been using out in the field.  This
> > uses the standard blankings but a slightly slower clock rate, thus
> > getting a refresh rate 58.3 Hz.
> >
> > NOTE: this won't really do anything except cause DRM to properly
> > report the refresh rate since vop_crtc_mode_fixup() was rounding the
> > pixel clock to 74.25 MHz anyway.  Apparently the adjusted rate isn't
> > exposed to userspace so it's important that the rate we're trying to
> > achieve is mostly right.
>
> I just thought it would be worth mentioning that I have picked &
> tested a close to 60Hz mode on two veyron speedys of mine, but thought
> it too much effort to try and upstream, especially as it was done as a
> change to the actual panel info:
> https://github.com/urjaman/linux/commit/23d46278911e18df138b7adde1bebc23f606baae
>
> The difference would be in this format just setting hfront-porch = 87
> and vback-porch = 14.
> Anyways the point is: I support moving this mode info into the dts,
> and I'd like to know how if at all should i go about getting a
> 60Hz(ish) mode upstreamed?

I'm a bit torn here.  I like the idea of actually getting 60 Hz and
you also increase the vblank time by a little bit (which means that if
anyone ever gets DDRFreq upstream it will work better).  ...but I'm
also slightly nervous changing something like this without a really
good motivation.  As you said in your commit message the pixels clocks
claimed by the spec don't actually all work and thus, to some extent,
we can only rely on trial-and-error here.  While your new mode works
well on your device (and you wisely gave it a bit of margin), it is
_possible_ that there could be devices out there where it doesn't work
(especially across various temperature extremes).  All devices were
tested in the factory with the old timings and presumably have been
running fine for years like that...

I will certainly admit that it's unlikely devices would be affected,
but at the same time I'd want to know how much of a difference going
from 58.3 Hz to 60 Hz made for you.  Could you actually notice any
visible difference, or was it just nice to be at 60 Hz?


-Doug
Urja Rannikko April 8, 2019, 4:26 p.m. UTC | #3
Hi,

On Mon, Apr 8, 2019 at 3:21 PM Doug Anderson <dianders@chromium.org> wrote:
>
> Hi,
>
> On Sat, Apr 6, 2019 at 6:16 PM Urja Rannikko <urjaman@gmail.com> wrote:
> >
> > Hi,
<snip>
> >
> > The difference would be in this format just setting hfront-porch = 87
> > and vback-porch = 14.
> > Anyways the point is: I support moving this mode info into the dts,
> > and I'd like to know how if at all should i go about getting a
> > 60Hz(ish) mode upstreamed?
>
> I'm a bit torn here.  I like the idea of actually getting 60 Hz and
> you also increase the vblank time by a little bit (which means that if
> anyone ever gets DDRFreq upstream it will work better).  ...but I'm
> also slightly nervous changing something like this without a really
> good motivation.  As you said in your commit message the pixels clocks
> claimed by the spec don't actually all work and thus, to some extent,
> we can only rely on trial-and-error here.  While your new mode works
> well on your device (and you wisely gave it a bit of margin), it is
> _possible_ that there could be devices out there where it doesn't work
> (especially across various temperature extremes).  All devices were
> tested in the factory with the old timings and presumably have been
> running fine for years like that...
Re: the trial and error: it might be the case that the panels actually
work at 1506 vtotal if you also adjust the sync length and/or back
porch "accordingly", whatever that accordingly would be for this panel
since the datasheet doesnt tell. I missed this point when i was doing
my testing and just adjusted the variables with the most
"adjustability" (bigger starting value) to them.

> I will certainly admit that it's unlikely devices would be affected,
> but at the same time I'd want to know how much of a difference going
> from 58.3 Hz to 60 Hz made for you.  Could you actually notice any
> visible difference, or was it just nice to be at 60 Hz?
Honestly I was just doing this because i really liked the idea of
actually making it 60Hz and my eyes arent that good at noticing
high-fps things - i think the one case where it might be visible to a
keen eye is 720p60 playback where you'd need to skip "about 2" (1.7..)
frames a second if running at 58.3 Hz. But currently the C201 isnt
doing a lot of that given i dont think i/we have a good software setup
for it. That could be changing in the future with panfrost and the VPU
hardware decoder support, etc.

Anyways I'm thinking it would be prudent to first get this framework
of device-tree modes in and then maybe adjust the mode later.

Testing the 60Hz mode is simple enough:
xrandr --newmode 1366x768p60 74.25 1366 1453 1483 1543 768 776 790 802
-HSync -VSync
xrandr --addmode eDP-1 1366x768p60
xrandr --output eDP-1 --mode 1366x768p60

(The mode name can be anything...)
So meanwhile I would appreciate it if we could get some test reports
of the mode with other veyron chromebooks that have this panel :)
Doug Anderson April 13, 2019, 12:07 a.m. UTC | #4
Hi,

On Mon, Apr 8, 2019 at 9:26 AM Urja Rannikko <urjaman@gmail.com> wrote:
>
> Hi,
>
> On Mon, Apr 8, 2019 at 3:21 PM Doug Anderson <dianders@chromium.org> wrote:
> >
> > Hi,
> >
> > On Sat, Apr 6, 2019 at 6:16 PM Urja Rannikko <urjaman@gmail.com> wrote:
> > >
> > > Hi,
> <snip>
> > >
> > > The difference would be in this format just setting hfront-porch = 87
> > > and vback-porch = 14.
> > > Anyways the point is: I support moving this mode info into the dts,
> > > and I'd like to know how if at all should i go about getting a
> > > 60Hz(ish) mode upstreamed?
> >
> > I'm a bit torn here.  I like the idea of actually getting 60 Hz and
> > you also increase the vblank time by a little bit (which means that if
> > anyone ever gets DDRFreq upstream it will work better).  ...but I'm
> > also slightly nervous changing something like this without a really
> > good motivation.  As you said in your commit message the pixels clocks
> > claimed by the spec don't actually all work and thus, to some extent,
> > we can only rely on trial-and-error here.  While your new mode works
> > well on your device (and you wisely gave it a bit of margin), it is
> > _possible_ that there could be devices out there where it doesn't work
> > (especially across various temperature extremes).  All devices were
> > tested in the factory with the old timings and presumably have been
> > running fine for years like that...
> Re: the trial and error: it might be the case that the panels actually
> work at 1506 vtotal if you also adjust the sync length and/or back
> porch "accordingly", whatever that accordingly would be for this panel
> since the datasheet doesnt tell. I missed this point when i was doing
> my testing and just adjusted the variables with the most
> "adjustability" (bigger starting value) to them.

Fair enough.  I guess I just have my gut instincts that say that this
will break some device somewhere, but it's certainly possible that's
wrong.


> > I will certainly admit that it's unlikely devices would be affected,
> > but at the same time I'd want to know how much of a difference going
> > from 58.3 Hz to 60 Hz made for you.  Could you actually notice any
> > visible difference, or was it just nice to be at 60 Hz?
> Honestly I was just doing this because i really liked the idea of
> actually making it 60Hz and my eyes arent that good at noticing
> high-fps things - i think the one case where it might be visible to a
> keen eye is 720p60 playback where you'd need to skip "about 2" (1.7..)
> frames a second if running at 58.3 Hz. But currently the C201 isnt
> doing a lot of that given i dont think i/we have a good software setup
> for it. That could be changing in the future with panfrost and the VPU
> hardware decoder support, etc.

Yeah, I'd definitely wonder how easy this is to notice.  Since I
haven't ever heard of anyone who thought that the current 58.3 Hz was
causing problems that they could actually notice it makes me hesitant
to change it.


> Anyways I'm thinking it would be prudent to first get this framework
> of device-tree modes in and then maybe adjust the mode later.

If you're OK with it, let's aim to land things with the current 58.3
and then we can do think about the 60 Hz mode.

...I'm probably sounding super paranoid here since (presumably) anyone
who is running upstream Linux on their Chromebook is more than capable
of bisecting problems like this if it causes problems, but I guess I'm
worried that this could eventually make its way into a Chrome OS tree
(either through a kernel uprev or simple cherry-picks) and affect
people.


> Testing the 60Hz mode is simple enough:
> xrandr --newmode 1366x768p60 74.25 1366 1453 1483 1543 768 776 790 802
> -HSync -VSync
> xrandr --addmode eDP-1 1366x768p60
> xrandr --output eDP-1 --mode 1366x768p60
>
> (The mode name can be anything...)
> So meanwhile I would appreciate it if we could get some test reports
> of the mode with other veyron chromebooks that have this panel :)

I still haven't done this and I'm not in front of my Chromebook now.
I'll try to give it a try soon.  The kinds of problems I'd expect (if
there are any) would be flickers that happen _very_ rarely though, but
I can at least do some basic testing.

One other note to justify my paranoia: on a past Chromebook we
certainly had crazy flickering problems.  On one device one day you'd
get two flickers and then see nothing for the next two weeks.  ...some
devices would see at least a flicker a day and some would see no
flickers ever.  Obviously it's unlikely that would happen here, but
such past experience just makes me paranoid.  ;-)


-Doug
Heiko Stuebner July 11, 2019, 9:27 p.m. UTC | #5
Am Montag, 1. April 2019, 19:17:23 CEST schrieb Douglas Anderson:
> Let's document the display timings that most veyron chromebooks (like
> jaq, jerry, mighty, speedy) have been using out in the field.  This
> uses the standard blankings but a slightly slower clock rate, thus
> getting a refresh rate 58.3 Hz.
> 
> NOTE: this won't really do anything except cause DRM to properly
> report the refresh rate since vop_crtc_mode_fixup() was rounding the
> pixel clock to 74.25 MHz anyway.  Apparently the adjusted rate isn't
> exposed to userspace so it's important that the rate we're trying to
> achieve is mostly right.
> 
> For the downstream kernel change related to this see See
> https://crrev.com/c/324558.
> 
> NOTE: minnie uses a different panel will be fixed up in a future
> patch, so for now we'll just delete the panel timings there.
> 
> Signed-off-by: Douglas Anderson <dianders@chromium.org>

applied for 5.3

Thanks
Heiko
Heiko Stuebner July 11, 2019, 9:52 p.m. UTC | #6
Am Donnerstag, 11. Juli 2019, 23:27:44 CEST schrieb Heiko Stübner:
> Am Montag, 1. April 2019, 19:17:23 CEST schrieb Douglas Anderson:
> > Let's document the display timings that most veyron chromebooks (like
> > jaq, jerry, mighty, speedy) have been using out in the field.  This
> > uses the standard blankings but a slightly slower clock rate, thus
> > getting a refresh rate 58.3 Hz.
> > 
> > NOTE: this won't really do anything except cause DRM to properly
> > report the refresh rate since vop_crtc_mode_fixup() was rounding the
> > pixel clock to 74.25 MHz anyway.  Apparently the adjusted rate isn't
> > exposed to userspace so it's important that the rate we're trying to
> > achieve is mostly right.
> > 
> > For the downstream kernel change related to this see See
> > https://crrev.com/c/324558.
> > 
> > NOTE: minnie uses a different panel will be fixed up in a future
> > patch, so for now we'll just delete the panel timings there.
> > 
> > Signed-off-by: Douglas Anderson <dianders@chromium.org>
> 
> applied for 5.3

5.4 obviously
[just to not confuse people reading that later]
diff mbox series

Patch

diff --git a/arch/arm/boot/dts/rk3288-veyron-chromebook.dtsi b/arch/arm/boot/dts/rk3288-veyron-chromebook.dtsi
index b54746df3661..0b1789b50c21 100644
--- a/arch/arm/boot/dts/rk3288-veyron-chromebook.dtsi
+++ b/arch/arm/boot/dts/rk3288-veyron-chromebook.dtsi
@@ -76,6 +76,20 @@ 
 		power-supply = <&vcc33_lcd>;
 		backlight = <&backlight>;
 
+		panel-timing {
+			clock-frequency = <74250000>;
+			hactive = <1366>;
+			hfront-porch = <136>;
+			hback-porch = <60>;
+			hsync-len = <30>;
+			hsync-active = <0>;
+			vactive = <768>;
+			vfront-porch = <8>;
+			vback-porch = <12>;
+			vsync-len = <12>;
+			vsync-active = <0>;
+		};
+
 		ports {
 			panel_in: port {
 				panel_in_edp: endpoint {
diff --git a/arch/arm/boot/dts/rk3288-veyron-minnie.dts b/arch/arm/boot/dts/rk3288-veyron-minnie.dts
index f95d0c5fcf71..ca7512ade222 100644
--- a/arch/arm/boot/dts/rk3288-veyron-minnie.dts
+++ b/arch/arm/boot/dts/rk3288-veyron-minnie.dts
@@ -142,6 +142,8 @@ 
 &panel {
 	compatible = "auo,b101ean01", "simple-panel";
 	power-supply= <&panel_regulator>;
+
+	/delete-node/ panel-timing;
 };
 
 &rk808 {