On 2026-07-09 7:41, Leo Li wrote:
>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.
Agreed, this check is not needed at all and will be removed in the first patch
also.
>
>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)
