RE: [PATCH 2/5] drm/xe/vsec: Use correct pm state get

["Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>]

[-- Attachment #1: Type: text/plain, Size: 2783 bytes --]

On Wed, 28 Jan 2026, Ruhl, Michael J wrote:

> >-----Original Message-----
> >From: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> >Sent: Wednesday, January 28, 2026 7:42 AM
> >To: Ruhl, Michael J <michael.j.ruhl@intel.com>
> >Cc: platform-driver-x86@vger.kernel.org; intel-xe@lists.freedesktop.org; Hans
> >de Goede <hansg@kernel.org>; Brost, Matthew <matthew.brost@intel.com>;
> >Vivi, Rodrigo <rodrigo.vivi@intel.com>; thomas.hellstrom@linux.intel.com;
> >airlied@gmail.com; simona@ffwll.ch; david.e.box@linux.intel.com
> >Subject: Re: [PATCH 2/5] drm/xe/vsec: Use correct pm state get
> >
> >On Tue, 27 Jan 2026, Michael J. Ruhl wrote:
> >
> >> Crashlog needs to be collected at all times.  The current pm
> >> check assumes telemetry only.
> >>
> >> Update read path to enable device for crashlog instances.
> >>
> >> Signed-off-by: Michael J. Ruhl <michael.j.ruhl@intel.com>
> >> ---
> >>  drivers/gpu/drm/xe/xe_vsec.c | 16 +++++++++++++---
> >>  1 file changed, 13 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/xe/xe_vsec.c b/drivers/gpu/drm/xe/xe_vsec.c
> >> index 4ebb4dbe1c9b..44607f1eaa88 100644
> >> --- a/drivers/gpu/drm/xe/xe_vsec.c
> >> +++ b/drivers/gpu/drm/xe/xe_vsec.c
> >> @@ -145,6 +145,7 @@ int xe_pmt_telem_read(struct pci_dev *pdev, u32
> >guid, u64 *data, loff_t user_off
> >>  {
> >>  	struct xe_device *xe = pdev_to_xe_device(pdev);
> >>  	void __iomem *telem_addr = xe->mmio.regs +
> >BMG_TELEMETRY_OFFSET;
> >> +	u32 cap_type = FIELD_GET(GUID_CAP_TYPE, guid);
> >>  	u32 mem_region;
> >>  	u32 offset;
> >>  	int ret;
> >> @@ -160,9 +161,18 @@ int xe_pmt_telem_read(struct pci_dev *pdev, u32
> >guid, u64 *data, loff_t user_off
> >>  	if (!xe->soc_remapper.set_telem_region)
> >>  		return -ENODEV;
> >>
> >> -	/* indicate that we are not at an appropriate power level */
> >> -	if (!xe_pm_runtime_get_if_active(xe))
> >> -		return -ENODATA;
> >> +	/* make sure that we are not at an inappropriate power level */
> >> +	switch (cap_type) {
> >> +	case CRASHLOG:
> >> +		xe_pm_runtime_get(xe);
> >
> >While this is xe code so I don't know much about it, it feels odd to me to
> >just add a get without put or explanation in the changelog why this change
> >doesn't result in imbalance.
> 
> There is an existing xe_pm_runtime_put() before this function exits.  (original
> line ~170, new line ~181).  Is this what you are looking for?
> 
> The TELEM entry will not take the reference if the device is not powered,
> so a put is unnecessary (exit on not enabled).
> 
> CRASLOG requires power, so the _get() will power the device and take
> reference.

I see, I didn't look up the function but just read the patch and found it 
odd. I guess it's fine as is.

-- 
 i.