Re: [PATCH V3 02/11] accel/amdxdna: Add a new driver for AMD AI Engine

[Lizhi Hou]

On 10/4/24 10:21, Jeffrey Hugo wrote:
> On 9/11/2024 12:05 PM, Lizhi Hou wrote:
> > diff --git a/drivers/accel/amdxdna/aie2_pci.c 
> > b/drivers/accel/amdxdna/aie2_pci.c
> > new file mode 100644
> > index 000000000000..e21b32557fc2
> > --- /dev/null
> > +++ b/drivers/accel/amdxdna/aie2_pci.c
> > @@ -0,0 +1,183 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright (C) 2023-2024, Advanced Micro Devices, Inc.
> > + */
> > +
> > +#include <drm/drm_device.h>
> > +#include <drm/drm_managed.h>
> > +#include <drm/drm_print.h>
> > +#include <linux/errno.h>
> > +#include <linux/firmware.h>
> > +#include <linux/iommu.h>
> > +#include <linux/pci.h>
> > +
> > +#include "amdxdna_pci_drv.h"
> > +#include "aie2_pci.h"
>
> Feels like a blank line is missing here.
sure. Will add it.
> > +static void aie2_hw_stop(struct amdxdna_dev *xdna)
> > +{
> > +    struct pci_dev *pdev = to_pci_dev(xdna->ddev.dev);
> > +    struct amdxdna_dev_hdl *ndev = xdna->dev_handle;
> > +
> > +    aie2_psp_stop(ndev->psp_hdl);
> > +    aie2_smu_fini(ndev);
> > +    pci_disable_device(pdev);
> > +}
> > diff --git a/drivers/accel/amdxdna/aie2_pci.h 
> > b/drivers/accel/amdxdna/aie2_pci.h
> > new file mode 100644
> > index 000000000000..b3c360a10c04
> > --- /dev/null
> > +++ b/drivers/accel/amdxdna/aie2_pci.h
> > @@ -0,0 +1,130 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +/*
> > + * Copyright (C) 2023-2024, Advanced Micro Devices, Inc.
> > + */
> > +
> > +#ifndef _AIE2_PCI_H_
> > +#define _AIE2_PCI_H_
> > +
> > +#define AIE2_INTERVAL    20000    /* us */
> > +#define AIE2_TIMEOUT    1000000    /* us */
> > +
> > +/* Firmware determines device memory base address and size */
> > +#define AIE2_DEVM_BASE    0x4000000
> > +#define AIE2_DEVM_SIZE    (64 * SZ_1M)
>
> SZ_64M? Then the code, and reader's brains, are not doing multiplication.
ok.
> > diff --git a/drivers/accel/amdxdna/amdxdna_pci_drv.c 
> > b/drivers/accel/amdxdna/amdxdna_pci_drv.c
> > new file mode 100644
> > index 000000000000..33133f2503cd
> > --- /dev/null
> > +++ b/drivers/accel/amdxdna/amdxdna_pci_drv.c
> > @@ -0,0 +1,128 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright (C) 2022-2024, Advanced Micro Devices, Inc.
> > + */
> > +
> > +#include <drm/drm_accel.h>
> > +#include <drm/drm_drv.h>
> > +#include <drm/drm_gem.h>
> > +#include <drm/drm_ioctl.h>
> > +#include <drm/drm_managed.h>
> > +#include <linux/pci.h>
> > +
> > +#include "amdxdna_pci_drv.h"
> > +
> > +/*
> > + *  Bind the driver base on PCI class and later use the (device_id, 
> > rev_id)
> > + *  pair as a key to select the devices. The devices with same 
> > device_id
> > + *  have very similar interface to host driver.
> > + */
>
> Looks like this comment didn't get updated when the class matching was 
> removed.

I will fix the comment.


Thanks,

Lizhi

> > +static const struct pci_device_id pci_ids[] = {
> > +    { PCI_DEVICE(PCI_VENDOR_ID_AMD, 0x1502) },
> > +    { PCI_DEVICE(PCI_VENDOR_ID_AMD, 0x17f0) },
> > +    {0}
> > +};
> > +
> > +MODULE_DEVICE_TABLE(pci, pci_ids);