Re: [PATCH 0/2] split out intel_display_power

From: Imre Deak <imre.deak@intel.com>
To: Jani Nikula <jani.nikula@linux.intel.com>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH 0/2] split out intel_display_power
Date: Tue, 4 Jun 2019 10:12:22 +0300	[thread overview]
Message-ID: <20190604071222.GA23036@ideak-desk.fi.intel.com> (raw)
In-Reply-To: <874l56o6fk.fsf@intel.com>

On Mon, Jun 03, 2019 at 09:43:59PM +0300, Jani Nikula wrote:
> On Sat, 01 Jun 2019, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> > Quoting Daniele Ceraolo Spurio (2019-05-31 23:24:07)
> >> Separate the display PM from the PCI-level runtime PM.
> >> I'll follow this up with v2 of the rpm encapsulation series [1], but
> >> I'd like to get this in before that to avoid having to carry this
> >> big dumb diff in that series.
> >
> > With RUNTIME_PM_DEBUG disabled,
> >
> > add/remove: 3/1 grow/shrink: 6/8 up/down: 396/-393 (3)
> > Function                                     old     new   delta
> > intel_runtime_pm_release                       -     274    +274
> > intel_runtime_pm_put_raw                       -      45     +45
> > intel_runtime_pm_put_unchecked                10      48     +38
> > intel_display_power_put_async_work           179     192     +13
> > intel_display_power_flush_work               117     126      +9
> > __intel_display_power_put_async              193     199      +6
> > intel_runtime_pm_get_raw                       -       4      +4
> > intel_display_power_grab_async_put_ref       117     121      +4
> > __warned                                     469     472      +3
> > intel_runtime_pm_get                          10       7      -3
> > intel_power_domains_enable                    38      33      -5
> > intel_display_power_put_unchecked             23      18      -5
> > intel_display_power_get_if_enabled           143     138      -5
> > intel_display_power_get                       84      79      -5
> > intel_power_domains_suspend                  490     480     -10
> > intel_power_domains_fini_hw                  116     106     -10
> > release_async_put_domains                    220     203     -17
> > __intel_runtime_pm_put.constprop             333       -    -333
> > Total: Before=23394388, After=23394391, chg +0.00%
> >
> > which is my biggest worry when meddling with these, that we accidentally
> > explode production code with unused debugging (all those wakerefs).
> >
> > Lgtm, I would like Jani to indicate that he's happy with this split as
> > well since he has been looking at very similar ideas.
> 
> I might bikeshed the naming, from the POV that functions would be nice
> to be (eventually) named based on the name of the file they reside
> in. But I guess intel_display_power.[ch] is as good as any I could come
> up with, and not everything is going to follow the naming pattern
> anyway.
> 
> I'd still like to get an ack from Imre before merging, but from my side
> this is,
> 
> Acked-by: Jani Nikula <jani.nikula@intel.com>
> 
> Thanks for doing this.

Keeping the display power related functions grouped in a separate file
makes sense to me:

Acked-by: Imre Deak <imre.deak@intel.com>

> 
> 
> 
> > -Chris
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx