On 2018-01-24 06:35 PM, Mario Kleiner wrote: > On 01/22/2018 07:01 PM, Michel Dänzer wrote: >> On 2018-01-22 03:14 AM, Mario Kleiner wrote: >>> Ok, 3rd revision, now with per-x-screen drmmode_crtc_funcs rec >>> and set_gamma = NULL in the depth 30 case. Also back to Fredrik's >>> original exa 10 bit patch, just with his signed-off tacked on. >>> >>> Tested with single and dual x-screen, depth 24, depth 30 and mixed >>> 24 and 30 on separate x-screens. Also tested against current tip >>> of ati-ddx master. >> >> Thanks for the thorough testing. >> >> The patches look mostly good now, apart from some cosmetic issues (lines >> shouldn't be unnecessarily shorter than 72 columns, and acronyms should >> be spelled in all upper case, in the commit logs), but I can live with >> those. >> >> However, I gave the patches a quick spin (without a 30-bit capable >> monitor though) on the Turks card that's currently sitting in my >> development machine, and I noticed that Xorg crashes on shutdown because >> the XvMC extension failed to initialize. Can you take a look? > > It's missing a definition to allow init on a depth 30 screen. I've fixed > that in a patch i just sent out, so now the extension initializes. Both patches merged, thanks! > It failed here as well, but never resulted in a crash for me with my > servers up to 1.19.5. You fixed a bug in the server causing the crash, > but maybe it makes still sense for older unfixed servers to prevent > trouble by initializing the extension, or instead disabling it on a > depth 30 screen. > > With that patch it exposes support, but then the two apps i could get to > use XvMC at all with .mpg movies (mplayer and xine) fail to display on > depth 30 for reasons that seem to be unrelated to XvMC. > > The tests for mesa's gallium/state_trackers/xvmc/test sort of work > though, although with some false color. After many hours of poking > around, all i know is that i don't understand how XvMC is wired up? > > Having the extension enabled in the ddx is needed for any test to work. > But when i set a breakpoint on EVERGREENDisplayTexturedVideo() in > evergreen_textured_videofuncs.c, that breakpoint is never hit, even in > depth 24 when XvMC tests work, so i'm not sure if i'm looking at mostly > unused code there? I guess so. :) >>> I am also currently looking into broken pageflipping with the ati-ddx >>> under DRI2. This seems to be due to recent changes in the drmAddFB() >>> calls to no longer pass in the depth/bpp of the x-screens root window >>> pixmap, but instead the depth/bpp of the pixmap that should be >>> flipped onto the scanout. On DRI3 this behaves as i'd expect, but >>> on DRI2 the passed in pixmaps don't seem to have depth 24 on a depth >>> 24 screen or depth 30 on a depth 30 screen, like the root windows, >>> but instead the pixmaps come in at depth 32. That leads the pagelip >>> ioctl to fail, with the kernel complaining that "Page flip is not >>> allowed to change frame buffer format." >>> >>> If i hack mesa gallium/st's dri2_drawable_get_buffers() to avoid >>> use of the DRI2GetBuffersWithFormat() request and instead use the >>> older DRI2GetBuffers() request, then pageflips work again, as the >>> old request derives the buffers depth from the glx drawables depth, >>> which is 24 or 30 for a depth 24 or depth 30 x-screen, so a better >>> match. >> >> Hmm. Can you track down where the "format" value in >> radeon_dri2_create_buffer2 comes from? It's supposed to be the depth. > > It is the depth that comes from gallium/state_trackers/dri/dri2.c: > dri2_drawable_get_buffers(), where the switch-case statement translates > pipe_format into depth values for the DRI2GetBuffersWithFormat request > under DRI2 (originally introduced in commit 576161289 to transmit depth > instead of bpp). We have a case statement that translates RGBA8888 and > BGRA1010102 formats into depth 32. > > It only happens if a client wants a fbconfig with alpha channel, for > destination alpha blending etc., as my application happens to. If you > only run glxgears or a desktop compositor like kde-5's, it won't happen > as they only use bgrx configs without alpha. I sent out a patch which should fix this, please test. -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer
