[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <d21c069438cc9c314e29fe9b75a97cf15ab6e087.camel@linux.intel.com>
Date: Mon, 12 Aug 2024 10:09:10 -0700
From: srinivas pandruvada <srinivas.pandruvada@...ux.intel.com>
To: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
Cc: Hans de Goede <hdegoede@...hat.com>, Andy Shevchenko
<andriy.shevchenko@...ux.intel.com>, platform-driver-x86@...r.kernel.org,
LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] platform/x86/intel-uncore-freq: Do not present separate
package-die domain
Hi Ilpo,
On Mon, 2024-08-12 at 14:16 +0300, Ilpo Järvinen wrote:
> On Wed, 31 Jul 2024, Srinivas Pandruvada wrote:
>
> > The scope of uncore control is per power domain in a package and
> > die.
> > A package-die can have multiple power domains on some processors.
> > In this
> > case package-die domain (root domain) aggregates all information
> > from
> > power domains in it.
> >
> > On some processors, CPUID enumerates the die number same as power
> > domain
> > ID. In this case there is one to one relationship between package-
> > die and
> > power domain ID. There is no use of aggregating information from
> > all
> > power domain IDs as the information will be duplicate and
> > confusing. In
> > this case do not create separate package-die domain.
>
> Hi Srinivas,
>
> I got confused by this changelog because its order is quite
> illogical.
>
> First paragraph talks about case A. When you say "all information"
> is "aggregated", I immediately make the assumption that the
> aggregated
> information is what is wanted because, well, you normally want "all
> information" and nothing else is being told here.
>
> Second paragraph starts to talk about case B and then suddenly
> switches to
> talk what should have been done in case A (that aggregated
> information is
> useless/confusing).
>
Is this any better:
"
The scope of uncore control is per power domain in a package and die
with TPMI.
There are two types of processor configurations possible:
1. A compute die is not enumerated in CPUID. In this case there is only
one die in a package. In this case there will be multiple power domains
in a single die.
2. A power domain in a package is enumerated as a compute die in CPUID.
So there is one to one relationship between a die and power domain.
To allow die level controls, the current implementation creates a root
domain and aggregates all information from power domains in it. This
is well suited for configuration 1 above.
But when newer processors use configuration 2 above, this will present
redundant information, So no use of aggregating. In this case do not
create separate root domain.
"
Thanks,
Srinivas
> So I think some reorganization of the sentences would be useful to
> not
> jump between cases mid-paragraph without any hints.
>
> (I hope my explanation is enough to highlight why it was hard to
> follow).
>
> When I finally understood what the changelog was saying, I found out
> the
> code change is fine too but it first looked like it was doing exactly
> the
> opposite to my expectations/reasonale given in the changelog.
>
Powered by blists - more mailing lists