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

[Blue Swirl]

On 5/11/10, Shin-ichiro KAWASAKI <address@hidden> wrote:
> Hello Blue Swirl, and thank you for the review.
>
>  Here's the patch modified according to your comments.
>  abort() is used instead of assert(), and const modifier added for
> CPU*MemoryFunc.

The patch does not apply. It seems to be based on some version before
61d3cf93e2676282ba1a8d568b2406257f208b26.

>
>  Best Regards,
>  Shin-ichiro KAWASAKI
>
> ---------------------------------------------------------------------------
>
>  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 |  172
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>   1 files changed, 169 insertions(+), 3 deletions(-)
>
>  diff --git a/hw/sm501.c b/hw/sm501.c
>  index cd1f595..ade88ff 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;

While at it, could you remove the underscore prefix from the field names?

>  +
>   } SM501State;
>   static uint32_t get_local_mem_size_index(uint32_t size)
>  @@ -617,6 +629,69 @@ 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;
>  +
>  +    if (addressing != 0x0) {
>  +        printf("%s: only XY addressing is supported.\n", __func__);
>  +        abort();
>  +    }
>  +
>  +    if ((s->_2d_source_base & 0x08000000) ||
>  +        (s->_2d_destination_base & 0x08000000)) {
>  +        printf("%s: only local memory is supported.\n", __func__);
>  +        abort();
>  +    }
>  +
>  +    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);
>  +        abort();
>  +        break;
>  +    }
>  +}
>  +
>   static uint32_t sm501_system_config_read(void *opaque, target_phys_addr_t
> addr)
>   {
>      SM501State * s = (SM501State *)opaque;
>  @@ -967,6 +1042,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);
>  +        abort();
>
>  +    }
>  +
>  +    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);
>  +        abort();
>  +    }
>  +}
>  +
>  +static CPUReadMemoryFunc * const sm501_2d_engine_readfn[] = {
>  +    NULL,
>  +    NULL,
>  +    &sm501_2d_engine_read,
>  +};
>  +
>  +static CPUWriteMemoryFunc * const sm501_2d_engine_writefn[] = {
>  +    NULL,
>  +    NULL,
>  +    &sm501_2d_engine_write,
>  +};
>  +
>   /* draw line functions for all console modes */
>   #include "pixel_ops.h"
>  @@ -1192,6 +1353,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 +1382,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,
>