* [PATCH 0/6] accel/ivpu: Fixes for linux-6.6-rc4
@ 2023-09-25 12:11 Stanislaw Gruszka
2023-09-25 12:11 ` [PATCH 1/6] accel/ivpu: Do not use wait event interruptible Stanislaw Gruszka
` (6 more replies)
0 siblings, 7 replies; 15+ messages in thread
From: Stanislaw Gruszka @ 2023-09-25 12:11 UTC (permalink / raw)
To: dri-devel; +Cc: Stanislaw Gruszka, Oded Gabbay, Jeffrey Hugo, Jacek Lawrynowicz
- dmesg flood fix
- HW power-on and interrupt handling fixes for VPU4
- FW loading/mapping fix
Jacek Lawrynowicz (1):
accel/ivpu: Don't flood dmesg with VPU ready message
Karol Wachowski (4):
accel/ivpu/40xx: Ensure clock resource ownership Ack before Power-Up
accel/ivpu/40xx: Disable frequency change interrupt
accel/ivpu/40xx: Fix missing VPUIP interrupts
accel/ivpu: Use cached buffers for FW loading
Stanislaw Gruszka (1):
accel/ivpu: Do not use wait event interruptible
drivers/accel/ivpu/ivpu_drv.c | 2 +-
drivers/accel/ivpu/ivpu_fw.c | 8 +++++---
drivers/accel/ivpu/ivpu_gem.h | 5 +++++
drivers/accel/ivpu/ivpu_hw_40xx.c | 28 +++++++++++++++++++--------
drivers/accel/ivpu/ivpu_hw_40xx_reg.h | 2 ++
drivers/accel/ivpu/ivpu_ipc.c | 11 ++++-------
6 files changed, 37 insertions(+), 19 deletions(-)
--
2.25.1
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 1/6] accel/ivpu: Do not use wait event interruptible
2023-09-25 12:11 [PATCH 0/6] accel/ivpu: Fixes for linux-6.6-rc4 Stanislaw Gruszka
@ 2023-09-25 12:11 ` Stanislaw Gruszka
2023-09-26 15:51 ` Jeffrey Hugo
2023-09-25 12:11 ` [PATCH 2/6] accel/ivpu: Don't flood dmesg with VPU ready message Stanislaw Gruszka
` (5 subsequent siblings)
6 siblings, 1 reply; 15+ messages in thread
From: Stanislaw Gruszka @ 2023-09-25 12:11 UTC (permalink / raw)
To: dri-devel
Cc: Stanislaw Gruszka, Oded Gabbay, Jeffrey Hugo, Jacek Lawrynowicz,
Karol Wachowski
If we receive signal when waiting for IPC message response in
ivpu_ipc_receive() we return error and continue to operate.
Then the driver can send another IPC messages and re-use occupied
slot of the message still processed by the firmware. This can result
in corrupting firmware memory and following FW crash with messages:
[ 3698.569719] intel_vpu 0000:00:0b.0: [drm] ivpu_ipc_send_receive_internal(): IPC receive failed: type 0x1103, ret -512
[ 3698.569747] intel_vpu 0000:00:0b.0: [drm] ivpu_jsm_unregister_db(): Failed to unregister doorbell 3: -512
[ 3698.569756] intel_vpu 0000:00:0b.0: [drm] ivpu_ipc_tx_prepare(): IPC message vpu:0x88980000 not released by firmware
[ 3698.569763] intel_vpu 0000:00:0b.0: [drm] ivpu_ipc_tx_prepare(): JSM message vpu:0x88980040 not released by firmware
[ 3698.570234] intel_vpu 0000:00:0b.0: [drm] ivpu_ipc_send_receive_internal(): IPC receive failed: type 0x110e, ret -512
[ 3698.570318] intel_vpu 0000:00:0b.0: [drm] *ERROR* ivpu_mmu_dump_event(): MMU EVTQ: 0x10 (Translation fault) SSID: 0 SID: 3, e[2] 00000000, e[3] 00000208, in addr: 0x88988000, fetch addr: 0x0
To fix the issue don't use interruptible variant of wait event to
allow firmware to finish IPC processing.
Fixes: 5d7422cfb498 ("accel/ivpu: Add IPC driver and JSM messages")
Reviewed-by: Karol Wachowski <karol.wachowski@linux.intel.com>
Signed-off-by: Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com>
---
drivers/accel/ivpu/ivpu_ipc.c | 11 ++++-------
1 file changed, 4 insertions(+), 7 deletions(-)
diff --git a/drivers/accel/ivpu/ivpu_ipc.c b/drivers/accel/ivpu/ivpu_ipc.c
index fa0af59e39ab..295c0d7b5039 100644
--- a/drivers/accel/ivpu/ivpu_ipc.c
+++ b/drivers/accel/ivpu/ivpu_ipc.c
@@ -209,10 +209,10 @@ int ivpu_ipc_receive(struct ivpu_device *vdev, struct ivpu_ipc_consumer *cons,
struct ivpu_ipc_rx_msg *rx_msg;
int wait_ret, ret = 0;
- wait_ret = wait_event_interruptible_timeout(cons->rx_msg_wq,
- (IS_KTHREAD() && kthread_should_stop()) ||
- !list_empty(&cons->rx_msg_list),
- msecs_to_jiffies(timeout_ms));
+ wait_ret = wait_event_timeout(cons->rx_msg_wq,
+ (IS_KTHREAD() && kthread_should_stop()) ||
+ !list_empty(&cons->rx_msg_list),
+ msecs_to_jiffies(timeout_ms));
if (IS_KTHREAD() && kthread_should_stop())
return -EINTR;
@@ -220,9 +220,6 @@ int ivpu_ipc_receive(struct ivpu_device *vdev, struct ivpu_ipc_consumer *cons,
if (wait_ret == 0)
return -ETIMEDOUT;
- if (wait_ret < 0)
- return -ERESTARTSYS;
-
spin_lock_irq(&cons->rx_msg_lock);
rx_msg = list_first_entry_or_null(&cons->rx_msg_list, struct ivpu_ipc_rx_msg, link);
if (!rx_msg) {
--
2.25.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 2/6] accel/ivpu: Don't flood dmesg with VPU ready message
2023-09-25 12:11 [PATCH 0/6] accel/ivpu: Fixes for linux-6.6-rc4 Stanislaw Gruszka
2023-09-25 12:11 ` [PATCH 1/6] accel/ivpu: Do not use wait event interruptible Stanislaw Gruszka
@ 2023-09-25 12:11 ` Stanislaw Gruszka
2023-09-26 15:52 ` Jeffrey Hugo
2023-09-25 12:11 ` [PATCH 3/6] accel/ivpu/40xx: Ensure clock resource ownership Ack before Power-Up Stanislaw Gruszka
` (4 subsequent siblings)
6 siblings, 1 reply; 15+ messages in thread
From: Stanislaw Gruszka @ 2023-09-25 12:11 UTC (permalink / raw)
To: dri-devel; +Cc: Stanislaw Gruszka, Oded Gabbay, Jeffrey Hugo, Jacek Lawrynowicz
From: Jacek Lawrynowicz <jacek.lawrynowicz@linux.intel.com>
Use ivpu_dbg() to print the VPU ready message so it doesn't pollute
the dmesg.
Signed-off-by: Jacek Lawrynowicz <jacek.lawrynowicz@linux.intel.com>
Reviewed-by: Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com>
Signed-off-by: Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com>
---
drivers/accel/ivpu/ivpu_drv.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/accel/ivpu/ivpu_drv.c b/drivers/accel/ivpu/ivpu_drv.c
index aa7314fdbc0f..467a60235370 100644
--- a/drivers/accel/ivpu/ivpu_drv.c
+++ b/drivers/accel/ivpu/ivpu_drv.c
@@ -327,7 +327,7 @@ static int ivpu_wait_for_ready(struct ivpu_device *vdev)
}
if (!ret)
- ivpu_info(vdev, "VPU ready message received successfully\n");
+ ivpu_dbg(vdev, PM, "VPU ready message received successfully\n");
else
ivpu_hw_diagnose_failure(vdev);
--
2.25.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 3/6] accel/ivpu/40xx: Ensure clock resource ownership Ack before Power-Up
2023-09-25 12:11 [PATCH 0/6] accel/ivpu: Fixes for linux-6.6-rc4 Stanislaw Gruszka
2023-09-25 12:11 ` [PATCH 1/6] accel/ivpu: Do not use wait event interruptible Stanislaw Gruszka
2023-09-25 12:11 ` [PATCH 2/6] accel/ivpu: Don't flood dmesg with VPU ready message Stanislaw Gruszka
@ 2023-09-25 12:11 ` Stanislaw Gruszka
2023-09-26 15:53 ` Jeffrey Hugo
2023-09-25 12:11 ` [PATCH 4/6] accel/ivpu/40xx: Disable frequency change interrupt Stanislaw Gruszka
` (3 subsequent siblings)
6 siblings, 1 reply; 15+ messages in thread
From: Stanislaw Gruszka @ 2023-09-25 12:11 UTC (permalink / raw)
To: dri-devel
Cc: Karol Wachowski, Oded Gabbay, Jeffrey Hugo, Jacek Lawrynowicz,
Stanislaw Gruszka
From: Karol Wachowski <karol.wachowski@linux.intel.com>
We need to wait for the CLOCK_RESOURCE_OWN_ACK bit to be set
after configuring the workpoint. This step ensures that the VPU
microcontroller clock is actively toggling and ready for operation.
Previously, we relied solely on the READY bit in the VPU_STATUS
register, which indicated the completion of the workpoint download.
However, this approach was insufficient, as the READY bit could be set
while the device was still running on a sideband clock until the PLL
locked. To guarantee that the PLL is locked and the device is running on
the main clock source, we now wait for the CLOCK_RESOURCE_OWN_ACK before
proceeding with the remainder of the power-up sequence.
Fixes: 79cdc56c4a54 ("accel/ivpu: Add initial support for VPU 4")
Signed-off-by: Karol Wachowski <karol.wachowski@linux.intel.com>
Reviewed-by: Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com>
Signed-off-by: Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com>
---
drivers/accel/ivpu/ivpu_hw_40xx.c | 14 ++++++++++++++
drivers/accel/ivpu/ivpu_hw_40xx_reg.h | 2 ++
2 files changed, 16 insertions(+)
diff --git a/drivers/accel/ivpu/ivpu_hw_40xx.c b/drivers/accel/ivpu/ivpu_hw_40xx.c
index 00c5dbbe6847..f4a251a58ca4 100644
--- a/drivers/accel/ivpu/ivpu_hw_40xx.c
+++ b/drivers/accel/ivpu/ivpu_hw_40xx.c
@@ -196,6 +196,14 @@ static int ivpu_pll_wait_for_status_ready(struct ivpu_device *vdev)
return REGB_POLL_FLD(VPU_40XX_BUTTRESS_VPU_STATUS, READY, 1, PLL_TIMEOUT_US);
}
+static int ivpu_wait_for_clock_own_resource_ack(struct ivpu_device *vdev)
+{
+ if (ivpu_is_simics(vdev))
+ return 0;
+
+ return REGB_POLL_FLD(VPU_40XX_BUTTRESS_VPU_STATUS, CLOCK_RESOURCE_OWN_ACK, 1, TIMEOUT_US);
+}
+
static void ivpu_pll_init_frequency_ratios(struct ivpu_device *vdev)
{
struct ivpu_hw_info *hw = vdev->hw;
@@ -556,6 +564,12 @@ static int ivpu_boot_pwr_domain_enable(struct ivpu_device *vdev)
{
int ret;
+ ret = ivpu_wait_for_clock_own_resource_ack(vdev);
+ if (ret) {
+ ivpu_err(vdev, "Timed out waiting for clock own resource ACK\n");
+ return ret;
+ }
+
ivpu_boot_pwr_island_trickle_drive(vdev, true);
ivpu_boot_pwr_island_drive(vdev, true);
diff --git a/drivers/accel/ivpu/ivpu_hw_40xx_reg.h b/drivers/accel/ivpu/ivpu_hw_40xx_reg.h
index 5139cfe88532..ff4a5d4f5821 100644
--- a/drivers/accel/ivpu/ivpu_hw_40xx_reg.h
+++ b/drivers/accel/ivpu/ivpu_hw_40xx_reg.h
@@ -70,6 +70,8 @@
#define VPU_40XX_BUTTRESS_VPU_STATUS_READY_MASK BIT_MASK(0)
#define VPU_40XX_BUTTRESS_VPU_STATUS_IDLE_MASK BIT_MASK(1)
#define VPU_40XX_BUTTRESS_VPU_STATUS_DUP_IDLE_MASK BIT_MASK(2)
+#define VPU_40XX_BUTTRESS_VPU_STATUS_CLOCK_RESOURCE_OWN_ACK_MASK BIT_MASK(6)
+#define VPU_40XX_BUTTRESS_VPU_STATUS_POWER_RESOURCE_OWN_ACK_MASK BIT_MASK(7)
#define VPU_40XX_BUTTRESS_VPU_STATUS_PERF_CLK_MASK BIT_MASK(11)
#define VPU_40XX_BUTTRESS_VPU_STATUS_DISABLE_CLK_RELINQUISH_MASK BIT_MASK(12)
--
2.25.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 4/6] accel/ivpu/40xx: Disable frequency change interrupt
2023-09-25 12:11 [PATCH 0/6] accel/ivpu: Fixes for linux-6.6-rc4 Stanislaw Gruszka
` (2 preceding siblings ...)
2023-09-25 12:11 ` [PATCH 3/6] accel/ivpu/40xx: Ensure clock resource ownership Ack before Power-Up Stanislaw Gruszka
@ 2023-09-25 12:11 ` Stanislaw Gruszka
2023-09-26 15:54 ` Jeffrey Hugo
2023-09-25 12:11 ` [PATCH 5/6] accel/ivpu/40xx: Fix missing VPUIP interrupts Stanislaw Gruszka
` (2 subsequent siblings)
6 siblings, 1 reply; 15+ messages in thread
From: Stanislaw Gruszka @ 2023-09-25 12:11 UTC (permalink / raw)
To: dri-devel
Cc: Karol Wachowski, Oded Gabbay, Jeffrey Hugo, Jacek Lawrynowicz,
Stanislaw Gruszka
From: Karol Wachowski <karol.wachowski@linux.intel.com>
Do not enable frequency change interrupt on 40xx as it might
lead to an interrupt storm in current design.
FREQ_CHANGE interrupt is triggered on D0I2 entry which will cause
KMD to check VPU interrupt sources by reading VPUIP registers.
Access to those registers will toggle necessary clocks and trigger
another FREQ_CHANGE interrupt possibly ending in an infinite loop.
FREQ_CHANGE interrupt has only debug purposes and can be permanently
disabled.
Fixes: 79cdc56c4a54 ("accel/ivpu: Add initial support for VPU 4")
Signed-off-by: Karol Wachowski <karol.wachowski@linux.intel.com>
Reviewed-by: Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com>
Signed-off-by: Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com>
---
drivers/accel/ivpu/ivpu_hw_40xx.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/accel/ivpu/ivpu_hw_40xx.c b/drivers/accel/ivpu/ivpu_hw_40xx.c
index f4a251a58ca4..87b1085d44cf 100644
--- a/drivers/accel/ivpu/ivpu_hw_40xx.c
+++ b/drivers/accel/ivpu/ivpu_hw_40xx.c
@@ -57,8 +57,7 @@
#define ICB_0_1_IRQ_MASK ((((u64)ICB_1_IRQ_MASK) << 32) | ICB_0_IRQ_MASK)
-#define BUTTRESS_IRQ_MASK ((REG_FLD(VPU_40XX_BUTTRESS_INTERRUPT_STAT, FREQ_CHANGE)) | \
- (REG_FLD(VPU_40XX_BUTTRESS_INTERRUPT_STAT, ATS_ERR)) | \
+#define BUTTRESS_IRQ_MASK ((REG_FLD(VPU_40XX_BUTTRESS_INTERRUPT_STAT, ATS_ERR)) | \
(REG_FLD(VPU_40XX_BUTTRESS_INTERRUPT_STAT, CFI0_ERR)) | \
(REG_FLD(VPU_40XX_BUTTRESS_INTERRUPT_STAT, CFI1_ERR)) | \
(REG_FLD(VPU_40XX_BUTTRESS_INTERRUPT_STAT, IMR0_ERR)) | \
--
2.25.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 5/6] accel/ivpu/40xx: Fix missing VPUIP interrupts
2023-09-25 12:11 [PATCH 0/6] accel/ivpu: Fixes for linux-6.6-rc4 Stanislaw Gruszka
` (3 preceding siblings ...)
2023-09-25 12:11 ` [PATCH 4/6] accel/ivpu/40xx: Disable frequency change interrupt Stanislaw Gruszka
@ 2023-09-25 12:11 ` Stanislaw Gruszka
2023-09-26 15:55 ` Jeffrey Hugo
2023-09-25 12:11 ` [PATCH 6/6] accel/ivpu: Use cached buffers for FW loading Stanislaw Gruszka
2023-09-27 6:05 ` [PATCH 0/6] accel/ivpu: Fixes for linux-6.6-rc4 Stanislaw Gruszka
6 siblings, 1 reply; 15+ messages in thread
From: Stanislaw Gruszka @ 2023-09-25 12:11 UTC (permalink / raw)
To: dri-devel
Cc: Karol Wachowski, Oded Gabbay, Jeffrey Hugo, Jacek Lawrynowicz,
Stanislaw Gruszka
From: Karol Wachowski <karol.wachowski@linux.intel.com>
Move sequence of masking and unmasking global interrupts from buttress
interrupt handler to generic one that handles both VPUIP and BTRS
interrupts.
Unmasking global interrupts will re-trigger MSI for any pending interrupts.
Lack of this sequence can randomly cause to miss any VPUIP interrupt that
comes after reading VPU_40XX_HOST_SS_ICB_STATUS_0 and before clearing
all active interrupt sources.
Fixes: 79cdc56c4a54 ("accel/ivpu: Add initial support for VPU 4")
Signed-off-by: Karol Wachowski <karol.wachowski@linux.intel.com>
Reviewed-by: Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com>
Signed-off-by: Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com>
---
drivers/accel/ivpu/ivpu_hw_40xx.c | 11 +++++------
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/drivers/accel/ivpu/ivpu_hw_40xx.c b/drivers/accel/ivpu/ivpu_hw_40xx.c
index 87b1085d44cf..8bdb59a45da6 100644
--- a/drivers/accel/ivpu/ivpu_hw_40xx.c
+++ b/drivers/accel/ivpu/ivpu_hw_40xx.c
@@ -1059,9 +1059,6 @@ static irqreturn_t ivpu_hw_40xx_irqb_handler(struct ivpu_device *vdev, int irq)
if (status == 0)
return IRQ_NONE;
- /* Disable global interrupt before handling local buttress interrupts */
- REGB_WR32(VPU_40XX_BUTTRESS_GLOBAL_INT_MASK, 0x1);
-
if (REG_TEST_FLD(VPU_40XX_BUTTRESS_INTERRUPT_STAT, FREQ_CHANGE, status))
ivpu_dbg(vdev, IRQ, "FREQ_CHANGE");
@@ -1109,9 +1106,6 @@ static irqreturn_t ivpu_hw_40xx_irqb_handler(struct ivpu_device *vdev, int irq)
/* This must be done after interrupts are cleared at the source. */
REGB_WR32(VPU_40XX_BUTTRESS_INTERRUPT_STAT, status);
- /* Re-enable global interrupt */
- REGB_WR32(VPU_40XX_BUTTRESS_GLOBAL_INT_MASK, 0x0);
-
if (schedule_recovery)
ivpu_pm_schedule_recovery(vdev);
@@ -1123,9 +1117,14 @@ static irqreturn_t ivpu_hw_40xx_irq_handler(int irq, void *ptr)
struct ivpu_device *vdev = ptr;
irqreturn_t ret = IRQ_NONE;
+ REGB_WR32(VPU_40XX_BUTTRESS_GLOBAL_INT_MASK, 0x1);
+
ret |= ivpu_hw_40xx_irqv_handler(vdev, irq);
ret |= ivpu_hw_40xx_irqb_handler(vdev, irq);
+ /* Re-enable global interrupts to re-trigger MSI for pending interrupts */
+ REGB_WR32(VPU_40XX_BUTTRESS_GLOBAL_INT_MASK, 0x0);
+
if (ret & IRQ_WAKE_THREAD)
return IRQ_WAKE_THREAD;
--
2.25.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 6/6] accel/ivpu: Use cached buffers for FW loading
2023-09-25 12:11 [PATCH 0/6] accel/ivpu: Fixes for linux-6.6-rc4 Stanislaw Gruszka
` (4 preceding siblings ...)
2023-09-25 12:11 ` [PATCH 5/6] accel/ivpu/40xx: Fix missing VPUIP interrupts Stanislaw Gruszka
@ 2023-09-25 12:11 ` Stanislaw Gruszka
2023-09-26 12:09 ` [PATCH v2 " Stanislaw Gruszka
2023-09-27 6:05 ` [PATCH 0/6] accel/ivpu: Fixes for linux-6.6-rc4 Stanislaw Gruszka
6 siblings, 1 reply; 15+ messages in thread
From: Stanislaw Gruszka @ 2023-09-25 12:11 UTC (permalink / raw)
To: dri-devel
Cc: Karol Wachowski, Oded Gabbay, Jeffrey Hugo, Jacek Lawrynowicz,
Stanislaw Gruszka
From: Karol Wachowski <karol.wachowski@linux.intel.com>
Create buffers with cache coherency on the CPU side (write-back) while
disabling snooping on the VPU side. These buffers require an explicit
cache flush after each CPU-side modification.
Configuring pages as write-combined may introduce significant delays,
potentially taking hundreds of milliseconds for 64 MB buffers.
Added internal DRM_IVPU_BO_NOSNOOP mask which disables snooping on the
VPU side. Allocate FW runtime memory buffer (64 MB) as cached with
snooping-disabled.
This fixes random long FW loading times and boot params memory
corruption on warmboot (due to missed wmb).
Fixes: 02d5b0aacd05 ("accel/ivpu: Implement firmware parsing and booting")
Signed-off-by: Karol Wachowski <karol.wachowski@linux.intel.com>
Reviewed-by: Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com>
Signed-off-by: Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com>
---
drivers/accel/ivpu/ivpu_fw.c | 8 +++++---
drivers/accel/ivpu/ivpu_gem.h | 5 +++++
2 files changed, 10 insertions(+), 3 deletions(-)
diff --git a/drivers/accel/ivpu/ivpu_fw.c b/drivers/accel/ivpu/ivpu_fw.c
index 9827ea4d7b83..301f5221d193 100644
--- a/drivers/accel/ivpu/ivpu_fw.c
+++ b/drivers/accel/ivpu/ivpu_fw.c
@@ -220,7 +220,8 @@ static int ivpu_fw_mem_init(struct ivpu_device *vdev)
if (ret)
return ret;
- fw->mem = ivpu_bo_alloc_internal(vdev, fw->runtime_addr, fw->runtime_size, DRM_IVPU_BO_WC);
+ fw->mem = ivpu_bo_alloc_internal(vdev, fw->runtime_addr, fw->runtime_size,
+ DRM_IVPU_BO_WC | DRM_IVPU_BO_NOSNOOP);
if (!fw->mem) {
ivpu_err(vdev, "Failed to allocate firmware runtime memory\n");
return -ENOMEM;
@@ -330,7 +331,7 @@ int ivpu_fw_load(struct ivpu_device *vdev)
memset(start, 0, size);
}
- wmb(); /* Flush WC buffers after writing fw->mem */
+ clflush_cache_range(fw->mem->kvaddr, fw->mem->base.size);
return 0;
}
@@ -432,6 +433,7 @@ void ivpu_fw_boot_params_setup(struct ivpu_device *vdev, struct vpu_boot_params
if (!ivpu_fw_is_cold_boot(vdev)) {
boot_params->save_restore_ret_address = 0;
vdev->pm->is_warmboot = true;
+ clflush_cache_range(vdev->fw->mem->kvaddr, SZ_4K);
return;
}
@@ -493,7 +495,7 @@ void ivpu_fw_boot_params_setup(struct ivpu_device *vdev, struct vpu_boot_params
boot_params->punit_telemetry_sram_size = ivpu_hw_reg_telemetry_size_get(vdev);
boot_params->vpu_telemetry_enable = ivpu_hw_reg_telemetry_enable_get(vdev);
- wmb(); /* Flush WC buffers after writing bootparams */
+ clflush_cache_range(vdev->fw->mem->kvaddr, SZ_4K);
ivpu_fw_boot_params_print(vdev, boot_params);
}
diff --git a/drivers/accel/ivpu/ivpu_gem.h b/drivers/accel/ivpu/ivpu_gem.h
index 6b0ceda5f253..f4130586ff1b 100644
--- a/drivers/accel/ivpu/ivpu_gem.h
+++ b/drivers/accel/ivpu/ivpu_gem.h
@@ -8,6 +8,8 @@
#include <drm/drm_gem.h>
#include <drm/drm_mm.h>
+#define DRM_IVPU_BO_NOSNOOP 0x10000000
+
struct dma_buf;
struct ivpu_bo_ops;
struct ivpu_file_priv;
@@ -83,6 +85,9 @@ static inline u32 ivpu_bo_cache_mode(struct ivpu_bo *bo)
static inline bool ivpu_bo_is_snooped(struct ivpu_bo *bo)
{
+ if (bo->flags & DRM_IVPU_BO_NOSNOOP)
+ return false;
+
return ivpu_bo_cache_mode(bo) == DRM_IVPU_BO_CACHED;
}
--
2.25.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v2 6/6] accel/ivpu: Use cached buffers for FW loading
2023-09-25 12:11 ` [PATCH 6/6] accel/ivpu: Use cached buffers for FW loading Stanislaw Gruszka
@ 2023-09-26 12:09 ` Stanislaw Gruszka
2023-09-26 15:56 ` Jeffrey Hugo
0 siblings, 1 reply; 15+ messages in thread
From: Stanislaw Gruszka @ 2023-09-26 12:09 UTC (permalink / raw)
To: dri-devel; +Cc: Karol Wachowski, Oded Gabbay, Jeffrey Hugo, Jacek Lawrynowicz
From: Karol Wachowski <karol.wachowski@linux.intel.com>
Create buffers with cache coherency on the CPU side (write-back) while
disabling snooping on the VPU side. These buffers require an explicit
cache flush after each CPU-side modification.
Configuring pages as write-combined may introduce significant delays,
potentially taking hundreds of milliseconds for 64 MB buffers.
Added internal DRM_IVPU_BO_NOSNOOP mask which disables snooping on the
VPU side. Allocate FW runtime memory buffer (64 MB) as cached with
snooping-disabled.
This fixes random long FW loading times and boot params memory
corruption on warmboot (due to missed wmb).
Fixes: 02d5b0aacd05 ("accel/ivpu: Implement firmware parsing and booting")
Signed-off-by: Karol Wachowski <karol.wachowski@linux.intel.com>
Reviewed-by: Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com>
Signed-off-by: Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com>
---
v2: Actually use DRM_IVPU_BO_CACHED instead of DRM_IVPU_BO_WC..
drivers/accel/ivpu/ivpu_fw.c | 8 +++++---
drivers/accel/ivpu/ivpu_gem.h | 5 +++++
2 files changed, 10 insertions(+), 3 deletions(-)
diff --git a/drivers/accel/ivpu/ivpu_fw.c b/drivers/accel/ivpu/ivpu_fw.c
index 9827ea4d7b83..0191cf8e5964 100644
--- a/drivers/accel/ivpu/ivpu_fw.c
+++ b/drivers/accel/ivpu/ivpu_fw.c
@@ -220,7 +220,8 @@ static int ivpu_fw_mem_init(struct ivpu_device *vdev)
if (ret)
return ret;
- fw->mem = ivpu_bo_alloc_internal(vdev, fw->runtime_addr, fw->runtime_size, DRM_IVPU_BO_WC);
+ fw->mem = ivpu_bo_alloc_internal(vdev, fw->runtime_addr, fw->runtime_size,
+ DRM_IVPU_BO_CACHED | DRM_IVPU_BO_NOSNOOP);
if (!fw->mem) {
ivpu_err(vdev, "Failed to allocate firmware runtime memory\n");
return -ENOMEM;
@@ -330,7 +331,7 @@ int ivpu_fw_load(struct ivpu_device *vdev)
memset(start, 0, size);
}
- wmb(); /* Flush WC buffers after writing fw->mem */
+ clflush_cache_range(fw->mem->kvaddr, fw->mem->base.size);
return 0;
}
@@ -432,6 +433,7 @@ void ivpu_fw_boot_params_setup(struct ivpu_device *vdev, struct vpu_boot_params
if (!ivpu_fw_is_cold_boot(vdev)) {
boot_params->save_restore_ret_address = 0;
vdev->pm->is_warmboot = true;
+ clflush_cache_range(vdev->fw->mem->kvaddr, SZ_4K);
return;
}
@@ -493,7 +495,7 @@ void ivpu_fw_boot_params_setup(struct ivpu_device *vdev, struct vpu_boot_params
boot_params->punit_telemetry_sram_size = ivpu_hw_reg_telemetry_size_get(vdev);
boot_params->vpu_telemetry_enable = ivpu_hw_reg_telemetry_enable_get(vdev);
- wmb(); /* Flush WC buffers after writing bootparams */
+ clflush_cache_range(vdev->fw->mem->kvaddr, SZ_4K);
ivpu_fw_boot_params_print(vdev, boot_params);
}
diff --git a/drivers/accel/ivpu/ivpu_gem.h b/drivers/accel/ivpu/ivpu_gem.h
index 6b0ceda5f253..f4130586ff1b 100644
--- a/drivers/accel/ivpu/ivpu_gem.h
+++ b/drivers/accel/ivpu/ivpu_gem.h
@@ -8,6 +8,8 @@
#include <drm/drm_gem.h>
#include <drm/drm_mm.h>
+#define DRM_IVPU_BO_NOSNOOP 0x10000000
+
struct dma_buf;
struct ivpu_bo_ops;
struct ivpu_file_priv;
@@ -83,6 +85,9 @@ static inline u32 ivpu_bo_cache_mode(struct ivpu_bo *bo)
static inline bool ivpu_bo_is_snooped(struct ivpu_bo *bo)
{
+ if (bo->flags & DRM_IVPU_BO_NOSNOOP)
+ return false;
+
return ivpu_bo_cache_mode(bo) == DRM_IVPU_BO_CACHED;
}
--
2.25.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 1/6] accel/ivpu: Do not use wait event interruptible
2023-09-25 12:11 ` [PATCH 1/6] accel/ivpu: Do not use wait event interruptible Stanislaw Gruszka
@ 2023-09-26 15:51 ` Jeffrey Hugo
0 siblings, 0 replies; 15+ messages in thread
From: Jeffrey Hugo @ 2023-09-26 15:51 UTC (permalink / raw)
To: Stanislaw Gruszka, dri-devel
Cc: Karol Wachowski, Oded Gabbay, Jacek Lawrynowicz
On 9/25/2023 6:11 AM, Stanislaw Gruszka wrote:
> If we receive signal when waiting for IPC message response in
> ivpu_ipc_receive() we return error and continue to operate.
> Then the driver can send another IPC messages and re-use occupied
> slot of the message still processed by the firmware. This can result
> in corrupting firmware memory and following FW crash with messages:
>
> [ 3698.569719] intel_vpu 0000:00:0b.0: [drm] ivpu_ipc_send_receive_internal(): IPC receive failed: type 0x1103, ret -512
> [ 3698.569747] intel_vpu 0000:00:0b.0: [drm] ivpu_jsm_unregister_db(): Failed to unregister doorbell 3: -512
> [ 3698.569756] intel_vpu 0000:00:0b.0: [drm] ivpu_ipc_tx_prepare(): IPC message vpu:0x88980000 not released by firmware
> [ 3698.569763] intel_vpu 0000:00:0b.0: [drm] ivpu_ipc_tx_prepare(): JSM message vpu:0x88980040 not released by firmware
> [ 3698.570234] intel_vpu 0000:00:0b.0: [drm] ivpu_ipc_send_receive_internal(): IPC receive failed: type 0x110e, ret -512
> [ 3698.570318] intel_vpu 0000:00:0b.0: [drm] *ERROR* ivpu_mmu_dump_event(): MMU EVTQ: 0x10 (Translation fault) SSID: 0 SID: 3, e[2] 00000000, e[3] 00000208, in addr: 0x88988000, fetch addr: 0x0
>
> To fix the issue don't use interruptible variant of wait event to
> allow firmware to finish IPC processing.
>
> Fixes: 5d7422cfb498 ("accel/ivpu: Add IPC driver and JSM messages")
> Reviewed-by: Karol Wachowski <karol.wachowski@linux.intel.com>
> Signed-off-by: Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com>
Reviewed-by: Jeffrey Hugo <quic_jhugo@quicinc.com>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/6] accel/ivpu: Don't flood dmesg with VPU ready message
2023-09-25 12:11 ` [PATCH 2/6] accel/ivpu: Don't flood dmesg with VPU ready message Stanislaw Gruszka
@ 2023-09-26 15:52 ` Jeffrey Hugo
0 siblings, 0 replies; 15+ messages in thread
From: Jeffrey Hugo @ 2023-09-26 15:52 UTC (permalink / raw)
To: Stanislaw Gruszka, dri-devel; +Cc: Oded Gabbay, Jacek Lawrynowicz
On 9/25/2023 6:11 AM, Stanislaw Gruszka wrote:
> From: Jacek Lawrynowicz <jacek.lawrynowicz@linux.intel.com>
>
> Use ivpu_dbg() to print the VPU ready message so it doesn't pollute
> the dmesg.
>
> Signed-off-by: Jacek Lawrynowicz <jacek.lawrynowicz@linux.intel.com>
> Reviewed-by: Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com>
> Signed-off-by: Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com>
Reviewed-by: Jeffrey Hugo <quic_jhugo@quicinc.com>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 3/6] accel/ivpu/40xx: Ensure clock resource ownership Ack before Power-Up
2023-09-25 12:11 ` [PATCH 3/6] accel/ivpu/40xx: Ensure clock resource ownership Ack before Power-Up Stanislaw Gruszka
@ 2023-09-26 15:53 ` Jeffrey Hugo
0 siblings, 0 replies; 15+ messages in thread
From: Jeffrey Hugo @ 2023-09-26 15:53 UTC (permalink / raw)
To: Stanislaw Gruszka, dri-devel
Cc: Karol Wachowski, Oded Gabbay, Jacek Lawrynowicz
On 9/25/2023 6:11 AM, Stanislaw Gruszka wrote:
> From: Karol Wachowski <karol.wachowski@linux.intel.com>
>
> We need to wait for the CLOCK_RESOURCE_OWN_ACK bit to be set
> after configuring the workpoint. This step ensures that the VPU
> microcontroller clock is actively toggling and ready for operation.
>
> Previously, we relied solely on the READY bit in the VPU_STATUS
> register, which indicated the completion of the workpoint download.
> However, this approach was insufficient, as the READY bit could be set
> while the device was still running on a sideband clock until the PLL
> locked. To guarantee that the PLL is locked and the device is running on
> the main clock source, we now wait for the CLOCK_RESOURCE_OWN_ACK before
> proceeding with the remainder of the power-up sequence.
>
> Fixes: 79cdc56c4a54 ("accel/ivpu: Add initial support for VPU 4")
> Signed-off-by: Karol Wachowski <karol.wachowski@linux.intel.com>
> Reviewed-by: Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com>
> Signed-off-by: Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com>
Reviewed-by: Jeffrey Hugo <quic_jhugo@quicinc.com>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 4/6] accel/ivpu/40xx: Disable frequency change interrupt
2023-09-25 12:11 ` [PATCH 4/6] accel/ivpu/40xx: Disable frequency change interrupt Stanislaw Gruszka
@ 2023-09-26 15:54 ` Jeffrey Hugo
0 siblings, 0 replies; 15+ messages in thread
From: Jeffrey Hugo @ 2023-09-26 15:54 UTC (permalink / raw)
To: Stanislaw Gruszka, dri-devel
Cc: Karol Wachowski, Oded Gabbay, Jacek Lawrynowicz
On 9/25/2023 6:11 AM, Stanislaw Gruszka wrote:
> From: Karol Wachowski <karol.wachowski@linux.intel.com>
>
> Do not enable frequency change interrupt on 40xx as it might
> lead to an interrupt storm in current design.
>
> FREQ_CHANGE interrupt is triggered on D0I2 entry which will cause
> KMD to check VPU interrupt sources by reading VPUIP registers.
> Access to those registers will toggle necessary clocks and trigger
> another FREQ_CHANGE interrupt possibly ending in an infinite loop.
>
> FREQ_CHANGE interrupt has only debug purposes and can be permanently
> disabled.
>
> Fixes: 79cdc56c4a54 ("accel/ivpu: Add initial support for VPU 4")
> Signed-off-by: Karol Wachowski <karol.wachowski@linux.intel.com>
> Reviewed-by: Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com>
> Signed-off-by: Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com>
Reviewed-by: Jeffrey Hugo <quic_jhugo@quicinc.com>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 5/6] accel/ivpu/40xx: Fix missing VPUIP interrupts
2023-09-25 12:11 ` [PATCH 5/6] accel/ivpu/40xx: Fix missing VPUIP interrupts Stanislaw Gruszka
@ 2023-09-26 15:55 ` Jeffrey Hugo
0 siblings, 0 replies; 15+ messages in thread
From: Jeffrey Hugo @ 2023-09-26 15:55 UTC (permalink / raw)
To: Stanislaw Gruszka, dri-devel
Cc: Karol Wachowski, Oded Gabbay, Jacek Lawrynowicz
On 9/25/2023 6:11 AM, Stanislaw Gruszka wrote:
> From: Karol Wachowski <karol.wachowski@linux.intel.com>
>
> Move sequence of masking and unmasking global interrupts from buttress
> interrupt handler to generic one that handles both VPUIP and BTRS
> interrupts.
>
> Unmasking global interrupts will re-trigger MSI for any pending interrupts.
> Lack of this sequence can randomly cause to miss any VPUIP interrupt that
> comes after reading VPU_40XX_HOST_SS_ICB_STATUS_0 and before clearing
> all active interrupt sources.
>
> Fixes: 79cdc56c4a54 ("accel/ivpu: Add initial support for VPU 4")
> Signed-off-by: Karol Wachowski <karol.wachowski@linux.intel.com>
> Reviewed-by: Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com>
> Signed-off-by: Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com>
Reviewed-by: Jeffrey Hugo <quic_jhugo@quicinc.com>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 6/6] accel/ivpu: Use cached buffers for FW loading
2023-09-26 12:09 ` [PATCH v2 " Stanislaw Gruszka
@ 2023-09-26 15:56 ` Jeffrey Hugo
0 siblings, 0 replies; 15+ messages in thread
From: Jeffrey Hugo @ 2023-09-26 15:56 UTC (permalink / raw)
To: Stanislaw Gruszka, dri-devel
Cc: Karol Wachowski, Oded Gabbay, Jacek Lawrynowicz
On 9/26/2023 6:09 AM, Stanislaw Gruszka wrote:
> From: Karol Wachowski <karol.wachowski@linux.intel.com>
>
> Create buffers with cache coherency on the CPU side (write-back) while
> disabling snooping on the VPU side. These buffers require an explicit
> cache flush after each CPU-side modification.
>
> Configuring pages as write-combined may introduce significant delays,
> potentially taking hundreds of milliseconds for 64 MB buffers.
>
> Added internal DRM_IVPU_BO_NOSNOOP mask which disables snooping on the
> VPU side. Allocate FW runtime memory buffer (64 MB) as cached with
> snooping-disabled.
>
> This fixes random long FW loading times and boot params memory
> corruption on warmboot (due to missed wmb).
>
> Fixes: 02d5b0aacd05 ("accel/ivpu: Implement firmware parsing and booting")
> Signed-off-by: Karol Wachowski <karol.wachowski@linux.intel.com>
> Reviewed-by: Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com>
> Signed-off-by: Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com>
Reviewed-by: Jeffrey Hugo <quic_jhugo@quicinc.com>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 0/6] accel/ivpu: Fixes for linux-6.6-rc4
2023-09-25 12:11 [PATCH 0/6] accel/ivpu: Fixes for linux-6.6-rc4 Stanislaw Gruszka
` (5 preceding siblings ...)
2023-09-25 12:11 ` [PATCH 6/6] accel/ivpu: Use cached buffers for FW loading Stanislaw Gruszka
@ 2023-09-27 6:05 ` Stanislaw Gruszka
6 siblings, 0 replies; 15+ messages in thread
From: Stanislaw Gruszka @ 2023-09-27 6:05 UTC (permalink / raw)
To: dri-devel; +Cc: Jani Nikula, Oded Gabbay, Jeffrey Hugo, Jacek Lawrynowicz
On Mon, Sep 25, 2023 at 02:11:31PM +0200, Stanislaw Gruszka wrote:
> - dmesg flood fix
> - HW power-on and interrupt handling fixes for VPU4
> - FW loading/mapping fix
Pushed to drm-misc-fixes.
I had to resolve conflict when rebuilding tip, hope everything is ok there.
Thanks
Stanislaw
> Jacek Lawrynowicz (1):
> accel/ivpu: Don't flood dmesg with VPU ready message
>
> Karol Wachowski (4):
> accel/ivpu/40xx: Ensure clock resource ownership Ack before Power-Up
> accel/ivpu/40xx: Disable frequency change interrupt
> accel/ivpu/40xx: Fix missing VPUIP interrupts
> accel/ivpu: Use cached buffers for FW loading
>
> Stanislaw Gruszka (1):
> accel/ivpu: Do not use wait event interruptible
>
> drivers/accel/ivpu/ivpu_drv.c | 2 +-
> drivers/accel/ivpu/ivpu_fw.c | 8 +++++---
> drivers/accel/ivpu/ivpu_gem.h | 5 +++++
> drivers/accel/ivpu/ivpu_hw_40xx.c | 28 +++++++++++++++++++--------
> drivers/accel/ivpu/ivpu_hw_40xx_reg.h | 2 ++
> drivers/accel/ivpu/ivpu_ipc.c | 11 ++++-------
> 6 files changed, 37 insertions(+), 19 deletions(-)
>
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2023-09-27 6:05 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-09-25 12:11 [PATCH 0/6] accel/ivpu: Fixes for linux-6.6-rc4 Stanislaw Gruszka
2023-09-25 12:11 ` [PATCH 1/6] accel/ivpu: Do not use wait event interruptible Stanislaw Gruszka
2023-09-26 15:51 ` Jeffrey Hugo
2023-09-25 12:11 ` [PATCH 2/6] accel/ivpu: Don't flood dmesg with VPU ready message Stanislaw Gruszka
2023-09-26 15:52 ` Jeffrey Hugo
2023-09-25 12:11 ` [PATCH 3/6] accel/ivpu/40xx: Ensure clock resource ownership Ack before Power-Up Stanislaw Gruszka
2023-09-26 15:53 ` Jeffrey Hugo
2023-09-25 12:11 ` [PATCH 4/6] accel/ivpu/40xx: Disable frequency change interrupt Stanislaw Gruszka
2023-09-26 15:54 ` Jeffrey Hugo
2023-09-25 12:11 ` [PATCH 5/6] accel/ivpu/40xx: Fix missing VPUIP interrupts Stanislaw Gruszka
2023-09-26 15:55 ` Jeffrey Hugo
2023-09-25 12:11 ` [PATCH 6/6] accel/ivpu: Use cached buffers for FW loading Stanislaw Gruszka
2023-09-26 12:09 ` [PATCH v2 " Stanislaw Gruszka
2023-09-26 15:56 ` Jeffrey Hugo
2023-09-27 6:05 ` [PATCH 0/6] accel/ivpu: Fixes for linux-6.6-rc4 Stanislaw Gruszka
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox