| Message ID | 1526907240-17639-23-git-send-email-ramalingam.c@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, May 21, 2018 at 06:23:41PM +0530, Ramalingam C wrote: > Adds the wrapper for all mei hdcp2.2 service functions. > > v2: > Rebased. > v3: > cldev is moved from mei_hdcp_data to hdcp. > v4: > %s/hdcp2_store_paring_info/hdcp2_store_pairing_info > > Signed-off-by: Ramalingam C <ramalingam.c@intel.com> This is a bit a style nit, but I'm not convinced of the value of these wrappers. They just do basic sanity checking (and with the component stuff, cldev should never be NULL before we get here), and those checks that are still needed could be done just once when we start a hdcp2 operation. Personally I'd drop these all and directly call the mei_ functions in the later patches (plus put just 1 set of the sanity checks at the beginning of a hdcp flow). More direct code is generally easier to read later on. -Daniel > --- > drivers/gpu/drm/i915/intel_hdcp.c | 194 ++++++++++++++++++++++++++++++++++++++ > 1 file changed, 194 insertions(+) > > diff --git a/drivers/gpu/drm/i915/intel_hdcp.c b/drivers/gpu/drm/i915/intel_hdcp.c > index c7d0fa319c01..57c380c91cd0 100644 > --- a/drivers/gpu/drm/i915/intel_hdcp.c > +++ b/drivers/gpu/drm/i915/intel_hdcp.c > @@ -10,10 +10,13 @@ > #include <drm/drm_hdcp.h> > #include <linux/i2c.h> > #include <linux/random.h> > +#include <linux/mei_hdcp.h> > > #include "intel_drv.h" > #include "i915_reg.h" > > +#define GET_MEI_DDI_INDEX(port) (((port) == PORT_A) ? DDI_A : \ > + (enum hdcp_physical_port) (port)) > #define KEY_LOAD_TRIES 5 > > static int intel_hdcp_poll_ksv_fifo(struct intel_digital_port *intel_dig_port, > @@ -896,3 +899,194 @@ int intel_hdcp_check_link(struct intel_connector *connector) > mutex_unlock(&hdcp->hdcp_mutex); > return ret; > } > + > +static int > +hdcp2_prepare_ake_init(struct intel_hdcp *hdcp, struct hdcp2_ake_init *ake_data) > +{ > + struct mei_hdcp_data *data = &hdcp->mei_data; > + struct intel_connector *connector = container_of(hdcp, > + struct intel_connector, > + hdcp); > + > + if (!hdcp->cldev) > + return -EINVAL; > + > + if (data->port == INVALID_PORT && connector->encoder) > + data->port = GET_MEI_DDI_INDEX(connector->encoder->port); > + > + /* Clear ME FW instance for the port, just incase */ > + mei_close_hdcp_session(hdcp->cldev, data); > + > + return mei_initiate_hdcp2_session(hdcp->cldev, data, ake_data); > +} > + > +static int hdcp2_close_mei_session(struct intel_hdcp *hdcp) > +{ > + struct mei_hdcp_data *data = &hdcp->mei_data; > + > + if (!hdcp->cldev || data->port == INVALID_PORT) > + return -EINVAL; > + > + return mei_close_hdcp_session(hdcp->cldev, data); > +} > + > +static int > +hdcp2_verify_rx_cert_prepare_km(struct intel_hdcp *hdcp, > + struct hdcp2_ake_send_cert *rx_cert, > + bool *paired, > + struct hdcp2_ake_no_stored_km *ek_pub_km, > + size_t *msg_sz) > +{ > + struct mei_hdcp_data *data = &hdcp->mei_data; > + int ret; > + > + if (!hdcp->cldev) > + return -EINVAL; > + > + ret = mei_verify_receiver_cert_prepare_km(hdcp->cldev, data, rx_cert, > + paired, ek_pub_km, msg_sz); > + if (ret < 0) > + mei_close_hdcp_session(hdcp->cldev, data); > + > + return ret; > +} > + > +static int hdcp2_verify_hprime(struct intel_hdcp *hdcp, > + struct hdcp2_ake_send_hprime *rx_hprime) > +{ > + struct mei_hdcp_data *data = &hdcp->mei_data; > + int ret; > + > + if (!hdcp->cldev) > + return -EINVAL; > + > + ret = mei_verify_hprime(hdcp->cldev, data, rx_hprime); > + if (ret < 0) > + mei_close_hdcp_session(hdcp->cldev, data); > + > + return ret; > +} > + > +static int > +hdcp2_store_pairing_info(struct intel_hdcp *hdcp, > + struct hdcp2_ake_send_pairing_info *pairing_info) > +{ > + struct mei_hdcp_data *data = &hdcp->mei_data; > + int ret; > + > + if (!hdcp->cldev) > + return -EINVAL; > + > + ret = mei_store_pairing_info(hdcp->cldev, data, pairing_info); > + if (ret < 0) > + mei_close_hdcp_session(hdcp->cldev, data); > + > + return ret; > +} > + > +static int > +hdcp2_prepare_lc_init(struct intel_hdcp *hdcp, struct hdcp2_lc_init *lc_init) > +{ > + struct mei_hdcp_data *data = &hdcp->mei_data; > + int ret; > + > + if (!hdcp->cldev) > + return -EINVAL; > + > + ret = mei_initiate_locality_check(hdcp->cldev, data, lc_init); > + if (ret < 0) > + mei_close_hdcp_session(hdcp->cldev, data); > + > + return ret; > +} > + > +static int > +hdcp2_verify_lprime(struct intel_hdcp *hdcp, > + struct hdcp2_lc_send_lprime *rx_lprime) > +{ > + struct mei_hdcp_data *data = &hdcp->mei_data; > + int ret; > + > + if (!hdcp->cldev) > + return -EINVAL; > + > + ret = mei_verify_lprime(hdcp->cldev, data, rx_lprime); > + if (ret < 0) > + mei_close_hdcp_session(hdcp->cldev, data); > + > + return ret; > +} > + > +static int hdcp2_prepare_skey(struct intel_hdcp *hdcp, > + struct hdcp2_ske_send_eks *ske_data) > +{ > + struct mei_hdcp_data *data = &hdcp->mei_data; > + int ret; > + > + if (!hdcp->cldev) > + return -EINVAL; > + > + ret = mei_get_session_key(hdcp->cldev, data, ske_data); > + if (ret < 0) > + mei_close_hdcp_session(hdcp->cldev, data); > + > + return ret; > +} > + > +static int > +hdcp2_verify_rep_topology_prepare_ack( > + struct intel_hdcp *hdcp, > + struct hdcp2_rep_send_receiverid_list *rep_topology, > + struct hdcp2_rep_send_ack *rep_send_ack) > +{ > + struct mei_hdcp_data *data = &hdcp->mei_data; > + int ret; > + > + if (!hdcp->cldev) > + return -EINVAL; > + > + ret = mei_repeater_check_flow_prepare_ack(hdcp->cldev, data, > + rep_topology, rep_send_ack); > + if (ret < 0) > + mei_close_hdcp_session(hdcp->cldev, data); > + > + return ret; > +} > + > +static int > +hdcp2_verify_mprime(struct intel_hdcp *hdcp, > + struct hdcp2_rep_stream_ready *stream_ready) > +{ > + struct mei_hdcp_data *data = &hdcp->mei_data; > + int ret; > + > + if (!hdcp->cldev) > + return -EINVAL; > + > + ret = mei_verify_mprime(hdcp->cldev, data, stream_ready); > + if (ret < 0) > + mei_close_hdcp_session(hdcp->cldev, data); > + > + return ret; > +} > + > + > +static int hdcp2_authenticate_port(struct intel_hdcp *hdcp) > +{ > + struct mei_hdcp_data *data = &hdcp->mei_data; > + int ret; > + > + if (!hdcp->cldev) > + return -EINVAL; > + > + ret = mei_enable_hdcp_authentication(hdcp->cldev, data); > + if (ret < 0) > + mei_close_hdcp_session(hdcp->cldev, data); > + > + return ret; > +} > + > +static inline int hdcp2_deauthenticate_port(struct intel_hdcp *hdcp) > +{ > + return hdcp2_close_mei_session(hdcp); > +} > -- > 2.7.4 >
On Thursday 31 May 2018 12:37 PM, Daniel Vetter wrote: > On Mon, May 21, 2018 at 06:23:41PM +0530, Ramalingam C wrote: >> Adds the wrapper for all mei hdcp2.2 service functions. >> >> v2: >> Rebased. >> v3: >> cldev is moved from mei_hdcp_data to hdcp. >> v4: >> %s/hdcp2_store_paring_info/hdcp2_store_pairing_info >> >> Signed-off-by: Ramalingam C <ramalingam.c@intel.com> > This is a bit a style nit, but I'm not convinced of the value of these > wrappers. They just do basic sanity checking (and with the component > stuff, cldev should never be NULL before we get here), and those checks > that are still needed could be done just once when we start a hdcp2 > operation. > > Personally I'd drop these all and directly call the mei_ functions in the > later patches (plus put just 1 set of the sanity checks at the beginning > of a hdcp flow). More direct code is generally easier to read later on. > -Daniel Daniel, lets say we allow dynamic component unbind on cldev removal, even then we need to check whether component is binded/unbinded before making a call to the service functions. Or may be checking the function ptr will be sufficient!? Need to check though. In that way small wrapper looks required my side. But yes, instead of all wrappers together, in v5 I am merging them into the consumers patches itself. Still if we feel wrappers are not required we could get away from it. Thanks, Ram > >> --- >> drivers/gpu/drm/i915/intel_hdcp.c | 194 ++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 194 insertions(+) >> >> diff --git a/drivers/gpu/drm/i915/intel_hdcp.c b/drivers/gpu/drm/i915/intel_hdcp.c >> index c7d0fa319c01..57c380c91cd0 100644 >> --- a/drivers/gpu/drm/i915/intel_hdcp.c >> +++ b/drivers/gpu/drm/i915/intel_hdcp.c >> @@ -10,10 +10,13 @@ >> #include <drm/drm_hdcp.h> >> #include <linux/i2c.h> >> #include <linux/random.h> >> +#include <linux/mei_hdcp.h> >> >> #include "intel_drv.h" >> #include "i915_reg.h" >> >> +#define GET_MEI_DDI_INDEX(port) (((port) == PORT_A) ? DDI_A : \ >> + (enum hdcp_physical_port) (port)) >> #define KEY_LOAD_TRIES 5 >> >> static int intel_hdcp_poll_ksv_fifo(struct intel_digital_port *intel_dig_port, >> @@ -896,3 +899,194 @@ int intel_hdcp_check_link(struct intel_connector *connector) >> mutex_unlock(&hdcp->hdcp_mutex); >> return ret; >> } >> + >> +static int >> +hdcp2_prepare_ake_init(struct intel_hdcp *hdcp, struct hdcp2_ake_init *ake_data) >> +{ >> + struct mei_hdcp_data *data = &hdcp->mei_data; >> + struct intel_connector *connector = container_of(hdcp, >> + struct intel_connector, >> + hdcp); >> + >> + if (!hdcp->cldev) >> + return -EINVAL; >> + >> + if (data->port == INVALID_PORT && connector->encoder) >> + data->port = GET_MEI_DDI_INDEX(connector->encoder->port); >> + >> + /* Clear ME FW instance for the port, just incase */ >> + mei_close_hdcp_session(hdcp->cldev, data); >> + >> + return mei_initiate_hdcp2_session(hdcp->cldev, data, ake_data); >> +} >> + >> +static int hdcp2_close_mei_session(struct intel_hdcp *hdcp) >> +{ >> + struct mei_hdcp_data *data = &hdcp->mei_data; >> + >> + if (!hdcp->cldev || data->port == INVALID_PORT) >> + return -EINVAL; >> + >> + return mei_close_hdcp_session(hdcp->cldev, data); >> +} >> + >> +static int >> +hdcp2_verify_rx_cert_prepare_km(struct intel_hdcp *hdcp, >> + struct hdcp2_ake_send_cert *rx_cert, >> + bool *paired, >> + struct hdcp2_ake_no_stored_km *ek_pub_km, >> + size_t *msg_sz) >> +{ >> + struct mei_hdcp_data *data = &hdcp->mei_data; >> + int ret; >> + >> + if (!hdcp->cldev) >> + return -EINVAL; >> + >> + ret = mei_verify_receiver_cert_prepare_km(hdcp->cldev, data, rx_cert, >> + paired, ek_pub_km, msg_sz); >> + if (ret < 0) >> + mei_close_hdcp_session(hdcp->cldev, data); >> + >> + return ret; >> +} >> + >> +static int hdcp2_verify_hprime(struct intel_hdcp *hdcp, >> + struct hdcp2_ake_send_hprime *rx_hprime) >> +{ >> + struct mei_hdcp_data *data = &hdcp->mei_data; >> + int ret; >> + >> + if (!hdcp->cldev) >> + return -EINVAL; >> + >> + ret = mei_verify_hprime(hdcp->cldev, data, rx_hprime); >> + if (ret < 0) >> + mei_close_hdcp_session(hdcp->cldev, data); >> + >> + return ret; >> +} >> + >> +static int >> +hdcp2_store_pairing_info(struct intel_hdcp *hdcp, >> + struct hdcp2_ake_send_pairing_info *pairing_info) >> +{ >> + struct mei_hdcp_data *data = &hdcp->mei_data; >> + int ret; >> + >> + if (!hdcp->cldev) >> + return -EINVAL; >> + >> + ret = mei_store_pairing_info(hdcp->cldev, data, pairing_info); >> + if (ret < 0) >> + mei_close_hdcp_session(hdcp->cldev, data); >> + >> + return ret; >> +} >> + >> +static int >> +hdcp2_prepare_lc_init(struct intel_hdcp *hdcp, struct hdcp2_lc_init *lc_init) >> +{ >> + struct mei_hdcp_data *data = &hdcp->mei_data; >> + int ret; >> + >> + if (!hdcp->cldev) >> + return -EINVAL; >> + >> + ret = mei_initiate_locality_check(hdcp->cldev, data, lc_init); >> + if (ret < 0) >> + mei_close_hdcp_session(hdcp->cldev, data); >> + >> + return ret; >> +} >> + >> +static int >> +hdcp2_verify_lprime(struct intel_hdcp *hdcp, >> + struct hdcp2_lc_send_lprime *rx_lprime) >> +{ >> + struct mei_hdcp_data *data = &hdcp->mei_data; >> + int ret; >> + >> + if (!hdcp->cldev) >> + return -EINVAL; >> + >> + ret = mei_verify_lprime(hdcp->cldev, data, rx_lprime); >> + if (ret < 0) >> + mei_close_hdcp_session(hdcp->cldev, data); >> + >> + return ret; >> +} >> + >> +static int hdcp2_prepare_skey(struct intel_hdcp *hdcp, >> + struct hdcp2_ske_send_eks *ske_data) >> +{ >> + struct mei_hdcp_data *data = &hdcp->mei_data; >> + int ret; >> + >> + if (!hdcp->cldev) >> + return -EINVAL; >> + >> + ret = mei_get_session_key(hdcp->cldev, data, ske_data); >> + if (ret < 0) >> + mei_close_hdcp_session(hdcp->cldev, data); >> + >> + return ret; >> +} >> + >> +static int >> +hdcp2_verify_rep_topology_prepare_ack( >> + struct intel_hdcp *hdcp, >> + struct hdcp2_rep_send_receiverid_list *rep_topology, >> + struct hdcp2_rep_send_ack *rep_send_ack) >> +{ >> + struct mei_hdcp_data *data = &hdcp->mei_data; >> + int ret; >> + >> + if (!hdcp->cldev) >> + return -EINVAL; >> + >> + ret = mei_repeater_check_flow_prepare_ack(hdcp->cldev, data, >> + rep_topology, rep_send_ack); >> + if (ret < 0) >> + mei_close_hdcp_session(hdcp->cldev, data); >> + >> + return ret; >> +} >> + >> +static int >> +hdcp2_verify_mprime(struct intel_hdcp *hdcp, >> + struct hdcp2_rep_stream_ready *stream_ready) >> +{ >> + struct mei_hdcp_data *data = &hdcp->mei_data; >> + int ret; >> + >> + if (!hdcp->cldev) >> + return -EINVAL; >> + >> + ret = mei_verify_mprime(hdcp->cldev, data, stream_ready); >> + if (ret < 0) >> + mei_close_hdcp_session(hdcp->cldev, data); >> + >> + return ret; >> +} >> + >> + >> +static int hdcp2_authenticate_port(struct intel_hdcp *hdcp) >> +{ >> + struct mei_hdcp_data *data = &hdcp->mei_data; >> + int ret; >> + >> + if (!hdcp->cldev) >> + return -EINVAL; >> + >> + ret = mei_enable_hdcp_authentication(hdcp->cldev, data); >> + if (ret < 0) >> + mei_close_hdcp_session(hdcp->cldev, data); >> + >> + return ret; >> +} >> + >> +static inline int hdcp2_deauthenticate_port(struct intel_hdcp *hdcp) >> +{ >> + return hdcp2_close_mei_session(hdcp); >> +} >> -- >> 2.7.4 >>
diff --git a/drivers/gpu/drm/i915/intel_hdcp.c b/drivers/gpu/drm/i915/intel_hdcp.c index c7d0fa319c01..57c380c91cd0 100644 --- a/drivers/gpu/drm/i915/intel_hdcp.c +++ b/drivers/gpu/drm/i915/intel_hdcp.c @@ -10,10 +10,13 @@ #include <drm/drm_hdcp.h> #include <linux/i2c.h> #include <linux/random.h> +#include <linux/mei_hdcp.h> #include "intel_drv.h" #include "i915_reg.h" +#define GET_MEI_DDI_INDEX(port) (((port) == PORT_A) ? DDI_A : \ + (enum hdcp_physical_port) (port)) #define KEY_LOAD_TRIES 5 static int intel_hdcp_poll_ksv_fifo(struct intel_digital_port *intel_dig_port, @@ -896,3 +899,194 @@ int intel_hdcp_check_link(struct intel_connector *connector) mutex_unlock(&hdcp->hdcp_mutex); return ret; } + +static int +hdcp2_prepare_ake_init(struct intel_hdcp *hdcp, struct hdcp2_ake_init *ake_data) +{ + struct mei_hdcp_data *data = &hdcp->mei_data; + struct intel_connector *connector = container_of(hdcp, + struct intel_connector, + hdcp); + + if (!hdcp->cldev) + return -EINVAL; + + if (data->port == INVALID_PORT && connector->encoder) + data->port = GET_MEI_DDI_INDEX(connector->encoder->port); + + /* Clear ME FW instance for the port, just incase */ + mei_close_hdcp_session(hdcp->cldev, data); + + return mei_initiate_hdcp2_session(hdcp->cldev, data, ake_data); +} + +static int hdcp2_close_mei_session(struct intel_hdcp *hdcp) +{ + struct mei_hdcp_data *data = &hdcp->mei_data; + + if (!hdcp->cldev || data->port == INVALID_PORT) + return -EINVAL; + + return mei_close_hdcp_session(hdcp->cldev, data); +} + +static int +hdcp2_verify_rx_cert_prepare_km(struct intel_hdcp *hdcp, + struct hdcp2_ake_send_cert *rx_cert, + bool *paired, + struct hdcp2_ake_no_stored_km *ek_pub_km, + size_t *msg_sz) +{ + struct mei_hdcp_data *data = &hdcp->mei_data; + int ret; + + if (!hdcp->cldev) + return -EINVAL; + + ret = mei_verify_receiver_cert_prepare_km(hdcp->cldev, data, rx_cert, + paired, ek_pub_km, msg_sz); + if (ret < 0) + mei_close_hdcp_session(hdcp->cldev, data); + + return ret; +} + +static int hdcp2_verify_hprime(struct intel_hdcp *hdcp, + struct hdcp2_ake_send_hprime *rx_hprime) +{ + struct mei_hdcp_data *data = &hdcp->mei_data; + int ret; + + if (!hdcp->cldev) + return -EINVAL; + + ret = mei_verify_hprime(hdcp->cldev, data, rx_hprime); + if (ret < 0) + mei_close_hdcp_session(hdcp->cldev, data); + + return ret; +} + +static int +hdcp2_store_pairing_info(struct intel_hdcp *hdcp, + struct hdcp2_ake_send_pairing_info *pairing_info) +{ + struct mei_hdcp_data *data = &hdcp->mei_data; + int ret; + + if (!hdcp->cldev) + return -EINVAL; + + ret = mei_store_pairing_info(hdcp->cldev, data, pairing_info); + if (ret < 0) + mei_close_hdcp_session(hdcp->cldev, data); + + return ret; +} + +static int +hdcp2_prepare_lc_init(struct intel_hdcp *hdcp, struct hdcp2_lc_init *lc_init) +{ + struct mei_hdcp_data *data = &hdcp->mei_data; + int ret; + + if (!hdcp->cldev) + return -EINVAL; + + ret = mei_initiate_locality_check(hdcp->cldev, data, lc_init); + if (ret < 0) + mei_close_hdcp_session(hdcp->cldev, data); + + return ret; +} + +static int +hdcp2_verify_lprime(struct intel_hdcp *hdcp, + struct hdcp2_lc_send_lprime *rx_lprime) +{ + struct mei_hdcp_data *data = &hdcp->mei_data; + int ret; + + if (!hdcp->cldev) + return -EINVAL; + + ret = mei_verify_lprime(hdcp->cldev, data, rx_lprime); + if (ret < 0) + mei_close_hdcp_session(hdcp->cldev, data); + + return ret; +} + +static int hdcp2_prepare_skey(struct intel_hdcp *hdcp, + struct hdcp2_ske_send_eks *ske_data) +{ + struct mei_hdcp_data *data = &hdcp->mei_data; + int ret; + + if (!hdcp->cldev) + return -EINVAL; + + ret = mei_get_session_key(hdcp->cldev, data, ske_data); + if (ret < 0) + mei_close_hdcp_session(hdcp->cldev, data); + + return ret; +} + +static int +hdcp2_verify_rep_topology_prepare_ack( + struct intel_hdcp *hdcp, + struct hdcp2_rep_send_receiverid_list *rep_topology, + struct hdcp2_rep_send_ack *rep_send_ack) +{ + struct mei_hdcp_data *data = &hdcp->mei_data; + int ret; + + if (!hdcp->cldev) + return -EINVAL; + + ret = mei_repeater_check_flow_prepare_ack(hdcp->cldev, data, + rep_topology, rep_send_ack); + if (ret < 0) + mei_close_hdcp_session(hdcp->cldev, data); + + return ret; +} + +static int +hdcp2_verify_mprime(struct intel_hdcp *hdcp, + struct hdcp2_rep_stream_ready *stream_ready) +{ + struct mei_hdcp_data *data = &hdcp->mei_data; + int ret; + + if (!hdcp->cldev) + return -EINVAL; + + ret = mei_verify_mprime(hdcp->cldev, data, stream_ready); + if (ret < 0) + mei_close_hdcp_session(hdcp->cldev, data); + + return ret; +} + + +static int hdcp2_authenticate_port(struct intel_hdcp *hdcp) +{ + struct mei_hdcp_data *data = &hdcp->mei_data; + int ret; + + if (!hdcp->cldev) + return -EINVAL; + + ret = mei_enable_hdcp_authentication(hdcp->cldev, data); + if (ret < 0) + mei_close_hdcp_session(hdcp->cldev, data); + + return ret; +} + +static inline int hdcp2_deauthenticate_port(struct intel_hdcp *hdcp) +{ + return hdcp2_close_mei_session(hdcp); +}
Adds the wrapper for all mei hdcp2.2 service functions. v2: Rebased. v3: cldev is moved from mei_hdcp_data to hdcp. v4: %s/hdcp2_store_paring_info/hdcp2_store_pairing_info Signed-off-by: Ramalingam C <ramalingam.c@intel.com> --- drivers/gpu/drm/i915/intel_hdcp.c | 194 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 194 insertions(+)