Re: [Intel-gfx] [PATCH v8 4/7] drm/i915: use a macro to define MOCS entries
On Tue, Jan 22, 2019 at 09:37:02PM +, Chris Wilson wrote: Quoting Lucas De Marchi (2019-01-22 21:33:25) On Tue, Jan 22, 2019 at 6:32 AM Chris Wilson wrote: > > Quoting Lucas De Marchi (2019-01-22 05:12:24) > > Let's use a macro to make tables smaller and at the same time allow us > > to add fields that apply to all entries in future. > > > > For the sake of readability, I'm calling an exception on 80 chars limit. > > Lines are aligned for easy comparison of the entry values. > > > + MOCS_ENTRY(I915_MOCS_UNCACHED, LE_1_UC | LE_TC_2_LLC_ELLC, \ > > + L3_1_UC), \ > > MOCS_ENTRY(I915_MOCS_UNCACHED, > LE_1_UC | LE_TC_2_LLC_ELLC, L3_1_UC), \ My intention was to split the lines for each *value*, so it's easy to see what control_value vs l3cc_value is set to (too difficult to spot mistakes on adding a comma rather than a |). But I'm not strongly against your version, so I'll switch to that. Have another new line :) Because you are right as I confused that \ for a |. MOCS_ENTRY(I915_MOCS_UNCACHED, \ LE_1_UC | LE_TC_2_LLC_ELLC, \ L3_1_UC), \ ok. The Ice Lake table is huge (see last patch) and this will mandate 3 lines per entry, but at least it will be clear. Lucas De Marchi -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v8 4/7] drm/i915: use a macro to define MOCS entries
On 2019-01-22 06:12, Lucas De Marchi wrote:
Let's use a macro to make tables smaller and at the same time allow us
to add fields that apply to all entries in future.
For the sake of readability, I'm calling an exception on 80 chars limit.
Lines are aligned for easy comparison of the entry values.
Signed-off-by: Lucas De Marchi
Reviewed-by: Tomasz Lis
---
drivers/gpu/drm/i915/intel_mocs.c | 39 +++
1 file changed, 14 insertions(+), 25 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_mocs.c
b/drivers/gpu/drm/i915/intel_mocs.c
index c7a2a8d81d90..faae2eefc5cc 100644
--- a/drivers/gpu/drm/i915/intel_mocs.c
+++ b/drivers/gpu/drm/i915/intel_mocs.c
@@ -71,6 +71,12 @@ struct drm_i915_mocs_table {
#define L3_2_RESERVED _L3_CACHEABILITY(2)
#define L3_3_WB _L3_CACHEABILITY(3)
+#define MOCS_ENTRY(__idx, __control_value, __l3cc_value) \
+ [__idx] = { \
+ .control_value = __control_value, \
+ .l3cc_value = __l3cc_value, \
+ }
+
/*
* MOCS tables
*
@@ -93,40 +99,23 @@ struct drm_i915_mocs_table {
* may only be updated incrementally by adding entries at the
* end.
*/
-
The idea behind this EOL was that the comment above relates to several
statements below, not just the first one.
But I'm not really sure what our commenting rules are in this case - a
comment which bundles several definitions.
-Tomasz
#define GEN9_MOCS_ENTRIES \
- [I915_MOCS_UNCACHED] = { \
- /* 0x0009 */ \
- .control_value = LE_1_UC | LE_TC_2_LLC_ELLC, \
- /* 0x0010 */ \
- .l3cc_value = L3_1_UC, \
- }, \
- [I915_MOCS_PTE] = { \
- /* 0x0038 */ \
- .control_value = LE_0_PAGETABLE | LE_TC_2_LLC_ELLC |
LE_LRUM(3), \
- /* 0x0030 */ \
- .l3cc_value = L3_3_WB, \
- }
+ MOCS_ENTRY(I915_MOCS_UNCACHED, LE_1_UC | LE_TC_2_LLC_ELLC, \
+ L3_1_UC), \
+ MOCS_ENTRY(I915_MOCS_PTE, LE_0_PAGETABLE | LE_TC_2_LLC_ELLC |
LE_LRUM(3), \
+ L3_3_WB) \
static const struct drm_i915_mocs_entry skylake_mocs_table[] = {
GEN9_MOCS_ENTRIES,
- [I915_MOCS_CACHED] = {
- /* 0x003b */
- .control_value = LE_3_WB | LE_TC_2_LLC_ELLC | LE_LRUM(3),
- /* 0x0030 */
- .l3cc_value = L3_3_WB,
- },
+ MOCS_ENTRY(I915_MOCS_CACHED,LE_3_WB | LE_TC_2_LLC_ELLC | LE_LRUM(3),
+ L3_3_WB)
};
/* NOTE: the LE_TGT_CACHE is not used on Broxton */
static const struct drm_i915_mocs_entry broxton_mocs_table[] = {
GEN9_MOCS_ENTRIES,
- [I915_MOCS_CACHED] = {
- /* 0x0039 */
- .control_value = LE_1_UC | LE_TC_2_LLC_ELLC | LE_LRUM(3),
- /* 0x0030 */
- .l3cc_value = L3_3_WB,
- },
+ MOCS_ENTRY(I915_MOCS_CACHED,LE_1_UC | LE_TC_2_LLC_ELLC | LE_LRUM(3),
+ L3_3_WB)
};
/**
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v8 4/7] drm/i915: use a macro to define MOCS entries
Quoting Lucas De Marchi (2019-01-22 21:33:25) > On Tue, Jan 22, 2019 at 6:32 AM Chris Wilson wrote: > > > > Quoting Lucas De Marchi (2019-01-22 05:12:24) > > > Let's use a macro to make tables smaller and at the same time allow us > > > to add fields that apply to all entries in future. > > > > > > For the sake of readability, I'm calling an exception on 80 chars limit. > > > Lines are aligned for easy comparison of the entry values. > > > > > + MOCS_ENTRY(I915_MOCS_UNCACHED, LE_1_UC | LE_TC_2_LLC_ELLC, \ > > > + L3_1_UC), \ > > > > MOCS_ENTRY(I915_MOCS_UNCACHED, > > LE_1_UC | LE_TC_2_LLC_ELLC, L3_1_UC), \ > > My intention was to split the lines for each *value*, so it's easy to > see what control_value vs l3cc_value is set to > (too difficult to spot mistakes on adding a comma rather than a |). > > But I'm not strongly against your version, so I'll switch to that. Have another new line :) Because you are right as I confused that \ for a |. MOCS_ENTRY(I915_MOCS_UNCACHED, \ LE_1_UC | LE_TC_2_LLC_ELLC, \ L3_1_UC), \ -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v8 4/7] drm/i915: use a macro to define MOCS entries
On Tue, Jan 22, 2019 at 6:32 AM Chris Wilson wrote: > > Quoting Lucas De Marchi (2019-01-22 05:12:24) > > Let's use a macro to make tables smaller and at the same time allow us > > to add fields that apply to all entries in future. > > > > For the sake of readability, I'm calling an exception on 80 chars limit. > > Lines are aligned for easy comparison of the entry values. > > > + MOCS_ENTRY(I915_MOCS_UNCACHED, LE_1_UC | LE_TC_2_LLC_ELLC, \ > > + L3_1_UC), \ > > MOCS_ENTRY(I915_MOCS_UNCACHED, > LE_1_UC | LE_TC_2_LLC_ELLC, L3_1_UC), \ My intention was to split the lines for each *value*, so it's easy to see what control_value vs l3cc_value is set to (too difficult to spot mistakes on adding a comma rather than a |). But I'm not strongly against your version, so I'll switch to that. thanks Lucas De Marchi > > is even more readable with the visual clustering of attribute flags. > -Chris > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Lucas De Marchi ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v8 4/7] drm/i915: use a macro to define MOCS entries
Quoting Lucas De Marchi (2019-01-22 05:12:24) > Let's use a macro to make tables smaller and at the same time allow us > to add fields that apply to all entries in future. > > For the sake of readability, I'm calling an exception on 80 chars limit. > Lines are aligned for easy comparison of the entry values. > + MOCS_ENTRY(I915_MOCS_UNCACHED, LE_1_UC | LE_TC_2_LLC_ELLC, \ > + L3_1_UC), \ MOCS_ENTRY(I915_MOCS_UNCACHED, LE_1_UC | LE_TC_2_LLC_ELLC, L3_1_UC), \ is even more readable with the visual clustering of attribute flags. -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v8 4/7] drm/i915: use a macro to define MOCS entries
Let's use a macro to make tables smaller and at the same time allow us
to add fields that apply to all entries in future.
For the sake of readability, I'm calling an exception on 80 chars limit.
Lines are aligned for easy comparison of the entry values.
Signed-off-by: Lucas De Marchi
---
drivers/gpu/drm/i915/intel_mocs.c | 39 +++
1 file changed, 14 insertions(+), 25 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_mocs.c
b/drivers/gpu/drm/i915/intel_mocs.c
index c7a2a8d81d90..faae2eefc5cc 100644
--- a/drivers/gpu/drm/i915/intel_mocs.c
+++ b/drivers/gpu/drm/i915/intel_mocs.c
@@ -71,6 +71,12 @@ struct drm_i915_mocs_table {
#define L3_2_RESERVED _L3_CACHEABILITY(2)
#define L3_3_WB_L3_CACHEABILITY(3)
+#define MOCS_ENTRY(__idx, __control_value, __l3cc_value) \
+ [__idx] = { \
+ .control_value = __control_value, \
+ .l3cc_value = __l3cc_value, \
+ }
+
/*
* MOCS tables
*
@@ -93,40 +99,23 @@ struct drm_i915_mocs_table {
* may only be updated incrementally by adding entries at the
* end.
*/
-
#define GEN9_MOCS_ENTRIES \
- [I915_MOCS_UNCACHED] = { \
- /* 0x0009 */ \
- .control_value = LE_1_UC | LE_TC_2_LLC_ELLC, \
- /* 0x0010 */ \
- .l3cc_value = L3_1_UC, \
- }, \
- [I915_MOCS_PTE] = { \
- /* 0x0038 */ \
- .control_value = LE_0_PAGETABLE | LE_TC_2_LLC_ELLC |
LE_LRUM(3), \
- /* 0x0030 */ \
- .l3cc_value = L3_3_WB, \
- }
+ MOCS_ENTRY(I915_MOCS_UNCACHED, LE_1_UC | LE_TC_2_LLC_ELLC, \
+ L3_1_UC), \
+ MOCS_ENTRY(I915_MOCS_PTE, LE_0_PAGETABLE | LE_TC_2_LLC_ELLC |
LE_LRUM(3), \
+ L3_3_WB) \
static const struct drm_i915_mocs_entry skylake_mocs_table[] = {
GEN9_MOCS_ENTRIES,
- [I915_MOCS_CACHED] = {
- /* 0x003b */
- .control_value = LE_3_WB | LE_TC_2_LLC_ELLC | LE_LRUM(3),
- /* 0x0030 */
- .l3cc_value = L3_3_WB,
- },
+ MOCS_ENTRY(I915_MOCS_CACHED,LE_3_WB | LE_TC_2_LLC_ELLC | LE_LRUM(3),
+ L3_3_WB)
};
/* NOTE: the LE_TGT_CACHE is not used on Broxton */
static const struct drm_i915_mocs_entry broxton_mocs_table[] = {
GEN9_MOCS_ENTRIES,
- [I915_MOCS_CACHED] = {
- /* 0x0039 */
- .control_value = LE_1_UC | LE_TC_2_LLC_ELLC | LE_LRUM(3),
- /* 0x0030 */
- .l3cc_value = L3_3_WB,
- },
+ MOCS_ENTRY(I915_MOCS_CACHED,LE_1_UC | LE_TC_2_LLC_ELLC | LE_LRUM(3),
+ L3_3_WB)
};
/**
--
2.20.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
