Improvements for SM501 display controller emulation
v2 cover Improvements for SM501 display controller emulation
Second try, addressing review comments.
BALATON Zoltan (14):
sm501: Fixed code style and a few typos in comments
sm501: Use defines instead of constants where available
sm501: QOMify
sm501: Get rid of base address in draw_hwc_line
sm501: Add emulation of chip connected via PCI
sm501: Add missing arbitration control register
sm501: Fix device endianness
sm501: Fix hardware cursor
sm501: Misc clean ups
sm501: Add support for panel layer
sm501: Add some more missing registers
sm501: Implement reading 2D engine registers
sm501: Add reset function and vmstate descriptor
ppc: Add SM501 device in config for ppc and ppcemb targets
default-configs/ppc-softmmu.mak | 1 + default-configs/ppcemb-softmmu.mak | 1 + hw/display/sm501.c | 1718 ++++++++++++++++++++++-------------- hw/display/sm501_template.h | 94 +- hw/sh4/r2d.c | 11 +- include/hw/devices.h | 5 - 6 files changed, 1089 insertions(+), 741 deletions(-)
Signature · 1 line
2.7.4
PATCH v2 1/14 sm501: Fixed code style and a few typos in comments
--- hw/display/sm501.c | 1132 ++++++++++++++++++++++--------------------- hw/display/sm501_template.h | 52 +- 2 files changed, 594 insertions(+), 590 deletions(-)
hw/display/sm501.c +568 -564
@@ -38,7 +38,7 @@/* * Status: 2010/05/07 * - Minimum implementation for Linux console : mmio regs and CRT layer. -* - 2D grapihcs acceleration partially supported : only fill rectangle. +* - 2D graphics acceleration partially supported : only fill rectangle. * * TODO: * - Panel support
@@ -49,13 +49,13 @@* - Performance tuning */ -//#define DEBUG_SM501 -//#define DEBUG_BITBLT +/*#define DEBUG_SM501*/ +/*#define DEBUG_BITBLT*/ #ifdef DEBUG_SM501 #define SM501_DPRINTF (fmt, ...) printf(fmt, ## __VA_ARGS__)#else -#define SM501_DPRINTF (fmt, ...) do {} while(0) +#define SM501_DPRINTF (fmt, ...) do {} while (0)#endif
@@ -65,379 +65,379 @@/* System Configuration area */ /* System config base */ -#define SM501_SYS_CONFIG (0x000000) +#define SM501_SYS_CONFIG (0x000000)/* config 1 */ -#define SM501_SYSTEM_CONTROL (0x000000) +#define SM501_SYSTEM_CONTROL (0x000000) -#define SM501_SYSCTRL_PANEL_TRISTATE (1<<0) -#define SM501_SYSCTRL_MEM_TRISTATE (1<<1) -#define SM501_SYSCTRL_CRT_TRISTATE (1<<2) +#define SM501_SYSCTRL_PANEL_TRISTATE (1 << 0) +#define SM501_SYSCTRL_MEM_TRISTATE (1 << 1) +#define SM501_SYSCTRL_CRT_TRISTATE (1 << 2) -#define SM501_SYSCTRL_PCI_SLAVE_BURST_MASK (3<<4) -#define SM501_SYSCTRL_PCI_SLAVE_BURST_1 (0<<4) -#define SM501_SYSCTRL_PCI_SLAVE_BURST_2 (1<<4) -#define SM501_SYSCTRL_PCI_SLAVE_BURST_4 (2<<4) -#define SM501_SYSCTRL_PCI_SLAVE_BURST_8 (3<<4) +#define SM501_SYSCTRL_PCI_SLAVE_BURST_MASK (3 << 4) +#define SM501_SYSCTRL_PCI_SLAVE_BURST_1 (0 << 4) +#define SM501_SYSCTRL_PCI_SLAVE_BURST_2 (1 << 4) +#define SM501_SYSCTRL_PCI_SLAVE_BURST_4 (2 << 4) +#define SM501_SYSCTRL_PCI_SLAVE_BURST_8 (3 << 4) -#define SM501_SYSCTRL_PCI_CLOCK_RUN_EN (1<<6) -#define SM501_SYSCTRL_PCI_RETRY_DISABLE (1<<7) -#define SM501_SYSCTRL_PCI_SUBSYS_LOCK (1<<11) -#define SM501_SYSCTRL_PCI_BURST_READ_EN (1<<15) +#define SM501_SYSCTRL_PCI_CLOCK_RUN_EN (1 << 6) +#define SM501_SYSCTRL_PCI_RETRY_DISABLE (1 << 7) +#define SM501_SYSCTRL_PCI_SUBSYS_LOCK (1 << 11) +#define SM501_SYSCTRL_PCI_BURST_READ_EN (1 << 15)/* miscellaneous control */ -#define SM501_MISC_CONTROL (0x000004) +#define SM501_MISC_CONTROL (0x000004) -#define SM501_MISC_BUS_SH (0x0) -#define SM501_MISC_BUS_PCI (0x1) -#define SM501_MISC_BUS_XSCALE (0x2) -#define SM501_MISC_BUS_NEC (0x6) -#define SM501_MISC_BUS_MASK (0x7) +#define SM501_MISC_BUS_SH (0x0) +#define SM501_MISC_BUS_PCI (0x1) +#define SM501_MISC_BUS_XSCALE (0x2) +#define SM501_MISC_BUS_NEC (0x6) +#define SM501_MISC_BUS_MASK (0x7) -#define SM501_MISC_VR_62MB (1<<3) -#define SM501_MISC_CDR_RESET (1<<7) -#define SM501_MISC_USB_LB (1<<8) -#define SM501_MISC_USB_SLAVE (1<<9) -#define SM501_MISC_BL_1 (1<<10) -#define SM501_MISC_MC (1<<11) -#define SM501_MISC_DAC_POWER (1<<12) -#define SM501_MISC_IRQ_INVERT (1<<16) -#define SM501_MISC_SH (1<<17) +#define SM501_MISC_VR_62MB (1 << 3) +#define SM501_MISC_CDR_RESET (1 << 7) +#define SM501_MISC_USB_LB (1 << 8) +#define SM501_MISC_USB_SLAVE (1 << 9) +#define SM501_MISC_BL_1 (1 << 10) +#define SM501_MISC_MC (1 << 11) +#define SM501_MISC_DAC_POWER (1 << 12) +#define SM501_MISC_IRQ_INVERT (1 << 16) +#define SM501_MISC_SH (1 << 17) -#define SM501_MISC_HOLD_EMPTY (0<<18) -#define SM501_MISC_HOLD_8 (1<<18) -#define SM501_MISC_HOLD_16 (2<<18) -#define SM501_MISC_HOLD_24 (3<<18) -#define SM501_MISC_HOLD_32 (4<<18) -#define SM501_MISC_HOLD_MASK (7<<18) +#define SM501_MISC_HOLD_EMPTY (0 << 18) +#define SM501_MISC_HOLD_8 (1 << 18) +#define SM501_MISC_HOLD_16 (2 << 18) +#define SM501_MISC_HOLD_24 (3 << 18) +#define SM501_MISC_HOLD_32 (4 << 18) +#define SM501_MISC_HOLD_MASK (7 << 18) -#define SM501_MISC_FREQ_12 (1<<24) -#define SM501_MISC_PNL_24BIT (1<<25) -#define SM501_MISC_8051_LE (1<<26) +#define SM501_MISC_FREQ_12 (1 << 24) +#define SM501_MISC_PNL_24BIT (1 << 25) +#define SM501_MISC_8051_LE (1 << 26) -#define SM501_GPIO31_0_CONTROL (0x000008) -#define SM501_GPIO63_32_CONTROL (0x00000C) -#define SM501_DRAM_CONTROL (0x000010) +#define SM501_GPIO31_0_CONTROL (0x000008) +#define SM501_GPIO63_32_CONTROL (0x00000C) +#define SM501_DRAM_CONTROL (0x000010)/* command list */ -#define SM501_ARBTRTN_CONTROL (0x000014) +#define SM501_ARBTRTN_CONTROL (0x000014)/* command list */ -#define SM501_COMMAND_LIST_STATUS (0x000024) +#define SM501_COMMAND_LIST_STATUS (0x000024)/* interrupt debug */ -#define SM501_RAW_IRQ_STATUS (0x000028) -#define SM501_RAW_IRQ_CLEAR (0x000028) -#define SM501_IRQ_STATUS (0x00002C) -#define SM501_IRQ_MASK (0x000030) -#define SM501_DEBUG_CONTROL (0x000034) +#define SM501_RAW_IRQ_STATUS (0x000028) +#define SM501_RAW_IRQ_CLEAR (0x000028) +#define SM501_IRQ_STATUS (0x00002C) +#define SM501_IRQ_MASK (0x000030) +#define SM501_DEBUG_CONTROL (0x000034)/* power management */ -#define SM501_POWERMODE_P2X_SRC (1<<29) -#define SM501_POWERMODE_V2X_SRC (1<<20) -#define SM501_POWERMODE_M_SRC (1<<12) -#define SM501_POWERMODE_M1_SRC (1<<4) - -#define SM501_CURRENT_GATE (0x000038) -#define SM501_CURRENT_CLOCK (0x00003C) -#define SM501_POWER_MODE_0_GATE (0x000040) -#define SM501_POWER_MODE_0_CLOCK (0x000044) -#define SM501_POWER_MODE_1_GATE (0x000048) -#define SM501_POWER_MODE_1_CLOCK (0x00004C) -#define SM501_SLEEP_MODE_GATE (0x000050) -#define SM501_POWER_MODE_CONTROL (0x000054) +#define SM501_POWERMODE_P2X_SRC (1 << 29) +#define SM501_POWERMODE_V2X_SRC (1 << 20) +#define SM501_POWERMODE_M_SRC (1 << 12) +#define SM501_POWERMODE_M1_SRC (1 << 4) + +#define SM501_CURRENT_GATE (0x000038) +#define SM501_CURRENT_CLOCK (0x00003C) +#define SM501_POWER_MODE_0_GATE (0x000040) +#define SM501_POWER_MODE_0_CLOCK (0x000044) +#define SM501_POWER_MODE_1_GATE (0x000048) +#define SM501_POWER_MODE_1_CLOCK (0x00004C) +#define SM501_SLEEP_MODE_GATE (0x000050) +#define SM501_POWER_MODE_CONTROL (0x000054)/* power gates for units within the 501 */ -#define SM501_GATE_HOST (0) -#define SM501_GATE_MEMORY (1) -#define SM501_GATE_DISPLAY (2) -#define SM501_GATE_2D_ENGINE (3) -#define SM501_GATE_CSC (4) -#define SM501_GATE_ZVPORT (5) -#define SM501_GATE_GPIO (6) -#define SM501_GATE_UART0 (7) -#define SM501_GATE_UART1 (8) -#define SM501_GATE_SSP (10) -#define SM501_GATE_USB_HOST (11) -#define SM501_GATE_USB_GADGET (12) -#define SM501_GATE_UCONTROLLER (17) -#define SM501_GATE_AC97 (18) +#define SM501_GATE_HOST (0) +#define SM501_GATE_MEMORY (1) +#define SM501_GATE_DISPLAY (2) +#define SM501_GATE_2D_ENGINE (3) +#define SM501_GATE_CSC (4) +#define SM501_GATE_ZVPORT (5) +#define SM501_GATE_GPIO (6) +#define SM501_GATE_UART0 (7) +#define SM501_GATE_UART1 (8) +#define SM501_GATE_SSP (10) +#define SM501_GATE_USB_HOST (11) +#define SM501_GATE_USB_GADGET (12) +#define SM501_GATE_UCONTROLLER (17) +#define SM501_GATE_AC97 (18)/* panel clock */ -#define SM501_CLOCK_P2XCLK (24) +#define SM501_CLOCK_P2XCLK (24)/* crt clock */ -#define SM501_CLOCK_V2XCLK (16) +#define SM501_CLOCK_V2XCLK (16)/* main clock */ -#define SM501_CLOCK_MCLK (8) +#define SM501_CLOCK_MCLK (8)/* SDRAM controller clock */ -#define SM501_CLOCK_M1XCLK (0) +#define SM501_CLOCK_M1XCLK (0)/* config 2 */ -#define SM501_PCI_MASTER_BASE (0x000058) -#define SM501_ENDIAN_CONTROL (0x00005C) -#define SM501_DEVICEID (0x000060) +#define SM501_PCI_MASTER_BASE (0x000058) +#define SM501_ENDIAN_CONTROL (0x00005C) +#define SM501_DEVICEID (0x000060)/* 0x050100A0 */ -#define SM501_DEVICEID_SM501 (0x05010000) -#define SM501_DEVICEID_IDMASK (0xffff0000) -#define SM501_DEVICEID_REVMASK (0x000000ff) +#define SM501_DEVICEID_SM501 (0x05010000) +#define SM501_DEVICEID_IDMASK (0xffff0000) +#define SM501_DEVICEID_REVMASK (0x000000ff) -#define SM501_PLLCLOCK_COUNT (0x000064) -#define SM501_MISC_TIMING (0x000068) -#define SM501_CURRENT_SDRAM_CLOCK (0x00006C) +#define SM501_PLLCLOCK_COUNT (0x000064) +#define SM501_MISC_TIMING (0x000068) +#define SM501_CURRENT_SDRAM_CLOCK (0x00006C) -#define SM501_PROGRAMMABLE_PLL_CONTROL (0x000074) +#define SM501_PROGRAMMABLE_PLL_CONTROL (0x000074)/* GPIO base */ -#define SM501_GPIO (0x010000) -#define SM501_GPIO_DATA_LOW (0x00) -#define SM501_GPIO_DATA_HIGH (0x04) -#define SM501_GPIO_DDR_LOW (0x08) -#define SM501_GPIO_DDR_HIGH (0x0C) -#define SM501_GPIO_IRQ_SETUP (0x10) -#define SM501_GPIO_IRQ_STATUS (0x14) -#define SM501_GPIO_IRQ_RESET (0x14) +#define SM501_GPIO (0x010000) +#define SM501_GPIO_DATA_LOW (0x00) +#define SM501_GPIO_DATA_HIGH (0x04) +#define SM501_GPIO_DDR_LOW (0x08) +#define SM501_GPIO_DDR_HIGH (0x0C) +#define SM501_GPIO_IRQ_SETUP (0x10) +#define SM501_GPIO_IRQ_STATUS (0x14) +#define SM501_GPIO_IRQ_RESET (0x14)/* I2C controller base */ -#define SM501_I2C (0x010040) -#define SM501_I2C_BYTE_COUNT (0x00) -#define SM501_I2C_CONTROL (0x01) -#define SM501_I2C_STATUS (0x02) -#define SM501_I2C_RESET (0x02) -#define SM501_I2C_SLAVE_ADDRESS (0x03) -#define SM501_I2C_DATA (0x04) +#define SM501_I2C (0x010040) +#define SM501_I2C_BYTE_COUNT (0x00) +#define SM501_I2C_CONTROL (0x01) +#define SM501_I2C_STATUS (0x02) +#define SM501_I2C_RESET (0x02) +#define SM501_I2C_SLAVE_ADDRESS (0x03) +#define SM501_I2C_DATA (0x04)/* SSP base */ -#define SM501_SSP (0x020000) +#define SM501_SSP (0x020000)/* Uart 0 base */ -#define SM501_UART0 (0x030000) +#define SM501_UART0 (0x030000)/* Uart 1 base */ -#define SM501_UART1 (0x030020) +#define SM501_UART1 (0x030020)/* USB host port base */ -#define SM501_USB_HOST (0x040000) +#define SM501_USB_HOST (0x040000)/* USB slave/gadget base */ -#define SM501_USB_GADGET (0x060000) +#define SM501_USB_GADGET (0x060000)/* USB slave/gadget data port base */ -#define SM501_USB_GADGET_DATA (0x070000) +#define SM501_USB_GADGET_DATA (0x070000)/* Display controller/video engine base */ -#define SM501_DC (0x080000) +#define SM501_DC (0x080000)/* common defines for the SM501 address registers */ -#define SM501_ADDR_FLIP (1<<31) -#define SM501_ADDR_EXT (1<<27) -#define SM501_ADDR_CS1 (1<<26) -#define SM501_ADDR_MASK (0x3f << 26) +#define SM501_ADDR_FLIP (1 << 31) +#define SM501_ADDR_EXT (1 << 27) +#define SM501_ADDR_CS1 (1 << 26) +#define SM501_ADDR_MASK (0x3f << 26) -#define SM501_FIFO_MASK (0x3 << 16) -#define SM501_FIFO_1 (0x0 << 16) -#define SM501_FIFO_3 (0x1 << 16) -#define SM501_FIFO_7 (0x2 << 16) -#define SM501_FIFO_11 (0x3 << 16) +#define SM501_FIFO_MASK (0x3 << 16) +#define SM501_FIFO_1 (0x0 << 16) +#define SM501_FIFO_3 (0x1 << 16) +#define SM501_FIFO_7 (0x2 << 16) +#define SM501_FIFO_11 (0x3 << 16)/* common registers for panel and the crt */ -#define SM501_OFF_DC_H_TOT (0x000) -#define SM501_OFF_DC_V_TOT (0x008) -#define SM501_OFF_DC_H_SYNC (0x004) -#define SM501_OFF_DC_V_SYNC (0x00C) - -#define SM501_DC_PANEL_CONTROL (0x000) - -#define SM501_DC_PANEL_CONTROL_FPEN (1<<27) -#define SM501_DC_PANEL_CONTROL_BIAS (1<<26) -#define SM501_DC_PANEL_CONTROL_DATA (1<<25) -#define SM501_DC_PANEL_CONTROL_VDD (1<<24) -#define SM501_DC_PANEL_CONTROL_DP (1<<23) - -#define SM501_DC_PANEL_CONTROL_TFT_888 (0<<21) -#define SM501_DC_PANEL_CONTROL_TFT_333 (1<<21) -#define SM501_DC_PANEL_CONTROL_TFT_444 (2<<21) - -#define SM501_DC_PANEL_CONTROL_DE (1<<20) - -#define SM501_DC_PANEL_CONTROL_LCD_TFT (0<<18) -#define SM501_DC_PANEL_CONTROL_LCD_STN8 (1<<18) -#define SM501_DC_PANEL_CONTROL_LCD_STN12 (2<<18) - -#define SM501_DC_PANEL_CONTROL_CP (1<<14) -#define SM501_DC_PANEL_CONTROL_VSP (1<<13) -#define SM501_DC_PANEL_CONTROL_HSP (1<<12) -#define SM501_DC_PANEL_CONTROL_CK (1<<9) -#define SM501_DC_PANEL_CONTROL_TE (1<<8) -#define SM501_DC_PANEL_CONTROL_VPD (1<<7) -#define SM501_DC_PANEL_CONTROL_VP (1<<6) -#define SM501_DC_PANEL_CONTROL_HPD (1<<5) -#define SM501_DC_PANEL_CONTROL_HP (1<<4) -#define SM501_DC_PANEL_CONTROL_GAMMA (1<<3) -#define SM501_DC_PANEL_CONTROL_EN (1<<2) - -#define SM501_DC_PANEL_CONTROL_8BPP (0<<0) -#define SM501_DC_PANEL_CONTROL_16BPP (1<<0) -#define SM501_DC_PANEL_CONTROL_32BPP (2<<0) - - -#define SM501_DC_PANEL_PANNING_CONTROL (0x004) -#define SM501_DC_PANEL_COLOR_KEY (0x008) -#define SM501_DC_PANEL_FB_ADDR (0x00C) -#define SM501_DC_PANEL_FB_OFFSET (0x010) -#define SM501_DC_PANEL_FB_WIDTH (0x014) -#define SM501_DC_PANEL_FB_HEIGHT (0x018) -#define SM501_DC_PANEL_TL_LOC (0x01C) -#define SM501_DC_PANEL_BR_LOC (0x020) -#define SM501_DC_PANEL_H_TOT (0x024) -#define SM501_DC_PANEL_H_SYNC (0x028) -#define SM501_DC_PANEL_V_TOT (0x02C) -#define SM501_DC_PANEL_V_SYNC (0x030) -#define SM501_DC_PANEL_CUR_LINE (0x034) - -#define SM501_DC_VIDEO_CONTROL (0x040) -#define SM501_DC_VIDEO_FB0_ADDR (0x044) -#define SM501_DC_VIDEO_FB_WIDTH (0x048) -#define SM501_DC_VIDEO_FB0_LAST_ADDR (0x04C) -#define SM501_DC_VIDEO_TL_LOC (0x050) -#define SM501_DC_VIDEO_BR_LOC (0x054) -#define SM501_DC_VIDEO_SCALE (0x058) -#define SM501_DC_VIDEO_INIT_SCALE (0x05C) -#define SM501_DC_VIDEO_YUV_CONSTANTS (0x060) -#define SM501_DC_VIDEO_FB1_ADDR (0x064) -#define SM501_DC_VIDEO_FB1_LAST_ADDR (0x068) - -#define SM501_DC_VIDEO_ALPHA_CONTROL (0x080) -#define SM501_DC_VIDEO_ALPHA_FB_ADDR (0x084) -#define SM501_DC_VIDEO_ALPHA_FB_OFFSET (0x088) -#define SM501_DC_VIDEO_ALPHA_FB_LAST_ADDR (0x08C) -#define SM501_DC_VIDEO_ALPHA_TL_LOC (0x090) -#define SM501_DC_VIDEO_ALPHA_BR_LOC (0x094) -#define SM501_DC_VIDEO_ALPHA_SCALE (0x098) -#define SM501_DC_VIDEO_ALPHA_INIT_SCALE (0x09C) -#define SM501_DC_VIDEO_ALPHA_CHROMA_KEY (0x0A0) -#define SM501_DC_VIDEO_ALPHA_COLOR_LOOKUP (0x0A4) - -#define SM501_DC_PANEL_HWC_BASE (0x0F0) -#define SM501_DC_PANEL_HWC_ADDR (0x0F0) -#define SM501_DC_PANEL_HWC_LOC (0x0F4) -#define SM501_DC_PANEL_HWC_COLOR_1_2 (0x0F8) -#define SM501_DC_PANEL_HWC_COLOR_3 (0x0FC) - -#define SM501_HWC_EN (1<<31) - -#define SM501_OFF_HWC_ADDR (0x00) -#define SM501_OFF_HWC_LOC (0x04) -#define SM501_OFF_HWC_COLOR_1_2 (0x08) -#define SM501_OFF_HWC_COLOR_3 (0x0C) - -#define SM501_DC_ALPHA_CONTROL (0x100) -#define SM501_DC_ALPHA_FB_ADDR (0x104) -#define SM501_DC_ALPHA_FB_OFFSET (0x108) -#define SM501_DC_ALPHA_TL_LOC (0x10C) -#define SM501_DC_ALPHA_BR_LOC (0x110) -#define SM501_DC_ALPHA_CHROMA_KEY (0x114) -#define SM501_DC_ALPHA_COLOR_LOOKUP (0x118) - -#define SM501_DC_CRT_CONTROL (0x200) - -#define SM501_DC_CRT_CONTROL_TVP (1<<15) -#define SM501_DC_CRT_CONTROL_CP (1<<14) -#define SM501_DC_CRT_CONTROL_VSP (1<<13) -#define SM501_DC_CRT_CONTROL_HSP (1<<12) -#define SM501_DC_CRT_CONTROL_VS (1<<11) -#define SM501_DC_CRT_CONTROL_BLANK (1<<10) -#define SM501_DC_CRT_CONTROL_SEL (1<<9) -#define SM501_DC_CRT_CONTROL_TE (1<<8) +#define SM501_OFF_DC_H_TOT (0x000) +#define SM501_OFF_DC_V_TOT (0x008) +#define SM501_OFF_DC_H_SYNC (0x004) +#define SM501_OFF_DC_V_SYNC (0x00C) + +#define SM501_DC_PANEL_CONTROL (0x000) + +#define SM501_DC_PANEL_CONTROL_FPEN (1 << 27) +#define SM501_DC_PANEL_CONTROL_BIAS (1 << 26) +#define SM501_DC_PANEL_CONTROL_DATA (1 << 25) +#define SM501_DC_PANEL_CONTROL_VDD (1 << 24) +#define SM501_DC_PANEL_CONTROL_DP (1 << 23) + +#define SM501_DC_PANEL_CONTROL_TFT_888 (0 << 21) +#define SM501_DC_PANEL_CONTROL_TFT_333 (1 << 21) +#define SM501_DC_PANEL_CONTROL_TFT_444 (2 << 21) + +#define SM501_DC_PANEL_CONTROL_DE (1 << 20) + +#define SM501_DC_PANEL_CONTROL_LCD_TFT (0 << 18) +#define SM501_DC_PANEL_CONTROL_LCD_STN8 (1 << 18) +#define SM501_DC_PANEL_CONTROL_LCD_STN12 (2 << 18) + +#define SM501_DC_PANEL_CONTROL_CP (1 << 14) +#define SM501_DC_PANEL_CONTROL_VSP (1 << 13) +#define SM501_DC_PANEL_CONTROL_HSP (1 << 12) +#define SM501_DC_PANEL_CONTROL_CK (1 << 9) +#define SM501_DC_PANEL_CONTROL_TE (1 << 8) +#define SM501_DC_PANEL_CONTROL_VPD (1 << 7) +#define SM501_DC_PANEL_CONTROL_VP (1 << 6) +#define SM501_DC_PANEL_CONTROL_HPD (1 << 5) +#define SM501_DC_PANEL_CONTROL_HP (1 << 4) +#define SM501_DC_PANEL_CONTROL_GAMMA (1 << 3) +#define SM501_DC_PANEL_CONTROL_EN (1 << 2) + +#define SM501_DC_PANEL_CONTROL_8BPP (0 << 0) +#define SM501_DC_PANEL_CONTROL_16BPP (1 << 0) +#define SM501_DC_PANEL_CONTROL_32BPP (2 << 0) + + +#define SM501_DC_PANEL_PANNING_CONTROL (0x004) +#define SM501_DC_PANEL_COLOR_KEY (0x008) +#define SM501_DC_PANEL_FB_ADDR (0x00C) +#define SM501_DC_PANEL_FB_OFFSET (0x010) +#define SM501_DC_PANEL_FB_WIDTH (0x014) +#define SM501_DC_PANEL_FB_HEIGHT (0x018) +#define SM501_DC_PANEL_TL_LOC (0x01C) +#define SM501_DC_PANEL_BR_LOC (0x020) +#define SM501_DC_PANEL_H_TOT (0x024) +#define SM501_DC_PANEL_H_SYNC (0x028) +#define SM501_DC_PANEL_V_TOT (0x02C) +#define SM501_DC_PANEL_V_SYNC (0x030) +#define SM501_DC_PANEL_CUR_LINE (0x034) + +#define SM501_DC_VIDEO_CONTROL (0x040) +#define SM501_DC_VIDEO_FB0_ADDR (0x044) +#define SM501_DC_VIDEO_FB_WIDTH (0x048) +#define SM501_DC_VIDEO_FB0_LAST_ADDR (0x04C) +#define SM501_DC_VIDEO_TL_LOC (0x050) +#define SM501_DC_VIDEO_BR_LOC (0x054) +#define SM501_DC_VIDEO_SCALE (0x058) +#define SM501_DC_VIDEO_INIT_SCALE (0x05C) +#define SM501_DC_VIDEO_YUV_CONSTANTS (0x060) +#define SM501_DC_VIDEO_FB1_ADDR (0x064) +#define SM501_DC_VIDEO_FB1_LAST_ADDR (0x068) + +#define SM501_DC_VIDEO_ALPHA_CONTROL (0x080) +#define SM501_DC_VIDEO_ALPHA_FB_ADDR (0x084) +#define SM501_DC_VIDEO_ALPHA_FB_OFFSET (0x088) +#define SM501_DC_VIDEO_ALPHA_FB_LAST_ADDR (0x08C) +#define SM501_DC_VIDEO_ALPHA_TL_LOC (0x090) +#define SM501_DC_VIDEO_ALPHA_BR_LOC (0x094) +#define SM501_DC_VIDEO_ALPHA_SCALE (0x098) +#define SM501_DC_VIDEO_ALPHA_INIT_SCALE (0x09C) +#define SM501_DC_VIDEO_ALPHA_CHROMA_KEY (0x0A0) +#define SM501_DC_VIDEO_ALPHA_COLOR_LOOKUP (0x0A4) + +#define SM501_DC_PANEL_HWC_BASE (0x0F0) +#define SM501_DC_PANEL_HWC_ADDR (0x0F0) +#define SM501_DC_PANEL_HWC_LOC (0x0F4) +#define SM501_DC_PANEL_HWC_COLOR_1_2 (0x0F8) +#define SM501_DC_PANEL_HWC_COLOR_3 (0x0FC) + +#define SM501_HWC_EN (1 << 31) + +#define SM501_OFF_HWC_ADDR (0x00) +#define SM501_OFF_HWC_LOC (0x04) +#define SM501_OFF_HWC_COLOR_1_2 (0x08) +#define SM501_OFF_HWC_COLOR_3 (0x0C) + +#define SM501_DC_ALPHA_CONTROL (0x100) +#define SM501_DC_ALPHA_FB_ADDR (0x104) +#define SM501_DC_ALPHA_FB_OFFSET (0x108) +#define SM501_DC_ALPHA_TL_LOC (0x10C) +#define SM501_DC_ALPHA_BR_LOC (0x110) +#define SM501_DC_ALPHA_CHROMA_KEY (0x114) +#define SM501_DC_ALPHA_COLOR_LOOKUP (0x118) + +#define SM501_DC_CRT_CONTROL (0x200) + +#define SM501_DC_CRT_CONTROL_TVP (1 << 15) +#define SM501_DC_CRT_CONTROL_CP (1 << 14) +#define SM501_DC_CRT_CONTROL_VSP (1 << 13) +#define SM501_DC_CRT_CONTROL_HSP (1 << 12) +#define SM501_DC_CRT_CONTROL_VS (1 << 11) +#define SM501_DC_CRT_CONTROL_BLANK (1 << 10) +#define SM501_DC_CRT_CONTROL_SEL (1 << 9) +#define SM501_DC_CRT_CONTROL_TE (1 << 8)#define SM501_DC_CRT_CONTROL_PIXEL_MASK (0xF << 4) -#define SM501_DC_CRT_CONTROL_GAMMA (1<<3) -#define SM501_DC_CRT_CONTROL_ENABLE (1<<2) +#define SM501_DC_CRT_CONTROL_GAMMA (1 << 3) +#define SM501_DC_CRT_CONTROL_ENABLE (1 << 2) -#define SM501_DC_CRT_CONTROL_8BPP (0<<0) -#define SM501_DC_CRT_CONTROL_16BPP (1<<0) -#define SM501_DC_CRT_CONTROL_32BPP (2<<0) +#define SM501_DC_CRT_CONTROL_8BPP (0 << 0) +#define SM501_DC_CRT_CONTROL_16BPP (1 << 0) +#define SM501_DC_CRT_CONTROL_32BPP (2 << 0) -#define SM501_DC_CRT_FB_ADDR (0x204) -#define SM501_DC_CRT_FB_OFFSET (0x208) -#define SM501_DC_CRT_H_TOT (0x20C) -#define SM501_DC_CRT_H_SYNC (0x210) -#define SM501_DC_CRT_V_TOT (0x214) -#define SM501_DC_CRT_V_SYNC (0x218) -#define SM501_DC_CRT_SIGNATURE_ANALYZER (0x21C) -#define SM501_DC_CRT_CUR_LINE (0x220) -#define SM501_DC_CRT_MONITOR_DETECT (0x224) +#define SM501_DC_CRT_FB_ADDR (0x204) +#define SM501_DC_CRT_FB_OFFSET (0x208) +#define SM501_DC_CRT_H_TOT (0x20C) +#define SM501_DC_CRT_H_SYNC (0x210) +#define SM501_DC_CRT_V_TOT (0x214) +#define SM501_DC_CRT_V_SYNC (0x218) +#define SM501_DC_CRT_SIGNATURE_ANALYZER (0x21C) +#define SM501_DC_CRT_CUR_LINE (0x220) +#define SM501_DC_CRT_MONITOR_DETECT (0x224) -#define SM501_DC_CRT_HWC_BASE (0x230) -#define SM501_DC_CRT_HWC_ADDR (0x230) -#define SM501_DC_CRT_HWC_LOC (0x234) -#define SM501_DC_CRT_HWC_COLOR_1_2 (0x238) -#define SM501_DC_CRT_HWC_COLOR_3 (0x23C) +#define SM501_DC_CRT_HWC_BASE (0x230) +#define SM501_DC_CRT_HWC_ADDR (0x230) +#define SM501_DC_CRT_HWC_LOC (0x234) +#define SM501_DC_CRT_HWC_COLOR_1_2 (0x238) +#define SM501_DC_CRT_HWC_COLOR_3 (0x23C) -#define SM501_DC_PANEL_PALETTE (0x400) +#define SM501_DC_PANEL_PALETTE (0x400) -#define SM501_DC_VIDEO_PALETTE (0x800) +#define SM501_DC_VIDEO_PALETTE (0x800) -#define SM501_DC_CRT_PALETTE (0xC00) +#define SM501_DC_CRT_PALETTE (0xC00)/* Zoom Video port base */ -#define SM501_ZVPORT (0x090000) +#define SM501_ZVPORT (0x090000)/* AC97/I2S base */ -#define SM501_AC97 (0x0A0000) +#define SM501_AC97 (0x0A0000)/* 8051 micro controller base */ -#define SM501_UCONTROLLER (0x0B0000) +#define SM501_UCONTROLLER (0x0B0000)/* 8051 micro controller SRAM base */ -#define SM501_UCONTROLLER_SRAM (0x0C0000) +#define SM501_UCONTROLLER_SRAM (0x0C0000)/* DMA base */ -#define SM501_DMA (0x0D0000) +#define SM501_DMA (0x0D0000)/* 2d engine base */ -#define SM501_2D_ENGINE (0x100000) -#define SM501_2D_SOURCE (0x00) -#define SM501_2D_DESTINATION (0x04) -#define SM501_2D_DIMENSION (0x08) -#define SM501_2D_CONTROL (0x0C) -#define SM501_2D_PITCH (0x10) -#define SM501_2D_FOREGROUND (0x14) -#define SM501_2D_BACKGROUND (0x18) -#define SM501_2D_STRETCH (0x1C) -#define SM501_2D_COLOR_COMPARE (0x20) -#define SM501_2D_COLOR_COMPARE_MASK (0x24) -#define SM501_2D_MASK (0x28) -#define SM501_2D_CLIP_TL (0x2C) -#define SM501_2D_CLIP_BR (0x30) -#define SM501_2D_MONO_PATTERN_LOW (0x34) -#define SM501_2D_MONO_PATTERN_HIGH (0x38) -#define SM501_2D_WINDOW_WIDTH (0x3C) -#define SM501_2D_SOURCE_BASE (0x40) -#define SM501_2D_DESTINATION_BASE (0x44) -#define SM501_2D_ALPHA (0x48) -#define SM501_2D_WRAP (0x4C) -#define SM501_2D_STATUS (0x50) - -#define SM501_CSC_Y_SOURCE_BASE (0xC8) -#define SM501_CSC_CONSTANTS (0xCC) -#define SM501_CSC_Y_SOURCE_X (0xD0) -#define SM501_CSC_Y_SOURCE_Y (0xD4) -#define SM501_CSC_U_SOURCE_BASE (0xD8) -#define SM501_CSC_V_SOURCE_BASE (0xDC) -#define SM501_CSC_SOURCE_DIMENSION (0xE0) -#define SM501_CSC_SOURCE_PITCH (0xE4) -#define SM501_CSC_DESTINATION (0xE8) -#define SM501_CSC_DESTINATION_DIMENSION (0xEC) -#define SM501_CSC_DESTINATION_PITCH (0xF0) -#define SM501_CSC_SCALE_FACTOR (0xF4) -#define SM501_CSC_DESTINATION_BASE (0xF8) -#define SM501_CSC_CONTROL (0xFC) +#define SM501_2D_ENGINE (0x100000) +#define SM501_2D_SOURCE (0x00) +#define SM501_2D_DESTINATION (0x04) +#define SM501_2D_DIMENSION (0x08) +#define SM501_2D_CONTROL (0x0C) +#define SM501_2D_PITCH (0x10) +#define SM501_2D_FOREGROUND (0x14) +#define SM501_2D_BACKGROUND (0x18) +#define SM501_2D_STRETCH (0x1C) +#define SM501_2D_COLOR_COMPARE (0x20) +#define SM501_2D_COLOR_COMPARE_MASK (0x24) +#define SM501_2D_MASK (0x28) +#define SM501_2D_CLIP_TL (0x2C) +#define SM501_2D_CLIP_BR (0x30) +#define SM501_2D_MONO_PATTERN_LOW (0x34) +#define SM501_2D_MONO_PATTERN_HIGH (0x38) +#define SM501_2D_WINDOW_WIDTH (0x3C) +#define SM501_2D_SOURCE_BASE (0x40) +#define SM501_2D_DESTINATION_BASE (0x44) +#define SM501_2D_ALPHA (0x48) +#define SM501_2D_WRAP (0x4C) +#define SM501_2D_STATUS (0x50) + +#define SM501_CSC_Y_SOURCE_BASE (0xC8) +#define SM501_CSC_CONSTANTS (0xCC) +#define SM501_CSC_Y_SOURCE_X (0xD0) +#define SM501_CSC_Y_SOURCE_Y (0xD4) +#define SM501_CSC_U_SOURCE_BASE (0xD8) +#define SM501_CSC_V_SOURCE_BASE (0xDC) +#define SM501_CSC_SOURCE_DIMENSION (0xE0) +#define SM501_CSC_SOURCE_PITCH (0xE4) +#define SM501_CSC_DESTINATION (0xE8) +#define SM501_CSC_DESTINATION_DIMENSION (0xEC) +#define SM501_CSC_DESTINATION_PITCH (0xF0) +#define SM501_CSC_SCALE_FACTOR (0xF4) +#define SM501_CSC_DESTINATION_BASE (0xF8) +#define SM501_CSC_CONTROL (0xFC)/* 2d engine data port base */ -#define SM501_2D_ENGINE_DATA (0x110000) +#define SM501_2D_ENGINE_DATA (0x110000)/* end of register definitions */
@@ -446,12 +446,12 @@/* SM501 local memory size taken from "linux/drivers/mfd/sm501.c" */ static const uint32_t sm501_mem_local_size[] = { - [0 ] =4 *1024 *1024 , - [1 ] =8 *1024 *1024 , - [2 ] =16 *1024 *1024 , - [3 ] =32 *1024 *1024 , - [4 ] =64 *1024 *1024 , - [5 ] =2 *1024 *1024 , + [0 ] =4 *1024 *1024 , + [1 ] =8 *1024 *1024 , + [2 ] =16 *1024 *1024 , + [3 ] =32 *1024 *1024 , + [4 ] =64 *1024 *1024 , + [5 ] =2 *1024 *1024 , };#define get_local_mem_size (s) sm501_mem_local_size[(s)->local_mem_size_index]
@@ -462,7 +462,7 @@ typedef struct SM501State {/* status & internal resources */ hwaddr base;uint32_t local_mem_size_index; -uint8_t * local_mem; +uint8_t *local_mem;MemoryRegion local_mem_region;uint32_t last_width;uint32_t last_height;
@@ -536,13 +536,13 @@ static uint32_t get_local_mem_size_index(uint32_t size)int i, index =0 ;for (i =0 ; i <ARRAY_SIZE (sm501_mem_local_size); i++) { -uint32_t new_size = sm501_mem_local_size[i]; -if (new_size >= size) { -if (norm_size ==0 || norm_size > new_size) { - norm_size = new_size; - index = i; - } - } +uint32_t new_size = sm501_mem_local_size[i]; +if (new_size >= size) { +if (norm_size ==0 || norm_size > new_size) { + norm_size = new_size; + index = i; + } + } }return index;
@@ -637,7 +637,7 @@ static int within_hwc_y_range(SM501State *state, int y, int crt)return (hwc_y <= y && y < hwc_y +SM501_HWC_HEIGHT ); } -static void sm501_2d_operation (SM501State * s) +static void sm501_2d_operation (SM501State *s) {/* obtain operation parameters */ int operation = (s->twoD_control >>16 ) &0x1f ;
@@ -653,8 +653,8 @@ static void sm501_2d_operation(SM501State * s)int addressing = (s->twoD_stretch >>16 ) &0xF ;/* get frame buffer info */ -uint8_t * src = s->local_mem + (s->twoD_source_base &0x03FFFFFF ); -uint8_t * dst = s->local_mem + (s->twoD_destination_base &0x03FFFFFF ); +uint8_t *src = s->local_mem + (s->twoD_source_base &0x03FFFFFF ); +uint8_t *dst = s->local_mem + (s->twoD_destination_base &0x03FFFFFF );int src_width = (s->dc_crt_h_total &0x00000FFF ) +1 ;int dst_width = (s->dc_crt_h_total &0x00000FFF ) +1 ;
@@ -671,20 +671,20 @@ static void sm501_2d_operation(SM501State * s)switch (operation) {case 0x00 :/* copy area */ -#define COPY_AREA (_bpp, _pixel_type, rtl) { \ - int y, x, index_d, index_s; \ - for (y = 0; y < operation_height; y++) { \ - for (x = 0; x < operation_width; x++) { \ - if (rtl) { \ - index_s = ((src_y - y) * src_width + src_x - x) * _bpp; \ - index_d = ((dst_y - y) * dst_width + dst_x - x) * _bpp; \ - } else { \ - index_s = ((src_y + y) * src_width + src_x + x) * _bpp; \ - index_d = ((dst_y + y) * dst_width + dst_x + x) * _bpp; \ - } \ - *(_pixel_type*)&dst[index_d] = *(_pixel_type*)&src[index_s];\ - } \ - } \ +#define COPY_AREA (_bpp, _pixel_type, rtl) { \ + int y, x, index_d, index_s; \ + for (y = 0; y < operation_height; y++) { \ + for (x = 0; x < operation_width; x++) { \ + if (rtl) { \ + index_s = ((src_y - y) * src_width + src_x - x) * _bpp; \ + index_d = ((dst_y - y) * dst_width + dst_x - x) * _bpp; \ + } else { \ + index_s = ((src_y + y) * src_width + src_x + x) * _bpp; \ + index_d = ((dst_y + y) * dst_width + dst_x + x) * _bpp; \ + } \ + *(_pixel_type *)&dst[index_d] = *(_pixel_type *)&src[index_s];\ + } \ + } \ }switch (format_flags) {case 0 :
@@ -705,7 +705,7 @@ static void sm501_2d_operation(SM501State * s)for (y =0 ; y < operation_height; y++) { \for (x =0 ; x < operation_width; x++) { \int index = ((dst_y + y) * dst_width + dst_x + x) * _bpp; \ - *(_pixel_type *)&dst[index] = (_pixel_type )color; \ + *(_pixel_type *)&dst[index] = (_pixel_type )color; \ } \ } \ }
@@ -733,50 +733,50 @@ static void sm501_2d_operation(SM501State * s)static uint64_t sm501_system_config_read (void *opaque,hwaddr addr,unsigned size) { -SM501State * s = (SM501State *)opaque; +SM501State *s = (SM501State *)opaque;uint32_t ret =0 ;SM501_DPRINTF ("sm501 system config regs : read addr=%x\n" , (int )addr); -switch (addr) { +switch (addr) {case SM501_SYSTEM_CONTROL : - ret = s->system_control; -break ; + ret = s->system_control; +break ;case SM501_MISC_CONTROL : - ret = s->misc_control; -break ; + ret = s->misc_control; +break ;case SM501_GPIO31_0_CONTROL : - ret = s->gpio_31_0_control; -break ; + ret = s->gpio_31_0_control; +break ;case SM501_GPIO63_32_CONTROL : - ret = s->gpio_63_32_control; -break ; + ret = s->gpio_63_32_control; +break ;case SM501_DEVICEID : - ret =0x050100A0 ; -break ; + ret =0x050100A0 ; +break ;case SM501_DRAM_CONTROL : - ret = (s->dram_control &0x07F107C0 ) | s->local_mem_size_index <<13 ; -break ; + ret = (s->dram_control &0x07F107C0 ) | s->local_mem_size_index <<13 ; +break ;case SM501_IRQ_MASK : - ret = s->irq_mask; -break ; + ret = s->irq_mask; +break ;case SM501_MISC_TIMING : -/* TODO : simulate gate control */ - ret = s->misc_timing; -break ; +/* TODO : simulate gate control */ + ret = s->misc_timing; +break ;case SM501_CURRENT_GATE : -/* TODO : simulate gate control */ - ret =0x00021807 ; -break ; +/* TODO : simulate gate control */ + ret =0x00021807 ; +break ;case SM501_CURRENT_CLOCK : - ret =0x2A1A0A09 ; -break ; + ret =0x2A1A0A09 ; +break ;case SM501_POWER_MODE_CONTROL : - ret = s->power_mode_control; -break ; + ret = s->power_mode_control; +break ;default : -printf ("sm501 system config : not implemented register read." -" addr=%x\n" , (int )addr); +printf ("sm501 system config : not implemented register read." +" addr=%x\n" , (int )addr);abort (); }
@@ -786,47 +786,47 @@ static uint64_t sm501_system_config_read(void *opaque, hwaddr addr,static void sm501_system_config_write (void *opaque,hwaddr addr,uint64_t value,unsigned size) { -SM501State * s = (SM501State *)opaque; +SM501State *s = (SM501State *)opaque;SM501_DPRINTF ("sm501 system config regs : write addr=%x, val=%x\n" , - (uint32_t )addr, (uint32_t )value); + (uint32_t )addr, (uint32_t )value); -switch (addr) { +switch (addr) {case SM501_SYSTEM_CONTROL : - s->system_control = value &0xE300B8F7 ; -break ; + s->system_control = value &0xE300B8F7 ; +break ;case SM501_MISC_CONTROL : - s->misc_control = value &0xFF7FFF20 ; -break ; + s->misc_control = value &0xFF7FFF20 ; +break ;case SM501_GPIO31_0_CONTROL : - s->gpio_31_0_control = value; -break ; + s->gpio_31_0_control = value; +break ;case SM501_GPIO63_32_CONTROL : - s->gpio_63_32_control = value; -break ; + s->gpio_63_32_control = value; +break ;case SM501_DRAM_CONTROL : - s->local_mem_size_index = (value >>13 ) &0x7 ; -/* rODO : check validity of size change */ - s->dram_control |= value &0x7FFFFFC3 ; -break ; + s->local_mem_size_index = (value >>13 ) &0x7 ; +/* TODO : check validity of size change */ + s->dram_control |= value &0x7FFFFFC3 ; +break ;case SM501_IRQ_MASK : - s->irq_mask = value; -break ; + s->irq_mask = value; +break ;case SM501_MISC_TIMING : - s->misc_timing = value &0xF31F1FFF ; -break ; + s->misc_timing = value &0xF31F1FFF ; +break ;case SM501_POWER_MODE_0_GATE :case SM501_POWER_MODE_1_GATE :case SM501_POWER_MODE_0_CLOCK :case SM501_POWER_MODE_1_CLOCK : -/* TODO : simulate gate & clock control */ -break ; +/* TODO : simulate gate & clock control */ +break ;case SM501_POWER_MODE_CONTROL : - s->power_mode_control = value &0x00000003 ; -break ; + s->power_mode_control = value &0x00000003 ; +break ;default : -printf ("sm501 system config : not implemented register write." -" addr=%x, val=%x\n" , (int )addr, (uint32_t )value); +printf ("sm501 system config : not implemented register write." +" addr=%x, val=%x\n" , (int )addr, (uint32_t )value);abort (); } }
@@ -843,119 +843,119 @@ static const MemoryRegionOps sm501_system_config_ops = {static uint32_t sm501_palette_read (void *opaque,hwaddr addr) { -SM501State * s = (SM501State *)opaque; +SM501State *s = (SM501State *)opaque;SM501_DPRINTF ("sm501 palette read addr=%x\n" , (int )addr);/* TODO : consider BYTE/WORD access */ /* TODO : consider endian */ assert (range_covers_byte (0 ,0x400 *3 , addr)); -return *(uint32_t *)&s->dc_palette[addr]; +return *(uint32_t *)&s->dc_palette[addr]; } -static void sm501_palette_write (void *opaque, -hwaddr addr,uint32_t value) +static void sm501_palette_write (void *opaque,hwaddr addr, +uint32_t value) { -SM501State * s = (SM501State *)opaque; +SM501State *s = (SM501State *)opaque;SM501_DPRINTF ("sm501 palette write addr=%x, val=%x\n" , - (int )addr, value); + (int )addr, value);/* TODO : consider BYTE/WORD access */ /* TODO : consider endian */ assert (range_covers_byte (0 ,0x400 *3 , addr)); - *(uint32_t *)&s->dc_palette[addr] = value; + *(uint32_t *)&s->dc_palette[addr] = value; }static uint64_t sm501_disp_ctrl_read (void *opaque,hwaddr addr,unsigned size) { -SM501State * s = (SM501State *)opaque; +SM501State *s = (SM501State *)opaque;uint32_t ret =0 ;SM501_DPRINTF ("sm501 disp ctrl regs : read addr=%x\n" , (int )addr); -switch (addr) { +switch (addr) {case SM501_DC_PANEL_CONTROL : - ret = s->dc_panel_control; -break ; + ret = s->dc_panel_control; +break ;case SM501_DC_PANEL_PANNING_CONTROL : - ret = s->dc_panel_panning_control; -break ; + ret = s->dc_panel_panning_control; +break ;case SM501_DC_PANEL_FB_ADDR : - ret = s->dc_panel_fb_addr; -break ; + ret = s->dc_panel_fb_addr; +break ;case SM501_DC_PANEL_FB_OFFSET : - ret = s->dc_panel_fb_offset; -break ; + ret = s->dc_panel_fb_offset; +break ;case SM501_DC_PANEL_FB_WIDTH : - ret = s->dc_panel_fb_width; -break ; + ret = s->dc_panel_fb_width; +break ;case SM501_DC_PANEL_FB_HEIGHT : - ret = s->dc_panel_fb_height; -break ; + ret = s->dc_panel_fb_height; +break ;case SM501_DC_PANEL_TL_LOC : - ret = s->dc_panel_tl_location; -break ; + ret = s->dc_panel_tl_location; +break ;case SM501_DC_PANEL_BR_LOC : - ret = s->dc_panel_br_location; -break ; + ret = s->dc_panel_br_location; +break ;case SM501_DC_PANEL_H_TOT : - ret = s->dc_panel_h_total; -break ; + ret = s->dc_panel_h_total; +break ;case SM501_DC_PANEL_H_SYNC : - ret = s->dc_panel_h_sync; -break ; + ret = s->dc_panel_h_sync; +break ;case SM501_DC_PANEL_V_TOT : - ret = s->dc_panel_v_total; -break ; + ret = s->dc_panel_v_total; +break ;case SM501_DC_PANEL_V_SYNC : - ret = s->dc_panel_v_sync; -break ; + ret = s->dc_panel_v_sync; +break ;case SM501_DC_CRT_CONTROL : - ret = s->dc_crt_control; -break ; + ret = s->dc_crt_control; +break ;case SM501_DC_CRT_FB_ADDR : - ret = s->dc_crt_fb_addr; -break ; + ret = s->dc_crt_fb_addr; +break ;case SM501_DC_CRT_FB_OFFSET : - ret = s->dc_crt_fb_offset; -break ; + ret = s->dc_crt_fb_offset; +break ;case SM501_DC_CRT_H_TOT : - ret = s->dc_crt_h_total; -break ; + ret = s->dc_crt_h_total; +break ;case SM501_DC_CRT_H_SYNC : - ret = s->dc_crt_h_sync; -break ; + ret = s->dc_crt_h_sync; +break ;case SM501_DC_CRT_V_TOT : - ret = s->dc_crt_v_total; -break ; + ret = s->dc_crt_v_total; +break ;case SM501_DC_CRT_V_SYNC : - ret = s->dc_crt_v_sync; -break ; + ret = s->dc_crt_v_sync; +break ;case SM501_DC_CRT_HWC_ADDR : - ret = s->dc_crt_hwc_addr; -break ; + ret = s->dc_crt_hwc_addr; +break ;case SM501_DC_CRT_HWC_LOC : - ret = s->dc_crt_hwc_location; -break ; + ret = s->dc_crt_hwc_location; +break ;case SM501_DC_CRT_HWC_COLOR_1_2 : - ret = s->dc_crt_hwc_color_1_2; -break ; + ret = s->dc_crt_hwc_color_1_2; +break ;case SM501_DC_CRT_HWC_COLOR_3 : - ret = s->dc_crt_hwc_color_3; -break ; + ret = s->dc_crt_hwc_color_3; +break ; - case SM501_DC_PANEL_PALETTE ... SM501_DC_PANEL_PALETTE + 0x400*3 - 4: + case SM501_DC_PANEL_PALETTE ... SM501_DC_PANEL_PALETTE + 0x400 * 3 - 4: ret =sm501_palette_read (opaque, addr -SM501_DC_PANEL_PALETTE );break ;default : -printf ("sm501 disp ctrl : not implemented register read." -" addr=%x\n" , (int )addr); +printf ("sm501 disp ctrl : not implemented register read." +" addr=%x\n" , (int )addr);abort (); }
@@ -965,104 +965,104 @@ static uint64_t sm501_disp_ctrl_read(void *opaque, hwaddr addr,static void sm501_disp_ctrl_write (void *opaque,hwaddr addr,uint64_t value,unsigned size) { -SM501State * s = (SM501State *)opaque; +SM501State *s = (SM501State *)opaque;SM501_DPRINTF ("sm501 disp ctrl regs : write addr=%x, val=%x\n" , - (unsigned )addr, (unsigned )value); + (unsigned )addr, (unsigned )value); -switch (addr) { +switch (addr) {case SM501_DC_PANEL_CONTROL : - s->dc_panel_control = value &0x0FFF73FF ; -break ; + s->dc_panel_control = value &0x0FFF73FF ; +break ;case SM501_DC_PANEL_PANNING_CONTROL : - s->dc_panel_panning_control = value &0xFF3FFF3F ; -break ; + s->dc_panel_panning_control = value &0xFF3FFF3F ; +break ;case SM501_DC_PANEL_FB_ADDR : - s->dc_panel_fb_addr = value &0x8FFFFFF0 ; -break ; + s->dc_panel_fb_addr = value &0x8FFFFFF0 ; +break ;case SM501_DC_PANEL_FB_OFFSET : - s->dc_panel_fb_offset = value &0x3FF03FF0 ; -break ; + s->dc_panel_fb_offset = value &0x3FF03FF0 ; +break ;case SM501_DC_PANEL_FB_WIDTH : - s->dc_panel_fb_width = value &0x0FFF0FFF ; -break ; + s->dc_panel_fb_width = value &0x0FFF0FFF ; +break ;case SM501_DC_PANEL_FB_HEIGHT : - s->dc_panel_fb_height = value &0x0FFF0FFF ; -break ; + s->dc_panel_fb_height = value &0x0FFF0FFF ; +break ;case SM501_DC_PANEL_TL_LOC : - s->dc_panel_tl_location = value &0x07FF07FF ; -break ; + s->dc_panel_tl_location = value &0x07FF07FF ; +break ;case SM501_DC_PANEL_BR_LOC : - s->dc_panel_br_location = value &0x07FF07FF ; -break ; + s->dc_panel_br_location = value &0x07FF07FF ; +break ;case SM501_DC_PANEL_H_TOT : - s->dc_panel_h_total = value &0x0FFF0FFF ; -break ; + s->dc_panel_h_total = value &0x0FFF0FFF ; +break ;case SM501_DC_PANEL_H_SYNC : - s->dc_panel_h_sync = value &0x00FF0FFF ; -break ; + s->dc_panel_h_sync = value &0x00FF0FFF ; +break ;case SM501_DC_PANEL_V_TOT : - s->dc_panel_v_total = value &0x0FFF0FFF ; -break ; + s->dc_panel_v_total = value &0x0FFF0FFF ; +break ;case SM501_DC_PANEL_V_SYNC : - s->dc_panel_v_sync = value &0x003F0FFF ; -break ; + s->dc_panel_v_sync = value &0x003F0FFF ; +break ;case SM501_DC_PANEL_HWC_ADDR : - s->dc_panel_hwc_addr = value &0x8FFFFFF0 ; -break ; + s->dc_panel_hwc_addr = value &0x8FFFFFF0 ; +break ;case SM501_DC_PANEL_HWC_LOC : - s->dc_panel_hwc_location = value &0x0FFF0FFF ; -break ; + s->dc_panel_hwc_location = value &0x0FFF0FFF ; +break ;case SM501_DC_PANEL_HWC_COLOR_1_2 : - s->dc_panel_hwc_color_1_2 = value; -break ; + s->dc_panel_hwc_color_1_2 = value; +break ;case SM501_DC_PANEL_HWC_COLOR_3 : - s->dc_panel_hwc_color_3 = value &0x0000FFFF ; -break ; + s->dc_panel_hwc_color_3 = value &0x0000FFFF ; +break ;case SM501_DC_CRT_CONTROL : - s->dc_crt_control = value &0x0003FFFF ; -break ; + s->dc_crt_control = value &0x0003FFFF ; +break ;case SM501_DC_CRT_FB_ADDR : - s->dc_crt_fb_addr = value &0x8FFFFFF0 ; -break ; + s->dc_crt_fb_addr = value &0x8FFFFFF0 ; +break ;case SM501_DC_CRT_FB_OFFSET : - s->dc_crt_fb_offset = value &0x3FF03FF0 ; -break ; + s->dc_crt_fb_offset = value &0x3FF03FF0 ; +break ;case SM501_DC_CRT_H_TOT : - s->dc_crt_h_total = value &0x0FFF0FFF ; -break ; + s->dc_crt_h_total = value &0x0FFF0FFF ; +break ;case SM501_DC_CRT_H_SYNC : - s->dc_crt_h_sync = value &0x00FF0FFF ; -break ; + s->dc_crt_h_sync = value &0x00FF0FFF ; +break ;case SM501_DC_CRT_V_TOT : - s->dc_crt_v_total = value &0x0FFF0FFF ; -break ; + s->dc_crt_v_total = value &0x0FFF0FFF ; +break ;case SM501_DC_CRT_V_SYNC : - s->dc_crt_v_sync = value &0x003F0FFF ; -break ; + s->dc_crt_v_sync = value &0x003F0FFF ; +break ;case SM501_DC_CRT_HWC_ADDR : - s->dc_crt_hwc_addr = value &0x8FFFFFF0 ; -break ; + s->dc_crt_hwc_addr = value &0x8FFFFFF0 ; +break ;case SM501_DC_CRT_HWC_LOC : - s->dc_crt_hwc_location = value &0x0FFF0FFF ; -break ; + s->dc_crt_hwc_location = value &0x0FFF0FFF ; +break ;case SM501_DC_CRT_HWC_COLOR_1_2 : - s->dc_crt_hwc_color_1_2 = value; -break ; + s->dc_crt_hwc_color_1_2 = value; +break ;case SM501_DC_CRT_HWC_COLOR_3 : - s->dc_crt_hwc_color_3 = value &0x0000FFFF ; -break ; + s->dc_crt_hwc_color_3 = value &0x0000FFFF ; +break ; - case SM501_DC_PANEL_PALETTE ... SM501_DC_PANEL_PALETTE + 0x400*3 - 4: + case SM501_DC_PANEL_PALETTE ... SM501_DC_PANEL_PALETTE + 0x400 * 3 - 4:sm501_palette_write (opaque, addr -SM501_DC_PANEL_PALETTE , value);break ;default : -printf ("sm501 disp ctrl : not implemented register write." -" addr=%x, val=%x\n" , (int )addr, (unsigned )value); +printf ("sm501 disp ctrl : not implemented register write." +" addr=%x, val=%x\n" , (int )addr, (unsigned )value);abort (); } }
@@ -1080,11 +1080,11 @@ static const MemoryRegionOps sm501_disp_ctrl_ops = {static uint64_t sm501_2d_engine_read (void *opaque,hwaddr addr,unsigned size) { -SM501State * s = (SM501State *)opaque; +SM501State *s = (SM501State *)opaque;uint32_t ret =0 ;SM501_DPRINTF ("sm501 2d engine regs : read addr=%x\n" , (int )addr); -switch (addr) { +switch (addr) {case SM501_2D_SOURCE_BASE : ret = s->twoD_source_base;break ;
@@ -1100,11 +1100,11 @@ static uint64_t sm501_2d_engine_read(void *opaque, hwaddr addr,static void sm501_2d_engine_write (void *opaque,hwaddr addr,uint64_t value,unsigned size) { -SM501State * s = (SM501State *)opaque; +SM501State *s = (SM501State *)opaque;SM501_DPRINTF ("sm501 2d engine regs : write addr=%x, val=%x\n" , (unsigned )addr, (unsigned )value); -switch (addr) { +switch (addr) {case SM501_2D_SOURCE : s->twoD_source = value;break ;
@@ -1168,9 +1168,9 @@ static const MemoryRegionOps sm501_2d_engine_ops = {/* draw line functions for all console modes */ typedef void draw_line_func (uint8_t *d,const uint8_t *s, -int width,const uint32_t *pal); +int width,const uint32_t *pal); -typedef void draw_hwc_line_func (SM501State * s,int crt,uint8_t * palette, +typedef void draw_hwc_line_func (SM501State *s,int crt,uint8_t *palette,int c_y,uint8_t *d,int width);#define DEPTH 8
@@ -1197,7 +1197,7 @@ typedef void draw_hwc_line_func(SM501State * s, int crt, uint8_t * palette,#define DEPTH 32#include "sm501_template.h" -static draw_line_func * draw_line8_funcs[] = { +static draw_line_func *draw_line8_funcs[] = { draw_line8_8, draw_line8_15, draw_line8_16,
@@ -1207,7 +1207,7 @@ static draw_line_func * draw_line8_funcs[] = { draw_line8_16bgr, }; -static draw_line_func * draw_line16_funcs[] = { +static draw_line_func *draw_line16_funcs[] = { draw_line16_8, draw_line16_15, draw_line16_16,
@@ -1217,7 +1217,7 @@ static draw_line_func * draw_line16_funcs[] = { draw_line16_16bgr, }; -static draw_line_func * draw_line32_funcs[] = { +static draw_line_func *draw_line32_funcs[] = { draw_line32_8, draw_line32_15, draw_line32_16,
@@ -1227,7 +1227,7 @@ static draw_line_func * draw_line32_funcs[] = { draw_line32_16bgr, }; -static draw_hwc_line_func * draw_hwc_line_funcs[] = { +static draw_hwc_line_func *draw_hwc_line_funcs[] = { draw_hwc_line_8, draw_hwc_line_15, draw_hwc_line_16,
@@ -1242,7 +1242,7 @@ static inline int get_depth_index(DisplaySurface *surface)switch (surface_bits_per_pixel (surface)) {default :case 8 : -return 0 ; +return 0 ;case 15 :return 1 ;case 16 :
@@ -1256,22 +1256,22 @@ static inline int get_depth_index(DisplaySurface *surface) } } -static void sm501_draw_crt (SM501State * s) +static void sm501_draw_crt (SM501State *s) {DisplaySurface *surface =qemu_console_surface (s->con);int y;int width = (s->dc_crt_h_total &0x00000FFF ) +1 ;int height = (s->dc_crt_v_total &0x00000FFF ) +1 ; -uint8_t * src = s->local_mem; +uint8_t *src = s->local_mem;int src_bpp =0 ;int dst_bpp =surface_bytes_per_pixel (surface); -uint32_t * palette = (uint32_t *)&s->dc_palette[SM501_DC_CRT_PALETTE - -SM501_DC_PANEL_PALETTE ]; +uint32_t *palette = (uint32_t *)&s->dc_palette[SM501_DC_CRT_PALETTE - +SM501_DC_PANEL_PALETTE ];uint8_t hwc_palette[3 *3 ];int ds_depth_index =get_depth_index (surface); -draw_line_func * draw_line =NULL ; -draw_hwc_line_func * draw_hwc_line =NULL ; +draw_line_func *draw_line =NULL ; +draw_hwc_line_func *draw_hwc_line =NULL ;int full_update =0 ;int y_start =-1 ;ram_addr_t page_min = ~0l ;
@@ -1281,22 +1281,22 @@ static void sm501_draw_crt(SM501State * s)/* choose draw_line function */ switch (s->dc_crt_control &3 ) {case SM501_DC_CRT_CONTROL_8BPP : - src_bpp =1 ; - draw_line = draw_line8_funcs[ds_depth_index]; -break ; + src_bpp =1 ; + draw_line = draw_line8_funcs[ds_depth_index]; +break ;case SM501_DC_CRT_CONTROL_16BPP : - src_bpp =2 ; - draw_line = draw_line16_funcs[ds_depth_index]; -break ; + src_bpp =2 ; + draw_line = draw_line16_funcs[ds_depth_index]; +break ;case SM501_DC_CRT_CONTROL_32BPP : - src_bpp =4 ; - draw_line = draw_line32_funcs[ds_depth_index]; -break ; + src_bpp =4 ; + draw_line = draw_line32_funcs[ds_depth_index]; +break ;default : -printf ("sm501 draw crt : invalid DC_CRT_CONTROL=%x.\n" , - s->dc_crt_control); +printf ("sm501 draw crt : invalid DC_CRT_CONTROL=%x.\n" , + s->dc_crt_control);abort (); -break ; +break ; }/* set up to draw hardware cursor */
@@ -1319,61 +1319,65 @@ static void sm501_draw_crt(SM501State * s)if (s->last_width != width || s->last_height != height) {qemu_console_resize (s->con, width, height); surface =qemu_console_surface (s->con); - s->last_width = width; - s->last_height = height; - full_update =1 ; + s->last_width = width; + s->last_height = height; + full_update =1 ; }/* draw each line according to conditions */ memory_region_sync_dirty_bitmap (&s->local_mem_region);for (y =0 ; y < height; y++) { -int update_hwc = draw_hwc_line ?within_hwc_y_range (s, y,1 ) :0 ; -int update = full_update || update_hwc; +int update_hwc = draw_hwc_line ?within_hwc_y_range (s, y,1 ) :0 ; +int update = full_update || update_hwc;ram_addr_t page0 = offset;ram_addr_t page1 = offset + width * src_bpp -1 ; -/* check dirty flags for each line */ +/* check dirty flags for each line */ update =memory_region_get_dirty (&s->local_mem_region, page0, page1 - page0,DIRTY_MEMORY_VGA ); -/* draw line and change status */ -if (update) { +/* draw line and change status */ +if (update) {uint8_t *d =surface_data (surface); d += y * width * dst_bpp;/* draw graphics layer */ draw_line (d, src, width, palette); -/* draw haredware cursor */ +/* draw hardware cursor */ if (update_hwc) {draw_hwc_line (s,1 , hwc_palette, y -get_hwc_y (s,1 ), d, width); } -if (y_start <0 ) - y_start = y; -if (page0 < page_min) - page_min = page0; -if (page1 > page_max) - page_max = page1; - }else { -if (y_start >=0 ) { -/* flush to display */ +if (y_start <0 ) { + y_start = y; + } +if (page0 < page_min) { + page_min = page0; + } +if (page1 > page_max) { + page_max = page1; + } + }else { +if (y_start >=0 ) { +/* flush to display */ dpy_gfx_update (s->con,0 , y_start, width, y - y_start); - y_start =-1 ; - } - } + y_start =-1 ; + } + } - src += width * src_bpp; - offset += width * src_bpp; + src += width * src_bpp; + offset += width * src_bpp; }/* complete flush to display */ -if (y_start >=0 ) +if (y_start >=0 ) {dpy_gfx_update (s->con,0 , y_start, width, y - y_start); + }/* clear dirty flags */ if (page_min != ~0l ) { -memory_region_reset_dirty (&s->local_mem_region, +memory_region_reset_dirty (&s->local_mem_region, page_min, page_max +TARGET_PAGE_SIZE ,DIRTY_MEMORY_VGA ); }
@@ -1381,10 +1385,11 @@ static void sm501_draw_crt(SM501State * s)static void sm501_update_display (void *opaque) { -SM501State * s = (SM501State *)opaque; +SM501State *s = (SM501State *)opaque; -if (s->dc_crt_control &SM501_DC_CRT_CONTROL_ENABLE ) -sm501_draw_crt (s); +if (s->dc_crt_control &SM501_DC_CRT_CONTROL_ENABLE ) { +sm501_draw_crt (s); + } }static const GraphicHwOps sm501_ops = {
@@ -1394,19 +1399,18 @@ static const GraphicHwOps sm501_ops = {void sm501_init (MemoryRegion *address_space_mem,uint32_t base,uint32_t local_mem_bytes,qemu_irq irq,Chardev *chr) { -SM501State * s; +SM501State *s;DeviceState *dev;MemoryRegion *sm501_system_config =g_new (MemoryRegion,1 );MemoryRegion *sm501_disp_ctrl =g_new (MemoryRegion,1 );MemoryRegion *sm501_2d_engine =g_new (MemoryRegion,1 );/* allocate management data region */ - s = (SM501State *)g_malloc0 (sizeof (SM501State)); + s =g_new0 (SM501State,1 ); s->base = base; - s->local_mem_size_index - =get_local_mem_size_index (local_mem_bytes); + s->local_mem_size_index =get_local_mem_size_index (local_mem_bytes);SM501_DPRINTF ("local mem size=%x. index=%d\n" ,get_local_mem_size (s), - s->local_mem_size_index); + s->local_mem_size_index); s->system_control =0x00100000 ; s->misc_control =0x00001000 ;/* assumes SH, active=low */ s->dc_panel_control =0x00010000 ;
@@ -1421,8 +1425,8 @@ void sm501_init(MemoryRegion *address_space_mem, uint32_t base,memory_region_add_subregion (address_space_mem, base, &s->local_mem_region);/* map mmio */ -memory_region_init_io (sm501_system_config,NULL , &sm501_system_config_ops, s, -"sm501-system-config" ,0x6c ); +memory_region_init_io (sm501_system_config,NULL , &sm501_system_config_ops, + s,"sm501-system-config" ,0x6c );memory_region_add_subregion (address_space_mem, base +MMIO_BASE_OFFSET , sm501_system_config); memory_region_init_io(sm501_disp_ctrl, NULL, &sm501_disp_ctrl_ops, s,
hw/display/sm501_template.h +26 -26
90 lines not on this page · raw message
Signature · 1 line
2.7.4
PATCH v2 2/14 sm501: Use defines instead of constants where available
In reply to BALATON ZoltanThis also fixes the initial value of misc_control register to match the comment which is likely what was intended but the DAC_POWER bit was set instead. This value is unused so it does not really matter but is fixed here for consistency.
--- hw/display/sm501.c | 8 ++++---- hw/display/sm501_template.h | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-)
hw/display/sm501.c +4 -4
@@ -555,7 +555,7 @@ static uint32_t get_local_mem_size_index(uint32_t size)static inline int is_hwc_enabled (SM501State *state,int crt) {uint32_t addr = crt ? state->dc_crt_hwc_addr : state->dc_panel_hwc_addr; -return addr &0x80000000 ; +return addr &SM501_HWC_EN ; }/**
@@ -1411,9 +1411,9 @@ void sm501_init(MemoryRegion *address_space_mem, uint32_t base, s->local_mem_size_index =get_local_mem_size_index (local_mem_bytes);SM501_DPRINTF ("local mem size=%x. index=%d\n" ,get_local_mem_size (s), s->local_mem_size_index); - s->system_control =0x00100000 ; - s->misc_control =0x00001000 ;/* assumes SH, active=low */ - s->dc_panel_control =0x00010000 ; + s->system_control =0x00100000 ;/* 2D engine FIFO empty */ + s->misc_control =SM501_MISC_IRQ_INVERT ;/* assumes SH, active=low */ + s->dc_panel_control =0x00010000 ;/* FIFO level 3 */ s->dc_crt_control =0x00010000 ;/* allocate local memory */
hw/display/sm501_template.h +1 -1
@@ -108,7 +108,7 @@ static void glue(draw_hwc_line_, PIXEL_NAME)(SM501State *s, int crt,/* get hardware cursor pattern */ uint32_t cursor_addr =get_hwc_address (s, crt);assert (0 <= c_y && c_y <SM501_HWC_HEIGHT ); - cursor_addr +=64 * c_y /4 ;/* 4 pixels per byte */ + cursor_addr +=SM501_HWC_WIDTH * c_y /4 ;/* 4 pixels per byte */ cursor_addr += s->base;/* get cursor position */
Signature · 1 line
2.7.4
Re: [Qemu-devel] [PATCH v2 02/14] sm501: Use defines instead of constants where available
Quoted 25 lines · BALATON Zoltan
This also fixes the initial value of
misc_controlregister to match the comment which is likely what was intended but theDAC_POWERbit was set instead. This value is unused so it does not really matter but is fixed here for consistency.Signed-off-by BALATON Zoltan <[email protected]>--- hw/display/sm501.c | 8 ++++---- hw/display/sm501_template.h | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-)hw/display/sm501.c +1
-1@@ -555,7 +555,7 @@ static uint32_t get_local_mem_size_index(uint32_t size)static inline int is_hwc_enabled (SM501State *state,int crt) {uint32_t addr = crt ? state->dc_crt_hwc_addr : state->dc_panel_hwc_addr; -return addr &0x80000000 ; +return addr &SM501_HWC_EN ; }/**
hw/display/sm501.c void sm501_init(MemoryRegion *address_space_mem, uint32_t base, · line 1417
Quoted 10 lines
hw/display/sm501.c +3
-3@@ -1411,9 +1411,9 @@ void sm501_init(MemoryRegion *address_space_mem, uint32_t base, s->local_mem_size_index =get_local_mem_size_index (local_mem_bytes);SM501_DPRINTF ("local mem size=%x. index=%d\n" ,get_local_mem_size (s), s->local_mem_size_index); - s->system_control =0x00100000 ; - s->misc_control =0x00001000 ;/* assumes SH, active=low */ - s->dc_panel_control =0x00010000 ; + s->system_control =0x00100000 ;/* 2D engine FIFO empty */ + s->misc_control =SM501_MISC_IRQ_INVERT ;/* assumes SH, active=low */ + s->dc_panel_control =0x00010000 ;/* FIFO level 3 */ s->dc_crt_control =0x00010000 ;
I just found a datasheet which says that the power-on-default for the misc-control register is 0b0000.0000.0000.00x0.0001.0000.xxx0.0xxx which means that the 0x1000 decimal value is correct and setting the IRQ_INVERT bit is wrong.
I think the "active=low" bit is referencing the fact that the DAC_POWER bit is 1 for "disable" and 0 for "enable".
It makes sense hardware-wise for the DAC to be powered off by default at reset as well I think.
thanks
-- PMM
Re: [Qemu-devel] [PATCH v2 02/14] sm501: Use defines instead of constants where available
Quoted 8 lines · Peter Maydell
On 7 November 2016 at 09:03, BALATON Zoltan <[email protected]> wrote:
I just found a datasheet which says that the power-on-default for the misc-control register is 0b0000.0000.0000.00x0.0001.0000.xxx0.0xxx which means that the0x1000decimal value is correct and setting theIRQ_INVERTbit is wrong.I think the "active=low" bit is referencing the fact that the
DAC_POWERbit is 1 for "disable" and 0 for "enable".
Actually it's referring to bit 17 (which is an "X" in the reset value above), which is documented as being set by a GPIO pin on reset, and having different meanings for SH and SA1110. In this case we're saying 'reset bit 17 to 0, meaning "SH series CPU, Active Low"'.
Note also that the low bits indicate the bus type which is 000 for SH and 001 for PCI, etc.
So a better comment here would be:
/* Bits 17 (SH), 7 (CDR), 6:5 (Test), 2:0 (Bus) are all supposed
* to be determined at reset by GPIO lines which set config bits.
* We hardwire them:
* SH = 0 : Hitachi Ready Polarity == Active Low
* CDR = 0 : do not reset clock divider
* TEST = 0 : Normal mode (not testing the silicon)
* BUS = 0 : Hitachi SH3/SH4
*/
s->misc_control = SM501_MISC_DAC_POWER;
...and you'll want to put something in to get the right Bus value for the PCI version.
thanks
-- PMM
Re: [Qemu-devel] [PATCH v2 02/14] sm501: Use defines instead of constants where available
Quoted 29 lines · Peter Maydell
Quoted 8 lines · Peter Maydell
On 7 November 2016 at 09:03, BALATON Zoltan <[email protected]> wrote:
I just found a datasheet which says that the power-on-default for the misc-control register is 0b0000.0000.0000.00x0.0001.0000.xxx0.0xxx which means that the0x1000decimal value is correct and setting theIRQ_INVERTbit is wrong.I think the "active=low" bit is referencing the fact that the
DAC_POWERbit is 1 for "disable" and 0 for "enable".Actually it's referring to bit 17 (which is an "X" in the reset value above), which is documented as being set by a GPIO pin on reset, and having different meanings for SH and SA1110. In this case we're saying 'reset bit 17 to 0, meaning "SH series CPU, Active Low"'.
Note also that the low bits indicate the bus type which is 000 for SH and 001 for PCI, etc.
So a better comment here would be:
/* Bits 17 (SH), 7 (CDR), 6:5 (Test), 2:0 (Bus) are all supposed
* to be determined at reset by GPIO lines which set config bits.
* We hardwire them:
* SH = 0 : Hitachi Ready Polarity == Active Low
* CDR = 0 : do not reset clock divider
* TEST = 0 : Normal mode (not testing the silicon)
* BUS = 0 : Hitachi SH3/SH4
*/
s->misc_control=SM501_MISC_DAC_POWER;
Quoted 2 lines
...and you'll want to put something in to get the right Bus value for the PCI version.
Thanks for investigating this, I'll update it in v3.
PATCH v2 3/14 sm501: QOMify
In reply to BALATON Zoltan---
v2: Add memory regions to device state instead of allocating them
hw/display/sm501.c | 139 +++++++++++++++++++++++++++++++++++---------------- hw/sh4/r2d.c | 11 +++- include/hw/devices.h | 5 -- 3 files changed, 106 insertions(+), 49 deletions(-)
hw/display/sm501.c +97 -42
@@ -58,8 +58,8 @@#define SM501_DPRINTF (fmt, ...) do {} while (0)#endif -#define MMIO_BASE_OFFSET 0x3e00000 +#define MMIO_SIZE 0x200000/* SM501 register definitions taken from "linux/include/linux/sm501-regs.h" */
@@ -464,6 +464,10 @@ typedef struct SM501State {uint32_t local_mem_size_index;uint8_t *local_mem;MemoryRegion local_mem_region; +MemoryRegion mmio_region; +MemoryRegion system_config_region; +MemoryRegion disp_ctrl_region; +MemoryRegion twoD_engine_region;uint32_t last_width;uint32_t last_height;
@@ -1396,67 +1400,118 @@ static const GraphicHwOps sm501_ops = { .gfx_update = sm501_update_display, }; -void sm501_init (MemoryRegion *address_space_mem,uint32_t base, -uint32_t local_mem_bytes,qemu_irq irq,Chardev *chr) +static void sm501_init (SM501State *s,DeviceState *dev,uint32_t base, +uint32_t local_mem_bytes) { -SM501State *s; -DeviceState *dev; -MemoryRegion *sm501_system_config =g_new (MemoryRegion,1 ); -MemoryRegion *sm501_disp_ctrl =g_new (MemoryRegion,1 ); -MemoryRegion *sm501_2d_engine =g_new (MemoryRegion,1 ); - -/* allocate management data region */ - s =g_new0 (SM501State,1 ); s->base = base; s->local_mem_size_index =get_local_mem_size_index (local_mem_bytes); -SM501_DPRINTF ("local mem size=%x. index=%d\n" ,get_local_mem_size (s), +SM501_DPRINTF ("sm501 local mem size=%x. index=%d\n" ,get_local_mem_size (s), s->local_mem_size_index); s->system_control =0x00100000 ;/* 2D engine FIFO empty */ s->misc_control =SM501_MISC_IRQ_INVERT ;/* assumes SH, active=low */ s->dc_panel_control =0x00010000 ;/* FIFO level 3 */ s->dc_crt_control =0x00010000 ; -/* allocate local memory */ -memory_region_init_ram (&s->local_mem_region,NULL ,"sm501.local" , +/* local memory */ +memory_region_init_ram (&s->local_mem_region,OBJECT (dev),"sm501.local" , local_mem_bytes, &error_fatal);vmstate_register_ram_global (&s->local_mem_region);memory_region_set_log (&s->local_mem_region, true,DIRTY_MEMORY_VGA ); s->local_mem =memory_region_get_ram_ptr (&s->local_mem_region); -memory_region_add_subregion (address_space_mem, base, &s->local_mem_region); - -/* map mmio */ -memory_region_init_io (sm501_system_config,NULL , &sm501_system_config_ops, - s,"sm501-system-config" ,0x6c ); -memory_region_add_subregion (address_space_mem, base +MMIO_BASE_OFFSET , - sm501_system_config); -memory_region_init_io (sm501_disp_ctrl,NULL , &sm501_disp_ctrl_ops, s, + +/* mmio */ +memory_region_init (&s->mmio_region,OBJECT (dev),"sm501.mmio" ,MMIO_SIZE ); +memory_region_init_io (&s->system_config_region,OBJECT (dev), + &sm501_system_config_ops, s, +"sm501-system-config" ,0x6c ); +memory_region_add_subregion (&s->mmio_region,SM501_SYS_CONFIG , + &s->system_config_region); +memory_region_init_io (&s->disp_ctrl_region,OBJECT (dev), + &sm501_disp_ctrl_ops, s,"sm501-disp-ctrl" ,0x1000 ); -memory_region_add_subregion (address_space_mem, - base +MMIO_BASE_OFFSET +SM501_DC , - sm501_disp_ctrl); -memory_region_init_io (sm501_2d_engine,NULL , &sm501_2d_engine_ops, s, +memory_region_add_subregion (&s->mmio_region,SM501_DC , + &s->disp_ctrl_region); +memory_region_init_io (&s->twoD_engine_region,OBJECT (dev), + &sm501_2d_engine_ops, s,"sm501-2d-engine" ,0x54 ); -memory_region_add_subregion (address_space_mem, - base +MMIO_BASE_OFFSET +SM501_2D_ENGINE , - sm501_2d_engine); +memory_region_add_subregion (&s->mmio_region,SM501_2D_ENGINE , + &s->twoD_engine_region); + +/* create qemu graphic console */ + s->con =graphic_console_init (DEVICE (dev),0 , &sm501_ops, s); +} + +#define TYPE_SYSBUS_SM501 "sysbus-sm501" +#define SYSBUS_SM501 (obj) \ + OBJECT_CHECK(SM501SysBusState, (obj), TYPE_SYSBUS_SM501) + +typedef struct { +/*< private >*/ +SysBusDevice parent_obj; +/*< public >*/ +SM501State state; +uint32_t vram_size; +uint32_t base; +void *chr_state; +}SM501SysBusState ; + +static void sm501_realize_sysbus (DeviceState *dev,Error **errp) +{ +SM501SysBusState *s =SYSBUS_SM501 (dev); +SysBusDevice *sbd =SYS_BUS_DEVICE (dev); +DeviceState *usb_dev; + +sm501_init (&s->state, dev, s->base, s->vram_size); +sysbus_init_mmio (sbd, &s->state.local_mem_region); +sysbus_init_mmio (sbd, &s->state.mmio_region);/* bridge to usb host emulation module */ - dev =qdev_create (NULL ,"sysbus-ohci" ); -qdev_prop_set_uint32 (dev,"num-ports" ,2 ); -qdev_prop_set_uint64 (dev,"dma-offset" , base); -qdev_init_nofail (dev); -sysbus_mmio_map (SYS_BUS_DEVICE (dev),0 , - base +MMIO_BASE_OFFSET +SM501_USB_HOST ); -sysbus_connect_irq (SYS_BUS_DEVICE (dev),0 , irq); + usb_dev =qdev_create (NULL ,"sysbus-ohci" ); +qdev_prop_set_uint32 (usb_dev,"num-ports" ,2 ); +qdev_prop_set_uint64 (usb_dev,"dma-offset" , s->base); +qdev_init_nofail (usb_dev); +memory_region_add_subregion (&s->state.mmio_region,SM501_USB_HOST , +sysbus_mmio_get_region (SYS_BUS_DEVICE (usb_dev),0 )); +sysbus_pass_irq (sbd,SYS_BUS_DEVICE (usb_dev));/* bridge to serial emulation module */ -if (chr) { -serial_mm_init (address_space_mem, - base +MMIO_BASE_OFFSET +SM501_UART0 ,2 , +if (s->chr_state) { +serial_mm_init (&s->state.mmio_region,SM501_UART0 ,2 ,NULL ,/* TODO : chain irq to IRL */ -115200 , chr,DEVICE_NATIVE_ENDIAN ); +115200 , s->chr_state,DEVICE_NATIVE_ENDIAN ); } +} -/* create qemu graphic console */ - s->con =graphic_console_init (DEVICE (dev),0 , &sm501_ops, s); +static Property sm501_sysbus_properties[] = { +DEFINE_PROP_UINT32 ("vram-size" , SM501SysBusState, vram_size,0 ), +DEFINE_PROP_UINT32 ("base" , SM501SysBusState, base,0 ), +DEFINE_PROP_PTR ("chr-state" , SM501SysBusState, chr_state), +DEFINE_PROP_END_OF_LIST (), +}; + +static void sm501_sysbus_class_init (ObjectClass *klass,void *data) +{ +DeviceClass *dc =DEVICE_CLASS (klass); + + dc->realize = sm501_realize_sysbus; +set_bit (DEVICE_CATEGORY_DISPLAY , dc->categories); + dc->desc ="SM501 Multimedia Companion" ; + dc->props = sm501_sysbus_properties; +/* Note: pointer property "chr-state" may remain null, thus +* no need for dc->cannot_instantiate_with_device_add_yet = true; +*/ } + +static const TypeInfo sm501_sysbus_info = { + .name =TYPE_SYSBUS_SM501 , + .parent =TYPE_SYS_BUS_DEVICE , + .instance_size =sizeof (SM501SysBusState), + .class_init = sm501_sysbus_class_init, +}; + +static void sm501_register_types (void ) +{ +type_register_static (&sm501_sysbus_info); +} + +type_init (sm501_register_types)
hw/sh4/r2d.c +9 -2
@@ -277,8 +277,15 @@ static void r2d_init(MachineState *machine)sysbus_connect_irq (busdev,2 , irq[PCI_INTC ]);sysbus_connect_irq (busdev,3 , irq[PCI_INTD ]); -sm501_init (address_space_mem,0x10000000 ,SM501_VRAM_SIZE , - irq[SM501 ], serial_hds[2 ]); + dev =qdev_create (NULL ,"sysbus-sm501" ); + busdev =SYS_BUS_DEVICE (dev); +qdev_prop_set_uint32 (dev,"vram-size" ,SM501_VRAM_SIZE ); +qdev_prop_set_uint32 (dev,"base" ,0x10000000 ); +qdev_prop_set_ptr (dev,"chr-state" , serial_hds[2 ]); +qdev_init_nofail (dev); +sysbus_mmio_map (busdev,0 ,0x10000000 ); +sysbus_mmio_map (busdev,1 ,0x13e00000 ); +sysbus_connect_irq (busdev,0 , irq[SM501 ]);/* onboard CF (True IDE mode, Master only). */ dinfo =drive_get (IF_IDE ,0 ,0 );
include/hw/devices.h +0 -5
@@ -62,9 +62,4 @@ void tc6393xb_gpio_out_set(TC6393xbState *s, int line,qemu_irq *tc6393xb_gpio_in_get (TC6393xbState *s);qemu_irq tc6393xb_l3v_get (TC6393xbState *s); -/* sm501.c */ -void sm501_init (struct MemoryRegion *address_space_mem,uint32_t base, -uint32_t local_mem_bytes,qemu_irq irq, -Chardev *chr); -#endif
Signature · 1 line
2.7.4
Re: [Qemu-devel] [PATCH v2 03/14] sm501: QOMify
Quoted 1 line · BALATON Zoltan
Signed-off-by BALATON Zoltan <[email protected]>
It would be helpful to mention in the commit message that we don't yet implement reset or vmstate and that those are coming in a later commit.
(There is almost always something useful to say in a commit message body; subject-line-only commit messages should be the rare exception in my opinion.)
hw/display/sm501.c static const GraphicHwOps sm501_ops = { · line 1502
Quoted 12 lines
hw/display/sm501.c +12
-0+ +static void sm501_sysbus_class_init (ObjectClass *klass,void *data) +{ +DeviceClass *dc =DEVICE_CLASS (klass); + + dc->realize = sm501_realize_sysbus; +set_bit (DEVICE_CATEGORY_DISPLAY , dc->categories); + dc->desc ="SM501 Multimedia Companion" ; + dc->props = sm501_sysbus_properties; +/* Note: pointer property "chr-state" may remain null, thus +* no need for dc->cannot_instantiate_with_device_add_yet = true; +*/
This comment should be indented.
Otherwise
thanks
-- PMM
PATCH v2 4/14 sm501: Get rid of base address in draw_hwc_line
In reply to BALATON ZoltanDo not use the base address to access data in local memory. This is in preparation to allow chip connected via PCI where base address depends on where the BAR is mapped so it will be unknown.
--- hw/display/sm501.c | 6 ++---- hw/display/sm501_template.h | 8 ++++---- 2 files changed, 6 insertions(+), 8 deletions(-)
hw/display/sm501.c +2 -4
@@ -460,7 +460,6 @@ typedef struct SM501State {QemuConsole *con;/* status & internal resources */ -hwaddr base;uint32_t local_mem_size_index;uint8_t *local_mem;MemoryRegion local_mem_region;
@@ -1400,10 +1399,9 @@ static const GraphicHwOps sm501_ops = { .gfx_update = sm501_update_display, }; -static void sm501_init (SM501State *s,DeviceState *dev,uint32_t base, +static void sm501_init (SM501State *s,DeviceState *dev,uint32_t local_mem_bytes) { - s->base = base; s->local_mem_size_index =get_local_mem_size_index (local_mem_bytes);SM501_DPRINTF ("sm501 local mem size=%x. index=%d\n" ,get_local_mem_size (s), s->local_mem_size_index);
@@ -1461,7 +1459,7 @@ static void sm501_realize_sysbus(DeviceState *dev, Error **errp)SysBusDevice *sbd =SYS_BUS_DEVICE (dev);DeviceState *usb_dev; -sm501_init (&s->state, dev, s->base, s->vram_size); +sm501_init (&s->state, dev, s->vram_size);sysbus_init_mmio (sbd, &s->state.local_mem_region);sysbus_init_mmio (sbd, &s->state.mmio_region);
hw/display/sm501_template.h +4 -4
@@ -103,13 +103,13 @@ static void glue(draw_hwc_line_, PIXEL_NAME)(SM501State *s, int crt, uint8_t *palette, int c_y, uint8_t *d, int width) { int x, i; -uint8_t bitset =0 ; +uint8_t *pixval, bitset =0 ;/* get hardware cursor pattern */ uint32_t cursor_addr =get_hwc_address (s, crt);assert (0 <= c_y && c_y <SM501_HWC_HEIGHT ); cursor_addr +=SM501_HWC_WIDTH * c_y /4 ;/* 4 pixels per byte */ - cursor_addr += s->base; + pixval = s->local_mem + cursor_addr;/* get cursor position */ x =get_hwc_x (s, crt);
@@ -120,8 +120,8 @@ static void glue(draw_hwc_line_, PIXEL_NAME)(SM501State *s, int crt,/* get pixel value */ if (i %4 ==0 ) { - bitset =ldub_phys (&address_space_memory, cursor_addr); - cursor_addr++; + bitset =ldub_p (pixval); + pixval++; } v = bitset &3 ; bitset >>=2 ;
Signature · 1 line
2.7.4
Re: [Qemu-devel] [PATCH v2 04/14] sm501: Get rid of base address in draw_hwc_line
Quoted 75 lines · BALATON Zoltan
Do not use the base address to access data in local memory. This is in preparation to allow chip connected via PCI where base address depends on where the BAR is mapped so it will be unknown.
Signed-off-by BALATON Zoltan <[email protected]>--- hw/display/sm501.c | 6 ++---- hw/display/sm501_template.h | 8 ++++---- 2 files changed, 6 insertions(+), 8 deletions(-)hw/display/sm501.c +2
-4@@ -460,7 +460,6 @@ typedef struct SM501State {QemuConsole *con;/* status & internal resources */ -hwaddr base;uint32_t local_mem_size_index;uint8_t *local_mem;MemoryRegion local_mem_region;@@ -1400,10 +1399,9 @@ static const GraphicHwOps sm501_ops = { .gfx_update = sm501_update_display, }; -static void sm501_init (SM501State *s,DeviceState *dev,uint32_t base, +static void sm501_init (SM501State *s,DeviceState *dev,uint32_t local_mem_bytes) { - s->base = base; s->local_mem_size_index =get_local_mem_size_index (local_mem_bytes);SM501_DPRINTF ("sm501 local mem size=%x. index=%d\n" ,get_local_mem_size (s), s->local_mem_size_index);@@ -1461,7 +1459,7 @@ static void sm501_realize_sysbus(DeviceState *dev, Error **errp)SysBusDevice *sbd =SYS_BUS_DEVICE (dev);DeviceState *usb_dev; -sm501_init (&s->state, dev, s->base, s->vram_size); +sm501_init (&s->state, dev, s->vram_size);sysbus_init_mmio (sbd, &s->state.local_mem_region);sysbus_init_mmio (sbd, &s->state.mmio_region);hw/display/sm501_template.h +4
-5@@ -103,13 +103,13 @@ static void glue(draw_hwc_line_, PIXEL_NAME)(SM501State *s, int crt, uint8_t *palette, int c_y, uint8_t *d, int width) { int x, i; -uint8_t bitset =0 ; +uint8_t *pixval, bitset =0 ;/* get hardware cursor pattern */ uint32_t cursor_addr =get_hwc_address (s, crt);assert (0 <= c_y && c_y <SM501_HWC_HEIGHT ); cursor_addr +=SM501_HWC_WIDTH * c_y /4 ;/* 4 pixels per byte */ - cursor_addr += s->base; + pixval = s->local_mem + cursor_addr;/* get cursor position */ x =get_hwc_x (s, crt);@@ -120,8 +120,8 @@ static void glue(draw_hwc_line_, PIXEL_NAME)(SM501State *s, int crt,/* get pixel value */ if (i %4 ==0 ) { - bitset =ldub_phys (&address_space_memory, cursor_addr); - cursor_addr++; + bitset =ldub_p (pixval); + pixval++; } v = bitset &3 ; bitset >>=2 ; --
Quoted 1 line
2.7.4
(The data sheet suggests that the hardware cursor can also be in "external memory", but it looks like we never supported that previously and I can't figure out how it is supposed to work either. So no problem with restricting it to be in the local video ram blob.)
thanks
-- PMM
Re: [Qemu-devel] [PATCH v2 04/14] sm501: Get rid of base address in draw_hwc_line
Quoted 66 lines · Peter Maydell
Quoted 65 lines · BALATON Zoltan
Do not use the base address to access data in local memory. This is in preparation to allow chip connected via PCI where base address depends on where the BAR is mapped so it will be unknown.
Signed-off-by BALATON Zoltan <[email protected]>--- hw/display/sm501.c | 6 ++---- hw/display/sm501_template.h | 8 ++++---- 2 files changed, 6 insertions(+), 8 deletions(-)hw/display/sm501.c +2
-4@@ -460,7 +460,6 @@ typedef struct SM501State {QemuConsole *con;/* status & internal resources */ -hwaddr base;uint32_t local_mem_size_index;uint8_t *local_mem;MemoryRegion local_mem_region;@@ -1400,10 +1399,9 @@ static const GraphicHwOps sm501_ops = { .gfx_update = sm501_update_display, }; -static void sm501_init (SM501State *s,DeviceState *dev,uint32_t base, +static void sm501_init (SM501State *s,DeviceState *dev,uint32_t local_mem_bytes) { - s->base = base; s->local_mem_size_index =get_local_mem_size_index (local_mem_bytes);SM501_DPRINTF ("sm501 local mem size=%x. index=%d\n" ,get_local_mem_size (s), s->local_mem_size_index);@@ -1461,7 +1459,7 @@ static void sm501_realize_sysbus(DeviceState *dev, Error **errp)SysBusDevice *sbd =SYS_BUS_DEVICE (dev);DeviceState *usb_dev; -sm501_init (&s->state, dev, s->base, s->vram_size); +sm501_init (&s->state, dev, s->vram_size);sysbus_init_mmio (sbd, &s->state.local_mem_region);sysbus_init_mmio (sbd, &s->state.mmio_region);hw/display/sm501_template.h +2
-2@@ -103,13 +103,13 @@ static void glue(draw_hwc_line_, PIXEL_NAME)(SM501State *s, int crt, uint8_t *palette, int c_y, uint8_t *d, int width) { int x, i; -uint8_t bitset =0 ; +uint8_t *pixval, bitset =0 ;/* get hardware cursor pattern */ uint32_t cursor_addr =get_hwc_address (s, crt);assert (0 <= c_y && c_y <SM501_HWC_HEIGHT ); cursor_addr +=SM501_HWC_WIDTH * c_y /4 ;/* 4 pixels per byte */ - cursor_addr += s->base; + pixval = s->local_mem + cursor_addr;/* get cursor position */ x =get_hwc_x (s, crt);@@ -120,8 +120,8 @@ static void glue(draw_hwc_line_, PIXEL_NAME)(SM501State *s, int crt,
Quoted 15 lines
Quoted 11 lines
hw/display/sm501_template.h +2
-3/* get pixel value */ if (i %4 ==0 ) { - bitset =ldub_phys (&address_space_memory, cursor_addr); - cursor_addr++; + bitset =ldub_p (pixval); + pixval++; } v = bitset &3 ; bitset >>=2 ; --2.7.4
(The data sheet suggests that the hardware cursor can also be in "external memory", but it looks like we never supported that previously and I can't figure out how it is supposed to work either.
Yes and as you say this was not implemented before and I haven't implemented it because no known client seems to use this. If found to be needed later, the Fix hardware cursor patch in this series cleans this up so the cursor data address is passed from the caller where it could possibly pass the external address that can be handled in get_hwc_address which is also changed to return the address instead of offset so dependency on local_memory is removed.
Quoted 8 lines
So no problem with restricting it to be in the local video ram blob.)
Reviewed-by Peter Maydell <[email protected]>thanks
-- PMM
PATCH v2 5/14 sm501: Add emulation of chip connected via PCI
In reply to BALATON ZoltanOnly the display controller part is created automatically on PCI
---
v2: Split off removing dependency on base address to separate patch
hw/display/sm501.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 52 insertions(+)
hw/display/sm501.c +52 -0
@@ -31,6 +31,7 @@#include "ui/console.h" #include "hw/devices.h" #include "hw/sysbus.h" +#include "hw/pci/pci.h" #include "qemu/range.h" #include "ui/pixel_ops.h" #include "exec/address-spaces.h"
@@ -1507,9 +1508,60 @@ static const TypeInfo sm501_sysbus_info = { .class_init = sm501_sysbus_class_init, }; +#define TYPE_PCI_SM501 "sm501" +#define PCI_SM501 (obj) OBJECT_CHECK(SM501PCIState, (obj), TYPE_PCI_SM501) + +typedef struct { +/*< private >*/ +PCIDevice parent_obj; +/*< public >*/ +SM501State state; +uint32_t vram_size; +}SM501PCIState ; + +static void sm501_realize_pci (PCIDevice *dev,Error **errp) +{ +SM501PCIState *s =PCI_SM501 (dev); + +sm501_init (&s->state,DEVICE (dev), s->vram_size); +pci_register_bar (dev,0 ,PCI_BASE_ADDRESS_SPACE_MEMORY , + &s->state.local_mem_region); +pci_register_bar (dev,1 ,PCI_BASE_ADDRESS_SPACE_MEMORY , + &s->state.mmio_region); +} + +static Property sm501_pci_properties[] = { +DEFINE_PROP_UINT32 ("vram-size" , SM501PCIState, vram_size, +64 *1024 *1024 ), +DEFINE_PROP_END_OF_LIST (), +}; + +static void sm501_pci_class_init (ObjectClass *klass,void *data) +{ +DeviceClass *dc =DEVICE_CLASS (klass); +PCIDeviceClass *k =PCI_DEVICE_CLASS (klass); + + k->realize = sm501_realize_pci; + k->vendor_id =0x126f ; + k->device_id =0x0501 ; + k->class_id =PCI_CLASS_DISPLAY_OTHER ; +set_bit (DEVICE_CATEGORY_DISPLAY , dc->categories); + dc->desc ="SM501 Display Controller" ; + dc->props = sm501_pci_properties; + dc->hotpluggable = false; +} + +static const TypeInfo sm501_pci_info = { + .name =TYPE_PCI_SM501 , + .parent =TYPE_PCI_DEVICE , + .instance_size =sizeof (SM501PCIState), + .class_init = sm501_pci_class_init, +}; +static void sm501_register_types (void ) {type_register_static (&sm501_sysbus_info); +type_register_static (&sm501_pci_info); }type_init (sm501_register_types)
Signature · 1 line
2.7.4
Re: [Qemu-devel] [PATCH v2 05/14] sm501: Add emulation of chip connected via PCI
Quoted 41 lines · BALATON Zoltan
Only the display controller part is created automatically on PCI
Signed-off-by BALATON Zoltan <[email protected]>---
v2: Split off removing dependency on base address to separate patch
hw/display/sm501.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 52 insertions(+)hw/display/sm501.c +16
-0@@ -31,6 +31,7 @@#include "ui/console.h" #include "hw/devices.h" #include "hw/sysbus.h" +#include "hw/pci/pci.h" #include "qemu/range.h" #include "ui/pixel_ops.h" #include "exec/address-spaces.h" @@ -1507,9 +1508,60 @@ static const TypeInfo sm501_sysbus_info = { .class_init = sm501_sysbus_class_init, }; +#define TYPE_PCI_SM501 "sm501" +#define PCI_SM501 (obj) OBJECT_CHECK(SM501PCIState, (obj), TYPE_PCI_SM501) + +typedef struct { +/*< private >*/ +PCIDevice parent_obj; +/*< public >*/ +SM501State state; +uint32_t vram_size; +}SM501PCIState ; + +static void sm501_realize_pci (PCIDevice *dev,Error **errp) +{ +SM501PCIState *s =PCI_SM501 (dev); +
hw/display/sm501.c static const TypeInfo sm501_sysbus_info = { · line 1535
Quoted 10 lines
hw/display/sm501.c +10
-0+sm501_init (&s->state,DEVICE (dev), s->vram_size); +pci_register_bar (dev,0 ,PCI_BASE_ADDRESS_SPACE_MEMORY , + &s->state.local_mem_region); +pci_register_bar (dev,1 ,PCI_BASE_ADDRESS_SPACE_MEMORY , + &s->state.mmio_region); +} + +static Property sm501_pci_properties[] = { +DEFINE_PROP_UINT32 ("vram-size" , SM501PCIState, vram_size, +64 *1024 *1024 ),
Something needs to sanity check this value, because it's user settable in the PCI device. (In the sysbus version you can only create it from board code, and we can assume board code doesn't pass us silly values.)
Or you could just hardcode the PCI device to always be 64MB, I guess :-)
hw/display/sm501.c static const TypeInfo sm501_sysbus_info = { · line 1546
Quoted 11 lines
hw/display/sm501.c +11
-0+ DEFINE_PROP_END_OF_LIST(), +}; + +static void sm501_pci_class_init (ObjectClass *klass,void *data) +{ +DeviceClass *dc =DEVICE_CLASS (klass); +PCIDeviceClass *k =PCI_DEVICE_CLASS (klass); + + k->realize = sm501_realize_pci; + k->vendor_id =0x126f ; + k->device_id =0x0501 ;
We could define some constants in include/hw/pci/pci_ids.h for these, I suppose.
hw/display/sm501.c static const TypeInfo sm501_sysbus_info = { · line 1551
Quoted 5 lines
hw/display/sm501.c +5
-0+ k->class_id =PCI_CLASS_DISPLAY_OTHER ; +set_bit (DEVICE_CATEGORY_DISPLAY , dc->categories); + dc->desc ="SM501 Display Controller" ; + dc->props = sm501_pci_properties; + dc->hotpluggable = false;
This needs a reset and vmsd as well.
Quoted 17 lines
hw/display/sm501.c +10
-1+} + +static const TypeInfo sm501_pci_info = { + .name =TYPE_PCI_SM501 , + .parent =TYPE_PCI_DEVICE , + .instance_size =sizeof (SM501PCIState), + .class_init = sm501_pci_class_init, +}; +static void sm501_register_types (void ) {type_register_static (&sm501_sysbus_info); +type_register_static (&sm501_pci_info); }type_init (sm501_register_types) --
Quoted 1 line
2.7.4
thanks
-- PMM
Re: [Qemu-devel] [PATCH v2 05/14] sm501: Add emulation of chip connected via PCI
Quoted 58 lines · Peter Maydell
Quoted 41 lines · BALATON Zoltan
Only the display controller part is created automatically on PCI
Signed-off-by BALATON Zoltan <[email protected]>---
v2: Split off removing dependency on base address to separate patch
hw/display/sm501.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 52 insertions(+)hw/display/sm501.c +16
-0@@ -31,6 +31,7 @@#include "ui/console.h" #include "hw/devices.h" #include "hw/sysbus.h" +#include "hw/pci/pci.h" #include "qemu/range.h" #include "ui/pixel_ops.h" #include "exec/address-spaces.h" @@ -1507,9 +1508,60 @@ static const TypeInfo sm501_sysbus_info = { .class_init = sm501_sysbus_class_init, }; +#define TYPE_PCI_SM501 "sm501" +#define PCI_SM501 (obj) OBJECT_CHECK(SM501PCIState, (obj), TYPE_PCI_SM501) + +typedef struct { +/*< private >*/ +PCIDevice parent_obj; +/*< public >*/ +SM501State state; +uint32_t vram_size; +}SM501PCIState ; + +static void sm501_realize_pci (PCIDevice *dev,Error **errp) +{ +SM501PCIState *s =PCI_SM501 (dev); +hw/display/sm501.c static const TypeInfo sm501_sysbus_info = { · line 1535
Quoted 10 lines
hw/display/sm501.c +10
-0+sm501_init (&s->state,DEVICE (dev), s->vram_size); +pci_register_bar (dev,0 ,PCI_BASE_ADDRESS_SPACE_MEMORY , + &s->state.local_mem_region); +pci_register_bar (dev,1 ,PCI_BASE_ADDRESS_SPACE_MEMORY , + &s->state.mmio_region); +} + +static Property sm501_pci_properties[] = { +DEFINE_PROP_UINT32 ("vram-size" , SM501PCIState, vram_size, +64 *1024 *1024 ),Something needs to sanity check this value, because it's user settable in the PCI device. (In the sysbus version you can only create it from board code, and we can assume board code doesn't pass us silly values.)
Quoted 2 lines
Or you could just hardcode the PCI device to always be 64MB, I guess :-)
Maybe a check could be added to get_local_mem_size_index(). What should it do for invalid values?
Quoted 6 lines
Quoted 6 lines
hw/display/sm501.c +6
-0+ DEFINE_PROP_END_OF_LIST(), +}; + +static void sm501_pci_class_init (ObjectClass *klass,void *data) +{ +DeviceClass *dc =DEVICE_CLASS (klass);
Quoted 8 lines
hw/display/sm501.c static const TypeInfo sm501_sysbus_info = { · line 1546
Quoted 5 lines
hw/display/sm501.c +5
-0+PCIDeviceClass *k =PCI_DEVICE_CLASS (klass); + + k->realize = sm501_realize_pci; + k->vendor_id =0x126f ; + k->device_id =0x0501 ;We could define some constants in
include/hw/pci/pci_ids.hfor these, I suppose.
Should I add that as well? Should it be in this or separate patch?
Quoted 7 lines
hw/display/sm501.c static const TypeInfo sm501_sysbus_info = { · line 1551
Quoted 5 lines
hw/display/sm501.c +5
-0+ k->class_id =PCI_CLASS_DISPLAY_OTHER ; +set_bit (DEVICE_CATEGORY_DISPLAY , dc->categories); + dc->desc ="SM501 Display Controller" ; + dc->props = sm501_pci_properties; + dc->hotpluggable = false;This needs a reset and vmsd as well.
Why? OK reset makes sense but if this is only used in sh and I think that might have other devices that don't support migration then adding a vmsd here only achieves that if we ever change anything in the state the vmsd needs to be changed, compatibility ensured, etc. And that's for nothing if the sh machine cannot be migrated anyway. There seems to be a lot of boilerplate already, it may exceed the actual code at some point.
But I can add vmstate for sysbus version if you think it's needed, I just don't see the point.
Quoted 23 lines
Quoted 17 lines
hw/display/sm501.c +10
-1+} + +static const TypeInfo sm501_pci_info = { + .name =TYPE_PCI_SM501 , + .parent =TYPE_PCI_DEVICE , + .instance_size =sizeof (SM501PCIState), + .class_init = sm501_pci_class_init, +}; +static void sm501_register_types (void ) {type_register_static (&sm501_sysbus_info); +type_register_static (&sm501_pci_info); }type_init (sm501_register_types) --Quoted 1 line
2.7.4
thanks
-- PMM
Re: [Qemu-devel] [PATCH v2 05/14] sm501: Add emulation of chip connected via PCI
Quoted 64 lines · BALATON Zoltan
Quoted 63 lines · Peter Maydell
Quoted 44 lines · BALATON Zoltan
Only the display controller part is created automatically on PCI
Signed-off-by BALATON Zoltan <[email protected]>---
v2: Split off removing dependency on base address to separate patch
hw/display/sm501.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 52 insertions(+)hw/display/sm501.c +16
-0@@ -31,6 +31,7 @@#include "ui/console.h" #include "hw/devices.h" #include "hw/sysbus.h" +#include "hw/pci/pci.h" #include "qemu/range.h" #include "ui/pixel_ops.h" #include "exec/address-spaces.h" @@ -1507,9 +1508,60 @@ static const TypeInfo sm501_sysbus_info = { .class_init = sm501_sysbus_class_init, }; +#define TYPE_PCI_SM501 "sm501" +#define PCI_SM501 (obj) OBJECT_CHECK(SM501PCIState, (obj), TYPE_PCI_SM501) + +typedef struct { +/*< private >*/ +PCIDevice parent_obj; +/*< public >*/ +SM501State state; +uint32_t vram_size; +}SM501PCIState ; + +static void sm501_realize_pci (PCIDevice *dev,Error **errp) +{ +SM501PCIState *s =PCI_SM501 (dev); +hw/display/sm501.c static const TypeInfo sm501_sysbus_info = { · line 1536
Quoted 10 lines
hw/display/sm501.c +10
-0+sm501_init (&s->state,DEVICE (dev), s->vram_size); +pci_register_bar (dev,0 ,PCI_BASE_ADDRESS_SPACE_MEMORY , + &s->state.local_mem_region); +pci_register_bar (dev,1 ,PCI_BASE_ADDRESS_SPACE_MEMORY , + &s->state.mmio_region); +} + +static Property sm501_pci_properties[] = { +DEFINE_PROP_UINT32 ("vram-size" , SM501PCIState, vram_size, +64 *1024 *1024 ),Something needs to sanity check this value, because it's user settable in the PCI device. (In the sysbus version you can only create it from board code, and we can assume board code doesn't pass us silly values.)
Quoted 6 lines
Quoted 2 lines
Or you could just hardcode the PCI device to always be 64MB, I guess :-)
Maybe a check could be added to
get_local_mem_size_index(). What should it do for invalid values?
The device realize function should fail:
if (user settable property foo is wrong) {
error_setg(errp, "foo must be X, Y or Z");
return;
}Quoted 17 lines
Quoted 7 lines
Quoted 7 lines
hw/display/sm501.c +7
-0+ DEFINE_PROP_END_OF_LIST(), +}; + +static void sm501_pci_class_init (ObjectClass *klass,void *data) +{ +DeviceClass *dc =DEVICE_CLASS (klass); +PCIDeviceClass *k =PCI_DEVICE_CLASS (klass);Quoted 8 lines
hw/display/sm501.c static const TypeInfo sm501_sysbus_info = { · line 1546
Quoted 4 lines
hw/display/sm501.c +4
-0+ + k->realize = sm501_realize_pci; + k->vendor_id =0x126f ; + k->device_id =0x0501 ;We could define some constants in
include/hw/pci/pci_ids.hfor these, I suppose.
Quoted 1 line
Should I add that as well? Should it be in this or separate patch?
In this patch will be fine.
Quoted 11 lines
Quoted 8 lines
hw/display/sm501.c static const TypeInfo sm501_sysbus_info = { · line 1551
Quoted 5 lines
hw/display/sm501.c +5
-0+ k->class_id =PCI_CLASS_DISPLAY_OTHER ; +set_bit (DEVICE_CATEGORY_DISPLAY , dc->categories); + dc->desc ="SM501 Display Controller" ; + dc->props = sm501_pci_properties; + dc->hotpluggable = false;This needs a reset and vmsd as well.
Why?
When I reviewed this patch I hadn't yet got to the later one which adds the vmsd and reset code (and this patch doesn't say in the commit that those will be added later). This is the PCI device bit, incidentally, but I do think that both PCI device and sysbus should have vmsd.
Quoted 9 lines
OK reset makes sense but if this is only used in sh and I think that might have other devices that don't support migration then adding a vmsd here only achieves that if we ever change anything in the state the vmsd needs to be changed, compatibility ensured, etc. And that's for nothing if the sh machine cannot be migrated anyway. There seems to be a lot of boilerplate already, it may exceed the actual code at some point.
But I can add vmstate for sysbus version if you think it's needed, I just don't see the point.
Every device should have vmstate. In this case the bulk of it is going to be shared with the PCI device anyway. For machines which don't yet support migration between versions (which includes anything sh) we have greater flexibility to break migration compat if we need to.
This is one of the many cases where old QEMU code doesn't meet modern standards. Our process for improving that is a gradual ratchet effect -- when we need to mess with a device anyway, we add the support for the new stuff. This gradually reduces the set of missing-migration-support devices and makes life easier for somebody who needs to add it to sh later. (A quick scan suggests that the other r2d devices which don't yet support vmstate are the fpga device and the pci host controller.)
Nobody's ever going to actually do VM migration for most of these embedded boards, but snapshot save/load is a very useful debugging tool. (You can snapshot eg after a long kernel boot and just before triggering the guest bug you're investigating, and then your debug sessions can start with loading the snapshot. Particularly handy if you are going to turn on a lot of debug logging.)
thanks
-- PMM
Re: [Qemu-devel] [PATCH v2 05/14] sm501: Add emulation of chip connected via PCI
Quoted 69 lines · Peter Maydell
Quoted 68 lines · BALATON Zoltan
Quoted 65 lines · Peter Maydell
Quoted 44 lines · BALATON Zoltan
Only the display controller part is created automatically on PCI
Signed-off-by BALATON Zoltan <[email protected]>---
v2: Split off removing dependency on base address to separate patch
hw/display/sm501.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 52 insertions(+)hw/display/sm501.c +16
-0@@ -31,6 +31,7 @@#include "ui/console.h" #include "hw/devices.h" #include "hw/sysbus.h" +#include "hw/pci/pci.h" #include "qemu/range.h" #include "ui/pixel_ops.h" #include "exec/address-spaces.h" @@ -1507,9 +1508,60 @@ static const TypeInfo sm501_sysbus_info = { .class_init = sm501_sysbus_class_init, }; +#define TYPE_PCI_SM501 "sm501" +#define PCI_SM501 (obj) OBJECT_CHECK(SM501PCIState, (obj), TYPE_PCI_SM501) + +typedef struct { +/*< private >*/ +PCIDevice parent_obj; +/*< public >*/ +SM501State state; +uint32_t vram_size; +}SM501PCIState ; + +static void sm501_realize_pci (PCIDevice *dev,Error **errp) +{ +SM501PCIState *s =PCI_SM501 (dev); +hw/display/sm501.c static const TypeInfo sm501_sysbus_info = { · line 1536
Quoted 10 lines
hw/display/sm501.c +10
-0+sm501_init (&s->state,DEVICE (dev), s->vram_size); +pci_register_bar (dev,0 ,PCI_BASE_ADDRESS_SPACE_MEMORY , + &s->state.local_mem_region); +pci_register_bar (dev,1 ,PCI_BASE_ADDRESS_SPACE_MEMORY , + &s->state.mmio_region); +} + +static Property sm501_pci_properties[] = { +DEFINE_PROP_UINT32 ("vram-size" , SM501PCIState, vram_size, +64 *1024 *1024 ),Something needs to sanity check this value, because it's user settable in the PCI device. (In the sysbus version you can only create it from board code, and we can assume board code doesn't pass us silly values.)
Or you could just hardcode the PCI device to always be 64MB, I guess :-)
Quoted 8 lines
Quoted 2 lines
Maybe a check could be added to
get_local_mem_size_index(). What should it do for invalid values?The device realize function should fail:
if (user settable property foo is wrong) { error_setg(errp, "foo must be X, Y or Z"); return; }
Actually I think this cannot fail because get_local_mem_size_index() will pick a valid value that's closest to the one specified. However the mem region was not created using this adjusted value. I've fixed this in the v3 I've just sent and also added a warning when adjusment was made.
Thank you,
BALATON Zoltan
PATCH v2 6/14 sm501: Add missing arbitration control register
In reply to BALATON Zoltan--- hw/display/sm501.c | 7 +++++++ 1 file changed, 7 insertions(+)
hw/display/sm501.c +7 -0
@@ -477,6 +477,7 @@ typedef struct SM501State {uint32_t gpio_31_0_control;uint32_t gpio_63_32_control;uint32_t dram_control; +uint32_t arbitration_control;uint32_t irq_mask;uint32_t misc_timing;uint32_t power_mode_control;
@@ -760,6 +761,9 @@ static uint64_t sm501_system_config_read(void *opaque, hwaddr addr,case SM501_DRAM_CONTROL : ret = (s->dram_control &0x07F107C0 ) | s->local_mem_size_index <<13 ;break ; +case SM501_ARBTRTN_CONTROL : + ret = s->arbitration_control; +break ;case SM501_IRQ_MASK : ret = s->irq_mask;break ;
@@ -812,6 +816,9 @@ static void sm501_system_config_write(void *opaque, hwaddr addr,/* TODO : check validity of size change */ s->dram_control |= value &0x7FFFFFC3 ;break ; +case SM501_ARBTRTN_CONTROL : + s->arbitration_control = value &0x37777777 ; +break ;case SM501_IRQ_MASK : s->irq_mask = value;break ;
Signature · 1 line
2.7.4
Peter Maydell
AppliedRe: [Qemu-devel] [PATCH v2 06/14] sm501: Add missing arbitration control register
Quoted 38 lines · BALATON Zoltan
Signed-off-by BALATON Zoltan <[email protected]>--- hw/display/sm501.c | 7 +++++++ 1 file changed, 7 insertions(+)hw/display/sm501.c +7
-1@@ -477,6 +477,7 @@ typedef struct SM501State {uint32_t gpio_31_0_control;uint32_t gpio_63_32_control;uint32_t dram_control; +uint32_t arbitration_control;uint32_t irq_mask;uint32_t misc_timing;uint32_t power_mode_control;@@ -760,6 +761,9 @@ static uint64_t sm501_system_config_read(void *opaque, hwaddr addr,case SM501_DRAM_CONTROL : ret = (s->dram_control &0x07F107C0 ) | s->local_mem_size_index <<13 ;break ; +case SM501_ARBTRTN_CONTROL : + ret = s->arbitration_control; +break ;case SM501_IRQ_MASK : ret = s->irq_mask;break ;@@ -812,6 +816,9 @@ static void sm501_system_config_write(void *opaque, hwaddr addr,/* TODO : check validity of size change */ s->dram_control |= value &0x7FFFFFC3 ;break ; +case SM501_ARBTRTN_CONTROL : + s->arbitration_control = value &0x37777777 ; +break ;case SM501_IRQ_MASK : s->irq_mask = value;break ; --
Quoted 1 line
2.7.4
Applied, thanks.
-- PMM
Peter Maydell
Reviewed-byRe: [Qemu-devel] [PATCH v2 06/14] sm501: Add missing arbitration control register
Quoted 34 lines · Peter Maydell
Quoted 33 lines · BALATON Zoltan
Signed-off-by BALATON Zoltan <[email protected]>--- hw/display/sm501.c | 7 +++++++ 1 file changed, 7 insertions(+)hw/display/sm501.c +6
-0@@ -477,6 +477,7 @@ typedef struct SM501State {uint32_t gpio_31_0_control;uint32_t gpio_63_32_control;uint32_t dram_control; +uint32_t arbitration_control;uint32_t irq_mask;uint32_t misc_timing;uint32_t power_mode_control;@@ -760,6 +761,9 @@ static uint64_t sm501_system_config_read(void *opaque, hwaddr addr,case SM501_DRAM_CONTROL : ret = (s->dram_control &0x07F107C0 ) | s->local_mem_size_index <<13 ;break ; +case SM501_ARBTRTN_CONTROL : + ret = s->arbitration_control; +break ;case SM501_IRQ_MASK : ret = s->irq_mask;break ;@@ -812,6 +816,9 @@ static void sm501_system_config_write(void *opaque, hwaddr addr,/* TODO : check validity of size change */ s->dram_control |= value &0x7FFFFFC3 ;break ; +case SM501_ARBTRTN_CONTROL : + s->arbitration_control = value &0x37777777 ;
Quoted 8 lines
Quoted 6 lines
hw/display/sm501.c +1
-1+break ;case SM501_IRQ_MASK : s->irq_mask = value;break ; --2.7.4
Applied, thanks.
Doh, wrong canned-email-reply template! I meant
thanks
-- PMM
PATCH v2 7/14 sm501: Fix device endianness
In reply to BALATON Zoltan---
v2: Split off small clean up to other patch
hw/display/sm501.c | 6 +++--- hw/display/sm501_template.h | 23 ++++++++++++++--------- 2 files changed, 17 insertions(+), 12 deletions(-)
hw/display/sm501.c +3 -3
@@ -849,7 +849,7 @@ static const MemoryRegionOps sm501_system_config_ops = { .min_access_size = 4, .max_access_size = 4, }, - .endianness = DEVICE_NATIVE_ENDIAN, + .endianness = DEVICE_LITTLE_ENDIAN, };static uint32_t sm501_palette_read (void *opaque,hwaddr addr)
@@ -1085,7 +1085,7 @@ static const MemoryRegionOps sm501_disp_ctrl_ops = { .min_access_size = 4, .max_access_size = 4, }, - .endianness = DEVICE_NATIVE_ENDIAN, + .endianness = DEVICE_LITTLE_ENDIAN, }; static uint64_t sm501_2d_engine_read(void *opaque, hwaddr addr,
@@ -1173,7 +1173,7 @@ static const MemoryRegionOps sm501_2d_engine_ops = { .min_access_size = 4, .max_access_size = 4, }, - .endianness = DEVICE_NATIVE_ENDIAN, + .endianness = DEVICE_LITTLE_ENDIAN, };/* draw line functions for all console modes */
hw/display/sm501_template.h +14 -9
@@ -64,10 +64,16 @@ static void glue(draw_line16_, PIXEL_NAME)(uint8_t r, g, b; do { - rgb565 = lduw_p(s); - r = ((rgb565 >> 11) & 0x1f) << 3; - g = ((rgb565 >> 5) & 0x3f) << 2; - b = ((rgb565 >> 0) & 0x1f) << 3; + rgb565 = lduw_le_p(s); +#if defined(TARGET_WORDS_BIGENDIAN) + r = (rgb565 >> 8) & 0xf8; + g = (rgb565 >> 3) & 0xfc; + b = (rgb565 << 3) & 0xf8; +#else + b = (rgb565 >> 8) & 0xf8; + g = (rgb565 >> 3) & 0xfc; + r = (rgb565 << 3) & 0xf8; +#endif *(PIXEL_TYPE *)d = glue(rgb_to_pixel, PIXEL_NAME)(r, g, b); s += 2; d += BPP;
@@ -80,15 +86,14 @@ static void glue(draw_line32_, PIXEL_NAME)(uint8_t r, g, b; do { - ldub_p(s); #if defined(TARGET_WORDS_BIGENDIAN) + r = s[0]; + g = s[1]; + b = s[2]; +#else r = s[1]; g = s[2]; b = s[3]; -#else - b = s[0]; - g = s[1]; - r = s[2]; #endif *(PIXEL_TYPE *)d = glue(rgb_to_pixel, PIXEL_NAME)(r, g, b); s += 4;
Signature · 1 line
2.7.4
Re: [Qemu-devel] [PATCH v2 07/14] sm501: Fix device endianness
Quoted 75 lines · BALATON Zoltan
Signed-off-by BALATON Zoltan <[email protected]>---
v2: Split off small clean up to other patch
hw/display/sm501.c | 6 +++--- hw/display/sm501_template.h | 23 ++++++++++++++--------- 2 files changed, 17 insertions(+), 12 deletions(-)hw/display/sm501.c +3
-3@@ -849,7 +849,7 @@ static const MemoryRegionOps sm501_system_config_ops = { .min_access_size = 4, .max_access_size = 4, }, - .endianness = DEVICE_NATIVE_ENDIAN, + .endianness = DEVICE_LITTLE_ENDIAN, };static uint32_t sm501_palette_read (void *opaque,hwaddr addr)@@ -1085,7 +1085,7 @@ static const MemoryRegionOps sm501_disp_ctrl_ops = { .min_access_size = 4, .max_access_size = 4, }, - .endianness = DEVICE_NATIVE_ENDIAN, + .endianness = DEVICE_LITTLE_ENDIAN, }; static uint64_t sm501_2d_engine_read(void *opaque, hwaddr addr,@@ -1173,7 +1173,7 @@ static const MemoryRegionOps sm501_2d_engine_ops = { .min_access_size = 4, .max_access_size = 4, }, - .endianness = DEVICE_NATIVE_ENDIAN, + .endianness = DEVICE_LITTLE_ENDIAN, };/* draw line functions for all console modes */ hw/display/sm501_template.h +14
-5@@ -64,10 +64,16 @@ static void glue(draw_line16_, PIXEL_NAME)(uint8_t r, g, b; do { - rgb565 = lduw_p(s); - r = ((rgb565 >> 11) & 0x1f) << 3; - g = ((rgb565 >> 5) & 0x3f) << 2; - b = ((rgb565 >> 0) & 0x1f) << 3; + rgb565 = lduw_le_p(s); +#if defined(TARGET_WORDS_BIGENDIAN) + r = (rgb565 >> 8) & 0xf8; + g = (rgb565 >> 3) & 0xfc; + b = (rgb565 << 3) & 0xf8; +#else + b = (rgb565 >> 8) & 0xf8; + g = (rgb565 >> 3) & 0xfc; + r = (rgb565 << 3) & 0xf8; +#endif *(PIXEL_TYPE *)d = glue(rgb_to_pixel, PIXEL_NAME)(r, g, b); s += 2; d += BPP;@@ -80,15 +86,14 @@ static void glue(draw_line32_, PIXEL_NAME)(uint8_t r, g, b; do { - ldub_p(s); #if defined(TARGET_WORDS_BIGENDIAN) + r = s[0]; + g = s[1]; + b = s[2]; +#else
hw/display/sm501_template.h static void glue(draw_line32_, PIXEL_NAME)( · line 99
Quoted 10 lines
hw/display/sm501_template.h +0
-4r = s[1 ]; g = s[2 ]; b = s[3 ]; -#else - b = s[0 ]; - g = s[1 ]; - r = s[2 ];#endif *(PIXEL_TYPE *)d =glue (rgb_to_pixel,PIXEL_NAME )(r, g, b); s +=4 ;
I still suspect this is not correct. In particular, if you look at the Linux driver:
http://lxr.free-electrons.com/source/drivers/video/fbdev/sm501fb.c#L341
we have for 32 bit two layouts within a 32-bit word:
BGRX (if "swap fb endian" is set)
XRGB (the usual)
(plus the cross-product with "32-bit load is BE or LE", since we've coded this as byte accesses rather than a ldl_*_p()).
and for 16 bit we have either RGB or BGR ordering. I'm not completely sure this lines up with the code you have.
Looking at how the SWAP_FB_ENDIAN flag gets set:
* for the r2d board it is set always
* for PCI devices, it is set only if big-endian
I suspect that what we actually have here is something like:
* for the PCI device it's always little endian (and the code ought not to need to depend on TARGET_WORDS_BIGENDIAN)
* sysbus device may be more complicated
Also I note there's an SM501_ENDIAN_CONTROL register on the device, which presumably if written changes the behaviour.
Plus for the sysbus device if we change the settings to DEVICE_LITTLE_ENDIAN for the various control register regions that suggests we should be consistent about the serial_mm_init() endian order and also the USB controller.
It's late evening here though so I can't investigate any further right now.
thanks
-- PMM
Re: [Qemu-devel] [PATCH v2 07/14] sm501: Fix device endianness
Quoted 92 lines · Peter Maydell
Quoted 75 lines · BALATON Zoltan
Signed-off-by BALATON Zoltan <[email protected]>---
v2: Split off small clean up to other patch
hw/display/sm501.c | 6 +++--- hw/display/sm501_template.h | 23 ++++++++++++++--------- 2 files changed, 17 insertions(+), 12 deletions(-)hw/display/sm501.c +3
-3@@ -849,7 +849,7 @@ static const MemoryRegionOps sm501_system_config_ops = { .min_access_size = 4, .max_access_size = 4, }, - .endianness = DEVICE_NATIVE_ENDIAN, + .endianness = DEVICE_LITTLE_ENDIAN, };static uint32_t sm501_palette_read (void *opaque,hwaddr addr)@@ -1085,7 +1085,7 @@ static const MemoryRegionOps sm501_disp_ctrl_ops = { .min_access_size = 4, .max_access_size = 4, }, - .endianness = DEVICE_NATIVE_ENDIAN, + .endianness = DEVICE_LITTLE_ENDIAN, }; static uint64_t sm501_2d_engine_read(void *opaque, hwaddr addr,@@ -1173,7 +1173,7 @@ static const MemoryRegionOps sm501_2d_engine_ops = { .min_access_size = 4, .max_access_size = 4, }, - .endianness = DEVICE_NATIVE_ENDIAN, + .endianness = DEVICE_LITTLE_ENDIAN, };/* draw line functions for all console modes */ hw/display/sm501_template.h +14
-5@@ -64,10 +64,16 @@ static void glue(draw_line16_, PIXEL_NAME)(uint8_t r, g, b; do { - rgb565 = lduw_p(s); - r = ((rgb565 >> 11) & 0x1f) << 3; - g = ((rgb565 >> 5) & 0x3f) << 2; - b = ((rgb565 >> 0) & 0x1f) << 3; + rgb565 = lduw_le_p(s); +#if defined(TARGET_WORDS_BIGENDIAN) + r = (rgb565 >> 8) & 0xf8; + g = (rgb565 >> 3) & 0xfc; + b = (rgb565 << 3) & 0xf8; +#else + b = (rgb565 >> 8) & 0xf8; + g = (rgb565 >> 3) & 0xfc; + r = (rgb565 << 3) & 0xf8; +#endif *(PIXEL_TYPE *)d = glue(rgb_to_pixel, PIXEL_NAME)(r, g, b); s += 2; d += BPP;@@ -80,15 +86,14 @@ static void glue(draw_line32_, PIXEL_NAME)(uint8_t r, g, b; do { - ldub_p(s); #if defined(TARGET_WORDS_BIGENDIAN) + r = s[0]; + g = s[1]; + b = s[2]; +#elsehw/display/sm501_template.h static void glue(draw_line32_, PIXEL_NAME)( · line 99
Quoted 10 lines
hw/display/sm501_template.h +0
-4r = s[1 ]; g = s[2 ]; b = s[3 ]; -#else - b = s[0 ]; - g = s[1 ]; - r = s[2 ];#endif *(PIXEL_TYPE *)d =glue (rgb_to_pixel,PIXEL_NAME )(r, g, b); s +=4 ;I still suspect this is not correct. In particular, if you look at the Linux driver:
http://lxr.free-electrons.com/source/drivers/video/fbdev/sm501fb.c#L341
Quoted 8 lines
we have for 32 bit two layouts within a 32-bit word:
BGRX (if "swap fb endian" is set)
XRGB (the usual)
(plus the cross-product with "32-bit load is BE or LE", since we've coded this as byte accesses rather than a ldl_*_p()).and for 16 bit we have either RGB or BGR ordering. I'm not completely sure this lines up with the code you have.
Maybe it's not correct but works for everything I could test better than the original code (which was broken even for the SH images one can find) so I think we could just go with this until someone complains and provides a test case. I've given up on trying to understand it because I don't really know this device and SH so I could only go by the images I could find and they seem to like it this way.
Quoted 4 lines
Looking at how the
SWAP_FB_ENDIANflag gets set:
* for the r2d board it is set always
* for PCI devices, it is set only if big-endian
Quoted 8 lines
I suspect that what we actually have here is something like:
* for the PCI device it's always little endian (and the code ought not to need to depend onTARGET_WORDS_BIGENDIAN)
* sysbus device may be more complicatedAlso I note there's an
SM501_ENDIAN_CONTROLregister on the device, which presumably if written changes the behaviour.
I've seen this but it's not emulated so it can be ignored for now. The spec also says that default is little endian so for regs at least DEVICE_LITTLE_ENDIAN should be OK.
Quoted 12 lines
Plus for the sysbus device if we change the settings to
DEVICE_LITTLE_ENDIANfor the various control register regions that suggests we should be consistent about theserial_mm_init()endian order and also the USB controller.It's late evening here though so I can't investigate any further right now.
thanks
-- PMM
Re: [Qemu-devel] [PATCH v2 07/14] sm501: Fix device endianness
Quoted 6 lines · BALATON Zoltan
Maybe it's not correct but works for everything I could test better than the original code (which was broken even for the SH images one can find) so I think we could just go with this until someone complains and provides a test case. I've given up on trying to understand it because I don't really know this device and SH so I could only go by the images I could find and they seem to like it this way.
So what cases have you tested? The Linux kernel seems to support:
* sh embedded device, little endian
* PCI card, little endian host
* PCI card, big endian host
and there are also
* 16 bit pixel format
* 32 bit pixel format
which makes 6 different combinations.
(It's a shame there's no sh4 BE code to run on this.)
Quoted 10 lines
Quoted 10 lines
Looking at how the
SWAP_FB_ENDIANflag gets set:
* for the r2d board it is set always
* for PCI devices, it is set only if big-endianI suspect that what we actually have here is something like:
* for the PCI device it's always little endian (and the code ought not to need to depend onTARGET_WORDS_BIGENDIAN)
* sysbus device may be more complicated
Quoted 6 lines
Quoted 2 lines
Also I note there's an
SM501_ENDIAN_CONTROLregister on the device, which presumably if written changes the behaviour.I've seen this but it's not emulated so it can be ignored for now. The spec also says that default is little endian so for regs at least
DEVICE_LITTLE_ENDIANshould be OK.
Yes, that makes sense. So we should be modelling this as an "always little endian device".
The changes you have to the memory region ops achieve this for the registers implemented in this device itself. It looks like SYSBUS_OHCI is already always little endian. You also need to change the argument to the serial_mm_init() call to pass DEVICE_LITTLE_ENDIAN.
thanks
-- PMM
Re: [Qemu-devel] [PATCH v2 07/14] sm501: Fix device endianness
Quoted 16 lines · Peter Maydell
Quoted 6 lines · BALATON Zoltan
Maybe it's not correct but works for everything I could test better than the original code (which was broken even for the SH images one can find) so I think we could just go with this until someone complains and provides a test case. I've given up on trying to understand it because I don't really know this device and SH so I could only go by the images I could find and they seem to like it this way.
So what cases have you tested? The Linux kernel seems to support:
* sh embedded device, little endian
* PCI card, little endian host
* PCI card, big endian host
and there are also
* 16 bit pixel format
* 32 bit pixel format
Quoted 2 lines
which makes 6 different combinations.
(It's a shame there's no sh4 BE code to run on this.)
As mentioned before I've used this image to test SH:
https://people.debian.org/~aurel32/qemu/sh4/
with video=800x600-16 kernel parameter where changing -16 to different bit depths (8, 32) reproduces the problem. On ppc I've tested with MorphOS which uses 16 bpp, Linux and U-boot which I think uses 8 bit. These run on real hardware so I think if they work the emulation should be OK.
You've said before that PCI is likely always little endian and SH embedded is LE too so unless something uses the device in BE mode (which we don't emulate) setting it to little endian should be correct. For frame buffer endianness I can only go with what I've seen clients doing and my patch does what worked for the above on SH and PPC. I can't prove it's correct but works for the systems I've targeted and also fixes the SH case which seemed to be broken.
Quoted 17 lines
Quoted 10 lines
Quoted 10 lines
Looking at how the
SWAP_FB_ENDIANflag gets set:
* for the r2d board it is set always
* for PCI devices, it is set only if big-endianI suspect that what we actually have here is something like:
* for the PCI device it's always little endian (and the code ought not to need to depend onTARGET_WORDS_BIGENDIAN)
* sysbus device may be more complicatedQuoted 6 lines
Quoted 2 lines
Also I note there's an
SM501_ENDIAN_CONTROLregister on the device, which presumably if written changes the behaviour.I've seen this but it's not emulated so it can be ignored for now. The spec also says that default is little endian so for regs at least
DEVICE_LITTLE_ENDIANshould be OK.
Quoted 8 lines
Yes, that makes sense. So we should be modelling this as an "always little endian device".
The changes you have to the memory region ops achieve this for the registers implemented in this device itself. It looks like
SYSBUS_OHCIis already always little endian. You also need to change the argument to theserial_mm_init()call to passDEVICE_LITTLE_ENDIAN.
OK, will do that.
Re: [Qemu-devel] [PATCH v2 07/14] sm501: Fix device endianness
Quoted 22 lines · BALATON Zoltan
Quoted 10 lines · Peter Maydell
So what cases have you tested? The Linux kernel seems to support:
* sh embedded device, little endian
* PCI card, little endian host
* PCI card, big endian host
and there are also
* 16 bit pixel format
* 32 bit pixel formatwhich makes 6 different combinations.
(It's a shame there's no sh4 BE code to run on this.)As mentioned before I've used this image to test SH:
https://people.debian.org/~aurel32/qemu/sh4/
with video=800x600-16 kernel parameter where changing -16 to different bit depths (8, 32) reproduces the problem. On ppc I've tested with MorphOS which uses 16 bpp, Linux and U-boot which I think uses 8 bit. These run on real hardware so I think if they work the emulation should be OK.
Quoted 7 lines
You've said before that PCI is likely always little endian and SH embedded is LE too so unless something uses the device in BE mode (which we don't emulate) setting it to little endian should be correct. For frame buffer endianness I can only go with what I've seen clients doing and my patch does what worked for the above on SH and PPC. I can't prove it's correct but works for the systems I've targeted and also fixes the SH case which seemed to be broken.
Right, but we should test the PCI-on-a-little-endian CPU case, because right now your code has #ifdef TARGET_WORDS_BIGENDIAN in it which means that the PCI device will behave differently on big and little endian CPUs, and you have a case that you haven't tested.
(At the moment with the set of cases you've tested the sh embedded device ones will always be using the "not TARGET_WORDS_BIGENDIAN" code and the PCI-on-PPC will always use the "is TARGET_WORDS_BIGENDIAN" code. But TARGET_WORDS_BIGENDIAN set/not set is orthogonal to PCI-vs-embedded.)
thanks
-- PMM
Re: [Qemu-devel] [PATCH v2 07/14] sm501: Fix device endianness
Quoted 31 lines · Peter Maydell
Quoted 22 lines · BALATON Zoltan
Quoted 10 lines · Peter Maydell
So what cases have you tested? The Linux kernel seems to support:
* sh embedded device, little endian
* PCI card, little endian host
* PCI card, big endian host
and there are also
* 16 bit pixel format
* 32 bit pixel formatwhich makes 6 different combinations.
(It's a shame there's no sh4 BE code to run on this.)As mentioned before I've used this image to test SH:
https://people.debian.org/~aurel32/qemu/sh4/
with video=800x600-16 kernel parameter where changing -16 to different bit depths (8, 32) reproduces the problem. On ppc I've tested with MorphOS which uses 16 bpp, Linux and U-boot which I think uses 8 bit. These run on real hardware so I think if they work the emulation should be OK.
Quoted 7 lines
You've said before that PCI is likely always little endian and SH embedded is LE too so unless something uses the device in BE mode (which we don't emulate) setting it to little endian should be correct. For frame buffer endianness I can only go with what I've seen clients doing and my patch does what worked for the above on SH and PPC. I can't prove it's correct but works for the systems I've targeted and also fixes the SH case which seemed to be broken.
Quoted 5 lines
Right, but we should test the PCI-on-a-little-endian CPU case, because right now your code has #ifdef
TARGET_WORDS_BIGENDIANin it which means that the PCI device will behave differently on big and little endian CPUs, and you have a case that you haven't tested.
I could not find an image to test this. I've tried removing the sysbus version from SH and add a PCI one instead but then Linux did not seem to find the card. I could not find any other OS images that would have this combination that is also known to work on any real hardware so I think if someone ever finds a problem with such combination then we can fix it when having a test case, since I could not figure out theoretically what should be the right way.
Quoted 6 lines
(At the moment with the set of cases you've tested the sh embedded device ones will always be using the "not
TARGET_WORDS_BIGENDIAN" code and the PCI-on-PPC will always use the "isTARGET_WORDS_BIGENDIAN" code. ButTARGET_WORDS_BIGENDIANset/not set is orthogonal to PCI-vs-embedded.)
Why do you think that framebuffer endianness is dependent on PCI-vs-embedded as opposed to target endianness? The latter seems more plausible to me. The TARGET_WORDS_BIGENDIAN was already there for 32 bit case. My patch only adds it to the 16 bit too and fixing the bytes for both (which seemed to be already wrong even on SH). So I think previously it was broken, my patch fixes it to be better (or at most broken in a different way) so I don't think it's worse than before thus, an improvement.
Quoted 4 lines
thanks
-- PMM
Re: [Qemu-devel] [PATCH v2 07/14] sm501: Fix device endianness
Quoted 11 lines · BALATON Zoltan
Quoted 5 lines · Peter Maydell
Right, but we should test the PCI-on-a-little-endian CPU case, because right now your code has #ifdef
TARGET_WORDS_BIGENDIANin it which means that the PCI device will behave differently on big and little endian CPUs, and you have a case that you haven't tested.I could not find an image to test this. I've tried removing the sysbus version from SH and add a PCI one instead but then Linux did not seem to find the card.
I was thinking about testing the PCI card on x86.
Quoted 14 lines
I could not find any other OS images that would have this combination that is also known to work on any real hardware so I think if someone ever finds a problem with such combination then we can fix it when having a test case, since I could not figure out theoretically what should be the right way.
Quoted 6 lines
(At the moment with the set of cases you've tested the sh embedded device ones will always be using the "not
TARGET_WORDS_BIGENDIAN" code and the PCI-on-PPC will always use the "isTARGET_WORDS_BIGENDIAN" code. ButTARGET_WORDS_BIGENDIANset/not set is orthogonal to PCI-vs-embedded.)
Quoted 2 lines
Why do you think that framebuffer endianness is dependent on PCI-vs-embedded as opposed to target endianness? The latter seems more plausible to me.
The PCI card should behave the same way whether you plug it into a PPC system or an x86 system (since it's the same hardware). So code in the handling of the PCI card that looks at TARGET_WORDS_BIGENDIAN is suspicious.
thanks
-- PMM
Re: [Qemu-devel] [PATCH v2 07/14] sm501: Fix device endianness
Quoted 29 lines · Peter Maydell
Quoted 11 lines · BALATON Zoltan
Quoted 5 lines · Peter Maydell
Right, but we should test the PCI-on-a-little-endian CPU case, because right now your code has #ifdef
TARGET_WORDS_BIGENDIANin it which means that the PCI device will behave differently on big and little endian CPUs, and you have a case that you haven't tested.I could not find an image to test this. I've tried removing the sysbus version from SH and add a PCI one instead but then Linux did not seem to find the card.
I was thinking about testing the PCI card on x86.
Quoted 14 lines
I could not find any other OS images that would have this combination that is also known to work on any real hardware so I think if someone ever finds a problem with such combination then we can fix it when having a test case, since I could not figure out theoretically what should be the right way.
Quoted 6 lines
(At the moment with the set of cases you've tested the sh embedded device ones will always be using the "not
TARGET_WORDS_BIGENDIAN" code and the PCI-on-PPC will always use the "isTARGET_WORDS_BIGENDIAN" code. ButTARGET_WORDS_BIGENDIANset/not set is orthogonal to PCI-vs-embedded.)
Quoted 7 lines
Quoted 2 lines
Why do you think that framebuffer endianness is dependent on PCI-vs-embedded as opposed to target endianness? The latter seems more plausible to me.
The PCI card should behave the same way whether you plug it into a PPC system or an x86 system (since it's the same hardware). So code in the handling of the PCI card that looks at
TARGET_WORDS_BIGENDIANis suspicious.
I've searched the web for this and read about possible cards having this chip working under Linux on x86 (or not working as these were mostly bug reports) but could not find an image that is known to be working with such card. I've tried a few recent Linux live CDs (Ubuntu and Fedora) with x86_64 and sm501 PCI emulation on QEMU but these did not seem to boot and find the card, only with the default VGA. Probably a card would have some BIOS and some VESA modes that the drivers would use so I'm not sure a PCI card would really behave the same on PPC and x64 (other video cards usually don't work between PC and PPC Macs because of the BIOS on them). Also on PPC this chip is mostly found on embedded boards not on pluggable cards.
Regards,
BALATON Zoltan
PATCH v2 8/14 sm501: Fix hardware cursor
In reply to BALATON Zoltan--- hw/display/sm501.c | 169 +++++++++++++++++++++++++------------------- hw/display/sm501_template.h | 25 +++---- 2 files changed, 107 insertions(+), 87 deletions(-)
hw/display/sm501.c +97 -72
@@ -553,6 +553,24 @@ static uint32_t get_local_mem_size_index(uint32_t size)return index; } +static inline int get_width (SM501State *s,int crt) +{ +int width = crt ? s->dc_crt_h_total : s->dc_panel_h_total; +return (width &0x00000FFF ) +1 ; +} + +static inline int get_height (SM501State *s,int crt) +{ +int height = crt ? s->dc_crt_v_total : s->dc_panel_v_total; +return (height &0x00000FFF ) +1 ; +} + +static inline int get_bpp (SM501State *s,int crt) +{ +int bpp = crt ? s->dc_crt_control : s->dc_panel_control; +return (8 << (bpp &3 )) /8 ; +} +/** * Check the availability of hardware cursor. * @param crt 0 for PANEL, 1 for CRT.
@@ -567,10 +585,10 @@ static inline int is_hwc_enabled(SM501State *state, int crt)* Get the address which holds cursor pattern data. * @param crt 0 for PANEL, 1 for CRT. */ -static inline uint32_t get_hwc_address (SM501State *state,int crt) +static inline uint8_t *get_hwc_address (SM501State *state,int crt) {uint32_t addr = crt ? state->dc_crt_hwc_addr : state->dc_panel_hwc_addr; -return (addr &0x03FFFFF0 )/* >> 4*/ ; +return state->local_mem + (addr &0x03FFFFF0 ); }/**
@@ -596,50 +614,48 @@ static inline uint32_t get_hwc_x(SM501State *state, int crt) }/** -* Get the cursor position in x coordinate. +* Get the hardware cursor palette. * @param crt 0 for PANEL, 1 for CRT. -* @param index 0, 1, 2 or 3 which specifies color of corsor dot. +* @param palette pointer to a [3 * 3] array to store color values in */ -static inline uint16_t get_hwc_color (SM501State *state,int crt,int index) +static inline void get_hwc_palette (SM501State *state,int crt,uint8_t *palette) { -uint32_t color_reg =0 ; -uint16_t color_565 =0 ; - -if (index ==0 ) { -return 0 ; - } - -switch (index) { -case 1 : -case 2 : - color_reg = crt ? state->dc_crt_hwc_color_1_2 - : state->dc_panel_hwc_color_1_2; -break ; -case 3 : - color_reg = crt ? state->dc_crt_hwc_color_3 - : state->dc_panel_hwc_color_3; -break ; -default : -printf ("invalid hw cursor color.\n" ); -abort (); - } +int i; +uint32_t color_reg; +uint16_t rgb565; + +for (i =0 ; i <3 ; i++) { +if (i +1 ==3 ) { + color_reg = crt ? state->dc_crt_hwc_color_3 + : state->dc_panel_hwc_color_3; + }else { + color_reg = crt ? state->dc_crt_hwc_color_1_2 + : state->dc_panel_hwc_color_1_2; + } -switch (index) { -case 1 : -case 3 : - color_565 = (uint16_t )(color_reg &0xFFFF ); -break ; -case 2 : - color_565 = (uint16_t )((color_reg >>16 ) &0xFFFF ); -break ; +if (i +1 ==2 ) { + rgb565 = (color_reg >>16 ) &0xFFFF ; + }else { + rgb565 = color_reg &0xFFFF ; + } + palette[i *3 +0 ] = (rgb565 <<3 ) &0xf8 ;/* red */ + palette[i *3 +1 ] = (rgb565 >>3 ) &0xfc ;/* green */ + palette[i *3 +2 ] = (rgb565 >>8 ) &0xf8 ;/* blue */ } -return color_565; } -static int within_hwc_y_range (SM501State *state,int y,int crt) +static inline void hwc_invalidate (SM501State *s,int crt) { -int hwc_y =get_hwc_y (state, crt); -return (hwc_y <= y && y < hwc_y +SM501_HWC_HEIGHT ); +int w =get_width (s, crt); +int h =get_height (s, crt); +int bpp =get_bpp (s, crt); +int start =get_hwc_y (s, crt); +int end =MIN (h, start +SM501_HWC_HEIGHT ) +1 ; + + start *= w * bpp; + end *= w * bpp; + +memory_region_set_dirty (&s->local_mem_region, start, end - start); }static void sm501_2d_operation (SM501State *s)
@@ -1020,10 +1036,18 @@ static void sm501_disp_ctrl_write(void *opaque, hwaddr addr,break ;case SM501_DC_PANEL_HWC_ADDR : - s->dc_panel_hwc_addr = value &0x8FFFFFF0 ; + value &=0x8FFFFFF0 ; +if (value != s->dc_panel_hwc_addr) { +hwc_invalidate (s,0 ); + s->dc_panel_hwc_addr = value; + }break ;case SM501_DC_PANEL_HWC_LOC : - s->dc_panel_hwc_location = value &0x0FFF0FFF ; + value &=0x0FFF0FFF ; +if (value != s->dc_panel_hwc_location) { +hwc_invalidate (s,0 ); + s->dc_panel_hwc_location = value; + }break ;case SM501_DC_PANEL_HWC_COLOR_1_2 : s->dc_panel_hwc_color_1_2 = value;
@@ -1055,10 +1079,18 @@ static void sm501_disp_ctrl_write(void *opaque, hwaddr addr,break ;case SM501_DC_CRT_HWC_ADDR : - s->dc_crt_hwc_addr = value &0x8FFFFFF0 ; + value &=0x8FFFFFF0 ; +if (value != s->dc_crt_hwc_addr) { +hwc_invalidate (s,1 ); + s->dc_crt_hwc_addr = value; + }break ;case SM501_DC_CRT_HWC_LOC : - s->dc_crt_hwc_location = value &0x0FFF0FFF ; + value &=0x0FFF0FFF ; +if (value != s->dc_crt_hwc_location) { +hwc_invalidate (s,1 ); + s->dc_crt_hwc_location = value; + }break ;case SM501_DC_CRT_HWC_COLOR_1_2 : s->dc_crt_hwc_color_1_2 = value;
@@ -1181,8 +1213,9 @@ static const MemoryRegionOps sm501_2d_engine_ops = {typedef void draw_line_func (uint8_t *d,const uint8_t *s,int width,const uint32_t *pal); -typedef void draw_hwc_line_func (SM501State *s,int crt,uint8_t *palette, -int c_y,uint8_t *d,int width); +typedef void draw_hwc_line_func (uint8_t *d,const uint8_t *s, +int width,const uint8_t *palette, +int c_x,int c_y);#define DEPTH 8#include "sm501_template.h"
@@ -1270,12 +1303,11 @@ static inline int get_depth_index(DisplaySurface *surface)static void sm501_draw_crt (SM501State *s) {DisplaySurface *surface =qemu_console_surface (s->con); -int y; -int width = (s->dc_crt_h_total &0x00000FFF ) +1 ; -int height = (s->dc_crt_v_total &0x00000FFF ) +1 ; - -uint8_t *src = s->local_mem; -int src_bpp =0 ; +int y, c_x, c_y; +uint8_t *hwc_src, *src = s->local_mem; +int width =get_width (s,1 ); +int height =get_height (s,1 ); +int src_bpp =get_bpp (s,1 );int dst_bpp =surface_bytes_per_pixel (surface);uint32_t *palette = (uint32_t *)&s->dc_palette[SM501_DC_CRT_PALETTE -SM501_DC_PANEL_PALETTE ];
@@ -1290,17 +1322,14 @@ static void sm501_draw_crt(SM501State *s)ram_addr_t offset =0 ;/* choose draw_line function */ -switch (s->dc_crt_control &3 ) { -case SM501_DC_CRT_CONTROL_8BPP : - src_bpp =1 ; +switch (src_bpp) { +case 1 : draw_line = draw_line8_funcs[ds_depth_index];break ; -case SM501_DC_CRT_CONTROL_16BPP : - src_bpp =2 ; +case 2 : draw_line = draw_line16_funcs[ds_depth_index];break ; -case SM501_DC_CRT_CONTROL_32BPP : - src_bpp =4 ; +case 4 : draw_line = draw_line32_funcs[ds_depth_index];break ;default :
@@ -1312,18 +1341,12 @@ static void sm501_draw_crt(SM501State *s)/* set up to draw hardware cursor */ if (is_hwc_enabled (s,1 )) { -int i; - -/* get cursor palette */ -for (i =0 ; i <3 ; i++) { -uint16_t rgb565 =get_hwc_color (s,1 , i +1 ); - hwc_palette[i *3 +0 ] = (rgb565 &0xf800 ) >>8 ;/* red */ - hwc_palette[i *3 +1 ] = (rgb565 &0x07e0 ) >>3 ;/* green */ - hwc_palette[i *3 +2 ] = (rgb565 &0x001f ) <<3 ;/* blue */ - } -/* choose cursor draw line function */ draw_hwc_line = draw_hwc_line_funcs[ds_depth_index]; + hwc_src =get_hwc_address (s,1 ); + c_x =get_hwc_x (s,1 ); + c_y =get_hwc_y (s,1 ); +get_hwc_palette (s,1 , hwc_palette); }/* adjust console size */
@@ -1338,14 +1361,16 @@ static void sm501_draw_crt(SM501State *s)/* draw each line according to conditions */ memory_region_sync_dirty_bitmap (&s->local_mem_region);for (y =0 ; y < height; y++) { -int update_hwc = draw_hwc_line ?within_hwc_y_range (s, y,1 ) :0 ; -int update = full_update || update_hwc; +int update, update_hwc;ram_addr_t page0 = offset;ram_addr_t page1 = offset + width * src_bpp -1 ; +/* check if hardware cursor is enabled and we're within its range */ + update_hwc = draw_hwc_line && c_y <= y && y < c_y +SM501_HWC_HEIGHT ; + update = full_update || update_hwc;/* check dirty flags for each line */ - update =memory_region_get_dirty (&s->local_mem_region, page0, - page1 - page0,DIRTY_MEMORY_VGA ); + update |=memory_region_get_dirty (&s->local_mem_region, page0, + page1 - page0,DIRTY_MEMORY_VGA );/* draw line and change status */ if (update) {
@@ -1357,7 +1382,7 @@ static void sm501_draw_crt(SM501State *s)/* draw hardware cursor */ if (update_hwc) { -draw_hwc_line (s,1 , hwc_palette, y -get_hwc_y (s,1 ), d, width); +draw_hwc_line (d, hwc_src, width, hwc_palette, c_x, y - c_y); }if (y_start <0 ) {
hw/display/sm501_template.h +10 -15
@@ -104,29 +104,24 @@ static void glue(draw_line32_, PIXEL_NAME)(/** * Draw hardware cursor image on the given line. */ -static void glue (draw_hwc_line_ ,PIXEL_NAME )(SM501State *s,int crt, -uint8_t *palette,int c_y,uint8_t *d,int width) +static void glue (draw_hwc_line_ ,PIXEL_NAME )(uint8_t *d,const uint8_t *s, +int width,const uint8_t *palette,int c_x,int c_y) { -int x, i; -uint8_t *pixval, bitset =0 ; - -/* get hardware cursor pattern */ -uint32_t cursor_addr =get_hwc_address (s, crt); -assert (0 <= c_y && c_y <SM501_HWC_HEIGHT ); - cursor_addr +=SM501_HWC_WIDTH * c_y /4 ;/* 4 pixels per byte */ - pixval = s->local_mem + cursor_addr; +int i; +uint8_t bitset =0 ;/* get cursor position */ - x =get_hwc_x (s, crt); - d += x *BPP ; +assert (0 <= c_y && c_y <SM501_HWC_HEIGHT ); + s +=SM501_HWC_WIDTH * c_y /4 ;/* 4 pixels per byte */ + d += c_x *BPP ; -for (i =0 ; i <SM501_HWC_WIDTH && x + i < width; i++) { +for (i =0 ; i <SM501_HWC_WIDTH && c_x + i < width; i++) {uint8_t v;/* get pixel value */ if (i %4 ==0 ) { - bitset =ldub_p (pixval); - pixval++; + bitset =ldub_p (s); + s++; } v = bitset &3 ; bitset >>=2 ;
Signature · 1 line
2.7.4
Re: [Qemu-devel] [PATCH v2 08/14] sm501: Fix hardware cursor
Quoted 1 line · BALATON Zoltan
Signed-off-by BALATON Zoltan <[email protected]>
Can you say in the commit message what the bugs you're fixing are, please? Otherwise it's a bit hard to disentangle here what's just refactoring and what's making behavioural changes.
Quoted 19 lines
--- hw/display/sm501.c | 169 +++++++++++++++++++++++++------------------- hw/display/sm501_template.h | 25 +++---- 2 files changed, 107 insertions(+), 87 deletions(-)hw/display/sm501.c +6
-0@@ -553,6 +553,24 @@ static uint32_t get_local_mem_size_index(uint32_t size)return index; } +static inline int get_width (SM501State *s,int crt) +{ +int width = crt ? s->dc_crt_h_total : s->dc_panel_h_total; +return (width &0x00000FFF ) +1 ; +} +
hw/display/sm501.c static uint32_t get_local_mem_size_index(uint32_t size) · line 571
Quoted 10 lines
hw/display/sm501.c +10
-0+static inline int get_height (SM501State *s,int crt) +{ +int height = crt ? s->dc_crt_v_total : s->dc_panel_v_total; +return (height &0x00000FFF ) +1 ; +} + +static inline int get_bpp (SM501State *s,int crt) +{ +int bpp = crt ? s->dc_crt_control : s->dc_panel_control; +return (8 << (bpp &3 )) /8 ;
Isn't this the same as "1 << (bpp & 3)" ?
hw/display/sm501.c static uint32_t get_local_mem_size_index(uint32_t size) · line 572
Quoted 1 line
hw/display/sm501.c +1
-0+}
thanks
-- PMM
PATCH v2 9/14 sm501: Misc clean ups
In reply to BALATON Zoltan- Rename a variable
- Move variable declarations out of loop to the beginning in draw_hwc_line
--- hw/display/sm501.c | 10 +++++----- hw/display/sm501_template.h | 10 ++++------ 2 files changed, 9 insertions(+), 11 deletions(-)
hw/display/sm501.c +5 -5
@@ -1312,7 +1312,7 @@ static void sm501_draw_crt(SM501State *s)uint32_t *palette = (uint32_t *)&s->dc_palette[SM501_DC_CRT_PALETTE -SM501_DC_PANEL_PALETTE ];uint8_t hwc_palette[3 *3 ]; -int ds_depth_index =get_depth_index (surface); +int dst_depth_index =get_depth_index (surface);draw_line_func *draw_line =NULL ;draw_hwc_line_func *draw_hwc_line =NULL ;int full_update =0 ;
@@ -1324,13 +1324,13 @@ static void sm501_draw_crt(SM501State *s)/* choose draw_line function */ switch (src_bpp) {case 1 : - draw_line = draw_line8_funcs[ds_depth_index]; + draw_line = draw_line8_funcs[dst_depth_index];break ;case 2 : - draw_line = draw_line16_funcs[ds_depth_index]; + draw_line = draw_line16_funcs[dst_depth_index];break ;case 4 : - draw_line = draw_line32_funcs[ds_depth_index]; + draw_line = draw_line32_funcs[dst_depth_index];break ;default : printf("sm501 draw crt : invalid DC_CRT_CONTROL=%x.\n",
@@ -1342,7 +1342,7 @@ static void sm501_draw_crt(SM501State *s)/* set up to draw hardware cursor */ if (is_hwc_enabled (s,1 )) {/* choose cursor draw line function */ - draw_hwc_line = draw_hwc_line_funcs[ds_depth_index]; + draw_hwc_line = draw_hwc_line_funcs[dst_depth_index]; hwc_src =get_hwc_address (s,1 ); c_x =get_hwc_x (s,1 ); c_y =get_hwc_y (s,1 );
hw/display/sm501_template.h +4 -6
@@ -108,7 +108,7 @@ static void glue(draw_hwc_line_, PIXEL_NAME)(uint8_t *d, const uint8_t *s, int width, const uint8_t *palette, int c_x, int c_y) { int i; -uint8_t bitset =0 ; +uint8_t r, g, b, v, bitset =0 ;/* get cursor position */ assert (0 <= c_y && c_y <SM501_HWC_HEIGHT );
@@ -116,8 +116,6 @@ static void glue(draw_hwc_line_, PIXEL_NAME)(uint8_t *d, const uint8_t *s, d += c_x *BPP ;for (i =0 ; i <SM501_HWC_WIDTH && c_x + i < width; i++) { -uint8_t v; -/* get pixel value */ if (i %4 ==0 ) { bitset =ldub_p (s);
@@ -129,9 +127,9 @@ static void glue(draw_hwc_line_, PIXEL_NAME)(uint8_t *d, const uint8_t *s,/* write pixel */ if (v) { v--; -uint8_t r = palette[v *3 +0 ]; -uint8_t g = palette[v *3 +1 ]; -uint8_t b = palette[v *3 +2 ]; + r = palette[v *3 +0 ]; + g = palette[v *3 +1 ]; + b = palette[v *3 +2 ]; *(PIXEL_TYPE *)d =glue (rgb_to_pixel,PIXEL_NAME )(r, g, b); } d +=BPP ;
Signature · 1 line
2.7.4
Re: [Qemu-devel] [PATCH v2 09/14] sm501: Misc clean ups
Quoted 8 lines · BALATON Zoltan
- Rename a variable
- Move variable declarations out of loop to the beginning indraw_hwc_lineSigned-off-by BALATON Zoltan <[email protected]>--- hw/display/sm501.c | 10 +++++----- hw/display/sm501_template.h | 10 ++++------ 2 files changed, 9 insertions(+), 11 deletions(-
thanks
-- PMM
PATCH v2 10/14 sm501: Add support for panel layer
In reply to BALATON Zoltan---
v2: Split off renaming a variable to separate clean up patch
hw/display/sm501.c | 63 +++++++++++++++++++++++++++--------------------------- 1 file changed, 32 insertions(+), 31 deletions(-)
hw/display/sm501.c +32 -31
@@ -2,6 +2,7 @@* QEMU SM501 Device * * Copyright (c) 2008 Shin-ichiro KAWASAKI +* Copyright (c) 2016 BALATON Zoltan * * Permission is hereby granted, free of charge, to any person obtaining a copy * of this software and associated documentation files (the "Software"), to deal
@@ -41,8 +42,11 @@* - Minimum implementation for Linux console : mmio regs and CRT layer. * - 2D graphics acceleration partially supported : only fill rectangle. * -* TODO: +* Status: 2016/12/04 +* - Misc fixes: endianness, hardware cursor * - Panel support +* +* TODO: * - Touch panel support * - USB support * - UART support
@@ -1300,18 +1304,16 @@ static inline int get_depth_index(DisplaySurface *surface) } } -static void sm501_draw_crt (SM501State *s) +static void sm501_update_display (void *opaque) { +SM501State *s = (SM501State *)opaque;DisplaySurface *surface =qemu_console_surface (s->con);int y, c_x, c_y; -uint8_t *hwc_src, *src = s->local_mem; -int width =get_width (s,1 ); -int height =get_height (s,1 ); -int src_bpp =get_bpp (s,1 ); +int crt = (s->dc_crt_control &SM501_DC_CRT_CONTROL_SEL ) ?1 :0 ; +int width =get_width (s, crt); +int height =get_height (s, crt); +int src_bpp =get_bpp (s, crt);int dst_bpp =surface_bytes_per_pixel (surface); -uint32_t *palette = (uint32_t *)&s->dc_palette[SM501_DC_CRT_PALETTE - -SM501_DC_PANEL_PALETTE ]; -uint8_t hwc_palette[3 *3 ];int dst_depth_index =get_depth_index (surface);draw_line_func *draw_line =NULL ;draw_hwc_line_func *draw_hwc_line =NULL ;
@@ -1319,7 +1321,19 @@ static void sm501_draw_crt(SM501State *s)int y_start =-1 ;ram_addr_t page_min = ~0l ;ram_addr_t page_max =0l ; -ram_addr_t offset =0 ; +ram_addr_t offset; +uint32_t *palette; +uint8_t hwc_palette[3 *3 ]; +uint8_t *hwc_src; + +if (!((crt ? s->dc_crt_control : s->dc_panel_control) + &SM501_DC_CRT_CONTROL_ENABLE )) { +return ; + } + + palette = (uint32_t *)(crt ? &s->dc_palette[SM501_DC_CRT_PALETTE - +SM501_DC_PANEL_PALETTE ] + : &s->dc_palette[0 ]);/* choose draw_line function */ switch (src_bpp) {
@@ -1333,20 +1347,19 @@ static void sm501_draw_crt(SM501State *s) draw_line = draw_line32_funcs[dst_depth_index];break ;default : -printf ("sm501 draw crt : invalid DC_CRT_CONTROL=%x.\n" , - s->dc_crt_control); +printf ("sm501 update display : invalid control register value.\n" );abort ();break ; }/* set up to draw hardware cursor */ -if (is_hwc_enabled (s,1 )) { +if (is_hwc_enabled (s, crt)) {/* choose cursor draw line function */ draw_hwc_line = draw_hwc_line_funcs[dst_depth_index]; - hwc_src =get_hwc_address (s,1 ); - c_x =get_hwc_x (s,1 ); - c_y =get_hwc_y (s,1 ); -get_hwc_palette (s,1 , hwc_palette); + hwc_src =get_hwc_address (s, crt); + c_x =get_hwc_x (s, crt); + c_y =get_hwc_y (s, crt); +get_hwc_palette (s, crt, hwc_palette); }/* adjust console size */
@@ -1360,7 +1373,7 @@ static void sm501_draw_crt(SM501State *s)/* draw each line according to conditions */ memory_region_sync_dirty_bitmap (&s->local_mem_region); -for (y =0 ; y < height; y++) { +for (y =0 , offset =0 ; y < height; y++, offset += width * src_bpp) {int update, update_hwc;ram_addr_t page0 = offset;ram_addr_t page1 = offset + width * src_bpp -1 ;
@@ -1378,7 +1391,7 @@ static void sm501_draw_crt(SM501State *s) d += y * width * dst_bpp;/* draw graphics layer */ -draw_line (d, src, width, palette); +draw_line (d, s->local_mem + offset, width, palette);/* draw hardware cursor */ if (update_hwc) {
@@ -1401,9 +1414,6 @@ static void sm501_draw_crt(SM501State *s) y_start =-1 ; } } - - src += width * src_bpp; - offset += width * src_bpp; }/* complete flush to display */
@@ -1419,15 +1429,6 @@ static void sm501_draw_crt(SM501State *s) } } -static void sm501_update_display (void *opaque) -{ -SM501State *s = (SM501State *)opaque; - -if (s->dc_crt_control &SM501_DC_CRT_CONTROL_ENABLE ) { -sm501_draw_crt (s); - } -} -static const GraphicHwOps sm501_ops = { .gfx_update = sm501_update_display, };
Signature · 1 line
2.7.4
Re: [Qemu-devel] [PATCH v2 10/14] sm501: Add support for panel layer
Quoted 125 lines · BALATON Zoltan
Signed-off-by BALATON Zoltan <[email protected]>---
v2: Split off renaming a variable to separate clean up patch
hw/display/sm501.c | 63 +++++++++++++++++++++++++++--------------------------- 1 file changed, 32 insertions(+), 31 deletions(-)hw/display/sm501.c +32
-19@@ -2,6 +2,7 @@* QEMU SM501 Device * * Copyright (c) 2008 Shin-ichiro KAWASAKI +* Copyright (c) 2016 BALATON Zoltan * * Permission is hereby granted, free of charge, to any person obtaining a copy * of this software and associated documentation files (the "Software"), to deal @@ -41,8 +42,11 @@* - Minimum implementation for Linux console : mmio regs and CRT layer. * - 2D graphics acceleration partially supported : only fill rectangle. * -* TODO: +* Status: 2016/12/04 +* - Misc fixes: endianness, hardware cursor * - Panel support +* +* TODO: * - Touch panel support * - USB support * - UART support @@ -1300,18 +1304,16 @@ static inline int get_depth_index(DisplaySurface *surface) } } -static void sm501_draw_crt (SM501State *s) +static void sm501_update_display (void *opaque) { +SM501State *s = (SM501State *)opaque;DisplaySurface *surface =qemu_console_surface (s->con);int y, c_x, c_y; -uint8_t *hwc_src, *src = s->local_mem; -int width =get_width (s,1 ); -int height =get_height (s,1 ); -int src_bpp =get_bpp (s,1 ); +int crt = (s->dc_crt_control &SM501_DC_CRT_CONTROL_SEL ) ?1 :0 ; +int width =get_width (s, crt); +int height =get_height (s, crt); +int src_bpp =get_bpp (s, crt);int dst_bpp =surface_bytes_per_pixel (surface); -uint32_t *palette = (uint32_t *)&s->dc_palette[SM501_DC_CRT_PALETTE - -SM501_DC_PANEL_PALETTE ]; -uint8_t hwc_palette[3 *3 ];int dst_depth_index =get_depth_index (surface);draw_line_func *draw_line =NULL ;draw_hwc_line_func *draw_hwc_line =NULL ;@@ -1319,7 +1321,19 @@ static void sm501_draw_crt(SM501State *s)int y_start =-1 ;ram_addr_t page_min = ~0l ;ram_addr_t page_max =0l ; -ram_addr_t offset =0 ; +ram_addr_t offset; +uint32_t *palette; +uint8_t hwc_palette[3 *3 ]; +uint8_t *hwc_src; + +if (!((crt ? s->dc_crt_control : s->dc_panel_control) + &SM501_DC_CRT_CONTROL_ENABLE )) { +return ; + } + + palette = (uint32_t *)(crt ? &s->dc_palette[SM501_DC_CRT_PALETTE - +SM501_DC_PANEL_PALETTE ] + : &s->dc_palette[0 ]);/* choose draw_line function */ switch (src_bpp) {@@ -1333,20 +1347,19 @@ static void sm501_draw_crt(SM501State *s) draw_line = draw_line32_funcs[dst_depth_index];break ;default : -printf ("sm501 draw crt : invalid DC_CRT_CONTROL=%x.\n" , - s->dc_crt_control); +printf ("sm501 update display : invalid control register value.\n" );abort ();break ; }/* set up to draw hardware cursor */ -if (is_hwc_enabled (s,1 )) { +if (is_hwc_enabled (s, crt)) {/* choose cursor draw line function */ draw_hwc_line = draw_hwc_line_funcs[dst_depth_index]; - hwc_src =get_hwc_address (s,1 ); - c_x =get_hwc_x (s,1 ); - c_y =get_hwc_y (s,1 ); -get_hwc_palette (s,1 , hwc_palette); + hwc_src =get_hwc_address (s, crt); + c_x =get_hwc_x (s, crt); + c_y =get_hwc_y (s, crt); +get_hwc_palette (s, crt, hwc_palette); }/* adjust console size */ @@ -1360,7 +1373,7 @@ static void sm501_draw_crt(SM501State *s)/* draw each line according to conditions */ memory_region_sync_dirty_bitmap (&s->local_mem_region); -for (y =0 ; y < height; y++) { +for (y =0 , offset =0 ; y < height; y++, offset += width * src_bpp) {int update, update_hwc;ram_addr_t page0 = offset;ram_addr_t page1 = offset + width * src_bpp -1 ;@@ -1378,7 +1391,7 @@ static void sm501_draw_crt(SM501State *s) d += y * width * dst_bpp;/* draw graphics layer */ -draw_line (d, src, width, palette); +draw_line (d, s->local_mem + offset, width, palette);/* draw hardware cursor */ if (update_hwc) {
hw/display/sm501.c static void sm501_draw_crt(SM501State *s) · line 1417
Quoted 7 lines
hw/display/sm501.c +0
-3@@ -1401,9 +1414,6 @@ static void sm501_draw_crt(SM501State *s) y_start =-1 ; } } - - src += width * src_bpp; - offset += width * src_bpp; }
This is another refactoring that's unrelated to adding panel layer support, right? (It's just changing from having src and offset updated at the end of the loop to dropping src and having offset be updated in the for()).
Quoted 18 lines
hw/display/sm501.c +0
-10/* complete flush to display */ @@ -1419,15 +1429,6 @@ static void sm501_draw_crt(SM501State *s) } } -static void sm501_update_display (void *opaque) -{ -SM501State *s = (SM501State *)opaque; - -if (s->dc_crt_control &SM501_DC_CRT_CONTROL_ENABLE ) { -sm501_draw_crt (s); - } -} -static const GraphicHwOps sm501_ops = { .gfx_update = sm501_update_display, }; --
Quoted 3 lines
2.7.4
thanks
-- PMM
Re: [Qemu-devel] [PATCH v2 10/14] sm501: Add support for panel layer
Quoted 123 lines · Peter Maydell
Quoted 122 lines · BALATON Zoltan
Signed-off-by BALATON Zoltan <[email protected]>---
v2: Split off renaming a variable to separate clean up patch
hw/display/sm501.c | 63 +++++++++++++++++++++++++++--------------------------- 1 file changed, 32 insertions(+), 31 deletions(-)hw/display/sm501.c +32
-19@@ -2,6 +2,7 @@* QEMU SM501 Device * * Copyright (c) 2008 Shin-ichiro KAWASAKI +* Copyright (c) 2016 BALATON Zoltan * * Permission is hereby granted, free of charge, to any person obtaining a copy * of this software and associated documentation files (the "Software"), to deal @@ -41,8 +42,11 @@* - Minimum implementation for Linux console : mmio regs and CRT layer. * - 2D graphics acceleration partially supported : only fill rectangle. * -* TODO: +* Status: 2016/12/04 +* - Misc fixes: endianness, hardware cursor * - Panel support +* +* TODO: * - Touch panel support * - USB support * - UART support @@ -1300,18 +1304,16 @@ static inline int get_depth_index(DisplaySurface *surface) } } -static void sm501_draw_crt (SM501State *s) +static void sm501_update_display (void *opaque) { +SM501State *s = (SM501State *)opaque;DisplaySurface *surface =qemu_console_surface (s->con);int y, c_x, c_y; -uint8_t *hwc_src, *src = s->local_mem; -int width =get_width (s,1 ); -int height =get_height (s,1 ); -int src_bpp =get_bpp (s,1 ); +int crt = (s->dc_crt_control &SM501_DC_CRT_CONTROL_SEL ) ?1 :0 ; +int width =get_width (s, crt); +int height =get_height (s, crt); +int src_bpp =get_bpp (s, crt);int dst_bpp =surface_bytes_per_pixel (surface); -uint32_t *palette = (uint32_t *)&s->dc_palette[SM501_DC_CRT_PALETTE - -SM501_DC_PANEL_PALETTE ]; -uint8_t hwc_palette[3 *3 ];int dst_depth_index =get_depth_index (surface);draw_line_func *draw_line =NULL ;draw_hwc_line_func *draw_hwc_line =NULL ;@@ -1319,7 +1321,19 @@ static void sm501_draw_crt(SM501State *s)int y_start =-1 ;ram_addr_t page_min = ~0l ;ram_addr_t page_max =0l ; -ram_addr_t offset =0 ; +ram_addr_t offset; +uint32_t *palette; +uint8_t hwc_palette[3 *3 ]; +uint8_t *hwc_src; + +if (!((crt ? s->dc_crt_control : s->dc_panel_control) + &SM501_DC_CRT_CONTROL_ENABLE )) { +return ; + } + + palette = (uint32_t *)(crt ? &s->dc_palette[SM501_DC_CRT_PALETTE - +SM501_DC_PANEL_PALETTE ] + : &s->dc_palette[0 ]);/* choose draw_line function */ switch (src_bpp) {@@ -1333,20 +1347,19 @@ static void sm501_draw_crt(SM501State *s) draw_line = draw_line32_funcs[dst_depth_index];break ;default : -printf ("sm501 draw crt : invalid DC_CRT_CONTROL=%x.\n" , - s->dc_crt_control); +printf ("sm501 update display : invalid control register value.\n" );abort ();break ; }/* set up to draw hardware cursor */ -if (is_hwc_enabled (s,1 )) { +if (is_hwc_enabled (s, crt)) {/* choose cursor draw line function */ draw_hwc_line = draw_hwc_line_funcs[dst_depth_index]; - hwc_src =get_hwc_address (s,1 ); - c_x =get_hwc_x (s,1 ); - c_y =get_hwc_y (s,1 ); -get_hwc_palette (s,1 , hwc_palette); + hwc_src =get_hwc_address (s, crt); + c_x =get_hwc_x (s, crt); + c_y =get_hwc_y (s, crt); +get_hwc_palette (s, crt, hwc_palette); }/* adjust console size */ @@ -1360,7 +1373,7 @@ static void sm501_draw_crt(SM501State *s)/* draw each line according to conditions */ memory_region_sync_dirty_bitmap (&s->local_mem_region); -for (y =0 ; y < height; y++) { +for (y =0 , offset =0 ; y < height; y++, offset += width * src_bpp) {int update, update_hwc;ram_addr_t page0 = offset;ram_addr_t page1 = offset + width * src_bpp -1 ;@@ -1378,7 +1391,7 @@ static void sm501_draw_crt(SM501State *s) d += y * width * dst_bpp;/* draw graphics layer */ -draw_line (d, src, width, palette); +draw_line (d, s->local_mem + offset, width, palette);
Quoted 15 lines
hw/display/sm501.c static void sm501_draw_crt(SM501State *s) · line 1417
Quoted 10 lines
hw/display/sm501.c +0
-3/* draw hardware cursor */ if (update_hwc) {@@ -1401,9 +1414,6 @@ static void sm501_draw_crt(SM501State *s) y_start =-1 ; } } - - src += width * src_bpp; - offset += width * src_bpp; }This is another refactoring that's unrelated to adding panel layer support, right? (It's just changing from having src and offset updated at the end of the loop to dropping src and having offset be updated in the
for()).
It's a clean up getting rid of the unneded src pointer. Should I move it to the cleanup patch or is it OK to leave it here?
Quoted 28 lines
Quoted 18 lines
hw/display/sm501.c +0
-10/* complete flush to display */ @@ -1419,15 +1429,6 @@ static void sm501_draw_crt(SM501State *s) } } -static void sm501_update_display (void *opaque) -{ -SM501State *s = (SM501State *)opaque; - -if (s->dc_crt_control &SM501_DC_CRT_CONTROL_ENABLE ) { -sm501_draw_crt (s); - } -} -static const GraphicHwOps sm501_ops = { .gfx_update = sm501_update_display, }; --Quoted 3 lines
2.7.4
Reviewed-by Peter Maydell <[email protected]>thanks
-- PMM
Re: [Qemu-devel] [PATCH v2 10/14] sm501: Add support for panel layer
Quoted 10 lines · BALATON Zoltan
Quoted 5 lines · Peter Maydell
This is another refactoring that's unrelated to adding panel layer support, right? (It's just changing from having src and offset updated at the end of the loop to dropping src and having offset be updated in the
for()).It's a clean up getting rid of the unneded src pointer. Should I move it to the cleanup patch or is it OK to leave it here?
I've reviewed it now, so you can leave it. It would have been better in its own patch, though. (The general principle is that each patch should do only one thing, and it should do all of that one thing, and the one thing should be fairly small and easy to understand.)
thanks
-- PMM
PATCH v2 11/14 sm501: Add some more missing registers
In reply to BALATON ZoltanValues are not used yet, only stored to allow clients to initialise these without failing as long as no 2D engine function is called that would use the written value.
---
v2: Fixed mask of video_control register for a read only bit
Changed IRQ status register to write ignored as IRQ is not implemented
hw/display/sm501.c | 49 ++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 48 insertions(+), 1 deletion(-)
hw/display/sm501.c +48 -1
@@ -511,6 +511,8 @@ typedef struct SM501State {uint32_t dc_panel_hwc_color_1_2;uint32_t dc_panel_hwc_color_3; +uint32_t dc_video_control; +uint32_t dc_crt_control;uint32_t dc_crt_fb_addr;uint32_t dc_crt_fb_offset;
@@ -530,13 +532,20 @@ typedef struct SM501State {uint32_t twoD_control;uint32_t twoD_pitch;uint32_t twoD_foreground; +uint32_t twoD_background;uint32_t twoD_stretch; +uint32_t twoD_color_compare;uint32_t twoD_color_compare_mask;uint32_t twoD_mask; +uint32_t twoD_clip_tl; +uint32_t twoD_clip_br; +uint32_t twoD_mono_pattern_low; +uint32_t twoD_mono_pattern_high;uint32_t twoD_window_width;uint32_t twoD_source_base;uint32_t twoD_destination_base; - +uint32_t twoD_alpha; +uint32_t twoD_wrap; } SM501State;static uint32_t get_local_mem_size_index (uint32_t size)
@@ -945,6 +954,10 @@ static uint64_t sm501_disp_ctrl_read(void *opaque, hwaddr addr, ret = s->dc_panel_v_sync;break ; +case SM501_DC_VIDEO_CONTROL : + ret = s->dc_video_control; +break ; +case SM501_DC_CRT_CONTROL : ret = s->dc_crt_control;break ;
@@ -1060,6 +1073,10 @@ static void sm501_disp_ctrl_write(void *opaque, hwaddr addr, s->dc_panel_hwc_color_3 = value &0x0000FFFF ;break ; +case SM501_DC_VIDEO_CONTROL : + s->dc_video_control = value &0x00037FFF ; +break ; +case SM501_DC_CRT_CONTROL : s->dc_crt_control = value &0x0003FFFF ;break ;
@@ -1135,6 +1152,9 @@ static uint64_t sm501_2d_engine_read(void *opaque, hwaddr addr,case SM501_2D_SOURCE_BASE : ret = s->twoD_source_base;break ; +case SM501_2D_STATUS : + ret =0 ;/* Should return interrupt status */ +break ;default :printf ("sm501 disp ctrl : not implemented register read." " addr=%x\n" , (int )addr);
@@ -1177,15 +1197,33 @@ static void sm501_2d_engine_write(void *opaque, hwaddr addr,case SM501_2D_FOREGROUND : s->twoD_foreground = value;break ; +case SM501_2D_BACKGROUND : + s->twoD_background = value; +break ;case SM501_2D_STRETCH : s->twoD_stretch = value;break ; +case SM501_2D_COLOR_COMPARE : + s->twoD_color_compare = value; +break ;case SM501_2D_COLOR_COMPARE_MASK : s->twoD_color_compare_mask = value;break ;case SM501_2D_MASK : s->twoD_mask = value;break ; +case SM501_2D_CLIP_TL : + s->twoD_clip_tl = value; +break ; +case SM501_2D_CLIP_BR : + s->twoD_clip_br = value; +break ; +case SM501_2D_MONO_PATTERN_LOW : + s->twoD_mono_pattern_low = value; +break ; +case SM501_2D_MONO_PATTERN_HIGH : + s->twoD_mono_pattern_high = value; +break ;case SM501_2D_WINDOW_WIDTH : s->twoD_window_width = value;break ;
@@ -1195,6 +1233,15 @@ static void sm501_2d_engine_write(void *opaque, hwaddr addr,case SM501_2D_DESTINATION_BASE : s->twoD_destination_base = value;break ; +case SM501_2D_ALPHA : + s->twoD_alpha = value; +break ; +case SM501_2D_WRAP : + s->twoD_wrap = value; +break ; +case SM501_2D_STATUS : +/* ignored, writing 0 should clear interrupt status */ +break ;default :printf ("sm501 2d engine : not implemented register write." " addr=%x, val=%x\n" , (int )addr, (unsigned )value);
Signature · 1 line
2.7.4
Re: [Qemu-devel] [PATCH v2 11/14] sm501: Add some more missing registers
Quoted 36 lines · BALATON Zoltan
Values are not used yet, only stored to allow clients to initialise these without failing as long as no 2D engine function is called that would use the written value.
Signed-off-by BALATON Zoltan <[email protected]>---
v2: Fixed mask of
video_controlregister for a read only bit
Changed IRQ status register to write ignored as IRQ is not implementedhw/display/sm501.c | 49 ++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 48 insertions(+), 1 deletion(-)hw/display/sm501.c +5
-0@@ -511,6 +511,8 @@ typedef struct SM501State {uint32_t dc_panel_hwc_color_1_2;uint32_t dc_panel_hwc_color_3; +uint32_t dc_video_control; +uint32_t dc_crt_control;uint32_t dc_crt_fb_addr;uint32_t dc_crt_fb_offset;@@ -530,13 +532,20 @@ typedef struct SM501State {uint32_t twoD_control;uint32_t twoD_pitch;uint32_t twoD_foreground; +uint32_t twoD_background;uint32_t twoD_stretch; +uint32_t twoD_color_compare;uint32_t twoD_color_compare_mask;uint32_t twoD_mask; +uint32_t twoD_clip_tl;
hw/display/sm501.c typedef struct SM501State { · line 549
Quoted 10 lines
hw/display/sm501.c +5
-1+uint32_t twoD_clip_br; +uint32_t twoD_mono_pattern_low; +uint32_t twoD_mono_pattern_high;uint32_t twoD_window_width;uint32_t twoD_source_base;uint32_t twoD_destination_base; - +uint32_t twoD_alpha; +uint32_t twoD_wrap; } SM501State;
You're still implementing almost all of these as writes to structure fields which can never be read. Either of these:
(1) a write function case which writes to the struct field, plus a read function case which returns the value in the struct field
(2) a write function which does nothing (and no read function case, or a read function case which returns 0)
[you can usually bunch these up into a lot of case FOO: which share a /* unimplemented, ignore writes */ body]
are OK, but a write function which writes to the struct field but nothing ever reading that field doesn't make sense.
(You can also LOG_UNIMP for dummy stuff like this if you like, but that's not something we insist on.)
thanks
-- PMM
Re: [Qemu-devel] [PATCH v2 11/14] sm501: Add some more missing registers
Quoted 58 lines · Peter Maydell
Quoted 36 lines · BALATON Zoltan
Values are not used yet, only stored to allow clients to initialise these without failing as long as no 2D engine function is called that would use the written value.
Signed-off-by BALATON Zoltan <[email protected]>---
v2: Fixed mask of
video_controlregister for a read only bit
Changed IRQ status register to write ignored as IRQ is not implementedhw/display/sm501.c | 49 ++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 48 insertions(+), 1 deletion(-)hw/display/sm501.c +5
-0@@ -511,6 +511,8 @@ typedef struct SM501State {uint32_t dc_panel_hwc_color_1_2;uint32_t dc_panel_hwc_color_3; +uint32_t dc_video_control; +uint32_t dc_crt_control;uint32_t dc_crt_fb_addr;uint32_t dc_crt_fb_offset;@@ -530,13 +532,20 @@ typedef struct SM501State {uint32_t twoD_control;uint32_t twoD_pitch;uint32_t twoD_foreground; +uint32_t twoD_background;uint32_t twoD_stretch; +uint32_t twoD_color_compare;uint32_t twoD_color_compare_mask;uint32_t twoD_mask; +uint32_t twoD_clip_tl;hw/display/sm501.c typedef struct SM501State { · line 549
Quoted 10 lines
hw/display/sm501.c +5
-1+uint32_t twoD_clip_br; +uint32_t twoD_mono_pattern_low; +uint32_t twoD_mono_pattern_high;uint32_t twoD_window_width;uint32_t twoD_source_base;uint32_t twoD_destination_base; - +uint32_t twoD_alpha; +uint32_t twoD_wrap; } SM501State;You're still implementing almost all of these as writes to structure fields which can never be read. Either of these:
(1) a write function case which writes to the struct field, plus a read function case which returns the value in the struct field
(2) a write function which does nothing (and no read function case, or a read function case which returns 0)
[you can usually bunch these up into a lot of case FOO: which share a /* unimplemented, ignore writes */ body]
Quoted 5 lines
are OK, but a write function which writes to the struct field but nothing ever reading that field doesn't make sense.
(You can also
LOG_UNIMPfor dummy stuff like this if you like, but that's not something we insist on.)
I've stored these in the state because these will be needed if we ever implement more of the 2D engine even if they are not used yet. (Also as you've noticed read is also imlemented.)
Quoted 5 lines
thanks
-- PMM
PATCH v2 12/14 sm501: Implement reading 2D engine registers
In reply to BALATON ZoltanClients normally only write to these registers, nothing is known to ever read them but they are documented as read/write so allow clients to also read the values.
--- hw/display/sm501.c | 57 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 57 insertions(+)
hw/display/sm501.c +57 -0
@@ -1149,9 +1149,66 @@ static uint64_t sm501_2d_engine_read(void *opaque, hwaddr addr,SM501_DPRINTF ("sm501 2d engine regs : read addr=%x\n" , (int )addr);switch (addr) { +case SM501_2D_SOURCE : + ret = s->twoD_source; +break ; +case SM501_2D_DESTINATION : + ret = s->twoD_destination; +break ; +case SM501_2D_DIMENSION : + ret = s->twoD_dimension; +break ; +case SM501_2D_CONTROL : + ret = s->twoD_control; +break ; +case SM501_2D_PITCH : + ret = s->twoD_pitch; +break ; +case SM501_2D_FOREGROUND : + ret = s->twoD_foreground; +break ; +case SM501_2D_BACKGROUND : + ret = s->twoD_background; +break ; +case SM501_2D_STRETCH : + ret = s->twoD_stretch; +break ; +case SM501_2D_COLOR_COMPARE : + ret = s->twoD_color_compare; +break ; +case SM501_2D_COLOR_COMPARE_MASK : + ret = s->twoD_color_compare_mask; +break ; +case SM501_2D_MASK : + ret = s->twoD_mask; +break ; +case SM501_2D_CLIP_TL : + ret = s->twoD_clip_tl; +break ; +case SM501_2D_CLIP_BR : + ret = s->twoD_clip_br; +break ; +case SM501_2D_MONO_PATTERN_LOW : + ret = s->twoD_mono_pattern_low; +break ; +case SM501_2D_MONO_PATTERN_HIGH : + ret = s->twoD_mono_pattern_high; +break ; +case SM501_2D_WINDOW_WIDTH : + ret = s->twoD_window_width; +break ;case SM501_2D_SOURCE_BASE : ret = s->twoD_source_base;break ; +case SM501_2D_DESTINATION_BASE : + ret = s->twoD_destination_base; +break ; +case SM501_2D_ALPHA : + ret = s->twoD_alpha; +break ; +case SM501_2D_WRAP : + ret = s->twoD_wrap; +break ;case SM501_2D_STATUS : ret =0 ;/* Should return interrupt status */ break ;
Signature · 1 line
2.7.4
Re: [Qemu-devel] [PATCH v2 12/14] sm501: Implement reading 2D engine registers
Quoted 8 lines · BALATON Zoltan
Clients normally only write to these registers, nothing is known to ever read them but they are documented as read/write so allow clients to also read the values.
Signed-off-by BALATON Zoltan <[email protected]>--- hw/display/sm501.c | 57 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 57 insertions(+)
Oh, the read code is in a separate patch? Please put all the handling for a particular register in the same patch. If you think the resulting patch is too long you can split it into several patches each of which handles a group of registers if you like (but it probably isn't necessary).
thanks
-- PMM
Re: [Qemu-devel] [PATCH v2 12/14] sm501: Implement reading 2D engine registers
Quoted 10 lines · Peter Maydell
Quoted 8 lines · BALATON Zoltan
Clients normally only write to these registers, nothing is known to ever read them but they are documented as read/write so allow clients to also read the values.
Signed-off-by BALATON Zoltan <[email protected]>--- hw/display/sm501.c | 57 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 57 insertions(+)
Quoted 5 lines
Oh, the read code is in a separate patch? Please put all the handling for a particular register in the same patch. If you think the resulting patch is too long you can split it into several patches each of which handles a group of registers if you like (but it probably isn't necessary).
So is it OK to squash this in the previous patch adding write and make the two a single patch?
Quoted 5 lines
thanks
-- PMM
Re: [Qemu-devel] [PATCH v2 12/14] sm501: Implement reading 2D engine registers
Quoted 22 lines · BALATON Zoltan
Quoted 14 lines · Peter Maydell
Quoted 10 lines · BALATON Zoltan
Clients normally only write to these registers, nothing is known to ever read them but they are documented as read/write so allow clients to also read the values.
Signed-off-by BALATON Zoltan <[email protected]>--- hw/display/sm501.c | 57 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 57 insertions(+)Quoted 5 lines
Oh, the read code is in a separate patch? Please put all the handling for a particular register in the same patch. If you think the resulting patch is too long you can split it into several patches each of which handles a group of registers if you like (but it probably isn't necessary).
Quoted 2 lines
So is it OK to squash this in the previous patch adding write and make the two a single patch?
Yes please.
thanks
-- PMM
PATCH v2 13/14 sm501: Add reset function and vmstate descriptor
In reply to BALATON Zoltan--- hw/display/sm501.c | 110 ++++++++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 105 insertions(+), 5 deletions(-)
hw/display/sm501.c +105 -5
@@ -65,6 +65,7 @@#define MMIO_BASE_OFFSET 0x3e00000#define MMIO_SIZE 0x200000 +#define DC_PALETTE_ENTRIES (0x400 * 3)/* SM501 register definitions taken from "linux/include/linux/sm501-regs.h" */
@@ -491,7 +492,7 @@ typedef struct SM501State {uint32_t uart0_mcr;uint32_t uart0_scr; -uint8_t dc_palette[0x400 *3 ]; +uint8_t dc_palette[DC_PALETTE_ENTRIES ];uint32_t dc_panel_control;uint32_t dc_panel_panning_control;
@@ -1537,16 +1538,32 @@ static const GraphicHwOps sm501_ops = { .gfx_update = sm501_update_display, }; +static void sm501_reset (void *p) +{ +SM501State *s = p; + + s->system_control =0x00100000 ;/* 2D engine FIFO empty */ + s->misc_control =SM501_MISC_IRQ_INVERT ;/* assumes SH, active=low */ + s->gpio_31_0_control =0 ; + s->gpio_63_32_control =0 ; + s->dram_control =0 ; + s->arbitration_control =0x05146732 ; + s->irq_mask =0 ; + s->misc_timing =0 ; + s->power_mode_control =0 ; + s->dc_panel_control =0x00010000 ;/* FIFO level 3 */ + s->dc_video_control =0 ; + s->dc_crt_control =0x00010000 ; + s->twoD_control =0 ; +} +static void sm501_init (SM501State *s,DeviceState *dev,uint32_t local_mem_bytes) { s->local_mem_size_index =get_local_mem_size_index (local_mem_bytes);SM501_DPRINTF ("sm501 local mem size=%x. index=%d\n" ,get_local_mem_size (s), s->local_mem_size_index); - s->system_control =0x00100000 ;/* 2D engine FIFO empty */ - s->misc_control =SM501_MISC_IRQ_INVERT ;/* assumes SH, active=low */ - s->dc_panel_control =0x00010000 ;/* FIFO level 3 */ - s->dc_crt_control =0x00010000 ; +qemu_register_reset (sm501_reset, s);/* local memory */ memory_region_init_ram(&s->local_mem_region, OBJECT(dev), "sm501.local",
@@ -1577,6 +1594,77 @@ static void sm501_init(SM501State *s, DeviceState *dev, s->con =graphic_console_init (DEVICE (dev),0 , &sm501_ops, s); } +static const VMStateDescription vmstate_sm501_regs = { + .name ="sm501-regs" , + .version_id =1 , + .minimum_version_id =1 , + .fields = (VMStateField []) { +VMSTATE_UINT32 (system_control, SM501State), +VMSTATE_UINT32 (misc_control, SM501State), +VMSTATE_UINT32 (gpio_31_0_control, SM501State), +VMSTATE_UINT32 (gpio_63_32_control, SM501State), +VMSTATE_UINT32 (dram_control, SM501State), +VMSTATE_UINT32 (arbitration_control, SM501State), +VMSTATE_UINT32 (irq_mask, SM501State), +VMSTATE_UINT32 (misc_timing, SM501State), +VMSTATE_UINT32 (power_mode_control, SM501State), +VMSTATE_UINT32 (uart0_ier, SM501State), +VMSTATE_UINT32 (uart0_lcr, SM501State), +VMSTATE_UINT32 (uart0_mcr, SM501State), +VMSTATE_UINT32 (uart0_scr, SM501State), +VMSTATE_UINT8_ARRAY (dc_palette, SM501State,DC_PALETTE_ENTRIES ), +VMSTATE_UINT32 (dc_panel_control, SM501State), +VMSTATE_UINT32 (dc_panel_panning_control, SM501State), +VMSTATE_UINT32 (dc_panel_fb_addr, SM501State), +VMSTATE_UINT32 (dc_panel_fb_offset, SM501State), +VMSTATE_UINT32 (dc_panel_fb_width, SM501State), +VMSTATE_UINT32 (dc_panel_fb_height, SM501State), +VMSTATE_UINT32 (dc_panel_tl_location, SM501State), +VMSTATE_UINT32 (dc_panel_br_location, SM501State), +VMSTATE_UINT32 (dc_panel_h_total, SM501State), +VMSTATE_UINT32 (dc_panel_h_sync, SM501State), +VMSTATE_UINT32 (dc_panel_v_total, SM501State), +VMSTATE_UINT32 (dc_panel_v_sync, SM501State), +VMSTATE_UINT32 (dc_panel_hwc_addr, SM501State), +VMSTATE_UINT32 (dc_panel_hwc_location, SM501State), +VMSTATE_UINT32 (dc_panel_hwc_color_1_2, SM501State), +VMSTATE_UINT32 (dc_panel_hwc_color_3, SM501State), +VMSTATE_UINT32 (dc_video_control, SM501State), +VMSTATE_UINT32 (dc_crt_control, SM501State), +VMSTATE_UINT32 (dc_crt_fb_addr, SM501State), +VMSTATE_UINT32 (dc_crt_fb_offset, SM501State), +VMSTATE_UINT32 (dc_crt_h_total, SM501State), +VMSTATE_UINT32 (dc_crt_h_sync, SM501State), +VMSTATE_UINT32 (dc_crt_v_total, SM501State), +VMSTATE_UINT32 (dc_crt_v_sync, SM501State), +VMSTATE_UINT32 (dc_crt_hwc_addr, SM501State), +VMSTATE_UINT32 (dc_crt_hwc_location, SM501State), +VMSTATE_UINT32 (dc_crt_hwc_color_1_2, SM501State), +VMSTATE_UINT32 (dc_crt_hwc_color_3, SM501State), +VMSTATE_UINT32 (twoD_source, SM501State), +VMSTATE_UINT32 (twoD_destination, SM501State), +VMSTATE_UINT32 (twoD_dimension, SM501State), +VMSTATE_UINT32 (twoD_control, SM501State), +VMSTATE_UINT32 (twoD_pitch, SM501State), +VMSTATE_UINT32 (twoD_foreground, SM501State), +VMSTATE_UINT32 (twoD_background, SM501State), +VMSTATE_UINT32 (twoD_stretch, SM501State), +VMSTATE_UINT32 (twoD_color_compare, SM501State), +VMSTATE_UINT32 (twoD_color_compare_mask, SM501State), +VMSTATE_UINT32 (twoD_mask, SM501State), +VMSTATE_UINT32 (twoD_clip_tl, SM501State), +VMSTATE_UINT32 (twoD_clip_br, SM501State), +VMSTATE_UINT32 (twoD_mono_pattern_low, SM501State), +VMSTATE_UINT32 (twoD_mono_pattern_high, SM501State), +VMSTATE_UINT32 (twoD_window_width, SM501State), +VMSTATE_UINT32 (twoD_source_base, SM501State), +VMSTATE_UINT32 (twoD_destination_base, SM501State), +VMSTATE_UINT32 (twoD_alpha, SM501State), +VMSTATE_UINT32 (twoD_wrap, SM501State), +VMSTATE_END_OF_LIST () + } +}; +#define TYPE_SYSBUS_SM501 "sysbus-sm501"#define SYSBUS_SM501 (obj) \ OBJECT_CHECK(SM501SysBusState, (obj), TYPE_SYSBUS_SM501)
@@ -1673,6 +1761,17 @@ static Property sm501_pci_properties[] = { DEFINE_PROP_END_OF_LIST(), }; +static const VMStateDescription vmstate_sm501 = { + .name ="sm501" , + .version_id =1 , + .minimum_version_id =1 , + .fields = (VMStateField []) { +VMSTATE_PCI_DEVICE (parent_obj, SM501PCIState), +VMSTATE_STRUCT (state, SM501PCIState,1 , vmstate_sm501_regs, SM501State), +VMSTATE_END_OF_LIST () + } +}; +static void sm501_pci_class_init (ObjectClass *klass,void *data) {DeviceClass *dc =DEVICE_CLASS (klass);
@@ -1686,6 +1785,7 @@ static void sm501_pci_class_init(ObjectClass *klass, void *data) dc->desc ="SM501 Display Controller" ; dc->props = sm501_pci_properties; dc->hotpluggable = false; + dc->vmsd = &vmstate_sm501; }static const TypeInfo sm501_pci_info = {
Signature · 1 line
2.7.4
Re: [Qemu-devel] [PATCH v2 13/14] sm501: Add reset function and vmstate descriptor
Quoted 27 lines · BALATON Zoltan
Signed-off-by BALATON Zoltan <[email protected]>--- hw/display/sm501.c | 110 ++++++++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 105 insertions(+), 5 deletions(-)hw/display/sm501.c +2
-1@@ -65,6 +65,7 @@#define MMIO_BASE_OFFSET 0x3e00000#define MMIO_SIZE 0x200000 +#define DC_PALETTE_ENTRIES (0x400 * 3)/* SM501 register definitions taken from "linux/include/linux/sm501-regs.h" */ @@ -491,7 +492,7 @@ typedef struct SM501State {uint32_t uart0_mcr;uint32_t uart0_scr; -uint8_t dc_palette[0x400 *3 ]; +uint8_t dc_palette[DC_PALETTE_ENTRIES ];uint32_t dc_panel_control;uint32_t dc_panel_panning_control;
hw/display/sm501.c static const GraphicHwOps sm501_ops = { · line 1546
Quoted 9 lines
hw/display/sm501.c +6
-0@@ -1537,16 +1538,32 @@ static const GraphicHwOps sm501_ops = { .gfx_update = sm501_update_display, }; +static void sm501_reset (void *p) +{ +SM501State *s = p; + + s->system_control =0x00100000 ;/* 2D engine FIFO empty */ + s->misc_control =SM501_MISC_IRQ_INVERT ;/* assumes SH, active=low */
Don't forget to update this patch when you fix this reset value in patch 1.
Quoted 14 lines
hw/display/sm501.c +13
-0+ s->gpio_31_0_control =0 ; + s->gpio_63_32_control =0 ; + s->dram_control =0 ; + s->arbitration_control =0x05146732 ; + s->irq_mask =0 ; + s->misc_timing =0 ; + s->power_mode_control =0 ; + s->dc_panel_control =0x00010000 ;/* FIFO level 3 */ + s->dc_video_control =0 ; + s->dc_crt_control =0x00010000 ; + s->twoD_control =0 ; +} + static void sm501_init(SM501State *s, DeviceState *dev,
hw/display/sm501.c static const GraphicHwOps sm501_ops = { · line 1566
Quoted 10 lines
hw/display/sm501.c +1
-4uint32_t local_mem_bytes) { s->local_mem_size_index =get_local_mem_size_index (local_mem_bytes);SM501_DPRINTF ("sm501 local mem size=%x. index=%d\n" ,get_local_mem_size (s), s->local_mem_size_index); - s->system_control =0x00100000 ;/* 2D engine FIFO empty */ - s->misc_control =SM501_MISC_IRQ_INVERT ;/* assumes SH, active=low */ - s->dc_panel_control =0x00010000 ;/* FIFO level 3 */ - s->dc_crt_control =0x00010000 ; +qemu_register_reset (sm501_reset, s);
Don't use qemu_register_reset(). Set the appropriate dc->reset function pointers instead.
Quoted 67 lines
hw/display/sm501.c +60
-0/* local memory */ memory_region_init_ram(&s->local_mem_region, OBJECT(dev), "sm501.local",@@ -1577,6 +1594,77 @@ static void sm501_init(SM501State *s, DeviceState *dev, s->con =graphic_console_init (DEVICE (dev),0 , &sm501_ops, s); } +static const VMStateDescription vmstate_sm501_regs = { + .name ="sm501-regs" , + .version_id =1 , + .minimum_version_id =1 , + .fields = (VMStateField []) { +VMSTATE_UINT32 (system_control, SM501State), +VMSTATE_UINT32 (misc_control, SM501State), +VMSTATE_UINT32 (gpio_31_0_control, SM501State), +VMSTATE_UINT32 (gpio_63_32_control, SM501State), +VMSTATE_UINT32 (dram_control, SM501State), +VMSTATE_UINT32 (arbitration_control, SM501State), +VMSTATE_UINT32 (irq_mask, SM501State), +VMSTATE_UINT32 (misc_timing, SM501State), +VMSTATE_UINT32 (power_mode_control, SM501State), +VMSTATE_UINT32 (uart0_ier, SM501State), +VMSTATE_UINT32 (uart0_lcr, SM501State), +VMSTATE_UINT32 (uart0_mcr, SM501State), +VMSTATE_UINT32 (uart0_scr, SM501State), +VMSTATE_UINT8_ARRAY (dc_palette, SM501State,DC_PALETTE_ENTRIES ), +VMSTATE_UINT32 (dc_panel_control, SM501State), +VMSTATE_UINT32 (dc_panel_panning_control, SM501State), +VMSTATE_UINT32 (dc_panel_fb_addr, SM501State), +VMSTATE_UINT32 (dc_panel_fb_offset, SM501State), +VMSTATE_UINT32 (dc_panel_fb_width, SM501State), +VMSTATE_UINT32 (dc_panel_fb_height, SM501State), +VMSTATE_UINT32 (dc_panel_tl_location, SM501State), +VMSTATE_UINT32 (dc_panel_br_location, SM501State), +VMSTATE_UINT32 (dc_panel_h_total, SM501State), +VMSTATE_UINT32 (dc_panel_h_sync, SM501State), +VMSTATE_UINT32 (dc_panel_v_total, SM501State), +VMSTATE_UINT32 (dc_panel_v_sync, SM501State), +VMSTATE_UINT32 (dc_panel_hwc_addr, SM501State), +VMSTATE_UINT32 (dc_panel_hwc_location, SM501State), +VMSTATE_UINT32 (dc_panel_hwc_color_1_2, SM501State), +VMSTATE_UINT32 (dc_panel_hwc_color_3, SM501State), +VMSTATE_UINT32 (dc_video_control, SM501State), +VMSTATE_UINT32 (dc_crt_control, SM501State), +VMSTATE_UINT32 (dc_crt_fb_addr, SM501State), +VMSTATE_UINT32 (dc_crt_fb_offset, SM501State), +VMSTATE_UINT32 (dc_crt_h_total, SM501State), +VMSTATE_UINT32 (dc_crt_h_sync, SM501State), +VMSTATE_UINT32 (dc_crt_v_total, SM501State), +VMSTATE_UINT32 (dc_crt_v_sync, SM501State), +VMSTATE_UINT32 (dc_crt_hwc_addr, SM501State), +VMSTATE_UINT32 (dc_crt_hwc_location, SM501State), +VMSTATE_UINT32 (dc_crt_hwc_color_1_2, SM501State), +VMSTATE_UINT32 (dc_crt_hwc_color_3, SM501State), +VMSTATE_UINT32 (twoD_source, SM501State), +VMSTATE_UINT32 (twoD_destination, SM501State), +VMSTATE_UINT32 (twoD_dimension, SM501State), +VMSTATE_UINT32 (twoD_control, SM501State), +VMSTATE_UINT32 (twoD_pitch, SM501State), +VMSTATE_UINT32 (twoD_foreground, SM501State), +VMSTATE_UINT32 (twoD_background, SM501State), +VMSTATE_UINT32 (twoD_stretch, SM501State), +VMSTATE_UINT32 (twoD_color_compare, SM501State), +VMSTATE_UINT32 (twoD_color_compare_mask, SM501State), +VMSTATE_UINT32 (twoD_mask, SM501State), +VMSTATE_UINT32 (twoD_clip_tl, SM501State), +VMSTATE_UINT32 (twoD_clip_br, SM501State),
hw/display/sm501.c static void sm501_init(SM501State *s, DeviceState *dev, · line 1666
Quoted 10 lines
hw/display/sm501.c +10
-0+VMSTATE_UINT32 (twoD_mono_pattern_low, SM501State), +VMSTATE_UINT32 (twoD_mono_pattern_high, SM501State), +VMSTATE_UINT32 (twoD_window_width, SM501State), +VMSTATE_UINT32 (twoD_source_base, SM501State), +VMSTATE_UINT32 (twoD_destination_base, SM501State), +VMSTATE_UINT32 (twoD_alpha, SM501State), +VMSTATE_UINT32 (twoD_wrap, SM501State), + VMSTATE_END_OF_LIST() + } +};
local_mem_size_index is also guest updatable state (it's some of the bits from the SM501_DRAM_CONTROL register), so we need to migrate it as well.
Quoted 31 lines
hw/display/sm501.c +13
-1+#define TYPE_SYSBUS_SM501 "sysbus-sm501"#define SYSBUS_SM501 (obj) \ OBJECT_CHECK(SM501SysBusState, (obj), TYPE_SYSBUS_SM501)@@ -1673,6 +1761,17 @@ static Property sm501_pci_properties[] = { DEFINE_PROP_END_OF_LIST(), }; +static const VMStateDescription vmstate_sm501 = { + .name ="sm501" , + .version_id =1 , + .minimum_version_id =1 , + .fields = (VMStateField []) { +VMSTATE_PCI_DEVICE (parent_obj, SM501PCIState), +VMSTATE_STRUCT (state, SM501PCIState,1 , vmstate_sm501_regs, SM501State), +VMSTATE_END_OF_LIST () + } +}; +static void sm501_pci_class_init (ObjectClass *klass,void *data) {DeviceClass *dc =DEVICE_CLASS (klass);@@ -1686,6 +1785,7 @@ static void sm501_pci_class_init(ObjectClass *klass, void *data) dc->desc ="SM501 Display Controller" ; dc->props = sm501_pci_properties; dc->hotpluggable = false; + dc->vmsd = &vmstate_sm501; }static const TypeInfo sm501_pci_info = { --
Quoted 1 line
2.7.4
What about reset and vmstate for the sysbus device?
Consider putting reset and vmstate in separate patches.
thanks
-- PMM
Re: [Qemu-devel] [PATCH v2 13/14] sm501: Add reset function and vmstate descriptor
Quoted 59 lines · Peter Maydell
Quoted 27 lines · BALATON Zoltan
Signed-off-by BALATON Zoltan <[email protected]>--- hw/display/sm501.c | 110 ++++++++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 105 insertions(+), 5 deletions(-)hw/display/sm501.c +2
-1@@ -65,6 +65,7 @@#define MMIO_BASE_OFFSET 0x3e00000#define MMIO_SIZE 0x200000 +#define DC_PALETTE_ENTRIES (0x400 * 3)/* SM501 register definitions taken from "linux/include/linux/sm501-regs.h" */ @@ -491,7 +492,7 @@ typedef struct SM501State {uint32_t uart0_mcr;uint32_t uart0_scr; -uint8_t dc_palette[0x400 *3 ]; +uint8_t dc_palette[DC_PALETTE_ENTRIES ];uint32_t dc_panel_control;uint32_t dc_panel_panning_control;hw/display/sm501.c static const GraphicHwOps sm501_ops = { · line 1546
Quoted 9 lines
hw/display/sm501.c +6
-0@@ -1537,16 +1538,32 @@ static const GraphicHwOps sm501_ops = { .gfx_update = sm501_update_display, }; +static void sm501_reset (void *p) +{ +SM501State *s = p; + + s->system_control =0x00100000 ;/* 2D engine FIFO empty */ + s->misc_control =SM501_MISC_IRQ_INVERT ;/* assumes SH, active=low */ Don't forget to update this patch when you fix this reset value in patch 1.
Quoted 19 lines
hw/display/sm501.c +13
-0+ s->gpio_31_0_control =0 ; + s->gpio_63_32_control =0 ; + s->dram_control =0 ; + s->arbitration_control =0x05146732 ; + s->irq_mask =0 ; + s->misc_timing =0 ; + s->power_mode_control =0 ; + s->dc_panel_control =0x00010000 ;/* FIFO level 3 */ + s->dc_video_control =0 ; + s->dc_crt_control =0x00010000 ; + s->twoD_control =0 ; +} +static void sm501_init (SM501State *s,DeviceState *dev,uint32_t local_mem_bytes) { s->local_mem_size_index =get_local_mem_size_index (local_mem_bytes);SM501_DPRINTF ("sm501 local mem size=%x. index=%d\n" ,get_local_mem_size (s), s->local_mem_size_index);
Quoted 8 lines
hw/display/sm501.c static const GraphicHwOps sm501_ops = { · line 1566
Quoted 5 lines
hw/display/sm501.c +1
-4- s->system_control =0x00100000 ;/* 2D engine FIFO empty */ - s->misc_control =SM501_MISC_IRQ_INVERT ;/* assumes SH, active=low */ - s->dc_panel_control =0x00010000 ;/* FIFO level 3 */ - s->dc_crt_control =0x00010000 ; +qemu_register_reset (sm501_reset, s);Don't use
qemu_register_reset(). Set the appropriatedc->resetfunction pointers instead.
Any reason for that? This way I could save two more boilerplate functions because I could define reset function once, otherwise I'd need two versions taking sysbus and pci states just to extract the SM501State function and call this function. Do you still think I should do that instead?
Quoted 72 lines
Quoted 72 lines
hw/display/sm501.c +66
-0/* local memory */ memory_region_init_ram(&s->local_mem_region, OBJECT(dev), "sm501.local",@@ -1577,6 +1594,77 @@ static void sm501_init(SM501State *s, DeviceState *dev, s->con =graphic_console_init (DEVICE (dev),0 , &sm501_ops, s); } +static const VMStateDescription vmstate_sm501_regs = { + .name ="sm501-regs" , + .version_id =1 , + .minimum_version_id =1 , + .fields = (VMStateField []) { +VMSTATE_UINT32 (system_control, SM501State), +VMSTATE_UINT32 (misc_control, SM501State), +VMSTATE_UINT32 (gpio_31_0_control, SM501State), +VMSTATE_UINT32 (gpio_63_32_control, SM501State), +VMSTATE_UINT32 (dram_control, SM501State), +VMSTATE_UINT32 (arbitration_control, SM501State), +VMSTATE_UINT32 (irq_mask, SM501State), +VMSTATE_UINT32 (misc_timing, SM501State), +VMSTATE_UINT32 (power_mode_control, SM501State), +VMSTATE_UINT32 (uart0_ier, SM501State), +VMSTATE_UINT32 (uart0_lcr, SM501State), +VMSTATE_UINT32 (uart0_mcr, SM501State), +VMSTATE_UINT32 (uart0_scr, SM501State), +VMSTATE_UINT8_ARRAY (dc_palette, SM501State,DC_PALETTE_ENTRIES ), +VMSTATE_UINT32 (dc_panel_control, SM501State), +VMSTATE_UINT32 (dc_panel_panning_control, SM501State), +VMSTATE_UINT32 (dc_panel_fb_addr, SM501State), +VMSTATE_UINT32 (dc_panel_fb_offset, SM501State), +VMSTATE_UINT32 (dc_panel_fb_width, SM501State), +VMSTATE_UINT32 (dc_panel_fb_height, SM501State), +VMSTATE_UINT32 (dc_panel_tl_location, SM501State), +VMSTATE_UINT32 (dc_panel_br_location, SM501State), +VMSTATE_UINT32 (dc_panel_h_total, SM501State), +VMSTATE_UINT32 (dc_panel_h_sync, SM501State), +VMSTATE_UINT32 (dc_panel_v_total, SM501State), +VMSTATE_UINT32 (dc_panel_v_sync, SM501State), +VMSTATE_UINT32 (dc_panel_hwc_addr, SM501State), +VMSTATE_UINT32 (dc_panel_hwc_location, SM501State), +VMSTATE_UINT32 (dc_panel_hwc_color_1_2, SM501State), +VMSTATE_UINT32 (dc_panel_hwc_color_3, SM501State), +VMSTATE_UINT32 (dc_video_control, SM501State), +VMSTATE_UINT32 (dc_crt_control, SM501State), +VMSTATE_UINT32 (dc_crt_fb_addr, SM501State), +VMSTATE_UINT32 (dc_crt_fb_offset, SM501State), +VMSTATE_UINT32 (dc_crt_h_total, SM501State), +VMSTATE_UINT32 (dc_crt_h_sync, SM501State), +VMSTATE_UINT32 (dc_crt_v_total, SM501State), +VMSTATE_UINT32 (dc_crt_v_sync, SM501State), +VMSTATE_UINT32 (dc_crt_hwc_addr, SM501State), +VMSTATE_UINT32 (dc_crt_hwc_location, SM501State), +VMSTATE_UINT32 (dc_crt_hwc_color_1_2, SM501State), +VMSTATE_UINT32 (dc_crt_hwc_color_3, SM501State), +VMSTATE_UINT32 (twoD_source, SM501State), +VMSTATE_UINT32 (twoD_destination, SM501State), +VMSTATE_UINT32 (twoD_dimension, SM501State), +VMSTATE_UINT32 (twoD_control, SM501State), +VMSTATE_UINT32 (twoD_pitch, SM501State), +VMSTATE_UINT32 (twoD_foreground, SM501State), +VMSTATE_UINT32 (twoD_background, SM501State), +VMSTATE_UINT32 (twoD_stretch, SM501State), +VMSTATE_UINT32 (twoD_color_compare, SM501State), +VMSTATE_UINT32 (twoD_color_compare_mask, SM501State), +VMSTATE_UINT32 (twoD_mask, SM501State), +VMSTATE_UINT32 (twoD_clip_tl, SM501State), +VMSTATE_UINT32 (twoD_clip_br, SM501State), +VMSTATE_UINT32 (twoD_mono_pattern_low, SM501State), +VMSTATE_UINT32 (twoD_mono_pattern_high, SM501State), +VMSTATE_UINT32 (twoD_window_width, SM501State), +VMSTATE_UINT32 (twoD_source_base, SM501State), +VMSTATE_UINT32 (twoD_destination_base, SM501State), +VMSTATE_UINT32 (twoD_alpha, SM501State),
Quoted 8 lines
hw/display/sm501.c static void sm501_init(SM501State *s, DeviceState *dev, · line 1666
Quoted 4 lines
hw/display/sm501.c +4
-0+VMSTATE_UINT32 (twoD_wrap, SM501State), + VMSTATE_END_OF_LIST() + } +};
local_mem_size_indexis also guest updatable state (it's some of the bits from theSM501_DRAM_CONTROLregister), so we need to migrate it as well.
OK, thanks for catching this.
Quoted 29 lines
Quoted 29 lines
hw/display/sm501.c +13
-0+#define TYPE_SYSBUS_SM501 "sysbus-sm501"#define SYSBUS_SM501 (obj) \ OBJECT_CHECK(SM501SysBusState, (obj), TYPE_SYSBUS_SM501)@@ -1673,6 +1761,17 @@ static Property sm501_pci_properties[] = { DEFINE_PROP_END_OF_LIST(), }; +static const VMStateDescription vmstate_sm501 = { + .name ="sm501" , + .version_id =1 , + .minimum_version_id =1 , + .fields = (VMStateField []) { +VMSTATE_PCI_DEVICE (parent_obj, SM501PCIState), +VMSTATE_STRUCT (state, SM501PCIState,1 , vmstate_sm501_regs, SM501State), +VMSTATE_END_OF_LIST () + } +}; +static void sm501_pci_class_init (ObjectClass *klass,void *data) {DeviceClass *dc =DEVICE_CLASS (klass);@@ -1686,6 +1785,7 @@ static void sm501_pci_class_init(ObjectClass *klass, void *data) dc->desc ="SM501 Display Controller" ; dc->props = sm501_pci_properties; dc->hotpluggable = false; + dc->vmsd = &vmstate_sm501; }
Quoted 7 lines
Quoted 3 lines
hw/display/sm501.c +0
-1static const TypeInfo sm501_pci_info = { --2.7.4
What about reset and vmstate for the sysbus device?
Consider putting reset and vmstate in separate patches.
So should it be 4 patches: reset for pci, reset for sysbus, vmstate for pci, vmstate for sysbus or 2 patches: reset for both, vmstate for both?
Quoted 5 lines
thanks
-- PMM
Re: [Qemu-devel] [PATCH v2 13/14] sm501: Add reset function and vmstate descriptor
Quoted 10 lines · BALATON Zoltan
Quoted 3 lines · Peter Maydell
Don't use
qemu_register_reset(). Set the appropriatedc->resetfunction pointers instead.Any reason for that? This way I could save two more boilerplate functions because I could define reset function once, otherwise I'd need two versions taking sysbus and pci states just to extract the SM501State function and call this function. Do you still think I should do that instead?
qemu_register_reset is a pre-QOM method for doing reset; the standard QOM way of saying "my device has some reset behaviour" is to set its reset method pointer.
Code calling qemu_register_reset() is generally either (a) doing something kind of weird or (b) old device code that hasn't yet been converted to QOM.
Quoted 2 lines
So should it be 4 patches: reset for pci, reset for sysbus, vmstate for pci, vmstate for sysbus or 2 patches: reset for both, vmstate for both?
I think I'd go with 2 patches, since the two are going to share the bulk of the implementation in each case.
thanks
-- PMM
Re: [Qemu-devel] [PATCH v2 13/14] sm501: Add reset function and vmstate descriptor
Quoted 12 lines · Peter Maydell
Quoted 10 lines · BALATON Zoltan
Quoted 3 lines · Peter Maydell
Don't use
qemu_register_reset(). Set the appropriatedc->resetfunction pointers instead.Any reason for that? This way I could save two more boilerplate functions because I could define reset function once, otherwise I'd need two versions taking sysbus and pci states just to extract the SM501State function and call this function. Do you still think I should do that instead?
Quoted 6 lines
qemu_register_resetis a pre-QOM method for doing reset; the standard QOM way of saying "my device has some reset behaviour" is to set its reset method pointer.
Code callingqemu_register_reset()is generally either (a) doing something kind of weird or (b) old device code that hasn't yet been converted to QOM.
Hmm, does having a device with both sysbus and pci versions qualify as weird? Does adding a comment saying that this way we don't need two more reset functions just to call the registered one justifies using qemu_register_reset() or this function is deprecated and should not be used and should go with the separate reset functions instead?
Quoted 5 lines
Quoted 2 lines
So should it be 4 patches: reset for pci, reset for sysbus, vmstate for pci, vmstate for sysbus or 2 patches: reset for both, vmstate for both?
I think I'd go with 2 patches, since the two are going to share the bulk of the implementation in each case.
OK, thanks for the explanation in previous reply as well, I'll do this then.
Re: [Qemu-devel] [PATCH v2 13/14] sm501: Add reset function and vmstate descriptor
Quoted 13 lines · BALATON Zoltan
Quoted 12 lines · Peter Maydell
Quoted 10 lines · BALATON Zoltan
Quoted 3 lines · Peter Maydell
Don't use
qemu_register_reset(). Set the appropriatedc->resetfunction pointers instead.Any reason for that? This way I could save two more boilerplate functions because I could define reset function once, otherwise I'd need two versions taking sysbus and pci states just to extract the SM501State function and call this function. Do you still think I should do that instead?
Quoted 8 lines
Quoted 6 lines
qemu_register_resetis a pre-QOM method for doing reset; the standard QOM way of saying "my device has some reset behaviour" is to set its reset method pointer.
Code callingqemu_register_reset()is generally either (a) doing something kind of weird or (b) old device code that hasn't yet been converted to QOM.Hmm, does having a device with both sysbus and pci versions qualify as weird? Does adding a comment saying that this way we don't need two more reset functions just to call the registered one justifies using
qemu_register_reset()or this function is deprecated and should not be used and should go with the separate reset functions instead?
On second thought I may go with the separate reset functions because then I can also update the bus type in the misc_control (not that anything I know depends on it but we can get it right).
Quoted 7 lines
Quoted 5 lines
Quoted 2 lines
So should it be 4 patches: reset for pci, reset for sysbus, vmstate for pci, vmstate for sysbus or 2 patches: reset for both, vmstate for both?
I think I'd go with 2 patches, since the two are going to share the bulk of the implementation in each case.
OK, thanks for the explanation in previous reply as well, I'll do this then.
So I may also add the reset function already in the QOMify and Add PCI version patches then only one more patch is needed for the vmstate.
Re: [Qemu-devel] [PATCH v2 13/14] sm501: Add reset function and vmstate descriptor
In reply to BALATON ZoltanQuoted 27 lines · BALATON Zoltan
Quoted 18 lines · Peter Maydell
Quoted 9 lines · BALATON Zoltan
Quoted 4 lines · Peter Maydell
Don't use
qemu_register_reset(). Set the appropriatedc->resetfunction pointers instead.Quoted 5 lines
Any reason for that? This way I could save two more boilerplate functions because I could define reset function once, otherwise I'd need two versions
taking sysbus and pci states just to extract the SM501State function and call this function. Do you still think I should do that instead?Quoted 6 lines
qemu_register_resetis a pre-QOM method for doing reset; the standard QOM way of saying "my device has some reset behaviour" is to set its reset method pointer.
Code callingqemu_register_reset()is generally either (a) doing something kind of weird or (b) old device code that hasn't yet been converted to QOM.
Quoted 5 lines
Hmm, does having a device with both sysbus and pci versions qualify as weird? Does adding a comment saying that this way we don't need two more reset functions just to call the registered one justifies using
qemu_register_reset()or this function is deprecated and should not be used and should go with the separate reset functions instead?
Just use the QOM reset method pointers. Every QOM device should have a reset method, and that should be how it gets
reset. (PCI is special anyway, it needs to get reset when the guest does a PCI bus reset, and that results in your device's reset method getting called. Functions registered with qemu_register_reset() only get called for full system reset.)
thanks
-- PMM
PATCH v2 14/14 ppc: Add SM501 device in config for ppc and ppcemb targets
In reply to BALATON ZoltanThis is not used by default on any emulated machine yet but it is still useful to have it compiled so it can be added from the command line for clients that can use it (e.g. MorphOS has no driver for any other emulated video cards but can output via SM501)
--- default-configs/ppc-softmmu.mak | 1 + default-configs/ppcemb-softmmu.mak | 1 + 2 files changed, 2 insertions(+)
default-configs/ppc-softmmu.mak +1 -0
@@ -45,6 +45,7 @@ CONFIG_OPENPIC_KVM=$(and $(CONFIG_E500),$(CONFIG_KVM)) CONFIG_PLATFORM_BUS=y CONFIG_ETSEC=y CONFIG_LIBDECNUMBER=y +CONFIG_SM501=y # For PReP CONFIG_SERIAL_ISA=y CONFIG_MC146818RTC=y
default-configs/ppcemb-softmmu.mak +1 -0
@@ -15,3 +15,4 @@ CONFIG_I8259=y CONFIG_XILINX=y CONFIG_XILINX_ETHLITE=y CONFIG_LIBDECNUMBER=y +CONFIG_SM501=y
Signature · 1 line
2.7.4
Quoted 1 line · BALATON Zoltan
Second try, addressing review comments.
So.. I'm not sure what to do with this :)
I can apply it to -trivial, but this smells a bit unfair on my side, while on the other hand, is there any more appropriate tree?
Thanks,
/mjt
Quoted 8 lines · Michael Tokarev
Quoted 1 line · BALATON Zoltan
Second try, addressing review comments.
So.. I'm not sure what to do with this :)
I can apply it to -trivial, but this smells a bit unfair on my side, while on the other hand, is there any more appropriate tree?
I think it's a bit of a stretch to argue that "1089 insertions(+), 741 deletions" is "trivial", but the difficulty is that sh4 isn't really very maintained. Since the purpose seems to be to improve ppc emulation somehow it could perhaps go via the ppc tree, but I dunno.
I have said I'll try to review this (since I looked at v1), but I don't think I'm going to get to this before the softfreeze deadline, I'm afraid. If you want to go through and review it all feel free -- you probably have as much knowledge about sh4 and this device as I do :-)
thanks
-- PMM