Re: [PATCH v1 03/12] drm/mediatek: Drop chain_mode_fixup call in mode_valid() - Chun-Kuang Hu

From: Chun-Kuang Hu <chunkuang.hu@kernel.org>
To: Sam Ravnborg <sam@ravnborg.org>
Cc: DRI Development <dri-devel@lists.freedesktop.org>,
	 Andrzej Hajda <andrzej.hajda@intel.com>,
	Neil Armstrong <narmstrong@baylibre.com>,
	 Robert Foss <robert.foss@linaro.org>,
	 Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
	Jonas Karlman <jonas@kwiboo.se>,
	 Jernej Skrabec <jernej.skrabec@gmail.com>,
	Arnd Bergmann <arnd@arndb.de>,
	 Benson Leung <bleung@chromium.org>,
	Cai Huoqing <cai.huoqing@linux.dev>,
	 chrome-platform@lists.linux.dev,
	Chun-Kuang Hu <chunkuang.hu@kernel.org>,
	 Dafna Hirschfeld <dafna.hirschfeld@collabora.com>,
	Daniel Vetter <daniel@ffwll.ch>,  David Airlie <airlied@linux.ie>,
	Enric Balletbo i Serra <enric.balletbo@collabora.com>,
	 Guenter Roeck <groeck@chromium.org>,
	Jitao Shi <jitao.shi@mediatek.com>,
	 Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>,
	 Linux ARM <linux-arm-kernel@lists.infradead.org>,
	 "moderated list:ARM/Mediatek SoC support"
	<linux-mediatek@lists.infradead.org>,
	linux-renesas-soc@vger.kernel.org,
	 Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
	 Matthias Brugger <matthias.bgg@gmail.com>,
	Maxime Ripard <mripard@kernel.org>,
	 Philip Chen <philipchen@chromium.org>,
	Philipp Zabel <p.zabel@pengutronix.de>,
	 Thomas Zimmermann <tzimmermann@suse.de>,
	Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
Subject: Re: [PATCH v1 03/12] drm/mediatek: Drop chain_mode_fixup call in mode_valid()
Date: Sun, 18 Sep 2022 12:45:59 +0800	[thread overview]
Message-ID: <CAAOTY_9Yo4Kx+UaRQSGcqHtA6hMBvFUUwHe6-vQX4Vt5nBJb+A@mail.gmail.com> (raw)
In-Reply-To: <20220717174454.46616-4-sam@ravnborg.org>

Hi, Sam:

Sam Ravnborg <sam@ravnborg.org> 於 2022年7月18日 週一 凌晨1:45寫道：
>
> The mode_valid implementation had a call to
> drm_bridge_chain_mode_fixup() which would be wrong as the mode_valid is
> not allowed to change anything - only to validate the mode.
>
> As the next bridge is often/always a connector the call had no effect
> anyway. So drop it.
>
> From the git history I could see this call was included in the original
> version of the driver so there was no help there to find out why it was
> added in the first place. But a lot has changed since the initial driver
> were added and is seems safe to remove the call now.

Acked-by: Chun-Kuang Hu <chunkuang.hu@kernel.org>

>
> Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
> Reviewed-by: Maxime Ripard <mripard@kernel.org>
> Cc: Chun-Kuang Hu <chunkuang.hu@kernel.org>
> Cc: Philipp Zabel <p.zabel@pengutronix.de>
> Cc: Matthias Brugger <matthias.bgg@gmail.com>
> Cc: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
> Cc: linux-mediatek@lists.infradead.org
> Cc: linux-arm-kernel@lists.infradead.org
> ---
>  drivers/gpu/drm/mediatek/mtk_hdmi.c | 11 -----------
>  1 file changed, 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/mediatek/mtk_hdmi.c b/drivers/gpu/drm/mediatek/mtk_hdmi.c
> index 3196189429bc..a63b76055f81 100644
> --- a/drivers/gpu/drm/mediatek/mtk_hdmi.c
> +++ b/drivers/gpu/drm/mediatek/mtk_hdmi.c
> @@ -1208,22 +1208,11 @@ static int mtk_hdmi_bridge_mode_valid(struct drm_bridge *bridge,
>                                       const struct drm_display_mode *mode)
>  {
>         struct mtk_hdmi *hdmi = hdmi_ctx_from_bridge(bridge);
> -       struct drm_bridge *next_bridge;
>
>         dev_dbg(hdmi->dev, "xres=%d, yres=%d, refresh=%d, intl=%d clock=%d\n",
>                 mode->hdisplay, mode->vdisplay, drm_mode_vrefresh(mode),
>                 !!(mode->flags & DRM_MODE_FLAG_INTERLACE), mode->clock * 1000);
>
> -       next_bridge = drm_bridge_get_next_bridge(&hdmi->bridge);
> -       if (next_bridge) {
> -               struct drm_display_mode adjusted_mode;
> -
> -               drm_mode_copy(&adjusted_mode, mode);
> -               if (!drm_bridge_chain_mode_fixup(next_bridge, mode,
> -                                                &adjusted_mode))
> -                       return MODE_BAD;
> -       }
> -
>         if (hdmi->conf) {
>                 if (hdmi->conf->cea_modes_only && !drm_match_cea_mode(mode))
>                         return MODE_BAD;
> --
> 2.34.1
>