[Mesa-dev] [PATCH v4 7/7] i965: enable INTEL_blackhole_render
v2: condition the extension on context isolation support from the
kernel (Chris)
v3: (Lionel)
The initial version of this change used a feature of the Gen7+
command parser to turn the primitive instructions into no-ops.
Unfortunately this doesn't play well with how we're using the
hardware outside of the user submitted commands. For example
resolves are implicit operations which should not be turned into
no-ops as part of the previously submitted commands (before
blackhole_render is enabled) might not be disabled. For example
this sequence :
glClear();
glEnable(GL_BLACKHOLE_RENDER_INTEL);
glDrawArrays(...);
glReadPixels(...);
glDisable(GL_BLACKHOLE_RENDER_INTEL);
While clear has been emitted outside the blackhole render, it
should still be resolved properly in the read pixels. Hence we
need to be more selective and only disable user submitted
commands.
This v3 manually turns primitives into MI_NOOP if blackhole render
is enabled. This lets us enable this feature on any platform.
v4: Limit support to gen7.5+ (Lionel)
v5: Enable Gen7.5 support again, requires a kernel update of the
command parser (Lionel)
v6: Disable Gen7.5 again... Kernel devs want these patches landed
before they accept the kernel patches to whitelist INSTPM (Lionel)
Signed-off-by: Lionel Landwerlin
---
src/mesa/drivers/dri/i965/brw_clear.c | 3 +
src/mesa/drivers/dri/i965/brw_context.h | 2 +
src/mesa/drivers/dri/i965/brw_defines.h | 8 ++-
src/mesa/drivers/dri/i965/brw_misc_state.c| 56 +++
src/mesa/drivers/dri/i965/brw_state.h | 4 ++
src/mesa/drivers/dri/i965/brw_state_upload.c | 2 +
src/mesa/drivers/dri/i965/genX_state_upload.c | 4 ++
src/mesa/drivers/dri/i965/intel_extensions.c | 8 +++
src/mesa/drivers/dri/i965/intel_fbo.c | 6 ++
src/mesa/drivers/dri/i965/intel_pixel_read.c | 3 +
src/mesa/drivers/dri/i965/intel_tex_copy.c| 3 +
src/mesa/drivers/dri/i965/intel_tex_image.c | 5 ++
12 files changed, 103 insertions(+), 1 deletion(-)
diff --git a/src/mesa/drivers/dri/i965/brw_clear.c
b/src/mesa/drivers/dri/i965/brw_clear.c
index b097dfe346c..d3e360b3e23 100644
--- a/src/mesa/drivers/dri/i965/brw_clear.c
+++ b/src/mesa/drivers/dri/i965/brw_clear.c
@@ -247,6 +247,9 @@ brw_clear(struct gl_context *ctx, GLbitfield mask)
if (!_mesa_check_conditional_render(ctx))
return;
+ if (ctx->IntelBlackholeRender)
+ return;
+
if (mask & (BUFFER_BIT_FRONT_LEFT | BUFFER_BIT_FRONT_RIGHT)) {
brw->front_buffer_dirty = true;
}
diff --git a/src/mesa/drivers/dri/i965/brw_context.h
b/src/mesa/drivers/dri/i965/brw_context.h
index 0880d18b6f0..23602df2138 100644
--- a/src/mesa/drivers/dri/i965/brw_context.h
+++ b/src/mesa/drivers/dri/i965/brw_context.h
@@ -218,6 +218,7 @@ enum brw_state_id {
BRW_STATE_CONSERVATIVE_RASTERIZATION,
BRW_STATE_DRAW_CALL,
BRW_STATE_AUX,
+ BRW_STATE_CS_NOOP,
BRW_NUM_STATE_BITS
};
@@ -309,6 +310,7 @@ enum brw_state_id {
#define BRW_NEW_CONSERVATIVE_RASTERIZATION (1ull <<
BRW_STATE_CONSERVATIVE_RASTERIZATION)
#define BRW_NEW_DRAW_CALL (1ull << BRW_STATE_DRAW_CALL)
#define BRW_NEW_AUX_STATE (1ull << BRW_STATE_AUX)
+#define BRW_NEW_CS_NOOP (1ull << BRW_STATE_CS_NOOP)
struct brw_state_flags {
/** State update flags signalled by mesa internals */
diff --git a/src/mesa/drivers/dri/i965/brw_defines.h
b/src/mesa/drivers/dri/i965/brw_defines.h
index 320426d6944..4e2d6acc706 100644
--- a/src/mesa/drivers/dri/i965/brw_defines.h
+++ b/src/mesa/drivers/dri/i965/brw_defines.h
@@ -1651,11 +1651,17 @@ enum brw_pixel_shader_coverage_mask_mode {
#define GEN10_CACHE_MODE_SS0x0e420
#define GEN10_FLOAT_BLEND_OPTIMIZATION_ENABLE (1 << 4)
-#define INSTPM 0x20c0
+#define INSTPM 0x20c0 /* Gen6-8 */
# define INSTPM_CONSTANT_BUFFER_ADDRESS_OFFSET_DISABLE (1 << 6)
+# define INSTPM_GLOBAL_DEBUG_ENABLE(1 << 4)
+# define INSTPM_MEDIA_INSTRUCTION_DISABLE (1 << 3)
+# define INSTPM_3D_RENDERER_INSTRUCTION_DISABLE(1 << 2)
+# define INSTPM_3D_STATE_INSTRUCTION_DISABLE (1 << 1)
#define CS_DEBUG_MODE2 0x20d8 /* Gen9+ */
# define CSDBG2_CONSTANT_BUFFER_ADDRESS_OFFSET_DISABLE (1 << 4)
+# define CSDBG2_MEDIA_INSTRUCTION_DISABLE (1 << 1)
+# define CSDBG2_3D_RENDERER_INSTRUCTION_DISABLE(1 << 0)
#define GEN7_RPSTAT1 0xA01C
#define GEN7_RPSTAT1_CURR_GT_FREQ_SHIFT 7
diff --git a/src/mesa/drivers/dri/i965/brw_misc_state.c
b/src/mesa/drivers/dri/i965/brw_misc_state.c
index 9a663b1d61c..baf64757b93 100644
--- a/src/mesa/drivers/dri/i965/brw_misc_state.c
+++ b/src/mesa/drivers/dri/i965/brw_misc_state.c
@@ -811,3 +811,59 @@ brw_upload_state_base_address(struct brw_context *brw)
brw->ctx.
Re: [Mesa-dev] [PATCH v4 7/7] i965: enable INTEL_blackhole_render
On Wed, Jun 20, 2018 at 06:25:34PM +0100, Lionel Landwerlin wrote:
> v2: condition the extension on context isolation support from the
> kernel (Chris)
>
> v3: (Lionel)
>
> The initial version of this change used a feature of the Gen7+
> command parser to turn the primitive instructions into no-ops.
> Unfortunately this doesn't play well with how we're using the
> hardware outside of the user submitted commands. For example
> resolves are implicit operations which should not be turned into
> no-ops as part of the previously submitted commands (before
> blackhole_render is enabled) might not be disabled. For example
> this sequence :
>
>glClear();
>glEnable(GL_BLACKHOLE_RENDER_INTEL);
>glDrawArrays(...);
>glReadPixels(...);
>glDisable(GL_BLACKHOLE_RENDER_INTEL);
>
> While clear has been emitted outside the blackhole render, it
> should still be resolved properly in the read pixels. Hence we
> need to be more selective and only disable user submitted
> commands.
>
> This v3 manually turns primitives into MI_NOOP if blackhole render
> is enabled. This lets us enable this feature on any platform.
>
> v4: Limit support to gen7.5+ (Lionel)
>
> v5: Enable Gen7.5 support again, requires a kernel update of the
> command parser (Lionel)
>
> v6: Disable Gen7.5 again... Kernel devs want these patches landed
> before they accept the kernel patches to whitelist INSTPM (Lionel)
Hm, this doesn't quite read how kernel patches are usually handled:
Ordering sequence is:
1. get everything reviewed and tested (both kernel and userspace), but do
not yet start merging
2. merge kernel (if you feel paranoid, wait until Dave Airlie accepted it
into drm-next)
3. merge userspace
Insisting that the userspace stuff lands before the kernel (even if it's
just prep work) is kinda the wrong way round, and needlessly complicates
the process.
This is all documented in full details in
https://dri.freedesktop.org/docs/drm/gpu/drm-uapi.html#open-source-userspace-requirements
Cheers, Daniel
>
> Signed-off-by: Lionel Landwerlin
> ---
> src/mesa/drivers/dri/i965/brw_clear.c | 3 +
> src/mesa/drivers/dri/i965/brw_context.h | 2 +
> src/mesa/drivers/dri/i965/brw_defines.h | 8 ++-
> src/mesa/drivers/dri/i965/brw_misc_state.c| 56 +++
> src/mesa/drivers/dri/i965/brw_state.h | 4 ++
> src/mesa/drivers/dri/i965/brw_state_upload.c | 2 +
> src/mesa/drivers/dri/i965/genX_state_upload.c | 4 ++
> src/mesa/drivers/dri/i965/intel_extensions.c | 8 +++
> src/mesa/drivers/dri/i965/intel_fbo.c | 6 ++
> src/mesa/drivers/dri/i965/intel_pixel_read.c | 3 +
> src/mesa/drivers/dri/i965/intel_tex_copy.c| 3 +
> src/mesa/drivers/dri/i965/intel_tex_image.c | 5 ++
> 12 files changed, 103 insertions(+), 1 deletion(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_clear.c
> b/src/mesa/drivers/dri/i965/brw_clear.c
> index b097dfe346c..d3e360b3e23 100644
> --- a/src/mesa/drivers/dri/i965/brw_clear.c
> +++ b/src/mesa/drivers/dri/i965/brw_clear.c
> @@ -247,6 +247,9 @@ brw_clear(struct gl_context *ctx, GLbitfield mask)
> if (!_mesa_check_conditional_render(ctx))
>return;
>
> + if (ctx->IntelBlackholeRender)
> + return;
> +
> if (mask & (BUFFER_BIT_FRONT_LEFT | BUFFER_BIT_FRONT_RIGHT)) {
>brw->front_buffer_dirty = true;
> }
> diff --git a/src/mesa/drivers/dri/i965/brw_context.h
> b/src/mesa/drivers/dri/i965/brw_context.h
> index 0880d18b6f0..23602df2138 100644
> --- a/src/mesa/drivers/dri/i965/brw_context.h
> +++ b/src/mesa/drivers/dri/i965/brw_context.h
> @@ -218,6 +218,7 @@ enum brw_state_id {
> BRW_STATE_CONSERVATIVE_RASTERIZATION,
> BRW_STATE_DRAW_CALL,
> BRW_STATE_AUX,
> + BRW_STATE_CS_NOOP,
> BRW_NUM_STATE_BITS
> };
>
> @@ -309,6 +310,7 @@ enum brw_state_id {
> #define BRW_NEW_CONSERVATIVE_RASTERIZATION (1ull <<
> BRW_STATE_CONSERVATIVE_RASTERIZATION)
> #define BRW_NEW_DRAW_CALL (1ull << BRW_STATE_DRAW_CALL)
> #define BRW_NEW_AUX_STATE (1ull << BRW_STATE_AUX)
> +#define BRW_NEW_CS_NOOP (1ull << BRW_STATE_CS_NOOP)
>
> struct brw_state_flags {
> /** State update flags signalled by mesa internals */
> diff --git a/src/mesa/drivers/dri/i965/brw_defines.h
> b/src/mesa/drivers/dri/i965/brw_defines.h
> index 320426d6944..4e2d6acc706 100644
> --- a/src/mesa/drivers/dri/i965/brw_defines.h
> +++ b/src/mesa/drivers/dri/i965/brw_defines.h
> @@ -1651,11 +1651,17 @@ enum brw_pixel_shader_coverage_mask_mode {
> #define GEN10_CACHE_MODE_SS0x0e420
> #define GEN10_FLOAT_BLEND_OPTIMIZATION_ENABLE (1 << 4)
>
> -#define INSTPM 0x20c0
> +#define INSTPM 0x20c0 /* Gen6-8 */
> # define INSTPM_CONSTANT_BUFFER_ADDRESS_OFFSET_DISABLE (1 << 6)
> +# define INSTPM_GLOBAL_DEBUG_ENABLE(1 << 4)
> +#
Re: [Mesa-dev] [PATCH v4 7/7] i965: enable INTEL_blackhole_render
On 10/07/18 15:04, Daniel Vetter wrote: On Wed, Jun 20, 2018 at 06:25:34PM +0100, Lionel Landwerlin wrote: v2: condition the extension on context isolation support from the kernel (Chris) v3: (Lionel) The initial version of this change used a feature of the Gen7+ command parser to turn the primitive instructions into no-ops. Unfortunately this doesn't play well with how we're using the hardware outside of the user submitted commands. For example resolves are implicit operations which should not be turned into no-ops as part of the previously submitted commands (before blackhole_render is enabled) might not be disabled. For example this sequence : glClear(); glEnable(GL_BLACKHOLE_RENDER_INTEL); glDrawArrays(...); glReadPixels(...); glDisable(GL_BLACKHOLE_RENDER_INTEL); While clear has been emitted outside the blackhole render, it should still be resolved properly in the read pixels. Hence we need to be more selective and only disable user submitted commands. This v3 manually turns primitives into MI_NOOP if blackhole render is enabled. This lets us enable this feature on any platform. v4: Limit support to gen7.5+ (Lionel) v5: Enable Gen7.5 support again, requires a kernel update of the command parser (Lionel) v6: Disable Gen7.5 again... Kernel devs want these patches landed before they accept the kernel patches to whitelist INSTPM (Lionel) Hm, this doesn't quite read how kernel patches are usually handled: Ordering sequence is: 1. get everything reviewed and tested (both kernel and userspace), but do not yet start merging 2. merge kernel (if you feel paranoid, wait until Dave Airlie accepted it into drm-next) 3. merge userspace Insisting that the userspace stuff lands before the kernel (even if it's just prep work) is kinda the wrong way round, and needlessly complicates the process. This is all documented in full details in https://dri.freedesktop.org/docs/drm/gpu/drm-uapi.html#open-source-userspace-requirements Cheers, Daniel Hey Daniel, I remember somebody using the work "land" on IRC but it could have been a mistake :) This is also to avoid committing code that might end up being wrong if somebody bumps the command parser version before my patches. Thanks for reminder, - Lionel ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
