From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.8 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 525E7C2D0DB for ; Mon, 20 Jan 2020 11:24:13 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 2E086207E0 for ; Mon, 20 Jan 2020 11:24:13 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 2E086207E0 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=intel.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=intel-gfx-bounces@lists.freedesktop.org Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 8E8256E8DF; Mon, 20 Jan 2020 11:24:12 +0000 (UTC) Received: from mga06.intel.com (mga06.intel.com [134.134.136.31]) by gabe.freedesktop.org (Postfix) with ESMTPS id 37C2A6E8DF for ; Mon, 20 Jan 2020 11:24:11 +0000 (UTC) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga006.jf.intel.com ([10.7.209.51]) by orsmga104.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 20 Jan 2020 03:24:10 -0800 X-IronPort-AV: E=Sophos;i="5.70,341,1574150400"; d="scan'208";a="228356460" Received: from jnikula-mobl3.fi.intel.com (HELO localhost) ([10.237.66.161]) by orsmga006-auth.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 20 Jan 2020 03:24:08 -0800 From: Jani Nikula To: Ramalingam C In-Reply-To: <20200120110044.GB14839@intel.com> Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo References: <20200120054954.5786-1-anshuman.gupta@intel.com> <20200120064215.GA14839@intel.com> <87v9p6fmjz.fsf@intel.com> <20200120110044.GB14839@intel.com> Date: Mon, 20 Jan 2020 13:24:05 +0200 Message-ID: <87sgkafk16.fsf@intel.com> MIME-Version: 1.0 Subject: Re: [Intel-gfx] [PATCH v3] drm/i915/hdcp: Update CP as per the kernel internal state X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: intel-gfx@lists.freedesktop.org Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" On Mon, 20 Jan 2020, Ramalingam C wrote: > On 2020-01-20 at 12:29:36 +0200, Jani Nikula wrote: >> On Mon, 20 Jan 2020, Ramalingam C wrote: >> > On 2020-01-20 at 11:19:54 +0530, Anshuman Gupta wrote: >> >> Content Protection property should be updated as per the kernel >> >> internal state. Let's say if Content protection is disabled >> >> by userspace, CP property should be set to UNDESIRED so that >> >> reauthentication will not happen until userspace request it again, >> >> but when kernel disables the HDCP due to any DDI disabling sequences >> >> like modeset/DPMS operation, kernel should set the property to >> >> DESIRED, so that when opportunity arises, kernel will start the >> >> HDCP authentication on its own. >> >> >> >> Somewhere in the line, state machine to set content protection to >> >> DESIRED from kernel was broken and IGT coverage was missing for it. >> >> This patch fixes it. >> >> IGT patch to catch further regression on this features is being >> >> worked upon. >> >> >> >> v2: >> >> - Incorporated the necessary locking. (Ram) >> >> - Set content protection property to CP_DESIRED only when >> >> user has not asked explicitly to set CP_UNDESIRED. >> >> >> >> v3: >> >> - Reset the is_hdcp_undesired flag to false. (Ram) >> >> - Rephrasing commit log and small comment for is_hdcp_desired >> >> flag. (Ram) >> >> >> >> CC: Ramalingam C >> >> Reviewed-by: Ramalingam C >> >> Signed-off-by: Anshuman Gupta >> >> --- >> >> drivers/gpu/drm/i915/display/intel_display_types.h | 6 ++++++ >> >> drivers/gpu/drm/i915/display/intel_hdcp.c | 13 ++++++++++++- >> >> 2 files changed, 18 insertions(+), 1 deletion(-) >> >> >> >> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h >> >> index 630a94892b7b..401a9a7689fb 100644 >> >> --- a/drivers/gpu/drm/i915/display/intel_display_types.h >> >> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h >> >> @@ -345,6 +345,12 @@ struct intel_hdcp { >> >> struct delayed_work check_work; >> >> struct work_struct prop_work; >> >> >> >> + /* >> >> + * Flag to differentiate that HDCP is being disabled originated from >> >> + * userspace or triggered from kernel DDI disable sequence. >> >> + */ >> >> + bool is_hdcp_undesired; >> > Jani and Daniel, >> > >> > This flag is added as we need to know the origin of the HDCP disable >> > (userspace or kernel modeset/DPMS off) at DDI disable sequence. We >> > couldn't do that as new_conn state is not available there to retrieve >> > the corresponding content protection state. >> > >> > Hence we do that at atomic check itself and pass the info through this flag, >> > which will be referred at hdcp_disable. >> > >> > If you think we could do it better please suggest the preferred >> > alternate method. Else I request your ack for merging this. >> >> I don't know hdcp code all that well, but it seems to me at the root of >> the problem is the duplication of the content protection state >> (drm_connector_state->content_protection) into the connector >> (intel_hdcp->value), and them going out of sync. >> >> If you relied on the connector state alone, you wouldn't have to worry >> about changing the intel_hdcp->value member at disable or anywhere; >> disable looks at old state and disables based on that. No history/future >> information needed. Isn't that roughly what everything else does, why is >> hdcp special? > As per uAPI designed for Chrome(first user of HDCP), after the userspace > request for HDCP by setting DESIRED to "content protection" only userspace can DISABLED it. > > Incase when HDCP is ENABLED and kernel ended up in disabled the DDI (hot > unplug or DPMS ops), it supposed to disable HDCP encryption and leave the > "Content protection" at DESIRED. So that when the DDI is re enabled, > HDCP authentication will be attempted automatically, as userspace is > still interested in HDCP encryption. > > So to set the state of "content protection" as ENABLED->DESIRED, we > need to know the HDCP disable in DDI disable is not because of userspace request > (derived based on new connector state existance and its content protection state). > > Hence we need this change. Okay, why do you need to track desired/undesired in intel_hdcp then? Just track enabled/disabled, and do everything else based on connector state? BR, Jani. > > -Ram >> >> BR, >> Jani. >> >> >> > >> > Thanks, >> > -Ram >> >> + >> >> /* HDCP1.4 Encryption status */ >> >> bool hdcp_encrypted; >> >> >> >> diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c b/drivers/gpu/drm/i915/display/intel_hdcp.c >> >> index 0fdbd39f6641..7f631ebd8395 100644 >> >> --- a/drivers/gpu/drm/i915/display/intel_hdcp.c >> >> +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c >> >> @@ -2002,11 +2002,18 @@ int intel_hdcp_disable(struct intel_connector *connector) >> >> mutex_lock(&hdcp->mutex); >> >> >> >> if (hdcp->value != DRM_MODE_CONTENT_PROTECTION_UNDESIRED) { >> >> - hdcp->value = DRM_MODE_CONTENT_PROTECTION_UNDESIRED; >> >> if (hdcp->hdcp2_encrypted) >> >> ret = _intel_hdcp2_disable(connector); >> >> else if (hdcp->hdcp_encrypted) >> >> ret = _intel_hdcp_disable(connector); >> >> + >> >> + if (hdcp->is_hdcp_undesired) { >> >> + hdcp->value = DRM_MODE_CONTENT_PROTECTION_UNDESIRED; >> >> + hdcp->is_hdcp_undesired = false; >> >> + } else { >> >> + hdcp->value = DRM_MODE_CONTENT_PROTECTION_DESIRED; >> >> + schedule_work(&hdcp->prop_work); >> >> + } >> >> } >> >> >> >> mutex_unlock(&hdcp->mutex); >> >> @@ -2044,6 +2051,7 @@ void intel_hdcp_atomic_check(struct drm_connector *connector, >> >> { >> >> u64 old_cp = old_state->content_protection; >> >> u64 new_cp = new_state->content_protection; >> >> + struct intel_connector *intel_conn = to_intel_connector(connector); >> >> struct drm_crtc_state *crtc_state; >> >> >> >> if (!new_state->crtc) { >> >> @@ -2069,6 +2077,9 @@ void intel_hdcp_atomic_check(struct drm_connector *connector, >> >> return; >> >> } >> >> >> >> + if (new_cp == DRM_MODE_CONTENT_PROTECTION_UNDESIRED) >> >> + intel_conn->hdcp.is_hdcp_undesired = true; >> >> + >> >> crtc_state = drm_atomic_get_new_crtc_state(new_state->state, >> >> new_state->crtc); >> >> crtc_state->mode_changed = true; >> >> -- >> >> 2.24.0 >> >> >> >> -- >> Jani Nikula, Intel Open Source Graphics Center -- Jani Nikula, Intel Open Source Graphics Center _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx