Bug#464353: How can we provide more info ?
On Thu, Mar 20, 2008 at 11:53:37 +0100, Michel Dänzer wrote:
>
> On Wed, 2008-03-19 at 22:13 +0100, Julien Cristau wrote:
> > from hw/xfree86/exa/examodule.c:
> > if (xf86IsOptionSet(pScreenPriv->options, EXAOPT_NO_COMPOSITE)) {
> > xf86DrvMsg(pScreen->myNum, X_INFO,
> > "EXA: Disabling Composite operation "
> > "(RENDER acceleration)\n");
> > pExaScr->info->CheckComposite = NULL;
> > pExaScr->info->PrepareComposite = NULL;
> > }
> >
> > That's broken. xf86IsOptionSet()'s return value is TRUE if the option
> > is specified in xorg.conf, whether it's set to true or false. I believe
> > this should use xf86ReturnOptValBool() or xf86GetOptValBool().
>
> Right. Option "Enable" "false" not working in the Monitor section may be
> due to the same or a similar reason. Julien, do you feel like going
> through the xserver tree and fixing these kinds of bugs? :)
>
All uses of this function were in hw/xfree86/common/xf86Config.c (where
it's used correctly), hw/xfree86/xaa/xaaInitAccel.c and
hw/xfree86/exa/examodule.c. The attached patches should fix the bug in
xaa and exa. Mind to have a look? :)
Cheers,
Julien
>From 6b9d2bb1f7f87acbf275027af9c2982e91e5faed Mon Sep 17 00:00:00 2001
From: Julien Cristau <jcristau@debian.org>
Date: Sat, 22 Mar 2008 17:28:48 +0100
Subject: [PATCH] exa: use xf86ReturnOptValBool instead of xf86IsOptionSet
The latter doesn't give you the option's value, it just tells you if
it's present in the configuration. So using Option "EXANoComposite" "false"
disabled composite acceleration.
---
hw/xfree86/exa/examodule.c | 13 +++++++------
1 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/hw/xfree86/exa/examodule.c b/hw/xfree86/exa/examodule.c
index 086639c..e18da0a 100644
--- a/hw/xfree86/exa/examodule.c
+++ b/hw/xfree86/exa/examodule.c
@@ -148,22 +148,23 @@ exaDDXDriverInit(ScreenPtr pScreen)
FALSE);
}
- if (xf86IsOptionSet(pScreenPriv->options, EXAOPT_NO_COMPOSITE)) {
- xf86DrvMsg(pScreen->myNum, X_INFO,
+ if (xf86ReturnOptValBool(pScreenPriv->options,
+ EXAOPT_NO_COMPOSITE, FALSE)) {
+ xf86DrvMsg(pScreen->myNum, X_CONFIG,
"EXA: Disabling Composite operation "
"(RENDER acceleration)\n");
pExaScr->info->CheckComposite = NULL;
pExaScr->info->PrepareComposite = NULL;
}
- if (xf86IsOptionSet(pScreenPriv->options, EXAOPT_NO_UTS)) {
- xf86DrvMsg(pScreen->myNum, X_INFO,
+ if (xf86ReturnOptValBool(pScreenPriv->options, EXAOPT_NO_UTS, FALSE)) {
+ xf86DrvMsg(pScreen->myNum, X_CONFIG,
"EXA: Disabling UploadToScreen\n");
pExaScr->info->UploadToScreen = NULL;
}
- if (xf86IsOptionSet(pScreenPriv->options, EXAOPT_NO_DFS)) {
- xf86DrvMsg(pScreen->myNum, X_INFO,
+ if (xf86ReturnOptValBool(pScreenPriv->options, EXAOPT_NO_DFS, FALSE)) {
+ xf86DrvMsg(pScreen->myNum, X_CONFIG,
"EXA: Disabling DownloadFromScreen\n");
pExaScr->info->DownloadFromScreen = NULL;
}
--
1.5.4.4
>From 4217ba0cf0c9bbea3774760e836ab372acf3237c Mon Sep 17 00:00:00 2001
From: Julien Cristau <jcristau@debian.org>
Date: Sat, 22 Mar 2008 17:31:08 +0100
Subject: [PATCH] xaa: use xf86ReturnOptValBool instead of xf86IsOptionSet
The latter doesn't return the option's value, just whether it's present
in the configuration.
---
hw/xfree86/xaa/xaaInitAccel.c | 57 ++++++++++++++++++++++++++---------------
1 files changed, 36 insertions(+), 21 deletions(-)
diff --git a/hw/xfree86/xaa/xaaInitAccel.c b/hw/xfree86/xaa/xaaInitAccel.c
index 1b7c154..53795f0 100644
--- a/hw/xfree86/xaa/xaaInitAccel.c
+++ b/hw/xfree86/xaa/xaaInitAccel.c
@@ -181,7 +181,7 @@ XAAInitAccel(ScreenPtr pScreen, XAAInfoRecPtr infoRec)
if(infoRec->SetupForScreenToScreenCopy &&
infoRec->SubsequentScreenToScreenCopy &&
- !xf86IsOptionSet(options, XAAOPT_SCREEN_TO_SCREEN_COPY)) {
+ !xf86ReturnOptValBool(options, XAAOPT_SCREEN_TO_SCREEN_COPY, FALSE)) {
HaveScreenToScreenCopy = TRUE;
} else {
infoRec->ScreenToScreenCopyFlags = 0;
@@ -192,10 +192,10 @@ XAAInitAccel(ScreenPtr pScreen, XAAInfoRecPtr infoRec)
/**** Solid Filled Rects ****/
if(infoRec->SetupForSolidFill && infoRec->SubsequentSolidFillRect &&
- !xf86IsOptionSet(options, XAAOPT_SOLID_FILL_RECT)) {
+ !xf86ReturnOptValBool(options, XAAOPT_SOLID_FILL_RECT, FALSE)) {
HaveSolidFillRect = TRUE;
if(infoRec->SubsequentSolidFillTrap &&
- !xf86IsOptionSet(options, XAAOPT_SOLID_FILL_TRAP))
+ !xf86ReturnOptValBool(options, XAAOPT_SOLID_FILL_TRAP, FALSE))
HaveSolidFillTrap = TRUE;
else
infoRec->SubsequentSolidFillTrap = NULL;
@@ -210,10 +210,11 @@ XAAInitAccel(ScreenPtr pScreen, XAAInfoRecPtr infoRec)
if(infoRec->SetupForSolidLine) {
if(infoRec->SubsequentSolidTwoPointLine &&
- !xf86IsOptionSet(options, XAAOPT_SOLID_TWO_POINT_LINE))
+ !xf86ReturnOptValBool(options,
+ XAAOPT_SOLID_TWO_POINT_LINE, FALSE))
HaveSolidTwoPointLine = TRUE;
if(infoRec->SubsequentSolidBresenhamLine &&
- !xf86IsOptionSet(options, XAAOPT_SOLID_BRESENHAM_LINE)) {
+ !xf86ReturnOptValBool(options, XAAOPT_SOLID_BRESENHAM_LINE, FALSE)) {
HaveSolidBresenhamLine = TRUE;
if(infoRec->SolidBresenhamLineErrorTermBits)
@@ -222,7 +223,8 @@ XAAInitAccel(ScreenPtr pScreen, XAAInfoRecPtr infoRec)
}
if(infoRec->SubsequentSolidHorVertLine &&
- !xf86IsOptionSet(options, XAAOPT_SOLID_HORVERT_LINE))
+ !xf86ReturnOptValBool(options,
+ XAAOPT_SOLID_HORVERT_LINE, FALSE))
HaveSolidHorVertLine = TRUE;
else if(HaveSolidTwoPointLine) {
infoRec->SubsequentSolidHorVertLine =
@@ -265,10 +267,14 @@ XAAInitAccel(ScreenPtr pScreen, XAAInfoRecPtr infoRec)
if(infoRec->SetupForMono8x8PatternFill &&
infoRec->SubsequentMono8x8PatternFillRect &&
- !xf86IsOptionSet(options, XAAOPT_MONO_8x8_PATTERN_FILL_RECT)) {
+ !xf86ReturnOptValBool(options,
+ XAAOPT_MONO_8x8_PATTERN_FILL_RECT,
+ FALSE)) {
HaveMono8x8PatternFillRect = TRUE;
if(infoRec->SubsequentMono8x8PatternFillTrap &&
- !xf86IsOptionSet(options, XAAOPT_MONO_8x8_PATTERN_FILL_TRAP))
+ !xf86ReturnOptValBool(options,
+ XAAOPT_MONO_8x8_PATTERN_FILL_TRAP,
+ FALSE))
HaveMono8x8PatternFillTrap = TRUE;
if(infoRec->Mono8x8PatternFillFlags &
@@ -318,10 +324,12 @@ XAAInitAccel(ScreenPtr pScreen, XAAInfoRecPtr infoRec)
if(infoRec->SetupForDashedLine && infoRec->DashPatternMaxLength) {
if(infoRec->SubsequentDashedTwoPointLine &&
- !xf86IsOptionSet(options, XAAOPT_DASHED_TWO_POINT_LINE))
+ !xf86ReturnOptValBool(options, XAAOPT_DASHED_TWO_POINT_LINE,
+ FALSE))
HaveDashedTwoPointLine = TRUE;
if(infoRec->SubsequentDashedBresenhamLine &&
- !xf86IsOptionSet(options, XAAOPT_DASHED_BRESENHAM_LINE)) {
+ !xf86ReturnOptValBool(options, XAAOPT_DASHED_BRESENHAM_LINE,
+ FALSE)) {
HaveDashedBresenhamLine = TRUE;
if(infoRec->DashedBresenhamLineErrorTermBits)
@@ -345,10 +353,11 @@ XAAInitAccel(ScreenPtr pScreen, XAAInfoRecPtr infoRec)
if(infoRec->SetupForColor8x8PatternFill &&
infoRec->SubsequentColor8x8PatternFillRect &&
- !xf86IsOptionSet(options, XAAOPT_COL_8x8_PATTERN_FILL_RECT)) {
+ !xf86ReturnOptValBool(options, XAAOPT_COL_8x8_PATTERN_FILL_RECT, FALSE)) {
HaveColor8x8PatternFillRect = TRUE;
if(infoRec->SubsequentColor8x8PatternFillTrap &&
- !xf86IsOptionSet(options, XAAOPT_COL_8x8_PATTERN_FILL_TRAP))
+ !xf86ReturnOptValBool(options, XAAOPT_COL_8x8_PATTERN_FILL_TRAP,
+ FALSE))
HaveColor8x8PatternFillTrap = TRUE;
else
infoRec->SubsequentColor8x8PatternFillTrap = NULL;
@@ -381,7 +390,8 @@ XAAInitAccel(ScreenPtr pScreen, XAAInfoRecPtr infoRec)
if(infoRec->SetupForCPUToScreenColorExpandFill &&
infoRec->ColorExpandBase &&
infoRec->SubsequentCPUToScreenColorExpandFill &&
- !xf86IsOptionSet(options, XAAOPT_CPU_TO_SCREEN_COL_EXP_FILL)) {
+ !xf86ReturnOptValBool(options, XAAOPT_CPU_TO_SCREEN_COL_EXP_FILL,
+ FALSE)) {
int dwordsNeeded = pScrn->virtualX;
infoRec->ColorExpandRange >>= 2; /* convert to DWORDS */
@@ -406,7 +416,9 @@ XAAInitAccel(ScreenPtr pScreen, XAAInfoRecPtr infoRec)
infoRec->SubsequentColorExpandScanline &&
infoRec->ScanlineColorExpandBuffers &&
(infoRec->NumScanlineColorExpandBuffers > 0) &&
- !xf86IsOptionSet(options, XAAOPT_SCANLINE_CPU_TO_SCREEN_COL_EXP_FILL)) {
+ !xf86ReturnOptValBool(options,
+ XAAOPT_SCANLINE_CPU_TO_SCREEN_COL_EXP_FILL,
+ FALSE)) {
HaveScanlineColorExpansion = TRUE;
} else {
infoRec->ScanlineCPUToScreenColorExpandFillFlags = 0;
@@ -419,7 +431,8 @@ XAAInitAccel(ScreenPtr pScreen, XAAInfoRecPtr infoRec)
if(infoRec->SetupForScreenToScreenColorExpandFill &&
infoRec->SubsequentScreenToScreenColorExpandFill &&
- !xf86IsOptionSet(options, XAAOPT_SCREEN_TO_SCREEN_COL_EXP_FILL)) {
+ !xf86ReturnOptValBool(options, XAAOPT_SCREEN_TO_SCREEN_COL_EXP_FILL,
+ FALSE)) {
HaveScreenToScreenColorExpandFill = TRUE;
if (!infoRec->CacheColorExpandDensity)
infoRec->CacheColorExpandDensity = 1;
@@ -433,7 +446,7 @@ XAAInitAccel(ScreenPtr pScreen, XAAInfoRecPtr infoRec)
if(infoRec->SetupForImageWrite && infoRec->ImageWriteBase &&
infoRec->SubsequentImageWriteRect &&
- !xf86IsOptionSet(options, XAAOPT_IMAGE_WRITE_RECT)) {
+ !xf86ReturnOptValBool(options, XAAOPT_IMAGE_WRITE_RECT, FALSE)) {
infoRec->ImageWriteRange >>= 2; /* convert to DWORDS */
if(infoRec->ImageWriteFlags & CPU_TRANSFER_BASE_FIXED)
@@ -452,7 +465,8 @@ XAAInitAccel(ScreenPtr pScreen, XAAInfoRecPtr infoRec)
infoRec->SubsequentImageWriteScanline &&
infoRec->ScanlineImageWriteBuffers &&
(infoRec->NumScanlineImageWriteBuffers > 0) &&
- !xf86IsOptionSet(options, XAAOPT_SCANLINE_IMAGE_WRITE_RECT)) {
+ !xf86ReturnOptValBool(options, XAAOPT_SCANLINE_IMAGE_WRITE_RECT,
+ FALSE)) {
HaveScanlineImageWriteRect = TRUE;
} else {
infoRec->ScanlineImageWriteFlags = 0;
@@ -518,7 +532,8 @@ XAAInitAccel(ScreenPtr pScreen, XAAInfoRecPtr infoRec)
#define XAAMSG(s) do { if (serverGeneration == 1) xf86ErrorF(s); } while (0)
if((infoRec->Flags & OFFSCREEN_PIXMAPS) && HaveScreenToScreenCopy &&
- !xf86IsOptionSet(options, XAAOPT_OFFSCREEN_PIXMAPS)) {
+ !xf86ReturnOptValBool(options, XAAOPT_OFFSCREEN_PIXMAPS,
+ FALSE)) {
XAAMSG("\tOffscreen Pixmaps\n");
} else {
infoRec->Flags &= ~OFFSCREEN_PIXMAPS;
@@ -800,7 +815,7 @@ XAAInitAccel(ScreenPtr pScreen, XAAInfoRecPtr infoRec)
/**** WriteBitmap ****/
if(infoRec->WriteBitmap &&
- !xf86IsOptionSet(options, XAAOPT_WRITE_BITMAP)) {
+ !xf86ReturnOptValBool(options, XAAOPT_WRITE_BITMAP, FALSE)) {
XAAMSG("\tDriver provided WriteBitmap replacement\n");
} else if(HaveColorExpansion) {
if (infoRec->CPUToScreenColorExpandFillFlags & TRIPLE_BITS_24BPP) {
@@ -959,7 +974,7 @@ XAAInitAccel(ScreenPtr pScreen, XAAInfoRecPtr infoRec)
/**** WritePixmap ****/
if(infoRec->WritePixmap &&
- !xf86IsOptionSet(options, XAAOPT_WRITE_PIXMAP)) {
+ !xf86ReturnOptValBool(options, XAAOPT_WRITE_PIXMAP, FALSE)) {
XAAMSG("\tDriver provided WritePixmap replacement\n");
} else if(HaveImageWriteRect) {
infoRec->WritePixmap = XAAWritePixmap;
@@ -1433,7 +1448,7 @@ XAAInitAccel(ScreenPtr pScreen, XAAInfoRecPtr infoRec)
else
infoRec->Flags &= ~PIXMAP_CACHE;
- if (xf86IsOptionSet(options, XAAOPT_PIXMAP_CACHE))
+ if (xf86ReturnOptValBool(options, XAAOPT_PIXMAP_CACHE, FALSE))
infoRec->Flags &= ~PIXMAP_CACHE;
if(infoRec->WriteMono8x8PatternToCache) {}
--
1.5.4.4
Reply to: