Re: [Intel-gfx] [PATCH 2/9] drm/i915/adl_s: MCHBAR memory info registers are moved

From: Aditya Swarup <aditya.swarup@intel.com>
To: "Souza, Jose" <jose.souza@intel.com>,
	"De Marchi, Lucas" <lucas.demarchi@intel.com>
Cc: "Nikula, Jani" <jani.nikula@intel.com>,
	"intel-gfx@lists.freedesktop.org"
	<intel-gfx@lists.freedesktop.org>
Subject: Re: [Intel-gfx] [PATCH 2/9] drm/i915/adl_s: MCHBAR memory info registers are moved
Date: Wed, 27 Jan 2021 21:54:26 -0800	[thread overview]
Message-ID: <6d535e16-eb6a-8fbf-8c30-72f19db9138d@intel.com> (raw)
In-Reply-To: <df70fa3d1af8bf8479b7baedf6b904d8ad59ff9b.camel@intel.com>

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
> 

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx