* [Intel-xe] [PATCH v4 01/22] drm/xe: Skip calling drm_dev_put on probe error
2023-11-29 1:16 [Intel-xe] [PATCH v4 00/22] drm/xe: Probe tweaks and reordering Michał Winiarski
@ 2023-11-29 1:16 ` Michał Winiarski
2023-11-29 1:16 ` [Intel-xe] [PATCH v4 02/22] drm/xe: Use managed pci_enable_device Michał Winiarski
` (21 subsequent siblings)
22 siblings, 0 replies; 32+ messages in thread
From: Michał Winiarski @ 2023-11-29 1:16 UTC (permalink / raw)
To: intel-xe; +Cc: Matt Roper, Lucas De Marchi, Michał Winiarski
DRM device used by Xe is managed, which means that final ref will be
dropped on driver detach.
Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
Reviewed-by: Matthew Brost <matthew.brost@intel.com>
---
drivers/gpu/drm/xe/xe_device.c | 12 +++++-------
drivers/gpu/drm/xe/xe_pci.c | 5 +----
2 files changed, 6 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/xe/xe_device.c b/drivers/gpu/drm/xe/xe_device.c
index 54202623e2553..296260f142dcb 100644
--- a/drivers/gpu/drm/xe/xe_device.c
+++ b/drivers/gpu/drm/xe/xe_device.c
@@ -215,11 +215,11 @@ struct xe_device *xe_device_create(struct pci_dev *pdev,
xe->drm.anon_inode->i_mapping,
xe->drm.vma_offset_manager, false, false);
if (WARN_ON(err))
- goto err_put;
+ goto err;
err = drmm_add_action_or_reset(&xe->drm, xe_device_destroy, NULL);
if (err)
- goto err_put;
+ goto err;
xe->info.devid = pdev->device;
xe->info.revid = pdev->revision;
@@ -258,18 +258,16 @@ struct xe_device *xe_device_create(struct pci_dev *pdev,
if (!xe->ordered_wq || !xe->unordered_wq) {
drm_err(&xe->drm, "Failed to allocate xe workqueues\n");
err = -ENOMEM;
- goto err_put;
+ goto err;
}
err = xe_display_create(xe);
if (WARN_ON(err))
- goto err_put;
+ goto err;
return xe;
-err_put:
- drm_dev_put(&xe->drm);
-
+err:
return ERR_PTR(err);
}
diff --git a/drivers/gpu/drm/xe/xe_pci.c b/drivers/gpu/drm/xe/xe_pci.c
index 994679a4eb897..db2a58747e25e 100644
--- a/drivers/gpu/drm/xe/xe_pci.c
+++ b/drivers/gpu/drm/xe/xe_pci.c
@@ -703,7 +703,7 @@ static int xe_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
pci_set_drvdata(pdev, xe);
err = pci_enable_device(pdev);
if (err)
- goto err_drm_put;
+ return err;
pci_set_master(pdev);
@@ -751,9 +751,6 @@ static int xe_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
err_pci_disable:
pci_disable_device(pdev);
-err_drm_put:
- drm_dev_put(&xe->drm);
-
return err;
}
--
2.43.0
^ permalink raw reply related [flat|nested] 32+ messages in thread* [Intel-xe] [PATCH v4 02/22] drm/xe: Use managed pci_enable_device
2023-11-29 1:16 [Intel-xe] [PATCH v4 00/22] drm/xe: Probe tweaks and reordering Michał Winiarski
2023-11-29 1:16 ` [Intel-xe] [PATCH v4 01/22] drm/xe: Skip calling drm_dev_put on probe error Michał Winiarski
@ 2023-11-29 1:16 ` Michał Winiarski
2023-11-29 16:02 ` Matt Roper
2023-11-29 1:16 ` [Intel-xe] [PATCH v4 03/22] drm/xe/irq: Don't call pci_free_irq_vectors Michał Winiarski
` (20 subsequent siblings)
22 siblings, 1 reply; 32+ messages in thread
From: Michał Winiarski @ 2023-11-29 1:16 UTC (permalink / raw)
To: intel-xe; +Cc: Matt Roper, Lucas De Marchi, Michał Winiarski
Xe uses devres for most of its driver-lifetime resources, use it for pci
device as well.
Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
---
v2 -> v3:
- Mark xe_pci_clear_master as static to fix W=1 build warning (CI)
v3 -> v4:
- Drop explicit call to pci_clear_master (Matt Roper)
drivers/gpu/drm/xe/xe_pci.c | 20 ++++++++------------
1 file changed, 8 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/xe/xe_pci.c b/drivers/gpu/drm/xe/xe_pci.c
index db2a58747e25e..cc5dfccfb8c08 100644
--- a/drivers/gpu/drm/xe/xe_pci.c
+++ b/drivers/gpu/drm/xe/xe_pci.c
@@ -693,25 +693,26 @@ static int xe_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
if (xe_display_driver_probe_defer(pdev))
return -EPROBE_DEFER;
+ err = pcim_enable_device(pdev);
+ if (err)
+ return err;
+
xe = xe_device_create(pdev, ent);
if (IS_ERR(xe))
return PTR_ERR(xe);
+ pci_set_drvdata(pdev, xe);
+
xe_pm_assert_unbounded_bridge(xe);
subplatform_desc = find_subplatform(xe, desc);
- pci_set_drvdata(pdev, xe);
- err = pci_enable_device(pdev);
- if (err)
- return err;
-
pci_set_master(pdev);
xe_sriov_probe_early(xe, desc->has_sriov);
err = xe_info_init(xe, desc, subplatform_desc);
if (err)
- goto err_pci_disable;
+ return err;
xe_display_probe(xe);
@@ -742,16 +743,11 @@ static int xe_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
err = xe_device_probe(xe);
if (err)
- goto err_pci_disable;
+ return err;
xe_pm_init(xe);
return 0;
-
-err_pci_disable:
- pci_disable_device(pdev);
-
- return err;
}
static void xe_pci_shutdown(struct pci_dev *pdev)
--
2.43.0
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [Intel-xe] [PATCH v4 02/22] drm/xe: Use managed pci_enable_device
2023-11-29 1:16 ` [Intel-xe] [PATCH v4 02/22] drm/xe: Use managed pci_enable_device Michał Winiarski
@ 2023-11-29 16:02 ` Matt Roper
0 siblings, 0 replies; 32+ messages in thread
From: Matt Roper @ 2023-11-29 16:02 UTC (permalink / raw)
To: Michał Winiarski; +Cc: Lucas De Marchi, intel-xe
On Wed, Nov 29, 2023 at 02:16:04AM +0100, Michał Winiarski wrote:
> Xe uses devres for most of its driver-lifetime resources, use it for pci
> device as well.
>
> Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
> ---
> v2 -> v3:
> - Mark xe_pci_clear_master as static to fix W=1 build warning (CI)
> v3 -> v4:
> - Drop explicit call to pci_clear_master (Matt Roper)
>
> drivers/gpu/drm/xe/xe_pci.c | 20 ++++++++------------
> 1 file changed, 8 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/xe/xe_pci.c b/drivers/gpu/drm/xe/xe_pci.c
> index db2a58747e25e..cc5dfccfb8c08 100644
> --- a/drivers/gpu/drm/xe/xe_pci.c
> +++ b/drivers/gpu/drm/xe/xe_pci.c
> @@ -693,25 +693,26 @@ static int xe_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> if (xe_display_driver_probe_defer(pdev))
> return -EPROBE_DEFER;
>
> + err = pcim_enable_device(pdev);
> + if (err)
> + return err;
> +
> xe = xe_device_create(pdev, ent);
> if (IS_ERR(xe))
> return PTR_ERR(xe);
>
> + pci_set_drvdata(pdev, xe);
> +
> xe_pm_assert_unbounded_bridge(xe);
> subplatform_desc = find_subplatform(xe, desc);
>
> - pci_set_drvdata(pdev, xe);
> - err = pci_enable_device(pdev);
> - if (err)
> - return err;
> -
> pci_set_master(pdev);
>
> xe_sriov_probe_early(xe, desc->has_sriov);
>
> err = xe_info_init(xe, desc, subplatform_desc);
> if (err)
> - goto err_pci_disable;
> + return err;
>
> xe_display_probe(xe);
>
> @@ -742,16 +743,11 @@ static int xe_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>
> err = xe_device_probe(xe);
> if (err)
> - goto err_pci_disable;
> + return err;
>
> xe_pm_init(xe);
>
> return 0;
> -
> -err_pci_disable:
> - pci_disable_device(pdev);
> -
> - return err;
> }
>
> static void xe_pci_shutdown(struct pci_dev *pdev)
> --
> 2.43.0
>
--
Matt Roper
Graphics Software Engineer
Linux GPU Platform Enablement
Intel Corporation
^ permalink raw reply [flat|nested] 32+ messages in thread
* [Intel-xe] [PATCH v4 03/22] drm/xe/irq: Don't call pci_free_irq_vectors
2023-11-29 1:16 [Intel-xe] [PATCH v4 00/22] drm/xe: Probe tweaks and reordering Michał Winiarski
2023-11-29 1:16 ` [Intel-xe] [PATCH v4 01/22] drm/xe: Skip calling drm_dev_put on probe error Michał Winiarski
2023-11-29 1:16 ` [Intel-xe] [PATCH v4 02/22] drm/xe: Use managed pci_enable_device Michał Winiarski
@ 2023-11-29 1:16 ` Michał Winiarski
2023-11-29 1:16 ` [Intel-xe] [PATCH v4 04/22] drm/xe: Move xe_set_dma_info outside of MMIO setup Michał Winiarski
` (19 subsequent siblings)
22 siblings, 0 replies; 32+ messages in thread
From: Michał Winiarski @ 2023-11-29 1:16 UTC (permalink / raw)
To: intel-xe; +Cc: Matt Roper, Lucas De Marchi, Michał Winiarski
For devres managed devices, pci_alloc_irq_vectors is also managed (see
pci_setup_msi_context for reference).
PCI device used by Xe is devres managed (it was enabled with
pcim_enable_device), which means that calls to pci_free_irq_vectors are
redundant and can be safely removed.
Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
---
drivers/gpu/drm/xe/xe_irq.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/xe/xe_irq.c b/drivers/gpu/drm/xe/xe_irq.c
index 25ba5167c1b92..d1f5ba4bb745a 100644
--- a/drivers/gpu/drm/xe/xe_irq.c
+++ b/drivers/gpu/drm/xe/xe_irq.c
@@ -585,7 +585,6 @@ static void irq_uninstall(struct drm_device *drm, void *arg)
irq = pci_irq_vector(pdev, 0);
free_irq(irq, xe);
- pci_free_irq_vectors(pdev);
}
int xe_irq_install(struct xe_device *xe)
@@ -612,7 +611,7 @@ int xe_irq_install(struct xe_device *xe)
err = request_irq(irq, irq_handler, IRQF_SHARED, DRIVER_NAME, xe);
if (err < 0) {
drm_err(&xe->drm, "Failed to request MSI/MSIX IRQ %d\n", err);
- goto free_pci_irq_vectors;
+ return err;
}
xe->irq.enabled = true;
@@ -627,8 +626,6 @@ int xe_irq_install(struct xe_device *xe)
free_irq_handler:
free_irq(irq, xe);
-free_pci_irq_vectors:
- pci_free_irq_vectors(pdev);
return err;
}
--
2.43.0
^ permalink raw reply related [flat|nested] 32+ messages in thread* [Intel-xe] [PATCH v4 04/22] drm/xe: Move xe_set_dma_info outside of MMIO setup
2023-11-29 1:16 [Intel-xe] [PATCH v4 00/22] drm/xe: Probe tweaks and reordering Michał Winiarski
` (2 preceding siblings ...)
2023-11-29 1:16 ` [Intel-xe] [PATCH v4 03/22] drm/xe/irq: Don't call pci_free_irq_vectors Michał Winiarski
@ 2023-11-29 1:16 ` Michał Winiarski
2023-11-29 1:16 ` [Intel-xe] [PATCH v4 05/22] drm/xe: Move xe_mmio_probe_tiles " Michał Winiarski
` (18 subsequent siblings)
22 siblings, 0 replies; 32+ messages in thread
From: Michał Winiarski @ 2023-11-29 1:16 UTC (permalink / raw)
To: intel-xe; +Cc: Matt Roper, Lucas De Marchi, Michał Winiarski
MMIO is going to be setup earlier during probe. Move xe_set_dma_info
outside of MMIO setup.
Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
Reviewed-by: Matthew Brost <matthew.brost@intel.com>
---
drivers/gpu/drm/xe/xe_device.c | 26 ++++++++++++++++++++++++++
drivers/gpu/drm/xe/xe_mmio.c | 26 --------------------------
2 files changed, 26 insertions(+), 26 deletions(-)
diff --git a/drivers/gpu/drm/xe/xe_device.c b/drivers/gpu/drm/xe/xe_device.c
index 296260f142dcb..dcb0871083931 100644
--- a/drivers/gpu/drm/xe/xe_device.c
+++ b/drivers/gpu/drm/xe/xe_device.c
@@ -353,6 +353,28 @@ static void xe_device_sanitize(struct drm_device *drm, void *arg)
xe_gt_sanitize(gt);
}
+static int xe_set_dma_info(struct xe_device *xe)
+{
+ unsigned int mask_size = xe->info.dma_mask_size;
+ int err;
+
+ dma_set_max_seg_size(xe->drm.dev, xe_sg_segment_size(xe->drm.dev));
+
+ err = dma_set_mask(xe->drm.dev, DMA_BIT_MASK(mask_size));
+ if (err)
+ goto mask_err;
+
+ err = dma_set_coherent_mask(xe->drm.dev, DMA_BIT_MASK(mask_size));
+ if (err)
+ goto mask_err;
+
+ return 0;
+
+mask_err:
+ drm_err(&xe->drm, "Can't set DMA mask/consistent mask (%d)\n", err);
+ return err;
+}
+
int xe_device_probe(struct xe_device *xe)
{
struct xe_tile *tile;
@@ -367,6 +389,10 @@ int xe_device_probe(struct xe_device *xe)
if (err)
return err;
+ err = xe_set_dma_info(xe);
+ if (err)
+ return err;
+
for_each_tile(tile, xe, id) {
err = xe_tile_alloc(tile);
if (err)
diff --git a/drivers/gpu/drm/xe/xe_mmio.c b/drivers/gpu/drm/xe/xe_mmio.c
index 0f846272e39c7..9e8adb73cc978 100644
--- a/drivers/gpu/drm/xe/xe_mmio.c
+++ b/drivers/gpu/drm/xe/xe_mmio.c
@@ -25,28 +25,6 @@
#define BAR_SIZE_SHIFT 20
-static int xe_set_dma_info(struct xe_device *xe)
-{
- unsigned int mask_size = xe->info.dma_mask_size;
- int err;
-
- dma_set_max_seg_size(xe->drm.dev, xe_sg_segment_size(xe->drm.dev));
-
- err = dma_set_mask(xe->drm.dev, DMA_BIT_MASK(mask_size));
- if (err)
- goto mask_err;
-
- err = dma_set_coherent_mask(xe->drm.dev, DMA_BIT_MASK(mask_size));
- if (err)
- goto mask_err;
-
- return 0;
-
-mask_err:
- drm_err(&xe->drm, "Can't set DMA mask/consistent mask (%d)\n", err);
- return err;
-}
-
static void
_resize_bar(struct xe_device *xe, int resno, resource_size_t size)
{
@@ -431,10 +409,6 @@ int xe_mmio_init(struct xe_device *xe)
if (err)
return err;
- err = xe_set_dma_info(xe);
- if (err)
- return err;
-
xe_mmio_probe_tiles(xe);
return 0;
--
2.43.0
^ permalink raw reply related [flat|nested] 32+ messages in thread* [Intel-xe] [PATCH v4 05/22] drm/xe: Move xe_mmio_probe_tiles outside of MMIO setup
2023-11-29 1:16 [Intel-xe] [PATCH v4 00/22] drm/xe: Probe tweaks and reordering Michał Winiarski
` (3 preceding siblings ...)
2023-11-29 1:16 ` [Intel-xe] [PATCH v4 04/22] drm/xe: Move xe_set_dma_info outside of MMIO setup Michał Winiarski
@ 2023-11-29 1:16 ` Michał Winiarski
2023-11-29 1:16 ` [Intel-xe] [PATCH v4 06/22] drm/xe: Split xe_info_init Michał Winiarski
` (17 subsequent siblings)
22 siblings, 0 replies; 32+ messages in thread
From: Michał Winiarski @ 2023-11-29 1:16 UTC (permalink / raw)
To: intel-xe; +Cc: Matt Roper, Lucas De Marchi, Michał Winiarski
MMIO is going to be setup earlier during probe. Move xe_mmio_probe_tiles
outside of MMIO setup.
Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
Reviewed-by: Matthew Brost <matthew.brost@intel.com>
---
drivers/gpu/drm/xe/xe_device.c | 2 ++
drivers/gpu/drm/xe/xe_mmio.c | 4 +---
drivers/gpu/drm/xe/xe_mmio.h | 1 +
3 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/xe/xe_device.c b/drivers/gpu/drm/xe/xe_device.c
index dcb0871083931..785bf2e610b7c 100644
--- a/drivers/gpu/drm/xe/xe_device.c
+++ b/drivers/gpu/drm/xe/xe_device.c
@@ -403,6 +403,8 @@ int xe_device_probe(struct xe_device *xe)
if (err)
return err;
+ xe_mmio_probe_tiles(xe);
+
err = drmm_add_action_or_reset(&xe->drm, xe_driver_flr_fini, xe);
if (err)
return err;
diff --git a/drivers/gpu/drm/xe/xe_mmio.c b/drivers/gpu/drm/xe/xe_mmio.c
index 9e8adb73cc978..9769d56988ece 100644
--- a/drivers/gpu/drm/xe/xe_mmio.c
+++ b/drivers/gpu/drm/xe/xe_mmio.c
@@ -295,7 +295,7 @@ int xe_mmio_probe_vram(struct xe_device *xe)
return 0;
}
-static void xe_mmio_probe_tiles(struct xe_device *xe)
+void xe_mmio_probe_tiles(struct xe_device *xe)
{
size_t tile_mmio_size = SZ_16M, tile_mmio_ext_size = xe->info.tile_mmio_ext_size;
u8 id, tile_count = xe->info.tile_count;
@@ -409,8 +409,6 @@ int xe_mmio_init(struct xe_device *xe)
if (err)
return err;
- xe_mmio_probe_tiles(xe);
-
return 0;
}
diff --git a/drivers/gpu/drm/xe/xe_mmio.h b/drivers/gpu/drm/xe/xe_mmio.h
index 218b796629adc..c054c27f6925d 100644
--- a/drivers/gpu/drm/xe/xe_mmio.h
+++ b/drivers/gpu/drm/xe/xe_mmio.h
@@ -21,6 +21,7 @@ struct xe_device;
#define LMEM_BAR 2
int xe_mmio_init(struct xe_device *xe);
+void xe_mmio_probe_tiles(struct xe_device *xe);
static inline u8 xe_mmio_read8(struct xe_gt *gt, struct xe_reg reg)
{
--
2.43.0
^ permalink raw reply related [flat|nested] 32+ messages in thread* [Intel-xe] [PATCH v4 06/22] drm/xe: Split xe_info_init
2023-11-29 1:16 [Intel-xe] [PATCH v4 00/22] drm/xe: Probe tweaks and reordering Michał Winiarski
` (4 preceding siblings ...)
2023-11-29 1:16 ` [Intel-xe] [PATCH v4 05/22] drm/xe: Move xe_mmio_probe_tiles " Michał Winiarski
@ 2023-11-29 1:16 ` Michał Winiarski
2023-11-29 1:16 ` [Intel-xe] [PATCH v4 07/22] drm/xe: Introduce xe_tile_init_early and use at earlier point in probe Michał Winiarski
` (16 subsequent siblings)
22 siblings, 0 replies; 32+ messages in thread
From: Michał Winiarski @ 2023-11-29 1:16 UTC (permalink / raw)
To: intel-xe; +Cc: Matt Roper, Lucas De Marchi, Michał Winiarski
Parts of xe_info_init are only dealing with processing driver_data.
Extract it into xe_info_init_early to be able to use it earlier during
probe.
Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
Reviewed-by: Matthew Brost <matthew.brost@intel.com>
Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com>
---
drivers/gpu/drm/xe/tests/xe_pci.c | 1 +
drivers/gpu/drm/xe/xe_pci.c | 44 ++++++++++++++++++++++---------
2 files changed, 32 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/xe/tests/xe_pci.c b/drivers/gpu/drm/xe/tests/xe_pci.c
index a40879da2fbe1..733df9a270db2 100644
--- a/drivers/gpu/drm/xe/tests/xe_pci.c
+++ b/drivers/gpu/drm/xe/tests/xe_pci.c
@@ -143,6 +143,7 @@ int xe_pci_fake_device_init(struct xe_device *xe, enum xe_platform platform,
return -ENODEV;
done:
+ xe_info_init_early(xe, desc, subplatform_desc);
xe_info_init(xe, desc, subplatform_desc);
return 0;
diff --git a/drivers/gpu/drm/xe/xe_pci.c b/drivers/gpu/drm/xe/xe_pci.c
index cc5dfccfb8c08..abdd2ca4b6ac2 100644
--- a/drivers/gpu/drm/xe/xe_pci.c
+++ b/drivers/gpu/drm/xe/xe_pci.c
@@ -534,6 +534,35 @@ static void handle_gmdid(struct xe_device *xe,
}
}
+/*
+ * Initialize device info content that only depends on static driver_data passed to the driver at
+ * probe time from PCI ID table.
+ */
+static void xe_info_init_early(struct xe_device *xe,
+ const struct xe_device_desc *desc,
+ const struct xe_subplatform_desc *subplatform_desc)
+{
+ xe->info.platform = desc->platform;
+ xe->info.subplatform = subplatform_desc ?
+ subplatform_desc->subplatform : XE_SUBPLATFORM_NONE;
+
+ xe->info.is_dgfx = desc->is_dgfx;
+ xe->info.has_heci_gscfi = desc->has_heci_gscfi;
+ xe->info.has_llc = desc->has_llc;
+ xe->info.has_sriov = desc->has_sriov;
+ xe->info.bypass_mtcfg = desc->bypass_mtcfg;
+ xe->info.supports_mmio_ext = desc->supports_mmio_ext;
+
+ xe->info.enable_display = IS_ENABLED(CONFIG_DRM_XE_DISPLAY) &&
+ xe_modparam.enable_display &&
+ desc->has_display;
+}
+
+/*
+ * Initialize device info content that does require knowledge about graphics / media IP version.
+ * Make sure that GT / tile structures allocated by the driver match the data present in device
+ * info.
+ */
static int xe_info_init(struct xe_device *xe,
const struct xe_device_desc *desc,
const struct xe_subplatform_desc *subplatform_desc)
@@ -545,10 +574,6 @@ static int xe_info_init(struct xe_device *xe,
struct xe_gt *gt;
u8 id;
- xe->info.platform = desc->platform;
- xe->info.subplatform = subplatform_desc ?
- subplatform_desc->subplatform : XE_SUBPLATFORM_NONE;
-
/*
* If this platform supports GMD_ID, we'll detect the proper IP
* descriptor to use from hardware registers. desc->graphics will only
@@ -574,14 +599,8 @@ static int xe_info_init(struct xe_device *xe,
if (!graphics_desc)
return -ENODEV;
- xe->info.is_dgfx = desc->is_dgfx;
- xe->info.has_heci_gscfi = desc->has_heci_gscfi;
xe->info.graphics_name = graphics_desc->name;
xe->info.media_name = media_desc ? media_desc->name : "none";
- xe->info.has_llc = desc->has_llc;
- xe->info.has_sriov = desc->has_sriov;
- xe->info.bypass_mtcfg = desc->bypass_mtcfg;
- xe->info.supports_mmio_ext = desc->supports_mmio_ext;
xe->info.tile_mmio_ext_size = graphics_desc->tile_mmio_ext_size;
xe->info.dma_mask_size = graphics_desc->dma_mask_size;
@@ -593,9 +612,6 @@ static int xe_info_init(struct xe_device *xe,
xe->info.has_flat_ccs = graphics_desc->has_flat_ccs;
xe->info.has_range_tlb_invalidation = graphics_desc->has_range_tlb_invalidation;
- xe->info.enable_display = IS_ENABLED(CONFIG_DRM_XE_DISPLAY) &&
- xe_modparam.enable_display &&
- desc->has_display;
/*
* All platforms have at least one primary GT. Any platform with media
* version 13 or higher has an additional dedicated media GT. And
@@ -708,6 +724,8 @@ static int xe_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
pci_set_master(pdev);
+ xe_info_init_early(xe, desc, subplatform_desc);
+
xe_sriov_probe_early(xe, desc->has_sriov);
err = xe_info_init(xe, desc, subplatform_desc);
--
2.43.0
^ permalink raw reply related [flat|nested] 32+ messages in thread* [Intel-xe] [PATCH v4 07/22] drm/xe: Introduce xe_tile_init_early and use at earlier point in probe
2023-11-29 1:16 [Intel-xe] [PATCH v4 00/22] drm/xe: Probe tweaks and reordering Michał Winiarski
` (5 preceding siblings ...)
2023-11-29 1:16 ` [Intel-xe] [PATCH v4 06/22] drm/xe: Split xe_info_init Michał Winiarski
@ 2023-11-29 1:16 ` Michał Winiarski
2023-11-29 1:16 ` [Intel-xe] [PATCH v4 08/22] drm/xe: Map the entire BAR0 and hold onto the initial mapping Michał Winiarski
` (15 subsequent siblings)
22 siblings, 0 replies; 32+ messages in thread
From: Michał Winiarski @ 2023-11-29 1:16 UTC (permalink / raw)
To: intel-xe; +Cc: Matt Roper, Lucas De Marchi, Michał Winiarski
It also merges the GT (which is part of tile) initialization happening
at xe_info_init with allocating other per-tile data structures into a
common helper function.
Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
Reviewed-by: Matthew Brost <matthew.brost@intel.com>
---
drivers/gpu/drm/xe/xe_device.c | 6 ------
drivers/gpu/drm/xe/xe_pci.c | 10 +++++-----
drivers/gpu/drm/xe/xe_tile.c | 32 +++++++++++++++++++++++++++++++-
drivers/gpu/drm/xe/xe_tile.h | 2 +-
4 files changed, 37 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/xe/xe_device.c b/drivers/gpu/drm/xe/xe_device.c
index 785bf2e610b7c..5e1f73c8c77ad 100644
--- a/drivers/gpu/drm/xe/xe_device.c
+++ b/drivers/gpu/drm/xe/xe_device.c
@@ -393,12 +393,6 @@ int xe_device_probe(struct xe_device *xe)
if (err)
return err;
- for_each_tile(tile, xe, id) {
- err = xe_tile_alloc(tile);
- if (err)
- return err;
- }
-
err = xe_mmio_init(xe);
if (err)
return err;
diff --git a/drivers/gpu/drm/xe/xe_pci.c b/drivers/gpu/drm/xe/xe_pci.c
index abdd2ca4b6ac2..2483650720c54 100644
--- a/drivers/gpu/drm/xe/xe_pci.c
+++ b/drivers/gpu/drm/xe/xe_pci.c
@@ -26,6 +26,7 @@
#include "xe_pm.h"
#include "xe_sriov.h"
#include "xe_step.h"
+#include "xe_tile.h"
enum toggle_d3cold {
D3COLD_DISABLE,
@@ -624,12 +625,11 @@ static int xe_info_init(struct xe_device *xe,
xe->info.tile_count = 1 + graphics_desc->max_remote_tiles;
for_each_tile(tile, xe, id) {
- tile->xe = xe;
- tile->id = id;
+ int err;
- tile->primary_gt = xe_gt_alloc(tile);
- if (IS_ERR(tile->primary_gt))
- return PTR_ERR(tile->primary_gt);
+ err = xe_tile_init_early(tile, xe, id);
+ if (err)
+ return err;
gt = tile->primary_gt;
gt->info.id = xe->info.gt_count++;
diff --git a/drivers/gpu/drm/xe/xe_tile.c b/drivers/gpu/drm/xe/xe_tile.c
index 131752a57f652..daf1ac39dcbb1 100644
--- a/drivers/gpu/drm/xe/xe_tile.c
+++ b/drivers/gpu/drm/xe/xe_tile.c
@@ -7,6 +7,7 @@
#include "xe_device.h"
#include "xe_ggtt.h"
+#include "xe_gt.h"
#include "xe_migrate.h"
#include "xe_sa.h"
#include "xe_tile.h"
@@ -80,7 +81,7 @@
*
* Returns -ENOMEM if allocations fail, otherwise 0.
*/
-int xe_tile_alloc(struct xe_tile *tile)
+static int xe_tile_alloc(struct xe_tile *tile)
{
struct drm_device *drm = &tile_to_xe(tile)->drm;
@@ -97,6 +98,35 @@ int xe_tile_alloc(struct xe_tile *tile)
return 0;
}
+/**
+ * xe_tile_init_early - Initialize the tile and primary GT
+ * @tile: Tile to initialize
+ * @xe: Parent Xe device
+ * @id: Tile ID
+ *
+ * Initializes per-tile resources that don't require any interactions with the hardware or any
+ * knowledge about the Graphics/Media IP version.
+ *
+ * Returns: 0 on success, negative error code on error.
+ */
+int xe_tile_init_early(struct xe_tile *tile, struct xe_device *xe, u8 id)
+{
+ int err;
+
+ tile->xe = xe;
+ tile->id = id;
+
+ err = xe_tile_alloc(tile);
+ if (err)
+ return err;
+
+ tile->primary_gt = xe_gt_alloc(tile);
+ if (IS_ERR(tile->primary_gt))
+ return PTR_ERR(tile->primary_gt);
+
+ return 0;
+}
+
static int tile_ttm_mgr_init(struct xe_tile *tile)
{
struct xe_device *xe = tile_to_xe(tile);
diff --git a/drivers/gpu/drm/xe/xe_tile.h b/drivers/gpu/drm/xe/xe_tile.h
index 782c47f8bd450..1c9e42ade6b05 100644
--- a/drivers/gpu/drm/xe/xe_tile.h
+++ b/drivers/gpu/drm/xe/xe_tile.h
@@ -10,7 +10,7 @@
struct xe_tile;
-int xe_tile_alloc(struct xe_tile *tile);
+int xe_tile_init_early(struct xe_tile *tile, struct xe_device *xe, u8 id);
int xe_tile_init_noalloc(struct xe_tile *tile);
void xe_tile_migrate_wait(struct xe_tile *tile);
--
2.43.0
^ permalink raw reply related [flat|nested] 32+ messages in thread* [Intel-xe] [PATCH v4 08/22] drm/xe: Map the entire BAR0 and hold onto the initial mapping
2023-11-29 1:16 [Intel-xe] [PATCH v4 00/22] drm/xe: Probe tweaks and reordering Michał Winiarski
` (6 preceding siblings ...)
2023-11-29 1:16 ` [Intel-xe] [PATCH v4 07/22] drm/xe: Introduce xe_tile_init_early and use at earlier point in probe Michał Winiarski
@ 2023-11-29 1:16 ` Michał Winiarski
2023-11-29 1:16 ` [Intel-xe] [PATCH v4 09/22] drm/xe/device: Introduce xe_device_probe_early Michał Winiarski
` (14 subsequent siblings)
22 siblings, 0 replies; 32+ messages in thread
From: Michał Winiarski @ 2023-11-29 1:16 UTC (permalink / raw)
To: intel-xe; +Cc: Matt Roper, Lucas De Marchi, Michał Winiarski
Both MMIO registers and GGTT for root tile will need to be used earlier
during probe. Don't rely on tile count to compute the mapping size.
Further more, there's no need to remap after figuring out the real
resource size.
Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
Reviewed-by: Matthew Brost <matthew.brost@intel.com>
---
drivers/gpu/drm/xe/xe_mmio.c | 12 ++++--------
1 file changed, 4 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/xe/xe_mmio.c b/drivers/gpu/drm/xe/xe_mmio.c
index 9769d56988ece..d1b59021c9712 100644
--- a/drivers/gpu/drm/xe/xe_mmio.c
+++ b/drivers/gpu/drm/xe/xe_mmio.c
@@ -300,7 +300,6 @@ void xe_mmio_probe_tiles(struct xe_device *xe)
size_t tile_mmio_size = SZ_16M, tile_mmio_ext_size = xe->info.tile_mmio_ext_size;
u8 id, tile_count = xe->info.tile_count;
struct xe_gt *gt = xe_root_mmio_gt(xe);
- const int mmio_bar = 0;
struct xe_tile *tile;
void *regs;
u32 mtcfg;
@@ -314,9 +313,6 @@ void xe_mmio_probe_tiles(struct xe_device *xe)
if (tile_count < xe->info.tile_count) {
drm_info(&xe->drm, "tile_count: %d, reduced_tile_count %d\n",
xe->info.tile_count, tile_count);
- pci_iounmap(to_pci_dev(xe->drm.dev), xe->mmio.regs);
- xe->mmio.size = (tile_mmio_size + tile_mmio_ext_size) * tile_count;
- xe->mmio.regs = pci_iomap(to_pci_dev(xe->drm.dev), mmio_bar, xe->mmio.size);
xe->info.tile_count = tile_count;
/*
@@ -381,17 +377,17 @@ static int xe_verify_lmem_ready(struct xe_device *xe)
int xe_mmio_init(struct xe_device *xe)
{
struct xe_tile *root_tile = xe_device_get_root_tile(xe);
+ struct pci_dev *pdev = to_pci_dev(xe->drm.dev);
const int mmio_bar = 0;
int err;
/*
- * Map the maximum expected BAR size, which will get remapped later
- * if we determine that we're running on a reduced-tile system.
+ * Map the entire BAR.
* The first 16MB of the BAR, belong to the root tile, and include:
* registers (0-4MB), reserved space (4MB-8MB) and GGTT (8MB-16MB).
*/
- xe->mmio.size = (SZ_16M + xe->info.tile_mmio_ext_size) * xe->info.tile_count;
- xe->mmio.regs = pci_iomap(to_pci_dev(xe->drm.dev), mmio_bar, xe->mmio.size);
+ xe->mmio.size = pci_resource_len(pdev, mmio_bar);
+ xe->mmio.regs = pci_iomap(pdev, mmio_bar, 0);
if (xe->mmio.regs == NULL) {
drm_err(&xe->drm, "failed to map registers\n");
return -EIO;
--
2.43.0
^ permalink raw reply related [flat|nested] 32+ messages in thread* [Intel-xe] [PATCH v4 09/22] drm/xe/device: Introduce xe_device_probe_early
2023-11-29 1:16 [Intel-xe] [PATCH v4 00/22] drm/xe: Probe tweaks and reordering Michał Winiarski
` (7 preceding siblings ...)
2023-11-29 1:16 ` [Intel-xe] [PATCH v4 08/22] drm/xe: Map the entire BAR0 and hold onto the initial mapping Michał Winiarski
@ 2023-11-29 1:16 ` Michał Winiarski
2023-11-29 1:16 ` [Intel-xe] [PATCH v4 10/22] drm/xe: Don't "peek" into GMD_ID Michał Winiarski
` (13 subsequent siblings)
22 siblings, 0 replies; 32+ messages in thread
From: Michał Winiarski @ 2023-11-29 1:16 UTC (permalink / raw)
To: intel-xe; +Cc: Matt Roper, Lucas De Marchi, Michał Winiarski
SR-IOV VF doesn't have access to MMIO registers used to determine
graphics/media ID. It can however communicate with GuC.
Introduce xe_device_probe_early, which initializes enough HW to use
MMIO GuC communication.
This will allow both VF and PF/native driver to have unified probe
ordering.
Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
Reviewed-by: Matthew Brost <matthew.brost@intel.com>
---
v1 -> v2:
- Add comment to "early" variant (Lucas)
drivers/gpu/drm/xe/xe_device.c | 22 ++++++++++++++++++----
drivers/gpu/drm/xe/xe_device.h | 5 +++++
drivers/gpu/drm/xe/xe_mmio.c | 16 ++++++++++------
drivers/gpu/drm/xe/xe_mmio.h | 1 +
drivers/gpu/drm/xe/xe_pci.c | 26 +++++++++++++++++++++-----
5 files changed, 55 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/xe/xe_device.c b/drivers/gpu/drm/xe/xe_device.c
index 5e1f73c8c77ad..f4be4b13a506e 100644
--- a/drivers/gpu/drm/xe/xe_device.c
+++ b/drivers/gpu/drm/xe/xe_device.c
@@ -375,6 +375,24 @@ static int xe_set_dma_info(struct xe_device *xe)
return err;
}
+/*
+ * Initialize MMIO resources that don't require any knowledge about tile count.
+ */
+int xe_device_probe_early(struct xe_device *xe)
+{
+ int err;
+
+ err = xe_mmio_init(xe);
+ if (err)
+ return err;
+
+ err = xe_mmio_root_tile_init(xe);
+ if (err)
+ return err;
+
+ return 0;
+}
+
int xe_device_probe(struct xe_device *xe)
{
struct xe_tile *tile;
@@ -393,10 +411,6 @@ int xe_device_probe(struct xe_device *xe)
if (err)
return err;
- err = xe_mmio_init(xe);
- if (err)
- return err;
-
xe_mmio_probe_tiles(xe);
err = drmm_add_action_or_reset(&xe->drm, xe_driver_flr_fini, xe);
diff --git a/drivers/gpu/drm/xe/xe_device.h b/drivers/gpu/drm/xe/xe_device.h
index 54694f98c91a2..3da83b2332063 100644
--- a/drivers/gpu/drm/xe/xe_device.h
+++ b/drivers/gpu/drm/xe/xe_device.h
@@ -37,6 +37,7 @@ static inline struct xe_device *ttm_to_xe_device(struct ttm_device *ttm)
struct xe_device *xe_device_create(struct pci_dev *pdev,
const struct pci_device_id *ent);
+int xe_device_probe_early(struct xe_device *xe);
int xe_device_probe(struct xe_device *xe);
void xe_device_remove(struct xe_device *xe);
void xe_device_shutdown(struct xe_device *xe);
@@ -123,6 +124,10 @@ static inline bool xe_device_uc_enabled(struct xe_device *xe)
for ((id__) = 0; (id__) < (xe__)->info.tile_count; (id__)++) \
for_each_if((tile__) = &(xe__)->tiles[(id__)])
+#define for_each_remote_tile(tile__, xe__, id__) \
+ for ((id__) = 1; (id__) < (xe__)->info.tile_count; (id__)++) \
+ for_each_if((tile__) = &(xe__)->tiles[(id__)])
+
/*
* FIXME: This only works for now since multi-tile and standalone media
* happen to be mutually exclusive. Future platforms may change this...
diff --git a/drivers/gpu/drm/xe/xe_mmio.c b/drivers/gpu/drm/xe/xe_mmio.c
index d1b59021c9712..6148c9da4f64e 100644
--- a/drivers/gpu/drm/xe/xe_mmio.c
+++ b/drivers/gpu/drm/xe/xe_mmio.c
@@ -15,10 +15,12 @@
#include "regs/xe_regs.h"
#include "xe_bo.h"
#include "xe_device.h"
+#include "xe_ggtt.h"
#include "xe_gt.h"
#include "xe_gt_mcr.h"
#include "xe_macros.h"
#include "xe_module.h"
+#include "xe_tile.h"
#define XEHP_MTCFG_ADDR XE_REG(0x101800)
#define TILE_COUNT REG_GENMASK(15, 8)
@@ -376,10 +378,8 @@ static int xe_verify_lmem_ready(struct xe_device *xe)
int xe_mmio_init(struct xe_device *xe)
{
- struct xe_tile *root_tile = xe_device_get_root_tile(xe);
struct pci_dev *pdev = to_pci_dev(xe->drm.dev);
const int mmio_bar = 0;
- int err;
/*
* Map the entire BAR.
@@ -393,12 +393,16 @@ int xe_mmio_init(struct xe_device *xe)
return -EIO;
}
- err = drmm_add_action_or_reset(&xe->drm, mmio_fini, xe);
- if (err)
- return err;
+ return drmm_add_action_or_reset(&xe->drm, mmio_fini, xe);
+}
+
+int xe_mmio_root_tile_init(struct xe_device *xe)
+{
+ struct xe_tile *root_tile = xe_device_get_root_tile(xe);
+ int err;
/* Setup first tile; other tiles (if present) will be setup later. */
- root_tile->mmio.size = xe->mmio.size;
+ root_tile->mmio.size = SZ_16M;
root_tile->mmio.regs = xe->mmio.regs;
err = xe_verify_lmem_ready(xe);
diff --git a/drivers/gpu/drm/xe/xe_mmio.h b/drivers/gpu/drm/xe/xe_mmio.h
index c054c27f6925d..98de5c13c89ba 100644
--- a/drivers/gpu/drm/xe/xe_mmio.h
+++ b/drivers/gpu/drm/xe/xe_mmio.h
@@ -21,6 +21,7 @@ struct xe_device;
#define LMEM_BAR 2
int xe_mmio_init(struct xe_device *xe);
+int xe_mmio_root_tile_init(struct xe_device *xe);
void xe_mmio_probe_tiles(struct xe_device *xe);
static inline u8 xe_mmio_read8(struct xe_gt *gt, struct xe_reg reg)
diff --git a/drivers/gpu/drm/xe/xe_pci.c b/drivers/gpu/drm/xe/xe_pci.c
index 2483650720c54..1d5e50bb49236 100644
--- a/drivers/gpu/drm/xe/xe_pci.c
+++ b/drivers/gpu/drm/xe/xe_pci.c
@@ -539,10 +539,12 @@ static void handle_gmdid(struct xe_device *xe,
* Initialize device info content that only depends on static driver_data passed to the driver at
* probe time from PCI ID table.
*/
-static void xe_info_init_early(struct xe_device *xe,
- const struct xe_device_desc *desc,
- const struct xe_subplatform_desc *subplatform_desc)
+static int xe_info_init_early(struct xe_device *xe,
+ const struct xe_device_desc *desc,
+ const struct xe_subplatform_desc *subplatform_desc)
{
+ int err;
+
xe->info.platform = desc->platform;
xe->info.subplatform = subplatform_desc ?
subplatform_desc->subplatform : XE_SUBPLATFORM_NONE;
@@ -557,6 +559,12 @@ static void xe_info_init_early(struct xe_device *xe,
xe->info.enable_display = IS_ENABLED(CONFIG_DRM_XE_DISPLAY) &&
xe_modparam.enable_display &&
desc->has_display;
+
+ err = xe_tile_init_early(xe_device_get_root_tile(xe), xe, 0);
+ if (err)
+ return err;
+
+ return 0;
}
/*
@@ -624,13 +632,15 @@ static int xe_info_init(struct xe_device *xe,
*/
xe->info.tile_count = 1 + graphics_desc->max_remote_tiles;
- for_each_tile(tile, xe, id) {
+ for_each_remote_tile(tile, xe, id) {
int err;
err = xe_tile_init_early(tile, xe, id);
if (err)
return err;
+ }
+ for_each_tile(tile, xe, id) {
gt = tile->primary_gt;
gt->info.id = xe->info.gt_count++;
gt->info.type = XE_GT_TYPE_MAIN;
@@ -724,10 +734,16 @@ static int xe_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
pci_set_master(pdev);
- xe_info_init_early(xe, desc, subplatform_desc);
+ err = xe_info_init_early(xe, desc, subplatform_desc);
+ if (err)
+ return err;
xe_sriov_probe_early(xe, desc->has_sriov);
+ err = xe_device_probe_early(xe);
+ if (err)
+ return err;
+
err = xe_info_init(xe, desc, subplatform_desc);
if (err)
return err;
--
2.43.0
^ permalink raw reply related [flat|nested] 32+ messages in thread* [Intel-xe] [PATCH v4 10/22] drm/xe: Don't "peek" into GMD_ID
2023-11-29 1:16 [Intel-xe] [PATCH v4 00/22] drm/xe: Probe tweaks and reordering Michał Winiarski
` (8 preceding siblings ...)
2023-11-29 1:16 ` [Intel-xe] [PATCH v4 09/22] drm/xe/device: Introduce xe_device_probe_early Michał Winiarski
@ 2023-11-29 1:16 ` Michał Winiarski
2023-11-29 1:16 ` [Intel-xe] [PATCH v4 11/22] drm/xe: Move system memory management init to earlier point in probe Michał Winiarski
` (12 subsequent siblings)
22 siblings, 0 replies; 32+ messages in thread
From: Michał Winiarski @ 2023-11-29 1:16 UTC (permalink / raw)
To: intel-xe; +Cc: Matt Roper, Lucas De Marchi, Michał Winiarski
Now that MMIO init got moved to device early, we can use regular
xe_mmio_read helpers to get to GMD_ID register.
Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
Reviewed-by: Matthew Brost <matthew.brost@intel.com>
Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com>
---
v3 -> v4:
- Don't define MEDIA_GMD_ID (Matt Roper)
- Keep the "peek_gmdid" for KUnit (Lucas)
drivers/gpu/drm/xe/xe_pci.c | 34 ++++++++++++++++------------------
1 file changed, 16 insertions(+), 18 deletions(-)
diff --git a/drivers/gpu/drm/xe/xe_pci.c b/drivers/gpu/drm/xe/xe_pci.c
index 1d5e50bb49236..be24ea8ac1928 100644
--- a/drivers/gpu/drm/xe/xe_pci.c
+++ b/drivers/gpu/drm/xe/xe_pci.c
@@ -21,6 +21,7 @@
#include "xe_drv.h"
#include "xe_gt.h"
#include "xe_macros.h"
+#include "xe_mmio.h"
#include "xe_module.h"
#include "xe_pci_types.h"
#include "xe_pm.h"
@@ -443,26 +444,22 @@ find_subplatform(const struct xe_device *xe, const struct xe_device_desc *desc)
return NULL;
}
-static void peek_gmdid(struct xe_device *xe, u32 gmdid_offset, u32 *ver, u32 *revid)
+enum xe_gmdid_type {
+ GMDID_GRAPHICS,
+ GMDID_MEDIA
+};
+
+static void read_gmdid(struct xe_device *xe, enum xe_gmdid_type type, u32 *ver, u32 *revid)
{
- struct pci_dev *pdev = to_pci_dev(xe->drm.dev);
- void __iomem *map = pci_iomap_range(pdev, 0, gmdid_offset, sizeof(u32));
+ struct xe_gt *gt = xe_root_mmio_gt(xe);
+ struct xe_reg gmdid_reg = GMD_ID;
u32 val;
- if (!map) {
- drm_err(&xe->drm, "Failed to read GMD_ID (%#x) from PCI BAR.\n",
- gmdid_offset);
- *ver = 0;
- *revid = 0;
+ if (type == GMDID_MEDIA)
+ gmdid_reg.addr += MEDIA_GT_GSI_OFFSET;
- return;
- }
-
- val = ioread32(map);
- pci_iounmap(pdev, map);
-
- *ver = REG_FIELD_GET(GMD_ID_ARCH_MASK, val) * 100 +
- REG_FIELD_GET(GMD_ID_RELEASE_MASK, val);
+ val = xe_mmio_read32(gt, gmdid_reg);
+ *ver = REG_FIELD_GET(GMD_ID_ARCH_MASK, val) * 100 + REG_FIELD_GET(GMD_ID_RELEASE_MASK, val);
*revid = REG_FIELD_GET(GMD_ID_REVID, val);
}
@@ -499,7 +496,8 @@ static void handle_gmdid(struct xe_device *xe,
{
u32 ver;
- peek_gmdid(xe, GMD_ID.addr, &ver, graphics_revid);
+ read_gmdid(xe, GMDID_GRAPHICS, &ver, graphics_revid);
+
for (int i = 0; i < ARRAY_SIZE(graphics_ip_map); i++) {
if (ver == graphics_ip_map[i].ver) {
xe->info.graphics_verx100 = ver;
@@ -514,7 +512,7 @@ static void handle_gmdid(struct xe_device *xe,
ver / 100, ver % 100);
}
- peek_gmdid(xe, GMD_ID.addr + 0x380000, &ver, media_revid);
+ read_gmdid(xe, GMDID_MEDIA, &ver, media_revid);
/* Media may legitimately be fused off / not present */
if (ver == 0)
--
2.43.0
^ permalink raw reply related [flat|nested] 32+ messages in thread* [Intel-xe] [PATCH v4 11/22] drm/xe: Move system memory management init to earlier point in probe
2023-11-29 1:16 [Intel-xe] [PATCH v4 00/22] drm/xe: Probe tweaks and reordering Michał Winiarski
` (9 preceding siblings ...)
2023-11-29 1:16 ` [Intel-xe] [PATCH v4 10/22] drm/xe: Don't "peek" into GMD_ID Michał Winiarski
@ 2023-11-29 1:16 ` Michał Winiarski
2023-11-29 1:16 ` [Intel-xe] [PATCH v4 12/22] drm/xe: Move force_wake " Michał Winiarski
` (11 subsequent siblings)
22 siblings, 0 replies; 32+ messages in thread
From: Michał Winiarski @ 2023-11-29 1:16 UTC (permalink / raw)
To: intel-xe; +Cc: Matt Roper, Lucas De Marchi, Michał Winiarski
GuC will need to be loaded earlier during probe. And in order to load
GuC, we will need the ability to create system memory allocations.
Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
---
drivers/gpu/drm/xe/xe_device.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/xe/xe_device.c b/drivers/gpu/drm/xe/xe_device.c
index f4be4b13a506e..dd56a8c3f80d4 100644
--- a/drivers/gpu/drm/xe/xe_device.c
+++ b/drivers/gpu/drm/xe/xe_device.c
@@ -413,6 +413,8 @@ int xe_device_probe(struct xe_device *xe)
xe_mmio_probe_tiles(xe);
+ xe_ttm_sys_mgr_init(xe);
+
err = drmm_add_action_or_reset(&xe->drm, xe_driver_flr_fini, xe);
if (err)
return err;
@@ -441,8 +443,6 @@ int xe_device_probe(struct xe_device *xe)
if (err)
goto err_irq_shutdown;
- xe_ttm_sys_mgr_init(xe);
-
for_each_tile(tile, xe, id) {
err = xe_tile_init_noalloc(tile);
if (err)
--
2.43.0
^ permalink raw reply related [flat|nested] 32+ messages in thread* [Intel-xe] [PATCH v4 12/22] drm/xe: Move force_wake init to earlier point in probe
2023-11-29 1:16 [Intel-xe] [PATCH v4 00/22] drm/xe: Probe tweaks and reordering Michał Winiarski
` (10 preceding siblings ...)
2023-11-29 1:16 ` [Intel-xe] [PATCH v4 11/22] drm/xe: Move system memory management init to earlier point in probe Michał Winiarski
@ 2023-11-29 1:16 ` Michał Winiarski
2023-11-29 1:16 ` [Intel-xe] [PATCH v4 13/22] drm/xe: Reorder GGTT " Michał Winiarski
` (10 subsequent siblings)
22 siblings, 0 replies; 32+ messages in thread
From: Michał Winiarski @ 2023-11-29 1:16 UTC (permalink / raw)
To: intel-xe; +Cc: Matt Roper, Lucas De Marchi, Michał Winiarski
GuC will need to be loaded earlier during probe. And in order to load
GuC, being able to take the forcewake is going to be needed.
Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
---
drivers/gpu/drm/xe/xe_device.c | 3 +++
drivers/gpu/drm/xe/xe_gt.c | 2 --
2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/xe/xe_device.c b/drivers/gpu/drm/xe/xe_device.c
index dd56a8c3f80d4..bcc10b7f23ab7 100644
--- a/drivers/gpu/drm/xe/xe_device.c
+++ b/drivers/gpu/drm/xe/xe_device.c
@@ -415,6 +415,9 @@ int xe_device_probe(struct xe_device *xe)
xe_ttm_sys_mgr_init(xe);
+ for_each_gt(gt, xe, id)
+ xe_force_wake_init_gt(gt, gt_to_fw(gt));
+
err = drmm_add_action_or_reset(&xe->drm, xe_driver_flr_fini, xe);
if (err)
return err;
diff --git a/drivers/gpu/drm/xe/xe_gt.c b/drivers/gpu/drm/xe/xe_gt.c
index 0dddb751c6a48..7f26899bbb1b6 100644
--- a/drivers/gpu/drm/xe/xe_gt.c
+++ b/drivers/gpu/drm/xe/xe_gt.c
@@ -292,8 +292,6 @@ int xe_gt_init_early(struct xe_gt *gt)
{
int err;
- xe_force_wake_init_gt(gt, gt_to_fw(gt));
-
err = xe_force_wake_get(gt_to_fw(gt), XE_FW_GT);
if (err)
return err;
--
2.43.0
^ permalink raw reply related [flat|nested] 32+ messages in thread* [Intel-xe] [PATCH v4 13/22] drm/xe: Reorder GGTT init to earlier point in probe
2023-11-29 1:16 [Intel-xe] [PATCH v4 00/22] drm/xe: Probe tweaks and reordering Michał Winiarski
` (11 preceding siblings ...)
2023-11-29 1:16 ` [Intel-xe] [PATCH v4 12/22] drm/xe: Move force_wake " Michał Winiarski
@ 2023-11-29 1:16 ` Michał Winiarski
2023-11-29 1:16 ` [Intel-xe] [PATCH v4 14/22] drm/xe: Add a helper for DRM device-lifetime BO create Michał Winiarski
` (9 subsequent siblings)
22 siblings, 0 replies; 32+ messages in thread
From: Michał Winiarski @ 2023-11-29 1:16 UTC (permalink / raw)
To: intel-xe; +Cc: Matt Roper, Lucas De Marchi, Michał Winiarski
GuC will need to be loaded earlier during probe. Having functional GGTT
is one of the prerequisites.
Also rename xe_ggtt_init_noalloc to xe_ggtt_init_early to match the new
call site.
Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
---
drivers/gpu/drm/xe/xe_device.c | 7 +++++++
drivers/gpu/drm/xe/xe_ggtt.c | 20 ++++++++++++++++----
drivers/gpu/drm/xe/xe_ggtt.h | 2 +-
drivers/gpu/drm/xe/xe_tile.c | 4 ----
4 files changed, 24 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/xe/xe_device.c b/drivers/gpu/drm/xe/xe_device.c
index bcc10b7f23ab7..65e9aa5e6c31e 100644
--- a/drivers/gpu/drm/xe/xe_device.c
+++ b/drivers/gpu/drm/xe/xe_device.c
@@ -24,6 +24,7 @@
#include "xe_drv.h"
#include "xe_exec_queue.h"
#include "xe_exec.h"
+#include "xe_ggtt.h"
#include "xe_gt.h"
#include "xe_irq.h"
#include "xe_mmio.h"
@@ -418,6 +419,12 @@ int xe_device_probe(struct xe_device *xe)
for_each_gt(gt, xe, id)
xe_force_wake_init_gt(gt, gt_to_fw(gt));
+ for_each_tile(tile, xe, id) {
+ err = xe_ggtt_init_early(tile->mem.ggtt);
+ if (err)
+ return err;
+ }
+
err = drmm_add_action_or_reset(&xe->drm, xe_driver_flr_fini, xe);
if (err)
return err;
diff --git a/drivers/gpu/drm/xe/xe_ggtt.c b/drivers/gpu/drm/xe/xe_ggtt.c
index 0e2a41837f169..fab3cc04882da 100644
--- a/drivers/gpu/drm/xe/xe_ggtt.c
+++ b/drivers/gpu/drm/xe/xe_ggtt.c
@@ -96,14 +96,20 @@ static void xe_ggtt_clear(struct xe_ggtt *ggtt, u64 start, u64 size)
}
}
-static void ggtt_fini_noalloc(struct drm_device *drm, void *arg)
+static void ggtt_fini_early(struct drm_device *drm, void *arg)
{
struct xe_ggtt *ggtt = arg;
mutex_destroy(&ggtt->lock);
drm_mm_takedown(&ggtt->mm);
+}
+
+static void ggtt_fini(struct drm_device *drm, void *arg)
+{
+ struct xe_ggtt *ggtt = arg;
xe_bo_unpin_map_no_vm(ggtt->scratch);
+ ggtt->scratch = NULL;
}
static void primelockdep(struct xe_ggtt *ggtt)
@@ -124,7 +130,12 @@ static const struct xe_ggtt_pt_ops xelpg_pt_ops = {
.pte_encode_bo = xelpg_ggtt_pte_encode_bo,
};
-int xe_ggtt_init_noalloc(struct xe_ggtt *ggtt)
+/*
+ * Early GGTT initialization, which allows to create new mappings usable by the GuC.
+ * Mappings are not usable by the HW engines, as it doesn't have scratch / initial clear done to it
+ * yet. That will happen in the regular, non-early GGTT init.
+ */
+int xe_ggtt_init_early(struct xe_ggtt *ggtt)
{
struct xe_device *xe = tile_to_xe(ggtt->tile);
struct pci_dev *pdev = to_pci_dev(xe->drm.dev);
@@ -178,7 +189,7 @@ int xe_ggtt_init_noalloc(struct xe_ggtt *ggtt)
mutex_init(&ggtt->lock);
primelockdep(ggtt);
- return drmm_add_action_or_reset(&xe->drm, ggtt_fini_noalloc, ggtt);
+ return drmm_add_action_or_reset(&xe->drm, ggtt_fini_early, ggtt);
}
static void xe_ggtt_initial_clear(struct xe_ggtt *ggtt)
@@ -226,7 +237,8 @@ int xe_ggtt_init(struct xe_ggtt *ggtt)
xe_map_memset(xe, &ggtt->scratch->vmap, 0, 0, ggtt->scratch->size);
xe_ggtt_initial_clear(ggtt);
- return 0;
+
+ return drmm_add_action_or_reset(&xe->drm, ggtt_fini, ggtt);
err:
ggtt->scratch = NULL;
return err;
diff --git a/drivers/gpu/drm/xe/xe_ggtt.h b/drivers/gpu/drm/xe/xe_ggtt.h
index 3faa3c6d0375f..a09c166dff701 100644
--- a/drivers/gpu/drm/xe/xe_ggtt.h
+++ b/drivers/gpu/drm/xe/xe_ggtt.h
@@ -12,7 +12,7 @@ struct drm_printer;
void xe_ggtt_set_pte(struct xe_ggtt *ggtt, u64 addr, u64 pte);
void xe_ggtt_invalidate(struct xe_ggtt *ggtt);
-int xe_ggtt_init_noalloc(struct xe_ggtt *ggtt);
+int xe_ggtt_init_early(struct xe_ggtt *ggtt);
int xe_ggtt_init(struct xe_ggtt *ggtt);
void xe_ggtt_printk(struct xe_ggtt *ggtt, const char *prefix);
diff --git a/drivers/gpu/drm/xe/xe_tile.c b/drivers/gpu/drm/xe/xe_tile.c
index daf1ac39dcbb1..731869e49b016 100644
--- a/drivers/gpu/drm/xe/xe_tile.c
+++ b/drivers/gpu/drm/xe/xe_tile.c
@@ -166,10 +166,6 @@ int xe_tile_init_noalloc(struct xe_tile *tile)
if (err)
goto err_mem_access;
- err = xe_ggtt_init_noalloc(tile->mem.ggtt);
- if (err)
- goto err_mem_access;
-
tile->mem.kernel_bb_pool = xe_sa_bo_manager_init(tile, SZ_1M, 16);
if (IS_ERR(tile->mem.kernel_bb_pool))
err = PTR_ERR(tile->mem.kernel_bb_pool);
--
2.43.0
^ permalink raw reply related [flat|nested] 32+ messages in thread* [Intel-xe] [PATCH v4 14/22] drm/xe: Add a helper for DRM device-lifetime BO create
2023-11-29 1:16 [Intel-xe] [PATCH v4 00/22] drm/xe: Probe tweaks and reordering Michał Winiarski
` (12 preceding siblings ...)
2023-11-29 1:16 ` [Intel-xe] [PATCH v4 13/22] drm/xe: Reorder GGTT " Michał Winiarski
@ 2023-11-29 1:16 ` Michał Winiarski
2023-11-29 9:38 ` Michal Wajdeczko
2023-11-29 1:16 ` [Intel-xe] [PATCH v4 15/22] drm/xe/uc: Split xe_uc_fw_init Michał Winiarski
` (8 subsequent siblings)
22 siblings, 1 reply; 32+ messages in thread
From: Michał Winiarski @ 2023-11-29 1:16 UTC (permalink / raw)
To: intel-xe; +Cc: Matt Roper, Lucas De Marchi, Michał Winiarski
A helper for managed BO allocations makes it possible to remove specific
"fini" actions and will simplify the following patches adding ability to
execute a release action for specific BO directly.
Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
---
drivers/gpu/drm/xe/xe_bo.c | 33 ++++++++++++++++++++++++++++
drivers/gpu/drm/xe/xe_bo.h | 4 ++++
drivers/gpu/drm/xe/xe_ggtt.c | 6 +----
drivers/gpu/drm/xe/xe_guc_ads.c | 20 +++--------------
drivers/gpu/drm/xe/xe_guc_ct.c | 8 +++----
drivers/gpu/drm/xe/xe_guc_hwconfig.c | 18 +++------------
drivers/gpu/drm/xe/xe_guc_log.c | 19 +++-------------
drivers/gpu/drm/xe/xe_guc_pc.c | 9 +++-----
drivers/gpu/drm/xe/xe_hw_engine.c | 8 +++----
drivers/gpu/drm/xe/xe_uc_fw.c | 9 ++++----
10 files changed, 60 insertions(+), 74 deletions(-)
diff --git a/drivers/gpu/drm/xe/xe_bo.c b/drivers/gpu/drm/xe/xe_bo.c
index c5aa01c0861c7..7f5b616a7bbeb 100644
--- a/drivers/gpu/drm/xe/xe_bo.c
+++ b/drivers/gpu/drm/xe/xe_bo.c
@@ -9,6 +9,7 @@
#include <drm/drm_drv.h>
#include <drm/drm_gem_ttm_helper.h>
+#include <drm/drm_managed.h>
#include <drm/ttm/ttm_device.h>
#include <drm/ttm/ttm_placement.h>
#include <drm/ttm/ttm_tt.h>
@@ -1493,6 +1494,38 @@ struct xe_bo *xe_bo_create_from_data(struct xe_device *xe, struct xe_tile *tile,
return bo;
}
+static void __xe_bo_release(struct drm_device *drm, void *arg)
+{
+ xe_bo_unpin_map_no_vm(arg);
+}
+
+struct xe_bo *xe_managed_bo_create_pin_map(struct xe_device *xe, struct xe_tile *tile,
+ size_t size, u32 flags)
+{
+ struct xe_bo *bo;
+ int ret;
+
+ bo = xe_bo_create_pin_map(xe, tile, NULL, size, ttm_bo_type_kernel, flags);
+ if (IS_ERR(bo))
+ return bo;
+
+ ret = drmm_add_action_or_reset(&xe->drm, __xe_bo_release, bo);
+ if (ret)
+ return ERR_PTR(ret);
+
+ return bo;
+}
+
+struct xe_bo *xe_managed_bo_create_from_data(struct xe_device *xe, struct xe_tile *tile,
+ const void *data, size_t size, u32 flags)
+{
+ struct xe_bo *bo = xe_managed_bo_create_pin_map(xe, tile, size, flags);
+
+ xe_map_memcpy_to(xe, &bo->vmap, 0, data, size);
+
+ return bo;
+}
+
/*
* XXX: This is in the VM bind data path, likely should calculate this once and
* store, with a recalculation if the BO is moved.
diff --git a/drivers/gpu/drm/xe/xe_bo.h b/drivers/gpu/drm/xe/xe_bo.h
index 9f373f6d25f22..2cf32713bde9e 100644
--- a/drivers/gpu/drm/xe/xe_bo.h
+++ b/drivers/gpu/drm/xe/xe_bo.h
@@ -109,6 +109,10 @@ struct xe_bo *xe_bo_create_pin_map_at(struct xe_device *xe, struct xe_tile *tile
struct xe_bo *xe_bo_create_from_data(struct xe_device *xe, struct xe_tile *tile,
const void *data, size_t size,
enum ttm_bo_type type, u32 flags);
+struct xe_bo *xe_managed_bo_create_pin_map(struct xe_device *xe, struct xe_tile *tile,
+ size_t size, u32 flags);
+struct xe_bo *xe_managed_bo_create_from_data(struct xe_device *xe, struct xe_tile *tile,
+ const void *data, size_t size, u32 flags);
int xe_bo_placement_for_flags(struct xe_device *xe, struct xe_bo *bo,
u32 bo_flags);
diff --git a/drivers/gpu/drm/xe/xe_ggtt.c b/drivers/gpu/drm/xe/xe_ggtt.c
index fab3cc04882da..8dc783e9120d2 100644
--- a/drivers/gpu/drm/xe/xe_ggtt.c
+++ b/drivers/gpu/drm/xe/xe_ggtt.c
@@ -108,7 +108,6 @@ static void ggtt_fini(struct drm_device *drm, void *arg)
{
struct xe_ggtt *ggtt = arg;
- xe_bo_unpin_map_no_vm(ggtt->scratch);
ggtt->scratch = NULL;
}
@@ -225,10 +224,7 @@ int xe_ggtt_init(struct xe_ggtt *ggtt)
else
flags |= XE_BO_CREATE_VRAM_IF_DGFX(ggtt->tile);
- ggtt->scratch = xe_bo_create_pin_map(xe, ggtt->tile, NULL, XE_PAGE_SIZE,
- ttm_bo_type_kernel,
- flags);
-
+ ggtt->scratch = xe_managed_bo_create_pin_map(xe, ggtt->tile, XE_PAGE_SIZE, flags);
if (IS_ERR(ggtt->scratch)) {
err = PTR_ERR(ggtt->scratch);
goto err;
diff --git a/drivers/gpu/drm/xe/xe_guc_ads.c b/drivers/gpu/drm/xe/xe_guc_ads.c
index 88789826e7817..2f5ff090aa6bd 100644
--- a/drivers/gpu/drm/xe/xe_guc_ads.c
+++ b/drivers/gpu/drm/xe/xe_guc_ads.c
@@ -202,13 +202,6 @@ static size_t guc_ads_size(struct xe_guc_ads *ads)
guc_ads_private_data_size(ads);
}
-static void guc_ads_fini(struct drm_device *drm, void *arg)
-{
- struct xe_guc_ads *ads = arg;
-
- xe_bo_unpin_map_no_vm(ads->bo);
-}
-
static bool needs_wa_1607983814(struct xe_device *xe)
{
return GRAPHICS_VERx100(xe) < 1250;
@@ -274,25 +267,18 @@ int xe_guc_ads_init(struct xe_guc_ads *ads)
struct xe_gt *gt = ads_to_gt(ads);
struct xe_tile *tile = gt_to_tile(gt);
struct xe_bo *bo;
- int err;
ads->golden_lrc_size = calculate_golden_lrc_size(ads);
ads->regset_size = calculate_regset_size(gt);
- bo = xe_bo_create_pin_map(xe, tile, NULL, guc_ads_size(ads) +
- MAX_GOLDEN_LRC_SIZE,
- ttm_bo_type_kernel,
- XE_BO_CREATE_VRAM_IF_DGFX(tile) |
- XE_BO_CREATE_GGTT_BIT);
+ bo = xe_managed_bo_create_pin_map(xe, tile, guc_ads_size(ads) + MAX_GOLDEN_LRC_SIZE,
+ XE_BO_CREATE_VRAM_IF_DGFX(tile) |
+ XE_BO_CREATE_GGTT_BIT);
if (IS_ERR(bo))
return PTR_ERR(bo);
ads->bo = bo;
- err = drmm_add_action_or_reset(&xe->drm, guc_ads_fini, ads);
- if (err)
- return err;
-
return 0;
}
diff --git a/drivers/gpu/drm/xe/xe_guc_ct.c b/drivers/gpu/drm/xe/xe_guc_ct.c
index c44e750746958..93c886ef6fdb4 100644
--- a/drivers/gpu/drm/xe/xe_guc_ct.c
+++ b/drivers/gpu/drm/xe/xe_guc_ct.c
@@ -112,7 +112,6 @@ static void guc_ct_fini(struct drm_device *drm, void *arg)
struct xe_guc_ct *ct = arg;
xa_destroy(&ct->fence_lookup);
- xe_bo_unpin_map_no_vm(ct->bo);
}
static void g2h_worker_func(struct work_struct *w);
@@ -146,10 +145,9 @@ int xe_guc_ct_init(struct xe_guc_ct *ct)
primelockdep(ct);
- bo = xe_bo_create_pin_map(xe, tile, NULL, guc_ct_size(),
- ttm_bo_type_kernel,
- XE_BO_CREATE_VRAM_IF_DGFX(tile) |
- XE_BO_CREATE_GGTT_BIT);
+ bo = xe_managed_bo_create_pin_map(xe, tile, guc_ct_size(),
+ XE_BO_CREATE_VRAM_IF_DGFX(tile) |
+ XE_BO_CREATE_GGTT_BIT);
if (IS_ERR(bo))
return PTR_ERR(bo);
diff --git a/drivers/gpu/drm/xe/xe_guc_hwconfig.c b/drivers/gpu/drm/xe/xe_guc_hwconfig.c
index 57d325ec8ce32..8e3db5d7192c3 100644
--- a/drivers/gpu/drm/xe/xe_guc_hwconfig.c
+++ b/drivers/gpu/drm/xe/xe_guc_hwconfig.c
@@ -47,13 +47,6 @@ static int guc_hwconfig_copy(struct xe_guc *guc)
return 0;
}
-static void guc_hwconfig_fini(struct drm_device *drm, void *arg)
-{
- struct xe_guc *guc = arg;
-
- xe_bo_unpin_map_no_vm(guc->hwconfig.bo);
-}
-
int xe_guc_hwconfig_init(struct xe_guc *guc)
{
struct xe_device *xe = guc_to_xe(guc);
@@ -83,19 +76,14 @@ int xe_guc_hwconfig_init(struct xe_guc *guc)
if (!size)
return -EINVAL;
- bo = xe_bo_create_pin_map(xe, tile, NULL, PAGE_ALIGN(size),
- ttm_bo_type_kernel,
- XE_BO_CREATE_VRAM_IF_DGFX(tile) |
- XE_BO_CREATE_GGTT_BIT);
+ bo = xe_managed_bo_create_pin_map(xe, tile, PAGE_ALIGN(size),
+ XE_BO_CREATE_VRAM_IF_DGFX(tile) |
+ XE_BO_CREATE_GGTT_BIT);
if (IS_ERR(bo))
return PTR_ERR(bo);
guc->hwconfig.bo = bo;
guc->hwconfig.size = size;
- err = drmm_add_action_or_reset(&xe->drm, guc_hwconfig_fini, guc);
- if (err)
- return err;
-
return guc_hwconfig_copy(guc);
}
diff --git a/drivers/gpu/drm/xe/xe_guc_log.c b/drivers/gpu/drm/xe/xe_guc_log.c
index 27c3827bfd054..bcd2f4d34081d 100644
--- a/drivers/gpu/drm/xe/xe_guc_log.c
+++ b/drivers/gpu/drm/xe/xe_guc_log.c
@@ -77,24 +77,15 @@ void xe_guc_log_print(struct xe_guc_log *log, struct drm_printer *p)
}
}
-static void guc_log_fini(struct drm_device *drm, void *arg)
-{
- struct xe_guc_log *log = arg;
-
- xe_bo_unpin_map_no_vm(log->bo);
-}
-
int xe_guc_log_init(struct xe_guc_log *log)
{
struct xe_device *xe = log_to_xe(log);
struct xe_tile *tile = gt_to_tile(log_to_gt(log));
struct xe_bo *bo;
- int err;
- bo = xe_bo_create_pin_map(xe, tile, NULL, guc_log_size(),
- ttm_bo_type_kernel,
- XE_BO_CREATE_VRAM_IF_DGFX(tile) |
- XE_BO_CREATE_GGTT_BIT);
+ bo = xe_managed_bo_create_pin_map(xe, tile, guc_log_size(),
+ XE_BO_CREATE_VRAM_IF_DGFX(tile) |
+ XE_BO_CREATE_GGTT_BIT);
if (IS_ERR(bo))
return PTR_ERR(bo);
@@ -102,9 +93,5 @@ int xe_guc_log_init(struct xe_guc_log *log)
log->bo = bo;
log->level = xe_modparam.guc_log_level;
- err = drmm_add_action_or_reset(&xe->drm, guc_log_fini, log);
- if (err)
- return err;
-
return 0;
}
diff --git a/drivers/gpu/drm/xe/xe_guc_pc.c b/drivers/gpu/drm/xe/xe_guc_pc.c
index e9dd6c3d750bd..f5009a7968afa 100644
--- a/drivers/gpu/drm/xe/xe_guc_pc.c
+++ b/drivers/gpu/drm/xe/xe_guc_pc.c
@@ -936,7 +936,6 @@ void xe_guc_pc_fini(struct xe_guc_pc *pc)
XE_WARN_ON(xe_guc_pc_gucrc_disable(pc));
XE_WARN_ON(xe_guc_pc_stop(pc));
sysfs_remove_files(pc_to_gt(pc)->sysfs, pc_attrs);
- xe_bo_unpin_map_no_vm(pc->bo);
mutex_destroy(&pc->freq_lock);
}
@@ -955,11 +954,9 @@ int xe_guc_pc_init(struct xe_guc_pc *pc)
mutex_init(&pc->freq_lock);
- bo = xe_bo_create_pin_map(xe, tile, NULL, size,
- ttm_bo_type_kernel,
- XE_BO_CREATE_VRAM_IF_DGFX(tile) |
- XE_BO_CREATE_GGTT_BIT);
-
+ bo = xe_managed_bo_create_pin_map(xe, tile, size,
+ XE_BO_CREATE_VRAM_IF_DGFX(tile) |
+ XE_BO_CREATE_GGTT_BIT);
if (IS_ERR(bo))
return PTR_ERR(bo);
diff --git a/drivers/gpu/drm/xe/xe_hw_engine.c b/drivers/gpu/drm/xe/xe_hw_engine.c
index e831e63c5e485..34990f396a1a8 100644
--- a/drivers/gpu/drm/xe/xe_hw_engine.c
+++ b/drivers/gpu/drm/xe/xe_hw_engine.c
@@ -238,8 +238,6 @@ static void hw_engine_fini(struct drm_device *drm, void *arg)
xe_execlist_port_destroy(hwe->exl_port);
xe_lrc_finish(&hwe->kernel_lrc);
- xe_bo_unpin_map_no_vm(hwe->hwsp);
-
hwe->gt = NULL;
}
@@ -427,9 +425,9 @@ static int hw_engine_init(struct xe_gt *gt, struct xe_hw_engine *hwe,
xe_reg_sr_apply_mmio(&hwe->reg_sr, gt);
xe_reg_sr_apply_whitelist(hwe);
- hwe->hwsp = xe_bo_create_pin_map(xe, tile, NULL, SZ_4K, ttm_bo_type_kernel,
- XE_BO_CREATE_VRAM_IF_DGFX(tile) |
- XE_BO_CREATE_GGTT_BIT);
+ hwe->hwsp = xe_managed_bo_create_pin_map(xe, tile, SZ_4K,
+ XE_BO_CREATE_VRAM_IF_DGFX(tile) |
+ XE_BO_CREATE_GGTT_BIT);
if (IS_ERR(hwe->hwsp)) {
err = PTR_ERR(hwe->hwsp);
goto err_name;
diff --git a/drivers/gpu/drm/xe/xe_uc_fw.c b/drivers/gpu/drm/xe/xe_uc_fw.c
index 25778887d846f..5b86c51941335 100644
--- a/drivers/gpu/drm/xe/xe_uc_fw.c
+++ b/drivers/gpu/drm/xe/xe_uc_fw.c
@@ -266,7 +266,6 @@ static void uc_fw_fini(struct drm_device *drm, void *arg)
if (!xe_uc_fw_is_available(uc_fw))
return;
- xe_bo_unpin_map_no_vm(uc_fw->bo);
xe_uc_fw_change_status(uc_fw, XE_UC_FIRMWARE_SELECTED);
}
@@ -575,10 +574,9 @@ int xe_uc_fw_init(struct xe_uc_fw *uc_fw)
if (err)
goto fail;
- obj = xe_bo_create_from_data(xe, tile, fw->data, fw->size,
- ttm_bo_type_kernel,
- XE_BO_CREATE_VRAM_IF_DGFX(tile) |
- XE_BO_CREATE_GGTT_BIT);
+ obj = xe_managed_bo_create_from_data(xe, tile, fw->data, fw->size,
+ XE_BO_CREATE_VRAM_IF_DGFX(tile) |
+ XE_BO_CREATE_GGTT_BIT);
if (IS_ERR(obj)) {
drm_notice(&xe->drm, "%s firmware %s: failed to create / populate bo",
xe_uc_fw_type_repr(uc_fw->type), uc_fw->path);
@@ -609,6 +607,7 @@ int xe_uc_fw_init(struct xe_uc_fw *uc_fw)
xe_uc_fw_type_repr(uc_fw->type), XE_UC_FIRMWARE_URL);
release_firmware(fw); /* OK even if fw is NULL */
+
return err;
}
--
2.43.0
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [Intel-xe] [PATCH v4 14/22] drm/xe: Add a helper for DRM device-lifetime BO create
2023-11-29 1:16 ` [Intel-xe] [PATCH v4 14/22] drm/xe: Add a helper for DRM device-lifetime BO create Michał Winiarski
@ 2023-11-29 9:38 ` Michal Wajdeczko
2023-11-29 20:32 ` Michał Winiarski
0 siblings, 1 reply; 32+ messages in thread
From: Michal Wajdeczko @ 2023-11-29 9:38 UTC (permalink / raw)
To: Michał Winiarski, intel-xe; +Cc: Lucas De Marchi, Matt Roper
On 29.11.2023 02:16, Michał Winiarski wrote:
> A helper for managed BO allocations makes it possible to remove specific
> "fini" actions and will simplify the following patches adding ability to
> execute a release action for specific BO directly.
>
> Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
> ---
> drivers/gpu/drm/xe/xe_bo.c | 33 ++++++++++++++++++++++++++++
> drivers/gpu/drm/xe/xe_bo.h | 4 ++++
> drivers/gpu/drm/xe/xe_ggtt.c | 6 +----
> drivers/gpu/drm/xe/xe_guc_ads.c | 20 +++--------------
> drivers/gpu/drm/xe/xe_guc_ct.c | 8 +++----
> drivers/gpu/drm/xe/xe_guc_hwconfig.c | 18 +++------------
> drivers/gpu/drm/xe/xe_guc_log.c | 19 +++-------------
> drivers/gpu/drm/xe/xe_guc_pc.c | 9 +++-----
> drivers/gpu/drm/xe/xe_hw_engine.c | 8 +++----
> drivers/gpu/drm/xe/xe_uc_fw.c | 9 ++++----
> 10 files changed, 60 insertions(+), 74 deletions(-)
>
> diff --git a/drivers/gpu/drm/xe/xe_bo.c b/drivers/gpu/drm/xe/xe_bo.c
> index c5aa01c0861c7..7f5b616a7bbeb 100644
> --- a/drivers/gpu/drm/xe/xe_bo.c
> +++ b/drivers/gpu/drm/xe/xe_bo.c
> @@ -9,6 +9,7 @@
>
> #include <drm/drm_drv.h>
> #include <drm/drm_gem_ttm_helper.h>
> +#include <drm/drm_managed.h>
> #include <drm/ttm/ttm_device.h>
> #include <drm/ttm/ttm_placement.h>
> #include <drm/ttm/ttm_tt.h>
> @@ -1493,6 +1494,38 @@ struct xe_bo *xe_bo_create_from_data(struct xe_device *xe, struct xe_tile *tile,
> return bo;
> }
>
> +static void __xe_bo_release(struct drm_device *drm, void *arg)
nit: __release_xe_bo() ? this is a drm action, not a xe_bo function
> +{
> + xe_bo_unpin_map_no_vm(arg);
> +}
> +
> +struct xe_bo *xe_managed_bo_create_pin_map(struct xe_device *xe, struct xe_tile *tile,
> + size_t size, u32 flags)
> +{
> + struct xe_bo *bo;
> + int ret;
> +
> + bo = xe_bo_create_pin_map(xe, tile, NULL, size, ttm_bo_type_kernel, flags);
> + if (IS_ERR(bo))
> + return bo;
> +
> + ret = drmm_add_action_or_reset(&xe->drm, __xe_bo_release, bo);
> + if (ret)
> + return ERR_PTR(ret);
> +
> + return bo;
> +}
> +
> +struct xe_bo *xe_managed_bo_create_from_data(struct xe_device *xe, struct xe_tile *tile,
> + const void *data, size_t size, u32 flags)
> +{
> + struct xe_bo *bo = xe_managed_bo_create_pin_map(xe, tile, size, flags);
> +
> + xe_map_memcpy_to(xe, &bo->vmap, 0, data, size);
will crash if "bo" is a ERR_PTR
> +
> + return bo;
> +}
> +
> /*
> * XXX: This is in the VM bind data path, likely should calculate this once and
> * store, with a recalculation if the BO is moved.
> diff --git a/drivers/gpu/drm/xe/xe_bo.h b/drivers/gpu/drm/xe/xe_bo.h
> index 9f373f6d25f22..2cf32713bde9e 100644
> --- a/drivers/gpu/drm/xe/xe_bo.h
> +++ b/drivers/gpu/drm/xe/xe_bo.h
> @@ -109,6 +109,10 @@ struct xe_bo *xe_bo_create_pin_map_at(struct xe_device *xe, struct xe_tile *tile
> struct xe_bo *xe_bo_create_from_data(struct xe_device *xe, struct xe_tile *tile,
> const void *data, size_t size,
> enum ttm_bo_type type, u32 flags);
> +struct xe_bo *xe_managed_bo_create_pin_map(struct xe_device *xe, struct xe_tile *tile,
> + size_t size, u32 flags);
> +struct xe_bo *xe_managed_bo_create_from_data(struct xe_device *xe, struct xe_tile *tile,
> + const void *data, size_t size, u32 flags);
nit: isn't "xe_device *xe" redundant if we also pass a "xe_tile *tile" ?
>
> int xe_bo_placement_for_flags(struct xe_device *xe, struct xe_bo *bo,
> u32 bo_flags);
> diff --git a/drivers/gpu/drm/xe/xe_ggtt.c b/drivers/gpu/drm/xe/xe_ggtt.c
> index fab3cc04882da..8dc783e9120d2 100644
> --- a/drivers/gpu/drm/xe/xe_ggtt.c
> +++ b/drivers/gpu/drm/xe/xe_ggtt.c
> @@ -108,7 +108,6 @@ static void ggtt_fini(struct drm_device *drm, void *arg)
> {
> struct xe_ggtt *ggtt = arg;
>
> - xe_bo_unpin_map_no_vm(ggtt->scratch);
> ggtt->scratch = NULL;
> }
>
> @@ -225,10 +224,7 @@ int xe_ggtt_init(struct xe_ggtt *ggtt)
> else
> flags |= XE_BO_CREATE_VRAM_IF_DGFX(ggtt->tile);
>
> - ggtt->scratch = xe_bo_create_pin_map(xe, ggtt->tile, NULL, XE_PAGE_SIZE,
> - ttm_bo_type_kernel,
> - flags);
> -
> + ggtt->scratch = xe_managed_bo_create_pin_map(xe, ggtt->tile, XE_PAGE_SIZE, flags);
> if (IS_ERR(ggtt->scratch)) {
> err = PTR_ERR(ggtt->scratch);
> goto err;
> diff --git a/drivers/gpu/drm/xe/xe_guc_ads.c b/drivers/gpu/drm/xe/xe_guc_ads.c
> index 88789826e7817..2f5ff090aa6bd 100644
> --- a/drivers/gpu/drm/xe/xe_guc_ads.c
> +++ b/drivers/gpu/drm/xe/xe_guc_ads.c
> @@ -202,13 +202,6 @@ static size_t guc_ads_size(struct xe_guc_ads *ads)
> guc_ads_private_data_size(ads);
> }
>
> -static void guc_ads_fini(struct drm_device *drm, void *arg)
> -{
> - struct xe_guc_ads *ads = arg;
> -
> - xe_bo_unpin_map_no_vm(ads->bo);
> -}
> -
> static bool needs_wa_1607983814(struct xe_device *xe)
> {
> return GRAPHICS_VERx100(xe) < 1250;
> @@ -274,25 +267,18 @@ int xe_guc_ads_init(struct xe_guc_ads *ads)
> struct xe_gt *gt = ads_to_gt(ads);
> struct xe_tile *tile = gt_to_tile(gt);
> struct xe_bo *bo;
> - int err;
>
> ads->golden_lrc_size = calculate_golden_lrc_size(ads);
> ads->regset_size = calculate_regset_size(gt);
>
> - bo = xe_bo_create_pin_map(xe, tile, NULL, guc_ads_size(ads) +
> - MAX_GOLDEN_LRC_SIZE,
> - ttm_bo_type_kernel,
> - XE_BO_CREATE_VRAM_IF_DGFX(tile) |
> - XE_BO_CREATE_GGTT_BIT);
> + bo = xe_managed_bo_create_pin_map(xe, tile, guc_ads_size(ads) + MAX_GOLDEN_LRC_SIZE,
> + XE_BO_CREATE_VRAM_IF_DGFX(tile) |
> + XE_BO_CREATE_GGTT_BIT);
> if (IS_ERR(bo))
> return PTR_ERR(bo);
>
> ads->bo = bo;
>
> - err = drmm_add_action_or_reset(&xe->drm, guc_ads_fini, ads);
> - if (err)
> - return err;
> -
> return 0;
> }
>
> diff --git a/drivers/gpu/drm/xe/xe_guc_ct.c b/drivers/gpu/drm/xe/xe_guc_ct.c
> index c44e750746958..93c886ef6fdb4 100644
> --- a/drivers/gpu/drm/xe/xe_guc_ct.c
> +++ b/drivers/gpu/drm/xe/xe_guc_ct.c
> @@ -112,7 +112,6 @@ static void guc_ct_fini(struct drm_device *drm, void *arg)
> struct xe_guc_ct *ct = arg;
>
> xa_destroy(&ct->fence_lookup);
> - xe_bo_unpin_map_no_vm(ct->bo);
> }
>
> static void g2h_worker_func(struct work_struct *w);
> @@ -146,10 +145,9 @@ int xe_guc_ct_init(struct xe_guc_ct *ct)
>
> primelockdep(ct);
>
> - bo = xe_bo_create_pin_map(xe, tile, NULL, guc_ct_size(),
> - ttm_bo_type_kernel,
> - XE_BO_CREATE_VRAM_IF_DGFX(tile) |
> - XE_BO_CREATE_GGTT_BIT);
> + bo = xe_managed_bo_create_pin_map(xe, tile, guc_ct_size(),
> + XE_BO_CREATE_VRAM_IF_DGFX(tile) |
> + XE_BO_CREATE_GGTT_BIT);
> if (IS_ERR(bo))
> return PTR_ERR(bo);
>
> diff --git a/drivers/gpu/drm/xe/xe_guc_hwconfig.c b/drivers/gpu/drm/xe/xe_guc_hwconfig.c
> index 57d325ec8ce32..8e3db5d7192c3 100644
> --- a/drivers/gpu/drm/xe/xe_guc_hwconfig.c
> +++ b/drivers/gpu/drm/xe/xe_guc_hwconfig.c
> @@ -47,13 +47,6 @@ static int guc_hwconfig_copy(struct xe_guc *guc)
> return 0;
> }
>
> -static void guc_hwconfig_fini(struct drm_device *drm, void *arg)
> -{
> - struct xe_guc *guc = arg;
> -
> - xe_bo_unpin_map_no_vm(guc->hwconfig.bo);
> -}
> -
> int xe_guc_hwconfig_init(struct xe_guc *guc)
> {
> struct xe_device *xe = guc_to_xe(guc);
> @@ -83,19 +76,14 @@ int xe_guc_hwconfig_init(struct xe_guc *guc)
> if (!size)
> return -EINVAL;
>
> - bo = xe_bo_create_pin_map(xe, tile, NULL, PAGE_ALIGN(size),
> - ttm_bo_type_kernel,
> - XE_BO_CREATE_VRAM_IF_DGFX(tile) |
> - XE_BO_CREATE_GGTT_BIT);
> + bo = xe_managed_bo_create_pin_map(xe, tile, PAGE_ALIGN(size),
> + XE_BO_CREATE_VRAM_IF_DGFX(tile) |
> + XE_BO_CREATE_GGTT_BIT);
> if (IS_ERR(bo))
> return PTR_ERR(bo);
> guc->hwconfig.bo = bo;
> guc->hwconfig.size = size;
>
> - err = drmm_add_action_or_reset(&xe->drm, guc_hwconfig_fini, guc);
> - if (err)
> - return err;
> -
> return guc_hwconfig_copy(guc);
> }
>
> diff --git a/drivers/gpu/drm/xe/xe_guc_log.c b/drivers/gpu/drm/xe/xe_guc_log.c
> index 27c3827bfd054..bcd2f4d34081d 100644
> --- a/drivers/gpu/drm/xe/xe_guc_log.c
> +++ b/drivers/gpu/drm/xe/xe_guc_log.c
> @@ -77,24 +77,15 @@ void xe_guc_log_print(struct xe_guc_log *log, struct drm_printer *p)
> }
> }
>
> -static void guc_log_fini(struct drm_device *drm, void *arg)
> -{
> - struct xe_guc_log *log = arg;
> -
> - xe_bo_unpin_map_no_vm(log->bo);
> -}
> -
> int xe_guc_log_init(struct xe_guc_log *log)
> {
> struct xe_device *xe = log_to_xe(log);
> struct xe_tile *tile = gt_to_tile(log_to_gt(log));
> struct xe_bo *bo;
> - int err;
>
> - bo = xe_bo_create_pin_map(xe, tile, NULL, guc_log_size(),
> - ttm_bo_type_kernel,
> - XE_BO_CREATE_VRAM_IF_DGFX(tile) |
> - XE_BO_CREATE_GGTT_BIT);
> + bo = xe_managed_bo_create_pin_map(xe, tile, guc_log_size(),
> + XE_BO_CREATE_VRAM_IF_DGFX(tile) |
> + XE_BO_CREATE_GGTT_BIT);
> if (IS_ERR(bo))
> return PTR_ERR(bo);
>
> @@ -102,9 +93,5 @@ int xe_guc_log_init(struct xe_guc_log *log)
> log->bo = bo;
> log->level = xe_modparam.guc_log_level;
>
> - err = drmm_add_action_or_reset(&xe->drm, guc_log_fini, log);
> - if (err)
> - return err;
> -
> return 0;
> }
> diff --git a/drivers/gpu/drm/xe/xe_guc_pc.c b/drivers/gpu/drm/xe/xe_guc_pc.c
> index e9dd6c3d750bd..f5009a7968afa 100644
> --- a/drivers/gpu/drm/xe/xe_guc_pc.c
> +++ b/drivers/gpu/drm/xe/xe_guc_pc.c
> @@ -936,7 +936,6 @@ void xe_guc_pc_fini(struct xe_guc_pc *pc)
> XE_WARN_ON(xe_guc_pc_gucrc_disable(pc));
> XE_WARN_ON(xe_guc_pc_stop(pc));
> sysfs_remove_files(pc_to_gt(pc)->sysfs, pc_attrs);
> - xe_bo_unpin_map_no_vm(pc->bo);
> mutex_destroy(&pc->freq_lock);
> }
>
> @@ -955,11 +954,9 @@ int xe_guc_pc_init(struct xe_guc_pc *pc)
>
> mutex_init(&pc->freq_lock);
>
> - bo = xe_bo_create_pin_map(xe, tile, NULL, size,
> - ttm_bo_type_kernel,
> - XE_BO_CREATE_VRAM_IF_DGFX(tile) |
> - XE_BO_CREATE_GGTT_BIT);
> -
> + bo = xe_managed_bo_create_pin_map(xe, tile, size,
> + XE_BO_CREATE_VRAM_IF_DGFX(tile) |
> + XE_BO_CREATE_GGTT_BIT);
> if (IS_ERR(bo))
> return PTR_ERR(bo);
>
> diff --git a/drivers/gpu/drm/xe/xe_hw_engine.c b/drivers/gpu/drm/xe/xe_hw_engine.c
> index e831e63c5e485..34990f396a1a8 100644
> --- a/drivers/gpu/drm/xe/xe_hw_engine.c
> +++ b/drivers/gpu/drm/xe/xe_hw_engine.c
> @@ -238,8 +238,6 @@ static void hw_engine_fini(struct drm_device *drm, void *arg)
> xe_execlist_port_destroy(hwe->exl_port);
> xe_lrc_finish(&hwe->kernel_lrc);
>
> - xe_bo_unpin_map_no_vm(hwe->hwsp);
> -
> hwe->gt = NULL;
> }
>
> @@ -427,9 +425,9 @@ static int hw_engine_init(struct xe_gt *gt, struct xe_hw_engine *hwe,
> xe_reg_sr_apply_mmio(&hwe->reg_sr, gt);
> xe_reg_sr_apply_whitelist(hwe);
>
> - hwe->hwsp = xe_bo_create_pin_map(xe, tile, NULL, SZ_4K, ttm_bo_type_kernel,
> - XE_BO_CREATE_VRAM_IF_DGFX(tile) |
> - XE_BO_CREATE_GGTT_BIT);
> + hwe->hwsp = xe_managed_bo_create_pin_map(xe, tile, SZ_4K,
> + XE_BO_CREATE_VRAM_IF_DGFX(tile) |
> + XE_BO_CREATE_GGTT_BIT);
> if (IS_ERR(hwe->hwsp)) {
> err = PTR_ERR(hwe->hwsp);
> goto err_name;
> diff --git a/drivers/gpu/drm/xe/xe_uc_fw.c b/drivers/gpu/drm/xe/xe_uc_fw.c
> index 25778887d846f..5b86c51941335 100644
> --- a/drivers/gpu/drm/xe/xe_uc_fw.c
> +++ b/drivers/gpu/drm/xe/xe_uc_fw.c
> @@ -266,7 +266,6 @@ static void uc_fw_fini(struct drm_device *drm, void *arg)
> if (!xe_uc_fw_is_available(uc_fw))
> return;
>
> - xe_bo_unpin_map_no_vm(uc_fw->bo);
> xe_uc_fw_change_status(uc_fw, XE_UC_FIRMWARE_SELECTED);
> }
>
> @@ -575,10 +574,9 @@ int xe_uc_fw_init(struct xe_uc_fw *uc_fw)
> if (err)
> goto fail;
>
> - obj = xe_bo_create_from_data(xe, tile, fw->data, fw->size,
> - ttm_bo_type_kernel,
> - XE_BO_CREATE_VRAM_IF_DGFX(tile) |
> - XE_BO_CREATE_GGTT_BIT);
> + obj = xe_managed_bo_create_from_data(xe, tile, fw->data, fw->size,
> + XE_BO_CREATE_VRAM_IF_DGFX(tile) |
> + XE_BO_CREATE_GGTT_BIT);
> if (IS_ERR(obj)) {
> drm_notice(&xe->drm, "%s firmware %s: failed to create / populate bo",
> xe_uc_fw_type_repr(uc_fw->type), uc_fw->path);
> @@ -609,6 +607,7 @@ int xe_uc_fw_init(struct xe_uc_fw *uc_fw)
> xe_uc_fw_type_repr(uc_fw->type), XE_UC_FIRMWARE_URL);
>
> release_firmware(fw); /* OK even if fw is NULL */
> +
> return err;
> }
>
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [Intel-xe] [PATCH v4 14/22] drm/xe: Add a helper for DRM device-lifetime BO create
2023-11-29 9:38 ` Michal Wajdeczko
@ 2023-11-29 20:32 ` Michał Winiarski
0 siblings, 0 replies; 32+ messages in thread
From: Michał Winiarski @ 2023-11-29 20:32 UTC (permalink / raw)
To: Michal Wajdeczko; +Cc: Lucas De Marchi, Matt Roper, intel-xe
On Wed, Nov 29, 2023 at 10:38:44AM +0100, Michal Wajdeczko wrote:
>
>
> On 29.11.2023 02:16, Michał Winiarski wrote:
> > A helper for managed BO allocations makes it possible to remove specific
> > "fini" actions and will simplify the following patches adding ability to
> > execute a release action for specific BO directly.
> >
> > Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
> > ---
> > drivers/gpu/drm/xe/xe_bo.c | 33 ++++++++++++++++++++++++++++
> > drivers/gpu/drm/xe/xe_bo.h | 4 ++++
> > drivers/gpu/drm/xe/xe_ggtt.c | 6 +----
> > drivers/gpu/drm/xe/xe_guc_ads.c | 20 +++--------------
> > drivers/gpu/drm/xe/xe_guc_ct.c | 8 +++----
> > drivers/gpu/drm/xe/xe_guc_hwconfig.c | 18 +++------------
> > drivers/gpu/drm/xe/xe_guc_log.c | 19 +++-------------
> > drivers/gpu/drm/xe/xe_guc_pc.c | 9 +++-----
> > drivers/gpu/drm/xe/xe_hw_engine.c | 8 +++----
> > drivers/gpu/drm/xe/xe_uc_fw.c | 9 ++++----
> > 10 files changed, 60 insertions(+), 74 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/xe/xe_bo.c b/drivers/gpu/drm/xe/xe_bo.c
> > index c5aa01c0861c7..7f5b616a7bbeb 100644
> > --- a/drivers/gpu/drm/xe/xe_bo.c
> > +++ b/drivers/gpu/drm/xe/xe_bo.c
> > @@ -9,6 +9,7 @@
> >
> > #include <drm/drm_drv.h>
> > #include <drm/drm_gem_ttm_helper.h>
> > +#include <drm/drm_managed.h>
> > #include <drm/ttm/ttm_device.h>
> > #include <drm/ttm/ttm_placement.h>
> > #include <drm/ttm/ttm_tt.h>
> > @@ -1493,6 +1494,38 @@ struct xe_bo *xe_bo_create_from_data(struct xe_device *xe, struct xe_tile *tile,
> > return bo;
> > }
> >
> > +static void __xe_bo_release(struct drm_device *drm, void *arg)
>
> nit: __release_xe_bo() ? this is a drm action, not a xe_bo function
Xe doesn't follow this convention for release actions passed to
drmm_add_action (it's usually xe_*).
>
> > +{
> > + xe_bo_unpin_map_no_vm(arg);
> > +}
> > +
> > +struct xe_bo *xe_managed_bo_create_pin_map(struct xe_device *xe, struct xe_tile *tile,
> > + size_t size, u32 flags)
> > +{
> > + struct xe_bo *bo;
> > + int ret;
> > +
> > + bo = xe_bo_create_pin_map(xe, tile, NULL, size, ttm_bo_type_kernel, flags);
> > + if (IS_ERR(bo))
> > + return bo;
> > +
> > + ret = drmm_add_action_or_reset(&xe->drm, __xe_bo_release, bo);
> > + if (ret)
> > + return ERR_PTR(ret);
> > +
> > + return bo;
> > +}
> > +
> > +struct xe_bo *xe_managed_bo_create_from_data(struct xe_device *xe, struct xe_tile *tile,
> > + const void *data, size_t size, u32 flags)
> > +{
> > + struct xe_bo *bo = xe_managed_bo_create_pin_map(xe, tile, size, flags);
> > +
> > + xe_map_memcpy_to(xe, &bo->vmap, 0, data, size);
>
> will crash if "bo" is a ERR_PTR
Yup, I'll add a check.
>
> > +
> > + return bo;
> > +}
> > +
> > /*
> > * XXX: This is in the VM bind data path, likely should calculate this once and
> > * store, with a recalculation if the BO is moved.
> > diff --git a/drivers/gpu/drm/xe/xe_bo.h b/drivers/gpu/drm/xe/xe_bo.h
> > index 9f373f6d25f22..2cf32713bde9e 100644
> > --- a/drivers/gpu/drm/xe/xe_bo.h
> > +++ b/drivers/gpu/drm/xe/xe_bo.h
> > @@ -109,6 +109,10 @@ struct xe_bo *xe_bo_create_pin_map_at(struct xe_device *xe, struct xe_tile *tile
> > struct xe_bo *xe_bo_create_from_data(struct xe_device *xe, struct xe_tile *tile,
> > const void *data, size_t size,
> > enum ttm_bo_type type, u32 flags);
> > +struct xe_bo *xe_managed_bo_create_pin_map(struct xe_device *xe, struct xe_tile *tile,
> > + size_t size, u32 flags);
> > +struct xe_bo *xe_managed_bo_create_from_data(struct xe_device *xe, struct xe_tile *tile,
> > + const void *data, size_t size, u32 flags);
>
> nit: isn't "xe_device *xe" redundant if we also pass a "xe_tile *tile" ?
It is, but it is also redundant for all of the non-managed variants of
this function, so I would prefer to keep it consistent in this series.
Thanks,
-Michał
>
> >
> > int xe_bo_placement_for_flags(struct xe_device *xe, struct xe_bo *bo,
> > u32 bo_flags);
> > diff --git a/drivers/gpu/drm/xe/xe_ggtt.c b/drivers/gpu/drm/xe/xe_ggtt.c
> > index fab3cc04882da..8dc783e9120d2 100644
> > --- a/drivers/gpu/drm/xe/xe_ggtt.c
> > +++ b/drivers/gpu/drm/xe/xe_ggtt.c
> > @@ -108,7 +108,6 @@ static void ggtt_fini(struct drm_device *drm, void *arg)
> > {
> > struct xe_ggtt *ggtt = arg;
> >
> > - xe_bo_unpin_map_no_vm(ggtt->scratch);
> > ggtt->scratch = NULL;
> > }
> >
> > @@ -225,10 +224,7 @@ int xe_ggtt_init(struct xe_ggtt *ggtt)
> > else
> > flags |= XE_BO_CREATE_VRAM_IF_DGFX(ggtt->tile);
> >
> > - ggtt->scratch = xe_bo_create_pin_map(xe, ggtt->tile, NULL, XE_PAGE_SIZE,
> > - ttm_bo_type_kernel,
> > - flags);
> > -
> > + ggtt->scratch = xe_managed_bo_create_pin_map(xe, ggtt->tile, XE_PAGE_SIZE, flags);
> > if (IS_ERR(ggtt->scratch)) {
> > err = PTR_ERR(ggtt->scratch);
> > goto err;
> > diff --git a/drivers/gpu/drm/xe/xe_guc_ads.c b/drivers/gpu/drm/xe/xe_guc_ads.c
> > index 88789826e7817..2f5ff090aa6bd 100644
> > --- a/drivers/gpu/drm/xe/xe_guc_ads.c
> > +++ b/drivers/gpu/drm/xe/xe_guc_ads.c
> > @@ -202,13 +202,6 @@ static size_t guc_ads_size(struct xe_guc_ads *ads)
> > guc_ads_private_data_size(ads);
> > }
> >
> > -static void guc_ads_fini(struct drm_device *drm, void *arg)
> > -{
> > - struct xe_guc_ads *ads = arg;
> > -
> > - xe_bo_unpin_map_no_vm(ads->bo);
> > -}
> > -
> > static bool needs_wa_1607983814(struct xe_device *xe)
> > {
> > return GRAPHICS_VERx100(xe) < 1250;
> > @@ -274,25 +267,18 @@ int xe_guc_ads_init(struct xe_guc_ads *ads)
> > struct xe_gt *gt = ads_to_gt(ads);
> > struct xe_tile *tile = gt_to_tile(gt);
> > struct xe_bo *bo;
> > - int err;
> >
> > ads->golden_lrc_size = calculate_golden_lrc_size(ads);
> > ads->regset_size = calculate_regset_size(gt);
> >
> > - bo = xe_bo_create_pin_map(xe, tile, NULL, guc_ads_size(ads) +
> > - MAX_GOLDEN_LRC_SIZE,
> > - ttm_bo_type_kernel,
> > - XE_BO_CREATE_VRAM_IF_DGFX(tile) |
> > - XE_BO_CREATE_GGTT_BIT);
> > + bo = xe_managed_bo_create_pin_map(xe, tile, guc_ads_size(ads) + MAX_GOLDEN_LRC_SIZE,
> > + XE_BO_CREATE_VRAM_IF_DGFX(tile) |
> > + XE_BO_CREATE_GGTT_BIT);
> > if (IS_ERR(bo))
> > return PTR_ERR(bo);
> >
> > ads->bo = bo;
> >
> > - err = drmm_add_action_or_reset(&xe->drm, guc_ads_fini, ads);
> > - if (err)
> > - return err;
> > -
> > return 0;
> > }
> >
> > diff --git a/drivers/gpu/drm/xe/xe_guc_ct.c b/drivers/gpu/drm/xe/xe_guc_ct.c
> > index c44e750746958..93c886ef6fdb4 100644
> > --- a/drivers/gpu/drm/xe/xe_guc_ct.c
> > +++ b/drivers/gpu/drm/xe/xe_guc_ct.c
> > @@ -112,7 +112,6 @@ static void guc_ct_fini(struct drm_device *drm, void *arg)
> > struct xe_guc_ct *ct = arg;
> >
> > xa_destroy(&ct->fence_lookup);
> > - xe_bo_unpin_map_no_vm(ct->bo);
> > }
> >
> > static void g2h_worker_func(struct work_struct *w);
> > @@ -146,10 +145,9 @@ int xe_guc_ct_init(struct xe_guc_ct *ct)
> >
> > primelockdep(ct);
> >
> > - bo = xe_bo_create_pin_map(xe, tile, NULL, guc_ct_size(),
> > - ttm_bo_type_kernel,
> > - XE_BO_CREATE_VRAM_IF_DGFX(tile) |
> > - XE_BO_CREATE_GGTT_BIT);
> > + bo = xe_managed_bo_create_pin_map(xe, tile, guc_ct_size(),
> > + XE_BO_CREATE_VRAM_IF_DGFX(tile) |
> > + XE_BO_CREATE_GGTT_BIT);
> > if (IS_ERR(bo))
> > return PTR_ERR(bo);
> >
> > diff --git a/drivers/gpu/drm/xe/xe_guc_hwconfig.c b/drivers/gpu/drm/xe/xe_guc_hwconfig.c
> > index 57d325ec8ce32..8e3db5d7192c3 100644
> > --- a/drivers/gpu/drm/xe/xe_guc_hwconfig.c
> > +++ b/drivers/gpu/drm/xe/xe_guc_hwconfig.c
> > @@ -47,13 +47,6 @@ static int guc_hwconfig_copy(struct xe_guc *guc)
> > return 0;
> > }
> >
> > -static void guc_hwconfig_fini(struct drm_device *drm, void *arg)
> > -{
> > - struct xe_guc *guc = arg;
> > -
> > - xe_bo_unpin_map_no_vm(guc->hwconfig.bo);
> > -}
> > -
> > int xe_guc_hwconfig_init(struct xe_guc *guc)
> > {
> > struct xe_device *xe = guc_to_xe(guc);
> > @@ -83,19 +76,14 @@ int xe_guc_hwconfig_init(struct xe_guc *guc)
> > if (!size)
> > return -EINVAL;
> >
> > - bo = xe_bo_create_pin_map(xe, tile, NULL, PAGE_ALIGN(size),
> > - ttm_bo_type_kernel,
> > - XE_BO_CREATE_VRAM_IF_DGFX(tile) |
> > - XE_BO_CREATE_GGTT_BIT);
> > + bo = xe_managed_bo_create_pin_map(xe, tile, PAGE_ALIGN(size),
> > + XE_BO_CREATE_VRAM_IF_DGFX(tile) |
> > + XE_BO_CREATE_GGTT_BIT);
> > if (IS_ERR(bo))
> > return PTR_ERR(bo);
> > guc->hwconfig.bo = bo;
> > guc->hwconfig.size = size;
> >
> > - err = drmm_add_action_or_reset(&xe->drm, guc_hwconfig_fini, guc);
> > - if (err)
> > - return err;
> > -
> > return guc_hwconfig_copy(guc);
> > }
> >
> > diff --git a/drivers/gpu/drm/xe/xe_guc_log.c b/drivers/gpu/drm/xe/xe_guc_log.c
> > index 27c3827bfd054..bcd2f4d34081d 100644
> > --- a/drivers/gpu/drm/xe/xe_guc_log.c
> > +++ b/drivers/gpu/drm/xe/xe_guc_log.c
> > @@ -77,24 +77,15 @@ void xe_guc_log_print(struct xe_guc_log *log, struct drm_printer *p)
> > }
> > }
> >
> > -static void guc_log_fini(struct drm_device *drm, void *arg)
> > -{
> > - struct xe_guc_log *log = arg;
> > -
> > - xe_bo_unpin_map_no_vm(log->bo);
> > -}
> > -
> > int xe_guc_log_init(struct xe_guc_log *log)
> > {
> > struct xe_device *xe = log_to_xe(log);
> > struct xe_tile *tile = gt_to_tile(log_to_gt(log));
> > struct xe_bo *bo;
> > - int err;
> >
> > - bo = xe_bo_create_pin_map(xe, tile, NULL, guc_log_size(),
> > - ttm_bo_type_kernel,
> > - XE_BO_CREATE_VRAM_IF_DGFX(tile) |
> > - XE_BO_CREATE_GGTT_BIT);
> > + bo = xe_managed_bo_create_pin_map(xe, tile, guc_log_size(),
> > + XE_BO_CREATE_VRAM_IF_DGFX(tile) |
> > + XE_BO_CREATE_GGTT_BIT);
> > if (IS_ERR(bo))
> > return PTR_ERR(bo);
> >
> > @@ -102,9 +93,5 @@ int xe_guc_log_init(struct xe_guc_log *log)
> > log->bo = bo;
> > log->level = xe_modparam.guc_log_level;
> >
> > - err = drmm_add_action_or_reset(&xe->drm, guc_log_fini, log);
> > - if (err)
> > - return err;
> > -
> > return 0;
> > }
> > diff --git a/drivers/gpu/drm/xe/xe_guc_pc.c b/drivers/gpu/drm/xe/xe_guc_pc.c
> > index e9dd6c3d750bd..f5009a7968afa 100644
> > --- a/drivers/gpu/drm/xe/xe_guc_pc.c
> > +++ b/drivers/gpu/drm/xe/xe_guc_pc.c
> > @@ -936,7 +936,6 @@ void xe_guc_pc_fini(struct xe_guc_pc *pc)
> > XE_WARN_ON(xe_guc_pc_gucrc_disable(pc));
> > XE_WARN_ON(xe_guc_pc_stop(pc));
> > sysfs_remove_files(pc_to_gt(pc)->sysfs, pc_attrs);
> > - xe_bo_unpin_map_no_vm(pc->bo);
> > mutex_destroy(&pc->freq_lock);
> > }
> >
> > @@ -955,11 +954,9 @@ int xe_guc_pc_init(struct xe_guc_pc *pc)
> >
> > mutex_init(&pc->freq_lock);
> >
> > - bo = xe_bo_create_pin_map(xe, tile, NULL, size,
> > - ttm_bo_type_kernel,
> > - XE_BO_CREATE_VRAM_IF_DGFX(tile) |
> > - XE_BO_CREATE_GGTT_BIT);
> > -
> > + bo = xe_managed_bo_create_pin_map(xe, tile, size,
> > + XE_BO_CREATE_VRAM_IF_DGFX(tile) |
> > + XE_BO_CREATE_GGTT_BIT);
> > if (IS_ERR(bo))
> > return PTR_ERR(bo);
> >
> > diff --git a/drivers/gpu/drm/xe/xe_hw_engine.c b/drivers/gpu/drm/xe/xe_hw_engine.c
> > index e831e63c5e485..34990f396a1a8 100644
> > --- a/drivers/gpu/drm/xe/xe_hw_engine.c
> > +++ b/drivers/gpu/drm/xe/xe_hw_engine.c
> > @@ -238,8 +238,6 @@ static void hw_engine_fini(struct drm_device *drm, void *arg)
> > xe_execlist_port_destroy(hwe->exl_port);
> > xe_lrc_finish(&hwe->kernel_lrc);
> >
> > - xe_bo_unpin_map_no_vm(hwe->hwsp);
> > -
> > hwe->gt = NULL;
> > }
> >
> > @@ -427,9 +425,9 @@ static int hw_engine_init(struct xe_gt *gt, struct xe_hw_engine *hwe,
> > xe_reg_sr_apply_mmio(&hwe->reg_sr, gt);
> > xe_reg_sr_apply_whitelist(hwe);
> >
> > - hwe->hwsp = xe_bo_create_pin_map(xe, tile, NULL, SZ_4K, ttm_bo_type_kernel,
> > - XE_BO_CREATE_VRAM_IF_DGFX(tile) |
> > - XE_BO_CREATE_GGTT_BIT);
> > + hwe->hwsp = xe_managed_bo_create_pin_map(xe, tile, SZ_4K,
> > + XE_BO_CREATE_VRAM_IF_DGFX(tile) |
> > + XE_BO_CREATE_GGTT_BIT);
> > if (IS_ERR(hwe->hwsp)) {
> > err = PTR_ERR(hwe->hwsp);
> > goto err_name;
> > diff --git a/drivers/gpu/drm/xe/xe_uc_fw.c b/drivers/gpu/drm/xe/xe_uc_fw.c
> > index 25778887d846f..5b86c51941335 100644
> > --- a/drivers/gpu/drm/xe/xe_uc_fw.c
> > +++ b/drivers/gpu/drm/xe/xe_uc_fw.c
> > @@ -266,7 +266,6 @@ static void uc_fw_fini(struct drm_device *drm, void *arg)
> > if (!xe_uc_fw_is_available(uc_fw))
> > return;
> >
> > - xe_bo_unpin_map_no_vm(uc_fw->bo);
> > xe_uc_fw_change_status(uc_fw, XE_UC_FIRMWARE_SELECTED);
> > }
> >
> > @@ -575,10 +574,9 @@ int xe_uc_fw_init(struct xe_uc_fw *uc_fw)
> > if (err)
> > goto fail;
> >
> > - obj = xe_bo_create_from_data(xe, tile, fw->data, fw->size,
> > - ttm_bo_type_kernel,
> > - XE_BO_CREATE_VRAM_IF_DGFX(tile) |
> > - XE_BO_CREATE_GGTT_BIT);
> > + obj = xe_managed_bo_create_from_data(xe, tile, fw->data, fw->size,
> > + XE_BO_CREATE_VRAM_IF_DGFX(tile) |
> > + XE_BO_CREATE_GGTT_BIT);
> > if (IS_ERR(obj)) {
> > drm_notice(&xe->drm, "%s firmware %s: failed to create / populate bo",
> > xe_uc_fw_type_repr(uc_fw->type), uc_fw->path);
> > @@ -609,6 +607,7 @@ int xe_uc_fw_init(struct xe_uc_fw *uc_fw)
> > xe_uc_fw_type_repr(uc_fw->type), XE_UC_FIRMWARE_URL);
> >
> > release_firmware(fw); /* OK even if fw is NULL */
> > +
> > return err;
> > }
> >
^ permalink raw reply [flat|nested] 32+ messages in thread
* [Intel-xe] [PATCH v4 15/22] drm/xe/uc: Split xe_uc_fw_init
2023-11-29 1:16 [Intel-xe] [PATCH v4 00/22] drm/xe: Probe tweaks and reordering Michał Winiarski
` (13 preceding siblings ...)
2023-11-29 1:16 ` [Intel-xe] [PATCH v4 14/22] drm/xe: Add a helper for DRM device-lifetime BO create Michał Winiarski
@ 2023-11-29 1:16 ` Michał Winiarski
2023-11-29 1:16 ` [Intel-xe] [PATCH v4 16/22] drm/xe/uc: Store firmware binary in system-memory backed BO Michał Winiarski
` (7 subsequent siblings)
22 siblings, 0 replies; 32+ messages in thread
From: Michał Winiarski @ 2023-11-29 1:16 UTC (permalink / raw)
To: intel-xe; +Cc: Matt Roper, Lucas De Marchi, Michał Winiarski
The function does a driver specific "request firmware" step that
includes validating the input, followed by wrapping the firmware binary
into a buffer object. Split it into smaller parts.
Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
---
drivers/gpu/drm/xe/xe_uc_fw.c | 80 ++++++++++++++++++++++++++---------
1 file changed, 61 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/xe/xe_uc_fw.c b/drivers/gpu/drm/xe/xe_uc_fw.c
index 5b86c51941335..31b4886ac43eb 100644
--- a/drivers/gpu/drm/xe/xe_uc_fw.c
+++ b/drivers/gpu/drm/xe/xe_uc_fw.c
@@ -523,14 +523,11 @@ static int parse_headers(struct xe_uc_fw *uc_fw, const struct firmware *fw)
return 0;
}
-int xe_uc_fw_init(struct xe_uc_fw *uc_fw)
+static int uc_fw_request(struct xe_uc_fw *uc_fw, const struct firmware **firmware_p)
{
struct xe_device *xe = uc_fw_to_xe(uc_fw);
- struct xe_gt *gt = uc_fw_to_gt(uc_fw);
- struct xe_tile *tile = gt_to_tile(gt);
struct device *dev = xe->drm.dev;
const struct firmware *fw = NULL;
- struct xe_bo *obj;
int err;
/*
@@ -574,9 +571,39 @@ int xe_uc_fw_init(struct xe_uc_fw *uc_fw)
if (err)
goto fail;
- obj = xe_managed_bo_create_from_data(xe, tile, fw->data, fw->size,
- XE_BO_CREATE_VRAM_IF_DGFX(tile) |
- XE_BO_CREATE_GGTT_BIT);
+ *firmware_p = fw;
+
+ return 0;
+
+fail:
+ xe_uc_fw_change_status(uc_fw, err == -ENOENT ?
+ XE_UC_FIRMWARE_MISSING :
+ XE_UC_FIRMWARE_ERROR);
+
+ drm_notice(&xe->drm, "%s firmware %s: fetch failed with error %d\n",
+ xe_uc_fw_type_repr(uc_fw->type), uc_fw->path, err);
+ drm_info(&xe->drm, "%s firmware(s) can be downloaded from %s\n",
+ xe_uc_fw_type_repr(uc_fw->type), XE_UC_FIRMWARE_URL);
+
+ release_firmware(fw); /* OK even if fw is NULL */
+
+ return err;
+}
+
+static void uc_fw_release(const struct firmware *fw)
+{
+ release_firmware(fw);
+}
+
+static int uc_fw_copy(struct xe_uc_fw *uc_fw, const void *data, size_t size, u32 flags)
+{
+ struct xe_device *xe = uc_fw_to_xe(uc_fw);
+ struct xe_gt *gt = uc_fw_to_gt(uc_fw);
+ struct xe_tile *tile = gt_to_tile(gt);
+ struct xe_bo *obj;
+ int err;
+
+ obj = xe_managed_bo_create_from_data(xe, tile, data, size, flags);
if (IS_ERR(obj)) {
drm_notice(&xe->drm, "%s firmware %s: failed to create / populate bo",
xe_uc_fw_type_repr(uc_fw->type), uc_fw->path);
@@ -585,28 +612,43 @@ int xe_uc_fw_init(struct xe_uc_fw *uc_fw)
}
uc_fw->bo = obj;
- uc_fw->size = fw->size;
- xe_uc_fw_change_status(uc_fw, XE_UC_FIRMWARE_AVAILABLE);
+ uc_fw->size = size;
- release_firmware(fw);
+ xe_uc_fw_change_status(uc_fw, XE_UC_FIRMWARE_AVAILABLE);
err = drmm_add_action_or_reset(&xe->drm, uc_fw_fini, uc_fw);
if (err)
- return err;
+ goto fail;
return 0;
fail:
- xe_uc_fw_change_status(uc_fw, err == -ENOENT ?
- XE_UC_FIRMWARE_MISSING :
- XE_UC_FIRMWARE_ERROR);
-
- drm_notice(&xe->drm, "%s firmware %s: fetch failed with error %d\n",
+ xe_uc_fw_change_status(uc_fw, XE_UC_FIRMWARE_ERROR);
+ drm_notice(&xe->drm, "%s firmware %s: copy failed with error %d\n",
xe_uc_fw_type_repr(uc_fw->type), uc_fw->path, err);
- drm_info(&xe->drm, "%s firmware(s) can be downloaded from %s\n",
- xe_uc_fw_type_repr(uc_fw->type), XE_UC_FIRMWARE_URL);
- release_firmware(fw); /* OK even if fw is NULL */
+ return err;
+}
+
+int xe_uc_fw_init(struct xe_uc_fw *uc_fw)
+{
+ const struct firmware *fw = NULL;
+ struct xe_gt *gt = uc_fw_to_gt(uc_fw);
+ struct xe_tile *tile = gt_to_tile(gt);
+ int err;
+
+ err = uc_fw_request(uc_fw, &fw);
+ if (err)
+ return err;
+
+ /* no error and no firmware means nothing to copy */
+ if (!fw)
+ return 0;
+
+ err = uc_fw_copy(uc_fw, fw->data, fw->size,
+ XE_BO_CREATE_VRAM_IF_DGFX(tile) | XE_BO_CREATE_GGTT_BIT);
+
+ uc_fw_release(fw);
return err;
}
--
2.43.0
^ permalink raw reply related [flat|nested] 32+ messages in thread* [Intel-xe] [PATCH v4 16/22] drm/xe/uc: Store firmware binary in system-memory backed BO
2023-11-29 1:16 [Intel-xe] [PATCH v4 00/22] drm/xe: Probe tweaks and reordering Michał Winiarski
` (14 preceding siblings ...)
2023-11-29 1:16 ` [Intel-xe] [PATCH v4 15/22] drm/xe/uc: Split xe_uc_fw_init Michał Winiarski
@ 2023-11-29 1:16 ` Michał Winiarski
2023-11-29 1:16 ` [Intel-xe] [PATCH v4 17/22] drm/xe/uc: Extract xe_uc_sanitize_reset Michał Winiarski
` (6 subsequent siblings)
22 siblings, 0 replies; 32+ messages in thread
From: Michał Winiarski @ 2023-11-29 1:16 UTC (permalink / raw)
To: intel-xe; +Cc: Matt Roper, Lucas De Marchi, Michał Winiarski
The firmware loading for GuC is about to be moved, and will happen much
earlier in the probe process, when local-memory is not yet available.
While this has the potential to make the firmware loading process
slower, this is only happening during probe and full device reset.
Since both are not hot-paths - store all UC-like firmware in system
memory.
Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
---
drivers/gpu/drm/xe/xe_uc_fw.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/xe/xe_uc_fw.c b/drivers/gpu/drm/xe/xe_uc_fw.c
index 31b4886ac43eb..606d686dc765e 100644
--- a/drivers/gpu/drm/xe/xe_uc_fw.c
+++ b/drivers/gpu/drm/xe/xe_uc_fw.c
@@ -633,8 +633,6 @@ static int uc_fw_copy(struct xe_uc_fw *uc_fw, const void *data, size_t size, u32
int xe_uc_fw_init(struct xe_uc_fw *uc_fw)
{
const struct firmware *fw = NULL;
- struct xe_gt *gt = uc_fw_to_gt(uc_fw);
- struct xe_tile *tile = gt_to_tile(gt);
int err;
err = uc_fw_request(uc_fw, &fw);
@@ -646,7 +644,7 @@ int xe_uc_fw_init(struct xe_uc_fw *uc_fw)
return 0;
err = uc_fw_copy(uc_fw, fw->data, fw->size,
- XE_BO_CREATE_VRAM_IF_DGFX(tile) | XE_BO_CREATE_GGTT_BIT);
+ XE_BO_CREATE_SYSTEM_BIT | XE_BO_CREATE_GGTT_BIT);
uc_fw_release(fw);
--
2.43.0
^ permalink raw reply related [flat|nested] 32+ messages in thread* [Intel-xe] [PATCH v4 17/22] drm/xe/uc: Extract xe_uc_sanitize_reset
2023-11-29 1:16 [Intel-xe] [PATCH v4 00/22] drm/xe: Probe tweaks and reordering Michał Winiarski
` (15 preceding siblings ...)
2023-11-29 1:16 ` [Intel-xe] [PATCH v4 16/22] drm/xe/uc: Store firmware binary in system-memory backed BO Michał Winiarski
@ 2023-11-29 1:16 ` Michał Winiarski
2023-11-29 1:16 ` [Intel-xe] [PATCH v4 18/22] drm/xe/guc: Split GuC params used for "hwconfig" and "post-hwconfig" Michał Winiarski
` (5 subsequent siblings)
22 siblings, 0 replies; 32+ messages in thread
From: Michał Winiarski @ 2023-11-29 1:16 UTC (permalink / raw)
To: intel-xe; +Cc: Matt Roper, Lucas De Marchi, Michał Winiarski
Earlier GuC load will require more fine-grained control over reset.
Extract it outside of xe_uc_init_hw.
Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
Reviewed-by: Matthew Brost <matthew.brost@intel.com>
---
drivers/gpu/drm/xe/xe_gt.c | 4 ++++
drivers/gpu/drm/xe/xe_uc.c | 14 ++++++++------
drivers/gpu/drm/xe/xe_uc.h | 2 +-
3 files changed, 13 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/xe/xe_gt.c b/drivers/gpu/drm/xe/xe_gt.c
index 7f26899bbb1b6..9eaab11cf29a6 100644
--- a/drivers/gpu/drm/xe/xe_gt.c
+++ b/drivers/gpu/drm/xe/xe_gt.c
@@ -537,6 +537,10 @@ static int do_gt_restart(struct xe_gt *gt)
for_each_hw_engine(hwe, gt, id)
xe_hw_engine_enable_ring(hwe);
+ err = xe_uc_sanitize_reset(>->uc);
+ if (err)
+ return err;
+
err = xe_uc_init_hw(>->uc);
if (err)
return err;
diff --git a/drivers/gpu/drm/xe/xe_uc.c b/drivers/gpu/drm/xe/xe_uc.c
index 784f53c5f282f..13e76e6805ca1 100644
--- a/drivers/gpu/drm/xe/xe_uc.c
+++ b/drivers/gpu/drm/xe/xe_uc.c
@@ -69,10 +69,16 @@ int xe_uc_init(struct xe_uc *uc)
*/
int xe_uc_init_post_hwconfig(struct xe_uc *uc)
{
+ int ret;
+
/* GuC submission not enabled, nothing to do */
if (!xe_device_uc_enabled(uc_to_xe(uc)))
return 0;
+ ret = xe_uc_sanitize_reset(uc);
+ if (ret)
+ return ret;
+
return xe_guc_init_post_hwconfig(&uc->guc);
}
@@ -90,13 +96,13 @@ static int uc_reset(struct xe_uc *uc)
return 0;
}
-void xe_uc_sanitize(struct xe_uc *uc)
+static void xe_uc_sanitize(struct xe_uc *uc)
{
xe_huc_sanitize(&uc->huc);
xe_guc_sanitize(&uc->guc);
}
-static int xe_uc_sanitize_reset(struct xe_uc *uc)
+int xe_uc_sanitize_reset(struct xe_uc *uc)
{
xe_uc_sanitize(uc);
@@ -136,10 +142,6 @@ int xe_uc_init_hw(struct xe_uc *uc)
if (!xe_device_uc_enabled(uc_to_xe(uc)))
return 0;
- ret = xe_uc_sanitize_reset(uc);
- if (ret)
- return ret;
-
ret = xe_huc_upload(&uc->huc);
if (ret)
return ret;
diff --git a/drivers/gpu/drm/xe/xe_uc.h b/drivers/gpu/drm/xe/xe_uc.h
index 4109ae7028af5..5d5110c0c834b 100644
--- a/drivers/gpu/drm/xe/xe_uc.h
+++ b/drivers/gpu/drm/xe/xe_uc.h
@@ -19,6 +19,6 @@ void xe_uc_stop_prepare(struct xe_uc *uc);
int xe_uc_stop(struct xe_uc *uc);
int xe_uc_start(struct xe_uc *uc);
int xe_uc_suspend(struct xe_uc *uc);
-void xe_uc_sanitize(struct xe_uc *uc);
+int xe_uc_sanitize_reset(struct xe_uc *uc);
#endif
--
2.43.0
^ permalink raw reply related [flat|nested] 32+ messages in thread* [Intel-xe] [PATCH v4 18/22] drm/xe/guc: Split GuC params used for "hwconfig" and "post-hwconfig"
2023-11-29 1:16 [Intel-xe] [PATCH v4 00/22] drm/xe: Probe tweaks and reordering Michał Winiarski
` (16 preceding siblings ...)
2023-11-29 1:16 ` [Intel-xe] [PATCH v4 17/22] drm/xe/uc: Extract xe_uc_sanitize_reset Michał Winiarski
@ 2023-11-29 1:16 ` Michał Winiarski
2023-11-29 1:16 ` [Intel-xe] [PATCH v4 19/22] drm/managed: Add drmm_release_action Michał Winiarski
` (4 subsequent siblings)
22 siblings, 0 replies; 32+ messages in thread
From: Michał Winiarski @ 2023-11-29 1:16 UTC (permalink / raw)
To: intel-xe; +Cc: Matt Roper, Lucas De Marchi, Michał Winiarski
Move params that are not used for initial "hwconfig" load to
"post-hwconfig" phase.
Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
Reviewed-by: Matthew Brost <matthew.brost@intel.com>
Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com>
---
drivers/gpu/drm/xe/xe_guc.c | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
diff --git a/drivers/gpu/drm/xe/xe_guc.c b/drivers/gpu/drm/xe/xe_guc.c
index 08142d8ee0523..fd3396ebd3edf 100644
--- a/drivers/gpu/drm/xe/xe_guc.c
+++ b/drivers/gpu/drm/xe/xe_guc.c
@@ -180,6 +180,26 @@ static void guc_init_params(struct xe_guc *guc)
BUILD_BUG_ON(sizeof(guc->params) != GUC_CTL_MAX_DWORDS * sizeof(u32));
BUILD_BUG_ON(GUC_CTL_MAX_DWORDS + 2 != SOFT_SCRATCH_COUNT);
+ params[GUC_CTL_LOG_PARAMS] = guc_ctl_log_params_flags(guc);
+ params[GUC_CTL_FEATURE] = 0;
+ params[GUC_CTL_DEBUG] = guc_ctl_debug_flags(guc);
+ params[GUC_CTL_ADS] = guc_ctl_ads_flags(guc);
+ params[GUC_CTL_WA] = 0;
+ params[GUC_CTL_DEVID] = guc_ctl_devid(guc);
+
+ for (i = 0; i < GUC_CTL_MAX_DWORDS; i++)
+ drm_dbg(&xe->drm, "GuC param[%2d] = 0x%08x\n", i, params[i]);
+}
+
+static void guc_init_params_post_hwconfig(struct xe_guc *guc)
+{
+ struct xe_device *xe = guc_to_xe(guc);
+ u32 *params = guc->params;
+ int i;
+
+ BUILD_BUG_ON(sizeof(guc->params) != GUC_CTL_MAX_DWORDS * sizeof(u32));
+ BUILD_BUG_ON(GUC_CTL_MAX_DWORDS + 2 != SOFT_SCRATCH_COUNT);
+
params[GUC_CTL_LOG_PARAMS] = guc_ctl_log_params_flags(guc);
params[GUC_CTL_FEATURE] = guc_ctl_feature_flags(guc);
params[GUC_CTL_DEBUG] = guc_ctl_debug_flags(guc);
@@ -277,6 +297,8 @@ int xe_guc_init(struct xe_guc *guc)
*/
int xe_guc_init_post_hwconfig(struct xe_guc *guc)
{
+ guc_init_params_post_hwconfig(guc);
+
return xe_guc_ads_init_post_hwconfig(&guc->ads);
}
--
2.43.0
^ permalink raw reply related [flat|nested] 32+ messages in thread* [Intel-xe] [PATCH v4 19/22] drm/managed: Add drmm_release_action
2023-11-29 1:16 [Intel-xe] [PATCH v4 00/22] drm/xe: Probe tweaks and reordering Michał Winiarski
` (17 preceding siblings ...)
2023-11-29 1:16 ` [Intel-xe] [PATCH v4 18/22] drm/xe/guc: Split GuC params used for "hwconfig" and "post-hwconfig" Michał Winiarski
@ 2023-11-29 1:16 ` Michał Winiarski
2023-11-29 9:43 ` Michal Wajdeczko
2023-11-29 1:16 ` [Intel-xe] [PATCH v4 20/22] drm/xe/guc: Allocate GuC data structures in system memory for initial load Michał Winiarski
` (3 subsequent siblings)
22 siblings, 1 reply; 32+ messages in thread
From: Michał Winiarski @ 2023-11-29 1:16 UTC (permalink / raw)
To: intel-xe; +Cc: Matt Roper, Lucas De Marchi, Michał Winiarski
Similar to devres equivalent, it allows to call the "release" action
directly and remove the resource from the managed resources list.
Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
---
drivers/gpu/drm/drm_managed.c | 28 ++++++++++++++++++++++++++++
include/drm/drm_managed.h | 4 ++++
2 files changed, 32 insertions(+)
diff --git a/drivers/gpu/drm/drm_managed.c b/drivers/gpu/drm/drm_managed.c
index bcd111404b128..f819940b8a959 100644
--- a/drivers/gpu/drm/drm_managed.c
+++ b/drivers/gpu/drm/drm_managed.c
@@ -176,6 +176,34 @@ int __drmm_add_action_or_reset(struct drm_device *dev,
}
EXPORT_SYMBOL(__drmm_add_action_or_reset);
+void drmm_release_action(struct drm_device *dev,
+ drmres_release_t action,
+ void *data)
+{
+ struct drmres *dr_match = NULL, *dr;
+ unsigned long flags;
+
+ spin_lock_irqsave(&dev->managed.lock, flags);
+ list_for_each_entry_reverse(dr, &dev->managed.resources, node.entry) {
+ if (dr->node.release == action) {
+ if (!data || (data && *(void **)dr->data == data)) {
+ dr_match = dr;
+ del_dr(dev, dr_match);
+ break;
+ }
+ }
+ }
+ spin_unlock_irqrestore(&dev->managed.lock, flags);
+
+ if (WARN_ON(!dr_match))
+ return;
+
+ action(dev, data);
+
+ free_dr(dr_match);
+}
+EXPORT_SYMBOL(drmm_release_action);
+
/**
* drmm_kmalloc - &drm_device managed kmalloc()
* @dev: DRM device
diff --git a/include/drm/drm_managed.h b/include/drm/drm_managed.h
index ad08f834af408..f547b09ca0239 100644
--- a/include/drm/drm_managed.h
+++ b/include/drm/drm_managed.h
@@ -45,6 +45,10 @@ int __must_check __drmm_add_action_or_reset(struct drm_device *dev,
drmres_release_t action,
void *data, const char *name);
+void drmm_release_action(struct drm_device *dev,
+ drmres_release_t action,
+ void *data);
+
void *drmm_kmalloc(struct drm_device *dev, size_t size, gfp_t gfp) __malloc;
/**
--
2.43.0
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [Intel-xe] [PATCH v4 19/22] drm/managed: Add drmm_release_action
2023-11-29 1:16 ` [Intel-xe] [PATCH v4 19/22] drm/managed: Add drmm_release_action Michał Winiarski
@ 2023-11-29 9:43 ` Michal Wajdeczko
2023-11-29 17:52 ` Rodrigo Vivi
0 siblings, 1 reply; 32+ messages in thread
From: Michal Wajdeczko @ 2023-11-29 9:43 UTC (permalink / raw)
To: Michał Winiarski, intel-xe; +Cc: Lucas De Marchi, Matt Roper
On 29.11.2023 02:16, Michał Winiarski wrote:
> Similar to devres equivalent, it allows to call the "release" action
> directly and remove the resource from the managed resources list.
>
> Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
> ---
> drivers/gpu/drm/drm_managed.c | 28 ++++++++++++++++++++++++++++
> include/drm/drm_managed.h | 4 ++++
> 2 files changed, 32 insertions(+)
shouldn't this go to the dri-devel first ?
>
> diff --git a/drivers/gpu/drm/drm_managed.c b/drivers/gpu/drm/drm_managed.c
> index bcd111404b128..f819940b8a959 100644
> --- a/drivers/gpu/drm/drm_managed.c
> +++ b/drivers/gpu/drm/drm_managed.c
> @@ -176,6 +176,34 @@ int __drmm_add_action_or_reset(struct drm_device *dev,
> }
> EXPORT_SYMBOL(__drmm_add_action_or_reset);
>
> +void drmm_release_action(struct drm_device *dev,
> + drmres_release_t action,
> + void *data)
> +{
> + struct drmres *dr_match = NULL, *dr;
> + unsigned long flags;
> +
> + spin_lock_irqsave(&dev->managed.lock, flags);
> + list_for_each_entry_reverse(dr, &dev->managed.resources, node.entry) {
> + if (dr->node.release == action) {
> + if (!data || (data && *(void **)dr->data == data)) {
> + dr_match = dr;
> + del_dr(dev, dr_match);
> + break;
> + }
> + }
> + }
> + spin_unlock_irqrestore(&dev->managed.lock, flags);
> +
> + if (WARN_ON(!dr_match))
> + return;
> +
> + action(dev, data);
> +
> + free_dr(dr_match);
> +}
> +EXPORT_SYMBOL(drmm_release_action);
> +
> /**
> * drmm_kmalloc - &drm_device managed kmalloc()
> * @dev: DRM device
> diff --git a/include/drm/drm_managed.h b/include/drm/drm_managed.h
> index ad08f834af408..f547b09ca0239 100644
> --- a/include/drm/drm_managed.h
> +++ b/include/drm/drm_managed.h
> @@ -45,6 +45,10 @@ int __must_check __drmm_add_action_or_reset(struct drm_device *dev,
> drmres_release_t action,
> void *data, const char *name);
>
> +void drmm_release_action(struct drm_device *dev,
> + drmres_release_t action,
> + void *data);
> +
> void *drmm_kmalloc(struct drm_device *dev, size_t size, gfp_t gfp) __malloc;
>
> /**
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [Intel-xe] [PATCH v4 19/22] drm/managed: Add drmm_release_action
2023-11-29 9:43 ` Michal Wajdeczko
@ 2023-11-29 17:52 ` Rodrigo Vivi
2023-11-29 22:17 ` Michał Winiarski
0 siblings, 1 reply; 32+ messages in thread
From: Rodrigo Vivi @ 2023-11-29 17:52 UTC (permalink / raw)
To: Michal Wajdeczko
Cc: Matt Roper, Lucas De Marchi, Michał Winiarski, intel-xe
On Wed, Nov 29, 2023 at 10:43:24AM +0100, Michal Wajdeczko wrote:
>
>
> On 29.11.2023 02:16, Michał Winiarski wrote:
> > Similar to devres equivalent, it allows to call the "release" action
> > directly and remove the resource from the managed resources list.
> >
> > Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
> > ---
> > drivers/gpu/drm/drm_managed.c | 28 ++++++++++++++++++++++++++++
> > include/drm/drm_managed.h | 4 ++++
> > 2 files changed, 32 insertions(+)
>
> shouldn't this go to the dri-devel first ?
yes, please. Let's send there and try to get that quickly through drm-misc,
or the proper reviews and acks there before getting to drm-xe-next or this
might block our initial pull request.
>
> >
> > diff --git a/drivers/gpu/drm/drm_managed.c b/drivers/gpu/drm/drm_managed.c
> > index bcd111404b128..f819940b8a959 100644
> > --- a/drivers/gpu/drm/drm_managed.c
> > +++ b/drivers/gpu/drm/drm_managed.c
> > @@ -176,6 +176,34 @@ int __drmm_add_action_or_reset(struct drm_device *dev,
> > }
> > EXPORT_SYMBOL(__drmm_add_action_or_reset);
> >
> > +void drmm_release_action(struct drm_device *dev,
> > + drmres_release_t action,
> > + void *data)
> > +{
> > + struct drmres *dr_match = NULL, *dr;
> > + unsigned long flags;
> > +
> > + spin_lock_irqsave(&dev->managed.lock, flags);
> > + list_for_each_entry_reverse(dr, &dev->managed.resources, node.entry) {
> > + if (dr->node.release == action) {
> > + if (!data || (data && *(void **)dr->data == data)) {
> > + dr_match = dr;
> > + del_dr(dev, dr_match);
> > + break;
> > + }
> > + }
> > + }
> > + spin_unlock_irqrestore(&dev->managed.lock, flags);
> > +
> > + if (WARN_ON(!dr_match))
> > + return;
> > +
> > + action(dev, data);
> > +
> > + free_dr(dr_match);
> > +}
> > +EXPORT_SYMBOL(drmm_release_action);
> > +
> > /**
> > * drmm_kmalloc - &drm_device managed kmalloc()
> > * @dev: DRM device
> > diff --git a/include/drm/drm_managed.h b/include/drm/drm_managed.h
> > index ad08f834af408..f547b09ca0239 100644
> > --- a/include/drm/drm_managed.h
> > +++ b/include/drm/drm_managed.h
> > @@ -45,6 +45,10 @@ int __must_check __drmm_add_action_or_reset(struct drm_device *dev,
> > drmres_release_t action,
> > void *data, const char *name);
> >
> > +void drmm_release_action(struct drm_device *dev,
> > + drmres_release_t action,
> > + void *data);
> > +
> > void *drmm_kmalloc(struct drm_device *dev, size_t size, gfp_t gfp) __malloc;
> >
> > /**
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [Intel-xe] [PATCH v4 19/22] drm/managed: Add drmm_release_action
2023-11-29 17:52 ` Rodrigo Vivi
@ 2023-11-29 22:17 ` Michał Winiarski
0 siblings, 0 replies; 32+ messages in thread
From: Michał Winiarski @ 2023-11-29 22:17 UTC (permalink / raw)
To: Rodrigo Vivi; +Cc: Matt Roper, Lucas De Marchi, intel-xe
On Wed, Nov 29, 2023 at 12:52:55PM -0500, Rodrigo Vivi wrote:
> On Wed, Nov 29, 2023 at 10:43:24AM +0100, Michal Wajdeczko wrote:
> >
> >
> > On 29.11.2023 02:16, Michał Winiarski wrote:
> > > Similar to devres equivalent, it allows to call the "release" action
> > > directly and remove the resource from the managed resources list.
> > >
> > > Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
> > > ---
> > > drivers/gpu/drm/drm_managed.c | 28 ++++++++++++++++++++++++++++
> > > include/drm/drm_managed.h | 4 ++++
> > > 2 files changed, 32 insertions(+)
> >
> > shouldn't this go to the dri-devel first ?
>
> yes, please. Let's send there and try to get that quickly through drm-misc,
> or the proper reviews and acks there before getting to drm-xe-next or this
> might block our initial pull request.
Posted this (+kerneldoc +kunit) to dri-devel:
https://lore.kernel.org/dri-devel/20231129221412.1180549-1-michal.winiarski@intel.com/
-Michał
>
> >
> > >
> > > diff --git a/drivers/gpu/drm/drm_managed.c b/drivers/gpu/drm/drm_managed.c
> > > index bcd111404b128..f819940b8a959 100644
> > > --- a/drivers/gpu/drm/drm_managed.c
> > > +++ b/drivers/gpu/drm/drm_managed.c
> > > @@ -176,6 +176,34 @@ int __drmm_add_action_or_reset(struct drm_device *dev,
> > > }
> > > EXPORT_SYMBOL(__drmm_add_action_or_reset);
> > >
> > > +void drmm_release_action(struct drm_device *dev,
> > > + drmres_release_t action,
> > > + void *data)
> > > +{
> > > + struct drmres *dr_match = NULL, *dr;
> > > + unsigned long flags;
> > > +
> > > + spin_lock_irqsave(&dev->managed.lock, flags);
> > > + list_for_each_entry_reverse(dr, &dev->managed.resources, node.entry) {
> > > + if (dr->node.release == action) {
> > > + if (!data || (data && *(void **)dr->data == data)) {
> > > + dr_match = dr;
> > > + del_dr(dev, dr_match);
> > > + break;
> > > + }
> > > + }
> > > + }
> > > + spin_unlock_irqrestore(&dev->managed.lock, flags);
> > > +
> > > + if (WARN_ON(!dr_match))
> > > + return;
> > > +
> > > + action(dev, data);
> > > +
> > > + free_dr(dr_match);
> > > +}
> > > +EXPORT_SYMBOL(drmm_release_action);
> > > +
> > > /**
> > > * drmm_kmalloc - &drm_device managed kmalloc()
> > > * @dev: DRM device
> > > diff --git a/include/drm/drm_managed.h b/include/drm/drm_managed.h
> > > index ad08f834af408..f547b09ca0239 100644
> > > --- a/include/drm/drm_managed.h
> > > +++ b/include/drm/drm_managed.h
> > > @@ -45,6 +45,10 @@ int __must_check __drmm_add_action_or_reset(struct drm_device *dev,
> > > drmres_release_t action,
> > > void *data, const char *name);
> > >
> > > +void drmm_release_action(struct drm_device *dev,
> > > + drmres_release_t action,
> > > + void *data);
> > > +
> > > void *drmm_kmalloc(struct drm_device *dev, size_t size, gfp_t gfp) __malloc;
> > >
> > > /**
^ permalink raw reply [flat|nested] 32+ messages in thread
* [Intel-xe] [PATCH v4 20/22] drm/xe/guc: Allocate GuC data structures in system memory for initial load
2023-11-29 1:16 [Intel-xe] [PATCH v4 00/22] drm/xe: Probe tweaks and reordering Michał Winiarski
` (18 preceding siblings ...)
2023-11-29 1:16 ` [Intel-xe] [PATCH v4 19/22] drm/managed: Add drmm_release_action Michał Winiarski
@ 2023-11-29 1:16 ` Michał Winiarski
2023-11-29 1:16 ` [Intel-xe] [PATCH v4 21/22] drm/xe/guc: Move GuC power control init to "post-hwconfig" Michał Winiarski
` (2 subsequent siblings)
22 siblings, 0 replies; 32+ messages in thread
From: Michał Winiarski @ 2023-11-29 1:16 UTC (permalink / raw)
To: intel-xe; +Cc: Matt Roper, Lucas De Marchi, Michał Winiarski
GuC load will need to happen at an earlier point in probe, where local
memory is not yet available. Use system memory for GuC data structures
used for initial "hwconfig" load, and realloc at a later,
"post-hwconfig" load if needed, when local memory is available.
Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
---
v3 -> v4:
- Assert that realloc transisions from RAM to VRAM to make sure
iosys_map API is used correctly (Lucas)
- Fix GuC resource realloc by introducing drmm_release_action
drivers/gpu/drm/xe/xe_bo.c | 5 +++
drivers/gpu/drm/xe/xe_bo.h | 1 +
drivers/gpu/drm/xe/xe_guc.c | 48 ++++++++++++++++++++++++++++
drivers/gpu/drm/xe/xe_guc_ads.c | 2 +-
drivers/gpu/drm/xe/xe_guc_ct.c | 2 +-
drivers/gpu/drm/xe/xe_guc_hwconfig.c | 2 +-
drivers/gpu/drm/xe/xe_guc_log.c | 2 +-
7 files changed, 58 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/xe/xe_bo.c b/drivers/gpu/drm/xe/xe_bo.c
index 7f5b616a7bbeb..0a6d782c90d41 100644
--- a/drivers/gpu/drm/xe/xe_bo.c
+++ b/drivers/gpu/drm/xe/xe_bo.c
@@ -1526,6 +1526,11 @@ struct xe_bo *xe_managed_bo_create_from_data(struct xe_device *xe, struct xe_til
return bo;
}
+void xe_managed_bo_release(struct xe_device *xe, struct xe_bo *bo)
+{
+ drmm_release_action(&xe->drm, __xe_bo_release, bo);
+}
+
/*
* XXX: This is in the VM bind data path, likely should calculate this once and
* store, with a recalculation if the BO is moved.
diff --git a/drivers/gpu/drm/xe/xe_bo.h b/drivers/gpu/drm/xe/xe_bo.h
index 2cf32713bde9e..772e448692b17 100644
--- a/drivers/gpu/drm/xe/xe_bo.h
+++ b/drivers/gpu/drm/xe/xe_bo.h
@@ -113,6 +113,7 @@ struct xe_bo *xe_managed_bo_create_pin_map(struct xe_device *xe, struct xe_tile
size_t size, u32 flags);
struct xe_bo *xe_managed_bo_create_from_data(struct xe_device *xe, struct xe_tile *tile,
const void *data, size_t size, u32 flags);
+void xe_managed_bo_release(struct xe_device *xe, struct xe_bo *bo);
int xe_bo_placement_for_flags(struct xe_device *xe, struct xe_bo *bo,
u32 bo_flags);
diff --git a/drivers/gpu/drm/xe/xe_guc.c b/drivers/gpu/drm/xe/xe_guc.c
index fd3396ebd3edf..36457477616a6 100644
--- a/drivers/gpu/drm/xe/xe_guc.c
+++ b/drivers/gpu/drm/xe/xe_guc.c
@@ -239,6 +239,48 @@ static void guc_fini(struct drm_device *drm, void *arg)
xe_force_wake_put(gt_to_fw(guc_to_gt(guc)), XE_FORCEWAKE_ALL);
}
+static int __guc_bo_reinit(struct xe_guc *guc, struct xe_bo **src)
+{
+ struct xe_tile *tile = gt_to_tile(guc_to_gt(guc));
+ struct xe_device *xe = guc_to_xe(guc);
+ struct xe_bo *bo;
+
+ xe_assert(xe, !(*src)->vmap.is_iomem);
+
+ bo = xe_managed_bo_create_from_data(xe, tile, &(*src)->vmap.vaddr, (*src)->size,
+ XE_BO_CREATE_VRAM_IF_DGFX(tile) |
+ XE_BO_CREATE_GGTT_BIT);
+ if (IS_ERR(bo))
+ return PTR_ERR(bo);
+
+ xe_managed_bo_release(xe, *src);
+ *src = bo;
+
+ return 0;
+}
+
+static int xe_guc_realloc_post_hwconfig(struct xe_guc *guc)
+{
+ int ret;
+
+ if (!IS_DGFX(guc_to_xe(guc)))
+ return 0;
+
+ ret = __guc_bo_reinit(guc, &guc->log.bo);
+ if (ret)
+ return ret;
+
+ ret = __guc_bo_reinit(guc, &guc->ads.bo);
+ if (ret)
+ return ret;
+
+ ret = __guc_bo_reinit(guc, &guc->ct.bo);
+ if (ret)
+ return ret;
+
+ return 0;
+}
+
int xe_guc_init(struct xe_guc *guc)
{
struct xe_device *xe = guc_to_xe(guc);
@@ -297,6 +339,12 @@ int xe_guc_init(struct xe_guc *guc)
*/
int xe_guc_init_post_hwconfig(struct xe_guc *guc)
{
+ int ret;
+
+ ret = xe_guc_realloc_post_hwconfig(guc);
+ if (ret)
+ return ret;
+
guc_init_params_post_hwconfig(guc);
return xe_guc_ads_init_post_hwconfig(&guc->ads);
diff --git a/drivers/gpu/drm/xe/xe_guc_ads.c b/drivers/gpu/drm/xe/xe_guc_ads.c
index 2f5ff090aa6bd..61b443981f99a 100644
--- a/drivers/gpu/drm/xe/xe_guc_ads.c
+++ b/drivers/gpu/drm/xe/xe_guc_ads.c
@@ -272,7 +272,7 @@ int xe_guc_ads_init(struct xe_guc_ads *ads)
ads->regset_size = calculate_regset_size(gt);
bo = xe_managed_bo_create_pin_map(xe, tile, guc_ads_size(ads) + MAX_GOLDEN_LRC_SIZE,
- XE_BO_CREATE_VRAM_IF_DGFX(tile) |
+ XE_BO_CREATE_SYSTEM_BIT |
XE_BO_CREATE_GGTT_BIT);
if (IS_ERR(bo))
return PTR_ERR(bo);
diff --git a/drivers/gpu/drm/xe/xe_guc_ct.c b/drivers/gpu/drm/xe/xe_guc_ct.c
index 93c886ef6fdb4..9f5761cdaaeee 100644
--- a/drivers/gpu/drm/xe/xe_guc_ct.c
+++ b/drivers/gpu/drm/xe/xe_guc_ct.c
@@ -146,7 +146,7 @@ int xe_guc_ct_init(struct xe_guc_ct *ct)
primelockdep(ct);
bo = xe_managed_bo_create_pin_map(xe, tile, guc_ct_size(),
- XE_BO_CREATE_VRAM_IF_DGFX(tile) |
+ XE_BO_CREATE_SYSTEM_BIT |
XE_BO_CREATE_GGTT_BIT);
if (IS_ERR(bo))
return PTR_ERR(bo);
diff --git a/drivers/gpu/drm/xe/xe_guc_hwconfig.c b/drivers/gpu/drm/xe/xe_guc_hwconfig.c
index 8e3db5d7192c3..ca59a0d8df825 100644
--- a/drivers/gpu/drm/xe/xe_guc_hwconfig.c
+++ b/drivers/gpu/drm/xe/xe_guc_hwconfig.c
@@ -77,7 +77,7 @@ int xe_guc_hwconfig_init(struct xe_guc *guc)
return -EINVAL;
bo = xe_managed_bo_create_pin_map(xe, tile, PAGE_ALIGN(size),
- XE_BO_CREATE_VRAM_IF_DGFX(tile) |
+ XE_BO_CREATE_SYSTEM_BIT |
XE_BO_CREATE_GGTT_BIT);
if (IS_ERR(bo))
return PTR_ERR(bo);
diff --git a/drivers/gpu/drm/xe/xe_guc_log.c b/drivers/gpu/drm/xe/xe_guc_log.c
index bcd2f4d34081d..45135c3520e54 100644
--- a/drivers/gpu/drm/xe/xe_guc_log.c
+++ b/drivers/gpu/drm/xe/xe_guc_log.c
@@ -84,7 +84,7 @@ int xe_guc_log_init(struct xe_guc_log *log)
struct xe_bo *bo;
bo = xe_managed_bo_create_pin_map(xe, tile, guc_log_size(),
- XE_BO_CREATE_VRAM_IF_DGFX(tile) |
+ XE_BO_CREATE_SYSTEM_BIT |
XE_BO_CREATE_GGTT_BIT);
if (IS_ERR(bo))
return PTR_ERR(bo);
--
2.43.0
^ permalink raw reply related [flat|nested] 32+ messages in thread* [Intel-xe] [PATCH v4 21/22] drm/xe/guc: Move GuC power control init to "post-hwconfig"
2023-11-29 1:16 [Intel-xe] [PATCH v4 00/22] drm/xe: Probe tweaks and reordering Michał Winiarski
` (19 preceding siblings ...)
2023-11-29 1:16 ` [Intel-xe] [PATCH v4 20/22] drm/xe/guc: Allocate GuC data structures in system memory for initial load Michał Winiarski
@ 2023-11-29 1:16 ` Michał Winiarski
2023-11-29 1:16 ` [Intel-xe] [PATCH v4 22/22] drm/xe: Initialize GuC earlier during probe Michał Winiarski
2023-11-29 4:48 ` [Intel-xe] ✗ CI.Patch_applied: failure for drm/xe: Probe tweaks and reordering (rev3) Patchwork
22 siblings, 0 replies; 32+ messages in thread
From: Michał Winiarski @ 2023-11-29 1:16 UTC (permalink / raw)
To: intel-xe; +Cc: Matt Roper, Lucas De Marchi, Michał Winiarski
SLPC is not used at "hwconfig" stage. Move the initialization of data
structures used for SLPC to a later point in probe.
Also - move the xe_guc_pc_init_early to happen just prior to initial
"hwconfig" load.
Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
---
v3 -> v4:
- Convert xe_guc_pc_fini to act as drm-managed release function
drivers/gpu/drm/xe/xe_gt.c | 3 ---
drivers/gpu/drm/xe/xe_guc.c | 14 ++++++++------
drivers/gpu/drm/xe/xe_guc_pc.c | 17 ++++++++++++++---
drivers/gpu/drm/xe/xe_guc_pc.h | 1 -
4 files changed, 22 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/xe/xe_gt.c b/drivers/gpu/drm/xe/xe_gt.c
index 9eaab11cf29a6..964e76f931a28 100644
--- a/drivers/gpu/drm/xe/xe_gt.c
+++ b/drivers/gpu/drm/xe/xe_gt.c
@@ -348,9 +348,6 @@ static int gt_fw_domain_init(struct xe_gt *gt)
if (err)
goto err_force_wake;
- /* Raise GT freq to speed up HuC/GuC load */
- xe_guc_pc_init_early(>->uc.guc.pc);
-
err = xe_uc_init_hwconfig(>->uc);
if (err)
goto err_force_wake;
diff --git a/drivers/gpu/drm/xe/xe_guc.c b/drivers/gpu/drm/xe/xe_guc.c
index 36457477616a6..6777269f6a9bb 100644
--- a/drivers/gpu/drm/xe/xe_guc.c
+++ b/drivers/gpu/drm/xe/xe_guc.c
@@ -234,7 +234,6 @@ static void guc_fini(struct drm_device *drm, void *arg)
struct xe_guc *guc = arg;
xe_force_wake_get(gt_to_fw(guc_to_gt(guc)), XE_FORCEWAKE_ALL);
- xe_guc_pc_fini(&guc->pc);
xe_uc_fini_hw(&guc_to_gt(guc)->uc);
xe_force_wake_put(gt_to_fw(guc_to_gt(guc)), XE_FORCEWAKE_ALL);
}
@@ -307,11 +306,7 @@ int xe_guc_init(struct xe_guc *guc)
if (ret)
goto out;
- ret = xe_guc_pc_init(&guc->pc);
- if (ret)
- goto out;
-
- ret = drmm_add_action_or_reset(>_to_xe(gt)->drm, guc_fini, guc);
+ ret = drmm_add_action_or_reset(&xe->drm, guc_fini, guc);
if (ret)
goto out;
@@ -347,6 +342,10 @@ int xe_guc_init_post_hwconfig(struct xe_guc *guc)
guc_init_params_post_hwconfig(guc);
+ ret = xe_guc_pc_init(&guc->pc);
+ if (ret)
+ return ret;
+
return xe_guc_ads_init_post_hwconfig(&guc->ads);
}
@@ -557,6 +556,9 @@ int xe_guc_min_load_for_hwconfig(struct xe_guc *guc)
xe_guc_ads_populate_minimal(&guc->ads);
+ /* Raise GT freq to speed up HuC/GuC load */
+ xe_guc_pc_init_early(&guc->pc);
+
ret = __xe_guc_upload(guc);
if (ret)
return ret;
diff --git a/drivers/gpu/drm/xe/xe_guc_pc.c b/drivers/gpu/drm/xe/xe_guc_pc.c
index f5009a7968afa..4dbc89e91bbd4 100644
--- a/drivers/gpu/drm/xe/xe_guc_pc.c
+++ b/drivers/gpu/drm/xe/xe_guc_pc.c
@@ -931,12 +931,15 @@ int xe_guc_pc_stop(struct xe_guc_pc *pc)
return ret;
}
-void xe_guc_pc_fini(struct xe_guc_pc *pc)
+static void xe_guc_pc_fini(struct drm_device *drm, void *arg)
{
+ struct xe_guc_pc *pc = arg;
+
+ 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));
sysfs_remove_files(pc_to_gt(pc)->sysfs, pc_attrs);
- mutex_destroy(&pc->freq_lock);
+ xe_force_wake_put(gt_to_fw(pc_to_gt(pc)), XE_FORCEWAKE_ALL);
}
/**
@@ -952,7 +955,9 @@ int xe_guc_pc_init(struct xe_guc_pc *pc)
u32 size = PAGE_ALIGN(sizeof(struct slpc_shared_data));
int err;
- mutex_init(&pc->freq_lock);
+ err = drmm_mutex_init(&xe->drm, &pc->freq_lock);
+ if (err)
+ return err;
bo = xe_managed_bo_create_pin_map(xe, tile, size,
XE_BO_CREATE_VRAM_IF_DGFX(tile) |
@@ -966,5 +971,11 @@ int xe_guc_pc_init(struct xe_guc_pc *pc)
if (err)
return err;
+ err = drmm_add_action_or_reset(&xe->drm, xe_guc_pc_fini, pc);
+ if (err) {
+ sysfs_remove_files(gt->sysfs, pc_attrs);
+ return err;
+ }
+
return 0;
}
diff --git a/drivers/gpu/drm/xe/xe_guc_pc.h b/drivers/gpu/drm/xe/xe_guc_pc.h
index 054788e006f32..4cd023d9e8bec 100644
--- a/drivers/gpu/drm/xe/xe_guc_pc.h
+++ b/drivers/gpu/drm/xe/xe_guc_pc.h
@@ -9,7 +9,6 @@
#include "xe_guc_pc_types.h"
int xe_guc_pc_init(struct xe_guc_pc *pc);
-void xe_guc_pc_fini(struct xe_guc_pc *pc);
int xe_guc_pc_start(struct xe_guc_pc *pc);
int xe_guc_pc_stop(struct xe_guc_pc *pc);
int xe_guc_pc_gucrc_disable(struct xe_guc_pc *pc);
--
2.43.0
^ permalink raw reply related [flat|nested] 32+ messages in thread* [Intel-xe] [PATCH v4 22/22] drm/xe: Initialize GuC earlier during probe
2023-11-29 1:16 [Intel-xe] [PATCH v4 00/22] drm/xe: Probe tweaks and reordering Michał Winiarski
` (20 preceding siblings ...)
2023-11-29 1:16 ` [Intel-xe] [PATCH v4 21/22] drm/xe/guc: Move GuC power control init to "post-hwconfig" Michał Winiarski
@ 2023-11-29 1:16 ` Michał Winiarski
2023-11-29 21:48 ` Welty, Brian
2023-11-29 4:48 ` [Intel-xe] ✗ CI.Patch_applied: failure for drm/xe: Probe tweaks and reordering (rev3) Patchwork
22 siblings, 1 reply; 32+ messages in thread
From: Michał Winiarski @ 2023-11-29 1:16 UTC (permalink / raw)
To: intel-xe; +Cc: Matt Roper, Lucas De Marchi, Michał Winiarski
SR-IOV VF has limited access to MMIO registers. Fortunately, it is able
to access a curated subset that is needed to initialize the driver by
communicating with SR-IOV PF using GuC CT.
Initialize GuC earlier in order to keep the unified probe ordering
between VF and PF modes.
Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
---
v3 -> v4:
- Introduce xe_gt_init_hwconfig (Matt Brost)
- Init PAT before loading GuC
drivers/gpu/drm/xe/xe_device.c | 6 ++++
drivers/gpu/drm/xe/xe_gt.c | 51 ++++++++++++++++++++++++----------
drivers/gpu/drm/xe/xe_gt.h | 1 +
drivers/gpu/drm/xe/xe_uc.c | 11 ++++++--
4 files changed, 52 insertions(+), 17 deletions(-)
diff --git a/drivers/gpu/drm/xe/xe_device.c b/drivers/gpu/drm/xe/xe_device.c
index 65e9aa5e6c31e..a236c36cdae3c 100644
--- a/drivers/gpu/drm/xe/xe_device.c
+++ b/drivers/gpu/drm/xe/xe_device.c
@@ -425,6 +425,12 @@ int xe_device_probe(struct xe_device *xe)
return err;
}
+ for_each_gt(gt, xe, id) {
+ err = xe_gt_init_hwconfig(gt);
+ if (err)
+ return err;
+ }
+
err = drmm_add_action_or_reset(&xe->drm, xe_driver_flr_fini, xe);
if (err)
return err;
diff --git a/drivers/gpu/drm/xe/xe_gt.c b/drivers/gpu/drm/xe/xe_gt.c
index 964e76f931a28..d2766c2c58f0f 100644
--- a/drivers/gpu/drm/xe/xe_gt.c
+++ b/drivers/gpu/drm/xe/xe_gt.c
@@ -297,7 +297,6 @@ int xe_gt_init_early(struct xe_gt *gt)
return err;
xe_gt_topology_init(gt);
- xe_gt_mcr_init(gt);
err = xe_force_wake_put(gt_to_fw(gt), XE_FW_GT);
if (err)
@@ -336,22 +335,12 @@ static int gt_fw_domain_init(struct xe_gt *gt)
if (err)
goto err_hw_fence_irq;
- xe_pat_init(gt);
-
if (!xe_gt_is_media_type(gt)) {
err = xe_ggtt_init(gt_to_tile(gt)->mem.ggtt);
if (err)
goto err_force_wake;
}
- err = xe_uc_init(>->uc);
- if (err)
- goto err_force_wake;
-
- err = xe_uc_init_hwconfig(>->uc);
- if (err)
- goto err_force_wake;
-
xe_gt_idle_sysfs_init(>->gtidle);
/* XXX: Fake that we pull the engine mask from hwconfig blob */
@@ -415,10 +404,6 @@ static int all_fw_domain_init(struct xe_gt *gt)
if (err)
goto err_force_wake;
- err = xe_uc_init_post_hwconfig(>->uc);
- if (err)
- goto err_force_wake;
-
if (!xe_gt_is_media_type(gt)) {
/*
* USM has its only SA pool to non-block behind user operations
@@ -442,6 +427,10 @@ static int all_fw_domain_init(struct xe_gt *gt)
}
}
+ err = xe_uc_init_post_hwconfig(>->uc);
+ if (err)
+ goto err_force_wake;
+
err = xe_uc_init_hw(>->uc);
if (err)
goto err_force_wake;
@@ -462,6 +451,38 @@ static int all_fw_domain_init(struct xe_gt *gt)
return err;
}
+/*
+ * Initialize enough GT to be able to load GuC in order to obtain hwconfig and enable CTB
+ * communication.
+ */
+int xe_gt_init_hwconfig(struct xe_gt *gt)
+{
+ int err;
+
+ xe_device_mem_access_get(gt_to_xe(gt));
+ err = xe_force_wake_get(gt_to_fw(gt), XE_FW_GT);
+ if (err)
+ goto out;
+
+ xe_gt_mcr_init(gt);
+ xe_pat_init(gt);
+
+ err = xe_uc_init(>->uc);
+ if (err)
+ goto out_fw;
+
+ err = xe_uc_init_hwconfig(>->uc);
+ if (err)
+ goto out_fw;
+
+out_fw:
+ xe_force_wake_put(gt_to_fw(gt), XE_FW_GT);
+out:
+ xe_device_mem_access_put(gt_to_xe(gt));
+
+ return err;
+}
+
int xe_gt_init(struct xe_gt *gt)
{
int err;
diff --git a/drivers/gpu/drm/xe/xe_gt.h b/drivers/gpu/drm/xe/xe_gt.h
index caded203a8a03..e9d6aeceb56af 100644
--- a/drivers/gpu/drm/xe/xe_gt.h
+++ b/drivers/gpu/drm/xe/xe_gt.h
@@ -31,6 +31,7 @@ static inline bool xe_fault_inject_gt_reset(void)
#endif
struct xe_gt *xe_gt_alloc(struct xe_tile *tile);
+int xe_gt_init_hwconfig(struct xe_gt *gt);
int xe_gt_init_early(struct xe_gt *gt);
int xe_gt_init(struct xe_gt *gt);
int xe_gt_record_default_lrcs(struct xe_gt *gt);
diff --git a/drivers/gpu/drm/xe/xe_uc.c b/drivers/gpu/drm/xe/xe_uc.c
index 13e76e6805ca1..123baafc9ec38 100644
--- a/drivers/gpu/drm/xe/xe_uc.c
+++ b/drivers/gpu/drm/xe/xe_uc.c
@@ -29,13 +29,15 @@ uc_to_xe(struct xe_uc *uc)
/* Should be called once at driver load only */
int xe_uc_init(struct xe_uc *uc)
{
+ struct xe_device *xe = uc_to_xe(uc);
int ret;
+ xe_device_mem_access_get(xe);
+
/*
* We call the GuC/HuC init functions even if GuC submission is off to
* correctly move our tracking of the FW state to "disabled".
*/
-
ret = xe_guc_init(&uc->guc);
if (ret)
goto err;
@@ -45,7 +47,7 @@ int xe_uc_init(struct xe_uc *uc)
goto err;
if (!xe_device_uc_enabled(uc_to_xe(uc)))
- return 0;
+ goto out;
ret = xe_wopcm_init(&uc->wopcm);
if (ret)
@@ -55,9 +57,14 @@ int xe_uc_init(struct xe_uc *uc)
if (ret)
goto err;
+out:
+ xe_device_mem_access_put(xe);
+
return 0;
err:
+ xe_device_mem_access_put(xe);
+
return ret;
}
--
2.43.0
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [Intel-xe] [PATCH v4 22/22] drm/xe: Initialize GuC earlier during probe
2023-11-29 1:16 ` [Intel-xe] [PATCH v4 22/22] drm/xe: Initialize GuC earlier during probe Michał Winiarski
@ 2023-11-29 21:48 ` Welty, Brian
2023-11-29 22:20 ` Michał Winiarski
0 siblings, 1 reply; 32+ messages in thread
From: Welty, Brian @ 2023-11-29 21:48 UTC (permalink / raw)
To: Michał Winiarski, intel-xe; +Cc: Lucas De Marchi, Matt Roper
On 11/28/2023 5:16 PM, Michał Winiarski wrote:
> SR-IOV VF has limited access to MMIO registers. Fortunately, it is able
> to access a curated subset that is needed to initialize the driver by
> communicating with SR-IOV PF using GuC CT.
> Initialize GuC earlier in order to keep the unified probe ordering
> between VF and PF modes.
>
> Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
> ---
> v3 -> v4:
> - Introduce xe_gt_init_hwconfig (Matt Brost)
> - Init PAT before loading GuC
>
> drivers/gpu/drm/xe/xe_device.c | 6 ++++
> drivers/gpu/drm/xe/xe_gt.c | 51 ++++++++++++++++++++++++----------
> drivers/gpu/drm/xe/xe_gt.h | 1 +
> drivers/gpu/drm/xe/xe_uc.c | 11 ++++++--
> 4 files changed, 52 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/xe/xe_device.c b/drivers/gpu/drm/xe/xe_device.c
> index 65e9aa5e6c31e..a236c36cdae3c 100644
> --- a/drivers/gpu/drm/xe/xe_device.c
> +++ b/drivers/gpu/drm/xe/xe_device.c
> @@ -425,6 +425,12 @@ int xe_device_probe(struct xe_device *xe)
> return err;
> }
>
> + for_each_gt(gt, xe, id) {
> + err = xe_gt_init_hwconfig(gt);
> + if (err)
> + return err;
> + }
> +
> err = drmm_add_action_or_reset(&xe->drm, xe_driver_flr_fini, xe);
> if (err)
> return err;
> diff --git a/drivers/gpu/drm/xe/xe_gt.c b/drivers/gpu/drm/xe/xe_gt.c
> index 964e76f931a28..d2766c2c58f0f 100644
> --- a/drivers/gpu/drm/xe/xe_gt.c
> +++ b/drivers/gpu/drm/xe/xe_gt.c
> @@ -297,7 +297,6 @@ int xe_gt_init_early(struct xe_gt *gt)
> return err;
>
> xe_gt_topology_init(gt);
> - xe_gt_mcr_init(gt);
>
> err = xe_force_wake_put(gt_to_fw(gt), XE_FW_GT);
> if (err)
> @@ -336,22 +335,12 @@ static int gt_fw_domain_init(struct xe_gt *gt)
> if (err)
> goto err_hw_fence_irq;
>
> - xe_pat_init(gt);
> -
> if (!xe_gt_is_media_type(gt)) {
> err = xe_ggtt_init(gt_to_tile(gt)->mem.ggtt);
> if (err)
> goto err_force_wake;
> }
>
> - err = xe_uc_init(>->uc);
> - if (err)
> - goto err_force_wake;
> -
> - err = xe_uc_init_hwconfig(>->uc);
> - if (err)
> - goto err_force_wake;
> -
> xe_gt_idle_sysfs_init(>->gtidle);
>
> /* XXX: Fake that we pull the engine mask from hwconfig blob */
> gt->info.engine_mask = gt->info.__engine_mask;
I've been wondering about the XXX above. I added the line above that
comes next.
Should that move into xe_gt_init_hwconfig() as well?
And replace it with code to read it from hwconfig blob?
I'm asking because I was looking at a change that would like to make use
of a valid engine_mask earlier than xe_gt_init().
-Brian
> @@ -415,10 +404,6 @@ static int all_fw_domain_init(struct xe_gt *gt)
> if (err)
> goto err_force_wake;
>
> - err = xe_uc_init_post_hwconfig(>->uc);
> - if (err)
> - goto err_force_wake;
> -
> if (!xe_gt_is_media_type(gt)) {
> /*
> * USM has its only SA pool to non-block behind user operations
> @@ -442,6 +427,10 @@ static int all_fw_domain_init(struct xe_gt *gt)
> }
> }
>
> + err = xe_uc_init_post_hwconfig(>->uc);
> + if (err)
> + goto err_force_wake;
> +
> err = xe_uc_init_hw(>->uc);
> if (err)
> goto err_force_wake;
> @@ -462,6 +451,38 @@ static int all_fw_domain_init(struct xe_gt *gt)
> return err;
> }
>
> +/*
> + * Initialize enough GT to be able to load GuC in order to obtain hwconfig and enable CTB
> + * communication.
> + */
> +int xe_gt_init_hwconfig(struct xe_gt *gt)
> +{
> + int err;
> +
> + xe_device_mem_access_get(gt_to_xe(gt));
> + err = xe_force_wake_get(gt_to_fw(gt), XE_FW_GT);
> + if (err)
> + goto out;
> +
> + xe_gt_mcr_init(gt);
> + xe_pat_init(gt);
> +
> + err = xe_uc_init(>->uc);
> + if (err)
> + goto out_fw;
> +
> + err = xe_uc_init_hwconfig(>->uc);
> + if (err)
> + goto out_fw;
> +
> +out_fw:
> + xe_force_wake_put(gt_to_fw(gt), XE_FW_GT);
> +out:
> + xe_device_mem_access_put(gt_to_xe(gt));
> +
> + return err;
> +}
> +
> int xe_gt_init(struct xe_gt *gt)
> {
> int err;
> diff --git a/drivers/gpu/drm/xe/xe_gt.h b/drivers/gpu/drm/xe/xe_gt.h
> index caded203a8a03..e9d6aeceb56af 100644
> --- a/drivers/gpu/drm/xe/xe_gt.h
> +++ b/drivers/gpu/drm/xe/xe_gt.h
> @@ -31,6 +31,7 @@ static inline bool xe_fault_inject_gt_reset(void)
> #endif
>
> struct xe_gt *xe_gt_alloc(struct xe_tile *tile);
> +int xe_gt_init_hwconfig(struct xe_gt *gt);
> int xe_gt_init_early(struct xe_gt *gt);
> int xe_gt_init(struct xe_gt *gt);
> int xe_gt_record_default_lrcs(struct xe_gt *gt);
> diff --git a/drivers/gpu/drm/xe/xe_uc.c b/drivers/gpu/drm/xe/xe_uc.c
> index 13e76e6805ca1..123baafc9ec38 100644
> --- a/drivers/gpu/drm/xe/xe_uc.c
> +++ b/drivers/gpu/drm/xe/xe_uc.c
> @@ -29,13 +29,15 @@ uc_to_xe(struct xe_uc *uc)
> /* Should be called once at driver load only */
> int xe_uc_init(struct xe_uc *uc)
> {
> + struct xe_device *xe = uc_to_xe(uc);
> int ret;
>
> + xe_device_mem_access_get(xe);
> +
> /*
> * We call the GuC/HuC init functions even if GuC submission is off to
> * correctly move our tracking of the FW state to "disabled".
> */
> -
> ret = xe_guc_init(&uc->guc);
> if (ret)
> goto err;
> @@ -45,7 +47,7 @@ int xe_uc_init(struct xe_uc *uc)
> goto err;
>
> if (!xe_device_uc_enabled(uc_to_xe(uc)))
> - return 0;
> + goto out;
>
> ret = xe_wopcm_init(&uc->wopcm);
> if (ret)
> @@ -55,9 +57,14 @@ int xe_uc_init(struct xe_uc *uc)
> if (ret)
> goto err;
>
> +out:
> + xe_device_mem_access_put(xe);
> +
> return 0;
>
> err:
> + xe_device_mem_access_put(xe);
> +
> return ret;
> }
>
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [Intel-xe] [PATCH v4 22/22] drm/xe: Initialize GuC earlier during probe
2023-11-29 21:48 ` Welty, Brian
@ 2023-11-29 22:20 ` Michał Winiarski
0 siblings, 0 replies; 32+ messages in thread
From: Michał Winiarski @ 2023-11-29 22:20 UTC (permalink / raw)
To: Welty, Brian; +Cc: Lucas De Marchi, Matt Roper, intel-xe
On Wed, Nov 29, 2023 at 01:48:50PM -0800, Welty, Brian wrote:
>
> On 11/28/2023 5:16 PM, Michał Winiarski wrote:
> > SR-IOV VF has limited access to MMIO registers. Fortunately, it is able
> > to access a curated subset that is needed to initialize the driver by
> > communicating with SR-IOV PF using GuC CT.
> > Initialize GuC earlier in order to keep the unified probe ordering
> > between VF and PF modes.
> >
> > Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
> > ---
> > v3 -> v4:
> > - Introduce xe_gt_init_hwconfig (Matt Brost)
> > - Init PAT before loading GuC
> >
> > drivers/gpu/drm/xe/xe_device.c | 6 ++++
> > drivers/gpu/drm/xe/xe_gt.c | 51 ++++++++++++++++++++++++----------
> > drivers/gpu/drm/xe/xe_gt.h | 1 +
> > drivers/gpu/drm/xe/xe_uc.c | 11 ++++++--
> > 4 files changed, 52 insertions(+), 17 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/xe/xe_device.c b/drivers/gpu/drm/xe/xe_device.c
> > index 65e9aa5e6c31e..a236c36cdae3c 100644
> > --- a/drivers/gpu/drm/xe/xe_device.c
> > +++ b/drivers/gpu/drm/xe/xe_device.c
> > @@ -425,6 +425,12 @@ int xe_device_probe(struct xe_device *xe)
> > return err;
> > }
> > + for_each_gt(gt, xe, id) {
> > + err = xe_gt_init_hwconfig(gt);
> > + if (err)
> > + return err;
> > + }
> > +
> > err = drmm_add_action_or_reset(&xe->drm, xe_driver_flr_fini, xe);
> > if (err)
> > return err;
> > diff --git a/drivers/gpu/drm/xe/xe_gt.c b/drivers/gpu/drm/xe/xe_gt.c
> > index 964e76f931a28..d2766c2c58f0f 100644
> > --- a/drivers/gpu/drm/xe/xe_gt.c
> > +++ b/drivers/gpu/drm/xe/xe_gt.c
> > @@ -297,7 +297,6 @@ int xe_gt_init_early(struct xe_gt *gt)
> > return err;
> > xe_gt_topology_init(gt);
> > - xe_gt_mcr_init(gt);
> > err = xe_force_wake_put(gt_to_fw(gt), XE_FW_GT);
> > if (err)
> > @@ -336,22 +335,12 @@ static int gt_fw_domain_init(struct xe_gt *gt)
> > if (err)
> > goto err_hw_fence_irq;
> > - xe_pat_init(gt);
> > -
> > if (!xe_gt_is_media_type(gt)) {
> > err = xe_ggtt_init(gt_to_tile(gt)->mem.ggtt);
> > if (err)
> > goto err_force_wake;
> > }
> > - err = xe_uc_init(>->uc);
> > - if (err)
> > - goto err_force_wake;
> > -
> > - err = xe_uc_init_hwconfig(>->uc);
> > - if (err)
> > - goto err_force_wake;
> > -
> > xe_gt_idle_sysfs_init(>->gtidle);
> > /* XXX: Fake that we pull the engine mask from hwconfig blob */
> > gt->info.engine_mask = gt->info.__engine_mask;
>
>
> I've been wondering about the XXX above. I added the line above that comes
> next.
> Should that move into xe_gt_init_hwconfig() as well?
> And replace it with code to read it from hwconfig blob?
>
> I'm asking because I was looking at a change that would like to make use of
> a valid engine_mask earlier than xe_gt_init().
Yeah - makes sense, I'll move it to xe_gt_init_hwconfig().
Is that point in the codebase early enough for the change that you're
referring to?
-Michał
>
> -Brian
>
>
>
> > @@ -415,10 +404,6 @@ static int all_fw_domain_init(struct xe_gt *gt)
> > if (err)
> > goto err_force_wake;
> > - err = xe_uc_init_post_hwconfig(>->uc);
> > - if (err)
> > - goto err_force_wake;
> > -
> > if (!xe_gt_is_media_type(gt)) {
> > /*
> > * USM has its only SA pool to non-block behind user operations
> > @@ -442,6 +427,10 @@ static int all_fw_domain_init(struct xe_gt *gt)
> > }
> > }
> > + err = xe_uc_init_post_hwconfig(>->uc);
> > + if (err)
> > + goto err_force_wake;
> > +
> > err = xe_uc_init_hw(>->uc);
> > if (err)
> > goto err_force_wake;
> > @@ -462,6 +451,38 @@ static int all_fw_domain_init(struct xe_gt *gt)
> > return err;
> > }
> > +/*
> > + * Initialize enough GT to be able to load GuC in order to obtain hwconfig and enable CTB
> > + * communication.
> > + */
> > +int xe_gt_init_hwconfig(struct xe_gt *gt)
> > +{
> > + int err;
> > +
> > + xe_device_mem_access_get(gt_to_xe(gt));
> > + err = xe_force_wake_get(gt_to_fw(gt), XE_FW_GT);
> > + if (err)
> > + goto out;
> > +
> > + xe_gt_mcr_init(gt);
> > + xe_pat_init(gt);
> > +
> > + err = xe_uc_init(>->uc);
> > + if (err)
> > + goto out_fw;
> > +
> > + err = xe_uc_init_hwconfig(>->uc);
> > + if (err)
> > + goto out_fw;
> > +
> > +out_fw:
> > + xe_force_wake_put(gt_to_fw(gt), XE_FW_GT);
> > +out:
> > + xe_device_mem_access_put(gt_to_xe(gt));
> > +
> > + return err;
> > +}
> > +
> > int xe_gt_init(struct xe_gt *gt)
> > {
> > int err;
> > diff --git a/drivers/gpu/drm/xe/xe_gt.h b/drivers/gpu/drm/xe/xe_gt.h
> > index caded203a8a03..e9d6aeceb56af 100644
> > --- a/drivers/gpu/drm/xe/xe_gt.h
> > +++ b/drivers/gpu/drm/xe/xe_gt.h
> > @@ -31,6 +31,7 @@ static inline bool xe_fault_inject_gt_reset(void)
> > #endif
> > struct xe_gt *xe_gt_alloc(struct xe_tile *tile);
> > +int xe_gt_init_hwconfig(struct xe_gt *gt);
> > int xe_gt_init_early(struct xe_gt *gt);
> > int xe_gt_init(struct xe_gt *gt);
> > int xe_gt_record_default_lrcs(struct xe_gt *gt);
> > diff --git a/drivers/gpu/drm/xe/xe_uc.c b/drivers/gpu/drm/xe/xe_uc.c
> > index 13e76e6805ca1..123baafc9ec38 100644
> > --- a/drivers/gpu/drm/xe/xe_uc.c
> > +++ b/drivers/gpu/drm/xe/xe_uc.c
> > @@ -29,13 +29,15 @@ uc_to_xe(struct xe_uc *uc)
> > /* Should be called once at driver load only */
> > int xe_uc_init(struct xe_uc *uc)
> > {
> > + struct xe_device *xe = uc_to_xe(uc);
> > int ret;
> > + xe_device_mem_access_get(xe);
> > +
> > /*
> > * We call the GuC/HuC init functions even if GuC submission is off to
> > * correctly move our tracking of the FW state to "disabled".
> > */
> > -
> > ret = xe_guc_init(&uc->guc);
> > if (ret)
> > goto err;
> > @@ -45,7 +47,7 @@ int xe_uc_init(struct xe_uc *uc)
> > goto err;
> > if (!xe_device_uc_enabled(uc_to_xe(uc)))
> > - return 0;
> > + goto out;
> > ret = xe_wopcm_init(&uc->wopcm);
> > if (ret)
> > @@ -55,9 +57,14 @@ int xe_uc_init(struct xe_uc *uc)
> > if (ret)
> > goto err;
> > +out:
> > + xe_device_mem_access_put(xe);
> > +
> > return 0;
> > err:
> > + xe_device_mem_access_put(xe);
> > +
> > return ret;
> > }
^ permalink raw reply [flat|nested] 32+ messages in thread
* [Intel-xe] ✗ CI.Patch_applied: failure for drm/xe: Probe tweaks and reordering (rev3)
2023-11-29 1:16 [Intel-xe] [PATCH v4 00/22] drm/xe: Probe tweaks and reordering Michał Winiarski
` (21 preceding siblings ...)
2023-11-29 1:16 ` [Intel-xe] [PATCH v4 22/22] drm/xe: Initialize GuC earlier during probe Michał Winiarski
@ 2023-11-29 4:48 ` Patchwork
22 siblings, 0 replies; 32+ messages in thread
From: Patchwork @ 2023-11-29 4:48 UTC (permalink / raw)
To: Michał Winiarski; +Cc: intel-xe
== Series Details ==
Series: drm/xe: Probe tweaks and reordering (rev3)
URL : https://patchwork.freedesktop.org/series/126392/
State : failure
== Summary ==
=== Applying kernel patches on branch 'drm-xe-next' with base: ===
Base commit: 1b020536e drm/xe/gsc: Define GSC FW for MTL
=== git am output follows ===
error: patch failed: drivers/gpu/drm/xe/xe_uc_fw.c:575
error: drivers/gpu/drm/xe/xe_uc_fw.c: patch does not apply
hint: Use 'git am --show-current-patch' to see the failed patch
Applying: drm/xe: Skip calling drm_dev_put on probe error
Applying: drm/xe: Use managed pci_enable_device
Applying: drm/xe/irq: Don't call pci_free_irq_vectors
Applying: drm/xe: Move xe_set_dma_info outside of MMIO setup
Applying: drm/xe: Move xe_mmio_probe_tiles outside of MMIO setup
Applying: drm/xe: Split xe_info_init
Applying: drm/xe: Introduce xe_tile_init_early and use at earlier point in probe
Applying: drm/xe: Map the entire BAR0 and hold onto the initial mapping
Applying: drm/xe/device: Introduce xe_device_probe_early
Applying: drm/xe: Don't "peek" into GMD_ID
Applying: drm/xe: Move system memory management init to earlier point in probe
Applying: drm/xe: Move force_wake init to earlier point in probe
Applying: drm/xe: Reorder GGTT init to earlier point in probe
Applying: drm/xe: Add a helper for DRM device-lifetime BO create
Patch failed at 0014 drm/xe: Add a helper for DRM device-lifetime BO create
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
^ permalink raw reply [flat|nested] 32+ messages in thread