Re: [Qemu-devel] [RFC v2 23/23] 40p: Add an IBM 8514/A graphics card

[Blue Swirl]

On Sun, Jun 19, 2011 at 9:40 PM, Andreas Färber <address@hidden> wrote:
> Am 19.06.2011 um 15:27 schrieb Blue Swirl:
>
>> On Sun, Jun 19, 2011 at 1:04 PM, Andreas Färber <address@hidden>
>> wrote:
>>>
>>> Am 18.06.2011 um 22:42 schrieb Blue Swirl:
>>>
>>>> On Thu, Jun 16, 2011 at 3:02 AM, Andreas Färber <address@hidden>
>>>> wrote:
>>>>>
>>>>> The IBM E15 is equivalent to an S3 Vision864.
>>>>>
>>>>> Lacking S3 SDAC (86C716) support, the DAC indizes are translated
>>>>> to greyscale colors. This works sufficiently to observe firmware
>>>>> boot progress.
>>>>>
>>>>> Cc: Hervé Poussineau <address@hidden>
>>>>>
>>>>> Fixed off-by-one drawing issue.
>>>>> Replaced hardcoded color for RECT.
>>>>> Separate I/O debug output for readability.
>>>>> Start cleaning up the naming s3 vs. ibm8514.
>>>>> Prepare support for DAC_MASK, DAC_R_INDEX, DAC_W_INDEX, DAC_DATA regs.
>>>>>
>>>>> Cc: Roy Tam <address@hidden>
>>>>> Signed-off-by: Andreas Färber <address@hidden>
>>>>> ---
>>>>>  Makefile.objs                   |    1 +
>>>>>  default-configs/ppc-softmmu.mak |    1 +
>>>>>  hw/pci_ids.h                    |    3 +
>>>>>  hw/ppc_prep.c                   |    2 +
>>>>>  hw/vga-ibm8514.c                |  780
>>>>> +++++++++++++++++++++++++++++++++++++++
>>>>>  5 files changed, 787 insertions(+), 0 deletions(-)
>>>>>  create mode 100644 hw/vga-ibm8514.c
>>>
>>>>> diff --git a/hw/vga-ibm8514.c b/hw/vga-ibm8514.c
>>>>> new file mode 100644
>>>>> index 0000000..a87afe1
>>>>> --- /dev/null
>>>>> +++ b/hw/vga-ibm8514.c
>
>>>>> +static void my_update_display(void *opaque)
>>>>> +{
>>>>> +    VGACommonState *s = opaque;
>>>>> +    int w;
>>>>> +    uint8_t *vram;
>>>>> +    uint8_t *data_display, *dd;
>>>>> +    int x, y;
>>>>> +    unsigned int (*rgb_to_pixel)(unsigned int r, unsigned int g,
>>>>> unsigned int b);
>>>>> +
>>>>> +    if (ds_get_width(s->ds) != 640 || ds_get_height(s->ds) != 480) {
>>>>> +        qemu_console_resize(s->ds, 640, 480);
>>>>> +    }
>>>>> +
>>>>> +    switch (ds_get_bits_per_pixel(s->ds)) {
>>>>> +        case 8:
>>>>> +            rgb_to_pixel = rgb_to_pixel8;
>>>>> +            w = 1;
>>>>> +            break;
>>>>> +        case 15:
>>>>> +            rgb_to_pixel = rgb_to_pixel15;
>>>>> +            w = 2;
>>>>> +            break;
>>>>> +        case 16:
>>>>> +            rgb_to_pixel = rgb_to_pixel16;
>>>>> +            w = 2;
>>>>> +            break;
>>>>> +        case 32:
>>>>> +            rgb_to_pixel = rgb_to_pixel32;
>>>>> +            w = 4;
>>>>> +            break;
>>>>> +        default:
>>>>> +            BADF("unknown host depth %d\n",
>>>>> ds_get_bits_per_pixel(s->ds));
>>>>> +            return;
>>>>> +    }
>>>>> +
>>>>> +    vram = s->vram_ptr;
>>>>> +    /* XXX: out of range in vram? */
>>>>> +    data_display = dd = ds_get_data(s->ds);
>>>>> +    for (y = 0; y < 480; y++) {
>>>>> +        for (x = 0; x < 640; x++) {
>>>>> +            unsigned int color;
>>>>> +            color = (*rgb_to_pixel)(vram[0], vram[1], vram[2]);
>>>>> +            memcpy(dd, &color, w);
>>>>
>>>> Please take a look at tcx.c for a 8 bit mode frame buffer with palette
>>>> translation. Also VGA_DIRTY bit handling should be added to this loop
>>>> to speed it up.
>>>
>>> Will look into it.
>>>
>>> I doubt this is causing the long delays though.
>>
>> The difference is that only areas which have been written after last
>> update are copied to display instead of updating the whole screen
>> every time. IIRC for TCX it was a major speedup.
>
> Some remarks:
>
> * TCX seems to assume that TARGET_HOST_PAGE == 4 * 1024 in the unrolled loop
> in tcx_update_display(). You might want to assert that.

TCX was only used on Sparc32. Though some Ultra machines had SBus,
there the page size would be 8k (also bigger pages, but that is what
QEMU uses for TARGET_PAGE_SIZE).

> * The check page < page_min with page_min == -1 seems unintended in
> tcx_update_display() and would have undesired effects on resetting the dirty
> bit. I used page_min < 0 instead.

No, the check is correct since the type of page_min is ram_addr_t
(unsigned long).

> * The reset code seems wrong wrt dirty bit: after zero-ing VRAM the dirty
> bit should be set, not reset. Otherwise the screen is not redrawn.

Actually reset should not touch VRAM, the screen is cleared by OpenBIOS.