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

[Andy Shevchenko <andriy.shevchenko@linux.intel.com>]

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?

...

> +/*
> + * Copyright © 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.

> +#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.

...

> +++ b/drivers/gpu/drm/xe/xe_vsec.h


> +/*
> + * Copyright © 2024 Intel Corporation
> + */

One line?

-- 
With Best Regards,
Andy Shevchenko