On 08/09/16 03:19 AM, Eric Anholt wrote: > Hans de Goede <hdego...@redhat.com> writes: > >> Hi, >> >> On 06-09-16 22:22, Eric Anholt wrote: >>> Hans de Goede <hdego...@redhat.com> writes: >>> >>>> When a GPU gets hotplugged while X is already running, glamor_egl_init() >>>> gets called and changes the current egl context at a point where glamor >>>> does not expect this. >>>> >>>> This causes glamor to e.g. crash in the next glamor_create_pixmap() call >>>> (caled through the master's screen->CreatePixmap), note this is not the >>>> only troublesome entry point I've seen other backtraces when using a >>>> composting window manager. >>>> >>>> Adding glamor_make_current calls to all entry points is quite expensive, >>>> so this commit consists of a miminal fix for this problem by restoring the >>>> original egl context when leaving glamor_egl_init() with an error, based >>>> on only usb gpu's getting hotplugged and they do not support glamor. >>> >>> I think the problem is just mismatching our lastGLContext with the >>> actual makecurrent state. Couldn't we just use glamor_make_current() >>> instead of our own eglMakeCurrent() call in this function? >> >> We cannot use glamor_make_current() in the problem scenario I'm >> trying to fix, because we do not need to set the egl context >> from the current screen, we need to not mess up the egl context >> which was current *before* glamor_egl_init() gets called for >> a different screen then the one which owns the current egl context. >> >> What happens is: >> >> 1.current egl context is the master GPUs / Screens egl context >> 2. USB GPU gets hotplugged >> 3. modesetting driver's PreInit() gets called on the >> USB-GPU's Screen >> 4. PreInit() calls glamor_egl_init() which creates a new >> egl context for the USB-GPU Screen, makes that current, >> then fails and sets the current egl context to NONE >> 6. master Screen->CreatePixmap gets called, this is >> glamor_make_pixmap, this fails (in an assert -> boom) >> because the current egl context is NONE > > There's this callchain: > > glamor_make_pixmap() -> > glamor_create_fbo() -> > glamor_create_tex() -> > glamor_make_current() > > And every entrypoint should be doing that before making any other GL > calls (if there are any missing, they must be fixed). The problem is > that glamor_egl_init() is making a GL context current without updating > lastGLcontext, so glamor_make_current() short-circuits. Calling > glamor_make_current() from glamor_egl_init() was my proposal for keeping > lastGLcontext updated. > > If we can't use that for some reason, we should just NULL out > lastGLcontext at our custom MakeCurrent time so that it's never out of > sync with reality.
Seconded. -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer
signature.asc
Description: OpenPGP digital signature
_______________________________________________ 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