Re: [PATCH 12/12] OMAPDSS: DPI: always use DSI PLL if available

From: Tomi Valkeinen <tomi.valkeinen@ti.com>
To: Rob Clark <rob.clark@linaro.org>
Cc: Tomi Valkeinen <tomba@iki.fi>, Archit Taneja <archit@ti.com>,
	linux-omap@vger.kernel.org, linux-fbdev@vger.kernel.org,
	dri-devel@lists.freedesktop.org
Subject: Re: [PATCH 12/12] OMAPDSS: DPI: always use DSI PLL if available
Date: Thu, 08 Nov 2012 07:39:13 +0000	[thread overview]
Message-ID: <509B61A1.30902@ti.com> (raw)
In-Reply-To: <CAF6AEGsvRV=ez_aqz_tkb+azNMy4jQuSVdd7oKg6=nTnguX-Kw@mail.gmail.com>

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

On 2012-11-07 21:18, Rob Clark wrote:
> On Wed, Nov 7, 2012 at 9:13 AM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
>> On 2012-11-07 16:32, Rob Clark wrote:
>>> On Wed, Nov 7, 2012 at 4:01 AM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
>>
>>>> Hotplugging is not some abstract future scenario, we already have
>>>> hardware that could use it. For example, omap3 SDP board has a
>>>> switchable output to DVI or LCD panel. In this case we know what the two
>>>> options are, but the disabled component is still effectually removed
>>>> from the system, and plugged back in when it's enabled.
>>>
>>> I would look at this as two different connectors which can not be used
>>> at the same time.  You have this scenario with desktop graphics cards.
>>
>> Yes, that's an option with fixed amount of display devices. But doesn't
>> work for capes.
> 
> Only if capes are hotpluggable.. otherwise probe what cape(s?) are
> present at boot time (is this possible to detect a cape from sw?), and
> create the associated connector(s).

Well, a cape can be anything. For beaglebone they have capes with
eeprom, and you can detect it.

The reason I'd like to have hotplug is that it would simplify panel
drivers in the case where we have multiple possible panels for the same
output, like the DVI/LCD case above for omap3 SDP.

If we don't have hotplug, then both DVI and LCD panel devices are
present at the same time, and they will share resources. In the minimum
they are sharing the video output, but more often than not they share
gpios/powers/etc.

It's normal that a driver will acquire resources for its device in its
probe, and thus we would have two drivers acquiring the same resources
at boot time, leading to the other driver failing. We currently manage
this by acquiring the resources late, only when the panel is being
enabled. But I think that's rather ugly.

It would be much cleaner if the panel device does not exist at all if
the panel is disconnected, and is created only when it is connected.
This of course creates the problem of who is responsible for creating
the panel device, and what triggers it. I think that's case specific,
and for capes, it'd be the cape driver.

But then again, I guess it's acceptable that we don't allow changing the
plugged-in panels at runtime. The user would have to select them with
kernel parameters or such. I guess this would be ok for capes and
development boards. I'm not aware of a production board that would
switch panels at runtime, although I know these were on the table in Nokia.

> Anyways, I think we are stretching a bit hard for use cases for
> hot-pluggable panels..  I just prefer to ignore hotplug for now and
> come back to it when there is a more legitimate use-case.

Ok, fair enough. But let's keep hotplug in mind, and if we're going to
create code that would make hotplug impossible to implement, let's stop
for a moment and think if we can do that in some other way.

>> I'm not so sure. The output (dpi/dsi/hdmi...) is the second step in our
>> chain. The primary "output" is in the DISPC module, the overlay manager.
>> That's where the timings, pixel clock, etc. are programmed. The second
>> step, our output, is really a converter IP. It receives the primary
>> output, converts it and outputs something else. Just like an external
>> converter chip would do.
>>
> 
> the timings, pixel clock, vblank/framedone irqs, that all maps to crtc.
> 
> encoder == converter, so I think this fits.  "An encoder takes pixel
> data from a CRTC and converts it to a format suitable for any attached
> connectors" (from drm docbook)

Oh, ok. Then what you say makes sense. I thought encoder contains the
timings, as you said previously: "Basically the encoder is doing the
"control" stuff (power on/off, set timings, etc)".

In the case we have a long chain of display blocks, encoder could cover
all of them, not just the output block of OMAP? I don't know where the
panel goes, though.

>> And the output can be quite a bit anything. For example, with DBI or DSI
>> command mode outputs we don't have any of the conventional video
>> timings. It doesn't make sense to program, say, video blanking periods
>> to those outputs. But even with DBI and DSI we do have video blanking
>> periods in the DISPC's output, the ovl mgr.
> 
> the encoder has mode_fixup() which can alter the timings that end up
> getting set, which might be a way to account for this.  I guess really
> it should be the panel driver that is telling the encoder what
> adjusted timings to give to the crtc.. so the panel driver doesn't
> quite map to connector.

The panel can't say what timings to give to crtc, it depends on what's
between the crtc and the panel. In case of OMAP DSI, the dsi driver
needs to specify the timings for crtc, based on the panel.

 Tomi

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 897 bytes --]