On Sun, Nov 4, 2012 at 6:30 PM, Ben Widawsky <ben at bwidawsk.net> wrote: >> Inspired by some recent patches I've figured I need to clean up and >> put some organization behind our driver_private struct. It shrunk >> from almost 500 lines to about 160. Things still there which could be >> gathered: >> - vbt and vbt-derived values >> - shared/global modeset state >> - random smaller bits and pieces >> >> Comments highly welcome. > > This is in reference to all your extraction of substructs ie. the l3 > parity... > > This really seems like moving around code mostly for the sake of moving > around code (note that I said mostly). What I would have liked to see as a > motivation/result of this is instead of passing dev_priv around all over > the place, pass around the new extracted structures. If you still need > dev_priv (for something other than reg reads or writes), then I don't > think you've done anything useful. > > Changing the function prototypes to the new substructs would be a > provably useful thing to do. I've played around with passing the substructs to functions that handle individual parts and notice that pretty much everywhere we need dev_priv to handle reg I/O. So I've decided to keep things as-is to avoid inconsistency (we already have that mess in the modeset code with drm_* vs. intel_* ...). Otoh if that "move register blocks around" thing becomes more prevalent on vlv, we might want to switch to passing substructs around, and also adding per-block reg I/O variants which implicitly take the substruct and add the required mmio_base offset. But even without that I very much like the new order in i915_dev_private. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch
