On Mon, Oct 17, 2016 at 09:30:45AM +0200, Daniel Vetter wrote: > On Fri, Oct 14, 2016 at 06:30:30PM +0800, Zhenyu Wang wrote: > > > > Hi, > > > > This is first pull request to merge GVT-g device model in i915 > > which contains core GVT-g device model work to virtualize GPU > > resources. This tries to add feature of Intel GVT-g technology > > for full GPU virtualization. This version will support KVM based > > virtualization solution named as KVMGT. > > > > More background is on official project home: https://01.org/igvt-g > > > > To manage mediated device between virtual GPU and physical device it > > will rely on VFIO/mdev framework, this version has not included GVT-g > > device model integration work for VFIO/mdev yet as VFIO community is > > still under work to refine code base. Currently we're basing on > > VFIO/mdev v8 patch series (http://www.spinics.net/lists/kvm/msg138515.html) > > and doing more testings on that. > > > > There're also several KVM change dependences. KVM maintainer has > > merged two and we will ensure left hits KVM tree before sending new > > pull request to enable that. > > > > p.s, There would require some coordinate work for VFIO/mdev. We will > > send device model work for that after Alex merged mdev framework in > > VFIO tree. Alex has promised to merge that in early of Nov. > > > > Let me know if there's any issue with this our first pull request. > > Ok applied, but a few things to keep in mind before your next pull > request: > > - Dont rebase everything 5 seconds before sending out the pull request. > That just invalidates all the testing you've done, so not a good idea. > In general try to avoid rebases as much as possible, and only rebase to > take out a truly embarassing mistake. And then only rebase up to the > patch that needs a hotfix, not your entire tree. > > - Similar, don't base your pull requests upon a random commit of the day > (that's why I noticed you rebased). Instead pick something meaningful, > like a tag I (or Dave Airlie or Linus Torvalds) push out. Or another > good option is to base it right on top of the last merge commit from > gvt. Once you've picked a baseline, don't change it (except when you > have a good reason). And if you need a patch from upstream also don't > rebase, just send out a pull request with your current patch pile, and > then continue applying more stuff on top once I merged that. > > - One technical nit on the integration: My idea was that i915 core code > only calls a few specific functions and structures exposed through > intel_gvt.h. But that file now seems to include gvt-internal headers, > which is a bit a mess. Please clean that up in the next pull request: > > * Anything that core i915 code or headers needs must be moved into > intel_gvt.h. > * Everything else, including the 2 gvt includes we now have (gvt/gvt.h > and i915_pvinfo.h) should only be included from code in > drm/i915/gvt.h. So either sprinkle include directives over your source > files for everything, or make gvt/gvt.h the main gvt header that pulls > in everything. > > The idea here is similar to drm core vs. i915: drm core headers never > pull in i915 headers, and all communication happens through the > well-defined interfaces in drm core header files. I think our goal with > gvt should be similar, with all the interfaces being in intel_gvt.h. > Otherwise I fear the submaintainer model will be a bit painful, if we > don't aim for strict separation here. > > - There's not yet a MAINTAINERS entry for i915/gvt with gvt mailing lists, > git repos and your name on it. Please fix that in the next pull request, > too. > > - gvt seems to lack kernel-doc entirely. I think we need at least an > overview file and interface documentation for the stuff in > intel_gvt.[hc]. Please run > > $ make hmtldocs > > to make sure it all looks pretty (you need to add stanzas in > Documenation/gpu/i915.rst to include things). Another item for the next > pull request please. Quick addition: Since this will be a patch touching i915 core code pls submit it to intel-gfx for review. You can then apply it to your tree once it's reviewed (or Joonas or someone can commit directly to drm-intel-next-queued). And another item: - Please add me to the moderation whitelist of igt-g-dev, I don't want to be spammed by moderation mails every time I reply to your pull requests ;-) Cheers, Daniel > > Also, this is the first time ever I've taken a pull request, so some > learning involved on my side too. Please bear with me ;-) > > Cheers, Daniel > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
