LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Bjorn Andersson <bjorn.andersson@linaro.org>
To: Stephen Boyd <swboyd@chromium.org>
Cc: Rajendra Nayak <rnayak@codeaurora.org>,
	ulf.hansson@linaro.org, viresh.kumar@linaro.org,
	linux-pm@vger.kernel.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-arm-msm@vger.kernel.org,
	rojay@codeaurora.org, stephan@gerhold.net
Subject: Re: [PATCH v4 2/2] arm64: dts: sc7180: Add required-opps for i2c
Date: Fri, 16 Jul 2021 16:59:07 -0500	[thread overview]
Message-ID: <YPIBK/NJgBNZVI8Y@yoga> (raw)
In-Reply-To: <CAE-0n50qx80cMFPJ1x9rc+EMR1L+j2CUMyDjWAbnE9mPHjf-TQ@mail.gmail.com>

On Fri 16 Jul 16:49 CDT 2021, Stephen Boyd wrote:

> Quoting Bjorn Andersson (2021-07-16 13:52:12)
> > On Fri 16 Jul 15:21 CDT 2021, Stephen Boyd wrote:
> >
> > > Quoting Bjorn Andersson (2021-07-16 13:18:56)
> > > > On Fri 16 Jul 05:00 CDT 2021, Rajendra Nayak wrote:
> > > >
> > > > > qup-i2c devices on sc7180 are clocked with a fixed clock (19.2 MHz)
> > > > > Though qup-i2c does not support DVFS, it still needs to vote for a
> > > > > performance state on 'CX' to satisfy the 19.2 Mhz clock frequency
> > > > > requirement.
> > > > >
> > > >
> > > > Sounds good, but...
> > > >
> > > > > Use 'required-opps' to pass this information from
> > > > > device tree, and also add the power-domains property to specify
> > > > > the CX power-domain.
> > > > >
> > > >
> > > > ..is the required-opps really needed with my rpmhpd patch in place?
> > > >
> > >
> > > Yes? Because rpmhpd_opp_low_svs is not the lowest performance state for
> > > CX.
> >
> > On e.g. sm8250 the first available non-zero corner presented in cmd-db
> > is low_svs.
> 
> Indeed. On sc7180 it's not the first non-zero corner. I suppose
> retention for CX isn't actually used when the SoC is awake so your
> rpmhpd patch is putting in a vote for something that doesn't do anything
> at runtime for CX? I imagine that rpmh only sets the aggregate corner to
> retention when the whole SoC is suspended/sleeping, otherwise things
> wouldn't go very well. Similarly, min_svs may be VDD minimization? If
> so, those first two states are basically states that shouldn't be used
> at runtime, almost like sleep states.
> 

But if that's the case, I don't think it's appropriate for the "enabled
state" of the domain to use any of those corners.

As this means that anyone who needs any of the rpmhpd domains active
also needs to specify required-opps, which wouldn't be needed for any
other power domain provider.

And more importantly it means that a device sitting in a GDSC, which
would be parented by a rpmhpd domain has no way to specify the GDSC and
trickle the minimum-vote up to the rpmhpd domain. (And I know that we
don't describe the parentship of the GDSCs today, but this patch
tells me that it's around the corner - for more than MMCX)

Regards,
Bjorn

> >
> > And if this (which?) clock requires a higher corner than the lowest
> > possible in order to tick at this "lowest" frequency, I'm certainly
> > interested in some more details.
> >

  reply	other threads:[~2021-07-16 21:59 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-16 10:00 [PATCH v4 0/2] PM / Domains: Add support for 'required-opps' to set default perf state Rajendra Nayak
2021-07-16 10:00 ` [PATCH v4 1/2] " Rajendra Nayak
2021-07-16 20:19   ` Stephen Boyd
2021-07-19 11:50     ` Rajendra Nayak
2021-07-16 10:00 ` [PATCH v4 2/2] arm64: dts: sc7180: Add required-opps for i2c Rajendra Nayak
2021-07-16 19:39   ` Stephen Boyd
2021-07-16 20:18   ` Bjorn Andersson
2021-07-16 20:21     ` Stephen Boyd
2021-07-16 20:52       ` Bjorn Andersson
2021-07-16 21:49         ` Stephen Boyd
2021-07-16 21:59           ` Bjorn Andersson [this message]
2021-07-19  9:37             ` Rajendra Nayak
2021-07-19 19:19               ` Bjorn Andersson
2021-07-20  4:29                 ` Rajendra Nayak
2021-07-25 17:01                   ` Bjorn Andersson
2021-07-27  7:35                     ` Rajendra Nayak
2021-07-28  3:46                       ` Bjorn Andersson
2021-07-28 14:01                         ` Dmitry Baryshkov
2021-07-28 18:47                           ` Bjorn Andersson

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=YPIBK/NJgBNZVI8Y@yoga \
    --to=bjorn.andersson@linaro.org \
    --cc=devicetree@vger.kernel.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=rnayak@codeaurora.org \
    --cc=rojay@codeaurora.org \
    --cc=stephan@gerhold.net \
    --cc=swboyd@chromium.org \
    --cc=ulf.hansson@linaro.org \
    --cc=viresh.kumar@linaro.org \
    --subject='Re: [PATCH v4 2/2] arm64: dts: sc7180: Add required-opps for i2c' \
    /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).