[Intel-gfx] [PATCH 5/5] drm/i915: Move hdcp msg detection into shim

Ramalingam C ramalingam.c at intel.com
Tue Feb 27 11:20:48 UTC 2018



On Monday 26 February 2018 11:31 PM, Sean Paul wrote:
> On Mon, Feb 26, 2018 at 10:42:39PM +0530, Ramalingam C wrote:
>> DP and HDMI HDCP specifications are varying with respect to
>> detecting the R0 and ksv_fifo availability.
>>
>> DP will produce CP_IRQ and set a bit for indicating the R0 and
>> FIFO_READY status.
> I'm not sure what the benefit is? Keeping things common was a deliberate
> decision on my part to keep complexity down and increase the amount of common
> code.
I am proposing this expecting that this will address the DP compliance 
failure
incase of regular repeater tests(attempt to read wrong FIFO size, due to 
wrong device count). Possibly a corner case!?.

And since anyway we have the shim for handling
the DP and HDMI specific code, i feel this will do only better but harm.

--Ram
>
> Sean
>
>> Whereas HDMI will set a bit for FIFO_READY but there is no bit
>> indication for R0 ready. And also polling of READY bit is needed for
>> HDMI as ther is no CP_IRQ.
>>
>> So Fielding the CP_IRQ for DP and the polling the READY bit for a
>> period and blindly waiting for a fixed time for R0 incase of HDMI are
>> moved into corresponding hdcp_shim.
>>
>> Signed-off-by: Ramalingam C <ramalingam.c at intel.com>
>> ---
>>   drivers/gpu/drm/i915/intel_dp.c   | 70 +++++++++++++++++++++++++++++++++++----
>>   drivers/gpu/drm/i915/intel_drv.h  |  6 ++--
>>   drivers/gpu/drm/i915/intel_hdcp.c | 39 ++--------------------
>>   drivers/gpu/drm/i915/intel_hdmi.c | 28 +++++++++++++++-
>>   4 files changed, 98 insertions(+), 45 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
>> index 2c3eb90b9499..afe310a91d3c 100644
>> --- a/drivers/gpu/drm/i915/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>> @@ -4436,6 +4436,7 @@ static bool
>>   intel_dp_short_pulse(struct intel_dp *intel_dp)
>>   {
>>   	struct drm_i915_private *dev_priv = to_i915(intel_dp_to_dev(intel_dp));
>> +	struct intel_connector *connector = intel_dp->attached_connector;
>>   	u8 sink_irq_vector = 0;
>>   	u8 old_sink_count = intel_dp->sink_count;
>>   	bool ret;
>> @@ -4470,8 +4471,11 @@ intel_dp_short_pulse(struct intel_dp *intel_dp)
>>   
>>   		if (sink_irq_vector & DP_AUTOMATED_TEST_REQUEST)
>>   			intel_dp_handle_test_request(intel_dp);
>> -		if (sink_irq_vector & (DP_CP_IRQ | DP_SINK_SPECIFIC_IRQ))
>> -			DRM_DEBUG_DRIVER("CP or sink specific irq unhandled\n");
>> +		if (sink_irq_vector & DP_CP_IRQ)
>> +			if (connector->hdcp_shim)
>> +				complete_all(&connector->cp_irq_recved);
>> +		if (sink_irq_vector & DP_SINK_SPECIFIC_IRQ)
>> +			DRM_DEBUG_DRIVER("Sink specific irq unhandled\n");
>>   	}
>>   
>>   	intel_dp_check_link_status(intel_dp);
>> @@ -5083,6 +5087,25 @@ void intel_dp_encoder_suspend(struct intel_encoder *intel_encoder)
>>   	pps_unlock(intel_dp);
>>   }
>>   
>> +static int intel_dp_hdcp_wait_for_cp_irq(struct completion *cp_irq_recved,
>> +					 int timeout)
>> +{
>> +	long ret;
>> +
>> +	if (completion_done(cp_irq_recved))
>> +		reinit_completion(cp_irq_recved);
>> +
>> +	ret = wait_for_completion_interruptible_timeout(cp_irq_recved,
>> +							msecs_to_jiffies(
>> +							timeout));
>> +	reinit_completion(cp_irq_recved);
>> +	if (ret < 0)
>> +		return (int)ret;
>> +	else if (!ret)
>> +		return -ETIMEDOUT;
>> +	return 0;
>> +}
>> +
>>   static
>>   int intel_dp_hdcp_write_an_aksv(struct intel_digital_port *intel_dig_port,
>>   				u8 *an)
>> @@ -5188,11 +5211,38 @@ int intel_dp_hdcp_repeater_present(struct intel_digital_port *intel_dig_port,
>>   	return 0;
>>   }
>>   
>> +static int intel_dp_hdcp_wait_for_r0(struct intel_digital_port *intel_dig_port)
>> +{
>> +	struct intel_dp *dp = &intel_dig_port->dp;
>> +	struct intel_connector *connector = dp->attached_connector;
>> +	int ret;
>> +	u8 bstatus;
>> +
>> +	intel_dp_hdcp_wait_for_cp_irq(&connector->cp_irq_recved, 100);
>> +
>> +	ret = drm_dp_dpcd_read(&intel_dig_port->dp.aux, DP_AUX_HDCP_BSTATUS,
>> +			       &bstatus, 1);
>> +	if (ret != 1) {
>> +		DRM_ERROR("Read bstatus from DP/AUX failed (%d)\n", ret);
>> +		return ret >= 0 ? -EIO : ret;
>> +	}
>> +
>> +	if (!(bstatus & DP_BSTATUS_R0_PRIME_READY))
>> +		return -ETIMEDOUT;
>> +
>> +	return 0;
>> +}
>> +
>>   static
>>   int intel_dp_hdcp_read_ri_prime(struct intel_digital_port *intel_dig_port,
>>   				u8 *ri_prime)
>>   {
>>   	ssize_t ret;
>> +
>> +	ret = intel_dp_hdcp_wait_for_r0(intel_dig_port);
>> +	if (!ret)
>> +		return ret;
>> +
>>   	ret = drm_dp_dpcd_read(&intel_dig_port->dp.aux, DP_AUX_HDCP_RI_PRIME,
>>   			       ri_prime, DRM_HDCP_RI_LEN);
>>   	if (ret != DRM_HDCP_RI_LEN) {
>> @@ -5203,18 +5253,26 @@ int intel_dp_hdcp_read_ri_prime(struct intel_digital_port *intel_dig_port,
>>   }
>>   
>>   static
>> -int intel_dp_hdcp_read_ksv_ready(struct intel_digital_port *intel_dig_port,
>> -				 bool *ksv_ready)
>> +int intel_dp_hdcp_ksv_ready(struct intel_digital_port *intel_dig_port)
>>   {
>> +	struct intel_dp *dp = &intel_dig_port->dp;
>> +	struct intel_connector *connector = dp->attached_connector;
>>   	ssize_t ret;
>>   	u8 bstatus;
>> +
>> +	intel_dp_hdcp_wait_for_cp_irq(&connector->cp_irq_recved,
>> +				      5 * 1000 * 1000);
>> +
>>   	ret = drm_dp_dpcd_read(&intel_dig_port->dp.aux, DP_AUX_HDCP_BSTATUS,
>>   			       &bstatus, 1);
>>   	if (ret != 1) {
>>   		DRM_ERROR("Read bstatus from DP/AUX failed (%zd)\n", ret);
>>   		return ret >= 0 ? -EIO : ret;
>>   	}
>> -	*ksv_ready = bstatus & DP_BSTATUS_READY;
>> +
>> +	if (!(bstatus & DP_BSTATUS_READY))
>> +		return -ETIMEDOUT;
>> +
>>   	return 0;
>>   }
>>   
>> @@ -5305,7 +5363,7 @@ static const struct intel_hdcp_shim intel_dp_hdcp_shim = {
>>   	.read_bstatus = intel_dp_hdcp_read_bstatus,
>>   	.repeater_present = intel_dp_hdcp_repeater_present,
>>   	.read_ri_prime = intel_dp_hdcp_read_ri_prime,
>> -	.read_ksv_ready = intel_dp_hdcp_read_ksv_ready,
>> +	.wait_for_ksv_ready = intel_dp_hdcp_ksv_ready,
>>   	.read_ksv_fifo = intel_dp_hdcp_read_ksv_fifo,
>>   	.read_v_prime_part = intel_dp_hdcp_read_v_prime_part,
>>   	.toggle_signalling = intel_dp_hdcp_toggle_signalling,
>> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
>> index 8f38e584d375..ab975107c978 100644
>> --- a/drivers/gpu/drm/i915/intel_drv.h
>> +++ b/drivers/gpu/drm/i915/intel_drv.h
>> @@ -353,8 +353,7 @@ struct intel_hdcp_shim {
>>   	int (*read_ri_prime)(struct intel_digital_port *intel_dig_port, u8 *ri);
>>   
>>   	/* Determines if the receiver's KSV FIFO is ready for consumption */
>> -	int (*read_ksv_ready)(struct intel_digital_port *intel_dig_port,
>> -			      bool *ksv_ready);
>> +	int (*wait_for_ksv_ready)(struct intel_digital_port *intel_dig_port);
>>   
>>   	/* Reads the ksv fifo for num_downstream devices */
>>   	int (*read_ksv_fifo)(struct intel_digital_port *intel_dig_port,
>> @@ -413,6 +412,9 @@ struct intel_connector {
>>   	uint64_t hdcp_value; /* protected by hdcp_mutex */
>>   	struct delayed_work hdcp_check_work;
>>   	struct work_struct hdcp_prop_work;
>> +
>> +	/* To indicate the assertion of CP_IRQ */
>> +	struct completion cp_irq_recved;
>>   };
>>   
>>   struct intel_digital_connector_state {
>> diff --git a/drivers/gpu/drm/i915/intel_hdcp.c b/drivers/gpu/drm/i915/intel_hdcp.c
>> index 14be14a45e5a..a077e1333886 100644
>> --- a/drivers/gpu/drm/i915/intel_hdcp.c
>> +++ b/drivers/gpu/drm/i915/intel_hdcp.c
>> @@ -57,27 +57,6 @@ static bool wait_for_hdcp_port(struct intel_connector *connector)
>>   	return true;
>>   }
>>   
>> -static int intel_hdcp_poll_ksv_fifo(struct intel_digital_port *intel_dig_port,
>> -				    const struct intel_hdcp_shim *shim)
>> -{
>> -	int ret, read_ret;
>> -	bool ksv_ready;
>> -
>> -	/* Poll for ksv list ready (spec says max time allowed is 5s) */
>> -	ret = __wait_for(read_ret = shim->read_ksv_ready(intel_dig_port,
>> -							 &ksv_ready),
>> -			 read_ret || ksv_ready, 5 * 1000 * 1000, 1000,
>> -			 100 * 1000);
>> -	if (ret)
>> -		return ret;
>> -	if (read_ret)
>> -		return read_ret;
>> -	if (!ksv_ready)
>> -		return -ETIMEDOUT;
>> -
>> -	return 0;
>> -}
>> -
>>   static bool hdcp_key_loadable(struct drm_i915_private *dev_priv)
>>   {
>>   	struct i915_power_domains *power_domains = &dev_priv->power_domains;
>> @@ -222,7 +201,7 @@ int intel_hdcp_auth_downstream(struct intel_digital_port *intel_dig_port,
>>   
>>   	dev_priv = intel_dig_port->base.base.dev->dev_private;
>>   
>> -	ret = intel_hdcp_poll_ksv_fifo(intel_dig_port, shim);
>> +	ret = shim->wait_for_ksv_ready(intel_dig_port);
>>   	if (ret) {
>>   		DRM_ERROR("KSV list failed to become ready (%d)\n", ret);
>>   		return ret;
>> @@ -476,7 +455,6 @@ static int intel_hdcp_auth(struct intel_digital_port *intel_dig_port,
>>   {
>>   	struct drm_i915_private *dev_priv;
>>   	enum port port;
>> -	unsigned long r0_prime_gen_start;
>>   	int ret, i, tries = 2;
>>   	union {
>>   		u32 reg[2];
>> @@ -531,8 +509,6 @@ static int intel_hdcp_auth(struct intel_digital_port *intel_dig_port,
>>   	if (ret)
>>   		return ret;
>>   
>> -	r0_prime_gen_start = jiffies;
>> -
>>   	memset(&bksv, 0, sizeof(bksv));
>>   
>>   	/* HDCP spec states that we must retry the bksv if it is invalid */
>> @@ -572,22 +548,11 @@ static int intel_hdcp_auth(struct intel_digital_port *intel_dig_port,
>>   	}
>>   
>>   	/*
>> -	 * Wait for R0' to become available. The spec says minimum 100ms from
>> -	 * Aksv, but some monitors can take longer than this. So we are
>> -	 * combinely waiting for 300mSec just to be sure in case of HDMI.
>>   	 * DP HDCP Spec mandates the two more reattempt to read R0, incase
>>   	 * of R0 mismatch.
>> -	 *
>> -	 * On DP, there's an R0_READY bit available but no such bit
>> -	 * exists on HDMI. Since the upper-bound is the same, we'll just do
>> -	 * the stupid thing instead of polling on one and not the other.
>>   	 */
>> -
>>   	tries = 3;
>>   	for (i = 0; i < tries; i++) {
>> -		wait_remaining_ms_from_jiffies(r0_prime_gen_start,
>> -					       100 * (i + 1));
>> -
>>   		ri.reg = 0;
>>   		ret = shim->read_ri_prime(intel_dig_port, ri.shim);
>>   		if (ret)
>> @@ -749,6 +714,8 @@ int intel_hdcp_init(struct intel_connector *connector,
>>   	mutex_init(&connector->hdcp_mutex);
>>   	INIT_DELAYED_WORK(&connector->hdcp_check_work, intel_hdcp_check_work);
>>   	INIT_WORK(&connector->hdcp_prop_work, intel_hdcp_prop_work);
>> +
>> +	init_completion(&connector->cp_irq_recved);
>>   	return 0;
>>   }
>>   
>> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
>> index f5d7bfb43006..532d13f91602 100644
>> --- a/drivers/gpu/drm/i915/intel_hdmi.c
>> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
>> @@ -1007,6 +1007,9 @@ int intel_hdmi_hdcp_read_ri_prime(struct intel_digital_port *intel_dig_port,
>>   				  u8 *ri_prime)
>>   {
>>   	int ret;
>> +
>> +	wait_remaining_ms_from_jiffies(jiffies, 100);
>> +
>>   	ret = intel_hdmi_hdcp_read(intel_dig_port, DRM_HDCP_DDC_RI_PRIME,
>>   				   ri_prime, DRM_HDCP_RI_LEN);
>>   	if (ret)
>> @@ -1027,6 +1030,29 @@ int intel_hdmi_hdcp_read_ksv_ready(struct intel_digital_port *intel_dig_port,
>>   		return ret;
>>   	}
>>   	*ksv_ready = val & DRM_HDCP_DDC_BCAPS_KSV_FIFO_READY;
>> +
>> +	return 0;
>> +}
>> +
>> +static
>> +int intel_hdmi_hdcp_ksv_ready(struct intel_digital_port *intel_dig_port)
>> +{
>> +	int ret, read_ret;
>> +	bool ksv_ready;
>> +
>> +	/* Poll for ksv list ready (spec says max time allowed is 5s) */
>> +	ret = __wait_for(read_ret =
>> +			 intel_hdmi_hdcp_read_ksv_ready(intel_dig_port,
>> +							&ksv_ready),
>> +			 read_ret || ksv_ready, 5 * 1000 * 1000, 1000,
>> +			 100 * 1000);
>> +	if (ret)
>> +		return ret;
>> +	if (read_ret)
>> +		return read_ret;
>> +	if (!ksv_ready)
>> +		return -ETIMEDOUT;
>> +
>>   	return 0;
>>   }
>>   
>> @@ -1112,7 +1138,7 @@ static const struct intel_hdcp_shim intel_hdmi_hdcp_shim = {
>>   	.read_bstatus = intel_hdmi_hdcp_read_bstatus,
>>   	.repeater_present = intel_hdmi_hdcp_repeater_present,
>>   	.read_ri_prime = intel_hdmi_hdcp_read_ri_prime,
>> -	.read_ksv_ready = intel_hdmi_hdcp_read_ksv_ready,
>> +	.wait_for_ksv_ready = intel_hdmi_hdcp_ksv_ready,
>>   	.read_ksv_fifo = intel_hdmi_hdcp_read_ksv_fifo,
>>   	.read_v_prime_part = intel_hdmi_hdcp_read_v_prime_part,
>>   	.toggle_signalling = intel_hdmi_hdcp_toggle_signalling,
>> -- 
>> 2.7.4
>>



More information about the Intel-gfx mailing list