Re: [RFC 3/5] video: panel: Add MIPI DBI bus support

From: Tomi Valkeinen <tomi.valkeinen@ti.com>
To: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: linux-fbdev@vger.kernel.org, dri-devel@lists.freedesktop.org,
	linux-leds@vger.kernel.org, linux-media@vger.kernel.org,
	Bryan Wu <bryan.wu@canonical.com>,
	Richard Purdie <rpurdie@rpsys.net>,
	Marcus Lorentzon <marcus.lorentzon@linaro.org>,
	Sumit Semwal <sumit.semwal@ti.com>, Archit Taneja <archit@ti.com>,
	Sebastien Guiriec <s-guiriec@ti.com>,
	Inki Dae <inki.dae@samsung.com>,
	Kyungmin Park <kyungmin.park@samsung.com>
Subject: Re: [RFC 3/5] video: panel: Add MIPI DBI bus support
Date: Fri, 17 Aug 2012 12:03:02 +0300	[thread overview]
Message-ID: <1345194182.3158.66.camel@deskari> (raw)
In-Reply-To: <1345164583-18924-4-git-send-email-laurent.pinchart@ideasonboard.com>

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

On Fri, 2012-08-17 at 02:49 +0200, Laurent Pinchart wrote:

> +/* -----------------------------------------------------------------------------
> + * Bus operations
> + */
> +
> +void panel_dbi_write_command(struct panel_dbi_device *dev, unsigned long cmd)
> +{
> +	dev->bus->ops->write_command(dev->bus, cmd);
> +}
> +EXPORT_SYMBOL_GPL(panel_dbi_write_command);
> +
> +void panel_dbi_write_data(struct panel_dbi_device *dev, unsigned long data)
> +{
> +	dev->bus->ops->write_data(dev->bus, data);
> +}
> +EXPORT_SYMBOL_GPL(panel_dbi_write_data);
> +
> +unsigned long panel_dbi_read_data(struct panel_dbi_device *dev)
> +{
> +	return dev->bus->ops->read_data(dev->bus);
> +}
> +EXPORT_SYMBOL_GPL(panel_dbi_read_data);

I'm not that familiar with how to implement bus drivers, can you
describe in pseudo code how the SoC's DBI driver would register these?

I think write/read data functions are a bit limited. Shouldn't they be
something like write_data(const u8 *buf, int size) and read_data(u8
*buf, int len)?

Something that's totally missing is configuring the DBI bus. There are a
bunch of timing related values that need to be configured. See
include/video/omapdss.h struct rfbi_timings. While the struct is OMAP
specific, if I recall right most of the values match to the MIPI DBI
spec.

And this makes me wonder, you use DBI bus for SYS-80 panel. The busses
may look similar in operation, but are they really so similar when you
take into account the timings (and perhaps something else, it's been
years since I read the MIPI DBI spec)?

Then there's the start_transfer. This is something I'm not sure what is
the best way to handle it, but the same problems that I mentioned in the
previous post related to enable apply here also. For example, what if
the panel needs to be update in two parts? This is done in Nokia N9.
From panel's perspective, it'd be best to handle it somewhat like this
(although asynchronously, probably):

write_update_area(0, 0, xres, yres / 2);
write_memory_start()
start_pixel_transfer();

wait_transfer_done();

write_update_area(0, yres / 2, xres, yres / 2);
write_memory_start()
start_pixel_transfer();

Why I said I'm not sure about this is that it does complicate things, as
the actual pixel data often comes from the display subsystem hardware,
which should probably be controlled by the display driver.

I think there also needs to be some kind of transfer_done notifier, for
both the display driver and the panel driver. Although if the display
driver handles starting the actual pixel transfer, then it'll get the
transfer_done via whatever interrupt the SoC has.

Also as food for thought, videomode timings does not really make sense
for DBI panels, at least when you just consider the DBI side. There's
really just the resolution of the display, and then the DBI timings. No
pck, syncs, etc. Of course in the end there's the actual panel, which
does have these video timings. But often they cannot be configured, and
often you don't even know them as the specs don't tell them.

 Tomi

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]