[Intel-gfx] [PATCH 15/21] drm/i915/slpc: Notification of Refresh Rate change

[Daniel Vetter]

On Thu, Apr 28, 2016 at 10:38:27AM +0200, Daniel Vetter wrote:
> On Wed, Apr 27, 2016 at 06:10:59PM -0700, tom.orourke at intel.com wrote:
> > From: Sagar Arun Kamble <sagar.a.kamble at intel.com>
> > 
> > This patch will inform GuC SLPC about changes in the refresh rate
> > due to Seamless DRRS. Refresh rate changes due to Static DRRS will
> > be notified via commit path.
> > 
> > v2: Rebased on previous changed patch and printed error message if
> > H2G action fails.
> > v2(torourke): Updates suggested by Paulo: replace HAS_SLPC with
> > intel_slpc_active. return void instead of ignored error code.
> > 
> > Signed-off-by: Sagar Arun Kamble <sagar.a.kamble at intel.com>
> > Signed-off-by: Tom O'Rourke <Tom.O'Rourke at intel.com>
> 
> So all this display notification stuff looks real fancy, but we have it
> already in the kernel. We notice when we're late in a frame, and then
> aggressively ramp up.
> 
> I want to see an implementation that reuses that infrastructure and just
> tells guc to hurry up, and then benchmark against this one (wrt overall
> frame latency distribution in spikey workloads). All this complexity and
> an entire 2nd codepath needs to be justified in a unified driver, and I
> see exactly none of that going on.

+ power consumption benchmarks ofc, since current code also aggressively
downclocks again when the burst is over. My concern here is that
fundamentally guc just doesn't know enough to make a good decision here,
or at least it can react only much later. Whereas the kernel actually
knows what atomic display update it has pending, and what exactly it needs
to boost to get that to the screen asap.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch