Re: [Intel-gfx] [PATCH 5/8] drm/i915: Optimize HDCP key load

[Sean Paul]

On Fri, Feb 2, 2018 at 9:33 AM, Ramalingam C  wrote:
>
>
> On Friday 02 February 2018 07:48 PM, Sean Paul wrote:
>>
>> On Fri, Feb 02, 2018 at 04:15:17PM +0530, Ramalingam C wrote:
>>>
>>> HDCP key need not be cleared on each hdcp disable. And HDCP key Load
>>> is skipped if key is already loaded.
>>>
>> I had previously encountered issues without clearing the key in my
>> testing.
>> IIRC, without clearing the keys things acted differently. How much time
>> are we
>> saving by optimizing this?
>>
>> Sean
>
> Time profiling is not done. As per the Bspec, Once Keys are loaded, they
> will be cleared only when PG1/PG0 is off.
> So on Resume we need to load the keys. Since we have the status register for
> key load state, I feel we could rely on them.
>
> Now with our code state, I am not seeing the need for reloading the keys. I
> have tested on KBL.
> I think this is worth an attempt as no failures are observed.
>

Alright, sounds good to me, thanks for explaining!

Reviewed-by: Sean Paul 



> --Ram
>
>>
>>
>>> Signed-off-by: Ramalingam C 
>>> ---
>>>   drivers/gpu/drm/i915/intel_hdcp.c | 6 --
>>>   1 file changed, 4 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/intel_hdcp.c
>>> b/drivers/gpu/drm/i915/intel_hdcp.c
>>> index fa2e7c727d00..5de9afd275b2 100644
>>> --- a/drivers/gpu/drm/i915/intel_hdcp.c
>>> +++ b/drivers/gpu/drm/i915/intel_hdcp.c
>>> @@ -51,6 +51,10 @@ static int intel_hdcp_load_keys(struct
>>> drm_i915_private *dev_priv)
>>> int ret;
>>> u32 val;
>>>   + val = I915_READ(HDCP_KEY_STATUS);
>>> +   if ((val & HDCP_KEY_LOAD_DONE) && (val & HDCP_KEY_LOAD_STATUS))
>>> +   return 0;
>>> +
>>> /*
>>>  * On HSW and BDW HW loads the HDCP1.4 Key when Display comes
>>>  * out of reset. So if Key is not already loaded, its an error
>>> state.
>>> @@ -542,8 +546,6 @@ static int _intel_hdcp_disable(struct intel_connector
>>> *connector)
>>> return -ETIMEDOUT;
>>> }
>>>   - intel_hdcp_clear_keys(dev_priv);
>>> -
>>> ret = connector->hdcp_shim->toggle_signalling(intel_dig_port,
>>> false);
>>> if (ret) {
>>> DRM_ERROR("Failed to disable HDCP signalling\n");
>>> --
>>> 2.7.4
>>>
>
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 5/8] drm/i915: Optimize HDCP key load

[Ramalingam C]

On Friday 02 February 2018 07:48 PM, Sean Paul wrote:

On Fri, Feb 02, 2018 at 04:15:17PM +0530, Ramalingam C wrote:

HDCP key need not be cleared on each hdcp disable. And HDCP key Load
is skipped if key is already loaded.


I had previously encountered issues without clearing the key in my testing.
IIRC, without clearing the keys things acted differently. How much time are we
saving by optimizing this?

Sean
Time profiling is not done. As per the Bspec, Once Keys are loaded, they 
will be cleared only when PG1/PG0 is off.
So on Resume we need to load the keys. Since we have the status register 
for key load state, I feel we could rely on them.


Now with our code state, I am not seeing the need for reloading the 
keys. I have tested on KBL.

I think this is worth an attempt as no failures are observed.

--Ram




Signed-off-by: Ramalingam C 
---
  drivers/gpu/drm/i915/intel_hdcp.c | 6 --
  1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_hdcp.c 
b/drivers/gpu/drm/i915/intel_hdcp.c
index fa2e7c727d00..5de9afd275b2 100644
--- a/drivers/gpu/drm/i915/intel_hdcp.c
+++ b/drivers/gpu/drm/i915/intel_hdcp.c
@@ -51,6 +51,10 @@ static int intel_hdcp_load_keys(struct drm_i915_private 
*dev_priv)
int ret;
u32 val;
  
+	val = I915_READ(HDCP_KEY_STATUS);

+   if ((val & HDCP_KEY_LOAD_DONE) && (val & HDCP_KEY_LOAD_STATUS))
+   return 0;
+
/*
 * On HSW and BDW HW loads the HDCP1.4 Key when Display comes
 * out of reset. So if Key is not already loaded, its an error state.
@@ -542,8 +546,6 @@ static int _intel_hdcp_disable(struct intel_connector 
*connector)
return -ETIMEDOUT;
}
  
-	intel_hdcp_clear_keys(dev_priv);

-
ret = connector->hdcp_shim->toggle_signalling(intel_dig_port, false);
if (ret) {
DRM_ERROR("Failed to disable HDCP signalling\n");
--
2.7.4



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

Re: [Intel-gfx] [PATCH 5/8] drm/i915: Optimize HDCP key load

[Sean Paul]

On Fri, Feb 02, 2018 at 04:15:17PM +0530, Ramalingam C wrote:
> HDCP key need not be cleared on each hdcp disable. And HDCP key Load
> is skipped if key is already loaded.
> 

I had previously encountered issues without clearing the key in my testing.
IIRC, without clearing the keys things acted differently. How much time are we
saving by optimizing this?

Sean


> Signed-off-by: Ramalingam C 
> ---
>  drivers/gpu/drm/i915/intel_hdcp.c | 6 --
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_hdcp.c 
> b/drivers/gpu/drm/i915/intel_hdcp.c
> index fa2e7c727d00..5de9afd275b2 100644
> --- a/drivers/gpu/drm/i915/intel_hdcp.c
> +++ b/drivers/gpu/drm/i915/intel_hdcp.c
> @@ -51,6 +51,10 @@ static int intel_hdcp_load_keys(struct drm_i915_private 
> *dev_priv)
>   int ret;
>   u32 val;
>  
> + val = I915_READ(HDCP_KEY_STATUS);
> + if ((val & HDCP_KEY_LOAD_DONE) && (val & HDCP_KEY_LOAD_STATUS))
> + return 0;
> +
>   /*
>* On HSW and BDW HW loads the HDCP1.4 Key when Display comes
>* out of reset. So if Key is not already loaded, its an error state.
> @@ -542,8 +546,6 @@ static int _intel_hdcp_disable(struct intel_connector 
> *connector)
>   return -ETIMEDOUT;
>   }
>  
> - intel_hdcp_clear_keys(dev_priv);
> -
>   ret = connector->hdcp_shim->toggle_signalling(intel_dig_port, false);
>   if (ret) {
>   DRM_ERROR("Failed to disable HDCP signalling\n");
> -- 
> 2.7.4
> 

-- 
Sean Paul, Software Engineer, Google / Chromium OS
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 5/8] drm/i915: Optimize HDCP key load

[Ramalingam C]

HDCP key need not be cleared on each hdcp disable. And HDCP key Load
is skipped if key is already loaded.

Signed-off-by: Ramalingam C 
---
 drivers/gpu/drm/i915/intel_hdcp.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_hdcp.c 
b/drivers/gpu/drm/i915/intel_hdcp.c
index fa2e7c727d00..5de9afd275b2 100644
--- a/drivers/gpu/drm/i915/intel_hdcp.c
+++ b/drivers/gpu/drm/i915/intel_hdcp.c
@@ -51,6 +51,10 @@ static int intel_hdcp_load_keys(struct drm_i915_private 
*dev_priv)
int ret;
u32 val;
 
+   val = I915_READ(HDCP_KEY_STATUS);
+   if ((val & HDCP_KEY_LOAD_DONE) && (val & HDCP_KEY_LOAD_STATUS))
+   return 0;
+
/*
 * On HSW and BDW HW loads the HDCP1.4 Key when Display comes
 * out of reset. So if Key is not already loaded, its an error state.
@@ -542,8 +546,6 @@ static int _intel_hdcp_disable(struct intel_connector 
*connector)
return -ETIMEDOUT;
}
 
-   intel_hdcp_clear_keys(dev_priv);
-
ret = connector->hdcp_shim->toggle_signalling(intel_dig_port, false);
if (ret) {
DRM_ERROR("Failed to disable HDCP signalling\n");
-- 
2.7.4

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