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

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

On Thursday 17 May 2018 06:31 PM, Ramalingam C wrote:
>
>
> 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.
Sorry my bad, caller is already handling the error case. So such debug 
msg is needed only for disable sequence.
>>
>>> +    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