[Mesa-dev] [PATCH 4/4] i965: Use PTE MOCS for all external buffers

[Kenneth Graunke]

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 <kenneth at whitecape.org>
> Cc: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Daniel Vetter <daniel.vetter at ffwll.ch>
> Cc: Lyude Paul <lyude at 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 <kenneth at 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.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: This is a digitally signed message part.
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20171113/0190d19a/attachment-0001.sig>