[PATCH] drm/i915: Add the predicate source registers to the register whitelist

[PATCH] drm/i915: Add the predicate source registers to the register whitelist

[Neil Roberts]

The predicate source registers are needed to implement conditional
rendering without stalling. The two source registers are used to load
the previous values of the PS_DEPTH_COUNT register saved from
PIPE_CONTROL commands. These can then be compared and used to set the
predicate enable bit via the MI_PREDICATE command.

Signed-off-by: Neil Roberts <neil@linux.intel.com>
---

There is a corresponding patch for Mesa which is using these registers
on the mailing list here:

http://lists.freedesktop.org/archives/mesa-dev/2014-November/070347.html

There are some other registers such as MI_PREDICATE_DATA which can be
used for more advanced predicate checking but I haven't added them to
the list because they aren't needed to implement
GL_NV_conditional_render.

 drivers/gpu/drm/i915/i915_cmd_parser.c | 2 ++
 drivers/gpu/drm/i915/i915_reg.h        | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_cmd_parser.c b/drivers/gpu/drm/i915/i915_cmd_parser.c
index 4c35e2a..9732155 100644
--- a/drivers/gpu/drm/i915/i915_cmd_parser.c
+++ b/drivers/gpu/drm/i915/i915_cmd_parser.c
@@ -408,6 +408,8 @@ static const u32 gen7_render_regs[] = {
 	REG64(PS_INVOCATION_COUNT),
 	REG64(PS_DEPTH_COUNT),
 	OACONTROL, /* Only allowed for LRI and SRM. See below. */
+	REG64(MI_PREDICATE_SRC0),
+	REG64(MI_PREDICATE_SRC1),
 	GEN7_3DPRIM_END_OFFSET,
 	GEN7_3DPRIM_START_VERTEX,
 	GEN7_3DPRIM_VERTEX_COUNT,
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index ea84e1e..9275d41 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -314,6 +314,8 @@
 #define   MI_BATCH_GTT		    (2<<6) /* aliased with (1<<7) on gen4 */
 #define MI_BATCH_BUFFER_START_GEN8	MI_INSTR(0x31, 1)
 
+#define MI_PREDICATE_SRC0	(0x2400)
+#define MI_PREDICATE_SRC1	(0x2408)
 
 #define MI_PREDICATE_RESULT_2	(0x2214)
 #define  LOWER_SLICE_ENABLED	(1<<0)
-- 
1.9.3

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Add the predicate source registers to the register whitelist

[Volkin, Bradley D]

On Fri, Nov 07, 2014 at 07:34:31AM -0800, Neil Roberts wrote:
> The predicate source registers are needed to implement conditional
> rendering without stalling. The two source registers are used to load
> the previous values of the PS_DEPTH_COUNT register saved from
> PIPE_CONTROL commands. These can then be compared and used to set the
> predicate enable bit via the MI_PREDICATE command.
> 
> Signed-off-by: Neil Roberts <neil@linux.intel.com>

Reviewed-by: Brad Volkin <bradley.d.volkin@intel.com>

> ---
> 
> There is a corresponding patch for Mesa which is using these registers
> on the mailing list here:
> 
> http://lists.freedesktop.org/archives/mesa-dev/2014-November/070347.html
> 
> There are some other registers such as MI_PREDICATE_DATA which can be
> used for more advanced predicate checking but I haven't added them to
> the list because they aren't needed to implement
> GL_NV_conditional_render.
> 
>  drivers/gpu/drm/i915/i915_cmd_parser.c | 2 ++
>  drivers/gpu/drm/i915/i915_reg.h        | 2 ++
>  2 files changed, 4 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_cmd_parser.c b/drivers/gpu/drm/i915/i915_cmd_parser.c
> index 4c35e2a..9732155 100644
> --- a/drivers/gpu/drm/i915/i915_cmd_parser.c
> +++ b/drivers/gpu/drm/i915/i915_cmd_parser.c
> @@ -408,6 +408,8 @@ static const u32 gen7_render_regs[] = {
>  	REG64(PS_INVOCATION_COUNT),
>  	REG64(PS_DEPTH_COUNT),
>  	OACONTROL, /* Only allowed for LRI and SRM. See below. */
> +	REG64(MI_PREDICATE_SRC0),
> +	REG64(MI_PREDICATE_SRC1),
>  	GEN7_3DPRIM_END_OFFSET,
>  	GEN7_3DPRIM_START_VERTEX,
>  	GEN7_3DPRIM_VERTEX_COUNT,
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index ea84e1e..9275d41 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -314,6 +314,8 @@
>  #define   MI_BATCH_GTT		    (2<<6) /* aliased with (1<<7) on gen4 */
>  #define MI_BATCH_BUFFER_START_GEN8	MI_INSTR(0x31, 1)
>  
> +#define MI_PREDICATE_SRC0	(0x2400)
> +#define MI_PREDICATE_SRC1	(0x2408)
>  
>  #define MI_PREDICATE_RESULT_2	(0x2214)
>  #define  LOWER_SLICE_ENABLED	(1<<0)
> -- 
> 1.9.3
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v2] drm/i915: Add the predicate source registers to the register whitelist

[Neil Roberts]

The predicate source registers are needed to implement conditional
rendering without stalling. The two source registers are used to load
the previous values of the PS_DEPTH_COUNT register saved from
PIPE_CONTROL commands. These can then be compared and used to set the
predicate enable bit via the MI_PREDICATE command.

The command parser version number is increased to 2 to make it easier
to detect the new functionality in user space.

Signed-off-by: Neil Roberts <neil@linux.intel.com>
---

Here is a v2 of the patch that also updates the command parser version
to make it easier to detect whether we can use the register in Mesa.
I have posted a second version of the Mesa patch which uses this here:

http://lists.freedesktop.org/archives/mesa-dev/2014-November/070362.html

This was suggested by Glenn Kennard.

 drivers/gpu/drm/i915/i915_cmd_parser.c | 6 +++++-
 drivers/gpu/drm/i915/i915_reg.h        | 2 ++
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_cmd_parser.c b/drivers/gpu/drm/i915/i915_cmd_parser.c
index 4c35e2a..3eb421e 100644
--- a/drivers/gpu/drm/i915/i915_cmd_parser.c
+++ b/drivers/gpu/drm/i915/i915_cmd_parser.c
@@ -408,6 +408,8 @@ static const u32 gen7_render_regs[] = {
 	REG64(PS_INVOCATION_COUNT),
 	REG64(PS_DEPTH_COUNT),
 	OACONTROL, /* Only allowed for LRI and SRM. See below. */
+	REG64(MI_PREDICATE_SRC0),
+	REG64(MI_PREDICATE_SRC1),
 	GEN7_3DPRIM_END_OFFSET,
 	GEN7_3DPRIM_START_VERTEX,
 	GEN7_3DPRIM_VERTEX_COUNT,
@@ -1054,6 +1056,8 @@ int i915_cmd_parser_get_version(void)
 	 *
 	 * 1. Initial version. Checks batches and reports violations, but leaves
 	 *    hardware parsing enabled (so does not allow new use cases).
+	 * 2. Allow access to the MI_PREDICATE_SRC0 and
+	 *    MI_PREDICATE_SRC1 registers.
 	 */
-	return 1;
+	return 2;
 }
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index ea84e1e..9275d41 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -314,6 +314,8 @@
 #define   MI_BATCH_GTT		    (2<<6) /* aliased with (1<<7) on gen4 */
 #define MI_BATCH_BUFFER_START_GEN8	MI_INSTR(0x31, 1)
 
+#define MI_PREDICATE_SRC0	(0x2400)
+#define MI_PREDICATE_SRC1	(0x2408)
 
 #define MI_PREDICATE_RESULT_2	(0x2214)
 #define  LOWER_SLICE_ENABLED	(1<<0)
-- 
1.9.3

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Add the predicate source registers to the register whitelist

[Kenneth Graunke]

[-- Attachment #1.1: Type: text/plain, Size: 2119 bytes --]

On Friday, November 07, 2014 03:34:31 PM Neil Roberts wrote:
> The predicate source registers are needed to implement conditional
> rendering without stalling. The two source registers are used to load
> the previous values of the PS_DEPTH_COUNT register saved from
> PIPE_CONTROL commands. These can then be compared and used to set the
> predicate enable bit via the MI_PREDICATE command.
> 
> Signed-off-by: Neil Roberts <neil@linux.intel.com>
> ---
> 
> There is a corresponding patch for Mesa which is using these registers
> on the mailing list here:
> 
> http://lists.freedesktop.org/archives/mesa-dev/2014-November/070347.html
> 
> There are some other registers such as MI_PREDICATE_DATA which can be
> used for more advanced predicate checking but I haven't added them to
> the list because they aren't needed to implement
> GL_NV_conditional_render.
> 
>  drivers/gpu/drm/i915/i915_cmd_parser.c | 2 ++
>  drivers/gpu/drm/i915/i915_reg.h        | 2 ++
>  2 files changed, 4 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_cmd_parser.c 
b/drivers/gpu/drm/i915/i915_cmd_parser.c
> index 4c35e2a..9732155 100644
> --- a/drivers/gpu/drm/i915/i915_cmd_parser.c
> +++ b/drivers/gpu/drm/i915/i915_cmd_parser.c
> @@ -408,6 +408,8 @@ static const u32 gen7_render_regs[] = {
>  	REG64(PS_INVOCATION_COUNT),
>  	REG64(PS_DEPTH_COUNT),
>  	OACONTROL, /* Only allowed for LRI and SRM. See below. */
> +	REG64(MI_PREDICATE_SRC0),
> +	REG64(MI_PREDICATE_SRC1),
>  	GEN7_3DPRIM_END_OFFSET,
>  	GEN7_3DPRIM_START_VERTEX,
>  	GEN7_3DPRIM_VERTEX_COUNT,
> diff --git a/drivers/gpu/drm/i915/i915_reg.h 
b/drivers/gpu/drm/i915/i915_reg.h
> index ea84e1e..9275d41 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -314,6 +314,8 @@
>  #define   MI_BATCH_GTT		    (2<<6) /* aliased with (1<<7) on gen4 */
>  #define MI_BATCH_BUFFER_START_GEN8	MI_INSTR(0x31, 1)
>  
> +#define MI_PREDICATE_SRC0	(0x2400)
> +#define MI_PREDICATE_SRC1	(0x2408)
>  
>  #define MI_PREDICATE_RESULT_2	(0x2214)
>  #define  LOWER_SLICE_ENABLED	(1<<0)
> 

Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>

[-- Attachment #1.2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2] drm/i915: Add the predicate source registers to the register whitelist

[Daniel Vetter]

On Fri, Nov 07, 2014 at 07:00:26PM +0000, Neil Roberts wrote:
> The predicate source registers are needed to implement conditional
> rendering without stalling. The two source registers are used to load
> the previous values of the PS_DEPTH_COUNT register saved from
> PIPE_CONTROL commands. These can then be compared and used to set the
> predicate enable bit via the MI_PREDICATE command.
> 
> The command parser version number is increased to 2 to make it easier
> to detect the new functionality in user space.
> 
> Signed-off-by: Neil Roberts <neil@linux.intel.com>
> ---
> 
> Here is a v2 of the patch that also updates the command parser version
> to make it easier to detect whether we can use the register in Mesa.
> I have posted a second version of the Mesa patch which uses this here:
> 
> http://lists.freedesktop.org/archives/mesa-dev/2014-November/070362.html
> 
> This was suggested by Glenn Kennard.

Ok, I've merged this one, since that means you at least won't get an
-EINVAL. But you can't yet enable the feature in mesa since it's still
no-opped out. Let's hope that v3 actually starts granting rights (but
please don't encode that in any merge mesa patch before it's official in
the kernel, Dave Airlie _will_ revert it even if it breaks the build) and
that this happens soon.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2] drm/i915: Add the predicate source registers to the register whitelist

[Neil Roberts]

Hi,

Daniel Vetter <daniel@ffwll.ch> writes:

> Ok, I've merged this one, since that means you at least won't get an
> -EINVAL. But you can't yet enable the feature in mesa since it's still
> no-opped out. Let's hope that v3 actually starts granting rights (but
> please don't encode that in any merge mesa patch before it's official in
> the kernel, Dave Airlie _will_ revert it even if it breaks the build) and
> that this happens soon.

Thanks for merging the patch.

The current patch proposed for Mesa only enables the feature if the
version is >= 2 *and* it can successfully do a pipeline write to some
other register. Checking whether it can write to a register is already
used to decide whether to enable some transform feedback extensions and
indirect draw calls. I think in that case it should be safe to land the
Mesa patches even though it would only work on IvyBridge and not Haswell
or Gen8+. If we fix the kernel to allow the write for Haswell too then I
think it would magically start working without any extra changes. I'm
not really sure if I understand what you're saying about it being
no-opped. It's effectively not no-opped on IvyBridge so I think we
should already be able to enable the feature. Do you agree?

- Neil

> -Daniel
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2] drm/i915: Add the predicate source registers to the register whitelist

[Daniel Vetter]

On Fri, Jan 09, 2015 at 04:41:10PM +0000, Neil Roberts wrote:
> Hi,
>
> Daniel Vetter <daniel@ffwll.ch> writes:
>
> > Ok, I've merged this one, since that means you at least won't get an
> > -EINVAL. But you can't yet enable the feature in mesa since it's still
> > no-opped out. Let's hope that v3 actually starts granting rights (but
> > please don't encode that in any merge mesa patch before it's official in
> > the kernel, Dave Airlie _will_ revert it even if it breaks the build) and
> > that this happens soon.
>
> Thanks for merging the patch.
>
> The current patch proposed for Mesa only enables the feature if the
> version is >= 2 *and* it can successfully do a pipeline write to some
> other register. Checking whether it can write to a register is already
> used to decide whether to enable some transform feedback extensions and
> indirect draw calls. I think in that case it should be safe to land the
> Mesa patches even though it would only work on IvyBridge and not Haswell
> or Gen8+. If we fix the kernel to allow the write for Haswell too then I
> think it would magically start working without any extra changes. I'm
> not really sure if I understand what you're saying about it being
> no-opped. It's effectively not no-opped on IvyBridge so I think we
> should already be able to enable the feature. Do you agree?

Makes sense, thanks for clarifying.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx