On Thu, Oct 16, 2014 at 03:05:07PM +0000, Cheng, Yao wrote:
Ok, bunch of comments. First a high-level one: I think this qualifies as a new subsystem of i915, and so it would be good to extract this into a new file (i915_ved.c maybe), including adding kerneldoc for the setup function, a short DOC: overview section and pulling it all into the drm kerneldoc (probably a new subsection in the driver core section).
Aside from the lack of documentation just a few small comments below. Overall I really like how cleanly we can integrate vxd support into i915, so good work.
I915_ved.c sounds to be a good place for these code, thx for this suggestion!
For the kerneldoc, I'll add a subsection in the core section for your review.
Yeah, i915 driver core section sounds like the right place. Btw there's a small blog post from me with some tips for how to go about this:
http://blog.ffwll.ch/2014/06/documentation-for-drmi915.html
+static void valleyview_ved_cleanup(struct drm_device *dev) {
- int irq;
- struct drm_i915_private *dev_priv = dev->dev_private;
- irq = platform_get_irq(dev_priv->ved_platdev, 0);
- if (irq >= 0)
irq_free_desc(irq);- platform_device_unregister(dev_priv->ved_platdev);
I think you should unregister the platform device _before_ you free the irq. Otherwise the driver cleanup might freak out. Aside: Does the module reload test for i915 in i-g-t still work with the vxd driver loaded on vlv? Iirc you need to manually unload the vxd driver first to avoid inherit races in the platform device support code, so if that's the case you need to supply a patch for igt, too.
Sorry, what is i-g-t? I'll follow your suggestion, test i-g-t, and patch it if needed.
i-g-t is the i915 kernel driver regression test suite. For a quick intro see:
http://blog.ffwll.ch/2013/08/recent-drmi915-testsuite-improvements.html
The igt repo is at
http://cgit.freedesktop.org/xorg/app/intel-gpu-tools/
The README file in there also has some good information.
Cheers, Daniel