mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] drm/amdgpu/dm: Convert IRQ logging to drm_* helpers
@ 2025-12-18  6:35 suryasaimadhu
  2025-12-18 10:18 ` Christian König
  2025-12-21  9:19 ` kernel test robot
  0 siblings, 2 replies; 10+ messages in thread
From: suryasaimadhu @ 2025-12-18  6:35 UTC (permalink / raw)
  To: harry.wentland, sunpeng.li, alexander.deucher, christian.koenig,
	airlied, simona, amd-gfx
  Cc: siqueira, mario.limonciello, wayne.lin, roman.li, timur.kristof,
	dri-devel, linux-kernel, suryasaimadhu

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*
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);
-- 
2.43.0


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] drm/amdgpu/dm: Convert IRQ logging to drm_* helpers
  2025-12-18  6:35 [PATCH] drm/amdgpu/dm: Convert IRQ logging to drm_* helpers suryasaimadhu
@ 2025-12-18 10:18 ` Christian König
  2025-12-18 10:33   ` Sai Madhu
  2025-12-21  9:19 ` kernel test robot
  1 sibling, 1 reply; 10+ messages in thread
From: Christian König @ 2025-12-18 10:18 UTC (permalink / raw)
  To: suryasaimadhu, harry.wentland, sunpeng.li, alexander.deucher,
	airlied, simona, amd-gfx
  Cc: siqueira, mario.limonciello, wayne.lin, roman.li, timur.kristof,
	dri-devel, linux-kernel



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);


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] drm/amdgpu/dm: Convert IRQ logging to drm_* helpers
  2025-12-18 10:18 ` Christian König
@ 2025-12-18 10:33   ` Sai Madhu
  0 siblings, 0 replies; 10+ messages in thread
From: Sai Madhu @ 2025-12-18 10:33 UTC (permalink / raw)
  To: Christian König
  Cc: harry.wentland, sunpeng.li, alexander.deucher, airlied, simona,
	amd-gfx, siqueira, mario.limonciello, wayne.lin, roman.li,
	timur.kristof, dri-devel, linux-kernel

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] drm/amdgpu/dm: Convert IRQ logging to drm_* helpers
  2025-12-18  6:35 [PATCH] drm/amdgpu/dm: Convert IRQ logging to drm_* helpers suryasaimadhu
  2025-12-18 10:18 ` Christian König
@ 2025-12-21  9:19 ` kernel test robot
  1 sibling, 0 replies; 10+ messages in thread
From: kernel test robot @ 2025-12-21  9:19 UTC (permalink / raw)
  To: suryasaimadhu, harry.wentland, sunpeng.li, alexander.deucher,
	christian.koenig, airlied, simona, amd-gfx
  Cc: oe-kbuild-all, siqueira, mario.limonciello, wayne.lin, roman.li,
	timur.kristof, dri-devel, linux-kernel, suryasaimadhu

Hi suryasaimadhu,

kernel test robot noticed the following build warnings:

[auto build test WARNING on v6.18]
[also build test WARNING on next-20251219]
[cannot apply to drm-misc/drm-misc-next linus/master v6.19-rc1]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/suryasaimadhu/drm-amdgpu-dm-Convert-IRQ-logging-to-drm_-helpers/20251218-144203
base:   v6.18
patch link:    https://lore.kernel.org/r/20251218063512.4572-1-suryasaimadhu369%40gmail.com
patch subject: [PATCH] drm/amdgpu/dm: Convert IRQ logging to drm_* helpers
config: x86_64-rhel-9.4 (https://download.01.org/0day-ci/archive/20251221/202512211051.rcNkAntw-lkp@intel.com/config)
compiler: gcc-14 (Debian 14.2.0-19) 14.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251221/202512211051.rcNkAntw-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202512211051.rcNkAntw-lkp@intel.com/

All warnings (new ones prefixed by >>):

   drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm_irq.c: In function 'amdgpu_dm_irq_register_interrupt':
>> drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm_irq.c:308:9: warning: this 'if' clause does not guard... [-Wmisleading-indentation]
     308 |         if (false == validate_irq_registration_params(int_params, ih))
         |         ^~
   drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm_irq.c:310:17: note: ...this statement, but the latter is misleadingly indented as if it were guarded by the 'if'
     310 |                 return DAL_INVALID_IRQ_HANDLER_IDX;
         |                 ^~~~~~
   drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm_irq.c: In function 'amdgpu_dm_irq_unregister_interrupt':
   drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm_irq.c:375:9: warning: this 'if' clause does not guard... [-Wmisleading-indentation]
     375 |         if (false == validate_irq_unregistration_params(irq_source, ih))
         |         ^~
   drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm_irq.c:377:17: note: ...this statement, but the latter is misleadingly indented as if it were guarded by the 'if'
     377 |                 return;
         |                 ^~~~~~


vim +/if +308 drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm_irq.c

4562236b3bc0a2 Harry Wentland       2017-09-12  258  
d3a1c5548c1f21 suryasaimadhu        2025-12-18  259  static bool validate_irq_unregistration_params(
d3a1c5548c1f21 suryasaimadhu        2025-12-18  260  	enum dc_irq_source irq_source,
4562236b3bc0a2 Harry Wentland       2017-09-12  261  	irq_handler_idx handler_idx)
4562236b3bc0a2 Harry Wentland       2017-09-12  262  {
2d0b69fc712c07 Srinivasan Shanmugam 2023-06-22  263  	if (handler_idx == DAL_INVALID_IRQ_HANDLER_IDX) {
4562236b3bc0a2 Harry Wentland       2017-09-12  264  		return false;
4562236b3bc0a2 Harry Wentland       2017-09-12  265  	}
4562236b3bc0a2 Harry Wentland       2017-09-12  266  
4562236b3bc0a2 Harry Wentland       2017-09-12  267  	if (!DAL_VALID_IRQ_SRC_NUM(irq_source)) {
4562236b3bc0a2 Harry Wentland       2017-09-12  268  		return false;
4562236b3bc0a2 Harry Wentland       2017-09-12  269  	}
4562236b3bc0a2 Harry Wentland       2017-09-12  270  
4562236b3bc0a2 Harry Wentland       2017-09-12  271  	return true;
4562236b3bc0a2 Harry Wentland       2017-09-12  272  }
4562236b3bc0a2 Harry Wentland       2017-09-12  273  /******************************************************************************
4562236b3bc0a2 Harry Wentland       2017-09-12  274   * Public functions.
4562236b3bc0a2 Harry Wentland       2017-09-12  275   *
4562236b3bc0a2 Harry Wentland       2017-09-12  276   * Note: caller is responsible for input validation.
4562236b3bc0a2 Harry Wentland       2017-09-12  277   *****************************************************************************/
4562236b3bc0a2 Harry Wentland       2017-09-12  278  
b8592b48450b99 Leo Li               2018-09-14  279  /**
b8592b48450b99 Leo Li               2018-09-14  280   * amdgpu_dm_irq_register_interrupt() - Register a handler within DM.
b8592b48450b99 Leo Li               2018-09-14  281   * @adev: The base driver device containing the DM device.
b8592b48450b99 Leo Li               2018-09-14  282   * @int_params: Interrupt parameters containing the source, and handler context
b8592b48450b99 Leo Li               2018-09-14  283   * @ih: Function pointer to the interrupt handler to register
b8592b48450b99 Leo Li               2018-09-14  284   * @handler_args: Arguments passed to the handler when the interrupt occurs
b8592b48450b99 Leo Li               2018-09-14  285   *
b8592b48450b99 Leo Li               2018-09-14  286   * Register an interrupt handler for the given IRQ source, under the given
b8592b48450b99 Leo Li               2018-09-14  287   * context. The context can either be high or low. High context handlers are
b8592b48450b99 Leo Li               2018-09-14  288   * executed directly within ISR context, while low context is executed within a
b8592b48450b99 Leo Li               2018-09-14  289   * workqueue, thereby allowing operations that sleep.
b8592b48450b99 Leo Li               2018-09-14  290   *
b8592b48450b99 Leo Li               2018-09-14  291   * Registered handlers are called in a FIFO manner, i.e. the most recently
b8592b48450b99 Leo Li               2018-09-14  292   * registered handler will be called first.
b8592b48450b99 Leo Li               2018-09-14  293   *
b8592b48450b99 Leo Li               2018-09-14  294   * Return: Handler data &struct amdgpu_dm_irq_handler_data containing the IRQ
b8592b48450b99 Leo Li               2018-09-14  295   *         source, handler function, and args
b8592b48450b99 Leo Li               2018-09-14  296   */
e637525659ed04 Alex Deucher         2017-10-11  297  void *amdgpu_dm_irq_register_interrupt(struct amdgpu_device *adev,
4562236b3bc0a2 Harry Wentland       2017-09-12  298  				       struct dc_interrupt_params *int_params,
4562236b3bc0a2 Harry Wentland       2017-09-12  299  				       void (*ih)(void *),
4562236b3bc0a2 Harry Wentland       2017-09-12  300  				       void *handler_args)
4562236b3bc0a2 Harry Wentland       2017-09-12  301  {
d3a1c5548c1f21 suryasaimadhu        2025-12-18  302  	struct drm_device *dev = adev_to_drm(adev);
4562236b3bc0a2 Harry Wentland       2017-09-12  303  	struct list_head *hnd_list;
4562236b3bc0a2 Harry Wentland       2017-09-12  304  	struct amdgpu_dm_irq_handler_data *handler_data;
4562236b3bc0a2 Harry Wentland       2017-09-12  305  	unsigned long irq_table_flags;
4562236b3bc0a2 Harry Wentland       2017-09-12  306  	enum dc_irq_source irq_source;
4562236b3bc0a2 Harry Wentland       2017-09-12  307  
4562236b3bc0a2 Harry Wentland       2017-09-12 @308  	if (false == validate_irq_registration_params(int_params, ih))
d3a1c5548c1f21 suryasaimadhu        2025-12-18  309  		drm_err(dev, "DM_IRQ: invalid registration parameters\n");
4562236b3bc0a2 Harry Wentland       2017-09-12  310  		return DAL_INVALID_IRQ_HANDLER_IDX;
4562236b3bc0a2 Harry Wentland       2017-09-12  311  
4562236b3bc0a2 Harry Wentland       2017-09-12  312  	handler_data = kzalloc(sizeof(*handler_data), GFP_KERNEL);
4562236b3bc0a2 Harry Wentland       2017-09-12  313  	if (!handler_data) {
d3a1c5548c1f21 suryasaimadhu        2025-12-18  314  		drm_err(dev, "DM_IRQ: failed to allocate irq handler!\n");
4562236b3bc0a2 Harry Wentland       2017-09-12  315  		return DAL_INVALID_IRQ_HANDLER_IDX;
4562236b3bc0a2 Harry Wentland       2017-09-12  316  	}
4562236b3bc0a2 Harry Wentland       2017-09-12  317  
a7fbf17aa8bfaa Leo Li               2018-09-18  318  	init_handler_common_data(handler_data, ih, handler_args, &adev->dm);
4562236b3bc0a2 Harry Wentland       2017-09-12  319  
4562236b3bc0a2 Harry Wentland       2017-09-12  320  	irq_source = int_params->irq_source;
4562236b3bc0a2 Harry Wentland       2017-09-12  321  
4562236b3bc0a2 Harry Wentland       2017-09-12  322  	handler_data->irq_source = irq_source;
4562236b3bc0a2 Harry Wentland       2017-09-12  323  
4562236b3bc0a2 Harry Wentland       2017-09-12  324  	/* Lock the list, add the handler. */
4562236b3bc0a2 Harry Wentland       2017-09-12  325  	DM_IRQ_TABLE_LOCK(adev, irq_table_flags);
4562236b3bc0a2 Harry Wentland       2017-09-12  326  
4562236b3bc0a2 Harry Wentland       2017-09-12  327  	switch (int_params->int_context) {
4562236b3bc0a2 Harry Wentland       2017-09-12  328  	case INTERRUPT_HIGH_IRQ_CONTEXT:
4562236b3bc0a2 Harry Wentland       2017-09-12  329  		hnd_list = &adev->dm.irq_handler_list_high_tab[irq_source];
4562236b3bc0a2 Harry Wentland       2017-09-12  330  		break;
4562236b3bc0a2 Harry Wentland       2017-09-12  331  	case INTERRUPT_LOW_IRQ_CONTEXT:
4562236b3bc0a2 Harry Wentland       2017-09-12  332  	default:
b6f91fc183f758 Xiaogang Chen        2021-02-25  333  		hnd_list = &adev->dm.irq_handler_list_low_tab[irq_source];
b6f91fc183f758 Xiaogang Chen        2021-02-25  334  		INIT_WORK(&handler_data->work, dm_irq_work_func);
4562236b3bc0a2 Harry Wentland       2017-09-12  335  		break;
4562236b3bc0a2 Harry Wentland       2017-09-12  336  	}
4562236b3bc0a2 Harry Wentland       2017-09-12  337  
a7fbf17aa8bfaa Leo Li               2018-09-18  338  	list_add_tail(&handler_data->list, hnd_list);
4562236b3bc0a2 Harry Wentland       2017-09-12  339  
4562236b3bc0a2 Harry Wentland       2017-09-12  340  	DM_IRQ_TABLE_UNLOCK(adev, irq_table_flags);
4562236b3bc0a2 Harry Wentland       2017-09-12  341  
4562236b3bc0a2 Harry Wentland       2017-09-12  342  	/* This pointer will be stored by code which requested interrupt
4562236b3bc0a2 Harry Wentland       2017-09-12  343  	 * registration.
4562236b3bc0a2 Harry Wentland       2017-09-12  344  	 * The same pointer will be needed in order to unregister the
2d0b69fc712c07 Srinivasan Shanmugam 2023-06-22  345  	 * interrupt.
2d0b69fc712c07 Srinivasan Shanmugam 2023-06-22  346  	 */
4562236b3bc0a2 Harry Wentland       2017-09-12  347  
4562236b3bc0a2 Harry Wentland       2017-09-12  348  	DRM_DEBUG_KMS(
4562236b3bc0a2 Harry Wentland       2017-09-12  349  		"DM_IRQ: added irq handler: %p for: dal_src=%d, irq context=%d\n",
4562236b3bc0a2 Harry Wentland       2017-09-12  350  		handler_data,
4562236b3bc0a2 Harry Wentland       2017-09-12  351  		irq_source,
4562236b3bc0a2 Harry Wentland       2017-09-12  352  		int_params->int_context);
4562236b3bc0a2 Harry Wentland       2017-09-12  353  
4562236b3bc0a2 Harry Wentland       2017-09-12  354  	return handler_data;
4562236b3bc0a2 Harry Wentland       2017-09-12  355  }
4562236b3bc0a2 Harry Wentland       2017-09-12  356  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH] drm/amdgpu/dm: Convert IRQ logging to drm_* helpers
@ 2025-12-18 10:05 suryasaimadhu
  0 siblings, 0 replies; 10+ messages in thread
From: suryasaimadhu @ 2025-12-18 10:05 UTC (permalink / raw)
  To: harry.wentland, sunpeng.li, alexander.deucher, christian.koenig,
	airlied, simona, amd-gfx
  Cc: siqueira, mario.limonciello, wayne.lin, roman.li, timur.kristof,
	dri-devel, linux-kernel, suryasaimadhu

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

v3:
- Pass struct amdgpu_device to helpers instead of struct drm_device
- Compact drm_* logging statements

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*
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..82ea217e8195 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;
 	}
 
@@ -311,11 +305,12 @@ void *amdgpu_dm_irq_register_interrupt(struct amdgpu_device *adev,
 	enum dc_irq_source irq_source;
 
 	if (false == validate_irq_registration_params(int_params, ih))
+		drm_err(adev_to_drm(adev), "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(adev_to_drm(adev), "DM_IRQ: failed to allocate irq handler!\n");
 		return DAL_INVALID_IRQ_HANDLER_IDX;
 	}
 
@@ -376,6 +371,7 @@ void amdgpu_dm_irq_unregister_interrupt(struct amdgpu_device *adev,
 	int i;
 
 	if (false == validate_irq_unregistration_params(irq_source, ih))
+		drm_err(adev_to_drm(adev), "DM_IRQ: invalid unregistration parameters\n");
 		return;
 
 	memset(&int_params, 0, sizeof(int_params));
@@ -396,8 +392,8 @@ 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(
-		"DM_IRQ: failed to find irq handler:%p for irq_source:%d!\n",
+		drm_err(adev_to_drm(adev),
+			"DM_IRQ: failed to find irq handler:%p for irq_source:%d\n",
 			ih, irq_source);
 	}
 }
@@ -596,7 +592,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 +607,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,10 +716,9 @@ 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);
+			func, crtc_id);
 		return 0;
 	}
 
-- 
2.43.0


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] drm/amdgpu/dm: Convert IRQ logging to drm_* helpers
       [not found]   ` <CAB0uMAfx4uiXZg5HB=--0Hpnx6nD8pu4a=pWmFK2B8HHx4R=VQ@mail.gmail.com>
@ 2025-12-18  9:28     ` Sai Madhu
  0 siblings, 0 replies; 10+ messages in thread
From: Sai Madhu @ 2025-12-18  9:28 UTC (permalink / raw)
  To: Christian König
  Cc: harry.wentland, sunpeng.li, alexander.deucher, airlied, simona,
	amd-gfx, siqueira, mario.limonciello, wayne.lin, roman.li,
	timur.kristof, dri-devel, linux-kernel

Thanks for the review!



You’re right — I’ll switch those remaining cases to pass adev and derive
the drm_device via adev_to_drm(), and I’ll also compact the logging
statements a bit.



I’ll send a v3 shortly.


On Thu, 18 Dec 2025 at 14:57, Sai Madhu <suryasaimadhu369@gmail.com> wrote:
>
> Thanks for the review!
>
>
>
> You’re right — I’ll switch those remaining cases to pass adev and derive
> the drm_device via adev_to_drm(), and I’ll also compact the logging
> statements a bit.
>
>
>
> I’ll send a v3 shortly.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] drm/amdgpu/dm: Convert IRQ logging to drm_* helpers
  2025-12-17 17:01 suryasaimadhu
  2025-12-17 17:02 ` Mario Limonciello
@ 2025-12-18  9:14 ` Christian König
       [not found]   ` <CAB0uMAfx4uiXZg5HB=--0Hpnx6nD8pu4a=pWmFK2B8HHx4R=VQ@mail.gmail.com>
  1 sibling, 1 reply; 10+ messages in thread
From: Christian König @ 2025-12-18  9:14 UTC (permalink / raw)
  To: suryasaimadhu, harry.wentland, sunpeng.li, alexander.deucher,
	airlied, simona, amd-gfx
  Cc: siqueira, mario.limonciello, wayne.lin, roman.li, timur.kristof,
	dri-devel, linux-kernel



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@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*
> 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.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] drm/amdgpu/dm: Convert IRQ logging to drm_* helpers
  2025-12-17 17:02 ` Mario Limonciello
@ 2025-12-18  5:08   ` Sai Madhu
  0 siblings, 0 replies; 10+ messages in thread
From: Sai Madhu @ 2025-12-18  5:08 UTC (permalink / raw)
  To: Mario Limonciello
  Cc: harry.wentland, sunpeng.li, alexander.deucher, christian.koenig,
	airlied, simona, amd-gfx, siqueira, wayne.lin, roman.li,
	timur.kristof, dri-devel, linux-kernel

Thanks for the review.



You’re right — I should be using adev_to_drm() instead of accessing
adev->ddev directly.
I’ll rework the patch to:
  - use adev_to_drm(adev) for drm_* logging
  - drop any unrelated changes



I’ll send a v2 shortly.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] drm/amdgpu/dm: Convert IRQ logging to drm_* helpers
  2025-12-17 17:01 suryasaimadhu
@ 2025-12-17 17:02 ` Mario Limonciello
  2025-12-18  5:08   ` Sai Madhu
  2025-12-18  9:14 ` Christian König
  1 sibling, 1 reply; 10+ messages in thread
From: Mario Limonciello @ 2025-12-17 17:02 UTC (permalink / raw)
  To: suryasaimadhu, harry.wentland, sunpeng.li, alexander.deucher,
	christian.koenig, airlied, simona, amd-gfx
  Cc: siqueira, wayne.lin, roman.li, timur.kristof, dri-devel, linux-kernel

On 12/17/25 11:01 AM, 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@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
Unrelated changes.

>   
>   # *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,
>   				 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");

Use adev_to_drm().

>   		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);


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH] drm/amdgpu/dm: Convert IRQ logging to drm_* helpers
@ 2025-12-17 17:01 suryasaimadhu
  2025-12-17 17:02 ` Mario Limonciello
  2025-12-18  9:14 ` Christian König
  0 siblings, 2 replies; 10+ messages in thread
From: suryasaimadhu @ 2025-12-17 17:01 UTC (permalink / raw)
  To: harry.wentland, sunpeng.li, alexander.deucher, christian.koenig,
	airlied, simona, amd-gfx
  Cc: siqueira, mario.limonciello, wayne.lin, roman.li, timur.kristof,
	dri-devel, linux-kernel, suryasaimadhu

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@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*
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,
 				 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);
-- 
2.43.0


^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2025-12-21  9:20 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-12-18  6:35 [PATCH] drm/amdgpu/dm: Convert IRQ logging to drm_* helpers suryasaimadhu
2025-12-18 10:18 ` Christian König
2025-12-18 10:33   ` Sai Madhu
2025-12-21  9:19 ` kernel test robot
  -- strict thread matches above, loose matches on Subject: below --
2025-12-18 10:05 suryasaimadhu
2025-12-17 17:01 suryasaimadhu
2025-12-17 17:02 ` Mario Limonciello
2025-12-18  5:08   ` Sai Madhu
2025-12-18  9:14 ` Christian König
     [not found]   ` <CAB0uMAfx4uiXZg5HB=--0Hpnx6nD8pu4a=pWmFK2B8HHx4R=VQ@mail.gmail.com>
2025-12-18  9:28     ` Sai Madhu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®