* [PATCH 2/5] staging/xgifb: Move duplicated code for dram to helper function
2013-02-04 23:29 [PATCH 1/5] staging/xgifb: Replace XGI340_CR6B table with simple if/else Peter Huewe
@ 2013-02-04 23:29 ` Peter Huewe
2013-02-04 23:29 ` [PATCH 3/5] staging/xgifb: Don't write the same values x times Peter Huewe
` (3 subsequent siblings)
4 siblings, 0 replies; 7+ messages in thread
From: Peter Huewe @ 2013-02-04 23:29 UTC (permalink / raw)
To: Arnaud Patard
Cc: Greg Kroah-Hartman, Miguel Gómez, Aaro Koskinen, Peter Huewe,
Sam Hansen, devel, linux-kernel
XGINew_SetDRAMDefaultRegister340 uses the same code fragment 4 times
with only a slight variation each time.
-> Move this code to a helper function - this saves some lines and
~450bytes in the .o / .ko
Tested-by: Aaro Koskinen <aaro.koskinen@iki.fi>
Signed-off-by: Peter Huewe <peterhuewe@gmx.de>
---
drivers/staging/xgifb/vb_init.c | 58 +++++++++++---------------------------
1 files changed, 17 insertions(+), 41 deletions(-)
diff --git a/drivers/staging/xgifb/vb_init.c b/drivers/staging/xgifb/vb_init.c
index a778df6..25fc7bc 100644
--- a/drivers/staging/xgifb/vb_init.c
+++ b/drivers/staging/xgifb/vb_init.c
@@ -413,6 +413,19 @@ static void XGINew_DDR2_DefaultRegister(
XGINew_DDR2_MRS_XG20(HwDeviceExtension, P3c4, pVBInfo);
}
+static void XGI_SetDRAM_Helper(unsigned long P3d4, u8 seed, u8 temp2, u8 reg,
+ u8 shift_factor, u8 mask1, u8 mask2)
+{
+ u8 j;
+ for (j = 0; j < 4; j++) {
+ temp2 |= (((seed >> (2 * j)) & 0x03) << shift_factor);
+ xgifb_reg_set(P3d4, reg, temp2);
+ xgifb_reg_get(P3d4, reg);
+ temp2 &= mask1;
+ temp2 += mask2;
+ }
+}
+
static void XGINew_SetDRAMDefaultRegister340(
struct xgi_hw_device_info *HwDeviceExtension,
unsigned long Port, struct vb_device_info *pVBInfo)
@@ -426,53 +439,24 @@ static void XGINew_SetDRAMDefaultRegister340(
xgifb_reg_set(P3d4, 0x69, pVBInfo->CR40[6][pVBInfo->ram_type]);
xgifb_reg_set(P3d4, 0x6A, pVBInfo->CR40[7][pVBInfo->ram_type]);
- temp2 = 0;
for (i = 0; i < 4; i++) {
/* CR6B DQS fine tune delay */
temp = (pVBInfo->ram_type <= 2) ? 0xaa : 0x00;
- for (j = 0; j < 4; j++) {
- temp1 = ((temp >> (2 * j)) & 0x03) << 2;
- temp2 |= temp1;
- xgifb_reg_set(P3d4, 0x6B, temp2);
- /* Insert read command for delay */
- xgifb_reg_get(P3d4, 0x6B);
- temp2 &= 0xF0;
- temp2 += 0x10;
- }
+ XGI_SetDRAM_Helper(P3d4, temp, 0, 0x6B, 2, 0xF0, 0x10);
}
- temp2 = 0;
for (i = 0; i < 4; i++) {
/* CR6E DQM fine tune delay */
- temp = 0;
- for (j = 0; j < 4; j++) {
- temp1 = ((temp >> (2 * j)) & 0x03) << 2;
- temp2 |= temp1;
- xgifb_reg_set(P3d4, 0x6E, temp2);
- /* Insert read command for delay */
- xgifb_reg_get(P3d4, 0x6E);
- temp2 &= 0xF0;
- temp2 += 0x10;
- }
+ XGI_SetDRAM_Helper(P3d4, 0, 0, 0x6E, 2, 0xF0, 0x10);
}
temp3 = 0;
for (k = 0; k < 4; k++) {
/* CR6E_D[1:0] select channel */
xgifb_reg_and_or(P3d4, 0x6E, 0xFC, temp3);
- temp2 = 0;
for (i = 0; i < 8; i++) {
/* CR6F DQ fine tune delay */
- temp = 0;
- for (j = 0; j < 4; j++) {
- temp1 = (temp >> (2 * j)) & 0x03;
- temp2 |= temp1;
- xgifb_reg_set(P3d4, 0x6F, temp2);
- /* Insert read command for delay */
- xgifb_reg_get(P3d4, 0x6F);
- temp2 &= 0xF8;
- temp2 += 0x08;
- }
+ XGI_SetDRAM_Helper(P3d4, 0, 0, 0x6F, 0, 0xF8, 0x08);
}
temp3 += 0x01;
}
@@ -486,15 +470,7 @@ static void XGINew_SetDRAMDefaultRegister340(
temp2 = 0x80;
/* CR89 terminator type select */
- temp = 0;
- for (j = 0; j < 4; j++) {
- temp1 = (temp >> (2 * j)) & 0x03;
- temp2 |= temp1;
- xgifb_reg_set(P3d4, 0x89, temp2);
- xgifb_reg_get(P3d4, 0x89); /* Insert read command for delay */
- temp2 &= 0xF0;
- temp2 += 0x10;
- }
+ XGI_SetDRAM_Helper(P3d4, 0, temp2, 0x89, 0, 0xF0, 0x10);
temp = 0;
temp1 = temp & 0x03;
--
1.7.8.6
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH 3/5] staging/xgifb: Don't write the same values x times
2013-02-04 23:29 [PATCH 1/5] staging/xgifb: Replace XGI340_CR6B table with simple if/else Peter Huewe
2013-02-04 23:29 ` [PATCH 2/5] staging/xgifb: Move duplicated code for dram to helper function Peter Huewe
@ 2013-02-04 23:29 ` Peter Huewe
2013-02-04 23:29 ` [PATCH 4/5] staging/xgifb: Consolidate if/else for 'identical' branches Peter Huewe
` (2 subsequent siblings)
4 siblings, 0 replies; 7+ messages in thread
From: Peter Huewe @ 2013-02-04 23:29 UTC (permalink / raw)
To: Arnaud Patard
Cc: Greg Kroah-Hartman, Miguel Gómez, Aaro Koskinen, Peter Huewe,
Sam Hansen, devel, linux-kernel
With the previous patch 'Move duplicated code for dram to helper
function' it becomes evident that the code is performing the same
thing 4 or 8 times in a row without changing arguments to the function
and thus writing the same values over and over again.
It was tested that these repeats are unnecessary.
-> we can safely remove them.
Tested-by: Aaro Koskinen <aaro.koskinen@iki.fi>
Signed-off-by: Peter Huewe <peterhuewe@gmx.de>
---
drivers/staging/xgifb/vb_init.c | 19 ++++++-------------
1 files changed, 6 insertions(+), 13 deletions(-)
diff --git a/drivers/staging/xgifb/vb_init.c b/drivers/staging/xgifb/vb_init.c
index 25fc7bc..1c6e0f3 100644
--- a/drivers/staging/xgifb/vb_init.c
+++ b/drivers/staging/xgifb/vb_init.c
@@ -439,25 +439,18 @@ static void XGINew_SetDRAMDefaultRegister340(
xgifb_reg_set(P3d4, 0x69, pVBInfo->CR40[6][pVBInfo->ram_type]);
xgifb_reg_set(P3d4, 0x6A, pVBInfo->CR40[7][pVBInfo->ram_type]);
- for (i = 0; i < 4; i++) {
- /* CR6B DQS fine tune delay */
- temp = (pVBInfo->ram_type <= 2) ? 0xaa : 0x00;
- XGI_SetDRAM_Helper(P3d4, temp, 0, 0x6B, 2, 0xF0, 0x10);
- }
+ /* CR6B DQS fine tune delay */
+ temp = (pVBInfo->ram_type <= 2) ? 0xaa : 0x00;
+ XGI_SetDRAM_Helper(P3d4, temp, 0, 0x6B, 2, 0xF0, 0x10);
- for (i = 0; i < 4; i++) {
- /* CR6E DQM fine tune delay */
- XGI_SetDRAM_Helper(P3d4, 0, 0, 0x6E, 2, 0xF0, 0x10);
- }
+ /* CR6E DQM fine tune delay */
+ XGI_SetDRAM_Helper(P3d4, 0, 0, 0x6E, 2, 0xF0, 0x10);
temp3 = 0;
for (k = 0; k < 4; k++) {
/* CR6E_D[1:0] select channel */
xgifb_reg_and_or(P3d4, 0x6E, 0xFC, temp3);
- for (i = 0; i < 8; i++) {
- /* CR6F DQ fine tune delay */
- XGI_SetDRAM_Helper(P3d4, 0, 0, 0x6F, 0, 0xF8, 0x08);
- }
+ XGI_SetDRAM_Helper(P3d4, 0, 0, 0x6F, 0, 0xF8, 0x08);
temp3 += 0x01;
}
--
1.7.8.6
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH 4/5] staging/xgifb: Consolidate if/else for 'identical' branches
2013-02-04 23:29 [PATCH 1/5] staging/xgifb: Replace XGI340_CR6B table with simple if/else Peter Huewe
2013-02-04 23:29 ` [PATCH 2/5] staging/xgifb: Move duplicated code for dram to helper function Peter Huewe
2013-02-04 23:29 ` [PATCH 3/5] staging/xgifb: Don't write the same values x times Peter Huewe
@ 2013-02-04 23:29 ` Peter Huewe
2013-02-04 23:29 ` [PATCH 5/5] staging/xgifb: Consolidate if branches with similar conditions Peter Huewe
2013-02-05 19:18 ` [PATCH 1/5] staging/xgifb: Replace XGI340_CR6B table with simple if/else Greg Kroah-Hartman
4 siblings, 0 replies; 7+ messages in thread
From: Peter Huewe @ 2013-02-04 23:29 UTC (permalink / raw)
To: Arnaud Patard
Cc: Greg Kroah-Hartman, Miguel Gómez, Aaro Koskinen, Peter Huewe,
Sam Hansen, devel, linux-kernel
Since XGI_LCDDesStruct is fully contained in XGI330_LCDDataDesStruct2
and the offsets for the first members is identical we can consolidate
the if/else branches here and use XGI330_LCDDataDesStruct2 for
everything.
Signed-off-by: Peter Huewe <peterhuewe@gmx.de>
---
drivers/staging/xgifb/vb_setmode.c | 31 ++++++-------------------------
1 files changed, 6 insertions(+), 25 deletions(-)
diff --git a/drivers/staging/xgifb/vb_setmode.c b/drivers/staging/xgifb/vb_setmode.c
index f44a254..131a267 100644
--- a/drivers/staging/xgifb/vb_setmode.c
+++ b/drivers/staging/xgifb/vb_setmode.c
@@ -1470,18 +1470,11 @@ static void XGI_SetLVDSRegs(unsigned short ModeNo, unsigned short ModeIdIndex,
{
unsigned short tempbx, tempax, tempcx, tempdx, push1, push2, modeflag;
unsigned long temp, temp1, temp2, temp3, push3;
- struct XGI_LCDDesStruct const *LCDPtr = NULL;
struct XGI330_LCDDataDesStruct2 const *LCDPtr1 = NULL;
modeflag = XGI330_EModeIDTable[ModeIdIndex].Ext_ModeFlag;
- if (pVBInfo->LCDInfo & EnableScalingLCD)
- LCDPtr1 = XGI_GetLcdPtr(XGI_EPLLCDDesDataPtr, ModeNo,
- ModeIdIndex, RefreshRateTableIndex,
- pVBInfo);
- else
- LCDPtr = XGI_GetLcdPtr(XGI_EPLLCDDesDataPtr, ModeNo,
- ModeIdIndex, RefreshRateTableIndex,
- pVBInfo);
+ LCDPtr1 = XGI_GetLcdPtr(XGI_EPLLCDDesDataPtr, ModeNo, ModeIdIndex,
+ RefreshRateTableIndex, pVBInfo);
XGI_GetLCDSync(&tempax, &tempbx, pVBInfo);
push1 = tempbx;
@@ -1513,10 +1506,7 @@ static void XGI_SetLVDSRegs(unsigned short ModeNo, unsigned short ModeIdIndex,
tempax = pVBInfo->HT;
- if (pVBInfo->LCDInfo & EnableScalingLCD)
- tempbx = LCDPtr1->LCDHDES;
- else
- tempbx = LCDPtr->LCDHDES;
+ tempbx = LCDPtr1->LCDHDES;
tempcx = pVBInfo->HDE;
tempbx = tempbx & 0x0fff;
@@ -1537,10 +1527,7 @@ static void XGI_SetLVDSRegs(unsigned short ModeNo, unsigned short ModeIdIndex,
tempax = pVBInfo->HT;
- if (pVBInfo->LCDInfo & EnableScalingLCD)
- tempbx = LCDPtr1->LCDHRS;
- else
- tempbx = LCDPtr->LCDHRS;
+ tempbx = LCDPtr1->LCDHRS;
tempcx = push2;
@@ -1565,10 +1552,7 @@ static void XGI_SetLVDSRegs(unsigned short ModeNo, unsigned short ModeIdIndex,
(unsigned short) (tempbx & 0xff));
tempax = pVBInfo->VT;
- if (pVBInfo->LCDInfo & EnableScalingLCD)
- tempbx = LCDPtr1->LCDVDES;
- else
- tempbx = LCDPtr->LCDVDES;
+ tempbx = LCDPtr1->LCDVDES;
tempcx = pVBInfo->VDE;
tempbx = tempbx & 0x0fff;
@@ -1589,10 +1573,7 @@ static void XGI_SetLVDSRegs(unsigned short ModeNo, unsigned short ModeIdIndex,
| tempbx));
tempax = pVBInfo->VT;
- if (pVBInfo->LCDInfo & EnableScalingLCD)
- tempbx = LCDPtr1->LCDVRS;
- else
- tempbx = LCDPtr->LCDVRS;
+ tempbx = LCDPtr1->LCDVRS;
tempcx = push1;
--
1.7.8.6
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH 5/5] staging/xgifb: Consolidate if branches with similar conditions
2013-02-04 23:29 [PATCH 1/5] staging/xgifb: Replace XGI340_CR6B table with simple if/else Peter Huewe
` (2 preceding siblings ...)
2013-02-04 23:29 ` [PATCH 4/5] staging/xgifb: Consolidate if/else for 'identical' branches Peter Huewe
@ 2013-02-04 23:29 ` Peter Huewe
2013-02-05 19:18 ` [PATCH 1/5] staging/xgifb: Replace XGI340_CR6B table with simple if/else Greg Kroah-Hartman
4 siblings, 0 replies; 7+ messages in thread
From: Peter Huewe @ 2013-02-04 23:29 UTC (permalink / raw)
To: Arnaud Patard
Cc: Greg Kroah-Hartman, Miguel Gómez, Aaro Koskinen, Peter Huewe,
Sam Hansen, devel, linux-kernel
1) The same condition (pVBInfo->IF_DEF_LVDS == 0) was checked in the if
clause directly in front of this one.
2) The same condition pVBInfo->VBType & (VB_SIS301B | VB_SIS302B |
VB_SIS301LV | VB_SIS302LV | VB_XGI301C) was checked in the if
clause directly in front of this one.
Signed-off-by: Peter Huewe <peterhuewe@gmx.de>
---
drivers/staging/xgifb/vb_setmode.c | 13 ++-----------
1 files changed, 2 insertions(+), 11 deletions(-)
diff --git a/drivers/staging/xgifb/vb_setmode.c b/drivers/staging/xgifb/vb_setmode.c
index 131a267..bd81656 100644
--- a/drivers/staging/xgifb/vb_setmode.c
+++ b/drivers/staging/xgifb/vb_setmode.c
@@ -2299,9 +2299,7 @@ static unsigned char XGI_GetLCDInfo(unsigned short ModeNo,
& VB_XGI301C)) && (tempax & XGI_LCDDualLink)) {
tempbx |= SetLCDDualLink;
}
- }
- if (pVBInfo->IF_DEF_LVDS == 0) {
if ((pVBInfo->LCDResInfo == Panel_1400x1050) && (pVBInfo->VBInfo
& SetCRT2ToLCD) && (resinfo == 9) &&
(!(tempbx & EnableScalingLCD)))
@@ -5024,12 +5022,8 @@ static void XGI_SetLCDCap(struct vb_device_info *pVBInfo)
tempcx = pVBInfo->LCDCapList[XGI_GetLCDCapPtr(pVBInfo)].LCD_Capability;
- if (pVBInfo->VBType &
- (VB_SIS301B |
- VB_SIS302B |
- VB_SIS301LV |
- VB_SIS302LV |
- VB_XGI301C)) { /* 301LV/302LV only */
+ if (pVBInfo->VBType & (VB_SIS301B | VB_SIS302B | VB_SIS301LV |
+ VB_SIS302LV | VB_XGI301C)) {
if (pVBInfo->VBType &
(VB_SIS301LV | VB_SIS302LV | VB_XGI301C)) {
/* Set 301LV Capability */
@@ -5041,10 +5035,7 @@ static void XGI_SetLCDCap(struct vb_device_info *pVBInfo)
~((EnableVBCLKDRVLOW | EnablePLLSPLOW) >> 8),
(unsigned short) ((tempcx & (EnableVBCLKDRVLOW
| EnablePLLSPLOW)) >> 8));
- }
- if (pVBInfo->VBType & (VB_SIS301B | VB_SIS302B | VB_SIS301LV
- | VB_SIS302LV | VB_XGI301C)) {
if (pVBInfo->VBInfo & SetCRT2ToLCD)
XGI_SetLCDCap_B(tempcx, pVBInfo);
else if (pVBInfo->VBInfo & XGI_SetCRT2ToLCDA)
--
1.7.8.6
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH 1/5] staging/xgifb: Replace XGI340_CR6B table with simple if/else
2013-02-04 23:29 [PATCH 1/5] staging/xgifb: Replace XGI340_CR6B table with simple if/else Peter Huewe
` (3 preceding siblings ...)
2013-02-04 23:29 ` [PATCH 5/5] staging/xgifb: Consolidate if branches with similar conditions Peter Huewe
@ 2013-02-05 19:18 ` Greg Kroah-Hartman
2013-02-05 19:38 ` [PATCH] staging/xgifb: Remove unused variable Peter Huewe
4 siblings, 1 reply; 7+ messages in thread
From: Greg Kroah-Hartman @ 2013-02-05 19:18 UTC (permalink / raw)
To: Peter Huewe
Cc: Arnaud Patard, Miguel Gómez, Aaro Koskinen, Sam Hansen,
devel, linux-kernel
On Tue, Feb 05, 2013 at 12:29:43AM +0100, Peter Huewe wrote:
> The XGI340_CR6B lookup table consists of the entries
> {0xaa, 0xaa, 0xaa, 0xaa} for an index <= 2 and
> {0x00, 0x00, 0x00, 0x00} for all other indices.
>
> The only user XGINew_SetDRAMDefaultRegister340 loops over these 4 values
> of a line with a for loop and since all entries are the same for each line
> we can simply replace the whole lookup table with a simple if/else assignment.
>
> Tested-by: Aaro Koskinen <aaro.koskinen@iki.fi>
> Signed-off-by: Peter Huewe <peterhuewe@gmx.de>
> ---
> drivers/staging/xgifb/vb_init.c | 2 +-
> drivers/staging/xgifb/vb_table.h | 11 -----------
> 2 files changed, 1 insertions(+), 12 deletions(-)
With this series applied, I now get the following warning when building:
driverrs/staging/xgifb/vb_init.c: In function ‘XGINew_SetDRAMDefaultRegister340’:
drivers/staging/xgifb/vb_init.c:433:43: warning: unused variable ‘i’ [-Wunused-variable]
Care to send a follow-on patch to fix this up?
thanks,
greg k-h
^ permalink raw reply [flat|nested] 7+ messages in thread* [PATCH] staging/xgifb: Remove unused variable
2013-02-05 19:18 ` [PATCH 1/5] staging/xgifb: Replace XGI340_CR6B table with simple if/else Greg Kroah-Hartman
@ 2013-02-05 19:38 ` Peter Huewe
0 siblings, 0 replies; 7+ messages in thread
From: Peter Huewe @ 2013-02-05 19:38 UTC (permalink / raw)
To: Arnaud Patard
Cc: Greg Kroah-Hartman, Miguel Gómez, Aaro Koskinen, Peter Huewe,
Sam Hansen, devel, linux-kernel
After the patch
'staging/xgifb: Don't write the same values x times'
the local variable i is unused, which leads to the following warning:
driverrs/staging/xgifb/vb_init.c: In function
‘XGINew_SetDRAMDefaultRegister340’:
drivers/staging/xgifb/vb_init.c:433:43: warning: unused variable ‘i’
[-Wunused-variable]
This patch fixes this
Signed-off-by: Peter Huewe <peterhuewe@gmx.de>
---
Sorry that I missed that one in the patch series.
drivers/staging/xgifb/vb_init.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/staging/xgifb/vb_init.c b/drivers/staging/xgifb/vb_init.c
index 1c6e0f3..7b8f41d 100644
--- a/drivers/staging/xgifb/vb_init.c
+++ b/drivers/staging/xgifb/vb_init.c
@@ -430,7 +430,7 @@ static void XGINew_SetDRAMDefaultRegister340(
struct xgi_hw_device_info *HwDeviceExtension,
unsigned long Port, struct vb_device_info *pVBInfo)
{
- unsigned char temp, temp1, temp2, temp3, i, j, k;
+ unsigned char temp, temp1, temp2, temp3, j, k;
unsigned long P3d4 = Port, P3c4 = Port - 0x10;
--
1.7.8.6
^ permalink raw reply related [flat|nested] 7+ messages in thread