* [PATCH v2 1/4] drm: add devm release action
2024-04-02 8:58 [PATCH v2 0/4] drm/xe: Support PCIe FLR Aravind Iddamsetty
@ 2024-04-02 8:58 ` Aravind Iddamsetty
2024-04-15 4:42 ` Aravind Iddamsetty
2024-04-16 10:28 ` Aravind Iddamsetty
0 siblings, 2 replies; 17+ messages in thread
From: Aravind Iddamsetty @ 2024-04-02 8:58 UTC (permalink / raw)
To: intel-xe, thomas.hellstrom, rodrigo.vivi, lucas.demarchi; +Cc: dri-devel
In scenarios where drm_dev_put is directly called by driver we want to
release devm_drm_dev_init_release action associated with struct
drm_device.
v2: Directly expose the original function, instead of introducing a
helper (Rodrigo)
Cc: Thomas Hellstr_m <thomas.hellstrom@linux.intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Aravind Iddamsetty <aravind.iddamsetty@linux.intel.com>
---
drivers/gpu/drm/drm_drv.c | 6 ++++++
include/drm/drm_drv.h | 2 ++
2 files changed, 8 insertions(+)
diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index 243cacb3575c..ba60cbb0725f 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -714,6 +714,12 @@ static int devm_drm_dev_init(struct device *parent,
devm_drm_dev_init_release, dev);
}
+void devm_drm_dev_release_action(struct drm_device *dev)
+{
+ devm_release_action(dev->dev, devm_drm_dev_init_release, dev);
+}
+EXPORT_SYMBOL(devm_drm_dev_release_action);
+
void *__devm_drm_dev_alloc(struct device *parent,
const struct drm_driver *driver,
size_t size, size_t offset)
diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h
index 8878260d7529..fa9123684874 100644
--- a/include/drm/drm_drv.h
+++ b/include/drm/drm_drv.h
@@ -444,6 +444,8 @@ struct drm_driver {
const struct file_operations *fops;
};
+void devm_drm_dev_release_action(struct drm_device *dev);
+
void *__devm_drm_dev_alloc(struct device *parent,
const struct drm_driver *driver,
size_t size, size_t offset);
--
2.25.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH v2 1/4] drm: add devm release action
2024-04-02 8:58 ` [PATCH v2 1/4] drm: add devm release action Aravind Iddamsetty
@ 2024-04-15 4:42 ` Aravind Iddamsetty
2024-04-16 10:28 ` Aravind Iddamsetty
1 sibling, 0 replies; 17+ messages in thread
From: Aravind Iddamsetty @ 2024-04-15 4:42 UTC (permalink / raw)
To: intel-xe, thomas.hellstrom, rodrigo.vivi, lucas.demarchi; +Cc: dri-devel
Hi Thomas,
Could you share your thoughts on this please.
Thanks,
Aravind.
On 02/04/24 14:28, Aravind Iddamsetty wrote:
> In scenarios where drm_dev_put is directly called by driver we want to
> release devm_drm_dev_init_release action associated with struct
> drm_device.
>
> v2: Directly expose the original function, instead of introducing a
> helper (Rodrigo)
>
> Cc: Thomas Hellstr_m <thomas.hellstrom@linux.intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: Aravind Iddamsetty <aravind.iddamsetty@linux.intel.com>
> ---
> drivers/gpu/drm/drm_drv.c | 6 ++++++
> include/drm/drm_drv.h | 2 ++
> 2 files changed, 8 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> index 243cacb3575c..ba60cbb0725f 100644
> --- a/drivers/gpu/drm/drm_drv.c
> +++ b/drivers/gpu/drm/drm_drv.c
> @@ -714,6 +714,12 @@ static int devm_drm_dev_init(struct device *parent,
> devm_drm_dev_init_release, dev);
> }
>
> +void devm_drm_dev_release_action(struct drm_device *dev)
> +{
> + devm_release_action(dev->dev, devm_drm_dev_init_release, dev);
> +}
> +EXPORT_SYMBOL(devm_drm_dev_release_action);
> +
> void *__devm_drm_dev_alloc(struct device *parent,
> const struct drm_driver *driver,
> size_t size, size_t offset)
> diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h
> index 8878260d7529..fa9123684874 100644
> --- a/include/drm/drm_drv.h
> +++ b/include/drm/drm_drv.h
> @@ -444,6 +444,8 @@ struct drm_driver {
> const struct file_operations *fops;
> };
>
> +void devm_drm_dev_release_action(struct drm_device *dev);
> +
> void *__devm_drm_dev_alloc(struct device *parent,
> const struct drm_driver *driver,
> size_t size, size_t offset);
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 1/4] drm: add devm release action
2024-04-02 8:58 ` [PATCH v2 1/4] drm: add devm release action Aravind Iddamsetty
2024-04-15 4:42 ` Aravind Iddamsetty
@ 2024-04-16 10:28 ` Aravind Iddamsetty
1 sibling, 0 replies; 17+ messages in thread
From: Aravind Iddamsetty @ 2024-04-16 10:28 UTC (permalink / raw)
To: intel-xe, thomas.hellstrom, rodrigo.vivi, lucas.demarchi
Cc: dri-devel, daniel@ffwll.ch, maarten.lankhorst
Hi Daniel/ Maarten,
Could you please ack this patch.
Thanks,
Aravind.
On 02/04/24 14:28, Aravind Iddamsetty wrote:
> In scenarios where drm_dev_put is directly called by driver we want to
> release devm_drm_dev_init_release action associated with struct
> drm_device.
>
> v2: Directly expose the original function, instead of introducing a
> helper (Rodrigo)
>
> Cc: Thomas Hellstr_m <thomas.hellstrom@linux.intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: Aravind Iddamsetty <aravind.iddamsetty@linux.intel.com>
> ---
> drivers/gpu/drm/drm_drv.c | 6 ++++++
> include/drm/drm_drv.h | 2 ++
> 2 files changed, 8 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> index 243cacb3575c..ba60cbb0725f 100644
> --- a/drivers/gpu/drm/drm_drv.c
> +++ b/drivers/gpu/drm/drm_drv.c
> @@ -714,6 +714,12 @@ static int devm_drm_dev_init(struct device *parent,
> devm_drm_dev_init_release, dev);
> }
>
> +void devm_drm_dev_release_action(struct drm_device *dev)
> +{
> + devm_release_action(dev->dev, devm_drm_dev_init_release, dev);
> +}
> +EXPORT_SYMBOL(devm_drm_dev_release_action);
> +
> void *__devm_drm_dev_alloc(struct device *parent,
> const struct drm_driver *driver,
> size_t size, size_t offset)
> diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h
> index 8878260d7529..fa9123684874 100644
> --- a/include/drm/drm_drv.h
> +++ b/include/drm/drm_drv.h
> @@ -444,6 +444,8 @@ struct drm_driver {
> const struct file_operations *fops;
> };
>
> +void devm_drm_dev_release_action(struct drm_device *dev);
> +
> void *__devm_drm_dev_alloc(struct device *parent,
> const struct drm_driver *driver,
> size_t size, size_t offset);
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v2 0/4] drm/xe: Support PCIe FLR
@ 2024-04-17 8:41 Aravind Iddamsetty
2024-04-17 8:41 ` [PATCH v2 1/4] drm: add devm release action Aravind Iddamsetty
` (3 more replies)
0 siblings, 4 replies; 17+ messages in thread
From: Aravind Iddamsetty @ 2024-04-17 8:41 UTC (permalink / raw)
To: dri-devel, daniel, maarten.lankhorst, airlied, mripard,
tzimmermann, rodrigo.vivi
Cc: intel-xe, Lucas De Marchi
Resending by adding the r-b's and to get ack from DRM maintainers
for the first patch in the series.
PCI subsystem provides callbacks to inform the driver about a request to
do function level reset by user, initiated by writing to sysfs entry
/sys/bus/pci/devices/.../reset. This will allow the driver to handle FLR
without the need to do unbind and rebind as the driver needs to
reinitialize the device afresh post FLR.
v2:
1. Directly expose the devm_drm_dev_release_action instead of introducing
a helper (Rodrigo)
2. separate out gt idle and pci save/restore to a separate patch (Lucas)
3. Fixed the warnings seen around xe_guc_submit_stop, xe_guc_puc_fini
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Lucas De Marchi <lucas.demarchi@intel.com>
dmesg snip showing FLR recovery:
[ 590.486336] xe 0000:4d:00.0: enabling device (0140 -> 0142)
[ 590.506933] xe 0000:4d:00.0: [drm] Using GuC firmware from
xe/pvc_guc_70.20.0.bin version 70.20.0
[ 590.542355] xe 0000:4d:00.0: [drm] Using GuC firmware from
xe/pvc_guc_70.20.0.bin version 70.20.0
[ 590.578532] xe 0000:4d:00.0: [drm] VISIBLE VRAM: 0x0000202000000000,
0x0000002000000000
[ 590.578556] xe 0000:4d:00.0: [drm] VRAM[0, 0]: Actual physical size
0x0000001000000000, usable size exclude stolen 0x0000000fff000000, CPU
accessible size 0x0000000fff000000
[ 590.578560] xe 0000:4d:00.0: [drm] VRAM[0, 0]: DPA range:
[0x0000000000000000-1000000000], io range:
[0x0000202000000000-202fff000000]
[ 590.578585] xe 0000:4d:00.0: [drm] VRAM[1, 1]: Actual physical size
0x0000001000000000, usable size exclude stolen 0x0000000fff000000, CPU
accessible size 0x0000000fff000000
[ 590.578589] xe 0000:4d:00.0: [drm] VRAM[1, 1]: DPA range:
[0x0000001000000000-2000000000], io range:
[0x0000203000000000-203fff000000]
[ 590.578592] xe 0000:4d:00.0: [drm] Total VRAM: 0x0000202000000000,
0x0000002000000000
[ 590.578594] xe 0000:4d:00.0: [drm] Available VRAM:
0x0000202000000000, 0x0000001ffe000000
[ 590.738899] xe 0000:4d:00.0: [drm] GT0: CCS_MODE=0 config:00400000,
num_engines:1, num_slices:4
[ 590.889991] xe 0000:4d:00.0: [drm] GT1: CCS_MODE=0 config:00400000,
num_engines:1, num_slices:4
[ 590.892835] [drm] Initialized xe 1.1.0 20201103 for 0000:4d:00.0 on
minor 1
[ 590.900215] xe 0000:9a:00.0: enabling device (0140 -> 0142)
[ 590.915991] xe 0000:9a:00.0: [drm] Using GuC firmware from
xe/pvc_guc_70.20.0.bin version 70.20.0
[ 590.957450] xe 0000:9a:00.0: [drm] Using GuC firmware from
xe/pvc_guc_70.20.0.bin version 70.20.0
[ 590.989863] xe 0000:9a:00.0: [drm] VISIBLE VRAM: 0x000020e000000000,
0x0000002000000000
[ 590.989888] xe 0000:9a:00.0: [drm] VRAM[0, 0]: Actual physical size
0x0000001000000000, usable size exclude stolen 0x0000000fff000000, CPU
accessible size 0x0000000fff000000
[ 590.989893] xe 0000:9a:00.0: [drm] VRAM[0, 0]: DPA range:
[0x0000000000000000-1000000000], io range:
[0x000020e000000000-20efff000000]
[ 590.989918] xe 0000:9a:00.0: [drm] VRAM[1, 1]: Actual physical size
0x0000001000000000, usable size exclude stolen 0x0000000fff000000, CPU
accessible size 0x0000000fff000000
[ 590.989921] xe 0000:9a:00.0: [drm] VRAM[1, 1]: DPA range:
[0x0000001000000000-2000000000], io range:
[0x000020f000000000-20ffff000000]
[ 590.989924] xe 0000:9a:00.0: [drm] Total VRAM: 0x000020e000000000,
0x0000002000000000
[ 590.989927] xe 0000:9a:00.0: [drm] Available VRAM:
0x000020e000000000, 0x0000001ffe000000
[ 591.142061] xe 0000:9a:00.0: [drm] GT0: CCS_MODE=0 config:00400000,
num_engines:1, num_slices:4
[ 591.293505] xe 0000:9a:00.0: [drm] GT1: CCS_MODE=0 config:00400000,
num_engines:1, num_slices:4
[ 591.295487] [drm] Initialized xe 1.1.0 20201103 for 0000:9a:00.0 on
minor 2
[ 610.685993] Console: switching to colour dummy device 80x25
[ 610.686118] [IGT] xe_exec_basic: executing
[ 610.755398] xe 0000:4d:00.0: [drm] GT0: CCS_MODE=0 config:00400000,
num_engines:1, num_slices:4
[ 610.771783] xe 0000:4d:00.0: [drm] GT1: CCS_MODE=0 config:00400000,
num_engines:1, num_slices:4
[ 610.773542] [IGT] xe_exec_basic: starting subtest once-basic
[ 610.960251] [IGT] xe_exec_basic: finished subtest once-basic, SUCCESS
[ 610.962741] [IGT] xe_exec_basic: exiting, ret=0
[ 610.977203] Console: switching to colour frame buffer device 128x48
[ 611.006675] xe_exec_basic (3237) used greatest stack depth: 11128
bytes left
[ 644.682201] xe 0000:4d:00.0: [drm] GT0: CCS_MODE=0 config:00400000,
num_engines:1, num_slices:4
[ 644.699060] xe 0000:4d:00.0: [drm] GT1: CCS_MODE=0 config:00400000,
num_engines:1, num_slices:4
[ 644.699118] xe 0000:4d:00.0: preparing for PCIe FLR reset
[ 644.699149] xe 0000:4d:00.0: [drm] removing device access to
userspace
[ 644.928577] xe 0000:4d:00.0: PCI device went through FLR, reenabling
the device
[ 656.104233] xe 0000:4d:00.0: [drm] Using GuC firmware from
xe/pvc_guc_70.20.0.bin version 70.20.0
[ 656.149525] xe 0000:4d:00.0: [drm] Using GuC firmware from
xe/pvc_guc_70.20.0.bin version 70.20.0
[ 656.182711] xe 0000:4d:00.0: [drm] VISIBLE VRAM: 0x0000202000000000,
0x0000002000000000
[ 656.182737] xe 0000:4d:00.0: [drm] VRAM[0, 0]: Actual physical size
0x0000001000000000, usable size exclude stolen 0x0000000fff000000, CPU
accessible size 0x0000000fff000000
[ 656.182742] xe 0000:4d:00.0: [drm] VRAM[0, 0]: DPA range:
[0x0000000000000000-1000000000], io range:
[0x0000202000000000-202fff000000]
[ 656.182768] xe 0000:4d:00.0: [drm] VRAM[1, 1]: Actual physical size
0x0000001000000000, usable size exclude stolen 0x0000000fff000000, CPU
accessible size 0x0000000fff000000
[ 656.182772] xe 0000:4d:00.0: [drm] VRAM[1, 1]: DPA range:
[0x0000001000000000-2000000000], io range:
[0x0000203000000000-203fff000000]
[ 656.182775] xe 0000:4d:00.0: [drm] Total VRAM: 0x0000202000000000,
0x0000002000000000
[ 656.182778] xe 0000:4d:00.0: [drm] Available VRAM:
0x0000202000000000, 0x0000001ffe000000
[ 656.348657] xe 0000:4d:00.0: [drm] GT0: CCS_MODE=0 config:00400000,
num_engines:1, num_slices:4
[ 656.507619] xe 0000:4d:00.0: [drm] GT1: CCS_MODE=0 config:00400000,
num_engines:1, num_slices:4
[ 656.510848] [drm] Initialized xe 1.1.0 20201103 for 0000:4d:00.0 on
minor 1
[ 665.754402] Console: switching to colour dummy device 80x25
[ 665.754484] [IGT] xe_exec_basic: executing
[ 665.805853] xe 0000:4d:00.0: [drm] GT0: CCS_MODE=0 config:00400000,
num_engines:1, num_slices:4
[ 665.819825] xe 0000:4d:00.0: [drm] GT1: CCS_MODE=0 config:00400000,
num_engines:1, num_slices:4
[ 665.820359] [IGT] xe_exec_basic: starting subtest once-basic
[ 665.968899] [IGT] xe_exec_basic: finished subtest once-basic, SUCCESS
[ 665.969534] [IGT] xe_exec_basic: exiting, ret=0
[ 665.981027] Console: switching to colour frame buffer device 128x48
Aravind Iddamsetty (4):
drm: add devm release action
drm/xe: Save and restore PCI state
drm/xe: Extract xe_gt_idle() helper
drm/xe/FLR: Support PCIe FLR
drivers/gpu/drm/drm_drv.c | 6 ++
drivers/gpu/drm/xe/Makefile | 1 +
drivers/gpu/drm/xe/xe_device_types.h | 6 ++
drivers/gpu/drm/xe/xe_gt.c | 31 +++++++---
drivers/gpu/drm/xe/xe_gt.h | 1 +
drivers/gpu/drm/xe/xe_guc_pc.c | 4 ++
drivers/gpu/drm/xe/xe_pci.c | 57 +++++++++++++++--
drivers/gpu/drm/xe/xe_pci.h | 6 +-
drivers/gpu/drm/xe/xe_pci_err.c | 93 ++++++++++++++++++++++++++++
drivers/gpu/drm/xe/xe_pci_err.h | 13 ++++
include/drm/drm_drv.h | 2 +
11 files changed, 205 insertions(+), 15 deletions(-)
create mode 100644 drivers/gpu/drm/xe/xe_pci_err.c
create mode 100644 drivers/gpu/drm/xe/xe_pci_err.h
--
2.25.1
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v2 1/4] drm: add devm release action
2024-04-17 8:41 [PATCH v2 0/4] drm/xe: Support PCIe FLR Aravind Iddamsetty
@ 2024-04-17 8:41 ` Aravind Iddamsetty
2024-04-17 13:55 ` Rodrigo Vivi
2024-04-17 8:41 ` [PATCH 2/4] drm/xe: Save and restore PCI state Aravind Iddamsetty
` (2 subsequent siblings)
3 siblings, 1 reply; 17+ messages in thread
From: Aravind Iddamsetty @ 2024-04-17 8:41 UTC (permalink / raw)
To: dri-devel, daniel, maarten.lankhorst, airlied, mripard,
tzimmermann, rodrigo.vivi
Cc: intel-xe, Thomas Hellstr_m
In scenarios where drm_dev_put is directly called by driver we want to
release devm_drm_dev_init_release action associated with struct
drm_device.
v2: Directly expose the original function, instead of introducing a
helper (Rodrigo)
Cc: Thomas Hellstr_m <thomas.hellstrom@linux.intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Aravind Iddamsetty <aravind.iddamsetty@linux.intel.com>
---
drivers/gpu/drm/drm_drv.c | 6 ++++++
include/drm/drm_drv.h | 2 ++
2 files changed, 8 insertions(+)
diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index 243cacb3575c..ba60cbb0725f 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -714,6 +714,12 @@ static int devm_drm_dev_init(struct device *parent,
devm_drm_dev_init_release, dev);
}
+void devm_drm_dev_release_action(struct drm_device *dev)
+{
+ devm_release_action(dev->dev, devm_drm_dev_init_release, dev);
+}
+EXPORT_SYMBOL(devm_drm_dev_release_action);
+
void *__devm_drm_dev_alloc(struct device *parent,
const struct drm_driver *driver,
size_t size, size_t offset)
diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h
index 8878260d7529..fa9123684874 100644
--- a/include/drm/drm_drv.h
+++ b/include/drm/drm_drv.h
@@ -444,6 +444,8 @@ struct drm_driver {
const struct file_operations *fops;
};
+void devm_drm_dev_release_action(struct drm_device *dev);
+
void *__devm_drm_dev_alloc(struct device *parent,
const struct drm_driver *driver,
size_t size, size_t offset);
--
2.25.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 2/4] drm/xe: Save and restore PCI state
2024-04-17 8:41 [PATCH v2 0/4] drm/xe: Support PCIe FLR Aravind Iddamsetty
2024-04-17 8:41 ` [PATCH v2 1/4] drm: add devm release action Aravind Iddamsetty
@ 2024-04-17 8:41 ` Aravind Iddamsetty
2024-04-17 8:41 ` [PATCH 3/4] drm/xe: Extract xe_gt_idle() helper Aravind Iddamsetty
2024-04-17 8:41 ` [PATCH v2 4/4] drm/xe/FLR: Support PCIe FLR Aravind Iddamsetty
3 siblings, 0 replies; 17+ messages in thread
From: Aravind Iddamsetty @ 2024-04-17 8:41 UTC (permalink / raw)
To: dri-devel, daniel, maarten.lankhorst, airlied, mripard,
tzimmermann, rodrigo.vivi
Cc: intel-xe, Lucas De Marchi
Save and restore PCI states where ever needed.
Cc: Lucas De Marchi <lucas.demarchi@intel.com>
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Aravind Iddamsetty <aravind.iddamsetty@linux.intel.com>
---
drivers/gpu/drm/xe/xe_device_types.h | 3 ++
drivers/gpu/drm/xe/xe_pci.c | 48 ++++++++++++++++++++++++++--
drivers/gpu/drm/xe/xe_pci.h | 4 ++-
3 files changed, 51 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/xe/xe_device_types.h b/drivers/gpu/drm/xe/xe_device_types.h
index 8244b177a6a3..0a66555229e9 100644
--- a/drivers/gpu/drm/xe/xe_device_types.h
+++ b/drivers/gpu/drm/xe/xe_device_types.h
@@ -462,6 +462,9 @@ struct xe_device {
/** @needs_flr_on_fini: requests function-reset on fini */
bool needs_flr_on_fini;
+ /** @pci_state: PCI state of device */
+ struct pci_saved_state *pci_state;
+
/* private: */
#if IS_ENABLED(CONFIG_DRM_XE_DISPLAY)
diff --git a/drivers/gpu/drm/xe/xe_pci.c b/drivers/gpu/drm/xe/xe_pci.c
index fa2cc80a08a3..f9957cd16356 100644
--- a/drivers/gpu/drm/xe/xe_pci.c
+++ b/drivers/gpu/drm/xe/xe_pci.c
@@ -391,6 +391,41 @@ MODULE_DEVICE_TABLE(pci, pciidlist);
#undef INTEL_VGA_DEVICE
+static bool xe_save_pci_state(struct pci_dev *pdev)
+{
+ struct xe_device *xe = pci_get_drvdata(pdev);
+
+ if (pci_save_state(pdev))
+ return false;
+
+ kfree(xe->pci_state);
+
+ xe->pci_state = pci_store_saved_state(pdev);
+ if (!xe->pci_state) {
+ drm_err(&xe->drm, "Failed to store PCI saved state\n");
+ return false;
+ }
+
+ return true;
+}
+
+void xe_load_pci_state(struct pci_dev *pdev)
+{
+ struct xe_device *xe = pci_get_drvdata(pdev);
+ int ret;
+
+ if (!xe->pci_state)
+ return;
+
+ ret = pci_load_saved_state(pdev, xe->pci_state);
+ if (ret) {
+ drm_warn(&xe->drm, "Failed to load PCI state err:%d\n", ret);
+ return;
+ }
+
+ pci_restore_state(pdev);
+}
+
/* is device_id present in comma separated list of ids */
static bool device_id_in_list(u16 device_id, const char *devices, bool negative)
{
@@ -696,6 +731,8 @@ static void xe_pci_remove(struct pci_dev *pdev)
xe_device_remove(xe);
xe_pm_runtime_fini(xe);
+
+ kfree(xe->pci_state);
pci_set_drvdata(pdev, NULL);
}
@@ -792,6 +829,9 @@ static int xe_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
drm_dbg(&xe->drm, "d3cold: capable=%s\n",
str_yes_no(xe->d3cold.capable));
+ if (xe_save_pci_state(pdev))
+ pci_restore_state(pdev);
+
return 0;
}
@@ -839,7 +879,7 @@ static int xe_pci_suspend(struct device *dev)
*/
d3cold_toggle(pdev, D3COLD_ENABLE);
- pci_save_state(pdev);
+ xe_save_pci_state(pdev);
pci_disable_device(pdev);
return 0;
@@ -863,6 +903,8 @@ static int xe_pci_resume(struct device *dev)
pci_set_master(pdev);
+ xe_load_pci_state(pdev);
+
err = xe_pm_resume(pdev_to_xe_device(pdev));
if (err)
return err;
@@ -880,7 +922,7 @@ static int xe_pci_runtime_suspend(struct device *dev)
if (err)
return err;
- pci_save_state(pdev);
+ xe_save_pci_state(pdev);
if (xe->d3cold.allowed) {
d3cold_toggle(pdev, D3COLD_ENABLE);
@@ -905,7 +947,7 @@ static int xe_pci_runtime_resume(struct device *dev)
if (err)
return err;
- pci_restore_state(pdev);
+ xe_load_pci_state(pdev);
if (xe->d3cold.allowed) {
err = pci_enable_device(pdev);
diff --git a/drivers/gpu/drm/xe/xe_pci.h b/drivers/gpu/drm/xe/xe_pci.h
index 611c1209b14c..73b90a430d1f 100644
--- a/drivers/gpu/drm/xe/xe_pci.h
+++ b/drivers/gpu/drm/xe/xe_pci.h
@@ -6,7 +6,9 @@
#ifndef _XE_PCI_H_
#define _XE_PCI_H_
+struct pci_dev;
+
int xe_register_pci_driver(void);
void xe_unregister_pci_driver(void);
-
+void xe_load_pci_state(struct pci_dev *pdev);
#endif
--
2.25.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 3/4] drm/xe: Extract xe_gt_idle() helper
2024-04-17 8:41 [PATCH v2 0/4] drm/xe: Support PCIe FLR Aravind Iddamsetty
2024-04-17 8:41 ` [PATCH v2 1/4] drm: add devm release action Aravind Iddamsetty
2024-04-17 8:41 ` [PATCH 2/4] drm/xe: Save and restore PCI state Aravind Iddamsetty
@ 2024-04-17 8:41 ` Aravind Iddamsetty
2024-04-17 13:50 ` Michal Wajdeczko
2024-04-17 8:41 ` [PATCH v2 4/4] drm/xe/FLR: Support PCIe FLR Aravind Iddamsetty
3 siblings, 1 reply; 17+ messages in thread
From: Aravind Iddamsetty @ 2024-04-17 8:41 UTC (permalink / raw)
To: dri-devel, daniel, maarten.lankhorst, airlied, mripard,
tzimmermann, rodrigo.vivi
Cc: intel-xe, Lucas De Marchi
This would be used in other places outside of gt_reset path.
Cc: Lucas De Marchi <lucas.demarchi@intel.com>
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Aravind Iddamsetty <aravind.iddamsetty@linux.intel.com>
---
drivers/gpu/drm/xe/xe_gt.c | 31 +++++++++++++++++++++----------
drivers/gpu/drm/xe/xe_gt.h | 1 +
2 files changed, 22 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/xe/xe_gt.c b/drivers/gpu/drm/xe/xe_gt.c
index cfa5da900461..59f497d575ad 100644
--- a/drivers/gpu/drm/xe/xe_gt.c
+++ b/drivers/gpu/drm/xe/xe_gt.c
@@ -629,6 +629,26 @@ static int do_gt_restart(struct xe_gt *gt)
return 0;
}
+/* Idle the GT */
+int xe_gt_idle(struct xe_gt *gt)
+{
+ int err;
+
+ xe_gt_sanitize(gt);
+
+ xe_uc_gucrc_disable(>->uc);
+ xe_uc_stop_prepare(>->uc);
+ xe_gt_pagefault_reset(gt);
+
+ err = xe_uc_stop(>->uc);
+ if (err)
+ return err;
+
+ xe_gt_tlb_invalidation_reset(gt);
+
+ return err;
+}
+
static int gt_reset(struct xe_gt *gt)
{
int err;
@@ -645,21 +665,12 @@ static int gt_reset(struct xe_gt *gt)
}
xe_pm_runtime_get(gt_to_xe(gt));
- xe_gt_sanitize(gt);
err = xe_force_wake_get(gt_to_fw(gt), XE_FORCEWAKE_ALL);
if (err)
goto err_msg;
- xe_uc_gucrc_disable(>->uc);
- xe_uc_stop_prepare(>->uc);
- xe_gt_pagefault_reset(gt);
-
- err = xe_uc_stop(>->uc);
- if (err)
- goto err_out;
-
- xe_gt_tlb_invalidation_reset(gt);
+ xe_gt_idle(gt);
err = do_gt_reset(gt);
if (err)
diff --git a/drivers/gpu/drm/xe/xe_gt.h b/drivers/gpu/drm/xe/xe_gt.h
index ed6ea8057e35..d62af1725ff6 100644
--- a/drivers/gpu/drm/xe/xe_gt.h
+++ b/drivers/gpu/drm/xe/xe_gt.h
@@ -43,6 +43,7 @@ int xe_gt_resume(struct xe_gt *gt);
void xe_gt_reset_async(struct xe_gt *gt);
void xe_gt_sanitize(struct xe_gt *gt);
void xe_gt_remove(struct xe_gt *gt);
+int xe_gt_idle(struct xe_gt *gt);
/**
* xe_gt_any_hw_engine_by_reset_domain - scan the list of engines and return the
--
2.25.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v2 4/4] drm/xe/FLR: Support PCIe FLR
2024-04-17 8:41 [PATCH v2 0/4] drm/xe: Support PCIe FLR Aravind Iddamsetty
` (2 preceding siblings ...)
2024-04-17 8:41 ` [PATCH 3/4] drm/xe: Extract xe_gt_idle() helper Aravind Iddamsetty
@ 2024-04-17 8:41 ` Aravind Iddamsetty
2024-04-17 13:59 ` Michal Wajdeczko
3 siblings, 1 reply; 17+ messages in thread
From: Aravind Iddamsetty @ 2024-04-17 8:41 UTC (permalink / raw)
To: dri-devel, daniel, maarten.lankhorst, airlied, mripard,
tzimmermann, rodrigo.vivi
Cc: intel-xe, Lucas De Marchi
PCI subsystem provides callbacks to inform the driver about a request to
do function level reset by user, initiated by writing to sysfs entry
/sys/bus/pci/devices/.../reset. This will allow the driver to handle FLR
without the need to do unbind and rebind as the driver needs to
reinitialize the device afresh post FLR.
v2:
1. separate out gt idle and pci save/restore to a separate patch (Lucas)
2. Fixed the warnings seen around xe_guc_submit_stop, xe_guc_puc_fini
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Lucas De Marchi <lucas.demarchi@intel.com>
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Aravind Iddamsetty <aravind.iddamsetty@linux.intel.com>
---
drivers/gpu/drm/xe/Makefile | 1 +
drivers/gpu/drm/xe/xe_device_types.h | 3 +
drivers/gpu/drm/xe/xe_guc_pc.c | 4 ++
drivers/gpu/drm/xe/xe_pci.c | 9 ++-
drivers/gpu/drm/xe/xe_pci.h | 2 +
drivers/gpu/drm/xe/xe_pci_err.c | 93 ++++++++++++++++++++++++++++
drivers/gpu/drm/xe/xe_pci_err.h | 13 ++++
7 files changed, 124 insertions(+), 1 deletion(-)
create mode 100644 drivers/gpu/drm/xe/xe_pci_err.c
create mode 100644 drivers/gpu/drm/xe/xe_pci_err.h
diff --git a/drivers/gpu/drm/xe/Makefile b/drivers/gpu/drm/xe/Makefile
index 522bffa5c4e1..9686ea181ad1 100644
--- a/drivers/gpu/drm/xe/Makefile
+++ b/drivers/gpu/drm/xe/Makefile
@@ -117,6 +117,7 @@ xe-y += xe_bb.o \
xe_module.o \
xe_pat.o \
xe_pci.o \
+ xe_pci_err.o \
xe_pcode.o \
xe_pm.o \
xe_preempt_fence.o \
diff --git a/drivers/gpu/drm/xe/xe_device_types.h b/drivers/gpu/drm/xe/xe_device_types.h
index 0a66555229e9..8c749b378a92 100644
--- a/drivers/gpu/drm/xe/xe_device_types.h
+++ b/drivers/gpu/drm/xe/xe_device_types.h
@@ -465,6 +465,9 @@ struct xe_device {
/** @pci_state: PCI state of device */
struct pci_saved_state *pci_state;
+ /** @pci_device_is_reset: device went through PCIe FLR */
+ bool pci_device_is_reset;
+
/* private: */
#if IS_ENABLED(CONFIG_DRM_XE_DISPLAY)
diff --git a/drivers/gpu/drm/xe/xe_guc_pc.c b/drivers/gpu/drm/xe/xe_guc_pc.c
index 521ae24f2314..6108a4dfa05f 100644
--- a/drivers/gpu/drm/xe/xe_guc_pc.c
+++ b/drivers/gpu/drm/xe/xe_guc_pc.c
@@ -902,6 +902,10 @@ static void xe_guc_pc_fini(struct drm_device *drm, void *arg)
return;
}
+ /* We already have done this before going through a reset, so skip here */
+ if (xe->pci_device_is_reset)
+ return;
+
XE_WARN_ON(xe_force_wake_get(gt_to_fw(pc_to_gt(pc)), XE_FORCEWAKE_ALL));
XE_WARN_ON(xe_guc_pc_gucrc_disable(pc));
XE_WARN_ON(xe_guc_pc_stop(pc));
diff --git a/drivers/gpu/drm/xe/xe_pci.c b/drivers/gpu/drm/xe/xe_pci.c
index f9957cd16356..d986ae7fc7f7 100644
--- a/drivers/gpu/drm/xe/xe_pci.c
+++ b/drivers/gpu/drm/xe/xe_pci.c
@@ -23,6 +23,7 @@
#include "xe_macros.h"
#include "xe_mmio.h"
#include "xe_module.h"
+#include "xe_pci_err.h"
#include "xe_pci_types.h"
#include "xe_pm.h"
#include "xe_sriov.h"
@@ -736,7 +737,7 @@ static void xe_pci_remove(struct pci_dev *pdev)
pci_set_drvdata(pdev, NULL);
}
-static int xe_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
+int xe_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
{
const struct xe_device_desc *desc = (const void *)ent->driver_data;
const struct xe_subplatform_desc *subplatform_desc;
@@ -976,6 +977,11 @@ static const struct dev_pm_ops xe_pm_ops = {
};
#endif
+const struct pci_error_handlers xe_pci_err_handlers = {
+ .reset_prepare = xe_pci_reset_prepare,
+ .reset_done = xe_pci_reset_done,
+};
+
static struct pci_driver xe_pci_driver = {
.name = DRIVER_NAME,
.id_table = pciidlist,
@@ -985,6 +991,7 @@ static struct pci_driver xe_pci_driver = {
#ifdef CONFIG_PM_SLEEP
.driver.pm = &xe_pm_ops,
#endif
+ .err_handler = &xe_pci_err_handlers,
};
int xe_register_pci_driver(void)
diff --git a/drivers/gpu/drm/xe/xe_pci.h b/drivers/gpu/drm/xe/xe_pci.h
index 73b90a430d1f..9faf5380a09e 100644
--- a/drivers/gpu/drm/xe/xe_pci.h
+++ b/drivers/gpu/drm/xe/xe_pci.h
@@ -7,8 +7,10 @@
#define _XE_PCI_H_
struct pci_dev;
+struct pci_device_id;
int xe_register_pci_driver(void);
void xe_unregister_pci_driver(void);
void xe_load_pci_state(struct pci_dev *pdev);
+int xe_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent);
#endif
diff --git a/drivers/gpu/drm/xe/xe_pci_err.c b/drivers/gpu/drm/xe/xe_pci_err.c
new file mode 100644
index 000000000000..81c440e08fbc
--- /dev/null
+++ b/drivers/gpu/drm/xe/xe_pci_err.c
@@ -0,0 +1,93 @@
+// SPDX-License-Identifier: MIT
+/*
+ * Copyright © 2024 Intel Corporation
+ */
+
+#include <linux/pci.h>
+#include <drm/drm_drv.h>
+
+#include "xe_device.h"
+#include "xe_gt.h"
+#include "xe_gt_printk.h"
+#include "xe_pci.h"
+#include "xe_pci_err.h"
+#include "xe_pm.h"
+#include "xe_uc.h"
+
+/**
+ * xe_pci_reset_prepare - Called when user issued a PCIe reset
+ * via /sys/bus/pci/devices/.../reset.
+ * @pdev: PCI device struct
+ */
+void xe_pci_reset_prepare(struct pci_dev *pdev)
+{
+ struct xe_device *xe = pci_get_drvdata(pdev);
+ struct xe_gt *gt;
+ int id, err;
+
+ pci_warn(pdev, "preparing for PCIe reset\n");
+
+ drm_warn(&xe->drm, "removing device access to userspace\n");
+ drm_dev_unplug(&xe->drm);
+
+ xe_pm_runtime_get(xe);
+ /* idle the GTs */
+ for_each_gt(gt, xe, id) {
+ err = xe_force_wake_get(gt_to_fw(gt), XE_FORCEWAKE_ALL);
+ if (err)
+ goto reset;
+ xe_uc_reset_prepare(>->uc);
+ err = xe_gt_idle(gt);
+ if (err) {
+ xe_gt_err(gt, "failed to idle gt (%pe)\n", ERR_PTR(err));
+ goto reset;
+ }
+
+ err = xe_force_wake_put(gt_to_fw(gt), XE_FORCEWAKE_ALL);
+ XE_WARN_ON(err);
+ }
+ xe_pm_runtime_put(xe);
+
+reset:
+ pci_disable_device(pdev);
+}
+
+/**
+ * xe_pci_reset_done - Called when PCIe reset is done.
+ * @pdev: PCI device struct
+ */
+void xe_pci_reset_done(struct pci_dev *pdev)
+{
+ const struct pci_device_id *ent = pci_match_id(pdev->driver->id_table, pdev);
+ struct xe_device *xe = pci_get_drvdata(pdev);
+
+ dev_info(&pdev->dev,
+ "device went through PCIe reset, reenabling the device\n");
+
+ if (pci_enable_device(pdev)) {
+ dev_err(&pdev->dev,
+ "Cannot re-enable PCI device after reset\n");
+ return;
+ }
+ pci_set_master(pdev);
+ xe_load_pci_state(pdev);
+
+ xe->pci_device_is_reset = true;
+ /*
+ * We want to completely clean the driver and even destroy
+ * the xe private data and reinitialize afresh similar to
+ * probe
+ */
+ pdev->driver->remove(pdev);
+ if (pci_dev_msi_enabled(pdev))
+ pci_free_irq_vectors(pdev);
+
+ devm_drm_dev_release_action(&xe->drm);
+ pci_disable_device(pdev);
+
+ /*
+ * if this fails the driver might be in a stale state, only option is
+ * to unbind and rebind
+ */
+ xe_pci_probe(pdev, ent);
+}
diff --git a/drivers/gpu/drm/xe/xe_pci_err.h b/drivers/gpu/drm/xe/xe_pci_err.h
new file mode 100644
index 000000000000..95a4c8ce9cf1
--- /dev/null
+++ b/drivers/gpu/drm/xe/xe_pci_err.h
@@ -0,0 +1,13 @@
+/* SPDX-License-Identifier: MIT */
+/*
+ * Copyright © 2024 Intel Corporation
+ */
+
+#ifndef _XE_PCI_ERR_H_
+#define _XE_PCI_ERR_H_
+
+struct pci_dev;
+
+void xe_pci_reset_prepare(struct pci_dev *pdev);
+void xe_pci_reset_done(struct pci_dev *pdev);
+#endif
--
2.25.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH 3/4] drm/xe: Extract xe_gt_idle() helper
2024-04-17 8:41 ` [PATCH 3/4] drm/xe: Extract xe_gt_idle() helper Aravind Iddamsetty
@ 2024-04-17 13:50 ` Michal Wajdeczko
2024-04-18 10:13 ` Aravind Iddamsetty
0 siblings, 1 reply; 17+ messages in thread
From: Michal Wajdeczko @ 2024-04-17 13:50 UTC (permalink / raw)
To: Aravind Iddamsetty, dri-devel, daniel, maarten.lankhorst, airlied,
mripard, tzimmermann, rodrigo.vivi
Cc: intel-xe, Lucas De Marchi
On 17.04.2024 10:41, Aravind Iddamsetty wrote:
> This would be used in other places outside of gt_reset path.
>
> Cc: Lucas De Marchi <lucas.demarchi@intel.com>
>
> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: Aravind Iddamsetty <aravind.iddamsetty@linux.intel.com>
> ---
> drivers/gpu/drm/xe/xe_gt.c | 31 +++++++++++++++++++++----------
> drivers/gpu/drm/xe/xe_gt.h | 1 +
> 2 files changed, 22 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/xe/xe_gt.c b/drivers/gpu/drm/xe/xe_gt.c
> index cfa5da900461..59f497d575ad 100644
> --- a/drivers/gpu/drm/xe/xe_gt.c
> +++ b/drivers/gpu/drm/xe/xe_gt.c
> @@ -629,6 +629,26 @@ static int do_gt_restart(struct xe_gt *gt)
> return 0;
> }
>
> +/* Idle the GT */
as this is a public function, can we have a proper kernel-doc here?
and maybe some hint what should be done if this function fails?
> +int xe_gt_idle(struct xe_gt *gt)
> +{
> + int err;
> +
> + xe_gt_sanitize(gt);
> +
> + xe_uc_gucrc_disable(>->uc);
> + xe_uc_stop_prepare(>->uc);
> + xe_gt_pagefault_reset(gt);
> +
> + err = xe_uc_stop(>->uc);
> + if (err)
> + return err;
> +
> + xe_gt_tlb_invalidation_reset(gt);
> +
> + return err;
> +}
> +
> static int gt_reset(struct xe_gt *gt)
> {
> int err;
> @@ -645,21 +665,12 @@ static int gt_reset(struct xe_gt *gt)
> }
>
> xe_pm_runtime_get(gt_to_xe(gt));
> - xe_gt_sanitize(gt);
>
> err = xe_force_wake_get(gt_to_fw(gt), XE_FORCEWAKE_ALL);
> if (err)
> goto err_msg;
>
> - xe_uc_gucrc_disable(>->uc);
> - xe_uc_stop_prepare(>->uc);
> - xe_gt_pagefault_reset(gt);
> -
> - err = xe_uc_stop(>->uc);
> - if (err)
> - goto err_out;
> -
> - xe_gt_tlb_invalidation_reset(gt);
> + xe_gt_idle(gt);
>
> err = do_gt_reset(gt);
> if (err)
> diff --git a/drivers/gpu/drm/xe/xe_gt.h b/drivers/gpu/drm/xe/xe_gt.h
> index ed6ea8057e35..d62af1725ff6 100644
> --- a/drivers/gpu/drm/xe/xe_gt.h
> +++ b/drivers/gpu/drm/xe/xe_gt.h
> @@ -43,6 +43,7 @@ int xe_gt_resume(struct xe_gt *gt);
> void xe_gt_reset_async(struct xe_gt *gt);
> void xe_gt_sanitize(struct xe_gt *gt);
> void xe_gt_remove(struct xe_gt *gt);
> +int xe_gt_idle(struct xe_gt *gt);
>
> /**
> * xe_gt_any_hw_engine_by_reset_domain - scan the list of engines and return the
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 1/4] drm: add devm release action
2024-04-17 8:41 ` [PATCH v2 1/4] drm: add devm release action Aravind Iddamsetty
@ 2024-04-17 13:55 ` Rodrigo Vivi
2024-04-17 14:03 ` Maxime Ripard
0 siblings, 1 reply; 17+ messages in thread
From: Rodrigo Vivi @ 2024-04-17 13:55 UTC (permalink / raw)
To: Aravind Iddamsetty, Daniel Vetter, Dave Airlie
Cc: dri-devel, daniel, maarten.lankhorst, airlied, mripard,
tzimmermann, intel-xe, Thomas Hellstr_m
On Wed, Apr 17, 2024 at 02:11:42PM +0530, Aravind Iddamsetty wrote:
> In scenarios where drm_dev_put is directly called by driver we want to
> release devm_drm_dev_init_release action associated with struct
> drm_device.
>
> v2: Directly expose the original function, instead of introducing a
> helper (Rodrigo)
>
> Cc: Thomas Hellstr_m <thomas.hellstrom@linux.intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
>
> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Sima, Dave, or drm-misc, ack to get this through drm-xe-next?
> Signed-off-by: Aravind Iddamsetty <aravind.iddamsetty@linux.intel.com>
> ---
> drivers/gpu/drm/drm_drv.c | 6 ++++++
> include/drm/drm_drv.h | 2 ++
> 2 files changed, 8 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> index 243cacb3575c..ba60cbb0725f 100644
> --- a/drivers/gpu/drm/drm_drv.c
> +++ b/drivers/gpu/drm/drm_drv.c
> @@ -714,6 +714,12 @@ static int devm_drm_dev_init(struct device *parent,
> devm_drm_dev_init_release, dev);
> }
>
> +void devm_drm_dev_release_action(struct drm_device *dev)
> +{
> + devm_release_action(dev->dev, devm_drm_dev_init_release, dev);
> +}
> +EXPORT_SYMBOL(devm_drm_dev_release_action);
> +
> void *__devm_drm_dev_alloc(struct device *parent,
> const struct drm_driver *driver,
> size_t size, size_t offset)
> diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h
> index 8878260d7529..fa9123684874 100644
> --- a/include/drm/drm_drv.h
> +++ b/include/drm/drm_drv.h
> @@ -444,6 +444,8 @@ struct drm_driver {
> const struct file_operations *fops;
> };
>
> +void devm_drm_dev_release_action(struct drm_device *dev);
> +
> void *__devm_drm_dev_alloc(struct device *parent,
> const struct drm_driver *driver,
> size_t size, size_t offset);
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 4/4] drm/xe/FLR: Support PCIe FLR
2024-04-17 8:41 ` [PATCH v2 4/4] drm/xe/FLR: Support PCIe FLR Aravind Iddamsetty
@ 2024-04-17 13:59 ` Michal Wajdeczko
2024-04-18 10:11 ` Aravind Iddamsetty
0 siblings, 1 reply; 17+ messages in thread
From: Michal Wajdeczko @ 2024-04-17 13:59 UTC (permalink / raw)
To: Aravind Iddamsetty, dri-devel, daniel, maarten.lankhorst, airlied,
mripard, tzimmermann, rodrigo.vivi
Cc: intel-xe, Lucas De Marchi
On 17.04.2024 10:41, Aravind Iddamsetty wrote:
> PCI subsystem provides callbacks to inform the driver about a request to
> do function level reset by user, initiated by writing to sysfs entry
> /sys/bus/pci/devices/.../reset. This will allow the driver to handle FLR
> without the need to do unbind and rebind as the driver needs to
> reinitialize the device afresh post FLR.
>
> v2:
> 1. separate out gt idle and pci save/restore to a separate patch (Lucas)
> 2. Fixed the warnings seen around xe_guc_submit_stop, xe_guc_puc_fini
>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Lucas De Marchi <lucas.demarchi@intel.com>
>
> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: Aravind Iddamsetty <aravind.iddamsetty@linux.intel.com>
> ---
> drivers/gpu/drm/xe/Makefile | 1 +
> drivers/gpu/drm/xe/xe_device_types.h | 3 +
> drivers/gpu/drm/xe/xe_guc_pc.c | 4 ++
> drivers/gpu/drm/xe/xe_pci.c | 9 ++-
> drivers/gpu/drm/xe/xe_pci.h | 2 +
> drivers/gpu/drm/xe/xe_pci_err.c | 93 ++++++++++++++++++++++++++++
> drivers/gpu/drm/xe/xe_pci_err.h | 13 ++++
> 7 files changed, 124 insertions(+), 1 deletion(-)
> create mode 100644 drivers/gpu/drm/xe/xe_pci_err.c
> create mode 100644 drivers/gpu/drm/xe/xe_pci_err.h
>
> diff --git a/drivers/gpu/drm/xe/Makefile b/drivers/gpu/drm/xe/Makefile
> index 522bffa5c4e1..9686ea181ad1 100644
> --- a/drivers/gpu/drm/xe/Makefile
> +++ b/drivers/gpu/drm/xe/Makefile
> @@ -117,6 +117,7 @@ xe-y += xe_bb.o \
> xe_module.o \
> xe_pat.o \
> xe_pci.o \
> + xe_pci_err.o \
> xe_pcode.o \
> xe_pm.o \
> xe_preempt_fence.o \
> diff --git a/drivers/gpu/drm/xe/xe_device_types.h b/drivers/gpu/drm/xe/xe_device_types.h
> index 0a66555229e9..8c749b378a92 100644
> --- a/drivers/gpu/drm/xe/xe_device_types.h
> +++ b/drivers/gpu/drm/xe/xe_device_types.h
> @@ -465,6 +465,9 @@ struct xe_device {
> /** @pci_state: PCI state of device */
> struct pci_saved_state *pci_state;
>
> + /** @pci_device_is_reset: device went through PCIe FLR */
> + bool pci_device_is_reset;
> +
> /* private: */
>
> #if IS_ENABLED(CONFIG_DRM_XE_DISPLAY)
> diff --git a/drivers/gpu/drm/xe/xe_guc_pc.c b/drivers/gpu/drm/xe/xe_guc_pc.c
> index 521ae24f2314..6108a4dfa05f 100644
> --- a/drivers/gpu/drm/xe/xe_guc_pc.c
> +++ b/drivers/gpu/drm/xe/xe_guc_pc.c
> @@ -902,6 +902,10 @@ static void xe_guc_pc_fini(struct drm_device *drm, void *arg)
> return;
> }
>
> + /* We already have done this before going through a reset, so skip here */
> + if (xe->pci_device_is_reset)
> + return;
> +
> XE_WARN_ON(xe_force_wake_get(gt_to_fw(pc_to_gt(pc)), XE_FORCEWAKE_ALL));
> XE_WARN_ON(xe_guc_pc_gucrc_disable(pc));
> XE_WARN_ON(xe_guc_pc_stop(pc));
> diff --git a/drivers/gpu/drm/xe/xe_pci.c b/drivers/gpu/drm/xe/xe_pci.c
> index f9957cd16356..d986ae7fc7f7 100644
> --- a/drivers/gpu/drm/xe/xe_pci.c
> +++ b/drivers/gpu/drm/xe/xe_pci.c
> @@ -23,6 +23,7 @@
> #include "xe_macros.h"
> #include "xe_mmio.h"
> #include "xe_module.h"
> +#include "xe_pci_err.h"
> #include "xe_pci_types.h"
> #include "xe_pm.h"
> #include "xe_sriov.h"
> @@ -736,7 +737,7 @@ static void xe_pci_remove(struct pci_dev *pdev)
> pci_set_drvdata(pdev, NULL);
> }
>
> -static int xe_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> +int xe_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> {
> const struct xe_device_desc *desc = (const void *)ent->driver_data;
> const struct xe_subplatform_desc *subplatform_desc;
> @@ -976,6 +977,11 @@ static const struct dev_pm_ops xe_pm_ops = {
> };
> #endif
>
> +const struct pci_error_handlers xe_pci_err_handlers = {
static ?
> + .reset_prepare = xe_pci_reset_prepare,
> + .reset_done = xe_pci_reset_done,
> +};
> +
> static struct pci_driver xe_pci_driver = {
> .name = DRIVER_NAME,
> .id_table = pciidlist,
> @@ -985,6 +991,7 @@ static struct pci_driver xe_pci_driver = {
> #ifdef CONFIG_PM_SLEEP
> .driver.pm = &xe_pm_ops,
> #endif
> + .err_handler = &xe_pci_err_handlers,
> };
>
> int xe_register_pci_driver(void)
> diff --git a/drivers/gpu/drm/xe/xe_pci.h b/drivers/gpu/drm/xe/xe_pci.h
> index 73b90a430d1f..9faf5380a09e 100644
> --- a/drivers/gpu/drm/xe/xe_pci.h
> +++ b/drivers/gpu/drm/xe/xe_pci.h
> @@ -7,8 +7,10 @@
> #define _XE_PCI_H_
>
> struct pci_dev;
> +struct pci_device_id;
>
> int xe_register_pci_driver(void);
> void xe_unregister_pci_driver(void);
> void xe_load_pci_state(struct pci_dev *pdev);
> +int xe_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent);
> #endif
> diff --git a/drivers/gpu/drm/xe/xe_pci_err.c b/drivers/gpu/drm/xe/xe_pci_err.c
> new file mode 100644
> index 000000000000..81c440e08fbc
> --- /dev/null
> +++ b/drivers/gpu/drm/xe/xe_pci_err.c
> @@ -0,0 +1,93 @@
> +// SPDX-License-Identifier: MIT
> +/*
> + * Copyright © 2024 Intel Corporation
> + */
> +
> +#include <linux/pci.h>
> +#include <drm/drm_drv.h>
> +
> +#include "xe_device.h"
> +#include "xe_gt.h"
> +#include "xe_gt_printk.h"
> +#include "xe_pci.h"
> +#include "xe_pci_err.h"
> +#include "xe_pm.h"
> +#include "xe_uc.h"
> +
> +/**
> + * xe_pci_reset_prepare - Called when user issued a PCIe reset
> + * via /sys/bus/pci/devices/.../reset.
> + * @pdev: PCI device struct
> + */
> +void xe_pci_reset_prepare(struct pci_dev *pdev)
> +{
> + struct xe_device *xe = pci_get_drvdata(pdev);
> + struct xe_gt *gt;
> + int id, err;
> +
> + pci_warn(pdev, "preparing for PCIe reset\n");
> +
> + drm_warn(&xe->drm, "removing device access to userspace\n");
do we need two separate warnings here?
[ ] xe 0000:4d:00.0: preparing for PCIe FLR reset
[ ] xe 0000:4d:00.0: [drm] removing device access to userspace
> + drm_dev_unplug(&xe->drm);
> +
> + xe_pm_runtime_get(xe);
> + /* idle the GTs */
> + for_each_gt(gt, xe, id) {
> + err = xe_force_wake_get(gt_to_fw(gt), XE_FORCEWAKE_ALL);
> + if (err)
> + goto reset;
> + xe_uc_reset_prepare(>->uc);
> + err = xe_gt_idle(gt);
> + if (err) {
> + xe_gt_err(gt, "failed to idle gt (%pe)\n", ERR_PTR(err));
> + goto reset;
> + }
> +
> + err = xe_force_wake_put(gt_to_fw(gt), XE_FORCEWAKE_ALL);
> + XE_WARN_ON(err);
> + }
> + xe_pm_runtime_put(xe);
> +
> +reset:
> + pci_disable_device(pdev);
> +}
> +
> +/**
> + * xe_pci_reset_done - Called when PCIe reset is done.
> + * @pdev: PCI device struct
> + */
> +void xe_pci_reset_done(struct pci_dev *pdev)
> +{
> + const struct pci_device_id *ent = pci_match_id(pdev->driver->id_table, pdev);
> + struct xe_device *xe = pci_get_drvdata(pdev);
> +
> + dev_info(&pdev->dev,
> + "device went through PCIe reset, reenabling the device\n");
> +
> + if (pci_enable_device(pdev)) {
> + dev_err(&pdev->dev,
> + "Cannot re-enable PCI device after reset\n");
> + return;
> + }
> + pci_set_master(pdev);
> + xe_load_pci_state(pdev);
> +
> + xe->pci_device_is_reset = true;
> + /*
> + * We want to completely clean the driver and even destroy
> + * the xe private data and reinitialize afresh similar to
> + * probe
> + */
> + pdev->driver->remove(pdev);
> + if (pci_dev_msi_enabled(pdev))
> + pci_free_irq_vectors(pdev);
> +
> + devm_drm_dev_release_action(&xe->drm);
> + pci_disable_device(pdev);
> +
> + /*
> + * if this fails the driver might be in a stale state, only option is
> + * to unbind and rebind
> + */
> + xe_pci_probe(pdev, ent);
> +}
> diff --git a/drivers/gpu/drm/xe/xe_pci_err.h b/drivers/gpu/drm/xe/xe_pci_err.h
> new file mode 100644
> index 000000000000..95a4c8ce9cf1
> --- /dev/null
> +++ b/drivers/gpu/drm/xe/xe_pci_err.h
> @@ -0,0 +1,13 @@
> +/* SPDX-License-Identifier: MIT */
> +/*
> + * Copyright © 2024 Intel Corporation
> + */
> +
> +#ifndef _XE_PCI_ERR_H_
> +#define _XE_PCI_ERR_H_
> +
> +struct pci_dev;
> +
> +void xe_pci_reset_prepare(struct pci_dev *pdev);
> +void xe_pci_reset_done(struct pci_dev *pdev);
> +#endif
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 1/4] drm: add devm release action
2024-04-17 13:55 ` Rodrigo Vivi
@ 2024-04-17 14:03 ` Maxime Ripard
0 siblings, 0 replies; 17+ messages in thread
From: Maxime Ripard @ 2024-04-17 14:03 UTC (permalink / raw)
To: Rodrigo Vivi
Cc: Aravind Iddamsetty, Daniel Vetter, Dave Airlie, dri-devel, daniel,
maarten.lankhorst, tzimmermann, intel-xe, Thomas Hellstr_m
[-- Attachment #1: Type: text/plain, Size: 807 bytes --]
On Wed, Apr 17, 2024 at 09:55:55AM -0400, Rodrigo Vivi wrote:
> On Wed, Apr 17, 2024 at 02:11:42PM +0530, Aravind Iddamsetty wrote:
> > In scenarios where drm_dev_put is directly called by driver we want to
> > release devm_drm_dev_init_release action associated with struct
> > drm_device.
> >
> > v2: Directly expose the original function, instead of introducing a
> > helper (Rodrigo)
> >
> > Cc: Thomas Hellstr_m <thomas.hellstrom@linux.intel.com>
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> >
> > Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
>
> Sima, Dave, or drm-misc, ack to get this through drm-xe-next?
On principle, I'm fine with some later iteration of that patch going
through drm/xe, but the documentation needs to be improved in its
current state.
Maxime
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 273 bytes --]
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 4/4] drm/xe/FLR: Support PCIe FLR
2024-04-17 13:59 ` Michal Wajdeczko
@ 2024-04-18 10:11 ` Aravind Iddamsetty
0 siblings, 0 replies; 17+ messages in thread
From: Aravind Iddamsetty @ 2024-04-18 10:11 UTC (permalink / raw)
To: Michal Wajdeczko, dri-devel, daniel, maarten.lankhorst, airlied,
mripard, tzimmermann, rodrigo.vivi
Cc: intel-xe, Lucas De Marchi
On 17/04/24 19:29, Michal Wajdeczko wrote:
>
> On 17.04.2024 10:41, Aravind Iddamsetty wrote:
>> PCI subsystem provides callbacks to inform the driver about a request to
>> do function level reset by user, initiated by writing to sysfs entry
>> /sys/bus/pci/devices/.../reset. This will allow the driver to handle FLR
>> without the need to do unbind and rebind as the driver needs to
>> reinitialize the device afresh post FLR.
>>
>> v2:
>> 1. separate out gt idle and pci save/restore to a separate patch (Lucas)
>> 2. Fixed the warnings seen around xe_guc_submit_stop, xe_guc_puc_fini
>>
>> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
>> Cc: Lucas De Marchi <lucas.demarchi@intel.com>
>>
>> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
>> Signed-off-by: Aravind Iddamsetty <aravind.iddamsetty@linux.intel.com>
>> ---
>> drivers/gpu/drm/xe/Makefile | 1 +
>> drivers/gpu/drm/xe/xe_device_types.h | 3 +
>> drivers/gpu/drm/xe/xe_guc_pc.c | 4 ++
>> drivers/gpu/drm/xe/xe_pci.c | 9 ++-
>> drivers/gpu/drm/xe/xe_pci.h | 2 +
>> drivers/gpu/drm/xe/xe_pci_err.c | 93 ++++++++++++++++++++++++++++
>> drivers/gpu/drm/xe/xe_pci_err.h | 13 ++++
>> 7 files changed, 124 insertions(+), 1 deletion(-)
>> create mode 100644 drivers/gpu/drm/xe/xe_pci_err.c
>> create mode 100644 drivers/gpu/drm/xe/xe_pci_err.h
>>
>> diff --git a/drivers/gpu/drm/xe/Makefile b/drivers/gpu/drm/xe/Makefile
>> index 522bffa5c4e1..9686ea181ad1 100644
>> --- a/drivers/gpu/drm/xe/Makefile
>> +++ b/drivers/gpu/drm/xe/Makefile
>> @@ -117,6 +117,7 @@ xe-y += xe_bb.o \
>> xe_module.o \
>> xe_pat.o \
>> xe_pci.o \
>> + xe_pci_err.o \
>> xe_pcode.o \
>> xe_pm.o \
>> xe_preempt_fence.o \
>> diff --git a/drivers/gpu/drm/xe/xe_device_types.h b/drivers/gpu/drm/xe/xe_device_types.h
>> index 0a66555229e9..8c749b378a92 100644
>> --- a/drivers/gpu/drm/xe/xe_device_types.h
>> +++ b/drivers/gpu/drm/xe/xe_device_types.h
>> @@ -465,6 +465,9 @@ struct xe_device {
>> /** @pci_state: PCI state of device */
>> struct pci_saved_state *pci_state;
>>
>> + /** @pci_device_is_reset: device went through PCIe FLR */
>> + bool pci_device_is_reset;
>> +
>> /* private: */
>>
>> #if IS_ENABLED(CONFIG_DRM_XE_DISPLAY)
>> diff --git a/drivers/gpu/drm/xe/xe_guc_pc.c b/drivers/gpu/drm/xe/xe_guc_pc.c
>> index 521ae24f2314..6108a4dfa05f 100644
>> --- a/drivers/gpu/drm/xe/xe_guc_pc.c
>> +++ b/drivers/gpu/drm/xe/xe_guc_pc.c
>> @@ -902,6 +902,10 @@ static void xe_guc_pc_fini(struct drm_device *drm, void *arg)
>> return;
>> }
>>
>> + /* We already have done this before going through a reset, so skip here */
>> + if (xe->pci_device_is_reset)
>> + return;
>> +
>> XE_WARN_ON(xe_force_wake_get(gt_to_fw(pc_to_gt(pc)), XE_FORCEWAKE_ALL));
>> XE_WARN_ON(xe_guc_pc_gucrc_disable(pc));
>> XE_WARN_ON(xe_guc_pc_stop(pc));
>> diff --git a/drivers/gpu/drm/xe/xe_pci.c b/drivers/gpu/drm/xe/xe_pci.c
>> index f9957cd16356..d986ae7fc7f7 100644
>> --- a/drivers/gpu/drm/xe/xe_pci.c
>> +++ b/drivers/gpu/drm/xe/xe_pci.c
>> @@ -23,6 +23,7 @@
>> #include "xe_macros.h"
>> #include "xe_mmio.h"
>> #include "xe_module.h"
>> +#include "xe_pci_err.h"
>> #include "xe_pci_types.h"
>> #include "xe_pm.h"
>> #include "xe_sriov.h"
>> @@ -736,7 +737,7 @@ static void xe_pci_remove(struct pci_dev *pdev)
>> pci_set_drvdata(pdev, NULL);
>> }
>>
>> -static int xe_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>> +int xe_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>> {
>> const struct xe_device_desc *desc = (const void *)ent->driver_data;
>> const struct xe_subplatform_desc *subplatform_desc;
>> @@ -976,6 +977,11 @@ static const struct dev_pm_ops xe_pm_ops = {
>> };
>> #endif
>>
>> +const struct pci_error_handlers xe_pci_err_handlers = {
> static ?
thanks will fix it.
>
>> + .reset_prepare = xe_pci_reset_prepare,
>> + .reset_done = xe_pci_reset_done,
>> +};
>> +
>> static struct pci_driver xe_pci_driver = {
>> .name = DRIVER_NAME,
>> .id_table = pciidlist,
>> @@ -985,6 +991,7 @@ static struct pci_driver xe_pci_driver = {
>> #ifdef CONFIG_PM_SLEEP
>> .driver.pm = &xe_pm_ops,
>> #endif
>> + .err_handler = &xe_pci_err_handlers,
>> };
>>
>> int xe_register_pci_driver(void)
>> diff --git a/drivers/gpu/drm/xe/xe_pci.h b/drivers/gpu/drm/xe/xe_pci.h
>> index 73b90a430d1f..9faf5380a09e 100644
>> --- a/drivers/gpu/drm/xe/xe_pci.h
>> +++ b/drivers/gpu/drm/xe/xe_pci.h
>> @@ -7,8 +7,10 @@
>> #define _XE_PCI_H_
>>
>> struct pci_dev;
>> +struct pci_device_id;
>>
>> int xe_register_pci_driver(void);
>> void xe_unregister_pci_driver(void);
>> void xe_load_pci_state(struct pci_dev *pdev);
>> +int xe_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent);
>> #endif
>> diff --git a/drivers/gpu/drm/xe/xe_pci_err.c b/drivers/gpu/drm/xe/xe_pci_err.c
>> new file mode 100644
>> index 000000000000..81c440e08fbc
>> --- /dev/null
>> +++ b/drivers/gpu/drm/xe/xe_pci_err.c
>> @@ -0,0 +1,93 @@
>> +// SPDX-License-Identifier: MIT
>> +/*
>> + * Copyright © 2024 Intel Corporation
>> + */
>> +
>> +#include <linux/pci.h>
>> +#include <drm/drm_drv.h>
>> +
>> +#include "xe_device.h"
>> +#include "xe_gt.h"
>> +#include "xe_gt_printk.h"
>> +#include "xe_pci.h"
>> +#include "xe_pci_err.h"
>> +#include "xe_pm.h"
>> +#include "xe_uc.h"
>> +
>> +/**
>> + * xe_pci_reset_prepare - Called when user issued a PCIe reset
>> + * via /sys/bus/pci/devices/.../reset.
>> + * @pdev: PCI device struct
>> + */
>> +void xe_pci_reset_prepare(struct pci_dev *pdev)
>> +{
>> + struct xe_device *xe = pci_get_drvdata(pdev);
>> + struct xe_gt *gt;
>> + int id, err;
>> +
>> + pci_warn(pdev, "preparing for PCIe reset\n");
>> +
>> + drm_warn(&xe->drm, "removing device access to userspace\n");
> do we need two separate warnings here?
>
> [ ] xe 0000:4d:00.0: preparing for PCIe FLR reset
> [ ] xe 0000:4d:00.0: [drm] removing device access to userspace
well yes i wanted to convey that a PCIe reset is triggered by the PCIe subsystem and
in response to that we are removing the drm device hence a drm warn.
Thanks,
Aravind.
>
>> + drm_dev_unplug(&xe->drm);
>> +
>> + xe_pm_runtime_get(xe);
>> + /* idle the GTs */
>> + for_each_gt(gt, xe, id) {
>> + err = xe_force_wake_get(gt_to_fw(gt), XE_FORCEWAKE_ALL);
>> + if (err)
>> + goto reset;
>> + xe_uc_reset_prepare(>->uc);
>> + err = xe_gt_idle(gt);
>> + if (err) {
>> + xe_gt_err(gt, "failed to idle gt (%pe)\n", ERR_PTR(err));
>> + goto reset;
>> + }
>> +
>> + err = xe_force_wake_put(gt_to_fw(gt), XE_FORCEWAKE_ALL);
>> + XE_WARN_ON(err);
>> + }
>> + xe_pm_runtime_put(xe);
>> +
>> +reset:
>> + pci_disable_device(pdev);
>> +}
>> +
>> +/**
>> + * xe_pci_reset_done - Called when PCIe reset is done.
>> + * @pdev: PCI device struct
>> + */
>> +void xe_pci_reset_done(struct pci_dev *pdev)
>> +{
>> + const struct pci_device_id *ent = pci_match_id(pdev->driver->id_table, pdev);
>> + struct xe_device *xe = pci_get_drvdata(pdev);
>> +
>> + dev_info(&pdev->dev,
>> + "device went through PCIe reset, reenabling the device\n");
>> +
>> + if (pci_enable_device(pdev)) {
>> + dev_err(&pdev->dev,
>> + "Cannot re-enable PCI device after reset\n");
>> + return;
>> + }
>> + pci_set_master(pdev);
>> + xe_load_pci_state(pdev);
>> +
>> + xe->pci_device_is_reset = true;
>> + /*
>> + * We want to completely clean the driver and even destroy
>> + * the xe private data and reinitialize afresh similar to
>> + * probe
>> + */
>> + pdev->driver->remove(pdev);
>> + if (pci_dev_msi_enabled(pdev))
>> + pci_free_irq_vectors(pdev);
>> +
>> + devm_drm_dev_release_action(&xe->drm);
>> + pci_disable_device(pdev);
>> +
>> + /*
>> + * if this fails the driver might be in a stale state, only option is
>> + * to unbind and rebind
>> + */
>> + xe_pci_probe(pdev, ent);
>> +}
>> diff --git a/drivers/gpu/drm/xe/xe_pci_err.h b/drivers/gpu/drm/xe/xe_pci_err.h
>> new file mode 100644
>> index 000000000000..95a4c8ce9cf1
>> --- /dev/null
>> +++ b/drivers/gpu/drm/xe/xe_pci_err.h
>> @@ -0,0 +1,13 @@
>> +/* SPDX-License-Identifier: MIT */
>> +/*
>> + * Copyright © 2024 Intel Corporation
>> + */
>> +
>> +#ifndef _XE_PCI_ERR_H_
>> +#define _XE_PCI_ERR_H_
>> +
>> +struct pci_dev;
>> +
>> +void xe_pci_reset_prepare(struct pci_dev *pdev);
>> +void xe_pci_reset_done(struct pci_dev *pdev);
>> +#endif
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 3/4] drm/xe: Extract xe_gt_idle() helper
2024-04-17 13:50 ` Michal Wajdeczko
@ 2024-04-18 10:13 ` Aravind Iddamsetty
0 siblings, 0 replies; 17+ messages in thread
From: Aravind Iddamsetty @ 2024-04-18 10:13 UTC (permalink / raw)
To: Michal Wajdeczko, dri-devel, daniel, maarten.lankhorst, airlied,
mripard, tzimmermann, rodrigo.vivi
Cc: intel-xe, Lucas De Marchi
On 17/04/24 19:20, Michal Wajdeczko wrote:
>
> On 17.04.2024 10:41, Aravind Iddamsetty wrote:
>> This would be used in other places outside of gt_reset path.
>>
>> Cc: Lucas De Marchi <lucas.demarchi@intel.com>
>>
>> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
>> Signed-off-by: Aravind Iddamsetty <aravind.iddamsetty@linux.intel.com>
>> ---
>> drivers/gpu/drm/xe/xe_gt.c | 31 +++++++++++++++++++++----------
>> drivers/gpu/drm/xe/xe_gt.h | 1 +
>> 2 files changed, 22 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/xe/xe_gt.c b/drivers/gpu/drm/xe/xe_gt.c
>> index cfa5da900461..59f497d575ad 100644
>> --- a/drivers/gpu/drm/xe/xe_gt.c
>> +++ b/drivers/gpu/drm/xe/xe_gt.c
>> @@ -629,6 +629,26 @@ static int do_gt_restart(struct xe_gt *gt)
>> return 0;
>> }
>>
>> +/* Idle the GT */
> as this is a public function, can we have a proper kernel-doc here?
>
> and maybe some hint what should be done if this function fails?
Ok will do.
Thanks,
Aravind.
>
>> +int xe_gt_idle(struct xe_gt *gt)
>> +{
>> + int err;
>> +
>> + xe_gt_sanitize(gt);
>> +
>> + xe_uc_gucrc_disable(>->uc);
>> + xe_uc_stop_prepare(>->uc);
>> + xe_gt_pagefault_reset(gt);
>> +
>> + err = xe_uc_stop(>->uc);
>> + if (err)
>> + return err;
>> +
>> + xe_gt_tlb_invalidation_reset(gt);
>> +
>> + return err;
>> +}
>> +
>> static int gt_reset(struct xe_gt *gt)
>> {
>> int err;
>> @@ -645,21 +665,12 @@ static int gt_reset(struct xe_gt *gt)
>> }
>>
>> xe_pm_runtime_get(gt_to_xe(gt));
>> - xe_gt_sanitize(gt);
>>
>> err = xe_force_wake_get(gt_to_fw(gt), XE_FORCEWAKE_ALL);
>> if (err)
>> goto err_msg;
>>
>> - xe_uc_gucrc_disable(>->uc);
>> - xe_uc_stop_prepare(>->uc);
>> - xe_gt_pagefault_reset(gt);
>> -
>> - err = xe_uc_stop(>->uc);
>> - if (err)
>> - goto err_out;
>> -
>> - xe_gt_tlb_invalidation_reset(gt);
>> + xe_gt_idle(gt);
>>
>> err = do_gt_reset(gt);
>> if (err)
>> diff --git a/drivers/gpu/drm/xe/xe_gt.h b/drivers/gpu/drm/xe/xe_gt.h
>> index ed6ea8057e35..d62af1725ff6 100644
>> --- a/drivers/gpu/drm/xe/xe_gt.h
>> +++ b/drivers/gpu/drm/xe/xe_gt.h
>> @@ -43,6 +43,7 @@ int xe_gt_resume(struct xe_gt *gt);
>> void xe_gt_reset_async(struct xe_gt *gt);
>> void xe_gt_sanitize(struct xe_gt *gt);
>> void xe_gt_remove(struct xe_gt *gt);
>> +int xe_gt_idle(struct xe_gt *gt);
>>
>> /**
>> * xe_gt_any_hw_engine_by_reset_domain - scan the list of engines and return the
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v2 1/4] drm: add devm release action
2024-04-19 8:58 [PATCH v3 0/4] drm/xe: " Aravind Iddamsetty
@ 2024-04-19 8:58 ` Aravind Iddamsetty
2024-04-19 9:11 ` Maxime Ripard
0 siblings, 1 reply; 17+ messages in thread
From: Aravind Iddamsetty @ 2024-04-19 8:58 UTC (permalink / raw)
To: dri-devel, daniel, maarten.lankhorst, airlied, mripard,
tzimmermann, rodrigo.vivi
Cc: intel-xe, Thomas Hellstr_m
In scenarios where drm_dev_put is directly called by driver we want to
release devm_drm_dev_init_release action associated with struct
drm_device.
v2: Directly expose the original function, instead of introducing a
helper (Rodrigo)
Cc: Thomas Hellstr_m <thomas.hellstrom@linux.intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Aravind Iddamsetty <aravind.iddamsetty@linux.intel.com>
---
drivers/gpu/drm/drm_drv.c | 6 ++++++
include/drm/drm_drv.h | 2 ++
2 files changed, 8 insertions(+)
diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index 243cacb3575c..ba60cbb0725f 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -714,6 +714,12 @@ static int devm_drm_dev_init(struct device *parent,
devm_drm_dev_init_release, dev);
}
+void devm_drm_dev_release_action(struct drm_device *dev)
+{
+ devm_release_action(dev->dev, devm_drm_dev_init_release, dev);
+}
+EXPORT_SYMBOL(devm_drm_dev_release_action);
+
void *__devm_drm_dev_alloc(struct device *parent,
const struct drm_driver *driver,
size_t size, size_t offset)
diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h
index 8878260d7529..fa9123684874 100644
--- a/include/drm/drm_drv.h
+++ b/include/drm/drm_drv.h
@@ -444,6 +444,8 @@ struct drm_driver {
const struct file_operations *fops;
};
+void devm_drm_dev_release_action(struct drm_device *dev);
+
void *__devm_drm_dev_alloc(struct device *parent,
const struct drm_driver *driver,
size_t size, size_t offset);
--
2.25.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH v2 1/4] drm: add devm release action
2024-04-19 8:58 ` [PATCH v2 1/4] drm: add devm release action Aravind Iddamsetty
@ 2024-04-19 9:11 ` Maxime Ripard
2024-04-20 8:12 ` Aravind Iddamsetty
0 siblings, 1 reply; 17+ messages in thread
From: Maxime Ripard @ 2024-04-19 9:11 UTC (permalink / raw)
To: Aravind Iddamsetty
Cc: dri-devel, daniel, maarten.lankhorst, airlied, tzimmermann,
rodrigo.vivi, intel-xe, Thomas Hellstr_m
[-- Attachment #1: Type: text/plain, Size: 1236 bytes --]
On Fri, Apr 19, 2024 at 02:28:23PM +0530, Aravind Iddamsetty wrote:
> In scenarios where drm_dev_put is directly called by driver we want to
> release devm_drm_dev_init_release action associated with struct
> drm_device.
>
> v2: Directly expose the original function, instead of introducing a
> helper (Rodrigo)
>
> Cc: Thomas Hellstr_m <thomas.hellstrom@linux.intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
>
> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: Aravind Iddamsetty <aravind.iddamsetty@linux.intel.com>
> ---
> drivers/gpu/drm/drm_drv.c | 6 ++++++
> include/drm/drm_drv.h | 2 ++
> 2 files changed, 8 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> index 243cacb3575c..ba60cbb0725f 100644
> --- a/drivers/gpu/drm/drm_drv.c
> +++ b/drivers/gpu/drm/drm_drv.c
> @@ -714,6 +714,12 @@ static int devm_drm_dev_init(struct device *parent,
> devm_drm_dev_init_release, dev);
> }
>
> +void devm_drm_dev_release_action(struct drm_device *dev)
> +{
> + devm_release_action(dev->dev, devm_drm_dev_init_release, dev);
> +}
> +EXPORT_SYMBOL(devm_drm_dev_release_action);
Again, this needs to be documented.
Maxime
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 273 bytes --]
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 1/4] drm: add devm release action
2024-04-19 9:11 ` Maxime Ripard
@ 2024-04-20 8:12 ` Aravind Iddamsetty
0 siblings, 0 replies; 17+ messages in thread
From: Aravind Iddamsetty @ 2024-04-20 8:12 UTC (permalink / raw)
To: Maxime Ripard
Cc: dri-devel, daniel, maarten.lankhorst, airlied, tzimmermann,
rodrigo.vivi, intel-xe, Thomas Hellstr_m
On 19/04/24 14:41, Maxime Ripard wrote:
> On Fri, Apr 19, 2024 at 02:28:23PM +0530, Aravind Iddamsetty wrote:
>> In scenarios where drm_dev_put is directly called by driver we want to
>> release devm_drm_dev_init_release action associated with struct
>> drm_device.
>>
>> v2: Directly expose the original function, instead of introducing a
>> helper (Rodrigo)
>>
>> Cc: Thomas Hellstr_m <thomas.hellstrom@linux.intel.com>
>> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
>>
>> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
>> Signed-off-by: Aravind Iddamsetty <aravind.iddamsetty@linux.intel.com>
>> ---
>> drivers/gpu/drm/drm_drv.c | 6 ++++++
>> include/drm/drm_drv.h | 2 ++
>> 2 files changed, 8 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
>> index 243cacb3575c..ba60cbb0725f 100644
>> --- a/drivers/gpu/drm/drm_drv.c
>> +++ b/drivers/gpu/drm/drm_drv.c
>> @@ -714,6 +714,12 @@ static int devm_drm_dev_init(struct device *parent,
>> devm_drm_dev_init_release, dev);
>> }
>>
>> +void devm_drm_dev_release_action(struct drm_device *dev)
>> +{
>> + devm_release_action(dev->dev, devm_drm_dev_init_release, dev);
>> +}
>> +EXPORT_SYMBOL(devm_drm_dev_release_action);
> Again, this needs to be documented.
sorry I missed your earlier email, will address this.
Thanks,
Aravind.
>
> Maxime
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2024-04-20 8:09 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-04-17 8:41 [PATCH v2 0/4] drm/xe: Support PCIe FLR Aravind Iddamsetty
2024-04-17 8:41 ` [PATCH v2 1/4] drm: add devm release action Aravind Iddamsetty
2024-04-17 13:55 ` Rodrigo Vivi
2024-04-17 14:03 ` Maxime Ripard
2024-04-17 8:41 ` [PATCH 2/4] drm/xe: Save and restore PCI state Aravind Iddamsetty
2024-04-17 8:41 ` [PATCH 3/4] drm/xe: Extract xe_gt_idle() helper Aravind Iddamsetty
2024-04-17 13:50 ` Michal Wajdeczko
2024-04-18 10:13 ` Aravind Iddamsetty
2024-04-17 8:41 ` [PATCH v2 4/4] drm/xe/FLR: Support PCIe FLR Aravind Iddamsetty
2024-04-17 13:59 ` Michal Wajdeczko
2024-04-18 10:11 ` Aravind Iddamsetty
-- strict thread matches above, loose matches on Subject: below --
2024-04-19 8:58 [PATCH v3 0/4] drm/xe: " Aravind Iddamsetty
2024-04-19 8:58 ` [PATCH v2 1/4] drm: add devm release action Aravind Iddamsetty
2024-04-19 9:11 ` Maxime Ripard
2024-04-20 8:12 ` Aravind Iddamsetty
2024-04-02 8:58 [PATCH v2 0/4] drm/xe: Support PCIe FLR Aravind Iddamsetty
2024-04-02 8:58 ` [PATCH v2 1/4] drm: add devm release action Aravind Iddamsetty
2024-04-15 4:42 ` Aravind Iddamsetty
2024-04-16 10:28 ` Aravind Iddamsetty
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox