diff mbox series

[RFC,06/19] devfreq: imx8m-ddrc: Add late system sleep PM ops

Message ID 1631554694-9599-7-git-send-email-abel.vesa@nxp.com (mailing list archive)
State New, archived
Headers show
Series Add interconnect and devfreq support for i.MX8MQ | expand

Commit Message

Abel Vesa Sept. 13, 2021, 5:38 p.m. UTC
Seems that, in order to be able to resume from suspend, the dram rate
needs to be the highest one available. Therefore, add the late system
suspend/resume PM ops which set the highest rate on suspend and the
latest one used before suspending on resume.

Signed-off-by: Abel Vesa <abel.vesa@nxp.com>
---
 drivers/devfreq/imx8m-ddrc.c | 28 +++++++++++++++++++++++++++-
 1 file changed, 27 insertions(+), 1 deletion(-)

Comments

Chanwoo Choi Sept. 15, 2021, 3:37 a.m. UTC | #1
Hi,

As I commented on patch5, you keep the OPP list on devicetree file
and then you better to use the 'suspend_opp' property
for setting the highest frequency during suspend/resume.

On 21. 9. 14. 오전 2:38, Abel Vesa wrote:
> Seems that, in order to be able to resume from suspend, the dram rate
> needs to be the highest one available. Therefore, add the late system
> suspend/resume PM ops which set the highest rate on suspend and the
> latest one used before suspending on resume.
> 
> Signed-off-by: Abel Vesa <abel.vesa@nxp.com>
> ---
>   drivers/devfreq/imx8m-ddrc.c | 28 +++++++++++++++++++++++++++-
>   1 file changed, 27 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/devfreq/imx8m-ddrc.c b/drivers/devfreq/imx8m-ddrc.c
> index f18a5c3c1c03..f39741b4a0b0 100644
> --- a/drivers/devfreq/imx8m-ddrc.c
> +++ b/drivers/devfreq/imx8m-ddrc.c
> @@ -72,6 +72,8 @@ struct imx8m_ddrc {
>   	struct clk *dram_alt;
>   	struct clk *dram_apb;
>   
> +	unsigned long suspend_rate;
> +	unsigned long resume_rate;
>   	int freq_count;
>   	struct imx8m_ddrc_freq freq_table[IMX8M_DDRC_MAX_FREQ_COUNT];
>   };
> @@ -271,6 +273,22 @@ static int imx8m_ddrc_target(struct device *dev, unsigned long *freq, u32 flags)
>   	return ret;
>   }
>   
> +static int imx8m_ddrc_suspend(struct device *dev)
> +{
> +	struct imx8m_ddrc *priv = dev_get_drvdata(dev);
> +
> +	priv->resume_rate = clk_get_rate(priv->dram_core);
> +
> +	return imx8m_ddrc_target(dev, &priv->suspend_rate, 0);
> +}
> +
> +static int imx8m_ddrc_resume(struct device *dev)
> +{
> +	struct imx8m_ddrc *priv = dev_get_drvdata(dev);
> +
> +	return imx8m_ddrc_target(dev, &priv->resume_rate, 0);
> +}
> +
>   static int imx8m_ddrc_get_cur_freq(struct device *dev, unsigned long *freq)
>   {
>   	struct imx8m_ddrc *priv = dev_get_drvdata(dev);
> @@ -324,6 +342,9 @@ static int imx8m_ddrc_init_freq_info(struct device *dev)
>   
>   		if (dev_pm_opp_add(dev, freq->rate * 250000, 0))
>   			return -ENODEV;
> +
> +		if (index ==  0)
> +			priv->suspend_rate = freq->rate * 250000;
>   	}
>   
>   	return 0;
> @@ -399,11 +420,16 @@ static const struct of_device_id imx8m_ddrc_of_match[] = {
>   };
>   MODULE_DEVICE_TABLE(of, imx8m_ddrc_of_match);
>   
> +static const struct dev_pm_ops imx8m_ddrc_pm_ops = {
> +	SET_LATE_SYSTEM_SLEEP_PM_OPS(imx8m_ddrc_suspend, imx8m_ddrc_resume)
> +};
> +
>   static struct platform_driver imx8m_ddrc_platdrv = {
>   	.probe		= imx8m_ddrc_probe,
>   	.driver = {
>   		.name	= "imx8m-ddrc-devfreq",
> -		.of_match_table = imx8m_ddrc_of_match,
> +		.pm = &imx8m_ddrc_pm_ops,
> +		.of_match_table = of_match_ptr(imx8m_ddrc_of_match),
>   	},
>   };
>   module_platform_driver(imx8m_ddrc_platdrv);
>
Abel Vesa Oct. 25, 2021, 8:59 p.m. UTC | #2
On 21-09-15 12:37:45, Chanwoo Choi wrote:
> Hi,
> 
> As I commented on patch5, you keep the OPP list on devicetree file
> and then you better to use the 'suspend_opp' property
> for setting the highest frequency during suspend/resume.
> 

Hi,

I think there is no mechanism in place to ensure that the suspend opp
will be set only after all the icc users have suspended. I only tested
briefly, but I can tell you that there are cases where some icc user
asks for a different opp right after the suspend opp was set. This leads
to suspending with a different rate than the one from suspend opp.
So I guess I still need the late system sleep pm opps to circumvent such
situations.

> On 21. 9. 14. 오전 2:38, Abel Vesa wrote:
> > Seems that, in order to be able to resume from suspend, the dram rate
> > needs to be the highest one available. Therefore, add the late system
> > suspend/resume PM ops which set the highest rate on suspend and the
> > latest one used before suspending on resume.
> > 
> > Signed-off-by: Abel Vesa <abel.vesa@nxp.com>
> > ---
> >   drivers/devfreq/imx8m-ddrc.c | 28 +++++++++++++++++++++++++++-
> >   1 file changed, 27 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/devfreq/imx8m-ddrc.c b/drivers/devfreq/imx8m-ddrc.c
> > index f18a5c3c1c03..f39741b4a0b0 100644
> > --- a/drivers/devfreq/imx8m-ddrc.c
> > +++ b/drivers/devfreq/imx8m-ddrc.c
> > @@ -72,6 +72,8 @@ struct imx8m_ddrc {
> >   	struct clk *dram_alt;
> >   	struct clk *dram_apb;
> > +	unsigned long suspend_rate;
> > +	unsigned long resume_rate;
> >   	int freq_count;
> >   	struct imx8m_ddrc_freq freq_table[IMX8M_DDRC_MAX_FREQ_COUNT];
> >   };
> > @@ -271,6 +273,22 @@ static int imx8m_ddrc_target(struct device *dev, unsigned long *freq, u32 flags)
> >   	return ret;
> >   }
> > +static int imx8m_ddrc_suspend(struct device *dev)
> > +{
> > +	struct imx8m_ddrc *priv = dev_get_drvdata(dev);
> > +
> > +	priv->resume_rate = clk_get_rate(priv->dram_core);
> > +
> > +	return imx8m_ddrc_target(dev, &priv->suspend_rate, 0);
> > +}
> > +
> > +static int imx8m_ddrc_resume(struct device *dev)
> > +{
> > +	struct imx8m_ddrc *priv = dev_get_drvdata(dev);
> > +
> > +	return imx8m_ddrc_target(dev, &priv->resume_rate, 0);
> > +}
> > +
> >   static int imx8m_ddrc_get_cur_freq(struct device *dev, unsigned long *freq)
> >   {
> >   	struct imx8m_ddrc *priv = dev_get_drvdata(dev);
> > @@ -324,6 +342,9 @@ static int imx8m_ddrc_init_freq_info(struct device *dev)
> >   		if (dev_pm_opp_add(dev, freq->rate * 250000, 0))
> >   			return -ENODEV;
> > +
> > +		if (index ==  0)
> > +			priv->suspend_rate = freq->rate * 250000;
> >   	}
> >   	return 0;
> > @@ -399,11 +420,16 @@ static const struct of_device_id imx8m_ddrc_of_match[] = {
> >   };
> >   MODULE_DEVICE_TABLE(of, imx8m_ddrc_of_match);
> > +static const struct dev_pm_ops imx8m_ddrc_pm_ops = {
> > +	SET_LATE_SYSTEM_SLEEP_PM_OPS(imx8m_ddrc_suspend, imx8m_ddrc_resume)
> > +};
> > +
> >   static struct platform_driver imx8m_ddrc_platdrv = {
> >   	.probe		= imx8m_ddrc_probe,
> >   	.driver = {
> >   		.name	= "imx8m-ddrc-devfreq",
> > -		.of_match_table = imx8m_ddrc_of_match,
> > +		.pm = &imx8m_ddrc_pm_ops,
> > +		.of_match_table = of_match_ptr(imx8m_ddrc_of_match),
> >   	},
> >   };
> >   module_platform_driver(imx8m_ddrc_platdrv);
> > 
> 
> 
> -- 
> Best Regards,
> Samsung Electronics
> Chanwoo Choi
Martin Kepplinger Nov. 10, 2021, 12:15 p.m. UTC | #3
Am Montag, dem 13.09.2021 um 20:38 +0300 schrieb Abel Vesa:
> Seems that, in order to be able to resume from suspend, the dram rate
> needs to be the highest one available. Therefore, add the late system
> suspend/resume PM ops which set the highest rate on suspend and the
> latest one used before suspending on resume.

Hi Abel, wouldn't this mean that s2idle / freeze would be kind of
broken by this?

Does is make sense to test the lowest rate? How would I force that
here? (just for testing)

Also, you could think about splitting this series up a bit and do this
patch seperately onto mainline (before or after the other work).

thank you
                          martin


> 
> Signed-off-by: Abel Vesa <abel.vesa@nxp.com>
> ---
>  drivers/devfreq/imx8m-ddrc.c | 28 +++++++++++++++++++++++++++-
>  1 file changed, 27 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/devfreq/imx8m-ddrc.c b/drivers/devfreq/imx8m-
> ddrc.c
> index f18a5c3c1c03..f39741b4a0b0 100644
> --- a/drivers/devfreq/imx8m-ddrc.c
> +++ b/drivers/devfreq/imx8m-ddrc.c
> @@ -72,6 +72,8 @@ struct imx8m_ddrc {
>         struct clk *dram_alt;
>         struct clk *dram_apb;
>  
> +       unsigned long suspend_rate;
> +       unsigned long resume_rate;
>         int freq_count;
>         struct imx8m_ddrc_freq freq_table[IMX8M_DDRC_MAX_FREQ_COUNT];
>  };
> @@ -271,6 +273,22 @@ static int imx8m_ddrc_target(struct device *dev,
> unsigned long *freq, u32 flags)
>         return ret;
>  }
>  
> +static int imx8m_ddrc_suspend(struct device *dev)
> +{
> +       struct imx8m_ddrc *priv = dev_get_drvdata(dev);
> +
> +       priv->resume_rate = clk_get_rate(priv->dram_core);
> +
> +       return imx8m_ddrc_target(dev, &priv->suspend_rate, 0);
> +}
> +
> +static int imx8m_ddrc_resume(struct device *dev)
> +{
> +       struct imx8m_ddrc *priv = dev_get_drvdata(dev);
> +
> +       return imx8m_ddrc_target(dev, &priv->resume_rate, 0);
> +}
> +
>  static int imx8m_ddrc_get_cur_freq(struct device *dev, unsigned long
> *freq)
>  {
>         struct imx8m_ddrc *priv = dev_get_drvdata(dev);
> @@ -324,6 +342,9 @@ static int imx8m_ddrc_init_freq_info(struct
> device *dev)
>  
>                 if (dev_pm_opp_add(dev, freq->rate * 250000, 0))
>                         return -ENODEV;
> +
> +               if (index ==  0)
> +                       priv->suspend_rate = freq->rate * 250000;
>         }
>  
>         return 0;
> @@ -399,11 +420,16 @@ static const struct of_device_id
> imx8m_ddrc_of_match[] = {
>  };
>  MODULE_DEVICE_TABLE(of, imx8m_ddrc_of_match);
>  
> +static const struct dev_pm_ops imx8m_ddrc_pm_ops = {
> +       SET_LATE_SYSTEM_SLEEP_PM_OPS(imx8m_ddrc_suspend,
> imx8m_ddrc_resume)
> +};
> +
>  static struct platform_driver imx8m_ddrc_platdrv = {
>         .probe          = imx8m_ddrc_probe,
>         .driver = {
>                 .name   = "imx8m-ddrc-devfreq",
> -               .of_match_table = imx8m_ddrc_of_match,
> +               .pm = &imx8m_ddrc_pm_ops,
> +               .of_match_table = of_match_ptr(imx8m_ddrc_of_match),
>         },
>  };
>  module_platform_driver(imx8m_ddrc_platdrv);
Abel Vesa Nov. 30, 2021, 8:06 p.m. UTC | #4
On 21-11-10 13:15:26, Martin Kepplinger wrote:
> Am Montag, dem 13.09.2021 um 20:38 +0300 schrieb Abel Vesa:
> > Seems that, in order to be able to resume from suspend, the dram rate
> > needs to be the highest one available. Therefore, add the late system
> > suspend/resume PM ops which set the highest rate on suspend and the
> > latest one used before suspending on resume.
> 
> Hi Abel, wouldn't this mean that s2idle / freeze would be kind of
> broken by this?
> 

Nope. Only the DDR rate needs to be raised at 800M before suspending.
Everything else stays the same.

> Does is make sense to test the lowest rate? How would I force that
> here? (just for testing)

You can try, but it will surely freeze. See [1] what you need to change
for testing.
> 
> Also, you could think about splitting this series up a bit and do this
> patch seperately onto mainline (before or after the other work).
> 

Well, I sent as RFC until now. Seems there are no big issues with the
approach. So I'll split the patches between subsystems on the next
iteration.

> thank you
>                           martin
> 
> 
> > 
> > Signed-off-by: Abel Vesa <abel.vesa@nxp.com>
> > ---
> >  drivers/devfreq/imx8m-ddrc.c | 28 +++++++++++++++++++++++++++-
> >  1 file changed, 27 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/devfreq/imx8m-ddrc.c b/drivers/devfreq/imx8m-
> > ddrc.c
> > index f18a5c3c1c03..f39741b4a0b0 100644
> > --- a/drivers/devfreq/imx8m-ddrc.c
> > +++ b/drivers/devfreq/imx8m-ddrc.c
> > @@ -72,6 +72,8 @@ struct imx8m_ddrc {
> >         struct clk *dram_alt;
> >         struct clk *dram_apb;
> >  
> > +       unsigned long suspend_rate;
> > +       unsigned long resume_rate;
> >         int freq_count;
> >         struct imx8m_ddrc_freq freq_table[IMX8M_DDRC_MAX_FREQ_COUNT];
> >  };
> > @@ -271,6 +273,22 @@ static int imx8m_ddrc_target(struct device *dev,
> > unsigned long *freq, u32 flags)
> >         return ret;
> >  }
> >  
> > +static int imx8m_ddrc_suspend(struct device *dev)
> > +{
> > +       struct imx8m_ddrc *priv = dev_get_drvdata(dev);
> > +
> > +       priv->resume_rate = clk_get_rate(priv->dram_core);
> > +
> > +       return imx8m_ddrc_target(dev, &priv->suspend_rate, 0);
> > +}
> > +
> > +static int imx8m_ddrc_resume(struct device *dev)
> > +{
> > +       struct imx8m_ddrc *priv = dev_get_drvdata(dev);
> > +
> > +       return imx8m_ddrc_target(dev, &priv->resume_rate, 0);
> > +}
> > +
> >  static int imx8m_ddrc_get_cur_freq(struct device *dev, unsigned long
> > *freq)
> >  {
> >         struct imx8m_ddrc *priv = dev_get_drvdata(dev);
> > @@ -324,6 +342,9 @@ static int imx8m_ddrc_init_freq_info(struct
> > device *dev)
> >  
> >                 if (dev_pm_opp_add(dev, freq->rate * 250000, 0))
> >                         return -ENODEV;
> > +
> > +               if (index ==  0)

[1] Change this line to:
		    if (index == 1)

It will select the 166935483 freq for suspending.

> > +                       priv->suspend_rate = freq->rate * 250000;
> >         }
> >  
> >         return 0;
> > @@ -399,11 +420,16 @@ static const struct of_device_id
> > imx8m_ddrc_of_match[] = {
> >  };
> >  MODULE_DEVICE_TABLE(of, imx8m_ddrc_of_match);
> >  
> > +static const struct dev_pm_ops imx8m_ddrc_pm_ops = {
> > +       SET_LATE_SYSTEM_SLEEP_PM_OPS(imx8m_ddrc_suspend,
> > imx8m_ddrc_resume)
> > +};
> > +
> >  static struct platform_driver imx8m_ddrc_platdrv = {
> >         .probe          = imx8m_ddrc_probe,
> >         .driver = {
> >                 .name   = "imx8m-ddrc-devfreq",
> > -               .of_match_table = imx8m_ddrc_of_match,
> > +               .pm = &imx8m_ddrc_pm_ops,
> > +               .of_match_table = of_match_ptr(imx8m_ddrc_of_match),
> >         },
> >  };
> >  module_platform_driver(imx8m_ddrc_platdrv);
> 
>
Martin Kepplinger Dec. 1, 2021, 9:35 a.m. UTC | #5
Am Dienstag, dem 30.11.2021 um 22:06 +0200 schrieb Abel Vesa:
> On 21-11-10 13:15:26, Martin Kepplinger wrote:
> > Am Montag, dem 13.09.2021 um 20:38 +0300 schrieb Abel Vesa:
> > > Seems that, in order to be able to resume from suspend, the dram
> > > rate
> > > needs to be the highest one available. Therefore, add the late
> > > system
> > > suspend/resume PM ops which set the highest rate on suspend and
> > > the
> > > latest one used before suspending on resume.
> > 
> > Hi Abel, wouldn't this mean that s2idle / freeze would be kind of
> > broken by this?
> > 
> 
> Nope. Only the DDR rate needs to be raised at 800M before suspending.
> Everything else stays the same.

well, for s2idle, linux stays running, so no calling out to atf (dram
retention...) is happening, so it'll stay at 800M *during* being
suspended.

I tested that by observing power consumption of the system - although
for now without the cpu-sleep state (via your workaround) due to
https://lore.kernel.org/linux-arm-kernel/6ca0bcabfa3b6643f9ab7e311cd8697df223c5cb.camel@puri.sm/


> 
> > Does is make sense to test the lowest rate? How would I force that
> > here? (just for testing)
> 
> You can try, but it will surely freeze. See [1] what you need to
> change
> for testing.

thanks, that looks nicer than forcing 50M.

> > 
> > Also, you could think about splitting this series up a bit and do
> > this
> > patch seperately onto mainline (before or after the other work).
> > 
> 
> Well, I sent as RFC until now. Seems there are no big issues with the
> approach. So I'll split the patches between subsystems on the next
> iteration.

great, looking forward to it!

> 
> > thank you
> >                           martin
> > 
> > 
> > > 
> > > Signed-off-by: Abel Vesa <abel.vesa@nxp.com>
> > > ---
> > >  drivers/devfreq/imx8m-ddrc.c | 28 +++++++++++++++++++++++++++-
> > >  1 file changed, 27 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/devfreq/imx8m-ddrc.c
> > > b/drivers/devfreq/imx8m-
> > > ddrc.c
> > > index f18a5c3c1c03..f39741b4a0b0 100644
> > > --- a/drivers/devfreq/imx8m-ddrc.c
> > > +++ b/drivers/devfreq/imx8m-ddrc.c
> > > @@ -72,6 +72,8 @@ struct imx8m_ddrc {
> > >         struct clk *dram_alt;
> > >         struct clk *dram_apb;
> > >  
> > > +       unsigned long suspend_rate;
> > > +       unsigned long resume_rate;
> > >         int freq_count;
> > >         struct imx8m_ddrc_freq
> > > freq_table[IMX8M_DDRC_MAX_FREQ_COUNT];
> > >  };
> > > @@ -271,6 +273,22 @@ static int imx8m_ddrc_target(struct device
> > > *dev,
> > > unsigned long *freq, u32 flags)
> > >         return ret;
> > >  }
> > >  
> > > +static int imx8m_ddrc_suspend(struct device *dev)
> > > +{
> > > +       struct imx8m_ddrc *priv = dev_get_drvdata(dev);
> > > +
> > > +       priv->resume_rate = clk_get_rate(priv->dram_core);
> > > +
> > > +       return imx8m_ddrc_target(dev, &priv->suspend_rate, 0);
> > > +}
> > > +
> > > +static int imx8m_ddrc_resume(struct device *dev)
> > > +{
> > > +       struct imx8m_ddrc *priv = dev_get_drvdata(dev);
> > > +
> > > +       return imx8m_ddrc_target(dev, &priv->resume_rate, 0);
> > > +}
> > > +
> > >  static int imx8m_ddrc_get_cur_freq(struct device *dev, unsigned
> > > long
> > > *freq)
> > >  {
> > >         struct imx8m_ddrc *priv = dev_get_drvdata(dev);
> > > @@ -324,6 +342,9 @@ static int imx8m_ddrc_init_freq_info(struct
> > > device *dev)
> > >  
> > >                 if (dev_pm_opp_add(dev, freq->rate * 250000, 0))
> > >                         return -ENODEV;
> > > +
> > > +               if (index ==  0)
> 
> [1] Change this line to:
>                     if (index == 1)
> 
> It will select the 166935483 freq for suspending.
> 
> > > +                       priv->suspend_rate = freq->rate * 250000;
> > >         }
> > >  
> > >         return 0;
> > > @@ -399,11 +420,16 @@ static const struct of_device_id
> > > imx8m_ddrc_of_match[] = {
> > >  };
> > >  MODULE_DEVICE_TABLE(of, imx8m_ddrc_of_match);
> > >  
> > > +static const struct dev_pm_ops imx8m_ddrc_pm_ops = {
> > > +       SET_LATE_SYSTEM_SLEEP_PM_OPS(imx8m_ddrc_suspend,
> > > imx8m_ddrc_resume)
> > > +};
> > > +
> > >  static struct platform_driver imx8m_ddrc_platdrv = {
> > >         .probe          = imx8m_ddrc_probe,
> > >         .driver = {
> > >                 .name   = "imx8m-ddrc-devfreq",
> > > -               .of_match_table = imx8m_ddrc_of_match,
> > > +               .pm = &imx8m_ddrc_pm_ops,
> > > +               .of_match_table =
> > > of_match_ptr(imx8m_ddrc_of_match),
> > >         },
> > >  };
> > >  module_platform_driver(imx8m_ddrc_platdrv);
> > 
> >
Martin Kepplinger Dec. 6, 2021, 12:33 p.m. UTC | #6
Am Dienstag, dem 30.11.2021 um 22:06 +0200 schrieb Abel Vesa:
> On 21-11-10 13:15:26, Martin Kepplinger wrote:
> > Am Montag, dem 13.09.2021 um 20:38 +0300 schrieb Abel Vesa:
> > > Seems that, in order to be able to resume from suspend, the dram
> > > rate
> > > needs to be the highest one available. Therefore, add the late
> > > system
> > > suspend/resume PM ops which set the highest rate on suspend and
> > > the
> > > latest one used before suspending on resume.
> > 
> > Hi Abel, wouldn't this mean that s2idle / freeze would be kind of
> > broken by this?
> > 
> 
> Nope. Only the DDR rate needs to be raised at 800M before suspending.
> Everything else stays the same.

fyi I just tested this and you're right. freezes when not at 800M. So
for this patchset, I think this is fine as it enables and fixes stuff.

It would not hurt to mention s2idle at least, where of course 800M
should not be selected, as no userspace is running at all. But I'd be
fine with looking at that later.

> 
> > Does is make sense to test the lowest rate? How would I force that
> > here? (just for testing)
> 
> You can try, but it will surely freeze. See [1] what you need to
> change
> for testing.
> > 
> > Also, you could think about splitting this series up a bit and do
> > this
> > patch seperately onto mainline (before or after the other work).
> > 
> 
> Well, I sent as RFC until now. Seems there are no big issues with the
> approach. So I'll split the patches between subsystems on the next
> iteration.
> 
> > thank you
> >                           martin
> > 
> > 
> > > 
> > > Signed-off-by: Abel Vesa <abel.vesa@nxp.com>
> > > ---
> > >  drivers/devfreq/imx8m-ddrc.c | 28 +++++++++++++++++++++++++++-
> > >  1 file changed, 27 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/devfreq/imx8m-ddrc.c
> > > b/drivers/devfreq/imx8m-
> > > ddrc.c
> > > index f18a5c3c1c03..f39741b4a0b0 100644
> > > --- a/drivers/devfreq/imx8m-ddrc.c
> > > +++ b/drivers/devfreq/imx8m-ddrc.c
> > > @@ -72,6 +72,8 @@ struct imx8m_ddrc {
> > >         struct clk *dram_alt;
> > >         struct clk *dram_apb;
> > >  
> > > +       unsigned long suspend_rate;
> > > +       unsigned long resume_rate;
> > >         int freq_count;
> > >         struct imx8m_ddrc_freq
> > > freq_table[IMX8M_DDRC_MAX_FREQ_COUNT];
> > >  };
> > > @@ -271,6 +273,22 @@ static int imx8m_ddrc_target(struct device
> > > *dev,
> > > unsigned long *freq, u32 flags)
> > >         return ret;
> > >  }
> > >  
> > > +static int imx8m_ddrc_suspend(struct device *dev)
> > > +{
> > > +       struct imx8m_ddrc *priv = dev_get_drvdata(dev);
> > > +
> > > +       priv->resume_rate = clk_get_rate(priv->dram_core);
> > > +
> > > +       return imx8m_ddrc_target(dev, &priv->suspend_rate, 0);
> > > +}
> > > +
> > > +static int imx8m_ddrc_resume(struct device *dev)
> > > +{
> > > +       struct imx8m_ddrc *priv = dev_get_drvdata(dev);
> > > +
> > > +       return imx8m_ddrc_target(dev, &priv->resume_rate, 0);
> > > +}
> > > +
> > >  static int imx8m_ddrc_get_cur_freq(struct device *dev, unsigned
> > > long
> > > *freq)
> > >  {
> > >         struct imx8m_ddrc *priv = dev_get_drvdata(dev);
> > > @@ -324,6 +342,9 @@ static int imx8m_ddrc_init_freq_info(struct
> > > device *dev)
> > >  
> > >                 if (dev_pm_opp_add(dev, freq->rate * 250000, 0))
> > >                         return -ENODEV;
> > > +
> > > +               if (index ==  0)
> 
> [1] Change this line to:
>                     if (index == 1)
> 
> It will select the 166935483 freq for suspending.
> 
> > > +                       priv->suspend_rate = freq->rate * 250000;
> > >         }
> > >  
> > >         return 0;
> > > @@ -399,11 +420,16 @@ static const struct of_device_id
> > > imx8m_ddrc_of_match[] = {
> > >  };
> > >  MODULE_DEVICE_TABLE(of, imx8m_ddrc_of_match);
> > >  
> > > +static const struct dev_pm_ops imx8m_ddrc_pm_ops = {
> > > +       SET_LATE_SYSTEM_SLEEP_PM_OPS(imx8m_ddrc_suspend,
> > > imx8m_ddrc_resume)
> > > +};
> > > +
> > >  static struct platform_driver imx8m_ddrc_platdrv = {
> > >         .probe          = imx8m_ddrc_probe,
> > >         .driver = {
> > >                 .name   = "imx8m-ddrc-devfreq",
> > > -               .of_match_table = imx8m_ddrc_of_match,
> > > +               .pm = &imx8m_ddrc_pm_ops,
> > > +               .of_match_table =
> > > of_match_ptr(imx8m_ddrc_of_match),
> > >         },
> > >  };
> > >  module_platform_driver(imx8m_ddrc_platdrv);
> > 
> >
diff mbox series

Patch

diff --git a/drivers/devfreq/imx8m-ddrc.c b/drivers/devfreq/imx8m-ddrc.c
index f18a5c3c1c03..f39741b4a0b0 100644
--- a/drivers/devfreq/imx8m-ddrc.c
+++ b/drivers/devfreq/imx8m-ddrc.c
@@ -72,6 +72,8 @@  struct imx8m_ddrc {
 	struct clk *dram_alt;
 	struct clk *dram_apb;
 
+	unsigned long suspend_rate;
+	unsigned long resume_rate;
 	int freq_count;
 	struct imx8m_ddrc_freq freq_table[IMX8M_DDRC_MAX_FREQ_COUNT];
 };
@@ -271,6 +273,22 @@  static int imx8m_ddrc_target(struct device *dev, unsigned long *freq, u32 flags)
 	return ret;
 }
 
+static int imx8m_ddrc_suspend(struct device *dev)
+{
+	struct imx8m_ddrc *priv = dev_get_drvdata(dev);
+
+	priv->resume_rate = clk_get_rate(priv->dram_core);
+
+	return imx8m_ddrc_target(dev, &priv->suspend_rate, 0);
+}
+
+static int imx8m_ddrc_resume(struct device *dev)
+{
+	struct imx8m_ddrc *priv = dev_get_drvdata(dev);
+
+	return imx8m_ddrc_target(dev, &priv->resume_rate, 0);
+}
+
 static int imx8m_ddrc_get_cur_freq(struct device *dev, unsigned long *freq)
 {
 	struct imx8m_ddrc *priv = dev_get_drvdata(dev);
@@ -324,6 +342,9 @@  static int imx8m_ddrc_init_freq_info(struct device *dev)
 
 		if (dev_pm_opp_add(dev, freq->rate * 250000, 0))
 			return -ENODEV;
+
+		if (index ==  0)
+			priv->suspend_rate = freq->rate * 250000;
 	}
 
 	return 0;
@@ -399,11 +420,16 @@  static const struct of_device_id imx8m_ddrc_of_match[] = {
 };
 MODULE_DEVICE_TABLE(of, imx8m_ddrc_of_match);
 
+static const struct dev_pm_ops imx8m_ddrc_pm_ops = {
+	SET_LATE_SYSTEM_SLEEP_PM_OPS(imx8m_ddrc_suspend, imx8m_ddrc_resume)
+};
+
 static struct platform_driver imx8m_ddrc_platdrv = {
 	.probe		= imx8m_ddrc_probe,
 	.driver = {
 		.name	= "imx8m-ddrc-devfreq",
-		.of_match_table = imx8m_ddrc_of_match,
+		.pm = &imx8m_ddrc_pm_ops,
+		.of_match_table = of_match_ptr(imx8m_ddrc_of_match),
 	},
 };
 module_platform_driver(imx8m_ddrc_platdrv);