<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
</head>
<body text="#000000" bgcolor="#FFFFFF">
<pre>So basically we want all logs related to HW failure and I915 programing should be errors whereas all sink specific errors in debug level.
I will prepare the change.</pre>
<pre>danvet, seanpaul has discussed few points about convenience of debugging on customer platforms. What is the take on it?</pre>
<p>--Ram<br>
</p>
<div class="moz-cite-prefix">On 10/26/2018 5:09 PM, Daniel Vetter
wrote:<br>
</div>
<blockquote type="cite"
cite="mid:20181026113920.GL21967@phenom.ffwll.local">
<pre class="moz-quote-pre" wrap="">On Thu, Oct 25, 2018 at 11:47:52PM +0530, Ramalingam C wrote:
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">Conceptually user should be knowing the feature status through uAPI.
So HDCP authentication failure information need not DRM_ERRORS.
They are needed only for ENG debugging.
And also in HDCP we tolerate the retries for HDCP authentication.
Hence if we print the failure info initial attempts as ERRORs CI will
fail the IGT.
So we present the information of the failures in form of DRM_DEBUG
msgs for debugging purpose.
Signed-off-by: Ramalingam C <a class="moz-txt-link-rfc2396E" href="mailto:ramalingam.c@intel.com"><ramalingam.c@intel.com></a>
---
drivers/gpu/drm/i915/intel_hdcp.c | 46 +++++++++++++++++++--------------------
1 file changed, 23 insertions(+), 23 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_hdcp.c b/drivers/gpu/drm/i915/intel_hdcp.c
index 5b423a78518d..d92b04c3ed4e 100644
--- a/drivers/gpu/drm/i915/intel_hdcp.c
+++ b/drivers/gpu/drm/i915/intel_hdcp.c
@@ -166,8 +166,8 @@ static int intel_hdcp_load_keys(struct drm_i915_private *dev_priv)
SKL_PCODE_LOAD_HDCP_KEYS, 1);
mutex_unlock(&dev_priv->pcu_lock);
if (ret) {
- DRM_ERROR("Failed to initiate HDCP key load (%d)\n",
- ret);
+ DRM_DEBUG_KMS("Failed to initiate HDCP key load (%d)\n",
+ ret);
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
This one indicates an i915 hw failure, not anything wrong with the sink.
So should stay at DRM_ERROR.
Btw I noticed that 2 DRM_DEBUG_KMS in intel_hdcp_validate_v_prime also
indicate programming/i915-side hw errors. So should be upgraded to
DRM_ERROR I think (but in a separate patch).
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap=""> return ret;
}
} else {
@@ -195,7 +195,7 @@ static int intel_write_sha_text(struct drm_i915_private *dev_priv, u32 sha_text)
I915_WRITE(HDCP_SHA_TEXT, sha_text);
if (intel_wait_for_register(dev_priv, HDCP_REP_CTL,
HDCP_SHA1_READY, HDCP_SHA1_READY, 1)) {
- DRM_ERROR("Timed out waiting for SHA1 ready\n");
+ DRM_DEBUG_KMS("Timed out waiting for SHA1 ready\n");
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
Same here.
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap=""> return -ETIMEDOUT;
}
return 0;
@@ -219,7 +219,7 @@ u32 intel_hdcp_get_repeater_ctl(struct intel_digital_port *intel_dig_port)
default:
break;
}
- DRM_ERROR("Unknown port %d\n", port);
+ DRM_DEBUG_KMS("Unknown port %d\n", port);
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
Also a programming error.
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap=""> return -EINVAL;
}
@@ -448,7 +448,7 @@ int intel_hdcp_auth_downstream(struct intel_digital_port *intel_dig_port,
ret = intel_hdcp_poll_ksv_fifo(intel_dig_port, shim);
if (ret) {
- DRM_ERROR("KSV list failed to become ready (%d)\n", ret);
+ DRM_DEBUG_KMS("KSV list failed to become ready (%d)\n", ret);
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
This one is a sink issue, so correct to change to DRM_DEBUG_KMS. If I
don't comment, assume that I agree with your change.
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap=""> return ret;
}
@@ -458,7 +458,7 @@ int intel_hdcp_auth_downstream(struct intel_digital_port *intel_dig_port,
if (DRM_HDCP_MAX_DEVICE_EXCEEDED(bstatus[0]) ||
DRM_HDCP_MAX_CASCADE_EXCEEDED(bstatus[1])) {
- DRM_ERROR("Max Topology Limit Exceeded\n");
+ DRM_DEBUG_KMS("Max Topology Limit Exceeded\n");
return -EPERM;
}
@@ -494,7 +494,7 @@ int intel_hdcp_auth_downstream(struct intel_digital_port *intel_dig_port,
}
if (i == tries) {
- DRM_ERROR("V Prime validation failed.(%d)\n", ret);
+ DRM_DEBUG_KMS("V Prime validation failed.(%d)\n", ret);
goto err;
}
@@ -543,7 +543,7 @@ static int intel_hdcp_auth(struct intel_digital_port *intel_dig_port,
if (ret)
return ret;
if (!hdcp_capable) {
- DRM_ERROR("Panel is not HDCP capable\n");
+ DRM_DEBUG_KMS("Panel is not HDCP capable\n");
return -EINVAL;
}
}
@@ -557,7 +557,7 @@ static int intel_hdcp_auth(struct intel_digital_port *intel_dig_port,
if (intel_wait_for_register(dev_priv, PORT_HDCP_STATUS(port),
HDCP_STATUS_AN_READY,
HDCP_STATUS_AN_READY, 1)) {
- DRM_ERROR("Timed out waiting for An\n");
+ DRM_DEBUG_KMS("Timed out waiting for An\n");
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
Again this would indicate an i915 hw issue, so better to keep it at
DRM_ERROR.
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap=""> return -ETIMEDOUT;
}
@@ -594,7 +594,7 @@ static int intel_hdcp_auth(struct intel_digital_port *intel_dig_port,
/* Wait for R0 ready */
if (wait_for(I915_READ(PORT_HDCP_STATUS(port)) &
(HDCP_STATUS_R0_READY | HDCP_STATUS_ENC), 1)) {
- DRM_ERROR("Timed out waiting for R0 ready\n");
+ DRM_DEBUG_KMS("Timed out waiting for R0 ready\n");
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
Same, all the wait_for and wait_for_register indicate i915-side hw issues,
and should stay at DRM_ERROR. Otherwise we just reduce test coverage.
Please leave all of them as DRM_ERROR.
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap=""> return -ETIMEDOUT;
}
@@ -629,15 +629,15 @@ static int intel_hdcp_auth(struct intel_digital_port *intel_dig_port,
}
if (i == tries) {
- DRM_ERROR("Timed out waiting for Ri prime match (%x)\n",
- I915_READ(PORT_HDCP_STATUS(port)));
+ DRM_DEBUG_KMS("Timed out waiting for Ri prime match (%x)\n",
+ I915_READ(PORT_HDCP_STATUS(port)));
return -ETIMEDOUT;
}
/* Wait for encryption confirmation */
if (intel_wait_for_register(dev_priv, PORT_HDCP_STATUS(port),
HDCP_STATUS_ENC, HDCP_STATUS_ENC, 20)) {
- DRM_ERROR("Timed out waiting for encryption\n");
+ DRM_DEBUG_KMS("Timed out waiting for encryption\n");
return -ETIMEDOUT;
}
@@ -666,13 +666,13 @@ static int _intel_hdcp_disable(struct intel_connector *connector)
I915_WRITE(PORT_HDCP_CONF(port), 0);
if (intel_wait_for_register(dev_priv, PORT_HDCP_STATUS(port), ~0, 0,
20)) {
- DRM_ERROR("Failed to disable HDCP, timeout clearing status\n");
+ DRM_DEBUG_KMS("Failed to disable HDCP, timeout.\n");
return -ETIMEDOUT;
}
ret = connector->hdcp_shim->toggle_signalling(intel_dig_port, false);
if (ret) {
- DRM_ERROR("Failed to disable HDCP signalling\n");
+ DRM_DEBUG_KMS("Failed to disable HDCP signalling\n");
return ret;
}
@@ -689,7 +689,7 @@ static int _intel_hdcp_enable(struct intel_connector *connector)
connector->base.name, connector->base.base.id);
if (!hdcp_key_loadable(dev_priv)) {
- DRM_ERROR("HDCP key Load is not possible\n");
+ DRM_DEBUG_KMS("HDCP key Load is not possible\n");
return -ENXIO;
}
@@ -700,7 +700,7 @@ static int _intel_hdcp_enable(struct intel_connector *connector)
intel_hdcp_clear_keys(dev_priv);
}
if (ret) {
- DRM_ERROR("Could not load HDCP keys, (%d)\n", ret);
+ DRM_DEBUG_KMS("Could not load HDCP keys, (%d)\n", ret);
return ret;
}
@@ -717,7 +717,7 @@ static int _intel_hdcp_enable(struct intel_connector *connector)
_intel_hdcp_disable(connector);
}
- DRM_ERROR("HDCP authentication failed (%d tries/%d)\n", tries, ret);
+ DRM_DEBUG_KMS("HDCP authentication failed (%d tries/%d)\n", tries, ret);
return ret;
}
@@ -871,9 +871,9 @@ int intel_hdcp_check_link(struct intel_connector *connector)
goto out;
if (!(I915_READ(PORT_HDCP_STATUS(port)) & HDCP_STATUS_ENC)) {
- DRM_ERROR("%s:%d HDCP check failed: link is not encrypted,%x\n",
- connector->base.name, connector->base.base.id,
- I915_READ(PORT_HDCP_STATUS(port)));
+ DRM_DEBUG_KMS("%s:%d HDCP Link is not encrypted,%x\n",
+ connector->base.name, connector->base.base.id,
+ I915_READ(PORT_HDCP_STATUS(port)));
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
Again I think this would indicate an i915 hw issue, let's leave at
DRM_ERROR.
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap=""> ret = -ENXIO;
connector->hdcp_value = DRM_MODE_CONTENT_PROTECTION_DESIRED;
schedule_work(&connector->hdcp_prop_work);
@@ -895,7 +895,7 @@ int intel_hdcp_check_link(struct intel_connector *connector)
ret = _intel_hdcp_disable(connector);
if (ret) {
- DRM_ERROR("Failed to disable hdcp (%d)\n", ret);
+ DRM_DEBUG_KMS("Failed to disable hdcp (%d)\n", ret);
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
Disabling hdcp should always succeeds, again let's leave at DRM_ERROR.
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap=""> connector->hdcp_value = DRM_MODE_CONTENT_PROTECTION_DESIRED;
schedule_work(&connector->hdcp_prop_work);
goto out;
@@ -903,7 +903,7 @@ int intel_hdcp_check_link(struct intel_connector *connector)
ret = _intel_hdcp_enable(connector);
if (ret) {
- DRM_ERROR("Failed to enable hdcp (%d)\n", ret);
+ DRM_DEBUG_KMS("Failed to enable hdcp (%d)\n", ret);
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
Yeah, this can fail when the sink is gone.
When you respin this, can you pls include the added patch to make the 2
debug outputs into DRM_ERROR, and the 4 patches you've identified we can
merge already? I want to make sure CI is still approving of all of them,
now that the kms_content_protection testcase has landed.
Thanks, Daniel
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap=""> connector->hdcp_value = DRM_MODE_CONTENT_PROTECTION_DESIRED;
schedule_work(&connector->hdcp_prop_work);
goto out;
--
2.7.4
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
</pre>
</blockquote>
</body>
</html>