On Mon, May 03, 2021 at 08:21:48PM +0200, Werner Sembach wrote:
> When encoder validation of a display mode fails, retry with less bandwidth
> heavy YCbCr420 color mode, if available. This enables some HDMI 1.4 setups
> to support 4k60Hz output, which previously failed silently.
>
> AMDGPU had nearly the exact same issue. This problem description is
> therefore copied from my commit message of the AMDGPU patch.
>
> On some setups, while the monitor and the gpu support display modes with
> pixel clocks of up to 600MHz, the link encoder might not. This prevents
> YCbCr444 and RGB encoding for 4k60Hz, but YCbCr420 encoding might still be
> possible. However, which color mode is used is decided before the link
> encoder capabilities are checked. This patch fixes the problem by retrying
> to find a display mode with YCbCr420 enforced and using it, if it is
> valid.
>
> Signed-off-by: Werner Sembach <w...@tuxedocomputers.com>
> ---
>
> >From 4ea0c8839b47e846d46c613e38af475231994f0f Mon Sep 17 00:00:00 2001
> From: Werner Sembach <w...@tuxedocomputers.com>
> Date: Mon, 3 May 2021 16:23:17 +0200
> Subject: [PATCH 4/4] Use YCbCr420 as fallback when RGB fails
>
> ---
> drivers/gpu/drm/i915/display/intel_hdmi.c | 10 +++++++++-
> 1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c
> b/drivers/gpu/drm/i915/display/intel_hdmi.c
> index e2553ac6fd13..20c800f2ed60 100644
> --- a/drivers/gpu/drm/i915/display/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
> @@ -1913,7 +1913,7 @@ intel_hdmi_mode_valid(struct drm_connector *connector,
> clock *= 2;
> }
>
> - if (connector->ycbcr_420_allowed &&
> drm_mode_is_420_only(&connector->display_info, mode))
> + if (connector->ycbcr_420_allowed &&
> drm_mode_is_420(&connector->display_info, mode))
> clock /= 2;
This is too early. We want to keep clock as is for checking whether RGB
output is possible with 420_also modes.
So the structure you had in your original patch was the correct way to
go about it. Which I think was something along the lines of:
if (420_only)
clock /= 2;
status = intel_hdmi_mode_clock_valid()
if (status != OK) {
if (420_only || !420_also || !420_allowed)
return status;
clock /= 2;
status = intel_hdmi_mode_clock_valid()
}
>
> status = intel_hdmi_mode_clock_valid(hdmi, clock, has_hdmi_sink);
> @@ -2119,6 +2119,14 @@ int intel_hdmi_compute_output_format(struct
> intel_encoder *encoder,
> crtc_state->output_format = INTEL_OUTPUT_FORMAT_RGB;
>
> ret = intel_hdmi_compute_clock(encoder, crtc_state);
> + if (ret) {
> + if (crtc_state->output_format != INTEL_OUTPUT_FORMAT_YCBCR420 ||
> + connector->ycbcr_420_allowed ||
> + drm_mode_is_420_also(&connector->display_info,
> adjusted_mode)) {
That needs s/||/&&/ or we flip the conditions around to:
if (ret) {
if (output_format == 420 || !420_allowed || !420_also)
return ret;
output_format = 420;
...
}
which would have the benefit of avoiding the extra indent level.
> + crtc_state->output_format =
> INTEL_OUTPUT_FORMAT_YCBCR420;
> + ret = intel_hdmi_compute_clock(encoder, crtc_state);
> + }
> + }
>
> return ret;
> }
> --
> 2.25.1
--
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
