[Mesa-dev] [PATCH 3/3] radeonsi/gfx9: implement primitive binning

[Nicolai Hähnle]

On 04.09.2017 20:04, Marek Olšák wrote:
> I actually made a mistake while porting the code. All UINT_MAX
> occurences should stay, and UINT_MAX should be the terminator, so I'm
> adding this:

Does it really make a difference? Let's take the 1 RB / 1 SE case.

With sum == 16, both loops run until i == 3 and then break due to the 
if-condition, returning {16, 128}.

With sum >= 17, the original loop runs until i == 4 and exits before the 
body at i == 4, returning {0, 0}.

The new loop runs until i == 4 and then breaks due to the if-condition, 
also returning {0, 0} (actually, except when sum == UINT_MAX, where 
there's an additional iteration, but sum is never that big).

On depth-only rendering, I don't quite understand the argument. In the 1 
RB / 1 SE case for example, this code returns {128, 128} for color and 
{128, 256} for depth and then chooses {128, 128} because of the smaller 
area. Wouldn't {128, 256} be more appropriate as it reflects the DB 
cache size?

Larger bins probably mean that DFSM punchout isn't as effective, though 
I wonder how much punchout helps for depth-only rendering anyway.

Anyway. It's more important to land a first implementation, and I think 
this is good enough. I still think it makes sense to remove the UINT_MAX 
lines (though it's possible I missed some of the logic), but the 
alternative below is also okay. R-b either way.

Cheers,
Nicolai

> 
> diff --git a/src/gallium/drivers/radeonsi/si_state_binning.c
> b/src/gallium/drivers/radeonsi/si_state_binning.c
> index 56bcdc8..d75e86e 100644
> --- a/src/gallium/drivers/radeonsi/si_state_binning.c
> +++ b/src/gallium/drivers/radeonsi/si_state_binning.c
> @@ -55,7 +55,7 @@ static struct uvec2 si_find_bin_size(struct
> si_screen *sscreen,
>          const struct si_bin_size_map *subtable =
>                  &table[log_num_rb_per_se][log_num_se][0];
> 
> -       for (i = 0; subtable[i].bin_size_x != 0; i++) {
> +       for (i = 0; subtable[i].start != UINT_MAX; i++) {
>                  if (sum >= subtable[i].start && sum < subtable[i + 1].start)
>                          break;
>          }
> 
> Marek
> 


-- 
Lerne, wie die Welt wirklich ist,
Aber vergiss niemals, wie sie sein sollte.