Skip to contentSkip to conversation

drm/panel: refactor INNOLUX P079ZCA panel driver

PATCH RESEND Superseded Lin Huang 3 participants 6 messages Dec 4, 2017 – Mar 13, 2018

Subsystems (4)

v3 of 4 Superseded Latest is v4 →

lore
Share & tools
lore

Lin Huang

author
#0 lore raw

PATCH RESEND v3 1/3 drm/panel: refactor INNOLUX P079ZCA panel driver

Refactor Innolux P079ZCA panel driver, let it support multi panel.

---
Changes in v2:
- Change regulator property name to meet the panel datasheet
Changes in v3:
- this patch only refactor P079ZCA panel to support multi panel, support P097PFG panel in another patch

 drivers/gpu/drm/panel/panel-innolux-p079zca.c | 147 ++++++++++++++++++--------
 1 file changed, 105 insertions(+), 42 deletions(-)
drivers/gpu/drm/panel/panel-innolux-p079zca.c +105 -42
index 6ba9344..1597744 100644
@@ -20,12 +20,32 @@
 
 #include <video/mipi_display.h>
 
+struct panel_desc {
+	const struct drm_display_mode *modes;
+	unsigned int bpc;
+	struct {
+		unsigned int width;
+		unsigned int height;
+	} size;
+};
+
+struct panel_desc_dsi {
+	struct panel_desc desc;
+
+	unsigned long flags;
+	enum mipi_dsi_pixel_format format;
+	unsigned int lanes;
+};
+
 struct innolux_panel {
 	struct drm_panel base;
 	struct mipi_dsi_device *link;
+	const struct panel_desc_dsi *dsi_desc;
 
 	struct backlight_device *backlight;
-	struct regulator *supply;
+	struct regulator *vddi;
+	struct regulator *avdd;
+	struct regulator *avee;
 	struct gpio_desc *enable_gpio;
 
 	bool prepared;
@@ -78,9 +98,9 @@ static int innolux_panel_unprepare(struct drm_panel *panel)
 	/* T8: 80ms - 1000ms */
 	msleep(80);
 
-	err = regulator_disable(innolux->supply);
-	if (err < 0)
-		return err;
+	regulator_disable(innolux->avee);
+	regulator_disable(innolux->avdd);
+	regulator_disable(innolux->vddi);
 
 	innolux->prepared = false;
 
@@ -97,10 +117,18 @@ static int innolux_panel_prepare(struct drm_panel *panel)
 
 	gpiod_set_value_cansleep(innolux->enable_gpio, 0);
 
-	err = regulator_enable(innolux->supply);
+	err = regulator_enable(innolux->vddi);
 	if (err < 0)
 		return err;
 
+	err = regulator_enable(innolux->avdd);
+	if (err < 0)
+		goto disable_vddi;
+
+	err = regulator_enable(innolux->avee);
+	if (err < 0)
+		goto disable_avdd;
+
 	/* T2: 15ms - 1000ms */
 	usleep_range(15000, 16000);
 
@@ -134,12 +162,13 @@ static int innolux_panel_prepare(struct drm_panel *panel)
 	return 0;
 
 poweroff:
-	regulator_err = regulator_disable(innolux->supply);
-	if (regulator_err)
-		DRM_DEV_ERROR(panel->dev, "failed to disable regulator: %d\n",
-			      regulator_err);
-
 	gpiod_set_value_cansleep(innolux->enable_gpio, 0);
+	regulator_disable(innolux->avee);
+disable_avdd:
+	regulator_disable(innolux->avdd);
+disable_vddi:
+	regulator_disable(innolux->vddi);
+
 	return err;
 }
 
@@ -164,7 +193,7 @@ static int innolux_panel_enable(struct drm_panel *panel)
 	return 0;
 }
 
-static const struct drm_display_mode default_mode = {
+static const struct drm_display_mode innolux_p079zca_mode = {
 	.clock = 56900,
 	.hdisplay = 768,
 	.hsync_start = 768 + 40,
@@ -177,15 +206,31 @@ static const struct drm_display_mode default_mode = {
 	.vrefresh = 60,
 };
 
+static const struct panel_desc_dsi innolux_p079zca_panel_desc = {
+	.desc = {
+		.modes = &innolux_p079zca_mode,
+		.bpc = 8,
+		.size = {
+			.width = 120,
+			.height = 160,
+		},
+	},
+	.flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_SYNC_PULSE |
+		 MIPI_DSI_MODE_LPM,
+	.format = MIPI_DSI_FMT_RGB888,
+	.lanes = 4,
+};
+
 static int innolux_panel_get_modes(struct drm_panel *panel)
 {
 	struct drm_display_mode *mode;
+	struct innolux_panel *innolux = to_innolux_panel(panel);
+	const struct drm_display_mode *m = innolux->dsi_desc->desc.modes;
 
-	mode = drm_mode_duplicate(panel->drm, &default_mode);
+	mode = drm_mode_duplicate(panel->drm, m);
 	if (!mode) {
 		DRM_DEV_ERROR(panel->drm->dev, "failed to add mode %ux%ux@%u\n",
-			      default_mode.hdisplay, default_mode.vdisplay,
-			      default_mode.vrefresh);
+			      m->hdisplay, m->vdisplay, m->vrefresh);
 		return -ENOMEM;
 	}
 
@@ -193,9 +238,11 @@ static int innolux_panel_get_modes(struct drm_panel *panel)
 
 	drm_mode_probed_add(panel->connector, mode);
 
-	panel->connector->display_info.width_mm = 120;
-	panel->connector->display_info.height_mm = 160;
-	panel->connector->display_info.bpc = 8;
+	panel->connector->display_info.width_mm =
+			innolux->dsi_desc->desc.size.width;
+	panel->connector->display_info.height_mm =
+			innolux->dsi_desc->desc.size.height;
+	panel->connector->display_info.bpc = innolux->dsi_desc->desc.bpc;
 
 	return 1;
 }
@@ -209,20 +256,36 @@ static const struct drm_panel_funcs innolux_panel_funcs = {
 };
 
 static const struct of_device_id innolux_of_match[] = {
-	{ .compatible = "innolux,p079zca", },
-	{ }
+	{ .compatible = "innolux,p079zca",
+	  .data = &innolux_p079zca_panel_desc
+	},
 };
 MODULE_DEVICE_TABLE(of, innolux_of_match);
 
-static int innolux_panel_add(struct innolux_panel *innolux)
+static int innolux_panel_add(struct mipi_dsi_device *dsi,
+			     const struct panel_desc_dsi *desc)
 {
-	struct device *dev = &innolux->link->dev;
+	struct innolux_panel *innolux;
+	struct device *dev = &dsi->dev;
 	struct device_node *np;
 	int err;
 
-	innolux->supply = devm_regulator_get(dev, "power");
-	if (IS_ERR(innolux->supply))
-		return PTR_ERR(innolux->supply);
+	innolux = devm_kzalloc(dev, sizeof(*innolux), GFP_KERNEL);
+	if (!innolux)
+		return -ENOMEM;
+
+	innolux->dsi_desc = desc;
+	innolux->vddi = devm_regulator_get(dev, "power");
+	if (IS_ERR(innolux->vddi))
+		return PTR_ERR(innolux->vddi);
+
+	innolux->avdd = devm_regulator_get(dev, "avdd");
+	if (IS_ERR(innolux->avdd))
+		return PTR_ERR(innolux->avdd);
+
+	innolux->avee = devm_regulator_get(dev, "avee");
+	if (IS_ERR(innolux->avee))
+		return PTR_ERR(innolux->avee);
 
 	innolux->enable_gpio = devm_gpiod_get_optional(dev, "enable",
 						       GPIOD_OUT_HIGH);
@@ -243,14 +306,16 @@ static int innolux_panel_add(struct innolux_panel *innolux)
 
 	drm_panel_init(&innolux->base);
 	innolux->base.funcs = &innolux_panel_funcs;
-	innolux->base.dev = &innolux->link->dev;
+	innolux->base.dev = dev;
 
 	err = drm_panel_add(&innolux->base);
 	if (err < 0)
 		goto put_backlight;
 
-	return 0;
+	dev_set_drvdata(dev, innolux);
+	innolux->link = dsi;
 
+	return 0;
 put_backlight:
 	put_device(&innolux->backlight->dev);
 
@@ -267,28 +332,25 @@ static void innolux_panel_del(struct innolux_panel *innolux)
 
 static int innolux_panel_probe(struct mipi_dsi_device *dsi)
 {
-	struct innolux_panel *innolux;
-	int err;
 
-	dsi->lanes = 4;
-	dsi->format = MIPI_DSI_FMT_RGB888;
-	dsi->mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_SYNC_PULSE |
-			  MIPI_DSI_MODE_LPM;
-
-	innolux = devm_kzalloc(&dsi->dev, sizeof(*innolux), GFP_KERNEL);
-	if (!innolux)
-		return -ENOMEM;
+	const struct panel_desc_dsi *dsi_desc;
+	const struct of_device_id *id;
+	int err;
 
-	mipi_dsi_set_drvdata(dsi, innolux);
+	id = of_match_node(innolux_of_match, dsi->dev.of_node);
+	if (!id)
+		return -ENODEV;
 
-	innolux->link = dsi;
+	dsi_desc = id->data;
+	dsi->mode_flags = dsi_desc->flags;
+	dsi->format = dsi_desc->format;
+	dsi->lanes = dsi_desc->lanes;
 
-	err = innolux_panel_add(innolux);
+	err = innolux_panel_add(dsi, dsi_desc);
 	if (err < 0)
 		return err;
 
-	err = mipi_dsi_attach(dsi);
-	return err;
+	return mipi_dsi_attach(dsi);
 }
 
 static int innolux_panel_remove(struct mipi_dsi_device *dsi)
@@ -326,7 +388,7 @@ static void innolux_panel_shutdown(struct mipi_dsi_device *dsi)
 
 static struct mipi_dsi_driver innolux_panel_driver = {
 	.driver = {
-		.name = "panel-innolux-p079zca",
+		.name = "panel-innolux-dsi",
 		.of_match_table = innolux_of_match,
 	},
 	.probe = innolux_panel_probe,
@@ -336,5 +398,6 @@ static struct mipi_dsi_driver innolux_panel_driver = {
 module_mipi_dsi_driver(innolux_panel_driver);
 
 MODULE_AUTHOR("Chris Zhong <[email protected]>");
+MODULE_AUTHOR("Lin Huang <[email protected]>");
 MODULE_DESCRIPTION("Innolux P079ZCA panel driver");
 MODULE_LICENSE("GPL v2");
Signature · 1 line
2.7.4
author
#1 lore raw

PATCH RESEND v3 2/3 drm/panel: support Innolux P097PFG panel

Support Innolux P097PFG 9.7" 1536x2048 TFT LCD panel, it reuse the Innolux P079ZCA panel driver.

---
 drivers/gpu/drm/panel/Kconfig                 |  9 ++++----
 drivers/gpu/drm/panel/panel-innolux-p079zca.c | 31 +++++++++++++++++++++++++++
 2 files changed, 35 insertions(+), 5 deletions(-)
drivers/gpu/drm/panel/Kconfig +4 -5
index 726f3fb..429cf59 100644
@@ -29,15 +29,14 @@ config DRM_PANEL_SIMPLE
 	  low power state.
 
 config DRM_PANEL_INNOLUX_P079ZCA
-	tristate "Innolux P079ZCA panel"
+	tristate "Innolux P079ZCA or P097PFG panel"
 	depends on OF
 	depends on DRM_MIPI_DSI
 	depends on BACKLIGHT_CLASS_DEVICE
 	help
-	  Say Y here if you want to enable support for Innolux P079ZCA
-	  TFT-LCD modules. The panel has a 1024x768 resolution and uses
-	  24 bit RGB per pixel. It provides a MIPI DSI interface to
-	  the host and has a built-in LED backlight.
+	  Say Y here if you want to enable support for Innolux P079ZCA or
+	  Innolux P097PFG panel. They provide a MIPI DSI interface to
+	  the host and have a built-in LED backlight.
 
 config DRM_PANEL_JDI_LT070ME05000
 	tristate "JDI LT070ME05000 WUXGA DSI panel"
drivers/gpu/drm/panel/panel-innolux-p079zca.c +31 -0
index 1597744..5d690b7 100644
@@ -221,6 +221,34 @@ static const struct panel_desc_dsi innolux_p079zca_panel_desc = {
 	.lanes = 4,
 };
 
+static const struct drm_display_mode innolux_p097pfg_mode = {
+	.clock = 220000,
+	.hdisplay = 1536,
+	.hsync_start = 1536 + 100,
+	.hsync_end = 1536 + 100 + 24,
+	.htotal = 1536 + 100 + 24 + 100,
+	.vdisplay = 2048,
+	.vsync_start = 2048 + 18,
+	.vsync_end = 2048 + 18 + 2,
+	.vtotal = 2048 + 18 + 2 + 18,
+	.vrefresh = 60,
+};
+
+static const struct panel_desc_dsi innolux_p097pfg_panel_desc = {
+	.desc = {
+		.modes = &innolux_p097pfg_mode,
+		.bpc = 8,
+		.size = {
+			.width = 147,
+			.height = 196,
+		},
+	},
+	.flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_SYNC_PULSE |
+		 MIPI_DSI_MODE_LPM,
+	.format = MIPI_DSI_FMT_RGB888,
+	.lanes = 8,
+};
+
 static int innolux_panel_get_modes(struct drm_panel *panel)
 {
 	struct drm_display_mode *mode;
@@ -259,6 +287,9 @@ static const struct of_device_id innolux_of_match[] = {
 	{ .compatible = "innolux,p079zca",
 	  .data = &innolux_p079zca_panel_desc
 	},
+	{ .compatible = "innolux,p097pfg",
+	  .data = &innolux_p097pfg_panel_desc
+	}
 };
 MODULE_DEVICE_TABLE(of, innolux_of_match);
 
Signature · 1 line
2.7.4
author
#2 lore raw

PATCH RESEND v3 3/3 dt-bindings: Add INNOLUX P097PFG panel bindings

In reply to Lin Huang

The Innolux P097PFG panel is 9.7" panel with 1536X2048 resolution, it reuse P079ZCA panel driver, so improve p079ZCA dt-binding to support P097PFG.

---
 .../devicetree/bindings/display/panel/innolux,p079zca.txt     | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)
Documentation/devicetree/bindings/display/panel/innolux,p079zca.txt +9 -2
index d0f5516..8cadd8c 100644
@@ -1,13 +1,18 @@
 Innolux P079ZCA 7.85" 768x1024 TFT LCD panel
+Innolux P097PFG 9.7" 1536x2048 TFT LCD panel
 
 Required properties:
-- compatible: should be "innolux,p079zca"
+- compatible: should be should be one of the following.
+    -"innolux,p079zca" for Innolux 7.9" P079ZCA 768*1024 panel
+    -"innolux,p097pfg" for Innolux 9.7" P097PFG 1536*2048 panel
 - reg: DSI virtual channel of the peripheral
-- power-supply: phandle of the regulator that provides the supply voltage
 - enable-gpios: panel enable gpio
 
 Optional properties:
 - backlight: phandle of the backlight device attached to the panel
+- power-supply: phandle of the regulator that provides the supply voltage
+- avdd-supply: phandle of the regulator that provides positive voltage
+- avee-supply: phandle of the regulator that provides negative voltage
 
 Example:
 
@@ -16,6 +21,8 @@ Example:
 			compatible = "innolux,p079zca";
 			reg = <0>;
 			power-supply = <...>;
+			avdd-supply = <...>;
+			avee-supply = <...>;
 			backlight = <&backlight>;
 			enable-gpios = <&gpio1 13 GPIO_ACTIVE_HIGH>;
 		};
Signature · 1 line
2.7.4
#3 lore raw
In reply to Lin Huang

Hi HL,

Quoted 17 lines · Lin Huang

Refactor Innolux P079ZCA panel driver, let it support multi panel.

---
Changes in v2:
- Change regulator property name to meet the panel datasheet
Changes in v3:
- this patch only refactor P079ZCA panel to support multi panel, support P097PFG panel in another patch

 drivers/gpu/drm/panel/panel-innolux-p079zca.c | 147 ++++++++++++++++++--------
 1 file changed, 105 insertions(+), 42 deletions(-)
drivers/gpu/drm/panel/panel-innolux-p079zca.c +0 -0
index 6ba9344..1597744 100644

...

drivers/gpu/drm/panel/panel-innolux-p079zca.c static int innolux_panel_prepare(struct drm_panel *panel) · line 137

Quoted 5 lines
drivers/gpu/drm/panel/panel-innolux-p079zca.c +0 -1
@@ -134,12 +162,13 @@ static int innolux_panel_prepare(struct drm_panel *panel)
 	return 0;
 
 poweroff:
-	regulator_err = regulator_disable(innolux->supply);

I think 'regulator_err' is unused after this patch. Please remove it.

Quoted 14 lines
drivers/gpu/drm/panel/panel-innolux-p079zca.c +6 -4
-	if (regulator_err)
-		DRM_DEV_ERROR(panel->dev, "failed to disable regulator: %d\n",
-			      regulator_err);
-
 	gpiod_set_value_cansleep(innolux->enable_gpio, 0);
+	regulator_disable(innolux->avee);
+disable_avdd:
+	regulator_disable(innolux->avdd);
+disable_vddi:
+	regulator_disable(innolux->vddi);
+
 	return err;
 }
 

...

drivers/gpu/drm/panel/panel-innolux-p079zca.c static const struct drm_panel_funcs innolux_panel_funcs = { · line 213

Quoted 6 lines
drivers/gpu/drm/panel/panel-innolux-p079zca.c +0 -2
@@ -209,20 +256,36 @@ static const struct drm_panel_funcs innolux_panel_funcs = {
 };
 
 static const struct of_device_id innolux_of_match[] = {
-	{ .compatible = "innolux,p079zca", },
-	{ }

You're deleting this terminator. Please don't.

drivers/gpu/drm/panel/panel-innolux-p079zca.c static const struct drm_panel_funcs innolux_panel_funcs = { · line 264

Quoted 6 lines
drivers/gpu/drm/panel/panel-innolux-p079zca.c +3 -0
+	{ .compatible = "innolux,p079zca",
+	  .data = &innolux_p079zca_panel_desc
+	},
 };
 MODULE_DEVICE_TABLE(of, innolux_of_match);
 

...

Brian
_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel

#4 lore raw
In reply to Lin Huang
Quoted 27 lines · Lin Huang

Refactor Innolux P079ZCA panel driver, let it support multi panel.

---
Changes in v2:
- Change regulator property name to meet the panel datasheet
Changes in v3:
- this patch only refactor P079ZCA panel to support multi panel, support P097PFG panel in another patch

 drivers/gpu/drm/panel/panel-innolux-p079zca.c | 147 ++++++++++++++++++--------
 1 file changed, 105 insertions(+), 42 deletions(-)
drivers/gpu/drm/panel/panel-innolux-p079zca.c +6 -0
index 6ba9344..1597744 100644
@@ -20,12 +20,32 @@
 
 #include <video/mipi_display.h>
 
+struct panel_desc {
+	const struct drm_display_mode *modes;
+	unsigned int bpc;
+	struct {
+		unsigned int width;
+		unsigned int height;

drivers/gpu/drm/panel/panel-innolux-p079zca.c line 38

Quoted 10 lines
drivers/gpu/drm/panel/panel-innolux-p079zca.c +10 -0
+	} size;
+};
+
+struct panel_desc_dsi {
+	struct panel_desc desc;
+
+	unsigned long flags;
+	enum mipi_dsi_pixel_format format;
+	unsigned int lanes;
+};

There's no need for the two layers here. Just move everything from struct panel_desc_dsi into struct panel_desc.

drivers/gpu/drm/panel/panel-innolux-p079zca.c line 43

Quoted 5 lines
drivers/gpu/drm/panel/panel-innolux-p079zca.c +2 -0
+
 struct innolux_panel {
 	struct drm_panel base;
 	struct mipi_dsi_device *link;
+	const struct panel_desc_dsi *dsi_desc;

And then this can just become:

	const struct panel_desc *desc;

The _dsi suffix is redundant because this driver is exclusively for DSI devices.

Quoted 19 lines
drivers/gpu/drm/panel/panel-innolux-p079zca.c +10 -5
-static int innolux_panel_add(struct innolux_panel *innolux)
+static int innolux_panel_add(struct mipi_dsi_device *dsi,
+			     const struct panel_desc_dsi *desc)
 {
-	struct device *dev = &innolux->link->dev;
+	struct innolux_panel *innolux;
+	struct device *dev = &dsi->dev;
 	struct device_node *np;
 	int err;
 
-	innolux->supply = devm_regulator_get(dev, "power");
-	if (IS_ERR(innolux->supply))
-		return PTR_ERR(innolux->supply);
+	innolux = devm_kzalloc(dev, sizeof(*innolux), GFP_KERNEL);
+	if (!innolux)
+		return -ENOMEM;
+
+	innolux->dsi_desc = desc;
+	innolux->vddi = devm_regulator_get(dev, "power");

drivers/gpu/drm/panel/panel-innolux-p079zca.c static const struct drm_panel_funcs innolux_panel_funcs = { · line 288

Quoted 10 lines
drivers/gpu/drm/panel/panel-innolux-p079zca.c +10 -0
+	if (IS_ERR(innolux->vddi))
+		return PTR_ERR(innolux->vddi);
+
+	innolux->avdd = devm_regulator_get(dev, "avdd");
+	if (IS_ERR(innolux->avdd))
+		return PTR_ERR(innolux->avdd);
+
+	innolux->avee = devm_regulator_get(dev, "avee");
+	if (IS_ERR(innolux->avee))
+		return PTR_ERR(innolux->avee);

According to the device tree bindings these regulators are all optional. Now devm_regulator_get() will return a dummy regulator if one has not been specified in DT, so this seems like it should work fine, even for existing DTBs that don't have the avdd and avee regulators. However, the DT bindings seem to be wrong, because these are in fact required regulators.

Quoted 64 lines
drivers/gpu/drm/panel/panel-innolux-p079zca.c +16 -17
 	innolux->enable_gpio = devm_gpiod_get_optional(dev, "enable",
 						       GPIOD_OUT_HIGH);
@@ -243,14 +306,16 @@ static int innolux_panel_add(struct innolux_panel *innolux)
 
 	drm_panel_init(&innolux->base);
 	innolux->base.funcs = &innolux_panel_funcs;
-	innolux->base.dev = &innolux->link->dev;
+	innolux->base.dev = dev;
 
 	err = drm_panel_add(&innolux->base);
 	if (err < 0)
 		goto put_backlight;
 
-	return 0;
+	dev_set_drvdata(dev, innolux);
+	innolux->link = dsi;
 
+	return 0;
 put_backlight:
 	put_device(&innolux->backlight->dev);
 
@@ -267,28 +332,25 @@ static void innolux_panel_del(struct innolux_panel *innolux)
 
 static int innolux_panel_probe(struct mipi_dsi_device *dsi)
 {
-	struct innolux_panel *innolux;
-	int err;
 
-	dsi->lanes = 4;
-	dsi->format = MIPI_DSI_FMT_RGB888;
-	dsi->mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_SYNC_PULSE |
-			  MIPI_DSI_MODE_LPM;
-
-	innolux = devm_kzalloc(&dsi->dev, sizeof(*innolux), GFP_KERNEL);
-	if (!innolux)
-		return -ENOMEM;
+	const struct panel_desc_dsi *dsi_desc;
+	const struct of_device_id *id;
+	int err;
 
-	mipi_dsi_set_drvdata(dsi, innolux);
+	id = of_match_node(innolux_of_match, dsi->dev.of_node);
+	if (!id)
+		return -ENODEV;
 
-	innolux->link = dsi;
+	dsi_desc = id->data;
+	dsi->mode_flags = dsi_desc->flags;
+	dsi->format = dsi_desc->format;
+	dsi->lanes = dsi_desc->lanes;
 
-	err = innolux_panel_add(innolux);
+	err = innolux_panel_add(dsi, dsi_desc);
 	if (err < 0)
 		return err;
 
-	err = mipi_dsi_attach(dsi);
-	return err;
+	return mipi_dsi_attach(dsi);
 }
 
 static int innolux_panel_remove(struct mipi_dsi_device *dsi)

drivers/gpu/drm/panel/panel-innolux-p079zca.c static void innolux_panel_shutdown(struct mipi_dsi_device *dsi) · line 391

Quoted 5 lines
drivers/gpu/drm/panel/panel-innolux-p079zca.c +1 -1
@@ -326,7 +388,7 @@ static void innolux_panel_shutdown(struct mipi_dsi_device *dsi)
 
 static struct mipi_dsi_driver innolux_panel_driver = {
 	.driver = {
-		.name = "panel-innolux-p079zca",
+		.name = "panel-innolux-dsi",

No need to change that. Also, probably a bad idea to change it because there are (or will be) likely other DSI panels from Innolux that are not compatible with this driver.

Thierry

author
#5 lore raw

Hi Thierry Reding,

Quoted 32 lines · Thierry Reding
Quoted 31 lines · Lin Huang

Refactor Innolux P079ZCA panel driver, let it support multi panel.

---
Changes in v2:
- Change regulator property name to meet the panel datasheet
Changes in v3:
- this patch only refactor P079ZCA panel to support multi panel, support P097PFG panel in another patch

  drivers/gpu/drm/panel/panel-innolux-p079zca.c | 147 ++++++++++++++++++--------
  1 file changed, 105 insertions(+), 42 deletions(-)
drivers/gpu/drm/panel/panel-innolux-p079zca.c +10 -0
index 6ba9344..1597744 100644
@@ -20,12 +20,32 @@
 
  #include <video/mipi_display.h>
 
+struct panel_desc {
+	const struct drm_display_mode *modes;
+	unsigned int bpc;
+	struct {
+		unsigned int width;
+		unsigned int height;
+	} size;
+};
+
+struct panel_desc_dsi {
Quoted 8 lines

drivers/gpu/drm/panel/panel-innolux-p079zca.c line 38

Quoted 6 lines
drivers/gpu/drm/panel/panel-innolux-p079zca.c +6 -0
+	struct panel_desc desc;
+
+	unsigned long flags;
+	enum mipi_dsi_pixel_format format;
+	unsigned int lanes;
+};

There's no need for the two layers here. Just move everything from struct panel_desc_dsi into struct panel_desc.

Okay, will fix it.

Quoted 8 lines

drivers/gpu/drm/panel/panel-innolux-p079zca.c line 43

Quoted 6 lines
drivers/gpu/drm/panel/panel-innolux-p079zca.c +2 -0
+
  struct innolux_panel {
  	struct drm_panel base;
  	struct mipi_dsi_device *link;
+	const struct panel_desc_dsi *dsi_desc;

And then this can just become:

Quoted 5 lines
	const struct panel_desc *desc;

The _dsi suffix is redundant because this driver is exclusively for DSI devices.

Got it.

Quoted 27 lines
Quoted 27 lines
drivers/gpu/drm/panel/panel-innolux-p079zca.c +18 -5
-static int innolux_panel_add(struct innolux_panel *innolux)
+static int innolux_panel_add(struct mipi_dsi_device *dsi,
+			     const struct panel_desc_dsi *desc)
  {
-	struct device *dev = &innolux->link->dev;
+	struct innolux_panel *innolux;
+	struct device *dev = &dsi->dev;
  	struct device_node *np;
  	int err;
 
-	innolux->supply = devm_regulator_get(dev, "power");
-	if (IS_ERR(innolux->supply))
-		return PTR_ERR(innolux->supply);
+	innolux = devm_kzalloc(dev, sizeof(*innolux), GFP_KERNEL);
+	if (!innolux)
+		return -ENOMEM;
+
+	innolux->dsi_desc = desc;
+	innolux->vddi = devm_regulator_get(dev, "power");
+	if (IS_ERR(innolux->vddi))
+		return PTR_ERR(innolux->vddi);
+
+	innolux->avdd = devm_regulator_get(dev, "avdd");
+	if (IS_ERR(innolux->avdd))
+		return PTR_ERR(innolux->avdd);
+
+	innolux->avee = devm_regulator_get(dev, "avee");
Quoted 8 lines

drivers/gpu/drm/panel/panel-innolux-p079zca.c static const struct drm_panel_funcs innolux_panel_funcs = { · line 288

Quoted 2 lines
drivers/gpu/drm/panel/panel-innolux-p079zca.c +2 -0
+	if (IS_ERR(innolux->avee))
+		return PTR_ERR(innolux->avee);

According to the device tree bindings these regulators are all optional. Now devm_regulator_get() will return a dummy regulator if one has not been specified in DT, so this seems like it should work fine, even for existing DTBs that don't have the avdd and avee regulators. However, the DT bindings seem to be wrong, because these are in fact required regulators.

Actually, the vddi is request, and avdd and avee is optional, i will only check vddi error later.

Quoted 65 lines
Quoted 65 lines

innolux->enable_gpio = devm_gpiod_get_optional(dev, "enable",
GPIOD_OUT_HIGH);

drivers/gpu/drm/panel/panel-innolux-p079zca.c +16 -17
@@ -243,14 +306,16 @@ static int innolux_panel_add(struct innolux_panel *innolux)
 
  	drm_panel_init(&innolux->base);
  	innolux->base.funcs = &innolux_panel_funcs;
-	innolux->base.dev = &innolux->link->dev;
+	innolux->base.dev = dev;
 
  	err = drm_panel_add(&innolux->base);
  	if (err < 0)
  		goto put_backlight;
 
-	return 0;
+	dev_set_drvdata(dev, innolux);
+	innolux->link = dsi;
 
+	return 0;
  put_backlight:
  	put_device(&innolux->backlight->dev);
 
@@ -267,28 +332,25 @@ static void innolux_panel_del(struct innolux_panel *innolux)
 
  static int innolux_panel_probe(struct mipi_dsi_device *dsi)
  {
-	struct innolux_panel *innolux;
-	int err;
 
-	dsi->lanes = 4;
-	dsi->format = MIPI_DSI_FMT_RGB888;
-	dsi->mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_SYNC_PULSE |
-			  MIPI_DSI_MODE_LPM;
-
-	innolux = devm_kzalloc(&dsi->dev, sizeof(*innolux), GFP_KERNEL);
-	if (!innolux)
-		return -ENOMEM;
+	const struct panel_desc_dsi *dsi_desc;
+	const struct of_device_id *id;
+	int err;
 
-	mipi_dsi_set_drvdata(dsi, innolux);
+	id = of_match_node(innolux_of_match, dsi->dev.of_node);
+	if (!id)
+		return -ENODEV;
 
-	innolux->link = dsi;
+	dsi_desc = id->data;
+	dsi->mode_flags = dsi_desc->flags;
+	dsi->format = dsi_desc->format;
+	dsi->lanes = dsi_desc->lanes;
 
-	err = innolux_panel_add(innolux);
+	err = innolux_panel_add(dsi, dsi_desc);
  	if (err < 0)
  		return err;
 
-	err = mipi_dsi_attach(dsi);
-	return err;
+	return mipi_dsi_attach(dsi);
  }
 
  static int innolux_panel_remove(struct mipi_dsi_device *dsi)
@@ -326,7 +388,7 @@ static void innolux_panel_shutdown(struct mipi_dsi_device *dsi)
 
Quoted 7 lines

drivers/gpu/drm/panel/panel-innolux-p079zca.c static void innolux_panel_shutdown(struct mipi_dsi_device *dsi) · line 391

Quoted 4 lines
drivers/gpu/drm/panel/panel-innolux-p079zca.c +1 -1
  static struct mipi_dsi_driver innolux_panel_driver = {
  	.driver = {
-		.name = "panel-innolux-p079zca",
+		.name = "panel-innolux-dsi",

No need to change that. Also, probably a bad idea to change it because there are (or will be) likely other DSI panels from Innolux that are not compatible with this driver.

Okay, will fix it.

Quoted 2 lines

Thierry