On Mon, Jan 27, 2014 at 11:07:02PM -0800, Ben Widawsky wrote: > Create logical sections in an attempt to clean up, and continue to keep > future additions clean. > > Signed-off-by: Ben Widawsky <ben@xxxxxxxxxxxx> > --- > drivers/gpu/drm/i915/i915_gpu_error.c | 60 +++++++++++++++++++++-------------- > 1 file changed, 36 insertions(+), 24 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c > index 67c82e5..7ded9c2 100644 > --- a/drivers/gpu/drm/i915/i915_gpu_error.c > +++ b/drivers/gpu/drm/i915/i915_gpu_error.c > @@ -1019,41 +1019,53 @@ static void i915_capture_reg_state(struct drm_i915_private *dev_priv, > struct drm_device *dev = dev_priv->dev; > int pipe; > > - error->eir = I915_READ(EIR); > - error->pgtbl_er = I915_READ(PGTBL_ER); > - if (HAS_HW_CONTEXTS(dev)) > - error->ccid = I915_READ(CCID); > - > - if (HAS_PCH_SPLIT(dev)) > - error->ier = I915_READ(DEIER) | I915_READ(GTIER); > - else if (IS_VALLEYVIEW(dev)) > + /* General organization > + * 1. GEN specific registers 1. Registers specific to a single generation 2. Registers specific to multiple generations Does that come out neater than chronological ordering? Yes, it probably does thanks to IS_VALLEYVIEW etc. > + * 2. >= GEN specific registers > + * 3. Feature specific registers. > + * 4. Everything else > + * Please try to follow the order. */ I would split these into two (and I would repeat the section headings as well as the section numbers). > + * /* > + * 1: */ Otherwise, it lgtm. -Chris -- Chris Wilson, Intel Open Source Technology Centre _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx