Hi Neil,
Subject: Re: dw_hdmi is showing wrong colour after commit 7cd70656d1285b79("drm/bridge: display-connector: implement bus fmts callbacks")
On 14/01/2022 09:29, Biju Das wrote:
Hi Neil,
- renesas-soc
Subject: Re: dw_hdmi is showing wrong colour after commit 7cd70656d1285b79("drm/bridge: display-connector: implement bus fmts callbacks")
Hi,
On 13/01/2022 21:01, Fabio Estevam wrote:
Hi Biju,
On Thu, Jan 13, 2022 at 2:45 PM Biju Das biju.das.jz@bp.renesas.com
wrote:
Hi All,
RZ/G2{H, M, N} SoC has dw_hdmi IP and it was working ok(colour) till the commit 7cd70656d1285b79("drm/bridge: display-connector: implement bus fmts
callbacks").
After this patch, the screen becomes greenish(may be it is setting it
into YUV format??).
By checking the code, previously it used to call get_input_fmt callback
and set colour as RGB24.
After this commit, it calls get_output_fmt_callbck and returns 3 outputformats(YUV16, YUV24 and RGB24) And get_input_fmt callback, I see
the outputformat as YUV16 instead of RGB24.
Not sure, I am the only one seeing this issue with dw_HDMI driver.
This patch was introduced to maintain the bridge color format negotiation after using DRM_BRIDGE_ATTACH_NO_CONNECTOR, but it seems it behaves incorrectly if the first bridge doesn't implement the negotiation callbacks.
Let me check the code to see how to fix that.
Thanks for the information, I am happy to test the patch/fix.
Cheers, Biju
I have tested linux-next 20220112 on a imx6q-sabresd board, which
shows:
dwhdmi-imx 120000.hdmi: Detected HDMI TX controller v1.30a with HDCP (DWC HDMI 3D TX PHY)
The colors are shown correctly here.
The imx doesn't use DRM_BRIDGE_ATTACH_NO_CONNECTOR so the negotiation fails and use the RGB fallback input & output format.
Anyway thanks for testing
Neil
Can you test :
==><=============================== diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c index c96847fc0ebc..7019acd37716 100644 --- a/drivers/gpu/drm/drm_bridge.c +++ b/drivers/gpu/drm/drm_bridge.c @@ -955,7 +955,14 @@ drm_atomic_bridge_chain_select_bus_fmts(struct drm_bridge *bridge, last_bridge_state = drm_atomic_get_new_bridge_state(crtc_state-
state,
last_bridge);
if (last_bridge->funcs->atomic_get_output_bus_fmts) {
/*
* Only negociate with real values if both end of the bridge chain
* support negociation callbacks, otherwise you can end in a
situation
* where the selected output format doesn't match with the first
bridge
* output format.
*/
if (bridge->funcs->atomic_get_input_bus_fmts &&
last_bridge->funcs->atomic_get_output_bus_fmts) { const struct drm_bridge_funcs *funcs = last_bridge->funcs; /*
@@ -980,7 +987,12 @@ drm_atomic_bridge_chain_select_bus_fmts(struct drm_bridge *bridge, if (!out_bus_fmts) return -ENOMEM;
if (conn->display_info.num_bus_formats &&
/*
* If first bridge doesn't support negociation, use
MEDIA_BUS_FMT_FIXED
* as a safe value for the whole bridge chain
*/
if (bridge->funcs->atomic_get_input_bus_fmts &&
conn->display_info.num_bus_formats && conn->display_info.bus_formats) out_bus_fmts[0] = conn-
display_info.bus_formats[0];
else
==><===============================
This should exclude your situation where the first bridge doesn't support negociation.
I have tested this fix with Linux next-20220114. Still I see colour issue.
It is still negotiating and it is calling get_output_fmt_callbck
[ 3.460155] ########dw_hdmi_bridge_atomic_get_output_bus_fmts MEDIA_BUS_FMT_UYVY8_1X16=0######### [ 3.460180] ########dw_hdmi_bridge_atomic_get_output_bus_fmts MEDIA_BUS_FMT_YUV8_1X24=1######### [ 3.460202] ########dw_hdmi_bridge_atomic_get_output_bus_fmts MEDIA_BUS_FMT_RGB888_1X24=2#########
And In get_input_fmt callback, I See the outputformat as YUV16 instead of RGB24.
[ 3.460319] ########dw_hdmi_bridge_atomic_get_input_bus_fmts MEDIA_BUS_FMT_UYVY8_1X16######### [ 3.473644] ########hdmi_video_sample MEDIA_BUS_FMT_UYVY8_1X16#########
Regards, Biju