Re: [RFC PATCH 1/9] drm/i915/spi: add spi device for discrete graphics

[Jani Nikula <jani.nikula@linux.intel.com>]

On Tue, 16 Feb 2021, Tomas Winkler <tomas.winkler@intel.com> wrote:
> Enable access to internal spi on descrete devices via a child device.
> The spi child device is exposed via MFD framework.
>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Lucas De Marchi <lucas.demarchi@intel.com> # v3
> Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
> ---
>  drivers/gpu/drm/i915/Kconfig         |  1 +
>  drivers/gpu/drm/i915/Makefile        |  3 ++
>  drivers/gpu/drm/i915/i915_drv.c      |  9 +++++
>  drivers/gpu/drm/i915/i915_drv.h      |  4 +++
>  drivers/gpu/drm/i915/i915_reg.h      |  1 +
>  drivers/gpu/drm/i915/spi/intel_spi.c | 53 ++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/spi/intel_spi.h | 19 ++++++++++
>  7 files changed, 90 insertions(+)
>  create mode 100644 drivers/gpu/drm/i915/spi/intel_spi.c
>  create mode 100644 drivers/gpu/drm/i915/spi/intel_spi.h
>
> diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig
> index 1e1cb245fca7..abcaa8da45ac 100644
> --- a/drivers/gpu/drm/i915/Kconfig
> +++ b/drivers/gpu/drm/i915/Kconfig
> @@ -26,6 +26,7 @@ config DRM_I915
>  	select SND_HDA_I915 if SND_HDA_CORE
>  	select CEC_CORE if CEC_NOTIFIER
>  	select VMAP_PFN
> +	select MFD_CORE
>  	help
>  	  Choose this option if you have a system that has "Intel Graphics
>  	  Media Accelerator" or "HD Graphics" integrated graphics,
> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> index 235679637d1c..7fa9120feb8d 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -191,6 +191,9 @@ i915-y += gt/uc/intel_uc.o \
>  	  gt/uc/intel_huc_debugfs.o \
>  	  gt/uc/intel_huc_fw.o
>  
> +# graphics spi device (DGFX) support
> +i915-y += spi/intel_spi.o
> +
>  # modesetting core code
>  i915-y += \
>  	display/intel_atomic.o \
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 3edd5e47ad68..48c383d37212 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -38,6 +38,7 @@
>  #include <linux/slab.h>
>  #include <linux/vga_switcheroo.h>
>  #include <linux/vt.h>
> +#include <linux/mfd/core.h>
>  
>  #include <drm/drm_atomic_helper.h>
>  #include <drm/drm_ioctl.h>
> @@ -67,6 +68,8 @@
>  #include "gt/intel_gt_pm.h"
>  #include "gt/intel_rc6.h"
>  
> +#include "spi/intel_spi.h"
> +
>  #include "i915_debugfs.h"
>  #include "i915_drm_client.h"
>  #include "i915_drv.h"
> @@ -684,6 +687,8 @@ static void i915_driver_register(struct drm_i915_private *dev_priv)
>  
>  	intel_gt_driver_register(&dev_priv->gt);
>  
> +	intel_spi_init(&dev_priv->spi, dev_priv);
> +
>  	intel_display_driver_register(dev_priv);
>  
>  	intel_power_domains_enable(dev_priv);
> @@ -710,6 +715,10 @@ static void i915_driver_unregister(struct drm_i915_private *dev_priv)
>  
>  	intel_display_driver_unregister(dev_priv);
>  
> +	mfd_remove_devices(&dev_priv->drm.pdev->dev);

Please use to_pci_dev(dev_priv->drm.dev).

> +
> +	intel_spi_fini(&dev_priv->spi);
> +
>  	intel_gt_driver_unregister(&dev_priv->gt);
>  
>  	i915_perf_unregister(dev_priv);
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index f8413b3b9da8..f12ec7606d75 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -85,6 +85,8 @@
>  #include "gt/intel_workarounds.h"
>  #include "gt/uc/intel_uc.h"
>  
> +#include "spi/intel_spi.h"
> +
>  #include "intel_device_info.h"
>  #include "intel_pch.h"
>  #include "intel_runtime_pm.h"
> @@ -1117,6 +1119,8 @@ struct drm_i915_private {
>  
>  	struct i915_perf perf;
>  
> +	struct intel_spi spi;
> +
>  	/* Abstract the submission mechanism (legacy ringbuffer or execlists) away */
>  	struct intel_gt gt;
>  
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 224ad897af34..426c5dd63673 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -2510,6 +2510,7 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
>  #define VEBOX_RING_BASE		0x1a000
>  #define GEN11_VEBOX_RING_BASE		0x1c8000
>  #define GEN11_VEBOX2_RING_BASE		0x1d8000
> +#define GEN12_GUNIT_SPI_BASE	0x00102040
>  #define BLT_RING_BASE		0x22000
>  #define RING_TAIL(base)		_MMIO((base) + 0x30)
>  #define RING_HEAD(base)		_MMIO((base) + 0x34)
> diff --git a/drivers/gpu/drm/i915/spi/intel_spi.c b/drivers/gpu/drm/i915/spi/intel_spi.c
> new file mode 100644
> index 000000000000..07da7197bd5d
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/spi/intel_spi.c
> @@ -0,0 +1,53 @@
> +// SPDX-License-Identifier: MIT
> +/*
> + * Copyright(c) 2019-2021, Intel Corporation. All rights reserved.
> + */
> +
> +#include <linux/mfd/core.h>
> +#include <linux/irq.h>
> +#include "i915_reg.h"
> +#include "i915_drv.h"
> +#include "gt/intel_gt.h"
> +#include "spi/intel_spi.h"
> +
> +static const struct resource spi_resources[] = {
> +	DEFINE_RES_MEM_NAMED(GEN12_GUNIT_SPI_BASE, 0x80, "i915-spi-mmio"),
> +};
> +
> +static const struct mfd_cell intel_spi_cell = {
> +	.id = 2,
> +	.name = "i915-spi",
> +	.num_resources = ARRAY_SIZE(spi_resources),
> +	.resources = spi_resources,
> +};
> +
> +void intel_spi_init(struct intel_spi *spi, struct drm_i915_private *dev_priv)
> +{
> +	struct pci_dev *pdev = dev_priv->drm.pdev;

This is going away, please use:

	struct pci_dev *pdev = to_pci_dev(dev_priv->drm.dev);

While at it, please s/dev_priv/i915/g, that's where we're headed
everywhere. It's just a long process, but let's try to not add new
dev_priv users.

> +	int ret;
> +
> +	/* Only the DGFX devices have internal SPI */
> +	if (!IS_DGFX(dev_priv))
> +		return;
> +
> +	ret = mfd_add_devices(&pdev->dev, PLATFORM_DEVID_AUTO,
> +			      &intel_spi_cell, 1,
> +			      &pdev->resource[0], -1, NULL);
> +
> +	if (ret)
> +		dev_err(&pdev->dev, "creating i915-spi cell failed\n");
> +
> +	spi->i915 = dev_priv;
> +}
> +
> +void intel_spi_fini(struct intel_spi *spi)
> +{
> +	struct pci_dev *pdev;
> +
> +	if (!spi->i915)
> +		return;
> +
> +	pdev = spi->i915->drm.pdev;

Ditto here, please use to_pci_dev().

> +
> +	dev_dbg(&pdev->dev, "removing i915-spi cell\n");
> +}
> diff --git a/drivers/gpu/drm/i915/spi/intel_spi.h b/drivers/gpu/drm/i915/spi/intel_spi.h
> new file mode 100644
> index 000000000000..276551fed993
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/spi/intel_spi.h
> @@ -0,0 +1,19 @@
> +/* SPDX-License-Identifier: MIT */
> +/*
> + * Copyright(c) 2019-2021, Intel Corporation. All rights reserved.
> + */
> +
> +#ifndef __INTEL_SPI_DEV_H__
> +#define __INTEL_SPI_DEV_H__

Please make this match the file name.

> +
> +struct drm_i915_private;
> +
> +struct intel_spi {
> +	struct drm_i915_private *i915;
> +};
> +
> +void intel_spi_init(struct intel_spi *spi, struct drm_i915_private *i915);
> +
> +void intel_spi_fini(struct intel_spi *spi);
> +
> +#endif /* __INTEL_SPI_DEV_H__ */

-- 
Jani Nikula, Intel Open Source Graphics Center

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/