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+ker...@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@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Reply via email to