Improvements for sm501 display controller emulation
cover Improvements for sm501 display controller emulation
This series improves the sm501 display controller emulation fixing endianness problems that caused mixed up colors in LE hosts, fix hardware cursor and adding panel layer support and some missing registers. The first few patches update the code style and QOMify the device before changes are made to it in subsequent patches.
Including qemu-trivial list as well, both because some of the patches are trivial and also I'm not sure how actively maintained this part is so that also may need attention from the trivial list to get this merged.
The changes were tested with sh4 image at
https://people.debian.org/~aurel32/qemu/sh4/
which accepts video= kernel parameter to excercise different screen modes.
BALATON Zoltan (10):
sm501: Fixed code style and a few typos in comments
sm501: Use defines instead of constants where available
sm501: QOMify
sm501: Add emulation of chip connected via PCI
sm501: Add missing arbitration control register
sm501: Fix device endianness
sm501: Fix hardware cursor
sm501: Add support for panel layer
sm501: Add some more missing registers
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 | 1546 ++++++++++++++++++++---------------- hw/display/sm501_template.h | 92 +-- hw/sh4/r2d.c | 11 +- include/hw/devices.h | 5 - 6 files changed, 920 insertions(+), 736 deletions(-)
Signature · 1 line
2.7.4
PATCH 1/10 sm501: Fixed code style and a few typos in comments
Signed-off-by BALATON Zoltan <[email protected]>
--- 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
Re: [Qemu-devel] [PATCH 01/10] sm501: Fixed code style and a few typos in comments
Quoted 5 lines · BALATON Zoltan
Signed-off-by BALATON Zoltan <[email protected]>--- hw/display/sm501.c | 1132 ++++++++++++++++++++++--------------------- hw/display/sm501_template.h | 52 +- 2 files changed, 594 insertions(+), 590 deletions(-)
Reviewed-by Peter Maydell <[email protected]>
(checked the ignore-whitespace-only diff.)
thanks
-- PMM
PATCH 2/10 sm501: Use defines instead of constants where available
In reply to BALATON ZoltanSigned-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 +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 02/10] sm501: Use defines instead of constants where available
Quoted 20 lines · BALATON Zoltan
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 1416
Quoted 9 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->misc_control was set to 0x1000, but is now set to SM501_MISC_IRQ_INVERT, which is (1 << 16), which is not the same value... 0x1000 would be SM501_MISC_DAC_POWER.
thanks
-- PMM
Re: [Qemu-devel] [PATCH 02/10] sm501: Use defines instead of constants where available
Quoted 19 lines · Peter Maydell
Quoted 18 lines · BALATON Zoltan
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 ; }
Quoted 15 lines
hw/display/sm501.c void sm501_init(MemoryRegion *address_space_mem, uint32_t base, · line 1416
Quoted 11 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->misc_controlwas set to0x1000, but is now set toSM501_MISC_IRQ_INVERT, which is (1 << 16), which is not the same value...0x1000would beSM501_MISC_DAC_POWER.
Thanks for reviewing these patches.
The comment suggests that the intended bit was the IRQ invert bit so the constant used before may have been a bug. (In my understanding nothing really uses this bit so I could set it either way you prefer but I think this is the correct one.) What should I do?
1. Leave it as it is
2. Set it to the old constant contradicting the comment
3. Make it a separate patch?
PATCH 3/10 sm501: QOMify
In reply to BALATON ZoltanSigned-off-by BALATON Zoltan <[email protected]>
--- hw/display/sm501.c | 133 +++++++++++++++++++++++++++++++++++---------------- hw/sh4/r2d.c | 11 ++++- include/hw/devices.h | 5 -- 3 files changed, 101 insertions(+), 48 deletions(-)
hw/display/sm501.c +92 -41
@@ -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,7 @@ typedef struct SM501State {uint32_t local_mem_size_index;uint8_t *local_mem;MemoryRegion local_mem_region; +MemoryRegion mmio_region;uint32_t last_width;uint32_t last_height;
@@ -1396,20 +1397,14 @@ 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 ); +MemoryRegion *mr; + 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 */
@@ -1417,46 +1412,102 @@ void sm501_init(MemoryRegion *address_space_mem, uint32_t base, s->dc_crt_control =0x00010000 ;/* allocate local memory */ -memory_region_init_ram (&s->local_mem_region,NULL ,"sm501.local" , +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, + +/* allocate mmio */ +memory_region_init (&s->mmio_region,OBJECT (dev),"sm501.mmio" ,MMIO_SIZE ); + mr =g_new (MemoryRegion,1 ); +memory_region_init_io (mr,OBJECT (dev), &sm501_system_config_ops, s, +"sm501-system-config" ,0x6c ); +memory_region_add_subregion (&s->mmio_region,SM501_SYS_CONFIG , mr); + mr =g_new (MemoryRegion,1 ); +memory_region_init_io (mr,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 , mr); + mr =g_new (MemoryRegion,1 ); +memory_region_init_io (mr,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 , mr); + +/* 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 03/10] sm501: QOMify
Quoted 67 lines · BALATON Zoltan
Signed-off-by BALATON Zoltan <[email protected]>--- hw/display/sm501.c | 133 +++++++++++++++++++++++++++++++++++---------------- hw/sh4/r2d.c | 11 ++++- include/hw/devices.h | 5 -- 3 files changed, 101 insertions(+), 48 deletions(-)hw/display/sm501.c +8
-15@@ -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,7 @@ typedef struct SM501State {uint32_t local_mem_size_index;uint8_t *local_mem;MemoryRegion local_mem_region; +MemoryRegion mmio_region;uint32_t last_width;uint32_t last_height;@@ -1396,20 +1397,14 @@ 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 ); +MemoryRegion *mr; + 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 */ @@ -1417,46 +1412,102 @@ void sm501_init(MemoryRegion *address_space_mem, uint32_t base, s->dc_crt_control =0x00010000 ;/* allocate local memory */ -memory_region_init_ram (&s->local_mem_region,NULL ,"sm501.local" , +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); -
hw/display/sm501.c void sm501_init(MemoryRegion *address_space_mem, uint32_t base, · line 1423
Quoted 10 lines
hw/display/sm501.c +4
-6-/* 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, + +/* allocate mmio */ +memory_region_init (&s->mmio_region,OBJECT (dev),"sm501.mmio" ,MMIO_SIZE ); + mr =g_new (MemoryRegion,1 );
There's no need to dynamically allocate any of these memory regions; just make them be MemoryRegion fields inside SM501State.
Quoted 49 lines
hw/display/sm501.c +37
-8+memory_region_init_io (mr,OBJECT (dev), &sm501_system_config_ops, s, +"sm501-system-config" ,0x6c ); +memory_region_add_subregion (&s->mmio_region,SM501_SYS_CONFIG , mr); + mr =g_new (MemoryRegion,1 ); +memory_region_init_io (mr,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 , mr); + mr =g_new (MemoryRegion,1 ); +memory_region_init_io (mr,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 , mr); + +/* 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" );
hw/display/sm501.c void sm501_init(MemoryRegion *address_space_mem, uint32_t base, · line 1468
Quoted 10 lines
hw/display/sm501.c +4
-6-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);
Why is a display controller device creating a USB controller? This looks like something that should be being done by the board.
Quoted 5 lines
hw/display/sm501.c +3
-0+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 */
hw/display/sm501.c void sm501_init(MemoryRegion *address_space_mem, uint32_t base, · line 1479
Quoted 10 lines
hw/display/sm501.c +4
-4-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 ); } +}
Similarly, what's going on here with the serial?
Quoted 12 lines
hw/display/sm501.c +8
-2-/* 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) +{
hw/display/sm501.c void sm501_init(MemoryRegion *address_space_mem, uint32_t base, · line 1499
Quoted 10 lines
hw/display/sm501.c +9
-0+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; +*/ }
You also need a VMState struct registered in dc->vmsd and a reset function registered in dc->reset.
thanks
-- PMM
Re: [Qemu-devel] [PATCH 03/10] sm501: QOMify
Quoted 161 lines · Peter Maydell
Quoted 67 lines · BALATON Zoltan
Signed-off-by BALATON Zoltan <[email protected]>--- hw/display/sm501.c | 133 +++++++++++++++++++++++++++++++++++---------------- hw/sh4/r2d.c | 11 ++++- include/hw/devices.h | 5 -- 3 files changed, 101 insertions(+), 48 deletions(-)hw/display/sm501.c +8
-15@@ -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,7 @@ typedef struct SM501State {uint32_t local_mem_size_index;uint8_t *local_mem;MemoryRegion local_mem_region; +MemoryRegion mmio_region;uint32_t last_width;uint32_t last_height;@@ -1396,20 +1397,14 @@ 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 ); +MemoryRegion *mr; + 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 */ @@ -1417,46 +1412,102 @@ void sm501_init(MemoryRegion *address_space_mem, uint32_t base, s->dc_crt_control =0x00010000 ;/* allocate local memory */ -memory_region_init_ram (&s->local_mem_region,NULL ,"sm501.local" , +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); -hw/display/sm501.c void sm501_init(MemoryRegion *address_space_mem, uint32_t base, · line 1423
Quoted 10 lines
hw/display/sm501.c +4
-6-/* 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, + +/* allocate mmio */ +memory_region_init (&s->mmio_region,OBJECT (dev),"sm501.mmio" ,MMIO_SIZE ); + mr =g_new (MemoryRegion,1 );There's no need to dynamically allocate any of these memory regions; just make them be MemoryRegion fields inside SM501State.
Quoted 49 lines
hw/display/sm501.c +37
-8+memory_region_init_io (mr,OBJECT (dev), &sm501_system_config_ops, s, +"sm501-system-config" ,0x6c ); +memory_region_add_subregion (&s->mmio_region,SM501_SYS_CONFIG , mr); + mr =g_new (MemoryRegion,1 ); +memory_region_init_io (mr,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 , mr); + mr =g_new (MemoryRegion,1 ); +memory_region_init_io (mr,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 , mr); + +/* 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" );hw/display/sm501.c void sm501_init(MemoryRegion *address_space_mem, uint32_t base, · line 1468
Quoted 10 lines
hw/display/sm501.c +4
-6-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);Why is a display controller device creating a USB controller? This looks like something that should be being done by the board.
Quoted 5 lines
hw/display/sm501.c +3
-0+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 */ hw/display/sm501.c void sm501_init(MemoryRegion *address_space_mem, uint32_t base, · line 1479
Quoted 10 lines
hw/display/sm501.c +4
-4-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 ); } +}
Quoted 1 line
Similarly, what's going on here with the serial?
The SM501/SM502 is a multimedia chip that besides a display controller also contains some other functions (see http://cateee.net/lkddb/web-lkddb/MFD_SM501.html) and this is what is emulated here as these are part of the chip itself.
Quoted 11 lines
Quoted 10 lines
hw/display/sm501.c +6
-2-/* 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 (), +};
Quoted 15 lines
hw/display/sm501.c void sm501_init(MemoryRegion *address_space_mem, uint32_t base, · line 1499
Quoted 12 lines
hw/display/sm501.c +11
-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; +*/ }You also need a VMState struct registered in
dc->vmsdand a reset function registered indc->reset.
Even if no migration is supported? Is there a simple example I could look at on what should go into these?
Re: [Qemu-devel] [PATCH 03/10] sm501: QOMify
Quoted 182 lines · BALATON Zoltan
Quoted 181 lines · Peter Maydell
Quoted 80 lines · BALATON Zoltan
Signed-off-by BALATON Zoltan <[email protected]>--- hw/display/sm501.c | 133 +++++++++++++++++++++++++++++++++++---------------- hw/sh4/r2d.c | 11 ++++- include/hw/devices.h | 5 -- 3 files changed, 101 insertions(+), 48 deletions(-)hw/display/sm501.c +1
-1@@ -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" */hw/display/sm501.c +6
-11@@ -464,6 +464,7 @@ typedef struct SM501State {uint32_t local_mem_size_index;uint8_t *local_mem;MemoryRegion local_mem_region; +MemoryRegion mmio_region;uint32_t last_width;uint32_t last_height;@@ -1396,20 +1397,14 @@ 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 ); +MemoryRegion *mr; + 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*/
hw/display/sm501.c +1
-6@@ -1417,46 +1412,102 @@ void sm501_init(MemoryRegion *address_space_mem,uint32_t base, s->dc_crt_control = 0x00010000;/* allocate local memory */ -memory_region_init_ram (&s->local_mem_region,NULL ,"sm501.local" , +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 );hw/display/sm501.c void sm501_init(MemoryRegion *address_space_mem, · line 1430
Quoted 10 lines
hw/display/sm501.c +4
-3-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, + +/* allocate mmio */ +memory_region_init (&s->mmio_region,OBJECT (dev),"sm501.mmio" ,MMIO_SIZE ); + mr =g_new (MemoryRegion,1 );There's no need to dynamically allocate any of these memory regions; just make them be MemoryRegion fields inside SM501State.
Quoted 51 lines
hw/display/sm501.c +37
-8+memory_region_init_io (mr,OBJECT (dev), &sm501_system_config_ops, s, +"sm501-system-config" ,0x6c ); +memory_region_add_subregion (&s->mmio_region,SM501_SYS_CONFIG , mr); + mr =g_new (MemoryRegion,1 ); +memory_region_init_io (mr,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 , mr); + mr =g_new (MemoryRegion,1 ); +memory_region_init_io (mr,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 , mr); + +/* 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" );hw/display/sm501.c void sm501_init(MemoryRegion *address_space_mem, uint32_t base, · line 1468
Quoted 10 lines
hw/display/sm501.c +4
-6-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);Why is a display controller device creating a USB controller? This looks like something that should be being done by the board.
Quoted 16 lines
hw/display/sm501.c +7
-4+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 ); } +}
Quoted 7 lines
Quoted 1 line
Similarly, what's going on here with the serial?
The SM501/SM502 is a multimedia chip that besides a display controller also contains some other functions (see
http://cateee.net/lkddb/web-lkddb/MFD_SM501.html) and this is what is emulated here as these are part of the chip itself.
Hmm; that's pretty ugly but I guess it's sort of like a container device that needs to contain various things inside it.
Quoted 7 lines
Quoted 7 lines
hw/display/sm501.c void sm501_init(MemoryRegion *address_space_mem, uint32_t base, · line 1484
Quoted 7 lines
hw/display/sm501.c +4
-2-/* 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),
Pointer properties and properties which tell the device what address it's at are both signs that something's not quite modelled right. There may be no better way to do things right now, or then again there may be.
Quoted 22 lines
Quoted 4 lines
Quoted 2 lines
hw/display/sm501.c +2
-0+ DEFINE_PROP_END_OF_LIST(), +};Quoted 16 lines
Quoted 11 lines
hw/display/sm501.c +11
-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; +*/ Quoted 1 line
}
You also need a VMState struct registered in
dc->vmsdand a reset function registered indc->reset.
Quoted 2 lines
Even if no migration is supported? Is there a simple example I could look at on what should go into these?
The idea is to support migration. There are examples of doing VMState structures all over the tree. You just need a structure that lists all the bits of your state data structure that contain guest-modifiable state.
Reset is straightforward: it's just a function that resets the state of the device as if the system had been hard powercycled.
thanks
-- PMM
Re: [Qemu-devel] [PATCH 03/10] sm501: QOMify
Quoted 17 lines · Peter Maydell
Quoted 4 lines · BALATON Zoltan
The SM501/SM502 is a multimedia chip that besides a display controller also contains some other functions (see
http://cateee.net/lkddb/web-lkddb/MFD_SM501.html) and this is what is emulated here as these are part of the chip itself.Hmm; that's pretty ugly but I guess it's sort of like a container device that needs to contain various things inside it.
Quoted 7 lines
Quoted 7 lines
hw/display/sm501.c void sm501_init(MemoryRegion *address_space_mem, uint32_t base, · line 1484
Quoted 7 lines
hw/display/sm501.c +4
-2-/* 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),
Quoted 4 lines
Pointer properties and properties which tell the device what address it's at are both signs that something's not quite modelled right. There may be no better way to do things right now, or then again there may be.
Maybe but I think that would not be part of this series but some other clean up separately. This series makes it a bit cleaner but does not aim to fix everything.
Quoted 25 lines
Quoted 22 lines
Quoted 4 lines
Quoted 2 lines
(quoted diff) +2
-0+ DEFINE_PROP_END_OF_LIST(), +};Quoted 16 lines
Quoted 11 lines
hw/display/sm501.c +11
-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; +*/ Quoted 1 line
}
You also need a VMState struct registered in
dc->vmsdand a reset function registered indc->reset.Quoted 2 lines
Even if no migration is supported? Is there a simple example I could look at on what should go into these?
Quoted 8 lines
The idea is to support migration. There are examples of doing VMState structures all over the tree. You just need a structure that lists all the bits of your state data structure that contain guest-modifiable state.
Reset is straightforward: it's just a function that resets the state of the device as if the system had been hard powercycled.
I've tried to add these but vmstate is only added to the PCI version because the OHCI device I've looked at also does the same, which is likely beacuse the sysbus version is only used on SH4 which does not support migration anyway. The PCI verison can be instantiated on machines that have migration support so it makes sense to do it there. I hope this is acceptable.
PATCH 4/10 sm501: Add emulation of chip connected via PCI
In reply to BALATON ZoltanOnly the display controller part is created automatically on PCI
Signed-off-by BALATON Zoltan <[email protected]>
--- hw/display/sm501.c | 58 +++++++++++++++++++++++++++++++++++++++++---- hw/display/sm501_template.h | 8 +++---- 2 files changed, 58 insertions(+), 8 deletions(-)
hw/display/sm501.c +54 -4
@@ -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"
@@ -460,7 +461,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;
@@ -1397,12 +1397,11 @@ 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) {MemoryRegion *mr; - 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);
@@ -1457,7 +1456,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);
@@ -1505,9 +1504,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)
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 04/10] sm501: Add emulation of chip connected via PCI
Quoted 86 lines · BALATON Zoltan
Only the display controller part is created automatically on PCI
Signed-off-by BALATON Zoltan <[email protected]>--- hw/display/sm501.c | 58 +++++++++++++++++++++++++++++++++++++++++---- hw/display/sm501_template.h | 8 +++---- 2 files changed, 58 insertions(+), 8 deletions(-)hw/display/sm501.c +34
-4@@ -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" @@ -460,7 +461,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;@@ -1397,12 +1397,11 @@ 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) {MemoryRegion *mr; - 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);@@ -1457,7 +1456,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);@@ -1505,9 +1504,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);
hw/display/sm501.c static const TypeInfo sm501_sysbus_info = { · line 1547
Quoted 10 lines
hw/display/sm501.c +10
-0+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;
This needs a reset function and a vmsd as well.
Quoted 37 lines
hw/display/sm501.c +10
-0+} + +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)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);
hw/display/sm501_template.h static void glue(draw_hwc_line_, PIXEL_NAME)(SM501State *s, int crt, · line 127
Quoted 10 lines
hw/display/sm501_template.h +2
-2@@ -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 ;
Dropping the requirement for a base addr is not really the same change as adding the PCI device.
thanks
-- PMM
Re: [Qemu-devel] [PATCH 04/10] sm501: Add emulation of chip connected via PCI
Quoted 138 lines · Peter Maydell
Quoted 86 lines · BALATON Zoltan
Only the display controller part is created automatically on PCI
Signed-off-by BALATON Zoltan <[email protected]>--- hw/display/sm501.c | 58 +++++++++++++++++++++++++++++++++++++++++---- hw/display/sm501_template.h | 8 +++---- 2 files changed, 58 insertions(+), 8 deletions(-)hw/display/sm501.c +34
-4@@ -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" @@ -460,7 +461,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;@@ -1397,12 +1397,11 @@ 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) {MemoryRegion *mr; - 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);@@ -1457,7 +1456,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);@@ -1505,9 +1504,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);hw/display/sm501.c static const TypeInfo sm501_sysbus_info = { · line 1547
Quoted 10 lines
hw/display/sm501.c +10
-0+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;This needs a reset function and a vmsd as well.
Quoted 38 lines
hw/display/sm501.c +10
-0+} + +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)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 12 lines
hw/display/sm501_template.h static void glue(draw_hwc_line_, PIXEL_NAME)(SM501State *s, int crt, · line 127
Quoted 9 lines
hw/display/sm501_template.h +2
-2/* 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 ;Dropping the requirement for a base addr is not really the same change as adding the PCI device.
This is needed for PCI device because the base is not accessible before something maps the BARs of the device so I had to use something else that is known. So this change is part of making it a PCI device but I could make this a separate patch if you think that's better. Should I split it off or can leave it here?
Re: [Qemu-devel] [PATCH 04/10] sm501: Add emulation of chip connected via PCI
Quoted 10 lines · BALATON Zoltan
Quoted 2 lines · Peter Maydell
Dropping the requirement for a base addr is not really the same change as adding the PCI device.
This is needed for PCI device because the base is not accessible before something maps the BARs of the device so I had to use something else that is known. So this change is part of making it a PCI device but I could make this a separate patch if you think that's better. Should I split it off or can leave it here?
I would split it off.
thanks
-- PMM
PATCH 5/10 sm501: Add missing arbitration control register
In reply to BALATON ZoltanSigned-off-by BALATON Zoltan <[email protected]>
--- hw/display/sm501.c | 7 +++++++ 1 file changed, 7 insertions(+)
hw/display/sm501.c +7 -0
@@ -474,6 +474,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;
@@ -757,6 +758,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 ;
@@ -809,6 +813,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
PATCH 6/10 sm501: Fix device endianness
In reply to BALATON ZoltanSigned-off-by BALATON Zoltan <[email protected]>
--- hw/display/sm501.c | 6 +++--- hw/display/sm501_template.h | 31 ++++++++++++++++++------------- 2 files changed, 21 insertions(+), 16 deletions(-)
hw/display/sm501.c +3 -3
@@ -846,7 +846,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)
@@ -1082,7 +1082,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,
@@ -1170,7 +1170,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 +18 -13
@@ -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;
@@ -103,7 +108,7 @@ 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 *pixval, bitset =0 ; +uint8_t *pixval, r, g, b, bitset =0 ;/* get hardware cursor pattern */ uint32_t cursor_addr =get_hwc_address (s, crt);
@@ -129,9 +134,9 @@ static void glue(draw_hwc_line_, PIXEL_NAME)(SM501State *s, int crt,/* 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 06/10] sm501: Fix device endianness
Quoted 29 lines · BALATON Zoltan
Signed-off-by BALATON Zoltan <[email protected]>--- hw/display/sm501.c | 6 +++--- hw/display/sm501_template.h | 31 ++++++++++++++++++------------- 2 files changed, 21 insertions(+), 16 deletions(-)hw/display/sm501.c +2
-2@@ -846,7 +846,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)@@ -1082,7 +1082,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,
hw/display/sm501.c static const MemoryRegionOps sm501_2d_engine_ops = { · line 1174
Quoted 6 lines
hw/display/sm501.c +1
-1@@ -1170,7 +1170,7 @@ static const MemoryRegionOps sm501_2d_engine_ops = { .min_access_size = 4, .max_access_size = 4, }, - .endianness = DEVICE_NATIVE_ENDIAN, + .endianness = DEVICE_LITTLE_ENDIAN, };
Does this still all work for the sh4eb (big-endian) sysbus device case?
Quoted 34 lines
hw/display/sm501.c +0
-0/* draw line functions for all console modes */ hw/display/sm501_template.h +12
-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];
hw/display/sm501_template.h static void glue(draw_line32_, PIXEL_NAME)( · line 97
Quoted 10 lines
hw/display/sm501_template.h +2
-4+ b = s[2 ]; +#else r = s[1 ]; g = s[2 ]; b = s[3 ]; -#else - b = s[0 ]; - g = s[1 ]; - r = s[2 ];#endif
This looks really suspicious. Previously we had
TARGET_WORDS_BIGENDIAN -> bytes are XRGB
otherwise bytes are BGRX
Now we have
TARGET_WORDS_BIGENDIAN -> bytes are RGBX
otherwise -> bytes are XRGB
That doesn't seem very plausible at all.
Quoted 16 lines
hw/display/sm501_template.h +1
-1*(PIXEL_TYPE *)d =glue (rgb_to_pixel,PIXEL_NAME )(r, g, b); s +=4 ;@@ -103,7 +108,7 @@ 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 *pixval, bitset =0 ; +uint8_t *pixval, r, g, b, bitset =0 ;/* get hardware cursor pattern */ uint32_t cursor_addr =get_hwc_address (s, crt);@@ -129,9 +134,9 @@ static void glue(draw_hwc_line_, PIXEL_NAME)(SM501State *s, int crt,/* write pixel */ if (v) {
hw/display/sm501_template.h static void glue(draw_hwc_line_, PIXEL_NAME)(SM501State *s, int crt, · line 142
Quoted 10 lines
hw/display/sm501_template.h +3
-3v--; -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 ;
This doesn't seem to be related to the rest of this patch?
thanks
-- PMM
Re: [Qemu-devel] [PATCH 06/10] sm501: Fix device endianness
Quoted 28 lines · Peter Maydell
Quoted 27 lines · BALATON Zoltan
Signed-off-by BALATON Zoltan <[email protected]>--- hw/display/sm501.c | 6 +++--- hw/display/sm501_template.h | 31 ++++++++++++++++++------------- 2 files changed, 21 insertions(+), 16 deletions(-)hw/display/sm501.c +2
-2@@ -846,7 +846,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)@@ -1082,7 +1082,7 @@ static const MemoryRegionOps sm501_disp_ctrl_ops = { .min_access_size = 4, .max_access_size = 4, }, - .endianness = DEVICE_NATIVE_ENDIAN, + .endianness = DEVICE_LITTLE_ENDIAN, };
Quoted 10 lines
hw/display/sm501.c static const MemoryRegionOps sm501_2d_engine_ops = { · line 1174
Quoted 8 lines
hw/display/sm501.c +1
-1static uint64_t sm501_2d_engine_read(void *opaque, hwaddr addr,@@ -1170,7 +1170,7 @@ static const MemoryRegionOps sm501_2d_engine_ops = { .min_access_size = 4, .max_access_size = 4, }, - .endianness = DEVICE_NATIVE_ENDIAN, + .endianness = DEVICE_LITTLE_ENDIAN, };Does this still all work for the sh4eb (big-endian) sysbus device case?
Not sure. Is there some image somewhere I can test with?
Quoted 49 lines
Quoted 34 lines
hw/display/sm501.c +0
-0/* draw line functions for all console modes */ hw/display/sm501_template.h +12
-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];hw/display/sm501_template.h static void glue(draw_line32_, PIXEL_NAME)( · line 97
Quoted 10 lines
hw/display/sm501_template.h +2
-4+ b = s[2 ]; +#else r = s[1 ]; g = s[2 ]; b = s[3 ]; -#else - b = s[0 ]; - g = s[1 ]; - r = s[2 ];#endif This looks really suspicious. Previously we had
TARGET_WORDS_BIGENDIAN-> bytes are XRGB
otherwise bytes are BGRX
Quoted 5 lines
Now we have
TARGET_WORDS_BIGENDIAN-> bytes are RGBX
otherwise -> bytes are XRGBThat doesn't seem very plausible at all.
I've tested it with sh4 and ppc guests running on x86_64 host and these work now while previous code resulted in mixed up colors. Maybe the host endianness could also be a factor and the previous code assumed big endian host or the previous code was already broken and only worked with the default 8 bit depth. I'm not completely sure I understand endian handling in QEMU to know if this is correct besides on what I've tested but at least little endian and big endian guests should work on little endian hosts now with my patch. I can't test on big endian host.
I've used this image: https://people.debian.org/~aurel32/qemu/sh4/
with video=800x600-16 kernel parameter where changing -16 to different bit depths reproduces the problem.
Quoted 20 lines
Quoted 20 lines
hw/display/sm501_template.h +1
-4*(PIXEL_TYPE *)d =glue (rgb_to_pixel,PIXEL_NAME )(r, g, b); s +=4 ;@@ -103,7 +108,7 @@ 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 *pixval, bitset =0 ; +uint8_t *pixval, r, g, b, bitset =0 ;/* get hardware cursor pattern */ uint32_t cursor_addr =get_hwc_address (s, crt);@@ -129,9 +134,9 @@ static void glue(draw_hwc_line_, PIXEL_NAME)(SM501State *s, int crt,/* 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 ];
Quoted 8 lines
hw/display/sm501_template.h static void glue(draw_hwc_line_, PIXEL_NAME)(SM501State *s, int crt, · line 142
Quoted 6 lines
hw/display/sm501_template.h +3
-0+ 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 ;This doesn't seem to be related to the rest of this patch?
Should I split it off? Making every simple change another patch may double the size of the series but if that's preferred I can make this a separate patch.
Re: [Qemu-devel] [PATCH 06/10] sm501: Fix device endianness
Quoted 47 lines · BALATON Zoltan
Quoted 32 lines · Peter Maydell
Quoted 30 lines · BALATON Zoltan
Signed-off-by BALATON Zoltan <[email protected]>--- hw/display/sm501.c | 6 +++--- hw/display/sm501_template.h | 31 ++++++++++++++++++------------- 2 files changed, 21 insertions(+), 16 deletions(-)hw/display/sm501.c +1
-1@@ -846,7 +846,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_tsm501_palette_read(void *opaque, hwaddr addr)hw/display/sm501.c +1
-1@@ -1082,7 +1082,7 @@ static const MemoryRegionOps sm501_disp_ctrl_ops = { .min_access_size = 4, .max_access_size = 4, }, - .endianness = DEVICE_NATIVE_ENDIAN, + .endianness = DEVICE_LITTLE_ENDIAN, };Quoted 12 lines
hw/display/sm501.c static const MemoryRegionOps sm501_2d_engine_ops = · line 1175
Quoted 9 lines
static
uint64_tsm501_2d_engine_read(void *opaque, hwaddr addr,hw/display/sm501.c +1
-1@@ -1170,7 +1170,7 @@ static const MemoryRegionOps sm501_2d_engine_ops = { .min_access_size = 4, .max_access_size = 4, }, - .endianness = DEVICE_NATIVE_ENDIAN, + .endianness = DEVICE_LITTLE_ENDIAN, };Does this still all work for the sh4eb (big-endian) sysbus device case?
Quoted 1 line
Not sure. Is there some image somewhere I can test with?
I don't know, I'm afraid; I don't know anything about sh4. I'm just going by looking at the code changes and flagging up things which are potentially problems.
Quoted 66 lines
Quoted 50 lines
Quoted 34 lines
hw/display/sm501.c +0
-0/* draw line functions for all console modes */ hw/display/sm501_template.h +12
-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];hw/display/sm501_template.h static void glue(draw_line32_, PIXEL_NAME)( · line 97
Quoted 10 lines
hw/display/sm501_template.h +2
-4+ b = s[2 ]; +#else r = s[1 ]; g = s[2 ]; b = s[3 ]; -#else - b = s[0 ]; - g = s[1 ]; - r = s[2 ];#endif This looks really suspicious. Previously we had
TARGET_WORDS_BIGENDIAN-> bytes are XRGB
otherwise bytes are BGRXQuoted 5 lines
Now we have
TARGET_WORDS_BIGENDIAN-> bytes are RGBX
otherwise -> bytes are XRGBThat doesn't seem very plausible at all.
I've tested it with sh4 and ppc guests running on
x86_64host and these work now while previous code resulted in mixed up colors. Maybe the host endianness could also be a factor and the previous code assumed big endian host or the previous code was already broken and only worked with the default 8 bit depth. I'm not completely sure I understand endian handling in QEMU to know if this is correct besides on what I've tested but at least little endian and big endian guests should work on little endian hosts now with my patch. I can't test on big endian host.
Quoted 3 lines
I've used this image: https://people.debian.org/~aurel32/qemu/sh4/
with video=800x600-16 kernel parameter where changing -16 to different bit depths reproduces the problem.
Again, I don't know sh, and I don't know this particular device. I'm just pointing out that the code change here looks really implausible. Maybe the data sheet says that this is what the pixel format looks like; but it's very odd.
Quoted 34 lines
Quoted 24 lines
Quoted 24 lines
hw/display/sm501_template.h +1
-1*(PIXEL_TYPE *)d =glue (rgb_to_pixel,PIXEL_NAME )(r, g, b); s +=4 ;@@ -103,7 +108,7 @@ 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 *pixval, bitset =0 ; +uint8_t *pixval, r, g, b, bitset =0 ;/* get hardware cursor pattern */ uint32_t cursor_addr = get_hwc_address(s, crt);hw/display/sm501_template.h +1
-3@@ -129,9 +134,9 @@ static void glue(draw_hwc_line_, PIXEL_NAME)(SM501State *s, int crt,/* 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 ];Quoted 8 lines
Quoted 5 lines
hw/display/sm501_template.h +2
-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;This doesn't seem to be related to the rest of this patch?
Quoted 3 lines
Should I split it off? Making every simple change another patch may double the size of the series but if that's preferred I can make this a separate patch.
The aim is to make it easier to review. Patches which are just simple cleanup of code are easy to review because it's possible to say "no change of behaviour". Patches which are making functional changes require more thought, and it gets worse if the same patch is also making no-change-of-behaviour code changes.
thanks
-- PMM
Re: [Qemu-devel] [PATCH 06/10] sm501: Fix device endianness
Quoted 119 lines · Peter Maydell
Quoted 46 lines · BALATON Zoltan
Quoted 31 lines · Peter Maydell
Quoted 30 lines · BALATON Zoltan
Signed-off-by BALATON Zoltan <[email protected]>--- hw/display/sm501.c | 6 +++--- hw/display/sm501_template.h | 31 ++++++++++++++++++------------- 2 files changed, 21 insertions(+), 16 deletions(-)hw/display/sm501.c +1
-1@@ -846,7 +846,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_tsm501_palette_read(void *opaque, hwaddr addr)hw/display/sm501.c +1
-1@@ -1082,7 +1082,7 @@ static const MemoryRegionOps sm501_disp_ctrl_ops = { .min_access_size = 4, .max_access_size = 4, }, - .endianness = DEVICE_NATIVE_ENDIAN, + .endianness = DEVICE_LITTLE_ENDIAN, };Quoted 12 lines
hw/display/sm501.c static const MemoryRegionOps sm501_2d_engine_ops = · line 1175
Quoted 9 lines
static
uint64_tsm501_2d_engine_read(void *opaque, hwaddr addr,hw/display/sm501.c +1
-1@@ -1170,7 +1170,7 @@ static const MemoryRegionOps sm501_2d_engine_ops = { .min_access_size = 4, .max_access_size = 4, }, - .endianness = DEVICE_NATIVE_ENDIAN, + .endianness = DEVICE_LITTLE_ENDIAN, };Does this still all work for the sh4eb (big-endian) sysbus device case?
Quoted 1 line
Not sure. Is there some image somewhere I can test with?
I don't know, I'm afraid; I don't know anything about sh4. I'm just going by looking at the code changes and flagging up things which are potentially problems.
Quoted 66 lines
Quoted 50 lines
Quoted 34 lines
hw/display/sm501.c +0
-0/* draw line functions for all console modes */ hw/display/sm501_template.h +12
-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];hw/display/sm501_template.h static void glue(draw_line32_, PIXEL_NAME)( · line 97
Quoted 10 lines
hw/display/sm501_template.h +2
-4+ b = s[2 ]; +#else r = s[1 ]; g = s[2 ]; b = s[3 ]; -#else - b = s[0 ]; - g = s[1 ]; - r = s[2 ];#endif This looks really suspicious. Previously we had
TARGET_WORDS_BIGENDIAN-> bytes are XRGB
otherwise bytes are BGRXQuoted 5 lines
Now we have
TARGET_WORDS_BIGENDIAN-> bytes are RGBX
otherwise -> bytes are XRGBThat doesn't seem very plausible at all.
I've tested it with sh4 and ppc guests running on
x86_64host and these work now while previous code resulted in mixed up colors. Maybe the host endianness could also be a factor and the previous code assumed big endian host or the previous code was already broken and only worked with the default 8 bit depth. I'm not completely sure I understand endian handling in QEMU to know if this is correct besides on what I've tested but at least little endian and big endian guests should work on little endian hosts now with my patch. I can't test on big endian host.
Quoted 8 lines
Quoted 3 lines
I've used this image: https://people.debian.org/~aurel32/qemu/sh4/
with video=800x600-16 kernel parameter where changing -16 to different bit depths reproduces the problem.Again, I don't know sh, and I don't know this particular device. I'm just pointing out that the code change here looks really implausible. Maybe the data sheet says that this is what the pixel format looks like; but it's very odd.
I don't know much about it either. I could not find docs about framebuffer endianness of this device but all clients I've seen treat it as little endian, both on little endian SH4 and big endian PPC. Maybe the previous code was already broken, because on a black & white text console (like the SH4 test image listed on qemu.org) the problem is not obvious, it still looks like white text on black with mixed up color components. It can only be seen when some graphics is displayed like the Tux logo on framebuffer in the above Debian image. If it ever worked it could have been easily broken by some qemu console changes. Or maybe it worked on big endian host only but I could not verify that. All I can say it works for me now on LE host with both LE and BE guests that I could test.
PATCH 7/10 sm501: Fix hardware cursor
In reply to BALATON ZoltanSigned-off-by BALATON Zoltan <[email protected]>
--- hw/display/sm501.c | 169 +++++++++++++++++++++++++------------------- hw/display/sm501_template.h | 25 +++---- 2 files changed, 107 insertions(+), 87 deletions(-)
hw/display/sm501.c +97 -72
@@ -550,6 +550,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.
@@ -564,10 +582,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 ); }/**
@@ -593,50 +611,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)
@@ -1017,10 +1033,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;
@@ -1052,10 +1076,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;
@@ -1178,8 +1210,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"
@@ -1267,12 +1300,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 ];
@@ -1287,17 +1319,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 :
@@ -1309,18 +1338,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 */
@@ -1335,14 +1358,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) {
@@ -1354,7 +1379,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, r, g, b, 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 r, g, b, 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
PATCH 8/10 sm501: Add support for panel layer
In reply to BALATON ZoltanSigned-off-by BALATON Zoltan <[email protected]>
--- hw/display/sm501.c | 73 +++++++++++++++++++++++++++--------------------------- 1 file changed, 37 insertions(+), 36 deletions(-)
hw/display/sm501.c +37 -36
@@ -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
@@ -1297,53 +1301,62 @@ 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 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 ;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) {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" , - 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[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); + draw_hwc_line = draw_hwc_line_funcs[dst_depth_index]; + 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 */
@@ -1357,7 +1370,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 ;
@@ -1375,7 +1388,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) {
@@ -1398,9 +1411,6 @@ static void sm501_draw_crt(SM501State *s) y_start =-1 ; } } - - src += width * src_bpp; - offset += width * src_bpp; }/* complete flush to display */
@@ -1416,15 +1426,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 08/10] sm501: Add support for panel layer
Quoted 44 lines · BALATON Zoltan
Signed-off-by BALATON Zoltan <[email protected]>--- hw/display/sm501.c | 73 +++++++++++++++++++++++++++--------------------------- 1 file changed, 37 insertions(+), 36 deletions(-)hw/display/sm501.c +7
-6@@ -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 @@ -1297,53 +1301,62 @@ 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 );
hw/display/sm501.c static inline int get_depth_index(DisplaySurface *surface) · line 1314
Quoted 10 lines
hw/display/sm501.c +5
-4+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 ds_depth_index =get_depth_index (surface); +int dst_depth_index =get_depth_index (surface);
Please don't change variable names in the middle of a patch that's adding new functionality, it makes the patch harder to review.
Quoted 29 lines
hw/display/sm501.c +14
-3draw_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 ;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) {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];
hw/display/sm501.c static inline int get_depth_index(DisplaySurface *surface) · line 1347
Quoted 10 lines
hw/display/sm501.c +3
-3+ 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" , - s->dc_crt_control); +printf ("sm501 update display : invalid control register value.\n" );
This shouldn't be in the same patch as "add panel" either.
Quoted 66 lines
hw/display/sm501.c +8
-21abort ();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[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); + draw_hwc_line = draw_hwc_line_funcs[dst_depth_index]; + 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 */ @@ -1357,7 +1370,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 ;@@ -1375,7 +1388,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) {@@ -1398,9 +1411,6 @@ static void sm501_draw_crt(SM501State *s) y_start =-1 ; } } - - src += width * src_bpp; - offset += width * src_bpp; }/* complete flush to display */ @@ -1416,15 +1426,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 2 lines
2.7.4
thanks
-- PMM
Re: [Qemu-devel] [PATCH 08/10] sm501: Add support for panel layer
Quoted 50 lines · Peter Maydell
Quoted 49 lines · BALATON Zoltan
Signed-off-by BALATON Zoltan <[email protected]>--- hw/display/sm501.c | 73 +++++++++++++++++++++++++++--------------------------- 1 file changed, 37 insertions(+), 36 deletions(-)hw/display/sm501.c +11
-6@@ -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 @@ -1297,53 +1301,62 @@ 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);
Quoted 8 lines
hw/display/sm501.c static inline int get_depth_index(DisplaySurface *surface) · line 1314
Quoted 5 lines
hw/display/sm501.c +1
-4-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);Please don't change variable names in the middle of a patch that's adding new functionality, it makes the patch harder to review.
Where should I do it then? Again another patch?
Quoted 33 lines
Quoted 33 lines
hw/display/sm501.c +15
-4draw_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 ;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) {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];
Quoted 8 lines
hw/display/sm501.c static inline int get_depth_index(DisplaySurface *surface) · line 1347
Quoted 6 lines
hw/display/sm501.c +2
-2+ 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" );This shouldn't be in the same patch as "add panel" either.
I think it's related because adding panel layer makes this function not only specific the the CRT layer, hence the change in the error message.
Re: [Qemu-devel] [PATCH 08/10] sm501: Add support for panel layer
Quoted 6 lines · BALATON Zoltan
Quoted 2 lines · Peter Maydell
Please don't change variable names in the middle of a patch that's adding new functionality, it makes the patch harder to review.
Where should I do it then? Again another patch?
Yes. Either make it its own patch, or drop the change altogether. Anything that makes the core "this is making a bug fix or adding new functionality" patch bigger by adding unnecessary code change to it makes that patch harder to review. (Conversely a patch that's just "change this variable name" is trivially easy to review.)
thanks
-- PMM
PATCH 9/10 sm501: Add some more missing registers
In reply to BALATON ZoltanWrite only to allow clients to initialise these without failing
Signed-off-by BALATON Zoltan <[email protected]>
--- hw/display/sm501.c | 42 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+)
hw/display/sm501.c +42 -0
@@ -508,6 +508,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;
@@ -527,12 +529,21 @@ 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; +uint32_t twoD_status; } SM501State;
@@ -1057,6 +1068,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 &0x0003FFFF ; +break ; +case SM501_DC_CRT_CONTROL : s->dc_crt_control = value &0x0003FFFF ;break ;
@@ -1174,15 +1189,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 ;
@@ -1192,6 +1225,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 : + s->twoD_status = value; +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 09/10] sm501: Add some more missing registers
Quoted 3 lines · BALATON Zoltan
Write only to allow clients to initialise these without failing
Signed-off-by BALATON Zoltan <[email protected]>
What's the point in write-only register values?
What does the real hardware do here?
If the registers are writes-ignored, there's no need to store the data written into the state struct; if the registers are reads-as-written then implement them that way.
(If they get state struct fields the vmstate needs to be updated accordingly, as does reset.)
thanks
-- PMM
Re: [Qemu-devel] [PATCH 09/10] sm501: Add some more missing registers
Quoted 6 lines · Peter Maydell
Quoted 3 lines · BALATON Zoltan
Write only to allow clients to initialise these without failing
Signed-off-by BALATON Zoltan <[email protected]>What's the point in write-only register values?
U-boot writes this register during setting up the device and without this it would abort QEMU.
Quoted 1 line
What does the real hardware do here?
This register contains bits to set up FIFO parameters and memory priorities which we are not emulating so these can be ignored here but the hardware would change parameters according the value written.
Quoted 3 lines
If the registers are writes-ignored, there's no need to store the data written into the state struct; if the registers are reads-as-written then implement them that way.
I'm not sure what you get on real hardware but it's documented to be R/W (except reserved bits that are masked which are always 0). Why is it not implemented as read-as-written or what do you mean by that?
Re: [Qemu-devel] [PATCH 09/10] sm501: Add some more missing registers
Quoted 6 lines · BALATON Zoltan
Quoted 5 lines · Peter Maydell
Quoted 3 lines · BALATON Zoltan
Write only to allow clients to initialise these without failing
Signed-off-by BALATON Zoltan <[email protected]>
Quoted 7 lines
Quoted 1 line
What's the point in write-only register values?
U-boot writes this register during setting up the device and without this it would abort QEMU.
Quoted 1 line
What does the real hardware do here?
This register contains bits to set up FIFO parameters and memory priorities which we are not emulating so these can be ignored here but the hardware would change parameters according the value written.
Sorry, this is for the arbitration_control register. The other registers added in this patch are for the 2D engine which is only partially emulated but writing the registers is OK as long as no operation using them is called. (That case is handled in sm501_2d_operation.) We need to allow writes as these are initialised during boot but not used afterwards. Later when implementing more of the 2D engine we may use the written values.
Quoted 3 lines
Quoted 3 lines
If the registers are writes-ignored, there's no need to store the data written into the state struct; if the registers are reads-as-written then implement them that way.
Still not sure what do you mean by read-as-written because I think that's exactly what is done here, value stored and read back as is, except for video_control where there are reserved bits that are always 0.
Re: [Qemu-devel] [PATCH 09/10] sm501: Add some more missing registers
Quoted 13 lines · BALATON Zoltan
Quoted 11 lines · BALATON Zoltan
Quoted 9 lines · Peter Maydell
Quoted 4 lines · BALATON Zoltan
Write only to allow clients to initialise these without failing
Signed-off-by BALATON Zoltan <[email protected]>What's the point in write-only register values?
Quoted 8 lines
Quoted 3 lines
Quoted 3 lines
If the registers are writes-ignored, there's no need to store the data written into the state struct; if the registers are reads-as-written then implement them that way.
Still not sure what do you mean by read-as-written because I think that's exactly what is done here, value stored and read back as is, except for
video_controlwhere there are reserved bits that are always 0.
There are several ways we can make a register behave:
"Read only" -> the guest can only read, it cannot write "Writes ignored" -> the guest can write but it has no effect "Reads as written" -> the guest can write, and we store the data so that when the guest reads it gets back the value it wrote, but it doesn't have any effect on device behaviour "Reads as zero" -> the guest can read but it always gets zero "Write only" -> the guest can write values and we store them but don't let the guest read them back. This is pointless.
For cases where we don't actually implement some bit of hardware behaviour yet, it can be a reasonable choice to do either "read-as-zero/writes-ignored", or "reads as written", depending on what the guest is expecting. (writes-ignored is the easiest behaviour to implement, but sometimes guests won't work if you do that.)
If you're implementing "reads as written" then the commit message shouldn't say "write only"... But the patch seems to only add code to the register-write function, not to the register-read function. That means we're storing values the guest writes but never doing anything with them. Either throw away the values immediately ("writes ignored") or let the guest read them back ("reads as written").
(Optionally, we can also log the access via LOG_UNIMP to let the user know the guest is trying to use some bit of the device that doesn't work yet.)
thanks
-- PMM
PATCH 10/10 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)
Signed-off-by BALATON Zoltan <[email protected]>
--- 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
Re: [Qemu-devel] [PATCH 10/10] ppc: Add SM501 device in config for ppc and ppcemb targets
Quoted 6 lines · BALATON Zoltan
This 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)
Signed-off-by BALATON Zoltan <[email protected]>
If this is a generic PCI device then shouldn't it go into default-configs/pci.mak ?
thanks
-- PMM
Re: [Qemu-devel] [PATCH 10/10] ppc: Add SM501 device in config for ppc and ppcemb targets
Quoted 10 lines · Peter Maydell
Quoted 6 lines · BALATON Zoltan
This 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)
Signed-off-by BALATON Zoltan <[email protected]>If this is a generic PCI device then shouldn't it go into default-configs/pci.mak ?
This is a multimedia chip usually found on sh4 systems and some ppc boards that's why I've only added it to these but I can move it to pci.mak if you think it's a better place for it.
Thanks again for taking the time to review, if you could answer these questions I can try to prepare a v2.
Re: [Qemu-devel] [PATCH 10/10] ppc: Add SM501 device in config for ppc and ppcemb targets
Quoted 7 lines · BALATON Zoltan
Quoted 6 lines · Peter Maydell
Quoted 5 lines · BALATON Zoltan
This 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)
Quoted 8 lines
Quoted 4 lines
Quoted 1 line
Signed-off-by BALATON Zoltan <[email protected]>If this is a generic PCI device then shouldn't it go into default-configs/pci.mak ?
This is a multimedia chip usually found on sh4 systems and some ppc boards that's why I've only added it to these but I can move it to pci.mak if you think it's a better place for it.
If it is not really usable for other boards, then I think it should also not go into pci.mak. No need to confuse the users of the other boards with unusable PCI cards here.
Thomas
Quoted 26 lines · BALATON Zoltan
This series improves the sm501 display controller emulation fixing endianness problems that caused mixed up colors in LE hosts, fix hardware cursor and adding panel layer support and some missing registers. The first few patches update the code style and QOMify the device before changes are made to it in subsequent patches.
Including qemu-trivial list as well, both because some of the patches are trivial and also I'm not sure how actively maintained this part is so that also may need attention from the trivial list to get this merged.
The changes were tested with sh4 image at
https://people.debian.org/~aurel32/qemu/sh4/
which accepts video= kernel parameter to excercise different screen modes.BALATON Zoltan (10):
sm501: Fixed code style and a few typos in comments
sm501: Use defines instead of constants where available
sm501: QOMify
sm501: Add emulation of chip connected via PCI
sm501: Add missing arbitration control register
sm501: Fix device endianness
sm501: Fix hardware cursor
sm501: Add support for panel layer
sm501: Add some more missing registers
ppc: Add SM501 device in config for ppc and ppcemb targets
Quoted 7 lines
default-configs/ppc-softmmu.mak | 1 + default-configs/ppcemb-softmmu.mak | 1 + hw/display/sm501.c | 1546 ++++++++++++++++++++---------------- hw/display/sm501_template.h | 92 +-- hw/sh4/r2d.c | 11 +- include/hw/devices.h | 5 - 6 files changed, 920 insertions(+), 736 deletions(-)
Ping? Is there anyone who could take a look and merge this? It could get tested more during the freeze and could be reverted any time if found to cause any problems but if it's not merged now it may get delayed for months until the next opportunity. Patchwork links:
http://patchwork.ozlabs.org/patch/729575/
http://patchwork.ozlabs.org/patch/729570/
http://patchwork.ozlabs.org/patch/729571/
http://patchwork.ozlabs.org/patch/729574/
http://patchwork.ozlabs.org/patch/729576/
http://patchwork.ozlabs.org/patch/729577/
http://patchwork.ozlabs.org/patch/729572/
http://patchwork.ozlabs.org/patch/729573/
http://patchwork.ozlabs.org/patch/729569/
http://patchwork.ozlabs.org/patch/729568/