[PATCH v2 0/3] drm/komeda: Convert logging to drm_* helpers

[PATCH v2 0/3] drm/komeda: Convert logging to drm_* helpers

[Rahul Kumar]

This series converts Komeda logging from the legacy DRM_ERROR/WARN/INFO()
and DRM_DEBUG() macros to the modern drm_*() helpers. The drm_*() helpers
take a struct drm_device * and allow the DRM core to include device
information in the log output. This improves readability and brings the
driver in line with current DRM logging guidelines.

To support this conversion, a small Komeda internal header
(komeda_drv.h) is introduced to provide access to struct komeda_drv
where needed. No functional changes are intended.

Changes in v2:
- Corrected the use of dev_get_drvdata(); it returns struct komeda_drv *,
  not struct komeda_kms_dev *.
- Added komeda_drv.h to make struct komeda_drv available for logging
  conversion.
- Split the series into 3 small patches as requested.

Rahul Kumar (3):
  drm/komeda: Add komeda_drv.h to make struct komeda_drv available
  drm/komeda: Convert logging in komeda_pipeline.c to drm_* with
  drm_device parameter
  drm/komeda: Convert logging in d71_dev.c to drm_* with drm_device
  parameter

 .../gpu/drm/arm/display/komeda/d71/d71_dev.c  | 24 +++++---
 .../gpu/drm/arm/display/komeda/komeda_drv.c   |  6 +-
 .../gpu/drm/arm/display/komeda/komeda_drv.h   | 24 ++++++++
 .../drm/arm/display/komeda/komeda_pipeline.c  | 61 +++++++++++++------
 4 files changed, 84 insertions(+), 31 deletions(-)
 create mode 100644 drivers/gpu/drm/arm/display/komeda/komeda_drv.h

-- 
2.43.0

[PATCH v2 1/3] drm/komeda: Add komeda_drv.h to make struct komeda_drv available

[Rahul Kumar]

struct komeda_drv was defined only inside komeda_drv.c and not visible
to other Komeda components. Add a new header file, komeda_drv.h, to
make this structure available across the driver.

Signed-off-by: Rahul Kumar <rk0006818@gmail.com>
---
 .../gpu/drm/arm/display/komeda/komeda_drv.c   |  6 +----
 .../gpu/drm/arm/display/komeda/komeda_drv.h   | 24 +++++++++++++++++++
 2 files changed, 25 insertions(+), 5 deletions(-)
 create mode 100644 drivers/gpu/drm/arm/display/komeda/komeda_drv.h

diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_drv.c b/drivers/gpu/drm/arm/display/komeda/komeda_drv.c
index 358c1512b087..aff3cf1f134b 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_drv.c
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_drv.c
@@ -14,11 +14,7 @@
 #include <drm/drm_of.h>
 #include "komeda_dev.h"
 #include "komeda_kms.h"
-
-struct komeda_drv {
-	struct komeda_dev *mdev;
-	struct komeda_kms_dev *kms;
-};
+#include "komeda_drv.h"
 
 struct komeda_dev *dev_to_mdev(struct device *dev)
 {
diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_drv.h b/drivers/gpu/drm/arm/display/komeda/komeda_drv.h
new file mode 100644
index 000000000000..b105255a798c
--- /dev/null
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_drv.h
@@ -0,0 +1,24 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Komeda top-level driver structure
+ *
+ * (C) COPYRIGHT 2025 Rahul Kumar <rk0006818@gmail.com>
+ */
+#ifndef _KOMEDA_DRV_H_
+#define _KOMEDA_DRV_H_
+
+#include "komeda_dev.h"
+#include "komeda_kms.h"
+
+/**
+ * struct komeda_drv - Komeda high-level driver glue
+ *
+ * This structure links the core Komeda hardware device (struct komeda_dev)
+ * with the DRM/KMS integration layer (struct komeda_kms_dev).
+ */
+struct komeda_drv {
+	struct komeda_dev     *mdev;
+	struct komeda_kms_dev *kms;
+};
+
+#endif /* !_KOMEDA_DRV_H_ */
-- 
2.43.0

[PATCH v2 2/3] drm/komeda: Convert logging in komeda_pipeline.c to drm_* with drm_device parameter

[Rahul Kumar]

Replace DRM_ERROR/WARN/INFO() and DRM_DEBUG() calls in
drivers/gpu/drm/arm/display/komeda/komeda_pipeline.c with the
drm_err(), drm_warn(), drm_info() and drm_dbg() helpers.

The drm_*() logging macros require a struct drm_device * parameter,
which allows the DRM core to prefix log messages with the device
instance. This is important for distinguishing logs when multiple
Komeda or other DRM devices are present.

This conversion follows the DRM TODO entry:
"Convert logging to drm_* functions with drm_device parameter".

Signed-off-by: Rahul Kumar <rk0006818@gmail.com>
---
Changes since v1:
- Fixed incorrect use of dev_get_drvdata(): it returns struct komeda_drv *,
  not struct komeda_kms_dev *, as pointed out by Liviu Dudau.
- Updated DRM device pointer retrieval to use drv->kms.
- Combined both changes into a 0/3 series.
Link to v1:
https://lore.kernel.org/all/aRcKzbV_cKbC6vlV@e110455-lin.cambridge.arm.com/
---
 .../drm/arm/display/komeda/komeda_pipeline.c  | 61 +++++++++++++------
 1 file changed, 42 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.c b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.c
index 81e244f0c0ca..0d1f870c31d9 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.c
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.c
@@ -12,22 +12,26 @@
 #include "komeda_dev.h"
 #include "komeda_kms.h"
 #include "komeda_pipeline.h"
+#include "komeda_drv.h"
 
 /** komeda_pipeline_add - Add a pipeline to &komeda_dev */
 struct komeda_pipeline *
 komeda_pipeline_add(struct komeda_dev *mdev, size_t size,
 		    const struct komeda_pipeline_funcs *funcs)
 {
+	struct komeda_drv *drv = dev_get_drvdata(mdev->dev);
+	struct komeda_kms_dev *kms = drv->kms;
+	struct drm_device *drm = &kms->base;
 	struct komeda_pipeline *pipe;
 
 	if (mdev->n_pipelines + 1 > KOMEDA_MAX_PIPELINES) {
-		DRM_ERROR("Exceed max support %d pipelines.\n",
-			  KOMEDA_MAX_PIPELINES);
+		drm_err(drm, "Exceed max support %d pipelines.\n",
+			KOMEDA_MAX_PIPELINES);
 		return ERR_PTR(-ENOSPC);
 	}
 
 	if (size < sizeof(*pipe)) {
-		DRM_ERROR("Request pipeline size too small.\n");
+		drm_err(drm, "Request pipeline size too small.\n");
 		return ERR_PTR(-EINVAL);
 	}
 
@@ -71,6 +75,9 @@ static struct komeda_component **
 komeda_pipeline_get_component_pos(struct komeda_pipeline *pipe, int id)
 {
 	struct komeda_dev *mdev = pipe->mdev;
+	struct komeda_drv *drv = dev_get_drvdata(mdev->dev);
+	struct komeda_kms_dev *kms = drv->kms;
+	struct drm_device *drm = &kms->base;
 	struct komeda_pipeline *temp = NULL;
 	struct komeda_component **pos = NULL;
 
@@ -88,7 +95,7 @@ komeda_pipeline_get_component_pos(struct komeda_pipeline *pipe, int id)
 	case KOMEDA_COMPONENT_COMPIZ1:
 		temp = mdev->pipelines[id - KOMEDA_COMPONENT_COMPIZ0];
 		if (!temp) {
-			DRM_ERROR("compiz-%d doesn't exist.\n", id);
+			drm_err(drm, "compiz-%d doesn't exist.\n", id);
 			return NULL;
 		}
 		pos = to_cpos(temp->compiz);
@@ -107,7 +114,7 @@ komeda_pipeline_get_component_pos(struct komeda_pipeline *pipe, int id)
 	case KOMEDA_COMPONENT_IPS1:
 		temp = mdev->pipelines[id - KOMEDA_COMPONENT_IPS0];
 		if (!temp) {
-			DRM_ERROR("ips-%d doesn't exist.\n", id);
+			drm_err(drm, "ips-%d doesn't exist.\n", id);
 			return NULL;
 		}
 		pos = to_cpos(temp->improc);
@@ -117,7 +124,7 @@ komeda_pipeline_get_component_pos(struct komeda_pipeline *pipe, int id)
 		break;
 	default:
 		pos = NULL;
-		DRM_ERROR("Unknown pipeline resource ID: %d.\n", id);
+		drm_err(drm, "Unknown pipeline resource ID: %d.\n", id);
 		break;
 	}
 
@@ -169,6 +176,9 @@ komeda_component_add(struct komeda_pipeline *pipe,
 		     u8 max_active_outputs, u32 __iomem *reg,
 		     const char *name_fmt, ...)
 {
+	struct komeda_drv *drv = dev_get_drvdata(pipe->mdev->dev);
+	struct komeda_kms_dev *kms = drv->kms;
+	struct drm_device *drm = &kms->base;
 	struct komeda_component **pos;
 	struct komeda_component *c;
 	int idx, *num = NULL;
@@ -187,14 +197,14 @@ komeda_component_add(struct komeda_pipeline *pipe,
 		idx = id - KOMEDA_COMPONENT_LAYER0;
 		num = &pipe->n_layers;
 		if (idx != pipe->n_layers) {
-			DRM_ERROR("please add Layer by id sequence.\n");
+			drm_err(drm, "please add Layer by id sequence.\n");
 			return ERR_PTR(-EINVAL);
 		}
 	} else if (has_bit(id,  KOMEDA_PIPELINE_SCALERS)) {
 		idx = id - KOMEDA_COMPONENT_SCALER0;
 		num = &pipe->n_scalers;
 		if (idx != pipe->n_scalers) {
-			DRM_ERROR("please add Scaler by id sequence.\n");
+			drm_err(drm, "please add Scaler by id sequence.\n");
 			return ERR_PTR(-EINVAL);
 		}
 	}
@@ -240,27 +250,34 @@ static void komeda_component_dump(struct komeda_component *c)
 	if (!c)
 		return;
 
-	DRM_DEBUG("	%s: ID %d-0x%08lx.\n",
-		  c->name, c->id, BIT(c->id));
-	DRM_DEBUG("		max_active_inputs:%d, supported_inputs: 0x%08x.\n",
-		  c->max_active_inputs, c->supported_inputs);
-	DRM_DEBUG("		max_active_outputs:%d, supported_outputs: 0x%08x.\n",
-		  c->max_active_outputs, c->supported_outputs);
+	struct komeda_drv *drv = dev_get_drvdata(c->pipeline->mdev->dev);
+	struct komeda_kms_dev *kms = drv->kms;
+	struct drm_device *drm = &kms->base;
+
+	drm_dbg(drm, "	%s: ID %d-0x%08lx.\n",
+		c->name, c->id, BIT(c->id));
+	drm_dbg(drm, "		max_active_inputs:%d, supported_inputs: 0x%08x.\n",
+		c->max_active_inputs, c->supported_inputs);
+	drm_dbg(drm, "		max_active_outputs:%d, supported_outputs: 0x%08x.\n",
+		c->max_active_outputs, c->supported_outputs);
 }
 
 void komeda_pipeline_dump(struct komeda_pipeline *pipe)
 {
+	struct komeda_drv *drv = dev_get_drvdata(pipe->mdev->dev);
+	struct komeda_kms_dev *kms = drv->kms;
+	struct drm_device *drm = &kms->base;
 	struct komeda_component *c;
 	int id;
 	unsigned long avail_comps = pipe->avail_comps;
 
-	DRM_INFO("Pipeline-%d: n_layers: %d, n_scalers: %d, output: %s.\n",
+	drm_info(drm, "Pipeline-%d: n_layers: %d, n_scalers: %d, output: %s.\n",
 		 pipe->id, pipe->n_layers, pipe->n_scalers,
 		 pipe->dual_link ? "dual-link" : "single-link");
-	DRM_INFO("	output_link[0]: %s.\n",
+	drm_info(drm, "	output_link[0]: %s.\n",
 		 pipe->of_output_links[0] ?
 		 pipe->of_output_links[0]->full_name : "none");
-	DRM_INFO("	output_link[1]: %s.\n",
+	drm_info(drm, "	output_link[1]: %s.\n",
 		 pipe->of_output_links[1] ?
 		 pipe->of_output_links[1]->full_name : "none");
 
@@ -274,6 +291,9 @@ void komeda_pipeline_dump(struct komeda_pipeline *pipe)
 static void komeda_component_verify_inputs(struct komeda_component *c)
 {
 	struct komeda_pipeline *pipe = c->pipeline;
+	struct komeda_drv *drv = dev_get_drvdata(pipe->mdev->dev);
+	struct komeda_kms_dev *kms = drv->kms;
+	struct drm_device *drm = &kms->base;
 	struct komeda_component *input;
 	int id;
 	unsigned long supported_inputs = c->supported_inputs;
@@ -282,7 +302,7 @@ static void komeda_component_verify_inputs(struct komeda_component *c)
 		input = komeda_pipeline_get_component(pipe, id);
 		if (!input) {
 			c->supported_inputs &= ~(BIT(id));
-			DRM_WARN("Can not find input(ID-%d) for component: %s.\n",
+			drm_warn(drm, "Can not find input(ID-%d) for component: %s.\n",
 				 id, c->name);
 			continue;
 		}
@@ -306,6 +326,9 @@ komeda_get_layer_split_right_layer(struct komeda_pipeline *pipe,
 
 static void komeda_pipeline_assemble(struct komeda_pipeline *pipe)
 {
+	struct komeda_drv *drv = dev_get_drvdata(pipe->mdev->dev);
+	struct komeda_kms_dev *kms = drv->kms;
+	struct drm_device *drm = &kms->base;
 	struct komeda_component *c;
 	struct komeda_layer *layer;
 	int i, id;
@@ -324,7 +347,7 @@ static void komeda_pipeline_assemble(struct komeda_pipeline *pipe)
 
 	if (pipe->dual_link && !pipe->ctrlr->supports_dual_link) {
 		pipe->dual_link = false;
-		DRM_WARN("PIPE-%d doesn't support dual-link, ignore DT dual-link configuration.\n",
+		drm_warn(drm, "PIPE-%d doesn't support dual-link, ignore DT dual-link configuration.\n",
 			 pipe->id);
 	}
 }
-- 
2.43.0

[PATCH v2 3/3] drm/komeda: Convert logging in d71_dev.c to drm_* with drm_device parameter

[Rahul Kumar]

Replace DRM_DEBUG() and DRM_ERROR() calls in
drivers/gpu/drm/arm/display/komeda/d71/d71_dev.c with the
drm_dbg() and drm_err() helpers in functions where a drm_device
parameter is available.

The drm_*() logging macros require a struct drm_device * parameter,
which allows the DRM core to prefix log messages with the device
instance. This improves debugging clarity when multiple Komeda or
other DRM devices are present.

Logging in early hardware probing functions such as d71_identify()
is intentionally left unchanged because they do not have access to
a drm_device pointer at that stage of initialization.

This conversion follows the DRM TODO entry:
"Convert logging to drm_* functions with drm_device parameter".

Signed-off-by: Rahul Kumar <rk0006818@gmail.com>
---
Changes since v1:
- Fixed incorrect use of dev_get_drvdata(): it returns struct komeda_drv *,
  not struct komeda_kms_dev *, as pointed out by Liviu Dudau.
- Updated DRM device pointer retrieval to use drv->kms.
- Combined both changes into a 0/3 series.
Link to v1:
https://lore.kernel.org/all/aRdT1qscQqO7-U6h@e110455-lin.cambridge.arm.com/
---
 .../gpu/drm/arm/display/komeda/d71/d71_dev.c  | 24 +++++++++++++------
 1 file changed, 17 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/arm/display/komeda/d71/d71_dev.c b/drivers/gpu/drm/arm/display/komeda/d71/d71_dev.c
index 80973975bfdb..39c51bbe2bb9 100644
--- a/drivers/gpu/drm/arm/display/komeda/d71/d71_dev.c
+++ b/drivers/gpu/drm/arm/display/komeda/d71/d71_dev.c
@@ -9,6 +9,7 @@
 #include <drm/drm_print.h>
 #include "d71_dev.h"
 #include "malidp_io.h"
+#include "komeda_drv.h"
 
 static u64 get_lpu_event(struct d71_pipeline *d71_pipeline)
 {
@@ -348,6 +349,9 @@ static void d71_cleanup(struct komeda_dev *mdev)
 
 static int d71_enum_resources(struct komeda_dev *mdev)
 {
+	struct komeda_drv *drv = dev_get_drvdata(mdev->dev);
+	struct komeda_kms_dev *kms = drv->kms;
+	struct drm_device *drm = &kms->base;
 	struct d71_dev *d71;
 	struct komeda_pipeline *pipe;
 	struct block_header blk;
@@ -366,7 +370,7 @@ static int d71_enum_resources(struct komeda_dev *mdev)
 
 	err = d71_reset(d71);
 	if (err) {
-		DRM_ERROR("Fail to reset d71 device.\n");
+		drm_err(drm, "Fail to reset d71 device.\n");
 		goto err_cleanup;
 	}
 
@@ -376,8 +380,8 @@ static int d71_enum_resources(struct komeda_dev *mdev)
 	d71->num_pipelines = (value >> 8) & 0x7;
 
 	if (d71->num_pipelines > D71_MAX_PIPELINE) {
-		DRM_ERROR("d71 supports %d pipelines, but got: %d.\n",
-			  D71_MAX_PIPELINE, d71->num_pipelines);
+		drm_err(drm, "d71 supports %d pipelines, but got: %d.\n",
+			D71_MAX_PIPELINE, d71->num_pipelines);
 		err = -EINVAL;
 		goto err_cleanup;
 	}
@@ -455,8 +459,8 @@ static int d71_enum_resources(struct komeda_dev *mdev)
 		offset += D71_BLOCK_SIZE;
 	}
 
-	DRM_DEBUG("total %d (out of %d) blocks are found.\n",
-		  i, d71->num_blocks);
+	drm_dbg(drm, "total %d (out of %d) blocks are found.\n",
+		i, d71->num_blocks);
 
 	return 0;
 
@@ -555,6 +559,9 @@ static void d71_init_fmt_tbl(struct komeda_dev *mdev)
 
 static int d71_connect_iommu(struct komeda_dev *mdev)
 {
+	struct komeda_drv *drv = dev_get_drvdata(mdev->dev);
+	struct komeda_kms_dev *kms = drv->kms;
+	struct drm_device *drm = &kms->base;
 	struct d71_dev *d71 = mdev->chip_data;
 	u32 __iomem *reg = d71->gcu_addr;
 	u32 check_bits = (d71->num_pipelines == 2) ?
@@ -569,7 +576,7 @@ static int d71_connect_iommu(struct komeda_dev *mdev)
 	ret = dp_wait_cond(has_bits(check_bits, malidp_read32(reg, BLK_STATUS)),
 			100, 1000, 1000);
 	if (ret < 0) {
-		DRM_ERROR("timed out connecting to TCU!\n");
+		drm_err(drm, "timed out connecting to TCU!\n");
 		malidp_write32_mask(reg, BLK_CONTROL, 0x7, INACTIVE_MODE);
 		return ret;
 	}
@@ -582,6 +589,9 @@ static int d71_connect_iommu(struct komeda_dev *mdev)
 
 static int d71_disconnect_iommu(struct komeda_dev *mdev)
 {
+	struct komeda_drv *drv = dev_get_drvdata(mdev->dev);
+	struct komeda_kms_dev *kms = drv->kms;
+	struct drm_device *drm = &kms->base;
 	struct d71_dev *d71 = mdev->chip_data;
 	u32 __iomem *reg = d71->gcu_addr;
 	u32 check_bits = (d71->num_pipelines == 2) ?
@@ -593,7 +603,7 @@ static int d71_disconnect_iommu(struct komeda_dev *mdev)
 	ret = dp_wait_cond(((malidp_read32(reg, BLK_STATUS) & check_bits) == 0),
 			100, 1000, 1000);
 	if (ret < 0) {
-		DRM_ERROR("timed out disconnecting from TCU!\n");
+		drm_err(drm, "timed out disconnecting from TCU!\n");
 		malidp_write32_mask(reg, BLK_CONTROL, 0x7, INACTIVE_MODE);
 	}
 
-- 
2.43.0

Re: [PATCH v2 3/3] drm/komeda: Convert logging in d71_dev.c to drm_* with drm_device parameter

[Rahul Kumar]

Hi Liviu,

On Tue, Nov 18, 2025 at 4:32 PM Rahul Kumar <rk0006818@gmail.com> wrote:
>
> Replace DRM_DEBUG() and DRM_ERROR() calls in
> drivers/gpu/drm/arm/display/komeda/d71/d71_dev.c with the
> drm_dbg() and drm_err() helpers in functions where a drm_device
> parameter is available.
>
> The drm_*() logging macros require a struct drm_device * parameter,
> which allows the DRM core to prefix log messages with the device
> instance. This improves debugging clarity when multiple Komeda or
> other DRM devices are present.
>
> Logging in early hardware probing functions such as d71_identify()
> is intentionally left unchanged because they do not have access to
> a drm_device pointer at that stage of initialization.
>
> This conversion follows the DRM TODO entry:
> "Convert logging to drm_* functions with drm_device parameter".
>
> Signed-off-by: Rahul Kumar <rk0006818@gmail.com>
> ---
> Changes since v1:
> - Fixed incorrect use of dev_get_drvdata(): it returns struct komeda_drv *,
>   not struct komeda_kms_dev *, as pointed out by Liviu Dudau.
> - Updated DRM device pointer retrieval to use drv->kms.
> - Combined both changes into a 0/3 series.
> Link to v1:
> https://lore.kernel.org/all/aRdT1qscQqO7-U6h@e110455-lin.cambridge.arm.com/
> ---
>  .../gpu/drm/arm/display/komeda/d71/d71_dev.c  | 24 +++++++++++++------
>  1 file changed, 17 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/arm/display/komeda/d71/d71_dev.c b/drivers/gpu/drm/arm/display/komeda/d71/d71_dev.c
> index 80973975bfdb..39c51bbe2bb9 100644
> --- a/drivers/gpu/drm/arm/display/komeda/d71/d71_dev.c
> +++ b/drivers/gpu/drm/arm/display/komeda/d71/d71_dev.c
> @@ -9,6 +9,7 @@
>  #include <drm/drm_print.h>
>  #include "d71_dev.h"
>  #include "malidp_io.h"
> +#include "komeda_drv.h"
>
>  static u64 get_lpu_event(struct d71_pipeline *d71_pipeline)
>  {
> @@ -348,6 +349,9 @@ static void d71_cleanup(struct komeda_dev *mdev)
>
>  static int d71_enum_resources(struct komeda_dev *mdev)
>  {
> +       struct komeda_drv *drv = dev_get_drvdata(mdev->dev);
> +       struct komeda_kms_dev *kms = drv->kms;
> +       struct drm_device *drm = &kms->base;
>         struct d71_dev *d71;
>         struct komeda_pipeline *pipe;
>         struct block_header blk;
> @@ -366,7 +370,7 @@ static int d71_enum_resources(struct komeda_dev *mdev)
>
>         err = d71_reset(d71);
>         if (err) {
> -               DRM_ERROR("Fail to reset d71 device.\n");
> +               drm_err(drm, "Fail to reset d71 device.\n");
>                 goto err_cleanup;
>         }
>
> @@ -376,8 +380,8 @@ static int d71_enum_resources(struct komeda_dev *mdev)
>         d71->num_pipelines = (value >> 8) & 0x7;
>
>         if (d71->num_pipelines > D71_MAX_PIPELINE) {
> -               DRM_ERROR("d71 supports %d pipelines, but got: %d.\n",
> -                         D71_MAX_PIPELINE, d71->num_pipelines);
> +               drm_err(drm, "d71 supports %d pipelines, but got: %d.\n",
> +                       D71_MAX_PIPELINE, d71->num_pipelines);
>                 err = -EINVAL;
>                 goto err_cleanup;
>         }
> @@ -455,8 +459,8 @@ static int d71_enum_resources(struct komeda_dev *mdev)
>                 offset += D71_BLOCK_SIZE;
>         }
>
> -       DRM_DEBUG("total %d (out of %d) blocks are found.\n",
> -                 i, d71->num_blocks);
> +       drm_dbg(drm, "total %d (out of %d) blocks are found.\n",
> +               i, d71->num_blocks);
>
>         return 0;
>
> @@ -555,6 +559,9 @@ static void d71_init_fmt_tbl(struct komeda_dev *mdev)
>
>  static int d71_connect_iommu(struct komeda_dev *mdev)
>  {
> +       struct komeda_drv *drv = dev_get_drvdata(mdev->dev);
> +       struct komeda_kms_dev *kms = drv->kms;
> +       struct drm_device *drm = &kms->base;
>         struct d71_dev *d71 = mdev->chip_data;
>         u32 __iomem *reg = d71->gcu_addr;
>         u32 check_bits = (d71->num_pipelines == 2) ?
> @@ -569,7 +576,7 @@ static int d71_connect_iommu(struct komeda_dev *mdev)
>         ret = dp_wait_cond(has_bits(check_bits, malidp_read32(reg, BLK_STATUS)),
>                         100, 1000, 1000);
>         if (ret < 0) {
> -               DRM_ERROR("timed out connecting to TCU!\n");
> +               drm_err(drm, "timed out connecting to TCU!\n");
>                 malidp_write32_mask(reg, BLK_CONTROL, 0x7, INACTIVE_MODE);
>                 return ret;
>         }
> @@ -582,6 +589,9 @@ static int d71_connect_iommu(struct komeda_dev *mdev)
>
>  static int d71_disconnect_iommu(struct komeda_dev *mdev)
>  {
> +       struct komeda_drv *drv = dev_get_drvdata(mdev->dev);
> +       struct komeda_kms_dev *kms = drv->kms;
> +       struct drm_device *drm = &kms->base;
>         struct d71_dev *d71 = mdev->chip_data;
>         u32 __iomem *reg = d71->gcu_addr;
>         u32 check_bits = (d71->num_pipelines == 2) ?
> @@ -593,7 +603,7 @@ static int d71_disconnect_iommu(struct komeda_dev *mdev)
>         ret = dp_wait_cond(((malidp_read32(reg, BLK_STATUS) & check_bits) == 0),
>                         100, 1000, 1000);
>         if (ret < 0) {
> -               DRM_ERROR("timed out disconnecting from TCU!\n");
> +               drm_err(drm, "timed out disconnecting from TCU!\n");
>                 malidp_write32_mask(reg, BLK_CONTROL, 0x7, INACTIVE_MODE);
>         }
>
> --
> 2.43.0


Just following up on my v2 Komeda logging patch series.
Please let me know if any changes or suggestions are needed.

Thanks,
Rahul

Re: [PATCH v2 3/3] drm/komeda: Convert logging in d71_dev.c to drm_* with drm_device parameter

[Liviu Dudau]

On Tue, Nov 25, 2025 at 12:15:28PM +0530, Rahul Kumar wrote:
> Hi Liviu,

Hi Rahul,

Sorry, you caught me with your new series just before going on holiday so
did not started the review. Since coming back I had some other internal
issues to take care of, I will have a look at your series next week when
I should have more time for it.

Best regards,
Liviu

> 
> On Tue, Nov 18, 2025 at 4:32 PM Rahul Kumar <rk0006818@gmail.com> wrote:
> >
> > Replace DRM_DEBUG() and DRM_ERROR() calls in
> > drivers/gpu/drm/arm/display/komeda/d71/d71_dev.c with the
> > drm_dbg() and drm_err() helpers in functions where a drm_device
> > parameter is available.
> >
> > The drm_*() logging macros require a struct drm_device * parameter,
> > which allows the DRM core to prefix log messages with the device
> > instance. This improves debugging clarity when multiple Komeda or
> > other DRM devices are present.
> >
> > Logging in early hardware probing functions such as d71_identify()
> > is intentionally left unchanged because they do not have access to
> > a drm_device pointer at that stage of initialization.
> >
> > This conversion follows the DRM TODO entry:
> > "Convert logging to drm_* functions with drm_device parameter".
> >
> > Signed-off-by: Rahul Kumar <rk0006818@gmail.com>
> > ---
> > Changes since v1:
> > - Fixed incorrect use of dev_get_drvdata(): it returns struct komeda_drv *,
> >   not struct komeda_kms_dev *, as pointed out by Liviu Dudau.
> > - Updated DRM device pointer retrieval to use drv->kms.
> > - Combined both changes into a 0/3 series.
> > Link to v1:
> > https://lore.kernel.org/all/aRdT1qscQqO7-U6h@e110455-lin.cambridge.arm.com/
> > ---
> >  .../gpu/drm/arm/display/komeda/d71/d71_dev.c  | 24 +++++++++++++------
> >  1 file changed, 17 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/arm/display/komeda/d71/d71_dev.c b/drivers/gpu/drm/arm/display/komeda/d71/d71_dev.c
> > index 80973975bfdb..39c51bbe2bb9 100644
> > --- a/drivers/gpu/drm/arm/display/komeda/d71/d71_dev.c
> > +++ b/drivers/gpu/drm/arm/display/komeda/d71/d71_dev.c
> > @@ -9,6 +9,7 @@
> >  #include <drm/drm_print.h>
> >  #include "d71_dev.h"
> >  #include "malidp_io.h"
> > +#include "komeda_drv.h"
> >
> >  static u64 get_lpu_event(struct d71_pipeline *d71_pipeline)
> >  {
> > @@ -348,6 +349,9 @@ static void d71_cleanup(struct komeda_dev *mdev)
> >
> >  static int d71_enum_resources(struct komeda_dev *mdev)
> >  {
> > +       struct komeda_drv *drv = dev_get_drvdata(mdev->dev);
> > +       struct komeda_kms_dev *kms = drv->kms;
> > +       struct drm_device *drm = &kms->base;
> >         struct d71_dev *d71;
> >         struct komeda_pipeline *pipe;
> >         struct block_header blk;
> > @@ -366,7 +370,7 @@ static int d71_enum_resources(struct komeda_dev *mdev)
> >
> >         err = d71_reset(d71);
> >         if (err) {
> > -               DRM_ERROR("Fail to reset d71 device.\n");
> > +               drm_err(drm, "Fail to reset d71 device.\n");
> >                 goto err_cleanup;
> >         }
> >
> > @@ -376,8 +380,8 @@ static int d71_enum_resources(struct komeda_dev *mdev)
> >         d71->num_pipelines = (value >> 8) & 0x7;
> >
> >         if (d71->num_pipelines > D71_MAX_PIPELINE) {
> > -               DRM_ERROR("d71 supports %d pipelines, but got: %d.\n",
> > -                         D71_MAX_PIPELINE, d71->num_pipelines);
> > +               drm_err(drm, "d71 supports %d pipelines, but got: %d.\n",
> > +                       D71_MAX_PIPELINE, d71->num_pipelines);
> >                 err = -EINVAL;
> >                 goto err_cleanup;
> >         }
> > @@ -455,8 +459,8 @@ static int d71_enum_resources(struct komeda_dev *mdev)
> >                 offset += D71_BLOCK_SIZE;
> >         }
> >
> > -       DRM_DEBUG("total %d (out of %d) blocks are found.\n",
> > -                 i, d71->num_blocks);
> > +       drm_dbg(drm, "total %d (out of %d) blocks are found.\n",
> > +               i, d71->num_blocks);
> >
> >         return 0;
> >
> > @@ -555,6 +559,9 @@ static void d71_init_fmt_tbl(struct komeda_dev *mdev)
> >
> >  static int d71_connect_iommu(struct komeda_dev *mdev)
> >  {
> > +       struct komeda_drv *drv = dev_get_drvdata(mdev->dev);
> > +       struct komeda_kms_dev *kms = drv->kms;
> > +       struct drm_device *drm = &kms->base;
> >         struct d71_dev *d71 = mdev->chip_data;
> >         u32 __iomem *reg = d71->gcu_addr;
> >         u32 check_bits = (d71->num_pipelines == 2) ?
> > @@ -569,7 +576,7 @@ static int d71_connect_iommu(struct komeda_dev *mdev)
> >         ret = dp_wait_cond(has_bits(check_bits, malidp_read32(reg, BLK_STATUS)),
> >                         100, 1000, 1000);
> >         if (ret < 0) {
> > -               DRM_ERROR("timed out connecting to TCU!\n");
> > +               drm_err(drm, "timed out connecting to TCU!\n");
> >                 malidp_write32_mask(reg, BLK_CONTROL, 0x7, INACTIVE_MODE);
> >                 return ret;
> >         }
> > @@ -582,6 +589,9 @@ static int d71_connect_iommu(struct komeda_dev *mdev)
> >
> >  static int d71_disconnect_iommu(struct komeda_dev *mdev)
> >  {
> > +       struct komeda_drv *drv = dev_get_drvdata(mdev->dev);
> > +       struct komeda_kms_dev *kms = drv->kms;
> > +       struct drm_device *drm = &kms->base;
> >         struct d71_dev *d71 = mdev->chip_data;
> >         u32 __iomem *reg = d71->gcu_addr;
> >         u32 check_bits = (d71->num_pipelines == 2) ?
> > @@ -593,7 +603,7 @@ static int d71_disconnect_iommu(struct komeda_dev *mdev)
> >         ret = dp_wait_cond(((malidp_read32(reg, BLK_STATUS) & check_bits) == 0),
> >                         100, 1000, 1000);
> >         if (ret < 0) {
> > -               DRM_ERROR("timed out disconnecting from TCU!\n");
> > +               drm_err(drm, "timed out disconnecting from TCU!\n");
> >                 malidp_write32_mask(reg, BLK_CONTROL, 0x7, INACTIVE_MODE);
> >         }
> >
> > --
> > 2.43.0
> 
> 
> Just following up on my v2 Komeda logging patch series.
> Please let me know if any changes or suggestions are needed.
> 
> Thanks,
> Rahul

Re: [PATCH v2 3/3] drm/komeda: Convert logging in d71_dev.c to drm_* with drm_device parameter

[Rahul Kumar]

On Tue, Dec 2, 2025 at 7:04 PM Liviu Dudau <liviu.dudau@arm.com> wrote:
>
> On Tue, Nov 25, 2025 at 12:15:28PM +0530, Rahul Kumar wrote:
> > Hi Liviu,
>
> Hi Rahul,
>
> Sorry, you caught me with your new series just before going on holiday so
> did not started the review. Since coming back I had some other internal
> issues to take care of, I will have a look at your series next week when
> I should have more time for it.
>
> Best regards,
> Liviu
>

Hi Liviu,

Thanks for the update. No worries at all, I appreciate you letting me know.
Please take your time, and I'm happy to make any changes once you
review the series.

Best regards,
Rahul

> >
> > On Tue, Nov 18, 2025 at 4:32 PM Rahul Kumar <rk0006818@gmail.com> wrote:
> > >
> > > Replace DRM_DEBUG() and DRM_ERROR() calls in
> > > drivers/gpu/drm/arm/display/komeda/d71/d71_dev.c with the
> > > drm_dbg() and drm_err() helpers in functions where a drm_device
> > > parameter is available.
> > >
> > > The drm_*() logging macros require a struct drm_device * parameter,
> > > which allows the DRM core to prefix log messages with the device
> > > instance. This improves debugging clarity when multiple Komeda or
> > > other DRM devices are present.
> > >
> > > Logging in early hardware probing functions such as d71_identify()
> > > is intentionally left unchanged because they do not have access to
> > > a drm_device pointer at that stage of initialization.
> > >
> > > This conversion follows the DRM TODO entry:
> > > "Convert logging to drm_* functions with drm_device parameter".
> > >
> > > Signed-off-by: Rahul Kumar <rk0006818@gmail.com>
> > > ---
> > > Changes since v1:
> > > - Fixed incorrect use of dev_get_drvdata(): it returns struct komeda_drv *,
> > >   not struct komeda_kms_dev *, as pointed out by Liviu Dudau.
> > > - Updated DRM device pointer retrieval to use drv->kms.
> > > - Combined both changes into a 0/3 series.
> > > Link to v1:
> > > https://lore.kernel.org/all/aRdT1qscQqO7-U6h@e110455-lin.cambridge.arm.com/
> > > ---
> > >  .../gpu/drm/arm/display/komeda/d71/d71_dev.c  | 24 +++++++++++++------
> > >  1 file changed, 17 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/arm/display/komeda/d71/d71_dev.c b/drivers/gpu/drm/arm/display/komeda/d71/d71_dev.c
> > > index 80973975bfdb..39c51bbe2bb9 100644
> > > --- a/drivers/gpu/drm/arm/display/komeda/d71/d71_dev.c
> > > +++ b/drivers/gpu/drm/arm/display/komeda/d71/d71_dev.c
> > > @@ -9,6 +9,7 @@
> > >  #include <drm/drm_print.h>
> > >  #include "d71_dev.h"
> > >  #include "malidp_io.h"
> > > +#include "komeda_drv.h"
> > >
> > >  static u64 get_lpu_event(struct d71_pipeline *d71_pipeline)
> > >  {
> > > @@ -348,6 +349,9 @@ static void d71_cleanup(struct komeda_dev *mdev)
> > >
> > >  static int d71_enum_resources(struct komeda_dev *mdev)
> > >  {
> > > +       struct komeda_drv *drv = dev_get_drvdata(mdev->dev);
> > > +       struct komeda_kms_dev *kms = drv->kms;
> > > +       struct drm_device *drm = &kms->base;
> > >         struct d71_dev *d71;
> > >         struct komeda_pipeline *pipe;
> > >         struct block_header blk;
> > > @@ -366,7 +370,7 @@ static int d71_enum_resources(struct komeda_dev *mdev)
> > >
> > >         err = d71_reset(d71);
> > >         if (err) {
> > > -               DRM_ERROR("Fail to reset d71 device.\n");
> > > +               drm_err(drm, "Fail to reset d71 device.\n");
> > >                 goto err_cleanup;
> > >         }
> > >
> > > @@ -376,8 +380,8 @@ static int d71_enum_resources(struct komeda_dev *mdev)
> > >         d71->num_pipelines = (value >> 8) & 0x7;
> > >
> > >         if (d71->num_pipelines > D71_MAX_PIPELINE) {
> > > -               DRM_ERROR("d71 supports %d pipelines, but got: %d.\n",
> > > -                         D71_MAX_PIPELINE, d71->num_pipelines);
> > > +               drm_err(drm, "d71 supports %d pipelines, but got: %d.\n",
> > > +                       D71_MAX_PIPELINE, d71->num_pipelines);
> > >                 err = -EINVAL;
> > >                 goto err_cleanup;
> > >         }
> > > @@ -455,8 +459,8 @@ static int d71_enum_resources(struct komeda_dev *mdev)
> > >                 offset += D71_BLOCK_SIZE;
> > >         }
> > >
> > > -       DRM_DEBUG("total %d (out of %d) blocks are found.\n",
> > > -                 i, d71->num_blocks);
> > > +       drm_dbg(drm, "total %d (out of %d) blocks are found.\n",
> > > +               i, d71->num_blocks);
> > >
> > >         return 0;
> > >
> > > @@ -555,6 +559,9 @@ static void d71_init_fmt_tbl(struct komeda_dev *mdev)
> > >
> > >  static int d71_connect_iommu(struct komeda_dev *mdev)
> > >  {
> > > +       struct komeda_drv *drv = dev_get_drvdata(mdev->dev);
> > > +       struct komeda_kms_dev *kms = drv->kms;
> > > +       struct drm_device *drm = &kms->base;
> > >         struct d71_dev *d71 = mdev->chip_data;
> > >         u32 __iomem *reg = d71->gcu_addr;
> > >         u32 check_bits = (d71->num_pipelines == 2) ?
> > > @@ -569,7 +576,7 @@ static int d71_connect_iommu(struct komeda_dev *mdev)
> > >         ret = dp_wait_cond(has_bits(check_bits, malidp_read32(reg, BLK_STATUS)),
> > >                         100, 1000, 1000);
> > >         if (ret < 0) {
> > > -               DRM_ERROR("timed out connecting to TCU!\n");
> > > +               drm_err(drm, "timed out connecting to TCU!\n");
> > >                 malidp_write32_mask(reg, BLK_CONTROL, 0x7, INACTIVE_MODE);
> > >                 return ret;
> > >         }
> > > @@ -582,6 +589,9 @@ static int d71_connect_iommu(struct komeda_dev *mdev)
> > >
> > >  static int d71_disconnect_iommu(struct komeda_dev *mdev)
> > >  {
> > > +       struct komeda_drv *drv = dev_get_drvdata(mdev->dev);
> > > +       struct komeda_kms_dev *kms = drv->kms;
> > > +       struct drm_device *drm = &kms->base;
> > >         struct d71_dev *d71 = mdev->chip_data;
> > >         u32 __iomem *reg = d71->gcu_addr;
> > >         u32 check_bits = (d71->num_pipelines == 2) ?
> > > @@ -593,7 +603,7 @@ static int d71_disconnect_iommu(struct komeda_dev *mdev)
> > >         ret = dp_wait_cond(((malidp_read32(reg, BLK_STATUS) & check_bits) == 0),
> > >                         100, 1000, 1000);
> > >         if (ret < 0) {
> > > -               DRM_ERROR("timed out disconnecting from TCU!\n");
> > > +               drm_err(drm, "timed out disconnecting from TCU!\n");
> > >                 malidp_write32_mask(reg, BLK_CONTROL, 0x7, INACTIVE_MODE);
> > >         }
> > >
> > > --
> > > 2.43.0
> >
> >
> > Just following up on my v2 Komeda logging patch series.
> > Please let me know if any changes or suggestions are needed.
> >
> > Thanks,
> > Rahul

Re: [PATCH v2 0/3] drm/komeda: Convert logging to drm_* helpers

[Liviu Dudau]

Hi Rahul,

Appologies for the amount of time it took me to get to review the series,
I hope this is a one-off instance.

On Tue, Nov 18, 2025 at 04:29:31PM +0530, Rahul Kumar wrote:
> This series converts Komeda logging from the legacy DRM_ERROR/WARN/INFO()
> and DRM_DEBUG() macros to the modern drm_*() helpers. The drm_*() helpers
> take a struct drm_device * and allow the DRM core to include device
> information in the log output. This improves readability and brings the
> driver in line with current DRM logging guidelines.
> 
> To support this conversion, a small Komeda internal header
> (komeda_drv.h) is introduced to provide access to struct komeda_drv
> where needed. No functional changes are intended.

Here are some comments for the general direction of the patches that can
help you improve the series.

First, you don't need to expose the structure komeda_drv, as that is meant
to be internal to komeda_drv.c. What you can do is take inspiration from the
function following the definition of the structure and add a new accessor,
something like this:

--8<------------------------
+struct komeda_kms_dev *dev_to_kmsdev(struct device *dev)
+{
+	struct komeda_drv *mdrv = dev_get_drvdata(dev);
+
+	return mdrv ? mdrv->kms : NULL;
+}
-->8------------------------

Then you can reduce the boilerplate code that you have spread through your
patches to just calling this function to get the komeda_kms_dev. Alternatively,
you can make the function to return a struct drm_device *.

If you read through the code and try to understand its structure you will
realise that there is a top-down organisation of concepts, from a drm_device
down to a komeda_dev that is trying to abstract the hardware differences
between D71 and future Arm Display IP. But some functions, like komeda_pipeline_dump()
are only used in a limited scope, which means you could add the drm_device pointer
to the function signature because komeda_crtc_add() is the only caller of that
function and it already has drm_device pointer, so no point into going through hoops
to get it inside komeda_pipeline_dump().

Another thing that you can do with the series is to be comprehensive and
remove ALL calls to DRM_ERROR/WARN/INFO/DEBUG() where appropriate (leaving
the probe call path untouched as the drm_device is not valid yet).
komeda_pipeline_state.c, komeda_wb_connector.c and komeda_framebuffer.c
should at minimum be cleaned up as well.

There is a third thing that can be done, but that's not on you. komeda
driver is a bit too verbose with the error and debug messages and I could
just go and remove them as they don't add too much information unless
you're doing bring up on a new platform.


Best regards,
Liviu

> 
> Changes in v2:
> - Corrected the use of dev_get_drvdata(); it returns struct komeda_drv *,
>   not struct komeda_kms_dev *.
> - Added komeda_drv.h to make struct komeda_drv available for logging
>   conversion.
> - Split the series into 3 small patches as requested.
> 
> Rahul Kumar (3):
>   drm/komeda: Add komeda_drv.h to make struct komeda_drv available
>   drm/komeda: Convert logging in komeda_pipeline.c to drm_* with
>   drm_device parameter
>   drm/komeda: Convert logging in d71_dev.c to drm_* with drm_device
>   parameter
> 
>  .../gpu/drm/arm/display/komeda/d71/d71_dev.c  | 24 +++++---
>  .../gpu/drm/arm/display/komeda/komeda_drv.c   |  6 +-
>  .../gpu/drm/arm/display/komeda/komeda_drv.h   | 24 ++++++++
>  .../drm/arm/display/komeda/komeda_pipeline.c  | 61 +++++++++++++------
>  4 files changed, 84 insertions(+), 31 deletions(-)
>  create mode 100644 drivers/gpu/drm/arm/display/komeda/komeda_drv.h
> 
> -- 
> 2.43.0
>