Re: [Mesa-dev] [PATCH v03 14/38] i965: Move MOCS macros to brw_context.h.
On Wed, May 03, 2017 at 11:36:29PM -0700, Kenneth Graunke wrote: > On Wednesday, May 3, 2017 7:52:01 PM PDT Pohjolainen, Topi wrote: > > On Wed, May 03, 2017 at 05:11:45PM -0700, Rafael Antognolli wrote: > > > On Wed, May 03, 2017 at 08:28:24PM +0300, Pohjolainen, Topi wrote: > > > > On Mon, May 01, 2017 at 06:43:02PM -0700, Rafael Antognolli wrote: > > > > > These macros are defined in brw_defines.h, which contains a lot of > > > > > macros that conflict with autogenerated code from genxml. But we need > > > > > to > > > > > use them (the MOCS macros) in some of that same genxml code. > > > > > > > > > > Moving them to brw_context.h solves that problem and we don't have to > > > > > include brw_defines.h. > > > > > > > > I've been hoping to remove things from brw_context.h - it starts to > > > > resemble a dump yard for all sort of things. I think in this case we > > > > could put these into brw_state.h instead? Or did you already try that? > > > > > > I just tried this and it works fine too. I'm OK with either place to put > > > these macros. > > > > That would get my: > > > > Reviewed-by: Topi Pohjolainen > > I do like that better. Since I already landed this patch...Rafael, > would you mind sending a patch to move them from brw_context.h to > brw_state.h, and applying Topi's R-b? I can push it. Just sent it. I added them to the end of the header, as I couldn't figure out a place that would make more sense. But feel free to move them around if you prefer. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v03 14/38] i965: Move MOCS macros to brw_context.h.
On Wednesday, May 3, 2017 7:52:01 PM PDT Pohjolainen, Topi wrote: > On Wed, May 03, 2017 at 05:11:45PM -0700, Rafael Antognolli wrote: > > On Wed, May 03, 2017 at 08:28:24PM +0300, Pohjolainen, Topi wrote: > > > On Mon, May 01, 2017 at 06:43:02PM -0700, Rafael Antognolli wrote: > > > > These macros are defined in brw_defines.h, which contains a lot of > > > > macros that conflict with autogenerated code from genxml. But we need to > > > > use them (the MOCS macros) in some of that same genxml code. > > > > > > > > Moving them to brw_context.h solves that problem and we don't have to > > > > include brw_defines.h. > > > > > > I've been hoping to remove things from brw_context.h - it starts to > > > resemble a dump yard for all sort of things. I think in this case we > > > could put these into brw_state.h instead? Or did you already try that? > > > > I just tried this and it works fine too. I'm OK with either place to put > > these macros. > > That would get my: > > Reviewed-by: Topi Pohjolainen I do like that better. Since I already landed this patch...Rafael, would you mind sending a patch to move them from brw_context.h to brw_state.h, and applying Topi's R-b? I can push it. signature.asc Description: This is a digitally signed message part. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v03 14/38] i965: Move MOCS macros to brw_context.h.
On Wed, May 03, 2017 at 05:11:45PM -0700, Rafael Antognolli wrote: > On Wed, May 03, 2017 at 08:28:24PM +0300, Pohjolainen, Topi wrote: > > On Mon, May 01, 2017 at 06:43:02PM -0700, Rafael Antognolli wrote: > > > These macros are defined in brw_defines.h, which contains a lot of > > > macros that conflict with autogenerated code from genxml. But we need to > > > use them (the MOCS macros) in some of that same genxml code. > > > > > > Moving them to brw_context.h solves that problem and we don't have to > > > include brw_defines.h. > > > > I've been hoping to remove things from brw_context.h - it starts to > > resemble a dump yard for all sort of things. I think in this case we > > could put these into brw_state.h instead? Or did you already try that? > > I just tried this and it works fine too. I'm OK with either place to put > these macros. That would get my: Reviewed-by: Topi Pohjolainen > > > > > > > Signed-off-by: Rafael Antognolli > > > --- > > > src/mesa/drivers/dri/i965/brw_context.h | 41 +- > > > src/mesa/drivers/dri/i965/brw_defines.h | 42 +-- > > > 2 files changed, 41 insertions(+), 42 deletions(-) > > > > > > diff --git a/src/mesa/drivers/dri/i965/brw_context.h > > > b/src/mesa/drivers/dri/i965/brw_context.h > > > index c7d6e49..5e627ae 100644 > > > --- a/src/mesa/drivers/dri/i965/brw_context.h > > > +++ b/src/mesa/drivers/dri/i965/brw_context.h > > > @@ -397,6 +397,47 @@ struct brw_cache { > > > bool bo_used_by_gpu; > > > }; > > > > > > +/* Memory Object Control State: > > > + * Specifying zero for L3 means "uncached in L3", at least on Haswell > > > + * and Baytrail, since there are no PTE flags for setting L3 > > > cacheability. > > > + * On Ivybridge, the PTEs do have a cache-in-L3 bit, so setting MOCS to 0 > > > + * may still respect that. > > > + */ > > > +#define GEN7_MOCS_L31 > > > + > > > +/* Ivybridge only: cache in LLC. > > > + * Specifying zero here means to use the PTE values set by the kernel; > > > + * non-zero overrides the PTE values. > > > + */ > > > +#define IVB_MOCS_LLC(1 << 1) > > > + > > > +/* Baytrail only: snoop in CPU cache */ > > > +#define BYT_MOCS_SNOOP (1 << 1) > > > + > > > +/* Haswell only: LLC/eLLC controls (write-back or uncached). > > > + * Specifying zero here means to use the PTE values set by the kernel, > > > + * which is useful since it offers additional control (write-through > > > + * cacheing and age). Non-zero overrides the PTE values. > > > + */ > > > +#define HSW_MOCS_UC_LLC_UC_ELLC (1 << 1) > > > +#define HSW_MOCS_WB_LLC_WB_ELLC (2 << 1) > > > +#define HSW_MOCS_UC_LLC_WB_ELLC (3 << 1) > > > + > > > +/* Broadwell: these defines always use all available caches (L3, LLC, > > > eLLC), > > > + * and let you force write-back (WB) or write-through (WT) caching, or > > > leave > > > + * it up to the page table entry (PTE) specified by the kernel. > > > + */ > > > +#define BDW_MOCS_WB 0x78 > > > +#define BDW_MOCS_WT 0x58 > > > +#define BDW_MOCS_PTE 0x18 > > > + > > > +/* Skylake: MOCS is now an index into an array of 62 different caching > > > + * configurations programmed by the kernel. > > > + */ > > > +/* TC=LLC/eLLC, LeCC=WB, LRUM=3, L3CC=WB */ > > > +#define SKL_MOCS_WB (2 << 1) > > > +/* TC=LLC/eLLC, LeCC=PTE, LRUM=3, L3CC=WB */ > > > +#define SKL_MOCS_PTE (1 << 1) > > > > > > /* Considered adding a member to this struct to document which flags > > > * an update might raise so that ordering of the state atoms can be > > > diff --git a/src/mesa/drivers/dri/i965/brw_defines.h > > > b/src/mesa/drivers/dri/i965/brw_defines.h > > > index 08106c0..130a1ed 100644 > > > --- a/src/mesa/drivers/dri/i965/brw_defines.h > > > +++ b/src/mesa/drivers/dri/i965/brw_defines.h > > > @@ -1365,48 +1365,6 @@ enum brw_pixel_shader_coverage_mask_mode { > > > */ > > > #define BRW_MAX_NUM_BUFFER_ENTRIES (1 << 27) > > > > > > -/* Memory Object Control State: > > > - * Specifying zero for L3 means "uncached in L3", at least on Haswell > > > - * and Baytrail, since there are no PTE flags for setting L3 > > > cacheability. > > > - * On Ivybridge, the PTEs do have a cache-in-L3 bit, so setting MOCS to 0 > > > - * may still respect that. > > > - */ > > > -#define GEN7_MOCS_L31 > > > - > > > -/* Ivybridge only: cache in LLC. > > > - * Specifying zero here means to use the PTE values set by the kernel; > > > - * non-zero overrides the PTE values. > > > - */ > > > -#define IVB_MOCS_LLC(1 << 1) > > > - > > > -/* Baytrail only: snoop in CPU cache */ > > > -#define BYT_MOCS_SNOOP (1 << 1) > > > - > > > -/* Haswell only: LLC/eLLC controls (write-back or uncached). > > > - * Specifying zero here means to use the PTE values set by the kernel, > > > - * which is useful since it offers additional control (write-through > > > - * cacheing and age). Non-zero overrides
Re: [Mesa-dev] [PATCH v03 14/38] i965: Move MOCS macros to brw_context.h.
On Wed, May 03, 2017 at 08:28:24PM +0300, Pohjolainen, Topi wrote: > On Mon, May 01, 2017 at 06:43:02PM -0700, Rafael Antognolli wrote: > > These macros are defined in brw_defines.h, which contains a lot of > > macros that conflict with autogenerated code from genxml. But we need to > > use them (the MOCS macros) in some of that same genxml code. > > > > Moving them to brw_context.h solves that problem and we don't have to > > include brw_defines.h. > > I've been hoping to remove things from brw_context.h - it starts to > resemble a dump yard for all sort of things. I think in this case we > could put these into brw_state.h instead? Or did you already try that? I just tried this and it works fine too. I'm OK with either place to put these macros. > > > > Signed-off-by: Rafael Antognolli > > --- > > src/mesa/drivers/dri/i965/brw_context.h | 41 +- > > src/mesa/drivers/dri/i965/brw_defines.h | 42 +-- > > 2 files changed, 41 insertions(+), 42 deletions(-) > > > > diff --git a/src/mesa/drivers/dri/i965/brw_context.h > > b/src/mesa/drivers/dri/i965/brw_context.h > > index c7d6e49..5e627ae 100644 > > --- a/src/mesa/drivers/dri/i965/brw_context.h > > +++ b/src/mesa/drivers/dri/i965/brw_context.h > > @@ -397,6 +397,47 @@ struct brw_cache { > > bool bo_used_by_gpu; > > }; > > > > +/* Memory Object Control State: > > + * Specifying zero for L3 means "uncached in L3", at least on Haswell > > + * and Baytrail, since there are no PTE flags for setting L3 cacheability. > > + * On Ivybridge, the PTEs do have a cache-in-L3 bit, so setting MOCS to 0 > > + * may still respect that. > > + */ > > +#define GEN7_MOCS_L31 > > + > > +/* Ivybridge only: cache in LLC. > > + * Specifying zero here means to use the PTE values set by the kernel; > > + * non-zero overrides the PTE values. > > + */ > > +#define IVB_MOCS_LLC(1 << 1) > > + > > +/* Baytrail only: snoop in CPU cache */ > > +#define BYT_MOCS_SNOOP (1 << 1) > > + > > +/* Haswell only: LLC/eLLC controls (write-back or uncached). > > + * Specifying zero here means to use the PTE values set by the kernel, > > + * which is useful since it offers additional control (write-through > > + * cacheing and age). Non-zero overrides the PTE values. > > + */ > > +#define HSW_MOCS_UC_LLC_UC_ELLC (1 << 1) > > +#define HSW_MOCS_WB_LLC_WB_ELLC (2 << 1) > > +#define HSW_MOCS_UC_LLC_WB_ELLC (3 << 1) > > + > > +/* Broadwell: these defines always use all available caches (L3, LLC, > > eLLC), > > + * and let you force write-back (WB) or write-through (WT) caching, or > > leave > > + * it up to the page table entry (PTE) specified by the kernel. > > + */ > > +#define BDW_MOCS_WB 0x78 > > +#define BDW_MOCS_WT 0x58 > > +#define BDW_MOCS_PTE 0x18 > > + > > +/* Skylake: MOCS is now an index into an array of 62 different caching > > + * configurations programmed by the kernel. > > + */ > > +/* TC=LLC/eLLC, LeCC=WB, LRUM=3, L3CC=WB */ > > +#define SKL_MOCS_WB (2 << 1) > > +/* TC=LLC/eLLC, LeCC=PTE, LRUM=3, L3CC=WB */ > > +#define SKL_MOCS_PTE (1 << 1) > > > > /* Considered adding a member to this struct to document which flags > > * an update might raise so that ordering of the state atoms can be > > diff --git a/src/mesa/drivers/dri/i965/brw_defines.h > > b/src/mesa/drivers/dri/i965/brw_defines.h > > index 08106c0..130a1ed 100644 > > --- a/src/mesa/drivers/dri/i965/brw_defines.h > > +++ b/src/mesa/drivers/dri/i965/brw_defines.h > > @@ -1365,48 +1365,6 @@ enum brw_pixel_shader_coverage_mask_mode { > > */ > > #define BRW_MAX_NUM_BUFFER_ENTRIES (1 << 27) > > > > -/* Memory Object Control State: > > - * Specifying zero for L3 means "uncached in L3", at least on Haswell > > - * and Baytrail, since there are no PTE flags for setting L3 cacheability. > > - * On Ivybridge, the PTEs do have a cache-in-L3 bit, so setting MOCS to 0 > > - * may still respect that. > > - */ > > -#define GEN7_MOCS_L31 > > - > > -/* Ivybridge only: cache in LLC. > > - * Specifying zero here means to use the PTE values set by the kernel; > > - * non-zero overrides the PTE values. > > - */ > > -#define IVB_MOCS_LLC(1 << 1) > > - > > -/* Baytrail only: snoop in CPU cache */ > > -#define BYT_MOCS_SNOOP (1 << 1) > > - > > -/* Haswell only: LLC/eLLC controls (write-back or uncached). > > - * Specifying zero here means to use the PTE values set by the kernel, > > - * which is useful since it offers additional control (write-through > > - * cacheing and age). Non-zero overrides the PTE values. > > - */ > > -#define HSW_MOCS_UC_LLC_UC_ELLC (1 << 1) > > -#define HSW_MOCS_WB_LLC_WB_ELLC (2 << 1) > > -#define HSW_MOCS_UC_LLC_WB_ELLC (3 << 1) > > - > > -/* Broadwell: these defines always use all available caches (L3, LLC, > > eLLC), > > - * and let you force write-back (WB) or write-through (WT) caching, or
Re: [Mesa-dev] [PATCH v03 14/38] i965: Move MOCS macros to brw_context.h.
On Mon, May 01, 2017 at 06:43:02PM -0700, Rafael Antognolli wrote: > These macros are defined in brw_defines.h, which contains a lot of > macros that conflict with autogenerated code from genxml. But we need to > use them (the MOCS macros) in some of that same genxml code. > > Moving them to brw_context.h solves that problem and we don't have to > include brw_defines.h. I've been hoping to remove things from brw_context.h - it starts to resemble a dump yard for all sort of things. I think in this case we could put these into brw_state.h instead? Or did you already try that? > > Signed-off-by: Rafael Antognolli > --- > src/mesa/drivers/dri/i965/brw_context.h | 41 +- > src/mesa/drivers/dri/i965/brw_defines.h | 42 +-- > 2 files changed, 41 insertions(+), 42 deletions(-) > > diff --git a/src/mesa/drivers/dri/i965/brw_context.h > b/src/mesa/drivers/dri/i965/brw_context.h > index c7d6e49..5e627ae 100644 > --- a/src/mesa/drivers/dri/i965/brw_context.h > +++ b/src/mesa/drivers/dri/i965/brw_context.h > @@ -397,6 +397,47 @@ struct brw_cache { > bool bo_used_by_gpu; > }; > > +/* Memory Object Control State: > + * Specifying zero for L3 means "uncached in L3", at least on Haswell > + * and Baytrail, since there are no PTE flags for setting L3 cacheability. > + * On Ivybridge, the PTEs do have a cache-in-L3 bit, so setting MOCS to 0 > + * may still respect that. > + */ > +#define GEN7_MOCS_L31 > + > +/* Ivybridge only: cache in LLC. > + * Specifying zero here means to use the PTE values set by the kernel; > + * non-zero overrides the PTE values. > + */ > +#define IVB_MOCS_LLC(1 << 1) > + > +/* Baytrail only: snoop in CPU cache */ > +#define BYT_MOCS_SNOOP (1 << 1) > + > +/* Haswell only: LLC/eLLC controls (write-back or uncached). > + * Specifying zero here means to use the PTE values set by the kernel, > + * which is useful since it offers additional control (write-through > + * cacheing and age). Non-zero overrides the PTE values. > + */ > +#define HSW_MOCS_UC_LLC_UC_ELLC (1 << 1) > +#define HSW_MOCS_WB_LLC_WB_ELLC (2 << 1) > +#define HSW_MOCS_UC_LLC_WB_ELLC (3 << 1) > + > +/* Broadwell: these defines always use all available caches (L3, LLC, eLLC), > + * and let you force write-back (WB) or write-through (WT) caching, or leave > + * it up to the page table entry (PTE) specified by the kernel. > + */ > +#define BDW_MOCS_WB 0x78 > +#define BDW_MOCS_WT 0x58 > +#define BDW_MOCS_PTE 0x18 > + > +/* Skylake: MOCS is now an index into an array of 62 different caching > + * configurations programmed by the kernel. > + */ > +/* TC=LLC/eLLC, LeCC=WB, LRUM=3, L3CC=WB */ > +#define SKL_MOCS_WB (2 << 1) > +/* TC=LLC/eLLC, LeCC=PTE, LRUM=3, L3CC=WB */ > +#define SKL_MOCS_PTE (1 << 1) > > /* Considered adding a member to this struct to document which flags > * an update might raise so that ordering of the state atoms can be > diff --git a/src/mesa/drivers/dri/i965/brw_defines.h > b/src/mesa/drivers/dri/i965/brw_defines.h > index 08106c0..130a1ed 100644 > --- a/src/mesa/drivers/dri/i965/brw_defines.h > +++ b/src/mesa/drivers/dri/i965/brw_defines.h > @@ -1365,48 +1365,6 @@ enum brw_pixel_shader_coverage_mask_mode { > */ > #define BRW_MAX_NUM_BUFFER_ENTRIES (1 << 27) > > -/* Memory Object Control State: > - * Specifying zero for L3 means "uncached in L3", at least on Haswell > - * and Baytrail, since there are no PTE flags for setting L3 cacheability. > - * On Ivybridge, the PTEs do have a cache-in-L3 bit, so setting MOCS to 0 > - * may still respect that. > - */ > -#define GEN7_MOCS_L31 > - > -/* Ivybridge only: cache in LLC. > - * Specifying zero here means to use the PTE values set by the kernel; > - * non-zero overrides the PTE values. > - */ > -#define IVB_MOCS_LLC(1 << 1) > - > -/* Baytrail only: snoop in CPU cache */ > -#define BYT_MOCS_SNOOP (1 << 1) > - > -/* Haswell only: LLC/eLLC controls (write-back or uncached). > - * Specifying zero here means to use the PTE values set by the kernel, > - * which is useful since it offers additional control (write-through > - * cacheing and age). Non-zero overrides the PTE values. > - */ > -#define HSW_MOCS_UC_LLC_UC_ELLC (1 << 1) > -#define HSW_MOCS_WB_LLC_WB_ELLC (2 << 1) > -#define HSW_MOCS_UC_LLC_WB_ELLC (3 << 1) > - > -/* Broadwell: these defines always use all available caches (L3, LLC, eLLC), > - * and let you force write-back (WB) or write-through (WT) caching, or leave > - * it up to the page table entry (PTE) specified by the kernel. > - */ > -#define BDW_MOCS_WB 0x78 > -#define BDW_MOCS_WT 0x58 > -#define BDW_MOCS_PTE 0x18 > - > -/* Skylake: MOCS is now an index into an array of 62 different caching > - * configurations programmed by the kernel. > - */ > -/* TC=LLC/eLLC, LeCC=WB, LRUM=3, L3CC=WB */ > -#define SKL_MOCS_WB (2 << 1) > -/