All of lore.kernel.org
 help / color / mirror / Atom feed
From: Thierry Reding <thierry.reding@gmail.com>
To: Maxime Ripard <maxime.ripard@bootlin.com>
Cc: Chen-Yu Tsai <wens@csie.org>, Mark Rutland <mark.rutland@arm.com>,
	Rob Herring <robh+dt@kernel.org>,
	Frank Rowand <frowand.list@gmail.com>,
	dri-devel@lists.freedesktop.org,
	Gustavo Padovan <gustavo@padovan.org>,
	Daniel Vetter <daniel.vetter@intel.com>,
	Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
	Sean Paul <seanpaul@chromium.org>,
	devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org,
	Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Subject: Re: [PATCH v4 6/8] drm/panel: Add Ilitek ILI9881c panel driver
Date: Thu, 26 Apr 2018 17:07:12 +0200	[thread overview]
Message-ID: <20180426150712.GF31888@ulmo> (raw)
In-Reply-To: <a99bb44577ae18e77e60dba904618db23ee5dca7.1522835818.git-series.maxime.ripard@bootlin.com>

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

On Wed, Apr 04, 2018 at 11:57:14AM +0200, Maxime Ripard wrote:
> The LHR050H41 panel is the panel shipped with the BananaPi M2-Magic, and is
> based on the Ilitek ILI9881c Controller. Add a driver for it, modelled
> after the other Ilitek controller drivers.
> 
> Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
> ---
>  drivers/gpu/drm/panel/Kconfig                 |   9 +-
>  drivers/gpu/drm/panel/Makefile                |   1 +-
>  drivers/gpu/drm/panel/panel-ilitek-ili9881c.c | 489 +++++++++++++++++++-
>  3 files changed, 499 insertions(+)
>  create mode 100644 drivers/gpu/drm/panel/panel-ilitek-ili9881c.c
> 
> diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
> index 25682ff3449a..6020c30a33b3 100644
> --- a/drivers/gpu/drm/panel/Kconfig
> +++ b/drivers/gpu/drm/panel/Kconfig
> @@ -46,6 +46,15 @@ config DRM_PANEL_ILITEK_IL9322
>  	  Say Y here if you want to enable support for Ilitek IL9322
>  	  QVGA (320x240) RGB, YUV and ITU-T BT.656 panels.
>  
> +config DRM_PANEL_ILITEK_ILI9881C
> +	tristate "Ilitek ILI9881C-based panels"
> +	depends on OF
> +	depends on DRM_MIPI_DSI
> +	depends on BACKLIGHT_CLASS_DEVICE
> +	help
> +	  Say Y if you want to enable support for panels based on the
> +	  Ilitek ILI9881c controller.
> +
>  config DRM_PANEL_INNOLUX_P079ZCA
>  	tristate "Innolux P079ZCA panel"
>  	depends on OF
> diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
> index f26efc11d746..5ccaaa9d13af 100644
> --- a/drivers/gpu/drm/panel/Makefile
> +++ b/drivers/gpu/drm/panel/Makefile
> @@ -3,6 +3,7 @@ obj-$(CONFIG_DRM_PANEL_ARM_VERSATILE) += panel-arm-versatile.o
>  obj-$(CONFIG_DRM_PANEL_LVDS) += panel-lvds.o
>  obj-$(CONFIG_DRM_PANEL_SIMPLE) += panel-simple.o
>  obj-$(CONFIG_DRM_PANEL_ILITEK_IL9322) += panel-ilitek-ili9322.o
> +obj-$(CONFIG_DRM_PANEL_ILITEK_ILI9881C) += panel-ilitek-ili9881c.o
>  obj-$(CONFIG_DRM_PANEL_INNOLUX_P079ZCA) += panel-innolux-p079zca.o
>  obj-$(CONFIG_DRM_PANEL_JDI_LT070ME05000) += panel-jdi-lt070me05000.o
>  obj-$(CONFIG_DRM_PANEL_LG_LG4573) += panel-lg-lg4573.o
> diff --git a/drivers/gpu/drm/panel/panel-ilitek-ili9881c.c b/drivers/gpu/drm/panel/panel-ilitek-ili9881c.c
> new file mode 100644
> index 000000000000..8992a6431c30
> --- /dev/null
> +++ b/drivers/gpu/drm/panel/panel-ilitek-ili9881c.c
> @@ -0,0 +1,489 @@
> +// SPDX-License-Identifier: GPL-2.0+

This isn't a valid SPDX license specifier. The module license is GPL v2,
so the corresponding specifier would be: GPL-2.0-only.

> +/*
> + * Copyright (C) 2017, Free Electrons

-2018?

> + * Author: Maxime Ripard <maxime.ripard@free-electrons.com>

No need for this, it's already in MODULE_AUTHOR.

> + */
> +
> +#include <linux/backlight.h>
> +#include <linux/delay.h>
> +#include <linux/device.h>
> +#include <linux/err.h>
> +#include <linux/errno.h>
> +#include <linux/fb.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +
> +#include <linux/gpio/consumer.h>
> +
> +#include <drm/drm_mipi_dsi.h>
> +#include <drm/drm_modes.h>
> +#include <drm/drm_panel.h>
> +
> +#include <video/mipi_display.h>
> +
> +struct ili9881c {
> +	struct drm_panel	panel;
> +	struct mipi_dsi_device	*dsi;
> +
> +	struct backlight_device *backlight;
> +	struct gpio_desc	*power;
> +	struct gpio_desc	*reset;
> +};
> +
> +enum ili9881c_op {
> +	ILI9881C_SWITCH_PAGE,
> +	ILI9881C_COMMAND,
> +};
> +
> +struct ili9881c_instr {
> +	enum ili9881c_op	op;
> +
> +	union arg {
> +		struct cmd {
> +			u8	cmd;
> +			u8	data;
> +		} cmd;
> +		u8	page;
> +	} arg;
> +};
> +
> +#define ILI9881C_SWITCH_PAGE_INSTR(_page)	\
> +	{					\
> +		.op = ILI9881C_SWITCH_PAGE,	\
> +		.arg = {			\
> +			.page = (_page),	\
> +		},				\
> +	}
> +
> +#define ILI9881C_COMMAND_INSTR(_cmd, _data)		\
> +	{						\
> +		.op = ILI9881C_COMMAND,		\
> +		.arg = {				\
> +			.cmd = {			\
> +				.cmd = (_cmd),		\
> +				.data = (_data),	\
> +			},				\
> +		},					\
> +	}
> +
> +static struct ili9881c_instr ili9881c_init[] = {

These are never modified, so they could be const, right?

> +	ILI9881C_SWITCH_PAGE_INSTR(3),
> +	ILI9881C_COMMAND_INSTR(0x01, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x02, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x03, 0x73),
> +	ILI9881C_COMMAND_INSTR(0x04, 0x03),
> +	ILI9881C_COMMAND_INSTR(0x05, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x06, 0x06),
> +	ILI9881C_COMMAND_INSTR(0x07, 0x06),
> +	ILI9881C_COMMAND_INSTR(0x08, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x09, 0x18),
> +	ILI9881C_COMMAND_INSTR(0x0a, 0x04),
> +	ILI9881C_COMMAND_INSTR(0x0b, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x0c, 0x02),
> +	ILI9881C_COMMAND_INSTR(0x0d, 0x03),
> +	ILI9881C_COMMAND_INSTR(0x0e, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x0f, 0x25),
> +	ILI9881C_COMMAND_INSTR(0x10, 0x25),
> +	ILI9881C_COMMAND_INSTR(0x11, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x12, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x13, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x14, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x15, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x16, 0x0C),
> +	ILI9881C_COMMAND_INSTR(0x17, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x18, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x19, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x1a, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x1b, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x1c, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x1d, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x1e, 0xC0),
> +	ILI9881C_COMMAND_INSTR(0x1f, 0x80),
> +	ILI9881C_COMMAND_INSTR(0x20, 0x04),
> +	ILI9881C_COMMAND_INSTR(0x21, 0x01),
> +	ILI9881C_COMMAND_INSTR(0x22, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x23, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x24, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x25, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x26, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x27, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x28, 0x33),
> +	ILI9881C_COMMAND_INSTR(0x29, 0x03),
> +	ILI9881C_COMMAND_INSTR(0x2a, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x2b, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x2c, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x2d, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x2e, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x2f, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x30, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x31, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x32, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x33, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x34, 0x04),
> +	ILI9881C_COMMAND_INSTR(0x35, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x36, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x37, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x38, 0x3C),
> +	ILI9881C_COMMAND_INSTR(0x39, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x3a, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x3b, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x3c, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x3d, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x3e, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x3f, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x40, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x41, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x42, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x43, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x44, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x50, 0x01),
> +	ILI9881C_COMMAND_INSTR(0x51, 0x23),
> +	ILI9881C_COMMAND_INSTR(0x52, 0x45),
> +	ILI9881C_COMMAND_INSTR(0x53, 0x67),
> +	ILI9881C_COMMAND_INSTR(0x54, 0x89),
> +	ILI9881C_COMMAND_INSTR(0x55, 0xab),
> +	ILI9881C_COMMAND_INSTR(0x56, 0x01),
> +	ILI9881C_COMMAND_INSTR(0x57, 0x23),
> +	ILI9881C_COMMAND_INSTR(0x58, 0x45),
> +	ILI9881C_COMMAND_INSTR(0x59, 0x67),
> +	ILI9881C_COMMAND_INSTR(0x5a, 0x89),
> +	ILI9881C_COMMAND_INSTR(0x5b, 0xab),
> +	ILI9881C_COMMAND_INSTR(0x5c, 0xcd),
> +	ILI9881C_COMMAND_INSTR(0x5d, 0xef),
> +	ILI9881C_COMMAND_INSTR(0x5e, 0x11),
> +	ILI9881C_COMMAND_INSTR(0x5f, 0x02),
> +	ILI9881C_COMMAND_INSTR(0x60, 0x02),
> +	ILI9881C_COMMAND_INSTR(0x61, 0x02),
> +	ILI9881C_COMMAND_INSTR(0x62, 0x02),
> +	ILI9881C_COMMAND_INSTR(0x63, 0x02),
> +	ILI9881C_COMMAND_INSTR(0x64, 0x02),
> +	ILI9881C_COMMAND_INSTR(0x65, 0x02),
> +	ILI9881C_COMMAND_INSTR(0x66, 0x02),
> +	ILI9881C_COMMAND_INSTR(0x67, 0x02),
> +	ILI9881C_COMMAND_INSTR(0x68, 0x02),
> +	ILI9881C_COMMAND_INSTR(0x69, 0x02),
> +	ILI9881C_COMMAND_INSTR(0x6a, 0x0C),
> +	ILI9881C_COMMAND_INSTR(0x6b, 0x02),
> +	ILI9881C_COMMAND_INSTR(0x6c, 0x0F),
> +	ILI9881C_COMMAND_INSTR(0x6d, 0x0E),
> +	ILI9881C_COMMAND_INSTR(0x6e, 0x0D),
> +	ILI9881C_COMMAND_INSTR(0x6f, 0x06),
> +	ILI9881C_COMMAND_INSTR(0x70, 0x07),
> +	ILI9881C_COMMAND_INSTR(0x71, 0x02),
> +	ILI9881C_COMMAND_INSTR(0x72, 0x02),
> +	ILI9881C_COMMAND_INSTR(0x73, 0x02),
> +	ILI9881C_COMMAND_INSTR(0x74, 0x02),
> +	ILI9881C_COMMAND_INSTR(0x75, 0x02),
> +	ILI9881C_COMMAND_INSTR(0x76, 0x02),
> +	ILI9881C_COMMAND_INSTR(0x77, 0x02),
> +	ILI9881C_COMMAND_INSTR(0x78, 0x02),
> +	ILI9881C_COMMAND_INSTR(0x79, 0x02),
> +	ILI9881C_COMMAND_INSTR(0x7a, 0x02),
> +	ILI9881C_COMMAND_INSTR(0x7b, 0x02),
> +	ILI9881C_COMMAND_INSTR(0x7c, 0x02),
> +	ILI9881C_COMMAND_INSTR(0x7d, 0x02),
> +	ILI9881C_COMMAND_INSTR(0x7e, 0x02),
> +	ILI9881C_COMMAND_INSTR(0x7f, 0x02),
> +	ILI9881C_COMMAND_INSTR(0x80, 0x0C),
> +	ILI9881C_COMMAND_INSTR(0x81, 0x02),
> +	ILI9881C_COMMAND_INSTR(0x82, 0x0F),
> +	ILI9881C_COMMAND_INSTR(0x83, 0x0E),
> +	ILI9881C_COMMAND_INSTR(0x84, 0x0D),
> +	ILI9881C_COMMAND_INSTR(0x85, 0x06),
> +	ILI9881C_COMMAND_INSTR(0x86, 0x07),
> +	ILI9881C_COMMAND_INSTR(0x87, 0x02),
> +	ILI9881C_COMMAND_INSTR(0x88, 0x02),
> +	ILI9881C_COMMAND_INSTR(0x89, 0x02),
> +	ILI9881C_COMMAND_INSTR(0x8A, 0x02),
> +	ILI9881C_SWITCH_PAGE_INSTR(4),
> +	ILI9881C_COMMAND_INSTR(0x6C, 0x15),
> +	ILI9881C_COMMAND_INSTR(0x6E, 0x22),
> +	ILI9881C_COMMAND_INSTR(0x6F, 0x33),
> +	ILI9881C_COMMAND_INSTR(0x3A, 0xA4),
> +	ILI9881C_COMMAND_INSTR(0x8D, 0x0D),
> +	ILI9881C_COMMAND_INSTR(0x87, 0xBA),
> +	ILI9881C_COMMAND_INSTR(0x26, 0x76),
> +	ILI9881C_COMMAND_INSTR(0xB2, 0xD1),
> +	ILI9881C_SWITCH_PAGE_INSTR(1),
> +	ILI9881C_COMMAND_INSTR(0x22, 0x0A),
> +	ILI9881C_COMMAND_INSTR(0x53, 0xDC),
> +	ILI9881C_COMMAND_INSTR(0x55, 0xA7),
> +	ILI9881C_COMMAND_INSTR(0x50, 0x78),
> +	ILI9881C_COMMAND_INSTR(0x51, 0x78),
> +	ILI9881C_COMMAND_INSTR(0x31, 0x02),
> +	ILI9881C_COMMAND_INSTR(0x60, 0x14),
> +	ILI9881C_COMMAND_INSTR(0xA0, 0x2A),
> +	ILI9881C_COMMAND_INSTR(0xA1, 0x39),
> +	ILI9881C_COMMAND_INSTR(0xA2, 0x46),
> +	ILI9881C_COMMAND_INSTR(0xA3, 0x0e),
> +	ILI9881C_COMMAND_INSTR(0xA4, 0x12),
> +	ILI9881C_COMMAND_INSTR(0xA5, 0x25),
> +	ILI9881C_COMMAND_INSTR(0xA6, 0x19),
> +	ILI9881C_COMMAND_INSTR(0xA7, 0x1d),
> +	ILI9881C_COMMAND_INSTR(0xA8, 0xa6),
> +	ILI9881C_COMMAND_INSTR(0xA9, 0x1C),
> +	ILI9881C_COMMAND_INSTR(0xAA, 0x29),
> +	ILI9881C_COMMAND_INSTR(0xAB, 0x85),
> +	ILI9881C_COMMAND_INSTR(0xAC, 0x1C),
> +	ILI9881C_COMMAND_INSTR(0xAD, 0x1B),
> +	ILI9881C_COMMAND_INSTR(0xAE, 0x51),
> +	ILI9881C_COMMAND_INSTR(0xAF, 0x22),
> +	ILI9881C_COMMAND_INSTR(0xB0, 0x2d),
> +	ILI9881C_COMMAND_INSTR(0xB1, 0x4f),
> +	ILI9881C_COMMAND_INSTR(0xB2, 0x59),
> +	ILI9881C_COMMAND_INSTR(0xB3, 0x3F),
> +	ILI9881C_COMMAND_INSTR(0xC0, 0x2A),
> +	ILI9881C_COMMAND_INSTR(0xC1, 0x3a),
> +	ILI9881C_COMMAND_INSTR(0xC2, 0x45),
> +	ILI9881C_COMMAND_INSTR(0xC3, 0x0e),
> +	ILI9881C_COMMAND_INSTR(0xC4, 0x11),
> +	ILI9881C_COMMAND_INSTR(0xC5, 0x24),
> +	ILI9881C_COMMAND_INSTR(0xC6, 0x1a),
> +	ILI9881C_COMMAND_INSTR(0xC7, 0x1c),
> +	ILI9881C_COMMAND_INSTR(0xC8, 0xaa),
> +	ILI9881C_COMMAND_INSTR(0xC9, 0x1C),
> +	ILI9881C_COMMAND_INSTR(0xCA, 0x29),
> +	ILI9881C_COMMAND_INSTR(0xCB, 0x96),
> +	ILI9881C_COMMAND_INSTR(0xCC, 0x1C),
> +	ILI9881C_COMMAND_INSTR(0xCD, 0x1B),
> +	ILI9881C_COMMAND_INSTR(0xCE, 0x51),
> +	ILI9881C_COMMAND_INSTR(0xCF, 0x22),
> +	ILI9881C_COMMAND_INSTR(0xD0, 0x2b),
> +	ILI9881C_COMMAND_INSTR(0xD1, 0x4b),
> +	ILI9881C_COMMAND_INSTR(0xD2, 0x59),
> +	ILI9881C_COMMAND_INSTR(0xD3, 0x3F),
> +};
> +
> +static inline struct ili9881c *panel_to_ili9881c(struct drm_panel *panel)
> +{
> +	return container_of(panel, struct ili9881c, panel);
> +}
> +
> +static int ili9881c_switch_page(struct ili9881c *ctx, u8 page)
> +{
> +	u8 buf[4] = { 0xff, 0x98, 0x81, page };
> +	int ret;
> +
> +	ret = mipi_dsi_dcs_write_buffer(ctx->dsi, buf, sizeof(buf));
> +	if (ret < 0)
> +		return ret;
> +
> +	return 0;
> +}
> +
> +static int ili9881c_send_cmd_data(struct ili9881c *ctx, u8 cmd, u8 data)
> +{
> +	u8 buf[2] = { cmd, data };
> +	int ret;
> +
> +	ret = mipi_dsi_dcs_write_buffer(ctx->dsi, buf, sizeof(buf));
> +	if (ret < 0)
> +		return ret;
> +
> +	return 0;
> +}

According to this you're sending DCS commands, but none of the above
look like valid DCS commands. Do you know what's going on here? Also,
can you include a reference to a datasheet where these instructions
come from?

Thierry

> +
> +static int ili9881c_prepare(struct drm_panel *panel)
> +{
> +	struct ili9881c *ctx = panel_to_ili9881c(panel);
> +	unsigned int i;
> +	int ret;
> +
> +	/* Power the panel */
> +	gpiod_set_value(ctx->power, 1);
> +	msleep(5);
> +
> +	/* And reset it */
> +	gpiod_set_value(ctx->reset, 1);
> +	msleep(20);
> +
> +	gpiod_set_value(ctx->reset, 0);
> +	msleep(20);
> +
> +	for (i = 0; i < ARRAY_SIZE(ili9881c_init); i++) {
> +		struct ili9881c_instr *instr = &ili9881c_init[i];
> +
> +		if (instr->op == ILI9881C_SWITCH_PAGE)
> +			ret = ili9881c_switch_page(ctx, instr->arg.page);
> +		else if (instr->op == ILI9881C_COMMAND)
> +			ret = ili9881c_send_cmd_data(ctx, instr->arg.cmd.cmd,
> +						      instr->arg.cmd.data);
> +
> +		if (ret)
> +			return ret;
> +	}
> +
> +	ret = ili9881c_switch_page(ctx, 0);
> +	if (ret)
> +		return ret;
> +
> +	ret = mipi_dsi_dcs_set_tear_on(ctx->dsi, MIPI_DSI_DCS_TEAR_MODE_VBLANK);
> +	if (ret)
> +		return ret;
> +
> +	mipi_dsi_dcs_exit_sleep_mode(ctx->dsi);
> +	if (ret)
> +		return ret;
> +
> +	return 0;
> +}
> +
> +static int ili9881c_enable(struct drm_panel *panel)
> +{
> +	struct ili9881c *ctx = panel_to_ili9881c(panel);
> +
> +	msleep(120);
> +
> +	mipi_dsi_dcs_set_display_on(ctx->dsi);
> +	backlight_enable(ctx->backlight);
> +
> +	return 0;
> +}
> +
> +static int ili9881c_disable(struct drm_panel *panel)
> +{
> +	struct ili9881c *ctx = panel_to_ili9881c(panel);
> +
> +	backlight_disable(ctx->backlight);
> +	return mipi_dsi_dcs_set_display_off(ctx->dsi);
> +}
> +
> +static int ili9881c_unprepare(struct drm_panel *panel)
> +{
> +	struct ili9881c *ctx = panel_to_ili9881c(panel);
> +
> +	mipi_dsi_dcs_enter_sleep_mode(ctx->dsi);
> +	gpiod_set_value(ctx->power, 0);
> +	gpiod_set_value(ctx->reset, 1);
> +
> +	return 0;
> +}
> +
> +static const struct drm_display_mode default_mode = {
> +	.clock		= 62000,
> +	.vrefresh	= 60,
> +
> +	.hdisplay	= 720,
> +	.hsync_start	= 720 + 10,
> +	.hsync_end	= 720 + 10 + 20,
> +	.htotal		= 720 + 10 + 20 + 30,
> +
> +	.vdisplay	= 1280,
> +	.vsync_start	= 1280 + 10,
> +	.vsync_end	= 1280 + 10 + 10,
> +	.vtotal		= 1280 + 10 + 10 + 20,
> +};
> +
> +static int ili9881c_get_modes(struct drm_panel *panel)
> +{
> +	struct drm_connector *connector = panel->connector;
> +	struct ili9881c *ctx = panel_to_ili9881c(panel);
> +	struct drm_display_mode *mode;
> +
> +	mode = drm_mode_duplicate(panel->drm, &default_mode);
> +	if (!mode) {
> +		dev_err(&ctx->dsi->dev, "failed to add mode %ux%ux@%u\n",
> +			default_mode.hdisplay, default_mode.vdisplay,
> +			default_mode.vrefresh);
> +		return -ENOMEM;
> +	}
> +
> +	drm_mode_set_name(mode);
> +
> +	mode->type = DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED;
> +	drm_mode_probed_add(connector, mode);
> +
> +	panel->connector->display_info.width_mm = 62;
> +	panel->connector->display_info.height_mm = 110;
> +
> +	return 1;
> +}
> +
> +static const struct drm_panel_funcs ili9881c_funcs = {
> +	.prepare	= ili9881c_prepare,
> +	.unprepare	= ili9881c_unprepare,
> +	.enable		= ili9881c_enable,
> +	.disable	= ili9881c_disable,
> +	.get_modes	= ili9881c_get_modes,
> +};
> +
> +static int ili9881c_dsi_probe(struct mipi_dsi_device *dsi)
> +{
> +	struct device_node *np;
> +	struct ili9881c *ctx;
> +	int ret;
> +
> +	ctx = devm_kzalloc(&dsi->dev, sizeof(*ctx), GFP_KERNEL);
> +	if (!ctx)
> +		return -ENOMEM;
> +	mipi_dsi_set_drvdata(dsi, ctx);
> +	ctx->dsi = dsi;
> +
> +	drm_panel_init(&ctx->panel);
> +	ctx->panel.dev = &dsi->dev;
> +	ctx->panel.funcs = &ili9881c_funcs;
> +
> +	ctx->power = devm_gpiod_get(&dsi->dev, "power", GPIOD_OUT_LOW);
> +	if (IS_ERR(ctx->power)) {
> +		dev_err(&dsi->dev, "Couldn't get our power GPIO\n");
> +		return PTR_ERR(ctx->power);
> +	}
> +
> +	ctx->reset = devm_gpiod_get(&dsi->dev, "reset", GPIOD_OUT_LOW);
> +	if (IS_ERR(ctx->reset)) {
> +		dev_err(&dsi->dev, "Couldn't get our reset GPIO\n");
> +		return PTR_ERR(ctx->reset);
> +	}
> +
> +	np = of_parse_phandle(dsi->dev.of_node, "backlight", 0);
> +	if (np) {
> +		ctx->backlight = of_find_backlight_by_node(np);
> +		of_node_put(np);
> +
> +		if (!ctx->backlight)
> +			return -EPROBE_DEFER;
> +	}
> +
> +	ret = drm_panel_add(&ctx->panel);
> +	if (ret < 0)
> +		return ret;
> +
> +	dsi->mode_flags = MIPI_DSI_MODE_VIDEO_SYNC_PULSE;
> +	dsi->format = MIPI_DSI_FMT_RGB888;
> +	dsi->lanes = 4;
> +
> +	return mipi_dsi_attach(dsi);
> +}
> +
> +static int ili9881c_dsi_remove(struct mipi_dsi_device *dsi)
> +{
> +	struct ili9881c *ctx = mipi_dsi_get_drvdata(dsi);
> +
> +	mipi_dsi_detach(dsi);
> +	drm_panel_remove(&ctx->panel);
> +
> +	if (ctx->backlight)
> +		put_device(&ctx->backlight->dev);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id ili9881c_of_match[] = {
> +	{ .compatible = "bananapi,lhr050h41" },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, ili9881c_of_match);
> +
> +static struct mipi_dsi_driver ili9881c_dsi_driver = {
> +	.probe		= ili9881c_dsi_probe,
> +	.remove		= ili9881c_dsi_remove,
> +	.driver = {
> +		.name		= "ili9881c-dsi",
> +		.of_match_table	= ili9881c_of_match,
> +	},
> +};
> +module_mipi_dsi_driver(ili9881c_dsi_driver);
> +
> +MODULE_AUTHOR("Maxime Ripard <maxime.ripard@free-electrons.com>");
> +MODULE_DESCRIPTION("Ilitek ILI9881C Controller Driver");
> +MODULE_LICENSE("GPL v2");
> -- 
> git-series 0.9.1

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

WARNING: multiple messages have this Message-ID (diff)
From: Thierry Reding <thierry.reding@gmail.com>
To: Maxime Ripard <maxime.ripard@bootlin.com>
Cc: Mark Rutland <mark.rutland@arm.com>,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
	dri-devel@lists.freedesktop.org, Chen-Yu Tsai <wens@csie.org>,
	Rob Herring <robh+dt@kernel.org>,
	Thomas Petazzoni <thomas.petazzoni@bootlin.com>,
	Daniel Vetter <daniel.vetter@intel.com>,
	Frank Rowand <frowand.list@gmail.com>,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v4 6/8] drm/panel: Add Ilitek ILI9881c panel driver
Date: Thu, 26 Apr 2018 17:07:12 +0200	[thread overview]
Message-ID: <20180426150712.GF31888@ulmo> (raw)
In-Reply-To: <a99bb44577ae18e77e60dba904618db23ee5dca7.1522835818.git-series.maxime.ripard@bootlin.com>


[-- Attachment #1.1: Type: text/plain, Size: 18156 bytes --]

On Wed, Apr 04, 2018 at 11:57:14AM +0200, Maxime Ripard wrote:
> The LHR050H41 panel is the panel shipped with the BananaPi M2-Magic, and is
> based on the Ilitek ILI9881c Controller. Add a driver for it, modelled
> after the other Ilitek controller drivers.
> 
> Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
> ---
>  drivers/gpu/drm/panel/Kconfig                 |   9 +-
>  drivers/gpu/drm/panel/Makefile                |   1 +-
>  drivers/gpu/drm/panel/panel-ilitek-ili9881c.c | 489 +++++++++++++++++++-
>  3 files changed, 499 insertions(+)
>  create mode 100644 drivers/gpu/drm/panel/panel-ilitek-ili9881c.c
> 
> diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
> index 25682ff3449a..6020c30a33b3 100644
> --- a/drivers/gpu/drm/panel/Kconfig
> +++ b/drivers/gpu/drm/panel/Kconfig
> @@ -46,6 +46,15 @@ config DRM_PANEL_ILITEK_IL9322
>  	  Say Y here if you want to enable support for Ilitek IL9322
>  	  QVGA (320x240) RGB, YUV and ITU-T BT.656 panels.
>  
> +config DRM_PANEL_ILITEK_ILI9881C
> +	tristate "Ilitek ILI9881C-based panels"
> +	depends on OF
> +	depends on DRM_MIPI_DSI
> +	depends on BACKLIGHT_CLASS_DEVICE
> +	help
> +	  Say Y if you want to enable support for panels based on the
> +	  Ilitek ILI9881c controller.
> +
>  config DRM_PANEL_INNOLUX_P079ZCA
>  	tristate "Innolux P079ZCA panel"
>  	depends on OF
> diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
> index f26efc11d746..5ccaaa9d13af 100644
> --- a/drivers/gpu/drm/panel/Makefile
> +++ b/drivers/gpu/drm/panel/Makefile
> @@ -3,6 +3,7 @@ obj-$(CONFIG_DRM_PANEL_ARM_VERSATILE) += panel-arm-versatile.o
>  obj-$(CONFIG_DRM_PANEL_LVDS) += panel-lvds.o
>  obj-$(CONFIG_DRM_PANEL_SIMPLE) += panel-simple.o
>  obj-$(CONFIG_DRM_PANEL_ILITEK_IL9322) += panel-ilitek-ili9322.o
> +obj-$(CONFIG_DRM_PANEL_ILITEK_ILI9881C) += panel-ilitek-ili9881c.o
>  obj-$(CONFIG_DRM_PANEL_INNOLUX_P079ZCA) += panel-innolux-p079zca.o
>  obj-$(CONFIG_DRM_PANEL_JDI_LT070ME05000) += panel-jdi-lt070me05000.o
>  obj-$(CONFIG_DRM_PANEL_LG_LG4573) += panel-lg-lg4573.o
> diff --git a/drivers/gpu/drm/panel/panel-ilitek-ili9881c.c b/drivers/gpu/drm/panel/panel-ilitek-ili9881c.c
> new file mode 100644
> index 000000000000..8992a6431c30
> --- /dev/null
> +++ b/drivers/gpu/drm/panel/panel-ilitek-ili9881c.c
> @@ -0,0 +1,489 @@
> +// SPDX-License-Identifier: GPL-2.0+

This isn't a valid SPDX license specifier. The module license is GPL v2,
so the corresponding specifier would be: GPL-2.0-only.

> +/*
> + * Copyright (C) 2017, Free Electrons

-2018?

> + * Author: Maxime Ripard <maxime.ripard@free-electrons.com>

No need for this, it's already in MODULE_AUTHOR.

> + */
> +
> +#include <linux/backlight.h>
> +#include <linux/delay.h>
> +#include <linux/device.h>
> +#include <linux/err.h>
> +#include <linux/errno.h>
> +#include <linux/fb.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +
> +#include <linux/gpio/consumer.h>
> +
> +#include <drm/drm_mipi_dsi.h>
> +#include <drm/drm_modes.h>
> +#include <drm/drm_panel.h>
> +
> +#include <video/mipi_display.h>
> +
> +struct ili9881c {
> +	struct drm_panel	panel;
> +	struct mipi_dsi_device	*dsi;
> +
> +	struct backlight_device *backlight;
> +	struct gpio_desc	*power;
> +	struct gpio_desc	*reset;
> +};
> +
> +enum ili9881c_op {
> +	ILI9881C_SWITCH_PAGE,
> +	ILI9881C_COMMAND,
> +};
> +
> +struct ili9881c_instr {
> +	enum ili9881c_op	op;
> +
> +	union arg {
> +		struct cmd {
> +			u8	cmd;
> +			u8	data;
> +		} cmd;
> +		u8	page;
> +	} arg;
> +};
> +
> +#define ILI9881C_SWITCH_PAGE_INSTR(_page)	\
> +	{					\
> +		.op = ILI9881C_SWITCH_PAGE,	\
> +		.arg = {			\
> +			.page = (_page),	\
> +		},				\
> +	}
> +
> +#define ILI9881C_COMMAND_INSTR(_cmd, _data)		\
> +	{						\
> +		.op = ILI9881C_COMMAND,		\
> +		.arg = {				\
> +			.cmd = {			\
> +				.cmd = (_cmd),		\
> +				.data = (_data),	\
> +			},				\
> +		},					\
> +	}
> +
> +static struct ili9881c_instr ili9881c_init[] = {

These are never modified, so they could be const, right?

> +	ILI9881C_SWITCH_PAGE_INSTR(3),
> +	ILI9881C_COMMAND_INSTR(0x01, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x02, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x03, 0x73),
> +	ILI9881C_COMMAND_INSTR(0x04, 0x03),
> +	ILI9881C_COMMAND_INSTR(0x05, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x06, 0x06),
> +	ILI9881C_COMMAND_INSTR(0x07, 0x06),
> +	ILI9881C_COMMAND_INSTR(0x08, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x09, 0x18),
> +	ILI9881C_COMMAND_INSTR(0x0a, 0x04),
> +	ILI9881C_COMMAND_INSTR(0x0b, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x0c, 0x02),
> +	ILI9881C_COMMAND_INSTR(0x0d, 0x03),
> +	ILI9881C_COMMAND_INSTR(0x0e, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x0f, 0x25),
> +	ILI9881C_COMMAND_INSTR(0x10, 0x25),
> +	ILI9881C_COMMAND_INSTR(0x11, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x12, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x13, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x14, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x15, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x16, 0x0C),
> +	ILI9881C_COMMAND_INSTR(0x17, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x18, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x19, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x1a, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x1b, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x1c, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x1d, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x1e, 0xC0),
> +	ILI9881C_COMMAND_INSTR(0x1f, 0x80),
> +	ILI9881C_COMMAND_INSTR(0x20, 0x04),
> +	ILI9881C_COMMAND_INSTR(0x21, 0x01),
> +	ILI9881C_COMMAND_INSTR(0x22, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x23, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x24, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x25, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x26, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x27, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x28, 0x33),
> +	ILI9881C_COMMAND_INSTR(0x29, 0x03),
> +	ILI9881C_COMMAND_INSTR(0x2a, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x2b, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x2c, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x2d, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x2e, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x2f, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x30, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x31, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x32, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x33, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x34, 0x04),
> +	ILI9881C_COMMAND_INSTR(0x35, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x36, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x37, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x38, 0x3C),
> +	ILI9881C_COMMAND_INSTR(0x39, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x3a, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x3b, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x3c, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x3d, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x3e, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x3f, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x40, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x41, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x42, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x43, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x44, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x50, 0x01),
> +	ILI9881C_COMMAND_INSTR(0x51, 0x23),
> +	ILI9881C_COMMAND_INSTR(0x52, 0x45),
> +	ILI9881C_COMMAND_INSTR(0x53, 0x67),
> +	ILI9881C_COMMAND_INSTR(0x54, 0x89),
> +	ILI9881C_COMMAND_INSTR(0x55, 0xab),
> +	ILI9881C_COMMAND_INSTR(0x56, 0x01),
> +	ILI9881C_COMMAND_INSTR(0x57, 0x23),
> +	ILI9881C_COMMAND_INSTR(0x58, 0x45),
> +	ILI9881C_COMMAND_INSTR(0x59, 0x67),
> +	ILI9881C_COMMAND_INSTR(0x5a, 0x89),
> +	ILI9881C_COMMAND_INSTR(0x5b, 0xab),
> +	ILI9881C_COMMAND_INSTR(0x5c, 0xcd),
> +	ILI9881C_COMMAND_INSTR(0x5d, 0xef),
> +	ILI9881C_COMMAND_INSTR(0x5e, 0x11),
> +	ILI9881C_COMMAND_INSTR(0x5f, 0x02),
> +	ILI9881C_COMMAND_INSTR(0x60, 0x02),
> +	ILI9881C_COMMAND_INSTR(0x61, 0x02),
> +	ILI9881C_COMMAND_INSTR(0x62, 0x02),
> +	ILI9881C_COMMAND_INSTR(0x63, 0x02),
> +	ILI9881C_COMMAND_INSTR(0x64, 0x02),
> +	ILI9881C_COMMAND_INSTR(0x65, 0x02),
> +	ILI9881C_COMMAND_INSTR(0x66, 0x02),
> +	ILI9881C_COMMAND_INSTR(0x67, 0x02),
> +	ILI9881C_COMMAND_INSTR(0x68, 0x02),
> +	ILI9881C_COMMAND_INSTR(0x69, 0x02),
> +	ILI9881C_COMMAND_INSTR(0x6a, 0x0C),
> +	ILI9881C_COMMAND_INSTR(0x6b, 0x02),
> +	ILI9881C_COMMAND_INSTR(0x6c, 0x0F),
> +	ILI9881C_COMMAND_INSTR(0x6d, 0x0E),
> +	ILI9881C_COMMAND_INSTR(0x6e, 0x0D),
> +	ILI9881C_COMMAND_INSTR(0x6f, 0x06),
> +	ILI9881C_COMMAND_INSTR(0x70, 0x07),
> +	ILI9881C_COMMAND_INSTR(0x71, 0x02),
> +	ILI9881C_COMMAND_INSTR(0x72, 0x02),
> +	ILI9881C_COMMAND_INSTR(0x73, 0x02),
> +	ILI9881C_COMMAND_INSTR(0x74, 0x02),
> +	ILI9881C_COMMAND_INSTR(0x75, 0x02),
> +	ILI9881C_COMMAND_INSTR(0x76, 0x02),
> +	ILI9881C_COMMAND_INSTR(0x77, 0x02),
> +	ILI9881C_COMMAND_INSTR(0x78, 0x02),
> +	ILI9881C_COMMAND_INSTR(0x79, 0x02),
> +	ILI9881C_COMMAND_INSTR(0x7a, 0x02),
> +	ILI9881C_COMMAND_INSTR(0x7b, 0x02),
> +	ILI9881C_COMMAND_INSTR(0x7c, 0x02),
> +	ILI9881C_COMMAND_INSTR(0x7d, 0x02),
> +	ILI9881C_COMMAND_INSTR(0x7e, 0x02),
> +	ILI9881C_COMMAND_INSTR(0x7f, 0x02),
> +	ILI9881C_COMMAND_INSTR(0x80, 0x0C),
> +	ILI9881C_COMMAND_INSTR(0x81, 0x02),
> +	ILI9881C_COMMAND_INSTR(0x82, 0x0F),
> +	ILI9881C_COMMAND_INSTR(0x83, 0x0E),
> +	ILI9881C_COMMAND_INSTR(0x84, 0x0D),
> +	ILI9881C_COMMAND_INSTR(0x85, 0x06),
> +	ILI9881C_COMMAND_INSTR(0x86, 0x07),
> +	ILI9881C_COMMAND_INSTR(0x87, 0x02),
> +	ILI9881C_COMMAND_INSTR(0x88, 0x02),
> +	ILI9881C_COMMAND_INSTR(0x89, 0x02),
> +	ILI9881C_COMMAND_INSTR(0x8A, 0x02),
> +	ILI9881C_SWITCH_PAGE_INSTR(4),
> +	ILI9881C_COMMAND_INSTR(0x6C, 0x15),
> +	ILI9881C_COMMAND_INSTR(0x6E, 0x22),
> +	ILI9881C_COMMAND_INSTR(0x6F, 0x33),
> +	ILI9881C_COMMAND_INSTR(0x3A, 0xA4),
> +	ILI9881C_COMMAND_INSTR(0x8D, 0x0D),
> +	ILI9881C_COMMAND_INSTR(0x87, 0xBA),
> +	ILI9881C_COMMAND_INSTR(0x26, 0x76),
> +	ILI9881C_COMMAND_INSTR(0xB2, 0xD1),
> +	ILI9881C_SWITCH_PAGE_INSTR(1),
> +	ILI9881C_COMMAND_INSTR(0x22, 0x0A),
> +	ILI9881C_COMMAND_INSTR(0x53, 0xDC),
> +	ILI9881C_COMMAND_INSTR(0x55, 0xA7),
> +	ILI9881C_COMMAND_INSTR(0x50, 0x78),
> +	ILI9881C_COMMAND_INSTR(0x51, 0x78),
> +	ILI9881C_COMMAND_INSTR(0x31, 0x02),
> +	ILI9881C_COMMAND_INSTR(0x60, 0x14),
> +	ILI9881C_COMMAND_INSTR(0xA0, 0x2A),
> +	ILI9881C_COMMAND_INSTR(0xA1, 0x39),
> +	ILI9881C_COMMAND_INSTR(0xA2, 0x46),
> +	ILI9881C_COMMAND_INSTR(0xA3, 0x0e),
> +	ILI9881C_COMMAND_INSTR(0xA4, 0x12),
> +	ILI9881C_COMMAND_INSTR(0xA5, 0x25),
> +	ILI9881C_COMMAND_INSTR(0xA6, 0x19),
> +	ILI9881C_COMMAND_INSTR(0xA7, 0x1d),
> +	ILI9881C_COMMAND_INSTR(0xA8, 0xa6),
> +	ILI9881C_COMMAND_INSTR(0xA9, 0x1C),
> +	ILI9881C_COMMAND_INSTR(0xAA, 0x29),
> +	ILI9881C_COMMAND_INSTR(0xAB, 0x85),
> +	ILI9881C_COMMAND_INSTR(0xAC, 0x1C),
> +	ILI9881C_COMMAND_INSTR(0xAD, 0x1B),
> +	ILI9881C_COMMAND_INSTR(0xAE, 0x51),
> +	ILI9881C_COMMAND_INSTR(0xAF, 0x22),
> +	ILI9881C_COMMAND_INSTR(0xB0, 0x2d),
> +	ILI9881C_COMMAND_INSTR(0xB1, 0x4f),
> +	ILI9881C_COMMAND_INSTR(0xB2, 0x59),
> +	ILI9881C_COMMAND_INSTR(0xB3, 0x3F),
> +	ILI9881C_COMMAND_INSTR(0xC0, 0x2A),
> +	ILI9881C_COMMAND_INSTR(0xC1, 0x3a),
> +	ILI9881C_COMMAND_INSTR(0xC2, 0x45),
> +	ILI9881C_COMMAND_INSTR(0xC3, 0x0e),
> +	ILI9881C_COMMAND_INSTR(0xC4, 0x11),
> +	ILI9881C_COMMAND_INSTR(0xC5, 0x24),
> +	ILI9881C_COMMAND_INSTR(0xC6, 0x1a),
> +	ILI9881C_COMMAND_INSTR(0xC7, 0x1c),
> +	ILI9881C_COMMAND_INSTR(0xC8, 0xaa),
> +	ILI9881C_COMMAND_INSTR(0xC9, 0x1C),
> +	ILI9881C_COMMAND_INSTR(0xCA, 0x29),
> +	ILI9881C_COMMAND_INSTR(0xCB, 0x96),
> +	ILI9881C_COMMAND_INSTR(0xCC, 0x1C),
> +	ILI9881C_COMMAND_INSTR(0xCD, 0x1B),
> +	ILI9881C_COMMAND_INSTR(0xCE, 0x51),
> +	ILI9881C_COMMAND_INSTR(0xCF, 0x22),
> +	ILI9881C_COMMAND_INSTR(0xD0, 0x2b),
> +	ILI9881C_COMMAND_INSTR(0xD1, 0x4b),
> +	ILI9881C_COMMAND_INSTR(0xD2, 0x59),
> +	ILI9881C_COMMAND_INSTR(0xD3, 0x3F),
> +};
> +
> +static inline struct ili9881c *panel_to_ili9881c(struct drm_panel *panel)
> +{
> +	return container_of(panel, struct ili9881c, panel);
> +}
> +
> +static int ili9881c_switch_page(struct ili9881c *ctx, u8 page)
> +{
> +	u8 buf[4] = { 0xff, 0x98, 0x81, page };
> +	int ret;
> +
> +	ret = mipi_dsi_dcs_write_buffer(ctx->dsi, buf, sizeof(buf));
> +	if (ret < 0)
> +		return ret;
> +
> +	return 0;
> +}
> +
> +static int ili9881c_send_cmd_data(struct ili9881c *ctx, u8 cmd, u8 data)
> +{
> +	u8 buf[2] = { cmd, data };
> +	int ret;
> +
> +	ret = mipi_dsi_dcs_write_buffer(ctx->dsi, buf, sizeof(buf));
> +	if (ret < 0)
> +		return ret;
> +
> +	return 0;
> +}

According to this you're sending DCS commands, but none of the above
look like valid DCS commands. Do you know what's going on here? Also,
can you include a reference to a datasheet where these instructions
come from?

Thierry

> +
> +static int ili9881c_prepare(struct drm_panel *panel)
> +{
> +	struct ili9881c *ctx = panel_to_ili9881c(panel);
> +	unsigned int i;
> +	int ret;
> +
> +	/* Power the panel */
> +	gpiod_set_value(ctx->power, 1);
> +	msleep(5);
> +
> +	/* And reset it */
> +	gpiod_set_value(ctx->reset, 1);
> +	msleep(20);
> +
> +	gpiod_set_value(ctx->reset, 0);
> +	msleep(20);
> +
> +	for (i = 0; i < ARRAY_SIZE(ili9881c_init); i++) {
> +		struct ili9881c_instr *instr = &ili9881c_init[i];
> +
> +		if (instr->op == ILI9881C_SWITCH_PAGE)
> +			ret = ili9881c_switch_page(ctx, instr->arg.page);
> +		else if (instr->op == ILI9881C_COMMAND)
> +			ret = ili9881c_send_cmd_data(ctx, instr->arg.cmd.cmd,
> +						      instr->arg.cmd.data);
> +
> +		if (ret)
> +			return ret;
> +	}
> +
> +	ret = ili9881c_switch_page(ctx, 0);
> +	if (ret)
> +		return ret;
> +
> +	ret = mipi_dsi_dcs_set_tear_on(ctx->dsi, MIPI_DSI_DCS_TEAR_MODE_VBLANK);
> +	if (ret)
> +		return ret;
> +
> +	mipi_dsi_dcs_exit_sleep_mode(ctx->dsi);
> +	if (ret)
> +		return ret;
> +
> +	return 0;
> +}
> +
> +static int ili9881c_enable(struct drm_panel *panel)
> +{
> +	struct ili9881c *ctx = panel_to_ili9881c(panel);
> +
> +	msleep(120);
> +
> +	mipi_dsi_dcs_set_display_on(ctx->dsi);
> +	backlight_enable(ctx->backlight);
> +
> +	return 0;
> +}
> +
> +static int ili9881c_disable(struct drm_panel *panel)
> +{
> +	struct ili9881c *ctx = panel_to_ili9881c(panel);
> +
> +	backlight_disable(ctx->backlight);
> +	return mipi_dsi_dcs_set_display_off(ctx->dsi);
> +}
> +
> +static int ili9881c_unprepare(struct drm_panel *panel)
> +{
> +	struct ili9881c *ctx = panel_to_ili9881c(panel);
> +
> +	mipi_dsi_dcs_enter_sleep_mode(ctx->dsi);
> +	gpiod_set_value(ctx->power, 0);
> +	gpiod_set_value(ctx->reset, 1);
> +
> +	return 0;
> +}
> +
> +static const struct drm_display_mode default_mode = {
> +	.clock		= 62000,
> +	.vrefresh	= 60,
> +
> +	.hdisplay	= 720,
> +	.hsync_start	= 720 + 10,
> +	.hsync_end	= 720 + 10 + 20,
> +	.htotal		= 720 + 10 + 20 + 30,
> +
> +	.vdisplay	= 1280,
> +	.vsync_start	= 1280 + 10,
> +	.vsync_end	= 1280 + 10 + 10,
> +	.vtotal		= 1280 + 10 + 10 + 20,
> +};
> +
> +static int ili9881c_get_modes(struct drm_panel *panel)
> +{
> +	struct drm_connector *connector = panel->connector;
> +	struct ili9881c *ctx = panel_to_ili9881c(panel);
> +	struct drm_display_mode *mode;
> +
> +	mode = drm_mode_duplicate(panel->drm, &default_mode);
> +	if (!mode) {
> +		dev_err(&ctx->dsi->dev, "failed to add mode %ux%ux@%u\n",
> +			default_mode.hdisplay, default_mode.vdisplay,
> +			default_mode.vrefresh);
> +		return -ENOMEM;
> +	}
> +
> +	drm_mode_set_name(mode);
> +
> +	mode->type = DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED;
> +	drm_mode_probed_add(connector, mode);
> +
> +	panel->connector->display_info.width_mm = 62;
> +	panel->connector->display_info.height_mm = 110;
> +
> +	return 1;
> +}
> +
> +static const struct drm_panel_funcs ili9881c_funcs = {
> +	.prepare	= ili9881c_prepare,
> +	.unprepare	= ili9881c_unprepare,
> +	.enable		= ili9881c_enable,
> +	.disable	= ili9881c_disable,
> +	.get_modes	= ili9881c_get_modes,
> +};
> +
> +static int ili9881c_dsi_probe(struct mipi_dsi_device *dsi)
> +{
> +	struct device_node *np;
> +	struct ili9881c *ctx;
> +	int ret;
> +
> +	ctx = devm_kzalloc(&dsi->dev, sizeof(*ctx), GFP_KERNEL);
> +	if (!ctx)
> +		return -ENOMEM;
> +	mipi_dsi_set_drvdata(dsi, ctx);
> +	ctx->dsi = dsi;
> +
> +	drm_panel_init(&ctx->panel);
> +	ctx->panel.dev = &dsi->dev;
> +	ctx->panel.funcs = &ili9881c_funcs;
> +
> +	ctx->power = devm_gpiod_get(&dsi->dev, "power", GPIOD_OUT_LOW);
> +	if (IS_ERR(ctx->power)) {
> +		dev_err(&dsi->dev, "Couldn't get our power GPIO\n");
> +		return PTR_ERR(ctx->power);
> +	}
> +
> +	ctx->reset = devm_gpiod_get(&dsi->dev, "reset", GPIOD_OUT_LOW);
> +	if (IS_ERR(ctx->reset)) {
> +		dev_err(&dsi->dev, "Couldn't get our reset GPIO\n");
> +		return PTR_ERR(ctx->reset);
> +	}
> +
> +	np = of_parse_phandle(dsi->dev.of_node, "backlight", 0);
> +	if (np) {
> +		ctx->backlight = of_find_backlight_by_node(np);
> +		of_node_put(np);
> +
> +		if (!ctx->backlight)
> +			return -EPROBE_DEFER;
> +	}
> +
> +	ret = drm_panel_add(&ctx->panel);
> +	if (ret < 0)
> +		return ret;
> +
> +	dsi->mode_flags = MIPI_DSI_MODE_VIDEO_SYNC_PULSE;
> +	dsi->format = MIPI_DSI_FMT_RGB888;
> +	dsi->lanes = 4;
> +
> +	return mipi_dsi_attach(dsi);
> +}
> +
> +static int ili9881c_dsi_remove(struct mipi_dsi_device *dsi)
> +{
> +	struct ili9881c *ctx = mipi_dsi_get_drvdata(dsi);
> +
> +	mipi_dsi_detach(dsi);
> +	drm_panel_remove(&ctx->panel);
> +
> +	if (ctx->backlight)
> +		put_device(&ctx->backlight->dev);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id ili9881c_of_match[] = {
> +	{ .compatible = "bananapi,lhr050h41" },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, ili9881c_of_match);
> +
> +static struct mipi_dsi_driver ili9881c_dsi_driver = {
> +	.probe		= ili9881c_dsi_probe,
> +	.remove		= ili9881c_dsi_remove,
> +	.driver = {
> +		.name		= "ili9881c-dsi",
> +		.of_match_table	= ili9881c_of_match,
> +	},
> +};
> +module_mipi_dsi_driver(ili9881c_dsi_driver);
> +
> +MODULE_AUTHOR("Maxime Ripard <maxime.ripard@free-electrons.com>");
> +MODULE_DESCRIPTION("Ilitek ILI9881C Controller Driver");
> +MODULE_LICENSE("GPL v2");
> -- 
> git-series 0.9.1

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

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

WARNING: multiple messages have this Message-ID (diff)
From: thierry.reding@gmail.com (Thierry Reding)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v4 6/8] drm/panel: Add Ilitek ILI9881c panel driver
Date: Thu, 26 Apr 2018 17:07:12 +0200	[thread overview]
Message-ID: <20180426150712.GF31888@ulmo> (raw)
In-Reply-To: <a99bb44577ae18e77e60dba904618db23ee5dca7.1522835818.git-series.maxime.ripard@bootlin.com>

On Wed, Apr 04, 2018 at 11:57:14AM +0200, Maxime Ripard wrote:
> The LHR050H41 panel is the panel shipped with the BananaPi M2-Magic, and is
> based on the Ilitek ILI9881c Controller. Add a driver for it, modelled
> after the other Ilitek controller drivers.
> 
> Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
> ---
>  drivers/gpu/drm/panel/Kconfig                 |   9 +-
>  drivers/gpu/drm/panel/Makefile                |   1 +-
>  drivers/gpu/drm/panel/panel-ilitek-ili9881c.c | 489 +++++++++++++++++++-
>  3 files changed, 499 insertions(+)
>  create mode 100644 drivers/gpu/drm/panel/panel-ilitek-ili9881c.c
> 
> diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
> index 25682ff3449a..6020c30a33b3 100644
> --- a/drivers/gpu/drm/panel/Kconfig
> +++ b/drivers/gpu/drm/panel/Kconfig
> @@ -46,6 +46,15 @@ config DRM_PANEL_ILITEK_IL9322
>  	  Say Y here if you want to enable support for Ilitek IL9322
>  	  QVGA (320x240) RGB, YUV and ITU-T BT.656 panels.
>  
> +config DRM_PANEL_ILITEK_ILI9881C
> +	tristate "Ilitek ILI9881C-based panels"
> +	depends on OF
> +	depends on DRM_MIPI_DSI
> +	depends on BACKLIGHT_CLASS_DEVICE
> +	help
> +	  Say Y if you want to enable support for panels based on the
> +	  Ilitek ILI9881c controller.
> +
>  config DRM_PANEL_INNOLUX_P079ZCA
>  	tristate "Innolux P079ZCA panel"
>  	depends on OF
> diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
> index f26efc11d746..5ccaaa9d13af 100644
> --- a/drivers/gpu/drm/panel/Makefile
> +++ b/drivers/gpu/drm/panel/Makefile
> @@ -3,6 +3,7 @@ obj-$(CONFIG_DRM_PANEL_ARM_VERSATILE) += panel-arm-versatile.o
>  obj-$(CONFIG_DRM_PANEL_LVDS) += panel-lvds.o
>  obj-$(CONFIG_DRM_PANEL_SIMPLE) += panel-simple.o
>  obj-$(CONFIG_DRM_PANEL_ILITEK_IL9322) += panel-ilitek-ili9322.o
> +obj-$(CONFIG_DRM_PANEL_ILITEK_ILI9881C) += panel-ilitek-ili9881c.o
>  obj-$(CONFIG_DRM_PANEL_INNOLUX_P079ZCA) += panel-innolux-p079zca.o
>  obj-$(CONFIG_DRM_PANEL_JDI_LT070ME05000) += panel-jdi-lt070me05000.o
>  obj-$(CONFIG_DRM_PANEL_LG_LG4573) += panel-lg-lg4573.o
> diff --git a/drivers/gpu/drm/panel/panel-ilitek-ili9881c.c b/drivers/gpu/drm/panel/panel-ilitek-ili9881c.c
> new file mode 100644
> index 000000000000..8992a6431c30
> --- /dev/null
> +++ b/drivers/gpu/drm/panel/panel-ilitek-ili9881c.c
> @@ -0,0 +1,489 @@
> +// SPDX-License-Identifier: GPL-2.0+

This isn't a valid SPDX license specifier. The module license is GPL v2,
so the corresponding specifier would be: GPL-2.0-only.

> +/*
> + * Copyright (C) 2017, Free Electrons

-2018?

> + * Author: Maxime Ripard <maxime.ripard@free-electrons.com>

No need for this, it's already in MODULE_AUTHOR.

> + */
> +
> +#include <linux/backlight.h>
> +#include <linux/delay.h>
> +#include <linux/device.h>
> +#include <linux/err.h>
> +#include <linux/errno.h>
> +#include <linux/fb.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +
> +#include <linux/gpio/consumer.h>
> +
> +#include <drm/drm_mipi_dsi.h>
> +#include <drm/drm_modes.h>
> +#include <drm/drm_panel.h>
> +
> +#include <video/mipi_display.h>
> +
> +struct ili9881c {
> +	struct drm_panel	panel;
> +	struct mipi_dsi_device	*dsi;
> +
> +	struct backlight_device *backlight;
> +	struct gpio_desc	*power;
> +	struct gpio_desc	*reset;
> +};
> +
> +enum ili9881c_op {
> +	ILI9881C_SWITCH_PAGE,
> +	ILI9881C_COMMAND,
> +};
> +
> +struct ili9881c_instr {
> +	enum ili9881c_op	op;
> +
> +	union arg {
> +		struct cmd {
> +			u8	cmd;
> +			u8	data;
> +		} cmd;
> +		u8	page;
> +	} arg;
> +};
> +
> +#define ILI9881C_SWITCH_PAGE_INSTR(_page)	\
> +	{					\
> +		.op = ILI9881C_SWITCH_PAGE,	\
> +		.arg = {			\
> +			.page = (_page),	\
> +		},				\
> +	}
> +
> +#define ILI9881C_COMMAND_INSTR(_cmd, _data)		\
> +	{						\
> +		.op = ILI9881C_COMMAND,		\
> +		.arg = {				\
> +			.cmd = {			\
> +				.cmd = (_cmd),		\
> +				.data = (_data),	\
> +			},				\
> +		},					\
> +	}
> +
> +static struct ili9881c_instr ili9881c_init[] = {

These are never modified, so they could be const, right?

> +	ILI9881C_SWITCH_PAGE_INSTR(3),
> +	ILI9881C_COMMAND_INSTR(0x01, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x02, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x03, 0x73),
> +	ILI9881C_COMMAND_INSTR(0x04, 0x03),
> +	ILI9881C_COMMAND_INSTR(0x05, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x06, 0x06),
> +	ILI9881C_COMMAND_INSTR(0x07, 0x06),
> +	ILI9881C_COMMAND_INSTR(0x08, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x09, 0x18),
> +	ILI9881C_COMMAND_INSTR(0x0a, 0x04),
> +	ILI9881C_COMMAND_INSTR(0x0b, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x0c, 0x02),
> +	ILI9881C_COMMAND_INSTR(0x0d, 0x03),
> +	ILI9881C_COMMAND_INSTR(0x0e, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x0f, 0x25),
> +	ILI9881C_COMMAND_INSTR(0x10, 0x25),
> +	ILI9881C_COMMAND_INSTR(0x11, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x12, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x13, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x14, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x15, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x16, 0x0C),
> +	ILI9881C_COMMAND_INSTR(0x17, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x18, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x19, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x1a, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x1b, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x1c, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x1d, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x1e, 0xC0),
> +	ILI9881C_COMMAND_INSTR(0x1f, 0x80),
> +	ILI9881C_COMMAND_INSTR(0x20, 0x04),
> +	ILI9881C_COMMAND_INSTR(0x21, 0x01),
> +	ILI9881C_COMMAND_INSTR(0x22, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x23, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x24, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x25, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x26, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x27, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x28, 0x33),
> +	ILI9881C_COMMAND_INSTR(0x29, 0x03),
> +	ILI9881C_COMMAND_INSTR(0x2a, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x2b, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x2c, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x2d, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x2e, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x2f, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x30, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x31, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x32, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x33, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x34, 0x04),
> +	ILI9881C_COMMAND_INSTR(0x35, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x36, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x37, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x38, 0x3C),
> +	ILI9881C_COMMAND_INSTR(0x39, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x3a, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x3b, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x3c, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x3d, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x3e, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x3f, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x40, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x41, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x42, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x43, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x44, 0x00),
> +	ILI9881C_COMMAND_INSTR(0x50, 0x01),
> +	ILI9881C_COMMAND_INSTR(0x51, 0x23),
> +	ILI9881C_COMMAND_INSTR(0x52, 0x45),
> +	ILI9881C_COMMAND_INSTR(0x53, 0x67),
> +	ILI9881C_COMMAND_INSTR(0x54, 0x89),
> +	ILI9881C_COMMAND_INSTR(0x55, 0xab),
> +	ILI9881C_COMMAND_INSTR(0x56, 0x01),
> +	ILI9881C_COMMAND_INSTR(0x57, 0x23),
> +	ILI9881C_COMMAND_INSTR(0x58, 0x45),
> +	ILI9881C_COMMAND_INSTR(0x59, 0x67),
> +	ILI9881C_COMMAND_INSTR(0x5a, 0x89),
> +	ILI9881C_COMMAND_INSTR(0x5b, 0xab),
> +	ILI9881C_COMMAND_INSTR(0x5c, 0xcd),
> +	ILI9881C_COMMAND_INSTR(0x5d, 0xef),
> +	ILI9881C_COMMAND_INSTR(0x5e, 0x11),
> +	ILI9881C_COMMAND_INSTR(0x5f, 0x02),
> +	ILI9881C_COMMAND_INSTR(0x60, 0x02),
> +	ILI9881C_COMMAND_INSTR(0x61, 0x02),
> +	ILI9881C_COMMAND_INSTR(0x62, 0x02),
> +	ILI9881C_COMMAND_INSTR(0x63, 0x02),
> +	ILI9881C_COMMAND_INSTR(0x64, 0x02),
> +	ILI9881C_COMMAND_INSTR(0x65, 0x02),
> +	ILI9881C_COMMAND_INSTR(0x66, 0x02),
> +	ILI9881C_COMMAND_INSTR(0x67, 0x02),
> +	ILI9881C_COMMAND_INSTR(0x68, 0x02),
> +	ILI9881C_COMMAND_INSTR(0x69, 0x02),
> +	ILI9881C_COMMAND_INSTR(0x6a, 0x0C),
> +	ILI9881C_COMMAND_INSTR(0x6b, 0x02),
> +	ILI9881C_COMMAND_INSTR(0x6c, 0x0F),
> +	ILI9881C_COMMAND_INSTR(0x6d, 0x0E),
> +	ILI9881C_COMMAND_INSTR(0x6e, 0x0D),
> +	ILI9881C_COMMAND_INSTR(0x6f, 0x06),
> +	ILI9881C_COMMAND_INSTR(0x70, 0x07),
> +	ILI9881C_COMMAND_INSTR(0x71, 0x02),
> +	ILI9881C_COMMAND_INSTR(0x72, 0x02),
> +	ILI9881C_COMMAND_INSTR(0x73, 0x02),
> +	ILI9881C_COMMAND_INSTR(0x74, 0x02),
> +	ILI9881C_COMMAND_INSTR(0x75, 0x02),
> +	ILI9881C_COMMAND_INSTR(0x76, 0x02),
> +	ILI9881C_COMMAND_INSTR(0x77, 0x02),
> +	ILI9881C_COMMAND_INSTR(0x78, 0x02),
> +	ILI9881C_COMMAND_INSTR(0x79, 0x02),
> +	ILI9881C_COMMAND_INSTR(0x7a, 0x02),
> +	ILI9881C_COMMAND_INSTR(0x7b, 0x02),
> +	ILI9881C_COMMAND_INSTR(0x7c, 0x02),
> +	ILI9881C_COMMAND_INSTR(0x7d, 0x02),
> +	ILI9881C_COMMAND_INSTR(0x7e, 0x02),
> +	ILI9881C_COMMAND_INSTR(0x7f, 0x02),
> +	ILI9881C_COMMAND_INSTR(0x80, 0x0C),
> +	ILI9881C_COMMAND_INSTR(0x81, 0x02),
> +	ILI9881C_COMMAND_INSTR(0x82, 0x0F),
> +	ILI9881C_COMMAND_INSTR(0x83, 0x0E),
> +	ILI9881C_COMMAND_INSTR(0x84, 0x0D),
> +	ILI9881C_COMMAND_INSTR(0x85, 0x06),
> +	ILI9881C_COMMAND_INSTR(0x86, 0x07),
> +	ILI9881C_COMMAND_INSTR(0x87, 0x02),
> +	ILI9881C_COMMAND_INSTR(0x88, 0x02),
> +	ILI9881C_COMMAND_INSTR(0x89, 0x02),
> +	ILI9881C_COMMAND_INSTR(0x8A, 0x02),
> +	ILI9881C_SWITCH_PAGE_INSTR(4),
> +	ILI9881C_COMMAND_INSTR(0x6C, 0x15),
> +	ILI9881C_COMMAND_INSTR(0x6E, 0x22),
> +	ILI9881C_COMMAND_INSTR(0x6F, 0x33),
> +	ILI9881C_COMMAND_INSTR(0x3A, 0xA4),
> +	ILI9881C_COMMAND_INSTR(0x8D, 0x0D),
> +	ILI9881C_COMMAND_INSTR(0x87, 0xBA),
> +	ILI9881C_COMMAND_INSTR(0x26, 0x76),
> +	ILI9881C_COMMAND_INSTR(0xB2, 0xD1),
> +	ILI9881C_SWITCH_PAGE_INSTR(1),
> +	ILI9881C_COMMAND_INSTR(0x22, 0x0A),
> +	ILI9881C_COMMAND_INSTR(0x53, 0xDC),
> +	ILI9881C_COMMAND_INSTR(0x55, 0xA7),
> +	ILI9881C_COMMAND_INSTR(0x50, 0x78),
> +	ILI9881C_COMMAND_INSTR(0x51, 0x78),
> +	ILI9881C_COMMAND_INSTR(0x31, 0x02),
> +	ILI9881C_COMMAND_INSTR(0x60, 0x14),
> +	ILI9881C_COMMAND_INSTR(0xA0, 0x2A),
> +	ILI9881C_COMMAND_INSTR(0xA1, 0x39),
> +	ILI9881C_COMMAND_INSTR(0xA2, 0x46),
> +	ILI9881C_COMMAND_INSTR(0xA3, 0x0e),
> +	ILI9881C_COMMAND_INSTR(0xA4, 0x12),
> +	ILI9881C_COMMAND_INSTR(0xA5, 0x25),
> +	ILI9881C_COMMAND_INSTR(0xA6, 0x19),
> +	ILI9881C_COMMAND_INSTR(0xA7, 0x1d),
> +	ILI9881C_COMMAND_INSTR(0xA8, 0xa6),
> +	ILI9881C_COMMAND_INSTR(0xA9, 0x1C),
> +	ILI9881C_COMMAND_INSTR(0xAA, 0x29),
> +	ILI9881C_COMMAND_INSTR(0xAB, 0x85),
> +	ILI9881C_COMMAND_INSTR(0xAC, 0x1C),
> +	ILI9881C_COMMAND_INSTR(0xAD, 0x1B),
> +	ILI9881C_COMMAND_INSTR(0xAE, 0x51),
> +	ILI9881C_COMMAND_INSTR(0xAF, 0x22),
> +	ILI9881C_COMMAND_INSTR(0xB0, 0x2d),
> +	ILI9881C_COMMAND_INSTR(0xB1, 0x4f),
> +	ILI9881C_COMMAND_INSTR(0xB2, 0x59),
> +	ILI9881C_COMMAND_INSTR(0xB3, 0x3F),
> +	ILI9881C_COMMAND_INSTR(0xC0, 0x2A),
> +	ILI9881C_COMMAND_INSTR(0xC1, 0x3a),
> +	ILI9881C_COMMAND_INSTR(0xC2, 0x45),
> +	ILI9881C_COMMAND_INSTR(0xC3, 0x0e),
> +	ILI9881C_COMMAND_INSTR(0xC4, 0x11),
> +	ILI9881C_COMMAND_INSTR(0xC5, 0x24),
> +	ILI9881C_COMMAND_INSTR(0xC6, 0x1a),
> +	ILI9881C_COMMAND_INSTR(0xC7, 0x1c),
> +	ILI9881C_COMMAND_INSTR(0xC8, 0xaa),
> +	ILI9881C_COMMAND_INSTR(0xC9, 0x1C),
> +	ILI9881C_COMMAND_INSTR(0xCA, 0x29),
> +	ILI9881C_COMMAND_INSTR(0xCB, 0x96),
> +	ILI9881C_COMMAND_INSTR(0xCC, 0x1C),
> +	ILI9881C_COMMAND_INSTR(0xCD, 0x1B),
> +	ILI9881C_COMMAND_INSTR(0xCE, 0x51),
> +	ILI9881C_COMMAND_INSTR(0xCF, 0x22),
> +	ILI9881C_COMMAND_INSTR(0xD0, 0x2b),
> +	ILI9881C_COMMAND_INSTR(0xD1, 0x4b),
> +	ILI9881C_COMMAND_INSTR(0xD2, 0x59),
> +	ILI9881C_COMMAND_INSTR(0xD3, 0x3F),
> +};
> +
> +static inline struct ili9881c *panel_to_ili9881c(struct drm_panel *panel)
> +{
> +	return container_of(panel, struct ili9881c, panel);
> +}
> +
> +static int ili9881c_switch_page(struct ili9881c *ctx, u8 page)
> +{
> +	u8 buf[4] = { 0xff, 0x98, 0x81, page };
> +	int ret;
> +
> +	ret = mipi_dsi_dcs_write_buffer(ctx->dsi, buf, sizeof(buf));
> +	if (ret < 0)
> +		return ret;
> +
> +	return 0;
> +}
> +
> +static int ili9881c_send_cmd_data(struct ili9881c *ctx, u8 cmd, u8 data)
> +{
> +	u8 buf[2] = { cmd, data };
> +	int ret;
> +
> +	ret = mipi_dsi_dcs_write_buffer(ctx->dsi, buf, sizeof(buf));
> +	if (ret < 0)
> +		return ret;
> +
> +	return 0;
> +}

According to this you're sending DCS commands, but none of the above
look like valid DCS commands. Do you know what's going on here? Also,
can you include a reference to a datasheet where these instructions
come from?

Thierry

> +
> +static int ili9881c_prepare(struct drm_panel *panel)
> +{
> +	struct ili9881c *ctx = panel_to_ili9881c(panel);
> +	unsigned int i;
> +	int ret;
> +
> +	/* Power the panel */
> +	gpiod_set_value(ctx->power, 1);
> +	msleep(5);
> +
> +	/* And reset it */
> +	gpiod_set_value(ctx->reset, 1);
> +	msleep(20);
> +
> +	gpiod_set_value(ctx->reset, 0);
> +	msleep(20);
> +
> +	for (i = 0; i < ARRAY_SIZE(ili9881c_init); i++) {
> +		struct ili9881c_instr *instr = &ili9881c_init[i];
> +
> +		if (instr->op == ILI9881C_SWITCH_PAGE)
> +			ret = ili9881c_switch_page(ctx, instr->arg.page);
> +		else if (instr->op == ILI9881C_COMMAND)
> +			ret = ili9881c_send_cmd_data(ctx, instr->arg.cmd.cmd,
> +						      instr->arg.cmd.data);
> +
> +		if (ret)
> +			return ret;
> +	}
> +
> +	ret = ili9881c_switch_page(ctx, 0);
> +	if (ret)
> +		return ret;
> +
> +	ret = mipi_dsi_dcs_set_tear_on(ctx->dsi, MIPI_DSI_DCS_TEAR_MODE_VBLANK);
> +	if (ret)
> +		return ret;
> +
> +	mipi_dsi_dcs_exit_sleep_mode(ctx->dsi);
> +	if (ret)
> +		return ret;
> +
> +	return 0;
> +}
> +
> +static int ili9881c_enable(struct drm_panel *panel)
> +{
> +	struct ili9881c *ctx = panel_to_ili9881c(panel);
> +
> +	msleep(120);
> +
> +	mipi_dsi_dcs_set_display_on(ctx->dsi);
> +	backlight_enable(ctx->backlight);
> +
> +	return 0;
> +}
> +
> +static int ili9881c_disable(struct drm_panel *panel)
> +{
> +	struct ili9881c *ctx = panel_to_ili9881c(panel);
> +
> +	backlight_disable(ctx->backlight);
> +	return mipi_dsi_dcs_set_display_off(ctx->dsi);
> +}
> +
> +static int ili9881c_unprepare(struct drm_panel *panel)
> +{
> +	struct ili9881c *ctx = panel_to_ili9881c(panel);
> +
> +	mipi_dsi_dcs_enter_sleep_mode(ctx->dsi);
> +	gpiod_set_value(ctx->power, 0);
> +	gpiod_set_value(ctx->reset, 1);
> +
> +	return 0;
> +}
> +
> +static const struct drm_display_mode default_mode = {
> +	.clock		= 62000,
> +	.vrefresh	= 60,
> +
> +	.hdisplay	= 720,
> +	.hsync_start	= 720 + 10,
> +	.hsync_end	= 720 + 10 + 20,
> +	.htotal		= 720 + 10 + 20 + 30,
> +
> +	.vdisplay	= 1280,
> +	.vsync_start	= 1280 + 10,
> +	.vsync_end	= 1280 + 10 + 10,
> +	.vtotal		= 1280 + 10 + 10 + 20,
> +};
> +
> +static int ili9881c_get_modes(struct drm_panel *panel)
> +{
> +	struct drm_connector *connector = panel->connector;
> +	struct ili9881c *ctx = panel_to_ili9881c(panel);
> +	struct drm_display_mode *mode;
> +
> +	mode = drm_mode_duplicate(panel->drm, &default_mode);
> +	if (!mode) {
> +		dev_err(&ctx->dsi->dev, "failed to add mode %ux%ux@%u\n",
> +			default_mode.hdisplay, default_mode.vdisplay,
> +			default_mode.vrefresh);
> +		return -ENOMEM;
> +	}
> +
> +	drm_mode_set_name(mode);
> +
> +	mode->type = DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED;
> +	drm_mode_probed_add(connector, mode);
> +
> +	panel->connector->display_info.width_mm = 62;
> +	panel->connector->display_info.height_mm = 110;
> +
> +	return 1;
> +}
> +
> +static const struct drm_panel_funcs ili9881c_funcs = {
> +	.prepare	= ili9881c_prepare,
> +	.unprepare	= ili9881c_unprepare,
> +	.enable		= ili9881c_enable,
> +	.disable	= ili9881c_disable,
> +	.get_modes	= ili9881c_get_modes,
> +};
> +
> +static int ili9881c_dsi_probe(struct mipi_dsi_device *dsi)
> +{
> +	struct device_node *np;
> +	struct ili9881c *ctx;
> +	int ret;
> +
> +	ctx = devm_kzalloc(&dsi->dev, sizeof(*ctx), GFP_KERNEL);
> +	if (!ctx)
> +		return -ENOMEM;
> +	mipi_dsi_set_drvdata(dsi, ctx);
> +	ctx->dsi = dsi;
> +
> +	drm_panel_init(&ctx->panel);
> +	ctx->panel.dev = &dsi->dev;
> +	ctx->panel.funcs = &ili9881c_funcs;
> +
> +	ctx->power = devm_gpiod_get(&dsi->dev, "power", GPIOD_OUT_LOW);
> +	if (IS_ERR(ctx->power)) {
> +		dev_err(&dsi->dev, "Couldn't get our power GPIO\n");
> +		return PTR_ERR(ctx->power);
> +	}
> +
> +	ctx->reset = devm_gpiod_get(&dsi->dev, "reset", GPIOD_OUT_LOW);
> +	if (IS_ERR(ctx->reset)) {
> +		dev_err(&dsi->dev, "Couldn't get our reset GPIO\n");
> +		return PTR_ERR(ctx->reset);
> +	}
> +
> +	np = of_parse_phandle(dsi->dev.of_node, "backlight", 0);
> +	if (np) {
> +		ctx->backlight = of_find_backlight_by_node(np);
> +		of_node_put(np);
> +
> +		if (!ctx->backlight)
> +			return -EPROBE_DEFER;
> +	}
> +
> +	ret = drm_panel_add(&ctx->panel);
> +	if (ret < 0)
> +		return ret;
> +
> +	dsi->mode_flags = MIPI_DSI_MODE_VIDEO_SYNC_PULSE;
> +	dsi->format = MIPI_DSI_FMT_RGB888;
> +	dsi->lanes = 4;
> +
> +	return mipi_dsi_attach(dsi);
> +}
> +
> +static int ili9881c_dsi_remove(struct mipi_dsi_device *dsi)
> +{
> +	struct ili9881c *ctx = mipi_dsi_get_drvdata(dsi);
> +
> +	mipi_dsi_detach(dsi);
> +	drm_panel_remove(&ctx->panel);
> +
> +	if (ctx->backlight)
> +		put_device(&ctx->backlight->dev);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id ili9881c_of_match[] = {
> +	{ .compatible = "bananapi,lhr050h41" },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, ili9881c_of_match);
> +
> +static struct mipi_dsi_driver ili9881c_dsi_driver = {
> +	.probe		= ili9881c_dsi_probe,
> +	.remove		= ili9881c_dsi_remove,
> +	.driver = {
> +		.name		= "ili9881c-dsi",
> +		.of_match_table	= ili9881c_of_match,
> +	},
> +};
> +module_mipi_dsi_driver(ili9881c_dsi_driver);
> +
> +MODULE_AUTHOR("Maxime Ripard <maxime.ripard@free-electrons.com>");
> +MODULE_DESCRIPTION("Ilitek ILI9881C Controller Driver");
> +MODULE_LICENSE("GPL v2");
> -- 
> git-series 0.9.1
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180426/bccfd419/attachment-0001.sig>

  reply	other threads:[~2018-04-26 15:07 UTC|newest]

Thread overview: 43+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-04-04  9:57 [PATCH v4 0/8] drm/sun4i: Allwinner MIPI-DSI support Maxime Ripard
2018-04-04  9:57 ` Maxime Ripard
2018-04-04  9:57 ` Maxime Ripard
2018-04-04  9:57 ` [PATCH v4 1/8] drm/sun4i: tcon: Add TRI finish interrupt for vblank Maxime Ripard
2018-04-04  9:57   ` Maxime Ripard
2018-04-04  9:57   ` Maxime Ripard
2018-04-04  9:57 ` [PATCH v4 2/8] dt-bindings: display: Add Allwinner MIPI-DSI bindings Maxime Ripard
2018-04-04  9:57   ` Maxime Ripard
2018-04-04  9:57   ` Maxime Ripard
2018-04-04  9:57 ` [PATCH v4 3/8] drm/sun4i: Add Allwinner A31 MIPI-DSI controller support Maxime Ripard
2018-04-04  9:57   ` Maxime Ripard
2018-04-04  9:57   ` Maxime Ripard
2018-04-04  9:57 ` [PATCH v4 4/8] drm/sun4i: Tie the DSI controller in the TCON Maxime Ripard
2018-04-04  9:57   ` Maxime Ripard
2018-04-04  9:57   ` Maxime Ripard
2018-04-04  9:57 ` [PATCH v4 5/8] dt-bindings: panel: Add the Ilitek ILI9881c panel documentation Maxime Ripard
2018-04-04  9:57   ` Maxime Ripard
2018-04-04  9:57   ` Maxime Ripard
2018-04-04  9:57 ` [PATCH v4 6/8] drm/panel: Add Ilitek ILI9881c panel driver Maxime Ripard
2018-04-04  9:57   ` Maxime Ripard
2018-04-04  9:57   ` Maxime Ripard
2018-04-26 15:07   ` Thierry Reding [this message]
2018-04-26 15:07     ` Thierry Reding
2018-04-26 15:07     ` Thierry Reding
2018-05-03 13:59     ` Maxime Ripard
2018-05-03 13:59       ` Maxime Ripard
2018-05-03 13:59       ` Maxime Ripard
2018-04-04  9:57 ` [PATCH v4 7/8] ARM: dts: sun8i: a33: Add the DSI-related nodes Maxime Ripard
2018-04-04  9:57   ` Maxime Ripard
2018-04-04  9:57   ` Maxime Ripard
2018-04-04  9:57 ` [PATCH v4 8/8] [DO NOT MERGE] arm: dts: sun8i: bpi-m2m: Add DSI display Maxime Ripard
2018-04-04  9:57   ` Maxime Ripard
2018-04-04  9:57   ` Maxime Ripard
2018-04-11 12:43 ` [PATCH v4 0/8] drm/sun4i: Allwinner MIPI-DSI support Maxime Ripard
2018-04-11 12:43   ` Maxime Ripard
2018-04-11 12:43   ` Maxime Ripard
2018-04-13 12:00   ` Jagan Teki
2018-04-13 12:00     ` Jagan Teki
2018-04-13 12:09     ` Maxime Ripard
2018-04-13 12:09       ` Maxime Ripard
2018-04-13 12:09       ` Maxime Ripard
2018-04-13 12:18       ` Jagan Teki
2018-04-13 12:18         ` Jagan Teki

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180426150712.GF31888@ulmo \
    --to=thierry.reding@gmail.com \
    --cc=daniel.vetter@intel.com \
    --cc=devicetree@vger.kernel.org \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=frowand.list@gmail.com \
    --cc=gustavo@padovan.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=maarten.lankhorst@linux.intel.com \
    --cc=mark.rutland@arm.com \
    --cc=maxime.ripard@bootlin.com \
    --cc=robh+dt@kernel.org \
    --cc=seanpaul@chromium.org \
    --cc=thomas.petazzoni@bootlin.com \
    --cc=wens@csie.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.