Re: [PATCH] drm/msm: Disable frequency clamping on a630
On 9/10/2021 11:04 PM, Caleb Connolly wrote:
On 10/09/2021 18:18, Rob Clark wrote:
On Tue, Sep 7, 2021 at 7:20 PM Bjorn Andersson
wrote:
On Mon 09 Aug 10:26 PDT 2021, Akhil P Oommen wrote:
On 8/9/2021 9:48 PM, Caleb Connolly wrote:
On 09/08/2021 17:12, Rob Clark wrote:
On Mon, Aug 9, 2021 at 7:52 AM Akhil P Oommen
wrote:
[..]
I am a bit confused. We don't define a power domain for gpu in dt,
correct? Then what exactly set_opp do here? Do you think this
usleep is
what is helping here somehow to mask the issue?
The power domains (for cx and gx) are defined in the GMU DT, the
OPPs in
the GPU DT. For the sake of simplicity I'll refer to the lowest
frequency (25700) and OPP level (RPMH_REGULATOR_LEVEL_LOW_SVS) as
the "min" state, and the highest frequency (71000) and OPP level
(RPMH_REGULATOR_LEVEL_TURBO_L1) as the "max" state. These are
defined in
sdm845.dtsi under the gpu node.
The new devfreq behaviour unmasks what I think is a driver bug, it
inadvertently puts much more strain on the GPU regulators than they
usually get. With the new behaviour the GPU jumps from it's min
state to
the max state and back again extremely rapidly under workloads as
small
as refreshing UI. Where previously the GPU would rarely if ever go
above
342MHz when interacting with the device, it now jumps between min and
max many times per second.
If my understanding is correct, the current implementation of the GMU
set freq is the following:
- Get OPP for frequency to set
- Push the frequency to the GMU - immediately updating the core
clock
- Call dev_pm_opp_set_opp() which triggers a notify chain, this
winds
up somewhere in power management code and causes the gx regulator
level
to be updated
Nope. dev_pm_opp_set_opp() sets the bandwidth for gpu and nothing
else. We
were using a different api earlier which got deprecated -
dev_pm_opp_set_bw().
On the Lenovo Yoga C630 this is reproduced by starting alacritty and if
I'm lucky I managed to hit a few keys before it crashes, so I spent a
few hours looking into this as well...
As you say, the dev_pm_opp_set_opp() will only cast a interconnect vote.
The opp-level is just there for show and isn't used by anything, at
least not on 845.
Further more, I'm missing something in my tree, so the interconnect
doesn't hit sync_state, and as such we're not actually scaling the
buses. So the problem is not that Linux doesn't turn on the buses in
time.
So I suspect that the "AHB bus error" isn't saying that we turned off
the bus, but rather that the GPU becomes unstable or something of that
sort.
Lastly, I reverted 9bc95570175a ("drm/msm: Devfreq tuning") and ran
Aquarium for 20 minutes without a problem. I then switched the gpu
devfreq governor to "userspace" and ran the following:
while true; do
echo 25700 > /sys/class/devfreq/500.gpu/userspace/set_freq
echo 71000 > /sys/class/devfreq/500.gpu/userspace/set_freq
done
It took 19 iterations of this loop to crash the GPU.
I assume you still had aquarium running, to keep the gpu awake while
you ran that loop?
Fwiw, I modified this slightly to match sc7180's min/max gpu freq and
could not trigger any issue.. interestingly sc7180 has a lower min
freq (180) and higher max freq (800) so it was toggling over a wider
freq range. I also tried on a device that had the higher 825MHz opp
(since I noticed that was the only opp that used
RPMH_REGULATOR_LEVEL_TURBO_L1 and wanted to rule that out), but could
not reproduce.
I guess a630 (sdm845) should have higher power draw (it is 2x # of
shader cores and 2x GMEM size, but lower max freq).. the question is,
is this the reason we see this on sdm845 and not sc7180? Or is there
some other difference. On the gpu side of this, they are both closely
related (ie. the same "sub-generation" of a6xx, same gmu fw, etc)..
I'm less sure about the other parts (icc, rpmh, etc)
My guess would be power draw, nobody has mentioned this yet but I've
realised that the vdd_gfx rail is powered by a buck converter, which
could explain a lot of the symptoms.
Buck converters depend on high frequency switching and inductors to
work, this inherently leads to some lag time when changing voltages, and
also means that the behaviour of the regulator is defined in part by how
much current is being drawn. Wikipedia has a pretty good explanation:
https://en.wikipedia.org/wiki/Buck_converter
At the best of times these regulators have a known voltage ripple, when
under load and when rapidly switching voltages this will get a lot worse.
Someone with an oscilloscope and schematics could probe the rail and
probably see exactly what's going on when the GPU crashes. Because of
the lag time in the regulator changing voltage, it might be
undershooting whilst the GPU is trying to clock up and draw more current
- causing instability and crashes.
Both of you are correct. The GPU is very similar including the GMU (we
have same fw for both), except the
Re: [PATCH] drm/msm: Disable frequency clamping on a630
On 10/09/2021 18:18, Rob Clark wrote:
On Tue, Sep 7, 2021 at 7:20 PM Bjorn Andersson
wrote:
On Mon 09 Aug 10:26 PDT 2021, Akhil P Oommen wrote:
On 8/9/2021 9:48 PM, Caleb Connolly wrote:
On 09/08/2021 17:12, Rob Clark wrote:
On Mon, Aug 9, 2021 at 7:52 AM Akhil P Oommen
wrote:
[..]
I am a bit confused. We don't define a power domain for gpu in dt,
correct? Then what exactly set_opp do here? Do you think this usleep is
what is helping here somehow to mask the issue?
The power domains (for cx and gx) are defined in the GMU DT, the OPPs in
the GPU DT. For the sake of simplicity I'll refer to the lowest
frequency (25700) and OPP level (RPMH_REGULATOR_LEVEL_LOW_SVS) as
the "min" state, and the highest frequency (71000) and OPP level
(RPMH_REGULATOR_LEVEL_TURBO_L1) as the "max" state. These are defined in
sdm845.dtsi under the gpu node.
The new devfreq behaviour unmasks what I think is a driver bug, it
inadvertently puts much more strain on the GPU regulators than they
usually get. With the new behaviour the GPU jumps from it's min state to
the max state and back again extremely rapidly under workloads as small
as refreshing UI. Where previously the GPU would rarely if ever go above
342MHz when interacting with the device, it now jumps between min and
max many times per second.
If my understanding is correct, the current implementation of the GMU
set freq is the following:
- Get OPP for frequency to set
- Push the frequency to the GMU - immediately updating the core clock
- Call dev_pm_opp_set_opp() which triggers a notify chain, this winds
up somewhere in power management code and causes the gx regulator level
to be updated
Nope. dev_pm_opp_set_opp() sets the bandwidth for gpu and nothing else. We
were using a different api earlier which got deprecated -
dev_pm_opp_set_bw().
On the Lenovo Yoga C630 this is reproduced by starting alacritty and if
I'm lucky I managed to hit a few keys before it crashes, so I spent a
few hours looking into this as well...
As you say, the dev_pm_opp_set_opp() will only cast a interconnect vote.
The opp-level is just there for show and isn't used by anything, at
least not on 845.
Further more, I'm missing something in my tree, so the interconnect
doesn't hit sync_state, and as such we're not actually scaling the
buses. So the problem is not that Linux doesn't turn on the buses in
time.
So I suspect that the "AHB bus error" isn't saying that we turned off
the bus, but rather that the GPU becomes unstable or something of that
sort.
Lastly, I reverted 9bc95570175a ("drm/msm: Devfreq tuning") and ran
Aquarium for 20 minutes without a problem. I then switched the gpu
devfreq governor to "userspace" and ran the following:
while true; do
echo 25700 > /sys/class/devfreq/500.gpu/userspace/set_freq
echo 71000 > /sys/class/devfreq/500.gpu/userspace/set_freq
done
It took 19 iterations of this loop to crash the GPU.
I assume you still had aquarium running, to keep the gpu awake while
you ran that loop?
Fwiw, I modified this slightly to match sc7180's min/max gpu freq and
could not trigger any issue.. interestingly sc7180 has a lower min
freq (180) and higher max freq (800) so it was toggling over a wider
freq range. I also tried on a device that had the higher 825MHz opp
(since I noticed that was the only opp that used
RPMH_REGULATOR_LEVEL_TURBO_L1 and wanted to rule that out), but could
not reproduce.
I guess a630 (sdm845) should have higher power draw (it is 2x # of
shader cores and 2x GMEM size, but lower max freq).. the question is,
is this the reason we see this on sdm845 and not sc7180? Or is there
some other difference. On the gpu side of this, they are both closely
related (ie. the same "sub-generation" of a6xx, same gmu fw, etc)..
I'm less sure about the other parts (icc, rpmh, etc)
My guess would be power draw, nobody has mentioned this yet but I've realised that the vdd_gfx rail is powered by a buck
converter, which could explain a lot of the symptoms.
Buck converters depend on high frequency switching and inductors to work, this inherently leads to some lag time when
changing voltages, and also means that the behaviour of the regulator is defined in part by how much current is being
drawn. Wikipedia has a pretty good explanation: https://en.wikipedia.org/wiki/Buck_converter
At the best of times these regulators have a known voltage ripple, when under load and when rapidly switching voltages
this will get a lot worse.
Someone with an oscilloscope and schematics could probe the rail and probably see exactly what's going on when the GPU
crashes. Because of the lag time in the regulator changing voltage, it might be undershooting whilst the GPU is trying
to clock up and draw more current - causing instability and crashes.
BR,
-R
So the problem doesn't seem to be Rob's change, it's just that prior to
it the chance to hitting it is way lower. Question is still what it is
that we're triggering.
Re: [PATCH] drm/msm: Disable frequency clamping on a630
On Thu, Sep 9, 2021 at 1:54 PM Rob Clark wrote:
>
> On Thu, Sep 9, 2021 at 12:50 PM Akhil P Oommen wrote:
> >
> > On 9/9/2021 9:42 PM, Amit Pundir wrote:
> > > On Thu, 9 Sept 2021 at 17:47, Amit Pundir wrote:
> > >>
> > >> On Wed, 8 Sept 2021 at 07:50, Bjorn Andersson
> > >> wrote:
> > >>>
> > >>> On Mon 09 Aug 10:26 PDT 2021, Akhil P Oommen wrote:
> > >>>
> > On 8/9/2021 9:48 PM, Caleb Connolly wrote:
> > >
> > >
> > > On 09/08/2021 17:12, Rob Clark wrote:
> > >> On Mon, Aug 9, 2021 at 7:52 AM Akhil P Oommen
> > >> wrote:
> > >>> [..]
> > >>> I am a bit confused. We don't define a power domain for gpu in dt,
> > >>> correct? Then what exactly set_opp do here? Do you think this
> > >>> usleep is
> > >>> what is helping here somehow to mask the issue?
> > > The power domains (for cx and gx) are defined in the GMU DT, the OPPs
> > > in
> > > the GPU DT. For the sake of simplicity I'll refer to the lowest
> > > frequency (25700) and OPP level (RPMH_REGULATOR_LEVEL_LOW_SVS) as
> > > the "min" state, and the highest frequency (71000) and OPP level
> > > (RPMH_REGULATOR_LEVEL_TURBO_L1) as the "max" state. These are defined
> > > in
> > > sdm845.dtsi under the gpu node.
> > >
> > > The new devfreq behaviour unmasks what I think is a driver bug, it
> > > inadvertently puts much more strain on the GPU regulators than they
> > > usually get. With the new behaviour the GPU jumps from it's min state
> > > to
> > > the max state and back again extremely rapidly under workloads as
> > > small
> > > as refreshing UI. Where previously the GPU would rarely if ever go
> > > above
> > > 342MHz when interacting with the device, it now jumps between min and
> > > max many times per second.
> > >
> > > If my understanding is correct, the current implementation of the GMU
> > > set freq is the following:
> > >- Get OPP for frequency to set
> > >- Push the frequency to the GMU - immediately updating the core
> > > clock
> > >- Call dev_pm_opp_set_opp() which triggers a notify chain, this
> > > winds
> > > up somewhere in power management code and causes the gx regulator
> > > level
> > > to be updated
> >
> > Nope. dev_pm_opp_set_opp() sets the bandwidth for gpu and nothing
> > else. We
> > were using a different api earlier which got deprecated -
> > dev_pm_opp_set_bw().
> >
> > >>>
> > >>> On the Lenovo Yoga C630 this is reproduced by starting alacritty and if
> > >>> I'm lucky I managed to hit a few keys before it crashes, so I spent a
> > >>> few hours looking into this as well...
> > >>>
> > >>> As you say, the dev_pm_opp_set_opp() will only cast a interconnect vote.
> > >>> The opp-level is just there for show and isn't used by anything, at
> > >>> least not on 845.
> > >>>
> > >>> Further more, I'm missing something in my tree, so the interconnect
> > >>> doesn't hit sync_state, and as such we're not actually scaling the
> > >>> buses. So the problem is not that Linux doesn't turn on the buses in
> > >>> time.
> > >>>
> > >>> So I suspect that the "AHB bus error" isn't saying that we turned off
> > >>> the bus, but rather that the GPU becomes unstable or something of that
> > >>> sort.
> > >>>
> > >>>
> > >>> Lastly, I reverted 9bc95570175a ("drm/msm: Devfreq tuning") and ran
> > >>> Aquarium for 20 minutes without a problem. I then switched the gpu
> > >>> devfreq governor to "userspace" and ran the following:
> > >>>
> > >>> while true; do
> > >>>echo 25700 > /sys/class/devfreq/500.gpu/userspace/set_freq
> > >>>echo 71000 > /sys/class/devfreq/500.gpu/userspace/set_freq
> > >>> done
> > >>>
> > >>> It took 19 iterations of this loop to crash the GPU.
> > >>
> > >> Ack. With your above script, I can reproduce a crash too on db845c
> > >> (A630) running v5.14. I didn't get any crash log though and device
> > >> just rebooted to USB crash mode.
> > >>
> > >> And same crash on RB5 (A650) too https://hastebin.com/raw/ejutetuwun
> >
> > Are we sure this is the same issue? It could be, but I thought we were
> > seeing a bunch of random gpu errors (which may eventually hit device crash).
>
> In the sense that async-serror often seems to be a clk issue, it
> *could* be related.. but this would have to be triggered by CPU
> access. The symptom does seem very different.
>
The more I think about it, the more I think this is a different
issue.. a650 is somewhat different wrt gmu (ie. hfi vs legacy code
paths).
Amit, could you try the same experiment (with 9bc95570175a ("drm/msm:
Devfreq tuning") revert) while running something like webgl aquarium
to prevent the GPU from suspending? I'm kinda suspecting the issue
you hit is more likely some suspend/resume issue.
BR,
-R
Re: [PATCH] drm/msm: Disable frequency clamping on a630
On Tue, Sep 7, 2021 at 7:20 PM Bjorn Andersson
wrote:
>
> On Mon 09 Aug 10:26 PDT 2021, Akhil P Oommen wrote:
>
> > On 8/9/2021 9:48 PM, Caleb Connolly wrote:
> > >
> > >
> > > On 09/08/2021 17:12, Rob Clark wrote:
> > > > On Mon, Aug 9, 2021 at 7:52 AM Akhil P Oommen
> > > > wrote:
> [..]
> > > > > I am a bit confused. We don't define a power domain for gpu in dt,
> > > > > correct? Then what exactly set_opp do here? Do you think this usleep
> > > > > is
> > > > > what is helping here somehow to mask the issue?
> > > The power domains (for cx and gx) are defined in the GMU DT, the OPPs in
> > > the GPU DT. For the sake of simplicity I'll refer to the lowest
> > > frequency (25700) and OPP level (RPMH_REGULATOR_LEVEL_LOW_SVS) as
> > > the "min" state, and the highest frequency (71000) and OPP level
> > > (RPMH_REGULATOR_LEVEL_TURBO_L1) as the "max" state. These are defined in
> > > sdm845.dtsi under the gpu node.
> > >
> > > The new devfreq behaviour unmasks what I think is a driver bug, it
> > > inadvertently puts much more strain on the GPU regulators than they
> > > usually get. With the new behaviour the GPU jumps from it's min state to
> > > the max state and back again extremely rapidly under workloads as small
> > > as refreshing UI. Where previously the GPU would rarely if ever go above
> > > 342MHz when interacting with the device, it now jumps between min and
> > > max many times per second.
> > >
> > > If my understanding is correct, the current implementation of the GMU
> > > set freq is the following:
> > > - Get OPP for frequency to set
> > > - Push the frequency to the GMU - immediately updating the core clock
> > > - Call dev_pm_opp_set_opp() which triggers a notify chain, this winds
> > > up somewhere in power management code and causes the gx regulator level
> > > to be updated
> >
> > Nope. dev_pm_opp_set_opp() sets the bandwidth for gpu and nothing else. We
> > were using a different api earlier which got deprecated -
> > dev_pm_opp_set_bw().
> >
>
> On the Lenovo Yoga C630 this is reproduced by starting alacritty and if
> I'm lucky I managed to hit a few keys before it crashes, so I spent a
> few hours looking into this as well...
>
> As you say, the dev_pm_opp_set_opp() will only cast a interconnect vote.
> The opp-level is just there for show and isn't used by anything, at
> least not on 845.
>
> Further more, I'm missing something in my tree, so the interconnect
> doesn't hit sync_state, and as such we're not actually scaling the
> buses. So the problem is not that Linux doesn't turn on the buses in
> time.
>
> So I suspect that the "AHB bus error" isn't saying that we turned off
> the bus, but rather that the GPU becomes unstable or something of that
> sort.
>
>
> Lastly, I reverted 9bc95570175a ("drm/msm: Devfreq tuning") and ran
> Aquarium for 20 minutes without a problem. I then switched the gpu
> devfreq governor to "userspace" and ran the following:
>
> while true; do
> echo 25700 > /sys/class/devfreq/500.gpu/userspace/set_freq
> echo 71000 > /sys/class/devfreq/500.gpu/userspace/set_freq
> done
>
> It took 19 iterations of this loop to crash the GPU.
I assume you still had aquarium running, to keep the gpu awake while
you ran that loop?
Fwiw, I modified this slightly to match sc7180's min/max gpu freq and
could not trigger any issue.. interestingly sc7180 has a lower min
freq (180) and higher max freq (800) so it was toggling over a wider
freq range. I also tried on a device that had the higher 825MHz opp
(since I noticed that was the only opp that used
RPMH_REGULATOR_LEVEL_TURBO_L1 and wanted to rule that out), but could
not reproduce.
I guess a630 (sdm845) should have higher power draw (it is 2x # of
shader cores and 2x GMEM size, but lower max freq).. the question is,
is this the reason we see this on sdm845 and not sc7180? Or is there
some other difference. On the gpu side of this, they are both closely
related (ie. the same "sub-generation" of a6xx, same gmu fw, etc)..
I'm less sure about the other parts (icc, rpmh, etc)
BR,
-R
> So the problem doesn't seem to be Rob's change, it's just that prior to
> it the chance to hitting it is way lower. Question is still what it is
> that we're triggering.
>
> Regards,
> Bjorn
Re: [PATCH] drm/msm: Disable frequency clamping on a630
On Thu, Sep 9, 2021 at 12:50 PM Akhil P Oommen wrote:
>
> On 9/9/2021 9:42 PM, Amit Pundir wrote:
> > On Thu, 9 Sept 2021 at 17:47, Amit Pundir wrote:
> >>
> >> On Wed, 8 Sept 2021 at 07:50, Bjorn Andersson
> >> wrote:
> >>>
> >>> On Mon 09 Aug 10:26 PDT 2021, Akhil P Oommen wrote:
> >>>
> On 8/9/2021 9:48 PM, Caleb Connolly wrote:
> >
> >
> > On 09/08/2021 17:12, Rob Clark wrote:
> >> On Mon, Aug 9, 2021 at 7:52 AM Akhil P Oommen
> >> wrote:
> >>> [..]
> >>> I am a bit confused. We don't define a power domain for gpu in dt,
> >>> correct? Then what exactly set_opp do here? Do you think this usleep
> >>> is
> >>> what is helping here somehow to mask the issue?
> > The power domains (for cx and gx) are defined in the GMU DT, the OPPs in
> > the GPU DT. For the sake of simplicity I'll refer to the lowest
> > frequency (25700) and OPP level (RPMH_REGULATOR_LEVEL_LOW_SVS) as
> > the "min" state, and the highest frequency (71000) and OPP level
> > (RPMH_REGULATOR_LEVEL_TURBO_L1) as the "max" state. These are defined in
> > sdm845.dtsi under the gpu node.
> >
> > The new devfreq behaviour unmasks what I think is a driver bug, it
> > inadvertently puts much more strain on the GPU regulators than they
> > usually get. With the new behaviour the GPU jumps from it's min state to
> > the max state and back again extremely rapidly under workloads as small
> > as refreshing UI. Where previously the GPU would rarely if ever go above
> > 342MHz when interacting with the device, it now jumps between min and
> > max many times per second.
> >
> > If my understanding is correct, the current implementation of the GMU
> > set freq is the following:
> >- Get OPP for frequency to set
> >- Push the frequency to the GMU - immediately updating the core clock
> >- Call dev_pm_opp_set_opp() which triggers a notify chain, this winds
> > up somewhere in power management code and causes the gx regulator level
> > to be updated
>
> Nope. dev_pm_opp_set_opp() sets the bandwidth for gpu and nothing else.
> We
> were using a different api earlier which got deprecated -
> dev_pm_opp_set_bw().
>
> >>>
> >>> On the Lenovo Yoga C630 this is reproduced by starting alacritty and if
> >>> I'm lucky I managed to hit a few keys before it crashes, so I spent a
> >>> few hours looking into this as well...
> >>>
> >>> As you say, the dev_pm_opp_set_opp() will only cast a interconnect vote.
> >>> The opp-level is just there for show and isn't used by anything, at
> >>> least not on 845.
> >>>
> >>> Further more, I'm missing something in my tree, so the interconnect
> >>> doesn't hit sync_state, and as such we're not actually scaling the
> >>> buses. So the problem is not that Linux doesn't turn on the buses in
> >>> time.
> >>>
> >>> So I suspect that the "AHB bus error" isn't saying that we turned off
> >>> the bus, but rather that the GPU becomes unstable or something of that
> >>> sort.
> >>>
> >>>
> >>> Lastly, I reverted 9bc95570175a ("drm/msm: Devfreq tuning") and ran
> >>> Aquarium for 20 minutes without a problem. I then switched the gpu
> >>> devfreq governor to "userspace" and ran the following:
> >>>
> >>> while true; do
> >>>echo 25700 > /sys/class/devfreq/500.gpu/userspace/set_freq
> >>>echo 71000 > /sys/class/devfreq/500.gpu/userspace/set_freq
> >>> done
> >>>
> >>> It took 19 iterations of this loop to crash the GPU.
> >>
> >> Ack. With your above script, I can reproduce a crash too on db845c
> >> (A630) running v5.14. I didn't get any crash log though and device
> >> just rebooted to USB crash mode.
> >>
> >> And same crash on RB5 (A650) too https://hastebin.com/raw/ejutetuwun
>
> Are we sure this is the same issue? It could be, but I thought we were
> seeing a bunch of random gpu errors (which may eventually hit device crash).
In the sense that async-serror often seems to be a clk issue, it
*could* be related.. but this would have to be triggered by CPU
access. The symptom does seem very different.
BR,
-R
> -Akhil
>
> >
> > fwiw I can't reproduce this crash on RB5 so far with v5.15-rc1 merge
> > window (HEAD: 477f70cd2a67)
> >
> >>
> >>>
> >>> So the problem doesn't seem to be Rob's change, it's just that prior to
> >>> it the chance to hitting it is way lower. Question is still what it is
> >>> that we're triggering.
> >>>
> >>> Regards,
> >>> Bjorn
>
Re: [PATCH] drm/msm: Disable frequency clamping on a630
On 9/9/2021 9:42 PM, Amit Pundir wrote:
On Thu, 9 Sept 2021 at 17:47, Amit Pundir wrote:
On Wed, 8 Sept 2021 at 07:50, Bjorn Andersson
wrote:
On Mon 09 Aug 10:26 PDT 2021, Akhil P Oommen wrote:
On 8/9/2021 9:48 PM, Caleb Connolly wrote:
On 09/08/2021 17:12, Rob Clark wrote:
On Mon, Aug 9, 2021 at 7:52 AM Akhil P Oommen
wrote:
[..]
I am a bit confused. We don't define a power domain for gpu in dt,
correct? Then what exactly set_opp do here? Do you think this usleep is
what is helping here somehow to mask the issue?
The power domains (for cx and gx) are defined in the GMU DT, the OPPs in
the GPU DT. For the sake of simplicity I'll refer to the lowest
frequency (25700) and OPP level (RPMH_REGULATOR_LEVEL_LOW_SVS) as
the "min" state, and the highest frequency (71000) and OPP level
(RPMH_REGULATOR_LEVEL_TURBO_L1) as the "max" state. These are defined in
sdm845.dtsi under the gpu node.
The new devfreq behaviour unmasks what I think is a driver bug, it
inadvertently puts much more strain on the GPU regulators than they
usually get. With the new behaviour the GPU jumps from it's min state to
the max state and back again extremely rapidly under workloads as small
as refreshing UI. Where previously the GPU would rarely if ever go above
342MHz when interacting with the device, it now jumps between min and
max many times per second.
If my understanding is correct, the current implementation of the GMU
set freq is the following:
- Get OPP for frequency to set
- Push the frequency to the GMU - immediately updating the core clock
- Call dev_pm_opp_set_opp() which triggers a notify chain, this winds
up somewhere in power management code and causes the gx regulator level
to be updated
Nope. dev_pm_opp_set_opp() sets the bandwidth for gpu and nothing else. We
were using a different api earlier which got deprecated -
dev_pm_opp_set_bw().
On the Lenovo Yoga C630 this is reproduced by starting alacritty and if
I'm lucky I managed to hit a few keys before it crashes, so I spent a
few hours looking into this as well...
As you say, the dev_pm_opp_set_opp() will only cast a interconnect vote.
The opp-level is just there for show and isn't used by anything, at
least not on 845.
Further more, I'm missing something in my tree, so the interconnect
doesn't hit sync_state, and as such we're not actually scaling the
buses. So the problem is not that Linux doesn't turn on the buses in
time.
So I suspect that the "AHB bus error" isn't saying that we turned off
the bus, but rather that the GPU becomes unstable or something of that
sort.
Lastly, I reverted 9bc95570175a ("drm/msm: Devfreq tuning") and ran
Aquarium for 20 minutes without a problem. I then switched the gpu
devfreq governor to "userspace" and ran the following:
while true; do
echo 25700 > /sys/class/devfreq/500.gpu/userspace/set_freq
echo 71000 > /sys/class/devfreq/500.gpu/userspace/set_freq
done
It took 19 iterations of this loop to crash the GPU.
Ack. With your above script, I can reproduce a crash too on db845c
(A630) running v5.14. I didn't get any crash log though and device
just rebooted to USB crash mode.
And same crash on RB5 (A650) too https://hastebin.com/raw/ejutetuwun
Are we sure this is the same issue? It could be, but I thought we were
seeing a bunch of random gpu errors (which may eventually hit device crash).
-Akhil
fwiw I can't reproduce this crash on RB5 so far with v5.15-rc1 merge
window (HEAD: 477f70cd2a67)
So the problem doesn't seem to be Rob's change, it's just that prior to
it the chance to hitting it is way lower. Question is still what it is
that we're triggering.
Regards,
Bjorn
Re: [PATCH] drm/msm: Disable frequency clamping on a630
On Thu, 9 Sept 2021 at 17:47, Amit Pundir wrote:
>
> On Wed, 8 Sept 2021 at 07:50, Bjorn Andersson
> wrote:
> >
> > On Mon 09 Aug 10:26 PDT 2021, Akhil P Oommen wrote:
> >
> > > On 8/9/2021 9:48 PM, Caleb Connolly wrote:
> > > >
> > > >
> > > > On 09/08/2021 17:12, Rob Clark wrote:
> > > > > On Mon, Aug 9, 2021 at 7:52 AM Akhil P Oommen
> > > > > wrote:
> > [..]
> > > > > > I am a bit confused. We don't define a power domain for gpu in dt,
> > > > > > correct? Then what exactly set_opp do here? Do you think this
> > > > > > usleep is
> > > > > > what is helping here somehow to mask the issue?
> > > > The power domains (for cx and gx) are defined in the GMU DT, the OPPs in
> > > > the GPU DT. For the sake of simplicity I'll refer to the lowest
> > > > frequency (25700) and OPP level (RPMH_REGULATOR_LEVEL_LOW_SVS) as
> > > > the "min" state, and the highest frequency (71000) and OPP level
> > > > (RPMH_REGULATOR_LEVEL_TURBO_L1) as the "max" state. These are defined in
> > > > sdm845.dtsi under the gpu node.
> > > >
> > > > The new devfreq behaviour unmasks what I think is a driver bug, it
> > > > inadvertently puts much more strain on the GPU regulators than they
> > > > usually get. With the new behaviour the GPU jumps from it's min state to
> > > > the max state and back again extremely rapidly under workloads as small
> > > > as refreshing UI. Where previously the GPU would rarely if ever go above
> > > > 342MHz when interacting with the device, it now jumps between min and
> > > > max many times per second.
> > > >
> > > > If my understanding is correct, the current implementation of the GMU
> > > > set freq is the following:
> > > > - Get OPP for frequency to set
> > > > - Push the frequency to the GMU - immediately updating the core clock
> > > > - Call dev_pm_opp_set_opp() which triggers a notify chain, this winds
> > > > up somewhere in power management code and causes the gx regulator level
> > > > to be updated
> > >
> > > Nope. dev_pm_opp_set_opp() sets the bandwidth for gpu and nothing else. We
> > > were using a different api earlier which got deprecated -
> > > dev_pm_opp_set_bw().
> > >
> >
> > On the Lenovo Yoga C630 this is reproduced by starting alacritty and if
> > I'm lucky I managed to hit a few keys before it crashes, so I spent a
> > few hours looking into this as well...
> >
> > As you say, the dev_pm_opp_set_opp() will only cast a interconnect vote.
> > The opp-level is just there for show and isn't used by anything, at
> > least not on 845.
> >
> > Further more, I'm missing something in my tree, so the interconnect
> > doesn't hit sync_state, and as such we're not actually scaling the
> > buses. So the problem is not that Linux doesn't turn on the buses in
> > time.
> >
> > So I suspect that the "AHB bus error" isn't saying that we turned off
> > the bus, but rather that the GPU becomes unstable or something of that
> > sort.
> >
> >
> > Lastly, I reverted 9bc95570175a ("drm/msm: Devfreq tuning") and ran
> > Aquarium for 20 minutes without a problem. I then switched the gpu
> > devfreq governor to "userspace" and ran the following:
> >
> > while true; do
> > echo 25700 > /sys/class/devfreq/500.gpu/userspace/set_freq
> > echo 71000 > /sys/class/devfreq/500.gpu/userspace/set_freq
> > done
> >
> > It took 19 iterations of this loop to crash the GPU.
>
> Ack. With your above script, I can reproduce a crash too on db845c
> (A630) running v5.14. I didn't get any crash log though and device
> just rebooted to USB crash mode.
>
> And same crash on RB5 (A650) too https://hastebin.com/raw/ejutetuwun
fwiw I can't reproduce this crash on RB5 so far with v5.15-rc1 merge
window (HEAD: 477f70cd2a67)
>
> >
> > So the problem doesn't seem to be Rob's change, it's just that prior to
> > it the chance to hitting it is way lower. Question is still what it is
> > that we're triggering.
> >
> > Regards,
> > Bjorn
Re: [PATCH] drm/msm: Disable frequency clamping on a630
On Wed, 8 Sept 2021 at 07:50, Bjorn Andersson
wrote:
>
> On Mon 09 Aug 10:26 PDT 2021, Akhil P Oommen wrote:
>
> > On 8/9/2021 9:48 PM, Caleb Connolly wrote:
> > >
> > >
> > > On 09/08/2021 17:12, Rob Clark wrote:
> > > > On Mon, Aug 9, 2021 at 7:52 AM Akhil P Oommen
> > > > wrote:
> [..]
> > > > > I am a bit confused. We don't define a power domain for gpu in dt,
> > > > > correct? Then what exactly set_opp do here? Do you think this usleep
> > > > > is
> > > > > what is helping here somehow to mask the issue?
> > > The power domains (for cx and gx) are defined in the GMU DT, the OPPs in
> > > the GPU DT. For the sake of simplicity I'll refer to the lowest
> > > frequency (25700) and OPP level (RPMH_REGULATOR_LEVEL_LOW_SVS) as
> > > the "min" state, and the highest frequency (71000) and OPP level
> > > (RPMH_REGULATOR_LEVEL_TURBO_L1) as the "max" state. These are defined in
> > > sdm845.dtsi under the gpu node.
> > >
> > > The new devfreq behaviour unmasks what I think is a driver bug, it
> > > inadvertently puts much more strain on the GPU regulators than they
> > > usually get. With the new behaviour the GPU jumps from it's min state to
> > > the max state and back again extremely rapidly under workloads as small
> > > as refreshing UI. Where previously the GPU would rarely if ever go above
> > > 342MHz when interacting with the device, it now jumps between min and
> > > max many times per second.
> > >
> > > If my understanding is correct, the current implementation of the GMU
> > > set freq is the following:
> > > - Get OPP for frequency to set
> > > - Push the frequency to the GMU - immediately updating the core clock
> > > - Call dev_pm_opp_set_opp() which triggers a notify chain, this winds
> > > up somewhere in power management code and causes the gx regulator level
> > > to be updated
> >
> > Nope. dev_pm_opp_set_opp() sets the bandwidth for gpu and nothing else. We
> > were using a different api earlier which got deprecated -
> > dev_pm_opp_set_bw().
> >
>
> On the Lenovo Yoga C630 this is reproduced by starting alacritty and if
> I'm lucky I managed to hit a few keys before it crashes, so I spent a
> few hours looking into this as well...
>
> As you say, the dev_pm_opp_set_opp() will only cast a interconnect vote.
> The opp-level is just there for show and isn't used by anything, at
> least not on 845.
>
> Further more, I'm missing something in my tree, so the interconnect
> doesn't hit sync_state, and as such we're not actually scaling the
> buses. So the problem is not that Linux doesn't turn on the buses in
> time.
>
> So I suspect that the "AHB bus error" isn't saying that we turned off
> the bus, but rather that the GPU becomes unstable or something of that
> sort.
>
>
> Lastly, I reverted 9bc95570175a ("drm/msm: Devfreq tuning") and ran
> Aquarium for 20 minutes without a problem. I then switched the gpu
> devfreq governor to "userspace" and ran the following:
>
> while true; do
> echo 25700 > /sys/class/devfreq/500.gpu/userspace/set_freq
> echo 71000 > /sys/class/devfreq/500.gpu/userspace/set_freq
> done
>
> It took 19 iterations of this loop to crash the GPU.
Ack. With your above script, I can reproduce a crash too on db845c
(A630) running v5.14. I didn't get any crash log though and device
just rebooted to USB crash mode.
And same crash on RB5 (A650) too https://hastebin.com/raw/ejutetuwun
>
> So the problem doesn't seem to be Rob's change, it's just that prior to
> it the chance to hitting it is way lower. Question is still what it is
> that we're triggering.
>
> Regards,
> Bjorn
Re: [PATCH] drm/msm: Disable frequency clamping on a630
On 08/09/2021 03:21, Bjorn Andersson wrote:
On Mon 09 Aug 10:26 PDT 2021, Akhil P Oommen wrote:
On 8/9/2021 9:48 PM, Caleb Connolly wrote:
On 09/08/2021 17:12, Rob Clark wrote:
On Mon, Aug 9, 2021 at 7:52 AM Akhil P Oommen
wrote:
[..]
I am a bit confused. We don't define a power domain for gpu in dt,
correct? Then what exactly set_opp do here? Do you think this usleep is
what is helping here somehow to mask the issue?
The power domains (for cx and gx) are defined in the GMU DT, the OPPs in
the GPU DT. For the sake of simplicity I'll refer to the lowest
frequency (25700) and OPP level (RPMH_REGULATOR_LEVEL_LOW_SVS) as
the "min" state, and the highest frequency (71000) and OPP level
(RPMH_REGULATOR_LEVEL_TURBO_L1) as the "max" state. These are defined in
sdm845.dtsi under the gpu node.
The new devfreq behaviour unmasks what I think is a driver bug, it
inadvertently puts much more strain on the GPU regulators than they
usually get. With the new behaviour the GPU jumps from it's min state to
the max state and back again extremely rapidly under workloads as small
as refreshing UI. Where previously the GPU would rarely if ever go above
342MHz when interacting with the device, it now jumps between min and
max many times per second.
If my understanding is correct, the current implementation of the GMU
set freq is the following:
- Get OPP for frequency to set
- Push the frequency to the GMU - immediately updating the core clock
- Call dev_pm_opp_set_opp() which triggers a notify chain, this winds
up somewhere in power management code and causes the gx regulator level
to be updated
Nope. dev_pm_opp_set_opp() sets the bandwidth for gpu and nothing else. We
were using a different api earlier which got deprecated -
dev_pm_opp_set_bw().
On the Lenovo Yoga C630 this is reproduced by starting alacritty and if
I'm lucky I managed to hit a few keys before it crashes, so I spent a
few hours looking into this as well...
As you say, the dev_pm_opp_set_opp() will only cast a interconnect vote.
The opp-level is just there for show and isn't used by anything, at
least not on 845.
Further more, I'm missing something in my tree, so the interconnect
doesn't hit sync_state, and as such we're not actually scaling the
buses. So the problem is not that Linux doesn't turn on the buses in
time.
So I suspect that the "AHB bus error" isn't saying that we turned off
the bus, but rather that the GPU becomes unstable or something of that
sort.
Lastly, I reverted 9bc95570175a ("drm/msm: Devfreq tuning") and ran
Aquarium for 20 minutes without a problem. I then switched the gpu
devfreq governor to "userspace" and ran the following:
while true; do
echo 25700 > /sys/class/devfreq/500.gpu/userspace/set_freq
echo 71000 > /sys/class/devfreq/500.gpu/userspace/set_freq
done
It took 19 iterations of this loop to crash the GPU.
So the problem doesn't seem to be Rob's change, it's just that prior to
it the chance to hitting it is way lower. Question is still what it is
that we're triggering.
Do the opp-levels in DTS represent how the hardware behaves? If so then it does
just
appear to be that whatever is responsible for scaling the GX rail voltage
has no time limits and will attempt to switch the regulator between min/max
voltage as often as we tell it to which is probably not something the hardware
expected.
Regards,
Bjorn
--
Kind Regards,
Caleb (they/them)
Re: [PATCH] drm/msm: Disable frequency clamping on a630
On Mon 09 Aug 10:26 PDT 2021, Akhil P Oommen wrote:
> On 8/9/2021 9:48 PM, Caleb Connolly wrote:
> >
> >
> > On 09/08/2021 17:12, Rob Clark wrote:
> > > On Mon, Aug 9, 2021 at 7:52 AM Akhil P Oommen
> > > wrote:
[..]
> > > > I am a bit confused. We don't define a power domain for gpu in dt,
> > > > correct? Then what exactly set_opp do here? Do you think this usleep is
> > > > what is helping here somehow to mask the issue?
> > The power domains (for cx and gx) are defined in the GMU DT, the OPPs in
> > the GPU DT. For the sake of simplicity I'll refer to the lowest
> > frequency (25700) and OPP level (RPMH_REGULATOR_LEVEL_LOW_SVS) as
> > the "min" state, and the highest frequency (71000) and OPP level
> > (RPMH_REGULATOR_LEVEL_TURBO_L1) as the "max" state. These are defined in
> > sdm845.dtsi under the gpu node.
> >
> > The new devfreq behaviour unmasks what I think is a driver bug, it
> > inadvertently puts much more strain on the GPU regulators than they
> > usually get. With the new behaviour the GPU jumps from it's min state to
> > the max state and back again extremely rapidly under workloads as small
> > as refreshing UI. Where previously the GPU would rarely if ever go above
> > 342MHz when interacting with the device, it now jumps between min and
> > max many times per second.
> >
> > If my understanding is correct, the current implementation of the GMU
> > set freq is the following:
> > - Get OPP for frequency to set
> > - Push the frequency to the GMU - immediately updating the core clock
> > - Call dev_pm_opp_set_opp() which triggers a notify chain, this winds
> > up somewhere in power management code and causes the gx regulator level
> > to be updated
>
> Nope. dev_pm_opp_set_opp() sets the bandwidth for gpu and nothing else. We
> were using a different api earlier which got deprecated -
> dev_pm_opp_set_bw().
>
On the Lenovo Yoga C630 this is reproduced by starting alacritty and if
I'm lucky I managed to hit a few keys before it crashes, so I spent a
few hours looking into this as well...
As you say, the dev_pm_opp_set_opp() will only cast a interconnect vote.
The opp-level is just there for show and isn't used by anything, at
least not on 845.
Further more, I'm missing something in my tree, so the interconnect
doesn't hit sync_state, and as such we're not actually scaling the
buses. So the problem is not that Linux doesn't turn on the buses in
time.
So I suspect that the "AHB bus error" isn't saying that we turned off
the bus, but rather that the GPU becomes unstable or something of that
sort.
Lastly, I reverted 9bc95570175a ("drm/msm: Devfreq tuning") and ran
Aquarium for 20 minutes without a problem. I then switched the gpu
devfreq governor to "userspace" and ran the following:
while true; do
echo 25700 > /sys/class/devfreq/500.gpu/userspace/set_freq
echo 71000 > /sys/class/devfreq/500.gpu/userspace/set_freq
done
It took 19 iterations of this loop to crash the GPU.
So the problem doesn't seem to be Rob's change, it's just that prior to
it the chance to hitting it is way lower. Question is still what it is
that we're triggering.
Regards,
Bjorn
Re: [PATCH] drm/msm: Disable frequency clamping on a630
On Mon 09 Aug 14:08 PDT 2021, Rob Clark wrote:
> On Mon, Aug 9, 2021 at 1:35 PM Caleb Connolly
> wrote:
> >
> >
> >
> > On 09/08/2021 18:58, Rob Clark wrote:
> > > On Mon, Aug 9, 2021 at 10:28 AM Akhil P Oommen
> > > wrote:
> > >>
> > >> On 8/9/2021 9:48 PM, Caleb Connolly wrote:
> > >>>
> > >>>
> > >>> On 09/08/2021 17:12, Rob Clark wrote:
> > On Mon, Aug 9, 2021 at 7:52 AM Akhil P Oommen
> > wrote:
> > >
> > > On 8/8/2021 10:22 PM, Rob Clark wrote:
> > >> On Sun, Aug 8, 2021 at 7:33 AM Caleb Connolly
> > >> wrote:
> > >>>
> > >>>
> > >>>
> > >>> On 07/08/2021 21:04, Rob Clark wrote:
> > On Sat, Aug 7, 2021 at 12:21 PM Caleb Connolly
> > wrote:
> > >
> > > Hi Rob, Akhil,
> > >
> > > On 29/07/2021 21:53, Rob Clark wrote:
> > >> On Thu, Jul 29, 2021 at 1:28 PM Caleb Connolly
> > >> wrote:
> > >>>
> > >>>
> > >>>
> > >>> On 29/07/2021 21:24, Rob Clark wrote:
> > On Thu, Jul 29, 2021 at 1:06 PM Caleb Connolly
> > wrote:
> > >
> > > Hi Rob,
> > >
> > > I've done some more testing! It looks like before that patch
> > > ("drm/msm: Devfreq tuning") the GPU would never get above
> > > the second frequency in the OPP table (342MHz) (at least, not
> > > in glxgears). With the patch applied it would more
> > > aggressively jump up to the max frequency which seems to be
> > > unstable at the default regulator voltages.
> >
> > *ohh*, yeah, ok, that would explain it
> >
> > > Hacking the pm8005 s1 regulator (which provides VDD_GFX) up
> > > to 0.988v (instead of the stock 0.516v) makes the GPU stable
> > > at the higher frequencies.
> > >
> > > Applying this patch reverts the behaviour, and the GPU never
> > > goes above 342MHz in glxgears, losing ~30% performance in
> > > glxgear.
> > >
> > > I think (?) that enabling CPR support would be the proper
> > > solution to this - that would ensure that the regulators run
> > > at the voltage the hardware needs to be stable.
> > >
> > > Is hacking the voltage higher (although ideally not quite
> > > that high) an acceptable short term solution until we have
> > > CPR? Or would it be safer to just not make use of the higher
> > > frequencies on a630 for now?
> > >
> >
> > tbh, I'm not sure about the regulator stuff and CPR.. Bjorn is
> > already
> > on CC and I added sboyd, maybe one of them knows better.
> >
> > In the short term, removing the higher problematic OPPs from
> > dts might
> > be a better option than this patch (which I'm dropping), since
> > there
> > is nothing stopping other workloads from hitting higher OPPs.
> > >>> Oh yeah that sounds like a more sensible workaround than mine .
> >
> > I'm slightly curious why I didn't have problems at higher OPPs
> > on my
> > c630 laptop (sdm850)
> > >>> Perhaps you won the sillicon lottery - iirc sdm850 is binned
> > >>> for higher clocks as is out of the factory.
> > >>>
> > >>> Would it be best to drop the OPPs for all devices? Or just
> > >>> those affected? I guess it's possible another c630 might
> > >>> crash where yours doesn't?
> > >>
> > >> I've not heard any reports of similar issues from the handful of
> > >> other
> > >> folks with c630's on #aarch64-laptops.. but I can't really say
> > >> if that
> > >> is luck or not.
> > > It looks like this affects at least the OnePlus 6 and PocoPhone
> > > F1, I've done some more poking and the following diff
> > > seems to fix the stability issues completely, it seems the delay
> > > is required to let the update propagate.
> > >
> > > This doesn't feel like the right fix, but hopefully it's enough
> > > to come up with a better solution than disabling the new
> > > devfreq behaviour on a630.
> > >
> > > diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> > > b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> > > index d7cec7f0dde0..69e2a5e84dae 100644
> > > --- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> > > +++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> > > @@ -139,6 +139,10 @@ void a6xx_gmu_set_freq(struct msm_gpu *gpu,
> > > struct dev_pm_opp *opp)
> > > return;
> > > }
> > >
> > > +
Re: [PATCH] drm/msm: Disable frequency clamping on a630
On Tue, Sep 7, 2021 at 1:25 AM Amit Pundir wrote: > > On Tue, 7 Sept 2021 at 07:11, Rob Clark wrote: > > > > One thing I thought of, which would be worth ruling out, is whether > > this issue only occurs with freq changes immediately after resuming > > the GPU, vs freq changes in general. Could you try the below patch. > > And if it "fixes" the issue, then try reducing the delay until you > > start seeing GPU hangs again. > > It doesn't fix the crash and I can still reproduce it > https://pastebin.com/raw/bxK4mAhB Ok, thanks for confirming. That implies the limitation is about changing freq in general, rather than immediately after resume.. the latter could be a new scenario after 9bc95570175a. But this confirms that this is an issue that has been there all along. I'm still not quite sure what the correct fix is BR, -R > > > > > -- > > diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c > > b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c > > index 40c9fef457a4..278b85207ea3 100644 > > --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c > > +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c > > @@ -1513,6 +1513,8 @@ static int a6xx_pm_resume(struct msm_gpu *gpu) > > if (ret) > > return ret; > > > > + msleep(5); > > + > > msm_devfreq_resume(gpu); > > > > a6xx_llc_activate(a6xx_gpu); > > -- > > > > BR, > > -R
Re: [PATCH] drm/msm: Disable frequency clamping on a630
On Tue, 7 Sept 2021 at 07:11, Rob Clark wrote: > > One thing I thought of, which would be worth ruling out, is whether > this issue only occurs with freq changes immediately after resuming > the GPU, vs freq changes in general. Could you try the below patch. > And if it "fixes" the issue, then try reducing the delay until you > start seeing GPU hangs again. It doesn't fix the crash and I can still reproduce it https://pastebin.com/raw/bxK4mAhB > > -- > diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c > b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c > index 40c9fef457a4..278b85207ea3 100644 > --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c > +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c > @@ -1513,6 +1513,8 @@ static int a6xx_pm_resume(struct msm_gpu *gpu) > if (ret) > return ret; > > + msleep(5); > + > msm_devfreq_resume(gpu); > > a6xx_llc_activate(a6xx_gpu); > -- > > BR, > -R
Re: [PATCH] drm/msm: Disable frequency clamping on a630
On Tue, 7 Sept 2021 at 02:53, Rob Clark wrote: > > On Mon, Sep 6, 2021 at 1:50 PM Rob Clark wrote: > > > > > > ok, I guess it is at least not a board specific thing (ie. won't need > > to introduce some dt binding).. > > > > It would be nice to know what the maximum we can safely increase freq > > in one step, if we need to limit that. > > Also, one sanity check.. for android builds, are you using the same > a630_gmu.bin from linux-firmware? If not, does the l-f gmu fw change > things? We are using the same a630_gmu.bin from linux-firmware. > > For freq changes, we basically ask gmu for the freq we want, and it > votes for the requested freq.. so a gmu fw bug could be possible here. > > BR, > -R
Re: [PATCH] drm/msm: Disable frequency clamping on a630
On Mon, Sep 6, 2021 at 12:58 PM Amit Pundir wrote:
>
> On Mon, 6 Sept 2021 at 21:54, Rob Clark wrote:
> >
> > On Mon, Sep 6, 2021 at 1:02 AM Amit Pundir wrote:
> > >
> > > On Sat, 4 Sept 2021 at 01:55, Rob Clark wrote:
> > > >
> > > > On Fri, Sep 3, 2021 at 12:39 PM John Stultz
> > > > wrote:
> > > > >
> > > > > On Thu, Jul 29, 2021 at 1:49 PM Rob Clark wrote:
> > > > > > On Thu, Jul 29, 2021 at 1:28 PM Caleb Connolly
> > > > > > wrote:
> > > > > > > On 29/07/2021 21:24, Rob Clark wrote:
> > > > > > > > On Thu, Jul 29, 2021 at 1:06 PM Caleb Connolly
> > > > > > > > wrote:
> > > > > > > >>
> > > > > > > >> Hi Rob,
> > > > > > > >>
> > > > > > > >> I've done some more testing! It looks like before that patch
> > > > > > > >> ("drm/msm: Devfreq tuning") the GPU would never get above
> > > > > > > >> the second frequency in the OPP table (342MHz) (at least, not
> > > > > > > >> in glxgears). With the patch applied it would more
> > > > > > > >> aggressively jump up to the max frequency which seems to be
> > > > > > > >> unstable at the default regulator voltages.
> > > > > > > >
> > > > > > > > *ohh*, yeah, ok, that would explain it
> > > > > > > >
> > > > > > > >> Hacking the pm8005 s1 regulator (which provides VDD_GFX) up to
> > > > > > > >> 0.988v (instead of the stock 0.516v) makes the GPU stable
> > > > > > > >> at the higher frequencies.
> > > > > > > >>
> > > > > > > >> Applying this patch reverts the behaviour, and the GPU never
> > > > > > > >> goes above 342MHz in glxgears, losing ~30% performance in
> > > > > > > >> glxgear.
> > > > > > > >>
> > > > > > > >> I think (?) that enabling CPR support would be the proper
> > > > > > > >> solution to this - that would ensure that the regulators run
> > > > > > > >> at the voltage the hardware needs to be stable.
> > > > > > > >>
> > > > > > > >> Is hacking the voltage higher (although ideally not quite that
> > > > > > > >> high) an acceptable short term solution until we have
> > > > > > > >> CPR? Or would it be safer to just not make use of the higher
> > > > > > > >> frequencies on a630 for now?
> > > > > > > >>
> > > > > > > >
> > > > > > > > tbh, I'm not sure about the regulator stuff and CPR.. Bjorn is
> > > > > > > > already
> > > > > > > > on CC and I added sboyd, maybe one of them knows better.
> > > > > > > >
> > > > > > > > In the short term, removing the higher problematic OPPs from
> > > > > > > > dts might
> > > > > > > > be a better option than this patch (which I'm dropping), since
> > > > > > > > there
> > > > > > > > is nothing stopping other workloads from hitting higher OPPs.
> > > > > > > Oh yeah that sounds like a more sensible workaround than mine .
> > > > > > > >
> > > > > > > > I'm slightly curious why I didn't have problems at higher OPPs
> > > > > > > > on my
> > > > > > > > c630 laptop (sdm850)
> > > > > > > Perhaps you won the sillicon lottery - iirc sdm850 is binned for
> > > > > > > higher clocks as is out of the factory.
> > > > > > >
> > > > > > > Would it be best to drop the OPPs for all devices? Or just those
> > > > > > > affected? I guess it's possible another c630 might
> > > > > > > crash where yours doesn't?
> > > > > >
> > > > > > I've not heard any reports of similar issues from the handful of
> > > > > > other
> > > > > > folks with c630's on #aarch64-laptops.. but I can't really say if
> > > > > > that
> > > > > > is luck or not.
> > > > > >
> > > > > > Maybe just remove it for affected devices? But I'll defer to Bjorn.
> > > > >
> > > > > Just as another datapoint, I was just marveling at how suddenly smooth
> > > > > the UI was performing on db845c and Caleb pointed me at the "drm/msm:
> > > > > Devfreq tuning" patch as the likely cause of the improvement, and
> > > > > mid-discussion my board crashed into USB crash mode:
> > > > > [ 146.157696][C0] adreno 500.gpu: CP | AHB bus error
> > > > > [ 146.163303][C0] adreno 500.gpu: CP | AHB bus error
> > > > > [ 146.168837][C0] adreno 500.gpu: RBBM | ATB bus overflow
> > > > > [ 146.174960][C0] adreno 500.gpu: CP | HW fault |
> > > > > status=0x
> > > > > [ 146.181917][C0] adreno 500.gpu: CP | AHB bus error
> > > > > [ 146.187547][C0] adreno 500.gpu: CP illegal instruction
> > > > > error
> > > > > [ 146.194009][C0] adreno 500.gpu: CP | AHB bus error
> > > > > [ 146.308909][T9] Internal error: synchronous external abort:
> > > > > 9610 [#1] PREEMPT SMP
> > > > > [ 146.317150][T9] Modules linked in:
> > > > > [ 146.320941][T9] CPU: 3 PID: 9 Comm: kworker/u16:1 Tainted: G
> > > > > W 5.14.0-mainline-06795-g42b258c2275c #24
> > > > > [ 146.331974][T9] Hardware name: Thundercomm Dragonboar
> > > > > Format: Log Type - Time(microsec) - Message - Optional Info
> > > > > Log Type: B - Since Boot(Power On Reset), D - Delta, S - Statistic
> > > > > S - QC_IMAGE_VERSION_STRING=BOOT.XF.2.0-00371-SDM845LZB-1
> > > > > S -
Re: [PATCH] drm/msm: Disable frequency clamping on a630
On Mon, Sep 6, 2021 at 1:50 PM Rob Clark wrote:
>
> On Mon, Sep 6, 2021 at 12:58 PM Amit Pundir wrote:
> >
> > On Mon, 6 Sept 2021 at 21:54, Rob Clark wrote:
> > >
> > > On Mon, Sep 6, 2021 at 1:02 AM Amit Pundir wrote:
> > > >
> > > > On Sat, 4 Sept 2021 at 01:55, Rob Clark wrote:
> > > > >
> > > > > On Fri, Sep 3, 2021 at 12:39 PM John Stultz
> > > > > wrote:
> > > > > >
> > > > > > On Thu, Jul 29, 2021 at 1:49 PM Rob Clark
> > > > > > wrote:
> > > > > > > On Thu, Jul 29, 2021 at 1:28 PM Caleb Connolly
> > > > > > > wrote:
> > > > > > > > On 29/07/2021 21:24, Rob Clark wrote:
> > > > > > > > > On Thu, Jul 29, 2021 at 1:06 PM Caleb Connolly
> > > > > > > > > wrote:
> > > > > > > > >>
> > > > > > > > >> Hi Rob,
> > > > > > > > >>
> > > > > > > > >> I've done some more testing! It looks like before that patch
> > > > > > > > >> ("drm/msm: Devfreq tuning") the GPU would never get above
> > > > > > > > >> the second frequency in the OPP table (342MHz) (at least,
> > > > > > > > >> not in glxgears). With the patch applied it would more
> > > > > > > > >> aggressively jump up to the max frequency which seems to be
> > > > > > > > >> unstable at the default regulator voltages.
> > > > > > > > >
> > > > > > > > > *ohh*, yeah, ok, that would explain it
> > > > > > > > >
> > > > > > > > >> Hacking the pm8005 s1 regulator (which provides VDD_GFX) up
> > > > > > > > >> to 0.988v (instead of the stock 0.516v) makes the GPU stable
> > > > > > > > >> at the higher frequencies.
> > > > > > > > >>
> > > > > > > > >> Applying this patch reverts the behaviour, and the GPU never
> > > > > > > > >> goes above 342MHz in glxgears, losing ~30% performance in
> > > > > > > > >> glxgear.
> > > > > > > > >>
> > > > > > > > >> I think (?) that enabling CPR support would be the proper
> > > > > > > > >> solution to this - that would ensure that the regulators run
> > > > > > > > >> at the voltage the hardware needs to be stable.
> > > > > > > > >>
> > > > > > > > >> Is hacking the voltage higher (although ideally not quite
> > > > > > > > >> that high) an acceptable short term solution until we have
> > > > > > > > >> CPR? Or would it be safer to just not make use of the higher
> > > > > > > > >> frequencies on a630 for now?
> > > > > > > > >>
> > > > > > > > >
> > > > > > > > > tbh, I'm not sure about the regulator stuff and CPR.. Bjorn
> > > > > > > > > is already
> > > > > > > > > on CC and I added sboyd, maybe one of them knows better.
> > > > > > > > >
> > > > > > > > > In the short term, removing the higher problematic OPPs from
> > > > > > > > > dts might
> > > > > > > > > be a better option than this patch (which I'm dropping),
> > > > > > > > > since there
> > > > > > > > > is nothing stopping other workloads from hitting higher OPPs.
> > > > > > > > Oh yeah that sounds like a more sensible workaround than mine .
> > > > > > > > >
> > > > > > > > > I'm slightly curious why I didn't have problems at higher
> > > > > > > > > OPPs on my
> > > > > > > > > c630 laptop (sdm850)
> > > > > > > > Perhaps you won the sillicon lottery - iirc sdm850 is binned
> > > > > > > > for higher clocks as is out of the factory.
> > > > > > > >
> > > > > > > > Would it be best to drop the OPPs for all devices? Or just
> > > > > > > > those affected? I guess it's possible another c630 might
> > > > > > > > crash where yours doesn't?
> > > > > > >
> > > > > > > I've not heard any reports of similar issues from the handful of
> > > > > > > other
> > > > > > > folks with c630's on #aarch64-laptops.. but I can't really say if
> > > > > > > that
> > > > > > > is luck or not.
> > > > > > >
> > > > > > > Maybe just remove it for affected devices? But I'll defer to
> > > > > > > Bjorn.
> > > > > >
> > > > > > Just as another datapoint, I was just marveling at how suddenly
> > > > > > smooth
> > > > > > the UI was performing on db845c and Caleb pointed me at the
> > > > > > "drm/msm:
> > > > > > Devfreq tuning" patch as the likely cause of the improvement, and
> > > > > > mid-discussion my board crashed into USB crash mode:
> > > > > > [ 146.157696][C0] adreno 500.gpu: CP | AHB bus error
> > > > > > [ 146.163303][C0] adreno 500.gpu: CP | AHB bus error
> > > > > > [ 146.168837][C0] adreno 500.gpu: RBBM | ATB bus overflow
> > > > > > [ 146.174960][C0] adreno 500.gpu: CP | HW fault |
> > > > > > status=0x
> > > > > > [ 146.181917][C0] adreno 500.gpu: CP | AHB bus error
> > > > > > [ 146.187547][C0] adreno 500.gpu: CP illegal instruction
> > > > > > error
> > > > > > [ 146.194009][C0] adreno 500.gpu: CP | AHB bus error
> > > > > > [ 146.308909][T9] Internal error: synchronous external abort:
> > > > > > 9610 [#1] PREEMPT SMP
> > > > > > [ 146.317150][T9] Modules linked in:
> > > > > > [ 146.320941][T9] CPU: 3 PID: 9 Comm: kworker/u16:1 Tainted: G
> > > > > > W 5.14.0-mainline-06795-g42b258c2275c #24
> > > > > > [ 146.331974][
Re: [PATCH] drm/msm: Disable frequency clamping on a630
On Mon, Sep 6, 2021 at 12:58 PM Amit Pundir wrote:
>
> On Mon, 6 Sept 2021 at 21:54, Rob Clark wrote:
> >
> > On Mon, Sep 6, 2021 at 1:02 AM Amit Pundir wrote:
> > >
> > > On Sat, 4 Sept 2021 at 01:55, Rob Clark wrote:
> > > >
> > > > On Fri, Sep 3, 2021 at 12:39 PM John Stultz
> > > > wrote:
> > > > >
> > > > > On Thu, Jul 29, 2021 at 1:49 PM Rob Clark wrote:
> > > > > > On Thu, Jul 29, 2021 at 1:28 PM Caleb Connolly
> > > > > > wrote:
> > > > > > > On 29/07/2021 21:24, Rob Clark wrote:
> > > > > > > > On Thu, Jul 29, 2021 at 1:06 PM Caleb Connolly
> > > > > > > > wrote:
> > > > > > > >>
> > > > > > > >> Hi Rob,
> > > > > > > >>
> > > > > > > >> I've done some more testing! It looks like before that patch
> > > > > > > >> ("drm/msm: Devfreq tuning") the GPU would never get above
> > > > > > > >> the second frequency in the OPP table (342MHz) (at least, not
> > > > > > > >> in glxgears). With the patch applied it would more
> > > > > > > >> aggressively jump up to the max frequency which seems to be
> > > > > > > >> unstable at the default regulator voltages.
> > > > > > > >
> > > > > > > > *ohh*, yeah, ok, that would explain it
> > > > > > > >
> > > > > > > >> Hacking the pm8005 s1 regulator (which provides VDD_GFX) up to
> > > > > > > >> 0.988v (instead of the stock 0.516v) makes the GPU stable
> > > > > > > >> at the higher frequencies.
> > > > > > > >>
> > > > > > > >> Applying this patch reverts the behaviour, and the GPU never
> > > > > > > >> goes above 342MHz in glxgears, losing ~30% performance in
> > > > > > > >> glxgear.
> > > > > > > >>
> > > > > > > >> I think (?) that enabling CPR support would be the proper
> > > > > > > >> solution to this - that would ensure that the regulators run
> > > > > > > >> at the voltage the hardware needs to be stable.
> > > > > > > >>
> > > > > > > >> Is hacking the voltage higher (although ideally not quite that
> > > > > > > >> high) an acceptable short term solution until we have
> > > > > > > >> CPR? Or would it be safer to just not make use of the higher
> > > > > > > >> frequencies on a630 for now?
> > > > > > > >>
> > > > > > > >
> > > > > > > > tbh, I'm not sure about the regulator stuff and CPR.. Bjorn is
> > > > > > > > already
> > > > > > > > on CC and I added sboyd, maybe one of them knows better.
> > > > > > > >
> > > > > > > > In the short term, removing the higher problematic OPPs from
> > > > > > > > dts might
> > > > > > > > be a better option than this patch (which I'm dropping), since
> > > > > > > > there
> > > > > > > > is nothing stopping other workloads from hitting higher OPPs.
> > > > > > > Oh yeah that sounds like a more sensible workaround than mine .
> > > > > > > >
> > > > > > > > I'm slightly curious why I didn't have problems at higher OPPs
> > > > > > > > on my
> > > > > > > > c630 laptop (sdm850)
> > > > > > > Perhaps you won the sillicon lottery - iirc sdm850 is binned for
> > > > > > > higher clocks as is out of the factory.
> > > > > > >
> > > > > > > Would it be best to drop the OPPs for all devices? Or just those
> > > > > > > affected? I guess it's possible another c630 might
> > > > > > > crash where yours doesn't?
> > > > > >
> > > > > > I've not heard any reports of similar issues from the handful of
> > > > > > other
> > > > > > folks with c630's on #aarch64-laptops.. but I can't really say if
> > > > > > that
> > > > > > is luck or not.
> > > > > >
> > > > > > Maybe just remove it for affected devices? But I'll defer to Bjorn.
> > > > >
> > > > > Just as another datapoint, I was just marveling at how suddenly smooth
> > > > > the UI was performing on db845c and Caleb pointed me at the "drm/msm:
> > > > > Devfreq tuning" patch as the likely cause of the improvement, and
> > > > > mid-discussion my board crashed into USB crash mode:
> > > > > [ 146.157696][C0] adreno 500.gpu: CP | AHB bus error
> > > > > [ 146.163303][C0] adreno 500.gpu: CP | AHB bus error
> > > > > [ 146.168837][C0] adreno 500.gpu: RBBM | ATB bus overflow
> > > > > [ 146.174960][C0] adreno 500.gpu: CP | HW fault |
> > > > > status=0x
> > > > > [ 146.181917][C0] adreno 500.gpu: CP | AHB bus error
> > > > > [ 146.187547][C0] adreno 500.gpu: CP illegal instruction
> > > > > error
> > > > > [ 146.194009][C0] adreno 500.gpu: CP | AHB bus error
> > > > > [ 146.308909][T9] Internal error: synchronous external abort:
> > > > > 9610 [#1] PREEMPT SMP
> > > > > [ 146.317150][T9] Modules linked in:
> > > > > [ 146.320941][T9] CPU: 3 PID: 9 Comm: kworker/u16:1 Tainted: G
> > > > > W 5.14.0-mainline-06795-g42b258c2275c #24
> > > > > [ 146.331974][T9] Hardware name: Thundercomm Dragonboar
> > > > > Format: Log Type - Time(microsec) - Message - Optional Info
> > > > > Log Type: B - Since Boot(Power On Reset), D - Delta, S - Statistic
> > > > > S - QC_IMAGE_VERSION_STRING=BOOT.XF.2.0-00371-SDM845LZB-1
> > > > > S -
Re: [PATCH] drm/msm: Disable frequency clamping on a630
On Mon, 6 Sept 2021 at 21:54, Rob Clark wrote:
>
> On Mon, Sep 6, 2021 at 1:02 AM Amit Pundir wrote:
> >
> > On Sat, 4 Sept 2021 at 01:55, Rob Clark wrote:
> > >
> > > On Fri, Sep 3, 2021 at 12:39 PM John Stultz
> > > wrote:
> > > >
> > > > On Thu, Jul 29, 2021 at 1:49 PM Rob Clark wrote:
> > > > > On Thu, Jul 29, 2021 at 1:28 PM Caleb Connolly
> > > > > wrote:
> > > > > > On 29/07/2021 21:24, Rob Clark wrote:
> > > > > > > On Thu, Jul 29, 2021 at 1:06 PM Caleb Connolly
> > > > > > > wrote:
> > > > > > >>
> > > > > > >> Hi Rob,
> > > > > > >>
> > > > > > >> I've done some more testing! It looks like before that patch
> > > > > > >> ("drm/msm: Devfreq tuning") the GPU would never get above
> > > > > > >> the second frequency in the OPP table (342MHz) (at least, not in
> > > > > > >> glxgears). With the patch applied it would more
> > > > > > >> aggressively jump up to the max frequency which seems to be
> > > > > > >> unstable at the default regulator voltages.
> > > > > > >
> > > > > > > *ohh*, yeah, ok, that would explain it
> > > > > > >
> > > > > > >> Hacking the pm8005 s1 regulator (which provides VDD_GFX) up to
> > > > > > >> 0.988v (instead of the stock 0.516v) makes the GPU stable
> > > > > > >> at the higher frequencies.
> > > > > > >>
> > > > > > >> Applying this patch reverts the behaviour, and the GPU never
> > > > > > >> goes above 342MHz in glxgears, losing ~30% performance in
> > > > > > >> glxgear.
> > > > > > >>
> > > > > > >> I think (?) that enabling CPR support would be the proper
> > > > > > >> solution to this - that would ensure that the regulators run
> > > > > > >> at the voltage the hardware needs to be stable.
> > > > > > >>
> > > > > > >> Is hacking the voltage higher (although ideally not quite that
> > > > > > >> high) an acceptable short term solution until we have
> > > > > > >> CPR? Or would it be safer to just not make use of the higher
> > > > > > >> frequencies on a630 for now?
> > > > > > >>
> > > > > > >
> > > > > > > tbh, I'm not sure about the regulator stuff and CPR.. Bjorn is
> > > > > > > already
> > > > > > > on CC and I added sboyd, maybe one of them knows better.
> > > > > > >
> > > > > > > In the short term, removing the higher problematic OPPs from dts
> > > > > > > might
> > > > > > > be a better option than this patch (which I'm dropping), since
> > > > > > > there
> > > > > > > is nothing stopping other workloads from hitting higher OPPs.
> > > > > > Oh yeah that sounds like a more sensible workaround than mine .
> > > > > > >
> > > > > > > I'm slightly curious why I didn't have problems at higher OPPs on
> > > > > > > my
> > > > > > > c630 laptop (sdm850)
> > > > > > Perhaps you won the sillicon lottery - iirc sdm850 is binned for
> > > > > > higher clocks as is out of the factory.
> > > > > >
> > > > > > Would it be best to drop the OPPs for all devices? Or just those
> > > > > > affected? I guess it's possible another c630 might
> > > > > > crash where yours doesn't?
> > > > >
> > > > > I've not heard any reports of similar issues from the handful of other
> > > > > folks with c630's on #aarch64-laptops.. but I can't really say if that
> > > > > is luck or not.
> > > > >
> > > > > Maybe just remove it for affected devices? But I'll defer to Bjorn.
> > > >
> > > > Just as another datapoint, I was just marveling at how suddenly smooth
> > > > the UI was performing on db845c and Caleb pointed me at the "drm/msm:
> > > > Devfreq tuning" patch as the likely cause of the improvement, and
> > > > mid-discussion my board crashed into USB crash mode:
> > > > [ 146.157696][C0] adreno 500.gpu: CP | AHB bus error
> > > > [ 146.163303][C0] adreno 500.gpu: CP | AHB bus error
> > > > [ 146.168837][C0] adreno 500.gpu: RBBM | ATB bus overflow
> > > > [ 146.174960][C0] adreno 500.gpu: CP | HW fault |
> > > > status=0x
> > > > [ 146.181917][C0] adreno 500.gpu: CP | AHB bus error
> > > > [ 146.187547][C0] adreno 500.gpu: CP illegal instruction error
> > > > [ 146.194009][C0] adreno 500.gpu: CP | AHB bus error
> > > > [ 146.308909][T9] Internal error: synchronous external abort:
> > > > 9610 [#1] PREEMPT SMP
> > > > [ 146.317150][T9] Modules linked in:
> > > > [ 146.320941][T9] CPU: 3 PID: 9 Comm: kworker/u16:1 Tainted: G
> > > > W 5.14.0-mainline-06795-g42b258c2275c #24
> > > > [ 146.331974][T9] Hardware name: Thundercomm Dragonboar
> > > > Format: Log Type - Time(microsec) - Message - Optional Info
> > > > Log Type: B - Since Boot(Power On Reset), D - Delta, S - Statistic
> > > > S - QC_IMAGE_VERSION_STRING=BOOT.XF.2.0-00371-SDM845LZB-1
> > > > S - IMAGE_VARIANT_STRING=SDM845LA
> > > > S - OEM_IMAGE_VERSION_STRING=TSBJ-FA-PC-02170
> > > >
> > > > So Caleb sent me to this thread. :)
> > > >
> > > > I'm still trying to trip it again, but it does seem like db845c is
> > > > also seeing some stability issues with Linus' HEAD.
> > > >
> > >
> > >
Re: [PATCH] drm/msm: Disable frequency clamping on a630
On Mon, Sep 6, 2021 at 1:02 AM Amit Pundir wrote:
>
> On Sat, 4 Sept 2021 at 01:55, Rob Clark wrote:
> >
> > On Fri, Sep 3, 2021 at 12:39 PM John Stultz wrote:
> > >
> > > On Thu, Jul 29, 2021 at 1:49 PM Rob Clark wrote:
> > > > On Thu, Jul 29, 2021 at 1:28 PM Caleb Connolly
> > > > wrote:
> > > > > On 29/07/2021 21:24, Rob Clark wrote:
> > > > > > On Thu, Jul 29, 2021 at 1:06 PM Caleb Connolly
> > > > > > wrote:
> > > > > >>
> > > > > >> Hi Rob,
> > > > > >>
> > > > > >> I've done some more testing! It looks like before that patch
> > > > > >> ("drm/msm: Devfreq tuning") the GPU would never get above
> > > > > >> the second frequency in the OPP table (342MHz) (at least, not in
> > > > > >> glxgears). With the patch applied it would more
> > > > > >> aggressively jump up to the max frequency which seems to be
> > > > > >> unstable at the default regulator voltages.
> > > > > >
> > > > > > *ohh*, yeah, ok, that would explain it
> > > > > >
> > > > > >> Hacking the pm8005 s1 regulator (which provides VDD_GFX) up to
> > > > > >> 0.988v (instead of the stock 0.516v) makes the GPU stable
> > > > > >> at the higher frequencies.
> > > > > >>
> > > > > >> Applying this patch reverts the behaviour, and the GPU never goes
> > > > > >> above 342MHz in glxgears, losing ~30% performance in
> > > > > >> glxgear.
> > > > > >>
> > > > > >> I think (?) that enabling CPR support would be the proper solution
> > > > > >> to this - that would ensure that the regulators run
> > > > > >> at the voltage the hardware needs to be stable.
> > > > > >>
> > > > > >> Is hacking the voltage higher (although ideally not quite that
> > > > > >> high) an acceptable short term solution until we have
> > > > > >> CPR? Or would it be safer to just not make use of the higher
> > > > > >> frequencies on a630 for now?
> > > > > >>
> > > > > >
> > > > > > tbh, I'm not sure about the regulator stuff and CPR.. Bjorn is
> > > > > > already
> > > > > > on CC and I added sboyd, maybe one of them knows better.
> > > > > >
> > > > > > In the short term, removing the higher problematic OPPs from dts
> > > > > > might
> > > > > > be a better option than this patch (which I'm dropping), since there
> > > > > > is nothing stopping other workloads from hitting higher OPPs.
> > > > > Oh yeah that sounds like a more sensible workaround than mine .
> > > > > >
> > > > > > I'm slightly curious why I didn't have problems at higher OPPs on my
> > > > > > c630 laptop (sdm850)
> > > > > Perhaps you won the sillicon lottery - iirc sdm850 is binned for
> > > > > higher clocks as is out of the factory.
> > > > >
> > > > > Would it be best to drop the OPPs for all devices? Or just those
> > > > > affected? I guess it's possible another c630 might
> > > > > crash where yours doesn't?
> > > >
> > > > I've not heard any reports of similar issues from the handful of other
> > > > folks with c630's on #aarch64-laptops.. but I can't really say if that
> > > > is luck or not.
> > > >
> > > > Maybe just remove it for affected devices? But I'll defer to Bjorn.
> > >
> > > Just as another datapoint, I was just marveling at how suddenly smooth
> > > the UI was performing on db845c and Caleb pointed me at the "drm/msm:
> > > Devfreq tuning" patch as the likely cause of the improvement, and
> > > mid-discussion my board crashed into USB crash mode:
> > > [ 146.157696][C0] adreno 500.gpu: CP | AHB bus error
> > > [ 146.163303][C0] adreno 500.gpu: CP | AHB bus error
> > > [ 146.168837][C0] adreno 500.gpu: RBBM | ATB bus overflow
> > > [ 146.174960][C0] adreno 500.gpu: CP | HW fault |
> > > status=0x
> > > [ 146.181917][C0] adreno 500.gpu: CP | AHB bus error
> > > [ 146.187547][C0] adreno 500.gpu: CP illegal instruction error
> > > [ 146.194009][C0] adreno 500.gpu: CP | AHB bus error
> > > [ 146.308909][T9] Internal error: synchronous external abort:
> > > 9610 [#1] PREEMPT SMP
> > > [ 146.317150][T9] Modules linked in:
> > > [ 146.320941][T9] CPU: 3 PID: 9 Comm: kworker/u16:1 Tainted: G
> > > W 5.14.0-mainline-06795-g42b258c2275c #24
> > > [ 146.331974][T9] Hardware name: Thundercomm Dragonboar
> > > Format: Log Type - Time(microsec) - Message - Optional Info
> > > Log Type: B - Since Boot(Power On Reset), D - Delta, S - Statistic
> > > S - QC_IMAGE_VERSION_STRING=BOOT.XF.2.0-00371-SDM845LZB-1
> > > S - IMAGE_VARIANT_STRING=SDM845LA
> > > S - OEM_IMAGE_VERSION_STRING=TSBJ-FA-PC-02170
> > >
> > > So Caleb sent me to this thread. :)
> > >
> > > I'm still trying to trip it again, but it does seem like db845c is
> > > also seeing some stability issues with Linus' HEAD.
> > >
> >
> > Caleb's original pastebin seems to have expired (or at least require
> > some sort of ubuntu login to access).. were the crashes he was seeing
> > also 'AHB bus error'?
>
> I can reproduce this hard crash
> https://www.irccloud.com/pastebin/Cu6UJntE/ and a gpu lockup
>
Re: [PATCH] drm/msm: Disable frequency clamping on a630
On Sat, 4 Sept 2021 at 01:55, Rob Clark wrote:
>
> On Fri, Sep 3, 2021 at 12:39 PM John Stultz wrote:
> >
> > On Thu, Jul 29, 2021 at 1:49 PM Rob Clark wrote:
> > > On Thu, Jul 29, 2021 at 1:28 PM Caleb Connolly
> > > wrote:
> > > > On 29/07/2021 21:24, Rob Clark wrote:
> > > > > On Thu, Jul 29, 2021 at 1:06 PM Caleb Connolly
> > > > > wrote:
> > > > >>
> > > > >> Hi Rob,
> > > > >>
> > > > >> I've done some more testing! It looks like before that patch
> > > > >> ("drm/msm: Devfreq tuning") the GPU would never get above
> > > > >> the second frequency in the OPP table (342MHz) (at least, not in
> > > > >> glxgears). With the patch applied it would more
> > > > >> aggressively jump up to the max frequency which seems to be unstable
> > > > >> at the default regulator voltages.
> > > > >
> > > > > *ohh*, yeah, ok, that would explain it
> > > > >
> > > > >> Hacking the pm8005 s1 regulator (which provides VDD_GFX) up to
> > > > >> 0.988v (instead of the stock 0.516v) makes the GPU stable
> > > > >> at the higher frequencies.
> > > > >>
> > > > >> Applying this patch reverts the behaviour, and the GPU never goes
> > > > >> above 342MHz in glxgears, losing ~30% performance in
> > > > >> glxgear.
> > > > >>
> > > > >> I think (?) that enabling CPR support would be the proper solution
> > > > >> to this - that would ensure that the regulators run
> > > > >> at the voltage the hardware needs to be stable.
> > > > >>
> > > > >> Is hacking the voltage higher (although ideally not quite that high)
> > > > >> an acceptable short term solution until we have
> > > > >> CPR? Or would it be safer to just not make use of the higher
> > > > >> frequencies on a630 for now?
> > > > >>
> > > > >
> > > > > tbh, I'm not sure about the regulator stuff and CPR.. Bjorn is already
> > > > > on CC and I added sboyd, maybe one of them knows better.
> > > > >
> > > > > In the short term, removing the higher problematic OPPs from dts might
> > > > > be a better option than this patch (which I'm dropping), since there
> > > > > is nothing stopping other workloads from hitting higher OPPs.
> > > > Oh yeah that sounds like a more sensible workaround than mine .
> > > > >
> > > > > I'm slightly curious why I didn't have problems at higher OPPs on my
> > > > > c630 laptop (sdm850)
> > > > Perhaps you won the sillicon lottery - iirc sdm850 is binned for higher
> > > > clocks as is out of the factory.
> > > >
> > > > Would it be best to drop the OPPs for all devices? Or just those
> > > > affected? I guess it's possible another c630 might
> > > > crash where yours doesn't?
> > >
> > > I've not heard any reports of similar issues from the handful of other
> > > folks with c630's on #aarch64-laptops.. but I can't really say if that
> > > is luck or not.
> > >
> > > Maybe just remove it for affected devices? But I'll defer to Bjorn.
> >
> > Just as another datapoint, I was just marveling at how suddenly smooth
> > the UI was performing on db845c and Caleb pointed me at the "drm/msm:
> > Devfreq tuning" patch as the likely cause of the improvement, and
> > mid-discussion my board crashed into USB crash mode:
> > [ 146.157696][C0] adreno 500.gpu: CP | AHB bus error
> > [ 146.163303][C0] adreno 500.gpu: CP | AHB bus error
> > [ 146.168837][C0] adreno 500.gpu: RBBM | ATB bus overflow
> > [ 146.174960][C0] adreno 500.gpu: CP | HW fault | status=0x
> > [ 146.181917][C0] adreno 500.gpu: CP | AHB bus error
> > [ 146.187547][C0] adreno 500.gpu: CP illegal instruction error
> > [ 146.194009][C0] adreno 500.gpu: CP | AHB bus error
> > [ 146.308909][T9] Internal error: synchronous external abort:
> > 9610 [#1] PREEMPT SMP
> > [ 146.317150][T9] Modules linked in:
> > [ 146.320941][T9] CPU: 3 PID: 9 Comm: kworker/u16:1 Tainted: G
> > W 5.14.0-mainline-06795-g42b258c2275c #24
> > [ 146.331974][T9] Hardware name: Thundercomm Dragonboar
> > Format: Log Type - Time(microsec) - Message - Optional Info
> > Log Type: B - Since Boot(Power On Reset), D - Delta, S - Statistic
> > S - QC_IMAGE_VERSION_STRING=BOOT.XF.2.0-00371-SDM845LZB-1
> > S - IMAGE_VARIANT_STRING=SDM845LA
> > S - OEM_IMAGE_VERSION_STRING=TSBJ-FA-PC-02170
> >
> > So Caleb sent me to this thread. :)
> >
> > I'm still trying to trip it again, but it does seem like db845c is
> > also seeing some stability issues with Linus' HEAD.
> >
>
> Caleb's original pastebin seems to have expired (or at least require
> some sort of ubuntu login to access).. were the crashes he was seeing
> also 'AHB bus error'?
I can reproduce this hard crash
https://www.irccloud.com/pastebin/Cu6UJntE/ and a gpu lockup
https://www.irccloud.com/pastebin/6Ryd2Pug/ at times reliably, by
running antutu benchmark on pocof1.
Reverting 9bc95570175a ("drm/msm: Devfreq tuning") helps and I no
longer see these errors.
Complete dmesg for hardcrash https://pastebin.com/raw/GLZVQFQN
Regards,
Amit Pundir
>
> If you
Re: [PATCH] drm/msm: Disable frequency clamping on a630
On Fri, Sep 3, 2021 at 12:39 PM John Stultz wrote:
>
> On Thu, Jul 29, 2021 at 1:49 PM Rob Clark wrote:
> > On Thu, Jul 29, 2021 at 1:28 PM Caleb Connolly
> > wrote:
> > > On 29/07/2021 21:24, Rob Clark wrote:
> > > > On Thu, Jul 29, 2021 at 1:06 PM Caleb Connolly
> > > > wrote:
> > > >>
> > > >> Hi Rob,
> > > >>
> > > >> I've done some more testing! It looks like before that patch
> > > >> ("drm/msm: Devfreq tuning") the GPU would never get above
> > > >> the second frequency in the OPP table (342MHz) (at least, not in
> > > >> glxgears). With the patch applied it would more
> > > >> aggressively jump up to the max frequency which seems to be unstable
> > > >> at the default regulator voltages.
> > > >
> > > > *ohh*, yeah, ok, that would explain it
> > > >
> > > >> Hacking the pm8005 s1 regulator (which provides VDD_GFX) up to 0.988v
> > > >> (instead of the stock 0.516v) makes the GPU stable
> > > >> at the higher frequencies.
> > > >>
> > > >> Applying this patch reverts the behaviour, and the GPU never goes
> > > >> above 342MHz in glxgears, losing ~30% performance in
> > > >> glxgear.
> > > >>
> > > >> I think (?) that enabling CPR support would be the proper solution to
> > > >> this - that would ensure that the regulators run
> > > >> at the voltage the hardware needs to be stable.
> > > >>
> > > >> Is hacking the voltage higher (although ideally not quite that high)
> > > >> an acceptable short term solution until we have
> > > >> CPR? Or would it be safer to just not make use of the higher
> > > >> frequencies on a630 for now?
> > > >>
> > > >
> > > > tbh, I'm not sure about the regulator stuff and CPR.. Bjorn is already
> > > > on CC and I added sboyd, maybe one of them knows better.
> > > >
> > > > In the short term, removing the higher problematic OPPs from dts might
> > > > be a better option than this patch (which I'm dropping), since there
> > > > is nothing stopping other workloads from hitting higher OPPs.
> > > Oh yeah that sounds like a more sensible workaround than mine .
> > > >
> > > > I'm slightly curious why I didn't have problems at higher OPPs on my
> > > > c630 laptop (sdm850)
> > > Perhaps you won the sillicon lottery - iirc sdm850 is binned for higher
> > > clocks as is out of the factory.
> > >
> > > Would it be best to drop the OPPs for all devices? Or just those
> > > affected? I guess it's possible another c630 might
> > > crash where yours doesn't?
> >
> > I've not heard any reports of similar issues from the handful of other
> > folks with c630's on #aarch64-laptops.. but I can't really say if that
> > is luck or not.
> >
> > Maybe just remove it for affected devices? But I'll defer to Bjorn.
>
> Just as another datapoint, I was just marveling at how suddenly smooth
> the UI was performing on db845c and Caleb pointed me at the "drm/msm:
> Devfreq tuning" patch as the likely cause of the improvement, and
> mid-discussion my board crashed into USB crash mode:
> [ 146.157696][C0] adreno 500.gpu: CP | AHB bus error
> [ 146.163303][C0] adreno 500.gpu: CP | AHB bus error
> [ 146.168837][C0] adreno 500.gpu: RBBM | ATB bus overflow
> [ 146.174960][C0] adreno 500.gpu: CP | HW fault | status=0x
> [ 146.181917][C0] adreno 500.gpu: CP | AHB bus error
> [ 146.187547][C0] adreno 500.gpu: CP illegal instruction error
> [ 146.194009][C0] adreno 500.gpu: CP | AHB bus error
> [ 146.308909][T9] Internal error: synchronous external abort:
> 9610 [#1] PREEMPT SMP
> [ 146.317150][T9] Modules linked in:
> [ 146.320941][T9] CPU: 3 PID: 9 Comm: kworker/u16:1 Tainted: G
> W 5.14.0-mainline-06795-g42b258c2275c #24
> [ 146.331974][T9] Hardware name: Thundercomm Dragonboar
> Format: Log Type - Time(microsec) - Message - Optional Info
> Log Type: B - Since Boot(Power On Reset), D - Delta, S - Statistic
> S - QC_IMAGE_VERSION_STRING=BOOT.XF.2.0-00371-SDM845LZB-1
> S - IMAGE_VARIANT_STRING=SDM845LA
> S - OEM_IMAGE_VERSION_STRING=TSBJ-FA-PC-02170
>
> So Caleb sent me to this thread. :)
>
> I'm still trying to trip it again, but it does seem like db845c is
> also seeing some stability issues with Linus' HEAD.
>
Caleb's original pastebin seems to have expired (or at least require
some sort of ubuntu login to access).. were the crashes he was seeing
also 'AHB bus error'?
If you have a reliable reproducer, I guess it would be worth seeing if
increasing the min_freq (ie. to limit how far we jump the freq in one
shot) "fixes" it?
I guess I could check downstream kgsl to see if they were doing
something to increase freq in smaller increments.. I don't recall that
they were but it has been a while since I dug thru that code. And I
suppose downstream it could also be done in their custom tz governor.
BR,
-R
Re: [PATCH] drm/msm: Disable frequency clamping on a630
On Thu, Jul 29, 2021 at 1:49 PM Rob Clark wrote:
> On Thu, Jul 29, 2021 at 1:28 PM Caleb Connolly
> wrote:
> > On 29/07/2021 21:24, Rob Clark wrote:
> > > On Thu, Jul 29, 2021 at 1:06 PM Caleb Connolly
> > > wrote:
> > >>
> > >> Hi Rob,
> > >>
> > >> I've done some more testing! It looks like before that patch ("drm/msm:
> > >> Devfreq tuning") the GPU would never get above
> > >> the second frequency in the OPP table (342MHz) (at least, not in
> > >> glxgears). With the patch applied it would more
> > >> aggressively jump up to the max frequency which seems to be unstable at
> > >> the default regulator voltages.
> > >
> > > *ohh*, yeah, ok, that would explain it
> > >
> > >> Hacking the pm8005 s1 regulator (which provides VDD_GFX) up to 0.988v
> > >> (instead of the stock 0.516v) makes the GPU stable
> > >> at the higher frequencies.
> > >>
> > >> Applying this patch reverts the behaviour, and the GPU never goes above
> > >> 342MHz in glxgears, losing ~30% performance in
> > >> glxgear.
> > >>
> > >> I think (?) that enabling CPR support would be the proper solution to
> > >> this - that would ensure that the regulators run
> > >> at the voltage the hardware needs to be stable.
> > >>
> > >> Is hacking the voltage higher (although ideally not quite that high) an
> > >> acceptable short term solution until we have
> > >> CPR? Or would it be safer to just not make use of the higher frequencies
> > >> on a630 for now?
> > >>
> > >
> > > tbh, I'm not sure about the regulator stuff and CPR.. Bjorn is already
> > > on CC and I added sboyd, maybe one of them knows better.
> > >
> > > In the short term, removing the higher problematic OPPs from dts might
> > > be a better option than this patch (which I'm dropping), since there
> > > is nothing stopping other workloads from hitting higher OPPs.
> > Oh yeah that sounds like a more sensible workaround than mine .
> > >
> > > I'm slightly curious why I didn't have problems at higher OPPs on my
> > > c630 laptop (sdm850)
> > Perhaps you won the sillicon lottery - iirc sdm850 is binned for higher
> > clocks as is out of the factory.
> >
> > Would it be best to drop the OPPs for all devices? Or just those affected?
> > I guess it's possible another c630 might
> > crash where yours doesn't?
>
> I've not heard any reports of similar issues from the handful of other
> folks with c630's on #aarch64-laptops.. but I can't really say if that
> is luck or not.
>
> Maybe just remove it for affected devices? But I'll defer to Bjorn.
Just as another datapoint, I was just marveling at how suddenly smooth
the UI was performing on db845c and Caleb pointed me at the "drm/msm:
Devfreq tuning" patch as the likely cause of the improvement, and
mid-discussion my board crashed into USB crash mode:
[ 146.157696][C0] adreno 500.gpu: CP | AHB bus error
[ 146.163303][C0] adreno 500.gpu: CP | AHB bus error
[ 146.168837][C0] adreno 500.gpu: RBBM | ATB bus overflow
[ 146.174960][C0] adreno 500.gpu: CP | HW fault | status=0x
[ 146.181917][C0] adreno 500.gpu: CP | AHB bus error
[ 146.187547][C0] adreno 500.gpu: CP illegal instruction error
[ 146.194009][C0] adreno 500.gpu: CP | AHB bus error
[ 146.308909][T9] Internal error: synchronous external abort:
9610 [#1] PREEMPT SMP
[ 146.317150][T9] Modules linked in:
[ 146.320941][T9] CPU: 3 PID: 9 Comm: kworker/u16:1 Tainted: G
W 5.14.0-mainline-06795-g42b258c2275c #24
[ 146.331974][T9] Hardware name: Thundercomm Dragonboar
Format: Log Type - Time(microsec) - Message - Optional Info
Log Type: B - Since Boot(Power On Reset), D - Delta, S - Statistic
S - QC_IMAGE_VERSION_STRING=BOOT.XF.2.0-00371-SDM845LZB-1
S - IMAGE_VARIANT_STRING=SDM845LA
S - OEM_IMAGE_VERSION_STRING=TSBJ-FA-PC-02170
So Caleb sent me to this thread. :)
I'm still trying to trip it again, but it does seem like db845c is
also seeing some stability issues with Linus' HEAD.
thanks
-john
Re: [PATCH] drm/msm: Disable frequency clamping on a630
On Mon, Aug 9, 2021 at 1:35 PM Caleb Connolly wrote:
>
>
>
> On 09/08/2021 18:58, Rob Clark wrote:
> > On Mon, Aug 9, 2021 at 10:28 AM Akhil P Oommen
> > wrote:
> >>
> >> On 8/9/2021 9:48 PM, Caleb Connolly wrote:
> >>>
> >>>
> >>> On 09/08/2021 17:12, Rob Clark wrote:
> On Mon, Aug 9, 2021 at 7:52 AM Akhil P Oommen
> wrote:
> >
> > On 8/8/2021 10:22 PM, Rob Clark wrote:
> >> On Sun, Aug 8, 2021 at 7:33 AM Caleb Connolly
> >> wrote:
> >>>
> >>>
> >>>
> >>> On 07/08/2021 21:04, Rob Clark wrote:
> On Sat, Aug 7, 2021 at 12:21 PM Caleb Connolly
> wrote:
> >
> > Hi Rob, Akhil,
> >
> > On 29/07/2021 21:53, Rob Clark wrote:
> >> On Thu, Jul 29, 2021 at 1:28 PM Caleb Connolly
> >> wrote:
> >>>
> >>>
> >>>
> >>> On 29/07/2021 21:24, Rob Clark wrote:
> On Thu, Jul 29, 2021 at 1:06 PM Caleb Connolly
> wrote:
> >
> > Hi Rob,
> >
> > I've done some more testing! It looks like before that patch
> > ("drm/msm: Devfreq tuning") the GPU would never get above
> > the second frequency in the OPP table (342MHz) (at least, not
> > in glxgears). With the patch applied it would more
> > aggressively jump up to the max frequency which seems to be
> > unstable at the default regulator voltages.
>
> *ohh*, yeah, ok, that would explain it
>
> > Hacking the pm8005 s1 regulator (which provides VDD_GFX) up
> > to 0.988v (instead of the stock 0.516v) makes the GPU stable
> > at the higher frequencies.
> >
> > Applying this patch reverts the behaviour, and the GPU never
> > goes above 342MHz in glxgears, losing ~30% performance in
> > glxgear.
> >
> > I think (?) that enabling CPR support would be the proper
> > solution to this - that would ensure that the regulators run
> > at the voltage the hardware needs to be stable.
> >
> > Is hacking the voltage higher (although ideally not quite
> > that high) an acceptable short term solution until we have
> > CPR? Or would it be safer to just not make use of the higher
> > frequencies on a630 for now?
> >
>
> tbh, I'm not sure about the regulator stuff and CPR.. Bjorn is
> already
> on CC and I added sboyd, maybe one of them knows better.
>
> In the short term, removing the higher problematic OPPs from
> dts might
> be a better option than this patch (which I'm dropping), since
> there
> is nothing stopping other workloads from hitting higher OPPs.
> >>> Oh yeah that sounds like a more sensible workaround than mine .
>
> I'm slightly curious why I didn't have problems at higher OPPs
> on my
> c630 laptop (sdm850)
> >>> Perhaps you won the sillicon lottery - iirc sdm850 is binned
> >>> for higher clocks as is out of the factory.
> >>>
> >>> Would it be best to drop the OPPs for all devices? Or just
> >>> those affected? I guess it's possible another c630 might
> >>> crash where yours doesn't?
> >>
> >> I've not heard any reports of similar issues from the handful of
> >> other
> >> folks with c630's on #aarch64-laptops.. but I can't really say
> >> if that
> >> is luck or not.
> > It looks like this affects at least the OnePlus 6 and PocoPhone
> > F1, I've done some more poking and the following diff
> > seems to fix the stability issues completely, it seems the delay
> > is required to let the update propagate.
> >
> > This doesn't feel like the right fix, but hopefully it's enough
> > to come up with a better solution than disabling the new
> > devfreq behaviour on a630.
> >
> > diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> > b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> > index d7cec7f0dde0..69e2a5e84dae 100644
> > --- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> > +++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> > @@ -139,6 +139,10 @@ void a6xx_gmu_set_freq(struct msm_gpu *gpu,
> > struct dev_pm_opp *opp)
> > return;
> > }
> >
> > + dev_pm_opp_set_opp(>pdev->dev, opp);
> > +
> > + usleep_range(300, 500);
> > +
>
> >
> > I am a bit confused. We don't define a power domain for gpu in dt,
> > correct? Then what exactly set_opp do here? Do you think this
Re: [PATCH] drm/msm: Disable frequency clamping on a630
On 09/08/2021 18:58, Rob Clark wrote:
On Mon, Aug 9, 2021 at 10:28 AM Akhil P Oommen wrote:
On 8/9/2021 9:48 PM, Caleb Connolly wrote:
On 09/08/2021 17:12, Rob Clark wrote:
On Mon, Aug 9, 2021 at 7:52 AM Akhil P Oommen
wrote:
On 8/8/2021 10:22 PM, Rob Clark wrote:
On Sun, Aug 8, 2021 at 7:33 AM Caleb Connolly
wrote:
On 07/08/2021 21:04, Rob Clark wrote:
On Sat, Aug 7, 2021 at 12:21 PM Caleb Connolly
wrote:
Hi Rob, Akhil,
On 29/07/2021 21:53, Rob Clark wrote:
On Thu, Jul 29, 2021 at 1:28 PM Caleb Connolly
wrote:
On 29/07/2021 21:24, Rob Clark wrote:
On Thu, Jul 29, 2021 at 1:06 PM Caleb Connolly
wrote:
Hi Rob,
I've done some more testing! It looks like before that patch
("drm/msm: Devfreq tuning") the GPU would never get above
the second frequency in the OPP table (342MHz) (at least, not
in glxgears). With the patch applied it would more
aggressively jump up to the max frequency which seems to be
unstable at the default regulator voltages.
*ohh*, yeah, ok, that would explain it
Hacking the pm8005 s1 regulator (which provides VDD_GFX) up
to 0.988v (instead of the stock 0.516v) makes the GPU stable
at the higher frequencies.
Applying this patch reverts the behaviour, and the GPU never
goes above 342MHz in glxgears, losing ~30% performance in
glxgear.
I think (?) that enabling CPR support would be the proper
solution to this - that would ensure that the regulators run
at the voltage the hardware needs to be stable.
Is hacking the voltage higher (although ideally not quite
that high) an acceptable short term solution until we have
CPR? Or would it be safer to just not make use of the higher
frequencies on a630 for now?
tbh, I'm not sure about the regulator stuff and CPR.. Bjorn is
already
on CC and I added sboyd, maybe one of them knows better.
In the short term, removing the higher problematic OPPs from
dts might
be a better option than this patch (which I'm dropping), since
there
is nothing stopping other workloads from hitting higher OPPs.
Oh yeah that sounds like a more sensible workaround than mine .
I'm slightly curious why I didn't have problems at higher OPPs
on my
c630 laptop (sdm850)
Perhaps you won the sillicon lottery - iirc sdm850 is binned
for higher clocks as is out of the factory.
Would it be best to drop the OPPs for all devices? Or just
those affected? I guess it's possible another c630 might
crash where yours doesn't?
I've not heard any reports of similar issues from the handful of
other
folks with c630's on #aarch64-laptops.. but I can't really say
if that
is luck or not.
It looks like this affects at least the OnePlus 6 and PocoPhone
F1, I've done some more poking and the following diff
seems to fix the stability issues completely, it seems the delay
is required to let the update propagate.
This doesn't feel like the right fix, but hopefully it's enough
to come up with a better solution than disabling the new
devfreq behaviour on a630.
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
index d7cec7f0dde0..69e2a5e84dae 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
@@ -139,6 +139,10 @@ void a6xx_gmu_set_freq(struct msm_gpu *gpu,
struct dev_pm_opp *opp)
return;
}
+ dev_pm_opp_set_opp(>pdev->dev, opp);
+
+ usleep_range(300, 500);
+
I am a bit confused. We don't define a power domain for gpu in dt,
correct? Then what exactly set_opp do here? Do you think this usleep is
what is helping here somehow to mask the issue?
The power domains (for cx and gx) are defined in the GMU DT, the OPPs in
the GPU DT. For the sake of simplicity I'll refer to the lowest
frequency (25700) and OPP level (RPMH_REGULATOR_LEVEL_LOW_SVS) as
the "min" state, and the highest frequency (71000) and OPP level
(RPMH_REGULATOR_LEVEL_TURBO_L1) as the "max" state. These are defined in
sdm845.dtsi under the gpu node.
The new devfreq behaviour unmasks what I think is a driver bug, it
inadvertently puts much more strain on the GPU regulators than they
usually get. With the new behaviour the GPU jumps from it's min state to
the max state and back again extremely rapidly under workloads as small
as refreshing UI. Where previously the GPU would rarely if ever go above
342MHz when interacting with the device, it now jumps between min and
max many times per second.
If my understanding is correct, the current implementation of the GMU
set freq is the following:
- Get OPP for frequency to set
- Push the frequency to the GMU - immediately updating the core clock
- Call dev_pm_opp_set_opp() which triggers a notify chain, this winds
up somewhere in power management code and causes the gx regulator level
to be updated
Nope. dev_pm_opp_set_opp() sets the bandwidth for gpu and nothing else.
We were using a different api earlier which got deprecated -
dev_pm_opp_set_bw().
Huh ok, thanks for the correction. So it's the GMU writes
Re: [PATCH] drm/msm: Disable frequency clamping on a630
On Mon, Aug 9, 2021 at 10:28 AM Akhil P Oommen wrote:
>
> On 8/9/2021 9:48 PM, Caleb Connolly wrote:
> >
> >
> > On 09/08/2021 17:12, Rob Clark wrote:
> >> On Mon, Aug 9, 2021 at 7:52 AM Akhil P Oommen
> >> wrote:
> >>>
> >>> On 8/8/2021 10:22 PM, Rob Clark wrote:
> On Sun, Aug 8, 2021 at 7:33 AM Caleb Connolly
> wrote:
> >
> >
> >
> > On 07/08/2021 21:04, Rob Clark wrote:
> >> On Sat, Aug 7, 2021 at 12:21 PM Caleb Connolly
> >> wrote:
> >>>
> >>> Hi Rob, Akhil,
> >>>
> >>> On 29/07/2021 21:53, Rob Clark wrote:
> On Thu, Jul 29, 2021 at 1:28 PM Caleb Connolly
> wrote:
> >
> >
> >
> > On 29/07/2021 21:24, Rob Clark wrote:
> >> On Thu, Jul 29, 2021 at 1:06 PM Caleb Connolly
> >> wrote:
> >>>
> >>> Hi Rob,
> >>>
> >>> I've done some more testing! It looks like before that patch
> >>> ("drm/msm: Devfreq tuning") the GPU would never get above
> >>> the second frequency in the OPP table (342MHz) (at least, not
> >>> in glxgears). With the patch applied it would more
> >>> aggressively jump up to the max frequency which seems to be
> >>> unstable at the default regulator voltages.
> >>
> >> *ohh*, yeah, ok, that would explain it
> >>
> >>> Hacking the pm8005 s1 regulator (which provides VDD_GFX) up
> >>> to 0.988v (instead of the stock 0.516v) makes the GPU stable
> >>> at the higher frequencies.
> >>>
> >>> Applying this patch reverts the behaviour, and the GPU never
> >>> goes above 342MHz in glxgears, losing ~30% performance in
> >>> glxgear.
> >>>
> >>> I think (?) that enabling CPR support would be the proper
> >>> solution to this - that would ensure that the regulators run
> >>> at the voltage the hardware needs to be stable.
> >>>
> >>> Is hacking the voltage higher (although ideally not quite
> >>> that high) an acceptable short term solution until we have
> >>> CPR? Or would it be safer to just not make use of the higher
> >>> frequencies on a630 for now?
> >>>
> >>
> >> tbh, I'm not sure about the regulator stuff and CPR.. Bjorn is
> >> already
> >> on CC and I added sboyd, maybe one of them knows better.
> >>
> >> In the short term, removing the higher problematic OPPs from
> >> dts might
> >> be a better option than this patch (which I'm dropping), since
> >> there
> >> is nothing stopping other workloads from hitting higher OPPs.
> > Oh yeah that sounds like a more sensible workaround than mine .
> >>
> >> I'm slightly curious why I didn't have problems at higher OPPs
> >> on my
> >> c630 laptop (sdm850)
> > Perhaps you won the sillicon lottery - iirc sdm850 is binned
> > for higher clocks as is out of the factory.
> >
> > Would it be best to drop the OPPs for all devices? Or just
> > those affected? I guess it's possible another c630 might
> > crash where yours doesn't?
>
> I've not heard any reports of similar issues from the handful of
> other
> folks with c630's on #aarch64-laptops.. but I can't really say
> if that
> is luck or not.
> >>> It looks like this affects at least the OnePlus 6 and PocoPhone
> >>> F1, I've done some more poking and the following diff
> >>> seems to fix the stability issues completely, it seems the delay
> >>> is required to let the update propagate.
> >>>
> >>> This doesn't feel like the right fix, but hopefully it's enough
> >>> to come up with a better solution than disabling the new
> >>> devfreq behaviour on a630.
> >>>
> >>> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> >>> b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> >>> index d7cec7f0dde0..69e2a5e84dae 100644
> >>> --- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> >>> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> >>> @@ -139,6 +139,10 @@ void a6xx_gmu_set_freq(struct msm_gpu *gpu,
> >>> struct dev_pm_opp *opp)
> >>> return;
> >>> }
> >>>
> >>> + dev_pm_opp_set_opp(>pdev->dev, opp);
> >>> +
> >>> + usleep_range(300, 500);
> >>> +
> >>
> >>>
> >>> I am a bit confused. We don't define a power domain for gpu in dt,
> >>> correct? Then what exactly set_opp do here? Do you think this usleep is
> >>> what is helping here somehow to mask the issue?
> > The power domains (for cx and gx) are defined in the GMU DT, the OPPs in
> > the GPU DT. For the sake of simplicity I'll refer to the lowest
> > frequency (25700) and OPP level (RPMH_REGULATOR_LEVEL_LOW_SVS) as
> > the "min" state, and the highest frequency
Re: [PATCH] drm/msm: Disable frequency clamping on a630
On 8/9/2021 9:48 PM, Caleb Connolly wrote:
On 09/08/2021 17:12, Rob Clark wrote:
On Mon, Aug 9, 2021 at 7:52 AM Akhil P Oommen
wrote:
On 8/8/2021 10:22 PM, Rob Clark wrote:
On Sun, Aug 8, 2021 at 7:33 AM Caleb Connolly
wrote:
On 07/08/2021 21:04, Rob Clark wrote:
On Sat, Aug 7, 2021 at 12:21 PM Caleb Connolly
wrote:
Hi Rob, Akhil,
On 29/07/2021 21:53, Rob Clark wrote:
On Thu, Jul 29, 2021 at 1:28 PM Caleb Connolly
wrote:
On 29/07/2021 21:24, Rob Clark wrote:
On Thu, Jul 29, 2021 at 1:06 PM Caleb Connolly
wrote:
Hi Rob,
I've done some more testing! It looks like before that patch
("drm/msm: Devfreq tuning") the GPU would never get above
the second frequency in the OPP table (342MHz) (at least, not
in glxgears). With the patch applied it would more
aggressively jump up to the max frequency which seems to be
unstable at the default regulator voltages.
*ohh*, yeah, ok, that would explain it
Hacking the pm8005 s1 regulator (which provides VDD_GFX) up
to 0.988v (instead of the stock 0.516v) makes the GPU stable
at the higher frequencies.
Applying this patch reverts the behaviour, and the GPU never
goes above 342MHz in glxgears, losing ~30% performance in
glxgear.
I think (?) that enabling CPR support would be the proper
solution to this - that would ensure that the regulators run
at the voltage the hardware needs to be stable.
Is hacking the voltage higher (although ideally not quite
that high) an acceptable short term solution until we have
CPR? Or would it be safer to just not make use of the higher
frequencies on a630 for now?
tbh, I'm not sure about the regulator stuff and CPR.. Bjorn is
already
on CC and I added sboyd, maybe one of them knows better.
In the short term, removing the higher problematic OPPs from
dts might
be a better option than this patch (which I'm dropping), since
there
is nothing stopping other workloads from hitting higher OPPs.
Oh yeah that sounds like a more sensible workaround than mine .
I'm slightly curious why I didn't have problems at higher OPPs
on my
c630 laptop (sdm850)
Perhaps you won the sillicon lottery - iirc sdm850 is binned
for higher clocks as is out of the factory.
Would it be best to drop the OPPs for all devices? Or just
those affected? I guess it's possible another c630 might
crash where yours doesn't?
I've not heard any reports of similar issues from the handful of
other
folks with c630's on #aarch64-laptops.. but I can't really say
if that
is luck or not.
It looks like this affects at least the OnePlus 6 and PocoPhone
F1, I've done some more poking and the following diff
seems to fix the stability issues completely, it seems the delay
is required to let the update propagate.
This doesn't feel like the right fix, but hopefully it's enough
to come up with a better solution than disabling the new
devfreq behaviour on a630.
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
index d7cec7f0dde0..69e2a5e84dae 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
@@ -139,6 +139,10 @@ void a6xx_gmu_set_freq(struct msm_gpu *gpu,
struct dev_pm_opp *opp)
return;
}
+ dev_pm_opp_set_opp(>pdev->dev, opp);
+
+ usleep_range(300, 500);
+
I am a bit confused. We don't define a power domain for gpu in dt,
correct? Then what exactly set_opp do here? Do you think this usleep is
what is helping here somehow to mask the issue?
The power domains (for cx and gx) are defined in the GMU DT, the OPPs in
the GPU DT. For the sake of simplicity I'll refer to the lowest
frequency (25700) and OPP level (RPMH_REGULATOR_LEVEL_LOW_SVS) as
the "min" state, and the highest frequency (71000) and OPP level
(RPMH_REGULATOR_LEVEL_TURBO_L1) as the "max" state. These are defined in
sdm845.dtsi under the gpu node.
The new devfreq behaviour unmasks what I think is a driver bug, it
inadvertently puts much more strain on the GPU regulators than they
usually get. With the new behaviour the GPU jumps from it's min state to
the max state and back again extremely rapidly under workloads as small
as refreshing UI. Where previously the GPU would rarely if ever go above
342MHz when interacting with the device, it now jumps between min and
max many times per second.
If my understanding is correct, the current implementation of the GMU
set freq is the following:
- Get OPP for frequency to set
- Push the frequency to the GMU - immediately updating the core clock
- Call dev_pm_opp_set_opp() which triggers a notify chain, this winds
up somewhere in power management code and causes the gx regulator level
to be updated
Nope. dev_pm_opp_set_opp() sets the bandwidth for gpu and nothing else.
We were using a different api earlier which got deprecated -
dev_pm_opp_set_bw().
The regulator will then take some time to reach it's new voltage level
and stabilise. I believe that rapid
Re: [PATCH] drm/msm: Disable frequency clamping on a630
On 09/08/2021 17:12, Rob Clark wrote:
On Mon, Aug 9, 2021 at 7:52 AM Akhil P Oommen wrote:
On 8/8/2021 10:22 PM, Rob Clark wrote:
On Sun, Aug 8, 2021 at 7:33 AM Caleb Connolly wrote:
On 07/08/2021 21:04, Rob Clark wrote:
On Sat, Aug 7, 2021 at 12:21 PM Caleb Connolly
wrote:
Hi Rob, Akhil,
On 29/07/2021 21:53, Rob Clark wrote:
On Thu, Jul 29, 2021 at 1:28 PM Caleb Connolly
wrote:
On 29/07/2021 21:24, Rob Clark wrote:
On Thu, Jul 29, 2021 at 1:06 PM Caleb Connolly
wrote:
Hi Rob,
I've done some more testing! It looks like before that patch ("drm/msm: Devfreq
tuning") the GPU would never get above
the second frequency in the OPP table (342MHz) (at least, not in glxgears).
With the patch applied it would more
aggressively jump up to the max frequency which seems to be unstable at the
default regulator voltages.
*ohh*, yeah, ok, that would explain it
Hacking the pm8005 s1 regulator (which provides VDD_GFX) up to 0.988v (instead
of the stock 0.516v) makes the GPU stable
at the higher frequencies.
Applying this patch reverts the behaviour, and the GPU never goes above 342MHz
in glxgears, losing ~30% performance in
glxgear.
I think (?) that enabling CPR support would be the proper solution to this -
that would ensure that the regulators run
at the voltage the hardware needs to be stable.
Is hacking the voltage higher (although ideally not quite that high) an
acceptable short term solution until we have
CPR? Or would it be safer to just not make use of the higher frequencies on
a630 for now?
tbh, I'm not sure about the regulator stuff and CPR.. Bjorn is already
on CC and I added sboyd, maybe one of them knows better.
In the short term, removing the higher problematic OPPs from dts might
be a better option than this patch (which I'm dropping), since there
is nothing stopping other workloads from hitting higher OPPs.
Oh yeah that sounds like a more sensible workaround than mine .
I'm slightly curious why I didn't have problems at higher OPPs on my
c630 laptop (sdm850)
Perhaps you won the sillicon lottery - iirc sdm850 is binned for higher clocks
as is out of the factory.
Would it be best to drop the OPPs for all devices? Or just those affected? I
guess it's possible another c630 might
crash where yours doesn't?
I've not heard any reports of similar issues from the handful of other
folks with c630's on #aarch64-laptops.. but I can't really say if that
is luck or not.
It looks like this affects at least the OnePlus 6 and PocoPhone F1, I've done
some more poking and the following diff
seems to fix the stability issues completely, it seems the delay is required to
let the update propagate.
This doesn't feel like the right fix, but hopefully it's enough to come up with
a better solution than disabling the new
devfreq behaviour on a630.
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
index d7cec7f0dde0..69e2a5e84dae 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
@@ -139,6 +139,10 @@ void a6xx_gmu_set_freq(struct msm_gpu *gpu, struct
dev_pm_opp *opp)
return;
}
+ dev_pm_opp_set_opp(>pdev->dev, opp);
+
+ usleep_range(300, 500);
+
I am a bit confused. We don't define a power domain for gpu in dt,
correct? Then what exactly set_opp do here? Do you think this usleep is
what is helping here somehow to mask the issue?
The power domains (for cx and gx) are defined in the GMU DT, the OPPs in the GPU DT. For the sake of simplicity I'll
refer to the lowest frequency (25700) and OPP level (RPMH_REGULATOR_LEVEL_LOW_SVS) as the "min" state, and the
highest frequency (71000) and OPP level (RPMH_REGULATOR_LEVEL_TURBO_L1) as the "max" state. These are defined in
sdm845.dtsi under the gpu node.
The new devfreq behaviour unmasks what I think is a driver bug, it inadvertently puts much more strain on the GPU
regulators than they usually get. With the new behaviour the GPU jumps from it's min state to the max state and back
again extremely rapidly under workloads as small as refreshing UI. Where previously the GPU would rarely if ever go
above 342MHz when interacting with the device, it now jumps between min and max many times per second.
If my understanding is correct, the current implementation of the GMU set freq
is the following:
- Get OPP for frequency to set
- Push the frequency to the GMU - immediately updating the core clock
- Call dev_pm_opp_set_opp() which triggers a notify chain, this winds up somewhere in power management code and causes
the gx regulator level to be updated
The regulator will then take some time to reach it's new voltage level and stabilise. I believe that rapid transitions
between min and max state - in combination with the increased current load from the GPU core - lead to the regulator
becoming unstable (e.g. when it's requested to transition from it's lowest to highest levels
Re: [PATCH] drm/msm: Disable frequency clamping on a630
On Mon, Aug 9, 2021 at 7:52 AM Akhil P Oommen wrote:
>
> On 8/8/2021 10:22 PM, Rob Clark wrote:
> > On Sun, Aug 8, 2021 at 7:33 AM Caleb Connolly
> > wrote:
> >>
> >>
> >>
> >> On 07/08/2021 21:04, Rob Clark wrote:
> >>> On Sat, Aug 7, 2021 at 12:21 PM Caleb Connolly
> >>> wrote:
>
> Hi Rob, Akhil,
>
> On 29/07/2021 21:53, Rob Clark wrote:
> > On Thu, Jul 29, 2021 at 1:28 PM Caleb Connolly
> > wrote:
> >>
> >>
> >>
> >> On 29/07/2021 21:24, Rob Clark wrote:
> >>> On Thu, Jul 29, 2021 at 1:06 PM Caleb Connolly
> >>> wrote:
>
> Hi Rob,
>
> I've done some more testing! It looks like before that patch
> ("drm/msm: Devfreq tuning") the GPU would never get above
> the second frequency in the OPP table (342MHz) (at least, not in
> glxgears). With the patch applied it would more
> aggressively jump up to the max frequency which seems to be unstable
> at the default regulator voltages.
> >>>
> >>> *ohh*, yeah, ok, that would explain it
> >>>
> Hacking the pm8005 s1 regulator (which provides VDD_GFX) up to
> 0.988v (instead of the stock 0.516v) makes the GPU stable
> at the higher frequencies.
>
> Applying this patch reverts the behaviour, and the GPU never goes
> above 342MHz in glxgears, losing ~30% performance in
> glxgear.
>
> I think (?) that enabling CPR support would be the proper solution
> to this - that would ensure that the regulators run
> at the voltage the hardware needs to be stable.
>
> Is hacking the voltage higher (although ideally not quite that high)
> an acceptable short term solution until we have
> CPR? Or would it be safer to just not make use of the higher
> frequencies on a630 for now?
>
> >>>
> >>> tbh, I'm not sure about the regulator stuff and CPR.. Bjorn is already
> >>> on CC and I added sboyd, maybe one of them knows better.
> >>>
> >>> In the short term, removing the higher problematic OPPs from dts might
> >>> be a better option than this patch (which I'm dropping), since there
> >>> is nothing stopping other workloads from hitting higher OPPs.
> >> Oh yeah that sounds like a more sensible workaround than mine .
> >>>
> >>> I'm slightly curious why I didn't have problems at higher OPPs on my
> >>> c630 laptop (sdm850)
> >> Perhaps you won the sillicon lottery - iirc sdm850 is binned for
> >> higher clocks as is out of the factory.
> >>
> >> Would it be best to drop the OPPs for all devices? Or just those
> >> affected? I guess it's possible another c630 might
> >> crash where yours doesn't?
> >
> > I've not heard any reports of similar issues from the handful of other
> > folks with c630's on #aarch64-laptops.. but I can't really say if that
> > is luck or not.
> It looks like this affects at least the OnePlus 6 and PocoPhone F1, I've
> done some more poking and the following diff
> seems to fix the stability issues completely, it seems the delay is
> required to let the update propagate.
>
> This doesn't feel like the right fix, but hopefully it's enough to come
> up with a better solution than disabling the new
> devfreq behaviour on a630.
>
> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> index d7cec7f0dde0..69e2a5e84dae 100644
> --- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> @@ -139,6 +139,10 @@ void a6xx_gmu_set_freq(struct msm_gpu *gpu, struct
> dev_pm_opp *opp)
> return;
> }
>
> + dev_pm_opp_set_opp(>pdev->dev, opp);
> +
> + usleep_range(300, 500);
> +
> >>>
>
> I am a bit confused. We don't define a power domain for gpu in dt,
> correct? Then what exactly set_opp do here? Do you think this usleep is
> what is helping here somehow to mask the issue?
Hmm, I thought "opp-level = RPMH_REGULATOR_LEVEL_*" did *something*,
but tbh I'm not sure exactly what..
> I feel we should just leave the new dcvs feature (shall we call it NAP?)
> disabled for a630 (and 10ms devfreq interval), until this is root caused.
I suppose "NAP" is a reasonable name.
But I think that reverting to previous behavior would not be enough,
there is nothing stopping devfreq from jumping from min to max freq,
which AFAIU should be enough to trigger this. I guess that there just
hasn't been enough testing with different game workloads on those
phones to trigger this.
That said, I haven't seen similar issues on my sdm850 laptop, where I
defn have triggered mix->max freq transitions.. I guess it would be
interesting to know if this issue could be reproduced on db845c,
Re: [PATCH] drm/msm: Disable frequency clamping on a630
On 8/8/2021 10:22 PM, Rob Clark wrote:
On Sun, Aug 8, 2021 at 7:33 AM Caleb Connolly wrote:
On 07/08/2021 21:04, Rob Clark wrote:
On Sat, Aug 7, 2021 at 12:21 PM Caleb Connolly
wrote:
Hi Rob, Akhil,
On 29/07/2021 21:53, Rob Clark wrote:
On Thu, Jul 29, 2021 at 1:28 PM Caleb Connolly
wrote:
On 29/07/2021 21:24, Rob Clark wrote:
On Thu, Jul 29, 2021 at 1:06 PM Caleb Connolly
wrote:
Hi Rob,
I've done some more testing! It looks like before that patch ("drm/msm: Devfreq
tuning") the GPU would never get above
the second frequency in the OPP table (342MHz) (at least, not in glxgears).
With the patch applied it would more
aggressively jump up to the max frequency which seems to be unstable at the
default regulator voltages.
*ohh*, yeah, ok, that would explain it
Hacking the pm8005 s1 regulator (which provides VDD_GFX) up to 0.988v (instead
of the stock 0.516v) makes the GPU stable
at the higher frequencies.
Applying this patch reverts the behaviour, and the GPU never goes above 342MHz
in glxgears, losing ~30% performance in
glxgear.
I think (?) that enabling CPR support would be the proper solution to this -
that would ensure that the regulators run
at the voltage the hardware needs to be stable.
Is hacking the voltage higher (although ideally not quite that high) an
acceptable short term solution until we have
CPR? Or would it be safer to just not make use of the higher frequencies on
a630 for now?
tbh, I'm not sure about the regulator stuff and CPR.. Bjorn is already
on CC and I added sboyd, maybe one of them knows better.
In the short term, removing the higher problematic OPPs from dts might
be a better option than this patch (which I'm dropping), since there
is nothing stopping other workloads from hitting higher OPPs.
Oh yeah that sounds like a more sensible workaround than mine .
I'm slightly curious why I didn't have problems at higher OPPs on my
c630 laptop (sdm850)
Perhaps you won the sillicon lottery - iirc sdm850 is binned for higher clocks
as is out of the factory.
Would it be best to drop the OPPs for all devices? Or just those affected? I
guess it's possible another c630 might
crash where yours doesn't?
I've not heard any reports of similar issues from the handful of other
folks with c630's on #aarch64-laptops.. but I can't really say if that
is luck or not.
It looks like this affects at least the OnePlus 6 and PocoPhone F1, I've done
some more poking and the following diff
seems to fix the stability issues completely, it seems the delay is required to
let the update propagate.
This doesn't feel like the right fix, but hopefully it's enough to come up with
a better solution than disabling the new
devfreq behaviour on a630.
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
index d7cec7f0dde0..69e2a5e84dae 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
@@ -139,6 +139,10 @@ void a6xx_gmu_set_freq(struct msm_gpu *gpu, struct
dev_pm_opp *opp)
return;
}
+ dev_pm_opp_set_opp(>pdev->dev, opp);
+
+ usleep_range(300, 500);
+
I am a bit confused. We don't define a power domain for gpu in dt,
correct? Then what exactly set_opp do here? Do you think this usleep is
what is helping here somehow to mask the issue?
I feel we should just leave the new dcvs feature (shall we call it NAP?)
disabled for a630 (and 10ms devfreq interval), until this is root caused.
Hmm, this is going to be in the critical path on idle -> active
transition (ie. think response time to user-input).. so we defn don't
want to do this unconditionally..
If I understand the problem, we just want to limit how far we jump the
gpu freq in one go.. maybe deleting the lowest (and perhaps highest)
OPP would accomplish that? Could that be done in the board(s)'s
toplevel dts files?
That would be a workaround, however I'd really like to avoid limiting
performance as a solution if I can help it,
especially as the fix might just be "set the opp first, wait for it to apply, then
set the core clock".
Is there a sensible way to get a callback from the opp notify chain? Or from
rpmh directly? Or is this solution really
not the right way to go?
It does seem a bit strange to me that we are telling GMU to change
freq before calling dev_pm_opp_set_opp().. if dev_pm_opp_set_opp() is
increasing voltage, it seems like you'd want to do that *before*
increasing freq (but reverse the order when decreasing freq).. But I'm
not an expert on the ways of the GMU.. maybe Akhil or Jordan knows
better how this is supposed to work.
For legacy gmu, we trigger DCVS using DCVS OOB which comes later in this
function. But the order between regulator and clock which you mentioned
is correct.
But the delay seems like papering something over, and I'm trying to go
in the other direction and reduce latency between user input and
pageflip..
BR,
-R
BR,
-R
Re: [PATCH] drm/msm: Disable frequency clamping on a630
On Sun, Aug 8, 2021 at 7:33 AM Caleb Connolly wrote:
>
>
>
> On 07/08/2021 21:04, Rob Clark wrote:
> > On Sat, Aug 7, 2021 at 12:21 PM Caleb Connolly
> > wrote:
> >>
> >> Hi Rob, Akhil,
> >>
> >> On 29/07/2021 21:53, Rob Clark wrote:
> >>> On Thu, Jul 29, 2021 at 1:28 PM Caleb Connolly
> >>> wrote:
>
>
>
> On 29/07/2021 21:24, Rob Clark wrote:
> > On Thu, Jul 29, 2021 at 1:06 PM Caleb Connolly
> > wrote:
> >>
> >> Hi Rob,
> >>
> >> I've done some more testing! It looks like before that patch
> >> ("drm/msm: Devfreq tuning") the GPU would never get above
> >> the second frequency in the OPP table (342MHz) (at least, not in
> >> glxgears). With the patch applied it would more
> >> aggressively jump up to the max frequency which seems to be unstable
> >> at the default regulator voltages.
> >
> > *ohh*, yeah, ok, that would explain it
> >
> >> Hacking the pm8005 s1 regulator (which provides VDD_GFX) up to 0.988v
> >> (instead of the stock 0.516v) makes the GPU stable
> >> at the higher frequencies.
> >>
> >> Applying this patch reverts the behaviour, and the GPU never goes
> >> above 342MHz in glxgears, losing ~30% performance in
> >> glxgear.
> >>
> >> I think (?) that enabling CPR support would be the proper solution to
> >> this - that would ensure that the regulators run
> >> at the voltage the hardware needs to be stable.
> >>
> >> Is hacking the voltage higher (although ideally not quite that high)
> >> an acceptable short term solution until we have
> >> CPR? Or would it be safer to just not make use of the higher
> >> frequencies on a630 for now?
> >>
> >
> > tbh, I'm not sure about the regulator stuff and CPR.. Bjorn is already
> > on CC and I added sboyd, maybe one of them knows better.
> >
> > In the short term, removing the higher problematic OPPs from dts might
> > be a better option than this patch (which I'm dropping), since there
> > is nothing stopping other workloads from hitting higher OPPs.
> Oh yeah that sounds like a more sensible workaround than mine .
> >
> > I'm slightly curious why I didn't have problems at higher OPPs on my
> > c630 laptop (sdm850)
> Perhaps you won the sillicon lottery - iirc sdm850 is binned for higher
> clocks as is out of the factory.
>
> Would it be best to drop the OPPs for all devices? Or just those
> affected? I guess it's possible another c630 might
> crash where yours doesn't?
> >>>
> >>> I've not heard any reports of similar issues from the handful of other
> >>> folks with c630's on #aarch64-laptops.. but I can't really say if that
> >>> is luck or not.
> >> It looks like this affects at least the OnePlus 6 and PocoPhone F1, I've
> >> done some more poking and the following diff
> >> seems to fix the stability issues completely, it seems the delay is
> >> required to let the update propagate.
> >>
> >> This doesn't feel like the right fix, but hopefully it's enough to come up
> >> with a better solution than disabling the new
> >> devfreq behaviour on a630.
> >>
> >> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> >> b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> >> index d7cec7f0dde0..69e2a5e84dae 100644
> >> --- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> >> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> >> @@ -139,6 +139,10 @@ void a6xx_gmu_set_freq(struct msm_gpu *gpu, struct
> >> dev_pm_opp *opp)
> >> return;
> >> }
> >>
> >> + dev_pm_opp_set_opp(>pdev->dev, opp);
> >> +
> >> + usleep_range(300, 500);
> >> +
> >
> > Hmm, this is going to be in the critical path on idle -> active
> > transition (ie. think response time to user-input).. so we defn don't
> > want to do this unconditionally..
> >
> > If I understand the problem, we just want to limit how far we jump the
> > gpu freq in one go.. maybe deleting the lowest (and perhaps highest)
> > OPP would accomplish that? Could that be done in the board(s)'s
> > toplevel dts files?
> That would be a workaround, however I'd really like to avoid limiting
> performance as a solution if I can help it,
> especially as the fix might just be "set the opp first, wait for it to apply,
> then set the core clock".
>
> Is there a sensible way to get a callback from the opp notify chain? Or from
> rpmh directly? Or is this solution really
> not the right way to go?
It does seem a bit strange to me that we are telling GMU to change
freq before calling dev_pm_opp_set_opp().. if dev_pm_opp_set_opp() is
increasing voltage, it seems like you'd want to do that *before*
increasing freq (but reverse the order when decreasing freq).. But I'm
not an expert on the ways of the GMU.. maybe Akhil or Jordan knows
better how this is supposed to work.
But the delay seems like papering something over, and I'm trying to go
in the other direction and reduce
Re: [PATCH] drm/msm: Disable frequency clamping on a630
On 07/08/2021 21:04, Rob Clark wrote:
On Sat, Aug 7, 2021 at 12:21 PM Caleb Connolly
wrote:
Hi Rob, Akhil,
On 29/07/2021 21:53, Rob Clark wrote:
On Thu, Jul 29, 2021 at 1:28 PM Caleb Connolly
wrote:
On 29/07/2021 21:24, Rob Clark wrote:
On Thu, Jul 29, 2021 at 1:06 PM Caleb Connolly
wrote:
Hi Rob,
I've done some more testing! It looks like before that patch ("drm/msm: Devfreq
tuning") the GPU would never get above
the second frequency in the OPP table (342MHz) (at least, not in glxgears).
With the patch applied it would more
aggressively jump up to the max frequency which seems to be unstable at the
default regulator voltages.
*ohh*, yeah, ok, that would explain it
Hacking the pm8005 s1 regulator (which provides VDD_GFX) up to 0.988v (instead
of the stock 0.516v) makes the GPU stable
at the higher frequencies.
Applying this patch reverts the behaviour, and the GPU never goes above 342MHz
in glxgears, losing ~30% performance in
glxgear.
I think (?) that enabling CPR support would be the proper solution to this -
that would ensure that the regulators run
at the voltage the hardware needs to be stable.
Is hacking the voltage higher (although ideally not quite that high) an
acceptable short term solution until we have
CPR? Or would it be safer to just not make use of the higher frequencies on
a630 for now?
tbh, I'm not sure about the regulator stuff and CPR.. Bjorn is already
on CC and I added sboyd, maybe one of them knows better.
In the short term, removing the higher problematic OPPs from dts might
be a better option than this patch (which I'm dropping), since there
is nothing stopping other workloads from hitting higher OPPs.
Oh yeah that sounds like a more sensible workaround than mine .
I'm slightly curious why I didn't have problems at higher OPPs on my
c630 laptop (sdm850)
Perhaps you won the sillicon lottery - iirc sdm850 is binned for higher clocks
as is out of the factory.
Would it be best to drop the OPPs for all devices? Or just those affected? I
guess it's possible another c630 might
crash where yours doesn't?
I've not heard any reports of similar issues from the handful of other
folks with c630's on #aarch64-laptops.. but I can't really say if that
is luck or not.
It looks like this affects at least the OnePlus 6 and PocoPhone F1, I've done
some more poking and the following diff
seems to fix the stability issues completely, it seems the delay is required to
let the update propagate.
This doesn't feel like the right fix, but hopefully it's enough to come up with
a better solution than disabling the new
devfreq behaviour on a630.
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
index d7cec7f0dde0..69e2a5e84dae 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
@@ -139,6 +139,10 @@ void a6xx_gmu_set_freq(struct msm_gpu *gpu, struct
dev_pm_opp *opp)
return;
}
+ dev_pm_opp_set_opp(>pdev->dev, opp);
+
+ usleep_range(300, 500);
+
Hmm, this is going to be in the critical path on idle -> active
transition (ie. think response time to user-input).. so we defn don't
want to do this unconditionally..
If I understand the problem, we just want to limit how far we jump the
gpu freq in one go.. maybe deleting the lowest (and perhaps highest)
OPP would accomplish that? Could that be done in the board(s)'s
toplevel dts files?
That would be a workaround, however I'd really like to avoid limiting performance as a solution if I can help it,
especially as the fix might just be "set the opp first, wait for it to apply, then set the core clock".
Is there a sensible way to get a callback from the opp notify chain? Or from rpmh directly? Or is this solution really
not the right way to go?
BR,
-R
gmu_write(gmu, REG_A6XX_GMU_DCVS_ACK_OPTION, 0);
gmu_write(gmu, REG_A6XX_GMU_DCVS_PERF_SETTING,
@@ -158,7 +162,6 @@ void a6xx_gmu_set_freq(struct msm_gpu *gpu, struct
dev_pm_opp *opp)
if (ret)
dev_err(gmu->dev, "GMU set GPU frequency error: %d\n", ret);
- dev_pm_opp_set_opp(>pdev->dev, opp);
pm_runtime_put(gmu->dev);
}
Maybe just remove it for affected devices? But I'll defer to Bjorn.
BR,
-R
--
Kind Regards,
Caleb (they/them)
--
Kind Regards,
Caleb (they/them)
Re: [PATCH] drm/msm: Disable frequency clamping on a630
On Sat, Aug 7, 2021 at 12:21 PM Caleb Connolly
wrote:
>
> Hi Rob, Akhil,
>
> On 29/07/2021 21:53, Rob Clark wrote:
> > On Thu, Jul 29, 2021 at 1:28 PM Caleb Connolly
> > wrote:
> >>
> >>
> >>
> >> On 29/07/2021 21:24, Rob Clark wrote:
> >>> On Thu, Jul 29, 2021 at 1:06 PM Caleb Connolly
> >>> wrote:
>
> Hi Rob,
>
> I've done some more testing! It looks like before that patch ("drm/msm:
> Devfreq tuning") the GPU would never get above
> the second frequency in the OPP table (342MHz) (at least, not in
> glxgears). With the patch applied it would more
> aggressively jump up to the max frequency which seems to be unstable at
> the default regulator voltages.
> >>>
> >>> *ohh*, yeah, ok, that would explain it
> >>>
> Hacking the pm8005 s1 regulator (which provides VDD_GFX) up to 0.988v
> (instead of the stock 0.516v) makes the GPU stable
> at the higher frequencies.
>
> Applying this patch reverts the behaviour, and the GPU never goes above
> 342MHz in glxgears, losing ~30% performance in
> glxgear.
>
> I think (?) that enabling CPR support would be the proper solution to
> this - that would ensure that the regulators run
> at the voltage the hardware needs to be stable.
>
> Is hacking the voltage higher (although ideally not quite that high) an
> acceptable short term solution until we have
> CPR? Or would it be safer to just not make use of the higher frequencies
> on a630 for now?
>
> >>>
> >>> tbh, I'm not sure about the regulator stuff and CPR.. Bjorn is already
> >>> on CC and I added sboyd, maybe one of them knows better.
> >>>
> >>> In the short term, removing the higher problematic OPPs from dts might
> >>> be a better option than this patch (which I'm dropping), since there
> >>> is nothing stopping other workloads from hitting higher OPPs.
> >> Oh yeah that sounds like a more sensible workaround than mine .
> >>>
> >>> I'm slightly curious why I didn't have problems at higher OPPs on my
> >>> c630 laptop (sdm850)
> >> Perhaps you won the sillicon lottery - iirc sdm850 is binned for higher
> >> clocks as is out of the factory.
> >>
> >> Would it be best to drop the OPPs for all devices? Or just those affected?
> >> I guess it's possible another c630 might
> >> crash where yours doesn't?
> >
> > I've not heard any reports of similar issues from the handful of other
> > folks with c630's on #aarch64-laptops.. but I can't really say if that
> > is luck or not.
> It looks like this affects at least the OnePlus 6 and PocoPhone F1, I've done
> some more poking and the following diff
> seems to fix the stability issues completely, it seems the delay is required
> to let the update propagate.
>
> This doesn't feel like the right fix, but hopefully it's enough to come up
> with a better solution than disabling the new
> devfreq behaviour on a630.
>
> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> index d7cec7f0dde0..69e2a5e84dae 100644
> --- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> @@ -139,6 +139,10 @@ void a6xx_gmu_set_freq(struct msm_gpu *gpu, struct
> dev_pm_opp *opp)
> return;
> }
>
> + dev_pm_opp_set_opp(>pdev->dev, opp);
> +
> + usleep_range(300, 500);
> +
Hmm, this is going to be in the critical path on idle -> active
transition (ie. think response time to user-input).. so we defn don't
want to do this unconditionally..
If I understand the problem, we just want to limit how far we jump the
gpu freq in one go.. maybe deleting the lowest (and perhaps highest)
OPP would accomplish that? Could that be done in the board(s)'s
toplevel dts files?
BR,
-R
> gmu_write(gmu, REG_A6XX_GMU_DCVS_ACK_OPTION, 0);
>
> gmu_write(gmu, REG_A6XX_GMU_DCVS_PERF_SETTING,
> @@ -158,7 +162,6 @@ void a6xx_gmu_set_freq(struct msm_gpu *gpu, struct
> dev_pm_opp *opp)
> if (ret)
> dev_err(gmu->dev, "GMU set GPU frequency error: %d\n", ret);
>
> - dev_pm_opp_set_opp(>pdev->dev, opp);
> pm_runtime_put(gmu->dev);
> }
> >
> > Maybe just remove it for affected devices? But I'll defer to Bjorn.
> >
> > BR,
> > -R
> >
>
> --
> Kind Regards,
> Caleb (they/them)
Re: [PATCH] drm/msm: Disable frequency clamping on a630
Hi Rob, Akhil,
On 29/07/2021 21:53, Rob Clark wrote:
On Thu, Jul 29, 2021 at 1:28 PM Caleb Connolly
wrote:
On 29/07/2021 21:24, Rob Clark wrote:
On Thu, Jul 29, 2021 at 1:06 PM Caleb Connolly
wrote:
Hi Rob,
I've done some more testing! It looks like before that patch ("drm/msm: Devfreq
tuning") the GPU would never get above
the second frequency in the OPP table (342MHz) (at least, not in glxgears).
With the patch applied it would more
aggressively jump up to the max frequency which seems to be unstable at the
default regulator voltages.
*ohh*, yeah, ok, that would explain it
Hacking the pm8005 s1 regulator (which provides VDD_GFX) up to 0.988v (instead
of the stock 0.516v) makes the GPU stable
at the higher frequencies.
Applying this patch reverts the behaviour, and the GPU never goes above 342MHz
in glxgears, losing ~30% performance in
glxgear.
I think (?) that enabling CPR support would be the proper solution to this -
that would ensure that the regulators run
at the voltage the hardware needs to be stable.
Is hacking the voltage higher (although ideally not quite that high) an
acceptable short term solution until we have
CPR? Or would it be safer to just not make use of the higher frequencies on
a630 for now?
tbh, I'm not sure about the regulator stuff and CPR.. Bjorn is already
on CC and I added sboyd, maybe one of them knows better.
In the short term, removing the higher problematic OPPs from dts might
be a better option than this patch (which I'm dropping), since there
is nothing stopping other workloads from hitting higher OPPs.
Oh yeah that sounds like a more sensible workaround than mine .
I'm slightly curious why I didn't have problems at higher OPPs on my
c630 laptop (sdm850)
Perhaps you won the sillicon lottery - iirc sdm850 is binned for higher clocks
as is out of the factory.
Would it be best to drop the OPPs for all devices? Or just those affected? I
guess it's possible another c630 might
crash where yours doesn't?
I've not heard any reports of similar issues from the handful of other
folks with c630's on #aarch64-laptops.. but I can't really say if that
is luck or not.
It looks like this affects at least the OnePlus 6 and PocoPhone F1, I've done some more poking and the following diff
seems to fix the stability issues completely, it seems the delay is required to let the update propagate.
This doesn't feel like the right fix, but hopefully it's enough to come up with a better solution than disabling the new
devfreq behaviour on a630.
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
index d7cec7f0dde0..69e2a5e84dae 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
@@ -139,6 +139,10 @@ void a6xx_gmu_set_freq(struct msm_gpu *gpu, struct
dev_pm_opp *opp)
return;
}
+ dev_pm_opp_set_opp(>pdev->dev, opp);
+
+ usleep_range(300, 500);
+
gmu_write(gmu, REG_A6XX_GMU_DCVS_ACK_OPTION, 0);
gmu_write(gmu, REG_A6XX_GMU_DCVS_PERF_SETTING,
@@ -158,7 +162,6 @@ void a6xx_gmu_set_freq(struct msm_gpu *gpu, struct
dev_pm_opp *opp)
if (ret)
dev_err(gmu->dev, "GMU set GPU frequency error: %d\n", ret);
- dev_pm_opp_set_opp(>pdev->dev, opp);
pm_runtime_put(gmu->dev);
}
Maybe just remove it for affected devices? But I'll defer to Bjorn.
BR,
-R
--
Kind Regards,
Caleb (they/them)
Re: [PATCH] drm/msm: Disable frequency clamping on a630
On Thu, Jul 29, 2021 at 1:28 PM Caleb Connolly
wrote:
>
>
>
> On 29/07/2021 21:24, Rob Clark wrote:
> > On Thu, Jul 29, 2021 at 1:06 PM Caleb Connolly
> > wrote:
> >>
> >> Hi Rob,
> >>
> >> I've done some more testing! It looks like before that patch ("drm/msm:
> >> Devfreq tuning") the GPU would never get above
> >> the second frequency in the OPP table (342MHz) (at least, not in
> >> glxgears). With the patch applied it would more
> >> aggressively jump up to the max frequency which seems to be unstable at
> >> the default regulator voltages.
> >
> > *ohh*, yeah, ok, that would explain it
> >
> >> Hacking the pm8005 s1 regulator (which provides VDD_GFX) up to 0.988v
> >> (instead of the stock 0.516v) makes the GPU stable
> >> at the higher frequencies.
> >>
> >> Applying this patch reverts the behaviour, and the GPU never goes above
> >> 342MHz in glxgears, losing ~30% performance in
> >> glxgear.
> >>
> >> I think (?) that enabling CPR support would be the proper solution to this
> >> - that would ensure that the regulators run
> >> at the voltage the hardware needs to be stable.
> >>
> >> Is hacking the voltage higher (although ideally not quite that high) an
> >> acceptable short term solution until we have
> >> CPR? Or would it be safer to just not make use of the higher frequencies
> >> on a630 for now?
> >>
> >
> > tbh, I'm not sure about the regulator stuff and CPR.. Bjorn is already
> > on CC and I added sboyd, maybe one of them knows better.
> >
> > In the short term, removing the higher problematic OPPs from dts might
> > be a better option than this patch (which I'm dropping), since there
> > is nothing stopping other workloads from hitting higher OPPs.
> Oh yeah that sounds like a more sensible workaround than mine .
> >
> > I'm slightly curious why I didn't have problems at higher OPPs on my
> > c630 laptop (sdm850)
> Perhaps you won the sillicon lottery - iirc sdm850 is binned for higher
> clocks as is out of the factory.
>
> Would it be best to drop the OPPs for all devices? Or just those affected? I
> guess it's possible another c630 might
> crash where yours doesn't?
I've not heard any reports of similar issues from the handful of other
folks with c630's on #aarch64-laptops.. but I can't really say if that
is luck or not.
Maybe just remove it for affected devices? But I'll defer to Bjorn.
BR,
-R
Re: [PATCH] drm/msm: Disable frequency clamping on a630
On 29/07/2021 21:24, Rob Clark wrote:
On Thu, Jul 29, 2021 at 1:06 PM Caleb Connolly
wrote:
Hi Rob,
I've done some more testing! It looks like before that patch ("drm/msm: Devfreq
tuning") the GPU would never get above
the second frequency in the OPP table (342MHz) (at least, not in glxgears).
With the patch applied it would more
aggressively jump up to the max frequency which seems to be unstable at the
default regulator voltages.
*ohh*, yeah, ok, that would explain it
Hacking the pm8005 s1 regulator (which provides VDD_GFX) up to 0.988v (instead
of the stock 0.516v) makes the GPU stable
at the higher frequencies.
Applying this patch reverts the behaviour, and the GPU never goes above 342MHz
in glxgears, losing ~30% performance in
glxgear.
I think (?) that enabling CPR support would be the proper solution to this -
that would ensure that the regulators run
at the voltage the hardware needs to be stable.
Is hacking the voltage higher (although ideally not quite that high) an
acceptable short term solution until we have
CPR? Or would it be safer to just not make use of the higher frequencies on
a630 for now?
tbh, I'm not sure about the regulator stuff and CPR.. Bjorn is already
on CC and I added sboyd, maybe one of them knows better.
In the short term, removing the higher problematic OPPs from dts might
be a better option than this patch (which I'm dropping), since there
is nothing stopping other workloads from hitting higher OPPs.
Oh yeah that sounds like a more sensible workaround than mine .
I'm slightly curious why I didn't have problems at higher OPPs on my
c630 laptop (sdm850)
Perhaps you won the sillicon lottery - iirc sdm850 is binned for higher clocks
as is out of the factory.
Would it be best to drop the OPPs for all devices? Or just those affected? I guess it's possible another c630 might
crash where yours doesn't?
BR,
-R
On 29/07/2021 19:39, Rob Clark wrote:
From: Rob Clark
The more frequent frequency transitions resulting from clamping freq to
minimum when the GPU is idle seems to be causing some issue with the bus
getting voted off when it should be on. (An enable racing with an async
disable?) This might be a problem outside of the GPU, as I can't
reproduce this on a618 which uses the same GMU fw and same mechanism to
communicate with GMU to set opp. For now, just revert to previous
devfreq behavior on a630 until the issue is understood.
Reported-by: Caleb Connolly
Fixes: 9bc95570175a ("drm/msm: Devfreq tuning")
Signed-off-by: Rob Clark
---
drivers/gpu/drm/msm/adreno/adreno_gpu.c | 3 +++
drivers/gpu/drm/msm/msm_gpu.h | 2 ++
drivers/gpu/drm/msm/msm_gpu_devfreq.c | 12
3 files changed, 17 insertions(+)
diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
index 748665232d29..9fd08b413010 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
@@ -945,6 +945,9 @@ int adreno_gpu_init(struct drm_device *drm, struct
platform_device *pdev,
pm_runtime_use_autosuspend(dev);
pm_runtime_enable(dev);
+ if (adreno_is_a630(adreno_gpu))
+ gpu->devfreq.disable_freq_clamping = true;
+
return msm_gpu_init(drm, pdev, _gpu->base, >base,
adreno_gpu->info->name, _gpu_config);
}
diff --git a/drivers/gpu/drm/msm/msm_gpu.h b/drivers/gpu/drm/msm/msm_gpu.h
index 0e4b45bff2e6..7e11b667f939 100644
--- a/drivers/gpu/drm/msm/msm_gpu.h
+++ b/drivers/gpu/drm/msm/msm_gpu.h
@@ -112,6 +112,8 @@ struct msm_gpu_devfreq {
* it is inactive.
*/
unsigned long idle_freq;
+
+ bool disable_freq_clamping;
};
struct msm_gpu {
diff --git a/drivers/gpu/drm/msm/msm_gpu_devfreq.c
b/drivers/gpu/drm/msm/msm_gpu_devfreq.c
index 0a1ee20296a2..a832af436251 100644
--- a/drivers/gpu/drm/msm/msm_gpu_devfreq.c
+++ b/drivers/gpu/drm/msm/msm_gpu_devfreq.c
@@ -94,6 +94,12 @@ void msm_devfreq_init(struct msm_gpu *gpu)
if (!gpu->funcs->gpu_busy)
return;
+ /* Revert to previous polling interval if we aren't using freq clamping
+ * to preserve previous behavior
+ */
+ if (gpu->devfreq.disable_freq_clamping)
+ msm_devfreq_profile.polling_ms = 10;
+
msm_devfreq_profile.initial_freq = gpu->fast_rate;
/*
@@ -151,6 +157,9 @@ void msm_devfreq_active(struct msm_gpu *gpu)
unsigned int idle_time;
unsigned long target_freq = df->idle_freq;
+ if (gpu->devfreq.disable_freq_clamping)
+ return;
+
/*
* Hold devfreq lock to synchronize with get_dev_status()/
* target() callbacks
@@ -186,6 +195,9 @@ void msm_devfreq_idle(struct msm_gpu *gpu)
struct msm_gpu_devfreq *df = >devfreq;
unsigned long idle_freq, target_freq = 0;
+ if (gpu->devfreq.disable_freq_clamping)
+ return;
+
/*
* Hold devfreq lock to synchronize with get_dev_status()/
Re: [PATCH] drm/msm: Disable frequency clamping on a630
On Thu, Jul 29, 2021 at 1:06 PM Caleb Connolly
wrote:
>
> Hi Rob,
>
> I've done some more testing! It looks like before that patch ("drm/msm:
> Devfreq tuning") the GPU would never get above
> the second frequency in the OPP table (342MHz) (at least, not in glxgears).
> With the patch applied it would more
> aggressively jump up to the max frequency which seems to be unstable at the
> default regulator voltages.
*ohh*, yeah, ok, that would explain it
> Hacking the pm8005 s1 regulator (which provides VDD_GFX) up to 0.988v
> (instead of the stock 0.516v) makes the GPU stable
> at the higher frequencies.
>
> Applying this patch reverts the behaviour, and the GPU never goes above
> 342MHz in glxgears, losing ~30% performance in
> glxgear.
>
> I think (?) that enabling CPR support would be the proper solution to this -
> that would ensure that the regulators run
> at the voltage the hardware needs to be stable.
>
> Is hacking the voltage higher (although ideally not quite that high) an
> acceptable short term solution until we have
> CPR? Or would it be safer to just not make use of the higher frequencies on
> a630 for now?
>
tbh, I'm not sure about the regulator stuff and CPR.. Bjorn is already
on CC and I added sboyd, maybe one of them knows better.
In the short term, removing the higher problematic OPPs from dts might
be a better option than this patch (which I'm dropping), since there
is nothing stopping other workloads from hitting higher OPPs.
I'm slightly curious why I didn't have problems at higher OPPs on my
c630 laptop (sdm850)
BR,
-R
>
> On 29/07/2021 19:39, Rob Clark wrote:
> > From: Rob Clark
> >
> > The more frequent frequency transitions resulting from clamping freq to
> > minimum when the GPU is idle seems to be causing some issue with the bus
> > getting voted off when it should be on. (An enable racing with an async
> > disable?) This might be a problem outside of the GPU, as I can't
> > reproduce this on a618 which uses the same GMU fw and same mechanism to
> > communicate with GMU to set opp. For now, just revert to previous
> > devfreq behavior on a630 until the issue is understood.
> >
> > Reported-by: Caleb Connolly
> > Fixes: 9bc95570175a ("drm/msm: Devfreq tuning")
> > Signed-off-by: Rob Clark
> > ---
> > drivers/gpu/drm/msm/adreno/adreno_gpu.c | 3 +++
> > drivers/gpu/drm/msm/msm_gpu.h | 2 ++
> > drivers/gpu/drm/msm/msm_gpu_devfreq.c | 12
> > 3 files changed, 17 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> > b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> > index 748665232d29..9fd08b413010 100644
> > --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> > +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> > @@ -945,6 +945,9 @@ int adreno_gpu_init(struct drm_device *drm, struct
> > platform_device *pdev,
> > pm_runtime_use_autosuspend(dev);
> > pm_runtime_enable(dev);
> >
> > + if (adreno_is_a630(adreno_gpu))
> > + gpu->devfreq.disable_freq_clamping = true;
> > +
> > return msm_gpu_init(drm, pdev, _gpu->base, >base,
> > adreno_gpu->info->name, _gpu_config);
> > }
> > diff --git a/drivers/gpu/drm/msm/msm_gpu.h b/drivers/gpu/drm/msm/msm_gpu.h
> > index 0e4b45bff2e6..7e11b667f939 100644
> > --- a/drivers/gpu/drm/msm/msm_gpu.h
> > +++ b/drivers/gpu/drm/msm/msm_gpu.h
> > @@ -112,6 +112,8 @@ struct msm_gpu_devfreq {
> >* it is inactive.
> >*/
> > unsigned long idle_freq;
> > +
> > + bool disable_freq_clamping;
> > };
> >
> > struct msm_gpu {
> > diff --git a/drivers/gpu/drm/msm/msm_gpu_devfreq.c
> > b/drivers/gpu/drm/msm/msm_gpu_devfreq.c
> > index 0a1ee20296a2..a832af436251 100644
> > --- a/drivers/gpu/drm/msm/msm_gpu_devfreq.c
> > +++ b/drivers/gpu/drm/msm/msm_gpu_devfreq.c
> > @@ -94,6 +94,12 @@ void msm_devfreq_init(struct msm_gpu *gpu)
> > if (!gpu->funcs->gpu_busy)
> > return;
> >
> > + /* Revert to previous polling interval if we aren't using freq
> > clamping
> > + * to preserve previous behavior
> > + */
> > + if (gpu->devfreq.disable_freq_clamping)
> > + msm_devfreq_profile.polling_ms = 10;
> > +
> > msm_devfreq_profile.initial_freq = gpu->fast_rate;
> >
> > /*
> > @@ -151,6 +157,9 @@ void msm_devfreq_active(struct msm_gpu *gpu)
> > unsigned int idle_time;
> > unsigned long target_freq = df->idle_freq;
> >
> > + if (gpu->devfreq.disable_freq_clamping)
> > + return;
> > +
> > /*
> >* Hold devfreq lock to synchronize with get_dev_status()/
> >* target() callbacks
> > @@ -186,6 +195,9 @@ void msm_devfreq_idle(struct msm_gpu *gpu)
> > struct msm_gpu_devfreq *df = >devfreq;
> > unsigned long idle_freq, target_freq = 0;
> >
> > + if (gpu->devfreq.disable_freq_clamping)
> > + return;
> > +
> > /*
> >* Hold devfreq lock to synchronize with get_dev_status()/
> >*
Re: [PATCH] drm/msm: Disable frequency clamping on a630
Hi Rob,
I've done some more testing! It looks like before that patch ("drm/msm: Devfreq tuning") the GPU would never get above
the second frequency in the OPP table (342MHz) (at least, not in glxgears). With the patch applied it would more
aggressively jump up to the max frequency which seems to be unstable at the default regulator voltages.
Hacking the pm8005 s1 regulator (which provides VDD_GFX) up to 0.988v (instead of the stock 0.516v) makes the GPU stable
at the higher frequencies.
Applying this patch reverts the behaviour, and the GPU never goes above 342MHz in glxgears, losing ~30% performance in
glxgear.
I think (?) that enabling CPR support would be the proper solution to this - that would ensure that the regulators run
at the voltage the hardware needs to be stable.
Is hacking the voltage higher (although ideally not quite that high) an acceptable short term solution until we have
CPR? Or would it be safer to just not make use of the higher frequencies on a630 for now?
On 29/07/2021 19:39, Rob Clark wrote:
From: Rob Clark
The more frequent frequency transitions resulting from clamping freq to
minimum when the GPU is idle seems to be causing some issue with the bus
getting voted off when it should be on. (An enable racing with an async
disable?) This might be a problem outside of the GPU, as I can't
reproduce this on a618 which uses the same GMU fw and same mechanism to
communicate with GMU to set opp. For now, just revert to previous
devfreq behavior on a630 until the issue is understood.
Reported-by: Caleb Connolly
Fixes: 9bc95570175a ("drm/msm: Devfreq tuning")
Signed-off-by: Rob Clark
---
drivers/gpu/drm/msm/adreno/adreno_gpu.c | 3 +++
drivers/gpu/drm/msm/msm_gpu.h | 2 ++
drivers/gpu/drm/msm/msm_gpu_devfreq.c | 12
3 files changed, 17 insertions(+)
diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
index 748665232d29..9fd08b413010 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
@@ -945,6 +945,9 @@ int adreno_gpu_init(struct drm_device *drm, struct
platform_device *pdev,
pm_runtime_use_autosuspend(dev);
pm_runtime_enable(dev);
+ if (adreno_is_a630(adreno_gpu))
+ gpu->devfreq.disable_freq_clamping = true;
+
return msm_gpu_init(drm, pdev, _gpu->base, >base,
adreno_gpu->info->name, _gpu_config);
}
diff --git a/drivers/gpu/drm/msm/msm_gpu.h b/drivers/gpu/drm/msm/msm_gpu.h
index 0e4b45bff2e6..7e11b667f939 100644
--- a/drivers/gpu/drm/msm/msm_gpu.h
+++ b/drivers/gpu/drm/msm/msm_gpu.h
@@ -112,6 +112,8 @@ struct msm_gpu_devfreq {
* it is inactive.
*/
unsigned long idle_freq;
+
+ bool disable_freq_clamping;
};
struct msm_gpu {
diff --git a/drivers/gpu/drm/msm/msm_gpu_devfreq.c
b/drivers/gpu/drm/msm/msm_gpu_devfreq.c
index 0a1ee20296a2..a832af436251 100644
--- a/drivers/gpu/drm/msm/msm_gpu_devfreq.c
+++ b/drivers/gpu/drm/msm/msm_gpu_devfreq.c
@@ -94,6 +94,12 @@ void msm_devfreq_init(struct msm_gpu *gpu)
if (!gpu->funcs->gpu_busy)
return;
+ /* Revert to previous polling interval if we aren't using freq clamping
+* to preserve previous behavior
+*/
+ if (gpu->devfreq.disable_freq_clamping)
+ msm_devfreq_profile.polling_ms = 10;
+
msm_devfreq_profile.initial_freq = gpu->fast_rate;
/*
@@ -151,6 +157,9 @@ void msm_devfreq_active(struct msm_gpu *gpu)
unsigned int idle_time;
unsigned long target_freq = df->idle_freq;
+ if (gpu->devfreq.disable_freq_clamping)
+ return;
+
/*
* Hold devfreq lock to synchronize with get_dev_status()/
* target() callbacks
@@ -186,6 +195,9 @@ void msm_devfreq_idle(struct msm_gpu *gpu)
struct msm_gpu_devfreq *df = >devfreq;
unsigned long idle_freq, target_freq = 0;
+ if (gpu->devfreq.disable_freq_clamping)
+ return;
+
/*
* Hold devfreq lock to synchronize with get_dev_status()/
* target() callbacks
--
Kind Regards,
Caleb (they/them)
[PATCH] drm/msm: Disable frequency clamping on a630
From: Rob Clark
The more frequent frequency transitions resulting from clamping freq to
minimum when the GPU is idle seems to be causing some issue with the bus
getting voted off when it should be on. (An enable racing with an async
disable?) This might be a problem outside of the GPU, as I can't
reproduce this on a618 which uses the same GMU fw and same mechanism to
communicate with GMU to set opp. For now, just revert to previous
devfreq behavior on a630 until the issue is understood.
Reported-by: Caleb Connolly
Fixes: 9bc95570175a ("drm/msm: Devfreq tuning")
Signed-off-by: Rob Clark
---
drivers/gpu/drm/msm/adreno/adreno_gpu.c | 3 +++
drivers/gpu/drm/msm/msm_gpu.h | 2 ++
drivers/gpu/drm/msm/msm_gpu_devfreq.c | 12
3 files changed, 17 insertions(+)
diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
index 748665232d29..9fd08b413010 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
@@ -945,6 +945,9 @@ int adreno_gpu_init(struct drm_device *drm, struct
platform_device *pdev,
pm_runtime_use_autosuspend(dev);
pm_runtime_enable(dev);
+ if (adreno_is_a630(adreno_gpu))
+ gpu->devfreq.disable_freq_clamping = true;
+
return msm_gpu_init(drm, pdev, _gpu->base, >base,
adreno_gpu->info->name, _gpu_config);
}
diff --git a/drivers/gpu/drm/msm/msm_gpu.h b/drivers/gpu/drm/msm/msm_gpu.h
index 0e4b45bff2e6..7e11b667f939 100644
--- a/drivers/gpu/drm/msm/msm_gpu.h
+++ b/drivers/gpu/drm/msm/msm_gpu.h
@@ -112,6 +112,8 @@ struct msm_gpu_devfreq {
* it is inactive.
*/
unsigned long idle_freq;
+
+ bool disable_freq_clamping;
};
struct msm_gpu {
diff --git a/drivers/gpu/drm/msm/msm_gpu_devfreq.c
b/drivers/gpu/drm/msm/msm_gpu_devfreq.c
index 0a1ee20296a2..a832af436251 100644
--- a/drivers/gpu/drm/msm/msm_gpu_devfreq.c
+++ b/drivers/gpu/drm/msm/msm_gpu_devfreq.c
@@ -94,6 +94,12 @@ void msm_devfreq_init(struct msm_gpu *gpu)
if (!gpu->funcs->gpu_busy)
return;
+ /* Revert to previous polling interval if we aren't using freq clamping
+* to preserve previous behavior
+*/
+ if (gpu->devfreq.disable_freq_clamping)
+ msm_devfreq_profile.polling_ms = 10;
+
msm_devfreq_profile.initial_freq = gpu->fast_rate;
/*
@@ -151,6 +157,9 @@ void msm_devfreq_active(struct msm_gpu *gpu)
unsigned int idle_time;
unsigned long target_freq = df->idle_freq;
+ if (gpu->devfreq.disable_freq_clamping)
+ return;
+
/*
* Hold devfreq lock to synchronize with get_dev_status()/
* target() callbacks
@@ -186,6 +195,9 @@ void msm_devfreq_idle(struct msm_gpu *gpu)
struct msm_gpu_devfreq *df = >devfreq;
unsigned long idle_freq, target_freq = 0;
+ if (gpu->devfreq.disable_freq_clamping)
+ return;
+
/*
* Hold devfreq lock to synchronize with get_dev_status()/
* target() callbacks
--
2.31.1
