[PATCH 3/6] bitops: bitmap helper to set variable length values

[Sebastian Fricke]

Hey Yury and Andy,

thanks for you review and discussion, after some further communication
with Nicolas Dufresne and Andrzej Pietrasiewicz, we came to the
conclusion that I will prepare a more general bit-writer API, which fits
better to the use-case we have on a lot of multimedia hardware.

I'll probably reuse the bitmap API as the backbone of that
implementation and use the `*_set8` to write the content back to memory,
but as we have a few more potential users for such an API, I'll strive
towards creating a new kernel API instead of extending an existing API
that isn't designed for our use-case.

Thanks a lot for your input and ideas.

Greetings,
Sebastian

On 14.07.2022 14:24, Andy Shevchenko wrote:
>On Wed, Jul 13, 2022 at 01:42:17PM -0700, Yury Norov wrote:
>> On Wed, Jul 13, 2022 at 10:14:24PM +0200, Andy Shevchenko wrote:
>> > On Wed, Jul 13, 2022 at 9:44 PM Yury Norov <yury.norov at gmail.com> wrote:
>> > > On Wed, Jul 13, 2022 at 09:10:33PM +0200, Andy Shevchenko wrote:
>> > > > On Wed, Jul 13, 2022 at 8:56 PM Yury Norov <yury.norov at gmail.com> wrote:
>> > > > > On Wed, Jul 13, 2022 at 06:31:59PM +0200, Sebastian Fricke wrote:
>
>...
>
>> > > > > I'd suggest you to try implementing
>> > > > >         bitmap_copy_from(dst, src, dst_off, len)
>> > > > > or even
>> > > > >         bitmap_copy_from(dst, dst_off, src, src_off, len)
>> > > > > if you expect that you'll need more flexibility in the future.
>> > > >
>> > > > Do you think it would be useful?
>> > > >
>> > > > We have bitmap_replace() & bitmap_remap(). Wouldn't that be enough?
>> > >
>> > > bitmap_replace and bitmap_remap have no an 'offset' parameter.
>> >
>> > True.
>> >
>> > But then it's a bit too generic to have this src_off, no?
>>
>> That's why I said:
>>
>> > > > > if you expect that you'll need more flexibility in the future.
>>
>> My preferred option is bitmap_copy_from(dst, src, dst_off, len).
>>
>> > I would rather expect for asymmetrical bitmaps that the other side
>> > will be either one of the fixed width types (it makes sense to have
>> > for 32- or 64-bit arguments.
>>
>> Look at patch #6 - it copies 1,4,5,9,10,32,37... - pretty much a random
>> number number of bits.
>
>It's too poor randomness, as u64 covers all what in patch 6.
>
>> > When you have a source bitmap of x bits and  you would like to copy it
>> > into a y-bit one, I would think that either you have a small amount of
>> > bits in x anyway, or x is a full-sized bitmap (same order as y).
>>
>> It sounds like a speculation to me. Why shouldn't we let people to
>> copy with an offset any number of bits?
>
>Because it's a common case. You have a value in the register / variable, which
>naturally is one of the POD types. Now you want to inject this into bitmap at
>the arbitrary offset. Value itself also needs to be variadic size in bits.
>
>Basically what he is trying to achieve is something like bitfield.h API over
>bitmaps. Dunno, if actually bitfield.h in the certain driver wouldn't be
>enough.
>
>> > Also
>> > keep in mind that granularity is long, so less than long it makes no
>> > sense.
>> >
>> >   bitmap_copy_from_T(unsigned long *map, start, len, T src),
>> >
>> > where T is type, start is the offset in map, len is the amount of bits
>> > from src starting from 0. That's what is required in most of the cases
>> > I believe.
>>
>> But not in Sebastian's case, according to patch #6.
>
>I think it's a case, see above.
>
>-- 
>With Best Regards,
>Andy Shevchenko
>
>