| Message ID | 20210421153401.13847-5-ville.syrjala@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915: Fix older platforms | expand |
On Wed, Apr 21, 2021 at 06:34:01PM +0300, Ville Syrjala wrote: > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > Currently we try to detect a symmetric memory configurations > using a magic DCC2_MODIFIED_ENHANCED_DISABLE bit. That bit is > either only set on a very specific subset of machines or it > just does not exist (it's not mentioned in any public chipset > datasheets I've found). As it happens my CL/CTG machines never > set said bit, even if I populate the channels with identical > sticks. > > So let's do the L-shaped memory detection the same way as the > desktop variants, ie. just look at the DRAM rank boundary > registers to see if both channels have an identical size. > > With this my CL/CTG no longer claim L-shaped memory when I use > identical sticks. Also tested with non-matching sticks just to > make sure the L-shaped memory is still properly detected. > > And for completeness let's update the debugfs code to dump > the correct set of registers on each platform. > > Cc: Chris Wilson <chris@chris-wilson.co.uk> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> Did you check this with the swapping igt? I have some vague memories of bug reports where somehow the machine was acting like it's L-shaped memory despite that banks were populated equally. I've iirc tried all kinds of tricks to figure it out, all to absolutely no avail. tbh I'd just not touch this, not really worth it. -Daniel > --- > drivers/gpu/drm/i915/gt/intel_ggtt_fencing.c | 15 ++++++++------- > drivers/gpu/drm/i915/i915_debugfs.c | 16 ++++++++++++---- > drivers/gpu/drm/i915/i915_reg.h | 4 ++++ > 3 files changed, 24 insertions(+), 11 deletions(-) > > diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt_fencing.c b/drivers/gpu/drm/i915/gt/intel_ggtt_fencing.c > index 0fa6c38893f7..754f20768de5 100644 > --- a/drivers/gpu/drm/i915/gt/intel_ggtt_fencing.c > +++ b/drivers/gpu/drm/i915/gt/intel_ggtt_fencing.c > @@ -693,14 +693,15 @@ static void detect_bit_6_swizzle(struct i915_ggtt *ggtt) > swizzle_x = I915_BIT_6_SWIZZLE_9_10_17; > swizzle_y = I915_BIT_6_SWIZZLE_9_17; > } > - break; > - } > > - /* check for L-shaped memory aka modified enhanced addressing */ > - if (IS_GEN(i915, 4) && > - !(intel_uncore_read(uncore, DCC2) & DCC2_MODIFIED_ENHANCED_DISABLE)) { > - swizzle_x = I915_BIT_6_SWIZZLE_UNKNOWN; > - swizzle_y = I915_BIT_6_SWIZZLE_UNKNOWN; > + /* check for L-shaped memory aka modified enhanced addressing */ > + if (IS_GEN(i915, 4) && > + intel_uncore_read16(uncore, C0DRB3_CL) != > + intel_uncore_read16(uncore, C1DRB3_CL)) { > + swizzle_x = I915_BIT_6_SWIZZLE_UNKNOWN; > + swizzle_y = I915_BIT_6_SWIZZLE_UNKNOWN; > + } > + break; > } > > if (dcc == 0xffffffff) { > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c > index 8dd374691102..6de11ffcde38 100644 > --- a/drivers/gpu/drm/i915/i915_debugfs.c > +++ b/drivers/gpu/drm/i915/i915_debugfs.c > @@ -621,10 +621,18 @@ static int i915_swizzle_info(struct seq_file *m, void *data) > intel_uncore_read(uncore, DCC)); > seq_printf(m, "DDC2 = 0x%08x\n", > intel_uncore_read(uncore, DCC2)); > - seq_printf(m, "C0DRB3 = 0x%04x\n", > - intel_uncore_read16(uncore, C0DRB3_BW)); > - seq_printf(m, "C1DRB3 = 0x%04x\n", > - intel_uncore_read16(uncore, C1DRB3_BW)); > + > + if (IS_G45(dev_priv) || IS_I965G(dev_priv) || IS_G33(dev_priv)) { > + seq_printf(m, "C0DRB3 = 0x%04x\n", > + intel_uncore_read16(uncore, C0DRB3_BW)); > + seq_printf(m, "C1DRB3 = 0x%04x\n", > + intel_uncore_read16(uncore, C1DRB3_BW)); > + } else if (IS_GEN(dev_priv, 4)) { > + seq_printf(m, "C0DRB3 = 0x%04x\n", > + intel_uncore_read16(uncore, C0DRB3_CL)); > + seq_printf(m, "C1DRB3 = 0x%04x\n", > + intel_uncore_read16(uncore, C1DRB3_CL)); > + } > } else if (INTEL_GEN(dev_priv) >= 6) { > seq_printf(m, "MAD_DIMM_C0 = 0x%08x\n", > intel_uncore_read(uncore, MAD_DIMM_C0)); > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index 0587b2455ea1..055c258179a1 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -3790,6 +3790,10 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg) > #define C0DRB3_BW _MMIO(MCHBAR_MIRROR_BASE + 0x206) > #define C1DRB3_BW _MMIO(MCHBAR_MIRROR_BASE + 0x606) > > +/* 965gm,ctg DRAM channel configuration */ > +#define C0DRB3_CL _MMIO(MCHBAR_MIRROR_BASE + 0x1206) > +#define C1DRB3_CL _MMIO(MCHBAR_MIRROR_BASE + 0x1306) > + > /* snb MCH registers for reading the DRAM channel configuration */ > #define MAD_DIMM_C0 _MMIO(MCHBAR_MIRROR_BASE_SNB + 0x5004) > #define MAD_DIMM_C1 _MMIO(MCHBAR_MIRROR_BASE_SNB + 0x5008) > -- > 2.26.3 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Thu, Apr 22, 2021 at 11:49:43AM +0200, Daniel Vetter wrote: > On Wed, Apr 21, 2021 at 06:34:01PM +0300, Ville Syrjala wrote: > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > Currently we try to detect a symmetric memory configurations > > using a magic DCC2_MODIFIED_ENHANCED_DISABLE bit. That bit is > > either only set on a very specific subset of machines or it > > just does not exist (it's not mentioned in any public chipset > > datasheets I've found). As it happens my CL/CTG machines never > > set said bit, even if I populate the channels with identical > > sticks. > > > > So let's do the L-shaped memory detection the same way as the > > desktop variants, ie. just look at the DRAM rank boundary > > registers to see if both channels have an identical size. > > > > With this my CL/CTG no longer claim L-shaped memory when I use > > identical sticks. Also tested with non-matching sticks just to > > make sure the L-shaped memory is still properly detected. > > > > And for completeness let's update the debugfs code to dump > > the correct set of registers on each platform. > > > > Cc: Chris Wilson <chris@chris-wilson.co.uk> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > Did you check this with the swapping igt? I have some vague memories of > bug reports where somehow the machine was acting like it's L-shaped memory > despite that banks were populated equally. I've iirc tried all kinds of > tricks to figure it out, all to absolutely no avail. Did you have a specific test in mind? I ran a bunch of things that seemed swizzle related. All passed just fine. Chris did have similar concerns and suggested we should have better tests. I guess what I should try to do is some selftests which make sure we test both high and low physical addresses and check the swizzle pattern is as expected. But haven't found the time to do that yet. > > tbh I'd just not touch this, not really worth it. It's totally worth it to get gen4 machines working again. > -Daniel > > --- > > drivers/gpu/drm/i915/gt/intel_ggtt_fencing.c | 15 ++++++++------- > > drivers/gpu/drm/i915/i915_debugfs.c | 16 ++++++++++++---- > > drivers/gpu/drm/i915/i915_reg.h | 4 ++++ > > 3 files changed, 24 insertions(+), 11 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt_fencing.c b/drivers/gpu/drm/i915/gt/intel_ggtt_fencing.c > > index 0fa6c38893f7..754f20768de5 100644 > > --- a/drivers/gpu/drm/i915/gt/intel_ggtt_fencing.c > > +++ b/drivers/gpu/drm/i915/gt/intel_ggtt_fencing.c > > @@ -693,14 +693,15 @@ static void detect_bit_6_swizzle(struct i915_ggtt *ggtt) > > swizzle_x = I915_BIT_6_SWIZZLE_9_10_17; > > swizzle_y = I915_BIT_6_SWIZZLE_9_17; > > } > > - break; > > - } > > > > - /* check for L-shaped memory aka modified enhanced addressing */ > > - if (IS_GEN(i915, 4) && > > - !(intel_uncore_read(uncore, DCC2) & DCC2_MODIFIED_ENHANCED_DISABLE)) { > > - swizzle_x = I915_BIT_6_SWIZZLE_UNKNOWN; > > - swizzle_y = I915_BIT_6_SWIZZLE_UNKNOWN; > > + /* check for L-shaped memory aka modified enhanced addressing */ > > + if (IS_GEN(i915, 4) && > > + intel_uncore_read16(uncore, C0DRB3_CL) != > > + intel_uncore_read16(uncore, C1DRB3_CL)) { > > + swizzle_x = I915_BIT_6_SWIZZLE_UNKNOWN; > > + swizzle_y = I915_BIT_6_SWIZZLE_UNKNOWN; > > + } > > + break; > > } > > > > if (dcc == 0xffffffff) { > > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c > > index 8dd374691102..6de11ffcde38 100644 > > --- a/drivers/gpu/drm/i915/i915_debugfs.c > > +++ b/drivers/gpu/drm/i915/i915_debugfs.c > > @@ -621,10 +621,18 @@ static int i915_swizzle_info(struct seq_file *m, void *data) > > intel_uncore_read(uncore, DCC)); > > seq_printf(m, "DDC2 = 0x%08x\n", > > intel_uncore_read(uncore, DCC2)); > > - seq_printf(m, "C0DRB3 = 0x%04x\n", > > - intel_uncore_read16(uncore, C0DRB3_BW)); > > - seq_printf(m, "C1DRB3 = 0x%04x\n", > > - intel_uncore_read16(uncore, C1DRB3_BW)); > > + > > + if (IS_G45(dev_priv) || IS_I965G(dev_priv) || IS_G33(dev_priv)) { > > + seq_printf(m, "C0DRB3 = 0x%04x\n", > > + intel_uncore_read16(uncore, C0DRB3_BW)); > > + seq_printf(m, "C1DRB3 = 0x%04x\n", > > + intel_uncore_read16(uncore, C1DRB3_BW)); > > + } else if (IS_GEN(dev_priv, 4)) { > > + seq_printf(m, "C0DRB3 = 0x%04x\n", > > + intel_uncore_read16(uncore, C0DRB3_CL)); > > + seq_printf(m, "C1DRB3 = 0x%04x\n", > > + intel_uncore_read16(uncore, C1DRB3_CL)); > > + } > > } else if (INTEL_GEN(dev_priv) >= 6) { > > seq_printf(m, "MAD_DIMM_C0 = 0x%08x\n", > > intel_uncore_read(uncore, MAD_DIMM_C0)); > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > > index 0587b2455ea1..055c258179a1 100644 > > --- a/drivers/gpu/drm/i915/i915_reg.h > > +++ b/drivers/gpu/drm/i915/i915_reg.h > > @@ -3790,6 +3790,10 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg) > > #define C0DRB3_BW _MMIO(MCHBAR_MIRROR_BASE + 0x206) > > #define C1DRB3_BW _MMIO(MCHBAR_MIRROR_BASE + 0x606) > > > > +/* 965gm,ctg DRAM channel configuration */ > > +#define C0DRB3_CL _MMIO(MCHBAR_MIRROR_BASE + 0x1206) > > +#define C1DRB3_CL _MMIO(MCHBAR_MIRROR_BASE + 0x1306) > > + > > /* snb MCH registers for reading the DRAM channel configuration */ > > #define MAD_DIMM_C0 _MMIO(MCHBAR_MIRROR_BASE_SNB + 0x5004) > > #define MAD_DIMM_C1 _MMIO(MCHBAR_MIRROR_BASE_SNB + 0x5008) > > -- > > 2.26.3 > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch
On Thu, Apr 22, 2021 at 11:49:43AM +0200, Daniel Vetter wrote: > On Wed, Apr 21, 2021 at 06:34:01PM +0300, Ville Syrjala wrote: > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > Currently we try to detect a symmetric memory configurations > > using a magic DCC2_MODIFIED_ENHANCED_DISABLE bit. That bit is > > either only set on a very specific subset of machines or it > > just does not exist (it's not mentioned in any public chipset > > datasheets I've found). As it happens my CL/CTG machines never > > set said bit, even if I populate the channels with identical > > sticks. > > > > So let's do the L-shaped memory detection the same way as the > > desktop variants, ie. just look at the DRAM rank boundary > > registers to see if both channels have an identical size. > > > > With this my CL/CTG no longer claim L-shaped memory when I use > > identical sticks. Also tested with non-matching sticks just to > > make sure the L-shaped memory is still properly detected. > > > > And for completeness let's update the debugfs code to dump > > the correct set of registers on each platform. > > > > Cc: Chris Wilson <chris@chris-wilson.co.uk> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > Did you check this with the swapping igt? I have some vague memories of > bug reports where somehow the machine was acting like it's L-shaped memory > despite that banks were populated equally. I've iirc tried all kinds of > tricks to figure it out, all to absolutely no avail. BTW looking at the patches/dumps in eg. https://bugs.freedesktop.org/show_bug.cgi?id=28813 I can't immediately see a single thing that is actually using the correct register offsets for cl/ctg. So I'm a bit sceptical about how well this was researched in the past.
On Thu, Apr 22, 2021 at 04:11:22PM +0300, Ville Syrjälä wrote: > On Thu, Apr 22, 2021 at 11:49:43AM +0200, Daniel Vetter wrote: > > On Wed, Apr 21, 2021 at 06:34:01PM +0300, Ville Syrjala wrote: > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > > > Currently we try to detect a symmetric memory configurations > > > using a magic DCC2_MODIFIED_ENHANCED_DISABLE bit. That bit is > > > either only set on a very specific subset of machines or it > > > just does not exist (it's not mentioned in any public chipset > > > datasheets I've found). As it happens my CL/CTG machines never > > > set said bit, even if I populate the channels with identical > > > sticks. > > > > > > So let's do the L-shaped memory detection the same way as the > > > desktop variants, ie. just look at the DRAM rank boundary > > > registers to see if both channels have an identical size. > > > > > > With this my CL/CTG no longer claim L-shaped memory when I use > > > identical sticks. Also tested with non-matching sticks just to > > > make sure the L-shaped memory is still properly detected. > > > > > > And for completeness let's update the debugfs code to dump > > > the correct set of registers on each platform. > > > > > > Cc: Chris Wilson <chris@chris-wilson.co.uk> > > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > Did you check this with the swapping igt? I have some vague memories of > > bug reports where somehow the machine was acting like it's L-shaped memory > > despite that banks were populated equally. I've iirc tried all kinds of > > tricks to figure it out, all to absolutely no avail. > > Did you have a specific test in mind? I ran a bunch of things > that seemed swizzle related. All passed just fine. gem_tiled_swapping should be the one. It tries to cycle your entire system memory through tiled buffers into swap and out of it. -Daniel > > Chris did have similar concerns and suggested we should have > better tests. I guess what I should try to do is some selftests > which make sure we test both high and low physical addresses > and check the swizzle pattern is as expected. But haven't > found the time to do that yet. > > > > > tbh I'd just not touch this, not really worth it. > > It's totally worth it to get gen4 machines working again. > > > > -Daniel > > > --- > > > drivers/gpu/drm/i915/gt/intel_ggtt_fencing.c | 15 ++++++++------- > > > drivers/gpu/drm/i915/i915_debugfs.c | 16 ++++++++++++---- > > > drivers/gpu/drm/i915/i915_reg.h | 4 ++++ > > > 3 files changed, 24 insertions(+), 11 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt_fencing.c b/drivers/gpu/drm/i915/gt/intel_ggtt_fencing.c > > > index 0fa6c38893f7..754f20768de5 100644 > > > --- a/drivers/gpu/drm/i915/gt/intel_ggtt_fencing.c > > > +++ b/drivers/gpu/drm/i915/gt/intel_ggtt_fencing.c > > > @@ -693,14 +693,15 @@ static void detect_bit_6_swizzle(struct i915_ggtt *ggtt) > > > swizzle_x = I915_BIT_6_SWIZZLE_9_10_17; > > > swizzle_y = I915_BIT_6_SWIZZLE_9_17; > > > } > > > - break; > > > - } > > > > > > - /* check for L-shaped memory aka modified enhanced addressing */ > > > - if (IS_GEN(i915, 4) && > > > - !(intel_uncore_read(uncore, DCC2) & DCC2_MODIFIED_ENHANCED_DISABLE)) { > > > - swizzle_x = I915_BIT_6_SWIZZLE_UNKNOWN; > > > - swizzle_y = I915_BIT_6_SWIZZLE_UNKNOWN; > > > + /* check for L-shaped memory aka modified enhanced addressing */ > > > + if (IS_GEN(i915, 4) && > > > + intel_uncore_read16(uncore, C0DRB3_CL) != > > > + intel_uncore_read16(uncore, C1DRB3_CL)) { > > > + swizzle_x = I915_BIT_6_SWIZZLE_UNKNOWN; > > > + swizzle_y = I915_BIT_6_SWIZZLE_UNKNOWN; > > > + } > > > + break; > > > } > > > > > > if (dcc == 0xffffffff) { > > > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c > > > index 8dd374691102..6de11ffcde38 100644 > > > --- a/drivers/gpu/drm/i915/i915_debugfs.c > > > +++ b/drivers/gpu/drm/i915/i915_debugfs.c > > > @@ -621,10 +621,18 @@ static int i915_swizzle_info(struct seq_file *m, void *data) > > > intel_uncore_read(uncore, DCC)); > > > seq_printf(m, "DDC2 = 0x%08x\n", > > > intel_uncore_read(uncore, DCC2)); > > > - seq_printf(m, "C0DRB3 = 0x%04x\n", > > > - intel_uncore_read16(uncore, C0DRB3_BW)); > > > - seq_printf(m, "C1DRB3 = 0x%04x\n", > > > - intel_uncore_read16(uncore, C1DRB3_BW)); > > > + > > > + if (IS_G45(dev_priv) || IS_I965G(dev_priv) || IS_G33(dev_priv)) { > > > + seq_printf(m, "C0DRB3 = 0x%04x\n", > > > + intel_uncore_read16(uncore, C0DRB3_BW)); > > > + seq_printf(m, "C1DRB3 = 0x%04x\n", > > > + intel_uncore_read16(uncore, C1DRB3_BW)); > > > + } else if (IS_GEN(dev_priv, 4)) { > > > + seq_printf(m, "C0DRB3 = 0x%04x\n", > > > + intel_uncore_read16(uncore, C0DRB3_CL)); > > > + seq_printf(m, "C1DRB3 = 0x%04x\n", > > > + intel_uncore_read16(uncore, C1DRB3_CL)); > > > + } > > > } else if (INTEL_GEN(dev_priv) >= 6) { > > > seq_printf(m, "MAD_DIMM_C0 = 0x%08x\n", > > > intel_uncore_read(uncore, MAD_DIMM_C0)); > > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > > > index 0587b2455ea1..055c258179a1 100644 > > > --- a/drivers/gpu/drm/i915/i915_reg.h > > > +++ b/drivers/gpu/drm/i915/i915_reg.h > > > @@ -3790,6 +3790,10 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg) > > > #define C0DRB3_BW _MMIO(MCHBAR_MIRROR_BASE + 0x206) > > > #define C1DRB3_BW _MMIO(MCHBAR_MIRROR_BASE + 0x606) > > > > > > +/* 965gm,ctg DRAM channel configuration */ > > > +#define C0DRB3_CL _MMIO(MCHBAR_MIRROR_BASE + 0x1206) > > > +#define C1DRB3_CL _MMIO(MCHBAR_MIRROR_BASE + 0x1306) > > > + > > > /* snb MCH registers for reading the DRAM channel configuration */ > > > #define MAD_DIMM_C0 _MMIO(MCHBAR_MIRROR_BASE_SNB + 0x5004) > > > #define MAD_DIMM_C1 _MMIO(MCHBAR_MIRROR_BASE_SNB + 0x5008) > > > -- > > > 2.26.3 > > > > > > _______________________________________________ > > > Intel-gfx mailing list > > > Intel-gfx@lists.freedesktop.org > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > > > -- > > Daniel Vetter > > Software Engineer, Intel Corporation > > http://blog.ffwll.ch > > -- > Ville Syrjälä > Intel
On Mon, Apr 26, 2021 at 06:08:59PM +0200, Daniel Vetter wrote: > On Thu, Apr 22, 2021 at 04:11:22PM +0300, Ville Syrjälä wrote: > > On Thu, Apr 22, 2021 at 11:49:43AM +0200, Daniel Vetter wrote: > > > On Wed, Apr 21, 2021 at 06:34:01PM +0300, Ville Syrjala wrote: > > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > > > > > Currently we try to detect a symmetric memory configurations > > > > using a magic DCC2_MODIFIED_ENHANCED_DISABLE bit. That bit is > > > > either only set on a very specific subset of machines or it > > > > just does not exist (it's not mentioned in any public chipset > > > > datasheets I've found). As it happens my CL/CTG machines never > > > > set said bit, even if I populate the channels with identical > > > > sticks. > > > > > > > > So let's do the L-shaped memory detection the same way as the > > > > desktop variants, ie. just look at the DRAM rank boundary > > > > registers to see if both channels have an identical size. > > > > > > > > With this my CL/CTG no longer claim L-shaped memory when I use > > > > identical sticks. Also tested with non-matching sticks just to > > > > make sure the L-shaped memory is still properly detected. > > > > > > > > And for completeness let's update the debugfs code to dump > > > > the correct set of registers on each platform. > > > > > > > > Cc: Chris Wilson <chris@chris-wilson.co.uk> > > > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > > > Did you check this with the swapping igt? I have some vague memories of > > > bug reports where somehow the machine was acting like it's L-shaped memory > > > despite that banks were populated equally. I've iirc tried all kinds of > > > tricks to figure it out, all to absolutely no avail. > > > > Did you have a specific test in mind? I ran a bunch of things > > that seemed swizzle related. All passed just fine. > > gem_tiled_swapping should be the one. It tries to cycle your entire system > memory through tiled buffers into swap and out of it. Passes with symmetric config, fails with L-shaped config (if I hack out the L-shape detection of course). So seems pretty solid. A kernel based self test that looks at the physical address would still be nice I suppose. Though depending on the size of your RAM sticks figuring out where exactly the switchover from two channels to one channels happens probably requires a bit of work due to the PCI hole/etc. Both my cl and ctg report this btw: bit6 swizzle for X-tiling = bit9/bit10/bit11 bit6 swizzle for Y-tiling = bit9/bit11 so unfortunately can't be sure the other swizzle modes would be correctly detected.
On Mon, Apr 26, 2021 at 08:18:39PM +0300, Ville Syrjälä wrote: > On Mon, Apr 26, 2021 at 06:08:59PM +0200, Daniel Vetter wrote: > > On Thu, Apr 22, 2021 at 04:11:22PM +0300, Ville Syrjälä wrote: > > > On Thu, Apr 22, 2021 at 11:49:43AM +0200, Daniel Vetter wrote: > > > > On Wed, Apr 21, 2021 at 06:34:01PM +0300, Ville Syrjala wrote: > > > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > > > > > > > Currently we try to detect a symmetric memory configurations > > > > > using a magic DCC2_MODIFIED_ENHANCED_DISABLE bit. That bit is > > > > > either only set on a very specific subset of machines or it > > > > > just does not exist (it's not mentioned in any public chipset > > > > > datasheets I've found). As it happens my CL/CTG machines never > > > > > set said bit, even if I populate the channels with identical > > > > > sticks. > > > > > > > > > > So let's do the L-shaped memory detection the same way as the > > > > > desktop variants, ie. just look at the DRAM rank boundary > > > > > registers to see if both channels have an identical size. > > > > > > > > > > With this my CL/CTG no longer claim L-shaped memory when I use > > > > > identical sticks. Also tested with non-matching sticks just to > > > > > make sure the L-shaped memory is still properly detected. > > > > > > > > > > And for completeness let's update the debugfs code to dump > > > > > the correct set of registers on each platform. > > > > > > > > > > Cc: Chris Wilson <chris@chris-wilson.co.uk> > > > > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > > > > > Did you check this with the swapping igt? I have some vague memories of > > > > bug reports where somehow the machine was acting like it's L-shaped memory > > > > despite that banks were populated equally. I've iirc tried all kinds of > > > > tricks to figure it out, all to absolutely no avail. > > > > > > Did you have a specific test in mind? I ran a bunch of things > > > that seemed swizzle related. All passed just fine. > > > > gem_tiled_swapping should be the one. It tries to cycle your entire system > > memory through tiled buffers into swap and out of it. > > Passes with symmetric config, fails with L-shaped config (if I hack > out the L-shape detection of course). So seems pretty solid. > > A kernel based self test that looks at the physical address would > still be nice I suppose. Though depending on the size of your RAM > sticks figuring out where exactly the switchover from two channels > to one channels happens probably requires a bit of work due to > the PCI hole/etc. > > Both my cl and ctg report this btw: > bit6 swizzle for X-tiling = bit9/bit10/bit11 > bit6 swizzle for Y-tiling = bit9/bit11 > so unfortunately can't be sure the other swizzle modes would be > correctly detected. I think testing-wise this is as good as it gets. -Daniel
On Tue, Apr 27, 2021 at 10:58:07AM +0200, Daniel Vetter wrote: > On Mon, Apr 26, 2021 at 08:18:39PM +0300, Ville Syrjälä wrote: > > On Mon, Apr 26, 2021 at 06:08:59PM +0200, Daniel Vetter wrote: > > > On Thu, Apr 22, 2021 at 04:11:22PM +0300, Ville Syrjälä wrote: > > > > On Thu, Apr 22, 2021 at 11:49:43AM +0200, Daniel Vetter wrote: > > > > > On Wed, Apr 21, 2021 at 06:34:01PM +0300, Ville Syrjala wrote: > > > > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > > > > > > > > > Currently we try to detect a symmetric memory configurations > > > > > > using a magic DCC2_MODIFIED_ENHANCED_DISABLE bit. That bit is > > > > > > either only set on a very specific subset of machines or it > > > > > > just does not exist (it's not mentioned in any public chipset > > > > > > datasheets I've found). As it happens my CL/CTG machines never > > > > > > set said bit, even if I populate the channels with identical > > > > > > sticks. > > > > > > > > > > > > So let's do the L-shaped memory detection the same way as the > > > > > > desktop variants, ie. just look at the DRAM rank boundary > > > > > > registers to see if both channels have an identical size. > > > > > > > > > > > > With this my CL/CTG no longer claim L-shaped memory when I use > > > > > > identical sticks. Also tested with non-matching sticks just to > > > > > > make sure the L-shaped memory is still properly detected. > > > > > > > > > > > > And for completeness let's update the debugfs code to dump > > > > > > the correct set of registers on each platform. > > > > > > > > > > > > Cc: Chris Wilson <chris@chris-wilson.co.uk> > > > > > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > > > > > > > Did you check this with the swapping igt? I have some vague memories of > > > > > bug reports where somehow the machine was acting like it's L-shaped memory > > > > > despite that banks were populated equally. I've iirc tried all kinds of > > > > > tricks to figure it out, all to absolutely no avail. > > > > > > > > Did you have a specific test in mind? I ran a bunch of things > > > > that seemed swizzle related. All passed just fine. > > > > > > gem_tiled_swapping should be the one. It tries to cycle your entire system > > > memory through tiled buffers into swap and out of it. > > > > Passes with symmetric config, fails with L-shaped config (if I hack > > out the L-shape detection of course). So seems pretty solid. > > > > A kernel based self test that looks at the physical address would > > still be nice I suppose. Though depending on the size of your RAM > > sticks figuring out where exactly the switchover from two channels > > to one channels happens probably requires a bit of work due to > > the PCI hole/etc. > > > > Both my cl and ctg report this btw: > > bit6 swizzle for X-tiling = bit9/bit10/bit11 > > bit6 swizzle for Y-tiling = bit9/bit11 > > so unfortunately can't be sure the other swizzle modes would be > > correctly detected. > > I think testing-wise this is as good as it gets. So what do we think about putting this in? Currently this only works by sheer luck more or less. On my machines we have a false positive which is safe now that the pin quirk got fixed, but if some other machines have a false negative then things are not going to go so well.
diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt_fencing.c b/drivers/gpu/drm/i915/gt/intel_ggtt_fencing.c index 0fa6c38893f7..754f20768de5 100644 --- a/drivers/gpu/drm/i915/gt/intel_ggtt_fencing.c +++ b/drivers/gpu/drm/i915/gt/intel_ggtt_fencing.c @@ -693,14 +693,15 @@ static void detect_bit_6_swizzle(struct i915_ggtt *ggtt) swizzle_x = I915_BIT_6_SWIZZLE_9_10_17; swizzle_y = I915_BIT_6_SWIZZLE_9_17; } - break; - } - /* check for L-shaped memory aka modified enhanced addressing */ - if (IS_GEN(i915, 4) && - !(intel_uncore_read(uncore, DCC2) & DCC2_MODIFIED_ENHANCED_DISABLE)) { - swizzle_x = I915_BIT_6_SWIZZLE_UNKNOWN; - swizzle_y = I915_BIT_6_SWIZZLE_UNKNOWN; + /* check for L-shaped memory aka modified enhanced addressing */ + if (IS_GEN(i915, 4) && + intel_uncore_read16(uncore, C0DRB3_CL) != + intel_uncore_read16(uncore, C1DRB3_CL)) { + swizzle_x = I915_BIT_6_SWIZZLE_UNKNOWN; + swizzle_y = I915_BIT_6_SWIZZLE_UNKNOWN; + } + break; } if (dcc == 0xffffffff) { diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index 8dd374691102..6de11ffcde38 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -621,10 +621,18 @@ static int i915_swizzle_info(struct seq_file *m, void *data) intel_uncore_read(uncore, DCC)); seq_printf(m, "DDC2 = 0x%08x\n", intel_uncore_read(uncore, DCC2)); - seq_printf(m, "C0DRB3 = 0x%04x\n", - intel_uncore_read16(uncore, C0DRB3_BW)); - seq_printf(m, "C1DRB3 = 0x%04x\n", - intel_uncore_read16(uncore, C1DRB3_BW)); + + if (IS_G45(dev_priv) || IS_I965G(dev_priv) || IS_G33(dev_priv)) { + seq_printf(m, "C0DRB3 = 0x%04x\n", + intel_uncore_read16(uncore, C0DRB3_BW)); + seq_printf(m, "C1DRB3 = 0x%04x\n", + intel_uncore_read16(uncore, C1DRB3_BW)); + } else if (IS_GEN(dev_priv, 4)) { + seq_printf(m, "C0DRB3 = 0x%04x\n", + intel_uncore_read16(uncore, C0DRB3_CL)); + seq_printf(m, "C1DRB3 = 0x%04x\n", + intel_uncore_read16(uncore, C1DRB3_CL)); + } } else if (INTEL_GEN(dev_priv) >= 6) { seq_printf(m, "MAD_DIMM_C0 = 0x%08x\n", intel_uncore_read(uncore, MAD_DIMM_C0)); diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 0587b2455ea1..055c258179a1 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -3790,6 +3790,10 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg) #define C0DRB3_BW _MMIO(MCHBAR_MIRROR_BASE + 0x206) #define C1DRB3_BW _MMIO(MCHBAR_MIRROR_BASE + 0x606) +/* 965gm,ctg DRAM channel configuration */ +#define C0DRB3_CL _MMIO(MCHBAR_MIRROR_BASE + 0x1206) +#define C1DRB3_CL _MMIO(MCHBAR_MIRROR_BASE + 0x1306) + /* snb MCH registers for reading the DRAM channel configuration */ #define MAD_DIMM_C0 _MMIO(MCHBAR_MIRROR_BASE_SNB + 0x5004) #define MAD_DIMM_C1 _MMIO(MCHBAR_MIRROR_BASE_SNB + 0x5008)