On Tue, 11 Feb 2014, "Volkin, Bradley D" <bradley.d.volkin@xxxxxxxxx> wrote:
> On Fri, Feb 07, 2014 at 06:45:48AM -0800, Daniel Vetter wrote:
>> On Fri, Feb 07, 2014 at 03:58:46PM +0200, Jani Nikula wrote:
>> > On Wed, 29 Jan 2014, bradley.d.volkin@xxxxxxxxx wrote:
>> > > +static int valid_reg(const u32 *table, int count, u32 addr)
>> > > +{
>> > > + if (table && count != 0) {
>> > > + int i;
>> > > +
>> > > + for (i = 0; i < count; i++) {
>> > > + if (table[i] == addr)
>> > > + return 1;
>> > > + }
>> > > + }
>> >
>> > You go to great lengths to validate the register tables are sorted, but
>> > in the end you don't take advantage of this fact by bailing out early if
>> > the lookup goes past the addr.
>> >
>> > Is this optimization the main reason for having the tables sorted, or
>> > are there other reasons too (I couldn't find any)?
>> >
>> > I'm beginning to wonder if this is a premature optimization that adds
>> > extra code. For master restricted registers you will always scan the
>> > regular reg table completely first. Perhaps a better option would be to
>> > have all registers in the same table, with a separate master flag,
>> > ordered by how frequently they are expected to be used. We do want to
>> > optimize for the happy day scenario. But maybe it's too early to tell.
>> >
>> > I'm inclined to ripping out the sort requirement and check, if the sole
>> > purpose is optimization, for simplicity's sake.
>>
>> tbh I don't mind the sorting requirement, and iirc Brad has patches
>> already for binary search. Once we start to rely on the sorting we can
>> easily add a little functions which checks for that at ring
>> initialization, so I also don't see any concerns wrt code fragility.
>
> Sorry for the delayed response. The background here is that I originally
> just had the tables sorted with a comment to say as much. The idea was that
> if the linear search became an issue, switching algorithms would be easier.
> Chris suggested just moving to bsearch and checking that the tables are sorted
> as part of the v1 series review. I implemented bsearch and found that the perf
> change was the same to slightly worse. So I added the sorting check and kept
> the linear search until we have better data.
Ok. For the linear search I think you could add the check if you've
iterated past the register and bail out early, and gather the data with
that.
BR,
Jani.
>
> Thanks,
> Brad
>
>> -Daniel
>> --
>> Daniel Vetter
>> Software Engineer, Intel Corporation
>> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
