| Message ID | 20221129003152.3652072-1-alan.previn.teres.alexis@intel.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | drm/i915/pxp: Prepare intel_pxp entry points for MTL | expand |
(++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 >
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 >
> 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.
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