drm/i915/pxp: start the arb session on demand

Now that we can handle destruction and re-creation of the arb session,
we can postpone the start of the session to the first submission that
requires it, to avoid keeping it running with no user.

v10: increase timeout when waiting in intel_pxp_start as firmware
     session startup is slower right after boot.
v13: increase the same timeout by 50 milisec because previous timeout
     was not enough to cover two lower level 100 milisec timeouts
     in the session termination + creation steps.

Signed-off-by: Alan Previn <alan.previn.teres.alexis@intel.com>
Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20210924191452.1539378-12-alan.previn.teres.alexis@intel.com

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c
index 08360d6..51861a6 100644 (file)
--- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
@@ -267,7 +267,9 @@ static int proto_context_set_protected(struct drm_i915_private *i915,
                 * which in turn requires the device to be active.
                 */
                pc->pxp_wakeref = intel_runtime_pm_get(&i915->runtime_pm);
-               ret = intel_pxp_wait_for_arb_start(&i915->gt.pxp);
+
+               if (!intel_pxp_is_active(&i915->gt.pxp))
+                       ret = intel_pxp_start(&i915->gt.pxp);
        }
 
        return ret;

diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp.c b/drivers/gpu/drm/i915/pxp/intel_pxp.c
index ee507fd..7f241c0 100644 (file)
--- a/drivers/gpu/drm/i915/pxp/intel_pxp.c
+++ b/drivers/gpu/drm/i915/pxp/intel_pxp.c
@@ -91,6 +91,7 @@ void intel_pxp_init(struct intel_pxp *pxp)
        init_completion(&pxp->termination);
        complete_all(&pxp->termination);
 
+       mutex_init(&pxp->arb_mutex);
        INIT_WORK(&pxp->session_work, intel_pxp_session_work);
 
        ret = create_vcs_context(pxp);
@@ -127,7 +128,7 @@ void intel_pxp_mark_termination_in_progress(struct intel_pxp *pxp)
        reinit_completion(&pxp->termination);
 }
 
-static void intel_pxp_queue_termination(struct intel_pxp *pxp)
+static void pxp_queue_termination(struct intel_pxp *pxp)
 {
        struct intel_gt *gt = pxp_to_gt(pxp);
 
@@ -146,31 +147,41 @@ static void intel_pxp_queue_termination(struct intel_pxp *pxp)
  * the arb session is restarted from the irq work when we receive the
  * termination completion interrupt
  */
-int intel_pxp_wait_for_arb_start(struct intel_pxp *pxp)
+int intel_pxp_start(struct intel_pxp *pxp)
 {
+       int ret = 0;
+
        if (!intel_pxp_is_enabled(pxp))
-               return 0;
+               return -ENODEV;
+
+       mutex_lock(&pxp->arb_mutex);
+
+       if (pxp->arb_is_valid)
+               goto unlock;
+
+       pxp_queue_termination(pxp);
 
        if (!wait_for_completion_timeout(&pxp->termination,
-                                        msecs_to_jiffies(100)))
-               return -ETIMEDOUT;
+                                       msecs_to_jiffies(250))) {
+               ret = -ETIMEDOUT;
+               goto unlock;
+       }
+
+       /* make sure the compiler doesn't optimize the double access */
+       barrier();
 
        if (!pxp->arb_is_valid)
-               return -EIO;
+               ret = -EIO;
 
-       return 0;
+unlock:
+       mutex_unlock(&pxp->arb_mutex);
+       return ret;
 }
 
 void intel_pxp_init_hw(struct intel_pxp *pxp)
 {
        kcr_pxp_enable(pxp_to_gt(pxp));
        intel_pxp_irq_enable(pxp);
-
-       /*
-        * the session could've been attacked while we weren't loaded, so
-        * handle it as if it was and re-create it.
-        */
-       intel_pxp_queue_termination(pxp);
 }
 
 void intel_pxp_fini_hw(struct intel_pxp *pxp)

diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp.h b/drivers/gpu/drm/i915/pxp/intel_pxp.h
index 430f01e..bce0014 100644 (file)
--- a/drivers/gpu/drm/i915/pxp/intel_pxp.h
+++ b/drivers/gpu/drm/i915/pxp/intel_pxp.h
@@ -26,7 +26,8 @@ void intel_pxp_init_hw(struct intel_pxp *pxp);
 void intel_pxp_fini_hw(struct intel_pxp *pxp);
 
 void intel_pxp_mark_termination_in_progress(struct intel_pxp *pxp);
-int intel_pxp_wait_for_arb_start(struct intel_pxp *pxp);
+
+int intel_pxp_start(struct intel_pxp *pxp);
 
 int intel_pxp_key_check(struct intel_pxp *pxp, struct drm_i915_gem_object *obj);
 
@@ -40,11 +41,16 @@ static inline void intel_pxp_fini(struct intel_pxp *pxp)
 {
 }
 
-static inline int intel_pxp_wait_for_arb_start(struct intel_pxp *pxp)
+static inline int intel_pxp_start(struct intel_pxp *pxp)
 {
        return -ENODEV;
 }
 
+static inline bool intel_pxp_is_active(const struct intel_pxp *pxp)
+{
+       return false;
+}
+
 static inline int intel_pxp_key_check(struct intel_pxp *pxp,
                                      struct drm_i915_gem_object *obj)
 {

diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_irq.c b/drivers/gpu/drm/i915/pxp/intel_pxp_irq.c
index 300cf2b..7b25efa 100644 (file)
--- a/drivers/gpu/drm/i915/pxp/intel_pxp_irq.c
+++ b/drivers/gpu/drm/i915/pxp/intel_pxp_irq.c
@@ -32,7 +32,7 @@ void intel_pxp_irq_handler(struct intel_pxp *pxp, u16 iir)
                   GEN12_DISPLAY_APP_TERMINATED_PER_FW_REQ_INTERRUPT)) {
                /* immediately mark PXP as inactive on termination */
                intel_pxp_mark_termination_in_progress(pxp);
-               pxp->session_events |= PXP_TERMINATION_REQUEST;
+               pxp->session_events |= PXP_TERMINATION_REQUEST | PXP_INVAL_REQUIRED;
        }
 
        if (iir & GEN12_DISPLAY_STATE_RESET_COMPLETE_INTERRUPT)

diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_session.c b/drivers/gpu/drm/i915/pxp/intel_pxp_session.c
index c6a5e41..a95cc44 100644 (file)
--- a/drivers/gpu/drm/i915/pxp/intel_pxp_session.c
+++ b/drivers/gpu/drm/i915/pxp/intel_pxp_session.c
@@ -88,9 +88,6 @@ static int pxp_terminate_arb_session_and_global(struct intel_pxp *pxp)
        /* must mark termination in progress calling this function */
        GEM_WARN_ON(pxp->arb_is_valid);
 
-       /* invalidate protected objects */
-       intel_pxp_invalidate(pxp);
-
        /* terminate the hw sessions */
        ret = intel_pxp_terminate_session(pxp, ARB_SESSION);
        if (ret) {
@@ -147,6 +144,9 @@ void intel_pxp_session_work(struct work_struct *work)
        if (!events)
                return;
 
+       if (events & PXP_INVAL_REQUIRED)
+               intel_pxp_invalidate(pxp);
+
        if (events & PXP_TERMINATION_REQUEST) {
                events &= ~PXP_TERMINATION_COMPLETE;
                pxp_terminate(pxp);

diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_tee.c b/drivers/gpu/drm/i915/pxp/intel_pxp_tee.c
index 494bd6b..3fc3ddf 100644 (file)
--- a/drivers/gpu/drm/i915/pxp/intel_pxp_tee.c
+++ b/drivers/gpu/drm/i915/pxp/intel_pxp_tee.c
@@ -78,9 +78,7 @@ unlock:
 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);
-       int ret;
 
        mutex_lock(&pxp->tee_mutex);
        pxp->pxp_component = data;
@@ -89,14 +87,8 @@ static int i915_pxp_tee_component_bind(struct device *i915_kdev,
 
        /* the component is required to fully start the PXP HW */
        intel_pxp_init_hw(pxp);
-       ret = intel_pxp_wait_for_arb_start(pxp);
-       if (ret) {
-               drm_err(&i915->drm, "Failed to create arb session during bind\n");
-               intel_pxp_fini_hw(pxp);
-               pxp->pxp_component = NULL;
-       }
 
-       return ret;
+       return 0;
 }
 
 static void i915_pxp_tee_component_unbind(struct device *i915_kdev,

diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_types.h b/drivers/gpu/drm/i915/pxp/intel_pxp_types.h
index c394ab2..ae24064 100644 (file)
--- a/drivers/gpu/drm/i915/pxp/intel_pxp_types.h
+++ b/drivers/gpu/drm/i915/pxp/intel_pxp_types.h
@@ -27,6 +27,7 @@ struct intel_pxp {
         * even if the keys are gone, so we can't rely on the HW state of the
         * session to know if it's valid and need to track the status in SW.
         */
+       struct mutex arb_mutex; /* protects arb session start */
        bool arb_is_valid;
 
        /*
@@ -53,6 +54,7 @@ struct intel_pxp {
        u32 session_events; /* protected with gt->irq_lock */
 #define PXP_TERMINATION_REQUEST  BIT(0)
 #define PXP_TERMINATION_COMPLETE BIT(1)
+#define PXP_INVAL_REQUIRED       BIT(2)
 };
 
 #endif /* __INTEL_PXP_TYPES_H__ */