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@xxxxxxxxxxxxxxx wrote: > > From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > > > > 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@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx