Re: [Mesa-dev] i965/gen9: Compression support for single-sampled
On Wed, Feb 10, 2016 at 02:13:27PM +0200, Pohjolainen, Topi wrote: > On Tue, Feb 09, 2016 at 05:34:53PM -0800, Ben Widawsky wrote: > > On Mon, Feb 08, 2016 at 06:51:20PM +0200, Topi Pohjolainen wrote: > > > This series enables compression for single sampled color surfaces, > > > also referred to as "lossless compression". This is yet only for > > > driver internal use easing pressure on memory bandwidth and caches > > > when writing, blending and sampling surfaces uing gpu. > > > > > > As a side effect the need for color buffer resolves after fast > > > clears is also decreased. Current understanding is that sampling > > > engine doesn't understand meta data (auxiliary buffer) for single > > > sampled fast cleared surfaces. However, if the meta data is written > > > with lossless compression enabled, even sampling engine is capable > > > of reading both the color buffer and the auxiliary, and resolves > > > can be omitted in those case. > > > > > > The final enabling patch is dependent on earlier two-patch series > > > fixing state restore mechanism in i965-meta operations. > > > > > > There are some performance numbers available in the final commit. > > > > > > Topi Pohjolainen (23): > > > i965: Let caller of intel_miptree_create_layout() decide msaa layout > > > i965: Use miptree non-aligned dimensions directly for x-tiled > > > i965: Separate miptree creation from auxiliary buffer setup > > > i965: Don't try to create aux buffer for non-msrt aux-buffer > > > i965: Stop considering if msrt aux buffers need aux buffer > > > i965/gen9: Add buffer layout representing lossless compression > > > i965/gen9: Allow halign == 16 also for losslessly compressed > > > i965: Allow fast clear to be used with lossless compression > > > i965: Add resolve option for lossless compression > > > i965: Use constant pointer when checking for compression > > > i965/gen8: Remove dead assertion > > > i965: Refactor resolving of auxiliary mode > > > i965: Resolve color buffer also in lossless compression case > > > i965: Add means for limiting color resolves > > > i965: Add a flag telling color resolve pass to ignore CCS_E > > > i965: Add a few assertions on lossless compression > > > i965: Set buffer cleared after actually clearing it > > > i965/gen9: Prepare surface state setup for lossless compression > > > i965/gen9: Refactor msrt mcs initialization > > > i965: Expose logic telling if non-msrt mcs is supported > > > i965/gen9: Setup MCS for compressed texture surfaces > > > i965: Add helper for checking for lossless compressible > > > i965/gen9: Enable lossless compression > > > > > > src/mesa/drivers/dri/i965/brw_blorp_blit.cpp| 21 +- > > > src/mesa/drivers/dri/i965/brw_context.c | 22 ++- > > > src/mesa/drivers/dri/i965/brw_context.h | 2 +- > > > src/mesa/drivers/dri/i965/brw_defines.h | 2 + > > > src/mesa/drivers/dri/i965/brw_meta_fast_clear.c | 16 +- > > > src/mesa/drivers/dri/i965/brw_surface_formats.c | 2 +- > > > src/mesa/drivers/dri/i965/brw_tex_layout.c | 2 + > > > src/mesa/drivers/dri/i965/gen8_surface_state.c | 90 + > > > src/mesa/drivers/dri/i965/intel_blit.c | 4 +- > > > src/mesa/drivers/dri/i965/intel_copy_image.c| 4 +- > > > src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 249 > > > +--- > > > src/mesa/drivers/dri/i965/intel_mipmap_tree.h | 32 ++- > > > src/mesa/drivers/dri/i965/intel_pixel_bitmap.c | 2 +- > > > src/mesa/drivers/dri/i965/intel_pixel_read.c| 2 +- > > > src/mesa/drivers/dri/i965/intel_tex_image.c | 2 +- > > > src/mesa/drivers/dri/i965/intel_tex_subimage.c | 2 +- > > > 16 files changed, 318 insertions(+), 136 deletions(-) > > > > > > > Need to take a break, my head hurts. > > In addition to the comments I already left, 5, 8, 9, 10, and 11 are: > > Reviewed-by: Ben Widawsky> > > > I think they might change at least a bit if you consider the feedback I've > > given > > so far, but it's really up to you. > > Ok. > > > > > 8 is kind of useless on its own IMO, but whatever you like. > > I suppose it is a matter of taste. It is non-functional change that just > introduces new enumeration and makes sure all the switch-cases take it > into account (unreachable()). To me it is clearer to have these separated > from real functional. But like said, matter of taste. Where would you > merge this? Patch 6 looked like a decent candidate. I really want you to make the choice, I'm just giving the unfiltered (but solicited) feedback :-) > > > > > I'd say you should push 10, and 11 now. > > > > Done. Thanks for the review so far! ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] i965/gen9: Compression support for single-sampled
On Tue, Feb 09, 2016 at 05:34:53PM -0800, Ben Widawsky wrote: > On Mon, Feb 08, 2016 at 06:51:20PM +0200, Topi Pohjolainen wrote: > > This series enables compression for single sampled color surfaces, > > also referred to as "lossless compression". This is yet only for > > driver internal use easing pressure on memory bandwidth and caches > > when writing, blending and sampling surfaces uing gpu. > > > > As a side effect the need for color buffer resolves after fast > > clears is also decreased. Current understanding is that sampling > > engine doesn't understand meta data (auxiliary buffer) for single > > sampled fast cleared surfaces. However, if the meta data is written > > with lossless compression enabled, even sampling engine is capable > > of reading both the color buffer and the auxiliary, and resolves > > can be omitted in those case. > > > > The final enabling patch is dependent on earlier two-patch series > > fixing state restore mechanism in i965-meta operations. > > > > There are some performance numbers available in the final commit. > > > > Topi Pohjolainen (23): > > i965: Let caller of intel_miptree_create_layout() decide msaa layout > > i965: Use miptree non-aligned dimensions directly for x-tiled > > i965: Separate miptree creation from auxiliary buffer setup > > i965: Don't try to create aux buffer for non-msrt aux-buffer > > i965: Stop considering if msrt aux buffers need aux buffer > > i965/gen9: Add buffer layout representing lossless compression > > i965/gen9: Allow halign == 16 also for losslessly compressed > > i965: Allow fast clear to be used with lossless compression > > i965: Add resolve option for lossless compression > > i965: Use constant pointer when checking for compression > > i965/gen8: Remove dead assertion > > i965: Refactor resolving of auxiliary mode > > i965: Resolve color buffer also in lossless compression case > > i965: Add means for limiting color resolves > > i965: Add a flag telling color resolve pass to ignore CCS_E > > i965: Add a few assertions on lossless compression > > i965: Set buffer cleared after actually clearing it > > i965/gen9: Prepare surface state setup for lossless compression > > i965/gen9: Refactor msrt mcs initialization > > i965: Expose logic telling if non-msrt mcs is supported > > i965/gen9: Setup MCS for compressed texture surfaces > > i965: Add helper for checking for lossless compressible > > i965/gen9: Enable lossless compression > > > > src/mesa/drivers/dri/i965/brw_blorp_blit.cpp| 21 +- > > src/mesa/drivers/dri/i965/brw_context.c | 22 ++- > > src/mesa/drivers/dri/i965/brw_context.h | 2 +- > > src/mesa/drivers/dri/i965/brw_defines.h | 2 + > > src/mesa/drivers/dri/i965/brw_meta_fast_clear.c | 16 +- > > src/mesa/drivers/dri/i965/brw_surface_formats.c | 2 +- > > src/mesa/drivers/dri/i965/brw_tex_layout.c | 2 + > > src/mesa/drivers/dri/i965/gen8_surface_state.c | 90 + > > src/mesa/drivers/dri/i965/intel_blit.c | 4 +- > > src/mesa/drivers/dri/i965/intel_copy_image.c| 4 +- > > src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 249 > > +--- > > src/mesa/drivers/dri/i965/intel_mipmap_tree.h | 32 ++- > > src/mesa/drivers/dri/i965/intel_pixel_bitmap.c | 2 +- > > src/mesa/drivers/dri/i965/intel_pixel_read.c| 2 +- > > src/mesa/drivers/dri/i965/intel_tex_image.c | 2 +- > > src/mesa/drivers/dri/i965/intel_tex_subimage.c | 2 +- > > 16 files changed, 318 insertions(+), 136 deletions(-) > > > > Need to take a break, my head hurts. > In addition to the comments I already left, 5, 8, 9, 10, and 11 are: > Reviewed-by: Ben Widawsky> > I think they might change at least a bit if you consider the feedback I've > given > so far, but it's really up to you. Ok. > > 8 is kind of useless on its own IMO, but whatever you like. I suppose it is a matter of taste. It is non-functional change that just introduces new enumeration and makes sure all the switch-cases take it into account (unreachable()). To me it is clearer to have these separated from real functional. But like said, matter of taste. Where would you merge this? > > I'd say you should push 10, and 11 now. > Done. Thanks for the review so far! ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] i965/gen9: Compression support for single-sampled
On Mon, Feb 08, 2016 at 06:51:20PM +0200, Topi Pohjolainen wrote: > This series enables compression for single sampled color surfaces, > also referred to as "lossless compression". This is yet only for > driver internal use easing pressure on memory bandwidth and caches > when writing, blending and sampling surfaces uing gpu. > > As a side effect the need for color buffer resolves after fast > clears is also decreased. Current understanding is that sampling > engine doesn't understand meta data (auxiliary buffer) for single > sampled fast cleared surfaces. However, if the meta data is written > with lossless compression enabled, even sampling engine is capable > of reading both the color buffer and the auxiliary, and resolves > can be omitted in those case. > > The final enabling patch is dependent on earlier two-patch series > fixing state restore mechanism in i965-meta operations. > > There are some performance numbers available in the final commit. > > Topi Pohjolainen (23): > i965: Let caller of intel_miptree_create_layout() decide msaa layout > i965: Use miptree non-aligned dimensions directly for x-tiled > i965: Separate miptree creation from auxiliary buffer setup > i965: Don't try to create aux buffer for non-msrt aux-buffer > i965: Stop considering if msrt aux buffers need aux buffer > i965/gen9: Add buffer layout representing lossless compression > i965/gen9: Allow halign == 16 also for losslessly compressed > i965: Allow fast clear to be used with lossless compression > i965: Add resolve option for lossless compression > i965: Use constant pointer when checking for compression > i965/gen8: Remove dead assertion > i965: Refactor resolving of auxiliary mode > i965: Resolve color buffer also in lossless compression case > i965: Add means for limiting color resolves > i965: Add a flag telling color resolve pass to ignore CCS_E > i965: Add a few assertions on lossless compression > i965: Set buffer cleared after actually clearing it > i965/gen9: Prepare surface state setup for lossless compression > i965/gen9: Refactor msrt mcs initialization > i965: Expose logic telling if non-msrt mcs is supported > i965/gen9: Setup MCS for compressed texture surfaces > i965: Add helper for checking for lossless compressible > i965/gen9: Enable lossless compression > > src/mesa/drivers/dri/i965/brw_blorp_blit.cpp| 21 +- > src/mesa/drivers/dri/i965/brw_context.c | 22 ++- > src/mesa/drivers/dri/i965/brw_context.h | 2 +- > src/mesa/drivers/dri/i965/brw_defines.h | 2 + > src/mesa/drivers/dri/i965/brw_meta_fast_clear.c | 16 +- > src/mesa/drivers/dri/i965/brw_surface_formats.c | 2 +- > src/mesa/drivers/dri/i965/brw_tex_layout.c | 2 + > src/mesa/drivers/dri/i965/gen8_surface_state.c | 90 + > src/mesa/drivers/dri/i965/intel_blit.c | 4 +- > src/mesa/drivers/dri/i965/intel_copy_image.c| 4 +- > src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 249 > +--- > src/mesa/drivers/dri/i965/intel_mipmap_tree.h | 32 ++- > src/mesa/drivers/dri/i965/intel_pixel_bitmap.c | 2 +- > src/mesa/drivers/dri/i965/intel_pixel_read.c| 2 +- > src/mesa/drivers/dri/i965/intel_tex_image.c | 2 +- > src/mesa/drivers/dri/i965/intel_tex_subimage.c | 2 +- > 16 files changed, 318 insertions(+), 136 deletions(-) > Need to take a break, my head hurts. In addition to the comments I already left, 5, 8, 9, 10, and 11 are: Reviewed-by: Ben WidawskyI think they might change at least a bit if you consider the feedback I've given so far, but it's really up to you. 8 is kind of useless on its own IMO, but whatever you like. I'd say you should push 10, and 11 now. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] i965/gen9: Compression support for single-sampled
This series enables compression for single sampled color surfaces, also referred to as "lossless compression". This is yet only for driver internal use easing pressure on memory bandwidth and caches when writing, blending and sampling surfaces uing gpu. As a side effect the need for color buffer resolves after fast clears is also decreased. Current understanding is that sampling engine doesn't understand meta data (auxiliary buffer) for single sampled fast cleared surfaces. However, if the meta data is written with lossless compression enabled, even sampling engine is capable of reading both the color buffer and the auxiliary, and resolves can be omitted in those case. The final enabling patch is dependent on earlier two-patch series fixing state restore mechanism in i965-meta operations. There are some performance numbers available in the final commit. Topi Pohjolainen (23): i965: Let caller of intel_miptree_create_layout() decide msaa layout i965: Use miptree non-aligned dimensions directly for x-tiled i965: Separate miptree creation from auxiliary buffer setup i965: Don't try to create aux buffer for non-msrt aux-buffer i965: Stop considering if msrt aux buffers need aux buffer i965/gen9: Add buffer layout representing lossless compression i965/gen9: Allow halign == 16 also for losslessly compressed i965: Allow fast clear to be used with lossless compression i965: Add resolve option for lossless compression i965: Use constant pointer when checking for compression i965/gen8: Remove dead assertion i965: Refactor resolving of auxiliary mode i965: Resolve color buffer also in lossless compression case i965: Add means for limiting color resolves i965: Add a flag telling color resolve pass to ignore CCS_E i965: Add a few assertions on lossless compression i965: Set buffer cleared after actually clearing it i965/gen9: Prepare surface state setup for lossless compression i965/gen9: Refactor msrt mcs initialization i965: Expose logic telling if non-msrt mcs is supported i965/gen9: Setup MCS for compressed texture surfaces i965: Add helper for checking for lossless compressible i965/gen9: Enable lossless compression src/mesa/drivers/dri/i965/brw_blorp_blit.cpp| 21 +- src/mesa/drivers/dri/i965/brw_context.c | 22 ++- src/mesa/drivers/dri/i965/brw_context.h | 2 +- src/mesa/drivers/dri/i965/brw_defines.h | 2 + src/mesa/drivers/dri/i965/brw_meta_fast_clear.c | 16 +- src/mesa/drivers/dri/i965/brw_surface_formats.c | 2 +- src/mesa/drivers/dri/i965/brw_tex_layout.c | 2 + src/mesa/drivers/dri/i965/gen8_surface_state.c | 90 + src/mesa/drivers/dri/i965/intel_blit.c | 4 +- src/mesa/drivers/dri/i965/intel_copy_image.c| 4 +- src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 249 +--- src/mesa/drivers/dri/i965/intel_mipmap_tree.h | 32 ++- src/mesa/drivers/dri/i965/intel_pixel_bitmap.c | 2 +- src/mesa/drivers/dri/i965/intel_pixel_read.c| 2 +- src/mesa/drivers/dri/i965/intel_tex_image.c | 2 +- src/mesa/drivers/dri/i965/intel_tex_subimage.c | 2 +- 16 files changed, 318 insertions(+), 136 deletions(-) -- 2.5.0 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev