Quoting Kenneth Graunke (2018-09-04 16:13:59)
> 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?
All Baytrail+ atoms (which were the first we found it on) and it seems
all next generation GGTT, Canonlake+.
Option 3. The window of error is roughly the time it takes to do an
uncached read, so the likelihood of two clients observing different
values for the same location in memory is small (but non-zero), but
honestly I expect it to be hidden behind inter-client synchronisation
(or else they would be expecting stale data and don't care). The
biggest risk (imo) would be something like using a batchbuffer where you
want to modify commands after submission. (Who would do such a thing!
It's bitten me more times than I'd care to admit ;) I can imagine a few
graphical glitches, if you tried to stream textures while also
processing them on the GPU.
So for option 3, make the perf_debug() recommend a newer kernel to avoid
taking the GTT path? I would like to keep the warning comment around so
that as soon as we can, we can realise that assert.
-Chris
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
