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

[Marek Olšák]

So this series is back in the review process. Only the last 4 patches need
an Rb and it's good to go.

Marek

On Fri, Apr 27, 2018 at 4:29 PM, Nicolai Hähnle <nhaehnle at gmail.com> wrote:

> On 27.04.2018 19:21, Gurchetan Singh wrote:
>
>> On Fri, Apr 27, 2018 at 2:00 AM, Nicolai Hähnle <nhaehnle at gmail.com>
>> wrote:
>>
>>> That was the whole point of the USER_STRIDE business. There are two
>>> alternatives I can see:
>>>
>>> 1. Change minigbm so that it always maps the entire texture regardless of
>>> what the caller requests. This is a very simple fix, but it has the
>>> downside
>>> that the driver will always blit the entire surface to staging even if
>>> only
>>> a single pixel is needed. That can obviously bite us for performance and
>>> power, which is why I didn't want to go down that route.
>>>
>>
>> The DRI backend maps the entire texture currently:
>>
>> https://chromium.googlesource.com/chromiumos/platform/minigb
>> m/+/master/dri.c#266
>>
>> This is consistent with what other drivers do as well, since GEM
>> ioctls (i.e, DRM_IOCTL_I915_GEM_MMAP_GTT) don't support partial
>> mappings.  I assume the AMD map ioctl also maps the entire buffer, but
>> the shadow buffer is smaller.
>>
>
> I assume that those other drivers don't have to copy pixels as part of
> their map / unmap, so for them mapping the entire texture is okay, since
> there is no performance/power difference either way.
>
> We do have to actually copy pixels during map / unmap, so mapping the
> entire texture is a no-go if we're not actually going to use it all.
>
>
> 2. Instead of having a USER_STRIDE interface, just simplify the story to
>>> saying that whenever a transfer allocates a staging texture, it should
>>> allocate a texture sized for the entire texture, but perform blits only
>>> for
>>> the region that the user has requested.
>>> This ends up being the same thing in the end, but perhaps it's better as
>>> an
>>> interface because it's more explicit about what's happening.
>>>
>>> By the way: the fact that gralloc wants to *cache* mappings will also
>>> cause
>>> us pain, but that's perhaps something we should really just change in
>>> gralloc.
>>>
>>
>> We modified minigbm to not cache mappings for AMD devices
>> (crrev.com/c/990892).
>>
>
> Yes, I see now, thanks for that.
>
> Cheers,
> Nicolai
>
>
>
>
>>
>>> Cheers,
>>> Nicolai
>>>
>>> --
>>> Lerne, wie die Welt wirklich ist,
>>> Aber vergiss niemals, wie sie sein sollte.
>>>
>>
>
> --
> Lerne, wie die Welt wirklich ist,
> Aber vergiss niemals, wie sie sein sollte.
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20180427/c056f1a0/attachment.html>