On 2020-05-21 at 10:27:21 +0530, Ramalingam C wrote:
On 2020-05-20 at 15:47:44 -0400, Sean Paul wrote:
From: Sean Paul seanpaul@chromium.org
If userspace sets the CP property to DESIRED while it's already ENABLED, the driver will try to re-enable HDCP. On some displays, this will result in R0' mismatches. I'm guessing this is because the display is still sending back Ri instead of re-authenticating.
At any rate, we can fix this inefficiency easily enough by just nooping the DESIRED property set if HDCP is already ENABLED.
AFAIU may below patch also solves above issue implicitly. https://patchwork.freedesktop.org/patch/365758/?series=72251&rev=4 Besides that +1 for below Ram comment, it would be better if such type of duplicate enable request should filter by drm_atomic_connector_set_property(). Thanks, Anshuman Gupta.
Sean,
This will skip the hdcp enable.
But at present too we will be getting below WARN_ON from intel_hdcp_enable, to indicate userspace is going wrong with request. drm_WARN_ON(&dev_priv->drm, hdcp->value == DRM_MODE_CONTENT_PROTECTION_ENABLED);
And if we need to filter this out, could we validate the incoming hdcp request at drm_atomic_connector_set_property() itself? No point in going into the atomic commit without a valid request. something like
diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c index a1e5e262bae2..d98b2eeae78d 100644 --- a/drivers/gpu/drm/drm_atomic_uapi.c +++ b/drivers/gpu/drm/drm_atomic_uapi.c @@ -746,6 +746,12 @@ static int drm_atomic_connector_set_property(struct drm_connector *connector, DRM_DEBUG_KMS("only drivers can set CP Enabled\n"); return -EINVAL; }
if (config->content_protection_property ==DRM_MODE_CONTENT_PROTECTION_ENABLED &&val == DRM_MODE_CONTENT_PROTECTION_DESIRED) {DRM_DEBUG_KMS("Redundant req for content protection\n");return -EINVAL;} state->content_protection = val; } else if (property == config->hdcp_content_type_property) { state->hdcp_content_type = val;-Ram
Signed-off-by: Sean Paul seanpaul@chromium.org
I suspect this is the actual root cause I was chasing with "drm/i915/hdcp: Add additional R0' wait". I was able to reproduce the R0` messages by marking HDCP desired while it was already enabled. This _should_ work, but it seems like some displays handle it more graciously than others.
drivers/gpu/drm/i915/display/intel_hdcp.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c b/drivers/gpu/drm/i915/display/intel_hdcp.c index 2cbc4619b4ce..f770fe0c5595 100644 --- a/drivers/gpu/drm/i915/display/intel_hdcp.c +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c @@ -2156,12 +2156,16 @@ void intel_hdcp_atomic_check(struct drm_connector *connector, }
/*
* Nothing to do if the state didn't change, or HDCP was activated since* the last commit. And also no change in hdcp content type.
* Nothing to do if content type is unchanged and one of:* - state didn't change* - HDCP was activated since the last commit* - attempting to set to desired while already enabled
new_cp == DRM_MODE_CONTENT_PROTECTION_ENABLED)) {
new_cp == DRM_MODE_CONTENT_PROTECTION_ENABLED) ||(old_cp == DRM_MODE_CONTENT_PROTECTION_ENABLED &&new_cp == DRM_MODE_CONTENT_PROTECTION_DESIRED)) {-- Sean Paul, Software Engineer, Google / Chromium OS