Commit be0d9b6c7aeaad1683059c00131cabd4c894c17c
Committed by
Linus Torvalds
1 parent
7275b4b6bc
Exists in
master
and in
7 other branches
[PATCH] fbdev: Fix incorrect unaligned access in little-endian machines
The drawing function cfbfillrect does not work correctly when access is not unsigned-long aligned. It manifests as extra lines of pixels that are not complete drawn. Reversing the shift operator solves the problem, so I would presume that this bug would manifest only on little endian machines. The function cfbcopyarea may also have this bug. Aligned access should present no problems. Signed-off-by: Antonino Daplas <adaplas@pol.net> Signed-off-by: Andrew Morton <akpm@osdl.org> Signed-off-by: Linus Torvalds <torvalds@osdl.org>
Showing 4 changed files with 36 additions and 36 deletions Side-by-side Diff
drivers/video/cfbcopyarea.c
... | ... | @@ -64,8 +64,8 @@ |
64 | 64 | int const shift = dst_idx-src_idx; |
65 | 65 | int left, right; |
66 | 66 | |
67 | - first = ~0UL >> dst_idx; | |
68 | - last = ~(~0UL >> ((dst_idx+n) % bits)); | |
67 | + first = FB_SHIFT_HIGH(~0UL, dst_idx); | |
68 | + last = ~(FB_SHIFT_HIGH(~0UL, (dst_idx+n) % bits)); | |
69 | 69 | |
70 | 70 | if (!shift) { |
71 | 71 | // Same alignment for source and dest |
... | ... | @@ -216,8 +216,8 @@ |
216 | 216 | |
217 | 217 | shift = dst_idx-src_idx; |
218 | 218 | |
219 | - first = ~0UL << (bits - 1 - dst_idx); | |
220 | - last = ~(~0UL << (bits - 1 - ((dst_idx-n) % bits))); | |
219 | + first = FB_SHIFT_LOW(~0UL, bits - 1 - dst_idx); | |
220 | + last = ~(FB_SHIFT_LOW(~0UL, bits - 1 - ((dst_idx-n) % bits))); | |
221 | 221 | |
222 | 222 | if (!shift) { |
223 | 223 | // Same alignment for source and dest |
drivers/video/cfbfillrect.c
... | ... | @@ -110,8 +110,8 @@ |
110 | 110 | if (!n) |
111 | 111 | return; |
112 | 112 | |
113 | - first = ~0UL >> dst_idx; | |
114 | - last = ~(~0UL >> ((dst_idx+n) % bits)); | |
113 | + first = FB_SHIFT_HIGH(~0UL, dst_idx); | |
114 | + last = ~(FB_SHIFT_HIGH(~0UL, (dst_idx+n) % bits)); | |
115 | 115 | |
116 | 116 | if (dst_idx+n <= bits) { |
117 | 117 | // Single word |
... | ... | @@ -167,8 +167,8 @@ |
167 | 167 | if (!n) |
168 | 168 | return; |
169 | 169 | |
170 | - first = ~0UL >> dst_idx; | |
171 | - last = ~(~0UL >> ((dst_idx+n) % bits)); | |
170 | + first = FB_SHIFT_HIGH(~0UL, dst_idx); | |
171 | + last = ~(FB_SHIFT_HIGH(~0UL, (dst_idx+n) % bits)); | |
172 | 172 | |
173 | 173 | if (dst_idx+n <= bits) { |
174 | 174 | // Single word |
... | ... | @@ -221,8 +221,8 @@ |
221 | 221 | if (!n) |
222 | 222 | return; |
223 | 223 | |
224 | - first = ~0UL >> dst_idx; | |
225 | - last = ~(~0UL >> ((dst_idx+n) % bits)); | |
224 | + first = FB_SHIFT_HIGH(~0UL, dst_idx); | |
225 | + last = ~(FB_SHIFT_HIGH(~0UL, (dst_idx+n) % bits)); | |
226 | 226 | |
227 | 227 | if (dst_idx+n <= bits) { |
228 | 228 | // Single word |
... | ... | @@ -290,8 +290,8 @@ |
290 | 290 | if (!n) |
291 | 291 | return; |
292 | 292 | |
293 | - first = ~0UL >> dst_idx; | |
294 | - last = ~(~0UL >> ((dst_idx+n) % bits)); | |
293 | + first = FB_SHIFT_HIGH(~0UL, dst_idx); | |
294 | + last = ~(FB_SHIFT_HIGH(~0UL, (dst_idx+n) % bits)); | |
295 | 295 | |
296 | 296 | if (dst_idx+n <= bits) { |
297 | 297 | // Single word |
drivers/video/cfbimgblt.c
... | ... | @@ -76,18 +76,6 @@ |
76 | 76 | #define FB_WRITEL fb_writel |
77 | 77 | #define FB_READL fb_readl |
78 | 78 | |
79 | -#if defined (__BIG_ENDIAN) | |
80 | -#define LEFT_POS(bpp) (32 - bpp) | |
81 | -#define SHIFT_HIGH(val, bits) ((val) >> (bits)) | |
82 | -#define SHIFT_LOW(val, bits) ((val) << (bits)) | |
83 | -#define BIT_NR(b) (7 - (b)) | |
84 | -#else | |
85 | -#define LEFT_POS(bpp) (0) | |
86 | -#define SHIFT_HIGH(val, bits) ((val) << (bits)) | |
87 | -#define SHIFT_LOW(val, bits) ((val) >> (bits)) | |
88 | -#define BIT_NR(b) (b) | |
89 | -#endif | |
90 | - | |
91 | 79 | static inline void color_imageblit(const struct fb_image *image, |
92 | 80 | struct fb_info *p, u8 __iomem *dst1, |
93 | 81 | u32 start_index, |
... | ... | @@ -109,7 +97,7 @@ |
109 | 97 | val = 0; |
110 | 98 | |
111 | 99 | if (start_index) { |
112 | - u32 start_mask = ~(SHIFT_HIGH(~(u32)0, start_index)); | |
100 | + u32 start_mask = ~(FB_SHIFT_HIGH(~(u32)0, start_index)); | |
113 | 101 | val = FB_READL(dst) & start_mask; |
114 | 102 | shift = start_index; |
115 | 103 | } |
116 | 104 | |
117 | 105 | |
... | ... | @@ -119,20 +107,20 @@ |
119 | 107 | color = palette[*src]; |
120 | 108 | else |
121 | 109 | color = *src; |
122 | - color <<= LEFT_POS(bpp); | |
123 | - val |= SHIFT_HIGH(color, shift); | |
110 | + color <<= FB_LEFT_POS(bpp); | |
111 | + val |= FB_SHIFT_HIGH(color, shift); | |
124 | 112 | if (shift >= null_bits) { |
125 | 113 | FB_WRITEL(val, dst++); |
126 | 114 | |
127 | 115 | val = (shift == null_bits) ? 0 : |
128 | - SHIFT_LOW(color, 32 - shift); | |
116 | + FB_SHIFT_LOW(color, 32 - shift); | |
129 | 117 | } |
130 | 118 | shift += bpp; |
131 | 119 | shift &= (32 - 1); |
132 | 120 | src++; |
133 | 121 | } |
134 | 122 | if (shift) { |
135 | - u32 end_mask = SHIFT_HIGH(~(u32)0, shift); | |
123 | + u32 end_mask = FB_SHIFT_HIGH(~(u32)0, shift); | |
136 | 124 | |
137 | 125 | FB_WRITEL((FB_READL(dst) & end_mask) | val, dst); |
138 | 126 | } |
... | ... | @@ -162,8 +150,8 @@ |
162 | 150 | u32 i, j, l; |
163 | 151 | |
164 | 152 | dst2 = (u32 __iomem *) dst1; |
165 | - fgcolor <<= LEFT_POS(bpp); | |
166 | - bgcolor <<= LEFT_POS(bpp); | |
153 | + fgcolor <<= FB_LEFT_POS(bpp); | |
154 | + bgcolor <<= FB_LEFT_POS(bpp); | |
167 | 155 | |
168 | 156 | for (i = image->height; i--; ) { |
169 | 157 | shift = val = 0; |
170 | 158 | |
171 | 159 | |
... | ... | @@ -174,21 +162,21 @@ |
174 | 162 | |
175 | 163 | /* write leading bits */ |
176 | 164 | if (start_index) { |
177 | - u32 start_mask = ~(SHIFT_HIGH(~(u32)0, start_index)); | |
165 | + u32 start_mask = ~(FB_SHIFT_HIGH(~(u32)0,start_index)); | |
178 | 166 | val = FB_READL(dst) & start_mask; |
179 | 167 | shift = start_index; |
180 | 168 | } |
181 | 169 | |
182 | 170 | while (j--) { |
183 | 171 | l--; |
184 | - color = (*s & 1 << (BIT_NR(l))) ? fgcolor : bgcolor; | |
185 | - val |= SHIFT_HIGH(color, shift); | |
172 | + color = (*s & 1 << (FB_BIT_NR(l))) ? fgcolor : bgcolor; | |
173 | + val |= FB_SHIFT_HIGH(color, shift); | |
186 | 174 | |
187 | 175 | /* Did the bitshift spill bits to the next long? */ |
188 | 176 | if (shift >= null_bits) { |
189 | 177 | FB_WRITEL(val, dst++); |
190 | 178 | val = (shift == null_bits) ? 0 : |
191 | - SHIFT_LOW(color,32 - shift); | |
179 | + FB_SHIFT_LOW(color,32 - shift); | |
192 | 180 | } |
193 | 181 | shift += bpp; |
194 | 182 | shift &= (32 - 1); |
... | ... | @@ -197,7 +185,7 @@ |
197 | 185 | |
198 | 186 | /* write trailing bits */ |
199 | 187 | if (shift) { |
200 | - u32 end_mask = SHIFT_HIGH(~(u32)0, shift); | |
188 | + u32 end_mask = FB_SHIFT_HIGH(~(u32)0, shift); | |
201 | 189 | |
202 | 190 | FB_WRITEL((FB_READL(dst) & end_mask) | val, dst); |
203 | 191 | } |
include/linux/fb.h
... | ... | @@ -835,6 +835,18 @@ |
835 | 835 | |
836 | 836 | #endif |
837 | 837 | |
838 | +#if defined (__BIG_ENDIAN) | |
839 | +#define FB_LEFT_POS(bpp) (32 - bpp) | |
840 | +#define FB_SHIFT_HIGH(val, bits) ((val) >> (bits)) | |
841 | +#define FB_SHIFT_LOW(val, bits) ((val) << (bits)) | |
842 | +#define FB_BIT_NR(b) (7 - (b)) | |
843 | +#else | |
844 | +#define FB_LEFT_POS(bpp) (0) | |
845 | +#define FB_SHIFT_HIGH(val, bits) ((val) << (bits)) | |
846 | +#define FB_SHIFT_LOW(val, bits) ((val) >> (bits)) | |
847 | +#define FB_BIT_NR(b) (b) | |
848 | +#endif | |
849 | + | |
838 | 850 | /* |
839 | 851 | * `Generic' versions of the frame buffer device operations |
840 | 852 | */ |