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 <[email protected]>
> ---
>  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
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to