| Message ID | 1376068510-30363-9-git-send-email-laurent.pinchart+renesas@ideasonboard.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Aug 9, 2013 at 1:14 PM, Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com> wrote: > MIPI DBI is a configurable-width parallel display bus that transmits > commands and data. > > Add a new DBI Linux bus type that implements the usual bus > infrastructure (including devices and drivers (un)registration and > matching, and bus configuration and access functions). > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > drivers/video/display/Kconfig | 8 ++ > drivers/video/display/Makefile | 1 + > drivers/video/display/mipi-dbi-bus.c | 234 +++++++++++++++++++++++++++++++++++ > include/video/display.h | 4 + > include/video/mipi-dbi-bus.h | 125 +++++++++++++++++++ > 5 files changed, 372 insertions(+) > create mode 100644 drivers/video/display/mipi-dbi-bus.c > create mode 100644 include/video/mipi-dbi-bus.h > > diff --git a/drivers/video/display/Kconfig b/drivers/video/display/Kconfig > index 1d533e7..f7532c1 100644 > --- a/drivers/video/display/Kconfig > +++ b/drivers/video/display/Kconfig > @@ -2,3 +2,11 @@ menuconfig DISPLAY_CORE > tristate "Display Core" > ---help--- > Support common display framework for graphics devices. > + > +if DISPLAY_CORE > + > +config DISPLAY_MIPI_DBI > + tristate > + default n > + > +endif # DISPLAY_CORE > diff --git a/drivers/video/display/Makefile b/drivers/video/display/Makefile > index b907aad..59022d2 100644 > --- a/drivers/video/display/Makefile > +++ b/drivers/video/display/Makefile > @@ -1,3 +1,4 @@ > display-y := display-core.o \ > display-notifier.o > obj-$(CONFIG_DISPLAY_CORE) += display.o > +obj-$(CONFIG_DISPLAY_MIPI_DBI) += mipi-dbi-bus.o > diff --git a/drivers/video/display/mipi-dbi-bus.c b/drivers/video/display/mipi-dbi-bus.c > new file mode 100644 > index 0000000..791fb4d > --- /dev/null > +++ b/drivers/video/display/mipi-dbi-bus.c > @@ -0,0 +1,234 @@ > +/* > + * MIPI DBI Bus > + * > + * 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/device.h> > +#include <linux/export.h> > +#include <linux/kernel.h> > +#include <linux/list.h> > +#include <linux/module.h> > +#include <linux/mutex.h> > +#include <linux/pm.h> > +#include <linux/pm_runtime.h> > + > +#include <video/mipi-dbi-bus.h> > + > +/* ----------------------------------------------------------------------------- > + * Bus operations > + */ > + > +int mipi_dbi_set_data_width(struct mipi_dbi_device *dev, unsigned int width) > +{ > + if (width != 8 && width != 16) > + return -EINVAL; > + > + dev->data_width = width; > + return 0; > +} > +EXPORT_SYMBOL_GPL(mipi_dbi_set_data_width); > + > +int mipi_dbi_write_command(struct mipi_dbi_device *dev, u16 cmd) > +{ > + return dev->bus->ops->write_command(dev->bus, dev, cmd); > +} > +EXPORT_SYMBOL_GPL(mipi_dbi_write_command); > + > +int mipi_dbi_write_data(struct mipi_dbi_device *dev, const u8 *data, > + size_t len) > +{ > + return dev->bus->ops->write_data(dev->bus, dev, data, len); > +} > +EXPORT_SYMBOL_GPL(mipi_dbi_write_data); > + > +int mipi_dbi_read_data(struct mipi_dbi_device *dev, u8 *data, size_t len) > +{ > + return dev->bus->ops->read_data(dev->bus, dev, data, len); > +} > +EXPORT_SYMBOL_GPL(mipi_dbi_read_data); > + > +/* ----------------------------------------------------------------------------- > + * Bus type > + */ > + > +static const struct mipi_dbi_device_id * > +mipi_dbi_match_id(const struct mipi_dbi_device_id *id, > + struct mipi_dbi_device *dev) > +{ > + while (id->name[0]) { > + if (strcmp(dev->name, id->name) == 0) { > + dev->id_entry = id; > + return id; > + } > + id++; > + } > + return NULL; > +} > + > +static int mipi_dbi_match(struct device *_dev, struct device_driver *_drv) > +{ > + struct mipi_dbi_device *dev = to_mipi_dbi_device(_dev); > + struct mipi_dbi_driver *drv = to_mipi_dbi_driver(_drv); > + > + if (drv->id_table) > + return mipi_dbi_match_id(drv->id_table, dev) != NULL; > + > + return (strcmp(dev->name, _drv->name) == 0); > +} > + > +static ssize_t modalias_show(struct device *_dev, struct device_attribute *a, > + char *buf) > +{ > + struct mipi_dbi_device *dev = to_mipi_dbi_device(_dev); > + int len = snprintf(buf, PAGE_SIZE, MIPI_DBI_MODULE_PREFIX "%s\n", > + dev->name); > + > + return (len >= PAGE_SIZE) ? (PAGE_SIZE - 1) : len; > +} > + > +static struct device_attribute mipi_dbi_dev_attrs[] = { > + __ATTR_RO(modalias), > + __ATTR_NULL, > +}; > + > +static int mipi_dbi_uevent(struct device *_dev, struct kobj_uevent_env *env) > +{ > + struct mipi_dbi_device *dev = to_mipi_dbi_device(_dev); > + > + add_uevent_var(env, "MODALIAS=%s%s", MIPI_DBI_MODULE_PREFIX, > + dev->name); > + return 0; > +} > + > +static const struct dev_pm_ops mipi_dbi_dev_pm_ops = { > + .runtime_suspend = pm_generic_runtime_suspend, > + .runtime_resume = pm_generic_runtime_resume, > + .suspend = pm_generic_suspend, > + .resume = pm_generic_resume, > + .freeze = pm_generic_freeze, > + .thaw = pm_generic_thaw, > + .poweroff = pm_generic_poweroff, > + .restore = pm_generic_restore, > +}; > + > +static struct bus_type mipi_dbi_bus_type = { > + .name = "mipi-dbi", > + .dev_attrs = mipi_dbi_dev_attrs, > + .match = mipi_dbi_match, > + .uevent = mipi_dbi_uevent, > + .pm = &mipi_dbi_dev_pm_ops, > +}; > + > +/* ----------------------------------------------------------------------------- > + * Device and driver (un)registration > + */ > + > +/** > + * mipi_dbi_device_register - register a DBI device > + * @dev: DBI device we're registering > + */ > +int mipi_dbi_device_register(struct mipi_dbi_device *dev, > + struct mipi_dbi_bus *bus) > +{ > + device_initialize(&dev->dev); > + > + dev->bus = bus; > + dev->dev.bus = &mipi_dbi_bus_type; > + dev->dev.parent = bus->dev; > + > + if (dev->id != -1) > + dev_set_name(&dev->dev, "%s.%d", dev->name, dev->id); > + else > + dev_set_name(&dev->dev, "%s", dev->name); > + > + return device_add(&dev->dev); > +} > +EXPORT_SYMBOL_GPL(mipi_dbi_device_register); > + > +/** > + * mipi_dbi_device_unregister - unregister a DBI device > + * @dev: DBI device we're unregistering > + */ > +void mipi_dbi_device_unregister(struct mipi_dbi_device *dev) > +{ > + device_del(&dev->dev); > + put_device(&dev->dev); > +} > +EXPORT_SYMBOL_GPL(mipi_dbi_device_unregister); > + > +static int mipi_dbi_drv_probe(struct device *_dev) > +{ > + struct mipi_dbi_driver *drv = to_mipi_dbi_driver(_dev->driver); > + struct mipi_dbi_device *dev = to_mipi_dbi_device(_dev); > + > + return drv->probe(dev); > +} > + > +static int mipi_dbi_drv_remove(struct device *_dev) > +{ > + struct mipi_dbi_driver *drv = to_mipi_dbi_driver(_dev->driver); > + struct mipi_dbi_device *dev = to_mipi_dbi_device(_dev); > + int ret; > + > + ret = drv->remove(dev); > + if (ret < 0) > + return ret; > + > + mipi_dbi_set_drvdata(dev, NULL); > + > + return 0; > +} > + > +/** > + * mipi_dbi_driver_register - register a driver for DBI devices > + * @drv: DBI driver structure > + */ > +int mipi_dbi_driver_register(struct mipi_dbi_driver *drv) > +{ > + drv->driver.bus = &mipi_dbi_bus_type; > + if (drv->probe) > + drv->driver.probe = mipi_dbi_drv_probe; > + if (drv->remove) > + drv->driver.remove = mipi_dbi_drv_remove; > + > + return driver_register(&drv->driver); > +} > +EXPORT_SYMBOL_GPL(mipi_dbi_driver_register); > + > +/** > + * mipi_dbi_driver_unregister - unregister a driver for DBI devices > + * @drv: DBI driver structure > + */ > +void mipi_dbi_driver_unregister(struct mipi_dbi_driver *drv) > +{ > + driver_unregister(&drv->driver); > +} > +EXPORT_SYMBOL_GPL(mipi_dbi_driver_unregister); > + > +/* ----------------------------------------------------------------------------- > + * Init/exit > + */ > + > +static int __init mipi_dbi_init(void) > +{ > + return bus_register(&mipi_dbi_bus_type); > +} > + > +static void __exit mipi_dbi_exit(void) > +{ > + bus_unregister(&mipi_dbi_bus_type); > +} > + > +module_init(mipi_dbi_init); > +module_exit(mipi_dbi_exit) > + > +MODULE_AUTHOR("Laurent Pinchart <laurent.pinchart@ideasonboard.com>"); > +MODULE_DESCRIPTION("MIPI DBI Bus"); > +MODULE_LICENSE("GPL"); > diff --git a/include/video/display.h b/include/video/display.h > index ba319d6..3138401 100644 > --- a/include/video/display.h > +++ b/include/video/display.h > @@ -17,6 +17,7 @@ > #include <linux/list.h> > #include <linux/module.h> > #include <media/media-entity.h> > +#include <video/mipi-dbi-bus.h> > > #define DISPLAY_PIXEL_CODING(option, type, from, to, variant) \ > (((option) << 17) | ((type) << 13) | ((variant) << 10) | \ > @@ -189,6 +190,9 @@ enum display_entity_interface_type { > > struct display_entity_interface_params { > enum display_entity_interface_type type; > + union { > + struct mipi_dbi_interface_params dbi; > + } p; > }; > > struct display_entity_control_ops { > diff --git a/include/video/mipi-dbi-bus.h b/include/video/mipi-dbi-bus.h > new file mode 100644 > index 0000000..876b69d > --- /dev/null > +++ b/include/video/mipi-dbi-bus.h > @@ -0,0 +1,125 @@ > +/* > + * MIPI DBI Bus > + * > + * 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 __MIPI_DBI_BUS_H__ > +#define __MIPI_DBI_BUS_H__ > + > +#include <linux/device.h> > + > +struct mipi_dbi_bus; > +struct mipi_dbi_device; > + > +struct mipi_dbi_bus_ops { > + int (*write_command)(struct mipi_dbi_bus *bus, > + struct mipi_dbi_device *dev, u16 cmd); > + int (*write_data)(struct mipi_dbi_bus *bus, struct mipi_dbi_device *dev, > + const u8 *data, size_t len); > + int (*read_data)(struct mipi_dbi_bus *bus, struct mipi_dbi_device *dev, > + u8 *data, size_t len); > +}; > + > +struct mipi_dbi_bus { > + struct device *dev; > + const struct mipi_dbi_bus_ops *ops; > +}; > + > +#define MIPI_DBI_MODULE_PREFIX "mipi-dbi:" > +#define MIPI_DBI_NAME_SIZE 32 > + > +struct mipi_dbi_device_id { > + char name[MIPI_DBI_NAME_SIZE]; > + __kernel_ulong_t driver_data /* Data private to the driver */ > + __aligned(sizeof(__kernel_ulong_t)); > +}; > + > +enum mipi_dbi_interface_type { > + MIPI_DBI_INTERFACE_TYPE_A, > + MIPI_DBI_INTERFACE_TYPE_B, > +}; > + > +#define MIPI_DBI_INTERFACE_TE (1 << 0) > + > +struct mipi_dbi_interface_params { > + enum mipi_dbi_interface_type type; > + unsigned int flags; > + > + unsigned int cs_setup; > + unsigned int rd_setup; > + unsigned int rd_latch; > + unsigned int rd_cycle; > + unsigned int rd_hold; > + unsigned int wr_setup; > + unsigned int wr_cycle; > + unsigned int wr_hold; > +}; > + > +#define MIPI_DBI_FLAG_ALIGN_LEFT (1 << 0) > + > +struct mipi_dbi_device { > + const char *name; > + int id; > + struct device dev; just fwiw, we need the "framework" to be agnostic of the association between devices and entities in the framework. Some things that are multiple entities may actually map to a single device in hw (and possibly visa versa?). Otherwise we end up having to create fake devices in some cases. Really the entities, or objects, or whatever you call 'em should just extend some kref'd base class. Sort of like how existing kms objects extend drm_mode_object. So any 'struct device' moves down into the derived class, not in the base class. Configuration data is passed in separate, not grabbed out of dev->platform_data, etc. Probably there is room for some helpers to pull stuff out of DT/ACPI/whatever. BR, -R > + const struct mipi_dbi_device_id *id_entry; > + struct mipi_dbi_bus *bus; > + > + unsigned int flags; > + unsigned int bus_width; > + unsigned int data_width; > +}; > + > +#define to_mipi_dbi_device(d) container_of(d, struct mipi_dbi_device, dev) > + > +int mipi_dbi_device_register(struct mipi_dbi_device *dev, > + struct mipi_dbi_bus *bus); > +void mipi_dbi_device_unregister(struct mipi_dbi_device *dev); > + > +struct mipi_dbi_driver { > + int(*probe)(struct mipi_dbi_device *); > + int(*remove)(struct mipi_dbi_device *); > + struct device_driver driver; > + const struct mipi_dbi_device_id *id_table; > +}; > + > +#define to_mipi_dbi_driver(d) container_of(d, struct mipi_dbi_driver, driver) > + > +int mipi_dbi_driver_register(struct mipi_dbi_driver *drv); > +void mipi_dbi_driver_unregister(struct mipi_dbi_driver *drv); > + > +static inline void *mipi_dbi_get_drvdata(const struct mipi_dbi_device *dev) > +{ > + return dev_get_drvdata(&dev->dev); > +} > + > +static inline void mipi_dbi_set_drvdata(struct mipi_dbi_device *dev, > + void *data) > +{ > + dev_set_drvdata(&dev->dev, data); > +} > + > +/* module_mipi_dbi_driver() - Helper macro for drivers that don't do > + * anything special in module init/exit. This eliminates a lot of > + * boilerplate. Each module may only use this macro once, and > + * calling it replaces module_init() and module_exit() > + */ > +#define module_mipi_dbi_driver(__mipi_dbi_driver) \ > + module_driver(__mipi_dbi_driver, mipi_dbi_driver_register, \ > + mipi_dbi_driver_unregister) > + > +int mipi_dbi_set_data_width(struct mipi_dbi_device *dev, unsigned int width); > + > +int mipi_dbi_write_command(struct mipi_dbi_device *dev, u16 cmd); > +int mipi_dbi_read_data(struct mipi_dbi_device *dev, u8 *data, size_t len); > +int mipi_dbi_write_data(struct mipi_dbi_device *dev, const u8 *data, > + size_t len); > + > +#endif /* __MIPI_DBI_BUS__ */ > -- > 1.8.1.5 >
Hi Rob, On Tuesday 13 August 2013 20:52:15 Rob Clark wrote: > On Fri, Aug 9, 2013 at 1:14 PM, Laurent Pinchart wrote: > > MIPI DBI is a configurable-width parallel display bus that transmits > > commands and data. > > > > Add a new DBI Linux bus type that implements the usual bus > > infrastructure (including devices and drivers (un)registration and > > matching, and bus configuration and access functions). > > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > --- > > > > drivers/video/display/Kconfig | 8 ++ > > drivers/video/display/Makefile | 1 + > > drivers/video/display/mipi-dbi-bus.c | 234 ++++++++++++++++++++++++++++++ > > include/video/display.h | 4 + > > include/video/mipi-dbi-bus.h | 125 +++++++++++++++++++ > > 5 files changed, 372 insertions(+) > > create mode 100644 drivers/video/display/mipi-dbi-bus.c > > create mode 100644 include/video/mipi-dbi-bus.h [snip] > > diff --git a/include/video/mipi-dbi-bus.h b/include/video/mipi-dbi-bus.h > > new file mode 100644 > > index 0000000..876b69d > > --- /dev/null > > +++ b/include/video/mipi-dbi-bus.h [snip] > > +struct mipi_dbi_device { > > + const char *name; > > + int id; > > + struct device dev; > > just fwiw, we need the "framework" to be agnostic of the association between > devices and entities in the framework. Some things that are multiple > entities may actually map to a single device in hw I haven't written down that requirement, but I've kept it in mind while designing CDF. We have similar use cases in V4L2. > (and possibly visa versa?). I don't think that would be needed, but if you can think of a use case I'm all ears. > Otherwise we end up having to create fake devices in some cases. Really the > entities, or objects, or whatever you call 'em should just extend some > kref'd base class. Sort of like how existing kms objects extend > drm_mode_object. struct display_entity { struct list_head list; struct device *dev; struct module *owner; struct kref ref; char name[32]; struct media_entity entity; const struct display_entity_ops *ops; void(*release)(struct display_entity *ent); }; (from patch 02/19) > So any 'struct device' moves down into the derived class, not in the base > class. Configuration data is passed in separate, not grabbed out of dev-> > platform_data, etc. Probably there is room for some helpers to pull stuff > out of DT/ACPI/whatever. struct mipi_dbi_device is not a display entity, it's a physical device plugged on a DBI bus. It's thus similar in purpose to struct pci_device or struct platform_device. The DBI device driver will then create one or more display entities depending on its needs. > > + const struct mipi_dbi_device_id *id_entry; > > + struct mipi_dbi_bus *bus; > > + > > + unsigned int flags; > > + unsigned int bus_width; > > + unsigned int data_width; > > +};
Hi, On 09/08/13 20:14, Laurent Pinchart wrote: > MIPI DBI is a configurable-width parallel display bus that transmits > commands and data. > > Add a new DBI Linux bus type that implements the usual bus > infrastructure (including devices and drivers (un)registration and > matching, and bus configuration and access functions). This has been discussed before, but I don't remember that the issue would have been cleared, so I'm bringing it up again. What benefit does a real Linux DBI (or DSI) bus give us, compared to representing the DBI the same way as DPI? DBI & DSI are in practice point-to-point buses, and they do not support probing. Is it just that because DBI and DSI can be used to control a device, they have to be Linux buses? How do you see handling the devices where DBI or DSI is used for video only, and the control is handled via, say, i2c? The module has to register two drivers, and try to keep those in sync? I feel that could get rather hacky. A real Linux bus would be necessary if we had devices that used DBI or DSI only for control, and some other video bus for video data. But that sounds so silly that I think we can just forget about the case. Thus DBI and DSI are used either for video only, or video and control. Tomi
Hi Laurent, On 9 August 2013 22:44, Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com> wrote: > MIPI DBI is a configurable-width parallel display bus that transmits > commands and data. > > Add a new DBI Linux bus type that implements the usual bus > infrastructure (including devices and drivers (un)registration and > matching, and bus configuration and access functions). > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > drivers/video/display/Kconfig | 8 ++ > drivers/video/display/Makefile | 1 + > drivers/video/display/mipi-dbi-bus.c | 234 +++++++++++++++++++++++++++++++++++ > include/video/display.h | 4 + > include/video/mipi-dbi-bus.h | 125 +++++++++++++++++++ > 5 files changed, 372 insertions(+) > create mode 100644 drivers/video/display/mipi-dbi-bus.c > create mode 100644 include/video/mipi-dbi-bus.h > > diff --git a/drivers/video/display/Kconfig b/drivers/video/display/Kconfig > index 1d533e7..f7532c1 100644 > --- a/drivers/video/display/Kconfig > +++ b/drivers/video/display/Kconfig > @@ -2,3 +2,11 @@ menuconfig DISPLAY_CORE > tristate "Display Core" > ---help--- > Support common display framework for graphics devices. > + > +if DISPLAY_CORE > + > +config DISPLAY_MIPI_DBI > + tristate > + default n > + > +endif # DISPLAY_CORE > diff --git a/drivers/video/display/Makefile b/drivers/video/display/Makefile > index b907aad..59022d2 100644 > --- a/drivers/video/display/Makefile > +++ b/drivers/video/display/Makefile > @@ -1,3 +1,4 @@ > display-y := display-core.o \ > display-notifier.o > obj-$(CONFIG_DISPLAY_CORE) += display.o > +obj-$(CONFIG_DISPLAY_MIPI_DBI) += mipi-dbi-bus.o > diff --git a/drivers/video/display/mipi-dbi-bus.c b/drivers/video/display/mipi-dbi-bus.c > new file mode 100644 > index 0000000..791fb4d > --- /dev/null > +++ b/drivers/video/display/mipi-dbi-bus.c > @@ -0,0 +1,234 @@ > +/* > + * MIPI DBI Bus > + * > + * 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/device.h> > +#include <linux/export.h> > +#include <linux/kernel.h> > +#include <linux/list.h> > +#include <linux/module.h> > +#include <linux/mutex.h> > +#include <linux/pm.h> > +#include <linux/pm_runtime.h> > + > +#include <video/mipi-dbi-bus.h> > + > +/* ----------------------------------------------------------------------------- > + * Bus operations > + */ > + > +int mipi_dbi_set_data_width(struct mipi_dbi_device *dev, unsigned int width) > +{ > + if (width != 8 && width != 16) > + return -EINVAL; > + > + dev->data_width = width; > + return 0; > +} > +EXPORT_SYMBOL_GPL(mipi_dbi_set_data_width); > + > +int mipi_dbi_write_command(struct mipi_dbi_device *dev, u16 cmd) > +{ > + return dev->bus->ops->write_command(dev->bus, dev, cmd); > +} > +EXPORT_SYMBOL_GPL(mipi_dbi_write_command); > + > +int mipi_dbi_write_data(struct mipi_dbi_device *dev, const u8 *data, > + size_t len) > +{ > + return dev->bus->ops->write_data(dev->bus, dev, data, len); > +} > +EXPORT_SYMBOL_GPL(mipi_dbi_write_data); > + > +int mipi_dbi_read_data(struct mipi_dbi_device *dev, u8 *data, size_t len) > +{ > + return dev->bus->ops->read_data(dev->bus, dev, data, len); > +} > +EXPORT_SYMBOL_GPL(mipi_dbi_read_data); > + > +/* ----------------------------------------------------------------------------- > + * Bus type > + */ > + > +static const struct mipi_dbi_device_id * > +mipi_dbi_match_id(const struct mipi_dbi_device_id *id, > + struct mipi_dbi_device *dev) > +{ > + while (id->name[0]) { > + if (strcmp(dev->name, id->name) == 0) { > + dev->id_entry = id; > + return id; > + } > + id++; > + } > + return NULL; > +} > + > +static int mipi_dbi_match(struct device *_dev, struct device_driver *_drv) > +{ > + struct mipi_dbi_device *dev = to_mipi_dbi_device(_dev); > + struct mipi_dbi_driver *drv = to_mipi_dbi_driver(_drv); > + > + if (drv->id_table) > + return mipi_dbi_match_id(drv->id_table, dev) != NULL; > + > + return (strcmp(dev->name, _drv->name) == 0); > +} > + > +static ssize_t modalias_show(struct device *_dev, struct device_attribute *a, > + char *buf) > +{ > + struct mipi_dbi_device *dev = to_mipi_dbi_device(_dev); > + int len = snprintf(buf, PAGE_SIZE, MIPI_DBI_MODULE_PREFIX "%s\n", > + dev->name); > + > + return (len >= PAGE_SIZE) ? (PAGE_SIZE - 1) : len; > +} > + > +static struct device_attribute mipi_dbi_dev_attrs[] = { > + __ATTR_RO(modalias), > + __ATTR_NULL, > +}; > + > +static int mipi_dbi_uevent(struct device *_dev, struct kobj_uevent_env *env) > +{ > + struct mipi_dbi_device *dev = to_mipi_dbi_device(_dev); > + > + add_uevent_var(env, "MODALIAS=%s%s", MIPI_DBI_MODULE_PREFIX, > + dev->name); > + return 0; > +} > + > +static const struct dev_pm_ops mipi_dbi_dev_pm_ops = { > + .runtime_suspend = pm_generic_runtime_suspend, > + .runtime_resume = pm_generic_runtime_resume, > + .suspend = pm_generic_suspend, > + .resume = pm_generic_resume, > + .freeze = pm_generic_freeze, > + .thaw = pm_generic_thaw, > + .poweroff = pm_generic_poweroff, > + .restore = pm_generic_restore, > +}; > + > +static struct bus_type mipi_dbi_bus_type = { > + .name = "mipi-dbi", > + .dev_attrs = mipi_dbi_dev_attrs, > + .match = mipi_dbi_match, > + .uevent = mipi_dbi_uevent, > + .pm = &mipi_dbi_dev_pm_ops, > +}; > + > +/* ----------------------------------------------------------------------------- > + * Device and driver (un)registration > + */ > + > +/** > + * mipi_dbi_device_register - register a DBI device > + * @dev: DBI device we're registering > + */ > +int mipi_dbi_device_register(struct mipi_dbi_device *dev, > + struct mipi_dbi_bus *bus) > +{ > + device_initialize(&dev->dev); > + > + dev->bus = bus; > + dev->dev.bus = &mipi_dbi_bus_type; > + dev->dev.parent = bus->dev; > + > + if (dev->id != -1) > + dev_set_name(&dev->dev, "%s.%d", dev->name, dev->id); > + else > + dev_set_name(&dev->dev, "%s", dev->name); > + > + return device_add(&dev->dev); > +} The function looks very much specific to NON-DT case where you will be calling mipi_dbi_device_register() in the machine file. I was actually trying to migrate to CDFv3 and adding MIPI DSI support for exynos5250, but in my case where exynos5250 is fully DT based, in which case we need something like ./drivers/of/platform.c for MIPI DBI and MIPI DSI to add the MIPI DBI/DSI device via DT way, ./drivers/of/mipi_dbi.c and ./drivers/of/mipi_dsi.c may look like below, int of_mipi_dbi_device_register(struct device_node *np, const char *bus_id, struct device *parent) { struct mipi_dbi_device *dev; dev = of_device_alloc(np, bus_id, parent); if (!dev) return NULL; device_initialize(dev); dev->bus = &mipi_dbi_bus_type; dev->parent = parent; return of_device_add(dev); } Correct me if I am wrong. > +EXPORT_SYMBOL_GPL(mipi_dbi_device_register); > + > +/** > + * mipi_dbi_device_unregister - unregister a DBI device > + * @dev: DBI device we're unregistering > + */ > +void mipi_dbi_device_unregister(struct mipi_dbi_device *dev) > +{ > + device_del(&dev->dev); > + put_device(&dev->dev); > +} > +EXPORT_SYMBOL_GPL(mipi_dbi_device_unregister); > + > +static int mipi_dbi_drv_probe(struct device *_dev) > +{ > + struct mipi_dbi_driver *drv = to_mipi_dbi_driver(_dev->driver); > + struct mipi_dbi_device *dev = to_mipi_dbi_device(_dev); Here we are assuming that mipi_dbi_device can be obtained by using _dev pointer, which may NOT be true in DT case, i think. let me know if i am missing something. if you can give me a example for DT case, that would be helpful. > + > + return drv->probe(dev); > +} > + > +static int mipi_dbi_drv_remove(struct device *_dev) > +{ > + struct mipi_dbi_driver *drv = to_mipi_dbi_driver(_dev->driver); > + struct mipi_dbi_device *dev = to_mipi_dbi_device(_dev); > + int ret; > + > + ret = drv->remove(dev); > + if (ret < 0) > + return ret; > + > + mipi_dbi_set_drvdata(dev, NULL); > + > + return 0; > +} > + > +/** > + * mipi_dbi_driver_register - register a driver for DBI devices > + * @drv: DBI driver structure > + */ > +int mipi_dbi_driver_register(struct mipi_dbi_driver *drv) > +{ > + drv->driver.bus = &mipi_dbi_bus_type; > + if (drv->probe) > + drv->driver.probe = mipi_dbi_drv_probe; > + if (drv->remove) > + drv->driver.remove = mipi_dbi_drv_remove; > + > + return driver_register(&drv->driver); > +} > +EXPORT_SYMBOL_GPL(mipi_dbi_driver_register); > + > +/** > + * mipi_dbi_driver_unregister - unregister a driver for DBI devices > + * @drv: DBI driver structure > + */ > +void mipi_dbi_driver_unregister(struct mipi_dbi_driver *drv) > +{ > + driver_unregister(&drv->driver); > +} > +EXPORT_SYMBOL_GPL(mipi_dbi_driver_unregister); > + > +/* ----------------------------------------------------------------------------- > + * Init/exit > + */ > + > +static int __init mipi_dbi_init(void) > +{ > + return bus_register(&mipi_dbi_bus_type); > +} > + > +static void __exit mipi_dbi_exit(void) > +{ > + bus_unregister(&mipi_dbi_bus_type); > +} > + > +module_init(mipi_dbi_init); > +module_exit(mipi_dbi_exit) > + > +MODULE_AUTHOR("Laurent Pinchart <laurent.pinchart@ideasonboard.com>"); > +MODULE_DESCRIPTION("MIPI DBI Bus"); > +MODULE_LICENSE("GPL"); > diff --git a/include/video/display.h b/include/video/display.h > index ba319d6..3138401 100644 > --- a/include/video/display.h > +++ b/include/video/display.h > @@ -17,6 +17,7 @@ > #include <linux/list.h> > #include <linux/module.h> > #include <media/media-entity.h> > +#include <video/mipi-dbi-bus.h> > > #define DISPLAY_PIXEL_CODING(option, type, from, to, variant) \ > (((option) << 17) | ((type) << 13) | ((variant) << 10) | \ > @@ -189,6 +190,9 @@ enum display_entity_interface_type { > > struct display_entity_interface_params { > enum display_entity_interface_type type; > + union { > + struct mipi_dbi_interface_params dbi; > + } p; > }; > > struct display_entity_control_ops { > diff --git a/include/video/mipi-dbi-bus.h b/include/video/mipi-dbi-bus.h > new file mode 100644 > index 0000000..876b69d > --- /dev/null > +++ b/include/video/mipi-dbi-bus.h > @@ -0,0 +1,125 @@ > +/* > + * MIPI DBI Bus > + * > + * 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 __MIPI_DBI_BUS_H__ > +#define __MIPI_DBI_BUS_H__ > + > +#include <linux/device.h> > + > +struct mipi_dbi_bus; > +struct mipi_dbi_device; > + > +struct mipi_dbi_bus_ops { > + int (*write_command)(struct mipi_dbi_bus *bus, > + struct mipi_dbi_device *dev, u16 cmd); > + int (*write_data)(struct mipi_dbi_bus *bus, struct mipi_dbi_device *dev, > + const u8 *data, size_t len); > + int (*read_data)(struct mipi_dbi_bus *bus, struct mipi_dbi_device *dev, > + u8 *data, size_t len); > +}; > + > +struct mipi_dbi_bus { > + struct device *dev; > + const struct mipi_dbi_bus_ops *ops; > +}; > + > +#define MIPI_DBI_MODULE_PREFIX "mipi-dbi:" > +#define MIPI_DBI_NAME_SIZE 32 > + > +struct mipi_dbi_device_id { > + char name[MIPI_DBI_NAME_SIZE]; > + __kernel_ulong_t driver_data /* Data private to the driver */ > + __aligned(sizeof(__kernel_ulong_t)); > +}; > + > +enum mipi_dbi_interface_type { > + MIPI_DBI_INTERFACE_TYPE_A, > + MIPI_DBI_INTERFACE_TYPE_B, > +}; > + > +#define MIPI_DBI_INTERFACE_TE (1 << 0) > + > +struct mipi_dbi_interface_params { > + enum mipi_dbi_interface_type type; > + unsigned int flags; > + > + unsigned int cs_setup; > + unsigned int rd_setup; > + unsigned int rd_latch; > + unsigned int rd_cycle; > + unsigned int rd_hold; > + unsigned int wr_setup; > + unsigned int wr_cycle; > + unsigned int wr_hold; > +}; > + > +#define MIPI_DBI_FLAG_ALIGN_LEFT (1 << 0) > + > +struct mipi_dbi_device { > + const char *name; > + int id; > + struct device dev; > + > + const struct mipi_dbi_device_id *id_entry; > + struct mipi_dbi_bus *bus; > + > + unsigned int flags; > + unsigned int bus_width; > + unsigned int data_width; > +}; > + > +#define to_mipi_dbi_device(d) container_of(d, struct mipi_dbi_device, dev) > + > +int mipi_dbi_device_register(struct mipi_dbi_device *dev, > + struct mipi_dbi_bus *bus); > +void mipi_dbi_device_unregister(struct mipi_dbi_device *dev); > + > +struct mipi_dbi_driver { > + int(*probe)(struct mipi_dbi_device *); > + int(*remove)(struct mipi_dbi_device *); > + struct device_driver driver; > + const struct mipi_dbi_device_id *id_table; > +}; > + > +#define to_mipi_dbi_driver(d) container_of(d, struct mipi_dbi_driver, driver) > + > +int mipi_dbi_driver_register(struct mipi_dbi_driver *drv); > +void mipi_dbi_driver_unregister(struct mipi_dbi_driver *drv); > + > +static inline void *mipi_dbi_get_drvdata(const struct mipi_dbi_device *dev) > +{ > + return dev_get_drvdata(&dev->dev); > +} > + > +static inline void mipi_dbi_set_drvdata(struct mipi_dbi_device *dev, > + void *data) > +{ > + dev_set_drvdata(&dev->dev, data); > +} > + > +/* module_mipi_dbi_driver() - Helper macro for drivers that don't do > + * anything special in module init/exit. This eliminates a lot of > + * boilerplate. Each module may only use this macro once, and > + * calling it replaces module_init() and module_exit() > + */ > +#define module_mipi_dbi_driver(__mipi_dbi_driver) \ > + module_driver(__mipi_dbi_driver, mipi_dbi_driver_register, \ > + mipi_dbi_driver_unregister) > + > +int mipi_dbi_set_data_width(struct mipi_dbi_device *dev, unsigned int width); > + > +int mipi_dbi_write_command(struct mipi_dbi_device *dev, u16 cmd); > +int mipi_dbi_read_data(struct mipi_dbi_device *dev, u8 *data, size_t len); > +int mipi_dbi_write_data(struct mipi_dbi_device *dev, const u8 *data, > + size_t len); > + > +#endif /* __MIPI_DBI_BUS__ */ > -- > 1.8.1.5 >
Hi Laurent, On 9 August 2013 22:44, Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com> wrote: > MIPI DBI is a configurable-width parallel display bus that transmits > commands and data. > > Add a new DBI Linux bus type that implements the usual bus > infrastructure (including devices and drivers (un)registration and > matching, and bus configuration and access functions). > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > drivers/video/display/Kconfig | 8 ++ > drivers/video/display/Makefile | 1 + > drivers/video/display/mipi-dbi-bus.c | 234 +++++++++++++++++++++++++++++++++++ > include/video/display.h | 4 + > include/video/mipi-dbi-bus.h | 125 +++++++++++++++++++ > 5 files changed, 372 insertions(+) > create mode 100644 drivers/video/display/mipi-dbi-bus.c > create mode 100644 include/video/mipi-dbi-bus.h > > diff --git a/drivers/video/display/Kconfig b/drivers/video/display/Kconfig > index 1d533e7..f7532c1 100644 > --- a/drivers/video/display/Kconfig > +++ b/drivers/video/display/Kconfig > @@ -2,3 +2,11 @@ menuconfig DISPLAY_CORE > tristate "Display Core" > ---help--- > Support common display framework for graphics devices. > + > +if DISPLAY_CORE > + > +config DISPLAY_MIPI_DBI > + tristate > + default n > + > +endif # DISPLAY_CORE > diff --git a/drivers/video/display/Makefile b/drivers/video/display/Makefile > index b907aad..59022d2 100644 > --- a/drivers/video/display/Makefile > +++ b/drivers/video/display/Makefile > @@ -1,3 +1,4 @@ > display-y := display-core.o \ > display-notifier.o > obj-$(CONFIG_DISPLAY_CORE) += display.o > +obj-$(CONFIG_DISPLAY_MIPI_DBI) += mipi-dbi-bus.o > diff --git a/drivers/video/display/mipi-dbi-bus.c b/drivers/video/display/mipi-dbi-bus.c > new file mode 100644 > index 0000000..791fb4d > --- /dev/null > +++ b/drivers/video/display/mipi-dbi-bus.c > @@ -0,0 +1,234 @@ > +/* > + * MIPI DBI Bus > + * > + * 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/device.h> > +#include <linux/export.h> > +#include <linux/kernel.h> > +#include <linux/list.h> > +#include <linux/module.h> > +#include <linux/mutex.h> > +#include <linux/pm.h> > +#include <linux/pm_runtime.h> > + > +#include <video/mipi-dbi-bus.h> > + > +/* ----------------------------------------------------------------------------- > + * Bus operations > + */ > + > +int mipi_dbi_set_data_width(struct mipi_dbi_device *dev, unsigned int width) > +{ > + if (width != 8 && width != 16) > + return -EINVAL; > + > + dev->data_width = width; > + return 0; > +} > +EXPORT_SYMBOL_GPL(mipi_dbi_set_data_width); > + > +int mipi_dbi_write_command(struct mipi_dbi_device *dev, u16 cmd) > +{ > + return dev->bus->ops->write_command(dev->bus, dev, cmd); Can you help me in pointing out where these function pointer ( ops->write_command ) are assigned in case MIPI DBI. In case of exynos ( drivers/video/exynos/exynos_mipi_dsi.c ), we assign them as below and register DSI as a platform device static struct mipi_dsim_master_ops master_ops = { .cmd_read = exynos_mipi_dsi_rd_data, .cmd_write = exynos_mipi_dsi_wr_data, .get_dsim_frame_done = exynos_mipi_dsi_get_frame_done_status, .clear_dsim_frame_done = exynos_mipi_dsi_clear_frame_done, .set_early_blank_mode = exynos_mipi_dsi_early_blank_mode, .set_blank_mode = exynos_mipi_dsi_blank_mode, }; Since now you are saying to have it as linux BUS, how should we register these ops and how we can configure the MIPI DSI hw itself if we register it as bus. I could not find any help in mipi-dbi-bus.c, how we actually configure the MIPI DBI h/w itself. ideally mipi-dbi-bus.c should have done 2 things ====================================== 1. provide a framework to register the DBI kind of panel = which is supported 2. provide a framework to register the actuall MIPI DBI H/W itself = which i think is missing ( correct me, if i am missing anything ) > +} > +EXPORT_SYMBOL_GPL(mipi_dbi_write_command); > + > +int mipi_dbi_write_data(struct mipi_dbi_device *dev, const u8 *data, > + size_t len) > +{ > + return dev->bus->ops->write_data(dev->bus, dev, data, len); > +} > +EXPORT_SYMBOL_GPL(mipi_dbi_write_data); > + > +int mipi_dbi_read_data(struct mipi_dbi_device *dev, u8 *data, size_t len) > +{ > + return dev->bus->ops->read_data(dev->bus, dev, data, len); > +} > +EXPORT_SYMBOL_GPL(mipi_dbi_read_data); > + > +/* ----------------------------------------------------------------------------- > + * Bus type > + */ > + > +static const struct mipi_dbi_device_id * > +mipi_dbi_match_id(const struct mipi_dbi_device_id *id, > + struct mipi_dbi_device *dev) > +{ > + while (id->name[0]) { > + if (strcmp(dev->name, id->name) == 0) { > + dev->id_entry = id; > + return id; > + } > + id++; > + } > + return NULL; > +} > + > +static int mipi_dbi_match(struct device *_dev, struct device_driver *_drv) > +{ > + struct mipi_dbi_device *dev = to_mipi_dbi_device(_dev); > + struct mipi_dbi_driver *drv = to_mipi_dbi_driver(_drv); > + > + if (drv->id_table) > + return mipi_dbi_match_id(drv->id_table, dev) != NULL; > + > + return (strcmp(dev->name, _drv->name) == 0); > +} > + > +static ssize_t modalias_show(struct device *_dev, struct device_attribute *a, > + char *buf) > +{ > + struct mipi_dbi_device *dev = to_mipi_dbi_device(_dev); > + int len = snprintf(buf, PAGE_SIZE, MIPI_DBI_MODULE_PREFIX "%s\n", > + dev->name); > + > + return (len >= PAGE_SIZE) ? (PAGE_SIZE - 1) : len; > +} > + > +static struct device_attribute mipi_dbi_dev_attrs[] = { > + __ATTR_RO(modalias), > + __ATTR_NULL, > +}; > + > +static int mipi_dbi_uevent(struct device *_dev, struct kobj_uevent_env *env) > +{ > + struct mipi_dbi_device *dev = to_mipi_dbi_device(_dev); > + > + add_uevent_var(env, "MODALIAS=%s%s", MIPI_DBI_MODULE_PREFIX, > + dev->name); > + return 0; > +} > + > +static const struct dev_pm_ops mipi_dbi_dev_pm_ops = { > + .runtime_suspend = pm_generic_runtime_suspend, > + .runtime_resume = pm_generic_runtime_resume, > + .suspend = pm_generic_suspend, > + .resume = pm_generic_resume, > + .freeze = pm_generic_freeze, > + .thaw = pm_generic_thaw, > + .poweroff = pm_generic_poweroff, > + .restore = pm_generic_restore, > +}; > + > +static struct bus_type mipi_dbi_bus_type = { > + .name = "mipi-dbi", > + .dev_attrs = mipi_dbi_dev_attrs, > + .match = mipi_dbi_match, > + .uevent = mipi_dbi_uevent, > + .pm = &mipi_dbi_dev_pm_ops, > +}; > + > +/* ----------------------------------------------------------------------------- > + * Device and driver (un)registration > + */ > + > +/** > + * mipi_dbi_device_register - register a DBI device > + * @dev: DBI device we're registering > + */ > +int mipi_dbi_device_register(struct mipi_dbi_device *dev, > + struct mipi_dbi_bus *bus) > +{ > + device_initialize(&dev->dev); > + > + dev->bus = bus; > + dev->dev.bus = &mipi_dbi_bus_type; > + dev->dev.parent = bus->dev; > + > + if (dev->id != -1) > + dev_set_name(&dev->dev, "%s.%d", dev->name, dev->id); > + else > + dev_set_name(&dev->dev, "%s", dev->name); > + > + return device_add(&dev->dev); > +} > +EXPORT_SYMBOL_GPL(mipi_dbi_device_register); > + > +/** > + * mipi_dbi_device_unregister - unregister a DBI device > + * @dev: DBI device we're unregistering > + */ > +void mipi_dbi_device_unregister(struct mipi_dbi_device *dev) > +{ > + device_del(&dev->dev); > + put_device(&dev->dev); > +} > +EXPORT_SYMBOL_GPL(mipi_dbi_device_unregister); > + > +static int mipi_dbi_drv_probe(struct device *_dev) > +{ > + struct mipi_dbi_driver *drv = to_mipi_dbi_driver(_dev->driver); > + struct mipi_dbi_device *dev = to_mipi_dbi_device(_dev); > + > + return drv->probe(dev); > +} > + > +static int mipi_dbi_drv_remove(struct device *_dev) > +{ > + struct mipi_dbi_driver *drv = to_mipi_dbi_driver(_dev->driver); > + struct mipi_dbi_device *dev = to_mipi_dbi_device(_dev); > + int ret; > + > + ret = drv->remove(dev); > + if (ret < 0) > + return ret; > + > + mipi_dbi_set_drvdata(dev, NULL); > + > + return 0; > +} > + > +/** > + * mipi_dbi_driver_register - register a driver for DBI devices > + * @drv: DBI driver structure > + */ > +int mipi_dbi_driver_register(struct mipi_dbi_driver *drv) > +{ > + drv->driver.bus = &mipi_dbi_bus_type; > + if (drv->probe) > + drv->driver.probe = mipi_dbi_drv_probe; > + if (drv->remove) > + drv->driver.remove = mipi_dbi_drv_remove; > + > + return driver_register(&drv->driver); > +} > +EXPORT_SYMBOL_GPL(mipi_dbi_driver_register); > + > +/** > + * mipi_dbi_driver_unregister - unregister a driver for DBI devices > + * @drv: DBI driver structure > + */ > +void mipi_dbi_driver_unregister(struct mipi_dbi_driver *drv) > +{ > + driver_unregister(&drv->driver); > +} > +EXPORT_SYMBOL_GPL(mipi_dbi_driver_unregister); > + > +/* ----------------------------------------------------------------------------- > + * Init/exit > + */ > + > +static int __init mipi_dbi_init(void) > +{ > + return bus_register(&mipi_dbi_bus_type); > +} > + > +static void __exit mipi_dbi_exit(void) > +{ > + bus_unregister(&mipi_dbi_bus_type); > +} > + > +module_init(mipi_dbi_init); > +module_exit(mipi_dbi_exit) > + > +MODULE_AUTHOR("Laurent Pinchart <laurent.pinchart@ideasonboard.com>"); > +MODULE_DESCRIPTION("MIPI DBI Bus"); > +MODULE_LICENSE("GPL"); > diff --git a/include/video/display.h b/include/video/display.h > index ba319d6..3138401 100644 > --- a/include/video/display.h > +++ b/include/video/display.h > @@ -17,6 +17,7 @@ > #include <linux/list.h> > #include <linux/module.h> > #include <media/media-entity.h> > +#include <video/mipi-dbi-bus.h> > > #define DISPLAY_PIXEL_CODING(option, type, from, to, variant) \ > (((option) << 17) | ((type) << 13) | ((variant) << 10) | \ > @@ -189,6 +190,9 @@ enum display_entity_interface_type { > > struct display_entity_interface_params { > enum display_entity_interface_type type; > + union { > + struct mipi_dbi_interface_params dbi; > + } p; > }; > > struct display_entity_control_ops { > diff --git a/include/video/mipi-dbi-bus.h b/include/video/mipi-dbi-bus.h > new file mode 100644 > index 0000000..876b69d > --- /dev/null > +++ b/include/video/mipi-dbi-bus.h > @@ -0,0 +1,125 @@ > +/* > + * MIPI DBI Bus > + * > + * 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 __MIPI_DBI_BUS_H__ > +#define __MIPI_DBI_BUS_H__ > + > +#include <linux/device.h> > + > +struct mipi_dbi_bus; > +struct mipi_dbi_device; > + > +struct mipi_dbi_bus_ops { > + int (*write_command)(struct mipi_dbi_bus *bus, > + struct mipi_dbi_device *dev, u16 cmd); > + int (*write_data)(struct mipi_dbi_bus *bus, struct mipi_dbi_device *dev, > + const u8 *data, size_t len); > + int (*read_data)(struct mipi_dbi_bus *bus, struct mipi_dbi_device *dev, > + u8 *data, size_t len); > +}; > + > +struct mipi_dbi_bus { > + struct device *dev; > + const struct mipi_dbi_bus_ops *ops; > +}; > + > +#define MIPI_DBI_MODULE_PREFIX "mipi-dbi:" > +#define MIPI_DBI_NAME_SIZE 32 > + > +struct mipi_dbi_device_id { > + char name[MIPI_DBI_NAME_SIZE]; > + __kernel_ulong_t driver_data /* Data private to the driver */ > + __aligned(sizeof(__kernel_ulong_t)); > +}; > + > +enum mipi_dbi_interface_type { > + MIPI_DBI_INTERFACE_TYPE_A, > + MIPI_DBI_INTERFACE_TYPE_B, > +}; > + > +#define MIPI_DBI_INTERFACE_TE (1 << 0) > + > +struct mipi_dbi_interface_params { > + enum mipi_dbi_interface_type type; > + unsigned int flags; > + > + unsigned int cs_setup; > + unsigned int rd_setup; > + unsigned int rd_latch; > + unsigned int rd_cycle; > + unsigned int rd_hold; > + unsigned int wr_setup; > + unsigned int wr_cycle; > + unsigned int wr_hold; > +}; > + > +#define MIPI_DBI_FLAG_ALIGN_LEFT (1 << 0) > + > +struct mipi_dbi_device { > + const char *name; > + int id; > + struct device dev; > + > + const struct mipi_dbi_device_id *id_entry; > + struct mipi_dbi_bus *bus; > + > + unsigned int flags; > + unsigned int bus_width; > + unsigned int data_width; > +}; > + > +#define to_mipi_dbi_device(d) container_of(d, struct mipi_dbi_device, dev) > + > +int mipi_dbi_device_register(struct mipi_dbi_device *dev, > + struct mipi_dbi_bus *bus); > +void mipi_dbi_device_unregister(struct mipi_dbi_device *dev); > + > +struct mipi_dbi_driver { > + int(*probe)(struct mipi_dbi_device *); > + int(*remove)(struct mipi_dbi_device *); > + struct device_driver driver; > + const struct mipi_dbi_device_id *id_table; > +}; > + > +#define to_mipi_dbi_driver(d) container_of(d, struct mipi_dbi_driver, driver) > + > +int mipi_dbi_driver_register(struct mipi_dbi_driver *drv); > +void mipi_dbi_driver_unregister(struct mipi_dbi_driver *drv); > + > +static inline void *mipi_dbi_get_drvdata(const struct mipi_dbi_device *dev) > +{ > + return dev_get_drvdata(&dev->dev); > +} > + > +static inline void mipi_dbi_set_drvdata(struct mipi_dbi_device *dev, > + void *data) > +{ > + dev_set_drvdata(&dev->dev, data); > +} > + > +/* module_mipi_dbi_driver() - Helper macro for drivers that don't do > + * anything special in module init/exit. This eliminates a lot of > + * boilerplate. Each module may only use this macro once, and > + * calling it replaces module_init() and module_exit() > + */ > +#define module_mipi_dbi_driver(__mipi_dbi_driver) \ > + module_driver(__mipi_dbi_driver, mipi_dbi_driver_register, \ > + mipi_dbi_driver_unregister) > + > +int mipi_dbi_set_data_width(struct mipi_dbi_device *dev, unsigned int width); > + > +int mipi_dbi_write_command(struct mipi_dbi_device *dev, u16 cmd); > +int mipi_dbi_read_data(struct mipi_dbi_device *dev, u8 *data, size_t len); > +int mipi_dbi_write_data(struct mipi_dbi_device *dev, const u8 *data, > + size_t len); > + > +#endif /* __MIPI_DBI_BUS__ */ > -- > 1.8.1.5 >
Hi Tomi, On Monday 26 August 2013 14:10:50 Tomi Valkeinen wrote: > On 09/08/13 20:14, Laurent Pinchart wrote: > > MIPI DBI is a configurable-width parallel display bus that transmits > > commands and data. > > > > Add a new DBI Linux bus type that implements the usual bus > > infrastructure (including devices and drivers (un)registration and > > matching, and bus configuration and access functions). > > This has been discussed before, but I don't remember that the issue would > have been cleared, so I'm bringing it up again. > > What benefit does a real Linux DBI (or DSI) bus give us, compared to > representing the DBI the same way as DPI? DBI & DSI are in practice point- > to-point buses, and they do not support probing. Is it just that because DBI > and DSI can be used to control a device, they have to be Linux buses? The idea was to reuse the Linux bus infrastructure to benefit from features such as power management. Implementing DBI/DSI control functions using a DBI/DSI bus is also pretty easy, and would allow controlling DBI/DSI entities much like we control I2C entities. I don't like the idea of using an I2C bus with I2C access functions on one side, and embedding the DBI/DSI access functions as part of CDF entity operations on the other side very much. This being said, this isn't the part of CDF that matters the most to me (it's actually not part of CDF strictly speaking). If your model works well enough it won't be too hard to convince me :-) > How do you see handling the devices where DBI or DSI is used for video only, > and the control is handled via, say, i2c? The module has to register two > drivers, and try to keep those in sync? I feel that could get rather hacky. If DBI or DSI is used for video only you don't need DBI/DSI control operations, right ? So the entity driver will just be a regular I2C device driver. > A real Linux bus would be necessary if we had devices that used DBI or DSI > only for control, and some other video bus for video data. But that sounds > so silly that I think we can just forget about the case. I certainly hope so :-) > Thus DBI and DSI are used either for video only, or video and control.
Hi Vikas, On Wednesday 04 September 2013 16:20:59 Vikas Sajjan wrote: > On 9 August 2013 22:44, Laurent Pinchart wrote: > > MIPI DBI is a configurable-width parallel display bus that transmits > > commands and data. > > > > Add a new DBI Linux bus type that implements the usual bus > > infrastructure (including devices and drivers (un)registration and > > matching, and bus configuration and access functions). > > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > --- > > > > drivers/video/display/Kconfig | 8 ++ > > drivers/video/display/Makefile | 1 + > > drivers/video/display/mipi-dbi-bus.c | 234 ++++++++++++++++++++++++++++++ > > include/video/display.h | 4 + > > include/video/mipi-dbi-bus.h | 125 +++++++++++++++++++ > > 5 files changed, 372 insertions(+) > > create mode 100644 drivers/video/display/mipi-dbi-bus.c > > create mode 100644 include/video/mipi-dbi-bus.h [snip] > > diff --git a/drivers/video/display/mipi-dbi-bus.c > > b/drivers/video/display/mipi-dbi-bus.c new file mode 100644 > > index 0000000..791fb4d > > --- /dev/null > > +++ b/drivers/video/display/mipi-dbi-bus.c [snip] > > +/* ---------------------------------------------------------------------- > > + * Device and driver (un)registration > > + */ > > + > > +/** > > + * mipi_dbi_device_register - register a DBI device > > + * @dev: DBI device we're registering > > + */ > > +int mipi_dbi_device_register(struct mipi_dbi_device *dev, > > + struct mipi_dbi_bus *bus) > > +{ > > + device_initialize(&dev->dev); > > + > > + dev->bus = bus; > > + dev->dev.bus = &mipi_dbi_bus_type; > > + dev->dev.parent = bus->dev; > > + > > + if (dev->id != -1) > > + dev_set_name(&dev->dev, "%s.%d", dev->name, dev->id); > > + else > > + dev_set_name(&dev->dev, "%s", dev->name); > > + > > + return device_add(&dev->dev); > > +} > > The function looks very much specific to NON-DT case where you will be > calling mipi_dbi_device_register() in the machine file. You're absolutely right. > I was actually trying to migrate to CDFv3 and adding MIPI DSI support > for exynos5250, > but in my case where exynos5250 is fully DT based, in which case we > need something like ./drivers/of/platform.c for MIPI DBI and MIPI DSI > to add the MIPI DBI/DSI device via DT way, ./drivers/of/mipi_dbi.c and > ./drivers/of/mipi_dsi.c > > may look like below, > > int of_mipi_dbi_device_register(struct device_node *np, > const char *bus_id, > struct device *parent) > { > struct mipi_dbi_device *dev; > dev = of_device_alloc(np, bus_id, parent); > > if (!dev) > return NULL; > device_initialize(dev); > > dev->bus = &mipi_dbi_bus_type; > dev->parent = parent; > > return of_device_add(dev); > } > > Correct me if I am wrong. You're correct, but the implementation will need to be a little bit more complex than that. From an API point of view, something like of_i2c_register_devices() (drivers/of/of_i2c.c) would probably make sense. the function should iterate over child nodes, and call of_mipi_dbi_device_register() (we could maybe rename that to of_mipi_dbi_device_create() to mimic the platform device code) for each child. In your above code, you should replace of_device_alloc() with of_mipi_dbi_device_alloc(), as of_device_alloc() allocates a struct platform_device. You should also call mipi_dsi_device_put() on the device if of_device_add() returns a failure. Would you like to send a patch on top of 08/19 to implement this ? > > +EXPORT_SYMBOL_GPL(mipi_dbi_device_register); > > + > > +/** > > + * mipi_dbi_device_unregister - unregister a DBI device > > + * @dev: DBI device we're unregistering > > + */ > > +void mipi_dbi_device_unregister(struct mipi_dbi_device *dev) > > +{ > > + device_del(&dev->dev); > > + put_device(&dev->dev); > > +} > > +EXPORT_SYMBOL_GPL(mipi_dbi_device_unregister); > > + > > +static int mipi_dbi_drv_probe(struct device *_dev) > > +{ > > + struct mipi_dbi_driver *drv = to_mipi_dbi_driver(_dev->driver); > > + struct mipi_dbi_device *dev = to_mipi_dbi_device(_dev); > > Here we are assuming that mipi_dbi_device can be obtained by using > _dev pointer, which may NOT be true in DT case, i think. Why wouldn't it be true (if we create the devices as explained above) ? > let me know if i am missing something. > > if you can give me a example for DT case, that would be helpful. I'm afraid I don't have any, as the DBI drivers I wrote are used by a platform that doesn't support DT. > > + > > + return drv->probe(dev); > > +}
Hi Vikas, On Wednesday 04 September 2013 18:22:45 Vikas Sajjan wrote: > On 9 August 2013 22:44, Laurent Pinchart wrote: > > MIPI DBI is a configurable-width parallel display bus that transmits > > commands and data. > > > > Add a new DBI Linux bus type that implements the usual bus > > infrastructure (including devices and drivers (un)registration and > > matching, and bus configuration and access functions). > > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > --- > > > > drivers/video/display/Kconfig | 8 ++ > > drivers/video/display/Makefile | 1 + > > drivers/video/display/mipi-dbi-bus.c | 234 ++++++++++++++++++++++++++++++ > > include/video/display.h | 4 + > > include/video/mipi-dbi-bus.h | 125 +++++++++++++++++++ > > 5 files changed, 372 insertions(+) > > create mode 100644 drivers/video/display/mipi-dbi-bus.c > > create mode 100644 include/video/mipi-dbi-bus.h [snip] > > diff --git a/drivers/video/display/mipi-dbi-bus.c > > b/drivers/video/display/mipi-dbi-bus.c new file mode 100644 > > index 0000000..791fb4d > > --- /dev/null > > +++ b/drivers/video/display/mipi-dbi-bus.c [snip] > > +/* ---------------------------------------------------------------------- > > + * Bus operations > > + */ > > + > > +int mipi_dbi_set_data_width(struct mipi_dbi_device *dev, unsigned int > > width) > > +{ > > + if (width != 8 && width != 16) > > + return -EINVAL; > > + > > + dev->data_width = width; > > + return 0; > > +} > > +EXPORT_SYMBOL_GPL(mipi_dbi_set_data_width); > > + > > +int mipi_dbi_write_command(struct mipi_dbi_device *dev, u16 cmd) > > +{ > > + return dev->bus->ops->write_command(dev->bus, dev, cmd); > > Can you help me in pointing out where these function pointer > (ops->write_command) are assigned in case MIPI DBI. > > In case of exynos (drivers/video/exynos/exynos_mipi_dsi.c), we > assign them as below and register DSI as a platform device > > static struct mipi_dsim_master_ops master_ops = { > .cmd_read = exynos_mipi_dsi_rd_data, > .cmd_write = exynos_mipi_dsi_wr_data, > .get_dsim_frame_done = > exynos_mipi_dsi_get_frame_done_status, > .clear_dsim_frame_done = exynos_mipi_dsi_clear_frame_done, > .set_early_blank_mode = exynos_mipi_dsi_early_blank_mode, > .set_blank_mode = exynos_mipi_dsi_blank_mode, }; > > Since now you are saying to have it as linux BUS, how should we > register these ops and how we can configure the MIPI DSI hw itself if > we register it as bus. I could not find any help in mipi-dbi-bus.c, > how we actually configure the MIPI DBI h/w itself. > > ideally mipi-dbi-bus.c should have done 2 things > ====================================== > > 1. provide a framework to register the DBI kind of panel = which is > supported > > 2. provide a framework to register the actuall MIPI DBI H/W itself = > which i think is missing (correct me, if i am missing anything) Indeed, you're right... I'll go hide in a dark corner now. Thinking about it, I should instead implement the missing code, that would more helpful. The patch is missing MIPI DBI bus registration. If you have already written bus registration code feel free to send a patch, otherwise I'll fix it (I'll wait for your confirmation first). > > +}
On 06/09/13 17:09, Laurent Pinchart wrote: > Hi Tomi, > > On Monday 26 August 2013 14:10:50 Tomi Valkeinen wrote: >> On 09/08/13 20:14, Laurent Pinchart wrote: >>> MIPI DBI is a configurable-width parallel display bus that transmits >>> commands and data. >>> >>> Add a new DBI Linux bus type that implements the usual bus >>> infrastructure (including devices and drivers (un)registration and >>> matching, and bus configuration and access functions). >> >> This has been discussed before, but I don't remember that the issue would >> have been cleared, so I'm bringing it up again. >> >> What benefit does a real Linux DBI (or DSI) bus give us, compared to >> representing the DBI the same way as DPI? DBI & DSI are in practice point- >> to-point buses, and they do not support probing. Is it just that because DBI >> and DSI can be used to control a device, they have to be Linux buses? > > The idea was to reuse the Linux bus infrastructure to benefit from features > such as power management. Implementing DBI/DSI control functions using a > DBI/DSI bus is also pretty easy, and would allow controlling DBI/DSI entities > much like we control I2C entities. I don't like the idea of using an I2C bus > with I2C access functions on one side, and embedding the DBI/DSI access > functions as part of CDF entity operations on the other side very much. While I somewhat like the thought of having DBI and DSI as Linux buses, I just don't see how it would work. Well, I'm mostly thinking about DSI here, I have not used DBI for years. Also, I might remember wrong, but I think I saw Linus expressing his opinion at some point that he doesn't really like the idea of having a Linux bus for simple point-to-point buses, which don't have probing support, and so there isn't much "bus" to speak of, just a point to point link. And I think we get the same power management with just having a device as a child device of another. > This being said, this isn't the part of CDF that matters the most to me (it's > actually not part of CDF strictly speaking). If your model works well enough > it won't be too hard to convince me :-) > >> How do you see handling the devices where DBI or DSI is used for video only, >> and the control is handled via, say, i2c? The module has to register two >> drivers, and try to keep those in sync? I feel that could get rather hacky. > > If DBI or DSI is used for video only you don't need DBI/DSI control > operations, right ? So the entity driver will just be a regular I2C device > driver. Well, DSI can't be used just like that. You need to configure it. The thing is, DSI is used for both control and video. They are really the same thing, both are sent as DSI packets. Both need similar kind of configuration for the bus. If the DSI peripheral sits on a DSI bus, I imagine this configuration is done via the DSI bus support. But if there's no DSI bus, how is the configuration done? I guess a possibility is to have a fixed configuration that cannot be changed dynamically. But I have a feeling that it'll be a bit limiting for some cases. And even with fixed configuration, I think the configuration would somehow be passed as DSI bus parameters from the board files or in the DT data (the same way as, say i2c bus speed). If there's no DSI bus, as the DSI peripheral sits on the i2c bus, where are the parameters? Tomi
On 06/09/13 18:43, Tomi Valkeinen wrote: > On 06/09/13 17:09, Laurent Pinchart wrote: >> Hi Tomi, >> >> On Monday 26 August 2013 14:10:50 Tomi Valkeinen wrote: >>> On 09/08/13 20:14, Laurent Pinchart wrote: >>>> MIPI DBI is a configurable-width parallel display bus that transmits >>>> commands and data. >>>> >>>> Add a new DBI Linux bus type that implements the usual bus >>>> infrastructure (including devices and drivers (un)registration and >>>> matching, and bus configuration and access functions). >>> >>> This has been discussed before, but I don't remember that the issue would >>> have been cleared, so I'm bringing it up again. >>> >>> What benefit does a real Linux DBI (or DSI) bus give us, compared to >>> representing the DBI the same way as DPI? DBI & DSI are in practice point- >>> to-point buses, and they do not support probing. Is it just that because DBI >>> and DSI can be used to control a device, they have to be Linux buses? >> >> The idea was to reuse the Linux bus infrastructure to benefit from features >> such as power management. Implementing DBI/DSI control functions using a >> DBI/DSI bus is also pretty easy, and would allow controlling DBI/DSI entities >> much like we control I2C entities. I don't like the idea of using an I2C bus >> with I2C access functions on one side, and embedding the DBI/DSI access >> functions as part of CDF entity operations on the other side very much. > > While I somewhat like the thought of having DBI and DSI as Linux buses, > I just don't see how it would work. Well, I'm mostly thinking about DSI > here, I have not used DBI for years. > > Also, I might remember wrong, but I think I saw Linus expressing his > opinion at some point that he doesn't really like the idea of having a > Linux bus for simple point-to-point buses, which don't have probing > support, and so there isn't much "bus" to speak of, just a point to > point link. > > And I think we get the same power management with just having a device > as a child device of another. > >> This being said, this isn't the part of CDF that matters the most to me (it's >> actually not part of CDF strictly speaking). If your model works well enough >> it won't be too hard to convince me :-) >> >>> How do you see handling the devices where DBI or DSI is used for video only, >>> and the control is handled via, say, i2c? The module has to register two >>> drivers, and try to keep those in sync? I feel that could get rather hacky. >> >> If DBI or DSI is used for video only you don't need DBI/DSI control >> operations, right ? So the entity driver will just be a regular I2C device >> driver. > > Well, DSI can't be used just like that. You need to configure it. > > The thing is, DSI is used for both control and video. They are really > the same thing, both are sent as DSI packets. Both need similar kind of > configuration for the bus. If the DSI peripheral sits on a DSI bus, I > imagine this configuration is done via the DSI bus support. But if > there's no DSI bus, how is the configuration done? > > I guess a possibility is to have a fixed configuration that cannot be > changed dynamically. But I have a feeling that it'll be a bit limiting > for some cases. > > And even with fixed configuration, I think the configuration would > somehow be passed as DSI bus parameters from the board files or in the > DT data (the same way as, say i2c bus speed). If there's no DSI bus, as > the DSI peripheral sits on the i2c bus, where are the parameters? Continuing my thoughts on the bus issue, I think there's a fundamental difference between "real" buses like i2c and DSI/DBI: i2c peripherals are designed with the mind set that there are other peripherals on the bus, whereas DSI/DBI peripherals are known to be alone, and the DSI/DBI peripheral may thus require the bus parameters to be changed according to the whims of the peripheral. Some examples coming to my mind from the hardware I know are the need to change the DBI bus width depending on what is being sent, changing the DSI bus clock speed, changing DSI return packet size. It's ok for the DBI/DSI peripheral HW designers to require things like that, because the spec doesn't say those can't be done, and the peripheral is alone on the bus. We can support those kinds of operations both with the bus model you have, and my no-bus model. But with your bus model, I believe at least some of those operations may be needed even when the peripheral is controlled via i2c/spi. This also is related to the control model. I'm not sure of the details of the pipeline controller, but I fear that having the controller be the entity that knows about the strange needs of individual video peripherals will lead to lots of customized controllers for individual boards. That said, I agree that it's difficult to embed all the information into individual device drivers (although that's where it should be), because sometimes a wider view of the pipeline is needed to properly configure things. Tomi
Hi Laurent, On 6 September 2013 20:07, Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote: > Hi Vikas, > > On Wednesday 04 September 2013 16:20:59 Vikas Sajjan wrote: >> On 9 August 2013 22:44, Laurent Pinchart wrote: >> > MIPI DBI is a configurable-width parallel display bus that transmits >> > commands and data. >> > >> > Add a new DBI Linux bus type that implements the usual bus >> > infrastructure (including devices and drivers (un)registration and >> > matching, and bus configuration and access functions). >> > >> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> >> > --- >> > >> > drivers/video/display/Kconfig | 8 ++ >> > drivers/video/display/Makefile | 1 + >> > drivers/video/display/mipi-dbi-bus.c | 234 ++++++++++++++++++++++++++++++ >> > include/video/display.h | 4 + >> > include/video/mipi-dbi-bus.h | 125 +++++++++++++++++++ >> > 5 files changed, 372 insertions(+) >> > create mode 100644 drivers/video/display/mipi-dbi-bus.c >> > create mode 100644 include/video/mipi-dbi-bus.h > > [snip] > >> > diff --git a/drivers/video/display/mipi-dbi-bus.c >> > b/drivers/video/display/mipi-dbi-bus.c new file mode 100644 >> > index 0000000..791fb4d >> > --- /dev/null >> > +++ b/drivers/video/display/mipi-dbi-bus.c > > [snip] > >> > +/* ---------------------------------------------------------------------- >> > + * Device and driver (un)registration >> > + */ >> > + >> > +/** >> > + * mipi_dbi_device_register - register a DBI device >> > + * @dev: DBI device we're registering >> > + */ >> > +int mipi_dbi_device_register(struct mipi_dbi_device *dev, >> > + struct mipi_dbi_bus *bus) >> > +{ >> > + device_initialize(&dev->dev); >> > + >> > + dev->bus = bus; >> > + dev->dev.bus = &mipi_dbi_bus_type; >> > + dev->dev.parent = bus->dev; >> > + >> > + if (dev->id != -1) >> > + dev_set_name(&dev->dev, "%s.%d", dev->name, dev->id); >> > + else >> > + dev_set_name(&dev->dev, "%s", dev->name); >> > + >> > + return device_add(&dev->dev); >> > +} >> >> The function looks very much specific to NON-DT case where you will be >> calling mipi_dbi_device_register() in the machine file. > > You're absolutely right. > >> I was actually trying to migrate to CDFv3 and adding MIPI DSI support >> for exynos5250, >> but in my case where exynos5250 is fully DT based, in which case we >> need something like ./drivers/of/platform.c for MIPI DBI and MIPI DSI >> to add the MIPI DBI/DSI device via DT way, ./drivers/of/mipi_dbi.c and >> ./drivers/of/mipi_dsi.c >> >> may look like below, >> >> int of_mipi_dbi_device_register(struct device_node *np, >> const char *bus_id, >> struct device *parent) >> { >> struct mipi_dbi_device *dev; >> dev = of_device_alloc(np, bus_id, parent); >> >> if (!dev) >> return NULL; >> device_initialize(dev); >> >> dev->bus = &mipi_dbi_bus_type; >> dev->parent = parent; >> >> return of_device_add(dev); >> } >> >> Correct me if I am wrong. > > You're correct, but the implementation will need to be a little bit more > complex than that. From an API point of view, something like > of_i2c_register_devices() (drivers/of/of_i2c.c) would probably make sense. the > function should iterate over child nodes, and call > of_mipi_dbi_device_register() (we could maybe rename that to > of_mipi_dbi_device_create() to mimic the platform device code) for each child. > > In your above code, you should replace of_device_alloc() with > of_mipi_dbi_device_alloc(), as of_device_alloc() allocates a struct > platform_device. You should also call mipi_dsi_device_put() on the device if > of_device_add() returns a failure. > > Would you like to send a patch on top of 08/19 to implement this ? > Sure, will send the patch to add MIPI DBI/DSI device via DT way. >> > +EXPORT_SYMBOL_GPL(mipi_dbi_device_register); >> > + >> > +/** >> > + * mipi_dbi_device_unregister - unregister a DBI device >> > + * @dev: DBI device we're unregistering >> > + */ >> > +void mipi_dbi_device_unregister(struct mipi_dbi_device *dev) >> > +{ >> > + device_del(&dev->dev); >> > + put_device(&dev->dev); >> > +} >> > +EXPORT_SYMBOL_GPL(mipi_dbi_device_unregister); >> > + >> > +static int mipi_dbi_drv_probe(struct device *_dev) >> > +{ >> > + struct mipi_dbi_driver *drv = to_mipi_dbi_driver(_dev->driver); >> > + struct mipi_dbi_device *dev = to_mipi_dbi_device(_dev); >> >> Here we are assuming that mipi_dbi_device can be obtained by using >> _dev pointer, which may NOT be true in DT case, i think. > > Why wouldn't it be true (if we create the devices as explained above) ? Yeah, with the above method, it should be possible. > >> let me know if i am missing something. >> >> if you can give me a example for DT case, that would be helpful. > > I'm afraid I don't have any, as the DBI drivers I wrote are used by a platform > that doesn't support DT. > >> > + >> > + return drv->probe(dev); >> > +} > > -- > Regards, > > Laurent Pinchart >
diff --git a/drivers/video/display/Kconfig b/drivers/video/display/Kconfig index 1d533e7..f7532c1 100644 --- a/drivers/video/display/Kconfig +++ b/drivers/video/display/Kconfig @@ -2,3 +2,11 @@ menuconfig DISPLAY_CORE tristate "Display Core" ---help--- Support common display framework for graphics devices. + +if DISPLAY_CORE + +config DISPLAY_MIPI_DBI + tristate + default n + +endif # DISPLAY_CORE diff --git a/drivers/video/display/Makefile b/drivers/video/display/Makefile index b907aad..59022d2 100644 --- a/drivers/video/display/Makefile +++ b/drivers/video/display/Makefile @@ -1,3 +1,4 @@ display-y := display-core.o \ display-notifier.o obj-$(CONFIG_DISPLAY_CORE) += display.o +obj-$(CONFIG_DISPLAY_MIPI_DBI) += mipi-dbi-bus.o diff --git a/drivers/video/display/mipi-dbi-bus.c b/drivers/video/display/mipi-dbi-bus.c new file mode 100644 index 0000000..791fb4d --- /dev/null +++ b/drivers/video/display/mipi-dbi-bus.c @@ -0,0 +1,234 @@ +/* + * MIPI DBI Bus + * + * 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/device.h> +#include <linux/export.h> +#include <linux/kernel.h> +#include <linux/list.h> +#include <linux/module.h> +#include <linux/mutex.h> +#include <linux/pm.h> +#include <linux/pm_runtime.h> + +#include <video/mipi-dbi-bus.h> + +/* ----------------------------------------------------------------------------- + * Bus operations + */ + +int mipi_dbi_set_data_width(struct mipi_dbi_device *dev, unsigned int width) +{ + if (width != 8 && width != 16) + return -EINVAL; + + dev->data_width = width; + return 0; +} +EXPORT_SYMBOL_GPL(mipi_dbi_set_data_width); + +int mipi_dbi_write_command(struct mipi_dbi_device *dev, u16 cmd) +{ + return dev->bus->ops->write_command(dev->bus, dev, cmd); +} +EXPORT_SYMBOL_GPL(mipi_dbi_write_command); + +int mipi_dbi_write_data(struct mipi_dbi_device *dev, const u8 *data, + size_t len) +{ + return dev->bus->ops->write_data(dev->bus, dev, data, len); +} +EXPORT_SYMBOL_GPL(mipi_dbi_write_data); + +int mipi_dbi_read_data(struct mipi_dbi_device *dev, u8 *data, size_t len) +{ + return dev->bus->ops->read_data(dev->bus, dev, data, len); +} +EXPORT_SYMBOL_GPL(mipi_dbi_read_data); + +/* ----------------------------------------------------------------------------- + * Bus type + */ + +static const struct mipi_dbi_device_id * +mipi_dbi_match_id(const struct mipi_dbi_device_id *id, + struct mipi_dbi_device *dev) +{ + while (id->name[0]) { + if (strcmp(dev->name, id->name) == 0) { + dev->id_entry = id; + return id; + } + id++; + } + return NULL; +} + +static int mipi_dbi_match(struct device *_dev, struct device_driver *_drv) +{ + struct mipi_dbi_device *dev = to_mipi_dbi_device(_dev); + struct mipi_dbi_driver *drv = to_mipi_dbi_driver(_drv); + + if (drv->id_table) + return mipi_dbi_match_id(drv->id_table, dev) != NULL; + + return (strcmp(dev->name, _drv->name) == 0); +} + +static ssize_t modalias_show(struct device *_dev, struct device_attribute *a, + char *buf) +{ + struct mipi_dbi_device *dev = to_mipi_dbi_device(_dev); + int len = snprintf(buf, PAGE_SIZE, MIPI_DBI_MODULE_PREFIX "%s\n", + dev->name); + + return (len >= PAGE_SIZE) ? (PAGE_SIZE - 1) : len; +} + +static struct device_attribute mipi_dbi_dev_attrs[] = { + __ATTR_RO(modalias), + __ATTR_NULL, +}; + +static int mipi_dbi_uevent(struct device *_dev, struct kobj_uevent_env *env) +{ + struct mipi_dbi_device *dev = to_mipi_dbi_device(_dev); + + add_uevent_var(env, "MODALIAS=%s%s", MIPI_DBI_MODULE_PREFIX, + dev->name); + return 0; +} + +static const struct dev_pm_ops mipi_dbi_dev_pm_ops = { + .runtime_suspend = pm_generic_runtime_suspend, + .runtime_resume = pm_generic_runtime_resume, + .suspend = pm_generic_suspend, + .resume = pm_generic_resume, + .freeze = pm_generic_freeze, + .thaw = pm_generic_thaw, + .poweroff = pm_generic_poweroff, + .restore = pm_generic_restore, +}; + +static struct bus_type mipi_dbi_bus_type = { + .name = "mipi-dbi", + .dev_attrs = mipi_dbi_dev_attrs, + .match = mipi_dbi_match, + .uevent = mipi_dbi_uevent, + .pm = &mipi_dbi_dev_pm_ops, +}; + +/* ----------------------------------------------------------------------------- + * Device and driver (un)registration + */ + +/** + * mipi_dbi_device_register - register a DBI device + * @dev: DBI device we're registering + */ +int mipi_dbi_device_register(struct mipi_dbi_device *dev, + struct mipi_dbi_bus *bus) +{ + device_initialize(&dev->dev); + + dev->bus = bus; + dev->dev.bus = &mipi_dbi_bus_type; + dev->dev.parent = bus->dev; + + if (dev->id != -1) + dev_set_name(&dev->dev, "%s.%d", dev->name, dev->id); + else + dev_set_name(&dev->dev, "%s", dev->name); + + return device_add(&dev->dev); +} +EXPORT_SYMBOL_GPL(mipi_dbi_device_register); + +/** + * mipi_dbi_device_unregister - unregister a DBI device + * @dev: DBI device we're unregistering + */ +void mipi_dbi_device_unregister(struct mipi_dbi_device *dev) +{ + device_del(&dev->dev); + put_device(&dev->dev); +} +EXPORT_SYMBOL_GPL(mipi_dbi_device_unregister); + +static int mipi_dbi_drv_probe(struct device *_dev) +{ + struct mipi_dbi_driver *drv = to_mipi_dbi_driver(_dev->driver); + struct mipi_dbi_device *dev = to_mipi_dbi_device(_dev); + + return drv->probe(dev); +} + +static int mipi_dbi_drv_remove(struct device *_dev) +{ + struct mipi_dbi_driver *drv = to_mipi_dbi_driver(_dev->driver); + struct mipi_dbi_device *dev = to_mipi_dbi_device(_dev); + int ret; + + ret = drv->remove(dev); + if (ret < 0) + return ret; + + mipi_dbi_set_drvdata(dev, NULL); + + return 0; +} + +/** + * mipi_dbi_driver_register - register a driver for DBI devices + * @drv: DBI driver structure + */ +int mipi_dbi_driver_register(struct mipi_dbi_driver *drv) +{ + drv->driver.bus = &mipi_dbi_bus_type; + if (drv->probe) + drv->driver.probe = mipi_dbi_drv_probe; + if (drv->remove) + drv->driver.remove = mipi_dbi_drv_remove; + + return driver_register(&drv->driver); +} +EXPORT_SYMBOL_GPL(mipi_dbi_driver_register); + +/** + * mipi_dbi_driver_unregister - unregister a driver for DBI devices + * @drv: DBI driver structure + */ +void mipi_dbi_driver_unregister(struct mipi_dbi_driver *drv) +{ + driver_unregister(&drv->driver); +} +EXPORT_SYMBOL_GPL(mipi_dbi_driver_unregister); + +/* ----------------------------------------------------------------------------- + * Init/exit + */ + +static int __init mipi_dbi_init(void) +{ + return bus_register(&mipi_dbi_bus_type); +} + +static void __exit mipi_dbi_exit(void) +{ + bus_unregister(&mipi_dbi_bus_type); +} + +module_init(mipi_dbi_init); +module_exit(mipi_dbi_exit) + +MODULE_AUTHOR("Laurent Pinchart <laurent.pinchart@ideasonboard.com>"); +MODULE_DESCRIPTION("MIPI DBI Bus"); +MODULE_LICENSE("GPL"); diff --git a/include/video/display.h b/include/video/display.h index ba319d6..3138401 100644 --- a/include/video/display.h +++ b/include/video/display.h @@ -17,6 +17,7 @@ #include <linux/list.h> #include <linux/module.h> #include <media/media-entity.h> +#include <video/mipi-dbi-bus.h> #define DISPLAY_PIXEL_CODING(option, type, from, to, variant) \ (((option) << 17) | ((type) << 13) | ((variant) << 10) | \ @@ -189,6 +190,9 @@ enum display_entity_interface_type { struct display_entity_interface_params { enum display_entity_interface_type type; + union { + struct mipi_dbi_interface_params dbi; + } p; }; struct display_entity_control_ops { diff --git a/include/video/mipi-dbi-bus.h b/include/video/mipi-dbi-bus.h new file mode 100644 index 0000000..876b69d --- /dev/null +++ b/include/video/mipi-dbi-bus.h @@ -0,0 +1,125 @@ +/* + * MIPI DBI Bus + * + * 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 __MIPI_DBI_BUS_H__ +#define __MIPI_DBI_BUS_H__ + +#include <linux/device.h> + +struct mipi_dbi_bus; +struct mipi_dbi_device; + +struct mipi_dbi_bus_ops { + int (*write_command)(struct mipi_dbi_bus *bus, + struct mipi_dbi_device *dev, u16 cmd); + int (*write_data)(struct mipi_dbi_bus *bus, struct mipi_dbi_device *dev, + const u8 *data, size_t len); + int (*read_data)(struct mipi_dbi_bus *bus, struct mipi_dbi_device *dev, + u8 *data, size_t len); +}; + +struct mipi_dbi_bus { + struct device *dev; + const struct mipi_dbi_bus_ops *ops; +}; + +#define MIPI_DBI_MODULE_PREFIX "mipi-dbi:" +#define MIPI_DBI_NAME_SIZE 32 + +struct mipi_dbi_device_id { + char name[MIPI_DBI_NAME_SIZE]; + __kernel_ulong_t driver_data /* Data private to the driver */ + __aligned(sizeof(__kernel_ulong_t)); +}; + +enum mipi_dbi_interface_type { + MIPI_DBI_INTERFACE_TYPE_A, + MIPI_DBI_INTERFACE_TYPE_B, +}; + +#define MIPI_DBI_INTERFACE_TE (1 << 0) + +struct mipi_dbi_interface_params { + enum mipi_dbi_interface_type type; + unsigned int flags; + + unsigned int cs_setup; + unsigned int rd_setup; + unsigned int rd_latch; + unsigned int rd_cycle; + unsigned int rd_hold; + unsigned int wr_setup; + unsigned int wr_cycle; + unsigned int wr_hold; +}; + +#define MIPI_DBI_FLAG_ALIGN_LEFT (1 << 0) + +struct mipi_dbi_device { + const char *name; + int id; + struct device dev; + + const struct mipi_dbi_device_id *id_entry; + struct mipi_dbi_bus *bus; + + unsigned int flags; + unsigned int bus_width; + unsigned int data_width; +}; + +#define to_mipi_dbi_device(d) container_of(d, struct mipi_dbi_device, dev) + +int mipi_dbi_device_register(struct mipi_dbi_device *dev, + struct mipi_dbi_bus *bus); +void mipi_dbi_device_unregister(struct mipi_dbi_device *dev); + +struct mipi_dbi_driver { + int(*probe)(struct mipi_dbi_device *); + int(*remove)(struct mipi_dbi_device *); + struct device_driver driver; + const struct mipi_dbi_device_id *id_table; +}; + +#define to_mipi_dbi_driver(d) container_of(d, struct mipi_dbi_driver, driver) + +int mipi_dbi_driver_register(struct mipi_dbi_driver *drv); +void mipi_dbi_driver_unregister(struct mipi_dbi_driver *drv); + +static inline void *mipi_dbi_get_drvdata(const struct mipi_dbi_device *dev) +{ + return dev_get_drvdata(&dev->dev); +} + +static inline void mipi_dbi_set_drvdata(struct mipi_dbi_device *dev, + void *data) +{ + dev_set_drvdata(&dev->dev, data); +} + +/* module_mipi_dbi_driver() - Helper macro for drivers that don't do + * anything special in module init/exit. This eliminates a lot of + * boilerplate. Each module may only use this macro once, and + * calling it replaces module_init() and module_exit() + */ +#define module_mipi_dbi_driver(__mipi_dbi_driver) \ + module_driver(__mipi_dbi_driver, mipi_dbi_driver_register, \ + mipi_dbi_driver_unregister) + +int mipi_dbi_set_data_width(struct mipi_dbi_device *dev, unsigned int width); + +int mipi_dbi_write_command(struct mipi_dbi_device *dev, u16 cmd); +int mipi_dbi_read_data(struct mipi_dbi_device *dev, u8 *data, size_t len); +int mipi_dbi_write_data(struct mipi_dbi_device *dev, const u8 *data, + size_t len); + +#endif /* __MIPI_DBI_BUS__ */
MIPI DBI is a configurable-width parallel display bus that transmits commands and data. Add a new DBI Linux bus type that implements the usual bus infrastructure (including devices and drivers (un)registration and matching, and bus configuration and access functions). Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> --- drivers/video/display/Kconfig | 8 ++ drivers/video/display/Makefile | 1 + drivers/video/display/mipi-dbi-bus.c | 234 +++++++++++++++++++++++++++++++++++ include/video/display.h | 4 + include/video/mipi-dbi-bus.h | 125 +++++++++++++++++++ 5 files changed, 372 insertions(+) create mode 100644 drivers/video/display/mipi-dbi-bus.c create mode 100644 include/video/mipi-dbi-bus.h