lkml.org 
[lkml]   [2023]   [Feb]   [2]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v5 3/3] soc: mediatek: mtk-svs: add thermal voltage compensation if needed
Hi Roger,

I have some doubts, please see below.

On 02/02/2023 13:41, Roger Lu wrote:
> Some extreme test environment may keep IC temperature very low or very high
> during system boot stage. For stability concern, we add thermal voltage
> compenstation if needed no matter svs bank phase is in init02 or mon mode.
>
> Signed-off-by: Roger Lu <roger.lu@mediatek.com>
> Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> ---
> drivers/soc/mediatek/mtk-svs.c | 17 +++++++++--------
> 1 file changed, 9 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/soc/mediatek/mtk-svs.c b/drivers/soc/mediatek/mtk-svs.c
> index 299f580847bd..e104866d1ab5 100644
> --- a/drivers/soc/mediatek/mtk-svs.c
> +++ b/drivers/soc/mediatek/mtk-svs.c
> @@ -558,7 +558,7 @@ static int svs_adjust_pm_opp_volts(struct svs_bank *svsb)
> }
>
> /* Get thermal effect */
> - if (svsb->phase == SVSB_PHASE_MON) {
> + if (!IS_ERR_OR_NULL(svsb->tzd)) {
> ret = thermal_zone_get_temp(svsb->tzd, &tzone_temp);
> if (ret || (svsb->temp > SVSB_TEMP_UPPER_BOUND &&
> svsb->temp < SVSB_TEMP_LOWER_BOUND)) {
> @@ -573,7 +573,8 @@ static int svs_adjust_pm_opp_volts(struct svs_bank *svsb)
> temp_voffset += svsb->tzone_ltemp_voffset;
>
> /* 2-line bank update all opp volts when running mon mode */
> - if (svsb->type == SVSB_HIGH || svsb->type == SVSB_LOW) {
> + if (svsb->phase == SVSB_PHASE_MON && (svsb->type == SVSB_HIGH ||
> + svsb->type == SVSB_LOW)) {
> opp_start = 0;
> opp_stop = svsb->opp_count;
> }
> @@ -589,11 +590,6 @@ static int svs_adjust_pm_opp_volts(struct svs_bank *svsb)
> /* do nothing */
> goto unlock_mutex;
> case SVSB_PHASE_INIT02:
> - svsb_volt = max(svsb->volt[i], svsb->vmin);
> - opp_volt = svs_bank_volt_to_opp_volt(svsb_volt,
> - svsb->volt_step,
> - svsb->volt_base);
> - break;
> case SVSB_PHASE_MON:
> svsb_volt = max(svsb->volt[i] + temp_voffset, svsb->vmin);
> opp_volt = svs_bank_volt_to_opp_volt(svsb_volt,
> @@ -1683,7 +1679,7 @@ static int svs_bank_resource_setup(struct svs_platform *svsp)
> }
> }
>
> - if (svsb->mode_support & SVSB_MODE_MON) {
> + if (!IS_ERR_OR_NULL(svsb->tzone_name)) {
> svsb->tzd = thermal_zone_get_zone_by_name(svsb->tzone_name);
> if (IS_ERR(svsb->tzd)) {
> dev_err(svsb->dev, "cannot get \"%s\" thermal zone\n",
> @@ -2127,6 +2123,7 @@ static struct svs_bank svs_mt8192_banks[] = {
> .type = SVSB_LOW,
> .set_freq_pct = svs_set_bank_freq_pct_v3,
> .get_volts = svs_get_bank_volts_v3,
> + .tzone_name = "gpu1",
> .volt_flags = SVSB_REMOVE_DVTFIXED_VOLT,
> .mode_support = SVSB_MODE_INIT02,
> .opp_count = MAX_OPP_ENTRIES,
> @@ -2144,6 +2141,10 @@ static struct svs_bank svs_mt8192_banks[] = {
> .core_sel = 0x0fff0100,
> .int_st = BIT(0),
> .ctl0 = 0x00540003,
> + .tzone_htemp = 85000,
> + .tzone_htemp_voffset = 0,
> + .tzone_ltemp = 25000,
> + .tzone_ltemp_voffset = 7,

Which is the exact same tzone then in the other bank. Which brings me to a good
point:
Is the tzone bank specific or the same for all banks?
At least for mt8192 they are not. I suppose with this change to the code mt8183
could take advantage of this on all it's banks as well. In that case, can we
start to restructure the struct svs_bank to only have the tzone values declared
once?

Background is that I'm very unhappy with the svs_bank data strucutre. It seems
like a "throw it all in here". It should be structured for functional parts of
the banks. Maybe using structs, maybe unions where possible. In any case having
a flat struct of over 50 members isn't really what we want.

Regards,
Matthias

> },
> {
> .sw_id = SVSB_GPU,

\
 
 \ /
  Last update: 2023-03-27 00:07    [W:0.086 / U:1.684 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site