Hi Tomasz, Thank you for your RFC. On Wednesday 30 January 2013 16:38:59 Tomasz Figa wrote: > 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. Could you briefly outline the rationale behind this ? I'm wondering whether get_params could limit us if a device needs to modify parameters at runtime. For instance a panel might need to change clock frequencies or number of used data lane depending on various runtime conditions. I don't have a real use case here, so it might just be a bit of over-engineering. > - 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). >From my experience with MIPI CSI (Camera Serial Interface, similar to DSI) some transceivers can reroute data lanes internally. Is that true for DSI as well ? In that case we would need a list of data lanes, not just a mask. > 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. One of the main locking issues here are cross-references. The display entity holds a reference to the video source (for video operations), and the display controller driver holds a reference to the display entity (for control operations), resulting in a cross-references lock situation. One possible solution would be to first unbind the display entity device from its driver to break the cycle. > - 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 -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html