Hi,
Sorry for the delay and for the noise on this older version. I first
want to understand the code better.
On Thursday 22 Oct 2020 at 11:55:28 (+0100), Lukasz Luba wrote:
[..]
>
> >
> > > +{
> > > + /* Make some space if needed */
> > > + if (status->busy_time > 0xffff) {
> > > + status->busy_time >>= 10;
> > > + status->total_time >>= 10;
> > > + }
> >
> > How about removing the above code and adding here:
> >
> > status->busy_time = status->busy_time ? : 1;
>
> It's not equivalent. The code operates on raw device values, which
> might be big (e.g. read from counters). If it's lager than the 0xffff,
> it is going to be shifted to get smaller.
>
Yes, the big values are handled below through the division and by making
total_time = 1024. These two initial checks are only to cover the
possibility for busy_time and total_time being 0, or busy_time >
total_time.
> >
> > > +
> > > + if (status->busy_time > status->total_time)
> >
> > This check would then cover the possibility that total_time is 0.
> >
> > > + status->busy_time = status->total_time;
> >
> > But a reversal is needed here:
> > status->total_time = status->busy_time;
>
> No, I want to clamp the busy_time, which should not be bigger that
> total time. It could happen when we deal with 'raw' values from device
> counters.
>
Yes, I understand. But isn't making total_time = busy_time accomplishing
the same thing?
> >
> > > +
> > > + status->busy_time *= 100;
> > > + status->busy_time /= status->total_time ? : 1;
> > > +
> > > + /* Avoid division by 0 */
> > > + status->busy_time = status->busy_time ? : 1;
> > > + status->total_time = 100;
> >
> > Then all of this code can be replaced by:
> >
> > status->busy_time = (unsigned long)div64_u64((u64)status->busy_time << 10,
> > status->total_time);
> > status->total_time = 1 << 10;
>
> No, the total_time closed to 'unsigned long' would overflow.
>
I'm not sure I understand. total_time gets a value of 1024, it's not
itself shifted by 10.
> >
> > This way you gain some resolution to busy_time and the divisions in the
> > callers would just become shifts by 10.
>
>
> I don't want to gain more resolution here. I want to be prepare for raw
> (not processed yet) big values coming from driver.
>
Agreed! The higher resolution is an extra benefit. The more important
benefit is that, through my suggestion, you'd be replacing all future
divisions by shifts.
Thanks,
Ionela.
> Regards,
> Lukasz
>
> >
> > Hope it helps,
> > Ionela.
> >
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
