diff mbox

[RFC,v3] drm/hdcp: drm enum property for CP State

Message ID 1500919921-9662-1-git-send-email-ramalingam.c@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Ramalingam C July 24, 2017, 6:12 p.m. UTC
DRM connector property is created to represent the content protection
state of the connector and to configure the same.

Content protection states defined:
	DRM_MODE_CONTENT_PROTECTION_UNSUPPORTED 	- Unsupported
	DRM_MODE_CONTENT_PROTECTION_DISABLE		- Disabled
	DRM_MODE_CONTENT_PROTECTION_ENABLE		- Enabled

v2: Redesigned the property to match with CP needs of CrOS [Sean].

v3: Renamed the state names. Header is removed [sean].

Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
---
 drivers/gpu/drm/drm_connector.c | 14 ++++++++++++++
 include/drm/drm_mode_config.h   |  5 +++++
 include/uapi/drm/drm_mode.h     |  5 +++++
 3 files changed, 24 insertions(+)

Comments

Sean Paul July 25, 2017, 12:34 p.m. UTC | #1
On Mon, Jul 24, 2017 at 2:12 PM, Ramalingam C <ramalingam.c@intel.com> wrote:
> DRM connector property is created to represent the content protection
> state of the connector and to configure the same.
>
> Content protection states defined:
>         DRM_MODE_CONTENT_PROTECTION_UNSUPPORTED         - Unsupported
>         DRM_MODE_CONTENT_PROTECTION_DISABLE             - Disabled
>         DRM_MODE_CONTENT_PROTECTION_ENABLE              - Enabled
>
> v2: Redesigned the property to match with CP needs of CrOS [Sean].
>
> v3: Renamed the state names. Header is removed [sean].
>
> Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
> ---
>  drivers/gpu/drm/drm_connector.c | 14 ++++++++++++++
>  include/drm/drm_mode_config.h   |  5 +++++
>  include/uapi/drm/drm_mode.h     |  5 +++++
>  3 files changed, 24 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index 5cd61af..d6aaa08 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -617,6 +617,13 @@ static const struct drm_prop_enum_list drm_link_status_enum_list[] = {
>  };
>  DRM_ENUM_NAME_FN(drm_get_link_status_name, drm_link_status_enum_list)
>
> +static const struct drm_prop_enum_list drm_cp_enum_list[] = {
> +       { DRM_MODE_CONTENT_PROTECTION_UNSUPPORTED,      "Unsupported" },

You're still changing the enum names from the original patch/CrOS
implementation.

https://lists.freedesktop.org/archives/dri-devel/2014-December/073336.html

https://cs.chromium.org/chromium/src/ui/ozone/platform/drm/gpu/drm_display.cc?l=27



> +       { DRM_MODE_CONTENT_PROTECTION_DISABLE,          "Disabled" },
> +       { DRM_MODE_CONTENT_PROTECTION_ENABLE,           "Enabled" },
> +};
> +DRM_ENUM_NAME_FN(drm_get_cp_status_name, drm_cp_enum_list)
> +
>  /**
>   * drm_display_info_set_bus_formats - set the supported bus formats
>   * @info: display info to store bus formats in
> @@ -789,6 +796,13 @@ int drm_connector_create_standard_properties(struct drm_device *dev)
>                 return -ENOMEM;
>         dev->mode_config.link_status_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;
> +       dev->mode_config.cp_property = prop;
> +
>         return 0;
>  }
>
> diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
> index 4298171..7acb8b2 100644
> --- a/include/drm/drm_mode_config.h
> +++ b/include/drm/drm_mode_config.h
> @@ -538,6 +538,11 @@ struct drm_mode_config {
>          */
>         struct drm_property *link_status_property;
>         /**
> +        * @cp_property: Default connector property for CP
> +        * of a connector

Can you please elaborate on this, so readers can understand how this
property works? Perhaps just copy the docs from the original patch?

> +        */
> +       struct drm_property *cp_property;
> +       /**
>          * @plane_type_property: Default plane property to differentiate
>          * CURSOR, PRIMARY and OVERLAY legacy uses of planes.
>          */
> diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
> index 403339f..554a770 100644
> --- a/include/uapi/drm/drm_mode.h
> +++ b/include/uapi/drm/drm_mode.h
> @@ -127,6 +127,11 @@ extern "C" {
>  #define DRM_MODE_LINK_STATUS_GOOD      0
>  #define DRM_MODE_LINK_STATUS_BAD       1
>
> +/* Content Protection options */
> +#define DRM_MODE_CONTENT_PROTECTION_UNSUPPORTED                0
> +#define DRM_MODE_CONTENT_PROTECTION_DISABLE            1
> +#define DRM_MODE_CONTENT_PROTECTION_ENABLE             2
> +
>  /*
>   * DRM_MODE_ROTATE_<degrees>
>   *
> --
> 2.7.4
>
Ramalingam C July 26, 2017, 9:54 a.m. UTC | #2
On Tuesday 25 July 2017 06:04 PM, Sean Paul wrote:
> On Mon, Jul 24, 2017 at 2:12 PM, Ramalingam C <ramalingam.c@intel.com> wrote:
>> DRM connector property is created to represent the content protection
>> state of the connector and to configure the same.
>>
>> Content protection states defined:
>>          DRM_MODE_CONTENT_PROTECTION_UNSUPPORTED         - Unsupported
>>          DRM_MODE_CONTENT_PROTECTION_DISABLE             - Disabled
>>          DRM_MODE_CONTENT_PROTECTION_ENABLE              - Enabled
>>
>> v2: Redesigned the property to match with CP needs of CrOS [Sean].
>>
>> v3: Renamed the state names. Header is removed [sean].
>>
>> Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
>> ---
>>   drivers/gpu/drm/drm_connector.c | 14 ++++++++++++++
>>   include/drm/drm_mode_config.h   |  5 +++++
>>   include/uapi/drm/drm_mode.h     |  5 +++++
>>   3 files changed, 24 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
>> index 5cd61af..d6aaa08 100644
>> --- a/drivers/gpu/drm/drm_connector.c
>> +++ b/drivers/gpu/drm/drm_connector.c
>> @@ -617,6 +617,13 @@ static const struct drm_prop_enum_list drm_link_status_enum_list[] = {
>>   };
>>   DRM_ENUM_NAME_FN(drm_get_link_status_name, drm_link_status_enum_list)
>>
>> +static const struct drm_prop_enum_list drm_cp_enum_list[] = {
>> +       { DRM_MODE_CONTENT_PROTECTION_UNSUPPORTED,      "Unsupported" },
> You're still changing the enum names from the original patch/CrOS
> implementation.
>
> https://lists.freedesktop.org/archives/dri-devel/2014-December/073336.html
>
> https://cs.chromium.org/chromium/src/ui/ozone/platform/drm/gpu/drm_display.cc?l=27

Sean,

I think we have bit of confusion here.

And in previous discussion you were fine with new state of property that is
"unsupported" - indicates no common HDCP version is supported on HDCP 
src and Sink combo.

when CP is not possible, if property exist, userspace will interpret it 
as CP is supported and attempt for enabling.
I prefer indicating Unsupported state than failing such requests blindly.

In that case to interpret the new state, we need to change CrOS User 
space code.

In the RFC you mentioned above, two version of uapi interfaces are 
discussed.
         V1 uses single property to configure the CP and also for status 
indication.
         V2 uses two properties one for configuring and another one for 
status of Content protection.

But CrOS user space is currently using the V1 interface.

which will be preferred approach right now (V1/V2)?
In either way we need to change the CrOS :(

Lets say we need to modify CrOS user space, will there be any help for that?
I am not aware what it takes though.

I am trying to discuss both uapi versions of V1 and V2. I prefer V2 though.

If V1 is preferred we need a single property as below
"content protection" property  with {"Unsupported", "Undesired", 
"Desired", "Enabled"}
                         "Type1_desired" and "Type1_Enabled" are needed 
for HDCP2.2


If V2 is preferred we need two properties as below

"content protection" property with {"Unsupported", "Undesired", "Desired"}
             ("Type1_desired" - needed For HDCP2.2)
"content protection status" property with {"Disabled", "Enabled"}
             ("Type1_enabled" - needed for HDCP2.2)

         -   Not providing the ksv, as reflecting one ksv is not serving 
any purpose.
             Even for revocation check you need provision to list all 
device IDs (Max 32devices x 5Bytes ID) attached to repeater.
             So it is good to reflect the current protection level of 
link through enum.

And Usage will be as below:
     By default property "content protection" will be set to
         - "Unsupported"    (If CP is not possible on the Link)
         - "Undesired"        (If Link is not protected. CP is possible)
     By default property "content protection status" will be set to
         - "Disabled"

     the sequence of enabling protection on a link:
         - User space will set "content protection"  "Undesired" -> 
"Desired"
         - kernel will start the Authentication.
         - Once Auth is successful, kernel will change "content 
protection status" as "Disabled" -> "Enabled"

     The sequence of disabling protection on a link:
         - User space will set "content protection"  "Desired" -> 
"Undesired"
         - kernel will start the disable encryption.
         - Once it is successful, kernel will change "content protection 
status" as "Enabled" -> "Disabled"

     The sequence in case of encryption Failure on protected link:
         - When Failure of encryption occurs, kernel will change 
"content protection status" as "Enabled" -> "Disabled"
         - Kernel will try for retry for authentication and encryption.
                 - If the link is encrypted once again, kernel will 
change "content protection status" as "Disabled" -> "Enabled"
                 - But if retry failed, kernel will change "content 
protection" as "Desired" -> "Undesired"
         - So the "Desired" state of "content protection" property 
indicates that kernel is authenticating/re authenticating the link for 
encryption.

--Ram
>
>
>
>> +       { DRM_MODE_CONTENT_PROTECTION_DISABLE,          "Disabled" },
>> +       { DRM_MODE_CONTENT_PROTECTION_ENABLE,           "Enabled" },
>> +};
>> +DRM_ENUM_NAME_FN(drm_get_cp_status_name, drm_cp_enum_list)
>> +
>>   /**
>>    * drm_display_info_set_bus_formats - set the supported bus formats
>>    * @info: display info to store bus formats in
>> @@ -789,6 +796,13 @@ int drm_connector_create_standard_properties(struct drm_device *dev)
>>                  return -ENOMEM;
>>          dev->mode_config.link_status_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;
>> +       dev->mode_config.cp_property = prop;
>> +
>>          return 0;
>>   }
>>
>> diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
>> index 4298171..7acb8b2 100644
>> --- a/include/drm/drm_mode_config.h
>> +++ b/include/drm/drm_mode_config.h
>> @@ -538,6 +538,11 @@ struct drm_mode_config {
>>           */
>>          struct drm_property *link_status_property;
>>          /**
>> +        * @cp_property: Default connector property for CP
>> +        * of a connector
> Can you please elaborate on this, so readers can understand how this
> property works? Perhaps just copy the docs from the original patch?
>
>> +        */
>> +       struct drm_property *cp_property;
>> +       /**
>>           * @plane_type_property: Default plane property to differentiate
>>           * CURSOR, PRIMARY and OVERLAY legacy uses of planes.
>>           */
>> diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
>> index 403339f..554a770 100644
>> --- a/include/uapi/drm/drm_mode.h
>> +++ b/include/uapi/drm/drm_mode.h
>> @@ -127,6 +127,11 @@ extern "C" {
>>   #define DRM_MODE_LINK_STATUS_GOOD      0
>>   #define DRM_MODE_LINK_STATUS_BAD       1
>>
>> +/* Content Protection options */
>> +#define DRM_MODE_CONTENT_PROTECTION_UNSUPPORTED                0
>> +#define DRM_MODE_CONTENT_PROTECTION_DISABLE            1
>> +#define DRM_MODE_CONTENT_PROTECTION_ENABLE             2
>> +
>>   /*
>>    * DRM_MODE_ROTATE_<degrees>
>>    *
>> --
>> 2.7.4
>>
Sean Paul July 26, 2017, 2:52 p.m. UTC | #3
On Wed, Jul 26, 2017 at 03:24:10PM +0530, Ramalingam C wrote:
> 
> 
> On Tuesday 25 July 2017 06:04 PM, Sean Paul wrote:
> > On Mon, Jul 24, 2017 at 2:12 PM, Ramalingam C <ramalingam.c@intel.com> wrote:
> > > DRM connector property is created to represent the content protection
> > > state of the connector and to configure the same.
> > > 
> > > Content protection states defined:
> > >          DRM_MODE_CONTENT_PROTECTION_UNSUPPORTED         - Unsupported
> > >          DRM_MODE_CONTENT_PROTECTION_DISABLE             - Disabled
> > >          DRM_MODE_CONTENT_PROTECTION_ENABLE              - Enabled
> > > 
> > > v2: Redesigned the property to match with CP needs of CrOS [Sean].
> > > 
> > > v3: Renamed the state names. Header is removed [sean].
> > > 
> > > Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
> > > ---
> > >   drivers/gpu/drm/drm_connector.c | 14 ++++++++++++++
> > >   include/drm/drm_mode_config.h   |  5 +++++
> > >   include/uapi/drm/drm_mode.h     |  5 +++++
> > >   3 files changed, 24 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> > > index 5cd61af..d6aaa08 100644
> > > --- a/drivers/gpu/drm/drm_connector.c
> > > +++ b/drivers/gpu/drm/drm_connector.c
> > > @@ -617,6 +617,13 @@ static const struct drm_prop_enum_list drm_link_status_enum_list[] = {
> > >   };
> > >   DRM_ENUM_NAME_FN(drm_get_link_status_name, drm_link_status_enum_list)
> > > 
> > > +static const struct drm_prop_enum_list drm_cp_enum_list[] = {
> > > +       { DRM_MODE_CONTENT_PROTECTION_UNSUPPORTED,      "Unsupported" },
> > You're still changing the enum names from the original patch/CrOS
> > implementation.
> > 
> > https://lists.freedesktop.org/archives/dri-devel/2014-December/073336.html
> > 
> > https://cs.chromium.org/chromium/src/ui/ozone/platform/drm/gpu/drm_display.cc?l=27
> 
> Sean,
> 
> I think we have bit of confusion here.

Agreed :)

> 
> And in previous discussion you were fine with new state of property that is
> "unsupported" - indicates no common HDCP version is supported on HDCP src
> and Sink combo.
> 
> when CP is not possible, if property exist, userspace will interpret it as
> CP is supported and attempt for enabling.
> I prefer indicating Unsupported state than failing such requests blindly.
> 
> In that case to interpret the new state, we need to change CrOS User space
> code.
> 
> In the RFC you mentioned above, two version of uapi interfaces are
> discussed.
>         V1 uses single property to configure the CP and also for status
> indication.
>         V2 uses two properties one for configuring and another one for
> status of Content protection.
> 
> But CrOS user space is currently using the V1 interface.
> 
> which will be preferred approach right now (V1/V2)?
> In either way we need to change the CrOS :(

We can't upstream code without a userspace implementation, so V1. Without any
modifications.

As has been mentioned before in this thread, it's a lot easier to upstream code
that is proven to work, than it is to merge speculative API. Further, if this
patch were merged, it would be CrOS which is currently the only consumer... not
a good look :)


> 
> Lets say we need to modify CrOS user space, will there be any help for that?
> I am not aware what it takes though.

You could probably reach out to chromium-dev@chromium.org or the author of the
Chrome hdcp implementation for help.

Sean

> 
> I am trying to discuss both uapi versions of V1 and V2. I prefer V2 though.
> 
> If V1 is preferred we need a single property as below
> "content protection" property  with {"Unsupported", "Undesired", "Desired",
> "Enabled"}
>                         "Type1_desired" and "Type1_Enabled" are needed for
> HDCP2.2
> 
> 
> If V2 is preferred we need two properties as below
> 
> "content protection" property with {"Unsupported", "Undesired", "Desired"}
>             ("Type1_desired" - needed For HDCP2.2)
> "content protection status" property with {"Disabled", "Enabled"}
>             ("Type1_enabled" - needed for HDCP2.2)
> 
>         -   Not providing the ksv, as reflecting one ksv is not serving any
> purpose.
>             Even for revocation check you need provision to list all device
> IDs (Max 32devices x 5Bytes ID) attached to repeater.
>             So it is good to reflect the current protection level of link
> through enum.
> 
> And Usage will be as below:
>     By default property "content protection" will be set to
>         - "Unsupported"    (If CP is not possible on the Link)
>         - "Undesired"        (If Link is not protected. CP is possible)
>     By default property "content protection status" will be set to
>         - "Disabled"
> 
>     the sequence of enabling protection on a link:
>         - User space will set "content protection"  "Undesired" -> "Desired"
>         - kernel will start the Authentication.
>         - Once Auth is successful, kernel will change "content protection
> status" as "Disabled" -> "Enabled"
> 
>     The sequence of disabling protection on a link:
>         - User space will set "content protection"  "Desired" -> "Undesired"
>         - kernel will start the disable encryption.
>         - Once it is successful, kernel will change "content protection
> status" as "Enabled" -> "Disabled"
> 
>     The sequence in case of encryption Failure on protected link:
>         - When Failure of encryption occurs, kernel will change "content
> protection status" as "Enabled" -> "Disabled"
>         - Kernel will try for retry for authentication and encryption.
>                 - If the link is encrypted once again, kernel will change
> "content protection status" as "Disabled" -> "Enabled"
>                 - But if retry failed, kernel will change "content
> protection" as "Desired" -> "Undesired"
>         - So the "Desired" state of "content protection" property indicates
> that kernel is authenticating/re authenticating the link for encryption.
> 
> --Ram
> > 
> > 
> > 
> > > +       { DRM_MODE_CONTENT_PROTECTION_DISABLE,          "Disabled" },
> > > +       { DRM_MODE_CONTENT_PROTECTION_ENABLE,           "Enabled" },
> > > +};
> > > +DRM_ENUM_NAME_FN(drm_get_cp_status_name, drm_cp_enum_list)
> > > +
> > >   /**
> > >    * drm_display_info_set_bus_formats - set the supported bus formats
> > >    * @info: display info to store bus formats in
> > > @@ -789,6 +796,13 @@ int drm_connector_create_standard_properties(struct drm_device *dev)
> > >                  return -ENOMEM;
> > >          dev->mode_config.link_status_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;
> > > +       dev->mode_config.cp_property = prop;
> > > +
> > >          return 0;
> > >   }
> > > 
> > > diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
> > > index 4298171..7acb8b2 100644
> > > --- a/include/drm/drm_mode_config.h
> > > +++ b/include/drm/drm_mode_config.h
> > > @@ -538,6 +538,11 @@ struct drm_mode_config {
> > >           */
> > >          struct drm_property *link_status_property;
> > >          /**
> > > +        * @cp_property: Default connector property for CP
> > > +        * of a connector
> > Can you please elaborate on this, so readers can understand how this
> > property works? Perhaps just copy the docs from the original patch?
> > 
> > > +        */
> > > +       struct drm_property *cp_property;
> > > +       /**
> > >           * @plane_type_property: Default plane property to differentiate
> > >           * CURSOR, PRIMARY and OVERLAY legacy uses of planes.
> > >           */
> > > diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
> > > index 403339f..554a770 100644
> > > --- a/include/uapi/drm/drm_mode.h
> > > +++ b/include/uapi/drm/drm_mode.h
> > > @@ -127,6 +127,11 @@ extern "C" {
> > >   #define DRM_MODE_LINK_STATUS_GOOD      0
> > >   #define DRM_MODE_LINK_STATUS_BAD       1
> > > 
> > > +/* Content Protection options */
> > > +#define DRM_MODE_CONTENT_PROTECTION_UNSUPPORTED                0
> > > +#define DRM_MODE_CONTENT_PROTECTION_DISABLE            1
> > > +#define DRM_MODE_CONTENT_PROTECTION_ENABLE             2
> > > +
> > >   /*
> > >    * DRM_MODE_ROTATE_<degrees>
> > >    *
> > > --
> > > 2.7.4
> > > 
>
Ramalingam C July 26, 2017, 4:51 p.m. UTC | #4
> -----Original Message-----
> From: Sean Paul [mailto:seanpaul@chromium.org]
> Sent: Wednesday, July 26, 2017 8:23 PM
> To: C, Ramalingam <ramalingam.c@intel.com>
> Cc: Sean Paul <seanpaul@chromium.org>; Vetter, Daniel
> <daniel.vetter@intel.com>; Intel Graphics Development <intel-
> gfx@lists.freedesktop.org>; dri-devel <dri-devel@lists.freedesktop.org>; Daniel
> Vetter <daniel@ffwll.ch>
> Subject: Re: [RFC v3] drm/hdcp: drm enum property for CP State
> 
> On Wed, Jul 26, 2017 at 03:24:10PM +0530, Ramalingam C wrote:
> >
> >
> > On Tuesday 25 July 2017 06:04 PM, Sean Paul wrote:
> > > On Mon, Jul 24, 2017 at 2:12 PM, Ramalingam C <ramalingam.c@intel.com>
> wrote:
> > > > DRM connector property is created to represent the content
> > > > protection state of the connector and to configure the same.
> > > >
> > > > Content protection states defined:
> > > >          DRM_MODE_CONTENT_PROTECTION_UNSUPPORTED         -
> Unsupported
> > > >          DRM_MODE_CONTENT_PROTECTION_DISABLE             - Disabled
> > > >          DRM_MODE_CONTENT_PROTECTION_ENABLE              - Enabled
> > > >
> > > > v2: Redesigned the property to match with CP needs of CrOS [Sean].
> > > >
> > > > v3: Renamed the state names. Header is removed [sean].
> > > >
> > > > Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
> > > > ---
> > > >   drivers/gpu/drm/drm_connector.c | 14 ++++++++++++++
> > > >   include/drm/drm_mode_config.h   |  5 +++++
> > > >   include/uapi/drm/drm_mode.h     |  5 +++++
> > > >   3 files changed, 24 insertions(+)
> > > >
> > > > diff --git a/drivers/gpu/drm/drm_connector.c
> > > > b/drivers/gpu/drm/drm_connector.c index 5cd61af..d6aaa08 100644
> > > > --- a/drivers/gpu/drm/drm_connector.c
> > > > +++ b/drivers/gpu/drm/drm_connector.c
> > > > @@ -617,6 +617,13 @@ static const struct drm_prop_enum_list
> drm_link_status_enum_list[] = {
> > > >   };
> > > >   DRM_ENUM_NAME_FN(drm_get_link_status_name,
> > > > drm_link_status_enum_list)
> > > >
> > > > +static const struct drm_prop_enum_list drm_cp_enum_list[] = {
> > > > +       { DRM_MODE_CONTENT_PROTECTION_UNSUPPORTED,
> "Unsupported" },
> > > You're still changing the enum names from the original patch/CrOS
> > > implementation.
> > >
> > > https://lists.freedesktop.org/archives/dri-devel/2014-December/07333
> > > 6.html
> > >
> > > https://cs.chromium.org/chromium/src/ui/ozone/platform/drm/gpu/drm_d
> > > isplay.cc?l=27
> >
> > Sean,
> >
> > I think we have bit of confusion here.
> 
> Agreed :)
> 
> >
> > And in previous discussion you were fine with new state of property
> > that is "unsupported" - indicates no common HDCP version is supported
> > on HDCP src and Sink combo.
> >
> > when CP is not possible, if property exist, userspace will interpret
> > it as CP is supported and attempt for enabling.
> > I prefer indicating Unsupported state than failing such requests blindly.
> >
> > In that case to interpret the new state, we need to change CrOS User
> > space code.
> >
> > In the RFC you mentioned above, two version of uapi interfaces are
> > discussed.
> >         V1 uses single property to configure the CP and also for
> > status indication.
> >         V2 uses two properties one for configuring and another one for
> > status of Content protection.
> >
> > But CrOS user space is currently using the V1 interface.
> >
> > which will be preferred approach right now (V1/V2)?
> > In either way we need to change the CrOS :(
> 
> We can't upstream code without a userspace implementation, so V1. Without
> any modifications.

Based on uAPI rules mentioned by Daniel, what I infer is that once a uAPI is in place,
usecase of uAPI cant be changed. It can be only be extended for future needs.

So IMHO it is better to shape the userspace as per the uAPI
interfaces that community want to have for longer run. And mainly we should adapt to a
interface which can be conveniently extended for HDCP2.2 and future specs versions.

So IMO it is worth seeing what it takes for CrOS Userspace changes for HDCP2.2 interfaces.
And In the interest of having HDCP2.2 in their stack sooner,  if the CrOS community or Google
 is interested in providing that userspace support, it will be smoother to put the
complete and preferred uAPI for HDCP2.2 and 1.4 in place.

Sean, I think you will be better person to comment on this from both sides, CrOS and DRM.


> 
> As has been mentioned before in this thread, it's a lot easier to upstream code
> that is proven to work, than it is to merge speculative API. Further, if this patch
> were merged, it would be CrOS which is currently the only consumer... not a
> good look :)

Not exactly. CrOS will be the first consumer, Android will follow soon after this.
We need to enable this feature on HwC for that purpose. Yup might take required time :)

> 
> 
> >
> > Lets say we need to modify CrOS user space, will there be any help for that?
> > I am not aware what it takes though.
> 
> You could probably reach out to chromium-dev@chromium.org or the author of
> the Chrome hdcp implementation for help.

Thank you for the contact.
And before reaching out, it is better to conclude the HDCP1.4/2.2 uAPI interfaces that is agreeable for this community.

In that perspective, can we consider and review the uAPI interface with two properties discussed for V2 of your earlier patches?
Complete use case is discussed in my previous mail.

Based on userspace commitment for HDCP2.2, we can add blob property for SRM too.

> 
> Sean
> 
> >
> > I am trying to discuss both uapi versions of V1 and V2. I prefer V2 though.
> >
> > If V1 is preferred we need a single property as below "content
> > protection" property  with {"Unsupported", "Undesired", "Desired",
> > "Enabled"}
> >                         "Type1_desired" and "Type1_Enabled" are needed
> > for
> > HDCP2.2
> >
> >
> > If V2 is preferred we need two properties as below
> >
> > "content protection" property with {"Unsupported", "Undesired", "Desired"}
> >             ("Type1_desired" - needed For HDCP2.2) "content protection
> > status" property with {"Disabled", "Enabled"}
> >             ("Type1_enabled" - needed for HDCP2.2)

This uAPI interface looks fit to serve the all needs of HDCP1.4 and 2.2 as explained below.

Sean and Daniel,
	I request you to share your views. Thanks.

--Ram
> >
> >         -   Not providing the ksv, as reflecting one ksv is not serving any
> > purpose.
> >             Even for revocation check you need provision to list all
> > device IDs (Max 32devices x 5Bytes ID) attached to repeater.
> >             So it is good to reflect the current protection level of
> > link through enum.
> >
> > And Usage will be as below:
> >     By default property "content protection" will be set to
> >         - "Unsupported"    (If CP is not possible on the Link)
> >         - "Undesired"        (If Link is not protected. CP is possible)
> >     By default property "content protection status" will be set to
> >         - "Disabled"
> >
> >     the sequence of enabling protection on a link:
> >         - User space will set "content protection"  "Undesired" -> "Desired"
> >         - kernel will start the Authentication.
> >         - Once Auth is successful, kernel will change "content
> > protection status" as "Disabled" -> "Enabled"
> >
> >     The sequence of disabling protection on a link:
> >         - User space will set "content protection"  "Desired" -> "Undesired"
> >         - kernel will start the disable encryption.
> >         - Once it is successful, kernel will change "content
> > protection status" as "Enabled" -> "Disabled"
> >
> >     The sequence in case of encryption Failure on protected link:
> >         - When Failure of encryption occurs, kernel will change
> > "content protection status" as "Enabled" -> "Disabled"
> >         - Kernel will try for retry for authentication and encryption.
> >                 - If the link is encrypted once again, kernel will
> > change "content protection status" as "Disabled" -> "Enabled"
> >                 - But if retry failed, kernel will change "content
> > protection" as "Desired" -> "Undesired"
> >         - So the "Desired" state of "content protection" property
> > indicates that kernel is authenticating/re authenticating the link for encryption.
> >
> > --Ram
> > >
> > >
> > >
> > > > +       { DRM_MODE_CONTENT_PROTECTION_DISABLE,          "Disabled" },
> > > > +       { DRM_MODE_CONTENT_PROTECTION_ENABLE,           "Enabled" },
> > > > +};
> > > > +DRM_ENUM_NAME_FN(drm_get_cp_status_name, drm_cp_enum_list)
> > > > +
> > > >   /**
> > > >    * drm_display_info_set_bus_formats - set the supported bus formats
> > > >    * @info: display info to store bus formats in @@ -789,6 +796,13
> > > > @@ int drm_connector_create_standard_properties(struct drm_device
> *dev)
> > > >                  return -ENOMEM;
> > > >          dev->mode_config.link_status_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;
> > > > +       dev->mode_config.cp_property = prop;
> > > > +
> > > >          return 0;
> > > >   }
> > > >
> > > > diff --git a/include/drm/drm_mode_config.h
> > > > b/include/drm/drm_mode_config.h index 4298171..7acb8b2 100644
> > > > --- a/include/drm/drm_mode_config.h
> > > > +++ b/include/drm/drm_mode_config.h
> > > > @@ -538,6 +538,11 @@ struct drm_mode_config {
> > > >           */
> > > >          struct drm_property *link_status_property;
> > > >          /**
> > > > +        * @cp_property: Default connector property for CP
> > > > +        * of a connector
> > > Can you please elaborate on this, so readers can understand how this
> > > property works? Perhaps just copy the docs from the original patch?
> > >
> > > > +        */
> > > > +       struct drm_property *cp_property;
> > > > +       /**
> > > >           * @plane_type_property: Default plane property to differentiate
> > > >           * CURSOR, PRIMARY and OVERLAY legacy uses of planes.
> > > >           */
> > > > diff --git a/include/uapi/drm/drm_mode.h
> > > > b/include/uapi/drm/drm_mode.h index 403339f..554a770 100644
> > > > --- a/include/uapi/drm/drm_mode.h
> > > > +++ b/include/uapi/drm/drm_mode.h
> > > > @@ -127,6 +127,11 @@ extern "C" {
> > > >   #define DRM_MODE_LINK_STATUS_GOOD      0
> > > >   #define DRM_MODE_LINK_STATUS_BAD       1
> > > >
> > > > +/* Content Protection options */
> > > > +#define DRM_MODE_CONTENT_PROTECTION_UNSUPPORTED                0
> > > > +#define DRM_MODE_CONTENT_PROTECTION_DISABLE            1
> > > > +#define DRM_MODE_CONTENT_PROTECTION_ENABLE             2
> > > > +
> > > >   /*
> > > >    * DRM_MODE_ROTATE_<degrees>
> > > >    *
> > > > --
> > > > 2.7.4
> > > >
> >
> 
> --
> Sean Paul, Software Engineer, Google / Chromium OS
Sean Paul July 26, 2017, 5:54 p.m. UTC | #5
On Wed, Jul 26, 2017 at 12:51 PM, C, Ramalingam <ramalingam.c@intel.com> wrote:
>
>> -----Original Message-----
>> From: Sean Paul [mailto:seanpaul@chromium.org]
>> Sent: Wednesday, July 26, 2017 8:23 PM
>> To: C, Ramalingam <ramalingam.c@intel.com>
>> Cc: Sean Paul <seanpaul@chromium.org>; Vetter, Daniel
>> <daniel.vetter@intel.com>; Intel Graphics Development <intel-
>> gfx@lists.freedesktop.org>; dri-devel <dri-devel@lists.freedesktop.org>; Daniel
>> Vetter <daniel@ffwll.ch>
>> Subject: Re: [RFC v3] drm/hdcp: drm enum property for CP State
>>
>> On Wed, Jul 26, 2017 at 03:24:10PM +0530, Ramalingam C wrote:
>> >
>> >
>> > On Tuesday 25 July 2017 06:04 PM, Sean Paul wrote:
>> > > On Mon, Jul 24, 2017 at 2:12 PM, Ramalingam C <ramalingam.c@intel.com>
>> wrote:
>> > > > DRM connector property is created to represent the content
>> > > > protection state of the connector and to configure the same.
>> > > >
>> > > > Content protection states defined:
>> > > >          DRM_MODE_CONTENT_PROTECTION_UNSUPPORTED         -
>> Unsupported
>> > > >          DRM_MODE_CONTENT_PROTECTION_DISABLE             - Disabled
>> > > >          DRM_MODE_CONTENT_PROTECTION_ENABLE              - Enabled
>> > > >
>> > > > v2: Redesigned the property to match with CP needs of CrOS [Sean].
>> > > >
>> > > > v3: Renamed the state names. Header is removed [sean].
>> > > >
>> > > > Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
>> > > > ---
>> > > >   drivers/gpu/drm/drm_connector.c | 14 ++++++++++++++
>> > > >   include/drm/drm_mode_config.h   |  5 +++++
>> > > >   include/uapi/drm/drm_mode.h     |  5 +++++
>> > > >   3 files changed, 24 insertions(+)
>> > > >
>> > > > diff --git a/drivers/gpu/drm/drm_connector.c
>> > > > b/drivers/gpu/drm/drm_connector.c index 5cd61af..d6aaa08 100644
>> > > > --- a/drivers/gpu/drm/drm_connector.c
>> > > > +++ b/drivers/gpu/drm/drm_connector.c
>> > > > @@ -617,6 +617,13 @@ static const struct drm_prop_enum_list
>> drm_link_status_enum_list[] = {
>> > > >   };
>> > > >   DRM_ENUM_NAME_FN(drm_get_link_status_name,
>> > > > drm_link_status_enum_list)
>> > > >
>> > > > +static const struct drm_prop_enum_list drm_cp_enum_list[] = {
>> > > > +       { DRM_MODE_CONTENT_PROTECTION_UNSUPPORTED,
>> "Unsupported" },
>> > > You're still changing the enum names from the original patch/CrOS
>> > > implementation.
>> > >
>> > > https://lists.freedesktop.org/archives/dri-devel/2014-December/07333
>> > > 6.html
>> > >
>> > > https://cs.chromium.org/chromium/src/ui/ozone/platform/drm/gpu/drm_d
>> > > isplay.cc?l=27
>> >
>> > Sean,
>> >
>> > I think we have bit of confusion here.
>>
>> Agreed :)
>>
>> >
>> > And in previous discussion you were fine with new state of property
>> > that is "unsupported" - indicates no common HDCP version is supported
>> > on HDCP src and Sink combo.
>> >
>> > when CP is not possible, if property exist, userspace will interpret
>> > it as CP is supported and attempt for enabling.
>> > I prefer indicating Unsupported state than failing such requests blindly.
>> >
>> > In that case to interpret the new state, we need to change CrOS User
>> > space code.
>> >
>> > In the RFC you mentioned above, two version of uapi interfaces are
>> > discussed.
>> >         V1 uses single property to configure the CP and also for
>> > status indication.
>> >         V2 uses two properties one for configuring and another one for
>> > status of Content protection.
>> >
>> > But CrOS user space is currently using the V1 interface.
>> >
>> > which will be preferred approach right now (V1/V2)?
>> > In either way we need to change the CrOS :(
>>
>> We can't upstream code without a userspace implementation, so V1. Without
>> any modifications.
>
> Based on uAPI rules mentioned by Daniel, what I infer is that once a uAPI is in place,
> usecase of uAPI cant be changed. It can be only be extended for future needs.
>
> So IMHO it is better to shape the userspace as per the uAPI
> interfaces that community want to have for longer run. And mainly we should adapt to a
> interface which can be conveniently extended for HDCP2.2 and future specs versions.
>
> So IMO it is worth seeing what it takes for CrOS Userspace changes for HDCP2.2 interfaces.
> And In the interest of having HDCP2.2 in their stack sooner,  if the CrOS community or Google
>  is interested in providing that userspace support, it will be smoother to put the
> complete and preferred uAPI for HDCP2.2 and 1.4 in place.
>
> Sean, I think you will be better person to comment on this from both sides, CrOS and DRM.
>
>
>>
>> As has been mentioned before in this thread, it's a lot easier to upstream code
>> that is proven to work, than it is to merge speculative API. Further, if this patch
>> were merged, it would be CrOS which is currently the only consumer... not a
>> good look :)
>
> Not exactly. CrOS will be the first consumer, Android will follow soon after this.
> We need to enable this feature on HwC for that purpose. Yup might take required time :)
>
>>
>>
>> >
>> > Lets say we need to modify CrOS user space, will there be any help for that?
>> > I am not aware what it takes though.
>>
>> You could probably reach out to chromium-dev@chromium.org or the author of
>> the Chrome hdcp implementation for help.
>
> Thank you for the contact.
> And before reaching out, it is better to conclude the HDCP1.4/2.2 uAPI interfaces that is agreeable for this community.
>
> In that perspective, can we consider and review the uAPI interface with two properties discussed for V2 of your earlier patches?
> Complete use case is discussed in my previous mail.
>
> Based on userspace commitment for HDCP2.2, we can add blob property for SRM too.
>
>>
>> Sean
>>
>> >
>> > I am trying to discuss both uapi versions of V1 and V2. I prefer V2 though.
>> >
>> > If V1 is preferred we need a single property as below "content
>> > protection" property  with {"Unsupported", "Undesired", "Desired",
>> > "Enabled"}
>> >                         "Type1_desired" and "Type1_Enabled" are needed
>> > for
>> > HDCP2.2
>> >
>> >
>> > If V2 is preferred we need two properties as below
>> >
>> > "content protection" property with {"Unsupported", "Undesired", "Desired"}
>> >             ("Type1_desired" - needed For HDCP2.2) "content protection
>> > status" property with {"Disabled", "Enabled"}
>> >             ("Type1_enabled" - needed for HDCP2.2)
>
> This uAPI interface looks fit to serve the all needs of HDCP1.4 and 2.2 as explained below.
>
> Sean and Daniel,
>         I request you to share your views. Thanks.

Just going to summarize what I mentioned on IRC for the benefit of the list.

Any change to the original proposal will require a userspace
implementation to go along with it. We currently only have one
userspace implementation, which is Chrome. Chrome implements the
original RFC (linked above). If you change anything from the original
RFC, you no longer have a userspace implementation.

In order to have a userspace implementation, you can choose to alter
Chrome or another compositor.

In order to change Chrome, I suspect you'll need to do one of the
following to avoid breaking devices running old kernels with the RFC
uapi:
1- Make the proposed uapi changes compatible with the original RFC
2- Change all CrOS kernels to the new proposal at the same time as
userspace changes (this is actually possible with CrOS since it's all
packaged/updated up together)

In my opinion, option 1 is going to be the easiest path forward.
Others have also suggested same. So before we get too far ahead of
ourselves with HDCP 2.2, let's figure out how to upstream *something*
with a userspace implementation. If you want to go for option 2,
you'll need to do some legwork in Chrome beforehand.

Sean


>
> --Ram


<snip>

>> > > > --
>> > > > 2.7.4
>> > > >
>> >
>>
>> --
>> Sean Paul, Software Engineer, Google / Chromium OS
Ramalingam C Aug. 2, 2017, 3:32 p.m. UTC | #6
On Wednesday 26 July 2017 11:24 PM, Sean Paul wrote:
> On Wed, Jul 26, 2017 at 12:51 PM, C, Ramalingam <ramalingam.c@intel.com> wrote:
>>> -----Original Message-----
>>> From: Sean Paul [mailto:seanpaul@chromium.org]
>>> Sent: Wednesday, July 26, 2017 8:23 PM
>>> To: C, Ramalingam <ramalingam.c@intel.com>
>>> Cc: Sean Paul <seanpaul@chromium.org>; Vetter, Daniel
>>> <daniel.vetter@intel.com>; Intel Graphics Development <intel-
>>> gfx@lists.freedesktop.org>; dri-devel <dri-devel@lists.freedesktop.org>; Daniel
>>> Vetter <daniel@ffwll.ch>
>>> Subject: Re: [RFC v3] drm/hdcp: drm enum property for CP State
>>>
>>> On Wed, Jul 26, 2017 at 03:24:10PM +0530, Ramalingam C wrote:
>>>>
>>>> On Tuesday 25 July 2017 06:04 PM, Sean Paul wrote:
>>>>> On Mon, Jul 24, 2017 at 2:12 PM, Ramalingam C <ramalingam.c@intel.com>
>>> wrote:
>>>>>> DRM connector property is created to represent the content
>>>>>> protection state of the connector and to configure the same.
>>>>>>
>>>>>> Content protection states defined:
>>>>>>           DRM_MODE_CONTENT_PROTECTION_UNSUPPORTED         -
>>> Unsupported
>>>>>>           DRM_MODE_CONTENT_PROTECTION_DISABLE             - Disabled
>>>>>>           DRM_MODE_CONTENT_PROTECTION_ENABLE              - Enabled
>>>>>>
>>>>>> v2: Redesigned the property to match with CP needs of CrOS [Sean].
>>>>>>
>>>>>> v3: Renamed the state names. Header is removed [sean].
>>>>>>
>>>>>> Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
>>>>>> ---
>>>>>>    drivers/gpu/drm/drm_connector.c | 14 ++++++++++++++
>>>>>>    include/drm/drm_mode_config.h   |  5 +++++
>>>>>>    include/uapi/drm/drm_mode.h     |  5 +++++
>>>>>>    3 files changed, 24 insertions(+)
>>>>>>
>>>>>> diff --git a/drivers/gpu/drm/drm_connector.c
>>>>>> b/drivers/gpu/drm/drm_connector.c index 5cd61af..d6aaa08 100644
>>>>>> --- a/drivers/gpu/drm/drm_connector.c
>>>>>> +++ b/drivers/gpu/drm/drm_connector.c
>>>>>> @@ -617,6 +617,13 @@ static const struct drm_prop_enum_list
>>> drm_link_status_enum_list[] = {
>>>>>>    };
>>>>>>    DRM_ENUM_NAME_FN(drm_get_link_status_name,
>>>>>> drm_link_status_enum_list)
>>>>>>
>>>>>> +static const struct drm_prop_enum_list drm_cp_enum_list[] = {
>>>>>> +       { DRM_MODE_CONTENT_PROTECTION_UNSUPPORTED,
>>> "Unsupported" },
>>>>> You're still changing the enum names from the original patch/CrOS
>>>>> implementation.
>>>>>
>>>>> https://lists.freedesktop.org/archives/dri-devel/2014-December/07333
>>>>> 6.html
>>>>>
>>>>> https://cs.chromium.org/chromium/src/ui/ozone/platform/drm/gpu/drm_d
>>>>> isplay.cc?l=27
>>>> Sean,
>>>>
>>>> I think we have bit of confusion here.
>>> Agreed :)
>>>
>>>> And in previous discussion you were fine with new state of property
>>>> that is "unsupported" - indicates no common HDCP version is supported
>>>> on HDCP src and Sink combo.
>>>>
>>>> when CP is not possible, if property exist, userspace will interpret
>>>> it as CP is supported and attempt for enabling.
>>>> I prefer indicating Unsupported state than failing such requests blindly.
>>>>
>>>> In that case to interpret the new state, we need to change CrOS User
>>>> space code.
>>>>
>>>> In the RFC you mentioned above, two version of uapi interfaces are
>>>> discussed.
>>>>          V1 uses single property to configure the CP and also for
>>>> status indication.
>>>>          V2 uses two properties one for configuring and another one for
>>>> status of Content protection.
>>>>
>>>> But CrOS user space is currently using the V1 interface.
>>>>
>>>> which will be preferred approach right now (V1/V2)?
>>>> In either way we need to change the CrOS :(
>>> We can't upstream code without a userspace implementation, so V1. Without
>>> any modifications.
>> Based on uAPI rules mentioned by Daniel, what I infer is that once a uAPI is in place,
>> usecase of uAPI cant be changed. It can be only be extended for future needs.
>>
>> So IMHO it is better to shape the userspace as per the uAPI
>> interfaces that community want to have for longer run. And mainly we should adapt to a
>> interface which can be conveniently extended for HDCP2.2 and future specs versions.
>>
>> So IMO it is worth seeing what it takes for CrOS Userspace changes for HDCP2.2 interfaces.
>> And In the interest of having HDCP2.2 in their stack sooner,  if the CrOS community or Google
>>   is interested in providing that userspace support, it will be smoother to put the
>> complete and preferred uAPI for HDCP2.2 and 1.4 in place.
>>
>> Sean, I think you will be better person to comment on this from both sides, CrOS and DRM.
>>
>>
>>> As has been mentioned before in this thread, it's a lot easier to upstream code
>>> that is proven to work, than it is to merge speculative API. Further, if this patch
>>> were merged, it would be CrOS which is currently the only consumer... not a
>>> good look :)
>> Not exactly. CrOS will be the first consumer, Android will follow soon after this.
>> We need to enable this feature on HwC for that purpose. Yup might take required time :)
>>
>>>
>>>> Lets say we need to modify CrOS user space, will there be any help for that?
>>>> I am not aware what it takes though.
>>> You could probably reach out to chromium-dev@chromium.org or the author of
>>> the Chrome hdcp implementation for help.
>> Thank you for the contact.
>> And before reaching out, it is better to conclude the HDCP1.4/2.2 uAPI interfaces that is agreeable for this community.
>>
>> In that perspective, can we consider and review the uAPI interface with two properties discussed for V2 of your earlier patches?
>> Complete use case is discussed in my previous mail.
>>
>> Based on userspace commitment for HDCP2.2, we can add blob property for SRM too.
>>
>>> Sean
>>>
>>>> I am trying to discuss both uapi versions of V1 and V2. I prefer V2 though.
>>>>
>>>> If V1 is preferred we need a single property as below "content
>>>> protection" property  with {"Unsupported", "Undesired", "Desired",
>>>> "Enabled"}
>>>>                          "Type1_desired" and "Type1_Enabled" are needed
>>>> for
>>>> HDCP2.2
>>>>
>>>>
>>>> If V2 is preferred we need two properties as below
>>>>
>>>> "content protection" property with {"Unsupported", "Undesired", "Desired"}
>>>>              ("Type1_desired" - needed For HDCP2.2) "content protection
>>>> status" property with {"Disabled", "Enabled"}
>>>>              ("Type1_enabled" - needed for HDCP2.2)
>> This uAPI interface looks fit to serve the all needs of HDCP1.4 and 2.2 as explained below.
>>
>> Sean and Daniel,
>>          I request you to share your views. Thanks.
> Just going to summarize what I mentioned on IRC for the benefit of the list.
>
> Any change to the original proposal will require a userspace
> implementation to go along with it. We currently only have one
> userspace implementation, which is Chrome. Chrome implements the
> original RFC (linked above). If you change anything from the original
> RFC, you no longer have a userspace implementation.
>
> In order to have a userspace implementation, you can choose to alter
> Chrome or another compositor.
>
> In order to change Chrome, I suspect you'll need to do one of the
> following to avoid breaking devices running old kernels with the RFC
> uapi:
> 1- Make the proposed uapi changes compatible with the original RFC
> 2- Change all CrOS kernels to the new proposal at the same time as
> userspace changes (this is actually possible with CrOS since it's all
> packaged/updated up together)
>
> In my opinion, option 1 is going to be the easiest path forward.
> Others have also suggested same. So before we get too far ahead of
> ourselves with HDCP 2.2, let's figure out how to upstream *something*
> with a userspace implementation. If you want to go for option 2,
> you'll need to do some legwork in Chrome beforehand.
>
> Sean
Based on given options, I will make proposed uapi changes compatible 
with with original RFC
to go with option 1.  I will send the updated patch in some time. Thanks 
Sean.

As per my off line discussion with Daniel, I working on using the DRM 
HDCP helper functions than
abstraction layer in DRM. Once that work is complete, I will re-spine 
the complete series for
review than phase by phase.

--Ram
>
>
>> --Ram
>
> <snip>
>
>>>>>> --
>>>>>> 2.7.4
>>>>>>
>>> --
>>> Sean Paul, Software Engineer, Google / Chromium OS
diff mbox

Patch

diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index 5cd61af..d6aaa08 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -617,6 +617,13 @@  static const struct drm_prop_enum_list drm_link_status_enum_list[] = {
 };
 DRM_ENUM_NAME_FN(drm_get_link_status_name, drm_link_status_enum_list)
 
+static const struct drm_prop_enum_list drm_cp_enum_list[] = {
+	{ DRM_MODE_CONTENT_PROTECTION_UNSUPPORTED,	"Unsupported" },
+	{ DRM_MODE_CONTENT_PROTECTION_DISABLE,		"Disabled" },
+	{ DRM_MODE_CONTENT_PROTECTION_ENABLE,		"Enabled" },
+};
+DRM_ENUM_NAME_FN(drm_get_cp_status_name, drm_cp_enum_list)
+
 /**
  * drm_display_info_set_bus_formats - set the supported bus formats
  * @info: display info to store bus formats in
@@ -789,6 +796,13 @@  int drm_connector_create_standard_properties(struct drm_device *dev)
 		return -ENOMEM;
 	dev->mode_config.link_status_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;
+	dev->mode_config.cp_property = prop;
+
 	return 0;
 }
 
diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
index 4298171..7acb8b2 100644
--- a/include/drm/drm_mode_config.h
+++ b/include/drm/drm_mode_config.h
@@ -538,6 +538,11 @@  struct drm_mode_config {
 	 */
 	struct drm_property *link_status_property;
 	/**
+	 * @cp_property: Default connector property for CP
+	 * of a connector
+	 */
+	struct drm_property *cp_property;
+	/**
 	 * @plane_type_property: Default plane property to differentiate
 	 * CURSOR, PRIMARY and OVERLAY legacy uses of planes.
 	 */
diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
index 403339f..554a770 100644
--- a/include/uapi/drm/drm_mode.h
+++ b/include/uapi/drm/drm_mode.h
@@ -127,6 +127,11 @@  extern "C" {
 #define DRM_MODE_LINK_STATUS_GOOD	0
 #define DRM_MODE_LINK_STATUS_BAD	1
 
+/* Content Protection options */
+#define DRM_MODE_CONTENT_PROTECTION_UNSUPPORTED		0
+#define DRM_MODE_CONTENT_PROTECTION_DISABLE		1
+#define DRM_MODE_CONTENT_PROTECTION_ENABLE		2
+
 /*
  * DRM_MODE_ROTATE_<degrees>
  *