Re: [Intel-gfx] [PATCH 00/10] shrink dev_priv by 300 lines
On Sun, Nov 4, 2012 at 6:30 PM, Ben Widawsky 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 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 00/10] shrink dev_priv by 300 lines
On Fri, 2 Nov 2012 19:55:01 +0100 Daniel Vetter wrote: > Hi all, > > 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. > > Cheers, Daniel 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. > > Daniel Vetter (10): > drm/i915: move the suspend/resume register file out of dev_priv > drm/i915: move dev_priv->(rps|ips) out of line > drm/i915: move pwrctx/renderctx to the other ilk power state > drm/i915: move dri1 dungeon out of dev_priv > drm/i915: extract dev_priv fbc state into separate substruct > drm/i915: extract l3_parity substruct from dev_priv > drm/i915: move dev_priv->mm out of line > drm/i915: extract hangcheck/reset/error_state state into substruct > drm/i915: kill dev_priv->modeset_on_lid > drm/i915: move fence_regs to the fence lru > > drivers/gpu/drm/i915/i915_debugfs.c | 32 +- > drivers/gpu/drm/i915/i915_dma.c | 43 +- > drivers/gpu/drm/i915/i915_drv.c | 15 +- > drivers/gpu/drm/i915/i915_drv.h | 608 > ++-- drivers/gpu/drm/i915/i915_gem.c > | 42 +- drivers/gpu/drm/i915/i915_gem_stolen.c | 14 +- > drivers/gpu/drm/i915/i915_irq.c | 62 +-- > drivers/gpu/drm/i915/i915_suspend.c | 692 > > drivers/gpu/drm/i915/i915_sysfs.c | 6 +- > drivers/gpu/drm/i915/intel_display.c| 6 +- > drivers/gpu/drm/i915/intel_lvds.c | 40 -- > drivers/gpu/drm/i915/intel_panel.c | 20 +- > drivers/gpu/drm/i915/intel_pm.c | 80 ++-- > drivers/gpu/drm/i915/intel_ringbuffer.c | 2 +- 14 files changed, > 823 insertions(+), 839 deletions(-) > -- Ben Widawsky, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 00/10] shrink dev_priv by 300 lines
Hi all, 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. Cheers, Daniel Daniel Vetter (10): drm/i915: move the suspend/resume register file out of dev_priv drm/i915: move dev_priv->(rps|ips) out of line drm/i915: move pwrctx/renderctx to the other ilk power state drm/i915: move dri1 dungeon out of dev_priv drm/i915: extract dev_priv fbc state into separate substruct drm/i915: extract l3_parity substruct from dev_priv drm/i915: move dev_priv->mm out of line drm/i915: extract hangcheck/reset/error_state state into substruct drm/i915: kill dev_priv->modeset_on_lid drm/i915: move fence_regs to the fence lru drivers/gpu/drm/i915/i915_debugfs.c | 32 +- drivers/gpu/drm/i915/i915_dma.c | 43 +- drivers/gpu/drm/i915/i915_drv.c | 15 +- drivers/gpu/drm/i915/i915_drv.h | 608 ++-- drivers/gpu/drm/i915/i915_gem.c | 42 +- drivers/gpu/drm/i915/i915_gem_stolen.c | 14 +- drivers/gpu/drm/i915/i915_irq.c | 62 +-- drivers/gpu/drm/i915/i915_suspend.c | 692 drivers/gpu/drm/i915/i915_sysfs.c | 6 +- drivers/gpu/drm/i915/intel_display.c| 6 +- drivers/gpu/drm/i915/intel_lvds.c | 40 -- drivers/gpu/drm/i915/intel_panel.c | 20 +- drivers/gpu/drm/i915/intel_pm.c | 80 ++-- drivers/gpu/drm/i915/intel_ringbuffer.c | 2 +- 14 files changed, 823 insertions(+), 839 deletions(-) -- 1.7.11.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
