On Tuesday, September 4, 2018 2:57:29 AM PDT Lionel Landwerlin wrote:
> Both brw_bo_map_cpu() & brw_bo_map_wc() assert if mapping the
> underlying BO fails. Failing back to brw_bo_map_gtt() doesn't seem to
> make any sense for that reason.
>
> We also only call brw_bo_map_gtt() for tiled buffers which as far as
> we know are never mapped coherent (coherent is a parameter reserved
> for buffer object which are always allocated linear). So explicitly
> assert if we ever run into this case.
>
> This makes checking the kernel about whether GTT maps are actually
> coherent unnecessary.
>
> Signed-off-by: Lionel Landwerlin <lionel.g.landwer...@intel.com>
> ---
> src/mesa/drivers/dri/i965/brw_bufmgr.c | 21 ++-------------------
> 1 file changed, 2 insertions(+), 19 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_bufmgr.c
> b/src/mesa/drivers/dri/i965/brw_bufmgr.c
> index f1675b191c1..e0008905164 100644
> --- a/src/mesa/drivers/dri/i965/brw_bufmgr.c
> +++ b/src/mesa/drivers/dri/i965/brw_bufmgr.c
> @@ -1095,6 +1095,8 @@ brw_bo_map_gtt(struct brw_context *brw, struct brw_bo
> *bo, unsigned flags)
> {
> struct brw_bufmgr *bufmgr = bo->bufmgr;
>
> + assert((flags & MAP_COHERENT) == 0);
> +
> /* Get a mapping of the buffer if we haven't before. */
> if (bo->map_gtt == NULL) {
> DBG("bo_map_gtt: mmap %d (%s)\n", bo->gem_handle, bo->name);
> @@ -1186,25 +1188,6 @@ brw_bo_map(struct brw_context *brw, struct brw_bo *bo,
> unsigned flags)
> else
> map = brw_bo_map_wc(brw, bo, flags);
>
> - /* Allow the attempt to fail by falling back to the GTT where necessary.
> - *
> - * Not every buffer can be mmaped directly using the CPU (or WC), for
> - * example buffers that wrap stolen memory or are imported from other
> - * devices. For those, we have little choice but to use a GTT mmapping.
> - * However, if we use a slow GTT mmapping for reads where we expected fast
> - * access, that order of magnitude difference in throughput will be
> clearly
> - * expressed by angry users.
> - *
> - * We skip MAP_RAW because we want to avoid map_gtt's fence detiling.
> - */
> - if (!map && !(flags & MAP_RAW)) {
> - if (brw) {
> - perf_debug("Fallback GTT mapping for %s with access flags %x\n",
> - bo->name, flags);
> - }
> - map = brw_bo_map_gtt(brw, bo, flags);
> - }
> -
> return map;
> }
>
> This will break on Cherryview Chromebook kernels, which are too old to support WC maps, and therefore fall back to GTT maps even for linear buffers. :( Which brings up an awful point...in that case...we will hit GTT maps with the MAP_COHERENT bit set. Assuming Cherryview has the buffering Chris mentioned, then we're hosed. It seems like our options are: 1. Mandate new kernels with WC support (not likely to fly...) 2. Disable ARB_buffer_storage/EXT_buffer_storage if the platform has this buffering and the kernel doesn't support WC maps 3. Just continue exposing the broken semi-coherent map, treat it as a known bug that would be fixed by upgrading the kernel... Chris, what platforms are affected by this? --Ken
signature.asc
Description: This is a digitally signed message part.
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
