Depth 30 enablement for ati-ddx + exa.

Depth 30 enablement for ati-ddx + exa.

[Mario Kleiner]

Hi,

this is the patch series for ati-ddx to enable depth 30
color display under exa, which i used for testing the
corresponding mesa 30 bit rendering patches. Works well,
and photometer measurements confirmed we get 10 bpc per
color channel.

1/3 fixes a server crash if x-screen depth 30 is selected.

2/3 is actually Fredrik Hoeglunds work, which he sent out many
years ago to the mailing list. I've tested it without problems
on HD 5770 for the evergreen exa path, and on some HD 4000 series
(rv630 iirc) for the r600 exa path, and it seems to work nicely.

It also has a r-b by me as in "looks good to me", but i'm
not familiar enough with exa to really judge if it is complete or
if something for some more unusual case might be missing.

I guess it also might need a signed off by Fredrik?

3/3 switches depth 30 on for exa + Linux 3.16 and later, when depth
30 scanout was added to radeon-kms.

Thanks,
-mario

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

[PATCH 1/3] Fix xf86HandleColormaps() crash at color depth 30.

[Mario Kleiner]

The size of the X-Server pScreenPriv->PreAllocIndices
array allocated within xf86HandleColormaps() is given
by the given maxColors argument, but the range of
indices by which the PreAllocIndices array is indexed
in routines like CMapReinstallMap() seems to be up to
1023 on a 10 bpc / depth 30 screen, leading to an
out-of-bounds access and server crash.

Raising maxColors to 1024 fixes the crash at server
startup with X-Screen color depth 30.

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

diff --git a/src/drmmode_display.c b/src/drmmode_display.c
index 7ad3235..67db86e 100644
--- a/src/drmmode_display.c
+++ b/src/drmmode_display.c
@@ -2730,8 +2730,9 @@ 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,
+	if (!xf86HandleColormaps(pScreen, 1024, 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 2/3] 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.

Reviewed-and-Tested-by: Mario Kleiner <mario.kleiner.de@gmail.com>
Cc: Fredrik Höglund <fredrik@kde.org>
---
 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 3/3] Support exa screen color depth 30 on Linux 3.16 and later.

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

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..d888ac0 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 a9300d4..296bc6c 100644
--- a/src/radeon_kms.c
+++ b/src/radeon_kms.c
@@ -1248,6 +1248,7 @@ static Bool RADEONPreInitVisual(ScrnInfoPtr pScrn)
     case 15:
     case 16:
     case 24:
+    case 30:
 	break;
 
     default:
@@ -1763,6 +1764,22 @@ Bool RADEONPreInit_KMS(ScrnInfoPtr pScrn, int flags)
 
     if (!RADEONPreInitAccel_KMS(pScrn))              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;
+    }
+
+    /* 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;
+    }
+
     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: [PATCH 1/3] Fix xf86HandleColormaps() crash at color depth 30.

[Michel Dänzer]

On 2017-12-18 11:36 PM, Mario Kleiner wrote:
> The size of the X-Server pScreenPriv->PreAllocIndices
> array allocated within xf86HandleColormaps() is given
> by the given maxColors argument, but the range of
> indices by which the PreAllocIndices array is indexed
> in routines like CMapReinstallMap() seems to be up to
> 1023 on a 10 bpc / depth 30 screen, leading to an
> out-of-bounds access and server crash.
> 
> Raising maxColors to 1024 fixes the crash at server
> startup with X-Screen color depth 30.
> 
> Signed-off-by: Mario Kleiner <mario.kleiner.de@gmail.com>
> ---
>  src/drmmode_display.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/src/drmmode_display.c b/src/drmmode_display.c
> index 7ad3235..67db86e 100644
> --- a/src/drmmode_display.c
> +++ b/src/drmmode_display.c
> @@ -2730,8 +2730,9 @@ 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,
> +	if (!xf86HandleColormaps(pScreen, 1024, 10,
>  				 NULL, NULL,
>  				 CMAP_PALETTED_TRUECOLOR
>  #if 0 /* This option messes up text mode! (eich@suse.de) */
> 

The hardware CLUT isn't actually used at depth 30, so a better solution
would be to skip the xf86HandleColormaps call (and probably also set
xf86CrtcFuncsRec::gamma_set = NULL) for pScrn->depth == 30.


-- 
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: [PATCH 3/3] Support exa screen color depth 30 on Linux 3.16 and later.

[Michel Dänzer]

On 2017-12-18 11:36 PM, Mario Kleiner wrote:
> 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.
> 
> 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..d888ac0 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;

"EXA" (acronyms should be spelled with capital letters)


> @@ -1763,6 +1764,22 @@ Bool RADEONPreInit_KMS(ScrnInfoPtr pScrn, int flags)
>  
>      if (!RADEONPreInitAccel_KMS(pScrn))              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;
> +    }
> +
> +    /* Depth 30 not yet supported under glamor. */
> +    if ((pScrn->depth == 30) && info->use_glamor) {

There's no need for the nested parentheses in the tests.


> +	xf86DrvMsg(pScrn->scrnIndex, X_ERROR,
> +		   "Given depth (%d) is not supported under glamor accel. Select exa.\n",
> +		   pScrn->depth);
> +	goto fail;
> +    }

I'd check for glamor first, so that users with GCN cards don't upgrade
their kernel just to find out that it doesn't work with glamor anyway.


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

[Michel Dänzer]

On 2017-12-18 11:36 PM, Mario Kleiner wrote:
> 
> 2/3 is actually Fredrik Hoeglunds work, which he sent out many
> years ago to the mailing list. I've tested it without problems
> on HD 5770 for the evergreen exa path, and on some HD 4000 series
> (rv630 iirc) for the r600 exa path, and it seems to work nicely.
> 
> It also has a r-b by me as in "looks good to me", but i'm
> not familiar enough with exa to really judge if it is complete or
> if something for some more unusual case might be missing.
> 
> I guess it also might need a signed off by Fredrik?

Yes, that would be good.


-- 
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: [PATCH 1/3] Fix xf86HandleColormaps() crash at color depth 30.

[Mario Kleiner]

On 12/19/2017 09:58 AM, Michel Dänzer wrote:
> On 2017-12-18 11:36 PM, Mario Kleiner wrote:
>> The size of the X-Server pScreenPriv->PreAllocIndices
>> array allocated within xf86HandleColormaps() is given
>> by the given maxColors argument, but the range of
>> indices by which the PreAllocIndices array is indexed
>> in routines like CMapReinstallMap() seems to be up to
>> 1023 on a 10 bpc / depth 30 screen, leading to an
>> out-of-bounds access and server crash.
>>
>> Raising maxColors to 1024 fixes the crash at server
>> startup with X-Screen color depth 30.
>>
>> Signed-off-by: Mario Kleiner <mario.kleiner.de@gmail.com>
>> ---
>>   src/drmmode_display.c | 3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/drmmode_display.c b/src/drmmode_display.c
>> index 7ad3235..67db86e 100644
>> --- a/src/drmmode_display.c
>> +++ b/src/drmmode_display.c
>> @@ -2730,8 +2730,9 @@ 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,
>> +	if (!xf86HandleColormaps(pScreen, 1024, 10,
>>   				 NULL, NULL,
>>   				 CMAP_PALETTED_TRUECOLOR
>>   #if 0 /* This option messes up text mode! (eich@suse.de) */
>>
> 
> The hardware CLUT isn't actually used at depth 30, so a better solution
> would be to skip the xf86HandleColormaps call (and probably also set
> xf86CrtcFuncsRec::gamma_set = NULL) for pScrn->depth == 30.
> 

I know. Skipping the xf86HandleColormaps() function was the solution i 
first used, but then i thought we might want to keep it as 
future-proofing in case we'd implement hw gamma tables in the kms 
drivers for depth > 24 by switching to the piecewise linear hw gamma 
tables. Haven't looked yet what DC uses in amdgpu-kms, and what 
therefore makes sense for the amdgpu-ddx?

But no strong opinion, so whatever you prefer?

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

Re: [PATCH 2/3] exa: Accelerate ARGB2101010 pictures

[Fredrik Höglund]

On Monday 18 December 2017, Mario Kleiner wrote:
> 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.
> 
> Reviewed-and-Tested-by: Mario Kleiner <mario.kleiner.de@gmail.com>
> Cc: Fredrik Höglund <fredrik@kde.org>

I think the only question is whether SetSolidConsts() should support
2101010 formats. I believe solid pictures are always a8r8g8b8, so it
shouldn't matter in practice. But that function already supports all other
formats, so maybe the 2101010 ones should be added for the sake of
consistency.

But either way:

Signed-off-by: Fredrik Höglund <fredrik@kde.org>

> ---
>  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 */
> 

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

Re: [PATCH 1/3] Fix xf86HandleColormaps() crash at color depth 30.

[Michel Dänzer]

On 2017-12-22 02:49 AM, Mario Kleiner wrote:
> On 12/19/2017 09:58 AM, Michel Dänzer wrote:
>> On 2017-12-18 11:36 PM, Mario Kleiner wrote:
>>> The size of the X-Server pScreenPriv->PreAllocIndices
>>> array allocated within xf86HandleColormaps() is given
>>> by the given maxColors argument, but the range of
>>> indices by which the PreAllocIndices array is indexed
>>> in routines like CMapReinstallMap() seems to be up to
>>> 1023 on a 10 bpc / depth 30 screen, leading to an
>>> out-of-bounds access and server crash.
>>>
>>> Raising maxColors to 1024 fixes the crash at server
>>> startup with X-Screen color depth 30.
>>>
>>> Signed-off-by: Mario Kleiner <mario.kleiner.de@gmail.com>
>>> ---
>>>   src/drmmode_display.c | 3 ++-
>>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/src/drmmode_display.c b/src/drmmode_display.c
>>> index 7ad3235..67db86e 100644
>>> --- a/src/drmmode_display.c
>>> +++ b/src/drmmode_display.c
>>> @@ -2730,8 +2730,9 @@ 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,
>>> +    if (!xf86HandleColormaps(pScreen, 1024, 10,
>>>                    NULL, NULL,
>>>                    CMAP_PALETTED_TRUECOLOR
>>>   #if 0 /* This option messes up text mode! (eich@suse.de) */
>>>
>>
>> The hardware CLUT isn't actually used at depth 30, so a better solution
>> would be to skip the xf86HandleColormaps call (and probably also set
>> xf86CrtcFuncsRec::gamma_set = NULL) for pScrn->depth == 30.
>>
> 
> I know. Skipping the xf86HandleColormaps() function was the solution i
> first used, but then i thought we might want to keep it as
> future-proofing in case we'd implement hw gamma tables in the kms
> drivers for depth > 24 by switching to the piecewise linear hw gamma
> tables.

We can cross that bridge when we get there.


> Haven't looked yet what DC uses in amdgpu-kms, and what therefore makes
> sense for the amdgpu-ddx?

The legacy/non-atomic KMS gamma ramp has no effect at depth 30 with DC
as well, so the plan for xf86-video-amdgpu is what I described above.
Let's not diverge on this without good reason.


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