On 2026-06-29 12:51, Geoffrey McRae wrote:
> DM currently queues some deferred display work on system workqueues.
> Low-context IRQ handlers are queued on system_highpri_wq, while deferred
> vmin/vmax updates are queued on system_percpu_wq.
>
> Both paths can execute long-running display work. HPD and HPD RX handling
> may involve link detection, AUX transactions, connector state updates, and
> hotplug notification. The vmin/vmax update path calls into DC under
> dc_lock to adjust stream timing. These paths can therefore trigger the
> workqueue CPU hog detector when run from per-CPU workers:
>
> workqueue: dm_irq_work_func [amdgpu] hogged CPU for >10000us
> workqueue: dm_handle_vmin_vmax_update [amdgpu] hogged CPU for >10000us
>
> Move the deferred low-context IRQ work to a dedicated high-priority
> unbound workqueue, preserving the priority of the previous
> system_highpri_wq usage while avoiding long-running work on per-CPU
> workers.
>
> Move deferred vmin/vmax updates to a separate normal-priority unbound
> workqueue.
>
> High-context IRQ handlers remain unchanged and continue to run directly
> from the IRQ path.
>
> Signed-off-by: Geoffrey McRae <[email protected]>
> Cc: Harry Wentland <[email protected]>
> Cc: Leo Li <[email protected]>
> Cc: Alex Deucher <[email protected]>
> Cc: Christian König <[email protected]>
> ---
> .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 4 +++
> .../drm/amd/display/amdgpu_dm/amdgpu_dm_irq.c | 34 +++++++++++++++++--
> 2 files changed, 35 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
> index 88687a7e01a5..4a7965f76acb 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
> @@ -324,6 +324,8 @@ struct hpd_rx_irq_offload_work {
> * @ddev: DRM base driver structure
> * @display_indexes_num: Max number of display streams supported
> * @irq_handler_list_table_lock: Synchronizes access to IRQ tables
> + * @irq_wq: Dedicated high-priority unbound workqueue for deferred IRQ work
> + * @vmin_vmax_wq: Dedicated unbound workqueue for deferred vmin/vmax updates
> * @backlight_dev: Backlight control device
> * @backlight_link: Link on which to control backlight
> * @backlight_caps: Capabilities of the backlight device
> @@ -565,6 +567,8 @@ struct amdgpu_display_manager {
> dmub_outbox_params[1];
>
> spinlock_t irq_handler_list_table_lock;
> + struct workqueue_struct *irq_wq;
> + struct workqueue_struct *vmin_vmax_wq;
>
> struct backlight_device *backlight_dev[AMDGPU_DM_MAX_NUM_EDP];
>
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_irq.c
> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_irq.c
> index 3a5de9364ed1..f4bfd7e42f9a 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_irq.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_irq.c
> @@ -397,6 +397,21 @@ int amdgpu_dm_irq_init(struct amdgpu_device *adev)
>
> spin_lock_init(&adev->dm.irq_handler_list_table_lock);
>
> + adev->dm.irq_wq = alloc_workqueue("amdgpu_dm_irq",
> + WQ_UNBOUND | WQ_HIGHPRI, 0);
> +
> + if (!adev->dm.irq_wq)
> + return -ENOMEM;
> +
> + adev->dm.vmin_vmax_wq = alloc_workqueue("amdgpu_dm_vmin_vmax",
> + WQ_UNBOUND, 0);
> +
> + if (!adev->dm.vmin_vmax_wq) {
> + destroy_workqueue(adev->dm.irq_wq);
> + adev->dm.irq_wq = NULL;
> + return -ENOMEM;
> + }
> +
> for (src = 0; src < DAL_IRQ_SOURCES_NUMBER; src++) {
> /* low context handler list init */
> lh = &adev->dm.irq_handler_list_low_tab[src];
> @@ -491,6 +506,16 @@ void amdgpu_dm_irq_fini(struct amdgpu_device *adev)
> list_del(&handler->list);
> kfree(handler);
> }
> +
> + if (adev->dm.vmin_vmax_wq) {
> + destroy_workqueue(adev->dm.vmin_vmax_wq);
> + adev->dm.vmin_vmax_wq = NULL;
> + }
> +
> + if (adev->dm.irq_wq) {
> + destroy_workqueue(adev->dm.irq_wq);
> + adev->dm.irq_wq = NULL;
> + }
> }
> EXPORT_IF_KUNIT(amdgpu_dm_irq_fini);
>
> @@ -610,11 +635,14 @@ static void amdgpu_dm_irq_schedule_work(struct
> amdgpu_device *adev,
> if (READ_ONCE(adev->dm.irq_fini_in_progress))
> goto out_unlock;
>
> + if (!adev->dm.irq_wq)
> + goto out_unlock;
> +
Do we need this check if amdgpu_dm_irq_init() bails when dm.irq_wq fails to
init? No
handlers should be registered in that case.
Patch 2/2 LGTM otherwise.
- Leo
> if (list_empty(handler_list))
> goto out_unlock;
>
> list_for_each_entry(handler_data, handler_list, list) {
> - if (queue_work(system_highpri_wq, &handler_data->work)) {
> + if (queue_work(adev->dm.irq_wq, &handler_data->work)) {
> work_queued = true;
> break;
> }
> @@ -642,7 +670,7 @@ static void amdgpu_dm_irq_schedule_work(struct
> amdgpu_device *adev,
>
> INIT_WORK(&handler_data_add->work, dm_irq_work_func);
>
> - if (queue_work(system_highpri_wq, &handler_data_add->work))
> + if (queue_work(adev->dm.irq_wq, &handler_data_add->work))
> DRM_DEBUG("Queued work for handling interrupt from "
> "display for IRQ source %d\n",
> irq_source);
> @@ -1905,7 +1933,7 @@ static void schedule_dc_vmin_vmax(struct amdgpu_device
> *adev,
> offload_work->stream = stream;
> offload_work->adjust = adjust_copy;
>
> - queue_work(system_percpu_wq, &offload_work->work);
> + queue_work(adev->dm.vmin_vmax_wq, &offload_work->work);
> }
>
> static void dm_vupdate_high_irq(void *interrupt_params)
