Re: Re: [RFC] Fix attempt for Mesa + X-Server 1.20 + modesetting-ddx hangs on KDE5.

[Michel Dänzer]

On 2018-05-08 06:41 PM, Adam Jackson wrote:
> On Fri, 2018-05-04 at 15:45 +0200, Mario Kleiner wrote:
> 
>> The real problem, if i understand it correctly, is the way the life-time
>> of dri3_drawables and loader_dri3_drawables is managed atm. by Mesa's
>> bindContext() functions. Whenever glXMakeCurrent() etc. are called to
>> assign new/different GLXDrawables to the same context (ie. one context
>> reused for drawing into many different drawables, as opposed to using
>> one dedicated context for each drawable), we destroy the underlying
>> DRIDrawables/dri3_drawables_loader_dri3_drawables and they lose all
>> state wrt. pending bufferswaps, msc, sbc, ust.
> 
> That's utterly, utterly, utterly broken.
> 
>> Therefore one of these patches is either a good enough fix for the KDE
>> hang problems atm. or a diagnosis of the problem as a starting point for
>> brighter people to deal with the root cause ;-)
> 
> I'll see what I can come up with. I'm not sure there's a great fix for
> this that doesn't involve a few more roundtrips at MakeCurrent time,
> since we can lose drawables asynchronously, but such is life.

I had an idea, at least for SBC:

In dri3_destroy_drawable, store the drawable's send_sbc value in a hash
table (keyed on the XID) in struct dri3_screen. Then in
dri3_create_drawable, if there's an entry for the drawable's XID in the
hash table, initialize send_sbc and recv_sbc to that.

If nobody beats me to it, I'll try this tomorrow.


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH xscope] Improve help and manpage.

[Walter Harms]

> Alan Coopersmith  hat am 6. Mai 2018 um 04:35
> geschrieben:
> 
> 
> On 07/ 8/13 05:40 PM, Stéphane Aulery wrote:
> > Add some details on the verbosity and options -T and -A.
> > These two options are not very clear to me.
> > 
> > Signed-off-by: Stéphane Aulery 
> > ---
> >  man/xscope.man |   89
> > +---
> >  scope.c|   33 -
> >  2 files changed, 84 insertions(+), 38 deletions(-)
> 
> Sorry for letting this get lost in our massive backlog, but I've finally
> gotten this pushed to git master:
> https://cgit.freedesktop.org/xorg/app/xscope/commit/?id=51c3ab198e6fa440961d02f6af13b1d73d698101
> 
> I've also pushed a followup to fix up the NAS protocol options (-A, -n, -a),
> though I'm not sure anyone still uses that protocol:
> https://cgit.freedesktop.org/xorg/app/xscope/commit/?id=f7af80a0ad7807e35f426a2377d7312b415c9d68
> 
> Even after digging through the code, I can't explain why the -T option exists
> - it just seems to add 0x20 to every character code before printing strings.
> Maybe someone else can figure that out.
> 

mybe this from wikipedia can help:

"In typography, letter-spacing, usually called tracking by typographers, 
refers to a consistent degree of increase (or sometimes decrease) of space 
between letters to affect density in a line or block of text."

T= tracking

most likely that was intended for printing.

re,
 wh

> Thanks for the patch!
> 
> -- 
>   -Alan Coopersmith-   alan.coopersm...@oracle.com
>Oracle Solaris Engineering - https://blogs.oracle.com/alanc
> ___
> xorg-devel@lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: https://lists.x.org/mailman/listinfo/xorg-devel
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH xserver] modesetting: set gbm as dependency in meson build

[Adam Jackson]

On Wed, 2018-05-02 at 21:32 +0200, Roman Gilg wrote:
> Modifiers support needs gbm as a dependency. Without setting the dependency
> included headers are not found reliably and the build might fail if the
> headers are not placed in the default system include paths.
> 
> Signed-off-by: Roman Gilg 

Merged, thanks:

remote: I: patch #219982 updated using rev 
9d628ee5facf6318368d5b8ca181e083adcffe8c.
remote: I: 1 patch(es) updated to state Accepted.
To ssh://git.freedesktop.org/git/xorg/xserver
   0445705a8b..9d628ee5fa  master -> master

- ajax
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [RFC] Fix attempt for Mesa + X-Server 1.20 + modesetting-ddx hangs on KDE5.

[Adam Jackson]

On Fri, 2018-05-04 at 15:45 +0200, Mario Kleiner wrote:

> The real problem, if i understand it correctly, is the way the life-time
> of dri3_drawables and loader_dri3_drawables is managed atm. by Mesa's
> bindContext() functions. Whenever glXMakeCurrent() etc. are called to
> assign new/different GLXDrawables to the same context (ie. one context
> reused for drawing into many different drawables, as opposed to using
> one dedicated context for each drawable), we destroy the underlying
> DRIDrawables/dri3_drawables_loader_dri3_drawables and they lose all
> state wrt. pending bufferswaps, msc, sbc, ust.

That's utterly, utterly, utterly broken.

> Therefore one of these patches is either a good enough fix for the KDE
> hang problems atm. or a diagnosis of the problem as a starting point for
> brighter people to deal with the root cause ;-)

I'll see what I can come up with. I'm not sure there's a great fix for
this that doesn't involve a few more roundtrips at MakeCurrent time,
since we can lose drawables asynchronously, but such is life.

- ajax
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH xserver 0/4] Final cleanups and polish

[Adam Jackson]

On Mon, 2018-05-07 at 17:21 -0400, Adam Jackson wrote:
> 1/4 is the only vaguely contentious one, the rest should all be
> obviously correct.

Merged these:

remote: I: patch #221270 updated using rev 
cc66777d85f3509b0f9dfc9210d0a0415a2a388d.
remote: I: patch #221271 updated using rev 
b9f415cbad47412bfb218cf7375d0c2856a27d1b.
remote: I: patch #221272 updated using rev 
9f21872ad8591c1f6f8ea1e466f212c3582d3c33.
remote: I: patch #221273 updated using rev 
0445705a8bbf76a5532997823ea8110daff26c6d.
remote: I: 4 patch(es) updated to state Accepted.
To ssh://git.freedesktop.org/git/xorg/xserver
   b23a0e4ded..0445705a8b  master -> master

- ajax
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH xserver 6/6] xwayland: Fix a 32-bit build warning.

[Adam Jackson]

On Mon, 2018-05-07 at 16:46 -0700, Eric Anholt wrote:
> Signed-off-by: Eric Anholt 

Merged, thanks:

remote: I: patch #221286 updated using rev 
e1ccd0fa0e1081edf8a2c69ce6e8e3f67a4aecba.
remote: I: patch #221285 updated using rev 
4ec02b573ef2424965d7ce2d33d150ddb92ec544.
remote: I: patch #221288 updated using rev 
5e86484a183f051d7a8a67ea13c23a3d6e69e76b.
remote: I: patch #221290 updated using rev 
4c754b01fafc4b042d9918c05a71157f6fa6392a.
remote: I: patch #221287 updated using rev 
ef95331603ff31d3643360c399b3865db5b0b97d.
remote: I: patch #221289 updated using rev 
b23a0e4ded62500f2c248f23962ff5ac718467e6.
remote: I: 6 patch(es) updated to state Accepted.
To ssh://git.freedesktop.org/git/xorg/xserver
   cf838f5ca8..b23a0e4ded  master -> master

- ajax
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH xserver 1/4] xwayland: Don't create a "fake" crtc for Present

[Roman Gilg]

Thanks for looking into this. You are right that the fake CRTCs are
not needed. The reason I introduced them in the past was that with a
change (reset) of the msc counter the client might want to see a
different CRTC as well. But with the persistent window structs the
CRTC stays the same all the time and the msc counter is monotonically
counting for a window so there should be no need to have different
CRTCs for different msc counters.

Tested the patch in a Plasma session with VLC and Skype.

Tested-by: Roman Gilg 
Reviewed-by: Roman Gilg 

On Mon, May 7, 2018 at 11:21 PM, Adam Jackson  wrote:
> We probably don't want a fake crtc to be visible to clients, and we
> definitely don't want to generate events every time we create such a
> fake (which would happen as a side effect from RRCrtcCreate hitting
> RRTellChanged). As it happens we're not actually using that crtc for
> anything because xwayland doesn't store any state on the crtc object,
> so it suffices to use the real crtc for the screen.
>
> Signed-off-by: Adam Jackson 
> ---
>  hw/xwayland/xwayland-present.c | 21 -
>  hw/xwayland/xwayland.h |  1 -
>  2 files changed, 4 insertions(+), 18 deletions(-)
>
> diff --git a/hw/xwayland/xwayland-present.c b/hw/xwayland/xwayland-present.c
> index ae9f47ebae..b5ae80dcf0 100644
> --- a/hw/xwayland/xwayland-present.c
> +++ b/hw/xwayland/xwayland-present.c
> @@ -51,14 +51,10 @@ xwl_present_window_get_priv(WindowPtr window)
>  struct xwl_present_window *xwl_present_window = 
> xwl_present_window_priv(window);
>
>  if (xwl_present_window == NULL) {
> -ScreenPtr screen = window->drawable.pScreen;
> -
>  xwl_present_window = calloc (1, sizeof (struct xwl_present_window));
>  if (!xwl_present_window)
>  return NULL;
>
> -xwl_present_window->crtc_fake = RRCrtcCreate(screen,
> - xwl_present_window);
>  xwl_present_window->window = window;
>  xwl_present_window->msc = 1;
>  xwl_present_window->ust = GetTimeInMicros();
> @@ -131,8 +127,6 @@ xwl_present_cleanup(WindowPtr window)
>  if (xwl_window && xwl_window->present_window == window)
>  xwl_window->present_window = NULL;
>
> -RRCrtcDestroy(xwl_present_window->crtc_fake);
> -
>  if (xwl_present_window->frame_callback) {
>  wl_callback_destroy(xwl_present_window->frame_callback);
>  xwl_present_window->frame_callback = NULL;
> @@ -306,10 +300,13 @@ static RRCrtcPtr
>  xwl_present_get_crtc(WindowPtr present_window)
>  {
>  struct xwl_present_window *xwl_present_window = 
> xwl_present_window_get_priv(present_window);
> +rrScrPrivPtr rr_private;
> +
>  if (xwl_present_window == NULL)
>  return NULL;
>
> -return xwl_present_window->crtc_fake;
> +rr_private = rrGetScrPriv(present_window->drawable.pScreen);
> +return rr_private->crtcs[0];
>  }
>
>  static int
> @@ -342,9 +339,6 @@ xwl_present_queue_vblank(WindowPtr present_window,
>  if (!xwl_window)
>  return BadMatch;
>
> -if (xwl_present_window->crtc_fake != crtc)
> -return BadRequest;
> -
>  if (xwl_window->present_window &&
>  xwl_window->present_window != present_window)
>  return BadMatch;
> @@ -454,13 +448,6 @@ xwl_present_flip(WindowPtr present_window,
>  if (!xwl_window)
>  return FALSE;
>
> -/*
> - * Make sure the client doesn't try to flip to another crtc
> - * than the one created for 'xwl_window'.
> - */
> -if (xwl_present_window->crtc_fake != crtc)
> -return FALSE;
> -
>  present_box = RegionExtents(_window->winSize);
>  damage_box = RegionExtents(damage);
>
> diff --git a/hw/xwayland/xwayland.h b/hw/xwayland/xwayland.h
> index ce290d4909..25112e2cb8 100644
> --- a/hw/xwayland/xwayland.h
> +++ b/hw/xwayland/xwayland.h
> @@ -182,7 +182,6 @@ struct xwl_present_window {
>  WindowPtr window;
>  struct xorg_list link;
>
> -RRCrtcPtr crtc_fake;
>  uint64_t msc;
>  uint64_t ust;
>
> --
> 2.17.0
>
> ___
> xorg-devel@lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: https://lists.x.org/mailman/listinfo/xorg-devel
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [Mesa-dev] [PATCH 2/2] loader_dri3: Variant 2: Wait for pending swaps to complete before drawable_fini.

[Michel Dänzer]

On 2018-05-05 06:25 AM, Mario Kleiner wrote:
> On Sat, May 5, 2018 at 4:08 AM, Mike Lothian  wrote:
>> I definately saw the steam bug with patch 1 but not with plasmashell,
>> I started seeing it with patch 2 but it seemed to fix itself
> 
> I had two hangs of kwin_x11 within the last 6 hours when alt-tabbing
> between windows, where it got stuck in the
> loader_dri3_swapbuffer_barrier() from patch 1/2. Not sure how that is
> possible, or if the stacktrace was misleading, because i had to VT
> switch to a text console to attach the debugger and this might be just
> a side effect of that. But if it is true, then patch 1/2 would not be
> it. Also 1/2 has a potential performance impact, whereas 2/2 doesn't.
> However 2/2 would also need more work, as i can think of more complex
> scenarios where it would filter the wrong events, although not in the
> case of plasmashell or steam. Probably we'd need to sacrifice a few
> sbc bits in the Present events serial field to transport a unique tag
> for each incarnation of the loader_dri3_drawable, like a mini-hash of
> the draw->eid. Ugly ugly...

How about the below?

Idle notify events shouldn't need special treatment, since the pixmap
XIDs of the buffers will be different between loader_dri3_drawable
incarnations, aren't they?


This still leaves the issue that the SBC moves backwards, which could
theoretically result in hangs with apps using glXWaitForSbcOML. Fixing
that would probably require changing the loader_dri3_drawable lifetime
cycle, which would probably be very invasive, if feasible at all. Maybe
we don't need to care about that for the time being, until there's a
real world app running into it.


diff --git a/src/loader/loader_dri3_helper.c b/src/loader/loader_dri3_helper.c
index 6db8303d26d..f0ff2f07bde 100644
--- a/src/loader/loader_dri3_helper.c
+++ b/src/loader/loader_dri3_helper.c
@@ -370,9 +370,17 @@ dri3_handle_present_event(struct loader_dri3_drawable 
*draw,
* checking for wrap.
*/
   if (ce->kind == XCB_PRESENT_COMPLETE_KIND_PIXMAP) {
- draw->recv_sbc = (draw->send_sbc & 0xLL) | ce->serial;
- if (draw->recv_sbc > draw->send_sbc)
-draw->recv_sbc -= 0x1;
+ uint64_t recv_sbc = (draw->send_sbc & 0xLL) | 
ce->serial;
+
+ /* Only assume wraparound if that results in exactly the previous
+  * SBC + 1, otherwise ignore received SBC > sent SBC (those are
+  * probably from a previous loader_dri3_drawable instance) to avoid
+  * calculating bogus target MSC values in loader_dri3_swap_buffers_msc
+  */
+ if (recv_sbc <= draw->send_sbc)
+draw->recv_sbc = recv_sbc;
+ else if (recv_sbc == (draw->recv_sbc + 0x10001ULL))
+draw->recv_sbc = recv_sbc - 0x1ULL;

  /* When moving from flip to copy, we assume that we can allocate in
   * a more optimal way if we don't need to cater for the display


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel