Re: [Mesa-dev] [PATCH 00/64] i965: Start using ISL for filling out surface states

[Chad Versace]

I can't find patches 1 and 2 in my inbox, but they have my r-b.
Reviewed-by: Chad Versace 

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 00/64] i965: Start using ISL for filling out surface states

[Emil Velikov]

On 13 June 2016 at 16:10, Jason Ekstrand  wrote:
>
>
> On Mon, Jun 13, 2016 at 2:24 AM, Emil Velikov 
> wrote:
>>
>> On 11 June 2016 at 17:02, Jason Ekstrand  wrote:
>>
>> > The next 20 patches or so are general ISL cleanups and fixes.  If no one
>> > is
>> > too opposed, I'd like to back-port the whole pile to 12.0.  There are
>> > two
>> > reasons for this: First, ISL is new and this is a substantial cleanup;
>> > back-porting it will make back-porting will keep the initial release of
>> > ISL
>> > cleaner and make back-porting other patches easier in the future.
>> > Second,
>> > in the middle of the series are a couple of changes that fix some 850
>> > Vulkan CTS tests on Haswell.
>> >
>> No serious objections on getting these in 12.0. A bunch of small
>> suggestions though.
>>
>> >   i965/gen4: Subtract 1 from buffer sizes
>> Please polish the commit message a bit.
>
>
> I added "The PRM states that the values put in Width, Height, and Depth
> should be various bits from the value size - 1.  We seem to have done this
> wrong more-or-less from the start."
>
>>
>> >   isl/state: Put surface format setup at the top
>> >   isl/state: Put all dimension setup together and towards the top
>> >   isl/state: Put pitch calculations together
>> Are these solely for cosmetic/ease of read reasons or there's
>> something more to it ? Can you please mention in the commit message ?
>
>
> How about: "This is purely cosmetic, but it makes things look a bit more
> readable."
>
Both are quite good. Thanks !

>>
>> >   genxml: Add enough XML for gens 4, 4.5, and 5 to get SURFACE_STATE
>> Let me see if I can quickly cook something for the Android build.
>>
>>
>> >   isl: Add support for filling out surface states all the way back to
>> > gen4
>> Ditto.
>
>
> If you want to give me something, I'm happy to squash it in.
>
Should be on the list + your inbox.

>>
>>
>> >   isl/formats: Mark RAW as having a block size of 1 byte
>> Worth mentioning why we want that ?
>
>
> That patch can actually be dropped.  The original idea was that, for buffer
> surfaces, we needed to divide by the element size so the size should be
> non-zero.  Turns out we need to divide by stride so it can be 0 and the
> patch isn't needed.  I'll defer to Chad on whether it should be 0 or 1.
>
If things change and we do want the patch, the above (or alike) as
commit message would be amazing.

-Emil
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 00/64] i965: Start using ISL for filling out surface states

[Jason Ekstrand]

On Mon, Jun 13, 2016 at 2:24 AM, Emil Velikov 
wrote:

> On 11 June 2016 at 17:02, Jason Ekstrand  wrote:
>
> > The next 20 patches or so are general ISL cleanups and fixes.  If no one
> is
> > too opposed, I'd like to back-port the whole pile to 12.0.  There are two
> > reasons for this: First, ISL is new and this is a substantial cleanup;
> > back-porting it will make back-porting will keep the initial release of
> ISL
> > cleaner and make back-porting other patches easier in the future.
> Second,
> > in the middle of the series are a couple of changes that fix some 850
> > Vulkan CTS tests on Haswell.
> >
> No serious objections on getting these in 12.0. A bunch of small
> suggestions though.
>
> >   i965/gen4: Subtract 1 from buffer sizes
> Please polish the commit message a bit.
>

I added "The PRM states that the values put in Width, Height, and Depth
should be various bits from the value size - 1.  We seem to have done this
wrong more-or-less from the start."


> >   isl/state: Put surface format setup at the top
> >   isl/state: Put all dimension setup together and towards the top
> >   isl/state: Put pitch calculations together
> Are these solely for cosmetic/ease of read reasons or there's
> something more to it ? Can you please mention in the commit message ?
>

How about: "This is purely cosmetic, but it makes things look a bit more
readable."




> >   genxml: Add enough XML for gens 4, 4.5, and 5 to get SURFACE_STATE
> Let me see if I can quickly cook something for the Android build.
>
>
> >   isl: Add support for filling out surface states all the way back to
> > gen4
> Ditto.
>

If you want to give me something, I'm happy to squash it in.


>
> >   isl/formats: Mark RAW as having a block size of 1 byte
> Worth mentioning why we want that ?
>

That patch can actually be dropped.  The original idea was that, for buffer
surfaces, we needed to divide by the element size so the size should be
non-zero.  Turns out we need to divide by stride so it can be 0 and the
patch isn't needed.  I'll defer to Chad on whether it should be 0 or 1.


> Thanks
> Emil
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 00/64] i965: Start using ISL for filling out surface states

[Emil Velikov]

On 11 June 2016 at 17:02, Jason Ekstrand  wrote:

> The next 20 patches or so are general ISL cleanups and fixes.  If no one is
> too opposed, I'd like to back-port the whole pile to 12.0.  There are two
> reasons for this: First, ISL is new and this is a substantial cleanup;
> back-porting it will make back-porting will keep the initial release of ISL
> cleaner and make back-porting other patches easier in the future.  Second,
> in the middle of the series are a couple of changes that fix some 850
> Vulkan CTS tests on Haswell.
>
No serious objections on getting these in 12.0. A bunch of small
suggestions though.

>   i965/gen4: Subtract 1 from buffer sizes
Please polish the commit message a bit.

>   isl/state: Put surface format setup at the top
>   isl/state: Put all dimension setup together and towards the top
>   isl/state: Put pitch calculations together
Are these solely for cosmetic/ease of read reasons or there's
something more to it ? Can you please mention in the commit message ?

>   genxml: Add enough XML for gens 4, 4.5, and 5 to get SURFACE_STATE
Let me see if I can quickly cook something for the Android build.


>   isl: Add support for filling out surface states all the way back to
> gen4
Ditto.


>   isl/formats: Mark RAW as having a block size of 1 byte
Worth mentioning why we want that ?

Thanks
Emil
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 00/64] i965: Start using ISL for filling out surface states

[Jason Ekstrand]

For those of you who like branches.  The whole series can be found here:

https://cgit.freedesktop.org/~jekstrand/mesa/log/?h=review/i965-isl-v1

On Sat, Jun 11, 2016 at 9:02 AM, Jason Ekstrand 
wrote:

> We would like to eventually start using ISL inside of the GL driver to
> replace the fairly sprawling layout code in brw_tex_layout.c and
> intel_mipmap_tree.c.  However, that is a very big change that no one is
> ready to make yet.  A smaller change, I thought, would be to start using
> ISL in blorp.  In order to do that, I needed a function to get an isl_surf
> from an intel_mipmap_tree.  How do you test such a function to ensure that
> it's working in all of the cases?  Use ISL for emitting all surface states
> on everything and run it through Jenkins of course!  Hence this series.
>
> This series is one of the most educational projects I've worked on in a
> bit.  It turns out there are a lot of subtlties in surface layout and I
> found bugs in both the i965 and ISL state setup code.  I've tried to keep
> all of the functional changes contained to the first 8 or so patches which
> only touch the GL driver.  That way those fixes can be back-ported to
> stable and are bisectable.
>
> The next 20 patches or so are general ISL cleanups and fixes.  If no one is
> too opposed, I'd like to back-port the whole pile to 12.0.  There are two
> reasons for this: First, ISL is new and this is a substantial cleanup;
> back-porting it will make back-porting will keep the initial release of ISL
> cleaner and make back-porting other patches easier in the future.  Second,
> in the middle of the series are a couple of changes that fix some 850
> Vulkan CTS tests on Haswell.
>
> The next 9 patches add support to ISL for filling out surface states on
> gen4, 4x, 5, and 6 as well as support for color compression.  I'm not sure
> if the CCS formats are 100% correct or of that's even the exact approach we
> want to take.  Chad, I'd like you to chip in here.
>
> Finally, starting with blorp, we replace almost all of the surface state
> setup code in i965 with paths based on ISL.  For textures/renderbuffers we
> delete 1 path for gen4-5, 3 for gen6, 4 for gen7, and 3 for gen8 along with
> 3 different paths for emitting buffer surfaces.
>
> As far as review goes, I'd like to get the i965 bugfixes and ISL cleanups
> landed soon-ish and back-ported for 12.0.  Everything after that is a bit
> more up-in-the-air.  It won't be all that hard to rebase because it's mosly
> just whole-sale replacing the code we have with new code.
>
> Cc: Chad Versace 
> Cc: Nanley Chery 
> Cc: Kenneth Graunke 
> Cc: Topi Pohjolainen 
>
> Jason Ekstrand (64):
>   i965: Drop Max3DTextureLevels to 512 on Sandy Bridge and prior
>   i965/blorp/gen8: Use the correct max level and layer in
> emit_surface_states
>   i965/gen8: Use the qpitch from the aux_mt for AUX_QPITCH
>   i965/fs: Use a default Y coordinate of 0 for TXF on gen9+
>   i965: Remove fake W-tiled render target support
>   i965/gen4: Subtract 1 from buffer sizes
>   i965/gen7,8: Set SURFACE_IS_ARRAY for all non-3D texture types
>   i965/blorp: Only set src_z for gen8+ 3D textures
>   genxml/gen8,9: Prefix the multisample format enum with MSFMT
>   isl/state: Don't use designated initializers for the surface state
>   isl/state: Remove some unused fields
>   isl/state: Put surface format setup at the top
>   isl/state: Put all dimension setup together and towards the top
>   isl/state: Put pitch calculations together
>   isl/state: Return an extent3d from the halign/valign helper
>   isl/state: Refactor the per-gen isl_to_gen_h/valign tables
>   isl/state: Refactor the setup of clear colors
>   isl/state: Don't force-disable L2 bypass for everything
>   isl/state: Set SurfaceArray based on the surface dimension
>   isl/format: Mark R9G9B9E5 as containing 9-bit unsigned float channels
>   isl/state: Set the IntegerSurfaceFormat bit on Haswell
>   isl/state: Use the layout for computing qpitch rather than dimensions
>   isl/state: Only set cube face enables if usage includes CUBE_BIT
>   isl/state: Emit no-op mip tail setup on SKL
>   isl/state: Use TILEWALK_XMAJOR for linear surfaces on gen7
>   isl/state: Don't set SurfacePitch for gen9 1-D textures
>   isl/state: Add assertions for buffer surface restrictions
>   isl/state: Don't use designated initializers for buffer surface state
>   isl/state: Allow for full 31-bit buffer texture sizes
>   anv,isl: Lower storage image formats in anv
>   genxml: Put append counter fields before MCS in RENDER_SURFACE_STATE
> on gen7
>   genxml: Add enough XML for gens 4, 4.5, and 5 to get SURFACE_STATE
>   genxml: Make X/Y Offset field of SURFACE_STATE a uint
>   genxml: Add macros and #includes for gens 4-6
>   isl: Add an ISL_DEV_IS_G4X macro
>   isl: Add support for filling out surface states all the way back to
> gen4
>   isl: Add surface formats for on-MSAA CCS surfaces
>   isl/state: Add support for handling color control surfaces
>   isl/stat

[Mesa-dev] [PATCH 00/64] i965: Start using ISL for filling out surface states

[Jason Ekstrand]

We would like to eventually start using ISL inside of the GL driver to
replace the fairly sprawling layout code in brw_tex_layout.c and
intel_mipmap_tree.c.  However, that is a very big change that no one is
ready to make yet.  A smaller change, I thought, would be to start using
ISL in blorp.  In order to do that, I needed a function to get an isl_surf
from an intel_mipmap_tree.  How do you test such a function to ensure that
it's working in all of the cases?  Use ISL for emitting all surface states
on everything and run it through Jenkins of course!  Hence this series.

This series is one of the most educational projects I've worked on in a
bit.  It turns out there are a lot of subtlties in surface layout and I
found bugs in both the i965 and ISL state setup code.  I've tried to keep
all of the functional changes contained to the first 8 or so patches which
only touch the GL driver.  That way those fixes can be back-ported to
stable and are bisectable.

The next 20 patches or so are general ISL cleanups and fixes.  If no one is
too opposed, I'd like to back-port the whole pile to 12.0.  There are two
reasons for this: First, ISL is new and this is a substantial cleanup;
back-porting it will make back-porting will keep the initial release of ISL
cleaner and make back-porting other patches easier in the future.  Second,
in the middle of the series are a couple of changes that fix some 850
Vulkan CTS tests on Haswell.

The next 9 patches add support to ISL for filling out surface states on
gen4, 4x, 5, and 6 as well as support for color compression.  I'm not sure
if the CCS formats are 100% correct or of that's even the exact approach we
want to take.  Chad, I'd like you to chip in here.

Finally, starting with blorp, we replace almost all of the surface state
setup code in i965 with paths based on ISL.  For textures/renderbuffers we
delete 1 path for gen4-5, 3 for gen6, 4 for gen7, and 3 for gen8 along with
3 different paths for emitting buffer surfaces.

As far as review goes, I'd like to get the i965 bugfixes and ISL cleanups
landed soon-ish and back-ported for 12.0.  Everything after that is a bit
more up-in-the-air.  It won't be all that hard to rebase because it's mosly
just whole-sale replacing the code we have with new code.

Cc: Chad Versace 
Cc: Nanley Chery 
Cc: Kenneth Graunke 
Cc: Topi Pohjolainen 

Jason Ekstrand (64):
  i965: Drop Max3DTextureLevels to 512 on Sandy Bridge and prior
  i965/blorp/gen8: Use the correct max level and layer in
emit_surface_states
  i965/gen8: Use the qpitch from the aux_mt for AUX_QPITCH
  i965/fs: Use a default Y coordinate of 0 for TXF on gen9+
  i965: Remove fake W-tiled render target support
  i965/gen4: Subtract 1 from buffer sizes
  i965/gen7,8: Set SURFACE_IS_ARRAY for all non-3D texture types
  i965/blorp: Only set src_z for gen8+ 3D textures
  genxml/gen8,9: Prefix the multisample format enum with MSFMT
  isl/state: Don't use designated initializers for the surface state
  isl/state: Remove some unused fields
  isl/state: Put surface format setup at the top
  isl/state: Put all dimension setup together and towards the top
  isl/state: Put pitch calculations together
  isl/state: Return an extent3d from the halign/valign helper
  isl/state: Refactor the per-gen isl_to_gen_h/valign tables
  isl/state: Refactor the setup of clear colors
  isl/state: Don't force-disable L2 bypass for everything
  isl/state: Set SurfaceArray based on the surface dimension
  isl/format: Mark R9G9B9E5 as containing 9-bit unsigned float channels
  isl/state: Set the IntegerSurfaceFormat bit on Haswell
  isl/state: Use the layout for computing qpitch rather than dimensions
  isl/state: Only set cube face enables if usage includes CUBE_BIT
  isl/state: Emit no-op mip tail setup on SKL
  isl/state: Use TILEWALK_XMAJOR for linear surfaces on gen7
  isl/state: Don't set SurfacePitch for gen9 1-D textures
  isl/state: Add assertions for buffer surface restrictions
  isl/state: Don't use designated initializers for buffer surface state
  isl/state: Allow for full 31-bit buffer texture sizes
  anv,isl: Lower storage image formats in anv
  genxml: Put append counter fields before MCS in RENDER_SURFACE_STATE
on gen7
  genxml: Add enough XML for gens 4, 4.5, and 5 to get SURFACE_STATE
  genxml: Make X/Y Offset field of SURFACE_STATE a uint
  genxml: Add macros and #includes for gens 4-6
  isl: Add an ISL_DEV_IS_G4X macro
  isl: Add support for filling out surface states all the way back to
gen4
  isl: Add surface formats for on-MSAA CCS surfaces
  isl/state: Add support for handling color control surfaces
  isl/state: Add support for OffsetX/Y in surface state
  i965/miptree: Add a helper for getting an isl_surf from a miptree
  i965/miptree: Add a helper for getting the ISL clear color from a
miptree
  i965/miptree: Add a helper for getting the aux isl_surf from a miptree
  i965/blorp: Add a generic ISL-based surface state emit path
  i965/blorp: Use the generic ISL path fo