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 <[email protected]> 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 <[email protected]> > > > > 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); >
