Re: [PATCH] ram: stm32mp1: Conditionally enable ASR

[Patrick DELAUNAY <patrick.delaunay@foss.st.com>]

Hi Marek,

on ST platform the ASR/SSR/HSR request are already provided by the DDR 
settings with pwrctl register value

it is managed in TF-A by

arm-trusted-firmware/drivers/st/ddr/stm32mp1_ddr_helpers.c

enumstm32mp1_ddr_sr_mode ddr_read_sr_mode(void)
{
uint32_tpwrctl = mmio_read_32(stm32mp_ddrctrl_base() + DDRCTRL_PWRCTL);
switch(pwrctl & (DDRCTRL_PWRCTL_EN_DFI_DRAM_CLK_DISABLE |
DDRCTRL_PWRCTL_SELFREF_EN)) {
case0U:
returnDDR_SSR_MODE;
caseDDRCTRL_PWRCTL_EN_DFI_DRAM_CLK_DISABLE:
returnDDR_HSR_MODE;
caseDDRCTRL_PWRCTL_EN_DFI_DRAM_CLK_DISABLE | DDRCTRL_PWRCTL_SELFREF_EN:
returnDDR_ASR_MODE;
default:
returnDDR_SR_MODE_INVALID;
}
}

no need to add an other property

I think


On 4/13/22 04:49, Marek Vasut wrote:
> Enable DRAM ASR, auto self-refresh, conditionally, based on DT property
> "st,mem-enable-asr" . While ASR does save considerable amount of power
> at runtime automatically, it also causes LTDC underruns on large panels.
> Let user select whether or not ASR is required or not, generally ASR
> should be enabled on portable and battery operated devices.
>
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Patrick Delaunay <patrick.delaunay@foss.st.com>
> Cc: Patrice Chotard <patrice.chotard@foss.st.com>
> ---
>   drivers/ram/stm32mp1/stm32mp1_ddr.c | 3 ++-
>   drivers/ram/stm32mp1/stm32mp1_ddr.h | 1 +
>   drivers/ram/stm32mp1/stm32mp1_ram.c | 1 +
>   3 files changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/ram/stm32mp1/stm32mp1_ddr.c b/drivers/ram/stm32mp1/stm32mp1_ddr.c
> index 528a171b454..fd11e02aff4 100644
> --- a/drivers/ram/stm32mp1/stm32mp1_ddr.c
> +++ b/drivers/ram/stm32mp1/stm32mp1_ddr.c
> @@ -845,7 +845,8 @@ start:
>   				 config->c_reg.pwrctl);
>   
>   /* Enable auto-self-refresh, which saves a bit of power at runtime. */
> -	stm32mp1_asr_enable(priv);


+ if (config->c_reg.pwrctl & (DDRCTRL_PWRCTL_EN_DFI_DRAM_CLK_DISABLE |

DDRCTRL_PWRCTL_SELFREF_SW) ==

(DDRCTRL_PWRCTL_EN_DFI_DRAM_CLK_DISABLE |DDRCTRL_PWRCTL_SELFREF_SW))

+	stm32mp1_asr_enable(priv);


in DDR setting

#define DDR_PWRCTL 0x00000000

     => SSR

#define DDR_PWRCTL 0x00000028

     => ASR


> +	if (config->info.enable_asr)
> +		stm32mp1_asr_enable(priv);
>   
>   	/* enable uMCTL2 AXI port 0 and 1 */
>   	setbits_le32(&priv->ctl->pctrl_0, DDRCTRL_PCTRL_N_PORT_EN);
> diff --git a/drivers/ram/stm32mp1/stm32mp1_ddr.h b/drivers/ram/stm32mp1/stm32mp1_ddr.h
> index 861efff92be..c74a9cea2cc 100644
> --- a/drivers/ram/stm32mp1/stm32mp1_ddr.h
> +++ b/drivers/ram/stm32mp1/stm32mp1_ddr.h
> @@ -144,6 +144,7 @@ struct stm32mp1_ddr_info {
>   	const char *name;
>   	u32 speed; /* in kHZ */
>   	u32 size;  /* memory size in byte = col * row * width */
> +	bool enable_asr;
>   };
>   
>   struct stm32mp1_ddr_config {
> diff --git a/drivers/ram/stm32mp1/stm32mp1_ram.c b/drivers/ram/stm32mp1/stm32mp1_ram.c
> index 49b1262461b..f39cfad4764 100644
> --- a/drivers/ram/stm32mp1/stm32mp1_ram.c
> +++ b/drivers/ram/stm32mp1/stm32mp1_ram.c
> @@ -122,6 +122,7 @@ static int stm32mp1_ddr_setup(struct udevice *dev)
>   	config.info.speed = ofnode_read_u32_default(node, "st,mem-speed", 0);
>   	config.info.size = ofnode_read_u32_default(node, "st,mem-size", 0);
>   	config.info.name = ofnode_read_string(node, "st,mem-name");
> +	config.info.enable_asr = ofnode_read_bool(node, "st,mem-enable-asr");
>   	if (!config.info.name) {
>   		dev_dbg(dev, "no st,mem-name\n");
>   		return -EINVAL;


Regards


Patrick