Re: DRM Accel BoF at Linux Plumbers
On 5/21/2024 8:41 AM, Tomeu Vizoso wrote: On Tue, May 21, 2024 at 2:12 PM Daniel Vetter wrote: On Sat, May 18, 2024 at 10:46:01AM +0200, Tomeu Vizoso wrote: Hi, I would like to use the chance at the next Plumbers to discuss the present challenges related to ML accelerators in mainline. I'm myself more oriented towards edge-oriented deployments, and don't know enough about how these accelerators are being used in the cloud (and maybe desktop?) to tell if there is enough overlap to warrant a common BoF. In any case, these are the topics I would like to discuss, some probably more relevant to the edge than to the cloud or desktop: * What is stopping vendors from mainlining their drivers? * How could we make it easier for them? * Userspace API: how close are we from a common API that we can ask userspace drivers to implement? What can be done to further this goal? * Automated testing: DRM CI can be used, but would be good to have a common test suite to run there. This is probably dependent on a common userspace API. * Other shared userspace infrastructure (compiler, execution, synchronization, virtualization, ...) * Firmware-mediated IP: what should we do about it, if anything? * Any standing issues in DRM infra (GEM, gpu scheduler, DMABuf, etc) that are hurting accel drivers? What do people think, should we have a drivers/accel-wide BoF at Plumbers? If so, what other topics should we have in the agenda? Yeah sounds good, and I'll try to at least attend lpc this year since it's rather close ... Might be good to explicitly ping teams of merged and in-flight drivers we have in accel already. Sounds like a good idea to me. Will check if the people that sent the previous aborted attempts are still interested in this Looks like the Intel VPU folks are missing from this thread. I like the idea of a BoF. I suspect I will be remote but this list of topics looks good to me. Nothing obvious missing from what I can tell. -Jeff
Re: [PATCH 12/12] accel/ivpu: Share NPU busy time in sysfs
On 5/8/2024 7:29 AM, Jacek Lawrynowicz wrote: From: Tomasz Rusinowicz The driver tracks the time spent by NPU executing jobs and shares it through sysfs `npu_busy_time_us` file. It can be then used by user space applications to monitor device utilization. NPU is considered 'busy' starting with a first job submitted to firmware and ending when there is no more jobs pending/executing. Signed-off-by: Tomasz Rusinowicz Signed-off-by: Jacek Lawrynowicz This feels like something that would normally be handled by perf. Why not use that mechanism? -Jeff
Re: [PATCH 11/12] accel/ivpu: Increase reset counter when warm boot fails
On 5/8/2024 7:29 AM, Jacek Lawrynowicz wrote: Failed warm boot causes a cold boot that looses FW state and is equivalent to a recovery or reset, so reset_counter should be incremented in order for this failure to be detected by tests. Signed-off-by: Jacek Lawrynowicz Reviewed-by: Jeffrey Hugo
Re: [PATCH 09/12] accel/ivpu: Add force snoop module parameter
On 5/8/2024 7:21 AM, Jacek Lawrynowicz wrote: From: "Wachowski, Karol" Add module parameter that enforces snooping for all NPU accesses, both through MMU PTEs mappings and through TCU page table walk override register bits for MMU page walks / configuration access. Signed-off-by: Wachowski, Karol Signed-off-by: Jacek Lawrynowicz Reviewed-by: Jeffrey Hugo
Re: [PATCH 08/12] accel/ivpu: Add NPU profiling support
On 5/8/2024 7:21 AM, Jacek Lawrynowicz wrote: From: Tomasz Rusinowicz Implement time based Metric Streamer profiling UAPI. This is a generic mechanism allowing user mode tools to sample NPU metrics. These metrics are defined by the FW and transparent to the driver. The user space can check for this feature by checking DRM_IVPU_CAP_METRIC_STREAMER driver capability. Signed-off-by: Tomasz Rusinowicz Signed-off-by: Jacek Lawrynowicz --- drivers/accel/ivpu/Makefile | 3 +- drivers/accel/ivpu/ivpu_drv.c | 14 +- drivers/accel/ivpu/ivpu_drv.h | 3 + drivers/accel/ivpu/ivpu_jsm_msg.c | 98 ++ drivers/accel/ivpu/ivpu_jsm_msg.h | 8 +- drivers/accel/ivpu/ivpu_ms.c | 309 ++ drivers/accel/ivpu/ivpu_ms.h | 36 drivers/accel/ivpu/ivpu_pm.c | 4 + include/uapi/drm/ivpu_accel.h | 69 ++- 9 files changed, 540 insertions(+), 4 deletions(-) create mode 100644 drivers/accel/ivpu/ivpu_ms.c create mode 100644 drivers/accel/ivpu/ivpu_ms.h diff --git a/drivers/accel/ivpu/Makefile b/drivers/accel/ivpu/Makefile index 95ff7ad16338..726cf8f28ea3 100644 --- a/drivers/accel/ivpu/Makefile +++ b/drivers/accel/ivpu/Makefile @@ -1,5 +1,5 @@ # SPDX-License-Identifier: GPL-2.0-only -# Copyright (C) 2023 Intel Corporation +# Copyright (C) 2022-2024 Intel Corporation Are you sure this is correct? Seems odd to be adding 2022 in addition to 2024 at this point. -Jeff
Re: [PATCH 07/12] accel/ivpu: Add resume engine support
On 5/8/2024 7:21 AM, Jacek Lawrynowicz wrote: From: "Wachowski, Karol" Create debugfs interface that triggers sending resume engine IPC command to VPU. Why? Who would use this and for what purpose? -Jeff
Re: [PATCH 06/12] accel/ivpu: Implement support for hardware scheduler
On 5/8/2024 7:21 AM, Jacek Lawrynowicz wrote: +#define IVPU_FOCUS_PRESENT_TIMER_MS 1000 + static char *ivpu_firmware; module_param_named_unsafe(firmware, ivpu_firmware, charp, 0644); MODULE_PARM_DESC(firmware, "NPU firmware binary in /lib/firmware/.."); @@ -467,6 +469,10 @@ static void ivpu_fw_boot_params_print(struct ivpu_device *vdev, struct vpu_boot_ boot_params->punit_telemetry_sram_size); ivpu_dbg(vdev, FW_BOOT, "boot_params.vpu_telemetry_enable = 0x%x\n", boot_params->vpu_telemetry_enable); + ivpu_dbg(vdev, FW_BOOT, "boot_params.vpu_scheduling_mode = 0x%x\n", +boot_params->vpu_scheduling_mode); + ivpu_dbg(vdev, FW_BOOT, "boot_params.vpu_focus_present_timer_ms = %u\n", +boot_params->vpu_focus_present_timer_ms); The timer value is hard coded. Does that not make this log message redundant? -Jeff
Re: [PATCH 05/12] accel/ivpu: Add HWS JSM messages
On 5/8/2024 7:21 AM, Jacek Lawrynowicz wrote: From: "Wachowski, Karol" Add JSM messages that will be used to implement hardware scheduler. Most of these messages are used to create and manage HWS specific command queues. Signed-off-by: Wachowski, Karol Signed-off-by: Jacek Lawrynowicz Reviewed-by: Jeffrey Hugo
Re: [PATCH 04/12] accel/ivpu: Implement support for preemption buffers
On 5/8/2024 7:21 AM, Jacek Lawrynowicz wrote: From: "Wachowski, Karol" Allocate per-context preemption buffers that are required by HWS. There are two preemption buffers: * primary - allocated in user memory range (PIOVA accessible) * secondary - allocated in shave memory range Signed-off-by: Wachowski, Karol Signed-off-by: Jacek Lawrynowicz Reviewed-by: Jeffrey Hugo
Re: [PATCH 03/12] accel/ivpu: Create priority based command queues
On 5/8/2024 7:21 AM, Jacek Lawrynowicz wrote: From: "Wachowski, Karol" Create multiple command queues per engine with different priorities. The cmdqs are created on-demand and they support 4 priority levels. These priorities will later be used by the HWS (hardware scheduler). Signed-off-by: Wachowski, Karol Signed-off-by: Jacek Lawrynowicz Reviewed-by: Jeffrey Hugo
Re: [PATCH 02/12] accel/ivpu: Add sched_mode module param
On 5/8/2024 7:20 AM, Jacek Lawrynowicz wrote: From: "Wachowski, Karol" This param will be used to enable/disable HWS (hardware scheduler). The HWS is a FW side feature and may not be available on all HW generations and FW versions. Signed-off-by: Wachowski, Karol Signed-off-by: Jacek Lawrynowicz --- drivers/accel/ivpu/ivpu_drv.c | 4 drivers/accel/ivpu/ivpu_drv.h | 1 + drivers/accel/ivpu/ivpu_hw.h | 3 ++- drivers/accel/ivpu/ivpu_hw_37xx.c | 1 + drivers/accel/ivpu/ivpu_hw_40xx.c | 3 ++- 5 files changed, 10 insertions(+), 2 deletions(-) diff --git a/drivers/accel/ivpu/ivpu_drv.c b/drivers/accel/ivpu/ivpu_drv.c index 51d3f1a55d02..db47e7ef6322 100644 --- a/drivers/accel/ivpu/ivpu_drv.c +++ b/drivers/accel/ivpu/ivpu_drv.c @@ -51,6 +51,10 @@ u8 ivpu_pll_max_ratio = U8_MAX; module_param_named(pll_max_ratio, ivpu_pll_max_ratio, byte, 0644); MODULE_PARM_DESC(pll_max_ratio, "Maximum PLL ratio used to set NPU frequency"); +bool ivpu_sched_mode; +module_param_named(sched_mode, ivpu_sched_mode, bool, 0644); +MODULE_PARM_DESC(sched_mode, "Scheduler mode: 0 - OS scheduler, 1 - HW scheduler"); "OS scheduler" Host OS (aka Linux) or device side OS? Seems a bit ambiguous. Also looks like this must be set before the device is initialized, yet it does not look like that is communicated. -Jeff
Re: [PATCH 01/12] accel/ivpu: Update VPU FW API headers
On 5/8/2024 7:20 AM, Jacek Lawrynowicz wrote: Update JSM API to 3.16.0. Signed-off-by: Jacek Lawrynowicz Reviewed-by: Jeffrey Hugo
Re: [PATCH] drm/etnaviv: Create an accel device node if compute-only
On 4/24/2024 12:37 AM, Tomeu Vizoso wrote: If we expose a render node for NPUs without rendering capabilities, the userspace stack will offer it to compositors and applications for rendering, which of course won't work. Userspace is probably right in not questioning whether a render node might not be capable of supporting rendering, so change it in the kernel instead by exposing a /dev/accel node. Before we bring the device up we don't know whether it is capable of rendering or not (depends on the features of its blocks), so first try to probe a rendering node, and if we find out that there is no rendering hardware, abort and retry with an accel node. Signed-off-by: Tomeu Vizoso Cc: Oded Gabbay I hope Oded chimes in as Accel maintainer. I think Airlie/Vetter had also previously mentioned they'd have opinions on what is Accel vs DRM. This gets a nack from me in its current state. This is not a strong nack, and I don't want to discourage you. I think there is a path forward. The Accel subsystem documentation says that accel drivers will reside in drivers/accel/ but this does not. Also, the commit text for "accel: add dedicated minor for accelerator devices" mentions - "for drivers that declare they handle compute accelerator, using a new driver feature flag called DRIVER_COMPUTE_ACCEL. It is important to note that this driver feature is mutually exclusive with DRIVER_RENDER. Devices that want to expose both graphics and compute device char files should be handled by two drivers that are connected using the auxiliary bus framework." I don't see any of that happening here (two drivers connected by aux bus, one in drivers/accel). I think this is the first case we've had of a combo DRM/Accel usecase, and so there isn't an existing example to refer you to on how to structure things. I think you are going to be the first example where we figure all of this out. On a more implementation note, ioctls for Accel devices should not be marked DRM_RENDER_ALLOW. Seems like your attempt to reuse as much of the code as possible trips over this. -Jeff
Re: [PATCH] accel/qaic: mark debugfs stub functions as static inline
On 4/9/2024 7:39 AM, Arnd Bergmann wrote:
From: Arnd Bergmann
The alternative stub functions are listed as global, which produces
a build failure in some configs:
In file included from drivers/accel/qaic/qaic_drv.c:31:
drivers/accel/qaic/qaic_debugfs.h:16:5: error: no previous prototype for
'qaic_bootlog_register' [-Werror=missing-prototypes]
16 | int qaic_bootlog_register(void) { return 0; }
| ^
drivers/accel/qaic/qaic_debugfs.h:17:6: error: no previous prototype for
'qaic_bootlog_unregister' [-Werror=missing-prototypes]
17 | void qaic_bootlog_unregister(void) {}
| ^~~
drivers/accel/qaic/qaic_debugfs.h:18:6: error: no previous prototype for
'qaic_debugfs_init' [-Werror=missing-prototypes]
18 | void qaic_debugfs_init(struct qaic_drm_device *qddev) {}
| ^
Make them static inline as intended.
Fixes: 5f8df5c6def6 ("accel/qaic: Add bootlog debugfs")
Signed-off-by: Arnd Bergmann
Pushed to drm-misc-next
-Jeff
Re: [PATCH] accel/qaic: mark debugfs stub functions as static inline
On 4/9/2024 7:39 AM, Arnd Bergmann wrote:
From: Arnd Bergmann
The alternative stub functions are listed as global, which produces
a build failure in some configs:
In file included from drivers/accel/qaic/qaic_drv.c:31:
drivers/accel/qaic/qaic_debugfs.h:16:5: error: no previous prototype for
'qaic_bootlog_register' [-Werror=missing-prototypes]
16 | int qaic_bootlog_register(void) { return 0; }
| ^
drivers/accel/qaic/qaic_debugfs.h:17:6: error: no previous prototype for
'qaic_bootlog_unregister' [-Werror=missing-prototypes]
17 | void qaic_bootlog_unregister(void) {}
| ^~~
drivers/accel/qaic/qaic_debugfs.h:18:6: error: no previous prototype for
'qaic_debugfs_init' [-Werror=missing-prototypes]
18 | void qaic_debugfs_init(struct qaic_drm_device *qddev) {}
| ^
Make them static inline as intended.
Fixes: 5f8df5c6def6 ("accel/qaic: Add bootlog debugfs")
Signed-off-by: Arnd Bergmann
Doh. Thank you for addressing this so quickly.
Reviewed-by: Jeffrey Hugo
Re: [PATCH] accel/qaic: Add Sahara implementation for firmware loading
On 3/21/2024 9:49 PM, Jeffrey Hugo wrote: The AIC100 secondary bootloader uses the Sahara protocol for two purposes - loading the runtime firmware images from the host, and offloading crashdumps to the host. The crashdump functionality is only invoked when the AIC100 device encounters a crash and dumps are enabled. Also the collection of the dump is optional - the host can reject collecting the dump. The Sahara protocol contains many features and modes including firmware upload, crashdump download, and client commands. For simplicity, implement the parts of the protocol needed for loading firmware to the device. Fundamentally, the Sahara protocol is an embedded file transfer protocol. Both sides negotiate a connection through a simple exchange of hello messages. After handshaking through a hello message, the device either sends a message requesting images, or a message advertising the memory dump available for the host. For image transfer, the remote device issues a read data request that provides an image (by ID), an offset, and a length. The host has an internal mapping of image IDs to filenames. The host is expected to access the image and transfer the requested chunk to the device. The device can issue additional read requests, or signal that it has consumed enough data from this image with an end of image message. The host confirms the end of image, and the device can proceed with another image by starting over with the hello exchange again. Some images may be optional, and only provided as part of a provisioning flow. The host is not aware of this information, and thus should report an error to the device when an image is not available. The device will evaluate if the image is required or not, and take the appropriate action. Signed-off-by: Jeffrey Hugo Reviewed-by: Carl Vanderlip Reviewed-by: Pranjal Ramajor Asha Kanojiya Pushed to drm-misc-next -Jeff
Re: [PATCH v2 0/3] accel/qaic: Add debugfs entries
On 3/22/2024 11:57 AM, Jeffrey Hugo wrote: Add 3 debugfs entries that can be useful in debugging a variety of issues. bootlog - output the device bootloader log fifo_size - output the configured dbc fifo size queued - output how many requests are queued in the dbc fifo Bootlog is unique to the device, where as fifo_size/queued is per-dbc. v2: -Use size_add() for bounds check -Move locking into reset_bootlog() -Clamp num dbcs supported to 256 to address a sprintf warning Jeffrey Hugo (3): accel/qaic: Add bootlog debugfs accel/qaic: Add fifo size debugfs accel/qaic: Add fifo queued debugfs drivers/accel/qaic/Makefile | 2 + drivers/accel/qaic/qaic.h | 9 + drivers/accel/qaic/qaic_data.c| 9 + drivers/accel/qaic/qaic_debugfs.c | 338 ++ drivers/accel/qaic/qaic_debugfs.h | 20 ++ drivers/accel/qaic/qaic_drv.c | 16 +- 6 files changed, 393 insertions(+), 1 deletion(-) create mode 100644 drivers/accel/qaic/qaic_debugfs.c create mode 100644 drivers/accel/qaic/qaic_debugfs.h Pushed to drm-misc-next.
Re: [PATCH 8/8] accel/ivpu: Fix deadlock in context_xa
On 4/2/2024 4:49 AM, Jacek Lawrynowicz wrote:
ivpu_device->context_xa is locked both in kernel thread and IRQ context.
It requires XA_FLAGS_LOCK_IRQ flag to be passed during initialization
otherwise the lock could be acquired from a thread and interrupted by
an IRQ that locks it for the second time causing the deadlock.
This deadlock was reported by lockdep and observed in internal tests.
Fixes: 35b137630f08 ("accel/ivpu: Introduce a new DRM driver for Intel VPU")
Cc: # v6.3+
Signed-off-by: Jacek Lawrynowicz
Reviewed-by: Jeffrey Hugo
Re: [PATCH 7/8] accel/ivpu: Fix missed error message after VPU rename
On 4/2/2024 4:49 AM, Jacek Lawrynowicz wrote: Change "VPU" to "NPU" in ivpu_suspend() so it matches all other error messages. Signed-off-by: Jacek Lawrynowicz Reviewed-by: Jeffrey Hugo
Re: [PATCH 6/8] accel/ivpu: Return max freq for DRM_IVPU_PARAM_CORE_CLOCK_RATE
On 4/2/2024 4:49 AM, Jacek Lawrynowicz wrote:
DRM_IVPU_PARAM_CORE_CLOCK_RATE returned current NPU frequency which
Commit text should be present tense, so returned->returns
could be 0 if device was sleeping. This value wasn't really useful to
also wasn't->isn't
the user space, so return max freq instead which can be used to estimate
NPU performance.
Fixes: c39dc15191c4 ("accel/ivpu: Read clock rate only if device is up")
Cc: # v6.7
Signed-off-by: Jacek Lawrynowicz
With the above,
Reviewed-by: Jeffrey Hugo
Re: [PATCH 5/8] accel/ivpu: Improve clarity of MMU error messages
On 4/2/2024 4:49 AM, Jacek Lawrynowicz wrote: From: "Wachowski, Karol" This patch improves readability and clarity of MMU error messages. Previously, the error strings were somewhat confusing and could lead to ambiguous interpretations, making it difficult to diagnose issues. Signed-off-by: Wachowski, Karol Signed-off-by: Jacek Lawrynowicz Reviewed-by: Jeffrey Hugo
Re: [PATCH 4/8] accel/ivpu: Put NPU back to D3hot after failed resume
On 4/2/2024 4:49 AM, Jacek Lawrynowicz wrote:
Put NPU in D3hot after ivpu_resume() fails to power up the device.
This will assure that D3->D0 power cycle will be performed before
the next resume and also will minimize power usage in this corner case.
Fixes: 28083ff18d3f ("accel/ivpu: Fix DevTLB errors on suspend/resume and
recovery")
Cc: # v6.8+
Signed-off-by: Jacek Lawrynowicz
Reviewed-by: Jeffrey Hugo
Re: [PATCH 3/8] accel/ivpu: Fix PCI D0 state entry in resume
On 4/2/2024 4:49 AM, Jacek Lawrynowicz wrote:
From: "Wachowski, Karol"
In case of failed power up we end up left in PCI D3hot
state making it impossible to access NPU registers on retry.
Enter D0 state on retry before proceeding with power up sequence.
Fixes: 28083ff18d3f ("accel/ivpu: Fix DevTLB errors on suspend/resume and
recovery")
Cc: # v6.8+
Signed-off-by: Wachowski, Karol
Signed-off-by: Jacek Lawrynowicz
Reviewed_by: Jeffrey Hugo
Re: [PATCH 2/8] accel/ivpu: Remove d3hot_after_power_off WA
On 4/2/2024 4:49 AM, Jacek Lawrynowicz wrote: Always enter D3hot after entering D0i3 an all platforms. This minimizes power usage. Signed-off-by: Jacek Lawrynowicz Reviewed-by: Jeffrey Hugo
Re: [PATCH 1/8] accel/ivpu: Check return code of ipc->lock init
On 4/2/2024 4:49 AM, Jacek Lawrynowicz wrote:
From: "Wachowski, Karol"
Return value of drmm_mutex_init(ipc->lock) was unchecked.
Fixes: 5d7422cfb498 ("accel/ivpu: Add IPC driver and JSM messages")
Cc: # v6.3+
Signed-off-by: Wachowski, Karol
Signed-off-by: Jacek Lawrynowicz
Reviewed-by: Jeffrey Hugo
[PATCH v2 0/2] drm: Add DRM managed workqueues
Based on work at https://lore.kernel.org/dri-devel/20230118032413.6496-1-jiash...@iscas.ac.cn/ The API in the origional work seemed to have two issues: 1. The output parameter was not correctly defined 2. The allocating functions did not return the allocated object like the other drmm functions I tweaked the implementation to address both of these. >From what I can tell, the i915 change no longer applies to the code base, likely due to refactoring from merging xe. I dropped it. v2: -Fix make htmldocs warnings Jeffrey Hugo (1): accel/qaic: Use drmm_alloc_workqueue() Jiasheng Jiang (1): drm: Add DRM-managed alloc_workqueue() and alloc_ordered_workqueue() drivers/accel/qaic/qaic_drv.c | 30 ++-- drivers/gpu/drm/drm_managed.c | 87 +++ include/drm/drm_managed.h | 8 3 files changed, 99 insertions(+), 26 deletions(-) -- 2.34.1
[PATCH v2 1/2] drm: Add DRM-managed alloc_workqueue() and alloc_ordered_workqueue()
From: Jiasheng Jiang
Add drmm_alloc_workqueue() and drmm_alloc_ordered_workqueue(), the helpers
that provide managed workqueue cleanup. The workqueue will be destroyed
with the final reference of the DRM device.
Signed-off-by: Jiasheng Jiang
Reviewed-by: Daniel Vetter
[jhugo: fix API to return the alloc'd workqueue]
Signed-off-by: Jeffrey Hugo
Reviewed-by: Carl Vanderlip
Reviewed-by: Pranjal Ramajor Asha Kanojiya
---
drivers/gpu/drm/drm_managed.c | 87 +++
include/drm/drm_managed.h | 8
2 files changed, 95 insertions(+)
diff --git a/drivers/gpu/drm/drm_managed.c b/drivers/gpu/drm/drm_managed.c
index 7646f67bda4e..fe31117b3811 100644
--- a/drivers/gpu/drm/drm_managed.c
+++ b/drivers/gpu/drm/drm_managed.c
@@ -310,3 +310,90 @@ void __drmm_mutex_release(struct drm_device *dev, void
*res)
mutex_destroy(lock);
}
EXPORT_SYMBOL(__drmm_mutex_release);
+
+static void drmm_destroy_workqueue(struct drm_device *dev, void *res)
+{
+ struct workqueue_struct *wq = res;
+
+ destroy_workqueue(wq);
+}
+
+/**
+ * drmm_alloc_workqueue - _device-managed alloc_workqueue()
+ * @dev: DRM device
+ * @fmt: printf format for the name of the workqueue
+ * @flags: WQ_* flags
+ * @max_active: max in-flight work items per CPU, 0 for default
+ * remaining args: args for @fmt
+ *
+ * Returns:
+ * Valid pointer on success, NULL on error.
+ *
+ * This is a _device-managed version of alloc_workqueue().
+ * The initialized lock is automatically destroyed on the final
+ * drm_dev_put().
+ */
+struct workqueue_struct *drmm_alloc_workqueue(struct drm_device *dev,
+ const char *fmt, unsigned int
flags,
+ int max_active, ...)
+{
+ struct workqueue_struct *wq;
+ va_list args;
+ int ret;
+
+ va_start(args, max_active);
+ wq = alloc_workqueue(fmt, flags, max_active, args);
+ va_end(args);
+
+ if (!wq)
+ return NULL;
+
+ ret = drmm_add_action_or_reset(dev, drmm_destroy_workqueue, wq);
+ if (ret) {
+ destroy_workqueue(wq);
+ return NULL;
+ }
+
+ return wq;
+}
+EXPORT_SYMBOL(drmm_alloc_workqueue);
+
+/**
+ * drmm_alloc_ordered_workqueue - _device-managed
+ * alloc_ordered_workqueue()
+ * @dev: DRM device
+ * @fmt: printf format for the name of the workqueue
+ * @flags: WQ_* flags
+ * remaining args: args for @fmt
+ *
+ * Returns:
+ * Valid pointer on success, NULL on error.
+ *
+ * This is a _device-managed version of alloc_ordered_workqueue().
+ * The initialized lock is automatically destroyed on the final
+ * drm_dev_put().
+ */
+struct workqueue_struct *drmm_alloc_ordered_workqueue(struct drm_device *dev,
+ const char *fmt,
+ unsigned int flags, ...)
+{
+ struct workqueue_struct *wq;
+ va_list args;
+ int ret;
+
+ va_start(args, flags);
+ wq = alloc_ordered_workqueue(fmt, flags, args);
+ va_end(args);
+
+ if (!wq)
+ return NULL;
+
+ ret = drmm_add_action_or_reset(dev, drmm_destroy_workqueue, wq);
+ if (ret) {
+ destroy_workqueue(wq);
+ return NULL;
+ }
+
+ return wq;
+}
+EXPORT_SYMBOL(drmm_alloc_ordered_workqueue);
diff --git a/include/drm/drm_managed.h b/include/drm/drm_managed.h
index f547b09ca023..cb42fb252648 100644
--- a/include/drm/drm_managed.h
+++ b/include/drm/drm_managed.h
@@ -127,4 +127,12 @@ void __drmm_mutex_release(struct drm_device *dev, void
*res);
drmm_add_action_or_reset(dev, __drmm_mutex_release, lock); \
}) \
+struct workqueue_struct *drmm_alloc_workqueue(struct drm_device *dev,
+ const char *fmt, unsigned int
flags,
+ int max_active, ...);
+
+struct workqueue_struct *drmm_alloc_ordered_workqueue(struct drm_device *dev,
+ const char *fmt,
+ unsigned int flags, ...);
+
#endif
--
2.34.1
[PATCH v2 2/2] accel/qaic: Use drmm_alloc_workqueue()
Now that drmm_alloc_workqueue() exists, we can stop open coding our own
implementation.
Signed-off-by: Jeffrey Hugo
Reviewed-by: Carl Vanderlip
Reviewed-by: Pranjal Ramajor Asha Kanojiya
---
drivers/accel/qaic/qaic_drv.c | 30 --
1 file changed, 4 insertions(+), 26 deletions(-)
diff --git a/drivers/accel/qaic/qaic_drv.c b/drivers/accel/qaic/qaic_drv.c
index d1a632dbaec6..82c0c3ad57bf 100644
--- a/drivers/accel/qaic/qaic_drv.c
+++ b/drivers/accel/qaic/qaic_drv.c
@@ -44,28 +44,6 @@ MODULE_PARM_DESC(datapath_polling, "Operate the datapath in
polling mode");
static bool link_up;
static DEFINE_IDA(qaic_usrs);
-static void qaicm_wq_release(struct drm_device *dev, void *res)
-{
- struct workqueue_struct *wq = res;
-
- destroy_workqueue(wq);
-}
-
-static struct workqueue_struct *qaicm_wq_init(struct drm_device *dev, const
char *fmt)
-{
- struct workqueue_struct *wq;
- int ret;
-
- wq = alloc_workqueue(fmt, WQ_UNBOUND, 0);
- if (!wq)
- return ERR_PTR(-ENOMEM);
- ret = drmm_add_action_or_reset(dev, qaicm_wq_release, wq);
- if (ret)
- return ERR_PTR(ret);
-
- return wq;
-}
-
static void qaicm_srcu_release(struct drm_device *dev, void *res)
{
struct srcu_struct *lock = res;
@@ -383,11 +361,11 @@ static struct qaic_device *create_qdev(struct pci_dev
*pdev, const struct pci_de
if (ret)
return NULL;
- qdev->cntl_wq = qaicm_wq_init(drm, "qaic_cntl");
- if (IS_ERR(qdev->cntl_wq))
+ qdev->cntl_wq = drmm_alloc_workqueue(drm, "qaic_cntl", WQ_UNBOUND,
WQ_UNBOUND_MAX_ACTIVE);
+ if (!qdev->cntl_wq)
return NULL;
- qdev->qts_wq = qaicm_wq_init(drm, "qaic_ts");
- if (IS_ERR(qdev->qts_wq))
+ qdev->qts_wq = drmm_alloc_workqueue(drm, "qaic_ts", WQ_UNBOUND,
WQ_UNBOUND_MAX_ACTIVE);
+ if (!qdev->qts_wq)
return NULL;
ret = qaicm_srcu_init(drm, >dev_lock);
--
2.34.1
[PATCH v2 1/3] accel/qaic: Add bootlog debugfs
During the boot process of AIC100, the bootloaders (PBL and SBL) log
messages to device RAM. During SBL, if the host opens the QAIC_LOGGING
channel, SBL will offload the contents of the log buffer to the host,
and stream any new messages that SBL logs.
This log of the boot process can be very useful for an initial triage of
any boot related issues. For example, if SBL rejects one of the runtime
firmware images for a validation failure, SBL will log a reason why.
Add the ability of the driver to open the logging channel, receive the
messages, and store them. Also define a debugfs entry called "bootlog"
by hooking into the DRM debugfs framework. When the bootlog debugfs
entry is read, the current contents of the log that the host is caching
is displayed to the user. The driver will retain the cache until it
detects that the device has rebooted. At that point, the cache will be
freed, and the driver will wait for a new log. With this scheme, the
driver will only have a cache of the log from the current device boot.
Note that if the driver initializes a device and it is already in the
runtime state (QSM), no bootlog will be available through this mechanism
because the driver and SBL have not communicated.
Signed-off-by: Jeffrey Hugo
Reviewed-by: Carl Vanderlip
Reviewed-by: Pranjal Ramajor Asha Kanojiya
Reviewed-by: Jacek Lawrynowicz
---
drivers/accel/qaic/Makefile | 2 +
drivers/accel/qaic/qaic.h | 8 +
drivers/accel/qaic/qaic_debugfs.c | 272 ++
drivers/accel/qaic/qaic_debugfs.h | 20 +++
drivers/accel/qaic/qaic_drv.c | 16 +-
5 files changed, 317 insertions(+), 1 deletion(-)
create mode 100644 drivers/accel/qaic/qaic_debugfs.c
create mode 100644 drivers/accel/qaic/qaic_debugfs.h
diff --git a/drivers/accel/qaic/Makefile b/drivers/accel/qaic/Makefile
index 3f7f6dfde7f2..2cadcc1baa0e 100644
--- a/drivers/accel/qaic/Makefile
+++ b/drivers/accel/qaic/Makefile
@@ -11,3 +11,5 @@ qaic-y := \
qaic_data.o \
qaic_drv.o \
qaic_timesync.o
+
+qaic-$(CONFIG_DEBUG_FS) += qaic_debugfs.o
diff --git a/drivers/accel/qaic/qaic.h b/drivers/accel/qaic/qaic.h
index 9256653b3036..03d9c9fbffb3 100644
--- a/drivers/accel/qaic/qaic.h
+++ b/drivers/accel/qaic/qaic.h
@@ -153,6 +153,14 @@ struct qaic_device {
struct mhi_device *qts_ch;
/* Work queue for tasks related to MHI "QAIC_TIMESYNC" channel */
struct workqueue_struct *qts_wq;
+ /* Head of list of page allocated by MHI bootlog device */
+ struct list_headbootlog;
+ /* MHI bootlog channel device */
+ struct mhi_device *bootlog_ch;
+ /* Work queue for tasks related to MHI bootlog device */
+ struct workqueue_struct *bootlog_wq;
+ /* Synchronizes access of pages in MHI bootlog device */
+ struct mutexbootlog_mutex;
};
struct qaic_drm_device {
diff --git a/drivers/accel/qaic/qaic_debugfs.c
b/drivers/accel/qaic/qaic_debugfs.c
new file mode 100644
index ..9d0d43fb5b8f
--- /dev/null
+++ b/drivers/accel/qaic/qaic_debugfs.c
@@ -0,0 +1,272 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+/* Copyright (c) 2020, The Linux Foundation. All rights reserved. */
+/* Copyright (c) 2021-2024 Qualcomm Innovation Center, Inc. All rights
reserved. */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include "qaic.h"
+#include "qaic_debugfs.h"
+
+#define BOOTLOG_POOL_SIZE 16
+#define BOOTLOG_MSG_SIZE 512
+
+struct bootlog_msg {
+ /* Buffer for bootlog messages */
+ char str[BOOTLOG_MSG_SIZE];
+ /* Root struct of device, used to access device resources */
+ struct qaic_device *qdev;
+ /* Work struct to schedule work coming on QAIC_LOGGING channel */
+ struct work_struct work;
+};
+
+struct bootlog_page {
+ /* Node in list of bootlog pages maintained by root device struct */
+ struct list_head node;
+ /* Total size of the buffer that holds the bootlogs. It is PAGE_SIZE */
+ unsigned int size;
+ /* Offset for the next bootlog */
+ unsigned int offset;
+};
+
+static int bootlog_show(struct seq_file *s, void *unused)
+{
+ struct bootlog_page *page;
+ struct qaic_device *qdev;
+ void *page_end;
+ void *log;
+
+ qdev = s->private;
+ mutex_lock(>bootlog_mutex);
+ list_for_each_entry(page, >bootlog, node) {
+ log = page + 1;
+ page_end = (void *)page + page->offset;
+ while (log < page_end) {
+ seq_printf(s, "%s", (char *)log);
+ log += strlen(log) + 1;
+ }
+ }
+ mutex_unlock(>bootlog_mutex);
+
+ return 0;
+}
+
+static int bootlog_fops_open(struct inode *inode, struct file *file)
+{
+ return s
[PATCH v2 3/3] accel/qaic: Add fifo queued debugfs
When debugging functional issues with workload input processing, it is
useful to know if requests are backing up in the fifo, or perhaps
getting stuck elsewhere. To answer the question of how many requests are
in the fifo, implement a "queued" debugfs entry per-dbc that returns the
number of pending requests when read.
Signed-off-by: Jeffrey Hugo
Reviewed-by: Carl Vanderlip
Reviewed-by: Pranjal Ramajor Asha Kanojiya
Reviewed-by: Jacek Lawrynowicz
---
drivers/accel/qaic/qaic.h | 1 +
drivers/accel/qaic/qaic_data.c| 9 +
drivers/accel/qaic/qaic_debugfs.c | 31 +++
3 files changed, 41 insertions(+)
diff --git a/drivers/accel/qaic/qaic.h b/drivers/accel/qaic/qaic.h
index 03d9c9fbffb3..02561b6cecc6 100644
--- a/drivers/accel/qaic/qaic.h
+++ b/drivers/accel/qaic/qaic.h
@@ -288,6 +288,7 @@ int disable_dbc(struct qaic_device *qdev, u32 dbc_id,
struct qaic_user *usr);
void enable_dbc(struct qaic_device *qdev, u32 dbc_id, struct qaic_user *usr);
void wakeup_dbc(struct qaic_device *qdev, u32 dbc_id);
void release_dbc(struct qaic_device *qdev, u32 dbc_id);
+void qaic_data_get_fifo_info(struct dma_bridge_chan *dbc, u32 *head, u32
*tail);
void wake_all_cntl(struct qaic_device *qdev);
void qaic_dev_reset_clean_local_state(struct qaic_device *qdev);
diff --git a/drivers/accel/qaic/qaic_data.c b/drivers/accel/qaic/qaic_data.c
index 2459fe4a3f95..e86e71c1cdd8 100644
--- a/drivers/accel/qaic/qaic_data.c
+++ b/drivers/accel/qaic/qaic_data.c
@@ -1981,3 +1981,12 @@ void release_dbc(struct qaic_device *qdev, u32 dbc_id)
dbc->in_use = false;
wake_up(>dbc_release);
}
+
+void qaic_data_get_fifo_info(struct dma_bridge_chan *dbc, u32 *head, u32 *tail)
+{
+ if (!dbc || !head || !tail)
+ return;
+
+ *head = readl(dbc->dbc_base + REQHP_OFF);
+ *tail = readl(dbc->dbc_base + REQTP_OFF);
+}
diff --git a/drivers/accel/qaic/qaic_debugfs.c
b/drivers/accel/qaic/qaic_debugfs.c
index b362960941d7..20b653d99e52 100644
--- a/drivers/accel/qaic/qaic_debugfs.c
+++ b/drivers/accel/qaic/qaic_debugfs.c
@@ -98,6 +98,36 @@ static const struct file_operations fifo_size_fops = {
.release = single_release,
};
+static int read_dbc_queued(struct seq_file *s, void *unused)
+{
+ struct dma_bridge_chan *dbc = s->private;
+ u32 tail = 0, head = 0;
+
+ qaic_data_get_fifo_info(dbc, , );
+
+ if (head == U32_MAX || tail == U32_MAX)
+ seq_printf(s, "%u\n", 0);
+ else if (head > tail)
+ seq_printf(s, "%u\n", dbc->nelem - head + tail);
+ else
+ seq_printf(s, "%u\n", tail - head);
+
+ return 0;
+}
+
+static int queued_open(struct inode *inode, struct file *file)
+{
+ return single_open(file, read_dbc_queued, inode->i_private);
+}
+
+static const struct file_operations queued_fops = {
+ .owner = THIS_MODULE,
+ .open = queued_open,
+ .read = seq_read,
+ .llseek = seq_lseek,
+ .release = single_release,
+};
+
void qaic_debugfs_init(struct qaic_drm_device *qddev)
{
struct qaic_device *qdev = qddev->qdev;
@@ -117,6 +147,7 @@ void qaic_debugfs_init(struct qaic_drm_device *qddev)
snprintf(name, QAIC_DBC_DIR_NAME, "dbc%03u", i);
debugfs_dir = debugfs_create_dir(name, debugfs_root);
debugfs_create_file("fifo_size", 0400, debugfs_dir,
>dbc[i], _size_fops);
+ debugfs_create_file("queued", 0400, debugfs_dir, >dbc[i],
_fops);
}
}
--
2.34.1
[PATCH v2 2/3] accel/qaic: Add fifo size debugfs
Each DMA Bridge Channel (dbc) has a unique configured fifo size which is
specified by the userspace client of that dbc. Since the fifo is
circular, it is useful to know the configured size when debugging
issues.
Add a per-dbc subdirectory in debugfs and in each subdirectory add a
fifo_size entry that will display the size of that dbc's fifo when read.
Signed-off-by: Jeffrey Hugo
Reviewed-by: Carl Vanderlip
Reviewed-by: Pranjal Ramajor Asha Kanojiya
Reviewed-by: Jacek Lawrynowicz
---
drivers/accel/qaic/qaic_debugfs.c | 35 +++
1 file changed, 35 insertions(+)
diff --git a/drivers/accel/qaic/qaic_debugfs.c
b/drivers/accel/qaic/qaic_debugfs.c
index 9d0d43fb5b8f..b362960941d7 100644
--- a/drivers/accel/qaic/qaic_debugfs.c
+++ b/drivers/accel/qaic/qaic_debugfs.c
@@ -12,6 +12,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -21,6 +22,7 @@
#define BOOTLOG_POOL_SIZE 16
#define BOOTLOG_MSG_SIZE 512
+#define QAIC_DBC_DIR_NAME 9
struct bootlog_msg {
/* Buffer for bootlog messages */
@@ -75,14 +77,47 @@ static const struct file_operations bootlog_fops = {
.release = single_release,
};
+static int read_dbc_fifo_size(struct seq_file *s, void *unused)
+{
+ struct dma_bridge_chan *dbc = s->private;
+
+ seq_printf(s, "%u\n", dbc->nelem);
+ return 0;
+}
+
+static int fifo_size_open(struct inode *inode, struct file *file)
+{
+ return single_open(file, read_dbc_fifo_size, inode->i_private);
+}
+
+static const struct file_operations fifo_size_fops = {
+ .owner = THIS_MODULE,
+ .open = fifo_size_open,
+ .read = seq_read,
+ .llseek = seq_lseek,
+ .release = single_release,
+};
+
void qaic_debugfs_init(struct qaic_drm_device *qddev)
{
struct qaic_device *qdev = qddev->qdev;
struct dentry *debugfs_root;
+ struct dentry *debugfs_dir;
+ char name[QAIC_DBC_DIR_NAME];
+ u32 i;
debugfs_root = to_drm(qddev)->debugfs_root;
debugfs_create_file("bootlog", 0400, debugfs_root, qdev, _fops);
+ /*
+* 256 dbcs per device is likely the max we will ever see and lets
static checking see a
+* reasonable range.
+*/
+ for (i = 0; i < qdev->num_dbc && i < 256; ++i) {
+ snprintf(name, QAIC_DBC_DIR_NAME, "dbc%03u", i);
+ debugfs_dir = debugfs_create_dir(name, debugfs_root);
+ debugfs_create_file("fifo_size", 0400, debugfs_dir,
>dbc[i], _size_fops);
+ }
}
static struct bootlog_page *alloc_bootlog_page(struct qaic_device *qdev)
--
2.34.1
[PATCH v2 0/3] accel/qaic: Add debugfs entries
Add 3 debugfs entries that can be useful in debugging a variety of issues. bootlog - output the device bootloader log fifo_size - output the configured dbc fifo size queued - output how many requests are queued in the dbc fifo Bootlog is unique to the device, where as fifo_size/queued is per-dbc. v2: -Use size_add() for bounds check -Move locking into reset_bootlog() -Clamp num dbcs supported to 256 to address a sprintf warning Jeffrey Hugo (3): accel/qaic: Add bootlog debugfs accel/qaic: Add fifo size debugfs accel/qaic: Add fifo queued debugfs drivers/accel/qaic/Makefile | 2 + drivers/accel/qaic/qaic.h | 9 + drivers/accel/qaic/qaic_data.c| 9 + drivers/accel/qaic/qaic_debugfs.c | 338 ++ drivers/accel/qaic/qaic_debugfs.h | 20 ++ drivers/accel/qaic/qaic_drv.c | 16 +- 6 files changed, 393 insertions(+), 1 deletion(-) create mode 100644 drivers/accel/qaic/qaic_debugfs.c create mode 100644 drivers/accel/qaic/qaic_debugfs.h -- 2.34.1
[PATCH] accel/qaic: Add Sahara implementation for firmware loading
The AIC100 secondary bootloader uses the Sahara protocol for two
purposes - loading the runtime firmware images from the host, and
offloading crashdumps to the host. The crashdump functionality is only
invoked when the AIC100 device encounters a crash and dumps are enabled.
Also the collection of the dump is optional - the host can reject
collecting the dump.
The Sahara protocol contains many features and modes including firmware
upload, crashdump download, and client commands. For simplicity,
implement the parts of the protocol needed for loading firmware to the
device.
Fundamentally, the Sahara protocol is an embedded file transfer
protocol. Both sides negotiate a connection through a simple exchange of
hello messages. After handshaking through a hello message, the device
either sends a message requesting images, or a message advertising the
memory dump available for the host. For image transfer, the remote device
issues a read data request that provides an image (by ID), an offset, and
a length. The host has an internal mapping of image IDs to filenames. The
host is expected to access the image and transfer the requested chunk to
the device. The device can issue additional read requests, or signal that
it has consumed enough data from this image with an end of image message.
The host confirms the end of image, and the device can proceed with
another image by starting over with the hello exchange again.
Some images may be optional, and only provided as part of a provisioning
flow. The host is not aware of this information, and thus should report
an error to the device when an image is not available. The device will
evaluate if the image is required or not, and take the appropriate
action.
Signed-off-by: Jeffrey Hugo
Reviewed-by: Carl Vanderlip
Reviewed-by: Pranjal Ramajor Asha Kanojiya
---
drivers/accel/qaic/Makefile | 3 +-
drivers/accel/qaic/qaic_drv.c | 10 +
drivers/accel/qaic/sahara.c | 450 ++
drivers/accel/qaic/sahara.h | 10 +
4 files changed, 472 insertions(+), 1 deletion(-)
create mode 100644 drivers/accel/qaic/sahara.c
create mode 100644 drivers/accel/qaic/sahara.h
diff --git a/drivers/accel/qaic/Makefile b/drivers/accel/qaic/Makefile
index 3f7f6dfde7f2..df02c1c0d6a6 100644
--- a/drivers/accel/qaic/Makefile
+++ b/drivers/accel/qaic/Makefile
@@ -10,4 +10,5 @@ qaic-y := \
qaic_control.o \
qaic_data.o \
qaic_drv.o \
- qaic_timesync.o
+ qaic_timesync.o \
+ sahara.o
diff --git a/drivers/accel/qaic/qaic_drv.c b/drivers/accel/qaic/qaic_drv.c
index d1a632dbaec6..ccfbac88c724 100644
--- a/drivers/accel/qaic/qaic_drv.c
+++ b/drivers/accel/qaic/qaic_drv.c
@@ -29,6 +29,7 @@
#include "mhi_controller.h"
#include "qaic.h"
#include "qaic_timesync.h"
+#include "sahara.h"
MODULE_IMPORT_NS(DMA_BUF);
@@ -635,12 +636,20 @@ static int __init qaic_init(void)
goto free_pci;
}
+ ret = sahara_register();
+ if (ret) {
+ pr_debug("qaic: sahara_register failed %d\n", ret);
+ goto free_mhi;
+ }
+
ret = qaic_timesync_init();
if (ret)
pr_debug("qaic: qaic_timesync_init failed %d\n", ret);
return 0;
+free_mhi:
+ mhi_driver_unregister(_mhi_driver);
free_pci:
pci_unregister_driver(_pci_driver);
return ret;
@@ -665,6 +674,7 @@ static void __exit qaic_exit(void)
*/
link_up = true;
qaic_timesync_deinit();
+ sahara_unregister();
mhi_driver_unregister(_mhi_driver);
pci_unregister_driver(_pci_driver);
}
diff --git a/drivers/accel/qaic/sahara.c b/drivers/accel/qaic/sahara.c
new file mode 100644
index ..d5da8e166998
--- /dev/null
+++ b/drivers/accel/qaic/sahara.c
@@ -0,0 +1,450 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+/* Copyright (c) 2024 Qualcomm Innovation Center, Inc. All rights reserved. */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include "sahara.h"
+
+#define SAHARA_HELLO_CMD 0x1 /* Min protocol version 1.0 */
+#define SAHARA_HELLO_RESP_CMD 0x2 /* Min protocol version 1.0 */
+#define SAHARA_READ_DATA_CMD 0x3 /* Min protocol version 1.0 */
+#define SAHARA_END_OF_IMAGE_CMD0x4 /* Min protocol version
1.0 */
+#define SAHARA_DONE_CMD0x5 /* Min protocol version
1.0 */
+#define SAHARA_DONE_RESP_CMD 0x6 /* Min protocol version 1.0 */
+#define SAHARA_RESET_CMD 0x7 /* Min protocol version 1.0 */
+#define SAHARA_RESET_RESP_CMD 0x8 /* Min protocol version 1.0 */
+#define SAHARA_MEM_DEBUG_CMD 0x9 /* Min protocol version 2.0 */
+#define SAHARA_MEM_READ_CMD0xa /* Min protocol version 2.0 */
+#define SAHARA_CMD_READY_CMD 0xb /* Min protocol version 2.1 */
+#define SAHAR
Re: [PATCH 1/3] accel/qaic: Add bootlog debugfs
On 3/14/2024 5:41 AM, Jacek Lawrynowicz wrote:
Hi,
On 11.03.2024 17:58, Jeffrey Hugo wrote:
During the boot process of AIC100, the bootloaders (PBL and SBL) log
messages to device RAM. During SBL, if the host opens the QAIC_LOGGING
channel, SBL will offload the contents of the log buffer to the host,
and stream any new messages that SBL logs.
This log of the boot process can be very useful for an initial triage of
any boot related issues. For example, if SBL rejects one of the runtime
firmware images for a validation failure, SBL will log a reason why.
Add the ability of the driver to open the logging channel, receive the
messages, and store them. Also define a debugfs entry called "bootlog"
by hooking into the DRM debugfs framework. When the bootlog debugfs
entry is read, the current contents of the log that the host is caching
is displayed to the user. The driver will retain the cache until it
detects that the device has rebooted. At that point, the cache will be
freed, and the driver will wait for a new log. With this scheme, the
driver will only have a cache of the log from the current device boot.
Note that if the driver initializes a device and it is already in the
runtime state (QSM), no bootlog will be available through this mechanism
because the driver and SBL have not communicated.
Signed-off-by: Jeffrey Hugo
Reviewed-by: Carl Vanderlip
Reviewed-by: Pranjal Ramajor Asha Kanojiya
---
drivers/accel/qaic/Makefile | 2 +
drivers/accel/qaic/qaic.h | 8 +
drivers/accel/qaic/qaic_debugfs.c | 271 ++
drivers/accel/qaic/qaic_debugfs.h | 20 +++
drivers/accel/qaic/qaic_drv.c | 16 +-
5 files changed, 316 insertions(+), 1 deletion(-)
create mode 100644 drivers/accel/qaic/qaic_debugfs.c
create mode 100644 drivers/accel/qaic/qaic_debugfs.h
diff --git a/drivers/accel/qaic/Makefile b/drivers/accel/qaic/Makefile
index 3f7f6dfde7f2..2cadcc1baa0e 100644
--- a/drivers/accel/qaic/Makefile
+++ b/drivers/accel/qaic/Makefile
@@ -11,3 +11,5 @@ qaic-y := \
qaic_data.o \
qaic_drv.o \
qaic_timesync.o
+
+qaic-$(CONFIG_DEBUG_FS) += qaic_debugfs.o
diff --git a/drivers/accel/qaic/qaic.h b/drivers/accel/qaic/qaic.h
index 9256653b3036..03d9c9fbffb3 100644
--- a/drivers/accel/qaic/qaic.h
+++ b/drivers/accel/qaic/qaic.h
@@ -153,6 +153,14 @@ struct qaic_device {
struct mhi_device *qts_ch;
/* Work queue for tasks related to MHI "QAIC_TIMESYNC" channel */
struct workqueue_struct *qts_wq;
+ /* Head of list of page allocated by MHI bootlog device */
+ struct list_headbootlog;
+ /* MHI bootlog channel device */
+ struct mhi_device *bootlog_ch;
+ /* Work queue for tasks related to MHI bootlog device */
+ struct workqueue_struct *bootlog_wq;
+ /* Synchronizes access of pages in MHI bootlog device */
+ struct mutexbootlog_mutex;
};
struct qaic_drm_device {
diff --git a/drivers/accel/qaic/qaic_debugfs.c
b/drivers/accel/qaic/qaic_debugfs.c
new file mode 100644
index ..4f87fe29be1a
--- /dev/null
+++ b/drivers/accel/qaic/qaic_debugfs.c
@@ -0,0 +1,271 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+/* Copyright (c) 2020, The Linux Foundation. All rights reserved. */
+/* Copyright (c) 2021-2024 Qualcomm Innovation Center, Inc. All rights
reserved. */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include "qaic.h"
+#include "qaic_debugfs.h"
+
+#define BOOTLOG_POOL_SIZE 16
+#define BOOTLOG_MSG_SIZE 512
+
+struct bootlog_msg {
+ /* Buffer for bootlog messages */
+ char str[BOOTLOG_MSG_SIZE];
+ /* Root struct of device, used to access device resources */
+ struct qaic_device *qdev;
+ /* Work struct to schedule work coming on QAIC_LOGGING channel */
+ struct work_struct work;
+};
+
+struct bootlog_page {
+ /* Node in list of bootlog pages maintained by root device struct */
+ struct list_head node;
+ /* Total size of the buffer that holds the bootlogs. It is PAGE_SIZE */
+ unsigned int size;
+ /* Offset for the next bootlog */
+ unsigned int offset;
+};
+
+static int bootlog_show(struct seq_file *s, void *unused)
+{
+ struct bootlog_page *page;
+ struct qaic_device *qdev;
+ void *page_end;
+ void *log;
+
+ qdev = s->private;
+ mutex_lock(>bootlog_mutex);
+ list_for_each_entry(page, >bootlog, node) {
+ log = page + 1;
+ page_end = (void *)page + page->offset;
+ while (log < page_end) {
+ seq_printf(s, "%s", (char *)log);
+ log += strlen(log) + 1;
+ }
+ }
+ mutex_unlock(>bootlog_mutex);
+
+ return 0;
+}
+
+static int bootlog
[PATCH 0/2] drm: Add DRM managed workqueues
Based on work at https://lore.kernel.org/dri-devel/20230118032413.6496-1-jiash...@iscas.ac.cn/ The API in the origional work seemed to have two issues: 1. The output parameter was not correctly defined 2. The allocating functions did not return the allocated object like the other drmm functions I tweaked the implementation to address both of these. >From what I can tell, the i915 change no longer applies to the code base, likely due to refactoring from merging xe. I dropped it. Jeffrey Hugo (1): accel/qaic: Use drmm_alloc_workqueue() Jiasheng Jiang (1): drm: Add DRM-managed alloc_workqueue() and alloc_ordered_workqueue() drivers/accel/qaic/qaic_drv.c | 30 ++--- drivers/gpu/drm/drm_managed.c | 82 +++ include/drm/drm_managed.h | 8 3 files changed, 94 insertions(+), 26 deletions(-) -- 2.34.1
[PATCH 2/2] accel/qaic: Use drmm_alloc_workqueue()
Now that drmm_alloc_workqueue() exists, we can stop open coding our own
implementation.
Signed-off-by: Jeffrey Hugo
Reviewed-by: Carl Vanderlip
Reviewed-by: Pranjal Ramajor Asha Kanojiya
---
drivers/accel/qaic/qaic_drv.c | 30 --
1 file changed, 4 insertions(+), 26 deletions(-)
diff --git a/drivers/accel/qaic/qaic_drv.c b/drivers/accel/qaic/qaic_drv.c
index f072edb74f22..9bc09b87a7e1 100644
--- a/drivers/accel/qaic/qaic_drv.c
+++ b/drivers/accel/qaic/qaic_drv.c
@@ -45,28 +45,6 @@ MODULE_PARM_DESC(datapath_polling, "Operate the datapath in
polling mode");
static bool link_up;
static DEFINE_IDA(qaic_usrs);
-static void qaicm_wq_release(struct drm_device *dev, void *res)
-{
- struct workqueue_struct *wq = res;
-
- destroy_workqueue(wq);
-}
-
-static struct workqueue_struct *qaicm_wq_init(struct drm_device *dev, const
char *fmt)
-{
- struct workqueue_struct *wq;
- int ret;
-
- wq = alloc_workqueue(fmt, WQ_UNBOUND, 0);
- if (!wq)
- return ERR_PTR(-ENOMEM);
- ret = drmm_add_action_or_reset(dev, qaicm_wq_release, wq);
- if (ret)
- return ERR_PTR(ret);
-
- return wq;
-}
-
static void qaicm_srcu_release(struct drm_device *dev, void *res)
{
struct srcu_struct *lock = res;
@@ -391,11 +369,11 @@ static struct qaic_device *create_qdev(struct pci_dev
*pdev, const struct pci_de
if (ret)
return NULL;
- qdev->cntl_wq = qaicm_wq_init(drm, "qaic_cntl");
- if (IS_ERR(qdev->cntl_wq))
+ qdev->cntl_wq = drmm_alloc_workqueue(drm, "qaic_cntl", WQ_UNBOUND,
WQ_UNBOUND_MAX_ACTIVE);
+ if (!qdev->cntl_wq)
return NULL;
- qdev->qts_wq = qaicm_wq_init(drm, "qaic_ts");
- if (IS_ERR(qdev->qts_wq))
+ qdev->qts_wq = drmm_alloc_workqueue(drm, "qaic_ts", WQ_UNBOUND,
WQ_UNBOUND_MAX_ACTIVE);
+ if (!qdev->qts_wq)
return NULL;
ret = qaicm_srcu_init(drm, >dev_lock);
--
2.34.1
[PATCH 1/2] drm: Add DRM-managed alloc_workqueue() and alloc_ordered_workqueue()
From: Jiasheng Jiang
Add drmm_alloc_workqueue() and drmm_alloc_ordered_workqueue(), the helpers
that provide managed workqueue cleanup. The workqueue will be destroyed
with the final reference of the DRM device.
Signed-off-by: Jiasheng Jiang
Reviewed-by: Daniel Vetter
[jhugo: fix API to return the alloc'd workqueue]
Signed-off-by: Jeffrey Hugo
Reviewed-by: Carl Vanderlip
Reviewed-by: Pranjal Ramajor Asha Kanojiya
---
drivers/gpu/drm/drm_managed.c | 82 +++
include/drm/drm_managed.h | 8
2 files changed, 90 insertions(+)
diff --git a/drivers/gpu/drm/drm_managed.c b/drivers/gpu/drm/drm_managed.c
index 7646f67bda4e..d5135a471ff2 100644
--- a/drivers/gpu/drm/drm_managed.c
+++ b/drivers/gpu/drm/drm_managed.c
@@ -310,3 +310,85 @@ void __drmm_mutex_release(struct drm_device *dev, void
*res)
mutex_destroy(lock);
}
EXPORT_SYMBOL(__drmm_mutex_release);
+
+static void drmm_destroy_workqueue(struct drm_device *dev, void *res)
+{
+ struct workqueue_struct *wq = res;
+
+ destroy_workqueue(wq);
+}
+
+/**
+ * drmm_alloc_workqueue - _device-managed alloc_workqueue()
+ * @dev: DRM device
+ * @wq: workqueue to be allocated
+ *
+ * Returns:
+ * Valid pointer on success, NULL on error.
+ *
+ * This is a _device-managed version of alloc_workqueue().
+ * The initialized lock is automatically destroyed on the final
+ * drm_dev_put().
+ */
+struct workqueue_struct *drmm_alloc_workqueue(struct drm_device *dev,
+ const char *fmt, unsigned int
flags,
+ int max_active, ...)
+{
+ struct workqueue_struct *wq;
+ va_list args;
+ int ret;
+
+ va_start(args, max_active);
+ wq = alloc_workqueue(fmt, flags, max_active, args);
+ va_end(args);
+
+ if (!wq)
+ return NULL;
+
+ ret = drmm_add_action_or_reset(dev, drmm_destroy_workqueue, wq);
+ if (ret) {
+ destroy_workqueue(wq);
+ return NULL;
+ }
+
+ return wq;
+}
+EXPORT_SYMBOL(drmm_alloc_workqueue);
+
+/**
+ * drmm_alloc_ordered_workqueue - _device-managed
+ * alloc_ordered_workqueue()
+ * @dev: DRM device
+ * @wq: workqueue to be allocated
+ *
+ * Returns:
+ * Valid pointer on success, NULL on error.
+ *
+ * This is a _device-managed version of alloc_ordered_workqueue().
+ * The initialized lock is automatically destroyed on the final
+ * drm_dev_put().
+ */
+struct workqueue_struct *drmm_alloc_ordered_workqueue(struct drm_device *dev,
+ const char *fmt,
+ unsigned int flags, ...)
+{
+ struct workqueue_struct *wq;
+ va_list args;
+ int ret;
+
+ va_start(args, flags);
+ wq = alloc_ordered_workqueue(fmt, flags, args);
+ va_end(args);
+
+ if (!wq)
+ return NULL;
+
+ ret = drmm_add_action_or_reset(dev, drmm_destroy_workqueue, wq);
+ if (ret) {
+ destroy_workqueue(wq);
+ return NULL;
+ }
+
+ return wq;
+}
+EXPORT_SYMBOL(drmm_alloc_ordered_workqueue);
diff --git a/include/drm/drm_managed.h b/include/drm/drm_managed.h
index f547b09ca023..cb42fb252648 100644
--- a/include/drm/drm_managed.h
+++ b/include/drm/drm_managed.h
@@ -127,4 +127,12 @@ void __drmm_mutex_release(struct drm_device *dev, void
*res);
drmm_add_action_or_reset(dev, __drmm_mutex_release, lock); \
}) \
+struct workqueue_struct *drmm_alloc_workqueue(struct drm_device *dev,
+ const char *fmt, unsigned int
flags,
+ int max_active, ...);
+
+struct workqueue_struct *drmm_alloc_ordered_workqueue(struct drm_device *dev,
+ const char *fmt,
+ unsigned int flags, ...);
+
#endif
--
2.34.1
[PATCH 3/3] accel/qaic: Add fifo queued debugfs
When debugging functional issues with workload input processing, it is
useful to know if requests are backing up in the fifo, or perhaps
getting stuck elsewhere. To answer the question of how many requests are
in the fifo, implement a "queued" debugfs entry per-dbc that returns the
number of pending requests when read.
Signed-off-by: Jeffrey Hugo
Reviewed-by: Carl Vanderlip
Reviewed-by: Pranjal Ramajor Asha Kanojiya
---
drivers/accel/qaic/qaic.h | 1 +
drivers/accel/qaic/qaic_data.c| 9 +
drivers/accel/qaic/qaic_debugfs.c | 31 +++
3 files changed, 41 insertions(+)
diff --git a/drivers/accel/qaic/qaic.h b/drivers/accel/qaic/qaic.h
index 03d9c9fbffb3..02561b6cecc6 100644
--- a/drivers/accel/qaic/qaic.h
+++ b/drivers/accel/qaic/qaic.h
@@ -288,6 +288,7 @@ int disable_dbc(struct qaic_device *qdev, u32 dbc_id,
struct qaic_user *usr);
void enable_dbc(struct qaic_device *qdev, u32 dbc_id, struct qaic_user *usr);
void wakeup_dbc(struct qaic_device *qdev, u32 dbc_id);
void release_dbc(struct qaic_device *qdev, u32 dbc_id);
+void qaic_data_get_fifo_info(struct dma_bridge_chan *dbc, u32 *head, u32
*tail);
void wake_all_cntl(struct qaic_device *qdev);
void qaic_dev_reset_clean_local_state(struct qaic_device *qdev);
diff --git a/drivers/accel/qaic/qaic_data.c b/drivers/accel/qaic/qaic_data.c
index 2459fe4a3f95..e86e71c1cdd8 100644
--- a/drivers/accel/qaic/qaic_data.c
+++ b/drivers/accel/qaic/qaic_data.c
@@ -1981,3 +1981,12 @@ void release_dbc(struct qaic_device *qdev, u32 dbc_id)
dbc->in_use = false;
wake_up(>dbc_release);
}
+
+void qaic_data_get_fifo_info(struct dma_bridge_chan *dbc, u32 *head, u32 *tail)
+{
+ if (!dbc || !head || !tail)
+ return;
+
+ *head = readl(dbc->dbc_base + REQHP_OFF);
+ *tail = readl(dbc->dbc_base + REQTP_OFF);
+}
diff --git a/drivers/accel/qaic/qaic_debugfs.c
b/drivers/accel/qaic/qaic_debugfs.c
index 9d56cd451b64..12a65b98701d 100644
--- a/drivers/accel/qaic/qaic_debugfs.c
+++ b/drivers/accel/qaic/qaic_debugfs.c
@@ -97,6 +97,36 @@ static const struct file_operations fifo_size_fops = {
.release = single_release,
};
+static int read_dbc_queued(struct seq_file *s, void *unused)
+{
+ struct dma_bridge_chan *dbc = s->private;
+ u32 tail = 0, head = 0;
+
+ qaic_data_get_fifo_info(dbc, , );
+
+ if (head == U32_MAX || tail == U32_MAX)
+ seq_printf(s, "%u\n", 0);
+ else if (head > tail)
+ seq_printf(s, "%u\n", dbc->nelem - head + tail);
+ else
+ seq_printf(s, "%u\n", tail - head);
+
+ return 0;
+}
+
+static int queued_open(struct inode *inode, struct file *file)
+{
+ return single_open(file, read_dbc_queued, inode->i_private);
+}
+
+static const struct file_operations queued_fops = {
+ .owner = THIS_MODULE,
+ .open = queued_open,
+ .read = seq_read,
+ .llseek = seq_lseek,
+ .release = single_release,
+};
+
void qaic_debugfs_init(struct qaic_drm_device *qddev)
{
struct qaic_device *qdev = qddev->qdev;
@@ -112,6 +142,7 @@ void qaic_debugfs_init(struct qaic_drm_device *qddev)
snprintf(name, QAIC_DBC_DIR_NAME, "dbc%03u", i);
debugfs_dir = debugfs_create_dir(name, debugfs_root);
debugfs_create_file("fifo_size", 0400, debugfs_dir,
>dbc[i], _size_fops);
+ debugfs_create_file("queued", 0400, debugfs_dir, >dbc[i],
_fops);
}
}
--
2.34.1
[PATCH 1/3] accel/qaic: Add bootlog debugfs
During the boot process of AIC100, the bootloaders (PBL and SBL) log
messages to device RAM. During SBL, if the host opens the QAIC_LOGGING
channel, SBL will offload the contents of the log buffer to the host,
and stream any new messages that SBL logs.
This log of the boot process can be very useful for an initial triage of
any boot related issues. For example, if SBL rejects one of the runtime
firmware images for a validation failure, SBL will log a reason why.
Add the ability of the driver to open the logging channel, receive the
messages, and store them. Also define a debugfs entry called "bootlog"
by hooking into the DRM debugfs framework. When the bootlog debugfs
entry is read, the current contents of the log that the host is caching
is displayed to the user. The driver will retain the cache until it
detects that the device has rebooted. At that point, the cache will be
freed, and the driver will wait for a new log. With this scheme, the
driver will only have a cache of the log from the current device boot.
Note that if the driver initializes a device and it is already in the
runtime state (QSM), no bootlog will be available through this mechanism
because the driver and SBL have not communicated.
Signed-off-by: Jeffrey Hugo
Reviewed-by: Carl Vanderlip
Reviewed-by: Pranjal Ramajor Asha Kanojiya
---
drivers/accel/qaic/Makefile | 2 +
drivers/accel/qaic/qaic.h | 8 +
drivers/accel/qaic/qaic_debugfs.c | 271 ++
drivers/accel/qaic/qaic_debugfs.h | 20 +++
drivers/accel/qaic/qaic_drv.c | 16 +-
5 files changed, 316 insertions(+), 1 deletion(-)
create mode 100644 drivers/accel/qaic/qaic_debugfs.c
create mode 100644 drivers/accel/qaic/qaic_debugfs.h
diff --git a/drivers/accel/qaic/Makefile b/drivers/accel/qaic/Makefile
index 3f7f6dfde7f2..2cadcc1baa0e 100644
--- a/drivers/accel/qaic/Makefile
+++ b/drivers/accel/qaic/Makefile
@@ -11,3 +11,5 @@ qaic-y := \
qaic_data.o \
qaic_drv.o \
qaic_timesync.o
+
+qaic-$(CONFIG_DEBUG_FS) += qaic_debugfs.o
diff --git a/drivers/accel/qaic/qaic.h b/drivers/accel/qaic/qaic.h
index 9256653b3036..03d9c9fbffb3 100644
--- a/drivers/accel/qaic/qaic.h
+++ b/drivers/accel/qaic/qaic.h
@@ -153,6 +153,14 @@ struct qaic_device {
struct mhi_device *qts_ch;
/* Work queue for tasks related to MHI "QAIC_TIMESYNC" channel */
struct workqueue_struct *qts_wq;
+ /* Head of list of page allocated by MHI bootlog device */
+ struct list_headbootlog;
+ /* MHI bootlog channel device */
+ struct mhi_device *bootlog_ch;
+ /* Work queue for tasks related to MHI bootlog device */
+ struct workqueue_struct *bootlog_wq;
+ /* Synchronizes access of pages in MHI bootlog device */
+ struct mutexbootlog_mutex;
};
struct qaic_drm_device {
diff --git a/drivers/accel/qaic/qaic_debugfs.c
b/drivers/accel/qaic/qaic_debugfs.c
new file mode 100644
index ..4f87fe29be1a
--- /dev/null
+++ b/drivers/accel/qaic/qaic_debugfs.c
@@ -0,0 +1,271 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+/* Copyright (c) 2020, The Linux Foundation. All rights reserved. */
+/* Copyright (c) 2021-2024 Qualcomm Innovation Center, Inc. All rights
reserved. */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include "qaic.h"
+#include "qaic_debugfs.h"
+
+#define BOOTLOG_POOL_SIZE 16
+#define BOOTLOG_MSG_SIZE 512
+
+struct bootlog_msg {
+ /* Buffer for bootlog messages */
+ char str[BOOTLOG_MSG_SIZE];
+ /* Root struct of device, used to access device resources */
+ struct qaic_device *qdev;
+ /* Work struct to schedule work coming on QAIC_LOGGING channel */
+ struct work_struct work;
+};
+
+struct bootlog_page {
+ /* Node in list of bootlog pages maintained by root device struct */
+ struct list_head node;
+ /* Total size of the buffer that holds the bootlogs. It is PAGE_SIZE */
+ unsigned int size;
+ /* Offset for the next bootlog */
+ unsigned int offset;
+};
+
+static int bootlog_show(struct seq_file *s, void *unused)
+{
+ struct bootlog_page *page;
+ struct qaic_device *qdev;
+ void *page_end;
+ void *log;
+
+ qdev = s->private;
+ mutex_lock(>bootlog_mutex);
+ list_for_each_entry(page, >bootlog, node) {
+ log = page + 1;
+ page_end = (void *)page + page->offset;
+ while (log < page_end) {
+ seq_printf(s, "%s", (char *)log);
+ log += strlen(log) + 1;
+ }
+ }
+ mutex_unlock(>bootlog_mutex);
+
+ return 0;
+}
+
+static int bootlog_fops_open(struct inode *inode, struct file *file)
+{
+ return single_open(file, bootlog_show, inode
[PATCH 2/3] accel/qaic: Add fifo size debugfs
Each DMA Bridge Channel (dbc) has a unique configured fifo size which is
specified by the userspace client of that dbc. Since the fifo is
circular, it is useful to know the configured size when debugging
issues.
Add a per-dbc subdirectory in debugfs and in each subdirectory add a
fifo_size entry that will display the size of that dbc's fifo when read.
Signed-off-by: Jeffrey Hugo
Reviewed-by: Carl Vanderlip
Reviewed-by: Pranjal Ramajor Asha Kanojiya
---
drivers/accel/qaic/qaic_debugfs.c | 31 +++
1 file changed, 31 insertions(+)
diff --git a/drivers/accel/qaic/qaic_debugfs.c
b/drivers/accel/qaic/qaic_debugfs.c
index 4f87fe29be1a..9d56cd451b64 100644
--- a/drivers/accel/qaic/qaic_debugfs.c
+++ b/drivers/accel/qaic/qaic_debugfs.c
@@ -11,6 +11,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -20,6 +21,7 @@
#define BOOTLOG_POOL_SIZE 16
#define BOOTLOG_MSG_SIZE 512
+#define QAIC_DBC_DIR_NAME 9
struct bootlog_msg {
/* Buffer for bootlog messages */
@@ -74,14 +76,43 @@ static const struct file_operations bootlog_fops = {
.release = single_release,
};
+static int read_dbc_fifo_size(struct seq_file *s, void *unused)
+{
+ struct dma_bridge_chan *dbc = s->private;
+
+ seq_printf(s, "%u\n", dbc->nelem);
+ return 0;
+}
+
+static int fifo_size_open(struct inode *inode, struct file *file)
+{
+ return single_open(file, read_dbc_fifo_size, inode->i_private);
+}
+
+static const struct file_operations fifo_size_fops = {
+ .owner = THIS_MODULE,
+ .open = fifo_size_open,
+ .read = seq_read,
+ .llseek = seq_lseek,
+ .release = single_release,
+};
+
void qaic_debugfs_init(struct qaic_drm_device *qddev)
{
struct qaic_device *qdev = qddev->qdev;
struct dentry *debugfs_root;
+ struct dentry *debugfs_dir;
+ char name[QAIC_DBC_DIR_NAME];
+ u32 i;
debugfs_root = to_drm(qddev)->debugfs_root;
debugfs_create_file("bootlog", 0400, debugfs_root, qdev, _fops);
+ for (i = 0; i < qdev->num_dbc; ++i) {
+ snprintf(name, QAIC_DBC_DIR_NAME, "dbc%03u", i);
+ debugfs_dir = debugfs_create_dir(name, debugfs_root);
+ debugfs_create_file("fifo_size", 0400, debugfs_dir,
>dbc[i], _size_fops);
+ }
}
static struct bootlog_page *alloc_bootlog_page(struct qaic_device *qdev)
--
2.34.1
[PATCH 0/3] accel/qaic: Add debugfs entries
Add 3 debugfs entries that can be useful in debugging a variety of issues. bootlog - output the device bootloader log fifo_size - output the configured dbc fifo size queued - output how many requests are queued in the dbc fifo Bootlog is unique to the device, where as fifo_size/queued is per-dbc. Jeffrey Hugo (3): accel/qaic: Add bootlog debugfs accel/qaic: Add fifo size debugfs accel/qaic: Add fifo queued debugfs drivers/accel/qaic/Makefile | 2 + drivers/accel/qaic/qaic.h | 9 + drivers/accel/qaic/qaic_data.c| 9 + drivers/accel/qaic/qaic_debugfs.c | 333 ++ drivers/accel/qaic/qaic_debugfs.h | 20 ++ drivers/accel/qaic/qaic_drv.c | 16 +- 6 files changed, 388 insertions(+), 1 deletion(-) create mode 100644 drivers/accel/qaic/qaic_debugfs.c create mode 100644 drivers/accel/qaic/qaic_debugfs.h -- 2.34.1
Re: [PATCH] accel/qaic: Constify aic100_channels
On 2/22/2024 6:06 PM, Jeff Johnson wrote: MHI allows the channel configs to be const, so constify aic100_channels to prevent runtime modification. Signed-off-by: Jeff Johnson Applied to drm-misc-next -Jeff
i915 build error on drm-misc-next
With the x86_64_defconfig I see the following when building drm-misc-next: CC drivers/gpu/drm/i915/display/intel_crt.o CC drivers/gpu/drm/i915/display/intel_cx0_phy.o CC drivers/gpu/drm/i915/display/intel_ddi.o CC drivers/gpu/drm/i915/display/intel_ddi_buf_trans.o CC drivers/gpu/drm/i915/display/intel_display_device.o CC drivers/gpu/drm/i915/display/intel_display_trace.o CC drivers/gpu/drm/i915/display/intel_dkl_phy.o CC drivers/gpu/drm/i915/display/intel_dp.o CC drivers/gpu/drm/i915/display/intel_dp_aux.o CC drivers/gpu/drm/i915/display/intel_dp_aux_backlight.o CC drivers/gpu/drm/i915/display/intel_dp_hdcp.o CC drivers/gpu/drm/i915/display/intel_dp_link_training.o CC drivers/gpu/drm/i915/display/intel_dp_mst.o CC drivers/gpu/drm/i915/display/intel_dsi.o CC drivers/gpu/drm/i915/display/intel_dsi_dcs_backlight.o CC drivers/gpu/drm/i915/display/intel_dsi_vbt.o CC drivers/gpu/drm/i915/display/intel_dvo.o CC drivers/gpu/drm/i915/display/intel_gmbus.o CC drivers/gpu/drm/i915/display/intel_hdmi.o CC drivers/gpu/drm/i915/display/intel_lspcon.o CC drivers/gpu/drm/i915/display/intel_lvds.o CC drivers/gpu/drm/i915/display/intel_panel.o CC drivers/gpu/drm/i915/display/intel_pps.o drivers/gpu/drm/i915/display/intel_dp.c: In function ‘intel_write_dp_vsc_sdp’: drivers/gpu/drm/i915/display/intel_dp.c:4232:15: error: implicit declaration of function ‘intel_dp_vsc_sdp_pack’; did you mean ‘drm_dp_vsc_sdp_pack’? [-Werror=implicit-function-declaration] 4232 | len = intel_dp_vsc_sdp_pack(vsc, , sizeof(sdp)); | ^ | drm_dp_vsc_sdp_pack Is this a known issue? -Jeff
Re: [PATCH] accel/qaic: Constify aic100_channels
On 2/22/2024 6:06 PM, Jeff Johnson wrote: MHI allows the channel configs to be const, so constify aic100_channels to prevent runtime modification. Signed-off-by: Jeff Johnson Reviewed-by: Jeffrey Hugo I plan to apply to drm-misc-next before the rc6 freeze.
Re: [PATCH v2] accel/ivpu: Don't enable any tiles by default on VPU40xx
On 2/20/2024 6:16 AM, Jacek Lawrynowicz wrote:
From: Andrzej Kacprowski
There is no point in requesting 1 tile on VPU40xx as the FW will
probably need more tiles to run workloads, so it will have to
reconfigure PLL anyway. Don't enable any tiles and allow the FW to
perform initial tile configuration.
This improves NPU boot stability as the tiles are always enabled only
by the FW from the same initial state.
Fixes: 79cdc56c4a54 ("accel/ivpu: Add initial support for VPU 4")
Cc: sta...@vger.kernel.org
Signed-off-by: Andrzej Kacprowski
Signed-off-by: Jacek Lawrynowicz
---
Reviewed-by: Jeffrey Hugo
Re: [PATCH 8/8] accel/ivpu: Rename VPU to NPU in message strings
On 2/14/2024 1:13 AM, Jacek Lawrynowicz wrote: VPU was renamed to NPU but due to large overhead of renaming all the sources only user visible messages are being updated. Signed-off-by: Jacek Lawrynowicz Reviewed-by: Jeffrey Hugo
Re: [PATCH 7/8] accel/ivpu: Refactor BO creation functions
On 2/14/2024 1:13 AM, Jacek Lawrynowicz wrote: From: "Wachowski, Karol" Rename BO allocate/create functions, so the code is more consistent. There are now two matching buffer creation functions: - ivpu_bo_create_ioctl() - create a BO from user space - ivpu_bo_create() - create a BO from kernel space ivpu_bo_alloc() is now only used to allocate struct ivpu_bo which better matches its name. Signed-off-by: Wachowski, Karol Missing your SOB. Otherwise looks good to me. -Jeff
Re: [PATCH 6/8] accel/ivpu: Fix ivpu_reset_engine_fn merge issue
On 2/14/2024 1:13 AM, Jacek Lawrynowicz wrote: ivpu_reset_engine_fn and ivpu_reset_engine_fops were separated during merge so move them back together to keep the file consistent. Signed-off-by: Jacek Lawrynowicz Reviewed-by: Jeffrey Hugo
Re: [PATCH 5/8] accel/ivpu: Use lazy allocation for doorbell IDs
On 2/14/2024 1:13 AM, Jacek Lawrynowicz wrote: From: "Wachowski, Karol" Reserve/allocate and free doorbells for command queues when needed using xarray. This allows to avoid reserving a doorbell for a contexts that never issues a job. Signed-off-by: Wachowski, Karol Missing your SOB. Otherwise looks good to me. -Jeff
Re: [PATCH 4/8] accel/ivpu: Add support for FW boot param system_time_us
On 2/14/2024 1:13 AM, Jacek Lawrynowicz wrote: From: Krystian Pradzynski Add support for FW boot API param system_time_us. According to the API description this field should be set to system time in microseconds starting from 1970. Signed-off-by: Krystian Pradzynski Missing your SOB. Otherwise looks good to me. -Jeff
Re: [PATCH 3/8] accel/ivpu: Update FW API headers
On 2/14/2024 1:13 AM, Jacek Lawrynowicz wrote: Update Boot API to 3.22.0 and JSM API to 3.15.6 Signed-off-by: Jacek Lawrynowicz Reviewed-by: Jeffrey Hugo
Re: [PATCH 2/8] accel/ivpu: Remove legacy firmware name
On 2/14/2024 1:12 AM, Jacek Lawrynowicz wrote: We are now using NPU IP generation based FW names instead of platform code names, so mtl_vpu.bin can be removed. Signed-off-by: Jacek Lawrynowicz Reviewed-by: Jeffrey Hugo
Re: [PATCH 1/8] accel/ivpu: Rename TILE_SKU_BOTH_MTL to TILE_SKU_BOTH
On 2/14/2024 1:12 AM, Jacek Lawrynowicz wrote: Remove legacy postfix from TILE_SKU_BOTH macro. This was missed when renaming MTL to VPU37XX. Signed-off-by: Jacek Lawrynowicz Reviewed-by: Jeffrey Hugo
Re: [PATCH v2] accel/ivpu: Fix DevTLB errors on suspend/resume and recovery
On 2/7/2024 3:24 AM, Jacek Lawrynowicz wrote:
Issue IP reset before shutdown in order to
complete all upstream requests to the SOC.
Without this DevTLB is complaining about
incomplete transactions and NPU cannot resume from
suspend.
This problem is only happening on recent IFWI
releases.
IP reset in rare corner cases can mess up PCI
configuration, so save it before the reset.
After this happens it is also impossible to
issue PLL requests and D0->D3->D0 cycle is needed
to recover the NPU. Add WP 0 request on power up,
so the PUNIT is always notified about NPU reset.
Use D0/D3 cycle for recovery as it can recover
from failed IP reset and FLR cannot.
Fixes: 3f7c0634926d ("accel/ivpu/37xx: Fix hangs related to MMIO reset")
Signed-off-by: Jacek Lawrynowicz
---
Reviewed-by: Jeffrey Hugo
Nit below
drivers/accel/ivpu/ivpu_hw_37xx.c | 44 ++-
drivers/accel/ivpu/ivpu_pm.c | 39 +++
2 files changed, 54 insertions(+), 29 deletions(-)
diff --git a/drivers/accel/ivpu/ivpu_hw_37xx.c
b/drivers/accel/ivpu/ivpu_hw_37xx.c
index 77accd029c4a..89af1006df55 100644
--- a/drivers/accel/ivpu/ivpu_hw_37xx.c
+++ b/drivers/accel/ivpu/ivpu_hw_37xx.c
@@ -510,16 +510,6 @@ static int ivpu_boot_pwr_domain_enable(struct ivpu_device
*vdev)
return ret;
}
-static int ivpu_boot_pwr_domain_disable(struct ivpu_device *vdev)
-{
- ivpu_boot_dpu_active_drive(vdev, false);
- ivpu_boot_pwr_island_isolation_drive(vdev, true);
- ivpu_boot_pwr_island_trickle_drive(vdev, false);
- ivpu_boot_pwr_island_drive(vdev, false);
-
- return ivpu_boot_wait_for_pwr_island_status(vdev, 0x0);
-}
-
static void ivpu_boot_no_snoop_enable(struct ivpu_device *vdev)
{
u32 val = REGV_RD32(VPU_37XX_HOST_IF_TCU_PTW_OVERRIDES);
@@ -616,12 +606,37 @@ static int ivpu_hw_37xx_info_init(struct ivpu_device
*vdev)
return 0;
}
+static int ivpu_hw_37xx_ip_reset(struct ivpu_device *vdev)
+{
+ int ret;
+ u32 val;
+
+ if (IVPU_WA(punit_disabled))
+ return 0;
+
+ ret = REGB_POLL_FLD(VPU_37XX_BUTTRESS_VPU_IP_RESET, TRIGGER, 0,
TIMEOUT_US);
+ if (ret) {
+ ivpu_err(vdev, "Timed out waiting for TRIGGER bit\n");
+ return ret;
+ }
+
+ val = REGB_RD32(VPU_37XX_BUTTRESS_VPU_IP_RESET);
+ val = REG_SET_FLD(VPU_37XX_BUTTRESS_VPU_IP_RESET, TRIGGER, val);
+ REGB_WR32(VPU_37XX_BUTTRESS_VPU_IP_RESET, val);
+
+ ret = REGB_POLL_FLD(VPU_37XX_BUTTRESS_VPU_IP_RESET, TRIGGER, 0,
TIMEOUT_US);
+ if (ret)
+ ivpu_err(vdev, "Timed out waiting for RESET completion\n");
+
+ return ret;
+}
+
static int ivpu_hw_37xx_reset(struct ivpu_device *vdev)
{
int ret = 0;
- if (ivpu_boot_pwr_domain_disable(vdev)) {
- ivpu_err(vdev, "Failed to disable power domain\n");
+ if (ivpu_hw_37xx_ip_reset(vdev)) {
+ ivpu_err(vdev, "Failed to reset NPU\n");
ret = -EIO;
}
@@ -661,6 +676,11 @@ static int ivpu_hw_37xx_power_up(struct ivpu_device *vdev)
{
int ret;
+ /* PLL requests may fail when powering down, so issue WP 0 here */
+ ret = ivpu_pll_disable(vdev);
+ if (ret)
+ ivpu_warn(vdev, "Failed to disable PLL: %d\n", ret);
+
ret = ivpu_hw_37xx_d0i3_disable(vdev);
if (ret)
ivpu_warn(vdev, "Failed to disable D0I3: %d\n", ret);
diff --git a/drivers/accel/ivpu/ivpu_pm.c b/drivers/accel/ivpu/ivpu_pm.c
index f501f27ebafd..5f73854234ba 100644
--- a/drivers/accel/ivpu/ivpu_pm.c
+++ b/drivers/accel/ivpu/ivpu_pm.c
@@ -58,11 +58,14 @@ static int ivpu_suspend(struct ivpu_device *vdev)
{
int ret;
+ /* Save PCI state before powering down as it sometimes gets corrupted if NPU hangs */
+ pci_save_state(to_pci_dev(vdev->drm.dev));
+
ret = ivpu_shutdown(vdev);
- if (ret) {
+ if (ret)
ivpu_err(vdev, "Failed to shutdown VPU: %d\n", ret);
In the two logs you add in this change, the log has "NPU". Here, there
is "VPU". As far as I understand VPU is the old term and NPU is the new
term therefore it seems like all the logs should be updated to use the
new term for consistency. Outside of scope for this change though.
- return ret;
- }
+
+ pci_set_power_state(to_pci_dev(vdev->drm.dev), PCI_D3hot);
return ret;
}
@@ -71,6 +74,9 @@ static int ivpu_resume(struct ivpu_device *vdev)
{
int ret;
+ pci_set_power_state(to_pci_dev(vdev->drm.dev), PCI_D0);
+ pci_restore_state(to_pci_dev(vdev->drm.dev));
+
retry:
ret = ivpu_hw_power_up(vdev);
if (ret) {
@@ -120,15 +126,20 @@ static void ivpu_pm_recovery_work(struct work_struct
*work)
ivpu_fw_log_dump(vdev);
-retry:
-
[PATCH] dt-bindings: drm/bridge: ti-sn65dsi86: Fix bouncing @codeaurora address
The servers for the @codeaurora domain are long retired and any messages sent there bounce. Sandeep Panda's email address is no longer valid and should be repleaced. However Sandeep has left the company and has not been active sice, therefore it looks like this binding is orphaned. Doug is listed as the reviewer for this file in MAINTAINERS and has volunteered to be listed within the file as the binding maintainer. Therefore replace Sandeep with Doug to make the documentation current. Signed-off-by: Jeffrey Hugo --- .../devicetree/bindings/display/bridge/ti,sn65dsi86.yaml| 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.yaml b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.yaml index 6ec6d287bff4..c93878b6d718 100644 --- a/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.yaml +++ b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.yaml @@ -7,7 +7,7 @@ $schema: http://devicetree.org/meta-schemas/core.yaml# title: SN65DSI86 DSI to eDP bridge chip maintainers: - - Sandeep Panda + - Douglas Anderson description: | The Texas Instruments SN65DSI86 bridge takes MIPI DSI in and outputs eDP. -- 2.34.1
Re: TI SN65DSI86 bridge chip DT binding maintainer
On 2/2/2024 12:03 PM, Doug Anderson wrote: Hi, On Fri, Feb 2, 2024 at 10:29 AM Jeffrey Hugo wrote: Hi Doug, The DT binding for the TI SN65DSI86 [1] lists Sandeep Panda as the maintainer within the file. This is no longer valid because the @codeaurora domain bounces, and Sandeep appears to have left the company. As such the binding appears to be orphaned, although the file itself is misleading because it needs to be updated. I'd like to find a new maintainer and from what I've seen on list, you seem to be interested and active in this particular bridge chip. I also see that you are listed as a reviewer of the C code and binding within MAINTAINERS. Would you be willing to volunteer to maintain the binding going forward? [1]: Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.yaml Yeah, this is fine. Officially "maintainership" is through the drm-misc tree, but I can have my name listed in the file. I'm already listed in the MAINTAINERS file as the reviewer for this file. If you want to send a patch I can apply it, or I can post a patch and then apply once someone from Qualcomm gives it a Reviewed-by. Awesome. I'll send a patch out. Thanks! -Jeff
TI SN65DSI86 bridge chip DT binding maintainer
Hi Doug, The DT binding for the TI SN65DSI86 [1] lists Sandeep Panda as the maintainer within the file. This is no longer valid because the @codeaurora domain bounces, and Sandeep appears to have left the company. As such the binding appears to be orphaned, although the file itself is misleading because it needs to be updated. I'd like to find a new maintainer and from what I've seen on list, you seem to be interested and active in this particular bridge chip. I also see that you are listed as a reviewer of the C code and binding within MAINTAINERS. Would you be willing to volunteer to maintain the binding going forward? [1]: Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.yaml -Jeff
[PATCH] dt-bindings: backlight: qcom-wled: Fix bouncing email addresses
Bjorn is no longer at Linaro. Update his email address to @kernel to match the .mailmap entry. The servers for @codeaurora are long retired and messages sent there will bounce. Update Kiran's email address to match the .mailmap entry. This will help anyone that is looking to reach out about this binding and is not using .mailmap to pre-process their message. Signed-off-by: Jeffrey Hugo --- .../devicetree/bindings/leds/backlight/qcom-wled.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Documentation/devicetree/bindings/leds/backlight/qcom-wled.yaml b/Documentation/devicetree/bindings/leds/backlight/qcom-wled.yaml index 5f1849bdabba..a8490781011d 100644 --- a/Documentation/devicetree/bindings/leds/backlight/qcom-wled.yaml +++ b/Documentation/devicetree/bindings/leds/backlight/qcom-wled.yaml @@ -7,8 +7,8 @@ $schema: http://devicetree.org/meta-schemas/core.yaml# title: Qualcomm Technologies, Inc. WLED driver maintainers: - - Bjorn Andersson - - Kiran Gunda + - Bjorn Andersson + - Kiran Gunda description: | WLED (White Light Emitting Diode) driver is used for controlling display -- 2.34.1
Re: [PATCH 7/7] accel/ivpu: Add job status for jobs aborted by the driver
On 1/26/2024 5:28 AM, Jacek Lawrynowicz wrote: From: Grzegorz Trzebiatowski Add DRM_IVPU_JOB_STATUS_ABORTED to indicate that the job was aborted by the driver due to e.g. TDR or user context MMU faults. This will help UMD and tests distinguish if job was aborted by the FW or the driver. Signed-off-by: Grzegorz Trzebiatowski Signed-off-by: Jacek Lawrynowicz Reviewed-by: Jeffrey Hugo
Re: [PATCH 6/7] accel/ivpu/40xx: Stop passing SKU boot parameters to FW
On 1/26/2024 5:28 AM, Jacek Lawrynowicz wrote: From: Krystian Pradzynski This parameter was never used by the 40xx FW. Signed-off-by: Krystian Pradzynski Signed-off-by: Jacek Lawrynowicz Reviewed-by: Jeffrey Hugo
Re: [PATCH 5/7] accel/ivpu/40xx: Enable D0i3 message
On 1/26/2024 5:28 AM, Jacek Lawrynowicz wrote: From: Krystian Pradzynski All recent 40xx firmware already supports D0i3 entry message and this WA is no longer needed. Can I assume that the workaround only applies to pre-production firmware?
Re: [PATCH 4/7] accel/ivpu: Gracefully shutdown NPU before reset
On 1/26/2024 5:28 AM, Jacek Lawrynowicz wrote:
From: "Wachowski, Karol"
Replace forceful disable of power domains with requests to disable
TOP NOC CPU_CTRL and HOSTIF_L2CACHE through QREQN.
In case of failure retry multiple times following HAS sequence of
checking both QACCEPN and QDENYN registers.
This fixes VPU hangs with PCODE released in January 2024 onwards.
Fixes: 3f7c0634926d ("accel/ivpu/37xx: Fix hangs related to MMIO reset")
Signed-off-by: Wachowski, Karol
Signed-off-by: Jacek Lawrynowicz
---
drivers/accel/ivpu/ivpu_hw_37xx.c | 122 +++---
1 file changed, 60 insertions(+), 62 deletions(-)
diff --git a/drivers/accel/ivpu/ivpu_hw_37xx.c
b/drivers/accel/ivpu/ivpu_hw_37xx.c
index 77accd029c4a..b1a3a19c8986 100644
--- a/drivers/accel/ivpu/ivpu_hw_37xx.c
+++ b/drivers/accel/ivpu/ivpu_hw_37xx.c
@@ -332,28 +332,6 @@ static int ivpu_boot_top_noc_qrenqn_check(struct
ivpu_device *vdev, u32 exp_val)
return 0;
}
-static int ivpu_boot_top_noc_qacceptn_check(struct ivpu_device *vdev, u32 exp_val)
-{
- u32 val = REGV_RD32(VPU_37XX_TOP_NOC_QACCEPTN);
-
- if (!REG_TEST_FLD_NUM(VPU_37XX_TOP_NOC_QACCEPTN, CPU_CTRL, exp_val,
val) ||
- !REG_TEST_FLD_NUM(VPU_37XX_TOP_NOC_QACCEPTN, HOSTIF_L2CACHE,
exp_val, val))
- return -EIO;
-
- return 0;
-}
-
-static int ivpu_boot_top_noc_qdeny_check(struct ivpu_device *vdev, u32 exp_val)
-{
- u32 val = REGV_RD32(VPU_37XX_TOP_NOC_QDENY);
-
- if (!REG_TEST_FLD_NUM(VPU_37XX_TOP_NOC_QDENY, CPU_CTRL, exp_val, val) ||
- !REG_TEST_FLD_NUM(VPU_37XX_TOP_NOC_QDENY, HOSTIF_L2CACHE, exp_val,
val))
- return -EIO;
-
- return 0;
-}
-
static int ivpu_boot_host_ss_configure(struct ivpu_device *vdev)
{
ivpu_boot_host_ss_rst_clr_assert(vdev);
@@ -396,37 +374,68 @@ static int ivpu_boot_host_ss_axi_enable(struct
ivpu_device *vdev)
return ivpu_boot_host_ss_axi_drive(vdev, true);
}
-static int ivpu_boot_host_ss_top_noc_drive(struct ivpu_device *vdev, bool enable)
+static int ivpu_boot_host_ss_top_noc_qacceptn_check(struct ivpu_device *vdev,
bool enable, u32 mask)
+{
+ u32 val = REGV_RD32(VPU_37XX_TOP_NOC_QACCEPTN) & mask;
+
+ if (enable && val == mask)
+ return 0;
+
+ if (!enable && val == 0)
+ return 0;
+
+ ivpu_dbg(vdev, PM, "Failed qacceptn check 0x%x (mask 0x%x enable
%d)\n", val, mask, enable);
+ return -EIO;
+}
+
+static int ivpu_boot_host_ss_top_noc_qdeny_check(struct ivpu_device *vdev, u32
mask)
+{
+ u32 val = REGV_RD32(VPU_37XX_TOP_NOC_QDENY) & mask;
+
+ if (val) {
+ ivpu_dbg(vdev, PM, "Failed qdeny check 0x%x (mask 0x%x)\n",
val, mask);
+ return -EIO;
+ }
+
+ return 0;
+}
+
+static int ivpu_boot_host_ss_top_noc_drive(struct ivpu_device *vdev, bool
enable, u32 mask)
{
- int ret;
u32 val;
val = REGV_RD32(VPU_37XX_TOP_NOC_QREQN);
- if (enable) {
- val = REG_SET_FLD(VPU_37XX_TOP_NOC_QREQN, CPU_CTRL, val);
- val = REG_SET_FLD(VPU_37XX_TOP_NOC_QREQN, HOSTIF_L2CACHE, val);
- } else {
- val = REG_CLR_FLD(VPU_37XX_TOP_NOC_QREQN, CPU_CTRL, val);
- val = REG_CLR_FLD(VPU_37XX_TOP_NOC_QREQN, HOSTIF_L2CACHE, val);
- }
- REGV_WR32(VPU_37XX_TOP_NOC_QREQN, val);
+ if (enable)
+ REGV_WR32(VPU_37XX_TOP_NOC_QREQN, val | mask);
+ else
+ REGV_WR32(VPU_37XX_TOP_NOC_QREQN, val & ~mask);
- ret = ivpu_boot_top_noc_qacceptn_check(vdev, enable ? 0x1 : 0x0);
- if (ret) {
- ivpu_err(vdev, "Failed qacceptn check: %d\n", ret);
- return ret;
- }
+ if (!ivpu_boot_host_ss_top_noc_qacceptn_check(vdev, enable, mask))
+ return 0;
- ret = ivpu_boot_top_noc_qdeny_check(vdev, 0x0);
- if (ret)
- ivpu_err(vdev, "Failed qdeny check: %d\n", ret);
+ if (!enable && ivpu_boot_host_ss_top_noc_qdeny_check(vdev, mask))
+ REGV_WR32(VPU_37XX_TOP_NOC_QREQN, val | mask);
- return ret;
+ return -EIO;
}
static int ivpu_boot_host_ss_top_noc_enable(struct ivpu_device *vdev)
{
- return ivpu_boot_host_ss_top_noc_drive(vdev, true);
+ return ivpu_boot_host_ss_top_noc_drive(vdev, true,
+
VPU_37XX_TOP_NOC_QREQN_CPU_CTRL_MASK |
+
VPU_37XX_TOP_NOC_QREQN_HOSTIF_L2CACHE_MASK);
+}
+
+static int ivpu_boot_host_ss_top_noc_cpu_ctrl_disable(struct ivpu_device *vdev)
+{
+ return ivpu_boot_host_ss_top_noc_drive(vdev, false,
+
VPU_37XX_TOP_NOC_QREQN_CPU_CTRL_MASK);
+}
+
+static int ivpu_boot_host_ss_top_noc_hostif_l2cache_disable(struct ivpu_device
*vdev)
+{
+ return ivpu_boot_host_ss_top_noc_drive(vdev, false,
+
Re: [PATCH 3/7] accel/ivpu: Disable d3hot_delay on all NPU generations
On 1/26/2024 5:28 AM, Jacek Lawrynowicz wrote: NPU does not require this delay regardless of the generation. All generations are integrated into the SOC. Signed-off-by: Jacek Lawrynowicz Reviewed-by: Jeffrey Hugo
Re: [PATCH 2/7] accel/ivpu: Correct MMU queue size checking functions
On 1/26/2024 5:27 AM, Jacek Lawrynowicz wrote: From: "Wachowski, Karol" Do not use kernel CIRC_SPACE and CIRC_CNT that incorrectly return space of a queue when wrap bit was set. Use correct implementation that compares producer, consumer and wrap bit values. Without this fix it was possible to lose events in case when event queue was full. Signed-off-by: Wachowski, Karol Signed-off-by: Jacek Lawrynowicz Reviewed-by: Jeffrey Hugo
Re: [PATCH 1/7] accel/ivpu: Force snooping for MMU writes
On 1/26/2024 5:27 AM, Jacek Lawrynowicz wrote: From: "Wachowski, Karol" Set AW_SNOOP_OVERRIDE bit in VPU_37/40XX_HOST_IF_TCU_PTW_OVERRIDES to force snooping for MMU write accesses (setting event queue events). MMU event queue buffer is the only buffer written by MMU and mapped as write-back which break cache coherency. Force write transactions to be snooped solving the problem. Signed-off-by: Wachowski, Karol Signed-off-by: Jacek Lawrynowicz Reviewed-by: Jeffrey Hugo
Re: [PATCH 3/3] accel/ivpu: Improve recovery and reset support
On 1/22/2024 5:09 AM, Jacek Lawrynowicz wrote: - Synchronize job submission with reset/recovery using reset_lock - Always print recovery reason and call diagnose_failure() - Don't allow for autosupend during recovery - Prevent immediate autosuspend after reset/recovery - Prevent force_recovery for issuing TDR when device is suspended - Reset VPU instead triggering recovery after changing debugfs params This change appears to be doing 6 different things. While they are all reset related, it seems like this should be 6 individual changes. Is there some dependency in the code that I am missing where having these all combined is the better technical solution?
Re: [PATCH 2/3] accel/ivpu: Improve stability of ivpu_submit_ioctl()
On 1/22/2024 5:09 AM, Jacek Lawrynowicz wrote:
- Wake up the device as late as possible
Can you amend with why this is a good idea?
- Remove job reference counting in order to simplify the code
- Don't put jobs that are not fully submitted on submitted_jobs_xa in
order to avoid potential races with reset/recovery
Signed-off-by: Jacek Lawrynowicz
---
drivers/accel/ivpu/ivpu_job.c | 139 +++---
drivers/accel/ivpu/ivpu_job.h | 1 -
2 files changed, 62 insertions(+), 78 deletions(-)
diff --git a/drivers/accel/ivpu/ivpu_job.c b/drivers/accel/ivpu/ivpu_job.c
index 4fed0c05e051..d9b47a04b35f 100644
--- a/drivers/accel/ivpu/ivpu_job.c
+++ b/drivers/accel/ivpu/ivpu_job.c
@@ -125,7 +125,7 @@ void ivpu_cmdq_release_all_locked(struct ivpu_file_priv
*file_priv)
/*
* Mark the doorbell as unregistered and reset job queue pointers.
* This function needs to be called when the VPU hardware is restarted
- * and FW looses job queue state. The next time job queue is used it
+ * and FW loses job queue state. The next time job queue is used it
* will be registered again.
*/
static void ivpu_cmdq_reset_locked(struct ivpu_file_priv *file_priv, u16
engine)
@@ -239,60 +239,32 @@ static struct dma_fence *ivpu_fence_create(struct
ivpu_device *vdev)
return >base;
}
-static void job_get(struct ivpu_job *job, struct ivpu_job **link)
+static void ivpu_job_destroy(struct ivpu_job *job)
{
struct ivpu_device *vdev = job->vdev;
-
- kref_get(>ref);
- *link = job;
-
- ivpu_dbg(vdev, KREF, "Job get: id %u refcount %u\n", job->job_id,
kref_read(>ref));
-}
-
-static void job_release(struct kref *ref)
-{
- struct ivpu_job *job = container_of(ref, struct ivpu_job, ref);
- struct ivpu_device *vdev = job->vdev;
u32 i;
+ ivpu_dbg(vdev, JOB, "Job destroyed: id %3u ctx %2d engine %d",
+job->job_id, job->file_priv->ctx.id, job->engine_idx);
+
for (i = 0; i < job->bo_count; i++)
if (job->bos[i])
drm_gem_object_put(>bos[i]->base.base);
dma_fence_put(job->done_fence);
ivpu_file_priv_put(>file_priv);
-
- ivpu_dbg(vdev, KREF, "Job released: id %u\n", job->job_id);
kfree(job);
-
- /* Allow the VPU to get suspended, must be called after
ivpu_file_priv_put() */
- ivpu_rpm_put(vdev);
-}
-
-static void job_put(struct ivpu_job *job)
-{
- struct ivpu_device *vdev = job->vdev;
-
- ivpu_dbg(vdev, KREF, "Job put: id %u refcount %u\n", job->job_id,
kref_read(>ref));
- kref_put(>ref, job_release);
}
static struct ivpu_job *
-ivpu_create_job(struct ivpu_file_priv *file_priv, u32 engine_idx, u32 bo_count)
+ivpu_job_create(struct ivpu_file_priv *file_priv, u32 engine_idx, u32 bo_count)
{
struct ivpu_device *vdev = file_priv->vdev;
struct ivpu_job *job;
- int ret;
-
- ret = ivpu_rpm_get(vdev);
- if (ret < 0)
- return NULL;
job = kzalloc(struct_size(job, bos, bo_count), GFP_KERNEL);
if (!job)
- goto err_rpm_put;
-
- kref_init(>ref);
+ return NULL;
job->vdev = vdev;
job->engine_idx = engine_idx;
@@ -306,17 +278,14 @@ ivpu_create_job(struct ivpu_file_priv *file_priv, u32
engine_idx, u32 bo_count)
job->file_priv = ivpu_file_priv_get(file_priv);
ivpu_dbg(vdev, JOB, "Job created: ctx %2d engine %d", file_priv->ctx.id, job->engine_idx);
-
return job;
err_free_job:
kfree(job);
-err_rpm_put:
- ivpu_rpm_put(vdev);
return NULL;
}
-static int ivpu_job_done(struct ivpu_device *vdev, u32 job_id, u32 job_status)
+static int ivpu_job_signal_and_destroy(struct ivpu_device *vdev, u32 job_id,
u32 job_status)
{
struct ivpu_job *job;
@@ -333,9 +302,10 @@ static int ivpu_job_done(struct ivpu_device *vdev, u32 job_id, u32 job_status)
ivpu_dbg(vdev, JOB, "Job complete: id %3u ctx %2d engine %d status
0x%x\n",
job->job_id, job->file_priv->ctx.id, job->engine_idx,
job_status);
+ ivpu_job_destroy(job);
ivpu_stop_job_timeout_detection(vdev);
- job_put(job);
+ ivpu_rpm_put(vdev);
Since this put() corresponds to a get() that is not in this function, I
suggest adding a comment that points to where the corresponding get() is.
Reviewed-by: Jeffrey Hugo
Re: [PATCH 1/3] accel/ivpu: Fix dev open/close races with unbind
On 1/22/2024 5:09 AM, Jacek Lawrynowicz wrote: - Add context_list_lock to synchronize user context addition/removal - Use drm_dev_enter() to prevent unbinding the device during ivpu_open() and vpu address allocation Signed-off-by: Jacek Lawrynowicz Reviewed-by: Jeffrey Hugo
Re: [PATCH v2 9/9] accel/ivpu: Deprecate DRM_IVPU_PARAM_CONTEXT_PRIORITY param
On 1/15/2024 6:44 AM, Jacek Lawrynowicz wrote: From: "Wachowski, Karol" DRM_IVPU_PARAM_CONTEXT_PRIORITY has been deprecated because it has been replaced with DRM_IVPU_JOB_PRIORITY levels set with submit IOCTL and was unused anyway. Signed-off-by: Wachowski, Karol Signed-off-by: Jacek Lawrynowicz Reviewed-by: Jeffrey Hugo
Re: [PATCH v2 8/9] accel/ivpu: Improve buffer object debug logs
On 1/15/2024 6:44 AM, Jacek Lawrynowicz wrote: Make debug logs more readable and consistent: - don't print handle as it is not always available for all buffers - use hashed ivpu_bo ptr as main buffer identifier - remove unused fields from ivpu_bo_print_info() Signed-off-by: Jacek Lawrynowicz Reviewed-by: Jeffrey Hugo
Re: [PATCH v2 7/9] accel/ivpu: Disable buffer sharing among VPU contexts
On 1/15/2024 6:44 AM, Jacek Lawrynowicz wrote: This was not supported properly. A buffer was imported to another VPU context as a separate buffer object with duplicated sgt. Both exported and imported buffers could be DMA mapped causing a double mapping on the same device. Buffers imported from another VPU context will now just increase reference count, leaving only a single sgt, fixing the problem above. Buffers still can't be shared among VPU contexts because each has its own MMU mapping and ivpu_bo only supports single MMU mappings. The solution would be to use a mapping list as in panfrost or etnaviv drivers and it will be implemented in future if required. Signed-off-by: Jacek Lawrynowicz Reviewed-by: Jeffrey Hugo
Re: QAIC reset failure
On 1/16/2024 9:58 AM, Baruch Siach wrote: Hi qaic driver maintainers, Sorry I was holiday last week and I am just now catching up on email and seeing this. I am testing an A100 device on arm64 platform. Kernel version is current Linus master as of commit 052d534373b7. The driver is unable to reset the device properly. [ 137.706765] pci :01:00.0: enabling device ( -> 0002) [ 137.712528] pci :02:00.0: enabling device ( -> 0002) [ 137.718230] qaic :03:00.0: enabling device ( -> 0002) [ 137.725720] [drm] Initialized qaic 0.0.0 20190618 for :03:00.0 on minor 0 [ 137.734326] mhi mhi0: Requested to power ON [ 137.738520] mhi mhi0: Power on setup success [ 137.855108] mhi mhi0: Wait for device to enter SBL or Mission mode This all looks good [ 137.861578] qaic_timesync mhi0_QAIC_TIMESYNC: 20: Failed to receive START channel command completion [ 137.870733] qaic_timesync mhi0_QAIC_TIMESYNC: 21: Failed to reset channel, still resetting [ 137.879063] qaic_timesync mhi0_QAIC_TIMESYNC: 20: Failed to reset channel, still resetting [ 137.887334] qaic_timesync: probe of mhi0_QAIC_TIMESYNC failed with error -5 [ 137.894866] qaic_timesync mhi0_QAIC_TIMESYNC: 20: Failed to receive START channel command completion [ 137.904006] qaic_timesync mhi0_QAIC_TIMESYNC: 21: Failed to reset channel, still resetting [ 137.912263] qaic_timesync mhi0_QAIC_TIMESYNC: 20: Failed to reset channel, still resetting [ 137.920517] qaic_timesync: probe of mhi0_QAIC_TIMESYNC failed with error -5 [ 140.807091] mhi mhi0: Device failed to enter MHI Ready [ 143.695094] mhi mhi0: Device failed to enter MHI Ready This looks like the device stopped responding to the host, early in boot. Trying to access channels while the device is not in MHI Ready state is odd. This is with firmware from SDK version 1.12.2.0. I tried also version 1.10.0.193 with similar results. Some more state information from MHI debugfs below. /sys/kernel/debug/mhi/mhi0/regdump: Host PM state: SYS ERROR Process Device state: RESET EE: DISABLE Device EE: PRIMARY BOOTLOADER state: SYS ERROR MHI_REGLEN: 0x100 MHI_VER: 0x100 MHI_CFG: 0x800 MHI_CTRL: 0x0 MHI_STATUS: 0xff04 MHI_WAKE_DB: 0x1 BHI_EXECENV: 0x0 BHI_STATUS: 0xa93f0935 BHI_ERRCODE: 0x0 BHI_ERRDBG1: 0xc030 BHI_ERRDBG2: 0xb BHI_ERRDBG3: 0xcabb0 This suggests that the device crashed, which is unexpected. /sys/kernel/debug/mhi/mhi0/states: PM state: SYS ERROR Process Device: Inactive MHI state: RESET EE: DISABLE wake: true M0: 2 M2: 0 M3: 0 device wake: 0 pending packets: 0 Any idea? We may need our firmware engineers involved. I think there is already a thread with some of the POCs involved. -Jeff
Re: [PATCH 10/10] accel/ivpu: Remove deprecated DRM_IVPU_PARAM_CONTEXT_PRIORITY
On 1/10/2024 7:33 AM, Jacek Lawrynowicz wrote:
On 05.01.2024 18:29, Jeffrey Hugo wrote:
On 1/5/2024 4:22 AM, Jacek Lawrynowicz wrote:
From: "Wachowski, Karol"
DRM_IVPU_PARAM_CONTEXT_PRIORITY has been deprecated because it
has been replaced with DRM_IVPU_JOB_PRIORITY levels set with
submit IOCTL and was unused anyway.
Signed-off-by: Wachowski, Karol
Signed-off-by: Jacek Lawrynowicz
---
drivers/accel/ivpu/ivpu_drv.c | 11 ---
drivers/accel/ivpu/ivpu_drv.h | 1 -
drivers/accel/ivpu/ivpu_job.c | 3 +++
include/uapi/drm/ivpu_accel.h | 21 -
4 files changed, 23 insertions(+), 13 deletions(-)
diff --git a/drivers/accel/ivpu/ivpu_drv.c b/drivers/accel/ivpu/ivpu_drv.c
index ec66c2c39877..546c0899bb9e 100644
--- a/drivers/accel/ivpu/ivpu_drv.c
+++ b/drivers/accel/ivpu/ivpu_drv.c
@@ -177,9 +177,6 @@ static int ivpu_get_param_ioctl(struct drm_device *dev,
void *data, struct drm_f
case DRM_IVPU_PARAM_CONTEXT_BASE_ADDRESS:
args->value = vdev->hw->ranges.user.start;
break;
- case DRM_IVPU_PARAM_CONTEXT_PRIORITY:
- args->value = file_priv->priority;
- break;
case DRM_IVPU_PARAM_CONTEXT_ID:
args->value = file_priv->ctx.id;
break;
@@ -219,17 +216,10 @@ static int ivpu_get_param_ioctl(struct drm_device *dev,
void *data, struct drm_f
static int ivpu_set_param_ioctl(struct drm_device *dev, void *data, struct
drm_file *file)
{
- struct ivpu_file_priv *file_priv = file->driver_priv;
struct drm_ivpu_param *args = data;
int ret = 0;
switch (args->param) {
- case DRM_IVPU_PARAM_CONTEXT_PRIORITY:
- if (args->value <= DRM_IVPU_CONTEXT_PRIORITY_REALTIME)
- file_priv->priority = args->value;
- else
- ret = -EINVAL;
- break;
default:
ret = -EINVAL;
}
@@ -258,7 +248,6 @@ static int ivpu_open(struct drm_device *dev, struct
drm_file *file)
}
file_priv->vdev = vdev;
- file_priv->priority = DRM_IVPU_CONTEXT_PRIORITY_NORMAL;
kref_init(_priv->ref);
mutex_init(_priv->lock);
diff --git a/drivers/accel/ivpu/ivpu_drv.h b/drivers/accel/ivpu/ivpu_drv.h
index 9b6e336626e3..7a6bc1918780 100644
--- a/drivers/accel/ivpu/ivpu_drv.h
+++ b/drivers/accel/ivpu/ivpu_drv.h
@@ -146,7 +146,6 @@ struct ivpu_file_priv {
struct mutex lock; /* Protects cmdq */
struct ivpu_cmdq *cmdq[IVPU_NUM_ENGINES];
struct ivpu_mmu_context ctx;
- u32 priority;
bool has_mmu_faults;
};
diff --git a/drivers/accel/ivpu/ivpu_job.c b/drivers/accel/ivpu/ivpu_job.c
index 7206cf9cdb4a..82e40bb4803c 100644
--- a/drivers/accel/ivpu/ivpu_job.c
+++ b/drivers/accel/ivpu/ivpu_job.c
@@ -488,6 +488,9 @@ int ivpu_submit_ioctl(struct drm_device *dev, void *data,
struct drm_file *file)
if (params->engine > DRM_IVPU_ENGINE_COPY)
return -EINVAL;
+ if (params->priority > DRM_IVPU_JOB_PRIORITY_REALTIME)
+ return -EINVAL;
+
if (params->buffer_count == 0 || params->buffer_count >
JOB_MAX_BUFFER_COUNT)
return -EINVAL;
diff --git a/include/uapi/drm/ivpu_accel.h b/include/uapi/drm/ivpu_accel.h
index de1944e42c65..cc9a0504ee2f 100644
--- a/include/uapi/drm/ivpu_accel.h
+++ b/include/uapi/drm/ivpu_accel.h
@@ -13,7 +13,7 @@ extern "C" {
#endif
#define DRM_IVPU_DRIVER_MAJOR 1
-#define DRM_IVPU_DRIVER_MINOR 0
+#define DRM_IVPU_DRIVER_MINOR 1
I remember when this driver was going through initial review before acceptance,
Oded mentioned that the DRM driver version mechanism was deprecated and to not
use it. Based on that, it seems like you should not be incrementing the minor
number.
I wanted to use minor version in tests to verify the UAPI but this is not very
important. I can leave this as is.
#define DRM_IVPU_GET_PARAM 0x00
#define DRM_IVPU_SET_PARAM 0x01
@@ -64,11 +64,18 @@ extern "C" {
#define DRM_IVPU_PLATFORM_TYPE_SILICON 0
+/* Deprecated - to be removed */
#define DRM_IVPU_CONTEXT_PRIORITY_IDLE 0
#define DRM_IVPU_CONTEXT_PRIORITY_NORMAL 1
#define DRM_IVPU_CONTEXT_PRIORITY_FOCUS 2
#define DRM_IVPU_CONTEXT_PRIORITY_REALTIME 3
$SUBJECT suggests these are being removed, not just deprecated. Also,
shouldn't DRM_IVPU_PARAM_CONTEXT_PRIORITY which is a few lines above this be
deprecated/removed/something?
OK, I'll correct the subject and add "deprecated" comment to
DRM_IVPU_PARAM_CONTEXT_PRIORITY.
+#define DRM_IVPU_JOB_PRIORITY_DEFAULT 0
+#define DRM_IVPU_JOB_PRIORITY_IDLE 1
+#define DRM_IVPU_JOB_PRIORITY_NORMAL 2
+#define DRM_IVPU_JOB_PRIORITY_FOCUS 3
+#define DRM_IVPU_JOB_PRIORITY_REALTIME 4
+
/**
* DRM_IVPU_CAP_METRIC_STREAMER
*
@@ -286,6 +293,18 @@ struct drm_ivpu_submit {
* to be executed. The offset has to be
Re: [PATCH 10/10] accel/ivpu: Remove deprecated DRM_IVPU_PARAM_CONTEXT_PRIORITY
On 1/5/2024 4:22 AM, Jacek Lawrynowicz wrote:
From: "Wachowski, Karol"
DRM_IVPU_PARAM_CONTEXT_PRIORITY has been deprecated because it
has been replaced with DRM_IVPU_JOB_PRIORITY levels set with
submit IOCTL and was unused anyway.
Signed-off-by: Wachowski, Karol
Signed-off-by: Jacek Lawrynowicz
---
drivers/accel/ivpu/ivpu_drv.c | 11 ---
drivers/accel/ivpu/ivpu_drv.h | 1 -
drivers/accel/ivpu/ivpu_job.c | 3 +++
include/uapi/drm/ivpu_accel.h | 21 -
4 files changed, 23 insertions(+), 13 deletions(-)
diff --git a/drivers/accel/ivpu/ivpu_drv.c b/drivers/accel/ivpu/ivpu_drv.c
index ec66c2c39877..546c0899bb9e 100644
--- a/drivers/accel/ivpu/ivpu_drv.c
+++ b/drivers/accel/ivpu/ivpu_drv.c
@@ -177,9 +177,6 @@ static int ivpu_get_param_ioctl(struct drm_device *dev,
void *data, struct drm_f
case DRM_IVPU_PARAM_CONTEXT_BASE_ADDRESS:
args->value = vdev->hw->ranges.user.start;
break;
- case DRM_IVPU_PARAM_CONTEXT_PRIORITY:
- args->value = file_priv->priority;
- break;
case DRM_IVPU_PARAM_CONTEXT_ID:
args->value = file_priv->ctx.id;
break;
@@ -219,17 +216,10 @@ static int ivpu_get_param_ioctl(struct drm_device *dev,
void *data, struct drm_f
static int ivpu_set_param_ioctl(struct drm_device *dev, void *data, struct drm_file *file)
{
- struct ivpu_file_priv *file_priv = file->driver_priv;
struct drm_ivpu_param *args = data;
int ret = 0;
switch (args->param) {
- case DRM_IVPU_PARAM_CONTEXT_PRIORITY:
- if (args->value <= DRM_IVPU_CONTEXT_PRIORITY_REALTIME)
- file_priv->priority = args->value;
- else
- ret = -EINVAL;
- break;
default:
ret = -EINVAL;
}
@@ -258,7 +248,6 @@ static int ivpu_open(struct drm_device *dev, struct
drm_file *file)
}
file_priv->vdev = vdev;
- file_priv->priority = DRM_IVPU_CONTEXT_PRIORITY_NORMAL;
kref_init(_priv->ref);
mutex_init(_priv->lock);
diff --git a/drivers/accel/ivpu/ivpu_drv.h b/drivers/accel/ivpu/ivpu_drv.h
index 9b6e336626e3..7a6bc1918780 100644
--- a/drivers/accel/ivpu/ivpu_drv.h
+++ b/drivers/accel/ivpu/ivpu_drv.h
@@ -146,7 +146,6 @@ struct ivpu_file_priv {
struct mutex lock; /* Protects cmdq */
struct ivpu_cmdq *cmdq[IVPU_NUM_ENGINES];
struct ivpu_mmu_context ctx;
- u32 priority;
bool has_mmu_faults;
};
diff --git a/drivers/accel/ivpu/ivpu_job.c b/drivers/accel/ivpu/ivpu_job.c
index 7206cf9cdb4a..82e40bb4803c 100644
--- a/drivers/accel/ivpu/ivpu_job.c
+++ b/drivers/accel/ivpu/ivpu_job.c
@@ -488,6 +488,9 @@ int ivpu_submit_ioctl(struct drm_device *dev, void *data,
struct drm_file *file)
if (params->engine > DRM_IVPU_ENGINE_COPY)
return -EINVAL;
+ if (params->priority > DRM_IVPU_JOB_PRIORITY_REALTIME)
+ return -EINVAL;
+
if (params->buffer_count == 0 || params->buffer_count >
JOB_MAX_BUFFER_COUNT)
return -EINVAL;
diff --git a/include/uapi/drm/ivpu_accel.h b/include/uapi/drm/ivpu_accel.h
index de1944e42c65..cc9a0504ee2f 100644
--- a/include/uapi/drm/ivpu_accel.h
+++ b/include/uapi/drm/ivpu_accel.h
@@ -13,7 +13,7 @@ extern "C" {
#endif
#define DRM_IVPU_DRIVER_MAJOR 1
-#define DRM_IVPU_DRIVER_MINOR 0
+#define DRM_IVPU_DRIVER_MINOR 1
I remember when this driver was going through initial review before
acceptance, Oded mentioned that the DRM driver version mechanism was
deprecated and to not use it. Based on that, it seems like you should
not be incrementing the minor number.
#define DRM_IVPU_GET_PARAM 0x00
#define DRM_IVPU_SET_PARAM 0x01
@@ -64,11 +64,18 @@ extern "C" {
#define DRM_IVPU_PLATFORM_TYPE_SILICON 0
+/* Deprecated - to be removed */
#define DRM_IVPU_CONTEXT_PRIORITY_IDLE0
#define DRM_IVPU_CONTEXT_PRIORITY_NORMAL1
#define DRM_IVPU_CONTEXT_PRIORITY_FOCUS 2
#define DRM_IVPU_CONTEXT_PRIORITY_REALTIME 3
$SUBJECT suggests these are being removed, not just deprecated. Also,
shouldn't DRM_IVPU_PARAM_CONTEXT_PRIORITY which is a few lines above
this be deprecated/removed/something?
+#define DRM_IVPU_JOB_PRIORITY_DEFAULT 0
+#define DRM_IVPU_JOB_PRIORITY_IDLE 1
+#define DRM_IVPU_JOB_PRIORITY_NORMAL 2
+#define DRM_IVPU_JOB_PRIORITY_FOCUS3
+#define DRM_IVPU_JOB_PRIORITY_REALTIME 4
+
/**
* DRM_IVPU_CAP_METRIC_STREAMER
*
@@ -286,6 +293,18 @@ struct drm_ivpu_submit {
* to be executed. The offset has to be 8-byte aligned.
*/
__u32 commands_offset;
+
+ /**
+* @priority:
+*
+* Priority to be set for related job command queue, can be one of the
following:
+* %DRM_IVPU_JOB_PRIORITY_DEFAULT
+* %DRM_IVPU_JOB_PRIORITY_IDLE
+*
Re: [PATCH 09/10] accel/ivpu: Improve buffer object debug logs
On 1/5/2024 4:22 AM, Jacek Lawrynowicz wrote:
Make debug logs more readable and consistent:
- don't print handle as it is not always available for all buffers
- use hashed ivpu_bo ptr as main buffer identifier
- remove unused fields from ivpu_bo_print_info()
Signed-off-by: Jacek Lawrynowicz
---
drivers/accel/ivpu/ivpu_gem.c | 72 +++
drivers/accel/ivpu/ivpu_gem.h | 1 -
2 files changed, 23 insertions(+), 50 deletions(-)
diff --git a/drivers/accel/ivpu/ivpu_gem.c b/drivers/accel/ivpu/ivpu_gem.c
index 8cb4d337552e..dd327d7eda0d 100644
--- a/drivers/accel/ivpu/ivpu_gem.c
+++ b/drivers/accel/ivpu/ivpu_gem.c
@@ -24,14 +24,11 @@ static const struct drm_gem_object_funcs ivpu_gem_funcs;
static inline void ivpu_dbg_bo(struct ivpu_device *vdev, struct ivpu_bo *bo, const char *action)
{
- if (bo->ctx)
- ivpu_dbg(vdev, BO, "%6s: size %zu has_pages %d dma_mapped %d handle
%u ctx %d vpu_addr 0x%llx mmu_mapped %d\n",
-action, ivpu_bo_size(bo), (bool)bo->base.pages,
(bool)bo->base.sgt,
-bo->handle, bo->ctx->id, bo->vpu_addr, bo->mmu_mapped);
- else
- ivpu_dbg(vdev, BO, "%6s: size %zu has_pages %d dma_mapped %d handle
%u (not added to context)\n",
-action, ivpu_bo_size(bo), (bool)bo->base.pages,
(bool)bo->base.sgt,
-bo->handle);
+ ivpu_dbg(vdev, BO,
+"%6s: bo %8p vpu_addr %9llx size %8zu ctx %d has_pages %d
dma_mapped %d mmu_mapped %d wc %d imported %d\n",
+action, bo, bo->vpu_addr, ivpu_bo_size(bo), bo->ctx ?
bo->ctx->id : 0,
+(bool)bo->base.pages, (bool)bo->base.sgt, bo->mmu_mapped,
bo->base.map_wc,
+(bool)bo->base.base.import_attach);
}
/*
@@ -49,12 +46,7 @@ int __must_check ivpu_bo_pin(struct ivpu_bo *bo)
mutex_lock(>lock);
ivpu_dbg_bo(vdev, bo, "pin");
-
- if (!bo->ctx) {
- ivpu_err(vdev, "vpu_addr not allocated for BO %d\n",
bo->handle);
- ret = -EINVAL;
- goto unlock;
- }
+ drm_WARN_ON(>drm, !bo->ctx);
if (!bo->mmu_mapped) {
struct sg_table *sgt = drm_gem_shmem_get_pages_sgt(>base);
@@ -108,9 +100,7 @@ static void ivpu_bo_unbind_locked(struct ivpu_bo *bo)
{
struct ivpu_device *vdev = ivpu_bo_to_vdev(bo);
- lockdep_assert_held(>lock);
-
- ivpu_dbg_bo(vdev, bo, "unbind");
+ lockdep_assert(lockdep_is_held(>lock) ||
!kref_read(>base.base.refcount));
if (bo->mmu_mapped) {
drm_WARN_ON(>drm, !bo->ctx);
@@ -122,7 +112,6 @@ static void ivpu_bo_unbind_locked(struct ivpu_bo *bo)
if (bo->ctx) {
ivpu_mmu_context_remove_node(bo->ctx, >mm_node);
- bo->vpu_addr = 0;
bo->ctx = NULL;
}
@@ -139,13 +128,6 @@ static void ivpu_bo_unbind_locked(struct ivpu_bo *bo)
dma_resv_unlock(bo->base.base.resv);
}
-static void ivpu_bo_unbind(struct ivpu_bo *bo)
-{
- mutex_lock(>lock);
- ivpu_bo_unbind_locked(bo);
- mutex_unlock(>lock);
-}
This does not seem to be related to $SUBJECT
-
void ivpu_bo_remove_all_bos_from_context(struct ivpu_device *vdev, struct
ivpu_mmu_context *ctx)
{
struct ivpu_bo *bo;
@@ -156,8 +138,10 @@ void ivpu_bo_remove_all_bos_from_context(struct
ivpu_device *vdev, struct ivpu_m
mutex_lock(>bo_list_lock);
list_for_each_entry(bo, >bo_list, bo_list_node) {
mutex_lock(>lock);
- if (bo->ctx == ctx)
+ if (bo->ctx == ctx) {
+ ivpu_dbg_bo(vdev, bo, "unbind");
ivpu_bo_unbind_locked(bo);
+ }
mutex_unlock(>lock);
}
mutex_unlock(>bo_list_lock);
@@ -209,9 +193,6 @@ ivpu_bo_create(struct ivpu_device *vdev, u64 size, u32
flags)
list_add_tail(>bo_list_node, >bo_list);
mutex_unlock(>bo_list_lock);
- ivpu_dbg(vdev, BO, "create: vpu_addr 0x%llx size %zu flags 0x%x\n",
-bo->vpu_addr, bo->base.base.size, flags);
-
return bo;
}
@@ -243,15 +224,15 @@ static void ivpu_bo_free(struct drm_gem_object *obj)
struct ivpu_device *vdev = to_ivpu_device(obj->dev);
struct ivpu_bo *bo = to_ivpu_bo(obj);
+ ivpu_dbg_bo(vdev, bo, "free");
+
mutex_lock(>bo_list_lock);
list_del(>bo_list_node);
mutex_unlock(>bo_list_lock);
drm_WARN_ON(>drm, !dma_resv_test_signaled(obj->resv, DMA_RESV_USAGE_READ));
- ivpu_dbg_bo(vdev, bo, "free");
-
- ivpu_bo_unbind(bo);
+ ivpu_bo_unbind_locked(bo);
This does not seem to be related to $SUBJECT
Re: [PATCH 08/10] accel/ivpu: Disable buffer sharing among VPU contexts
On 1/5/2024 4:22 AM, Jacek Lawrynowicz wrote:
This was not supported properly. A buffer was imported to another VPU
context as a separate buffer object with duplicated sgt.
Both exported and imported buffers could be DMA mapped causing a double
mapping on the same device.
Imported buffer from another VPU context will now have just reference
increased and there will be a single sgt fixing above problem but
buffers still can't be shared among VPU contexts because each context
have its own MMU mapping and ivpu_bo supports only single MMU mapping.
The solution would be to use a mapping list as in panfrost or etnaviv
drivers and it will be implemented in future if required.
Signed-off-by: Jacek Lawrynowicz
---
drivers/accel/ivpu/ivpu_gem.c | 44 +--
1 file changed, 6 insertions(+), 38 deletions(-)
diff --git a/drivers/accel/ivpu/ivpu_gem.c b/drivers/accel/ivpu/ivpu_gem.c
index 4de454bfbf91..8cb4d337552e 100644
--- a/drivers/accel/ivpu/ivpu_gem.c
+++ b/drivers/accel/ivpu/ivpu_gem.c
@@ -222,6 +222,12 @@ static int ivpu_bo_open(struct drm_gem_object *obj, struct
drm_file *file)
struct ivpu_bo *bo = to_ivpu_bo(obj);
struct ivpu_addr_range *range;
+ if (bo->ctx) {
+ ivpu_warn(vdev, "Can't add BO (vpu_addr 0x%llx) to ctx %u: already
in ctx %u\n",
+ bo->vpu_addr, file_priv->ctx.id, bo->ctx->id);
Looks like the vpu_addr is being used as a unique identifier for the BO.
Is that really the best value to use? Seems like if I want to attack
another context, knowing the device address of something that context
owns would be useful information.
Re: [PATCH 07/10] accel/ivpu: Free buffer sgt on unbind
On 1/5/2024 4:22 AM, Jacek Lawrynowicz wrote: Call dma_unmap() on all buffers before to VPU is unbinded to avoid to -> the ? "device driver has pending DMA allocations while released from device" warning when DMA-API debug is enabled. Signed-off-by: Jacek Lawrynowicz Reviewed-by: Jeffrey Hugo
Re: [PATCH 05/10] accel/ivpu: Fix potential infinite loops in IRQ handlers
On 1/5/2024 4:22 AM, Jacek Lawrynowicz wrote:
Limit number of iterations in ivpu_mmu_irq_evtq_handler() and
ivpu_ipc_irq_handler().
"potential infinite loops" sounds like something that has not been
observed. Has a problem actually occurred?
Are you concerned that the FW is broken and spamming Linux with events?
Why a limit of 100 events? Seems arbitrary.
I suspect threaded irqs might be useful here, but it is hard to tell.
Signed-off-by: Jacek Lawrynowicz
---
drivers/accel/ivpu/ivpu_ipc.c | 6 ++
drivers/accel/ivpu/ivpu_mmu.c | 21 +
2 files changed, 19 insertions(+), 8 deletions(-)
diff --git a/drivers/accel/ivpu/ivpu_ipc.c b/drivers/accel/ivpu/ivpu_ipc.c
index e86621f16f85..f69780248803 100644
--- a/drivers/accel/ivpu/ivpu_ipc.c
+++ b/drivers/accel/ivpu/ivpu_ipc.c
@@ -389,12 +389,18 @@ void ivpu_ipc_irq_handler(struct ivpu_device *vdev, bool
*wake_thread)
unsigned long flags;
bool dispatched;
u32 vpu_addr;
+ int msg_count = 0;
/*
* Driver needs to purge all messages from IPC FIFO to clear IPC
interrupt.
* Without purge IPC FIFO to 0 next IPC interrupts won't be generated.
*/
while (ivpu_hw_reg_ipc_rx_count_get(vdev)) {
+ if (++msg_count > IPC_MAX_RX_MSG) {
+ ivpu_pm_schedule_recovery(vdev);
+ return;
+ }
+
vpu_addr = ivpu_hw_reg_ipc_rx_addr_get(vdev);
if (vpu_addr == REG_IO_ERROR) {
ivpu_err_ratelimited(vdev, "Failed to read IPC rx addr
register\n");
diff --git a/drivers/accel/ivpu/ivpu_mmu.c b/drivers/accel/ivpu/ivpu_mmu.c
index 1f813625aab3..c82929b0ae9d 100644
--- a/drivers/accel/ivpu/ivpu_mmu.c
+++ b/drivers/accel/ivpu/ivpu_mmu.c
@@ -236,6 +236,8 @@
#define IVPU_MMU_CERROR_ABT 0x2
#define IVPU_MMU_CERROR_ATC_INV_SYNC 0x3
+#define IVPU_MMU_MAX_EVENT_COUNT 100
+
static const char *ivpu_mmu_event_to_str(u32 cmd)
{
switch (cmd) {
@@ -887,7 +889,7 @@ static u32 *ivpu_mmu_get_event(struct ivpu_device *vdev)
void ivpu_mmu_irq_evtq_handler(struct ivpu_device *vdev)
{
- bool schedule_recovery = false;
+ int event_count = 0;
u32 *event;
u32 ssid;
@@ -895,16 +897,19 @@ void ivpu_mmu_irq_evtq_handler(struct ivpu_device *vdev)
while ((event = ivpu_mmu_get_event(vdev)) != NULL) {
ivpu_mmu_dump_event(vdev, event);
+ if (++event_count > IVPU_MMU_MAX_EVENT_COUNT) {
+ ivpu_pm_schedule_recovery(vdev);
+ return;
+ }
ssid = FIELD_GET(IVPU_MMU_EVT_SSID_MASK, event[0]);
- if (ssid == IVPU_GLOBAL_CONTEXT_MMU_SSID)
- schedule_recovery = true;
- else
- ivpu_mmu_user_context_mark_invalid(vdev, ssid);
- }
+ if (ssid == IVPU_GLOBAL_CONTEXT_MMU_SSID) {
+ ivpu_pm_schedule_recovery(vdev);
+ return;
+ }
- if (schedule_recovery)
- ivpu_pm_schedule_recovery(vdev);
+ ivpu_mmu_user_context_mark_invalid(vdev, ssid);
+ }
}
void ivpu_mmu_evtq_dump(struct ivpu_device *vdev)
Re: [PATCH 06/10] accel/ivpu: Fix for missing lock around drm_gem_shmem_vmap()
On 1/5/2024 4:22 AM, Jacek Lawrynowicz wrote: drm_gem_shmem_vmap/vunmap requires dma resv lock to be held. This was missed during conversion to shmem helper. > Signed-off-by: Jacek Lawrynowicz Fixes tag? Reviewed-by: Jeffrey Hugo
Re: [PATCH 04/10] accel/ivpu: Add diagnostic messages when VPU fails to boot or suspend
On 1/5/2024 4:22 AM, Jacek Lawrynowicz wrote: From: "Wachowski, Karol" Make boot/suspend failure debugging easier by dumping FW logs and error registers. Signed-off-by: Wachowski, Karol Signed-off-by: Jacek Lawrynowicz Reviewed-by: Jeffrey Hugo
Re: [PATCH 03/10] accel/ivpu: Add debug prints for MMU map/unmap operations
On 1/5/2024 4:22 AM, Jacek Lawrynowicz wrote:
From: "Wachowski, Karol"
It is common need to be able to see IOVA/physical to VPU addresses
Errant double space between "to" and "see"
mappings. Especially when debugging different kind of memory related
issues. Lack of such logs forces user to modify and recompile KMD manually.
This commit adds those logs under MMU debug mask which can be turned on
dynamically with module param during KMD load.
As far as I understand, the preference is to not expose any kind of raw
addresses as it is seen as a security issue, and usually the addresses
don't have any real value to someone reading logs, etc. I beleive I
picked this up from GregKH.
However, this commit text suggests there is value, and I see that one
needs to be root to enable this which could probably be considered a
sufficent gate to avoiding the data getting into the wrong hands.
Is it possible to provide more details as a justification for this?
Perhaps an example of a past issue where this data was necessary for debug?
Signed-off-by: Wachowski, Karol
Signed-off-by: Jacek Lawrynowicz
---
drivers/accel/ivpu/ivpu_drv.h | 1 +
drivers/accel/ivpu/ivpu_mmu_context.c | 9 +
2 files changed, 10 insertions(+)
diff --git a/drivers/accel/ivpu/ivpu_drv.h b/drivers/accel/ivpu/ivpu_drv.h
index ebc4b84f27b2..9b6e336626e3 100644
--- a/drivers/accel/ivpu/ivpu_drv.h
+++ b/drivers/accel/ivpu/ivpu_drv.h
@@ -56,6 +56,7 @@
#define IVPU_DBG_JSM BIT(10)
#define IVPU_DBG_KREF BIT(11)
#define IVPU_DBG_RPM BIT(12)
+#define IVPU_DBG_MMU_MAP BIT(13)
#define ivpu_err(vdev, fmt, ...) \
drm_err(&(vdev)->drm, "%s(): " fmt, __func__, ##__VA_ARGS__)
diff --git a/drivers/accel/ivpu/ivpu_mmu_context.c
b/drivers/accel/ivpu/ivpu_mmu_context.c
index 12a8c09d4547..fe6161299236 100644
--- a/drivers/accel/ivpu/ivpu_mmu_context.c
+++ b/drivers/accel/ivpu/ivpu_mmu_context.c
@@ -355,6 +355,9 @@ ivpu_mmu_context_map_sgt(struct ivpu_device *vdev, struct
ivpu_mmu_context *ctx,
dma_addr_t dma_addr = sg_dma_address(sg) - sg->offset;
size_t size = sg_dma_len(sg) + sg->offset;
+ ivpu_dbg(vdev, MMU_MAP, "Map ctx: %u dma_addr: 0x%llx vpu_addr: 0x%llx size: %lu\n",
+ctx->id, dma_addr, vpu_addr, size);
+
ret = ivpu_mmu_context_map_pages(vdev, ctx, vpu_addr, dma_addr,
size, prot);
if (ret) {
ivpu_err(vdev, "Failed to map context pages\n");
@@ -366,6 +369,7 @@ ivpu_mmu_context_map_sgt(struct ivpu_device *vdev, struct
ivpu_mmu_context *ctx,
/* Ensure page table modifications are flushed from wc buffers to memory */
wmb();
+
This looks like an unrelated whitespace change (although I see it pairs
with the whitespace change below).
mutex_unlock(>lock);
ret = ivpu_mmu_invalidate_tlb(vdev, ctx->id);
@@ -388,14 +392,19 @@ ivpu_mmu_context_unmap_sgt(struct ivpu_device *vdev,
struct ivpu_mmu_context *ct
mutex_lock(>lock);
for_each_sgtable_dma_sg(sgt, sg, i) {
+ dma_addr_t dma_addr = sg_dma_address(sg) - sg->offset;
size_t size = sg_dma_len(sg) + sg->offset;
+ ivpu_dbg(vdev, MMU_MAP, "Unmap ctx: %u dma_addr: 0x%llx vpu_addr: 0x%llx size: %lu\n",
+ctx->id, dma_addr, vpu_addr, size);
+
ivpu_mmu_context_unmap_pages(ctx, vpu_addr, size);
vpu_addr += size;
}
/* Ensure page table modifications are flushed from wc buffers to memory */
wmb();
+
This looks like an unrelated whitespace change.
mutex_unlock(>lock);
ret = ivpu_mmu_invalidate_tlb(vdev, ctx->id);
Re: [PATCH 02/10] accel/ivpu: Call diagnose failure in ivpu_mmu_cmdq_sync()
On 1/5/2024 4:22 AM, Jacek Lawrynowicz wrote: From: "Wachowski, Karol" Check for possible failure reasons in the buttress. Some errors (like external abort) should have corresponding buttress errors registers set indicating the real reason of failure. Signed-off-by: Wachowski, Karol Signed-off-by: Jacek Lawrynowicz Reviewed-by: Jeffrey Hugo
Re: [PATCH 01/10] accel/ivpu: Dump MMU events in case of VPU boot timeout
On 1/5/2024 4:22 AM, Jacek Lawrynowicz wrote: From: "Wachowski, Karol" Add ivpu_mmu_evtq_dump() function that dumps existing MMU events from MMU event queue. Call this function if VPU boot failed. Previously MMU events were only checked in interrupt handler, but if VPU failed to boot due to MMU faults, those faults were missed because of interrupts not yet being enabled. This will allow checking potential fault reason of VPU not booting. Signed-off-by: Wachowski, Karol Signed-off-by: Jacek Lawrynowicz Reviewed-by: Jeffrey Hugo
Re: [PATCH 01/16] accel/ivpu: Dump MMU events in case of VPU boot timeout
On 1/4/2024 4:56 AM, Jacek Lawrynowicz wrote: From: "Wachowski, Karol" Add ivpu_mmu_evtq_dump() function that dumps existing MMU events from MMU event queue. Call this function if VPU boot failed. Previously MMU events were only checked in interrupt handler, but if VPU failed to boot due to MMU faults, those faults were missed because of interrupts not yet being enabled. This will allow checking potential fault reason of VPU not booting. Signed-off-by: Wachowski, Karol Signed-off-by: Jacek Lawrynowicz Reviewed-by: Jeffrey Hugo
Re: linux-next: manual merge of the drm tree with the mm tree
On 1/1/2024 4:52 PM, Stephen Rothwell wrote:
Hi all,
Today's linux-next merge of the drm tree got a conflict in:
drivers/accel/qaic/qaic_data.c
between commit:
78f5d33f3dd4 ("mm, treewide: rename MAX_ORDER to MAX_PAGE_ORDER")
from the mm tree and commit:
47fbee5f27ed ("accel/qaic: Update MAX_ORDER use to be inclusive")
from the drm tree.
I fixed it up (see below) and can carry the fix as necessary. This
is now fixed as far as linux-next is concerned, but any non trivial
conflicts should be mentioned to your upstream maintainer when your tree
is submitted for merging. You may also want to consider cooperating
with the maintainer of the conflicting tree to minimise any particularly
complex conflicts.
Thanks Stephen. Your fixup is correct.
-Jeff
Re: [PATCH 0/7] qaic cleanups for 6.8
On 12/15/2023 11:05 AM, Jeffrey Hugo wrote: On 12/8/2023 9:34 AM, Jeffrey Hugo wrote: A set of cleanups to the driver to improve error cases and reduce some code duplication. Jeffrey Hugo (2): accel/qaic: Fix MHI channel struct field order accel/qaic: Order pci_remove() operations in reverse of probe() Pranjal Ramajor Asha Kanojiya (5): accel/qaic: Deprecate ->size field from attach slice IOCTL structure accel/qaic: Remove bo->queued field accel/qaic: Drop the reference to BO in error path of create BO IOCTL accel/qaic: Call drm_gem_create_mmap_offset() once for each BO accel/qaic: Leverage DRM managed APIs to release resources drivers/accel/qaic/mhi_controller.c | 4 +- drivers/accel/qaic/qaic.h | 3 +- drivers/accel/qaic/qaic_data.c | 59 ++-- drivers/accel/qaic/qaic_drv.c | 140 ++-- include/uapi/drm/qaic_accel.h | 13 +-- 5 files changed, 119 insertions(+), 100 deletions(-) 1-5 pushed to drm-misc-next -Jeff 6 and 7 pushed to drm-misc-next -Jeff
Re: [PATCH 6/7] accel/qaic: Leverage DRM managed APIs to release resources
On 12/20/2023 12:02 AM, Jacek Lawrynowicz wrote: On 15.12.2023 19:06, Jeffrey Hugo wrote: On 12/8/2023 9:34 AM, Jeffrey Hugo wrote: From: Pranjal Ramajor Asha Kanojiya Offload the balancing of init and destroy calls to DRM managed APIs. mutex destroy for ->cntl_mutex is not called during device release and destroy workqueue is not called in error path of create_qdev(). So, use DRM managed APIs to manage the release of resources and avoid such problems. Signed-off-by: Pranjal Ramajor Asha Kanojiya Reviewed-by: Jeffrey Hugo Signed-off-by: Jeffrey Hugo Jacek, I saw a review from you for 1-5, and 7 but not this patch (6). Did I miss something? Sorry, I was out of office for a couple days and I wasn't able to finish the review. Reviewed-by: Jacek Lawrynowicz No problem. I thought maybe I had an issue on my end. I appreciate the reviews. Hopefully your out of office was enjoyable. Happy Holidays. -Jeff
Re: [PATCH 6/7] accel/qaic: Leverage DRM managed APIs to release resources
On 12/8/2023 9:34 AM, Jeffrey Hugo wrote: From: Pranjal Ramajor Asha Kanojiya Offload the balancing of init and destroy calls to DRM managed APIs. mutex destroy for ->cntl_mutex is not called during device release and destroy workqueue is not called in error path of create_qdev(). So, use DRM managed APIs to manage the release of resources and avoid such problems. Signed-off-by: Pranjal Ramajor Asha Kanojiya Reviewed-by: Jeffrey Hugo Signed-off-by: Jeffrey Hugo Jacek, I saw a review from you for 1-5, and 7 but not this patch (6). Did I miss something? -Jeff
Re: [PATCH 0/7] qaic cleanups for 6.8
On 12/8/2023 9:34 AM, Jeffrey Hugo wrote: A set of cleanups to the driver to improve error cases and reduce some code duplication. Jeffrey Hugo (2): accel/qaic: Fix MHI channel struct field order accel/qaic: Order pci_remove() operations in reverse of probe() Pranjal Ramajor Asha Kanojiya (5): accel/qaic: Deprecate ->size field from attach slice IOCTL structure accel/qaic: Remove bo->queued field accel/qaic: Drop the reference to BO in error path of create BO IOCTL accel/qaic: Call drm_gem_create_mmap_offset() once for each BO accel/qaic: Leverage DRM managed APIs to release resources drivers/accel/qaic/mhi_controller.c | 4 +- drivers/accel/qaic/qaic.h | 3 +- drivers/accel/qaic/qaic_data.c | 59 ++-- drivers/accel/qaic/qaic_drv.c | 140 ++-- include/uapi/drm/qaic_accel.h | 13 +-- 5 files changed, 119 insertions(+), 100 deletions(-) 1-5 pushed to drm-misc-next -Jeff
Re: [PATCH 0/2] qaic fixes for 6.7
On 12/8/2023 9:30 AM, Jeffrey Hugo wrote: A pair of fixes to the driver. First one is an improvement to dma_buf handling based on a greater understanding of that framework. The second is a reliability fix that allows some cards to boot. Jeffrey Hugo (1): accel/qaic: Implement quirk for SOC_HW_VERSION Pranjal Ramajor Asha Kanojiya (1): accel/qaic: Fix GEM import path code drivers/accel/qaic/mhi_controller.c | 15 ++- drivers/accel/qaic/qaic_data.c | 6 ++ 2 files changed, 16 insertions(+), 5 deletions(-) Applied to drm-misc-fixes -Jeff
[PATCH 0/7] qaic cleanups for 6.8
A set of cleanups to the driver to improve error cases and reduce some code duplication. Jeffrey Hugo (2): accel/qaic: Fix MHI channel struct field order accel/qaic: Order pci_remove() operations in reverse of probe() Pranjal Ramajor Asha Kanojiya (5): accel/qaic: Deprecate ->size field from attach slice IOCTL structure accel/qaic: Remove bo->queued field accel/qaic: Drop the reference to BO in error path of create BO IOCTL accel/qaic: Call drm_gem_create_mmap_offset() once for each BO accel/qaic: Leverage DRM managed APIs to release resources drivers/accel/qaic/mhi_controller.c | 4 +- drivers/accel/qaic/qaic.h | 3 +- drivers/accel/qaic/qaic_data.c | 59 ++-- drivers/accel/qaic/qaic_drv.c | 140 ++-- include/uapi/drm/qaic_accel.h | 13 +-- 5 files changed, 119 insertions(+), 100 deletions(-) -- 2.34.1
[PATCH 4/7] accel/qaic: Drop the reference to BO in error path of create BO IOCTL
From: Pranjal Ramajor Asha Kanojiya
Do not free BO explicitly in error path, just drop its reference, cleanup
will be taken care by DRM as we have registered for ->free() callback.
This patch makes sure that there is only one code path for BO to be freed.
Signed-off-by: Pranjal Ramajor Asha Kanojiya
Reviewed-by: Carl Vanderlip
Reviewed-by: Jeffrey Hugo
Signed-off-by: Jeffrey Hugo
---
drivers/accel/qaic/qaic_data.c | 9 +
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/accel/qaic/qaic_data.c b/drivers/accel/qaic/qaic_data.c
index 89ab8fa19315..7faa00705c1d 100644
--- a/drivers/accel/qaic/qaic_data.c
+++ b/drivers/accel/qaic/qaic_data.c
@@ -574,6 +574,9 @@ static void qaic_free_sgt(struct sg_table *sgt)
{
struct scatterlist *sg;
+ if (!sgt)
+ return;
+
for (sg = sgt->sgl; sg; sg = sg_next(sg))
if (sg_page(sg))
__free_pages(sg_page(sg), get_order(sg->length));
@@ -717,7 +720,7 @@ int qaic_create_bo_ioctl(struct drm_device *dev, void
*data, struct drm_file *fi
ret = drm_gem_handle_create(file_priv, obj, >handle);
if (ret)
- goto free_sgt;
+ goto free_bo;
bo->handle = args->handle;
drm_gem_object_put(obj);
@@ -726,10 +729,8 @@ int qaic_create_bo_ioctl(struct drm_device *dev, void
*data, struct drm_file *fi
return 0;
-free_sgt:
- qaic_free_sgt(bo->sgt);
free_bo:
- kfree(bo);
+ drm_gem_object_put(obj);
unlock_dev_srcu:
srcu_read_unlock(>dev_lock, qdev_rcu_id);
unlock_usr_srcu:
--
2.34.1
[PATCH 7/7] accel/qaic: Order pci_remove() operations in reverse of probe()
In probe() we create the drm_device, and then register the MHI controller. In remove(), we should unregister the controller first, then remove the drm_device. Update the remove() operations to match. Signed-off-by: Jeffrey Hugo Reviewed-by: Pranjal Ramajor Asha Kanojiya Reviewed-by: Carl Vanderlip --- drivers/accel/qaic/qaic_drv.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/accel/qaic/qaic_drv.c b/drivers/accel/qaic/qaic_drv.c index 10a43d02844f..d1a632dbaec6 100644 --- a/drivers/accel/qaic/qaic_drv.c +++ b/drivers/accel/qaic/qaic_drv.c @@ -550,8 +550,8 @@ static void qaic_pci_remove(struct pci_dev *pdev) return; qaic_dev_reset_clean_local_state(qdev); - qaic_destroy_drm_device(qdev, QAIC_NO_PARTITION); qaic_mhi_free_controller(qdev->mhi_cntrl, link_up); + qaic_destroy_drm_device(qdev, QAIC_NO_PARTITION); } static void qaic_pci_shutdown(struct pci_dev *pdev) -- 2.34.1
[PATCH 1/7] accel/qaic: Deprecate ->size field from attach slice IOCTL structure
From: Pranjal Ramajor Asha Kanojiya
->size in struct qaic_attach_slice_hdr is redundant since we have BO handle
and its size can be retrieved from base BO structure.
Signed-off-by: Pranjal Ramajor Asha Kanojiya
Reviewed-by: Jeffrey Hugo
Signed-off-by: Jeffrey Hugo
---
drivers/accel/qaic/qaic_data.c | 17 -
include/uapi/drm/qaic_accel.h | 13 +
2 files changed, 5 insertions(+), 25 deletions(-)
diff --git a/drivers/accel/qaic/qaic_data.c b/drivers/accel/qaic/qaic_data.c
index cf2898eda7ae..0c6f1328df68 100644
--- a/drivers/accel/qaic/qaic_data.c
+++ b/drivers/accel/qaic/qaic_data.c
@@ -830,9 +830,6 @@ static int qaic_prepare_import_bo(struct qaic_bo *bo,
struct qaic_attach_slice_h
struct sg_table *sgt;
int ret;
- if (obj->import_attach->dmabuf->size < hdr->size)
- return -EINVAL;
-
sgt = dma_buf_map_attachment(obj->import_attach, hdr->dir);
if (IS_ERR(sgt)) {
ret = PTR_ERR(sgt);
@@ -849,9 +846,6 @@ static int qaic_prepare_export_bo(struct qaic_device *qdev,
struct qaic_bo *bo,
{
int ret;
- if (bo->base.size < hdr->size)
- return -EINVAL;
-
ret = dma_map_sgtable(>pdev->dev, bo->sgt, hdr->dir, 0);
if (ret)
return -EFAULT;
@@ -952,9 +946,6 @@ int qaic_attach_slice_bo_ioctl(struct drm_device *dev, void
*data, struct drm_fi
if (arg_size / args->hdr.count != sizeof(*slice_ent))
return -EINVAL;
- if (args->hdr.size == 0)
- return -EINVAL;
-
if (!(args->hdr.dir == DMA_TO_DEVICE || args->hdr.dir ==
DMA_FROM_DEVICE))
return -EINVAL;
@@ -994,16 +985,16 @@ int qaic_attach_slice_bo_ioctl(struct drm_device *dev,
void *data, struct drm_fi
goto free_slice_ent;
}
- ret = qaic_validate_req(qdev, slice_ent, args->hdr.count,
args->hdr.size);
- if (ret)
- goto free_slice_ent;
-
obj = drm_gem_object_lookup(file_priv, args->hdr.handle);
if (!obj) {
ret = -ENOENT;
goto free_slice_ent;
}
+ ret = qaic_validate_req(qdev, slice_ent, args->hdr.count, obj->size);
+ if (ret)
+ goto put_bo;
+
bo = to_qaic_bo(obj);
ret = mutex_lock_interruptible(>lock);
if (ret)
diff --git a/include/uapi/drm/qaic_accel.h b/include/uapi/drm/qaic_accel.h
index 9dab32316aee..d3ca876a08e9 100644
--- a/include/uapi/drm/qaic_accel.h
+++ b/include/uapi/drm/qaic_accel.h
@@ -242,18 +242,7 @@ struct qaic_attach_slice_entry {
* @dbc_id: In. Associate the sliced BO with this DBC.
* @handle: In. GEM handle of the BO to slice.
* @dir: In. Direction of data flow. 1 = DMA_TO_DEVICE, 2 = DMA_FROM_DEVICE
- * @size: In. Total length of BO being used. This should not exceed base
- * size of BO (struct drm_gem_object.base)
- * For BOs being allocated using DRM_IOCTL_QAIC_CREATE_BO, size of
- * BO requested is PAGE_SIZE aligned then allocated hence allocated
- * BO size maybe bigger. This size should not exceed the new
- * PAGE_SIZE aligned BO size.
- * @dev_addr: In. Device address this slice pushes to or pulls from.
- * @db_addr: In. Address of the doorbell to ring.
- * @db_data: In. Data to write to the doorbell.
- * @db_len: In. Size of the doorbell data in bits - 32, 16, or 8. 0 is for
- * inactive doorbells.
- * @offset: In. Start of this slice as an offset from the start of the BO.
+ * @size: Deprecated. This value is ignored and size of @handle is used
instead.
*/
struct qaic_attach_slice_hdr {
__u32 count;
--
2.34.1
[PATCH 6/7] accel/qaic: Leverage DRM managed APIs to release resources
From: Pranjal Ramajor Asha Kanojiya
Offload the balancing of init and destroy calls to DRM managed APIs.
mutex destroy for ->cntl_mutex is not called during device release and
destroy workqueue is not called in error path of create_qdev(). So, use
DRM managed APIs to manage the release of resources and avoid such
problems.
Signed-off-by: Pranjal Ramajor Asha Kanojiya
Reviewed-by: Jeffrey Hugo
Signed-off-by: Jeffrey Hugo
---
drivers/accel/qaic/qaic.h | 1 +
drivers/accel/qaic/qaic_drv.c | 138 ++
2 files changed, 89 insertions(+), 50 deletions(-)
diff --git a/drivers/accel/qaic/qaic.h b/drivers/accel/qaic/qaic.h
index 2b3ef588b717..9256653b3036 100644
--- a/drivers/accel/qaic/qaic.h
+++ b/drivers/accel/qaic/qaic.h
@@ -30,6 +30,7 @@
#define to_qaic_drm_device(dev) container_of(dev, struct qaic_drm_device, drm)
#define to_drm(qddev) (&(qddev)->drm)
#define to_accel_kdev(qddev) (to_drm(qddev)->accel->kdev) /* Return Linux
device of accel node */
+#define to_qaic_device(dev) (to_qaic_drm_device((dev))->qdev)
enum __packed dev_states {
/* Device is offline or will be very soon */
diff --git a/drivers/accel/qaic/qaic_drv.c b/drivers/accel/qaic/qaic_drv.c
index 2a313eb69b12..10a43d02844f 100644
--- a/drivers/accel/qaic/qaic_drv.c
+++ b/drivers/accel/qaic/qaic_drv.c
@@ -44,6 +44,53 @@ MODULE_PARM_DESC(datapath_polling, "Operate the datapath in
polling mode");
static bool link_up;
static DEFINE_IDA(qaic_usrs);
+static void qaicm_wq_release(struct drm_device *dev, void *res)
+{
+ struct workqueue_struct *wq = res;
+
+ destroy_workqueue(wq);
+}
+
+static struct workqueue_struct *qaicm_wq_init(struct drm_device *dev, const
char *fmt)
+{
+ struct workqueue_struct *wq;
+ int ret;
+
+ wq = alloc_workqueue(fmt, WQ_UNBOUND, 0);
+ if (!wq)
+ return ERR_PTR(-ENOMEM);
+ ret = drmm_add_action_or_reset(dev, qaicm_wq_release, wq);
+ if (ret)
+ return ERR_PTR(ret);
+
+ return wq;
+}
+
+static void qaicm_srcu_release(struct drm_device *dev, void *res)
+{
+ struct srcu_struct *lock = res;
+
+ cleanup_srcu_struct(lock);
+}
+
+static int qaicm_srcu_init(struct drm_device *dev, struct srcu_struct *lock)
+{
+ int ret;
+
+ ret = init_srcu_struct(lock);
+ if (ret)
+ return ret;
+
+ return drmm_add_action_or_reset(dev, qaicm_srcu_release, lock);
+}
+
+static void qaicm_pci_release(struct drm_device *dev, void *res)
+{
+ struct qaic_device *qdev = to_qaic_device(dev);
+
+ pci_set_drvdata(qdev->pdev, NULL);
+}
+
static void free_usr(struct kref *kref)
{
struct qaic_user *usr = container_of(kref, struct qaic_user, ref_count);
@@ -299,74 +346,73 @@ void qaic_dev_reset_clean_local_state(struct qaic_device
*qdev)
release_dbc(qdev, i);
}
-static void cleanup_qdev(struct qaic_device *qdev)
-{
- int i;
-
- for (i = 0; i < qdev->num_dbc; ++i)
- cleanup_srcu_struct(>dbc[i].ch_lock);
- cleanup_srcu_struct(>dev_lock);
- pci_set_drvdata(qdev->pdev, NULL);
- destroy_workqueue(qdev->cntl_wq);
- destroy_workqueue(qdev->qts_wq);
-}
-
static struct qaic_device *create_qdev(struct pci_dev *pdev, const struct
pci_device_id *id)
{
+ struct device *dev = >dev;
struct qaic_drm_device *qddev;
struct qaic_device *qdev;
- int i;
+ struct drm_device *drm;
+ int i, ret;
- qdev = devm_kzalloc(>dev, sizeof(*qdev), GFP_KERNEL);
+ qdev = devm_kzalloc(dev, sizeof(*qdev), GFP_KERNEL);
if (!qdev)
return NULL;
qdev->dev_state = QAIC_OFFLINE;
if (id->device == PCI_DEV_AIC100) {
qdev->num_dbc = 16;
- qdev->dbc = devm_kcalloc(>dev, qdev->num_dbc,
sizeof(*qdev->dbc), GFP_KERNEL);
+ qdev->dbc = devm_kcalloc(dev, qdev->num_dbc,
sizeof(*qdev->dbc), GFP_KERNEL);
if (!qdev->dbc)
return NULL;
}
- qdev->cntl_wq = alloc_workqueue("qaic_cntl", WQ_UNBOUND, 0);
- if (!qdev->cntl_wq)
+ qddev = devm_drm_dev_alloc(>dev, _accel_driver, struct
qaic_drm_device, drm);
+ if (IS_ERR(qddev))
+ return NULL;
+
+ drm = to_drm(qddev);
+ pci_set_drvdata(pdev, qdev);
+
+ ret = drmm_mutex_init(drm, >users_mutex);
+ if (ret)
+ return NULL;
+ ret = drmm_add_action_or_reset(drm, qaicm_pci_release, NULL);
+ if (ret)
+ return NULL;
+ ret = drmm_mutex_init(drm, >cntl_mutex);
+ if (ret)
return NULL;
- qdev->qts_wq = alloc_workqueue("qaic_ts", WQ_UNBOUND, 0);
- if (!qdev->qts_wq) {
- destroy_workqueue(qdev->cntl_wq);
+ qdev->cntl_wq
[PATCH 5/7] accel/qaic: Call drm_gem_create_mmap_offset() once for each BO
From: Pranjal Ramajor Asha Kanojiya Every time QAIC_MMAP_BO ioctl is called for a BO, drm_gem_create_mmap_offset() is called. Calling drm_gem_create_mmap_offset() more then once for a BO seems redundant. Signed-off-by: Pranjal Ramajor Asha Kanojiya Reviewed-by: Jeffrey Hugo Signed-off-by: Jeffrey Hugo --- drivers/accel/qaic/qaic_data.c | 10 ++ 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/drivers/accel/qaic/qaic_data.c b/drivers/accel/qaic/qaic_data.c index 7faa00705c1d..f88d925c8001 100644 --- a/drivers/accel/qaic/qaic_data.c +++ b/drivers/accel/qaic/qaic_data.c @@ -718,6 +718,10 @@ int qaic_create_bo_ioctl(struct drm_device *dev, void *data, struct drm_file *fi if (ret) goto free_bo; + ret = drm_gem_create_mmap_offset(obj); + if (ret) + goto free_bo; + ret = drm_gem_handle_create(file_priv, obj, >handle); if (ret) goto free_bo; @@ -745,7 +749,7 @@ int qaic_mmap_bo_ioctl(struct drm_device *dev, void *data, struct drm_file *file struct drm_gem_object *obj; struct qaic_device *qdev; struct qaic_user *usr; - int ret; + int ret = 0; usr = file_priv->driver_priv; usr_rcu_id = srcu_read_lock(>qddev_lock); @@ -767,9 +771,7 @@ int qaic_mmap_bo_ioctl(struct drm_device *dev, void *data, struct drm_file *file goto unlock_dev_srcu; } - ret = drm_gem_create_mmap_offset(obj); - if (ret == 0) - args->offset = drm_vma_node_offset_addr(>vma_node); + args->offset = drm_vma_node_offset_addr(>vma_node); drm_gem_object_put(obj); -- 2.34.1
[PATCH 3/7] accel/qaic: Fix MHI channel struct field order
The timesync channels have their struct fields out of order with the rest
of the channels. Fix them so there is a consistent style in the file.
Signed-off-by: Jeffrey Hugo
Reviewed-by: Carl Vanderlip
---
drivers/accel/qaic/mhi_controller.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/accel/qaic/mhi_controller.c
b/drivers/accel/qaic/mhi_controller.c
index 832464f2833a..364eede0ac02 100644
--- a/drivers/accel/qaic/mhi_controller.c
+++ b/drivers/accel/qaic/mhi_controller.c
@@ -358,8 +358,8 @@ static struct mhi_channel_config aic100_channels[] = {
.wake_capable = false,
},
{
- .num = 21,
.name = "QAIC_TIMESYNC",
+ .num = 21,
.num_elements = 32,
.local_elements = 0,
.event_ring = 0,
@@ -390,8 +390,8 @@ static struct mhi_channel_config aic100_channels[] = {
.wake_capable = false,
},
{
- .num = 23,
.name = "QAIC_TIMESYNC_PERIODIC",
+ .num = 23,
.num_elements = 32,
.local_elements = 0,
.event_ring = 0,
--
2.34.1
[PATCH 2/7] accel/qaic: Remove bo->queued field
From: Pranjal Ramajor Asha Kanojiya
->queued field is used to track whether the BO is submitted to hardware for
DMA or not. Since same information can be retrieved using ->xfer_list field
of same structure remove ->queued as it is redundant.
Signed-off-by: Pranjal Ramajor Asha Kanojiya
Reviewed-by: Jeffrey Hugo
Signed-off-by: Jeffrey Hugo
---
drivers/accel/qaic/qaic.h | 2 --
drivers/accel/qaic/qaic_data.c | 23 +++
2 files changed, 11 insertions(+), 14 deletions(-)
diff --git a/drivers/accel/qaic/qaic.h b/drivers/accel/qaic/qaic.h
index 582836f9538f..2b3ef588b717 100644
--- a/drivers/accel/qaic/qaic.h
+++ b/drivers/accel/qaic/qaic.h
@@ -191,8 +191,6 @@ struct qaic_bo {
u32 nr_slice;
/* Number of slice that have been transferred by DMA engine */
u32 nr_slice_xfer_done;
- /* true = BO is queued for execution, true = BO is not queued */
- boolqueued;
/*
* If true then user has attached slicing information to this BO by
* calling DRM_IOCTL_QAIC_ATTACH_SLICE_BO ioctl.
diff --git a/drivers/accel/qaic/qaic_data.c b/drivers/accel/qaic/qaic_data.c
index 0c6f1328df68..89ab8fa19315 100644
--- a/drivers/accel/qaic/qaic_data.c
+++ b/drivers/accel/qaic/qaic_data.c
@@ -141,6 +141,11 @@ struct dbc_rsp {
__le16 status;
} __packed;
+static inline bool bo_queued(struct qaic_bo *bo)
+{
+ return !list_empty(>xfer_list);
+}
+
inline int get_dbc_req_elem_size(void)
{
return sizeof(struct dbc_req);
@@ -648,6 +653,7 @@ static void qaic_init_bo(struct qaic_bo *bo, bool reinit)
}
complete_all(>xfer_done);
INIT_LIST_HEAD(>slices);
+ INIT_LIST_HEAD(>xfer_list);
}
static struct qaic_bo *qaic_alloc_init_bo(void)
@@ -1166,7 +1172,6 @@ static int send_bo_list_to_device(struct qaic_device
*qdev, struct drm_file *fil
struct bo_slice *slice;
unsigned long flags;
struct qaic_bo *bo;
- bool queued;
int i, j;
int ret;
@@ -1198,9 +1203,7 @@ static int send_bo_list_to_device(struct qaic_device
*qdev, struct drm_file *fil
}
spin_lock_irqsave(>xfer_lock, flags);
- queued = bo->queued;
- bo->queued = true;
- if (queued) {
+ if (bo_queued(bo)) {
spin_unlock_irqrestore(>xfer_lock, flags);
ret = -EINVAL;
goto unlock_bo;
@@ -1223,7 +1226,6 @@ static int send_bo_list_to_device(struct qaic_device
*qdev, struct drm_file *fil
else
ret = copy_exec_reqs(qdev, slice, dbc->id,
head, tail);
if (ret) {
- bo->queued = false;
spin_unlock_irqrestore(>xfer_lock, flags);
goto unlock_bo;
}
@@ -1246,8 +1248,7 @@ static int send_bo_list_to_device(struct qaic_device
*qdev, struct drm_file *fil
spin_lock_irqsave(>xfer_lock, flags);
bo = list_last_entry(>xfer_list, struct qaic_bo,
xfer_list);
obj = >base;
- bo->queued = false;
- list_del(>xfer_list);
+ list_del_init(>xfer_list);
spin_unlock_irqrestore(>xfer_lock, flags);
dma_sync_sgtable_for_cpu(>pdev->dev, bo->sgt, bo->dir);
drm_gem_object_put(obj);
@@ -1608,8 +1609,7 @@ irqreturn_t dbc_irq_threaded_fn(int irq, void *data)
*/
dma_sync_sgtable_for_cpu(>pdev->dev, bo->sgt,
bo->dir);
bo->nr_slice_xfer_done = 0;
- bo->queued = false;
- list_del(>xfer_list);
+ list_del_init(>xfer_list);
bo->perf_stats.req_processed_ts = ktime_get_ns();
complete_all(>xfer_done);
drm_gem_object_put(>base);
@@ -1868,7 +1868,7 @@ int qaic_detach_slice_bo_ioctl(struct drm_device *dev,
void *data, struct drm_fi
/* Check if BO is committed to H/W for DMA */
spin_lock_irqsave(>xfer_lock, flags);
- if (bo->queued) {
+ if (bo_queued(bo)) {
spin_unlock_irqrestore(>xfer_lock, flags);
ret = -EBUSY;
goto unlock_ch_srcu;
@@ -1898,8 +1898,7 @@ static void empty_xfer_list(struct qaic_device *qdev,
struct dma_bridge_chan *db
spin_lock_irqsave(>xfer_lock, flags);
while (!list_empty(>xfer_list)) {
bo = list_first_entry(>xfer_list, typeof(*bo), xfer_list);
- bo->queued = false;
- list_del(>xfer_list);
+ list_del_init(&g
[PATCH 1/2] accel/qaic: Fix GEM import path code
From: Pranjal Ramajor Asha Kanojiya
Do not modify the size of dmabuf as it is immutable.
Fixes: ff13be830333 ("accel/qaic: Add datapath")
Signed-off-by: Pranjal Ramajor Asha Kanojiya
Reviewed-by: Jeffrey Hugo
Signed-off-by: Jeffrey Hugo
---
drivers/accel/qaic/qaic_data.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/accel/qaic/qaic_data.c b/drivers/accel/qaic/qaic_data.c
index 4a8e43a7a6a4..d42f002bc0cf 100644
--- a/drivers/accel/qaic/qaic_data.c
+++ b/drivers/accel/qaic/qaic_data.c
@@ -777,7 +777,6 @@ struct drm_gem_object *qaic_gem_prime_import(struct
drm_device *dev, struct dma_
struct dma_buf_attachment *attach;
struct drm_gem_object *obj;
struct qaic_bo *bo;
- size_t size;
int ret;
bo = qaic_alloc_init_bo();
@@ -795,13 +794,12 @@ struct drm_gem_object *qaic_gem_prime_import(struct
drm_device *dev, struct dma_
goto attach_fail;
}
- size = PAGE_ALIGN(attach->dmabuf->size);
- if (size == 0) {
+ if (!attach->dmabuf->size) {
ret = -EINVAL;
goto size_align_fail;
}
- drm_gem_private_object_init(dev, obj, size);
+ drm_gem_private_object_init(dev, obj, attach->dmabuf->size);
/*
* skipping dma_buf_map_attachment() as we do not know the direction
* just yet. Once the direction is known in the subsequent IOCTL to
--
2.34.1
[PATCH 2/2] accel/qaic: Implement quirk for SOC_HW_VERSION
The SOC_HW_VERSION register in the BHI space is not correctly initialized
by the device and in many cases contains uninitialized data. The register
could contain 0x which is a special value to indicate a link
error in PCIe, therefore if observed, we could incorrectly think the
device is down.
Intercept reads for this register, and provide the correct value - every
production instance would read 0x60110200 if the device was operating as
intended.
Fixes: a36bf7af868b ("accel/qaic: Add MHI controller")
Signed-off-by: Jeffrey Hugo
Reviewed-by: Pranjal Ramajor Asha Kanojiya
---
drivers/accel/qaic/mhi_controller.c | 15 ++-
1 file changed, 14 insertions(+), 1 deletion(-)
diff --git a/drivers/accel/qaic/mhi_controller.c
b/drivers/accel/qaic/mhi_controller.c
index 5036e58e7235..1405623b03e4 100644
--- a/drivers/accel/qaic/mhi_controller.c
+++ b/drivers/accel/qaic/mhi_controller.c
@@ -404,8 +404,21 @@ static struct mhi_controller_config aic100_config = {
static int mhi_read_reg(struct mhi_controller *mhi_cntrl, void __iomem *addr,
u32 *out)
{
- u32 tmp = readl_relaxed(addr);
+ u32 tmp;
+ /*
+* SOC_HW_VERSION quirk
+* The SOC_HW_VERSION register (offset 0x224) is not reliable and
+* may contain uninitialized values, including 0x. This could
+* cause a false positive link down error. Instead, intercept any
+* reads and provide the correct value of the register.
+*/
+ if (addr - mhi_cntrl->regs == 0x224) {
+ *out = 0x60110200;
+ return 0;
+ }
+
+ tmp = readl_relaxed(addr);
if (tmp == U32_MAX)
return -EIO;
--
2.34.1
