diff mbox series

[2/9] drm/i915/adl_s: MCHBAR memory info registers are moved

Message ID 20210127041159.136409-3-aditya.swarup@intel.com (mailing list archive)
State New, archived
Headers show
Series Final set of patches for ADLS enabling | expand

Commit Message

Aditya Swarup Jan. 27, 2021, 4:11 a.m. UTC 
From: Caz Yokoyama <caz.yokoyama@intel.com>

The crwebview indicates on ADL-S that some of our MCHBAR
registers have moved from their traditional 0x50XX offsets to
new locations. The meaning and bit layout of the registers
remain same.

v2: Simplify logic to a single if else chain and fix indents.(Lucas)

Cc: Lucas De Marchi <lucas.demarchi@intel.com>
Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Imre Deak <imre.deak@intel.com>
Cc: Matt Roper <matthew.d.roper@intel.com>
Signed-off-by: Caz Yokoyama <caz.yokoyama@intel.com>
Signed-off-by: Aditya Swarup <aditya.swarup@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h   |  5 +++++
 drivers/gpu/drm/i915/intel_dram.c | 24 ++++++++++++++++++------
 2 files changed, 23 insertions(+), 6 deletions(-)

Comments

Lucas De Marchi Jan. 27, 2021, 3:07 p.m. UTC | #1 
On Tue, Jan 26, 2021 at 08:11:52PM -0800, Aditya Swarup wrote:
>From: Caz Yokoyama <caz.yokoyama@intel.com>
>
>The crwebview indicates on ADL-S that some of our MCHBAR
>registers have moved from their traditional 0x50XX offsets to
>new locations. The meaning and bit layout of the registers
>remain same.
>
>v2: Simplify logic to a single if else chain and fix indents.(Lucas)
>
>Cc: Lucas De Marchi <lucas.demarchi@intel.com>
>Cc: Jani Nikula <jani.nikula@intel.com>
>Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>Cc: Imre Deak <imre.deak@intel.com>
>Cc: Matt Roper <matthew.d.roper@intel.com>
>Signed-off-by: Caz Yokoyama <caz.yokoyama@intel.com>
>Signed-off-by: Aditya Swarup <aditya.swarup@intel.com>
>---
> drivers/gpu/drm/i915/i915_reg.h   |  5 +++++
> drivers/gpu/drm/i915/intel_dram.c | 24 ++++++++++++++++++------
> 2 files changed, 23 insertions(+), 6 deletions(-)
>
>diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>index aa872446337b..3031897239a0 100644
>--- a/drivers/gpu/drm/i915/i915_reg.h
>+++ b/drivers/gpu/drm/i915/i915_reg.h
>@@ -10916,6 +10916,8 @@ enum skl_power_gate {
> #define  SKL_DRAM_DDR_TYPE_LPDDR3		(2 << 0)
> #define  SKL_DRAM_DDR_TYPE_LPDDR4		(3 << 0)
>
>+#define ADLS_MAD_INTER_CHANNEL_0_0_0_MCHBAR _MMIO(MCHBAR_MIRROR_BASE_SNB + 0x6048)
>+
> #define SKL_MAD_DIMM_CH0_0_0_0_MCHBAR_MCMAIN	_MMIO(MCHBAR_MIRROR_BASE_SNB + 0x500C)
> #define SKL_MAD_DIMM_CH1_0_0_0_MCHBAR_MCMAIN	_MMIO(MCHBAR_MIRROR_BASE_SNB + 0x5010)
> #define  SKL_DRAM_S_SHIFT			16
>@@ -10943,6 +10945,9 @@ enum skl_power_gate {
> #define  CNL_DRAM_RANK_3			(0x2 << 9)
> #define  CNL_DRAM_RANK_4			(0x3 << 9)
>
>+#define ADLS_MAD_DIMM_CH0_0_0_0_MCHBAR		_MMIO(MCHBAR_MIRROR_BASE_SNB + 0x6054)
>+#define ADLS_MAD_DIMM_CH1_0_0_0_MCHBAR		_MMIO(MCHBAR_MIRROR_BASE_SNB + 0x6058)
>+
> /*
>  * Please see hsw_read_dcomp() and hsw_write_dcomp() before using this register,
>  * since on HSW we can't write to it using intel_uncore_write.
>diff --git a/drivers/gpu/drm/i915/intel_dram.c b/drivers/gpu/drm/i915/intel_dram.c
>index 4754296a250e..84f84e118531 100644
>--- a/drivers/gpu/drm/i915/intel_dram.c
>+++ b/drivers/gpu/drm/i915/intel_dram.c
>@@ -181,17 +181,24 @@ skl_dram_get_channels_info(struct drm_i915_private *i915)
> {
> 	struct dram_info *dram_info = &i915->dram_info;
> 	struct dram_channel_info ch0 = {}, ch1 = {};
>+	i915_reg_t ch0_reg, ch1_reg;
> 	u32 val;
> 	int ret;
>
>-	val = intel_uncore_read(&i915->uncore,
>-				SKL_MAD_DIMM_CH0_0_0_0_MCHBAR_MCMAIN);
>+	if (IS_ALDERLAKE_S(i915)) {
>+		ch0_reg = ADLS_MAD_DIMM_CH0_0_0_0_MCHBAR;
>+		ch1_reg = ADLS_MAD_DIMM_CH1_0_0_0_MCHBAR;
>+	} else {
>+		ch0_reg = ADLS_MAD_DIMM_CH0_0_0_0_MCHBAR;
>+		ch1_reg = ADLS_MAD_DIMM_CH1_0_0_0_MCHBAR;

I commented on the wrong version of the patch, but the bug is still
here. And this patch conflict with Jose's patch.

Lucas De Marchi
Souza, Jose Jan. 27, 2021, 4:48 p.m. UTC | #2 
On Wed, 2021-01-27 at 07:07 -0800, Lucas De Marchi wrote:
> On Tue, Jan 26, 2021 at 08:11:52PM -0800, Aditya Swarup wrote:
> > From: Caz Yokoyama <caz.yokoyama@intel.com>
> > 
> > The crwebview indicates on ADL-S that some of our MCHBAR
> > registers have moved from their traditional 0x50XX offsets to
> > new locations. The meaning and bit layout of the registers
> > remain same.
> > 
> > v2: Simplify logic to a single if else chain and fix indents.(Lucas)
> > 
> > Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> > Cc: Jani Nikula <jani.nikula@intel.com>
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Cc: Imre Deak <imre.deak@intel.com>
> > Cc: Matt Roper <matthew.d.roper@intel.com>
> > Signed-off-by: Caz Yokoyama <caz.yokoyama@intel.com>
> > Signed-off-by: Aditya Swarup <aditya.swarup@intel.com>
> > ---
> > drivers/gpu/drm/i915/i915_reg.h   |  5 +++++
> > drivers/gpu/drm/i915/intel_dram.c | 24 ++++++++++++++++++------
> > 2 files changed, 23 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > index aa872446337b..3031897239a0 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -10916,6 +10916,8 @@ enum skl_power_gate {
> > #define  SKL_DRAM_DDR_TYPE_LPDDR3		(2 << 0)
> > #define  SKL_DRAM_DDR_TYPE_LPDDR4		(3 << 0)
> > 
> > +#define ADLS_MAD_INTER_CHANNEL_0_0_0_MCHBAR _MMIO(MCHBAR_MIRROR_BASE_SNB + 0x6048)
> > +
> > #define SKL_MAD_DIMM_CH0_0_0_0_MCHBAR_MCMAIN	_MMIO(MCHBAR_MIRROR_BASE_SNB + 0x500C)
> > #define SKL_MAD_DIMM_CH1_0_0_0_MCHBAR_MCMAIN	_MMIO(MCHBAR_MIRROR_BASE_SNB + 0x5010)
> > #define  SKL_DRAM_S_SHIFT			16
> > @@ -10943,6 +10945,9 @@ enum skl_power_gate {
> > #define  CNL_DRAM_RANK_3			(0x2 << 9)
> > #define  CNL_DRAM_RANK_4			(0x3 << 9)
> > 
> > +#define ADLS_MAD_DIMM_CH0_0_0_0_MCHBAR		_MMIO(MCHBAR_MIRROR_BASE_SNB + 0x6054)
> > +#define ADLS_MAD_DIMM_CH1_0_0_0_MCHBAR		_MMIO(MCHBAR_MIRROR_BASE_SNB + 0x6058)
> > +
> > /*
> >  * Please see hsw_read_dcomp() and hsw_write_dcomp() before using this register,
> >  * since on HSW we can't write to it using intel_uncore_write.
> > diff --git a/drivers/gpu/drm/i915/intel_dram.c b/drivers/gpu/drm/i915/intel_dram.c
> > index 4754296a250e..84f84e118531 100644
> > --- a/drivers/gpu/drm/i915/intel_dram.c
> > +++ b/drivers/gpu/drm/i915/intel_dram.c
> > @@ -181,17 +181,24 @@ skl_dram_get_channels_info(struct drm_i915_private *i915)
> > {
> > 	struct dram_info *dram_info = &i915->dram_info;
> > 	struct dram_channel_info ch0 = {}, ch1 = {};
> > +	i915_reg_t ch0_reg, ch1_reg;
> > 	u32 val;
> > 	int ret;
> > 
> > -	val = intel_uncore_read(&i915->uncore,
> > -				SKL_MAD_DIMM_CH0_0_0_0_MCHBAR_MCMAIN);
> > +	if (IS_ALDERLAKE_S(i915)) {
> > +		ch0_reg = ADLS_MAD_DIMM_CH0_0_0_0_MCHBAR;
> > +		ch1_reg = ADLS_MAD_DIMM_CH1_0_0_0_MCHBAR;
> > +	} else {
> > +		ch0_reg = ADLS_MAD_DIMM_CH0_0_0_0_MCHBAR;
> > +		ch1_reg = ADLS_MAD_DIMM_CH1_0_0_0_MCHBAR;
> 
> I commented on the wrong version of the patch, but the bug is still
> here. And this patch conflict with Jose's patch.

Yep, for GEN12+ we should use PCODE to read DRAM information.
Lucas left some comments, working in the fixes and soon another version will be send.
It already takes care of all GEN12 platforms.

https://patchwork.freedesktop.org/series/86092/

> 
> Lucas De Marchi
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Aditya Swarup Jan. 28, 2021, 5:54 a.m. UTC | #3 
On 1/27/21 8:48 AM, Souza, Jose wrote:
> On Wed, 2021-01-27 at 07:07 -0800, Lucas De Marchi wrote:
>> On Tue, Jan 26, 2021 at 08:11:52PM -0800, Aditya Swarup wrote:
>>> From: Caz Yokoyama <caz.yokoyama@intel.com>
>>>
>>> The crwebview indicates on ADL-S that some of our MCHBAR
>>> registers have moved from their traditional 0x50XX offsets to
>>> new locations. The meaning and bit layout of the registers
>>> remain same.
>>>
>>> v2: Simplify logic to a single if else chain and fix indents.(Lucas)
>>>
>>> Cc: Lucas De Marchi <lucas.demarchi@intel.com>
>>> Cc: Jani Nikula <jani.nikula@intel.com>
>>> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>> Cc: Imre Deak <imre.deak@intel.com>
>>> Cc: Matt Roper <matthew.d.roper@intel.com>
>>> Signed-off-by: Caz Yokoyama <caz.yokoyama@intel.com>
>>> Signed-off-by: Aditya Swarup <aditya.swarup@intel.com>
>>> ---
>>> drivers/gpu/drm/i915/i915_reg.h   |  5 +++++
>>> drivers/gpu/drm/i915/intel_dram.c | 24 ++++++++++++++++++------
>>> 2 files changed, 23 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>>> index aa872446337b..3031897239a0 100644
>>> --- a/drivers/gpu/drm/i915/i915_reg.h
>>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>>> @@ -10916,6 +10916,8 @@ enum skl_power_gate {
>>> #define  SKL_DRAM_DDR_TYPE_LPDDR3		(2 << 0)
>>> #define  SKL_DRAM_DDR_TYPE_LPDDR4		(3 << 0)
>>>
>>> +#define ADLS_MAD_INTER_CHANNEL_0_0_0_MCHBAR _MMIO(MCHBAR_MIRROR_BASE_SNB + 0x6048)
>>> +
>>> #define SKL_MAD_DIMM_CH0_0_0_0_MCHBAR_MCMAIN	_MMIO(MCHBAR_MIRROR_BASE_SNB + 0x500C)
>>> #define SKL_MAD_DIMM_CH1_0_0_0_MCHBAR_MCMAIN	_MMIO(MCHBAR_MIRROR_BASE_SNB + 0x5010)
>>> #define  SKL_DRAM_S_SHIFT			16
>>> @@ -10943,6 +10945,9 @@ enum skl_power_gate {
>>> #define  CNL_DRAM_RANK_3			(0x2 << 9)
>>> #define  CNL_DRAM_RANK_4			(0x3 << 9)
>>>
>>> +#define ADLS_MAD_DIMM_CH0_0_0_0_MCHBAR		_MMIO(MCHBAR_MIRROR_BASE_SNB + 0x6054)
>>> +#define ADLS_MAD_DIMM_CH1_0_0_0_MCHBAR		_MMIO(MCHBAR_MIRROR_BASE_SNB + 0x6058)
>>> +
>>> /*
>>>  * Please see hsw_read_dcomp() and hsw_write_dcomp() before using this register,
>>>  * since on HSW we can't write to it using intel_uncore_write.
>>> diff --git a/drivers/gpu/drm/i915/intel_dram.c b/drivers/gpu/drm/i915/intel_dram.c
>>> index 4754296a250e..84f84e118531 100644
>>> --- a/drivers/gpu/drm/i915/intel_dram.c
>>> +++ b/drivers/gpu/drm/i915/intel_dram.c
>>> @@ -181,17 +181,24 @@ skl_dram_get_channels_info(struct drm_i915_private *i915)
>>> {
>>> 	struct dram_info *dram_info = &i915->dram_info;
>>> 	struct dram_channel_info ch0 = {}, ch1 = {};
>>> +	i915_reg_t ch0_reg, ch1_reg;
>>> 	u32 val;
>>> 	int ret;
>>>
>>> -	val = intel_uncore_read(&i915->uncore,
>>> -				SKL_MAD_DIMM_CH0_0_0_0_MCHBAR_MCMAIN);
>>> +	if (IS_ALDERLAKE_S(i915)) {
>>> +		ch0_reg = ADLS_MAD_DIMM_CH0_0_0_0_MCHBAR;
>>> +		ch1_reg = ADLS_MAD_DIMM_CH1_0_0_0_MCHBAR;
>>> +	} else {
>>> +		ch0_reg = ADLS_MAD_DIMM_CH0_0_0_0_MCHBAR;
>>> +		ch1_reg = ADLS_MAD_DIMM_CH1_0_0_0_MCHBAR;
>>
>> I commented on the wrong version of the patch, but the bug is still
>> here. And this patch conflict with Jose's patch.
> 
> Yep, for GEN12+ we should use PCODE to read DRAM information.
> Lucas left some comments, working in the fixes and soon another version will be send.
> It already takes care of all GEN12 platforms.
> 
> https://patchwork.freedesktop.org/series/86092/

Since I didn't see the removal of code 
skl_dram_get_channels_info/get_dram_type, I have corrected this patch and submitted as part of rev2
just in case to please CI not to report errors with the next revision. Please ignore/drop the MCHBAR patch
if your patch series has been merged.

Aditya 

> 
>>
>> Lucas De Marchi
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index aa872446337b..3031897239a0 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -10916,6 +10916,8 @@  enum skl_power_gate {
 #define  SKL_DRAM_DDR_TYPE_LPDDR3		(2 << 0)
 #define  SKL_DRAM_DDR_TYPE_LPDDR4		(3 << 0)
 
+#define ADLS_MAD_INTER_CHANNEL_0_0_0_MCHBAR _MMIO(MCHBAR_MIRROR_BASE_SNB + 0x6048)
+
 #define SKL_MAD_DIMM_CH0_0_0_0_MCHBAR_MCMAIN	_MMIO(MCHBAR_MIRROR_BASE_SNB + 0x500C)
 #define SKL_MAD_DIMM_CH1_0_0_0_MCHBAR_MCMAIN	_MMIO(MCHBAR_MIRROR_BASE_SNB + 0x5010)
 #define  SKL_DRAM_S_SHIFT			16
@@ -10943,6 +10945,9 @@  enum skl_power_gate {
 #define  CNL_DRAM_RANK_3			(0x2 << 9)
 #define  CNL_DRAM_RANK_4			(0x3 << 9)
 
+#define ADLS_MAD_DIMM_CH0_0_0_0_MCHBAR		_MMIO(MCHBAR_MIRROR_BASE_SNB + 0x6054)
+#define ADLS_MAD_DIMM_CH1_0_0_0_MCHBAR		_MMIO(MCHBAR_MIRROR_BASE_SNB + 0x6058)
+
 /*
  * Please see hsw_read_dcomp() and hsw_write_dcomp() before using this register,
  * since on HSW we can't write to it using intel_uncore_write.
diff --git a/drivers/gpu/drm/i915/intel_dram.c b/drivers/gpu/drm/i915/intel_dram.c
index 4754296a250e..84f84e118531 100644
--- a/drivers/gpu/drm/i915/intel_dram.c
+++ b/drivers/gpu/drm/i915/intel_dram.c
@@ -181,17 +181,24 @@  skl_dram_get_channels_info(struct drm_i915_private *i915)
 {
 	struct dram_info *dram_info = &i915->dram_info;
 	struct dram_channel_info ch0 = {}, ch1 = {};
+	i915_reg_t ch0_reg, ch1_reg;
 	u32 val;
 	int ret;
 
-	val = intel_uncore_read(&i915->uncore,
-				SKL_MAD_DIMM_CH0_0_0_0_MCHBAR_MCMAIN);
+	if (IS_ALDERLAKE_S(i915)) {
+		ch0_reg = ADLS_MAD_DIMM_CH0_0_0_0_MCHBAR;
+		ch1_reg = ADLS_MAD_DIMM_CH1_0_0_0_MCHBAR;
+	} else {
+		ch0_reg = ADLS_MAD_DIMM_CH0_0_0_0_MCHBAR;
+		ch1_reg = ADLS_MAD_DIMM_CH1_0_0_0_MCHBAR;
+	}
+
+	val = intel_uncore_read(&i915->uncore, ch0_reg);
 	ret = skl_dram_get_channel_info(i915, &ch0, 0, val);
 	if (ret == 0)
 		dram_info->num_channels++;
 
-	val = intel_uncore_read(&i915->uncore,
-				SKL_MAD_DIMM_CH1_0_0_0_MCHBAR_MCMAIN);
+	val = intel_uncore_read(&i915->uncore, ch1_reg);
 	ret = skl_dram_get_channel_info(i915, &ch1, 1, val);
 	if (ret == 0)
 		dram_info->num_channels++;
@@ -229,10 +236,15 @@  skl_dram_get_channels_info(struct drm_i915_private *i915)
 static enum intel_dram_type
 skl_get_dram_type(struct drm_i915_private *i915)
 {
+	i915_reg_t reg;
 	u32 val;
 
-	val = intel_uncore_read(&i915->uncore,
-				SKL_MAD_INTER_CHANNEL_0_0_0_MCHBAR_MCMAIN);
+	if (IS_ALDERLAKE_S(i915))
+		reg = ADLS_MAD_INTER_CHANNEL_0_0_0_MCHBAR;
+	else
+		reg = SKL_MAD_INTER_CHANNEL_0_0_0_MCHBAR_MCMAIN;
+
+	val = intel_uncore_read(&i915->uncore, reg);
 
 	switch (val & SKL_DRAM_DDR_TYPE_MASK) {
 	case SKL_DRAM_DDR_TYPE_DDR3: