[PATCH v11 00/17] ati-vga: Implement HOST_DATA transfers to enable X.org text rendering

Chad Jablonski posted 17 patches 3 weeks, 2 days ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20260303024730.1489136-1-chad@jablonski.xyz
There is a newer version of this series
hw/display/ati.c      | 137 ++++++++++++-
hw/display/ati_2d.c   | 462 +++++++++++++++++++++++++++++-------------
hw/display/ati_dbg.c  |   9 +
hw/display/ati_int.h  |  26 ++-
hw/display/ati_regs.h |  20 ++
5 files changed, 509 insertions(+), 145 deletions(-)
[PATCH v11 00/17] ati-vga: Implement HOST_DATA transfers to enable X.org text rendering
Posted by Chad Jablonski 3 weeks, 2 days ago
This series implements HOST_DATA as a blit source enabling text rendering in
xterm under X.org with 2D acceleration.

The series builds up functionality incrementally:

* Patches   1-6:  Bug fixes and register implementations
* Patches  7-14:  Refactor of ati_2d_blt to decouple from ATIVGAState
* Patch      15:  Scissor clipping implementation
* Patches 16-17:  HOST_DATA register writes, color expansion, and
                  accumulator flush

Tested with xterm on X.org using the r128 driver built with
--disable-dri (MMIO mode).

Hardware tests can be found at:
https://codeberg.org/cjab/ati-tests/src/commit/32ed42625cb3f094d4a3d3ce3ad26cfa5f872ac2/tests/common/host_data.c

Changes from v10:
- Rename ati_{flush,finish}_host_data to ati_host_data_{flush,finish}
- Use sizeof accumulator when flushing SRC_COLOR datatype
- Simplify ati_host_data_finish by flushing and immediately ending the blit
- Combine HOST_DATA* and HOST_DATA_LAST handlers
- Reset host_data.next within ati_host_data_flush

Changes from v9:
- Remove unneeded FIXME comment in ati_2d_do_blt
- ati_set_dirty is no longer called when the blit exits early
- Drop patch 15 moving src bounds validation to ati_2d_blt
- Check src vram bounds using clipped values, only when host_data is inactive
- Return to 128-bit accumulator for simplicity
- Small tweak to color expansion to avoid a multiply

Changes from v8:
- Rename tmp_stride to tmp_stride_words
- Small formatting tweaks
- Add clipping debug logging
- Replace extract8 with BIT masking in expansion hot path
- Add SRC_COLOR support
- Fix SRC_COLOR value (was 0x20000, should have been 0x30000)
- Add partial SRC_MONO_FRGD support
- Src bound validation now only done on ROP3_SRCCOPY blits
- Other improved log messages
- Host data accumulator expanded from 128-bits to 256-bits based on hardware
  testing. Real hardware flushes after 128-bits but early HOST_DATA_LAST writes
  make use of the entire 256-bits to finish the blit.
- Added ati_finish_host_data to match hardware in the early HOST_DATA_LAST case
- Added active flag for host_data blits ignoring HOST_DATA writes
  when no blit is active.
- Removed ati_host_data_reset. This is now handled either when flushing or a new
  blit is started.

Changes from v7:
- Avoid potential issue with dst_offset calculation (found by Zoltan)
- Remove unnecessary newlines and other small formatting fixes
- Squash patch 15 into patches 13 and 14
- Note: Zoltan's suggestion to embed ATI2DCtx in ATIVGAState will be
  revisited in a follow-up series

Changes from v6:
- Reduce churn by adding ctx_ struct and pointer
- Moved extraction of ati_set_dirty helper earlier in the series
- Moved register logging into setup_2d_blt_ctx
- Remove unnecessary comments

Changes from v5:
- Added previously standalone aperture size patch to series
- Introduced ATI2DCtx to hold all blit state (suggested by Zoltan)
- Made refactor patches more granular
- Simplified DP_GUI_MASTER_CNTL bit shifting
- Sorted clipping regs by offsets
- Removed dst_x/y update after blit (confirmed by hardware testing)

Changes from v4:
- Refactored ati_2d_blt to accept src and dst state
- Implemented host_data blits on top of the refactored ati_2d_blt
  (suggested by Zoltan)
- Dropped separate field storage for DP_DATATYPE/DP_MIX (per Zoltan)

Changes from v3:
- New patch 1 fixing DP_DATATYPE/MIX register aliasing, this supersedes old patch 5
- Fix MSB/LSB HOST_DATA bit ordering, it is per-byte rather than per-word
- Fixed a missing break in register write handler
- Squashed and reorganized the dst/scissor helper patches (per Zoltan)
- Simplified register field constants and increment logic (per Zoltan)
- Tested with MorphOS and TVPaint (opaque bitmap fonts now work)

Changes from v2:
- Verified with hardware that clipping default bit latches
- Verified with hardware that pitch/offset default bits latch
  (supersedes Zoltan's "ati-vga: Separate default control bit for
  source")
- A new approach to HOST_DATA using a 128-bit accumulator instead of a
  large buffer
- Fixed a longstanding bug in (DST/SRC)_PITCH reads that zeroed pitch value.
- Removed mask from the DP_GUI_MASTER_CNTL write, all fields are
  important
- Created helpers for code shared between ati_2d_blt and
  ati_host_data_flush

Changes from v1:
- Split monolithic patch into 7 logical patches as requested
- Integrate HOST_DATA blit into existing ati_2d_blt()
- Add fallback implementation for non-pixman builds
- Remove unnecessary initialization in realize (kept in reset only)
- Fixed DP_GUI_MASTER_CNTL mask to preserve GMC_SRC_SOURCE field
- Implement scissor clipping

Chad Jablonski (17):
  ati-vga: Fix framebuffer mapping by using hardware-correct aperture
    sizes
  ati-vga: Fix DST_PITCH and SRC_PITCH reads
  ati-vga: Read aliased values from DP_GUI_MASTER_CNTL
  ati-vga: Latch src and dst pitch and offset on master_cntl default
  ati-vga: Implement foreground and background color register writes
  ati-vga: Add scissor clipping register support
  ati-vga: Remove dst_x/y updates after blit
  ati-vga: Consolidate dirty region tracking in ati_2d_blt
  ati-vga: Remove src and dst stride mutation in ati_2d_blt
  ati-vga: Use local variables for register values in ati_2d_blt
  ati-vga: Introduce ATI2DCtx struct for 2D blit context
  ati-vga: Extract setup_2d_blt_ctx from ati_2d_blt
  ati-vga: Split ati_2d_do_blt from ati_2d_blt
  ati-vga: Remove ATIVGAState param from ati_2d_do_blt
  ati-vga: Implement scissor rectangle clipping for 2D operations
  ati-vga: Implement HOST_DATA register writes
  ati-vga: Implement HOST_DATA flush to VRAM

 hw/display/ati.c      | 137 ++++++++++++-
 hw/display/ati_2d.c   | 462 +++++++++++++++++++++++++++++-------------
 hw/display/ati_dbg.c  |   9 +
 hw/display/ati_int.h  |  26 ++-
 hw/display/ati_regs.h |  20 ++
 5 files changed, 509 insertions(+), 145 deletions(-)

-- 
2.52.0
Re: [PATCH v11 00/17] ati-vga: Implement HOST_DATA transfers to enable X.org text rendering
Posted by Philippe Mathieu-Daudé 2 weeks, 4 days ago
Hi Chad,

On 3/3/26 03:47, Chad Jablonski wrote:
> This series implements HOST_DATA as a blit source enabling text rendering in
> xterm under X.org with 2D acceleration.
> 
> The series builds up functionality incrementally:
> 
> * Patches   1-6:  Bug fixes and register implementations
> * Patches  7-14:  Refactor of ati_2d_blt to decouple from ATIVGAState
> * Patch      15:  Scissor clipping implementation
> * Patches 16-17:  HOST_DATA register writes, color expansion, and
>                    accumulator flush


> Chad Jablonski (17):
>    ati-vga: Fix framebuffer mapping by using hardware-correct aperture
>      sizes
>    ati-vga: Fix DST_PITCH and SRC_PITCH reads
>    ati-vga: Read aliased values from DP_GUI_MASTER_CNTL
>    ati-vga: Latch src and dst pitch and offset on master_cntl default
>    ati-vga: Implement foreground and background color register writes
>    ati-vga: Add scissor clipping register support
>    ati-vga: Remove dst_x/y updates after blit
>    ati-vga: Consolidate dirty region tracking in ati_2d_blt
>    ati-vga: Remove src and dst stride mutation in ati_2d_blt
>    ati-vga: Use local variables for register values in ati_2d_blt
>    ati-vga: Introduce ATI2DCtx struct for 2D blit context
>    ati-vga: Extract setup_2d_blt_ctx from ati_2d_blt
>    ati-vga: Split ati_2d_do_blt from ati_2d_blt
>    ati-vga: Remove ATIVGAState param from ati_2d_do_blt
>    ati-vga: Implement scissor rectangle clipping for 2D operations
>    ati-vga: Implement HOST_DATA register writes
>    ati-vga: Implement HOST_DATA flush to VRAM

Zoltan asked me to look at your series [*] but unfortunately I'm
getting a build failure when building without PIXMAN starting
with patch #9:

../hw/display/ati_2d.c: In function ‘ati_2d_do_blt’:
../hw/display/ati_2d.c:185:13: error: unused variable ‘src_stride_words’ 
[-Werror=unused-variable]
   185 |         int src_stride_words = ctx->src_stride / sizeof(uint32_t);
       |             ^~~~~~~~~~~~~~~~
../hw/display/ati_2d.c:147:9: error: unused variable ‘dst_stride_words’ 
[-Werror=unused-variable]
   147 |     int dst_stride_words = ctx->dst_stride / sizeof(uint32_t);
       |         ^~~~~~~~~~~~~~~~
../hw/display/ati_2d.c:137:10: error: unused variable ‘use_pixman_blt’ 
[-Werror=unused-variable]
   137 |     bool use_pixman_blt = use_pixman & BIT(1);
       |          ^~~~~~~~~~~~~~
../hw/display/ati_2d.c:136:10: error: unused variable ‘use_pixman_fill’ 
[-Werror=unused-variable]
   136 |     bool use_pixman_fill = use_pixman & BIT(0);
       |          ^~~~~~~~~~~~~~~

I'm queueing the first 8 patches hoping it helps.

Also, please fix the checkpatch.pl coding style errors
before re-posting.

Regards,

Phil.

[*] 
https://lore.kernel.org/qemu-devel/7f1df452-05c2-efa3-fdce-a5d987cfdcc8@eik.bme.hu/


Re: [PATCH v11 00/17] ati-vga: Implement HOST_DATA transfers to enable X.org text rendering
Posted by BALATON Zoltan 2 weeks, 4 days ago
On Sun, 8 Mar 2026, Philippe Mathieu-Daudé wrote:
> Hi Chad,
>
> On 3/3/26 03:47, Chad Jablonski wrote:
>> This series implements HOST_DATA as a blit source enabling text rendering 
>> in
>> xterm under X.org with 2D acceleration.
>> 
>> The series builds up functionality incrementally:
>> 
>> * Patches   1-6:  Bug fixes and register implementations
>> * Patches  7-14:  Refactor of ati_2d_blt to decouple from ATIVGAState
>> * Patch      15:  Scissor clipping implementation
>> * Patches 16-17:  HOST_DATA register writes, color expansion, and
>>                    accumulator flush
>
>
>> Chad Jablonski (17):
>>    ati-vga: Fix framebuffer mapping by using hardware-correct aperture
>>      sizes
>>    ati-vga: Fix DST_PITCH and SRC_PITCH reads
>>    ati-vga: Read aliased values from DP_GUI_MASTER_CNTL
>>    ati-vga: Latch src and dst pitch and offset on master_cntl default
>>    ati-vga: Implement foreground and background color register writes
>>    ati-vga: Add scissor clipping register support
>>    ati-vga: Remove dst_x/y updates after blit
>>    ati-vga: Consolidate dirty region tracking in ati_2d_blt
>>    ati-vga: Remove src and dst stride mutation in ati_2d_blt
>>    ati-vga: Use local variables for register values in ati_2d_blt
>>    ati-vga: Introduce ATI2DCtx struct for 2D blit context
>>    ati-vga: Extract setup_2d_blt_ctx from ati_2d_blt
>>    ati-vga: Split ati_2d_do_blt from ati_2d_blt
>>    ati-vga: Remove ATIVGAState param from ati_2d_do_blt
>>    ati-vga: Implement scissor rectangle clipping for 2D operations
>>    ati-vga: Implement HOST_DATA register writes
>>    ati-vga: Implement HOST_DATA flush to VRAM
>
> Zoltan asked me to look at your series [*] but unfortunately I'm
> getting a build failure when building without PIXMAN starting
> with patch #9:
>
> ../hw/display/ati_2d.c: In function ‘ati_2d_do_blt’:
> ../hw/display/ati_2d.c:185:13: error: unused variable ‘src_stride_words’ 
> [-Werror=unused-variable]
>  185 |         int src_stride_words = ctx->src_stride / sizeof(uint32_t);
>      |             ^~~~~~~~~~~~~~~~
> ../hw/display/ati_2d.c:147:9: error: unused variable ‘dst_stride_words’ 
> [-Werror=unused-variable]
>  147 |     int dst_stride_words = ctx->dst_stride / sizeof(uint32_t);
>      |         ^~~~~~~~~~~~~~~~
> ../hw/display/ati_2d.c:137:10: error: unused variable ‘use_pixman_blt’ 
> [-Werror=unused-variable]
>  137 |     bool use_pixman_blt = use_pixman & BIT(1);
>      |          ^~~~~~~~~~~~~~
> ../hw/display/ati_2d.c:136:10: error: unused variable ‘use_pixman_fill’ 
> [-Werror=unused-variable]
>  136 |     bool use_pixman_fill = use_pixman & BIT(0);
>      |          ^~~~~~~~~~~~~~~
>
> I'm queueing the first 8 patches hoping it helps.
>
> Also, please fix the checkpatch.pl coding style errors
> before re-posting.

Sorry, my mistake I did not notice this during review and testing. I'm 
fixing it up and will send updated version of the last part of the series 
not yet queued. Thanks a lot for your help.

Regards,
BALATON Zoltan
Re: [PATCH v11 00/17] ati-vga: Implement HOST_DATA transfers to enable X.org text rendering
Posted by BALATON Zoltan 2 weeks, 4 days ago
On Mon, 9 Mar 2026, BALATON Zoltan wrote:
> On Sun, 8 Mar 2026, Philippe Mathieu-Daudé wrote:
>> Hi Chad,
>> 
>> On 3/3/26 03:47, Chad Jablonski wrote:
>>> This series implements HOST_DATA as a blit source enabling text rendering 
>>> in
>>> xterm under X.org with 2D acceleration.
>>> 
>>> The series builds up functionality incrementally:
>>> 
>>> * Patches   1-6:  Bug fixes and register implementations
>>> * Patches  7-14:  Refactor of ati_2d_blt to decouple from ATIVGAState
>>> * Patch      15:  Scissor clipping implementation
>>> * Patches 16-17:  HOST_DATA register writes, color expansion, and
>>>                    accumulator flush
>> 
>> 
>>> Chad Jablonski (17):
>>>    ati-vga: Fix framebuffer mapping by using hardware-correct aperture
>>>      sizes
>>>    ati-vga: Fix DST_PITCH and SRC_PITCH reads
>>>    ati-vga: Read aliased values from DP_GUI_MASTER_CNTL
>>>    ati-vga: Latch src and dst pitch and offset on master_cntl default
>>>    ati-vga: Implement foreground and background color register writes
>>>    ati-vga: Add scissor clipping register support
>>>    ati-vga: Remove dst_x/y updates after blit
>>>    ati-vga: Consolidate dirty region tracking in ati_2d_blt
>>>    ati-vga: Remove src and dst stride mutation in ati_2d_blt
>>>    ati-vga: Use local variables for register values in ati_2d_blt
>>>    ati-vga: Introduce ATI2DCtx struct for 2D blit context
>>>    ati-vga: Extract setup_2d_blt_ctx from ati_2d_blt
>>>    ati-vga: Split ati_2d_do_blt from ati_2d_blt
>>>    ati-vga: Remove ATIVGAState param from ati_2d_do_blt
>>>    ati-vga: Implement scissor rectangle clipping for 2D operations
>>>    ati-vga: Implement HOST_DATA register writes
>>>    ati-vga: Implement HOST_DATA flush to VRAM
>> 
>> Zoltan asked me to look at your series [*] but unfortunately I'm
>> getting a build failure when building without PIXMAN starting
>> with patch #9:
>> 
>> ../hw/display/ati_2d.c: In function ‘ati_2d_do_blt’:
>> ../hw/display/ati_2d.c:185:13: error: unused variable ‘src_stride_words’ 
>> [-Werror=unused-variable]
>>  185 |         int src_stride_words = ctx->src_stride / sizeof(uint32_t);
>>      |             ^~~~~~~~~~~~~~~~
>> ../hw/display/ati_2d.c:147:9: error: unused variable ‘dst_stride_words’ 
>> [-Werror=unused-variable]
>>  147 |     int dst_stride_words = ctx->dst_stride / sizeof(uint32_t);
>>      |         ^~~~~~~~~~~~~~~~
>> ../hw/display/ati_2d.c:137:10: error: unused variable ‘use_pixman_blt’ 
>> [-Werror=unused-variable]
>>  137 |     bool use_pixman_blt = use_pixman & BIT(1);
>>      |          ^~~~~~~~~~~~~~
>> ../hw/display/ati_2d.c:136:10: error: unused variable ‘use_pixman_fill’ 
>> [-Werror=unused-variable]
>>  136 |     bool use_pixman_fill = use_pixman & BIT(0);
>>      |          ^~~~~~~~~~~~~~~
>> 
>> I'm queueing the first 8 patches hoping it helps.
>> 
>> Also, please fix the checkpatch.pl coding style errors
>> before re-posting.
>
> Sorry, my mistake I did not notice this during review and testing. I'm fixing 
> it up and will send updated version of the last part of the series not yet 
> queued. Thanks a lot for your help.

I sent updated v12 fixing the above. Before that I've also sent this small 
fix independent of this series:
https://patchew.org/QEMU/cover.1773009887.git.balaton@eik.bme.hu/

Thank you,
BALATON Zoltan