Re: [RFC 00/17] OMAPDSS: Change way of passing timings from panel driver to interface

Archit Taneja
2012-08-01T10:37:21Z

On Wednesday 01 August 2012 04:01 PM, Archit Taneja wrote:
> This series tries to make interface drivers less dependent on omap_dss_device
> which represents a panel/device connected to that interface. The current way of
> configuring an interface is to populate the panel's omap_dss_device instance
> with parameters common to the panel and the interface, they are either populated
> in the board file, or in the panel driver. Panel timings, number of lanes
> connected to interface, and pixel format are examples of such parameters, these
> are then extracted by the interface driver to configure itself.
>
> This approach has some disadvantages:
>
> - The omap_dss_device contains fields which could be handled independently by
> the panel driver. For example, we have an enum field in omap_dss_device to
> tell what mode the panel operates in. This information could be handled by the
> panel driver itself. But it's a part of omap_dss_device since we need to pass
> this down to the interface driver.
>
> - An interface can't exist by itself. That is, it needs a panel to be connected
> to it to configure itself. It's not practical to configure an interface
> without a panel, but it's theoretically possible, and we may need it if we
> expose the interface as an entity to a user of OMAPDSS. It's also useful if we
> represent writeback as an interface, writeback isn't connected to a panel.
>
> - There is a lack of clarity in how the interface configures itself. Since the
> interface driver extracts info from omap_dss_device, it's unclear from a panel
> driver point of view about what information in omap_dss_device the interface
> is using and what it's not using.
>
> - There are issues with checking the correctness of the parameters in
> omap_dss_device. We currently fill up the omap_dss_device completely, and then
> try to enable the interface, this results in catching a wrong parameter at a
> much later point, rather than catching it immediately.
>
> The alternative approach is for the interface drivers to expose functions/api to
> the panel drivers to configure such parameters. This way, the panel driver can
> pass the parameters to the interface itself, rather than filling up
> omap_dss_device. This would need the panel driver to keep a copy of the
> parameters so that it can use to configure it later. This resolves all the
> issues mentioned above, and also gives us a chance to make a generic set of
> function ops for interfaces, this can make a panel driver independent of
> the underlying platform.
>
> The current series starts of this work by creating a set_timings function for
> all interfaces passing omap_video_timings, this prevents dssdev->panel.timings
> references. The first few patches are some minor cleanups which are useful for
> the patches which come later.
>
> There are some points on which I need suggestions/clarifications:
> - How do we make sure that these functions are called by the panel driver at the
> right time? For example, when setting timings for DSI video mode, we would
> need to call omapdss_dsi_set_timings() before we call
> omapdss_dsi_display_enable(), otherwise the copy of timings contained in DSI
> driver data would be invalid. Also, what should the behaviour of such a
> set_timings operation if the interface is already enabled. It is clear for
> DPI and HDMI, but I'm not clear about what to do about other interfaces. Do we
> add checks for the state of the interface/panel?
>
> - A specific issue about DSI/RFBI getting the resolution via
> device->driver->get_resolution(), does this provide a result based on panel
> rotation? Can this somehow be replaced by timings?
>
> - For SDI, the set_timings operation is simplified, instead of disabling and
> then enabling the panel with a new set of timings, only the new timings are
> configured. This is similar to what is done in DPI. I am not clear if this
> will work for SDI or not.
>
> - There is no set_timings() function for RFBI yet, this needs to be though of
> and fixed.
>
> The reference tree and branch:

forgot to add the link here:

git://gitorious.org/~boddob/linux-omap-dss2/archit-dss2-clone.git
pass_timings_interface

Archit
>
> This is based on Tomi's for-florian-merged branch, and has 2 of his patches which
> got missed the last merge window.
>
> This hasn't been tested thoroughly with all interfaces yet. I was interested in
> getting some comments.
>
> Archit Taneja (17):
> OMAPDSS: APPLY: Constify timings argument in dss_mgr_set_timings
> OMAPDSS: DPI: Remove omap_dss_device arguments in
> dpi_set_dsi_clk/dpi_set_dispc_clk
> OMAPDSS: HDMI: Remove omap_dss_device argument from hdmi_compute_pll
> OMAPDSS: DPI: Add locking for DPI interface
> OMAPDSS: DPI: Maintain our own timings field in driver data
> OMAPDSS: DPI displays: Take care of panel timings in the driver
> itself
> OMAPDSS: Displays: Add locking in generic DPI panel driver
> OMAPDSS: DSI: Maintain own copy of timings in driver data
> OMAPDSS: HDMI: Use our own omap_video_timings field when setting
> interface timings
> OMAPDSS: HDMI: Add a get_timing function for HDMI interface
> OMAPDSS: HDMI: Add locking for hdmi interface get/set timing
> functions
> OMAPDSS: SDI: Create a separate function for timing/clock
> configurations
> OMAPDSS: SDI: Create a function to set timings
> OMAPDSS: SDI: Maintain our own timings field in driver data
> OMAPDSS: VENC: Split VENC into interface and panel driver
> OMAPDSS: VENC: Maintain our own timings field in driver data
> OMAPDSS: VENC: Add a get_timing function for VENC interface
>
> drivers/video/omap2/displays/panel-acx565akm.c | 13 +-
> drivers/video/omap2/displays/panel-generic-dpi.c | 75 ++++++-
> .../omap2/displays/panel-lgphilips-lb035q02.c | 2 +
> .../omap2/displays/panel-nec-nl8048hl11-01b.c | 2 +
> drivers/video/omap2/displays/panel-picodlp.c | 3 +
> .../video/omap2/displays/panel-sharp-ls037v7dw01.c | 2 +
> drivers/video/omap2/displays/panel-taal.c | 2 +
> drivers/video/omap2/displays/panel-tfp410.c | 5 +-
> .../video/omap2/displays/panel-tpo-td043mtea1.c | 6 +-
> drivers/video/omap2/dss/Makefile | 2 +-
> drivers/video/omap2/dss/apply.c | 4 +-
> drivers/video/omap2/dss/dpi.c | 52 +++--
> drivers/video/omap2/dss/dsi.c | 27 ++-
> drivers/video/omap2/dss/dss.h | 19 +-
> drivers/video/omap2/dss/hdmi.c | 60 +++--
> drivers/video/omap2/dss/hdmi_panel.c | 12 +-
> drivers/video/omap2/dss/sdi.c | 87 +++++---
> drivers/video/omap2/dss/venc.c | 233 ++++++--------------
> drivers/video/omap2/dss/venc_panel.c | 231 +++++++++++++++++++
> include/video/omapdss.h | 8 +-
> 20 files changed, 579 insertions(+), 266 deletions(-)
> create mode 100644 drivers/video/omap2/dss/venc_panel.c
>