* [Intel-gfx] [PATCH v5 0/1] drm/i915/pxp: Prepare intel_pxp entry points for MTL
@ 2022-11-29 0:31 Alan Previn
2022-11-29 0:31 ` [Intel-gfx] [PATCH v5 1/1] drm/i915/pxp: Promote pxp subsystem to top-level of i915 Alan Previn
` (4 more replies)
0 siblings, 5 replies; 13+ messages in thread
From: Alan Previn @ 2022-11-29 0:31 UTC (permalink / raw)
To: intel-gfx
MTL has two tiles that is represented by the intel_gt structure in the i915
code. The PXP feature has a control-structure that currently hangs off the
intel_gt structure. In MTL, the standalone media tile (i.e. not the root
tile) contains the VDBOX and KCR engine which are among several assets
that PXP relies on for establishing and tearing down the PXP session.
However PXP is a global feature as other engines on other tiles can reference
the PXP session in an object's info within batch buffer instructions. That
coherrency is handled implicitly by the HW. In fact, for the forseeable future,
we are expecting this link whereby only one of the tiles will be the control-gt
for the PXP subsystem.
Keeping the intel_pxp structure within the intel_gt structure makes some
internal functionalities more straight forward but adds code complexity to
code readibility and maintainibility to many external-to-pxp subsystems
which may need to pick the correct intel_gt structure. An example of this
would be the intel_pxp_is_active or intel_pxp_is_enabled functionality which
should be viewed as a global level inquiry, not a per-gt inquiry.
That said, this series promotes the intel_pxp structure into the
drm_i915_private structure making it a top-level subsystem and the PXP
subsystem will select the control gt internally and keep a pointer to
it for internal reference.
Changes from prior revs:
v4: - Instead of maintaining intel_pxp as an intel_gt structure member and
creating a number of convoluted helpers that takes in i915 as input
and redirects to the correct intel_gt or takes any intel_gt and internally
replaces with the correct intel_gt, promote it to be a top-level i915
structure.
v3: - Rename gt level helper functions to "intel_pxp_is_enabled/supported/
active_on_gt" (Daniele)
- Upgrade _gt_supports_pxp to replace what was intel_gtpxp_is_supported
as the new intel_pxp_is_supported_on_gt to check for PXP feature
support vs the tee support for huc authentication. Fix pxp-debugfs-
registration to use only the former to decide support. (Daniele)
- Couple minor optimizations.
v2: - Avoid introduction of new device info or gt variables and use
existing checks / macros to differentiate the correct GT->PXP
control ownership (Daniele Ceraolo Spurio)
- Don't reuse the updated global-checkers for per-GT callers (such
as other files within PXP) to avoid unnecessary GT-reparsing,
expose a replacement helper like the prior ones. (Daniele).
v1: Add one more patch to the series for the intel_pxp suspend/resume
for similiar refactoring
Alan Previn (1):
drm/i915/pxp: Promote pxp subsystem to top-level of i915
.../drm/i915/display/skl_universal_plane.c | 2 +-
drivers/gpu/drm/i915/gem/i915_gem_context.c | 6 +-
drivers/gpu/drm/i915/gem/i915_gem_create.c | 2 +-
.../gpu/drm/i915/gem/i915_gem_execbuffer.c | 2 +-
drivers/gpu/drm/i915/gt/intel_gt.c | 5 --
drivers/gpu/drm/i915/gt/intel_gt_debugfs.c | 1 -
drivers/gpu/drm/i915/gt/intel_gt_irq.c | 2 +-
drivers/gpu/drm/i915/gt/intel_gt_pm.c | 8 ---
drivers/gpu/drm/i915/gt/intel_gt_types.h | 3 -
drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c | 2 +-
drivers/gpu/drm/i915/i915_driver.c | 20 ++++++
drivers/gpu/drm/i915/i915_drv.h | 3 +
drivers/gpu/drm/i915/pxp/intel_pxp.c | 72 ++++++++++++++-----
drivers/gpu/drm/i915/pxp/intel_pxp.h | 6 +-
drivers/gpu/drm/i915/pxp/intel_pxp_cmd.c | 8 ++-
drivers/gpu/drm/i915/pxp/intel_pxp_debugfs.c | 41 +++++++----
drivers/gpu/drm/i915/pxp/intel_pxp_debugfs.h | 4 +-
drivers/gpu/drm/i915/pxp/intel_pxp_irq.c | 10 ++-
drivers/gpu/drm/i915/pxp/intel_pxp_pm.c | 4 +-
drivers/gpu/drm/i915/pxp/intel_pxp_tee.c | 8 ++-
drivers/gpu/drm/i915/pxp/intel_pxp_types.h | 11 +++
21 files changed, 150 insertions(+), 70 deletions(-)
base-commit: d21d6474a37e5d43075a24668807ea40a7ee9fc1
--
2.34.1
^ permalink raw reply [flat|nested] 13+ messages in thread
* [Intel-gfx] [PATCH v5 1/1] drm/i915/pxp: Promote pxp subsystem to top-level of i915
2022-11-29 0:31 [Intel-gfx] [PATCH v5 0/1] drm/i915/pxp: Prepare intel_pxp entry points for MTL Alan Previn
@ 2022-11-29 0:31 ` Alan Previn
2022-11-29 18:35 ` Teres Alexis, Alan Previn
2022-11-29 21:28 ` Rodrigo Vivi
2022-11-29 0:51 ` [Intel-gfx] [PATCH v5 0/1] drm/i915/pxp: Prepare intel_pxp entry points for MTL Teres Alexis, Alan Previn
` (3 subsequent siblings)
4 siblings, 2 replies; 13+ messages in thread
From: Alan Previn @ 2022-11-29 0:31 UTC (permalink / raw)
To: intel-gfx
Starting with MTL, there will be two GT-tiles, a render and media
tile. PXP as a service for supporting workloads with protected
contexts and protected buffers can be subscribed by process
workloads on any tile. However, depending on the platform,
only one of the tiles is used for control events pertaining to PXP
operation (such as creating the arbitration session and session
tear-down). In the case of MTL, this is the media-tile.
Signed-off-by: Alan Previn <alan.previn.teres.alexis@intel.com>
---
.../drm/i915/display/skl_universal_plane.c | 2 +-
drivers/gpu/drm/i915/gem/i915_gem_context.c | 6 +-
drivers/gpu/drm/i915/gem/i915_gem_create.c | 2 +-
.../gpu/drm/i915/gem/i915_gem_execbuffer.c | 2 +-
drivers/gpu/drm/i915/gt/intel_gt.c | 5 --
drivers/gpu/drm/i915/gt/intel_gt_debugfs.c | 1 -
drivers/gpu/drm/i915/gt/intel_gt_irq.c | 2 +-
drivers/gpu/drm/i915/gt/intel_gt_pm.c | 8 ---
drivers/gpu/drm/i915/gt/intel_gt_types.h | 3 -
drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c | 2 +-
drivers/gpu/drm/i915/i915_driver.c | 20 ++++++
drivers/gpu/drm/i915/i915_drv.h | 3 +
drivers/gpu/drm/i915/pxp/intel_pxp.c | 72 ++++++++++++++-----
drivers/gpu/drm/i915/pxp/intel_pxp.h | 6 +-
drivers/gpu/drm/i915/pxp/intel_pxp_cmd.c | 8 ++-
drivers/gpu/drm/i915/pxp/intel_pxp_debugfs.c | 41 +++++++----
drivers/gpu/drm/i915/pxp/intel_pxp_debugfs.h | 4 +-
drivers/gpu/drm/i915/pxp/intel_pxp_irq.c | 10 ++-
drivers/gpu/drm/i915/pxp/intel_pxp_pm.c | 4 +-
drivers/gpu/drm/i915/pxp/intel_pxp_tee.c | 8 ++-
drivers/gpu/drm/i915/pxp/intel_pxp_types.h | 11 +++
21 files changed, 150 insertions(+), 70 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c b/drivers/gpu/drm/i915/display/skl_universal_plane.c
index 76490cc59d8f..4b79c2d2d617 100644
--- a/drivers/gpu/drm/i915/display/skl_universal_plane.c
+++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c
@@ -1848,7 +1848,7 @@ static bool bo_has_valid_encryption(struct drm_i915_gem_object *obj)
{
struct drm_i915_private *i915 = to_i915(obj->base.dev);
- return intel_pxp_key_check(&to_gt(i915)->pxp, obj, false) == 0;
+ return intel_pxp_key_check(i915->pxp, obj, false) == 0;
}
static bool pxp_is_borked(struct drm_i915_gem_object *obj)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c
index 7f2831efc798..46e71f62fcec 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
@@ -257,7 +257,7 @@ static int proto_context_set_protected(struct drm_i915_private *i915,
if (!protected) {
pc->uses_protected_content = false;
- } else if (!intel_pxp_is_enabled(&to_gt(i915)->pxp)) {
+ } else if (!intel_pxp_is_enabled(i915->pxp)) {
ret = -ENODEV;
} else if ((pc->user_flags & BIT(UCONTEXT_RECOVERABLE)) ||
!(pc->user_flags & BIT(UCONTEXT_BANNABLE))) {
@@ -271,8 +271,8 @@ static int proto_context_set_protected(struct drm_i915_private *i915,
*/
pc->pxp_wakeref = intel_runtime_pm_get(&i915->runtime_pm);
- if (!intel_pxp_is_active(&to_gt(i915)->pxp))
- ret = intel_pxp_start(&to_gt(i915)->pxp);
+ if (!intel_pxp_is_active(i915->pxp))
+ ret = intel_pxp_start(i915->pxp);
}
return ret;
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_create.c b/drivers/gpu/drm/i915/gem/i915_gem_create.c
index 33673fe7ee0a..005a7f842784 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_create.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_create.c
@@ -384,7 +384,7 @@ static int ext_set_protected(struct i915_user_extension __user *base, void *data
if (ext.flags)
return -EINVAL;
- if (!intel_pxp_is_enabled(&to_gt(ext_data->i915)->pxp))
+ if (!intel_pxp_is_enabled(ext_data->i915->pxp))
return -ENODEV;
ext_data->flags |= I915_BO_PROTECTED;
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
index 29e9e8d5b6fe..ed74d173a092 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
@@ -869,7 +869,7 @@ static struct i915_vma *eb_lookup_vma(struct i915_execbuffer *eb, u32 handle)
*/
if (i915_gem_context_uses_protected_content(eb->gem_context) &&
i915_gem_object_is_protected(obj)) {
- err = intel_pxp_key_check(&vm->gt->pxp, obj, true);
+ err = intel_pxp_key_check(vm->gt->i915->pxp, obj, true);
if (err) {
i915_gem_object_put(obj);
return ERR_PTR(err);
diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c b/drivers/gpu/drm/i915/gt/intel_gt.c
index 7ef0edb2e37c..1fd6b34ecda3 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt.c
+++ b/drivers/gpu/drm/i915/gt/intel_gt.c
@@ -8,7 +8,6 @@
#include "gem/i915_gem_internal.h"
#include "gem/i915_gem_lmem.h"
-#include "pxp/intel_pxp.h"
#include "i915_drv.h"
#include "i915_perf_oa_regs.h"
@@ -753,8 +752,6 @@ int intel_gt_init(struct intel_gt *gt)
intel_migrate_init(>->migrate, gt);
- intel_pxp_init(>->pxp);
-
goto out_fw;
err_gt:
__intel_gt_disable(gt);
@@ -794,8 +791,6 @@ void intel_gt_driver_unregister(struct intel_gt *gt)
intel_rps_driver_unregister(>->rps);
intel_gsc_fini(>->gsc);
- intel_pxp_fini(>->pxp);
-
/*
* Upon unregistering the device to prevent any new users, cancel
* all in-flight requests so that we can quickly unbind the active
diff --git a/drivers/gpu/drm/i915/gt/intel_gt_debugfs.c b/drivers/gpu/drm/i915/gt/intel_gt_debugfs.c
index dd53641f3637..7876f4c3d0f1 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_debugfs.c
+++ b/drivers/gpu/drm/i915/gt/intel_gt_debugfs.c
@@ -99,7 +99,6 @@ void intel_gt_debugfs_register(struct intel_gt *gt)
intel_sseu_debugfs_register(gt, root);
intel_uc_debugfs_register(>->uc, root);
- intel_pxp_debugfs_register(>->pxp, root);
}
void intel_gt_debugfs_register_files(struct dentry *root,
diff --git a/drivers/gpu/drm/i915/gt/intel_gt_irq.c b/drivers/gpu/drm/i915/gt/intel_gt_irq.c
index 6f6b9e04d916..8fac2660e16b 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_irq.c
+++ b/drivers/gpu/drm/i915/gt/intel_gt_irq.c
@@ -76,7 +76,7 @@ gen11_other_irq_handler(struct intel_gt *gt, const u8 instance,
return gen11_rps_irq_handler(&media_gt->rps, iir);
if (instance == OTHER_KCR_INSTANCE)
- return intel_pxp_irq_handler(>->pxp, iir);
+ return intel_pxp_irq_handler(gt->i915->pxp, iir);
if (instance == OTHER_GSC_INSTANCE)
return intel_gsc_irq_handler(gt, iir);
diff --git a/drivers/gpu/drm/i915/gt/intel_gt_pm.c b/drivers/gpu/drm/i915/gt/intel_gt_pm.c
index 16db85fab0b1..c065950d0bad 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_pm.c
+++ b/drivers/gpu/drm/i915/gt/intel_gt_pm.c
@@ -304,8 +304,6 @@ int intel_gt_resume(struct intel_gt *gt)
intel_uc_resume(>->uc);
- intel_pxp_resume(>->pxp);
-
user_forcewake(gt, false);
out_fw:
@@ -339,8 +337,6 @@ void intel_gt_suspend_prepare(struct intel_gt *gt)
{
user_forcewake(gt, true);
wait_for_suspend(gt);
-
- intel_pxp_suspend_prepare(>->pxp);
}
static suspend_state_t pm_suspend_target(void)
@@ -365,7 +361,6 @@ void intel_gt_suspend_late(struct intel_gt *gt)
GEM_BUG_ON(gt->awake);
intel_uc_suspend(>->uc);
- intel_pxp_suspend(>->pxp);
/*
* On disabling the device, we want to turn off HW access to memory
@@ -393,7 +388,6 @@ void intel_gt_suspend_late(struct intel_gt *gt)
void intel_gt_runtime_suspend(struct intel_gt *gt)
{
- intel_pxp_runtime_suspend(>->pxp);
intel_uc_runtime_suspend(>->uc);
GT_TRACE(gt, "\n");
@@ -411,8 +405,6 @@ int intel_gt_runtime_resume(struct intel_gt *gt)
if (ret)
return ret;
- intel_pxp_runtime_resume(>->pxp);
-
return 0;
}
diff --git a/drivers/gpu/drm/i915/gt/intel_gt_types.h b/drivers/gpu/drm/i915/gt/intel_gt_types.h
index c1d9cd255e06..07ebeffbc8bd 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_types.h
+++ b/drivers/gpu/drm/i915/gt/intel_gt_types.h
@@ -30,7 +30,6 @@
#include "intel_rps_types.h"
#include "intel_migrate_types.h"
#include "intel_wakeref.h"
-#include "pxp/intel_pxp_types.h"
#include "intel_wopcm.h"
struct drm_i915_private;
@@ -267,8 +266,6 @@ struct intel_gt {
u8 wb_index; /* Only used on HAS_L3_CCS_READ() platforms */
} mocs;
- struct intel_pxp pxp;
-
/* gt/gtN sysfs */
struct kobject sysfs_gt;
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c b/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c
index 4f246416db17..534b0aa43316 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c
@@ -32,7 +32,7 @@ int intel_huc_fw_load_and_auth_via_gsc(struct intel_huc *huc)
GEM_WARN_ON(intel_uc_fw_is_loaded(&huc->fw));
- ret = intel_pxp_huc_load_and_auth(&huc_to_gt(huc)->pxp);
+ ret = intel_pxp_huc_load_and_auth(huc_to_gt(huc)->i915->pxp);
if (ret)
return ret;
diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c
index 69103ae37779..3be3c53437e9 100644
--- a/drivers/gpu/drm/i915/i915_driver.c
+++ b/drivers/gpu/drm/i915/i915_driver.c
@@ -73,6 +73,8 @@
#include "gt/intel_gt_pm.h"
#include "gt/intel_rc6.h"
+#include "pxp/intel_pxp.h"
+#include "pxp/intel_pxp_debugfs.h"
#include "pxp/intel_pxp_pm.h"
#include "i915_file_private.h"
@@ -763,6 +765,8 @@ static void i915_driver_register(struct drm_i915_private *dev_priv)
for_each_gt(gt, dev_priv, i)
intel_gt_driver_register(gt);
+ intel_pxp_debugfs_register(dev_priv->pxp);
+
i915_hwmon_register(dev_priv);
intel_display_driver_register(dev_priv);
@@ -794,6 +798,8 @@ static void i915_driver_unregister(struct drm_i915_private *dev_priv)
intel_display_driver_unregister(dev_priv);
+ intel_pxp_fini(&dev_priv->pxp);
+
for_each_gt(gt, dev_priv, i)
intel_gt_driver_unregister(gt);
@@ -937,6 +943,8 @@ int i915_driver_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
if (ret)
goto out_cleanup_modeset2;
+ intel_pxp_init(&i915->pxp);
+
ret = intel_modeset_init(i915);
if (ret)
goto out_cleanup_gem;
@@ -996,6 +1004,8 @@ void i915_driver_remove(struct drm_i915_private *i915)
/* Flush any external code that still may be under the RCU lock */
synchronize_rcu();
+ intel_pxp_fini(&i915->pxp);
+
i915_gem_suspend(i915);
intel_gvt_driver_remove(i915);
@@ -1172,6 +1182,8 @@ static int i915_drm_prepare(struct drm_device *dev)
{
struct drm_i915_private *i915 = to_i915(dev);
+ intel_pxp_suspend_prepare(i915->pxp);
+
/*
* NB intel_display_suspend() may issue new requests after we've
* ostensibly marked the GPU as ready-to-sleep here. We need to
@@ -1252,6 +1264,8 @@ static int i915_drm_suspend_late(struct drm_device *dev, bool hibernation)
disable_rpm_wakeref_asserts(rpm);
+ intel_pxp_suspend(dev_priv->pxp);
+
i915_gem_suspend_late(dev_priv);
for_each_gt(gt, dev_priv, i)
@@ -1358,6 +1372,8 @@ static int i915_drm_resume(struct drm_device *dev)
i915_gem_resume(dev_priv);
+ intel_pxp_resume(dev_priv->pxp);
+
intel_modeset_init_hw(dev_priv);
intel_init_clock_gating(dev_priv);
intel_hpd_init(dev_priv);
@@ -1641,6 +1657,8 @@ static int intel_runtime_suspend(struct device *kdev)
*/
i915_gem_runtime_suspend(dev_priv);
+ intel_pxp_runtime_suspend(dev_priv->pxp);
+
for_each_gt(gt, dev_priv, i)
intel_gt_runtime_suspend(gt);
@@ -1745,6 +1763,8 @@ static int intel_runtime_resume(struct device *kdev)
for_each_gt(gt, dev_priv, i)
intel_gt_runtime_resume(gt);
+ intel_pxp_runtime_resume(dev_priv->pxp);
+
/*
* On VLV/CHV display interrupts are part of the display
* power well, so hpd is reinitialized from there. For
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index a380db36d52c..679fa8a424d5 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -72,6 +72,7 @@ struct intel_encoder;
struct intel_limit;
struct intel_overlay_error_state;
struct vlv_s0ix_state;
+struct intel_pxp;
#define I915_GEM_GPU_DOMAINS \
(I915_GEM_DOMAIN_RENDER | \
@@ -364,6 +365,8 @@ struct drm_i915_private {
struct file *mmap_singleton;
} gem;
+ struct intel_pxp *pxp;
+
u8 pch_ssc_use;
/* For i915gm/i945gm vblank irq workaround */
diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp.c b/drivers/gpu/drm/i915/pxp/intel_pxp.c
index 5efe61f67546..fbef1f3be591 100644
--- a/drivers/gpu/drm/i915/pxp/intel_pxp.c
+++ b/drivers/gpu/drm/i915/pxp/intel_pxp.c
@@ -3,13 +3,19 @@
* Copyright(c) 2020 Intel Corporation.
*/
#include <linux/workqueue.h>
+
+#include "gem/i915_gem_context.h"
+
+#include "gt/intel_context.h"
+#include "gt/intel_gt.h"
+
+#include "i915_drv.h"
+
#include "intel_pxp.h"
#include "intel_pxp_irq.h"
#include "intel_pxp_session.h"
#include "intel_pxp_tee.h"
-#include "gem/i915_gem_context.h"
-#include "gt/intel_context.h"
-#include "i915_drv.h"
+#include "intel_pxp_types.h"
/**
* DOC: PXP
@@ -39,9 +45,9 @@
* performed via the mei_pxp component module.
*/
-struct intel_gt *pxp_to_gt(const struct intel_pxp *pxp)
+struct intel_gt *pxp_to_gt(struct intel_pxp *pxp)
{
- return container_of(pxp, struct intel_gt, pxp);
+ return pxp->ctrl_gt;
}
bool intel_pxp_is_enabled(const struct intel_pxp *pxp)
@@ -138,31 +144,63 @@ static void pxp_init_full(struct intel_pxp *pxp)
destroy_vcs_context(pxp);
}
-void intel_pxp_init(struct intel_pxp *pxp)
+static struct intel_gt *pxp_get_kcr_owner_gt(struct drm_i915_private *i915)
{
- struct intel_gt *gt = pxp_to_gt(pxp);
+ struct intel_gt *gt = NULL;
+ int i = 0;
+
+ for_each_gt(gt, i915, i) {
+ /* There can be only one GT that supports PXP */
+ if (HAS_ENGINE(gt, GSC0))
+ return gt;
+ }
/* we rely on the mei PXP module */
- if (!IS_ENABLED(CONFIG_INTEL_MEI_PXP))
- return;
+ if (IS_ENABLED(CONFIG_INTEL_MEI_PXP))
+ return &i915->gt0;
+
+ return NULL;
+}
+
+int intel_pxp_init(struct intel_pxp **pxp_store_ptr)
+{
+ struct intel_pxp *newpxp;
+
+ newpxp = kzalloc(sizeof(*newpxp), GFP_KERNEL);
+ if (!newpxp)
+ return -ENOMEM;
+
+ *pxp_store_ptr = newpxp;
+
+ newpxp->i915 = container_of(pxp_store_ptr, struct drm_i915_private, pxp);
+ newpxp->ctrl_gt = pxp_get_kcr_owner_gt(newpxp->i915);
+
+ if (!newpxp->ctrl_gt)
+ return -ENODEV;
/*
* If HuC is loaded by GSC but PXP is disabled, we can skip the init of
* the full PXP session/object management and just init the tee channel.
*/
- if (HAS_PXP(gt->i915))
- pxp_init_full(pxp);
- else if (intel_huc_is_loaded_by_gsc(>->uc.huc) && intel_uc_uses_huc(>->uc))
- intel_pxp_tee_component_init(pxp);
+ if (HAS_PXP(newpxp->i915))
+ pxp_init_full(newpxp);
+ else if (intel_huc_is_loaded_by_gsc(&newpxp->ctrl_gt->uc.huc) &&
+ intel_uc_uses_huc(&newpxp->ctrl_gt->uc))
+ intel_pxp_tee_component_init(newpxp);
+
+ return 0;
}
-void intel_pxp_fini(struct intel_pxp *pxp)
+void intel_pxp_fini(struct intel_pxp **pxp)
{
- pxp->arb_is_valid = false;
+ (*pxp)->arb_is_valid = false;
- intel_pxp_tee_component_fini(pxp);
+ intel_pxp_tee_component_fini(*pxp);
- destroy_vcs_context(pxp);
+ destroy_vcs_context(*pxp);
+
+ kfree(*pxp);
+ pxp = NULL;
}
void intel_pxp_mark_termination_in_progress(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 2da309088c6d..424e52bbd4f9 100644
--- a/drivers/gpu/drm/i915/pxp/intel_pxp.h
+++ b/drivers/gpu/drm/i915/pxp/intel_pxp.h
@@ -12,12 +12,12 @@
struct intel_pxp;
struct drm_i915_gem_object;
-struct intel_gt *pxp_to_gt(const struct intel_pxp *pxp);
+struct intel_gt *pxp_to_gt(struct intel_pxp *pxp);
bool intel_pxp_is_enabled(const struct intel_pxp *pxp);
bool intel_pxp_is_active(const struct intel_pxp *pxp);
-void intel_pxp_init(struct intel_pxp *pxp);
-void intel_pxp_fini(struct intel_pxp *pxp);
+int intel_pxp_init(struct intel_pxp **pxp);
+void intel_pxp_fini(struct intel_pxp **pxp);
void intel_pxp_init_hw(struct intel_pxp *pxp);
void intel_pxp_fini_hw(struct intel_pxp *pxp);
diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_cmd.c b/drivers/gpu/drm/i915/pxp/intel_pxp_cmd.c
index f41e45763d0d..0eee51c4a772 100644
--- a/drivers/gpu/drm/i915/pxp/intel_pxp_cmd.c
+++ b/drivers/gpu/drm/i915/pxp/intel_pxp_cmd.c
@@ -3,9 +3,6 @@
* Copyright(c) 2020, Intel Corporation. All rights reserved.
*/
-#include "intel_pxp.h"
-#include "intel_pxp_cmd.h"
-#include "intel_pxp_session.h"
#include "gt/intel_context.h"
#include "gt/intel_engine_pm.h"
#include "gt/intel_gpu_commands.h"
@@ -13,6 +10,11 @@
#include "i915_trace.h"
+#include "intel_pxp.h"
+#include "intel_pxp_cmd.h"
+#include "intel_pxp_session.h"
+#include "intel_pxp_types.h"
+
/* stall until prior PXP and MFX/HCP/HUC objects are cmopleted */
#define MFX_WAIT_PXP (MFX_WAIT | \
MFX_WAIT_DW0_PXP_SYNC_CONTROL_FLAG | \
diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_debugfs.c b/drivers/gpu/drm/i915/pxp/intel_pxp_debugfs.c
index 4359e8be4101..652c00aefe3f 100644
--- a/drivers/gpu/drm/i915/pxp/intel_pxp_debugfs.c
+++ b/drivers/gpu/drm/i915/pxp/intel_pxp_debugfs.c
@@ -9,10 +9,13 @@
#include <drm/drm_print.h>
#include "gt/intel_gt_debugfs.h"
+
#include "i915_drv.h"
+
#include "intel_pxp.h"
#include "intel_pxp_debugfs.h"
#include "intel_pxp_irq.h"
+#include "intel_pxp_types.h"
static int pxp_info_show(struct seq_file *m, void *data)
{
@@ -30,7 +33,19 @@ static int pxp_info_show(struct seq_file *m, void *data)
return 0;
}
-DEFINE_INTEL_GT_DEBUGFS_ATTRIBUTE(pxp_info);
+
+static int pxp_info_open(struct inode *inode, struct file *file)
+{
+ return single_open(file, pxp_info_show, inode->i_private);
+}
+
+static const struct file_operations pxp_info_fops = {
+ .owner = THIS_MODULE,
+ .open = pxp_info_open,
+ .read = seq_read,
+ .llseek = seq_lseek,
+ .release = single_release,
+};
static int pxp_terminate_get(void *data, u64 *val)
{
@@ -59,23 +74,23 @@ static int pxp_terminate_set(void *data, u64 val)
}
DEFINE_SIMPLE_ATTRIBUTE(pxp_terminate_fops, pxp_terminate_get, pxp_terminate_set, "%llx\n");
-void intel_pxp_debugfs_register(struct intel_pxp *pxp, struct dentry *gt_root)
+
+void intel_pxp_debugfs_register(struct intel_pxp *pxp)
{
- static const struct intel_gt_debugfs_file files[] = {
- { "info", &pxp_info_fops, NULL },
- { "terminate_state", &pxp_terminate_fops, NULL },
- };
- struct dentry *root;
+ struct drm_minor *minor = pxp->i915->drm.primary;
+ struct dentry *pxproot;
- if (!gt_root)
+ if (!HAS_PXP(pxp->i915))
return;
- if (!HAS_PXP((pxp_to_gt(pxp)->i915)))
+ pxproot = debugfs_create_dir("pxp", minor->debugfs_root);
+ if (IS_ERR(pxproot))
return;
- root = debugfs_create_dir("pxp", gt_root);
- if (IS_ERR(root))
- return;
+ debugfs_create_file("info", 0444, pxproot,
+ pxp, &pxp_info_fops);
+
+ debugfs_create_file("terminate_state", 0644, pxproot,
+ pxp, &pxp_terminate_fops);
- intel_gt_debugfs_register_files(root, files, ARRAY_SIZE(files), pxp);
}
diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_debugfs.h b/drivers/gpu/drm/i915/pxp/intel_pxp_debugfs.h
index 7e0c3d2f5d7e..299382b59e66 100644
--- a/drivers/gpu/drm/i915/pxp/intel_pxp_debugfs.h
+++ b/drivers/gpu/drm/i915/pxp/intel_pxp_debugfs.h
@@ -10,10 +10,10 @@ struct intel_pxp;
struct dentry;
#ifdef CONFIG_DRM_I915_PXP
-void intel_pxp_debugfs_register(struct intel_pxp *pxp, struct dentry *root);
+void intel_pxp_debugfs_register(struct intel_pxp *pxp);
#else
static inline void
-intel_pxp_debugfs_register(struct intel_pxp *pxp, struct dentry *root)
+intel_pxp_debugfs_register(struct intel_pxp *pxp)
{
}
#endif
diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_irq.c b/drivers/gpu/drm/i915/pxp/intel_pxp_irq.c
index c28be430718a..fd30befbf784 100644
--- a/drivers/gpu/drm/i915/pxp/intel_pxp_irq.c
+++ b/drivers/gpu/drm/i915/pxp/intel_pxp_irq.c
@@ -3,14 +3,18 @@
* Copyright(c) 2020 Intel Corporation.
*/
#include <linux/workqueue.h>
-#include "intel_pxp.h"
-#include "intel_pxp_irq.h"
-#include "intel_pxp_session.h"
+
#include "gt/intel_gt_irq.h"
#include "gt/intel_gt_regs.h"
#include "gt/intel_gt_types.h"
+
#include "i915_irq.h"
#include "i915_reg.h"
+
+#include "intel_pxp.h"
+#include "intel_pxp_irq.h"
+#include "intel_pxp_session.h"
+#include "intel_pxp_types.h"
#include "intel_runtime_pm.h"
/**
diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_pm.c b/drivers/gpu/drm/i915/pxp/intel_pxp_pm.c
index 6a7d4e2ee138..37371f44a550 100644
--- a/drivers/gpu/drm/i915/pxp/intel_pxp_pm.c
+++ b/drivers/gpu/drm/i915/pxp/intel_pxp_pm.c
@@ -3,11 +3,13 @@
* Copyright(c) 2020 Intel Corporation.
*/
+#include "i915_drv.h"
+
#include "intel_pxp.h"
#include "intel_pxp_irq.h"
#include "intel_pxp_pm.h"
#include "intel_pxp_session.h"
-#include "i915_drv.h"
+#include "intel_pxp_types.h"
void intel_pxp_suspend_prepare(struct intel_pxp *pxp)
{
diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_tee.c b/drivers/gpu/drm/i915/pxp/intel_pxp_tee.c
index b0c9170b1395..16782d119bfd 100644
--- a/drivers/gpu/drm/i915/pxp/intel_pxp_tee.c
+++ b/drivers/gpu/drm/i915/pxp/intel_pxp_tee.c
@@ -11,17 +11,19 @@
#include "gem/i915_gem_lmem.h"
#include "i915_drv.h"
+
#include "intel_pxp.h"
-#include "intel_pxp_session.h"
-#include "intel_pxp_tee.h"
#include "intel_pxp_cmd_interface_42.h"
#include "intel_pxp_huc.h"
+#include "intel_pxp_session.h"
+#include "intel_pxp_tee.h"
+#include "intel_pxp_types.h"
static inline struct intel_pxp *i915_dev_to_pxp(struct device *i915_kdev)
{
struct drm_i915_private *i915 = kdev_to_i915(i915_kdev);
- return &to_gt(i915)->pxp;
+ return i915->pxp;
}
static int intel_pxp_tee_io_message(struct intel_pxp *pxp,
diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_types.h b/drivers/gpu/drm/i915/pxp/intel_pxp_types.h
index f74b1e11a505..d550cdba3399 100644
--- a/drivers/gpu/drm/i915/pxp/intel_pxp_types.h
+++ b/drivers/gpu/drm/i915/pxp/intel_pxp_types.h
@@ -12,12 +12,23 @@
#include <linux/workqueue.h>
struct intel_context;
+struct intel_gt;
struct i915_pxp_component;
+struct drm_i915_private;
/**
* struct intel_pxp - pxp state
*/
struct intel_pxp {
+ /** @i915: back poiner to i915*/
+ struct drm_i915_private *i915;
+
+ /**
+ * @ctrl_gt: poiner to the tile that owns the controls for PXP subsystem assets that
+ * the VDBOX, the KCR engine (and GSC CS depending on the platform)
+ */
+ struct intel_gt *ctrl_gt;
+
/**
* @pxp_component: i915_pxp_component struct of the bound mei_pxp
* module. Only set and cleared inside component bind/unbind functions,
--
2.34.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [Intel-gfx] [PATCH v5 0/1] drm/i915/pxp: Prepare intel_pxp entry points for MTL
2022-11-29 0:31 [Intel-gfx] [PATCH v5 0/1] drm/i915/pxp: Prepare intel_pxp entry points for MTL Alan Previn
2022-11-29 0:31 ` [Intel-gfx] [PATCH v5 1/1] drm/i915/pxp: Promote pxp subsystem to top-level of i915 Alan Previn
@ 2022-11-29 0:51 ` Teres Alexis, Alan Previn
2022-11-29 0:53 ` [Intel-gfx] ✗ Fi.CI.SPARSE: warning for " Patchwork
` (2 subsequent siblings)
4 siblings, 0 replies; 13+ messages in thread
From: Teres Alexis, Alan Previn @ 2022-11-29 0:51 UTC (permalink / raw)
To: Vivi, Rodrigo, intel-gfx
(++tagging Rodrigo)
This series is a replacement for https://patchwork.freedesktop.org/series/109429/. Patchwork bestowed a new URL as the
series is significantly different now with the new design approach direction from Rodrigo.
...alan
On Mon, 2022-11-28 at 16:31 -0800, Alan Previn wrote:
> MTL has two tiles that is represented by the intel_gt structure in the i915
> code. The PXP feature has a control-structure that currently hangs off the
> intel_gt structure. In MTL, the standalone media tile (i.e. not the root
> tile) contains the VDBOX and KCR engine which are among several assets
> that PXP relies on for establishing and tearing down the PXP session.
>
> However PXP is a global feature as other engines on other tiles can reference
> the PXP session in an object's info within batch buffer instructions. That
> coherrency is handled implicitly by the HW. In fact, for the forseeable future,
> we are expecting this link whereby only one of the tiles will be the control-gt
> for the PXP subsystem.
>
> Keeping the intel_pxp structure within the intel_gt structure makes some
> internal functionalities more straight forward but adds code complexity to
> code readibility and maintainibility to many external-to-pxp subsystems
> which may need to pick the correct intel_gt structure. An example of this
> would be the intel_pxp_is_active or intel_pxp_is_enabled functionality which
> should be viewed as a global level inquiry, not a per-gt inquiry.
>
> That said, this series promotes the intel_pxp structure into the
> drm_i915_private structure making it a top-level subsystem and the PXP
> subsystem will select the control gt internally and keep a pointer to
> it for internal reference.
>
> Changes from prior revs:
> v4: - Instead of maintaining intel_pxp as an intel_gt structure member and
> creating a number of convoluted helpers that takes in i915 as input
> and redirects to the correct intel_gt or takes any intel_gt and internally
> replaces with the correct intel_gt, promote it to be a top-level i915
> structure.
> v3: - Rename gt level helper functions to "intel_pxp_is_enabled/supported/
> active_on_gt" (Daniele)
> - Upgrade _gt_supports_pxp to replace what was intel_gtpxp_is_supported
> as the new intel_pxp_is_supported_on_gt to check for PXP feature
> support vs the tee support for huc authentication. Fix pxp-debugfs-
> registration to use only the former to decide support. (Daniele)
> - Couple minor optimizations.
> v2: - Avoid introduction of new device info or gt variables and use
> existing checks / macros to differentiate the correct GT->PXP
> control ownership (Daniele Ceraolo Spurio)
> - Don't reuse the updated global-checkers for per-GT callers (such
> as other files within PXP) to avoid unnecessary GT-reparsing,
> expose a replacement helper like the prior ones. (Daniele).
> v1: Add one more patch to the series for the intel_pxp suspend/resume
> for similiar refactoring
>
> Alan Previn (1):
> drm/i915/pxp: Promote pxp subsystem to top-level of i915
>
> .../drm/i915/display/skl_universal_plane.c | 2 +-
> drivers/gpu/drm/i915/gem/i915_gem_context.c | 6 +-
> drivers/gpu/drm/i915/gem/i915_gem_create.c | 2 +-
> .../gpu/drm/i915/gem/i915_gem_execbuffer.c | 2 +-
> drivers/gpu/drm/i915/gt/intel_gt.c | 5 --
> drivers/gpu/drm/i915/gt/intel_gt_debugfs.c | 1 -
> drivers/gpu/drm/i915/gt/intel_gt_irq.c | 2 +-
> drivers/gpu/drm/i915/gt/intel_gt_pm.c | 8 ---
> drivers/gpu/drm/i915/gt/intel_gt_types.h | 3 -
> drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c | 2 +-
> drivers/gpu/drm/i915/i915_driver.c | 20 ++++++
> drivers/gpu/drm/i915/i915_drv.h | 3 +
> drivers/gpu/drm/i915/pxp/intel_pxp.c | 72 ++++++++++++++-----
> drivers/gpu/drm/i915/pxp/intel_pxp.h | 6 +-
> drivers/gpu/drm/i915/pxp/intel_pxp_cmd.c | 8 ++-
> drivers/gpu/drm/i915/pxp/intel_pxp_debugfs.c | 41 +++++++----
> drivers/gpu/drm/i915/pxp/intel_pxp_debugfs.h | 4 +-
> drivers/gpu/drm/i915/pxp/intel_pxp_irq.c | 10 ++-
> drivers/gpu/drm/i915/pxp/intel_pxp_pm.c | 4 +-
> drivers/gpu/drm/i915/pxp/intel_pxp_tee.c | 8 ++-
> drivers/gpu/drm/i915/pxp/intel_pxp_types.h | 11 +++
> 21 files changed, 150 insertions(+), 70 deletions(-)
>
>
> base-commit: d21d6474a37e5d43075a24668807ea40a7ee9fc1
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* [Intel-gfx] ✗ Fi.CI.SPARSE: warning for drm/i915/pxp: Prepare intel_pxp entry points for MTL
2022-11-29 0:31 [Intel-gfx] [PATCH v5 0/1] drm/i915/pxp: Prepare intel_pxp entry points for MTL Alan Previn
2022-11-29 0:31 ` [Intel-gfx] [PATCH v5 1/1] drm/i915/pxp: Promote pxp subsystem to top-level of i915 Alan Previn
2022-11-29 0:51 ` [Intel-gfx] [PATCH v5 0/1] drm/i915/pxp: Prepare intel_pxp entry points for MTL Teres Alexis, Alan Previn
@ 2022-11-29 0:53 ` Patchwork
2022-11-29 1:07 ` [Intel-gfx] ✗ Fi.CI.BAT: failure " Patchwork
2022-11-29 21:29 ` [Intel-gfx] [PATCH v5 0/1] " Rodrigo Vivi
4 siblings, 0 replies; 13+ messages in thread
From: Patchwork @ 2022-11-29 0:53 UTC (permalink / raw)
To: Alan Previn; +Cc: intel-gfx
== Series Details ==
Series: drm/i915/pxp: Prepare intel_pxp entry points for MTL
URL : https://patchwork.freedesktop.org/series/111408/
State : warning
== Summary ==
Error: dim sparse failed
Sparse version: v0.6.2
Fast mode used, each commit won't be checked separately.
^ permalink raw reply [flat|nested] 13+ messages in thread
* [Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915/pxp: Prepare intel_pxp entry points for MTL
2022-11-29 0:31 [Intel-gfx] [PATCH v5 0/1] drm/i915/pxp: Prepare intel_pxp entry points for MTL Alan Previn
` (2 preceding siblings ...)
2022-11-29 0:53 ` [Intel-gfx] ✗ Fi.CI.SPARSE: warning for " Patchwork
@ 2022-11-29 1:07 ` Patchwork
2022-11-29 21:29 ` [Intel-gfx] [PATCH v5 0/1] " Rodrigo Vivi
4 siblings, 0 replies; 13+ messages in thread
From: Patchwork @ 2022-11-29 1:07 UTC (permalink / raw)
To: Alan Previn; +Cc: intel-gfx
[-- Attachment #1: Type: text/plain, Size: 22092 bytes --]
== Series Details ==
Series: drm/i915/pxp: Prepare intel_pxp entry points for MTL
URL : https://patchwork.freedesktop.org/series/111408/
State : failure
== Summary ==
CI Bug Log - changes from CI_DRM_12440 -> Patchwork_111408v1
====================================================
Summary
-------
**FAILURE**
Serious unknown changes coming with Patchwork_111408v1 absolutely need to be
verified manually.
If you think the reported changes have nothing to do with the changes
introduced in Patchwork_111408v1, please notify your bug team to allow them
to document this new failure mode, which will reduce false positives in CI.
External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111408v1/index.html
Participating hosts (34 -> 38)
------------------------------
Additional (4): bat-atsm-1 bat-adls-5 fi-tgl-dsi bat-dg1-5
Possible new issues
-------------------
Here are the unknown changes that may have been introduced in Patchwork_111408v1:
### IGT changes ###
#### Possible regressions ####
* igt@core_hotunplug@unbind-rebind:
- fi-bsw-kefka: [PASS][1] -> [INCOMPLETE][2]
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12440/fi-bsw-kefka/igt@core_hotunplug@unbind-rebind.html
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111408v1/fi-bsw-kefka/igt@core_hotunplug@unbind-rebind.html
- fi-adl-ddr5: [PASS][3] -> [INCOMPLETE][4]
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12440/fi-adl-ddr5/igt@core_hotunplug@unbind-rebind.html
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111408v1/fi-adl-ddr5/igt@core_hotunplug@unbind-rebind.html
- fi-cfl-guc: [PASS][5] -> [INCOMPLETE][6]
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12440/fi-cfl-guc/igt@core_hotunplug@unbind-rebind.html
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111408v1/fi-cfl-guc/igt@core_hotunplug@unbind-rebind.html
- fi-cfl-8700k: [PASS][7] -> [INCOMPLETE][8]
[7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12440/fi-cfl-8700k/igt@core_hotunplug@unbind-rebind.html
[8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111408v1/fi-cfl-8700k/igt@core_hotunplug@unbind-rebind.html
- fi-rkl-11600: [PASS][9] -> [INCOMPLETE][10]
[9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12440/fi-rkl-11600/igt@core_hotunplug@unbind-rebind.html
[10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111408v1/fi-rkl-11600/igt@core_hotunplug@unbind-rebind.html
- fi-icl-u2: [PASS][11] -> [INCOMPLETE][12]
[11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12440/fi-icl-u2/igt@core_hotunplug@unbind-rebind.html
[12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111408v1/fi-icl-u2/igt@core_hotunplug@unbind-rebind.html
- bat-dg1-5: NOTRUN -> [INCOMPLETE][13]
[13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111408v1/bat-dg1-5/igt@core_hotunplug@unbind-rebind.html
- fi-bdw-gvtdvm: [PASS][14] -> [INCOMPLETE][15]
[14]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12440/fi-bdw-gvtdvm/igt@core_hotunplug@unbind-rebind.html
[15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111408v1/fi-bdw-gvtdvm/igt@core_hotunplug@unbind-rebind.html
- fi-rkl-guc: [PASS][16] -> [INCOMPLETE][17]
[16]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12440/fi-rkl-guc/igt@core_hotunplug@unbind-rebind.html
[17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111408v1/fi-rkl-guc/igt@core_hotunplug@unbind-rebind.html
- fi-skl-guc: [PASS][18] -> [INCOMPLETE][19]
[18]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12440/fi-skl-guc/igt@core_hotunplug@unbind-rebind.html
[19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111408v1/fi-skl-guc/igt@core_hotunplug@unbind-rebind.html
- bat-dg1-6: [PASS][20] -> [INCOMPLETE][21]
[20]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12440/bat-dg1-6/igt@core_hotunplug@unbind-rebind.html
[21]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111408v1/bat-dg1-6/igt@core_hotunplug@unbind-rebind.html
- fi-skl-6700k2: [PASS][22] -> [INCOMPLETE][23]
[22]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12440/fi-skl-6700k2/igt@core_hotunplug@unbind-rebind.html
[23]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111408v1/fi-skl-6700k2/igt@core_hotunplug@unbind-rebind.html
- fi-cfl-8109u: [PASS][24] -> [INCOMPLETE][25]
[24]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12440/fi-cfl-8109u/igt@core_hotunplug@unbind-rebind.html
[25]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111408v1/fi-cfl-8109u/igt@core_hotunplug@unbind-rebind.html
- bat-adlp-4: [PASS][26] -> [INCOMPLETE][27]
[26]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12440/bat-adlp-4/igt@core_hotunplug@unbind-rebind.html
[27]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111408v1/bat-adlp-4/igt@core_hotunplug@unbind-rebind.html
- fi-ivb-3770: [PASS][28] -> [INCOMPLETE][29]
[28]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12440/fi-ivb-3770/igt@core_hotunplug@unbind-rebind.html
[29]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111408v1/fi-ivb-3770/igt@core_hotunplug@unbind-rebind.html
- fi-elk-e7500: [PASS][30] -> [INCOMPLETE][31]
[30]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12440/fi-elk-e7500/igt@core_hotunplug@unbind-rebind.html
[31]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111408v1/fi-elk-e7500/igt@core_hotunplug@unbind-rebind.html
- fi-ilk-650: [PASS][32] -> [INCOMPLETE][33]
[32]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12440/fi-ilk-650/igt@core_hotunplug@unbind-rebind.html
[33]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111408v1/fi-ilk-650/igt@core_hotunplug@unbind-rebind.html
#### Warnings ####
* igt@core_hotunplug@unbind-rebind:
- fi-blb-e6850: [INCOMPLETE][34] ([i915#7605]) -> [INCOMPLETE][35]
[34]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12440/fi-blb-e6850/igt@core_hotunplug@unbind-rebind.html
[35]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111408v1/fi-blb-e6850/igt@core_hotunplug@unbind-rebind.html
- fi-pnv-d510: [INCOMPLETE][36] ([i915#7605]) -> [INCOMPLETE][37]
[36]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12440/fi-pnv-d510/igt@core_hotunplug@unbind-rebind.html
[37]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111408v1/fi-pnv-d510/igt@core_hotunplug@unbind-rebind.html
#### Suppressed ####
The following results come from untrusted machines, tests, or statuses.
They do not affect the overall result.
* igt@core_hotunplug@unbind-rebind:
- {fi-ehl-2}: [PASS][38] -> [INCOMPLETE][39]
[38]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12440/fi-ehl-2/igt@core_hotunplug@unbind-rebind.html
[39]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111408v1/fi-ehl-2/igt@core_hotunplug@unbind-rebind.html
- {bat-dg2-9}: [PASS][40] -> [INCOMPLETE][41]
[40]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12440/bat-dg2-9/igt@core_hotunplug@unbind-rebind.html
[41]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111408v1/bat-dg2-9/igt@core_hotunplug@unbind-rebind.html
- {bat-rpls-2}: [PASS][42] -> [INCOMPLETE][43]
[42]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12440/bat-rpls-2/igt@core_hotunplug@unbind-rebind.html
[43]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111408v1/bat-rpls-2/igt@core_hotunplug@unbind-rebind.html
- {bat-jsl-1}: [PASS][44] -> [INCOMPLETE][45]
[44]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12440/bat-jsl-1/igt@core_hotunplug@unbind-rebind.html
[45]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111408v1/bat-jsl-1/igt@core_hotunplug@unbind-rebind.html
- {fi-jsl-1}: [PASS][46] -> [INCOMPLETE][47]
[46]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12440/fi-jsl-1/igt@core_hotunplug@unbind-rebind.html
[47]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111408v1/fi-jsl-1/igt@core_hotunplug@unbind-rebind.html
- {bat-rpls-1}: [PASS][48] -> [INCOMPLETE][49]
[48]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12440/bat-rpls-1/igt@core_hotunplug@unbind-rebind.html
[49]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111408v1/bat-rpls-1/igt@core_hotunplug@unbind-rebind.html
- {bat-kbl-2}: [PASS][50] -> [INCOMPLETE][51]
[50]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12440/bat-kbl-2/igt@core_hotunplug@unbind-rebind.html
[51]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111408v1/bat-kbl-2/igt@core_hotunplug@unbind-rebind.html
- {bat-adlp-6}: [PASS][52] -> [INCOMPLETE][53]
[52]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12440/bat-adlp-6/igt@core_hotunplug@unbind-rebind.html
[53]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111408v1/bat-adlp-6/igt@core_hotunplug@unbind-rebind.html
- {bat-adls-5}: NOTRUN -> [INCOMPLETE][54]
[54]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111408v1/bat-adls-5/igt@core_hotunplug@unbind-rebind.html
- {bat-atsm-1}: NOTRUN -> [INCOMPLETE][55]
[55]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111408v1/bat-atsm-1/igt@core_hotunplug@unbind-rebind.html
- {bat-dg1-7}: [PASS][56] -> [INCOMPLETE][57]
[56]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12440/bat-dg1-7/igt@core_hotunplug@unbind-rebind.html
[57]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111408v1/bat-dg1-7/igt@core_hotunplug@unbind-rebind.html
- {bat-jsl-3}: [PASS][58] -> [INCOMPLETE][59]
[58]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12440/bat-jsl-3/igt@core_hotunplug@unbind-rebind.html
[59]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111408v1/bat-jsl-3/igt@core_hotunplug@unbind-rebind.html
- {bat-dg2-11}: [PASS][60] -> [INCOMPLETE][61]
[60]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12440/bat-dg2-11/igt@core_hotunplug@unbind-rebind.html
[61]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111408v1/bat-dg2-11/igt@core_hotunplug@unbind-rebind.html
- {bat-adln-1}: [PASS][62] -> [INCOMPLETE][63]
[62]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12440/bat-adln-1/igt@core_hotunplug@unbind-rebind.html
[63]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111408v1/bat-adln-1/igt@core_hotunplug@unbind-rebind.html
- {fi-tgl-dsi}: NOTRUN -> [INCOMPLETE][64]
[64]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111408v1/fi-tgl-dsi/igt@core_hotunplug@unbind-rebind.html
- {bat-dg2-8}: [PASS][65] -> [INCOMPLETE][66]
[65]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12440/bat-dg2-8/igt@core_hotunplug@unbind-rebind.html
[66]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111408v1/bat-dg2-8/igt@core_hotunplug@unbind-rebind.html
- {bat-adlm-1}: [PASS][67] -> [INCOMPLETE][68]
[67]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12440/bat-adlm-1/igt@core_hotunplug@unbind-rebind.html
[68]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111408v1/bat-adlm-1/igt@core_hotunplug@unbind-rebind.html
- {bat-rplp-1}: [PASS][69] -> [INCOMPLETE][70]
[69]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12440/bat-rplp-1/igt@core_hotunplug@unbind-rebind.html
[70]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111408v1/bat-rplp-1/igt@core_hotunplug@unbind-rebind.html
Known issues
------------
Here are the changes found in Patchwork_111408v1 that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@core_hotunplug@unbind-rebind:
- fi-apl-guc: [PASS][71] -> [INCOMPLETE][72] ([i915#7073])
[71]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12440/fi-apl-guc/igt@core_hotunplug@unbind-rebind.html
[72]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111408v1/fi-apl-guc/igt@core_hotunplug@unbind-rebind.html
* igt@gem_mmap@basic:
- bat-dg1-5: NOTRUN -> [SKIP][73] ([i915#4083])
[73]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111408v1/bat-dg1-5/igt@gem_mmap@basic.html
* igt@gem_tiled_fence_blits@basic:
- bat-dg1-5: NOTRUN -> [SKIP][74] ([i915#4077]) +2 similar issues
[74]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111408v1/bat-dg1-5/igt@gem_tiled_fence_blits@basic.html
* igt@gem_tiled_pread_basic:
- bat-dg1-5: NOTRUN -> [SKIP][75] ([i915#4079]) +1 similar issue
[75]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111408v1/bat-dg1-5/igt@gem_tiled_pread_basic.html
* igt@i915_pm_backlight@basic-brightness:
- bat-dg1-5: NOTRUN -> [SKIP][76] ([i915#7561])
[76]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111408v1/bat-dg1-5/igt@i915_pm_backlight@basic-brightness.html
* igt@i915_pm_rps@basic-api:
- bat-dg1-5: NOTRUN -> [SKIP][77] ([i915#6621])
[77]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111408v1/bat-dg1-5/igt@i915_pm_rps@basic-api.html
* igt@kms_addfb_basic@basic-x-tiled-legacy:
- bat-dg1-5: NOTRUN -> [SKIP][78] ([i915#4212]) +7 similar issues
[78]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111408v1/bat-dg1-5/igt@kms_addfb_basic@basic-x-tiled-legacy.html
* igt@kms_addfb_basic@basic-y-tiled-legacy:
- bat-dg1-5: NOTRUN -> [SKIP][79] ([i915#4215])
[79]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111408v1/bat-dg1-5/igt@kms_addfb_basic@basic-y-tiled-legacy.html
* igt@kms_chamelium@hdmi-hpd-fast:
- bat-dg1-5: NOTRUN -> [SKIP][80] ([fdo#111827]) +7 similar issues
[80]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111408v1/bat-dg1-5/igt@kms_chamelium@hdmi-hpd-fast.html
* igt@kms_cursor_legacy@basic-busy-flip-before-cursor:
- bat-dg1-5: NOTRUN -> [SKIP][81] ([i915#4103] / [i915#4213])
[81]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111408v1/bat-dg1-5/igt@kms_cursor_legacy@basic-busy-flip-before-cursor.html
* igt@kms_force_connector_basic@force-load-detect:
- bat-dg1-5: NOTRUN -> [SKIP][82] ([fdo#109285])
[82]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111408v1/bat-dg1-5/igt@kms_force_connector_basic@force-load-detect.html
* igt@kms_psr@sprite_plane_onoff:
- bat-dg1-5: NOTRUN -> [SKIP][83] ([i915#1072] / [i915#4078]) +3 similar issues
[83]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111408v1/bat-dg1-5/igt@kms_psr@sprite_plane_onoff.html
* igt@kms_setmode@basic-clone-single-crtc:
- bat-dg1-5: NOTRUN -> [SKIP][84] ([i915#3555])
[84]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111408v1/bat-dg1-5/igt@kms_setmode@basic-clone-single-crtc.html
* igt@prime_vgem@basic-fence-read:
- bat-dg1-5: NOTRUN -> [SKIP][85] ([i915#3708]) +3 similar issues
[85]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111408v1/bat-dg1-5/igt@prime_vgem@basic-fence-read.html
* igt@prime_vgem@basic-gtt:
- bat-dg1-5: NOTRUN -> [SKIP][86] ([i915#3708] / [i915#4077]) +1 similar issue
[86]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111408v1/bat-dg1-5/igt@prime_vgem@basic-gtt.html
* igt@prime_vgem@basic-userptr:
- bat-dg1-5: NOTRUN -> [SKIP][87] ([i915#3708] / [i915#4873])
[87]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111408v1/bat-dg1-5/igt@prime_vgem@basic-userptr.html
* igt@runner@aborted:
- fi-skl-6700k2: NOTRUN -> [FAIL][88] ([i915#4312])
[88]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111408v1/fi-skl-6700k2/igt@runner@aborted.html
- fi-cfl-8109u: NOTRUN -> [FAIL][89] ([i915#4312])
[89]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111408v1/fi-cfl-8109u/igt@runner@aborted.html
- bat-adlp-4: NOTRUN -> [FAIL][90] ([i915#4312])
[90]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111408v1/bat-adlp-4/igt@runner@aborted.html
- fi-ivb-3770: NOTRUN -> [FAIL][91] ([i915#4312])
[91]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111408v1/fi-ivb-3770/igt@runner@aborted.html
- fi-elk-e7500: NOTRUN -> [FAIL][92] ([i915#4312])
[92]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111408v1/fi-elk-e7500/igt@runner@aborted.html
- fi-ilk-650: NOTRUN -> [FAIL][93] ([i915#4312])
[93]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111408v1/fi-ilk-650/igt@runner@aborted.html
- fi-icl-u2: NOTRUN -> [FAIL][94] ([i915#4312])
[94]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111408v1/fi-icl-u2/igt@runner@aborted.html
- fi-apl-guc: NOTRUN -> [FAIL][95] ([i915#4312])
[95]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111408v1/fi-apl-guc/igt@runner@aborted.html
- bat-dg1-5: NOTRUN -> [FAIL][96] ([i915#4312])
[96]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111408v1/bat-dg1-5/igt@runner@aborted.html
- fi-rkl-guc: NOTRUN -> [FAIL][97] ([i915#4312])
[97]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111408v1/fi-rkl-guc/igt@runner@aborted.html
- fi-skl-guc: NOTRUN -> [FAIL][98] ([i915#4312])
[98]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111408v1/fi-skl-guc/igt@runner@aborted.html
- bat-dg1-6: NOTRUN -> [FAIL][99] ([i915#4312])
[99]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111408v1/bat-dg1-6/igt@runner@aborted.html
- fi-cfl-8700k: NOTRUN -> [FAIL][100] ([i915#4312])
[100]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111408v1/fi-cfl-8700k/igt@runner@aborted.html
- fi-rkl-11600: NOTRUN -> [FAIL][101] ([i915#4312])
[101]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111408v1/fi-rkl-11600/igt@runner@aborted.html
- fi-bdw-gvtdvm: NOTRUN -> [FAIL][102] ([i915#4312])
[102]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111408v1/fi-bdw-gvtdvm/igt@runner@aborted.html
- fi-bsw-kefka: NOTRUN -> [FAIL][103] ([i915#4312])
[103]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111408v1/fi-bsw-kefka/igt@runner@aborted.html
- fi-adl-ddr5: NOTRUN -> [FAIL][104] ([i915#4312])
[104]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111408v1/fi-adl-ddr5/igt@runner@aborted.html
- fi-cfl-guc: NOTRUN -> [FAIL][105] ([i915#4312])
[105]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111408v1/fi-cfl-guc/igt@runner@aborted.html
#### Possible fixes ####
* igt@gem_tiled_blits@basic:
- fi-pnv-d510: [SKIP][106] ([fdo#109271]) -> [PASS][107] +1 similar issue
[106]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12440/fi-pnv-d510/igt@gem_tiled_blits@basic.html
[107]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111408v1/fi-pnv-d510/igt@gem_tiled_blits@basic.html
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
[fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
[fdo#109284]: https://bugs.freedesktop.org/show_bug.cgi?id=109284
[fdo#109285]: https://bugs.freedesktop.org/show_bug.cgi?id=109285
[fdo#109295]: https://bugs.freedesktop.org/show_bug.cgi?id=109295
[fdo#110189]: https://bugs.freedesktop.org/show_bug.cgi?id=110189
[fdo#111827]: https://bugs.freedesktop.org/show_bug.cgi?id=111827
[i915#1072]: https://gitlab.freedesktop.org/drm/intel/issues/1072
[i915#1836]: https://gitlab.freedesktop.org/drm/intel/issues/1836
[i915#2190]: https://gitlab.freedesktop.org/drm/intel/issues/2190
[i915#2582]: https://gitlab.freedesktop.org/drm/intel/issues/2582
[i915#3282]: https://gitlab.freedesktop.org/drm/intel/issues/3282
[i915#3291]: https://gitlab.freedesktop.org/drm/intel/issues/3291
[i915#3301]: https://gitlab.freedesktop.org/drm/intel/issues/3301
[i915#3555]: https://gitlab.freedesktop.org/drm/intel/issues/3555
[i915#3708]: https://gitlab.freedesktop.org/drm/intel/issues/3708
[i915#4077]: https://gitlab.freedesktop.org/drm/intel/issues/4077
[i915#4078]: https://gitlab.freedesktop.org/drm/intel/issues/4078
[i915#4079]: https://gitlab.freedesktop.org/drm/intel/issues/4079
[i915#4083]: https://gitlab.freedesktop.org/drm/intel/issues/4083
[i915#4103]: https://gitlab.freedesktop.org/drm/intel/issues/4103
[i915#4212]: https://gitlab.freedesktop.org/drm/intel/issues/4212
[i915#4213]: https://gitlab.freedesktop.org/drm/intel/issues/4213
[i915#4215]: https://gitlab.freedesktop.org/drm/intel/issues/4215
[i915#4312]: https://gitlab.freedesktop.org/drm/intel/issues/4312
[i915#4873]: https://gitlab.freedesktop.org/drm/intel/issues/4873
[i915#6077]: https://gitlab.freedesktop.org/drm/intel/issues/6077
[i915#6078]: https://gitlab.freedesktop.org/drm/intel/issues/6078
[i915#6093]: https://gitlab.freedesktop.org/drm/intel/issues/6093
[i915#6094]: https://gitlab.freedesktop.org/drm/intel/issues/6094
[i915#6166]: https://gitlab.freedesktop.org/drm/intel/issues/6166
[i915#6621]: https://gitlab.freedesktop.org/drm/intel/issues/6621
[i915#7073]: https://gitlab.freedesktop.org/drm/intel/issues/7073
[i915#7357]: https://gitlab.freedesktop.org/drm/intel/issues/7357
[i915#7456]: https://gitlab.freedesktop.org/drm/intel/issues/7456
[i915#7561]: https://gitlab.freedesktop.org/drm/intel/issues/7561
[i915#7605]: https://gitlab.freedesktop.org/drm/intel/issues/7605
Build changes
-------------
* Linux: CI_DRM_12440 -> Patchwork_111408v1
CI-20190529: 20190529
CI_DRM_12440: d21d6474a37e5d43075a24668807ea40a7ee9fc1 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_7073: d021d66e389f4a759dc749b5f74f278ecd2e6cbf @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
Patchwork_111408v1: d21d6474a37e5d43075a24668807ea40a7ee9fc1 @ git://anongit.freedesktop.org/gfx-ci/linux
### Linux commits
363500e70ead drm/i915/pxp: Promote pxp subsystem to top-level of i915
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111408v1/index.html
[-- Attachment #2: Type: text/html, Size: 24254 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Intel-gfx] [PATCH v5 1/1] drm/i915/pxp: Promote pxp subsystem to top-level of i915
2022-11-29 0:31 ` [Intel-gfx] [PATCH v5 1/1] drm/i915/pxp: Promote pxp subsystem to top-level of i915 Alan Previn
@ 2022-11-29 18:35 ` Teres Alexis, Alan Previn
2022-11-29 21:28 ` Rodrigo Vivi
1 sibling, 0 replies; 13+ messages in thread
From: Teres Alexis, Alan Previn @ 2022-11-29 18:35 UTC (permalink / raw)
To: intel-gfx
On Mon, 2022-11-28 at 16:31 -0800, Alan Previn wrote:
> Starting with MTL, there will be two GT-tiles, a render and media
> tile. PXP as a service for supporting workloads with protected
> contexts and protected buffers can be subscribed by process
>
>
Alan: [snip]
> diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c
> index 69103ae37779..3be3c53437e9 100644
> --- a/drivers/gpu/drm/i915/i915_driver.c
> +++ b/drivers/gpu/drm/i915/i915_driver.c
> @@ -73,6 +73,8 @@
> #include "gt/intel_gt_pm.h"
> #include "gt/intel_rc6.h"
>
> +#include "pxp/intel_pxp.h"
> +#include "pxp/intel_pxp_debugfs.h"
> #include "pxp/intel_pxp_pm.h"
>
> #include "i915_file_private.h"
> @@ -763,6 +765,8 @@ static void i915_driver_register(struct drm_i915_private *dev_priv)
> for_each_gt(gt, dev_priv, i)
> intel_gt_driver_register(gt);
>
> + intel_pxp_debugfs_register(dev_priv->pxp);
> +
> i915_hwmon_register(dev_priv);
>
> intel_display_driver_register(dev_priv);
> @@ -794,6 +798,8 @@ static void i915_driver_unregister(struct drm_i915_private *dev_priv)
>
> intel_display_driver_unregister(dev_priv);
>
> + (&dev_priv->pxp);
> +
> for_each_gt(gt, dev_priv, i)
> intel_gt_driver_unregister(gt);
>
> @@ -937,6 +943,8 @@ int i915_driver_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> if (ret)
> goto out_cleanup_modeset2;
>
> + intel_pxp_init(&i915->pxp);
> +
> ret = intel_modeset_init(i915);
> if (ret)
> goto out_cleanup_gem;
> @@ -996,6 +1004,8 @@ void i915_driver_remove(struct drm_i915_private *i915)
> /* Flush any external code that still may be under the RCU lock */
> synchronize_rcu();
>
> + intel_pxp_fini(&i915->pxp);
> +
>
>
This is a bug that is causing the regression on BAT - will post a new rev ASAP.
(intel_pxp_fini should only be called during unregister, not again during remove).
Alan:[snip]
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Intel-gfx] [PATCH v5 1/1] drm/i915/pxp: Promote pxp subsystem to top-level of i915
2022-11-29 0:31 ` [Intel-gfx] [PATCH v5 1/1] drm/i915/pxp: Promote pxp subsystem to top-level of i915 Alan Previn
2022-11-29 18:35 ` Teres Alexis, Alan Previn
@ 2022-11-29 21:28 ` Rodrigo Vivi
2022-11-30 0:10 ` Teres Alexis, Alan Previn
2022-12-01 23:45 ` Teres Alexis, Alan Previn
1 sibling, 2 replies; 13+ messages in thread
From: Rodrigo Vivi @ 2022-11-29 21:28 UTC (permalink / raw)
To: Alan Previn; +Cc: intel-gfx
On Mon, Nov 28, 2022 at 04:31:52PM -0800, Alan Previn wrote:
> Starting with MTL, there will be two GT-tiles, a render and media
> tile. PXP as a service for supporting workloads with protected
> contexts and protected buffers can be subscribed by process
> workloads on any tile. However, depending on the platform,
> only one of the tiles is used for control events pertaining to PXP
> operation (such as creating the arbitration session and session
> tear-down). In the case of MTL, this is the media-tile.
Imho this patch shows that having the pxp under i915 instead of gt
is the right way to go.
but I have a few comments and doubts below...
>
> Signed-off-by: Alan Previn <alan.previn.teres.alexis@intel.com>
> ---
> .../drm/i915/display/skl_universal_plane.c | 2 +-
> drivers/gpu/drm/i915/gem/i915_gem_context.c | 6 +-
> drivers/gpu/drm/i915/gem/i915_gem_create.c | 2 +-
> .../gpu/drm/i915/gem/i915_gem_execbuffer.c | 2 +-
> drivers/gpu/drm/i915/gt/intel_gt.c | 5 --
> drivers/gpu/drm/i915/gt/intel_gt_debugfs.c | 1 -
> drivers/gpu/drm/i915/gt/intel_gt_irq.c | 2 +-
> drivers/gpu/drm/i915/gt/intel_gt_pm.c | 8 ---
> drivers/gpu/drm/i915/gt/intel_gt_types.h | 3 -
> drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c | 2 +-
> drivers/gpu/drm/i915/i915_driver.c | 20 ++++++
> drivers/gpu/drm/i915/i915_drv.h | 3 +
> drivers/gpu/drm/i915/pxp/intel_pxp.c | 72 ++++++++++++++-----
> drivers/gpu/drm/i915/pxp/intel_pxp.h | 6 +-
> drivers/gpu/drm/i915/pxp/intel_pxp_cmd.c | 8 ++-
> drivers/gpu/drm/i915/pxp/intel_pxp_debugfs.c | 41 +++++++----
> drivers/gpu/drm/i915/pxp/intel_pxp_debugfs.h | 4 +-
> drivers/gpu/drm/i915/pxp/intel_pxp_irq.c | 10 ++-
> drivers/gpu/drm/i915/pxp/intel_pxp_pm.c | 4 +-
> drivers/gpu/drm/i915/pxp/intel_pxp_tee.c | 8 ++-
> drivers/gpu/drm/i915/pxp/intel_pxp_types.h | 11 +++
> 21 files changed, 150 insertions(+), 70 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c b/drivers/gpu/drm/i915/display/skl_universal_plane.c
> index 76490cc59d8f..4b79c2d2d617 100644
> --- a/drivers/gpu/drm/i915/display/skl_universal_plane.c
> +++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c
> @@ -1848,7 +1848,7 @@ static bool bo_has_valid_encryption(struct drm_i915_gem_object *obj)
> {
> struct drm_i915_private *i915 = to_i915(obj->base.dev);
>
> - return intel_pxp_key_check(&to_gt(i915)->pxp, obj, false) == 0;
> + return intel_pxp_key_check(i915->pxp, obj, false) == 0;
> }
>
> static bool pxp_is_borked(struct drm_i915_gem_object *obj)
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c
> index 7f2831efc798..46e71f62fcec 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
> @@ -257,7 +257,7 @@ static int proto_context_set_protected(struct drm_i915_private *i915,
>
> if (!protected) {
> pc->uses_protected_content = false;
> - } else if (!intel_pxp_is_enabled(&to_gt(i915)->pxp)) {
> + } else if (!intel_pxp_is_enabled(i915->pxp)) {
> ret = -ENODEV;
> } else if ((pc->user_flags & BIT(UCONTEXT_RECOVERABLE)) ||
> !(pc->user_flags & BIT(UCONTEXT_BANNABLE))) {
> @@ -271,8 +271,8 @@ static int proto_context_set_protected(struct drm_i915_private *i915,
> */
> pc->pxp_wakeref = intel_runtime_pm_get(&i915->runtime_pm);
>
> - if (!intel_pxp_is_active(&to_gt(i915)->pxp))
> - ret = intel_pxp_start(&to_gt(i915)->pxp);
> + if (!intel_pxp_is_active(i915->pxp))
> + ret = intel_pxp_start(i915->pxp);
> }
>
> return ret;
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_create.c b/drivers/gpu/drm/i915/gem/i915_gem_create.c
> index 33673fe7ee0a..005a7f842784 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_create.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_create.c
> @@ -384,7 +384,7 @@ static int ext_set_protected(struct i915_user_extension __user *base, void *data
> if (ext.flags)
> return -EINVAL;
>
> - if (!intel_pxp_is_enabled(&to_gt(ext_data->i915)->pxp))
> + if (!intel_pxp_is_enabled(ext_data->i915->pxp))
> return -ENODEV;
>
> ext_data->flags |= I915_BO_PROTECTED;
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
> index 29e9e8d5b6fe..ed74d173a092 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
> @@ -869,7 +869,7 @@ static struct i915_vma *eb_lookup_vma(struct i915_execbuffer *eb, u32 handle)
> */
> if (i915_gem_context_uses_protected_content(eb->gem_context) &&
> i915_gem_object_is_protected(obj)) {
> - err = intel_pxp_key_check(&vm->gt->pxp, obj, true);
> + err = intel_pxp_key_check(vm->gt->i915->pxp, obj, true);
> if (err) {
> i915_gem_object_put(obj);
> return ERR_PTR(err);
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c b/drivers/gpu/drm/i915/gt/intel_gt.c
> index 7ef0edb2e37c..1fd6b34ecda3 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt.c
> +++ b/drivers/gpu/drm/i915/gt/intel_gt.c
> @@ -8,7 +8,6 @@
>
> #include "gem/i915_gem_internal.h"
> #include "gem/i915_gem_lmem.h"
> -#include "pxp/intel_pxp.h"
>
> #include "i915_drv.h"
> #include "i915_perf_oa_regs.h"
> @@ -753,8 +752,6 @@ int intel_gt_init(struct intel_gt *gt)
>
> intel_migrate_init(>->migrate, gt);
>
> - intel_pxp_init(>->pxp);
> -
> goto out_fw;
> err_gt:
> __intel_gt_disable(gt);
> @@ -794,8 +791,6 @@ void intel_gt_driver_unregister(struct intel_gt *gt)
> intel_rps_driver_unregister(>->rps);
> intel_gsc_fini(>->gsc);
>
> - intel_pxp_fini(>->pxp);
> -
> /*
> * Upon unregistering the device to prevent any new users, cancel
> * all in-flight requests so that we can quickly unbind the active
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_debugfs.c b/drivers/gpu/drm/i915/gt/intel_gt_debugfs.c
> index dd53641f3637..7876f4c3d0f1 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt_debugfs.c
> +++ b/drivers/gpu/drm/i915/gt/intel_gt_debugfs.c
> @@ -99,7 +99,6 @@ void intel_gt_debugfs_register(struct intel_gt *gt)
> intel_sseu_debugfs_register(gt, root);
>
> intel_uc_debugfs_register(>->uc, root);
> - intel_pxp_debugfs_register(>->pxp, root);
> }
>
> void intel_gt_debugfs_register_files(struct dentry *root,
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_irq.c b/drivers/gpu/drm/i915/gt/intel_gt_irq.c
> index 6f6b9e04d916..8fac2660e16b 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt_irq.c
> +++ b/drivers/gpu/drm/i915/gt/intel_gt_irq.c
> @@ -76,7 +76,7 @@ gen11_other_irq_handler(struct intel_gt *gt, const u8 instance,
> return gen11_rps_irq_handler(&media_gt->rps, iir);
>
> if (instance == OTHER_KCR_INSTANCE)
> - return intel_pxp_irq_handler(>->pxp, iir);
> + return intel_pxp_irq_handler(gt->i915->pxp, iir);
>
> if (instance == OTHER_GSC_INSTANCE)
> return intel_gsc_irq_handler(gt, iir);
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_pm.c b/drivers/gpu/drm/i915/gt/intel_gt_pm.c
> index 16db85fab0b1..c065950d0bad 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt_pm.c
> +++ b/drivers/gpu/drm/i915/gt/intel_gt_pm.c
> @@ -304,8 +304,6 @@ int intel_gt_resume(struct intel_gt *gt)
>
> intel_uc_resume(>->uc);
>
> - intel_pxp_resume(>->pxp);
> -
> user_forcewake(gt, false);
>
> out_fw:
> @@ -339,8 +337,6 @@ void intel_gt_suspend_prepare(struct intel_gt *gt)
> {
> user_forcewake(gt, true);
> wait_for_suspend(gt);
> -
> - intel_pxp_suspend_prepare(>->pxp);
> }
>
> static suspend_state_t pm_suspend_target(void)
> @@ -365,7 +361,6 @@ void intel_gt_suspend_late(struct intel_gt *gt)
> GEM_BUG_ON(gt->awake);
>
> intel_uc_suspend(>->uc);
> - intel_pxp_suspend(>->pxp);
>
> /*
> * On disabling the device, we want to turn off HW access to memory
> @@ -393,7 +388,6 @@ void intel_gt_suspend_late(struct intel_gt *gt)
>
> void intel_gt_runtime_suspend(struct intel_gt *gt)
> {
> - intel_pxp_runtime_suspend(>->pxp);
> intel_uc_runtime_suspend(>->uc);
>
> GT_TRACE(gt, "\n");
> @@ -411,8 +405,6 @@ int intel_gt_runtime_resume(struct intel_gt *gt)
> if (ret)
> return ret;
>
> - intel_pxp_runtime_resume(>->pxp);
> -
> return 0;
> }
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_types.h b/drivers/gpu/drm/i915/gt/intel_gt_types.h
> index c1d9cd255e06..07ebeffbc8bd 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt_types.h
> +++ b/drivers/gpu/drm/i915/gt/intel_gt_types.h
> @@ -30,7 +30,6 @@
> #include "intel_rps_types.h"
> #include "intel_migrate_types.h"
> #include "intel_wakeref.h"
> -#include "pxp/intel_pxp_types.h"
> #include "intel_wopcm.h"
>
> struct drm_i915_private;
> @@ -267,8 +266,6 @@ struct intel_gt {
> u8 wb_index; /* Only used on HAS_L3_CCS_READ() platforms */
> } mocs;
>
> - struct intel_pxp pxp;
> -
> /* gt/gtN sysfs */
> struct kobject sysfs_gt;
>
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c b/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c
> index 4f246416db17..534b0aa43316 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c
> @@ -32,7 +32,7 @@ int intel_huc_fw_load_and_auth_via_gsc(struct intel_huc *huc)
>
> GEM_WARN_ON(intel_uc_fw_is_loaded(&huc->fw));
>
> - ret = intel_pxp_huc_load_and_auth(&huc_to_gt(huc)->pxp);
> + ret = intel_pxp_huc_load_and_auth(huc_to_gt(huc)->i915->pxp);
> if (ret)
> return ret;
>
> diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c
> index 69103ae37779..3be3c53437e9 100644
> --- a/drivers/gpu/drm/i915/i915_driver.c
> +++ b/drivers/gpu/drm/i915/i915_driver.c
> @@ -73,6 +73,8 @@
> #include "gt/intel_gt_pm.h"
> #include "gt/intel_rc6.h"
>
> +#include "pxp/intel_pxp.h"
> +#include "pxp/intel_pxp_debugfs.h"
> #include "pxp/intel_pxp_pm.h"
>
> #include "i915_file_private.h"
> @@ -763,6 +765,8 @@ static void i915_driver_register(struct drm_i915_private *dev_priv)
> for_each_gt(gt, dev_priv, i)
> intel_gt_driver_register(gt);
>
> + intel_pxp_debugfs_register(dev_priv->pxp);
> +
> i915_hwmon_register(dev_priv);
>
> intel_display_driver_register(dev_priv);
> @@ -794,6 +798,8 @@ static void i915_driver_unregister(struct drm_i915_private *dev_priv)
>
> intel_display_driver_unregister(dev_priv);
>
> + intel_pxp_fini(&dev_priv->pxp);
> +
> for_each_gt(gt, dev_priv, i)
> intel_gt_driver_unregister(gt);
>
> @@ -937,6 +943,8 @@ int i915_driver_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> if (ret)
> goto out_cleanup_modeset2;
>
> + intel_pxp_init(&i915->pxp);
> +
> ret = intel_modeset_init(i915);
> if (ret)
> goto out_cleanup_gem;
> @@ -996,6 +1004,8 @@ void i915_driver_remove(struct drm_i915_private *i915)
> /* Flush any external code that still may be under the RCU lock */
> synchronize_rcu();
>
> + intel_pxp_fini(&i915->pxp);
> +
> i915_gem_suspend(i915);
>
> intel_gvt_driver_remove(i915);
> @@ -1172,6 +1182,8 @@ static int i915_drm_prepare(struct drm_device *dev)
> {
> struct drm_i915_private *i915 = to_i915(dev);
>
> + intel_pxp_suspend_prepare(i915->pxp);
> +
> /*
> * NB intel_display_suspend() may issue new requests after we've
> * ostensibly marked the GPU as ready-to-sleep here. We need to
> @@ -1252,6 +1264,8 @@ static int i915_drm_suspend_late(struct drm_device *dev, bool hibernation)
>
> disable_rpm_wakeref_asserts(rpm);
>
> + intel_pxp_suspend(dev_priv->pxp);
> +
> i915_gem_suspend_late(dev_priv);
>
> for_each_gt(gt, dev_priv, i)
> @@ -1358,6 +1372,8 @@ static int i915_drm_resume(struct drm_device *dev)
>
> i915_gem_resume(dev_priv);
>
> + intel_pxp_resume(dev_priv->pxp);
> +
> intel_modeset_init_hw(dev_priv);
> intel_init_clock_gating(dev_priv);
> intel_hpd_init(dev_priv);
> @@ -1641,6 +1657,8 @@ static int intel_runtime_suspend(struct device *kdev)
> */
> i915_gem_runtime_suspend(dev_priv);
>
> + intel_pxp_runtime_suspend(dev_priv->pxp);
> +
> for_each_gt(gt, dev_priv, i)
> intel_gt_runtime_suspend(gt);
>
> @@ -1745,6 +1763,8 @@ static int intel_runtime_resume(struct device *kdev)
> for_each_gt(gt, dev_priv, i)
> intel_gt_runtime_resume(gt);
>
> + intel_pxp_runtime_resume(dev_priv->pxp);
> +
> /*
> * On VLV/CHV display interrupts are part of the display
> * power well, so hpd is reinitialized from there. For
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index a380db36d52c..679fa8a424d5 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -72,6 +72,7 @@ struct intel_encoder;
> struct intel_limit;
> struct intel_overlay_error_state;
> struct vlv_s0ix_state;
> +struct intel_pxp;
>
> #define I915_GEM_GPU_DOMAINS \
> (I915_GEM_DOMAIN_RENDER | \
> @@ -364,6 +365,8 @@ struct drm_i915_private {
> struct file *mmap_singleton;
> } gem;
>
> + struct intel_pxp *pxp;
> +
> u8 pch_ssc_use;
>
> /* For i915gm/i945gm vblank irq workaround */
> diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp.c b/drivers/gpu/drm/i915/pxp/intel_pxp.c
> index 5efe61f67546..fbef1f3be591 100644
> --- a/drivers/gpu/drm/i915/pxp/intel_pxp.c
> +++ b/drivers/gpu/drm/i915/pxp/intel_pxp.c
> @@ -3,13 +3,19 @@
> * Copyright(c) 2020 Intel Corporation.
> */
> #include <linux/workqueue.h>
> +
> +#include "gem/i915_gem_context.h"
> +
> +#include "gt/intel_context.h"
> +#include "gt/intel_gt.h"
> +
> +#include "i915_drv.h"
> +
> #include "intel_pxp.h"
> #include "intel_pxp_irq.h"
> #include "intel_pxp_session.h"
> #include "intel_pxp_tee.h"
> -#include "gem/i915_gem_context.h"
> -#include "gt/intel_context.h"
> -#include "i915_drv.h"
> +#include "intel_pxp_types.h"
>
> /**
> * DOC: PXP
> @@ -39,9 +45,9 @@
> * performed via the mei_pxp component module.
> */
>
> -struct intel_gt *pxp_to_gt(const struct intel_pxp *pxp)
> +struct intel_gt *pxp_to_gt(struct intel_pxp *pxp)
> {
> - return container_of(pxp, struct intel_gt, pxp);
> + return pxp->ctrl_gt;
> }
>
> bool intel_pxp_is_enabled(const struct intel_pxp *pxp)
> @@ -138,31 +144,63 @@ static void pxp_init_full(struct intel_pxp *pxp)
> destroy_vcs_context(pxp);
> }
>
> -void intel_pxp_init(struct intel_pxp *pxp)
> +static struct intel_gt *pxp_get_kcr_owner_gt(struct drm_i915_private *i915)
pxp_get_ctrl_gt or pxp_get_serving_gt sounds better in my opinion...
what's "owner"?
> {
> - struct intel_gt *gt = pxp_to_gt(pxp);
> + struct intel_gt *gt = NULL;
> + int i = 0;
> +
> + for_each_gt(gt, i915, i) {
> + /* There can be only one GT that supports PXP */
> + if (HAS_ENGINE(gt, GSC0))
> + return gt;
> + }
>
> /* we rely on the mei PXP module */
> - if (!IS_ENABLED(CONFIG_INTEL_MEI_PXP))
> - return;
> + if (IS_ENABLED(CONFIG_INTEL_MEI_PXP))
> + return &i915->gt0;
> +
> + return NULL;
> +}
> +
> +int intel_pxp_init(struct intel_pxp **pxp_store_ptr)
Please let's avoid the ** here and everywhere.
> +{
> + struct intel_pxp *newpxp;
> +
> + newpxp = kzalloc(sizeof(*newpxp), GFP_KERNEL);
> + if (!newpxp)
> + return -ENOMEM;
> +
> + *pxp_store_ptr = newpxp;
> +
> + newpxp->i915 = container_of(pxp_store_ptr, struct drm_i915_private, pxp);
> + newpxp->ctrl_gt = pxp_get_kcr_owner_gt(newpxp->i915);
> +
> + if (!newpxp->ctrl_gt)
> + return -ENODEV;
>
> /*
> * If HuC is loaded by GSC but PXP is disabled, we can skip the init of
> * the full PXP session/object management and just init the tee channel.
> */
> - if (HAS_PXP(gt->i915))
> - pxp_init_full(pxp);
> - else if (intel_huc_is_loaded_by_gsc(>->uc.huc) && intel_uc_uses_huc(>->uc))
> - intel_pxp_tee_component_init(pxp);
> + if (HAS_PXP(newpxp->i915))
> + pxp_init_full(newpxp);
> + else if (intel_huc_is_loaded_by_gsc(&newpxp->ctrl_gt->uc.huc) &&
> + intel_uc_uses_huc(&newpxp->ctrl_gt->uc))
> + intel_pxp_tee_component_init(newpxp);
> +
> + return 0;
> }
>
> -void intel_pxp_fini(struct intel_pxp *pxp)
> +void intel_pxp_fini(struct intel_pxp **pxp)
> {
> - pxp->arb_is_valid = false;
> + (*pxp)->arb_is_valid = false;
>
> - intel_pxp_tee_component_fini(pxp);
> + intel_pxp_tee_component_fini(*pxp);
>
> - destroy_vcs_context(pxp);
> + destroy_vcs_context(*pxp);
> +
> + kfree(*pxp);
> + pxp = NULL;
> }
>
> void intel_pxp_mark_termination_in_progress(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 2da309088c6d..424e52bbd4f9 100644
> --- a/drivers/gpu/drm/i915/pxp/intel_pxp.h
> +++ b/drivers/gpu/drm/i915/pxp/intel_pxp.h
> @@ -12,12 +12,12 @@
> struct intel_pxp;
> struct drm_i915_gem_object;
>
> -struct intel_gt *pxp_to_gt(const struct intel_pxp *pxp);
> +struct intel_gt *pxp_to_gt(struct intel_pxp *pxp);
> bool intel_pxp_is_enabled(const struct intel_pxp *pxp);
> bool intel_pxp_is_active(const struct intel_pxp *pxp);
>
> -void intel_pxp_init(struct intel_pxp *pxp);
> -void intel_pxp_fini(struct intel_pxp *pxp);
> +int intel_pxp_init(struct intel_pxp **pxp);
> +void intel_pxp_fini(struct intel_pxp **pxp);
>
> void intel_pxp_init_hw(struct intel_pxp *pxp);
> void intel_pxp_fini_hw(struct intel_pxp *pxp);
> diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_cmd.c b/drivers/gpu/drm/i915/pxp/intel_pxp_cmd.c
> index f41e45763d0d..0eee51c4a772 100644
> --- a/drivers/gpu/drm/i915/pxp/intel_pxp_cmd.c
> +++ b/drivers/gpu/drm/i915/pxp/intel_pxp_cmd.c
> @@ -3,9 +3,6 @@
> * Copyright(c) 2020, Intel Corporation. All rights reserved.
> */
>
> -#include "intel_pxp.h"
> -#include "intel_pxp_cmd.h"
> -#include "intel_pxp_session.h"
> #include "gt/intel_context.h"
> #include "gt/intel_engine_pm.h"
> #include "gt/intel_gpu_commands.h"
> @@ -13,6 +10,11 @@
>
> #include "i915_trace.h"
>
> +#include "intel_pxp.h"
> +#include "intel_pxp_cmd.h"
> +#include "intel_pxp_session.h"
> +#include "intel_pxp_types.h"
> +
> /* stall until prior PXP and MFX/HCP/HUC objects are cmopleted */
> #define MFX_WAIT_PXP (MFX_WAIT | \
> MFX_WAIT_DW0_PXP_SYNC_CONTROL_FLAG | \
> diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_debugfs.c b/drivers/gpu/drm/i915/pxp/intel_pxp_debugfs.c
> index 4359e8be4101..652c00aefe3f 100644
> --- a/drivers/gpu/drm/i915/pxp/intel_pxp_debugfs.c
> +++ b/drivers/gpu/drm/i915/pxp/intel_pxp_debugfs.c
> @@ -9,10 +9,13 @@
> #include <drm/drm_print.h>
>
> #include "gt/intel_gt_debugfs.h"
> +
> #include "i915_drv.h"
> +
> #include "intel_pxp.h"
> #include "intel_pxp_debugfs.h"
> #include "intel_pxp_irq.h"
> +#include "intel_pxp_types.h"
>
> static int pxp_info_show(struct seq_file *m, void *data)
> {
> @@ -30,7 +33,19 @@ static int pxp_info_show(struct seq_file *m, void *data)
>
> return 0;
> }
> -DEFINE_INTEL_GT_DEBUGFS_ATTRIBUTE(pxp_info);
> +
> +static int pxp_info_open(struct inode *inode, struct file *file)
> +{
> + return single_open(file, pxp_info_show, inode->i_private);
> +}
> +
> +static const struct file_operations pxp_info_fops = {
> + .owner = THIS_MODULE,
> + .open = pxp_info_open,
> + .read = seq_read,
> + .llseek = seq_lseek,
> + .release = single_release,
> +};
>
> static int pxp_terminate_get(void *data, u64 *val)
> {
> @@ -59,23 +74,23 @@ static int pxp_terminate_set(void *data, u64 val)
> }
>
> DEFINE_SIMPLE_ATTRIBUTE(pxp_terminate_fops, pxp_terminate_get, pxp_terminate_set, "%llx\n");
> -void intel_pxp_debugfs_register(struct intel_pxp *pxp, struct dentry *gt_root)
> +
> +void intel_pxp_debugfs_register(struct intel_pxp *pxp)
> {
> - static const struct intel_gt_debugfs_file files[] = {
> - { "info", &pxp_info_fops, NULL },
> - { "terminate_state", &pxp_terminate_fops, NULL },
> - };
> - struct dentry *root;
> + struct drm_minor *minor = pxp->i915->drm.primary;
> + struct dentry *pxproot;
>
> - if (!gt_root)
> + if (!HAS_PXP(pxp->i915))
> return;
>
> - if (!HAS_PXP((pxp_to_gt(pxp)->i915)))
> + pxproot = debugfs_create_dir("pxp", minor->debugfs_root);
> + if (IS_ERR(pxproot))
> return;
>
> - root = debugfs_create_dir("pxp", gt_root);
> - if (IS_ERR(root))
> - return;
> + debugfs_create_file("info", 0444, pxproot,
> + pxp, &pxp_info_fops);
> +
> + debugfs_create_file("terminate_state", 0644, pxproot,
> + pxp, &pxp_terminate_fops);
>
> - intel_gt_debugfs_register_files(root, files, ARRAY_SIZE(files), pxp);
> }
> diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_debugfs.h b/drivers/gpu/drm/i915/pxp/intel_pxp_debugfs.h
> index 7e0c3d2f5d7e..299382b59e66 100644
> --- a/drivers/gpu/drm/i915/pxp/intel_pxp_debugfs.h
> +++ b/drivers/gpu/drm/i915/pxp/intel_pxp_debugfs.h
> @@ -10,10 +10,10 @@ struct intel_pxp;
> struct dentry;
>
> #ifdef CONFIG_DRM_I915_PXP
> -void intel_pxp_debugfs_register(struct intel_pxp *pxp, struct dentry *root);
> +void intel_pxp_debugfs_register(struct intel_pxp *pxp);
> #else
> static inline void
> -intel_pxp_debugfs_register(struct intel_pxp *pxp, struct dentry *root)
> +intel_pxp_debugfs_register(struct intel_pxp *pxp)
> {
> }
> #endif
> diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_irq.c b/drivers/gpu/drm/i915/pxp/intel_pxp_irq.c
> index c28be430718a..fd30befbf784 100644
> --- a/drivers/gpu/drm/i915/pxp/intel_pxp_irq.c
> +++ b/drivers/gpu/drm/i915/pxp/intel_pxp_irq.c
> @@ -3,14 +3,18 @@
> * Copyright(c) 2020 Intel Corporation.
> */
> #include <linux/workqueue.h>
> -#include "intel_pxp.h"
> -#include "intel_pxp_irq.h"
> -#include "intel_pxp_session.h"
> +
> #include "gt/intel_gt_irq.h"
> #include "gt/intel_gt_regs.h"
> #include "gt/intel_gt_types.h"
> +
> #include "i915_irq.h"
> #include "i915_reg.h"
> +
> +#include "intel_pxp.h"
> +#include "intel_pxp_irq.h"
> +#include "intel_pxp_session.h"
> +#include "intel_pxp_types.h"
> #include "intel_runtime_pm.h"
>
> /**
> diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_pm.c b/drivers/gpu/drm/i915/pxp/intel_pxp_pm.c
> index 6a7d4e2ee138..37371f44a550 100644
> --- a/drivers/gpu/drm/i915/pxp/intel_pxp_pm.c
> +++ b/drivers/gpu/drm/i915/pxp/intel_pxp_pm.c
> @@ -3,11 +3,13 @@
> * Copyright(c) 2020 Intel Corporation.
> */
>
> +#include "i915_drv.h"
> +
> #include "intel_pxp.h"
> #include "intel_pxp_irq.h"
> #include "intel_pxp_pm.h"
> #include "intel_pxp_session.h"
> -#include "i915_drv.h"
> +#include "intel_pxp_types.h"
>
> void intel_pxp_suspend_prepare(struct intel_pxp *pxp)
> {
> diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_tee.c b/drivers/gpu/drm/i915/pxp/intel_pxp_tee.c
> index b0c9170b1395..16782d119bfd 100644
> --- a/drivers/gpu/drm/i915/pxp/intel_pxp_tee.c
> +++ b/drivers/gpu/drm/i915/pxp/intel_pxp_tee.c
> @@ -11,17 +11,19 @@
> #include "gem/i915_gem_lmem.h"
>
> #include "i915_drv.h"
> +
> #include "intel_pxp.h"
> -#include "intel_pxp_session.h"
> -#include "intel_pxp_tee.h"
> #include "intel_pxp_cmd_interface_42.h"
> #include "intel_pxp_huc.h"
> +#include "intel_pxp_session.h"
> +#include "intel_pxp_tee.h"
> +#include "intel_pxp_types.h"
>
> static inline struct intel_pxp *i915_dev_to_pxp(struct device *i915_kdev)
> {
> struct drm_i915_private *i915 = kdev_to_i915(i915_kdev);
>
> - return &to_gt(i915)->pxp;
> + return i915->pxp;
> }
>
> static int intel_pxp_tee_io_message(struct intel_pxp *pxp,
> diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_types.h b/drivers/gpu/drm/i915/pxp/intel_pxp_types.h
> index f74b1e11a505..d550cdba3399 100644
> --- a/drivers/gpu/drm/i915/pxp/intel_pxp_types.h
> +++ b/drivers/gpu/drm/i915/pxp/intel_pxp_types.h
> @@ -12,12 +12,23 @@
> #include <linux/workqueue.h>
>
> struct intel_context;
> +struct intel_gt;
> struct i915_pxp_component;
> +struct drm_i915_private;
>
> /**
> * struct intel_pxp - pxp state
> */
> struct intel_pxp {
> + /** @i915: back poiner to i915*/
> + struct drm_i915_private *i915;
do you really need this pointer back here?
or using a container_of should be enough?
> +
> + /**
> + * @ctrl_gt: poiner to the tile that owns the controls for PXP subsystem assets that
> + * the VDBOX, the KCR engine (and GSC CS depending on the platform)
> + */
> + struct intel_gt *ctrl_gt;
> +
> /**
> * @pxp_component: i915_pxp_component struct of the bound mei_pxp
> * module. Only set and cleared inside component bind/unbind functions,
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Intel-gfx] [PATCH v5 0/1] drm/i915/pxp: Prepare intel_pxp entry points for MTL
2022-11-29 0:31 [Intel-gfx] [PATCH v5 0/1] drm/i915/pxp: Prepare intel_pxp entry points for MTL Alan Previn
` (3 preceding siblings ...)
2022-11-29 1:07 ` [Intel-gfx] ✗ Fi.CI.BAT: failure " Patchwork
@ 2022-11-29 21:29 ` Rodrigo Vivi
2022-11-30 0:35 ` Teres Alexis, Alan Previn
4 siblings, 1 reply; 13+ messages in thread
From: Rodrigo Vivi @ 2022-11-29 21:29 UTC (permalink / raw)
To: Alan Previn; +Cc: intel-gfx
On Mon, Nov 28, 2022 at 04:31:51PM -0800, Alan Previn wrote:
> MTL has two tiles that is represented by the intel_gt structure in the i915
> code. The PXP feature has a control-structure that currently hangs off the
> intel_gt structure. In MTL, the standalone media tile (i.e. not the root
> tile) contains the VDBOX and KCR engine which are among several assets
> that PXP relies on for establishing and tearing down the PXP session.
>
> However PXP is a global feature as other engines on other tiles can reference
> the PXP session in an object's info within batch buffer instructions. That
> coherrency is handled implicitly by the HW. In fact, for the forseeable future,
> we are expecting this link whereby only one of the tiles will be the control-gt
> for the PXP subsystem.
>
> Keeping the intel_pxp structure within the intel_gt structure makes some
> internal functionalities more straight forward but adds code complexity to
> code readibility and maintainibility to many external-to-pxp subsystems
> which may need to pick the correct intel_gt structure. An example of this
> would be the intel_pxp_is_active or intel_pxp_is_enabled functionality which
> should be viewed as a global level inquiry, not a per-gt inquiry.
>
> That said, this series promotes the intel_pxp structure into the
> drm_i915_private structure making it a top-level subsystem and the PXP
> subsystem will select the control gt internally and keep a pointer to
> it for internal reference.
In general, no need for cover letter in single/standalone patches.
In this case, I believe this here is a very good information to be on the
commit message. It looks more complete and informative for later history,
then the current one.
>
> Changes from prior revs:
> v4: - Instead of maintaining intel_pxp as an intel_gt structure member and
> creating a number of convoluted helpers that takes in i915 as input
> and redirects to the correct intel_gt or takes any intel_gt and internally
> replaces with the correct intel_gt, promote it to be a top-level i915
> structure.
> v3: - Rename gt level helper functions to "intel_pxp_is_enabled/supported/
> active_on_gt" (Daniele)
> - Upgrade _gt_supports_pxp to replace what was intel_gtpxp_is_supported
> as the new intel_pxp_is_supported_on_gt to check for PXP feature
> support vs the tee support for huc authentication. Fix pxp-debugfs-
> registration to use only the former to decide support. (Daniele)
> - Couple minor optimizations.
> v2: - Avoid introduction of new device info or gt variables and use
> existing checks / macros to differentiate the correct GT->PXP
> control ownership (Daniele Ceraolo Spurio)
> - Don't reuse the updated global-checkers for per-GT callers (such
> as other files within PXP) to avoid unnecessary GT-reparsing,
> expose a replacement helper like the prior ones. (Daniele).
> v1: Add one more patch to the series for the intel_pxp suspend/resume
> for similiar refactoring
>
> Alan Previn (1):
> drm/i915/pxp: Promote pxp subsystem to top-level of i915
>
> .../drm/i915/display/skl_universal_plane.c | 2 +-
> drivers/gpu/drm/i915/gem/i915_gem_context.c | 6 +-
> drivers/gpu/drm/i915/gem/i915_gem_create.c | 2 +-
> .../gpu/drm/i915/gem/i915_gem_execbuffer.c | 2 +-
> drivers/gpu/drm/i915/gt/intel_gt.c | 5 --
> drivers/gpu/drm/i915/gt/intel_gt_debugfs.c | 1 -
> drivers/gpu/drm/i915/gt/intel_gt_irq.c | 2 +-
> drivers/gpu/drm/i915/gt/intel_gt_pm.c | 8 ---
> drivers/gpu/drm/i915/gt/intel_gt_types.h | 3 -
> drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c | 2 +-
> drivers/gpu/drm/i915/i915_driver.c | 20 ++++++
> drivers/gpu/drm/i915/i915_drv.h | 3 +
> drivers/gpu/drm/i915/pxp/intel_pxp.c | 72 ++++++++++++++-----
> drivers/gpu/drm/i915/pxp/intel_pxp.h | 6 +-
> drivers/gpu/drm/i915/pxp/intel_pxp_cmd.c | 8 ++-
> drivers/gpu/drm/i915/pxp/intel_pxp_debugfs.c | 41 +++++++----
> drivers/gpu/drm/i915/pxp/intel_pxp_debugfs.h | 4 +-
> drivers/gpu/drm/i915/pxp/intel_pxp_irq.c | 10 ++-
> drivers/gpu/drm/i915/pxp/intel_pxp_pm.c | 4 +-
> drivers/gpu/drm/i915/pxp/intel_pxp_tee.c | 8 ++-
> drivers/gpu/drm/i915/pxp/intel_pxp_types.h | 11 +++
> 21 files changed, 150 insertions(+), 70 deletions(-)
>
>
> base-commit: d21d6474a37e5d43075a24668807ea40a7ee9fc1
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Intel-gfx] [PATCH v5 1/1] drm/i915/pxp: Promote pxp subsystem to top-level of i915
2022-11-29 21:28 ` Rodrigo Vivi
@ 2022-11-30 0:10 ` Teres Alexis, Alan Previn
2022-11-30 8:50 ` Jani Nikula
2022-12-01 23:45 ` Teres Alexis, Alan Previn
1 sibling, 1 reply; 13+ messages in thread
From: Teres Alexis, Alan Previn @ 2022-11-30 0:10 UTC (permalink / raw)
To: Vivi, Rodrigo; +Cc: Nikula, Jani, intel-gfx
++Nikula if he has suggestions on the bottom most comment.
On Tue, 2022-11-29 at 16:28 -0500, Vivi, Rodrigo wrote:
> On Mon, Nov 28, 2022 at 04:31:52PM -0800, Alan Previn wrote:
> > Starting with MTL, there will be two GT-tiles, a render and media
> > tile. PXP as a service for supporting workloads with protected
> > contexts and protected buffers can be subscribed by process
> > workloads on any tile. However, depending on the platform,
> > only one of the tiles is used for control events pertaining to PXP
> > operation (such as creating the arbitration session and session
> > tear-down). In the case of MTL, this is the media-tile.
>
> Imho this patch shows that having the pxp under i915 instead of gt
> is the right way to go.
>
Alan: yes, agreed.
> but I have a few comments and doubts below...
>
> >
> >
Alan: [snip]
> > @@ -138,31 +144,63 @@ static void pxp_init_full(struct intel_pxp *pxp)
> > destroy_vcs_context(pxp);
> > }
> >
> > -void intel_pxp_init(struct intel_pxp *pxp)
> > +static struct intel_gt *pxp_get_kcr_owner_gt(struct drm_i915_private *i915)
>
> pxp_get_ctrl_gt or pxp_get_serving_gt sounds better in my opinion...
> what's "owner"?
>
Alan: Sure- will change to pxp_get_ctrl_gt (as per the name in the header file).
> > {
> > - struct intel_gt *gt = pxp_to_gt(pxp);
> > + struct intel_gt *gt = NULL;
> > + int i = 0;
> > +
> > + for_each_gt(gt, i915, i) {
> > + /* There can be only one GT that supports PXP */
> > + if (HAS_ENGINE(gt, GSC0))
> > + return gt;
> > + }
> >
> > /* we rely on the mei PXP module */
> > - if (!IS_ENABLED(CONFIG_INTEL_MEI_PXP))
> > - return;
> > + if (IS_ENABLED(CONFIG_INTEL_MEI_PXP))
> > + return &i915->gt0;
> > +
> > + return NULL;
> > +}
> > +
> > +int intel_pxp_init(struct intel_pxp **pxp_store_ptr)
>
> Please let's avoid the ** here and everywhere.
>
Alan: In order to to avoid causing the entire driver into a rebuild because of any change in the intel_pxp structure,
the only way to accomplish that is to use a ptr in i915. But using a ptr means we allocate the memory at init time and
free it at fini time and those 2 cases would require the ptr-to-ptr to ensure we get the correct store. The only way i
can avoid the ** is be passing i915 as the param and then populating the ptr via i915->pxp. Would this work?
> >
> >
Alan:[snip]
> > @@ -12,12 +12,23 @@
> > #include <linux/workqueue.h>
> >
> > struct intel_context;
> > +struct intel_gt;
> > struct i915_pxp_component;
> > +struct drm_i915_private;
> >
> > /**
> > * struct intel_pxp - pxp state
> > */
> > struct intel_pxp {
> > + /** @i915: back poiner to i915*/
> > + struct drm_i915_private *i915;
>
> do you really need this pointer back here?
> or using a container_of should be enough?
>
Alan: this is the same thing for above. We can use container_of if the caller passes the ptr-to-ptr ... if caller only
passes the pxp ptr, it will be passing, by reference, an allocated address. The only way I can think of to avoid this
is by dropping the ptr-to-ptr method and therefore pulling in the pxp type header into drm_i915_private header file -
which is againts the direction we are trying to head towards. (cc-ing Nikula is he has some ideas on this)
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Intel-gfx] [PATCH v5 0/1] drm/i915/pxp: Prepare intel_pxp entry points for MTL
2022-11-29 21:29 ` [Intel-gfx] [PATCH v5 0/1] " Rodrigo Vivi
@ 2022-11-30 0:35 ` Teres Alexis, Alan Previn
0 siblings, 0 replies; 13+ messages in thread
From: Teres Alexis, Alan Previn @ 2022-11-30 0:35 UTC (permalink / raw)
To: Vivi, Rodrigo; +Cc: intel-gfx
>
Alan: [snip]
> In general, no need for cover letter in single/standalone patches.
> In this case, I believe this here is a very good information to be on the
> commit message. It looks more complete and informative for later history,
> then the current one.
>
>
Alan: Okay will republish as single patch and merge the comments from cover letter to the single patch.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Intel-gfx] [PATCH v5 1/1] drm/i915/pxp: Promote pxp subsystem to top-level of i915
2022-11-30 0:10 ` Teres Alexis, Alan Previn
@ 2022-11-30 8:50 ` Jani Nikula
2022-12-01 19:06 ` Teres Alexis, Alan Previn
0 siblings, 1 reply; 13+ messages in thread
From: Jani Nikula @ 2022-11-30 8:50 UTC (permalink / raw)
To: Teres Alexis, Alan Previn, Vivi, Rodrigo; +Cc: intel-gfx
On Wed, 30 Nov 2022, "Teres Alexis, Alan Previn" <alan.previn.teres.alexis@intel.com> wrote:
> ++Nikula if he has suggestions on the bottom most comment.
>
> On Tue, 2022-11-29 at 16:28 -0500, Vivi, Rodrigo wrote:
>> On Mon, Nov 28, 2022 at 04:31:52PM -0800, Alan Previn wrote:
>> > Starting with MTL, there will be two GT-tiles, a render and media
>> > tile. PXP as a service for supporting workloads with protected
>> > contexts and protected buffers can be subscribed by process
>> > workloads on any tile. However, depending on the platform,
>> > only one of the tiles is used for control events pertaining to PXP
>> > operation (such as creating the arbitration session and session
>> > tear-down). In the case of MTL, this is the media-tile.
>>
>> Imho this patch shows that having the pxp under i915 instead of gt
>> is the right way to go.
>>
> Alan: yes, agreed.
>
>> but I have a few comments and doubts below...
>>
>> >
>> >
> Alan: [snip]
>
>> > @@ -138,31 +144,63 @@ static void pxp_init_full(struct intel_pxp *pxp)
>> > destroy_vcs_context(pxp);
>> > }
>> >
>> > -void intel_pxp_init(struct intel_pxp *pxp)
>> > +static struct intel_gt *pxp_get_kcr_owner_gt(struct drm_i915_private *i915)
>>
>> pxp_get_ctrl_gt or pxp_get_serving_gt sounds better in my opinion...
>> what's "owner"?
>>
> Alan: Sure- will change to pxp_get_ctrl_gt (as per the name in the header file).
>
>> > {
>> > - struct intel_gt *gt = pxp_to_gt(pxp);
>> > + struct intel_gt *gt = NULL;
>> > + int i = 0;
>> > +
>> > + for_each_gt(gt, i915, i) {
>> > + /* There can be only one GT that supports PXP */
>> > + if (HAS_ENGINE(gt, GSC0))
>> > + return gt;
>> > + }
>> >
>> > /* we rely on the mei PXP module */
>> > - if (!IS_ENABLED(CONFIG_INTEL_MEI_PXP))
>> > - return;
>> > + if (IS_ENABLED(CONFIG_INTEL_MEI_PXP))
>> > + return &i915->gt0;
>> > +
>> > + return NULL;
>> > +}
>> > +
>> > +int intel_pxp_init(struct intel_pxp **pxp_store_ptr)
>>
>> Please let's avoid the ** here and everywhere.
>>
> Alan: In order to to avoid causing the entire driver into a rebuild because of any change in the intel_pxp structure,
> the only way to accomplish that is to use a ptr in i915. But using a ptr means we allocate the memory at init time and
> free it at fini time and those 2 cases would require the ptr-to-ptr to ensure we get the correct store. The only way i
> can avoid the ** is be passing i915 as the param and then populating the ptr via i915->pxp. Would this work?
In general, one of two approaches should be used:
1) The caller takes care of storing/clearing the pointer:
struct intel_pxp *intel_pxp_init(void);
void intel_pxp_fini(struct intel_pxp *pxp);
2) The callee takes care of storing/clearing the pointer:
int intel_pxp_init(struct drm_i915_private *i915);
void intel_pxp_fini(struct drm_i915_private *i915);
Passing pointers to pointers is not as clean.
In this case, I'd choose 2) just because it's being called at the high
level, and it's pretty much in line with everything else.
BR,
Jani.
>
>> >
>> >
> Alan:[snip]
>
>> > @@ -12,12 +12,23 @@
>> > #include <linux/workqueue.h>
>> >
>> > struct intel_context;
>> > +struct intel_gt;
>> > struct i915_pxp_component;
>> > +struct drm_i915_private;
>> >
>> > /**
>> > * struct intel_pxp - pxp state
>> > */
>> > struct intel_pxp {
>> > + /** @i915: back poiner to i915*/
>> > + struct drm_i915_private *i915;
>>
>> do you really need this pointer back here?
>> or using a container_of should be enough?
>>
> Alan: this is the same thing for above. We can use container_of if the caller passes the ptr-to-ptr ... if caller only
> passes the pxp ptr, it will be passing, by reference, an allocated address. The only way I can think of to avoid this
> is by dropping the ptr-to-ptr method and therefore pulling in the pxp type header into drm_i915_private header file -
> which is againts the direction we are trying to head towards. (cc-ing Nikula is he has some ideas on this)
>>
--
Jani Nikula, Intel Open Source Graphics Center
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Intel-gfx] [PATCH v5 1/1] drm/i915/pxp: Promote pxp subsystem to top-level of i915
2022-11-30 8:50 ` Jani Nikula
@ 2022-12-01 19:06 ` Teres Alexis, Alan Previn
0 siblings, 0 replies; 13+ messages in thread
From: Teres Alexis, Alan Previn @ 2022-12-01 19:06 UTC (permalink / raw)
To: Vivi, Rodrigo, Nikula, Jani; +Cc: intel-gfx
> > >
> > > Please let's avoid the ** here and everywhere.
> > >
> > Alan: In order to to avoid causing the entire driver into a rebuild because of any change in the intel_pxp structure,
> > the only way to accomplish that is to use a ptr in i915. But using a ptr means we allocate the memory at init time and
> > free it at fini time and those 2 cases would require the ptr-to-ptr to ensure we get the correct store. The only way i
> > can avoid the ** is be passing i915 as the param and then populating the ptr via i915->pxp. Would this work?
>
> In general, one of two approaches should be used:
>
> 1) The caller takes care of storing/clearing the pointer:
>
> struct intel_pxp *intel_pxp_init(void);
> void intel_pxp_fini(struct intel_pxp *pxp);
>
> 2) The callee takes care of storing/clearing the pointer:
>
> int intel_pxp_init(struct drm_i915_private *i915);
> void intel_pxp_fini(struct drm_i915_private *i915);
>
> Passing pointers to pointers is not as clean.
>
> In this case, I'd choose 2) just because it's being called at the high
> level, and it's pretty much in line with everything else.
>
>
>
> BR,
> Jani.
I like option 2. Also, I just remembered I used a similiar approach for guc-error-capture (where intel_guc_capture_init
received a ptr to 'struct intel_guc' which owns a 'struct intel_guc_state_capture *capture;' member and did the
allocation). I did that back then exactly to address Jani's concerns because i was tired of waiting on build times when
developing offline :D
Will respin accordingly.
....alan
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Intel-gfx] [PATCH v5 1/1] drm/i915/pxp: Promote pxp subsystem to top-level of i915
2022-11-29 21:28 ` Rodrigo Vivi
2022-11-30 0:10 ` Teres Alexis, Alan Previn
@ 2022-12-01 23:45 ` Teres Alexis, Alan Previn
1 sibling, 0 replies; 13+ messages in thread
From: Teres Alexis, Alan Previn @ 2022-12-01 23:45 UTC (permalink / raw)
To: Vivi, Rodrigo; +Cc: intel-gfx
> > struct intel_pxp {
> > + /** @i915: back poiner to i915*/
> > + struct drm_i915_private *i915;
>
> do you really need this pointer back here?
> or using a container_of should be enough?
I realize i can drop the i915 backptr as i can use pxp->ctrl_gt->i915.
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2022-12-01 23:45 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-29 0:31 [Intel-gfx] [PATCH v5 0/1] drm/i915/pxp: Prepare intel_pxp entry points for MTL Alan Previn
2022-11-29 0:31 ` [Intel-gfx] [PATCH v5 1/1] drm/i915/pxp: Promote pxp subsystem to top-level of i915 Alan Previn
2022-11-29 18:35 ` Teres Alexis, Alan Previn
2022-11-29 21:28 ` Rodrigo Vivi
2022-11-30 0:10 ` Teres Alexis, Alan Previn
2022-11-30 8:50 ` Jani Nikula
2022-12-01 19:06 ` Teres Alexis, Alan Previn
2022-12-01 23:45 ` Teres Alexis, Alan Previn
2022-11-29 0:51 ` [Intel-gfx] [PATCH v5 0/1] drm/i915/pxp: Prepare intel_pxp entry points for MTL Teres Alexis, Alan Previn
2022-11-29 0:53 ` [Intel-gfx] ✗ Fi.CI.SPARSE: warning for " Patchwork
2022-11-29 1:07 ` [Intel-gfx] ✗ Fi.CI.BAT: failure " Patchwork
2022-11-29 21:29 ` [Intel-gfx] [PATCH v5 0/1] " Rodrigo Vivi
2022-11-30 0:35 ` Teres Alexis, Alan Previn
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).