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, URIBL_BLOCKED,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 54817C35249 for ; Wed, 5 Feb 2020 05:16:04 +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 2F728217BA for ; Wed, 5 Feb 2020 05:16:04 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 2F728217BA 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 9DC336E175; Wed, 5 Feb 2020 05:16:03 +0000 (UTC) Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by gabe.freedesktop.org (Postfix) with ESMTPS id 6F6906E175 for ; Wed, 5 Feb 2020 05:16:02 +0000 (UTC) X-Amp-Result: UNSCANNABLE X-Amp-File-Uploaded: False Received: from orsmga002.jf.intel.com ([10.7.209.21]) by orsmga102.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 04 Feb 2020 21:16:00 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.70,404,1574150400"; d="scan'208";a="249590617" Received: from unknown (HELO intel.com) ([10.223.74.178]) by orsmga002.jf.intel.com with ESMTP; 04 Feb 2020 21:15:59 -0800 Date: Wed, 5 Feb 2020 10:37:14 +0530 From: Anshuman Gupta To: Jani Nikula Message-ID: <20200205050713.GH24118@intel.com> References: <20200128135425.14596-1-anshuman.gupta@intel.com> <20200128135425.14596-2-anshuman.gupta@intel.com> <87d0b3d5os.fsf@intel.com> <20200128154444.GE24118@intel.com> <20200128161545.GF24118@intel.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20200128161545.GF24118@intel.com> User-Agent: Mutt/1.5.24 (2015-08-30) Subject: Re: [Intel-gfx] [PATCH 1/4] 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-28 at 21:45:45 +0530, Anshuman Gupta wrote: Hi Jani , As per my understanding intel_hdcp_atomic_check() is not sufficient to fix the broken hdcp uapi state, as the state fixup required in case of modeset. If you do not have any concern, can we continue with the patch. Thanks, Anshuman Gupta. > On 2020-01-28 at 21:14:44 +0530, Anshuman Gupta wrote: > > On 2020-01-28 at 16:19:31 +0200, Jani Nikula wrote: > > > On Tue, 28 Jan 2020, 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. > > > > > > > > CC: Ramalingam C > > > > Signed-off-by: Anshuman Gupta > > > > --- > > > > drivers/gpu/drm/i915/display/intel_display.c | 4 +++ > > > > drivers/gpu/drm/i915/display/intel_hdcp.c | 26 ++++++++++++++++++++ > > > > drivers/gpu/drm/i915/display/intel_hdcp.h | 2 ++ > > > > 3 files changed, 32 insertions(+) > > > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c > > > > index da5266e76738..934cdf1f1858 100644 > > > > --- a/drivers/gpu/drm/i915/display/intel_display.c > > > > +++ b/drivers/gpu/drm/i915/display/intel_display.c > > > > @@ -14595,6 +14595,10 @@ static int intel_atomic_check(struct drm_device *dev, > > > > goto fail; > > > > > > > > if (any_ms) { > > > > + /* > > > > + * When there is modeset fix the hdcp uapi CP state. > > > > + */ > > > > + intel_hdcp_post_need_modeset_check(state); > > > > ret = intel_modeset_checks(state); > > > > if (ret) > > > > goto fail; > > > > diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c b/drivers/gpu/drm/i915/display/intel_hdcp.c > > > > index 0fdbd39f6641..be083136eee2 100644 > > > > --- a/drivers/gpu/drm/i915/display/intel_hdcp.c > > > > +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c > > > > @@ -2074,6 +2074,32 @@ void intel_hdcp_atomic_check(struct drm_connector *connector, > > > > crtc_state->mode_changed = true; > > > > } > > > > > > > > +/** > > > > + * intel_hdcp_post_need_modeset_check. > > > > + * @state: intel atomic state. > > > > + * > > > > + * This function fix the HDCP uapi state when hdcp disabling initiated from > > > > + * modeset DDI disabling sequence. It updates uapi CP state from ENABLED to > > > > + * DESIRED so that HDCP uapi state can be restored as per HDCP Auth state. > > > > + * This function should be called only in case of in case of modeset. > > > > + * FIXME: As per HDCP content protection property uapi doc, an uevent() > > > > + * need to be sent if there is transition from ENABLED->DESIRED. > > > > + */ > > > > +void intel_hdcp_post_need_modeset_check(struct intel_atomic_state *state) > > > > +{ > > > > + struct drm_connector *connector; > > > > + struct drm_connector_state *old_state; > > > > + struct drm_connector_state *new_state; > > > > + int i; > > > > + > > > > + for_each_oldnew_connector_in_state(&state->base, connector, old_state, > > > > + new_state, i) { > > > > + if (old_state->content_protection == DRM_MODE_CONTENT_PROTECTION_ENABLED && > > > > + new_state->content_protection != DRM_MODE_CONTENT_PROTECTION_UNDESIRED) > > > > + new_state->content_protection = DRM_MODE_CONTENT_PROTECTION_DESIRED; > > > > + } > > > > +} > > > > + > > > > > > Why does this feel like duplication of what you already have in > > > intel_hdcp_atomic_check()? > > intel_hdcp_atomic_check() have checks that for disconnected connector and it doesn't look for > typo here, "intel_hdcp_atomic_check() checks that for disconnected connector and it doesn't check for new state shouldn't be UNDESIRED" > > old state, that is not sufficient to fix the hdcp CP uapi state, it need to be fix only in case of > > modeset, Later on a fastset check can disable the modeset and we would endup calling intel_hdcp_enable > > while hdcp is already enabled. That is the reason i think we would require a new API to > > fix the uapi state. > > Thanks , > > Anshuman Gupta. > > > > > > BR, > > > Jani. > > > > > > > > > > /* Handles the CP_IRQ raised from the DP HDCP sink */ > > > > void intel_hdcp_handle_cp_irq(struct intel_connector *connector) > > > > { > > > > diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.h b/drivers/gpu/drm/i915/display/intel_hdcp.h > > > > index f3c3272e712a..7bf46bc3c348 100644 > > > > --- a/drivers/gpu/drm/i915/display/intel_hdcp.h > > > > +++ b/drivers/gpu/drm/i915/display/intel_hdcp.h > > > > @@ -13,6 +13,7 @@ > > > > struct drm_connector; > > > > struct drm_connector_state; > > > > struct drm_i915_private; > > > > +struct intel_atomic_state; > > > > struct intel_connector; > > > > struct intel_hdcp_shim; > > > > enum port; > > > > @@ -21,6 +22,7 @@ enum transcoder; > > > > void intel_hdcp_atomic_check(struct drm_connector *connector, > > > > struct drm_connector_state *old_state, > > > > struct drm_connector_state *new_state); > > > > +void intel_hdcp_post_need_modeset_check(struct intel_atomic_state *state); > > > > int intel_hdcp_init(struct intel_connector *connector, > > > > const struct intel_hdcp_shim *hdcp_shim); > > > > int intel_hdcp_enable(struct intel_connector *connector, > > > > > > -- > > > Jani Nikula, Intel Open Source Graphics Center _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx