[Mesa-dev] [RFC] i965: Attempt to merge some surface state setup logic

[Ben Widawsky]

On Wed, Jul 30, 2014 at 08:03:56PM +0300, Topi Pohjolainen wrote:
> First five patches refactor common decision making found in depth,
> texture and renderbuffer surface setup logic.
> 
> In principle, there is opportunity to share code between texture and
> renderbuffer setup logic (Kenneth has experimented with this earlier [1]).
> On the other hand there is also a lot of overlap between gen7 and
> gen8 renderbuffer paths (same applies for paths dealing with texture
> surfaces). I tried this in the last patch, and I like the way the code
> looks afterwards.
> 
> Anyway, I don't have strong preference which way to go and thought
> better to share the work before running regression tests on all
> SNB, IVB, HSW and BDW involved.
> 
> This is based on Jordan's layered rendering and can be found in:
> 
> git://people.freedesktop.org/~tpohjola/mesa:surf_state_refactor
> 
> [1] http://lists.freedesktop.org/archives/mesa-dev/2013-September/044693.html
> 
> Topi Pohjolainen (7):
>   i965: Refactor render target type resolving
>   i965/blorp: Refactor depth override
>   i965: Refactor renderbuffer depth override
>   i965: Refactor renderbuffer target override
>   i965: Refactor array type resolving
>   i965/gen7: Add support for gen8 tiling
>   i965: Merge common bits of gen7/8 renderbuffer surface setup
> 
>  src/mesa/drivers/dri/i965/brw_context.h           |  12 ++
>  src/mesa/drivers/dri/i965/brw_state.h             |  23 +++-
>  src/mesa/drivers/dri/i965/brw_wm_surface_state.c  |  16 +++
>  src/mesa/drivers/dri/i965/gen6_blorp.cpp          |  21 +--
>  src/mesa/drivers/dri/i965/gen6_depth_state.c      |  27 +---
>  src/mesa/drivers/dri/i965/gen6_surface_state.c    |  18 +--
>  src/mesa/drivers/dri/i965/gen7_blorp.cpp          |  23 +---
>  src/mesa/drivers/dri/i965/gen7_misc_state.c       |  31 +----
>  src/mesa/drivers/dri/i965/gen7_wm_surface_state.c | 149 +++++++++++-----------
>  src/mesa/drivers/dri/i965/gen8_depth_state.c      |  31 +----
>  src/mesa/drivers/dri/i965/gen8_surface_state.c    |  77 +----------
>  src/mesa/drivers/dri/i965/intel_fbo.h             |  14 ++
>  12 files changed, 158 insertions(+), 284 deletions(-)
> 

It looks like an overall improvement to me. I don't know how I feel
about patch 7. I always go back and forth on whether to combine these
things. And given the amount of churn in the HW between gen7, and gen8,
I am a bit hesitant. What was the reason Ken dropped that one?

I didn't spot anything obviously wrong in any of the patches.

Run the piglits I say

-- 
Ben Widawsky, Intel Open Source Technology Center