On Fri, Feb 2, 2018 at 9:51 AM, Ramalingam C <ramalingam.c@xxxxxxxxx> wrote:
>
>
> On Friday 02 February 2018 08:15 PM, Sean Paul wrote:
>>
>> On Fri, Feb 02, 2018 at 07:52:24PM +0530, Ramalingam C wrote:
>>>
>>>
>>> On Friday 02 February 2018 07:39 PM, Sean Paul wrote:
>>>>
>>>> On Fri, Feb 02, 2018 at 04:15:13PM +0530, Ramalingam C wrote:
>>>>>
>>>>> We enable the HDCP encryption as a part of first stage authentication.
>>>>> So when second stage authentication fails, we need to disable the HDCP
>>>>> encryption and signalling.
>>>>>
>>>>> This patch handles above requirement.
>>>>>
>>>>> For reusing the _intel_hdcp_disable from generic authentication flow,
>>>>> this patch retain the reference to connector till the generic hdcp
>>>>> flow.
>>>>> This will be handy to provide the connector details in HDCP enable
>>>>> debug info.
>>>>>
>>>>> Signed-off-by: Ramalingam C <ramalingam.c@xxxxxxxxx>
>>>>> ---
>>>>> drivers/gpu/drm/i915/intel_hdcp.c | 41
>>>>> +++++++++++++++++++++++----------------
>>>>> 1 file changed, 24 insertions(+), 17 deletions(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/i915/intel_hdcp.c
>>>>> b/drivers/gpu/drm/i915/intel_hdcp.c
>>>>> index b97184eccd9c..0021511ae4d7 100644
>>>>> --- a/drivers/gpu/drm/i915/intel_hdcp.c
>>>>> +++ b/drivers/gpu/drm/i915/intel_hdcp.c
>>>>> @@ -16,6 +16,8 @@
>>>>> #define KEY_LOAD_TRIES 5
>>>>> +static int _intel_hdcp_disable(struct intel_connector *connector);
>>>>> +
>>>>> static int intel_hdcp_poll_ksv_fifo(struct intel_digital_port
>>>>> *intel_dig_port,
>>>>> const struct intel_hdcp_shim *shim)
>>>>> {
>>>>> @@ -138,17 +140,24 @@ bool intel_hdcp_is_ksv_valid(u8 *ksv)
>>>>> return true;
>>>>> }
>>>>> +static
>>>>> +struct intel_digital_port *conn_to_dig_port(struct intel_connector
>>>>> *connector)
>>>>> +{
>>>>> + return enc_to_dig_port(&connector->encoder->base);
>>>>> +}
>>>>> +
>>>>> /* Implements Part 2 of the HDCP authorization procedure */
>>>>> static
>>>>> -int intel_hdcp_auth_downstream(struct intel_digital_port
>>>>> *intel_dig_port,
>>>>> - const struct intel_hdcp_shim *shim)
>>>>> +int intel_hdcp_auth_downstream(struct intel_connector *connector)
>>>>> {
>>>>> struct drm_i915_private *dev_priv;
>>>>> u32 vprime, sha_text, sha_leftovers, rep_ctl;
>>>>> u8 bstatus[2], num_downstream, *ksv_fifo;
>>>>> int ret, i, j, sha_idx;
>>>>> + const struct intel_hdcp_shim *shim = connector->hdcp_shim;
>>>>> + struct intel_digital_port *intel_dig_port =
>>>>> conn_to_dig_port(connector);
>>>>> - dev_priv = intel_dig_port->base.base.dev->dev_private;
>>>>> + dev_priv = to_i915(connector->base.dev);
>>>>> ret = intel_hdcp_poll_ksv_fifo(intel_dig_port, shim);
>>>>> if (ret) {
>>>>> @@ -385,8 +394,7 @@ int intel_hdcp_auth_downstream(struct
>>>>> intel_digital_port *intel_dig_port,
>>>>> }
>>>>> /* Implements Part 1 of the HDCP authorization procedure */
>>>>> -static int intel_hdcp_auth(struct intel_digital_port *intel_dig_port,
>>>>> - const struct intel_hdcp_shim *shim)
>>>>> +static int intel_hdcp_auth(struct intel_connector *connector)
>>>>> {
>>>>> struct drm_i915_private *dev_priv;
>>>>> enum port port;
>>>>> @@ -405,9 +413,10 @@ static int intel_hdcp_auth(struct
>>>>> intel_digital_port *intel_dig_port,
>>>>> u8 shim[DRM_HDCP_RI_LEN];
>>>>> } ri;
>>>>> bool repeater_present;
>>>>> + const struct intel_hdcp_shim *shim = connector->hdcp_shim;
>>>>> + struct intel_digital_port *intel_dig_port =
>>>>> conn_to_dig_port(connector);
>>>>> - dev_priv = intel_dig_port->base.base.dev->dev_private;
>>>>> -
>>>>> + dev_priv = to_i915(connector->base.dev);
>>>>> port = intel_dig_port->base.port;
>>>>> /* Initialize An with 2 random values and acquire it */
>>>>> @@ -498,19 +507,18 @@ static int intel_hdcp_auth(struct
>>>>> intel_digital_port *intel_dig_port,
>>>>> * on those as well.
>>>>> */
>>>>> - if (repeater_present)
>>>>> - return intel_hdcp_auth_downstream(intel_dig_port,
>>>>> shim);
>>>>> + if (repeater_present) {
>>>>> + ret = intel_hdcp_auth_downstream(connector);
>>>>> + if (ret) {
>>>>> + _intel_hdcp_disable(connector);
>>>>
>>>> If you just call this from _intel_hdcp_enable when intel_hdcp_auth()
>>>> fails, you
>>>> can avoid all of this code, it'd just be one line.
>>>
>>> Yes. Thought about that option too. Actually I would prefer having the
>>> reference of intel_connector untill generic hdcp auth implementation.
>>> we can pass the intel digital port for encoder specific shims.
>>>
>>> This will help me with
>>> providing the connector details for state transitions as in next
>>> patches
>>> next steps like SRM passing in for revocation and downstream
>>> topology
>>> gathering (perhaps will start with availability of complementing user
>>> space)
>>>
>>> At this point if we dont want this code, I will disable hdcp from enable
>>> functions.
>>
>> Yes please. Let's not try to predict the future and keep things as simple
>> as
>> possible.
>>
>> Sean
>
> but patch 3(adding connector info into hdcp state transition debug logs)
> still needs conenctor ref at auth.
> Do we still want to drop the connector ref passing? If so we will lose patch
> 3 too.
I was thinking about this further, and it doesn't really make sense to
change some of the messages and not all of them. So perhaps
middle-of-the-road solution would be to add a new DEBUG_KMS message at
the beginning of both _intel_hdcp_enable and _intel_hdcp_disable
listing the connector name/id and reporting that hdcp is being
enabled/disabled. That way anything that follows can be attributed to
that connector. This also avoids having to shuffle the arguments
around.
Sean
>
> -Ram
>
>
>>
>>> -Ram
>>>>
>>>>
>>>>> + return ret;
>>>>> + }
>>>>> + }
>>>>> DRM_DEBUG_KMS("HDCP is enabled (no repeater present)\n");
>>>>> return 0;
>>>>> }
>>>>> -static
>>>>> -struct intel_digital_port *conn_to_dig_port(struct intel_connector
>>>>> *connector)
>>>>> -{
>>>>> - return
>>>>> enc_to_dig_port(&intel_attached_encoder(&connector->base)->base);
>>>>> -}
>>>>> -
>>>>> static int _intel_hdcp_disable(struct intel_connector *connector)
>>>>> {
>>>>> struct drm_i915_private *dev_priv =
>>>>> connector->base.dev->dev_private;
>>>>> @@ -558,8 +566,7 @@ static int _intel_hdcp_enable(struct
>>>>> intel_connector *connector)
>>>>> return ret;
>>>>> }
>>>>> - ret = intel_hdcp_auth(conn_to_dig_port(connector),
>>>>> - connector->hdcp_shim);
>>>>> + ret = intel_hdcp_auth(connector);
>>>>> if (ret) {
>>>>> DRM_ERROR("Failed to authenticate HDCP (%d)\n", ret);
>>>>> return ret;
>>>>> --
>>>>> 2.7.4
>>>>>
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
