On Thu, Dec 14, 2023 at 12:17:34PM +0100, Heiko Stübner wrote: > Am Donnerstag, 14. Dezember 2023, 12:12:08 CET schrieb Alex Bee: > > Hi Maxime > > > > Am 14.12.23 um 08:56 schrieb Maxime Ripard: > > > 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'm aware of that and I mentioned it in the cover letter. Your series is > > not merged yet and it didn't get much feedback so far. What is the > > status there? Especially because you are removing things from inno-hdmi > > driver (which aren't really required to remove there) which will I have > > to reintroduce. > > Sadly I haven't found the time to look closer at Maxime's series so far, > but I got the impression that it separates into multiple cleanup steps > for a number of controllers. Yeah, one of the previous version comment was to support more controllers than vc4, which is fair. So I ended up reworking and converting multiple controllers, but most of the clean up changes can be applied outside of that series just fine. I just didn't find someone to test / review them yet :) Maxime
Attachment:
signature.asc
Description: PGP signature