> -----Original Message-----
> From: Winkler, Tomas
> Sent: Friday, February 8, 2019 3:05 AM
> To: C, Ramalingam <ramalingam.c@xxxxxxxxx>; intel-gfx@xxxxxxxxxxxxxxxxxxxxx;
> dri-devel@xxxxxxxxxxxxxxxxxxxxx; daniel.vetter@xxxxxxxx; Shankar, Uma
> <uma.shankar@xxxxxxxxx>
> Subject: RE: [PATCH v11 28/42] misc/mei/hdcp: Initiate Wired HDCP2.2 Tx
> Session
>
> > Request ME FW to start the HDCP2.2 session for an intel port.
> > Prepares payloads for command WIRED_INITIATE_HDCP2_SESSION and sends
> > to ME FW.
> >
> > On Success, ME FW will start a HDCP2.2 session for the port and
> > provides the content for HDCP2.2 AKE_Init message.
> >
> > v2: Rebased.
> > v3:
> > cldev is add as a separate parameter [Tomas]
> > Redundant comment and typecast are removed [Tomas]
> > v4:
> > %zd is used for size [Alexander]
> > %s/return -1/return -EIO [Alexander]
> > Spellings in commit msg is fixed [Uma]
> > v5: Rebased.
> > v6:
> > Collected the rb-ed by.
> > Realigning the patches in the series.
> > v7:
> > Adjust to the new mei interface.
> > Fix for kdoc.
> > v8:
> > K-Doc Addition.
> > memcpy for const length.
> > v9:
> > s/mei_hdcp_ddi/mei_fw_ddi
> > s/i915_port/mei_i915_port [Tomas]
> > renamed func as mei_hdcp_* [Tomas]
> > Instead of macro, inline func for ddi index is used. [Tomas]
> >
> > Signed-off-by: Ramalingam C <ramalingam.c@xxxxxxxxx>
> > Reviewed-by: Uma Shankar <uma.shankar@xxxxxxxxx>
> > ---
> > drivers/misc/mei/hdcp/mei_hdcp.c | 89
> > ++++++++++++++++++++++++++++++++++++++++
> > drivers/misc/mei/hdcp/mei_hdcp.h | 23 +++++++++++
> > 2 files changed, 112 insertions(+)
> >
> > diff --git a/drivers/misc/mei/hdcp/mei_hdcp.c
> > b/drivers/misc/mei/hdcp/mei_hdcp.c
> > index b8580b91e255..77dda13fcc88 100644
> > --- a/drivers/misc/mei/hdcp/mei_hdcp.c
> > +++ b/drivers/misc/mei/hdcp/mei_hdcp.c
> > @@ -23,6 +23,95 @@
> > #include <linux/slab.h>
> > #include <linux/uuid.h>
> > #include <linux/mei_cl_bus.h>
> > +#include <drm/drm_connector.h>
> > +#include <drm/i915_component.h>
> > +#include <drm/i915_mei_hdcp_interface.h>
> > +
> > +#include "mei_hdcp.h"
> > +
> > +static inline u8 mei_get_ddi_index(short int port) {
> > + enum mei_i915_port i915_port = (enum mei_i915_port)port;
> > +
> > + return (u8)(i915_port == PORT_A ? MEI_DDI_A : i915_port); }
>
> This translations from enum port to short int, than to eum mei_i915_port and
> then to u8 bothers me.
> Should be a cleaner
> Maybe it's a bit overhead but it will be much more readlbe if we do switch
> statement here:
>
> switch (port)
> {
> case PORT_A:
> return MEI_DDI_A:
> case PORT_B:
> return MEI_DDI_B
> etc.
> case: I915_MAX_PORTS
> default:
> MEI_DDI_INVALID_PORT
> }
> Otherwise looks good to me.
Only one value differs by value that is PORTA. All other input and output values are same.
In this situation switch will be overkill. Let me know if you still insist.
-Ram
>
> > +
> > +/**
> > + * mei_hdcp_initiate_session() - Initiate a Wired HDCP2.2 Tx Session
> > +in ME FW
> > + * @dev: device corresponding to the mei_cl_device
> > + * @hdcp_data: Intel HW specific hdcp data
> > + * @ake_data: AKE_Init msg output.
> > + *
> > + * Return: 0 on Success, <0 on Failure.
> > + */
> > +static int
> > +mei_hdcp_initiate_session(struct device *dev, struct hdcp_port_data *data,
> > + struct hdcp2_ake_init *ake_data) {
> > + struct wired_cmd_initiate_hdcp2_session_in session_init_in = { { 0 } };
> > + struct wired_cmd_initiate_hdcp2_session_out
> > + session_init_out = { { 0 } };
> > + struct mei_cl_device *cldev;
> > + ssize_t byte;
> > +
> > + if (!dev || !data || !ake_data)
> > + return -EINVAL;
> > +
> > + cldev = to_mei_cl_device(dev);
> > +
> > + session_init_in.header.api_version = HDCP_API_VERSION;
> > + session_init_in.header.command_id =
> > WIRED_INITIATE_HDCP2_SESSION;
> > + session_init_in.header.status = ME_HDCP_STATUS_SUCCESS;
> > + session_init_in.header.buffer_len =
> > +
> > WIRED_CMD_BUF_LEN_INITIATE_HDCP2_SESSION_IN;
> > +
> > + session_init_in.port.integrated_port_type = data->port_type;
> > + session_init_in.port.physical_port = mei_get_ddi_index(data->port);
> > + session_init_in.protocol = data->protocol;
> > +
> > + byte = mei_cldev_send(cldev, (u8 *)&session_init_in,
> > + sizeof(session_init_in));
> > + if (byte < 0) {
> > + dev_dbg(dev, "mei_cldev_send failed. %zd\n", byte);
> > + return byte;
> > + }
> > +
> > + byte = mei_cldev_recv(cldev, (u8 *)&session_init_out,
> > + sizeof(session_init_out));
> > + if (byte < 0) {
> > + dev_dbg(dev, "mei_cldev_recv failed. %zd\n", byte);
> > + return byte;
> > + }
> > +
> > + if (session_init_out.header.status != ME_HDCP_STATUS_SUCCESS) {
> > + dev_dbg(dev, "ME cmd 0x%08X Failed. Status: 0x%X\n",
> > + WIRED_INITIATE_HDCP2_SESSION,
> > + session_init_out.header.status);
> > + return -EIO;
> > + }
> > +
> > + ake_data->msg_id = HDCP_2_2_AKE_INIT;
> > + ake_data->tx_caps = session_init_out.tx_caps;
> > + memcpy(ake_data->r_tx, session_init_out.r_tx, HDCP_2_2_RTX_LEN);
> > +
> > + return 0;
> > +}
> > +
> > +static __attribute__((unused))
> > +struct i915_hdcp_component_ops mei_hdcp_ops = {
> > + .owner = THIS_MODULE,
> > + .initiate_hdcp2_session = mei_hdcp_initiate_session,
> > + .verify_receiver_cert_prepare_km = NULL,
> > + .verify_hprime = NULL,
> > + .store_pairing_info = NULL,
> > + .initiate_locality_check = NULL,
> > + .verify_lprime = NULL,
> > + .get_session_key = NULL,
> > + .repeater_check_flow_prepare_ack = NULL,
> > + .verify_mprime = NULL,
> > + .enable_hdcp_authentication = NULL,
> > + .close_hdcp_session = NULL,
> > +};
> >
> > static int mei_hdcp_probe(struct mei_cl_device *cldev,
> > const struct mei_cl_device_id *id) diff --git
> > a/drivers/misc/mei/hdcp/mei_hdcp.h b/drivers/misc/mei/hdcp/mei_hdcp.h
> > index 582a7e27ae29..28686f2ae88c 100644
> > --- a/drivers/misc/mei/hdcp/mei_hdcp.h
> > +++ b/drivers/misc/mei/hdcp/mei_hdcp.h
> > @@ -363,4 +363,27 @@ struct wired_cmd_repeater_auth_stream_req_out {
> > struct hdcp_port_id port;
> > } __packed;
> >
> > +enum mei_fw_ddi {
> > + MEI_DDI_INVALID_PORT = 0x0,
> > +
> > + MEI_DDI_B = 1,
> > + MEI_DDI_C,
> > + MEI_DDI_D,
> > + MEI_DDI_E,
> > + MEI_DDI_F,
> > + MEI_DDI_A = 7,
> > + MEI_DDI_RANGE_END = MEI_DDI_A,
> > +};
> > +
> > +enum mei_i915_port {
> This is the same as enum port why not using that and keep them in sync.
> > + PORT_NONE = -1,
> > +
> > + PORT_A = 0,
> > + PORT_B,
> > + PORT_C,
> > + PORT_D,
> > + PORT_E,
> > + PORT_F,
> > + I915_MAX_PORTS,
> > +};
> > #endif /* __MEI_HDCP_H__ */
> > --
> > 2.7.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
