On Wed, Apr 10, 2013 at 02:41:39PM -0700, Jesse Barnes wrote: > On Wed, 10 Apr 2013 20:45:11 +0100 > Chris Wilson <chris at chris-wilson.co.uk> wrote: > > > + ring_freq = (gpu_freq * 5 + 3) / 4; > > + ring_freq = max(max_ring_freq, ring_freq); > > Should this be min(max_ring_freq, ring_freq)? Otherwise it seems like > every ia_freq will correspond to the highest ring freq. s/max_ring_freq/min_ring_freq/, bad name in retrospect. The algorithm is drawn from the pm guide so that the ring is always running at least as fast as memory whilst the gpu is active, and then scales up cache bandwidth as the gpu gets busier and faster. By that formula, only the first few GPU freqs run the ring at the recommended minimum frequency. > Also, where are you setting ia_freq in the HSW case? Looks like it'll > be 0? Right. Again the guide says to not abitrarily raise the CPU frequencies, which makes sense as cpufreq will raise those itself based on demand. > I guess that means we just need the gpu and ring freq > correlation on HSW, but that reinforces Daniel's point that a separate > function would probably be clearer. Or you could update the changelog > with "scale the ring frequency directly with the GPU frequency" or > something. If you look at the changelog, it uses those very terms. :-p -Chris -- Chris Wilson, Intel Open Source Technology Centre
