* [RFC PATCH 4/4] video: display: Add Samsung s6e8ax0 display panel driver
From: Tomasz Figa @ 2013-01-30 15:39 UTC (permalink / raw)
To: dri-devel
Cc: linux-fbdev, linux-samsung-soc, kyungmin.park, m.szyprowski,
t.figa, tomasz.figa, Daniel Vetter, Marcus Lorentzon,
Laurent Pinchart, rob, tomi.valkeinen, Vikas Sajjan, inki.dae,
dh09.lee, ville.syrjala, s.nawrocki
In-Reply-To: <1359560343-31636-1-git-send-email-t.figa@samsung.com>
This patch adds Common Display Framework driver for Samsung s6e8ax0
MIPI DSI display panel.
Signed-off-by: Tomasz Figa <t.figa@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
drivers/video/display/Kconfig | 3 +
drivers/video/display/Makefile | 1 +
drivers/video/display/panel-s6e8ax0.c | 1027 +++++++++++++++++++++++++++++++++
include/video/panel-s6e8ax0.h | 41 ++
4 files changed, 1072 insertions(+)
create mode 100644 drivers/video/display/panel-s6e8ax0.c
create mode 100644 include/video/panel-s6e8ax0.h
diff --git a/drivers/video/display/Kconfig b/drivers/video/display/Kconfig
index b14527a..f19ec04 100644
--- a/drivers/video/display/Kconfig
+++ b/drivers/video/display/Kconfig
@@ -5,6 +5,9 @@ menuconfig DISPLAY_CORE
if DISPLAY_CORE
+config DISPLAY_PANEL_S6E8AX0
+ tristate "S6E8AX0 DSI video mode panel"
+ select OF_VIDEOMODE
config DISPLAY_SOURCE_EXYNOS_DSI
tristate "Samsung SoC MIPI DSI Master"
diff --git a/drivers/video/display/Makefile b/drivers/video/display/Makefile
index 40a283a..0f7fdc2 100644
--- a/drivers/video/display/Makefile
+++ b/drivers/video/display/Makefile
@@ -1,2 +1,3 @@
obj-$(CONFIG_DISPLAY_CORE) += display-core.o
+obj-$(CONFIG_DISPLAY_PANEL_S6E8AX0) += panel-s6e8ax0.o
obj-$(CONFIG_DISPLAY_SOURCE_EXYNOS_DSI) += source-exynos_dsi.o
diff --git a/drivers/video/display/panel-s6e8ax0.c b/drivers/video/display/panel-s6e8ax0.c
new file mode 100644
index 0000000..4c09fe2
--- /dev/null
+++ b/drivers/video/display/panel-s6e8ax0.c
@@ -0,0 +1,1027 @@
+/* linux/drivers/video/exynos/s6e8ax0.c
+ *
+ * MIPI-DSI based s6e8ax0 AMOLED lcd 4.65 inch panel driver.
+ *
+ * Inki Dae, <inki.dae@samsung.com>
+ * Donghwa Lee, <dh09.lee@samsung.com>
+ * Tomasz Figa, <t.figa@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 <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/errno.h>
+#include <linux/mutex.h>
+#include <linux/wait.h>
+#include <linux/ctype.h>
+#include <linux/io.h>
+#include <linux/delay.h>
+#include <linux/irq.h>
+#include <linux/interrupt.h>
+
+#include <linux/fb.h>
+#include <linux/gpio.h>
+#include <linux/lcd.h>
+#include <linux/of.h>
+#include <linux/of_gpio.h>
+#include <linux/of_videomode.h>
+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+#include <linux/backlight.h>
+#include <linux/regulator/consumer.h>
+
+#include <video/display.h>
+#include <video/mipi_display.h>
+#include <video/panel-s6e8ax0.h>
+
+#define LDI_MTP_LENGTH 24
+#define DSIM_PM_STABLE_TIME 10
+#define MIN_BRIGHTNESS 0
+#define MAX_BRIGHTNESS 24
+#define GAMMA_TABLE_COUNT 26
+
+struct s6e8ax0 {
+ struct display_entity entity;
+ struct device *dev;
+
+ struct s6e8ax0_platform_data *pdata;
+ struct backlight_device *bd;
+ struct lcd_device *ld;
+ struct regulator_bulk_data supplies[2];
+
+ bool enabled;
+ unsigned int id;
+ unsigned int gamma;
+ unsigned int acl_enable;
+ unsigned int cur_acl;
+ int power;
+
+ unsigned int reset_gpio;
+};
+
+#define to_panel(p) container_of(p, struct s6e8ax0, entity)
+
+static const unsigned char s6e8ax0_22_gamma_30[] = {
+ 0xfa, 0x01, 0x60, 0x10, 0x60, 0xf5, 0x00, 0xff, 0xad, 0xaf,
+ 0xbA, 0xc3, 0xd8, 0xc5, 0x9f, 0xc6, 0x9e, 0xc1, 0xdc, 0xc0,
+ 0x00, 0x61, 0x00, 0x5a, 0x00, 0x74,
+};
+
+static const unsigned char s6e8ax0_22_gamma_50[] = {
+ 0xfa, 0x01, 0x60, 0x10, 0x60, 0xe8, 0x1f, 0xf7, 0xad, 0xc0,
+ 0xb5, 0xc4, 0xdc, 0xc4, 0x9e, 0xc6, 0x9c, 0xbb, 0xd8, 0xbb,
+ 0x00, 0x70, 0x00, 0x68, 0x00, 0x86,
+};
+
+static const unsigned char s6e8ax0_22_gamma_60[] = {
+ 0xfa, 0x01, 0x60, 0x10, 0x60, 0xde, 0x1f, 0xef, 0xad, 0xc4,
+ 0xb3, 0xc3, 0xdd, 0xc4, 0x9e, 0xc6, 0x9c, 0xbc, 0xd6, 0xba,
+ 0x00, 0x75, 0x00, 0x6e, 0x00, 0x8d,
+};
+
+static const unsigned char s6e8ax0_22_gamma_70[] = {
+ 0xfa, 0x01, 0x60, 0x10, 0x60, 0xd8, 0x1f, 0xe7, 0xaf, 0xc8,
+ 0xb4, 0xc4, 0xdd, 0xc3, 0x9d, 0xc6, 0x9c, 0xbb, 0xd6, 0xb9,
+ 0x00, 0x7a, 0x00, 0x72, 0x00, 0x93,
+};
+
+static const unsigned char s6e8ax0_22_gamma_80[] = {
+ 0xfa, 0x01, 0x60, 0x10, 0x60, 0xc9, 0x1f, 0xde, 0xae, 0xc9,
+ 0xb1, 0xc3, 0xdd, 0xc2, 0x9d, 0xc5, 0x9b, 0xbc, 0xd6, 0xbb,
+ 0x00, 0x7f, 0x00, 0x77, 0x00, 0x99,
+};
+
+static const unsigned char s6e8ax0_22_gamma_90[] = {
+ 0xfa, 0x01, 0x60, 0x10, 0x60, 0xc7, 0x1f, 0xd9, 0xb0, 0xcc,
+ 0xb2, 0xc3, 0xdc, 0xc1, 0x9c, 0xc6, 0x9c, 0xbc, 0xd4, 0xb9,
+ 0x00, 0x83, 0x00, 0x7b, 0x00, 0x9e,
+};
+
+static const unsigned char s6e8ax0_22_gamma_100[] = {
+ 0xfa, 0x01, 0x60, 0x10, 0x60, 0xbd, 0x80, 0xcd, 0xba, 0xce,
+ 0xb3, 0xc4, 0xde, 0xc3, 0x9c, 0xc4, 0x9, 0xb8, 0xd3, 0xb6,
+ 0x00, 0x88, 0x00, 0x80, 0x00, 0xa5,
+};
+
+static const unsigned char s6e8ax0_22_gamma_120[] = {
+ 0xfa, 0x01, 0x60, 0x10, 0x60, 0xb9, 0x95, 0xc8, 0xb1, 0xcf,
+ 0xb2, 0xc6, 0xdf, 0xc5, 0x9b, 0xc3, 0x99, 0xb6, 0xd2, 0xb6,
+ 0x00, 0x8f, 0x00, 0x86, 0x00, 0xac,
+};
+
+static const unsigned char s6e8ax0_22_gamma_130[] = {
+ 0xfa, 0x01, 0x60, 0x10, 0x60, 0xb7, 0xa0, 0xc7, 0xb1, 0xd0,
+ 0xb2, 0xc4, 0xdd, 0xc3, 0x9a, 0xc3, 0x98, 0xb6, 0xd0, 0xb4,
+ 0x00, 0x92, 0x00, 0x8a, 0x00, 0xb1,
+};
+
+static const unsigned char s6e8ax0_22_gamma_140[] = {
+ 0xfa, 0x01, 0x60, 0x10, 0x60, 0xb7, 0xa0, 0xc5, 0xb2, 0xd0,
+ 0xb3, 0xc3, 0xde, 0xc3, 0x9b, 0xc2, 0x98, 0xb6, 0xd0, 0xb4,
+ 0x00, 0x95, 0x00, 0x8d, 0x00, 0xb5,
+};
+
+static const unsigned char s6e8ax0_22_gamma_150[] = {
+ 0xfa, 0x01, 0x60, 0x10, 0x60, 0xb3, 0xa0, 0xc2, 0xb2, 0xd0,
+ 0xb2, 0xc1, 0xdd, 0xc2, 0x9b, 0xc2, 0x98, 0xb4, 0xcf, 0xb1,
+ 0x00, 0x99, 0x00, 0x90, 0x00, 0xba,
+};
+
+static const unsigned char s6e8ax0_22_gamma_160[] = {
+ 0xfa, 0x01, 0x60, 0x10, 0x60, 0xaf, 0xa5, 0xbf, 0xb0, 0xd0,
+ 0xb1, 0xc3, 0xde, 0xc2, 0x99, 0xc1, 0x97, 0xb4, 0xce, 0xb1,
+ 0x00, 0x9c, 0x00, 0x93, 0x00, 0xbe,
+};
+
+static const unsigned char s6e8ax0_22_gamma_170[] = {
+ 0xfa, 0x01, 0x60, 0x10, 0x60, 0xaf, 0xb5, 0xbf, 0xb1, 0xd1,
+ 0xb1, 0xc3, 0xde, 0xc3, 0x99, 0xc0, 0x96, 0xb4, 0xce, 0xb1,
+ 0x00, 0x9f, 0x00, 0x96, 0x00, 0xc2,
+};
+
+static const unsigned char s6e8ax0_22_gamma_180[] = {
+ 0xfa, 0x01, 0x60, 0x10, 0x60, 0xaf, 0xb7, 0xbe, 0xb3, 0xd2,
+ 0xb3, 0xc3, 0xde, 0xc2, 0x97, 0xbf, 0x95, 0xb4, 0xcd, 0xb1,
+ 0x00, 0xa2, 0x00, 0x99, 0x00, 0xc5,
+};
+
+static const unsigned char s6e8ax0_22_gamma_190[] = {
+ 0xfa, 0x01, 0x60, 0x10, 0x60, 0xaf, 0xb9, 0xbe, 0xb2, 0xd2,
+ 0xb2, 0xc3, 0xdd, 0xc3, 0x98, 0xbf, 0x95, 0xb2, 0xcc, 0xaf,
+ 0x00, 0xa5, 0x00, 0x9c, 0x00, 0xc9,
+};
+
+static const unsigned char s6e8ax0_22_gamma_200[] = {
+ 0xfa, 0x01, 0x60, 0x10, 0x60, 0xaf, 0xb9, 0xbc, 0xb2, 0xd2,
+ 0xb1, 0xc4, 0xdd, 0xc3, 0x97, 0xbe, 0x95, 0xb1, 0xcb, 0xae,
+ 0x00, 0xa8, 0x00, 0x9f, 0x00, 0xcd,
+};
+
+static const unsigned char s6e8ax0_22_gamma_210[] = {
+ 0xfa, 0x01, 0x60, 0x10, 0x60, 0xb1, 0xc1, 0xbd, 0xb1, 0xd1,
+ 0xb1, 0xc2, 0xde, 0xc2, 0x97, 0xbe, 0x94, 0xB0, 0xc9, 0xad,
+ 0x00, 0xae, 0x00, 0xa4, 0x00, 0xd4,
+};
+
+static const unsigned char s6e8ax0_22_gamma_220[] = {
+ 0xfa, 0x01, 0x60, 0x10, 0x60, 0xb1, 0xc7, 0xbd, 0xb1, 0xd1,
+ 0xb1, 0xc2, 0xdd, 0xc2, 0x97, 0xbd, 0x94, 0xb0, 0xc9, 0xad,
+ 0x00, 0xad, 0x00, 0xa2, 0x00, 0xd3,
+};
+
+static const unsigned char s6e8ax0_22_gamma_230[] = {
+ 0xfa, 0x01, 0x60, 0x10, 0x60, 0xb1, 0xc3, 0xbd, 0xb2, 0xd1,
+ 0xb1, 0xc3, 0xdd, 0xc1, 0x96, 0xbd, 0x94, 0xb0, 0xc9, 0xad,
+ 0x00, 0xb0, 0x00, 0xa7, 0x00, 0xd7,
+};
+
+static const unsigned char s6e8ax0_22_gamma_240[] = {
+ 0xfa, 0x01, 0x60, 0x10, 0x60, 0xb1, 0xcb, 0xbd, 0xb1, 0xd2,
+ 0xb1, 0xc3, 0xdD, 0xc2, 0x95, 0xbd, 0x93, 0xaf, 0xc8, 0xab,
+ 0x00, 0xb3, 0x00, 0xa9, 0x00, 0xdb,
+};
+
+static const unsigned char s6e8ax0_22_gamma_250[] = {
+ 0xfa, 0x01, 0x60, 0x10, 0x60, 0xb3, 0xcc, 0xbe, 0xb0, 0xd2,
+ 0xb0, 0xc3, 0xdD, 0xc2, 0x94, 0xbc, 0x92, 0xae, 0xc8, 0xab,
+ 0x00, 0xb6, 0x00, 0xab, 0x00, 0xde,
+};
+
+static const unsigned char s6e8ax0_22_gamma_260[] = {
+ 0xfa, 0x01, 0x60, 0x10, 0x60, 0xb3, 0xd0, 0xbe, 0xaf, 0xd1,
+ 0xaf, 0xc2, 0xdd, 0xc1, 0x96, 0xbc, 0x93, 0xaf, 0xc8, 0xac,
+ 0x00, 0xb7, 0x00, 0xad, 0x00, 0xe0,
+};
+
+static const unsigned char s6e8ax0_22_gamma_270[] = {
+ 0xfa, 0x01, 0x60, 0x10, 0x60, 0xb2, 0xcF, 0xbd, 0xb0, 0xd2,
+ 0xaf, 0xc2, 0xdc, 0xc1, 0x95, 0xbd, 0x93, 0xae, 0xc6, 0xaa,
+ 0x00, 0xba, 0x00, 0xb0, 0x00, 0xe4,
+};
+
+static const unsigned char s6e8ax0_22_gamma_280[] = {
+ 0xfa, 0x01, 0x60, 0x10, 0x60, 0xb2, 0xd0, 0xbd, 0xaf, 0xd0,
+ 0xad, 0xc4, 0xdd, 0xc3, 0x95, 0xbd, 0x93, 0xac, 0xc5, 0xa9,
+ 0x00, 0xbd, 0x00, 0xb2, 0x00, 0xe7,
+};
+
+static const unsigned char s6e8ax0_22_gamma_300[] = {
+ 0xfa, 0x01, 0x60, 0x10, 0x60, 0xb5, 0xd3, 0xbd, 0xb1, 0xd2,
+ 0xb0, 0xc0, 0xdc, 0xc0, 0x94, 0xba, 0x91, 0xac, 0xc5, 0xa9,
+ 0x00, 0xc2, 0x00, 0xb7, 0x00, 0xed,
+};
+
+static const unsigned char *s6e8ax0_22_gamma_table[] = {
+ s6e8ax0_22_gamma_30,
+ s6e8ax0_22_gamma_50,
+ s6e8ax0_22_gamma_60,
+ s6e8ax0_22_gamma_70,
+ s6e8ax0_22_gamma_80,
+ s6e8ax0_22_gamma_90,
+ s6e8ax0_22_gamma_100,
+ s6e8ax0_22_gamma_120,
+ s6e8ax0_22_gamma_130,
+ s6e8ax0_22_gamma_140,
+ s6e8ax0_22_gamma_150,
+ s6e8ax0_22_gamma_160,
+ s6e8ax0_22_gamma_170,
+ s6e8ax0_22_gamma_180,
+ s6e8ax0_22_gamma_190,
+ s6e8ax0_22_gamma_200,
+ s6e8ax0_22_gamma_210,
+ s6e8ax0_22_gamma_220,
+ s6e8ax0_22_gamma_230,
+ s6e8ax0_22_gamma_240,
+ s6e8ax0_22_gamma_250,
+ s6e8ax0_22_gamma_260,
+ s6e8ax0_22_gamma_270,
+ s6e8ax0_22_gamma_280,
+ s6e8ax0_22_gamma_300,
+};
+
+static void s6e8ax0_panel_cond(struct s6e8ax0 *lcd)
+{
+ static const unsigned char data_to_send[] = {
+ 0xf8, 0x3d, 0x35, 0x00, 0x00, 0x00, 0x93, 0x00, 0x3c, 0x7d,
+ 0x08, 0x27, 0x7d, 0x3f, 0x00, 0x00, 0x00, 0x20, 0x04, 0x08,
+ 0x6e, 0x00, 0x00, 0x00, 0x02, 0x08, 0x08, 0x23, 0x23, 0xc0,
+ 0xc8, 0x08, 0x48, 0xc1, 0x00, 0xc1, 0xff, 0xff, 0xc8
+ };
+ static const unsigned char data_to_send_panel_reverse[] = {
+ 0xf8, 0x19, 0x35, 0x00, 0x00, 0x00, 0x93, 0x00, 0x3c, 0x7d,
+ 0x08, 0x27, 0x7d, 0x3f, 0x00, 0x00, 0x00, 0x20, 0x04, 0x08,
+ 0x6e, 0x00, 0x00, 0x00, 0x02, 0x08, 0x08, 0x23, 0x23, 0xc0,
+ 0xc1, 0x01, 0x41, 0xc1, 0x00, 0xc1, 0xf6, 0xf6, 0xc1
+ };
+
+ if (lcd->pdata->panel_reverse)
+ dsi_dcs_write(lcd->entity.source, 0,
+ data_to_send_panel_reverse,
+ ARRAY_SIZE(data_to_send_panel_reverse));
+ else
+ dsi_dcs_write(lcd->entity.source, 0,
+ data_to_send, ARRAY_SIZE(data_to_send));
+}
+
+static void s6e8ax0_display_cond(struct s6e8ax0 *lcd)
+{
+ static const unsigned char data_to_send[] = {
+ 0xf2, 0x80, 0x03, 0x0d
+ };
+
+ dsi_dcs_write(lcd->entity.source, 0,
+ data_to_send, ARRAY_SIZE(data_to_send));
+}
+
+/* Gamma 2.2 Setting (200cd, 7500K, 10MPCD) */
+static void s6e8ax0_gamma_cond(struct s6e8ax0 *lcd)
+{
+ unsigned int gamma = lcd->bd->props.brightness;
+
+ dsi_dcs_write(lcd->entity.source, 0,
+ s6e8ax0_22_gamma_table[gamma],
+ GAMMA_TABLE_COUNT);
+}
+
+static void s6e8ax0_gamma_update(struct s6e8ax0 *lcd)
+{
+ static const unsigned char data_to_send[] = {
+ 0xf7, 0x03
+ };
+
+ dsi_dcs_write(lcd->entity.source, 0, data_to_send,
+ ARRAY_SIZE(data_to_send));
+}
+
+static void s6e8ax0_etc_cond1(struct s6e8ax0 *lcd)
+{
+ static const unsigned char data_to_send[] = {
+ 0xd1, 0xfe, 0x80, 0x00, 0x01, 0x0b, 0x00, 0x00, 0x40,
+ 0x0d, 0x00, 0x00
+ };
+
+ dsi_dcs_write(lcd->entity.source, 0,
+ data_to_send, ARRAY_SIZE(data_to_send));
+}
+
+static void s6e8ax0_etc_cond2(struct s6e8ax0 *lcd)
+{
+ static const unsigned char data_to_send[] = {
+ 0xb6, 0x0c, 0x02, 0x03, 0x32, 0xff, 0x44, 0x44, 0xc0,
+ 0x00
+ };
+
+ dsi_dcs_write(lcd->entity.source, 0,
+ data_to_send, ARRAY_SIZE(data_to_send));
+}
+
+static void s6e8ax0_etc_cond3(struct s6e8ax0 *lcd)
+{
+ static const unsigned char data_to_send[] = {
+ 0xe1, 0x10, 0x1c, 0x17, 0x08, 0x1d
+ };
+
+ dsi_dcs_write(lcd->entity.source, 0,
+ data_to_send, ARRAY_SIZE(data_to_send));
+}
+
+static void s6e8ax0_etc_cond4(struct s6e8ax0 *lcd)
+{
+ static const unsigned char data_to_send[] = {
+ 0xe2, 0xed, 0x07, 0xc3, 0x13, 0x0d, 0x03
+ };
+
+ dsi_dcs_write(lcd->entity.source, 0,
+ data_to_send, ARRAY_SIZE(data_to_send));
+}
+
+static void s6e8ax0_etc_cond5(struct s6e8ax0 *lcd)
+{
+ static const unsigned char data_to_send[] = {
+ 0xf4, 0xcf, 0x0a, 0x12, 0x10, 0x19, 0x33, 0x02
+ };
+
+ dsi_dcs_write(lcd->entity.source, 0,
+ data_to_send, ARRAY_SIZE(data_to_send));
+}
+static void s6e8ax0_etc_cond6(struct s6e8ax0 *lcd)
+{
+ static const unsigned char data_to_send[] = {
+ 0xe3, 0x40
+ };
+
+ dsi_dcs_write(lcd->entity.source, 0,
+ data_to_send, ARRAY_SIZE(data_to_send));
+}
+
+static void s6e8ax0_etc_cond7(struct s6e8ax0 *lcd)
+{
+ static const unsigned char data_to_send[] = {
+ 0xe4, 0x00, 0x00, 0x14, 0x80, 0x00, 0x00, 0x00
+ };
+
+ dsi_dcs_write(lcd->entity.source, 0,
+ data_to_send, ARRAY_SIZE(data_to_send));
+}
+
+static void s6e8ax0_elvss_set(struct s6e8ax0 *lcd)
+{
+ static const unsigned char data_to_send[] = {
+ 0xb1, 0x04, 0x00
+ };
+
+ dsi_dcs_write(lcd->entity.source, 0,
+ data_to_send, ARRAY_SIZE(data_to_send));
+}
+
+static void s6e8ax0_elvss_nvm_set(struct s6e8ax0 *lcd)
+{
+ static const unsigned char data_to_send[] = {
+ 0xd9, 0x5c, 0x20, 0x0c, 0x0f, 0x41, 0x00, 0x10, 0x11,
+ 0x12, 0xd1, 0x00, 0x00, 0x00, 0x00, 0x80, 0xcb, 0xed,
+ 0x64, 0xaf
+ };
+
+ dsi_dcs_write(lcd->entity.source, 0,
+ data_to_send, ARRAY_SIZE(data_to_send));
+}
+
+static void s6e8ax0_sleep_in(struct s6e8ax0 *lcd)
+{
+ static const unsigned char data_to_send[] = {
+ 0x10
+ };
+
+ dsi_dcs_write(lcd->entity.source, 0,
+ data_to_send, ARRAY_SIZE(data_to_send));
+}
+
+static void s6e8ax0_sleep_out(struct s6e8ax0 *lcd)
+{
+ static const unsigned char data_to_send[] = {
+ 0x11
+ };
+
+ dsi_dcs_write(lcd->entity.source, 0,
+ data_to_send, ARRAY_SIZE(data_to_send));
+}
+
+static void s6e8ax0_display_on(struct s6e8ax0 *lcd)
+{
+ static const unsigned char data_to_send[] = {
+ 0x29
+ };
+
+ dsi_dcs_write(lcd->entity.source, 0,
+ data_to_send, ARRAY_SIZE(data_to_send));
+}
+
+static void s6e8ax0_display_off(struct s6e8ax0 *lcd)
+{
+ static const unsigned char data_to_send[] = {
+ 0x28
+ };
+
+ dsi_dcs_write(lcd->entity.source, 0,
+ data_to_send, ARRAY_SIZE(data_to_send));
+}
+
+static void s6e8ax0_apply_level2_key(struct s6e8ax0 *lcd)
+{
+ static const unsigned char data_to_send[] = {
+ 0xf0, 0x5a, 0x5a
+ };
+
+ dsi_dcs_write(lcd->entity.source, 0,
+ data_to_send, ARRAY_SIZE(data_to_send));
+}
+
+static void s6e8ax0_acl_on(struct s6e8ax0 *lcd)
+{
+ static const unsigned char data_to_send[] = {
+ 0xc0, 0x01
+ };
+
+ dsi_dcs_write(lcd->entity.source, 0,
+ data_to_send, ARRAY_SIZE(data_to_send));
+}
+
+static void s6e8ax0_acl_off(struct s6e8ax0 *lcd)
+{
+ static const unsigned char data_to_send[] = {
+ 0xc0, 0x00
+ };
+
+ dsi_dcs_write(lcd->entity.source, 0,
+ data_to_send, ARRAY_SIZE(data_to_send));
+}
+
+/* Full white 50% reducing setting */
+static void s6e8ax0_acl_ctrl_set(struct s6e8ax0 *lcd)
+{
+ /* Full white 50% reducing setting */
+ static const unsigned char cutoff_50[] = {
+ 0xc1, 0x47, 0x53, 0x13, 0x53, 0x00, 0x00, 0x02, 0xcf,
+ 0x00, 0x00, 0x04, 0xff, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x01, 0x08, 0x0f, 0x16, 0x1d, 0x24, 0x2a, 0x31, 0x38,
+ 0x3f, 0x46
+ };
+ /* Full white 45% reducing setting */
+ static const unsigned char cutoff_45[] = {
+ 0xc1, 0x47, 0x53, 0x13, 0x53, 0x00, 0x00, 0x02, 0xcf,
+ 0x00, 0x00, 0x04, 0xff, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x01, 0x07, 0x0d, 0x13, 0x19, 0x1f, 0x25, 0x2b, 0x31,
+ 0x37, 0x3d
+ };
+ /* Full white 40% reducing setting */
+ static const unsigned char cutoff_40[] = {
+ 0xc1, 0x47, 0x53, 0x13, 0x53, 0x00, 0x00, 0x02, 0xcf,
+ 0x00, 0x00, 0x04, 0xff, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x01, 0x06, 0x0c, 0x11, 0x16, 0x1c, 0x21, 0x26, 0x2b,
+ 0x31, 0x36
+ };
+
+ if (lcd->acl_enable) {
+ if (lcd->cur_acl = 0) {
+ if (lcd->gamma = 0 || lcd->gamma = 1) {
+ s6e8ax0_acl_off(lcd);
+ dev_dbg(lcd->dev,
+ "cur_acl=%d\n", lcd->cur_acl);
+ } else
+ s6e8ax0_acl_on(lcd);
+ }
+ switch (lcd->gamma) {
+ case 0: /* 30cd */
+ s6e8ax0_acl_off(lcd);
+ lcd->cur_acl = 0;
+ break;
+ case 1 ... 3: /* 50cd ~ 90cd */
+ dsi_dcs_write(lcd->entity.source, 0,
+ cutoff_40,
+ ARRAY_SIZE(cutoff_40));
+ lcd->cur_acl = 40;
+ break;
+ case 4 ... 7: /* 120cd ~ 210cd */
+ dsi_dcs_write(lcd->entity.source, 0,
+ cutoff_45,
+ ARRAY_SIZE(cutoff_45));
+ lcd->cur_acl = 45;
+ break;
+ case 8 ... 10: /* 220cd ~ 300cd */
+ dsi_dcs_write(lcd->entity.source, 0,
+ cutoff_50,
+ ARRAY_SIZE(cutoff_50));
+ lcd->cur_acl = 50;
+ break;
+ default:
+ break;
+ }
+ } else {
+ s6e8ax0_acl_off(lcd);
+ lcd->cur_acl = 0;
+ dev_dbg(lcd->dev, "cur_acl = %d\n", lcd->cur_acl);
+ }
+}
+
+static void s6e8ax0_read_id(struct s6e8ax0 *lcd, u8 *mtp_id)
+{
+ unsigned int ret;
+ u8 addr = 0xd1; /* MTP ID */
+
+ ret = dsi_dcs_read(lcd->entity.source, 0, addr, mtp_id, 3);
+}
+
+static int s6e8ax0_panel_init(struct s6e8ax0 *lcd)
+{
+ s6e8ax0_apply_level2_key(lcd);
+ s6e8ax0_sleep_out(lcd);
+ msleep(1);
+ s6e8ax0_panel_cond(lcd);
+ s6e8ax0_display_cond(lcd);
+ s6e8ax0_gamma_cond(lcd);
+ s6e8ax0_gamma_update(lcd);
+
+ s6e8ax0_etc_cond1(lcd);
+ s6e8ax0_etc_cond2(lcd);
+ s6e8ax0_etc_cond3(lcd);
+ s6e8ax0_etc_cond4(lcd);
+ s6e8ax0_etc_cond5(lcd);
+ s6e8ax0_etc_cond6(lcd);
+ s6e8ax0_etc_cond7(lcd);
+
+ s6e8ax0_elvss_nvm_set(lcd);
+ s6e8ax0_elvss_set(lcd);
+
+ s6e8ax0_acl_ctrl_set(lcd);
+ s6e8ax0_acl_on(lcd);
+
+ /* if ID3 value is not 33h, branch private elvss mode */
+ msleep(lcd->pdata->power_on_delay);
+
+ return 0;
+}
+
+static int s6e8ax0_update_gamma_ctrl(struct s6e8ax0 *lcd, int brightness)
+{
+ dsi_dcs_write(lcd->entity.source, 0,
+ s6e8ax0_22_gamma_table[brightness],
+ ARRAY_SIZE(s6e8ax0_22_gamma_table));
+
+ /* update gamma table. */
+ s6e8ax0_gamma_update(lcd);
+ lcd->gamma = brightness;
+
+ return 0;
+}
+
+static int s6e8ax0_gamma_ctrl(struct s6e8ax0 *lcd, int gamma)
+{
+ s6e8ax0_update_gamma_ctrl(lcd, gamma);
+
+ return 0;
+}
+
+static int s6e8ax0_get_brightness(struct backlight_device *bd)
+{
+ return bd->props.brightness;
+}
+
+static int s6e8ax0_set_brightness(struct backlight_device *bd)
+{
+ int ret = 0, brightness = bd->props.brightness;
+ struct s6e8ax0 *lcd = bl_get_data(bd);
+
+ if (brightness < MIN_BRIGHTNESS ||
+ brightness > bd->props.max_brightness) {
+ dev_err(lcd->dev,
+ "lcd brightness should be %d to %d.\n",
+ MIN_BRIGHTNESS, MAX_BRIGHTNESS);
+ return -EINVAL;
+ }
+
+ ret = s6e8ax0_gamma_ctrl(lcd, brightness);
+ if (ret) {
+ dev_err(lcd->dev,
+ "lcd brightness setting failed.\n");
+ return -EIO;
+ }
+
+ return ret;
+}
+
+static const struct backlight_ops s6e8ax0_backlight_ops = {
+ .get_brightness = s6e8ax0_get_brightness,
+ .update_status = s6e8ax0_set_brightness,
+};
+
+static int s6e8ax0_set_power(struct lcd_device *ld, int power)
+{
+ struct s6e8ax0 *lcd = lcd_get_data(ld);
+ enum display_entity_state state;
+ int ret;
+
+ if (power = FB_BLANK_POWERDOWN)
+ state = DISPLAY_ENTITY_STATE_OFF;
+ else if (power != FB_BLANK_UNBLANK)
+ state = DISPLAY_ENTITY_STATE_STANDBY;
+ else
+ state = DISPLAY_ENTITY_STATE_ON;
+
+ ret = display_entity_set_state(&lcd->entity, state);
+ if (ret)
+ return ret;
+
+ lcd->power = power;
+ return 0;
+}
+
+static int s6e8ax0_get_power(struct lcd_device *ld)
+{
+ struct s6e8ax0 *lcd = lcd_get_data(ld);
+
+ return lcd->power;
+}
+
+static struct lcd_ops s6e8ax0_lcd_ops = {
+ .set_power = s6e8ax0_set_power,
+ .get_power = s6e8ax0_get_power,
+};
+
+static void s6e8ax0_set_sequence(struct s6e8ax0 *lcd)
+{
+ u8 mtp_id[3] = {0, };
+
+ msleep(100);
+
+ s6e8ax0_read_id(lcd, mtp_id);
+ if (mtp_id[0] = 0x00)
+ dev_err(lcd->dev, "read id failed\n");
+
+ dev_info(lcd->dev, "Read ID : %x, %x, %x\n",
+ mtp_id[0], mtp_id[1], mtp_id[2]);
+
+ if (mtp_id[2] = 0x33)
+ dev_info(lcd->dev,
+ "ID-3 is 0xff does not support dynamic elvss\n");
+ else
+ dev_info(lcd->dev,
+ "ID-3 is 0x%x support dynamic elvss\n", mtp_id[2]);
+
+ s6e8ax0_panel_init(lcd);
+ s6e8ax0_display_on(lcd);
+}
+
+#ifdef CONFIG_OF
+static int s6e8ax0_generic_reset(struct device *dev)
+{
+ struct s6e8ax0 *lcd = dev_get_drvdata(dev);
+
+ mdelay(10);
+ gpio_set_value(lcd->reset_gpio, 0);
+ mdelay(10);
+ gpio_set_value(lcd->reset_gpio, 1);
+
+ return 0;
+}
+
+static struct s6e8ax0_platform_data *s6e8ax0_parse_dt(struct s6e8ax0 *lcd)
+{
+ struct device_node *node = lcd->dev->of_node;
+ struct s6e8ax0_platform_data *data;
+ const __be32 *prop_data;
+ int ret;
+
+ data = devm_kzalloc(lcd->dev, sizeof(*data), GFP_KERNEL);
+ if (!data) {
+ dev_err(lcd->dev, "failed to allocate platform data.\n");
+ return NULL;
+ }
+
+ ret = of_get_videomode(node, &data->mode, 0);
+ if (ret) {
+ dev_err(lcd->dev, "failed to read video mode from DT\n");
+ return NULL;
+ }
+
+ lcd->reset_gpio = of_get_named_gpio(node, "reset-gpio", 0);
+ if (lcd->reset_gpio < 0)
+ return NULL;
+
+ prop_data = of_get_property(node, "reset-delay", NULL);
+ if (!prop_data)
+ return NULL;
+ data->reset_delay = be32_to_cpu(*prop_data);
+
+ prop_data = of_get_property(node, "power-off-delay", NULL);
+ if (!prop_data)
+ return NULL;
+ data->power_off_delay = be32_to_cpu(*prop_data);
+
+ prop_data = of_get_property(node, "power-on-delay", NULL);
+ if (!prop_data)
+ return NULL;
+ data->power_on_delay = be32_to_cpu(*prop_data);
+
+ data->reset = s6e8ax0_generic_reset;
+
+ return data;
+}
+
+static struct of_device_id s6e8ax0_of_match[] = {
+ { .compatible = "samsung,s6e8ax0" },
+ { }
+};
+
+MODULE_DEVICE_TABLE(of, s6e8ax0_of_match);
+#else
+static struct s6e8ax0_platform_data *s6e8ax0_parse_dt(struct s6e8ax0 *lcd)
+{
+ return NULL;
+}
+#endif
+
+static const struct display_entity_interface_params s6e8ax0_params = {
+ .type = DISPLAY_ENTITY_INTERFACE_DSI,
+ .p.dsi = {
+ .format = DSI_FMT_RGB888,
+ .mode = DSI_MODE_VIDEO | DSI_MODE_VIDEO_BURST
+ | DSI_MODE_VIDEO_HFP | DSI_MODE_VIDEO_HBP
+ | DSI_MODE_VIDEO_HSA | DSI_MODE_EOT_PACKET
+ | DSI_MODE_VSYNC_FLUSH,
+ .data_lanes = 0xf,
+ .hs_clk_freq = 500000000,
+ .esc_clk_freq = 10000000,
+ },
+};
+
+static int s6e8ax0_set_state(struct display_entity *entity,
+ enum display_entity_state state)
+{
+ struct s6e8ax0 *panel = to_panel(entity);
+
+ switch (state) {
+ case DISPLAY_ENTITY_STATE_OFF:
+ if (entity->state = DISPLAY_ENTITY_STATE_ON) {
+ s6e8ax0_sleep_in(panel);
+ panel->entity.source->common_ops->set_stream(entity->source,
+ DISPLAY_ENTITY_STREAM_STOPPED);
+
+ msleep(panel->pdata->power_off_delay);
+ s6e8ax0_display_off(panel);
+
+ panel->entity.source->ops.dsi->disable(entity->source);
+
+ regulator_bulk_disable(ARRAY_SIZE(panel->supplies),
+ panel->supplies);
+ } else if (entity->state = DISPLAY_ENTITY_STATE_STANDBY) {
+ msleep(panel->pdata->power_off_delay);
+ s6e8ax0_display_off(panel);
+
+ panel->entity.source->ops.dsi->disable(entity->source);
+
+ regulator_bulk_disable(ARRAY_SIZE(panel->supplies),
+ panel->supplies);
+ }
+ break;
+
+ case DISPLAY_ENTITY_STATE_STANDBY:
+ if (entity->state = DISPLAY_ENTITY_STATE_ON) {
+ s6e8ax0_sleep_in(panel);
+ panel->entity.source->common_ops->set_stream(entity->source,
+ DISPLAY_ENTITY_STREAM_STOPPED);
+ } else if (entity->state = DISPLAY_ENTITY_STATE_OFF) {
+ msleep(panel->pdata->power_on_delay);
+
+ regulator_bulk_enable(ARRAY_SIZE(panel->supplies),
+ panel->supplies);
+
+ msleep(panel->pdata->reset_delay);
+
+ /* lcd reset */
+ if (panel->pdata->reset)
+ panel->pdata->reset(panel->dev);
+ msleep(5);
+
+ panel->entity.source->ops.dsi->enable(entity->source);
+
+ s6e8ax0_set_sequence(panel);
+ s6e8ax0_sleep_in(panel);
+ }
+ break;
+
+ case DISPLAY_ENTITY_STATE_ON:
+ if (entity->state = DISPLAY_ENTITY_STATE_OFF) {
+ msleep(panel->pdata->power_on_delay);
+
+ regulator_bulk_enable(ARRAY_SIZE(panel->supplies),
+ panel->supplies);
+
+ msleep(panel->pdata->reset_delay);
+
+ /* lcd reset */
+ if (panel->pdata->reset)
+ panel->pdata->reset(panel->dev);
+ msleep(5);
+
+ panel->entity.source->ops.dsi->enable(entity->source);
+
+ s6e8ax0_set_sequence(panel);
+
+ panel->entity.source->common_ops->set_stream(entity->source,
+ DISPLAY_ENTITY_STREAM_CONTINUOUS);
+ } else {
+ panel->entity.source->common_ops->set_stream(entity->source,
+ DISPLAY_ENTITY_STREAM_CONTINUOUS);
+ s6e8ax0_sleep_out(panel);
+ }
+ break;
+
+ default:
+ break;
+ }
+
+ return 0;
+}
+
+static int s6e8ax0_get_modes(struct display_entity *entity,
+ const struct videomode **modes)
+{
+ struct s6e8ax0 *panel = to_panel(entity);
+
+ *modes = &panel->pdata->mode;
+ return 1;
+}
+
+static int s6e8ax0_get_size(struct display_entity *entity,
+ unsigned int *width, unsigned int *height)
+{
+ struct s6e8ax0 *panel = to_panel(entity);
+
+ *width = panel->pdata->width;
+ *height = panel->pdata->height;
+ return 0;
+}
+
+static int s6e8ax0_get_params(struct display_entity *entity,
+ struct display_entity_interface_params *params)
+{
+ *params = s6e8ax0_params;
+ return 0;
+}
+
+static const struct display_entity_control_ops s6e8ax0_control_ops = {
+ .set_state = s6e8ax0_set_state,
+ .get_modes = s6e8ax0_get_modes,
+ .get_size = s6e8ax0_get_size,
+ .get_params = s6e8ax0_get_params,
+};
+
+static void s6e8ax0_release(struct display_entity *entity)
+{
+ struct s6e8ax0 *panel = to_panel(entity);
+
+ backlight_device_unregister(panel->bd);
+ lcd_device_unregister(panel->ld);
+ regulator_bulk_free(ARRAY_SIZE(panel->supplies), panel->supplies);
+ kfree(panel);
+}
+
+static int s6e8ax0_probe(struct platform_device *pdev)
+{
+ struct s6e8ax0 *lcd;
+ int ret;
+
+ lcd = kzalloc(sizeof(struct s6e8ax0), GFP_KERNEL);
+ if (!lcd) {
+ dev_err(&pdev->dev, "failed to allocate s6e8ax0 structure.\n");
+ return -ENOMEM;
+ }
+
+ lcd->dev = &pdev->dev;
+ lcd->pdata = (struct s6e8ax0_platform_data *)pdev->dev.platform_data;
+
+ if (!lcd->pdata) {
+ lcd->pdata = s6e8ax0_parse_dt(lcd);
+ if (!lcd->pdata) {
+ dev_err(&pdev->dev, "failed to find platform data\n");
+ return -ENODEV;
+ }
+ }
+
+ lcd->supplies[0].supply = "vdd3";
+ lcd->supplies[1].supply = "vci";
+ ret = regulator_bulk_get(&pdev->dev,
+ ARRAY_SIZE(lcd->supplies), lcd->supplies);
+ if (ret) {
+ dev_err(&pdev->dev, "Failed to get regulators: %d\n", ret);
+ goto err_regulator_bulk_get;
+ }
+
+ lcd->ld = lcd_device_register("s6e8ax0", &pdev->dev, lcd,
+ &s6e8ax0_lcd_ops);
+ if (IS_ERR(lcd->ld)) {
+ dev_err(&pdev->dev, "failed to register lcd ops.\n");
+ ret = PTR_ERR(lcd->ld);
+ goto err_lcd_register;
+ }
+
+ lcd->bd = backlight_device_register("s6e8ax0-bl", &pdev->dev, lcd,
+ &s6e8ax0_backlight_ops, NULL);
+ if (IS_ERR(lcd->bd)) {
+ dev_err(&pdev->dev, "failed to register backlight ops.\n");
+ ret = PTR_ERR(lcd->bd);
+ goto err_backlight_register;
+ }
+
+ lcd->acl_enable = 1;
+ lcd->cur_acl = 0;
+ lcd->power = FB_BLANK_UNBLANK;
+
+ lcd->bd->props.max_brightness = MAX_BRIGHTNESS;
+ lcd->bd->props.brightness = MAX_BRIGHTNESS;
+
+ lcd->entity.of_node = pdev->dev.of_node;
+ lcd->entity.dev = &pdev->dev;
+ lcd->entity.release = s6e8ax0_release;
+ lcd->entity.ops = &s6e8ax0_control_ops;
+
+ platform_set_drvdata(pdev, lcd);
+
+ ret = display_entity_register(&lcd->entity);
+ if (ret < 0)
+ goto err_display_register;
+
+ display_entity_set_state(&lcd->entity, DISPLAY_ENTITY_STATE_ON);
+
+ dev_dbg(&pdev->dev, "probed s6e8ax0 panel driver.\n");
+
+ return 0;
+
+err_display_register:
+ backlight_device_unregister(lcd->bd);
+err_backlight_register:
+ lcd_device_unregister(lcd->ld);
+err_lcd_register:
+ regulator_bulk_free(ARRAY_SIZE(lcd->supplies), lcd->supplies);
+err_regulator_bulk_get:
+ kfree(lcd);
+
+ return ret;
+}
+
+static int s6e8ax0_remove(struct platform_device *dev)
+{
+ struct s6e8ax0 *lcd = platform_get_drvdata(dev);
+
+ platform_set_drvdata(dev, NULL);
+ display_entity_unregister(&lcd->entity);
+
+ return 0;
+}
+
+static int s6e8ax0_suspend(struct device *dev)
+{
+ struct s6e8ax0 *lcd = dev_get_drvdata(dev);
+
+ if (lcd->power = FB_BLANK_UNBLANK)
+ display_entity_set_state(&lcd->entity,
+ DISPLAY_ENTITY_STATE_OFF);
+
+ return 0;
+}
+
+static int s6e8ax0_resume(struct device *dev)
+{
+ struct s6e8ax0 *lcd = dev_get_drvdata(dev);
+
+ if (lcd->power = FB_BLANK_UNBLANK)
+ display_entity_set_state(&lcd->entity,
+ DISPLAY_ENTITY_STATE_ON);
+
+ return 0;
+}
+
+static struct dev_pm_ops s6e8ax0_pm_ops = {
+ SET_SYSTEM_SLEEP_PM_OPS(s6e8ax0_suspend, s6e8ax0_resume)
+};
+
+static struct platform_driver s6e8ax0_driver = {
+ .probe = s6e8ax0_probe,
+ .remove = s6e8ax0_remove,
+ .driver = {
+ .name = "panel_s6e8ax0",
+ .owner = THIS_MODULE,
+ .of_match_table = of_match_ptr(s6e8ax0_of_match),
+ .pm = &s6e8ax0_pm_ops,
+ },
+};
+module_platform_driver(s6e8ax0_driver);
+
+MODULE_AUTHOR("Donghwa Lee <dh09.lee@samsung.com>");
+MODULE_AUTHOR("Inki Dae <inki.dae@samsung.com>");
+MODULE_AUTHOR("Tomasz Figa <t.figa@samsung.com>");
+MODULE_DESCRIPTION("MIPI-DSI based s6e8ax0 AMOLED LCD Panel Driver");
+MODULE_LICENSE("GPL");
diff --git a/include/video/panel-s6e8ax0.h b/include/video/panel-s6e8ax0.h
new file mode 100644
index 0000000..e522bfb
--- /dev/null
+++ b/include/video/panel-s6e8ax0.h
@@ -0,0 +1,41 @@
+/*
+ * Renesas R61505-based Display Panels
+ *
+ * Copyright (C) 2012 Renesas Solutions Corp.
+ *
+ * Contacts: Laurent Pinchart <laurent.pinchart@ideasonboard.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.
+ */
+
+#ifndef __PANEL_S6E8AX0_H__
+#define __PANEL_S6E8AX0_H__
+
+#include <linux/videomode.h>
+
+struct s6e8ax0_platform_data {
+ unsigned long width; /* Panel width in mm */
+ unsigned long height; /* Panel height in mm */
+ struct videomode mode;
+
+ /* reset lcd panel device. */
+ int (*reset)(struct device *dev);
+
+ /* it indicates whether lcd panel was enabled
+ from bootloader or not. */
+ int lcd_enabled;
+ /* it means delay for stable time when it becomes low to high
+ or high to low that is dependent on whether reset gpio is
+ low active or high active. */
+ unsigned int reset_delay;
+ /* stable time needing to become lcd power on. */
+ unsigned int power_on_delay;
+ /* stable time needing to become lcd power off. */
+ unsigned int power_off_delay;
+ /* panel is reversed */
+ bool panel_reverse;
+};
+
+#endif /* __PANEL_S6E8AX0_H__ */
--
1.8.1
^ permalink raw reply related
* BUG: circular locking dependency detected
From: Russell King @ 2013-01-30 20:04 UTC (permalink / raw)
To: linux-fbdev
This looks like a bug in the framebuffer/console layers. Looks like
we have one path where we call the notifier list, and a called
function takes the console lock, and another path where we hold the
console lock while calling the notifier list.
===========================
[ INFO: possible circular locking dependency detected ]
3.8.0-rc4+ #656 Not tainted
-------------------------------------------------------
kworker/0:1/442 is trying to acquire lock:
((fb_notifier_list).rwsem){.+.+.+}, at: [<c004ea48>] __blocking_notifier_call_chain+0x34/0x68
but task is already holding lock:
(console_lock){+.+.+.}, at: [<c01c2b48>] console_callback+0x14/0x138
which lock already depends on the new lock.
the existing dependency chain (in reverse order) is:
-> #1 (console_lock){+.+.+.}:
[<c006f36c>] __lock_acquire+0x1d20/0x1e80
[<c006fa04>] lock_acquire+0x68/0x7c
[<c002a894>] console_lock+0x5c/0x70
[<c01c0adc>] register_con_driver+0x44/0x150
[<c01c1158>] take_over_console+0x24/0x3b4
[<c019d778>] fbcon_takeover+0x70/0xd4
[<c01a3108>] fbcon_event_notify+0x7c8/0x818
[<c004e538>] notifier_call_chain+0x4c/0x8c
[<c004ea64>] __blocking_notifier_call_chain+0x50/0x68
[<c004ea9c>] blocking_notifier_call_chain+0x20/0x28
[<c0196e70>] fb_notifier_call_chain+0x20/0x28
[<c0198194>] register_framebuffer+0x18c/0x238
[<c01a6148>] clcdfb_probe+0x2b0/0x3c0
[<c01a6da4>] amba_probe+0x88/0xa0
[<c01d1730>] driver_probe_device+0x84/0x218
[<c01d1960>] __driver_attach+0x9c/0xa0
[<c01cfe5c>] bus_for_each_dev+0x5c/0x88
[<c01d1398>] driver_attach+0x20/0x28
[<c01d0ddc>] bus_add_driver+0xa4/0x244
[<c01d1ed4>] driver_register+0x80/0x14c
[<c01a6848>] amba_driver_register+0x48/0x5c
[<c043aae0>] amba_clcdfb_init+0x28/0x3c
[<c000868c>] do_one_initcall+0x44/0x1ac
[<c0425928>] kernel_init_freeable+0x104/0x1c8
[<c03242ec>] kernel_init+0x10/0xec
[<c0014510>] ret_from_fork+0x14/0x24
-> #0 ((fb_notifier_list).rwsem){.+.+.+}:
[<c006bc44>] print_circular_bug+0x84/0x2f0
[<c006f458>] __lock_acquire+0x1e0c/0x1e80
[<c006fa04>] lock_acquire+0x68/0x7c
[<c032b3a0>] down_read+0x34/0x44
[<c004ea48>] __blocking_notifier_call_chain+0x34/0x68
[<c004ea9c>] blocking_notifier_call_chain+0x20/0x28
[<c0196e70>] fb_notifier_call_chain+0x20/0x28
[<c0197690>] fb_blank+0x40/0xac
[<c019f874>] fbcon_blank+0x1f4/0x29c
[<c01c09e0>] do_blank_screen+0x1b8/0x270
[<c01c2ba8>] console_callback+0x74/0x138
[<c00408c8>] process_one_work+0x1b4/0x4ec
[<c0043610>] worker_thread+0x17c/0x4bc
[<c004891c>] kthread+0xb0/0xbc
[<c0014510>] ret_from_fork+0x14/0x24
other info that might help us debug this:
Possible unsafe locking scenario:
CPU0 CPU1
---- ----
lock(console_lock);
lock((fb_notifier_list).rwsem);
lock(console_lock);
lock((fb_notifier_list).rwsem);
*** DEADLOCK ***
3 locks held by kworker/0:1/442:
#0: (events){.+.+..}, at: [<c0040854>] process_one_work+0x140/0x4ec
#1: (console_work){+.+...}, at: [<c0040854>] process_one_work+0x140/0x4ec
#2: (console_lock){+.+.+.}, at: [<c01c2b48>] console_callback+0x14/0x138
stack backtrace:
Backtrace:
[<c00185d8>] (dump_backtrace+0x0/0x10c) from [<c03294c8>] (dump_stack+0x18/0x1c)
r6:c05323f0 r5:c0524800 r4:c05323f0 r3:cf8f6b80
[<c03294b0>] (dump_stack+0x0/0x1c) from [<c006bda4>] (print_circular_bug+0x1e4/0x2f0)
[<c006bbc0>] (print_circular_bug+0x0/0x2f0) from [<c006f458>] (__lock_acquire+0x1e0c/0x1e80)
[<c006d64c>] (__lock_acquire+0x0/0x1e80) from [<c006fa04>] (lock_acquire+0x68/0x7c)
[<c006f99c>] (lock_acquire+0x0/0x7c) from [<c032b3a0>] (down_read+0x34/0x44)
r7:00000010 r6:cfb7dd20 r5:00000002 r4:c04715cc
[<c032b36c>] (down_read+0x0/0x44) from [<c004ea48>] (__blocking_notifier_call_chain+0x34/0x68)
r5:ffffffff r4:c04715cc
[<c004ea14>] (__blocking_notifier_call_chain+0x0/0x68) from [<c004ea9c>] (blocking_notifier_call_chain+0x20/0x28)
r7:cf0ffc00 r6:00000001 r5:cfb7dd20 r4:cfb5f800
[<c004ea7c>] (blocking_notifier_call_chain+0x0/0x28) from [<c0196e70>] (fb_notifier_call_chain+0x20/0x28)
[<c0196e50>] (fb_notifier_call_chain+0x0/0x28) from [<c0197690>] (fb_blank+0x40/0xac)
[<c0197650>] (fb_blank+0x0/0xac) from [<c019f874>] (fbcon_blank+0x1f4/0x29c)
r6:00000001 r5:cf80a000 r4:cfb5f800
[<c019f680>] (fbcon_blank+0x0/0x29c) from [<c01c09e0>] (do_blank_screen+0x1b8/0x270)
[<c01c0828>] (do_blank_screen+0x0/0x270) from [<c01c2ba8>] (console_callback+0x74/0x138)
r7:c0ba8640 r6:c0bac300 r5:c099a51c r4:c099a51c
[<c01c2b34>] (console_callback+0x0/0x138) from [<c00408c8>] (process_one_work+0x1b4/0x4ec)
r6:c0bac300 r5:cf9489c0 r4:c0472e3c r3:c01c2b34
[<c0040714>] (process_one_work+0x0/0x4ec) from [<c0043610>] (worker_thread+0x17c/0x4bc)
[<c0043494>] (worker_thread+0x0/0x4bc) from [<c004891c>] (kthread+0xb0/0xbc)
[<c004886c>] (kthread+0x0/0xbc) from [<c0014510>] (ret_from_fork+0x14/0x24)
r8:00000000 r7:00000000 r6:00000000 r5:c004886c r4:cf84dde8
--
Russell King
^ permalink raw reply
* BUG: circular locking dependency detected
From: Russell King @ 2013-01-30 20:06 UTC (permalink / raw)
To: Florian Tobias Schandinat, linux-fbdev, linux-kernel, akpm
In-Reply-To: <20130130200400.GB31748@flint.arm.linux.org.uk>
This looks like a bug in the framebuffer/console layers. Looks like
we have one path where we call the notifier list, and a called
function takes the console lock, and another path where we hold the
console lock while calling the notifier list.
===========================
[ INFO: possible circular locking dependency detected ]
3.8.0-rc4+ #656 Not tainted
-------------------------------------------------------
kworker/0:1/442 is trying to acquire lock:
((fb_notifier_list).rwsem){.+.+.+}, at: [<c004ea48>] __blocking_notifier_call_chain+0x34/0x68
but task is already holding lock:
(console_lock){+.+.+.}, at: [<c01c2b48>] console_callback+0x14/0x138
which lock already depends on the new lock.
the existing dependency chain (in reverse order) is:
-> #1 (console_lock){+.+.+.}:
[<c006f36c>] __lock_acquire+0x1d20/0x1e80
[<c006fa04>] lock_acquire+0x68/0x7c
[<c002a894>] console_lock+0x5c/0x70
[<c01c0adc>] register_con_driver+0x44/0x150
[<c01c1158>] take_over_console+0x24/0x3b4
[<c019d778>] fbcon_takeover+0x70/0xd4
[<c01a3108>] fbcon_event_notify+0x7c8/0x818
[<c004e538>] notifier_call_chain+0x4c/0x8c
[<c004ea64>] __blocking_notifier_call_chain+0x50/0x68
[<c004ea9c>] blocking_notifier_call_chain+0x20/0x28
[<c0196e70>] fb_notifier_call_chain+0x20/0x28
[<c0198194>] register_framebuffer+0x18c/0x238
[<c01a6148>] clcdfb_probe+0x2b0/0x3c0
[<c01a6da4>] amba_probe+0x88/0xa0
[<c01d1730>] driver_probe_device+0x84/0x218
[<c01d1960>] __driver_attach+0x9c/0xa0
[<c01cfe5c>] bus_for_each_dev+0x5c/0x88
[<c01d1398>] driver_attach+0x20/0x28
[<c01d0ddc>] bus_add_driver+0xa4/0x244
[<c01d1ed4>] driver_register+0x80/0x14c
[<c01a6848>] amba_driver_register+0x48/0x5c
[<c043aae0>] amba_clcdfb_init+0x28/0x3c
[<c000868c>] do_one_initcall+0x44/0x1ac
[<c0425928>] kernel_init_freeable+0x104/0x1c8
[<c03242ec>] kernel_init+0x10/0xec
[<c0014510>] ret_from_fork+0x14/0x24
-> #0 ((fb_notifier_list).rwsem){.+.+.+}:
[<c006bc44>] print_circular_bug+0x84/0x2f0
[<c006f458>] __lock_acquire+0x1e0c/0x1e80
[<c006fa04>] lock_acquire+0x68/0x7c
[<c032b3a0>] down_read+0x34/0x44
[<c004ea48>] __blocking_notifier_call_chain+0x34/0x68
[<c004ea9c>] blocking_notifier_call_chain+0x20/0x28
[<c0196e70>] fb_notifier_call_chain+0x20/0x28
[<c0197690>] fb_blank+0x40/0xac
[<c019f874>] fbcon_blank+0x1f4/0x29c
[<c01c09e0>] do_blank_screen+0x1b8/0x270
[<c01c2ba8>] console_callback+0x74/0x138
[<c00408c8>] process_one_work+0x1b4/0x4ec
[<c0043610>] worker_thread+0x17c/0x4bc
[<c004891c>] kthread+0xb0/0xbc
[<c0014510>] ret_from_fork+0x14/0x24
other info that might help us debug this:
Possible unsafe locking scenario:
CPU0 CPU1
---- ----
lock(console_lock);
lock((fb_notifier_list).rwsem);
lock(console_lock);
lock((fb_notifier_list).rwsem);
*** DEADLOCK ***
3 locks held by kworker/0:1/442:
#0: (events){.+.+..}, at: [<c0040854>] process_one_work+0x140/0x4ec
#1: (console_work){+.+...}, at: [<c0040854>] process_one_work+0x140/0x4ec
#2: (console_lock){+.+.+.}, at: [<c01c2b48>] console_callback+0x14/0x138
stack backtrace:
Backtrace:
[<c00185d8>] (dump_backtrace+0x0/0x10c) from [<c03294c8>] (dump_stack+0x18/0x1c)
r6:c05323f0 r5:c0524800 r4:c05323f0 r3:cf8f6b80
[<c03294b0>] (dump_stack+0x0/0x1c) from [<c006bda4>] (print_circular_bug+0x1e4/0x2f0)
[<c006bbc0>] (print_circular_bug+0x0/0x2f0) from [<c006f458>] (__lock_acquire+0x1e0c/0x1e80)
[<c006d64c>] (__lock_acquire+0x0/0x1e80) from [<c006fa04>] (lock_acquire+0x68/0x7c)
[<c006f99c>] (lock_acquire+0x0/0x7c) from [<c032b3a0>] (down_read+0x34/0x44)
r7:00000010 r6:cfb7dd20 r5:00000002 r4:c04715cc
[<c032b36c>] (down_read+0x0/0x44) from [<c004ea48>] (__blocking_notifier_call_chain+0x34/0x68)
r5:ffffffff r4:c04715cc
[<c004ea14>] (__blocking_notifier_call_chain+0x0/0x68) from [<c004ea9c>] (blocking_notifier_call_chain+0x20/0x28)
r7:cf0ffc00 r6:00000001 r5:cfb7dd20 r4:cfb5f800
[<c004ea7c>] (blocking_notifier_call_chain+0x0/0x28) from [<c0196e70>] (fb_notifier_call_chain+0x20/0x28)
[<c0196e50>] (fb_notifier_call_chain+0x0/0x28) from [<c0197690>] (fb_blank+0x40/0xac)
[<c0197650>] (fb_blank+0x0/0xac) from [<c019f874>] (fbcon_blank+0x1f4/0x29c)
r6:00000001 r5:cf80a000 r4:cfb5f800
[<c019f680>] (fbcon_blank+0x0/0x29c) from [<c01c09e0>] (do_blank_screen+0x1b8/0x270)
[<c01c0828>] (do_blank_screen+0x0/0x270) from [<c01c2ba8>] (console_callback+0x74/0x138)
r7:c0ba8640 r6:c0bac300 r5:c099a51c r4:c099a51c
[<c01c2b34>] (console_callback+0x0/0x138) from [<c00408c8>] (process_one_work+0x1b4/0x4ec)
r6:c0bac300 r5:cf9489c0 r4:c0472e3c r3:c01c2b34
[<c0040714>] (process_one_work+0x0/0x4ec) from [<c0043610>] (worker_thread+0x17c/0x4bc)
[<c0043494>] (worker_thread+0x0/0x4bc) from [<c004891c>] (kthread+0xb0/0xbc)
[<c004886c>] (kthread+0x0/0xbc) from [<c0014510>] (ret_from_fork+0x14/0x24)
r8:00000000 r7:00000000 r6:00000000 r5:c004886c r4:cf84dde8
--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of:
^ permalink raw reply
* Re: [RFC 1/4] video: panel: add CLAA101WA01A panel support
From: Stephen Warren @ 2013-01-30 20:19 UTC (permalink / raw)
To: Mark Zhang
Cc: Alexandre Courbot, Laurent Pinchart, Thierry Reding, Mark Zhang,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
linux-tegra-u79uwXL29TY76Z2rM5mHXA, gnurou-Re5JQEeQqe8AvxtiuMwx3w
In-Reply-To: <5108C9C1.1090707-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
On 01/30/2013 12:20 AM, Mark Zhang wrote:
> On 01/30/2013 11:02 AM, Alexandre Courbot wrote:
>> Add support for the Chunghwa CLAA101WA01A display panel.
>> +static int panel_claa101_get_modes(struct display_entity *entity,
>> + const struct videomode **modes)
>> +{
>> + /* TODO get modes from EDID? */
>
> Why not move the "nvidia,ddc" from encoder's DT to panel's DT? In that
> case, you can get EDID here. I know drm has some helpers to fetch EDID
> but I recall there are some other functions which has no drm
> dependencies which may be suitable for you.
DDC access is a property of the display controller, not the panel
itself. The panel might be hooked up to a display controller's DDC/I2C
channel as the target, but it isn't the host/controller of the DDC/I2C
channel. As such, placing the nvidia,ddc property into the display
controller node makes sense.
Re: the other discussion in this thread: Probably the simplest is if
tegradrm/other-CDF-users do something like:
1) If DDC I2C channel available for this display channel, query DDC.
2) If not, or perhaps also if that fails, query the panel driver for the
mode list.
That would cover all bases very simply.
^ permalink raw reply
* Re: [RFC 1/4] video: panel: add CLAA101WA01A panel support
From: Stephen Warren @ 2013-01-30 20:27 UTC (permalink / raw)
To: Alexandre Courbot
Cc: Laurent Pinchart, Thierry Reding, Mark Zhang,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
linux-tegra-u79uwXL29TY76Z2rM5mHXA, gnurou-Re5JQEeQqe8AvxtiuMwx3w
In-Reply-To: <1359514939-15653-2-git-send-email-acourbot-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
On 01/29/2013 08:02 PM, Alexandre Courbot wrote:
> Add support for the Chunghwa CLAA101WA01A display panel.
> diff --git a/Documentation/devicetree/bindings/video/display/chunghwa,claa101wa01a.txt b/Documentation/devicetree/bindings/video/display/chunghwa,claa101wa01a.txt
> +Chunghwa CLAA101WA01A Display Panel
> +
> +Required properties:
> +- compatible: "chunghwa,claa101wa01a"
> +- pnl-supply: regulator controlling power supply to the panel
> +- bl-supply: regulator controlling power supply to the backlight
> +- pnl-enable-gpios: GPIO that enables the panel
> +- bl-enable-gpios: GPIO that enables the backlight
So this looks like a reasonable binding to me. The one issue is that
it's very generic, and if we go this route, we'll probably end up with
tens or hundreds of identical or extremely similar simple bindings, and
associated drivers.
We can avoid this instead by defining something like a "simple-lcd"
binding, and associated driver, that will work for perhaps 90%, 95%,
99%, even 100%(?) of panels. Just like the above, but with:
compatible="simple-panel", "chunghwa,claa101wa01a"
instead, and the driver binding to "simple-panel" rather than
"chunghwa,claa101wa01a".
The driver can assume that a specific set of supplies (and perhaps
GPIOs) is always present, and that the /sequence/ of manipulating those
is fixed. This will avoid the need for anything like the power sequences
code. If a particular panel doesn't fit those assumptions, including the
exact sequence of manipulations for each state transition (which should
be documented in the binding) then it can get a custom driver, this also
avoiding having to define custom sequences in DT.
Things that might be parameterized/optional:
* Perhaps some GPIOs aren't always present.
* If some regulators aren't SW-controllable, DT should still provide a
fixed/dummy regulator so the driver doesn't care.
* Wait times between regulator/GPIO/... manipulation could be specified
in DT.
* For panels without EDID, CDF DT bindings can provide the list of
supported modes, otherwise we assume that the display controller that
drives the panel has been told how to access the EDID, just like it
would for an "external" display.
^ permalink raw reply
* Re: [RFC 1/4] video: panel: add CLAA101WA01A panel support
From: Stephen Warren @ 2013-01-30 20:30 UTC (permalink / raw)
To: Alexandre Courbot
Cc: Laurent Pinchart, Thierry Reding, Mark Zhang,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
linux-tegra-u79uwXL29TY76Z2rM5mHXA, gnurou-Re5JQEeQqe8AvxtiuMwx3w
In-Reply-To: <1359514939-15653-2-git-send-email-acourbot-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
On 01/29/2013 08:02 PM, Alexandre Courbot wrote:
> Add support for the Chunghwa CLAA101WA01A display panel.
Since this defines a new DT binding, especially since it's for a new
class of device, this patch should be Cc:
devicetree-discuss@lists.ozlabs.org too, next time you post.
^ permalink raw reply
* Re: [PATCH v5 0/8] video: mmp display subsystem
From: Andrew Morton @ 2013-01-30 21:49 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1358410826-2557-1-git-send-email-zzhu3@marvell.com>
On Thu, 17 Jan 2013 16:20:26 +0800
Zhou Zhu <zzhu3@marvell.com> wrote:
> This series added support for display controller in
> Marvell "mmp" series processors.
> It also contains patches to enable display panel on
> TTC_DKB board.
I'll put these into -mm so they get a bit of linux-next exposure.
I'd really prefer not to merge them myself - it's been a few years
since I did fbdev things and I seem to have forgotten most of it :(.
Hopefully Florian or some other fbdev person can comment on this
patchset.
I had a few minorish comments on code details.
^ permalink raw reply
* Re: [PATCH v5 1/8] video: mmp display subsystem
From: Andrew Morton @ 2013-01-30 21:49 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1358410985-2594-1-git-send-email-zzhu3@marvell.com>
On Thu, 17 Jan 2013 16:23:05 +0800
Zhou Zhu <zzhu3@marvell.com> wrote:
> Added mmp display subsystem to support Marvell MMP display controllers.
>
> This subsystem contains 4 parts:
> --fb folder
> --core.c
> --hw folder
> --panel folder
>
> 1. fb folder contains implementation of fb.
> fb get path and ovly from common interface and operates on these structures.
>
> 2. core.c provides common interface for a hardware abstraction.
> Major parts of this interface are:
> a) Path: path is a output device connected to a panel or HDMI TV.
> Main operations of the path is set/get timing/output color.
> fb operates output device through path structure.
> b) Ovly: Ovly is a buffer shown on the path.
> Ovly describes frame buffer and its source/destination size, offset, input
> color, buffer address, z-order, and so on.
> Each fb device maps to one ovly.
>
> 3. hw folder contains implementation of hardware operations defined by core.c.
> It registers paths for fb use.
>
> 4. panel folder contains implementation of panels.
> It's connected to path. Panel drivers would also regiester panels and linked
> to path when probe.
>
>
> ...
>
> +#define list_find(_item, _list, _field, _name)\
> + do {\
> + int found = 0;\
> + list_for_each_entry(_item, &_list, node) {\
> + dev_dbg(_item->dev, "checking %s, target %s",\
> + _item->_field, _name);\
> + if (strcmp(_name, _item->_field) = 0) {\
> + found = 1;\
> + break;\
> + } \
> + } \
> + if (!found)\
> + _item = NULL;\
> + } while (0)
ick.
This could have been implemented as a nice C function, I suspect.
> +/*
> + * panel list is used to pair panel/path when path/panel registered
> + * path list is used for both buffer driver and platdriver
> + * plat driver do path register/unregister
> + * panel driver do panel register/unregister
> + * buffer driver get registered path
> + */
> +static LIST_HEAD(panel_list);
> +static LIST_HEAD(path_list);
> +static DEFINE_MUTEX(disp_lock);
> +
> +int mmp_register_panel(struct mmp_panel *panel)
> +{
> + struct mmp_path *path;
> +
> + mutex_lock(&disp_lock);
> +
> + /* add */
> + list_add_tail(&panel->node, &panel_list);
> +
> + /* try to register to path */
> + list_find(path, path_list, name, panel->plat_path_name);
> + if (path) {
> + dev_info(panel->dev, "register to path %s\n",
> + panel->plat_path_name);
> + path->panel = panel;
> + }
> +
> + mutex_unlock(&disp_lock);
> + return 1;
> +}
> +EXPORT_SYMBOL_GPL(mmp_register_panel);
Should this always return success?
Should it return an error if the registration failed?
This function should return 0 on success or a negative errno on error.
This is a convention we very commonly use.
If this function really can only return success then let's make it a
void returning function.
> +void mmp_unregister_panel(struct mmp_panel *panel)
> +{
> + mutex_lock(&disp_lock);
> + list_del(&panel->node);
> + mutex_unlock(&disp_lock);
> +}
> +EXPORT_SYMBOL_GPL(mmp_unregister_panel);
> +
> +struct mmp_path *mmp_get_path(const char *name)
> +{
> + struct mmp_path *path;
> +
> + mutex_lock(&disp_lock);
> + list_find(path, path_list, name, name);
> + mutex_unlock(&disp_lock);
> +
> + return path;
> +}
> +EXPORT_SYMBOL_GPL(mmp_get_path);
> +
> +struct mmp_path *mmp_register_path(struct mmp_path_info *info)
It's generally desirable to document at least the exported interfaces.
> +{
> + int i, size;
A more appropriate type for `size' is size_t.
> + struct mmp_path *path = NULL;
> + struct mmp_panel *panel;
> +
> + size = sizeof(struct mmp_path)
> + + sizeof(struct mmp_ovly) * info->ovly_num;
> + path = kzalloc(size, GFP_KERNEL);
> + if (!path)
> + goto failed;
> +
> + /* path set */
> + path->ovlys = (void *)path + sizeof(struct mmp_path);
A trick we often use is to put a zero-length array at the end of the
struct. So `struct mmp_path' gets:
struct mmp_ovly overlays[0];
}
added to it. Then you'll find that the mmp_path.ovlys field can just
go away and the code will become shorter and more efficient.
I wish you haven't used "ovly". The kernel has a strong tradition of
spelling things out in full and avoiding the weird abbreviations. Just
use "overlay". The code becomes mroe readable and there's never a
problem remembering the name of the identifiers.
> + mutex_init(&path->access_ok);
> + path->dev = info->dev;
> + path->id = info->id;
> + path->name = info->name;
> + path->output_type = info->output_type;
> + path->ovly_num = info->ovly_num;
> + path->plat_data = info->plat_data;
> + path->ops.set_mode = info->set_mode;
>
> ...
>
^ permalink raw reply
* Re: [PATCH v5 2/8] video: mmp fb support
From: Andrew Morton @ 2013-01-30 21:49 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1358410998-2631-1-git-send-email-zzhu3@marvell.com>
On Thu, 17 Jan 2013 16:23:18 +0800
Zhou Zhu <zzhu3@marvell.com> wrote:
> Added fb support for Marvell mmp display subsystem.
> This driver is configured using "buffer driver mach info".
> With configured name of path, this driver get path using
> using exported interface of mmp display driver.
> Then this driver get ovly using configured id and operates
> on this ovly to show buffers on display devices.
>
> ...
>
> +static void *alloc_framebuffer(size_t size, dma_addr_t *dma)
> +{
> + int nr, i = 0;
> + struct page **pages;
> + void *start;
> +
> + nr = size >> PAGE_SHIFT;
If `size' is not a multiple of PAGE_SIZE, this will cause various
overruns later on.
> + start = alloc_pages_exact(size, GFP_KERNEL | __GFP_ZERO);
> + if (start = NULL)
> + return NULL;
> +
> + *dma = virt_to_phys(start);
> + pages = vmalloc(sizeof(struct page *) * nr);
> + if (pages = NULL)
> + return NULL;
Did we leak the memory at `start'?
> +
> + while (i < nr) {
> + pages[i] = phys_to_page(*dma + (i << PAGE_SHIFT));
> + i++;
> + }
> + start = vmap(pages, nr, 0, pgprot_writecombine(pgprot_kernel));
> +
> + vfree(pages);
> + return start;
> +}
>
> ...
>
> +static int mmpfb_set_par(struct fb_info *info)
> +{
> + struct mmpfb_info *fbi = info->par;
> + struct fb_var_screeninfo *var = &info->var;
> + struct mmp_addr addr;
> + struct mmp_win win;
> + struct mmp_mode mode;
> +
> + int ret = var_update(info);
Strange code layout here. I suggest:
struct mmp_win win;
struct mmp_mode mode;
int ret;
ret = var_update(info);
> + if (ret != 0)
> + return ret;
> +
> + /* set window/path according to new videomode */
> + fbmode_to_mmpmode(&mode, &fbi->mode, fbi->output_fmt);
> + mmp_path_set_mode(fbi->path, &mode);
> +
> + memset(&win, 0, sizeof(win));
> + win.xsrc = win.xdst = fbi->mode.xres;
> + win.ysrc = win.ydst = fbi->mode.yres;
> + win.pix_fmt = fbi->pix_fmt;
> + mmp_ovly_set_win(fbi->ovly, &win);
> +
> + /* set address always */
> + memset(&addr, 0, sizeof(addr));
> + addr.phys[0] = (var->yoffset * var->xres_virtual + var->xoffset)
> + * var->bits_per_pixel / 8 + fbi->fb_start_dma;
> + mmp_ovly_set_addr(fbi->ovly, &addr);
> +
> + return 0;
> +}
>
> ...
>
^ permalink raw reply
* Re: [PATCH v5 3/8] video: mmp display controller support
From: Andrew Morton @ 2013-01-30 21:49 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1358411023-2667-1-git-send-email-zzhu3@marvell.com>
On Thu, 17 Jan 2013 16:23:43 +0800
Zhou Zhu <zzhu3@marvell.com> wrote:
> From: Guoqing Li <ligq@marvell.com>
>
> Marvell mmp series display controller support in mmpdisp subsystem.
> This driver focus on implementation of hardware operations of path/ovly,
> which is defined in mmp display subsystem interface.
> This driver registers all pathes to mmp display framework.
>
> ...
>
> +#define ovly_is_vid(ovly) (ovly->dmafetch_id % 2)
> +#define path_to_path_plat(path) \
> + ((struct mmphw_path_plat *)path->plat_data)
> +#define ovly_to_ctrl(ovly) \
> + (path_to_ctrl(ovly->path))
> +#define path_to_ctrl(path) \
> + (path_to_path_plat(path)->ctrl)
> +#define ctrl_regs(path) \
> + (path_to_ctrl(path)->reg_base)
These could be implemented in C rather than in CPP. The result would
be improved typechecking and improved readability.
>
> ...
>
^ permalink raw reply
* Re: BUG: circular locking dependency detected
From: Russell King @ 2013-01-30 21:52 UTC (permalink / raw)
To: Florian Tobias Schandinat, linux-fbdev, linux-kernel, akpm,
Daniel Vetter, Greg Kroah-Hartman
In-Reply-To: <20130130200648.GA13298@flint.arm.linux.org.uk>
Also adding Greg and Daniel to this as Daniel introduced the lockdep
checking.
This looks extremely horrid to be to solve - the paths are rather deep
where the dependency occurs. The two paths between the locks are:
console_lock+0x5c/0x70
register_con_driver+0x44/0x150
take_over_console+0x24/0x3b4
fbcon_takeover+0x70/0xd4
fbcon_event_notify+0x7c8/0x818
notifier_call_chain+0x4c/0x8c
__blocking_notifier_call_chain+0x50/0x68
blocking_notifier_call_chain+0x20/0x28
and
__blocking_notifier_call_chain+0x34/0x68
blocking_notifier_call_chain+0x20/0x28
fb_notifier_call_chain+0x20/0x28
fb_blank+0x40/0xac
fbcon_blank+0x1f4/0x29c
do_blank_screen+0x1b8/0x270
console_callback+0x74/0x138
On Wed, Jan 30, 2013 at 08:06:48PM +0000, Russell King wrote:
> This looks like a bug in the framebuffer/console layers. Looks like
> we have one path where we call the notifier list, and a called
> function takes the console lock, and another path where we hold the
> console lock while calling the notifier list.
>
> ===========================
> [ INFO: possible circular locking dependency detected ]
> 3.8.0-rc4+ #656 Not tainted
> -------------------------------------------------------
> kworker/0:1/442 is trying to acquire lock:
> ((fb_notifier_list).rwsem){.+.+.+}, at: [<c004ea48>] __blocking_notifier_call_chain+0x34/0x68
>
> but task is already holding lock:
> (console_lock){+.+.+.}, at: [<c01c2b48>] console_callback+0x14/0x138
>
> which lock already depends on the new lock.
>
>
> the existing dependency chain (in reverse order) is:
>
> -> #1 (console_lock){+.+.+.}:
> [<c006f36c>] __lock_acquire+0x1d20/0x1e80
> [<c006fa04>] lock_acquire+0x68/0x7c
> [<c002a894>] console_lock+0x5c/0x70
> [<c01c0adc>] register_con_driver+0x44/0x150
> [<c01c1158>] take_over_console+0x24/0x3b4
> [<c019d778>] fbcon_takeover+0x70/0xd4
> [<c01a3108>] fbcon_event_notify+0x7c8/0x818
> [<c004e538>] notifier_call_chain+0x4c/0x8c
> [<c004ea64>] __blocking_notifier_call_chain+0x50/0x68
> [<c004ea9c>] blocking_notifier_call_chain+0x20/0x28
> [<c0196e70>] fb_notifier_call_chain+0x20/0x28
> [<c0198194>] register_framebuffer+0x18c/0x238
> [<c01a6148>] clcdfb_probe+0x2b0/0x3c0
> [<c01a6da4>] amba_probe+0x88/0xa0
> [<c01d1730>] driver_probe_device+0x84/0x218
> [<c01d1960>] __driver_attach+0x9c/0xa0
> [<c01cfe5c>] bus_for_each_dev+0x5c/0x88
> [<c01d1398>] driver_attach+0x20/0x28
> [<c01d0ddc>] bus_add_driver+0xa4/0x244
> [<c01d1ed4>] driver_register+0x80/0x14c
> [<c01a6848>] amba_driver_register+0x48/0x5c
> [<c043aae0>] amba_clcdfb_init+0x28/0x3c
> [<c000868c>] do_one_initcall+0x44/0x1ac
> [<c0425928>] kernel_init_freeable+0x104/0x1c8
> [<c03242ec>] kernel_init+0x10/0xec
> [<c0014510>] ret_from_fork+0x14/0x24
>
> -> #0 ((fb_notifier_list).rwsem){.+.+.+}:
> [<c006bc44>] print_circular_bug+0x84/0x2f0
> [<c006f458>] __lock_acquire+0x1e0c/0x1e80
> [<c006fa04>] lock_acquire+0x68/0x7c
> [<c032b3a0>] down_read+0x34/0x44
> [<c004ea48>] __blocking_notifier_call_chain+0x34/0x68
> [<c004ea9c>] blocking_notifier_call_chain+0x20/0x28
> [<c0196e70>] fb_notifier_call_chain+0x20/0x28
> [<c0197690>] fb_blank+0x40/0xac
> [<c019f874>] fbcon_blank+0x1f4/0x29c
> [<c01c09e0>] do_blank_screen+0x1b8/0x270
> [<c01c2ba8>] console_callback+0x74/0x138
> [<c00408c8>] process_one_work+0x1b4/0x4ec
> [<c0043610>] worker_thread+0x17c/0x4bc
> [<c004891c>] kthread+0xb0/0xbc
> [<c0014510>] ret_from_fork+0x14/0x24
>
> other info that might help us debug this:
>
> Possible unsafe locking scenario:
>
> CPU0 CPU1
> ---- ----
> lock(console_lock);
> lock((fb_notifier_list).rwsem);
> lock(console_lock);
> lock((fb_notifier_list).rwsem);
>
> *** DEADLOCK ***
>
> 3 locks held by kworker/0:1/442:
> #0: (events){.+.+..}, at: [<c0040854>] process_one_work+0x140/0x4ec
> #1: (console_work){+.+...}, at: [<c0040854>] process_one_work+0x140/0x4ec
> #2: (console_lock){+.+.+.}, at: [<c01c2b48>] console_callback+0x14/0x138
>
> stack backtrace:
> Backtrace:
> [<c00185d8>] (dump_backtrace+0x0/0x10c) from [<c03294c8>] (dump_stack+0x18/0x1c)
> r6:c05323f0 r5:c0524800 r4:c05323f0 r3:cf8f6b80
> [<c03294b0>] (dump_stack+0x0/0x1c) from [<c006bda4>] (print_circular_bug+0x1e4/0x2f0)
> [<c006bbc0>] (print_circular_bug+0x0/0x2f0) from [<c006f458>] (__lock_acquire+0x1e0c/0x1e80)
> [<c006d64c>] (__lock_acquire+0x0/0x1e80) from [<c006fa04>] (lock_acquire+0x68/0x7c)
> [<c006f99c>] (lock_acquire+0x0/0x7c) from [<c032b3a0>] (down_read+0x34/0x44)
> r7:00000010 r6:cfb7dd20 r5:00000002 r4:c04715cc
> [<c032b36c>] (down_read+0x0/0x44) from [<c004ea48>] (__blocking_notifier_call_chain+0x34/0x68)
> r5:ffffffff r4:c04715cc
> [<c004ea14>] (__blocking_notifier_call_chain+0x0/0x68) from [<c004ea9c>] (blocking_notifier_call_chain+0x20/0x28)
> r7:cf0ffc00 r6:00000001 r5:cfb7dd20 r4:cfb5f800
> [<c004ea7c>] (blocking_notifier_call_chain+0x0/0x28) from [<c0196e70>] (fb_notifier_call_chain+0x20/0x28)
> [<c0196e50>] (fb_notifier_call_chain+0x0/0x28) from [<c0197690>] (fb_blank+0x40/0xac)
> [<c0197650>] (fb_blank+0x0/0xac) from [<c019f874>] (fbcon_blank+0x1f4/0x29c)
> r6:00000001 r5:cf80a000 r4:cfb5f800
> [<c019f680>] (fbcon_blank+0x0/0x29c) from [<c01c09e0>] (do_blank_screen+0x1b8/0x270)
> [<c01c0828>] (do_blank_screen+0x0/0x270) from [<c01c2ba8>] (console_callback+0x74/0x138)
> r7:c0ba8640 r6:c0bac300 r5:c099a51c r4:c099a51c
> [<c01c2b34>] (console_callback+0x0/0x138) from [<c00408c8>] (process_one_work+0x1b4/0x4ec)
> r6:c0bac300 r5:cf9489c0 r4:c0472e3c r3:c01c2b34
> [<c0040714>] (process_one_work+0x0/0x4ec) from [<c0043610>] (worker_thread+0x17c/0x4bc)
> [<c0043494>] (worker_thread+0x0/0x4bc) from [<c004891c>] (kthread+0xb0/0xbc)
> [<c004886c>] (kthread+0x0/0xbc) from [<c0014510>] (ret_from_fork+0x14/0x24)
> r8:00000000 r7:00000000 r6:00000000 r5:c004886c r4:cf84dde8
>
> --
> Russell King
> Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
> maintainer of:
--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of:
^ permalink raw reply
* Re: BUG: circular locking dependency detected
From: Daniel Vetter @ 2013-01-30 22:07 UTC (permalink / raw)
To: Russell King
Cc: Florian Tobias Schandinat, linux-fbdev, linux-kernel, akpm,
Greg Kroah-Hartman, dri-devel, Dave Airlie
In-Reply-To: <20130130215211.GA28418@flint.arm.linux.org.uk>
On Wed, Jan 30, 2013 at 10:52 PM, Russell King <rmk@arm.linux.org.uk> wrote:
> Also adding Greg and Daniel to this as Daniel introduced the lockdep
> checking.
>
> This looks extremely horrid to be to solve - the paths are rather deep
> where the dependency occurs. The two paths between the locks are:
>
> console_lock+0x5c/0x70
> register_con_driver+0x44/0x150
> take_over_console+0x24/0x3b4
> fbcon_takeover+0x70/0xd4
> fbcon_event_notify+0x7c8/0x818
> notifier_call_chain+0x4c/0x8c
> __blocking_notifier_call_chain+0x50/0x68
> blocking_notifier_call_chain+0x20/0x28
>
> and
>
> __blocking_notifier_call_chain+0x34/0x68
> blocking_notifier_call_chain+0x20/0x28
> fb_notifier_call_chain+0x20/0x28
> fb_blank+0x40/0xac
> fbcon_blank+0x1f4/0x29c
> do_blank_screen+0x1b8/0x270
> console_callback+0x74/0x138
You want Dave Airlie's pile of locking reworks, which fixes all
currently known offenders around console_lock and fb_notifier. Patches
won't go into 3.9 since it took a few rounds until they did not cause
regression by making these deadlocks easier to hit.
http://cgit.freedesktop.org/~airlied/linux/log/?hûcon-locking-fixes
Long term solution would be to abolish the fb_notifier, at least for
the purpose of linking fbdevs up with the fbcon and just replace those
with direct function calls. But that requires that we no longer allow
fbdev drivers and the fbcon to be loaded in any arbitrary order. Or
just force fbcon to be built-in if enabled, imo the sane choice (no
one's bothering with config_vt=m either, after all).
Cheers, Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
^ permalink raw reply
* Re: BUG: circular locking dependency detected
From: Russell King @ 2013-01-30 22:19 UTC (permalink / raw)
To: Daniel Vetter, Linus Torvalds
Cc: Florian Tobias Schandinat, linux-fbdev, linux-kernel, akpm,
Greg Kroah-Hartman, dri-devel, Dave Airlie
In-Reply-To: <CAKMK7uHEggaGfLTq4c3MfY7csQSFpb=kRwdbJUQMN9Cz9K=V4A@mail.gmail.com>
On Wed, Jan 30, 2013 at 11:07:16PM +0100, Daniel Vetter wrote:
> On Wed, Jan 30, 2013 at 10:52 PM, Russell King <rmk@arm.linux.org.uk> wrote:
> > Also adding Greg and Daniel to this as Daniel introduced the lockdep
> > checking.
> >
> > This looks extremely horrid to be to solve - the paths are rather deep
> > where the dependency occurs. The two paths between the locks are:
> >
> > console_lock+0x5c/0x70
> > register_con_driver+0x44/0x150
> > take_over_console+0x24/0x3b4
> > fbcon_takeover+0x70/0xd4
> > fbcon_event_notify+0x7c8/0x818
> > notifier_call_chain+0x4c/0x8c
> > __blocking_notifier_call_chain+0x50/0x68
> > blocking_notifier_call_chain+0x20/0x28
> >
> > and
> >
> > __blocking_notifier_call_chain+0x34/0x68
> > blocking_notifier_call_chain+0x20/0x28
> > fb_notifier_call_chain+0x20/0x28
> > fb_blank+0x40/0xac
> > fbcon_blank+0x1f4/0x29c
> > do_blank_screen+0x1b8/0x270
> > console_callback+0x74/0x138
>
> You want Dave Airlie's pile of locking reworks, which fixes all
> currently known offenders around console_lock and fb_notifier. Patches
> won't go into 3.9 since it took a few rounds until they did not cause
> regression by making these deadlocks easier to hit.
>
> http://cgit.freedesktop.org/~airlied/linux/log/?hûcon-locking-fixes
>
> Long term solution would be to abolish the fb_notifier, at least for
> the purpose of linking fbdevs up with the fbcon and just replace those
> with direct function calls. But that requires that we no longer allow
> fbdev drivers and the fbcon to be loaded in any arbitrary order. Or
> just force fbcon to be built-in if enabled, imo the sane choice (no
> one's bothering with config_vt=m either, after all).
So... what you seem to be telling me is that 3.9 is going to be a
release which issues lockdep complaints when the console blanks, and
you think that's acceptable?
Adding Linus and Andrew so they're aware of this issue...
--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of:
^ permalink raw reply
* Re: BUG: circular locking dependency detected
From: Daniel Vetter @ 2013-01-30 22:27 UTC (permalink / raw)
To: Russell King
Cc: Linus Torvalds, Andrew Morton, Florian Tobias Schandinat,
linux-fbdev, linux-kernel, Greg Kroah-Hartman, dri-devel,
Dave Airlie
In-Reply-To: <20130130221946.GA14801@flint.arm.linux.org.uk>
On Wed, Jan 30, 2013 at 11:19 PM, Russell King <rmk@arm.linux.org.uk> wrote:
> So... what you seem to be telling me is that 3.9 is going to be a
> release which issues lockdep complaints when the console blanks, and
> you think that's acceptable?
>
> Adding Linus and Andrew so they're aware of this issue...
Linus was the guy who shot down the patches for 3.9 since one of the
earlier iterations caused instant deadlocks - I've been pushing them
to merge them ;-)
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
^ permalink raw reply
* Re: BUG: circular locking dependency detected
From: Linus Torvalds @ 2013-01-30 23:52 UTC (permalink / raw)
To: Russell King
Cc: Daniel Vetter, Andrew Morton, Florian Tobias Schandinat,
linux-fbdev@vger.kernel.org, Linux Kernel Mailing List,
Greg Kroah-Hartman, dri-devel, Dave Airlie
In-Reply-To: <20130130221946.GA14801@flint.arm.linux.org.uk>
On Thu, Jan 31, 2013 at 9:19 AM, Russell King <rmk@arm.linux.org.uk> wrote:
>
> So... what you seem to be telling me is that 3.9 is going to be a
> release which issues lockdep complaints when the console blanks, and
> you think that's acceptable?
>
> Adding Linus and Andrew so they're aware of this issue...
Oh, we're extremely aware of it. And it's not a new issue, the locking
problem have apparently been around forever, although I'm not sure why
the lockdep splat itself started happening only recently.
They'll make it into 3.9, it's 3.8 that won't have them. The patches
initially caused way *worse* behavior than just a lockdep splat - they
caused actual hard lockups (and that was *after* the initial series of
fixes). That got fixed (hopefully for the last case!) fairly recently,
and I'm not willing to take the scary patch-series that has had
several problem cases.
LInus
^ permalink raw reply
* Re: BUG: circular locking dependency detected
From: Dave Airlie @ 2013-01-31 0:04 UTC (permalink / raw)
To: Linus Torvalds
Cc: Russell King, Daniel Vetter, Andrew Morton,
Florian Tobias Schandinat, linux-fbdev@vger.kernel.org,
Linux Kernel Mailing List, Greg Kroah-Hartman, dri-devel
In-Reply-To: <CA+55aFyRHK3dmwQf_mdfB18dmoaRZqBG9FvHRqTs7cqBjgUs+g@mail.gmail.com>
On Thu, Jan 31, 2013 at 9:52 AM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
> On Thu, Jan 31, 2013 at 9:19 AM, Russell King <rmk@arm.linux.org.uk> wrote:
>>
>> So... what you seem to be telling me is that 3.9 is going to be a
>> release which issues lockdep complaints when the console blanks, and
>> you think that's acceptable?
>>
>> Adding Linus and Andrew so they're aware of this issue...
>
> Oh, we're extremely aware of it. And it's not a new issue, the locking
> problem have apparently been around forever, although I'm not sure why
> the lockdep splat itself started happening only recently.
>
> They'll make it into 3.9, it's 3.8 that won't have them. The patches
> initially caused way *worse* behavior than just a lockdep splat - they
> caused actual hard lockups (and that was *after* the initial series of
> fixes). That got fixed (hopefully for the last case!) fairly recently,
> and I'm not willing to take the scary patch-series that has had
> several problem cases.
Well we didn't have any lock validation support before Daniel added it
a couple of kernels back,
so instead of hidden locking problems we've had from time began, we now have
lockdep detectable locking problems.
Dave.
^ permalink raw reply
* Re: BUG: circular locking dependency detected
From: Russell King @ 2013-01-31 0:09 UTC (permalink / raw)
To: Linus Torvalds
Cc: Daniel Vetter, Andrew Morton, Florian Tobias Schandinat,
linux-fbdev@vger.kernel.org, Linux Kernel Mailing List,
Greg Kroah-Hartman, dri-devel, Dave Airlie
In-Reply-To: <CA+55aFyRHK3dmwQf_mdfB18dmoaRZqBG9FvHRqTs7cqBjgUs+g@mail.gmail.com>
On Thu, Jan 31, 2013 at 10:52:51AM +1100, Linus Torvalds wrote:
> On Thu, Jan 31, 2013 at 9:19 AM, Russell King <rmk@arm.linux.org.uk> wrote:
> >
> > So... what you seem to be telling me is that 3.9 is going to be a
> > release which issues lockdep complaints when the console blanks, and
> > you think that's acceptable?
> >
> > Adding Linus and Andrew so they're aware of this issue...
>
> Oh, we're extremely aware of it. And it's not a new issue, the locking
> problem have apparently been around forever, although I'm not sure why
> the lockdep splat itself started happening only recently.
Well, the reason the splat started happening recently is because of:
commit daee779718a319ff9f83e1ba3339334ac650bb22
Author: Daniel Vetter <daniel.vetter@ffwll.ch>
Date: Sat Sep 22 19:52:11 2012 +0200
console: implement lockdep support for console_lock
Dave Airlie recently discovered a locking bug in the fbcon layer,
where a timer_del_sync (for the blinking cursor) deadlocks with the
timer itself, since both (want to) hold the console_lock:
https://lkml.org/lkml/2012/8/21/36
which, if I'm looking at the git history right, appears to have come
in during the last merge window?
Yes, the locking may be wrong, but we've lived with that locking for
a long time without problem.
Can we at least silence these warnings by temporarily disabling the
lockdep tracking added by the above commit for this lock, until the
fixes for this are merged during the next merge window?
--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of:
^ permalink raw reply
* Re: BUG: circular locking dependency detected
From: Russell King @ 2013-01-31 0:13 UTC (permalink / raw)
To: Dave Airlie
Cc: Linus Torvalds, Daniel Vetter, Andrew Morton,
Florian Tobias Schandinat, linux-fbdev@vger.kernel.org,
Linux Kernel Mailing List, Greg Kroah-Hartman, dri-devel
In-Reply-To: <CAPM=9tz6YBh2o3T8Ogt4K=wxSrC3Jc4PCJMzfpbyY=B132fA5g@mail.gmail.com>
On Thu, Jan 31, 2013 at 10:04:05AM +1000, Dave Airlie wrote:
> On Thu, Jan 31, 2013 at 9:52 AM, Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
> > On Thu, Jan 31, 2013 at 9:19 AM, Russell King <rmk@arm.linux.org.uk> wrote:
> >>
> >> So... what you seem to be telling me is that 3.9 is going to be a
> >> release which issues lockdep complaints when the console blanks, and
> >> you think that's acceptable?
> >>
> >> Adding Linus and Andrew so they're aware of this issue...
> >
> > Oh, we're extremely aware of it. And it's not a new issue, the locking
> > problem have apparently been around forever, although I'm not sure why
> > the lockdep splat itself started happening only recently.
> >
> > They'll make it into 3.9, it's 3.8 that won't have them. The patches
> > initially caused way *worse* behavior than just a lockdep splat - they
> > caused actual hard lockups (and that was *after* the initial series of
> > fixes). That got fixed (hopefully for the last case!) fairly recently,
> > and I'm not willing to take the scary patch-series that has had
> > several problem cases.
>
> Well we didn't have any lock validation support before Daniel added it
> a couple of kernels back,
> so instead of hidden locking problems we've had from time began, we now have
> lockdep detectable locking problems.
Which may or may not be a good thing depending how you look at it; it
means that once your kernel blanks, you get a lockdep dump. At that
point you lose lockdep checking for everything else because lockdep
disables itself after the first dump.
--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of:
^ permalink raw reply
* Re: BUG: circular locking dependency detected
From: Linus Torvalds @ 2013-01-31 0:26 UTC (permalink / raw)
To: Russell King
Cc: Dave Airlie, Daniel Vetter, Andrew Morton,
Florian Tobias Schandinat, linux-fbdev@vger.kernel.org,
Linux Kernel Mailing List, Greg Kroah-Hartman, dri-devel
In-Reply-To: <20130131001339.GC14801@flint.arm.linux.org.uk>
On Thu, Jan 31, 2013 at 11:13 AM, Russell King <rmk@arm.linux.org.uk> wrote:
>
> Which may or may not be a good thing depending how you look at it; it
> means that once your kernel blanks, you get a lockdep dump. At that
> point you lose lockdep checking for everything else because lockdep
> disables itself after the first dump.
Fair enough, we may want to revert the lockdep checking for
console_lock, and make re-enabling it part of the patch-series that
fixes the locking.
Daniel/Dave? Does that sound reasonable?
Linus
^ permalink raw reply
* Re: [RFC 1/4] video: panel: add CLAA101WA01A panel support
From: Mark Zhang @ 2013-01-31 3:51 UTC (permalink / raw)
To: Stephen Warren
Cc: Alexandre Courbot, Laurent Pinchart, Thierry Reding, Mark Zhang,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
linux-tegra-u79uwXL29TY76Z2rM5mHXA, gnurou-Re5JQEeQqe8AvxtiuMwx3w
In-Reply-To: <51098064.7030902-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
On 01/31/2013 04:19 AM, Stephen Warren wrote:
> On 01/30/2013 12:20 AM, Mark Zhang wrote:
>> On 01/30/2013 11:02 AM, Alexandre Courbot wrote:
>>> Add support for the Chunghwa CLAA101WA01A display panel.
>
>>> +static int panel_claa101_get_modes(struct display_entity *entity,
>>> + const struct videomode **modes)
>>> +{
>>> + /* TODO get modes from EDID? */
>>
>> Why not move the "nvidia,ddc" from encoder's DT to panel's DT? In that
>> case, you can get EDID here. I know drm has some helpers to fetch EDID
>> but I recall there are some other functions which has no drm
>> dependencies which may be suitable for you.
>
> DDC access is a property of the display controller, not the panel
> itself. The panel might be hooked up to a display controller's DDC/I2C
> channel as the target, but it isn't the host/controller of the DDC/I2C
> channel. As such, placing the nvidia,ddc property into the display
> controller node makes sense.
>
Yes, DC triggers the DDC access and is the host of the DDC/I2C channel.
So I think it's reasonable to put nvidia,ddc property into the display
controller node. But the video mode info in EDID which be fetched via
DDC is the property of the panel, so this info should be provided by
panel driver. Actually display controller cares about the video modes,
not the way to get these info. So I think it's better to do the whole
work like this:
1) display controller relied on CDF to call "display_entity_get_modes"
2) panel driver calls the function which is hooked to display controller
to get the video modes via DDC.
3) if the video modes can't be fetched via DDC, panel driver provides
the info(either by hard-coded or defined in DT) anyway
> Re: the other discussion in this thread: Probably the simplest is if
> tegradrm/other-CDF-users do something like:
>
> 1) If DDC I2C channel available for this display channel, query DDC.
Just like I said above, display controller should not query DDC
directly. Since CDF already defines these for us, display controller
should call CDF's functions, like: display_entity_get_modes,
display_entity_get_size... I think this is the key difference I wanna
talk about. And also in this way, display controller doesn't need to
care about this 2 steps logics, just call "display_entity_get_modes" is OK.
Mark
> 2) If not, or perhaps also if that fails, query the panel driver for the
> mode list.
>
> That would cover all bases very simply.
>
^ permalink raw reply
* Re: [RFC 1/4] video: panel: add CLAA101WA01A panel support
From: Alexandre Courbot @ 2013-01-31 4:14 UTC (permalink / raw)
To: Stephen Warren
Cc: Laurent Pinchart, Thierry Reding, Mark Zhang,
Linux Kernel Mailing List,
linux-fbdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Alexandre Courbot
In-Reply-To: <51098229.7080508-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
On Thu, Jan 31, 2013 at 5:27 AM, Stephen Warren <swarren@wwwdotorg.org> wrote:
> So this looks like a reasonable binding to me. The one issue is that
> it's very generic, and if we go this route, we'll probably end up with
> tens or hundreds of identical or extremely similar simple bindings, and
> associated drivers.
>
> We can avoid this instead by defining something like a "simple-lcd"
> binding, and associated driver, that will work for perhaps 90%, 95%,
> 99%, even 100%(?) of panels.
That seems totally doable indeed. Actually the right way to do this
might be by extending the simple DPI panel driver Laurent included in
his patchset.
> Just like the above, but with:
>
> compatible="simple-panel", "chunghwa,claa101wa01a"
>
> instead, and the driver binding to "simple-panel" rather than
> "chunghwa,claa101wa01a".
Just out of curiosity, why don't we rather define
compatible="chunghwa,claa101wa01a", "simple-panel"
in that order? I thought DT compatible strings should go from more to
less specific. The device would still bind to "simple-panel" if no
more specific driver exists.
> The driver can assume that a specific set of supplies (and perhaps
> GPIOs) is always present, and that the /sequence/ of manipulating those
> is fixed. This will avoid the need for anything like the power sequences
> code. If a particular panel doesn't fit those assumptions, including the
> exact sequence of manipulations for each state transition (which should
> be documented in the binding) then it can get a custom driver, this also
> avoiding having to define custom sequences in DT.
>
> Things that might be parameterized/optional:
>
> * Perhaps some GPIOs aren't always present.
> * If some regulators aren't SW-controllable, DT should still provide a
> fixed/dummy regulator so the driver doesn't care.
How about making all regulators and GPIO optional in the driver?
> * Wait times between regulator/GPIO/... manipulation could be specified
> in DT.
> * For panels without EDID, CDF DT bindings can provide the list of
> supported modes, otherwise we assume that the display controller that
> drives the panel has been told how to access the EDID, just like it
> would for an "external" display.
Excellent. Thanks for the feedback.
Alex.
^ permalink raw reply
* Re: [RFC 1/4] video: panel: add CLAA101WA01A panel support
From: Alexandre Courbot @ 2013-01-31 4:24 UTC (permalink / raw)
To: Mark Zhang
Cc: Stephen Warren, Laurent Pinchart, Thierry Reding, Mark Zhang,
Linux Kernel Mailing List,
linux-fbdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Alexandre Courbot
In-Reply-To: <5109EA2A.8020204-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
On Thu, Jan 31, 2013 at 12:51 PM, Mark Zhang <nvmarkzhang@gmail.com> wrote:
>> DDC access is a property of the display controller, not the panel
>> itself. The panel might be hooked up to a display controller's DDC/I2C
>> channel as the target, but it isn't the host/controller of the DDC/I2C
>> channel. As such, placing the nvidia,ddc property into the display
>> controller node makes sense.
>>
>
> Yes, DC triggers the DDC access and is the host of the DDC/I2C channel.
> So I think it's reasonable to put nvidia,ddc property into the display
> controller node. But the video mode info in EDID which be fetched via
> DDC is the property of the panel, so this info should be provided by
> panel driver. Actually display controller cares about the video modes,
> not the way to get these info. So I think it's better to do the whole
> work like this:
>
> 1) display controller relied on CDF to call "display_entity_get_modes"
> 2) panel driver calls the function which is hooked to display controller
> to get the video modes via DDC.
> 3) if the video modes can't be fetched via DDC, panel driver provides
> the info(either by hard-coded or defined in DT) anyway
This would require a callback dedicated to this in display_entity and
would break its simple design.
> Just like I said above, display controller should not query DDC
> directly. Since CDF already defines these for us, display controller
> should call CDF's functions, like: display_entity_get_modes,
> display_entity_get_size... I think this is the key difference I wanna
> talk about. And also in this way, display controller doesn't need to
> care about this 2 steps logics, just call "display_entity_get_modes" is OK.
Well the fact is that it *is* the display controller that queries DDC
directly. The panel is just a bus here, and the EDID EEPROM could
perfectly work without it. If you model what you described with DT
nodes, I'm afraid you will end up with something both complex (with DC
node referencing panel node and back again for the EDID bus) and that
does not picture reality accurately.
Alex.
^ permalink raw reply
* Re: [RFC 1/4] video: panel: add CLAA101WA01A panel support
From: Mark Zhang @ 2013-01-31 4:54 UTC (permalink / raw)
To: Alexandre Courbot
Cc: Stephen Warren, Laurent Pinchart, Thierry Reding, Mark Zhang,
Linux Kernel Mailing List, linux-fbdev@vger.kernel.org,
linux-tegra@vger.kernel.org
In-Reply-To: <CAAVeFuJf9Ww7g8Q9RAG9Na1SZq2-n2MMxcAEBek52ZVG7RpL7g@mail.gmail.com>
On 01/31/2013 12:24 PM, Alexandre Courbot wrote:
> On Thu, Jan 31, 2013 at 12:51 PM, Mark Zhang <nvmarkzhang@gmail.com> wrote:
>>> DDC access is a property of the display controller, not the panel
>>> itself. The panel might be hooked up to a display controller's DDC/I2C
>>> channel as the target, but it isn't the host/controller of the DDC/I2C
>>> channel. As such, placing the nvidia,ddc property into the display
>>> controller node makes sense.
>>>
>>
>> Yes, DC triggers the DDC access and is the host of the DDC/I2C channel.
>> So I think it's reasonable to put nvidia,ddc property into the display
>> controller node. But the video mode info in EDID which be fetched via
>> DDC is the property of the panel, so this info should be provided by
>> panel driver. Actually display controller cares about the video modes,
>> not the way to get these info. So I think it's better to do the whole
>> work like this:
>>
>> 1) display controller relied on CDF to call "display_entity_get_modes"
>> 2) panel driver calls the function which is hooked to display controller
>> to get the video modes via DDC.
>> 3) if the video modes can't be fetched via DDC, panel driver provides
>> the info(either by hard-coded or defined in DT) anyway
>
> This would require a callback dedicated to this in display_entity and
> would break its simple design.
We just need to add a function pointer param which can be used by panel
driver in "display_entity_get_modes", don't we?
>
>> Just like I said above, display controller should not query DDC
>> directly. Since CDF already defines these for us, display controller
>> should call CDF's functions, like: display_entity_get_modes,
>> display_entity_get_size... I think this is the key difference I wanna
>> talk about. And also in this way, display controller doesn't need to
>> care about this 2 steps logics, just call "display_entity_get_modes" is OK.
>
> Well the fact is that it *is* the display controller that queries DDC
> directly. The panel is just a bus here, and the EDID EEPROM could
> perfectly work without it. If you model what you described with DT
> nodes, I'm afraid you will end up with something both complex (with DC
> node referencing panel node and back again for the EDID bus) and that
> does not picture reality accurately.
Display controller don't know whether the panel has EDID EEPROM but the
panel driver knows. So why we need to make display controller queries
EDID blindly? Since panel driver knows about all "video modes/panel
size" stuffs, why not we let it handle all these things?
>
> Alex.
>
^ permalink raw reply
* Re: BUG: circular locking dependency detected
From: Greg Kroah-Hartman @ 2013-01-31 5:40 UTC (permalink / raw)
To: Linus Torvalds
Cc: Russell King, Dave Airlie, Daniel Vetter, Andrew Morton,
Florian Tobias Schandinat, linux-fbdev@vger.kernel.org,
Linux Kernel Mailing List, dri-devel
In-Reply-To: <CA+55aFxfeXWAiXV18qGgOk9+2HHiD7LUuyk55brHFXd+_c6eZQ@mail.gmail.com>
On Thu, Jan 31, 2013 at 11:26:53AM +1100, Linus Torvalds wrote:
> On Thu, Jan 31, 2013 at 11:13 AM, Russell King <rmk@arm.linux.org.uk> wrote:
> >
> > Which may or may not be a good thing depending how you look at it; it
> > means that once your kernel blanks, you get a lockdep dump. At that
> > point you lose lockdep checking for everything else because lockdep
> > disables itself after the first dump.
>
> Fair enough, we may want to revert the lockdep checking for
> console_lock, and make re-enabling it part of the patch-series that
> fixes the locking.
>
> Daniel/Dave? Does that sound reasonable?
Reverting the patch is fine with me. Just let me know so I can queue it
up again for 3.9.
thanks,
greg k-h
^ permalink raw reply
* Re: [RFC 1/4] video: panel: add CLAA101WA01A panel support
From: Alexandre Courbot @ 2013-01-31 6:36 UTC (permalink / raw)
To: Mark Zhang
Cc: Stephen Warren, Laurent Pinchart, Thierry Reding, Mark Zhang,
Linux Kernel Mailing List, linux-fbdev@vger.kernel.org,
linux-tegra@vger.kernel.org, acourbot@nvidia.com
In-Reply-To: <5109F916.8080404@gmail.com>
On Thu, Jan 31, 2013 at 1:54 PM, Mark Zhang <nvmarkzhang@gmail.com> wrote:
> Display controller don't know whether the panel has EDID EEPROM but the
> panel driver knows. So why we need to make display controller queries
> EDID blindly? Since panel driver knows about all "video modes/panel
> size" stuffs, why not we let it handle all these things?
The DC actually does know whether the connected panel supports EDID -
in this case, the output node will have a property for the DDC bus,
e.g.
nvidia,ddc-i2c-bus = <&lcd_ddc>;
If no DDC bus is specified or if transfer fails, the DC driver can
still query the panel driver for hardcoded modes.
Also passing a function pointer to get_modes() does not relief the DC
driver from probing for the DDC bus. You will still have to handle
both conditions (DDC bus present or not), the check will just be moved
into your callback function.
Alex.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox