Re: [PATCH] drm/amdgpu/dm: Convert IRQ logging to drm_* helpers

["Christian König" <christian.koenig@amd.com>]

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@gmail.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);