On Wednesday, April 22, 2015 11:47:34 PM Topi Pohjolainen wrote:
> Signed-off-by: Topi Pohjolainen <[email protected]>
> ---
>  src/mesa/drivers/dri/i965/brw_state.h     |  8 +++++
>  src/mesa/drivers/dri/i965/gen6_wm_state.c | 56 
> ++++++++++++++++++-------------
>  2 files changed, 41 insertions(+), 23 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_state.h 
> b/src/mesa/drivers/dri/i965/brw_state.h
> index 23f36c0..ca3274d 100644
> --- a/src/mesa/drivers/dri/i965/brw_state.h
> +++ b/src/mesa/drivers/dri/i965/brw_state.h
> @@ -292,6 +292,14 @@ void brw_update_sampler_state(struct brw_context *brw,
>                                uint32_t *sampler_state,
>                                uint32_t batch_offset_for_sampler_state);
>  
> +/* gen6_wm_state.c */
> +void
> +gen6_wm_state_set_programs(const struct brw_wm_prog_data *prog_data,
> +                           const struct brw_stage_state *stage_state,
> +                           int min_inv_per_frag,
> +                           uint32_t *ksp0, uint32_t *ksp2,
> +                           uint32_t *dw4, uint32_t *dw5, uint32_t *dw6);
> +
>  /* gen6_sf_state.c */
>  void
>  calculate_attr_overrides(const struct brw_context *brw,
> diff --git a/src/mesa/drivers/dri/i965/gen6_wm_state.c 
> b/src/mesa/drivers/dri/i965/gen6_wm_state.c
> index 8e673a4..bc921e5 100644
> --- a/src/mesa/drivers/dri/i965/gen6_wm_state.c
> +++ b/src/mesa/drivers/dri/i965/gen6_wm_state.c
> @@ -65,6 +65,37 @@ const struct brw_tracked_state gen6_wm_push_constants = {
>     .emit = gen6_upload_wm_push_constants,
>  };
>  
> +void
> +gen6_wm_state_set_programs(const struct brw_wm_prog_data *prog_data,
> +                           const struct brw_stage_state *stage_state,
> +                           int min_inv_per_frag,
> +                           uint32_t *ksp0, uint32_t *ksp2,
> +                           uint32_t *dw4, uint32_t *dw5, uint32_t *dw6)
> +{
> +   if (prog_data->prog_offset_16 || prog_data->no_8) {
> +      *dw5 |= GEN6_WM_16_DISPATCH_ENABLE;
> +
> +      if (!prog_data->no_8 && min_inv_per_frag == 1) {
> +         *dw5 |= GEN6_WM_8_DISPATCH_ENABLE;
> +         *dw4 |= (prog_data->base.dispatch_grf_start_reg <<
> +                  GEN6_WM_DISPATCH_START_GRF_SHIFT_0);
> +         *dw4 |= (prog_data->dispatch_grf_start_reg_16 <<
> +                  GEN6_WM_DISPATCH_START_GRF_SHIFT_2);
> +         *ksp0 = stage_state->prog_offset;
> +         *ksp2 = stage_state->prog_offset + prog_data->prog_offset_16;
> +      } else {
> +         *dw4 |= (prog_data->dispatch_grf_start_reg_16 <<
> +                  GEN6_WM_DISPATCH_START_GRF_SHIFT_0);
> +         *ksp0 = stage_state->prog_offset + prog_data->prog_offset_16;
> +      }
> +   } else {
> +      *dw5 |= GEN6_WM_8_DISPATCH_ENABLE;
> +      *dw4 |= (prog_data->base.dispatch_grf_start_reg <<
> +               GEN6_WM_DISPATCH_START_GRF_SHIFT_0);
> +      *ksp0 = stage_state->prog_offset;
> +   }
> +}
> +

This split feels awkward to me - the code to emit 3DSTATE_WM is now
split across multiple functions...and it has 5 out parameters.  I really
prefer keeping the code to fill out a packet's DWords together in one
function.

Could we keep it in one function, but instead make upload_wm_state()
take additional parameters, rather than poking at brw-> directly?

Sorry for the trouble...

Attachment: signature.asc
Description: This is a digitally signed message part.

_______________________________________________
mesa-dev mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to