Re: [PATCH v13 6/9] dw-hdmi/ingenic-dw-hdmi: repair interworking with hdmi-connector

From: Paul Cercueil <paul@crapouillou.net>
To: "H. Nikolaus Schaller" <hns@goldelico.com>
Cc: Mark Rutland <mark.rutland@arm.com>,
	Paul Boddie <paul@boddie.org.uk>,
	Geert Uytterhoeven <geert+renesas@glider.be>,
	Neil Armstrong <narmstrong@baylibre.com>,
	David Airlie <airlied@linux.ie>, Rob Herring <robh+dt@kernel.org>,
	linux-mips@vger.kernel.org,
	Laurent Pinchart <Laurent.pinchart@ideasonboard.com>,
	Andrzej Hajda <andrzej.hajda@intel.com>,
	Miquel Raynal <miquel.raynal@bootlin.com>,
	Sam Ravnborg <sam@ravnborg.org>,
	Jernej Skrabec <jernej.skrabec@gmail.com>,
	devicetree@vger.kernel.org, Kees Cook <keescook@chromium.org>,
	Jonas Karlman <jonas@kwiboo.se>, Mark Brown <broonie@kernel.org>,
	Maxime Ripard <maxime@cerno.tech>,
	letux-kernel@openphoenux.org,
	Thomas Bogendoerfer <tsbogend@alpha.franken.de>,
	dri-devel@lists.freedesktop.org,
	Liam Girdwood <lgirdwood@gmail.com>,
	Robert Foss <robert.foss@linaro.org>,
	linux-kernel@vger.kernel.org,
	Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>,
	"Eric W. Biederman" <ebiederm@xmission.com>,
	Hans Verkuil <hverkuil-cisco@xs4all.nl>
Subject: Re: [PATCH v13 6/9] dw-hdmi/ingenic-dw-hdmi: repair interworking with hdmi-connector
Date: Wed, 09 Feb 2022 12:18:14 +0000	[thread overview]
Message-ID: <E6E17R.ND5XW1IG4SYA@crapouillou.net> (raw)
In-Reply-To: <866f84621974992f4831bd471ae5a53414de9255.1643819482.git.hns@goldelico.com>

Hi Nikolaus,

Le mer., févr. 2 2022 at 17:31:20 +0100, H. Nikolaus Schaller 
<hns@goldelico.com> a écrit :
> Commit 7cd70656d1285b ("drm/bridge: display-connector: implement bus 
> fmts callbacks")
> 
> introduced a new mechanism to negotiate bus formats between hdmi 
> connector
> and the synopsys hdmi driver inside the jz4780.
> 
> By this, the dw-hdmi is no longer the only bridge and sets up a list
> of formats in dw_hdmi_bridge_atomic_get_output_bus_fmts().
> 
> This includes MEDIA_BUS_FMT_UYVY8_1X16 which is chosen for the jz4780 
> but only
> produces a black screen.
> 
> This fix is based on the observation that max_bpc = 0 when running 
> this
> function while info->bpc = 8. Since the formats checks before this 
> always test
> for max_bpc >= info->pbc indirectly my assumption is that we must 
> check it
> here as well.

This fix looks really strange to me, so I'll let the DRM experts 
comment.

It would still be better to move the patch before the introduction of 
dw-ingenic-hdmi.c, so that once this one is introduced, everything 
works. This also enables bisectability.

Cheers,
-Paul

> 
> Adding the proposed patch makes the CI20/jz4780 panel work again in
> MEDIA_BUS_FMT_RGB888_1X24 mode.
> 
> Fixes: 7cd70656d1285b ("drm/bridge: display-connector: implement bus 
> fmts callbacks")
> Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
> ---
>  drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c 
> b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> index 52e7cd2e020d3..34703a15ee4ff 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> @@ -2620,10 +2620,10 @@ static u32 
> *dw_hdmi_bridge_atomic_get_output_bus_fmts(struct drm_bridge *bridge,
>  		output_fmts[i++] = MEDIA_BUS_FMT_RGB101010_1X30;
>  	}
> 
> -	if (info->color_formats & DRM_COLOR_FORMAT_YCRCB422)
> +	if (max_bpc >= info->bpc && info->color_formats & 
> DRM_COLOR_FORMAT_YCRCB422)
>  		output_fmts[i++] = MEDIA_BUS_FMT_UYVY8_1X16;
> 
> -	if (info->color_formats & DRM_COLOR_FORMAT_YCRCB444)
> +	if (max_bpc >= info->bpc && info->color_formats & 
> DRM_COLOR_FORMAT_YCRCB444)
>  		output_fmts[i++] = MEDIA_BUS_FMT_YUV8_1X24;
> 
>  	/* Default 8bit RGB fallback */
> --
> 2.33.0
> 