On Mon, 2012-02-06 at 16:54 +0530, Archit Taneja wrote:

On Monday 06 February 2012 04:12 PM, Tomi Valkeinen wrote:

quoted

Hi,

On Mon, 2012-01-30 at 10:52 +0530, Archit Taneja wrote:

quoted

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.

quoted

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

Not necessarily. As we always search the list from index 0 forward, the
most often needed features could be in the beginning of the list so they
are found faster. At least some features are clearly needed only in some
enable/disable paths or similar, and they could clearly be low priority.

 Tomi