[PATCH v2 0/5] drm/panel: panel-simple: Fix panel-dpi probe error

[PATCH v2 0/5] drm/panel: panel-simple: Fix panel-dpi probe error

[Maxime Ripard]

Hi,

Here's a series fixing (hopefully) the panel-simple regression for
panels with a panel-dpi compatible.

It's only build tested, so if you could give that series a try
Francesco, I'd really appreciate it.

Thanks!
Maxime 

Link: https://lore.kernel.org/dri-devel/20250612081834.GA248237@francesco-nb/
Signed-off-by: Maxime Ripard <mripard@kernel.org>
---
Changes in v2:
- Fix build failure if !DRM_MIPI_DSI and DRM_PANEL_SIMPLE=m
- Link to v1: https://lore.kernel.org/r/20250625-drm-panel-simple-fixes-v1-0-c428494a86b8@kernel.org

---
Maxime Ripard (5):
      drm/mipi-dsi: Add dev_is_mipi_dsi function
      drm/panel: panel-simple: make panel_dpi_probe return a panel_desc
      drm/panel: panel-simple: Make panel_simple_probe return its panel
      drm/panel: panel-simple: Add function to look panel data up
      drm/panel: panel-simple: get rid of panel_dpi hack

 drivers/gpu/drm/drm_mipi_dsi.c       |   3 +-
 drivers/gpu/drm/panel/panel-simple.c | 132 ++++++++++++++++++++++-------------
 include/drm/drm_mipi_dsi.h           |   3 +
 3 files changed, 87 insertions(+), 51 deletions(-)
---
base-commit: 86731a2a651e58953fc949573895f2fa6d456841
change-id: 20250624-drm-panel-simple-fixes-ad7f88b9a328

Best regards,
-- 
Maxime Ripard <mripard@kernel.org>

[PATCH v2 1/5] drm/mipi-dsi: Add dev_is_mipi_dsi function

[Maxime Ripard]

This will be especially useful for generic panels (like panel-simple)
which can take different code path depending on if they are MIPI-DSI
devices or platform devices.

Signed-off-by: Maxime Ripard <mripard@kernel.org>
---
 drivers/gpu/drm/drm_mipi_dsi.c | 3 ++-
 include/drm/drm_mipi_dsi.h     | 3 +++
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_mipi_dsi.c b/drivers/gpu/drm/drm_mipi_dsi.c
index e5184a0c24651756ee0b1eb27d94083d63eb35a7..21fd647f8ce1a6a862e2f8fb5320e701f26f614f 100644
--- a/drivers/gpu/drm/drm_mipi_dsi.c
+++ b/drivers/gpu/drm/drm_mipi_dsi.c
@@ -89,16 +89,17 @@ static const struct dev_pm_ops mipi_dsi_device_pm_ops = {
 	.thaw = pm_generic_thaw,
 	.poweroff = pm_generic_poweroff,
 	.restore = pm_generic_restore,
 };
 
-static const struct bus_type mipi_dsi_bus_type = {
+const struct bus_type mipi_dsi_bus_type = {
 	.name = "mipi-dsi",
 	.match = mipi_dsi_device_match,
 	.uevent = mipi_dsi_uevent,
 	.pm = &mipi_dsi_device_pm_ops,
 };
+EXPORT_SYMBOL_GPL(mipi_dsi_bus_type);
 
 /**
  * of_find_mipi_dsi_device_by_node() - find the MIPI DSI device matching a
  *    device tree node
  * @np: device tree node
diff --git a/include/drm/drm_mipi_dsi.h b/include/drm/drm_mipi_dsi.h
index b37860f4a895c25ef8ba1c5b3f44827ef53aa100..6d2c08e8110151a97620389197f1ef79c058329d 100644
--- a/include/drm/drm_mipi_dsi.h
+++ b/include/drm/drm_mipi_dsi.h
@@ -221,10 +221,13 @@ struct mipi_dsi_multi_context {
 
 #define MIPI_DSI_MODULE_PREFIX "mipi-dsi:"
 
 #define to_mipi_dsi_device(__dev)	container_of_const(__dev, struct mipi_dsi_device, dev)
 
+extern const struct bus_type mipi_dsi_bus_type;
+#define dev_is_mipi_dsi(dev)	((dev)->bus == &mipi_dsi_bus_type)
+
 /**
  * mipi_dsi_pixel_format_to_bpp - obtain the number of bits per pixel for any
  *                                given pixel format defined by the MIPI DSI
  *                                specification
  * @fmt: MIPI DSI pixel format

-- 
2.49.0

[PATCH v2 2/5] drm/panel: panel-simple: make panel_dpi_probe return a panel_desc

[Maxime Ripard]

If the panel-simple driver is probed from a panel-dpi compatible, the
driver will use an empty panel_desc structure as a descriminant. It
will then allocate and fill another panel_desc as part of its probe.

However, that allocation needs to happen after the panel_simple
structure has been allocated, since panel_dpi_probe(), the function
doing the panel_desc allocation and initialization, takes a panel_simple
pointer as an argument.

This pointer is used to fill the panel_simple->desc pointer that is
still initialized with the empty panel_desc when panel_dpi_probe() is
called.

Since commit de04bb0089a9 ("drm/panel/panel-simple: Use the new
allocation in place of devm_kzalloc()"), we will need the panel
connector type found in panel_desc to allocate panel_simple. This
creates a circular dependency where we need panel_desc to create
panel_simple, and need panel_simple to create panel_desc.

Let's break that dependency by making panel_dpi_probe simply return the
panel_desc it initialized and move the panel_simple->desc assignment to
the caller.

This will not fix the breaking commit entirely, but will move us towards
the right direction.

Fixes: de04bb0089a9 ("drm/panel/panel-simple: Use the new allocation in place of devm_kzalloc()")
Signed-off-by: Maxime Ripard <mripard@kernel.org>
---
 drivers/gpu/drm/panel/panel-simple.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c
index 0a3b26bb4d731c54614e24e38018c308acd5367a..89188e683822f9202ec580c9a294e42083b9704a 100644
--- a/drivers/gpu/drm/panel/panel-simple.c
+++ b/drivers/gpu/drm/panel/panel-simple.c
@@ -430,12 +430,11 @@ static const struct drm_panel_funcs panel_simple_funcs = {
 	.get_timings = panel_simple_get_timings,
 };
 
 static struct panel_desc panel_dpi;
 
-static int panel_dpi_probe(struct device *dev,
-			   struct panel_simple *panel)
+static struct panel_desc *panel_dpi_probe(struct device *dev)
 {
 	struct display_timing *timing;
 	const struct device_node *np;
 	struct panel_desc *desc;
 	unsigned int bus_flags;
@@ -443,21 +442,21 @@ static int panel_dpi_probe(struct device *dev,
 	int ret;
 
 	np = dev->of_node;
 	desc = devm_kzalloc(dev, sizeof(*desc), GFP_KERNEL);
 	if (!desc)
-		return -ENOMEM;
+		return ERR_PTR(-ENOMEM);
 
 	timing = devm_kzalloc(dev, sizeof(*timing), GFP_KERNEL);
 	if (!timing)
-		return -ENOMEM;
+		return ERR_PTR(-ENOMEM);
 
 	ret = of_get_display_timing(np, "panel-timing", timing);
 	if (ret < 0) {
 		dev_err(dev, "%pOF: no panel-timing node found for \"panel-dpi\" binding\n",
 			np);
-		return ret;
+		return ERR_PTR(ret);
 	}
 
 	desc->timings = timing;
 	desc->num_timings = 1;
 
@@ -471,13 +470,11 @@ static int panel_dpi_probe(struct device *dev,
 	desc->bus_flags = bus_flags;
 
 	/* We do not know the connector for the DT node, so guess it */
 	desc->connector_type = DRM_MODE_CONNECTOR_DPI;
 
-	panel->desc = desc;
-
-	return 0;
+	return desc;
 }
 
 #define PANEL_SIMPLE_BOUNDS_CHECK(to_check, bounds, field) \
 	(to_check->field.typ >= bounds->field.min && \
 	 to_check->field.typ <= bounds->field.max)
@@ -611,14 +608,17 @@ static int panel_simple_probe(struct device *dev, const struct panel_desc *desc)
 			return -EPROBE_DEFER;
 	}
 
 	if (desc == &panel_dpi) {
 		/* Handle the generic panel-dpi binding */
-		err = panel_dpi_probe(dev, panel);
-		if (err)
+		desc = panel_dpi_probe(dev);
+		if (IS_ERR(desc)) {
+			err = PTR_ERR(desc);
 			goto free_ddc;
-		desc = panel->desc;
+		}
+
+		panel->desc = desc;
 	} else {
 		if (!of_get_display_timing(dev->of_node, "panel-timing", &dt))
 			panel_simple_parse_panel_timing_node(dev, panel, &dt);
 	}
 

-- 
2.49.0

[PATCH v2 3/5] drm/panel: panel-simple: Make panel_simple_probe return its panel

[Maxime Ripard]

In order to fix the regession introduced by commit de04bb0089a9
("drm/panel/panel-simple: Use the new allocation in place of
devm_kzalloc()"), we need to move the panel_desc lookup into the common
panel_simple_probe() function.

There's two callers for that function, the probe implementations of the
platform and MIPI-DSI drivers panel-simple implements.

The MIPI-DSI driver's probe will need to access the current panel_desc
to initialize properly, which won't be possible anymore if we make that
lookup in panel_simple_probe().

However, we can make panel_simple_probe() return the initialized
panel_simple structure it allocated, which will contain a pointer to the
associated panel_desc in its desc field.

This doesn't fix de04bb0089a9 ("drm/panel/panel-simple: Use the new
allocation in place of devm_kzalloc()") still, but makes progress
towards that goal.

Fixes: de04bb0089a9 ("drm/panel/panel-simple: Use the new allocation in place of devm_kzalloc()")
Signed-off-by: Maxime Ripard <mripard@kernel.org>
---
 drivers/gpu/drm/panel/panel-simple.c | 33 ++++++++++++++++++++-------------
 1 file changed, 20 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c
index 89188e683822f9202ec580c9a294e42083b9704a..e70ee2d4a538caaae673507b93e02b444a2e1640 100644
--- a/drivers/gpu/drm/panel/panel-simple.c
+++ b/drivers/gpu/drm/panel/panel-simple.c
@@ -565,11 +565,11 @@ static int panel_simple_override_nondefault_lvds_datamapping(struct device *dev,
 	}
 
 	return 0;
 }
 
-static int panel_simple_probe(struct device *dev, const struct panel_desc *desc)
+static struct panel_simple *panel_simple_probe(struct device *dev, const struct panel_desc *desc)
 {
 	struct panel_simple *panel;
 	struct display_timing dt;
 	struct device_node *ddc;
 	int connector_type;
@@ -577,37 +577,37 @@ static int panel_simple_probe(struct device *dev, const struct panel_desc *desc)
 	int err;
 
 	panel = devm_drm_panel_alloc(dev, struct panel_simple, base,
 				     &panel_simple_funcs, desc->connector_type);
 	if (IS_ERR(panel))
-		return PTR_ERR(panel);
+		return ERR_CAST(panel);
 
 	panel->desc = desc;
 
 	panel->supply = devm_regulator_get(dev, "power");
 	if (IS_ERR(panel->supply))
-		return PTR_ERR(panel->supply);
+		return ERR_CAST(panel->supply);
 
 	panel->enable_gpio = devm_gpiod_get_optional(dev, "enable",
 						     GPIOD_OUT_LOW);
 	if (IS_ERR(panel->enable_gpio))
-		return dev_err_probe(dev, PTR_ERR(panel->enable_gpio),
-				     "failed to request GPIO\n");
+		return dev_err_cast_probe(dev, panel->enable_gpio,
+					  "failed to request GPIO\n");
 
 	err = of_drm_get_panel_orientation(dev->of_node, &panel->orientation);
 	if (err) {
 		dev_err(dev, "%pOF: failed to get orientation %d\n", dev->of_node, err);
-		return err;
+		return ERR_PTR(err);
 	}
 
 	ddc = of_parse_phandle(dev->of_node, "ddc-i2c-bus", 0);
 	if (ddc) {
 		panel->ddc = of_find_i2c_adapter_by_node(ddc);
 		of_node_put(ddc);
 
 		if (!panel->ddc)
-			return -EPROBE_DEFER;
+			return ERR_PTR(-EPROBE_DEFER);
 	}
 
 	if (desc == &panel_dpi) {
 		/* Handle the generic panel-dpi binding */
 		desc = panel_dpi_probe(dev);
@@ -701,20 +701,20 @@ static int panel_simple_probe(struct device *dev, const struct panel_desc *desc)
 		goto disable_pm_runtime;
 	}
 
 	drm_panel_add(&panel->base);
 
-	return 0;
+	return panel;
 
 disable_pm_runtime:
 	pm_runtime_dont_use_autosuspend(dev);
 	pm_runtime_disable(dev);
 free_ddc:
 	if (panel->ddc)
 		put_device(&panel->ddc->dev);
 
-	return err;
+	return ERR_PTR(err);
 }
 
 static void panel_simple_shutdown(struct device *dev)
 {
 	struct panel_simple *panel = dev_get_drvdata(dev);
@@ -5375,16 +5375,21 @@ static const struct of_device_id platform_of_match[] = {
 MODULE_DEVICE_TABLE(of, platform_of_match);
 
 static int panel_simple_platform_probe(struct platform_device *pdev)
 {
 	const struct panel_desc *desc;
+	struct panel_simple *panel;
 
 	desc = of_device_get_match_data(&pdev->dev);
 	if (!desc)
 		return -ENODEV;
 
-	return panel_simple_probe(&pdev->dev, desc);
+	panel = panel_simple_probe(&pdev->dev, desc);
+	if (IS_ERR(panel))
+		return PTR_ERR(panel);
+
+	return 0;
 }
 
 static void panel_simple_platform_remove(struct platform_device *pdev)
 {
 	panel_simple_remove(&pdev->dev);
@@ -5651,20 +5656,22 @@ static const struct of_device_id dsi_of_match[] = {
 MODULE_DEVICE_TABLE(of, dsi_of_match);
 
 static int panel_simple_dsi_probe(struct mipi_dsi_device *dsi)
 {
 	const struct panel_desc_dsi *desc;
+	struct panel_simple *panel;
 	int err;
 
 	desc = of_device_get_match_data(&dsi->dev);
 	if (!desc)
 		return -ENODEV;
 
-	err = panel_simple_probe(&dsi->dev, &desc->desc);
-	if (err < 0)
-		return err;
+	panel = panel_simple_probe(&dsi->dev, &desc->desc);
+	if (IS_ERR(panel))
+		return PTR_ERR(panel);
 
+	desc = container_of(panel->desc, struct panel_desc_dsi, desc);
 	dsi->mode_flags = desc->flags;
 	dsi->format = desc->format;
 	dsi->lanes = desc->lanes;
 
 	err = mipi_dsi_attach(dsi);

-- 
2.49.0

[PATCH v2 4/5] drm/panel: panel-simple: Add function to look panel data up

[Maxime Ripard]

Commit de04bb0089a9 ("drm/panel/panel-simple: Use the new allocation in
place of devm_kzalloc()") moved the call to drm_panel_init into the
devm_drm_panel_alloc(), which needs a connector type to initialize
properly.

In the panel-dpi compatible case, the passed panel_desc structure is an
empty one used as a discriminant, and the connector type it contains
isn't actually initialized.

It is initialized through a call to panel_dpi_probe() later in the
function, which used to be before the call to drm_panel_init() that got
merged into devm_drm_panel_alloc().

So, we do need a proper panel_desc pointer before the call to
devm_drm_panel_alloc() now. All cases associate their panel_desc with
the panel compatible and use of_device_get_match_data, except for the
panel-dpi compatible.

In that case, we're expected to call panel_dpi_probe, which will
allocate and initialize the panel_desc for us.

Let's create such a helper function that would be called first in the
driver and will lookup the desc by compatible, or allocate one if
relevant.

Reported-by: Francesco Dolcini <francesco@dolcini.it>
Closes: https://lore.kernel.org/all/20250612081834.GA248237@francesco-nb/
Fixes: de04bb0089a9 ("drm/panel/panel-simple: Use the new allocation in place of devm_kzalloc()")
Signed-off-by: Maxime Ripard <mripard@kernel.org>
---
 drivers/gpu/drm/panel/panel-simple.c | 82 +++++++++++++++++++++---------------
 1 file changed, 49 insertions(+), 33 deletions(-)

diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c
index e70ee2d4a538caaae673507b93e02b444a2e1640..4c831af86414aba6a6b44dbc6020c0268dbd78b7 100644
--- a/drivers/gpu/drm/panel/panel-simple.c
+++ b/drivers/gpu/drm/panel/panel-simple.c
@@ -24,10 +24,11 @@
 #include <linux/delay.h>
 #include <linux/gpio/consumer.h>
 #include <linux/i2c.h>
 #include <linux/media-bus-format.h>
 #include <linux/module.h>
+#include <linux/of_device.h>
 #include <linux/of_platform.h>
 #include <linux/platform_device.h>
 #include <linux/pm_runtime.h>
 #include <linux/regulator/consumer.h>
 
@@ -134,10 +135,18 @@ struct panel_desc {
 
 	/** @connector_type: LVDS, eDP, DSI, DPI, etc. */
 	int connector_type;
 };
 
+struct panel_desc_dsi {
+	struct panel_desc desc;
+
+	unsigned long flags;
+	enum mipi_dsi_pixel_format format;
+	unsigned int lanes;
+};
+
 struct panel_simple {
 	struct drm_panel base;
 
 	ktime_t unprepared_time;
 
@@ -565,19 +574,53 @@ static int panel_simple_override_nondefault_lvds_datamapping(struct device *dev,
 	}
 
 	return 0;
 }
 
-static struct panel_simple *panel_simple_probe(struct device *dev, const struct panel_desc *desc)
+static const struct panel_desc *panel_simple_get_desc(struct device *dev)
 {
+	if (IS_ENABLED(CONFIG_DRM_MIPI_DSI) &&
+	    dev_is_mipi_dsi(dev)) {
+		const struct panel_desc_dsi *dsi_desc;
+
+		dsi_desc = of_device_get_match_data(dev);
+		if (!dsi_desc)
+			return ERR_PTR(-ENODEV);
+
+		return &dsi_desc->desc;
+	}
+
+	if (dev_is_platform(dev)) {
+		const struct panel_desc *desc;
+
+		desc = of_device_get_match_data(dev);
+		if (!desc)
+			return ERR_PTR(-ENODEV);
+
+		if (desc == &panel_dpi)
+			return panel_dpi_probe(dev);
+
+		return desc;
+	}
+
+	return ERR_PTR(-ENODEV);
+}
+
+static struct panel_simple *panel_simple_probe(struct device *dev)
+{
+	const struct panel_desc *desc;
 	struct panel_simple *panel;
 	struct display_timing dt;
 	struct device_node *ddc;
 	int connector_type;
 	u32 bus_flags;
 	int err;
 
+	desc = panel_simple_get_desc(dev);
+	if (IS_ERR(desc))
+		return ERR_CAST(desc);
+
 	panel = devm_drm_panel_alloc(dev, struct panel_simple, base,
 				     &panel_simple_funcs, desc->connector_type);
 	if (IS_ERR(panel))
 		return ERR_CAST(panel);
 
@@ -606,23 +649,13 @@ static struct panel_simple *panel_simple_probe(struct device *dev, const struct
 
 		if (!panel->ddc)
 			return ERR_PTR(-EPROBE_DEFER);
 	}
 
-	if (desc == &panel_dpi) {
-		/* Handle the generic panel-dpi binding */
-		desc = panel_dpi_probe(dev);
-		if (IS_ERR(desc)) {
-			err = PTR_ERR(desc);
-			goto free_ddc;
-		}
-
-		panel->desc = desc;
-	} else {
-		if (!of_get_display_timing(dev->of_node, "panel-timing", &dt))
-			panel_simple_parse_panel_timing_node(dev, panel, &dt);
-	}
+	if ((desc != &panel_dpi) &&
+	    !of_get_display_timing(dev->of_node, "panel-timing", &dt))
+		panel_simple_parse_panel_timing_node(dev, panel, &dt);
 
 	if (desc->connector_type == DRM_MODE_CONNECTOR_LVDS) {
 		/* Optional data-mapping property for overriding bus format */
 		err = panel_simple_override_nondefault_lvds_datamapping(dev, panel);
 		if (err)
@@ -5374,18 +5407,13 @@ static const struct of_device_id platform_of_match[] = {
 };
 MODULE_DEVICE_TABLE(of, platform_of_match);
 
 static int panel_simple_platform_probe(struct platform_device *pdev)
 {
-	const struct panel_desc *desc;
 	struct panel_simple *panel;
 
-	desc = of_device_get_match_data(&pdev->dev);
-	if (!desc)
-		return -ENODEV;
-
-	panel = panel_simple_probe(&pdev->dev, desc);
+	panel = panel_simple_probe(&pdev->dev);
 	if (IS_ERR(panel))
 		return PTR_ERR(panel);
 
 	return 0;
 }
@@ -5415,18 +5443,10 @@ static struct platform_driver panel_simple_platform_driver = {
 	.probe = panel_simple_platform_probe,
 	.remove = panel_simple_platform_remove,
 	.shutdown = panel_simple_platform_shutdown,
 };
 
-struct panel_desc_dsi {
-	struct panel_desc desc;
-
-	unsigned long flags;
-	enum mipi_dsi_pixel_format format;
-	unsigned int lanes;
-};
-
 static const struct drm_display_mode auo_b080uan01_mode = {
 	.clock = 154500,
 	.hdisplay = 1200,
 	.hsync_start = 1200 + 62,
 	.hsync_end = 1200 + 62 + 4,
@@ -5659,15 +5679,11 @@ static int panel_simple_dsi_probe(struct mipi_dsi_device *dsi)
 {
 	const struct panel_desc_dsi *desc;
 	struct panel_simple *panel;
 	int err;
 
-	desc = of_device_get_match_data(&dsi->dev);
-	if (!desc)
-		return -ENODEV;
-
-	panel = panel_simple_probe(&dsi->dev, &desc->desc);
+	panel = panel_simple_probe(&dsi->dev);
 	if (IS_ERR(panel))
 		return PTR_ERR(panel);
 
 	desc = container_of(panel->desc, struct panel_desc_dsi, desc);
 	dsi->mode_flags = desc->flags;

-- 
2.49.0

[PATCH v2 5/5] drm/panel: panel-simple: get rid of panel_dpi hack

[Maxime Ripard]

The empty panel_dpi struct was only ever used as a discriminant, but
it's kind of a hack, and with the reworks done in the previous patches,
we shouldn't need it anymore.

Let's get rid of it.

Signed-off-by: Maxime Ripard <mripard@kernel.org>
---
 drivers/gpu/drm/panel/panel-simple.c | 27 ++++++++++++++++++---------
 1 file changed, 18 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c
index 4c831af86414aba6a6b44dbc6020c0268dbd78b7..9f81fa960b460290759f5f9eba97045ab55fe5b8 100644
--- a/drivers/gpu/drm/panel/panel-simple.c
+++ b/drivers/gpu/drm/panel/panel-simple.c
@@ -437,12 +437,10 @@ static const struct drm_panel_funcs panel_simple_funcs = {
 	.get_modes = panel_simple_get_modes,
 	.get_orientation = panel_simple_get_orientation,
 	.get_timings = panel_simple_get_timings,
 };
 
-static struct panel_desc panel_dpi;
-
 static struct panel_desc *panel_dpi_probe(struct device *dev)
 {
 	struct display_timing *timing;
 	const struct device_node *np;
 	struct panel_desc *desc;
@@ -591,15 +589,21 @@ static const struct panel_desc *panel_simple_get_desc(struct device *dev)
 
 	if (dev_is_platform(dev)) {
 		const struct panel_desc *desc;
 
 		desc = of_device_get_match_data(dev);
-		if (!desc)
-			return ERR_PTR(-ENODEV);
-
-		if (desc == &panel_dpi)
-			return panel_dpi_probe(dev);
+		if (!desc) {
+			/*
+			 * panel-dpi probes without a descriptor and
+			 * panel_dpi_probe() will initialize one for us
+			 * based on the device tree.
+			 */
+			if (of_device_is_compatible(dev->of_node, "panel-dpi"))
+				return panel_dpi_probe(dev);
+			else
+				return ERR_PTR(-ENODEV);
+		}
 
 		return desc;
 	}
 
 	return ERR_PTR(-ENODEV);
@@ -649,11 +653,11 @@ static struct panel_simple *panel_simple_probe(struct device *dev)
 
 		if (!panel->ddc)
 			return ERR_PTR(-EPROBE_DEFER);
 	}
 
-	if ((desc != &panel_dpi) &&
+	if (!of_device_is_compatible(dev->of_node, "panel-dpi") &&
 	    !of_get_display_timing(dev->of_node, "panel-timing", &dt))
 		panel_simple_parse_panel_timing_node(dev, panel, &dt);
 
 	if (desc->connector_type == DRM_MODE_CONNECTOR_LVDS) {
 		/* Optional data-mapping property for overriding bus format */
@@ -5398,11 +5402,16 @@ static const struct of_device_id platform_of_match[] = {
 		.compatible = "microchip,ac69t88a",
 		.data = &mchp_ac69t88a,
 	}, {
 		/* Must be the last entry */
 		.compatible = "panel-dpi",
-		.data = &panel_dpi,
+
+		/*
+		 * Explicitly NULL, the panel_desc structure will be
+		 * allocated by panel_dpi_probe().
+		 */
+		.data = NULL,
 	}, {
 		/* sentinel */
 	}
 };
 MODULE_DEVICE_TABLE(of, platform_of_match);

-- 
2.49.0

Re: [PATCH v2 0/5] drm/panel: panel-simple: Fix panel-dpi probe error

[Francesco Dolcini]

Hello Maxime,
it works for me, thanks!

On Thu, Jun 26, 2025 at 12:04:58PM +0200, Maxime Ripard wrote:
> Here's a series fixing (hopefully) the panel-simple regression for
> panels with a panel-dpi compatible.
> 
> It's only build tested, so if you could give that series a try
> Francesco, I'd really appreciate it.
> 
> Thanks!
> Maxime 
> 
> Link: https://lore.kernel.org/dri-devel/20250612081834.GA248237@francesco-nb/
> Signed-off-by: Maxime Ripard <mripard@kernel.org>

For the whole series,

Tested-by: Francesco Dolcini <francesco.dolcini@toradex.com> # Toradex Colibri iMX6

Francesco

Re: [PATCH v2 1/5] drm/mipi-dsi: Add dev_is_mipi_dsi function

[Javier Martinez Canillas]

Maxime Ripard <mripard@kernel.org> writes:

Hello Maxime,

> This will be especially useful for generic panels (like panel-simple)
> which can take different code path depending on if they are MIPI-DSI
> devices or platform devices.
>
> Signed-off-by: Maxime Ripard <mripard@kernel.org>
> ---
>  drivers/gpu/drm/drm_mipi_dsi.c | 3 ++-
>  include/drm/drm_mipi_dsi.h     | 3 +++
>  2 files changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/drm_mipi_dsi.c b/drivers/gpu/drm/drm_mipi_dsi.c
> index e5184a0c24651756ee0b1eb27d94083d63eb35a7..21fd647f8ce1a6a862e2f8fb5320e701f26f614f 100644
> --- a/drivers/gpu/drm/drm_mipi_dsi.c
> +++ b/drivers/gpu/drm/drm_mipi_dsi.c
> @@ -89,16 +89,17 @@ static const struct dev_pm_ops mipi_dsi_device_pm_ops = {
>  	.thaw = pm_generic_thaw,
>  	.poweroff = pm_generic_poweroff,
>  	.restore = pm_generic_restore,
>  };
>  
> -static const struct bus_type mipi_dsi_bus_type = {
> +const struct bus_type mipi_dsi_bus_type = {
>  	.name = "mipi-dsi",
>  	.match = mipi_dsi_device_match,
>  	.uevent = mipi_dsi_uevent,
>  	.pm = &mipi_dsi_device_pm_ops,
>  };
> +EXPORT_SYMBOL_GPL(mipi_dsi_bus_type);
>  
>  /**
>   * of_find_mipi_dsi_device_by_node() - find the MIPI DSI device matching a
>   *    device tree node
>   * @np: device tree node
> diff --git a/include/drm/drm_mipi_dsi.h b/include/drm/drm_mipi_dsi.h
> index b37860f4a895c25ef8ba1c5b3f44827ef53aa100..6d2c08e8110151a97620389197f1ef79c058329d 100644
> --- a/include/drm/drm_mipi_dsi.h
> +++ b/include/drm/drm_mipi_dsi.h
> @@ -221,10 +221,13 @@ struct mipi_dsi_multi_context {
>  
>  #define MIPI_DSI_MODULE_PREFIX "mipi-dsi:"
>  
>  #define to_mipi_dsi_device(__dev)	container_of_const(__dev, struct mipi_dsi_device, dev)
>  
> +extern const struct bus_type mipi_dsi_bus_type;
> +#define dev_is_mipi_dsi(dev)	((dev)->bus == &mipi_dsi_bus_type)
> +

Usually I prefer to have static inline functions instead of macros to have
type checking. I see that this header has a mix of both, so I don't have a
strong opinion on what to use in this case.

Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat

Re: [PATCH v2 2/5] drm/panel: panel-simple: make panel_dpi_probe return a panel_desc

[Javier Martinez Canillas]

Maxime Ripard <mripard@kernel.org> writes:

> If the panel-simple driver is probed from a panel-dpi compatible, the
> driver will use an empty panel_desc structure as a descriminant. It
> will then allocate and fill another panel_desc as part of its probe.
>
> However, that allocation needs to happen after the panel_simple
> structure has been allocated, since panel_dpi_probe(), the function
> doing the panel_desc allocation and initialization, takes a panel_simple
> pointer as an argument.
>
> This pointer is used to fill the panel_simple->desc pointer that is
> still initialized with the empty panel_desc when panel_dpi_probe() is
> called.
>
> Since commit de04bb0089a9 ("drm/panel/panel-simple: Use the new
> allocation in place of devm_kzalloc()"), we will need the panel
> connector type found in panel_desc to allocate panel_simple. This
> creates a circular dependency where we need panel_desc to create
> panel_simple, and need panel_simple to create panel_desc.
>
> Let's break that dependency by making panel_dpi_probe simply return the
> panel_desc it initialized and move the panel_simple->desc assignment to
> the caller.
>
> This will not fix the breaking commit entirely, but will move us towards
> the right direction.
>
> Fixes: de04bb0089a9 ("drm/panel/panel-simple: Use the new allocation in place of devm_kzalloc()")
> Signed-off-by: Maxime Ripard <mripard@kernel.org>
> ---

Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat

Re: [PATCH v2 3/5] drm/panel: panel-simple: Make panel_simple_probe return its panel

[Javier Martinez Canillas]

Maxime Ripard <mripard@kernel.org> writes:

> In order to fix the regession introduced by commit de04bb0089a9
> ("drm/panel/panel-simple: Use the new allocation in place of
> devm_kzalloc()"), we need to move the panel_desc lookup into the common
> panel_simple_probe() function.
>
> There's two callers for that function, the probe implementations of the
> platform and MIPI-DSI drivers panel-simple implements.
>
> The MIPI-DSI driver's probe will need to access the current panel_desc
> to initialize properly, which won't be possible anymore if we make that
> lookup in panel_simple_probe().
>
> However, we can make panel_simple_probe() return the initialized
> panel_simple structure it allocated, which will contain a pointer to the
> associated panel_desc in its desc field.
>
> This doesn't fix de04bb0089a9 ("drm/panel/panel-simple: Use the new
> allocation in place of devm_kzalloc()") still, but makes progress
> towards that goal.
>
> Fixes: de04bb0089a9 ("drm/panel/panel-simple: Use the new allocation in place of devm_kzalloc()")
> Signed-off-by: Maxime Ripard <mripard@kernel.org>
> ---

[...]

> -static int panel_simple_probe(struct device *dev, const struct panel_desc *desc)
> +static struct panel_simple *panel_simple_probe(struct device *dev, const struct panel_desc *desc)
>  {

[...]

>  
>  	ddc = of_parse_phandle(dev->of_node, "ddc-i2c-bus", 0);
>  	if (ddc) {
>  		panel->ddc = of_find_i2c_adapter_by_node(ddc);
>  		of_node_put(ddc);
>  
>  		if (!panel->ddc)
> -			return -EPROBE_DEFER;
> +			return ERR_PTR(-EPROBE_DEFER);

Not related with your patch, but it would be great to also add a
dev_err_probe() here to record the reason of this probe deferral.

Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat

Re: [PATCH v2 4/5] drm/panel: panel-simple: Add function to look panel data up

[Javier Martinez Canillas]

Maxime Ripard <mripard@kernel.org> writes:

> Commit de04bb0089a9 ("drm/panel/panel-simple: Use the new allocation in
> place of devm_kzalloc()") moved the call to drm_panel_init into the
> devm_drm_panel_alloc(), which needs a connector type to initialize
> properly.
>
> In the panel-dpi compatible case, the passed panel_desc structure is an
> empty one used as a discriminant, and the connector type it contains
> isn't actually initialized.
>
> It is initialized through a call to panel_dpi_probe() later in the
> function, which used to be before the call to drm_panel_init() that got
> merged into devm_drm_panel_alloc().
>
> So, we do need a proper panel_desc pointer before the call to
> devm_drm_panel_alloc() now. All cases associate their panel_desc with
> the panel compatible and use of_device_get_match_data, except for the
> panel-dpi compatible.
>
> In that case, we're expected to call panel_dpi_probe, which will
> allocate and initialize the panel_desc for us.
>
> Let's create such a helper function that would be called first in the
> driver and will lookup the desc by compatible, or allocate one if
> relevant.
>
> Reported-by: Francesco Dolcini <francesco@dolcini.it>
> Closes: https://lore.kernel.org/all/20250612081834.GA248237@francesco-nb/
> Fixes: de04bb0089a9 ("drm/panel/panel-simple: Use the new allocation in place of devm_kzalloc()")
> Signed-off-by: Maxime Ripard <mripard@kernel.org>
> ---

Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat

Re: [PATCH v2 5/5] drm/panel: panel-simple: get rid of panel_dpi hack

[Javier Martinez Canillas]

Maxime Ripard <mripard@kernel.org> writes:

> The empty panel_dpi struct was only ever used as a discriminant, but
> it's kind of a hack, and with the reworks done in the previous patches,
> we shouldn't need it anymore.
>
> Let's get rid of it.
>
> Signed-off-by: Maxime Ripard <mripard@kernel.org>
> ---

Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat

Re: [PATCH v2 0/5] drm/panel: panel-simple: Fix panel-dpi probe error

[Maxime Ripard]

On Thu, 26 Jun 2025 12:04:58 +0200, Maxime Ripard wrote:
> Here's a series fixing (hopefully) the panel-simple regression for
> panels with a panel-dpi compatible.
> 
> It's only build tested, so if you could give that series a try
> Francesco, I'd really appreciate it.
> 
> Thanks!
> Maxime
> 
> [...]

Applied to misc/kernel.git (drm-misc-fixes).

Thanks!
Maxime

Re: [PATCH v2 0/5] drm/panel: panel-simple: Fix panel-dpi probe error

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 783 bytes --]

Hi,

On Thu, Jun 26, 2025 at 12:21:28PM +0200, Francesco Dolcini wrote:
> Hello Maxime,
> it works for me, thanks!
> 
> On Thu, Jun 26, 2025 at 12:04:58PM +0200, Maxime Ripard wrote:
> > Here's a series fixing (hopefully) the panel-simple regression for
> > panels with a panel-dpi compatible.
> > 
> > It's only build tested, so if you could give that series a try
> > Francesco, I'd really appreciate it.
> > 
> > Thanks!
> > Maxime 
> > 
> > Link: https://lore.kernel.org/dri-devel/20250612081834.GA248237@francesco-nb/
> > Signed-off-by: Maxime Ripard <mripard@kernel.org>
> 
> For the whole series,
> 
> Tested-by: Francesco Dolcini <francesco.dolcini@toradex.com> # Toradex Colibri iMX6

Thanks again for testing, and sorry for the regression

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 273 bytes --]

Re: [PATCH v2 1/5] drm/mipi-dsi: Add dev_is_mipi_dsi function

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 2506 bytes --]

On Fri, Jun 27, 2025 at 11:04:16AM +0200, Javier Martinez Canillas wrote:
> Maxime Ripard <mripard@kernel.org> writes:
> 
> Hello Maxime,
> 
> > This will be especially useful for generic panels (like panel-simple)
> > which can take different code path depending on if they are MIPI-DSI
> > devices or platform devices.
> >
> > Signed-off-by: Maxime Ripard <mripard@kernel.org>
> > ---
> >  drivers/gpu/drm/drm_mipi_dsi.c | 3 ++-
> >  include/drm/drm_mipi_dsi.h     | 3 +++
> >  2 files changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/drm_mipi_dsi.c b/drivers/gpu/drm/drm_mipi_dsi.c
> > index e5184a0c24651756ee0b1eb27d94083d63eb35a7..21fd647f8ce1a6a862e2f8fb5320e701f26f614f 100644
> > --- a/drivers/gpu/drm/drm_mipi_dsi.c
> > +++ b/drivers/gpu/drm/drm_mipi_dsi.c
> > @@ -89,16 +89,17 @@ static const struct dev_pm_ops mipi_dsi_device_pm_ops = {
> >  	.thaw = pm_generic_thaw,
> >  	.poweroff = pm_generic_poweroff,
> >  	.restore = pm_generic_restore,
> >  };
> >  
> > -static const struct bus_type mipi_dsi_bus_type = {
> > +const struct bus_type mipi_dsi_bus_type = {
> >  	.name = "mipi-dsi",
> >  	.match = mipi_dsi_device_match,
> >  	.uevent = mipi_dsi_uevent,
> >  	.pm = &mipi_dsi_device_pm_ops,
> >  };
> > +EXPORT_SYMBOL_GPL(mipi_dsi_bus_type);
> >  
> >  /**
> >   * of_find_mipi_dsi_device_by_node() - find the MIPI DSI device matching a
> >   *    device tree node
> >   * @np: device tree node
> > diff --git a/include/drm/drm_mipi_dsi.h b/include/drm/drm_mipi_dsi.h
> > index b37860f4a895c25ef8ba1c5b3f44827ef53aa100..6d2c08e8110151a97620389197f1ef79c058329d 100644
> > --- a/include/drm/drm_mipi_dsi.h
> > +++ b/include/drm/drm_mipi_dsi.h
> > @@ -221,10 +221,13 @@ struct mipi_dsi_multi_context {
> >  
> >  #define MIPI_DSI_MODULE_PREFIX "mipi-dsi:"
> >  
> >  #define to_mipi_dsi_device(__dev)	container_of_const(__dev, struct mipi_dsi_device, dev)
> >  
> > +extern const struct bus_type mipi_dsi_bus_type;
> > +#define dev_is_mipi_dsi(dev)	((dev)->bus == &mipi_dsi_bus_type)
> > +
> 
> Usually I prefer to have static inline functions instead of macros to have
> type checking. I see that this header has a mix of both, so I don't have a
> strong opinion on what to use in this case.
> 
> Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>

Thanks for your review!

For the record, it's also how the platform bus defines its equivalent
macro, so that's why I went with it.

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 273 bytes --]

Re: [PATCH v2 1/5] drm/mipi-dsi: Add dev_is_mipi_dsi function

[Javier Martinez Canillas]

Maxime Ripard <mripard@kernel.org> writes:

> On Fri, Jun 27, 2025 at 11:04:16AM +0200, Javier Martinez Canillas wrote:
>> Maxime Ripard <mripard@kernel.org> writes:
>> 

[...]

>> 
>> Usually I prefer to have static inline functions instead of macros to have
>> type checking. I see that this header has a mix of both, so I don't have a
>> strong opinion on what to use in this case.
>> 
>> Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
>
> Thanks for your review!
>
> For the record, it's also how the platform bus defines its equivalent
> macro, so that's why I went with it.
>

Got it. I think is OK.

> Maxime

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat