[Intel-gfx] [PATCH v5 1/1] drm/i915/dg1: Add HWMON power sensor support
As part of the System Managemenent Interface (SMI), use the HWMON subsystem to display power utilization. The following standard HWMON power sensors are currently supported (and appropriately scaled): /sys/class/drm/card0/device/hwmon/hwmon - energy1_input - power1_cap - power1_max Some non-standard HWMON power information is also provided, such as enable bits and intervals. Signed-off-by: Dale B Stimson --- .../ABI/testing/sysfs-driver-intel-i915-hwmon | 116 +++ drivers/gpu/drm/i915/Kconfig | 1 + drivers/gpu/drm/i915/Makefile | 1 + drivers/gpu/drm/i915/i915_drv.c | 6 + drivers/gpu/drm/i915/i915_drv.h | 2 + drivers/gpu/drm/i915/i915_hwmon.c | 770 ++ drivers/gpu/drm/i915/i915_hwmon.h | 49 ++ drivers/gpu/drm/i915/i915_reg.h | 52 ++ 8 files changed, 997 insertions(+) create mode 100644 Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon create mode 100644 drivers/gpu/drm/i915/i915_hwmon.c create mode 100644 drivers/gpu/drm/i915/i915_hwmon.h diff --git a/Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon b/Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon new file mode 100644 index 0..910fb8e46f74d --- /dev/null +++ b/Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon @@ -0,0 +1,116 @@ +What: /sys/devices/.../hwmon/hwmon/energy1_input +Date: June 2021 +KernelVersion: 5.14 +Contact:dri-de...@lists.freedesktop.org +Description: +RO. Energy input of device in microjoules. + + The returned textual representation is an unsigned integer + number that can be stored in 64-bits. Warning: The hardware + register is 32-bits wide and can overflow by wrapping around. + A single wrap-around between calls to read this value can + be detected and will be accounted for in the returned value. + At a power consumption of 1 watt, the 32-bit hardware register + would wrap-around approximately every 3 days. + + Only supported for particular Intel i915 graphics platforms. + +What: /sys/devices/.../hwmon/hwmon/power1_max_enable +Date: June 2021 +KernelVersion: 5.14 +Contact:dri-de...@lists.freedesktop.org +Description: +RO. Sustained power limit is enabled - true or false. + +The power controller will throttle the operating frequency +if the power averaged over a window (typically seconds) +exceeds this limit. + +See power1_max_enable power1_max power1_max_interval + + Only supported for particular Intel i915 graphics platforms. + +What: /sys/devices/.../hwmon/hwmon/power1_max +Date: June 2021 +KernelVersion: 5.14 +Contact:dri-de...@lists.freedesktop.org +Description: +RW. Sustained power limit in milliwatts + +The power controller will throttle the operating frequency +if the power averaged over a window (typically seconds) +exceeds this limit. + +See power1_max_enable power1_max power1_max_interval + + Only supported for particular Intel i915 graphics platforms. + +What: /sys/devices/.../hwmon/hwmon/power1_max_interval +Date: June 2021 +KernelVersion: 5.14 +Contact:dri-de...@lists.freedesktop.org +Description: +RW. Sustained power limit interval in milliseconds over +which sustained power is averaged. + +See power1_max_enable power1_max power1_max_interval + + Only supported for particular Intel i915 graphics platforms. + +What: /sys/devices/.../hwmon/hwmon/power1_cap_enable +Date: June 2021 +KernelVersion: 5.14 +Contact:dri-de...@lists.freedesktop.org +Description: + RW. Power burst limit is enabled - true or false + +See power1_cap_enable power1_cap + + Only supported for particular Intel i915 graphics platforms. + +What: /sys/devices/.../hwmon/hwmon/power1_cap +Date: June 2021 +KernelVersion: 5.14 +Contact:dri-de...@lists.freedesktop.org +Description: + RW. Power burst limit in milliwatts. + +See power1_cap_enable power1_cap + + Only supported for particular Intel i915 graphics platforms. + +What: /sys/devices/.../hwmon/hwmon/power_default_limit +Date: June 2021 +KernelVersion: 5.14 +Contact:dri-de...@lists.freedesktop.org +Description: +RO. Default power limit. + + Only supported for particular Intel i915 graphics platforms. + +What: /sys/devices/.../hwmon/hwmon/power_min_limit +Date: June 2021 +KernelVersion: 5.14
[Intel-gfx] [PATCH v5 0/1] drm/i915/dg1: Add HWMON power sensor support
drm/i915/dg1: Add HWMON power support
As part of the System Managemenent Interface (SMI), use the HWMON
subsystem to display power utilization.
The following standard HWMON entries are currently supported
(and appropriately scaled):
/sys/class/drm/card0/device/hwmon/hwmon
- energy1_input
- power1_cap
- power1_max
Some non-standard HWMON power information is also provided, such as
enable bits and intervals.
-
v5 Simplify header file inclusion requirements by having
"struct drm_i915_private" definition contain
"struct i915_hwmon *hwmon"
instead of
"struct i915_hwmon hwmon"
v5 Arg 3 (private data pointer) of hwmon_device_register_with_info()
is now i915.
v5 File i915_hwmon.h rearranged. New struct i915_energy_info in
struct i915_hwmon to hold variables energy_counter_prev and
energy_counter_overflow.
v4 Commit mesage minor rewording
v4 Move call to i915_hwmon_register() to a more appropriate location,
so that it is done after intel_gt_driver_register().
The call to i915_perf_unregister() is moved correspondingly.
v4 The proper register to read energy status is PCU_PACKAGE_ENERGY_STATUS.
v4 Attribute power1_max_enable is read-only.
v3 Added documentation of these hwmon attributes in file
Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon
v3 Commit mesage minor rewording
v3 Function name changes:
i915_hwmon_init() -> i915_hwmon_register()
i915_hwmon_fini() -> i915_hwmon_unregister()
v3 i915_hwmon_register and i915_hwmon_unregister now take arg i915.
v3 i915_hwmon_register() now returns void instead of int.
v3 Macro FIELD_SHIFT() added to compute shift value from constant
field mask.
v3 Certain functions now longer require "inline" due to addition of new
parameter field_shift, allowing access to constant expressions for
the field mask at each call site. These functions now do field
access via shift and masking and no longer use le32*() functions
(as le32*() required a local constant expression for the mask).
_field_read_and_scale()
_field_read64_and_scale()
_field_scale_and_write()
v3 Some comments were modified.
v3 Now using sysfs_emit() instead of scnprintf().
V2 Rename local function parameter field_mask to field_msk in order to avoid
shadowing the name of function field_mask() from include/linux/bitfield.h.
V2 Change a comment introduction from "/**" to "/*", as it is not intended
to match a pattern that triggers documentation.
Reported-by: kernel test robot
V2 Slight movement of calls:
- i915_hwmon_init slightly later, after call to i915_setup_sysfs()
- i915_hwmon_fini slightly earlier, before i915_teardown_sysfs()
V2 Fixed some strong typing issues with le32 functions.
Detected by sparse in a run by kernel test robot:
Reported-by: kernel test robot
Dale B Stimson (1):
drm/i915/dg1: Add HWMON power sensor support
.../ABI/testing/sysfs-driver-intel-i915-hwmon | 116 +++
drivers/gpu/drm/i915/Kconfig | 1 +
drivers/gpu/drm/i915/Makefile | 1 +
drivers/gpu/drm/i915/i915_drv.c | 6 +
drivers/gpu/drm/i915/i915_drv.h | 2 +
drivers/gpu/drm/i915/i915_hwmon.c | 770 ++
drivers/gpu/drm/i915/i915_hwmon.h | 49 ++
drivers/gpu/drm/i915/i915_reg.h | 52 ++
8 files changed, 997 insertions(+)
create mode 100644 Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon
create mode 100644 drivers/gpu/drm/i915/i915_hwmon.c
create mode 100644 drivers/gpu/drm/i915/i915_hwmon.h
Range-diff against v4:
1: a2f69c455f908 ! 1: 99a80043ae127 drm/i915/dg1: Add HWMON power sensor
support
@@ drivers/gpu/drm/i915/i915_drv.c: static void
i915_driver_unregister(struct drm_i
i915_teardown_sysfs(dev_priv);
## drivers/gpu/drm/i915/i915_drv.h ##
-@@
- #include
- #include
-
-+#include "i915_hwmon.h"
- #include "i915_params.h"
- #include "i915_reg.h"
- #include "i915_utils.h"
@@ drivers/gpu/drm/i915/i915_drv.h: struct drm_i915_private {
struct i915_perf perf;
-+ struct i915_hwmon hwmon;
++ struct i915_hwmon *hwmon;
+
/* Abstract the submission mechanism (legacy ringbuffer or execlists)
away */
struct intel_gt gt;
@@ drivers/gpu/drm/i915/i915_hwmon.c (new)
+ i915_reg_t reg, u32 clear, u32 set)
+{
+ struct drm_i915_private *i915 = uncore->i915;
-+ struct i915_hwmon *hwmon = &i915->hwmon;
++ struct i915_hwmon *hwmon = i915->hwmon;
+ intel_wakeref_t wakeref;
+
+ mutex_lock(&hwmon->hwmon_lock);
@@ drivers/gpu/drm/i915/i915_hwmon.c (new)
+{
+ struct drm_i915_private *i91
Re: [Intel-gfx] [PATCH v4 1/1] drm/i915/dg1: Add HWMON power sensor support
On 2021-06-01 14:39:11, Sundaresan, Sujaritha wrote: > Date: Tue, 1 Jun 2021 14:39:11 -0700 > From: "Sundaresan, Sujaritha" > To: Dale B Stimson , > intel-gfx@lists.freedesktop.org, dri-de...@lists.freedesktop.org > CC: Jon Ewins , Jani Nikula > > Subject: Re: [PATCH v4 1/1] drm/i915/dg1: Add HWMON power sensor support > User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:78.0) > Gecko/20100101 Thunderbird/78.10.2 > > > On 5/27/2021 5:44 PM, Dale B Stimson wrote: > > As part of the System Managemenent Interface (SMI), use the HWMON > > subsystem to display power utilization. > > > > The following standard HWMON power sensors are currently supported > > (and appropriately scaled): > >/sys/class/drm/card0/device/hwmon/hwmon > > - energy1_input > > - power1_cap > > - power1_max > > > > Some non-standard HWMON power information is also provided, such as > > enable bits and intervals. > > > > Signed-off-by: Dale B Stimson > > --- > > .../ABI/testing/sysfs-driver-intel-i915-hwmon | 116 +++ > > drivers/gpu/drm/i915/Kconfig | 1 + > > drivers/gpu/drm/i915/Makefile | 1 + > > drivers/gpu/drm/i915/i915_drv.c | 6 + > > drivers/gpu/drm/i915/i915_drv.h | 3 + > > drivers/gpu/drm/i915/i915_hwmon.c | 757 ++ > > drivers/gpu/drm/i915/i915_hwmon.h | 42 + > > drivers/gpu/drm/i915/i915_reg.h | 52 ++ > > 8 files changed, 978 insertions(+) > > create mode 100644 Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon > > create mode 100644 drivers/gpu/drm/i915/i915_hwmon.c > > create mode 100644 drivers/gpu/drm/i915/i915_hwmon.h > > > > diff --git a/Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon > > b/Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon > > new file mode 100644 > > index 0..2ee7c413ca190 > > --- /dev/null > > +++ b/Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon > > @@ -0,0 +1,116 @@ > > +What: /sys/devices/.../hwmon/hwmon/energy1_input > > +Date: June 2021 > > +KernelVersion: 5.14 > > +Contact:dri-de...@lists.freedesktop.org > > +Description: > > +RO. Energy input of device in microjoules. > > + > > + The returned textual representation is an unsigned integer > > + number that can be stored in 64-bits. Warning: The hardware > > + register is 32-bits wide and can overflow by wrapping around. > > + A single wrap-around between calls to read this value can > > + be detected and will be accounted for in the returned value. > > + At a power consumption of 1 watt, the 32-bit hardware register > > + would wrap-around approximately every 3 days. > > + > > + Only supported for particular Intel i915 graphics platforms. > > + > > +What: /sys/devices/.../hwmon/hwmon/power1_max_enable > > +Date: June 2021 > > +KernelVersion: 5.14 > > +Contact:dri-de...@lists.freedesktop.org > > +Description: > > +RW. Sustained power limit is enabled - true or false. > > Hi Dale, > > This attribute should be read-only ? That is correct. The hardware implementation is read-only. > > + > > +The power controller will throttle the operating frequency > > +if the power averaged over a window (typically seconds) > > +exceeds this limit. > > + > > +See power1_max_enable power1_max power1_max_interval > > + > > + Only supported for particular Intel i915 graphics platforms. > > + > > +What: /sys/devices/.../hwmon/hwmon/power1_max > > +Date: June 2021 > > +KernelVersion: 5.14 > > +Contact:dri-de...@lists.freedesktop.org > > +Description: > > +RW. Sustained power limit in milliwatts > > + > > +The power controller will throttle the operating frequency > > +if the power averaged over a window (typically seconds) > > +exceeds this limit. > > + > > +See power1_max_enable power1_max power1_max_interval > > + > > + Only supported for particular Intel i915 graphics platforms. > > + > > +What: /sys/devices/.../hwmon/hwmon/power1_max_interval > > +Date: June 2021 > > +KernelVersion: 5.14 > > +Contact:
[Intel-gfx] [PATCH v4 1/1] drm/i915/dg1: Add HWMON power sensor support
As part of the System Managemenent Interface (SMI), use the HWMON subsystem to display power utilization. The following standard HWMON power sensors are currently supported (and appropriately scaled): /sys/class/drm/card0/device/hwmon/hwmon - energy1_input - power1_cap - power1_max Some non-standard HWMON power information is also provided, such as enable bits and intervals. Signed-off-by: Dale B Stimson --- .../ABI/testing/sysfs-driver-intel-i915-hwmon | 116 +++ drivers/gpu/drm/i915/Kconfig | 1 + drivers/gpu/drm/i915/Makefile | 1 + drivers/gpu/drm/i915/i915_drv.c | 6 + drivers/gpu/drm/i915/i915_drv.h | 3 + drivers/gpu/drm/i915/i915_hwmon.c | 757 ++ drivers/gpu/drm/i915/i915_hwmon.h | 42 + drivers/gpu/drm/i915/i915_reg.h | 52 ++ 8 files changed, 978 insertions(+) create mode 100644 Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon create mode 100644 drivers/gpu/drm/i915/i915_hwmon.c create mode 100644 drivers/gpu/drm/i915/i915_hwmon.h diff --git a/Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon b/Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon new file mode 100644 index 0..2ee7c413ca190 --- /dev/null +++ b/Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon @@ -0,0 +1,116 @@ +What: /sys/devices/.../hwmon/hwmon/energy1_input +Date: June 2021 +KernelVersion: 5.14 +Contact:dri-de...@lists.freedesktop.org +Description: +RO. Energy input of device in microjoules. + + The returned textual representation is an unsigned integer + number that can be stored in 64-bits. Warning: The hardware + register is 32-bits wide and can overflow by wrapping around. + A single wrap-around between calls to read this value can + be detected and will be accounted for in the returned value. + At a power consumption of 1 watt, the 32-bit hardware register + would wrap-around approximately every 3 days. + + Only supported for particular Intel i915 graphics platforms. + +What: /sys/devices/.../hwmon/hwmon/power1_max_enable +Date: June 2021 +KernelVersion: 5.14 +Contact:dri-de...@lists.freedesktop.org +Description: +RW. Sustained power limit is enabled - true or false. + +The power controller will throttle the operating frequency +if the power averaged over a window (typically seconds) +exceeds this limit. + +See power1_max_enable power1_max power1_max_interval + + Only supported for particular Intel i915 graphics platforms. + +What: /sys/devices/.../hwmon/hwmon/power1_max +Date: June 2021 +KernelVersion: 5.14 +Contact:dri-de...@lists.freedesktop.org +Description: +RW. Sustained power limit in milliwatts + +The power controller will throttle the operating frequency +if the power averaged over a window (typically seconds) +exceeds this limit. + +See power1_max_enable power1_max power1_max_interval + + Only supported for particular Intel i915 graphics platforms. + +What: /sys/devices/.../hwmon/hwmon/power1_max_interval +Date: June 2021 +KernelVersion: 5.14 +Contact:dri-de...@lists.freedesktop.org +Description: +RW. Sustained power limit interval in milliseconds over +which sustained power is averaged. + +See power1_max_enable power1_max power1_max_interval + + Only supported for particular Intel i915 graphics platforms. + +What: /sys/devices/.../hwmon/hwmon/power1_cap_enable +Date: June 2021 +KernelVersion: 5.14 +Contact:dri-de...@lists.freedesktop.org +Description: + RW. Power burst limit is enabled - true or false + +See power1_cap_enable power1_cap + + Only supported for particular Intel i915 graphics platforms. + +What: /sys/devices/.../hwmon/hwmon/power1_cap +Date: June 2021 +KernelVersion: 5.14 +Contact:dri-de...@lists.freedesktop.org +Description: + RW. Power burst limit in milliwatts. + +See power1_cap_enable power1_cap + + Only supported for particular Intel i915 graphics platforms. + +What: /sys/devices/.../hwmon/hwmon/power_default_limit +Date: June 2021 +KernelVersion: 5.14 +Contact:dri-de...@lists.freedesktop.org +Description: +RO. Default power limit. + + Only supported for particular Intel i915 graphics platforms. + +What: /sys/devices/.../hwmon/hwmon/power_min_limit +Date: June 2021 +KernelVersion: 5.14
[Intel-gfx] [PATCH v4 0/1] drm/i915/dg1: Add HWMON power sensor support
drm/i915/dg1: Add HWMON power support
As part of the System Managemenent Interface (SMI), use the HWMON
subsystem to display power utilization.
The following standard HWMON entries are currently supported
(and appropriately scaled):
/sys/class/drm/card0/device/hwmon/hwmon
- energy1_input
- power1_cap
- power1_max
Some non-standard HWMON power information is also provided, such as
enable bits and intervals.
-
v4 Commit mesage minor rewording
v4 Move call to i915_hwmon_register() to a more appropriate location,
so that it is done after intel_gt_driver_register().
The call to i915_perf_unregister() is moved correspondingly.
v4 The proper register to read energy status is PCU_PACKAGE_ENERGY_STATUS.
v4 Attribute power1_max_enable is read-only.
v3 Added documentation of these hwmon attributes in file
Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon
v3 Commit mesage minor rewording
v3 Function name changes:
i915_hwmon_init() -> i915_hwmon_register()
i915_hwmon_fini() -> i915_hwmon_unregister()
v3 i915_hwmon_register and i915_hwmon_unregister now take arg i915.
v3 i915_hwmon_register() now returns void instead of int.
v3 Macro FIELD_SHIFT() added to compute shift value from constant
field mask.
v3 Certain functions now longer require "inline" due to addition of new
parameter field_shift, allowing access to constant expressions for
the field mask at each call site. These functions now do field
access via shift and masking and no longer use le32*() functions
(as le32*() required a local constant expression for the mask).
_field_read_and_scale()
_field_read64_and_scale()
_field_scale_and_write()
v3 Some comments were modified.
v3 Now using sysfs_emit() instead of scnprintf().
V2 Rename local function parameter field_mask to field_msk in order to avoid
shadowing the name of function field_mask() from include/linux/bitfield.h.
V2 Change a comment introduction from "/**" to "/*", as it is not intended
to match a pattern that triggers documentation.
Reported-by: kernel test robot
V2 Slight movement of calls:
- i915_hwmon_init slightly later, after call to i915_setup_sysfs()
- i915_hwmon_fini slightly earlier, before i915_teardown_sysfs()
V2 Fixed some strong typing issues with le32 functions.
Detected by sparse in a run by kernel test robot:
Reported-by: kernel test robot
Dale B Stimson (1):
drm/i915/dg1: Add HWMON power sensor support
.../ABI/testing/sysfs-driver-intel-i915-hwmon | 116 +++
drivers/gpu/drm/i915/Kconfig | 1 +
drivers/gpu/drm/i915/Makefile | 1 +
drivers/gpu/drm/i915/i915_drv.c | 6 +
drivers/gpu/drm/i915/i915_drv.h | 3 +
drivers/gpu/drm/i915/i915_hwmon.c | 757 ++
drivers/gpu/drm/i915/i915_hwmon.h | 42 +
drivers/gpu/drm/i915/i915_reg.h | 52 ++
8 files changed, 978 insertions(+)
create mode 100644 Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon
create mode 100644 drivers/gpu/drm/i915/i915_hwmon.c
create mode 100644 drivers/gpu/drm/i915/i915_hwmon.h
Range-diff against v3:
1: ed34d683a0ef1 ! 1: bc8bd78b2c006 drm/i915/dg1: Add HWMON power support
@@ Metadata
Author: Dale B Stimson
## Commit message ##
-drm/i915/dg1: Add HWMON power support
+drm/i915/dg1: Add HWMON power sensor support
As part of the System Managemenent Interface (SMI), use the HWMON
subsystem to display power utilization.
-The following standard HWMON entries are currently supported
+The following standard HWMON power sensors are currently supported
(and appropriately scaled):
/sys/class/drm/card0/device/hwmon/hwmon
- energy1_input
@@ drivers/gpu/drm/i915/i915_drv.c
#include "i915_irq.h"
#include "i915_memcpy.h"
@@ drivers/gpu/drm/i915/i915_drv.c: static void
i915_driver_register(struct drm_i915_private *dev_priv)
- i915_debugfs_register(dev_priv);
- i915_setup_sysfs(dev_priv);
+
+ intel_gt_driver_register(&dev_priv->gt);
+ i915_hwmon_register(dev_priv);
+
- /* Depends on sysfs having been initialized */
- i915_perf_register(dev_priv);
+ intel_display_driver_register(dev_priv);
+ intel_power_domains_enable(dev_priv);
@@ drivers/gpu/drm/i915/i915_drv.c: static void
i915_driver_unregister(struct drm_i915_private *dev_priv)
+
+ intel_display_driver_unregister(dev_priv);
+
++ i915_hwmon_unregister(dev_priv);
++
intel_gt_driver_unregister(&dev_priv->gt);
i915_perf_unregister(dev_priv);
-+
-+ i915_hwmon_unregister(dev_priv);
+
i915_pmu_unregister(dev_priv);
@@ driver
[Intel-gfx] [PATCH v4 1/1] drm/i915/dg1: Add HWMON power sensor support
As part of the System Managemenent Interface (SMI), use the HWMON subsystem to display power utilization. The following standard HWMON power sensors are currently supported (and appropriately scaled): /sys/class/drm/card0/device/hwmon/hwmon - energy1_input - power1_cap - power1_max Some non-standard HWMON power information is also provided, such as enable bits and intervals. Signed-off-by: Dale B Stimson --- .../ABI/testing/sysfs-driver-intel-i915-hwmon | 116 +++ drivers/gpu/drm/i915/Kconfig | 1 + drivers/gpu/drm/i915/Makefile | 1 + drivers/gpu/drm/i915/i915_drv.c | 6 + drivers/gpu/drm/i915/i915_drv.h | 3 + drivers/gpu/drm/i915/i915_hwmon.c | 757 ++ drivers/gpu/drm/i915/i915_hwmon.h | 42 + drivers/gpu/drm/i915/i915_reg.h | 52 ++ 8 files changed, 978 insertions(+) create mode 100644 Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon create mode 100644 drivers/gpu/drm/i915/i915_hwmon.c create mode 100644 drivers/gpu/drm/i915/i915_hwmon.h diff --git a/Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon b/Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon new file mode 100644 index 0..2ee7c413ca190 --- /dev/null +++ b/Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon @@ -0,0 +1,116 @@ +What: /sys/devices/.../hwmon/hwmon/energy1_input +Date: June 2021 +KernelVersion: 5.14 +Contact:dri-de...@lists.freedesktop.org +Description: +RO. Energy input of device in microjoules. + + The returned textual representation is an unsigned integer + number that can be stored in 64-bits. Warning: The hardware + register is 32-bits wide and can overflow by wrapping around. + A single wrap-around between calls to read this value can + be detected and will be accounted for in the returned value. + At a power consumption of 1 watt, the 32-bit hardware register + would wrap-around approximately every 3 days. + + Only supported for particular Intel i915 graphics platforms. + +What: /sys/devices/.../hwmon/hwmon/power1_max_enable +Date: June 2021 +KernelVersion: 5.14 +Contact:dri-de...@lists.freedesktop.org +Description: +RW. Sustained power limit is enabled - true or false. + +The power controller will throttle the operating frequency +if the power averaged over a window (typically seconds) +exceeds this limit. + +See power1_max_enable power1_max power1_max_interval + + Only supported for particular Intel i915 graphics platforms. + +What: /sys/devices/.../hwmon/hwmon/power1_max +Date: June 2021 +KernelVersion: 5.14 +Contact:dri-de...@lists.freedesktop.org +Description: +RW. Sustained power limit in milliwatts + +The power controller will throttle the operating frequency +if the power averaged over a window (typically seconds) +exceeds this limit. + +See power1_max_enable power1_max power1_max_interval + + Only supported for particular Intel i915 graphics platforms. + +What: /sys/devices/.../hwmon/hwmon/power1_max_interval +Date: June 2021 +KernelVersion: 5.14 +Contact:dri-de...@lists.freedesktop.org +Description: +RW. Sustained power limit interval in milliseconds over +which sustained power is averaged. + +See power1_max_enable power1_max power1_max_interval + + Only supported for particular Intel i915 graphics platforms. + +What: /sys/devices/.../hwmon/hwmon/power1_cap_enable +Date: June 2021 +KernelVersion: 5.14 +Contact:dri-de...@lists.freedesktop.org +Description: + RW. Power burst limit is enabled - true or false + +See power1_cap_enable power1_cap + + Only supported for particular Intel i915 graphics platforms. + +What: /sys/devices/.../hwmon/hwmon/power1_cap +Date: June 2021 +KernelVersion: 5.14 +Contact:dri-de...@lists.freedesktop.org +Description: + RW. Power burst limit in milliwatts. + +See power1_cap_enable power1_cap + + Only supported for particular Intel i915 graphics platforms. + +What: /sys/devices/.../hwmon/hwmon/power_default_limit +Date: June 2021 +KernelVersion: 5.14 +Contact:dri-de...@lists.freedesktop.org +Description: +RO. Default power limit. + + Only supported for particular Intel i915 graphics platforms. + +What: /sys/devices/.../hwmon/hwmon/power_min_limit +Date: June 2021 +KernelVersion: 5.14
[Intel-gfx] [PATCH v4 0/1] drm/i915/dg1: Add HWMON power sensor support
drm/i915/dg1: Add HWMON power support
As part of the System Managemenent Interface (SMI), use the HWMON
subsystem to display power utilization.
The following standard HWMON entries are currently supported
(and appropriately scaled):
/sys/class/drm/card0/device/hwmon/hwmon
- energy1_input
- power1_cap
- power1_max
Some non-standard HWMON power information is also provided, such as
enable bits and intervals.
-
v4 Commit mesage minor rewording
v4 Move call to i915_hwmon_register() to a more appropriate location,
so that it is done after intel_gt_driver_register().
The call to i915_perf_unregister() is moved correspondingly.
v4 The proper register to read energy status is PCU_PACKAGE_ENERGY_STATUS.
v4 Attribute power1_max_enable is read-only.
v3 Added documentation of these hwmon attributes in file
Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon
v3 Commit mesage minor rewording
v3 Function name changes:
i915_hwmon_init() -> i915_hwmon_register()
i915_hwmon_fini() -> i915_hwmon_unregister()
v3 i915_hwmon_register and i915_hwmon_unregister now take arg i915.
v3 i915_hwmon_register() now returns void instead of int.
v3 Macro FIELD_SHIFT() added to compute shift value from constant
field mask.
v3 Certain functions now longer require "inline" due to addition of new
parameter field_shift, allowing access to constant expressions for
the field mask at each call site. These functions now do field
access via shift and masking and no longer use le32*() functions
(as le32*() required a local constant expression for the mask).
_field_read_and_scale()
_field_read64_and_scale()
_field_scale_and_write()
v3 Some comments were modified.
v3 Now using sysfs_emit() instead of scnprintf().
V2 Rename local function parameter field_mask to field_msk in order to avoid
shadowing the name of function field_mask() from include/linux/bitfield.h.
V2 Change a comment introduction from "/**" to "/*", as it is not intended
to match a pattern that triggers documentation.
Reported-by: kernel test robot
V2 Slight movement of calls:
- i915_hwmon_init slightly later, after call to i915_setup_sysfs()
- i915_hwmon_fini slightly earlier, before i915_teardown_sysfs()
V2 Fixed some strong typing issues with le32 functions.
Detected by sparse in a run by kernel test robot:
Reported-by: kernel test robot
Dale B Stimson (1):
drm/i915/dg1: Add HWMON power sensor support
.../ABI/testing/sysfs-driver-intel-i915-hwmon | 116 +++
drivers/gpu/drm/i915/Kconfig | 1 +
drivers/gpu/drm/i915/Makefile | 1 +
drivers/gpu/drm/i915/i915_drv.c | 6 +
drivers/gpu/drm/i915/i915_drv.h | 3 +
drivers/gpu/drm/i915/i915_hwmon.c | 757 ++
drivers/gpu/drm/i915/i915_hwmon.h | 42 +
drivers/gpu/drm/i915/i915_reg.h | 52 ++
8 files changed, 978 insertions(+)
create mode 100644 Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon
create mode 100644 drivers/gpu/drm/i915/i915_hwmon.c
create mode 100644 drivers/gpu/drm/i915/i915_hwmon.h
Range-diff against v3:
1: ed34d683a0ef1 ! 1: bc8bd78b2c006 drm/i915/dg1: Add HWMON power support
@@ Metadata
Author: Dale B Stimson
## Commit message ##
-drm/i915/dg1: Add HWMON power support
+drm/i915/dg1: Add HWMON power sensor support
As part of the System Managemenent Interface (SMI), use the HWMON
subsystem to display power utilization.
-The following standard HWMON entries are currently supported
+The following standard HWMON power sensors are currently supported
(and appropriately scaled):
/sys/class/drm/card0/device/hwmon/hwmon
- energy1_input
@@ drivers/gpu/drm/i915/i915_drv.c
#include "i915_irq.h"
#include "i915_memcpy.h"
@@ drivers/gpu/drm/i915/i915_drv.c: static void
i915_driver_register(struct drm_i915_private *dev_priv)
- i915_debugfs_register(dev_priv);
- i915_setup_sysfs(dev_priv);
+
+ intel_gt_driver_register(&dev_priv->gt);
+ i915_hwmon_register(dev_priv);
+
- /* Depends on sysfs having been initialized */
- i915_perf_register(dev_priv);
+ intel_display_driver_register(dev_priv);
+ intel_power_domains_enable(dev_priv);
@@ drivers/gpu/drm/i915/i915_drv.c: static void
i915_driver_unregister(struct drm_i915_private *dev_priv)
+
+ intel_display_driver_unregister(dev_priv);
+
++ i915_hwmon_unregister(dev_priv);
++
intel_gt_driver_unregister(&dev_priv->gt);
i915_perf_unregister(dev_priv);
-+
-+ i915_hwmon_unregister(dev_priv);
+
i915_pmu_unregister(dev_priv);
@@ driver
[Intel-gfx] [PATCH v4 1/1] drm/i915/dg1: Add HWMON power sensor support
As part of the System Managemenent Interface (SMI), use the HWMON subsystem to display power utilization. The following standard HWMON power sensors are currently supported (and appropriately scaled): /sys/class/drm/card0/device/hwmon/hwmon - energy1_input - power1_cap - power1_max Some non-standard HWMON power information is also provided, such as enable bits and intervals. Signed-off-by: Dale B Stimson --- .../ABI/testing/sysfs-driver-intel-i915-hwmon | 116 +++ drivers/gpu/drm/i915/Kconfig | 1 + drivers/gpu/drm/i915/Makefile | 1 + drivers/gpu/drm/i915/i915_drv.c | 6 + drivers/gpu/drm/i915/i915_drv.h | 3 + drivers/gpu/drm/i915/i915_hwmon.c | 757 ++ drivers/gpu/drm/i915/i915_hwmon.h | 42 + drivers/gpu/drm/i915/i915_reg.h | 52 ++ 8 files changed, 978 insertions(+) create mode 100644 Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon create mode 100644 drivers/gpu/drm/i915/i915_hwmon.c create mode 100644 drivers/gpu/drm/i915/i915_hwmon.h diff --git a/Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon b/Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon new file mode 100644 index 0..2ee7c413ca190 --- /dev/null +++ b/Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon @@ -0,0 +1,116 @@ +What: /sys/devices/.../hwmon/hwmon/energy1_input +Date: June 2021 +KernelVersion: 5.14 +Contact:dri-de...@lists.freedesktop.org +Description: +RO. Energy input of device in microjoules. + + The returned textual representation is an unsigned integer + number that can be stored in 64-bits. Warning: The hardware + register is 32-bits wide and can overflow by wrapping around. + A single wrap-around between calls to read this value can + be detected and will be accounted for in the returned value. + At a power consumption of 1 watt, the 32-bit hardware register + would wrap-around approximately every 3 days. + + Only supported for particular Intel i915 graphics platforms. + +What: /sys/devices/.../hwmon/hwmon/power1_max_enable +Date: June 2021 +KernelVersion: 5.14 +Contact:dri-de...@lists.freedesktop.org +Description: +RW. Sustained power limit is enabled - true or false. + +The power controller will throttle the operating frequency +if the power averaged over a window (typically seconds) +exceeds this limit. + +See power1_max_enable power1_max power1_max_interval + + Only supported for particular Intel i915 graphics platforms. + +What: /sys/devices/.../hwmon/hwmon/power1_max +Date: June 2021 +KernelVersion: 5.14 +Contact:dri-de...@lists.freedesktop.org +Description: +RW. Sustained power limit in milliwatts + +The power controller will throttle the operating frequency +if the power averaged over a window (typically seconds) +exceeds this limit. + +See power1_max_enable power1_max power1_max_interval + + Only supported for particular Intel i915 graphics platforms. + +What: /sys/devices/.../hwmon/hwmon/power1_max_interval +Date: June 2021 +KernelVersion: 5.14 +Contact:dri-de...@lists.freedesktop.org +Description: +RW. Sustained power limit interval in milliseconds over +which sustained power is averaged. + +See power1_max_enable power1_max power1_max_interval + + Only supported for particular Intel i915 graphics platforms. + +What: /sys/devices/.../hwmon/hwmon/power1_cap_enable +Date: June 2021 +KernelVersion: 5.14 +Contact:dri-de...@lists.freedesktop.org +Description: + RW. Power burst limit is enabled - true or false + +See power1_cap_enable power1_cap + + Only supported for particular Intel i915 graphics platforms. + +What: /sys/devices/.../hwmon/hwmon/power1_cap +Date: June 2021 +KernelVersion: 5.14 +Contact:dri-de...@lists.freedesktop.org +Description: + RW. Power burst limit in milliwatts. + +See power1_cap_enable power1_cap + + Only supported for particular Intel i915 graphics platforms. + +What: /sys/devices/.../hwmon/hwmon/power_default_limit +Date: June 2021 +KernelVersion: 5.14 +Contact:dri-de...@lists.freedesktop.org +Description: +RO. Default power limit. + + Only supported for particular Intel i915 graphics platforms. + +What: /sys/devices/.../hwmon/hwmon/power_min_limit +Date: June 2021 +KernelVersion: 5.14
[Intel-gfx] [PATCH v4 0/1] drm/i915/dg1: Add HWMON power sensor support
drm/i915/dg1: Add HWMON power support
As part of the System Managemenent Interface (SMI), use the HWMON
subsystem to display power utilization.
The following standard HWMON entries are currently supported
(and appropriately scaled):
/sys/class/drm/card0/device/hwmon/hwmon
- energy1_input
- power1_cap
- power1_max
Some non-standard HWMON power information is also provided, such as
enable bits and intervals.
-
v4 Commit mesage minor rewording
v4 Move call to i915_hwmon_register() to a more appropriate location,
so that it is done after intel_gt_driver_register().
The call to i915_perf_unregister() is moved correspondingly.
v4 The proper register to read energy status is PCU_PACKAGE_ENERGY_STATUS.
v4 Attribute power1_max_enable is read-only.
v3 Added documentation of these hwmon attributes in file
Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon
v3 Commit mesage minor rewording
v3 Function name changes:
i915_hwmon_init() -> i915_hwmon_register()
i915_hwmon_fini() -> i915_hwmon_unregister()
v3 i915_hwmon_register and i915_hwmon_unregister now take arg i915.
v3 i915_hwmon_register() now returns void instead of int.
v3 Macro FIELD_SHIFT() added to compute shift value from constant
field mask.
v3 Certain functions now longer require "inline" due to addition of new
parameter field_shift, allowing access to constant expressions for
the field mask at each call site. These functions now do field
access via shift and masking and no longer use le32*() functions
(as le32*() required a local constant expression for the mask).
_field_read_and_scale()
_field_read64_and_scale()
_field_scale_and_write()
v3 Some comments were modified.
v3 Now using sysfs_emit() instead of scnprintf().
V2 Rename local function parameter field_mask to field_msk in order to avoid
shadowing the name of function field_mask() from include/linux/bitfield.h.
V2 Change a comment introduction from "/**" to "/*", as it is not intended
to match a pattern that triggers documentation.
Reported-by: kernel test robot
V2 Slight movement of calls:
- i915_hwmon_init slightly later, after call to i915_setup_sysfs()
- i915_hwmon_fini slightly earlier, before i915_teardown_sysfs()
V2 Fixed some strong typing issues with le32 functions.
Detected by sparse in a run by kernel test robot:
Reported-by: kernel test robot
Dale B Stimson (1):
drm/i915/dg1: Add HWMON power sensor support
.../ABI/testing/sysfs-driver-intel-i915-hwmon | 116 +++
drivers/gpu/drm/i915/Kconfig | 1 +
drivers/gpu/drm/i915/Makefile | 1 +
drivers/gpu/drm/i915/i915_drv.c | 6 +
drivers/gpu/drm/i915/i915_drv.h | 3 +
drivers/gpu/drm/i915/i915_hwmon.c | 757 ++
drivers/gpu/drm/i915/i915_hwmon.h | 42 +
drivers/gpu/drm/i915/i915_reg.h | 52 ++
8 files changed, 978 insertions(+)
create mode 100644 Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon
create mode 100644 drivers/gpu/drm/i915/i915_hwmon.c
create mode 100644 drivers/gpu/drm/i915/i915_hwmon.h
Range-diff against v3:
1: ed34d683a0ef1 ! 1: bc8bd78b2c006 drm/i915/dg1: Add HWMON power support
@@ Metadata
Author: Dale B Stimson
## Commit message ##
-drm/i915/dg1: Add HWMON power support
+drm/i915/dg1: Add HWMON power sensor support
As part of the System Managemenent Interface (SMI), use the HWMON
subsystem to display power utilization.
-The following standard HWMON entries are currently supported
+The following standard HWMON power sensors are currently supported
(and appropriately scaled):
/sys/class/drm/card0/device/hwmon/hwmon
- energy1_input
@@ drivers/gpu/drm/i915/i915_drv.c
#include "i915_irq.h"
#include "i915_memcpy.h"
@@ drivers/gpu/drm/i915/i915_drv.c: static void
i915_driver_register(struct drm_i915_private *dev_priv)
- i915_debugfs_register(dev_priv);
- i915_setup_sysfs(dev_priv);
+
+ intel_gt_driver_register(&dev_priv->gt);
+ i915_hwmon_register(dev_priv);
+
- /* Depends on sysfs having been initialized */
- i915_perf_register(dev_priv);
+ intel_display_driver_register(dev_priv);
+ intel_power_domains_enable(dev_priv);
@@ drivers/gpu/drm/i915/i915_drv.c: static void
i915_driver_unregister(struct drm_i915_private *dev_priv)
+
+ intel_display_driver_unregister(dev_priv);
+
++ i915_hwmon_unregister(dev_priv);
++
intel_gt_driver_unregister(&dev_priv->gt);
i915_perf_unregister(dev_priv);
-+
-+ i915_hwmon_unregister(dev_priv);
+
i915_pmu_unregister(dev_priv);
@@ driver
[Intel-gfx] [PATCH v3 1/1] drm/i915/dg1: Add HWMON power support
As part of the System Managemenent Interface (SMI), use the HWMON subsystem to display power utilization. The following standard HWMON entries are currently supported (and appropriately scaled): /sys/class/drm/card0/device/hwmon/hwmon - energy1_input - power1_cap - power1_max Some non-standard HWMON power information is also provided, such as enable bits and intervals. Signed-off-by: Dale B Stimson --- .../ABI/testing/sysfs-driver-intel-i915-hwmon | 116 +++ drivers/gpu/drm/i915/Kconfig | 1 + drivers/gpu/drm/i915/Makefile | 1 + drivers/gpu/drm/i915/i915_drv.c | 6 + drivers/gpu/drm/i915/i915_drv.h | 3 + drivers/gpu/drm/i915/i915_hwmon.c | 799 ++ drivers/gpu/drm/i915/i915_hwmon.h | 44 + drivers/gpu/drm/i915/i915_reg.h | 53 ++ 8 files changed, 1023 insertions(+) create mode 100644 Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon create mode 100644 drivers/gpu/drm/i915/i915_hwmon.c create mode 100644 drivers/gpu/drm/i915/i915_hwmon.h diff --git a/Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon b/Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon new file mode 100644 index 0..2ee7c413ca190 --- /dev/null +++ b/Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon @@ -0,0 +1,116 @@ +What: /sys/devices/.../hwmon/hwmon/energy1_input +Date: June 2021 +KernelVersion: 5.14 +Contact:dri-de...@lists.freedesktop.org +Description: +RO. Energy input of device in microjoules. + + The returned textual representation is an unsigned integer + number that can be stored in 64-bits. Warning: The hardware + register is 32-bits wide and can overflow by wrapping around. + A single wrap-around between calls to read this value can + be detected and will be accounted for in the returned value. + At a power consumption of 1 watt, the 32-bit hardware register + would wrap-around approximately every 3 days. + + Only supported for particular Intel i915 graphics platforms. + +What: /sys/devices/.../hwmon/hwmon/power1_max_enable +Date: June 2021 +KernelVersion: 5.14 +Contact:dri-de...@lists.freedesktop.org +Description: +RW. Sustained power limit is enabled - true or false. + +The power controller will throttle the operating frequency +if the power averaged over a window (typically seconds) +exceeds this limit. + +See power1_max_enable power1_max power1_max_interval + + Only supported for particular Intel i915 graphics platforms. + +What: /sys/devices/.../hwmon/hwmon/power1_max +Date: June 2021 +KernelVersion: 5.14 +Contact:dri-de...@lists.freedesktop.org +Description: +RW. Sustained power limit in milliwatts + +The power controller will throttle the operating frequency +if the power averaged over a window (typically seconds) +exceeds this limit. + +See power1_max_enable power1_max power1_max_interval + + Only supported for particular Intel i915 graphics platforms. + +What: /sys/devices/.../hwmon/hwmon/power1_max_interval +Date: June 2021 +KernelVersion: 5.14 +Contact:dri-de...@lists.freedesktop.org +Description: +RW. Sustained power limit interval in milliseconds over +which sustained power is averaged. + +See power1_max_enable power1_max power1_max_interval + + Only supported for particular Intel i915 graphics platforms. + +What: /sys/devices/.../hwmon/hwmon/power1_cap_enable +Date: June 2021 +KernelVersion: 5.14 +Contact:dri-de...@lists.freedesktop.org +Description: + RW. Power burst limit is enabled - true or false + +See power1_cap_enable power1_cap + + Only supported for particular Intel i915 graphics platforms. + +What: /sys/devices/.../hwmon/hwmon/power1_cap +Date: June 2021 +KernelVersion: 5.14 +Contact:dri-de...@lists.freedesktop.org +Description: + RW. Power burst limit in milliwatts. + +See power1_cap_enable power1_cap + + Only supported for particular Intel i915 graphics platforms. + +What: /sys/devices/.../hwmon/hwmon/power_default_limit +Date: June 2021 +KernelVersion: 5.14 +Contact:dri-de...@lists.freedesktop.org +Description: +RO. Default power limit. + + Only supported for particular Intel i915 graphics platforms. + +What: /sys/devices/.../hwmon/hwmon/power_min_limit +Date: June 2021 +KernelVersion: 5.14 +Contact
[Intel-gfx] [PATCH v3 0/1] drm/i915/dg1: Add HWMON power support
drm/i915/dg1: Add HWMON power support
As part of the System Managemenent Interface (SMI), use the HWMON
subsystem to display power utilization.
The following standard HWMON entries are currently supported
(and appropriately scaled):
/sys/class/drm/card0/device/hwmon/hwmon
- energy1_input
- power1_cap
- power1_max
Some non-standard HWMON power information is also provided, such as
enable bits and intervals.
-
v3 Added documentation of these hwmon attributes in file
Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon
v3 Commit mesage minor rewording
v3 Function name changes:
i915_hwmon_init() -> i915_hwmon_register()
i915_hwmon_fini() -> i915_hwmon_unregister()
v3 i915_hwmon_register and i915_hwmon_unregister now take arg i915.
v3 i915_hwmon_register() now returns void instead of int.
v3 Macro FIELD_SHIFT() added to compute shift value from constant
field mask.
v3 Certain functions now longer require "inline" due to addition of new
parameter field_shift, allowing access to constant expressions for
the field mask at each call site. These functions now do field
access via shift and masking and no longer use le32*() functions
(as le32*() required a local constant expression for the mask).
_field_read_and_scale()
_field_read64_and_scale()
_field_scale_and_write()
v3 Some comments were modified.
v3 Now using sysfs_emit() instead of scnprintf().
V2 Rename local function parameter field_mask to field_msk in order to avoid
shadowing the name of function field_mask() from include/linux/bitfield.h.
V2 Change a comment introduction from "/**" to "/*", as it is not intended
to match a pattern that triggers documentation.
Reported-by: kernel test robot
V2 Slight movement of calls:
- i915_hwmon_init slightly later, after call to i915_setup_sysfs()
- i915_hwmon_fini slightly earlier, before i915_teardown_sysfs()
V2 Fixed some strong typing issues with le32 functions.
Detected by sparse in a run by kernel test robot:
Reported-by: kernel test robot
Dale B Stimson (1):
drm/i915/dg1: Add HWMON power support
.../ABI/testing/sysfs-driver-intel-i915-hwmon | 116 +++
drivers/gpu/drm/i915/Kconfig | 1 +
drivers/gpu/drm/i915/Makefile | 1 +
drivers/gpu/drm/i915/i915_drv.c | 6 +
drivers/gpu/drm/i915/i915_drv.h | 3 +
drivers/gpu/drm/i915/i915_hwmon.c | 799 ++
drivers/gpu/drm/i915/i915_hwmon.h | 44 +
drivers/gpu/drm/i915/i915_reg.h | 53 ++
8 files changed, 1023 insertions(+)
create mode 100644 Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon
create mode 100644 drivers/gpu/drm/i915/i915_hwmon.c
create mode 100644 drivers/gpu/drm/i915/i915_hwmon.h
Range-diff against v2:
1: 6e841c4d62a5a ! 1: afa81d35e1c55 drm/i915/dg1: Add HWMON power sensor
support
@@ Metadata
Author: Dale B Stimson
## Commit message ##
-drm/i915/dg1: Add HWMON power sensor support
+drm/i915/dg1: Add HWMON power support
As part of the System Managemenent Interface (SMI), use the HWMON
subsystem to display power utilization.
-The following standard HWMON power sensors are currently supported
+The following standard HWMON entries are currently supported
(and appropriately scaled):
/sys/class/drm/card0/device/hwmon/hwmon
- energy1_input
@@ drivers/gpu/drm/i915/i915_drv.c: static void i915_driver_register(struct
drm_i91
i915_debugfs_register(dev_priv);
i915_setup_sysfs(dev_priv);
-+ /* Register with hwmon */
-+ if (i915_hwmon_init(&dev_priv->drm))
-+ drm_err(&dev_priv->drm, "Failed to register driver hwmon!\n");
++ i915_hwmon_register(dev_priv);
+
/* Depends on sysfs having been initialized */
i915_perf_register(dev_priv);
@@ drivers/gpu/drm/i915/i915_drv.c: static void
i915_driver_unregister(struct drm_i
i915_perf_unregister(dev_priv);
+
-+ i915_hwmon_fini(&dev_priv->drm);
++ i915_hwmon_unregister(dev_priv);
+
i915_pmu_unregister(dev_priv);
i915_teardown_sysfs(dev_priv);
-+
- drm_dev_unplug(&dev_priv->drm);
-
- i915_gem_driver_unregister(dev_priv);
## drivers/gpu/drm/i915/i915_drv.h ##
@@
@@ drivers/gpu/drm/i915/i915_drv.h: struct drm_i915_private {
## drivers/gpu/drm/i915/i915_hwmon.c (new) ##
@@
+// SPDX-License-Identifier: MIT
-+
+/*
+ * Copyright © 2020 Intel Corporation
+ */
@@ drivers/gpu/drm/i915/i915_hwmon.c (new)
+#define SF_POWER 100
+#define SF_ENERGY 100
+
++#define FIELD_SHIFT(__mask)
Re: [Intel-gfx] [PATCH v2 1/1] drm/i915/dg1: Add HWMON power sensor support
On 2021-04-21 18:03:51, Jani Nikula wrote: > On Tue, 13 Apr 2021, Dale B Stimson wrote: > > As part of the System Managemenent Interface (SMI), use the HWMON > > subsystem to display power utilization. > > > > The following standard HWMON power sensors are currently supported > > (and appropriately scaled): > > /sys/class/drm/card0/device/hwmon/hwmon > > - energy1_input > > - power1_cap > > - power1_max > > > > Some non-standard HWMON power information is also provided, such as > > enable bits and intervals. > > > > Signed-off-by: Dale B Stimson > > --- > > drivers/gpu/drm/i915/Kconfig | 1 + > > drivers/gpu/drm/i915/Makefile | 1 + > > drivers/gpu/drm/i915/i915_drv.c | 9 + > > drivers/gpu/drm/i915/i915_drv.h | 3 + > > drivers/gpu/drm/i915/i915_hwmon.c | 788 ++ > > drivers/gpu/drm/i915/i915_hwmon.h | 41 ++ > > drivers/gpu/drm/i915/i915_reg.h | 53 ++ > > 7 files changed, 896 insertions(+) > > create mode 100644 drivers/gpu/drm/i915/i915_hwmon.c > > create mode 100644 drivers/gpu/drm/i915/i915_hwmon.h > > > > diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig > > index 1e1cb245fca77..ec8d5a0d7ea96 100644 > > --- a/drivers/gpu/drm/i915/Kconfig > > +++ b/drivers/gpu/drm/i915/Kconfig > > @@ -14,6 +14,7 @@ config DRM_I915 > > select DRM_MIPI_DSI > > select RELAY > > select IRQ_WORK > > + select HWMON > > # i915 depends on ACPI_VIDEO when ACPI is enabled > > # but for select to work, need to select ACPI_VIDEO's dependencies, ick > > select BACKLIGHT_CLASS_DEVICE if ACPI > > diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile > > index d0d936d9137bc..e213e2b129e20 100644 > > --- a/drivers/gpu/drm/i915/Makefile > > +++ b/drivers/gpu/drm/i915/Makefile > > @@ -37,6 +37,7 @@ i915-y += i915_drv.o \ > > i915_config.o \ > > i915_irq.o \ > > i915_getparam.o \ > > + i915_hwmon.o \ > > i915_mitigations.o \ > > i915_params.o \ > > i915_pci.o \ > > diff --git a/drivers/gpu/drm/i915/i915_drv.c > > b/drivers/gpu/drm/i915/i915_drv.c > > index 305557e1942aa..84c7de3b34c7d 100644 > > --- a/drivers/gpu/drm/i915/i915_drv.c > > +++ b/drivers/gpu/drm/i915/i915_drv.c > > @@ -69,6 +69,7 @@ > > > > #include "i915_debugfs.h" > > #include "i915_drv.h" > > +#include "i915_hwmon.h" > > #include "i915_ioc32.h" > > #include "i915_irq.h" > > #include "i915_memcpy.h" > > @@ -675,6 +676,10 @@ static void i915_driver_register(struct > > drm_i915_private *dev_priv) > > i915_debugfs_register(dev_priv); > > i915_setup_sysfs(dev_priv); > > > > + /* Register with hwmon */ > > + if (i915_hwmon_init(&dev_priv->drm)) > > Please pass in i915, not struct drm_device. Done. > This is i915_driver_register. Almost all functions being have _register > in them. Why not this one? I have changed the function names to get i915_hwmon_register and i915_hwmon_unregister. > > + drm_err(&dev_priv->drm, "Failed to register driver hwmon!\n"); > > Not sure we want this error message at this level. I have removed this error message and changed i915_hwmon_register so it returns void instead of int. > > > + > > /* Depends on sysfs having been initialized */ > > i915_perf_register(dev_priv); > > > > @@ -709,9 +714,13 @@ static void i915_driver_unregister(struct > > drm_i915_private *dev_priv) > > intel_gt_driver_unregister(&dev_priv->gt); > > > > i915_perf_unregister(dev_priv); > > + > > + i915_hwmon_fini(&dev_priv->drm); > > + > > Naming, again _unregister in most places. Fixed. > > > i915_pmu_unregister(dev_priv); > > > > i915_teardown_sysfs(dev_priv); > > + > > Stray newline. Fixed. > > > drm_dev_unplug(&dev_priv->drm); > > > > i915_gem_driver_unregister(dev_priv); > > diff --git a/drivers/gpu/drm/i915/i915_drv.h > > b/drivers/gpu/drm/i915/i915_drv.h > > index 69e43bf91a153..7e9b452c77e2b 100644 > > --- a/drivers/gpu/drm/i915/i915_drv.h > > +++ b/drivers/gpu/drm/i915/i915_drv.h > > @@ -61,6 +61,7 @@ > > #include > > #include > > > > +#include "i915_hwmon.h" > > #include "i915_params.h" > > #include "
[Intel-gfx] [PATCH v1 0/1] drm/i915/dg1: Add HWMON power sensor support
As part of the System Managemenent Interface (SMI), use the HWMON
subsystem to display power utilization.
The following standard HWMON power sensors are currently supported
(and appropriately scaled):
/sys/class/drm/card0/device/hwmon/hwmon
- energy1_input
- power1_cap
- power1_max
Some non-standard HWMON power information is also provided, such as
enable bits and intervals.
-
V2 Rename local function parameter field_mask to field_msk in order to avoid
shadowing the name of function field_mask() from include/linux/bitfield.h.
V2 Change a comment introduction from "/**" to "/*", as it is not intended
to match a pattern that triggers documentation.
Reported-by: kernel test robot
V2 Slight movement of calls:
- i915_hwmon_init slightly later, after call to i915_setup_sysfs()
- i915_hwmon_fini slightly earlier, before i915_teardown_sysfs()
V2 Fixed some strong typing issues with le32 functions.
Detected by sparse in a run by kernel test robot:
Reported-by: kernel test robot
Dale B Stimson (1):
drm/i915/dg1: Add HWMON power sensor support
drivers/gpu/drm/i915/Kconfig | 1 +
drivers/gpu/drm/i915/Makefile | 1 +
drivers/gpu/drm/i915/i915_drv.c | 9 +
drivers/gpu/drm/i915/i915_drv.h | 3 +
drivers/gpu/drm/i915/i915_hwmon.c | 788 ++
drivers/gpu/drm/i915/i915_hwmon.h | 41 ++
drivers/gpu/drm/i915/i915_reg.h | 53 ++
7 files changed, 896 insertions(+)
create mode 100644 drivers/gpu/drm/i915/i915_hwmon.c
create mode 100644 drivers/gpu/drm/i915/i915_hwmon.h
Range-diff against v1:
1: 34631511e00c1 ! 1: 25117970961b4 drm/i915/dg1: Add HWMON power sensor
support
@@ drivers/gpu/drm/i915/i915_hwmon.c (new)
+
+#include
+#include
++#include
+
+#include "i915_drv.h"
+#include "gt/intel_gt.h"
@@ drivers/gpu/drm/i915/i915_hwmon.c (new)
+ */
+static __always_inline u64
+_field_read_and_scale(struct intel_uncore *uncore, i915_reg_t rgadr,
-+u32 field_mask, int nshift, unsigned int scale_factor)
++u32 field_msk, int nshift, unsigned int scale_factor)
+{
+ intel_wakeref_t wakeref;
+ u32 reg_value;
@@ drivers/gpu/drm/i915/i915_hwmon.c (new)
+ with_intel_runtime_pm(uncore->rpm, wakeref)
+ reg_value = intel_uncore_read(uncore, rgadr);
+
-+ reg_value = le32_get_bits(reg_value, field_mask);
++ reg_value = le32_get_bits(cpu_to_le32(reg_value), field_msk);
+ scaled_val = mul_u32_u32(scale_factor, reg_value);
+
+ /* Shift, rounding to nearest */
@@ drivers/gpu/drm/i915/i915_hwmon.c (new)
+ */
+static __always_inline u64
+_field_read64_and_scale(struct intel_uncore *uncore, i915_reg_t rgadr,
-+ u64 field_mask, int nshift, unsigned int scale_factor)
++ u64 field_msk, int nshift, unsigned int scale_factor)
+{
+ intel_wakeref_t wakeref;
+ u64 reg_value;
@@ drivers/gpu/drm/i915/i915_hwmon.c (new)
+ with_intel_runtime_pm(uncore->rpm, wakeref)
+ reg_value = intel_uncore_read64(uncore, rgadr);
+
-+ reg_value = le64_get_bits(reg_value, field_mask);
++ reg_value = le64_get_bits(cpu_to_le64(reg_value), field_msk);
+ scaled_val = scale_factor * reg_value;
+
+ /* Shift, rounding to nearest */
@@ drivers/gpu/drm/i915/i915_hwmon.c (new)
+static __always_inline void
+_field_scale_and_write(struct intel_uncore *uncore,
+ i915_reg_t rgadr,
-+ u32 field_mask, int nshift,
++ u32 field_msk, int nshift,
+ unsigned int scale_factor, long lval)
+{
+ u32 nval;
@@ drivers/gpu/drm/i915/i915_hwmon.c (new)
+ /* Computation in 64-bits to avoid overflow. Round to nearest. */
+ nval = DIV_ROUND_CLOSEST_ULL((u64)lval << nshift, scale_factor);
+
-+ bits_to_clear = field_mask;
-+ bits_to_set = le32_encode_bits(nval, field_mask);
++ bits_to_clear = field_msk;
++ bits_to_set = le32_to_cpu(le32_encode_bits(nval, field_msk));
+
+ _locked_with_pm_intel_uncore_rmw(uncore, rgadr,
+ bits_to_clear, bits_to_set);
@@ drivers/gpu/drm/i915/i915_hwmon.c (new)
+ struct intel_uncore *uncore = &i915->uncore;
+ intel_wakeref_t wakeref;
+ u32 val_sku_unit;
++ __le32 le_sku_unit;
+
+ if (IS_DG1(i915)) {
+ hwmon->rg.pkg_power_sku_unit = PCU_PACKAGE_POWER_SKU_UNIT;
@@ drivers/gpu/drm/i915/i915_hwmon.c (new)
+
+ intel_runtime_pm_put(uncore->rpm, wakeref);
+
-+ hwmon->scl_shift_power = le32_get_bits(val_sku_unit, PKG_PWR_UNIT);
-+ hwmon->scl_shift_energy = le32_get_bits(val_sku_unit, PKG_ENERGY_UNIT);
[Intel-gfx] [PATCH v2 1/1] drm/i915/dg1: Add HWMON power sensor support
As part of the System Managemenent Interface (SMI), use the HWMON
subsystem to display power utilization.
The following standard HWMON power sensors are currently supported
(and appropriately scaled):
/sys/class/drm/card0/device/hwmon/hwmon
- energy1_input
- power1_cap
- power1_max
Some non-standard HWMON power information is also provided, such as
enable bits and intervals.
Signed-off-by: Dale B Stimson
---
drivers/gpu/drm/i915/Kconfig | 1 +
drivers/gpu/drm/i915/Makefile | 1 +
drivers/gpu/drm/i915/i915_drv.c | 9 +
drivers/gpu/drm/i915/i915_drv.h | 3 +
drivers/gpu/drm/i915/i915_hwmon.c | 788 ++
drivers/gpu/drm/i915/i915_hwmon.h | 41 ++
drivers/gpu/drm/i915/i915_reg.h | 53 ++
7 files changed, 896 insertions(+)
create mode 100644 drivers/gpu/drm/i915/i915_hwmon.c
create mode 100644 drivers/gpu/drm/i915/i915_hwmon.h
diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig
index 1e1cb245fca77..ec8d5a0d7ea96 100644
--- a/drivers/gpu/drm/i915/Kconfig
+++ b/drivers/gpu/drm/i915/Kconfig
@@ -14,6 +14,7 @@ config DRM_I915
select DRM_MIPI_DSI
select RELAY
select IRQ_WORK
+ select HWMON
# i915 depends on ACPI_VIDEO when ACPI is enabled
# but for select to work, need to select ACPI_VIDEO's dependencies, ick
select BACKLIGHT_CLASS_DEVICE if ACPI
diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index d0d936d9137bc..e213e2b129e20 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -37,6 +37,7 @@ i915-y += i915_drv.o \
i915_config.o \
i915_irq.o \
i915_getparam.o \
+ i915_hwmon.o \
i915_mitigations.o \
i915_params.o \
i915_pci.o \
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 305557e1942aa..84c7de3b34c7d 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -69,6 +69,7 @@
#include "i915_debugfs.h"
#include "i915_drv.h"
+#include "i915_hwmon.h"
#include "i915_ioc32.h"
#include "i915_irq.h"
#include "i915_memcpy.h"
@@ -675,6 +676,10 @@ static void i915_driver_register(struct drm_i915_private
*dev_priv)
i915_debugfs_register(dev_priv);
i915_setup_sysfs(dev_priv);
+ /* Register with hwmon */
+ if (i915_hwmon_init(&dev_priv->drm))
+ drm_err(&dev_priv->drm, "Failed to register driver hwmon!\n");
+
/* Depends on sysfs having been initialized */
i915_perf_register(dev_priv);
@@ -709,9 +714,13 @@ static void i915_driver_unregister(struct drm_i915_private
*dev_priv)
intel_gt_driver_unregister(&dev_priv->gt);
i915_perf_unregister(dev_priv);
+
+ i915_hwmon_fini(&dev_priv->drm);
+
i915_pmu_unregister(dev_priv);
i915_teardown_sysfs(dev_priv);
+
drm_dev_unplug(&dev_priv->drm);
i915_gem_driver_unregister(dev_priv);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 69e43bf91a153..7e9b452c77e2b 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -61,6 +61,7 @@
#include
#include
+#include "i915_hwmon.h"
#include "i915_params.h"
#include "i915_reg.h"
#include "i915_utils.h"
@@ -1109,6 +1110,8 @@ struct drm_i915_private {
struct i915_perf perf;
+ struct i915_hwmon hwmon;
+
/* Abstract the submission mechanism (legacy ringbuffer or execlists)
away */
struct intel_gt gt;
diff --git a/drivers/gpu/drm/i915/i915_hwmon.c
b/drivers/gpu/drm/i915/i915_hwmon.c
new file mode 100644
index 0..ab8f32f7ed1de
--- /dev/null
+++ b/drivers/gpu/drm/i915/i915_hwmon.c
@@ -0,0 +1,788 @@
+// SPDX-License-Identifier: MIT
+
+/*
+ * Copyright © 2020 Intel Corporation
+ */
+
+/*
+ * Power-related hwmon entries.
+ */
+
+#include
+#include
+#include
+
+#include "i915_drv.h"
+#include "gt/intel_gt.h"
+#include "i915_hwmon.h"
+
+/*
+ * SF_* - scale factors for particular quantities.
+ * The hwmon standard says that quantities of the given types are specified
+ * in the given units:
+ * - time - milliseconds
+ * - power - microwatts
+ * - energy - microjoules
+ */
+
+#define SF_TIME 1000
+#define SF_POWER 100
+#define SF_ENERGY 100
+
+static void
+_locked_with_pm_intel_uncore_rmw(struct intel_uncore *uncore,
+i915_reg_t reg, u32 clear, u32 set)
+{
+ struct drm_i915_private *i915 = uncore->i915;
+ struct i915_hwmon *hwmon = &i915->hwmon;
+ intel_wakeref_t wakeref;
+
+ mutex_lock(&hwmon->hwmon_lock);
+
+ with_intel_runtime_pm(uncore->rpm, wakeref)
+
[Intel-gfx] [PATCH] drm/i915/dg1: Add HWMON power sensor support
As part of the System Managemenent Interface (SMI), use the HWMON
subsystem to display power utilization.
The following standard HWMON power sensors are currently supported
(and appropriately scaled):
/sys/class/drm/card0/device/hwmon/hwmon
- energy1_input
- power1_cap
- power1_max
Some non-standard HWMON power information is also provided, such as
enable bits and intervals.
Signed-off-by: Dale B Stimson
---
drivers/gpu/drm/i915/Kconfig | 1 +
drivers/gpu/drm/i915/Makefile | 1 +
drivers/gpu/drm/i915/i915_drv.c | 8 +
drivers/gpu/drm/i915/i915_drv.h | 3 +
drivers/gpu/drm/i915/i915_hwmon.c | 785 ++
drivers/gpu/drm/i915/i915_hwmon.h | 41 ++
drivers/gpu/drm/i915/i915_reg.h | 53 ++
7 files changed, 892 insertions(+)
create mode 100644 drivers/gpu/drm/i915/i915_hwmon.c
create mode 100644 drivers/gpu/drm/i915/i915_hwmon.h
diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig
index 1e1cb245fca77..ec8d5a0d7ea96 100644
--- a/drivers/gpu/drm/i915/Kconfig
+++ b/drivers/gpu/drm/i915/Kconfig
@@ -14,6 +14,7 @@ config DRM_I915
select DRM_MIPI_DSI
select RELAY
select IRQ_WORK
+ select HWMON
# i915 depends on ACPI_VIDEO when ACPI is enabled
# but for select to work, need to select ACPI_VIDEO's dependencies, ick
select BACKLIGHT_CLASS_DEVICE if ACPI
diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 14f1ab399ad08..1c110d306e493 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -37,6 +37,7 @@ i915-y += i915_drv.o \
i915_config.o \
i915_irq.o \
i915_getparam.o \
+ i915_hwmon.o \
i915_mitigations.o \
i915_params.o \
i915_pci.o \
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index b2018e85afc2d..958517e5157b4 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -69,6 +69,7 @@
#include "i915_debugfs.h"
#include "i915_drv.h"
+#include "i915_hwmon.h"
#include "i915_ioc32.h"
#include "i915_irq.h"
#include "i915_memcpy.h"
@@ -671,6 +672,10 @@ static void i915_driver_register(struct drm_i915_private
*dev_priv)
return;
}
+ /* Register with hwmon */
+ if (i915_hwmon_init(&dev_priv->drm))
+ drm_err(&dev_priv->drm, "Failed to register driver hwmon!\n");
+
i915_debugfs_register(dev_priv);
i915_setup_sysfs(dev_priv);
@@ -711,6 +716,9 @@ static void i915_driver_unregister(struct drm_i915_private
*dev_priv)
i915_pmu_unregister(dev_priv);
i915_teardown_sysfs(dev_priv);
+
+ i915_hwmon_fini(&dev_priv->drm);
+
drm_dev_unplug(&dev_priv->drm);
i915_gem_driver_unregister(dev_priv);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index fa596dace4902..7998461e2f00c 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -61,6 +61,7 @@
#include
#include
+#include "i915_hwmon.h"
#include "i915_params.h"
#include "i915_reg.h"
#include "i915_utils.h"
@@ -1108,6 +1109,8 @@ struct drm_i915_private {
struct i915_perf perf;
+ struct i915_hwmon hwmon;
+
/* Abstract the submission mechanism (legacy ringbuffer or execlists)
away */
struct intel_gt gt;
diff --git a/drivers/gpu/drm/i915/i915_hwmon.c
b/drivers/gpu/drm/i915/i915_hwmon.c
new file mode 100644
index 0..373bb937a373d
--- /dev/null
+++ b/drivers/gpu/drm/i915/i915_hwmon.c
@@ -0,0 +1,785 @@
+// SPDX-License-Identifier: MIT
+
+/*
+ * Copyright © 2020 Intel Corporation
+ */
+
+/*
+ * Power-related hwmon entries.
+ */
+
+#include
+#include
+
+#include "i915_drv.h"
+#include "gt/intel_gt.h"
+#include "i915_hwmon.h"
+
+/**
+ * SF_* - scale factors for particular quantities.
+ * The hwmon standard says that quantities of the given types are specified
+ * in the given units:
+ * - time - milliseconds
+ * - power - microwatts
+ * - energy - microjoules
+ */
+
+#define SF_TIME 1000
+#define SF_POWER 100
+#define SF_ENERGY 100
+
+static void
+_locked_with_pm_intel_uncore_rmw(struct intel_uncore *uncore,
+i915_reg_t reg, u32 clear, u32 set)
+{
+ struct drm_i915_private *i915 = uncore->i915;
+ struct i915_hwmon *hwmon = &i915->hwmon;
+ intel_wakeref_t wakeref;
+
+ mutex_lock(&hwmon->hwmon_lock);
+
+ with_intel_runtime_pm(uncore->rpm, wakeref)
+ intel_uncore_rmw(uncore, reg, clear, set);
+
+ mutex_unlock(&hwmon->hwmon_lock);
+}
+
+/*
+ * _field_read_and_scale()
+ * Return type of u64 allows for the case where
[Intel-gfx] [PATCH i-g-t v4 2/2] i915/gem_ctx_isolation: Check engine relative registers (revised)
From: Chris Wilson
Some of the non-privileged registers are at the same offset on each
engine. We can improve our coverage for unknown HW layout by using the
reported engine->mmio_base for relative offsets.
Subsequent to sign-off by Chris Wilson, added by Dale B Stimson:
Modify previous "i915/gem_ctx_isolation: Check engine relative registers"
to support alternative mmio_base infrastructure API.
Signed-off-by: Chris Wilson
Signed-off-by: Dale B Stimson
---
lib/i915/gem_mmio_base.c | 7 +-
tests/i915/gem_ctx_isolation.c | 229 -
2 files changed, 141 insertions(+), 95 deletions(-)
diff --git a/lib/i915/gem_mmio_base.c b/lib/i915/gem_mmio_base.c
index d1b83221a..c712b4431 100644
--- a/lib/i915/gem_mmio_base.c
+++ b/lib/i915/gem_mmio_base.c
@@ -286,13 +286,14 @@ struct eng_mmio_base_table_s
*gem_engine_mmio_base_info_get(int fd_dev)
{
struct eng_mmio_base_table_s *mbp = NULL;
- /* If and when better ways are provided to find the mmio_base
-* information, they may be added them here in order of preference.
+ /* If and when more desirable ways exist to find the mmio_base
+* information, they may be added here, in order of consideration.
*/
#if 0
+ /* Anticipating a future method: */
if (!mbp)
- mbp = _mmio_base_info_get_via_sysfs(fd_dev);
+ mbp = _gem_engine_mmio_base_info_get_sysfs(fd_dev);
#endif
if (!mbp)
diff --git a/tests/i915/gem_ctx_isolation.c b/tests/i915/gem_ctx_isolation.c
index 1b66fec11..07ffbb84a 100644
--- a/tests/i915/gem_ctx_isolation.c
+++ b/tests/i915/gem_ctx_isolation.c
@@ -70,6 +70,7 @@ static const struct named_register {
uint32_t ignore_bits;
uint32_t write_mask; /* some registers bits do not exist */
bool masked;
+ bool relative;
} nonpriv_registers[] = {
{ "NOPID", NOCTX, RCS0, 0x2094 },
{ "MI_PREDICATE_RESULT_2", NOCTX, RCS0, 0x23bc },
@@ -109,7 +110,6 @@ static const struct named_register {
{ "PS_DEPTH_COUNT_1", GEN8, RCS0, 0x22f8, 2 },
{ "BB_OFFSET", GEN8, RCS0, 0x2158, .ignore_bits = 0x7 },
{ "MI_PREDICATE_RESULT_1", GEN8, RCS0, 0x241c },
- { "CS_GPR", GEN8, RCS0, 0x2600, 32 },
{ "OA_CTX_CONTROL", GEN8, RCS0, 0x2360 },
{ "OACTXID", GEN8, RCS0, 0x2364 },
{ "PS_INVOCATION_COUNT_2", GEN8, RCS0, 0x2448, 2, .write_mask = ~0x3 },
@@ -138,79 +138,56 @@ static const struct named_register {
{ "CTX_PREEMPT", NOCTX /* GEN10 */, RCS0, 0x2248 },
{ "CS_CHICKEN1", GEN11, RCS0, 0x2580, .masked = true },
- { "HDC_CHICKEN1", GEN_RANGE(10, 10), RCS0, 0x7304, .masked = true },
/* Privileged (enabled by w/a + FORCE_TO_NONPRIV) */
{ "CTX_PREEMPT", NOCTX /* GEN9 */, RCS0, 0x2248 },
{ "CS_CHICKEN1", GEN_RANGE(9, 10), RCS0, 0x2580, .masked = true },
{ "COMMON_SLICE_CHICKEN2", GEN_RANGE(9, 9), RCS0, 0x7014, .masked =
true },
- { "HDC_CHICKEN1", GEN_RANGE(9, 9), RCS0, 0x7304, .masked = true },
+ { "HDC_CHICKEN1", GEN_RANGE(9, 10), RCS0, 0x7304, .masked = true },
{ "SLICE_COMMON_ECO_CHICKEN1", GEN_RANGE(11, 11) /* + glk */, RCS0,
0x731c, .masked = true },
{ "L3SQREG4", NOCTX /* GEN9:skl,kbl */, RCS0, 0xb118, .write_mask =
~0x10 },
{ "HALF_SLICE_CHICKEN7", GEN_RANGE(11, 11), RCS0, 0xe194, .masked =
true },
{ "SAMPLER_MODE", GEN_RANGE(11, 11), RCS0, 0xe18c, .masked = true },
- { "BCS_GPR", GEN9, BCS0, 0x22600, 32 },
{ "BCS_SWCTRL", GEN8, BCS0, 0x22200, .write_mask = 0x3, .masked = true
},
{ "MFC_VDBOX1", NOCTX, VCS0, 0x12800, 64 },
{ "MFC_VDBOX2", NOCTX, VCS1, 0x1c800, 64 },
- { "VCS0_GPR", GEN_RANGE(9, 10), VCS0, 0x12600, 32 },
- { "VCS1_GPR", GEN_RANGE(9, 10), VCS1, 0x1c600, 32 },
- { "VECS_GPR", GEN_RANGE(9, 10), VECS0, 0x1a600, 32 },
-
- { "VCS0_GPR", GEN11, VCS0, 0x1c0600, 32 },
- { "VCS1_GPR", GEN11, VCS1, 0x1c4600, 32 },
- { "VCS2_GPR", GEN11, VCS2, 0x1d0600, 32 },
- { "VCS3_GPR", GEN11, VCS3, 0x1d4600, 32 },
- { "VECS_GPR", GEN11, VECS0, 0x1c8600, 32 },
+ { "xCS_GPR", GEN9, ALL, 0x600, 32, .relative = true },
{}
}, ignore_registers[] = {
{ "RCS timestamp", GEN6, ~0u, 0x2358 },
{ "BCS timestamp", GEN7, ~0u, 0x22358 },
- { "VCS0 timestamp", GEN_RANGE(7, 10), ~0u, 0x12358 },
- { "VCS1 timestamp", GEN_RANGE(7, 10), ~0u, 0x1c358 },
- { "VECS timestamp", GEN_RANGE(8, 10), ~0u, 0x1a358 },
-
- { "VCS0 time
[Intel-gfx] [PATCH i-g-t v4 1/2] i915/gem_mmio_base.c - get mmio_base from debugfs (if possible)
Signed-off-by: Dale B Stimson
---
lib/Makefile.sources | 2 +
lib/i915/gem_mmio_base.c | 353 +++
lib/i915/gem_mmio_base.h | 19 +++
lib/igt.h| 1 +
lib/meson.build | 1 +
5 files changed, 376 insertions(+)
create mode 100644 lib/i915/gem_mmio_base.c
create mode 100644 lib/i915/gem_mmio_base.h
diff --git a/lib/Makefile.sources b/lib/Makefile.sources
index 3e573f267..4c5d50d5d 100644
--- a/lib/Makefile.sources
+++ b/lib/Makefile.sources
@@ -7,6 +7,8 @@ lib_source_list = \
i915/gem_context.h \
i915/gem_engine_topology.c \
i915/gem_engine_topology.h \
+ i915/gem_mmio_base.c\
+ i915/gem_mmio_base.h\
i915/gem_scheduler.c\
i915/gem_scheduler.h\
i915/gem_submission.c \
diff --git a/lib/i915/gem_mmio_base.c b/lib/i915/gem_mmio_base.c
new file mode 100644
index 0..d1b83221a
--- /dev/null
+++ b/lib/i915/gem_mmio_base.c
@@ -0,0 +1,353 @@
+// Copyright (C) 2020 Intel Corporation
+//
+// SPDX-License-Identifier: MIT
+
+#include
+
+#include
+
+#include "igt.h"
+
+struct eng_mmio_base_s {
+ char name[8];
+ uint32_t mmio_base;
+};
+
+struct eng_mmio_base_table_s {
+ unsigned int mb_cnt;
+ struct eng_mmio_base_s mb_tab[GEM_MAX_ENGINES];
+};
+
+
+static struct eng_mmio_base_table_s *_gem_engine_mmio_info_dup(
+ const struct eng_mmio_base_table_s *mbpi)
+{
+ struct eng_mmio_base_table_s *mbpo;
+ size_t nbytes;
+
+ nbytes = offsetof(typeof(struct eng_mmio_base_table_s),
mb_tab[mbpi->mb_cnt]);
+ mbpo = malloc(nbytes);
+ igt_assert(mbpo);
+ memcpy(mbpo, mbpi, nbytes);
+
+ return mbpo;
+}
+
+void gem_engine_mmio_base_info_free(struct eng_mmio_base_table_s *mbp)
+{
+ if (mbp)
+ free(mbp);
+}
+
+static void _gem_engine_mmio_info_legacy_add(struct eng_mmio_base_table_s *mbp,
+ const char *eng_name, uint32_t mmio_base)
+{
+ if (mmio_base) {
+ strncpy(mbp->mb_tab[mbp->mb_cnt].name, eng_name,
+ sizeof(mbp->mb_tab[0].name));
+ mbp->mb_tab[mbp->mb_cnt].mmio_base = mmio_base;
+ mbp->mb_cnt++;
+ }
+}
+
+/**
+ * _gem_engine_mmio_base_info_get_legacy:
+ * @fd_dev: file descriptor upon which device is open or -1 to use defaults.
+ *
+ * Provides per-engine mmio_base information from legacy built-in values
+ * for the case when the information is not otherwise available.
+ *
+ * Returns:
+ * Pointer to dynamically allocated struct eng_mmio_base_table_s describing
+ * engine config or NULL.
+ * The allocated size does not include unused engine entries.
+ * If non-NULL, it is caller's responsibility to free.
+ */
+static struct eng_mmio_base_table_s *_gem_engine_mmio_base_info_get_legacy(int
fd_dev)
+{
+ int gen;
+ uint32_t mmio_base;
+ struct eng_mmio_base_table_s mbt;
+ struct eng_mmio_base_table_s *mbp;
+
+ memset(&mbt, 0, sizeof(mbt));
+
+ gen = intel_gen(intel_get_drm_devid(fd_dev));
+
+ /* The mmio_base values for engine instances 1 and higher cannot
+* be reliability determinated a priori. */
+
+ _gem_engine_mmio_info_legacy_add(&mbt, "rcs0", 0x2000);
+ _gem_engine_mmio_info_legacy_add(&mbt, "bcs0", 0x22000);
+
+ if (gen < 6)
+ mmio_base = 0x4000;
+ else if (gen < 11)
+ mmio_base = 0x12000;
+ else
+ mmio_base = 0x1c;
+ _gem_engine_mmio_info_legacy_add(&mbt, "vcs0", mmio_base);
+
+ if (gen < 11)
+ mmio_base = 0x1a000;
+ else
+ mmio_base = 0x1c8000;
+ _gem_engine_mmio_info_legacy_add(&mbt, "vecs0", mmio_base);
+
+ if (mbt.mb_cnt <= 0)
+ return NULL;
+
+ mbp = _gem_engine_mmio_info_dup(&mbt);
+
+ return mbp;
+}
+
+
+/**
+ * _gem_engine_mmio_base_info_get_debugfs:
+ * @fd_dev: file descriptor upon which device is open or -1 to use defaults.
+ *
+ * Obtains per-engine mmio_base information from debugfs.
+ *
+ * Returns:
+ * Pointer to dynamically allocated struct eng_mmio_base_table_s describing
+ * engine config or NULL.
+ * The allocated size does not include unused engine entries.
+ * If non-NULL, it is caller's responsibility to free.
+ *
+ * Looking in debugfs for per-engine instances of:
+ *
+ * ...
+ * MMIO base:
+ *
+ * Example of relevant lines from debugfs:
+ * vcs0
+ * MMIO base: 0x001c
+ * vcs1
+ * MMIO base: 0x001d
+ *
+ * In order to qualify as the introduction of a new per-engine section, an
+ * input line must consist solely of an engine name. An engine name must
+ * be 7 or fewer characters in length and must consist of an engine class
[Intel-gfx] [RFC i-g-t] lib/i915/gem_mmio_base.c - add support for mmio_base via sysfs
Obtaining mmio_base info via sysfs (if available). This is in addition
to the already existing support for obtaining that data via debugfs.
The use of sysfs data is preferred.
This patch depends upon the following proposed IGT patch, which is not
presently merged:
i915/gem_mmio_base.c - get mmio_base from debugfs (if possible)
The availability of sysfs info for mmio_base depends on the presence of
these two proposed kernel patches, not presently merged:
drm/i915/gt: Expose engine properties via sysfs
drm/i915/gt: Expose engine->mmio_base via sysfs
Signed-off-by: Dale B Stimson
---
lib/i915/gem_mmio_base.c | 79
1 file changed, 79 insertions(+)
diff --git a/lib/i915/gem_mmio_base.c b/lib/i915/gem_mmio_base.c
index 514f87f42..8a29a9150 100644
--- a/lib/i915/gem_mmio_base.c
+++ b/lib/i915/gem_mmio_base.c
@@ -5,8 +5,11 @@
#include
#include
+#include
+#include
#include "igt.h"
+#include "igt_sysfs.h"
struct eng_mmio_base_s {
char name[8];
@@ -102,6 +105,82 @@ static struct eng_mmio_base_table_s
*_gem_engine_mmio_base_info_get_legacy(int f
return mbp;
}
+static struct eng_mmio_base_table_s *_gem_engine_mmio_base_info_get_sysfs(int
fd_dev)
+{
+ struct eng_mmio_base_table_s mbt;
+ struct eng_mmio_base_table_s *mbp = NULL;
+ const size_t name_max = sizeof(mbt.mb_tab[0].name);
+ int fd_top = -1;
+ int fd_engines = -1;
+ int fd_this;
+ int fd_mmio_base;
+ DIR *dirp = NULL;
+ struct dirent *pde;
+ const char *eng_name;
+ ssize_t nb;
+ char rbuf[32];
+ unsigned long int ulv;
+ uint32_t uiv;
+
+ memset(&mbt, 0, sizeof(mbt));
+
+ fd_top = igt_sysfs_open(fd_dev);
+ if (fd_top < 0)
+ goto mbsf_cleanup;
+
+ fd_engines = openat(fd_top, "engine", O_RDONLY);
+ if (fd_engines < 0)
+ goto mbsf_cleanup;
+
+ dirp = fdopendir(fd_engines);
+ if (!dirp)
+ goto mbsf_cleanup;
+
+ for (;;) {
+ pde = readdir(dirp);
+ if (!pde)
+ break;
+
+ if ((pde->d_type == DT_DIR) || (pde->d_type == DT_UNKNOWN)) {
+ eng_name = pde->d_name;
+ if (eng_name[0] == '.')
+ continue;
+ fd_this = openat(fd_engines, eng_name, O_RDONLY);
+ if (fd_this < 0)
+ continue;
+ fd_mmio_base = openat(fd_this, "mmio_base", O_RDONLY);
+ close(fd_this);
+ if (fd_mmio_base < 0)
+ goto mbsf_cleanup;
+ nb = read(fd_mmio_base, rbuf, sizeof(rbuf));
+ close(fd_mmio_base);
+ if (nb > 0) {
+ ulv = strtoul(rbuf, NULL, 16);
+ uiv = (uint32_t) ulv;
+
+ strncpy(mbt.mb_tab[mbt.mb_cnt].name,
+ eng_name, name_max);
+ mbt.mb_tab[mbt.mb_cnt].mmio_base = uiv;
+ mbt.mb_cnt++;
+ }
+ }
+ }
+
+ if (mbt.mb_cnt > 0)
+ mbp = _gem_engine_mmio_info_dup(&mbt);
+
+mbsf_cleanup:
+ if (dirp)
+ closedir(dirp);
+
+ if (fd_engines >= 0)
+ close(fd_engines);
+ if (fd_top >= 0)
+ close(fd_top);
+
+ return mbp;
+}
+
/**
* _gem_engine_mmio_base_info_get_debugfs:
--
2.25.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t v4 0/2] mmio_base via debugfs infrastructure + gem_ctx_isolation
v4: Functionally identical to V3. - Commentary - name of potential future method for getting info from sysfs changed from _mmio_base_info_get_via_sysfs() to _gem_engine_mmio_base_info_get_sysfs(). - Patches squashed in order to avoid compilation break in between, from: i915/gem_ctx_isolation: Check engine relative registers i915/gem_ctx_isolation: Check engine relative registers - Part 2 to: i915/gem_ctx_isolation: Check engine relative registers (revised) - The narrative below has been modified. For the record, a separate patch exists that provides support for the proposed sysfs source of mmio_base information via new function _gem_engine_mmio_base_info_get_sysfs(). It depends on this patch series. It is planned to send it to the ML separately. lib/i915/gem_mmio_base.c - add support for mmio_base via sysfs v3: - Remove v2 early-exit patches (previous 4/5 and 5/5). The underlying issue was addressed via a separately-submitted patch. v2: - Introduce and use igt_exit_early() so that a failed initialization (in igt_fixture) will not attempt to invoke the per-engine loop. - Initialize mmio_base db inside initial igt_fixture instead of after. - Some additional functions handle NULL input mmio_base db pointer. - Variables mbp and mmio_base initialized to NULL/0 so their values will not be uninitialized for --list-subtests. - Failure to obtain an mmio_base db is now igt_debug instead of igt_warn. This patch series provides infrastructure to allow determination of i915 per-engine mmio_base (which is otherwise sometimes hard to get). The provided method uses debugfs mmio_base information if present. Otherwise, a default determination is provided when possible. Also, gem_ctx_isolation is modified to use the new infrastructure. For example, on TGL, gem_ctx_isolation (without these or similar changes) will fail for subtests that use engine vcs1. On 2020-01-27 (most recently), Chris Wilson sent this patch series to the ML: [igt-dev] [PATCH i-g-t 1/5] i915: Start putting the mmio_base to wider use [igt-dev] [PATCH i-g-t 2/5] i915/gem_ctx_isolation: Check engine relative registers plus the following, which are not addressed here: [igt-dev] [PATCH i-g-t 3/5] i915: Exercise preemption timeout controls in sysfs [igt-dev] [PATCH i-g-t 4/5] i915: Exercise sysfs heartbeat controls [igt-dev] [PATCH i-g-t 5/5] i915: Exercise timeslice sysfs property This patch series is: i915/gem_mmio_base.c - get mmio_base from debugfs (if possible) i915/gem_ctx_isolation: Check engine relative registers (revised) The first patch of the earlier series obtains mmio_base information via sysfs, and depends on a proposed kernel change that would provide the mmio_base information via sysfs. The mmio_base information used by this patch series is available through debugfs now (as of kernel 5.4). If the per-engine mmio_base information is ever added to sysfs, a patch is already available to plug that into the infrastructure proposed here as an additional higher-priority source of that information. As compared to the original patch series, this submission replaces the first patch and retains the second patch with modifications to support the changed API. Some differences from the 2020-01-27 patches: The mmio_base information is fetched once into local data structures, and is obtained from them thereafter instead of being fetched from the kernel everytime it is wanted. The function that obtains the mmio_base information is called by a particular test that wants it, and returns a handle through which the mmio_base can be requested for any particular engine. These patches introduce new source files lib/i915/gem_mmio_base.c and lib/i915/gem_mmio_base.h. The 2020-01-27 patch series define function gem_engine_mmio_base() with its first parameter as "fd". The new patch series replace the first parameter with the mmio_base object handle. Chris Wilson (1): i915/gem_ctx_isolation: Check engine relative registers (revised) Dale B Stimson (1): i915/gem_mmio_base.c - get mmio_base from debugfs (if possible) lib/Makefile.sources | 2 + lib/i915/gem_mmio_base.c | 354 + lib/i915/gem_mmio_base.h | 19 ++ lib/igt.h | 1 + lib/meson.build| 1 + tests/i915/gem_ctx_isolation.c | 229 - 6 files changed, 514 insertions(+), 92 deletions(-) create mode 100644 lib/i915/gem_mmio_base.c create mode 100644 lib/i915/gem_mmio_base.h -- 2.25.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t v3 2/3] i915/gem_ctx_isolation: Check engine relative registers
From: Chris Wilson
Some of the non-privileged registers are at the same offset on each
engine. We can improve our coverage for unknown HW layout by using the
reported engine->mmio_base for relative offsets.
Signed-off-by: Chris Wilson
Reviewed-by: Dale B Stimson
---
tests/i915/gem_ctx_isolation.c | 164 -
1 file changed, 100 insertions(+), 64 deletions(-)
diff --git a/tests/i915/gem_ctx_isolation.c b/tests/i915/gem_ctx_isolation.c
index 1b66fec11..eff4b1df2 100644
--- a/tests/i915/gem_ctx_isolation.c
+++ b/tests/i915/gem_ctx_isolation.c
@@ -70,6 +70,7 @@ static const struct named_register {
uint32_t ignore_bits;
uint32_t write_mask; /* some registers bits do not exist */
bool masked;
+ bool relative;
} nonpriv_registers[] = {
{ "NOPID", NOCTX, RCS0, 0x2094 },
{ "MI_PREDICATE_RESULT_2", NOCTX, RCS0, 0x23bc },
@@ -109,7 +110,6 @@ static const struct named_register {
{ "PS_DEPTH_COUNT_1", GEN8, RCS0, 0x22f8, 2 },
{ "BB_OFFSET", GEN8, RCS0, 0x2158, .ignore_bits = 0x7 },
{ "MI_PREDICATE_RESULT_1", GEN8, RCS0, 0x241c },
- { "CS_GPR", GEN8, RCS0, 0x2600, 32 },
{ "OA_CTX_CONTROL", GEN8, RCS0, 0x2360 },
{ "OACTXID", GEN8, RCS0, 0x2364 },
{ "PS_INVOCATION_COUNT_2", GEN8, RCS0, 0x2448, 2, .write_mask = ~0x3 },
@@ -138,79 +138,56 @@ static const struct named_register {
{ "CTX_PREEMPT", NOCTX /* GEN10 */, RCS0, 0x2248 },
{ "CS_CHICKEN1", GEN11, RCS0, 0x2580, .masked = true },
- { "HDC_CHICKEN1", GEN_RANGE(10, 10), RCS0, 0x7304, .masked = true },
/* Privileged (enabled by w/a + FORCE_TO_NONPRIV) */
{ "CTX_PREEMPT", NOCTX /* GEN9 */, RCS0, 0x2248 },
{ "CS_CHICKEN1", GEN_RANGE(9, 10), RCS0, 0x2580, .masked = true },
{ "COMMON_SLICE_CHICKEN2", GEN_RANGE(9, 9), RCS0, 0x7014, .masked =
true },
- { "HDC_CHICKEN1", GEN_RANGE(9, 9), RCS0, 0x7304, .masked = true },
+ { "HDC_CHICKEN1", GEN_RANGE(9, 10), RCS0, 0x7304, .masked = true },
{ "SLICE_COMMON_ECO_CHICKEN1", GEN_RANGE(11, 11) /* + glk */, RCS0,
0x731c, .masked = true },
{ "L3SQREG4", NOCTX /* GEN9:skl,kbl */, RCS0, 0xb118, .write_mask =
~0x10 },
{ "HALF_SLICE_CHICKEN7", GEN_RANGE(11, 11), RCS0, 0xe194, .masked =
true },
{ "SAMPLER_MODE", GEN_RANGE(11, 11), RCS0, 0xe18c, .masked = true },
- { "BCS_GPR", GEN9, BCS0, 0x22600, 32 },
{ "BCS_SWCTRL", GEN8, BCS0, 0x22200, .write_mask = 0x3, .masked = true
},
{ "MFC_VDBOX1", NOCTX, VCS0, 0x12800, 64 },
{ "MFC_VDBOX2", NOCTX, VCS1, 0x1c800, 64 },
- { "VCS0_GPR", GEN_RANGE(9, 10), VCS0, 0x12600, 32 },
- { "VCS1_GPR", GEN_RANGE(9, 10), VCS1, 0x1c600, 32 },
- { "VECS_GPR", GEN_RANGE(9, 10), VECS0, 0x1a600, 32 },
-
- { "VCS0_GPR", GEN11, VCS0, 0x1c0600, 32 },
- { "VCS1_GPR", GEN11, VCS1, 0x1c4600, 32 },
- { "VCS2_GPR", GEN11, VCS2, 0x1d0600, 32 },
- { "VCS3_GPR", GEN11, VCS3, 0x1d4600, 32 },
- { "VECS_GPR", GEN11, VECS0, 0x1c8600, 32 },
+ { "xCS_GPR", GEN9, ALL, 0x600, 32, .relative = true },
{}
}, ignore_registers[] = {
{ "RCS timestamp", GEN6, ~0u, 0x2358 },
{ "BCS timestamp", GEN7, ~0u, 0x22358 },
- { "VCS0 timestamp", GEN_RANGE(7, 10), ~0u, 0x12358 },
- { "VCS1 timestamp", GEN_RANGE(7, 10), ~0u, 0x1c358 },
- { "VECS timestamp", GEN_RANGE(8, 10), ~0u, 0x1a358 },
-
- { "VCS0 timestamp", GEN11, ~0u, 0x1c0358 },
- { "VCS1 timestamp", GEN11, ~0u, 0x1c4358 },
- { "VCS2 timestamp", GEN11, ~0u, 0x1d0358 },
- { "VCS3 timestamp", GEN11, ~0u, 0x1d4358 },
- { "VECS timestamp", GEN11, ~0u, 0x1c8358 },
+ { "xCS timestamp", GEN8, ALL, 0x358, .relative = true },
/* huc read only */
- { "BSD0 0x2000", GEN11, ~0u, 0x1c + 0x2000 },
- { "BSD0 0x2000", GEN11, ~0u, 0x1c + 0x2014 },
- { "BSD0 0x2000", GEN11, ~0u, 0x1c + 0x23b0 },
-
- { "BSD1 0x2000", GEN11, ~0u, 0x1c4000 + 0x2000 },
- { "BSD1 0x2000", GEN11, ~0u, 0x1c4000 + 0x2014 },
- { "BSD1 0x2000", GEN11, ~0u, 0x1c4000 + 0x23b0 },
-
- { "BSD2 0x2000", GEN11, ~0u, 0x1d + 0x2000 },
- { "BSD2 0x2000", GEN11, ~0u, 0x1d + 0x2014 },
- { "BSD2 0x2000", GEN11, ~0u, 0x1d + 0x23b0 },
-
- { "BSD3 0x2000", GEN11, ~0u, 0x1d4000 + 0x2000 },
- { "BSD
[Intel-gfx] [PATCH i-g-t v3 0/3] mmio_base via debugfs infrastructure + gem_ctx_isolation
v3: - Remove v2 early-exit patches (previous 4/5 and 5/5). The underlying issue was addressed via a separately-submitted patch. v2: - Introduce and use igt_exit_early() so that a failed initialization (in igt_fixture) will not attempt to invoke the per-engine loop. - Initialize mmio_base db inside initial igt_fixture instead of after. - Some additional functions handle NULL input mmio_base db pointer. - Variables mbp and mmio_base initialized to NULL/0 so their values will not be uninitialized for --list-subtests. - Failure to obtain an mmio_base db is now igt_debug instead of igt_warn. This patch series provides infrastructure to allow determination of i915 per-engine mmio_base (which is otherwise sometimes hard to get). The provided method uses debugfs mmio_base information if present. Otherwise, a default determination is provided when possible. Also, gem_ctx_isolation is modified to use the new infrastructure. For example, on TGL, gem_ctx_isolation (without these or similar changes) will fail for subtests that use engine vcs1. The patches in this series are as they are intended to be submitted (subject to comments), except I would like to squash the two gem_ctx_isolation "relative registers" patches into one (as discussed below). Also, function gem_engine_mmio_base_info_dump() could be removed. On 2020-01-27, Chris wilson sent to the ML: [igt-dev] [PATCH i-g-t 1/5] i915: Start putting the mmio_base to wider use [igt-dev] [PATCH i-g-t 2/5] i915/gem_ctx_isolation: Check engine relative registers plus the following, which are not addressed here: [igt-dev] [PATCH i-g-t 3/5] i915: Exercise preemption timeout controls in sysfs [igt-dev] [PATCH i-g-t 4/5] i915: Exercise sysfs heartbeat controls [igt-dev] [PATCH i-g-t 5/5] i915: Exercise timeslice sysfs property This patch list is: i915/gem_mmio_base.c - get mmio_base from debugfs (if possible) i915/gem_ctx_isolation: Check engine relative registers i915/gem_ctx_isolation: Check engine relative registers - Part 2 The first 2020-01-27 patch obtains mmio_base information via sysfs, and depends on a proposed kernel change that would provide the mmio_base information via sysfs. It is unclear when or whether that kernel change will progress. The mmio_base information used by this patch series is available through debugfs now (as of kernel 5.4). If the per-engine mmio_base information is ever added to sysfs, it would be easy to plug that into the infrastructure proposed here as an additional higher-priority source of that information. This submission replaces the first patch (switching from sysfs to debugfs), retains the second 2020-01-27 patch i915/gem_ctx_isolation: Check engine relative registers and has a third patch that modifies the original second patch to support the altered API: i915/gem_ctx_isolation: Check engine relative registers - Part 2 I propose squashing the two gem_ctx_isolation "relative registers" patches into one patch with author == "Chris Wilson" if Chris agrees. Some differences from the 2020-01-27 patches: The mmio_base information is fetched once into local data structures, and is obtained from them thereafter instead of being fetched from the kernel everytime it is wanted. The function that obtains the mmio_base information is called by a particular test that wants it, and returns a handle through which the mmio_base can be requested for any particular engine. These patches introduce new source files lib/i915/gem_mmio_base.c and lib/i915/gem_mmio_base.h. Should the code instead be placed into lib/i915/gem_engine_topology.c? Function gem_engine_mmio_base_info_dump presently exists to dump the mmio_base information to stdout for debugging or informational purposes. This function is not currently called. I presume this function should be removed. Is there any desire to keep it around for future use? The 2020-01-27 patches define function gem_engine_mmio_base() with its first parameter as "fd". The new patches replace the first parameter with the mmio_base object handle. Chris Wilson (1): i915/gem_ctx_isolation: Check engine relative registers Dale B Stimson (2): i915/gem_mmio_base.c - get mmio_base from debugfs (if possible) i915/gem_ctx_isolation: Check engine relative registers - Part 2 lib/Makefile.sources | 2 + lib/i915/gem_mmio_base.c | 353 + lib/i915/gem_mmio_base.h | 19 ++ lib/igt.h | 1 + lib/meson.build| 1 + tests/i915/gem_ctx_isolation.c | 229 - 6 files changed, 513 insertions(+), 92 deletions(-) create mode 100644 lib/i915/gem_mmio_base.c create mode 100644 lib/i915/gem_mmio_base.h -- 2.25.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t v3 3/3] i915/gem_ctx_isolation: Check engine relative registers - Part 2
Modify previous i915/gem_ctx_isolation "Check engine relative registers"
for modified mmio_base infrastructure.
Signed-off-by: Dale B Stimson
---
tests/i915/gem_ctx_isolation.c | 87 +++---
1 file changed, 48 insertions(+), 39 deletions(-)
diff --git a/tests/i915/gem_ctx_isolation.c b/tests/i915/gem_ctx_isolation.c
index eff4b1df2..07ffbb84a 100644
--- a/tests/i915/gem_ctx_isolation.c
+++ b/tests/i915/gem_ctx_isolation.c
@@ -233,12 +233,12 @@ static bool ignore_register(uint32_t offset, uint32_t
mmio_base)
static void tmpl_regs(int fd,
uint32_t ctx,
const struct intel_execution_engine2 *e,
+ uint32_t mmio_base,
uint32_t handle,
uint32_t value)
{
const unsigned int gen_bit = 1 << intel_gen(intel_get_drm_devid(fd));
const unsigned int engine_bit = ENGINE(e->class, e->instance);
- const uint32_t mmio_base = gem_engine_mmio_base(fd, e->name);
unsigned int regs_size;
uint32_t *regs;
@@ -278,12 +278,12 @@ static void tmpl_regs(int fd,
static uint32_t read_regs(int fd,
uint32_t ctx,
const struct intel_execution_engine2 *e,
+ uint32_t mmio_base,
unsigned int flags)
{
const unsigned int gen = intel_gen(intel_get_drm_devid(fd));
const unsigned int gen_bit = 1 << gen;
const unsigned int engine_bit = ENGINE(e->class, e->instance);
- const uint32_t mmio_base = gem_engine_mmio_base(fd, e->name);
const bool r64b = gen >= 8;
struct drm_i915_gem_exec_object2 obj[2];
struct drm_i915_gem_relocation_entry *reloc;
@@ -359,12 +359,12 @@ static uint32_t read_regs(int fd,
static void write_regs(int fd,
uint32_t ctx,
const struct intel_execution_engine2 *e,
+ uint32_t mmio_base,
unsigned int flags,
uint32_t value)
{
const unsigned int gen_bit = 1 << intel_gen(intel_get_drm_devid(fd));
const unsigned int engine_bit = ENGINE(e->class, e->instance);
- const uint32_t mmio_base = gem_engine_mmio_base(fd, e->name);
struct drm_i915_gem_exec_object2 obj;
struct drm_i915_gem_execbuffer2 execbuf;
unsigned int batch_size;
@@ -420,13 +420,13 @@ static void write_regs(int fd,
static void restore_regs(int fd,
uint32_t ctx,
const struct intel_execution_engine2 *e,
+uint32_t mmio_base,
unsigned int flags,
uint32_t regs)
{
const unsigned int gen = intel_gen(intel_get_drm_devid(fd));
const unsigned int gen_bit = 1 << gen;
const unsigned int engine_bit = ENGINE(e->class, e->instance);
- const uint32_t mmio_base = gem_engine_mmio_base(fd, e->name);
const bool r64b = gen >= 8;
struct drm_i915_gem_exec_object2 obj[2];
struct drm_i915_gem_execbuffer2 execbuf;
@@ -498,12 +498,12 @@ static void restore_regs(int fd,
__attribute__((unused))
static void dump_regs(int fd,
const struct intel_execution_engine2 *e,
+ uint32_t mmio_base,
unsigned int regs)
{
const int gen = intel_gen(intel_get_drm_devid(fd));
const unsigned int gen_bit = 1 << gen;
const unsigned int engine_bit = ENGINE(e->class, e->instance);
- const uint32_t mmio_base = gem_engine_mmio_base(fd, e->name);
unsigned int regs_size;
uint32_t *out;
@@ -541,9 +541,9 @@ static void dump_regs(int fd,
}
static void compare_regs(int fd, const struct intel_execution_engine2 *e,
+uint32_t mmio_base,
uint32_t A, uint32_t B, const char *who)
{
- const uint32_t mmio_base = gem_engine_mmio_base(fd, e->name);
unsigned int num_errors;
unsigned int regs_size;
uint32_t *a, *b;
@@ -596,6 +596,7 @@ static void compare_regs(int fd, const struct
intel_execution_engine2 *e,
static void nonpriv(int fd,
const struct intel_execution_engine2 *e,
+ uint32_t mmio_base,
unsigned int flags)
{
static const uint32_t values[] = {
@@ -623,16 +624,16 @@ static void nonpriv(int fd,
ctx = gem_context_clone_with_engines(fd, 0);
- tmpl = read_regs(fd, ctx, e, flags);
- regs[0] = read_regs(fd, ctx, e, flags);
+ tmpl = read_regs(fd, ctx, e, mmio_base, flags);
+ regs[0] = read_regs(fd, ctx, e, mmio_base, flags);
- tmpl_regs(fd, ctx, e, tmpl, values[v]);
+ tmpl_regs(fd, ctx, e, mmio_ba
[Intel-gfx] [PATCH i-g-t v3 1/3] i915/gem_mmio_base.c - get mmio_base from debugfs (if possible)
Signed-off-by: Dale B Stimson
---
lib/Makefile.sources | 2 +
lib/i915/gem_mmio_base.c | 353 +++
lib/i915/gem_mmio_base.h | 19 +++
lib/igt.h| 1 +
lib/meson.build | 1 +
5 files changed, 376 insertions(+)
create mode 100644 lib/i915/gem_mmio_base.c
create mode 100644 lib/i915/gem_mmio_base.h
diff --git a/lib/Makefile.sources b/lib/Makefile.sources
index 3e573f267..4c5d50d5d 100644
--- a/lib/Makefile.sources
+++ b/lib/Makefile.sources
@@ -7,6 +7,8 @@ lib_source_list = \
i915/gem_context.h \
i915/gem_engine_topology.c \
i915/gem_engine_topology.h \
+ i915/gem_mmio_base.c\
+ i915/gem_mmio_base.h\
i915/gem_scheduler.c\
i915/gem_scheduler.h\
i915/gem_submission.c \
diff --git a/lib/i915/gem_mmio_base.c b/lib/i915/gem_mmio_base.c
new file mode 100644
index 0..d1b83221a
--- /dev/null
+++ b/lib/i915/gem_mmio_base.c
@@ -0,0 +1,353 @@
+// Copyright (C) 2020 Intel Corporation
+//
+// SPDX-License-Identifier: MIT
+
+#include
+
+#include
+
+#include "igt.h"
+
+struct eng_mmio_base_s {
+ char name[8];
+ uint32_t mmio_base;
+};
+
+struct eng_mmio_base_table_s {
+ unsigned int mb_cnt;
+ struct eng_mmio_base_s mb_tab[GEM_MAX_ENGINES];
+};
+
+
+static struct eng_mmio_base_table_s *_gem_engine_mmio_info_dup(
+ const struct eng_mmio_base_table_s *mbpi)
+{
+ struct eng_mmio_base_table_s *mbpo;
+ size_t nbytes;
+
+ nbytes = offsetof(typeof(struct eng_mmio_base_table_s),
mb_tab[mbpi->mb_cnt]);
+ mbpo = malloc(nbytes);
+ igt_assert(mbpo);
+ memcpy(mbpo, mbpi, nbytes);
+
+ return mbpo;
+}
+
+void gem_engine_mmio_base_info_free(struct eng_mmio_base_table_s *mbp)
+{
+ if (mbp)
+ free(mbp);
+}
+
+static void _gem_engine_mmio_info_legacy_add(struct eng_mmio_base_table_s *mbp,
+ const char *eng_name, uint32_t mmio_base)
+{
+ if (mmio_base) {
+ strncpy(mbp->mb_tab[mbp->mb_cnt].name, eng_name,
+ sizeof(mbp->mb_tab[0].name));
+ mbp->mb_tab[mbp->mb_cnt].mmio_base = mmio_base;
+ mbp->mb_cnt++;
+ }
+}
+
+/**
+ * _gem_engine_mmio_base_info_get_legacy:
+ * @fd_dev: file descriptor upon which device is open or -1 to use defaults.
+ *
+ * Provides per-engine mmio_base information from legacy built-in values
+ * for the case when the information is not otherwise available.
+ *
+ * Returns:
+ * Pointer to dynamically allocated struct eng_mmio_base_table_s describing
+ * engine config or NULL.
+ * The allocated size does not include unused engine entries.
+ * If non-NULL, it is caller's responsibility to free.
+ */
+static struct eng_mmio_base_table_s *_gem_engine_mmio_base_info_get_legacy(int
fd_dev)
+{
+ int gen;
+ uint32_t mmio_base;
+ struct eng_mmio_base_table_s mbt;
+ struct eng_mmio_base_table_s *mbp;
+
+ memset(&mbt, 0, sizeof(mbt));
+
+ gen = intel_gen(intel_get_drm_devid(fd_dev));
+
+ /* The mmio_base values for engine instances 1 and higher cannot
+* be reliability determinated a priori. */
+
+ _gem_engine_mmio_info_legacy_add(&mbt, "rcs0", 0x2000);
+ _gem_engine_mmio_info_legacy_add(&mbt, "bcs0", 0x22000);
+
+ if (gen < 6)
+ mmio_base = 0x4000;
+ else if (gen < 11)
+ mmio_base = 0x12000;
+ else
+ mmio_base = 0x1c;
+ _gem_engine_mmio_info_legacy_add(&mbt, "vcs0", mmio_base);
+
+ if (gen < 11)
+ mmio_base = 0x1a000;
+ else
+ mmio_base = 0x1c8000;
+ _gem_engine_mmio_info_legacy_add(&mbt, "vecs0", mmio_base);
+
+ if (mbt.mb_cnt <= 0)
+ return NULL;
+
+ mbp = _gem_engine_mmio_info_dup(&mbt);
+
+ return mbp;
+}
+
+
+/**
+ * _gem_engine_mmio_base_info_get_debugfs:
+ * @fd_dev: file descriptor upon which device is open or -1 to use defaults.
+ *
+ * Obtains per-engine mmio_base information from debugfs.
+ *
+ * Returns:
+ * Pointer to dynamically allocated struct eng_mmio_base_table_s describing
+ * engine config or NULL.
+ * The allocated size does not include unused engine entries.
+ * If non-NULL, it is caller's responsibility to free.
+ *
+ * Looking in debugfs for per-engine instances of:
+ *
+ * ...
+ * MMIO base:
+ *
+ * Example of relevant lines from debugfs:
+ * vcs0
+ * MMIO base: 0x001c
+ * vcs1
+ * MMIO base: 0x001d
+ *
+ * In order to qualify as the introduction of a new per-engine section, an
+ * input line must consist solely of an engine name. An engine name must
+ * be 7 or fewer characters in length and must consist of an engine class
Re: [Intel-gfx] [igt-dev] [PATCH i-g-t v2 5/5] i915/gem_ctx_isolation.c - If initialization fails, exit
On 2020-02-13 10:29:55, Petri Latvala wrote:
> On Wed, Feb 12, 2020 at 05:28:40PM -0800, Dale B Stimson wrote:
> > At the start of igt_main, failure of the initial tests for successful
> > initialization transfer control to the end of an igt_fixture block.
> > From there, execution of the main per-engine loop is attempted.
> > Instead, the test should be caused to exit.
> >
> > If initialization fails, exit.
> >
> > Signed-off-by: Dale B Stimson
> > ---
> > tests/i915/gem_ctx_isolation.c | 15 +++
> > 1 file changed, 15 insertions(+)
> >
> > diff --git a/tests/i915/gem_ctx_isolation.c b/tests/i915/gem_ctx_isolation.c
> > index 07ffbb84a..b11158dab 100644
> > --- a/tests/i915/gem_ctx_isolation.c
> > +++ b/tests/i915/gem_ctx_isolation.c
> > @@ -898,10 +898,13 @@ igt_main
> > int fd = -1;
> > struct eng_mmio_base_table_s *mbp = NULL;
> > uint32_t mmio_base = 0;
> > + /* igt_fixture block is skipped if --list-subtests, so start with true.
> > */
> > + bool init_successful = true;
> >
> > igt_fixture {
> > int gen;
> >
> > + init_successful = false;
> > fd = drm_open_driver(DRIVER_INTEL);
> > igt_require_gem(fd);
> > igt_require(gem_has_contexts(fd));
> > @@ -916,8 +919,20 @@ igt_main
> > igt_skip_on(gen > LAST_KNOWN_GEN);
> >
> > mbp = gem_engine_mmio_base_info_get(fd);
> > + init_successful = true;
> > }
> >
> > + if (!init_successful) {
> > + igt_exit_early();
> > + }
> > +
>
> NAK. All this dancing around the infrastructure just makes changing
> the infrastructure later be awkward and produce weird errors.
>
> If something in the fixture failed, with this code you never enter the
> subtest, making the test result 'notrun' instead of the correct 'skip'
> or 'fail'.
>
> What is the problem this is trying to solve? Incorrect engine list
> used? If you have a subtest per static engine, all CI does is execute
> per static engine. Converting this test to use dynamic subtests for
> engines is the way forward.
>
>
> --
> Petri Latvala
NAK understood and accepted.
I will address this in a different way, targeting the underlying issues
instead of the symptom. Please see my patch (just sent to ML):
lib/i915/gem_engine_topology.c - intel_get_current_engine invalid result
To answer to your question about what this was trying to solve:
Briefly, and specifically addressing gem_ctx_isolation:
As-is observed behavior when open (or debugfs open) fails: per-engine loop
executes forever:
Subtest vecs0-nonpriv: FAIL
Subtest vecs0-nonpriv-switch: FAIL
Subtest vecs0-clean: FAIL
Subtest vecs0-dirty-create: FAIL
Subtest vecs0-dirty-switch: FAIL
Subtest vecs0-none: FAIL
Subtest vecs0-S3: FAIL
Subtest vecs0-S4: FAIL
Subtest vecs0-reset: FAIL
And repeat, ad infinitum for vecs0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t] lib/i915/gem_engine_topology.c - intel_get_current_engine invalid result
Function intel_get_current_engine() should return NULL (instead of
engine 0) if there are no engines.
Function intel_init_engine_list() should not store potential engine
data in the output structure unless the engine is present.
Function intel_init_engine_list() should arguably not filter the static
engine list with gem_has_ring if fd == -1, so that subtests can still
be individually invoked to show subtest FAIL instead of test notrun.
Symptom: A device open failure in gem_ctx_isolation resulted in
an endless __for_each_physical_engine "per-engine" loop with the
purported last potential engine being processed every time.
Diagnosis: device open (or debugfs open) failed, leaving fd == -1.
Control skipped the rest of the initial igt_fixture block, after
which an attempt was made to iterate through engines using macro
__for_each_physical_engine.
Macro __for_each_physical_engine called intel_init_engine_list()
to initialize the loop control data. Because fd == -1,
intel_init_engine_list() fell back to using __for_each_static_engine().
All of the engines in the static engine list are rejected due to
gem_has_ring returning false (because of fd == -1), leaving 0 engines.
That resulted in loop control data with engine_data.nengines == 0
and the data for the last engine considered stored at index 0.
Still in macro __for_each_physical_engine, intel_get_current_engine()
was called to get the engine to process. It should have returned NULL,
but instead returned the engine entry at index 0, which
had received information describing the last potential engine.
This happened without end.
Signed-off-by: Dale B Stimson
---
lib/i915/gem_engine_topology.c | 29 -
1 file changed, 16 insertions(+), 13 deletions(-)
diff --git a/lib/i915/gem_engine_topology.c b/lib/i915/gem_engine_topology.c
index 9daa03df4..b8ed49bc9 100644
--- a/lib/i915/gem_engine_topology.c
+++ b/lib/i915/gem_engine_topology.c
@@ -156,10 +156,10 @@ static void query_engine_list(int fd, struct
intel_engine_data *ed)
struct intel_execution_engine2 *
intel_get_current_engine(struct intel_engine_data *ed)
{
- if (!ed->n)
- ed->current_engine = &ed->engines[0];
- else if (ed->n >= ed->nengines)
+ if (ed->n >= ed->nengines)
ed->current_engine = NULL;
+ else if (!ed->n)
+ ed->current_engine = &ed->engines[0];
return ed->current_engine;
}
@@ -222,18 +222,21 @@ struct intel_engine_data intel_init_engine_list(int fd,
uint32_t ctx_id)
igt_debug("using pre-allocated engine list\n");
__for_each_static_engine(e2) {
- struct intel_execution_engine2 *__e2 =
- &engine_data.engines[engine_data.nengines];
-
- strcpy(__e2->name, e2->name);
- __e2->instance = e2->instance;
- __e2->class = e2->class;
- __e2->flags = e2->flags;
- __e2->is_virtual = false;
-
if (igt_only_list_subtests() ||
- gem_has_ring(fd, e2->flags))
+ (fd < 0) ||
+ gem_has_ring(fd, e2->flags)) {
+ struct intel_execution_engine2 *__e2 =
+ &engine_data.engines[
+ engine_data.nengines];
+
+ strcpy(__e2->name, e2->name);
+ __e2->instance = e2->instance;
+ __e2->class = e2->class;
+ __e2->flags = e2->flags;
+ __e2->is_virtual = false;
+
engine_data.nengines++;
+}
}
return engine_data;
}
--
2.25.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t v2 3/5] i915/gem_ctx_isolation: Check engine relative registers - Part 2
Modify previous i915/gem_ctx_isolation "Check engine relative registers"
for modified mmio_base infrastructure.
Signed-off-by: Dale B Stimson
---
tests/i915/gem_ctx_isolation.c | 87 +++---
1 file changed, 48 insertions(+), 39 deletions(-)
diff --git a/tests/i915/gem_ctx_isolation.c b/tests/i915/gem_ctx_isolation.c
index eff4b1df2..07ffbb84a 100644
--- a/tests/i915/gem_ctx_isolation.c
+++ b/tests/i915/gem_ctx_isolation.c
@@ -233,12 +233,12 @@ static bool ignore_register(uint32_t offset, uint32_t
mmio_base)
static void tmpl_regs(int fd,
uint32_t ctx,
const struct intel_execution_engine2 *e,
+ uint32_t mmio_base,
uint32_t handle,
uint32_t value)
{
const unsigned int gen_bit = 1 << intel_gen(intel_get_drm_devid(fd));
const unsigned int engine_bit = ENGINE(e->class, e->instance);
- const uint32_t mmio_base = gem_engine_mmio_base(fd, e->name);
unsigned int regs_size;
uint32_t *regs;
@@ -278,12 +278,12 @@ static void tmpl_regs(int fd,
static uint32_t read_regs(int fd,
uint32_t ctx,
const struct intel_execution_engine2 *e,
+ uint32_t mmio_base,
unsigned int flags)
{
const unsigned int gen = intel_gen(intel_get_drm_devid(fd));
const unsigned int gen_bit = 1 << gen;
const unsigned int engine_bit = ENGINE(e->class, e->instance);
- const uint32_t mmio_base = gem_engine_mmio_base(fd, e->name);
const bool r64b = gen >= 8;
struct drm_i915_gem_exec_object2 obj[2];
struct drm_i915_gem_relocation_entry *reloc;
@@ -359,12 +359,12 @@ static uint32_t read_regs(int fd,
static void write_regs(int fd,
uint32_t ctx,
const struct intel_execution_engine2 *e,
+ uint32_t mmio_base,
unsigned int flags,
uint32_t value)
{
const unsigned int gen_bit = 1 << intel_gen(intel_get_drm_devid(fd));
const unsigned int engine_bit = ENGINE(e->class, e->instance);
- const uint32_t mmio_base = gem_engine_mmio_base(fd, e->name);
struct drm_i915_gem_exec_object2 obj;
struct drm_i915_gem_execbuffer2 execbuf;
unsigned int batch_size;
@@ -420,13 +420,13 @@ static void write_regs(int fd,
static void restore_regs(int fd,
uint32_t ctx,
const struct intel_execution_engine2 *e,
+uint32_t mmio_base,
unsigned int flags,
uint32_t regs)
{
const unsigned int gen = intel_gen(intel_get_drm_devid(fd));
const unsigned int gen_bit = 1 << gen;
const unsigned int engine_bit = ENGINE(e->class, e->instance);
- const uint32_t mmio_base = gem_engine_mmio_base(fd, e->name);
const bool r64b = gen >= 8;
struct drm_i915_gem_exec_object2 obj[2];
struct drm_i915_gem_execbuffer2 execbuf;
@@ -498,12 +498,12 @@ static void restore_regs(int fd,
__attribute__((unused))
static void dump_regs(int fd,
const struct intel_execution_engine2 *e,
+ uint32_t mmio_base,
unsigned int regs)
{
const int gen = intel_gen(intel_get_drm_devid(fd));
const unsigned int gen_bit = 1 << gen;
const unsigned int engine_bit = ENGINE(e->class, e->instance);
- const uint32_t mmio_base = gem_engine_mmio_base(fd, e->name);
unsigned int regs_size;
uint32_t *out;
@@ -541,9 +541,9 @@ static void dump_regs(int fd,
}
static void compare_regs(int fd, const struct intel_execution_engine2 *e,
+uint32_t mmio_base,
uint32_t A, uint32_t B, const char *who)
{
- const uint32_t mmio_base = gem_engine_mmio_base(fd, e->name);
unsigned int num_errors;
unsigned int regs_size;
uint32_t *a, *b;
@@ -596,6 +596,7 @@ static void compare_regs(int fd, const struct
intel_execution_engine2 *e,
static void nonpriv(int fd,
const struct intel_execution_engine2 *e,
+ uint32_t mmio_base,
unsigned int flags)
{
static const uint32_t values[] = {
@@ -623,16 +624,16 @@ static void nonpriv(int fd,
ctx = gem_context_clone_with_engines(fd, 0);
- tmpl = read_regs(fd, ctx, e, flags);
- regs[0] = read_regs(fd, ctx, e, flags);
+ tmpl = read_regs(fd, ctx, e, mmio_base, flags);
+ regs[0] = read_regs(fd, ctx, e, mmio_base, flags);
- tmpl_regs(fd, ctx, e, tmpl, values[v]);
+ tmpl_regs(fd, ctx, e, mmio_ba
[Intel-gfx] [PATCH i-g-t v2 5/5] i915/gem_ctx_isolation.c - If initialization fails, exit
At the start of igt_main, failure of the initial tests for successful
initialization transfer control to the end of an igt_fixture block.
>From there, execution of the main per-engine loop is attempted.
Instead, the test should be caused to exit.
If initialization fails, exit.
Signed-off-by: Dale B Stimson
---
tests/i915/gem_ctx_isolation.c | 15 +++
1 file changed, 15 insertions(+)
diff --git a/tests/i915/gem_ctx_isolation.c b/tests/i915/gem_ctx_isolation.c
index 07ffbb84a..b11158dab 100644
--- a/tests/i915/gem_ctx_isolation.c
+++ b/tests/i915/gem_ctx_isolation.c
@@ -898,10 +898,13 @@ igt_main
int fd = -1;
struct eng_mmio_base_table_s *mbp = NULL;
uint32_t mmio_base = 0;
+ /* igt_fixture block is skipped if --list-subtests, so start with true.
*/
+ bool init_successful = true;
igt_fixture {
int gen;
+ init_successful = false;
fd = drm_open_driver(DRIVER_INTEL);
igt_require_gem(fd);
igt_require(gem_has_contexts(fd));
@@ -916,8 +919,20 @@ igt_main
igt_skip_on(gen > LAST_KNOWN_GEN);
mbp = gem_engine_mmio_base_info_get(fd);
+ init_successful = true;
}
+ if (!init_successful) {
+ igt_exit_early();
+ }
+
+ /**
+* With --list-subtests the above igt_fixture block is skipped and so
+* the device is not open. Because fd < 0, __for_each_physical_engine
+* falls back to a static engine list, which will affect the output
+* of --list-subtests.
+*/
+
/* __for_each_physical_engine switches context to all engines. */
__for_each_physical_engine(fd, e) {
igt_subtest_group {
--
2.25.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t v2 1/5] i915/gem_mmio_base.c - get mmio_base from debugfs (if possible)
Signed-off-by: Dale B Stimson
---
lib/Makefile.sources | 2 +
lib/i915/gem_mmio_base.c | 353 +++
lib/i915/gem_mmio_base.h | 19 +++
lib/igt.h| 1 +
lib/meson.build | 1 +
5 files changed, 376 insertions(+)
create mode 100644 lib/i915/gem_mmio_base.c
create mode 100644 lib/i915/gem_mmio_base.h
diff --git a/lib/Makefile.sources b/lib/Makefile.sources
index 3e573f267..4c5d50d5d 100644
--- a/lib/Makefile.sources
+++ b/lib/Makefile.sources
@@ -7,6 +7,8 @@ lib_source_list = \
i915/gem_context.h \
i915/gem_engine_topology.c \
i915/gem_engine_topology.h \
+ i915/gem_mmio_base.c\
+ i915/gem_mmio_base.h\
i915/gem_scheduler.c\
i915/gem_scheduler.h\
i915/gem_submission.c \
diff --git a/lib/i915/gem_mmio_base.c b/lib/i915/gem_mmio_base.c
new file mode 100644
index 0..d1b83221a
--- /dev/null
+++ b/lib/i915/gem_mmio_base.c
@@ -0,0 +1,353 @@
+// Copyright (C) 2020 Intel Corporation
+//
+// SPDX-License-Identifier: MIT
+
+#include
+
+#include
+
+#include "igt.h"
+
+struct eng_mmio_base_s {
+ char name[8];
+ uint32_t mmio_base;
+};
+
+struct eng_mmio_base_table_s {
+ unsigned int mb_cnt;
+ struct eng_mmio_base_s mb_tab[GEM_MAX_ENGINES];
+};
+
+
+static struct eng_mmio_base_table_s *_gem_engine_mmio_info_dup(
+ const struct eng_mmio_base_table_s *mbpi)
+{
+ struct eng_mmio_base_table_s *mbpo;
+ size_t nbytes;
+
+ nbytes = offsetof(typeof(struct eng_mmio_base_table_s),
mb_tab[mbpi->mb_cnt]);
+ mbpo = malloc(nbytes);
+ igt_assert(mbpo);
+ memcpy(mbpo, mbpi, nbytes);
+
+ return mbpo;
+}
+
+void gem_engine_mmio_base_info_free(struct eng_mmio_base_table_s *mbp)
+{
+ if (mbp)
+ free(mbp);
+}
+
+static void _gem_engine_mmio_info_legacy_add(struct eng_mmio_base_table_s *mbp,
+ const char *eng_name, uint32_t mmio_base)
+{
+ if (mmio_base) {
+ strncpy(mbp->mb_tab[mbp->mb_cnt].name, eng_name,
+ sizeof(mbp->mb_tab[0].name));
+ mbp->mb_tab[mbp->mb_cnt].mmio_base = mmio_base;
+ mbp->mb_cnt++;
+ }
+}
+
+/**
+ * _gem_engine_mmio_base_info_get_legacy:
+ * @fd_dev: file descriptor upon which device is open or -1 to use defaults.
+ *
+ * Provides per-engine mmio_base information from legacy built-in values
+ * for the case when the information is not otherwise available.
+ *
+ * Returns:
+ * Pointer to dynamically allocated struct eng_mmio_base_table_s describing
+ * engine config or NULL.
+ * The allocated size does not include unused engine entries.
+ * If non-NULL, it is caller's responsibility to free.
+ */
+static struct eng_mmio_base_table_s *_gem_engine_mmio_base_info_get_legacy(int
fd_dev)
+{
+ int gen;
+ uint32_t mmio_base;
+ struct eng_mmio_base_table_s mbt;
+ struct eng_mmio_base_table_s *mbp;
+
+ memset(&mbt, 0, sizeof(mbt));
+
+ gen = intel_gen(intel_get_drm_devid(fd_dev));
+
+ /* The mmio_base values for engine instances 1 and higher cannot
+* be reliability determinated a priori. */
+
+ _gem_engine_mmio_info_legacy_add(&mbt, "rcs0", 0x2000);
+ _gem_engine_mmio_info_legacy_add(&mbt, "bcs0", 0x22000);
+
+ if (gen < 6)
+ mmio_base = 0x4000;
+ else if (gen < 11)
+ mmio_base = 0x12000;
+ else
+ mmio_base = 0x1c;
+ _gem_engine_mmio_info_legacy_add(&mbt, "vcs0", mmio_base);
+
+ if (gen < 11)
+ mmio_base = 0x1a000;
+ else
+ mmio_base = 0x1c8000;
+ _gem_engine_mmio_info_legacy_add(&mbt, "vecs0", mmio_base);
+
+ if (mbt.mb_cnt <= 0)
+ return NULL;
+
+ mbp = _gem_engine_mmio_info_dup(&mbt);
+
+ return mbp;
+}
+
+
+/**
+ * _gem_engine_mmio_base_info_get_debugfs:
+ * @fd_dev: file descriptor upon which device is open or -1 to use defaults.
+ *
+ * Obtains per-engine mmio_base information from debugfs.
+ *
+ * Returns:
+ * Pointer to dynamically allocated struct eng_mmio_base_table_s describing
+ * engine config or NULL.
+ * The allocated size does not include unused engine entries.
+ * If non-NULL, it is caller's responsibility to free.
+ *
+ * Looking in debugfs for per-engine instances of:
+ *
+ * ...
+ * MMIO base:
+ *
+ * Example of relevant lines from debugfs:
+ * vcs0
+ * MMIO base: 0x001c
+ * vcs1
+ * MMIO base: 0x001d
+ *
+ * In order to qualify as the introduction of a new per-engine section, an
+ * input line must consist solely of an engine name. An engine name must
+ * be 7 or fewer characters in length and must consist of an engine class
[Intel-gfx] [PATCH i-g-t v2 4/5] lib/igt_core.c - Introduce igt_exit_early()
Call igt_exit() after dealing with assumptions not valid for early calls.
In particular:
igt_exit() assumes that subtests have been considered for execution.
With --run-subtest, for an early exit (where subtests had not yet been
considered):
- igt_exit() would complain about "Unknown subtest"
- igt_exit() would exit prematurely.
Signed-off-by: Dale B Stimson
---
lib/igt_core.c | 27 +++
lib/igt_core.h | 1 +
2 files changed, 28 insertions(+)
diff --git a/lib/igt_core.c b/lib/igt_core.c
index 70465130c..78704b93a 100644
--- a/lib/igt_core.c
+++ b/lib/igt_core.c
@@ -2028,6 +2028,33 @@ void igt_exit(void)
exit(igt_exitcode);
}
+/**
+ * igt_exit_early()
+ *
+ * Call igt_exit() after dealing with assumptions not valid for early calls.
+ *
+ * In particular:
+ * igt_exit() assumes that subtests have been considered for execution
+ * and complains if a subtest specified by --run-subtest was not executed.
+ * (The expectation is that the subtest would have been executed if it
existed).
+ *
+ * In particular:
+ * igt_exit() assumes that subtests have been considered for execution.
+ * With --run-subtest, for an early exit (where subtests had not yet been
+ * considered):
+ * - igt_exit() would complain about "Unknown subtest"
+ * - igt_exit() would exit prematurely.
+ */
+void igt_exit_early(void)
+{
+ if (run_single_subtest) {
+ free(run_single_subtest);
+ run_single_subtest = NULL;
+ }
+
+ igt_exit();
+}
+
/* fork support code */
static int helper_process_count;
static pid_t helper_process_pids[] =
diff --git a/lib/igt_core.h b/lib/igt_core.h
index c17a7ba81..a1fe4c361 100644
--- a/lib/igt_core.h
+++ b/lib/igt_core.h
@@ -500,6 +500,7 @@ void __igt_fail_assert(const char *domain, const char *file,
const char *format, ...)
__attribute__((noreturn));
void igt_exit(void) __attribute__((noreturn));
+void igt_exit_early(void) __attribute__((noreturn));
void igt_fatal_error(void) __attribute__((noreturn));
/**
--
2.25.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t v2 0/5] mmio_base via debugfs infrastructure + gem_ctx_isolation
v2: - Introduce and use igt_exit_early() so that a failed initialization (in igt_fixture) will not attempt to invoke the per-engine loop. - Initialize mmio_base db inside initial igt_fixture instead of after. - Some additional functions handle NULL input mmio_base db pointer. - Variables mbp and mmio_base initialized to NULL/0 so their values will not be uninitialized for --list-subtests. - Failure to obtain an mmio_base db is now igt_debug instead of igt_warn. This patch series provides infrastructure to allow determination of i915 per-engine mmio_base (which is otherwise sometimes hard to get). The provided method uses debugfs mmio_base information if present. Otherwise, a default determination is provided when possible. Also, gem_ctx_isolation is modified to use the new infrastructure. For example, on TGL, gem_ctx_isolation (without these or similar changes) will fail for subtests that use engine vcs1. The patches in this series are as they are intended to be submitted (subject to comments), except I would like to squash the two gem_ctx_isolation "relative registers" patches into one (as discussed below). Also, function gem_engine_mmio_base_info_dump() could be removed. On 2020-01-27, Chris wilson sent to the ML: [igt-dev] [PATCH i-g-t 1/5] i915: Start putting the mmio_base to wider use [igt-dev] [PATCH i-g-t 2/5] i915/gem_ctx_isolation: Check engine relative registers plus the following, which are not addressed here: [igt-dev] [PATCH i-g-t 3/5] i915: Exercise preemption timeout controls in sysfs [igt-dev] [PATCH i-g-t 4/5] i915: Exercise sysfs heartbeat controls [igt-dev] [PATCH i-g-t 5/5] i915: Exercise timeslice sysfs property This patch list is: i915/gem_mmio_base.c - get mmio_base from debugfs (if possible) i915/gem_ctx_isolation: Check engine relative registers i915/gem_ctx_isolation: Check engine relative registers - Part 2 The first 2020-01-27 patch obtains mmio_base information via sysfs, and depends on a proposed kernel change that would provide the mmio_base information via sysfs. It is unclear when or whether that kernel change will progress. The mmio_base information used by this patch series is available through debugfs now (as of kernel 5.4). If the per-engine mmio_base information is ever added to sysfs, it would be easy to plug that into the infrastructure proposed here as an additional higher-priority source of that information. This submission replaces the first patch (switching from sysfs to debugfs), retains the second 2020-01-27 patch i915/gem_ctx_isolation: Check engine relative registers and has a third patch that modifies the original second patch to support the altered API: i915/gem_ctx_isolation: Check engine relative registers - Part 2 I propose squashing the two gem_ctx_isolation "relative registers" patches into one patch with author == "Chris Wilson" if Chris agrees. Some differences from the 2020-01-27 patches: The mmio_base information is fetched once into local data structures, and is obtained from them thereafter instead of being fetched from the kernel everytime it is wanted. The function that obtains the mmio_base information is called by a particular test that wants it, and returns a handle through which the mmio_base can be requested for any particular engine. These patches introduce new source files lib/i915/gem_mmio_base.c and lib/i915/gem_mmio_base.h. Should the code instead be placed into lib/i915/gem_engine_topology.c? Function gem_engine_mmio_base_info_dump presently exists to dump the mmio_base information to stdout for debugging or informational purposes. This function is not currently called. I presume this function should be removed. Is there any desire to keep it around for future use? The 2020-01-27 patches define function gem_engine_mmio_base() with its first parameter as "fd". The new patches replace the first parameter with the mmio_base object handle. Chris Wilson (1): i915/gem_ctx_isolation: Check engine relative registers Dale B Stimson (4): i915/gem_mmio_base.c - get mmio_base from debugfs (if possible) i915/gem_ctx_isolation: Check engine relative registers - Part 2 lib/igt_core.c - Introduce igt_exit_early() i915/gem_ctx_isolation.c - If initialization fails, exit lib/Makefile.sources | 2 + lib/i915/gem_mmio_base.c | 353 + lib/i915/gem_mmio_base.h | 19 ++ lib/igt.h | 1 + lib/igt_core.c | 27 +++ lib/igt_core.h | 1 + lib/meson.build| 1 + tests/i915/gem_ctx_isolation.c | 244 ++- 8 files changed, 556 insertions(+), 92 deletions(-) create mode 100644 lib/i915/gem_mmio_base.c create mode 100644 lib/i915/gem_mmio_base.h -- 2.25.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t v2 2/5] i915/gem_ctx_isolation: Check engine relative registers
From: Chris Wilson
Some of the non-privileged registers are at the same offset on each
engine. We can improve our coverage for unknown HW layout by using the
reported engine->mmio_base for relative offsets.
Signed-off-by: Chris Wilson
Reviewed-by: Dale B Stimson
---
tests/i915/gem_ctx_isolation.c | 164 -
1 file changed, 100 insertions(+), 64 deletions(-)
diff --git a/tests/i915/gem_ctx_isolation.c b/tests/i915/gem_ctx_isolation.c
index 1b66fec11..eff4b1df2 100644
--- a/tests/i915/gem_ctx_isolation.c
+++ b/tests/i915/gem_ctx_isolation.c
@@ -70,6 +70,7 @@ static const struct named_register {
uint32_t ignore_bits;
uint32_t write_mask; /* some registers bits do not exist */
bool masked;
+ bool relative;
} nonpriv_registers[] = {
{ "NOPID", NOCTX, RCS0, 0x2094 },
{ "MI_PREDICATE_RESULT_2", NOCTX, RCS0, 0x23bc },
@@ -109,7 +110,6 @@ static const struct named_register {
{ "PS_DEPTH_COUNT_1", GEN8, RCS0, 0x22f8, 2 },
{ "BB_OFFSET", GEN8, RCS0, 0x2158, .ignore_bits = 0x7 },
{ "MI_PREDICATE_RESULT_1", GEN8, RCS0, 0x241c },
- { "CS_GPR", GEN8, RCS0, 0x2600, 32 },
{ "OA_CTX_CONTROL", GEN8, RCS0, 0x2360 },
{ "OACTXID", GEN8, RCS0, 0x2364 },
{ "PS_INVOCATION_COUNT_2", GEN8, RCS0, 0x2448, 2, .write_mask = ~0x3 },
@@ -138,79 +138,56 @@ static const struct named_register {
{ "CTX_PREEMPT", NOCTX /* GEN10 */, RCS0, 0x2248 },
{ "CS_CHICKEN1", GEN11, RCS0, 0x2580, .masked = true },
- { "HDC_CHICKEN1", GEN_RANGE(10, 10), RCS0, 0x7304, .masked = true },
/* Privileged (enabled by w/a + FORCE_TO_NONPRIV) */
{ "CTX_PREEMPT", NOCTX /* GEN9 */, RCS0, 0x2248 },
{ "CS_CHICKEN1", GEN_RANGE(9, 10), RCS0, 0x2580, .masked = true },
{ "COMMON_SLICE_CHICKEN2", GEN_RANGE(9, 9), RCS0, 0x7014, .masked =
true },
- { "HDC_CHICKEN1", GEN_RANGE(9, 9), RCS0, 0x7304, .masked = true },
+ { "HDC_CHICKEN1", GEN_RANGE(9, 10), RCS0, 0x7304, .masked = true },
{ "SLICE_COMMON_ECO_CHICKEN1", GEN_RANGE(11, 11) /* + glk */, RCS0,
0x731c, .masked = true },
{ "L3SQREG4", NOCTX /* GEN9:skl,kbl */, RCS0, 0xb118, .write_mask =
~0x10 },
{ "HALF_SLICE_CHICKEN7", GEN_RANGE(11, 11), RCS0, 0xe194, .masked =
true },
{ "SAMPLER_MODE", GEN_RANGE(11, 11), RCS0, 0xe18c, .masked = true },
- { "BCS_GPR", GEN9, BCS0, 0x22600, 32 },
{ "BCS_SWCTRL", GEN8, BCS0, 0x22200, .write_mask = 0x3, .masked = true
},
{ "MFC_VDBOX1", NOCTX, VCS0, 0x12800, 64 },
{ "MFC_VDBOX2", NOCTX, VCS1, 0x1c800, 64 },
- { "VCS0_GPR", GEN_RANGE(9, 10), VCS0, 0x12600, 32 },
- { "VCS1_GPR", GEN_RANGE(9, 10), VCS1, 0x1c600, 32 },
- { "VECS_GPR", GEN_RANGE(9, 10), VECS0, 0x1a600, 32 },
-
- { "VCS0_GPR", GEN11, VCS0, 0x1c0600, 32 },
- { "VCS1_GPR", GEN11, VCS1, 0x1c4600, 32 },
- { "VCS2_GPR", GEN11, VCS2, 0x1d0600, 32 },
- { "VCS3_GPR", GEN11, VCS3, 0x1d4600, 32 },
- { "VECS_GPR", GEN11, VECS0, 0x1c8600, 32 },
+ { "xCS_GPR", GEN9, ALL, 0x600, 32, .relative = true },
{}
}, ignore_registers[] = {
{ "RCS timestamp", GEN6, ~0u, 0x2358 },
{ "BCS timestamp", GEN7, ~0u, 0x22358 },
- { "VCS0 timestamp", GEN_RANGE(7, 10), ~0u, 0x12358 },
- { "VCS1 timestamp", GEN_RANGE(7, 10), ~0u, 0x1c358 },
- { "VECS timestamp", GEN_RANGE(8, 10), ~0u, 0x1a358 },
-
- { "VCS0 timestamp", GEN11, ~0u, 0x1c0358 },
- { "VCS1 timestamp", GEN11, ~0u, 0x1c4358 },
- { "VCS2 timestamp", GEN11, ~0u, 0x1d0358 },
- { "VCS3 timestamp", GEN11, ~0u, 0x1d4358 },
- { "VECS timestamp", GEN11, ~0u, 0x1c8358 },
+ { "xCS timestamp", GEN8, ALL, 0x358, .relative = true },
/* huc read only */
- { "BSD0 0x2000", GEN11, ~0u, 0x1c + 0x2000 },
- { "BSD0 0x2000", GEN11, ~0u, 0x1c + 0x2014 },
- { "BSD0 0x2000", GEN11, ~0u, 0x1c + 0x23b0 },
-
- { "BSD1 0x2000", GEN11, ~0u, 0x1c4000 + 0x2000 },
- { "BSD1 0x2000", GEN11, ~0u, 0x1c4000 + 0x2014 },
- { "BSD1 0x2000", GEN11, ~0u, 0x1c4000 + 0x23b0 },
-
- { "BSD2 0x2000", GEN11, ~0u, 0x1d + 0x2000 },
- { "BSD2 0x2000", GEN11, ~0u, 0x1d + 0x2014 },
- { "BSD2 0x2000", GEN11, ~0u, 0x1d + 0x23b0 },
-
- { "BSD3 0x2000", GEN11, ~0u, 0x1d4000 + 0x2000 },
- { "BSD
Re: [Intel-gfx] [PATCH i-g-t 0/3] mmio_base via debugfs infrastructure + gem_ctx_isolation
My apologies for the multiple submissions of this patch series. I had to work out an issue with an unsuspected git config value in order to make the references function with patchwork. On 2020-02-12 14:34:28, Dale B Stimson wrote: > This patch series provides infrastructure to allow determination of i915 > per-engine mmio_base (which is otherwise sometimes hard to get). The provided > method uses debugfs mmio_base information if present. Otherwise, a default > determination is provided when possible. Also, gem_ctx_isolation is modified > to use the new infrastructure. > > For example, on TGL, gem_ctx_isolation (without these or similar changes) > will fail for subtests that use engine vcs1. > > The patches in this series are as they are intended to be submitted (subject > to comments), except I would like to squash the two gem_ctx_isolation > "relative registers" patches into one (as discussed below). Also, function > gem_engine_mmio_base_info_dump() could be removed. > > On 2020-01-27, Chris wilson sent to the ML: > [igt-dev] [PATCH i-g-t 1/5] i915: Start putting the mmio_base to wider use > [igt-dev] [PATCH i-g-t 2/5] i915/gem_ctx_isolation: Check engine relative > registers > plus the following, which are not addressed here: > [igt-dev] [PATCH i-g-t 3/5] i915: Exercise preemption timeout controls in > sysfs > [igt-dev] [PATCH i-g-t 4/5] i915: Exercise sysfs heartbeat controls > [igt-dev] [PATCH i-g-t 5/5] i915: Exercise timeslice sysfs property > > This patch list is: > i915/gem_mmio_base.c - get mmio_base from debugfs (if possible) > i915/gem_ctx_isolation: Check engine relative registers > i915/gem_ctx_isolation: Check engine relative registers - Part 2 > > The first 2020-01-27 patch obtains mmio_base information via sysfs, and > depends > on a proposed kernel change that would provide the mmio_base information > via sysfs. It is unclear when or whether that kernel change will progress. > > The mmio_base information used by this patch series is available through > debugfs now (as of kernel 5.4). If the per-engine mmio_base information is > ever added to sysfs, it would be easy to plug that into the infrastructure > proposed here as an additional higher-priority source of that information. > > This submission replaces the first patch (switching from sysfs to debugfs), > retains the second 2020-01-27 patch > i915/gem_ctx_isolation: Check engine relative registers > and has a third patch that modifies the original second patch to support the > altered API: > i915/gem_ctx_isolation: Check engine relative registers - Part 2 > > I propose squashing the two gem_ctx_isolation "relative registers" patches > into one patch with author == "Chris Wilson" if Chris agrees. > > Some differences from the 2020-01-27 patches: > > The mmio_base information is fetched once into local data structures, and > is obtained from them thereafter instead of being fetched from the kernel > everytime it is wanted. > > The function that obtains the mmio_base information is called by a particular > test that wants it, and returns a handle through which the mmio_base can be > requested for any particular engine. > > These patches introduce new source files lib/i915/gem_mmio_base.c > and lib/i915/gem_mmio_base.h. Should the code instead be placed into > lib/i915/gem_engine_topology.c? > > Function gem_engine_mmio_base_info_dump presently exists to dump the > mmio_base information to stdout for debugging or informational purposes. > This function is not currently called. I presume this function should > be removed. Is there any desire to keep it around for future use? > > The 2020-01-27 patches define function gem_engine_mmio_base() with its first > parameter as "fd". The new patches replace the first parameter with the > mmio_base object handle. > > Chris Wilson (1): > i915/gem_ctx_isolation: Check engine relative registers > > Dale B Stimson (2): > i915/gem_mmio_base.c - get mmio_base from debugfs (if possible) > i915/gem_ctx_isolation: Check engine relative registers - Part 2 > > lib/Makefile.sources | 2 + > lib/i915/gem_mmio_base.c | 346 + > lib/i915/gem_mmio_base.h | 19 ++ > lib/igt.h | 1 + > lib/meson.build| 1 + > tests/i915/gem_ctx_isolation.c | 229 +- > 6 files changed, 506 insertions(+), 92 deletions(-) > create mode 100644 lib/i915/gem_mmio_base.c > create mode 100644 lib/i915/gem_mmio_base.h > > -- > 2.25.0 > ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t 2/3] i915/gem_ctx_isolation: Check engine relative registers
From: Chris Wilson
Some of the non-privileged registers are at the same offset on each
engine. We can improve our coverage for unknown HW layout by using the
reported engine->mmio_base for relative offsets.
Signed-off-by: Chris Wilson
Reviewed-by: Dale B Stimson
---
tests/i915/gem_ctx_isolation.c | 164 -
1 file changed, 100 insertions(+), 64 deletions(-)
diff --git a/tests/i915/gem_ctx_isolation.c b/tests/i915/gem_ctx_isolation.c
index 1b66fec11..eff4b1df2 100644
--- a/tests/i915/gem_ctx_isolation.c
+++ b/tests/i915/gem_ctx_isolation.c
@@ -70,6 +70,7 @@ static const struct named_register {
uint32_t ignore_bits;
uint32_t write_mask; /* some registers bits do not exist */
bool masked;
+ bool relative;
} nonpriv_registers[] = {
{ "NOPID", NOCTX, RCS0, 0x2094 },
{ "MI_PREDICATE_RESULT_2", NOCTX, RCS0, 0x23bc },
@@ -109,7 +110,6 @@ static const struct named_register {
{ "PS_DEPTH_COUNT_1", GEN8, RCS0, 0x22f8, 2 },
{ "BB_OFFSET", GEN8, RCS0, 0x2158, .ignore_bits = 0x7 },
{ "MI_PREDICATE_RESULT_1", GEN8, RCS0, 0x241c },
- { "CS_GPR", GEN8, RCS0, 0x2600, 32 },
{ "OA_CTX_CONTROL", GEN8, RCS0, 0x2360 },
{ "OACTXID", GEN8, RCS0, 0x2364 },
{ "PS_INVOCATION_COUNT_2", GEN8, RCS0, 0x2448, 2, .write_mask = ~0x3 },
@@ -138,79 +138,56 @@ static const struct named_register {
{ "CTX_PREEMPT", NOCTX /* GEN10 */, RCS0, 0x2248 },
{ "CS_CHICKEN1", GEN11, RCS0, 0x2580, .masked = true },
- { "HDC_CHICKEN1", GEN_RANGE(10, 10), RCS0, 0x7304, .masked = true },
/* Privileged (enabled by w/a + FORCE_TO_NONPRIV) */
{ "CTX_PREEMPT", NOCTX /* GEN9 */, RCS0, 0x2248 },
{ "CS_CHICKEN1", GEN_RANGE(9, 10), RCS0, 0x2580, .masked = true },
{ "COMMON_SLICE_CHICKEN2", GEN_RANGE(9, 9), RCS0, 0x7014, .masked =
true },
- { "HDC_CHICKEN1", GEN_RANGE(9, 9), RCS0, 0x7304, .masked = true },
+ { "HDC_CHICKEN1", GEN_RANGE(9, 10), RCS0, 0x7304, .masked = true },
{ "SLICE_COMMON_ECO_CHICKEN1", GEN_RANGE(11, 11) /* + glk */, RCS0,
0x731c, .masked = true },
{ "L3SQREG4", NOCTX /* GEN9:skl,kbl */, RCS0, 0xb118, .write_mask =
~0x10 },
{ "HALF_SLICE_CHICKEN7", GEN_RANGE(11, 11), RCS0, 0xe194, .masked =
true },
{ "SAMPLER_MODE", GEN_RANGE(11, 11), RCS0, 0xe18c, .masked = true },
- { "BCS_GPR", GEN9, BCS0, 0x22600, 32 },
{ "BCS_SWCTRL", GEN8, BCS0, 0x22200, .write_mask = 0x3, .masked = true
},
{ "MFC_VDBOX1", NOCTX, VCS0, 0x12800, 64 },
{ "MFC_VDBOX2", NOCTX, VCS1, 0x1c800, 64 },
- { "VCS0_GPR", GEN_RANGE(9, 10), VCS0, 0x12600, 32 },
- { "VCS1_GPR", GEN_RANGE(9, 10), VCS1, 0x1c600, 32 },
- { "VECS_GPR", GEN_RANGE(9, 10), VECS0, 0x1a600, 32 },
-
- { "VCS0_GPR", GEN11, VCS0, 0x1c0600, 32 },
- { "VCS1_GPR", GEN11, VCS1, 0x1c4600, 32 },
- { "VCS2_GPR", GEN11, VCS2, 0x1d0600, 32 },
- { "VCS3_GPR", GEN11, VCS3, 0x1d4600, 32 },
- { "VECS_GPR", GEN11, VECS0, 0x1c8600, 32 },
+ { "xCS_GPR", GEN9, ALL, 0x600, 32, .relative = true },
{}
}, ignore_registers[] = {
{ "RCS timestamp", GEN6, ~0u, 0x2358 },
{ "BCS timestamp", GEN7, ~0u, 0x22358 },
- { "VCS0 timestamp", GEN_RANGE(7, 10), ~0u, 0x12358 },
- { "VCS1 timestamp", GEN_RANGE(7, 10), ~0u, 0x1c358 },
- { "VECS timestamp", GEN_RANGE(8, 10), ~0u, 0x1a358 },
-
- { "VCS0 timestamp", GEN11, ~0u, 0x1c0358 },
- { "VCS1 timestamp", GEN11, ~0u, 0x1c4358 },
- { "VCS2 timestamp", GEN11, ~0u, 0x1d0358 },
- { "VCS3 timestamp", GEN11, ~0u, 0x1d4358 },
- { "VECS timestamp", GEN11, ~0u, 0x1c8358 },
+ { "xCS timestamp", GEN8, ALL, 0x358, .relative = true },
/* huc read only */
- { "BSD0 0x2000", GEN11, ~0u, 0x1c + 0x2000 },
- { "BSD0 0x2000", GEN11, ~0u, 0x1c + 0x2014 },
- { "BSD0 0x2000", GEN11, ~0u, 0x1c + 0x23b0 },
-
- { "BSD1 0x2000", GEN11, ~0u, 0x1c4000 + 0x2000 },
- { "BSD1 0x2000", GEN11, ~0u, 0x1c4000 + 0x2014 },
- { "BSD1 0x2000", GEN11, ~0u, 0x1c4000 + 0x23b0 },
-
- { "BSD2 0x2000", GEN11, ~0u, 0x1d + 0x2000 },
- { "BSD2 0x2000", GEN11, ~0u, 0x1d + 0x2014 },
- { "BSD2 0x2000", GEN11, ~0u, 0x1d + 0x23b0 },
-
- { "BSD3 0x2000", GEN11, ~0u, 0x1d4000 + 0x2000 },
- { "BSD
[Intel-gfx] [PATCH i-g-t 0/3] mmio_base via debugfs infrastructure + gem_ctx_isolation
This patch series provides infrastructure to allow determination of i915 per-engine mmio_base (which is otherwise sometimes hard to get). The provided method uses debugfs mmio_base information if present. Otherwise, a default determination is provided when possible. Also, gem_ctx_isolation is modified to use the new infrastructure. For example, on TGL, gem_ctx_isolation (without these or similar changes) will fail for subtests that use engine vcs1. The patches in this series are as they are intended to be submitted (subject to comments), except I would like to squash the two gem_ctx_isolation "relative registers" patches into one (as discussed below). Also, function gem_engine_mmio_base_info_dump() could be removed. On 2020-01-27, Chris wilson sent to the ML: [igt-dev] [PATCH i-g-t 1/5] i915: Start putting the mmio_base to wider use [igt-dev] [PATCH i-g-t 2/5] i915/gem_ctx_isolation: Check engine relative registers plus the following, which are not addressed here: [igt-dev] [PATCH i-g-t 3/5] i915: Exercise preemption timeout controls in sysfs [igt-dev] [PATCH i-g-t 4/5] i915: Exercise sysfs heartbeat controls [igt-dev] [PATCH i-g-t 5/5] i915: Exercise timeslice sysfs property This patch list is: i915/gem_mmio_base.c - get mmio_base from debugfs (if possible) i915/gem_ctx_isolation: Check engine relative registers i915/gem_ctx_isolation: Check engine relative registers - Part 2 The first 2020-01-27 patch obtains mmio_base information via sysfs, and depends on a proposed kernel change that would provide the mmio_base information via sysfs. It is unclear when or whether that kernel change will progress. The mmio_base information used by this patch series is available through debugfs now (as of kernel 5.4). If the per-engine mmio_base information is ever added to sysfs, it would be easy to plug that into the infrastructure proposed here as an additional higher-priority source of that information. This submission replaces the first patch (switching from sysfs to debugfs), retains the second 2020-01-27 patch i915/gem_ctx_isolation: Check engine relative registers and has a third patch that modifies the original second patch to support the altered API: i915/gem_ctx_isolation: Check engine relative registers - Part 2 I propose squashing the two gem_ctx_isolation "relative registers" patches into one patch with author == "Chris Wilson" if Chris agrees. Some differences from the 2020-01-27 patches: The mmio_base information is fetched once into local data structures, and is obtained from them thereafter instead of being fetched from the kernel everytime it is wanted. The function that obtains the mmio_base information is called by a particular test that wants it, and returns a handle through which the mmio_base can be requested for any particular engine. These patches introduce new source files lib/i915/gem_mmio_base.c and lib/i915/gem_mmio_base.h. Should the code instead be placed into lib/i915/gem_engine_topology.c? Function gem_engine_mmio_base_info_dump presently exists to dump the mmio_base information to stdout for debugging or informational purposes. This function is not currently called. I presume this function should be removed. Is there any desire to keep it around for future use? The 2020-01-27 patches define function gem_engine_mmio_base() with its first parameter as "fd". The new patches replace the first parameter with the mmio_base object handle. Chris Wilson (1): i915/gem_ctx_isolation: Check engine relative registers Dale B Stimson (2): i915/gem_mmio_base.c - get mmio_base from debugfs (if possible) i915/gem_ctx_isolation: Check engine relative registers - Part 2 lib/Makefile.sources | 2 + lib/i915/gem_mmio_base.c | 346 + lib/i915/gem_mmio_base.h | 19 ++ lib/igt.h | 1 + lib/meson.build| 1 + tests/i915/gem_ctx_isolation.c | 229 +- 6 files changed, 506 insertions(+), 92 deletions(-) create mode 100644 lib/i915/gem_mmio_base.c create mode 100644 lib/i915/gem_mmio_base.h -- 2.25.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t 1/3] i915/gem_mmio_base.c - get mmio_base from debugfs (if possible)
Signed-off-by: Dale B Stimson
---
lib/Makefile.sources | 2 +
lib/i915/gem_mmio_base.c | 346 +++
lib/i915/gem_mmio_base.h | 19 +++
lib/igt.h| 1 +
lib/meson.build | 1 +
5 files changed, 369 insertions(+)
create mode 100644 lib/i915/gem_mmio_base.c
create mode 100644 lib/i915/gem_mmio_base.h
diff --git a/lib/Makefile.sources b/lib/Makefile.sources
index 3e573f267..4c5d50d5d 100644
--- a/lib/Makefile.sources
+++ b/lib/Makefile.sources
@@ -7,6 +7,8 @@ lib_source_list = \
i915/gem_context.h \
i915/gem_engine_topology.c \
i915/gem_engine_topology.h \
+ i915/gem_mmio_base.c\
+ i915/gem_mmio_base.h\
i915/gem_scheduler.c\
i915/gem_scheduler.h\
i915/gem_submission.c \
diff --git a/lib/i915/gem_mmio_base.c b/lib/i915/gem_mmio_base.c
new file mode 100644
index 0..8718c092f
--- /dev/null
+++ b/lib/i915/gem_mmio_base.c
@@ -0,0 +1,346 @@
+// Copyright (C) 2020 Intel Corporation
+//
+// SPDX-License-Identifier: MIT
+
+#include
+
+#include
+
+#include "igt.h"
+
+struct eng_mmio_base_s {
+ char name[8];
+ uint32_t mmio_base;
+};
+
+struct eng_mmio_base_table_s {
+ unsigned int mb_cnt;
+ struct eng_mmio_base_s mb_tab[GEM_MAX_ENGINES];
+};
+
+
+static struct eng_mmio_base_table_s *_gem_engine_mmio_info_dup(
+ const struct eng_mmio_base_table_s *mbpi)
+{
+ struct eng_mmio_base_table_s *mbpo;
+ size_t nbytes;
+
+ nbytes = offsetof(typeof(struct eng_mmio_base_table_s),
mb_tab[mbpi->mb_cnt]);
+ mbpo = malloc(nbytes);
+ igt_assert(mbpo);
+ memcpy(mbpo, mbpi, nbytes);
+
+ return mbpo;
+}
+
+void gem_engine_mmio_base_info_free(struct eng_mmio_base_table_s *mbp)
+{
+ free(mbp);
+}
+
+static void _gem_engine_mmio_info_legacy_add(struct eng_mmio_base_table_s *mbp,
+ const char *eng_name, uint32_t mmio_base)
+{
+ if (mmio_base) {
+ strncpy(mbp->mb_tab[mbp->mb_cnt].name, eng_name,
+ sizeof(mbp->mb_tab[0].name));
+ mbp->mb_tab[mbp->mb_cnt].mmio_base = mmio_base;
+ mbp->mb_cnt++;
+ }
+}
+
+/**
+ * _gem_engine_mmio_base_info_get_legacy:
+ * @fd_dev: file descriptor upon which device is open or -1 to use defaults.
+ *
+ * Provides per-engine mmio_base information from legacy built-in values
+ * for the case when the information is not otherwise available.
+ *
+ * Returns:
+ * Pointer to dynamically allocated struct eng_mmio_base_table_s describing
+ * engine config or NULL.
+ * The allocated size does not include unused engine entries.
+ * If non-NULL, it is caller's responsibility to free.
+ */
+static struct eng_mmio_base_table_s *_gem_engine_mmio_base_info_get_legacy(int
fd_dev)
+{
+ int gen;
+ uint32_t mmio_base;
+ struct eng_mmio_base_table_s mbt;
+ struct eng_mmio_base_table_s *mbp;
+
+ memset(&mbt, 0, sizeof(mbt));
+
+ gen = intel_gen(intel_get_drm_devid(fd_dev));
+
+ /* The mmio_base values for engine instances 1 and higher cannot
+* be reliability determinated a priori. */
+
+ _gem_engine_mmio_info_legacy_add(&mbt, "rcs0", 0x2000);
+ _gem_engine_mmio_info_legacy_add(&mbt, "bcs0", 0x22000);
+
+ if (gen < 6)
+ mmio_base = 0x4000;
+ else if (gen < 11)
+ mmio_base = 0x12000;
+ else
+ mmio_base = 0x1c;
+ _gem_engine_mmio_info_legacy_add(&mbt, "vcs0", mmio_base);
+
+ if (gen < 11)
+ mmio_base = 0x1a000;
+ else
+ mmio_base = 0x1c8000;
+ _gem_engine_mmio_info_legacy_add(&mbt, "vecs0", mmio_base);
+
+ if (mbt.mb_cnt <= 0)
+ return NULL;
+
+ mbp = _gem_engine_mmio_info_dup(&mbt);
+
+ return mbp;
+}
+
+
+/**
+ * _gem_engine_mmio_base_info_get_debugfs:
+ * @fd_dev: file descriptor upon which device is open or -1 to use defaults.
+ *
+ * Obtains per-engine mmio_base information from debugfs.
+ *
+ * Returns:
+ * Pointer to dynamically allocated struct eng_mmio_base_table_s describing
+ * engine config or NULL.
+ * The allocated size does not include unused engine entries.
+ * If non-NULL, it is caller's responsibility to free.
+ *
+ * Looking in debugfs for per-engine instances of:
+ *
+ * ...
+ * MMIO base:
+ *
+ * Example of relevant lines from debugfs:
+ * vcs0
+ * MMIO base: 0x001c
+ * vcs1
+ * MMIO base: 0x001d
+ *
+ * In order to qualify as the introduction of a new per-engine section, an
+ * input line must consist solely of an engine name. An engine name must
+ * be 7 or fewer characters in length and must consist of an engine class
[Intel-gfx] [PATCH i-g-t 3/3] i915/gem_ctx_isolation: Check engine relative registers - Part 2
Modify previous i915/gem_ctx_isolation "Check engine relative registers"
for modified mmio_base infrastructure.
Signed-off-by: Dale B Stimson
---
tests/i915/gem_ctx_isolation.c | 87 +++---
1 file changed, 48 insertions(+), 39 deletions(-)
diff --git a/tests/i915/gem_ctx_isolation.c b/tests/i915/gem_ctx_isolation.c
index eff4b1df2..eec78c729 100644
--- a/tests/i915/gem_ctx_isolation.c
+++ b/tests/i915/gem_ctx_isolation.c
@@ -233,12 +233,12 @@ static bool ignore_register(uint32_t offset, uint32_t
mmio_base)
static void tmpl_regs(int fd,
uint32_t ctx,
const struct intel_execution_engine2 *e,
+ uint32_t mmio_base,
uint32_t handle,
uint32_t value)
{
const unsigned int gen_bit = 1 << intel_gen(intel_get_drm_devid(fd));
const unsigned int engine_bit = ENGINE(e->class, e->instance);
- const uint32_t mmio_base = gem_engine_mmio_base(fd, e->name);
unsigned int regs_size;
uint32_t *regs;
@@ -278,12 +278,12 @@ static void tmpl_regs(int fd,
static uint32_t read_regs(int fd,
uint32_t ctx,
const struct intel_execution_engine2 *e,
+ uint32_t mmio_base,
unsigned int flags)
{
const unsigned int gen = intel_gen(intel_get_drm_devid(fd));
const unsigned int gen_bit = 1 << gen;
const unsigned int engine_bit = ENGINE(e->class, e->instance);
- const uint32_t mmio_base = gem_engine_mmio_base(fd, e->name);
const bool r64b = gen >= 8;
struct drm_i915_gem_exec_object2 obj[2];
struct drm_i915_gem_relocation_entry *reloc;
@@ -359,12 +359,12 @@ static uint32_t read_regs(int fd,
static void write_regs(int fd,
uint32_t ctx,
const struct intel_execution_engine2 *e,
+ uint32_t mmio_base,
unsigned int flags,
uint32_t value)
{
const unsigned int gen_bit = 1 << intel_gen(intel_get_drm_devid(fd));
const unsigned int engine_bit = ENGINE(e->class, e->instance);
- const uint32_t mmio_base = gem_engine_mmio_base(fd, e->name);
struct drm_i915_gem_exec_object2 obj;
struct drm_i915_gem_execbuffer2 execbuf;
unsigned int batch_size;
@@ -420,13 +420,13 @@ static void write_regs(int fd,
static void restore_regs(int fd,
uint32_t ctx,
const struct intel_execution_engine2 *e,
+uint32_t mmio_base,
unsigned int flags,
uint32_t regs)
{
const unsigned int gen = intel_gen(intel_get_drm_devid(fd));
const unsigned int gen_bit = 1 << gen;
const unsigned int engine_bit = ENGINE(e->class, e->instance);
- const uint32_t mmio_base = gem_engine_mmio_base(fd, e->name);
const bool r64b = gen >= 8;
struct drm_i915_gem_exec_object2 obj[2];
struct drm_i915_gem_execbuffer2 execbuf;
@@ -498,12 +498,12 @@ static void restore_regs(int fd,
__attribute__((unused))
static void dump_regs(int fd,
const struct intel_execution_engine2 *e,
+ uint32_t mmio_base,
unsigned int regs)
{
const int gen = intel_gen(intel_get_drm_devid(fd));
const unsigned int gen_bit = 1 << gen;
const unsigned int engine_bit = ENGINE(e->class, e->instance);
- const uint32_t mmio_base = gem_engine_mmio_base(fd, e->name);
unsigned int regs_size;
uint32_t *out;
@@ -541,9 +541,9 @@ static void dump_regs(int fd,
}
static void compare_regs(int fd, const struct intel_execution_engine2 *e,
+uint32_t mmio_base,
uint32_t A, uint32_t B, const char *who)
{
- const uint32_t mmio_base = gem_engine_mmio_base(fd, e->name);
unsigned int num_errors;
unsigned int regs_size;
uint32_t *a, *b;
@@ -596,6 +596,7 @@ static void compare_regs(int fd, const struct
intel_execution_engine2 *e,
static void nonpriv(int fd,
const struct intel_execution_engine2 *e,
+ uint32_t mmio_base,
unsigned int flags)
{
static const uint32_t values[] = {
@@ -623,16 +624,16 @@ static void nonpriv(int fd,
ctx = gem_context_clone_with_engines(fd, 0);
- tmpl = read_regs(fd, ctx, e, flags);
- regs[0] = read_regs(fd, ctx, e, flags);
+ tmpl = read_regs(fd, ctx, e, mmio_base, flags);
+ regs[0] = read_regs(fd, ctx, e, mmio_base, flags);
- tmpl_regs(fd, ctx, e, tmpl, values[v]);
+ tmpl_regs(fd, ctx, e, mmio_ba
[Intel-gfx] [PATCH i-g-t 3/3] i915/gem_ctx_isolation: Check engine relative registers - Part 2
Modify previous i915/gem_ctx_isolation "Check engine relative registers"
for modified mmio_base infrastructure.
Signed-off-by: Dale B Stimson
---
tests/i915/gem_ctx_isolation.c | 87 +++---
1 file changed, 48 insertions(+), 39 deletions(-)
diff --git a/tests/i915/gem_ctx_isolation.c b/tests/i915/gem_ctx_isolation.c
index eff4b1df2..eec78c729 100644
--- a/tests/i915/gem_ctx_isolation.c
+++ b/tests/i915/gem_ctx_isolation.c
@@ -233,12 +233,12 @@ static bool ignore_register(uint32_t offset, uint32_t
mmio_base)
static void tmpl_regs(int fd,
uint32_t ctx,
const struct intel_execution_engine2 *e,
+ uint32_t mmio_base,
uint32_t handle,
uint32_t value)
{
const unsigned int gen_bit = 1 << intel_gen(intel_get_drm_devid(fd));
const unsigned int engine_bit = ENGINE(e->class, e->instance);
- const uint32_t mmio_base = gem_engine_mmio_base(fd, e->name);
unsigned int regs_size;
uint32_t *regs;
@@ -278,12 +278,12 @@ static void tmpl_regs(int fd,
static uint32_t read_regs(int fd,
uint32_t ctx,
const struct intel_execution_engine2 *e,
+ uint32_t mmio_base,
unsigned int flags)
{
const unsigned int gen = intel_gen(intel_get_drm_devid(fd));
const unsigned int gen_bit = 1 << gen;
const unsigned int engine_bit = ENGINE(e->class, e->instance);
- const uint32_t mmio_base = gem_engine_mmio_base(fd, e->name);
const bool r64b = gen >= 8;
struct drm_i915_gem_exec_object2 obj[2];
struct drm_i915_gem_relocation_entry *reloc;
@@ -359,12 +359,12 @@ static uint32_t read_regs(int fd,
static void write_regs(int fd,
uint32_t ctx,
const struct intel_execution_engine2 *e,
+ uint32_t mmio_base,
unsigned int flags,
uint32_t value)
{
const unsigned int gen_bit = 1 << intel_gen(intel_get_drm_devid(fd));
const unsigned int engine_bit = ENGINE(e->class, e->instance);
- const uint32_t mmio_base = gem_engine_mmio_base(fd, e->name);
struct drm_i915_gem_exec_object2 obj;
struct drm_i915_gem_execbuffer2 execbuf;
unsigned int batch_size;
@@ -420,13 +420,13 @@ static void write_regs(int fd,
static void restore_regs(int fd,
uint32_t ctx,
const struct intel_execution_engine2 *e,
+uint32_t mmio_base,
unsigned int flags,
uint32_t regs)
{
const unsigned int gen = intel_gen(intel_get_drm_devid(fd));
const unsigned int gen_bit = 1 << gen;
const unsigned int engine_bit = ENGINE(e->class, e->instance);
- const uint32_t mmio_base = gem_engine_mmio_base(fd, e->name);
const bool r64b = gen >= 8;
struct drm_i915_gem_exec_object2 obj[2];
struct drm_i915_gem_execbuffer2 execbuf;
@@ -498,12 +498,12 @@ static void restore_regs(int fd,
__attribute__((unused))
static void dump_regs(int fd,
const struct intel_execution_engine2 *e,
+ uint32_t mmio_base,
unsigned int regs)
{
const int gen = intel_gen(intel_get_drm_devid(fd));
const unsigned int gen_bit = 1 << gen;
const unsigned int engine_bit = ENGINE(e->class, e->instance);
- const uint32_t mmio_base = gem_engine_mmio_base(fd, e->name);
unsigned int regs_size;
uint32_t *out;
@@ -541,9 +541,9 @@ static void dump_regs(int fd,
}
static void compare_regs(int fd, const struct intel_execution_engine2 *e,
+uint32_t mmio_base,
uint32_t A, uint32_t B, const char *who)
{
- const uint32_t mmio_base = gem_engine_mmio_base(fd, e->name);
unsigned int num_errors;
unsigned int regs_size;
uint32_t *a, *b;
@@ -596,6 +596,7 @@ static void compare_regs(int fd, const struct
intel_execution_engine2 *e,
static void nonpriv(int fd,
const struct intel_execution_engine2 *e,
+ uint32_t mmio_base,
unsigned int flags)
{
static const uint32_t values[] = {
@@ -623,16 +624,16 @@ static void nonpriv(int fd,
ctx = gem_context_clone_with_engines(fd, 0);
- tmpl = read_regs(fd, ctx, e, flags);
- regs[0] = read_regs(fd, ctx, e, flags);
+ tmpl = read_regs(fd, ctx, e, mmio_base, flags);
+ regs[0] = read_regs(fd, ctx, e, mmio_base, flags);
- tmpl_regs(fd, ctx, e, tmpl, values[v]);
+ tmpl_regs(fd, ctx, e, mmio_ba
[Intel-gfx] [PATCH i-g-t 2/3] i915/gem_ctx_isolation: Check engine relative registers
From: Chris Wilson
Some of the non-privileged registers are at the same offset on each
engine. We can improve our coverage for unknown HW layout by using the
reported engine->mmio_base for relative offsets.
Signed-off-by: Chris Wilson
Reviewed-by: Dale B Stimson
---
tests/i915/gem_ctx_isolation.c | 164 -
1 file changed, 100 insertions(+), 64 deletions(-)
diff --git a/tests/i915/gem_ctx_isolation.c b/tests/i915/gem_ctx_isolation.c
index 1b66fec11..eff4b1df2 100644
--- a/tests/i915/gem_ctx_isolation.c
+++ b/tests/i915/gem_ctx_isolation.c
@@ -70,6 +70,7 @@ static const struct named_register {
uint32_t ignore_bits;
uint32_t write_mask; /* some registers bits do not exist */
bool masked;
+ bool relative;
} nonpriv_registers[] = {
{ "NOPID", NOCTX, RCS0, 0x2094 },
{ "MI_PREDICATE_RESULT_2", NOCTX, RCS0, 0x23bc },
@@ -109,7 +110,6 @@ static const struct named_register {
{ "PS_DEPTH_COUNT_1", GEN8, RCS0, 0x22f8, 2 },
{ "BB_OFFSET", GEN8, RCS0, 0x2158, .ignore_bits = 0x7 },
{ "MI_PREDICATE_RESULT_1", GEN8, RCS0, 0x241c },
- { "CS_GPR", GEN8, RCS0, 0x2600, 32 },
{ "OA_CTX_CONTROL", GEN8, RCS0, 0x2360 },
{ "OACTXID", GEN8, RCS0, 0x2364 },
{ "PS_INVOCATION_COUNT_2", GEN8, RCS0, 0x2448, 2, .write_mask = ~0x3 },
@@ -138,79 +138,56 @@ static const struct named_register {
{ "CTX_PREEMPT", NOCTX /* GEN10 */, RCS0, 0x2248 },
{ "CS_CHICKEN1", GEN11, RCS0, 0x2580, .masked = true },
- { "HDC_CHICKEN1", GEN_RANGE(10, 10), RCS0, 0x7304, .masked = true },
/* Privileged (enabled by w/a + FORCE_TO_NONPRIV) */
{ "CTX_PREEMPT", NOCTX /* GEN9 */, RCS0, 0x2248 },
{ "CS_CHICKEN1", GEN_RANGE(9, 10), RCS0, 0x2580, .masked = true },
{ "COMMON_SLICE_CHICKEN2", GEN_RANGE(9, 9), RCS0, 0x7014, .masked =
true },
- { "HDC_CHICKEN1", GEN_RANGE(9, 9), RCS0, 0x7304, .masked = true },
+ { "HDC_CHICKEN1", GEN_RANGE(9, 10), RCS0, 0x7304, .masked = true },
{ "SLICE_COMMON_ECO_CHICKEN1", GEN_RANGE(11, 11) /* + glk */, RCS0,
0x731c, .masked = true },
{ "L3SQREG4", NOCTX /* GEN9:skl,kbl */, RCS0, 0xb118, .write_mask =
~0x10 },
{ "HALF_SLICE_CHICKEN7", GEN_RANGE(11, 11), RCS0, 0xe194, .masked =
true },
{ "SAMPLER_MODE", GEN_RANGE(11, 11), RCS0, 0xe18c, .masked = true },
- { "BCS_GPR", GEN9, BCS0, 0x22600, 32 },
{ "BCS_SWCTRL", GEN8, BCS0, 0x22200, .write_mask = 0x3, .masked = true
},
{ "MFC_VDBOX1", NOCTX, VCS0, 0x12800, 64 },
{ "MFC_VDBOX2", NOCTX, VCS1, 0x1c800, 64 },
- { "VCS0_GPR", GEN_RANGE(9, 10), VCS0, 0x12600, 32 },
- { "VCS1_GPR", GEN_RANGE(9, 10), VCS1, 0x1c600, 32 },
- { "VECS_GPR", GEN_RANGE(9, 10), VECS0, 0x1a600, 32 },
-
- { "VCS0_GPR", GEN11, VCS0, 0x1c0600, 32 },
- { "VCS1_GPR", GEN11, VCS1, 0x1c4600, 32 },
- { "VCS2_GPR", GEN11, VCS2, 0x1d0600, 32 },
- { "VCS3_GPR", GEN11, VCS3, 0x1d4600, 32 },
- { "VECS_GPR", GEN11, VECS0, 0x1c8600, 32 },
+ { "xCS_GPR", GEN9, ALL, 0x600, 32, .relative = true },
{}
}, ignore_registers[] = {
{ "RCS timestamp", GEN6, ~0u, 0x2358 },
{ "BCS timestamp", GEN7, ~0u, 0x22358 },
- { "VCS0 timestamp", GEN_RANGE(7, 10), ~0u, 0x12358 },
- { "VCS1 timestamp", GEN_RANGE(7, 10), ~0u, 0x1c358 },
- { "VECS timestamp", GEN_RANGE(8, 10), ~0u, 0x1a358 },
-
- { "VCS0 timestamp", GEN11, ~0u, 0x1c0358 },
- { "VCS1 timestamp", GEN11, ~0u, 0x1c4358 },
- { "VCS2 timestamp", GEN11, ~0u, 0x1d0358 },
- { "VCS3 timestamp", GEN11, ~0u, 0x1d4358 },
- { "VECS timestamp", GEN11, ~0u, 0x1c8358 },
+ { "xCS timestamp", GEN8, ALL, 0x358, .relative = true },
/* huc read only */
- { "BSD0 0x2000", GEN11, ~0u, 0x1c + 0x2000 },
- { "BSD0 0x2000", GEN11, ~0u, 0x1c + 0x2014 },
- { "BSD0 0x2000", GEN11, ~0u, 0x1c + 0x23b0 },
-
- { "BSD1 0x2000", GEN11, ~0u, 0x1c4000 + 0x2000 },
- { "BSD1 0x2000", GEN11, ~0u, 0x1c4000 + 0x2014 },
- { "BSD1 0x2000", GEN11, ~0u, 0x1c4000 + 0x23b0 },
-
- { "BSD2 0x2000", GEN11, ~0u, 0x1d + 0x2000 },
- { "BSD2 0x2000", GEN11, ~0u, 0x1d + 0x2014 },
- { "BSD2 0x2000", GEN11, ~0u, 0x1d + 0x23b0 },
-
- { "BSD3 0x2000", GEN11, ~0u, 0x1d4000 + 0x2000 },
- { "BSD
[Intel-gfx] [PATCH i-g-t 0/3] mmio_base via debugfs infrastructure + gem_ctx_isolation
This patch series provides infrastructure to allow determination of i915 per-engine mmio_base (which is otherwise sometimes hard to get). The provided method uses debugfs mmio_base information if present. Otherwise, a default determination is provided when possible. Also, gem_ctx_isolation is modified to use the new infrastructure. For example, on TGL, gem_ctx_isolation (without these or similar changes) will fail for subtests that use engine vcs1. The patches in this series are as they are intended to be submitted (subject to comments), except I would like to squash the two gem_ctx_isolation "relative registers" patches into one (as discussed below). Also, function gem_engine_mmio_base_info_dump() could be removed. On 2020-01-27, Chris wilson sent to the ML: [igt-dev] [PATCH i-g-t 1/5] i915: Start putting the mmio_base to wider use [igt-dev] [PATCH i-g-t 2/5] i915/gem_ctx_isolation: Check engine relative registers plus the following, which are not addressed here: [igt-dev] [PATCH i-g-t 3/5] i915: Exercise preemption timeout controls in sysfs [igt-dev] [PATCH i-g-t 4/5] i915: Exercise sysfs heartbeat controls [igt-dev] [PATCH i-g-t 5/5] i915: Exercise timeslice sysfs property This patch list is: i915/gem_mmio_base.c - get mmio_base from debugfs (if possible) i915/gem_ctx_isolation: Check engine relative registers i915/gem_ctx_isolation: Check engine relative registers - Part 2 The first 2020-01-27 patch obtains mmio_base information via sysfs, and depends on a proposed kernel change that would provide the mmio_base information via sysfs. It is unclear when or whether that kernel change will progress. The mmio_base information used by this patch series is available through debugfs now (as of kernel 5.4). If the per-engine mmio_base information is ever added to sysfs, it would be easy to plug that into the infrastructure proposed here as an additional higher-priority source of that information. This submission replaces the first patch (switching from sysfs to debugfs), retains the second 2020-01-27 patch i915/gem_ctx_isolation: Check engine relative registers and has a third patch that modifies the original second patch to support the altered API: i915/gem_ctx_isolation: Check engine relative registers - Part 2 I propose squashing the two gem_ctx_isolation "relative registers" patches into one patch with author == "Chris Wilson" if Chris agrees. Some differences from the 2020-01-27 patches: The mmio_base information is fetched once into local data structures, and is obtained from them thereafter instead of being fetched from the kernel everytime it is wanted. The function that obtains the mmio_base information is called by a particular test that wants it, and returns a handle through which the mmio_base can be requested for any particular engine. These patches introduce new source files lib/i915/gem_mmio_base.c and lib/i915/gem_mmio_base.h. Should the code instead be placed into lib/i915/gem_engine_topology.c? Function gem_engine_mmio_base_info_dump presently exists to dump the mmio_base information to stdout for debugging or informational purposes. This function is not currently called. I presume this function should be removed. Is there any desire to keep it around for future use? The 2020-01-27 patches define function gem_engine_mmio_base() with its first parameter as "fd". The new patches replace the first parameter with the mmio_base object handle. Chris Wilson (1): i915/gem_ctx_isolation: Check engine relative registers Dale B Stimson (2): i915/gem_mmio_base.c - get mmio_base from debugfs (if possible) i915/gem_ctx_isolation: Check engine relative registers - Part 2 lib/Makefile.sources | 2 + lib/i915/gem_mmio_base.c | 346 + lib/i915/gem_mmio_base.h | 19 ++ lib/igt.h | 1 + lib/meson.build| 1 + tests/i915/gem_ctx_isolation.c | 229 +- 6 files changed, 506 insertions(+), 92 deletions(-) create mode 100644 lib/i915/gem_mmio_base.c create mode 100644 lib/i915/gem_mmio_base.h -- 2.25.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t 1/3] i915/gem_mmio_base.c - get mmio_base from debugfs (if possible)
Signed-off-by: Dale B Stimson
---
lib/Makefile.sources | 2 +
lib/i915/gem_mmio_base.c | 346 +++
lib/i915/gem_mmio_base.h | 19 +++
lib/igt.h| 1 +
lib/meson.build | 1 +
5 files changed, 369 insertions(+)
create mode 100644 lib/i915/gem_mmio_base.c
create mode 100644 lib/i915/gem_mmio_base.h
diff --git a/lib/Makefile.sources b/lib/Makefile.sources
index 3e573f267..4c5d50d5d 100644
--- a/lib/Makefile.sources
+++ b/lib/Makefile.sources
@@ -7,6 +7,8 @@ lib_source_list = \
i915/gem_context.h \
i915/gem_engine_topology.c \
i915/gem_engine_topology.h \
+ i915/gem_mmio_base.c\
+ i915/gem_mmio_base.h\
i915/gem_scheduler.c\
i915/gem_scheduler.h\
i915/gem_submission.c \
diff --git a/lib/i915/gem_mmio_base.c b/lib/i915/gem_mmio_base.c
new file mode 100644
index 0..8718c092f
--- /dev/null
+++ b/lib/i915/gem_mmio_base.c
@@ -0,0 +1,346 @@
+// Copyright (C) 2020 Intel Corporation
+//
+// SPDX-License-Identifier: MIT
+
+#include
+
+#include
+
+#include "igt.h"
+
+struct eng_mmio_base_s {
+ char name[8];
+ uint32_t mmio_base;
+};
+
+struct eng_mmio_base_table_s {
+ unsigned int mb_cnt;
+ struct eng_mmio_base_s mb_tab[GEM_MAX_ENGINES];
+};
+
+
+static struct eng_mmio_base_table_s *_gem_engine_mmio_info_dup(
+ const struct eng_mmio_base_table_s *mbpi)
+{
+ struct eng_mmio_base_table_s *mbpo;
+ size_t nbytes;
+
+ nbytes = offsetof(typeof(struct eng_mmio_base_table_s),
mb_tab[mbpi->mb_cnt]);
+ mbpo = malloc(nbytes);
+ igt_assert(mbpo);
+ memcpy(mbpo, mbpi, nbytes);
+
+ return mbpo;
+}
+
+void gem_engine_mmio_base_info_free(struct eng_mmio_base_table_s *mbp)
+{
+ free(mbp);
+}
+
+static void _gem_engine_mmio_info_legacy_add(struct eng_mmio_base_table_s *mbp,
+ const char *eng_name, uint32_t mmio_base)
+{
+ if (mmio_base) {
+ strncpy(mbp->mb_tab[mbp->mb_cnt].name, eng_name,
+ sizeof(mbp->mb_tab[0].name));
+ mbp->mb_tab[mbp->mb_cnt].mmio_base = mmio_base;
+ mbp->mb_cnt++;
+ }
+}
+
+/**
+ * _gem_engine_mmio_base_info_get_legacy:
+ * @fd_dev: file descriptor upon which device is open or -1 to use defaults.
+ *
+ * Provides per-engine mmio_base information from legacy built-in values
+ * for the case when the information is not otherwise available.
+ *
+ * Returns:
+ * Pointer to dynamically allocated struct eng_mmio_base_table_s describing
+ * engine config or NULL.
+ * The allocated size does not include unused engine entries.
+ * If non-NULL, it is caller's responsibility to free.
+ */
+static struct eng_mmio_base_table_s *_gem_engine_mmio_base_info_get_legacy(int
fd_dev)
+{
+ int gen;
+ uint32_t mmio_base;
+ struct eng_mmio_base_table_s mbt;
+ struct eng_mmio_base_table_s *mbp;
+
+ memset(&mbt, 0, sizeof(mbt));
+
+ gen = intel_gen(intel_get_drm_devid(fd_dev));
+
+ /* The mmio_base values for engine instances 1 and higher cannot
+* be reliability determinated a priori. */
+
+ _gem_engine_mmio_info_legacy_add(&mbt, "rcs0", 0x2000);
+ _gem_engine_mmio_info_legacy_add(&mbt, "bcs0", 0x22000);
+
+ if (gen < 6)
+ mmio_base = 0x4000;
+ else if (gen < 11)
+ mmio_base = 0x12000;
+ else
+ mmio_base = 0x1c;
+ _gem_engine_mmio_info_legacy_add(&mbt, "vcs0", mmio_base);
+
+ if (gen < 11)
+ mmio_base = 0x1a000;
+ else
+ mmio_base = 0x1c8000;
+ _gem_engine_mmio_info_legacy_add(&mbt, "vecs0", mmio_base);
+
+ if (mbt.mb_cnt <= 0)
+ return NULL;
+
+ mbp = _gem_engine_mmio_info_dup(&mbt);
+
+ return mbp;
+}
+
+
+/**
+ * _gem_engine_mmio_base_info_get_debugfs:
+ * @fd_dev: file descriptor upon which device is open or -1 to use defaults.
+ *
+ * Obtains per-engine mmio_base information from debugfs.
+ *
+ * Returns:
+ * Pointer to dynamically allocated struct eng_mmio_base_table_s describing
+ * engine config or NULL.
+ * The allocated size does not include unused engine entries.
+ * If non-NULL, it is caller's responsibility to free.
+ *
+ * Looking in debugfs for per-engine instances of:
+ *
+ * ...
+ * MMIO base:
+ *
+ * Example of relevant lines from debugfs:
+ * vcs0
+ * MMIO base: 0x001c
+ * vcs1
+ * MMIO base: 0x001d
+ *
+ * In order to qualify as the introduction of a new per-engine section, an
+ * input line must consist solely of an engine name. An engine name must
+ * be 7 or fewer characters in length and must consist of an engine class
[Intel-gfx] [PATCH i-g-t 3/3] i915/gem_ctx_isolation: Check engine relative registers - Part 2
Modify previous i915/gem_ctx_isolation "Check engine relative registers"
for modified mmio_base infrastructure.
Signed-off-by: Dale B Stimson
---
tests/i915/gem_ctx_isolation.c | 87 +++---
1 file changed, 48 insertions(+), 39 deletions(-)
diff --git a/tests/i915/gem_ctx_isolation.c b/tests/i915/gem_ctx_isolation.c
index eff4b1df2..eec78c729 100644
--- a/tests/i915/gem_ctx_isolation.c
+++ b/tests/i915/gem_ctx_isolation.c
@@ -233,12 +233,12 @@ static bool ignore_register(uint32_t offset, uint32_t
mmio_base)
static void tmpl_regs(int fd,
uint32_t ctx,
const struct intel_execution_engine2 *e,
+ uint32_t mmio_base,
uint32_t handle,
uint32_t value)
{
const unsigned int gen_bit = 1 << intel_gen(intel_get_drm_devid(fd));
const unsigned int engine_bit = ENGINE(e->class, e->instance);
- const uint32_t mmio_base = gem_engine_mmio_base(fd, e->name);
unsigned int regs_size;
uint32_t *regs;
@@ -278,12 +278,12 @@ static void tmpl_regs(int fd,
static uint32_t read_regs(int fd,
uint32_t ctx,
const struct intel_execution_engine2 *e,
+ uint32_t mmio_base,
unsigned int flags)
{
const unsigned int gen = intel_gen(intel_get_drm_devid(fd));
const unsigned int gen_bit = 1 << gen;
const unsigned int engine_bit = ENGINE(e->class, e->instance);
- const uint32_t mmio_base = gem_engine_mmio_base(fd, e->name);
const bool r64b = gen >= 8;
struct drm_i915_gem_exec_object2 obj[2];
struct drm_i915_gem_relocation_entry *reloc;
@@ -359,12 +359,12 @@ static uint32_t read_regs(int fd,
static void write_regs(int fd,
uint32_t ctx,
const struct intel_execution_engine2 *e,
+ uint32_t mmio_base,
unsigned int flags,
uint32_t value)
{
const unsigned int gen_bit = 1 << intel_gen(intel_get_drm_devid(fd));
const unsigned int engine_bit = ENGINE(e->class, e->instance);
- const uint32_t mmio_base = gem_engine_mmio_base(fd, e->name);
struct drm_i915_gem_exec_object2 obj;
struct drm_i915_gem_execbuffer2 execbuf;
unsigned int batch_size;
@@ -420,13 +420,13 @@ static void write_regs(int fd,
static void restore_regs(int fd,
uint32_t ctx,
const struct intel_execution_engine2 *e,
+uint32_t mmio_base,
unsigned int flags,
uint32_t regs)
{
const unsigned int gen = intel_gen(intel_get_drm_devid(fd));
const unsigned int gen_bit = 1 << gen;
const unsigned int engine_bit = ENGINE(e->class, e->instance);
- const uint32_t mmio_base = gem_engine_mmio_base(fd, e->name);
const bool r64b = gen >= 8;
struct drm_i915_gem_exec_object2 obj[2];
struct drm_i915_gem_execbuffer2 execbuf;
@@ -498,12 +498,12 @@ static void restore_regs(int fd,
__attribute__((unused))
static void dump_regs(int fd,
const struct intel_execution_engine2 *e,
+ uint32_t mmio_base,
unsigned int regs)
{
const int gen = intel_gen(intel_get_drm_devid(fd));
const unsigned int gen_bit = 1 << gen;
const unsigned int engine_bit = ENGINE(e->class, e->instance);
- const uint32_t mmio_base = gem_engine_mmio_base(fd, e->name);
unsigned int regs_size;
uint32_t *out;
@@ -541,9 +541,9 @@ static void dump_regs(int fd,
}
static void compare_regs(int fd, const struct intel_execution_engine2 *e,
+uint32_t mmio_base,
uint32_t A, uint32_t B, const char *who)
{
- const uint32_t mmio_base = gem_engine_mmio_base(fd, e->name);
unsigned int num_errors;
unsigned int regs_size;
uint32_t *a, *b;
@@ -596,6 +596,7 @@ static void compare_regs(int fd, const struct
intel_execution_engine2 *e,
static void nonpriv(int fd,
const struct intel_execution_engine2 *e,
+ uint32_t mmio_base,
unsigned int flags)
{
static const uint32_t values[] = {
@@ -623,16 +624,16 @@ static void nonpriv(int fd,
ctx = gem_context_clone_with_engines(fd, 0);
- tmpl = read_regs(fd, ctx, e, flags);
- regs[0] = read_regs(fd, ctx, e, flags);
+ tmpl = read_regs(fd, ctx, e, mmio_base, flags);
+ regs[0] = read_regs(fd, ctx, e, mmio_base, flags);
- tmpl_regs(fd, ctx, e, tmpl, values[v]);
+ tmpl_regs(fd, ctx, e, mmio_ba
[Intel-gfx] [PATCH i-g-t 2/3] i915/gem_ctx_isolation: Check engine relative registers
From: Chris Wilson
Some of the non-privileged registers are at the same offset on each
engine. We can improve our coverage for unknown HW layout by using the
reported engine->mmio_base for relative offsets.
Signed-off-by: Chris Wilson
---
tests/i915/gem_ctx_isolation.c | 164 -
1 file changed, 100 insertions(+), 64 deletions(-)
diff --git a/tests/i915/gem_ctx_isolation.c b/tests/i915/gem_ctx_isolation.c
index 1b66fec11..eff4b1df2 100644
--- a/tests/i915/gem_ctx_isolation.c
+++ b/tests/i915/gem_ctx_isolation.c
@@ -70,6 +70,7 @@ static const struct named_register {
uint32_t ignore_bits;
uint32_t write_mask; /* some registers bits do not exist */
bool masked;
+ bool relative;
} nonpriv_registers[] = {
{ "NOPID", NOCTX, RCS0, 0x2094 },
{ "MI_PREDICATE_RESULT_2", NOCTX, RCS0, 0x23bc },
@@ -109,7 +110,6 @@ static const struct named_register {
{ "PS_DEPTH_COUNT_1", GEN8, RCS0, 0x22f8, 2 },
{ "BB_OFFSET", GEN8, RCS0, 0x2158, .ignore_bits = 0x7 },
{ "MI_PREDICATE_RESULT_1", GEN8, RCS0, 0x241c },
- { "CS_GPR", GEN8, RCS0, 0x2600, 32 },
{ "OA_CTX_CONTROL", GEN8, RCS0, 0x2360 },
{ "OACTXID", GEN8, RCS0, 0x2364 },
{ "PS_INVOCATION_COUNT_2", GEN8, RCS0, 0x2448, 2, .write_mask = ~0x3 },
@@ -138,79 +138,56 @@ static const struct named_register {
{ "CTX_PREEMPT", NOCTX /* GEN10 */, RCS0, 0x2248 },
{ "CS_CHICKEN1", GEN11, RCS0, 0x2580, .masked = true },
- { "HDC_CHICKEN1", GEN_RANGE(10, 10), RCS0, 0x7304, .masked = true },
/* Privileged (enabled by w/a + FORCE_TO_NONPRIV) */
{ "CTX_PREEMPT", NOCTX /* GEN9 */, RCS0, 0x2248 },
{ "CS_CHICKEN1", GEN_RANGE(9, 10), RCS0, 0x2580, .masked = true },
{ "COMMON_SLICE_CHICKEN2", GEN_RANGE(9, 9), RCS0, 0x7014, .masked =
true },
- { "HDC_CHICKEN1", GEN_RANGE(9, 9), RCS0, 0x7304, .masked = true },
+ { "HDC_CHICKEN1", GEN_RANGE(9, 10), RCS0, 0x7304, .masked = true },
{ "SLICE_COMMON_ECO_CHICKEN1", GEN_RANGE(11, 11) /* + glk */, RCS0,
0x731c, .masked = true },
{ "L3SQREG4", NOCTX /* GEN9:skl,kbl */, RCS0, 0xb118, .write_mask =
~0x10 },
{ "HALF_SLICE_CHICKEN7", GEN_RANGE(11, 11), RCS0, 0xe194, .masked =
true },
{ "SAMPLER_MODE", GEN_RANGE(11, 11), RCS0, 0xe18c, .masked = true },
- { "BCS_GPR", GEN9, BCS0, 0x22600, 32 },
{ "BCS_SWCTRL", GEN8, BCS0, 0x22200, .write_mask = 0x3, .masked = true
},
{ "MFC_VDBOX1", NOCTX, VCS0, 0x12800, 64 },
{ "MFC_VDBOX2", NOCTX, VCS1, 0x1c800, 64 },
- { "VCS0_GPR", GEN_RANGE(9, 10), VCS0, 0x12600, 32 },
- { "VCS1_GPR", GEN_RANGE(9, 10), VCS1, 0x1c600, 32 },
- { "VECS_GPR", GEN_RANGE(9, 10), VECS0, 0x1a600, 32 },
-
- { "VCS0_GPR", GEN11, VCS0, 0x1c0600, 32 },
- { "VCS1_GPR", GEN11, VCS1, 0x1c4600, 32 },
- { "VCS2_GPR", GEN11, VCS2, 0x1d0600, 32 },
- { "VCS3_GPR", GEN11, VCS3, 0x1d4600, 32 },
- { "VECS_GPR", GEN11, VECS0, 0x1c8600, 32 },
+ { "xCS_GPR", GEN9, ALL, 0x600, 32, .relative = true },
{}
}, ignore_registers[] = {
{ "RCS timestamp", GEN6, ~0u, 0x2358 },
{ "BCS timestamp", GEN7, ~0u, 0x22358 },
- { "VCS0 timestamp", GEN_RANGE(7, 10), ~0u, 0x12358 },
- { "VCS1 timestamp", GEN_RANGE(7, 10), ~0u, 0x1c358 },
- { "VECS timestamp", GEN_RANGE(8, 10), ~0u, 0x1a358 },
-
- { "VCS0 timestamp", GEN11, ~0u, 0x1c0358 },
- { "VCS1 timestamp", GEN11, ~0u, 0x1c4358 },
- { "VCS2 timestamp", GEN11, ~0u, 0x1d0358 },
- { "VCS3 timestamp", GEN11, ~0u, 0x1d4358 },
- { "VECS timestamp", GEN11, ~0u, 0x1c8358 },
+ { "xCS timestamp", GEN8, ALL, 0x358, .relative = true },
/* huc read only */
- { "BSD0 0x2000", GEN11, ~0u, 0x1c + 0x2000 },
- { "BSD0 0x2000", GEN11, ~0u, 0x1c + 0x2014 },
- { "BSD0 0x2000", GEN11, ~0u, 0x1c + 0x23b0 },
-
- { "BSD1 0x2000", GEN11, ~0u, 0x1c4000 + 0x2000 },
- { "BSD1 0x2000", GEN11, ~0u, 0x1c4000 + 0x2014 },
- { "BSD1 0x2000", GEN11, ~0u, 0x1c4000 + 0x23b0 },
-
- { "BSD2 0x2000", GEN11, ~0u, 0x1d + 0x2000 },
- { "BSD2 0x2000", GEN11, ~0u, 0x1d + 0x2014 },
- { "BSD2 0x2000", GEN11, ~0u, 0x1d + 0x23b0 },
-
- { "BSD3 0x2000", GEN11, ~0u, 0x1d4000 + 0x2000 },
- { "BSD3 0x2000", GEN11, ~0u, 0x1d4000 + 0x2014 },
- { "BSD3 0x2000", GEN11, ~0u, 0x1d4000 + 0x23b0 },
+ { "BSD 0x2000", GEN11, ALL, 0x2000, .relative = true },
+ { "BSD 0x2014", GEN11, ALL, 0x2014, .relative = true },
+ { "BSD 0x23b0", GEN11, ALL, 0x23b0, .relative = true },
{}
};
-static const char *register_name(uint32_t offset, char *buf, size_t len)
+static const char *
+register_name(uint32_t offset, uint32_t mmio_base, char *buf, size_t len)
{
for (const struct named_register *r = nonpriv_registers; r->name; r++) {
unsigne
[Intel-gfx] [PATCH i-g-t 1/3] i915/gem_mmio_base.c - get mmio_base from debugfs (if possible)
Signed-off-by: Dale B Stimson
---
lib/Makefile.sources | 2 +
lib/i915/gem_mmio_base.c | 346 +++
lib/i915/gem_mmio_base.h | 19 +++
lib/igt.h| 1 +
lib/meson.build | 1 +
5 files changed, 369 insertions(+)
create mode 100644 lib/i915/gem_mmio_base.c
create mode 100644 lib/i915/gem_mmio_base.h
diff --git a/lib/Makefile.sources b/lib/Makefile.sources
index 3e573f267..4c5d50d5d 100644
--- a/lib/Makefile.sources
+++ b/lib/Makefile.sources
@@ -7,6 +7,8 @@ lib_source_list = \
i915/gem_context.h \
i915/gem_engine_topology.c \
i915/gem_engine_topology.h \
+ i915/gem_mmio_base.c\
+ i915/gem_mmio_base.h\
i915/gem_scheduler.c\
i915/gem_scheduler.h\
i915/gem_submission.c \
diff --git a/lib/i915/gem_mmio_base.c b/lib/i915/gem_mmio_base.c
new file mode 100644
index 0..8718c092f
--- /dev/null
+++ b/lib/i915/gem_mmio_base.c
@@ -0,0 +1,346 @@
+// Copyright (C) 2020 Intel Corporation
+//
+// SPDX-License-Identifier: MIT
+
+#include
+
+#include
+
+#include "igt.h"
+
+struct eng_mmio_base_s {
+ char name[8];
+ uint32_t mmio_base;
+};
+
+struct eng_mmio_base_table_s {
+ unsigned int mb_cnt;
+ struct eng_mmio_base_s mb_tab[GEM_MAX_ENGINES];
+};
+
+
+static struct eng_mmio_base_table_s *_gem_engine_mmio_info_dup(
+ const struct eng_mmio_base_table_s *mbpi)
+{
+ struct eng_mmio_base_table_s *mbpo;
+ size_t nbytes;
+
+ nbytes = offsetof(typeof(struct eng_mmio_base_table_s),
mb_tab[mbpi->mb_cnt]);
+ mbpo = malloc(nbytes);
+ igt_assert(mbpo);
+ memcpy(mbpo, mbpi, nbytes);
+
+ return mbpo;
+}
+
+void gem_engine_mmio_base_info_free(struct eng_mmio_base_table_s *mbp)
+{
+ free(mbp);
+}
+
+static void _gem_engine_mmio_info_legacy_add(struct eng_mmio_base_table_s *mbp,
+ const char *eng_name, uint32_t mmio_base)
+{
+ if (mmio_base) {
+ strncpy(mbp->mb_tab[mbp->mb_cnt].name, eng_name,
+ sizeof(mbp->mb_tab[0].name));
+ mbp->mb_tab[mbp->mb_cnt].mmio_base = mmio_base;
+ mbp->mb_cnt++;
+ }
+}
+
+/**
+ * _gem_engine_mmio_base_info_get_legacy:
+ * @fd_dev: file descriptor upon which device is open or -1 to use defaults.
+ *
+ * Provides per-engine mmio_base information from legacy built-in values
+ * for the case when the information is not otherwise available.
+ *
+ * Returns:
+ * Pointer to dynamically allocated struct eng_mmio_base_table_s describing
+ * engine config or NULL.
+ * The allocated size does not include unused engine entries.
+ * If non-NULL, it is caller's responsibility to free.
+ */
+static struct eng_mmio_base_table_s *_gem_engine_mmio_base_info_get_legacy(int
fd_dev)
+{
+ int gen;
+ uint32_t mmio_base;
+ struct eng_mmio_base_table_s mbt;
+ struct eng_mmio_base_table_s *mbp;
+
+ memset(&mbt, 0, sizeof(mbt));
+
+ gen = intel_gen(intel_get_drm_devid(fd_dev));
+
+ /* The mmio_base values for engine instances 1 and higher cannot
+* be reliability determinated a priori. */
+
+ _gem_engine_mmio_info_legacy_add(&mbt, "rcs0", 0x2000);
+ _gem_engine_mmio_info_legacy_add(&mbt, "bcs0", 0x22000);
+
+ if (gen < 6)
+ mmio_base = 0x4000;
+ else if (gen < 11)
+ mmio_base = 0x12000;
+ else
+ mmio_base = 0x1c;
+ _gem_engine_mmio_info_legacy_add(&mbt, "vcs0", mmio_base);
+
+ if (gen < 11)
+ mmio_base = 0x1a000;
+ else
+ mmio_base = 0x1c8000;
+ _gem_engine_mmio_info_legacy_add(&mbt, "vecs0", mmio_base);
+
+ if (mbt.mb_cnt <= 0)
+ return NULL;
+
+ mbp = _gem_engine_mmio_info_dup(&mbt);
+
+ return mbp;
+}
+
+
+/**
+ * _gem_engine_mmio_base_info_get_debugfs:
+ * @fd_dev: file descriptor upon which device is open or -1 to use defaults.
+ *
+ * Obtains per-engine mmio_base information from debugfs.
+ *
+ * Returns:
+ * Pointer to dynamically allocated struct eng_mmio_base_table_s describing
+ * engine config or NULL.
+ * The allocated size does not include unused engine entries.
+ * If non-NULL, it is caller's responsibility to free.
+ *
+ * Looking in debugfs for per-engine instances of:
+ *
+ * ...
+ * MMIO base:
+ *
+ * Example of relevant lines from debugfs:
+ * vcs0
+ * MMIO base: 0x001c
+ * vcs1
+ * MMIO base: 0x001d
+ *
+ * In order to qualify as the introduction of a new per-engine section, an
+ * input line must consist solely of an engine name. An engine name must
+ * be 7 or fewer characters in length and must consist of an engine class
[Intel-gfx] [PATCH i-g-t 0/3] mmio_base via debugfs infrastructure + gem_ctx_isolation
This patch series provides infrastructure to allow determination of i915 per-engine mmio_base (which is otherwise sometimes hard to get). The provided method uses debugfs mmio_base information if present. Otherwise, a default determination is provided when possible. Also, gem_ctx_isolation is modified to use the new infrastructure. For example, on TGL, gem_ctx_isolation (without these or similar changes) will fail for subtests that use engine vcs1. The patches in this series are as they are intended to be submitted (subject to comments), except I would like to squash the two gem_ctx_isolation "relative registers" patches into one (as discussed below). Also, function gem_engine_mmio_base_info_dump() could be removed. On 2020-01-27, Chris wilson sent to the ML: [igt-dev] [PATCH i-g-t 1/5] i915: Start putting the mmio_base to wider use [igt-dev] [PATCH i-g-t 2/5] i915/gem_ctx_isolation: Check engine relative registers plus the following, which are not addressed here: [igt-dev] [PATCH i-g-t 3/5] i915: Exercise preemption timeout controls in sysfs [igt-dev] [PATCH i-g-t 4/5] i915: Exercise sysfs heartbeat controls [igt-dev] [PATCH i-g-t 5/5] i915: Exercise timeslice sysfs property This patch list is: i915/gem_mmio_base.c - get mmio_base from debugfs (if possible) i915/gem_ctx_isolation: Check engine relative registers i915/gem_ctx_isolation: Check engine relative registers - Part 2 The first 2020-01-27 patch obtains mmio_base information via sysfs, and depends on a proposed kernel change that would provide the mmio_base information via sysfs. It is unclear when or whether that kernel change will progress. The mmio_base information used by this patch series is available through debugfs now (as of kernel 5.4). If the per-engine mmio_base information is ever added to sysfs, it would be easy to plug that into the infrastructure proposed here as an additional higher-priority source of that information. This submission replaces the first patch (switching from sysfs to debugfs), retains the second 2020-01-27 patch i915/gem_ctx_isolation: Check engine relative registers and has a third patch that modifies the original second patch to support the altered API: i915/gem_ctx_isolation: Check engine relative registers - Part 2 I propose squashing the two gem_ctx_isolation "relative registers" patches into one patch with author == "Chris Wilson" if Chris agrees. Some differences from the 2020-01-27 patches: The mmio_base information is fetched once into local data structures, and is obtained from them thereafter instead of being fetched from the kernel everytime it is wanted. The function that obtains the mmio_base information is called by a particular test that wants it, and returns a handle through which the mmio_base can be requested for any particular engine. These patches introduce new source files lib/i915/gem_mmio_base.c and lib/i915/gem_mmio_base.h. Should the code instead be placed into lib/i915/gem_engine_topology.c? Function gem_engine_mmio_base_info_dump presently exists to dump the mmio_base information to stdout for debugging or informational purposes. This function is not currently called. I presume this function should be removed. Is there any desire to keep it around for future use? The 2020-01-27 patches define function gem_engine_mmio_base() with its first parameter as "fd". The new patches replace the first parameter with the mmio_base object handle. Chris Wilson (1): i915/gem_ctx_isolation: Check engine relative registers Dale B Stimson (2): i915/gem_mmio_base.c - get mmio_base from debugfs (if possible) i915/gem_ctx_isolation: Check engine relative registers - Part 2 lib/Makefile.sources | 2 + lib/i915/gem_mmio_base.c | 346 + lib/i915/gem_mmio_base.h | 19 ++ lib/igt.h | 1 + lib/meson.build| 1 + tests/i915/gem_ctx_isolation.c | 229 +- 6 files changed, 506 insertions(+), 92 deletions(-) create mode 100644 lib/i915/gem_mmio_base.c create mode 100644 lib/i915/gem_mmio_base.h -- 2.25.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t v3 1/2] i915/gem_ctx_isolation: gem_engine_topology, part 1
From: Ramalingam C
Call function gem_class_can_store_dword instead of legacy function
gem_can_store_dword. This requires that e->class be available in the
calling function.
Instead of passing "engine" (== "e->flags") to functions, pass "e".
This makes e->class available where it is needed.
This commit is being kept separate from "part 2" in order to ensure
proper attribution to the author. The code associated with this
commit was written by Ramalingam C .
Since then, slight modifications have been done due to upstream
changes.
Signed-off-by: Ramalingam C .
X-Original-Author: Ramalingam C
Signed-off-by: Dale B Stimson
---
tests/i915/gem_ctx_isolation.c | 25 +++--
1 file changed, 11 insertions(+), 14 deletions(-)
diff --git a/tests/i915/gem_ctx_isolation.c b/tests/i915/gem_ctx_isolation.c
index 8b72a16ad..c45617456 100644
--- a/tests/i915/gem_ctx_isolation.c
+++ b/tests/i915/gem_ctx_isolation.c
@@ -575,7 +575,6 @@ static void nonpriv(int fd,
0x0505c0c0,
0xdeadbeef
};
- unsigned int engine = e->flags;
unsigned int num_values = ARRAY_SIZE(values);
/* Sigh -- hsw: we need cmdparser access to our own registers! */
@@ -593,7 +592,7 @@ static void nonpriv(int fd,
tmpl_regs(fd, ctx, e, tmpl, values[v]);
- spin = igt_spin_new(fd, .ctx = ctx, .engine = engine);
+ spin = igt_spin_new(fd, .ctx = ctx, .engine = e->flags);
igt_debug("%s[%d]: Setting all registers to 0x%08x\n",
__func__, v, values[v]);
@@ -606,12 +605,12 @@ static void nonpriv(int fd,
/* Explicit sync to keep the switch between write/read
*/
syncpt = igt_spin_new(fd,
.ctx = ctx,
- .engine = engine,
+ .engine = e->flags,
.flags = IGT_SPIN_FENCE_OUT);
dirt = igt_spin_new(fd,
.ctx = sw,
- .engine = engine,
+ .engine = e->flags,
.fence = syncpt->out_fence,
.flags = (IGT_SPIN_FENCE_IN |
IGT_SPIN_FENCE_OUT));
@@ -619,7 +618,7 @@ static void nonpriv(int fd,
syncpt = igt_spin_new(fd,
.ctx = ctx,
- .engine = engine,
+ .engine = e->flags,
.fence = dirt->out_fence,
.flags = IGT_SPIN_FENCE_IN);
igt_spin_free(fd, dirt);
@@ -660,7 +659,6 @@ static void isolation(int fd,
0x,
0xdeadbeef
};
- unsigned int engine = e->flags;
unsigned int num_values =
flags & (DIRTY1 | DIRTY2) ? ARRAY_SIZE(values) : 1;
@@ -673,7 +671,7 @@ static void isolation(int fd,
ctx[0] = gem_context_create(fd);
regs[0] = read_regs(fd, ctx[0], e, flags);
- spin = igt_spin_new(fd, .ctx = ctx[0], .engine = engine);
+ spin = igt_spin_new(fd, .ctx = ctx[0], .engine = e->flags);
if (flags & DIRTY1) {
igt_debug("%s[%d]: Setting all registers of ctx 0 to
0x%08x\n",
@@ -726,11 +724,11 @@ static void isolation(int fd,
#define S4 (4 << 8)
#define SLEEP_MASK (0xf << 8)
-static void inject_reset_context(int fd, unsigned int engine)
+static void inject_reset_context(int fd, const struct intel_execution_engine2
*e)
{
struct igt_spin_factory opts = {
.ctx = gem_context_create(fd),
- .engine = engine,
+ .engine = e->flags,
.flags = IGT_SPIN_FAST,
};
igt_spin_t *spin;
@@ -741,7 +739,7 @@ static void inject_reset_context(int fd, unsigned int
engine)
* HW for screwing up if the context was already broken.
*/
- if (gem_can_store_dword(fd, engine))
+ if (gem_class_can_store_dword(fd, e->class))
opts.flags |= IGT_SPIN_POLL_RUN;
spin = __igt_spin_factory(fd, &opts);
@@ -771,7 +769,6 @@ static void preservation(int fd,
0xdeadbeef
};
const unsigned int num_values = ARRAY_SIZE(values);
- unsigned int engine = e->flags;
uint32_t ctx[num_values +1 ];
uint32_t regs[num_values + 1][2];
igt_spin_t *spin;
@@ -779,7 +776,7 @@ static void preservatio
[Intel-gfx] [PATCH i-g-t v3 2/2] i915/gem_ctx_isolation: gem_engine_topology, part 2
Switch from the existing engine iteration to using macro
__for_each_physical_engine which all engines that are actually present.
The invocation of __for_each_physical_engine sets the engine mapping
for context 0 to be all existing engines. Subsequent context creation
is done via gem_context_clone_with_engines so that all contexts share
the same engine mapping.
Signed-off-by: Dale B Stimson
---
tests/i915/gem_ctx_isolation.c | 19 +++
1 file changed, 11 insertions(+), 8 deletions(-)
diff --git a/tests/i915/gem_ctx_isolation.c b/tests/i915/gem_ctx_isolation.c
index c45617456..1b66fec11 100644
--- a/tests/i915/gem_ctx_isolation.c
+++ b/tests/i915/gem_ctx_isolation.c
@@ -586,7 +586,8 @@ static void nonpriv(int fd,
igt_spin_t *spin = NULL;
uint32_t ctx, regs[2], tmpl;
- ctx = gem_context_create(fd);
+ ctx = gem_context_clone_with_engines(fd, 0);
+
tmpl = read_regs(fd, ctx, e, flags);
regs[0] = read_regs(fd, ctx, e, flags);
@@ -599,7 +600,7 @@ static void nonpriv(int fd,
write_regs(fd, ctx, e, flags, values[v]);
if (flags & DIRTY2) {
- uint32_t sw = gem_context_create(fd);
+ uint32_t sw = gem_context_clone_with_engines(fd, 0);
igt_spin_t *syncpt, *dirt;
/* Explicit sync to keep the switch between write/read
*/
@@ -668,7 +669,7 @@ static void isolation(int fd,
igt_spin_t *spin = NULL;
uint32_t ctx[2], regs[2], tmp;
- ctx[0] = gem_context_create(fd);
+ ctx[0] = gem_context_clone_with_engines(fd, 0);
regs[0] = read_regs(fd, ctx[0], e, flags);
spin = igt_spin_new(fd, .ctx = ctx[0], .engine = e->flags);
@@ -687,7 +688,7 @@ static void isolation(int fd,
* the default values from this context, but if goes badly we
* see the corruption from the previous context instead!
*/
- ctx[1] = gem_context_create(fd);
+ ctx[1] = gem_context_clone_with_engines(fd, 0);
regs[1] = read_regs(fd, ctx[1], e, flags);
if (flags & DIRTY2) {
@@ -727,7 +728,7 @@ static void isolation(int fd,
static void inject_reset_context(int fd, const struct intel_execution_engine2
*e)
{
struct igt_spin_factory opts = {
- .ctx = gem_context_create(fd),
+ .ctx = gem_context_clone_with_engines(fd, 0),
.engine = e->flags,
.flags = IGT_SPIN_FAST,
};
@@ -775,11 +776,11 @@ static void preservation(int fd,
gem_quiescent_gpu(fd);
- ctx[num_values] = gem_context_create(fd);
+ ctx[num_values] = gem_context_clone_with_engines(fd, 0);
spin = igt_spin_new(fd, .ctx = ctx[num_values], .engine = e->flags);
regs[num_values][0] = read_regs(fd, ctx[num_values], e, flags);
for (int v = 0; v < num_values; v++) {
- ctx[v] = gem_context_create(fd);
+ ctx[v] = gem_context_clone_with_engines(fd, 0);
write_regs(fd, ctx[v], e, flags, values[v]);
regs[v][0] = read_regs(fd, ctx[v], e, flags);
@@ -874,7 +875,9 @@ igt_main
igt_skip_on(gen > LAST_KNOWN_GEN);
}
- __for_each_static_engine(e) {
+ /* __for_each_physical_engine switches context to all engines. */
+
+ __for_each_physical_engine(fd, e) {
igt_subtest_group {
igt_fixture {
igt_require(has_context_isolation & (1 <<
e->class));
--
2.25.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t v3 0/2] i915/gem_ctx_isolation: __for_each_physical_engine + engine mapping
i915/gem_ctx_isolation: __for_each_physical_engine and gem_engine_topology V2: Use new function gem_context_clone_with_engines V3: Revise commit messages including titles Use __for_each_physical_engine with gem_engine_topology. Iterates through the definitive list of all engines present. Patch 1 was originally posted by Ramalingam C . Since then, slight modifications have been done to it due to upstream changes. Patch 1 is being kept separate from patch 2 in order to assure proper attribution to the author. Dale B Stimson (1): i915/gem_ctx_isolation: gem_engine_topology, part 2 Ramalingam C (1): i915/gem_ctx_isolation: gem_engine_topology, part 1 tests/i915/gem_ctx_isolation.c | 44 +- 1 file changed, 22 insertions(+), 22 deletions(-) -- 2.25.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t v2 2/2] i915/gem_ctx_isolation: use the gem_engine_topology library, part 2
Switch from simple iteration over all potential engines to using
macro __for_each_physical_engine which only returns engines that are
actually present.
For each context (as it is created) call gem_context_set_all_engines
so that execbuf will interpret the engine specification in the new way.
Signed-off-by: Dale B Stimson
---
tests/i915/gem_ctx_isolation.c | 19 +++
1 file changed, 11 insertions(+), 8 deletions(-)
diff --git a/tests/i915/gem_ctx_isolation.c b/tests/i915/gem_ctx_isolation.c
index c45617456..1b66fec11 100644
--- a/tests/i915/gem_ctx_isolation.c
+++ b/tests/i915/gem_ctx_isolation.c
@@ -586,7 +586,8 @@ static void nonpriv(int fd,
igt_spin_t *spin = NULL;
uint32_t ctx, regs[2], tmpl;
- ctx = gem_context_create(fd);
+ ctx = gem_context_clone_with_engines(fd, 0);
+
tmpl = read_regs(fd, ctx, e, flags);
regs[0] = read_regs(fd, ctx, e, flags);
@@ -599,7 +600,7 @@ static void nonpriv(int fd,
write_regs(fd, ctx, e, flags, values[v]);
if (flags & DIRTY2) {
- uint32_t sw = gem_context_create(fd);
+ uint32_t sw = gem_context_clone_with_engines(fd, 0);
igt_spin_t *syncpt, *dirt;
/* Explicit sync to keep the switch between write/read
*/
@@ -668,7 +669,7 @@ static void isolation(int fd,
igt_spin_t *spin = NULL;
uint32_t ctx[2], regs[2], tmp;
- ctx[0] = gem_context_create(fd);
+ ctx[0] = gem_context_clone_with_engines(fd, 0);
regs[0] = read_regs(fd, ctx[0], e, flags);
spin = igt_spin_new(fd, .ctx = ctx[0], .engine = e->flags);
@@ -687,7 +688,7 @@ static void isolation(int fd,
* the default values from this context, but if goes badly we
* see the corruption from the previous context instead!
*/
- ctx[1] = gem_context_create(fd);
+ ctx[1] = gem_context_clone_with_engines(fd, 0);
regs[1] = read_regs(fd, ctx[1], e, flags);
if (flags & DIRTY2) {
@@ -727,7 +728,7 @@ static void isolation(int fd,
static void inject_reset_context(int fd, const struct intel_execution_engine2
*e)
{
struct igt_spin_factory opts = {
- .ctx = gem_context_create(fd),
+ .ctx = gem_context_clone_with_engines(fd, 0),
.engine = e->flags,
.flags = IGT_SPIN_FAST,
};
@@ -775,11 +776,11 @@ static void preservation(int fd,
gem_quiescent_gpu(fd);
- ctx[num_values] = gem_context_create(fd);
+ ctx[num_values] = gem_context_clone_with_engines(fd, 0);
spin = igt_spin_new(fd, .ctx = ctx[num_values], .engine = e->flags);
regs[num_values][0] = read_regs(fd, ctx[num_values], e, flags);
for (int v = 0; v < num_values; v++) {
- ctx[v] = gem_context_create(fd);
+ ctx[v] = gem_context_clone_with_engines(fd, 0);
write_regs(fd, ctx[v], e, flags, values[v]);
regs[v][0] = read_regs(fd, ctx[v], e, flags);
@@ -874,7 +875,9 @@ igt_main
igt_skip_on(gen > LAST_KNOWN_GEN);
}
- __for_each_static_engine(e) {
+ /* __for_each_physical_engine switches context to all engines. */
+
+ __for_each_physical_engine(fd, e) {
igt_subtest_group {
igt_fixture {
igt_require(has_context_isolation & (1 <<
e->class));
--
2.25.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t v2 0/2] i915/gem_ctx_isolation: __for_each_physical_engine + engine mapping
i915/gem_ctx_isolation: __for_each_physical_engine and gem_engine_topology V2: Use new function gem_context_clone_with_engines Use __for_each_physical_engine with gem_engine_topology. Iterates through the definitive list of all engines present. Patch 1 was originally posted by Ramalingam C . Since then, slight modifications have been done to it due to upstream changes. Patch 1 is being kept separate from patch 2 in order to assure proper attribution to the author. Dale B Stimson (1): i915/gem_ctx_isolation: use the gem_engine_topology library, part 2 Ramalingam C (1): i915/gem_ctx_isolation: use the gem_engine_topology library, part 1 tests/i915/gem_ctx_isolation.c | 44 +- 1 file changed, 22 insertions(+), 22 deletions(-) -- 2.25.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t v2 1/2] i915/gem_ctx_isolation: use the gem_engine_topology library, part 1
From: Ramalingam C
Call function gem_class_can_store_dword instead of legacy function
gem_can_store_dword. This requires that e->class be available in the
calling function.
Instead of passing "engine" (== "e->flags") to functions, pass "e".
This makes e->class available where it is needed.
This commit is being kept separate from "part 2" in order to ensure
proper attribution to the author. The code associated with this
commit was written by Ramalingam C .
Since then, slight modifications have been done due to upstream
changes.
Signed-off-by: Ramalingam C .
X-Original-Author: Ramalingam C
Signed-off-by: Dale B Stimson
---
tests/i915/gem_ctx_isolation.c | 25 +++--
1 file changed, 11 insertions(+), 14 deletions(-)
diff --git a/tests/i915/gem_ctx_isolation.c b/tests/i915/gem_ctx_isolation.c
index 8b72a16ad..c45617456 100644
--- a/tests/i915/gem_ctx_isolation.c
+++ b/tests/i915/gem_ctx_isolation.c
@@ -575,7 +575,6 @@ static void nonpriv(int fd,
0x0505c0c0,
0xdeadbeef
};
- unsigned int engine = e->flags;
unsigned int num_values = ARRAY_SIZE(values);
/* Sigh -- hsw: we need cmdparser access to our own registers! */
@@ -593,7 +592,7 @@ static void nonpriv(int fd,
tmpl_regs(fd, ctx, e, tmpl, values[v]);
- spin = igt_spin_new(fd, .ctx = ctx, .engine = engine);
+ spin = igt_spin_new(fd, .ctx = ctx, .engine = e->flags);
igt_debug("%s[%d]: Setting all registers to 0x%08x\n",
__func__, v, values[v]);
@@ -606,12 +605,12 @@ static void nonpriv(int fd,
/* Explicit sync to keep the switch between write/read
*/
syncpt = igt_spin_new(fd,
.ctx = ctx,
- .engine = engine,
+ .engine = e->flags,
.flags = IGT_SPIN_FENCE_OUT);
dirt = igt_spin_new(fd,
.ctx = sw,
- .engine = engine,
+ .engine = e->flags,
.fence = syncpt->out_fence,
.flags = (IGT_SPIN_FENCE_IN |
IGT_SPIN_FENCE_OUT));
@@ -619,7 +618,7 @@ static void nonpriv(int fd,
syncpt = igt_spin_new(fd,
.ctx = ctx,
- .engine = engine,
+ .engine = e->flags,
.fence = dirt->out_fence,
.flags = IGT_SPIN_FENCE_IN);
igt_spin_free(fd, dirt);
@@ -660,7 +659,6 @@ static void isolation(int fd,
0x,
0xdeadbeef
};
- unsigned int engine = e->flags;
unsigned int num_values =
flags & (DIRTY1 | DIRTY2) ? ARRAY_SIZE(values) : 1;
@@ -673,7 +671,7 @@ static void isolation(int fd,
ctx[0] = gem_context_create(fd);
regs[0] = read_regs(fd, ctx[0], e, flags);
- spin = igt_spin_new(fd, .ctx = ctx[0], .engine = engine);
+ spin = igt_spin_new(fd, .ctx = ctx[0], .engine = e->flags);
if (flags & DIRTY1) {
igt_debug("%s[%d]: Setting all registers of ctx 0 to
0x%08x\n",
@@ -726,11 +724,11 @@ static void isolation(int fd,
#define S4 (4 << 8)
#define SLEEP_MASK (0xf << 8)
-static void inject_reset_context(int fd, unsigned int engine)
+static void inject_reset_context(int fd, const struct intel_execution_engine2
*e)
{
struct igt_spin_factory opts = {
.ctx = gem_context_create(fd),
- .engine = engine,
+ .engine = e->flags,
.flags = IGT_SPIN_FAST,
};
igt_spin_t *spin;
@@ -741,7 +739,7 @@ static void inject_reset_context(int fd, unsigned int
engine)
* HW for screwing up if the context was already broken.
*/
- if (gem_can_store_dword(fd, engine))
+ if (gem_class_can_store_dword(fd, e->class))
opts.flags |= IGT_SPIN_POLL_RUN;
spin = __igt_spin_factory(fd, &opts);
@@ -771,7 +769,6 @@ static void preservation(int fd,
0xdeadbeef
};
const unsigned int num_values = ARRAY_SIZE(values);
- unsigned int engine = e->flags;
uint32_t ctx[num_values +1 ];
uint32_t regs[num_values + 1][2];
igt_spin_t *spin;
@@ -779,7 +776,7 @@ static void preservatio
Re: [Intel-gfx] [igt-dev] [PATCH i-g-t 2/2] DBS: tests/i915/gem_ctx_isolation: use the gem_engine_topology library, part 2
On 2020-01-23 15:59:33, Tvrtko Ursulin wrote: > gem_context_clone_with_engines was agreed upon in principle some time ago > but never implemented. I have now posted this as > https://patchwork.freedesktop.org/series/72464/ and plan to merge it once it > passes CI. > > Dale, Arjun, Krishnaiah and Sreedhar - you have in progress patches which > use gem_context_set_all_engines which will be gone and you will need to > adjust your work accordingly. > > Sreedhar specifically for your change in gem_exec_parallel we will need to > add a new helper which transfers the engine map from one fd/context to > another. I will copy you on a patch which will add it. I have a new iteration of the gem_ctx_isolation patches (on top of your patch and using gem_context_clone_with_engines) which I will submit to the ML as soon as your patch merges. Thanks, Dale ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t 2/2] DBS: tests/i915/gem_ctx_isolation: use the gem_engine_topology library, part 2
Switch from simple iteration over all potential engines to using
macro __for_each_physical_engine which only returns engines that are
actually present.
For each context (as it is created) call gem_context_set_all_engines
so that execbuf will interpret the engine specification in the new way.
Signed-off-by: Dale B Stimson
---
tests/i915/gem_ctx_isolation.c | 41 ++
1 file changed, 32 insertions(+), 9 deletions(-)
diff --git a/tests/i915/gem_ctx_isolation.c b/tests/i915/gem_ctx_isolation.c
index 25113b054..31a20ed3a 100644
--- a/tests/i915/gem_ctx_isolation.c
+++ b/tests/i915/gem_ctx_isolation.c
@@ -240,6 +240,25 @@ static bool ignore_register(uint32_t offset)
return false;
}
+/*
+ * context_create_plus_all_engines - Same as gem_context_create plus setup.
+ *
+ * This is a convenience function that may be called instead of the sequence
+ * of gem_context_create followed by gem_context_set_all_engines.
+ * If gem_has_engine_topology(), then function gem_context_set_all_engines
+ * indicates that future execbuf calls for this context should interpret the
+ * engine specification in a gem_engine_topology-compatible way.
+ */
+static uint32_t context_create_plus_all_engines(int fd)
+{
+ uint32_t ctx;
+
+ ctx = gem_context_create(fd);
+ gem_context_set_all_engines(fd, ctx);
+
+ return ctx;
+}
+
static void tmpl_regs(int fd,
uint32_t ctx,
const struct intel_execution_engine2 *e,
@@ -586,7 +605,8 @@ static void nonpriv(int fd,
igt_spin_t *spin = NULL;
uint32_t ctx, regs[2], tmpl;
- ctx = gem_context_create(fd);
+ ctx = context_create_plus_all_engines(fd);
+
tmpl = read_regs(fd, ctx, e, flags);
regs[0] = read_regs(fd, ctx, e, flags);
@@ -599,7 +619,7 @@ static void nonpriv(int fd,
write_regs(fd, ctx, e, flags, values[v]);
if (flags & DIRTY2) {
- uint32_t sw = gem_context_create(fd);
+ uint32_t sw = context_create_plus_all_engines(fd);
igt_spin_t *syncpt, *dirt;
/* Explicit sync to keep the switch between write/read
*/
@@ -668,7 +688,7 @@ static void isolation(int fd,
igt_spin_t *spin = NULL;
uint32_t ctx[2], regs[2], tmp;
- ctx[0] = gem_context_create(fd);
+ ctx[0] = context_create_plus_all_engines(fd);
regs[0] = read_regs(fd, ctx[0], e, flags);
spin = igt_spin_new(fd, .ctx = ctx[0], .engine = e->flags);
@@ -687,7 +707,7 @@ static void isolation(int fd,
* the default values from this context, but if goes badly we
* see the corruption from the previous context instead!
*/
- ctx[1] = gem_context_create(fd);
+ ctx[1] = context_create_plus_all_engines(fd);
regs[1] = read_regs(fd, ctx[1], e, flags);
if (flags & DIRTY2) {
@@ -727,7 +747,7 @@ static void isolation(int fd,
static void inject_reset_context(int fd, const struct intel_execution_engine2
*e)
{
struct igt_spin_factory opts = {
- .ctx = gem_context_create(fd),
+ .ctx = context_create_plus_all_engines(fd),
.engine = e->flags,
.flags = IGT_SPIN_FAST,
};
@@ -775,11 +795,11 @@ static void preservation(int fd,
gem_quiescent_gpu(fd);
- ctx[num_values] = gem_context_create(fd);
+ ctx[num_values] = context_create_plus_all_engines(fd);
spin = igt_spin_new(fd, .ctx = ctx[num_values], .engine = e->flags);
regs[num_values][0] = read_regs(fd, ctx[num_values], e, flags);
for (int v = 0; v < num_values; v++) {
- ctx[v] = gem_context_create(fd);
+ ctx[v] = context_create_plus_all_engines(fd);
write_regs(fd, ctx[v], e, flags, values[v]);
regs[v][0] = read_regs(fd, ctx[v], e, flags);
@@ -854,6 +874,7 @@ static unsigned int __has_context_isolation(int fd)
igt_main
{
unsigned int has_context_isolation = 0;
+ const struct intel_execution_engine2 *e;
int fd = -1;
igt_fixture {
@@ -871,10 +892,12 @@ igt_main
igt_warn_on_f(gen > LAST_KNOWN_GEN,
"GEN not recognized! Test needs to be updated to
run.\n");
igt_skip_on(gen > LAST_KNOWN_GEN);
+
+ /* Context 0 is created on device open. */
+ gem_context_set_all_engines(fd, 0);
}
- for (const struct intel_execution_engine2 *e = intel_execution_engines2;
-e->name; e++) {
+ __for_each_physical_engine(fd, e) {
igt_subtest_group {
igt_fixture {
igt_require(
[Intel-gfx] [PATCH i-g-t 1/2] tests/i915/gem_ctx_isolation: use the gem_engine_topology library, part 1
From: Ramalingam C
Call function gem_class_can_store_dword instead of legacy function
gem_can_store_dword. This requires that e->class be available in the
calling function.
Instead of passing "engine" (== "e->flags") to functions, pass "e".
This makes e->class available where it is needed.
This commit is being kept separate from "part 2" in order to assure
proper attribution to the author. The code associated with this
commit was written by Ramalingam C .
Since then, slight modifications have been done due to upstream
changes.
Signed-off-by: Ramalingam C .
Signed-off-by: Dale B Stimson
---
tests/i915/gem_ctx_isolation.c | 25 +++--
1 file changed, 11 insertions(+), 14 deletions(-)
diff --git a/tests/i915/gem_ctx_isolation.c b/tests/i915/gem_ctx_isolation.c
index 6aa27133c..25113b054 100644
--- a/tests/i915/gem_ctx_isolation.c
+++ b/tests/i915/gem_ctx_isolation.c
@@ -575,7 +575,6 @@ static void nonpriv(int fd,
0x0505c0c0,
0xdeadbeef
};
- unsigned int engine = e->flags;
unsigned int num_values = ARRAY_SIZE(values);
/* Sigh -- hsw: we need cmdparser access to our own registers! */
@@ -593,7 +592,7 @@ static void nonpriv(int fd,
tmpl_regs(fd, ctx, e, tmpl, values[v]);
- spin = igt_spin_new(fd, .ctx = ctx, .engine = engine);
+ spin = igt_spin_new(fd, .ctx = ctx, .engine = e->flags);
igt_debug("%s[%d]: Setting all registers to 0x%08x\n",
__func__, v, values[v]);
@@ -606,12 +605,12 @@ static void nonpriv(int fd,
/* Explicit sync to keep the switch between write/read
*/
syncpt = igt_spin_new(fd,
.ctx = ctx,
- .engine = engine,
+ .engine = e->flags,
.flags = IGT_SPIN_FENCE_OUT);
dirt = igt_spin_new(fd,
.ctx = sw,
- .engine = engine,
+ .engine = e->flags,
.fence = syncpt->out_fence,
.flags = (IGT_SPIN_FENCE_IN |
IGT_SPIN_FENCE_OUT));
@@ -619,7 +618,7 @@ static void nonpriv(int fd,
syncpt = igt_spin_new(fd,
.ctx = ctx,
- .engine = engine,
+ .engine = e->flags,
.fence = dirt->out_fence,
.flags = IGT_SPIN_FENCE_IN);
igt_spin_free(fd, dirt);
@@ -660,7 +659,6 @@ static void isolation(int fd,
0x,
0xdeadbeef
};
- unsigned int engine = e->flags;
unsigned int num_values =
flags & (DIRTY1 | DIRTY2) ? ARRAY_SIZE(values) : 1;
@@ -673,7 +671,7 @@ static void isolation(int fd,
ctx[0] = gem_context_create(fd);
regs[0] = read_regs(fd, ctx[0], e, flags);
- spin = igt_spin_new(fd, .ctx = ctx[0], .engine = engine);
+ spin = igt_spin_new(fd, .ctx = ctx[0], .engine = e->flags);
if (flags & DIRTY1) {
igt_debug("%s[%d]: Setting all registers of ctx 0 to
0x%08x\n",
@@ -726,11 +724,11 @@ static void isolation(int fd,
#define S4 (4 << 8)
#define SLEEP_MASK (0xf << 8)
-static void inject_reset_context(int fd, unsigned int engine)
+static void inject_reset_context(int fd, const struct intel_execution_engine2
*e)
{
struct igt_spin_factory opts = {
.ctx = gem_context_create(fd),
- .engine = engine,
+ .engine = e->flags,
.flags = IGT_SPIN_FAST,
};
igt_spin_t *spin;
@@ -741,7 +739,7 @@ static void inject_reset_context(int fd, unsigned int
engine)
* HW for screwing up if the context was already broken.
*/
- if (gem_can_store_dword(fd, engine))
+ if (gem_class_can_store_dword(fd, e->class))
opts.flags |= IGT_SPIN_POLL_RUN;
spin = __igt_spin_factory(fd, &opts);
@@ -771,7 +769,6 @@ static void preservation(int fd,
0xdeadbeef
};
const unsigned int num_values = ARRAY_SIZE(values);
- unsigned int engine = e->flags;
uint32_t ctx[num_values +1 ];
uint32_t regs[num_values + 1][2];
igt_spin_t *spin;
@@ -779,7 +776,7 @@ static void preservation(int fd,
gem_quies
[Intel-gfx] [PATCH i-g-t 0/2] i915/gem_ctx_isolation: __for_each_physical_engine + engine mapping
i915/gem_ctx_isolation: __for_each_physical_engine and gem_engine_topology Use __for_each_physical_engine with gem_engine_topology. Iterates through the definitive list of all engines present. All contexts have their engine mapping set via gem_context_set_all_engines(). Patch 1 was originally posted by Ramalingam C . Since then, slight modifications have been done to it due to upstream changes. Patch 1 is being kept separate from patch 2 in order to assure proper attribution to the author. Dale B Stimson (1): DBS: tests/i915/gem_ctx_isolation: use the gem_engine_topology library, part 2 Ramalingam C (1): tests/i915/gem_ctx_isolation: use the gem_engine_topology library, part 1 tests/i915/gem_ctx_isolation.c | 66 ++ 1 file changed, 43 insertions(+), 23 deletions(-) -- 2.25.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t v4 1/1] gem_ctx_isolation.c - Gen11 enabling for context isolation test
CS_CHICKEN1 is not privileged anymore (as of Gen11), as evidenced
by its absence from the kernel whitelist for Gen11 combined with the
successful execution on ICL of the tests added by your recent patch
"i915/gem_ctx_isolation: Sanitycheck nonpriv access".
-Dale
On 2019-03-05 19:00:49, Chris Wilson wrote:
> Quoting Dale B Stimson (2019-03-05 01:03:08)
> > @@ -132,30 +136,49 @@ static const struct named_register {
> > { "PERF_CNT_1", NOCTX, RCS0, 0x91b8, 2 },
> > { "PERF_CNT_2", NOCTX, RCS0, 0x91c0, 2 },
> >
> > + { "CTX_PREEMPT", NOCTX /* GEN10 */, RCS0, 0x2248 },
> > + { "CS_CHICKEN1", GEN11, RCS0, 0x2580, .masked = true },
>
> CS_CHICKEN1 is still privileged? At least I'm push a patch to add it to
> the whitelist for gen11.
> -Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t v4 1/1] gem_ctx_isolation.c - Gen11 enabling for context isolation test
This patch is a change to igt file tests/i915/gem_ctx_isolation.c to
add and enable Gen11 support.
This patch accounts for whitelisted registers appropriately for the
different Gen levels.
This patch accounts for the changed MMIO offsets of Gen11.
This patch redefines MAX_REG from 0x4 to 0x20 due to the
larger total register space for Gen11 mmio offsets.
A current Gen11 SKU has two video engines (with indexes 0 and 2,
for VCS0 and VCS2), with VCS1 not being used.
Current kernel and igt limitations only allow for VCS0 and VCS1.
Those limitations are in the process of being removed. See for
example the RFC/PATCH series on igt-dev from Andy Shyti:
[igt-dev] [RFC PATCH v9 0/5] new engine discovery interface
which depends on in-process kernel "media scalability" patches.
Lacking the above infrastructure at the moment:
The array of registers to be tested includes VCS2 and VCS3 registers.
They are present as a provision for the future, but they will
not actually be tested as those engines are not yet known to the
underlying infrastructure.
When run on Gen11 this patch skips the sub-tests for the non-existent VCS1
with these warnings:
Test requirement not met in function gem_require_engine, file
../lib/igt_gt.h:114:
Test requirement: gem_has_engine(gem_fd, class, instance)
Signed-off-by: Dale B Stimson
---
tests/i915/gem_ctx_isolation.c | 47 +-
1 file changed, 35 insertions(+), 12 deletions(-)
diff --git a/tests/i915/gem_ctx_isolation.c b/tests/i915/gem_ctx_isolation.c
index e50cc9a7..f1000458 100644
--- a/tests/i915/gem_ctx_isolation.c
+++ b/tests/i915/gem_ctx_isolation.c
@@ -24,7 +24,7 @@
#include "igt.h"
#include "igt_dummyload.h"
-#define MAX_REG 0x4
+#define MAX_REG 0x20
#define NUM_REGS (MAX_REG / sizeof(uint32_t))
#define PAGE_ALIGN(x) ALIGN(x, 4096)
@@ -41,6 +41,8 @@ enum {
BCS0 = ENGINE(I915_ENGINE_CLASS_COPY, 0),
VCS0 = ENGINE(I915_ENGINE_CLASS_VIDEO, 0),
VCS1 = ENGINE(I915_ENGINE_CLASS_VIDEO, 1),
+ VCS2 = ENGINE(I915_ENGINE_CLASS_VIDEO, 2),
+ VCS3 = ENGINE(I915_ENGINE_CLASS_VIDEO, 3),
VECS0 = ENGINE(I915_ENGINE_CLASS_VIDEO_ENHANCE, 0),
};
@@ -52,10 +54,12 @@ enum {
#define GEN7 (ALL << 7)
#define GEN8 (ALL << 8)
#define GEN9 (ALL << 9)
+#define GEN10 (ALL << 10)
+#define GEN11 (ALL << 11)
#define NOCTX 0
-#define LAST_KNOWN_GEN 10
+#define LAST_KNOWN_GEN 11
static const struct named_register {
const char *name;
@@ -103,7 +107,7 @@ static const struct named_register {
{ "GPGPU_THREADS_DISPATCHED", GEN8, RCS0, 0x2290, 2 },
{ "PS_INVOCATION_COUNT_1", GEN8, RCS0, 0x22f0, 2, .write_mask = ~0x3 },
{ "PS_DEPTH_COUNT_1", GEN8, RCS0, 0x22f8, 2 },
- { "BB_OFFSET", GEN8, RCS0, 0x2158, .ignore_bits = 0x4 },
+ { "BB_OFFSET", GEN8, RCS0, 0x2158, .ignore_bits = 0x7 },
{ "MI_PREDICATE_RESULT_1", GEN8, RCS0, 0x241c },
{ "CS_GPR", GEN8, RCS0, 0x2600, 32 },
{ "OA_CTX_CONTROL", GEN8, RCS0, 0x2360 },
@@ -132,30 +136,49 @@ static const struct named_register {
{ "PERF_CNT_1", NOCTX, RCS0, 0x91b8, 2 },
{ "PERF_CNT_2", NOCTX, RCS0, 0x91c0, 2 },
+ { "CTX_PREEMPT", NOCTX /* GEN10 */, RCS0, 0x2248 },
+ { "CS_CHICKEN1", GEN11, RCS0, 0x2580, .masked = true },
+ { "HDC_CHICKEN1", GEN_RANGE(10, 10), RCS0, 0x7304, .masked = true },
+
/* Privileged (enabled by w/a + FORCE_TO_NONPRIV) */
- { "CTX_PREEMPT", NOCTX /* GEN_RANGE(9, 10) */, RCS0, 0x2248 },
+ { "CTX_PREEMPT", NOCTX /* GEN9 */, RCS0, 0x2248 },
{ "CS_CHICKEN1", GEN_RANGE(9, 10), RCS0, 0x2580, .masked = true },
- { "HDC_CHICKEN1", GEN_RANGE(9, 10), RCS0, 0x7304, .masked = true },
+ { "HDC_CHICKEN1", GEN_RANGE(9, 9), RCS0, 0x7304, .masked = true },
{ "L3SQREG4", NOCTX /* GEN9:skl,kbl */, RCS0, 0xb118, .write_mask =
~0x10 },
+ { "HALF_SLICE_CHICKEN7", GEN_RANGE(11, 11), RCS0, 0xe194, .masked =
true },
+ { "SAMPLER_MODE", GEN_RANGE(11, 11), RCS0, 0xe18c, .masked = true },
{ "BCS_GPR", GEN9, BCS0, 0x22600, 32 },
{ "BCS_SWCTRL", GEN8, BCS0, 0x22200, .write_mask = 0x3, .masked = true
},
- { "VCS0_GPR", GEN9, VCS0, 0x12600, 32 },
{ "MFC_VDBOX1", NOCTX, VCS0, 0x12800, 64 },
-
- { "VCS1_GPR", GEN9, VCS1, 0x1c600, 32 },
{ "MFC_VDBOX2", NOCTX, VCS1, 0x1c800, 64 },
- { "VECS_GPR", GEN9, VECS0, 0x1a600, 32 },
+ { "VCS0_GPR", GEN_RANGE(9, 10), VCS0, 0x12600, 32 },
+ { "VCS1_GPR", GEN_RANGE(9, 10), VCS1, 0x1c600, 32 },
+ { "VECS_GPR", GEN_RANGE(9,
[Intel-gfx] [PATCH i-g-t v4 0/1] gem_ctx_isolation.c - Gen11 enabling for context isolation test
V4:
I have tested these changes on both SKL and ICL with no regressions
detected.
I will note that both SKL and ICL seem to currently have (at least for
my environment) suspend/resume issues which occur with or without these
changes (and also for gem_exec_suspend). Therefore, the S3/S4 tests
were not done.
Testing on ICL shows that Gen11 requires BB_OFFSET .ignore_bits =
0x7 instead of 0x4. I presume that the preferred way to do this is to
change the existing table entry for BB_OFFSET instead of splitting it
into separate entries for GEN8-10 and GEN11.
For those registers that are force_nonpriv for some Gen levels and
not for others, I have chosen to show the two states as separate table
entries to make this clear. In particular, this applies as shown below.
Any objections to doing it that way?
+ { "CTX_PREEMPT", NOCTX /* GEN10 */, RCS0, 0x2248 },
+ { "CS_CHICKEN1", GEN11, RCS0, 0x2580, .masked = true },
+ { "HDC_CHICKEN1", GEN_RANGE(10, 10), RCS0, 0x7304, .masked = true },
+
/* Privileged (enabled by w/a + FORCE_TO_NONPRIV) */
- { "CTX_PREEMPT", NOCTX /* GEN_RANGE(9, 10) */, RCS0, 0x2248 },
+ { "CTX_PREEMPT", NOCTX /* GEN9 */, RCS0, 0x2248 },
{ "CS_CHICKEN1", GEN_RANGE(9, 10), RCS0, 0x2580, .masked = true },
- { "HDC_CHICKEN1", GEN_RANGE(9, 10), RCS0, 0x7304, .masked = true },
+ { "HDC_CHICKEN1", GEN_RANGE(9, 9), RCS0, 0x7304, .masked = true },
Dale B Stimson (1):
gem_ctx_isolation.c - Gen11 enabling for context isolation test
tests/i915/gem_ctx_isolation.c | 47 +-
1 file changed, 35 insertions(+), 12 deletions(-)
--
2.21.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t] i915/gem_ctx_isolation: Sanitycheck nonpriv access
Reviewed-By: Dale B Stimson
On Fri, Mar 01, 2019 at 08:19:19AM +, Chris Wilson wrote:
> Verify that our list of nonpriv registers exist and are writable.
>
> v2: TD_CTL has a write_mask of 0x instead of being a masked
> register.
>
> Signed-off-by: Chris Wilson
> Cc: Dale B Stimson
> Cc: Tvrtko Ursulin
> Cc: Joonas Lahtinen
> ---
> tests/i915/gem_ctx_isolation.c | 167 +++--
> 1 file changed, 138 insertions(+), 29 deletions(-)
>
> diff --git a/tests/i915/gem_ctx_isolation.c b/tests/i915/gem_ctx_isolation.c
> index 839d49ade..e50cc9a72 100644
> --- a/tests/i915/gem_ctx_isolation.c
> +++ b/tests/i915/gem_ctx_isolation.c
> @@ -59,16 +59,23 @@ enum {
>
> static const struct named_register {
> const char *name;
> - unsigned int gen_mask;
> - unsigned int engine_mask;
> - uint32_t offset;
> + unsigned int gen_mask; /* on which gen the register exists */
> + unsigned int engine_mask; /* preferred engine / powerwell */
> + uint32_t offset; /* address of register, from bottom of mmio bar */
> uint32_t count;
> uint32_t ignore_bits;
> + uint32_t write_mask; /* some registers bits do not exist */
> bool masked;
> } nonpriv_registers[] = {
> { "NOPID", NOCTX, RCS0, 0x2094 },
> { "MI_PREDICATE_RESULT_2", NOCTX, RCS0, 0x23bc },
> - { "INSTPM", GEN6, RCS0, 0x20c0, 1, BIT(8) /* ro counter */, true },
> + {
> + "INSTPM",
> + GEN6, RCS0, 0x20c0,
> + .ignore_bits = BIT(8) /* ro counter */,
> + .write_mask = BIT(8) /* rsvd varies between gen */,
> + .masked = true,
> + },
> { "IA_VERTICES_COUNT", GEN4, RCS0, 0x2310, 2 },
> { "IA_PRIMITIVES_COUNT", GEN4, RCS0, 0x2318, 2 },
> { "VS_INVOCATION_COUNT", GEN4, RCS0, 0x2320, 2 },
> @@ -78,7 +85,7 @@ static const struct named_register {
> { "GS_PRIMITIVES_COUNT", GEN4, RCS0, 0x2330, 2 },
> { "CL_INVOCATION_COUNT", GEN4, RCS0, 0x2338, 2 },
> { "CL_PRIMITIVES_COUNT", GEN4, RCS0, 0x2340, 2 },
> - { "PS_INVOCATION_COUNT_0", GEN4, RCS0, 0x22c8, 2 },
> + { "PS_INVOCATION_COUNT_0", GEN4, RCS0, 0x22c8, 2, .write_mask = ~0x3 },
> { "PS_DEPTH_COUNT_0", GEN4, RCS0, 0x22d8, 2 },
> { "GPUGPU_DISPATCHDIMX", GEN8, RCS0, 0x2500 },
> { "GPUGPU_DISPATCHDIMY", GEN8, RCS0, 0x2504 },
> @@ -86,7 +93,7 @@ static const struct named_register {
> { "MI_PREDICATE_SRC0", GEN8, RCS0, 0x2400, 2 },
> { "MI_PREDICATE_SRC1", GEN8, RCS0, 0x2408, 2 },
> { "MI_PREDICATE_DATA", GEN8, RCS0, 0x2410, 2 },
> - { "MI_PRED_RESULT", GEN8, RCS0, 0x2418 },
> + { "MI_PRED_RESULT", GEN8, RCS0, 0x2418, .write_mask = 0x1 },
> { "3DPRIM_END_OFFSET", GEN6, RCS0, 0x2420 },
> { "3DPRIM_START_VERTEX", GEN6, RCS0, 0x2430 },
> { "3DPRIM_VERTEX_COUNT", GEN6, RCS0, 0x2434 },
> @@ -94,45 +101,45 @@ static const struct named_register {
> { "3DPRIM_START_INSTANCE", GEN6, RCS0, 0x243c },
> { "3DPRIM_BASE_VERTEX", GEN6, RCS0, 0x2440 },
> { "GPGPU_THREADS_DISPATCHED", GEN8, RCS0, 0x2290, 2 },
> - { "PS_INVOCATION_COUNT_1", GEN8, RCS0, 0x22f0, 2 },
> + { "PS_INVOCATION_COUNT_1", GEN8, RCS0, 0x22f0, 2, .write_mask = ~0x3 },
> { "PS_DEPTH_COUNT_1", GEN8, RCS0, 0x22f8, 2 },
> { "BB_OFFSET", GEN8, RCS0, 0x2158, .ignore_bits = 0x4 },
> { "MI_PREDICATE_RESULT_1", GEN8, RCS0, 0x241c },
> { "CS_GPR", GEN8, RCS0, 0x2600, 32 },
> { "OA_CTX_CONTROL", GEN8, RCS0, 0x2360 },
> { "OACTXID", GEN8, RCS0, 0x2364 },
> - { "PS_INVOCATION_COUNT_2", GEN8, RCS0, 0x2448, 2 },
> + { "PS_INVOCATION_COUNT_2", GEN8, RCS0, 0x2448, 2, .write_mask = ~0x3 },
> { "PS_DEPTH_COUNT_2", GEN8, RCS0, 0x2450, 2 },
> - { "Cache_Mode_0", GEN7, RCS0, 0x7000 },
> - { "Cache_Mode_1", GEN7, RCS0, 0x7004 },
> - { "GT_MODE", GEN8, RCS0, 0x7008 },
> - { "L3_Config", GEN7, RCS0, 0x7034 },
> - { "TD_CTL", GEN8, RCS0, 0xe400 },
> + { "Cache_Mode_0", GEN7, RCS0, 0x7000, .masked = true },
> + { "Cache_Mode_1", GEN7, RCS0, 0x7004, .masked = true },
> + { "GT_MODE", GEN8, RCS0, 0x7008, .masked = true },
> + { "L3_Config", GEN8, RCS0, 0x7034 },
> + { "TD_CTL", GEN8, RCS0,
Re: [Intel-gfx] [PATCH i-g-t] i915/gem_ctx_isolation: Sanitycheck nonpriv access
On Sat, Feb 23, 2019 at 09:45:10AM +, Chris Wilson wrote:
> Verify that our list of nonpriv registers exist and are writable.
>
> Signed-off-by: Chris Wilson
> Cc: Dale B Stimson
> Cc: Tvrtko Ursulin
> Cc: Joonas Lahtinen
> ---
> tests/i915/gem_ctx_isolation.c | 164 +++--
> 1 file changed, 135 insertions(+), 29 deletions(-)
>
> diff --git a/tests/i915/gem_ctx_isolation.c b/tests/i915/gem_ctx_isolation.c
> index 839d49ad..991a997f 100644
> --- a/tests/i915/gem_ctx_isolation.c
> +++ b/tests/i915/gem_ctx_isolation.c
> @@ -59,16 +59,23 @@ enum {
>
> static const struct named_register {
> const char *name;
> - unsigned int gen_mask;
> - unsigned int engine_mask;
> - uint32_t offset;
> + unsigned int gen_mask; /* on which gen the register exists */
> + unsigned int engine_mask; /* preferred engine / powerwell */
> + uint32_t offset; /* address of register, from bottom of mmio bar */
> uint32_t count;
> uint32_t ignore_bits;
> + uint32_t write_mask; /* some registers bits do not exist */
> bool masked;
> } nonpriv_registers[] = {
> { "NOPID", NOCTX, RCS0, 0x2094 },
> { "MI_PREDICATE_RESULT_2", NOCTX, RCS0, 0x23bc },
> - { "INSTPM", GEN6, RCS0, 0x20c0, 1, BIT(8) /* ro counter */, true },
> + {
> + "INSTPM",
> + GEN6, RCS0, 0x20c0,
> + .ignore_bits = BIT(8) /* ro counter */,
> + .write_mask = BIT(8) /* rsvd varies between gen */,
> + .masked = true,
> + },
> { "IA_VERTICES_COUNT", GEN4, RCS0, 0x2310, 2 },
> { "IA_PRIMITIVES_COUNT", GEN4, RCS0, 0x2318, 2 },
> { "VS_INVOCATION_COUNT", GEN4, RCS0, 0x2320, 2 },
> @@ -78,7 +85,7 @@ static const struct named_register {
> { "GS_PRIMITIVES_COUNT", GEN4, RCS0, 0x2330, 2 },
> { "CL_INVOCATION_COUNT", GEN4, RCS0, 0x2338, 2 },
> { "CL_PRIMITIVES_COUNT", GEN4, RCS0, 0x2340, 2 },
> - { "PS_INVOCATION_COUNT_0", GEN4, RCS0, 0x22c8, 2 },
> + { "PS_INVOCATION_COUNT_0", GEN4, RCS0, 0x22c8, 2, .write_mask = ~0x3 },
I can't find a reason for adding ".write_mask = ~0x3".
Among other places, I've looked in:
https://01.org/sites/default/files/documentation/intel-gfx-prm-osrc-kbl-vol02c-commandreference-registers-part2_0.pdf
> { "PS_DEPTH_COUNT_0", GEN4, RCS0, 0x22d8, 2 },
> { "GPUGPU_DISPATCHDIMX", GEN8, RCS0, 0x2500 },
> { "GPUGPU_DISPATCHDIMY", GEN8, RCS0, 0x2504 },
> @@ -86,7 +93,7 @@ static const struct named_register {
> { "MI_PREDICATE_SRC0", GEN8, RCS0, 0x2400, 2 },
> { "MI_PREDICATE_SRC1", GEN8, RCS0, 0x2408, 2 },
> { "MI_PREDICATE_DATA", GEN8, RCS0, 0x2410, 2 },
> - { "MI_PRED_RESULT", GEN8, RCS0, 0x2418 },
> + { "MI_PRED_RESULT", GEN8, RCS0, 0x2418, .write_mask = 0x1 },
> { "3DPRIM_END_OFFSET", GEN6, RCS0, 0x2420 },
> { "3DPRIM_START_VERTEX", GEN6, RCS0, 0x2430 },
> { "3DPRIM_VERTEX_COUNT", GEN6, RCS0, 0x2434 },
> @@ -94,45 +101,45 @@ static const struct named_register {
> { "3DPRIM_START_INSTANCE", GEN6, RCS0, 0x243c },
> { "3DPRIM_BASE_VERTEX", GEN6, RCS0, 0x2440 },
> { "GPGPU_THREADS_DISPATCHED", GEN8, RCS0, 0x2290, 2 },
> - { "PS_INVOCATION_COUNT_1", GEN8, RCS0, 0x22f0, 2 },
> + { "PS_INVOCATION_COUNT_1", GEN8, RCS0, 0x22f0, 2, .write_mask = ~0x3 },
Same comment as for "PS_INVOCATION_COUNT_0" above.
> { "PS_DEPTH_COUNT_1", GEN8, RCS0, 0x22f8, 2 },
> { "BB_OFFSET", GEN8, RCS0, 0x2158, .ignore_bits = 0x4 },
> { "MI_PREDICATE_RESULT_1", GEN8, RCS0, 0x241c },
> { "CS_GPR", GEN8, RCS0, 0x2600, 32 },
> { "OA_CTX_CONTROL", GEN8, RCS0, 0x2360 },
> { "OACTXID", GEN8, RCS0, 0x2364 },
> - { "PS_INVOCATION_COUNT_2", GEN8, RCS0, 0x2448, 2 },
> + { "PS_INVOCATION_COUNT_2", GEN8, RCS0, 0x2448, 2, .write_mask = ~0x3 },
Same comment as for "PS_INVOCATION_COUNT_0" above.
> { "PS_DEPTH_COUNT_2", GEN8, RCS0, 0x2450, 2 },
> - { "Cache_Mode_0", GEN7, RCS0, 0x7000 },
> - { "Cache_Mode_1", GEN7, RCS0, 0x7004 },
> - { "GT_MODE", GEN8, RCS0, 0x7008 },
> - { "L3_Config", GEN7, RCS0, 0x7034 },
> - { "TD_CTL", GEN8, RCS0, 0xe400 },
> + { "Cache_Mode_0", GEN7, RCS0, 0x7000, .masked = true },
> +
