On Mon, Apr 23, 2018 at 06:44:53AM +0000, Lisovskiy, Stanislav wrote:
>
> > This table is seriously deprecated, because it's unreadable and
> > unmaintainable. Quoting from the docs:
>
> > "Because this table is very unwieldy, do not add any new properties here.
> > Instead document them in a section above."
>
> > We haven't yet moved all the existing properties over to the new layout
> > (patches very much welcome, especially if you spot that an entire area
> > you're touching like HDMI properties isn't moved yet). But the old table
> > is very much not cool to add stuff to.
>
> > Sorry that I didn't notice this earlier, missed it in the diffstat.
>
> >> as optional.
> >> I think it might be a bit misleading, if I add that one to standard
> >> connector properties,
> >> there are probably should be created some HDMI specific property chapter
> >> then, as there
> >> are already plenty of HDMI specific ones(force audio, broadcast rgb,
> >> aspect ratio).
> >>
> >> Shouldn't it then go in a separate patch may be? Because this work is
> >> surely needed, however
> >> goes a bit out of scope of this patch.
>
> > Don't make it worse by adding more unmaintainable and unreadable entries
> > to this table. That's the minimum. Moving all the standard hdmi properties
> > first would obviously be even better. That also makes reviewing easier,
> > since it's clearer what's there already, and how your new thing fits in.
>
> Ok, then I think I will add a new section called HDMI Specific Connector
> Properties
> under Standard Connector Properties and move content type description there.
> I can add another entries there too, however would prefer to do it in a
> separate patch
> as those are not subject of that commit.
Sounds all good.
-Daniel
>
> >
> > Best Regards,
> >
> > Lisovskiy Stanislav
> >
> > > ---
> > > 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" },
> > > + { 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 \
> > > --
> > > 2.17.0
> > >
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > intel-...@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >
> > --
> > Daniel Vetter
> > Software Engineer, Intel Corporation
> > http://blog.ffwll.ch
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
