Re: [Intel-gfx] [RFC v1 01/20] drm/hdcp: HDCP bitmask property for connectors

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On Thursday 13 July 2017 12:40 AM, Sean Paul wrote:
On Wed, Jul 12, 2017 at 08:26:02PM +0530, Ramalingam C wrote:
Daniel,

Thank you for reviewing the patch in short time. My views are below.


On Wednesday 12 July 2017 03:24 PM, Daniel Vetter wrote:
On Wed, Jul 12, 2017 at 01:58:45PM +0530, Ramalingam C wrote:
DRM connector property is created as bitmask to receive
HDCP enable/disable request along with content type.

And also there are Read only status bits for
	1. HDCP spec capability of the connector + panel
	2. HDCP encryption status of the connector
Hi Ram,
Thank you for posting this set! I'll do some code review in a separate
pass.
Sean, Thank you for reviewing the code.


Signed-off-by: Ramalingam C <ramalingam.c@xxxxxxxxx>
---
  drivers/gpu/drm/drm_connector.c | 30 +++++++++++++++++++++
  include/drm/drm_hdcp.h          | 58 +++++++++++++++++++++++++++++++++++++++++
  include/drm/drm_mode_config.h   |  5 ++++
  3 files changed, 93 insertions(+)
  create mode 100644 include/drm/drm_hdcp.h

diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index 8072e6e..04f8cf8 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -24,6 +24,7 @@
  #include <drm/drm_connector.h>
  #include <drm/drm_edid.h>
  #include <drm/drm_encoder.h>
+#include <drm/drm_hdcp.h>
  #include "drm_crtc_internal.h"
  #include "drm_internal.h"
@@ -617,6 +618,28 @@ 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_hdcp_enum_list[] = {
+	{ __builtin_ffs(DRM_HDCP_ENABLE) - 1,
+					"Enable HDCP Encryption" },
+	{ __builtin_ffs(DRM_HDCP_TYPE_BIT0) - 1,
+					"HDCP Content type bit0" },
+	{ __builtin_ffs(DRM_HDCP_TYPE_BIT1) - 1,
+					"HDCP Content type bit1" },
+	{ __builtin_ffs(DRM_HDCP1_SUPPORTED) - 1,
+					"HDCP1.x Supported" },
+	{ __builtin_ffs(DRM_HDCP2_SUPPORTED) - 1,
+					"HDCP2.x Supported" },
+	{ __builtin_ffs(DRM_HDCP_WIP) - 1,
+					"HDCP work in progress" },
+	{ __builtin_ffs(DRM_HDCP_AUTH_FAILED) - 1,
+					"HDCP Authentication Failed" },
+	{ __builtin_ffs(DRM_HDCP_LINK_INTEGRITY_FAILED) - 1,
+					"HDCP Link Integrity Failed" },
+	{ __builtin_ffs(DRM_HDCP_REAUTH_REQUESTED) - 1,
+					"HDCP Reauthentication Requested" },
Why all these intermediate steps and different failure modes? Either hdcp
works, or it doesnt (and we can split up with the type 0 or type 1 if
needed), but I don't know what userspace would do with all the other
stuff?
enum values  HDCP_ENABLE, HDCP_ENABLE_TYPE1 and HDCP_DISABLE along with
kobj-uevent
for HDCP state change, could do the bare minimal HDCP1.4 and HDCP2.2
configuration.

And without Type info it is not possible for HDCP2.2.
I've had requests from chrome team to expose HDCP version, so I don't think this
is too contentious.
Nice. So we could consider that we both need interface for HDCP1.4 and also HDCP2.2

Note that I've tried to use "content protection" instead of HDCP. My original
thinking was that since HDCP 1.x was broken, there was probably going to be
something else coming along. It's probably best to continue to keep the means of
protection vague, since you never know what will happen with HDCP 2.2 and what
will come after it.
Sure that will help in long run. Even I will name it as CP instead of HDCP


But to inform whether both HDCP source and Sink support HDCP1.4/2.2/None I
wanted to add
the capability bits. These capability flags could be avoided if the presence
of the drm_hdcp_property
on connector, assures the corresponding HDCP version support on both HDCP
src and sink.
But for that we need a way to detach/alter the DRM_property of connector
based on HDCP
sink's hdcp version capability. Need to explore on that direction if that is
allowed.
Meh, just change the content protection type to UNSUPPORTED or whatever.
Just thinking: In that case UNSUPPORTED will stand for not supporting any common HDCP ver
on the combination of HDCP src and sink?

In that case if HDCP src and sink combo doesn't support one of V1.4 and V2.2, then we wont
set the property state to UNSUPPORTED and
We will return error code -EINVAL for the
request (atomic ioctl) of not supported version.
Does it sound good enough?

Once HDCP protection is enabled link protection might break for many reasons
including hot unplug. So Other Error
flags are just to give more information to UMD on that scenario. But we can
avoid them by setting the state
as HDCP_DISABLE and notifying the UMD through Kobj-uevent for HDCP state
change.
I can't think of a scenario where this info is worth exposing to userspace. We
can just log failures for debugging purposes and we'll be fine.
Agreed. I will remove those error bits.

But one concern remains un-addressed that is compliance requirement will
force us to reauthenticate
in case of failures in authentication or on HDCP enabled link. In that
scenario UMD has to be in formed that
reauth is in progress. If UMD wants the port to be reenabled it has to wait
for the completion of reauth else
it has to place request for disabling the HDCP on port. We might want to add
a enum value as HDCP_DISABLE_REAUTH.
What is your opinion on this?
Let's worry about that if it actually becomes a problem. I don't anticipate
needing such fine grained controls.
As a policy, i want to retain the port in unauthenticated state, unless UMD wants it otherwise.
This policy will save the frequent sanity check (poll of RxStatus reg) of authenticated HDCP
sink from KMD part.

So when the CP protection breaks on encrypted port, I want to keep UMD informed that port is
under reauth process. So that UMD can take a call on the desired CP state of the port.

If we agree on above policy, it will be good to address the above need of interface at this point in time.

This also doesn't seem to do the lockdown mode to force hdcp mode. And
afair it also doesn't match what CrOS currently uses, which means we don't
have userspace for this.
AFAIK, Either way we don't have userspace for HDCP2.2 at present, as CrOS
interface is designed for
HDCP1.4(Need to cross check this with sean). But I will check with seanpaul
if there is any plan for extending it for HDCP2.2 too.
If so we could enable CrOS as consumer for HDCP2.2 against planned open
source consumer from my side.

As discussed in cover letter I am starting on a UMD library
which will be open sourced as a HDCP1.4 and 2.2 service provider for
chromebox stack and Android.
I'm not sure this really justifies its own library. Or at least, I hope it
doesn't. All userspace should have to do is 3 things:

1- Tell kernel it wants content protection
2- Determine what method is being used for content protection
Yes it is indirect selection of HDCP version. Userspace decides the cp for which content type
and drm decides which version can do that and implements that.

3- Determine if the display is protected at any given time
Yes it has to continue monitoring the encrypted port too. As this is needed to prevent
the
leakage of protected content on runtime encryption failed port.

This can be done in two ways
    1. through freq polling (min once in a Sec) on hdcp property.
    2. Broadcasting uevent from KMD on HDCP state change and monitor for it at UMD.
        This avoids the polling for sanity check and also for detecting request(EN/DIS able) completion.

I have opted for option 2, as that sounds better design. Please share your view on that too.
Chrome, Android (well, drm_hwc) already understand drm properties, so I'm not
sure how they would benefit from an additional layer of misdirection.
Benefit we are thinking of having a separate layer is maintainability. We can enhance the library for
future addition of HDCP versions by retaining the same interface hence not disturbing
any consumers (compositors).


Sean



        
I think the best approach would be if we simply try to upstream _exactly_
the property CrOS currently uses (not even bothering with type 0 vs type 1
if that's not required), since that is the currently only open source
userspace that asks for this.
At first stage, as we are developing the HDCP2.2 support at DRM at first
than HDCP1.4,
we need to extend CrOS property for HDCP2.2 and then upstream. That will be
the fruitful solution for us.

--Ram
  Going with a much more complicated or
different interface just because only risks a massive discussion and
subsequent long delays.
-Daniel

+};
+DRM_ENUM_NAME_FN(drm_get_hdcp_name, drm_hdcp_enum_list)
+
  /**
   * drm_display_info_set_bus_formats - set the supported bus formats
   * @info: display info to store bus formats in
@@ -789,6 +812,13 @@ int drm_connector_create_standard_properties(struct drm_device *dev)
  		return -ENOMEM;
  	dev->mode_config.link_status_property = prop;
+	prop = drm_property_create_bitmask(dev, 0, "hdcp", drm_hdcp_enum_list,
+					   ARRAY_SIZE(drm_hdcp_enum_list),
+					   DRM_HDCP_PROP_SUPPORTED_BITS);
+	if (!prop)
+		return -ENOMEM;
+	dev->mode_config.hdcp_property = prop;
+
  	return 0;
  }
diff --git a/include/drm/drm_hdcp.h b/include/drm/drm_hdcp.h
new file mode 100644
index 0000000..7cebf0f
--- /dev/null
+++ b/include/drm/drm_hdcp.h
@@ -0,0 +1,58 @@
+/*
+ * Copyright (c) 2017 Intel Corporation
+ *
+ * Permission to use, copy, modify, distribute, and sell this software and its
+ * documentation for any purpose is hereby granted without fee, provided that
+ * the above copyright notice appear in all copies and that both that copyright
+ * notice and this permission notice appear in supporting documentation, and
+ * that the name of the copyright holders not be used in advertising or
+ * publicity pertaining to distribution of the software without specific,
+ * written prior permission.  The copyright holders make no representations
+ * about the suitability of this software for any purpose.  It is provided "as
+ * is" without express or implied warranty.
+ *
+ * THE COPYRIGHT HOLDERS DISCLAIM ALL WARRANTIES WITH REGARD TO THIS SOFTWARE,
+ * INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS, IN NO
+ * EVENT SHALL THE COPYRIGHT HOLDERS BE LIABLE FOR ANY SPECIAL, INDIRECT OR
+ * CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM LOSS OF USE,
+ * DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR OTHER
+ * TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR PERFORMANCE
+ * OF THIS SOFTWARE.
+ */
+
+/*
+ * Header for HDCP specific data
+ */
+
+#ifndef __DRM_HDCP_H__
+#define __DRM_HDCP_H__
+
+/**
+ * HDCP property related information
+ */
+/* RW: HDCP Encryption Requests and Status bits */
+#define DRM_HDCP_ENABLE				BIT(0)
+#define DRM_HDCP_TYPE_BIT0			BIT(4)
+#define DRM_HDCP_TYPE_BIT1			BIT(5)
+
+/* RO: HDCP Version supported on Platform + panel */
+#define DRM_HDCP1_SUPPORTED			BIT(12)
+#define DRM_HDCP2_SUPPORTED			BIT(13)
+
+/* RO: Status of the requested operations */
+#define DRM_HDCP_WIP				BIT(20)
+#define DRM_HDCP_AUTH_FAILED			BIT(21)
+
+/* RO: Error Status From HDCP sink */
+#define DRM_HDCP_LINK_INTEGRITY_FAILED		BIT(22)
+#define DRM_HDCP_REAUTH_REQUESTED		BIT(23)
+
+#define DRM_HDCP_PROP_SUPPORTED_BITS	(DRM_HDCP_ENABLE | DRM_HDCP_TYPE_BIT0 \
+					| DRM_HDCP_TYPE_BIT1 | \
+					DRM_HDCP1_SUPPORTED | \
+					DRM_HDCP2_SUPPORTED | DRM_HDCP_WIP | \
+					DRM_HDCP_AUTH_FAILED | \
+					DRM_HDCP_LINK_INTEGRITY_FAILED | \
+					DRM_HDCP_REAUTH_REQUESTED)
+
+#endif	/* __DRM_HDCP_H__ */
diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
index 4298171..0c5bb90 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;
  	/**
+	 * @hdcp_property: Default connector property for HDCP
+	 * of a connector
+	 */
+	struct drm_property *hdcp_property;
+	/**
  	 * @plane_type_property: Default plane property to differentiate
  	 * CURSOR, PRIMARY and OVERLAY legacy uses of planes.
  	 */
-- 
2.7.4

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

        

      
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel


_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux