Re: [Intel-gfx] [PATCH] drm/i915: Only check PPGTT bits when using PPGTT
Jesse reportedly has a patch somewhere to (finally!) enable ppgtt on
vlv. Would we still need any part of this with ppgtt support on vlv?
-Daniel
On Wed, May 28, 2014 at 10:47 PM, bradley.d.vol...@intel.com wrote:
From: Brad Volkin bradley.d.vol...@intel.com
This extends use of the command parser to VLV.
Note that the patch checks that the PPGTT bit is set appropriately when
PPGTT is enabled but ignores it when PPGTT is disabled. It would be
awkward to correctly invert the expected value to check that the bit is
set appropriately in that case, and of limited value anyhow.
Signed-off-by: Brad Volkin bradley.d.vol...@intel.com
---
I've confirmed that the shmem pread setup stuff we added does fix the
caching issues I saw previously. I've done some basic testing with this
on both IVB and VLV and don't see regressions. I don't have any data on
the VLV perf impact though.
Also, I considered splitting the patch up a bit differently but decided
that a single patch seemed ok. I'm happy to split it up a bit if that's
what people prefer.
drivers/gpu/drm/i915/i915_cmd_parser.c | 187
+
drivers/gpu/drm/i915/i915_drv.h| 8 +-
2 files changed, 104 insertions(+), 91 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_cmd_parser.c
b/drivers/gpu/drm/i915/i915_cmd_parser.c
index 9d79543..fd35900 100644
--- a/drivers/gpu/drm/i915/i915_cmd_parser.c
+++ b/drivers/gpu/drm/i915/i915_cmd_parser.c
@@ -110,6 +110,7 @@
#define W CMD_DESC_REGISTER
#define B CMD_DESC_BITMASK
#define M CMD_DESC_MASTER
+#define P CMD_DESC_PPGTT
/*Command Mask Fixed Len Action
-- */
@@ -124,20 +125,20 @@ static const struct drm_i915_cmd_descriptor
common_cmds[] = {
CMD( MI_STORE_DWORD_INDEX, SMI, !F, 0xFF, R ),
CMD( MI_LOAD_REGISTER_IMM(1), SMI, !F, 0xFF, W,
.reg = { .offset = 1, .mask = 0x007C } ),
- CMD( MI_STORE_REGISTER_MEM(1), SMI, !F, 0xFF, W | B,
+ CMD( MI_STORE_REGISTER_MEM(1), SMI, !F, 0xFF, W | P,
.reg = { .offset = 1, .mask = 0x007C },
- .bits = {{
+ .ppgtt = {
.offset = 0,
.mask = MI_GLOBAL_GTT,
.expected = 0,
- }}, ),
- CMD( MI_LOAD_REGISTER_MEM, SMI, !F, 0xFF, W | B,
+ }, ),
+ CMD( MI_LOAD_REGISTER_MEM, SMI, !F, 0xFF, W | P,
.reg = { .offset = 1, .mask = 0x007C },
- .bits = {{
+ .ppgtt = {
.offset = 0,
.mask = MI_GLOBAL_GTT,
.expected = 0,
- }}, ),
+ }, ),
CMD( MI_BATCH_BUFFER_START,SMI, !F, 0xFF, S ),
};
@@ -149,31 +150,31 @@ static const struct drm_i915_cmd_descriptor
render_cmds[] = {
CMD( MI_DISPLAY_FLIP, SMI, !F, 0xFF, R ),
CMD( MI_SET_CONTEXT, SMI, !F, 0xFF, R ),
CMD( MI_URB_CLEAR, SMI, !F, 0xFF, S ),
- CMD( MI_STORE_DWORD_IMM, SMI, !F, 0x3F, B,
- .bits = {{
+ CMD( MI_STORE_DWORD_IMM, SMI, !F, 0x3F, P,
+ .ppgtt = {
.offset = 0,
.mask = MI_GLOBAL_GTT,
.expected = 0,
- }}, ),
+ }, ),
CMD( MI_UPDATE_GTT,SMI, !F, 0xFF, R ),
- CMD( MI_CLFLUSH, SMI, !F, 0x3FF, B,
- .bits = {{
+ CMD( MI_CLFLUSH, SMI, !F, 0x3FF, P,
+ .ppgtt = {
.offset = 0,
.mask = MI_GLOBAL_GTT,
.expected = 0,
- }}, ),
- CMD( MI_REPORT_PERF_COUNT, SMI, !F, 0x3F, B,
- .bits = {{
+ }, ),
+ CMD( MI_REPORT_PERF_COUNT, SMI, !F, 0x3F, P,
+ .ppgtt = {
.offset = 1,
.mask = MI_REPORT_PERF_COUNT_GGTT,
.expected = 0,
- }}, ),
- CMD(
Re: [Intel-gfx] [PATCH] drm/i915: Only check PPGTT bits when using PPGTT
On Wed, May 28, 2014 at 02:29:34PM -0700, Daniel Vetter wrote:
Jesse reportedly has a patch somewhere to (finally!) enable ppgtt on
vlv. Would we still need any part of this with ppgtt support on vlv?
-Daniel
Of course, as soon as I accept that it'll never happen... :)
Off the top of my head, keeping this would let us run the command parser in
the case where ppgtt is force disabled, which might be nice. Otherwise the
parser stops working and you lose the ability to LRI whitelisted registers,
etc, which might not be what you expected.
My motivation was enabling the parser for vlv, which would happen without
any parser changes if it gets ppgtt, so I'll leave it up to you.
Brad
On Wed, May 28, 2014 at 10:47 PM, bradley.d.vol...@intel.com wrote:
From: Brad Volkin bradley.d.vol...@intel.com
This extends use of the command parser to VLV.
Note that the patch checks that the PPGTT bit is set appropriately when
PPGTT is enabled but ignores it when PPGTT is disabled. It would be
awkward to correctly invert the expected value to check that the bit is
set appropriately in that case, and of limited value anyhow.
Signed-off-by: Brad Volkin bradley.d.vol...@intel.com
---
I've confirmed that the shmem pread setup stuff we added does fix the
caching issues I saw previously. I've done some basic testing with this
on both IVB and VLV and don't see regressions. I don't have any data on
the VLV perf impact though.
Also, I considered splitting the patch up a bit differently but decided
that a single patch seemed ok. I'm happy to split it up a bit if that's
what people prefer.
drivers/gpu/drm/i915/i915_cmd_parser.c | 187
+
drivers/gpu/drm/i915/i915_drv.h| 8 +-
2 files changed, 104 insertions(+), 91 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_cmd_parser.c
b/drivers/gpu/drm/i915/i915_cmd_parser.c
index 9d79543..fd35900 100644
--- a/drivers/gpu/drm/i915/i915_cmd_parser.c
+++ b/drivers/gpu/drm/i915/i915_cmd_parser.c
@@ -110,6 +110,7 @@
#define W CMD_DESC_REGISTER
#define B CMD_DESC_BITMASK
#define M CMD_DESC_MASTER
+#define P CMD_DESC_PPGTT
/*Command Mask Fixed Len Action
-- */
@@ -124,20 +125,20 @@ static const struct drm_i915_cmd_descriptor
common_cmds[] = {
CMD( MI_STORE_DWORD_INDEX, SMI, !F, 0xFF, R ),
CMD( MI_LOAD_REGISTER_IMM(1), SMI, !F, 0xFF, W,
.reg = { .offset = 1, .mask = 0x007C } ),
- CMD( MI_STORE_REGISTER_MEM(1), SMI, !F, 0xFF, W | B,
+ CMD( MI_STORE_REGISTER_MEM(1), SMI, !F, 0xFF, W | P,
.reg = { .offset = 1, .mask = 0x007C },
- .bits = {{
+ .ppgtt = {
.offset = 0,
.mask = MI_GLOBAL_GTT,
.expected = 0,
- }}, ),
- CMD( MI_LOAD_REGISTER_MEM, SMI, !F, 0xFF, W | B,
+ }, ),
+ CMD( MI_LOAD_REGISTER_MEM, SMI, !F, 0xFF, W | P,
.reg = { .offset = 1, .mask = 0x007C },
- .bits = {{
+ .ppgtt = {
.offset = 0,
.mask = MI_GLOBAL_GTT,
.expected = 0,
- }}, ),
+ }, ),
CMD( MI_BATCH_BUFFER_START,SMI, !F, 0xFF, S ),
};
@@ -149,31 +150,31 @@ static const struct drm_i915_cmd_descriptor
render_cmds[] = {
CMD( MI_DISPLAY_FLIP, SMI, !F, 0xFF, R ),
CMD( MI_SET_CONTEXT, SMI, !F, 0xFF, R ),
CMD( MI_URB_CLEAR, SMI, !F, 0xFF, S ),
- CMD( MI_STORE_DWORD_IMM, SMI, !F, 0x3F, B,
- .bits = {{
+ CMD( MI_STORE_DWORD_IMM, SMI, !F, 0x3F, P,
+ .ppgtt = {
.offset = 0,
.mask = MI_GLOBAL_GTT,
.expected = 0,
- }}, ),
+ }, ),
CMD( MI_UPDATE_GTT,SMI, !F, 0xFF, R ),
- CMD( MI_CLFLUSH, SMI, !F, 0x3FF, B,
- .bits = {{
+ CMD( MI_CLFLUSH, SMI, !F, 0x3FF, P,
+ .ppgtt = {
.offset = 0,
.mask =
Re: [Intel-gfx] [PATCH] drm/i915: Only check PPGTT bits when using PPGTT
On Wed, May 28, 2014 at 02:45:26PM -0700, Volkin, Bradley D wrote:
On Wed, May 28, 2014 at 02:29:34PM -0700, Daniel Vetter wrote:
Jesse reportedly has a patch somewhere to (finally!) enable ppgtt on
vlv. Would we still need any part of this with ppgtt support on vlv?
-Daniel
Of course, as soon as I accept that it'll never happen... :)
Jesse just submitted his patch, please review it to make sure it happens
;-)
Cheers, Daniel
Off the top of my head, keeping this would let us run the command parser in
the case where ppgtt is force disabled, which might be nice. Otherwise the
parser stops working and you lose the ability to LRI whitelisted registers,
etc, which might not be what you expected.
My motivation was enabling the parser for vlv, which would happen without
any parser changes if it gets ppgtt, so I'll leave it up to you.
Brad
On Wed, May 28, 2014 at 10:47 PM, bradley.d.vol...@intel.com wrote:
From: Brad Volkin bradley.d.vol...@intel.com
This extends use of the command parser to VLV.
Note that the patch checks that the PPGTT bit is set appropriately when
PPGTT is enabled but ignores it when PPGTT is disabled. It would be
awkward to correctly invert the expected value to check that the bit is
set appropriately in that case, and of limited value anyhow.
Signed-off-by: Brad Volkin bradley.d.vol...@intel.com
---
I've confirmed that the shmem pread setup stuff we added does fix the
caching issues I saw previously. I've done some basic testing with this
on both IVB and VLV and don't see regressions. I don't have any data on
the VLV perf impact though.
Also, I considered splitting the patch up a bit differently but decided
that a single patch seemed ok. I'm happy to split it up a bit if that's
what people prefer.
drivers/gpu/drm/i915/i915_cmd_parser.c | 187
+
drivers/gpu/drm/i915/i915_drv.h| 8 +-
2 files changed, 104 insertions(+), 91 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_cmd_parser.c
b/drivers/gpu/drm/i915/i915_cmd_parser.c
index 9d79543..fd35900 100644
--- a/drivers/gpu/drm/i915/i915_cmd_parser.c
+++ b/drivers/gpu/drm/i915/i915_cmd_parser.c
@@ -110,6 +110,7 @@
#define W CMD_DESC_REGISTER
#define B CMD_DESC_BITMASK
#define M CMD_DESC_MASTER
+#define P CMD_DESC_PPGTT
/*Command Mask Fixed Len Action
--
*/
@@ -124,20 +125,20 @@ static const struct drm_i915_cmd_descriptor
common_cmds[] = {
CMD( MI_STORE_DWORD_INDEX, SMI, !F, 0xFF, R ),
CMD( MI_LOAD_REGISTER_IMM(1), SMI, !F, 0xFF, W,
.reg = { .offset = 1, .mask = 0x007C } ),
- CMD( MI_STORE_REGISTER_MEM(1), SMI, !F, 0xFF, W | B,
+ CMD( MI_STORE_REGISTER_MEM(1), SMI, !F, 0xFF, W | P,
.reg = { .offset = 1, .mask = 0x007C },
- .bits = {{
+ .ppgtt = {
.offset = 0,
.mask = MI_GLOBAL_GTT,
.expected = 0,
- }}, ),
- CMD( MI_LOAD_REGISTER_MEM, SMI, !F, 0xFF, W | B,
+ }, ),
+ CMD( MI_LOAD_REGISTER_MEM, SMI, !F, 0xFF, W | P,
.reg = { .offset = 1, .mask = 0x007C },
- .bits = {{
+ .ppgtt = {
.offset = 0,
.mask = MI_GLOBAL_GTT,
.expected = 0,
- }}, ),
+ }, ),
CMD( MI_BATCH_BUFFER_START,SMI, !F, 0xFF, S ),
};
@@ -149,31 +150,31 @@ static const struct drm_i915_cmd_descriptor
render_cmds[] = {
CMD( MI_DISPLAY_FLIP, SMI, !F, 0xFF, R ),
CMD( MI_SET_CONTEXT, SMI, !F, 0xFF, R ),
CMD( MI_URB_CLEAR, SMI, !F, 0xFF, S ),
- CMD( MI_STORE_DWORD_IMM, SMI, !F, 0x3F, B,
- .bits = {{
+ CMD( MI_STORE_DWORD_IMM, SMI, !F, 0x3F, P,
+ .ppgtt = {
.offset = 0,
.mask = MI_GLOBAL_GTT,
.expected = 0,
- }}, ),
+ }, ),
CMD( MI_UPDATE_GTT,SMI, !F, 0xFF, R ),
-
