Hi Ville,
Thank you for the patch.
On Fri, Apr 03, 2020 at 11:39:54PM +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
>
> Get rid of mode->vrefresh and just calculate it on demand. Saves
> a bit of space and avoids the cached value getting out of sync
> with reality.
>
> Mostly done with cocci, with the following manual fixups:
> - Remove the now empty loop in drm_helper_probe_single_connector_modes()
> - Fix __MODE() macro in ch7006_mode.c
> - Fix DRM_MODE_ARG() macro in drm_modes.h
> - Remove leftover comment from samsung_s6d16d0_mode
> - Drop the TODO
>
> @@
> @@
> struct drm_display_mode {
> ...
> - int vrefresh;
> ...
> };
>
> @@
> identifier N;
> expression E;
> @@
> struct drm_display_mode N = {
> - .vrefresh = E
> };
>
> @@
> identifier N;
> expression E;
> @@
> struct drm_display_mode N[...] = {
> ...,
> {
> - .vrefresh = E
> }
> ,...
> };
>
> @@
> expression E;
> @@
> {
> DRM_MODE(...),
> - .vrefresh = E,
> }
>
> @@
> identifier M, R;
> @@
> int drm_mode_vrefresh(const struct drm_display_mode *M)
> {
> ...
> - if (M->vrefresh > 0)
> - R = M->vrefresh;
> - else
> if (...) {
> ...
> }
> ...
> }
>
> @@
> struct drm_display_mode *p;
> expression E;
> @@
> (
> - p->vrefresh = E;
> |
> - p->vrefresh
> + drm_mode_vrefresh(p)
> )
>
> @@
> struct drm_display_mode s;
> expression E;
> @@
> (
> - s.vrefresh = E;
> |
> - s.vrefresh
> + drm_mode_vrefresh(&s)
> )
>
> @@
> expression E;
> @@
> - drm_mode_vrefresh(E) ? drm_mode_vrefresh(E) : drm_mode_vrefresh(E)
> + drm_mode_vrefresh(E)
>
> @find_substruct@
> identifier X;
> identifier S;
> @@
> struct X {
> ...
> struct drm_display_mode S;
> ...
> };
>
> @@
> identifier find_substruct.S;
> expression E;
> identifier I;
> @@
> {
> .S = {
> - .vrefresh = E
> }
> }
>
> @@
> identifier find_substruct.S;
> identifier find_substruct.X;
> expression E;
> identifier I;
> @@
> struct X I[...] = {
> ...,
> .S = {
> - .vrefresh = E
> }
> ,...
> };
>
> v2: Drop TODO
>
> Cc: Andrzej Hajda <a.hajda@xxxxxxxxxxx>
> Cc: Neil Armstrong <narmstrong@xxxxxxxxxxxx>
> Cc: Laurent Pinchart <Laurent.pinchart@xxxxxxxxxxxxxxxx>
> Cc: Jonas Karlman <jonas@xxxxxxxxx>
> Cc: Jernej Skrabec <jernej.skrabec@xxxxxxxx>
> Cc: Inki Dae <inki.dae@xxxxxxxxxxx>
> Cc: Joonyoung Shim <jy0922.shim@xxxxxxxxxxx>
> Cc: Seung-Woo Kim <sw0312.kim@xxxxxxxxxxx>
> Cc: Kyungmin Park <kyungmin.park@xxxxxxxxxxx>
> Cc: Linus Walleij <linus.walleij@xxxxxxxxxx>
> Cc: CK Hu <ck.hu@xxxxxxxxxxxx>
> Cc: Philipp Zabel <p.zabel@xxxxxxxxxxxxxx>
> Cc: Ben Skeggs <bskeggs@xxxxxxxxxx>
> Cc: Thierry Reding <thierry.reding@xxxxxxxxx>
> Cc: Sam Ravnborg <sam@xxxxxxxxxxxx>
> Cc: Jerry Han <hanxu5@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxx>
> Cc: Icenowy Zheng <icenowy@xxxxxxx>
> Cc: Jagan Teki <jagan@xxxxxxxxxxxxxxxxxxxx>
> Cc: Stefan Mavrodiev <stefan@xxxxxxxxxx>
> Cc: Robert Chiras <robert.chiras@xxxxxxx>
> Cc: "Guido Günther" <agx@xxxxxxxxxxx>
> Cc: Purism Kernel Team <kernel@xxxxxxx>
> Cc: Benjamin Gaignard <benjamin.gaignard@xxxxxxxxxx>
> Cc: Vincent Abriou <vincent.abriou@xxxxxx>
> Cc: VMware Graphics <linux-graphics-maintainer@xxxxxxxxxx>
> Cc: Thomas Hellstrom <thellstrom@xxxxxxxxxx>
> Cc: linux-amlogic@xxxxxxxxxxxxxxxxxxx
> Cc: nouveau@xxxxxxxxxxxxxxxxxxxxx
> Reviewed-by: Emil Velikov <emil.velikov@xxxxxxxxxxxxx>
> Reviewed-by: Sam Ravnborg <sam@xxxxxxxxxxxx>
> Acked-by: Linus Walleij <linus.walleij@xxxxxxxxxx>
> Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> ---
> Documentation/gpu/todo.rst | 20 --
> drivers/gpu/drm/bridge/sii902x.c | 2 +-
> drivers/gpu/drm/drm_client_modeset.c | 2 +-
> drivers/gpu/drm/drm_edid.c | 328 +++++++++---------
> drivers/gpu/drm/drm_modes.c | 9 +-
> drivers/gpu/drm/drm_probe_helper.c | 3 -
> drivers/gpu/drm/exynos/exynos_hdmi.c | 5 +-
> drivers/gpu/drm/exynos/exynos_mixer.c | 2 +-
> drivers/gpu/drm/i2c/ch7006_mode.c | 1 -
> drivers/gpu/drm/i915/display/intel_display.c | 1 -
> .../drm/i915/display/intel_display_debugfs.c | 4 +-
> drivers/gpu/drm/i915/display/intel_dp.c | 10 +-
> drivers/gpu/drm/i915/display/intel_tv.c | 3 -
> drivers/gpu/drm/mcde/mcde_dsi.c | 6 +-
> drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 4 +-
> drivers/gpu/drm/mediatek/mtk_hdmi.c | 2 +-
> drivers/gpu/drm/meson/meson_venc_cvbs.c | 2 -
> drivers/gpu/drm/nouveau/nouveau_connector.c | 5 +-
> drivers/gpu/drm/panel/panel-arm-versatile.c | 4 -
> drivers/gpu/drm/panel/panel-boe-himax8279d.c | 3 +-
> .../gpu/drm/panel/panel-boe-tv101wum-nl6.c | 6 +-
> drivers/gpu/drm/panel/panel-elida-kd35t133.c | 3 +-
> .../gpu/drm/panel/panel-feixin-k101-im2ba02.c | 3 +-
> .../drm/panel/panel-feiyang-fy07024di26a30d.c | 3 +-
> drivers/gpu/drm/panel/panel-ilitek-ili9322.c | 7 -
> drivers/gpu/drm/panel/panel-ilitek-ili9881c.c | 3 +-
> drivers/gpu/drm/panel/panel-innolux-p079zca.c | 4 +-
> .../gpu/drm/panel/panel-jdi-lt070me05000.c | 3 +-
> .../drm/panel/panel-kingdisplay-kd097d04.c | 3 +-
> .../drm/panel/panel-leadtek-ltk500hd1829.c | 3 +-
> drivers/gpu/drm/panel/panel-lg-lb035q02.c | 1 -
> drivers/gpu/drm/panel/panel-lg-lg4573.c | 3 +-
> drivers/gpu/drm/panel/panel-nec-nl8048hl11.c | 1 -
> drivers/gpu/drm/panel/panel-novatek-nt35510.c | 1 -
> drivers/gpu/drm/panel/panel-novatek-nt39016.c | 1 -
> .../drm/panel/panel-olimex-lcd-olinuxino.c | 1 -
> .../gpu/drm/panel/panel-orisetech-otm8009a.c | 3 +-
> .../drm/panel/panel-osd-osd101t2587-53ts.c | 3 +-
> .../drm/panel/panel-panasonic-vvx10f034n00.c | 3 +-
> .../drm/panel/panel-raspberrypi-touchscreen.c | 4 +-
> drivers/gpu/drm/panel/panel-raydium-rm67191.c | 3 +-
> drivers/gpu/drm/panel/panel-raydium-rm68200.c | 3 +-
> .../drm/panel/panel-rocktech-jh057n00900.c | 5 +-
> drivers/gpu/drm/panel/panel-ronbo-rb070d30.c | 1 -
> drivers/gpu/drm/panel/panel-samsung-s6d16d0.c | 6 -
> drivers/gpu/drm/panel/panel-samsung-s6e3ha2.c | 4 +-
> .../gpu/drm/panel/panel-samsung-s6e63j0x03.c | 3 +-
> drivers/gpu/drm/panel/panel-samsung-s6e63m0.c | 3 +-
> .../panel/panel-samsung-s6e88a0-ams452ef01.c | 1 -
> drivers/gpu/drm/panel/panel-seiko-43wvf1g.c | 3 +-
> .../gpu/drm/panel/panel-sharp-lq101r1sx01.c | 3 +-
> .../gpu/drm/panel/panel-sharp-ls037v7dw01.c | 1 -
> .../gpu/drm/panel/panel-sharp-ls043t1le01.c | 3 +-
> drivers/gpu/drm/panel/panel-simple.c | 87 +----
> drivers/gpu/drm/panel/panel-sitronix-st7701.c | 2 +-
> .../gpu/drm/panel/panel-sitronix-st7789v.c | 3 +-
> drivers/gpu/drm/panel/panel-sony-acx424akp.c | 2 -
> drivers/gpu/drm/panel/panel-sony-acx565akm.c | 1 -
> drivers/gpu/drm/panel/panel-tpo-td028ttec1.c | 1 -
> drivers/gpu/drm/panel/panel-tpo-td043mtea1.c | 1 -
> drivers/gpu/drm/panel/panel-tpo-tpg110.c | 5 -
> drivers/gpu/drm/panel/panel-truly-nt35597.c | 1 -
> .../gpu/drm/panel/panel-xinpeng-xpp055c272.c | 3 +-
> drivers/gpu/drm/sti/sti_hda.c | 1 -
> drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 2 -
> include/drm/drm_modes.h | 12 +-
> 66 files changed, 218 insertions(+), 417 deletions(-)
[snip]
> diff --git a/drivers/gpu/drm/mcde/mcde_dsi.c b/drivers/gpu/drm/mcde/mcde_dsi.c
> index 7af5ebb0c436..52031d826f2c 100644
> --- a/drivers/gpu/drm/mcde/mcde_dsi.c
> +++ b/drivers/gpu/drm/mcde/mcde_dsi.c
> @@ -538,7 +538,7 @@ static void mcde_dsi_setup_video_mode(struct mcde_dsi *d,
> */
> /* (ps/s) / (pixels/s) = ps/pixels */
> pclk = DIV_ROUND_UP_ULL(1000000000000,
> - (mode->vrefresh * mode->htotal * mode->vtotal));
> + (drm_mode_vrefresh(mode) * mode->htotal * mode->vtotal));
Shouldn't you just use the pixel clock here ?
Update: There's a patch further in this series that handles this, great
:-)
> dev_dbg(d->dev, "picoseconds between two pixels: %llu\n",
> pclk);
>
[snip]
> diff --git a/drivers/gpu/drm/nouveau/nouveau_connector.c b/drivers/gpu/drm/nouveau/nouveau_connector.c
> index 9a9a7f5003d3..ac80b1ac459c 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_connector.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_connector.c
> @@ -59,7 +59,6 @@ nouveau_conn_native_mode(struct drm_connector *connector)
> int high_w = 0, high_h = 0, high_v = 0;
>
> list_for_each_entry(mode, &connector->probed_modes, head) {
> - mode->vrefresh = drm_mode_vrefresh(mode);
> if (helper->mode_valid(connector, mode) != MODE_OK ||
> (mode->flags & DRM_MODE_FLAG_INTERLACE))
> continue;
> @@ -80,12 +79,12 @@ nouveau_conn_native_mode(struct drm_connector *connector)
> continue;
>
> if (mode->hdisplay == high_w && mode->vdisplay == high_h &&
> - mode->vrefresh < high_v)
> + drm_mode_vrefresh(mode) < high_v)
Here too, should the logic be modified to use the clock ?
This can be addressed in a separate patch, so for this,
Reviewed-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
> continue;
>
> high_w = mode->hdisplay;
> high_h = mode->vdisplay;
> - high_v = mode->vrefresh;
> + high_v = drm_mode_vrefresh(mode);
> largest = mode;
> }
>
--
Regards,
Laurent Pinchart
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
