Re: [PATCH] drm/i915/hwmon: Enable PL1 power limit

From: Matthew Auld <matthew.william.auld@gmail.com>
To: "Dixit, Ashutosh" <ashutosh.dixit@intel.com>
Cc: intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org,
	Rodrigo Vivi <rodrigo.vivi@intel.com>
Subject: Re: [PATCH] drm/i915/hwmon: Enable PL1 power limit
Date: Tue, 7 Feb 2023 19:00:19 +0000	[thread overview]
Message-ID: <CAM0jSHPrLEDqrWGV-ckTA2mPF3p5LkcSqTsFy0s86sZk+Wpz2g@mail.gmail.com> (raw)
In-Reply-To: <871qn1o0t8.wl-ashutosh.dixit@intel.com>

On Tue, 7 Feb 2023 at 17:19, Dixit, Ashutosh <ashutosh.dixit@intel.com> wrote:
>
> On Tue, 07 Feb 2023 08:12:25 -0800, Dixit, Ashutosh wrote:
> >
> > On Tue, 07 Feb 2023 01:32:44 -0800, Matthew Auld wrote:
> > >
> > > On Fri, 3 Feb 2023 at 15:54, Ashutosh Dixit <ashutosh.dixit@intel.com> wrote:
> > > >
> > > > Previous documentation suggested that PL1 power limit is always
> > > > enabled. However we now find this not to be the case on some
> > > > platforms (such as ATSM). Therefore enable PL1 power limit during hwmon
> > > > initialization.
> > >
> > > For some reason it looks like this change is impacting the atsm in CI:
> > > https://intel-gfx-ci.01.org/tree/drm-tip/bat-atsm-1.html
> >
> > Hmm, the change was meant for ATSM. Anyway let me try to get hold of an
> > ATSM and see if I can figure out what might be going on with these
> > seemingly unrelated failures and if I can repro them locally. Thanks!
>
> Rodrigo/Matt,
>
> I am proposing we revert this now and remerge again after investigating,
> even getting ATSM systems to investigate is not easy so it might take a few
> days to investigate. What do you guys think?

Yeah, maybe just revert for now.

>
> Thanks.
> --
> Ashutosh
>
>
> >
> > >
> > > >
> > > > Bspec: 51864
> > > >
> > > > v2: Add Bspec reference (Gwan-gyeong)
> > > > v3: Add Fixes tag
> > > >
> > > > Fixes: 99f55efb79114 ("drm/i915/hwmon: Power PL1 limit and TDP setting")
> > > > Signed-off-by: Ashutosh Dixit <ashutosh.dixit@intel.com>
> > > > Reviewed-by: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
> > > > ---
> > > >  drivers/gpu/drm/i915/i915_hwmon.c | 5 +++++
> > > >  1 file changed, 5 insertions(+)
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/i915_hwmon.c b/drivers/gpu/drm/i915/i915_hwmon.c
> > > > index 1225bc432f0d5..4683a5b96eff1 100644
> > > > --- a/drivers/gpu/drm/i915/i915_hwmon.c
> > > > +++ b/drivers/gpu/drm/i915/i915_hwmon.c
> > > > @@ -687,6 +687,11 @@ hwm_get_preregistration_info(struct drm_i915_private *i915)
> > > >                 for_each_gt(gt, i915, i)
> > > >                         hwm_energy(&hwmon->ddat_gt[i], &energy);
> > > >         }
> > > > +
> > > > +       /* Enable PL1 power limit */
> > > > +       if (i915_mmio_reg_valid(hwmon->rg.pkg_rapl_limit))
> > > > +               hwm_locked_with_pm_intel_uncore_rmw(ddat, hwmon->rg.pkg_rapl_limit,
> > > > +                                                   PKG_PWR_LIM_1_EN, PKG_PWR_LIM_1_EN);
> > > >  }
> > > >
> > > >  void i915_hwmon_register(struct drm_i915_private *i915)
> > > > --
> > > > 2.38.0
> > > >