On Monday, June 26, 2017 4:22:35 PM PDT Ian Romanick wrote:
> From: Jason Ekstrand <jason.ekstr...@intel.com>
>
> It's a bit rare, but blorp can trigger a urb reconfiguration. When that
> happens, we need to re-upload the URB config. Fortunately, this isn't as
> bad as it looks because gen7_upload_urb will not re-emit the packet if it
> would end up being a no-op so this doesn't mean that running blorp always
> triggers a URB reconfig.
>
> v2 (idr): Sort BRW_NEW_ tokens to match brw_recalculate_urb_fence and
> gen6_urb.
>
> v3 (idr): Don't whack BRW_NEW_URB_SIZE in blorp. Suggested by Jason.
> ---
> src/mesa/drivers/dri/i965/gen7_urb.c | 3 ++-
> src/mesa/drivers/dri/i965/genX_blorp_exec.c | 2 --
> 2 files changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/gen7_urb.c
> b/src/mesa/drivers/dri/i965/gen7_urb.c
> index 525c9c4..c4b479c 100644
> --- a/src/mesa/drivers/dri/i965/gen7_urb.c
> +++ b/src/mesa/drivers/dri/i965/gen7_urb.c
> @@ -236,7 +236,8 @@ gen7_upload_urb(struct brw_context *brw, unsigned vs_size,
> const struct brw_tracked_state gen7_urb = {
> .dirty = {
> .mesa = 0,
> - .brw = BRW_NEW_CONTEXT |
> + .brw = BRW_NEW_BLORP |
> + BRW_NEW_CONTEXT |
> BRW_NEW_URB_SIZE |
> BRW_NEW_GS_PROG_DATA |
> BRW_NEW_TCS_PROG_DATA |
> diff --git a/src/mesa/drivers/dri/i965/genX_blorp_exec.c
> b/src/mesa/drivers/dri/i965/genX_blorp_exec.c
> index 8fd17fb..af3d609 100644
> --- a/src/mesa/drivers/dri/i965/genX_blorp_exec.c
> +++ b/src/mesa/drivers/dri/i965/genX_blorp_exec.c
> @@ -183,8 +183,6 @@ blorp_emit_urb_config(struct blorp_batch *batch,
> brw->urb.vsize >= vs_entry_size)
> return;
>
> - brw->ctx.NewDriverState |= BRW_NEW_URB_SIZE;
> -
> gen7_upload_urb(brw, vs_entry_size, false, false);
> #elif GEN_GEN == 6
> gen6_upload_urb(brw, vs_entry_size, false, 0);
>
The commit message is a bit confusing. It makes it sound like we were
failing to re-emit 3DSTATE_URB_* from the main drawing path after BLORP
trashed it. However, the brw->ctx.NewDriverState |= BRW_NEW_URB_SIZE
line that you deleted here guaranteed that it would happen.
The real benefit appears to be that flagging BRW_NEW_URB_SIZE ("the
total size of the URB portion of the L3 cache has changed") causes
BLORP to think it needs to re-configure it again, so say, back-to-back
BLORP operations would configure it every time.
Using BRW_NEW_BLORP is definitely better - that's what it's there for.
Reviewed-by: Kenneth Graunke <kenn...@whitecape.org>
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
