On Fri, Mar 22, 2019 at 06:14:40AM +0530, Ramalingam C wrote:
> This patch adds a DRM ENUM property to the selected connectors.
> This property is used for mentioning the protected content's type
> from userspace to kernel HDCP authentication.
>
> Type of the stream is decided by the protected content providers.
> Type 0 content can be rendered on any HDCP protected display wires.
> But Type 1 content can be rendered only on HDCP2.2 protected paths.
>
> So when a userspace sets this property to Type 1 and starts the HDCP
> enable, kernel will honour it only if HDCP2.2 authentication is through
> for type 1. Else HDCP enable will be failed.
>
> v2:
> cp_content_type is replaced with content_protection_type [daniel]
> check at atomic_set_property is removed [Maarten]
> v3:
> %s/content_protection_type/hdcp_content_type [Pekka]
>
> Signed-off-by: Ramalingam C <ramalingam.c@xxxxxxxxx>
> ---
> drivers/gpu/drm/drm_atomic_uapi.c | 4 ++
> drivers/gpu/drm/drm_connector.c | 63 +++++++++++++++++++++++++++++++
> include/drm/drm_connector.h | 15 ++++++++
> include/uapi/drm/drm_mode.h | 4 ++
> 4 files changed, 86 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
> index 4eb81f10bc54..857ca6fa6fd0 100644
> --- a/drivers/gpu/drm/drm_atomic_uapi.c
> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> @@ -746,6 +746,8 @@ static int drm_atomic_connector_set_property(struct drm_connector *connector,
> return -EINVAL;
> }
> state->content_protection = val;
> + } else if (property == connector->hdcp_content_type_property) {
> + state->hdcp_content_type = val;
> } else if (property == connector->colorspace_property) {
> state->colorspace = val;
> } else if (property == config->writeback_fb_id_property) {
> @@ -822,6 +824,8 @@ drm_atomic_connector_get_property(struct drm_connector *connector,
> *val = state->scaling_mode;
> } else if (property == connector->content_protection_property) {
> *val = state->content_protection;
> + } else if (property == connector->hdcp_content_type_property) {
> + *val = state->hdcp_content_type;
> } else if (property == config->writeback_fb_id_property) {
> /* Writeback framebuffer is one-shot, write and forget */
> *val = 0;
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index 2355124849db..ff61c3208307 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -857,6 +857,13 @@ static const struct drm_prop_enum_list hdmi_colorspaces[] = {
> { DRM_MODE_COLORIMETRY_DCI_P3_RGB_THEATER, "DCI-P3_RGB_Theater" },
> };
>
> +static struct drm_prop_enum_list drm_hdcp_content_type_enum_list[] = {
> + { DRM_MODE_HDCP_CONTENT_TYPE0, "HDCP Type0" },
> + { DRM_MODE_HDCP_CONTENT_TYPE1, "HDCP Type1" },
> +};
> +DRM_ENUM_NAME_FN(drm_get_hdcp_content_type_name,
> + drm_hdcp_content_type_enum_list)
> +
> /**
> * DOC: standard connector properties
> *
> @@ -962,6 +969,23 @@ static const struct drm_prop_enum_list hdmi_colorspaces[] = {
> * the value transitions from ENABLED to DESIRED. This signifies the link
> * is no longer protected and userspace should take appropriate action
> * (whatever that might be).
> + * HDCP Content Type:
> + * This property is used by the userspace to configure the kernel with
> + * upcoming stream's content type. Content Type of a stream is decided by
> + * the owner of the stream, as HDCP Type0 or HDCP Type1.
> + *
> + * The value of the property can be one the below:
> + * - DRM_MODE_HDCP_CONTENT_TYPE0 = 0
> + * HDCP Type0 streams can be transmitted on a link which is
> + * encrypted with HDCP 1.4 or HDCP 2.2.
> + * - DRM_MODE_HDCP_CONTENT_TYPE1 = 1
> + * HDCP Type1 streams can be transmitted on a link which is
> + * encrypted only with HDCP 2.2.
> + *
> + * Please note this content type is introduced at HDCP 2.2 and used in its
> + * authentication process. If content type is changed when
> + * content_protection is not UNDESIRED, then kernel will disable the HDCP
> + * and re-enable with new type in the same atomic commit
> *
> * max bpc:
> * This range property is used by userspace to limit the bit depth. When
> @@ -1551,6 +1575,45 @@ int drm_connector_attach_content_protection_property(
> }
> EXPORT_SYMBOL(drm_connector_attach_content_protection_property);
>
> +/**
> + * drm_connector_attach_hdcp_content_type_property - attach HDCP
> + * content type property
> + *
> + * @connector: connector to attach HDCP content type property on.
> + *
> + * This is used to add support for sending the protected content's stream type
> + * from userspace to kernel on selected connectors. Protected content provider
> + * will decide their type of their content and declare the same to kernel.
> + *
> + * This information will be used during the HDCP 2.2 authentication.
> + *
> + * Content type will be set to &drm_connector_state.hdcp_content_type.
> + *
> + * Returns:
> + * Zero on success, negative errno on failure.
> + */
> +int
> +drm_connector_attach_hdcp_content_type_property(struct drm_connector *
> + connector)
> +{
> + struct drm_device *dev = connector->dev;
> + struct drm_property *prop;
> +
> + prop = drm_property_create_enum(dev, 0, "HDCP Content Type",
> + drm_hdcp_content_type_enum_list,
> + ARRAY_SIZE(
> + drm_hdcp_content_type_enum_list));
If we always create the same property, then the usual pattern is to have a
global one stored in dev->mode_config, created at init time, and only
attach it. You can attach the same property to multiple objects, and all
objects will have distinct values for that property.
drm_property = metadata describing the name/value range/..., _not_ the value itself
I guess would be good to address that with content protection property
too.
Also, not sure we need 2 functions for this, I'd just add a bool
enable_content_type to drm_connector_attach_content_protection_property(),
for a simpler interface in drivers.
> + if (!prop)
> + return -ENOMEM;
> +
> + drm_object_attach_property(&connector->base, prop,
> + DRM_MODE_HDCP_CONTENT_TYPE0);
> + connector->hdcp_content_type_property = prop;
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(drm_connector_attach_hdcp_content_type_property);
> +
> /**
> * drm_mode_create_aspect_ratio_property - create aspect ratio property
> * @dev: DRM device
> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> index c8061992d6cb..f0830985367f 100644
> --- a/include/drm/drm_connector.h
> +++ b/include/drm/drm_connector.h
> @@ -518,6 +518,12 @@ struct drm_connector_state {
> */
> unsigned int content_type;
>
> + /**
> + * @hdcp_content_type: Connector property to pass the type of
> + * protected content. This is most commonly used for HDCP.
> + */
> + unsigned int hdcp_content_type;
> +
> /**
> * @scaling_mode: Connector property to control the
> * upscaling, mostly used for built-in panels.
> @@ -1035,6 +1041,12 @@ struct drm_connector {
> */
> struct drm_property *colorspace_property;
>
> + /**
> + * @hdcp_content_type_property: DRM ENUM property for type of
> + * Protected Content.
> + */
> + struct drm_property *hdcp_content_type_property;
Please move these two next to the existing content protection stuff.
> +
> /**
> * @path_blob_ptr:
> *
> @@ -1294,6 +1306,7 @@ const char *drm_get_dvi_i_select_name(int val);
> const char *drm_get_tv_subconnector_name(int val);
> const char *drm_get_tv_select_name(int val);
> const char *drm_get_content_protection_name(int val);
> +const char *drm_get_hdcp_content_type_name(int val);
>
> int drm_mode_create_dvi_i_properties(struct drm_device *dev);
> int drm_mode_create_tv_margin_properties(struct drm_device *dev);
> @@ -1309,6 +1322,8 @@ int drm_connector_attach_vrr_capable_property(
> struct drm_connector *connector);
> int drm_connector_attach_content_protection_property(
> struct drm_connector *connector);
> +int drm_connector_attach_hdcp_content_type_property(
> + struct drm_connector *connector);
> int drm_mode_create_aspect_ratio_property(struct drm_device *dev);
> int drm_mode_create_colorspace_property(struct drm_connector *connector);
> int drm_mode_create_content_type_property(struct drm_device *dev);
> diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
> index a439c2e67896..44412e8b77cd 100644
> --- a/include/uapi/drm/drm_mode.h
> +++ b/include/uapi/drm/drm_mode.h
> @@ -210,6 +210,10 @@ extern "C" {
> #define DRM_MODE_CONTENT_PROTECTION_DESIRED 1
> #define DRM_MODE_CONTENT_PROTECTION_ENABLED 2
>
> +/* Content Type classification for HDCP2.2 vs others */
> +#define DRM_MODE_HDCP_CONTENT_TYPE0 0
> +#define DRM_MODE_HDCP_CONTENT_TYPE1 1
> +
> struct drm_mode_modeinfo {
> __u32 clock;
> __u16 hdisplay;
Aside from the nits looks all good to me. Ofc we need an r-b/ack from
userspace people on the interface itself.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
