Hi,
On Mon, Mar 09, 2020 at 10:53:04AM +0530, Harigovindan P wrote:
Add support for Visionox panel driver.
Signed-off-by: Harigovindan P harigovi@codeaurora.org
Changes in v2:
- Dropping redundant space in Kconfig(Sam Ravnborg).
- Changing structure for include files(Sam Ravnborg).
- Removing backlight related code and functions(Sam Ravnborg).
- Removing repeated printing of error message(Sam Ravnborg).
- Adding drm_connector as an argument for get_modes function.
Changes in v3:
- Adding arguments for drm_panel_init to support against mainline.
Changes in v4:
- Removing error messages from regulator_set_load.
- Removing dev struct entry.
- Removing checks.
- Dropping empty comment lines.
Changes in v5:
- Removing unused struct member variables.
- Removing blank lines.
- Fixed indentation.
- Invoking dsi_detach and panel_remove while early exiting from probe.
drivers/gpu/drm/panel/Kconfig | 8 + drivers/gpu/drm/panel/Makefile | 1 + .../gpu/drm/panel/panel-visionox-rm69299.c | 315 ++++++++++++++++++ 3 files changed, 324 insertions(+) create mode 100644 drivers/gpu/drm/panel/panel-visionox-rm69299.c
...
diff --git a/drivers/gpu/drm/panel/panel-visionox-rm69299.c b/drivers/gpu/drm/panel/panel-visionox-rm69299.c new file mode 100644 index 000000000000..2bd3af46d933 --- /dev/null +++ b/drivers/gpu/drm/panel/panel-visionox-rm69299.c
...
+static int visionox_35597_power_on(struct visionox_rm69299 *ctx) +{
s/35597/rm69299/ ?
+static const struct rm69299_config rm69299_dir = {
- .width_mm = 74,
- .height_mm = 131,
- .dm = &visionox_rm69299_1080x2248_60hz,
+};
Are there actually variants of the panel with different sizes? So far the driver supports a single type of panel, so I would say struct rm69299_config is not needed. It can be added later if the driver ever gets support for other panel variants. For now just assign the values directly in 'visionox_rm69299_get_modes'.
+static int visionox_rm69299_probe(struct mipi_dsi_device *dsi) +{
- struct device *dev = &dsi->dev;
- struct visionox_rm69299 *ctx;
- int ret;
- ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
- if (!ctx)
return -ENOMEM;- mipi_dsi_set_drvdata(dsi, ctx);
- ctx->supplies[0].supply = "vdda";
- ctx->supplies[1].supply = "vdd3p3";
- ret = devm_regulator_bulk_get(ctx->panel.dev, ARRAY_SIZE(ctx->supplies),
ctx->supplies);
nit: alignment is odd, either align with a tab after 'devm_regulator_bulk_get' or with 'ctx->panel.dev'.
- if (ret < 0)
return ret;- ctx->reset_gpio = devm_gpiod_get(ctx->panel.dev, "reset", GPIOD_OUT_LOW);
- if (IS_ERR(ctx->reset_gpio)) {
DRM_DEV_ERROR(dev, "cannot get reset gpio %ld\n",PTR_ERR(ctx->reset_gpio));return PTR_ERR(ctx->reset_gpio);- }
- drm_panel_init(&ctx->panel, dev, &visionox_rm69299_drm_funcs,
DRM_MODE_CONNECTOR_DSI);- ctx->panel.dev = dev;
- ctx->panel.funcs = &visionox_rm69299_drm_funcs;
- drm_panel_add(&ctx->panel);
- dsi->lanes = 4;
- dsi->format = MIPI_DSI_FMT_RGB888;
- dsi->mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_LPM |
MIPI_DSI_CLOCK_NON_CONTINUOUS;- ret = mipi_dsi_attach(dsi);
- if (ret < 0) {
DRM_DEV_ERROR(dev, "dsi attach failed ret = %d\n", ret);goto err_dsi_attach;- }
- ret = regulator_set_load(ctx->supplies[0].consumer, 32000);
- if (ret) {
mipi_dsi_detach(dsi);drm_panel_remove(&ctx->panel);
that's technically correct, but since you have the 'goto' above and do the same unwinding for the other 'regulator_set_load' call below it would be better to use a 'goto' here (and below) too. Actually the 'goto' above only makes sense if 'goto' is also used for the other cases.
return ret;- }
- ret = regulator_set_load(ctx->supplies[1].consumer, 13200);
- if (ret) {
mipi_dsi_detach(dsi);drm_panel_remove(&ctx->panel);return ret;- }
- return 0;
+err_dsi_attach:
- drm_panel_remove(&ctx->panel);
- return ret;
+}
+static int visionox_rm69299_remove(struct mipi_dsi_device *dsi) +{
- struct visionox_rm69299 *ctx = mipi_dsi_get_drvdata(dsi);
- mipi_dsi_detach(ctx->dsi);
- mipi_dsi_device_unregister(ctx->dsi);
- drm_panel_remove(&ctx->panel);
- return 0;
+}
+static const struct of_device_id visionox_rm69299_of_match[] = {
- {
.compatible = "visionox,rm69299-1080p-display",.data = &rm69299_dir,- },
+}; +MODULE_DEVICE_TABLE(of, visionox_rm69299_of_match);
+static struct mipi_dsi_driver visionox_rm69299_driver = {
- .driver = {
.name = "panel-visionox-rm69299",.of_match_table = visionox_rm69299_of_match,- },
- .probe = visionox_rm69299_probe,
- .remove = visionox_rm69299_remove,
+}; +module_mipi_dsi_driver(visionox_rm69299_driver);
+MODULE_DESCRIPTION("VISIONOX RM69299 DSI Panel Driver");
As commented on v4, this should be 'Visionox'.