* [RFC PATCH 1/4] video: add display-core
From: Tomasz Figa @ 2013-01-30 15:39 UTC (permalink / raw)
To: dri-devel
Cc: linux-fbdev, linux-samsung-soc, kyungmin.park, m.szyprowski,
t.figa, tomasz.figa, Daniel Vetter, Marcus Lorentzon,
Laurent Pinchart, rob, tomi.valkeinen, Vikas Sajjan, inki.dae,
dh09.lee, ville.syrjala, s.nawrocki
In-Reply-To: <1359560343-31636-1-git-send-email-t.figa@samsung.com>
Signed-off-by: Tomasz Figa <t.figa@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
drivers/video/display/display-core.c | 295 +++++++++++++++++++++++++++++++++++
include/video/display.h | 230 +++++++++++++++++++++++++++
2 files changed, 525 insertions(+)
create mode 100644 drivers/video/display/display-core.c
create mode 100644 include/video/display.h
diff --git a/drivers/video/display/display-core.c b/drivers/video/display/display-core.c
new file mode 100644
index 0000000..ed49384
--- /dev/null
+++ b/drivers/video/display/display-core.c
@@ -0,0 +1,295 @@
+/*
+ * Display Core
+ *
+ * Copyright (C) 2012 Renesas Solutions Corp.
+ *
+ * Contacts: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/export.h>
+#include <linux/kernel.h>
+#include <linux/list.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/of.h>
+#include <linux/videomode.h>
+
+#include <video/display.h>
+
+static struct video_source *video_source_bind(struct display_entity *entity);
+static void video_source_unbind(struct display_entity *entity);
+
+/* -----------------------------------------------------------------------------
+ * Display Entity
+ */
+
+static LIST_HEAD(display_entity_list);
+static DEFINE_MUTEX(display_entity_mutex);
+
+struct display_entity *display_entity_get_first(void)
+{
+ /* FIXME: Don't we need some locking here? */
+
+ if (list_empty(&display_entity_list))
+ return NULL;
+
+ return list_first_entry(&display_entity_list, struct display_entity,
+ list);
+}
+EXPORT_SYMBOL(display_entity_get_first);
+
+int display_entity_set_state(struct display_entity *entity,
+ enum display_entity_state state)
+{
+ int ret;
+
+ if (entity->state = state)
+ return 0;
+
+ if (!entity->ops || !entity->ops->set_state)
+ return 0;
+
+ ret = entity->ops->set_state(entity, state);
+ if (ret < 0)
+ return ret;
+
+ entity->state = state;
+ return 0;
+}
+EXPORT_SYMBOL_GPL(display_entity_set_state);
+
+int display_entity_get_modes(struct display_entity *entity,
+ const struct videomode **modes)
+{
+ if (!entity->ops || !entity->ops->get_modes)
+ return 0;
+
+ return entity->ops->get_modes(entity, modes);
+}
+EXPORT_SYMBOL_GPL(display_entity_get_modes);
+
+int display_entity_get_size(struct display_entity *entity,
+ unsigned int *width, unsigned int *height)
+{
+ if (!entity->ops || !entity->ops->get_size)
+ return -EOPNOTSUPP;
+
+ return entity->ops->get_size(entity, width, height);
+}
+EXPORT_SYMBOL_GPL(display_entity_get_size);
+
+int display_entity_get_params(struct display_entity *entity,
+ struct display_entity_interface_params *params)
+{
+ if (!entity->ops || !entity->ops->get_params)
+ return -EOPNOTSUPP;
+
+ return entity->ops->get_params(entity, params);
+}
+EXPORT_SYMBOL_GPL(display_entity_get_params);
+
+static void display_entity_release(struct kref *ref)
+{
+ struct display_entity *entity + container_of(ref, struct display_entity, ref);
+
+ if (entity->release)
+ entity->release(entity);
+}
+
+struct display_entity *display_entity_get(struct display_entity *entity)
+{
+ if (entity = NULL)
+ return NULL;
+
+ kref_get(&entity->ref);
+ return entity;
+}
+EXPORT_SYMBOL_GPL(display_entity_get);
+
+void display_entity_put(struct display_entity *entity)
+{
+ kref_put(&entity->ref, display_entity_release);
+}
+EXPORT_SYMBOL_GPL(display_entity_put);
+
+int __must_check __display_entity_register(struct display_entity *entity,
+ struct module *owner)
+{
+ struct video_source *src;
+
+ kref_init(&entity->ref);
+ entity->owner = owner;
+ entity->state = DISPLAY_ENTITY_STATE_OFF;
+ entity->source = NULL;
+
+ src = video_source_bind(entity);
+ if (!src)
+ return -EPROBE_DEFER;
+
+ mutex_lock(&display_entity_mutex);
+ list_add(&entity->list, &display_entity_list);
+ mutex_unlock(&display_entity_mutex);
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(__display_entity_register);
+
+void display_entity_unregister(struct display_entity *entity)
+{
+ video_source_unbind(entity);
+
+ mutex_lock(&display_entity_mutex);
+
+ list_del(&entity->list);
+ mutex_unlock(&display_entity_mutex);
+
+ display_entity_put(entity);
+}
+EXPORT_SYMBOL_GPL(display_entity_unregister);
+
+/* -----------------------------------------------------------------------------
+ * Video Source
+ */
+
+static LIST_HEAD(video_source_list);
+static DEFINE_MUTEX(video_source_mutex);
+
+static void video_source_release(struct kref *ref)
+{
+ struct video_source *src + container_of(ref, struct video_source, ref);
+
+ if (src->release)
+ src->release(src);
+}
+
+static struct video_source *video_source_get(struct video_source *src)
+{
+ if (src = NULL)
+ return NULL;
+
+ kref_get(&src->ref);
+ if (!try_module_get(src->owner)) {
+ kref_put(&src->ref, video_source_release);
+ return NULL;
+ }
+
+ return src;
+}
+
+static void video_source_put(struct video_source *src)
+{
+ module_put(src->owner);
+ kref_put(&src->ref, video_source_release);
+}
+
+int __must_check __video_source_register(struct video_source *src,
+ struct module *owner)
+{
+ kref_init(&src->ref);
+ src->owner = owner;
+
+ mutex_lock(&video_source_mutex);
+ list_add(&src->list, &video_source_list);
+
+ mutex_unlock(&video_source_mutex);
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(__video_source_register);
+
+void video_source_unregister(struct video_source *src)
+{
+ mutex_lock(&video_source_mutex);
+
+ list_del(&src->list);
+ mutex_unlock(&video_source_mutex);
+
+ kref_put(&src->ref, video_source_release);
+}
+EXPORT_SYMBOL_GPL(video_source_unregister);
+
+static struct video_source *video_source_bind(struct display_entity *entity)
+{
+ struct video_source *src = NULL;
+ int ret;
+
+ if (entity->source)
+ return entity->source;
+
+ mutex_lock(&video_source_mutex);
+
+ if (entity->of_node) {
+ struct device_node *np;
+
+ np = of_parse_phandle(entity->of_node, "video-source", 0);
+ if (!np)
+ goto unlock;
+
+ list_for_each_entry(src, &video_source_list, list) {
+ if (src->of_node = np)
+ goto found;
+ }
+
+ src = NULL;
+ goto unlock;
+ }
+
+ if (!entity->src_name)
+ goto unlock;
+
+ list_for_each_entry(src, &video_source_list, list) {
+ if (src->id != entity->src_id)
+ continue;
+ if (!strcmp(src->name, entity->src_name))
+ goto found;
+ }
+
+ src = NULL;
+ goto unlock;
+
+found:
+ video_source_get(src);
+
+ if (src->common_ops->bind) {
+ ret = src->common_ops->bind(src, entity);
+ if (ret != 0) {
+ video_source_put(src);
+ src = NULL;
+ goto unlock;
+ }
+ }
+
+ src->sink = entity;
+ entity->source = src;
+
+unlock:
+ mutex_unlock(&video_source_mutex);
+
+ return src;
+}
+
+static void video_source_unbind(struct display_entity *entity)
+{
+ struct video_source *src = entity->source;
+
+ if (!src)
+ return;
+
+ if (src->common_ops && src->common_ops->unbind)
+ src->common_ops->unbind(src, entity);
+
+ src->sink = NULL;
+ entity->source = NULL;
+
+ video_source_put(src);
+}
+
+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
new file mode 100644
index 0000000..7ffea2c
--- /dev/null
+++ b/include/video/display.h
@@ -0,0 +1,230 @@
+/*
+ * Display Core
+ *
+ * Copyright (C) 2012 Renesas Solutions Corp.
+ *
+ * Contacts: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#ifndef __DISPLAY_H__
+#define __DISPLAY_H__
+
+#include <linux/kref.h>
+#include <linux/list.h>
+#include <linux/module.h>
+#include <video/omapdss.h>
+
+struct display_entity;
+struct video_source;
+struct videomode;
+
+/* -----------------------------------------------------------------------------
+ * Display Entity
+ */
+
+/* Hack to get the first registered display entity */
+struct display_entity *display_entity_get_first(void);
+
+enum display_entity_state {
+ DISPLAY_ENTITY_STATE_OFF,
+ DISPLAY_ENTITY_STATE_STANDBY,
+ DISPLAY_ENTITY_STATE_ON,
+};
+
+enum display_entity_interface_type {
+ DISPLAY_ENTITY_INTERFACE_DPI,
+ DISPLAY_ENTITY_INTERFACE_DSI,
+};
+
+#define DSI_MODE_VIDEO (1 << 0)
+#define DSI_MODE_VIDEO_BURST (1 << 1)
+#define DSI_MODE_VIDEO_SYNC_PULSE (1 << 2)
+#define DSI_MODE_VIDEO_AUTO_VERT (1 << 3)
+#define DSI_MODE_VIDEO_HSE (1 << 4)
+#define DSI_MODE_VIDEO_HFP (1 << 5)
+#define DSI_MODE_VIDEO_HBP (1 << 6)
+#define DSI_MODE_VIDEO_HSA (1 << 7)
+#define DSI_MODE_VSYNC_FLUSH (1 << 8)
+#define DSI_MODE_EOT_PACKET (1 << 9)
+
+enum mipi_dsi_pixel_format {
+ DSI_FMT_RGB888,
+ DSI_FMT_RGB666,
+ DSI_FMT_RGB666_PACKED,
+ DSI_FMT_RGB565,
+};
+
+struct mipi_dsi_interface_params {
+ enum mipi_dsi_pixel_format format;
+ unsigned long mode;
+ unsigned long hs_clk_freq;
+ unsigned long esc_clk_freq;
+ unsigned char data_lanes;
+ unsigned char cmd_allow;
+};
+
+struct display_entity_interface_params {
+ enum display_entity_interface_type type;
+ union {
+ struct mipi_dsi_interface_params dsi;
+ } p;
+};
+
+struct display_entity_control_ops {
+ int (*set_state)(struct display_entity *ent,
+ enum display_entity_state state);
+ int (*update)(struct display_entity *ent,
+ void (*callback)(int, void *), void *data);
+ int (*get_modes)(struct display_entity *ent,
+ 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);
+};
+
+struct display_entity {
+ struct list_head list;
+ struct device *dev;
+ struct device_node *of_node;
+ struct module *owner;
+ struct kref ref;
+
+ const char *src_name;
+ int src_id;
+ struct video_source *source;
+
+ const struct display_entity_control_ops *ops;
+
+ void(*release)(struct display_entity *ent);
+
+ enum display_entity_state state;
+};
+
+int display_entity_set_state(struct display_entity *entity,
+ enum display_entity_state state);
+int display_entity_get_params(struct display_entity *entity,
+ struct display_entity_interface_params *params);
+int display_entity_get_modes(struct display_entity *entity,
+ const struct videomode **modes);
+int display_entity_get_size(struct display_entity *entity,
+ unsigned int *width, unsigned int *height);
+
+struct display_entity *display_entity_get(struct display_entity *entity);
+void display_entity_put(struct display_entity *entity);
+
+int __must_check __display_entity_register(struct display_entity *entity,
+ struct module *owner);
+void display_entity_unregister(struct display_entity *entity);
+
+#define display_entity_register(display_entity) \
+ __display_entity_register(display_entity, THIS_MODULE)
+
+
+/* -----------------------------------------------------------------------------
+ * Video Source
+ */
+
+enum video_source_stream_state {
+ DISPLAY_ENTITY_STREAM_STOPPED,
+ DISPLAY_ENTITY_STREAM_CONTINUOUS,
+};
+
+struct common_video_source_ops {
+ int (*set_stream)(struct video_source *src,
+ enum video_source_stream_state state);
+ int (*bind)(struct video_source *src, struct display_entity *sink);
+ int (*unbind)(struct video_source *src, struct display_entity *sink);
+};
+
+struct dpi_video_source_ops {
+ int (*set_videomode)(struct video_source *src,
+ const struct videomode *vm);
+ int (*set_data_lines)(struct video_source *src, int lines);
+};
+
+struct dsi_video_source_ops {
+ /* enable/disable dsi bus */
+ int (*enable)(struct video_source *src);
+ int (*disable)(struct video_source *src);
+
+ /* bus configuration */
+ int (*configure_pins)(struct video_source *src,
+ const struct omap_dsi_pin_config *pins);
+ int (*set_clocks)(struct video_source *src,
+ unsigned long ddr_clk,
+ unsigned long lp_clk);
+ /* NOTE: Do we really need configure_pins and set_clocks here? */
+
+ void (*enable_hs)(struct video_source *src, bool enable);
+
+ /* data transfer */
+ int (*dcs_write)(struct video_source *src, int channel,
+ const u8 *data, size_t len);
+ int (*dcs_read)(struct video_source *src, int channel, u8 dcs_cmd,
+ u8 *data, size_t len);
+ /* NOTE: Do we need more write and read types? */
+
+ int (*update)(struct video_source *src, int channel,
+ void (*callback)(int, void *), void *data);
+};
+
+struct dvi_video_source_ops {
+ int (*set_videomode)(struct video_source *src,
+ const struct videomode *vm);
+};
+
+struct video_source {
+ struct list_head list;
+ struct device *dev;
+ struct device_node *of_node;
+ struct module *owner;
+ struct kref ref;
+
+ struct display_entity *sink;
+
+ const char *name;
+ int id;
+
+ const struct common_video_source_ops *common_ops;
+
+ union {
+ const struct dpi_video_source_ops *dpi;
+ const struct dsi_video_source_ops *dsi;
+ const struct dvi_video_source_ops *dvi;
+ } ops;
+
+ void(*release)(struct video_source *src);
+};
+
+static inline int dsi_dcs_write(struct video_source *src, int channel,
+ const u8 *data, size_t len)
+{
+ if (!src->ops.dsi || !src->ops.dsi->dcs_write)
+ return -EINVAL;
+
+ return src->ops.dsi->dcs_write(src, channel, data, len);
+}
+
+static inline int dsi_dcs_read(struct video_source *src, int channel,
+ u8 dcs_cmd, u8 *data, size_t len)
+{
+ if (!src->ops.dsi || !src->ops.dsi->dcs_read)
+ return -EINVAL;
+
+ return src->ops.dsi->dcs_read(src, channel, dcs_cmd, data, len);
+}
+
+
+#define video_source_register(video_source) \
+ __video_source_register(video_source, THIS_MODULE)
+
+int __must_check __video_source_register(struct video_source *entity,
+ struct module *owner);
+void video_source_unregister(struct video_source *entity);
+
+#endif /* __DISPLAY_H__ */
--
1.8.1
^ permalink raw reply related
* [RFC PATCH 0/4] Common Display Framework-TF
From: Tomasz Figa @ 2013-01-30 15:38 UTC (permalink / raw)
To: dri-devel
Cc: linux-fbdev, linux-samsung-soc, kyungmin.park, m.szyprowski,
t.figa, tomasz.figa, Daniel Vetter, Marcus Lorentzon,
Laurent Pinchart, rob, tomi.valkeinen, Vikas Sajjan, inki.dae,
dh09.lee, ville.syrjala, s.nawrocki
Hi,
After pretty long time of trying to adapt Exynos-specific DSI display support
to Common Display Framework I'm ready to show some preliminary RFC patches.
This series shows some ideas for CDF that came to my mind during my work,
some changes based on comments received by Tomi's edition of CDF and also
preliminarys version of Exynos DSI (video source part only, still with some
FIXMEs) and Samsung S6E8AX0 DSI panel drivers.
It is heavily based on Tomi's work which can be found here:
http://thread.gmane.org/gmane.comp.video.dri.devel/78227
The code might be a bit hacky in places, as I wanted to get it to a kind
of complete and working state first. However I hope that some of the ideas
might useful for further works.
So, here comes the TF edition of Common Clock Framework.
--------------------------------------------------------
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.
- Defined a preliminary set of DSI bus parameters.
Following parameters are defined:
1. Pixel format used for video data transmission.
2. Mode flags (several bit flags ORed together):
a) DSI_MODE_VIDEO - entity uses video mode (as opposed to command mode),
following DSI_MODE_VIDEO_* flags are relevant only if this flag is set.
b) DSI_MODE_VIDEO_BURST - entity uses burst transfer for video data
c) DSI_MODE_VIDEO_SYNC_PULSE - entity uses sync pulses (as opposed
to sync events)
d) DSI_MODE_VIDEO_AUTO_VERT - entity uses automatic video mode
detection
e) DSI_MODE_VIDEO_HSE - entity needs horizontal sync end packets
f) DSI_MODE_VIDEO_HFP - entity needs horizontal front porch area
g) DSI_MODE_VIDEO_HBP - entity needs horizontal back porch area
h) DSI_MODE_VIDEO_HSA - entity needs horizontal sync active area
i) DSI_MODE_VSYNC_FLUSH - vertical sync pulse flushes video data
j) DSI_MODE_EOT_PACKET - entity needs EoT packets
3. Bit (serial) clock frequency in HS mode.
4. Escape mode clock frequency.
5. Mask of used data lanes (each bit represents single lane).
6. Command allow area in video mode - amount of lines after transmitting
video data when generic commands are accepted.
Feel free to suggest anything missing or wrong, as the list of
parameters is based merely on what was used in original Exynos MIPI
DSIM driver.
- Redesigned source-entity matching.
Now each source has name string and integer id and each entity has
source name and source id. In addition, they can have of_node specified,
which is used for Device Tree-based matching.
The matching procedure is as follows:
1. Matching takes place when display entity registers.
a) If there is of_node specified for the entity then "video-source"
phandle of this node is being parsed and list of registered sources
is traversed in search of a source with of_node received from
parsing the phandle.
b) Otherwise the matching key is a pair of source name and id.
2. If no matching source is found, display_entity_register returns
-EPROBE_DEFER error which causes the entity driver to defer its
probe until the source registers.
3. Otherwise an optional bind operation of video source is called,
sink field of source and source field of entity are set and the
matching ends successfully.
- Some initial concept of source-entity cross-locking.
Only video source is protected at the moment, as I still have to think
a bit about locking the entity in a way where removing it by user request
is still possible.
- Dropped any panels and chips that I can't test.
They are irrelevant for this series, so there is no point in including them.
- Added Exynos DSI video source driver.
This is a new driver for the DSI controller found in Exynos SoCs. It still
needs some work, but in current state can be considered an example of DSI
video source implementation for my version of CDF.
- Added Samsung S6E8AX0 DSI panel driver.
This is the old Exynos-specific driver, just migrated to CDF and with
some hacks to provide control over entity state to userspace using
lcd device. However it can be used to demonstrate video source ops in use.
Feel free to comment as much as you can.
Tomasz Figa (4):
video: add display-core
video: add makefile & kconfig
video: display: Add exynos-dsi video source driver
video: display: Add Samsung s6e8ax0 display panel driver
drivers/video/Kconfig | 1 +
drivers/video/Makefile | 1 +
drivers/video/display/Kconfig | 16 +
drivers/video/display/Makefile | 3 +
drivers/video/display/display-core.c | 295 +++++++
drivers/video/display/panel-s6e8ax0.c | 1027 ++++++++++++++++++++++
drivers/video/display/source-exynos_dsi.c | 1313 +++++++++++++++++++++++++++++
include/video/display.h | 230 +++++
include/video/exynos_dsi.h | 41 +
include/video/panel-s6e8ax0.h | 41 +
10 files changed, 2968 insertions(+)
create mode 100644 drivers/video/display/Kconfig
create mode 100644 drivers/video/display/Makefile
create mode 100644 drivers/video/display/display-core.c
create mode 100644 drivers/video/display/panel-s6e8ax0.c
create mode 100644 drivers/video/display/source-exynos_dsi.c
create mode 100644 include/video/display.h
create mode 100644 include/video/exynos_dsi.h
create mode 100644 include/video/panel-s6e8ax0.h
--
1.8.1
^ permalink raw reply
* Re: [PATCH 1/3] atmel_lcdfb: fix 16-bpp modes on older SOCs
From: Johan Hovold @ 2013-01-30 13:26 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20130129135435.GN7360@game.jcrosoft.org>
On Tue, Jan 29, 2013 at 02:54:35PM +0100, Jean-Christophe PLAGNIOL-VILLARD wrote:
> On 13:28 Mon 10 Dec , Johan Hovold wrote:
> > Fix regression introduced by commit 787f9fd23283 ("atmel_lcdfb: support
> > 16bit BGR:565 mode, remove unsupported 15bit modes") which broke 16-bpp
> > modes for older SOCs which use IBGR:555 (msb is intensity) rather
> > than BGR:565.
> >
> > Use SOC-type to determine the pixel layout.
> >
> > Tested on custom at91sam9263-board.
> >
> > Cc: <stable@vger.kernel.org>
> > Signed-off-by: Johan Hovold <jhovold@gmail.com>
> > ---
> > drivers/video/atmel_lcdfb.c | 22 +++++++++++++++-------
> > include/video/atmel_lcdc.h | 1 +
> > 2 files changed, 16 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/video/atmel_lcdfb.c b/drivers/video/atmel_lcdfb.c
> > index 1505539..1f68fa6 100644
> > --- a/drivers/video/atmel_lcdfb.c
> > +++ b/drivers/video/atmel_lcdfb.c
> > @@ -422,17 +422,22 @@ static int atmel_lcdfb_check_var(struct fb_var_screeninfo *var,
> > = var->bits_per_pixel;
> > break;
> > case 16:
> > + /* Older SOCs use IBGR:555 rather than BGR:565. */
> > + if (sinfo->have_intensity_bit)
> > + var->green.length = 5;
> > + else
> > + var->green.length = 6;
> > +
> > if (sinfo->lcd_wiring_mode = ATMEL_LCDC_WIRING_RGB) {
> > - /* RGB:565 mode */
> > - var->red.offset = 11;
> > + /* RGB:5X5 mode */
> > + var->red.offset = var->green.length + 5;
> > var->blue.offset = 0;
> > } else {
> > - /* BGR:565 mode */
> > + /* BGR:5X5 mode */
> > var->red.offset = 0;
> > - var->blue.offset = 11;
> > + var->blue.offset = var->green.length + 5;
> > }
> > var->green.offset = 5;
> > - var->green.length = 6;
> > var->red.length = var->blue.length = 5;
> > break;
> > case 32:
> > @@ -679,8 +684,7 @@ static int atmel_lcdfb_setcolreg(unsigned int regno, unsigned int red,
> >
> > case FB_VISUAL_PSEUDOCOLOR:
> > if (regno < 256) {
> > - if (cpu_is_at91sam9261() || cpu_is_at91sam9263()
> > - || cpu_is_at91sam9rl()) {
> > + if (sinfo->have_intensity_bit) {
> > /* old style I+BGR:555 */
> > val = ((red >> 11) & 0x001f);
> > val |= ((green >> 6) & 0x03e0);
> > @@ -870,6 +874,10 @@ 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;
> > + }
> nack
>
> you need to drop the cpu_is as this can only be use now for the core
>
> use platform_device_id to indetify the IP as done on at91-i2c as we can not
> detect the IP version
If this was a new feature and not a regression, fix I'd fully agree. There
are however currently four places in atmel_lcdfb where cpu_is macros are
used, and my patch only refactors one of them.
I can submit a follow up patch which add platform_device_id support, but
such a patch will be quite invasive and thus not 3.8-rc or stable material.
How about accepting this patch as it is, considering that it fixes an
obvious regression and also facilitate the move to platform_device_id by
introducing the intensity-bit flag?
Thanks,
Johan
^ permalink raw reply
* Re: [git pull] fbcon locking fixes.
From: Daniel Vetter @ 2013-01-30 11:59 UTC (permalink / raw)
To: Maarten Lankhorst
Cc: Dave Airlie, m.b.lankhorst, linux-fbdev@vger.kernel.org,
Linux Kernel Mailing List, DRI mailing list, Andrew Morton,
Linus Torvalds
In-Reply-To: <51067300.2050808@canonical.com>
On Mon, Jan 28, 2013 at 1:45 PM, Maarten Lankhorst
<maarten.lankhorst@canonical.com> wrote:
>> There was a path going into set_con2fb_path if an fb driver was
>> already registered, I just pushed the locking out further on anyone
>> going in there.
>>
>> it boots on my EFI macbook here.
>>
> I cherry picked those patches to my tree, and the full series no longer triggers a lockdep warning.
> It also no longer locks up during modprobing or vga-switcheroo either.
>
> Tested-by: Maarten Lankhorst <maarten.lankhorst@canonical.com>
QA reported spurious failures in some kms_flip tests and lockdep
splats, somehow both are fixed with the locking. I'm a bit confused
how these issues could cause failures in the flip tests, but alas:
Tested-by: Lu Hua <huax.lu@intel.com> (for all three patches).
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
^ permalink raw reply
* Re: [RFC 0/4] Use the Common Display Framework in tegra-drm
From: Sascha Hauer @ 2013-01-30 8:38 UTC (permalink / raw)
To: Alex Courbot
Cc: Thierry Reding, Laurent Pinchart, Stephen Warren, Mark Zhang,
linux-kernel@vger.kernel.org, linux-fbdev@vger.kernel.org,
linux-tegra@vger.kernel.org, gnurou@gmail.com
In-Reply-To: <5108D865.3070301@nvidia.com>
On Wed, Jan 30, 2013 at 05:23:01PM +0900, Alex Courbot wrote:
> On 01/30/2013 04:40 PM, Thierry Reding wrote:
> >Thanks *a lot* for taking care of this Alexandre! From a quick look at
> >the patches they seem generally fine. I'll go over them in a bit more
> >detail though.
>
> Glad you like it better than my previous attempts at controlling
> Tegra's panels and backlights. ;)
>
> >>1) The CDF has a get_modes() hook, but this is already implemented by
> >>tegra_connector_get_modes(). Ideally everything should be moved to the CDF hook,
> >>but Tegra's implementation uses DRM functions to retrieve the EDID and CDF
> >>should, AFAIK, remain DRM-agnostic.
> >
> >Maybe a good option would be to just not implement get_modes() if the
> >same information can be retrieved via EDID. That is, the DRM driver
> >could just go and fetch EDID when the nvidia,ddc-i2c-bus is available
> >(or parse the nvidia,edid blob) and only rely on CDF otherwise.
>
> Since EDID information is per-panel I'd intuitively say it should be
> provided by the panel driver.
Hm. If you know from the devicetree that you have a CLAA101WA01A then
you won't need EDID data at all. If instead you have EDID data you don't
have to put the information that this is a CLAA101WA01A into devicetree.
I think the next thing that will happen is that you want to use the EDID
data to 1) pick display timings 2) pick the right power sequence that
fits the panel found in EDID. You can't archieve 2) by hardcoding the
panel in the devicetree.
Sascha
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply
* Re: [RFC 1/4] video: panel: add CLAA101WA01A panel support
From: Alex Courbot @ 2013-01-30 8:28 UTC (permalink / raw)
To: Thierry Reding
Cc: Mark Zhang, Laurent Pinchart, Stephen Warren, Mark Zhang,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-fbdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
gnurou-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org
In-Reply-To: <20130130074852.GB17547-RM9K5IK7kjIQXX3q8xo1gnVAuStQJXxyR5q1nwbD4aMs9pC9oP6+/A@public.gmane.org>
On 01/30/2013 04:48 PM, Thierry Reding wrote:
> I already said this in another thread. I think we should only be using
> the CDF .get_modes() for static modes that cannot be obtained from EDID.
> Thinking about it, I'm not quite sure why EDID would be needed for this
> kind of panel anyway. Ventana probably has it because it keeps us from
> having to hardcode things, but if we provide drivers for the panel
> anyway, we can just as well hard-code the supported mode(s) in those
> drivers.
>
> What I'm trying to say is that the existence of EDID is board-specific,
> so boards other than Ventana may not have an EDID EEPROM. Or perhaps
> this particular display has the EEPROM integrated? Even in that case,
> some boards may use this panel and simply not connect the EEPROM to an
> I2C controller.
Actually this display has its own EEPROM and pins for the I2C bus.
That's why it would seems out-of-place to have EDID taking place outside
its driver.
But you are right that we should also handle the case where the I2C bus
is not connected and provide a static list of videomodes (that could be
an "offline" dump of the EDID data). However, the driver could take the
decision to use it if the EDID bus is not specified in the DT or if
transfer failed for some reason.
But then as you mention we might as well save time and directly serve
that offline version, since we know the content of the EEPROM anyway.
Alex.
^ permalink raw reply
* Re: [RFC 0/4] Use the Common Display Framework in tegra-drm
From: Alex Courbot @ 2013-01-30 8:23 UTC (permalink / raw)
To: Thierry Reding
Cc: Laurent Pinchart, Stephen Warren, Mark Zhang,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-fbdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
gnurou-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org
In-Reply-To: <20130130074020.GA17547-RM9K5IK7kjIQXX3q8xo1gnVAuStQJXxyR5q1nwbD4aMs9pC9oP6+/A@public.gmane.org>
On 01/30/2013 04:40 PM, Thierry Reding wrote:
> Thanks *a lot* for taking care of this Alexandre! From a quick look at
> the patches they seem generally fine. I'll go over them in a bit more
> detail though.
Glad you like it better than my previous attempts at controlling Tegra's
panels and backlights. ;)
>> 1) The CDF has a get_modes() hook, but this is already implemented by
>> tegra_connector_get_modes(). Ideally everything should be moved to the CDF hook,
>> but Tegra's implementation uses DRM functions to retrieve the EDID and CDF
>> should, AFAIK, remain DRM-agnostic.
>
> Maybe a good option would be to just not implement get_modes() if the
> same information can be retrieved via EDID. That is, the DRM driver
> could just go and fetch EDID when the nvidia,ddc-i2c-bus is available
> (or parse the nvidia,edid blob) and only rely on CDF otherwise.
Since EDID information is per-panel I'd intuitively say it should be
provided by the panel driver.
> So for Ventana the only reason why we need CDF is basically the power
> sequencing, right?
As of now, yes.
> I definitely think that we should aim for correct panel and backlight
> interaction. Perhaps this could work by looking up the real backlight
> via it's phandle and have the CDF driver use the backlight API to
> disable or enable the backlight as part of the power sequencing.
I have just written a bit of code that does that. It works well and
seems like a natural way to operate the backlight. However...
> I'm not sure what you mean by "cannot ignore FB events"? Can you provide
> a concrete problematic use-case?
... that's where thing stop looking nice. The backlight framework
forcibly registers a framebuffer notifier callback and switches the
backlight on and off on blank/unblank events, effectively duplicating
what the panel driver does. There is no way to disable this behavior at
the moment.
This could be solved by introducing a new function for controlling the
"ownership" of the backlight that would unregister the notifier when the
panel driver takes its reference to the backlight.
> Hijacking .update_status() sounds a bit risky. But perhaps you could
> wrap the real backlight in a CDF backlight to receive notifications.
> Obviously you'd get two backlight devices in sysfs, but that turn out
> not to be a problem.
For cosmetic reasons I'd prefer to avoid having two backlight devices
(which ended up in that terrible PWM backlight subdriver thing). Maybe
we can engineer the backlight framework to make such customizations easier?
Alex.
^ permalink raw reply
* Re: [RFC 1/4] video: panel: add CLAA101WA01A panel support
From: Mark Zhang @ 2013-01-30 8:08 UTC (permalink / raw)
To: Thierry Reding
Cc: Alex Courbot, Laurent Pinchart, Stephen Warren,
linux-kernel@vger.kernel.org, linux-fbdev@vger.kernel.org,
linux-tegra@vger.kernel.org, gnurou@gmail.com
In-Reply-To: <20130130074852.GB17547@avionic-0098.mockup.avionic-design.de>
On 01/30/2013 03:48 PM, Thierry Reding wrote:
> * PGP Signed by an unknown key
>
> On Wed, Jan 30, 2013 at 04:27:11PM +0900, Alex Courbot wrote:
>> On 01/30/2013 04:20 PM, Mark Zhang wrote:
> [...]
>>>> +static int panel_claa101_get_modes(struct display_entity *entity,
>>>> + const struct videomode **modes)
>>>> +{
>>>> + /* TODO get modes from EDID? */
>>>
>>> Why not move the "nvidia,ddc" from encoder's DT to panel's DT? In that
>>> case, you can get EDID here. I know drm has some helpers to fetch EDID
>>> but I recall there are some other functions which has no drm
>>> dependencies which may be suitable for you.
>>
>> I explained this in the cover letter - I'm not sure we want to have
>> a dependency on DRM here, as CDF entities could also be connected to
>> other subsystems. That's something we need to figure out. But yes,
>> ultimately this should be the place where EDID is retrieved.
>
> I already said this in another thread. I think we should only be using
> the CDF .get_modes() for static modes that cannot be obtained from EDID.
> Thinking about it, I'm not quite sure why EDID would be needed for this
> kind of panel anyway. Ventana probably has it because it keeps us from
> having to hardcode things, but if we provide drivers for the panel
> anyway, we can just as well hard-code the supported mode(s) in those
> drivers.
I don't think so. I think it's better that we only have one entry for
getting video modes. Since CDF already provides ".get_modes" for us, we
can rely on that. And according to my understanding, get video modes in
panel driver makes sense than getting it in crtc.
In this way, panel driver may get video modes from EDID or from
hard-coded display timing infos, but other modules such as crtc don't
need to care about that.
Mark
>
> What I'm trying to say is that the existence of EDID is board-specific,
> so boards other than Ventana may not have an EDID EEPROM. Or perhaps
> this particular display has the EEPROM integrated? Even in that case,
> some boards may use this panel and simply not connect the EEPROM to an
> I2C controller.
>
> Thierry
>
> * Unknown Key
> * 0x7F3EB3A1
>
^ permalink raw reply
* Re: [PATCH] pwm-backlight: handle BL_CORE_FBBLANK state
From: Thierry Reding @ 2013-01-30 7:58 UTC (permalink / raw)
To: Alexandre Courbot; +Cc: Richard Purdie, linux-kernel, linux-fbdev, gnurou
In-Reply-To: <1359526755-21145-1-git-send-email-acourbot@nvidia.com>
[-- Attachment #1: Type: text/plain, Size: 550 bytes --]
On Wed, Jan 30, 2013 at 03:19:15PM +0900, Alexandre Courbot wrote:
> According to include/linux/backlight.h, the fb_blank field is to be
> removed and blank status should preferably be set by setting the
> BL_CORE_FBBLANK bit of the state field. This patch ensures this
> condition is also taken into account when updating the backlight state.
>
> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
> ---
> drivers/video/backlight/pwm_bl.c | 7 +++----
> 1 file changed, 3 insertions(+), 4 deletions(-)
Applied, thanks.
Thierry
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [RFC 1/4] video: panel: add CLAA101WA01A panel support
From: Thierry Reding @ 2013-01-30 7:48 UTC (permalink / raw)
To: Alex Courbot
Cc: Mark Zhang, Laurent Pinchart, Stephen Warren, Mark Zhang,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-fbdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
gnurou-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org
In-Reply-To: <5108CB4F.7000103-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 1585 bytes --]
On Wed, Jan 30, 2013 at 04:27:11PM +0900, Alex Courbot wrote:
> On 01/30/2013 04:20 PM, Mark Zhang wrote:
[...]
> >>+static int panel_claa101_get_modes(struct display_entity *entity,
> >>+ const struct videomode **modes)
> >>+{
> >>+ /* TODO get modes from EDID? */
> >
> >Why not move the "nvidia,ddc" from encoder's DT to panel's DT? In that
> >case, you can get EDID here. I know drm has some helpers to fetch EDID
> >but I recall there are some other functions which has no drm
> >dependencies which may be suitable for you.
>
> I explained this in the cover letter - I'm not sure we want to have
> a dependency on DRM here, as CDF entities could also be connected to
> other subsystems. That's something we need to figure out. But yes,
> ultimately this should be the place where EDID is retrieved.
I already said this in another thread. I think we should only be using
the CDF .get_modes() for static modes that cannot be obtained from EDID.
Thinking about it, I'm not quite sure why EDID would be needed for this
kind of panel anyway. Ventana probably has it because it keeps us from
having to hardcode things, but if we provide drivers for the panel
anyway, we can just as well hard-code the supported mode(s) in those
drivers.
What I'm trying to say is that the existence of EDID is board-specific,
so boards other than Ventana may not have an EDID EEPROM. Or perhaps
this particular display has the EEPROM integrated? Even in that case,
some boards may use this panel and simply not connect the EEPROM to an
I2C controller.
Thierry
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [RFC 3/4] drm: tegra: use the Common Display Framework
From: Mark Zhang @ 2013-01-30 7:46 UTC (permalink / raw)
To: Alex Courbot
Cc: Laurent Pinchart, Thierry Reding, Stephen Warren, Mark Zhang,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-fbdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
gnurou-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org
In-Reply-To: <5108C55C.30104-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
On 01/30/2013 03:01 PM, Alex Courbot wrote:
> On 01/30/2013 03:50 PM, Mark Zhang wrote:
[...]
>
>>> + /* register display notifier */
>>> + output->display_notifier.dev = NULL;
>>
>> Set "display_notifier.dev" to NULL makes we have to compare with every
>> display entity, just like what you do in "display_notify_callback":
>>
>> entity->dev && entity->dev->of_node = pnode
>>
>> So can we get the "struct device *" of panel here? Seems currently the
>> "of" framework doesn't allow "device_node -> device".
>
> Nope. AFAICT the device might not be instanciated at this point. We
> become aware of it for the first time in the callback function. We also
> don't want to defer probing until the panel is parsed first, since the
> panel might also depend on resources of the display device.
>
Agree.
> Thanks,
> Alex.
>
^ permalink raw reply
* Re: [RFC 0/4] Use the Common Display Framework in tegra-drm
From: Thierry Reding @ 2013-01-30 7:40 UTC (permalink / raw)
To: Alexandre Courbot
Cc: Laurent Pinchart, Stephen Warren, Mark Zhang,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
linux-tegra-u79uwXL29TY76Z2rM5mHXA, gnurou-Re5JQEeQqe8AvxtiuMwx3w
In-Reply-To: <1359514939-15653-1-git-send-email-acourbot-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 3715 bytes --]
On Wed, Jan 30, 2013 at 12:02:15PM +0900, Alexandre Courbot wrote:
> This series leverages the (still work-in-progress) Common Display Framework to
> add panel support to the tegra-drm driver. It also adds a driver for the
> CLAA101WA01A panel used on the Ventana board.
>
> The CDF is a moving target but Tegra needs some sort of display framework and
> even in its current state the CDF seems to be the best candidate. Besides, by
> using the CDF from now on we hope to provide useful feedback to Laurent and the
> other CDF designers.
>
> The changes to tegra-drm are rather minimal. Panels are referenced from Tegra DC
> output nodes through the "nvidia,panel" property. This property is looked up
> when a display connect notification is received in order to see if it points to
> the connected display entity. If it does, the entity is then used for output.
>
> The DPMS states are then propagated to the output entity, which is then supposed
> to call back into the set_stream() hook in order to enable/disable the output
> stream as needed.
Thanks *a lot* for taking care of this Alexandre! From a quick look at
the patches they seem generally fine. I'll go over them in a bit more
detail though.
> Although the overall design seems to work ok, a few specific issues need to be
> addressed:
>
> 1) The CDF has a get_modes() hook, but this is already implemented by
> tegra_connector_get_modes(). Ideally everything should be moved to the CDF hook,
> but Tegra's implementation uses DRM functions to retrieve the EDID and CDF
> should, AFAIK, remain DRM-agnostic.
Maybe a good option would be to just not implement get_modes() if the
same information can be retrieved via EDID. That is, the DRM driver
could just go and fetch EDID when the nvidia,ddc-i2c-bus is available
(or parse the nvidia,edid blob) and only rely on CDF otherwise.
So for Ventana the only reason why we need CDF is basically the power
sequencing, right?
> 2) There is currently no panel/backlight interaction, e.g. backlight status is
> controlled through FB events, independantly from the panel state. It could make
> sense to have the panel DT node reference the backlight and control it as part
> of its own power on/off sequence. Right now however, a backlight device cannot
> ignore FB events.
I definitely think that we should aim for correct panel and backlight
interaction. Perhaps this could work by looking up the real backlight
via it's phandle and have the CDF driver use the backlight API to
disable or enable the backlight as part of the power sequencing.
I'm not sure what you mean by "cannot ignore FB events"? Can you provide
a concrete problematic use-case?
> 3) Probably related to 2), now the backlight's power controls are part of the
> panel driver, because the pwm-backlight driver cannot control the power
> regulator and enable GPIO. This means that the backlight power is not turned off
> when its brightness is set to 0 through sysfs. Once again this speaks in favor
> of having stronger panel/backlight interaction: for instance, the panel driver
> could reference the backlight and hijack its update_status() op to replace it by
> one that does the correct power sequencing before calling the original function.
> This would require some extra infrastructure though. Another possibility would
> be to have a dedicated backlight driver for each panel, with its own
> "compatible" string.
Hijacking .update_status() sounds a bit risky. But perhaps you could
wrap the real backlight in a CDF backlight to receive notifications.
Obviously you'd get two backlight devices in sysfs, but that turn out
not to be a problem.
Thierry
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH 1/4] pwm_backlight: Fix PWM levels support in non DT case
From: Peter Ujfalusi @ 2013-01-30 7:34 UTC (permalink / raw)
To: Thierry Reding
Cc: Richard Purdie, Grant Likely, Rob Landley,
Florian Tobias Schandinat, Andrew Morton, devicetree-discuss,
linux-doc, linux-kernel, linux-fbdev
In-Reply-To: <20130129123025.GA20166@avionic-0098.mockup.avionic-design.de>
On 01/29/2013 01:30 PM, Thierry Reding wrote:
>> Right. Now I know the background.
>> However I do not agree with the conclusion. Probably it is fine in some cases
>> to limit the number of configurable duty cycles to have only distinct steps.
>> To not go too far, on my laptop I have:
>> # cat /sys/class/backlight/acpi_video0/max_brightness
>> 15
>> # cat /sys/class/backlight/intel_backlight/max_brightness
>> 93
>
> FWIW, I have hardware with an Intel chipset that has max_brightness
> 13666. That doesn't mean all of these are necessarily valid or useful.
For sure this range is overkill, but if you reduce it to let's say 15 would it
be better? I don't think. It is up to the userspace to decide how to use it.
User should have full control over the HW in hand. In this particular case I
would expect userspace to give you around 20 steps to change brightness and
when you change it would step between those so you will have nice, smooth
changes and not big jumps.
> That's not true. The duty-cycle merely defines a percentage of how long
> the PWM signal will be high. You can choose an arbitrary number of
> subdivisions.
Sure all HW has limitation. The HW I have either have 127 or 255 time slots.
Probably the best thing way to deal with the backlight is to have a range of 0
- 100% Nothing else.
We should have an API to PWMs so user drivers could get the duty cycle from
the HW drivers. In this way backlight would only expose percentage and the
backlight driver would get the number of time slots from the PWM core to
calculate the actual value for the selected percentage.
> Since the brightness of a display isn't linearly proportional to the
> duty cycle of the PWM driving it, representing that brightness by a
> linear range is misleading. Furthermore some panels have regions where
> the backlight isn't lit at all or where changes in the PWM duty-cycle
> don't make any difference.
and all of this also depend on the surrounding light conditions as well. If
the same device is used in low light condition you care about the lower light
ranges more compared to when the same device is used in bright environment. In
these case the user space has to be adopted to the conditions and one should
not need to recompile the kernel/dtb just because the device is used in
different environment.
--
Péter
^ permalink raw reply
* Re: [RFC 3/4] drm: tegra: use the Common Display Framework
From: Alex Courbot @ 2013-01-30 7:30 UTC (permalink / raw)
To: Thierry Reding
Cc: Mark Zhang, Laurent Pinchart, Stephen Warren, Mark Zhang,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-fbdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
gnurou-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org
In-Reply-To: <20130130072406.GA17128-RM9K5IK7kjIQXX3q8xo1gnVAuStQJXxyR5q1nwbD4aMs9pC9oP6+/A@public.gmane.org>
On 01/30/2013 04:24 PM, Thierry Reding wrote:
>>> Could you pick up a somewhat meaningful name? You know, there are too
>>> many variables with name "drm/connector/output/encoder"... :)
>>
>> Well, it's supposed to be abstract. From the CDF point of view it
>> could be anything besides a panel. I know this makes it an output of
>> an output, but I can't think of anything better right now.
>
> How about renaming "this" to stream to match with what the output is in
> CDF speak.
Good idea.
> And the output's output is the panel, right? Why not just
> call it that? Even if it isn't directly connected to a panel entity but
> has indeed a whole pipeline in between, for tegra-drm it is still a
> panel.
Makes sense indeed. Besides the DT refer it as "panel" already.
Thanks,
Alex.
^ permalink raw reply
* Re: [RFC 1/4] video: panel: add CLAA101WA01A panel support
From: Alex Courbot @ 2013-01-30 7:27 UTC (permalink / raw)
To: Mark Zhang
Cc: Laurent Pinchart, Thierry Reding, Stephen Warren, Mark Zhang,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-fbdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
gnurou-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org
In-Reply-To: <5108C9C1.1090707-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
On 01/30/2013 04:20 PM, Mark Zhang wrote:
>> + /* OFF and STANDBY are equivalent to us */
>> + if (state = DISPLAY_ENTITY_STATE_STANDBY)
>> + state = DISPLAY_ENTITY_STATE_OFF;
>
> Do we need this? The "switch" below handles the same thing already.
Indeed, I have rewritten this part actually.
>> +static int panel_claa101_get_modes(struct display_entity *entity,
>> + const struct videomode **modes)
>> +{
>> + /* TODO get modes from EDID? */
>
> Why not move the "nvidia,ddc" from encoder's DT to panel's DT? In that
> case, you can get EDID here. I know drm has some helpers to fetch EDID
> but I recall there are some other functions which has no drm
> dependencies which may be suitable for you.
I explained this in the cover letter - I'm not sure we want to have a
dependency on DRM here, as CDF entities could also be connected to other
subsystems. That's something we need to figure out. But yes, ultimately
this should be the place where EDID is retrieved.
>> +static struct of_device_id panel_claa101_of_match[] = {
>> + { .compatible = "chunghwa,claa101wa01a", },
>> + { },
>> +};
>> +MODULE_DEVICE_TABLE(of, pwm_backlight_of_match);
>
> What does this mean? Why we need this?
Well, now you know where I copy my code from. :)
Thanks,
Alex.
^ permalink raw reply
* Re: [RFC 3/4] drm: tegra: use the Common Display Framework
From: Thierry Reding @ 2013-01-30 7:24 UTC (permalink / raw)
To: Alex Courbot
Cc: Mark Zhang, Laurent Pinchart, Stephen Warren, Mark Zhang,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-fbdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
gnurou-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org
In-Reply-To: <5108C55C.30104-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 968 bytes --]
On Wed, Jan 30, 2013 at 04:01:48PM +0900, Alex Courbot wrote:
> On 01/30/2013 03:50 PM, Mark Zhang wrote:
> >>@@ -147,6 +148,9 @@ struct tegra_output {
> >>
> >> struct drm_encoder encoder;
> >> struct drm_connector connector;
> >>+ struct display_entity this;
> >>+ struct display_entity *output;
> >
> >Could you pick up a somewhat meaningful name? You know, there are too
> >many variables with name "drm/connector/output/encoder"... :)
>
> Well, it's supposed to be abstract. From the CDF point of view it
> could be anything besides a panel. I know this makes it an output of
> an output, but I can't think of anything better right now.
How about renaming "this" to stream to match with what the output is in
CDF speak. And the output's output is the panel, right? Why not just
call it that? Even if it isn't directly connected to a panel entity but
has indeed a whole pipeline in between, for tegra-drm it is still a
panel.
Thierry
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [RFC 1/4] video: panel: add CLAA101WA01A panel support
From: Mark Zhang @ 2013-01-30 7:20 UTC (permalink / raw)
To: Alexandre Courbot
Cc: Laurent Pinchart, Thierry Reding, Stephen Warren, Mark Zhang,
linux-kernel, linux-fbdev, linux-tegra, gnurou
In-Reply-To: <1359514939-15653-2-git-send-email-acourbot@nvidia.com>
On 01/30/2013 11:02 AM, Alexandre Courbot wrote:
> Add support for the Chunghwa CLAA101WA01A display panel.
>
> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
> ---
[...]
> +
> +#include <video/display.h>
> +
> +#define CLAA101WA01A_WIDTH 223
> +#define CLAA101WA01A_HEIGHT 125
If I remember correct, the physical size of the panel can be fetched in
EDID. If this is correct, we don't have to hard-code here.
> +
> +struct panel_claa101 {
> + struct display_entity entity;
> + struct regulator *vdd_pnl;
> + struct regulator *vdd_bl;
> + /* Enable GPIOs */
> + int pnl_enable;
> + int bl_enable;
> +};
> +
> +#define to_panel_claa101(p) container_of(p, struct panel_claa101, entity)
> +
> +static int panel_claa101_set_state(struct display_entity *entity,
> + enum display_entity_state state)
> +{
> + struct panel_claa101 *panel = to_panel_claa101(entity);
> +
> + /* OFF and STANDBY are equivalent to us */
> + if (state = DISPLAY_ENTITY_STATE_STANDBY)
> + state = DISPLAY_ENTITY_STATE_OFF;
Do we need this? The "switch" below handles the same thing already.
> +
> + switch (state) {
> + case DISPLAY_ENTITY_STATE_OFF:
> + case DISPLAY_ENTITY_STATE_STANDBY:
> + if (entity->source)
> + display_entity_set_stream(entity->source,
> + DISPLAY_ENTITY_STREAM_STOPPED);
> +
> + /* TODO error checking? */
> + gpio_set_value_cansleep(panel->bl_enable, 0);
> + usleep_range(10000, 10000);
> + regulator_disable(panel->vdd_bl);
> + usleep_range(200000, 200000);
> + gpio_set_value_cansleep(panel->pnl_enable, 0);
> + regulator_disable(panel->vdd_pnl);
> + break;
> +
> + case DISPLAY_ENTITY_STATE_ON:
> + regulator_enable(panel->vdd_pnl);
> + gpio_set_value_cansleep(panel->pnl_enable, 1);
> + usleep_range(200000, 200000);
> + regulator_enable(panel->vdd_bl);
> + usleep_range(10000, 10000);
> + gpio_set_value_cansleep(panel->bl_enable, 1);
> +
> + if (entity->source)
> + display_entity_set_stream(entity->source,
> + DISPLAY_ENTITY_STREAM_CONTINUOUS);
> + break;
> + }
> +
> + return 0;
> +}
> +
> +static int panel_claa101_get_modes(struct display_entity *entity,
> + const struct videomode **modes)
> +{
> + /* TODO get modes from EDID? */
Why not move the "nvidia,ddc" from encoder's DT to panel's DT? In that
case, you can get EDID here. I know drm has some helpers to fetch EDID
but I recall there are some other functions which has no drm
dependencies which may be suitable for you.
> + return 0;
> +}
[...]
> +
> +static int __exit panel_claa101_remove(struct platform_device *pdev)
> +{
> + struct panel_claa101 *panel = platform_get_drvdata(pdev);
> +
> + display_entity_unregister(&panel->entity);
> +
> + return 0;
> +}
> +
> +#ifdef CONFIG_OF
We don't need this kind of "ifdef" in upstream kernel.
> +static struct of_device_id panel_claa101_of_match[] = {
> + { .compatible = "chunghwa,claa101wa01a", },
> + { },
> +};
> +MODULE_DEVICE_TABLE(of, pwm_backlight_of_match);
What does this mean? Why we need this?
> +#else
> +#endif
> +
> +static const struct dev_pm_ops panel_claa101_dev_pm_ops = {
> +};
> +
> +static struct platform_driver panel_claa101_driver = {
> + .probe = panel_claa101_probe,
> + .remove = panel_claa101_remove,
> + .driver = {
> + .name = "panel_claa101wa01a",
> + .owner = THIS_MODULE,
> +#ifdef CONFIG_PM
> + .pm = &panel_claa101_dev_pm_ops,
If you haven't implemented this in this patch set, I suggest removing this.
> +#endif
> +#ifdef CONFIG_OF
> + .of_match_table = of_match_ptr(panel_claa101_of_match),
> +#endif
> + },
> +};
> +
> +module_platform_driver(panel_claa101_driver);
> +
> +MODULE_AUTHOR("Alexandre Courbot <acourbot@nvidia.com>");
> +MODULE_DESCRIPTION("Chunghwa CLAA101WA01A Display Panel");
> +MODULE_LICENSE("GPL");
>
^ permalink raw reply
* Re: [RFC 3/4] drm: tegra: use the Common Display Framework
From: Alex Courbot @ 2013-01-30 7:01 UTC (permalink / raw)
To: Mark Zhang
Cc: Laurent Pinchart, Thierry Reding, Stephen Warren, Mark Zhang,
linux-kernel@vger.kernel.org, linux-fbdev@vger.kernel.org,
linux-tegra@vger.kernel.org, gnurou@gmail.com
In-Reply-To: <5108C298.1000500@gmail.com>
On 01/30/2013 03:50 PM, Mark Zhang wrote:
>> @@ -147,6 +148,9 @@ struct tegra_output {
>>
>> struct drm_encoder encoder;
>> struct drm_connector connector;
>> + struct display_entity this;
>> + struct display_entity *output;
>
> Could you pick up a somewhat meaningful name? You know, there are too
> many variables with name "drm/connector/output/encoder"... :)
Well, it's supposed to be abstract. From the CDF point of view it could
be anything besides a panel. I know this makes it an output of an
output, but I can't think of anything better right now.
>> + if (entity->dev && entity->dev->of_node = pnode) {
>> + dev_dbg(output->dev, "connecting panel\n");
>> + output->output = display_entity_get(entity);
>> + display_entity_connect(&output->this, output->output);
>> + }
>> + of_node_put(pnode);
>> +
>> + break;
>> +
>> + case DISPLAY_ENTITY_NOTIFIER_DISCONNECT:
>> + if (!output->output || output->output != entity)
>> + break;
>> +
>> + dev_dbg(output->dev, "disconnecting panel\n");
>> + display_entity_disconnect(&output->this, output->output);
>> + output->output = NULL;
>> + display_entity_put(&output->this);
>
> No "display_entity_get" for "output->this", so I don't think we need
> "display_entity_put" here. If you register this entity with "release"
> callback and you wanna release "output->this", call the "release"
> function manually.
Oh, this was supposed to be called on output->output actually, to
balance the display_entity_get() of the connect event. Thanks for
catching this.
>> + /* register display entity */
>> + memset(&output->this, 0, sizeof(output->this));
>> + output->this.dev = drm->dev;
>
> Use "output->dev" here. Actually the device you wanna register it to
> display entity is the "encoder"(in drm terms), not "drm->dev". If we use
> "drm->dev" here, we will have all same device for all encoders(HDMI,
> DSI...).
Yes, that's absolutely right.
>> + /* register display notifier */
>> + output->display_notifier.dev = NULL;
>
> Set "display_notifier.dev" to NULL makes we have to compare with every
> display entity, just like what you do in "display_notify_callback":
>
> entity->dev && entity->dev->of_node = pnode
>
> So can we get the "struct device *" of panel here? Seems currently the
> "of" framework doesn't allow "device_node -> device".
Nope. AFAICT the device might not be instanciated at this point. We
become aware of it for the first time in the callback function. We also
don't want to defer probing until the panel is parsed first, since the
panel might also depend on resources of the display device.
Thanks,
Alex.
^ permalink raw reply
* Re: [RFC 3/4] drm: tegra: use the Common Display Framework
From: Mark Zhang @ 2013-01-30 6:50 UTC (permalink / raw)
To: Alexandre Courbot
Cc: Laurent Pinchart, Thierry Reding, Stephen Warren, Mark Zhang,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
linux-tegra-u79uwXL29TY76Z2rM5mHXA, gnurou-Re5JQEeQqe8AvxtiuMwx3w
In-Reply-To: <1359514939-15653-4-git-send-email-acourbot-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
On 01/30/2013 11:02 AM, Alexandre Courbot wrote:
> Make the tegra-drm driver use the Common Display Framework, letting it
> control the panel state according to the DPMS status.
>
> A "nvidia,panel" property is added to the output node of the Tegra DC
> that references the panel connected to a given output.
>
> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
> ---
[...]
> diff --git a/drivers/gpu/drm/tegra/drm.h b/drivers/gpu/drm/tegra/drm.h
> index 741b5dc..5e63c56 100644
> --- a/drivers/gpu/drm/tegra/drm.h
> +++ b/drivers/gpu/drm/tegra/drm.h
> @@ -17,6 +17,7 @@
> #include <drm/drm_gem_cma_helper.h>
> #include <drm/drm_fb_cma_helper.h>
> #include <drm/drm_fixed.h>
> +#include <video/display.h>
>
> struct tegra_framebuffer {
> struct drm_framebuffer base;
> @@ -147,6 +148,9 @@ struct tegra_output {
>
> struct drm_encoder encoder;
> struct drm_connector connector;
> + struct display_entity this;
> + struct display_entity *output;
Could you pick up a somewhat meaningful name? You know, there are too
many variables with name "drm/connector/output/encoder"... :)
> + struct display_entity_notifier display_notifier;
> };
>
[...]
> +static int display_notify_callback(struct display_entity_notifier *notifier,
> + struct display_entity *entity, int event)
> +{
> + struct tegra_output *output = display_notifier_to_output(notifier);
> + struct device_node *pnode;
> +
> + switch (event) {
> + case DISPLAY_ENTITY_NOTIFIER_CONNECT:
> + if (output->output)
> + break;
> +
> + pnode = of_parse_phandle(output->of_node, "nvidia,panel", 0);
> + if (!pnode)
> + break;
> +
> + if (entity->dev && entity->dev->of_node = pnode) {
> + dev_dbg(output->dev, "connecting panel\n");
> + output->output = display_entity_get(entity);
> + display_entity_connect(&output->this, output->output);
> + }
> + of_node_put(pnode);
> +
> + break;
> +
> + case DISPLAY_ENTITY_NOTIFIER_DISCONNECT:
> + if (!output->output || output->output != entity)
> + break;
> +
> + dev_dbg(output->dev, "disconnecting panel\n");
> + display_entity_disconnect(&output->this, output->output);
> + output->output = NULL;
> + display_entity_put(&output->this);
No "display_entity_get" for "output->this", so I don't think we need
"display_entity_put" here. If you register this entity with "release"
callback and you wanna release "output->this", call the "release"
function manually.
Only when you have "display_entity_get", use "display_entity_put" to
release.
> +
> + break;
> +
> + default:
> + dev_dbg(output->dev, "unhandled display event\n");
> + break;
> + }
> +
> + return 0;
> +}
> +
[...]
> int tegra_output_init(struct drm_device *drm, struct tegra_output *output)
> {
> int connector, encoder, err;
> @@ -250,6 +341,23 @@ int tegra_output_init(struct drm_device *drm, struct tegra_output *output)
>
> output->encoder.possible_crtcs = 0x3;
>
> + /* register display entity */
> + memset(&output->this, 0, sizeof(output->this));
> + output->this.dev = drm->dev;
Use "output->dev" here. Actually the device you wanna register it to
display entity is the "encoder"(in drm terms), not "drm->dev". If we use
"drm->dev" here, we will have all same device for all encoders(HDMI,
DSI...).
> + output->this.ops.video = &tegra_output_video_ops;
> + err = display_entity_register(&output->this);
> + if (err) {
> + dev_err(output->dev, "cannot register display entity\n");
> + return err;
> + }
> +
> + /* register display notifier */
> + output->display_notifier.dev = NULL;
Set "display_notifier.dev" to NULL makes we have to compare with every
display entity, just like what you do in "display_notify_callback":
entity->dev && entity->dev->of_node = pnode
So can we get the "struct device *" of panel here? Seems currently the
"of" framework doesn't allow "device_node -> device".
> + output->display_notifier.notify = display_notify_callback;
> + err = display_entity_register_notifier(&output->display_notifier);
> + if (err)
> + return err;
> +
> return 0;
>
> free_hpd:
> @@ -260,6 +368,12 @@ free_hpd:
>
> int tegra_output_exit(struct tegra_output *output)
> {
> + if (output->output)
> + display_entity_put(output->output);
> +
> + display_entity_unregister_notifier(&output->display_notifier);
> + display_entity_unregister(&output->this);
> +
> if (gpio_is_valid(output->hpd_gpio)) {
> free_irq(output->hpd_irq, output);
> gpio_free(output->hpd_gpio);
>
^ permalink raw reply
* [PATCH] pwm-backlight: handle BL_CORE_FBBLANK state
From: Alexandre Courbot @ 2013-01-30 6:19 UTC (permalink / raw)
To: Thierry Reding, Richard Purdie
Cc: linux-kernel, linux-fbdev, gnurou, Alexandre Courbot
According to include/linux/backlight.h, the fb_blank field is to be
removed and blank status should preferably be set by setting the
BL_CORE_FBBLANK bit of the state field. This patch ensures this
condition is also taken into account when updating the backlight state.
Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
drivers/video/backlight/pwm_bl.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/video/backlight/pwm_bl.c b/drivers/video/backlight/pwm_bl.c
index 069983c..4af6d13 100644
--- a/drivers/video/backlight/pwm_bl.c
+++ b/drivers/video/backlight/pwm_bl.c
@@ -41,10 +41,9 @@ static int pwm_backlight_update_status(struct backlight_device *bl)
int brightness = bl->props.brightness;
int max = bl->props.max_brightness;
- if (bl->props.power != FB_BLANK_UNBLANK)
- brightness = 0;
-
- if (bl->props.fb_blank != FB_BLANK_UNBLANK)
+ if (bl->props.power != FB_BLANK_UNBLANK ||
+ bl->props.fb_blank != FB_BLANK_UNBLANK ||
+ bl->props.state & BL_CORE_FBBLANK)
brightness = 0;
if (pb->notify)
--
1.8.1.2
^ permalink raw reply related
* [RFC 4/4] tegra: enable CDF and claa101 panel
From: Alexandre Courbot @ 2013-01-30 3:02 UTC (permalink / raw)
To: Laurent Pinchart, Thierry Reding, Stephen Warren, Mark Zhang
Cc: linux-kernel, linux-fbdev, linux-tegra, gnurou, Alexandre Courbot
In-Reply-To: <1359514939-15653-1-git-send-email-acourbot@nvidia.com>
Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
arch/arm/configs/tegra_defconfig | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/arm/configs/tegra_defconfig b/arch/arm/configs/tegra_defconfig
index a7827fd..6da0de2 100644
--- a/arch/arm/configs/tegra_defconfig
+++ b/arch/arm/configs/tegra_defconfig
@@ -150,6 +150,8 @@ CONFIG_BACKLIGHT_LCD_SUPPORT=y
CONFIG_BACKLIGHT_CLASS_DEVICE=y
# CONFIG_BACKLIGHT_GENERIC is not set
CONFIG_BACKLIGHT_PWM=y
+CONFIG_DISPLAY_CORE=y
+CONFIG_DISPLAY_PANEL_CLAA101WA01A=y
CONFIG_FRAMEBUFFER_CONSOLE=y
CONFIG_LOGO=y
CONFIG_SOUND=y
--
1.8.1.1
^ permalink raw reply related
* [RFC 3/4] drm: tegra: use the Common Display Framework
From: Alexandre Courbot @ 2013-01-30 3:02 UTC (permalink / raw)
To: Laurent Pinchart, Thierry Reding, Stephen Warren, Mark Zhang
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
linux-tegra-u79uwXL29TY76Z2rM5mHXA, gnurou-Re5JQEeQqe8AvxtiuMwx3w,
Alexandre Courbot
In-Reply-To: <1359514939-15653-1-git-send-email-acourbot-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
Make the tegra-drm driver use the Common Display Framework, letting it
control the panel state according to the DPMS status.
A "nvidia,panel" property is added to the output node of the Tegra DC
that references the panel connected to a given output.
Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
.../bindings/gpu/nvidia,tegra20-host1x.txt | 2 +
drivers/gpu/drm/tegra/drm.h | 16 +++
drivers/gpu/drm/tegra/output.c | 118 ++++++++++++++++++++-
3 files changed, 134 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/gpu/nvidia,tegra20-host1x.txt b/Documentation/devicetree/bindings/gpu/nvidia,tegra20-host1x.txt
index b4fa934..9c65e8e 100644
--- a/Documentation/devicetree/bindings/gpu/nvidia,tegra20-host1x.txt
+++ b/Documentation/devicetree/bindings/gpu/nvidia,tegra20-host1x.txt
@@ -67,6 +67,7 @@ of the following host1x client modules:
- nvidia,ddc-i2c-bus: phandle of an I2C controller used for DDC EDID probing
- nvidia,hpd-gpio: specifies a GPIO used for hotplug detection
- nvidia,edid: supplies a binary EDID blob
+ - nvidia,panel: phandle of a display entity connected to this output
- hdmi: High Definition Multimedia Interface
@@ -81,6 +82,7 @@ of the following host1x client modules:
- nvidia,ddc-i2c-bus: phandle of an I2C controller used for DDC EDID probing
- nvidia,hpd-gpio: specifies a GPIO used for hotplug detection
- nvidia,edid: supplies a binary EDID blob
+ - nvidia,panel: phandle of a display entity connected to this output
- tvo: TV encoder output
diff --git a/drivers/gpu/drm/tegra/drm.h b/drivers/gpu/drm/tegra/drm.h
index 741b5dc..5e63c56 100644
--- a/drivers/gpu/drm/tegra/drm.h
+++ b/drivers/gpu/drm/tegra/drm.h
@@ -17,6 +17,7 @@
#include <drm/drm_gem_cma_helper.h>
#include <drm/drm_fb_cma_helper.h>
#include <drm/drm_fixed.h>
+#include <video/display.h>
struct tegra_framebuffer {
struct drm_framebuffer base;
@@ -147,6 +148,9 @@ struct tegra_output {
struct drm_encoder encoder;
struct drm_connector connector;
+ struct display_entity this;
+ struct display_entity *output;
+ struct display_entity_notifier display_notifier;
};
static inline struct tegra_output *encoder_to_output(struct drm_encoder *e)
@@ -159,6 +163,18 @@ static inline struct tegra_output *connector_to_output(struct drm_connector *c)
return container_of(c, struct tegra_output, connector);
}
+static inline
+struct tegra_output *display_entity_to_output(struct display_entity *e)
+{
+ return container_of(e, struct tegra_output, this);
+}
+
+static inline struct tegra_output *
+display_notifier_to_output(struct display_entity_notifier *n)
+{
+ return container_of(n, struct tegra_output, display_notifier);
+}
+
static inline int tegra_output_enable(struct tegra_output *output)
{
if (output && output->ops && output->ops->enable)
diff --git a/drivers/gpu/drm/tegra/output.c b/drivers/gpu/drm/tegra/output.c
index 8140fc6..d5bf37a 100644
--- a/drivers/gpu/drm/tegra/output.c
+++ b/drivers/gpu/drm/tegra/output.c
@@ -105,6 +105,29 @@ static const struct drm_encoder_funcs encoder_funcs = {
static void tegra_encoder_dpms(struct drm_encoder *encoder, int mode)
{
+ struct tegra_output *output = encoder_to_output(encoder);
+ enum display_entity_state state;
+
+ if (!output->output)
+ return;
+
+ switch (mode) {
+ case DRM_MODE_DPMS_ON:
+ state = DISPLAY_ENTITY_STATE_ON;
+ break;
+ case DRM_MODE_DPMS_STANDBY:
+ case DRM_MODE_DPMS_SUSPEND:
+ state = DISPLAY_ENTITY_STATE_STANDBY;
+ break;
+ case DRM_MODE_DPMS_OFF:
+ state = DISPLAY_ENTITY_STATE_OFF;
+ break;
+ default:
+ dev_warn(output->dev, "unknown DPMS state, ignoring\n");
+ return;
+ }
+
+ display_entity_set_state(output->output, state);
}
static bool tegra_encoder_mode_fixup(struct drm_encoder *encoder,
@@ -129,9 +152,14 @@ static void tegra_encoder_mode_set(struct drm_encoder *encoder,
struct tegra_output *output = encoder_to_output(encoder);
int err;
- err = tegra_output_enable(output);
+ if (output->output)
+ err = display_entity_set_state(output->output,
+ DISPLAY_ENTITY_STATE_ON);
+ else
+ err = tegra_output_enable(output);
+
if (err < 0)
- dev_err(encoder->dev->dev, "tegra_output_enable(): %d\n", err);
+ dev_err(encoder->dev->dev, "cannot enable output: %d\n", err);
}
static const struct drm_encoder_helper_funcs encoder_helper_funcs = {
@@ -185,6 +213,69 @@ int tegra_output_parse_dt(struct tegra_output *output)
return 0;
}
+static int display_notify_callback(struct display_entity_notifier *notifier,
+ struct display_entity *entity, int event)
+{
+ struct tegra_output *output = display_notifier_to_output(notifier);
+ struct device_node *pnode;
+
+ switch (event) {
+ case DISPLAY_ENTITY_NOTIFIER_CONNECT:
+ if (output->output)
+ break;
+
+ pnode = of_parse_phandle(output->of_node, "nvidia,panel", 0);
+ if (!pnode)
+ break;
+
+ if (entity->dev && entity->dev->of_node = pnode) {
+ dev_dbg(output->dev, "connecting panel\n");
+ output->output = display_entity_get(entity);
+ display_entity_connect(&output->this, output->output);
+ }
+ of_node_put(pnode);
+
+ break;
+
+ case DISPLAY_ENTITY_NOTIFIER_DISCONNECT:
+ if (!output->output || output->output != entity)
+ break;
+
+ dev_dbg(output->dev, "disconnecting panel\n");
+ display_entity_disconnect(&output->this, output->output);
+ output->output = NULL;
+ display_entity_put(&output->this);
+
+ break;
+
+ default:
+ dev_dbg(output->dev, "unhandled display event\n");
+ break;
+ }
+
+ return 0;
+}
+
+static int tegra_output_set_stream(struct display_entity *entity,
+ enum display_entity_stream_state state)
+{
+ struct tegra_output *output = display_entity_to_output(entity);
+
+ switch (state) {
+ case DISPLAY_ENTITY_STATE_OFF:
+ case DISPLAY_ENTITY_STATE_STANDBY:
+ return output->ops->disable(output);
+ case DISPLAY_ENTITY_STATE_ON:
+ return output->ops->enable(output);
+ }
+
+ return 0;
+}
+
+static struct display_entity_video_ops tegra_output_video_ops = {
+ .set_stream = tegra_output_set_stream,
+};
+
int tegra_output_init(struct drm_device *drm, struct tegra_output *output)
{
int connector, encoder, err;
@@ -250,6 +341,23 @@ int tegra_output_init(struct drm_device *drm, struct tegra_output *output)
output->encoder.possible_crtcs = 0x3;
+ /* register display entity */
+ memset(&output->this, 0, sizeof(output->this));
+ output->this.dev = drm->dev;
+ output->this.ops.video = &tegra_output_video_ops;
+ err = display_entity_register(&output->this);
+ if (err) {
+ dev_err(output->dev, "cannot register display entity\n");
+ return err;
+ }
+
+ /* register display notifier */
+ output->display_notifier.dev = NULL;
+ output->display_notifier.notify = display_notify_callback;
+ err = display_entity_register_notifier(&output->display_notifier);
+ if (err)
+ return err;
+
return 0;
free_hpd:
@@ -260,6 +368,12 @@ free_hpd:
int tegra_output_exit(struct tegra_output *output)
{
+ if (output->output)
+ display_entity_put(output->output);
+
+ display_entity_unregister_notifier(&output->display_notifier);
+ display_entity_unregister(&output->this);
+
if (gpio_is_valid(output->hpd_gpio)) {
free_irq(output->hpd_irq, output);
gpio_free(output->hpd_gpio);
--
1.8.1.1
^ permalink raw reply related
* [RFC 2/4] tegra: ventana: add display and backlight DT nodes
From: Alexandre Courbot @ 2013-01-30 3:02 UTC (permalink / raw)
To: Laurent Pinchart, Thierry Reding, Stephen Warren, Mark Zhang
Cc: linux-kernel, linux-fbdev, linux-tegra, gnurou, Alexandre Courbot
In-Reply-To: <1359514939-15653-1-git-send-email-acourbot@nvidia.com>
Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
arch/arm/boot/dts/tegra20-ventana.dts | 34 +++++++++++++++++++++++++++++++---
1 file changed, 31 insertions(+), 3 deletions(-)
diff --git a/arch/arm/boot/dts/tegra20-ventana.dts b/arch/arm/boot/dts/tegra20-ventana.dts
index adc4754..48f4e6d 100644
--- a/arch/arm/boot/dts/tegra20-ventana.dts
+++ b/arch/arm/boot/dts/tegra20-ventana.dts
@@ -10,6 +10,16 @@
reg = <0x00000000 0x40000000>;
};
+ host1x {
+ dc@54200000 {
+ rgb {
+ status = "okay";
+ nvidia,ddc-i2c-bus = <&lcd_ddc>;
+ nvidia,panel = <&panel>;
+ };
+ };
+ };
+
pinmux {
pinctrl-names = "default";
pinctrl-0 = <&state_default>;
@@ -341,7 +351,7 @@
#size-cells = <0>;
};
- i2c@1 {
+ lcd_ddc: i2c@1 {
reg = <1>;
#address-cells = <1>;
#size-cells = <0>;
@@ -516,6 +526,24 @@
bus-width = <8>;
};
+ backlight: backlight {
+ compatible = "pwm-backlight";
+ pwms = <&pwm 2 5000000>;
+
+ brightness-levels = <0 16 32 48 64 80 96 112 128 144 160 176 192 208 224 240 255>;
+ default-brightness-level = <10>;
+ };
+
+ panel: panel {
+ compatible = "chunghwa,claa101wa01a";
+
+ pnl-supply = <&vdd_pnl_reg>;
+ pnl-enable-gpios = <&gpio 10 0>;
+
+ bl-supply = <&vdd_bl_reg>;
+ bl-enable-gpios = <&gpio 28 0>;
+ };
+
regulators {
compatible = "simple-bus";
#address-cells = <1>;
@@ -549,7 +577,7 @@
enable-active-high;
};
- regulator@3 {
+ vdd_pnl_reg: regulator@3 {
compatible = "regulator-fixed";
reg = <3>;
regulator-name = "vdd_pnl";
@@ -559,7 +587,7 @@
enable-active-high;
};
- regulator@4 {
+ vdd_bl_reg: regulator@4 {
compatible = "regulator-fixed";
reg = <4>;
regulator-name = "vdd_bl";
--
1.8.1.1
^ permalink raw reply related
* [RFC 1/4] video: panel: add CLAA101WA01A panel support
From: Alexandre Courbot @ 2013-01-30 3:02 UTC (permalink / raw)
To: Laurent Pinchart, Thierry Reding, Stephen Warren, Mark Zhang
Cc: linux-kernel, linux-fbdev, linux-tegra, gnurou, Alexandre Courbot
In-Reply-To: <1359514939-15653-1-git-send-email-acourbot@nvidia.com>
Add support for the Chunghwa CLAA101WA01A display panel.
Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
.../video/display/chunghwa,claa101wa01a.txt | 8 +
drivers/video/display/Kconfig | 8 +
drivers/video/display/Makefile | 1 +
drivers/video/display/panel-claa101wa01a.c | 209 +++++++++++++++++++++
4 files changed, 226 insertions(+)
create mode 100644 Documentation/devicetree/bindings/video/display/chunghwa,claa101wa01a.txt
create mode 100644 drivers/video/display/panel-claa101wa01a.c
diff --git a/Documentation/devicetree/bindings/video/display/chunghwa,claa101wa01a.txt b/Documentation/devicetree/bindings/video/display/chunghwa,claa101wa01a.txt
new file mode 100644
index 0000000..cfdc7fd
--- /dev/null
+++ b/Documentation/devicetree/bindings/video/display/chunghwa,claa101wa01a.txt
@@ -0,0 +1,8 @@
+Chunghwa CLAA101WA01A Display Panel
+
+Required properties:
+- compatible: "chunghwa,claa101wa01a"
+- pnl-supply: regulator controlling power supply to the panel
+- bl-supply: regulator controlling power supply to the backlight
+- pnl-enable-gpios: GPIO that enables the panel
+- bl-enable-gpios: GPIO that enables the backlight
diff --git a/drivers/video/display/Kconfig b/drivers/video/display/Kconfig
index 9ca2e60..6902abb 100644
--- a/drivers/video/display/Kconfig
+++ b/drivers/video/display/Kconfig
@@ -32,4 +32,12 @@ config DISPLAY_PANEL_R61517
If you are in doubt, say N.
+config DISPLAY_PANEL_CLAA101WA01A
+ tristate "Chunghwa CLAA101WA01A Display Panel"
+ select BACKLIGHT_PWM
+ ---help---
+ Support for the Chunghwa CLAA101WA01A Display Panel.
+
+ If you are in doubt, say N.
+
endif # DISPLAY_CORE
diff --git a/drivers/video/display/Makefile b/drivers/video/display/Makefile
index ec557a1..19084a2 100644
--- a/drivers/video/display/Makefile
+++ b/drivers/video/display/Makefile
@@ -2,3 +2,4 @@ obj-$(CONFIG_DISPLAY_CORE) += display-core.o
obj-$(CONFIG_DISPLAY_PANEL_DPI) += panel-dpi.o
obj-$(CONFIG_DISPLAY_PANEL_R61505) += panel-r61505.o
obj-$(CONFIG_DISPLAY_PANEL_R61517) += panel-r61517.o
+obj-$(CONFIG_DISPLAY_PANEL_CLAA101WA01A) += panel-claa101wa01a.o
diff --git a/drivers/video/display/panel-claa101wa01a.c b/drivers/video/display/panel-claa101wa01a.c
new file mode 100644
index 0000000..93ae86b
--- /dev/null
+++ b/drivers/video/display/panel-claa101wa01a.c
@@ -0,0 +1,209 @@
+/*
+ * CLAA101WA01A Display Panel
+ *
+ * Copyright (C) 2013 NVIDIA CORPORATION. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/err.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <linux/delay.h>
+#include <linux/regulator/consumer.h>
+#include <linux/of.h>
+#include <linux/of_gpio.h>
+
+#include <video/display.h>
+
+#define CLAA101WA01A_WIDTH 223
+#define CLAA101WA01A_HEIGHT 125
+
+struct panel_claa101 {
+ struct display_entity entity;
+ struct regulator *vdd_pnl;
+ struct regulator *vdd_bl;
+ /* Enable GPIOs */
+ int pnl_enable;
+ int bl_enable;
+};
+
+#define to_panel_claa101(p) container_of(p, struct panel_claa101, entity)
+
+static int panel_claa101_set_state(struct display_entity *entity,
+ enum display_entity_state state)
+{
+ struct panel_claa101 *panel = to_panel_claa101(entity);
+
+ /* OFF and STANDBY are equivalent to us */
+ if (state = DISPLAY_ENTITY_STATE_STANDBY)
+ state = DISPLAY_ENTITY_STATE_OFF;
+
+ switch (state) {
+ case DISPLAY_ENTITY_STATE_OFF:
+ case DISPLAY_ENTITY_STATE_STANDBY:
+ if (entity->source)
+ display_entity_set_stream(entity->source,
+ DISPLAY_ENTITY_STREAM_STOPPED);
+
+ /* TODO error checking? */
+ gpio_set_value_cansleep(panel->bl_enable, 0);
+ usleep_range(10000, 10000);
+ regulator_disable(panel->vdd_bl);
+ usleep_range(200000, 200000);
+ gpio_set_value_cansleep(panel->pnl_enable, 0);
+ regulator_disable(panel->vdd_pnl);
+ break;
+
+ case DISPLAY_ENTITY_STATE_ON:
+ regulator_enable(panel->vdd_pnl);
+ gpio_set_value_cansleep(panel->pnl_enable, 1);
+ usleep_range(200000, 200000);
+ regulator_enable(panel->vdd_bl);
+ usleep_range(10000, 10000);
+ gpio_set_value_cansleep(panel->bl_enable, 1);
+
+ if (entity->source)
+ display_entity_set_stream(entity->source,
+ DISPLAY_ENTITY_STREAM_CONTINUOUS);
+ break;
+ }
+
+ return 0;
+}
+
+static int panel_claa101_get_modes(struct display_entity *entity,
+ const struct videomode **modes)
+{
+ /* TODO get modes from EDID? */
+ return 0;
+}
+
+static int panel_claa101_get_size(struct display_entity *entity,
+ unsigned int *width, unsigned int *height)
+{
+ *width = CLAA101WA01A_WIDTH;
+ *height = CLAA101WA01A_HEIGHT;
+
+ return 0;
+}
+
+static int panel_claa101_get_params(struct display_entity *entity,
+ struct display_entity_interface_params *params)
+{
+ return 0;
+}
+
+static const struct display_entity_control_ops panel_claa101_control_ops = {
+ .set_state = panel_claa101_set_state,
+ .get_modes = panel_claa101_get_modes,
+ .get_size = panel_claa101_get_size,
+ .get_params = panel_claa101_get_params,
+};
+
+static int __init panel_claa101_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct panel_claa101 *panel;
+ int err;
+
+ panel = devm_kzalloc(dev, sizeof(*panel), GFP_KERNEL);
+ if (!panel)
+ return -ENOMEM;
+
+ panel->vdd_pnl = devm_regulator_get(dev, "pnl");
+ if (IS_ERR(panel->vdd_pnl)) {
+ dev_err(dev, "cannot get vdd regulator\n");
+ return PTR_ERR(panel->vdd_pnl);
+ }
+
+ panel->vdd_bl = devm_regulator_get(dev, "bl");
+ if (IS_ERR(panel->vdd_bl)) {
+ dev_err(dev, "cannot get bl regulator\n");
+ return PTR_ERR(panel->vdd_bl);
+ }
+
+ err = of_get_named_gpio(dev->of_node, "pnl-enable-gpios", 0);
+ if (err < 0) {
+ dev_err(dev, "cannot find panel enable GPIO!\n");
+ return err;
+ }
+ panel->pnl_enable = err;
+ err = devm_gpio_request_one(dev, panel->pnl_enable,
+ GPIOF_DIR_OUT | GPIOF_INIT_LOW, "panel");
+ if (err < 0) {
+ dev_err(dev, "cannot acquire panel enable GPIO!\n");
+ return err;
+ }
+
+ err = of_get_named_gpio(dev->of_node, "bl-enable-gpios", 0);
+ if (err < 0) {
+ dev_err(dev, "cannot find backlight enable GPIO!\n");
+ return err;
+ }
+ panel->bl_enable = err;
+ err = devm_gpio_request_one(dev, panel->bl_enable,
+ GPIOF_DIR_OUT | GPIOF_INIT_LOW, "backlight");
+ if (err < 0) {
+ dev_err(dev, "cannot acquire backlight enable GPIO!\n");
+ return err;
+ }
+
+ panel->entity.dev = dev;
+ panel->entity.ops.ctrl = &panel_claa101_control_ops;
+ err = display_entity_register(&panel->entity);
+ if (err < 0)
+ return err;
+
+ platform_set_drvdata(pdev, panel);
+
+ dev_info(dev, "%s successful\n", __func__);
+
+ return 0;
+}
+
+static int __exit panel_claa101_remove(struct platform_device *pdev)
+{
+ struct panel_claa101 *panel = platform_get_drvdata(pdev);
+
+ display_entity_unregister(&panel->entity);
+
+ return 0;
+}
+
+#ifdef CONFIG_OF
+static struct of_device_id panel_claa101_of_match[] = {
+ { .compatible = "chunghwa,claa101wa01a", },
+ { },
+};
+MODULE_DEVICE_TABLE(of, pwm_backlight_of_match);
+#else
+#endif
+
+static const struct dev_pm_ops panel_claa101_dev_pm_ops = {
+};
+
+static struct platform_driver panel_claa101_driver = {
+ .probe = panel_claa101_probe,
+ .remove = panel_claa101_remove,
+ .driver = {
+ .name = "panel_claa101wa01a",
+ .owner = THIS_MODULE,
+#ifdef CONFIG_PM
+ .pm = &panel_claa101_dev_pm_ops,
+#endif
+#ifdef CONFIG_OF
+ .of_match_table = of_match_ptr(panel_claa101_of_match),
+#endif
+ },
+};
+
+module_platform_driver(panel_claa101_driver);
+
+MODULE_AUTHOR("Alexandre Courbot <acourbot@nvidia.com>");
+MODULE_DESCRIPTION("Chunghwa CLAA101WA01A Display Panel");
+MODULE_LICENSE("GPL");
--
1.8.1.1
^ permalink raw reply related
* [RFC 0/4] Use the Common Display Framework in tegra-drm
From: Alexandre Courbot @ 2013-01-30 3:02 UTC (permalink / raw)
To: Laurent Pinchart, Thierry Reding, Stephen Warren, Mark Zhang
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
linux-tegra-u79uwXL29TY76Z2rM5mHXA, gnurou-Re5JQEeQqe8AvxtiuMwx3w,
Alexandre Courbot
This series leverages the (still work-in-progress) Common Display Framework to
add panel support to the tegra-drm driver. It also adds a driver for the
CLAA101WA01A panel used on the Ventana board.
The CDF is a moving target but Tegra needs some sort of display framework and
even in its current state the CDF seems to be the best candidate. Besides, by
using the CDF from now on we hope to provide useful feedback to Laurent and the
other CDF designers.
The changes to tegra-drm are rather minimal. Panels are referenced from Tegra DC
output nodes through the "nvidia,panel" property. This property is looked up
when a display connect notification is received in order to see if it points to
the connected display entity. If it does, the entity is then used for output.
The DPMS states are then propagated to the output entity, which is then supposed
to call back into the set_stream() hook in order to enable/disable the output
stream as needed.
Although the overall design seems to work ok, a few specific issues need to be
addressed:
1) The CDF has a get_modes() hook, but this is already implemented by
tegra_connector_get_modes(). Ideally everything should be moved to the CDF hook,
but Tegra's implementation uses DRM functions to retrieve the EDID and CDF
should, AFAIK, remain DRM-agnostic.
2) There is currently no panel/backlight interaction, e.g. backlight status is
controlled through FB events, independantly from the panel state. It could make
sense to have the panel DT node reference the backlight and control it as part
of its own power on/off sequence. Right now however, a backlight device cannot
ignore FB events.
3) Probably related to 2), now the backlight's power controls are part of the
panel driver, because the pwm-backlight driver cannot control the power
regulator and enable GPIO. This means that the backlight power is not turned off
when its brightness is set to 0 through sysfs. Once again this speaks in favor
of having stronger panel/backlight interaction: for instance, the panel driver
could reference the backlight and hijack its update_status() op to replace it by
one that does the correct power sequencing before calling the original function.
This would require some extra infrastructure though. Another possibility would
be to have a dedicated backlight driver for each panel, with its own
"compatible" string.
The code is based on 3.8rc5 + Steffen's videomode patches and the CDF v2.
Alexandre Courbot (4):
video: panel: add CLAA101WA01A panel support
tegra: ventana: add display and backlight DT nodes
drm: tegra: use the Common Display Framework
tegra: enable CDF and claa101 panel
.../bindings/gpu/nvidia,tegra20-host1x.txt | 2 +
.../video/display/chunghwa,claa101wa01a.txt | 8 +
arch/arm/boot/dts/tegra20-ventana.dts | 34 +++-
arch/arm/configs/tegra_defconfig | 2 +
drivers/gpu/drm/tegra/drm.h | 16 ++
drivers/gpu/drm/tegra/output.c | 118 +++++++++++-
drivers/video/display/Kconfig | 8 +
drivers/video/display/Makefile | 1 +
drivers/video/display/panel-claa101wa01a.c | 209 +++++++++++++++++++++
9 files changed, 393 insertions(+), 5 deletions(-)
create mode 100644 Documentation/devicetree/bindings/video/display/chunghwa,claa101wa01a.txt
create mode 100644 drivers/video/display/panel-claa101wa01a.c
--
1.8.1.1
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox