Re: [PATCH 00/21] OMAPDSS: DISPC changes for writeback pipeline

[Tomi Valkeinen <tomi.valkeinen@ti.com>]

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

On Fri, 2012-09-14 at 15:54 +0530, Archit Taneja wrote:
> On Friday 14 September 2012 02:16 PM, Tomi Valkeinen wrote:
> > On Fri, 2012-09-14 at 11:27 +0300, Tomi Valkeinen wrote:
> >> On Thu, 2012-09-13 at 17:44 +0530, Archit Taneja wrote:
> >
> >>> This series prepares the low level DISPC driver(dispc.c) to configure writeback
> >>> registers. The aim is to reuse most of the code as most of its registers are
> >>> like overlay or manager registers, and are configured in the same way in most
> >>> cases. The first few patches rename dispc_ovl_* functions to dispc_plane_*
> >>
> >> I'm not sure if the renaming causes more confusion than clarity... It
> >> kinda creates a mishmash of ovl/plane names, and the term "plane"
> >> doesn't really sound like it's a base for both overlays and wb. Could we
> >> consider the wb as a special case, and keep the ovl name for most of the
> >> things and have "wb" used for wb specific things?
> >
> > And while WB is a combination of overlays and ovl managers, do you think
> > it'd be difficult to consider WB as a special, extended overlay? So just
> > call it an overlay, and consider it as an overlay with special features,
> > at least inside dispc.c. We probably need to have it as a totally
> > different entity from user's point of view (i.e. the list of overlays
> > wouldn't return WB, etc).
> 
> Yes, we could do that within dispc.c, we would still need some manager 
> like functions which set GO or ENABLE. But apart from that it should be 
> okay.

Yep, I was going through the WB registers, and to me it looks like 99%
of them are like overlay regs. Then there are a few bits like GO which
are special.

> I think for dispc.c in general, for future, it might be a good idea to 
> represent each piece of HW(like scalar or color converter, or a timing 

Scal_e_r! ;)

> FSM) as a little function/module, and construct 
> overlay/writeback/manager out of those, it might be cleaner. However, 
> this may be an overkill, and not needed much if there aren't any new 
> blocks comprising of these little blocks.

I agree. In the minimum we should try to somehow group functions related
to certain block, perhaps with name prefixes etc. I think it'll also
help understanding the code.

We probably currently have functions that touch multiple different
blocks. Those funcs should be split to handle only one of the blocks.

 Tomi


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