Re: [RFC 09/10] Clean up Radeon Header files

[BALATON Zoltan]

On Wed, 27 Nov 2019, Aleksandar Markovic wrote:
> On Tuesday, November 26, 2019, <address@hidden> wrote:
>
> > From: Aaron Dominick <address@hidden>
> >
> > ---
>
>
> Your commit message is poor. You should have clearly explained what do you
> do in this cleanup, and why.

That the commit message is not helpful is the smallest problem. Clearly 
there are more to improve in this series but please bear with new 
contributors who may need to get used to the workflow with git and QEMU 
patch submission and don't scare them away by criticising small problems 
without helping them.

This whole series is RFC and was stated in the original cover letter of 
the v1 series that this is work in progress submitted for enquiring about 
the direction taken not meant to be finished. That said it's still hard to 
review in this state so some improvements are needed. This v1 was an 
attempt for that but looks like it's not correct yet. I've found at least 
these problems:

- Series says it has 10 patches but I've only got 5-9 so it may have been 
edited by hand instead of properly using git format-patch.

- Patch 9 added to remove unneded headers at the end but that does not 
help to make Patch 5 more clear so this should be squashed into patch 5 
instead (try git rebase -i and squash patches 5 and 9 so we end up with 
only the necessary files added instead of one patch adding them and 
another removing them later).

- The resulting patch 5 (that should really be 1) should be split into 
two: one copying existing files without modifying them and another patch 
adding new headers to make it clear what changes are made that is hard to 
find if copying and modifying files are done in the same patch.

As for the original question if this is the right direction I can't really 
tell but I think it may be but to make it work we will need to implement 
Microengine (aka. command processor/CCE/PM4) that reads packets from a 
shared memory buffer and converts them to register accesses but it's not 
documented so either we can get some info about it from somewhere (maybe 
AMD) or manage to reverse engineer the microcode or just implement its 
functionality not modeling the actual microengine the real card has but 
only do packet parsing that it should do.

There was another question about GART which I can't answer not knowing 
much about it but maybe we only need the addresses and then the device can 
access system memory directly so we don't need to model the whole 
IOMMU/DMA but I'm not sure what GART is used for so I may be wrong. 
Comments from those who know more about ATI GPUs are welcome.

Regards,
BALATON Zoltan