Re: [PATCH v3 25/40] drm/i915: Enable and Disable HDCP2.2 port encryption

[Ramalingam C <ramalingam.c@intel.com>]

On Monday 14 May 2018 02:53 PM, Shankar, Uma wrote:
>
>> -----Original Message-----
>> From: dri-devel [mailto:dri-devel-bounces@lists.freedesktop.org] On Behalf Of
>> Ramalingam C
>> Sent: Tuesday, April 3, 2018 7:28 PM
>> To: intel-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org;
>> seanpaul@chromium.org; daniel@ffwll.ch; chris@chris-wilson.co.uk;
>> jani.nikula@linux.intel.com; Winkler, Tomas <tomas.winkler@intel.com>;
>> Usyskin, Alexander <alexander.usyskin@intel.com>
>> Cc: Vivi, Rodrigo <rodrigo.vivi@intel.com>
>> Subject: [PATCH v3 25/40] drm/i915: Enable and Disable HDCP2.2 port encryption
>>
>> Implements the enable and disable functions for HDCP2.2 encryption of the
>> PORT.
>>
>> v2:
>>   intel_wait_for_register is used instead of wait_for. [Chris Wilson]
>> v3:
>>   No Changes.
>>
>> Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
>> ---
>> drivers/gpu/drm/i915/intel_hdcp.c | 54
>> +++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 54 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_hdcp.c
>> b/drivers/gpu/drm/i915/intel_hdcp.c
>> index d70320da85e4..91cac643f083 100644
>> --- a/drivers/gpu/drm/i915/intel_hdcp.c
>> +++ b/drivers/gpu/drm/i915/intel_hdcp.c
>> @@ -19,6 +19,7 @@
>> 					 (enum hdcp_physical_port) (port))
>> #define KEY_LOAD_TRIES	5
>> #define HDCP2_LC_RETRY_CNT		3
>> +#define TIME_FOR_ENCRYPT_STATUS_CHANGE	32
>>
>> static int intel_hdcp_poll_ksv_fifo(struct intel_digital_port *intel_dig_port,
>> 				    const struct intel_hdcp_shim *shim) @@ -
>> 1330,3 +1331,56 @@ static int hdcp2_authenticate_sink(struct intel_connector
>> *connector)
>>
>> 	return ret;
>> }
>> +
>> +static int hdcp2_enable_encryption(struct intel_connector *connector) {
>> +	struct intel_digital_port *intel_dig_port = conn_to_dig_port(connector);
>> +	struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
>> +	struct intel_hdcp *hdcp = &connector->hdcp;
>> +	enum port port = connector->encoder->port;
>> +	int ret;
>> +
>> +	if (I915_READ(HDCP2_STATUS_DDI(port)) & LINK_ENCRYPTION_STATUS)
> Print a message saying "Encryption Already enabled" .
It doesn't serve any purpose uma. why do we need this?
>> +		return 0;
>> +
>> +	if (hdcp->hdcp_shim->toggle_signalling)
> Check for "hdcp->hdcp_shim" as well.
Without hdcp_shim structure, we wont reach here.
>> +		hdcp->hdcp_shim->toggle_signalling(intel_dig_port, true);
>> +
>> +	if (I915_READ(HDCP2_STATUS_DDI(port)) & LINK_AUTH_STATUS) {
>> +
>> +		/* Link is Authenticated. Now set for Encryption */
>> +		I915_WRITE(HDCP2_CTR_DDI(port),
>> +			   I915_READ(HDCP2_CTR_DDI(port)) |
>> +			   CTL_LINK_ENCRYPTION_REQ);
>> +	}
>> +
>> +	ret = intel_wait_for_register(dev_priv, HDCP2_STATUS_DDI(port),
>> +				      LINK_ENCRYPTION_STATUS,
>> +				      LINK_ENCRYPTION_STATUS,
>> +				      TIME_FOR_ENCRYPT_STATUS_CHANGE);
> Print a message in case of timeout.
Yes. Since this timeout is unexpected, debug msg would help.
>
>> +	return ret;
>> +}
>> +
>> +static int hdcp2_disable_encryption(struct intel_connector *connector)
>> +{
>> +	struct intel_digital_port *intel_dig_port = conn_to_dig_port(connector);
>> +	struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
>> +	struct intel_hdcp *hdcp = &connector->hdcp;
>> +	enum port port = connector->encoder->port;
>> +	int ret;
>> +
>> +	if (!(I915_READ(HDCP2_STATUS_DDI(port)) &
>> LINK_ENCRYPTION_STATUS))
> Put a info message saying "Already Disabled" .
I feel this msg wont help uma.
>
>> +		return 0;
>> +
>> +	I915_WRITE(HDCP2_CTR_DDI(port),
>> +		   I915_READ(HDCP2_CTR_DDI(port)) &
>> ~CTL_LINK_ENCRYPTION_REQ);
>> +
>> +	ret = intel_wait_for_register(dev_priv, HDCP2_STATUS_DDI(port),
>> +				      LINK_ENCRYPTION_STATUS, 0x0,
>> +				      TIME_FOR_ENCRYPT_STATUS_CHANGE);
> If this times out, do we still need to toggle the signalling ?
As per the Bspec Encryption will stop in a frame time. So 32mSec should 
be adequate.
So we need to toggle the hdcp signaling so that panel will know that 
unencrypted data is expected.

>> +
>> +	if (hdcp->hdcp_shim->toggle_signalling)
> Check for validity of " hdcp->hdcp_shim".
hdcp_shim will be valid at this point. but where as toggling function is 
only for hdmi.

--Ram
>
>> +		hdcp->hdcp_shim->toggle_signalling(intel_dig_port, false);
>> +
>> +	return ret;
>> +}
>> --
>> 2.7.4
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx