Re: [Intel-gfx] [PATCH v3 3/9] drm: Add Content Protection property

[Chris Wilson]

Quoting Daniel Vetter (2017-12-05 15:34:36)
> Two bits missing imo:
> - Should explain that userspace should poll this property to detect a
>   change from ENABLED to DESIRED (and take adequate actions and e.g. stop
>   the stream). No uevent will be sent out because the HDCP specs require
>   polling anyway.

What's more polling than poll()? :-p
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v3 3/9] drm: Add Content Protection property

[Daniel Vetter]

On Tue, Dec 05, 2017 at 12:15:02AM -0500, 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 
> Signed-off-by: Sean Paul 
> 
> [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
> + *

Re: [Intel-gfx] [PATCH v3 3/9] drm: Add Content Protection property

[Daniel Vetter]

On Tue, Dec 05, 2017 at 09:07:58AM +0100, 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 
> > Signed-off-by: Sean Paul 
> > 
> > [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 

Re: [Intel-gfx] [PATCH v3 3/9] drm: Add Content Protection property

[Hans Verkuil]

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 
> Signed-off-by: Sean Paul 
> 
> [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.