xserver: Branch 'master' - 3 commits
GitLab Mirror
gitlab-mirror at kemper.freedesktop.org
Mon Sep 27 09:50:40 UTC 2021
hw/xfree86/drivers/modesetting/drmmode_display.c | 16 ---------------
hw/xfree86/modes/xf86RandR12.c | 24 +++++++++++++++++++----
2 files changed, 21 insertions(+), 19 deletions(-)
New commits:
commit 545fa90cbf37a4c18f013dabc9f3bfb8310a5a98
Author: Mario Kleiner <mario.kleiner.de at gmail.com>
Date: Tue Sep 14 07:52:33 2021 +0200
Revert "modesetting: Only use GAMMA_LUT if its size is 1024"
This reverts commit 617f591fc44e24413e1f91017d16734999bbbac1.
The problem described in that commit exists, but the two
preceeding commits with improvements to the servers RandR
code should avoid the mentioned problems while allowing the
use of GAMMA_LUT's instead of legacy gamma lut.
Use of legacy gamma lut's is not a good fix, because it will reduce
color output precision of gpu's with more than 1024 GAMMA_LUT
slots, e.g., AMD, ARM MALI and KOMEDA with 4096 slot luts,
and some Mediathek parts with 512 slot luts. On KOMEDA, legacy
lut's are completely unsupported by the kms driver, so gamma
correction gets disabled.
The situation is especially bad on Intel Icelake and later:
Use of legacy gamma tables will cause the kms driver to switch
to hardware legacy lut's with 256 slots, 8 bit wide, without
interpolation. This way color output precision is restricted to
8 bpc and any deep color / HDR output (10 bpc, fp16, fixed point 16)
becomes impossible. The latest Intel gen gpu's would have worse
color precision than parts which are more than 10 years old.
Signed-off-by: Mario Kleiner <mario.kleiner.de at gmail.com>
diff --git a/hw/xfree86/drivers/modesetting/drmmode_display.c b/hw/xfree86/drivers/modesetting/drmmode_display.c
index 4ad6170ca..1dd6ee9ef 100644
--- a/hw/xfree86/drivers/modesetting/drmmode_display.c
+++ b/hw/xfree86/drivers/modesetting/drmmode_display.c
@@ -2406,21 +2406,7 @@ drmmode_crtc_init(ScrnInfoPtr pScrn, drmmode_ptr drmmode, drmModeResPtr mode_res
drmmode_crtc->use_gamma_lut =
drmmode_crtc->props[DRMMODE_CRTC_GAMMA_LUT_SIZE].prop_id &&
- /* Only use GAMMA_LUT if the size is 1024.
- *
- * Currently, the modesetting driver always passes a sigRGBbits value of
- * 10 to xf86HandleColormaps. This causes it to create a RRCrtc gamma
- * ramp of 1024 elements. If DRMMODE_CRTC_GAMMA_LUT_SIZE is larger than
- * 1024 (for example on Intel GEN11, where it has a value of 262145)
- * then xf86RandR12CrtcSetGamma will read past the end of the RRCrtc's
- * gamma ramp when trying to copy it into the larger xf86Crtc gamma
- * ramp.
- *
- * Since the larger GEN11 gamma ramp size hasn't been tested, just
- * disable it for now. This will cause the modesetting driver to disable
- * the CTM property and use the legacy DRM gamma ramp rather than the
- * GAMMA_LUT property. */
- drmmode_crtc->props[DRMMODE_CRTC_GAMMA_LUT_SIZE].value == 1024 &&
+ drmmode_crtc->props[DRMMODE_CRTC_GAMMA_LUT_SIZE].value &&
xf86ReturnOptValBool(drmmode->Options, OPTION_USE_GAMMA_LUT, TRUE);
if (drmmode_crtc->use_gamma_lut &&
commit 7326e131df3d1373dd796d9e2d931e81a3536bad
Author: Mario Kleiner <mario.kleiner.de at gmail.com>
Date: Tue Sep 14 07:51:46 2021 +0200
xfree86: Let xf86RandR12CrtcComputeGamma() deal with non-power-of-2 sizes.
The assumption in the upsampling code was that the crtc->gamma_size
size of the crtc's gamma table is a power of two. This is true for
almost all current driver + gpu combos at least on Linux, with typical
sizes of 256, 512, 1024 or 4096 slots.
However, Intel Gen-11 Icelake and later are outliers, as their gamma
table has 2^18 + 1 slots, very big and not a power of two!
Try to make upsampling behave at least reasonable: Replicate the
last gamma value to fill up remaining crtc->gamma_red/green/blue
slots, which would normally stay uninitialized. This is important,
because while the intel display driver does not actually use all
2^18+1 values passed as part of a GAMMA_LUT, it does need the
very last slot, which would not get initialized by the old code.
This should hopefully create reasonable behaviour with Icelake+
but is untested on the actual Intel hw due to lack of suitable
hw.
Signed-off-by: Mario Kleiner <mario.kleiner.de at gmail.com>
diff --git a/hw/xfree86/modes/xf86RandR12.c b/hw/xfree86/modes/xf86RandR12.c
index df46fb32b..5b90f4bf1 100644
--- a/hw/xfree86/modes/xf86RandR12.c
+++ b/hw/xfree86/modes/xf86RandR12.c
@@ -1255,8 +1255,8 @@ xf86RandR12CrtcComputeGamma(xf86CrtcPtr crtc, LOCO *palette,
{
int gamma_slots;
unsigned shift;
- CARD32 value;
int i, j;
+ CARD32 value = 0;
for (shift = 0; (gamma_size << shift) < (1 << 16); shift++);
@@ -1273,6 +1273,10 @@ xf86RandR12CrtcComputeGamma(xf86CrtcPtr crtc, LOCO *palette,
for (j = 0; j < gamma_slots; j++)
crtc->gamma_red[i * gamma_slots + j] = value;
}
+
+ /* Replicate last value until end of crtc for gamma_size not a power of 2 */
+ for (j = i * gamma_slots; j < crtc->gamma_size; j++)
+ crtc->gamma_red[j] = value;
} else {
/* Downsampling of larger palette to smaller hw lut size */
for (i = 0; i < crtc->gamma_size; i++) {
@@ -1299,6 +1303,10 @@ xf86RandR12CrtcComputeGamma(xf86CrtcPtr crtc, LOCO *palette,
for (j = 0; j < gamma_slots; j++)
crtc->gamma_green[i * gamma_slots + j] = value;
}
+
+ /* Replicate last value until end of crtc for gamma_size not a power of 2 */
+ for (j = i * gamma_slots; j < crtc->gamma_size; j++)
+ crtc->gamma_green[j] = value;
} else {
/* Downsampling of larger palette to smaller hw lut size */
for (i = 0; i < crtc->gamma_size; i++) {
@@ -1325,6 +1333,10 @@ xf86RandR12CrtcComputeGamma(xf86CrtcPtr crtc, LOCO *palette,
for (j = 0; j < gamma_slots; j++)
crtc->gamma_blue[i * gamma_slots + j] = value;
}
+
+ /* Replicate last value until end of crtc for gamma_size not a power of 2 */
+ for (j = i * gamma_slots; j < crtc->gamma_size; j++)
+ crtc->gamma_blue[j] = value;
} else {
/* Downsampling of larger palette to smaller hw lut size */
for (i = 0; i < crtc->gamma_size; i++) {
commit 966f567432e91762382db09129f8fb4e2e434437
Author: Mario Kleiner <mario.kleiner.de at gmail.com>
Date: Tue Sep 14 07:40:49 2021 +0200
xfree86: Avoid crash in xf86RandR12CrtcSetGamma() memcpy path.
If randrp->palette_size is zero, the memcpy() path can read past the
end of the randr_crtc's gammaRed/Green/Blue tables if the hw crtc's
gamma_size is greater than the randr_crtc's gammaSize.
Avoid this by clamping the to-be-copied size to the smaller of both
sizes.
Note that during regular server startup, the memcpy() path is only
taken initially twice, but then a suitable palette is created for
use during a session. Therefore during an actual running X-Session,
the xf86RandR12CrtcComputeGamma() will be used, which makes sure that
data is properly up- or down-sampled for mismatching source and
target crtc gamma sizes.
This should avoid reading past randr_crtc gamma memory for gpu's
with big crtc->gamma_size, e.g., AMD/MALI/KOMEDA 4096 slots, or
Intel Icelake and later with 262145 slots.
Tested against modesetting-ddx and amdgpu-ddx under screen color
depth 24 (8 bpc) and 30 (10 bpc) to make sure that clamping happens
properly.
This is an alternative fix for the one attempted in commit
617f591fc44e24413e1f91017d16734999bbbac1.
Signed-off-by: Mario Kleiner <mario.kleiner.de at gmail.com>
diff --git a/hw/xfree86/modes/xf86RandR12.c b/hw/xfree86/modes/xf86RandR12.c
index d4651f4e8..df46fb32b 100644
--- a/hw/xfree86/modes/xf86RandR12.c
+++ b/hw/xfree86/modes/xf86RandR12.c
@@ -1358,6 +1358,7 @@ xf86RandR12CrtcSetGamma(ScreenPtr pScreen, RRCrtcPtr randr_crtc)
{
XF86RandRInfoPtr randrp = XF86RANDRINFO(pScreen);
xf86CrtcPtr crtc = randr_crtc->devPrivate;
+ int max_size = crtc->gamma_size;
if (crtc->funcs->gamma_set == NULL)
return FALSE;
@@ -1372,12 +1373,15 @@ xf86RandR12CrtcSetGamma(ScreenPtr pScreen, RRCrtcPtr randr_crtc)
randr_crtc->gammaBlue,
randr_crtc->gammaSize);
} else {
+ if (max_size > randr_crtc->gammaSize)
+ max_size = randr_crtc->gammaSize;
+
memcpy(crtc->gamma_red, randr_crtc->gammaRed,
- crtc->gamma_size * sizeof(crtc->gamma_red[0]));
+ max_size * sizeof(crtc->gamma_red[0]));
memcpy(crtc->gamma_green, randr_crtc->gammaGreen,
- crtc->gamma_size * sizeof(crtc->gamma_green[0]));
+ max_size * sizeof(crtc->gamma_green[0]));
memcpy(crtc->gamma_blue, randr_crtc->gammaBlue,
- crtc->gamma_size * sizeof(crtc->gamma_blue[0]));
+ max_size * sizeof(crtc->gamma_blue[0]));
}
xf86RandR12CrtcReloadGamma(crtc);
More information about the xorg-commit
mailing list