Re: [PATCH v15 4/7] drm/bridge: dw-hdmi: repair interworking with hdmi-connector for jz4780

From: "H. Nikolaus Schaller" <hns@goldelico.com>
To: Neil Armstrong <narmstrong@baylibre.com>
Cc: Andrzej Hajda <andrzej.hajda@intel.com>,
	Robert Foss <robert.foss@linaro.org>,
	Paul Boddie <paul@boddie.org.uk>,
	Laurent Pinchart <Laurent.pinchart@ideasonboard.com>,
	Jernej Skrabec <jernej.skrabec@gmail.com>,
	David Airlie <airlied@linux.ie>, Daniel Vetter <daniel@ffwll.ch>,
	Paul Cercueil <paul@crapouillou.net>,
	Maxime Ripard <maxime@cerno.tech>,
	Harry Wentland <harry.wentland@amd.com>,
	Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>,
	letux-kernel@openphoenux.org, linux-mips@vger.kernel.org,
	linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org,
	Jonas Karlman <jonas@kwiboo.se>
Subject: Re: [PATCH v15 4/7] drm/bridge: dw-hdmi: repair interworking with hdmi-connector for jz4780
Date: Sat, 26 Feb 2022 18:12:34 +0100	[thread overview]
Message-ID: <3F01AD9B-EA49-4A34-A421-43E9E6268311@goldelico.com> (raw)
In-Reply-To: <9c6f5c0b-5879-736f-344f-dfa12a0d7937@baylibre.com>

Hi Neil,

> Am 21.02.2022 um 09:11 schrieb Neil Armstrong <narmstrong@baylibre.com>:
> 
> Hi,
> 
> On 12/02/2022 16:50, H. Nikolaus Schaller wrote:
>> 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.
>> 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 b0d8110dd412c..826a055a7a273 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_YCBCR422)
>> +	if (max_bpc >= info->bpc && info->color_formats & DRM_COLOR_FORMAT_YCBCR422)
>>  		output_fmts[i++] = MEDIA_BUS_FMT_UYVY8_1X16;
>>  -	if (info->color_formats & DRM_COLOR_FORMAT_YCBCR444)
>> +	if (max_bpc >= info->bpc && info->color_formats & DRM_COLOR_FORMAT_YCBCR444)
>>  		output_fmts[i++] = MEDIA_BUS_FMT_YUV8_1X24;
>>    	/* Default 8bit RGB fallback */
> 
> Please do the same for all other cases and change the patch subject to something more accurate like:
> "drm/bridge: dw-hdmi: take display info bpc in account for bus formats negociation"

thank you very much for this analysis.

In fact my fix was also not correctly doing the right comparisons. The rules should have the
same structure as for all cases before, i.e. check for >= 8 and then handle both color formats.
I have fixed and tested on my CI20 board and updated the description and Fixes: tags.

v16 will come now (as part of the jz4780 hdmi series).

BR and thanks,
Nikolaus