[RFC 0/3] solving omapdrm/omapdss layering issues

[Tomi Valkeinen]

On Tue, 2012-07-31 at 09:45 -0500, Rob Clark wrote:
> On Tue, Jul 31, 2012 at 8:40 AM, Tomi Valkeinen <tomi.valkeinen at ti.com> wrote:

> > It's not really about being friendly. Omapdss tries to do as little as
> > possible, while still supporting all its HW features. Shadow registers
> > are a bit tricky creating this mess.
> 
> What I mean by 'friendly' is it tries to abstract this for simple
> users, like an fbdev driver.  But this really quickly breaks down w/ a

No, that's not what omapdss tries to do. I'm not trying to hide the
shadow registers and the GO bit behind the omapdss API, I'm just trying
to make it work.

The omapdss API was made with omapfb, so it's true that the API may not
be good for omapdrm. But I'm happy to change the API.

> And btw, I think the current mapping of drm_encoder to mgr in omapdrm
> is not correct.  I'm just in the process of shuffling things around.
> I think drm/kms actually maps quite nicely to the underlying hardware
> with the following arrangement:
> 
>  drm_plane -> ovl
>  drm_crtc -> mgr
>  drm_encoder -> DSI/DPI/HDMI/VENC encoder
>  drm_connector -> pretty much what we call a panel driver today

Hmm, what was the arrangement earlier?

I guess the fact is that DRM concepts do not really match the OMAP DSS
hardware, and we'll have to use whatever gives us least problems.

> It would be quite useful if you could look at the omap_drm_apply
> mechanism I had in omapdrm, because that seems like a quite
> straightforward way to deal w/ shadowed registers.  I think it will

Yes, it seems straightforward, but it's not =).

I had a look at your omapdrm-on-dispc-2 branch. What you are doing there
is quite similar to what omapdss was doing earlier. It's not going to
work reliably with multiple outputs and fifomerge.

Configuring things like overlay color mode are quite simple. They only
affect that one overlay. Also things like manager default bg color are
simple, they affect only that one manager.

But enabling/disabling an overlay or a manager, changing the destination
mgr of an overlay, fifomerge... Those are not simple. You can't do them
directly, as you do in your branch.

As an example, consider the case of enabling an overlay (vid1), and
moving fifo buffers from currently enabled overlay (gfx) to vid1: you'll
first need to take the fifo buffers from gfx, set GO, and wait for the
settings to take effect. Only then you can set the fifo buffers for
vid1, enable it and set GO bit.

I didn't write omapdss's apply.c for fun or to make omapfb simpler. I
made it because the shadow register system is complex, and we need to
handle the tricky cases somewhere.

So, as I said before, I believe you'll just end up writing similar code
to what is currently in apply.c. It won't be as simple as your current
branch.

Also, as I mentioned earlier, you'll also need to handle the output side
of the shadow registers. These come from the output drivers (DPI, DSI,
etc, and indirectly from panel drivers). They are not currently handled
in the best manner in omapdss, but Archit is working on that and in his
version apply.c will handle also those properly.

About your code, I see you have these pre and post apply callbacks that
handle the configuration. Wouldn't it be rather easy to have omapdss's
apply.c call these?

And then one thing I don't think you've considered is manual update
displays. Of course, one option is to not support those with omapdrm,
but that's quite a big decision. omapdss's apply.c handles those also.

Also, can you check again my mail "Re: OMAPDSS vsyncs/apply" Sat, 12 May
2012 10:01:24, about the request_config() suggestion. I think that would
be somewhat similar to your pre/post callbacks. I'll try to write some
prototype for the request_config suggestion so that it's easier to
understand.

 Tomi


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part
URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20120801/0354c2ea/attachment.pgp>