On Wed, Jan 11, 2023 at 8:23 AM Quan, Evan <Evan.Quan@xxxxxxx> wrote: > > [AMD Official Use Only - General] > > Hi Matt, > > The problem with the approach(removal of interface functions from the ppt_funcs) is: > 1. If to drop support for some sysfs interface, you need to know the internal implementation details to mask out the corresponding function. > That's not straightforward and might be not an easy job for new comer. > 2. Sometimes the missing of some interfaces is unintentional. It needs to be added instead of being treated as unsupported. > 3. The removal of the interface cannot update the sysfs file permission together. That means the sysfs file might be still shown as writable while > actually the setting is not supported at all. > The patch series try to address these in a simple and straightforward way. > > Regarding the manual fan speed setting issue targeted by this patch, the SCPM feature of the new SMU13 asics prevents us from toggling the fan control feature from auto to manual. > Without that, the manual fan speed setting to some specific RPM/PWM is not possible. > About the capability in the OD table you mentioned, it might be a different issue. > Since as I know, the capability the OD table provided is to change the minimum/maximum fan speed limit. > That should be not affected by the SCPM and relevant. Right. Manual fan control is no longer possible. As Evan said, you can adjust the automatic fan curve using the OD interface, but that is it. Alex > > BR > Evan > > -----Original Message----- > > From: Matt Coffin <mcoffin13@xxxxxxxxx> > > Sent: Tuesday, January 10, 2023 9:57 PM > > To: amd-gfx@xxxxxxxxxxxxxxxxxxxxx > > Cc: Quan, Evan <Evan.Quan@xxxxxxx>; Lazar, Lijo <Lijo.Lazar@xxxxxxx> > > Subject: Re: [PATCH 8/8] drm/amd/pm: drop the support for manual fan > > speed setting on SMU13.0.7 > > > > On 1/9/23 23:48, Quan, Evan wrote: > > > [AMD Official Use Only - General] > > > > > > We need these to address the fan speed setting failure reported for the > > new SMU13 asics. > > My opinion shouldn't matter much given sparseness of activity, but, despite > > his... short tonality, I agree with Lijo's assessment there. > > > > As someone less familiar with the code base, the use of "multiple sources of > > truth" contributes to making it harder to understand and ramp up with. > > > > As for the sysfs fan control issue itself, this won't really "fix" the issue, but > > rather just remove write permissions from the hwmon files (if my > > testing+understanding is right), so it wouldn't seem to be a hyper-critical > > deliverable to me as a random outsider looking in (despite being effected by > > the issue personally). Even with that interface removed, there isn't another > > way to control the "auto" fans, as the FW reports it doesn't support that > > capability in the OD table, and ignores anything set therein for that purpose. > > Hopefully that's temporary until FW gets fixed? > > > > I also think the behavior of the other proposed solution (removal of interface > > functions from the ppt_funcs), is objectively a better experience w/r/t > > outcome. If the functions are NULL, then the hwmon device files go away > > *completely*, instead of just being masked from write permission, which > > would make the message clearer to the end user that it's not an error, but an > > actual lack of functionality. > > > > Cheers, sorry I couldn't help, but wanted to at least have something come of > > the last few days I spent implmenting OD on SMU13 for nothing! > > > > ~Matt
