iio: Add support for describing scan offsets in buffer ops

Hello,

This is a tentative proposal with no code attached. This issue came up while hacking on the mpu6050 driver but it applies to other complex devices and a solution belongs in the IIO core.

One IIO limitation is that while samples can be described through a relatively generic scan_type they must be placed in the output buffer according to an specific iio alignment algorithm. For example if the device has one 16bit channel and one 32bit channel the output needs to have 16bits of padding between them. When this does not match the hardware format the solution is for the driver to unpack the hardware samples and repack them in the iio format.

I propose that we add a new function in iio_buffer_setup_ops which looks like this:

int (*get_scan_offsets)(struct iio_dev *indio_dev,

                        off_t *scan_offsets,
			size_t *sample_bytes);

This would be called when enabling a buffer to fetch the offset in the sample for each channel (by scan_index) based on the currently configured scan mask. Then the iio core would then handle rearranging sample bytes as expected by the buffer consumer. Driver implementations could look something like this:

off_t pos = 0;

if (scan_mask & (1 << SCAN_INDEX_TEMP))) {
    scan_offsets[SCAN_INDEX_TEMP] = pos++;
}
/* No 2-bytes alignment here! */
if (scan_mask & SCAN_MASK_ACCEL_ANY) {
    scan_offsets[SCAN_INDEX_ACCEL_X] = pos + 0;
    scan_offsets[SCAN_INDEX_ACCEL_Y] = pos + 2;
    scan_offsets[SCAN_INDEX_ACCEL_Z] = pos + 4;
    pos += 6;
}

*sample_bytes = pos;

This could be implemented as an extension to the current buffer demuxing support which currently only deals with differences in scan_masks. This mechanism would very nicely deal with devices where it is not possible to select X/Y/Z individually because the iio core would just ignore the gap in offsets.

This can even be exposed to userspace in a compatible manner. User would do something like:

    ioctl(iio_fd, IIO_USER_HANDLES_SCAN_OFFSETS, 1);
    ioctl(iio_fd, IIO_GET_SCAN_OFFSETS, &scan_offsets, &sample_bytes);

When the consumer is marked as aware of scan offsets this way the iio core can skip demuxing for increased performance. The IIO core can implement IIO_GET_SCAN_OFFSETS itself when the driver has no custom implementation. An application could rely on IIO_GET_SCAN_OFFSETS for all devices as long as it's acceptable to ignore old kernels.

This would be particularly useful if IIO sample buffers are memory-mapped eventually. I think that this extension would be incrementally useful to that effort.

Quoted 24 lines · Crestez Dan Leonard

I propose that we add a new function in iio_buffer_setup_ops which looks like this:

int (*get_scan_offsets)(struct iio_dev *indio_dev,

                        off_t *scan_offsets,
			size_t *sample_bytes);

This would be called when enabling a buffer to fetch the offset in the sample for each channel (by scan_index) based on the currently configured scan mask. Then the iio core would then handle rearranging sample bytes as expected by the buffer consumer. Driver implementations could look something like this:

off_t pos = 0;

if (scan_mask & (1 << SCAN_INDEX_TEMP))) {
    scan_offsets[SCAN_INDEX_TEMP] = pos++;
}
/* No 2-bytes alignment here! */
if (scan_mask & SCAN_MASK_ACCEL_ANY) {
    scan_offsets[SCAN_INDEX_ACCEL_X] = pos + 0;
    scan_offsets[SCAN_INDEX_ACCEL_Y] = pos + 2;
    scan_offsets[SCAN_INDEX_ACCEL_Z] = pos + 4;
    pos += 6;
}

Quoted 1 line

*sample_bytes = pos;

Sure - this side is straight forward enough. It's the core complexity that makes me wonder if this is worthwhile. Btw if this is worth doing then we should really be looking at non byte aligned packed records as well (just to make it really hideous!)

The killer in here is endian requirements. If the core is repacking data it will need to understand these completely. Two options exist, either the core needs to 'edit' the shift provided by the driver, or it needs to do endian dependent data realignment.

Take a typical 3 byte unaligned packed record being expanded to a 4 byte aligned record. I think the easy cases are:

ABC -> 0ABC
CBA -> CBA0
so endian dependent shifts.

Quoted 3 lines

This would be particularly useful if IIO sample buffers are memory-mapped eventually. I think that this extension would be incrementally useful to that effort.

Perhaps. I'm interested to hear other views on this suggestion, but right now I'm not convinced. I'd argue that we want clean data presentations to userspace and that the restrictions as they currently stand are not really that burdensome on the data.

The exception is dma buffers - in those cases it really might be the case that we want to allow more complex alignment.

Quoted 2 lines · Jonathan Cameron

Quoted 1 line · Crestez Dan Leonard

I propose that we add a new function in iio_buffer_setup_ops which

looks

Quoted 7 lines

Quoted 7 lines

like this:

int (*get_scan_offsets)(struct iio_dev *indio_dev,

                        off_t *scan_offsets,
			size_t *sample_bytes);

This would be called when enabling a buffer to fetch the offset in

the

Quoted 2 lines

Quoted 2 lines

sample for each channel (by scan_index) based on the currently configured scan mask. Then the iio core would then handle

rearranging

Quoted 1 line

Quoted 1 line

sample bytes as expected by the buffer consumer. Driver

implementations

Quoted 14 lines

Quoted 13 lines

could look something like this:

off_t pos = 0;

if (scan_mask & (1 << SCAN_INDEX_TEMP))) {
    scan_offsets[SCAN_INDEX_TEMP] = pos++;
}
/* No 2-bytes alignment here! */
if (scan_mask & SCAN_MASK_ACCEL_ANY) {
    scan_offsets[SCAN_INDEX_ACCEL_X] = pos + 0;
    scan_offsets[SCAN_INDEX_ACCEL_Y] = pos + 2;
    scan_offsets[SCAN_INDEX_ACCEL_Z] = pos + 4;
    pos += 6;
}

Quoted 1 line

*sample_bytes = pos;

Quoted 1 line

Sure - this side is straight forward enough. It's the core

complexity

Quoted 1 line

that makes me wonder if this is worthwhile. Btw if this is worth

doing

Quoted 1 line

then we should really be looking at non byte aligned packed records

as

Quoted 1 line

well (just to make it really hideous!)

You could have a bunch of channels with different realbits/shift and the
same offset.

Quoted 2 lines

The killer in here is endian requirements. If the core is repacking

data

Quoted 1 line

it will need to understand these completely. Two options exist,

either

Quoted 1 line

the core needs to 'edit' the shift provided by the driver, or it

needs

Quoted 9 lines

to do endian dependent data realignment.

Take a typical 3 byte unaligned packed record being expanded to a 4 byte aligned record. I think the easy cases are:

ABC -> 0ABC
CBA -> CBA0
so endian dependent shifts.

I'm not sure how unaligned 24bit channels would be handled. Maybe it could be done storagebits=32, realbits=24, shift=8 or 0 and offsets that
are multiples of 3?

I guess it would have to know about endianness.

Quoted 7 lines

Quoted 3 lines

This would be particularly useful if IIO sample buffers are memory-mapped eventually. I think that this extension would be incrementally useful to that effort.

Perhaps. I'm interested to hear other views on this suggestion, but right now I'm not convinced. I'd argue that we want clean data presentations to userspace and that the restrictions as they

currently

Quoted 5 lines

stand are not really that burdensome on the data.

The exception is dma buffers - in those cases it really might be the case that we want to allow more complex alignment.

The point of exposing this through the iio core to userspace is that it could be used to support this kind of devices. It's not worth exposing as ABI unless it can support a lot of stuff at once.

It might make more sense to just provide some helpers for drivers to do their own easy reformatting.