Hi Alex,
Thanks for working on this!
On Wed, Dec 13, 2023 at 08:51:18PM +0100, Alex Bee wrote:
> The display controller will always give full range RGB regardless of the
> mode set, but HDMI requires certain modes to be transmitted in limited
> range RGB. This is especially required for HDMI sinks which do not support
> non-standard quantization ranges.
>
> This enables color space conversion for those modes and sets the
> quantization range accordingly in the AVI infoframe.
>
> Fixes: 412d4ae6b7a5 ("drm/rockchip: hdmi: add Innosilicon HDMI support")
> Signed-off-by: Alex Bee <knaerzche@xxxxxxxxx>
> ---
> drivers/gpu/drm/rockchip/inno_hdmi.c | 40 ++++++++++++++++++++++------
> 1 file changed, 32 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/rockchip/inno_hdmi.c b/drivers/gpu/drm/rockchip/inno_hdmi.c
> index 345253e033c5..32626a75723c 100644
> --- a/drivers/gpu/drm/rockchip/inno_hdmi.c
> +++ b/drivers/gpu/drm/rockchip/inno_hdmi.c
> @@ -33,6 +33,7 @@ struct hdmi_data_info {
> unsigned int enc_in_format;
> unsigned int enc_out_format;
> unsigned int colorimetry;
> + bool rgb_limited_range;
> };
>
> struct inno_hdmi_i2c {
> @@ -308,6 +309,18 @@ static int inno_hdmi_config_video_avi(struct inno_hdmi *hdmi,
> else
> frame.avi.colorspace = HDMI_COLORSPACE_RGB;
>
> + if (hdmi->hdmi_data.enc_out_format == HDMI_COLORSPACE_RGB) {
> + drm_hdmi_avi_infoframe_quant_range(&frame.avi,
> + &hdmi->connector, mode,
> + hdmi->hdmi_data.rgb_limited_range ?
> + HDMI_QUANTIZATION_RANGE_LIMITED :
> + HDMI_QUANTIZATION_RANGE_FULL);
> + } else {
> + frame.avi.quantization_range = HDMI_QUANTIZATION_RANGE_DEFAULT;
> + frame.avi.ycc_quantization_range =
> + HDMI_YCC_QUANTIZATION_RANGE_LIMITED;
> + }
> +
> return inno_hdmi_upload_frame(hdmi, rc, &frame, INFOFRAME_AVI, 0, 0, 0);
> }
>
> @@ -334,14 +347,22 @@ static int inno_hdmi_config_video_csc(struct inno_hdmi *hdmi)
> if (data->enc_in_format == data->enc_out_format) {
> if ((data->enc_in_format == HDMI_COLORSPACE_RGB) ||
> (data->enc_in_format >= HDMI_COLORSPACE_YUV444)) {
> - value = v_SOF_DISABLE | v_COLOR_DEPTH_NOT_INDICATED(1);
> - hdmi_writeb(hdmi, HDMI_VIDEO_CONTRL3, value);
> -
> - hdmi_modb(hdmi, HDMI_VIDEO_CONTRL,
> - m_VIDEO_AUTO_CSC | m_VIDEO_C0_C2_SWAP,
> - v_VIDEO_AUTO_CSC(AUTO_CSC_DISABLE) |
> - v_VIDEO_C0_C2_SWAP(C0_C2_CHANGE_DISABLE));
> - return 0;
> + if (data->enc_in_format == HDMI_COLORSPACE_RGB &&
> + data->enc_out_format == HDMI_COLORSPACE_RGB &&
> + hdmi->hdmi_data.rgb_limited_range) {
> + csc_mode = CSC_RGB_0_255_TO_RGB_16_235_8BIT;
> + auto_csc = AUTO_CSC_DISABLE;
> + c0_c2_change = C0_C2_CHANGE_DISABLE;
> + csc_enable = v_CSC_ENABLE;
> + } else {
> + value = v_SOF_DISABLE | v_COLOR_DEPTH_NOT_INDICATED(1);
> + hdmi_writeb(hdmi, HDMI_VIDEO_CONTRL3, value);
> + hdmi_modb(hdmi, HDMI_VIDEO_CONTRL,
> + m_VIDEO_AUTO_CSC | m_VIDEO_C0_C2_SWAP,
> + v_VIDEO_AUTO_CSC(AUTO_CSC_DISABLE) |
> + v_VIDEO_C0_C2_SWAP(C0_C2_CHANGE_DISABLE));
> + return 0;
> + }
> }
> }
>
> @@ -458,6 +479,9 @@ static int inno_hdmi_setup(struct inno_hdmi *hdmi,
> else
> hdmi->hdmi_data.colorimetry = HDMI_COLORIMETRY_ITU_709;
>
> + hdmi->hdmi_data.rgb_limited_range =
> + drm_default_rgb_quant_range(mode) == HDMI_QUANTIZATION_RANGE_LIMITED;
> +
This patch conflicts heavily with my inno_hdmi patches here (patches 22 to 38):
https://lore.kernel.org/dri-devel/20231207-kms-hdmi-connector-state-v5-0-6538e19d634d@xxxxxxxxxx/
I would appreciate if you could test and merge them into your series.
In particular, there's no need to store the range here: enc_out_format
is always RGB, so you'll always end up calling
drm_hdmi_avi_infoframe_quant_range(), and you'll always have the same csc values.
Maxime
Attachment:
signature.asc
Description: PGP signature
