On Friday, November 3, 2017 4:17:34 PM PST Jason Ekstrand wrote: > We were already using PTE for all render targets in case one happened to > get scanned out. However, this still wasn't 100% correct because there > are still possibly cases where we may want to texture from an external > buffer even though we don't know the caching mode. This can happen, for > instance, on buffers imported from another GPU via prime. > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101691 > Cc: Kenneth Graunke <kenn...@whitecape.org> > Cc: Chris Wilson <ch...@chris-wilson.co.uk> > Cc: Daniel Vetter <daniel.vet...@ffwll.ch> > Cc: Lyude Paul <ly...@redhat.com> > --- > src/mesa/drivers/dri/i965/brw_blorp.c | 7 ++++--- > src/mesa/drivers/dri/i965/brw_wm_surface_state.c | 20 +++++++++++++------- > 2 files changed, 17 insertions(+), 10 deletions(-) > > diff --git a/src/mesa/drivers/dri/i965/brw_blorp.c > b/src/mesa/drivers/dri/i965/brw_blorp.c > index 5a86af8..626bf44 100644 > --- a/src/mesa/drivers/dri/i965/brw_blorp.c > +++ b/src/mesa/drivers/dri/i965/brw_blorp.c > @@ -114,14 +114,14 @@ brw_blorp_init(struct brw_context *brw) > brw->blorp.upload_shader = brw_blorp_upload_shader; > } > > -static uint32_t tex_mocs[] = { > +static uint32_t wb_mocs[] = { > [7] = GEN7_MOCS_L3, > [8] = BDW_MOCS_WB, > [9] = SKL_MOCS_WB, > [10] = CNL_MOCS_WB, > }; > > -static uint32_t rb_mocs[] = { > +static uint32_t pte_mocs[] = { > [7] = GEN7_MOCS_L3, > [8] = BDW_MOCS_PTE, > [9] = SKL_MOCS_PTE, > @@ -158,7 +158,8 @@ blorp_surf_for_miptree(struct brw_context *brw, > .buffer = mt->bo, > .offset = mt->offset, > .reloc_flags = is_render_target ? EXEC_OBJECT_WRITE : 0, > - .mocs = is_render_target ? rb_mocs[devinfo->gen] : > tex_mocs[devinfo->gen], > + .mocs = (is_render_target || mt->bo->external) ? > pte_mocs[devinfo->gen] :
I think we should make this simply mt->bo->external, dropping the
is_render_target check. The whole is_render_target notion was a bad
approximation of "is scanned out?" which unfortunately applies to
internal FBOs too. bo->external is a much better approximation.
I would probably make that a patch 5 though, in case we get reports
that making it regresses performance by 1,000,000%...then we could
revert that separately from this important bug fix.
> + wb_mocs[devinfo->gen],
> };
>
> surf->aux_usage = aux_usage;
> diff --git a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> index 27c241a..f174270 100644
> --- a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> +++ b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> @@ -55,20 +55,25 @@
> #include "brw_defines.h"
> #include "brw_wm.h"
>
> -uint32_t tex_mocs[] = {
> +uint32_t wb_mocs[] = {
> [7] = GEN7_MOCS_L3,
> [8] = BDW_MOCS_WB,
> [9] = SKL_MOCS_WB,
> [10] = CNL_MOCS_WB,
> };
>
> -uint32_t rb_mocs[] = {
> +uint32_t pte_mocs[] = {
> [7] = GEN7_MOCS_L3,
> [8] = BDW_MOCS_PTE,
> [9] = SKL_MOCS_PTE,
> [10] = CNL_MOCS_PTE,
> };
>
> +static inline uint32_t get_tex_mocs(struct brw_bo *bo, unsigned int gen)
> +{
> + return (bo && bo->external ? pte_mocs : wb_mocs)[gen];
Need two more spaces here. I might just call it get_mocs().
Either way,
Reviewed-by: Kenneth Graunke <kenn...@whitecape.org>
It would also be nice to combine the various copies of these.
I would probably just make get_mocs() a static inline in brw_state.h.
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
