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=-8.2 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS, USER_AGENT_SANE_1 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 ADF4CC2D0DB for ; Mon, 20 Jan 2020 11:51:43 +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 8A983207FF for ; Mon, 20 Jan 2020 11:51:43 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 8A983207FF 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 235896E90E; Mon, 20 Jan 2020 11:51:43 +0000 (UTC) Received: from mga06.intel.com (mga06.intel.com [134.134.136.31]) by gabe.freedesktop.org (Postfix) with ESMTPS id 866D36E914 for ; Mon, 20 Jan 2020 11:51:41 +0000 (UTC) X-Amp-Result: UNKNOWN X-Amp-Original-Verdict: FILE UNKNOWN X-Amp-File-Uploaded: False Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by orsmga104.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 20 Jan 2020 03:51:40 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.70,341,1574150400"; d="scan'208";a="258681860" Received: from ramaling-i9x.iind.intel.com (HELO intel.com) ([10.99.66.154]) by fmsmga002.fm.intel.com with ESMTP; 20 Jan 2020 03:51:38 -0800 Date: Mon, 20 Jan 2020 17:21:34 +0530 From: Ramalingam C To: Jani Nikula Message-ID: <20200120115134.GB15991@intel.com> References: <20200120054954.5786-1-anshuman.gupta@intel.com> <20200120064215.GA14839@intel.com> <87v9p6fmjz.fsf@intel.com> <20200120110044.GB14839@intel.com> <87sgkafk16.fsf@intel.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <87sgkafk16.fsf@intel.com> User-Agent: Mutt/1.10.1 (2018-07-13) 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 2020-01-20 at 13:24:05 +0200, Jani Nikula wrote: > 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? Jani, hdcp->value is used to track the internal transistions during the link failure and re-establishing it. When ever kernel want to update the "content protection" we take the required locks and update the property state along with uevent. -Ram > > 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