Re: [Intel-gfx] [PATCH 01/10] drm: Add CP content type property

2019-03-05 Thread C, Ramalingam

> -Original Message-
> From: Maarten Lankhorst [mailto:maarten.lankho...@linux.intel.com]
> Sent: Tuesday, March 5, 2019 8:09 PM
> To: C, Ramalingam ; intel-gfx@lists.freedesktop.org;
> dri-de...@lists.freedesktop.org; daniel.vet...@ffwll.ch; Shankar, Uma
> 
> Subject: Re: [Intel-gfx] [PATCH 01/10] drm: Add CP content type property
> 
> Op 26-02-2019 om 08:36 schreef Ramalingam C:
> > This patch adds a DRM ENUM property to the selected connectors.
> > This property is used for pass the protected content's type from
> > userspace to kernel HDCP authentication.
> >
> > Type of the stream is decided by the protected content providers as
> > Type 0/1.
> >
> > 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 upon a content protection request with Type 1 as Content type from
> > userspace, Kernel will declare success only if the HDCP2.2
> > authentication is successful.
> >
> > Signed-off-by: Ramalingam C 
> > ---
> >  drivers/gpu/drm/drm_atomic_uapi.c | 10 ++
> >  drivers/gpu/drm/drm_connector.c   | 64
> +++
> >  include/drm/drm_connector.h   | 15 +
> >  include/uapi/drm/drm_mode.h   |  4 +++
> >  4 files changed, 93 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/drm_atomic_uapi.c
> > b/drivers/gpu/drm/drm_atomic_uapi.c
> > index 4eb81f10bc54..5289486565ce 100644
> > --- a/drivers/gpu/drm/drm_atomic_uapi.c
> > +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> > @@ -746,6 +746,14 @@ static int
> drm_atomic_connector_set_property(struct drm_connector *connector,
> > return -EINVAL;
> > }
> > state->content_protection = val;
> > +   } else if (property == connector->cp_content_type_property) {
> > +   if (state->content_protection !=
> > +   DRM_MODE_CONTENT_PROTECTION_UNDESIRED &&
> > +   state->cp_content_type != val) {
> > +   DRM_DEBUG_KMS("Disable CP, then change Type\n");
> > +   return -EINVAL;
> > +   }
> > +   state->cp_content_type = val;
> 
> You can't add checks in atomic_set_property.

Thanks Maarten for the review.
I am afraid we have the similar check for content_protection, just above this.

> 
> Until we have the full state, we can't do any checks. This has to be done in 
> the
> .atomic_check() callback.
> 
> Lets say atomic commit consists of the following:
> Set Content_Protection to desired.
> Set Content Type to Type 1.
> atomic setprop will fail
> 
> Plus the check itself is bogus. We should be able to change HDCP strictness
> without a modeset, the same way we enable and disable HDCP 1.4 with a
> fastset.

This is not modeset mandating, but the sequence of hdcp_disable->change 
Type->hdcp_enable from userspace.
Reason is we need to redo the hdcp authenticate with new Type value.

Alternate approach would be kernel performing disable and enable on Type change 
request from userspace.
For this we could disable hdcp at intel_hdcp_atomic_check and re-enable with 
new Type value at atomic_commit.

I thought it is better to leave the required preparation (disable and enable of 
hdcp) to userspace.
But if we prefer to push that into kernel also it is doable. Share your opinion 
please.

> 
> > } else if (property == connector->colorspace_property) {
> > state->colorspace = val;
> > } else if (property == config->writeback_fb_id_property) { @@ -822,6
> > +830,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->cp_content_type_property) {
> > +   *val = state->cp_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 07d65a16c623..5d7738e1e977
> > 100644
> > --- a/drivers/gpu/drm/drm_connector.c
> > +++ b/drivers/gpu/drm/drm_connector.c
> > @@ -853,6 +853,13 @@ static const struct drm_prop_enum_list
> hdmi_colorspaces[] = {
> > { DRM_MODE_COLORIMETRY_DCI_P3_RGB_THEATER, "DCI-
> P3_RGB_Theater" },
> > };
> >
> > +

Re: [Intel-gfx] [PATCH 01/10] drm: Add CP content type property

2019-03-05 Thread Maarten Lankhorst
Op 26-02-2019 om 08:36 schreef Ramalingam C:
> This patch adds a DRM ENUM property to the selected connectors.
> This property is used for pass the protected content's type
> from userspace to kernel HDCP authentication.
>
> Type of the stream is decided by the protected content providers as
> Type 0/1.
>
> 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 upon a content protection request with Type 1 as Content type from
> userspace, Kernel will declare success only if the HDCP2.2
> authentication is successful.
>
> Signed-off-by: Ramalingam C 
> ---
>  drivers/gpu/drm/drm_atomic_uapi.c | 10 ++
>  drivers/gpu/drm/drm_connector.c   | 64 
> +++
>  include/drm/drm_connector.h   | 15 +
>  include/uapi/drm/drm_mode.h   |  4 +++
>  4 files changed, 93 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c 
> b/drivers/gpu/drm/drm_atomic_uapi.c
> index 4eb81f10bc54..5289486565ce 100644
> --- a/drivers/gpu/drm/drm_atomic_uapi.c
> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> @@ -746,6 +746,14 @@ static int drm_atomic_connector_set_property(struct 
> drm_connector *connector,
>   return -EINVAL;
>   }
>   state->content_protection = val;
> + } else if (property == connector->cp_content_type_property) {
> + if (state->content_protection !=
> + DRM_MODE_CONTENT_PROTECTION_UNDESIRED &&
> + state->cp_content_type != val) {
> + DRM_DEBUG_KMS("Disable CP, then change Type\n");
> + return -EINVAL;
> + }
> + state->cp_content_type = val;

You can't add checks in atomic_set_property.

Until we have the full state, we can't do any checks. This has to be done in 
the .atomic_check() callback.

Lets say atomic commit consists of the following:
Set Content_Protection to desired.
Set Content Type to Type 1.
atomic setprop will fail

Plus the check itself is bogus. We should be able to change HDCP strictness 
without a modeset, the same way
we enable and disable HDCP 1.4 with a fastset.

>   } else if (property == connector->colorspace_property) {
>   state->colorspace = val;
>   } else if (property == config->writeback_fb_id_property) {
> @@ -822,6 +830,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->cp_content_type_property) {
> + *val = state->cp_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 07d65a16c623..5d7738e1e977 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -853,6 +853,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_cp_content_type_enum_list[] = {
> + { DRM_MODE_CP_CONTENT_TYPE0, "Type 0" },
> + { DRM_MODE_CP_CONTENT_TYPE1, "Type 1" },
> +};
> +
> +DRM_ENUM_NAME_FN(drm_get_cp_content_type_name, drm_cp_content_type_enum_list)
> +
>  /**
>   * DOC: standard connector properties
>   *
> @@ -958,6 +965,25 @@ 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).
> + * CP_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 Type 0 or Type 1.
> + *
> + *   The value of the property can be one the below:
> + * - DRM_MODE_CP_CONTENT_TYPE0 = 0
> + *   Type 0 streams can be transmitted on a link which is encrypted
> + *   with HDCP 1.4 or HDCP 2.2.
> + * - DRM_MODE_CP_CONTENT_TYPE1 = 1
> + *   Type 1 streams can be transmitted on a link which is encrypted
> + *   only with HDCP2.2.
> + *
> + *   Please note this content type is introduced at HDCP2.2 and used in its
> + *   authentication process.
> + *
> + *   Guideline for programming:
> + * - Property state can be changed only when "Content Protection state is
> + *   DRM_MODE_CONTENT_PROTECTION_UNDESIRED.

Why? We may need to force a modeset, but this looks like a silly restriction to 
expose this to userspace.

If userspace doesn't want a modeset, it can clear the 

[Intel-gfx] [PATCH 01/10] drm: Add CP content type property

2019-02-25 Thread Ramalingam C
This patch adds a DRM ENUM property to the selected connectors.
This property is used for pass the protected content's type
from userspace to kernel HDCP authentication.

Type of the stream is decided by the protected content providers as
Type 0/1.

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 upon a content protection request with Type 1 as Content type from
userspace, Kernel will declare success only if the HDCP2.2
authentication is successful.

Signed-off-by: Ramalingam C 
---
 drivers/gpu/drm/drm_atomic_uapi.c | 10 ++
 drivers/gpu/drm/drm_connector.c   | 64 +++
 include/drm/drm_connector.h   | 15 +
 include/uapi/drm/drm_mode.h   |  4 +++
 4 files changed, 93 insertions(+)

diff --git a/drivers/gpu/drm/drm_atomic_uapi.c 
b/drivers/gpu/drm/drm_atomic_uapi.c
index 4eb81f10bc54..5289486565ce 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -746,6 +746,14 @@ static int drm_atomic_connector_set_property(struct 
drm_connector *connector,
return -EINVAL;
}
state->content_protection = val;
+   } else if (property == connector->cp_content_type_property) {
+   if (state->content_protection !=
+   DRM_MODE_CONTENT_PROTECTION_UNDESIRED &&
+   state->cp_content_type != val) {
+   DRM_DEBUG_KMS("Disable CP, then change Type\n");
+   return -EINVAL;
+   }
+   state->cp_content_type = val;
} else if (property == connector->colorspace_property) {
state->colorspace = val;
} else if (property == config->writeback_fb_id_property) {
@@ -822,6 +830,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->cp_content_type_property) {
+   *val = state->cp_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 07d65a16c623..5d7738e1e977 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -853,6 +853,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_cp_content_type_enum_list[] = {
+   { DRM_MODE_CP_CONTENT_TYPE0, "Type 0" },
+   { DRM_MODE_CP_CONTENT_TYPE1, "Type 1" },
+};
+
+DRM_ENUM_NAME_FN(drm_get_cp_content_type_name, drm_cp_content_type_enum_list)
+
 /**
  * DOC: standard connector properties
  *
@@ -958,6 +965,25 @@ 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).
+ * CP_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 Type 0 or Type 1.
+ *
+ * The value of the property can be one the below:
+ *   - DRM_MODE_CP_CONTENT_TYPE0 = 0
+ * Type 0 streams can be transmitted on a link which is encrypted
+ * with HDCP 1.4 or HDCP 2.2.
+ *   - DRM_MODE_CP_CONTENT_TYPE1 = 1
+ * Type 1 streams can be transmitted on a link which is encrypted
+ * only with HDCP2.2.
+ *
+ * Please note this content type is introduced at HDCP2.2 and used in its
+ * authentication process.
+ *
+ * Guideline for programming:
+ *   - Property state can be changed only when "Content Protection state is
+ * DRM_MODE_CONTENT_PROTECTION_UNDESIRED.
  *
  * max bpc:
  * This range property is used by userspace to limit the bit depth. When
@@ -1548,6 +1574,44 @@ int drm_connector_attach_content_protection_property(
 EXPORT_SYMBOL(drm_connector_attach_content_protection_property);
 
 /**
+ * drm_connector_attach_cp_content_type_property - attach cp content type
+ * property
+ *
+ * @connector: connector to attach cp 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 HDCP2.2 authentication.
+ *
+ * Content type will be set to _connector_state.cp_content_type.
+