From mboxrd@z Thu Jan 1 00:00:00 1970 From: "C, Ramalingam" Subject: Re: [PATCH v8 03/35] linux/mei: Header for mei_hdcp driver interface Date: Fri, 7 Dec 2018 19:23:06 +0530 Message-ID: <1bde7d59-db1f-a7cf-9333-d903cafd8e42@intel.com> References: <1543315413-24302-1-git-send-email-ramalingam.c@intel.com> <1543315413-24302-4-git-send-email-ramalingam.c@intel.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============1410726975==" Return-path: In-Reply-To: <1543315413-24302-4-git-send-email-ramalingam.c@intel.com> Content-Language: en-US List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org, daniel.vetter@ffwll.ch, tomas.winkler@intel.com Cc: uma.shankar@intel.com List-Id: intel-gfx@lists.freedesktop.org This is a multi-part message in MIME format. --===============1410726975== Content-Type: multipart/alternative; boundary="------------352AE12D3D2183BC52C2D4C3" Content-Language: en-US This is a multi-part message in MIME format. --------------352AE12D3D2183BC52C2D4C3 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Hi, In one of the offline discussion Tomas has shared his review comments on v8. So I am sharing the abstract of his suggestions here for the discussion and for the agreement of interface in the community. Tomas please correct/add if I am missing any points. 1. Remove the include/linux/mei_hdcp.h to make the i915-mei interface more generic. 1. Move the definition of the struct mei_hdcp_data to i915 and mei_hdcp.c and pass the void* in the ops' functions. 2. Move the conversion of enum port value to mei_ddi_port value into mei_hdcp.c. Let I915 pass the enum port value as such. 3. Modified local definition of the struct mei_hdcp_data will looks like 4. +/* hdcp data per port */ +struct hdcp_port_data { + short int port; + u8 port_type; + u8 protocol; + u16 k; + u32 seq_num_m; + struct hdcp2_streamid_type *streams; }; 2. Add K-Doc compliant commenting in the mei_hdcp.c I have implemented these changes and posted for intel-gfx-trybot. Just incase anyone wants to refer the code please look at https://patchwork.freedesktop.org/series/53655/ . Not shared on #intel-gfx as further review discussions are on-going on intel-gfx. --Ram On 11/27/2018 4:13 PM, Ramalingam C wrote: > Data structures and Enum for the I915-MEI_HDCP interface are defined > at > > v2: > Rebased. > v3: > mei_cl_device is removed from mei_hdcp_data [Tomas] > v4: > Comment style and typo fixed [Uma] > v5: > Rebased. > v6: > No changes. > v7: > Remove redundant text from the License header > Change uintXX_t type to uXX_t types > Remove unneeded include to mei_cl_bus.h > Coding style fixed [Uma] > V8: > Tab cleanup > Fix kdoc and namespaces > Update MAINTAINERS > > Signed-off-by: Ramalingam C > Signed-off-by: Tomas Winkler > Reviewed-by: Uma Shankar > --- > MAINTAINERS | 1 + > include/linux/mei_hdcp.h | 91 ++++++++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 92 insertions(+) > create mode 100644 include/linux/mei_hdcp.h > > diff --git a/MAINTAINERS b/MAINTAINERS > index 1026150ae90f..2fd6555bf040 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -7540,6 +7540,7 @@ L: linux-kernel@vger.kernel.org > S: Supported > F: include/uapi/linux/mei.h > F: include/linux/mei_cl_bus.h > +F: include/linux/mei_hdcp.h > F: drivers/misc/mei/* > F: drivers/watchdog/mei_wdt.c > F: Documentation/misc-devices/mei/* > diff --git a/include/linux/mei_hdcp.h b/include/linux/mei_hdcp.h > new file mode 100644 > index 000000000000..716123003dd1 > --- /dev/null > +++ b/include/linux/mei_hdcp.h > @@ -0,0 +1,91 @@ > +/* SPDX-License-Identifier: (GPL-2.0+ OR BSD-3-Clause) */ > +/* > + * Copyright © 2017-2018 Intel Corporation > + * > + * Authors: > + * Ramalingam C > + */ > + > +#ifndef _LINUX_MEI_HDCP_H > +#define _LINUX_MEI_HDCP_H > + > +/** > + * enum mei_hdcp_ddi - The physical digital display interface (DDI) > + * available on the platform > + * @MEI_DDI_INVALID_PORT: Not a valid port > + * @MEI_DDI_RANGE_BEGIN: Beginning of the valid DDI port range > + * @MEI_DDI_B: Port DDI B > + * @MEI_DDI_C: Port DDI C > + * @MEI_DDI_D: Port DDI D > + * @MEI_DDI_E: Port DDI E > + * @MEI_DDI_F: Port DDI F > + * @MEI_DDI_A: Port DDI A > + * @MEI_DDI_RANGE_END: End of the valid DDI port range > + */ > +enum mei_hdcp_ddi { > + MEI_DDI_INVALID_PORT = 0x00, > + > + MEI_DDI_RANGE_BEGIN = 0x01, > + MEI_DDI_B = 0x01, > + MEI_DDI_C = 0x02, > + MEI_DDI_D = 0x03, > + MEI_DDI_E = 0x04, > + MEI_DDI_F = 0x05, > + MEI_DDI_A = 0x07, > + MEI_DDI_RANGE_END = MEI_DDI_A, > +}; > + > +/** > + * enum mei_hdcp_port_type - The types of HDCP 2.2 ports supported > + * > + * @MEI_HDCP_PORT_TYPE_INVALID: Invalid port > + * @MEI_HDCP_PORT_TYPE_INTEGRATED: ports that are integrated into Intel HW > + * @MEI_HDCP_PORT_TYPE_LSPCON: discrete wired Tx port with LSPCON (HDMI 2.0) > + * @MEI_HDCP_PORT_TYPE_CPDP: discrete wired Tx port using the CPDP (DP 1.3) > + */ > +enum mei_hdcp_port_type { > + MEI_HDCP_PORT_TYPE_INVALID = 0x00, > + MEI_HDCP_PORT_TYPE_INTEGRATED = 0x01, > + MEI_HDCP_PORT_TYPE_LSPCON = 0x02, > + MEI_HDCP_PORT_TYPE_CPDP = 0x03, > +}; > + > +/* > + * enum mei_hdcp_wired_protocol - Supported integrated wired HDCP protocol. > + * @HDCP_PROTOCOL_INVALID: invalid type > + * @HDCP_PROTOCOL_HDMI: HDMI > + * @HDCP_PROTOCOL_DP: DP > + * > + * Based on this value, Minor difference needed between wired specifications > + * are handled. > + */ > +enum mei_hdcp_wired_protocol { > + MEI_HDCP_PROTOCOL_INVALID, > + MEI_HDCP_PROTOCOL_HDMI, > + MEI_HDCP_PROTOCOL_DP > +}; > + > +/** > + * struct mei_hdcp_data - Input data to the mei_hdcp APIs > + * @port: The physical port (ddi). > + * @port_type: The port type. > + * @protocol: The Protocol on the port. > + * @k: Number of streams transmitted on the port. > + * In case of HDMI & DP SST, a single stream will be > + * transmitted on the port. > + * @seq_num_m: A sequence number of RepeaterAuth_Stream_Manage msg propagated. > + * Initialized to 0 on AKE_INIT. Incremented after every successful > + * transmission of RepeaterAuth_Stream_Manage message. When it rolls > + * over re-Auth has to be triggered. > + * @streams: array[k] of streamid > + */ > +struct mei_hdcp_data { > + enum mei_hdcp_ddi port; > + enum mei_hdcp_port_type port_type; > + enum mei_hdcp_wired_protocol protocol; > + u16 k; > + u32 seq_num_m; > + struct hdcp2_streamid_type *streams; > +}; > + > +#endif /* !_LINUX_MEI_HDCP_H */ --------------352AE12D3D2183BC52C2D4C3 Content-Type: text/html; charset=utf-8 Content-Transfer-Encoding: 8bit
Hi,

In one of the offline discussion Tomas has shared his review comments on v8.
So I am sharing the abstract of his suggestions here for the discussion and for the agreement of interface in the community.
Tomas please correct/add if I am missing any points.

  1. Remove the include/linux/mei_hdcp.h to make the i915-mei interface more generic.
    1. Move the definition of the struct mei_hdcp_data to i915 and mei_hdcp.c and pass the void* in the ops' functions.
    2. Move the conversion of enum port value to mei_ddi_port value into mei_hdcp.c. Let I915 pass the enum port value as such.
    3. Modified local definition of the struct mei_hdcp_data will looks like
    4. +/* h
       dcp data per port */
      +struct hdcp_port_data {
      +	short int port;
      +	u8 port_type;
      +	u8 protocol;
      +	u16 k;
      +	u32 seq_num_m;
      +	struct hdcp2_streamid_type *streams;
       };
  2. Add K-Doc compliant commenting in the mei_hdcp.c
I have implemented these changes and posted for intel-gfx-trybot. Just incase anyone wants to
refer the code please look at https://patchwork.freedesktop.org/series/53655/ .
Not shared on #intel-gfx as further review discussions are on-going on intel-gfx.

--Ram
On 11/27/2018 4:13 PM, Ramalingam C wrote:
Data structures and Enum for the I915-MEI_HDCP interface are defined
at <linux/mei_hdcp.h>

v2:
  Rebased.
v3:
  mei_cl_device is removed from mei_hdcp_data [Tomas]
v4:
  Comment style and typo fixed [Uma]
v5:
  Rebased.
v6:
  No changes.
v7:
  Remove redundant text from the License header
  Change uintXX_t type to uXX_t types
  Remove unneeded include to mei_cl_bus.h
  Coding style fixed [Uma]
V8:
  Tab cleanup
  Fix kdoc and namespaces
  Update MAINTAINERS

Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
Reviewed-by: Uma Shankar <uma.shankar@intel.com>
---
 MAINTAINERS              |  1 +
 include/linux/mei_hdcp.h | 91 ++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 92 insertions(+)
 create mode 100644 include/linux/mei_hdcp.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 1026150ae90f..2fd6555bf040 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7540,6 +7540,7 @@ L:	linux-kernel@vger.kernel.org
 S:	Supported
 F:	include/uapi/linux/mei.h
 F:	include/linux/mei_cl_bus.h
+F:	include/linux/mei_hdcp.h
 F:	drivers/misc/mei/*
 F:	drivers/watchdog/mei_wdt.c
 F:	Documentation/misc-devices/mei/*
diff --git a/include/linux/mei_hdcp.h b/include/linux/mei_hdcp.h
new file mode 100644
index 000000000000..716123003dd1
--- /dev/null
+++ b/include/linux/mei_hdcp.h
@@ -0,0 +1,91 @@
+/* SPDX-License-Identifier: (GPL-2.0+ OR BSD-3-Clause) */
+/*
+ * Copyright © 2017-2018 Intel Corporation
+ *
+ * Authors:
+ * Ramalingam C <ramalingam.c@intel.com>
+ */
+
+#ifndef _LINUX_MEI_HDCP_H
+#define _LINUX_MEI_HDCP_H
+
+/**
+ * enum mei_hdcp_ddi - The physical digital display interface (DDI)
+ *     available on the platform
+ * @MEI_DDI_INVALID_PORT: Not a valid port
+ * @MEI_DDI_RANGE_BEGIN: Beginning of the valid DDI port range
+ * @MEI_DDI_B: Port DDI B
+ * @MEI_DDI_C: Port DDI C
+ * @MEI_DDI_D: Port DDI D
+ * @MEI_DDI_E: Port DDI E
+ * @MEI_DDI_F: Port DDI F
+ * @MEI_DDI_A: Port DDI A
+ * @MEI_DDI_RANGE_END: End of the valid DDI port range
+ */
+enum mei_hdcp_ddi {
+	MEI_DDI_INVALID_PORT = 0x00,
+
+	MEI_DDI_RANGE_BEGIN = 0x01,
+	MEI_DDI_B           = 0x01,
+	MEI_DDI_C           = 0x02,
+	MEI_DDI_D           = 0x03,
+	MEI_DDI_E           = 0x04,
+	MEI_DDI_F           = 0x05,
+	MEI_DDI_A           = 0x07,
+	MEI_DDI_RANGE_END   = MEI_DDI_A,
+};
+
+/**
+ * enum mei_hdcp_port_type - The types of HDCP 2.2 ports supported
+ *
+ * @MEI_HDCP_PORT_TYPE_INVALID: Invalid port
+ * @MEI_HDCP_PORT_TYPE_INTEGRATED: ports that are integrated into Intel HW
+ * @MEI_HDCP_PORT_TYPE_LSPCON: discrete wired Tx port with LSPCON (HDMI 2.0)
+ * @MEI_HDCP_PORT_TYPE_CPDP: discrete wired Tx port using the CPDP (DP 1.3)
+ */
+enum mei_hdcp_port_type {
+	MEI_HDCP_PORT_TYPE_INVALID    = 0x00,
+	MEI_HDCP_PORT_TYPE_INTEGRATED = 0x01,
+	MEI_HDCP_PORT_TYPE_LSPCON     = 0x02,
+	MEI_HDCP_PORT_TYPE_CPDP       = 0x03,
+};
+
+/*
+ * enum mei_hdcp_wired_protocol - Supported integrated wired HDCP protocol.
+ * @HDCP_PROTOCOL_INVALID: invalid type
+ * @HDCP_PROTOCOL_HDMI: HDMI
+ * @HDCP_PROTOCOL_DP: DP
+ *
+ * Based on this value, Minor difference needed between wired specifications
+ * are handled.
+ */
+enum mei_hdcp_wired_protocol {
+	MEI_HDCP_PROTOCOL_INVALID,
+	MEI_HDCP_PROTOCOL_HDMI,
+	MEI_HDCP_PROTOCOL_DP
+};
+
+/**
+ * struct mei_hdcp_data - Input data to the mei_hdcp APIs
+ * @port: The physical port (ddi).
+ * @port_type: The port type.
+ * @protocol: The Protocol on the port.
+ * @k: Number of streams transmitted on the port.
+ *     In case of HDMI & DP SST, a single stream will be
+ *     transmitted on the port.
+ * @seq_num_m: A sequence number of RepeaterAuth_Stream_Manage msg propagated.
+ *     Initialized to 0 on AKE_INIT. Incremented after every successful
+ *     transmission of RepeaterAuth_Stream_Manage message. When it rolls
+ *     over re-Auth has to be triggered.
+ * @streams: array[k] of streamid
+ */
+struct mei_hdcp_data {
+	enum mei_hdcp_ddi port;
+	enum mei_hdcp_port_type port_type;
+	enum mei_hdcp_wired_protocol protocol;
+	u16 k;
+	u32 seq_num_m;
+	struct hdcp2_streamid_type *streams;
+};
+
+#endif /* !_LINUX_MEI_HDCP_H */
--------------352AE12D3D2183BC52C2D4C3-- --===============1410726975== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KZHJpLWRldmVs IG1haWxpbmcgbGlzdApkcmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHBzOi8vbGlz dHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8vZHJpLWRldmVsCg== --===============1410726975==--