[Intel-gfx] [PATCH 25/43] drm/i915: Define HDCP2.2 related variables

[Ramalingam C]

On Thursday 22 February 2018 08:29 PM, Sean Paul wrote:
> On Wed, Feb 14, 2018 at 07:43:40PM +0530, Ramalingam C wrote:
>> For upcoming implementation of HDCP2.2 in I915, all variable required
>> for HDCP2.2 are defined.
>>
>> This includes a translation layer called hdcp2_shim for encoder
>> specific HDCP2.2 spec deviations.
>>
>> Signed-off-by: Ramalingam C <ramalingam.c at intel.com>
>> ---
>>   drivers/gpu/drm/i915/i915_drv.h  |  5 +++
>>   drivers/gpu/drm/i915/intel_drv.h | 91 ++++++++++++++++++++++++++++++++++++++++
>>   2 files changed, 96 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>> index 3f8d824dc313..09d31db90a38 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -44,6 +44,7 @@
>>   #include <linux/pm_qos.h>
>>   #include <linux/reservation.h>
>>   #include <linux/shmem_fs.h>
>> +#include <linux/mei_hdcp.h>
>>   
>>   #include <drm/drmP.h>
>>   #include <drm/intel-gtt.h>
>> @@ -2365,6 +2366,10 @@ struct drm_i915_private {
>>   
>>   	struct i915_pmu pmu;
>>   
>> +	/* Mei interface handle */
>> +	struct mei_cl_device *mei_cldev;
>> +	int mei_ref_cnt;
>> +
>>   	/*
>>   	 * NOTE: This is the dri1/ums dungeon, don't add stuff here. Your patch
>>   	 * will be rejected. Instead look for a better place.
>> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
>> index 1c0d324c1044..9c05049c25d5 100644
>> --- a/drivers/gpu/drm/i915/intel_drv.h
>> +++ b/drivers/gpu/drm/i915/intel_drv.h
>> @@ -301,6 +301,62 @@ struct intel_panel {
>>   	} backlight;
>>   };
>>   
>> +struct intel_hdcp;
> Why?
Sorry this is left over from previous version of hdcp2_shim. I will 
remove it.
>
>> +
>> +#define HDCP_CAPABILITY				BIT(0)
>> +#define HDCP2_CAPABILITY			BIT(1)
> These names are pretty vague, without a comment it's hard to know what/where
> they're used.
These bit definitions are added to update and check the hdcp version 
capability in platform and panel.
If plat_cap and panel_cap are dropped, these definitions are irrelevant.
>
> It's really hard to review changes where things are just defined. Can you please
> squash this patch into the patch where you actually use it?
>
>> +
>> +/*
>> + * This structure serves as a translation layer between the generic HDCP2.2 code
>> + * and the bus-specific code. What that means is that HDCP over HDMI differs
>> + * from HDCP over DP, so to account for these differences, we need to
>> + * communicate with the receiver through this shim.
>> + *
>> + * For completeness, the 2 buses differ in the following ways:
>> + *	- DP AUX vs. DDC
>> + *		HDCP registers on the receiver are set via DP AUX for DP, and
>> + *		they are set via DDC for HDMI.
>> + *	- Receiver register offsets
>> + *		The offsets of the registers are different for DP vs. HDMI
>> + *	- Receiver register masks/offsets
>> + *		Few register bit definitions are different between DP and HDMI.
>> + *	- HDCP signalling
>> + *		Needed for HDMI but not for DP.
>> + *	- Configure stream type onto receiver
>> + *		Needed only for DP but not for HDMI.
>> + */
>> +struct intel_hdcp2_shim {
> Why don't you just use the same shim? No need to duplicate.
If that is ok, i would prefer to maintain hdcp1.4 and hdcp2.2 
definitions and stack independently.
IMO in that way it will be easy to debug and maintain the  issues from 
the corresponding versions.
Please let me know if you feel other way around.
>
>> +
>> +	/* Write HDCP2.2 messages */
>> +	int (*write_msg)(struct intel_digital_port *intel_dig_port, void *buf,
>> +			 size_t size);
>> +
>> +	/* Read HDCP2.2 messages */
>> +	int (*read_msg)(struct intel_digital_port *intel_dig_port,
>> +			uint8_t msg_id, void *buf, size_t size);
>> +
>> +	/*
>> +	 * Implementation of DP Errata for the communication of stream type to
>> +	 * Receivers. Not applicable for HDMI.
>> +	 */
>> +	int (*config_stream_type)(struct intel_digital_port *intel_dig_port,
>> +				  void *buf, size_t size);
>> +
>> +	/* Enables HDCP signalling on the port. Not applicable for DP. */
>> +	int (*toggle_signalling)(struct intel_digital_port *intel_dig_port,
>> +				 bool enable);
>> +
>> +	/* Link Integrity Check */
>> +	int (*check_link)(struct intel_digital_port *intel_dig_port);
>> +
>> +	/* Detects whether Panel is HDCP2.2 capable */
>> +	int (*hdcp_capable)(struct intel_digital_port *intel_dig_port,
>> +			    bool *capable);
>> +
>> +	/* Detects the HDCP protocol(DP/HDMI) required on the port */
>> +	enum hdcp_protocol (*hdcp_protocol)(void);
>> +};
>> +
>>   /*
>>    * This structure serves as a translation layer between the generic HDCP code
>>    * and the bus-specific code. What that means is that HDCP over HDMI differs
>> @@ -378,6 +434,10 @@ struct intel_hdcp_shim {
>>   struct intel_hdcp {
>>   	struct intel_connector *connector;
>>   
>> +	/* Flags to represent the HDCP capability of the platform and panel */
>> +	u32 plat_cap;
> I don't think you need this. A simple bool supports_hdcp2 would suffice here.
Added this to facilitate, multiple version support. Anyway lets handle 
issue at hand 2.2 first :)
I will replace it with bool.
>
>> +	u32 panel_cap;
> This should not be stored here. At best it will be complicated to keep
> up-to-date and at worst, it will get stale.
At present this is getting updated and used at the start of authentication.
So as of now we could move to local variable itself. I will bring in the 
change. Thanks

--Ram
>
>> +
>>   	const struct intel_hdcp_shim *hdcp_shim;
>>   	struct mutex hdcp_mutex;
>>   	uint64_t hdcp_value; /* protected by hdcp_mutex */
>> @@ -387,6 +447,37 @@ struct intel_hdcp {
>>   
>>   	/* To indicate the assertion of CP_IRQ */
>>   	struct completion cp_irq_recved;
>> +
>> +	/* HDCP2.2 related definitions */
>> +
>> +	/* Content Stream Type defined by content owner */
>> +	u8 content_type;
>> +
>> +	bool is_paired;
>> +	bool is_repeater;
>> +
>> +	/*
>> +	 * Count of ReceiverID_List received. Initialized to 0 at AKE_INIT.
>> +	 * Incremented after processing the RepeaterAuth_Send_ReceiverID_List.
>> +	 * When it rolls over re-auth has to be triggered.
>> +	 */
>> +	uint32_t seq_num_v;
>> +
>> +	/*
>> +	 * Count 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.
>> +	 */
>> +	uint32_t seq_num_m;
>> +
>> +	/* Translation layer for encoder specific functions */
>> +	const struct intel_hdcp2_shim *hdcp2_shim;
>> +
>> +	/* mei interface related information */
>> +	struct mei_hdcp_data *mei_hdcp_data;
>> +
>> +	struct delayed_work hdcp2_check_work;
>>   };
>>   
>>   struct intel_connector {
>> -- 
>> 2.7.4
>>