Hi,
Changes since v1 (only panel-osd-osd101t2587-53ts changed):
- Removed unused members from struct osd101t2587_panel
- Use backlight_enable/backlight_disable
- Use devm_of_find_backlight()
- osd101t2587_of_match table standardized
- osd101t2587_panel_unprepare() added to shutdown and remove callbacks to turn
power off
- Fix probe in case mipi_dsi_attach() would fail
Add support for OSD101T2045-53TS and OSD101T2587-53TS from One Stop Displays.
The two panel is similar with one big difference: OSD101T2587-53TS requires the
MIPI_DSI_TURN_ON_PERIPHERAL message, thus can not be handled by panel-simple.
Regards,
Peter
---
Peter Ujfalusi (4):
dt-bindings: display: Add bindings for OSD101T2045-53TS
drm/panel: simple: Add support for OSD101T2045-53TS
dt-bindings: display: Add bindings for OSD101T2587-53TS panel
drm/panel: Add OSD101T2587-53TS driver
.../display/panel/osd,osd101t2045-53ts.txt | 11 +
.../display/panel/osd,osd101t2587-53ts.txt | 14 +
drivers/gpu/drm/panel/Kconfig | 6 +
drivers/gpu/drm/panel/Makefile | 1 +
.../drm/panel/panel-osd-osd101t2587-53ts.c | 254 ++++++++++++++++++
drivers/gpu/drm/panel/panel-simple.c | 34 +++
6 files changed, 320 insertions(+)
create mode 100644 Documentation/devicetree/bindings/display/panel/osd,osd101t2045-53ts.txt
create mode 100644 Documentation/devicetree/bindings/display/panel/osd,osd101t2587-53ts.txt
create mode 100644 drivers/gpu/drm/panel/panel-osd-osd101t2587-53ts.c
--
Peter
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
This adds the device-tree bindings for the OSD101T2045-53TS 10.1"
1920x1200 panel from One Stop Displays.
Signed-off-by: Peter Ujfalusi <[email protected]>
---
.../bindings/display/panel/osd,osd101t2045-53ts.txt | 11 +++++++++++
1 file changed, 11 insertions(+)
create mode 100644 Documentation/devicetree/bindings/display/panel/osd,osd101t2045-53ts.txt
diff --git a/Documentation/devicetree/bindings/display/panel/osd,osd101t2045-53ts.txt b/Documentation/devicetree/bindings/display/panel/osd,osd101t2045-53ts.txt
new file mode 100644
index 000000000000..b3f6df59f7c1
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/panel/osd,osd101t2045-53ts.txt
@@ -0,0 +1,11 @@
+One Stop Displays OSD101T2045-53TS 10.1" 1920x1200 panel
+
+Required properties:
+- compatible: should be "osd,osd101t2045-53ts"
+- power-supply: as specified in the base binding
+
+Optional properties:
+- backlight: as specified in the base binding
+
+This binding is compatible with the simple-panel binding, which is specified
+in simple-panel.txt in this directory.
--
Peter
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
Add support for the OSD101T2045-53TS 10.1" 1920x1200 panel from One Stop
Displays to the panel-simple driver
Signed-off-by: Peter Ujfalusi <[email protected]>
---
drivers/gpu/drm/panel/panel-simple.c | 34 ++++++++++++++++++++++++++++
1 file changed, 34 insertions(+)
diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c
index 9e8218f6a3f2..0337583ffa49 100644
--- a/drivers/gpu/drm/panel/panel-simple.c
+++ b/drivers/gpu/drm/panel/panel-simple.c
@@ -2996,6 +2996,37 @@ static const struct panel_desc_dsi panasonic_vvx10f004b00 = {
.lanes = 4,
};
+static const struct drm_display_mode osd101t2045_53ts_mode = {
+ .clock = 154500,
+ .hdisplay = 1920,
+ .hsync_start = 1920 + 112,
+ .hsync_end = 1920 + 112 + 16,
+ .htotal = 1920 + 112 + 16 + 32,
+ .vdisplay = 1200,
+ .vsync_start = 1200 + 16,
+ .vsync_end = 1200 + 16 + 2,
+ .vtotal = 1200 + 16 + 2 + 16,
+ .vrefresh = 60,
+ .flags = DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC,
+};
+
+static const struct panel_desc_dsi osd101t2045_53ts = {
+ .desc = {
+ .modes = &osd101t2045_53ts_mode,
+ .num_modes = 1,
+ .bpc = 8,
+ .size = {
+ .width = 217,
+ .height = 136,
+ },
+ },
+ .flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_BURST |
+ MIPI_DSI_MODE_VIDEO_SYNC_PULSE |
+ MIPI_DSI_MODE_EOT_PACKET,
+ .format = MIPI_DSI_FMT_RGB888,
+ .lanes = 4,
+};
+
static const struct of_device_id dsi_of_match[] = {
{
.compatible = "auo,b080uan01",
@@ -3012,6 +3043,9 @@ static const struct of_device_id dsi_of_match[] = {
}, {
.compatible = "panasonic,vvx10f004b00",
.data = &panasonic_vvx10f004b00
+ }, {
+ .compatible = "osd,osd101t2045-53ts",
+ .data = &osd101t2045_53ts
}, {
/* sentinel */
}
--
Peter
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
This adds the device-tree bindings for the OSD101T2587-53TS 10.1"
1920x1200 panel from One Stop Displays.
Note: the panel is similar to OSD101T2045-53TS, but it needs additional
MIPI_DSI_TURN_ON_PERIPHERAL message from the host.
Signed-off-by: Peter Ujfalusi <[email protected]>
---
.../display/panel/osd,osd101t2587-53ts.txt | 14 ++++++++++++++
1 file changed, 14 insertions(+)
create mode 100644 Documentation/devicetree/bindings/display/panel/osd,osd101t2587-53ts.txt
diff --git a/Documentation/devicetree/bindings/display/panel/osd,osd101t2587-53ts.txt b/Documentation/devicetree/bindings/display/panel/osd,osd101t2587-53ts.txt
new file mode 100644
index 000000000000..2082cae1a0e3
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/panel/osd,osd101t2587-53ts.txt
@@ -0,0 +1,14 @@
+One Stop Displays OSD101T2587-53TS 10.1" 1920x1200 panel
+
+The panel is similar to OSD101T2045-53TS, but it needs additional
+MIPI_DSI_TURN_ON_PERIPHERAL message from the host.
+
+Required properties:
+- compatible: should be "osd,osd101t2587-53ts"
+- power-supply: as specified in the base binding
+
+Optional properties:
+- backlight: as specified in the base binding
+
+This binding is compatible with the simple-panel binding, which is specified
+in simple-panel.txt in this directory.
--
Peter
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
The panel is similar to OSD101T2045-53TS (which is handled by panel-simple)
with one big difference: osd101t2587-53ts needs MIPI_DSI_TURN_ON_PERIPHERAL
message to be sent from the host to be operational and thus can not be
handled by panel-simple.
Signed-off-by: Peter Ujfalusi <[email protected]>
---
drivers/gpu/drm/panel/Kconfig | 6 +
drivers/gpu/drm/panel/Makefile | 1 +
.../drm/panel/panel-osd-osd101t2587-53ts.c | 254 ++++++++++++++++++
3 files changed, 261 insertions(+)
create mode 100644 drivers/gpu/drm/panel/panel-osd-osd101t2587-53ts.c
diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
index 3e070153ef21..351661920838 100644
--- a/drivers/gpu/drm/panel/Kconfig
+++ b/drivers/gpu/drm/panel/Kconfig
@@ -122,6 +122,12 @@ config DRM_PANEL_ORISETECH_OTM8009A
Say Y here if you want to enable support for Orise Technology
otm8009a 480x800 dsi 2dl panel.
+config DRM_PANEL_OSD_OSD101T2587_53TS
+ tristate "OSD OSD101T2587-53TS DSI 1920x1200 video mode panel"
+ depends on OF
+ depends on DRM_MIPI_DSI
+ depends on BACKLIGHT_CLASS_DEVICE
+
config DRM_PANEL_PANASONIC_VVX10F034N00
tristate "Panasonic VVX10F034N00 1920x1200 video mode panel"
depends on OF
diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
index e7ab71968bbf..d9d99956db0c 100644
--- a/drivers/gpu/drm/panel/Makefile
+++ b/drivers/gpu/drm/panel/Makefile
@@ -10,6 +10,7 @@ obj-$(CONFIG_DRM_PANEL_KINGDISPLAY_KD097D04) += panel-kingdisplay-kd097d04.o
obj-$(CONFIG_DRM_PANEL_LG_LG4573) += panel-lg-lg4573.o
obj-$(CONFIG_DRM_PANEL_OLIMEX_LCD_OLINUXINO) += panel-olimex-lcd-olinuxino.o
obj-$(CONFIG_DRM_PANEL_ORISETECH_OTM8009A) += panel-orisetech-otm8009a.o
+obj-$(CONFIG_DRM_PANEL_OSD_OSD101T2587_53TS) += panel-osd-osd101t2587-53ts.o
obj-$(CONFIG_DRM_PANEL_PANASONIC_VVX10F034N00) += panel-panasonic-vvx10f034n00.o
obj-$(CONFIG_DRM_PANEL_RASPBERRYPI_TOUCHSCREEN) += panel-raspberrypi-touchscreen.o
obj-$(CONFIG_DRM_PANEL_RAYDIUM_RM68200) += panel-raydium-rm68200.o
diff --git a/drivers/gpu/drm/panel/panel-osd-osd101t2587-53ts.c b/drivers/gpu/drm/panel/panel-osd-osd101t2587-53ts.c
new file mode 100644
index 000000000000..196a2a883d17
--- /dev/null
+++ b/drivers/gpu/drm/panel/panel-osd-osd101t2587-53ts.c
@@ -0,0 +1,254 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2019 Texas Instruments Incorporated - http://www.ti.com
+ * Author: Peter Ujfalusi <[email protected]>
+ */
+
+#include <linux/backlight.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/regulator/consumer.h>
+
+#include <drm/drm_device.h>
+#include <drm/drm_crtc.h>
+#include <drm/drm_mipi_dsi.h>
+#include <drm/drm_panel.h>
+
+#include <video/mipi_display.h>
+
+struct osd101t2587_panel {
+ struct drm_panel base;
+ struct mipi_dsi_device *dsi;
+
+ struct backlight_device *backlight;
+ struct regulator *supply;
+
+ bool prepared;
+ bool enabled;
+
+ const struct drm_display_mode *default_mode;
+};
+
+static inline struct osd101t2587_panel *to_osd101t2587_panel(struct drm_panel *panel)
+{
+ return container_of(panel, struct osd101t2587_panel, base);
+}
+
+static int osd101t2587_panel_disable(struct drm_panel *panel)
+{
+ struct osd101t2587_panel *osd101t2587 = to_osd101t2587_panel(panel);
+ int ret;
+
+ if (!osd101t2587->enabled)
+ return 0;
+
+ backlight_disable(osd101t2587->backlight);
+
+ ret = mipi_dsi_shutdown_peripheral(osd101t2587->dsi);
+
+ osd101t2587->enabled = false;
+
+ return ret;
+}
+
+static int osd101t2587_panel_unprepare(struct drm_panel *panel)
+{
+ struct osd101t2587_panel *osd101t2587 = to_osd101t2587_panel(panel);
+
+ if (!osd101t2587->prepared)
+ return 0;
+
+ regulator_disable(osd101t2587->supply);
+ osd101t2587->prepared = false;
+
+ return 0;
+}
+
+static int osd101t2587_panel_prepare(struct drm_panel *panel)
+{
+ struct osd101t2587_panel *osd101t2587 = to_osd101t2587_panel(panel);
+ int ret;
+
+ if (osd101t2587->prepared)
+ return 0;
+
+ ret = regulator_enable(osd101t2587->supply);
+ if (!ret)
+ osd101t2587->prepared = true;
+
+ return ret;
+}
+
+static int osd101t2587_panel_enable(struct drm_panel *panel)
+{
+ struct osd101t2587_panel *osd101t2587 = to_osd101t2587_panel(panel);
+ int ret;
+
+ if (osd101t2587->enabled)
+ return 0;
+
+ ret = mipi_dsi_turn_on_peripheral(osd101t2587->dsi);
+ if (ret)
+ return ret;
+
+ backlight_enable(osd101t2587->backlight);
+
+ osd101t2587->enabled = true;
+
+ return ret;
+}
+
+static const struct drm_display_mode default_mode_osd101t2587 = {
+ .clock = 164400,
+ .hdisplay = 1920,
+ .hsync_start = 1920 + 152,
+ .hsync_end = 1920 + 152 + 52,
+ .htotal = 1920 + 152 + 52 + 20,
+ .vdisplay = 1200,
+ .vsync_start = 1200 + 24,
+ .vsync_end = 1200 + 24 + 6,
+ .vtotal = 1200 + 24 + 6 + 48,
+ .vrefresh = 60,
+ .flags = DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC,
+};
+
+static int osd101t2587_panel_get_modes(struct drm_panel *panel)
+{
+ struct osd101t2587_panel *osd101t2587 = to_osd101t2587_panel(panel);
+ struct drm_display_mode *mode;
+
+ mode = drm_mode_duplicate(panel->drm, osd101t2587->default_mode);
+ if (!mode) {
+ dev_err(panel->drm->dev, "failed to add mode %ux%ux@%u\n",
+ osd101t2587->default_mode->hdisplay,
+ osd101t2587->default_mode->vdisplay,
+ osd101t2587->default_mode->vrefresh);
+ return -ENOMEM;
+ }
+
+ drm_mode_set_name(mode);
+
+ drm_mode_probed_add(panel->connector, mode);
+
+ panel->connector->display_info.width_mm = 217;
+ panel->connector->display_info.height_mm = 136;
+
+ return 1;
+}
+
+static const struct drm_panel_funcs osd101t2587_panel_funcs = {
+ .disable = osd101t2587_panel_disable,
+ .unprepare = osd101t2587_panel_unprepare,
+ .prepare = osd101t2587_panel_prepare,
+ .enable = osd101t2587_panel_enable,
+ .get_modes = osd101t2587_panel_get_modes,
+};
+
+static const struct of_device_id osd101t2587_of_match[] = {
+ {
+ .compatible = "osd,osd101t2587-53ts",
+ .data = &default_mode_osd101t2587,
+ }, {
+ /* sentinel */
+ }
+};
+MODULE_DEVICE_TABLE(of, osd101t2587_of_match);
+
+static int osd101t2587_panel_add(struct osd101t2587_panel *osd101t2587)
+{
+ struct device *dev = &osd101t2587->dsi->dev;
+
+ osd101t2587->supply = devm_regulator_get(dev, "power");
+ if (IS_ERR(osd101t2587->supply))
+ return PTR_ERR(osd101t2587->supply);
+
+ osd101t2587->backlight = devm_of_find_backlight(dev);
+ if (IS_ERR(osd101t2587->backlight))
+ return PTR_ERR(osd101t2587->backlight);
+
+ drm_panel_init(&osd101t2587->base);
+ osd101t2587->base.funcs = &osd101t2587_panel_funcs;
+ osd101t2587->base.dev = &osd101t2587->dsi->dev;
+
+ return drm_panel_add(&osd101t2587->base);
+}
+
+static int osd101t2587_panel_probe(struct mipi_dsi_device *dsi)
+{
+ struct osd101t2587_panel *osd101t2587;
+ const struct of_device_id *id;
+ int ret;
+
+ id = of_match_node(osd101t2587_of_match, dsi->dev.of_node);
+ if (!id)
+ return -ENODEV;
+
+ dsi->lanes = 4;
+ dsi->format = MIPI_DSI_FMT_RGB888;
+ dsi->mode_flags = MIPI_DSI_MODE_VIDEO |
+ MIPI_DSI_MODE_VIDEO_BURST |
+ MIPI_DSI_MODE_VIDEO_SYNC_PULSE |
+ MIPI_DSI_MODE_EOT_PACKET;
+
+ osd101t2587 = devm_kzalloc(&dsi->dev, sizeof(*osd101t2587), GFP_KERNEL);
+ if (!osd101t2587)
+ return -ENOMEM;
+
+ mipi_dsi_set_drvdata(dsi, osd101t2587);
+
+ osd101t2587->dsi = dsi;
+ osd101t2587->default_mode = id->data;
+
+ ret = osd101t2587_panel_add(osd101t2587);
+ if (ret < 0)
+ return ret;
+
+ ret = mipi_dsi_attach(dsi);
+ if (ret)
+ drm_panel_remove(&osd101t2587->base);
+
+ return ret;
+}
+
+static int osd101t2587_panel_remove(struct mipi_dsi_device *dsi)
+{
+ struct osd101t2587_panel *osd101t2587 = mipi_dsi_get_drvdata(dsi);
+ int ret;
+
+ ret = osd101t2587_panel_disable(&osd101t2587->base);
+ if (ret < 0)
+ dev_warn(&dsi->dev, "failed to disable panel\n");
+
+ osd101t2587_panel_unprepare(&osd101t2587->base);
+
+ drm_panel_remove(&osd101t2587->base);
+
+ ret = mipi_dsi_detach(dsi);
+ if (ret < 0)
+ dev_warn(&dsi->dev, "failed to detach from DSI host\n");
+
+ return ret;
+}
+
+static void osd101t2587_panel_shutdown(struct mipi_dsi_device *dsi)
+{
+ struct osd101t2587_panel *osd101t2587 = mipi_dsi_get_drvdata(dsi);
+
+ osd101t2587_panel_disable(&osd101t2587->base);
+ osd101t2587_panel_unprepare(&osd101t2587->base);
+}
+
+static struct mipi_dsi_driver osd101t2587_panel_driver = {
+ .driver = {
+ .name = "panel-osd-osd101t2587-53ts",
+ .of_match_table = osd101t2587_of_match,
+ },
+ .probe = osd101t2587_panel_probe,
+ .remove = osd101t2587_panel_remove,
+ .shutdown = osd101t2587_panel_shutdown,
+};
+module_mipi_dsi_driver(osd101t2587_panel_driver);
+
+MODULE_AUTHOR("Peter Ujfalusi <[email protected]>");
+MODULE_DESCRIPTION("OSD101T2587-53TS DSI panel");
+MODULE_LICENSE("GPL v2");
--
Peter
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
On Fri, 22 Feb 2019 15:16:15 +0200, Peter Ujfalusi wrote:
> This adds the device-tree bindings for the OSD101T2045-53TS 10.1"
> 1920x1200 panel from One Stop Displays.
>
> Signed-off-by: Peter Ujfalusi <[email protected]>
> ---
> .../bindings/display/panel/osd,osd101t2045-53ts.txt | 11 +++++++++++
> 1 file changed, 11 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/display/panel/osd,osd101t2045-53ts.txt
>
Reviewed-by: Rob Herring <[email protected]>
On Fri, 22 Feb 2019 15:16:17 +0200, Peter Ujfalusi wrote:
> This adds the device-tree bindings for the OSD101T2587-53TS 10.1"
> 1920x1200 panel from One Stop Displays.
>
> Note: the panel is similar to OSD101T2045-53TS, but it needs additional
> MIPI_DSI_TURN_ON_PERIPHERAL message from the host.
>
> Signed-off-by: Peter Ujfalusi <[email protected]>
> ---
> .../display/panel/osd,osd101t2587-53ts.txt | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/display/panel/osd,osd101t2587-53ts.txt
>
Reviewed-by: Rob Herring <[email protected]>
Hi Peter.
Driver looks to be in good shape now.
With the few comments below addressed you can add my:
Reviewed-by: Sam Ravnborg <[email protected]>
Sam
On Fri, Feb 22, 2019 at 03:16:18PM +0200, Peter Ujfalusi wrote:
> The panel is similar to OSD101T2045-53TS (which is handled by panel-simple)
> with one big difference: osd101t2587-53ts needs MIPI_DSI_TURN_ON_PERIPHERAL
> message to be sent from the host to be operational and thus can not be
> handled by panel-simple.
>
> Signed-off-by: Peter Ujfalusi <[email protected]>
> ---
> drivers/gpu/drm/panel/Kconfig | 6 +
> drivers/gpu/drm/panel/Makefile | 1 +
> .../drm/panel/panel-osd-osd101t2587-53ts.c | 254 ++++++++++++++++++
> 3 files changed, 261 insertions(+)
> create mode 100644 drivers/gpu/drm/panel/panel-osd-osd101t2587-53ts.c
>
> diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
> index 3e070153ef21..351661920838 100644
> --- a/drivers/gpu/drm/panel/Kconfig
> +++ b/drivers/gpu/drm/panel/Kconfig
> @@ -122,6 +122,12 @@ config DRM_PANEL_ORISETECH_OTM8009A
> Say Y here if you want to enable support for Orise Technology
> otm8009a 480x800 dsi 2dl panel.
>
> +config DRM_PANEL_OSD_OSD101T2587_53TS
> + tristate "OSD OSD101T2587-53TS DSI 1920x1200 video mode panel"
> + depends on OF
> + depends on DRM_MIPI_DSI
> + depends on BACKLIGHT_CLASS_DEVICE
Please add a help-text
> +
> +static int osd101t2587_panel_unprepare(struct drm_panel *panel)
> +{
> + struct osd101t2587_panel *osd101t2587 = to_osd101t2587_panel(panel);
> +
> + if (!osd101t2587->prepared)
> + return 0;
> +
> + regulator_disable(osd101t2587->supply);
> + osd101t2587->prepared = false;
> +
> + return 0;
> +}
> +
> +static int osd101t2587_panel_prepare(struct drm_panel *panel)
> +{
> + struct osd101t2587_panel *osd101t2587 = to_osd101t2587_panel(panel);
> + int ret;
> +
> + if (osd101t2587->prepared)
> + return 0;
> +
> + ret = regulator_enable(osd101t2587->supply);
> + if (!ret)
> + osd101t2587->prepared = true;
Logic is wrong here. regulator_enable() will return a negative value on error
and 0 in the good case.
So osd101t2587->prepared is set to true only in the error case, not in the good case.
> +
> + ret = mipi_dsi_attach(dsi);
> + if (ret)
> + drm_panel_remove(&osd101t2587->base);
I do not see panel-simple.c do a drm_panel_remove() if mipi_dsi_attach() fails.
Maybe the driver core will call remove() is probe fails?
Or maybe panel-simple() should call drm_panel_remove()
Keep the above as is - I just wanted to express that this looks different
from the panle-simple() driver.
> +static int osd101t2587_panel_remove(struct mipi_dsi_device *dsi)
> +{
> + struct osd101t2587_panel *osd101t2587 = mipi_dsi_get_drvdata(dsi);
> + int ret;
> +
> + ret = osd101t2587_panel_disable(&osd101t2587->base);
> + if (ret < 0)
> + dev_warn(&dsi->dev, "failed to disable panel\n");
This is already warned in lower layers and I think you could
drop the dev_warn().
> +
> + osd101t2587_panel_unprepare(&osd101t2587->base);
> +
> + drm_panel_remove(&osd101t2587->base);
> +
> + ret = mipi_dsi_detach(dsi);
> + if (ret < 0)
> + dev_warn(&dsi->dev, "failed to detach from DSI host\n");
Add error code in logging.
Hi Sam,
On 23/02/2019 21.38, Sam Ravnborg wrote:
> Hi Peter.
>
> Driver looks to be in good shape now.
> With the few comments below addressed you can add my:
> Reviewed-by: Sam Ravnborg <[email protected]>
>
> Sam
>
> On Fri, Feb 22, 2019 at 03:16:18PM +0200, Peter Ujfalusi wrote:
>> The panel is similar to OSD101T2045-53TS (which is handled by panel-simple)
>> with one big difference: osd101t2587-53ts needs MIPI_DSI_TURN_ON_PERIPHERAL
>> message to be sent from the host to be operational and thus can not be
>> handled by panel-simple.
>>
>> Signed-off-by: Peter Ujfalusi <[email protected]>
>> ---
>> drivers/gpu/drm/panel/Kconfig | 6 +
>> drivers/gpu/drm/panel/Makefile | 1 +
>> .../drm/panel/panel-osd-osd101t2587-53ts.c | 254 ++++++++++++++++++
>> 3 files changed, 261 insertions(+)
>> create mode 100644 drivers/gpu/drm/panel/panel-osd-osd101t2587-53ts.c
>>
>> diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
>> index 3e070153ef21..351661920838 100644
>> --- a/drivers/gpu/drm/panel/Kconfig
>> +++ b/drivers/gpu/drm/panel/Kconfig
>> @@ -122,6 +122,12 @@ config DRM_PANEL_ORISETECH_OTM8009A
>> Say Y here if you want to enable support for Orise Technology
>> otm8009a 480x800 dsi 2dl panel.
>>
>> +config DRM_PANEL_OSD_OSD101T2587_53TS
>> + tristate "OSD OSD101T2587-53TS DSI 1920x1200 video mode panel"
>> + depends on OF
>> + depends on DRM_MIPI_DSI
>> + depends on BACKLIGHT_CLASS_DEVICE
> Please add a help-text
Sure, I forgot this.
>> +
>> +static int osd101t2587_panel_unprepare(struct drm_panel *panel)
>> +{
>> + struct osd101t2587_panel *osd101t2587 = to_osd101t2587_panel(panel);
>> +
>> + if (!osd101t2587->prepared)
>> + return 0;
>> +
>> + regulator_disable(osd101t2587->supply);
>> + osd101t2587->prepared = false;
>> +
>> + return 0;
>> +}
>> +
>> +static int osd101t2587_panel_prepare(struct drm_panel *panel)
>> +{
>> + struct osd101t2587_panel *osd101t2587 = to_osd101t2587_panel(panel);
>> + int ret;
>> +
>> + if (osd101t2587->prepared)
>> + return 0;
>> +
>> + ret = regulator_enable(osd101t2587->supply);
>> + if (!ret)
>> + osd101t2587->prepared = true;
>
> Logic is wrong here. regulator_enable() will return a negative value on error
> and 0 in the good case.
> So osd101t2587->prepared is set to true only in the error case, not in the good case.
It is good as it is:
'if (!0)' == 'if (1)'
'if (!-X)' == 'if (0)'
>> +
>> + ret = mipi_dsi_attach(dsi);
>> + if (ret)
>> + drm_panel_remove(&osd101t2587->base);
>
> I do not see panel-simple.c do a drm_panel_remove() if mipi_dsi_attach() fails.
> Maybe the driver core will call remove() is probe fails?
> Or maybe panel-simple() should call drm_panel_remove()
>
> Keep the above as is - I just wanted to express that this looks different
> from the panle-simple() driver.
I have a patch for panel-simple as well with the following commit message:
"drm/panel: simple: Fix panel_simple_dsi_probe
In case mipi_dsi_attach() fails remove the registered panel to avoid
added panel without corresponding device."
It has the same bug.
>> +static int osd101t2587_panel_remove(struct mipi_dsi_device *dsi)
>> +{
>> + struct osd101t2587_panel *osd101t2587 = mipi_dsi_get_drvdata(dsi);
>> + int ret;
>> +
>> + ret = osd101t2587_panel_disable(&osd101t2587->base);
>> + if (ret < 0)
>> + dev_warn(&dsi->dev, "failed to disable panel\n");
> This is already warned in lower layers and I think you could
> drop the dev_warn().
I think there is no warning from lower layer, but not sure as I never
hit this case.
>> +
>> + osd101t2587_panel_unprepare(&osd101t2587->base);
>> +
>> + drm_panel_remove(&osd101t2587->base);
>> +
>> + ret = mipi_dsi_detach(dsi);
>> + if (ret < 0)
>> + dev_warn(&dsi->dev, "failed to detach from DSI host\n");
> Add error code in logging.
OK
- Péter
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki