Re: [PATCH V2 00/13] intel_rapl: RAPL abstraction and MMIO RAPL support

[Zhang Rui <rui.zhang@intel.com>]

On 一, 2019-07-08 at 10:58 +0200, Rafael J. Wysocki wrote:
> On Mon, Jul 8, 2019 at 8:53 AM Zhang Rui <rui.zhang@intel.com> wrote:
> > 
> > 
> > On 六, 2019-07-06 at 10:19 +0200, Rafael J. Wysocki wrote:
> > > 
> > > On Fri, Jul 5, 2019 at 4:59 PM Pandruvada, Srinivas
> > > <srinivas.pandruvada@intel.com> wrote:
> > > > 
> > > > 
> > > > 
> > > > On Fri, 2019-07-05 at 10:57 +0200, Rafael J. Wysocki wrote:
> > > > > 
> > > > > 
> > > > > On Thu, Jul 4, 2019 at 6:34 PM Zhang Rui <rui.zhang@intel.com
> > > > > >
> > > > > wrote:
> > > > > > 
> > > > > > 
> > > > > > 
> > > > > > Besideis MSR interface, RAPL can also be controlled via the
> > > > > > MMIO
> > > > > > interface,
> > > > > > by accessing the MCHBar registers exposed by the processor
> > > > > > thermal
> > > > > > device.
> > > > > > 
> > > > > > Currently, we only have RAPL MSR interface in Linux kernel,
> > > > > > this
> > > > > > brings
> > > > > > problems on some platforms that BIOS performs a low power
> > > > > > limits
> > > > > > via the
> > > > > > MMIO interface by default. This results in poor system
> > > > > > performance,
> > > > > > and there is no way for us to change the MMIO MSR setting
> > > > > > in
> > > > > > Linux.
> > > > > > 
> > > > > > To fix this, RAPL MMIO interface support is introduced in
> > > > > > this
> > > > > > patch set.
> > > > > > 
> > > > > > Patch 1/13 to patch 11/13 abstract the RAPL code, and move
> > > > > > all
> > > > > > the
> > > > > > shared
> > > > > > code into a separate file, intel_rapl_common.c, so that it
> > > > > > can
> > > > > > be
> > > > > > used
> > > > > > by both MSR and MMIO interfaces.
> > > > > > Patch 12/13 introduced RAPL support via MMIO registers,
> > > > > > exposed
> > > > > > by
> > > > > > the
> > > > > > processor thermal devices.
> > > > > > Patch 13/13 fixes a module autoloading issue found later.
> > > > > > 
> > > > > > The patch series has been tested on Dell XPS 9360, a SKL
> > > > > > platform.
> > > > > > 
> > > > > > Note that this patch series are based on the -tip tree,
> > > > > > which
> > > > > > contains the
> > > > > > latest RAPL changes for multi-die support.
> > > > > > 
> > > > > > Changes in V2:
> > > > > >    - add kerneldoc for struct rapl_if_priv.
> > > > > >    - use intel_rapl_msr.c for RAPL MSR I/F driver, instead
> > > > > > of
> > > > > > intel_rapl.c.
> > > > > >    - changelog and coding style update.
> > > > > What tree is the series against?
> > > > > 
> > > > > It doesn't apply either on top of my powercap branch or on
> > > > > top of
> > > > > 5.2-rc7 for me.
> > > > This needs linux tip tree. There are some package/die changes
> > > > in
> > > > tip
> > > > tree, which this patch depends on.
> > > OK, so the changes in -tip need to go in first.
> > > 
> > exactly.
> > BTW, this patch set also conflicts with the RAPL support patches
> > for
> > icelake platforms.
> Do you mean commits
> 
> cc3ae777098b..88679b2587a0
> 
> in linux-next?
> 
yes.
> > 
> > Thus IMO, having a separate rapl branch, and apply
> > the icl rapl patches on top of this patch set will be much easier.
> OK, I'll do that if that's preferred.

thanks. I think you can apply the ICL RAPL patches by replacing
intel_rapl.c with intel_rapl_common.c in the patch files. Or I can
resend the ICL RAPL patches rebased on the this patch series.

thanks,
rui