[Mesa-dev] [PATCH 10/10] i915: Finish switching i915 to using miptrees instead of regions.

[Chad Versace]

On 01/28/2013 09:00 PM, Eric Anholt wrote:
> ---
>  src/mesa/drivers/dri/i915/i830_vtbl.c      |   52 ++++++++++++-------------
>  src/mesa/drivers/dri/i915/i915_vtbl.c      |   58 ++++++++++++++--------------
>  src/mesa/drivers/dri/intel/intel_context.h |    5 ++-
>  3 files changed, 58 insertions(+), 57 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/i915/i830_vtbl.c b/src/mesa/drivers/dri/i915/i830_vtbl.c

> @@ -768,19 +768,19 @@ i830_update_draw_buffer(struct intel_context *intel)
>        if (_mesa_is_winsys_fbo(fb)) {
>  	 /* drawing to window system buffer */
>  	 if (fb->_ColorDrawBufferIndexes[0] == BUFFER_FRONT_LEFT)
> -	    colorRegion = intel_get_rb_region(fb, BUFFER_FRONT_LEFT);
> +	    color_mt = intel_get_renderbuffer(fb, BUFFER_FRONT_LEFT)->mt;
>  	 else
> -	    colorRegion = intel_get_rb_region(fb, BUFFER_BACK_LEFT);
> +	    color_mt = intel_get_renderbuffer(fb, BUFFER_BACK_LEFT)->mt;
                                                                   ^^^
On a single-buffered framebuffer, I think this dereference will segfault.



>  static void
>  i915_set_draw_region(struct intel_context *intel,
> -                     struct intel_region *color_region,
> -                     struct intel_region *depth_region)
> +                     struct intel_mipmap_tree *color_mt,
> +                     struct intel_mipmap_tree *depth_mt)
>  {
>     struct i915_context *i915 = i915_context(&intel->ctx);
>     struct gl_context *ctx = &intel->ctx;
> @@ -586,25 +586,25 @@ i915_set_draw_region(struct intel_context *intel,
>  
>     drm_intel_bo_unreference(state->color_bo);
>     state->color_bo = NULL;
> -   if (color_region) {
> -      state->color_bo = color_region->bo;
> -      drm_intel_bo_unreference(color_region->bo);
> +   if (color_mt->region) {
                  ^^^^
> +      state->color_bo = color_mt->region->bo;
> +      drm_intel_bo_unreference(color_mt->region->bo);
>     }

The caller of i915_set_draw_region, which is i915_update_draw_buffer, has
codepaths where color_mt is NULL. I don't know if those codepaths are active,
but if they are, then `if (color_mt->region)` segfaults.

>     drm_intel_bo_unreference(state->depth_bo);
>     state->depth_bo = NULL;
> -   if (depth_region) {
> -      state->depth_bo = depth_region->bo;
> -      drm_intel_bo_unreference(depth_region->bo);
> +   if (depth_mt) {
> +      state->depth_bo = depth_mt->region->bo;
> +      drm_intel_bo_unreference(depth_mt->region->bo);
>     }


> @@ -623,10 +623,10 @@ i915_set_draw_region(struct intel_context *intel,
>      * can only be set when a depth buffer is already defined.
>      */
>     if (intel->is_945 && intel->use_early_z &&
> -       depth_region->tiling != I915_TILING_NONE)
> +       depth_mt->region->tiling != I915_TILING_NONE)
>        value |= CLASSIC_EARLY_DEPTH;
>  
> -   if (depth_region && depth_region->cpp == 4) {
> +   if (depth_mt->region && depth_mt->cpp == 4) {
                 ^^^^^
Ditto here. Segfaults if depth_mt is NULL.