[PATCH 5/5] drm/i2c: tda998x: improve correctness of quantisation range

[Brian Starkey]

Hi Russell,

On Fri, Jan 25, 2019 at 09:43:29AM +0000, Russell King wrote:
> CEA-861 says: "A Source shall not send a non-zero Q value that does
> not correspond to the default RGB Quantization Range for the
> transmitted Picture unless the Sink indicates support for the Q bit
> in a Video Capabilities Data Block."
> 
> Make TDA998x compliant by using the helper to set the quantisation
> range in the infoframe, and using the TDA998x's colour scaling to
> appropriately adjust the RGB values sent to the monitor.
> 
> This ensures that monitors that do not support the Q bit are sent
> RGB values that are within the expected range.  Monitors with
> support for the Q bit will be sent full-range RGB.
> 
> Signed-off-by: Russell King <rmk+kernel at armlinux.org.uk>
> ---
>  drivers/gpu/drm/i2c/tda998x_drv.c | 39 ++++++++++++++++++++++++++++++++++-----
>  1 file changed, 34 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c
> index b0ed2ef49c62..7d9aea79aff2 100644
> --- a/drivers/gpu/drm/i2c/tda998x_drv.c
> +++ b/drivers/gpu/drm/i2c/tda998x_drv.c
> @@ -50,6 +50,8 @@ struct tda998x_priv {
>  	bool is_on;
>  	bool supports_infoframes;
>  	bool sink_has_audio;
> +	bool has_rgb_quant;
> +	enum hdmi_quantization_range rgb_quant_range;
>  	u8 vip_cntrl_0;
>  	u8 vip_cntrl_1;
>  	u8 vip_cntrl_2;
> @@ -869,7 +871,9 @@ tda998x_write_avi(struct tda998x_priv *priv, const struct drm_display_mode *mode
>  
>  	drm_hdmi_avi_infoframe_from_display_mode(&frame.avi,
>  						 &priv->connector, mode);
> -	frame.avi.quantization_range = HDMI_QUANTIZATION_RANGE_FULL;
> +	drm_hdmi_avi_infoframe_quant_range(&frame.avi, mode,
> +					   priv->rgb_quant_range,
> +					   priv->has_rgb_quant, false);
>  
>  	tda998x_write_if(priv, DIP_IF_FLAGS_IF2, REG_IF2_HB0, &frame);
>  }
> @@ -1259,6 +1263,7 @@ static int tda998x_connector_get_modes(struct drm_connector *connector)
>  	mutex_lock(&priv->audio_mutex);
>  	n = drm_add_edid_modes(connector, edid);
>  	priv->sink_has_audio = drm_detect_monitor_audio(edid);
> +	priv->has_rgb_quant = drm_rgb_quant_range_selectable(edid);
>  	mutex_unlock(&priv->audio_mutex);
>  
>  	kfree(edid);
> @@ -1385,6 +1390,15 @@ static void tda998x_bridge_mode_set(struct drm_bridge *bridge,
>  	u8 reg, div, rep, sel_clk;
>  
>  	/*
> +	 * Since we are "computer" like, our source invariably produces
> +	 * full-range RGB.  If the monitor supports full-range, then use
> +	 * it, otherwise reduce to limited-range.
> +	 */
> +	priv->rgb_quant_range = priv->has_rgb_quant ?
> +		HDMI_QUANTIZATION_RANGE_FULL :
> +		drm_default_rgb_quant_range(adjusted_mode);
> +
> +	/*
>  	 * Internally TDA998x is using ITU-R BT.656 style sync but
>  	 * we get VESA style sync. TDA998x is using a reference pixel
>  	 * relative to ITU to sync to the input frame and for output
> @@ -1499,10 +1513,25 @@ static void tda998x_bridge_mode_set(struct drm_bridge *bridge,
>  	reg_write(priv, REG_PLL_SERIAL_2, PLL_SERIAL_2_SRL_NOSC(div) |
>  			PLL_SERIAL_2_SRL_PR(rep));
>  
> -	/* set color matrix bypass flag: */
> -	reg_write(priv, REG_MAT_CONTRL, MAT_CONTRL_MAT_BP |
> -				MAT_CONTRL_MAT_SC(1));
> -	reg_set(priv, REG_FEAT_POWERDOWN, FEAT_POWERDOWN_CSC);
> +	/* set color matrix according to output rgb quant range */
> +	if (priv->rgb_quant_range == HDMI_QUANTIZATION_RANGE_LIMITED) {
> +		static u8 tda998x_full_to_limited_range[] = {
> +			MAT_CONTRL_MAT_SC(2),
> +			0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> +			0x03, 0x6f, 0x00, 0x00, 0x00, 0x00,
> +			0x00, 0x00, 0x03, 0x6f, 0x00, 0x00,
> +			0x00, 0x00, 0x00, 0x00, 0x03, 0x6f,
> +			0x00, 0x40, 0x00, 0x40, 0x00, 0x40
> +		};

I couldn't figure out from the datasheet I have what the expected data
bit-depth is here, but I assume from this offset that it's 12-bit?

Should you also set HVF_CNTRL_1_VQR to 0b01 when using limited range?

Cheers,
-Brian

> +		reg_clear(priv, REG_FEAT_POWERDOWN, FEAT_POWERDOWN_CSC);
> +		reg_write_range(priv, REG_MAT_CONTRL,
> +				tda998x_full_to_limited_range,
> +				sizeof(tda998x_full_to_limited_range));
> +	} else {
> +		reg_write(priv, REG_MAT_CONTRL, MAT_CONTRL_MAT_BP |
> +					MAT_CONTRL_MAT_SC(1));
> +		reg_set(priv, REG_FEAT_POWERDOWN, FEAT_POWERDOWN_CSC);
> +	}
>  
>  	/* set BIAS tmds value: */
>  	reg_write(priv, REG_ANA_GENERAL, 0x09);
> -- 
> 2.7.4
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel