Re: [PATCH 4/7] drm/i915: Add i830 "pipes power well"

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Thu, Jun 01, 2017 at 03:46:43PM +0100, Chris Wilson wrote:
> On Thu, Jun 01, 2017 at 05:36:16PM +0300, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > 830 more or less requires both pipes and DPLLs to remain on as long
> > as either pipe is needed. However, when neither pipe is actually needed,
> > we can save a bit of power by turning everything off. To do that we add
> > a new "power well" that turns both pipes and DPLLs on and off in the
> > right order. Seems to save ~50mW on my Fujitsu-Siemens Lifebook S6010.
> > 
> > This also avoids having to abuse the load detection to force pipe A on
> > at init time. That was never very robust, and it only worked for one
> > pipe, whereas 830 really needs both pipes enabled. As a bonus the 830
> > pipe quirk is now a bit more isolated from the rest of the mode setting
> > infrastructure, which should mean that it's much less likely someone
> > will accidentally break it in the future. The extra cost is of course
> > slight code duplication, but that seems like a worthwile tradeoff here.
> 
> Defining it as a powerwell is an interesting way to do it, and seems
> quite apt. My only nit is a request not to add to intel_display.c if we
> can place it elsewhere, intel_gen2_pm.c ? gen2_runtime_pm.c?

Not sure a new file for just these two functions is warranted. I was
actually thinking of keeping them reasonably close by to the normal
pipe/dpll enable/disable code so that if people make a change to one
they would realize that they should perhaps change the other one as
well. Although I suppose intel_display.c is so big that even having
them in the same file won't really guarantee that.

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx