linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/4] Add support for the s6e63j0x03 panel on Rinato board
       [not found] <CGME20170615100341epcas5p18affc41c65c13d0daac005290cf1d249@epcas5p1.samsung.com>
@ 2017-06-15 10:03 ` Hoegeun Kwon
       [not found]   ` <CGME20170615100341epcas1p46fc952e3aa7341b96e18327895ef619e@epcas1p4.samsung.com>
                     ` (3 more replies)
  0 siblings, 4 replies; 8+ messages in thread
From: Hoegeun Kwon @ 2017-06-15 10:03 UTC (permalink / raw)
  To: thierry.reding, airlied, robh+dt, mark.rutland, catalin.marinas,
	will.deacon, kgene, krzk
  Cc: dri-devel, devicetree, linux-arm-kernel, linux-samsung-soc,
	linux-kernel, javier, a.hajda, Hoegeun Kwon

Hi all,

The purpose of this patch is add support for s6e63j0x03 AMOLED panel
on the rinato board(Samsung Galaxy Gear 2).

Changes for V2:
- Added the interface info in binding document.
- Added Reviewed-by: Andrzej Hajda <a.hajda@samsung.com> (1/3 patch).
- Added Acked-by: Rob Herring <robh@kernel.org> (1/3 patch).
- Fixed the tristate of config from video mode to command mode.
- Fixed the reset gpios from active high to low from rinato dts.
- Fixed a lot of things in panel driver, reflect Andrzej's review.

Best regards,
Hoegeun

Hoegeun Kwon (4):
  dt-bindings: Add support for samsung s6e63j0x03 panel binding
  drm/panel: Add support for s6e63j0x03 panel driver
  ARM: dts: exynos: Fix the active of reset gpios from rinato dts
  ARM: dts: exynos: Remove the display-timing and delay from rinato dts

 .../bindings/display/panel/samsung,s6e63j0x03.txt  |  24 ++
 arch/arm/boot/dts/exynos3250-rinato.dts            |  24 +-
 drivers/gpu/drm/panel/Kconfig                      |   7 +
 drivers/gpu/drm/panel/Makefile                     |   1 +
 drivers/gpu/drm/panel/panel-samsung-s6e63j0x03.c   | 476 +++++++++++++++++++++
 5 files changed, 509 insertions(+), 23 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/display/panel/samsung,s6e63j0x03.txt
 create mode 100644 drivers/gpu/drm/panel/panel-samsung-s6e63j0x03.c

-- 
1.9.1

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v2 1/4] dt-bindings: Add support for samsung s6e63j0x03 panel binding
       [not found]   ` <CGME20170615100341epcas1p46fc952e3aa7341b96e18327895ef619e@epcas1p4.samsung.com>
@ 2017-06-15 10:03     ` Hoegeun Kwon
  0 siblings, 0 replies; 8+ messages in thread
From: Hoegeun Kwon @ 2017-06-15 10:03 UTC (permalink / raw)
  To: thierry.reding, airlied, robh+dt, mark.rutland, catalin.marinas,
	will.deacon, kgene, krzk
  Cc: dri-devel, devicetree, linux-arm-kernel, linux-samsung-soc,
	linux-kernel, javier, a.hajda, Hoegeun Kwon

The Samsung s6e63j0x03 is a 1.63" 320x320 AMOLED panel connected using
MIPI-DSI interfaces.

Signed-off-by: Hoegeun Kwon <hoegeun.kwon@samsung.com>
Reviewed-by: Andrzej Hajda <a.hajda@samsung.com>
Acked-by: Rob Herring <robh@kernel.org>
---
 .../bindings/display/panel/samsung,s6e63j0x03.txt  | 24 ++++++++++++++++++++++
 1 file changed, 24 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/panel/samsung,s6e63j0x03.txt

diff --git a/Documentation/devicetree/bindings/display/panel/samsung,s6e63j0x03.txt b/Documentation/devicetree/bindings/display/panel/samsung,s6e63j0x03.txt
new file mode 100644
index 0000000..3f1a839
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/panel/samsung,s6e63j0x03.txt
@@ -0,0 +1,24 @@
+Samsung S6E63J0X03 1.63" 320x320 AMOLED panel (interface: MIPI-DSI command mode)
+
+Required properties:
+  - compatible: "samsung,s6e63j0x03"
+  - reg: the virtual channel number of a DSI peripheral
+  - vdd3-supply: I/O voltage supply
+  - vci-supply: voltage supply for analog circuits
+  - reset-gpios: a GPIO spec for the reset pin (active low)
+  - te-gpios: a GPIO spec for the tearing effect synchronization signal
+    gpio pin (active high)
+
+Example:
+&dsi {
+	...
+
+	panel@0 {
+		compatible = "samsung,s6e63j0x03";
+		reg = <0>;
+		vdd3-supply = <&ldo16_reg>;
+		vci-supply = <&ldo20_reg>;
+		reset-gpios = <&gpe0 1 GPIO_ACTIVE_LOW>;
+		te-gpios = <&gpx0 6 GPIO_ACTIVE_HIGH>;
+	};
+};
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v2 2/4] drm/panel: Add support for s6e63j0x03 panel driver
       [not found]   ` <CGME20170615100342epcas1p4b15ffce9ea2018e2bc90bb04061aded5@epcas1p4.samsung.com>
@ 2017-06-15 10:03     ` Hoegeun Kwon
  2017-06-19  9:14       ` Andrzej Hajda
  0 siblings, 1 reply; 8+ messages in thread
From: Hoegeun Kwon @ 2017-06-15 10:03 UTC (permalink / raw)
  To: thierry.reding, airlied, robh+dt, mark.rutland, catalin.marinas,
	will.deacon, kgene, krzk
  Cc: dri-devel, devicetree, linux-arm-kernel, linux-samsung-soc,
	linux-kernel, javier, a.hajda, Hoegeun Kwon, Inki Dae,
	Hyungwon Hwang

This patch adds MIPI-DSI based S6E63J0X03 AMOLED LCD panel driver
which uses mipi_dsi bus to communicate with panel. The panel has
320×320 resolution in 1.63" physical panel. This panel is used in
Samsung Galaxy Gear 2.

Signed-off-by: Inki Dae <inki.dae@samsung.com>
Signed-off-by: Hyungwon Hwang <human.hwang@samsung.com>
Signed-off-by: Hoegeun Kwon <hoegeun.kwon@samsung.com>
---
 drivers/gpu/drm/panel/Kconfig                    |   7 +
 drivers/gpu/drm/panel/Makefile                   |   1 +
 drivers/gpu/drm/panel/panel-samsung-s6e63j0x03.c | 476 +++++++++++++++++++++++
 3 files changed, 484 insertions(+)
 create mode 100644 drivers/gpu/drm/panel/panel-samsung-s6e63j0x03.c

diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
index 3e29a99..3f4afde 100644
--- a/drivers/gpu/drm/panel/Kconfig
+++ b/drivers/gpu/drm/panel/Kconfig
@@ -68,6 +68,13 @@ config DRM_PANEL_SAMSUNG_S6E3HA2
 	depends on DRM_MIPI_DSI
 	select VIDEOMODE_HELPERS
 
+config DRM_PANEL_SAMSUNG_S6E63J0X03
+	tristate "Samsung S6E63J0X03 DSI command mode panel"
+	depends on OF
+	depends on DRM_MIPI_DSI
+	depends on BACKLIGHT_CLASS_DEVICE
+	select VIDEOMODE_HELPERS
+
 config DRM_PANEL_SAMSUNG_S6E8AA0
 	tristate "Samsung S6E8AA0 DSI video mode panel"
 	depends on OF
diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
index 292b3c7..f028269 100644
--- a/drivers/gpu/drm/panel/Makefile
+++ b/drivers/gpu/drm/panel/Makefile
@@ -5,6 +5,7 @@ obj-$(CONFIG_DRM_PANEL_LG_LG4573) += panel-lg-lg4573.o
 obj-$(CONFIG_DRM_PANEL_PANASONIC_VVX10F034N00) += panel-panasonic-vvx10f034n00.o
 obj-$(CONFIG_DRM_PANEL_SAMSUNG_LD9040) += panel-samsung-ld9040.o
 obj-$(CONFIG_DRM_PANEL_SAMSUNG_S6E3HA2) += panel-samsung-s6e3ha2.o
+obj-$(CONFIG_DRM_PANEL_SAMSUNG_S6E63J0X03) += panel-samsung-s6e63j0x03.o
 obj-$(CONFIG_DRM_PANEL_SAMSUNG_S6E8AA0) += panel-samsung-s6e8aa0.o
 obj-$(CONFIG_DRM_PANEL_SHARP_LQ101R1SX01) += panel-sharp-lq101r1sx01.o
 obj-$(CONFIG_DRM_PANEL_SHARP_LS043T1LE01) += panel-sharp-ls043t1le01.o
diff --git a/drivers/gpu/drm/panel/panel-samsung-s6e63j0x03.c b/drivers/gpu/drm/panel/panel-samsung-s6e63j0x03.c
new file mode 100644
index 0000000..dd038bc
--- /dev/null
+++ b/drivers/gpu/drm/panel/panel-samsung-s6e63j0x03.c
@@ -0,0 +1,476 @@
+/*
+ * MIPI-DSI based S6E63J0X03 AMOLED lcd 1.63 inch panel driver.
+ *
+ * Copyright (c) 2014-2017 Samsung Electronics Co., Ltd
+ *
+ * Inki Dae <inki.dae@samsung.com>
+ * Hoegeun Kwon <hoegeun.kwon@samsung.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <drm/drmP.h>
+#include <drm/drm_mipi_dsi.h>
+#include <drm/drm_panel.h>
+#include <linux/backlight.h>
+#include <linux/gpio/consumer.h>
+#include <linux/regulator/consumer.h>
+#include <video/mipi_display.h>
+
+#define MCS_LEVEL2_KEY		0xf0
+#define MCS_MTP_KEY		0xf1
+#define MCS_MTP_SET3		0xd4
+
+#define MIN_BRIGHTNESS		0
+#define MAX_BRIGHTNESS		100
+#define DEFAULT_BRIGHTNESS	80
+
+#define NUM_GAMMA_STEPS		9
+#define GAMMA_CMD_CNT		28
+
+struct s6e63j0x03 {
+	struct device *dev;
+	struct drm_panel panel;
+	struct backlight_device *bl_dev;
+
+	struct regulator_bulk_data supplies[2];
+	struct gpio_desc *reset_gpio;
+};
+
+static const unsigned char gamma_tbl[NUM_GAMMA_STEPS][GAMMA_CMD_CNT] = {
+	{	/* Gamma 10 */
+		MCS_MTP_SET3,
+		0x00, 0x00, 0x00, 0x7f, 0x7f, 0x7f, 0x52, 0x6b, 0x6f, 0x26,
+		0x28, 0x2d, 0x28, 0x26, 0x27, 0x33, 0x34, 0x32, 0x36, 0x36,
+		0x35, 0x00, 0xab, 0x00, 0xae, 0x00, 0xbf
+	},
+	{	/* gamma 30 */
+		MCS_MTP_SET3,
+		0x00, 0x00, 0x00, 0x70, 0x7f, 0x7f, 0x4e, 0x64, 0x69, 0x26,
+		0x27, 0x2a, 0x28, 0x29, 0x27, 0x31, 0x32, 0x31, 0x35, 0x34,
+		0x35, 0x00, 0xc4, 0x00, 0xca, 0x00, 0xdc
+	},
+	{	/* gamma 60 */
+		MCS_MTP_SET3,
+		0x00, 0x00, 0x00, 0x65, 0x7b, 0x7d, 0x5f, 0x67, 0x68, 0x2a,
+		0x28, 0x29, 0x28, 0x2a, 0x27, 0x31, 0x2f, 0x30, 0x34, 0x33,
+		0x34, 0x00, 0xd9, 0x00, 0xe4, 0x00, 0xf5
+	},
+	{	/* gamma 90 */
+		MCS_MTP_SET3,
+		0x00, 0x00, 0x00, 0x4d, 0x6f, 0x71, 0x67, 0x6a, 0x6c, 0x29,
+		0x28, 0x28, 0x28, 0x29, 0x27, 0x30, 0x2e, 0x30, 0x32, 0x31,
+		0x31, 0x00, 0xea, 0x00, 0xf6, 0x01, 0x09
+	},
+	{	/* gamma 120 */
+		MCS_MTP_SET3,
+		0x00, 0x00, 0x00, 0x3d, 0x66, 0x68, 0x69, 0x69, 0x69, 0x28,
+		0x28, 0x27, 0x28, 0x28, 0x27, 0x30, 0x2e, 0x2f, 0x31, 0x31,
+		0x30, 0x00, 0xf9, 0x01, 0x05, 0x01, 0x1b
+	},
+	{	/* gamma 150 */
+		MCS_MTP_SET3,
+		0x00, 0x00, 0x00, 0x31, 0x51, 0x53, 0x66, 0x66, 0x67, 0x28,
+		0x29, 0x27, 0x28, 0x27, 0x27, 0x2e, 0x2d, 0x2e, 0x31, 0x31,
+		0x30, 0x01, 0x04, 0x01, 0x11, 0x01, 0x29
+	},
+	{	/* gamma 200 */
+		MCS_MTP_SET3,
+		0x00, 0x00, 0x00, 0x2f, 0x4f, 0x51, 0x67, 0x65, 0x65, 0x29,
+		0x2a, 0x28, 0x27, 0x25, 0x26, 0x2d, 0x2c, 0x2c, 0x30, 0x30,
+		0x30, 0x01, 0x14, 0x01, 0x23, 0x01, 0x3b
+	},
+	{	/* gamma 240 */
+		MCS_MTP_SET3,
+		0x00, 0x00, 0x00, 0x2c, 0x4d, 0x50, 0x65, 0x63, 0x64, 0x2a,
+		0x2c, 0x29, 0x26, 0x24, 0x25, 0x2c, 0x2b, 0x2b, 0x30, 0x30,
+		0x30, 0x01, 0x1e, 0x01, 0x2f, 0x01, 0x47
+	},
+	{	/* gamma 300 */
+		MCS_MTP_SET3,
+		0x00, 0x00, 0x00, 0x38, 0x61, 0x64, 0x65, 0x63, 0x64, 0x28,
+		0x2a, 0x27, 0x26, 0x23, 0x25, 0x2b, 0x2b, 0x2a, 0x30, 0x2f,
+		0x30, 0x01, 0x2d, 0x01, 0x3f, 0x01, 0x57
+	}
+};
+
+static inline struct s6e63j0x03 *panel_to_s6e63j0x03(struct drm_panel *panel)
+{
+	return container_of(panel, struct s6e63j0x03, panel);
+}
+
+static inline ssize_t s6e63j0x03_dcs_write_seq(struct s6e63j0x03 *ctx,
+					const void *seq, size_t len)
+{
+	struct mipi_dsi_device *dsi = to_mipi_dsi_device(ctx->dev);
+
+	return mipi_dsi_dcs_write_buffer(dsi, seq, len);
+}
+
+#define s6e63j0x03_dcs_write_seq_static(ctx, seq...) do {	\
+	static const u8 d[] = { seq };				\
+	int ret;						\
+	ret = s6e63j0x03_dcs_write_seq(ctx, d, ARRAY_SIZE(d));	\
+	if (ret < 0)						\
+		return ret;					\
+} while (0)
+
+#define s6e63j0x03_call_write_func(ret, func) do {	\
+	ret = (func);					\
+	if (ret < 0)					\
+		return ret;				\
+} while (0)
+
+static inline int s6e63j0x03_enable_lv2_command(struct s6e63j0x03 *ctx)
+{
+	s6e63j0x03_dcs_write_seq_static(ctx, MCS_LEVEL2_KEY, 0x5a, 0x5a);
+	return 0;
+}
+
+static inline int s6e63j0x03_apply_mtp_key(struct s6e63j0x03 *ctx, bool on)
+{
+	if (on)
+		s6e63j0x03_dcs_write_seq_static(ctx, MCS_MTP_KEY, 0x5a, 0x5a);
+	else
+		s6e63j0x03_dcs_write_seq_static(ctx, MCS_MTP_KEY, 0xa5, 0xa5);
+
+	return 0;
+}
+
+static int s6e63j0x03_power_on(struct s6e63j0x03 *ctx)
+{
+	int ret;
+
+	ret = regulator_bulk_enable(ARRAY_SIZE(ctx->supplies), ctx->supplies);
+	if (ret < 0)
+		return ret;
+
+	msleep(30);
+
+	gpiod_set_value(ctx->reset_gpio, 1);
+	usleep_range(1000, 2000);
+	gpiod_set_value(ctx->reset_gpio, 0);
+	usleep_range(5000, 6000);
+
+	return 0;
+}
+
+static int s6e63j0x03_power_off(struct s6e63j0x03 *ctx)
+{
+	return regulator_bulk_disable(ARRAY_SIZE(ctx->supplies), ctx->supplies);
+}
+
+static unsigned int s6e63j0x03_get_brightness_index(unsigned int brightness)
+{
+	unsigned int index;
+
+	index = brightness / (MAX_BRIGHTNESS / NUM_GAMMA_STEPS);
+
+	if (index >= NUM_GAMMA_STEPS)
+		index = NUM_GAMMA_STEPS - 1;
+
+	return index;
+}
+
+static int s6e63j0x03_update_gamma(struct s6e63j0x03 *ctx,
+					unsigned int brightness)
+{
+	struct backlight_device *bl_dev = ctx->bl_dev;
+	unsigned int index = s6e63j0x03_get_brightness_index(brightness);
+	int ret;
+
+	s6e63j0x03_call_write_func(ret, s6e63j0x03_apply_mtp_key(ctx, true));
+	s6e63j0x03_call_write_func(ret,
+		s6e63j0x03_dcs_write_seq(ctx, gamma_tbl[index], GAMMA_CMD_CNT));
+	s6e63j0x03_call_write_func(ret, s6e63j0x03_apply_mtp_key(ctx, false));
+
+	bl_dev->props.brightness = brightness;
+
+	return 0;
+}
+
+static int s6e63j0x03_set_brightness(struct backlight_device *bl_dev)
+{
+	struct s6e63j0x03 *ctx = (struct s6e63j0x03 *)bl_get_data(bl_dev);
+	unsigned int brightness = bl_dev->props.brightness;
+	int ret;
+
+	s6e63j0x03_call_write_func(ret,
+		s6e63j0x03_update_gamma(ctx, brightness));
+
+	return 0;
+}
+
+static const struct backlight_ops s6e63j0x03_bl_ops = {
+	.update_status = s6e63j0x03_set_brightness,
+};
+
+static int s6e63j0x03_disable(struct drm_panel *panel)
+{
+	struct s6e63j0x03 *ctx = panel_to_s6e63j0x03(panel);
+	struct mipi_dsi_device *dsi = to_mipi_dsi_device(ctx->dev);
+	int ret;
+
+	s6e63j0x03_call_write_func(ret, mipi_dsi_dcs_set_display_off(dsi));
+	ctx->bl_dev->props.power = FB_BLANK_NORMAL;
+
+	s6e63j0x03_call_write_func(ret, mipi_dsi_dcs_enter_sleep_mode(dsi));
+	msleep(120);
+
+	return 0;
+}
+
+static int s6e63j0x03_unprepare(struct drm_panel *panel)
+{
+	struct s6e63j0x03 *ctx = panel_to_s6e63j0x03(panel);
+	int ret;
+
+	ret = s6e63j0x03_power_off(ctx);
+	if (ret < 0)
+		return ret;
+
+	ctx->bl_dev->props.power = FB_BLANK_POWERDOWN;
+
+	return 0;
+}
+
+static int s6e63j0x03_panel_init(struct s6e63j0x03 *ctx)
+{
+	struct mipi_dsi_device *dsi = to_mipi_dsi_device(ctx->dev);
+	int ret;
+
+	s6e63j0x03_call_write_func(ret, s6e63j0x03_enable_lv2_command(ctx));
+	s6e63j0x03_call_write_func(ret, s6e63j0x03_apply_mtp_key(ctx, true));
+
+	/* set porch adjustment */
+	s6e63j0x03_dcs_write_seq_static(ctx, 0xf2, 0x1c, 0x28);
+	/* set frame freq */
+	s6e63j0x03_dcs_write_seq_static(ctx, 0xb5, 0x00, 0x02, 0x00);
+	/* set caset, paset */
+	s6e63j0x03_call_write_func(ret,
+		mipi_dsi_dcs_set_column_address(dsi, 0x0014, 0x0153));
+	s6e63j0x03_call_write_func(ret,
+		mipi_dsi_dcs_set_page_address(dsi, 0x0000, 0x013f));
+	/* set ltps timming 0, 1 */
+	s6e63j0x03_dcs_write_seq_static(ctx, 0xf8, 0x08, 0x08, 0x08, 0x17, 0x00,
+							0x2a, 0x02, 0x26, 0x00,
+							0x00, 0x02, 0x00, 0x00);
+	s6e63j0x03_dcs_write_seq_static(ctx, 0xf7, 0x02);
+	/* set param pos te_edge */
+	s6e63j0x03_dcs_write_seq_static(ctx, 0xb0, 0x01);
+	/* set te rising edge */
+	s6e63j0x03_dcs_write_seq_static(ctx, 0xe2, 0x0f);
+	/* set param pos default */
+	s6e63j0x03_dcs_write_seq_static(ctx, 0xb0, 0x00);
+
+	s6e63j0x03_call_write_func(ret, mipi_dsi_dcs_exit_sleep_mode(dsi));
+
+	s6e63j0x03_call_write_func(ret, s6e63j0x03_apply_mtp_key(ctx, false));
+
+	return 0;
+}
+
+static int s6e63j0x03_prepare(struct drm_panel *panel)
+{
+	struct s6e63j0x03 *ctx = panel_to_s6e63j0x03(panel);
+	int ret;
+
+	ret = s6e63j0x03_power_on(ctx);
+	if (ret < 0)
+		return ret;
+
+	ret = s6e63j0x03_panel_init(ctx);
+	if (ret < 0)
+		goto err;
+
+	ctx->bl_dev->props.power = FB_BLANK_NORMAL;
+
+	return 0;
+
+err:
+	s6e63j0x03_power_off(ctx);
+	return ret;
+}
+
+static int s6e63j0x03_enable(struct drm_panel *panel)
+{
+	struct s6e63j0x03 *ctx = panel_to_s6e63j0x03(panel);
+	struct mipi_dsi_device *dsi = to_mipi_dsi_device(ctx->dev);
+	int ret;
+
+	msleep(120);
+
+	s6e63j0x03_call_write_func(ret, s6e63j0x03_apply_mtp_key(ctx, true));
+
+	/* set elvss_cond */
+	s6e63j0x03_dcs_write_seq_static(ctx, 0xb1, 0x00, 0x09);
+	/* set pos */
+	s6e63j0x03_dcs_write_seq_static(ctx, MIPI_DCS_SET_ADDRESS_MODE, 0x40);
+	/* set default white brightness */
+	s6e63j0x03_call_write_func(ret,
+		mipi_dsi_dcs_set_display_brightness(dsi, 0x00ff));
+	/* set whilte ctrl */
+	s6e63j0x03_dcs_write_seq_static(ctx,
+					MIPI_DCS_WRITE_CONTROL_DISPLAY, 0x20);
+	/* set acl off */
+	s6e63j0x03_dcs_write_seq_static(ctx, MIPI_DCS_WRITE_POWER_SAVE, 0x00);
+
+	s6e63j0x03_call_write_func(ret,
+		mipi_dsi_dcs_set_tear_on(dsi, MIPI_DSI_DCS_TEAR_MODE_VBLANK));
+
+	s6e63j0x03_call_write_func(ret, s6e63j0x03_apply_mtp_key(ctx, false));
+
+	s6e63j0x03_call_write_func(ret, mipi_dsi_dcs_set_display_on(dsi));
+	ctx->bl_dev->props.power = FB_BLANK_UNBLANK;
+
+	return 0;
+}
+
+static const struct drm_display_mode default_mode = {
+	.clock = 4649,
+	.hdisplay = 320,
+	.hsync_start = 320 + 1,
+	.hsync_end = 320 + 1 + 1,
+	.htotal = 320 + 1 + 1 + 1,
+	.vdisplay = 320,
+	.vsync_start = 320 + 150,
+	.vsync_end = 320 + 150 + 1,
+	.vtotal = 320 + 150 + 1 + 2,
+	.vrefresh = 30,
+	.flags = 0,
+};
+
+static int s6e63j0x03_get_modes(struct drm_panel *panel)
+{
+	struct drm_connector *connector = panel->connector;
+	struct drm_display_mode *mode;
+
+	mode = drm_mode_duplicate(panel->drm, &default_mode);
+	if (!mode) {
+		DRM_ERROR("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);
+
+	connector->display_info.width_mm = 29;
+	connector->display_info.height_mm = 29;
+
+	return 1;
+}
+
+static const struct drm_panel_funcs s6e63j0x03_funcs = {
+	.disable = s6e63j0x03_disable,
+	.unprepare = s6e63j0x03_unprepare,
+	.prepare = s6e63j0x03_prepare,
+	.enable = s6e63j0x03_enable,
+	.get_modes = s6e63j0x03_get_modes,
+};
+
+static int s6e63j0x03_probe(struct mipi_dsi_device *dsi)
+{
+	struct device *dev = &dsi->dev;
+	struct s6e63j0x03 *ctx;
+	int ret;
+
+	ctx = devm_kzalloc(dev, sizeof(struct s6e63j0x03), GFP_KERNEL);
+	if (!ctx)
+		return -ENOMEM;
+
+	mipi_dsi_set_drvdata(dsi, ctx);
+
+	ctx->dev = dev;
+
+	dsi->lanes = 1;
+	dsi->format = MIPI_DSI_FMT_RGB888;
+	dsi->mode_flags = MIPI_DSI_MODE_EOT_PACKET;
+
+	ctx->supplies[0].supply = "vdd3";
+	ctx->supplies[1].supply = "vci";
+	ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(ctx->supplies),
+				      ctx->supplies);
+	if (ret < 0) {
+		dev_err(dev, "failed to get regulators: %d\n", ret);
+		return ret;
+	}
+
+	ctx->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW);
+	if (IS_ERR(ctx->reset_gpio)) {
+		dev_err(dev, "cannot get reset-gpio: %ld\n",
+				PTR_ERR(ctx->reset_gpio));
+		return PTR_ERR(ctx->reset_gpio);
+	}
+
+	drm_panel_init(&ctx->panel);
+	ctx->panel.dev = dev;
+	ctx->panel.funcs = &s6e63j0x03_funcs;
+
+	ctx->bl_dev = backlight_device_register("s6e63j0x03", dev, ctx,
+						&s6e63j0x03_bl_ops, NULL);
+	if (IS_ERR(ctx->bl_dev)) {
+		dev_err(dev, "failed to register backlight device\n");
+		return PTR_ERR(ctx->bl_dev);
+	}
+
+	ctx->bl_dev->props.max_brightness = MAX_BRIGHTNESS;
+	ctx->bl_dev->props.brightness = DEFAULT_BRIGHTNESS;
+	ctx->bl_dev->props.power = FB_BLANK_POWERDOWN;
+
+	ret = drm_panel_add(&ctx->panel);
+	if (ret < 0)
+		goto unregister_backlight;
+
+	ret = mipi_dsi_attach(dsi);
+	if (ret < 0)
+		goto remove_panel;
+
+	return ret;
+
+remove_panel:
+	drm_panel_remove(&ctx->panel);
+
+unregister_backlight:
+	backlight_device_unregister(ctx->bl_dev);
+
+	return ret;
+}
+
+static int s6e63j0x03_remove(struct mipi_dsi_device *dsi)
+{
+	struct s6e63j0x03 *ctx = mipi_dsi_get_drvdata(dsi);
+
+	mipi_dsi_detach(dsi);
+	drm_panel_remove(&ctx->panel);
+
+	backlight_device_unregister(ctx->bl_dev);
+
+	return 0;
+}
+
+static const struct of_device_id s6e63j0x03_of_match[] = {
+	{ .compatible = "samsung,s6e63j0x03" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, s6e63j0x03_of_match);
+
+static struct mipi_dsi_driver s6e63j0x03_driver = {
+	.probe = s6e63j0x03_probe,
+	.remove = s6e63j0x03_remove,
+	.driver = {
+		.name = "panel_samsung_s6e63j0x03",
+		.of_match_table = s6e63j0x03_of_match,
+	},
+};
+module_mipi_dsi_driver(s6e63j0x03_driver);
+
+MODULE_AUTHOR("Inki Dae <inki.dae@samsung.com>");
+MODULE_AUTHOR("Hoegeun Kwon <hoegeun.kwon@samsung.com>");
+MODULE_DESCRIPTION("MIPI-DSI based s6e63j0x03 AMOLED LCD Panel Driver");
+MODULE_LICENSE("GPL v2");
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v2 3/4] ARM: dts: exynos: Fix the active of reset gpios from rinato dts
       [not found]   ` <CGME20170615100342epcas1p4feeb3d12bb9e120bf40dc5945c4b2460@epcas1p4.samsung.com>
@ 2017-06-15 10:03     ` Hoegeun Kwon
  2017-06-16 16:20       ` Krzysztof Kozlowski
  0 siblings, 1 reply; 8+ messages in thread
From: Hoegeun Kwon @ 2017-06-15 10:03 UTC (permalink / raw)
  To: thierry.reding, airlied, robh+dt, mark.rutland, catalin.marinas,
	will.deacon, kgene, krzk
  Cc: dri-devel, devicetree, linux-arm-kernel, linux-samsung-soc,
	linux-kernel, javier, a.hajda, Hoegeun Kwon

This reset gpios is active low, therefore fix from active high to low.

Signed-off-by: Hoegeun Kwon <hoegeun.kwon@samsung.com>
---
 arch/arm/boot/dts/exynos3250-rinato.dts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/exynos3250-rinato.dts b/arch/arm/boot/dts/exynos3250-rinato.dts
index 82e676a..de2702e 100644
--- a/arch/arm/boot/dts/exynos3250-rinato.dts
+++ b/arch/arm/boot/dts/exynos3250-rinato.dts
@@ -225,7 +225,7 @@
 		reg = <0>;
 		vdd3-supply = <&ldo16_reg>;
 		vci-supply = <&ldo20_reg>;
-		reset-gpios = <&gpe0 1 GPIO_ACTIVE_HIGH>;
+		reset-gpios = <&gpe0 1 GPIO_ACTIVE_LOW>;
 		te-gpios = <&gpx0 6 GPIO_ACTIVE_HIGH>;
 		power-on-delay= <30>;
 		power-off-delay= <120>;
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v2 4/4] ARM: dts: exynos: Remove the display-timing and delay from rinato dts
       [not found]   ` <CGME20170615100342epcas1p43156787a99d5889734b54e8dbf83b42a@epcas1p4.samsung.com>
@ 2017-06-15 10:03     ` Hoegeun Kwon
  0 siblings, 0 replies; 8+ messages in thread
From: Hoegeun Kwon @ 2017-06-15 10:03 UTC (permalink / raw)
  To: thierry.reding, airlied, robh+dt, mark.rutland, catalin.marinas,
	will.deacon, kgene, krzk
  Cc: dri-devel, devicetree, linux-arm-kernel, linux-samsung-soc,
	linux-kernel, javier, a.hajda, Hoegeun Kwon

The display-timing and delay are included in the panel driver. So it
should be removed in dts.

Signed-off-by: Hoegeun Kwon <hoegeun.kwon@samsung.com>
---
 arch/arm/boot/dts/exynos3250-rinato.dts | 22 ----------------------
 1 file changed, 22 deletions(-)

diff --git a/arch/arm/boot/dts/exynos3250-rinato.dts b/arch/arm/boot/dts/exynos3250-rinato.dts
index de2702e..3df88383 100644
--- a/arch/arm/boot/dts/exynos3250-rinato.dts
+++ b/arch/arm/boot/dts/exynos3250-rinato.dts
@@ -227,28 +227,6 @@
 		vci-supply = <&ldo20_reg>;
 		reset-gpios = <&gpe0 1 GPIO_ACTIVE_LOW>;
 		te-gpios = <&gpx0 6 GPIO_ACTIVE_HIGH>;
-		power-on-delay= <30>;
-		power-off-delay= <120>;
-		reset-delay = <5>;
-		init-delay = <100>;
-		flip-horizontal;
-		flip-vertical;
-		panel-width-mm = <29>;
-		panel-height-mm = <29>;
-
-		display-timings {
-			timing-0 {
-				clock-frequency = <4600000>;
-				hactive = <320>;
-				vactive = <320>;
-				hfront-porch = <1>;
-				hback-porch = <1>;
-				hsync-len = <1>;
-				vfront-porch = <150>;
-				vback-porch = <1>;
-				vsync-len = <2>;
-			};
-		};
 	};
 };
 
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 3/4] ARM: dts: exynos: Fix the active of reset gpios from rinato dts
  2017-06-15 10:03     ` [PATCH v2 3/4] ARM: dts: exynos: Fix the active of reset gpios from rinato dts Hoegeun Kwon
@ 2017-06-16 16:20       ` Krzysztof Kozlowski
  0 siblings, 0 replies; 8+ messages in thread
From: Krzysztof Kozlowski @ 2017-06-16 16:20 UTC (permalink / raw)
  To: Hoegeun Kwon
  Cc: thierry.reding, airlied, robh+dt, mark.rutland, catalin.marinas,
	will.deacon, kgene, dri-devel, devicetree, linux-arm-kernel,
	linux-samsung-soc, linux-kernel, javier, a.hajda

On Thu, Jun 15, 2017 at 07:03:29PM +0900, Hoegeun Kwon wrote:
> This reset gpios is active low, therefore fix from active high to low.
> 
> Signed-off-by: Hoegeun Kwon <hoegeun.kwon@samsung.com>
> ---
>  arch/arm/boot/dts/exynos3250-rinato.dts | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 

Thanks, applied this one only. As mentioned before, 4/4 will wait for
driver.

Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 2/4] drm/panel: Add support for s6e63j0x03 panel driver
  2017-06-15 10:03     ` [PATCH v2 2/4] drm/panel: Add support for s6e63j0x03 panel driver Hoegeun Kwon
@ 2017-06-19  9:14       ` Andrzej Hajda
  2017-06-20  4:36         ` Hoegeun Kwon
  0 siblings, 1 reply; 8+ messages in thread
From: Andrzej Hajda @ 2017-06-19  9:14 UTC (permalink / raw)
  To: Hoegeun Kwon, thierry.reding, airlied, robh+dt, mark.rutland,
	catalin.marinas, will.deacon, kgene, krzk
  Cc: dri-devel, devicetree, linux-arm-kernel, linux-samsung-soc,
	linux-kernel, javier, Inki Dae, Hyungwon Hwang

On 15.06.2017 12:03, Hoegeun Kwon wrote:
> This patch adds MIPI-DSI based S6E63J0X03 AMOLED LCD panel driver
> which uses mipi_dsi bus to communicate with panel. The panel has
> 320×320 resolution in 1.63" physical panel. This panel is used in
> Samsung Galaxy Gear 2.
>
> Signed-off-by: Inki Dae <inki.dae@samsung.com>
> Signed-off-by: Hyungwon Hwang <human.hwang@samsung.com>
> Signed-off-by: Hoegeun Kwon <hoegeun.kwon@samsung.com>
> ---
>  drivers/gpu/drm/panel/Kconfig                    |   7 +
>  drivers/gpu/drm/panel/Makefile                   |   1 +
>  drivers/gpu/drm/panel/panel-samsung-s6e63j0x03.c | 476 +++++++++++++++++++++++
>  3 files changed, 484 insertions(+)
>  create mode 100644 drivers/gpu/drm/panel/panel-samsung-s6e63j0x03.c
>
> diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
> index 3e29a99..3f4afde 100644
> --- a/drivers/gpu/drm/panel/Kconfig
> +++ b/drivers/gpu/drm/panel/Kconfig
> @@ -68,6 +68,13 @@ config DRM_PANEL_SAMSUNG_S6E3HA2
>  	depends on DRM_MIPI_DSI
>  	select VIDEOMODE_HELPERS
>  
> +config DRM_PANEL_SAMSUNG_S6E63J0X03
> +	tristate "Samsung S6E63J0X03 DSI command mode panel"
> +	depends on OF
> +	depends on DRM_MIPI_DSI
> +	depends on BACKLIGHT_CLASS_DEVICE
> +	select VIDEOMODE_HELPERS
> +
>  config DRM_PANEL_SAMSUNG_S6E8AA0
>  	tristate "Samsung S6E8AA0 DSI video mode panel"
>  	depends on OF
> diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
> index 292b3c7..f028269 100644
> --- a/drivers/gpu/drm/panel/Makefile
> +++ b/drivers/gpu/drm/panel/Makefile
> @@ -5,6 +5,7 @@ obj-$(CONFIG_DRM_PANEL_LG_LG4573) += panel-lg-lg4573.o
>  obj-$(CONFIG_DRM_PANEL_PANASONIC_VVX10F034N00) += panel-panasonic-vvx10f034n00.o
>  obj-$(CONFIG_DRM_PANEL_SAMSUNG_LD9040) += panel-samsung-ld9040.o
>  obj-$(CONFIG_DRM_PANEL_SAMSUNG_S6E3HA2) += panel-samsung-s6e3ha2.o
> +obj-$(CONFIG_DRM_PANEL_SAMSUNG_S6E63J0X03) += panel-samsung-s6e63j0x03.o
>  obj-$(CONFIG_DRM_PANEL_SAMSUNG_S6E8AA0) += panel-samsung-s6e8aa0.o
>  obj-$(CONFIG_DRM_PANEL_SHARP_LQ101R1SX01) += panel-sharp-lq101r1sx01.o
>  obj-$(CONFIG_DRM_PANEL_SHARP_LS043T1LE01) += panel-sharp-ls043t1le01.o
> diff --git a/drivers/gpu/drm/panel/panel-samsung-s6e63j0x03.c b/drivers/gpu/drm/panel/panel-samsung-s6e63j0x03.c
> new file mode 100644
> index 0000000..dd038bc
> --- /dev/null
> +++ b/drivers/gpu/drm/panel/panel-samsung-s6e63j0x03.c
> @@ -0,0 +1,476 @@
> +/*
> + * MIPI-DSI based S6E63J0X03 AMOLED lcd 1.63 inch panel driver.
> + *
> + * Copyright (c) 2014-2017 Samsung Electronics Co., Ltd
> + *
> + * Inki Dae <inki.dae@samsung.com>
> + * Hoegeun Kwon <hoegeun.kwon@samsung.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <drm/drmP.h>
> +#include <drm/drm_mipi_dsi.h>
> +#include <drm/drm_panel.h>
> +#include <linux/backlight.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/regulator/consumer.h>
> +#include <video/mipi_display.h>
> +
> +#define MCS_LEVEL2_KEY		0xf0
> +#define MCS_MTP_KEY		0xf1
> +#define MCS_MTP_SET3		0xd4
> +
> +#define MIN_BRIGHTNESS		0

This macro is not used at all and according to API it must be 0, so it
can be removed.

> +#define MAX_BRIGHTNESS		100
> +#define DEFAULT_BRIGHTNESS	80
> +
> +#define NUM_GAMMA_STEPS		9
> +#define GAMMA_CMD_CNT		28
> +
> +struct s6e63j0x03 {
> +	struct device *dev;
> +	struct drm_panel panel;
> +	struct backlight_device *bl_dev;
> +
> +	struct regulator_bulk_data supplies[2];
> +	struct gpio_desc *reset_gpio;
> +};
> +
> +static const unsigned char gamma_tbl[NUM_GAMMA_STEPS][GAMMA_CMD_CNT] = {
> +	{	/* Gamma 10 */
> +		MCS_MTP_SET3,
> +		0x00, 0x00, 0x00, 0x7f, 0x7f, 0x7f, 0x52, 0x6b, 0x6f, 0x26,
> +		0x28, 0x2d, 0x28, 0x26, 0x27, 0x33, 0x34, 0x32, 0x36, 0x36,
> +		0x35, 0x00, 0xab, 0x00, 0xae, 0x00, 0xbf
> +	},
> +	{	/* gamma 30 */
> +		MCS_MTP_SET3,
> +		0x00, 0x00, 0x00, 0x70, 0x7f, 0x7f, 0x4e, 0x64, 0x69, 0x26,
> +		0x27, 0x2a, 0x28, 0x29, 0x27, 0x31, 0x32, 0x31, 0x35, 0x34,
> +		0x35, 0x00, 0xc4, 0x00, 0xca, 0x00, 0xdc
> +	},
> +	{	/* gamma 60 */
> +		MCS_MTP_SET3,
> +		0x00, 0x00, 0x00, 0x65, 0x7b, 0x7d, 0x5f, 0x67, 0x68, 0x2a,
> +		0x28, 0x29, 0x28, 0x2a, 0x27, 0x31, 0x2f, 0x30, 0x34, 0x33,
> +		0x34, 0x00, 0xd9, 0x00, 0xe4, 0x00, 0xf5
> +	},
> +	{	/* gamma 90 */
> +		MCS_MTP_SET3,
> +		0x00, 0x00, 0x00, 0x4d, 0x6f, 0x71, 0x67, 0x6a, 0x6c, 0x29,
> +		0x28, 0x28, 0x28, 0x29, 0x27, 0x30, 0x2e, 0x30, 0x32, 0x31,
> +		0x31, 0x00, 0xea, 0x00, 0xf6, 0x01, 0x09
> +	},
> +	{	/* gamma 120 */
> +		MCS_MTP_SET3,
> +		0x00, 0x00, 0x00, 0x3d, 0x66, 0x68, 0x69, 0x69, 0x69, 0x28,
> +		0x28, 0x27, 0x28, 0x28, 0x27, 0x30, 0x2e, 0x2f, 0x31, 0x31,
> +		0x30, 0x00, 0xf9, 0x01, 0x05, 0x01, 0x1b
> +	},
> +	{	/* gamma 150 */
> +		MCS_MTP_SET3,
> +		0x00, 0x00, 0x00, 0x31, 0x51, 0x53, 0x66, 0x66, 0x67, 0x28,
> +		0x29, 0x27, 0x28, 0x27, 0x27, 0x2e, 0x2d, 0x2e, 0x31, 0x31,
> +		0x30, 0x01, 0x04, 0x01, 0x11, 0x01, 0x29
> +	},
> +	{	/* gamma 200 */
> +		MCS_MTP_SET3,
> +		0x00, 0x00, 0x00, 0x2f, 0x4f, 0x51, 0x67, 0x65, 0x65, 0x29,
> +		0x2a, 0x28, 0x27, 0x25, 0x26, 0x2d, 0x2c, 0x2c, 0x30, 0x30,
> +		0x30, 0x01, 0x14, 0x01, 0x23, 0x01, 0x3b
> +	},
> +	{	/* gamma 240 */
> +		MCS_MTP_SET3,
> +		0x00, 0x00, 0x00, 0x2c, 0x4d, 0x50, 0x65, 0x63, 0x64, 0x2a,
> +		0x2c, 0x29, 0x26, 0x24, 0x25, 0x2c, 0x2b, 0x2b, 0x30, 0x30,
> +		0x30, 0x01, 0x1e, 0x01, 0x2f, 0x01, 0x47
> +	},
> +	{	/* gamma 300 */
> +		MCS_MTP_SET3,
> +		0x00, 0x00, 0x00, 0x38, 0x61, 0x64, 0x65, 0x63, 0x64, 0x28,
> +		0x2a, 0x27, 0x26, 0x23, 0x25, 0x2b, 0x2b, 0x2a, 0x30, 0x2f,
> +		0x30, 0x01, 0x2d, 0x01, 0x3f, 0x01, 0x57
> +	}
> +};
> +
> +static inline struct s6e63j0x03 *panel_to_s6e63j0x03(struct drm_panel *panel)
> +{
> +	return container_of(panel, struct s6e63j0x03, panel);
> +}
> +
> +static inline ssize_t s6e63j0x03_dcs_write_seq(struct s6e63j0x03 *ctx,
> +					const void *seq, size_t len)
> +{
> +	struct mipi_dsi_device *dsi = to_mipi_dsi_device(ctx->dev);
> +
> +	return mipi_dsi_dcs_write_buffer(dsi, seq, len);
> +}
> +
> +#define s6e63j0x03_dcs_write_seq_static(ctx, seq...) do {	\
> +	static const u8 d[] = { seq };				\
> +	int ret;						\
> +	ret = s6e63j0x03_dcs_write_seq(ctx, d, ARRAY_SIZE(d));	\
> +	if (ret < 0)						\
> +		return ret;					\
> +} while (0)
> +
> +#define s6e63j0x03_call_write_func(ret, func) do {	\
> +	ret = (func);					\
> +	if (ret < 0)					\
> +		return ret;				\
> +} while (0)

Both above macros violate kernel coding style: macros that affect
control flow are the 1st thing to avoid, see relevant chapter in the
kernel docs [1].
I know similar macros have been already merged with two latest panel
drivers, so I suspect you have just followed current practice. So this
question
is rather to Thierry, is it the method you want to use to avoid 'if
(ret) return ret' plague? What about explicit violation of kernel coding
style?

[1]:
https://www.kernel.org/doc/html/v4.10/process/coding-style.html#macros-enums-and-rtl

> +
> +static inline int s6e63j0x03_enable_lv2_command(struct s6e63j0x03 *ctx)
> +{
> +	s6e63j0x03_dcs_write_seq_static(ctx, MCS_LEVEL2_KEY, 0x5a, 0x5a);
> +	return 0;
> +}
> +
> +static inline int s6e63j0x03_apply_mtp_key(struct s6e63j0x03 *ctx, bool on)
> +{
> +	if (on)
> +		s6e63j0x03_dcs_write_seq_static(ctx, MCS_MTP_KEY, 0x5a, 0x5a);
> +	else
> +		s6e63j0x03_dcs_write_seq_static(ctx, MCS_MTP_KEY, 0xa5, 0xa5);
> +
> +	return 0;
> +}
> +
> +static int s6e63j0x03_power_on(struct s6e63j0x03 *ctx)
> +{
> +	int ret;
> +
> +	ret = regulator_bulk_enable(ARRAY_SIZE(ctx->supplies), ctx->supplies);
> +	if (ret < 0)
> +		return ret;
> +
> +	msleep(30);
> +
> +	gpiod_set_value(ctx->reset_gpio, 1);
> +	usleep_range(1000, 2000);
> +	gpiod_set_value(ctx->reset_gpio, 0);
> +	usleep_range(5000, 6000);
> +
> +	return 0;
> +}
> +
> +static int s6e63j0x03_power_off(struct s6e63j0x03 *ctx)
> +{
> +	return regulator_bulk_disable(ARRAY_SIZE(ctx->supplies), ctx->supplies);
> +}
> +
> +static unsigned int s6e63j0x03_get_brightness_index(unsigned int brightness)
> +{
> +	unsigned int index;
> +
> +	index = brightness / (MAX_BRIGHTNESS / NUM_GAMMA_STEPS);
> +
> +	if (index >= NUM_GAMMA_STEPS)
> +		index = NUM_GAMMA_STEPS - 1;
> +
> +	return index;
> +}
> +
> +static int s6e63j0x03_update_gamma(struct s6e63j0x03 *ctx,
> +					unsigned int brightness)
> +{
> +	struct backlight_device *bl_dev = ctx->bl_dev;
> +	unsigned int index = s6e63j0x03_get_brightness_index(brightness);
> +	int ret;
> +
> +	s6e63j0x03_call_write_func(ret, s6e63j0x03_apply_mtp_key(ctx, true));
> +	s6e63j0x03_call_write_func(ret,
> +		s6e63j0x03_dcs_write_seq(ctx, gamma_tbl[index], GAMMA_CMD_CNT));
> +	s6e63j0x03_call_write_func(ret, s6e63j0x03_apply_mtp_key(ctx, false));
> +
> +	bl_dev->props.brightness = brightness;
> +
> +	return 0;
> +}
> +
> +static int s6e63j0x03_set_brightness(struct backlight_device *bl_dev)
> +{
> +	struct s6e63j0x03 *ctx = (struct s6e63j0x03 *)bl_get_data(bl_dev);

Explicit casting can be removed.

> +	unsigned int brightness = bl_dev->props.brightness;
> +	int ret;
> +
> +	s6e63j0x03_call_write_func(ret,
> +		s6e63j0x03_update_gamma(ctx, brightness));
> +
> +	return 0;
> +}
> +
> +static const struct backlight_ops s6e63j0x03_bl_ops = {
> +	.update_status = s6e63j0x03_set_brightness,
> +};
> +
> +static int s6e63j0x03_disable(struct drm_panel *panel)
> +{
> +	struct s6e63j0x03 *ctx = panel_to_s6e63j0x03(panel);
> +	struct mipi_dsi_device *dsi = to_mipi_dsi_device(ctx->dev);
> +	int ret;
> +
> +	s6e63j0x03_call_write_func(ret, mipi_dsi_dcs_set_display_off(dsi));
> +	ctx->bl_dev->props.power = FB_BLANK_NORMAL;
> +
> +	s6e63j0x03_call_write_func(ret, mipi_dsi_dcs_enter_sleep_mode(dsi));
> +	msleep(120);
> +
> +	return 0;
> +}
> +
> +static int s6e63j0x03_unprepare(struct drm_panel *panel)
> +{
> +	struct s6e63j0x03 *ctx = panel_to_s6e63j0x03(panel);
> +	int ret;
> +
> +	ret = s6e63j0x03_power_off(ctx);
> +	if (ret < 0)
> +		return ret;
> +
> +	ctx->bl_dev->props.power = FB_BLANK_POWERDOWN;
> +
> +	return 0;
> +}
> +
> +static int s6e63j0x03_panel_init(struct s6e63j0x03 *ctx)
> +{
> +	struct mipi_dsi_device *dsi = to_mipi_dsi_device(ctx->dev);
> +	int ret;
> +
> +	s6e63j0x03_call_write_func(ret, s6e63j0x03_enable_lv2_command(ctx));
> +	s6e63j0x03_call_write_func(ret, s6e63j0x03_apply_mtp_key(ctx, true));
> +
> +	/* set porch adjustment */
> +	s6e63j0x03_dcs_write_seq_static(ctx, 0xf2, 0x1c, 0x28);
> +	/* set frame freq */
> +	s6e63j0x03_dcs_write_seq_static(ctx, 0xb5, 0x00, 0x02, 0x00);
> +	/* set caset, paset */
> +	s6e63j0x03_call_write_func(ret,
> +		mipi_dsi_dcs_set_column_address(dsi, 0x0014, 0x0153));
> +	s6e63j0x03_call_write_func(ret,
> +		mipi_dsi_dcs_set_page_address(dsi, 0x0000, 0x013f));

For two calls above arguments are line or column number, you should use
decimals here, sth like below:

    #define FIRST_COLUMN 20
    mipi_dsi_dcs_set_column_address(dsi, FIRST_COLUMN,
default_mode.hdisplay - 1 + FIRST_COLUMN );
    mipi_dsi_dcs_set_page_address(dsi, 0, default_mode.hdisplay - 1);

This way it will be easier to understand what is going on here.

Regards
Andrzej

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 2/4] drm/panel: Add support for s6e63j0x03 panel driver
  2017-06-19  9:14       ` Andrzej Hajda
@ 2017-06-20  4:36         ` Hoegeun Kwon
  0 siblings, 0 replies; 8+ messages in thread
From: Hoegeun Kwon @ 2017-06-20  4:36 UTC (permalink / raw)
  To: Andrzej Hajda, thierry.reding, airlied, robh+dt, mark.rutland,
	catalin.marinas, will.deacon, kgene, krzk
  Cc: dri-devel, devicetree, linux-arm-kernel, linux-samsung-soc,
	linux-kernel, javier, Inki Dae, Hyungwon Hwang, Hoegeun Kwon

On 06/19/2017 06:14 PM, Andrzej Hajda wrote:

> On 15.06.2017 12:03, Hoegeun Kwon wrote:
>> This patch adds MIPI-DSI based S6E63J0X03 AMOLED LCD panel driver
>> which uses mipi_dsi bus to communicate with panel. The panel has
>> 320×320 resolution in 1.63" physical panel. This panel is used in
>> Samsung Galaxy Gear 2.
>>
>> Signed-off-by: Inki Dae <inki.dae@samsung.com>
>> Signed-off-by: Hyungwon Hwang <human.hwang@samsung.com>
>> Signed-off-by: Hoegeun Kwon <hoegeun.kwon@samsung.com>
>> ---
>>   drivers/gpu/drm/panel/Kconfig                    |   7 +
>>   drivers/gpu/drm/panel/Makefile                   |   1 +
>>   drivers/gpu/drm/panel/panel-samsung-s6e63j0x03.c | 476 +++++++++++++++++++++++
>>   3 files changed, 484 insertions(+)
>>   create mode 100644 drivers/gpu/drm/panel/panel-samsung-s6e63j0x03.c
>>
>> diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
>> index 3e29a99..3f4afde 100644
>> --- a/drivers/gpu/drm/panel/Kconfig
>> +++ b/drivers/gpu/drm/panel/Kconfig
>> @@ -68,6 +68,13 @@ config DRM_PANEL_SAMSUNG_S6E3HA2
>>   	depends on DRM_MIPI_DSI
>>   	select VIDEOMODE_HELPERS
>>   
>> +config DRM_PANEL_SAMSUNG_S6E63J0X03
>> +	tristate "Samsung S6E63J0X03 DSI command mode panel"
>> +	depends on OF
>> +	depends on DRM_MIPI_DSI
>> +	depends on BACKLIGHT_CLASS_DEVICE
>> +	select VIDEOMODE_HELPERS
>> +
>>   config DRM_PANEL_SAMSUNG_S6E8AA0
>>   	tristate "Samsung S6E8AA0 DSI video mode panel"
>>   	depends on OF
>> diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
>> index 292b3c7..f028269 100644
>> --- a/drivers/gpu/drm/panel/Makefile
>> +++ b/drivers/gpu/drm/panel/Makefile
>> @@ -5,6 +5,7 @@ obj-$(CONFIG_DRM_PANEL_LG_LG4573) += panel-lg-lg4573.o
>>   obj-$(CONFIG_DRM_PANEL_PANASONIC_VVX10F034N00) += panel-panasonic-vvx10f034n00.o
>>   obj-$(CONFIG_DRM_PANEL_SAMSUNG_LD9040) += panel-samsung-ld9040.o
>>   obj-$(CONFIG_DRM_PANEL_SAMSUNG_S6E3HA2) += panel-samsung-s6e3ha2.o
>> +obj-$(CONFIG_DRM_PANEL_SAMSUNG_S6E63J0X03) += panel-samsung-s6e63j0x03.o
>>   obj-$(CONFIG_DRM_PANEL_SAMSUNG_S6E8AA0) += panel-samsung-s6e8aa0.o
>>   obj-$(CONFIG_DRM_PANEL_SHARP_LQ101R1SX01) += panel-sharp-lq101r1sx01.o
>>   obj-$(CONFIG_DRM_PANEL_SHARP_LS043T1LE01) += panel-sharp-ls043t1le01.o
>> diff --git a/drivers/gpu/drm/panel/panel-samsung-s6e63j0x03.c b/drivers/gpu/drm/panel/panel-samsung-s6e63j0x03.c
>> new file mode 100644
>> index 0000000..dd038bc
>> --- /dev/null
>> +++ b/drivers/gpu/drm/panel/panel-samsung-s6e63j0x03.c
>> @@ -0,0 +1,476 @@
>> +/*
>> + * MIPI-DSI based S6E63J0X03 AMOLED lcd 1.63 inch panel driver.
>> + *
>> + * Copyright (c) 2014-2017 Samsung Electronics Co., Ltd
>> + *
>> + * Inki Dae <inki.dae@samsung.com>
>> + * Hoegeun Kwon <hoegeun.kwon@samsung.com>
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + */
>> +
>> +#include <drm/drmP.h>
>> +#include <drm/drm_mipi_dsi.h>
>> +#include <drm/drm_panel.h>
>> +#include <linux/backlight.h>
>> +#include <linux/gpio/consumer.h>
>> +#include <linux/regulator/consumer.h>
>> +#include <video/mipi_display.h>
>> +
>> +#define MCS_LEVEL2_KEY		0xf0
>> +#define MCS_MTP_KEY		0xf1
>> +#define MCS_MTP_SET3		0xd4
>> +
>> +#define MIN_BRIGHTNESS		0
> This macro is not used at all and according to API it must be 0, so it
> can be removed.
>
>> +#define MAX_BRIGHTNESS		100
>> +#define DEFAULT_BRIGHTNESS	80
>> +
>> +#define NUM_GAMMA_STEPS		9
>> +#define GAMMA_CMD_CNT		28
>> +
>> +struct s6e63j0x03 {
>> +	struct device *dev;
>> +	struct drm_panel panel;
>> +	struct backlight_device *bl_dev;
>> +
>> +	struct regulator_bulk_data supplies[2];
>> +	struct gpio_desc *reset_gpio;
>> +};
>> +
>> +static const unsigned char gamma_tbl[NUM_GAMMA_STEPS][GAMMA_CMD_CNT] = {
>> +	{	/* Gamma 10 */
>> +		MCS_MTP_SET3,
>> +		0x00, 0x00, 0x00, 0x7f, 0x7f, 0x7f, 0x52, 0x6b, 0x6f, 0x26,
>> +		0x28, 0x2d, 0x28, 0x26, 0x27, 0x33, 0x34, 0x32, 0x36, 0x36,
>> +		0x35, 0x00, 0xab, 0x00, 0xae, 0x00, 0xbf
>> +	},
>> +	{	/* gamma 30 */
>> +		MCS_MTP_SET3,
>> +		0x00, 0x00, 0x00, 0x70, 0x7f, 0x7f, 0x4e, 0x64, 0x69, 0x26,
>> +		0x27, 0x2a, 0x28, 0x29, 0x27, 0x31, 0x32, 0x31, 0x35, 0x34,
>> +		0x35, 0x00, 0xc4, 0x00, 0xca, 0x00, 0xdc
>> +	},
>> +	{	/* gamma 60 */
>> +		MCS_MTP_SET3,
>> +		0x00, 0x00, 0x00, 0x65, 0x7b, 0x7d, 0x5f, 0x67, 0x68, 0x2a,
>> +		0x28, 0x29, 0x28, 0x2a, 0x27, 0x31, 0x2f, 0x30, 0x34, 0x33,
>> +		0x34, 0x00, 0xd9, 0x00, 0xe4, 0x00, 0xf5
>> +	},
>> +	{	/* gamma 90 */
>> +		MCS_MTP_SET3,
>> +		0x00, 0x00, 0x00, 0x4d, 0x6f, 0x71, 0x67, 0x6a, 0x6c, 0x29,
>> +		0x28, 0x28, 0x28, 0x29, 0x27, 0x30, 0x2e, 0x30, 0x32, 0x31,
>> +		0x31, 0x00, 0xea, 0x00, 0xf6, 0x01, 0x09
>> +	},
>> +	{	/* gamma 120 */
>> +		MCS_MTP_SET3,
>> +		0x00, 0x00, 0x00, 0x3d, 0x66, 0x68, 0x69, 0x69, 0x69, 0x28,
>> +		0x28, 0x27, 0x28, 0x28, 0x27, 0x30, 0x2e, 0x2f, 0x31, 0x31,
>> +		0x30, 0x00, 0xf9, 0x01, 0x05, 0x01, 0x1b
>> +	},
>> +	{	/* gamma 150 */
>> +		MCS_MTP_SET3,
>> +		0x00, 0x00, 0x00, 0x31, 0x51, 0x53, 0x66, 0x66, 0x67, 0x28,
>> +		0x29, 0x27, 0x28, 0x27, 0x27, 0x2e, 0x2d, 0x2e, 0x31, 0x31,
>> +		0x30, 0x01, 0x04, 0x01, 0x11, 0x01, 0x29
>> +	},
>> +	{	/* gamma 200 */
>> +		MCS_MTP_SET3,
>> +		0x00, 0x00, 0x00, 0x2f, 0x4f, 0x51, 0x67, 0x65, 0x65, 0x29,
>> +		0x2a, 0x28, 0x27, 0x25, 0x26, 0x2d, 0x2c, 0x2c, 0x30, 0x30,
>> +		0x30, 0x01, 0x14, 0x01, 0x23, 0x01, 0x3b
>> +	},
>> +	{	/* gamma 240 */
>> +		MCS_MTP_SET3,
>> +		0x00, 0x00, 0x00, 0x2c, 0x4d, 0x50, 0x65, 0x63, 0x64, 0x2a,
>> +		0x2c, 0x29, 0x26, 0x24, 0x25, 0x2c, 0x2b, 0x2b, 0x30, 0x30,
>> +		0x30, 0x01, 0x1e, 0x01, 0x2f, 0x01, 0x47
>> +	},
>> +	{	/* gamma 300 */
>> +		MCS_MTP_SET3,
>> +		0x00, 0x00, 0x00, 0x38, 0x61, 0x64, 0x65, 0x63, 0x64, 0x28,
>> +		0x2a, 0x27, 0x26, 0x23, 0x25, 0x2b, 0x2b, 0x2a, 0x30, 0x2f,
>> +		0x30, 0x01, 0x2d, 0x01, 0x3f, 0x01, 0x57
>> +	}
>> +};
>> +
>> +static inline struct s6e63j0x03 *panel_to_s6e63j0x03(struct drm_panel *panel)
>> +{
>> +	return container_of(panel, struct s6e63j0x03, panel);
>> +}
>> +
>> +static inline ssize_t s6e63j0x03_dcs_write_seq(struct s6e63j0x03 *ctx,
>> +					const void *seq, size_t len)
>> +{
>> +	struct mipi_dsi_device *dsi = to_mipi_dsi_device(ctx->dev);
>> +
>> +	return mipi_dsi_dcs_write_buffer(dsi, seq, len);
>> +}
>> +
>> +#define s6e63j0x03_dcs_write_seq_static(ctx, seq...) do {	\
>> +	static const u8 d[] = { seq };				\
>> +	int ret;						\
>> +	ret = s6e63j0x03_dcs_write_seq(ctx, d, ARRAY_SIZE(d));	\
>> +	if (ret < 0)						\
>> +		return ret;					\
>> +} while (0)
>> +
>> +#define s6e63j0x03_call_write_func(ret, func) do {	\
>> +	ret = (func);					\
>> +	if (ret < 0)					\
>> +		return ret;				\
>> +} while (0)
> Both above macros violate kernel coding style: macros that affect
> control flow are the 1st thing to avoid, see relevant chapter in the
> kernel docs [1].
> I know similar macros have been already merged with two latest panel
> drivers, so I suspect you have just followed current practice. So this
> question
> is rather to Thierry, is it the method you want to use to avoid 'if
> (ret) return ret' plague? What about explicit violation of kernel coding
> style?
>
> [1]:
> https://www.kernel.org/doc/html/v4.10/process/coding-style.html#macros-enums-and-rtl

Thanks for your comment.

Yes, I want avoid 'if (tet) return ret' plague.
But, this method is violation of kernel coding style.
So then we should use 'if (ret) return ret' style,
if we don't want toviolation of kernel coding style.
Do you want to do that?

Best regards,
Hoegeun

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2017-06-20  4:36 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <CGME20170615100341epcas5p18affc41c65c13d0daac005290cf1d249@epcas5p1.samsung.com>
2017-06-15 10:03 ` [PATCH v2 0/4] Add support for the s6e63j0x03 panel on Rinato board Hoegeun Kwon
     [not found]   ` <CGME20170615100341epcas1p46fc952e3aa7341b96e18327895ef619e@epcas1p4.samsung.com>
2017-06-15 10:03     ` [PATCH v2 1/4] dt-bindings: Add support for samsung s6e63j0x03 panel binding Hoegeun Kwon
     [not found]   ` <CGME20170615100342epcas1p4b15ffce9ea2018e2bc90bb04061aded5@epcas1p4.samsung.com>
2017-06-15 10:03     ` [PATCH v2 2/4] drm/panel: Add support for s6e63j0x03 panel driver Hoegeun Kwon
2017-06-19  9:14       ` Andrzej Hajda
2017-06-20  4:36         ` Hoegeun Kwon
     [not found]   ` <CGME20170615100342epcas1p4feeb3d12bb9e120bf40dc5945c4b2460@epcas1p4.samsung.com>
2017-06-15 10:03     ` [PATCH v2 3/4] ARM: dts: exynos: Fix the active of reset gpios from rinato dts Hoegeun Kwon
2017-06-16 16:20       ` Krzysztof Kozlowski
     [not found]   ` <CGME20170615100342epcas1p43156787a99d5889734b54e8dbf83b42a@epcas1p4.samsung.com>
2017-06-15 10:03     ` [PATCH v2 4/4] ARM: dts: exynos: Remove the display-timing and delay " Hoegeun Kwon

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).