New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 764871 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Nov 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

implement flushing in minigbm

Project Member Reported by gurcheta...@chromium.org, Sep 13 2017

Issue description

Currently, minigbm unmaps when the (*unlock) gralloc hook is called.  This necessarily does not need to be the case.  As an optimization, we can implement a drv_bo_flush with the appropriate driver specific hooks (for drivers where this is not feasible, we can just unmap).
 

Comment 1 by tfiga@chromium.org, Sep 14 2017

> Currently, minigbm unmaps when the (*unlock) gralloc hook is called.

That's not entirely true. It unmaps when unlock is called for the last last lock, when the lock count drops to zero. If we need to flush, we probably need to do it on every unlock. (The case of nested locks probably isn't very common, though...)
Project Member

Comment 2 by bugdroid1@chromium.org, Sep 15 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/minigbm/+/7dcdff1f5ac08389c76fab9b91741f0fc1c1ae39

commit 7dcdff1f5ac08389c76fab9b91741f0fc1c1ae39
Author: Gurchetan Singh <gurchetansingh@chromium.org>
Date: Fri Sep 15 17:50:26 2017

minigbm: move tegra_bo_import to be above tegra_bo_map, and run clang-format

IMO this looks nicer.

BUG= chromium:764871 
TEST=gbmtest on Tegra

Change-Id: I7839b5a348f95f38385a555a8fe3ab0982d3533f
Reviewed-on: https://chromium-review.googlesource.com/668217
Commit-Ready: Gurchetan Singh <gurchetansingh@chromium.org>
Tested-by: Gurchetan Singh <gurchetansingh@chromium.org>
Reviewed-by: Gurchetan Singh <gurchetansingh@chromium.org>

[modify] https://crrev.com/7dcdff1f5ac08389c76fab9b91741f0fc1c1ae39/tegra.c
[modify] https://crrev.com/7dcdff1f5ac08389c76fab9b91741f0fc1c1ae39/i915.c

Project Member

Comment 3 by bugdroid1@chromium.org, Sep 27 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/minigbm/+/ba6bd503a8ac4da5b3f909ded71a3d60451fb372

commit ba6bd503a8ac4da5b3f909ded71a3d60451fb372
Author: Gurchetan Singh <gurchetansingh@chromium.org>
Date: Wed Sep 27 03:43:59 2017

minigbm: add a default (*bo_unmap) implementation

The (*bo_unmap) function is responsible for unmapping the buffer
and cleaning up any data allocated during (*bo_map). Previously,
we called munmap in drv_bo_unmap(), which caused some confusion.
This method is cleaner.

BUG= chromium:764871 
TEST=emerge-betty minigbm

Change-Id: I4dc20cd6b15e79bce21d33f03ebc84480c582981
Reviewed-on: https://chromium-review.googlesource.com/671693
Commit-Ready: Gurchetan Singh <gurchetansingh@chromium.org>
Tested-by: Gurchetan Singh <gurchetansingh@chromium.org>
Reviewed-by: Joe Kniss <djmk@google.com>
Reviewed-by: Tomasz Figa <tfiga@chromium.org>

[modify] https://crrev.com/ba6bd503a8ac4da5b3f909ded71a3d60451fb372/vgem.c
[modify] https://crrev.com/ba6bd503a8ac4da5b3f909ded71a3d60451fb372/evdi.c
[modify] https://crrev.com/ba6bd503a8ac4da5b3f909ded71a3d60451fb372/helpers.h
[modify] https://crrev.com/ba6bd503a8ac4da5b3f909ded71a3d60451fb372/exynos.c
[modify] https://crrev.com/ba6bd503a8ac4da5b3f909ded71a3d60451fb372/nouveau.c
[modify] https://crrev.com/ba6bd503a8ac4da5b3f909ded71a3d60451fb372/vc4.c
[modify] https://crrev.com/ba6bd503a8ac4da5b3f909ded71a3d60451fb372/udl.c
[modify] https://crrev.com/ba6bd503a8ac4da5b3f909ded71a3d60451fb372/helpers.c
[modify] https://crrev.com/ba6bd503a8ac4da5b3f909ded71a3d60451fb372/amdgpu.c
[modify] https://crrev.com/ba6bd503a8ac4da5b3f909ded71a3d60451fb372/gma500.c
[modify] https://crrev.com/ba6bd503a8ac4da5b3f909ded71a3d60451fb372/drv.c
[modify] https://crrev.com/ba6bd503a8ac4da5b3f909ded71a3d60451fb372/cirrus.c
[modify] https://crrev.com/ba6bd503a8ac4da5b3f909ded71a3d60451fb372/marvell.c
[modify] https://crrev.com/ba6bd503a8ac4da5b3f909ded71a3d60451fb372/radeon.c
[modify] https://crrev.com/ba6bd503a8ac4da5b3f909ded71a3d60451fb372/virtio_gpu.c

Project Member

Comment 4 by bugdroid1@chromium.org, Sep 27 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/minigbm/+/de263f108cb707924711a5c37a115fcbe06df2aa

commit de263f108cb707924711a5c37a115fcbe06df2aa
Author: Gurchetan Singh <gurchetansingh@chromium.org>
Date: Wed Sep 27 17:59:31 2017

minigbm: unmap right before buffer destruction

Regardless of the allocator, we want to make sure any mappings
we create are eventually freed. This patch adds logic to
drv_bo_destroy to free any mappings before the GEM close
ioctl is called.

BUG= chromium:764871 
TEST=gbmtest, mmap_test -g on eve

Change-Id: I8f4edb4bc01ff6e1d71a60e0309e77e9fc3840f4
Reviewed-on: https://chromium-review.googlesource.com/441916
Commit-Ready: Gurchetan Singh <gurchetansingh@chromium.org>
Tested-by: Gurchetan Singh <gurchetansingh@chromium.org>
Reviewed-by: Joe Kniss <djmk@google.com>
Reviewed-by: Stéphane Marchesin <marcheu@chromium.org>
Reviewed-by: Tomasz Figa <tfiga@chromium.org>

[modify] https://crrev.com/de263f108cb707924711a5c37a115fcbe06df2aa/drv.c
[modify] https://crrev.com/de263f108cb707924711a5c37a115fcbe06df2aa/helpers.h
[modify] https://crrev.com/de263f108cb707924711a5c37a115fcbe06df2aa/helpers.c

Project Member

Comment 5 by bugdroid1@chromium.org, Sep 28 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/minigbm/+/ff7414151e34a82fc290f79168553d01f5e654eb

commit ff7414151e34a82fc290f79168553d01f5e654eb
Author: Gurchetan Singh <gurchetansingh@chromium.org>
Date: Thu Sep 28 06:52:08 2017

minigbm: add drv_bo_flush

As an optimization, we would like to separate buffer flush/writeback
and unmap. This patch adds the appropriate entry point in our
internal driver API. Overall, the proposed flow is:

	- drv_bo_map(..) creates a mapping and associated metadata
	- drv_bo_flush(..) writes memory from any cache or temporary
          buffer back to memory
        - drv_bo_unmap(..) flushes, and frees the mapping and
	  associated metadata.

The drv_bo_flush function just does some sanity checks on
the map data at this point, but otherwise is a no-op.

BUG= chromium:764871 
TEST=gbmtest, mmap_test -g on eve

Change-Id: If306f066a76bc5e0943c1170e2d6933fa5630eff
Reviewed-on: https://chromium-review.googlesource.com/668218
Commit-Ready: Gurchetan Singh <gurchetansingh@chromium.org>
Tested-by: Gurchetan Singh <gurchetansingh@chromium.org>
Reviewed-by: Stéphane Marchesin <marcheu@chromium.org>

[modify] https://crrev.com/ff7414151e34a82fc290f79168553d01f5e654eb/drv.c
[modify] https://crrev.com/ff7414151e34a82fc290f79168553d01f5e654eb/drv_priv.h
[modify] https://crrev.com/ff7414151e34a82fc290f79168553d01f5e654eb/drv.h

Project Member

Comment 6 by bugdroid1@chromium.org, Sep 28 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/minigbm/+/8e02e05537eab60fc39e2c660d41e43be2ac522b

commit 8e02e05537eab60fc39e2c660d41e43be2ac522b
Author: Gurchetan Singh <gurchetansingh@chromium.org>
Date: Thu Sep 28 06:52:08 2017

minigbm: add (*bo_flush) implementations

Let's take the flush and writeback steps in the driver unmap
functions and move them to the (*bo_flush) callback.

BUG= chromium:764871 
TEST=gbmtest, mmap_test -g on eve

Change-Id: Iecccafc25642b120bc84f2d1b8538303c8ce36eb
Reviewed-on: https://chromium-review.googlesource.com/668219
Commit-Ready: Gurchetan Singh <gurchetansingh@chromium.org>
Tested-by: Gurchetan Singh <gurchetansingh@chromium.org>
Reviewed-by: Stéphane Marchesin <marcheu@chromium.org>

[modify] https://crrev.com/8e02e05537eab60fc39e2c660d41e43be2ac522b/tegra.c
[modify] https://crrev.com/8e02e05537eab60fc39e2c660d41e43be2ac522b/mediatek.c
[modify] https://crrev.com/8e02e05537eab60fc39e2c660d41e43be2ac522b/rockchip.c
[modify] https://crrev.com/8e02e05537eab60fc39e2c660d41e43be2ac522b/i915.c

Project Member

Comment 7 by bugdroid1@chromium.org, Sep 28 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/minigbm/+/254dbb19c49fad4ab4f28b2d1f2982bd10bb7c0b

commit 254dbb19c49fad4ab4f28b2d1f2982bd10bb7c0b
Author: Gurchetan Singh <gurchetansingh@chromium.org>
Date: Thu Sep 28 06:52:08 2017

minigbm: flush buffer instead of unmapping

As an optimization, let's call:
	- drv_bo_flush when calling (*unlock).
	- drv_bo_flush during gbm_bo_unmap()

CL:441916 makes sure that the buffer is unmapped before the
buffer destroy ioctl is called, so we will not be leaking
mappings in any case.

BUG= chromium:764871 
TEST=Android boots, 8 CTS tests, and Youtube app works on Eve
     gbmtest, mmap_test -g

Change-Id: I429739a8c6435a434dac41ad125761364a3775d0
Reviewed-on: https://chromium-review.googlesource.com/668220
Commit-Ready: Gurchetan Singh <gurchetansingh@chromium.org>
Tested-by: Gurchetan Singh <gurchetansingh@chromium.org>
Reviewed-by: Stéphane Marchesin <marcheu@chromium.org>

[modify] https://crrev.com/254dbb19c49fad4ab4f28b2d1f2982bd10bb7c0b/cros_gralloc/cros_gralloc_buffer.cc
[modify] https://crrev.com/254dbb19c49fad4ab4f28b2d1f2982bd10bb7c0b/gbm.c

Project Member

Comment 8 by bugdroid1@chromium.org, Oct 3 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/minigbm/+/a1892b2800a5847525d010d6245cbcc1776c1bea

commit a1892b2800a5847525d010d6245cbcc1776c1bea
Author: Gurchetan Singh <gurchetansingh@chromium.org>
Date: Tue Oct 03 00:27:17 2017

minigbm: standardize naming of buffer creation flags

We use the terms "flags" and "usage" interchangeably in this repo,
since they essentally mean the same thing.  However, let's be a
little more consistent since it's kind of confusing, especially
when buffer map flags come to play. Let's:

	- refer to everything in the drv_* layer as use_flags
	- refer to everything in the gbm/gralloc layers as
	  usages

BUG= chromium:764871 
TEST=emerge-eve {arc-cros-gralloc, minigbm}

Change-Id: If987d72369b895f38cde87e50ce1080f78f2a084
Reviewed-on: https://chromium-review.googlesource.com/691423
Commit-Ready: Gurchetan Singh <gurchetansingh@chromium.org>
Tested-by: Gurchetan Singh <gurchetansingh@chromium.org>
Reviewed-by: Stéphane Marchesin <marcheu@chromium.org>

[modify] https://crrev.com/a1892b2800a5847525d010d6245cbcc1776c1bea/cros_gralloc/cros_gralloc_handle.h
[modify] https://crrev.com/a1892b2800a5847525d010d6245cbcc1776c1bea/vgem.c
[modify] https://crrev.com/a1892b2800a5847525d010d6245cbcc1776c1bea/helpers.h
[modify] https://crrev.com/a1892b2800a5847525d010d6245cbcc1776c1bea/cros_gralloc/cros_gralloc_driver.cc
[modify] https://crrev.com/a1892b2800a5847525d010d6245cbcc1776c1bea/gbm.c
[modify] https://crrev.com/a1892b2800a5847525d010d6245cbcc1776c1bea/virtio_gpu.c
[modify] https://crrev.com/a1892b2800a5847525d010d6245cbcc1776c1bea/cros_gralloc/gralloc0/gralloc0.cc
[modify] https://crrev.com/a1892b2800a5847525d010d6245cbcc1776c1bea/vc4.c
[modify] https://crrev.com/a1892b2800a5847525d010d6245cbcc1776c1bea/cros_gralloc/cros_gralloc_types.h
[modify] https://crrev.com/a1892b2800a5847525d010d6245cbcc1776c1bea/mediatek.c
[modify] https://crrev.com/a1892b2800a5847525d010d6245cbcc1776c1bea/rockchip.c
[modify] https://crrev.com/a1892b2800a5847525d010d6245cbcc1776c1bea/helpers.c
[modify] https://crrev.com/a1892b2800a5847525d010d6245cbcc1776c1bea/drv_priv.h
[modify] https://crrev.com/a1892b2800a5847525d010d6245cbcc1776c1bea/amdgpu.c
[modify] https://crrev.com/a1892b2800a5847525d010d6245cbcc1776c1bea/gbm_helpers.c
[modify] https://crrev.com/a1892b2800a5847525d010d6245cbcc1776c1bea/drv.h
[modify] https://crrev.com/a1892b2800a5847525d010d6245cbcc1776c1bea/tegra.c
[modify] https://crrev.com/a1892b2800a5847525d010d6245cbcc1776c1bea/drv.c
[modify] https://crrev.com/a1892b2800a5847525d010d6245cbcc1776c1bea/gbm_helpers.h
[modify] https://crrev.com/a1892b2800a5847525d010d6245cbcc1776c1bea/exynos.c
[modify] https://crrev.com/a1892b2800a5847525d010d6245cbcc1776c1bea/i915.c

Project Member

Comment 9 by bugdroid1@chromium.org, Oct 3 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/minigbm/+/f7f633aca589e0feb838773fc1afb85260747913

commit f7f633aca589e0feb838773fc1afb85260747913
Author: Gurchetan Singh <gurchetansingh@chromium.org>
Date: Tue Oct 03 03:25:45 2017

minigbm: standardize naming of map flags

It's helpful to differentiate map flags from normal buffer creation
flags. Note gralloc doesn't differentiate between map flags and buffer
creation flags. However, since flags are passed in with gralloc
(*lock)(), we can use a separate conversion function there.

BUG= chromium:764871 
TEST=Boot Android and play games on Eve

Change-Id: Ic8aee84d9ac945abf93d9a9bda78fe3f77711cc3
Reviewed-on: https://chromium-review.googlesource.com/691424
Commit-Ready: Gurchetan Singh <gurchetansingh@chromium.org>
Tested-by: Gurchetan Singh <gurchetansingh@chromium.org>
Reviewed-by: Stéphane Marchesin <marcheu@chromium.org>

[modify] https://crrev.com/f7f633aca589e0feb838773fc1afb85260747913/cros_gralloc/cros_gralloc_buffer.h
[modify] https://crrev.com/f7f633aca589e0feb838773fc1afb85260747913/cros_gralloc/cros_gralloc_driver.cc
[modify] https://crrev.com/f7f633aca589e0feb838773fc1afb85260747913/gbm.c
[modify] https://crrev.com/f7f633aca589e0feb838773fc1afb85260747913/cros_gralloc/cros_gralloc_driver.h
[modify] https://crrev.com/f7f633aca589e0feb838773fc1afb85260747913/cros_gralloc/gralloc0/gralloc0.cc
[modify] https://crrev.com/f7f633aca589e0feb838773fc1afb85260747913/drv.h
[modify] https://crrev.com/f7f633aca589e0feb838773fc1afb85260747913/drv.c
[modify] https://crrev.com/f7f633aca589e0feb838773fc1afb85260747913/cros_gralloc/cros_gralloc_buffer.cc

Project Member

Comment 10 by bugdroid1@chromium.org, Oct 3 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/minigbm/+/cfb88767557632701252c1545d3c17905c6c0f83

commit cfb88767557632701252c1545d3c17905c6c0f83
Author: Gurchetan Singh <gurchetansingh@chromium.org>
Date: Tue Oct 03 06:28:25 2017

minigbm: pass in map flags to (*bo_map) callback

Some eagle-eyed observers have commented that we lose some
potentially useful information for the (*bo_map) callback when
we convert our map flags to page protection flags. Let's not
lose this information, so pass in the map flags. We can convert
to page protection flags using a helper function.

BUG= chromium:764871 
TEST=Boot Android and play games on Eve

Change-Id: Ie2cf395109eb5a8de663dfb97a343d20ff0df30c
Reviewed-on: https://chromium-review.googlesource.com/691425
Commit-Ready: Gurchetan Singh <gurchetansingh@chromium.org>
Tested-by: Gurchetan Singh <gurchetansingh@chromium.org>
Reviewed-by: Stéphane Marchesin <marcheu@chromium.org>

[modify] https://crrev.com/cfb88767557632701252c1545d3c17905c6c0f83/helpers.h
[modify] https://crrev.com/cfb88767557632701252c1545d3c17905c6c0f83/tegra.c
[modify] https://crrev.com/cfb88767557632701252c1545d3c17905c6c0f83/vc4.c
[modify] https://crrev.com/cfb88767557632701252c1545d3c17905c6c0f83/rockchip.c
[modify] https://crrev.com/cfb88767557632701252c1545d3c17905c6c0f83/helpers.c
[modify] https://crrev.com/cfb88767557632701252c1545d3c17905c6c0f83/drv_priv.h
[modify] https://crrev.com/cfb88767557632701252c1545d3c17905c6c0f83/amdgpu.c
[modify] https://crrev.com/cfb88767557632701252c1545d3c17905c6c0f83/drv.c
[modify] https://crrev.com/cfb88767557632701252c1545d3c17905c6c0f83/mediatek.c
[modify] https://crrev.com/cfb88767557632701252c1545d3c17905c6c0f83/i915.c

Project Member

Comment 11 by bugdroid1@chromium.org, Oct 18 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/minigbm/+/c2ad63e7a5c60673cea7d96d14f7151430860e16

commit c2ad63e7a5c60673cea7d96d14f7151430860e16
Author: Gurchetan Singh <gurchetansingh@chromium.org>
Date: Wed Oct 18 06:14:19 2017

minigbm: add buffer invalidation function

We may need to invalidate a buffer before reading it. Some use
cases are:

	- DRM_IOCTL_I915_GEM_SET_DOMAIN
        - DMA_BUF_IOCTL_SYNC with the SYNC_START option
        - DRM_IOCTL_VIRTGPU_TRANSFER_FROM_HOST

This patch adds the function hook.

BUG= chromium:764871 
TEST=compiles

Change-Id: I85811407252b859a12294381c65ff3545424636b
Reviewed-on: https://chromium-review.googlesource.com/710322
Commit-Ready: Gurchetan Singh <gurchetansingh@chromium.org>
Tested-by: Gurchetan Singh <gurchetansingh@chromium.org>
Reviewed-by: Stéphane Marchesin <marcheu@chromium.org>

[modify] https://crrev.com/c2ad63e7a5c60673cea7d96d14f7151430860e16/drv.c
[modify] https://crrev.com/c2ad63e7a5c60673cea7d96d14f7151430860e16/drv_priv.h
[modify] https://crrev.com/c2ad63e7a5c60673cea7d96d14f7151430860e16/drv.h
[modify] https://crrev.com/c2ad63e7a5c60673cea7d96d14f7151430860e16/cros_gralloc/cros_gralloc_buffer.cc

Project Member

Comment 12 by bugdroid1@chromium.org, Oct 18 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/minigbm/+/2d1877f0417e5891ad6e2795db7cf395916b15b3

commit 2d1877f0417e5891ad6e2795db7cf395916b15b3
Author: Gurchetan Singh <gurchetansingh@chromium.org>
Date: Wed Oct 18 19:12:42 2017

minigbm: i915: call DRM_IOCTL_I915_GEM_SET_DOMAIN when invalidating

In the kernel, the i915_gem_set_domain_ioctl function calls these
functions:

	- i915_gem_object_set_to_gtt_domain
	- i915_gem_object_set_to_cpu_domain

These functions do various interesting things with caches, and some
CTS tests require this to pass.

BUG=b:67073097, b:67331142,  chromium:764871 
TEST=The following tests:

android.view.cts.SurfaceViewSyncTests
android.video.cts.VideoEncoderDecoderTest#testAvcGoog0Qual0720x0480
android.video.cts.VideoEncoderDecoderTest#testAvcGoog0Qual1280x0720
android.video.cts.VideoEncoderDecoderTest#testAvcGoog0Qual1920x1080
android.media.cts.EncodeDecodeTest#testVP8EncodeDecodeVideoFromSurfaceToSurface720p
android.media.cts.EncodeDecodeTest#testEncodeDecodeVideoFromPersistentSurfaceToSurface720p
android.media.cts.EncodeDecodeTest#testVP8EncodeDecodeVideoFromPersistentSurfaceToSurface720p

passes on Eve with the next patch applied.

Change-Id: I9acc14580f65eab6039d8b354bfbf51c31dfcf14
Reviewed-on: https://chromium-review.googlesource.com/710323
Commit-Ready: Gurchetan Singh <gurchetansingh@chromium.org>
Tested-by: Gurchetan Singh <gurchetansingh@chromium.org>
Reviewed-by: Stéphane Marchesin <marcheu@chromium.org>

[modify] https://crrev.com/2d1877f0417e5891ad6e2795db7cf395916b15b3/i915.c

Project Member

Comment 13 by bugdroid1@chromium.org, Nov 8 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/minigbm/+/425173a0c8170528f804b2a9bd02b71fa47d754b

commit 425173a0c8170528f804b2a9bd02b71fa47d754b
Author: Gurchetan Singh <gurchetansingh@chromium.org>
Date: Wed Nov 08 23:10:10 2017

minigbm: fix map time assertions

We should assert when the refcount is equal to zero as well.

BUG= chromium:764871 
TEST=gbmtest passes

Change-Id: Iaf8b5bd4bf51472ad7c564341b42a7079b58bd6e
Reviewed-on: https://chromium-review.googlesource.com/758143
Commit-Ready: Gurchetan Singh <gurchetansingh@chromium.org>
Tested-by: Gurchetan Singh <gurchetansingh@chromium.org>
Reviewed-by: Stéphane Marchesin <marcheu@chromium.org>

[modify] https://crrev.com/425173a0c8170528f804b2a9bd02b71fa47d754b/drv.c

Project Member

Comment 14 by bugdroid1@chromium.org, Nov 9 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/minigbm/+/47e629b70ddec014c7691a48509ef437c57761bb

commit 47e629b70ddec014c7691a48509ef437c57761bb
Author: Gurchetan Singh <gurchetansingh@chromium.org>
Date: Thu Nov 09 06:21:56 2017

minigbm: refactor and rename mapping struct

Since some drivers (AMDGPU, Tegra) may have to do expensive tiling
and detiling operations, we should try to take advantage of the
access regions passed in by gralloc and gbm. Let's refactor struct
map_data so we can separate the actual mapping and access region.

Here is the Coccinelle rule used in this change:

@@ struct map_info *M; @@
-   (M)
+   M->vma

In addition, struct map_data was also renamed to struct mapping.

BUG= chromium:764871 
TEST= mmap_test -g on Kevin

Change-Id: Idb094aa3b5f81e45ce3a2f4fb2d9bf8fba32bf29
Reviewed-on: https://chromium-review.googlesource.com/758144
Commit-Ready: Gurchetan Singh <gurchetansingh@chromium.org>
Tested-by: Gurchetan Singh <gurchetansingh@chromium.org>
Reviewed-by: Joe Kniss <djmk@google.com>
Reviewed-by: Stéphane Marchesin <marcheu@chromium.org>

[modify] https://crrev.com/47e629b70ddec014c7691a48509ef437c57761bb/cros_gralloc/cros_gralloc_buffer.h
[modify] https://crrev.com/47e629b70ddec014c7691a48509ef437c57761bb/gbm.c
[modify] https://crrev.com/47e629b70ddec014c7691a48509ef437c57761bb/tegra.c
[modify] https://crrev.com/47e629b70ddec014c7691a48509ef437c57761bb/vc4.c
[modify] https://crrev.com/47e629b70ddec014c7691a48509ef437c57761bb/mediatek.c
[modify] https://crrev.com/47e629b70ddec014c7691a48509ef437c57761bb/rockchip.c
[modify] https://crrev.com/47e629b70ddec014c7691a48509ef437c57761bb/helpers.c
[modify] https://crrev.com/47e629b70ddec014c7691a48509ef437c57761bb/drv_priv.h
[modify] https://crrev.com/47e629b70ddec014c7691a48509ef437c57761bb/amdgpu.c
[modify] https://crrev.com/47e629b70ddec014c7691a48509ef437c57761bb/drv.h
[modify] https://crrev.com/47e629b70ddec014c7691a48509ef437c57761bb/drv.c
[modify] https://crrev.com/47e629b70ddec014c7691a48509ef437c57761bb/helpers.h
[modify] https://crrev.com/47e629b70ddec014c7691a48509ef437c57761bb/cros_gralloc/cros_gralloc_buffer.cc
[modify] https://crrev.com/47e629b70ddec014c7691a48509ef437c57761bb/i915.c

Project Member

Comment 15 by bugdroid1@chromium.org, Nov 9 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/minigbm/+/39f66c0cf95dc64c263feb91b493c49acdbd6c71

commit 39f66c0cf95dc64c263feb91b493c49acdbd6c71
Author: Gurchetan Singh <gurchetansingh@chromium.org>
Date: Thu Nov 09 08:38:12 2017

minigbm: introduce drv_array

We want to allow multiple mappings of a single buffer with different
map flags. To do this, we'll need some sort of linked list or dynamic
array of mappings. We already use dynamic arrays in minigbm, so let's
centralize that functionality and use it.

BUG= chromium:764871 
TEST=emerge-kevin {minigbm, arc-cros-gralloc}

Change-Id: I2b391d6e8e690eac6368b1404a806b2bfbbcdf44
Reviewed-on: https://chromium-review.googlesource.com/758145
Commit-Ready: Gurchetan Singh <gurchetansingh@chromium.org>
Tested-by: Gurchetan Singh <gurchetansingh@chromium.org>
Reviewed-by: Stéphane Marchesin <marcheu@chromium.org>

[add] https://crrev.com/39f66c0cf95dc64c263feb91b493c49acdbd6c71/helpers_array.h
[add] https://crrev.com/39f66c0cf95dc64c263feb91b493c49acdbd6c71/helpers_array.c
[modify] https://crrev.com/39f66c0cf95dc64c263feb91b493c49acdbd6c71/helpers.h

Project Member

Comment 16 by bugdroid1@chromium.org, Nov 14 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/minigbm/+/cfedbcc8d475fca39ac0a0d4bc6e4881937e2875

commit cfedbcc8d475fca39ac0a0d4bc6e4881937e2875
Author: Gurchetan Singh <gurchetansingh@chromium.org>
Date: Tue Nov 14 11:22:34 2017

minigbm: use drv_array for mappings

Let's start allowing multiple mappings of the same buffer when
different map flags are passed in.

BUG= chromium:764871 
TEST=mmap_test -g on Kevin, gbmtest

Change-Id: I4eb0b6f4c3572a92001696c2720d5a5f7d9d73a4
Reviewed-on: https://chromium-review.googlesource.com/758146
Commit-Ready: Gurchetan Singh <gurchetansingh@chromium.org>
Tested-by: Gurchetan Singh <gurchetansingh@chromium.org>
Reviewed-by: Joe Kniss <djmk@google.com>
Reviewed-by: Stéphane Marchesin <marcheu@chromium.org>

[modify] https://crrev.com/cfedbcc8d475fca39ac0a0d4bc6e4881937e2875/drv.c
[modify] https://crrev.com/cfedbcc8d475fca39ac0a0d4bc6e4881937e2875/drv_priv.h
[modify] https://crrev.com/cfedbcc8d475fca39ac0a0d4bc6e4881937e2875/helpers.c

Project Member

Comment 17 by bugdroid1@chromium.org, Nov 16 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/minigbm/+/ee43c301c21976fb82538261c4e4288ffc754777

commit ee43c301c21976fb82538261c4e4288ffc754777
Author: Gurchetan Singh <gurchetansingh@chromium.org>
Date: Thu Nov 16 18:46:57 2017

minigbm: use struct vma for (*bo_map)/(*bo_unmap) callbacks

This sets better expectations for what we expect from the
backends.

BUG= chromium:764871 
TEST=mmap_test

Change-Id: I7fb815b58fae8e9fbd73bf7c0263c7db44488844
Reviewed-on: https://chromium-review.googlesource.com/770519
Commit-Ready: Gurchetan Singh <gurchetansingh@chromium.org>
Tested-by: Gurchetan Singh <gurchetansingh@chromium.org>
Reviewed-by: Joe Kniss <djmk@google.com>
Reviewed-by: Stéphane Marchesin <marcheu@chromium.org>

[modify] https://crrev.com/ee43c301c21976fb82538261c4e4288ffc754777/helpers.h
[modify] https://crrev.com/ee43c301c21976fb82538261c4e4288ffc754777/tegra.c
[modify] https://crrev.com/ee43c301c21976fb82538261c4e4288ffc754777/vc4.c
[modify] https://crrev.com/ee43c301c21976fb82538261c4e4288ffc754777/rockchip.c
[modify] https://crrev.com/ee43c301c21976fb82538261c4e4288ffc754777/helpers.c
[modify] https://crrev.com/ee43c301c21976fb82538261c4e4288ffc754777/drv_priv.h
[modify] https://crrev.com/ee43c301c21976fb82538261c4e4288ffc754777/amdgpu.c
[modify] https://crrev.com/ee43c301c21976fb82538261c4e4288ffc754777/drv.c
[modify] https://crrev.com/ee43c301c21976fb82538261c4e4288ffc754777/mediatek.c
[modify] https://crrev.com/ee43c301c21976fb82538261c4e4288ffc754777/i915.c

Project Member

Comment 18 by bugdroid1@chromium.org, Nov 16 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/minigbm/+/1ef809ecd434bfc0fd1d669ba925d58b1255d163

commit 1ef809ecd434bfc0fd1d669ba925d58b1255d163
Author: Gurchetan Singh <gurchetansingh@chromium.org>
Date: Thu Nov 16 21:52:02 2017

minigbm: plumb buffer access region

This will allow drivers to tile or detile only the regions requested
by the user. Note that the gralloc spec states that:

"This address will represent the top-left corner of the entire buffer,
even if accessRegion does not begin at the top-left corner."

(see hardware/interfaces/graphics/mapper/2.0/IMapper.hal in AOSP)

Also, the gralloc API makes it difficult to maintain two mappings of
the same buffer.  For example, say you have two access regions:

module->lock(mod, handle1, 0, 0, 5, 5, &addr);
module->lock(mod, handle1, 5, 5, 10, 10, &addr);

module->unlock(mod, handle1); // which access region should be unlocked?

In practice, this scenario never happens on Android.

It's not exactly clear what gbm should return.  Let's just return the
top left of the access region because that's what we where doing before.

BUG= chromium:764871 
TEST=gbmtest, mmap_test -g, the following CTS tests:

android.view.cts.SurfaceViewSyncTests
android.media.cts.EncodeDecodeTest
android.video.cts.VideoEncoderDecoderTest

Change-Id: I7ca0713871e03928b1d4402aa161588990c7e775
Reviewed-on: https://chromium-review.googlesource.com/758147
Commit-Ready: Gurchetan Singh <gurchetansingh@chromium.org>
Tested-by: Gurchetan Singh <gurchetansingh@chromium.org>
Reviewed-by: Stéphane Marchesin <marcheu@chromium.org>

[modify] https://crrev.com/1ef809ecd434bfc0fd1d669ba925d58b1255d163/cros_gralloc/cros_gralloc_buffer.h
[modify] https://crrev.com/1ef809ecd434bfc0fd1d669ba925d58b1255d163/cros_gralloc/cros_gralloc_driver.cc
[modify] https://crrev.com/1ef809ecd434bfc0fd1d669ba925d58b1255d163/gbm.c
[modify] https://crrev.com/1ef809ecd434bfc0fd1d669ba925d58b1255d163/cros_gralloc/cros_gralloc_driver.h
[modify] https://crrev.com/1ef809ecd434bfc0fd1d669ba925d58b1255d163/cros_gralloc/gralloc0/gralloc0.cc
[modify] https://crrev.com/1ef809ecd434bfc0fd1d669ba925d58b1255d163/drv.h
[modify] https://crrev.com/1ef809ecd434bfc0fd1d669ba925d58b1255d163/drv.c
[modify] https://crrev.com/1ef809ecd434bfc0fd1d669ba925d58b1255d163/helpers.h
[modify] https://crrev.com/1ef809ecd434bfc0fd1d669ba925d58b1255d163/cros_gralloc/cros_gralloc_buffer.cc

Project Member

Comment 19 by bugdroid1@chromium.org, Nov 16 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/minigbm/+/d300145b093cf67277648a664d3748b881d4a315

commit d300145b093cf67277648a664d3748b881d4a315
Author: Gurchetan Singh <gurchetansingh@chromium.org>
Date: Thu Nov 16 21:52:02 2017

minigbm: make drv_add_combinations return nothing

Two reasons for this:

	1) We can use drv_array for driver combinations (see next patch)
	2) It's less verbose.

BUG= chromium:764871 
TEST=gbmtest passes

Change-Id: I39bea6e2e9e4c76d2ca78566926a79bdc17f11d0
Reviewed-on: https://chromium-review.googlesource.com/758148
Commit-Ready: Gurchetan Singh <gurchetansingh@chromium.org>
Tested-by: Gurchetan Singh <gurchetansingh@chromium.org>
Reviewed-by: Stéphane Marchesin <marcheu@chromium.org>

[modify] https://crrev.com/d300145b093cf67277648a664d3748b881d4a315/vgem.c
[modify] https://crrev.com/d300145b093cf67277648a664d3748b881d4a315/evdi.c
[modify] https://crrev.com/d300145b093cf67277648a664d3748b881d4a315/helpers.h
[modify] https://crrev.com/d300145b093cf67277648a664d3748b881d4a315/virtio_gpu.c
[modify] https://crrev.com/d300145b093cf67277648a664d3748b881d4a315/exynos.c
[modify] https://crrev.com/d300145b093cf67277648a664d3748b881d4a315/vc4.c
[modify] https://crrev.com/d300145b093cf67277648a664d3748b881d4a315/nouveau.c
[modify] https://crrev.com/d300145b093cf67277648a664d3748b881d4a315/mediatek.c
[modify] https://crrev.com/d300145b093cf67277648a664d3748b881d4a315/udl.c
[modify] https://crrev.com/d300145b093cf67277648a664d3748b881d4a315/rockchip.c
[modify] https://crrev.com/d300145b093cf67277648a664d3748b881d4a315/helpers.c
[modify] https://crrev.com/d300145b093cf67277648a664d3748b881d4a315/amdgpu.c
[modify] https://crrev.com/d300145b093cf67277648a664d3748b881d4a315/tegra.c
[modify] https://crrev.com/d300145b093cf67277648a664d3748b881d4a315/gma500.c
[modify] https://crrev.com/d300145b093cf67277648a664d3748b881d4a315/radeon.c
[modify] https://crrev.com/d300145b093cf67277648a664d3748b881d4a315/marvell.c
[modify] https://crrev.com/d300145b093cf67277648a664d3748b881d4a315/i915.c

Project Member

Comment 20 by bugdroid1@chromium.org, Nov 16 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/minigbm/+/bc9a87d54c5296d467ead2b4ea35e317f00ee401

commit bc9a87d54c5296d467ead2b4ea35e317f00ee401
Author: Gurchetan Singh <gurchetansingh@chromium.org>
Date: Thu Nov 16 21:52:02 2017

minigbm: use drv_array for combinations and kms_items

This de-deuplicates the various dynamic arrays we use.

BUG= chromium:764871 
TEST=gbmtest passes

Change-Id: I94c8cf7c71fdb98b931aab00c5381853e2ae0d3f
Reviewed-on: https://chromium-review.googlesource.com/758149
Commit-Ready: Gurchetan Singh <gurchetansingh@chromium.org>
Tested-by: Gurchetan Singh <gurchetansingh@chromium.org>
Reviewed-by: Stéphane Marchesin <marcheu@chromium.org>

[modify] https://crrev.com/bc9a87d54c5296d467ead2b4ea35e317f00ee401/helpers.h
[modify] https://crrev.com/bc9a87d54c5296d467ead2b4ea35e317f00ee401/rockchip.c
[modify] https://crrev.com/bc9a87d54c5296d467ead2b4ea35e317f00ee401/helpers.c
[modify] https://crrev.com/bc9a87d54c5296d467ead2b4ea35e317f00ee401/drv_priv.h
[modify] https://crrev.com/bc9a87d54c5296d467ead2b4ea35e317f00ee401/drv.c
[modify] https://crrev.com/bc9a87d54c5296d467ead2b4ea35e317f00ee401/i915.c

Project Member

Comment 21 by bugdroid1@chromium.org, Nov 18 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/minigbm/+/ca2938a5c681e42a344f7237430726aa17ff1126

commit ca2938a5c681e42a344f7237430726aa17ff1126
Author: Tomasz Mikolajewski <mtomasz@google.com>
Date: Sat Nov 18 04:18:45 2017

Add a missing include for asserts.

The missing include would cause image builders fail on
-arcnext builds.

BUG= chromium:764871 
TEST=emerge arc-cros-gralloc and minigbm on -arcnext.

Change-Id: Ifde30bcde72a1fae4d13c95b17273880512cafe2
Reviewed-on: https://chromium-review.googlesource.com/776640
Commit-Ready: Gurchetan Singh <gurchetansingh@chromium.org>
Tested-by: Gurchetan Singh <gurchetansingh@chromium.org>
Reviewed-by: Gurchetan Singh <gurchetansingh@chromium.org>

[modify] https://crrev.com/ca2938a5c681e42a344f7237430726aa17ff1126/cros_gralloc/gralloc0/gralloc0.cc

Status: Fixed (was: Assigned)

Sign in to add a comment