Martin Sperl <kernel@xxxxxxxxxxxxxxxx> writes: > On 10.05.2016 03:05, Eric Anholt wrote: >> kernel@xxxxxxxxxxxxxxxx writes: >> >>> From: Martin Sperl <kernel@xxxxxxxxxxxxxxxx> >>> >>> Allow flags to be set on a per clock index basis, which control: >>> * the parent clocks selected when not setting clocks explicitly >> >> I don't think we need this other than avoiding PLLC. What else do you >> want to filter, and why? > What about potentially disabling PLLA_per or PLLH_aux? > Under some circumstances it may make sense to avoid those clocks. > (or testdebug for that matter). PLLH and its descendents are only used by vc4, and the parent choice done by this driver for pllh is correct. I don't know what you mean by "potentially disabling PLLA_PER". We don't expose the testdebug parents, because they're of no use to us. >>> * the clock mode with regards to integer only or higher order mash >> >> Please provide justification in the commit message explaining why there >> is no obvious mash setting to just implement in clk-bcm2835.c, but that >> that a better policy can be encoded in the DT. > > The normal fractional divider will vary the effective period length > between int(div)/Fparent and (int(div)+1)/Fparent. > > So the device (say DAC or other) connected needs to support in > principle Fparent/int(div) as a clock. > > But for higher order mash the HW will spread the period length between: > (int(div)-3)/Fparent and (int(div)+4)/Fparent. > > This value may be outside of the frequencies supported by the attached > HW. That is why mash is not enabled by default for mash clocks - only > div. > > That is also why we need to have this configurable. > > Also obviously higher parent clock frequencies result in higher dividers > for the same target-frequency. So the above effect is less dramatic > when using plld_per (or pllc for that matter) compared to the main > 19.2MHz oscillator, as for 32bit stereo audio at 96kHz (equivalent to > 6144000Hz) in the first case we have a divider of 81.38 while for the > second (osc) case we have a divider of 3.125. > > The use of mash3 is impossible for osc but the use of mash 2 would > spread the frequencies between 9.6MHz (div=2) and 3.84MHz (div=5). > > For plld_per as parent using mash3 instead we get the range: > 6410256.4Hz (div=78) and 5882352.9Hz (div=85), which is a much more > acceptable range and should not impact the device that much. > > So enabling mash 3 by default is not in the best interest hence it is > not done. It sounds like you're optimizing for reducing frequency spread. That sounds fairly easy to do in the parent-choice function. > As a side note: selection of parent oscillator and corresponding mash > may have distinct noise behavior on the same device, where for > 32bit*2@48k it is the main oscillator with mash3 while the second best > match would be pllc with mash2 - not mash3, which has the worsted SN > from all tested! But PLLC continues to be unusable, since it's changed at runtime, and evaluating its noise characteristics based on a snapshot of its frequency doesn't make any sense. > This behavior obviously changes with different "target frequencies" > so it may even be necessary to account for the different target > frequencies. This is why encoding mash in the DT doesn't make any sense to me. You need a runtime policy for choosing the mash setting based on the frequency requested, so let's figure out what policy we actually want.
Attachment:
signature.asc
Description: PGP signature
