Re: [PATCH 04/12] drm: Nuke mode->vrefresh

From: Linus Walleij <linus.walleij@linaro.org>
To: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
Cc: "Josh Wu" <josh.wu@atmel.com>,
	"Bhuvanchandra DV" <bhuvanchandra.dv@toradex.com>,
	"Neil Armstrong" <narmstrong@baylibre.com>,
	"Eric Anholt" <eric@anholt.net>,
	nouveau@lists.freedesktop.org, "Guido Günther" <agx@sigxcpu.org>,
	"open list:DRM PANEL DRIVERS" <dri-devel@lists.freedesktop.org>,
	"Gustaf Lindström" <gl@axentia.se>,
	"Andrzej Hajda" <a.hajda@samsung.com>,
	"Laurent Pinchart" <Laurent.pinchart@ideasonboard.com>,
	"Philipp Zabel" <philipp.zabel@gmail.com>,
	"Sam Ravnborg" <sam@ravnborg.org>,
	"Marian-Cristian Rotariu"
	<marian-cristian.rotariu.rb@bp.renesas.com>,
	"Jagan Teki" <jagan@amarulasolutions.com>,
	"Thomas Hellstrom" <thellstrom@vmware.com>,
	"Joonyoung Shim" <jy0922.shim@samsung.com>,
	"Jonathan Marek" <jonathan@marek.ca>,
	"Stefan Mavrodiev" <stefan@olimex.com>,
	"Adam Ford" <aford173@gmail.com>,
	"Jerry Han" <hanxu5@huaqin.corp-partner.google.com>
Subject: Re: [PATCH 04/12] drm: Nuke mode->vrefresh
Date: Tue, 25 Feb 2020 22:52:25 +0100	[thread overview]
Message-ID: <CACRpkdZk9QEy+Kzkmy4BXiHB+aq9hprf=dmA_-R23yqH3NCt1g@mail.gmail.com> (raw)
In-Reply-To: <20200225192720.GG13686@intel.com>

On Tue, Feb 25, 2020 at 8:27 PM Ville Syrjälä
<ville.syrjala@linux.intel.com> wrote:

> OK, so I went ahead a wrote a bit of cocci [1] to find the bad apples.

That's impressive :D

> Unfortunately it found a lot of strange stuff:

I will answer for the weirdness I caused.

I have long suspected that a whole bunch of the "simple" displays
are not simple but contains a display controller and memory.
That means that the speed over the link to the display and
actual refresh rate on the actual display is asymmetric because
well we are just updating a RAM, the resolution just limits how
much data we are sending, the clock limits the speed on the
bus over to the RAM on the other side.

In most cases I thing the clock is the way to go.

> panel-sony-acx424akp.c:51/sony_acx424akp_vid_mode: 60 vs. 727 (.clock=330000 .htotal=480 + 15 + 0 + 15 .vtotal=864 + 14 + 1 + 11)

I suspect clock should be adjusted after vfresh = 60 here instead of the
other way around. I couldn't quite test the video mode, but the vendor
driver (no documentation ....) does state 330 MHz which seems a
bit high.

Just drop vrefresh for now.

> panel-sony-acx424akp.c:71/sony_acx424akp_cmd_mode: 60 vs. 711 (.clock=420160 .htotal=480 + 154 + 16 + 32 .vtotal=864 + 1 + 1 + 1)

You can override this ignoring the vrefresh, this is a command-mode
only, and in command mode the refresh doesn't come into play, or is
very high and limited by a bunch of other overhead than just the
resolution. The command mode HS clock is @420+ MHz indeed.
Tests showed around 116 Hz for this particular display in practice
with continuous updates.

> panel-ilitek-ili9322.c:543/srgb_320x240_mode: 60 vs. 10168 (.clock=2453500 .htotal=320 + 359 + 1 + 241 .vtotal=262)
> panel-ilitek-ili9322.c:587/yuv_640x320_mode: 60 vs. 7768 (.clock=2454000 .htotal=640 + 252 + 1 + 28 .vtotal=320 + 4 + 1 + 18)
> panel-ilitek-ili9322.c:616/itu_r_bt_656_640_mode: 60 vs. 5358 (.clock=2454000 .htotal=640 + 3 + 1 + 272 .vtotal=500)
> panel-ilitek-ili9322.c:557/srgb_360x240_mode: 60 vs. 16178 (.clock=2700000 .htotal=360 + 35 + 1 + 241 .vtotal=262)
> panel-ilitek-ili9322.c:601/yuv_720x360_mode: 60 vs. 7071 (.clock=2700000 .htotal=720 + 252 + 1 + 24 .vtotal=360 + 4 + 1 + 18)
> panel-ilitek-ili9322.c:631/itu_r_bt_656_720_mode: 60 vs. 5422 (.clock=2700000 .htotal=720 + 3 + 1 + 272 .vtotal=500)
> panel-ilitek-ili9322.c:572/prgb_320x240_mode: 60 vs. 59725 (.clock=6400000 .htotal=320 + 38 + 1 + 50 .vtotal=262)

This is the datasheet if you want to take a look:
https://dflund.se/~triad/krad/dlink-dir-685/ILI9322DS_V1.12.pdf

The one platform using this is using the
8 bit ITU-R BT.656 640Y 320CbCr mode which
(I think) is similar to DSI command mode again: there is a stream
of data in a burst and you decide how often you want to send it
because the panel always has backing memory and the
speed out to the physical display is something completely
different.

You can safely delete vrefresh from all of these sites.

> panel-arm-versatile.c:161/versatile_panels[]: 60 vs. 61 (.clock=25000 .htotal=640 + 24 + 96 + 24 .vtotal=480 + 11 + 2 + 32)
> panel-arm-versatile.c:208/versatile_panels[]: 116 vs. 59 (.clock=5400 .htotal=240 + 10 + 10 + 20 .vtotal=320 + 2 + 2 + 2)
> panel-arm-versatile.c:184/versatile_panels[]: 390 vs. 1523 (.clock=62500 .htotal=176 + 2 + 3 + 3 .vtotal=220 + 0 + 2 + 1)

The only driver  drivers/gpu/drm/pl111/pl111_display.c
Uses mode->clock so just drop vrefresh.

Yours,
Linus Walleij
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx