From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:57598) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QUGr5-0001Go-Ni for qemu-devel@nongnu.org; Wed, 08 Jun 2011 07:23:27 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1QUGr4-0004xI-E0 for qemu-devel@nongnu.org; Wed, 08 Jun 2011 07:23:23 -0400 Received: from mail-bw0-f45.google.com ([209.85.214.45]:62324) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QUGr3-0004x4-Qp for qemu-devel@nongnu.org; Wed, 08 Jun 2011 07:23:22 -0400 Received: by bwz16 with SMTP id 16so358063bwz.4 for ; Wed, 08 Jun 2011 04:23:20 -0700 (PDT) From: Vasily Khoruzhick Date: Wed, 8 Jun 2011 14:22:42 +0300 References: <1306920487-18705-1-git-send-email-anarsoul@gmail.com> <1307115397-6496-1-git-send-email-anarsoul@gmail.com> In-Reply-To: MIME-Version: 1.0 Content-Type: Text/Plain; charset="utf-8" Content-Transfer-Encoding: 7bit Message-Id: <201106081422.42682.anarsoul@gmail.com> Subject: Re: [Qemu-devel] [PATCH v3 1/2] pxa2xx_lcd: add proper rotation support List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Maydell Cc: "qemu-devel@nongnu.org" On Wednesday 08 June 2011 12:50:50 Peter Maydell wrote: > On 3 June 2011 16:36, Vasily Khoruzhick wrote: > > Until now, pxa2xx_lcd only supported 90deg rotation, but > > some machines (for example Zipit Z2) needs 270deg rotation. > > > > Signed-off-by: Vasily Khoruzhick > > 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. Oh, ok. I'll add patch to fix pxa2xx_lcd.c codestyle to the series, and then fix this patch. > > > 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. Ok > 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. Well, -portrait argument is preserved, so all old code should work fine (including scripts to start qemu). Passing -rotate 180 or -rotate 270 will produce unpredictable result (but not a crash). I have no spitz image, so I can't test what happens. Regards Vasily