[v2] sh: sm501: add 2D engine support

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.

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 <kawasaki@juno.dti.ne.jp>

     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(-)

On 5/11/10, Shin-ichiro KAWASAKI <kawasaki@juno.dti.ne.jp> 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 <kawasaki@juno.dti.ne.jp>
>
>     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,
>

(2010/05/15 5:44), Blue Swirl wrote:
> On 5/11/10, Shin-ichiro KAWASAKI<kawasaki@juno.dti.ne.jp>  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.

Ah, I've not yet pulled newest one.

>>    *   - 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?
>

OK.  I'll rename them, from _2d_* into twoD_*.

Thank you for the review.

Best Regards,
Shin-ichiro KAWASAKI