Re: [PATCH 1/2] OMAPDSS: Features: Maintain dss_feats as a list

[Archit Taneja <a0393947@ti.com>]

On Monday 06 February 2012 04:12 PM, Tomi Valkeinen wrote:
> Hi,
>
> On Mon, 2012-01-30 at 10:52 +0530, Archit Taneja wrote:
>> The number of dss_feat_id members has increased to a large value, the current
>> way of assigning a subset of these features (for a particular OMAP) as a mask
>> is no longer feasible.
>>
>> Maintain the subset of features supported as lists. Make the function
>> dss_has_feature() traverse through this list.
>
> I think this makes the code easier to maintain, so in that sense it is
> good. But I do hesitate a bit, I think many features are checked in
> often used code paths (the configuration done on each frame when
> swapping/panning), and bit compare versus finding an item in a list
> could have performance impact.
>
> Then again, I'm purely guessing here, as it could well be that compared
> to the other code, checking the features is insignificant. Thus, I'm
> fine with this patch, and we can optimize it later if need be.
>
> However, I'm anyway giving a few ideas how this could also be handled:
>
> - 64 bit mask. Would be simple, but we'd still have a hard limit there.
>
> - Variable length bitmask, i.e. an int or byte array from which a
> particular bit is checked. There could be a ready made datatype for this
> in the kernel.
>

There seems to be a bitmask library:

tools/power/cpupower/utils/helpers/bitmask.c

I don't know how easy it would be to access this though.

> - Lists like in this patch, but in sorted order. Then, if we're looking
> for a feat which has, say, number 4 assigned to it, we can stop
> iterating the list when we hit a feat>  4 in the list. Quite simple
> optimization, but needs extra maintenance to keep the feat lists sorted.

This sounds fine. It shouldn't be too much of an effort to maintain 
sorted lists. We'll just need to ensure that a new feature added has the 
largest integer value, and is always added at the end of the list(s).

Archit

>
>   Tomi
>