Linux Framebuffer Layer development
 help / color / mirror / Atom feed
* [RFC PATCH v2 1/5] video: display: add event handling, set mode and hdmi ops to cdf core
From: Rahul Sharma @ 2013-02-07 11:59 UTC (permalink / raw)
  To: linux-media, dri-devel, alsa-devel, linux-fbdev
  Cc: tomi.valkeinen, laurent.pinchart, broonie, inki.dae,
	kyungmin.park, r.sh.open, joshi
In-Reply-To: <1360238377-14806-1-git-send-email-rahul.sharma@samsung.com>

This patch adds
1) Event Notification to CDF Core:
	Adds simple event notification mechanism supports multiple
	subscribers. This is used for hot-plug notification to the clients
	of hdmi display i.e. exynos-drm and alsa-codec. CDF Core maintains
	multiple subscriber list. When entity reports a event Core will
	route it to all of them. Un-superscription is not implemented which
	can be done if notification callback is Null.

2) set_mode to generic ops:
	It is meaningful for a panel like hdmi which supports multiple
	resolutions.

	HDMI needs conversion of Display Modes to Standard Display Timings.
	Though, it can be done within the driver but seems more meaningful
	if set_mode is called with Timing Details provided by CDF Core.

3) Provision for platform specific interfaces through void *private in display
entity:

	It has added void *private to display entity which can be used to
	expose interfaces which are very much specific to a particular platform.

	In exynos, hpd is connected to the soc via gpio bus. During initial
	hdmi poweron, hpd interrupt is not raised as there is no change in the
	gpio status. This is solved by providing a platform specific interface
	which is queried by the drm to get the hpd state. This interface may
	not be required by all platforms.

4) hdmi ops:
	get_edid: to query raw EDID data and length from the panel.
	check_mode: To check if a given mode is supported by exynos HDMI IP
			"AND" Connected HDMI Sink (tv/monitor).
	init_audio: Configure hdmi audio registers for Audio interface type
	(i2s/ spdif), SF, Audio Channels, BPS.
	set_audiostate: enable disable audio.

Signed-off-by: Rahul Sharma <rahul.sharma@samsung.com>
---
 drivers/video/display/display-core.c |  85 +++++++++++++++++++++++++++
 include/video/display.h              | 111 ++++++++++++++++++++++++++++++++++-
 2 files changed, 193 insertions(+), 3 deletions(-)

diff --git a/drivers/video/display/display-core.c b/drivers/video/display/display-core.c
index 55a7399..12fb882 100644
--- a/drivers/video/display/display-core.c
+++ b/drivers/video/display/display-core.c
@@ -99,6 +99,14 @@ int display_entity_get_modes(struct display_entity *entity,
 }
 EXPORT_SYMBOL_GPL(display_entity_get_modes);
 
+int display_entity_set_mode(struct display_entity *entity,
+		   const struct videomode *mode)
+{
+	if (!entity->opt_ctrl.hdmi || !entity->ops.ctrl->set_mode)
+		return 0;
+	return entity->ops.ctrl->set_mode(entity, mode);
+}
+EXPORT_SYMBOL_GPL(display_entity_set_mode);
 /**
 * display_entity_get_size - Get display entity physical size
 * @entity: The display entity
@@ -140,6 +148,37 @@ int display_entity_get_params(struct display_entity *entity,
 }
 EXPORT_SYMBOL_GPL(display_entity_get_params);
 
+int display_entity_subscribe_event(struct display_entity *entity,
+		struct display_event_subscriber *subscriber)
+{
+	if (!entity || !subscriber || !subscriber->notify)
+		return -EINVAL;
+
+	mutex_lock(&entity->entity_mutex);
+	list_add_tail(&subscriber->list, &entity->list_subscribers);
+	mutex_unlock(&entity->entity_mutex);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(display_entity_subscribe_event);
+
+int display_entity_notify_event_subscriber(struct display_entity *entity,
+		enum display_entity_event_type type, unsigned int value)
+{
+	struct display_event_subscriber *subscriber;
+
+	if (!entity || type < 0)
+		return -EINVAL;
+
+	mutex_lock(&entity->entity_mutex);
+	list_for_each_entry(subscriber, &entity->list_subscribers, list) {
+		subscriber->notify(entity, type, value, subscriber->context);
+	}
+	mutex_unlock(&entity->entity_mutex);
+	return 0;
+}
+EXPORT_SYMBOL_GPL(display_entity_notify_event_subscriber);
+
 /* -----------------------------------------------------------------------------
 * Video operations
 */
@@ -312,6 +351,9 @@ int __must_check __display_entity_register(struct display_entity *entity,
 	kref_init(&entity->ref);
 	entity->owner = owner;
 	entity->state = DISPLAY_ENTITY_STATE_OFF;
+	entity->list_subscribers.next = &entity->list_subscribers;
+	entity->list_subscribers.prev = &entity->list_subscribers;
+	mutex_init(&entity->entity_mutex);
 
 	mutex_lock(&display_entity_mutex);
 	list_add(&entity->list, &display_entity_list);
@@ -357,6 +399,49 @@ void display_entity_unregister(struct display_entity *entity)
 }
 EXPORT_SYMBOL_GPL(display_entity_unregister);
 
+/* -----------------------------------------------------------------------------
+ * Display Entity Hdmi ops
+ */
+
+int display_entity_hdmi_get_edid(struct display_entity *entity,
+			struct display_entity_edid *edid)
+{
+	if (!entity->opt_ctrl.hdmi || !entity->opt_ctrl.hdmi->get_edid)
+		return 0;
+
+	return entity->opt_ctrl.hdmi->get_edid(entity, edid);
+}
+EXPORT_SYMBOL_GPL(display_entity_hdmi_get_edid);
+
+int display_entity_hdmi_check_mode(struct display_entity *entity,
+		   const struct videomode *mode)
+{
+	if (!entity->opt_ctrl.hdmi || !entity->opt_ctrl.hdmi->check_mode)
+		return 0;
+
+	return entity->opt_ctrl.hdmi->check_mode(entity, mode);
+}
+EXPORT_SYMBOL_GPL(display_entity_hdmi_check_mode);
+
+int display_entity_hdmi_init_audio(struct display_entity *entity,
+		const struct display_entity_audio_params *params)
+{
+	if (!entity->opt_ctrl.hdmi || !entity->opt_ctrl.hdmi->init_audio)
+		return 0;
+
+	return entity->opt_ctrl.hdmi->init_audio(entity, params);
+}
+EXPORT_SYMBOL_GPL(display_entity_hdmi_init_audio);
+
+int display_entity_hdmi_set_audiostate(struct display_entity *entity,
+	enum display_entity_audiostate state)
+{
+	if (!entity->opt_ctrl.hdmi || !entity->opt_ctrl.hdmi->set_audiostate)
+		return 0;
+
+	return entity->opt_ctrl.hdmi->set_audiostate(entity, state);
+}
+EXPORT_SYMBOL_GPL(display_entity_hdmi_set_audiostate);
 MODULE_AUTHOR("Laurent Pinchart <laurent.pinchart@ideasonboard.com>");
 MODULE_DESCRIPTION("Display Core");
 MODULE_LICENSE("GPL");
diff --git a/include/video/display.h b/include/video/display.h
index 817f4ae..eae373f 100644
--- a/include/video/display.h
+++ b/include/video/display.h
@@ -66,22 +66,64 @@ enum display_entity_stream_state {
 	DISPLAY_ENTITY_STREAM_CONTINUOUS,
 };
 
+enum display_entity_event_type {
+	DISPLAY_ENTITY_HDMI_HOTPLUG,
+};
+
 enum display_entity_interface_type {
 	DISPLAY_ENTITY_INTERFACE_DPI,
+	DISPLAY_ENTITY_INTERFACE_HDMI,
+};
+
+enum display_entity_audio_interface {
+	DISPLAY_ENTITY_AUDIO_I2S,
+	DISPLAY_ENTITY_AUDIO_SPDIF,
+};
+
+enum display_entity_audiostate {
+	DISPLAY_ENTITY_AUDIOSTATE_OFF,
+	DISPLAY_ENTITY_AUDIOSTATE_ON,
 };
 
 struct display_entity_interface_params {
 	enum display_entity_interface_type type;
 };
 
+struct display_event_subscriber {
+	struct list_head list;
+	void(*notify)(struct display_entity *ent,
+		enum display_entity_event_type type,
+		unsigned int value, void *context);
+	void *context;
+};
+
+struct display_entity_edid {
+	u8		*edid;
+	int		len;
+};
+
+struct display_entity_audio_params {
+	enum display_entity_audio_interface	type;
+	int	channels;
+	int	sf;
+	int	bits_per_sample;
+};
+
 struct display_entity_control_ops {
 	int (*set_state)(struct display_entity *ent,
 			enum display_entity_state state);
+
 	int (*update)(struct display_entity *ent);
+
 	int (*get_modes)(struct display_entity *ent,
 			const struct videomode **modes);
+
+	int (*set_mode)(struct display_entity *entity,
+		const struct videomode *modes);
+
 	int (*get_params)(struct display_entity *ent,
 			struct display_entity_interface_params *params);
+
 	int (*get_size)(struct display_entity *ent,
 			unsigned int *width, unsigned int *height);
 };
@@ -91,8 +133,29 @@ struct display_entity_video_ops {
 			enum display_entity_stream_state state);
 };
 
+struct display_entity_hdmi_control_ops {
+
+	int (*get_edid)(struct display_entity *ent,
+		struct display_entity_edid *edid);
+
+	int (*check_mode)(struct display_entity *entity,
+		const struct videomode *modes);
+
+	int (*init_audio)(struct display_entity *entity,
+		const struct display_entity_audio_params *params);
+
+	int (*set_audiostate)(struct display_entity *entity,
+		enum display_entity_audiostate state);
+};
+
+struct display_entity_hdmi_video_ops {
+	int (*get_edid)(struct display_entity *ent,
+			    enum display_entity_stream_state state);
+};
+
 struct display_entity {
 	struct list_head list;
+	struct list_head list_subscribers;
 	struct device *dev;
 	struct module *owner;
 	struct kref ref;
@@ -104,26 +167,51 @@ struct display_entity {
 		const struct display_entity_video_ops *video;
 	} ops;
 
+	union {
+		const struct display_entity_hdmi_control_ops *hdmi;
+	} opt_ctrl;
+
+	union {
+		const struct display_entity_hdmi_video_ops *hdmi;
+	} opt_video;
+
 	void(*release)(struct display_entity *ent);
 
 	enum display_entity_state state;
+	struct mutex	entity_mutex;
+	void		*private;
 };
 
+/* generic display entity ops */
+
 int display_entity_set_state(struct display_entity *entity,
 				enum display_entity_state state);
+
 int display_entity_update(struct display_entity *entity);
+
 int display_entity_get_modes(struct display_entity *entity,
 				const struct videomode **modes);
+
+int display_entity_set_mode(struct display_entity *entity,
+		const struct videomode *modes);
+
 int display_entity_get_params(struct display_entity *entity,
-				struct display_entity_interface_params *params);
+		struct display_entity_interface_params *params);
+
 int display_entity_get_size(struct display_entity *entity,
-			unsigned int *width, unsigned int *height);
+		unsigned int *width, unsigned int *height);
 
 int display_entity_set_stream(struct display_entity *entity,
 				enum display_entity_stream_state state);
 
+int display_entity_subscribe_event(struct display_entity *entity,
+		struct display_event_subscriber *subscriber);
+
+int display_entity_notify_event_subscriber(struct display_entity *entity,
+		enum display_entity_event_type type, unsigned int value);
+
 static inline void display_entity_connect(struct display_entity *source,
-					struct display_entity *sink)
+				struct display_entity *sink)
 {
 	sink->source = source;
 }
@@ -147,4 +235,21 @@ void display_entity_unregister_notifier(struct display_entity_notifier *notifier
 #define display_entity_register(display_entity) \
 	__display_entity_register(display_entity, THIS_MODULE)
 
+/* hdmi ops */
+
+int display_entity_hdmi_get_edid(struct display_entity *entity,
+		struct display_entity_edid *edid);
+
+int display_entity_hdmi_check_mode(struct display_entity *entity,
+		const struct videomode *modes);
+
+int display_entity_hdmi_get_hpdstate(struct display_entity *entity,
+			unsigned int *hpd_state);
+
+int display_entity_hdmi_init_audio(struct display_entity *entity,
+	const struct display_entity_audio_params *params);
+
+int display_entity_hdmi_set_audiostate(struct display_entity *entity,
+	enum display_entity_audiostate state);
+
 #endif /* __DISPLAY_H__ */
-- 
1.8.0


^ permalink raw reply related

* Re: [RFC PATCH v2 2/5] drm/edid: temporarily exposing generic edid-read interface from drm
From: Daniel Vetter @ 2013-02-07 13:28 UTC (permalink / raw)
  To: Rahul Sharma
  Cc: linux-media, dri-devel, alsa-devel, linux-fbdev, broonie, joshi,
	kyungmin.park, tomi.valkeinen, laurent.pinchart
In-Reply-To: <1360238951-7022-1-git-send-email-rahul.sharma@samsung.com>

On Thu, Feb 7, 2013 at 1:09 PM, Rahul Sharma <rahul.sharma@samsung.com> wrote:
> It exposes generic interface from drm_edid.c to get the edid data and length
> by any display entity. Once I get clear idea about edid handling in CDF, I need
> to revert these temporary changes.

Just a quick reply about edid reading: One of the key results (at
least imo) of the fosdem cdf discussion was that we need to split up
the different parts of it clearly (i.e. abstract panel interface, dsi
support, discovery/dev matching, ...) to have more flexibility. One
idea is also to not use the panel interface for e.g. hdmi transcoders,
but only use the bus support (like dsi), since transcoders which
connect to external devices like hdmi need to expose _much_ more
features to the master driver and so it's better to have tighter
integration. Some of the things which need close cooperation between
drivers are e.g. edid reading, hotplug handling,
bpp/colorspace/restricted range stuff, ... I didn't read through the
patch which requires the exported drm edid stuff, but maybe this helps
a bit.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

^ permalink raw reply

* [PATCH 0/1] OMAP4: DSS: Add panel for Blaze Tablet boards
From: Ruslan Bilovol @ 2013-02-07 14:40 UTC (permalink / raw)
  To: tomi.valkeinen, FlorianSchandinat, linux-fbdev, linux-kernel,
	linux-omap

Hi,

This patch adds support for TC358765 DSI-to-LVDS transmitter
from Toshiba, that is used in OMAP4 Blaze Tablet development
platform. It was originally developed a long time ago and
was used internally survived few kernel migrations.
Different people worked in this driver during last two
years changing its sources to what each new version of
kernel expects.

Current version is ported from internal kernel v3.4 to 3.8.0-rc6
Tested basic functioning under busybox.

-
Ruslan

Tomi Valkeinen (1):
  OMAP4: DSS: Add panel for Blaze Tablet boards

 drivers/video/omap2/displays/Kconfig          |   15 +
 drivers/video/omap2/displays/Makefile         |    1 +
 drivers/video/omap2/displays/panel-tc358765.c | 1001 +++++++++++++++++++++++++
 drivers/video/omap2/displays/panel-tc358765.h |  170 +++++
 include/video/omap-panel-tc358765.h           |   53 ++
 5 files changed, 1240 insertions(+)
 create mode 100644 drivers/video/omap2/displays/panel-tc358765.c
 create mode 100644 drivers/video/omap2/displays/panel-tc358765.h
 create mode 100644 include/video/omap-panel-tc358765.h

-- 
1.7.9.5


^ permalink raw reply

* [PATCH 1/1] OMAP4: DSS: Add panel for Blaze Tablet boards
From: Ruslan Bilovol @ 2013-02-07 14:40 UTC (permalink / raw)
  To: tomi.valkeinen, FlorianSchandinat, linux-fbdev, linux-kernel,
	linux-omap
In-Reply-To: <1360248051-16541-1-git-send-email-ruslan.bilovol@ti.com>

From: Tomi Valkeinen <tomi.valkeinen@ti.com>

TC358765 is DSI-to-LVDS transmitter from Toshiba, used in
OMAP44XX Blaze Tablet and Blaze Tablet2 boards.

Signed-off-by: Dan Murphy <dmurphy@ti.com>
Signed-off-by: Sergiy Kibrik <sergiy.kibrik@globallogic.com>
Signed-off-by: Volodymyr Riazantsev <v.riazantsev@ti.com>
Signed-off-by: Ruslan Bilovol <ruslan.bilovol@ti.com>
---
 drivers/video/omap2/displays/Kconfig          |   15 +
 drivers/video/omap2/displays/Makefile         |    1 +
 drivers/video/omap2/displays/panel-tc358765.c | 1001 +++++++++++++++++++++++++
 drivers/video/omap2/displays/panel-tc358765.h |  170 +++++
 include/video/omap-panel-tc358765.h           |   53 ++
 5 files changed, 1240 insertions(+)
 create mode 100644 drivers/video/omap2/displays/panel-tc358765.c
 create mode 100644 drivers/video/omap2/displays/panel-tc358765.h
 create mode 100644 include/video/omap-panel-tc358765.h

diff --git a/drivers/video/omap2/displays/Kconfig b/drivers/video/omap2/displays/Kconfig
index c3853c9..c6ab261 100644
--- a/drivers/video/omap2/displays/Kconfig
+++ b/drivers/video/omap2/displays/Kconfig
@@ -72,4 +72,19 @@ config PANEL_N8X0
 	depends on BACKLIGHT_CLASS_DEVICE
 	help
 	  This is the LCD panel used on Nokia N8x0
+
+config PANEL_TC358765
+	tristate "Toshiba TC358765 DSI-2-LVDS bridge"
+	depends on OMAP2_DSS_DSI && I2C
+	select BACKLIGHT_CLASS_DEVICE
+	help
+		Toshiba TC358765 DSI-2-LVDS chip with 1024x768 panel,
+		which can be found in OMAP4-based Blaze Tablet boards
+		and some other boards.
+
+config TC358765_DEBUG
+	bool "Toshiba TC358765 DSI-2-LVDS chip debug"
+	depends on PANEL_TC358765 && DEBUG_FS
+	help
+	  Support of TC358765 DSI-2-LVDS chip register access via debugfs
 endmenu
diff --git a/drivers/video/omap2/displays/Makefile b/drivers/video/omap2/displays/Makefile
index 58a5176..b9f2ab6 100644
--- a/drivers/video/omap2/displays/Makefile
+++ b/drivers/video/omap2/displays/Makefile
@@ -9,3 +9,4 @@ obj-$(CONFIG_PANEL_PICODLP) +=  panel-picodlp.o
 obj-$(CONFIG_PANEL_TPO_TD043MTEA1) += panel-tpo-td043mtea1.o
 obj-$(CONFIG_PANEL_ACX565AKM) += panel-acx565akm.o
 obj-$(CONFIG_PANEL_N8X0) += panel-n8x0.o
+obj-$(CONFIG_PANEL_TC358765) += panel-tc358765.o
diff --git a/drivers/video/omap2/displays/panel-tc358765.c b/drivers/video/omap2/displays/panel-tc358765.c
new file mode 100644
index 0000000..e9d7e96
--- /dev/null
+++ b/drivers/video/omap2/displays/panel-tc358765.c
@@ -0,0 +1,1001 @@
+/*
+ * Toshiba TC358765 DSI-to-LVDS chip driver
+ *
+ * Copyright (C) Texas Instruments
+ * Author: Tomi Valkeinen <tomi.valkeinen@ti.com> (3.0)
+ * Author: Sergii Kibrik <sergiikibrik@ti.com> (3.4)
+ * Author: Ruslan Bilovol <ruslan.bilovol@ti.com> (3.8+)
+ *
+ * Based on original version from Jerry Alexander <x0135174@ti.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.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <linux/io.h>
+#include <linux/mm.h>
+#include <linux/delay.h>
+#include <linux/err.h>
+#include <linux/gpio.h>
+#include <linux/mutex.h>
+#include <linux/i2c.h>
+#include <linux/pm_runtime.h>
+#include <linux/debugfs.h>
+#include <linux/seq_file.h>
+#include <linux/uaccess.h>
+
+#include <video/omapdss.h>
+#include <video/omap-panel-tc358765.h>
+
+#include "panel-tc358765.h"
+
+#define A_RO 0x1
+#define A_WO 0x2
+#define A_RW (A_RO|A_WO)
+
+#define FLD_MASK(start, end)	(((1 << ((start) - (end) + 1)) - 1) << (end))
+#define FLD_VAL(val, start, end) (((val) << (end)) & FLD_MASK(start, end))
+#define FLD_GET(val, start, end) (((val) & FLD_MASK(start, end)) >> (end))
+#define FLD_MOD(orig, val, start, end) \
+	(((orig) & ~FLD_MASK(start, end)) | FLD_VAL(val, start, end))
+
+static struct omap_video_timings tc358765_timings;
+static struct tc358765_board_data *get_board_data(struct omap_dss_device
+					*dssdev) __attribute__ ((unused));
+
+/* device private data structure */
+struct tc358765_data {
+	struct mutex lock;
+
+	struct omap_dss_device *dssdev;
+
+	int channel0;
+	int channel1;
+
+	struct omap_dsi_pin_config pin_config;
+};
+
+static struct {
+	struct i2c_client *client;
+	struct mutex xfer_lock;
+} *tc358765_i2c;
+
+
+#ifdef CONFIG_TC358765_DEBUG
+
+struct {
+	struct device *dev;
+	struct dentry *dir;
+} tc358765_debug;
+
+struct tc358765_reg {
+	const char *name;
+	u16 reg;
+	u8 perm:2;
+} tc358765_regs[] = {
+	/* DSI D-PHY Layer Registers */
+	{ "D0W_DPHYCONTTX", D0W_DPHYCONTTX, A_RW },
+	{ "CLW_DPHYCONTRX", CLW_DPHYCONTRX, A_RW },
+	{ "D0W_DPHYCONTRX", D0W_DPHYCONTRX, A_RW },
+	{ "D1W_DPHYCONTRX", D1W_DPHYCONTRX, A_RW },
+	{ "D2W_DPHYCONTRX", D2W_DPHYCONTRX, A_RW },
+	{ "D3W_DPHYCONTRX", D3W_DPHYCONTRX, A_RW },
+	{ "COM_DPHYCONTRX", COM_DPHYCONTRX, A_RW },
+	{ "CLW_CNTRL", CLW_CNTRL, A_RW },
+	{ "D0W_CNTRL", D0W_CNTRL, A_RW },
+	{ "D1W_CNTRL", D1W_CNTRL, A_RW },
+	{ "D2W_CNTRL", D2W_CNTRL, A_RW },
+	{ "D3W_CNTRL", D3W_CNTRL, A_RW },
+	{ "DFTMODE_CNTRL", DFTMODE_CNTRL, A_RW },
+	/* DSI PPI Layer Registers */
+	{ "PPI_STARTPPI", PPI_STARTPPI, A_RW },
+	{ "PPI_BUSYPPI", PPI_BUSYPPI, A_RO },
+	{ "PPI_LINEINITCNT", PPI_LINEINITCNT, A_RW },
+	{ "PPI_LPTXTIMECNT", PPI_LPTXTIMECNT, A_RW },
+	{ "PPI_LANEENABLE", PPI_LANEENABLE, A_RW },
+	{ "PPI_TX_RX_TA", PPI_TX_RX_TA, A_RW },
+	{ "PPI_CLS_ATMR", PPI_CLS_ATMR, A_RW },
+	{ "PPI_D0S_ATMR", PPI_D0S_ATMR, A_RW },
+	{ "PPI_D1S_ATMR", PPI_D1S_ATMR, A_RW },
+	{ "PPI_D2S_ATMR", PPI_D2S_ATMR, A_RW },
+	{ "PPI_D3S_ATMR", PPI_D3S_ATMR, A_RW },
+	{ "PPI_D0S_CLRSIPOCOUNT", PPI_D0S_CLRSIPOCOUNT, A_RW },
+	{ "PPI_D1S_CLRSIPOCOUNT", PPI_D1S_CLRSIPOCOUNT, A_RW },
+	{ "PPI_D2S_CLRSIPOCOUNT", PPI_D2S_CLRSIPOCOUNT, A_RW },
+	{ "PPI_D3S_CLRSIPOCOUNT", PPI_D3S_CLRSIPOCOUNT, A_RW },
+	{ "CLS_PRE", CLS_PRE, A_RW },
+	{ "D0S_PRE", D0S_PRE, A_RW },
+	{ "D1S_PRE", D1S_PRE, A_RW },
+	{ "D2S_PRE", D2S_PRE, A_RW },
+	{ "D3S_PRE", D3S_PRE, A_RW },
+	{ "CLS_PREP", CLS_PREP, A_RW },
+	{ "D0S_PREP", D0S_PREP, A_RW },
+	{ "D1S_PREP", D1S_PREP, A_RW },
+	{ "D2S_PREP", D2S_PREP, A_RW },
+	{ "D3S_PREP", D3S_PREP, A_RW },
+	{ "CLS_ZERO", CLS_ZERO, A_RW },
+	{ "D0S_ZERO", D0S_ZERO, A_RW },
+	{ "D1S_ZERO", D1S_ZERO, A_RW },
+	{ "D2S_ZERO", D2S_ZERO, A_RW  },
+	{ "D3S_ZERO", D3S_ZERO, A_RW },
+	{ "PPI_CLRFLG", PPI_CLRFLG, A_RW },
+	{ "PPI_CLRSIPO", PPI_CLRSIPO, A_RW },
+	{ "PPI_HSTimeout", PPI_HSTimeout, A_RW },
+	{ "PPI_HSTimeoutEnable", PPI_HSTimeoutEnable, A_RW },
+	/* DSI Protocol Layer Registers */
+	{ "DSI_STARTDSI", DSI_STARTDSI, A_WO },
+	{ "DSI_BUSYDSI", DSI_BUSYDSI, A_RO },
+	{ "DSI_LANEENABLE", DSI_LANEENABLE, A_RW },
+	{ "DSI_LANESTATUS0", DSI_LANESTATUS0, A_RO },
+	{ "DSI_LANESTATUS1", DSI_LANESTATUS1, A_RO },
+	{ "DSI_INTSTATUS", DSI_INTSTATUS, A_RO },
+	{ "DSI_INTMASK", DSI_INTMASK, A_RW },
+	{ "DSI_INTCLR", DSI_INTCLR, A_WO },
+	{ "DSI_LPTXTO", DSI_LPTXTO, A_RW },
+	/* DSI General Registers */
+	{ "DSIERRCNT", DSIERRCNT, A_RW },
+	/* DSI Application Layer Registers */
+	{ "APLCTRL", APLCTRL, A_RW },
+	{ "RDPKTLN", RDPKTLN, A_RW },
+	/* Video Path Registers */
+	{ "VPCTRL", VPCTRL, A_RW },
+	{ "HTIM1", HTIM1, A_RW },
+	{ "HTIM2",  HTIM2, A_RW },
+	{ "VTIM1", VTIM1, A_RW },
+	{ "VTIM2", VTIM2, A_RW },
+	{ "VFUEN", VFUEN, A_RW },
+	/* LVDS Registers */
+	{ "LVMX0003", LVMX0003, A_RW },
+	{ "LVMX0407", LVMX0407, A_RW },
+	{ "LVMX0811", LVMX0811, A_RW },
+	{ "LVMX1215", LVMX1215, A_RW },
+	{ "LVMX1619", LVMX1619, A_RW },
+	{ "LVMX2023", LVMX2023, A_RW },
+	{ "LVMX2427", LVMX2427, A_RW },
+	{ "LVCFG", LVCFG, A_RW },
+	{ "LVPHY0", LVPHY0, A_RW },
+	{ "LVPHY1", LVPHY1, A_RW },
+	/* System Registers */
+	{ "SYSSTAT", SYSSTAT, A_RO },
+	{ "SYSRST", SYSRST, A_WO },
+	/* GPIO Registers */
+	{ "GPIOC", GPIOC, A_RW },
+	{ "GPIOO", GPIOO, A_RW },
+	{ "GPIOI", GPIOI, A_RO },
+	/* I2C Registers */
+	{ "I2CTIMCTRL", I2CTIMCTRL, A_RW },
+	{ "I2CMADDR", I2CMADDR, A_RW },
+	{ "WDATAQ", WDATAQ, A_WO },
+	{ "RDATAQ", RDATAQ, A_WO },
+	/* Chip/Rev Registers */
+	{ "IDREG", IDREG, A_RO },
+	/* Debug Registers */
+	{ "DEBUG00", DEBUG00, A_RW },
+	{ "DEBUG01", DEBUG01, A_RW },
+};
+#endif
+
+static int tc358765_read_block(u16 reg, u8 *data, int len)
+{
+	unsigned char wb[2];
+	struct i2c_msg msg[2];
+	int r;
+	mutex_lock(&tc358765_i2c->xfer_lock);
+	wb[0] = (reg & 0xff00) >> 8;
+	wb[1] = reg & 0xff;
+	msg[0].addr = tc358765_i2c->client->addr;
+	msg[0].len = 2;
+	msg[0].flags = 0;
+	msg[0].buf = wb;
+	msg[1].addr = tc358765_i2c->client->addr;
+	msg[1].flags = I2C_M_RD;
+	msg[1].len = len;
+	msg[1].buf = data;
+
+	r = i2c_transfer(tc358765_i2c->client->adapter, msg, ARRAY_SIZE(msg));
+	mutex_unlock(&tc358765_i2c->xfer_lock);
+
+	if (r = ARRAY_SIZE(msg))
+		return len;
+
+	return r;
+}
+
+static int tc358765_i2c_read(u16 reg, u32 *val)
+{
+	int r;
+	u8 data[4];
+	data[0] = data[1] = data[2] = data[3] = 0;
+
+	r = tc358765_read_block(reg, data, ARRAY_SIZE(data));
+	if (r != ARRAY_SIZE(data))
+		return r;
+
+	*val = ((int)data[3] << 24) | ((int)(data[2]) << 16) |
+	    ((int)(data[1]) << 8) | ((int)(data[0]));
+	return 0;
+}
+
+static int tc358765_dsi_read(struct omap_dss_device *dssdev, u16 reg, u32 *val)
+{
+	struct tc358765_data *d2d = dev_get_drvdata(&dssdev->dev);
+	u8 buf[4];
+	int r;
+
+	r = dsi_vc_generic_read_2(dssdev, d2d->channel1, ((u8 *)&reg)[0],
+				((u8 *)&reg)[1], buf, 4);
+	if (r < 0) {
+		dev_err(&dssdev->dev, "0x%x read failed with status %d\n",
+								reg, r);
+		return r;
+	}
+
+	*val = buf[0] | (buf[1] << 8) | (buf[2] << 16) | (buf[3] << 24);
+	return 0;
+}
+
+static int tc358765_read_register(struct omap_dss_device *dssdev,
+					u16 reg, u32 *val)
+{
+	int ret = 0;
+	pm_runtime_get_sync(&dssdev->dev);
+	/* I2C is preferred way of reading, but fall back to DSI
+	 * if I2C didn't got initialized
+	*/
+	if (tc358765_i2c)
+		ret = tc358765_i2c_read(reg, val);
+	else
+		ret = tc358765_dsi_read(dssdev, reg, val);
+	pm_runtime_put_sync(&dssdev->dev);
+
+	return ret;
+}
+
+static int tc358765_write_register(struct omap_dss_device *dssdev, u16 reg,
+		u32 value)
+{
+	struct tc358765_data *d2d = dev_get_drvdata(&dssdev->dev);
+	u8 buf[6];
+	int r;
+
+	buf[0] = (reg >> 0) & 0xff;
+	buf[1] = (reg >> 8) & 0xff;
+	buf[2] = (value >> 0) & 0xff;
+	buf[3] = (value >> 8) & 0xff;
+	buf[4] = (value >> 16) & 0xff;
+	buf[5] = (value >> 24) & 0xff;
+
+	r = dsi_vc_generic_write_nosync(dssdev, d2d->channel1, buf, 6);
+	if (r)
+		dev_err(&dssdev->dev, "reg write reg(%x) val(%x) failed: %d\n",
+			       reg, value, r);
+	return r;
+}
+
+/****************************
+********* DEBUG *************
+****************************/
+#ifdef CONFIG_TC358765_DEBUG
+static int tc358765_write_register_i2c(u16 reg, u32 val)
+{
+	int ret = -ENODEV;
+	unsigned char buf[6];
+	struct i2c_msg msg;
+
+	if (!tc358765_i2c) {
+		dev_err(tc358765_debug.dev, "%s: I2C not initilized\n",
+							__func__);
+		return ret;
+	}
+
+	buf[0] = (reg >> 8) & 0xff;
+	buf[1] = (reg >> 0) & 0xff;
+	buf[2] = (val >> 0) & 0xff;
+	buf[3] = (val >> 8) & 0xff;
+	buf[4] = (val >> 16) & 0xff;
+	buf[5] = (val >> 24) & 0xff;
+	msg.addr = tc358765_i2c->client->addr;
+	msg.len = sizeof(buf);
+	msg.flags = 0;
+	msg.buf = buf;
+
+	mutex_lock(&tc358765_i2c->xfer_lock);
+	ret = i2c_transfer(tc358765_i2c->client->adapter, &msg, 1);
+	mutex_unlock(&tc358765_i2c->xfer_lock);
+
+	if (ret != 1)
+		return ret;
+	return 0;
+}
+
+
+static int tc358765_registers_show(struct seq_file *seq, void *pos)
+{
+	struct device *dev = tc358765_debug.dev;
+	unsigned i, reg_count;
+	uint value;
+
+	if (!tc358765_i2c) {
+		dev_warn(dev,
+			"failed to read register: I2C not initialized\n");
+		return -ENODEV;
+	}
+
+	reg_count = sizeof(tc358765_regs) / sizeof(tc358765_regs[0]);
+	pm_runtime_get_sync(dev);
+	for (i = 0; i < reg_count; i++) {
+		if (tc358765_regs[i].perm & A_RO) {
+			tc358765_i2c_read(tc358765_regs[i].reg, &value);
+			seq_printf(seq, "%-20s = 0x%02X\n",
+					tc358765_regs[i].name, value);
+		}
+	}
+
+	pm_runtime_put_sync(dev);
+	return 0;
+}
+static ssize_t tc358765_seq_write(struct file *filp, const char __user *ubuf,
+						size_t size, loff_t *ppos)
+{
+	struct device *dev = tc358765_debug.dev;
+	unsigned i, reg_count;
+	u32 value = 0;
+	int error = 0;
+	/* kids, don't use register names that long */
+	char name[30];
+	char buf[50];
+
+	if (size >= sizeof(buf))
+		size = sizeof(buf);
+
+	if (copy_from_user(&buf, ubuf, size))
+		return -EFAULT;
+
+	buf[size-1] = '\0';
+	if (sscanf(buf, "%s %x", name, &value) != 2) {
+		dev_err(dev, "%s: unable to parse input\n", __func__);
+		return -1;
+	}
+
+	if (!tc358765_i2c) {
+		dev_warn(dev,
+			"failed to write register: I2C not initialized\n");
+		return -ENODEV;
+	}
+
+	reg_count = sizeof(tc358765_regs) / sizeof(tc358765_regs[0]);
+	for (i = 0; i < reg_count; i++) {
+		if (!strcmp(name, tc358765_regs[i].name)) {
+			if (!(tc358765_regs[i].perm & A_WO)) {
+				dev_err(dev, "%s is write-protected\n", name);
+				return -EACCES;
+			}
+
+			error = tc358765_write_register_i2c(
+				tc358765_regs[i].reg, value);
+			if (error) {
+				dev_err(dev, "%s: failed to write %s\n",
+					__func__, name);
+				return -1;
+			}
+
+			return size;
+		}
+	}
+
+	dev_err(dev, "%s: no such register %s\n", __func__, name);
+
+	return size;
+}
+
+static ssize_t tc358765_seq_open(struct inode *inode, struct file *file)
+{
+	return single_open(file, tc358765_registers_show, inode->i_private);
+}
+
+static const struct file_operations tc358765_debug_fops = {
+	.open		= tc358765_seq_open,
+	.read		= seq_read,
+	.write		= tc358765_seq_write,
+	.llseek		= seq_lseek,
+	.release	= single_release,
+};
+
+static int tc358765_initialize_debugfs(struct omap_dss_device *dssdev)
+{
+	tc358765_debug.dir = debugfs_create_dir("tc358765", NULL);
+	if (IS_ERR(tc358765_debug.dir)) {
+		int ret = PTR_ERR(tc358765_debug.dir);
+		tc358765_debug.dir = NULL;
+		return ret;
+	}
+
+	tc358765_debug.dev = &dssdev->dev;
+	debugfs_create_file("registers", S_IRWXU, tc358765_debug.dir,
+			dssdev, &tc358765_debug_fops);
+	return 0;
+}
+
+static void tc358765_uninitialize_debugfs(void)
+{
+	if (tc358765_debug.dir)
+		debugfs_remove_recursive(tc358765_debug.dir);
+	tc358765_debug.dir = NULL;
+	tc358765_debug.dev = NULL;
+}
+
+#else
+static int tc358765_initialize_debugfs(struct omap_dss_device *dssdev)
+{
+	return 0;
+}
+
+static void tc358765_uninitialize_debugfs(void)
+{
+}
+#endif
+
+static struct tc358765_board_data *get_board_data(struct omap_dss_device
+								*dssdev)
+{
+	return (struct tc358765_board_data *)dssdev->data;
+}
+
+static void tc358765_get_timings(struct omap_dss_device *dssdev,
+		struct omap_video_timings *timings)
+{
+	*timings = dssdev->panel.timings;
+}
+
+static void tc358765_set_timings(struct omap_dss_device *dssdev,
+		struct omap_video_timings *timings)
+{
+	dev_info(&dssdev->dev, "set_timings() not implemented\n");
+}
+
+static int tc358765_check_timings(struct omap_dss_device *dssdev,
+		struct omap_video_timings *timings)
+{
+	if (unlikely(!timings)) {
+		WARN(true, "%s: timings NULL pointer was passed\n", __func__);
+		return -EINVAL;
+	}
+
+	if (tc358765_timings.x_res != timings->x_res ||
+			tc358765_timings.y_res != timings->y_res ||
+			tc358765_timings.pixel_clock != timings->pixel_clock ||
+			tc358765_timings.hsw != timings->hsw ||
+			tc358765_timings.hfp != timings->hfp ||
+			tc358765_timings.hbp != timings->hbp ||
+			tc358765_timings.vsw != timings->vsw ||
+			tc358765_timings.vfp != timings->vfp ||
+			tc358765_timings.vbp != timings->vbp)
+		return -EINVAL;
+
+	return 0;
+}
+
+static void tc358765_get_resolution(struct omap_dss_device *dssdev,
+		u16 *xres, u16 *yres)
+{
+	*xres = tc358765_timings.x_res;
+	*yres = tc358765_timings.y_res;
+}
+
+static int tc358765_hw_reset(struct omap_dss_device *dssdev)
+{
+
+	if (dssdev = NULL || dssdev->reset_gpio = -1)
+		return 0;
+
+	gpio_set_value(dssdev->reset_gpio, 1);
+	udelay(200);
+	/* reset the panel */
+	gpio_set_value(dssdev->reset_gpio, 0);
+	/* assert reset */
+	udelay(200);
+	gpio_set_value(dssdev->reset_gpio, 1);
+	/* wait after releasing reset */
+	msleep(200);
+
+	return 0;
+}
+
+static void tc358765_probe_pdata(struct tc358765_data *d2d,
+		const struct tc358765_board_data *pdata)
+{
+	d2d->pin_config = pdata->pin_config;
+}
+
+static int tc358765_probe(struct omap_dss_device *dssdev)
+{
+	struct tc358765_data *d2d;
+	int r = 0;
+
+	dev_dbg(&dssdev->dev, "tc358765_probe\n");
+
+	dssdev->panel.dsi_pix_fmt = OMAP_DSS_DSI_FMT_RGB888;
+	tc358765_timings = dssdev->panel.timings;
+
+	d2d = kzalloc(sizeof(*d2d), GFP_KERNEL);
+	if (!d2d) {
+		r = -ENOMEM;
+		goto err;
+	}
+
+	d2d->dssdev = dssdev;
+
+	mutex_init(&d2d->lock);
+
+	dev_set_drvdata(&dssdev->dev, d2d);
+
+	if (dssdev->data) {
+		const struct tc358765_board_data *pdata = dssdev->data;
+
+		tc358765_probe_pdata(d2d, pdata);
+	} else {
+		return -ENODEV;
+	}
+
+	/* channel0 used for video packets */
+	r = omap_dsi_request_vc(dssdev, &d2d->channel0);
+	if (r) {
+		dev_err(&dssdev->dev, "failed to get virtual channel0\n");
+		goto err;
+	}
+
+	r = omap_dsi_set_vc_id(dssdev, d2d->channel0, 0);
+	if (r) {
+		dev_err(&dssdev->dev, "failed to set VC_ID0\n");
+		goto err_ch0;
+	}
+
+	/* channel1 used for registers access in LP mode */
+	r = omap_dsi_request_vc(dssdev, &d2d->channel1);
+	if (r) {
+		dev_err(&dssdev->dev, "failed to get virtual channel1\n");
+		goto err_ch0;
+	}
+
+	r = omap_dsi_set_vc_id(dssdev, d2d->channel1, 0);
+	if (r) {
+		dev_err(&dssdev->dev, "failed to set VC_ID1\n");
+		goto err_ch1;
+	}
+	r = tc358765_initialize_debugfs(dssdev);
+	if (r)
+		dev_warn(&dssdev->dev, "failed to create sysfs files\n");
+
+	dev_dbg(&dssdev->dev, "tc358765_probe done\n");
+	return 0;
+
+err_ch1:
+	omap_dsi_release_vc(dssdev, d2d->channel1);
+err_ch0:
+	omap_dsi_release_vc(dssdev, d2d->channel0);
+err:
+	mutex_destroy(&d2d->lock);
+	kfree(d2d);
+	return r;
+}
+
+static void tc358765_remove(struct omap_dss_device *dssdev)
+{
+	struct tc358765_data *d2d = dev_get_drvdata(&dssdev->dev);
+
+	tc358765_uninitialize_debugfs();
+
+	omap_dsi_release_vc(dssdev, d2d->channel0);
+	omap_dsi_release_vc(dssdev, d2d->channel1);
+	mutex_destroy(&d2d->lock);
+
+	kfree(d2d);
+}
+
+static int tc358765_init_ppi(struct omap_dss_device *dssdev)
+{
+	u32 go_cnt, sure_cnt, val = 0;
+	u8 lanes = 0;
+	int ret = 0;
+	struct tc358765_board_data *board_data = get_board_data(dssdev);
+	const int *pins = board_data->pin_config.pins;
+
+	/*
+	 * This register setting is required only if host wishes to
+	 * perform DSI read transactions
+	 */
+	go_cnt = (board_data->lp_time * 5 - 3) / 4;
+	sure_cnt = DIV_ROUND_UP(board_data->lp_time * 3, 2);
+	val = FLD_MOD(val, go_cnt, 26, 16);
+	val = FLD_MOD(val, sure_cnt, 10, 0);
+	ret |= tc358765_write_register(dssdev, PPI_TX_RX_TA, val);
+
+	/* SYSLPTX Timing Generation Counter */
+	ret |= tc358765_write_register(dssdev, PPI_LPTXTIMECNT,
+					board_data->lp_time);
+
+	/* D*S_CLRSIPOCOUNT = [(THS-SETTLE + THS-ZERO) /
+					HS_byte_clock_period ] */
+
+	if ((pins[0] & 1) || (pins[1] & 1))
+		lanes |= (1 << 0);
+
+	if ((pins[2] & 1) || (pins[3] & 1)) {
+		lanes |= (1 << 1);
+		ret |= tc358765_write_register(dssdev, PPI_D0S_CLRSIPOCOUNT,
+							board_data->clrsipo);
+	}
+	if ((pins[4] & 1) || (pins[5] & 1)) {
+		lanes |= (1 << 2);
+		ret |= tc358765_write_register(dssdev, PPI_D1S_CLRSIPOCOUNT,
+							board_data->clrsipo);
+	}
+	if ((pins[6] & 1) || (pins[7] & 1)) {
+		lanes |= (1 << 3);
+		ret |= tc358765_write_register(dssdev, PPI_D2S_CLRSIPOCOUNT,
+							board_data->clrsipo);
+	}
+	if ((pins[8] & 1) || (pins[9] & 1)) {
+		lanes |= (1 << 4);
+		ret |= tc358765_write_register(dssdev, PPI_D3S_CLRSIPOCOUNT,
+							board_data->clrsipo);
+	}
+
+	ret |= tc358765_write_register(dssdev, PPI_LANEENABLE, lanes);
+	ret |= tc358765_write_register(dssdev, DSI_LANEENABLE, lanes);
+
+	return ret;
+}
+
+static int tc358765_init_video_timings(struct omap_dss_device *dssdev)
+{
+	u32 val;
+	struct tc358765_board_data *board_data = get_board_data(dssdev);
+	int ret;
+	ret = tc358765_read_register(dssdev, VPCTRL, &val);
+	if (ret < 0) {
+		dev_warn(&dssdev->dev,
+			"couldn't access VPCTRL, going on with reset value\n");
+		val = 0;
+	}
+
+	if (dssdev->ctrl.pixel_size = 18) {
+		/* Magic Square FRC available for RGB666 only */
+		val = FLD_MOD(val, board_data->msf, 0, 0);
+		val = FLD_MOD(val, 0, 8, 8);
+	} else {
+		val = FLD_MOD(val, 1, 8, 8);
+	}
+
+	val = FLD_MOD(val, board_data->vtgen, 4, 4);
+	val = FLD_MOD(val, board_data->evtmode, 5, 5);
+	val = FLD_MOD(val, board_data->vsdelay, 31, 20);
+
+	ret = tc358765_write_register(dssdev, VPCTRL, val);
+
+	ret |= tc358765_write_register(dssdev, HTIM1,
+		(tc358765_timings.hbp << 16) | tc358765_timings.hsw);
+	ret |= tc358765_write_register(dssdev, HTIM2,
+		((tc358765_timings.hfp << 16) | tc358765_timings.x_res));
+	ret |= tc358765_write_register(dssdev, VTIM1,
+		((tc358765_timings.vbp << 16) |	tc358765_timings.vsw));
+	ret |= tc358765_write_register(dssdev, VTIM2,
+		((tc358765_timings.vfp << 16) |	tc358765_timings.y_res));
+	return ret;
+}
+
+static int tc358765_write_init_config(struct omap_dss_device *dssdev)
+{
+	struct tc358765_board_data *board_data = get_board_data(dssdev);
+	u32 val;
+	int r;
+
+	/* HACK: dummy read: if we read via DSI, first reads always fail */
+	tc358765_read_register(dssdev, DSI_INTSTATUS, &val);
+
+	r = tc358765_init_ppi(dssdev);
+	if (r) {
+		dev_err(&dssdev->dev, "failed to initialize PPI layer\n");
+		return r;
+	}
+
+	r = tc358765_write_register(dssdev, PPI_STARTPPI, 0x1);
+	if (r) {
+		dev_err(&dssdev->dev, "failed to start PPI-TX\n");
+		return r;
+	}
+
+	r = tc358765_write_register(dssdev, DSI_STARTDSI, 0x1);
+	if (r) {
+		dev_err(&dssdev->dev, "failed to start DSI-RX\n");
+		return r;
+	}
+
+	/* reset LVDS-PHY */
+	tc358765_write_register(dssdev, LVPHY0, (1 << 22));
+	mdelay(2);
+
+	r = tc358765_read_register(dssdev, LVPHY0, &val);
+	if (r < 0) {
+		dev_warn(&dssdev->dev, "couldn't access LVPHY0, going on with reset value\n");
+		val = 0;
+	}
+	val = FLD_MOD(val, 0, LV_RST_E, LV_RST_B);
+	val = FLD_MOD(val, board_data->lv_is, LV_IS_E, LV_IS_B);
+	val = FLD_MOD(val, board_data->lv_nd, LV_ND_E, LV_ND_B);
+	r = tc358765_write_register(dssdev, LVPHY0, val);
+
+	if (r) {
+		dev_err(&dssdev->dev, "failed to initialize LVDS-PHY\n");
+		return r;
+	}
+
+	r = tc358765_init_video_timings(dssdev);
+
+	if (r) {
+		dev_err(&dssdev->dev, "failed to initialize video path layer\n");
+		return r;
+	}
+
+	r = tc358765_read_register(dssdev, LVCFG, &val);
+	if (r < 0) {
+		dev_warn(&dssdev->dev,
+			"couldn't access LVCFG, going on with reset value\n");
+		val = 0;
+	}
+
+	val = FLD_MOD(val, board_data->pclkdiv, 9, 8);
+	val = FLD_MOD(val, board_data->pclksel, 11, 10);
+	val = FLD_MOD(val, board_data->lvdlink, 1, 1);
+	/* enable LVDS transmitter */
+	val = FLD_MOD(val, 1, 0, 0);
+	r = tc358765_write_register(dssdev, LVCFG, val);
+	if (r) {
+		dev_err(&dssdev->dev, "failed to start LVDS transmitter\n");
+		return r;
+	}
+
+	/* Issue a soft reset to LCD Controller for a clean start */
+	r = tc358765_write_register(dssdev, SYSRST, (1 << 2));
+	/* commit video configuration */
+	r |= tc358765_write_register(dssdev, VFUEN, 0x1);
+	if (r)
+		dev_err(&dssdev->dev, "failed to latch video timings\n");
+	return r;
+}
+
+static int tc358765_power_on(struct omap_dss_device *dssdev)
+{
+	struct tc358765_data *d2d = dev_get_drvdata(&dssdev->dev);
+	int r;
+
+	/* At power on the first vsync has not been received yet */
+
+	dev_dbg(&dssdev->dev, "power_on\n");
+
+	if (dssdev->platform_enable)
+		dssdev->platform_enable(dssdev);
+
+	r = omapdss_dsi_configure_pins(dssdev, &d2d->pin_config);
+	if (r) {
+		dev_err(&dssdev->dev, "failed to configure DSI pins\n");
+		goto err_disp_enable;
+	};
+
+	omapdss_dsi_set_size(dssdev, dssdev->panel.timings.x_res,
+					dssdev->panel.timings.y_res);
+	omapdss_dsi_set_pixel_format(dssdev, dssdev->panel.dsi_pix_fmt);
+	omapdss_dsi_set_operation_mode(dssdev, dssdev->panel.dsi_mode);
+	omapdss_dsi_set_timings(dssdev, &dssdev->panel.timings);
+	omapdss_dsi_set_videomode_timings(dssdev,
+					&dssdev->panel.dsi_vm_timings);
+
+	r = omapdss_dsi_set_clocks(dssdev, 187200000, 10000000);
+	if (r) {
+		dev_err(&dssdev->dev, "failed to set HS and LP clocks\n");
+		goto err_disp_enable;
+	}
+
+	r = omapdss_dsi_display_enable(dssdev);
+	if (r) {
+		dev_err(&dssdev->dev, "failed to enable DSI\n");
+		goto err_disp_enable;
+	}
+
+	/* reset tc358765 bridge */
+	tc358765_hw_reset(dssdev);
+
+	/*turn on HS clock to bring up bridge i2c slave */
+	omapdss_dsi_vc_enable_hs(dssdev, d2d->channel0, true);
+
+	/* configure D2L chip DSI-RX configuration registers */
+
+	r = tc358765_write_init_config(dssdev);
+	if (r)
+		goto err_write_init;
+
+	r = dsi_enable_video_output(dssdev, d2d->channel0);
+
+	dev_dbg(&dssdev->dev, "power_on done\n");
+
+	return r;
+
+err_write_init:
+	omapdss_dsi_display_disable(dssdev, false, false);
+err_disp_enable:
+	if (dssdev->platform_disable)
+		dssdev->platform_disable(dssdev);
+
+	return r;
+}
+
+static void tc358765_power_off(struct omap_dss_device *dssdev)
+{
+	struct tc358765_data *d2d = dev_get_drvdata(&dssdev->dev);
+
+	dsi_disable_video_output(dssdev, d2d->channel0);
+	dsi_disable_video_output(dssdev, d2d->channel1);
+
+	omapdss_dsi_display_disable(dssdev, false, false);
+
+	if (dssdev->platform_disable)
+		dssdev->platform_disable(dssdev);
+}
+
+static void tc358765_disable(struct omap_dss_device *dssdev)
+{
+	struct tc358765_data *d2d = dev_get_drvdata(&dssdev->dev);
+
+	dev_dbg(&dssdev->dev, "disable\n");
+
+	if (dssdev->state = OMAP_DSS_DISPLAY_ACTIVE) {
+		mutex_lock(&d2d->lock);
+		dsi_bus_lock(dssdev);
+
+		tc358765_power_off(dssdev);
+
+		dsi_bus_unlock(dssdev);
+		mutex_unlock(&d2d->lock);
+	}
+	dssdev->state = OMAP_DSS_DISPLAY_DISABLED;
+}
+
+static int tc358765_enable(struct omap_dss_device *dssdev)
+{
+	struct tc358765_data *d2d = dev_get_drvdata(&dssdev->dev);
+	int r = 0;
+
+	dev_dbg(&dssdev->dev, "enable\n");
+
+	if (dssdev->state != OMAP_DSS_DISPLAY_DISABLED)
+		return -EINVAL;
+
+	mutex_lock(&d2d->lock);
+	dsi_bus_lock(dssdev);
+
+	r = tc358765_power_on(dssdev);
+
+	dsi_bus_unlock(dssdev);
+
+	if (r) {
+		dev_dbg(&dssdev->dev, "enable failed\n");
+		dssdev->state = OMAP_DSS_DISPLAY_DISABLED;
+	} else {
+		dssdev->state = OMAP_DSS_DISPLAY_ACTIVE;
+	}
+
+	mutex_unlock(&d2d->lock);
+
+	return r;
+}
+
+static struct omap_dss_driver tc358765_driver = {
+	.probe		= tc358765_probe,
+	.remove		= tc358765_remove,
+
+	.enable		= tc358765_enable,
+	.disable	= tc358765_disable,
+
+	.get_resolution	= tc358765_get_resolution,
+	.get_recommended_bpp = omapdss_default_get_recommended_bpp,
+
+	.get_timings	= tc358765_get_timings,
+	.set_timings	= tc358765_set_timings,
+	.check_timings	= tc358765_check_timings,
+
+	.driver         = {
+		.name   = "tc358765",
+		.owner  = THIS_MODULE,
+	},
+};
+
+static int tc358765_i2c_probe(struct i2c_client *client,
+				   const struct i2c_device_id *id)
+{
+	tc358765_i2c = kzalloc(sizeof(*tc358765_i2c), GFP_KERNEL);
+	if (tc358765_i2c = NULL)
+		return -ENOMEM;
+
+	/* store i2c_client pointer on private data structure */
+	tc358765_i2c->client = client;
+
+	/* store private data structure pointer on i2c_client structure */
+	i2c_set_clientdata(client, tc358765_i2c);
+
+	/* init mutex */
+	mutex_init(&tc358765_i2c->xfer_lock);
+	dev_err(&client->dev, "D2L i2c initialized\n");
+
+	return 0;
+}
+
+/* driver remove function */
+static int __exit tc358765_i2c_remove(struct i2c_client *client)
+{
+	/* remove client data */
+	i2c_set_clientdata(client, NULL);
+
+	/* destroy mutex */
+	mutex_destroy(&tc358765_i2c->xfer_lock);
+
+	/* free private data memory */
+	kfree(tc358765_i2c);
+
+	return 0;
+}
+
+static const struct i2c_device_id tc358765_i2c_idtable[] = {
+	{"tc358765_i2c_driver", 0},
+	{},
+};
+
+static struct i2c_driver tc358765_i2c_driver = {
+	.probe = tc358765_i2c_probe,
+	.remove = __exit_p(tc358765_i2c_remove),
+	.id_table = tc358765_i2c_idtable,
+	.driver = {
+		   .name  = "d2l",
+		   .owner = THIS_MODULE,
+	},
+};
+
+
+static int __init tc358765_init(void)
+{
+	int r;
+	tc358765_i2c = NULL;
+	r = i2c_add_driver(&tc358765_i2c_driver);
+	if (r < 0) {
+		printk(KERN_WARNING "d2l i2c driver registration failed\n");
+		return r;
+	}
+
+	omap_dss_register_driver(&tc358765_driver);
+	return 0;
+}
+
+static void __exit tc358765_exit(void)
+{
+	omap_dss_unregister_driver(&tc358765_driver);
+	i2c_del_driver(&tc358765_i2c_driver);
+}
+
+module_init(tc358765_init);
+module_exit(tc358765_exit);
+
+MODULE_AUTHOR("Tomi Valkeinen <tomi.valkeinen@ti.com>");
+MODULE_DESCRIPTION("TC358765 DSI-2-LVDS Driver");
+MODULE_LICENSE("GPL");
diff --git a/drivers/video/omap2/displays/panel-tc358765.h b/drivers/video/omap2/displays/panel-tc358765.h
new file mode 100644
index 0000000..ffc105d
--- /dev/null
+++ b/drivers/video/omap2/displays/panel-tc358765.h
@@ -0,0 +1,170 @@
+/*
+ * Header for DSI-to-LVDS bridge driver
+ *
+ * Copyright (C) 2012 Texas Instruments Inc
+ * Author: Tomi Valkeinen <tomi.valkeinen@ti.com>
+ * Author: Sergii Kibrik <sergiikibrik@ti.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.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef __PANEL_TC358765_H__
+#define __PANEL_TC358765_H__
+
+/* DSI D-PHY Layer Registers */
+#define	D0W_DPHYCONTTX		0x0004		/* Data Lane 0 DPHY TX */
+#define	CLW_DPHYCONTRX		0x0020		/* Clock Lane DPHY RX */
+#define	D0W_DPHYCONTRX		0x0024		/* Data Land 0 DPHY Rx */
+#define	D1W_DPHYCONTRX		0x0028		/* Data Lane 1 DPHY Rx */
+#define	D2W_DPHYCONTRX		0x002c		/* Data Lane 2 DPHY Rx */
+#define	D3W_DPHYCONTRX		0x0030		/* Data Lane 3 DPHY Rx */
+#define	COM_DPHYCONTRX		0x0038		/* DPHY Rx Common */
+#define	CLW_CNTRL		0x0040		/* Clock Lane */
+#define	D0W_CNTRL		0x0044		/* Data Lane 0 */
+#define	D1W_CNTRL		0x0048		/* Data Lane 1 */
+#define	D2W_CNTRL		0x004c		/* Data Lane 2 */
+#define	D3W_CNTRL		0x0050		/* Data Lane 3 */
+#define	DFTMODE_CNTRL		0x0054		/* DFT Mode */
+
+/* DSI PPI Layer Registers */
+#define	PPI_STARTPPI		0x0104		/* Start control bit */
+#define	PPI_BUSYPPI		0x0108			/* Busy bit */
+#define	PPI_LINEINITCNT		0x0110		/* Line In initialization */
+#define	PPI_LPTXTIMECNT		0x0114		/* LPTX timing signal */
+#define	PPI_LANEENABLE		0x0134		/* Lane Enable */
+#define	PPI_TX_RX_TA		0x013c		/* BTA timing param */
+#define	PPI_CLS_ATMR		0x0140		/* Analog timer fcn */
+#define	PPI_D0S_ATMR		0x0144		/* Analog timer fcn Lane 0 */
+#define	PPI_D1S_ATMR		0x0148		/* Analog timer fcn Lane 1 */
+#define	PPI_D2S_ATMR		0x014c		/* Analog timer fcn Lane 2 */
+#define	PPI_D3S_ATMR		0x0150		/* Analog timer fcn Lane 3 */
+#define	PPI_D0S_CLRSIPOCOUNT	0x0164		/* Assertion timer Lane 0 */
+#define	PPI_D1S_CLRSIPOCOUNT	0x0168		/* Assertion timer Lane 1 */
+#define	PPI_D2S_CLRSIPOCOUNT	0x016c		/* Assertion timer Lane 1 */
+#define	PPI_D3S_CLRSIPOCOUNT	0x0170		/* Assertion timer Lane 1 */
+#define CLS_PRE			0x0180		/* PHY IO cntr */
+#define D0S_PRE			0x0184		/* PHY IO cntr */
+#define D1S_PRE			0x0188		/* PHY IO cntr */
+#define D2S_PRE			0x018c		/* PHY IO cntr */
+#define D3S_PRE			0x0190		/* PHY IO cntr */
+#define CLS_PREP		0x01a0		/* PHY IO cntr */
+#define D0S_PREP		0x01a4		/* PHY IO cntr */
+#define D1S_PREP		0x01a8		/* PHY IO cntr */
+#define D2S_PREP		0x01ac		/* PHY IO cntr */
+#define D3S_PREP		0x01b0		/* PHY IO cntr */
+#define CLS_ZERO		0x01c0		/* PHY IO cntr */
+#define	D0S_ZERO		0x01c4		/* PHY IO cntr */
+#define	D1S_ZERO		0x01c8		/* PHY IO cntr */
+#define	D2S_ZERO		0x01cc		/* PHY IO cntr */
+#define	D3S_ZERO		0x01d0		/* PHY IO cntr */
+#define PPI_CLRFLG		0x01e0		/* PRE cntrs */
+#define PPI_CLRSIPO		0x01e4		/* Clear SIPO */
+#define PPI_HSTimeout		0x01f0		/* HS RX timeout */
+#define PPI_HSTimeoutEnable	0x01f4		/* Enable HS Rx Timeout */
+
+/* DSI Protocol Layer Registers */
+#define DSI_STARTDSI		0x0204		/* DSI TX start bit */
+#define DSI_BUSYDSI		0x0208		/* DSI busy bit */
+#define DSI_LANEENABLE		0x0210		/* Lane enable */
+#define DSI_LANESTATUS0		0x0214		/* HS Rx mode */
+#define DSI_LANESTATUS1		0x0218		/* ULPS or STOP state */
+#define DSI_INTSTATUS		0x0220		/* Interrupt status */
+#define DSI_INTMASK		0x0224		/* Interrupt mask */
+#define DSI_INTCLR		0x0228		/* Interrupt clear */
+#define DSI_LPTXTO		0x0230		/* LP Tx Cntr */
+
+/* DSI General Registers */
+#define	DSIERRCNT		0x0300		/* DSI Error Count */
+
+/* DSI Application Layer Registers */
+#define APLCTRL			0x0400		/* Application Layer Cntrl */
+#define RDPKTLN			0x0404		/* Packet length */
+
+/* Video Path Registers */
+#define	VPCTRL			0x0450		/* Video Path */
+#define HTIM1			0x0454		/* Horizontal Timing */
+#define HTIM2			0x0458		/* Horizontal Timing */
+#define VTIM1			0x045c		/* Vertical Timing */
+#define VTIM2			0x0460		/* Vertical Timing */
+#define VFUEN			0x0464		/* Video Frame Timing */
+
+
+/* LVDS Registers - LVDS Mux Input */
+#define LVMX0003		0x0480		/* Bit 0 to 3*/
+#define LVMX0407		0x0484		/* Bit 4 to 7 */
+#define LVMX0811		0x0488		/* Bit 8 to 11 */
+#define LVMX1215		0x048c		/* Bit 12 to 15 */
+#define LVMX1619		0x0490		/* Bit 16 to 19 */
+#define LVMX2023		0x0494		/* Bit 20 to 23 */
+#define LVMX2427		0x0498		/* Bit 24 to 27 */
+
+#define LVCFG			0x049c		/* LVDS Config */
+#define	LVPHY0			0x04a0		/* LVDS PHY Reg 0 */
+#define	LVPHY1			0x04a1		/* LVDS PHY Reg 1 */
+
+/* LVDS PHY Register 0 (LVPHY0) entries */
+#define LV_RST_B		22		/* LV PHY reset */
+#define LV_RST_E		22
+#define LV_IS_B			14		/* Charge pump current control */
+#define LV_IS_E			15		/* pin for PLL portion */
+#define LV_ND_B			0		/* Frequency Range Select */
+#define LV_ND_E			4
+
+/* System Registers */
+#define SYSSTAT			0x0500		/* System Status */
+#define SYSRST			0x0504		/* System Reset */
+
+/* GPIO Registers */
+#define GPIOC			0x0520		/* GPIO Control */
+#define GPIOO			0x0520		/* GPIO Output */
+#define GPIOI			0x0520		/* GPIO Input */
+
+/* I2C Registers */
+#define I2CTIMCTRL		0x0540
+#define I2CMADDR		0x0544
+#define WDATAQ			0x0548
+#define RDATAQ			0x054C
+
+/* Chip Revision Registers */
+#define IDREG			0x0580		/* Chip and Revision ID */
+
+/* Debug Register */
+#define DEBUG00			0x05a0		/* Debug */
+#define DEBUG01			0x05a4		/* LVDS Data */
+
+/*DSI DCS commands */
+#define DCS_READ_NUM_ERRORS     0x05
+#define DCS_READ_POWER_MODE     0x0a
+#define DCS_READ_MADCTL         0x0b
+#define DCS_READ_PIXEL_FORMAT   0x0c
+#define DCS_RDDSDR              0x0f
+#define DCS_SLEEP_IN            0x10
+#define DCS_SLEEP_OUT           0x11
+#define DCS_DISPLAY_OFF         0x28
+#define DCS_DISPLAY_ON          0x29
+#define DCS_COLUMN_ADDR         0x2a
+#define DCS_PAGE_ADDR           0x2b
+#define DCS_MEMORY_WRITE        0x2c
+#define DCS_TEAR_OFF            0x34
+#define DCS_TEAR_ON             0x35
+#define DCS_MEM_ACC_CTRL        0x36
+#define DCS_PIXEL_FORMAT        0x3a
+#define DCS_BRIGHTNESS          0x51
+#define DCS_CTRL_DISPLAY        0x53
+#define DCS_WRITE_CABC          0x55
+#define DCS_READ_CABC           0x56
+#define DCS_GET_ID1             0xda
+#define DCS_GET_ID2             0xdb
+#define DCS_GET_ID3             0xdc
+
+#endif
diff --git a/include/video/omap-panel-tc358765.h b/include/video/omap-panel-tc358765.h
new file mode 100644
index 0000000..c58e081
--- /dev/null
+++ b/include/video/omap-panel-tc358765.h
@@ -0,0 +1,53 @@
+/*
+ * Header for DSI-to-LVDS bridge driver
+ *
+ * Copyright (C) 2012 Texas Instruments Inc
+ * Author: Sergii Kibrik <sergiikibrik@ti.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.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef __VIDEO_TC358765_BOARD_DATA_H__
+#define __VIDEO_TC358765_BOARD_DATA_H__
+
+/**
+ * struct tc358765_board_data - represent DSI-to-LVDS bridge configuration
+ * @lp_time: Timing Generation Counter
+ * @clrsipo: CLRSIPO counter (one value for all lanes)
+ * @lv_is: charge pump control pin
+ * @lv_nd: Feed Back Divider Ratio
+ * @pclkdiv: PCLK Divide Option
+ * @pclksel: PCLK Selection: HSRCK/HbyteHSClkx2/ByteHsClk
+ * @vsdelay: VSYNC Delay
+ * @lvdlink: is single or dual link
+ * @vtgen: drive video timing signals by the on-chip Video Timing Gen module
+ * @msf: enable/disable Magic Square
+ * @evtmode: event/pulse mode of video timing information transmission
+ * @pin_config: DSI pin configuration
+*/
+struct tc358765_board_data {
+	u16	lp_time;
+	u8	clrsipo;
+	u8	lv_is;
+	u8	lv_nd;
+	u8	pclkdiv;
+	u8	pclksel;
+	u16	vsdelay;
+	bool	lvdlink;
+	bool	vtgen;
+	bool	msf;
+	bool	evtmode;
+	struct omap_dsi_pin_config pin_config;
+};
+
+#endif
-- 
1.7.9.5


^ permalink raw reply related

* Re: [PATCH 1/1] OMAP4: DSS: Add panel for Blaze Tablet boards
From: Andi Shyti @ 2013-02-07 14:59 UTC (permalink / raw)
  To: Ruslan Bilovol
  Cc: tomi.valkeinen, FlorianSchandinat, linux-fbdev, linux-kernel,
	linux-omap
In-Reply-To: <1360248051-16541-2-git-send-email-ruslan.bilovol@ti.com>

Hi,

> TC358765 is DSI-to-LVDS transmitter from Toshiba, used in
> OMAP44XX Blaze Tablet and Blaze Tablet2 boards.

I had a really fast look and I have few comments

> +static int tc358765_read_register(struct omap_dss_device *dssdev,
> +					u16 reg, u32 *val)
> +{
> +	int ret = 0;
> +	pm_runtime_get_sync(&dssdev->dev);
> +	/* I2C is preferred way of reading, but fall back to DSI
> +	 * if I2C didn't got initialized
> +	*/
> +	if (tc358765_i2c)
> +		ret = tc358765_i2c_read(reg, val);
> +	else
> +		ret = tc358765_dsi_read(dssdev, reg, val);
> +	pm_runtime_put_sync(&dssdev->dev);
> +
> +	return ret;
> +}
> +
> +static int tc358765_write_register(struct omap_dss_device *dssdev, u16 reg,
> +		u32 value)
> +{
> +	struct tc358765_data *d2d = dev_get_drvdata(&dssdev->dev);
> +	u8 buf[6];
> +	int r;
> +
> +	buf[0] = (reg >> 0) & 0xff;
> +	buf[1] = (reg >> 8) & 0xff;
> +	buf[2] = (value >> 0) & 0xff;
> +	buf[3] = (value >> 8) & 0xff;
> +	buf[4] = (value >> 16) & 0xff;
> +	buf[5] = (value >> 24) & 0xff;
> +
> +	r = dsi_vc_generic_write_nosync(dssdev, d2d->channel1, buf, 6);
> +	if (r)
> +		dev_err(&dssdev->dev, "reg write reg(%x) val(%x) failed: %d\n",
> +			       reg, value, r);
> +	return r;
> +}
> +
> +/****************************
> +********* DEBUG *************
> +****************************/
> +#ifdef CONFIG_TC358765_DEBUG
> +static int tc358765_write_register_i2c(u16 reg, u32 val)
> +{
> +	int ret = -ENODEV;
> +	unsigned char buf[6];
> +	struct i2c_msg msg;
> +
> +	if (!tc358765_i2c) {
> +		dev_err(tc358765_debug.dev, "%s: I2C not initilized\n",
> +							__func__);
> +		return ret;
> +	}
> +
> +	buf[0] = (reg >> 8) & 0xff;
> +	buf[1] = (reg >> 0) & 0xff;
> +	buf[2] = (val >> 0) & 0xff;
> +	buf[3] = (val >> 8) & 0xff;
> +	buf[4] = (val >> 16) & 0xff;
> +	buf[5] = (val >> 24) & 0xff;
> +	msg.addr = tc358765_i2c->client->addr;
> +	msg.len = sizeof(buf);
> +	msg.flags = 0;
> +	msg.buf = buf;
> +
> +	mutex_lock(&tc358765_i2c->xfer_lock);
> +	ret = i2c_transfer(tc358765_i2c->client->adapter, &msg, 1);
> +	mutex_unlock(&tc358765_i2c->xfer_lock);
> +
> +	if (ret != 1)
> +		return ret;
> +	return 0;
> +}

What about using smbus?

> +
> +	if (copy_from_user(&buf, ubuf, size))
> +		return -EFAULT;
> +
> +	buf[size-1] = '\0';
> +	if (sscanf(buf, "%s %x", name, &value) != 2) {
> +		dev_err(dev, "%s: unable to parse input\n", __func__);
> +		return -1;
> +	}
> +
> +	if (!tc358765_i2c) {
> +		dev_warn(dev,
> +			"failed to write register: I2C not initialized\n");
> +		return -ENODEV;
> +	}
> +
> +	reg_count = sizeof(tc358765_regs) / sizeof(tc358765_regs[0]);
> +	for (i = 0; i < reg_count; i++) {
> +		if (!strcmp(name, tc358765_regs[i].name)) {
> +			if (!(tc358765_regs[i].perm & A_WO)) {
> +				dev_err(dev, "%s is write-protected\n", name);
> +				return -EACCES;
> +			}
> +
> +			error = tc358765_write_register_i2c(
> +				tc358765_regs[i].reg, value);
> +			if (error) {
> +				dev_err(dev, "%s: failed to write %s\n",
> +					__func__, name);
> +				return -1;

Could avoid returning -1 instead of returning a correct errno?

> +	gpio_set_value(dssdev->reset_gpio, 1);
> +	udelay(200);
> +	/* reset the panel */
> +	gpio_set_value(dssdev->reset_gpio, 0);
> +	/* assert reset */
> +	udelay(200);
> +	gpio_set_value(dssdev->reset_gpio, 1);
> +	/* wait after releasing reset */
> +	msleep(200);

I invite you to have a look at

Documentation/timers/timers-howto.txt

> +	/* reset LVDS-PHY */
> +	tc358765_write_register(dssdev, LVPHY0, (1 << 22));
> +	mdelay(2);

You should give me a really good reason for using mdelay.

> +	if (r) {
> +		dev_err(&dssdev->dev, "failed to configure DSI pins\n");
> +		goto err_disp_enable;
> +	};
        ^^^

???

> +static int tc358765_i2c_probe(struct i2c_client *client,
> +				   const struct i2c_device_id *id)
> +{
> +	tc358765_i2c = kzalloc(sizeof(*tc358765_i2c), GFP_KERNEL);

what about devm_kzalloc?


^ permalink raw reply

* [PATCH v2 0/3] ARM: at91/avr32/atmel_lcdfb: remove cpu_is macros
From: Johan Hovold @ 2013-02-07 15:31 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20130205201145.GE30595@game.jcrosoft.org>

Here's a v2 replacing the last two patches in the series (the first three are
unchanged since the first post). If preferred, I can repost the whole series
when these patches have been acked.

v2:
 - use clkdev to handle the lcdc bus clock
 - use -lcdfb suffix for device ids (e.g. "at91sam9g45-lcdfb")

Thanks,
Johan

Johan Hovold (3):
  ARM: at91/avr32/atmel_lcdfb: add bus-clock entry
  atmel_lcdfb: move lcdcon2 register access to compute_hozval
  ARM: at91/avr32/atmel_lcdfb: add platform device-id table

 arch/arm/mach-at91/at91sam9261.c         |   2 +
 arch/arm/mach-at91/at91sam9261_devices.c |   6 +-
 arch/arm/mach-at91/at91sam9263.c         |   1 +
 arch/arm/mach-at91/at91sam9263_devices.c |   2 +-
 arch/arm/mach-at91/at91sam9g45.c         |   2 +
 arch/arm/mach-at91/at91sam9g45_devices.c |   6 +-
 arch/arm/mach-at91/at91sam9rl.c          |   1 +
 arch/arm/mach-at91/at91sam9rl_devices.c  |   2 +-
 arch/avr32/mach-at32ap/at32ap700x.c      |   6 +-
 drivers/video/atmel_lcdfb.c              | 120 +++++++++++++++++++++++--------
 include/video/atmel_lcdc.h               |   4 +-
 11 files changed, 117 insertions(+), 35 deletions(-)

-- 
1.8.1.1


^ permalink raw reply

* [PATCH v2 1/3] ARM: at91/avr32/atmel_lcdfb: add bus-clock entry
From: Johan Hovold @ 2013-02-07 15:31 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1360251118-12715-1-git-send-email-jhovold@gmail.com>

Add hclk entry for the atmel_lcdfb bus clock.

On at91sam9261, at91sam9g10 and at32ap the bus clock has to be enabled
as well as the peripheral clock. Add the appropriate lookup entries to
these SOCs and fake clocks to the SOCs that do not use it.

This allows us to get rid of the conditional enabling of the clocks in
the driver which relied on the cpu_is macros.

Tested on at91sam9263 and at91sam9g45, compile-tested for other
AT91-SOCs, and untested for AVR32.

Signed-off-by: Johan Hovold <jhovold@gmail.com>
---
 arch/arm/mach-at91/at91sam9261.c    |  1 +
 arch/arm/mach-at91/at91sam9263.c    |  1 +
 arch/arm/mach-at91/at91sam9g45.c    |  1 +
 arch/arm/mach-at91/at91sam9rl.c     |  1 +
 arch/avr32/mach-at32ap/at32ap700x.c |  4 ++--
 drivers/video/atmel_lcdfb.c         | 23 ++++++++---------------
 6 files changed, 14 insertions(+), 17 deletions(-)

diff --git a/arch/arm/mach-at91/at91sam9261.c b/arch/arm/mach-at91/at91sam9261.c
index 2998a08..5838f12 100644
--- a/arch/arm/mach-at91/at91sam9261.c
+++ b/arch/arm/mach-at91/at91sam9261.c
@@ -169,6 +169,7 @@ static struct clk *periph_clocks[] __initdata = {
 };
 
 static struct clk_lookup periph_clocks_lookups[] = {
+	CLKDEV_CON_DEV_ID("hclk", "atmel_lcdfb.0", &hck1),
 	CLKDEV_CON_DEV_ID("spi_clk", "atmel_spi.0", &spi0_clk),
 	CLKDEV_CON_DEV_ID("spi_clk", "atmel_spi.1", &spi1_clk),
 	CLKDEV_CON_DEV_ID("t0_clk", "atmel_tcb.0", &tc0_clk),
diff --git a/arch/arm/mach-at91/at91sam9263.c b/arch/arm/mach-at91/at91sam9263.c
index 4618776..53d6123 100644
--- a/arch/arm/mach-at91/at91sam9263.c
+++ b/arch/arm/mach-at91/at91sam9263.c
@@ -190,6 +190,7 @@ static struct clk_lookup periph_clocks_lookups[] = {
 	CLKDEV_CON_DEV_ID("pclk", "at91rm9200_ssc.1", &ssc1_clk),
 	CLKDEV_CON_DEV_ID("pclk", "fff98000.ssc", &ssc0_clk),
 	CLKDEV_CON_DEV_ID("pclk", "fff9c000.ssc", &ssc1_clk),
+	CLKDEV_CON_DEV_ID("hclk", "atmel_lcdfb.0", &lcdc_clk),
 	CLKDEV_CON_DEV_ID("mci_clk", "atmel_mci.0", &mmc0_clk),
 	CLKDEV_CON_DEV_ID("mci_clk", "atmel_mci.1", &mmc1_clk),
 	CLKDEV_CON_DEV_ID("spi_clk", "atmel_spi.0", &spi0_clk),
diff --git a/arch/arm/mach-at91/at91sam9g45.c b/arch/arm/mach-at91/at91sam9g45.c
index b6f1342..1def8c9 100644
--- a/arch/arm/mach-at91/at91sam9g45.c
+++ b/arch/arm/mach-at91/at91sam9g45.c
@@ -228,6 +228,7 @@ static struct clk_lookup periph_clocks_lookups[] = {
 	CLKDEV_CON_ID("hclk", &macb_clk),
 	/* One additional fake clock for ohci */
 	CLKDEV_CON_ID("ohci_clk", &uhphs_clk),
+	CLKDEV_CON_DEV_ID("hclk", "atmel_lcdfb.0", &lcdc_clk),
 	CLKDEV_CON_DEV_ID("ehci_clk", "atmel-ehci", &uhphs_clk),
 	CLKDEV_CON_DEV_ID("hclk", "atmel_usba_udc", &utmi_clk),
 	CLKDEV_CON_DEV_ID("pclk", "atmel_usba_udc", &udphs_clk),
diff --git a/arch/arm/mach-at91/at91sam9rl.c b/arch/arm/mach-at91/at91sam9rl.c
index eb98704..4cd4fa9 100644
--- a/arch/arm/mach-at91/at91sam9rl.c
+++ b/arch/arm/mach-at91/at91sam9rl.c
@@ -179,6 +179,7 @@ static struct clk *periph_clocks[] __initdata = {
 };
 
 static struct clk_lookup periph_clocks_lookups[] = {
+	CLKDEV_CON_DEV_ID("hclk", "atmel_lcdfb.0", &lcdc_clk),
 	CLKDEV_CON_DEV_ID("hclk", "atmel_usba_udc", &utmi_clk),
 	CLKDEV_CON_DEV_ID("pclk", "atmel_usba_udc", &udphs_clk),
 	CLKDEV_CON_DEV_ID("t0_clk", "atmel_tcb.0", &tc0_clk),
diff --git a/arch/avr32/mach-at32ap/at32ap700x.c b/arch/avr32/mach-at32ap/at32ap700x.c
index b323d8d..cd25b01 100644
--- a/arch/avr32/mach-at32ap/at32ap700x.c
+++ b/arch/avr32/mach-at32ap/at32ap700x.c
@@ -1453,7 +1453,7 @@ static struct resource atmel_lcdfb0_resource[] = {
 	},
 };
 DEFINE_DEV_DATA(atmel_lcdfb, 0);
-DEV_CLK(hck1, atmel_lcdfb0, hsb, 7);
+DEV_CLK(hclk, atmel_lcdfb0, hsb, 7);
 static struct clk atmel_lcdfb0_pixclk = {
 	.name		= "lcdc_clk",
 	.dev		= &atmel_lcdfb0_device.dev,
@@ -2246,7 +2246,7 @@ static __initdata struct clk *init_clocks[] = {
 	&atmel_twi0_pclk,
 	&atmel_mci0_pclk,
 #if defined(CONFIG_CPU_AT32AP7000) || defined(CONFIG_CPU_AT32AP7002)
-	&atmel_lcdfb0_hck1,
+	&atmel_lcdfb0_hclk,
 	&atmel_lcdfb0_pixclk,
 #endif
 	&ssc0_pclk,
diff --git a/drivers/video/atmel_lcdfb.c b/drivers/video/atmel_lcdfb.c
index 93910e3..86cafae 100644
--- a/drivers/video/atmel_lcdfb.c
+++ b/drivers/video/atmel_lcdfb.c
@@ -820,15 +820,13 @@ static int __init atmel_lcdfb_init_fbinfo(struct atmel_lcdfb_info *sinfo)
 
 static void atmel_lcdfb_start_clock(struct atmel_lcdfb_info *sinfo)
 {
-	if (sinfo->bus_clk)
-		clk_enable(sinfo->bus_clk);
+	clk_enable(sinfo->bus_clk);
 	clk_enable(sinfo->lcdc_clk);
 }
 
 static void atmel_lcdfb_stop_clock(struct atmel_lcdfb_info *sinfo)
 {
-	if (sinfo->bus_clk)
-		clk_disable(sinfo->bus_clk);
+	clk_disable(sinfo->bus_clk);
 	clk_disable(sinfo->lcdc_clk);
 }
 
@@ -887,13 +885,10 @@ static int __init atmel_lcdfb_probe(struct platform_device *pdev)
 	info->fix = atmel_lcdfb_fix;
 
 	/* Enable LCDC Clocks */
-	if (cpu_is_at91sam9261() || cpu_is_at91sam9g10()
-	 || cpu_is_at32ap7000()) {
-		sinfo->bus_clk = clk_get(dev, "hck1");
-		if (IS_ERR(sinfo->bus_clk)) {
-			ret = PTR_ERR(sinfo->bus_clk);
-			goto free_info;
-		}
+	sinfo->bus_clk = clk_get(dev, "hclk");
+	if (IS_ERR(sinfo->bus_clk)) {
+		ret = PTR_ERR(sinfo->bus_clk);
+		goto free_info;
 	}
 	sinfo->lcdc_clk = clk_get(dev, "lcdc_clk");
 	if (IS_ERR(sinfo->lcdc_clk)) {
@@ -1054,8 +1049,7 @@ stop_clk:
 	atmel_lcdfb_stop_clock(sinfo);
 	clk_put(sinfo->lcdc_clk);
 put_bus_clk:
-	if (sinfo->bus_clk)
-		clk_put(sinfo->bus_clk);
+	clk_put(sinfo->bus_clk);
 free_info:
 	framebuffer_release(info);
 out:
@@ -1080,8 +1074,7 @@ static int __exit atmel_lcdfb_remove(struct platform_device *pdev)
 	unregister_framebuffer(info);
 	atmel_lcdfb_stop_clock(sinfo);
 	clk_put(sinfo->lcdc_clk);
-	if (sinfo->bus_clk)
-		clk_put(sinfo->bus_clk);
+	clk_put(sinfo->bus_clk);
 	fb_dealloc_cmap(&info->cmap);
 	free_irq(sinfo->irq_base, info);
 	iounmap(sinfo->mmio);
-- 
1.8.1.1


^ permalink raw reply related

* [PATCH v2 2/3] atmel_lcdfb: move lcdcon2 register access to compute_hozval
From: Johan Hovold @ 2013-02-07 15:31 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1360251118-12715-1-git-send-email-jhovold@gmail.com>

Pass atmel_lcd_info structure to compute_hozval and only do the register
access on SOCs that actually use it.

This will also simplify the removal of the cpu_is macros.

Signed-off-by: Johan Hovold <jhovold@gmail.com>
---
 drivers/video/atmel_lcdfb.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/video/atmel_lcdfb.c b/drivers/video/atmel_lcdfb.c
index 86cafae..c707130 100644
--- a/drivers/video/atmel_lcdfb.c
+++ b/drivers/video/atmel_lcdfb.c
@@ -193,14 +193,17 @@ static struct fb_fix_screeninfo atmel_lcdfb_fix __initdata = {
 	.accel		= FB_ACCEL_NONE,
 };
 
-static unsigned long compute_hozval(unsigned long xres, unsigned long lcdcon2)
+static unsigned long compute_hozval(struct atmel_lcdfb_info *sinfo,
+							unsigned long xres)
 {
+	unsigned long lcdcon2;
 	unsigned long value;
 
 	if (!(cpu_is_at91sam9261() || cpu_is_at91sam9g10()
 		|| cpu_is_at32ap7000()))
 		return xres;
 
+	lcdcon2 = lcdc_readl(sinfo, ATMEL_LCDC_LCDCON2);
 	value = xres;
 	if ((lcdcon2 & ATMEL_LCDC_DISTYPE) != ATMEL_LCDC_DISTYPE_TFT) {
 		/* STN display */
@@ -590,8 +593,7 @@ static int atmel_lcdfb_set_par(struct fb_info *info)
 	lcdc_writel(sinfo, ATMEL_LCDC_TIM2, value);
 
 	/* Horizontal value (aka line size) */
-	hozval_linesz = compute_hozval(info->var.xres,
-					lcdc_readl(sinfo, ATMEL_LCDC_LCDCON2));
+	hozval_linesz = compute_hozval(sinfo, info->var.xres);
 
 	/* Display size */
 	value = (hozval_linesz - 1) << ATMEL_LCDC_HOZVAL_OFFSET;
-- 
1.8.1.1


^ permalink raw reply related

* [PATCH v2 3/3] ARM: at91/avr32/atmel_lcdfb: add platform device-id table
From: Johan Hovold @ 2013-02-07 15:31 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1360251118-12715-1-git-send-email-jhovold@gmail.com>

Add platform device-id table in order to identify the controller and
determine its configuration.

The currently used configuration parameters are:

have_alt_pixclock
 - SOC uses an alternate pixel-clock calculation formula (at91sam9g45
   non-ES)

have_hozval
 - SOC has a HOZVAL field in LCDFRMCFG which is used to determine the
   linesize for STN displays (at91sam9261, at921sam9g10 and at32ap)

have_intensity_bit
 - SOC uses IBGR:555 rather than BGR:565 16-bit pixel layout
   (at91sam9261, at91sam9263 and at91sam9rl)

This allows us to remove all the remaining uses of cpu_is macros from
the driver.

Tested on at91sam9263 and at91sam9g45, compile-tested for other
AT91-SOCs, and untested for AVR32.

Signed-off-by: Johan Hovold <jhovold@gmail.com>
---
 arch/arm/mach-at91/at91sam9261.c         |  3 +-
 arch/arm/mach-at91/at91sam9261_devices.c |  6 ++-
 arch/arm/mach-at91/at91sam9263.c         |  2 +-
 arch/arm/mach-at91/at91sam9263_devices.c |  2 +-
 arch/arm/mach-at91/at91sam9g45.c         |  3 +-
 arch/arm/mach-at91/at91sam9g45_devices.c |  6 ++-
 arch/arm/mach-at91/at91sam9rl.c          |  2 +-
 arch/arm/mach-at91/at91sam9rl_devices.c  |  2 +-
 arch/avr32/mach-at32ap/at32ap700x.c      |  2 +
 drivers/video/atmel_lcdfb.c              | 89 ++++++++++++++++++++++++++++----
 include/video/atmel_lcdc.h               |  4 +-
 11 files changed, 102 insertions(+), 19 deletions(-)

diff --git a/arch/arm/mach-at91/at91sam9261.c b/arch/arm/mach-at91/at91sam9261.c
index 5838f12..0204f4c 100644
--- a/arch/arm/mach-at91/at91sam9261.c
+++ b/arch/arm/mach-at91/at91sam9261.c
@@ -169,7 +169,8 @@ static struct clk *periph_clocks[] __initdata = {
 };
 
 static struct clk_lookup periph_clocks_lookups[] = {
-	CLKDEV_CON_DEV_ID("hclk", "atmel_lcdfb.0", &hck1),
+	CLKDEV_CON_DEV_ID("hclk", "at91sam9261-lcdfb.0", &hck1),
+	CLKDEV_CON_DEV_ID("hclk", "at91sam9g10-lcdfb.0", &hck1),
 	CLKDEV_CON_DEV_ID("spi_clk", "atmel_spi.0", &spi0_clk),
 	CLKDEV_CON_DEV_ID("spi_clk", "atmel_spi.1", &spi1_clk),
 	CLKDEV_CON_DEV_ID("t0_clk", "atmel_tcb.0", &tc0_clk),
diff --git a/arch/arm/mach-at91/at91sam9261_devices.c b/arch/arm/mach-at91/at91sam9261_devices.c
index 92e0f86..629ea5f 100644
--- a/arch/arm/mach-at91/at91sam9261_devices.c
+++ b/arch/arm/mach-at91/at91sam9261_devices.c
@@ -488,7 +488,6 @@ static struct resource lcdc_resources[] = {
 };
 
 static struct platform_device at91_lcdc_device = {
-	.name		= "atmel_lcdfb",
 	.id		= 0,
 	.dev		= {
 				.dma_mask		= &lcdc_dmamask,
@@ -505,6 +504,11 @@ void __init at91_add_device_lcdc(struct atmel_lcdfb_info *data)
 		return;
 	}
 
+	if (cpu_is_at91sam9g10())
+		at91_lcdc_device.name = "at91sam9g10-lcdfb";
+	else
+		at91_lcdc_device.name = "at91sam9261-lcdfb";
+
 #if defined(CONFIG_FB_ATMEL_STN)
 	at91_set_A_periph(AT91_PIN_PB0, 0);     /* LCDVSYNC */
 	at91_set_A_periph(AT91_PIN_PB1, 0);     /* LCDHSYNC */
diff --git a/arch/arm/mach-at91/at91sam9263.c b/arch/arm/mach-at91/at91sam9263.c
index 53d6123..c0cbb81 100644
--- a/arch/arm/mach-at91/at91sam9263.c
+++ b/arch/arm/mach-at91/at91sam9263.c
@@ -190,7 +190,7 @@ static struct clk_lookup periph_clocks_lookups[] = {
 	CLKDEV_CON_DEV_ID("pclk", "at91rm9200_ssc.1", &ssc1_clk),
 	CLKDEV_CON_DEV_ID("pclk", "fff98000.ssc", &ssc0_clk),
 	CLKDEV_CON_DEV_ID("pclk", "fff9c000.ssc", &ssc1_clk),
-	CLKDEV_CON_DEV_ID("hclk", "atmel_lcdfb.0", &lcdc_clk),
+	CLKDEV_CON_DEV_ID("hclk", "at91sam9263-lcdfb.0", &lcdc_clk),
 	CLKDEV_CON_DEV_ID("mci_clk", "atmel_mci.0", &mmc0_clk),
 	CLKDEV_CON_DEV_ID("mci_clk", "atmel_mci.1", &mmc1_clk),
 	CLKDEV_CON_DEV_ID("spi_clk", "atmel_spi.0", &spi0_clk),
diff --git a/arch/arm/mach-at91/at91sam9263_devices.c b/arch/arm/mach-at91/at91sam9263_devices.c
index ed666f5..858c8aa 100644
--- a/arch/arm/mach-at91/at91sam9263_devices.c
+++ b/arch/arm/mach-at91/at91sam9263_devices.c
@@ -848,7 +848,7 @@ static struct resource lcdc_resources[] = {
 };
 
 static struct platform_device at91_lcdc_device = {
-	.name		= "atmel_lcdfb",
+	.name		= "at91sam9263-lcdfb",
 	.id		= 0,
 	.dev		= {
 				.dma_mask		= &lcdc_dmamask,
diff --git a/arch/arm/mach-at91/at91sam9g45.c b/arch/arm/mach-at91/at91sam9g45.c
index 1def8c9..b4968aa 100644
--- a/arch/arm/mach-at91/at91sam9g45.c
+++ b/arch/arm/mach-at91/at91sam9g45.c
@@ -228,7 +228,8 @@ static struct clk_lookup periph_clocks_lookups[] = {
 	CLKDEV_CON_ID("hclk", &macb_clk),
 	/* One additional fake clock for ohci */
 	CLKDEV_CON_ID("ohci_clk", &uhphs_clk),
-	CLKDEV_CON_DEV_ID("hclk", "atmel_lcdfb.0", &lcdc_clk),
+	CLKDEV_CON_DEV_ID("hclk", "at91sam9g45-lcdfb.0", &lcdc_clk),
+	CLKDEV_CON_DEV_ID("hclk", "at91sam9g45es-lcdfb.0", &lcdc_clk),
 	CLKDEV_CON_DEV_ID("ehci_clk", "atmel-ehci", &uhphs_clk),
 	CLKDEV_CON_DEV_ID("hclk", "atmel_usba_udc", &utmi_clk),
 	CLKDEV_CON_DEV_ID("pclk", "atmel_usba_udc", &udphs_clk),
diff --git a/arch/arm/mach-at91/at91sam9g45_devices.c b/arch/arm/mach-at91/at91sam9g45_devices.c
index 827c9f2..fe626d4 100644
--- a/arch/arm/mach-at91/at91sam9g45_devices.c
+++ b/arch/arm/mach-at91/at91sam9g45_devices.c
@@ -981,7 +981,6 @@ static struct resource lcdc_resources[] = {
 };
 
 static struct platform_device at91_lcdc_device = {
-	.name		= "atmel_lcdfb",
 	.id		= 0,
 	.dev		= {
 				.dma_mask		= &lcdc_dmamask,
@@ -997,6 +996,11 @@ void __init at91_add_device_lcdc(struct atmel_lcdfb_info *data)
 	if (!data)
 		return;
 
+	if (cpu_is_at91sam9g45es())
+		at91_lcdc_device.name = "at91sam9g45es-lcdfb";
+	else
+		at91_lcdc_device.name = "at91sam9g45-lcdfb";
+
 	at91_set_A_periph(AT91_PIN_PE0, 0);	/* LCDDPWR */
 
 	at91_set_A_periph(AT91_PIN_PE2, 0);	/* LCDCC */
diff --git a/arch/arm/mach-at91/at91sam9rl.c b/arch/arm/mach-at91/at91sam9rl.c
index 4cd4fa9..3de3e04 100644
--- a/arch/arm/mach-at91/at91sam9rl.c
+++ b/arch/arm/mach-at91/at91sam9rl.c
@@ -179,7 +179,7 @@ static struct clk *periph_clocks[] __initdata = {
 };
 
 static struct clk_lookup periph_clocks_lookups[] = {
-	CLKDEV_CON_DEV_ID("hclk", "atmel_lcdfb.0", &lcdc_clk),
+	CLKDEV_CON_DEV_ID("hclk", "at91sam9rl-lcdfb.0", &lcdc_clk),
 	CLKDEV_CON_DEV_ID("hclk", "atmel_usba_udc", &utmi_clk),
 	CLKDEV_CON_DEV_ID("pclk", "atmel_usba_udc", &udphs_clk),
 	CLKDEV_CON_DEV_ID("t0_clk", "atmel_tcb.0", &tc0_clk),
diff --git a/arch/arm/mach-at91/at91sam9rl_devices.c b/arch/arm/mach-at91/at91sam9rl_devices.c
index ddf223f..352468f 100644
--- a/arch/arm/mach-at91/at91sam9rl_devices.c
+++ b/arch/arm/mach-at91/at91sam9rl_devices.c
@@ -514,7 +514,7 @@ static struct resource lcdc_resources[] = {
 };
 
 static struct platform_device at91_lcdc_device = {
-	.name		= "atmel_lcdfb",
+	.name		= "at91sam9rl-lcdfb",
 	.id		= 0,
 	.dev		= {
 				.dma_mask		= &lcdc_dmamask,
diff --git a/arch/avr32/mach-at32ap/at32ap700x.c b/arch/avr32/mach-at32ap/at32ap700x.c
index cd25b01..7c2f668 100644
--- a/arch/avr32/mach-at32ap/at32ap700x.c
+++ b/arch/avr32/mach-at32ap/at32ap700x.c
@@ -1530,6 +1530,8 @@ at32_add_device_lcdc(unsigned int id, struct atmel_lcdfb_info *data,
 	memcpy(info, data, sizeof(struct atmel_lcdfb_info));
 	info->default_monspecs = monspecs;
 
+	pdev->name = "at32ap-lcdfb";
+
 	platform_device_register(pdev);
 	return pdev;
 
diff --git a/drivers/video/atmel_lcdfb.c b/drivers/video/atmel_lcdfb.c
index c707130..b68bfe6 100644
--- a/drivers/video/atmel_lcdfb.c
+++ b/drivers/video/atmel_lcdfb.c
@@ -34,6 +34,77 @@
 #define ATMEL_LCDC_DMA_BURST_LEN	8	/* words */
 #define ATMEL_LCDC_FIFO_SIZE		512	/* words */
 
+struct atmel_lcdfb_config {
+	bool have_alt_pixclock;
+	bool have_hozval;
+	bool have_intensity_bit;
+};
+
+static struct atmel_lcdfb_config at91sam9261_config = {
+	.have_hozval		= true,
+	.have_intensity_bit	= true,
+};
+
+static struct atmel_lcdfb_config at91sam9263_config = {
+	.have_intensity_bit	= true,
+};
+
+static struct atmel_lcdfb_config at91sam9g10_config = {
+	.have_hozval		= true,
+};
+
+static struct atmel_lcdfb_config at91sam9g45_config = {
+	.have_alt_pixclock	= true,
+};
+
+static struct atmel_lcdfb_config at91sam9g45es_config = {
+};
+
+static struct atmel_lcdfb_config at91sam9rl_config = {
+	.have_intensity_bit	= true,
+};
+
+static struct atmel_lcdfb_config at32ap_config = {
+	.have_hozval		= true,
+};
+
+static const struct platform_device_id atmel_lcdfb_devtypes[] = {
+	{
+		.name = "at91sam9261-lcdfb",
+		.driver_data = (unsigned long)&at91sam9261_config,
+	}, {
+		.name = "at91sam9263-lcdfb",
+		.driver_data = (unsigned long)&at91sam9263_config,
+	}, {
+		.name = "at91sam9g10-lcdfb",
+		.driver_data = (unsigned long)&at91sam9g10_config,
+	}, {
+		.name = "at91sam9g45-lcdfb",
+		.driver_data = (unsigned long)&at91sam9g45_config,
+	}, {
+		.name = "at91sam9g45es-lcdfb",
+		.driver_data = (unsigned long)&at91sam9g45es_config,
+	}, {
+		.name = "at91sam9rl-lcdfb",
+		.driver_data = (unsigned long)&at91sam9rl_config,
+	}, {
+		.name = "at32ap-lcdfb",
+		.driver_data = (unsigned long)&at32ap_config,
+	}, {
+		/* terminator */
+	}
+};
+
+static struct atmel_lcdfb_config *
+atmel_lcdfb_get_config(struct platform_device *pdev)
+{
+	unsigned long data;
+
+	data = platform_get_device_id(pdev)->driver_data;
+
+	return (struct atmel_lcdfb_config *)data;
+}
+
 #if defined(CONFIG_ARCH_AT91)
 #define	ATMEL_LCDFB_FBINFO_DEFAULT	(FBINFO_DEFAULT \
 					 | FBINFO_PARTIAL_PAN_OK \
@@ -199,8 +270,7 @@ static unsigned long compute_hozval(struct atmel_lcdfb_info *sinfo,
 	unsigned long lcdcon2;
 	unsigned long value;
 
-	if (!(cpu_is_at91sam9261() || cpu_is_at91sam9g10()
-		|| cpu_is_at32ap7000()))
+	if (!sinfo->config->have_hozval)
 		return xres;
 
 	lcdcon2 = lcdc_readl(sinfo, ATMEL_LCDC_LCDCON2);
@@ -426,7 +496,7 @@ static int atmel_lcdfb_check_var(struct fb_var_screeninfo *var,
 		break;
 	case 16:
 		/* Older SOCs use IBGR:555 rather than BGR:565. */
-		if (sinfo->have_intensity_bit)
+		if (sinfo->config->have_intensity_bit)
 			var->green.length = 5;
 		else
 			var->green.length = 6;
@@ -534,7 +604,7 @@ static int atmel_lcdfb_set_par(struct fb_info *info)
 	/* Now, the LCDC core... */
 
 	/* Set pixel clock */
-	if (cpu_is_at91sam9g45() && !cpu_is_at91sam9g45es())
+	if (sinfo->config->have_alt_pixclock)
 		pix_factor = 1;
 
 	clk_value_khz = clk_get_rate(sinfo->lcdc_clk) / 1000;
@@ -685,7 +755,7 @@ static int atmel_lcdfb_setcolreg(unsigned int regno, unsigned int red,
 
 	case FB_VISUAL_PSEUDOCOLOR:
 		if (regno < 256) {
-			if (sinfo->have_intensity_bit) {
+			if (sinfo->config->have_intensity_bit) {
 				/* old style I+BGR:555 */
 				val  = ((red   >> 11) & 0x001f);
 				val |= ((green >>  6) & 0x03e0);
@@ -873,10 +943,9 @@ static int __init atmel_lcdfb_probe(struct platform_device *pdev)
 	}
 	sinfo->info = info;
 	sinfo->pdev = pdev;
-	if (cpu_is_at91sam9261() || cpu_is_at91sam9263() ||
-							cpu_is_at91sam9rl()) {
-		sinfo->have_intensity_bit = true;
-	}
+	sinfo->config = atmel_lcdfb_get_config(pdev);
+	if (!sinfo->config)
+		goto free_info;
 
 	strcpy(info->fix.id, sinfo->pdev->name);
 	info->flags = ATMEL_LCDFB_FBINFO_DEFAULT;
@@ -1145,7 +1214,7 @@ static struct platform_driver atmel_lcdfb_driver = {
 	.remove		= __exit_p(atmel_lcdfb_remove),
 	.suspend	= atmel_lcdfb_suspend,
 	.resume		= atmel_lcdfb_resume,
-
+	.id_table	= atmel_lcdfb_devtypes,
 	.driver		= {
 		.name	= "atmel_lcdfb",
 		.owner	= THIS_MODULE,
diff --git a/include/video/atmel_lcdc.h b/include/video/atmel_lcdc.h
index 8deb226..0f5a2fc 100644
--- a/include/video/atmel_lcdc.h
+++ b/include/video/atmel_lcdc.h
@@ -31,6 +31,7 @@
 #define ATMEL_LCDC_WIRING_BGR	0
 #define ATMEL_LCDC_WIRING_RGB	1
 
+struct atmel_lcdfb_config;
 
  /* LCD Controller info data structure, stored in device platform_data */
 struct atmel_lcdfb_info {
@@ -61,7 +62,8 @@ struct atmel_lcdfb_info {
 	void (*atmel_lcdfb_power_control)(int on);
 	struct fb_monspecs	*default_monspecs;
 	u32			pseudo_palette[16];
-	bool			have_intensity_bit;
+
+	struct atmel_lcdfb_config *config;
 };
 
 #define ATMEL_LCDC_DMABADDR1	0x00
-- 
1.8.1.1


^ permalink raw reply related

* Re: Unmerged patches for 3.9
From: Andrew Morton @ 2013-02-07 21:41 UTC (permalink / raw)
  To: linux-fbdev
In-Reply-To: <CAK9yfHwOanv4n3gE-E1gopyTaEjL0km1qMmGLGM2h2WLSMppYA@mail.gmail.com>

On Thu, 7 Feb 2013 12:34:16 +0530
Sachin Kamat <sachin.kamat@linaro.org> wrote:

> Hi Florian,
> 
> I have the following 'Acked' patches missing in your tree.
> 
> https://patchwork.kernel.org/patch/1864681/
> https://patchwork.kernel.org/patch/1923041/
> https://patchwork.kernel.org/patch/1926501/
> 
> Could you please pick them up in your tree as they have been pending
> almost since a couple of months now.

Those aren't terribly urgent patches so I'll skip them for now.



It appears that Florian is still offline.  If anyone has urgent fbdev
patches (ie: bugfixes) then please send them to me ASAP and I'll start
working them towards 3.8 and perhaps -stable.

Thanks.

^ permalink raw reply

* Re: [PATCH 2/3 v2] fb: udlfb: fix hang at disconnect
From: Dave Airlie @ 2013-02-08  4:07 UTC (permalink / raw)
  To: Alexander Holler
  Cc: Greg KH, Andrew Morton, linux-kernel, linux-fbdev,
	Florian Tobias Schandinat, Bernie Thompson, Steve Glendinning,
	stable
In-Reply-To: <5111431D.8070906@ahsoftware.de>

>
> But I've just switched to udl (instead of udlfb) and will see if I can fix
> the bugs there to make it usable as a console. udl is a rewrite of udlfb
> with some additional features (e.g. drm), so hopefully fixing the remaining
> problems there will require less work.

I may have fixed the major udl problem with being a console, patch was
posted to dri-devel yesterday, and I've pushed it into -next.

Dave.

^ permalink raw reply

* [PATCH RESEND v2 1/2] drivers/video: fsl-diu-fb: fix pixel formats for 24 and 16 bpp
From: Anatolij Gustschin @ 2013-02-08  7:35 UTC (permalink / raw)
  To: linux-fbdev

Framebuffer colors for 24 and 16 bpp are currently wrong. The order
of the color component arguments in the MAKE_PF() is not natural
and causes some confusion. The generated pixel format values for 24
and 16 bpp depths do not much the values in the comments.

Fix the macro arguments to be in the natural RGB order and adjust
the arguments for all depths to generate correct pixel format values
(equal to the values mentioned in the comments).

Signed-off-by: Anatolij Gustschin <agust@denx.de>
Cc: Timur Tabi <timur@tabi.org>
Acked-by: Timur Tabi <timur@freescale.com>
---
 Changes in v2:
  - none, only added Timur's Acked-by

 This patch is not a very important fix but it would be good to
 have it in v3.8

 drivers/video/fsl-diu-fb.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/video/fsl-diu-fb.c b/drivers/video/fsl-diu-fb.c
index d3fc92e..1ca32c5 100644
--- a/drivers/video/fsl-diu-fb.c
+++ b/drivers/video/fsl-diu-fb.c
@@ -944,7 +944,7 @@ static u32 fsl_diu_get_pixel_format(unsigned int bits_per_pixel)
 #define PF_COMP_0_MASK		0x0000000F
 #define PF_COMP_0_SHIFT		0
 
-#define MAKE_PF(alpha, red, blue, green, size, c0, c1, c2, c3) \
+#define MAKE_PF(alpha, red, green, blue, size, c0, c1, c2, c3) \
 	cpu_to_le32(PF_BYTE_F | (alpha << PF_ALPHA_C_SHIFT) | \
 	(blue << PF_BLUE_C_SHIFT) | (green << PF_GREEN_C_SHIFT) | \
 	(red << PF_RED_C_SHIFT) | (c3 << PF_COMP_3_SHIFT) | \
@@ -954,10 +954,10 @@ static u32 fsl_diu_get_pixel_format(unsigned int bits_per_pixel)
 	switch (bits_per_pixel) {
 	case 32:
 		/* 0x88883316 */
-		return MAKE_PF(3, 2, 0, 1, 3, 8, 8, 8, 8);
+		return MAKE_PF(3, 2, 1, 0, 3, 8, 8, 8, 8);
 	case 24:
 		/* 0x88082219 */
-		return MAKE_PF(4, 0, 1, 2, 2, 0, 8, 8, 8);
+		return MAKE_PF(4, 0, 1, 2, 2, 8, 8, 8, 0);
 	case 16:
 		/* 0x65053118 */
 		return MAKE_PF(4, 2, 1, 0, 1, 5, 6, 5, 0);
-- 
1.7.5.4


^ permalink raw reply related

* [PATCH RESEND v2 2/2] drivers/video: fsl-diu-fb: fix bugs in interrupt handling
From: Anatolij Gustschin @ 2013-02-08  7:35 UTC (permalink / raw)
  To: linux-fbdev

Since commit f74de500 "drivers/video: fsl-diu-fb: streamline
enabling of interrupts" the interrupt handling in the driver
is broken. Enabling diu interrupt causes an interrupt storm and
results in system lockup.

The cookie for the interrupt handler function passed to request_irq()
is wrong (it must be a pointer to the diu struct, and not the address
of the pointer to the diu struct). As a result the interrupt handler
can not read diu registers and acknowledge the interrupt. Fix cookie
arguments for request_irq() and free_irq().

Registering the diu interrupt handler in probe() must happen before
install_fb() calls since this function registers framebuffer devices
and if fbcon tries to take over framebuffer after registering a frame
buffer device, it will call fb_open of the diu driver and enable the
interrupts. At this time the diu interrupt handler must be registered
already.

Disabling the interrupts in fsl_diu_release() must happen only if all
other AOIs are closed. Otherwise closing an overlay plane will disable
the interrupts even if the primary frame buffer plane is opened. Add
an appropriate check in the release function.

Signed-off-by: Anatolij Gustschin <agust@denx.de>
Cc: Timur Tabi <timur@tabi.org>
---
 This patch fixes a regression, it should be included in v3.8 since
 without it all mpc512x based boards (with DIU support enabled) do not
 boot
 
Changes in v2:
 - don't add can_handle_irq flag and do the interrupt registration
   before registering frame buffers instead, as pointed by Timur

 drivers/video/fsl-diu-fb.c |   58 +++++++++++++++++++++++++-------------------
 1 files changed, 33 insertions(+), 25 deletions(-)

diff --git a/drivers/video/fsl-diu-fb.c b/drivers/video/fsl-diu-fb.c
index 1ca32c5..d33e872 100644
--- a/drivers/video/fsl-diu-fb.c
+++ b/drivers/video/fsl-diu-fb.c
@@ -1232,6 +1232,16 @@ static int fsl_diu_ioctl(struct fb_info *info, unsigned int cmd,
 	return 0;
 }
 
+static inline void fsl_diu_enable_interrupts(struct fsl_diu_data *data)
+{
+	u32 int_mask = INT_UNDRUN; /* enable underrun detection */
+
+	if (IS_ENABLED(CONFIG_NOT_COHERENT_CACHE))
+		int_mask |= INT_VSYNC; /* enable vertical sync */
+
+	clrbits32(&data->diu_reg->int_mask, int_mask);
+}
+
 /* turn on fb if count = 1
  */
 static int fsl_diu_open(struct fb_info *info, int user)
@@ -1251,19 +1261,7 @@ static int fsl_diu_open(struct fb_info *info, int user)
 		if (res < 0)
 			mfbi->count--;
 		else {
-			struct fsl_diu_data *data = mfbi->parent;
-
-#ifdef CONFIG_NOT_COHERENT_CACHE
-			/*
-			 * Enable underrun detection and vertical sync
-			 * interrupts.
-			 */
-			clrbits32(&data->diu_reg->int_mask,
-				  INT_UNDRUN | INT_VSYNC);
-#else
-			/* Enable underrun detection */
-			clrbits32(&data->diu_reg->int_mask, INT_UNDRUN);
-#endif
+			fsl_diu_enable_interrupts(mfbi->parent);
 			fsl_diu_enable_panel(info);
 		}
 	}
@@ -1283,9 +1281,18 @@ static int fsl_diu_release(struct fb_info *info, int user)
 	mfbi->count--;
 	if (mfbi->count = 0) {
 		struct fsl_diu_data *data = mfbi->parent;
+		bool disable = true;
+		int i;
 
-		/* Disable interrupts */
-		out_be32(&data->diu_reg->int_mask, 0xffffffff);
+		/* Disable interrupts only if all AOIs are closed */
+		for (i = 0; i < NUM_AOIS; i++) {
+			struct mfb_info *mi = data->fsl_diu_info[i].par;
+
+			if (mi->count)
+				disable = false;
+		}
+		if (disable)
+			out_be32(&data->diu_reg->int_mask, 0xffffffff);
 		fsl_diu_disable_panel(info);
 	}
 
@@ -1614,14 +1621,6 @@ static int __devinit fsl_diu_probe(struct platform_device *pdev)
 	out_be32(&data->diu_reg->desc[1], data->dummy_ad.paddr);
 	out_be32(&data->diu_reg->desc[2], data->dummy_ad.paddr);
 
-	for (i = 0; i < NUM_AOIS; i++) {
-		ret = install_fb(&data->fsl_diu_info[i]);
-		if (ret) {
-			dev_err(&pdev->dev, "could not register fb %d\n", i);
-			goto error;
-		}
-	}
-
 	/*
 	 * Older versions of U-Boot leave interrupts enabled, so disable
 	 * all of them and clear the status register.
@@ -1630,12 +1629,21 @@ static int __devinit fsl_diu_probe(struct platform_device *pdev)
 	in_be32(&data->diu_reg->int_status);
 
 	ret = request_irq(data->irq, fsl_diu_isr, 0, "fsl-diu-fb",
-			  &data->diu_reg);
+			  data->diu_reg);
 	if (ret) {
 		dev_err(&pdev->dev, "could not claim irq\n");
 		goto error;
 	}
 
+	for (i = 0; i < NUM_AOIS; i++) {
+		ret = install_fb(&data->fsl_diu_info[i]);
+		if (ret) {
+			dev_err(&pdev->dev, "could not register fb %d\n", i);
+			free_irq(data->irq, data->diu_reg);
+			goto error;
+		}
+	}
+
 	sysfs_attr_init(&data->dev_attr.attr);
 	data->dev_attr.attr.name = "monitor";
 	data->dev_attr.attr.mode = S_IRUGO|S_IWUSR;
@@ -1667,7 +1675,7 @@ static int fsl_diu_remove(struct platform_device *pdev)
 	data = dev_get_drvdata(&pdev->dev);
 	disable_lcdc(&data->fsl_diu_info[0]);
 
-	free_irq(data->irq, &data->diu_reg);
+	free_irq(data->irq, data->diu_reg);
 
 	for (i = 0; i < NUM_AOIS; i++)
 		uninstall_fb(&data->fsl_diu_info[i]);
-- 
1.7.5.4


^ permalink raw reply related

* Re: [PATCH 2/3 v2] fb: udlfb: fix hang at disconnect
From: Alexander Holler @ 2013-02-08  9:53 UTC (permalink / raw)
  To: Dave Airlie
  Cc: Greg KH, Andrew Morton, linux-kernel, linux-fbdev,
	Florian Tobias Schandinat, Bernie Thompson, Steve Glendinning,
	stable
In-Reply-To: <CAPM=9tzNeQTXAmvZZ1RCmn6uTYHvweYKKnSPYAh7QYRGNMgeUw@mail.gmail.com>

Am 08.02.2013 05:07, schrieb Dave Airlie:
>>
>> But I've just switched to udl (instead of udlfb) and will see if I can fix
>> the bugs there to make it usable as a console. udl is a rewrite of udlfb
>> with some additional features (e.g. drm), so hopefully fixing the remaining
>> problems there will require less work.
>
> I may have fixed the major udl problem with being a console, patch was
> posted to dri-devel yesterday, and I've pushed it into -next.

Thanks a lot. I will check it.

Regards,

Alexander


^ permalink raw reply

* Re: Unmerged patches for 3.9
From: Sachin Kamat @ 2013-02-08 10:51 UTC (permalink / raw)
  To: linux-fbdev
In-Reply-To: <CAK9yfHwOanv4n3gE-E1gopyTaEjL0km1qMmGLGM2h2WLSMppYA@mail.gmail.com>

On 8 February 2013 03:10, Andrew Morton <akpm@linux-foundation.org> wrote:
> On Thu, 7 Feb 2013 12:34:16 +0530
> Sachin Kamat <sachin.kamat@linaro.org> wrote:
>
>> Hi Florian,
>>
>> I have the following 'Acked' patches missing in your tree.
>>
>> https://patchwork.kernel.org/patch/1864681/
>> https://patchwork.kernel.org/patch/1923041/
>> https://patchwork.kernel.org/patch/1926501/
>>
>> Could you please pick them up in your tree as they have been pending
>> almost since a couple of months now.
>
> Those aren't terribly urgent patches so I'll skip them for now.

Yes right. They need not go for 3.8. So would you be lining them up
for 3.9-rc1 instead?

>
> It appears that Florian is still offline.  If anyone has urgent fbdev
> patches (ie: bugfixes) then please send them to me ASAP and I'll start
> working them towards 3.8 and perhaps -stable.


-- 
With warm regards,
Sachin

^ permalink raw reply

* Re: [RFC v2 0/5] Common Display Framework
From: Tomi Valkeinen @ 2013-02-08 10:51 UTC (permalink / raw)
  To: Marcus Lorentzon
  Cc: Laurent Pinchart, Tomasz Figa, Thomas Petazzoni,
	linux-fbdev@vger.kernel.org, Philipp Zabel, Tom Gall,
	Ragesh Radhakrishnan, dri-devel@lists.freedesktop.org, Rob Clark,
	Kyungmin Park, sunil joshi, Benjamin Gaignard, Bryan Wu,
	Maxime Ripard, Vikas Sajjan, Sumit Semwal, Sebastien Guiriec,
	linux-media@vger.kernel.org
In-Reply-To: <510F8807.2020406@stericsson.com>

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

On 2013-02-04 12:05, Marcus Lorentzon wrote:

> As discussed at FOSDEM I will give DSI bus with full feature set a
> try.

Please do, but as a reminder I want to raise the issues I see with a DSI
bus:

- A device can be a child of only one bus. So if DSI is used only for
video, the device is a child of, say, i2c bus, and thus there's no DSI
bus. How to configure and use DSI in this case?

- If DSI is used for both control and video, we have two separate APIs
for the bus. What I mean here is that for the video-only case above, we
need a video-only-API for DSI. This API should contain all necessary
methods to configure DSI. But we need similar methods for the control
API also.

So, I hope you come up with some solution for this, but as I see it,
it's easily the most simple and clear option to have one video_source
style entity for the DSI bus itself, which is used for both control and
video.

 Tomi



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 899 bytes --]

^ permalink raw reply

* Re: [RFC PATCH 0/4] Common Display Framework-TF
From: Tomi Valkeinen @ 2013-02-08 11:40 UTC (permalink / raw)
  To: Tomasz Figa
  Cc: Laurent Pinchart, Tomasz Figa, dri-devel, linux-fbdev,
	linux-samsung-soc, kyungmin.park, m.szyprowski, Daniel Vetter,
	Marcus Lorentzon, rob, Vikas Sajjan, inki.dae, dh09.lee,
	ville.syrjala, s.nawrocki
In-Reply-To: <3272191.03RiBNfhoM@flatron>

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

Hi,

On 2013-02-03 21:17, Tomasz Figa wrote:
> Hi Laurent,
> 
> On Saturday 02 of February 2013 11:39:59 Laurent Pinchart wrote:
>> Hi Tomasz,
>>
>> Thank you for your RFC.
>>
>> On Wednesday 30 January 2013 16:38:59 Tomasz Figa wrote:

>>> Changes done to Tomi's edition of CDF:
>>>  - Replaced set_operation_mode, set_pixel_format and set_size video
>>>  source>  
>>>    operations with get_params display entity operation, as it was
>>>    originally in Laurent's version.
>>>    
>>>    We have discussed this matter on the mailing list and decided that
>>>    it
>>>    would be better to have the source ask the sink for its parameter
>>>    structure and do whatever appropriate with it.
>>
>> Could you briefly outline the rationale behind this ?
> 
> Display interfaces may be described by an arbitrary number of parameters. 
> Some will require just video timings, others like DSI will require a 
> significant number of additional bus-specific ones.
> 
> Separating all the parameters (all of which are usually set at the same 
> time) into a lot of ops setting single parameter will just add unnecessary 
> complexity.

I have nothing against combining the parameters that way. I think the
important thing here is just that we have to allow changing of the
parameters, whenever the display device needs that. So the bus
parameters cannot be a one time constant setting.

>> I'm wondering whether get_params could limit us if a device needs to
>> modify parameters at runtime. For instance a panel might need to change
>> clock frequencies or number of used data lane depending on various
>> runtime conditions. I don't have a real use case here, so it might just
>> be a bit of over-engineering.
> 
> Hmm, isn't it in the opposite direction (the user requests the display 
> interface to change, for example, video mode, which in turn reconfigures 
> the link and requests the panel to update its settings)?

Well, now, that's the question.

Let's consider a simplified case with DSI output from the SoC, and a DSI
panel. If the panel is a rather simple one, you could well call a method
in the API in the DSI output driver, which would do necessary
configuration and inform the panel driver to do any configuration it
needs to do, based on the parameters.

However, in my experience display devices, DSI devices in particular,
are often quite "interesting". If the control of the operation in
question is in the DSI output side, we are limited about what kind of
DSI panels we can use, as the DSI output driver has to know all the
tricks needed to make the panels work.

I'm having hard time trying to explain this, so let's try an example.
Consider the initial powering up of the bus. If the DSI output driver is
in control, something like the following probably happens:

- output driver asks for the parameters from the panel driver
- output driver programs the DSI output according to these parameters
- output driver informs the panel that the bus is now up and running

Then consider a totally invented case, but which perhaps describes the
problem with the above approach: The DSI panel requires the DSI bus
first to be started in HS mode, but with a certain predefined bus speed,
and only one lane used. This predefined bus setup is used to send
configuration commands to the panel hardware, and only after that can
you reconfigure the bus with proper speed and lanes.

That kind of thing is a bit difficult to manage with the DSI output
driver is in control. However, if the DSI panel driver is in control,
it's simple:

- panel driver calls config methods in the DSI output driver, setting
the predefined speed and one lane
- panel driver sends configuration commands to the panel
- panel driver calls config methods in the DSI output driver, setting
the final bus config

>>>    5. Mask of used data lanes (each bit represents single lane).
>>
>> From my experience with MIPI CSI (Camera Serial Interface, similar to
>> DSI) some transceivers can reroute data lanes internally. Is that true
>> for DSI as well ? In that case we would need a list of data lanes, not
>> just a mask.
> 
> Hmm, I don't remember at the moment, will have to look to the 
> specification. Exynos DSI master doesn't support such feature.

In OMAP you can configure the DSI pins quite freely. We have the
following struct:

struct omap_dsi_pin_config {
	int num_pins;
	/*
	 * pin numbers in the following order:
	 * clk+, clk-
	 * data1+, data1-
	 * data2+, data2-
	 * ...
	 */
	int pins[OMAP_DSS_MAX_DSI_PINS];
};

 Tomi



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 899 bytes --]

^ permalink raw reply

* Re: [PATCH 1/1] OMAP4: DSS: Add panel for Blaze Tablet boards
From: Ruslan Bilovol @ 2013-02-08 12:30 UTC (permalink / raw)
  To: Andi Shyti
  Cc: tomi.valkeinen, FlorianSchandinat, linux-fbdev, linux-kernel,
	linux-omap
In-Reply-To: <20130207145952.GA7860@jack.whiskey>

Hi,

Thanks for looking into

On Thu, Feb 7, 2013 at 4:59 PM, Andi Shyti <andi@etezian.org> wrote:
> Hi,
>
>> TC358765 is DSI-to-LVDS transmitter from Toshiba, used in
>> OMAP44XX Blaze Tablet and Blaze Tablet2 boards.
>
> I had a really fast look and I have few comments
>
>> +static int tc358765_read_register(struct omap_dss_device *dssdev,
>> +                                     u16 reg, u32 *val)
>> +{
>> +     int ret = 0;
>> +     pm_runtime_get_sync(&dssdev->dev);
>> +     /* I2C is preferred way of reading, but fall back to DSI
>> +      * if I2C didn't got initialized
>> +     */
>> +     if (tc358765_i2c)
>> +             ret = tc358765_i2c_read(reg, val);
>> +     else
>> +             ret = tc358765_dsi_read(dssdev, reg, val);
>> +     pm_runtime_put_sync(&dssdev->dev);
>> +
>> +     return ret;
>> +}
>> +
>> +static int tc358765_write_register(struct omap_dss_device *dssdev, u16 reg,
>> +             u32 value)
>> +{
>> +     struct tc358765_data *d2d = dev_get_drvdata(&dssdev->dev);
>> +     u8 buf[6];
>> +     int r;
>> +
>> +     buf[0] = (reg >> 0) & 0xff;
>> +     buf[1] = (reg >> 8) & 0xff;
>> +     buf[2] = (value >> 0) & 0xff;
>> +     buf[3] = (value >> 8) & 0xff;
>> +     buf[4] = (value >> 16) & 0xff;
>> +     buf[5] = (value >> 24) & 0xff;
>> +
>> +     r = dsi_vc_generic_write_nosync(dssdev, d2d->channel1, buf, 6);
>> +     if (r)
>> +             dev_err(&dssdev->dev, "reg write reg(%x) val(%x) failed: %d\n",
>> +                            reg, value, r);
>> +     return r;
>> +}
>> +
>> +/****************************
>> +********* DEBUG *************
>> +****************************/
>> +#ifdef CONFIG_TC358765_DEBUG
>> +static int tc358765_write_register_i2c(u16 reg, u32 val)
>> +{
>> +     int ret = -ENODEV;
>> +     unsigned char buf[6];
>> +     struct i2c_msg msg;
>> +
>> +     if (!tc358765_i2c) {
>> +             dev_err(tc358765_debug.dev, "%s: I2C not initilized\n",
>> +                                                     __func__);
>> +             return ret;
>> +     }
>> +
>> +     buf[0] = (reg >> 8) & 0xff;
>> +     buf[1] = (reg >> 0) & 0xff;
>> +     buf[2] = (val >> 0) & 0xff;
>> +     buf[3] = (val >> 8) & 0xff;
>> +     buf[4] = (val >> 16) & 0xff;
>> +     buf[5] = (val >> 24) & 0xff;
>> +     msg.addr = tc358765_i2c->client->addr;
>> +     msg.len = sizeof(buf);
>> +     msg.flags = 0;
>> +     msg.buf = buf;
>> +
>> +     mutex_lock(&tc358765_i2c->xfer_lock);
>> +     ret = i2c_transfer(tc358765_i2c->client->adapter, &msg, 1);
>> +     mutex_unlock(&tc358765_i2c->xfer_lock);
>> +
>> +     if (ret != 1)
>> +             return ret;
>> +     return 0;
>> +}
>
> What about using smbus?

OMAP2+ SoCs have full I2C controller so panel drivers traditionally use I2C.
Since implementation is OMAP-specific, I don't think we need to use smbus here.

Also please look at other panels implementation:
drivers/video/omap2/displays/panel-tfp410.c
drivers/video/omap2/displays/panel-picodlp.c

>
>> +
>> +     if (copy_from_user(&buf, ubuf, size))
>> +             return -EFAULT;
>> +
>> +     buf[size-1] = '\0';
>> +     if (sscanf(buf, "%s %x", name, &value) != 2) {
>> +             dev_err(dev, "%s: unable to parse input\n", __func__);
>> +             return -1;
>> +     }
>> +
>> +     if (!tc358765_i2c) {
>> +             dev_warn(dev,
>> +                     "failed to write register: I2C not initialized\n");
>> +             return -ENODEV;
>> +     }
>> +
>> +     reg_count = sizeof(tc358765_regs) / sizeof(tc358765_regs[0]);
>> +     for (i = 0; i < reg_count; i++) {
>> +             if (!strcmp(name, tc358765_regs[i].name)) {
>> +                     if (!(tc358765_regs[i].perm & A_WO)) {
>> +                             dev_err(dev, "%s is write-protected\n", name);
>> +                             return -EACCES;
>> +                     }
>> +
>> +                     error = tc358765_write_register_i2c(
>> +                             tc358765_regs[i].reg, value);
>> +                     if (error) {
>> +                             dev_err(dev, "%s: failed to write %s\n",
>> +                                     __func__, name);
>> +                             return -1;
>
> Could avoid returning -1 instead of returning a correct errno?

Agree. Will be fixed in next patchset

>
>> +     gpio_set_value(dssdev->reset_gpio, 1);
>> +     udelay(200);
>> +     /* reset the panel */
>> +     gpio_set_value(dssdev->reset_gpio, 0);
>> +     /* assert reset */
>> +     udelay(200);
>> +     gpio_set_value(dssdev->reset_gpio, 1);
>> +     /* wait after releasing reset */
>> +     msleep(200);
>
> I invite you to have a look at
>
> Documentation/timers/timers-howto.txt

Will be fixed in next patchset

>
>> +     /* reset LVDS-PHY */
>> +     tc358765_write_register(dssdev, LVPHY0, (1 << 22));
>> +     mdelay(2);
>
> You should give me a really good reason for using mdelay.

Will be fixed in next patchset

>
>> +     if (r) {
>> +             dev_err(&dssdev->dev, "failed to configure DSI pins\n");
>> +             goto err_disp_enable;
>> +     };
>         ^^^
>
> ???

Just a typo. Will be fixed in next patchset

>
>> +static int tc358765_i2c_probe(struct i2c_client *client,
>> +                                const struct i2c_device_id *id)
>> +{
>> +     tc358765_i2c = kzalloc(sizeof(*tc358765_i2c), GFP_KERNEL);
>
> what about devm_kzalloc?

Will be fixed in next patchset

-
Ruslan

^ permalink raw reply

* Re: [RFC PATCH 0/4] Common Display Framework-TF
From: Marcus Lorentzon @ 2013-02-08 12:40 UTC (permalink / raw)
  To: Tomi Valkeinen
  Cc: Tomasz Figa, Laurent Pinchart, Tomasz Figa,
	dri-devel@lists.freedesktop.org, linux-fbdev@vger.kernel.org,
	linux-samsung-soc@vger.kernel.org, kyungmin.park@samsung.com,
	m.szyprowski@samsung.com, Daniel Vetter, rob@ti.com, Vikas Sajjan,
	inki.dae@samsung.com, dh09.lee@samsung.com,
	ville.syrjala@intel.com, s.nawrocki@samsung.com
In-Reply-To: <5114E412.5030806@ti.com>

On 02/08/2013 12:40 PM, Tomi Valkeinen wrote:
> Hi,
>
> On 2013-02-03 21:17, Tomasz Figa wrote:
>> Hi Laurent,
>>
>> On Saturday 02 of February 2013 11:39:59 Laurent Pinchart wrote:
>>> Hi Tomasz,
>>>
>>> Thank you for your RFC.
>>>
>>> On Wednesday 30 January 2013 16:38:59 Tomasz Figa wrote:
>>>> Changes done to Tomi's edition of CDF:
>>>>   - Replaced set_operation_mode, set_pixel_format and set_size video
>>>>   source>
>>>>     operations with get_params display entity operation, as it was
>>>>     originally in Laurent's version.
>>>>     
>>>>     We have discussed this matter on the mailing list and decided that
>>>>     it
>>>>     would be better to have the source ask the sink for its parameter
>>>>     structure and do whatever appropriate with it.
>>> Could you briefly outline the rationale behind this ?
>> Display interfaces may be described by an arbitrary number of parameters.
>> Some will require just video timings, others like DSI will require a
>> significant number of additional bus-specific ones.
>>
>> Separating all the parameters (all of which are usually set at the same
>> time) into a lot of ops setting single parameter will just add unnecessary
>> complexity.
> I have nothing against combining the parameters that way. I think the
> important thing here is just that we have to allow changing of the
> parameters, whenever the display device needs that. So the bus
> parameters cannot be a one time constant setting.

I agree, but I think it should be 
setup->enable->video_on->video_off->disable->setup->...
I don't think there is any interface parameters that should be changed 
while link is enabled. And if there are, they should be identified and 
split out into a separate operation.
>>> I'm wondering whether get_params could limit us if a device needs to
>>> modify parameters at runtime. For instance a panel might need to change
>>> clock frequencies or number of used data lane depending on various
>>> runtime conditions. I don't have a real use case here, so it might just
>>> be a bit of over-engineering.
>> Hmm, isn't it in the opposite direction (the user requests the display
>> interface to change, for example, video mode, which in turn reconfigures
>> the link and requests the panel to update its settings)?
> Well, now, that's the question.
>
> Let's consider a simplified case with DSI output from the SoC, and a DSI
> panel. If the panel is a rather simple one, you could well call a method
> in the API in the DSI output driver, which would do necessary
> configuration and inform the panel driver to do any configuration it
> needs to do, based on the parameters.
>
> However, in my experience display devices, DSI devices in particular,
> are often quite "interesting". If the control of the operation in
> question is in the DSI output side, we are limited about what kind of
> DSI panels we can use, as the DSI output driver has to know all the
> tricks needed to make the panels work.
>
> I'm having hard time trying to explain this, so let's try an example.
> Consider the initial powering up of the bus. If the DSI output driver is
> in control, something like the following probably happens:
>
> - output driver asks for the parameters from the panel driver
> - output driver programs the DSI output according to these parameters
> - output driver informs the panel that the bus is now up and running
>
> Then consider a totally invented case, but which perhaps describes the
> problem with the above approach: The DSI panel requires the DSI bus
> first to be started in HS mode, but with a certain predefined bus speed,
> and only one lane used. This predefined bus setup is used to send
> configuration commands to the panel hardware, and only after that can
> you reconfigure the bus with proper speed and lanes.
>
> That kind of thing is a bit difficult to manage with the DSI output
> driver is in control. However, if the DSI panel driver is in control,
> it's simple:
>
> - panel driver calls config methods in the DSI output driver, setting
> the predefined speed and one lane
> - panel driver sends configuration commands to the panel
> - panel driver calls config methods in the DSI output driver, setting
> the final bus config

We have similar use cases and so I agree (and have implemented) the 
latter approach where panel driver is in control. I think it is 
important to separate the panel calling dispc (which it should not) from 
panel calling bus/videosource ops. Compare to I2C, no one would expect 
panel to call dispc to do I2C ops, instead panel call bus ops directly 
in response to the CDF ops request panel is running.
>>>>     5. Mask of used data lanes (each bit represents single lane).
>>>  From my experience with MIPI CSI (Camera Serial Interface, similar to
>>> DSI) some transceivers can reroute data lanes internally. Is that true
>>> for DSI as well ? In that case we would need a list of data lanes, not
>>> just a mask.
>> Hmm, I don't remember at the moment, will have to look to the
>> specification. Exynos DSI master doesn't support such feature.
> In OMAP you can configure the DSI pins quite freely. We have the
> following struct:
>
> struct omap_dsi_pin_config {
> 	int num_pins;
> 	/*
> 	 * pin numbers in the following order:
> 	 * clk+, clk-
> 	 * data1+, data1-
> 	 * data2+, data2-
> 	 * ...
> 	 */
> 	int pins[OMAP_DSS_MAX_DSI_PINS];
> };
>
Do you reroute after boot? Or is this just "board/product setup". We 
have some pinmuxing as well and DPhy sharing, but we have never used it 
after init/boot. If not runtime, I think this could be put in DT config 
for product instead of under dynamic control from panel.

/BR
/Marcus


^ permalink raw reply

* Re: [RFC v2 0/5] Common Display Framework
From: Marcus Lorentzon @ 2013-02-08 12:43 UTC (permalink / raw)
  To: Tomi Valkeinen
  Cc: Laurent Pinchart, Tomasz Figa, Thomas Petazzoni,
	linux-fbdev@vger.kernel.org, Philipp Zabel, Tom Gall,
	Ragesh Radhakrishnan, dri-devel@lists.freedesktop.org, Rob Clark,
	Kyungmin Park, sunil joshi, Benjamin Gaignard, Bryan Wu,
	Maxime Ripard, Vikas Sajjan, Sumit Semwal, Sebastien Guiriec,
	linux-media@vger.kernel.org
In-Reply-To: <5114D8A2.6010806@ti.com>

On 02/08/2013 11:51 AM, Tomi Valkeinen wrote:
> On 2013-02-04 12:05, Marcus Lorentzon wrote:
>
>> As discussed at FOSDEM I will give DSI bus with full feature set a
>> try.
> Please do, but as a reminder I want to raise the issues I see with a DSI
> bus:
>
> - A device can be a child of only one bus. So if DSI is used only for
> video, the device is a child of, say, i2c bus, and thus there's no DSI
> bus. How to configure and use DSI in this case?
>
> - If DSI is used for both control and video, we have two separate APIs
> for the bus. What I mean here is that for the video-only case above, we
> need a video-only-API for DSI. This API should contain all necessary
> methods to configure DSI. But we need similar methods for the control
> API also.
>
> So, I hope you come up with some solution for this, but as I see it,
> it's easily the most simple and clear option to have one video_source
> style entity for the DSI bus itself, which is used for both control and
> video.
>
>
Thanks, it is not that I'm totally against the video source stuff. And I 
share your concerns, none of the solutions are perfect. It just doesn't 
feel right to create this dummy source "device" without investigating 
the DSI bus route. But I will try to write up some history/problem 
description and ask Greg KH for guidance. If he has a strong opinion 
either way, there is not much more to discuss ;)

/BR
/Marcus


^ permalink raw reply

* Re: [PATCH 1/1] OMAP4: DSS: Add panel for Blaze Tablet boards
From: Andi Shyti @ 2013-02-08 12:48 UTC (permalink / raw)
  To: Ruslan Bilovol
  Cc: Andi Shyti, tomi.valkeinen, FlorianSchandinat, linux-fbdev,
	linux-kernel, linux-omap
In-Reply-To: <CAB=otbTH8bQp5X4Y2cvMYKwYqMbT0HQxmuRD9gRtna=-vy2ptA@mail.gmail.com>

Hi,

> Thanks for looking into

if you want you can put me in CC and I can review the next
patches (perhaps V2 of the patches)

> >> +     mutex_lock(&tc358765_i2c->xfer_lock);
> >> +     ret = i2c_transfer(tc358765_i2c->client->adapter, &msg, 1);
> >> +     mutex_unlock(&tc358765_i2c->xfer_lock);
> >
> > What about using smbus?
> 
> OMAP2+ SoCs have full I2C controller so panel drivers traditionally use I2C.
> Since implementation is OMAP-specific, I don't think we need to use smbus here.
> 
> Also please look at other panels implementation:
> drivers/video/omap2/displays/panel-tfp410.c
> drivers/video/omap2/displays/panel-picodlp.c

Yeah, it's the same problem with other i2c drivers, they should
be written to be working as much as possible with the emulated
smbus.

I don't actually know the internals of the omap2 driver
controller, but not being able to use smbus, definitely sucks!

> Will be fixed in next patchset

What about sending a patch V2? :)

Andi

^ permalink raw reply

* Re: [RFC PATCH 0/4] Common Display Framework-TF
From: Tomi Valkeinen @ 2013-02-08 13:04 UTC (permalink / raw)
  To: Marcus Lorentzon
  Cc: Tomasz Figa, Laurent Pinchart, Tomasz Figa,
	dri-devel@lists.freedesktop.org, linux-fbdev@vger.kernel.org,
	linux-samsung-soc@vger.kernel.org, kyungmin.park@samsung.com,
	m.szyprowski@samsung.com, Daniel Vetter, rob@ti.com, Vikas Sajjan,
	inki.dae@samsung.com, dh09.lee@samsung.com,
	ville.syrjala@intel.com, s.nawrocki@samsung.com
In-Reply-To: <5114F224.6010201@stericsson.com>

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

On 2013-02-08 14:40, Marcus Lorentzon wrote:

> I agree, but I think it should be
> setup->enable->video_on->video_off->disable->setup->...
> I don't think there is any interface parameters that should be changed
> while link is enabled. And if there are, they should be identified and
> split out into a separate operation.

Hmm. At least on OMAP and DSI video mode, it is possible to change at
least timings on the fly. But yes, generally the link has to be disabled
first before changing any parameters.

>> In OMAP you can configure the DSI pins quite freely. We have the
>> following struct:
>>
>> struct omap_dsi_pin_config {
>>     int num_pins;
>>     /*
>>      * pin numbers in the following order:
>>      * clk+, clk-
>>      * data1+, data1-
>>      * data2+, data2-
>>      * ...
>>      */
>>     int pins[OMAP_DSS_MAX_DSI_PINS];
>> };
>>
> Do you reroute after boot? Or is this just "board/product setup". We
> have some pinmuxing as well and DPhy sharing, but we have never used it
> after init/boot. If not runtime, I think this could be put in DT config
> for product instead of under dynamic control from panel.

The pin config with the struct above is done later, when the panel
driver configures the DSI bus. So in OMAP we have two distinct things
that need to be configured:

- The actual pin muxing, i.e. selecting the functions for pin N on the
OMAP package. The functions for a single pin could be for example GPIO
70, DSI DX0, UART1_CTS, etc. This is normally done once during board init.

- DSI pin configuration in the display subsystem. This is used to map
the pins to DSI functions. I.e. DSI DX0 pin is mapped to DATA1+. This is
done by the DSS driver, when the panel driver gives it the parameters.

So the first muxing basically assigns the pin to DSI in general, and
then DSI will internally route the pin to a an actual DSI function.

Whether the muxing needs to changed during runtime... I'm not sure. On
OMAP the DSI pin config also tells how many lanes are used. So if a DSI
panel would first want to use only one lane, and later change it to n
lanes, we'd need this kind of function.

I think it conceptually fits better if the pin config data is passed to
the panel via DT data, and the panel then gives the config to the DSI
master. It's just a part of the DSI bus parameters, like, say, clock
speed or whether to use HS or LP. That way the DT node for the panel
contains the information about the panel. (versus having pin config data
in the DSI master, in which case the DSI master's node contains data
about a specific DSI panel).

 Tomi



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 899 bytes --]

^ permalink raw reply

* Re: [RFC PATCH 0/4] Common Display Framework-TF
From: Marcus Lorentzon @ 2013-02-08 13:28 UTC (permalink / raw)
  To: Tomi Valkeinen
  Cc: Tomasz Figa, Laurent Pinchart, Tomasz Figa,
	dri-devel@lists.freedesktop.org, linux-fbdev@vger.kernel.org,
	linux-samsung-soc@vger.kernel.org, kyungmin.park@samsung.com,
	m.szyprowski@samsung.com, Daniel Vetter, rob@ti.com, Vikas Sajjan,
	inki.dae@samsung.com, dh09.lee@samsung.com,
	ville.syrjala@intel.com, s.nawrocki@samsung.com
In-Reply-To: <5114F7C9.4040803@ti.com>

On 02/08/2013 02:04 PM, Tomi Valkeinen wrote:
> On 2013-02-08 14:40, Marcus Lorentzon wrote:
>
>> I agree, but I think it should be
>> setup->enable->video_on->video_off->disable->setup->...
>> I don't think there is any interface parameters that should be changed
>> while link is enabled. And if there are, they should be identified and
>> split out into a separate operation.
> Hmm. At least on OMAP and DSI video mode, it is possible to change at
> least timings on the fly. But yes, generally the link has to be disabled
> first before changing any parameters.

When we do that we stop->setup->start during blanking. So our "DSS" is 
optimized to be able to do that without getting blocked. All DSI video 
mode panels (and DPI) products we have done so far have not had any 
issue with that (as long as DSI HS clock is set to continuous). I think 
this approach is less platform dependant, as long as there is no SoC 
that take more than a blanking period to reconfigure.
>>> In OMAP you can configure the DSI pins quite freely. We have the
>>> following struct:
>>>
>>> struct omap_dsi_pin_config {
>>>      int num_pins;
>>>      /*
>>>       * pin numbers in the following order:
>>>       * clk+, clk-
>>>       * data1+, data1-
>>>       * data2+, data2-
>>>       * ...
>>>       */
>>>      int pins[OMAP_DSS_MAX_DSI_PINS];
>>> };
>>>
>> Do you reroute after boot? Or is this just "board/product setup". We
>> have some pinmuxing as well and DPhy sharing, but we have never used it
>> after init/boot. If not runtime, I think this could be put in DT config
>> for product instead of under dynamic control from panel.
> The pin config with the struct above is done later, when the panel
> driver configures the DSI bus. So in OMAP we have two distinct things
> that need to be configured:
>
> - The actual pin muxing, i.e. selecting the functions for pin N on the
> OMAP package. The functions for a single pin could be for example GPIO
> 70, DSI DX0, UART1_CTS, etc. This is normally done once during board init.
>
> - DSI pin configuration in the display subsystem. This is used to map
> the pins to DSI functions. I.e. DSI DX0 pin is mapped to DATA1+. This is
> done by the DSS driver, when the panel driver gives it the parameters.
>
> So the first muxing basically assigns the pin to DSI in general, and
> then DSI will internally route the pin to a an actual DSI function.
>
> Whether the muxing needs to changed during runtime... I'm not sure. On
> OMAP the DSI pin config also tells how many lanes are used. So if a DSI
> panel would first want to use only one lane, and later change it to n
> lanes, we'd need this kind of function.
>
> I think it conceptually fits better if the pin config data is passed to
> the panel via DT data, and the panel then gives the config to the DSI
> master. It's just a part of the DSI bus parameters, like, say, clock
> speed or whether to use HS or LP. That way the DT node for the panel
> contains the information about the panel. (versus having pin config data
> in the DSI master, in which case the DSI master's node contains data
> about a specific DSI panel).
>
I think it still is OMAP specifics and doesn't belong in the panel 
drivers any longer. If you revisit this requirement in the CDF context 
where DSI ifc parameters should describe how to interface with a panel 
outside the SoC, there can't really be any dependencies on SoC internal 
routing. As you say, this is inside pinmux, so how can that affect the 
SoC external interface? I would suggest moving this to dispc-dsilink DT 
settings that are activated on dsilink->enable/disable. At least that is 
how I plan to solve similar STE SoC specific DSI config settings that 
are not really CDF panel generic, like some DPhy trim settings. They do 
depend on the panel and clock speed, but they are more product specific 
than panel driver specific. Then if there are these type of settings 
that every SoC have, then we could look at standardize those. But for 
starters I would try to keep it in product/board-DT per DSI link. So we 
should try to differentiate between DSI host and slave bus params and 
keep slave params in panel driver.

/BR
/Marcus


^ permalink raw reply

* Re: [RFC PATCH 0/4] Common Display Framework-TF
From: Tomi Valkeinen @ 2013-02-08 14:02 UTC (permalink / raw)
  To: Marcus Lorentzon
  Cc: Tomasz Figa, Laurent Pinchart, Tomasz Figa,
	dri-devel@lists.freedesktop.org, linux-fbdev@vger.kernel.org,
	linux-samsung-soc@vger.kernel.org, kyungmin.park@samsung.com,
	m.szyprowski@samsung.com, Daniel Vetter, rob@ti.com, Vikas Sajjan,
	inki.dae@samsung.com, dh09.lee@samsung.com,
	ville.syrjala@intel.com, s.nawrocki@samsung.com
In-Reply-To: <5114FD86.2020400@stericsson.com>

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

On 2013-02-08 15:28, Marcus Lorentzon wrote:

> When we do that we stop->setup->start during blanking. So our "DSS" is
> optimized to be able to do that without getting blocked. All DSI video
> mode panels (and DPI) products we have done so far have not had any
> issue with that (as long as DSI HS clock is set to continuous). I think
> this approach is less platform dependant, as long as there is no SoC
> that take more than a blanking period to reconfigure.

So do you stop, setup and start the link with CPU, and this has to be
happen during blanking? Isn't that prone to errors? Or did you mean that
the hardware handles that automatically?

In OMAP DSS there are so called shadow registers, that can be programmed
at any time. The you set a bit (GO bit), which tells the hardware to
take the new settings into use at the next vblank.

From DSI driver's perspective the link is never stopped when
reconfiguring the video timings. However, many other settings have to be
configured when the link is disabled.

>>>> In OMAP you can configure the DSI pins quite freely. We have the
>>>> following struct:
>>>>
>>>> struct omap_dsi_pin_config {
>>>>      int num_pins;
>>>>      /*
>>>>       * pin numbers in the following order:
>>>>       * clk+, clk-
>>>>       * data1+, data1-
>>>>       * data2+, data2-
>>>>       * ...
>>>>       */
>>>>      int pins[OMAP_DSS_MAX_DSI_PINS];
>>>> };
>>>>

> I think it still is OMAP specifics and doesn't belong in the panel
> drivers any longer. If you revisit this requirement in the CDF context
> where DSI ifc parameters should describe how to interface with a panel
> outside the SoC, there can't really be any dependencies on SoC internal
> routing. As you say, this is inside pinmux, so how can that affect the
> SoC external interface? I would suggest moving this to dispc-dsilink DT
> settings that are activated on dsilink->enable/disable. At least that is
> how I plan to solve similar STE SoC specific DSI config settings that
> are not really CDF panel generic, like some DPhy trim settings. They do
> depend on the panel and clock speed, but they are more product specific
> than panel driver specific. Then if there are these type of settings
> that every SoC have, then we could look at standardize those. But for
> starters I would try to keep it in product/board-DT per DSI link. So we
> should try to differentiate between DSI host and slave bus params and
> keep slave params in panel driver.

Ok, I think I was being a bit vague here. I explained the OMAP DSI
routing not because I meant that this API is specific to that, but
because it explains why this kind of routing info is needed, and a
bitmask is not enough.

If you look at the omap_dsi_pin_config struct, there's nothing OMAP
specific there (except the names =). All it tells is that this device
uses N DSI pins, and the device's clk+ function should be connected to
pin X on the DSI master, clk- should be connected to pin Y, etc. X and Y
are integers, and what they mean is specific to the DSI master.

When the DSI master is OMAP's DSI, the OMAP DSI driver does the pin
configuration as I explained. When the DSI master is something else,
say, a DSI bridge, it does whatever it needs to do (which could be
nothing) to assign a particular DSI function to a pin.

 Tomi



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 899 bytes --]

^ permalink raw reply

* Re: [RFC PATCH 0/4] Common Display Framework-TF
From: Marcus Lorentzon @ 2013-02-08 14:54 UTC (permalink / raw)
  To: Tomi Valkeinen
  Cc: Tomasz Figa, Laurent Pinchart, Tomasz Figa,
	dri-devel@lists.freedesktop.org, linux-fbdev@vger.kernel.org,
	linux-samsung-soc@vger.kernel.org, kyungmin.park@samsung.com,
	m.szyprowski@samsung.com, Daniel Vetter, rob@ti.com, Vikas Sajjan,
	inki.dae@samsung.com, dh09.lee@samsung.com,
	ville.syrjala@intel.com, s.nawrocki@samsung.com
In-Reply-To: <5115055B.2080509@ti.com>

On 02/08/2013 03:02 PM, Tomi Valkeinen wrote:
> On 2013-02-08 15:28, Marcus Lorentzon wrote:
>
>> When we do that we stop->setup->start during blanking. So our "DSS" is
>> optimized to be able to do that without getting blocked. All DSI video
>> mode panels (and DPI) products we have done so far have not had any
>> issue with that (as long as DSI HS clock is set to continuous). I think
>> this approach is less platform dependant, as long as there is no SoC
>> that take more than a blanking period to reconfigure.
> So do you stop, setup and start the link with CPU, and this has to be
> happen during blanking? Isn't that prone to errors? Or did you mean that
> the hardware handles that automatically?
>
> In OMAP DSS there are so called shadow registers, that can be programmed
> at any time. The you set a bit (GO bit), which tells the hardware to
> take the new settings into use at the next vblank.
>
>  From DSI driver's perspective the link is never stopped when
> reconfiguring the video timings. However, many other settings have to be
> configured when the link is disabled.

Yeah, you lucky guys with the GO bit ;). No, we actually do CPU 
stop,setup,start. But since it is video mode, master is driving the sync 
so it is not a hard deadline. It is enough to restart before pixels 
start to degrade. On an LCD that is not so much time, but on an OLED it 
could be 10 secs :). Anyway, we have had several mass products with this 
soft solution and it has worked well.
>>>>> In OMAP you can configure the DSI pins quite freely. We have the
>>>>> following struct:
>>>>>
>>>>> struct omap_dsi_pin_config {
>>>>>       int num_pins;
>>>>>       /*
>>>>>        * pin numbers in the following order:
>>>>>        * clk+, clk-
>>>>>        * data1+, data1-
>>>>>        * data2+, data2-
>>>>>        * ...
>>>>>        */
>>>>>       int pins[OMAP_DSS_MAX_DSI_PINS];
>>>>> };
>>>>>
>> I think it still is OMAP specifics and doesn't belong in the panel
>> drivers any longer. If you revisit this requirement in the CDF context
>> where DSI ifc parameters should describe how to interface with a panel
>> outside the SoC, there can't really be any dependencies on SoC internal
>> routing. As you say, this is inside pinmux, so how can that affect the
>> SoC external interface? I would suggest moving this to dispc-dsilink DT
>> settings that are activated on dsilink->enable/disable. At least that is
>> how I plan to solve similar STE SoC specific DSI config settings that
>> are not really CDF panel generic, like some DPhy trim settings. They do
>> depend on the panel and clock speed, but they are more product specific
>> than panel driver specific. Then if there are these type of settings
>> that every SoC have, then we could look at standardize those. But for
>> starters I would try to keep it in product/board-DT per DSI link. So we
>> should try to differentiate between DSI host and slave bus params and
>> keep slave params in panel driver.
> Ok, I think I was being a bit vague here. I explained the OMAP DSI
> routing not because I meant that this API is specific to that, but
> because it explains why this kind of routing info is needed, and a
> bitmask is not enough.
>
> If you look at the omap_dsi_pin_config struct, there's nothing OMAP
> specific there (except the names =). All it tells is that this device
> uses N DSI pins, and the device's clk+ function should be connected to
> pin X on the DSI master, clk- should be connected to pin Y, etc. X and Y
> are integers, and what they mean is specific to the DSI master.
>
> When the DSI master is OMAP's DSI, the OMAP DSI driver does the pin
> configuration as I explained. When the DSI master is something else,
> say, a DSI bridge, it does whatever it needs to do (which could be
> nothing) to assign a particular DSI function to a pin.
>
I understand, but removing the omap prefix doesn't mean it has to go in 
the panel slave port/bus settings. I still can't see why this should be 
configuration on the panel driver and not the DSI master driver. Number 
of pins might be useful since you might start with one lane and then 
activate the rest. But partial muxing (pre pinmux) doesn't seem to be 
something the panel should control or know anything about. Sounds like 
normal platform/DT data per product/board.

/BR
/Marcus


^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox