Re: [Qemu-devel] [PATCH] sh: sm501: add 2D engine support

[Blue Swirl]

On 5/9/10, Shin-ichiro KAWASAKI <address@hidden> wrote:
> In linux kernel v2.6.33, sm501 frame buffer driver modified to support
>  2D graphics engine on sm501 chip.  One example is "fill rectangle" operation.
>  But current qemu's sm501 emulation doesn't support it.  This results in
>  graphics console disturbance.
>
>  This patch introduces sm501 2D graphics engine emulation and solve this 
> problem.
>
>
>  Signed-off-by: Shin-ichiro KAWASAKI <address@hidden>
>
>     Add SM501 2D hardware engine support.
>
>      - Add 2D engine register set read/write handlers.
>      - Support 'fill rectangle'. Other operations are left for future work.
>      - Update SM501 support status comment.
>
>  ---
>   hw/sm501.c |  168 
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>   1 files changed, 165 insertions(+), 3 deletions(-)
>
>  diff --git a/hw/sm501.c b/hw/sm501.c
>  index cd1f595..2f1610b 100644
>  --- a/hw/sm501.c
>  +++ b/hw/sm501.c
>  @@ -29,16 +29,16 @@
>   #include "devices.h"
>
>   /*
>  - * Status: 2008/11/02
>  + * Status: 2010/05/07
>   *   - Minimum implementation for Linux console : mmio regs and CRT layer.
>  - *   - Always updates full screen.
>  + *   - 2D grapihcs acceleration partially supported : only fill rectangle.
>   *
>   * TODO:
>   *   - Panel support
>  - *   - Hardware cursor support
>   *   - Touch panel support
>   *   - USB support
>   *   - UART support
>  + *   - More 2D graphics engine support
>   *   - Performance tuning
>   */
>
>  @@ -508,6 +508,18 @@ typedef struct SM501State {
>      uint32_t dc_crt_hwc_color_1_2;
>      uint32_t dc_crt_hwc_color_3;
>
>  +    uint32_t _2d_destination;
>  +    uint32_t _2d_dimension;
>  +    uint32_t _2d_control;
>  +    uint32_t _2d_pitch;
>  +    uint32_t _2d_foreground;
>  +    uint32_t _2d_stretch;
>  +    uint32_t _2d_color_compare_mask;
>  +    uint32_t _2d_mask;
>  +    uint32_t _2d_window_width;
>  +    uint32_t _2d_source_base;
>  +    uint32_t _2d_destination_base;
>  +
>   } SM501State;
>
>   static uint32_t get_local_mem_size_index(uint32_t size)
>  @@ -617,6 +629,65 @@ 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)
>  +{
>  +    /* obtain operation parameters */
>  +    int operation = (s->_2d_control >> 16) & 0x1f;
>  +    int dst_x = (s->_2d_destination >> 16) & 0x01FFF;
>  +    int dst_y = s->_2d_destination & 0xFFFF;
>  +    int operation_width = (s->_2d_dimension >> 16) & 0x1FFF;
>  +    int operation_height = s->_2d_dimension & 0xFFFF;
>  +    uint32_t color = s->_2d_foreground;
>  +    int format_flags = (s->_2d_stretch >> 20) & 0x3;
>  +    int addressing = (s->_2d_stretch >> 16) & 0xF;
>  +
>  +    /* get frame buffer info */
>  +#if 0 /* for future use */
>  +    uint8_t * src = s->local_mem + (s->_2d_source_base & 0x03FFFFFF);
>  +#endif
>  +    uint8_t * dst = s->local_mem + (s->_2d_destination_base & 0x03FFFFFF);
>  +    int dst_width = (s->dc_crt_h_total & 0x00000FFF) + 1;
>  +
>  +    /* only XY addressing is supported */
>  +    assert(addressing == 0x0);
>  +
>  +    /* only local memory is supporetd */
>  +    assert(!(s->_2d_source_base & 0x08000000));
>  +    assert(!(s->_2d_destination_base & 0x08000000));

In general, assertions which can be triggered by the guest are bad.

>  +
>  +    switch (operation) {
>  +    case 0x01: /* fill rectangle */
>  +
>  +#define FILL_RECT(_bpp, _pixel_type) {                                      
> \
>  +        int y, x;                                                           
> \
>  +        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;            
> \
>  +            }                                                               
> \
>  +        }                                                                   
> \
>  +    }
>  +
>  +        switch (format_flags) {
>  +        case 0:
>  +            FILL_RECT(1, uint8_t);
>  +            break;
>  +        case 1:
>  +            FILL_RECT(2, uint16_t);
>  +            break;
>  +        case 2:
>  +            FILL_RECT(4, uint32_t);
>  +            break;
>  +        }
>  +        break;
>  +
>  +    default:
>  +        printf("non-implemented SM501 2D operation. %d\n", operation);
>  +        assert(0);
>  +        break;
>  +    }
>  +}
>  +
>   static uint32_t sm501_system_config_read(void *opaque, target_phys_addr_t 
> addr)
>   {
>      SM501State * s = (SM501State *)opaque;
>  @@ -967,6 +1038,92 @@ static CPUWriteMemoryFunc * const 
> sm501_disp_ctrl_writefn[] = {
>      &sm501_disp_ctrl_write,
>   };
>
>  +static uint32_t sm501_2d_engine_read(void *opaque, target_phys_addr_t addr)
>  +{
>  +    SM501State * s = (SM501State *)opaque;
>  +    uint32_t ret = 0;
>  +    SM501_DPRINTF("sm501 2d engine regs : read addr=%x\n", (int)addr);
>  +
>  +    switch(addr) {
>  +    case SM501_2D_SOURCE_BASE:
>  +        ret = s->_2d_source_base;
>  +        break;
>  +    default:
>  +        printf("sm501 disp ctrl : not implemented register read."
>  +               " addr=%x\n", (int)addr);
>  +        assert(0);

Please use abort() instead of assert(0).

>  +    }
>  +
>  +    return ret;
>  +}
>  +
>  +static void sm501_2d_engine_write(void *opaque,
>  +                                  target_phys_addr_t addr, uint32_t value)
>  +{
>  +    SM501State * s = (SM501State *)opaque;
>  +    SM501_DPRINTF("sm501 2d engine regs : write addr=%x, val=%x\n",
>  +                  addr, value);
>  +
>  +    switch(addr) {
>  +    case SM501_2D_DESTINATION:
>  +        s->_2d_destination = value;
>  +        break;
>  +    case SM501_2D_DIMENSION:
>  +        s->_2d_dimension = value;
>  +        break;
>  +    case SM501_2D_CONTROL:
>  +        s->_2d_control = value;
>  +
>  +        /* do 2d operation if start flag is set. */
>  +        if (value & 0x80000000) {
>  +            sm501_2d_operation(s);
>  +            s->_2d_control &= ~0x80000000; /* start flag down */
>  +        }
>  +
>  +        break;
>  +    case SM501_2D_PITCH:
>  +        s->_2d_pitch = value;
>  +        break;
>  +    case SM501_2D_FOREGROUND:
>  +        s->_2d_foreground = value;
>  +        break;
>  +    case SM501_2D_STRETCH:
>  +        s->_2d_stretch = value;
>  +        break;
>  +    case SM501_2D_COLOR_COMPARE_MASK:
>  +        s->_2d_color_compare_mask = value;
>  +        break;
>  +    case SM501_2D_MASK:
>  +        s->_2d_mask = value;
>  +        break;
>  +    case SM501_2D_WINDOW_WIDTH:
>  +        s->_2d_window_width = value;
>  +        break;
>  +    case SM501_2D_SOURCE_BASE:
>  +        s->_2d_source_base = value;
>  +        break;
>  +    case SM501_2D_DESTINATION_BASE:
>  +        s->_2d_destination_base = value;
>  +        break;
>  +    default:
>  +        printf("sm501 2d engine : not implemented register write."
>  +               " addr=%x, val=%x\n", (int)addr, value);
>  +        assert(0);
>  +    }
>  +}
>  +
>  +static CPUReadMemoryFunc *sm501_2d_engine_readfn[] = {

Please make CPU*MemoryFunc structs 'const'.

>  +    NULL,
>  +    NULL,
>  +    &sm501_2d_engine_read,
>  +};
>  +
>  +static CPUWriteMemoryFunc *sm501_2d_engine_writefn[] = {
>  +    NULL,
>  +    NULL,
>  +    &sm501_2d_engine_write,
>  +};
>  +
>   /* draw line functions for all console modes */
>
>   #include "pixel_ops.h"
>  @@ -1192,6 +1349,7 @@ void sm501_init(uint32_t base, uint32_t 
> local_mem_bytes, qemu_irq irq,
>      SM501State * s;
>      int sm501_system_config_index;
>      int sm501_disp_ctrl_index;
>  +    int sm501_2d_engine_index;
>
>      /* allocate management data region */
>      s = (SM501State *)qemu_mallocz(sizeof(SM501State));
>  @@ -1220,6 +1378,10 @@ void sm501_init(uint32_t base, uint32_t 
> local_mem_bytes, qemu_irq irq,
>                                                    sm501_disp_ctrl_writefn, 
> s);
>      cpu_register_physical_memory(base + MMIO_BASE_OFFSET + SM501_DC,
>                                   0x1000, sm501_disp_ctrl_index);
>  +    sm501_2d_engine_index = cpu_register_io_memory(sm501_2d_engine_readfn,
>  +                                                   sm501_2d_engine_writefn, 
> s);
>  +    cpu_register_physical_memory(base + MMIO_BASE_OFFSET + SM501_2D_ENGINE,
>  +                                 0x54, sm501_2d_engine_index);
>
>      /* bridge to usb host emulation module */
>      usb_ohci_init_sm501(base + MMIO_BASE_OFFSET + SM501_USB_HOST, base,
>
>
>