| Submitter | xdrudis |
|---|---|
| Date | 2011-02-17 06:35:45 |
| Message ID | <20110217063545.GE8966@ideafix.casa.ct> |
| Download | mbox | patch |
| Permalink | /patch/2648/ |
| State | Accepted |
| Commit | r6387 |
| Headers | show |
Comments
Am Donnerstag, den 17.02.2011, 07:35 +0100 schrieb xdrudis:
> see patch
Any opinion on these patches? Patch 1-8 seem to be refactorings only,
and splitting functions into smaller logical units looks good to me, but
I'd like to hear from someone deeper in the AMD code.
Patrick
On Thu, Feb 24, 2011 at 6:31 AM, Georgi, Patrick <Patrick.Georgi@secunet.com> wrote: > Am Donnerstag, den 17.02.2011, 07:35 +0100 schrieb xdrudis: >> see patch > Any opinion on these patches? Patch 1-8 seem to be refactorings only, > and splitting functions into smaller logical units looks good to me, but > I'd like to hear from someone deeper in the AMD code. > > > Patrick Yes, These patches are improvements in the SVI code, and I think we need to pull them in. I just haven't had a chance to test them or review them yet. I think that all new Fam10 systems are SVI now. If anyone else has time to try it, please do. Marc
On Thu, Feb 24, 2011 at 02:31:29PM +0100, Georgi, Patrick wrote: > Am Donnerstag, den 17.02.2011, 07:35 +0100 schrieb xdrudis: > > see patch > Any opinion on these patches? Patch 1-8 seem to be refactorings only, > and splitting functions into smaller logical units looks good to me, but > I'd like to hear from someone deeper in the AMD code. > Yes, if these 8 are not refactorings, then it's a bug. I know it's a little work to review it all, but it does not have to be one person. You can review just one patch, maybe. Testing is maybe better to do with all of them, or all without negative reviews, or something. I've tested them one by one and it is a little a waste of time. And I haven't found a single one that fixes it for me. Must be a combination, possibly not all but not sure which ones. They're secuential although not each and every one needs all previous ones. By the way testing for both SVI and PVI is welcome (for AMD FAM 10). I don't intend to break PVI, but I can't test it. Some of the later ones may be a little paranoid or a matter of taste but I tried to split them in small pieces so they can be rejected or modified.
On Thu, Feb 24, 2011 at 2:27 PM, xdrudis <xdrudis@tinet.cat> wrote: > On Thu, Feb 24, 2011 at 02:31:29PM +0100, Georgi, Patrick wrote: >> Am Donnerstag, den 17.02.2011, 07:35 +0100 schrieb xdrudis: >> > see patch >> Any opinion on these patches? Patch 1-8 seem to be refactorings only, >> and splitting functions into smaller logical units looks good to me, but >> I'd like to hear from someone deeper in the AMD code. >> > > Yes, if these 8 are not refactorings, then it's a bug. > > I know it's a little work to review it all, but it does not have > to be one person. You can review just one patch, maybe. > > Testing is maybe better to do with all of them, or all without > negative reviews, or something. I've tested them one by one > and it is a little a waste of time. And I haven't found a single > one that fixes it for me. Must be a combination, possibly not > all but not sure which ones. They're secuential although not > each and every one needs all previous ones. > > By the way testing for both SVI and PVI is welcome (for AMD FAM 10). > I don't intend to break PVI, but I can't test it. > > Some of the later ones may be a little paranoid or a matter of taste > but I tried to split them in small pieces so they can be rejected > or modified. Acked-by: Marc Jones <marcj303@gmail.com> r6387
Patch
Prepare for next patches (Improving BKDG implementation of P-states, CPU and northbridge frequency and voltage handling for Fam 10 in SVI mode). No change of behaviour intended. Refactor FAM10 fidvid . prep_fid_change was already long and it'd get longer with forthcoming patches. We now take apart VSRamp in step b of 2.4.1.7 BKDG to its own function. Signed-off-by: Xavi Drudis Ferran <xdrudis@tinet.cat> --- src/cpu/amd/model_10xxx/fidvid.c 2011-02-16 20:51:55.000000000 +0100 +++ src/cpu/amd/model_10xxx/fidvid.c 2011-02-16 20:51:55.000000000 +0100 @@ -66,6 +66,21 @@ static void enable_fid_change(u8 fid) } } +static void setVSRamp(device_t dev) { + /* BKDG r31116 2010-04-22 2.4.1.7 step b F3xD8[VSRampTime] + * If this field accepts 8 values between 10 and 500 us why + * does page 324 say "BIOS should set this field to 001b." + * (20 us) ? + * Shouldn't it depend on the voltage regulators, mainboard + * or something ? + */ + u32 dword; + dword = pci_read_config32(dev, 0xd8); + dword &= VSRAMP_MASK; + dword |= VSRAMP_VALUE; + pci_write_config32(dev, 0xd8, dword); +} + static void recalculateVsSlamTimeSettingOnCorePre(device_t dev) { u8 pviModeFlag; @@ -179,11 +194,8 @@ static void prep_fid_change(void) printk(BIOS_DEBUG, "Prep FID/VID Node:%02x \n", i); dev = NODE_PCI(i, 3); - dword = pci_read_config32(dev, 0xd8); - dword &= VSRAMP_MASK; - dword |= VSRAMP_VALUE; - pci_write_config32(dev, 0xd8, dword); - + setVSRamp(dev); + /* BKDG r31116 2010-04-22 2.4.1.7 step b F3xD8[VSSlamTime] */ /* Figure out the value for VsSlamTime and program it */ recalculateVsSlamTimeSettingOnCorePre(dev);