From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1KxMwT-000869-CM for qemu-devel@nongnu.org; Tue, 04 Nov 2008 09:31:37 -0500 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1KxMwP-000851-4h for qemu-devel@nongnu.org; Tue, 04 Nov 2008 09:31:36 -0500 Received: from [199.232.76.173] (port=34482 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1KxMwP-00084w-17 for qemu-devel@nongnu.org; Tue, 04 Nov 2008 09:31:33 -0500 Received: from vsmtp02.dti.ne.jp ([202.216.231.137]:48226) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1KxMwO-00084u-Es for qemu-devel@nongnu.org; Tue, 04 Nov 2008 09:31:32 -0500 Received: from [192.168.1.21] (PPPa1538.e11.eacc.dti.ne.jp [124.255.92.21]) by vsmtp02.dti.ne.jp (3.11v) with ESMTP AUTH id mA4EVPK6008617 for ; Tue, 4 Nov 2008 23:31:25 +0900 (JST) Message-ID: <49105CBD.8050501@juno.dti.ne.jp> Date: Tue, 04 Nov 2008 23:31:25 +0900 From: Shin-ichiro KAWASAKI MIME-Version: 1.0 Subject: Re: [Qemu-devel] [PATCH] SM501 emulation for R2D-SH4 References: <490D0C8F.9010601@juno.dti.ne.jp> In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Reply-To: qemu-devel@nongnu.org List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org Thank you for your review! >> I'm not sure about following two points. >> >> - Register definitions were copied from Linux : include/linux/sm501-regs.h > > I'd try to suck it into the .c file because the definitions are not > going to be used anywhere else in qemu. OK. I'll include it in sm501.c. >> +#include "vga_int.h" > > I think isn't needed? Right. Will be deleted. >> +static void sm501_draw_crt(SM501State * s) >> +{ >> + int x, y; >> + uint32_t crt_width = (s->dc_crt_h_total & 0x00000FFF) + 1; >> + uint32_t crt_height = (s->dc_crt_v_total & 0x00000FFF) + 1; >> + uint8_t * buf = s->local_mem; >> + uint32_t * palette = (uint32_t *)s->dc_crt_palette; >> + >> + /* adjust console size */ >> + if (s->last_width != crt_width || s->last_height != crt_height) { >> + qemu_console_resize(s->console, crt_width, crt_height); >> + s->last_width = crt_width; >> + s->last_height = crt_height; >> + } >> + >> + switch (s->dc_crt_control & 3) { >> + case SM501_DC_CRT_CONTROL_8BPP: >> + for (y = 0; y < crt_height; y++) { >> + for (x = 0; x < crt_width; x++) { >> + int i = (y * crt_width + x) * 4; >> + *(uint32_t *)&s->ds->data[i] = palette[*buf]; >> + buf++; >> + } >> + } >> + break; >> + case SM501_DC_CRT_CONTROL_16BPP: >> + for (y = 0; y < crt_height; y++) { >> + for (x = 0; x < crt_width; x++) { >> + int i = (y * crt_width + x) * 4; >> + uint32_t rgb565 = *(uint16_t*)buf; >> + int r = ((rgb565 >> 11) & 0x1f) << 3; >> + int g = ((rgb565 >> 5) & 0x3f) << 2; >> + int b = ((rgb565 >> 0) & 0x1f) << 3; >> + s->ds->data[i + 0] = b; >> + s->ds->data[i + 1] = g; >> + s->ds->data[i + 2] = r; >> + s->ds->data[i + 3] = 0; >> + buf += 2; >> + } >> + } >> + break; >> + case SM501_DC_CRT_CONTROL_32BPP: >> + for (y = 0; y < crt_height; y++) { >> + for (x = 0; x < crt_width; x++) { >> + int i = (y * crt_width + x) * 4; >> + *(uint32_t *)&s->ds->data[i] = *(uint32_t*)buf; >> + buf += 4; >> + } >> + } >> + break; > > All the cases assume the host is using 32 bpp colours, which is rare I > think. Because s->ds->depth is not checked, it will likely segfault. Right. I will introduce templates like hw/vga_template.h to fit s->ds->depth variety. Thanks! Shin-ichiro KAWASAKI