All of lore.kernel.org
 help / color / mirror / Atom feed
From: Rodrigo Vivi <rodrigo.vivi@intel.com>
To: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Huang@intel.com, "Huang, Sean Z" <sean.z.huang@intel.com>,
	dri-devel@lists.freedesktop.org,
	Chris Wilson <chris@chris-wilson.co.uk>
Subject: Re: [PATCH v5 12/15] drm/i915/pxp: Enable PXP power management
Date: Fri, 16 Jul 2021 07:42:22 -0400	[thread overview]
Message-ID: <YPFwni9CN35vFqio@intel.com> (raw)
In-Reply-To: <20210716041034.382-13-daniele.ceraolospurio@intel.com>

On Thu, Jul 15, 2021 at 09:10:31PM -0700, Daniele Ceraolo Spurio wrote:
> From: "Huang, Sean Z" <sean.z.huang@intel.com>
> 
> During the power event S3+ sleep/resume, hardware will lose all the
> encryption keys for every hardware session, even though the
> session state might still be marked as alive after resume. Therefore,
> we should consider the session as dead on suspend and invalidate all the
> objects. The session will be automatically restarted on the first
> protected submission on resume.
> 
> v2: runtime suspend also invalidates the keys
> v3: fix return codes, simplify rpm ops (Chris), use the new worker func
> v4: invalidate the objects on suspend, don't re-create the arb sesson on
> resume (delayed to first submission).
> v5: move irq changes back to irq patch (Rodrigo)


Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

> 
> Signed-off-by: Huang, Sean Z <sean.z.huang@intel.com>
> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
>  drivers/gpu/drm/i915/Makefile                |  1 +
>  drivers/gpu/drm/i915/gt/intel_gt_pm.c        | 15 +++++++-
>  drivers/gpu/drm/i915/i915_drv.c              |  2 +
>  drivers/gpu/drm/i915/pxp/intel_pxp_irq.c     |  1 +
>  drivers/gpu/drm/i915/pxp/intel_pxp_pm.c      | 40 ++++++++++++++++++++
>  drivers/gpu/drm/i915/pxp/intel_pxp_pm.h      | 23 +++++++++++
>  drivers/gpu/drm/i915/pxp/intel_pxp_session.c | 38 ++++++++++++++-----
>  drivers/gpu/drm/i915/pxp/intel_pxp_tee.c     |  9 +++++
>  8 files changed, 118 insertions(+), 11 deletions(-)
>  create mode 100644 drivers/gpu/drm/i915/pxp/intel_pxp_pm.c
>  create mode 100644 drivers/gpu/drm/i915/pxp/intel_pxp_pm.h
> 
> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> index 1714089a10f0..22575cebc0dc 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -280,6 +280,7 @@ i915-$(CONFIG_DRM_I915_PXP) += \
>  	pxp/intel_pxp.o \
>  	pxp/intel_pxp_cmd.o \
>  	pxp/intel_pxp_irq.o \
> +	pxp/intel_pxp_pm.o \
>  	pxp/intel_pxp_session.o \
>  	pxp/intel_pxp_tee.o
>  
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_pm.c b/drivers/gpu/drm/i915/gt/intel_gt_pm.c
> index aef3084e8b16..91151a02f7a2 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt_pm.c
> +++ b/drivers/gpu/drm/i915/gt/intel_gt_pm.c
> @@ -19,6 +19,7 @@
>  #include "intel_rc6.h"
>  #include "intel_rps.h"
>  #include "intel_wakeref.h"
> +#include "pxp/intel_pxp_pm.h"
>  
>  static void user_forcewake(struct intel_gt *gt, bool suspend)
>  {
> @@ -265,6 +266,8 @@ int intel_gt_resume(struct intel_gt *gt)
>  
>  	intel_uc_resume(&gt->uc);
>  
> +	intel_pxp_resume(&gt->pxp);
> +
>  	user_forcewake(gt, false);
>  
>  out_fw:
> @@ -299,6 +302,7 @@ void intel_gt_suspend_prepare(struct intel_gt *gt)
>  	user_forcewake(gt, true);
>  	wait_for_suspend(gt);
>  
> +	intel_pxp_suspend(&gt->pxp);
>  	intel_uc_suspend(&gt->uc);
>  }
>  
> @@ -349,6 +353,7 @@ void intel_gt_suspend_late(struct intel_gt *gt)
>  
>  void intel_gt_runtime_suspend(struct intel_gt *gt)
>  {
> +	intel_pxp_suspend(&gt->pxp);
>  	intel_uc_runtime_suspend(&gt->uc);
>  
>  	GT_TRACE(gt, "\n");
> @@ -356,11 +361,19 @@ void intel_gt_runtime_suspend(struct intel_gt *gt)
>  
>  int intel_gt_runtime_resume(struct intel_gt *gt)
>  {
> +	int ret;
> +
>  	GT_TRACE(gt, "\n");
>  	intel_gt_init_swizzling(gt);
>  	intel_ggtt_restore_fences(gt->ggtt);
>  
> -	return intel_uc_runtime_resume(&gt->uc);
> +	ret = intel_uc_runtime_resume(&gt->uc);
> +	if (ret)
> +		return ret;
> +
> +	intel_pxp_resume(&gt->pxp);
> +
> +	return 0;
>  }
>  
>  static ktime_t __intel_gt_get_awake_time(const struct intel_gt *gt)
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 30d8cd8c69b1..57903e15ba3b 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -67,6 +67,8 @@
>  #include "gt/intel_gt_pm.h"
>  #include "gt/intel_rc6.h"
>  
> +#include "pxp/intel_pxp_pm.h"
> +
>  #include "i915_debugfs.h"
>  #include "i915_drv.h"
>  #include "i915_ioc32.h"
> diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_irq.c b/drivers/gpu/drm/i915/pxp/intel_pxp_irq.c
> index 340f20d130a8..9e5847c653f2 100644
> --- a/drivers/gpu/drm/i915/pxp/intel_pxp_irq.c
> +++ b/drivers/gpu/drm/i915/pxp/intel_pxp_irq.c
> @@ -9,6 +9,7 @@
>  #include "gt/intel_gt_irq.h"
>  #include "i915_irq.h"
>  #include "i915_reg.h"
> +#include "intel_runtime_pm.h"
>  
>  /**
>   * intel_pxp_irq_handler - Handles PXP interrupts.
> diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_pm.c b/drivers/gpu/drm/i915/pxp/intel_pxp_pm.c
> new file mode 100644
> index 000000000000..4507756b2977
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/pxp/intel_pxp_pm.c
> @@ -0,0 +1,40 @@
> +// SPDX-License-Identifier: MIT
> +/*
> + * Copyright(c) 2020 Intel Corporation.
> + */
> +
> +#include "intel_pxp.h"
> +#include "intel_pxp_irq.h"
> +#include "intel_pxp_pm.h"
> +#include "intel_pxp_session.h"
> +
> +void intel_pxp_suspend(struct intel_pxp *pxp)
> +{
> +	if (!intel_pxp_is_enabled(pxp))
> +		return;
> +
> +	pxp->arb_is_valid = false;
> +
> +	/* invalidate protected objects */
> +	intel_pxp_invalidate(pxp);
> +
> +	intel_pxp_fini_hw(pxp);
> +
> +	pxp->hw_state_invalidated = false;
> +}
> +
> +void intel_pxp_resume(struct intel_pxp *pxp)
> +{
> +	if (!intel_pxp_is_enabled(pxp))
> +		return;
> +
> +	/*
> +	 * The PXP component gets automatically unbound when we go into S3 and
> +	 * re-bound after we come out, so in that scenario we can defer the
> +	 * hw init to the bind call.
> +	 */
> +	if (!pxp->pxp_component)
> +		return;
> +
> +	intel_pxp_init_hw(pxp);
> +}
> diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_pm.h b/drivers/gpu/drm/i915/pxp/intel_pxp_pm.h
> new file mode 100644
> index 000000000000..6f488789db6a
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/pxp/intel_pxp_pm.h
> @@ -0,0 +1,23 @@
> +/* SPDX-License-Identifier: MIT */
> +/*
> + * Copyright(c) 2020, Intel Corporation. All rights reserved.
> + */
> +
> +#ifndef __INTEL_PXP_PM_H__
> +#define __INTEL_PXP_PM_H__
> +
> +#include "i915_drv.h"
> +
> +#ifdef CONFIG_DRM_I915_PXP
> +void intel_pxp_suspend(struct intel_pxp *pxp);
> +void intel_pxp_resume(struct intel_pxp *pxp);
> +#else
> +static inline void intel_pxp_suspend(struct intel_pxp *pxp)
> +{
> +}
> +static inline void intel_pxp_resume(struct intel_pxp *pxp)
> +{
> +}
> +#endif
> +
> +#endif /* __INTEL_PXP_PM_H__ */
> diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_session.c b/drivers/gpu/drm/i915/pxp/intel_pxp_session.c
> index 1f13e58d47f9..79a84794e410 100644
> --- a/drivers/gpu/drm/i915/pxp/intel_pxp_session.c
> +++ b/drivers/gpu/drm/i915/pxp/intel_pxp_session.c
> @@ -21,29 +21,36 @@
>  
>  static bool intel_pxp_session_is_in_play(struct intel_pxp *pxp, u32 id)
>  {
> -	struct intel_gt *gt = pxp_to_gt(pxp);
> +	struct intel_uncore *uncore = pxp_to_gt(pxp)->uncore;
>  	intel_wakeref_t wakeref;
>  	u32 sip = 0;
>  
> -	with_intel_runtime_pm(gt->uncore->rpm, wakeref)
> -		sip = intel_uncore_read(gt->uncore, GEN12_KCR_SIP);
> +	/* if we're suspended the session is considered off */
> +	with_intel_runtime_pm_if_in_use(uncore->rpm, wakeref)
> +		sip = intel_uncore_read(uncore, GEN12_KCR_SIP);
>  
>  	return sip & BIT(id);
>  }
>  
>  static int pxp_wait_for_session_state(struct intel_pxp *pxp, u32 id, bool in_play)
>  {
> -	struct intel_gt *gt = pxp_to_gt(pxp);
> +	struct intel_uncore *uncore = pxp_to_gt(pxp)->uncore;
>  	intel_wakeref_t wakeref;
>  	u32 mask = BIT(id);
>  	int ret;
>  
> -	with_intel_runtime_pm(gt->uncore->rpm, wakeref)
> -		ret = intel_wait_for_register(gt->uncore,
> -					      GEN12_KCR_SIP,
> -					      mask,
> -					      in_play ? mask : 0,
> -					      100);
> +	/* if we're suspended the session is considered off */
> +	wakeref = intel_runtime_pm_get_if_in_use(uncore->rpm);
> +	if (!wakeref)
> +		return in_play ? -ENODEV : 0;
> +
> +	ret = intel_wait_for_register(uncore,
> +				      GEN12_KCR_SIP,
> +				      mask,
> +				      in_play ? mask : 0,
> +				      100);
> +
> +	intel_runtime_pm_put(uncore->rpm, wakeref);
>  
>  	return ret;
>  }
> @@ -132,6 +139,7 @@ void intel_pxp_session_work(struct work_struct *work)
>  {
>  	struct intel_pxp *pxp = container_of(work, typeof(*pxp), session_work);
>  	struct intel_gt *gt = pxp_to_gt(pxp);
> +	intel_wakeref_t wakeref;
>  	u32 events = 0;
>  
>  	spin_lock_irq(&gt->irq_lock);
> @@ -144,6 +152,14 @@ void intel_pxp_session_work(struct work_struct *work)
>  	if (events & PXP_INVAL_REQUIRED)
>  		intel_pxp_invalidate(pxp);
>  
> +	/*
> +	 * If we're processing an event while suspending then don't bother,
> +	 * we're going to re-init everything on resume anyway.
> +	 */
> +	wakeref = intel_runtime_pm_get_if_in_use(gt->uncore->rpm);
> +	if (!wakeref)
> +		return;
> +
>  	if (events & PXP_TERMINATION_REQUEST) {
>  		events &= ~PXP_TERMINATION_COMPLETE;
>  		pxp_terminate(pxp);
> @@ -151,4 +167,6 @@ void intel_pxp_session_work(struct work_struct *work)
>  
>  	if (events & PXP_TERMINATION_COMPLETE)
>  		pxp_terminate_complete(pxp);
> +
> +	intel_runtime_pm_put(gt->uncore->rpm, wakeref);
>  }
> diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_tee.c b/drivers/gpu/drm/i915/pxp/intel_pxp_tee.c
> index c0998047c1b6..b0094112b0d1 100644
> --- a/drivers/gpu/drm/i915/pxp/intel_pxp_tee.c
> +++ b/drivers/gpu/drm/i915/pxp/intel_pxp_tee.c
> @@ -78,16 +78,25 @@ static int intel_pxp_tee_io_message(struct intel_pxp *pxp,
>  static int i915_pxp_tee_component_bind(struct device *i915_kdev,
>  				       struct device *tee_kdev, void *data)
>  {
> +	struct drm_i915_private *i915 = kdev_to_i915(i915_kdev);
>  	struct intel_pxp *pxp = i915_dev_to_pxp(i915_kdev);
> +	intel_wakeref_t wakeref;
>  
>  	mutex_lock(&pxp->tee_mutex);
>  	pxp->pxp_component = data;
>  	pxp->pxp_component->tee_dev = tee_kdev;
>  	mutex_unlock(&pxp->tee_mutex);
>  
> +	/* if we are suspended, the HW will be re-initialized on resume */
> +	wakeref = intel_runtime_pm_get_if_in_use(&i915->runtime_pm);
> +	if (!wakeref)
> +		return 0;
> +
>  	/* the component is required to fully start the PXP HW */
>  	intel_pxp_init_hw(pxp);
>  
> +	intel_runtime_pm_put(&i915->runtime_pm, wakeref);
> +
>  	return 0;
>  }
>  
> -- 
> 2.32.0
> 

  reply	other threads:[~2021-07-16 11:42 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-16  4:10 [PATCH v5 00/15] drm/i915: Introduce Intel PXP Daniele Ceraolo Spurio
2021-07-16  4:10 ` [PATCH v5 01/15] drm/i915/pxp: Define PXP component interface Daniele Ceraolo Spurio
2021-07-16  4:10 ` [PATCH v5 02/15] mei: pxp: export pavp client to me client bus Daniele Ceraolo Spurio
2021-07-26 15:04   ` Winkler, Tomas
2021-07-26 15:47     ` Daniele Ceraolo Spurio
2021-07-16  4:10 ` [PATCH v5 03/15] drm/i915/pxp: define PXP device flag and kconfig Daniele Ceraolo Spurio
2021-07-21 18:50   ` Rodrigo Vivi
2021-07-16  4:10 ` [PATCH v5 04/15] drm/i915/pxp: allocate a vcs context for pxp usage Daniele Ceraolo Spurio
2021-07-16  4:10 ` [PATCH v5 05/15] drm/i915/pxp: Implement funcs to create the TEE channel Daniele Ceraolo Spurio
2021-07-21 18:47   ` Rodrigo Vivi
2021-07-16  4:10 ` [PATCH v5 06/15] drm/i915/pxp: set KCR reg init Daniele Ceraolo Spurio
2021-07-16  4:10 ` [PATCH v5 07/15] drm/i915/pxp: Create the arbitrary session after boot Daniele Ceraolo Spurio
2021-07-21 18:49   ` Rodrigo Vivi
2021-07-16  4:10 ` [PATCH v5 08/15] drm/i915/pxp: Implement arb session teardown Daniele Ceraolo Spurio
2021-07-16  4:10 ` [PATCH v5 09/15] drm/i915/pxp: Implement PXP irq handler Daniele Ceraolo Spurio
2021-07-21 18:59   ` Rodrigo Vivi
2021-07-21 21:07     ` Daniele Ceraolo Spurio
2021-07-16  4:10 ` [PATCH v5 10/15] drm/i915/pxp: interfaces for using protected objects Daniele Ceraolo Spurio
2021-07-19 18:51   ` Rodrigo Vivi
2021-07-16  4:10 ` [PATCH v5 11/15] drm/i915/pxp: start the arb session on demand Daniele Ceraolo Spurio
2021-07-16  4:10 ` [PATCH v5 12/15] drm/i915/pxp: Enable PXP power management Daniele Ceraolo Spurio
2021-07-16 11:42   ` Rodrigo Vivi [this message]
2021-07-16  4:10 ` [PATCH v5 13/15] drm/i915/pxp: Add plane decryption support Daniele Ceraolo Spurio
2021-07-16  4:10 ` [PATCH v5 14/15] drm/i915/pxp: black pixels on pxp disabled Daniele Ceraolo Spurio
2021-07-16  4:10 ` [PATCH v5 15/15] drm/i915/pxp: enable PXP for integrated Gen12 Daniele Ceraolo Spurio
2021-07-27  9:30 ` [PATCH v5 00/15] drm/i915: Introduce Intel PXP Lionel Landwerlin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YPFwni9CN35vFqio@intel.com \
    --to=rodrigo.vivi@intel.com \
    --cc=Huang@intel.com \
    --cc=chris@chris-wilson.co.uk \
    --cc=daniele.ceraolospurio@intel.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=sean.z.huang@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.