Re: [PATCH] drm/meson: fix HDMI2 420 display mode selection logic

[Neil Armstrong <narmstrong@baylibre.com>]

Hi,


On 15/05/2022 22:44, Adrián Larumbe wrote:
> Commit e67f6037ae1be34b2b68 ("drm/meson: split out encoder from
> meson_dw_hdmi") introduced a new way of calculating the display's pixel
> clock. However, it leads to the wrong value being reckoned for Odroid N2+
> boards, where clock frequency is never halved when the display's videomode
> supports YCBCR420 output format.

The current logic is designed to select YUV420 for:
- HDMI2 sinks when the selected mode is /only/ yu420
- non-HDMI2 sinks when the selected mode is /also/ yuv420

For the later, it's mainly for pre-HDMI2 TVs supporting 4k@24hz/30Hz but also
4k@60Hz in yuv420 mode, this is exposed using the drm_mode_is_420_also().

If I understand correctly, you want to always enable yuv420 when a mode /can/
use yu420, which is not how the code is written right now.

The current code code prioritizes YUV then RGB and 444->422>420 in case of YUV,
the when a HDMI2 sink is connected and support 4k@60Hz in RGB, YUV444 and YUV420,
the current logic will select 4k@60Hz-YUV444 because it the direct output (well
not exactly, the pipeline is 10bit) of the video pipeline with no color conversion
in the middle.

If you want to introduce different bus format priority, you should then
add HDMI content type, or non_desktop connector connector properties.

Not this should be done in meson driver and dw-hdmi.

Neil

> 
> Fix the selection logic.
> 
> Signed-off-by: Adrián Larumbe <adrian.larumbe@collabora.com>
> ---
>   drivers/gpu/drm/meson/meson_dw_hdmi.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/meson/meson_dw_hdmi.c b/drivers/gpu/drm/meson/meson_dw_hdmi.c
> index 5cd2b2ebbbd3..6c9640f4c82e 100644
> --- a/drivers/gpu/drm/meson/meson_dw_hdmi.c
> +++ b/drivers/gpu/drm/meson/meson_dw_hdmi.c
> @@ -380,7 +380,7 @@ static int dw_hdmi_phy_init(struct dw_hdmi *hdmi, void *data,
>   			 mode->clock > 340000 ? 40 : 10);
>   
>   	if (drm_mode_is_420_only(display, mode) ||
> -	    (!is_hdmi2_sink &&
> +	    (is_hdmi2_sink &&
>   	     drm_mode_is_420_also(display, mode)))
>   		mode_is_420 = true;
>   


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic