Re: [Intel-gfx] [PATCH v9 12/17] drm/i915/pxp: Enable PXP power management
On Wed, Sep 15, 2021 at 11:23:45AM -0400, Rodrigo Vivi wrote:
> On Wed, Sep 15, 2021 at 08:11:54AM -0700, Daniele Ceraolo Spurio wrote:
> >
> >
> > On 9/14/2021 12:13 PM, Rodrigo Vivi wrote:
> > > On Fri, Sep 10, 2021 at 08:36:22AM -0700, Daniele Ceraolo Spurio wrote:
> > > > From: "Huang, Sean Z"
> > > >
> > > > 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)
> > > > v6: drop invalidation in runtime suspend (Rodrigo)
> > > >
> > > > Signed-off-by: Huang, Sean Z
> > > > Signed-off-by: Daniele Ceraolo Spurio
> > > > Cc: Chris Wilson
> > > > Cc: Rodrigo Vivi
> > > ops, I had missed this patch. Sorry
> > > and thanks Alan for the ping.
> > >
> > > > ---
> > > > 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 | 46
> > > > 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, 124 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 b22b8c195bb8..366e82cec44d 100644
> > > > --- a/drivers/gpu/drm/i915/Makefile
> > > > +++ b/drivers/gpu/drm/i915/Makefile
> > > > @@ -286,6 +286,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 dea8e2479897..b47a8d8f1bb5 100644
> > > > --- a/drivers/gpu/drm/i915/gt/intel_gt_pm.c
> > > > +++ b/drivers/gpu/drm/i915/gt/intel_gt_pm.c
> > > > @@ -18,6 +18,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)
> > > > {
> > > > @@ -262,6 +263,8 @@ int intel_gt_resume(struct intel_gt *gt)
> > > > intel_uc_resume(>uc);
> > > > + intel_pxp_resume(>pxp);
> > > > +
> > > > user_forcewake(gt, false);
> > > > out_fw:
> > > > @@ -296,6 +299,7 @@ void intel_gt_suspend_prepare(struct intel_gt *gt)
> > > > user_forcewake(gt, true);
> > > > wait_for_suspend(gt);
> > > > + intel_pxp_suspend(>pxp, false);
> > > > intel_uc_suspend(>uc);
> > > > }
> > > > @@ -346,6 +350,7 @@ void intel_gt_suspend_late(struct intel_gt *gt)
> > > > void intel_gt_runtime_suspend(struct intel_gt *gt)
> > > > {
> > > > + intel_pxp_suspend(>pxp, true);
> > > We should actually remove this from here
> >
> > No we shouldn't. The PXP suspend does other things in addition to the
> > invalidation (e.g. marking the ARB session as invalid) and those must be
> > performed, otherwise the SW state won't match the HW. That's why I added a
> > variable instead of dropping the call. Similar for the resume.
>
> Why? We are blocking the runtime PM. This functions will never be called
> anyway...
Daniele reminded me offline that there are cases where we might not have the
active
context, hence the runtime_pm won't be blocked and we need to take care of the
pxp suspend and resume without needing to invalidate the context.
with that clarified:
Reviewed-by: Rodrigo Vivi
>
> >
> > Daniele
> >
> > >
> > > > intel_uc_runtime_suspend(>uc);
> > > > GT_TRACE(gt, "\n");
> > > > @@ -353,11 +358,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
Re: [PATCH v9 12/17] drm/i915/pxp: Enable PXP power management
On Wed, Sep 15, 2021 at 08:11:54AM -0700, Daniele Ceraolo Spurio wrote:
>
>
> On 9/14/2021 12:13 PM, Rodrigo Vivi wrote:
> > On Fri, Sep 10, 2021 at 08:36:22AM -0700, Daniele Ceraolo Spurio wrote:
> > > From: "Huang, Sean Z"
> > >
> > > 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)
> > > v6: drop invalidation in runtime suspend (Rodrigo)
> > >
> > > Signed-off-by: Huang, Sean Z
> > > Signed-off-by: Daniele Ceraolo Spurio
> > > Cc: Chris Wilson
> > > Cc: Rodrigo Vivi
> > ops, I had missed this patch. Sorry
> > and thanks Alan for the ping.
> >
> > > ---
> > > 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 | 46
> > > 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, 124 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 b22b8c195bb8..366e82cec44d 100644
> > > --- a/drivers/gpu/drm/i915/Makefile
> > > +++ b/drivers/gpu/drm/i915/Makefile
> > > @@ -286,6 +286,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 dea8e2479897..b47a8d8f1bb5 100644
> > > --- a/drivers/gpu/drm/i915/gt/intel_gt_pm.c
> > > +++ b/drivers/gpu/drm/i915/gt/intel_gt_pm.c
> > > @@ -18,6 +18,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)
> > > {
> > > @@ -262,6 +263,8 @@ int intel_gt_resume(struct intel_gt *gt)
> > > intel_uc_resume(>uc);
> > > + intel_pxp_resume(>pxp);
> > > +
> > > user_forcewake(gt, false);
> > > out_fw:
> > > @@ -296,6 +299,7 @@ void intel_gt_suspend_prepare(struct intel_gt *gt)
> > > user_forcewake(gt, true);
> > > wait_for_suspend(gt);
> > > + intel_pxp_suspend(>pxp, false);
> > > intel_uc_suspend(>uc);
> > > }
> > > @@ -346,6 +350,7 @@ void intel_gt_suspend_late(struct intel_gt *gt)
> > > void intel_gt_runtime_suspend(struct intel_gt *gt)
> > > {
> > > + intel_pxp_suspend(>pxp, true);
> > We should actually remove this from here
>
> No we shouldn't. The PXP suspend does other things in addition to the
> invalidation (e.g. marking the ARB session as invalid) and those must be
> performed, otherwise the SW state won't match the HW. That's why I added a
> variable instead of dropping the call. Similar for the resume.
Why? We are blocking the runtime PM. This functions will never be called
anyway...
>
> Daniele
>
> >
> > > intel_uc_runtime_suspend(>uc);
> > > GT_TRACE(gt, "\n");
> > > @@ -353,11 +358,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(>uc);
> > > + ret = intel_uc_runtime_resume(>uc);
> > > + if (ret)
> > > + return ret;
> > > +
> > > + intel_pxp_resume(>pxp);
> > And from here...
> >
> > > +
> > > + 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 59fb4c710c8c..d5bcc70a22d4 100644
> > > --- a/drivers/gpu/drm/i915/i915_drv.c
> > > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > > @@ -67,6 +67,8 @@
> > > #include
Re: [PATCH v9 12/17] drm/i915/pxp: Enable PXP power management
On 9/14/2021 12:13 PM, Rodrigo Vivi wrote:
On Fri, Sep 10, 2021 at 08:36:22AM -0700, Daniele Ceraolo Spurio wrote:
From: "Huang, Sean Z"
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)
v6: drop invalidation in runtime suspend (Rodrigo)
Signed-off-by: Huang, Sean Z
Signed-off-by: Daniele Ceraolo Spurio
Cc: Chris Wilson
Cc: Rodrigo Vivi
ops, I had missed this patch. Sorry
and thanks Alan for the ping.
---
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 | 46
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, 124 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 b22b8c195bb8..366e82cec44d 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -286,6 +286,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 dea8e2479897..b47a8d8f1bb5 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_pm.c
+++ b/drivers/gpu/drm/i915/gt/intel_gt_pm.c
@@ -18,6 +18,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)
{
@@ -262,6 +263,8 @@ int intel_gt_resume(struct intel_gt *gt)
intel_uc_resume(>uc);
+ intel_pxp_resume(>pxp);
+
user_forcewake(gt, false);
out_fw:
@@ -296,6 +299,7 @@ void intel_gt_suspend_prepare(struct intel_gt *gt)
user_forcewake(gt, true);
wait_for_suspend(gt);
+ intel_pxp_suspend(>pxp, false);
intel_uc_suspend(>uc);
}
@@ -346,6 +350,7 @@ void intel_gt_suspend_late(struct intel_gt *gt)
void intel_gt_runtime_suspend(struct intel_gt *gt)
{
+ intel_pxp_suspend(>pxp, true);
We should actually remove this from here
No we shouldn't. The PXP suspend does other things in addition to the
invalidation (e.g. marking the ARB session as invalid) and those must be
performed, otherwise the SW state won't match the HW. That's why I added
a variable instead of dropping the call. Similar for the resume.
Daniele
intel_uc_runtime_suspend(>uc);
GT_TRACE(gt, "\n");
@@ -353,11 +358,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(>uc);
+ ret = intel_uc_runtime_resume(>uc);
+ if (ret)
+ return ret;
+
+ intel_pxp_resume(>pxp);
And from here...
+
+ 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 59fb4c710c8c..d5bcc70a22d4 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 ..23fd86de5a24
--- /dev/null
Re: [PATCH v9 12/17] drm/i915/pxp: Enable PXP power management
On Fri, Sep 10, 2021 at 08:36:22AM -0700, Daniele Ceraolo Spurio wrote:
> From: "Huang, Sean Z"
>
> 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)
> v6: drop invalidation in runtime suspend (Rodrigo)
>
> Signed-off-by: Huang, Sean Z
> Signed-off-by: Daniele Ceraolo Spurio
> Cc: Chris Wilson
> Cc: Rodrigo Vivi
ops, I had missed this patch. Sorry
and thanks Alan for the ping.
> ---
> 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 | 46
> 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, 124 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 b22b8c195bb8..366e82cec44d 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -286,6 +286,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 dea8e2479897..b47a8d8f1bb5 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt_pm.c
> +++ b/drivers/gpu/drm/i915/gt/intel_gt_pm.c
> @@ -18,6 +18,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)
> {
> @@ -262,6 +263,8 @@ int intel_gt_resume(struct intel_gt *gt)
>
> intel_uc_resume(>uc);
>
> + intel_pxp_resume(>pxp);
> +
> user_forcewake(gt, false);
>
> out_fw:
> @@ -296,6 +299,7 @@ void intel_gt_suspend_prepare(struct intel_gt *gt)
> user_forcewake(gt, true);
> wait_for_suspend(gt);
>
> + intel_pxp_suspend(>pxp, false);
> intel_uc_suspend(>uc);
> }
>
> @@ -346,6 +350,7 @@ void intel_gt_suspend_late(struct intel_gt *gt)
>
> void intel_gt_runtime_suspend(struct intel_gt *gt)
> {
> + intel_pxp_suspend(>pxp, true);
We should actually remove this from here
> intel_uc_runtime_suspend(>uc);
>
> GT_TRACE(gt, "\n");
> @@ -353,11 +358,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(>uc);
> + ret = intel_uc_runtime_resume(>uc);
> + if (ret)
> + return ret;
> +
> + intel_pxp_resume(>pxp);
And from here...
> +
> + 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 59fb4c710c8c..d5bcc70a22d4 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 ..23fd86de5a24
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/pxp/intel_pxp_pm.c
> @@ -0,0 +1,46 @@
> +// SPDX-License-Identifier: MIT
> +/*
> + * Copyright(c) 2020 Intel Corporation.
> + */
> +
>
[PATCH v9 12/17] drm/i915/pxp: Enable PXP power management
From: "Huang, Sean Z"
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)
v6: drop invalidation in runtime suspend (Rodrigo)
Signed-off-by: Huang, Sean Z
Signed-off-by: Daniele Ceraolo Spurio
Cc: Chris Wilson
Cc: Rodrigo Vivi
---
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 | 46
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, 124 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 b22b8c195bb8..366e82cec44d 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -286,6 +286,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 dea8e2479897..b47a8d8f1bb5 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_pm.c
+++ b/drivers/gpu/drm/i915/gt/intel_gt_pm.c
@@ -18,6 +18,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)
{
@@ -262,6 +263,8 @@ int intel_gt_resume(struct intel_gt *gt)
intel_uc_resume(>uc);
+ intel_pxp_resume(>pxp);
+
user_forcewake(gt, false);
out_fw:
@@ -296,6 +299,7 @@ void intel_gt_suspend_prepare(struct intel_gt *gt)
user_forcewake(gt, true);
wait_for_suspend(gt);
+ intel_pxp_suspend(>pxp, false);
intel_uc_suspend(>uc);
}
@@ -346,6 +350,7 @@ void intel_gt_suspend_late(struct intel_gt *gt)
void intel_gt_runtime_suspend(struct intel_gt *gt)
{
+ intel_pxp_suspend(>pxp, true);
intel_uc_runtime_suspend(>uc);
GT_TRACE(gt, "\n");
@@ -353,11 +358,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(>uc);
+ ret = intel_uc_runtime_resume(>uc);
+ if (ret)
+ return ret;
+
+ intel_pxp_resume(>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 59fb4c710c8c..d5bcc70a22d4 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 ..23fd86de5a24
--- /dev/null
+++ b/drivers/gpu/drm/i915/pxp/intel_pxp_pm.c
@@ -0,0 +1,46 @@
+// 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, bool runtime)
+{
+ if (!intel_pxp_is_enabled(pxp))
+ return;
+
+ pxp->arb_is_valid = false;
+
+ /*
+* Contexts using protected objects keep a runtime PM reference, so we
+* can only runtime suspend when all of them have been
