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

[Lizhi Hou <lizhi.hou@amd.com>]

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);