On 04/19/18 14:38, StanLis wrote: > From: Stanislav Lisovskiy <stanislav.lisovs...@intel.com> > > Added content_type property to drm_connector_state > in order to properly handle external HDMI TV content-type setting. > > v2: > * Moved helper function which attaches content type property > to the drm core, as was suggested. > Removed redundant connector state initialization. > > v3: > * Removed caps in drm_content_type_enum_list. > After some discussion it turned out that HDMI Spec 1.4 > was wrongly assuming that IT Content(itc) bit doesn't affect > Content type states, however itc bit needs to be manupulated > as well. In order to not expose additional property for itc, > for sake of simplicity it was decided to bind those together > in same "content type" property. > > v4: > * Added it_content checking in intel_digital_connector_atomic_check. > Fixed documentation for new content type enum. > > v5: > * Moved patch revision's description to commit messages. > > Signed-off-by: Stanislav Lisovskiy <stanislav.lisovs...@intel.com> > --- > Documentation/gpu/kms-properties.csv | 1 + > drivers/gpu/drm/drm_atomic.c | 17 ++++++++++ > drivers/gpu/drm/drm_connector.c | 51 ++++++++++++++++++++++++++++ > drivers/gpu/drm/drm_edid.c | 2 ++ > include/drm/drm_connector.h | 18 ++++++++++ > include/drm/drm_mode_config.h | 5 +++ > include/uapi/drm/drm_mode.h | 7 ++++ > 7 files changed, 101 insertions(+) > > diff --git a/Documentation/gpu/kms-properties.csv > b/Documentation/gpu/kms-properties.csv > index 6b28b014cb7d..a91c9211b8d6 100644 > --- a/Documentation/gpu/kms-properties.csv > +++ b/Documentation/gpu/kms-properties.csv > @@ -17,6 +17,7 @@ Owner Module/Drivers,Group,Property Name,Type,Property > Values,Object attached,De > ,Virtual GPU,“suggested X”,RANGE,"Min=0, Max=0xffffffff",Connector,property > to suggest an X offset for a connector > ,,“suggested Y”,RANGE,"Min=0, Max=0xffffffff",Connector,property to suggest > an Y offset for a connector > ,Optional,"""aspect ratio""",ENUM,"{ ""None"", ""4:3"", ""16:9"" > }",Connector,TDB > +,Optional,"""content type""",ENUM,"{ ""No data"", ""Graphics"", ""Photo"", > ""Cinema"", ""Game"" }",Connector,TBD > i915,Generic,"""Broadcast RGB""",ENUM,"{ ""Automatic"", ""Full"", ""Limited > 16:235"" }",Connector,"When this property is set to Limited 16:235 and CTM is > set, the hardware will be programmed with the result of the multiplication of > CTM by the limited range matrix to ensure the pixels normaly in the range > 0..1.0 are remapped to the range 16/255..235/255." > ,,“audio”,ENUM,"{ ""force-dvi"", ""off"", ""auto"", ""on"" }",Connector,TBD > ,SDVO-TV,“mode”,ENUM,"{ ""NTSC_M"", ""NTSC_J"", ""NTSC_443"", ""PAL_B"" } > etc.",Connector,TBD > diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c > index 7d25c42f22db..479499f5848e 100644 > --- a/drivers/gpu/drm/drm_atomic.c > +++ b/drivers/gpu/drm/drm_atomic.c > @@ -1266,6 +1266,15 @@ static int drm_atomic_connector_set_property(struct > drm_connector *connector, > state->link_status = val; > } else if (property == config->aspect_ratio_property) { > state->picture_aspect_ratio = val; > + } else if (property == config->content_type_property) { > + /* > + * Lowest two bits of content_type property control > + * content_type, bit 2 controls itc bit. > + * It was decided to have a single property called > + * content_type, instead of content_type and itc. > + */ > + state->content_type = val & 3; > + state->it_content = val >> 2; > } else if (property == connector->scaling_mode_property) { > state->scaling_mode = val; > } else if (property == connector->content_protection_property) { > @@ -1351,6 +1360,14 @@ drm_atomic_connector_get_property(struct drm_connector > *connector, > *val = state->link_status; > } else if (property == config->aspect_ratio_property) { > *val = state->picture_aspect_ratio; > + } else if (property == config->content_type_property) { > + /* > + * Lowest two bits of content_type property control > + * content_type, bit 2 controls itc bit. > + * It was decided to have a single property called > + * content_type, instead of content_type and itc. > + */ > + *val = state->content_type | (state->it_content << 2); > } else if (property == connector->scaling_mode_property) { > *val = state->scaling_mode; > } else if (property == connector->content_protection_property) { > diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c > index b3cde897cd80..757a0712f7c1 100644 > --- a/drivers/gpu/drm/drm_connector.c > +++ b/drivers/gpu/drm/drm_connector.c > @@ -720,6 +720,14 @@ static const struct drm_prop_enum_list > drm_aspect_ratio_enum_list[] = { > { DRM_MODE_PICTURE_ASPECT_16_9, "16:9" }, > }; > > +static const struct drm_prop_enum_list drm_content_type_enum_list[] = { > + { DRM_MODE_CONTENT_TYPE_NO_DATA, "No data" },
Shouldn't this be "No Data"? At least, I see "Upside Down" below, so I assume each word should start with upper case. > + { DRM_MODE_CONTENT_TYPE_GRAPHICS, "Graphics" }, > + { DRM_MODE_CONTENT_TYPE_PHOTO, "Photo" }, > + { DRM_MODE_CONTENT_TYPE_CINEMA, "Cinema" }, > + { DRM_MODE_CONTENT_TYPE_GAME, "Game" }, > +}; > + > static const struct drm_prop_enum_list drm_panel_orientation_enum_list[] = { > { DRM_MODE_PANEL_ORIENTATION_NORMAL, "Normal" }, > { DRM_MODE_PANEL_ORIENTATION_BOTTOM_UP, "Upside Down" }, > @@ -996,6 +1004,22 @@ int drm_mode_create_dvi_i_properties(struct drm_device > *dev) > } > EXPORT_SYMBOL(drm_mode_create_dvi_i_properties); > > +/** > + * drm_connector_attach_content_type_property - attach content-type property > + * @dev: DRM device > + * > + * Called by a driver the first time a HDMI connector is made. > + */ > +int drm_connector_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_NO_DATA); > + return 0; > +} > +EXPORT_SYMBOL(drm_connector_attach_content_type_property); > + > /** > * drm_create_tv_properties - create TV specific connector properties > * @dev: DRM device > @@ -1260,6 +1284,33 @@ int drm_mode_create_aspect_ratio_property(struct > drm_device *dev) > } > EXPORT_SYMBOL(drm_mode_create_aspect_ratio_property); > > +/** > + * drm_mode_create_content_type_property - create content type property > + * @dev: DRM device > + * > + * Called by a driver the first time it's needed, must be attached to desired > + * connectors. > + * > + * Returns: > + * Zero on success, negative errno on failure. > + */ > +int drm_mode_create_content_type_property(struct drm_device *dev) > +{ > + if (dev->mode_config.content_type_property) > + return 0; > + > + dev->mode_config.content_type_property = > + drm_property_create_enum(dev, 0, "content type", > + drm_content_type_enum_list, > + ARRAY_SIZE(drm_content_type_enum_list)); > + > + if (dev->mode_config.content_type_property == NULL) > + return -ENOMEM; > + > + return 0; > +} > +EXPORT_SYMBOL(drm_mode_create_content_type_property); > + > /** > * drm_mode_create_suggested_offset_properties - create suggests offset > properties > * @dev: DRM device > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c > index 134069f36482..b1b7f9683e34 100644 > --- a/drivers/gpu/drm/drm_edid.c > +++ b/drivers/gpu/drm/drm_edid.c > @@ -4867,6 +4867,8 @@ drm_hdmi_avi_infoframe_from_display_mode(struct > hdmi_avi_infoframe *frame, > } > > frame->picture_aspect = HDMI_PICTURE_ASPECT_NONE; > + frame->content_type = HDMI_CONTENT_TYPE_GRAPHICS; > + frame->itc = 0; > > /* > * Populate picture aspect ratio from either > diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h > index 675cc3f8cf85..1fedab750f09 100644 > --- a/include/drm/drm_connector.h > +++ b/include/drm/drm_connector.h > @@ -418,6 +418,22 @@ struct drm_connector_state { > */ > 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 lowest 2 bits much > + * match the values for &enum hdmi_content_type > + */ > + enum hdmi_content_type content_type; > + > + /** > + * @it_content: Connector property to control the > + * HDMI infoframe it content setting(used with content type). > + * The %DRM_MODE_CONTENT_TYPE_\* values bit 2 > + * match the values of it_content > + */ > + bool it_content; > + > /** > * @scaling_mode: Connector property to control the > * upscaling, mostly used for built-in panels. > @@ -1089,11 +1105,13 @@ int drm_mode_create_tv_properties(struct drm_device > *dev, > unsigned int num_modes, > const char * const modes[]); > int drm_mode_create_scaling_mode_property(struct drm_device *dev); > +int drm_connector_attach_content_type_property(struct drm_connector *dev); > int drm_connector_attach_scaling_mode_property(struct drm_connector > *connector, > u32 scaling_mode_mask); > int drm_connector_attach_content_protection_property( > struct drm_connector *connector); > int drm_mode_create_aspect_ratio_property(struct drm_device *dev); > +int drm_mode_create_content_type_property(struct drm_device *dev); > int drm_mode_create_suggested_offset_properties(struct drm_device *dev); > > int drm_mode_connector_set_path_property(struct drm_connector *connector, > diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h > index 33b3a96d66d0..fb45839179dd 100644 > --- a/include/drm/drm_mode_config.h > +++ b/include/drm/drm_mode_config.h > @@ -726,6 +726,11 @@ struct drm_mode_config { > * HDMI infoframe aspect ratio setting. > */ > struct drm_property *aspect_ratio_property; > + /** > + * @content_type_property: Optional connector property to control the > + * HDMI infoframe content type setting. > + */ > + struct drm_property *content_type_property; > /** > * @degamma_lut_property: Optional CRTC property to set the LUT used to > * convert the framebuffer's colors to linear gamma. > diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h > index 50bcf4214ff9..3c234bfa80b9 100644 > --- a/include/uapi/drm/drm_mode.h > +++ b/include/uapi/drm/drm_mode.h > @@ -94,6 +94,13 @@ extern "C" { > #define DRM_MODE_PICTURE_ASPECT_4_3 1 > #define DRM_MODE_PICTURE_ASPECT_16_9 2 > > +/* HDMI content type and itc bit bound together for simplicity */ > +#define DRM_MODE_CONTENT_TYPE_NO_DATA 0 > +#define DRM_MODE_CONTENT_TYPE_GRAPHICS 4 > +#define DRM_MODE_CONTENT_TYPE_PHOTO 5 > +#define DRM_MODE_CONTENT_TYPE_CINEMA 6 > +#define DRM_MODE_CONTENT_TYPE_GAME 7 > + > /* Aspect ratio flag bitmask (4 bits 22:19) */ > #define DRM_MODE_FLAG_PIC_AR_MASK (0x0F<<19) > #define DRM_MODE_FLAG_PIC_AR_NONE \ > Other than that this looks good to me. Acked-by: Hans Verkuil <hans.verk...@cisco.com> Regards, Hans _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel