| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <b767cb12-3023-44cf-9d4f-2fbb40c1431e@amd.com>
Date: Thu, 18 Dec 2025 10:14:55 +0100
From: Christian König <christian.koenig@....com>
To: suryasaimadhu <suryasaimadhu369@...il.com>, harry.wentland@....com,
sunpeng.li@....com, alexander.deucher@....com, airlied@...il.com,
simona@...ll.ch, amd-gfx@...ts.freedesktop.org
Cc: siqueira@...lia.com, mario.limonciello@....com, wayne.lin@....com,
roman.li@....com, timur.kristof@...il.com, dri-devel@...ts.freedesktop.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH] drm/amdgpu/dm: Convert IRQ logging to drm_* helpers
On 12/17/25 18:01, suryasaimadhu wrote:
> Replace DRM_ERROR(), DRM_WARN(), and DRM_INFO() usage in
> drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_irq.c with the
> corresponding drm_err(), drm_warn(), and drm_info() helpers.
>
> The drm_* logging helpers take a struct drm_device * as their first
> argument, allowing the DRM core to prefix log messages with the
> specific device name and instance. This is required to correctly
> differentiate log messages when multiple AMD GPUs are present.
>
> This aligns amdgpu_dm with the DRM TODO item to convert legacy DRM
> logging macros to the device-scoped drm_* helpers while keeping
> debug logging unchanged.
>
> Signed-off-by: suryasaimadhu <suryasaimadhu369@...il.com>
>
> diff --git a/Makefile b/Makefile
> index 2f545ec1690f..e404e4767944 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1,8 +1,8 @@
> # SPDX-License-Identifier: GPL-2.0
> VERSION = 6
> -PATCHLEVEL = 18
> +PATCHLEVEL = 19
> SUBLEVEL = 0
> -EXTRAVERSION =
> +EXTRAVERSION = -rc1
> NAME = Baby Opossum Posse
>
> # *DOCUMENTATION*
> 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 0a2a3f233a0e..4401059ff907 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
> @@ -238,22 +238,22 @@ static void unregister_all_irq_handlers(struct amdgpu_device *adev)
> }
>
> static bool
> -validate_irq_registration_params(struct dc_interrupt_params *int_params,
> +validate_irq_registration_params(struct drm_device *dev, struct dc_interrupt_params *int_params,
Please give adev here instead of the drm_device.
> void (*ih)(void *))
> {
> if (NULL == int_params || NULL == ih) {
> - DRM_ERROR("DM_IRQ: invalid input!\n");
> + drm_err(dev, "DM_IRQ: invalid input!\n");
> return false;
> }
>
> if (int_params->int_context >= INTERRUPT_CONTEXT_NUMBER) {
> - DRM_ERROR("DM_IRQ: invalid context: %d!\n",
> + drm_err(dev, "DM_IRQ: invalid context: %d!\n",
> int_params->int_context);
> return false;
> }
>
> if (!DAL_VALID_IRQ_SRC_NUM(int_params->irq_source)) {
> - DRM_ERROR("DM_IRQ: invalid irq_source: %d!\n",
> + drm_err(dev, "DM_IRQ: invalid irq_source: %d!\n",
> int_params->irq_source);
> return false;
> }
> @@ -261,16 +261,18 @@ validate_irq_registration_params(struct dc_interrupt_params *int_params,
> return true;
> }
>
> -static bool validate_irq_unregistration_params(enum dc_irq_source irq_source,
> - irq_handler_idx handler_idx)
> +static bool validate_irq_unregistration_params(
> + struct drm_device *dev,
> + enum dc_irq_source irq_source,
> + irq_handler_idx handler_idx)
> {
> if (handler_idx == DAL_INVALID_IRQ_HANDLER_IDX) {
> - DRM_ERROR("DM_IRQ: invalid handler_idx==NULL!\n");
> + drm_err(dev, "DM_IRQ: invalid handler_idx==NULL!\n");
> return false;
> }
>
> if (!DAL_VALID_IRQ_SRC_NUM(irq_source)) {
> - DRM_ERROR("DM_IRQ: invalid irq_source:%d!\n", irq_source);
> + drm_err(dev, "DM_IRQ: invalid irq_source:%d!\n", irq_source);
> return false;
> }
>
> @@ -310,12 +312,12 @@ void *amdgpu_dm_irq_register_interrupt(struct amdgpu_device *adev,
> unsigned long irq_table_flags;
> enum dc_irq_source irq_source;
>
> - if (false == validate_irq_registration_params(int_params, ih))
> + if (false == validate_irq_registration_params(&adev->ddev, int_params, ih))
> return DAL_INVALID_IRQ_HANDLER_IDX;
>
> handler_data = kzalloc(sizeof(*handler_data), GFP_KERNEL);
> if (!handler_data) {
> - DRM_ERROR("DM_IRQ: failed to allocate irq handler!\n");
> + drm_err(&adev->ddev, "DM_IRQ: failed to allocate irq handler!\n");
> return DAL_INVALID_IRQ_HANDLER_IDX;
> }
>
> @@ -375,7 +377,7 @@ void amdgpu_dm_irq_unregister_interrupt(struct amdgpu_device *adev,
> struct dc_interrupt_params int_params;
> int i;
>
> - if (false == validate_irq_unregistration_params(irq_source, ih))
> + if (false == validate_irq_unregistration_params(&adev->ddev, irq_source, ih))
> return;
>
> memset(&int_params, 0, sizeof(int_params));
> @@ -396,7 +398,7 @@ void amdgpu_dm_irq_unregister_interrupt(struct amdgpu_device *adev,
> /* If we got here, it means we searched all irq contexts
> * for this irq source, but the handler was not found.
> */
> - DRM_ERROR(
> + drm_err(&adev->ddev,
> "DM_IRQ: failed to find irq handler:%p for irq_source:%d!\n",
> ih, irq_source);
> }
> @@ -596,7 +598,7 @@ static void amdgpu_dm_irq_schedule_work(struct amdgpu_device *adev,
> /*allocate a new amdgpu_dm_irq_handler_data*/
> handler_data_add = kzalloc(sizeof(*handler_data), GFP_ATOMIC);
> if (!handler_data_add) {
> - DRM_ERROR("DM_IRQ: failed to allocate irq handler!\n");
> + drm_err(&adev->ddev, "DM_IRQ: failed to allocate irq handler!\n");
> return;
> }
>
> @@ -611,11 +613,11 @@ 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))
> - DRM_DEBUG("Queued work for handling interrupt from "
> + drm_dbg(&adev->ddev, "Queued work for handling interrupt from "
> "display for IRQ source %d\n",
> irq_source);
> else
> - DRM_ERROR("Failed to queue work for handling interrupt "
> + drm_err(&adev->ddev, "Failed to queue work for handling interrupt "
> "from display for IRQ source %d\n",
> irq_source);
> }
> @@ -720,7 +722,7 @@ static inline int dm_irq_state(struct amdgpu_device *adev,
> struct amdgpu_crtc *acrtc = adev->mode_info.crtcs[crtc_id];
>
> if (!acrtc) {
> - DRM_ERROR(
> + drm_err(&adev->ddev,
> "%s: crtc is NULL at id :%d\n",
> func,
> crtc_id);
While at it could you double check if that print can't be written a bit more compact. Looks like somebody just spread all parameters on individual lines.
Apart from those nit picks looks good to me.
Regards,
Christian.
Powered by blists - more mailing lists