Re: [Mesa-dev] [Intel-gfx] [PATCH] i965: Revert absolute mode for constant buffer pointers.
Hi, After some brainstorming, we decided to go big and grab all the hardware provided defaults for all registers. That's for the sake of not having to decide which registers are important. This will also allow us not to be concerned of any context registers possibly containing 'secure' information to be leaked, which we currently don't expect from any known userspace. But you never know what somebody could be doing with unused-for-them registers :) The fix is not quite stable material, containing changes to the hardware initialization ordering and being rather invasive in nature. We're currently discussing the uAPI to expose to userspace, please stay tuned. It'll likely be a bitmask of engine classes, which will shortly be introduced from the PMU work. Regards, Joonas On Wed, 2017-10-25 at 10:53 -0700, Jason Ekstrand wrote: > On Wed, Oct 25, 2017 at 10:31 AM, Kenneth Graunke <kenn...@whitecape.org> > wrote: > > On Wednesday, October 25, 2017 7:33:41 AM PDT Jason Ekstrand wrote: > > > On October 25, 2017 06:05:16 Joonas Lahtinen wrote: > > [snip] > > > > There indeed seems to be quite a lot of missing registers from the i915 > > > > driver where the context is initialized. (Psst. You can read that as: > > > > "all the 33 non-privileged registers we could quickly list, are > > > > missing"). > > > > > > We probably don't need *all* of them initialized. For instance, the > > > initial values of the ALU registers or the indirect draw parameter > > > registers will probably never matter. However, if you want to just > > > initialized them all, that's fine. > > > > I agree - I think we can cut down the list substantially, if you like. > > Here's my breakdown of Skylake's non-privileged register list: > > > > Cache_Mode_0 0x7000 > > Cache_Mode_1 0x7004 > > GT_MODE0x7008 > > L3_Config 0x7034 > > TD_CTL 0xE400 > > TD_CTL20xE404 > > L3SQCREG4 0xB118 > > NOPID 0x2094 > > INSTPM 0x20C0 > > > >Should be initialized by the kernel. Several of these can severely > >break unsuspecting userspace, and we'd like to be able to rely on a > >default value. > > > > IA_VERTICES_COUNT 0x2310 > > IA_PRIMITIVES_COUNT0x2318 > > VS_INVOCATION_COUNT0x2320 > > HS_INVOCATION_COUNT0x2300 > > DS_INVOCATION_COUNT0x2308 > > GS_INVOCATION_COUNT0x2328 > > GS_PRIMITIVES_COUNT0x2330 > > SO_NUM_PRIMS_WRITTEN0 0x5200 > > SO_NUM_PRIMS_WRITTEN1 0x5208 > > SO_NUM_PRIMS_WRITTEN2 0x5210 > > SO_NUM_PRIMS_WRITTEN3 0x5218 > > SO_PRIM_STORAGE_NEEDED00x5240 > > SO_PRIM_STORAGE_NEEDED10x5248 > > SO_PRIM_STORAGE_NEEDED20x5250 > > SO_PRIM_STORAGE_NEEDED30x5258 > > CL_INVOCATION_COUNT0x2338 > > CL_PRIMITIVES_COUNT0x2340 > > PS_INVOCATION_COUNT_0 0x22C8 > > PS_DEPTH_COUNT_0 0x22D8 > > PS_INVOCATION_COUNT_1 0x22F0 > > PS_DEPTH_COUNT_1 0x22F8 > > PS_INVOCATION_COUNT_2 0x2448 > > PS_DEPTH_COUNT_2 0x2450 > > GPGPU_THREADS_DISPATCHED 0x2290 > > > >The kernel can skip these if you like. Statistics registers just count > >things, and userspace always calculates (end counter - start counter) > >deltas, so the initial value doesn't really matter. > > > > SO_WRITE_OFFSET0 0x5280 > > SO_WRITE_OFFSET1 0x5284 > > SO_WRITE_OFFSET2 0x5288 > > SO_WRITE_OFFSET3 0x528C > > GPUGPU_DISPATCHDIMX0x2500 > > GPUGPU_DISPATCHDIMY0x2504 > > GPUGPU_DISPATCHDIMZ0x2508 > > MI_PREDICATE_SRC0 0x2400 > > MI_PREDICATE_SRC0 0x2404 > > MI_PREDICATE_SRC1 0x2408 > > MI_PREDICATE_SRC1 0x240C > > MI_PREDICATE_DATA 0x2410 > > MI_PREDICATE_DATA 0x2414 > > MI_PREDICATE_RESULT0x2418 > > MI_PREDICATE_RESULT_1 0x241C > > MI_PREDICATE_RESULT_2 0x23BC > > 3DPRIM_END_OFFSET 0x2420 > > 3DPRIM_START_VERTEX0x2430 > > 3DPRIM_VERTEX_COUNT0x2434 > > 3DPRIM_INSTANCE_COUNT 0x2438 > > 3DPRIM_START_INSTANCE 0x243C > > 3DPRIM_BASE_VERTEX 0x2440 > > > >The kernel can skip these if you like, IMO. These registers are only > >used when enabling an optional feature - stream out (SO_WRITE_*), > >indirect compute dispatch
Re: [Mesa-dev] [PATCH] i965: Revert absolute mode for constant buffer pointers.
On Mon, 2017-10-23 at 16:19 -0700, Kenneth Graunke wrote: > On Monday, October 23, 2017 3:53:15 PM PDT Rodrigo Vivi wrote: > > On Mon, Oct 23, 2017 at 10:32:43PM +, Jordan Justen wrote: > > > On 2017-10-19 16:30:44, Kristian Høgsberg wrote: > > > > On Thu, Oct 19, 2017 at 4:18 PM, Kenneth Graunke > > > > <kenn...@whitecape.org> wrote: > > > > > The kernel doesn't initialize the value of the INSTPM or > > > > > CS_DEBUG_MODE2 > > > > > registers at context initialization time. Instead, they're inherited > > > > > from whatever happened to be running on the GPU prior to first run of > > > > > a > > > > > new context. So, when we started setting these, other contexts in the > > > > > system started inheriting our values. Since this controls whether > > > > > 3DSTATE_CONSTANT_* takes a pointer or an offset, getting the wrong > > > > > setting is fatal for almost any process which isn't expecting this. > > > > > > > > > > Unfortunately, VA-API and Beignet don't initialize this (nor does > > > > > older > > > > > Mesa), so they will die horribly if we start doing this. UXA and SNA > > > > > don't use any push constants, so they are unaffected. > > > > > > > > > > The kernel developers are apparently uninterested in making the proto- > > > > > context initialize these registers to guarantee deterministic > > > > > behavior. > > > > > > > > Could somebody from the kernel team elaborate here? This is obviously > > > > broken and undermines the security and containerization that hw > > > > contexts are supposed to provide. I'm really curious what the thinking > > > > is here... > > > > > > > > Kristian > > > > > > Cc intel-gfx, maintainers > > > > Is this the null-state/golden-context related discussions? > > > > I assume we are ok for older platforms, but the problem would be only for > > CNL+ where we are not adding the null context initialization yet. > > Am I getting it right? > > No, this problem exists on earlier platforms as well. We saw the issue > on Broadwell and Kabylake. + Daniel, Chris There indeed seems to be quite a lot of missing registers from the i915 driver where the context is initialized. (Psst. You can read that as: "all the 33 non-privileged registers we could quickly list, are missing"). You can see for yourself at execlists_init_reg_state() in "intel_lrc.c". So currently you can expect issues if some userspace sets fancy register values that the rest of the userspaces are not setting. We'll be providing a rework on the context register state initialization code to fix the issue. There's quite some detail to it, considering the golden render context, W/As and all. In the meanwhile, revert would be the only option for Mesa. Chris wrote a nice I-G-T test to replicate the scenario: https://patchwork.freedesktop.org/patch/184573/ What was also observed is that messing with some of the non-privileged register will not only take the GPU with them, but the whole machine. But that's not exactly a surprise. Regards, Joonas -- Joonas Lahtinen Open Source Technology Center Intel Corporation ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 4/7] i965: Record the presence of the kernel scheduler
On Fri, 2017-09-29 at 12:52 -0700, Kenneth Graunke wrote:
> On Friday, September 29, 2017 3:25:07 AM PDT Chris Wilson wrote:
> > Mention to the debug log if the kernel scheduler is enabled; and in
> > particular if it has preemption enabled.
> >
> > Signed-off-by: Chris Wilson <ch...@chris-wilson.co.uk>
> > Cc: Joonas Lahtinen <joonas.lahti...@linux.intel.com>
> > Cc: Ben Widawsky <b...@bwidawsk.net>
> > Reviewed-by: Joonas Lahtinen <joonas.lahti...@linux.intel.com>
> > ---
> > src/mesa/drivers/dri/i965/intel_screen.c | 11 +++
> > 1 file changed, 11 insertions(+)
> >
> > diff --git a/src/mesa/drivers/dri/i965/intel_screen.c
> > b/src/mesa/drivers/dri/i965/intel_screen.c
> > index bd1365f232..22d9f19298 100644
> > --- a/src/mesa/drivers/dri/i965/intel_screen.c
> > +++ b/src/mesa/drivers/dri/i965/intel_screen.c
> > @@ -2516,6 +2516,17 @@ __DRIconfig **intelInitScreen2(__DRIscreen
> > *dri_screen)
> >
> > intel_screen_init_surface_formats(screen);
> >
> > + if (INTEL_DEBUG & DEBUG_SUBMIT) {
>
> How about:
>
> if (INTEL_DEBUG & (DEBUG_SUBMIT | DEBUG_BAT)) {
Assuming that's DEBUG_BATCH (not some new Basic Acceptance Testing
variable), that works too :)
Regards, Joonas
--
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] i965: Record the presence of the kernel scheduler
On Wed, 2017-09-27 at 16:18 +0100, Chris Wilson wrote: > Mention to the debug log if the kernel scheduler is enabled; and in > particular if it has preemption enabled. > > Signed-off-by: Chris Wilson <ch...@chris-wilson.co.uk> > Cc: Joonas Lahtinen <joonas.lahti...@linux.intel.com> > Cc: Ben Widawsky <b...@bwidawsk.net> This should debugging much more pleasant. Reviewed-by: Joonas Lahtinen <joonas.lahti...@linux.intel.com> Regards, Joonas -- Joonas Lahtinen Open Source Technology Center Intel Corporation ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [RFC v3] drm/i915: Select engines via class and instance in execbuffer2
On ke, 2017-05-17 at 16:40 +0100, Tvrtko Ursulin wrote: > From: Tvrtko Ursulin <tvrtko.ursu...@intel.com> > > Building on top of the previous patch which exported the concept > of engine classes and instances, we can also use this instead of > the current awkward engine selection uAPI. > > This is primarily interesting for the VCS engine selection which > is a) currently done via disjoint set of flags, and b) the > current I915_EXEC_BSD flags has different semantics depending on > the underlying hardware which is bad. > > Proposed idea here is to reserve 16-bits of flags, to pass in > the engine class and instance (8 bits each), and a new flag > named I915_EXEC_CLASS_INSTACE to tell the kernel this new engine > selection API is in use. Would it make sense to use bitmask for future proofing? Then we could allow passing multiple options in the future. Other than that, looks good. Chris already commented some on the code itself. Regards, Joonas -- Joonas Lahtinen Open Source Technology Center Intel Corporation ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [Intel-gfx] [PATCH] drm/i915: Add I915_PARAM_MMAP_GTT_VERSION to advertise unlimited mmaps
On to, 2016-08-25 at 09:27 +0100, Chris Wilson wrote: > On Thu, Aug 25, 2016 at 11:00:54AM +0300, Joonas Lahtinen wrote: > > > > On ke, 2016-08-24 at 20:42 +0100, Chris Wilson wrote: > > > > > > +++ b/drivers/gpu/drm/i915/i915_drv.c > > > @@ -355,6 +355,14 @@ static int i915_getparam(struct drm_device *dev, > > > void *data, > > > case I915_PARAM_MIN_EU_IN_POOL: > > > value = INTEL_INFO(dev)->min_eu_in_pool; > > > break; > > > + case I915_PARAM_MMAP_GTT_VERSION: > > > + /* 0 - Objects have to be smaller than aperture, > > > + * all simultaneous users have to fit within the > > > + * available space within the aperture. > > > + * 1 - Objects can any size, and X,Y or untiled > > > + */ > > > + value = 1; > > The actual test would be if this function call succeeds, though. > > > > value = 0 would never be returned, so I'm not sure it'll be useful to > > document as such. So I might document as "Failure to execute the query > > means..." > It's also going to get crowded if we put all snippets of ABI information > here. Yep, but 0 is never returned so the documentation is incorrect as is. Better clearly describe the that before the parameter was introduced conditions were as above. > > If I > > #define I915_MMAP_GTT_VERSION 1 > /* ... */ > > at the start of i915_gem_fault() then hopefully we might remember to > update it. +1 on this. Regards, Joonas > -Chris > -- Joonas Lahtinen Open Source Technology Center Intel Corporation ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [Intel-gfx] [PATCH] drm/i915: Add I915_PARAM_MMAP_GTT_VERSION to advertise unlimited mmaps
On ke, 2016-08-24 at 20:42 +0100, Chris Wilson wrote: > +++ b/drivers/gpu/drm/i915/i915_drv.c > @@ -355,6 +355,14 @@ static int i915_getparam(struct drm_device *dev, void > *data, > case I915_PARAM_MIN_EU_IN_POOL: > value = INTEL_INFO(dev)->min_eu_in_pool; > break; > + case I915_PARAM_MMAP_GTT_VERSION: > + /* 0 - Objects have to be smaller than aperture, > + * all simultaneous users have to fit within the > + * available space within the aperture. > + * 1 - Objects can any size, and X,Y or untiled > + */ > + value = 1; The actual test would be if this function call succeeds, though. value = 0 would never be returned, so I'm not sure it'll be useful to document as such. So I might document as "Failure to execute the query means..." Regards, Joonas -- Joonas Lahtinen Open Source Technology Center Intel Corporation ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] i915: BINDING_TABLE_POINTER_* after CONSTANT_* for SKL & BXT
Hi, This could then be applied with the correction s/i915/i965/ in the commit message, right? Regards, Joonas On to, 2015-08-13 at 09:06 -0700, Ben Widawsky wrote: > On Thu, Aug 13, 2015 at 10:38:43AM +0300, Joonas Lahtinen wrote: > > Hi, > > > > On ke, 2015-08-12 at 18:34 -0700, Ben Widawsky wrote: > > > On Wed, Aug 12, 2015 at 03:09:44PM +0300, Joonas Lahtinen wrote: > > > > Add a comment about reinforcing command order so that > > > > 3DSTATE_BINDING_TABLE_POINTER_* commands are after > > > > 3DSTATE_CONSTANT_* commands for SKL & BXT, otherwise the > > > > GPU might hang. > > > > > > > > Changing the BLORP code is not relevant (where the order > > > > is "wrong"), as it is not used for GEN8 or up. > > > > > > > > Cc: Mika Kuoppala <mika.kuopp...@linux.intel.com> > > > > Cc: Arun Siluvery <arun.siluv...@linux.intel.com> > > > > Signed-off-by: Joonas Lahtinen <joonas.lahti...@linux.intel.com > > > > > > > > > --- > > > > src/mesa/drivers/dri/i965/brw_state_upload.c | 6 +++--- > > > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > > > > > diff --git a/src/mesa/drivers/dri/i965/brw_state_upload.c > > > > b/src/mesa/drivers/dri/i965/brw_state_upload.c > > > > index 9de42ce..9078e11 100644 > > > > --- a/src/mesa/drivers/dri/i965/brw_state_upload.c > > > > +++ b/src/mesa/drivers/dri/i965/brw_state_upload.c > > > > @@ -299,9 +299,9 @@ static const struct brw_tracked_state > > > > *gen8_render_atoms[] = > > > > _wm_abo_surfaces, > > > > _renderbuffer_surfaces, > > > > _texture_surfaces, > > > > - _vs_binding_table, > > > > - _gs_binding_table, > > > > - _wm_binding_table, > > > > + _vs_binding_table, /* Must come after vs_push_constants > > > > for > > > > Skylake and Broxton. */ > > > > + _gs_binding_table, /* Must come after gs_push_constants > > > > for > > > > Skylake and Broxton. */ > > > > + _wm_binding_table, /* Must come after wm_push_constants > > > > for > > > > Skylake and Broxton. */ > > > > > > > > _fs_samplers, > > > > _vs_samplers, > > > > > > Does anyone understand why this actually causes a hang on the IGT > > > test? I > > > certainly don't. The docs are pretty clear that the constant > > > command > > > is not > > > committed until the BTP command, but I can't make any sense of > > > how it > > > related to > > > a GPU hang. > > > > > > > Discussion about this continued in the driver list. > > > > > In any event, I don't think the comments are super useful, but > > > they're not > > > harmful either. I'd suggest one line instead: > > > "NOTE: push_constant_ff must precede binding table pointer > > > upload" > > > > > > > The table previously seemed to contain per-line comments for other > > ordering restrictions, so I just went with style that looks > > consitent > > with the rest. Also it makes some sense, as it's only the > > respective > > 3DSTATE_CONSTANT_* whose parsing is triggered by matching a > > 3DSTATE_BINDING_TABLE_POINTER_* command. They could be interleaved > > too. > > > > I'll correct the s/i915/i965/ as noted by Matt. How about the > > comments? > > Whatever you like. Ideally we'd probably try to capture such things > in the atom > debugging, but I did look at that code, and it seems like it'd be a > pain to add > for no real gain. > > > > > Regards, Joonas > > > > > Reviewed-by: Ben Widawsky <b...@bwidawsk.net> ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] i915: BINDING_TABLE_POINTER_* after CONSTANT_* for SKL BXT
Hi, On ke, 2015-08-12 at 18:34 -0700, Ben Widawsky wrote: On Wed, Aug 12, 2015 at 03:09:44PM +0300, Joonas Lahtinen wrote: Add a comment about reinforcing command order so that 3DSTATE_BINDING_TABLE_POINTER_* commands are after 3DSTATE_CONSTANT_* commands for SKL BXT, otherwise the GPU might hang. Changing the BLORP code is not relevant (where the order is wrong), as it is not used for GEN8 or up. Cc: Mika Kuoppala mika.kuopp...@linux.intel.com Cc: Arun Siluvery arun.siluv...@linux.intel.com Signed-off-by: Joonas Lahtinen joonas.lahti...@linux.intel.com --- src/mesa/drivers/dri/i965/brw_state_upload.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/mesa/drivers/dri/i965/brw_state_upload.c b/src/mesa/drivers/dri/i965/brw_state_upload.c index 9de42ce..9078e11 100644 --- a/src/mesa/drivers/dri/i965/brw_state_upload.c +++ b/src/mesa/drivers/dri/i965/brw_state_upload.c @@ -299,9 +299,9 @@ static const struct brw_tracked_state *gen8_render_atoms[] = brw_wm_abo_surfaces, gen6_renderbuffer_surfaces, brw_texture_surfaces, - brw_vs_binding_table, - brw_gs_binding_table, - brw_wm_binding_table, + brw_vs_binding_table, /* Must come after vs_push_constants for Skylake and Broxton. */ + brw_gs_binding_table, /* Must come after gs_push_constants for Skylake and Broxton. */ + brw_wm_binding_table, /* Must come after wm_push_constants for Skylake and Broxton. */ brw_fs_samplers, brw_vs_samplers, Does anyone understand why this actually causes a hang on the IGT test? I certainly don't. The docs are pretty clear that the constant command is not committed until the BTP command, but I can't make any sense of how it related to a GPU hang. Discussion about this continued in the driver list. In any event, I don't think the comments are super useful, but they're not harmful either. I'd suggest one line instead: NOTE: push_constant_ff must precede binding table pointer upload The table previously seemed to contain per-line comments for other ordering restrictions, so I just went with style that looks consitent with the rest. Also it makes some sense, as it's only the respective 3DSTATE_CONSTANT_* whose parsing is triggered by matching a 3DSTATE_BINDING_TABLE_POINTER_* command. They could be interleaved too. I'll correct the s/i915/i965/ as noted by Matt. How about the comments? Regards, Joonas Reviewed-by: Ben Widawsky b...@bwidawsk.net ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] i915: BINDING_TABLE_POINTER_* after CONSTANT_* for SKL BXT
Add a comment about reinforcing command order so that 3DSTATE_BINDING_TABLE_POINTER_* commands are after 3DSTATE_CONSTANT_* commands for SKL BXT, otherwise the GPU might hang. Changing the BLORP code is not relevant (where the order is wrong), as it is not used for GEN8 or up. Cc: Mika Kuoppala mika.kuopp...@linux.intel.com Cc: Arun Siluvery arun.siluv...@linux.intel.com Signed-off-by: Joonas Lahtinen joonas.lahti...@linux.intel.com --- src/mesa/drivers/dri/i965/brw_state_upload.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/mesa/drivers/dri/i965/brw_state_upload.c b/src/mesa/drivers/dri/i965/brw_state_upload.c index 9de42ce..9078e11 100644 --- a/src/mesa/drivers/dri/i965/brw_state_upload.c +++ b/src/mesa/drivers/dri/i965/brw_state_upload.c @@ -299,9 +299,9 @@ static const struct brw_tracked_state *gen8_render_atoms[] = brw_wm_abo_surfaces, gen6_renderbuffer_surfaces, brw_texture_surfaces, - brw_vs_binding_table, - brw_gs_binding_table, - brw_wm_binding_table, + brw_vs_binding_table, /* Must come after vs_push_constants for Skylake and Broxton. */ + brw_gs_binding_table, /* Must come after gs_push_constants for Skylake and Broxton. */ + brw_wm_binding_table, /* Must come after wm_push_constants for Skylake and Broxton. */ brw_fs_samplers, brw_vs_samplers, -- 2.4.3 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] egl: dri2: Use present extension. (Was: Re: [RFC] egl: Add DRI3 support to the EGL backend.)
This is still awaiting for comments. I'd rather hear what are the desirable modifications than try guessing. On ma, 2014-11-10 at 15:18 +0200, Joonas Lahtinen wrote: Hi, On pe, 2014-11-07 at 17:40 -0800, Eric Anholt wrote: Ian Romanick i...@freedesktop.org writes: On 11/06/2014 06:16 PM, Michel Dänzer wrote: On 06.11.2014 19:18, Joonas Lahtinen wrote: On to, 2014-11-06 at 18:12 +0900, Michel Dänzer wrote: On 05.11.2014 20:14, Joonas Lahtinen wrote: Modified not refer to DRI3, just uses the present extension to get rid of the excess buffer invalidations. AFAICT there's no fallback from your changes to the current behaviour if the X server doesn't support the Present extension. There probably needs to be such a fallback. It gets rid of such nasty hack (the intel_viewport one), that I thought there is no point making fallback. Because without this, the egl dri2 backend is fundamentally broken anyway. Well, AFAICT your code uses Present extension functionality unconditionally, without checking that the X server supports Present. I can't see how that could possibly work on an X server which doesn't support Present, but I think it would be better to keep it working at least as badly as it does now in that case. :) I was going to say pretty much the same thing. Aren't there (non-Intel) drivers that don't do Present? If I'm not mistaken, some parts of DRI3 (not sure about Present) are even disabled in the Intel driver when SNA is in use... or at least that was the case at one point. They actually get a fallback implementation if there's no driver support, which would be sufficient for this code. However, Present is too new for Mesa to be unconditionally relying on in my opinion. Based on above discussion, I would bring back the dynamic detection like in the original patch. But for present extension instead of DRI3. Technically it would be very much the same, different naming conventions. And also, re-use the USE_INVALIDATE extension instead of adding DRI3 extension. Would that be an acceptable solution? Regards, Joonas ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] egl: dri2: Use present extension. (Was: Re: [RFC] egl: Add DRI3 support to the EGL backend.)
Hi, On pe, 2014-11-07 at 17:40 -0800, Eric Anholt wrote: Ian Romanick i...@freedesktop.org writes: On 11/06/2014 06:16 PM, Michel Dänzer wrote: On 06.11.2014 19:18, Joonas Lahtinen wrote: On to, 2014-11-06 at 18:12 +0900, Michel Dänzer wrote: On 05.11.2014 20:14, Joonas Lahtinen wrote: Modified not refer to DRI3, just uses the present extension to get rid of the excess buffer invalidations. AFAICT there's no fallback from your changes to the current behaviour if the X server doesn't support the Present extension. There probably needs to be such a fallback. It gets rid of such nasty hack (the intel_viewport one), that I thought there is no point making fallback. Because without this, the egl dri2 backend is fundamentally broken anyway. Well, AFAICT your code uses Present extension functionality unconditionally, without checking that the X server supports Present. I can't see how that could possibly work on an X server which doesn't support Present, but I think it would be better to keep it working at least as badly as it does now in that case. :) I was going to say pretty much the same thing. Aren't there (non-Intel) drivers that don't do Present? If I'm not mistaken, some parts of DRI3 (not sure about Present) are even disabled in the Intel driver when SNA is in use... or at least that was the case at one point. They actually get a fallback implementation if there's no driver support, which would be sufficient for this code. However, Present is too new for Mesa to be unconditionally relying on in my opinion. Based on above discussion, I would bring back the dynamic detection like in the original patch. But for present extension instead of DRI3. Technically it would be very much the same, different naming conventions. And also, re-use the USE_INVALIDATE extension instead of adding DRI3 extension. Would that be an acceptable solution? Regards, Joonas ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] egl: dri2: Use present extension. (Was: Re: [RFC] egl: Add DRI3 support to the EGL backend.)
On to, 2014-11-06 at 18:12 +0900, Michel Dänzer wrote: On 05.11.2014 20:14, Joonas Lahtinen wrote: Modified not refer to DRI3, just uses the present extension to get rid of the excess buffer invalidations. AFAICT there's no fallback from your changes to the current behaviour if the X server doesn't support the Present extension. There probably needs to be such a fallback. It gets rid of such nasty hack (the intel_viewport one), that I thought there is no point making fallback. Because without this, the egl dri2 backend is fundamentally broken anyway. Regards, Joonas ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] egl: dri2: Use present extension. (Was: Re: [RFC] egl: Add DRI3 support to the EGL backend.)
On ke, 2014-11-05 at 15:19 +, Emil Velikov wrote:
Hi Joonas,
Does getting rid of the viewport hack give you any noticeable
performance improvement?
Yes, it significantly reduces the CPU load when multiple glViewport
calls are made per frame (4x4 grid or so).
Is there any interest in converting the
egl_dri2 backend to dri3, rather than just copying over the present bits ?
This could be one thing to do. But in the meanwhile, I would commit this
present extension patch so that the affected use cases get the
improvements.
Regards, Joonas
On 05/11/14 11:14, Joonas Lahtinen wrote:
Hi,
Modified not refer to DRI3, just uses the present extension to get rid
of the excess buffer invalidations.
Regards, Joonas
From 257e2a8c750f9dcf868cce9da8632df3cae0fcec Mon Sep 17 00:00:00 2001
From: Joonas Lahtinen joonas.lahti...@linux.intel.com
Date: Wed, 5 Nov 2014 12:25:32 +0200
Subject: [PATCH] egl: dri2: Use present extension.
Present extension is used to avoid excess buffer invalidations, because
the XCB interface doesn't supply that information.
Signed-off-by: Daniel van der Wath danielx.j.van.der.w...@intel.com
Signed-off-by: Joonas Lahtinen joonas.lahti...@linux.intel.com
---
configure.ac|5 +-
src/egl/drivers/dri2/egl_dri2.c |2 +-
src/egl/drivers/dri2/egl_dri2.h | 24 ++-
src/egl/drivers/dri2/platform_x11.c | 247
---
src/mesa/drivers/dri/i965/brw_context.c |9 +-
5 files changed, 262 insertions(+), 25 deletions(-)
diff --git a/configure.ac b/configure.ac
index fc7d372..75d90c0 100644
--- a/configure.ac
+++ b/configure.ac
@@ -952,7 +952,8 @@ xyesno)
fi
if test x$enable_dri = xyes; then
- dri_modules=$dri_modules xcb-dri2 = $XCBDRI2_REQUIRED
+ PKG_CHECK_MODULES([PRESENTPROTO], [presentproto =
$PRESENTPROTO_REQUIRED])
+ dri_modules=$dri_modules xcb-dri2 = $XCBDRI2_REQUIRED
xcb-present
Afaics you are not changing anything on the dri modules (or glx/dri2) to
require the above changes. Perhaps you need to push the presentproto
check in the x11 case below ?
fi
if test x$enable_dri3 = xyes; then
@@ -1564,7 +1565,7 @@ for plat in $egl_platforms; do
;;
x11)
- PKG_CHECK_MODULES([XCB_DRI2], [x11-xcb xcb xcb-dri2 =
$XCBDRI2_REQUIRED xcb-xfixes])
+ PKG_CHECK_MODULES([XCB_DRI2], [x11-xcb xcb xcb-dri2 =
$XCBDRI2_REQUIRED xcb-xfixes xcb-present])
;;
drm)
[snip]
diff --git a/src/egl/drivers/dri2/platform_x11.c
b/src/egl/drivers/dri2/platform_x11.c
index f8c4b70..a1445b2 100644
--- a/src/egl/drivers/dri2/platform_x11.c
+++ b/src/egl/drivers/dri2/platform_x11.c
@@ -188,6 +188,205 @@ get_xcb_screen(xcb_screen_iterator_t iter, int screen)
return NULL;
}
+/*
+ * Called by the XCB_PRESENT_COMPLETE_NOTIFY case.
+ */
+static void
+dri2_update_num_back(struct dri2_egl_surface *priv)
+{
+ priv-num_back = 1;
+ if (priv-flipping)
+ priv-num_back++;
+ if (priv-base.SwapInterval == 0)
+ priv-num_back++;
+}
+
This seems to be out of sync with dri3_glx. Don't you need something
similar to commit f7a36ef5fe23056299a77414f9ad8b5e5a1d ?
[snip]
+/**
+ *
+ * Process any present events that have been received from the X server
+ *
+ * From glx, we additionally invalidate the drawable here if there has a
been a special event.
+ */
+static void
+dri2_flush_present_events(struct dri2_egl_display *dri2_dpy, struct
dri2_egl_surface *priv)
+{
+ xcb_connection_t *c = dri2_dpy-conn;
+
+ /* Check to see if any configuration changes have occurred
+* since we were last invoked
+*/
+ if (priv-special_event) {
+ xcb_generic_event_t*ev;
+
+ while ((ev = xcb_poll_for_special_event(c, priv-special_event)) !=
NULL) {
+ xcb_present_generic_event_t *ge = (void *) ev;
+ dri2_handle_present_event(priv, ge);
+ _eglLog(_EGL_INFO, DRI: Invalidating buffer 0x%x\n,
priv-dri_drawable);
+ (*dri2_dpy-flush-invalidate)(priv-dri_drawable);
Hmm why does one need to invalidate at this stage - I take that it's
related to lack of fence objects ?
[snip]
diff --git a/src/mesa/drivers/dri/i965/brw_context.c
b/src/mesa/drivers/dri/i965/brw_context.c
index e1a994a..dbadd10 100644
--- a/src/mesa/drivers/dri/i965/brw_context.c
+++ b/src/mesa/drivers/dri/i965/brw_context.c
@@ -148,6 +148,9 @@ intel_viewport(struct gl_context *ctx)
__DRIcontext *driContext = brw-driContext;
if (_mesa_is_winsys_fbo(ctx-DrawBuffer)) {
+ if (unlikely(INTEL_DEBUG DEBUG_DRI))
+ fprintf(stderr, invalidating drawables\n);
+
dri2InvalidateDrawable(driContext-driDrawablePriv
[Mesa-dev] [PATCH] egl: dri2: Use present extension. (Was: Re: [RFC] egl: Add DRI3 support to the EGL backend.)
Hi,
Modified not refer to DRI3, just uses the present extension to get rid
of the excess buffer invalidations.
Regards, Joonas
From 257e2a8c750f9dcf868cce9da8632df3cae0fcec Mon Sep 17 00:00:00 2001
From: Joonas Lahtinen joonas.lahti...@linux.intel.com
Date: Wed, 5 Nov 2014 12:25:32 +0200
Subject: [PATCH] egl: dri2: Use present extension.
Present extension is used to avoid excess buffer invalidations, because
the XCB interface doesn't supply that information.
Signed-off-by: Daniel van der Wath danielx.j.van.der.w...@intel.com
Signed-off-by: Joonas Lahtinen joonas.lahti...@linux.intel.com
---
configure.ac|5 +-
src/egl/drivers/dri2/egl_dri2.c |2 +-
src/egl/drivers/dri2/egl_dri2.h | 24 ++-
src/egl/drivers/dri2/platform_x11.c | 247 ---
src/mesa/drivers/dri/i965/brw_context.c |9 +-
5 files changed, 262 insertions(+), 25 deletions(-)
diff --git a/configure.ac b/configure.ac
index fc7d372..75d90c0 100644
--- a/configure.ac
+++ b/configure.ac
@@ -952,7 +952,8 @@ xyesno)
fi
if test x$enable_dri = xyes; then
- dri_modules=$dri_modules xcb-dri2 = $XCBDRI2_REQUIRED
+ PKG_CHECK_MODULES([PRESENTPROTO], [presentproto =
$PRESENTPROTO_REQUIRED])
+ dri_modules=$dri_modules xcb-dri2 = $XCBDRI2_REQUIRED
xcb-present
fi
if test x$enable_dri3 = xyes; then
@@ -1564,7 +1565,7 @@ for plat in $egl_platforms; do
;;
x11)
- PKG_CHECK_MODULES([XCB_DRI2], [x11-xcb xcb xcb-dri2 =
$XCBDRI2_REQUIRED xcb-xfixes])
+ PKG_CHECK_MODULES([XCB_DRI2], [x11-xcb xcb xcb-dri2 =
$XCBDRI2_REQUIRED xcb-xfixes xcb-present])
;;
drm)
diff --git a/src/egl/drivers/dri2/egl_dri2.c b/src/egl/drivers/dri2/egl_dri2.c
index 20a7243..c1571d6 100644
--- a/src/egl/drivers/dri2/egl_dri2.c
+++ b/src/egl/drivers/dri2/egl_dri2.c
@@ -586,7 +586,7 @@ dri2_create_screen(_EGLDisplay *disp)
dri2_dpy-own_dri_screen = 1;
extensions = dri2_dpy-core-getExtensions(dri2_dpy-dri_screen);
-
+
if (dri2_dpy-dri2) {
unsigned i;
diff --git a/src/egl/drivers/dri2/egl_dri2.h b/src/egl/drivers/dri2/egl_dri2.h
index 52f05fb..33f75f8 100644
--- a/src/egl/drivers/dri2/egl_dri2.h
+++ b/src/egl/drivers/dri2/egl_dri2.h
@@ -32,6 +32,7 @@
#include xcb/xcb.h
#include xcb/dri2.h
#include xcb/xfixes.h
+#include xcb/present.h
#include X11/Xlib-xcb.h
#endif
@@ -74,6 +75,7 @@
#include eglimage.h
#define ARRAY_SIZE(a) (sizeof(a) / sizeof((a)[0]))
+#define DRI2_EGL_SURFACE_NUM_BUFFERS 5
struct wl_buffer;
@@ -218,7 +220,7 @@ struct dri2_egl_surface
{
_EGLSurface base;
__DRIdrawable *dri_drawable;
- __DRIbuffer buffers[5];
+ __DRIbuffer buffers[DRI2_EGL_SURFACE_NUM_BUFFERS];
int buffer_count;
int have_fake_front;
@@ -229,6 +231,26 @@ struct dri2_egl_surface
int bytes_per_pixel;
xcb_gcontext_t gc;
xcb_gcontext_t swapgc;
+ xcb_special_event_t *special_event;
+ xcb_present_event_t eid;
+ uint32_t *stamp;
+ uint8_t is_pixmap;
+ uint8_t flipping;
+
+ /* SBC numbers are tracked by using the serial numbers
+* in the present request and complete events
+*/
+ uint64_t send_sbc;
+ uint64_t recv_sbc;
+
+ /* Last received UST/MSC values */
+ uint64_t ust, msc;
+
+ int num_back;
+
+ /* Serial numbers for tracking wait_for_msc events */
+ uint32_t send_msc_serial;
+ uint32_t recv_msc_serial;
#endif
#ifdef HAVE_WAYLAND_PLATFORM
diff --git a/src/egl/drivers/dri2/platform_x11.c
b/src/egl/drivers/dri2/platform_x11.c
index f8c4b70..a1445b2 100644
--- a/src/egl/drivers/dri2/platform_x11.c
+++ b/src/egl/drivers/dri2/platform_x11.c
@@ -188,6 +188,205 @@ get_xcb_screen(xcb_screen_iterator_t iter, int screen)
return NULL;
}
+/*
+ * Called by the XCB_PRESENT_COMPLETE_NOTIFY case.
+ */
+static void
+dri2_update_num_back(struct dri2_egl_surface *priv)
+{
+ priv-num_back = 1;
+ if (priv-flipping)
+ priv-num_back++;
+ if (priv-base.SwapInterval == 0)
+ priv-num_back++;
+}
+
+/*
+ * In the glx version there's some more clean up, but here we can just call
releaseBuffer().
+ */
+static void
+dri2_free_render_buffer(struct dri2_egl_surface *pdraw, __DRIbuffer *buffer)
+{
+ struct dri2_egl_display *dri2_dpy =
dri2_egl_display(pdraw-base.Resource.Display);
+
+ (*dri2_dpy-dri2-releaseBuffer)(dri2_dpy-dri_screen, buffer);
+}
+
+/*
+ * Process one Present event
+ */
+static void
+dri2_handle_present_event(struct dri2_egl_surface *priv,
xcb_present_generic_event_t *ge)
+{
+ switch (ge-evtype) {
+ case XCB_PRESENT_CONFIGURE_NOTIFY: {
+ xcb_present_configure_notify_event_t *ce = (void *) ge;
+
+ priv-base.Width = ce-width;
+ priv-base.Height = ce-height;
+ break;
+ }
+ case XCB_PRESENT_COMPLETE_NOTIFY
Re: [Mesa-dev] [RFC] egl: Add DRI3 support to the EGL backend.
Hi,
On pe, 2014-10-24 at 17:37 +, Emil Velikov wrote:
Hi Joonas,
On 22/10/14 18:17, Joonas Lahtinen wrote:
Hi,
This patch introduced DRI3 support to the EGL backend.
Patch is on top of current master. With the patch you can observe
reduced CPU stress when many glViewport calls are made.
Notice that the DRI3 extension is only exposed if the DRI3 interface is
found working too instead of just existing. So you need to enable DRI3
from all three; Mesa, X driver and X server, to get the benefit.
It seems that you introduce yet another dri extension
(DRIdri3Extension) which afaict should not be needed ?
Am I missing something here or did you get carried away by looking at
__DRIcoreExtension, __DRIdri2Extension and __DRIswrastExtension :P
The DRI3 (just like the USE_INVALIDATE extension) extension is needed
for the drivers to know that they do not need to invalidate buffers on
each glViewport call.
One could use USE_INVALIDATE extension for same functionality, but I see
it would not be very good idea as the naming vs. context would be
confusing.
DRI3 is exposed dynamically depending on if DRI3 is usable or not. The
USE_INVALIDATE is exposed as long as GLX is used. So that also makes
them bit different (in logic too, not just naming).
Regards, Joonas
Thanks
Emil
Regards, Joonas
PS. Will be traveling over the weekend, so will react to comments on
Monday.
From c945e777e0aaf77a5ec450cdec1cf4db89ef0c8d Mon Sep 17 00:00:00 2001
From: Joonas Lahtinen joonas.lahti...@linux.intel.com
Date: Wed, 22 Oct 2014 21:05:31 +0300
Subject: [PATCH] egl: Add DRI3 support to the EGL backend.
DRI3 support is needed to avoid excess buffer invalidations, because
the XCB interface doesn't supply that information through DRI2.
Signed-off-by: Daniel van der Wath danielx.j.van.der.w...@intel.com
Signed-off-by: Joonas Lahtinen joonas.lahti...@linux.intel.com
---
configure.ac|4 +
include/GL/internal/dri_interface.h |8 +
src/egl/drivers/dri2/egl_dri2.c |9 +
src/egl/drivers/dri2/egl_dri2.h | 39 +++-
src/egl/drivers/dri2/platform_x11.c | 340
++-
src/egl/main/Makefile.am|4 +
src/mesa/drivers/dri/common/dri_util.c |2 +
src/mesa/drivers/dri/common/dri_util.h |1 +
src/mesa/drivers/dri/i965/brw_context.c | 12 +-
9 files changed, 406 insertions(+), 13 deletions(-)
diff --git a/configure.ac b/configure.ac
index 0ed9325..308fddf 100644
--- a/configure.ac
+++ b/configure.ac
@@ -43,6 +43,7 @@ VDPAU_REQUIRED=0.4.1
WAYLAND_REQUIRED=1.2.0
XCB_REQUIRED=1.9.3
XCBDRI2_REQUIRED=1.8
+XCBDRI3_REQUIRED=1.8
XCBGLX_REQUIRED=1.8.1
XSHMFENCE_REQUIRED=1.1
XVMC_REQUIRED=1.0.6
@@ -1557,6 +1558,9 @@ for plat in $egl_platforms; do
x11)
PKG_CHECK_MODULES([XCB_DRI2], [x11-xcb xcb xcb-dri2 =
$XCBDRI2_REQUIRED xcb-xfixes])
+ if test x$enable_dri3 = xyes; then
+ PKG_CHECK_MODULES([XCB_DRI3], [xcb-dri3 =
$XCBDRI3_REQUIRED xcb-present])
+ fi
;;
drm)
diff --git a/include/GL/internal/dri_interface.h
b/include/GL/internal/dri_interface.h
index 8c5ceb9..3bd9d18 100644
--- a/include/GL/internal/dri_interface.h
+++ b/include/GL/internal/dri_interface.h
@@ -83,6 +83,7 @@ typedef struct __DRIswrastExtensionRec
__DRIswrastExtension;
typedef struct __DRIbufferRec __DRIbuffer;
typedef struct __DRIdri2ExtensionRec __DRIdri2Extension;
typedef struct __DRIdri2LoaderExtensionRec __DRIdri2LoaderExtension;
+typedef struct __DRIdri3ExtensionRec __DRIdri3Extension;
typedef struct __DRI2flushExtensionRec __DRI2flushExtension;
typedef struct __DRI2throttleExtensionRec __DRI2throttleExtension;
@@ -999,6 +1000,13 @@ struct __DRIdri2ExtensionRec {
__DRIcreateNewScreen2FunccreateNewScreen2;
};
+#define __DRI_DRI3 DRI_DRI3
+#define __DRI_DRI3_VERSION 1
+
+struct __DRIdri3ExtensionRec {
+ __DRIextension base;
+};
+
/**
* This extension provides functionality to enable various EGLImage
diff --git a/src/egl/drivers/dri2/egl_dri2.c
b/src/egl/drivers/dri2/egl_dri2.c
index 20a7243..6024cb3 100644
--- a/src/egl/drivers/dri2/egl_dri2.c
+++ b/src/egl/drivers/dri2/egl_dri2.c
@@ -55,6 +55,12 @@ const __DRIuseInvalidateExtension use_invalidate = {
.base = { __DRI_USE_INVALIDATE, 1 }
};
+#ifdef HAVE_DRI3
+const __DRIdri3Extension dri3_extension = {
+ .base = { __DRI_DRI3, 1 }
+};
+#endif
+
EGLint dri2_to_egl_attribute_map[] = {
0,
EGL_BUFFER_SIZE,/* __DRI_ATTRIB_BUFFER_SIZE */
@@ -600,6 +606,9 @@ dri2_create_screen(_EGLDisplay *disp)
if (strcmp(extensions[i]-name, __DRI2_CONFIG_QUERY) == 0) {
dri2_dpy
Re: [Mesa-dev] [RFC] egl: Add DRI3 support to the EGL backend.
Hi, Comments below. On ti, 2014-10-28 at 12:44 +0100, Axel Davy wrote: On 28/10/2014 10:19, Joonas Lahtinen wrote : Hi, On pe, 2014-10-24 at 17:37 +, Emil Velikov wrote: Hi Joonas, On 22/10/14 18:17, Joonas Lahtinen wrote: Hi, This patch introduced DRI3 support to the EGL backend. Patch is on top of current master. With the patch you can observe reduced CPU stress when many glViewport calls are made. Notice that the DRI3 extension is only exposed if the DRI3 interface is found working too instead of just existing. So you need to enable DRI3 from all three; Mesa, X driver and X server, to get the benefit. It seems that you introduce yet another dri extension (DRIdri3Extension) which afaict should not be needed ? Am I missing something here or did you get carried away by looking at __DRIcoreExtension, __DRIdri2Extension and __DRIswrastExtension :P The DRI3 (just like the USE_INVALIDATE extension) extension is needed for the drivers to know that they do not need to invalidate buffers on each glViewport call. One could use USE_INVALIDATE extension for same functionality, but I see it would not be very good idea as the naming vs. context would be confusing. DRI3 is exposed dynamically depending on if DRI3 is usable or not. The USE_INVALIDATE is exposed as long as GLX is used. So that also makes them bit different (in logic too, not just naming). Regards, Joonas Thanks Emil Hi, Looking at the patch it seems you are not really using DRI3, and you are just using the Present extension to listen when the window is resized. That seems quite true. I'll have to refer to the original author and ask what's with the DRI3 requirement. Best I know it's been done following the the GLX DRI3 backend. A complete implementation, that would use several back buffers and use the Present extension for swapbuffers instead of copying on the front buffer, would be preferable. (vsync/tear-free support + the advantages Present will bring to compositors when they can handle Present events and avoid useless copies) Since you use Present over the DRI2 backend, that explains why you have to use hacks to use invalidate differently. With complete DRI3/Present implementation, you wouldn't need these hacks. Like for the egl Wayland backend you would use several back buffers and invalidate only for a resize and after swapbuffers. Even with the current patch, invalidate only happens on resizes. So it should be a substantial advantage. I don't see it as a hack, as it only gets rid of the USE_INVALIDATE hack (intel_viewport call). Removing the DRI3 test parts, and changing the naming to refer to present extension, might be next steps. After I get to know why the DRI3 part was there originally. Regards, Joonas Axel Davy ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [RFC] egl: Add DRI3 support to the EGL backend.
Hi,
This patch introduced DRI3 support to the EGL backend.
Patch is on top of current master. With the patch you can observe
reduced CPU stress when many glViewport calls are made.
Notice that the DRI3 extension is only exposed if the DRI3 interface is
found working too instead of just existing. So you need to enable DRI3
from all three; Mesa, X driver and X server, to get the benefit.
Regards, Joonas
PS. Will be traveling over the weekend, so will react to comments on
Monday.
From c945e777e0aaf77a5ec450cdec1cf4db89ef0c8d Mon Sep 17 00:00:00 2001
From: Joonas Lahtinen joonas.lahti...@linux.intel.com
Date: Wed, 22 Oct 2014 21:05:31 +0300
Subject: [PATCH] egl: Add DRI3 support to the EGL backend.
DRI3 support is needed to avoid excess buffer invalidations, because
the XCB interface doesn't supply that information through DRI2.
Signed-off-by: Daniel van der Wath danielx.j.van.der.w...@intel.com
Signed-off-by: Joonas Lahtinen joonas.lahti...@linux.intel.com
---
configure.ac|4 +
include/GL/internal/dri_interface.h |8 +
src/egl/drivers/dri2/egl_dri2.c |9 +
src/egl/drivers/dri2/egl_dri2.h | 39 +++-
src/egl/drivers/dri2/platform_x11.c | 340 ++-
src/egl/main/Makefile.am|4 +
src/mesa/drivers/dri/common/dri_util.c |2 +
src/mesa/drivers/dri/common/dri_util.h |1 +
src/mesa/drivers/dri/i965/brw_context.c | 12 +-
9 files changed, 406 insertions(+), 13 deletions(-)
diff --git a/configure.ac b/configure.ac
index 0ed9325..308fddf 100644
--- a/configure.ac
+++ b/configure.ac
@@ -43,6 +43,7 @@ VDPAU_REQUIRED=0.4.1
WAYLAND_REQUIRED=1.2.0
XCB_REQUIRED=1.9.3
XCBDRI2_REQUIRED=1.8
+XCBDRI3_REQUIRED=1.8
XCBGLX_REQUIRED=1.8.1
XSHMFENCE_REQUIRED=1.1
XVMC_REQUIRED=1.0.6
@@ -1557,6 +1558,9 @@ for plat in $egl_platforms; do
x11)
PKG_CHECK_MODULES([XCB_DRI2], [x11-xcb xcb xcb-dri2 =
$XCBDRI2_REQUIRED xcb-xfixes])
+ if test x$enable_dri3 = xyes; then
+ PKG_CHECK_MODULES([XCB_DRI3], [xcb-dri3 =
$XCBDRI3_REQUIRED xcb-present])
+ fi
;;
drm)
diff --git a/include/GL/internal/dri_interface.h
b/include/GL/internal/dri_interface.h
index 8c5ceb9..3bd9d18 100644
--- a/include/GL/internal/dri_interface.h
+++ b/include/GL/internal/dri_interface.h
@@ -83,6 +83,7 @@ typedef struct __DRIswrastExtensionRec
__DRIswrastExtension;
typedef struct __DRIbufferRec __DRIbuffer;
typedef struct __DRIdri2ExtensionRec __DRIdri2Extension;
typedef struct __DRIdri2LoaderExtensionRec __DRIdri2LoaderExtension;
+typedef struct __DRIdri3ExtensionRec __DRIdri3Extension;
typedef struct __DRI2flushExtensionRec __DRI2flushExtension;
typedef struct __DRI2throttleExtensionRec __DRI2throttleExtension;
@@ -999,6 +1000,13 @@ struct __DRIdri2ExtensionRec {
__DRIcreateNewScreen2FunccreateNewScreen2;
};
+#define __DRI_DRI3 DRI_DRI3
+#define __DRI_DRI3_VERSION 1
+
+struct __DRIdri3ExtensionRec {
+ __DRIextension base;
+};
+
/**
* This extension provides functionality to enable various EGLImage
diff --git a/src/egl/drivers/dri2/egl_dri2.c b/src/egl/drivers/dri2/egl_dri2.c
index 20a7243..6024cb3 100644
--- a/src/egl/drivers/dri2/egl_dri2.c
+++ b/src/egl/drivers/dri2/egl_dri2.c
@@ -55,6 +55,12 @@ const __DRIuseInvalidateExtension use_invalidate = {
.base = { __DRI_USE_INVALIDATE, 1 }
};
+#ifdef HAVE_DRI3
+const __DRIdri3Extension dri3_extension = {
+ .base = { __DRI_DRI3, 1 }
+};
+#endif
+
EGLint dri2_to_egl_attribute_map[] = {
0,
EGL_BUFFER_SIZE,/* __DRI_ATTRIB_BUFFER_SIZE */
@@ -600,6 +606,9 @@ dri2_create_screen(_EGLDisplay *disp)
if (strcmp(extensions[i]-name, __DRI2_CONFIG_QUERY) == 0) {
dri2_dpy-config = (__DRI2configQueryExtension *) extensions[i];
}
+ if (strcmp(extensions[i]-name, __DRI_DRI3) == 0) {
+dri2_dpy-dri3 = (__DRIdri3Extension *) extensions[i];
+ }
}
} else {
assert(dri2_dpy-swrast);
diff --git a/src/egl/drivers/dri2/egl_dri2.h b/src/egl/drivers/dri2/egl_dri2.h
index 52f05fb..d8713df 100644
--- a/src/egl/drivers/dri2/egl_dri2.h
+++ b/src/egl/drivers/dri2/egl_dri2.h
@@ -35,6 +35,11 @@
#include X11/Xlib-xcb.h
#endif
+#ifdef HAVE_DRI3
+#include xcb/dri3.h
+#include xcb/present.h
+#endif
+
#ifdef HAVE_WAYLAND_PLATFORM
#include wayland-client.h
#include wayland-egl-priv.h
@@ -74,6 +79,7 @@
#include eglimage.h
#define ARRAY_SIZE(a) (sizeof(a) / sizeof((a)[0]))
+#define DRI2_EGL_SURFACE_NUM_BUFFERS 5
struct wl_buffer;
@@ -150,12 +156,17 @@ struct dri2_egl_display
int dri2_major;
int dri2_minor;
+#ifdef HAVE_DRI3
+ int dri3_major;
+ int dri3_minor;
+#endif
__DRIscreen *dri_screen;
int
