[Mesa-dev] [PATCH v2 2/4] egl, dri: Propagate context priority hint to driver->CreateContext

[Chris Wilson]

On Wed, May 03, 2017 at 03:42:42PM +0100, Emil Velikov wrote:
> On 3 May 2017 at 15:29, Chris Wilson <chris at chris-wilson.co.uk> wrote:
> > On Wed, Apr 12, 2017 at 02:58:50PM +0100, Emil Velikov wrote:
> >> On 11 April 2017 at 17:11, Chris Wilson <chris at chris-wilson.co.uk> wrote:
> >> > Jump through the layers of abstraction between egl and dri in order to
> >> > feed the context priority attribute through to the backend. This
> >> > requires us to read the value from the base _egl_context, convert it to
> >> > a DRI attribute, parse it again in the generic context creator before
> >> > passing it to the driver as a function parameter.
> >> >
> >> > In order to not require us to pass back the actual value of the context
> >> > priority after creation, we impose that drivers should report the
> >> > available set of priorities during screen setup (and then they may chose
> >> > to fail if given an invalid value as that should have been checked at
> >> > the user boundary.)
> >> >
> >> > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> >> > ---
> >> >  include/GL/internal/dri_interface.h            |  6 ++++
> >> >  src/egl/drivers/dri2/egl_dri2.c                | 38 +++++++++++++++++++++-----
> >> >  src/gallium/state_trackers/dri/dri_context.c   |  1 +
> >> >  src/gallium/state_trackers/dri/dri_context.h   |  1 +
> >> >  src/mesa/drivers/dri/common/dri_util.c         |  7 ++++-
> >> >  src/mesa/drivers/dri/common/dri_util.h         |  9 +++---
> >> >  src/mesa/drivers/dri/i915/intel_screen.c       | 11 ++++----
> >> >  src/mesa/drivers/dri/i965/brw_context.c        |  7 +++--
> >> >  src/mesa/drivers/dri/i965/brw_context.h        | 17 ++++++------
> >> >  src/mesa/drivers/dri/nouveau/nouveau_context.c |  1 +
> >> >  src/mesa/drivers/dri/nouveau/nouveau_context.h |  4 +--
> >> >  src/mesa/drivers/dri/r200/r200_context.c       |  1 +
> >> >  src/mesa/drivers/dri/r200/r200_context.h       |  1 +
> >> >  src/mesa/drivers/dri/radeon/radeon_context.c   |  1 +
> >> >  src/mesa/drivers/dri/radeon/radeon_context.h   |  1 +
> >> >  src/mesa/drivers/dri/swrast/swrast.c           |  1 +
> >> >  16 files changed, 77 insertions(+), 30 deletions(-)
> >> >
> >> > diff --git a/include/GL/internal/dri_interface.h b/include/GL/internal/dri_interface.h
> >> > index 9881ddcbb0..969d304a95 100644
> >> > --- a/include/GL/internal/dri_interface.h
> >> > +++ b/include/GL/internal/dri_interface.h
> >> > @@ -1056,6 +1056,12 @@ struct __DRIdri2LoaderExtensionRec {
> >> >  #define __DRI_CTX_RESET_LOSE_CONTEXT           1
> >> >  /*@}*/
> >> >
> >> > +#define __DRI_CTX_ATTRIB_PRIORITY              4
> >> > +
> >> > +#define __DRI_CTX_PRIORITY_LOW                 0
> >> > +#define __DRI_CTX_PRIORITY_MEDIUM              1
> >> > +#define __DRI_CTX_PRIORITY_HIGH                        2
> >> > +
> >> Second Ben's question here - reuse the HAS_CTX_PRIO defines if you prefer.
> >
> > I still haven't found a way I can cleanly include the same definitions
> > into both the EGL and DRI2 interfaces. These bits get pulled into the
> > core EGL via the egl_dri2 module and used by drivers via the dri
> > interface. There doesn't seem to be any overlap between the core where
> > we want to report the information to the client, and the driver talking
> > dri (not egl).
> >
> Seems like I wasn't clear - I was wondering about reusing
> __DRI2_RENDERER_HAS_CONTEXT_PRIORITY_* here.

Ah. I was thinking of just how to get egl/dri using the same defines
(that's the implicit contract in the code). I'm meh as to whether to
#define __DRI_CTX_PRIORITY_LOW __DRI2_RENDERER_HAS_CONTEXT_PRIORITY_LOW
as that may give the impression that the value is meaningful and it
breaks the pattern with the other attribs (or so I think). These are
really just enum {
	__DRI_CTX_PRIORITY_LOW,
	__DRI_CTX_PRIORITY_MEDIUM,
	__DRI_CTX_PRIORITY_HIGH,
};
but I used #defines to follow suit.
 
> > I'm going to have to leave this to someone more familiar, if we do run
> > into a situation where the bits need updating.
> Sure thing - it was a very minor suggestion.
> 
> With 17.1 branched the file to update is docs/relnotes/17.2.0.html

Done. Hopefully the kernel will be ready before 17.2 or I'll be patching
17.3 instead!
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre