Re: [Mesa-dev] [PATCH 1/6] gallium: plumb context priority through to driver

Mon, 09 Oct 2017 18:42:08 -0700 


On 2017-10-05 11:29 AM, Nicolai Hähnle wrote:

On 04.10.2017 23:02, Rob Clark wrote:

On Wed, Oct 4, 2017 at 3:33 PM, Roland Scheidegger 
<srol...@vmware.com> wrote:

Am 04.10.2017 um 17:44 schrieb Rob Clark:

Signed-off-by: Rob Clark <robdcl...@gmail.com>
---
  src/gallium/drivers/etnaviv/etnaviv_screen.c        |  1 +
  src/gallium/drivers/freedreno/freedreno_screen.c    |  1 +
  src/gallium/drivers/i915/i915_screen.c              |  1 +
  src/gallium/drivers/llvmpipe/lp_screen.c            |  1 +
  src/gallium/drivers/nouveau/nv30/nv30_screen.c      |  1 +
  src/gallium/drivers/nouveau/nv50/nv50_screen.c      |  1 +
  src/gallium/drivers/nouveau/nvc0/nvc0_screen.c      |  1 +
  src/gallium/drivers/r300/r300_screen.c              |  1 +
  src/gallium/drivers/r600/r600_pipe.c                |  1 +
  src/gallium/drivers/radeonsi/si_pipe.c              |  1 +
  src/gallium/drivers/softpipe/sp_screen.c            |  1 +
  src/gallium/drivers/svga/svga_screen.c              |  1 +
  src/gallium/drivers/swr/swr_screen.cpp              |  1 +
  src/gallium/drivers/vc4/vc4_screen.c                |  1 +
  src/gallium/drivers/virgl/virgl_screen.c            |  1 +

  src/gallium/include/pipe/p_defines.h                | 21 
+++++++++++++++++++++

  src/gallium/include/state_tracker/st_api.h          |  2 ++
  src/gallium/state_trackers/dri/dri_context.c        | 11 +++++++++++
  src/gallium/state_trackers/dri/dri_query_renderer.c |  8 +++++++-
  src/mesa/state_tracker/st_manager.c                 |  5 +++++
  20 files changed, 61 insertions(+), 1 deletion(-)


diff --git a/src/gallium/drivers/etnaviv/etnaviv_screen.c 
b/src/gallium/drivers/etnaviv/etnaviv_screen.c

index 42905ab0620..16bd4b7c0fb 100644
--- a/src/gallium/drivers/etnaviv/etnaviv_screen.c
+++ b/src/gallium/drivers/etnaviv/etnaviv_screen.c

@@ -264,6 +264,7 @@ etna_screen_get_param(struct pipe_screen 
*pscreen, enum pipe_cap param)

     case PIPE_CAP_QUERY_SO_OVERFLOW:
     case PIPE_CAP_MEMOBJ:
     case PIPE_CAP_LOAD_CONSTBUF:
+   case PIPE_CAP_CONTEXT_PRIORITY_MASK:
        return 0;

     /* Stream output. */

diff --git a/src/gallium/drivers/freedreno/freedreno_screen.c 
b/src/gallium/drivers/freedreno/freedreno_screen.c

index 040c2c99ec0..96866d656be 100644
--- a/src/gallium/drivers/freedreno/freedreno_screen.c
+++ b/src/gallium/drivers/freedreno/freedreno_screen.c

@@ -325,6 +325,7 @@ fd_screen_get_param(struct pipe_screen *pscreen, 
enum pipe_cap param)

       case PIPE_CAP_QUERY_SO_OVERFLOW:
       case PIPE_CAP_MEMOBJ:
       case PIPE_CAP_LOAD_CONSTBUF:
+     case PIPE_CAP_CONTEXT_PRIORITY_MASK:
               return 0;

       case PIPE_CAP_MAX_VIEWPORTS:

diff --git a/src/gallium/drivers/i915/i915_screen.c 
b/src/gallium/drivers/i915/i915_screen.c

index 8411c0f15cc..7bcf479c4be 100644
--- a/src/gallium/drivers/i915/i915_screen.c
+++ b/src/gallium/drivers/i915/i915_screen.c

@@ -317,6 +317,7 @@ i915_get_param(struct pipe_screen *screen, enum 
pipe_cap cap)

     case PIPE_CAP_QUERY_SO_OVERFLOW:
     case PIPE_CAP_MEMOBJ:
     case PIPE_CAP_LOAD_CONSTBUF:
+   case PIPE_CAP_CONTEXT_PRIORITY_MASK:
        return 0;

     case PIPE_CAP_MAX_VIEWPORTS:

diff --git a/src/gallium/drivers/llvmpipe/lp_screen.c 
b/src/gallium/drivers/llvmpipe/lp_screen.c

index 53171162a54..19411adaf07 100644
--- a/src/gallium/drivers/llvmpipe/lp_screen.c
+++ b/src/gallium/drivers/llvmpipe/lp_screen.c

@@ -360,6 +360,7 @@ llvmpipe_get_param(struct pipe_screen *screen, 
enum pipe_cap param)

     case PIPE_CAP_NIR_SAMPLERS_AS_DEREF:
     case PIPE_CAP_MEMOBJ:
     case PIPE_CAP_LOAD_CONSTBUF:
+   case PIPE_CAP_CONTEXT_PRIORITY_MASK:
        return 0;
     }
     /* should only get here on unhandled cases */

diff --git a/src/gallium/drivers/nouveau/nv30/nv30_screen.c 
b/src/gallium/drivers/nouveau/nv30/nv30_screen.c

index a66b4fbe67b..782ba0a64db 100644
--- a/src/gallium/drivers/nouveau/nv30/nv30_screen.c
+++ b/src/gallium/drivers/nouveau/nv30/nv30_screen.c

@@ -224,6 +224,7 @@ nv30_screen_get_param(struct pipe_screen 
*pscreen, enum pipe_cap param)

     case PIPE_CAP_QUERY_SO_OVERFLOW:
     case PIPE_CAP_MEMOBJ:
     case PIPE_CAP_LOAD_CONSTBUF:
+   case PIPE_CAP_CONTEXT_PRIORITY_MASK:
        return 0;

     case PIPE_CAP_VENDOR_ID:

diff --git a/src/gallium/drivers/nouveau/nv50/nv50_screen.c 
b/src/gallium/drivers/nouveau/nv50/nv50_screen.c

index 479283e1b7c..997cb4e71dc 100644
--- a/src/gallium/drivers/nouveau/nv50/nv50_screen.c
+++ b/src/gallium/drivers/nouveau/nv50/nv50_screen.c

@@ -276,6 +276,7 @@ nv50_screen_get_param(struct pipe_screen 
*pscreen, enum pipe_cap param)

     case PIPE_CAP_QUERY_SO_OVERFLOW:
     case PIPE_CAP_MEMOBJ:
     case PIPE_CAP_LOAD_CONSTBUF:
+   case PIPE_CAP_CONTEXT_PRIORITY_MASK:
        return 0;

     case PIPE_CAP_VENDOR_ID:

diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_screen.c 
b/src/gallium/drivers/nouveau/nvc0/nvc0_screen.c

index ac850c493da..05913bccb65 100644
--- a/src/gallium/drivers/nouveau/nvc0/nvc0_screen.c
+++ b/src/gallium/drivers/nouveau/nvc0/nvc0_screen.c

@@ -305,6 +305,7 @@ nvc0_screen_get_param(struct pipe_screen 
*pscreen, enum pipe_cap param)

     case PIPE_CAP_QUERY_SO_OVERFLOW:
     case PIPE_CAP_MEMOBJ:
     case PIPE_CAP_LOAD_CONSTBUF:
+   case PIPE_CAP_CONTEXT_PRIORITY_MASK:
        return 0;

     case PIPE_CAP_VENDOR_ID:

diff --git a/src/gallium/drivers/r300/r300_screen.c 
b/src/gallium/drivers/r300/r300_screen.c

index 0c3e097535d..021f1df2db4 100644
--- a/src/gallium/drivers/r300/r300_screen.c
+++ b/src/gallium/drivers/r300/r300_screen.c

@@ -246,6 +246,7 @@ static int r300_get_param(struct pipe_screen* 
pscreen, enum pipe_cap param)

          case PIPE_CAP_QUERY_SO_OVERFLOW:
          case PIPE_CAP_MEMOBJ:
          case PIPE_CAP_LOAD_CONSTBUF:
+        case PIPE_CAP_CONTEXT_PRIORITY_MASK:
              return 0;

          /* SWTCL-only features. */

diff --git a/src/gallium/drivers/r600/r600_pipe.c 
b/src/gallium/drivers/r600/r600_pipe.c

index 655b5411ed5..98c0b10c4d0 100644
--- a/src/gallium/drivers/r600/r600_pipe.c
+++ b/src/gallium/drivers/r600/r600_pipe.c

@@ -402,6 +402,7 @@ static int r600_get_param(struct pipe_screen* 
pscreen, enum pipe_cap param)

       case PIPE_CAP_QUERY_SO_OVERFLOW:
       case PIPE_CAP_MEMOBJ:
       case PIPE_CAP_LOAD_CONSTBUF:
+     case PIPE_CAP_CONTEXT_PRIORITY_MASK:
               return 0;

       case PIPE_CAP_DOUBLES:

diff --git a/src/gallium/drivers/radeonsi/si_pipe.c 
b/src/gallium/drivers/radeonsi/si_pipe.c

index 33f5adbd3ad..2881bd8f9fb 100644
--- a/src/gallium/drivers/radeonsi/si_pipe.c
+++ b/src/gallium/drivers/radeonsi/si_pipe.c

@@ -488,6 +488,7 @@ static int si_get_param(struct pipe_screen* 
pscreen, enum pipe_cap param)

       case PIPE_CAP_QUERY_SO_OVERFLOW:
       case PIPE_CAP_MEMOBJ:
       case PIPE_CAP_LOAD_CONSTBUF:
+     case PIPE_CAP_CONTEXT_PRIORITY_MASK:
               return 1;

I don't think you want to return 1 here.



oh, opps.. I think I screwed that up somehow in rebase conflict 
resolution..


fwiw, the cap value is a bitmask of PIPE_CONTEXT_{HIGH,LOW}_PRIORITY
with default that everyone gets being MEDIUM.. but I will add
something to docs

I am assuming HIGH/MED/LOW is enough, since that is what EGL extension
wants.. last I looked, i915 is going in the direction of adding a
wider range of priority levels.. one possible open question, that I
should have mentioned, is whether we want to define one extra lower
priority level for compute, since if compute/opencl shares a ring,
maybe it wants to be lower priority than anything gfx?  But I guess
also we can tackle this easily enough when the need arises.



I'd say tackle it when the need arises, because the design space here is 
huge and it's not clear what the right answer should be.



For example, on amdgpu there's been some work on adding a priority 
setting to the kernel scheduler, but there's also a hardware feature 
that could potentially set priorities for how shader waves are scheduled 
for asynchronous compute. Are we going to expose that at some point? 
How? Who knows -- but without a concrete plan for what the app-facing 
API will look like, it's all moot, and I'd just go for the simplest 
Gallium interface possible that reflects the EGL_IMG_context_priority 
extension (and the DRI2 bits that already exist).




Agreed on the ironing out later.


Also wanted to point out that the patches for exposing amdgpu context 
priorities includes support the HW priorities as well :)



The kernel interface pretty much maps identically to what has been 
implemented on this gallium patch.



One caveat is that, as you mentioned, this feature is mainly intended 
for async compute worloads. The priorities between two GFX contexts is 
something the driver/hw doesn't really handle well. We only have a 
single GFX pipe (with a single queue), so we can only support priorities 
on the software scheduler side. Implementing GFX job pre-emption *might* 
help the scheduler by allowing a switch to occur on smaller granularity 
than a job. But the latency overhead of this approach is still subject 
to high variance, which can defeats the purpose of a high priority 
context in the first place.



On the compute side we have multiple queues and pipes, and a very 
flexible configuration model for priorities.


Regards,
Andres


Cheers,
Nicolai



BR,
-R



Otherwise, I agree with Brian, this should be mentioned in docs.
With that fixed,
Reviewed-by: Roland Scheidegger <srol...@vmware.com>


_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev





_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev






















					Reply via email to