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

[Ramalingam C <ramalingam.c@xxxxxxxxx>]

On Tuesday 25 July 2017 06:04 PM, Sean Paul wrote:

On Mon, Jul 24, 2017 at 2:12 PM, Ramalingam C <ramalingam.c@xxxxxxxxx> 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@xxxxxxxxx>
---
 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

_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx