| Message ID | 20190111175336.2593-1-lionel.g.landwerlin@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915/cmdparser: whitelist needed predicate registers for Anv | expand |
Hi Lionel,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on drm-intel/for-linux-next]
[also build test ERROR on v5.0-rc1 next-20190111]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Lionel-Landwerlin/drm-i915-cmdparser-whitelist-needed-predicate-registers-for-Anv/20190113-231821
base: git://anongit.freedesktop.org/drm-intel for-linux-next
config: i386-randconfig-x072-201902 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
All errors (new ones prefixed by >>):
>> drivers/gpu/drm/i915/i915_cmd_parser.c:552:8: error: 'MI_PREDICATE_DATA_UDW' undeclared here (not in a function); did you mean 'MI_PREDICATE_SRC1_UDW'?
REG64(MI_PREDICATE_DATA),
^
drivers/gpu/drm/i915/i915_cmd_parser.c:530:12: note: in definition of macro 'REG64'
{ .addr = _reg ## _UDW }
^~~~
vim +552 drivers/gpu/drm/i915/i915_cmd_parser.c
516
517 /* Convenience macro for adding 32-bit registers. */
518 #define REG32(_reg, ...) \
519 { .addr = (_reg), __VA_ARGS__ }
520
521 /*
522 * Convenience macro for adding 64-bit registers.
523 *
524 * Some registers that userspace accesses are 64 bits. The register
525 * access commands only allow 32-bit accesses. Hence, we have to include
526 * entries for both halves of the 64-bit registers.
527 */
528 #define REG64(_reg) \
529 { .addr = _reg }, \
530 { .addr = _reg ## _UDW }
531
532 #define REG64_IDX(_reg, idx) \
533 { .addr = _reg(idx) }, \
534 { .addr = _reg ## _UDW(idx) }
535
536 static const struct drm_i915_reg_descriptor gen7_render_regs[] = {
537 REG64(GPGPU_THREADS_DISPATCHED),
538 REG64(HS_INVOCATION_COUNT),
539 REG64(DS_INVOCATION_COUNT),
540 REG64(IA_VERTICES_COUNT),
541 REG64(IA_PRIMITIVES_COUNT),
542 REG64(VS_INVOCATION_COUNT),
543 REG64(GS_INVOCATION_COUNT),
544 REG64(GS_PRIMITIVES_COUNT),
545 REG64(CL_INVOCATION_COUNT),
546 REG64(CL_PRIMITIVES_COUNT),
547 REG64(PS_INVOCATION_COUNT),
548 REG64(PS_DEPTH_COUNT),
549 REG64_IDX(RING_TIMESTAMP, RENDER_RING_BASE),
550 REG64(MI_PREDICATE_SRC0),
551 REG64(MI_PREDICATE_SRC1),
> 552 REG64(MI_PREDICATE_DATA),
553 REG32(MI_PREDICATE_RESULT),
554 REG32(GEN7_3DPRIM_END_OFFSET),
555 REG32(GEN7_3DPRIM_START_VERTEX),
556 REG32(GEN7_3DPRIM_VERTEX_COUNT),
557 REG32(GEN7_3DPRIM_INSTANCE_COUNT),
558 REG32(GEN7_3DPRIM_START_INSTANCE),
559 REG32(GEN7_3DPRIM_BASE_VERTEX),
560 REG32(GEN7_GPGPU_DISPATCHDIMX),
561 REG32(GEN7_GPGPU_DISPATCHDIMY),
562 REG32(GEN7_GPGPU_DISPATCHDIMZ),
563 REG64_IDX(RING_TIMESTAMP, BSD_RING_BASE),
564 REG64_IDX(GEN7_SO_NUM_PRIMS_WRITTEN, 0),
565 REG64_IDX(GEN7_SO_NUM_PRIMS_WRITTEN, 1),
566 REG64_IDX(GEN7_SO_NUM_PRIMS_WRITTEN, 2),
567 REG64_IDX(GEN7_SO_NUM_PRIMS_WRITTEN, 3),
568 REG64_IDX(GEN7_SO_PRIM_STORAGE_NEEDED, 0),
569 REG64_IDX(GEN7_SO_PRIM_STORAGE_NEEDED, 1),
570 REG64_IDX(GEN7_SO_PRIM_STORAGE_NEEDED, 2),
571 REG64_IDX(GEN7_SO_PRIM_STORAGE_NEEDED, 3),
572 REG32(GEN7_SO_WRITE_OFFSET(0)),
573 REG32(GEN7_SO_WRITE_OFFSET(1)),
574 REG32(GEN7_SO_WRITE_OFFSET(2)),
575 REG32(GEN7_SO_WRITE_OFFSET(3)),
576 REG32(GEN7_L3SQCREG1),
577 REG32(GEN7_L3CNTLREG2),
578 REG32(GEN7_L3CNTLREG3),
579 REG64_IDX(RING_TIMESTAMP, BLT_RING_BASE),
580 };
581
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
Hi Lionel,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on drm-intel/for-linux-next]
[also build test ERROR on v5.0-rc1 next-20190111]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Lionel-Landwerlin/drm-i915-cmdparser-whitelist-needed-predicate-registers-for-Anv/20190113-231821
base: git://anongit.freedesktop.org/drm-intel for-linux-next
config: i386-randconfig-s1-201902 (attached as .config)
compiler: gcc-6 (Debian 6.4.0-9) 6.4.0 20171026
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
All errors (new ones prefixed by >>):
>> drivers/gpu//drm/i915/i915_cmd_parser.c:552:8: error: 'MI_PREDICATE_DATA_UDW' undeclared here (not in a function)
REG64(MI_PREDICATE_DATA),
^
drivers/gpu//drm/i915/i915_cmd_parser.c:530:12: note: in definition of macro 'REG64'
{ .addr = _reg ## _UDW }
^~~~
vim +/MI_PREDICATE_DATA_UDW +552 drivers/gpu//drm/i915/i915_cmd_parser.c
516
517 /* Convenience macro for adding 32-bit registers. */
518 #define REG32(_reg, ...) \
519 { .addr = (_reg), __VA_ARGS__ }
520
521 /*
522 * Convenience macro for adding 64-bit registers.
523 *
524 * Some registers that userspace accesses are 64 bits. The register
525 * access commands only allow 32-bit accesses. Hence, we have to include
526 * entries for both halves of the 64-bit registers.
527 */
528 #define REG64(_reg) \
529 { .addr = _reg }, \
530 { .addr = _reg ## _UDW }
531
532 #define REG64_IDX(_reg, idx) \
533 { .addr = _reg(idx) }, \
534 { .addr = _reg ## _UDW(idx) }
535
536 static const struct drm_i915_reg_descriptor gen7_render_regs[] = {
537 REG64(GPGPU_THREADS_DISPATCHED),
538 REG64(HS_INVOCATION_COUNT),
539 REG64(DS_INVOCATION_COUNT),
540 REG64(IA_VERTICES_COUNT),
541 REG64(IA_PRIMITIVES_COUNT),
542 REG64(VS_INVOCATION_COUNT),
543 REG64(GS_INVOCATION_COUNT),
544 REG64(GS_PRIMITIVES_COUNT),
545 REG64(CL_INVOCATION_COUNT),
546 REG64(CL_PRIMITIVES_COUNT),
547 REG64(PS_INVOCATION_COUNT),
548 REG64(PS_DEPTH_COUNT),
549 REG64_IDX(RING_TIMESTAMP, RENDER_RING_BASE),
550 REG64(MI_PREDICATE_SRC0),
551 REG64(MI_PREDICATE_SRC1),
> 552 REG64(MI_PREDICATE_DATA),
553 REG32(MI_PREDICATE_RESULT),
554 REG32(GEN7_3DPRIM_END_OFFSET),
555 REG32(GEN7_3DPRIM_START_VERTEX),
556 REG32(GEN7_3DPRIM_VERTEX_COUNT),
557 REG32(GEN7_3DPRIM_INSTANCE_COUNT),
558 REG32(GEN7_3DPRIM_START_INSTANCE),
559 REG32(GEN7_3DPRIM_BASE_VERTEX),
560 REG32(GEN7_GPGPU_DISPATCHDIMX),
561 REG32(GEN7_GPGPU_DISPATCHDIMY),
562 REG32(GEN7_GPGPU_DISPATCHDIMZ),
563 REG64_IDX(RING_TIMESTAMP, BSD_RING_BASE),
564 REG64_IDX(GEN7_SO_NUM_PRIMS_WRITTEN, 0),
565 REG64_IDX(GEN7_SO_NUM_PRIMS_WRITTEN, 1),
566 REG64_IDX(GEN7_SO_NUM_PRIMS_WRITTEN, 2),
567 REG64_IDX(GEN7_SO_NUM_PRIMS_WRITTEN, 3),
568 REG64_IDX(GEN7_SO_PRIM_STORAGE_NEEDED, 0),
569 REG64_IDX(GEN7_SO_PRIM_STORAGE_NEEDED, 1),
570 REG64_IDX(GEN7_SO_PRIM_STORAGE_NEEDED, 2),
571 REG64_IDX(GEN7_SO_PRIM_STORAGE_NEEDED, 3),
572 REG32(GEN7_SO_WRITE_OFFSET(0)),
573 REG32(GEN7_SO_WRITE_OFFSET(1)),
574 REG32(GEN7_SO_WRITE_OFFSET(2)),
575 REG32(GEN7_SO_WRITE_OFFSET(3)),
576 REG32(GEN7_L3SQCREG1),
577 REG32(GEN7_L3CNTLREG2),
578 REG32(GEN7_L3CNTLREG3),
579 REG64_IDX(RING_TIMESTAMP, BLT_RING_BASE),
580 };
581
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
Quoting Lionel Landwerlin (2019-01-11 19:53:36) > There is no reason not to whitelist those registers. In particular > MI_PREDICATE_RESULT can be loaded outside of MI_PREDICATE through > other registers to predicate other commands. Link to userspace changes? Regards, Joonas > > Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com> > --- > 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 95478db9998b..c5bf14c3f540 100644 > --- a/drivers/gpu/drm/i915/i915_cmd_parser.c > +++ b/drivers/gpu/drm/i915/i915_cmd_parser.c > @@ -549,6 +549,8 @@ static const struct drm_i915_reg_descriptor gen7_render_regs[] = { > REG64_IDX(RING_TIMESTAMP, RENDER_RING_BASE), > REG64(MI_PREDICATE_SRC0), > REG64(MI_PREDICATE_SRC1), > + REG64(MI_PREDICATE_DATA), > + REG32(MI_PREDICATE_RESULT), > REG32(GEN7_3DPRIM_END_OFFSET), > REG32(GEN7_3DPRIM_START_VERTEX), > REG32(GEN7_3DPRIM_VERTEX_COUNT), > @@ -1382,6 +1384,8 @@ int i915_cmd_parser_get_version(struct drm_i915_private *dev_priv) > * the parser enabled. > * 9. Don't whitelist or handle oacontrol specially, as ownership > * for oacontrol state is moving to i915-perf. > + * 10. Whitelist predicate data/result registers for conditional > + * rendering in Anv. > */ > - return 9; > + return 10; > } > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index ca8026ec0655..5c05d73eec8f 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -461,6 +461,8 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg) > #define MI_PREDICATE_SRC0_UDW _MMIO(0x2400 + 4) > #define MI_PREDICATE_SRC1 _MMIO(0x2408) > #define MI_PREDICATE_SRC1_UDW _MMIO(0x2408 + 4) > +#define MI_PREDICATE_DATA _MMIO(0x2410) > +#define MI_PREDICATE_RESULT _MMIO(0x2418) > > #define MI_PREDICATE_RESULT_2 _MMIO(0x2214) > #define LOWER_SLICE_ENABLED (1 << 0) > -- > 2.20.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On 01/02/2019 12:30, Joonas Lahtinen wrote: > Quoting Lionel Landwerlin (2019-01-11 19:53:36) >> There is no reason not to whitelist those registers. In particular >> MI_PREDICATE_RESULT can be loaded outside of MI_PREDICATE through >> other registers to predicate other commands. > Link to userspace changes? > > Regards, Joonas So we ended up using MI_PREDICATE instead of loading the predicate registers using MI_LRI/LRM. Although a bit sad, it's just easier to have one way to deal with those registers rather than add more code to check the command parser version. -Lionel > >> Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com> >> --- >> 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 95478db9998b..c5bf14c3f540 100644 >> --- a/drivers/gpu/drm/i915/i915_cmd_parser.c >> +++ b/drivers/gpu/drm/i915/i915_cmd_parser.c >> @@ -549,6 +549,8 @@ static const struct drm_i915_reg_descriptor gen7_render_regs[] = { >> REG64_IDX(RING_TIMESTAMP, RENDER_RING_BASE), >> REG64(MI_PREDICATE_SRC0), >> REG64(MI_PREDICATE_SRC1), >> + REG64(MI_PREDICATE_DATA), >> + REG32(MI_PREDICATE_RESULT), >> REG32(GEN7_3DPRIM_END_OFFSET), >> REG32(GEN7_3DPRIM_START_VERTEX), >> REG32(GEN7_3DPRIM_VERTEX_COUNT), >> @@ -1382,6 +1384,8 @@ int i915_cmd_parser_get_version(struct drm_i915_private *dev_priv) >> * the parser enabled. >> * 9. Don't whitelist or handle oacontrol specially, as ownership >> * for oacontrol state is moving to i915-perf. >> + * 10. Whitelist predicate data/result registers for conditional >> + * rendering in Anv. >> */ >> - return 9; >> + return 10; >> } >> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h >> index ca8026ec0655..5c05d73eec8f 100644 >> --- a/drivers/gpu/drm/i915/i915_reg.h >> +++ b/drivers/gpu/drm/i915/i915_reg.h >> @@ -461,6 +461,8 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg) >> #define MI_PREDICATE_SRC0_UDW _MMIO(0x2400 + 4) >> #define MI_PREDICATE_SRC1 _MMIO(0x2408) >> #define MI_PREDICATE_SRC1_UDW _MMIO(0x2408 + 4) >> +#define MI_PREDICATE_DATA _MMIO(0x2410) >> +#define MI_PREDICATE_RESULT _MMIO(0x2418) >> >> #define MI_PREDICATE_RESULT_2 _MMIO(0x2214) >> #define LOWER_SLICE_ENABLED (1 << 0) >> -- >> 2.20.1 >> >> _______________________________________________ >> Intel-gfx mailing list >> Intel-gfx@lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff --git a/drivers/gpu/drm/i915/i915_cmd_parser.c b/drivers/gpu/drm/i915/i915_cmd_parser.c index 95478db9998b..c5bf14c3f540 100644 --- a/drivers/gpu/drm/i915/i915_cmd_parser.c +++ b/drivers/gpu/drm/i915/i915_cmd_parser.c @@ -549,6 +549,8 @@ static const struct drm_i915_reg_descriptor gen7_render_regs[] = { REG64_IDX(RING_TIMESTAMP, RENDER_RING_BASE), REG64(MI_PREDICATE_SRC0), REG64(MI_PREDICATE_SRC1), + REG64(MI_PREDICATE_DATA), + REG32(MI_PREDICATE_RESULT), REG32(GEN7_3DPRIM_END_OFFSET), REG32(GEN7_3DPRIM_START_VERTEX), REG32(GEN7_3DPRIM_VERTEX_COUNT), @@ -1382,6 +1384,8 @@ int i915_cmd_parser_get_version(struct drm_i915_private *dev_priv) * the parser enabled. * 9. Don't whitelist or handle oacontrol specially, as ownership * for oacontrol state is moving to i915-perf. + * 10. Whitelist predicate data/result registers for conditional + * rendering in Anv. */ - return 9; + return 10; } diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index ca8026ec0655..5c05d73eec8f 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -461,6 +461,8 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg) #define MI_PREDICATE_SRC0_UDW _MMIO(0x2400 + 4) #define MI_PREDICATE_SRC1 _MMIO(0x2408) #define MI_PREDICATE_SRC1_UDW _MMIO(0x2408 + 4) +#define MI_PREDICATE_DATA _MMIO(0x2410) +#define MI_PREDICATE_RESULT _MMIO(0x2418) #define MI_PREDICATE_RESULT_2 _MMIO(0x2214) #define LOWER_SLICE_ENABLED (1 << 0)
There is no reason not to whitelist those registers. In particular MI_PREDICATE_RESULT can be loaded outside of MI_PREDICATE through other registers to predicate other commands. Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com> --- drivers/gpu/drm/i915/i915_cmd_parser.c | 6 +++++- drivers/gpu/drm/i915/i915_reg.h | 2 ++ 2 files changed, 7 insertions(+), 1 deletion(-)