On Monday 11 August 2014, 08:03:57, Адонай Элохим wrote:
> 2014-08-11 1:53 GMT+04:00 Niels Ole Salscheider
>
> <niels_ole@xxxxxxxxxxxxxxxxxxxxx>:
> > On Monday 11 August 2014, 01:19:32, Адонай Элохим wrote:
> >> Hello again, hope you are still reading my texts...
> >>
> >> I digged through the code and narrowed down the issue I wanted to fix.
> >> It appears to be related to the `bool thermal_active` dpm struct
> >> member and this piece of code:
> >>
> >> if (rdev->asic->dpm.force_performance_level) {
> >>
> >> if (rdev->pm.dpm.thermal_active) {
> >>
> >> enum radeon_dpm_forced_level level =
> >> rdev->pm.dpm.forced_level;
> >> /* force low perf level for thermal */
> >> radeon_dpm_force_performance_level(rdev,
> >>
> >> RADEON_DPM_FORCED_LEVEL_LOW);
> >>
> >> /* save the user's level */
> >> rdev->pm.dpm.forced_level = level;
> >>
> >> } else {
> >>
> >> /* otherwise, user selected level */
> >> radeon_dpm_force_performance_level(rdev,
> >>
> >> rdev->pm.dpm.forced_level); }
> >>
> >> }
> >>
> >> I did a double check here - at boot `thermal_active` is `false` and
> >> thus, performance level is properly initiated. But at resume from
> >> suspend `thermal_active` is true and performance level is strictly
> >> bound to low profile.
> >> Besides you cannot change it via echo 1 > /sys/.../force_dpm_level,
> >> again thanks to `thermal_active` checked there.
> >>
> >> Could you explain meaning of this small boolean to me? I'd like to
> >> make a small neat patch fixing this, but I'm scared of doing it in
> >> wrong way.
> >> Sorry if I'm being too persistent.
> >
> > I think thermal_active means that the temperature got too high so that low
> > clocks have to be used.
> >
> > Just some idea, but thermal.work only gets scheduled when the high to low
> > temperature interrupt occurs. When the temperature is too high before
> > suspend (so that thermal_active is true) and it gets low during standby
> > this interrupt will not occur. thermal.work is therefore not scheduled...
> >
> > Ole
>
> You were right, Ole. The driver thinks the temperature is too high.
> Thanks a lot!
> It seems the function ci_set_thermal_temperature_range is missing some
> lines:
>
>
> diff --git a/drivers/gpu/drm/radeon/ci_dpm.c
> b/drivers/gpu/drm/radeon/ci_dpm.c index 584090a..102a4bc 100644
> --- a/radeon/ci_dpm.c
> +++ b/radeon/ci_dpm.c
> @@ -869,6 +869,9 @@ static int ci_set_thermal_temperature_range(struct
> radeon_device *rdev,
> WREG32_SMC(CG_THERMAL_CTRL, tmp);
> #endif
>
> + rdev->pm.dpm.thermal.min_temp = low_temp;
> + rdev->pm.dpm.thermal.max_temp = high_temp;
> +
> return 0;
> }
>
>
> All other similar callbacks for different families of cards have these
> lines. I wonder if there is any specific case for not doing this...
This seems to be wrong indeed. I wonder why it happens after a suspend -
resume cycle, though. Does the hardware generate a corresponding interrupt
after resume?
There is however still an XXX comment in that function... Maybe Alex can
comment on that.
> How do I propose it as a patch anyway?
Fix it in your local git checkout, commit it (don't forget to pass -s to get
your signed-of-by line) and use git format-patch / git send-email to send it
to this list...
> >> Thanks,
> >> Oleg
> >>
> >> 2014-07-22 20:05 GMT+04:00 Alex Deucher <alexdeucher@xxxxxxxxx>:
> >> > On Tue, Jul 22, 2014 at 8:39 AM, Адонай Элохим <algonkvel@xxxxxxxxx>
> >
> > wrote:
> >> >> Hello all!
> >> >>
> >> >> I have some spare time and knowledge in C to try to fix some bugs I am
> >> >> seeing on my machine.
> >> >> So I've checked out and compiled all git trees that I may need and now
> >> >> I'm
> >> >> beginning to read articles.
> >> >>
> >> >> And this is the point from where I don't know where to go. I want to
> >> >> fix
> >> >> particular bug #79806 [1].
> >> >> For me there are many places where this bug can hide - mesa? dri?
> >> >> radeon
> >> >> kernel module? and I just don't know whether should I start reading
> >> >> articles about mesa hacking or about dri architecture or about kernel
> >> >> module development.
> >> >>
> >> >> Now I think the best thing for me is to start looking through radeon
> >> >> kernel
> >> >> module code (I've got ingenious idea that power management resides
> >> >> there)
> >> >> and read more about its architecture. Is this right? I mean, I just
> >> >> want
> >> >> to
> >> >> find out, is this a right place to start looking at for this bug?
> >> >
> >> > The power management is handled in the kernel driver. See radeon_pm.c
> >> > and the relevant *_dpm.c files depending on what asic you have.
> >> >
> >> > Alex
> >> >
> >> >> P.S. Sorry for my English in case it's not good, I'm learning it now
> >> >>
> >> >> P.P.S. And thanks for your hard work!
> >> >>
> >> >> -------------------------------------------
> >> >> [1] https://bugs.freedesktop.org/show_bug.cgi?id=79806
> >> >>
> >> >> _______________________________________________
> >> >> dri-devel mailing list
> >> >> dri-devel@xxxxxxxxxxxxxxxxxxxxx
> >> >> http://lists.freedesktop.org/mailman/listinfo/dri-devel
> >>
> >> _______________________________________________
> >> dri-devel mailing list
> >> dri-devel@xxxxxxxxxxxxxxxxxxxxx
> >> http://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel
