Re: [Qemu-devel] [PATCH v3 1/2] pxa2xx_lcd: add proper rotation support

[Peter Maydell <peter.maydell@linaro.org>]

On 3 June 2011 16:36, Vasily Khoruzhick <anarsoul@gmail.com> wrote:
> Until now, pxa2xx_lcd only supported 90deg rotation, but
> some machines (for example Zipit Z2) needs 270deg rotation.
>
> Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>

Still haven't tested this yet, but some very quick first
pass comments.


> +static void pxa2xx_lcdc_dma0_redraw_rot180(PXA2xxLCDState *s,
> +                target_phys_addr_t addr, int *miny, int *maxy)
> +{
> +    int src_width, dest_width;
> +    drawfn fn = NULL;
> +    if (s->dest_width)
> +        fn = s->line_fn[s->transp][s->bpp];
> +    if (!fn)
> +        return;

Braces (here and elsewhere); if we're adding whole new functions
we might as well do them in accordance with the styl.

>             case QEMU_OPTION_portrait:
> -                graphic_rotate = 1;
> +                graphic_rotate = 90;
> +                break;
> +            case QEMU_OPTION_rotate:
> +                graphic_rotate = atoi(optarg);
> +                if (graphic_rotate != 0 && graphic_rotate != 90 &&
> +                    graphic_rotate != 180 && graphic_rotate != 270) {
> +                    fprintf(stderr,
> +                        "qemu: only 90, 180, 270 deg rotation is available\n");
> +                    exit(1);
> +                }

We seem to generally use strtol() for argument parsing rather
than atoi, might be nicer to be consistent.

The other bit of code that looks at graphic_rotate
is hw/spitz.c:spitz_keyboard_register(). Does that require
changes? It doesn't seem entirely right for us to assert
'tablet mode' for everything except 0deg rotation.

-- PMM