| Message ID | 20180416231659.9333-3-stanislav.lisovskiy@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Apr 17, 2018 at 02:16:59AM +0300, StanLis wrote: > From: Stanislav Lisovskiy <stanislav.lisovskiy@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@intel.com> Hi Stanislav, Thank you for your patch. > --- > drivers/gpu/drm/i915/intel_atomic.c | 1 + > drivers/gpu/drm/i915/intel_drv.h | 1 + > drivers/gpu/drm/i915/intel_hdmi.c | 4 ++++ > drivers/gpu/drm/i915/intel_modes.c | 10 ++++++++++ > 4 files changed, 16 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_drv.h b/drivers/gpu/drm/i915/intel_drv.h > index 5bd2263407b2..07fd7ba21f38 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -1818,6 +1818,7 @@ int intel_ddc_get_modes(struct drm_connector *c, struct i2c_adapter *adapter); > void intel_attach_force_audio_property(struct drm_connector *connector); > void intel_attach_broadcast_rgb_property(struct drm_connector *connector); > void intel_attach_aspect_ratio_property(struct drm_connector *connector); > +void intel_attach_content_type_property(struct drm_connector *connector); > > > /* intel_overlay.c */ > diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c > index ee929f31f7db..cd484276e9b0 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; > + > /* TODO: handle pixel repetition for YCBCR420 outputs */ > intel_write_infoframe(encoder, crtc_state, &frame); > } > @@ -2065,7 +2067,9 @@ 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); > + intel_attach_content_type_property(connector); > connector->state->picture_aspect_ratio = HDMI_PICTURE_ASPECT_NONE; > + connector->state->content_type = HDMI_CONTENT_TYPE_GRAPHICS; This is redudant, the attach function already sets this. > } > > /* > diff --git a/drivers/gpu/drm/i915/intel_modes.c b/drivers/gpu/drm/i915/intel_modes.c > index b39846613e3c..232811ab71a3 100644 > --- a/drivers/gpu/drm/i915/intel_modes.c > +++ b/drivers/gpu/drm/i915/intel_modes.c > @@ -133,3 +133,13 @@ intel_attach_aspect_ratio_property(struct drm_connector *connector) > connector->dev->mode_config.aspect_ratio_property, > DRM_MODE_PICTURE_ASPECT_NONE); > } > + > +void > +intel_attach_content_type_property(struct drm_connector *connector) > +{ > + if (!drm_mode_create_content_type_property(connector->dev)) > + drm_object_attach_property(&connector->base, > + connector->dev->mode_config.content_type_property, > + DRM_MODE_CONTENT_TYPE_GRAPHICS); > +} I think the "in" thing to do is to add this helper to the core, since this is a core property. Sean > + > -- > 2.17.0 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Hi Sean, Thank you for comments! Could you please clarify a bit more here, as I've just started recently working on drm side, so I took an aspect ratio property as an example. > @@ -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; > + > /* TODO: handle pixel repetition for YCBCR420 outputs */ > intel_write_infoframe(encoder, crtc_state, &frame); } @@ -2065,7 > +2067,9 @@ 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); > + intel_attach_content_type_property(connector); > connector->state->picture_aspect_ratio = HDMI_PICTURE_ASPECT_NONE; > + connector->state->content_type = HDMI_CONTENT_TYPE_GRAPHICS; >This is redudant, the attach function already sets this. As you can see aspect ratio is set exactly same way, which is also an HDMI avi info frame property. Also there are actually two different enums: HDMI_CONTENT_TYPE_* and DRM_MODE_CONTENT_TYPE_* i.e: there are one in drm_connector.c: static const struct drm_prop_enum_list drm_aspect_ratio_enum_list[] = { { DRM_MODE_PICTURE_ASPECT_NONE, "Automatic" }, { DRM_MODE_PICTURE_ASPECT_4_3, "4:3" }, { DRM_MODE_PICTURE_ASPECT_16_9, "16:9" }, }; so I added static const struct drm_prop_enum_list drm_content_type_enum_list[] = { { DRM_MODE_CONTENT_TYPE_GRAPHICS, "GRAPHICS" }, { DRM_MODE_CONTENT_TYPE_PHOTO, "PHOTO" }, { DRM_MODE_CONTENT_TYPE_CINEMA, "CINEMA" }, { DRM_MODE_CONTENT_TYPE_GAME, "GAME" }, }; and the one in linux/hdmi.h: enum hdmi_picture_aspect { HDMI_PICTURE_ASPECT_NONE, HDMI_PICTURE_ASPECT_4_3, HDMI_PICTURE_ASPECT_16_9, HDMI_PICTURE_ASPECT_64_27, HDMI_PICTURE_ASPECT_256_135, HDMI_PICTURE_ASPECT_RESERVED, }; enum hdmi_content_type { HDMI_CONTENT_TYPE_GRAPHICS, HDMI_CONTENT_TYPE_PHOTO, HDMI_CONTENT_TYPE_CINEMA, HDMI_CONTENT_TYPE_GAME, }; For some reason the latter enums are used in drm_connector_state, but not the drm_content_type_enum_list(those are actually defined values which simply match): From drm_connector.c: /** * @picture_aspect_ratio: Connector property to control the * HDMI infoframe aspect ratio setting. * * The %DRM_MODE_PICTURE_ASPECT_\* values much match the * values for &enum hdmi_picture_aspect */ enum hdmi_picture_aspect picture_aspect_ratio; /** * @content_type: Connector property to control the * HDMI infoframe content type setting. * * The %DRM_MODE_CONTENT_TYPE_\* values much match the * values for &enum hdmi_content_type */ enum hdmi_content_type content_type; That's why I did it exactly as it is done with aspect ratio. Just want to clarify, as I was assuming this was done for reason. > } > > /* > diff --git a/drivers/gpu/drm/i915/intel_modes.c > b/drivers/gpu/drm/i915/intel_modes.c > index b39846613e3c..232811ab71a3 100644 > --- a/drivers/gpu/drm/i915/intel_modes.c > +++ b/drivers/gpu/drm/i915/intel_modes.c > @@ -133,3 +133,13 @@ intel_attach_aspect_ratio_property(struct drm_connector *connector) > connector->dev->mode_config.aspect_ratio_property, > DRM_MODE_PICTURE_ASPECT_NONE); > } > + > +void > +intel_attach_content_type_property(struct drm_connector *connector) { > + if (!drm_mode_create_content_type_property(connector->dev)) > + drm_object_attach_property(&connector->base, > + connector->dev->mode_config.content_type_property, > + DRM_MODE_CONTENT_TYPE_GRAPHICS); > +} >I think the "in" thing to do is to add this helper to the core, since this is a core property. Could you please explain a bit more, what do you mean by core here? I just thought it is one of HDMI infoframe properties, as stated in spec: https://www.hdmi.org/manufacturer/hdmi_1_4/content_type.aspx Best Regards, Lisovskiy Stanislav
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_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 5bd2263407b2..07fd7ba21f38 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -1818,6 +1818,7 @@ int intel_ddc_get_modes(struct drm_connector *c, struct i2c_adapter *adapter); void intel_attach_force_audio_property(struct drm_connector *connector); void intel_attach_broadcast_rgb_property(struct drm_connector *connector); void intel_attach_aspect_ratio_property(struct drm_connector *connector); +void intel_attach_content_type_property(struct drm_connector *connector); /* intel_overlay.c */ diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c index ee929f31f7db..cd484276e9b0 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; + /* TODO: handle pixel repetition for YCBCR420 outputs */ intel_write_infoframe(encoder, crtc_state, &frame); } @@ -2065,7 +2067,9 @@ 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); + intel_attach_content_type_property(connector); connector->state->picture_aspect_ratio = HDMI_PICTURE_ASPECT_NONE; + connector->state->content_type = HDMI_CONTENT_TYPE_GRAPHICS; } /* diff --git a/drivers/gpu/drm/i915/intel_modes.c b/drivers/gpu/drm/i915/intel_modes.c index b39846613e3c..232811ab71a3 100644 --- a/drivers/gpu/drm/i915/intel_modes.c +++ b/drivers/gpu/drm/i915/intel_modes.c @@ -133,3 +133,13 @@ intel_attach_aspect_ratio_property(struct drm_connector *connector) connector->dev->mode_config.aspect_ratio_property, DRM_MODE_PICTURE_ASPECT_NONE); } + +void +intel_attach_content_type_property(struct drm_connector *connector) +{ + if (!drm_mode_create_content_type_property(connector->dev)) + drm_object_attach_property(&connector->base, + connector->dev->mode_config.content_type_property, + DRM_MODE_CONTENT_TYPE_GRAPHICS); +} +