Message ID | 20230607105551.568639-7-15330273260@189.cn (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | drm/etnaviv: add pci device driver support | expand |
On Wed, Jun 07, 2023 at 06:55:49PM +0800, Sui Jingfeng wrote: > From: Sui Jingfeng <suijingfeng@loongson.cn> > > This patch adds PCI driver support on top of what we already have. Take > the GC1000 in LS7A1000/LS2K1000 as the first instance of the PCI device > driver. There is only one GPU core for the GC1000 in the LS7A1000 and > LS2K1000. Therefore, component frameworks can be avoided. > +#ifdef CONFIG_DRM_ETNAVIV_PCI_DRIVER > +#include "etnaviv_pci_drv.h" > +#endif With trivial stubs for etnaviv_register_pci_driver() and etnaviv_unregister_pci_driver(), I think you could get rid of all these #ifdefs. > +void etnaviv_drm_unbind(struct device *dev, bool component) > { > struct etnaviv_drm_private *priv = etna_private_ptr; > struct drm_device *drm = priv->drm; > @@ -746,6 +750,12 @@ static int __init etnaviv_init(void) > if (ret != 0) > goto unregister_gpu_driver; > > +#ifdef CONFIG_DRM_ETNAVIV_PCI_DRIVER > + ret = etnaviv_register_pci_driver(); > + if (ret != 0) > + goto unregister_platform_driver; > +#endif > + > /* > * If the DT contains at least one available GPU device, instantiate > * the DRM platform device. > @@ -763,7 +773,7 @@ static int __init etnaviv_init(void) > break; > } > > - return 0; > + return ret; > > unregister_platform_driver: > platform_driver_unregister(&etnaviv_platform_driver); > @@ -778,6 +788,10 @@ static void __exit etnaviv_exit(void) > etnaviv_destroy_platform_device(&etnaviv_platform_device); > platform_driver_unregister(&etnaviv_platform_driver); > platform_driver_unregister(&etnaviv_gpu_driver); > + > +#ifdef CONFIG_DRM_ETNAVIV_PCI_DRIVER > + etnaviv_unregister_pci_driver(); > +#endif > +static const struct pci_device_id etnaviv_pci_id_lists[] = { > + {PCI_VENDOR_ID_LOONGSON, 0x7a15, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0}, > + {PCI_VENDOR_ID_LOONGSON, 0x7a05, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0}, PCI_VDEVICE() Bjorn
Hi, On 2023/6/9 01:32, Bjorn Helgaas wrote: > On Wed, Jun 07, 2023 at 06:55:49PM +0800, Sui Jingfeng wrote: >> From: Sui Jingfeng <suijingfeng@loongson.cn> >> >> This patch adds PCI driver support on top of what we already have. Take >> the GC1000 in LS7A1000/LS2K1000 as the first instance of the PCI device >> driver. There is only one GPU core for the GC1000 in the LS7A1000 and >> LS2K1000. Therefore, component frameworks can be avoided. >> +#ifdef CONFIG_DRM_ETNAVIV_PCI_DRIVER >> +#include "etnaviv_pci_drv.h" >> +#endif > With trivial stubs for etnaviv_register_pci_driver() and > etnaviv_unregister_pci_driver(), I think you could get rid of all > these #ifdefs. OK, then, I will try to add dummy implement at etnaviv_pci_drv.h, Thanks. >> +void etnaviv_drm_unbind(struct device *dev, bool component) >> { >> struct etnaviv_drm_private *priv = etna_private_ptr; >> struct drm_device *drm = priv->drm; >> @@ -746,6 +750,12 @@ static int __init etnaviv_init(void) >> if (ret != 0) >> goto unregister_gpu_driver; >> >> +#ifdef CONFIG_DRM_ETNAVIV_PCI_DRIVER >> + ret = etnaviv_register_pci_driver(); >> + if (ret != 0) >> + goto unregister_platform_driver; >> +#endif >> + >> /* >> * If the DT contains at least one available GPU device, instantiate >> * the DRM platform device. >> @@ -763,7 +773,7 @@ static int __init etnaviv_init(void) >> break; >> } >> >> - return 0; >> + return ret; >> >> unregister_platform_driver: >> platform_driver_unregister(&etnaviv_platform_driver); >> @@ -778,6 +788,10 @@ static void __exit etnaviv_exit(void) >> etnaviv_destroy_platform_device(&etnaviv_platform_device); >> platform_driver_unregister(&etnaviv_platform_driver); >> platform_driver_unregister(&etnaviv_gpu_driver); >> + >> +#ifdef CONFIG_DRM_ETNAVIV_PCI_DRIVER >> + etnaviv_unregister_pci_driver(); >> +#endif >> +static const struct pci_device_id etnaviv_pci_id_lists[] = { >> + {PCI_VENDOR_ID_LOONGSON, 0x7a15, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0}, >> + {PCI_VENDOR_ID_LOONGSON, 0x7a05, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0}, > PCI_VDEVICE() This make it impossible to hook device-specific data in the future. But currently there no device specific data associated with the 0x7a05 and 0x7a15, so it's acceptable for now. Thanks. > Bjorn
On Fri, Jun 09, 2023 at 09:37:02AM +0800, Sui Jingfeng wrote: > On 2023/6/9 01:32, Bjorn Helgaas wrote: > > On Wed, Jun 07, 2023 at 06:55:49PM +0800, Sui Jingfeng wrote: > > > From: Sui Jingfeng <suijingfeng@loongson.cn> > > > > > > This patch adds PCI driver support on top of what we already have. Take > > > the GC1000 in LS7A1000/LS2K1000 as the first instance of the PCI device > > > driver. There is only one GPU core for the GC1000 in the LS7A1000 and > > > LS2K1000. Therefore, component frameworks can be avoided. > > > + {PCI_VENDOR_ID_LOONGSON, 0x7a15, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0}, > > > + {PCI_VENDOR_ID_LOONGSON, 0x7a05, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0}, > > > > PCI_VDEVICE() > > This make it impossible to hook device-specific data in the future. > > But currently there no device specific data associated with the > 0x7a05 and 0x7a15, > > so it's acceptable for now. Thanks. Haha, ISTR having this conversation before, sorry for repeating it. Indeed, it's fine as-is. But PCI_VDEVICE() actually *does* allow for vendor-specific data because it doesn't include the data element, which defaults to zero if you don't specify it. So for example, drivers/net/ethernet/realtek/r8169_main.c has this: { PCI_VDEVICE(REALTEK, 0x8129) }, { PCI_VDEVICE(REALTEK, 0x8136), RTL_CFG_NO_GBIT }, where 0x8129 has no driver_data (it defaults to zero), but 0x8136 does. Bjorn
Hi, On 2023/6/10 01:52, Bjorn Helgaas wrote: > On Fri, Jun 09, 2023 at 09:37:02AM +0800, Sui Jingfeng wrote: >> On 2023/6/9 01:32, Bjorn Helgaas wrote: >>> On Wed, Jun 07, 2023 at 06:55:49PM +0800, Sui Jingfeng wrote: >>>> From: Sui Jingfeng <suijingfeng@loongson.cn> >>>> >>>> This patch adds PCI driver support on top of what we already have. Take >>>> the GC1000 in LS7A1000/LS2K1000 as the first instance of the PCI device >>>> driver. There is only one GPU core for the GC1000 in the LS7A1000 and >>>> LS2K1000. Therefore, component frameworks can be avoided. >>>> + {PCI_VENDOR_ID_LOONGSON, 0x7a15, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0}, >>>> + {PCI_VENDOR_ID_LOONGSON, 0x7a05, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0}, >>> PCI_VDEVICE() >> This make it impossible to hook device-specific data in the future. >> >> But currently there no device specific data associated with the >> 0x7a05 and 0x7a15, >> >> so it's acceptable for now. Thanks. > Haha, ISTR having this conversation before, sorry for repeating it. > > Indeed, it's fine as-is. But PCI_VDEVICE() actually *does* allow for > vendor-specific data because it doesn't include the data element, > which defaults to zero if you don't specify it. > > So for example, drivers/net/ethernet/realtek/r8169_main.c has this: > > { PCI_VDEVICE(REALTEK, 0x8129) }, > { PCI_VDEVICE(REALTEK, 0x8136), RTL_CFG_NO_GBIT }, > > where 0x8129 has no driver_data (it defaults to zero), but 0x8136 > does. Yeah, I'm wrong. PCI_VDEVICE macro end with two zero. (I thought it was three) Thanks for the education. With those lessons learned, I somewhat know how to create patch. It should meet community's requirement before sending. I'm too naive in the before. Thanks a lot, really. > Bjorn
On Sat, Jun 10, 2023 at 02:07:58AM +0800, Sui Jingfeng wrote: > On 2023/6/10 01:52, Bjorn Helgaas wrote: > > On Fri, Jun 09, 2023 at 09:37:02AM +0800, Sui Jingfeng wrote: > > > On 2023/6/9 01:32, Bjorn Helgaas wrote: > > > > On Wed, Jun 07, 2023 at 06:55:49PM +0800, Sui Jingfeng wrote: > > > > > From: Sui Jingfeng <suijingfeng@loongson.cn> > > > > > > > > > > This patch adds PCI driver support on top of what we already have. Take > > > > > the GC1000 in LS7A1000/LS2K1000 as the first instance of the PCI device > > > > > driver. There is only one GPU core for the GC1000 in the LS7A1000 and > > > > > LS2K1000. Therefore, component frameworks can be avoided. > > > > > + {PCI_VENDOR_ID_LOONGSON, 0x7a15, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0}, > > > > > + {PCI_VENDOR_ID_LOONGSON, 0x7a05, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0}, > > > > PCI_VDEVICE() > > > This make it impossible to hook device-specific data in the future. > > > > > > But currently there no device specific data associated with the > > > 0x7a05 and 0x7a15, > > > > > > so it's acceptable for now. Thanks. > > Haha, ISTR having this conversation before, sorry for repeating it. > > > > Indeed, it's fine as-is. But PCI_VDEVICE() actually *does* allow for > > vendor-specific data because it doesn't include the data element, > > which defaults to zero if you don't specify it. > > > > So for example, drivers/net/ethernet/realtek/r8169_main.c has this: > > > > { PCI_VDEVICE(REALTEK, 0x8129) }, > > { PCI_VDEVICE(REALTEK, 0x8136), RTL_CFG_NO_GBIT }, > > > > where 0x8129 has no driver_data (it defaults to zero), but 0x8136 > > does. > > PCI_VDEVICE macro end with two zero. (I thought it was three) No worries, I thought the same thing the first five times I read it :)
diff --git a/drivers/gpu/drm/etnaviv/Kconfig b/drivers/gpu/drm/etnaviv/Kconfig index faa7fc68b009..1b5b162efb61 100644 --- a/drivers/gpu/drm/etnaviv/Kconfig +++ b/drivers/gpu/drm/etnaviv/Kconfig @@ -15,6 +15,16 @@ config DRM_ETNAVIV help DRM driver for Vivante GPUs. +config DRM_ETNAVIV_PCI_DRIVER + bool "enable ETNAVIV PCI driver support" + depends on DRM_ETNAVIV + depends on PCI + default y + help + Compile in support for PCI GPUs of Vivante. + For example, the GC1000 in LS7A1000 and LS2K1000. + Say Y if you have such a hardware. + config DRM_ETNAVIV_THERMAL bool "enable ETNAVIV thermal throttling" depends on DRM_ETNAVIV diff --git a/drivers/gpu/drm/etnaviv/Makefile b/drivers/gpu/drm/etnaviv/Makefile index 46e5ffad69a6..6829e1ebf2db 100644 --- a/drivers/gpu/drm/etnaviv/Makefile +++ b/drivers/gpu/drm/etnaviv/Makefile @@ -16,4 +16,6 @@ etnaviv-y := \ etnaviv_perfmon.o \ etnaviv_sched.o +etnaviv-$(CONFIG_DRM_ETNAVIV_PCI_DRIVER) += etnaviv_pci_drv.o + obj-$(CONFIG_DRM_ETNAVIV) += etnaviv.o diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c b/drivers/gpu/drm/etnaviv/etnaviv_drv.c index 93ca240cd4c0..033afe542a3a 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c +++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c @@ -23,6 +23,10 @@ #include "etnaviv_mmu.h" #include "etnaviv_perfmon.h" +#ifdef CONFIG_DRM_ETNAVIV_PCI_DRIVER +#include "etnaviv_pci_drv.h" +#endif + /* * etnaviv private data construction and destructions: */ @@ -538,7 +542,7 @@ static const struct drm_driver etnaviv_drm_driver = { static struct etnaviv_drm_private *etna_private_ptr; -static int etnaviv_drm_bind(struct device *dev, bool component) +int etnaviv_drm_bind(struct device *dev, bool component) { struct etnaviv_drm_private *priv; struct drm_device *drm; @@ -588,7 +592,7 @@ static int etnaviv_drm_bind(struct device *dev, bool component) return ret; } -static void etnaviv_drm_unbind(struct device *dev, bool component) +void etnaviv_drm_unbind(struct device *dev, bool component) { struct etnaviv_drm_private *priv = etna_private_ptr; struct drm_device *drm = priv->drm; @@ -746,6 +750,12 @@ static int __init etnaviv_init(void) if (ret != 0) goto unregister_gpu_driver; +#ifdef CONFIG_DRM_ETNAVIV_PCI_DRIVER + ret = etnaviv_register_pci_driver(); + if (ret != 0) + goto unregister_platform_driver; +#endif + /* * If the DT contains at least one available GPU device, instantiate * the DRM platform device. @@ -763,7 +773,7 @@ static int __init etnaviv_init(void) break; } - return 0; + return ret; unregister_platform_driver: platform_driver_unregister(&etnaviv_platform_driver); @@ -778,6 +788,10 @@ static void __exit etnaviv_exit(void) etnaviv_destroy_platform_device(&etnaviv_platform_device); platform_driver_unregister(&etnaviv_platform_driver); platform_driver_unregister(&etnaviv_gpu_driver); + +#ifdef CONFIG_DRM_ETNAVIV_PCI_DRIVER + etnaviv_unregister_pci_driver(); +#endif } module_exit(etnaviv_exit); diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.h b/drivers/gpu/drm/etnaviv/etnaviv_drv.h index e58f82e698de..9cd72948cfad 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_drv.h +++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.h @@ -83,6 +83,9 @@ bool etnaviv_cmd_validate_one(struct etnaviv_gpu *gpu, u32 *stream, unsigned int size, struct drm_etnaviv_gem_submit_reloc *relocs, unsigned int reloc_size); +int etnaviv_drm_bind(struct device *dev, bool component); +void etnaviv_drm_unbind(struct device *dev, bool component); + #ifdef CONFIG_DEBUG_FS void etnaviv_gem_describe_objects(struct etnaviv_drm_private *priv, struct seq_file *m); diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c index a6ac4c15b231..c5bc906936e4 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c @@ -1868,8 +1868,8 @@ static int etnaviv_gpu_register_irq(struct etnaviv_gpu *gpu, int irq) /* platform independent */ -static int etnaviv_gpu_driver_create(struct device *dev, void __iomem *mmio, - int irq, bool component, bool has_clk) +int etnaviv_gpu_driver_create(struct device *dev, void __iomem *mmio, + int irq, bool component, bool has_clk) { struct etnaviv_gpu *gpu; int err; @@ -1918,7 +1918,7 @@ static int etnaviv_gpu_driver_create(struct device *dev, void __iomem *mmio, return 0; } -static void etnaviv_gpu_driver_destroy(struct device *dev, bool component) +void etnaviv_gpu_driver_destroy(struct device *dev, bool component) { if (component) component_del(dev, &gpu_ops); @@ -1969,7 +1969,7 @@ static int etnaviv_gpu_rpm_resume(struct device *dev) return 0; } -static const struct dev_pm_ops etnaviv_gpu_pm_ops = { +const struct dev_pm_ops etnaviv_gpu_pm_ops = { RUNTIME_PM_OPS(etnaviv_gpu_rpm_suspend, etnaviv_gpu_rpm_resume, NULL) }; diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.h b/drivers/gpu/drm/etnaviv/etnaviv_gpu.h index 1ec829a649b5..8d9833996ed7 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.h +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.h @@ -209,6 +209,12 @@ void etnaviv_gpu_start_fe(struct etnaviv_gpu *gpu, u32 address, u16 prefetch); int etnaviv_gpu_bind(struct device *dev, struct device *master, void *data); void etnaviv_gpu_unbind(struct device *dev, struct device *master, void *data); +int etnaviv_gpu_driver_create(struct device *dev, void __iomem *mmio, + int irq, bool component, bool has_clk); + +void etnaviv_gpu_driver_destroy(struct device *dev, bool component); + extern struct platform_driver etnaviv_gpu_driver; +extern const struct dev_pm_ops etnaviv_gpu_pm_ops; #endif /* __ETNAVIV_GPU_H__ */ diff --git a/drivers/gpu/drm/etnaviv/etnaviv_pci_drv.c b/drivers/gpu/drm/etnaviv/etnaviv_pci_drv.c new file mode 100644 index 000000000000..3cf19c2f0425 --- /dev/null +++ b/drivers/gpu/drm/etnaviv/etnaviv_pci_drv.c @@ -0,0 +1,75 @@ +// SPDX-License-Identifier: GPL-2.0 + +#include <linux/pci.h> + +#include "etnaviv_drv.h" +#include "etnaviv_gpu.h" +#include "etnaviv_pci_drv.h" + +static int etnaviv_pci_probe(struct pci_dev *pdev, + const struct pci_device_id *ent) +{ + struct device *dev = &pdev->dev; + void __iomem *mmio; + int ret; + + ret = pcim_enable_device(pdev); + if (ret) { + dev_err(dev, "failed to enable\n"); + return ret; + } + + pci_set_master(pdev); + + ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32)); + if (ret) + return ret; + + /* Map registers, assume the PCI bar 0 contain the registers */ + mmio = pcim_iomap(pdev, 0, 0); + if (IS_ERR(mmio)) + return PTR_ERR(mmio); + + ret = etnaviv_gpu_driver_create(dev, mmio, pdev->irq, false, false); + if (ret) + return ret; + + return etnaviv_drm_bind(dev, false); +} + +static void etnaviv_pci_remove(struct pci_dev *pdev) +{ + struct device *dev = &pdev->dev; + + etnaviv_drm_unbind(dev, false); + + etnaviv_gpu_driver_destroy(dev, false); + + pci_clear_master(pdev); +} + +static const struct pci_device_id etnaviv_pci_id_lists[] = { + {PCI_VENDOR_ID_LOONGSON, 0x7a15, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0}, + {PCI_VENDOR_ID_LOONGSON, 0x7a05, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0}, + { } +}; + +static struct pci_driver etnaviv_pci_driver = { + .name = "etnaviv", + .id_table = etnaviv_pci_id_lists, + .probe = etnaviv_pci_probe, + .remove = etnaviv_pci_remove, + .driver.pm = pm_ptr(&etnaviv_gpu_pm_ops), +}; + +int etnaviv_register_pci_driver(void) +{ + return pci_register_driver(&etnaviv_pci_driver); +} + +void etnaviv_unregister_pci_driver(void) +{ + pci_unregister_driver(&etnaviv_pci_driver); +} + +MODULE_DEVICE_TABLE(pci, etnaviv_pci_id_lists); diff --git a/drivers/gpu/drm/etnaviv/etnaviv_pci_drv.h b/drivers/gpu/drm/etnaviv/etnaviv_pci_drv.h new file mode 100644 index 000000000000..25d07bdd2ea0 --- /dev/null +++ b/drivers/gpu/drm/etnaviv/etnaviv_pci_drv.h @@ -0,0 +1,9 @@ +/* SPDX-License-Identifier: GPL-2.0 */ + +#ifndef __ETNAVIV_PCI_DRV_H__ +#define __ETNAVIV_PCI_DRV_H__ + +int etnaviv_register_pci_driver(void); +void etnaviv_unregister_pci_driver(void); + +#endif