On Tue, Apr 26, 2022 at 02:08:00PM +0800, Cai Huoqing wrote: > The NVIDIA Deep Learning Accelerator (NVDLA) is an open source IP > which is integrated into NVIDIA Jetson AGX Xavier, > so add register head file of this accelerator. > > Signed-off-by: Cai Huoqing <cai.huoqing@xxxxxxxxx> > --- > drivers/gpu/drm/nvdla/nvdla_reg.h | 6411 +++++++++++++++++++++++++++++ > 1 file changed, 6411 insertions(+) > create mode 100644 drivers/gpu/drm/nvdla/nvdla_reg.h You probably want to change the ordering of the patches a little because this new header file is already being used in patch 2. With the current ordering you'll break the build between patches 2 and 3. The same will likely happen with patch 4 because I'm assuming (haven't looked yet) that the UAPI structures are getting used in the driver code as well. Other than that, not much to say about this. One note perhaps, see below. > > diff --git a/drivers/gpu/drm/nvdla/nvdla_reg.h b/drivers/gpu/drm/nvdla/nvdla_reg.h > new file mode 100644 > index 000000000000..5ca2897405bc > --- /dev/null > +++ b/drivers/gpu/drm/nvdla/nvdla_reg.h > @@ -0,0 +1,6411 @@ > +/* SPDX-License-Identifier: GPL-2.0 OR BSD-3-Clause */ > +/* > + * Copyright (C) 2017-2018 NVIDIA CORPORATION. > + * Copyright (C) 2022 Cai Huoqing > + */ > + > +#ifndef __NVDLA_REG_H_ > +#define __NVDLA_REG_H_ > + > +// Register NVDLA_CFGROM_CFGROM_HW_VERSION_0 > +#define NVDLA_CFGROM_CFGROM_HW_VERSION_0 _MK_ADDR_CONST(0x0) > +#define NVDLA_CFGROM_CFGROM_HW_VERSION_0_HW_VERSION_SHIFT _MK_SHIFT_CONST(0) > +#define NVDLA_CFGROM_CFGROM_HW_VERSION_0_HW_VERSION_FIELD _MK_FIELD_CONST(0xffffffff, NVDLA_CFGROM_CFGROM_HW_VERSION_0_HW_VERSION_SHIFT) I know that people have in the past expressed reluctance about the way that these fields are defined. I personally don't like these very much because they are very redundant (e.g. that CFGROM_ is duplicated for no obvious reason). I also think those _MK_* macros are very difficult to understand because they can be found nowhere else in the Linux sources so people aren't used to the format. And in fact the Linux kernel has its own set of macros to define fields. I also realize that this would probably be a pain to change. Let me see if I can find out how exactly those get generated, so perhaps there's a way to get them generated into a format that more closely matches the idioms used in Linux. > +// > +// ADDRESS SPACES > +// > + > +#define BASE_ADDRESS_NVDLA_CFGROM 0x0 > +#define BASE_ADDRESS_NVDLA_GLB 0x1000 > +#define BASE_ADDRESS_NVDLA_MCIF 0x2000 > +#define BASE_ADDRESS_NVDLA_CDMA 0x3000 > +#define BASE_ADDRESS_NVDLA_CSC 0x4000 > +#define BASE_ADDRESS_NVDLA_CMAC_A 0x5000 > +#define BASE_ADDRESS_NVDLA_CMAC_B 0x6000 > +#define BASE_ADDRESS_NVDLA_CACC 0x7000 > +#define BASE_ADDRESS_NVDLA_SDP_RDMA 0x8000 > +#define BASE_ADDRESS_NVDLA_SDP 0x9000 > +#define BASE_ADDRESS_NVDLA_PDP_RDMA 0xa000 > +#define BASE_ADDRESS_NVDLA_PDP 0xb000 > +#define BASE_ADDRESS_NVDLA_CDP_RDMA 0xc000 > +#define BASE_ADDRESS_NVDLA_CDP 0xd000 > +#define BASE_ADDRESS_NVDLA_GEC 0xe000 > +#define BASE_ADDRESS_NVDLA_CVIF 0xf000 > +#define BASE_ADDRESS_NVDLA_BDMA 0x10000 > +#define BASE_ADDRESS_NVDLA_RBK 0x11000 I'm wondering if it might make sense to turn these into separate reg entries in the device tree, though it might not be worth it. The one concern I have is that these might change internally in a newer revision, which might then make it a pain to adapt the driver to the differing offsets. That said, I'm not an expert on DLA, so I don't know how this has evolved in newer chip. I'll try to track down our local experts so that they can clarify a few things for us. Thierry
Attachment:
signature.asc
Description: PGP signature
