public inbox for linux-kernel-mentees@lists.linux-foundation.org
 help / color / mirror / Atom feed
* [PATCH] drm/komeda: Convert logging in komeda_crtc.c to drm_* with drm_device parameter
@ 2025-08-11  5:44 Rahul Kumar
  2025-08-11  7:36 ` kernel test robot
  2025-08-11  9:59 ` Liviu Dudau
  0 siblings, 2 replies; 7+ messages in thread
From: Rahul Kumar @ 2025-08-11  5:44 UTC (permalink / raw)
  To: liviu.dudau, maarten.lankhorst, mripard, tzimmermann, airlied,
	simona
  Cc: dri-devel, linux-kernel, linux-kernel-mentees, skhan, rk0006818

Replace all dev_err(), dev_warn(), dev_info() and DRM_ERROR/WARN/INFO()
calls in drivers/gpu/drm/arm/display/komeda/komeda_crtc.c with the
corresponding drm_err(), drm_warn(), and drm_info() helpers.

The new drm_*() logging functions take a struct drm_device * as the
first argument. This allows the DRM core to prefix log messages with
the specific DRM device name and instance, which is essential for
distinguishing logs when multiple GPUs or display controllers are present.

This change aligns komeda with the DRM TODO item: "Convert logging to
drm_* functions with drm_device parameter".

Signed-off-by: Rahul Kumar <rk0006818@gmail.com>
---
 .../gpu/drm/arm/display/komeda/komeda_crtc.c  | 37 +++++++++++--------
 1 file changed, 21 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c b/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
index 2ad33559a33a..b50ce3653ff6 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
@@ -111,6 +111,7 @@ komeda_crtc_atomic_check(struct drm_crtc *crtc,
 static int
 komeda_crtc_prepare(struct komeda_crtc *kcrtc)
 {
+	struct drm_device *drm = kcrtc->base.dev;
 	struct komeda_dev *mdev = kcrtc->base.dev->dev_private;
 	struct komeda_pipeline *master = kcrtc->master;
 	struct komeda_crtc_state *kcrtc_st = to_kcrtc_st(kcrtc->base.state);
@@ -128,8 +129,8 @@ komeda_crtc_prepare(struct komeda_crtc *kcrtc)
 
 	err = mdev->funcs->change_opmode(mdev, new_mode);
 	if (err) {
-		DRM_ERROR("failed to change opmode: 0x%x -> 0x%x.\n,",
-			  mdev->dpmode, new_mode);
+		drm_err(drm, "failed to change opmode: 0x%x -> 0x%x.\n,",
+			mdev->dpmode, new_mode);
 		goto unlock;
 	}
 
@@ -142,18 +143,18 @@ komeda_crtc_prepare(struct komeda_crtc *kcrtc)
 	if (new_mode != KOMEDA_MODE_DUAL_DISP) {
 		err = clk_set_rate(mdev->aclk, komeda_crtc_get_aclk(kcrtc_st));
 		if (err)
-			DRM_ERROR("failed to set aclk.\n");
+			drm_err(drm, "failed to set aclk.\n");
 		err = clk_prepare_enable(mdev->aclk);
 		if (err)
-			DRM_ERROR("failed to enable aclk.\n");
+			drm_err(drm, "failed to enable aclk.\n");
 	}
 
 	err = clk_set_rate(master->pxlclk, mode->crtc_clock * 1000);
 	if (err)
-		DRM_ERROR("failed to set pxlclk for pipe%d\n", master->id);
+		drm_err(drm, "failed to set pxlclk for pipe%d\n", master->id);
 	err = clk_prepare_enable(master->pxlclk);
 	if (err)
-		DRM_ERROR("failed to enable pxl clk for pipe%d.\n", master->id);
+		drm_err(drm, "failed to enable pxl clk for pipe%d.\n", master->id);
 
 unlock:
 	mutex_unlock(&mdev->lock);
@@ -164,6 +165,7 @@ komeda_crtc_prepare(struct komeda_crtc *kcrtc)
 static int
 komeda_crtc_unprepare(struct komeda_crtc *kcrtc)
 {
+	struct drm_device *drm = kcrtc->base.dev;
 	struct komeda_dev *mdev = kcrtc->base.dev->dev_private;
 	struct komeda_pipeline *master = kcrtc->master;
 	u32 new_mode;
@@ -180,8 +182,8 @@ komeda_crtc_unprepare(struct komeda_crtc *kcrtc)
 
 	err = mdev->funcs->change_opmode(mdev, new_mode);
 	if (err) {
-		DRM_ERROR("failed to change opmode: 0x%x -> 0x%x.\n,",
-			  mdev->dpmode, new_mode);
+		drm_err(drm, "failed to change opmode: 0x%x -> 0x%x.\n,",
+			mdev->dpmode, new_mode);
 		goto unlock;
 	}
 
@@ -200,6 +202,7 @@ komeda_crtc_unprepare(struct komeda_crtc *kcrtc)
 void komeda_crtc_handle_event(struct komeda_crtc   *kcrtc,
 			      struct komeda_events *evts)
 {
+	struct drm_device *drm = kcrtc->base.dev;
 	struct drm_crtc *crtc = &kcrtc->base;
 	u32 events = evts->pipes[kcrtc->master->id];
 
@@ -212,7 +215,7 @@ void komeda_crtc_handle_event(struct komeda_crtc   *kcrtc,
 		if (wb_conn)
 			drm_writeback_signal_completion(&wb_conn->base, 0);
 		else
-			DRM_WARN("CRTC[%d]: EOW happen but no wb_connector.\n",
+			drm_warn(drm, "CRTC[%d]: EOW happen but no wb_connector.\n",
 				 drm_crtc_index(&kcrtc->base));
 	}
 	/* will handle it together with the write back support */
@@ -236,7 +239,7 @@ void komeda_crtc_handle_event(struct komeda_crtc   *kcrtc,
 			crtc->state->event = NULL;
 			drm_crtc_send_vblank_event(crtc, event);
 		} else {
-			DRM_WARN("CRTC[%d]: FLIP happened but no pending commit.\n",
+			drm_warn(drm, "CRTC[%d]: FLIP happened but no pending commit.\n",
 				 drm_crtc_index(&kcrtc->base));
 		}
 		spin_unlock_irqrestore(&crtc->dev->event_lock, flags);
@@ -309,7 +312,7 @@ komeda_crtc_flush_and_wait_for_flip_done(struct komeda_crtc *kcrtc,
 
 	/* wait the flip take affect.*/
 	if (wait_for_completion_timeout(flip_done, HZ) == 0) {
-		DRM_ERROR("wait pipe%d flip done timeout\n", kcrtc->master->id);
+		drm_err(drm, "wait pipe%d flip done timeout\n", kcrtc->master->id);
 		if (!input_flip_done) {
 			unsigned long flags;
 
@@ -562,6 +565,7 @@ static const struct drm_crtc_funcs komeda_crtc_funcs = {
 int komeda_kms_setup_crtcs(struct komeda_kms_dev *kms,
 			   struct komeda_dev *mdev)
 {
+	struct drm_device *drm = &kms->base;
 	struct komeda_crtc *crtc;
 	struct komeda_pipeline *master;
 	char str[16];
@@ -581,7 +585,7 @@ int komeda_kms_setup_crtcs(struct komeda_kms_dev *kms,
 		else
 			sprintf(str, "None");
 
-		DRM_INFO("CRTC-%d: master(pipe-%d) slave(%s).\n",
+		drm_info(drm, "CRTC-%d: master(pipe-%d) slave(%s).\n",
 			 kms->n_crtcs, master->id, str);
 
 		kms->n_crtcs++;
@@ -609,10 +613,11 @@ get_crtc_primary(struct komeda_kms_dev *kms, struct komeda_crtc *crtc)
 	return NULL;
 }
 
-static int komeda_attach_bridge(struct device *dev,
-				struct komeda_pipeline *pipe,
+static int komeda_attach_bridge(struct komeda_pipeline *pipe,
 				struct drm_encoder *encoder)
 {
+	struct drm_device *drm = pipe->mdev->drm;
+	struct device *dev = drm->dev;
 	struct drm_bridge *bridge;
 	int err;
 
@@ -624,7 +629,7 @@ static int komeda_attach_bridge(struct device *dev,
 
 	err = drm_bridge_attach(encoder, bridge, NULL, 0);
 	if (err)
-		dev_err(dev, "bridge_attach() failed for pipe: %s\n",
+		drm_err(drm, "bridge_attach() failed for pipe: %s\n",
 			of_node_full_name(pipe->of_node));
 
 	return err;
@@ -658,7 +663,7 @@ static int komeda_crtc_add(struct komeda_kms_dev *kms,
 		return err;
 
 	if (pipe->of_output_links[0]) {
-		err = komeda_attach_bridge(base->dev, pipe, encoder);
+		err = komeda_attach_bridge(pipe, encoder);
 		if (err)
 			return err;
 	}
-- 
2.43.0


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

* Re: [PATCH] drm/komeda: Convert logging in komeda_crtc.c to drm_* with drm_device parameter
  2025-08-11  5:44 [PATCH] drm/komeda: Convert logging in komeda_crtc.c to drm_* with drm_device parameter Rahul Kumar
@ 2025-08-11  7:36 ` kernel test robot
  2025-08-11  9:59 ` Liviu Dudau
  1 sibling, 0 replies; 7+ messages in thread
From: kernel test robot @ 2025-08-11  7:36 UTC (permalink / raw)
  To: Rahul Kumar, liviu.dudau, maarten.lankhorst, mripard, tzimmermann,
	airlied, simona
  Cc: oe-kbuild-all, dri-devel, linux-kernel, linux-kernel-mentees,
	skhan, rk0006818

Hi Rahul,

kernel test robot noticed the following build errors:

[auto build test ERROR on linus/master]
[also build test ERROR on v6.17-rc1 next-20250808]
[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/Rahul-Kumar/drm-komeda-Convert-logging-in-komeda_crtc-c-to-drm_-with-drm_device-parameter/20250811-134646
base:   linus/master
patch link:    https://lore.kernel.org/r/20250811054459.15851-1-rk0006818%40gmail.com
patch subject: [PATCH] drm/komeda: Convert logging in komeda_crtc.c to drm_* with drm_device parameter
config: riscv-randconfig-001-20250811 (https://download.01.org/0day-ci/archive/20250811/202508111503.saEElm01-lkp@intel.com/config)
compiler: riscv64-linux-gcc (GCC) 8.5.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250811/202508111503.saEElm01-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/202508111503.saEElm01-lkp@intel.com/

All errors (new ones prefixed by >>):

   drivers/gpu/drm/arm/display/komeda/komeda_crtc.c: In function 'komeda_attach_bridge':
>> drivers/gpu/drm/arm/display/komeda/komeda_crtc.c:619:37: error: 'struct komeda_dev' has no member named 'drm'
     struct drm_device *drm = pipe->mdev->drm;
                                        ^~


vim +619 drivers/gpu/drm/arm/display/komeda/komeda_crtc.c

   615	
   616	static int komeda_attach_bridge(struct komeda_pipeline *pipe,
   617					struct drm_encoder *encoder)
   618	{
 > 619		struct drm_device *drm = pipe->mdev->drm;
   620		struct device *dev = drm->dev;
   621		struct drm_bridge *bridge;
   622		int err;
   623	
   624		bridge = devm_drm_of_get_bridge(dev, pipe->of_node,
   625						KOMEDA_OF_PORT_OUTPUT, 0);
   626		if (IS_ERR(bridge))
   627			return dev_err_probe(dev, PTR_ERR(bridge), "remote bridge not found for pipe: %s\n",
   628					     of_node_full_name(pipe->of_node));
   629	
   630		err = drm_bridge_attach(encoder, bridge, NULL, 0);
   631		if (err)
   632			drm_err(drm, "bridge_attach() failed for pipe: %s\n",
   633				of_node_full_name(pipe->of_node));
   634	
   635		return err;
   636	}
   637	

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

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

* Re: [PATCH] drm/komeda: Convert logging in komeda_crtc.c to drm_* with drm_device parameter
  2025-08-11  5:44 [PATCH] drm/komeda: Convert logging in komeda_crtc.c to drm_* with drm_device parameter Rahul Kumar
  2025-08-11  7:36 ` kernel test robot
@ 2025-08-11  9:59 ` Liviu Dudau
  1 sibling, 0 replies; 7+ messages in thread
From: Liviu Dudau @ 2025-08-11  9:59 UTC (permalink / raw)
  To: Rahul Kumar
  Cc: maarten.lankhorst, mripard, tzimmermann, airlied, simona,
	dri-devel, linux-kernel, linux-kernel-mentees, skhan

Hi,

On Mon, Aug 11, 2025 at 11:14:59AM +0530, Rahul Kumar wrote:
> Replace all dev_err(), dev_warn(), dev_info() and DRM_ERROR/WARN/INFO()
> calls in drivers/gpu/drm/arm/display/komeda/komeda_crtc.c with the
> corresponding drm_err(), drm_warn(), and drm_info() helpers.
> 
> The new drm_*() logging functions take a struct drm_device * as the
> first argument. This allows the DRM core to prefix log messages with
> the specific DRM device name and instance, which is essential for
> distinguishing logs when multiple GPUs or display controllers are present.
> 
> This change aligns komeda with the DRM TODO item: "Convert logging to
> drm_* functions with drm_device parameter".
> 
> Signed-off-by: Rahul Kumar <rk0006818@gmail.com>
> ---
>  .../gpu/drm/arm/display/komeda/komeda_crtc.c  | 37 +++++++++++--------
>  1 file changed, 21 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c b/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
> index 2ad33559a33a..b50ce3653ff6 100644
> --- a/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
> +++ b/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
> @@ -111,6 +111,7 @@ komeda_crtc_atomic_check(struct drm_crtc *crtc,
>  static int
>  komeda_crtc_prepare(struct komeda_crtc *kcrtc)
>  {
> +	struct drm_device *drm = kcrtc->base.dev;
>  	struct komeda_dev *mdev = kcrtc->base.dev->dev_private;
>  	struct komeda_pipeline *master = kcrtc->master;
>  	struct komeda_crtc_state *kcrtc_st = to_kcrtc_st(kcrtc->base.state);
> @@ -128,8 +129,8 @@ komeda_crtc_prepare(struct komeda_crtc *kcrtc)
>  
>  	err = mdev->funcs->change_opmode(mdev, new_mode);
>  	if (err) {
> -		DRM_ERROR("failed to change opmode: 0x%x -> 0x%x.\n,",
> -			  mdev->dpmode, new_mode);
> +		drm_err(drm, "failed to change opmode: 0x%x -> 0x%x.\n,",
> +			mdev->dpmode, new_mode);
>  		goto unlock;
>  	}
>  
> @@ -142,18 +143,18 @@ komeda_crtc_prepare(struct komeda_crtc *kcrtc)
>  	if (new_mode != KOMEDA_MODE_DUAL_DISP) {
>  		err = clk_set_rate(mdev->aclk, komeda_crtc_get_aclk(kcrtc_st));
>  		if (err)
> -			DRM_ERROR("failed to set aclk.\n");
> +			drm_err(drm, "failed to set aclk.\n");
>  		err = clk_prepare_enable(mdev->aclk);
>  		if (err)
> -			DRM_ERROR("failed to enable aclk.\n");
> +			drm_err(drm, "failed to enable aclk.\n");
>  	}
>  
>  	err = clk_set_rate(master->pxlclk, mode->crtc_clock * 1000);
>  	if (err)
> -		DRM_ERROR("failed to set pxlclk for pipe%d\n", master->id);
> +		drm_err(drm, "failed to set pxlclk for pipe%d\n", master->id);
>  	err = clk_prepare_enable(master->pxlclk);
>  	if (err)
> -		DRM_ERROR("failed to enable pxl clk for pipe%d.\n", master->id);
> +		drm_err(drm, "failed to enable pxl clk for pipe%d.\n", master->id);
>  
>  unlock:
>  	mutex_unlock(&mdev->lock);
> @@ -164,6 +165,7 @@ komeda_crtc_prepare(struct komeda_crtc *kcrtc)
>  static int
>  komeda_crtc_unprepare(struct komeda_crtc *kcrtc)
>  {
> +	struct drm_device *drm = kcrtc->base.dev;
>  	struct komeda_dev *mdev = kcrtc->base.dev->dev_private;
>  	struct komeda_pipeline *master = kcrtc->master;
>  	u32 new_mode;
> @@ -180,8 +182,8 @@ komeda_crtc_unprepare(struct komeda_crtc *kcrtc)
>  
>  	err = mdev->funcs->change_opmode(mdev, new_mode);
>  	if (err) {
> -		DRM_ERROR("failed to change opmode: 0x%x -> 0x%x.\n,",
> -			  mdev->dpmode, new_mode);
> +		drm_err(drm, "failed to change opmode: 0x%x -> 0x%x.\n,",
> +			mdev->dpmode, new_mode);
>  		goto unlock;
>  	}
>  
> @@ -200,6 +202,7 @@ komeda_crtc_unprepare(struct komeda_crtc *kcrtc)
>  void komeda_crtc_handle_event(struct komeda_crtc   *kcrtc,
>  			      struct komeda_events *evts)
>  {
> +	struct drm_device *drm = kcrtc->base.dev;
>  	struct drm_crtc *crtc = &kcrtc->base;
>  	u32 events = evts->pipes[kcrtc->master->id];
>  
> @@ -212,7 +215,7 @@ void komeda_crtc_handle_event(struct komeda_crtc   *kcrtc,
>  		if (wb_conn)
>  			drm_writeback_signal_completion(&wb_conn->base, 0);
>  		else
> -			DRM_WARN("CRTC[%d]: EOW happen but no wb_connector.\n",
> +			drm_warn(drm, "CRTC[%d]: EOW happen but no wb_connector.\n",
>  				 drm_crtc_index(&kcrtc->base));
>  	}
>  	/* will handle it together with the write back support */
> @@ -236,7 +239,7 @@ void komeda_crtc_handle_event(struct komeda_crtc   *kcrtc,
>  			crtc->state->event = NULL;
>  			drm_crtc_send_vblank_event(crtc, event);
>  		} else {
> -			DRM_WARN("CRTC[%d]: FLIP happened but no pending commit.\n",
> +			drm_warn(drm, "CRTC[%d]: FLIP happened but no pending commit.\n",
>  				 drm_crtc_index(&kcrtc->base));
>  		}
>  		spin_unlock_irqrestore(&crtc->dev->event_lock, flags);
> @@ -309,7 +312,7 @@ komeda_crtc_flush_and_wait_for_flip_done(struct komeda_crtc *kcrtc,
>  
>  	/* wait the flip take affect.*/
>  	if (wait_for_completion_timeout(flip_done, HZ) == 0) {
> -		DRM_ERROR("wait pipe%d flip done timeout\n", kcrtc->master->id);
> +		drm_err(drm, "wait pipe%d flip done timeout\n", kcrtc->master->id);
>  		if (!input_flip_done) {
>  			unsigned long flags;
>  
> @@ -562,6 +565,7 @@ static const struct drm_crtc_funcs komeda_crtc_funcs = {
>  int komeda_kms_setup_crtcs(struct komeda_kms_dev *kms,
>  			   struct komeda_dev *mdev)
>  {
> +	struct drm_device *drm = &kms->base;
>  	struct komeda_crtc *crtc;
>  	struct komeda_pipeline *master;
>  	char str[16];
> @@ -581,7 +585,7 @@ int komeda_kms_setup_crtcs(struct komeda_kms_dev *kms,
>  		else
>  			sprintf(str, "None");
>  
> -		DRM_INFO("CRTC-%d: master(pipe-%d) slave(%s).\n",
> +		drm_info(drm, "CRTC-%d: master(pipe-%d) slave(%s).\n",
>  			 kms->n_crtcs, master->id, str);
>  
>  		kms->n_crtcs++;
> @@ -609,10 +613,11 @@ get_crtc_primary(struct komeda_kms_dev *kms, struct komeda_crtc *crtc)
>  	return NULL;
>  }
>  
> -static int komeda_attach_bridge(struct device *dev,
> -				struct komeda_pipeline *pipe,
> +static int komeda_attach_bridge(struct komeda_pipeline *pipe,
>  				struct drm_encoder *encoder)
>  {
> +	struct drm_device *drm = pipe->mdev->drm;

As the kernel build bot has already said, there is no struct drm_device in komeda_dev. Not
sure what you're getting by trying to remove that parameter though.

Best regards,
Liviu


> +	struct device *dev = drm->dev;
>  	struct drm_bridge *bridge;
>  	int err;
>  
> @@ -624,7 +629,7 @@ static int komeda_attach_bridge(struct device *dev,
>  
>  	err = drm_bridge_attach(encoder, bridge, NULL, 0);
>  	if (err)
> -		dev_err(dev, "bridge_attach() failed for pipe: %s\n",
> +		drm_err(drm, "bridge_attach() failed for pipe: %s\n",
>  			of_node_full_name(pipe->of_node));
>  
>  	return err;
> @@ -658,7 +663,7 @@ static int komeda_crtc_add(struct komeda_kms_dev *kms,
>  		return err;
>  
>  	if (pipe->of_output_links[0]) {
> -		err = komeda_attach_bridge(base->dev, pipe, encoder);
> +		err = komeda_attach_bridge(pipe, encoder);
>  		if (err)
>  			return err;
>  	}
> -- 
> 2.43.0
> 

-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ¯\_(ツ)_/¯

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

* [PATCH] drm/komeda: Convert logging in komeda_crtc.c to drm_* with drm_device parameter
@ 2025-08-12 10:11 Rahul Kumar
  2025-08-12 11:12 ` Liviu Dudau
  0 siblings, 1 reply; 7+ messages in thread
From: Rahul Kumar @ 2025-08-12 10:11 UTC (permalink / raw)
  To: liviu.dudau, maarten.lankhorst, mripard, tzimmermann, airlied,
	simona
  Cc: dri-devel, linux-kernel, linux-kernel-mentees, skhan, rk0006818

Replace all dev_err(), dev_warn(), dev_info() and DRM_ERROR/WARN/INFO()
calls in drivers/gpu/drm/arm/display/komeda/komeda_crtc.c with the
corresponding drm_err(), drm_warn(), and drm_info() helpers.

The new drm_*() logging functions take a struct drm_device * as the
first argument. This allows the DRM core to prefix log messages with
the specific DRM device name and instance, which is essential for
distinguishing logs when multiple GPUs or display controllers are present.

This change aligns komeda with the DRM TODO item: "Convert logging to
drm_* functions with drm_device parameter".

Signed-off-by: Rahul Kumar <rk0006818@gmail.com>
---
 .../gpu/drm/arm/display/komeda/komeda_crtc.c  | 27 +++++++++++--------
 1 file changed, 16 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c b/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
index 2ad33559a33a..e4cc1fb34e94 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
@@ -111,6 +111,7 @@ komeda_crtc_atomic_check(struct drm_crtc *crtc,
 static int
 komeda_crtc_prepare(struct komeda_crtc *kcrtc)
 {
+	struct drm_device *drm = kcrtc->base.dev;
 	struct komeda_dev *mdev = kcrtc->base.dev->dev_private;
 	struct komeda_pipeline *master = kcrtc->master;
 	struct komeda_crtc_state *kcrtc_st = to_kcrtc_st(kcrtc->base.state);
@@ -128,7 +129,7 @@ komeda_crtc_prepare(struct komeda_crtc *kcrtc)
 
 	err = mdev->funcs->change_opmode(mdev, new_mode);
 	if (err) {
-		DRM_ERROR("failed to change opmode: 0x%x -> 0x%x.\n,",
+		drm_err(drm, "failed to change opmode: 0x%x -> 0x%x.\n,",
 			  mdev->dpmode, new_mode);
 		goto unlock;
 	}
@@ -142,18 +143,18 @@ komeda_crtc_prepare(struct komeda_crtc *kcrtc)
 	if (new_mode != KOMEDA_MODE_DUAL_DISP) {
 		err = clk_set_rate(mdev->aclk, komeda_crtc_get_aclk(kcrtc_st));
 		if (err)
-			DRM_ERROR("failed to set aclk.\n");
+			drm_err(drm, "failed to set aclk.\n");
 		err = clk_prepare_enable(mdev->aclk);
 		if (err)
-			DRM_ERROR("failed to enable aclk.\n");
+			drm_err(drm, "failed to enable aclk.\n");
 	}
 
 	err = clk_set_rate(master->pxlclk, mode->crtc_clock * 1000);
 	if (err)
-		DRM_ERROR("failed to set pxlclk for pipe%d\n", master->id);
+		drm_err(drm, "failed to set pxlclk for pipe%d\n", master->id);
 	err = clk_prepare_enable(master->pxlclk);
 	if (err)
-		DRM_ERROR("failed to enable pxl clk for pipe%d.\n", master->id);
+		drm_err(drm, "failed to enable pxl clk for pipe%d.\n", master->id);
 
 unlock:
 	mutex_unlock(&mdev->lock);
@@ -164,6 +165,7 @@ komeda_crtc_prepare(struct komeda_crtc *kcrtc)
 static int
 komeda_crtc_unprepare(struct komeda_crtc *kcrtc)
 {
+	struct drm_device *drm = kcrtc->base.dev;
 	struct komeda_dev *mdev = kcrtc->base.dev->dev_private;
 	struct komeda_pipeline *master = kcrtc->master;
 	u32 new_mode;
@@ -180,7 +182,7 @@ komeda_crtc_unprepare(struct komeda_crtc *kcrtc)
 
 	err = mdev->funcs->change_opmode(mdev, new_mode);
 	if (err) {
-		DRM_ERROR("failed to change opmode: 0x%x -> 0x%x.\n,",
+		drm_err(drm, "failed to change opmode: 0x%x -> 0x%x.\n,",
 			  mdev->dpmode, new_mode);
 		goto unlock;
 	}
@@ -200,6 +202,7 @@ komeda_crtc_unprepare(struct komeda_crtc *kcrtc)
 void komeda_crtc_handle_event(struct komeda_crtc   *kcrtc,
 			      struct komeda_events *evts)
 {
+	struct drm_device *drm = kcrtc->base.dev;
 	struct drm_crtc *crtc = &kcrtc->base;
 	u32 events = evts->pipes[kcrtc->master->id];
 
@@ -212,7 +215,7 @@ void komeda_crtc_handle_event(struct komeda_crtc   *kcrtc,
 		if (wb_conn)
 			drm_writeback_signal_completion(&wb_conn->base, 0);
 		else
-			DRM_WARN("CRTC[%d]: EOW happen but no wb_connector.\n",
+			drm_warn(drm, "CRTC[%d]: EOW happen but no wb_connector.\n",
 				 drm_crtc_index(&kcrtc->base));
 	}
 	/* will handle it together with the write back support */
@@ -236,7 +239,7 @@ void komeda_crtc_handle_event(struct komeda_crtc   *kcrtc,
 			crtc->state->event = NULL;
 			drm_crtc_send_vblank_event(crtc, event);
 		} else {
-			DRM_WARN("CRTC[%d]: FLIP happened but no pending commit.\n",
+			drm_warn(drm, "CRTC[%d]: FLIP happened but no pending commit.\n",
 				 drm_crtc_index(&kcrtc->base));
 		}
 		spin_unlock_irqrestore(&crtc->dev->event_lock, flags);
@@ -309,7 +312,7 @@ komeda_crtc_flush_and_wait_for_flip_done(struct komeda_crtc *kcrtc,
 
 	/* wait the flip take affect.*/
 	if (wait_for_completion_timeout(flip_done, HZ) == 0) {
-		DRM_ERROR("wait pipe%d flip done timeout\n", kcrtc->master->id);
+		drm_err(drm, "wait pipe%d flip done timeout\n", kcrtc->master->id);
 		if (!input_flip_done) {
 			unsigned long flags;
 
@@ -562,6 +565,7 @@ static const struct drm_crtc_funcs komeda_crtc_funcs = {
 int komeda_kms_setup_crtcs(struct komeda_kms_dev *kms,
 			   struct komeda_dev *mdev)
 {
+	struct drm_device *drm = &kms->base;
 	struct komeda_crtc *crtc;
 	struct komeda_pipeline *master;
 	char str[16];
@@ -581,7 +585,7 @@ int komeda_kms_setup_crtcs(struct komeda_kms_dev *kms,
 		else
 			sprintf(str, "None");
 
-		DRM_INFO("CRTC-%d: master(pipe-%d) slave(%s).\n",
+		drm_info(drm, "CRTC-%d: master(pipe-%d) slave(%s).\n",
 			 kms->n_crtcs, master->id, str);
 
 		kms->n_crtcs++;
@@ -613,6 +617,7 @@ static int komeda_attach_bridge(struct device *dev,
 				struct komeda_pipeline *pipe,
 				struct drm_encoder *encoder)
 {
+	struct drm_device *drm = encoder->dev;
 	struct drm_bridge *bridge;
 	int err;
 
@@ -624,7 +629,7 @@ static int komeda_attach_bridge(struct device *dev,
 
 	err = drm_bridge_attach(encoder, bridge, NULL, 0);
 	if (err)
-		dev_err(dev, "bridge_attach() failed for pipe: %s\n",
+		drm_err(drm, "bridge_attach() failed for pipe: %s\n",
 			of_node_full_name(pipe->of_node));
 
 	return err;
-- 
2.43.0


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

* Re: [PATCH] drm/komeda: Convert logging in komeda_crtc.c to drm_* with drm_device parameter
  2025-08-12 10:11 Rahul Kumar
@ 2025-08-12 11:12 ` Liviu Dudau
       [not found]   ` <CAKY2RyZX9D6J08S78zT97cHS3dJ7Cf0xURX7EougPmh-rMSMGQ@mail.gmail.com>
  2025-09-18 15:10   ` Rahul Kumar
  0 siblings, 2 replies; 7+ messages in thread
From: Liviu Dudau @ 2025-08-12 11:12 UTC (permalink / raw)
  To: Rahul Kumar
  Cc: maarten.lankhorst, mripard, tzimmermann, airlied, simona,
	dri-devel, linux-kernel, linux-kernel-mentees, skhan

On Tue, Aug 12, 2025 at 03:41:19PM +0530, Rahul Kumar wrote:
> Replace all dev_err(), dev_warn(), dev_info() and DRM_ERROR/WARN/INFO()
> calls in drivers/gpu/drm/arm/display/komeda/komeda_crtc.c with the
> corresponding drm_err(), drm_warn(), and drm_info() helpers.
> 
> The new drm_*() logging functions take a struct drm_device * as the
> first argument. This allows the DRM core to prefix log messages with
> the specific DRM device name and instance, which is essential for
> distinguishing logs when multiple GPUs or display controllers are present.
> 
> This change aligns komeda with the DRM TODO item: "Convert logging to
> drm_* functions with drm_device parameter".
> 
> Signed-off-by: Rahul Kumar <rk0006818@gmail.com>

Reviewed-by: Liviu Dudau <liviu.dudau@arm.com>

For future patches, when you make changes, please generate them with

git patch -v<version>

and include a change log so that reviewers can quickly figure out what
has changed between emails without having to go back and forth.

Best regards,
Liviu

> ---
>  .../gpu/drm/arm/display/komeda/komeda_crtc.c  | 27 +++++++++++--------
>  1 file changed, 16 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c b/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
> index 2ad33559a33a..e4cc1fb34e94 100644
> --- a/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
> +++ b/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
> @@ -111,6 +111,7 @@ komeda_crtc_atomic_check(struct drm_crtc *crtc,
>  static int
>  komeda_crtc_prepare(struct komeda_crtc *kcrtc)
>  {
> +	struct drm_device *drm = kcrtc->base.dev;
>  	struct komeda_dev *mdev = kcrtc->base.dev->dev_private;
>  	struct komeda_pipeline *master = kcrtc->master;
>  	struct komeda_crtc_state *kcrtc_st = to_kcrtc_st(kcrtc->base.state);
> @@ -128,7 +129,7 @@ komeda_crtc_prepare(struct komeda_crtc *kcrtc)
>  
>  	err = mdev->funcs->change_opmode(mdev, new_mode);
>  	if (err) {
> -		DRM_ERROR("failed to change opmode: 0x%x -> 0x%x.\n,",
> +		drm_err(drm, "failed to change opmode: 0x%x -> 0x%x.\n,",
>  			  mdev->dpmode, new_mode);
>  		goto unlock;
>  	}
> @@ -142,18 +143,18 @@ komeda_crtc_prepare(struct komeda_crtc *kcrtc)
>  	if (new_mode != KOMEDA_MODE_DUAL_DISP) {
>  		err = clk_set_rate(mdev->aclk, komeda_crtc_get_aclk(kcrtc_st));
>  		if (err)
> -			DRM_ERROR("failed to set aclk.\n");
> +			drm_err(drm, "failed to set aclk.\n");
>  		err = clk_prepare_enable(mdev->aclk);
>  		if (err)
> -			DRM_ERROR("failed to enable aclk.\n");
> +			drm_err(drm, "failed to enable aclk.\n");
>  	}
>  
>  	err = clk_set_rate(master->pxlclk, mode->crtc_clock * 1000);
>  	if (err)
> -		DRM_ERROR("failed to set pxlclk for pipe%d\n", master->id);
> +		drm_err(drm, "failed to set pxlclk for pipe%d\n", master->id);
>  	err = clk_prepare_enable(master->pxlclk);
>  	if (err)
> -		DRM_ERROR("failed to enable pxl clk for pipe%d.\n", master->id);
> +		drm_err(drm, "failed to enable pxl clk for pipe%d.\n", master->id);
>  
>  unlock:
>  	mutex_unlock(&mdev->lock);
> @@ -164,6 +165,7 @@ komeda_crtc_prepare(struct komeda_crtc *kcrtc)
>  static int
>  komeda_crtc_unprepare(struct komeda_crtc *kcrtc)
>  {
> +	struct drm_device *drm = kcrtc->base.dev;
>  	struct komeda_dev *mdev = kcrtc->base.dev->dev_private;
>  	struct komeda_pipeline *master = kcrtc->master;
>  	u32 new_mode;
> @@ -180,7 +182,7 @@ komeda_crtc_unprepare(struct komeda_crtc *kcrtc)
>  
>  	err = mdev->funcs->change_opmode(mdev, new_mode);
>  	if (err) {
> -		DRM_ERROR("failed to change opmode: 0x%x -> 0x%x.\n,",
> +		drm_err(drm, "failed to change opmode: 0x%x -> 0x%x.\n,",
>  			  mdev->dpmode, new_mode);
>  		goto unlock;
>  	}
> @@ -200,6 +202,7 @@ komeda_crtc_unprepare(struct komeda_crtc *kcrtc)
>  void komeda_crtc_handle_event(struct komeda_crtc   *kcrtc,
>  			      struct komeda_events *evts)
>  {
> +	struct drm_device *drm = kcrtc->base.dev;
>  	struct drm_crtc *crtc = &kcrtc->base;
>  	u32 events = evts->pipes[kcrtc->master->id];
>  
> @@ -212,7 +215,7 @@ void komeda_crtc_handle_event(struct komeda_crtc   *kcrtc,
>  		if (wb_conn)
>  			drm_writeback_signal_completion(&wb_conn->base, 0);
>  		else
> -			DRM_WARN("CRTC[%d]: EOW happen but no wb_connector.\n",
> +			drm_warn(drm, "CRTC[%d]: EOW happen but no wb_connector.\n",
>  				 drm_crtc_index(&kcrtc->base));
>  	}
>  	/* will handle it together with the write back support */
> @@ -236,7 +239,7 @@ void komeda_crtc_handle_event(struct komeda_crtc   *kcrtc,
>  			crtc->state->event = NULL;
>  			drm_crtc_send_vblank_event(crtc, event);
>  		} else {
> -			DRM_WARN("CRTC[%d]: FLIP happened but no pending commit.\n",
> +			drm_warn(drm, "CRTC[%d]: FLIP happened but no pending commit.\n",
>  				 drm_crtc_index(&kcrtc->base));
>  		}
>  		spin_unlock_irqrestore(&crtc->dev->event_lock, flags);
> @@ -309,7 +312,7 @@ komeda_crtc_flush_and_wait_for_flip_done(struct komeda_crtc *kcrtc,
>  
>  	/* wait the flip take affect.*/
>  	if (wait_for_completion_timeout(flip_done, HZ) == 0) {
> -		DRM_ERROR("wait pipe%d flip done timeout\n", kcrtc->master->id);
> +		drm_err(drm, "wait pipe%d flip done timeout\n", kcrtc->master->id);
>  		if (!input_flip_done) {
>  			unsigned long flags;
>  
> @@ -562,6 +565,7 @@ static const struct drm_crtc_funcs komeda_crtc_funcs = {
>  int komeda_kms_setup_crtcs(struct komeda_kms_dev *kms,
>  			   struct komeda_dev *mdev)
>  {
> +	struct drm_device *drm = &kms->base;
>  	struct komeda_crtc *crtc;
>  	struct komeda_pipeline *master;
>  	char str[16];
> @@ -581,7 +585,7 @@ int komeda_kms_setup_crtcs(struct komeda_kms_dev *kms,
>  		else
>  			sprintf(str, "None");
>  
> -		DRM_INFO("CRTC-%d: master(pipe-%d) slave(%s).\n",
> +		drm_info(drm, "CRTC-%d: master(pipe-%d) slave(%s).\n",
>  			 kms->n_crtcs, master->id, str);
>  
>  		kms->n_crtcs++;
> @@ -613,6 +617,7 @@ static int komeda_attach_bridge(struct device *dev,
>  				struct komeda_pipeline *pipe,
>  				struct drm_encoder *encoder)
>  {
> +	struct drm_device *drm = encoder->dev;
>  	struct drm_bridge *bridge;
>  	int err;
>  
> @@ -624,7 +629,7 @@ static int komeda_attach_bridge(struct device *dev,
>  
>  	err = drm_bridge_attach(encoder, bridge, NULL, 0);
>  	if (err)
> -		dev_err(dev, "bridge_attach() failed for pipe: %s\n",
> +		drm_err(drm, "bridge_attach() failed for pipe: %s\n",
>  			of_node_full_name(pipe->of_node));
>  
>  	return err;
> -- 
> 2.43.0
> 

-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ¯\_(ツ)_/¯

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

* Re: [PATCH] drm/komeda: Convert logging in komeda_crtc.c to drm_* with drm_device parameter
       [not found]   ` <CAKY2RyZX9D6J08S78zT97cHS3dJ7Cf0xURX7EougPmh-rMSMGQ@mail.gmail.com>
@ 2025-08-12 13:09     ` Rahul Kumar
  0 siblings, 0 replies; 7+ messages in thread
From: Rahul Kumar @ 2025-08-12 13:09 UTC (permalink / raw)
  To: Liviu Dudau
  Cc: maarten.lankhorst, mripard, tzimmermann, airlied, simona,
	dri-devel, linux-kernel, linux-kernel-mentees, skhan

Thanks for the review and feedback, Liviu.
I will follow this for my future patches.

Regards,
Rahul


On Tue, Aug 12, 2025 at 5:50 PM Rahul Kumar <rk0006818@gmail.com> wrote:
>
> Thanks for the review and feedback, Liviu.
> I will follow this for my future patches.
>
> Regards,
> Rahul
>
> On Tue, Aug 12, 2025 at 4:42 PM Liviu Dudau <liviu.dudau@arm.com> wrote:
>>
>> On Tue, Aug 12, 2025 at 03:41:19PM +0530, Rahul Kumar wrote:
>> > Replace all dev_err(), dev_warn(), dev_info() and DRM_ERROR/WARN/INFO()
>> > calls in drivers/gpu/drm/arm/display/komeda/komeda_crtc.c with the
>> > corresponding drm_err(), drm_warn(), and drm_info() helpers.
>> >
>> > The new drm_*() logging functions take a struct drm_device * as the
>> > first argument. This allows the DRM core to prefix log messages with
>> > the specific DRM device name and instance, which is essential for
>> > distinguishing logs when multiple GPUs or display controllers are present.
>> >
>> > This change aligns komeda with the DRM TODO item: "Convert logging to
>> > drm_* functions with drm_device parameter".
>> >
>> > Signed-off-by: Rahul Kumar <rk0006818@gmail.com>
>>
>> Reviewed-by: Liviu Dudau <liviu.dudau@arm.com>
>>
>> For future patches, when you make changes, please generate them with
>>
>> git patch -v<version>
>>
>> and include a change log so that reviewers can quickly figure out what
>> has changed between emails without having to go back and forth.
>>
>> Best regards,
>> Liviu
>>
>> > ---
>> >  .../gpu/drm/arm/display/komeda/komeda_crtc.c  | 27 +++++++++++--------
>> >  1 file changed, 16 insertions(+), 11 deletions(-)
>> >
>> > diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c b/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
>> > index 2ad33559a33a..e4cc1fb34e94 100644
>> > --- a/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
>> > +++ b/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
>> > @@ -111,6 +111,7 @@ komeda_crtc_atomic_check(struct drm_crtc *crtc,
>> >  static int
>> >  komeda_crtc_prepare(struct komeda_crtc *kcrtc)
>> >  {
>> > +     struct drm_device *drm = kcrtc->base.dev;
>> >       struct komeda_dev *mdev = kcrtc->base.dev->dev_private;
>> >       struct komeda_pipeline *master = kcrtc->master;
>> >       struct komeda_crtc_state *kcrtc_st = to_kcrtc_st(kcrtc->base.state);
>> > @@ -128,7 +129,7 @@ komeda_crtc_prepare(struct komeda_crtc *kcrtc)
>> >
>> >       err = mdev->funcs->change_opmode(mdev, new_mode);
>> >       if (err) {
>> > -             DRM_ERROR("failed to change opmode: 0x%x -> 0x%x.\n,",
>> > +             drm_err(drm, "failed to change opmode: 0x%x -> 0x%x.\n,",
>> >                         mdev->dpmode, new_mode);
>> >               goto unlock;
>> >       }
>> > @@ -142,18 +143,18 @@ komeda_crtc_prepare(struct komeda_crtc *kcrtc)
>> >       if (new_mode != KOMEDA_MODE_DUAL_DISP) {
>> >               err = clk_set_rate(mdev->aclk, komeda_crtc_get_aclk(kcrtc_st));
>> >               if (err)
>> > -                     DRM_ERROR("failed to set aclk.\n");
>> > +                     drm_err(drm, "failed to set aclk.\n");
>> >               err = clk_prepare_enable(mdev->aclk);
>> >               if (err)
>> > -                     DRM_ERROR("failed to enable aclk.\n");
>> > +                     drm_err(drm, "failed to enable aclk.\n");
>> >       }
>> >
>> >       err = clk_set_rate(master->pxlclk, mode->crtc_clock * 1000);
>> >       if (err)
>> > -             DRM_ERROR("failed to set pxlclk for pipe%d\n", master->id);
>> > +             drm_err(drm, "failed to set pxlclk for pipe%d\n", master->id);
>> >       err = clk_prepare_enable(master->pxlclk);
>> >       if (err)
>> > -             DRM_ERROR("failed to enable pxl clk for pipe%d.\n", master->id);
>> > +             drm_err(drm, "failed to enable pxl clk for pipe%d.\n", master->id);
>> >
>> >  unlock:
>> >       mutex_unlock(&mdev->lock);
>> > @@ -164,6 +165,7 @@ komeda_crtc_prepare(struct komeda_crtc *kcrtc)
>> >  static int
>> >  komeda_crtc_unprepare(struct komeda_crtc *kcrtc)
>> >  {
>> > +     struct drm_device *drm = kcrtc->base.dev;
>> >       struct komeda_dev *mdev = kcrtc->base.dev->dev_private;
>> >       struct komeda_pipeline *master = kcrtc->master;
>> >       u32 new_mode;
>> > @@ -180,7 +182,7 @@ komeda_crtc_unprepare(struct komeda_crtc *kcrtc)
>> >
>> >       err = mdev->funcs->change_opmode(mdev, new_mode);
>> >       if (err) {
>> > -             DRM_ERROR("failed to change opmode: 0x%x -> 0x%x.\n,",
>> > +             drm_err(drm, "failed to change opmode: 0x%x -> 0x%x.\n,",
>> >                         mdev->dpmode, new_mode);
>> >               goto unlock;
>> >       }
>> > @@ -200,6 +202,7 @@ komeda_crtc_unprepare(struct komeda_crtc *kcrtc)
>> >  void komeda_crtc_handle_event(struct komeda_crtc   *kcrtc,
>> >                             struct komeda_events *evts)
>> >  {
>> > +     struct drm_device *drm = kcrtc->base.dev;
>> >       struct drm_crtc *crtc = &kcrtc->base;
>> >       u32 events = evts->pipes[kcrtc->master->id];
>> >
>> > @@ -212,7 +215,7 @@ void komeda_crtc_handle_event(struct komeda_crtc   *kcrtc,
>> >               if (wb_conn)
>> >                       drm_writeback_signal_completion(&wb_conn->base, 0);
>> >               else
>> > -                     DRM_WARN("CRTC[%d]: EOW happen but no wb_connector.\n",
>> > +                     drm_warn(drm, "CRTC[%d]: EOW happen but no wb_connector.\n",
>> >                                drm_crtc_index(&kcrtc->base));
>> >       }
>> >       /* will handle it together with the write back support */
>> > @@ -236,7 +239,7 @@ void komeda_crtc_handle_event(struct komeda_crtc   *kcrtc,
>> >                       crtc->state->event = NULL;
>> >                       drm_crtc_send_vblank_event(crtc, event);
>> >               } else {
>> > -                     DRM_WARN("CRTC[%d]: FLIP happened but no pending commit.\n",
>> > +                     drm_warn(drm, "CRTC[%d]: FLIP happened but no pending commit.\n",
>> >                                drm_crtc_index(&kcrtc->base));
>> >               }
>> >               spin_unlock_irqrestore(&crtc->dev->event_lock, flags);
>> > @@ -309,7 +312,7 @@ komeda_crtc_flush_and_wait_for_flip_done(struct komeda_crtc *kcrtc,
>> >
>> >       /* wait the flip take affect.*/
>> >       if (wait_for_completion_timeout(flip_done, HZ) == 0) {
>> > -             DRM_ERROR("wait pipe%d flip done timeout\n", kcrtc->master->id);
>> > +             drm_err(drm, "wait pipe%d flip done timeout\n", kcrtc->master->id);
>> >               if (!input_flip_done) {
>> >                       unsigned long flags;
>> >
>> > @@ -562,6 +565,7 @@ static const struct drm_crtc_funcs komeda_crtc_funcs = {
>> >  int komeda_kms_setup_crtcs(struct komeda_kms_dev *kms,
>> >                          struct komeda_dev *mdev)
>> >  {
>> > +     struct drm_device *drm = &kms->base;
>> >       struct komeda_crtc *crtc;
>> >       struct komeda_pipeline *master;
>> >       char str[16];
>> > @@ -581,7 +585,7 @@ int komeda_kms_setup_crtcs(struct komeda_kms_dev *kms,
>> >               else
>> >                       sprintf(str, "None");
>> >
>> > -             DRM_INFO("CRTC-%d: master(pipe-%d) slave(%s).\n",
>> > +             drm_info(drm, "CRTC-%d: master(pipe-%d) slave(%s).\n",
>> >                        kms->n_crtcs, master->id, str);
>> >
>> >               kms->n_crtcs++;
>> > @@ -613,6 +617,7 @@ static int komeda_attach_bridge(struct device *dev,
>> >                               struct komeda_pipeline *pipe,
>> >                               struct drm_encoder *encoder)
>> >  {
>> > +     struct drm_device *drm = encoder->dev;
>> >       struct drm_bridge *bridge;
>> >       int err;
>> >
>> > @@ -624,7 +629,7 @@ static int komeda_attach_bridge(struct device *dev,
>> >
>> >       err = drm_bridge_attach(encoder, bridge, NULL, 0);
>> >       if (err)
>> > -             dev_err(dev, "bridge_attach() failed for pipe: %s\n",
>> > +             drm_err(drm, "bridge_attach() failed for pipe: %s\n",
>> >                       of_node_full_name(pipe->of_node));
>> >
>> >       return err;
>> > --
>> > 2.43.0
>> >
>>
>> --
>> ====================
>> | I would like to |
>> | fix the world,  |
>> | but they're not |
>> | giving me the   |
>>  \ source code!  /
>>   ---------------
>>     ¯\_(ツ)_/¯

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

* Re: [PATCH] drm/komeda: Convert logging in komeda_crtc.c to drm_* with drm_device parameter
  2025-08-12 11:12 ` Liviu Dudau
       [not found]   ` <CAKY2RyZX9D6J08S78zT97cHS3dJ7Cf0xURX7EougPmh-rMSMGQ@mail.gmail.com>
@ 2025-09-18 15:10   ` Rahul Kumar
  1 sibling, 0 replies; 7+ messages in thread
From: Rahul Kumar @ 2025-09-18 15:10 UTC (permalink / raw)
  To: Liviu Dudau
  Cc: maarten.lankhorst, mripard, tzimmermann, airlied, simona,
	dri-devel, linux-kernel, linux-kernel-mentees, skhan

Hi Liviu,

On Tue, Aug 12, 2025 at 4:42 PM Liviu Dudau <liviu.dudau@arm.com> wrote:
>
> On Tue, Aug 12, 2025 at 03:41:19PM +0530, Rahul Kumar wrote:
> > Replace all dev_err(), dev_warn(), dev_info() and DRM_ERROR/WARN/INFO()
> > calls in drivers/gpu/drm/arm/display/komeda/komeda_crtc.c with the
> > corresponding drm_err(), drm_warn(), and drm_info() helpers.
> >
> > The new drm_*() logging functions take a struct drm_device * as the
> > first argument. This allows the DRM core to prefix log messages with
> > the specific DRM device name and instance, which is essential for
> > distinguishing logs when multiple GPUs or display controllers are present.
> >
> > This change aligns komeda with the DRM TODO item: "Convert logging to
> > drm_* functions with drm_device parameter".
> >
> > Signed-off-by: Rahul Kumar <rk0006818@gmail.com>
>
> Reviewed-by: Liviu Dudau <liviu.dudau@arm.com>
>

I just wanted to check if there’s anything else I should do for this
patch, or if it’s already queued for merging.

Thanks,
Rahul

> For future patches, when you make changes, please generate them with
>
> git patch -v<version>
>
> and include a change log so that reviewers can quickly figure out what
> has changed between emails without having to go back and forth.
>
> Best regards,
> Liviu
>
> > ---
> >  .../gpu/drm/arm/display/komeda/komeda_crtc.c  | 27 +++++++++++--------
> >  1 file changed, 16 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c b/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
> > index 2ad33559a33a..e4cc1fb34e94 100644
> > --- a/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
> > +++ b/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
> > @@ -111,6 +111,7 @@ komeda_crtc_atomic_check(struct drm_crtc *crtc,
> >  static int
> >  komeda_crtc_prepare(struct komeda_crtc *kcrtc)
> >  {
> > +     struct drm_device *drm = kcrtc->base.dev;
> >       struct komeda_dev *mdev = kcrtc->base.dev->dev_private;
> >       struct komeda_pipeline *master = kcrtc->master;
> >       struct komeda_crtc_state *kcrtc_st = to_kcrtc_st(kcrtc->base.state);
> > @@ -128,7 +129,7 @@ komeda_crtc_prepare(struct komeda_crtc *kcrtc)
> >
> >       err = mdev->funcs->change_opmode(mdev, new_mode);
> >       if (err) {
> > -             DRM_ERROR("failed to change opmode: 0x%x -> 0x%x.\n,",
> > +             drm_err(drm, "failed to change opmode: 0x%x -> 0x%x.\n,",
> >                         mdev->dpmode, new_mode);
> >               goto unlock;
> >       }
> > @@ -142,18 +143,18 @@ komeda_crtc_prepare(struct komeda_crtc *kcrtc)
> >       if (new_mode != KOMEDA_MODE_DUAL_DISP) {
> >               err = clk_set_rate(mdev->aclk, komeda_crtc_get_aclk(kcrtc_st));
> >               if (err)
> > -                     DRM_ERROR("failed to set aclk.\n");
> > +                     drm_err(drm, "failed to set aclk.\n");
> >               err = clk_prepare_enable(mdev->aclk);
> >               if (err)
> > -                     DRM_ERROR("failed to enable aclk.\n");
> > +                     drm_err(drm, "failed to enable aclk.\n");
> >       }
> >
> >       err = clk_set_rate(master->pxlclk, mode->crtc_clock * 1000);
> >       if (err)
> > -             DRM_ERROR("failed to set pxlclk for pipe%d\n", master->id);
> > +             drm_err(drm, "failed to set pxlclk for pipe%d\n", master->id);
> >       err = clk_prepare_enable(master->pxlclk);
> >       if (err)
> > -             DRM_ERROR("failed to enable pxl clk for pipe%d.\n", master->id);
> > +             drm_err(drm, "failed to enable pxl clk for pipe%d.\n", master->id);
> >
> >  unlock:
> >       mutex_unlock(&mdev->lock);
> > @@ -164,6 +165,7 @@ komeda_crtc_prepare(struct komeda_crtc *kcrtc)
> >  static int
> >  komeda_crtc_unprepare(struct komeda_crtc *kcrtc)
> >  {
> > +     struct drm_device *drm = kcrtc->base.dev;
> >       struct komeda_dev *mdev = kcrtc->base.dev->dev_private;
> >       struct komeda_pipeline *master = kcrtc->master;
> >       u32 new_mode;
> > @@ -180,7 +182,7 @@ komeda_crtc_unprepare(struct komeda_crtc *kcrtc)
> >
> >       err = mdev->funcs->change_opmode(mdev, new_mode);
> >       if (err) {
> > -             DRM_ERROR("failed to change opmode: 0x%x -> 0x%x.\n,",
> > +             drm_err(drm, "failed to change opmode: 0x%x -> 0x%x.\n,",
> >                         mdev->dpmode, new_mode);
> >               goto unlock;
> >       }
> > @@ -200,6 +202,7 @@ komeda_crtc_unprepare(struct komeda_crtc *kcrtc)
> >  void komeda_crtc_handle_event(struct komeda_crtc   *kcrtc,
> >                             struct komeda_events *evts)
> >  {
> > +     struct drm_device *drm = kcrtc->base.dev;
> >       struct drm_crtc *crtc = &kcrtc->base;
> >       u32 events = evts->pipes[kcrtc->master->id];
> >
> > @@ -212,7 +215,7 @@ void komeda_crtc_handle_event(struct komeda_crtc   *kcrtc,
> >               if (wb_conn)
> >                       drm_writeback_signal_completion(&wb_conn->base, 0);
> >               else
> > -                     DRM_WARN("CRTC[%d]: EOW happen but no wb_connector.\n",
> > +                     drm_warn(drm, "CRTC[%d]: EOW happen but no wb_connector.\n",
> >                                drm_crtc_index(&kcrtc->base));
> >       }
> >       /* will handle it together with the write back support */
> > @@ -236,7 +239,7 @@ void komeda_crtc_handle_event(struct komeda_crtc   *kcrtc,
> >                       crtc->state->event = NULL;
> >                       drm_crtc_send_vblank_event(crtc, event);
> >               } else {
> > -                     DRM_WARN("CRTC[%d]: FLIP happened but no pending commit.\n",
> > +                     drm_warn(drm, "CRTC[%d]: FLIP happened but no pending commit.\n",
> >                                drm_crtc_index(&kcrtc->base));
> >               }
> >               spin_unlock_irqrestore(&crtc->dev->event_lock, flags);
> > @@ -309,7 +312,7 @@ komeda_crtc_flush_and_wait_for_flip_done(struct komeda_crtc *kcrtc,
> >
> >       /* wait the flip take affect.*/
> >       if (wait_for_completion_timeout(flip_done, HZ) == 0) {
> > -             DRM_ERROR("wait pipe%d flip done timeout\n", kcrtc->master->id);
> > +             drm_err(drm, "wait pipe%d flip done timeout\n", kcrtc->master->id);
> >               if (!input_flip_done) {
> >                       unsigned long flags;
> >
> > @@ -562,6 +565,7 @@ static const struct drm_crtc_funcs komeda_crtc_funcs = {
> >  int komeda_kms_setup_crtcs(struct komeda_kms_dev *kms,
> >                          struct komeda_dev *mdev)
> >  {
> > +     struct drm_device *drm = &kms->base;
> >       struct komeda_crtc *crtc;
> >       struct komeda_pipeline *master;
> >       char str[16];
> > @@ -581,7 +585,7 @@ int komeda_kms_setup_crtcs(struct komeda_kms_dev *kms,
> >               else
> >                       sprintf(str, "None");
> >
> > -             DRM_INFO("CRTC-%d: master(pipe-%d) slave(%s).\n",
> > +             drm_info(drm, "CRTC-%d: master(pipe-%d) slave(%s).\n",
> >                        kms->n_crtcs, master->id, str);
> >
> >               kms->n_crtcs++;
> > @@ -613,6 +617,7 @@ static int komeda_attach_bridge(struct device *dev,
> >                               struct komeda_pipeline *pipe,
> >                               struct drm_encoder *encoder)
> >  {
> > +     struct drm_device *drm = encoder->dev;
> >       struct drm_bridge *bridge;
> >       int err;
> >
> > @@ -624,7 +629,7 @@ static int komeda_attach_bridge(struct device *dev,
> >
> >       err = drm_bridge_attach(encoder, bridge, NULL, 0);
> >       if (err)
> > -             dev_err(dev, "bridge_attach() failed for pipe: %s\n",
> > +             drm_err(drm, "bridge_attach() failed for pipe: %s\n",
> >                       of_node_full_name(pipe->of_node));
> >
> >       return err;
> > --
> > 2.43.0
> >
>
> --
> ====================
> | I would like to |
> | fix the world,  |
> | but they're not |
> | giving me the   |
>  \ source code!  /
>   ---------------
>     ¯\_(ツ)_/¯

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

end of thread, other threads:[~2025-09-18 15:10 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-11  5:44 [PATCH] drm/komeda: Convert logging in komeda_crtc.c to drm_* with drm_device parameter Rahul Kumar
2025-08-11  7:36 ` kernel test robot
2025-08-11  9:59 ` Liviu Dudau
  -- strict thread matches above, loose matches on Subject: below --
2025-08-12 10:11 Rahul Kumar
2025-08-12 11:12 ` Liviu Dudau
     [not found]   ` <CAKY2RyZX9D6J08S78zT97cHS3dJ7Cf0xURX7EougPmh-rMSMGQ@mail.gmail.com>
2025-08-12 13:09     ` Rahul Kumar
2025-09-18 15:10   ` Rahul Kumar

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