On 04/18/18 14:01, Lisovskiy, Stanislav wrote: > Hi, > > Please see my reply inline: > ________________________________________ > From: dri-devel [[email protected]] on behalf of Hans > Verkuil [[email protected]] > Sent: Wednesday, April 18, 2018 2:35 PM > To: Lisovskiy, Stanislav; [email protected] > Cc: [email protected] > Subject: Re: [PATCH v2 2/2] i915: content-type property for HDMI connector > > On 04/18/18 18:51, StanLis wrote: >> From: Stanislav Lisovskiy <[email protected]> >> >> 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 <[email protected]> >> --- >> 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 > [email protected] > https://lists.freedesktop.org/mailman/listinfo/dri-devel > _______________________________________________ dri-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/dri-devel
