linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Borislav Petkov <bp@alien8.de>
To: Yazen Ghannam <yazen.ghannam@amd.com>
Cc: linux-edac@vger.kernel.org, linux-kernel@vger.kernel.org,
	mchehab@kernel.org, tony.luck@intel.com, james.morse@arm.com,
	rric@kernel.org, Smita.KoralahalliChannabasappa@amd.com,
	william.roche@oracle.com
Subject: Re: [PATCH 4/4] EDAC/amd64: Add DDR5 support and related register changes
Date: Fri, 10 Dec 2021 13:41:26 +0100	[thread overview]
Message-ID: <YbNK9jV06al93XDN@zn.tnic> (raw)
In-Reply-To: <20211208174356.1997855-5-yazen.ghannam@amd.com>

On Wed, Dec 08, 2021 at 05:43:56PM +0000, Yazen Ghannam wrote:
> Future AMD systems will support DDR5.
> 
> Add support for changes in register addresses for these systems.
> 
> Introduce a "family flags" bitmask that can be used to indicate any
> special behavior needed on a per-family basis.
> 
> Signed-off-by: Yazen Ghannam <yazen.ghannam@amd.com>
> ---
>  drivers/edac/amd64_edac.c | 61 +++++++++++++++++++++++++++++++++++----
>  drivers/edac/amd64_edac.h | 11 +++++++
>  2 files changed, 66 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/edac/amd64_edac.c b/drivers/edac/amd64_edac.c
> index 1df763128483..e37a8e0cef7e 100644
> --- a/drivers/edac/amd64_edac.c
> +++ b/drivers/edac/amd64_edac.c
> @@ -15,6 +15,36 @@ static struct msr __percpu *msrs;
>  
>  static struct amd64_family_type *fam_type;
>  
> +/* Family flag helpers */
> +static inline bool has_ddr5(void)
> +{
> +	return fam_type->flags.has_ddr5;

A flag about ddr5 *and* a function of the same name. Kinda too much,
don't ya think?

> @@ -1628,6 +1660,17 @@ static void determine_memory_type(struct amd64_pvt *pvt)
>  			dimm_cfg |= pvt->umc[i].dimm_cfg;
>  		}
>  
> +		/* Check if system supports DDR5 and has DDR5 DIMMs in use. */
> +		if (has_ddr5() && (umc_cfg & BIT(0))) {
> +			if (dimm_cfg & BIT(5))
> +				pvt->dram_type = MEM_LRDDR5;
> +			else if (dimm_cfg & BIT(4))
> +				pvt->dram_type = MEM_RDDR5;
> +			else
> +				pvt->dram_type = MEM_DDR5;
> +			return;
> +		}
> +
>  		if (dimm_cfg & BIT(5))
>  			pvt->dram_type = MEM_LRDDR4;
>  		else if (dimm_cfg & BIT(4))
> @@ -2174,8 +2217,13 @@ static int f17_addr_mask_to_cs_size(struct amd64_pvt *pvt, u8 umc,
>  	 * There is one mask per DIMM, and two Chip Selects per DIMM.
>  	 *	CS0 and CS1 -> DIMM0
>  	 *	CS2 and CS3 -> DIMM1
> +	 *
> +	 *	Systems with DDR5 support have one mask per Chip Select.
>  	 */
> -	dimm = csrow_nr >> 1;
> +	if (has_ddr5())
> +		dimm = csrow_nr;
> +	else
> +		dimm = csrow_nr >> 1;
>  
>  	/* Asymmetric dual-rank DIMM support. */
>  	if ((csrow_nr & 1) && (cs_mode & CS_ODD_SECONDARY))
> @@ -2937,6 +2985,7 @@ static struct amd64_family_type family_types[] = {
>  		.f0_id = PCI_DEVICE_ID_AMD_19H_M10H_DF_F0,
>  		.f6_id = PCI_DEVICE_ID_AMD_19H_M10H_DF_F6,
>  		.max_mcs = 12,
> +		.flags.has_ddr5 = 1,

So judging by the name, this means that model 0x10 has DDR5. But I think
you wanna say whether it supports DDR5 or not?

Or does M10 support DDR5 only?

But it doesn't look like it from the comment above:

	"Check if system supports DDR5 and has DDR5 DIMMs in use."

So why is this thing set statically only for this model instead of
detecting from the hw whether there are ddr5 or ddr5 DIMMs and what it
supports?

And then you can use the defines you just added in patch 1.

I'm confused.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

  reply	other threads:[~2021-12-10 12:41 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-12-08 17:43 [PATCH 0/4] AMD Family 19h Models 10h-1Fh Updates Yazen Ghannam
2021-12-08 17:43 ` [PATCH 1/4] EDAC: Add RDDR5 and LRDDR5 memory types Yazen Ghannam
2021-12-08 17:43 ` [PATCH 2/4] EDAC/amd64: Add support for AMD Family 19h Models 10h-1Fh and A0h-AFh Yazen Ghannam
2021-12-08 17:43 ` [PATCH 3/4] EDAC/amd64: Check register values from all UMCs Yazen Ghannam
2021-12-10 12:34   ` Borislav Petkov
2021-12-13 17:24     ` Yazen Ghannam
2021-12-08 17:43 ` [PATCH 4/4] EDAC/amd64: Add DDR5 support and related register changes Yazen Ghannam
2021-12-10 12:41   ` Borislav Petkov [this message]
2021-12-13 17:46     ` Yazen Ghannam
2021-12-14 16:22       ` Borislav Petkov
2021-12-10 12:44 ` [PATCH 0/4] AMD Family 19h Models 10h-1Fh Updates Borislav Petkov
2021-12-13 17:23   ` Yazen Ghannam

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YbNK9jV06al93XDN@zn.tnic \
    --to=bp@alien8.de \
    --cc=Smita.KoralahalliChannabasappa@amd.com \
    --cc=james.morse@arm.com \
    --cc=linux-edac@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mchehab@kernel.org \
    --cc=rric@kernel.org \
    --cc=tony.luck@intel.com \
    --cc=william.roche@oracle.com \
    --cc=yazen.ghannam@amd.com \
    --subject='Re: [PATCH 4/4] EDAC/amd64: Add DDR5 support and related register changes' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).