[PATCH 2/2] drm/xe/vsec: Support BMG devices

[Ruhl, Michael J]

> -----Original Message-----
> From: Andy Shevchenko <andriy.shevchenko at linux.intel.com>
> Sent: Monday, November 11, 2024 3:40 AM
> To: Ruhl, Michael J <michael.j.ruhl at intel.com>
> Cc: intel-xe at lists.freedesktop.org; platform-driver-x86 at vger.kernel.org;
> david.e.box at linux.intel.com; ilpo.jarvinen at linux.intel.com;
> hdegoede at redhat.com; Vivi, Rodrigo <rodrigo.vivi at intel.com>; De Marchi,
> Lucas <lucas.demarchi at intel.com>
> Subject: Re: [PATCH 2/2] drm/xe/vsec: Support BMG devices
> 
> On Fri, Nov 08, 2024 at 03:19:54PM -0500, Michael J. Ruhl wrote:
> > The Battlemage (BMG) discrete graphics card supports the Platform,
> > Monitoring Technology (PMT) feature directly on the primary PCI
> > device.
> >
> > Utilize the PMT callback API to add support for the BMG devices.
> 
> ...
> 
> > +	drmm_mutex_init(&xe->drm, &xe->pmt.lock);
> > +
> 
> No error checks? What's the point of the (second) 'm' then?

Umm... not sure.  I will take a look.

 
> ...
> 
> > +/*
> > + * Copyright (c) 2024 Intel Corporation  */
> 
> Can be one line.
> 
> > +#include <linux/bitfield.h>
> > +#include <linux/bits.h>
> > +#include <linux/cleanup.h>
> 
> + errno.h
> 
> > +#include <linux/intel_vsec.h>
> > +#include <linux/module.h>
> > +#include <linux/mutex.h>
> > +#include <linux/pci.h>
> 
> + types.h
> 
> > +#include "xe_device.h"
> > +#include "xe_device_types.h"
> > +#include "xe_drv.h"
> > +#include "xe_mmio.h"
> > +#include "xe_platform_types.h"
> > +#include "xe_pm.h"
> > +#include "xe_vsec.h"
> 
> + blank line?
> 
> > +#include "regs/xe_pmt.h"
> 
> ...
> 
> 
> I would add a comment here explaining that it's not a PCI ID, while looking as
> that.

Very good point I will add a comment.

> > +#define BMG_DEVICE_ID 0xE2F8
> 
> ...
> 
> > +static int guid_decode(u32 guid, int *index, u32 *offset)
> 
> xe_guid_decode() to avoid any possible churn in the future due to namespace
> collision.

Will do.
 
> ...
> 
> > +++ b/drivers/gpu/drm/xe/xe_vsec.h
> 
> 
> > +/*
> > + * Copyright (c) 2024 Intel Corporation  */
> 
> One line?

Will do.

Thank you for your comments!.  Will get them addressed.

M

> --
> With Best Regards,
> Andy Shevchenko
>