All of lore.kernel.org
 help / color / mirror / Atom feed
From: BALATON Zoltan <balaton@eik.bme.hu>
To: "aaron.zakhrov@gmail.com" <aaron.zakhrov@gmail.com>
Cc: "qemu-devel@nongnu.org" <qemu-devel@nongnu.org>,
	Aleksandar Markovic <aleksandar.m.mail@gmail.com>,
	"kraxel@redhat.com" <kraxel@redhat.com>
Subject: Re: [RFC 09/10] Clean up Radeon Header files
Date: Wed, 27 Nov 2019 15:42:28 +0100 (CET)	[thread overview]
Message-ID: <alpine.BSF.2.21.99999.352.1911271505300.80933@zero.eik.bme.hu> (raw)
In-Reply-To: <CAL1e-=hY5b73ocsG9xiUFVuJYL36ch4=AyqjPvmtaXRUi730pQ@mail.gmail.com>

On Wed, 27 Nov 2019, Aleksandar Markovic wrote:
> On Tuesday, November 26, 2019, <aaron.zakhrov@gmail.com> wrote:
>
>> From: Aaron Dominick <aaron.zakhrov@gmail.com>
>>
>> ---
>
>
> 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


  reply	other threads:[~2019-11-27 14:44 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-11-26 12:44 [RFC 00/10] R300 QEMU device V2 aaron.zakhrov
2019-11-26 12:44 ` [RFC 05/10] Add Radeon kernel headers. Will clean up later aaron.zakhrov
2019-11-26 12:44 ` [RFC 06/10] Fix MC STATUS resgister aaron.zakhrov
2019-11-26 12:44 ` [RFC 07/10] R300 fixes aaron.zakhrov
2019-11-26 12:44 ` [RFC 08/10] Got GPU init working. Stops at probing display aaron.zakhrov
2019-11-26 12:44 ` [RFC 09/10] Clean up Radeon Header files aaron.zakhrov
2019-11-27  9:55   ` Aleksandar Markovic
2019-11-27 14:42     ` BALATON Zoltan [this message]
2019-11-26 14:19 ` [RFC 00/10] R300 QEMU device V2 Daniel P. Berrangé
2019-11-27 15:00   ` Philippe Mathieu-Daudé
2019-11-27 15:05     ` Daniel P. Berrangé
2019-11-27 15:13       ` Philippe Mathieu-Daudé
2019-11-27 15:54         ` Daniel P. Berrangé
2019-11-27 16:12       ` Gerd Hoffmann
2019-11-27 16:32         ` Daniel P. Berrangé
2019-11-28  6:51           ` Aaron Zakhrov
2019-11-29 18:03             ` BALATON Zoltan
2019-11-27 16:17       ` BALATON Zoltan

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=alpine.BSF.2.21.99999.352.1911271505300.80933@zero.eik.bme.hu \
    --to=balaton@eik.bme.hu \
    --cc=aaron.zakhrov@gmail.com \
    --cc=aleksandar.m.mail@gmail.com \
    --cc=kraxel@redhat.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.