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

[Archit Taneja <a0393947@ti.com>]

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.

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 
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.

Archit