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

[Hans Verkuil]

On 04/18/18 14:01, Lisovskiy, Stanislav wrote:
> 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?

Yeah, that's wrong in the HDMI Spec. The correct standard to use is the CEA (now CTA)
861. There it clearly defines that the CN bits are only valid if ITC=1.

I know, I'm in the CTA-861 working group :-)

I don't think anyone ever noticed this bug in the HDMI spec before (I didn't). The
2.1 HDMI spec kind of corrects it in Appendix G.1 where it copies the CTA-861-G
text.

Anyway, in V4L2 we implement this with an extra value "NO_ITC" which sets ITC to 0
and sets CN0 and CN1 to 0 as well:

https://hverkuil.home.xs4all.nl/spec/uapi/v4l/extended-controls.html#digital-video-control-ids

The NO_ITC case is usually used for YCbCr video encoding. Note that most displays will
just ignore the content type.

Regards,

	Hans

> 
>>       /* 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
>