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

Andres Rodriguez andresx7 at gmail.com
Tue Oct 10 01:41:05 UTC 2017 


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 
>> <sroland at vmware.com> wrote:
>>> Am 04.10.2017 um 17:44 schrieb Rob Clark:
>>>> Signed-off-by: Rob Clark <robdclark at 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 <sroland at vmware.com>
>>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>>
> 
>

More information about the etnaviv mailing list