Re: [Mesa-dev] [PATCH 06/31] i965/blorp: Use gen6_upload_urb
On Sun, Aug 28, 2016 at 10:29 PM, Pohjolainen, Topi <
topi.pohjolai...@gmail.com> wrote:
> On Fri, Aug 19, 2016 at 09:55:43AM -0700, Jason Ekstrand wrote:
> > ---
> > src/mesa/drivers/dri/i965/genX_blorp_exec.c | 6 ++
> > 1 file changed, 2 insertions(+), 4 deletions(-)
> >
> > diff --git a/src/mesa/drivers/dri/i965/genX_blorp_exec.c
> b/src/mesa/drivers/dri/i965/genX_blorp_exec.c
> > index a6ac7b0..402ae3f 100644
> > --- a/src/mesa/drivers/dri/i965/genX_blorp_exec.c
> > +++ b/src/mesa/drivers/dri/i965/genX_blorp_exec.c
> > @@ -200,9 +200,9 @@ static void
> > emit_urb_config(struct brw_context *brw,
> > const struct brw_blorp_params *params)
> > {
> > -#if GEN_GEN >= 7
> > const unsigned vs_entry_size = gen7_blorp_get_vs_entry_size(params);
>
> About using logic marked as gen7 also for gen6 further down: The name of
> function is misleading, there is nothing gen7 specific in the way the entry
> size is calculated (earlier gens like to have the size as 64 byte chunks as
> well).
>
> It looks that you would need to do unnecessary rebasing just to change the
> name. Perhaps do it as a follow up?
>
Patch incoming
> >
> > +#if GEN_GEN >= 7
> > if (!(brw->ctx.NewDriverState & (BRW_NEW_CONTEXT |
> BRW_NEW_URB_SIZE)) &&
> > brw->urb.vsize >= vs_entry_size)
> >return;
> > @@ -211,9 +211,7 @@ emit_urb_config(struct brw_context *brw,
> >
> > gen7_upload_urb(brw, vs_entry_size, false, false);
> > #else
> > - blorp_emit(brw, GENX(3DSTATE_URB), urb) {
> > - urb.VSNumberofURBEntries = brw->urb.max_vs_entries;
> > - }
>
> I wonder how correct this was before. Actual entry size
> (VSURBEntryAllocationSize) was left to zero. Now we start actually setting
> it.
>
Yeah, I was a bit disturbed by that as well. :/
> Reviewed-by: Topi Pohjolainen
>
Thanks!
> > + gen6_upload_urb(brw, vs_entry_size, false, 0);
> > #endif
> > }
> >
> > --
> > 2.5.0.400.gff86faf
> >
> > ___
> > mesa-dev mailing list
> > mesa-dev@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 06/31] i965/blorp: Use gen6_upload_urb
On Fri, Aug 19, 2016 at 09:55:43AM -0700, Jason Ekstrand wrote:
> ---
> src/mesa/drivers/dri/i965/genX_blorp_exec.c | 6 ++
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/genX_blorp_exec.c
> b/src/mesa/drivers/dri/i965/genX_blorp_exec.c
> index a6ac7b0..402ae3f 100644
> --- a/src/mesa/drivers/dri/i965/genX_blorp_exec.c
> +++ b/src/mesa/drivers/dri/i965/genX_blorp_exec.c
> @@ -200,9 +200,9 @@ static void
> emit_urb_config(struct brw_context *brw,
> const struct brw_blorp_params *params)
> {
> -#if GEN_GEN >= 7
> const unsigned vs_entry_size = gen7_blorp_get_vs_entry_size(params);
About using logic marked as gen7 also for gen6 further down: The name of
function is misleading, there is nothing gen7 specific in the way the entry
size is calculated (earlier gens like to have the size as 64 byte chunks as
well).
It looks that you would need to do unnecessary rebasing just to change the
name. Perhaps do it as a follow up?
>
> +#if GEN_GEN >= 7
> if (!(brw->ctx.NewDriverState & (BRW_NEW_CONTEXT | BRW_NEW_URB_SIZE)) &&
> brw->urb.vsize >= vs_entry_size)
>return;
> @@ -211,9 +211,7 @@ emit_urb_config(struct brw_context *brw,
>
> gen7_upload_urb(brw, vs_entry_size, false, false);
> #else
> - blorp_emit(brw, GENX(3DSTATE_URB), urb) {
> - urb.VSNumberofURBEntries = brw->urb.max_vs_entries;
> - }
I wonder how correct this was before. Actual entry size
(VSURBEntryAllocationSize) was left to zero. Now we start actually setting it.
Reviewed-by: Topi Pohjolainen
> + gen6_upload_urb(brw, vs_entry_size, false, 0);
> #endif
> }
>
> --
> 2.5.0.400.gff86faf
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 06/31] i965/blorp: Use gen6_upload_urb
On Fri, Aug 19, 2016 at 09:55:43AM -0700, Jason Ekstrand wrote:
> ---
> src/mesa/drivers/dri/i965/genX_blorp_exec.c | 6 ++
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/genX_blorp_exec.c
> b/src/mesa/drivers/dri/i965/genX_blorp_exec.c
> index a6ac7b0..402ae3f 100644
> --- a/src/mesa/drivers/dri/i965/genX_blorp_exec.c
> +++ b/src/mesa/drivers/dri/i965/genX_blorp_exec.c
> @@ -200,9 +200,9 @@ static void
> emit_urb_config(struct brw_context *brw,
> const struct brw_blorp_params *params)
> {
> -#if GEN_GEN >= 7
> const unsigned vs_entry_size = gen7_blorp_get_vs_entry_size(params);
Initially it looked a little odd to use logic marked for gen7 also for gen6.
In practise this looks to work though. I'll read a little further the series
to see how this turns out in the end.
>
> +#if GEN_GEN >= 7
> if (!(brw->ctx.NewDriverState & (BRW_NEW_CONTEXT | BRW_NEW_URB_SIZE)) &&
> brw->urb.vsize >= vs_entry_size)
>return;
> @@ -211,9 +211,7 @@ emit_urb_config(struct brw_context *brw,
>
> gen7_upload_urb(brw, vs_entry_size, false, false);
> #else
> - blorp_emit(brw, GENX(3DSTATE_URB), urb) {
> - urb.VSNumberofURBEntries = brw->urb.max_vs_entries;
> - }
> + gen6_upload_urb(brw, vs_entry_size, false, 0);
> #endif
> }
>
> --
> 2.5.0.400.gff86faf
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 06/31] i965/blorp: Use gen6_upload_urb
---
src/mesa/drivers/dri/i965/genX_blorp_exec.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/src/mesa/drivers/dri/i965/genX_blorp_exec.c
b/src/mesa/drivers/dri/i965/genX_blorp_exec.c
index a6ac7b0..402ae3f 100644
--- a/src/mesa/drivers/dri/i965/genX_blorp_exec.c
+++ b/src/mesa/drivers/dri/i965/genX_blorp_exec.c
@@ -200,9 +200,9 @@ static void
emit_urb_config(struct brw_context *brw,
const struct brw_blorp_params *params)
{
-#if GEN_GEN >= 7
const unsigned vs_entry_size = gen7_blorp_get_vs_entry_size(params);
+#if GEN_GEN >= 7
if (!(brw->ctx.NewDriverState & (BRW_NEW_CONTEXT | BRW_NEW_URB_SIZE)) &&
brw->urb.vsize >= vs_entry_size)
return;
@@ -211,9 +211,7 @@ emit_urb_config(struct brw_context *brw,
gen7_upload_urb(brw, vs_entry_size, false, false);
#else
- blorp_emit(brw, GENX(3DSTATE_URB), urb) {
- urb.VSNumberofURBEntries = brw->urb.max_vs_entries;
- }
+ gen6_upload_urb(brw, vs_entry_size, false, 0);
#endif
}
--
2.5.0.400.gff86faf
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
