[PATCH v3 3/9] drm: Add Content Protection property
Sean Paul
seanpaul at chromium.org
Tue Dec 5 14:36:45 UTC 2017
On Tue, Dec 5, 2017 at 9:04 AM, Ramalingam C <ramalingam.c at intel.com> wrote:
>
>
> On Tuesday 05 December 2017 01:37 PM, Hans Verkuil wrote:
>
> On 12/05/2017 06:15 AM, Sean Paul wrote:
>
> This patch adds a new optional connector property to allow userspace to
> enable
> protection over the content it is displaying. This will typically be
> implemented
> by the driver using HDCP.
>
> The property is a tri-state with the following values:
> - OFF: Self explanatory, no content protection
> - DESIRED: Userspace requests that the driver enable protection
> - ENABLED: Once the driver has authenticated the link, it sets this value
>
> The driver is responsible for downgrading ENABLED to DESIRED if the link
> becomes
> unprotected. The driver should also maintain the desiredness of protection
> across hotplug/dpms/suspend.
>
> If this looks familiar, I posted [1] this 3 years ago. We have been using
> this
> in ChromeOS across exynos, mediatek, and rockchip over that time.
>
> Changes in v2:
> - Pimp kerneldoc for content_protection_property (Daniel)
> - Drop sysfs attribute
> Changes in v3:
> - None
>
> Cc: Daniel Vetter <daniel.vetter at intel.com>
> Signed-off-by: Sean Paul <seanpaul at chromium.org>
>
> [1]
> https://lists.freedesktop.org/archives/dri-devel/2014-December/073336.html
> ---
> drivers/gpu/drm/drm_atomic.c | 8 +++++
> drivers/gpu/drm/drm_connector.c | 71
> +++++++++++++++++++++++++++++++++++++++++
> drivers/gpu/drm/drm_sysfs.c | 1 +
> include/drm/drm_connector.h | 16 ++++++++++
> include/uapi/drm/drm_mode.h | 4 +++
> 5 files changed, 100 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index c2da5585e201..676025d755b2 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -1196,6 +1196,12 @@ static int drm_atomic_connector_set_property(struct
> drm_connector *connector,
> state->picture_aspect_ratio = val;
> } else if (property == connector->scaling_mode_property) {
> state->scaling_mode = val;
> + } else if (property == connector->content_protection_property) {
> + if (val == DRM_MODE_CONTENT_PROTECTION_ENABLED) {
> + DRM_DEBUG_KMS("only drivers can set CP Enabled\n");
> + return -EINVAL;
> + }
> + state->content_protection = val;
> } else if (connector->funcs->atomic_set_property) {
> return connector->funcs->atomic_set_property(connector,
> state, property, val);
> @@ -1275,6 +1281,8 @@ drm_atomic_connector_get_property(struct drm_connector
> *connector,
> *val = state->picture_aspect_ratio;
> } else if (property == connector->scaling_mode_property) {
> *val = state->scaling_mode;
> + } else if (property == connector->content_protection_property) {
> + *val = state->content_protection;
> } else if (connector->funcs->atomic_get_property) {
> return connector->funcs->atomic_get_property(connector,
> state, property, val);
> diff --git a/drivers/gpu/drm/drm_connector.c
> b/drivers/gpu/drm/drm_connector.c
> index f14b48e6e839..8626aa8f485e 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -698,6 +698,13 @@ static const struct drm_prop_enum_list
> drm_tv_subconnector_enum_list[] = {
> DRM_ENUM_NAME_FN(drm_get_tv_subconnector_name,
> drm_tv_subconnector_enum_list)
>
> +static struct drm_prop_enum_list drm_cp_enum_list[] = {
> + { DRM_MODE_CONTENT_PROTECTION_OFF, "Off" },
> + { DRM_MODE_CONTENT_PROTECTION_DESIRED, "Desired" },
> + { DRM_MODE_CONTENT_PROTECTION_ENABLED, "Enabled" },
> +};
> +DRM_ENUM_NAME_FN(drm_get_content_protection_name, drm_cp_enum_list)
> +
> /**
> * DOC: standard connector properties
> *
> @@ -764,6 +771,34 @@ DRM_ENUM_NAME_FN(drm_get_tv_subconnector_name,
> * after modeset, the kernel driver may set this to "BAD" and issue a
> * hotplug uevent. Drivers should update this value using
> * drm_mode_connector_set_link_status_property().
> + * Content Protection:
> + * This property is used by userspace to request the kernel protect future
> + * content communicated over the link. When requested, kernel will apply
> + * the appropriate means of protection (most often HDCP), and use the
> + * property to tell userspace the protection is active.
> + *
> + * The value of this property can be one of the following:
> + *
> + * - DRM_MODE_CONTENT_PROTECTION_OFF = 0
> + * The link is not protected, content is transmitted in the clear.
> + * - DRM_MODE_CONTENT_PROTECTION_DESIRED = 1
> + * Userspace has requested content protection, but the link is not
> + * currently protected. When in this state, kernel should enable
> + * Content Protection as soon as possible.
> + * - DRM_MODE_CONTENT_PROTECTION_ENABLED = 2
> + * Userspace has requested content protection, and the link is
> + * protected. Only the driver can set the property to this value.
> + * If userspace attempts to set to ENABLED, kernel will return
> + * -EINVAL.
> + *
> + * A few guidelines:
> + *
> + * - DESIRED state should be preserved until userspace de-asserts it by
> + * setting the property to OFF. This means ENABLED should only transition
> + * to OFF when the user explicitly requests it.
> + * - If the state is DESIRED, kernel should attempt to re-authenticate the
> + * link whenever possible. This includes across disable/enable, dpms,
> + * hotplug, downstream device changes, link status failures, etc..
> *
> * Connectors also have one standardized atomic property:
> *
> @@ -1047,6 +1082,42 @@ int drm_connector_attach_scaling_mode_property(struct
> drm_connector *connector,
> }
> EXPORT_SYMBOL(drm_connector_attach_scaling_mode_property);
>
> +/**
> + * drm_connector_attach_content_protection_property - attach content
> protection
> + * property
> + *
> + * @connector: connector to attach CP property on.
> + *
> + * This is used to add support for content protection on select connectors.
> + * Content Protection is intentionally vague to allow for different
> underlying
> + * technologies, however it is most implemented by HDCP.
> + *
> + * The content protection will be set to
> &drm_connector_state.content_protection
> + *
> + * Returns:
> + * Zero on success, negative errno on failure.
> + */
> +int drm_connector_attach_content_protection_property(
> + struct drm_connector *connector)
> +{
> + struct drm_device *dev = connector->dev;
> + struct drm_property *prop;
> +
> + prop = drm_property_create_enum(dev, 0, "Content Protection",
> + drm_cp_enum_list,
> + ARRAY_SIZE(drm_cp_enum_list));
> + if (!prop)
> + return -ENOMEM;
> +
> + drm_object_attach_property(&connector->base, prop,
> + DRM_MODE_CONTENT_PROTECTION_OFF);
> +
> + connector->content_protection_property = prop;
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(drm_connector_attach_content_protection_property);
> +
> /**
> * drm_mode_create_aspect_ratio_property - create aspect ratio property
> * @dev: DRM device
> diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c
> index 1c5b5ce1fd7f..2385c7e0bef5 100644
> --- a/drivers/gpu/drm/drm_sysfs.c
> +++ b/drivers/gpu/drm/drm_sysfs.c
> @@ -21,6 +21,7 @@
> #include <drm/drm_sysfs.h>
> #include <drm/drmP.h>
> #include "drm_internal.h"
> +#include "drm_crtc_internal.h"
>
> #define to_drm_minor(d) dev_get_drvdata(d)
> #define to_drm_connector(d) dev_get_drvdata(d)
> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> index 7a7140543012..828878addd03 100644
> --- a/include/drm/drm_connector.h
> +++ b/include/drm/drm_connector.h
> @@ -370,6 +370,12 @@ struct drm_connector_state {
> * upscaling, mostly used for built-in panels.
> */
> unsigned int scaling_mode;
> +
> + /**
> + * @content_protection: Connector property to request content
> + * protection. This is most commonly used for HDCP.
> + */
> + unsigned int content_protection;
> };
>
> /**
> @@ -718,6 +724,7 @@ struct drm_cmdline_mode {
> * @tile_h_size: horizontal size of this tile.
> * @tile_v_size: vertical size of this tile.
> * @scaling_mode_property: Optional atomic property to control the
> upscaling.
> + * @content_protection_property: Optional property to control content
> protection
> *
> * Each connector may be connected to one or more CRTCs, or may be clonable
> by
> * another connector if they can share a CRTC. Each connector also has a
> specific
> @@ -808,6 +815,12 @@ struct drm_connector {
>
> struct drm_property *scaling_mode_property;
>
> + /**
> + * @content_protection_property: DRM ENUM property for content
> + * protection
> + */
> + struct drm_property *content_protection_property;
> +
> /**
> * @path_blob_ptr:
> *
> @@ -1002,6 +1015,7 @@ const char *drm_get_dvi_i_subconnector_name(int val);
> 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);
>
> int drm_mode_create_dvi_i_properties(struct drm_device *dev);
> int drm_mode_create_tv_properties(struct drm_device *dev,
> @@ -1010,6 +1024,8 @@ int drm_mode_create_tv_properties(struct drm_device
> *dev,
> int drm_mode_create_scaling_mode_property(struct drm_device *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_suggested_offset_properties(struct drm_device *dev);
>
> diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
> index 5597a87154e5..03f4d22305c2 100644
> --- a/include/uapi/drm/drm_mode.h
> +++ b/include/uapi/drm/drm_mode.h
> @@ -173,6 +173,10 @@ extern "C" {
> DRM_MODE_REFLECT_X | \
> DRM_MODE_REFLECT_Y)
>
> +/* Content Protection Flags */
> +#define DRM_MODE_CONTENT_PROTECTION_OFF 0
> +#define DRM_MODE_CONTENT_PROTECTION_DESIRED 1
> +#define DRM_MODE_CONTENT_PROTECTION_ENABLED 2
>
> What about HDCP 1.4 and HDCP 2.2? Userspace would need to know which version
> was negotiated since content protected 4k videos require HDCP 2.2. Perhaps
> provide a property with the HDCP version?
>
> I'm also missing a method for userspace to read the BKSV from the
> transmitter.
>
> Hans,
>
> I guess you are asking about the use case explained at
> http://www.spinics.net/lists/intel-gfx/msg134813.html
>
> Sean,
> As this solution is only for 1.4, this version requirement might not matter
> here.
> But as a community could we at least ack such a need, so that inevitable
> extension of this uAPI will be well thought about?
>
> As we discussed before Enum values required are
>
> -OFF : Disable any content protection
> -DESIRED : For any possible HDCP protection (v1.4/2.2)
> -DESIRED_TYPE1 : For HDCP2.2 only
> -ENABLED : Highest HDCP protection is enabled (Could be v1.4/2.2)
> -ENABLED_TYPE1 : HDCP2.2 enabled
>
I'd rather keep the property as-is and expose an HDCP version property
alongside it (or perhaps something more elaborate that includes bksv
and the downstream bksvs). The reason I prefer that is it will also
cover the 1.2 vs 1.4 difference that is a more immediate need.
This property is intentionally vague about the underlying encryption,
and tying it to HDCP (and specific versions, at that) is not
consistent with the design.
> And another gap i am seeing with this uAPI is that there is no communication
> back about the
> HDCP authentication failure, as DESIRED will remain without transitioning
> into ENABLED.
> So userspace has to identify the failure of the HDCP req with polling and
> Timeout.
> Timeout also will vary with system to system, as
> with big downstream topology(worst case 127 devices with 7depth)
> authentication could take 5+ Sec.
> with only receiver < few 100 mSec(~200mSec?)
>
It's worked on CrOS for a number of years now, why change what isn't broken?
Sean
>
> So could it help userspace if we could indicate the authentication failure.
> Agreed that runtime link integrity lost is indicated by the ENABLED->DESIRED
> transition.
>
> --Ram
>
>
> Regards,
>
> Hans
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
>
>
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
More information about the dri-devel
mailing list