| 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: <CAB0uMAfwMURUEtN2qJQKCDxDFPnHz3DwAKUGDwKSr_nhLPDhPg@mail.gmail.com>
Date: Thu, 18 Dec 2025 16:03:48 +0530
From: Sai Madhu <suryasaimadhu369@...il.com>
To: Christian König <christian.koenig@....com>
Cc: harry.wentland@....com, sunpeng.li@....com, alexander.deucher@....com,
airlied@...il.com, simona@...ll.ch, amd-gfx@...ts.freedesktop.org,
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
Hi Christian,
Thanks for the review.
You're right — that change is unrelated to the logging conversion.
I'll drop it and resend the patch as v4.
On Thu, 18 Dec 2025 at 15:49, Christian König <christian.koenig@....com> wrote:
>
>
>
> On 12/18/25 07:35, 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.
> >
> > v2:
> > - Keep validation helpers DRM-agnostic
> > - Move drm_* logging to AMDGPU DM callers
> > - Use adev_to_drm() for drm_* logging
> >
> > 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*
>
> That here clearly doesn't belong into the patch.
>
> Apart from that looks ok to me.
>
> Regards,
> Christian.
>
> > 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..60bf1b8d886a 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
> > @@ -242,35 +242,29 @@ validate_irq_registration_params(struct dc_interrupt_params *int_params,
> > void (*ih)(void *))
> > {
> > if (NULL == int_params || NULL == ih) {
> > - DRM_ERROR("DM_IRQ: invalid input!\n");
> > return false;
> > }
> >
> > if (int_params->int_context >= INTERRUPT_CONTEXT_NUMBER) {
> > - DRM_ERROR("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",
> > - int_params->irq_source);
> > return false;
> > }
> >
> > 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(
> > + 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");
> > return false;
> > }
> >
> > if (!DAL_VALID_IRQ_SRC_NUM(irq_source)) {
> > - DRM_ERROR("DM_IRQ: invalid irq_source:%d!\n", irq_source);
> > return false;
> > }
> >
> > @@ -305,17 +299,19 @@ void *amdgpu_dm_irq_register_interrupt(struct amdgpu_device *adev,
> > void (*ih)(void *),
> > void *handler_args)
> > {
> > + struct drm_device *dev = adev_to_drm(adev);
> > struct list_head *hnd_list;
> > struct amdgpu_dm_irq_handler_data *handler_data;
> > unsigned long irq_table_flags;
> > enum dc_irq_source irq_source;
> >
> > if (false == validate_irq_registration_params(int_params, ih))
> > + drm_err(dev, "DM_IRQ: invalid registration parameters\n");
> > 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(dev, "DM_IRQ: failed to allocate irq handler!\n");
> > return DAL_INVALID_IRQ_HANDLER_IDX;
> > }
> >
> > @@ -371,11 +367,13 @@ void amdgpu_dm_irq_unregister_interrupt(struct amdgpu_device *adev,
> > enum dc_irq_source irq_source,
> > void *ih)
> > {
> > + struct drm_device *dev = adev_to_drm(adev);
> > struct list_head *handler_list;
> > struct dc_interrupt_params int_params;
> > int i;
> >
> > if (false == validate_irq_unregistration_params(irq_source, ih))
> > + drm_err(dev, "DM_IRQ: invalid unregistration parameters\n");
> > return;
> >
> > memset(&int_params, 0, sizeof(int_params));
> > @@ -396,7 +394,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(dev,
> > "DM_IRQ: failed to find irq handler:%p for irq_source:%d!\n",
> > ih, irq_source);
> > }
> > @@ -596,7 +594,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_to_drm(adev), "DM_IRQ: failed to allocate irq handler!\n");
> > return;
> > }
> >
> > @@ -611,11 +609,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_to_drm(adev), "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_to_drm(adev), "Failed to queue work for handling interrupt "
> > "from display for IRQ source %d\n",
> > irq_source);
> > }
> > @@ -720,7 +718,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_to_drm(adev),
> > "%s: crtc is NULL at id :%d\n",
> > func,
> > crtc_id);
>
Powered by blists - more mailing lists