On Mon, Mar 17, 2014 at 10:48:39PM -0700, Ben Widawsky wrote:
> The old code (I'm having trouble finding the commit) had a reason for
> doing things when there was an error, and would continue on, thus the
> !ret. For the newer code however, this looks completely silly.
> 
> Follow the normal idiom of if (ret) return ret.
> 
> Also, put the pde wiring in the gen specific init, now that GEN8 exists.
> 
> Signed-off-by: Ben Widawsky <b...@bwidawsk.net>
> ---
>  drivers/gpu/drm/i915/i915_gem_gtt.c | 22 +++++++++-------------
>  1 file changed, 9 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c 
> b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index 1620211..5f73284 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -1202,6 +1202,8 @@ static int gen6_ppgtt_init(struct i915_hw_ppgtt *ppgtt)
>       ppgtt->pd_offset =
>               ppgtt->node.start / PAGE_SIZE * sizeof(gen6_gtt_pte_t);
>  
> +     gen6_write_pdes(ppgtt);
> +
>       ppgtt->base.clear_range(&ppgtt->base, 0, ppgtt->base.total, true);
>  
>       DRM_DEBUG_DRIVER("Allocated pde space (%ldM) at GTT entry: %lx\n",
> @@ -1226,20 +1228,14 @@ int i915_gem_init_ppgtt(struct drm_device *dev, 
> struct i915_hw_ppgtt *ppgtt)
>       else
>               BUG();
>  
> -     if (!ret) {
> -             struct drm_i915_private *dev_priv = dev->dev_private;
> -             kref_init(&ppgtt->ref);
> -             drm_mm_init(&ppgtt->base.mm, ppgtt->base.start,
> -                         ppgtt->base.total);
> -             i915_init_vm(dev_priv, &ppgtt->base);
> -             if (INTEL_INFO(dev)->gen < 8) {
> -                     gen6_write_pdes(ppgtt);
> -                     DRM_DEBUG("Adding PPGTT at offset %x\n",
> -                               ppgtt->pd_offset << 10);
> -             }
> -     }
> +     if (ret)
> +             return ret;
>  
> -     return ret;
> +     kref_init(&ppgtt->ref);
> +     drm_mm_init(&ppgtt->base.mm, ppgtt->base.start, ppgtt->base.total);
> +     i915_init_vm(dev_priv, &ppgtt->base);

Didn't we just delete the dev_priv local variable?
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to