[Mesa-dev] [Mesa PATCH 2/3] i965: Use the new drm_intel_bo offset64 field.

Ian Romanick idr at freedesktop.org
Thu Jan 16 10:20:24 PST 2014


On 01/13/2014 03:56 PM, Kenneth Graunke wrote:
> libdrm 2.4.52 introduces a new 'uint64_t offset64' field, intended to
> replace the old 'unsigned long offset' field.  To preserve ABI, libdrm
> continues to store the presumed offset in both locations.
> 
> On Broadwell, a 64-bit kernel may place BOs at "high" (> 4G) addresses.
> However, with a 32-bit userspace, the 'unsigned long offset' field will
> only be 32-bit, which is not large enough to hold this value.  We need
> to use a proper uint64_t (like the kernel does).
> 
> Technically, a lot of this code doesn't affect Broadwell, so we could
> leave it using the old field.  But it makes sense to just switch to the
> new, properly typed field.
> 
> Signed-off-by: Kenneth Graunke <kenneth at whitecape.org>
> ---
>  configure.ac                                      |  2 +-
>  src/mesa/drivers/dri/i965/brw_cc.c                |  2 +-
>  src/mesa/drivers/dri/i965/brw_clip_state.c        |  2 +-
>  src/mesa/drivers/dri/i965/brw_context.h           |  2 +-
>  src/mesa/drivers/dri/i965/brw_sf_state.c          |  2 +-
>  src/mesa/drivers/dri/i965/brw_vs_state.c          |  4 ++--
>  src/mesa/drivers/dri/i965/brw_wm_sampler_state.c  |  2 +-
>  src/mesa/drivers/dri/i965/brw_wm_state.c          |  4 ++--
>  src/mesa/drivers/dri/i965/brw_wm_surface_state.c  | 14 +++++++-------
>  src/mesa/drivers/dri/i965/gen6_blorp.cpp          |  4 ++--
>  src/mesa/drivers/dri/i965/gen7_blorp.cpp          |  4 ++--
>  src/mesa/drivers/dri/i965/gen7_wm_surface_state.c | 14 +++++++-------
>  src/mesa/drivers/dri/i965/intel_batchbuffer.c     |  6 +++---
>  13 files changed, 31 insertions(+), 31 deletions(-)
> 
> This was generated by temporarily removing the 'offset' field from libdrm
> and fixing all the compile errors.  Obviously, we can't actually delete the
> field, but you can at least have some confidence that I caught all the
> existing uses.

Alternately, we could use GCC extensions to mark the field as
deprecated.  Then any uses of the field will generate a warning.  This
will prevent new uses from accidentally creeping in.  It would, however,
cause spurious warnings if someone uses new libdrm with old Mesa (e.g.,
10.0.x stable branch).

Either way, all 4 patches are

Reviewed-by: Ian Romanick <ian.d.romanick at intel.com>

> diff --git a/configure.ac b/configure.ac
> index 4b55140..fd189ea 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -29,7 +29,7 @@ AC_SUBST([OSMESA_VERSION])
>  dnl Versions for external dependencies
>  LIBDRM_REQUIRED=2.4.24
>  LIBDRM_RADEON_REQUIRED=2.4.50
> -LIBDRM_INTEL_REQUIRED=2.4.49
> +LIBDRM_INTEL_REQUIRED=2.4.52
>  LIBDRM_NVVIEUX_REQUIRED=2.4.33
>  LIBDRM_NOUVEAU_REQUIRED="2.4.33 libdrm >= 2.4.41"
>  LIBDRM_FREEDRENO_REQUIRED=2.4.51
> diff --git a/src/mesa/drivers/dri/i965/brw_cc.c b/src/mesa/drivers/dri/i965/brw_cc.c
> index 4bc3b23..497d91a 100644
> --- a/src/mesa/drivers/dri/i965/brw_cc.c
> +++ b/src/mesa/drivers/dri/i965/brw_cc.c
> @@ -215,7 +215,7 @@ static void upload_cc_unit(struct brw_context *brw)
>        cc->cc5.statistics_enable = 1;
>  
>     /* CACHE_NEW_CC_VP */
> -   cc->cc4.cc_viewport_state_offset = (brw->batch.bo->offset +
> +   cc->cc4.cc_viewport_state_offset = (brw->batch.bo->offset64 +
>  				       brw->cc.vp_offset) >> 5; /* reloc */
>  
>     brw->state.dirty.cache |= CACHE_NEW_CC_UNIT;
> diff --git a/src/mesa/drivers/dri/i965/brw_clip_state.c b/src/mesa/drivers/dri/i965/brw_clip_state.c
> index 66b3229..8647b0d 100644
> --- a/src/mesa/drivers/dri/i965/brw_clip_state.c
> +++ b/src/mesa/drivers/dri/i965/brw_clip_state.c
> @@ -132,7 +132,7 @@ brw_upload_clip_unit(struct brw_context *brw)
>     {
>        clip->clip5.guard_band_enable = 1;
>        clip->clip6.clipper_viewport_state_ptr =
> -         (brw->batch.bo->offset + brw->clip.vp_offset) >> 5;
> +         (brw->batch.bo->offset64 + brw->clip.vp_offset) >> 5;
>  
>        /* emit clip viewport relocation */
>        drm_intel_bo_emit_reloc(brw->batch.bo,
> diff --git a/src/mesa/drivers/dri/i965/brw_context.h b/src/mesa/drivers/dri/i965/brw_context.h
> index 63dd4a0..77c4c3e 100644
> --- a/src/mesa/drivers/dri/i965/brw_context.h
> +++ b/src/mesa/drivers/dri/i965/brw_context.h
> @@ -1809,7 +1809,7 @@ brw_program_reloc(struct brw_context *brw, uint32_t state_offset,
>  			   prog_offset,
>  			   I915_GEM_DOMAIN_INSTRUCTION, 0);
>  
> -   return brw->cache.bo->offset + prog_offset;
> +   return brw->cache.bo->offset64 + prog_offset;
>  }
>  
>  bool brw_do_cubemap_normalize(struct exec_list *instructions);
> diff --git a/src/mesa/drivers/dri/i965/brw_sf_state.c b/src/mesa/drivers/dri/i965/brw_sf_state.c
> index 69093f2..9bc0cd3 100644
> --- a/src/mesa/drivers/dri/i965/brw_sf_state.c
> +++ b/src/mesa/drivers/dri/i965/brw_sf_state.c
> @@ -173,7 +173,7 @@ static void upload_sf_unit( struct brw_context *brw )
>        sf->thread4.stats_enable = 1;
>  
>     /* CACHE_NEW_SF_VP */
> -   sf->sf5.sf_viewport_state_offset = (brw->batch.bo->offset +
> +   sf->sf5.sf_viewport_state_offset = (brw->batch.bo->offset64 +
>  				       brw->sf.vp_offset) >> 5; /* reloc */
>  
>     sf->sf5.viewport_transform = 1;
> diff --git a/src/mesa/drivers/dri/i965/brw_vs_state.c b/src/mesa/drivers/dri/i965/brw_vs_state.c
> index 015abf1..a3ea62d 100644
> --- a/src/mesa/drivers/dri/i965/brw_vs_state.c
> +++ b/src/mesa/drivers/dri/i965/brw_vs_state.c
> @@ -84,7 +84,7 @@ brw_upload_vs_unit(struct brw_context *brw)
>  
>     if (brw->vs.prog_data->base.total_scratch != 0) {
>        vs->thread2.scratch_space_base_pointer =
> -	 stage_state->scratch_bo->offset >> 10; /* reloc */
> +	 stage_state->scratch_bo->offset64 >> 10; /* reloc */
>        vs->thread2.per_thread_scratch_space =
>  	 ffs(brw->vs.prog_data->base.total_scratch) - 11;
>     } else {
> @@ -161,7 +161,7 @@ brw_upload_vs_unit(struct brw_context *brw)
>      */
>     if (stage_state->sampler_count) {
>        vs->vs5.sampler_state_pointer =
> -         (brw->batch.bo->offset + stage_state->sampler_offset) >> 5;
> +         (brw->batch.bo->offset64 + stage_state->sampler_offset) >> 5;
>        drm_intel_bo_emit_reloc(brw->batch.bo,
>                                stage_state->state_offset +
>                                offsetof(struct brw_vs_unit_state, vs5),
> diff --git a/src/mesa/drivers/dri/i965/brw_wm_sampler_state.c b/src/mesa/drivers/dri/i965/brw_wm_sampler_state.c
> index 0f65085..21138a0 100644
> --- a/src/mesa/drivers/dri/i965/brw_wm_sampler_state.c
> +++ b/src/mesa/drivers/dri/i965/brw_wm_sampler_state.c
> @@ -350,7 +350,7 @@ static void brw_update_sampler_state(struct brw_context *brw,
>        sampler->ss2.default_color_pointer = *sdc_offset >> 5;
>     } else {
>        /* reloc */
> -      sampler->ss2.default_color_pointer = (brw->batch.bo->offset +
> +      sampler->ss2.default_color_pointer = (brw->batch.bo->offset64 +
>  					    *sdc_offset) >> 5;
>  
>        drm_intel_bo_emit_reloc(brw->batch.bo,
> diff --git a/src/mesa/drivers/dri/i965/brw_wm_state.c b/src/mesa/drivers/dri/i965/brw_wm_state.c
> index d56ef20..75ed728 100644
> --- a/src/mesa/drivers/dri/i965/brw_wm_state.c
> +++ b/src/mesa/drivers/dri/i965/brw_wm_state.c
> @@ -125,7 +125,7 @@ brw_upload_wm_unit(struct brw_context *brw)
>  
>     if (brw->wm.prog_data->total_scratch != 0) {
>        wm->thread2.scratch_space_base_pointer =
> -	 brw->wm.base.scratch_bo->offset >> 10; /* reloc */
> +	 brw->wm.base.scratch_bo->offset64 >> 10; /* reloc */
>        wm->thread2.per_thread_scratch_space =
>  	 ffs(brw->wm.prog_data->total_scratch) - 11;
>     } else {
> @@ -151,7 +151,7 @@ brw_upload_wm_unit(struct brw_context *brw)
>  
>     if (brw->wm.base.sampler_count) {
>        /* reloc */
> -      wm->wm4.sampler_state_pointer = (brw->batch.bo->offset +
> +      wm->wm4.sampler_state_pointer = (brw->batch.bo->offset64 +
>  				       brw->wm.base.sampler_offset) >> 5;
>     } else {
>        wm->wm4.sampler_state_pointer = 0;
> diff --git a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> index e837631..80b9a8d 100644
> --- a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> +++ b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> @@ -208,7 +208,7 @@ gen4_emit_buffer_surface_state(struct brw_context *brw,
>     surf[0] = BRW_SURFACE_BUFFER << BRW_SURFACE_TYPE_SHIFT |
>               surface_format << BRW_SURFACE_FORMAT_SHIFT |
>               (brw->gen >= 6 ? BRW_SURFACE_RC_READ_WRITE : 0);
> -   surf[1] = (bo ? bo->offset : 0) + buffer_offset; /* reloc */
> +   surf[1] = (bo ? bo->offset64 : 0) + buffer_offset; /* reloc */
>     surf[2] = (buffer_size & 0x7f) << BRW_SURFACE_WIDTH_SHIFT |
>               ((buffer_size >> 7) & 0x1fff) << BRW_SURFACE_HEIGHT_SHIFT;
>     surf[3] = ((buffer_size >> 20) & 0x7f) << BRW_SURFACE_DEPTH_SHIFT |
> @@ -292,7 +292,7 @@ brw_update_texture_surface(struct gl_context *ctx,
>  				    sampler->sRGBDecode) <<
>  	       BRW_SURFACE_FORMAT_SHIFT));
>  
> -   surf[1] = intelObj->mt->region->bo->offset + intelObj->mt->offset; /* reloc */
> +   surf[1] = intelObj->mt->region->bo->offset64 + intelObj->mt->offset; /* reloc */
>  
>     surf[2] = ((intelObj->_MaxLevel - tObj->BaseLevel) << BRW_SURFACE_LOD_SHIFT |
>  	      (mt->logical_width0 - 1) << BRW_SURFACE_WIDTH_SHIFT |
> @@ -312,7 +312,7 @@ brw_update_texture_surface(struct gl_context *ctx,
>     drm_intel_bo_emit_reloc(brw->batch.bo,
>  			   *surf_offset + 4,
>  			   intelObj->mt->region->bo,
> -                           surf[1] - intelObj->mt->region->bo->offset,
> +                           surf[1] - intelObj->mt->region->bo->offset64,
>  			   I915_GEM_DOMAIN_SAMPLER, 0);
>  }
>  
> @@ -408,7 +408,7 @@ brw_update_sol_surface(struct brw_context *brw,
>        BRW_SURFACE_MIPMAPLAYOUT_BELOW << BRW_SURFACE_MIPLAYOUT_SHIFT |
>        surface_format << BRW_SURFACE_FORMAT_SHIFT |
>        BRW_SURFACE_RC_READ_WRITE;
> -   surf[1] = bo->offset + offset_bytes; /* reloc */
> +   surf[1] = bo->offset64 + offset_bytes; /* reloc */
>     surf[2] = (width << BRW_SURFACE_WIDTH_SHIFT |
>  	      height << BRW_SURFACE_HEIGHT_SHIFT);
>     surf[3] = (depth << BRW_SURFACE_DEPTH_SHIFT |
> @@ -555,7 +555,7 @@ brw_update_null_renderbuffer_surface(struct brw_context *brw, unsigned int unit)
>  		  1 << BRW_SURFACE_WRITEDISABLE_B_SHIFT |
>  		  1 << BRW_SURFACE_WRITEDISABLE_A_SHIFT);
>     }
> -   surf[1] = bo ? bo->offset : 0;
> +   surf[1] = bo ? bo->offset64 : 0;
>     surf[2] = ((fb->Width - 1) << BRW_SURFACE_WIDTH_SHIFT |
>                (fb->Height - 1) << BRW_SURFACE_HEIGHT_SHIFT);
>  
> @@ -635,7 +635,7 @@ brw_update_renderbuffer_surface(struct brw_context *brw,
>  
>     /* reloc */
>     surf[1] = (intel_renderbuffer_get_tile_offsets(irb, &tile_x, &tile_y) +
> -	      region->bo->offset);
> +	      region->bo->offset64);
>  
>     surf[2] = ((rb->Width - 1) << BRW_SURFACE_WIDTH_SHIFT |
>  	      (rb->Height - 1) << BRW_SURFACE_HEIGHT_SHIFT);
> @@ -680,7 +680,7 @@ brw_update_renderbuffer_surface(struct brw_context *brw,
>     drm_intel_bo_emit_reloc(brw->batch.bo,
>  			   brw->wm.base.surf_offset[surf_index] + 4,
>  			   region->bo,
> -			   surf[1] - region->bo->offset,
> +			   surf[1] - region->bo->offset64,
>  			   I915_GEM_DOMAIN_RENDER,
>  			   I915_GEM_DOMAIN_RENDER);
>  }
> diff --git a/src/mesa/drivers/dri/i965/gen6_blorp.cpp b/src/mesa/drivers/dri/i965/gen6_blorp.cpp
> index 2e8e8ab..90b9fbb 100644
> --- a/src/mesa/drivers/dri/i965/gen6_blorp.cpp
> +++ b/src/mesa/drivers/dri/i965/gen6_blorp.cpp
> @@ -401,7 +401,7 @@ gen6_blorp_emit_surface_state(struct brw_context *brw,
>  
>     /* reloc */
>     surf[1] = (surface->compute_tile_offsets(&tile_x, &tile_y) +
> -              region->bo->offset);
> +              region->bo->offset64);
>  
>     surf[2] = (0 << BRW_SURFACE_LOD_SHIFT |
>                (width - 1) << BRW_SURFACE_WIDTH_SHIFT |
> @@ -433,7 +433,7 @@ gen6_blorp_emit_surface_state(struct brw_context *brw,
>     drm_intel_bo_emit_reloc(brw->batch.bo,
>                             wm_surf_offset + 4,
>                             region->bo,
> -                           surf[1] - region->bo->offset,
> +                           surf[1] - region->bo->offset64,
>                             read_domains, write_domain);
>  
>     return wm_surf_offset;
> diff --git a/src/mesa/drivers/dri/i965/gen7_blorp.cpp b/src/mesa/drivers/dri/i965/gen7_blorp.cpp
> index c687454..4bf9396 100644
> --- a/src/mesa/drivers/dri/i965/gen7_blorp.cpp
> +++ b/src/mesa/drivers/dri/i965/gen7_blorp.cpp
> @@ -176,7 +176,7 @@ gen7_blorp_emit_surface_state(struct brw_context *brw,
>  
>     /* reloc */
>     surf[1] =
> -      surface->compute_tile_offsets(&tile_x, &tile_y) + region->bo->offset;
> +      surface->compute_tile_offsets(&tile_x, &tile_y) + region->bo->offset64;
>  
>     /* Note that the low bits of these fields are missing, so
>      * there's the possibility of getting in trouble.
> @@ -214,7 +214,7 @@ gen7_blorp_emit_surface_state(struct brw_context *brw,
>     drm_intel_bo_emit_reloc(brw->batch.bo,
>                             wm_surf_offset + 4,
>                             region->bo,
> -                           surf[1] - region->bo->offset,
> +                           surf[1] - region->bo->offset64,
>                             read_domains, write_domain);
>  
>     gen7_check_surface_setup(surf, is_render_target);
> diff --git a/src/mesa/drivers/dri/i965/gen7_wm_surface_state.c b/src/mesa/drivers/dri/i965/gen7_wm_surface_state.c
> index 761bc3b..57e359a 100644
> --- a/src/mesa/drivers/dri/i965/gen7_wm_surface_state.c
> +++ b/src/mesa/drivers/dri/i965/gen7_wm_surface_state.c
> @@ -122,11 +122,11 @@ gen7_set_surface_mcs_info(struct brw_context *brw,
>      * thus have their lower 12 bits zero), we can use an ordinary reloc to do
>      * the necessary address translation.
>      */
> -   assert ((mcs_mt->region->bo->offset & 0xfff) == 0);
> +   assert ((mcs_mt->region->bo->offset64 & 0xfff) == 0);
>  
>     surf[6] = GEN7_SURFACE_MCS_ENABLE |
>               SET_FIELD(pitch_tiles - 1, GEN7_SURFACE_MCS_PITCH) |
> -             mcs_mt->region->bo->offset;
> +             mcs_mt->region->bo->offset64;
>  
>     drm_intel_bo_emit_reloc(brw->batch.bo,
>                             surf_offset + 6 * 4,
> @@ -242,7 +242,7 @@ gen7_emit_buffer_surface_state(struct brw_context *brw,
>     surf[0] = BRW_SURFACE_BUFFER << BRW_SURFACE_TYPE_SHIFT |
>               surface_format << BRW_SURFACE_FORMAT_SHIFT |
>               BRW_SURFACE_RC_READ_WRITE;
> -   surf[1] = (bo ? bo->offset : 0) + buffer_offset; /* reloc */
> +   surf[1] = (bo ? bo->offset64 : 0) + buffer_offset; /* reloc */
>     surf[2] = SET_FIELD((buffer_size - 1) & 0x7f, GEN7_SURFACE_WIDTH) |
>               SET_FIELD(((buffer_size - 1) >> 7) & 0x3fff, GEN7_SURFACE_HEIGHT);
>     surf[3] = SET_FIELD(((buffer_size - 1) >> 21) & 0x3f, BRW_SURFACE_DEPTH) |
> @@ -312,7 +312,7 @@ gen7_update_texture_surface(struct gl_context *ctx,
>     if (mt->array_spacing_lod0)
>        surf[0] |= GEN7_SURFACE_ARYSPC_LOD0;
>  
> -   surf[1] = mt->region->bo->offset + mt->offset; /* reloc */
> +   surf[1] = mt->region->bo->offset64 + mt->offset; /* reloc */
>  
>     surf[2] = SET_FIELD(mt->logical_width0 - 1, GEN7_SURFACE_WIDTH) |
>               SET_FIELD(mt->logical_height0 - 1, GEN7_SURFACE_HEIGHT);
> @@ -357,7 +357,7 @@ gen7_update_texture_surface(struct gl_context *ctx,
>     drm_intel_bo_emit_reloc(brw->batch.bo,
>  			   *surf_offset + 4,
>  			   intelObj->mt->region->bo,
> -                           surf[1] - intelObj->mt->region->bo->offset,
> +                           surf[1] - intelObj->mt->region->bo->offset64,
>  			   I915_GEM_DOMAIN_SAMPLER, 0);
>  
>     gen7_check_surface_setup(surf, false /* is_render_target */);
> @@ -505,7 +505,7 @@ gen7_update_renderbuffer_surface(struct brw_context *brw,
>        surf[0] |= GEN7_SURFACE_IS_ARRAY;
>     }
>  
> -   surf[1] = region->bo->offset;
> +   surf[1] = region->bo->offset64;
>  
>     assert(brw->has_surface_tile_offset);
>  
> @@ -539,7 +539,7 @@ gen7_update_renderbuffer_surface(struct brw_context *brw,
>     drm_intel_bo_emit_reloc(brw->batch.bo,
>  			   brw->wm.base.surf_offset[surf_index] + 4,
>  			   region->bo,
> -			   surf[1] - region->bo->offset,
> +			   surf[1] - region->bo->offset64,
>  			   I915_GEM_DOMAIN_RENDER,
>  			   I915_GEM_DOMAIN_RENDER);
>  
> diff --git a/src/mesa/drivers/dri/i965/intel_batchbuffer.c b/src/mesa/drivers/dri/i965/intel_batchbuffer.c
> index cee76d5..966b95b 100644
> --- a/src/mesa/drivers/dri/i965/intel_batchbuffer.c
> +++ b/src/mesa/drivers/dri/i965/intel_batchbuffer.c
> @@ -155,7 +155,7 @@ do_batch_dump(struct brw_context *brw)
>     if (ret == 0) {
>        drm_intel_decode_set_batch_pointer(decode,
>  					 batch->bo->virtual,
> -					 batch->bo->offset,
> +					 batch->bo->offset64,
>  					 batch->used);
>     } else {
>        fprintf(stderr,
> @@ -164,7 +164,7 @@ do_batch_dump(struct brw_context *brw)
>  
>        drm_intel_decode_set_batch_pointer(decode,
>  					 batch->map,
> -					 batch->bo->offset,
> +					 batch->bo->offset64,
>  					 batch->used);
>     }
>  
> @@ -392,7 +392,7 @@ intel_batchbuffer_emit_reloc(struct brw_context *brw,
>      * the buffer doesn't move and we can short-circuit the relocation processing
>      * in the kernel
>      */
> -   intel_batchbuffer_emit_dword(brw, buffer->offset + delta);
> +   intel_batchbuffer_emit_dword(brw, buffer->offset64 + delta);
>  
>     return true;
>  }
> 



More information about the mesa-dev mailing list