diff mbox series

[21/60] drm/panel: Add driver for the Sharp LS037V7DW01 panel

Message ID 20190707181937.6250-18-laurent.pinchart@ideasonboard.com (mailing list archive)
State New, archived
Headers show
Series drm/omap: Replace custom display drivers with drm_bridge and drm_panel | expand

Commit Message

Laurent Pinchart July 7, 2019, 6:18 p.m. UTC 
This panel is used on the SDP3430.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/gpu/drm/panel/Kconfig                 |   7 +
 drivers/gpu/drm/panel/Makefile                |   1 +
 .../gpu/drm/panel/panel-sharp-ls037v7dw01.c   | 231 ++++++++++++++++++
 3 files changed, 239 insertions(+)
 create mode 100644 drivers/gpu/drm/panel/panel-sharp-ls037v7dw01.c

Comments

Sam Ravnborg July 8, 2019, 7:44 p.m. UTC | #1 
Hi Laurent.

Third panel driver in line for review.
Review comments that are duplicates from the first two will have only a
brief remark - if any.

On Sun, Jul 07, 2019 at 09:18:58PM +0300, Laurent Pinchart wrote:
> This panel is used on the SDP3430.
Add a little more context and put it in Kconfig help.
Maybe this is the TI board, and maybe it is something else.

> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  drivers/gpu/drm/panel/Kconfig                 |   7 +
>  drivers/gpu/drm/panel/Makefile                |   1 +
>  .../gpu/drm/panel/panel-sharp-ls037v7dw01.c   | 231 ++++++++++++++++++
>  3 files changed, 239 insertions(+)
>  create mode 100644 drivers/gpu/drm/panel/panel-sharp-ls037v7dw01.c
> 
> diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
> index da613c04b835..04fd152efe4c 100644
> --- a/drivers/gpu/drm/panel/Kconfig
> +++ b/drivers/gpu/drm/panel/Kconfig
> @@ -271,6 +271,13 @@ config DRM_PANEL_SHARP_LS043T1LE01
>  	  Say Y here if you want to enable support for Sharp LS043T1LE01 qHD
>  	  (540x960) DSI panel as found on the Qualcomm APQ8074 Dragonboard
>  
> +config DRM_PANEL_SHARP_LS037V7DW01
> +	tristate "Sharp LS037V7DW01 VGA LCD panel"
> +	depends on GPIOLIB && OF && REGULATOR
> +	help
> +	  Say Y here if you want to enable support for Sharp LS037V7DW01 VGA
> +	  (480x640) LCD panel.
> +
Alphabetical order, so it comes before DRM_PANEL_SHARP_LS043T1LE01

>  config DRM_PANEL_SITRONIX_ST7701
>  	tristate "Sitronix ST7701 panel driver"
>  	depends on OF
> diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
> index e81ed1535024..12dcd76eb87c 100644
> --- a/drivers/gpu/drm/panel/Makefile
> +++ b/drivers/gpu/drm/panel/Makefile
> @@ -27,6 +27,7 @@ obj-$(CONFIG_DRM_PANEL_SAMSUNG_S6E63M0) += panel-samsung-s6e63m0.o
>  obj-$(CONFIG_DRM_PANEL_SAMSUNG_S6E8AA0) += panel-samsung-s6e8aa0.o
>  obj-$(CONFIG_DRM_PANEL_SEIKO_43WVF1G) += panel-seiko-43wvf1g.o
>  obj-$(CONFIG_DRM_PANEL_SHARP_LQ101R1SX01) += panel-sharp-lq101r1sx01.o
> +obj-$(CONFIG_DRM_PANEL_SHARP_LS037V7DW01) += panel-sharp-ls037v7dw01.o
>  obj-$(CONFIG_DRM_PANEL_SHARP_LS043T1LE01) += panel-sharp-ls043t1le01.o
And here it is right.

> --- /dev/null
> +++ b/drivers/gpu/drm/panel/panel-sharp-ls037v7dw01.c
> @@ -0,0 +1,231 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Sharp LS037V7DW01 LCD Panel Driver
> + *
> + * Copyright (C) 2019 Texas Instruments Incorporated
> + *
> + * Based on the omapdrm-specific panel-sharp-ls037v7dw01 driver
> + *
> + * Copyright (C) 2013 Texas Instruments Incorporated
> + * Author: Tomi Valkeinen <tomi.valkeinen@ti.com>
Add your copyright?

> +struct ls037v7dw01_device {
> +	struct drm_panel panel;
> +	struct platform_device *pdev;
> +
> +	struct regulator *vcc;
The property is named envdd - should they use the same name?

> +	struct gpio_desc *resb_gpio;	/* low = reset active min 20 us */
> +	struct gpio_desc *ini_gpio;	/* high = power on */
> +	struct gpio_desc *mo_gpio;	/* low = 480x640, high = 240x320 */
> +	struct gpio_desc *lr_gpio;	/* high = conventional horizontal scanning */
> +	struct gpio_desc *ud_gpio;	/* high = conventional vertical scanning */
> +};
device versus panel, but bikeshedding, so feel free to ignore.

> +
> +static int ls037v7dw01_disable(struct drm_panel *panel)
> +{
> +	struct ls037v7dw01_device *lcd = to_ls037v7dw01_device(panel);
> +
> +	gpiod_set_value_cansleep(lcd->ini_gpio, 0);
> +	gpiod_set_value_cansleep(lcd->resb_gpio, 0);
> +
> +	/* Wait at least 5 vsyncs after disabling the LCD. */
> +	msleep(100);
> +
> +	return 0;
> +}
> +
> +static int ls037v7dw01_unprepare(struct drm_panel *panel)
> +{
> +	struct ls037v7dw01_device *lcd = to_ls037v7dw01_device(panel);
> +
> +	if (lcd->vcc)
> +		regulator_disable(lcd->vcc);
Why is the if (lcd-vcc) needed?
If I read the probe code correct then we either get a regulator or we
error out.

Same goes for all other checks of lcd->vcc

> +static const struct drm_display_mode ls037v7dw01_mode = {
> +	.clock = 19200,
> +	.hdisplay = 480,
> +	.hsync_start = 480 + 1,
> +	.hsync_end = 480 + 1 + 2,
> +	.htotal = 480 + 1 + 2 + 28,
> +	.vdisplay = 640,
> +	.vsync_start = 640 + 1,
> +	.vsync_end = 640 + 1 + 1,
> +	.vtotal = 640 + 1 + 1 + 1,
> +	.vrefresh = 58,
> +	.type = DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED,
> +	.flags = DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC,
> +};
> +
> +static int ls037v7dw01_get_modes(struct drm_panel *panel)
> +{
> +	struct drm_connector *connector = panel->connector;
> +	struct drm_display_mode *mode;
> +
> +	mode = drm_mode_duplicate(panel->drm, &ls037v7dw01_mode);
> +	if (!mode)
> +		return -ENOMEM;
> +
> +	drm_mode_set_name(mode);
> +	drm_mode_probed_add(connector, mode);
> +
> +	connector->display_info.width_mm = 56;
> +	connector->display_info.height_mm = 75;
> +	/*
> +	 * FIXME: According to the datasheet pixel data is sampled on the
> +	 * rising edge of the clock, but the code running on the SDP3430
> +	 * indicates sampling on the negative edge. This should be tested on a
> +	 * real device.
> +	 */
> +	connector->display_info.bus_flags = DRM_BUS_FLAG_DE_HIGH
> +					  | DRM_BUS_FLAG_SYNC_SAMPLE_POSEDGE
> +					  | DRM_BUS_FLAG_PIXDATA_SAMPLE_NEGEDGE;
> +
> +	return 1;
> +}
> +
> +static const struct drm_panel_funcs ls037v7dw01_funcs = {
> +	.disable = ls037v7dw01_disable,
> +	.unprepare = ls037v7dw01_unprepare,
> +	.prepare = ls037v7dw01_prepare,
> +	.enable = ls037v7dw01_enable,
> +	.get_modes = ls037v7dw01_get_modes,
> +};
> +
> +static int ls037v7dw01_probe(struct platform_device *pdev)
> +{
> +	struct ls037v7dw01_device *lcd;
> +
> +	lcd = devm_kzalloc(&pdev->dev, sizeof(*lcd), GFP_KERNEL);
> +	if (lcd == NULL)
> +		return -ENOMEM;
> +
> +	platform_set_drvdata(pdev, lcd);
> +	lcd->pdev = pdev;
> +
> +	lcd->vcc = devm_regulator_get(&pdev->dev, "envdd");
> +	if (IS_ERR(lcd->vcc)) {
> +		dev_err(&pdev->dev, "failed to get regulator\n");
> +		return PTR_ERR(lcd->vcc);
> +	}
> +
> +	lcd->ini_gpio = devm_gpiod_get_index(&pdev->dev, "enable", 0,
> +					    GPIOD_OUT_LOW);
> +	if (IS_ERR(lcd->ini_gpio)) {
> +		dev_err(&pdev->dev, "failed to get enable gpio\n");
> +		return PTR_ERR(lcd->ini_gpio);
> +	}
I fail to see why the _index() variant is used here.
But then I did not check the binding, so it may originate from that.
Same goes for ireset gpio

> +
> +	lcd->resb_gpio = devm_gpiod_get_index(&pdev->dev, "reset", 0,
> +					     GPIOD_OUT_LOW);
> +	if (IS_ERR(lcd->resb_gpio)) {
> +		dev_err(&pdev->dev, "failed to get reset gpio\n");
> +		return PTR_ERR(lcd->resb_gpio);
> +	}
> +
> +	lcd->mo_gpio = devm_gpiod_get_index(&pdev->dev, "mode", 0,
> +					   GPIOD_OUT_LOW);
> +	if (IS_ERR(lcd->mo_gpio)) {
> +		dev_err(&pdev->dev, "failed to get mode[0] gpio\n");
> +		return PTR_ERR(lcd->mo_gpio);
> +	}
> +
> +	lcd->lr_gpio = devm_gpiod_get_index(&pdev->dev, "mode", 1,
> +					   GPIOD_OUT_LOW);
> +	if (IS_ERR(lcd->lr_gpio)) {
> +		dev_err(&pdev->dev, "failed to get mode[1] gpio\n");
> +		return PTR_ERR(lcd->lr_gpio);
> +	}
> +
> +	lcd->ud_gpio = devm_gpiod_get_index(&pdev->dev, "mode", 2,
> +					   GPIOD_OUT_LOW);
> +	if (IS_ERR(lcd->ud_gpio)) {
> +		dev_err(&pdev->dev, "failed to get mode[2] gpio\n");
> +		return PTR_ERR(lcd->ud_gpio);
> +	}
Do we set mo, lr ,ud gpio when we call devm_gpiod_get, or are they
unused?

> +
> +	drm_panel_init(&lcd->panel);
> +	lcd->panel.dev = &pdev->dev;
> +	lcd->panel.funcs = &ls037v7dw01_funcs;
> +
> +	return drm_panel_add(&lcd->panel);
> +}
> +
> +static int ls037v7dw01_remove(struct platform_device *pdev)
> +{
> +	struct ls037v7dw01_device *lcd = platform_get_drvdata(pdev);
> +
> +	drm_panel_remove(&lcd->panel);
> +	ls037v7dw01_disable(&lcd->panel);
> +	ls037v7dw01_unprepare(&lcd->panel);
Use drm_panel_disable(), drm_panel_unprepare()

> +
> +	return 0;
> +}
> +
> +static const struct of_device_id ls037v7dw01_of_match[] = {
> +	{ .compatible = "sharp,ls037v7dw01", },
> +	{},
{ /* sentinel */ },

> +};
> +
> +MODULE_DEVICE_TABLE(of, ls037v7dw01_of_match);
> +
> +static struct platform_driver ls037v7dw01_driver = {
> +	.probe		= ls037v7dw01_probe,
> +	.remove		= __exit_p(ls037v7dw01_remove),
> +	.driver		= {
> +		.name = "panel-sharp-ls037v7dw01",
> +		.of_match_table = ls037v7dw01_of_match,
> +	},
> +};
> +
> +module_platform_driver(ls037v7dw01_driver);
> +
> +MODULE_AUTHOR("Tomi Valkeinen <tomi.valkeinen@ti.com>");
> +MODULE_DESCRIPTION("Sharp LS037V7DW01 Panel Driver");
> +MODULE_LICENSE("GPL");
"GPL v2"?

	Sam
Sam Ravnborg July 8, 2019, 7:47 p.m. UTC | #2 
Hi Laurent.

> > +
> > +MODULE_DEVICE_TABLE(of, ls037v7dw01_of_match);
> > +
> > +static struct platform_driver ls037v7dw01_driver = {
> > +	.probe		= ls037v7dw01_probe,
> > +	.remove		= __exit_p(ls037v7dw01_remove),

I hope _exit_p() is not needed.
No other panel drivers use it as far as I could see.

	Sam
Laurent Pinchart Aug. 8, 2019, 3:31 p.m. UTC | #3 
Hi Sam,

On Mon, Jul 08, 2019 at 09:44:52PM +0200, Sam Ravnborg wrote:
> Hi Laurent.
> 
> Third panel driver in line for review.
> Review comments that are duplicates from the first two will have only a
> brief remark - if any.

I'll address them the same way. Please consider all unanswered comments
below as addressed.

> On Sun, Jul 07, 2019 at 09:18:58PM +0300, Laurent Pinchart wrote:
> > This panel is used on the SDP3430.
>
> Add a little more context and put it in Kconfig help.
> Maybe this is the TI board, and maybe it is something else.
> 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> >  drivers/gpu/drm/panel/Kconfig                 |   7 +
> >  drivers/gpu/drm/panel/Makefile                |   1 +
> >  .../gpu/drm/panel/panel-sharp-ls037v7dw01.c   | 231 ++++++++++++++++++
> >  3 files changed, 239 insertions(+)
> >  create mode 100644 drivers/gpu/drm/panel/panel-sharp-ls037v7dw01.c
> > 
> > diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
> > index da613c04b835..04fd152efe4c 100644
> > --- a/drivers/gpu/drm/panel/Kconfig
> > +++ b/drivers/gpu/drm/panel/Kconfig
> > @@ -271,6 +271,13 @@ config DRM_PANEL_SHARP_LS043T1LE01
> >  	  Say Y here if you want to enable support for Sharp LS043T1LE01 qHD
> >  	  (540x960) DSI panel as found on the Qualcomm APQ8074 Dragonboard
> >  
> > +config DRM_PANEL_SHARP_LS037V7DW01
> > +	tristate "Sharp LS037V7DW01 VGA LCD panel"
> > +	depends on GPIOLIB && OF && REGULATOR
> > +	help
> > +	  Say Y here if you want to enable support for Sharp LS037V7DW01 VGA
> > +	  (480x640) LCD panel.
> > +
> 
> Alphabetical order, so it comes before DRM_PANEL_SHARP_LS043T1LE01
> 
> >  config DRM_PANEL_SITRONIX_ST7701
> >  	tristate "Sitronix ST7701 panel driver"
> >  	depends on OF
> > diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
> > index e81ed1535024..12dcd76eb87c 100644
> > --- a/drivers/gpu/drm/panel/Makefile
> > +++ b/drivers/gpu/drm/panel/Makefile
> > @@ -27,6 +27,7 @@ obj-$(CONFIG_DRM_PANEL_SAMSUNG_S6E63M0) += panel-samsung-s6e63m0.o
> >  obj-$(CONFIG_DRM_PANEL_SAMSUNG_S6E8AA0) += panel-samsung-s6e8aa0.o
> >  obj-$(CONFIG_DRM_PANEL_SEIKO_43WVF1G) += panel-seiko-43wvf1g.o
> >  obj-$(CONFIG_DRM_PANEL_SHARP_LQ101R1SX01) += panel-sharp-lq101r1sx01.o
> > +obj-$(CONFIG_DRM_PANEL_SHARP_LS037V7DW01) += panel-sharp-ls037v7dw01.o
> >  obj-$(CONFIG_DRM_PANEL_SHARP_LS043T1LE01) += panel-sharp-ls043t1le01.o
> 
> And here it is right.
> 
> > --- /dev/null
> > +++ b/drivers/gpu/drm/panel/panel-sharp-ls037v7dw01.c
> > @@ -0,0 +1,231 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Sharp LS037V7DW01 LCD Panel Driver
> > + *
> > + * Copyright (C) 2019 Texas Instruments Incorporated
> > + *
> > + * Based on the omapdrm-specific panel-sharp-ls037v7dw01 driver
> > + *
> > + * Copyright (C) 2013 Texas Instruments Incorporated
> > + * Author: Tomi Valkeinen <tomi.valkeinen@ti.com>
> 
> Add your copyright?

As with the previous patches, the copyright goes to TI.

> > +struct ls037v7dw01_device {
> > +	struct drm_panel panel;
> > +	struct platform_device *pdev;
> > +
> > +	struct regulator *vcc;
> 
> The property is named envdd - should they use the same name?

I'll rename that to vdd.

> > +	struct gpio_desc *resb_gpio;	/* low = reset active min 20 us */
> > +	struct gpio_desc *ini_gpio;	/* high = power on */
> > +	struct gpio_desc *mo_gpio;	/* low = 480x640, high = 240x320 */
> > +	struct gpio_desc *lr_gpio;	/* high = conventional horizontal scanning */
> > +	struct gpio_desc *ud_gpio;	/* high = conventional vertical scanning */
> > +};
> 
> device versus panel, but bikeshedding, so feel free to ignore.

I'll rename it.

> > +
> > +static int ls037v7dw01_disable(struct drm_panel *panel)
> > +{
> > +	struct ls037v7dw01_device *lcd = to_ls037v7dw01_device(panel);
> > +
> > +	gpiod_set_value_cansleep(lcd->ini_gpio, 0);
> > +	gpiod_set_value_cansleep(lcd->resb_gpio, 0);
> > +
> > +	/* Wait at least 5 vsyncs after disabling the LCD. */
> > +	msleep(100);
> > +
> > +	return 0;
> > +}
> > +
> > +static int ls037v7dw01_unprepare(struct drm_panel *panel)
> > +{
> > +	struct ls037v7dw01_device *lcd = to_ls037v7dw01_device(panel);
> > +
> > +	if (lcd->vcc)
> > +		regulator_disable(lcd->vcc);
> 
> Why is the if (lcd-vcc) needed?
> If I read the probe code correct then we either get a regulator or we
> error out.
> 
> Same goes for all other checks of lcd->vcc
> 
> > +static const struct drm_display_mode ls037v7dw01_mode = {
> > +	.clock = 19200,
> > +	.hdisplay = 480,
> > +	.hsync_start = 480 + 1,
> > +	.hsync_end = 480 + 1 + 2,
> > +	.htotal = 480 + 1 + 2 + 28,
> > +	.vdisplay = 640,
> > +	.vsync_start = 640 + 1,
> > +	.vsync_end = 640 + 1 + 1,
> > +	.vtotal = 640 + 1 + 1 + 1,
> > +	.vrefresh = 58,
> > +	.type = DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED,
> > +	.flags = DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC,
> > +};
> > +
> > +static int ls037v7dw01_get_modes(struct drm_panel *panel)
> > +{
> > +	struct drm_connector *connector = panel->connector;
> > +	struct drm_display_mode *mode;
> > +
> > +	mode = drm_mode_duplicate(panel->drm, &ls037v7dw01_mode);
> > +	if (!mode)
> > +		return -ENOMEM;
> > +
> > +	drm_mode_set_name(mode);
> > +	drm_mode_probed_add(connector, mode);
> > +
> > +	connector->display_info.width_mm = 56;
> > +	connector->display_info.height_mm = 75;
> > +	/*
> > +	 * FIXME: According to the datasheet pixel data is sampled on the
> > +	 * rising edge of the clock, but the code running on the SDP3430
> > +	 * indicates sampling on the negative edge. This should be tested on a
> > +	 * real device.
> > +	 */
> > +	connector->display_info.bus_flags = DRM_BUS_FLAG_DE_HIGH
> > +					  | DRM_BUS_FLAG_SYNC_SAMPLE_POSEDGE
> > +					  | DRM_BUS_FLAG_PIXDATA_SAMPLE_NEGEDGE;
> > +
> > +	return 1;
> > +}
> > +
> > +static const struct drm_panel_funcs ls037v7dw01_funcs = {
> > +	.disable = ls037v7dw01_disable,
> > +	.unprepare = ls037v7dw01_unprepare,
> > +	.prepare = ls037v7dw01_prepare,
> > +	.enable = ls037v7dw01_enable,
> > +	.get_modes = ls037v7dw01_get_modes,
> > +};
> > +
> > +static int ls037v7dw01_probe(struct platform_device *pdev)
> > +{
> > +	struct ls037v7dw01_device *lcd;
> > +
> > +	lcd = devm_kzalloc(&pdev->dev, sizeof(*lcd), GFP_KERNEL);
> > +	if (lcd == NULL)
> > +		return -ENOMEM;
> > +
> > +	platform_set_drvdata(pdev, lcd);
> > +	lcd->pdev = pdev;
> > +
> > +	lcd->vcc = devm_regulator_get(&pdev->dev, "envdd");
> > +	if (IS_ERR(lcd->vcc)) {
> > +		dev_err(&pdev->dev, "failed to get regulator\n");
> > +		return PTR_ERR(lcd->vcc);
> > +	}
> > +
> > +	lcd->ini_gpio = devm_gpiod_get_index(&pdev->dev, "enable", 0,
> > +					    GPIOD_OUT_LOW);
> > +	if (IS_ERR(lcd->ini_gpio)) {
> > +		dev_err(&pdev->dev, "failed to get enable gpio\n");
> > +		return PTR_ERR(lcd->ini_gpio);
> > +	}
> 
> I fail to see why the _index() variant is used here.
> But then I did not check the binding, so it may originate from that.
> Same goes for ireset gpio
> 
> > +
> > +	lcd->resb_gpio = devm_gpiod_get_index(&pdev->dev, "reset", 0,
> > +					     GPIOD_OUT_LOW);
> > +	if (IS_ERR(lcd->resb_gpio)) {
> > +		dev_err(&pdev->dev, "failed to get reset gpio\n");
> > +		return PTR_ERR(lcd->resb_gpio);
> > +	}
> > +
> > +	lcd->mo_gpio = devm_gpiod_get_index(&pdev->dev, "mode", 0,
> > +					   GPIOD_OUT_LOW);
> > +	if (IS_ERR(lcd->mo_gpio)) {
> > +		dev_err(&pdev->dev, "failed to get mode[0] gpio\n");
> > +		return PTR_ERR(lcd->mo_gpio);
> > +	}
> > +
> > +	lcd->lr_gpio = devm_gpiod_get_index(&pdev->dev, "mode", 1,
> > +					   GPIOD_OUT_LOW);
> > +	if (IS_ERR(lcd->lr_gpio)) {
> > +		dev_err(&pdev->dev, "failed to get mode[1] gpio\n");
> > +		return PTR_ERR(lcd->lr_gpio);
> > +	}
> > +
> > +	lcd->ud_gpio = devm_gpiod_get_index(&pdev->dev, "mode", 2,
> > +					   GPIOD_OUT_LOW);
> > +	if (IS_ERR(lcd->ud_gpio)) {
> > +		dev_err(&pdev->dev, "failed to get mode[2] gpio\n");
> > +		return PTR_ERR(lcd->ud_gpio);
> > +	}
> 
> Do we set mo, lr ,ud gpio when we call devm_gpiod_get, or are they
> unused?

They are set by GPIOD_OUT_LOW.

> > +
> > +	drm_panel_init(&lcd->panel);
> > +	lcd->panel.dev = &pdev->dev;
> > +	lcd->panel.funcs = &ls037v7dw01_funcs;
> > +
> > +	return drm_panel_add(&lcd->panel);
> > +}
> > +
> > +static int ls037v7dw01_remove(struct platform_device *pdev)
> > +{
> > +	struct ls037v7dw01_device *lcd = platform_get_drvdata(pdev);
> > +
> > +	drm_panel_remove(&lcd->panel);
> > +	ls037v7dw01_disable(&lcd->panel);
> > +	ls037v7dw01_unprepare(&lcd->panel);
> 
> Use drm_panel_disable(), drm_panel_unprepare()
> 
> > +
> > +	return 0;
> > +}
> > +
> > +static const struct of_device_id ls037v7dw01_of_match[] = {
> > +	{ .compatible = "sharp,ls037v7dw01", },
> > +	{},
> 
> { /* sentinel */ },
> 
> > +};
> > +
> > +MODULE_DEVICE_TABLE(of, ls037v7dw01_of_match);
> > +
> > +static struct platform_driver ls037v7dw01_driver = {
> > +	.probe		= ls037v7dw01_probe,
> > +	.remove		= __exit_p(ls037v7dw01_remove),
> > +	.driver		= {
> > +		.name = "panel-sharp-ls037v7dw01",
> > +		.of_match_table = ls037v7dw01_of_match,
> > +	},
> > +};
> > +
> > +module_platform_driver(ls037v7dw01_driver);
> > +
> > +MODULE_AUTHOR("Tomi Valkeinen <tomi.valkeinen@ti.com>");
> > +MODULE_DESCRIPTION("Sharp LS037V7DW01 Panel Driver");
> > +MODULE_LICENSE("GPL");
> 
> "GPL v2"?
diff mbox series

Patch

diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
index da613c04b835..04fd152efe4c 100644
--- a/drivers/gpu/drm/panel/Kconfig
+++ b/drivers/gpu/drm/panel/Kconfig
@@ -271,6 +271,13 @@  config DRM_PANEL_SHARP_LS043T1LE01
 	  Say Y here if you want to enable support for Sharp LS043T1LE01 qHD
 	  (540x960) DSI panel as found on the Qualcomm APQ8074 Dragonboard
 
+config DRM_PANEL_SHARP_LS037V7DW01
+	tristate "Sharp LS037V7DW01 VGA LCD panel"
+	depends on GPIOLIB && OF && REGULATOR
+	help
+	  Say Y here if you want to enable support for Sharp LS037V7DW01 VGA
+	  (480x640) LCD panel.
+
 config DRM_PANEL_SITRONIX_ST7701
 	tristate "Sitronix ST7701 panel driver"
 	depends on OF
diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
index e81ed1535024..12dcd76eb87c 100644
--- a/drivers/gpu/drm/panel/Makefile
+++ b/drivers/gpu/drm/panel/Makefile
@@ -27,6 +27,7 @@  obj-$(CONFIG_DRM_PANEL_SAMSUNG_S6E63M0) += panel-samsung-s6e63m0.o
 obj-$(CONFIG_DRM_PANEL_SAMSUNG_S6E8AA0) += panel-samsung-s6e8aa0.o
 obj-$(CONFIG_DRM_PANEL_SEIKO_43WVF1G) += panel-seiko-43wvf1g.o
 obj-$(CONFIG_DRM_PANEL_SHARP_LQ101R1SX01) += panel-sharp-lq101r1sx01.o
+obj-$(CONFIG_DRM_PANEL_SHARP_LS037V7DW01) += panel-sharp-ls037v7dw01.o
 obj-$(CONFIG_DRM_PANEL_SHARP_LS043T1LE01) += panel-sharp-ls043t1le01.o
 obj-$(CONFIG_DRM_PANEL_SITRONIX_ST7701) += panel-sitronix-st7701.o
 obj-$(CONFIG_DRM_PANEL_SITRONIX_ST7789V) += panel-sitronix-st7789v.o
diff --git a/drivers/gpu/drm/panel/panel-sharp-ls037v7dw01.c b/drivers/gpu/drm/panel/panel-sharp-ls037v7dw01.c
new file mode 100644
index 000000000000..4532455b4161
--- /dev/null
+++ b/drivers/gpu/drm/panel/panel-sharp-ls037v7dw01.c
@@ -0,0 +1,231 @@ 
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Sharp LS037V7DW01 LCD Panel Driver
+ *
+ * Copyright (C) 2019 Texas Instruments Incorporated
+ *
+ * Based on the omapdrm-specific panel-sharp-ls037v7dw01 driver
+ *
+ * Copyright (C) 2013 Texas Instruments Incorporated
+ * Author: Tomi Valkeinen <tomi.valkeinen@ti.com>
+ */
+
+#include <linux/delay.h>
+#include <linux/gpio/consumer.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/regulator/consumer.h>
+
+#include <drm/drm_connector.h>
+#include <drm/drm_modes.h>
+#include <drm/drm_panel.h>
+
+struct ls037v7dw01_device {
+	struct drm_panel panel;
+	struct platform_device *pdev;
+
+	struct regulator *vcc;
+	struct gpio_desc *resb_gpio;	/* low = reset active min 20 us */
+	struct gpio_desc *ini_gpio;	/* high = power on */
+	struct gpio_desc *mo_gpio;	/* low = 480x640, high = 240x320 */
+	struct gpio_desc *lr_gpio;	/* high = conventional horizontal scanning */
+	struct gpio_desc *ud_gpio;	/* high = conventional vertical scanning */
+};
+
+#define to_ls037v7dw01_device(p) \
+	container_of(p, struct ls037v7dw01_device, panel)
+
+static int ls037v7dw01_disable(struct drm_panel *panel)
+{
+	struct ls037v7dw01_device *lcd = to_ls037v7dw01_device(panel);
+
+	gpiod_set_value_cansleep(lcd->ini_gpio, 0);
+	gpiod_set_value_cansleep(lcd->resb_gpio, 0);
+
+	/* Wait at least 5 vsyncs after disabling the LCD. */
+	msleep(100);
+
+	return 0;
+}
+
+static int ls037v7dw01_unprepare(struct drm_panel *panel)
+{
+	struct ls037v7dw01_device *lcd = to_ls037v7dw01_device(panel);
+
+	if (lcd->vcc)
+		regulator_disable(lcd->vcc);
+
+	return 0;
+}
+
+static int ls037v7dw01_prepare(struct drm_panel *panel)
+{
+	struct ls037v7dw01_device *lcd = to_ls037v7dw01_device(panel);
+	int ret;
+
+	if (!lcd->vcc)
+		return 0;
+
+	ret = regulator_enable(lcd->vcc);
+	if (ret < 0)
+		dev_err(&lcd->pdev->dev, "%s: failed to enable regulator\n",
+			__func__);
+
+	return ret;
+}
+
+static int ls037v7dw01_enable(struct drm_panel *panel)
+{
+	struct ls037v7dw01_device *lcd = to_ls037v7dw01_device(panel);
+
+	/* Wait couple of vsyncs before enabling the LCD. */
+	msleep(50);
+
+	gpiod_set_value_cansleep(lcd->resb_gpio, 1);
+	gpiod_set_value_cansleep(lcd->ini_gpio, 1);
+
+	return 0;
+}
+
+static const struct drm_display_mode ls037v7dw01_mode = {
+	.clock = 19200,
+	.hdisplay = 480,
+	.hsync_start = 480 + 1,
+	.hsync_end = 480 + 1 + 2,
+	.htotal = 480 + 1 + 2 + 28,
+	.vdisplay = 640,
+	.vsync_start = 640 + 1,
+	.vsync_end = 640 + 1 + 1,
+	.vtotal = 640 + 1 + 1 + 1,
+	.vrefresh = 58,
+	.type = DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED,
+	.flags = DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC,
+};
+
+static int ls037v7dw01_get_modes(struct drm_panel *panel)
+{
+	struct drm_connector *connector = panel->connector;
+	struct drm_display_mode *mode;
+
+	mode = drm_mode_duplicate(panel->drm, &ls037v7dw01_mode);
+	if (!mode)
+		return -ENOMEM;
+
+	drm_mode_set_name(mode);
+	drm_mode_probed_add(connector, mode);
+
+	connector->display_info.width_mm = 56;
+	connector->display_info.height_mm = 75;
+	/*
+	 * FIXME: According to the datasheet pixel data is sampled on the
+	 * rising edge of the clock, but the code running on the SDP3430
+	 * indicates sampling on the negative edge. This should be tested on a
+	 * real device.
+	 */
+	connector->display_info.bus_flags = DRM_BUS_FLAG_DE_HIGH
+					  | DRM_BUS_FLAG_SYNC_SAMPLE_POSEDGE
+					  | DRM_BUS_FLAG_PIXDATA_SAMPLE_NEGEDGE;
+
+	return 1;
+}
+
+static const struct drm_panel_funcs ls037v7dw01_funcs = {
+	.disable = ls037v7dw01_disable,
+	.unprepare = ls037v7dw01_unprepare,
+	.prepare = ls037v7dw01_prepare,
+	.enable = ls037v7dw01_enable,
+	.get_modes = ls037v7dw01_get_modes,
+};
+
+static int ls037v7dw01_probe(struct platform_device *pdev)
+{
+	struct ls037v7dw01_device *lcd;
+
+	lcd = devm_kzalloc(&pdev->dev, sizeof(*lcd), GFP_KERNEL);
+	if (lcd == NULL)
+		return -ENOMEM;
+
+	platform_set_drvdata(pdev, lcd);
+	lcd->pdev = pdev;
+
+	lcd->vcc = devm_regulator_get(&pdev->dev, "envdd");
+	if (IS_ERR(lcd->vcc)) {
+		dev_err(&pdev->dev, "failed to get regulator\n");
+		return PTR_ERR(lcd->vcc);
+	}
+
+	lcd->ini_gpio = devm_gpiod_get_index(&pdev->dev, "enable", 0,
+					    GPIOD_OUT_LOW);
+	if (IS_ERR(lcd->ini_gpio)) {
+		dev_err(&pdev->dev, "failed to get enable gpio\n");
+		return PTR_ERR(lcd->ini_gpio);
+	}
+
+	lcd->resb_gpio = devm_gpiod_get_index(&pdev->dev, "reset", 0,
+					     GPIOD_OUT_LOW);
+	if (IS_ERR(lcd->resb_gpio)) {
+		dev_err(&pdev->dev, "failed to get reset gpio\n");
+		return PTR_ERR(lcd->resb_gpio);
+	}
+
+	lcd->mo_gpio = devm_gpiod_get_index(&pdev->dev, "mode", 0,
+					   GPIOD_OUT_LOW);
+	if (IS_ERR(lcd->mo_gpio)) {
+		dev_err(&pdev->dev, "failed to get mode[0] gpio\n");
+		return PTR_ERR(lcd->mo_gpio);
+	}
+
+	lcd->lr_gpio = devm_gpiod_get_index(&pdev->dev, "mode", 1,
+					   GPIOD_OUT_LOW);
+	if (IS_ERR(lcd->lr_gpio)) {
+		dev_err(&pdev->dev, "failed to get mode[1] gpio\n");
+		return PTR_ERR(lcd->lr_gpio);
+	}
+
+	lcd->ud_gpio = devm_gpiod_get_index(&pdev->dev, "mode", 2,
+					   GPIOD_OUT_LOW);
+	if (IS_ERR(lcd->ud_gpio)) {
+		dev_err(&pdev->dev, "failed to get mode[2] gpio\n");
+		return PTR_ERR(lcd->ud_gpio);
+	}
+
+	drm_panel_init(&lcd->panel);
+	lcd->panel.dev = &pdev->dev;
+	lcd->panel.funcs = &ls037v7dw01_funcs;
+
+	return drm_panel_add(&lcd->panel);
+}
+
+static int ls037v7dw01_remove(struct platform_device *pdev)
+{
+	struct ls037v7dw01_device *lcd = platform_get_drvdata(pdev);
+
+	drm_panel_remove(&lcd->panel);
+	ls037v7dw01_disable(&lcd->panel);
+	ls037v7dw01_unprepare(&lcd->panel);
+
+	return 0;
+}
+
+static const struct of_device_id ls037v7dw01_of_match[] = {
+	{ .compatible = "sharp,ls037v7dw01", },
+	{},
+};
+
+MODULE_DEVICE_TABLE(of, ls037v7dw01_of_match);
+
+static struct platform_driver ls037v7dw01_driver = {
+	.probe		= ls037v7dw01_probe,
+	.remove		= __exit_p(ls037v7dw01_remove),
+	.driver		= {
+		.name = "panel-sharp-ls037v7dw01",
+		.of_match_table = ls037v7dw01_of_match,
+	},
+};
+
+module_platform_driver(ls037v7dw01_driver);
+
+MODULE_AUTHOR("Tomi Valkeinen <tomi.valkeinen@ti.com>");
+MODULE_DESCRIPTION("Sharp LS037V7DW01 Panel Driver");
+MODULE_LICENSE("GPL");