[PATCH v3 0/7] ati-vga: CCE engine setup and PIO packet processing

Chad Jablonski posted 7 patches 1 month, 1 week ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20260606195216.2524314-1-chad@jablonski.xyz
hw/display/ati.c        | 505 +++++++++++++++++++++++++++-------------
hw/display/ati_cce.c    | 154 ++++++++++++
hw/display/ati_int.h    |  73 ++++++
hw/display/ati_regs.h   |  15 ++
hw/display/meson.build  |   3 +-
hw/display/trace-events |   9 +
6 files changed, 598 insertions(+), 161 deletions(-)
create mode 100644 hw/display/ati_cce.c
[PATCH v3 0/7] ati-vga: CCE engine setup and PIO packet processing
Posted by Chad Jablonski 1 month, 1 week ago
This series implements CCE engine setup and PIO packet submission/processing
for the Rage 128 and Radeon. This forms the foundation for full CCE support
for both chips in the future.

CCE engine microcode loading is shared across Rage 128 and Radeon. Microcode
is stored but not executed in this series. Type-0/1/2 packets are handled by
fixed-function hardware and do not require microcode. Ring buffer (BM modes)
and Type 3 packet processing will follow in future series.

There are differences between CCE setup for Rage 128 and Radeon. Patch 2
introduces chip-specific register handlers. The packet processing itself in
patch 7 is reusable for both chips.

The plan for the next series is to tackle BM modes for Radeon. Differences
between the two chips start to become larger in the BM modes.

Hardware tests available at:
r128: https://codeberg.org/cjab/ati-tests/src/commit/1ec2dc95d4ed8d11504723b8298847af9b5be075/tests/r128/cce.c
r100: https://codeberg.org/cjab/ati-tests/src/commit/1ec2dc95d4ed8d11504723b8298847af9b5be075/tests/r100/cce.c

Tested on Rage 128 Pro Ultra (PCI 1002:5446) and
Radeon QY (RV100) (PCI 1002:5159).

Changes from v2:
- MM_INDEX and MM_DATA are not available through ati_reg_write
- Use ati_int.h header instead of creating a new one
- Drop PM4_STAT register implementation for now. It will be easier to more
  accurately implement with BM modes.
- Introduce chip-specific register handlers
- Combine PM4_BUFFER_CNTL and PM4_MICRO_CNTL patches
- Add CCE mode registers for r100
- PM4_MICROCODE_DATAH is one bit wider for r100
- Testing on r100 showed that GUI MMIO is not disabled in CCE mode. Now
  gated for r128 only.
- Small style fixes

Changes from v1:
- Rebased onto master (no longer depends on HOST_DATA series)

Chad Jablonski (7):
  ati-vga: Extract ati_reg_write() from ati_mm_write()
  ati-vga: Chip-specific register handlers
  ati-vga: Implement CCE microcode register handling
  ati-vga: Implement CCE mode registers for r128
  ati-vga: Implement CCE mode registers for r100
  ati-vga: Disable GUI engine MMIO writes when CCE enabled (r128 only)
  ati-vga: Process Type-0/1/2 CCE packets via PIO

 hw/display/ati.c        | 505 +++++++++++++++++++++++++++-------------
 hw/display/ati_cce.c    | 154 ++++++++++++
 hw/display/ati_int.h    |  73 ++++++
 hw/display/ati_regs.h   |  15 ++
 hw/display/meson.build  |   3 +-
 hw/display/trace-events |   9 +
 6 files changed, 598 insertions(+), 161 deletions(-)
 create mode 100644 hw/display/ati_cce.c

-- 
2.53.0
Re: [PATCH v3 0/7] ati-vga: CCE engine setup and PIO packet processing
Posted by BALATON Zoltan 2 weeks, 3 days ago
Hello,

On Sat, 6 Jun 2026, Chad Jablonski wrote:
> This series implements CCE engine setup and PIO packet submission/processing
> for the Rage 128 and Radeon. This forms the foundation for full CCE support
> for both chips in the future.

Sorry, I did not have time to review this but AFAIU this is more of an RFC 
now as it does not work with any drivers yet so you'll follow up with more 
patches anyway.

> CCE engine microcode loading is shared across Rage 128 and Radeon. Microcode
> is stored but not executed in this series. Type-0/1/2 packets are handled by
> fixed-function hardware and do not require microcode. Ring buffer (BM modes)
> and Type 3 packet processing will follow in future series.

I was wondering if anything wants to read the microcode. If not we could 
just ignore the writes and not store the data and just log when something 
tried to read. (Or just do nothing for it as uniplemented register 
accesses are already logged.) Or are you aware something that needs the 
read registers?

> There are differences between CCE setup for Rage 128 and Radeon. Patch 2
> introduces chip-specific register handlers. The packet processing itself in
> patch 7 is reusable for both chips.
>
> The plan for the next series is to tackle BM modes for Radeon. Differences
> between the two chips start to become larger in the BM modes.

So far we handled these differences within the same function with if on 
model. The differences added by this series does not seem large enough to 
not fit in an if to keep the single functions but if we go with separate 
functions instead then maybe the existing ifs should also be removed and 
split into the model specific functions with the common one named 
ati_common_reg_* then? The current series only converts new registers so 
I'd either use ifs with the single handler function or convert everything 
to stay consistent.

Regards,
BALATON Zoltan

> Hardware tests available at:
> r128: https://codeberg.org/cjab/ati-tests/src/commit/1ec2dc95d4ed8d11504723b8298847af9b5be075/tests/r128/cce.c
> r100: https://codeberg.org/cjab/ati-tests/src/commit/1ec2dc95d4ed8d11504723b8298847af9b5be075/tests/r100/cce.c
>
> Tested on Rage 128 Pro Ultra (PCI 1002:5446) and
> Radeon QY (RV100) (PCI 1002:5159).
>
> Changes from v2:
> - MM_INDEX and MM_DATA are not available through ati_reg_write
> - Use ati_int.h header instead of creating a new one
> - Drop PM4_STAT register implementation for now. It will be easier to more
>  accurately implement with BM modes.
> - Introduce chip-specific register handlers
> - Combine PM4_BUFFER_CNTL and PM4_MICRO_CNTL patches
> - Add CCE mode registers for r100
> - PM4_MICROCODE_DATAH is one bit wider for r100
> - Testing on r100 showed that GUI MMIO is not disabled in CCE mode. Now
>  gated for r128 only.
> - Small style fixes
>
> Changes from v1:
> - Rebased onto master (no longer depends on HOST_DATA series)
>
> Chad Jablonski (7):
>  ati-vga: Extract ati_reg_write() from ati_mm_write()
>  ati-vga: Chip-specific register handlers
>  ati-vga: Implement CCE microcode register handling
>  ati-vga: Implement CCE mode registers for r128
>  ati-vga: Implement CCE mode registers for r100
>  ati-vga: Disable GUI engine MMIO writes when CCE enabled (r128 only)
>  ati-vga: Process Type-0/1/2 CCE packets via PIO
>
> hw/display/ati.c        | 505 +++++++++++++++++++++++++++-------------
> hw/display/ati_cce.c    | 154 ++++++++++++
> hw/display/ati_int.h    |  73 ++++++
> hw/display/ati_regs.h   |  15 ++
> hw/display/meson.build  |   3 +-
> hw/display/trace-events |   9 +
> 6 files changed, 598 insertions(+), 161 deletions(-)
> create mode 100644 hw/display/ati_cce.c
>
>
Re: [PATCH v3 0/7] ati-vga: CCE engine setup and PIO packet processing
Posted by Chad Jablonski 22 hours ago
>
> Sorry, I did not have time to review this but AFAIU this is more of an RFC 
> now as it does not work with any drivers yet so you'll follow up with more 
> patches anyway.
>

Thanks for taking another look at this. You're right, this should have been an
RFC.

>
> I was wondering if anything wants to read the microcode. If not we could 
> just ignore the writes and not store the data and just log when something 
> tried to read. (Or just do nothing for it as uniplemented register 
> accesses are already logged.) Or are you aware something that needs the 
> read registers?
>

I haven't run into anything that reads the microcode. I ended up
removing all of this.

>
> So far we handled these differences within the same function with if on 
> model. The differences added by this series does not seem large enough to 
> not fit in an if to keep the single functions but if we go with separate 
> functions instead then maybe the existing ifs should also be removed and 
> split into the model specific functions with the common one named 
> ati_common_reg_* then? The current series only converts new registers so 
> I'd either use ifs with the single handler function or convert everything 
> to stay consistent.
>

After building on this I have to agree with you. It turned out to
be much easier to deal with keeping the differences local to a single
register handler for now.

I'm abandoning this series for a simpler approach that just implements
the CP engine (bus mastering) for the r100 end-to-end for the Linux
radeon driver (UMS). Sending that series shortly.