[Mesa-dev] [PATCH 02/10] dri_interface: add __DRI_IMAGE_TRANSFER_USER_STRIDE

[Gurchetan Singh]

On Mon, Apr 30, 2018 at 2:38 PM, Marek Olšák <maraeo at gmail.com> wrote:
> On Mon, Apr 30, 2018 at 3:11 PM, Eric Anholt <eric at anholt.net> wrote:
>>
>> Marek Olšák <maraeo at gmail.com> writes:
>>
>> > From: Nicolai Hähnle <nicolai.haehnle at amd.com>
>> >
>> > Allow the caller to specify the row stride (in bytes) with which an
>> > image
>> > should be mapped. Note that completely ignoring USER_STRIDE is a valid
>> > implementation of mapImage.
>> >
>> > This is horrible API design. Unfortunately, cros_gralloc does indeed
>> > have
>> > a horrible API design -- in that arbitrary images should be allowed to
>> > be
>> > mapped with the stride that a linear image of the same width would have.
>> >
>> > There is no separate capability bit because it's unclear how stricter
>> > requirements should be defined.
>>
>> I think that this is a bad interface, and you should just push an extra
>> copy into the caller so that they're encouraged to fix their bad API
>> instead of pushing their mistake down into Mesa ABI that we get stuck
>> with forever.
>
>
> I agree that it's a very bad interface. I would also like cros_gralloc not
> to do silly things. I don't know what kind of hardware gralloc is designed
> around - it's certainly not designed around modern GPUs, that part is clear.
> The emulation of the user-specified stride increases memory usage
> (especially if the stride is larger than necessary), so it's a bad idea by
> definition. I don't know if it's still possible to change gralloc not to use
> it.

You're welcome to submit patches to cros_gralloc / minigbm, but
ultimately we have to conform to these Android gralloc interfaces:

https://android.googlesource.com/platform/hardware/libhardware/+/master/include/hardware/gralloc.h
https://android.googlesource.com/platform/hardware/libhardware/+/master/include/hardware/gralloc1.h
https://android.googlesource.com/platform/hardware/interfaces/+/master/graphics/allocator/2.0/IAllocator.hal
https://android.googlesource.com/platform/hardware/interfaces/+/master/graphics/mapper/2.0/IMapper.hal

minigbm is still on gralloc0, but I don't think the newer interfaces
will help with your problem (i.e, returning the stride in the lock
function).  The API is better versioned now, so perhaps you could file
a bug on the Google tracker (https://buganizer.corp.google.com/)
explaining the AMD use case?  It might take until the next release (Q)
to see any changes.

> The reason the feature is pushed into the driver is that only the driver can
> efficiently force a certain stride where the stride is much larger than
> box.width*bpp of the mapping, and still only tile/detile box.width*bpp.
>
> Marek