Depth 30 enablement for ati-ddx + exa. Rev 3

Depth 30 enablement for ati-ddx + exa. Rev 3

[Mario Kleiner]

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.

In other news:

When testing against current master in dual-x-screen mode,
i found out that the current ati-ddx commit at the top 1fe8ca75974c52
"Keep track of how many SW cursors are visible on each screen"
breaks multi x-screen setups by screwing up cursor handling.

If i start such a config on KUbuntu 16.04.3 LTS with X-Server 1.19.3
and sddm login manager, the displays come up with the sddm login
prompt on the display of x-screen 0, and a black display on screen 1,
as expected. Mouse cursor starts off on screen 0. As long as i move
the cursor only on x-screen 0 all is fine. As soon as i try to move
the cursor to the display of x-screen 1, the cursor disappears, the
whole display freezes and top shows x-org consuming 100% of a cpu
core, apparently caught in an infinite loop. Attaching gdb suggests
it is calling the new cursor handling code of that commit in some
infinite loop.

I had a look at the commit and couldn't find anything obviously
wrong with it. But then i just learned enough about cursor handling
to understand that commits logic in the first place, and obviously
the machine disagrees with my judgment. Reverting that commit makes
everything work again on dual x-screen. Haven't tested on amdgpu-ddx
but if it has the same patch i'd expect the same breakage.

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.

thanks,
-mario

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 1/4] Define per x-screen individual drmmode_crtc_funcs

[Mario Kleiner]

This allows to en-/disable some functions depending
on individual screen settings.

Prep work for more efficient depth 30 support.

Suggested-by: Michel Dänzer <michel.daenzer@amd.com>
Signed-off-by: Mario Kleiner <mario.kleiner.de@gmail.com>
---
 src/drmmode_display.c | 14 ++++++++++----
 src/radeon.h          |  2 ++
 2 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/src/drmmode_display.c b/src/drmmode_display.c
index f55677f..b1c874c 100644
--- a/src/drmmode_display.c
+++ b/src/drmmode_display.c
@@ -1395,8 +1395,9 @@ drmmode_crtc_init(ScrnInfoPtr pScrn, drmmode_ptr drmmode, drmModeResPtr mode_res
 	xf86CrtcPtr crtc;
 	drmmode_crtc_private_ptr drmmode_crtc;
 	RADEONEntPtr pRADEONEnt = RADEONEntPriv(pScrn);
+	RADEONInfoPtr info = RADEONPTR(pScrn);
 
-	crtc = xf86CrtcCreate(pScrn, &drmmode_crtc_funcs);
+	crtc = xf86CrtcCreate(pScrn, &info->drmmode_crtc_funcs);
 	if (crtc == NULL)
 		return 0;
 
@@ -2531,11 +2532,16 @@ Bool drmmode_pre_init(ScrnInfoPtr pScrn, drmmode_ptr drmmode, int cpp)
 	xf86DrvMsgVerb(pScrn->scrnIndex, X_INFO, RADEON_LOGLEVEL_DEBUG,
 		       "%d crtcs needed for screen.\n", crtcs_needed);
 
+	/* Need per-screen drmmode_crtc_funcs, based on our global template,
+	 * so we can disable some functions, depending on screen settings.
+	 */
+	info->drmmode_crtc_funcs = drmmode_crtc_funcs;
+
 	if (info->r600_shadow_fb) {
 		/* Rotation requires hardware acceleration */
-		drmmode_crtc_funcs.shadow_allocate = NULL;
-		drmmode_crtc_funcs.shadow_create = NULL;
-		drmmode_crtc_funcs.shadow_destroy = NULL;
+		info->drmmode_crtc_funcs.shadow_allocate = NULL;
+		info->drmmode_crtc_funcs.shadow_create = NULL;
+		info->drmmode_crtc_funcs.shadow_destroy = NULL;
 	}
 
 	drmmode->count_crtcs = mode_res->count_crtcs;
diff --git a/src/radeon.h b/src/radeon.h
index 9658e02..5b71736 100644
--- a/src/radeon.h
+++ b/src/radeon.h
@@ -635,6 +635,8 @@ typedef struct {
 	SetSharedPixmapBackingProcPtr SavedSetSharedPixmapBacking;
     } glamor;
 #endif /* USE_GLAMOR */
+
+    xf86CrtcFuncsRec drmmode_crtc_funcs;
 } RADEONInfoRec, *RADEONInfoPtr;
 
 /* radeon_accel.c */
-- 
2.7.4

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 2/4] Skip xf86HandleColormaps() at color depth 30. (v2)

[Mario Kleiner]

The hardware gamma luts get bypassed at color
depth 30 anyway, so skip their setup.

v2: Also don't hook up drmmode_crtc_gamma_set()
    to .gamma_set for a screen with depth 30,
    so the server can spare itself the effort of
    updating and setting unused cluts.
    Suggested by Michel.

Signed-off-by: Mario Kleiner <mario.kleiner.de@gmail.com>
---
 src/drmmode_display.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/src/drmmode_display.c b/src/drmmode_display.c
index b1c874c..21cc3c9 100644
--- a/src/drmmode_display.c
+++ b/src/drmmode_display.c
@@ -2544,6 +2544,12 @@ Bool drmmode_pre_init(ScrnInfoPtr pScrn, drmmode_ptr drmmode, int cpp)
 		info->drmmode_crtc_funcs.shadow_destroy = NULL;
 	}
 
+	/* Hw gamma lut's are currently bypassed by the hw at color depth 30,
+	 * so spare the server the effort to compute and update the cluts.
+	 */
+	if (pScrn->depth == 30)
+		info->drmmode_crtc_funcs.gamma_set = NULL;
+
 	drmmode->count_crtcs = mode_res->count_crtcs;
 	xf86CrtcSetSizeRange(pScrn, 320, 200, mode_res->max_width, mode_res->max_height);
 
@@ -2787,8 +2793,10 @@ Bool drmmode_setup_colormap(ScreenPtr pScreen, ScrnInfoPtr pScrn)
 		       "Initializing kms color map\n");
 	if (!miCreateDefColormap(pScreen))
 	    return FALSE;
-	/* all radeons support 10 bit CLUTs */
-	if (!xf86HandleColormaps(pScreen, 256, 10,
+
+	/* All radeons support 10 bit CLUTs. They get bypassed at depth 30. */
+	if (pScrn->depth != 30 &&
+	    !xf86HandleColormaps(pScreen, 256, 10,
 				 NULL, NULL,
 				 CMAP_PALETTED_TRUECOLOR
 #if 0 /* This option messes up text mode! (eich@suse.de) */
-- 
2.7.4

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 3/4] exa: Accelerate ARGB2101010 pictures

[Mario Kleiner]

From: Fredrik Höglund <fredrik@kde.org>

Tested by Mario on a Radeon HD 4000 series for the r600
exa path, and on Radeon HD 5770 for the evergreen exa
path.

Signed-off-by: Fredrik Höglund <fredrik@kde.org>
Reviewed-and-Tested-by: Mario Kleiner <mario.kleiner.de@gmail.com>
---
 src/evergreen_exa.c | 18 ++++++++++++++++++
 src/r600_exa.c      | 18 ++++++++++++++++++
 2 files changed, 36 insertions(+)

diff --git a/src/evergreen_exa.c b/src/evergreen_exa.c
index 90c9b6d..8584813 100644
--- a/src/evergreen_exa.c
+++ b/src/evergreen_exa.c
@@ -668,6 +668,10 @@ struct formatinfo {
 };
 
 static struct formatinfo EVERGREENTexFormats[] = {
+    {PICT_a2r10g10b10,	FMT_2_10_10_10},
+    {PICT_x2r10g10b10,	FMT_2_10_10_10},
+    {PICT_a2b10g10r10,	FMT_2_10_10_10},
+    {PICT_x2b10g10r10,	FMT_2_10_10_10},
     {PICT_a8r8g8b8,	FMT_8_8_8_8},
     {PICT_x8r8g8b8,	FMT_8_8_8_8},
     {PICT_a8b8g8r8,	FMT_8_8_8_8},
@@ -723,6 +727,12 @@ static uint32_t EVERGREENGetBlendCntl(int op, PicturePtr pMask, uint32_t dst_for
 static Bool EVERGREENGetDestFormat(PicturePtr pDstPicture, uint32_t *dst_format)
 {
     switch (pDstPicture->format) {
+    case PICT_a2r10g10b10:
+    case PICT_x2r10g10b10:
+    case PICT_a2b10g10r10:
+    case PICT_x2b10g10r10:
+	*dst_format = COLOR_2_10_10_10;
+	break;
     case PICT_a8r8g8b8:
     case PICT_x8r8g8b8:
     case PICT_a8b8g8r8:
@@ -891,6 +901,7 @@ static Bool EVERGREENTextureSetup(PicturePtr pPict, PixmapPtr pPix,
 
     /* component swizzles */
     switch (pPict->format) {
+    case PICT_a2r10g10b10:
     case PICT_a1r5g5b5:
     case PICT_a8r8g8b8:
 	pix_r = SQ_SEL_Z; /* R */
@@ -898,12 +909,14 @@ static Bool EVERGREENTextureSetup(PicturePtr pPict, PixmapPtr pPix,
 	pix_b = SQ_SEL_X; /* B */
 	pix_a = SQ_SEL_W; /* A */
 	break;
+    case PICT_a2b10g10r10:
     case PICT_a8b8g8r8:
 	pix_r = SQ_SEL_X; /* R */
 	pix_g = SQ_SEL_Y; /* G */
 	pix_b = SQ_SEL_Z; /* B */
 	pix_a = SQ_SEL_W; /* A */
 	break;
+    case PICT_x2b10g10r10:
     case PICT_x8b8g8r8:
 	pix_r = SQ_SEL_X; /* R */
 	pix_g = SQ_SEL_Y; /* G */
@@ -922,6 +935,7 @@ static Bool EVERGREENTextureSetup(PicturePtr pPict, PixmapPtr pPix,
 	pix_b = SQ_SEL_W; /* B */
 	pix_a = SQ_SEL_1; /* A */
 	break;
+    case PICT_x2r10g10b10:
     case PICT_x1r5g5b5:
     case PICT_x8r8g8b8:
     case PICT_r5g6b5:
@@ -1425,6 +1439,8 @@ static Bool EVERGREENPrepareComposite(int op, PicturePtr pSrcPicture,
     cb_conf.surface = accel_state->dst_obj.surface;
 
     switch (pDstPicture->format) {
+    case PICT_a2r10g10b10:
+    case PICT_x2r10g10b10:
     case PICT_a8r8g8b8:
     case PICT_x8r8g8b8:
     case PICT_a1r5g5b5:
@@ -1432,6 +1448,8 @@ static Bool EVERGREENPrepareComposite(int op, PicturePtr pSrcPicture,
     default:
 	cb_conf.comp_swap = 1; /* ARGB */
 	break;
+    case PICT_a2b10g10r10:
+    case PICT_x2b10g10r10:
     case PICT_a8b8g8r8:
     case PICT_x8b8g8r8:
 	cb_conf.comp_swap = 0; /* ABGR */
diff --git a/src/r600_exa.c b/src/r600_exa.c
index 22d4b31..c69b8fc 100644
--- a/src/r600_exa.c
+++ b/src/r600_exa.c
@@ -727,6 +727,10 @@ struct formatinfo {
 };
 
 static struct formatinfo R600TexFormats[] = {
+    {PICT_a2r10g10b10,	FMT_2_10_10_10},
+    {PICT_x2r10g10b10,	FMT_2_10_10_10},
+    {PICT_a2b10g10r10,	FMT_2_10_10_10},
+    {PICT_x2b10g10r10,	FMT_2_10_10_10},
     {PICT_a8r8g8b8,	FMT_8_8_8_8},
     {PICT_x8r8g8b8,	FMT_8_8_8_8},
     {PICT_a8b8g8r8,	FMT_8_8_8_8},
@@ -782,6 +786,12 @@ static uint32_t R600GetBlendCntl(int op, PicturePtr pMask, uint32_t dst_format)
 static Bool R600GetDestFormat(PicturePtr pDstPicture, uint32_t *dst_format)
 {
     switch (pDstPicture->format) {
+    case PICT_a2r10g10b10:
+    case PICT_x2r10g10b10:
+    case PICT_a2b10g10r10:
+    case PICT_x2b10g10r10:
+	*dst_format = COLOR_2_10_10_10;
+	break;
     case PICT_a8r8g8b8:
     case PICT_x8r8g8b8:
     case PICT_a8b8g8r8:
@@ -906,6 +916,7 @@ static Bool R600TextureSetup(PicturePtr pPict, PixmapPtr pPix,
 
     /* component swizzles */
     switch (pPict->format) {
+    case PICT_a2r10g10b10:
     case PICT_a1r5g5b5:
     case PICT_a8r8g8b8:
 	pix_r = SQ_SEL_Z; /* R */
@@ -913,12 +924,14 @@ static Bool R600TextureSetup(PicturePtr pPict, PixmapPtr pPix,
 	pix_b = SQ_SEL_X; /* B */
 	pix_a = SQ_SEL_W; /* A */
 	break;
+    case PICT_a2b10g10r10:
     case PICT_a8b8g8r8:
 	pix_r = SQ_SEL_X; /* R */
 	pix_g = SQ_SEL_Y; /* G */
 	pix_b = SQ_SEL_Z; /* B */
 	pix_a = SQ_SEL_W; /* A */
 	break;
+    case PICT_x2b10g10r10:
     case PICT_x8b8g8r8:
 	pix_r = SQ_SEL_X; /* R */
 	pix_g = SQ_SEL_Y; /* G */
@@ -937,6 +950,7 @@ static Bool R600TextureSetup(PicturePtr pPict, PixmapPtr pPix,
 	pix_b = SQ_SEL_W; /* B */
 	pix_a = SQ_SEL_1; /* A */
 	break;
+    case PICT_x2r10g10b10:
     case PICT_x1r5g5b5:
     case PICT_x8r8g8b8:
     case PICT_r5g6b5:
@@ -1464,6 +1478,8 @@ static Bool R600PrepareComposite(int op, PicturePtr pSrcPicture,
     cb_conf.surface = accel_state->dst_obj.surface;
 
     switch (pDstPicture->format) {
+    case PICT_a2r10g10b10:
+    case PICT_x2r10g10b10:
     case PICT_a8r8g8b8:
     case PICT_x8r8g8b8:
     case PICT_a1r5g5b5:
@@ -1471,6 +1487,8 @@ static Bool R600PrepareComposite(int op, PicturePtr pSrcPicture,
     default:
 	cb_conf.comp_swap = 1; /* ARGB */
 	break;
+    case PICT_a2b10g10r10:
+    case PICT_x2b10g10r10:
     case PICT_a8b8g8r8:
     case PICT_x8b8g8r8:
 	cb_conf.comp_swap = 0; /* ABGR */
-- 
2.7.4

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 4/4] Support exa screen color depth 30 on Linux 3.16 and later. (v2)

[Mario Kleiner]

Linux 3.16's radeon-kms introduced 10 bpc fb support.
Currently, as of X-Server 1.19, this works with exa accel,
but not yet when using glamor acceleration.

v2: Style fixes, check for glamor first, as suggested
    by Michel.

Signed-off-by: Mario Kleiner <mario.kleiner.de@gmail.com>
---
 man/radeon.man   |  3 ++-
 src/radeon_kms.c | 17 +++++++++++++++++
 2 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/man/radeon.man b/man/radeon.man
index f4441c4..889355b 100644
--- a/man/radeon.man
+++ b/man/radeon.man
@@ -18,7 +18,8 @@ following features:
 .PD 0
 .TP 2
 \(bu
-Full support for 8-, 15-, 16- and 24-bit pixel depths;
+Full support for 8-, 15-, 16- and 24-bit pixel depths, and for 30-bit depth on Linux 3.16
+and later with EXA acceleration;
 .TP
 \(bu
 RandR 1.2 and RandR 1.3 support;
diff --git a/src/radeon_kms.c b/src/radeon_kms.c
index c1f885e..bac98cb 100644
--- a/src/radeon_kms.c
+++ b/src/radeon_kms.c
@@ -1250,6 +1250,7 @@ static Bool RADEONPreInitVisual(ScrnInfoPtr pScrn)
     case 15:
     case 16:
     case 24:
+    case 30:
 	break;
 
     default:
@@ -1765,6 +1766,22 @@ Bool RADEONPreInit_KMS(ScrnInfoPtr pScrn, int flags)
 
     if (!RADEONPreInitAccel_KMS(pScrn))              goto fail;
 
+    /* Depth 30 not yet supported under glamor. */
+    if (pScrn->depth == 30 && info->use_glamor) {
+	xf86DrvMsg(pScrn->scrnIndex, X_ERROR,
+		   "Given depth (%d) is not supported under GLAMOR accel. Select EXA.\n",
+		   pScrn->depth);
+	goto fail;
+    }
+
+    /* Depth 30 only supported since Linux 3.16 / kms driver minor version 39 */
+    if (pScrn->depth == 30 && info->dri2.pKernelDRMVersion->version_minor < 39) {
+	xf86DrvMsg(pScrn->scrnIndex, X_ERROR,
+		   "Given depth (%d) is not supported. Kernel too old. Needs Linux 3.16+\n",
+		   pScrn->depth);
+	goto fail;
+    }
+
     radeon_drm_queue_init();
 
     info->allowColorTiling2D = FALSE;
-- 
2.7.4

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: Depth 30 enablement for ati-ddx + exa. Rev 3

[Michel Dänzer]

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?


> When testing against current master in dual-x-screen mode,
> i found out that the current ati-ddx commit at the top 1fe8ca75974c52
> "Keep track of how many SW cursors are visible on each screen"
> breaks multi x-screen setups by screwing up cursor handling.

I stumbled over this myself the other day. There's an issue with the
(un)wrapping of miPointerSpriteFuncRec entries. Not sure yet what
exactly's wrong with it though. I'll look into it.


> 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.


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: Depth 30 enablement for ati-ddx + exa. Rev 3

[Michel Dänzer]

On 2018-01-22 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?

That turned out to be an xserver bug:
https://patchwork.freedesktop.org/patch/200379/

So I pushed this series, thanks!


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: Depth 30 enablement for ati-ddx + exa. Rev 3

[Mario Kleiner]

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. 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?

> 
> 
>> When testing against current master in dual-x-screen mode,
>> i found out that the current ati-ddx commit at the top 1fe8ca75974c52
>> "Keep track of how many SW cursors are visible on each screen"
>> breaks multi x-screen setups by screwing up cursor handling.
> 
> I stumbled over this myself the other day. There's an issue with the
> (un)wrapping of miPointerSpriteFuncRec entries. Not sure yet what
> exactly's wrong with it though. I'll look into it.
> 

Great, thanks.

> 
>> 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.

-mario
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: Depth 30 enablement for ati-ddx + exa. Rev 3

[Michel Dänzer]

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
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: Depth 30 enablement for ati-ddx + exa. Rev 3

[Mario Kleiner]

On 01/25/2018 05:06 PM, Michel Dänzer wrote:
> 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:
...
>> 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. :)

Good, then i don't need to look at that code anymore :).

...
>>
>> 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.

Yes, that patch "[PATCH xf86-video-ati] Always use screen depth/bpp for 
KMS framebuffers" makes sense to me and fixes it nicely, as tested under 
dri3 and dri2 with glamor and exa:

Reviewed-and-Tested-by: Mario Kleiner <mario.kleiner.de@gmail.com>

I assume amdgpu-ddx will need the same treatment, as "Use reference 
counting for tracking KMS framebuffer lifetimes" which caused the 
problem has been ported to amdgpu-ddx.

-mario
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: Depth 30 enablement for ati-ddx + exa. Rev 3

[Michel Dänzer]

On 2018-01-26 10:42 PM, Mario Kleiner wrote:
> On 01/25/2018 05:06 PM, Michel Dänzer wrote:
>> On 2018-01-24 06:35 PM, Mario Kleiner wrote:
>>>
>>> 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.
> 
> Yes, that patch "[PATCH xf86-video-ati] Always use screen depth/bpp for
> KMS framebuffers" makes sense to me and fixes it nicely, as tested under
> dri3 and dri2 with glamor and exa:
> 
> Reviewed-and-Tested-by: Mario Kleiner <mario.kleiner.de@gmail.com>

Thanks, pushed.


> I assume amdgpu-ddx will need the same treatment, as "Use reference
> counting for tracking KMS framebuffer lifetimes" which caused the
> problem has been ported to amdgpu-ddx.

Right. All applicable patches get ported between xf86-video-ati/amdgpu
in due time.


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [ANNOUNCE] xf86-video-amdgpu 18.0.0

[Mario Kleiner]

Hi Michel,

wrt. our conversation from ~ 1 month ago:

On 01/22/2018 07:01 PM, Michel Dänzer wrote:
> On 2018-01-22 03:14 AM, Mario Kleiner wrote:
> 
>> When testing against current master in dual-x-screen mode,
>> i found out that the current ati-ddx commit at the top 1fe8ca75974c52
>> "Keep track of how many SW cursors are visible on each screen"
>> breaks multi x-screen setups by screwing up cursor handling.
> 
> I stumbled over this myself the other day. There's an issue with the
> (un)wrapping of miPointerSpriteFuncRec entries. Not sure yet what
> exactly's wrong with it though. I'll look into it.
> 

Has the multi-x-screen breakage been fixed for amdgpu 18.0.0? I can't 
see a commit hinting at a fix, and i assume it was broken by the new HW 
cursor + pageflip logic, just as in the xf86-video-ati case? I can't 
test anything myself atm. as my only  amdgpu capable gpu has left me :(

Broken multi-x-screen from an official driver release ending in a distro 
would be a big problem for many vision scientists, as they depend on AMD 
+ dual-x-screen support for their work.

Thanks,
-mario
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [ANNOUNCE] xf86-video-amdgpu 18.0.0

[Michel Dänzer]

On 2018-03-04 08:25 AM, Mario Kleiner wrote:
> Hi Michel,
> 
> wrt. our conversation from ~ 1 month ago:
> 
> On 01/22/2018 07:01 PM, Michel Dänzer wrote:
>> On 2018-01-22 03:14 AM, Mario Kleiner wrote:
>>
>>> When testing against current master in dual-x-screen mode,
>>> i found out that the current ati-ddx commit at the top 1fe8ca75974c52
>>> "Keep track of how many SW cursors are visible on each screen"
>>> breaks multi x-screen setups by screwing up cursor handling.
>>
>> I stumbled over this myself the other day. There's an issue with the
>> (un)wrapping of miPointerSpriteFuncRec entries. Not sure yet what
>> exactly's wrong with it though. I'll look into it.
>>
> 
> Has the multi-x-screen breakage been fixed for amdgpu 18.0.0? I can't
> see a commit hinting at a fix, and i assume it was broken by the new HW
> cursor + pageflip logic, just as in the xf86-video-ati case? I can't
> test anything myself atm. as my only  amdgpu capable gpu has left me :(
> 
> Broken multi-x-screen from an official driver release ending in a distro
> would be a big problem for many vision scientists, as they depend on AMD
> + dual-x-screen support for their work.

Crap, I forgot about this issue. :(

I'll try fixing it ASAP and making a point release by the end of next
week. Apologies for any inconvenience.


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx