Re: [Intel-gfx] [PATCH v7 07/11] drm: Add Content protection type property

2019-07-05 Thread Pekka Paalanen
On Fri, 5 Jul 2019 12:03:18 +0530
Ramalingam C  wrote:

> On 2019-07-05 at 16:00:37 +0300, Pekka Paalanen wrote:
> > On Thu, 4 Jul 2019 16:06:05 +0530
> > Ramalingam C  wrote:
> >   
> > > On 2019-07-04 at 14:11:59 +0300, Pekka Paalanen wrote:  
> > > > On Tue,  7 May 2019 21:57:41 +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.
> > > > > 
> > > > > Need ACK for this new conenctor property from userspace consumer.
> > > > > 
> > > > > 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]
> > > > > v4:
> > > > >   property is created for the first requested connector and then 
> > > > > reused.
> > > > >   [Danvet]
> > > > > v5:
> > > > >   kernel doc nits addressed [Daniel]
> > > > >   Rebased as part of patch reordering.
> > > > > 
> > > > > Signed-off-by: Ramalingam C 
> > > > > Reviewed-by: Daniel Vetter 
> > > > > ---
> > > > >  drivers/gpu/drm/drm_atomic_uapi.c |  4 
> > > > >  drivers/gpu/drm/drm_connector.c   | 18 
> > > > >  drivers/gpu/drm/drm_hdcp.c| 36 
> > > > > ++-
> > > > >  drivers/gpu/drm/i915/intel_hdcp.c |  4 +++-
> > > > >  include/drm/drm_connector.h   |  7 ++
> > > > >  include/drm/drm_hdcp.h|  2 +-
> > > > >  include/drm/drm_mode_config.h |  6 ++
> > > > >  include/uapi/drm/drm_mode.h   |  4 
> > > > >  8 files changed, 78 insertions(+), 3 deletions(-)
> > > > > 
> > > > > diff --git a/drivers/gpu/drm/drm_atomic_uapi.c 
> > > > > b/drivers/gpu/drm/drm_atomic_uapi.c
> > > > > index 4131e669785a..a85f3ccfe699 100644
> > > > > --- a/drivers/gpu/drm/drm_atomic_uapi.c
> > > > > +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> > > > > @@ -738,6 +738,8 @@ static int 
> > > > > drm_atomic_connector_set_property(struct drm_connector *connector,
> > > > >   return -EINVAL;
> > > > >   }
> > > > >   state->content_protection = val;
> > > > > + } else if (property == config->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) {
> > > > > @@ -816,6 +818,8 @@ drm_atomic_connector_get_property(struct 
> > > > > drm_connector *connector,
> > > > >   *val = state->scaling_mode;
> > > > >   } else if (property == config->content_protection_property) {
> > > > >   *val = state->content_protection;
> > > > > + } else if (property == config->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 764c7903edf6..de9e06583e8c 100644
> > > > > --- a/drivers/gpu/drm/drm_connector.c
> > > > > +++ b/drivers/gpu/drm/drm_connector.c
> > > > 
> > > > Hi,
> > > > 
> > > > below I have some comments and questions before I can say whether
> > > > https://gitlab.freedesktop.org/wayland/weston/merge_requests/48
> > > > adheres to this specification.
> > > > 
> > > > > @@ -955,6 +955,24 @@ 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
> > > > > + *   to be displayed 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
> > > > 
> > > 

Re: [Intel-gfx] [PATCH v7 07/11] drm: Add Content protection type property

2019-07-05 Thread Ramalingam C
On 2019-07-05 at 16:00:37 +0300, Pekka Paalanen wrote:
> On Thu, 4 Jul 2019 16:06:05 +0530
> Ramalingam C  wrote:
> 
> > On 2019-07-04 at 14:11:59 +0300, Pekka Paalanen wrote:
> > > On Tue,  7 May 2019 21:57:41 +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.
> > > > 
> > > > Need ACK for this new conenctor property from userspace consumer.
> > > > 
> > > > 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]
> > > > v4:
> > > >   property is created for the first requested connector and then reused.
> > > > [Danvet]
> > > > v5:
> > > >   kernel doc nits addressed [Daniel]
> > > >   Rebased as part of patch reordering.
> > > > 
> > > > Signed-off-by: Ramalingam C 
> > > > Reviewed-by: Daniel Vetter 
> > > > ---
> > > >  drivers/gpu/drm/drm_atomic_uapi.c |  4 
> > > >  drivers/gpu/drm/drm_connector.c   | 18 
> > > >  drivers/gpu/drm/drm_hdcp.c| 36 ++-
> > > >  drivers/gpu/drm/i915/intel_hdcp.c |  4 +++-
> > > >  include/drm/drm_connector.h   |  7 ++
> > > >  include/drm/drm_hdcp.h|  2 +-
> > > >  include/drm/drm_mode_config.h |  6 ++
> > > >  include/uapi/drm/drm_mode.h   |  4 
> > > >  8 files changed, 78 insertions(+), 3 deletions(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/drm_atomic_uapi.c 
> > > > b/drivers/gpu/drm/drm_atomic_uapi.c
> > > > index 4131e669785a..a85f3ccfe699 100644
> > > > --- a/drivers/gpu/drm/drm_atomic_uapi.c
> > > > +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> > > > @@ -738,6 +738,8 @@ static int drm_atomic_connector_set_property(struct 
> > > > drm_connector *connector,
> > > > return -EINVAL;
> > > > }
> > > > state->content_protection = val;
> > > > +   } else if (property == config->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) {
> > > > @@ -816,6 +818,8 @@ drm_atomic_connector_get_property(struct 
> > > > drm_connector *connector,
> > > > *val = state->scaling_mode;
> > > > } else if (property == config->content_protection_property) {
> > > > *val = state->content_protection;
> > > > +   } else if (property == config->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 764c7903edf6..de9e06583e8c 100644
> > > > --- a/drivers/gpu/drm/drm_connector.c
> > > > +++ b/drivers/gpu/drm/drm_connector.c  
> > > 
> > > Hi,
> > > 
> > > below I have some comments and questions before I can say whether
> > > https://gitlab.freedesktop.org/wayland/weston/merge_requests/48
> > > adheres to this specification.
> > >   
> > > > @@ -955,6 +955,24 @@ 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
> > > > + * to be displayed 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  
> > > 
> > > If this doc is meant as the UAPI doc, it needs to use the string names
> > > for enum property values, not the C language definitions (integers).  
> 
> > kernel documentation for all other properties followed this way. We
> > could add string associated 

Re: [Intel-gfx] [PATCH v7 07/11] drm: Add Content protection type property

2019-07-05 Thread Pekka Paalanen
On Thu, 4 Jul 2019 16:06:05 +0530
Ramalingam C  wrote:

> On 2019-07-04 at 14:11:59 +0300, Pekka Paalanen wrote:
> > On Tue,  7 May 2019 21:57:41 +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.
> > > 
> > > Need ACK for this new conenctor property from userspace consumer.
> > > 
> > > 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]
> > > v4:
> > >   property is created for the first requested connector and then reused.
> > >   [Danvet]
> > > v5:
> > >   kernel doc nits addressed [Daniel]
> > >   Rebased as part of patch reordering.
> > > 
> > > Signed-off-by: Ramalingam C 
> > > Reviewed-by: Daniel Vetter 
> > > ---
> > >  drivers/gpu/drm/drm_atomic_uapi.c |  4 
> > >  drivers/gpu/drm/drm_connector.c   | 18 
> > >  drivers/gpu/drm/drm_hdcp.c| 36 ++-
> > >  drivers/gpu/drm/i915/intel_hdcp.c |  4 +++-
> > >  include/drm/drm_connector.h   |  7 ++
> > >  include/drm/drm_hdcp.h|  2 +-
> > >  include/drm/drm_mode_config.h |  6 ++
> > >  include/uapi/drm/drm_mode.h   |  4 
> > >  8 files changed, 78 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/drm_atomic_uapi.c 
> > > b/drivers/gpu/drm/drm_atomic_uapi.c
> > > index 4131e669785a..a85f3ccfe699 100644
> > > --- a/drivers/gpu/drm/drm_atomic_uapi.c
> > > +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> > > @@ -738,6 +738,8 @@ static int drm_atomic_connector_set_property(struct 
> > > drm_connector *connector,
> > >   return -EINVAL;
> > >   }
> > >   state->content_protection = val;
> > > + } else if (property == config->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) {
> > > @@ -816,6 +818,8 @@ drm_atomic_connector_get_property(struct 
> > > drm_connector *connector,
> > >   *val = state->scaling_mode;
> > >   } else if (property == config->content_protection_property) {
> > >   *val = state->content_protection;
> > > + } else if (property == config->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 764c7903edf6..de9e06583e8c 100644
> > > --- a/drivers/gpu/drm/drm_connector.c
> > > +++ b/drivers/gpu/drm/drm_connector.c  
> > 
> > Hi,
> > 
> > below I have some comments and questions before I can say whether
> > https://gitlab.freedesktop.org/wayland/weston/merge_requests/48
> > adheres to this specification.
> >   
> > > @@ -955,6 +955,24 @@ 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
> > > + *   to be displayed 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  
> > 
> > If this doc is meant as the UAPI doc, it needs to use the string names
> > for enum property values, not the C language definitions (integers).  

> kernel documentation for all other properties followed this way. We
> could add string associated to the enum state too for this property.

Hi,

I don't really care what kernel internal APIs use, this may well be
correct for them, but the UAPI uses strings.

Because the kernel internal and UAPI docs are mixed up, it will be hard
to write proper docs. I guess can't help it this time.

It would be really good if the enum value strings were explicitly

Re: [Intel-gfx] [PATCH v7 07/11] drm: Add Content protection type property

2019-07-04 Thread Ramalingam C
On 2019-07-04 at 14:11:59 +0300, Pekka Paalanen wrote:
> On Tue,  7 May 2019 21:57:41 +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.
> > 
> > Need ACK for this new conenctor property from userspace consumer.
> > 
> > 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]
> > v4:
> >   property is created for the first requested connector and then reused.
> > [Danvet]
> > v5:
> >   kernel doc nits addressed [Daniel]
> >   Rebased as part of patch reordering.
> > 
> > Signed-off-by: Ramalingam C 
> > Reviewed-by: Daniel Vetter 
> > ---
> >  drivers/gpu/drm/drm_atomic_uapi.c |  4 
> >  drivers/gpu/drm/drm_connector.c   | 18 
> >  drivers/gpu/drm/drm_hdcp.c| 36 ++-
> >  drivers/gpu/drm/i915/intel_hdcp.c |  4 +++-
> >  include/drm/drm_connector.h   |  7 ++
> >  include/drm/drm_hdcp.h|  2 +-
> >  include/drm/drm_mode_config.h |  6 ++
> >  include/uapi/drm/drm_mode.h   |  4 
> >  8 files changed, 78 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_atomic_uapi.c 
> > b/drivers/gpu/drm/drm_atomic_uapi.c
> > index 4131e669785a..a85f3ccfe699 100644
> > --- a/drivers/gpu/drm/drm_atomic_uapi.c
> > +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> > @@ -738,6 +738,8 @@ static int drm_atomic_connector_set_property(struct 
> > drm_connector *connector,
> > return -EINVAL;
> > }
> > state->content_protection = val;
> > +   } else if (property == config->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) {
> > @@ -816,6 +818,8 @@ drm_atomic_connector_get_property(struct drm_connector 
> > *connector,
> > *val = state->scaling_mode;
> > } else if (property == config->content_protection_property) {
> > *val = state->content_protection;
> > +   } else if (property == config->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 764c7903edf6..de9e06583e8c 100644
> > --- a/drivers/gpu/drm/drm_connector.c
> > +++ b/drivers/gpu/drm/drm_connector.c
> 
> Hi,
> 
> below I have some comments and questions before I can say whether
> https://gitlab.freedesktop.org/wayland/weston/merge_requests/48
> adheres to this specification.
> 
> > @@ -955,6 +955,24 @@ 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
> > + * to be displayed 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
> 
> If this doc is meant as the UAPI doc, it needs to use the string names
> for enum property values, not the C language definitions (integers).
kernel documentation for all other properties followed this way. We
could add string associated to the enum state too for this property.
> 
> > + * HDCP Type0 streams can be transmitted on a link which is
> > + * encrypted with HDCP 1.4 or HDCP 2.2.
> 
> This wording forbids using any future HDCP version for type0.
We could change it to HDCP1.4 and higher version of HDCP.
> 
> > + *   - DRM_MODE_HDCP_CONTENT_TYPE1 = 1
> > + * HDCP Type1 streams can be transmitted on a link which is
> > + * encrypted only with HDCP 2.2.
> 
> This wording forbids using any future HDCP version for type1.
As of now type 1 is only on HDCP2.2 encrypted link, as no higher version is
available. But as similar to Type 0 we could assume that 

Re: [Intel-gfx] [PATCH v7 07/11] drm: Add Content protection type property

2019-07-04 Thread Pekka Paalanen
On Tue,  7 May 2019 21:57:41 +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.
> 
> Need ACK for this new conenctor property from userspace consumer.
> 
> 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]
> v4:
>   property is created for the first requested connector and then reused.
>   [Danvet]
> v5:
>   kernel doc nits addressed [Daniel]
>   Rebased as part of patch reordering.
> 
> Signed-off-by: Ramalingam C 
> Reviewed-by: Daniel Vetter 
> ---
>  drivers/gpu/drm/drm_atomic_uapi.c |  4 
>  drivers/gpu/drm/drm_connector.c   | 18 
>  drivers/gpu/drm/drm_hdcp.c| 36 ++-
>  drivers/gpu/drm/i915/intel_hdcp.c |  4 +++-
>  include/drm/drm_connector.h   |  7 ++
>  include/drm/drm_hdcp.h|  2 +-
>  include/drm/drm_mode_config.h |  6 ++
>  include/uapi/drm/drm_mode.h   |  4 
>  8 files changed, 78 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c 
> b/drivers/gpu/drm/drm_atomic_uapi.c
> index 4131e669785a..a85f3ccfe699 100644
> --- a/drivers/gpu/drm/drm_atomic_uapi.c
> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> @@ -738,6 +738,8 @@ static int drm_atomic_connector_set_property(struct 
> drm_connector *connector,
>   return -EINVAL;
>   }
>   state->content_protection = val;
> + } else if (property == config->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) {
> @@ -816,6 +818,8 @@ drm_atomic_connector_get_property(struct drm_connector 
> *connector,
>   *val = state->scaling_mode;
>   } else if (property == config->content_protection_property) {
>   *val = state->content_protection;
> + } else if (property == config->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 764c7903edf6..de9e06583e8c 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c

Hi,

below I have some comments and questions before I can say whether
https://gitlab.freedesktop.org/wayland/weston/merge_requests/48
adheres to this specification.

> @@ -955,6 +955,24 @@ 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
> + *   to be displayed 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

If this doc is meant as the UAPI doc, it needs to use the string names
for enum property values, not the C language definitions (integers).

> + *   HDCP Type0 streams can be transmitted on a link which is
> + *   encrypted with HDCP 1.4 or HDCP 2.2.

This wording forbids using any future HDCP version for type0.

> + * - DRM_MODE_HDCP_CONTENT_TYPE1 = 1
> + *   HDCP Type1 streams can be transmitted on a link which is
> + *   encrypted only with HDCP 2.2.

This wording forbids using any future HDCP version for type1.

> + *
> + *   Note that the HDCP Content Type property is specific to HDCP 2.2, and
> + *   defaults to type 0. It is only exposed by drivers supporting HDCP 2.2

Not possible with a future HDCP version greater than 2.2?

Is it intended to be possible to add more content types in the future?

> + *   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

Ok, very good to mention this.

>   *
>   * max bpc:
>   *   

[Intel-gfx] [PATCH v7 07/11] drm: Add Content protection type property

2019-05-07 Thread Ramalingam C
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.

Need ACK for this new conenctor property from userspace consumer.

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]
v4:
  property is created for the first requested connector and then reused.
[Danvet]
v5:
  kernel doc nits addressed [Daniel]
  Rebased as part of patch reordering.

Signed-off-by: Ramalingam C 
Reviewed-by: Daniel Vetter 
---
 drivers/gpu/drm/drm_atomic_uapi.c |  4 
 drivers/gpu/drm/drm_connector.c   | 18 
 drivers/gpu/drm/drm_hdcp.c| 36 ++-
 drivers/gpu/drm/i915/intel_hdcp.c |  4 +++-
 include/drm/drm_connector.h   |  7 ++
 include/drm/drm_hdcp.h|  2 +-
 include/drm/drm_mode_config.h |  6 ++
 include/uapi/drm/drm_mode.h   |  4 
 8 files changed, 78 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic_uapi.c 
b/drivers/gpu/drm/drm_atomic_uapi.c
index 4131e669785a..a85f3ccfe699 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -738,6 +738,8 @@ static int drm_atomic_connector_set_property(struct 
drm_connector *connector,
return -EINVAL;
}
state->content_protection = val;
+   } else if (property == config->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) {
@@ -816,6 +818,8 @@ drm_atomic_connector_get_property(struct drm_connector 
*connector,
*val = state->scaling_mode;
} else if (property == config->content_protection_property) {
*val = state->content_protection;
+   } else if (property == config->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 764c7903edf6..de9e06583e8c 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -955,6 +955,24 @@ 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
+ * to be displayed 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.
+ *
+ * Note that the HDCP Content Type property is specific to HDCP 2.2, and
+ * defaults to type 0. It is only exposed by drivers supporting HDCP 2.2
+ * 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
diff --git a/drivers/gpu/drm/drm_hdcp.c b/drivers/gpu/drm/drm_hdcp.c
index 0da7b3718bad..75402463466b 100644
--- a/drivers/gpu/drm/drm_hdcp.c
+++ b/drivers/gpu/drm/drm_hdcp.c
@@ -342,23 +342,41 @@ static struct drm_prop_enum_list drm_cp_enum_list[] = {
 };
 DRM_ENUM_NAME_FN(drm_get_content_protection_name, drm_cp_enum_list)
 
+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)
+
 /**
  * drm_connector_attach_content_protection_property - attach content protection
  *