[Mesa-dev] [PATCH 5/6] freedreno: context priority support

[Rob Clark]

On Wed, Oct 4, 2017 at 3:37 PM, Roland Scheidegger <sroland at vmware.com> wrote:
> Am 04.10.2017 um 17:44 schrieb Rob Clark:
>> For devices (and kernels) which support different priority ringbuffers,
>> expose context priority support.
>>
>> Signed-off-by: Rob Clark <robdclark at gmail.com>
>> ---
>>  src/gallium/drivers/freedreno/freedreno_context.c |  9 ++++++++-
>>  src/gallium/drivers/freedreno/freedreno_screen.c  | 12 +++++++++++-
>>  src/gallium/drivers/freedreno/freedreno_screen.h  |  1 +
>>  3 files changed, 20 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/gallium/drivers/freedreno/freedreno_context.c b/src/gallium/drivers/freedreno/freedreno_context.c
>> index 20480f4f8c1..7fdb848f380 100644
>> --- a/src/gallium/drivers/freedreno/freedreno_context.c
>> +++ b/src/gallium/drivers/freedreno/freedreno_context.c
>> @@ -249,10 +249,17 @@ fd_context_init(struct fd_context *ctx, struct pipe_screen *pscreen,
>>  {
>>       struct fd_screen *screen = fd_screen(pscreen);
>>       struct pipe_context *pctx;
>> +     unsigned prio = 1;
>>       int i;
>>
>> +     /* lower numerical value == higher priority: */
>> +     if (flags & PIPE_CONTEXT_HIGH_PRIORITY)
>> +             prio = 0;
>> +     else if (flags & PIPE_CONTEXT_LOW_PRIORITY)
>> +             prio = 2;
>> +
>>       ctx->screen = screen;
>> -     ctx->pipe = fd_pipe_new(screen->dev, FD_PIPE_3D);
>> +     ctx->pipe = fd_pipe_new2(screen->dev, FD_PIPE_3D, prio);
>>
>>       ctx->primtypes = primtypes;
>>       ctx->primtype_mask = 0;
>> diff --git a/src/gallium/drivers/freedreno/freedreno_screen.c b/src/gallium/drivers/freedreno/freedreno_screen.c
>> index 96866d656be..aa451f501ff 100644
>> --- a/src/gallium/drivers/freedreno/freedreno_screen.c
>> +++ b/src/gallium/drivers/freedreno/freedreno_screen.c
>> @@ -325,9 +325,11 @@ 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_CONTEXT_PRIORITY_MASK:
>> +             return screen->priority_mask;
>> +
>>       case PIPE_CAP_MAX_VIEWPORTS:
>>               return 1;
>>
>> @@ -803,6 +805,14 @@ fd_screen_create(struct fd_device *dev)
>>       }
>>       screen->chip_id = val;
>>
>> +     if (fd_pipe_get_param(screen->pipe, FD_NR_RINGS, &val)) {
>> +             DBG("could not get # of rings");
>> +             screen->priority_mask = 0;
>> +     } else {
>> +             /* # of rings equates to number of unique priority values: */
>> +             screen->priority_mask = (1 << val) - 1;
>> +     }
> This doesn't quite seem to guarantee you only return valid values for
> the cap,
> unless your number of rings doesn't exceed 3. Maybe that's always the case,
> but I think should either mention that in the comment or explicitly mask
> off invalid bits.
>

Actually, it would be four rings for the GPUs which support preemption
(currently, although the limit is semi arbitrary and could easily be
increased later)..  So this would set more bits than are currently
used.  I'm undecided whether that is good or bad.  If we decide later
to support more priorities in gallium, then I'd have to remember to
remove the masking out of extra bits.  But otoh I think the extra bits
as it is now are harmless.

BR,
-R

>
>>       DBG("Pipe Info:");
>>       DBG(" GPU-id:          %d", screen->gpu_id);
>>       DBG(" Chip-id:         0x%08x", screen->chip_id);
>> diff --git a/src/gallium/drivers/freedreno/freedreno_screen.h b/src/gallium/drivers/freedreno/freedreno_screen.h
>> index 68518ef721b..d5e497d4f65 100644
>> --- a/src/gallium/drivers/freedreno/freedreno_screen.h
>> +++ b/src/gallium/drivers/freedreno/freedreno_screen.h
>> @@ -67,6 +67,7 @@ struct fd_screen {
>>       uint32_t max_rts;        /* max # of render targets */
>>       uint32_t gmem_alignw, gmem_alignh;
>>       uint32_t num_vsc_pipes;
>> +     uint32_t priority_mask;
>>       bool has_timestamp;
>>
>>       void *compiler;          /* currently unused for a2xx */
>>
>