Re: [PATCH v2 08/12] intel-ipu3: params: compute and program ccs

[Tuukka Toivonen <tuukka.toivonen@intel.com>]

On Wed, 2017-10-11 at 16:31 +0300, Andy Shevchenko wrote:
> On Wed, Oct 11, 2017 at 10:29 AM, sakari.ailus@linux.intel.com
> <sakari.ailus@linux.intel.com> wrote:
> > On Wed, Oct 11, 2017 at 04:14:37AM +0000, Zhi, Yong wrote:
> > > > > +static unsigned int ipu3_css_scaler_get_exp(unsigned int
> > > > > counter,
> > > > > +                                           unsigned int
> > > > > divider) {
> > > > > +       unsigned int i = 0;
> > > > > +
> > > > > +       while (counter <= divider / 2) {
> > > > > +               divider /= 2;
> > > > > +               i++;
> > > > > +       }
> > > > > +
> > > > > +       return i;
> >         return (!counter || divider < counter) ?
> >                0 : fls(divider / counter) - 1;
> 
> Extra division is here (I dunno if counter is always power of 2 but
> it
> doesn't matter for compiler).
> 
> Basically above calculates how much bits we need to shift divider to
> get it less than counter.
> 
> I would consider to use something from log2.h.
> 
> Roughly like
> 
> if (!counter || divider < counter)
>  return 0;
> return order_base_2(divider) - order_base_2(counter);

The original loop is typical ran just couple of times, so I think
that fls or division are probably slower than the original loop.
Furthermore, these "optimizations" are also harder to read, so in
my opinion there's no advantage in using them.

- Tuukka