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

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

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

On Mon, 2012-02-06 at 16:54 +0530, Archit Taneja wrote:
> 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).

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


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