[Intel-gfx] [PATCH v2 2/2] i915: content-type property for HDMI connector

[Lisovskiy, Stanislav]

Hi, 

Please see my reply inline:
________________________________________
From: dri-devel [dri-devel-bounces at lists.freedesktop.org] on behalf of Hans Verkuil [hverkuil at xs4all.nl]
Sent: Wednesday, April 18, 2018 2:35 PM
To: Lisovskiy, Stanislav; dri-devel at lists.freedesktop.org
Cc: intel-gfx at lists.freedesktop.org
Subject: Re: [PATCH v2 2/2] i915: content-type property for HDMI connector

On 04/18/18 18:51, StanLis wrote:
> From: Stanislav Lisovskiy <stanislav.lisovskiy at intel.com>
>
> Added encoding of drm content_type property from
> drm_connector_state within AVI infoframe in order to properly handle
> external HDMI TV content-type setting.
>
> Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy at intel.com>
> ---
>  drivers/gpu/drm/i915/intel_atomic.c | 1 +
>  drivers/gpu/drm/i915/intel_hdmi.c   | 3 +++
>  2 files changed, 4 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c
> index 40285d1b91b7..61ddb5871d8a 100644
> --- a/drivers/gpu/drm/i915/intel_atomic.c
> +++ b/drivers/gpu/drm/i915/intel_atomic.c
> @@ -124,6 +124,7 @@ int intel_digital_connector_atomic_check(struct drm_connector *conn,
>       if (new_conn_state->force_audio != old_conn_state->force_audio ||
>           new_conn_state->broadcast_rgb != old_conn_state->broadcast_rgb ||
>           new_conn_state->base.picture_aspect_ratio != old_conn_state->base.picture_aspect_ratio ||
> +         new_conn_state->base.content_type != old_conn_state->base.content_type ||
>           new_conn_state->base.scaling_mode != old_conn_state->base.scaling_mode)
>               crtc_state->mode_changed = true;
>
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index ee929f31f7db..5cc72da9c086 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -491,6 +491,8 @@ static void intel_hdmi_set_avi_infoframe(struct drm_encoder *encoder,
>                                          intel_hdmi->rgb_quant_range_selectable,
>                                          is_hdmi2_sink);
>
> +     frame.avi.content_type = connector->state->content_type;
> +

>Is the ITC bit set in the AVI InfoFrame? The content type bits are only valid if
>the ITC bit is 1. And if there is no content type property, then ITC should be 0.

That's a good question, as I understood it is not set, however the HDMI 1.4 spec
says:

ITC CN1, CN0     Content Type
 0        0,  0          No Data

 1        0,  0          IT content

 X       0,  1          Photo
 X       1,  0          Cinema
 X       1,  1          Game

"X denotes don't care"

So I wonder, should I probably add additional property to control the itc bit?

>       /* TODO: handle pixel repetition for YCBCR420 outputs */
>       intel_write_infoframe(encoder, crtc_state, &frame);
>  }
> @@ -2065,6 +2067,7 @@ intel_hdmi_add_properties(struct intel_hdmi *intel_hdmi, struct drm_connector *c
>       intel_attach_force_audio_property(connector);
>       intel_attach_broadcast_rgb_property(connector);
>       intel_attach_aspect_ratio_property(connector);
> +     drm_connector_attach_content_type_property(connector);
>       connector->state->picture_aspect_ratio = HDMI_PICTURE_ASPECT_NONE;
>  }
>
>

Regards,

        Hans
_______________________________________________
dri-devel mailing list
dri-devel at lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel