On Sat, Apr 23, 2016 at 05:40:25PM +0300, Pohjolainen, Topi wrote:
> On Sat, Apr 23, 2016 at 07:32:33AM -0700, Jason Ekstrand wrote:
> > On Apr 23, 2016 3:46 AM, "Pohjolainen, Topi"
> > <[1]topi.pohjolai...@intel.com> wrote:
> > >
> > > On Fri, Apr 22, 2016 at 04:19:08PM -0700, Jason Ekstrand wrote:
> > > > Instead of having a virtual member function for getting the WM/PS
> > kernel,
> > > > we simply add fields for prog_data and the kernel to
> > brw_blorp_parms and
> > > > always make sure those get set as part of the different
> > constructors.
> > > > ---
> > > > src/mesa/drivers/dri/i965/brw_blorp.cpp | 12 ++-----
> > > > src/mesa/drivers/dri/i965/brw_blorp.h | 19 +++++-----
> > > > src/mesa/drivers/dri/i965/brw_blorp_blit.cpp | 12 ++-----
> > > > src/mesa/drivers/dri/i965/brw_blorp_clear.cpp | 50
> > ++++++++++++---------------
> > > > src/mesa/drivers/dri/i965/gen6_blorp.cpp | 25 ++++++--------
> > > > src/mesa/drivers/dri/i965/gen7_blorp.cpp | 28 +++++++--------
> > > > src/mesa/drivers/dri/i965/gen8_blorp.cpp | 18 ++++------
> > > > 7 files changed, 68 insertions(+), 96 deletions(-)
> > > >
> > > > diff --git a/src/mesa/drivers/dri/i965/brw_blorp.cpp
> > b/src/mesa/drivers/dri/i965/brw_blorp.cpp
> > > > index ce09b09..9dbbd83 100644
> > > > --- a/src/mesa/drivers/dri/i965/brw_blorp.cpp
> > > > +++ b/src/mesa/drivers/dri/i965/brw_blorp.cpp
> > > > @@ -165,10 +165,11 @@ brw_blorp_params::brw_blorp_params(unsigned
> > num_varyings,
> > > > depth_format(0),
> > > > hiz_op(GEN6_HIZ_OP_NONE),
> > > > fast_clear_op(0),
> > > > - use_wm_prog(false),
> > > > num_varyings(num_varyings),
> > > > num_draw_buffers(num_draw_buffers),
> > > > - num_layers(num_layers)
> > > > + num_layers(num_layers),
> > > > + wm_prog_kernel(BRW_BLORP_NO_WM_PROG),
> > > > + wm_prog_data(NULL)
> > > > {
> > > > color_write_disable[0] = false;
> > > > color_write_disable[1] = false;
> > > > @@ -354,10 +355,3 @@ brw_hiz_op_params::brw_hiz_op_params(struct
> > intel_mipmap_tree *mt,
> > > > default: unreachable("not reached");
> > > > }
> > > > }
> > > > -
> > > > -uint32_t
> > > > -brw_hiz_op_params::get_wm_prog(struct brw_context *brw,
> > > > - brw_blorp_prog_data **prog_data)
> > const
> > > > -{
> > > > - return 0;
> > > > -}
> > > > diff --git a/src/mesa/drivers/dri/i965/brw_blorp.h
> > b/src/mesa/drivers/dri/i965/brw_blorp.h
> > > > index 79dc59a..4981afd 100644
> > > > --- a/src/mesa/drivers/dri/i965/brw_blorp.h
> > > > +++ b/src/mesa/drivers/dri/i965/brw_blorp.h
> > > > @@ -229,6 +229,7 @@ struct brw_blorp_prog_data
> > > > bool persample_msaa_dispatch;
> > > > };
> > > >
> > > > +#define BRW_BLORP_NO_WM_PROG 1
> > >
> > > So in other words any offset other than one is regarded as valid? I
> > was
> > > wondering if could drop this and use the existence of wm_prog_data to
> > tell
> > > if there is kernel available or not. At least in current form valid
> > kernel
> > > always has prog_data and vice versa.
> >
> > I thought about that and would be happy to make the change If you
> > wanted. I just couldn't decide which I thought was cleaner.
>
> If you don't mind. It is in the end matter of taste but somehow I find it
> cleaner. You can slab there:
>
> Reviewed-by: Topi Pohjolainen <topi.pohjolai...@intel.com>
>
> I'll go through the rest next.
Just a few comments that you can take or leave but the series:
Reviewed-by: Topi Pohjolainen <topi.pohjolai...@intel.com>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
