Re: [PATCH v4 22/41] drm/i915: Wrappers for mei HDCP2.2 services

[Ramalingam C <ramalingam.c@xxxxxxxxx>]

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@xxxxxxxxx>
> 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

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