| Message ID | 1517568320-15579-6-git-send-email-ramalingam.c@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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 <ramalingam.c@intel.com> > --- > 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 >
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 <ramalingam.c@intel.com> >> --- >> 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 >>
On Fri, Feb 2, 2018 at 9:33 AM, Ramalingam C <ramalingam.c@intel.com> 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 <seanpaul@chromium.org> > --Ram > >> >> >>> Signed-off-by: Ramalingam C <ramalingam.c@intel.com> >>> --- >>> 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 >>> >
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");
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 <ramalingam.c@intel.com> --- drivers/gpu/drm/i915/intel_hdcp.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)