[Mesa-dev] [PATCH V5 7/7] intel: implement create image from texture

[Eric Anholt]

Abdiel Janulgue <abdiel.janulgue at linux.intel.com> writes:

> On Tuesday, January 22, 2013 11:02:34 AM Eric Anholt wrote:
>
>> 128 pixels of 32bpp is a tile width, and 128 pixels high of that is 16
>> tiles.  The values I see this function having are:
>> 
>> mask_x = 127
>> mask_y = 15
>> draw_x = 0
>> draw_y = 128
>> image->offset = (16 * 4096)
>> 
>> So when we go to texture from it, we'll add 16 * 4096 to the offset, and
>> then we'll... Aha!  We only make use of the intra-tile offsets (0, 0)
>> there, and drop the inter-tile offset (0, 128).  So normal texturing
>> won't double-count. 
>
>> If you SubImage, the mt->offset is ignored in the
>> mapping we do, which is broken for create_sub_image.
>> 
>> I think we should either
>> 
>> 1) Rename draw_x/draw_y in the image struct to tile_x/tile_y, set them
>> to (draw_x & mask_x) and (draw_y & mask_y), 
>
> Please correct me if I am wrong, but I was assuming the new helper function 
> intel_miptree_get_tile_offsets() would have done the same result anyway when it 
> calculates the tiled offsets in brw_update_texture_surface() ? 

Correct, but it would clarify that this is not an entire x/y offset from
the base of the bo to the image, (specifically that it does not contain
the inter-tile drawing offset because that gets ignored).

> Where  intel_miptree_get_tile_offsets() returns:
>
>  tile_x = mt->level[level].slice[slice].x_offset & mask_x;
>  tile_y = mt->level[level].slice[slice].y_offset & mask_y;
>
> (the x_/y_offsets here comes from draw_x/y in the exported image struct)
>
> I didn't notice the issue earlier because the code above resolved correctly 
> back to the exported lower mip-levels in my tests. But I didn't consider the 
> sub-image use case you mentioned. So of course there could still be a problem.
>
> In any case, I think I've seen a similar solution in renderbuffer code where 
> rendering to miplevels other than 0 just saves the pixel offsets in 
> intel_renderbuffer_set_draw_offset()  and intel_renderbuffer_tile_offsets() 
> returns the calculated tile offsets. But this still might have not been enough?

Yeah, though note that the rb doesn't also have an offset field.  (It
would be nice to not have an offset field, but the planar support that
the offset field is for doesn't necessarily map to an x/y position)

>> and make
>> intel_miptree_image_map() add the mt->offset to its returned pointers.
>
> I'm not sure how to follow thru with this one. Do you mean inside 
> intel_miptree_map_gtt() and append the mt->offset to the pointer there?

Well, all the mapping paths as appropriate.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20130123/b1265b99/attachment-0001.pgp>