On 23 December 2011 11:40, Evgeny Voevodin <e.voevo...@samsung.com> wrote:
> +/* Perform byte/halfword/word swap of data according to WINCON */ > +static inline void fimd_swap_data(unsigned int swap_ctl, uint64_t *data) > +{ > + int i; > + uint64_t res; > + uint64_t x = *data; > + > + if (swap_ctl & FIMD_WINCON_SWAP_BITS) { > + res = 0; > + for (i = 0; i < 64; i++) { > + if (x & (1ULL << (64 - i))) { > + res |= (1ULL << i); > + } > + } > + x = res; > + } > + if (swap_ctl & FIMD_WINCON_SWAP_BYTE) { > + x = ((x & 0x00000000000000FFULL) << 56) | > + ((x & 0x000000000000FF00ULL) << 40) | > + ((x & 0x0000000000FF0000ULL) << 24) | > + ((x & 0x00000000FF000000ULL) << 8) | > + ((x & 0x000000FF00000000ULL) >> 8) | > + ((x & 0x0000FF0000000000ULL) >> 24) | > + ((x & 0x00FF000000000000ULL) >> 40) | > + ((x & 0xFF00000000000000ULL) >> 56); > + } > + if (swap_ctl & FIMD_WINCON_SWAP_HWORD) { > + x = ((x & 0x000000000000FFFFULL) << 48) | > + ((x & 0x00000000FFFF0000ULL) << 16) | > + ((x & 0x0000FFFF00000000ULL) >> 16) | > + ((x & 0xFFFF000000000000ULL) >> 48); > + } > + if (swap_ctl & FIMD_WINCON_SWAP_WORD) { > + x = ((x & 0x00000000FFFFFFFFULL) << 32) | > + ((x & 0xFFFFFFFF00000000ULL) >> 32); > + } bswap.h provides bswap_64() which you can use for the first of these. > +/* Draw line with index in palette table in RAM frame buffer data */ > +#define DEF_DRAW_LINE_PALLETE(N) \ > +static void glue(draw_line_palette_, N)(Exynos4210fimdWindow *w, uint8_t > *src, \ > + uint8_t *dst, bool blend) \ The comment and the parameter are correct about the spelling of 'palette'; please fix the macro name. exynos4210_fimd_get_bppmode() has the same typo -- global search-n-replace seems like a good idea. > +static void exynos4210_fimd_write(void *opaque, target_phys_addr_t offset, > + uint64_t val, unsigned size) > +{ > + Exynos4210fimdState *s = (Exynos4210fimdState *)opaque; > + int w, i; > + > + DPRINT_L2("write offset 0x%08x, value=%ld(0x%08lx)\n", offset, val, val); > + > + if (offset == FIMD_VIDCON0) { > + if ((val & FIMD_VIDCON0_ENVID_MASK) == FIMD_VIDCON0_ENVID_MASK) { > + exynos4210_fimd_enable(s, true); > + } else { > + if ((val & FIMD_VIDCON0_ENVID) == 0) { > + exynos4210_fimd_enable(s, false); > + } > + } > + s->vidcon[0] = val; > + } else if (offset == FIMD_VIDCON1) { is using an if-else ladder here rather than a switch statement a deliberate decision ? Otherwise looks OK I guess, though I've only scanned it rather superficially. -- PMM