Add a basic platform driver of the DRM driver for JH7110 SoC.
Signed-off-by: Keith Zhao <[email protected]>
---
MAINTAINERS | 2 +
drivers/gpu/drm/Kconfig | 2 +
drivers/gpu/drm/Makefile | 1 +
drivers/gpu/drm/verisilicon/Kconfig | 13 ++
drivers/gpu/drm/verisilicon/Makefile | 6 +
drivers/gpu/drm/verisilicon/vs_drv.c | 284 +++++++++++++++++++++++++++
drivers/gpu/drm/verisilicon/vs_drv.h | 48 +++++
include/uapi/drm/drm_fourcc.h | 83 ++++++++
include/uapi/drm/vs_drm.h | 50 +++++
9 files changed, 489 insertions(+)
create mode 100644 drivers/gpu/drm/verisilicon/Kconfig
create mode 100644 drivers/gpu/drm/verisilicon/Makefile
create mode 100644 drivers/gpu/drm/verisilicon/vs_drv.c
create mode 100644 drivers/gpu/drm/verisilicon/vs_drv.h
create mode 100644 include/uapi/drm/vs_drm.h
diff --git a/MAINTAINERS b/MAINTAINERS
index 293aa13d484c..da5b6766a7bb 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7055,6 +7055,8 @@ L: [email protected]
S: Maintained
T: git git://anongit.freedesktop.org/drm/drm-misc
F: Documentation/devicetree/bindings/display/verisilicon/
+F: drivers/gpu/drm/verisilicon/
+F: include/uapi/drm/vs_drm.h
DRM DRIVERS FOR VIVANTE GPU IP
M: Lucas Stach <[email protected]>
diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index ba3fb04bb691..f7e461fa4656 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -371,6 +371,8 @@ source "drivers/gpu/drm/solomon/Kconfig"
source "drivers/gpu/drm/sprd/Kconfig"
+source "drivers/gpu/drm/verisilicon/Kconfig"
+
config DRM_HYPERV
tristate "DRM Support for Hyper-V synthetic video device"
depends on DRM && PCI && MMU && HYPERV
diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index a33257d2bc7f..e50622ee4e46 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -194,3 +194,4 @@ obj-y += gud/
obj-$(CONFIG_DRM_HYPERV) += hyperv/
obj-y += solomon/
obj-$(CONFIG_DRM_SPRD) += sprd/
+obj-$(CONFIG_DRM_VERISILICON) += verisilicon/
diff --git a/drivers/gpu/drm/verisilicon/Kconfig b/drivers/gpu/drm/verisilicon/Kconfig
new file mode 100644
index 000000000000..89d12185f73b
--- /dev/null
+++ b/drivers/gpu/drm/verisilicon/Kconfig
@@ -0,0 +1,13 @@
+# SPDX-License-Identifier: GPL-2.0
+
+config DRM_VERISILICON
+ tristate "DRM Support for VeriSilicon"
+ depends on DRM
+ select DRM_KMS_HELPER
+ select CMA
+ select DMA_CMA
+ help
+ Choose this option if you have a VeriSilicon soc chipset.
+ This driver provides VeriSilicon kernel mode
+ setting and buffer management. It does not
+ provide 2D or 3D acceleration.
diff --git a/drivers/gpu/drm/verisilicon/Makefile b/drivers/gpu/drm/verisilicon/Makefile
new file mode 100644
index 000000000000..64ce1b26546c
--- /dev/null
+++ b/drivers/gpu/drm/verisilicon/Makefile
@@ -0,0 +1,6 @@
+# SPDX-License-Identifier: GPL-2.0
+
+vs_drm-objs := vs_drv.o
+
+obj-$(CONFIG_DRM_VERISILICON) += vs_drm.o
+
diff --git a/drivers/gpu/drm/verisilicon/vs_drv.c b/drivers/gpu/drm/verisilicon/vs_drv.c
new file mode 100644
index 000000000000..24d333598477
--- /dev/null
+++ b/drivers/gpu/drm/verisilicon/vs_drv.c
@@ -0,0 +1,284 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2023 VeriSilicon Holdings Co., Ltd.
+ */
+
+#include <linux/clk.h>
+#include <linux/component.h>
+#include <linux/delay.h>
+#include <linux/dma-mapping.h>
+#include <linux/iommu.h>
+#include <linux/of_graph.h>
+#include <linux/of_reserved_mem.h>
+#include <linux/pm_runtime.h>
+#include <linux/reset.h>
+#include <linux/version.h>
+
+#include <drm/drm_aperture.h>
+#include <drm/drm_crtc.h>
+#include <drm/drm_crtc_helper.h>
+#include <drm/drm_debugfs.h>
+#include <drm/drm_drv.h>
+#include <drm/drm_fb_helper.h>
+#include <drm/drm_fbdev_generic.h>
+#include <drm/drm_file.h>
+#include <drm/drm_fourcc.h>
+#include <drm/drm_ioctl.h>
+#include <drm/drm_of.h>
+#include <drm/drm_prime.h>
+#include <drm/drm_probe_helper.h>
+#include <drm/drm_vblank.h>
+
+#include "vs_drv.h"
+
+#define DRV_NAME "starfive"
+#define DRV_DESC "Starfive DRM driver"
+#define DRV_DATE "202305161"
+#define DRV_MAJOR 1
+#define DRV_MINOR 0
+
+static struct platform_driver vs_drm_platform_driver;
+
+static const struct file_operations fops = {
+ .owner = THIS_MODULE,
+ .open = drm_open,
+ .release = drm_release,
+ .unlocked_ioctl = drm_ioctl,
+ .compat_ioctl = drm_compat_ioctl,
+ .poll = drm_poll,
+ .read = drm_read,
+};
+
+static struct drm_driver vs_drm_driver = {
+ .driver_features = DRIVER_MODESET | DRIVER_ATOMIC | DRIVER_GEM,
+ .lastclose = drm_fb_helper_lastclose,
+ .prime_handle_to_fd = drm_gem_prime_handle_to_fd,
+ .prime_fd_to_handle = drm_gem_prime_fd_to_handle,
+ .fops = &fops,
+ .name = DRV_NAME,
+ .desc = DRV_DESC,
+ .date = DRV_DATE,
+ .major = DRV_MAJOR,
+ .minor = DRV_MINOR,
+};
+
+void vs_drm_update_pitch_alignment(struct drm_device *drm_dev,
+ unsigned int alignment)
+{
+ struct vs_drm_private *priv = drm_dev->dev_private;
+
+ if (alignment > priv->pitch_alignment)
+ priv->pitch_alignment = alignment;
+}
+
+static int vs_drm_bind(struct device *dev)
+{
+ struct drm_device *drm_dev;
+ struct vs_drm_private *priv;
+ int ret;
+ static u64 dma_mask = DMA_BIT_MASK(40);
+
+ /* Remove existing drivers that may own the framebuffer memory. */
+ ret = drm_aperture_remove_framebuffers(false, &vs_drm_driver);
+ if (ret) {
+ DRM_DEV_ERROR(dev,
+ "Failed to remove existing framebuffers - %d.\n",
+ ret);
+ return ret;
+ }
+
+ drm_dev = drm_dev_alloc(&vs_drm_driver, dev);
+ if (IS_ERR(drm_dev))
+ return PTR_ERR(drm_dev);
+
+ dev_set_drvdata(dev, drm_dev);
+
+ priv = devm_kzalloc(drm_dev->dev, sizeof(struct vs_drm_private),
+ GFP_KERNEL);
+ if (!priv) {
+ ret = -ENOMEM;
+ goto err_put_dev;
+ }
+
+ priv->pitch_alignment = 64;
+ priv->dma_dev = drm_dev->dev;
+ priv->dma_dev->coherent_dma_mask = dma_mask;
+ drm_dev->dev_private = priv;
+
+ drm_mode_config_init(drm_dev);
+
+ /* Now try and bind all our sub-components */
+ ret = component_bind_all(dev, drm_dev);
+ if (ret)
+ goto err_mode;
+
+ ret = drm_vblank_init(drm_dev, drm_dev->mode_config.num_crtc);
+ if (ret)
+ goto err_bind;
+
+ drm_mode_config_reset(drm_dev);
+
+ drm_kms_helper_poll_init(drm_dev);
+
+ ret = drm_dev_register(drm_dev, 0);
+ if (ret)
+ goto err_helper;
+
+ drm_fbdev_generic_setup(drm_dev, 32);
+
+ return 0;
+
+err_helper:
+ drm_kms_helper_poll_fini(drm_dev);
+err_bind:
+ component_unbind_all(drm_dev->dev, drm_dev);
+err_mode:
+ drm_mode_config_cleanup(drm_dev);
+ if (priv->domain)
+ iommu_domain_free(priv->domain);
+err_put_dev:
+ drm_dev->dev_private = NULL;
+ dev_set_drvdata(dev, NULL);
+ drm_dev_put(drm_dev);
+ return ret;
+}
+
+static void vs_drm_unbind(struct device *dev)
+{
+ struct drm_device *drm_dev = dev_get_drvdata(dev);
+ struct vs_drm_private *priv = drm_dev->dev_private;
+
+ drm_dev_unregister(drm_dev);
+
+ drm_kms_helper_poll_fini(drm_dev);
+
+ component_unbind_all(drm_dev->dev, drm_dev);
+
+ drm_mode_config_cleanup(drm_dev);
+
+ if (priv->domain) {
+ iommu_domain_free(priv->domain);
+ priv->domain = NULL;
+ }
+
+ drm_dev->dev_private = NULL;
+ dev_set_drvdata(dev, NULL);
+ drm_dev_put(drm_dev);
+}
+
+static const struct component_master_ops vs_drm_ops = {
+ .bind = vs_drm_bind,
+ .unbind = vs_drm_unbind,
+};
+
+static struct platform_driver *drm_sub_drivers[] = {
+};
+
+#define NUM_DRM_DRIVERS \
+ (sizeof(drm_sub_drivers) / sizeof(struct platform_driver *))
+
+static int compare_dev(struct device *dev, void *data)
+{
+ return dev == (struct device *)data;
+}
+
+static struct component_match *vs_drm_match_add(struct device *dev)
+{
+ struct component_match *match = NULL;
+ int i;
+
+ for (i = 0; i < NUM_DRM_DRIVERS; ++i) {
+ struct platform_driver *drv = drm_sub_drivers[i];
+ struct device *p = NULL, *d;
+
+ while ((d = platform_find_device_by_driver(p, &drv->driver))) {
+ put_device(p);
+
+ component_match_add(dev, &match, compare_dev, d);
+ p = d;
+ }
+ put_device(p);
+ }
+
+ return match ?: ERR_PTR(-ENODEV);
+}
+
+static int vs_drm_platform_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct component_match *match;
+
+ match = vs_drm_match_add(dev);
+ if (IS_ERR(match))
+ return PTR_ERR(match);
+
+ return component_master_add_with_match(dev, &vs_drm_ops, match);
+}
+
+static int vs_drm_platform_remove(struct platform_device *pdev)
+{
+ component_master_del(&pdev->dev, &vs_drm_ops);
+ return 0;
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int vs_drm_suspend(struct device *dev)
+{
+ struct drm_device *drm = dev_get_drvdata(dev);
+
+ return drm_mode_config_helper_suspend(drm);
+}
+
+static int vs_drm_resume(struct device *dev)
+{
+ struct drm_device *drm = dev_get_drvdata(dev);
+
+ return drm_mode_config_helper_resume(drm);
+}
+#endif
+
+static SIMPLE_DEV_PM_OPS(vs_drm_pm_ops, vs_drm_suspend, vs_drm_resume);
+
+static const struct of_device_id vs_drm_dt_ids[] = {
+ { .compatible = "verisilicon,display-subsystem", },
+};
+
+MODULE_DEVICE_TABLE(of, vs_drm_dt_ids);
+
+static struct platform_driver vs_drm_platform_driver = {
+ .probe = vs_drm_platform_probe,
+ .remove = vs_drm_platform_remove,
+
+ .driver = {
+ .name = DRV_NAME,
+ .of_match_table = vs_drm_dt_ids,
+ .pm = &vs_drm_pm_ops,
+ },
+};
+
+static int __init vs_drm_init(void)
+{
+ int ret;
+
+ ret = platform_register_drivers(drm_sub_drivers, NUM_DRM_DRIVERS);
+ if (ret)
+ return ret;
+
+ ret = platform_driver_register(&vs_drm_platform_driver);
+ if (ret)
+ platform_unregister_drivers(drm_sub_drivers, NUM_DRM_DRIVERS);
+
+ return ret;
+}
+
+static void __exit vs_drm_fini(void)
+{
+ platform_driver_unregister(&vs_drm_platform_driver);
+ platform_unregister_drivers(drm_sub_drivers, NUM_DRM_DRIVERS);
+}
+
+module_init(vs_drm_init);
+module_exit(vs_drm_fini);
+
+MODULE_DESCRIPTION("VeriSilicon DRM Driver");
+MODULE_LICENSE("GPL");
diff --git a/drivers/gpu/drm/verisilicon/vs_drv.h b/drivers/gpu/drm/verisilicon/vs_drv.h
new file mode 100644
index 000000000000..0382b44e3bf0
--- /dev/null
+++ b/drivers/gpu/drm/verisilicon/vs_drv.h
@@ -0,0 +1,48 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) 2023 VeriSilicon Holdings Co., Ltd.
+ */
+
+#ifndef __VS_DRV_H__
+#define __VS_DRV_H__
+
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/version.h>
+#include <drm/drm_drv.h>
+#include <drm/drm_gem.h>
+
+/*
+ *
+ * @dma_dev: device for DMA API.
+ * - use the first attached device if support iommu
+ else use drm device (only contiguous buffer support)
+ * @domain: iommu domain for DRM.
+ * - all DC IOMMU share same domain to reduce mapping
+ * @pitch_alignment: buffer pitch alignment required by sub-devices.
+ *
+ */
+struct vs_drm_private {
+ struct device *dma_dev;
+ struct iommu_domain *domain;
+ unsigned int pitch_alignment;
+};
+
+void vs_drm_update_pitch_alignment(struct drm_device *drm_dev,
+ unsigned int alignment);
+
+static inline struct device *to_dma_dev(struct drm_device *dev)
+{
+ struct vs_drm_private *priv = dev->dev_private;
+
+ return priv->dma_dev;
+}
+
+static inline bool is_iommu_enabled(struct drm_device *dev)
+{
+ struct vs_drm_private *priv = dev->dev_private;
+
+ return priv->domain ? true : false;
+}
+
+#endif /* __VS_DRV_H__ */
diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
index de703c6be969..af4fb50f9207 100644
--- a/include/uapi/drm/drm_fourcc.h
+++ b/include/uapi/drm/drm_fourcc.h
@@ -419,6 +419,7 @@ extern "C" {
#define DRM_FORMAT_MOD_VENDOR_ARM 0x08
#define DRM_FORMAT_MOD_VENDOR_ALLWINNER 0x09
#define DRM_FORMAT_MOD_VENDOR_AMLOGIC 0x0a
+#define DRM_FORMAT_MOD_VENDOR_VS 0x0b
/* add more to the end as needed */
@@ -1519,6 +1520,88 @@ drm_fourcc_canonicalize_nvidia_format_mod(__u64 modifier)
#define AMD_FMT_MOD_CLEAR(field) \
(~((__u64)AMD_FMT_MOD_##field##_MASK << AMD_FMT_MOD_##field##_SHIFT))
+#define DRM_FORMAT_MOD_VS_TYPE_NORMAL 0x00
+#define DRM_FORMAT_MOD_VS_TYPE_COMPRESSED 0x01
+#define DRM_FORMAT_MOD_VS_TYPE_CUSTOM_10BIT 0x02
+#define DRM_FORMAT_MOD_VS_TYPE_MASK ((__u64)0x3 << 54)
+
+#define fourcc_mod_vs_code(type, val) \
+ fourcc_mod_code(VS, ((((__u64)type) << 54) | (val)))
+
+#define DRM_FORMAT_MOD_VS_DEC_TILE_MODE_MASK 0x3F
+#define DRM_FORMAT_MOD_VS_DEC_TILE_8X8_XMAJOR 0x00
+#define DRM_FORMAT_MOD_VS_DEC_TILE_8X8_YMAJOR 0x01
+#define DRM_FORMAT_MOD_VS_DEC_TILE_16X4 0x02
+#define DRM_FORMAT_MOD_VS_DEC_TILE_8X4 0x03
+#define DRM_FORMAT_MOD_VS_DEC_TILE_4X8 0x04
+#define DRM_FORMAT_MOD_VS_DEC_RASTER_16X4 0x06
+#define DRM_FORMAT_MOD_VS_DEC_TILE_64X4 0x07
+#define DRM_FORMAT_MOD_VS_DEC_TILE_32X4 0x08
+#define DRM_FORMAT_MOD_VS_DEC_RASTER_256X1 0x09
+#define DRM_FORMAT_MOD_VS_DEC_RASTER_128X1 0x0A
+#define DRM_FORMAT_MOD_VS_DEC_RASTER_64X4 0x0B
+#define DRM_FORMAT_MOD_VS_DEC_RASTER_256X2 0x0C
+#define DRM_FORMAT_MOD_VS_DEC_RASTER_128X2 0x0D
+#define DRM_FORMAT_MOD_VS_DEC_RASTER_128X4 0x0E
+#define DRM_FORMAT_MOD_VS_DEC_RASTER_64X1 0x0F
+#define DRM_FORMAT_MOD_VS_DEC_TILE_16X8 0x10
+#define DRM_FORMAT_MOD_VS_DEC_TILE_8X16 0x11
+#define DRM_FORMAT_MOD_VS_DEC_RASTER_512X1 0x12
+#define DRM_FORMAT_MOD_VS_DEC_RASTER_32X4 0x13
+#define DRM_FORMAT_MOD_VS_DEC_RASTER_64X2 0x14
+#define DRM_FORMAT_MOD_VS_DEC_RASTER_32X2 0x15
+#define DRM_FORMAT_MOD_VS_DEC_RASTER_32X1 0x16
+#define DRM_FORMAT_MOD_VS_DEC_RASTER_16X1 0x17
+#define DRM_FORMAT_MOD_VS_DEC_TILE_128X4 0x18
+#define DRM_FORMAT_MOD_VS_DEC_TILE_256X4 0x19
+#define DRM_FORMAT_MOD_VS_DEC_TILE_512X4 0x1A
+#define DRM_FORMAT_MOD_VS_DEC_TILE_16X16 0x1B
+#define DRM_FORMAT_MOD_VS_DEC_TILE_32X16 0x1C
+#define DRM_FORMAT_MOD_VS_DEC_TILE_64X16 0x1D
+#define DRM_FORMAT_MOD_VS_DEC_TILE_128X8 0x1E
+#define DRM_FORMAT_MOD_VS_DEC_TILE_8X4_S 0x1F
+#define DRM_FORMAT_MOD_VS_DEC_TILE_16X4_S 0x20
+#define DRM_FORMAT_MOD_VS_DEC_TILE_32X4_S 0x21
+#define DRM_FORMAT_MOD_VS_DEC_TILE_16X4_LSB 0x22
+#define DRM_FORMAT_MOD_VS_DEC_TILE_32X4_LSB 0x23
+#define DRM_FORMAT_MOD_VS_DEC_TILE_32X8 0x24
+
+#define DRM_FORMAT_MOD_VS_DEC_ALIGN_32 (0x01 << 6)
+#define DRM_FORMAT_MOD_VS_DEC_ALIGN_64 (0x01 << 7)
+
+#define fourcc_mod_vs_dec_code(tile, align) \
+ fourcc_mod_vs_code(DRM_FORMAT_MOD_VS_TYPE_COMPRESSED, \
+ ((tile) | (align)))
+
+#define DRM_FORMAT_MOD_VS_NORM_MODE_MASK 0x1F
+#define DRM_FORMAT_MOD_VS_LINEAR 0x00
+#define DRM_FORMAT_MOD_VS_TILED4x4 0x01
+#define DRM_FORMAT_MOD_VS_SUPER_TILED_XMAJOR 0x02
+#define DRM_FORMAT_MOD_VS_SUPER_TILED_YMAJOR 0x03
+#define DRM_FORMAT_MOD_VS_TILE_8X8 0x04
+#define DRM_FORMAT_MOD_VS_TILE_MODE1 0x05
+#define DRM_FORMAT_MOD_VS_TILE_MODE2 0x06
+#define DRM_FORMAT_MOD_VS_TILE_8X4 0x07
+#define DRM_FORMAT_MOD_VS_TILE_MODE4 0x08
+#define DRM_FORMAT_MOD_VS_TILE_MODE5 0x09
+#define DRM_FORMAT_MOD_VS_TILE_MODE6 0x0A
+#define DRM_FORMAT_MOD_VS_SUPER_TILED_XMAJOR_8X4 0x0B
+#define DRM_FORMAT_MOD_VS_SUPER_TILED_YMAJOR_4X8 0x0C
+#define DRM_FORMAT_MOD_VS_TILE_Y 0x0D
+#define DRM_FORMAT_MOD_VS_TILE_128X1 0x0F
+#define DRM_FORMAT_MOD_VS_TILE_256X1 0x10
+#define DRM_FORMAT_MOD_VS_TILE_32X1 0x11
+#define DRM_FORMAT_MOD_VS_TILE_64X1 0x12
+#define DRM_FORMAT_MOD_VS_TILE_MODE4X4 0x15
+
+#define fourcc_mod_vs_norm_code(tile) \
+ fourcc_mod_vs_code(DRM_FORMAT_MOD_VS_TYPE_NORMAL, \
+ (tile))
+
+#define fourcc_mod_vs_custom_code(tile) \
+ fourcc_mod_vs_code(DRM_FORMAT_MOD_VS_TYPE_CUSTOM_10BIT, \
+ (tile))
+
#if defined(__cplusplus)
}
#endif
diff --git a/include/uapi/drm/vs_drm.h b/include/uapi/drm/vs_drm.h
new file mode 100644
index 000000000000..96b7fc95d658
--- /dev/null
+++ b/include/uapi/drm/vs_drm.h
@@ -0,0 +1,50 @@
+/* SPDX-License-Identifier: GPL-2.0+ WITH Linux-syscall-note */
+/*
+ * Copyright (C) 2020 VeriSilicon Holdings Co., Ltd.
+ */
+
+#ifndef __VS_DRM_H__
+#define __VS_DRM_H__
+
+#include "drm.h"
+
+enum drm_vs_degamma_mode {
+ VS_DEGAMMA_DISABLE = 0,
+ VS_DEGAMMA_BT709 = 1,
+ VS_DEGAMMA_BT2020 = 2,
+};
+
+enum drm_vs_sync_dc_mode {
+ VS_SINGLE_DC = 0,
+ VS_MULTI_DC_PRIMARY = 1,
+ VS_MULTI_DC_SECONDARY = 2,
+};
+
+enum drm_vs_mmu_prefetch_mode {
+ VS_MMU_PREFETCH_DISABLE = 0,
+ VS_MMU_PREFETCH_ENABLE = 1,
+};
+
+struct drm_vs_watermark {
+ __u32 watermark;
+ __u8 qos_low;
+ __u8 qos_high;
+};
+
+struct drm_vs_color_mgmt {
+ __u32 colorkey;
+ __u32 colorkey_high;
+ __u32 clear_value;
+ bool clear_enable;
+ bool transparency;
+};
+
+struct drm_vs_roi {
+ bool enable;
+ __u16 roi_x;
+ __u16 roi_y;
+ __u16 roi_w;
+ __u16 roi_h;
+};
+
+#endif /* __VS_DRM_H__ */
--
2.34.1
Hi Keith,
Am Freitag, dem 02.06.2023 um 15:40 +0800 schrieb Keith Zhao:
> Add a basic platform driver of the DRM driver for JH7110 SoC.
>
> Signed-off-by: Keith Zhao <[email protected]>
> ---
> MAINTAINERS | 2 +
> drivers/gpu/drm/Kconfig | 2 +
> drivers/gpu/drm/Makefile | 1 +
> drivers/gpu/drm/verisilicon/Kconfig | 13 ++
> drivers/gpu/drm/verisilicon/Makefile | 6 +
> drivers/gpu/drm/verisilicon/vs_drv.c | 284 +++++++++++++++++++++++++++
> drivers/gpu/drm/verisilicon/vs_drv.h | 48 +++++
> include/uapi/drm/drm_fourcc.h | 83 ++++++++
> include/uapi/drm/vs_drm.h | 50 +++++
> 9 files changed, 489 insertions(+)
> create mode 100644 drivers/gpu/drm/verisilicon/Kconfig
> create mode 100644 drivers/gpu/drm/verisilicon/Makefile
> create mode 100644 drivers/gpu/drm/verisilicon/vs_drv.c
> create mode 100644 drivers/gpu/drm/verisilicon/vs_drv.h
> create mode 100644 include/uapi/drm/vs_drm.h
>
>
> [...]
> +#endif /* __VS_DRV_H__ */
> diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
> index de703c6be969..af4fb50f9207 100644
> --- a/include/uapi/drm/drm_fourcc.h
> +++ b/include/uapi/drm/drm_fourcc.h
> @@ -419,6 +419,7 @@ extern "C" {
> #define DRM_FORMAT_MOD_VENDOR_ARM 0x08
> #define DRM_FORMAT_MOD_VENDOR_ALLWINNER 0x09
> #define DRM_FORMAT_MOD_VENDOR_AMLOGIC 0x0a
> +#define DRM_FORMAT_MOD_VENDOR_VS 0x0b
>
> /* add more to the end as needed */
>
> @@ -1519,6 +1520,88 @@ drm_fourcc_canonicalize_nvidia_format_mod(__u64 modifier)
> #define AMD_FMT_MOD_CLEAR(field) \
> (~((__u64)AMD_FMT_MOD_##field##_MASK << AMD_FMT_MOD_##field##_SHIFT))
>
> +#define DRM_FORMAT_MOD_VS_TYPE_NORMAL 0x00
> +#define DRM_FORMAT_MOD_VS_TYPE_COMPRESSED 0x01
> +#define DRM_FORMAT_MOD_VS_TYPE_CUSTOM_10BIT 0x02
> +#define DRM_FORMAT_MOD_VS_TYPE_MASK ((__u64)0x3 << 54)
> +
> +#define fourcc_mod_vs_code(type, val) \
> + fourcc_mod_code(VS, ((((__u64)type) << 54) | (val)))
> +
> +#define DRM_FORMAT_MOD_VS_DEC_TILE_MODE_MASK 0x3F
> +#define DRM_FORMAT_MOD_VS_DEC_TILE_8X8_XMAJOR 0x00
> +#define DRM_FORMAT_MOD_VS_DEC_TILE_8X8_YMAJOR 0x01
> +#define DRM_FORMAT_MOD_VS_DEC_TILE_16X4 0x02
> +#define DRM_FORMAT_MOD_VS_DEC_TILE_8X4 0x03
> +#define DRM_FORMAT_MOD_VS_DEC_TILE_4X8 0x04
> +#define DRM_FORMAT_MOD_VS_DEC_RASTER_16X4 0x06
> +#define DRM_FORMAT_MOD_VS_DEC_TILE_64X4 0x07
> +#define DRM_FORMAT_MOD_VS_DEC_TILE_32X4 0x08
> +#define DRM_FORMAT_MOD_VS_DEC_RASTER_256X1 0x09
> +#define DRM_FORMAT_MOD_VS_DEC_RASTER_128X1 0x0A
> +#define DRM_FORMAT_MOD_VS_DEC_RASTER_64X4 0x0B
> +#define DRM_FORMAT_MOD_VS_DEC_RASTER_256X2 0x0C
> +#define DRM_FORMAT_MOD_VS_DEC_RASTER_128X2 0x0D
> +#define DRM_FORMAT_MOD_VS_DEC_RASTER_128X4 0x0E
> +#define DRM_FORMAT_MOD_VS_DEC_RASTER_64X1 0x0F
> +#define DRM_FORMAT_MOD_VS_DEC_TILE_16X8 0x10
> +#define DRM_FORMAT_MOD_VS_DEC_TILE_8X16 0x11
> +#define DRM_FORMAT_MOD_VS_DEC_RASTER_512X1 0x12
> +#define DRM_FORMAT_MOD_VS_DEC_RASTER_32X4 0x13
> +#define DRM_FORMAT_MOD_VS_DEC_RASTER_64X2 0x14
> +#define DRM_FORMAT_MOD_VS_DEC_RASTER_32X2 0x15
> +#define DRM_FORMAT_MOD_VS_DEC_RASTER_32X1 0x16
> +#define DRM_FORMAT_MOD_VS_DEC_RASTER_16X1 0x17
> +#define DRM_FORMAT_MOD_VS_DEC_TILE_128X4 0x18
> +#define DRM_FORMAT_MOD_VS_DEC_TILE_256X4 0x19
> +#define DRM_FORMAT_MOD_VS_DEC_TILE_512X4 0x1A
> +#define DRM_FORMAT_MOD_VS_DEC_TILE_16X16 0x1B
> +#define DRM_FORMAT_MOD_VS_DEC_TILE_32X16 0x1C
> +#define DRM_FORMAT_MOD_VS_DEC_TILE_64X16 0x1D
> +#define DRM_FORMAT_MOD_VS_DEC_TILE_128X8 0x1E
> +#define DRM_FORMAT_MOD_VS_DEC_TILE_8X4_S 0x1F
> +#define DRM_FORMAT_MOD_VS_DEC_TILE_16X4_S 0x20
> +#define DRM_FORMAT_MOD_VS_DEC_TILE_32X4_S 0x21
> +#define DRM_FORMAT_MOD_VS_DEC_TILE_16X4_LSB 0x22
> +#define DRM_FORMAT_MOD_VS_DEC_TILE_32X4_LSB 0x23
> +#define DRM_FORMAT_MOD_VS_DEC_TILE_32X8 0x24
> +
> +#define DRM_FORMAT_MOD_VS_DEC_ALIGN_32 (0x01 << 6)
> +#define DRM_FORMAT_MOD_VS_DEC_ALIGN_64 (0x01 << 7)
> +
> +#define fourcc_mod_vs_dec_code(tile, align) \
> + fourcc_mod_vs_code(DRM_FORMAT_MOD_VS_TYPE_COMPRESSED, \
> + ((tile) | (align)))
> +
> +#define DRM_FORMAT_MOD_VS_NORM_MODE_MASK 0x1F
> +#define DRM_FORMAT_MOD_VS_LINEAR 0x00
> +#define DRM_FORMAT_MOD_VS_TILED4x4 0x01
> +#define DRM_FORMAT_MOD_VS_SUPER_TILED_XMAJOR 0x02
> +#define DRM_FORMAT_MOD_VS_SUPER_TILED_YMAJOR 0x03
> +#define DRM_FORMAT_MOD_VS_TILE_8X8 0x04
> +#define DRM_FORMAT_MOD_VS_TILE_MODE1 0x05
> +#define DRM_FORMAT_MOD_VS_TILE_MODE2 0x06
> +#define DRM_FORMAT_MOD_VS_TILE_8X4 0x07
> +#define DRM_FORMAT_MOD_VS_TILE_MODE4 0x08
> +#define DRM_FORMAT_MOD_VS_TILE_MODE5 0x09
> +#define DRM_FORMAT_MOD_VS_TILE_MODE6 0x0A
> +#define DRM_FORMAT_MOD_VS_SUPER_TILED_XMAJOR_8X4 0x0B
> +#define DRM_FORMAT_MOD_VS_SUPER_TILED_YMAJOR_4X8 0x0C
> +#define DRM_FORMAT_MOD_VS_TILE_Y 0x0D
> +#define DRM_FORMAT_MOD_VS_TILE_128X1 0x0F
> +#define DRM_FORMAT_MOD_VS_TILE_256X1 0x10
> +#define DRM_FORMAT_MOD_VS_TILE_32X1 0x11
> +#define DRM_FORMAT_MOD_VS_TILE_64X1 0x12
> +#define DRM_FORMAT_MOD_VS_TILE_MODE4X4 0x15
> +
> +#define fourcc_mod_vs_norm_code(tile) \
> + fourcc_mod_vs_code(DRM_FORMAT_MOD_VS_TYPE_NORMAL, \
> + (tile))
> +
> +#define fourcc_mod_vs_custom_code(tile) \
> + fourcc_mod_vs_code(DRM_FORMAT_MOD_VS_TYPE_CUSTOM_10BIT, \
> + (tile))
> +
You are opening a new namespace for what is effectively the VIVANTE
tiling. While your list seems much more exhaustive than the (reverse
engineered) list provided under the VIVANTE namespace, this is still
unacceptable as it adds new aliases for existing modifiers.
Also any modifier additions should be in a separate patch and not
buried in another change.
Regards,
Lucas
Hi,
I appreciate that you split the driver into small patches. Please find
some comments below.
Am 02.06.23 um 09:40 schrieb Keith Zhao:
> Add a basic platform driver of the DRM driver for JH7110 SoC.
>
> Signed-off-by: Keith Zhao <[email protected]>
> ---
> MAINTAINERS | 2 +
> drivers/gpu/drm/Kconfig | 2 +
> drivers/gpu/drm/Makefile | 1 +
> drivers/gpu/drm/verisilicon/Kconfig | 13 ++
> drivers/gpu/drm/verisilicon/Makefile | 6 +
> drivers/gpu/drm/verisilicon/vs_drv.c | 284 +++++++++++++++++++++++++++
> drivers/gpu/drm/verisilicon/vs_drv.h | 48 +++++
> include/uapi/drm/drm_fourcc.h | 83 ++++++++
> include/uapi/drm/vs_drm.h | 50 +++++
> 9 files changed, 489 insertions(+)
> create mode 100644 drivers/gpu/drm/verisilicon/Kconfig
> create mode 100644 drivers/gpu/drm/verisilicon/Makefile
> create mode 100644 drivers/gpu/drm/verisilicon/vs_drv.c
> create mode 100644 drivers/gpu/drm/verisilicon/vs_drv.h
> create mode 100644 include/uapi/drm/vs_drm.h
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 293aa13d484c..da5b6766a7bb 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -7055,6 +7055,8 @@ L: [email protected]
> S: Maintained
> T: git git://anongit.freedesktop.org/drm/drm-misc
> F: Documentation/devicetree/bindings/display/verisilicon/
> +F: drivers/gpu/drm/verisilicon/
> +F: include/uapi/drm/vs_drm.h
>
> DRM DRIVERS FOR VIVANTE GPU IP
> M: Lucas Stach <[email protected]>
> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
> index ba3fb04bb691..f7e461fa4656 100644
> --- a/drivers/gpu/drm/Kconfig
> +++ b/drivers/gpu/drm/Kconfig
> @@ -371,6 +371,8 @@ source "drivers/gpu/drm/solomon/Kconfig"
>
> source "drivers/gpu/drm/sprd/Kconfig"
>
> +source "drivers/gpu/drm/verisilicon/Kconfig"
> +
> config DRM_HYPERV
> tristate "DRM Support for Hyper-V synthetic video device"
> depends on DRM && PCI && MMU && HYPERV
> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
> index a33257d2bc7f..e50622ee4e46 100644
> --- a/drivers/gpu/drm/Makefile
> +++ b/drivers/gpu/drm/Makefile
> @@ -194,3 +194,4 @@ obj-y += gud/
> obj-$(CONFIG_DRM_HYPERV) += hyperv/
> obj-y += solomon/
> obj-$(CONFIG_DRM_SPRD) += sprd/
> +obj-$(CONFIG_DRM_VERISILICON) += verisilicon/
> diff --git a/drivers/gpu/drm/verisilicon/Kconfig b/drivers/gpu/drm/verisilicon/Kconfig
> new file mode 100644
> index 000000000000..89d12185f73b
> --- /dev/null
> +++ b/drivers/gpu/drm/verisilicon/Kconfig
> @@ -0,0 +1,13 @@
> +# SPDX-License-Identifier: GPL-2.0
> +
> +config DRM_VERISILICON
> + tristate "DRM Support for VeriSilicon"
Can you rename the driver and files? 'VeriSilicon' seems
unpronounceable. Simply 'StarFive' and starfive/ would be fine.
> + depends on DRM
> + select DRM_KMS_HELPER
> + select CMA
> + select DMA_CMA
> + help
> + Choose this option if you have a VeriSilicon soc chipset.
> + This driver provides VeriSilicon kernel mode
> + setting and buffer management. It does not
> + provide 2D or 3D acceleration.
> diff --git a/drivers/gpu/drm/verisilicon/Makefile b/drivers/gpu/drm/verisilicon/Makefile
> new file mode 100644
> index 000000000000..64ce1b26546c
> --- /dev/null
> +++ b/drivers/gpu/drm/verisilicon/Makefile
> @@ -0,0 +1,6 @@
> +# SPDX-License-Identifier: GPL-2.0
> +
> +vs_drm-objs := vs_drv.o
> +
> +obj-$(CONFIG_DRM_VERISILICON) += vs_drm.o
> +
> diff --git a/drivers/gpu/drm/verisilicon/vs_drv.c b/drivers/gpu/drm/verisilicon/vs_drv.c
> new file mode 100644
> index 000000000000..24d333598477
> --- /dev/null
> +++ b/drivers/gpu/drm/verisilicon/vs_drv.c
> @@ -0,0 +1,284 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2023 VeriSilicon Holdings Co., Ltd.
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/component.h>
> +#include <linux/delay.h>
> +#include <linux/dma-mapping.h>
> +#include <linux/iommu.h>
> +#include <linux/of_graph.h>
> +#include <linux/of_reserved_mem.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/reset.h>
> +#include <linux/version.h>
> +
> +#include <drm/drm_aperture.h>
> +#include <drm/drm_crtc.h>
> +#include <drm/drm_crtc_helper.h>
> +#include <drm/drm_debugfs.h>
> +#include <drm/drm_drv.h>
> +#include <drm/drm_fb_helper.h>
> +#include <drm/drm_fbdev_generic.h>
> +#include <drm/drm_file.h>
> +#include <drm/drm_fourcc.h>
> +#include <drm/drm_ioctl.h>
> +#include <drm/drm_of.h>
> +#include <drm/drm_prime.h>
> +#include <drm/drm_probe_helper.h>
> +#include <drm/drm_vblank.h>
> +
> +#include "vs_drv.h"
> +
> +#define DRV_NAME "starfive"
> +#define DRV_DESC "Starfive DRM driver"
> +#define DRV_DATE "202305161"
> +#define DRV_MAJOR 1
> +#define DRV_MINOR 0
> +
> +static struct platform_driver vs_drm_platform_driver;
> +
> +static const struct file_operations fops = {
> + .owner = THIS_MODULE,
> + .open = drm_open,
> + .release = drm_release,
> + .unlocked_ioctl = drm_ioctl,
> + .compat_ioctl = drm_compat_ioctl,
> + .poll = drm_poll,
> + .read = drm_read,
> +};
> +
> +static struct drm_driver vs_drm_driver = {
> + .driver_features = DRIVER_MODESET | DRIVER_ATOMIC | DRIVER_GEM,
> + .lastclose = drm_fb_helper_lastclose,
> + .prime_handle_to_fd = drm_gem_prime_handle_to_fd,
> + .prime_fd_to_handle = drm_gem_prime_fd_to_handle,
> + .fops = &fops,
> + .name = DRV_NAME,
> + .desc = DRV_DESC,
> + .date = DRV_DATE,
> + .major = DRV_MAJOR,
> + .minor = DRV_MINOR,
> +};
> +
> +void vs_drm_update_pitch_alignment(struct drm_device *drm_dev,
> + unsigned int alignment)
> +{
> + struct vs_drm_private *priv = drm_dev->dev_private;
> +
> + if (alignment > priv->pitch_alignment)
> + priv->pitch_alignment = alignment;
> +}
> +
> +static int vs_drm_bind(struct device *dev)
> +{
> + struct drm_device *drm_dev;
> + struct vs_drm_private *priv;
> + int ret;
> + static u64 dma_mask = DMA_BIT_MASK(40);
> +
> + /* Remove existing drivers that may own the framebuffer memory. */
> + ret = drm_aperture_remove_framebuffers(false, &vs_drm_driver);
> + if (ret) {
> + DRM_DEV_ERROR(dev,
drm_err(), drm_info(), drm_warn(), etc. Here and everwhere else. The
DRM_DEV_*() print macros are obsolete.
> + "Failed to remove existing framebuffers - %d.\n",
> + ret);
> + return ret;
> + }
> +
> + drm_dev = drm_dev_alloc(&vs_drm_driver, dev);
> + if (IS_ERR(drm_dev))
> + return PTR_ERR(drm_dev);
> +
> + dev_set_drvdata(dev, drm_dev);
> +
> + priv = devm_kzalloc(drm_dev->dev, sizeof(struct vs_drm_private),
> + GFP_KERNEL);
> + if (!priv) {
> + ret = -ENOMEM;
> + goto err_put_dev;
> + }
> +
> + priv->pitch_alignment = 64;
> + priv->dma_dev = drm_dev->dev;
> + priv->dma_dev->coherent_dma_mask = dma_mask;
> + drm_dev->dev_private = priv;
dev_private is obsolete and about to go away at some point.
Please embed drm_device in vs_drm_private and allocate the memory with
devm_drm_dev_alloc().
> +
> + drm_mode_config_init(drm_dev);
drmm_mode_config_init() please.
> +
> + /* Now try and bind all our sub-components */
> + ret = component_bind_all(dev, drm_dev);
> + if (ret)
> + goto err_mode;
> +
> + ret = drm_vblank_init(drm_dev, drm_dev->mode_config.num_crtc);
> + if (ret)
> + goto err_bind;
> +
> + drm_mode_config_reset(drm_dev);
> +
> + drm_kms_helper_poll_init(drm_dev);
> +
> + ret = drm_dev_register(drm_dev, 0);
> + if (ret)
> + goto err_helper;
> +
> + drm_fbdev_generic_setup(drm_dev, 32);
> +
> + return 0;
> +
> +err_helper:
> + drm_kms_helper_poll_fini(drm_dev);
> +err_bind:
> + component_unbind_all(drm_dev->dev, drm_dev);
> +err_mode:
> + drm_mode_config_cleanup(drm_dev);
> + if (priv->domain)
> + iommu_domain_free(priv->domain);
> +err_put_dev:
> + drm_dev->dev_private = NULL;
> + dev_set_drvdata(dev, NULL);
> + drm_dev_put(drm_dev);
> + return ret;
> +}
> +
> +static void vs_drm_unbind(struct device *dev)
> +{
> + struct drm_device *drm_dev = dev_get_drvdata(dev);
> + struct vs_drm_private *priv = drm_dev->dev_private;
> +
> + drm_dev_unregister(drm_dev);
> +
> + drm_kms_helper_poll_fini(drm_dev);
> +
> + component_unbind_all(drm_dev->dev, drm_dev);
> +
> + drm_mode_config_cleanup(drm_dev);
> +
> + if (priv->domain) {
> + iommu_domain_free(priv->domain);
> + priv->domain = NULL;
> + }
> +
> + drm_dev->dev_private = NULL;
> + dev_set_drvdata(dev, NULL);
> + drm_dev_put(drm_dev);
You rather want to convert the individual steps of this cleanup to
managed functions (drmm_ and devm_) to automate the cleanup when as part
of drm_dev_put().
> +}
> +
> +static const struct component_master_ops vs_drm_ops = {
> + .bind = vs_drm_bind,
> + .unbind = vs_drm_unbind,
> +};
> +
> +static struct platform_driver *drm_sub_drivers[] = {
> +};
> +
> +#define NUM_DRM_DRIVERS \
> + (sizeof(drm_sub_drivers) / sizeof(struct platform_driver *))
Does this really work? sizeof(drm_sub_drivers) isn't know at compile
time. It is always assumed to be 0 AFAICT. Or do you fill this array in
later patches?
> +
> +static int compare_dev(struct device *dev, void *data)
> +{
> + return dev == (struct device *)data;
> +}
> +
> +static struct component_match *vs_drm_match_add(struct device *dev)
> +{
> + struct component_match *match = NULL;
> + int i;
> +
> + for (i = 0; i < NUM_DRM_DRIVERS; ++i) {
> + struct platform_driver *drv = drm_sub_drivers[i];
> + struct device *p = NULL, *d;
> +
> + while ((d = platform_find_device_by_driver(p, &drv->driver))) {
> + put_device(p);
> +
> + component_match_add(dev, &match, compare_dev, d);
> + p = d;
> + }
> + put_device(p);
> + }
> +
> + return match ?: ERR_PTR(-ENODEV);
> +}
> +
> +static int vs_drm_platform_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct component_match *match;
> +
> + match = vs_drm_match_add(dev);
> + if (IS_ERR(match))
> + return PTR_ERR(match);
> +
> + return component_master_add_with_match(dev, &vs_drm_ops, match);
> +}
> +
> +static int vs_drm_platform_remove(struct platform_device *pdev)
> +{
> + component_master_del(&pdev->dev, &vs_drm_ops);
> + return 0;
> +}
> +
> +#ifdef CONFIG_PM_SLEEP
> +static int vs_drm_suspend(struct device *dev)
> +{
> + struct drm_device *drm = dev_get_drvdata(dev);
> +
> + return drm_mode_config_helper_suspend(drm);
> +}
> +
> +static int vs_drm_resume(struct device *dev)
> +{
> + struct drm_device *drm = dev_get_drvdata(dev);
> +
> + return drm_mode_config_helper_resume(drm);
> +}
> +#endif
> +
> +static SIMPLE_DEV_PM_OPS(vs_drm_pm_ops, vs_drm_suspend, vs_drm_resume);
> +
> +static const struct of_device_id vs_drm_dt_ids[] = {
> + { .compatible = "verisilicon,display-subsystem", },
> +};
> +
> +MODULE_DEVICE_TABLE(of, vs_drm_dt_ids);
> +
> +static struct platform_driver vs_drm_platform_driver = {
> + .probe = vs_drm_platform_probe,
> + .remove = vs_drm_platform_remove,
> +
> + .driver = {
> + .name = DRV_NAME,
> + .of_match_table = vs_drm_dt_ids,
> + .pm = &vs_drm_pm_ops,
> + },
> +};
> +
> +static int __init vs_drm_init(void)
> +{
> + int ret;
> +
> + ret = platform_register_drivers(drm_sub_drivers, NUM_DRM_DRIVERS);
> + if (ret)
> + return ret;
> +
> + ret = platform_driver_register(&vs_drm_platform_driver);
> + if (ret)
> + platform_unregister_drivers(drm_sub_drivers, NUM_DRM_DRIVERS);
> +
> + return ret;
> +}
> +
> +static void __exit vs_drm_fini(void)
> +{
> + platform_driver_unregister(&vs_drm_platform_driver);
> + platform_unregister_drivers(drm_sub_drivers, NUM_DRM_DRIVERS);
> +}
> +
> +module_init(vs_drm_init);
> +module_exit(vs_drm_fini);
> +
> +MODULE_DESCRIPTION("VeriSilicon DRM Driver");
> +MODULE_LICENSE("GPL");
> diff --git a/drivers/gpu/drm/verisilicon/vs_drv.h b/drivers/gpu/drm/verisilicon/vs_drv.h
> new file mode 100644
> index 000000000000..0382b44e3bf0
> --- /dev/null
> +++ b/drivers/gpu/drm/verisilicon/vs_drv.h
> @@ -0,0 +1,48 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (C) 2023 VeriSilicon Holdings Co., Ltd.
> + */
> +
> +#ifndef __VS_DRV_H__
> +#define __VS_DRV_H__
> +
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/version.h>
> +#include <drm/drm_drv.h>
> +#include <drm/drm_gem.h>
> +
> +/*
> + *
> + * @dma_dev: device for DMA API.
> + * - use the first attached device if support iommu
> + else use drm device (only contiguous buffer support)
> + * @domain: iommu domain for DRM.
> + * - all DC IOMMU share same domain to reduce mapping
> + * @pitch_alignment: buffer pitch alignment required by sub-devices.
> + *
> + */
> +struct vs_drm_private {
> + struct device *dma_dev;
> + struct iommu_domain *domain;
> + unsigned int pitch_alignment;
> +};
As mentioned, this struct needs to embed struct drm_device.
> +
> +void vs_drm_update_pitch_alignment(struct drm_device *drm_dev,
> + unsigned int alignment);
> +
> +static inline struct device *to_dma_dev(struct drm_device *dev)
> +{
> + struct vs_drm_private *priv = dev->dev_private;
> +
> + return priv->dma_dev;
And this needs to be an upcast via container_of().
> +}
> +
> +static inline bool is_iommu_enabled(struct drm_device *dev)
> +{
> + struct vs_drm_private *priv = dev->dev_private;
> +
> + return priv->domain ? true : false;
> +}
> +
> +#endif /* __VS_DRV_H__ */
> diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
> index de703c6be969..af4fb50f9207 100644
> --- a/include/uapi/drm/drm_fourcc.h
> +++ b/include/uapi/drm/drm_fourcc.h
The UAPI changes shouldn't be needed in this patch?
> @@ -419,6 +419,7 @@ extern "C" {
> #define DRM_FORMAT_MOD_VENDOR_ARM 0x08
> #define DRM_FORMAT_MOD_VENDOR_ALLWINNER 0x09
> #define DRM_FORMAT_MOD_VENDOR_AMLOGIC 0x0a
> +#define DRM_FORMAT_MOD_VENDOR_VS 0x0b
>
> /* add more to the end as needed */
>
> @@ -1519,6 +1520,88 @@ drm_fourcc_canonicalize_nvidia_format_mod(__u64 modifier)
> #define AMD_FMT_MOD_CLEAR(field) \
> (~((__u64)AMD_FMT_MOD_##field##_MASK << AMD_FMT_MOD_##field##_SHIFT))
>
> +#define DRM_FORMAT_MOD_VS_TYPE_NORMAL 0x00
> +#define DRM_FORMAT_MOD_VS_TYPE_COMPRESSED 0x01
> +#define DRM_FORMAT_MOD_VS_TYPE_CUSTOM_10BIT 0x02
> +#define DRM_FORMAT_MOD_VS_TYPE_MASK ((__u64)0x3 << 54)
> +
> +#define fourcc_mod_vs_code(type, val) \
> + fourcc_mod_code(VS, ((((__u64)type) << 54) | (val)))
> +
> +#define DRM_FORMAT_MOD_VS_DEC_TILE_MODE_MASK 0x3F
> +#define DRM_FORMAT_MOD_VS_DEC_TILE_8X8_XMAJOR 0x00
> +#define DRM_FORMAT_MOD_VS_DEC_TILE_8X8_YMAJOR 0x01
> +#define DRM_FORMAT_MOD_VS_DEC_TILE_16X4 0x02
> +#define DRM_FORMAT_MOD_VS_DEC_TILE_8X4 0x03
> +#define DRM_FORMAT_MOD_VS_DEC_TILE_4X8 0x04
> +#define DRM_FORMAT_MOD_VS_DEC_RASTER_16X4 0x06
> +#define DRM_FORMAT_MOD_VS_DEC_TILE_64X4 0x07
> +#define DRM_FORMAT_MOD_VS_DEC_TILE_32X4 0x08
> +#define DRM_FORMAT_MOD_VS_DEC_RASTER_256X1 0x09
> +#define DRM_FORMAT_MOD_VS_DEC_RASTER_128X1 0x0A
> +#define DRM_FORMAT_MOD_VS_DEC_RASTER_64X4 0x0B
> +#define DRM_FORMAT_MOD_VS_DEC_RASTER_256X2 0x0C
> +#define DRM_FORMAT_MOD_VS_DEC_RASTER_128X2 0x0D
> +#define DRM_FORMAT_MOD_VS_DEC_RASTER_128X4 0x0E
> +#define DRM_FORMAT_MOD_VS_DEC_RASTER_64X1 0x0F
> +#define DRM_FORMAT_MOD_VS_DEC_TILE_16X8 0x10
> +#define DRM_FORMAT_MOD_VS_DEC_TILE_8X16 0x11
> +#define DRM_FORMAT_MOD_VS_DEC_RASTER_512X1 0x12
> +#define DRM_FORMAT_MOD_VS_DEC_RASTER_32X4 0x13
> +#define DRM_FORMAT_MOD_VS_DEC_RASTER_64X2 0x14
> +#define DRM_FORMAT_MOD_VS_DEC_RASTER_32X2 0x15
> +#define DRM_FORMAT_MOD_VS_DEC_RASTER_32X1 0x16
> +#define DRM_FORMAT_MOD_VS_DEC_RASTER_16X1 0x17
> +#define DRM_FORMAT_MOD_VS_DEC_TILE_128X4 0x18
> +#define DRM_FORMAT_MOD_VS_DEC_TILE_256X4 0x19
> +#define DRM_FORMAT_MOD_VS_DEC_TILE_512X4 0x1A
> +#define DRM_FORMAT_MOD_VS_DEC_TILE_16X16 0x1B
> +#define DRM_FORMAT_MOD_VS_DEC_TILE_32X16 0x1C
> +#define DRM_FORMAT_MOD_VS_DEC_TILE_64X16 0x1D
> +#define DRM_FORMAT_MOD_VS_DEC_TILE_128X8 0x1E
> +#define DRM_FORMAT_MOD_VS_DEC_TILE_8X4_S 0x1F
> +#define DRM_FORMAT_MOD_VS_DEC_TILE_16X4_S 0x20
> +#define DRM_FORMAT_MOD_VS_DEC_TILE_32X4_S 0x21
> +#define DRM_FORMAT_MOD_VS_DEC_TILE_16X4_LSB 0x22
> +#define DRM_FORMAT_MOD_VS_DEC_TILE_32X4_LSB 0x23
> +#define DRM_FORMAT_MOD_VS_DEC_TILE_32X8 0x24
> +
> +#define DRM_FORMAT_MOD_VS_DEC_ALIGN_32 (0x01 << 6)
> +#define DRM_FORMAT_MOD_VS_DEC_ALIGN_64 (0x01 << 7)
> +
> +#define fourcc_mod_vs_dec_code(tile, align) \
> + fourcc_mod_vs_code(DRM_FORMAT_MOD_VS_TYPE_COMPRESSED, \
> + ((tile) | (align)))
> +
> +#define DRM_FORMAT_MOD_VS_NORM_MODE_MASK 0x1F
> +#define DRM_FORMAT_MOD_VS_LINEAR 0x00
> +#define DRM_FORMAT_MOD_VS_TILED4x4 0x01
> +#define DRM_FORMAT_MOD_VS_SUPER_TILED_XMAJOR 0x02
> +#define DRM_FORMAT_MOD_VS_SUPER_TILED_YMAJOR 0x03
> +#define DRM_FORMAT_MOD_VS_TILE_8X8 0x04
> +#define DRM_FORMAT_MOD_VS_TILE_MODE1 0x05
> +#define DRM_FORMAT_MOD_VS_TILE_MODE2 0x06
> +#define DRM_FORMAT_MOD_VS_TILE_8X4 0x07
> +#define DRM_FORMAT_MOD_VS_TILE_MODE4 0x08
> +#define DRM_FORMAT_MOD_VS_TILE_MODE5 0x09
> +#define DRM_FORMAT_MOD_VS_TILE_MODE6 0x0A
> +#define DRM_FORMAT_MOD_VS_SUPER_TILED_XMAJOR_8X4 0x0B
> +#define DRM_FORMAT_MOD_VS_SUPER_TILED_YMAJOR_4X8 0x0C
> +#define DRM_FORMAT_MOD_VS_TILE_Y 0x0D
> +#define DRM_FORMAT_MOD_VS_TILE_128X1 0x0F
> +#define DRM_FORMAT_MOD_VS_TILE_256X1 0x10
> +#define DRM_FORMAT_MOD_VS_TILE_32X1 0x11
> +#define DRM_FORMAT_MOD_VS_TILE_64X1 0x12
> +#define DRM_FORMAT_MOD_VS_TILE_MODE4X4 0x15
> +
> +#define fourcc_mod_vs_norm_code(tile) \
> + fourcc_mod_vs_code(DRM_FORMAT_MOD_VS_TYPE_NORMAL, \
> + (tile))
> +
> +#define fourcc_mod_vs_custom_code(tile) \
> + fourcc_mod_vs_code(DRM_FORMAT_MOD_VS_TYPE_CUSTOM_10BIT, \
> + (tile))
> +
> #if defined(__cplusplus)
> }
> #endif
> diff --git a/include/uapi/drm/vs_drm.h b/include/uapi/drm/vs_drm.h
> new file mode 100644
> index 000000000000..96b7fc95d658
> --- /dev/null
> +++ b/include/uapi/drm/vs_drm.h
Another UAPI addition that appears to be unused. Please only add things
that you're using.
Best regards
Thomas
> @@ -0,0 +1,50 @@
> +/* SPDX-License-Identifier: GPL-2.0+ WITH Linux-syscall-note */
> +/*
> + * Copyright (C) 2020 VeriSilicon Holdings Co., Ltd.
> + */
> +
> +#ifndef __VS_DRM_H__
> +#define __VS_DRM_H__
> +
> +#include "drm.h"
> +
> +enum drm_vs_degamma_mode {
> + VS_DEGAMMA_DISABLE = 0,
> + VS_DEGAMMA_BT709 = 1,
> + VS_DEGAMMA_BT2020 = 2,
> +};
> +
> +enum drm_vs_sync_dc_mode {
> + VS_SINGLE_DC = 0,
> + VS_MULTI_DC_PRIMARY = 1,
> + VS_MULTI_DC_SECONDARY = 2,
> +};
> +
> +enum drm_vs_mmu_prefetch_mode {
> + VS_MMU_PREFETCH_DISABLE = 0,
> + VS_MMU_PREFETCH_ENABLE = 1,
> +};
> +
> +struct drm_vs_watermark {
> + __u32 watermark;
> + __u8 qos_low;
> + __u8 qos_high;
> +};
> +
> +struct drm_vs_color_mgmt {
> + __u32 colorkey;
> + __u32 colorkey_high;
> + __u32 clear_value;
> + bool clear_enable;
> + bool transparency;
> +};
> +
> +struct drm_vs_roi {
> + bool enable;
> + __u16 roi_x;
> + __u16 roi_y;
> + __u16 roi_w;
> + __u16 roi_h;
> +};
> +
> +#endif /* __VS_DRM_H__ */
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)
Hello Keith,
While compiling this driver as a module, a error happens:
drivers/gpu/drm/verisilicon/vs_drm: struct of_device_id is 200 bytes.
The last of 1 is:
0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00
0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00
0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00
0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00
0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x76 0x65 0x72 0x69 0x73 0x69
0x6c 0x69 0x63 0x6f 0x6e 0x2c 0x64 0x69 0x73 0x70 0x6c 0x61 0x79 0x2d
0x73 0x75 0x62 0x73 0x79 0x73 0x74 0x65 0x6d 0x00 0x00 0x00 0x00 0x00
0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00
0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00
0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00
0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00
0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00
0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00
0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00
0x00 0x00 0x00 0x00
FATAL: modpost: drivers/gpu/drm/verisilicon/vs_drm: struct of_device_id
is not terminated with a NULL entry!
> +
> +static const struct of_device_id vs_drm_dt_ids[] = {
> + { .compatible = "verisilicon,display-subsystem", },
> +};
> +
So, this should be:
static const struct of_device_id vs_drm_dt_ids[] = {
{ .compatible = "verisilicon,display-subsystem", },
{ },
};
After fixing this problem, another error happens:
ERROR: modpost: module vs_drm uses symbol dma_buf_mmap from namespace
DMA_BUF, but does not import it.
Please fix.
Best regards,
Shengyu
On 2023/7/4 2:42, Shengyu Qu wrote:
> Hello Keith,
>
> While compiling this driver as a module, a error happens:
>
> drivers/gpu/drm/verisilicon/vs_drm: struct of_device_id is 200 bytes. The last of 1 is:
> 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x76 0x65 0x72 0x69 0x73 0x69 0x6c 0x69 0x63 0x6f 0x6e 0x2c 0x64 0x69 0x73 0x70 0x6c 0x61 0x79 0x2d 0x73 0x75 0x62 0x73 0x79 0x73 0x74 0x65 0x6d 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00
> 0x00 0x00
> FATAL: modpost: drivers/gpu/drm/verisilicon/vs_drm: struct of_device_id is not terminated with a NULL entry!
>
>> +
>> +static const struct of_device_id vs_drm_dt_ids[] = {
>> + { .compatible = "verisilicon,display-subsystem", },
>> +};
>> +
>
> So, this should be:
>
> static const struct of_device_id vs_drm_dt_ids[] = {
> { .compatible = "verisilicon,display-subsystem", },
> { },
> };
>
> After fixing this problem, another error happens:
>
> ERROR: modpost: module vs_drm uses symbol dma_buf_mmap from namespace DMA_BUF, but does not import it.
>
> Please fix.
>
> Best regards,
> Shengyu
hello Shengyu ,
I can reproduce the error generated by compiling ko , and add MODULE_IMPORT_NS(DMA_BUF) at the beginning of vs_gem.c ,
it can fix the error message,
I will add this in my next patch , thanks
Hi Thomas,
Le lundi 19 juin 2023 à 14:59 +0200, Thomas Zimmermann a écrit :
> Hi,
>
> I appreciate that you split the driver into small patches. Please find
> some comments below.
>
> Am 02.06.23 um 09:40 schrieb Keith Zhao:
> > Add a basic platform driver of the DRM driver for JH7110 SoC.
> >
> > Signed-off-by: Keith Zhao <[email protected]>
> > ---
> > MAINTAINERS | 2 +
> > drivers/gpu/drm/Kconfig | 2 +
> > drivers/gpu/drm/Makefile | 1 +
> > drivers/gpu/drm/verisilicon/Kconfig | 13 ++
> > drivers/gpu/drm/verisilicon/Makefile | 6 +
> > drivers/gpu/drm/verisilicon/vs_drv.c | 284 +++++++++++++++++++++++++++
> > drivers/gpu/drm/verisilicon/vs_drv.h | 48 +++++
> > include/uapi/drm/drm_fourcc.h | 83 ++++++++
> > include/uapi/drm/vs_drm.h | 50 +++++
> > 9 files changed, 489 insertions(+)
> > create mode 100644 drivers/gpu/drm/verisilicon/Kconfig
> > create mode 100644 drivers/gpu/drm/verisilicon/Makefile
> > create mode 100644 drivers/gpu/drm/verisilicon/vs_drv.c
> > create mode 100644 drivers/gpu/drm/verisilicon/vs_drv.h
> > create mode 100644 include/uapi/drm/vs_drm.h
> >
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 293aa13d484c..da5b6766a7bb 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -7055,6 +7055,8 @@ L: [email protected]
> > S: Maintained
> > T: git git://anongit.freedesktop.org/drm/drm-misc
> > F: Documentation/devicetree/bindings/display/verisilicon/
> > +F: drivers/gpu/drm/verisilicon/
> > +F: include/uapi/drm/vs_drm.h
> >
> > DRM DRIVERS FOR VIVANTE GPU IP
> > M: Lucas Stach <[email protected]>
> > diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
> > index ba3fb04bb691..f7e461fa4656 100644
> > --- a/drivers/gpu/drm/Kconfig
> > +++ b/drivers/gpu/drm/Kconfig
> > @@ -371,6 +371,8 @@ source "drivers/gpu/drm/solomon/Kconfig"
> >
> > source "drivers/gpu/drm/sprd/Kconfig"
> >
> > +source "drivers/gpu/drm/verisilicon/Kconfig"
> > +
> > config DRM_HYPERV
> > tristate "DRM Support for Hyper-V synthetic video device"
> > depends on DRM && PCI && MMU && HYPERV
> > diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
> > index a33257d2bc7f..e50622ee4e46 100644
> > --- a/drivers/gpu/drm/Makefile
> > +++ b/drivers/gpu/drm/Makefile
> > @@ -194,3 +194,4 @@ obj-y += gud/
> > obj-$(CONFIG_DRM_HYPERV) += hyperv/
> > obj-y += solomon/
> > obj-$(CONFIG_DRM_SPRD) += sprd/
> > +obj-$(CONFIG_DRM_VERISILICON) += verisilicon/
> > diff --git a/drivers/gpu/drm/verisilicon/Kconfig b/drivers/gpu/drm/verisilicon/Kconfig
> > new file mode 100644
> > index 000000000000..89d12185f73b
> > --- /dev/null
> > +++ b/drivers/gpu/drm/verisilicon/Kconfig
> > @@ -0,0 +1,13 @@
> > +# SPDX-License-Identifier: GPL-2.0
> > +
> > +config DRM_VERISILICON
> > + tristate "DRM Support for VeriSilicon"
>
> Can you rename the driver and files? 'VeriSilicon' seems
> unpronounceable. Simply 'StarFive' and starfive/ would be fine.
Are you sure you want to request this ? If the display controller is a
Verisilicon design, it will be super odd to use on other SoC that aren't from
StarFive. Think about STM network driver, which is DesignWare.
Nicolas
>
> > + depends on DRM
> > + select DRM_KMS_HELPER
> > + select CMA
> > + select DMA_CMA
> > + help
> > + Choose this option if you have a VeriSilicon soc chipset.
> > + This driver provides VeriSilicon kernel mode
> > + setting and buffer management. It does not
> > + provide 2D or 3D acceleration.
> > diff --git a/drivers/gpu/drm/verisilicon/Makefile b/drivers/gpu/drm/verisilicon/Makefile
> > new file mode 100644
> > index 000000000000..64ce1b26546c
> > --- /dev/null
> > +++ b/drivers/gpu/drm/verisilicon/Makefile
> > @@ -0,0 +1,6 @@
> > +# SPDX-License-Identifier: GPL-2.0
> > +
> > +vs_drm-objs := vs_drv.o
> > +
> > +obj-$(CONFIG_DRM_VERISILICON) += vs_drm.o
> > +
> > diff --git a/drivers/gpu/drm/verisilicon/vs_drv.c b/drivers/gpu/drm/verisilicon/vs_drv.c
> > new file mode 100644
> > index 000000000000..24d333598477
> > --- /dev/null
> > +++ b/drivers/gpu/drm/verisilicon/vs_drv.c
> > @@ -0,0 +1,284 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright (C) 2023 VeriSilicon Holdings Co., Ltd.
> > + */
> > +
> > +#include <linux/clk.h>
> > +#include <linux/component.h>
> > +#include <linux/delay.h>
> > +#include <linux/dma-mapping.h>
> > +#include <linux/iommu.h>
> > +#include <linux/of_graph.h>
> > +#include <linux/of_reserved_mem.h>
> > +#include <linux/pm_runtime.h>
> > +#include <linux/reset.h>
> > +#include <linux/version.h>
> > +
> > +#include <drm/drm_aperture.h>
> > +#include <drm/drm_crtc.h>
> > +#include <drm/drm_crtc_helper.h>
> > +#include <drm/drm_debugfs.h>
> > +#include <drm/drm_drv.h>
> > +#include <drm/drm_fb_helper.h>
> > +#include <drm/drm_fbdev_generic.h>
> > +#include <drm/drm_file.h>
> > +#include <drm/drm_fourcc.h>
> > +#include <drm/drm_ioctl.h>
> > +#include <drm/drm_of.h>
> > +#include <drm/drm_prime.h>
> > +#include <drm/drm_probe_helper.h>
> > +#include <drm/drm_vblank.h>
> > +
> > +#include "vs_drv.h"
> > +
> > +#define DRV_NAME "starfive"
> > +#define DRV_DESC "Starfive DRM driver"
> > +#define DRV_DATE "202305161"
> > +#define DRV_MAJOR 1
> > +#define DRV_MINOR 0
> > +
> > +static struct platform_driver vs_drm_platform_driver;
> > +
> > +static const struct file_operations fops = {
> > + .owner = THIS_MODULE,
> > + .open = drm_open,
> > + .release = drm_release,
> > + .unlocked_ioctl = drm_ioctl,
> > + .compat_ioctl = drm_compat_ioctl,
> > + .poll = drm_poll,
> > + .read = drm_read,
> > +};
> > +
> > +static struct drm_driver vs_drm_driver = {
> > + .driver_features = DRIVER_MODESET | DRIVER_ATOMIC | DRIVER_GEM,
> > + .lastclose = drm_fb_helper_lastclose,
> > + .prime_handle_to_fd = drm_gem_prime_handle_to_fd,
> > + .prime_fd_to_handle = drm_gem_prime_fd_to_handle,
> > + .fops = &fops,
> > + .name = DRV_NAME,
> > + .desc = DRV_DESC,
> > + .date = DRV_DATE,
> > + .major = DRV_MAJOR,
> > + .minor = DRV_MINOR,
> > +};
> > +
> > +void vs_drm_update_pitch_alignment(struct drm_device *drm_dev,
> > + unsigned int alignment)
> > +{
> > + struct vs_drm_private *priv = drm_dev->dev_private;
> > +
> > + if (alignment > priv->pitch_alignment)
> > + priv->pitch_alignment = alignment;
> > +}
> > +
> > +static int vs_drm_bind(struct device *dev)
> > +{
> > + struct drm_device *drm_dev;
> > + struct vs_drm_private *priv;
> > + int ret;
> > + static u64 dma_mask = DMA_BIT_MASK(40);
> > +
> > + /* Remove existing drivers that may own the framebuffer memory. */
> > + ret = drm_aperture_remove_framebuffers(false, &vs_drm_driver);
> > + if (ret) {
> > + DRM_DEV_ERROR(dev,
>
> drm_err(), drm_info(), drm_warn(), etc. Here and everwhere else. The
> DRM_DEV_*() print macros are obsolete.
>
> > + "Failed to remove existing framebuffers - %d.\n",
> > + ret);
> > + return ret;
> > + }
> > +
> > + drm_dev = drm_dev_alloc(&vs_drm_driver, dev);
> > + if (IS_ERR(drm_dev))
> > + return PTR_ERR(drm_dev);
> > +
> > + dev_set_drvdata(dev, drm_dev);
> > +
> > + priv = devm_kzalloc(drm_dev->dev, sizeof(struct vs_drm_private),
> > + GFP_KERNEL);
> > + if (!priv) {
> > + ret = -ENOMEM;
> > + goto err_put_dev;
> > + }
> > +
> > + priv->pitch_alignment = 64;
> > + priv->dma_dev = drm_dev->dev;
> > + priv->dma_dev->coherent_dma_mask = dma_mask;
> > + drm_dev->dev_private = priv;
>
> dev_private is obsolete and about to go away at some point.
>
> Please embed drm_device in vs_drm_private and allocate the memory with
> devm_drm_dev_alloc().
>
> > +
> > + drm_mode_config_init(drm_dev);
>
> drmm_mode_config_init() please.
>
> > +
> > + /* Now try and bind all our sub-components */
> > + ret = component_bind_all(dev, drm_dev);
> > + if (ret)
> > + goto err_mode;
> > +
> > + ret = drm_vblank_init(drm_dev, drm_dev->mode_config.num_crtc);
> > + if (ret)
> > + goto err_bind;
> > +
> > + drm_mode_config_reset(drm_dev);
> > +
> > + drm_kms_helper_poll_init(drm_dev);
> > +
> > + ret = drm_dev_register(drm_dev, 0);
> > + if (ret)
> > + goto err_helper;
> > +
> > + drm_fbdev_generic_setup(drm_dev, 32);
> > +
> > + return 0;
> > +
> > +err_helper:
> > + drm_kms_helper_poll_fini(drm_dev);
> > +err_bind:
> > + component_unbind_all(drm_dev->dev, drm_dev);
> > +err_mode:
> > + drm_mode_config_cleanup(drm_dev);
> > + if (priv->domain)
> > + iommu_domain_free(priv->domain);
> > +err_put_dev:
> > + drm_dev->dev_private = NULL;
> > + dev_set_drvdata(dev, NULL);
> > + drm_dev_put(drm_dev);
> > + return ret;
> > +}
> > +
> > +static void vs_drm_unbind(struct device *dev)
> > +{
> > + struct drm_device *drm_dev = dev_get_drvdata(dev);
> > + struct vs_drm_private *priv = drm_dev->dev_private;
> > +
> > + drm_dev_unregister(drm_dev);
> > +
> > + drm_kms_helper_poll_fini(drm_dev);
> > +
> > + component_unbind_all(drm_dev->dev, drm_dev);
> > +
> > + drm_mode_config_cleanup(drm_dev);
> > +
> > + if (priv->domain) {
> > + iommu_domain_free(priv->domain);
> > + priv->domain = NULL;
> > + }
> > +
> > + drm_dev->dev_private = NULL;
> > + dev_set_drvdata(dev, NULL);
> > + drm_dev_put(drm_dev);
>
> You rather want to convert the individual steps of this cleanup to
> managed functions (drmm_ and devm_) to automate the cleanup when as part
> of drm_dev_put().
>
> > +}
> > +
> > +static const struct component_master_ops vs_drm_ops = {
> > + .bind = vs_drm_bind,
> > + .unbind = vs_drm_unbind,
> > +};
> > +
> > +static struct platform_driver *drm_sub_drivers[] = {
> > +};
> > +
> > +#define NUM_DRM_DRIVERS \
> > + (sizeof(drm_sub_drivers) / sizeof(struct platform_driver *))
>
> Does this really work? sizeof(drm_sub_drivers) isn't know at compile
> time. It is always assumed to be 0 AFAICT. Or do you fill this array in
> later patches?
>
>
> > +
> > +static int compare_dev(struct device *dev, void *data)
> > +{
> > + return dev == (struct device *)data;
> > +}
> > +
> > +static struct component_match *vs_drm_match_add(struct device *dev)
> > +{
> > + struct component_match *match = NULL;
> > + int i;
> > +
> > + for (i = 0; i < NUM_DRM_DRIVERS; ++i) {
> > + struct platform_driver *drv = drm_sub_drivers[i];
> > + struct device *p = NULL, *d;
> > +
> > + while ((d = platform_find_device_by_driver(p, &drv->driver))) {
> > + put_device(p);
> > +
> > + component_match_add(dev, &match, compare_dev, d);
> > + p = d;
> > + }
> > + put_device(p);
> > + }
> > +
> > + return match ?: ERR_PTR(-ENODEV);
> > +}
> > +
> > +static int vs_drm_platform_probe(struct platform_device *pdev)
> > +{
> > + struct device *dev = &pdev->dev;
> > + struct component_match *match;
> > +
> > + match = vs_drm_match_add(dev);
> > + if (IS_ERR(match))
> > + return PTR_ERR(match);
> > +
> > + return component_master_add_with_match(dev, &vs_drm_ops, match);
> > +}
> > +
> > +static int vs_drm_platform_remove(struct platform_device *pdev)
> > +{
> > + component_master_del(&pdev->dev, &vs_drm_ops);
> > + return 0;
> > +}
> > +
> > +#ifdef CONFIG_PM_SLEEP
> > +static int vs_drm_suspend(struct device *dev)
> > +{
> > + struct drm_device *drm = dev_get_drvdata(dev);
> > +
> > + return drm_mode_config_helper_suspend(drm);
> > +}
> > +
> > +static int vs_drm_resume(struct device *dev)
> > +{
> > + struct drm_device *drm = dev_get_drvdata(dev);
> > +
> > + return drm_mode_config_helper_resume(drm);
> > +}
> > +#endif
> > +
> > +static SIMPLE_DEV_PM_OPS(vs_drm_pm_ops, vs_drm_suspend, vs_drm_resume);
> > +
> > +static const struct of_device_id vs_drm_dt_ids[] = {
> > + { .compatible = "verisilicon,display-subsystem", },
> > +};
> > +
> > +MODULE_DEVICE_TABLE(of, vs_drm_dt_ids);
> > +
> > +static struct platform_driver vs_drm_platform_driver = {
> > + .probe = vs_drm_platform_probe,
> > + .remove = vs_drm_platform_remove,
> > +
> > + .driver = {
> > + .name = DRV_NAME,
> > + .of_match_table = vs_drm_dt_ids,
> > + .pm = &vs_drm_pm_ops,
> > + },
> > +};
> > +
> > +static int __init vs_drm_init(void)
> > +{
> > + int ret;
> > +
> > + ret = platform_register_drivers(drm_sub_drivers, NUM_DRM_DRIVERS);
> > + if (ret)
> > + return ret;
> > +
> > + ret = platform_driver_register(&vs_drm_platform_driver);
> > + if (ret)
> > + platform_unregister_drivers(drm_sub_drivers, NUM_DRM_DRIVERS);
> > +
> > + return ret;
> > +}
> > +
> > +static void __exit vs_drm_fini(void)
> > +{
> > + platform_driver_unregister(&vs_drm_platform_driver);
> > + platform_unregister_drivers(drm_sub_drivers, NUM_DRM_DRIVERS);
> > +}
> > +
> > +module_init(vs_drm_init);
> > +module_exit(vs_drm_fini);
> > +
> > +MODULE_DESCRIPTION("VeriSilicon DRM Driver");
> > +MODULE_LICENSE("GPL");
> > diff --git a/drivers/gpu/drm/verisilicon/vs_drv.h b/drivers/gpu/drm/verisilicon/vs_drv.h
> > new file mode 100644
> > index 000000000000..0382b44e3bf0
> > --- /dev/null
> > +++ b/drivers/gpu/drm/verisilicon/vs_drv.h
> > @@ -0,0 +1,48 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +/*
> > + * Copyright (C) 2023 VeriSilicon Holdings Co., Ltd.
> > + */
> > +
> > +#ifndef __VS_DRV_H__
> > +#define __VS_DRV_H__
> > +
> > +#include <linux/module.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/version.h>
> > +#include <drm/drm_drv.h>
> > +#include <drm/drm_gem.h>
> > +
> > +/*
> > + *
> > + * @dma_dev: device for DMA API.
> > + * - use the first attached device if support iommu
> > + else use drm device (only contiguous buffer support)
> > + * @domain: iommu domain for DRM.
> > + * - all DC IOMMU share same domain to reduce mapping
> > + * @pitch_alignment: buffer pitch alignment required by sub-devices.
> > + *
> > + */
> > +struct vs_drm_private {
> > + struct device *dma_dev;
> > + struct iommu_domain *domain;
> > + unsigned int pitch_alignment;
> > +};
>
> As mentioned, this struct needs to embed struct drm_device.
>
> > +
> > +void vs_drm_update_pitch_alignment(struct drm_device *drm_dev,
> > + unsigned int alignment);
> > +
> > +static inline struct device *to_dma_dev(struct drm_device *dev)
> > +{
> > + struct vs_drm_private *priv = dev->dev_private;
> > +
> > + return priv->dma_dev;
>
> And this needs to be an upcast via container_of().
>
> > +}
> > +
> > +static inline bool is_iommu_enabled(struct drm_device *dev)
> > +{
> > + struct vs_drm_private *priv = dev->dev_private;
> > +
> > + return priv->domain ? true : false;
> > +}
> > +
> > +#endif /* __VS_DRV_H__ */
> > diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
> > index de703c6be969..af4fb50f9207 100644
> > --- a/include/uapi/drm/drm_fourcc.h
> > +++ b/include/uapi/drm/drm_fourcc.h
>
> The UAPI changes shouldn't be needed in this patch?
>
> > @@ -419,6 +419,7 @@ extern "C" {
> > #define DRM_FORMAT_MOD_VENDOR_ARM 0x08
> > #define DRM_FORMAT_MOD_VENDOR_ALLWINNER 0x09
> > #define DRM_FORMAT_MOD_VENDOR_AMLOGIC 0x0a
> > +#define DRM_FORMAT_MOD_VENDOR_VS 0x0b
> >
> > /* add more to the end as needed */
> >
> > @@ -1519,6 +1520,88 @@ drm_fourcc_canonicalize_nvidia_format_mod(__u64 modifier)
> > #define AMD_FMT_MOD_CLEAR(field) \
> > (~((__u64)AMD_FMT_MOD_##field##_MASK << AMD_FMT_MOD_##field##_SHIFT))
> >
> > +#define DRM_FORMAT_MOD_VS_TYPE_NORMAL 0x00
> > +#define DRM_FORMAT_MOD_VS_TYPE_COMPRESSED 0x01
> > +#define DRM_FORMAT_MOD_VS_TYPE_CUSTOM_10BIT 0x02
> > +#define DRM_FORMAT_MOD_VS_TYPE_MASK ((__u64)0x3 << 54)
> > +
> > +#define fourcc_mod_vs_code(type, val) \
> > + fourcc_mod_code(VS, ((((__u64)type) << 54) | (val)))
> > +
> > +#define DRM_FORMAT_MOD_VS_DEC_TILE_MODE_MASK 0x3F
> > +#define DRM_FORMAT_MOD_VS_DEC_TILE_8X8_XMAJOR 0x00
> > +#define DRM_FORMAT_MOD_VS_DEC_TILE_8X8_YMAJOR 0x01
> > +#define DRM_FORMAT_MOD_VS_DEC_TILE_16X4 0x02
> > +#define DRM_FORMAT_MOD_VS_DEC_TILE_8X4 0x03
> > +#define DRM_FORMAT_MOD_VS_DEC_TILE_4X8 0x04
> > +#define DRM_FORMAT_MOD_VS_DEC_RASTER_16X4 0x06
> > +#define DRM_FORMAT_MOD_VS_DEC_TILE_64X4 0x07
> > +#define DRM_FORMAT_MOD_VS_DEC_TILE_32X4 0x08
> > +#define DRM_FORMAT_MOD_VS_DEC_RASTER_256X1 0x09
> > +#define DRM_FORMAT_MOD_VS_DEC_RASTER_128X1 0x0A
> > +#define DRM_FORMAT_MOD_VS_DEC_RASTER_64X4 0x0B
> > +#define DRM_FORMAT_MOD_VS_DEC_RASTER_256X2 0x0C
> > +#define DRM_FORMAT_MOD_VS_DEC_RASTER_128X2 0x0D
> > +#define DRM_FORMAT_MOD_VS_DEC_RASTER_128X4 0x0E
> > +#define DRM_FORMAT_MOD_VS_DEC_RASTER_64X1 0x0F
> > +#define DRM_FORMAT_MOD_VS_DEC_TILE_16X8 0x10
> > +#define DRM_FORMAT_MOD_VS_DEC_TILE_8X16 0x11
> > +#define DRM_FORMAT_MOD_VS_DEC_RASTER_512X1 0x12
> > +#define DRM_FORMAT_MOD_VS_DEC_RASTER_32X4 0x13
> > +#define DRM_FORMAT_MOD_VS_DEC_RASTER_64X2 0x14
> > +#define DRM_FORMAT_MOD_VS_DEC_RASTER_32X2 0x15
> > +#define DRM_FORMAT_MOD_VS_DEC_RASTER_32X1 0x16
> > +#define DRM_FORMAT_MOD_VS_DEC_RASTER_16X1 0x17
> > +#define DRM_FORMAT_MOD_VS_DEC_TILE_128X4 0x18
> > +#define DRM_FORMAT_MOD_VS_DEC_TILE_256X4 0x19
> > +#define DRM_FORMAT_MOD_VS_DEC_TILE_512X4 0x1A
> > +#define DRM_FORMAT_MOD_VS_DEC_TILE_16X16 0x1B
> > +#define DRM_FORMAT_MOD_VS_DEC_TILE_32X16 0x1C
> > +#define DRM_FORMAT_MOD_VS_DEC_TILE_64X16 0x1D
> > +#define DRM_FORMAT_MOD_VS_DEC_TILE_128X8 0x1E
> > +#define DRM_FORMAT_MOD_VS_DEC_TILE_8X4_S 0x1F
> > +#define DRM_FORMAT_MOD_VS_DEC_TILE_16X4_S 0x20
> > +#define DRM_FORMAT_MOD_VS_DEC_TILE_32X4_S 0x21
> > +#define DRM_FORMAT_MOD_VS_DEC_TILE_16X4_LSB 0x22
> > +#define DRM_FORMAT_MOD_VS_DEC_TILE_32X4_LSB 0x23
> > +#define DRM_FORMAT_MOD_VS_DEC_TILE_32X8 0x24
> > +
> > +#define DRM_FORMAT_MOD_VS_DEC_ALIGN_32 (0x01 << 6)
> > +#define DRM_FORMAT_MOD_VS_DEC_ALIGN_64 (0x01 << 7)
> > +
> > +#define fourcc_mod_vs_dec_code(tile, align) \
> > + fourcc_mod_vs_code(DRM_FORMAT_MOD_VS_TYPE_COMPRESSED, \
> > + ((tile) | (align)))
> > +
> > +#define DRM_FORMAT_MOD_VS_NORM_MODE_MASK 0x1F
> > +#define DRM_FORMAT_MOD_VS_LINEAR 0x00
> > +#define DRM_FORMAT_MOD_VS_TILED4x4 0x01
> > +#define DRM_FORMAT_MOD_VS_SUPER_TILED_XMAJOR 0x02
> > +#define DRM_FORMAT_MOD_VS_SUPER_TILED_YMAJOR 0x03
> > +#define DRM_FORMAT_MOD_VS_TILE_8X8 0x04
> > +#define DRM_FORMAT_MOD_VS_TILE_MODE1 0x05
> > +#define DRM_FORMAT_MOD_VS_TILE_MODE2 0x06
> > +#define DRM_FORMAT_MOD_VS_TILE_8X4 0x07
> > +#define DRM_FORMAT_MOD_VS_TILE_MODE4 0x08
> > +#define DRM_FORMAT_MOD_VS_TILE_MODE5 0x09
> > +#define DRM_FORMAT_MOD_VS_TILE_MODE6 0x0A
> > +#define DRM_FORMAT_MOD_VS_SUPER_TILED_XMAJOR_8X4 0x0B
> > +#define DRM_FORMAT_MOD_VS_SUPER_TILED_YMAJOR_4X8 0x0C
> > +#define DRM_FORMAT_MOD_VS_TILE_Y 0x0D
> > +#define DRM_FORMAT_MOD_VS_TILE_128X1 0x0F
> > +#define DRM_FORMAT_MOD_VS_TILE_256X1 0x10
> > +#define DRM_FORMAT_MOD_VS_TILE_32X1 0x11
> > +#define DRM_FORMAT_MOD_VS_TILE_64X1 0x12
> > +#define DRM_FORMAT_MOD_VS_TILE_MODE4X4 0x15
> > +
> > +#define fourcc_mod_vs_norm_code(tile) \
> > + fourcc_mod_vs_code(DRM_FORMAT_MOD_VS_TYPE_NORMAL, \
> > + (tile))
> > +
> > +#define fourcc_mod_vs_custom_code(tile) \
> > + fourcc_mod_vs_code(DRM_FORMAT_MOD_VS_TYPE_CUSTOM_10BIT, \
> > + (tile))
> > +
> > #if defined(__cplusplus)
> > }
> > #endif
> > diff --git a/include/uapi/drm/vs_drm.h b/include/uapi/drm/vs_drm.h
> > new file mode 100644
> > index 000000000000..96b7fc95d658
> > --- /dev/null
> > +++ b/include/uapi/drm/vs_drm.h
>
> Another UAPI addition that appears to be unused. Please only add things
> that you're using.
>
> Best regards
> Thomas
>
> > @@ -0,0 +1,50 @@
> > +/* SPDX-License-Identifier: GPL-2.0+ WITH Linux-syscall-note */
> > +/*
> > + * Copyright (C) 2020 VeriSilicon Holdings Co., Ltd.
> > + */
> > +
> > +#ifndef __VS_DRM_H__
> > +#define __VS_DRM_H__
> > +
> > +#include "drm.h"
> > +
> > +enum drm_vs_degamma_mode {
> > + VS_DEGAMMA_DISABLE = 0,
> > + VS_DEGAMMA_BT709 = 1,
> > + VS_DEGAMMA_BT2020 = 2,
> > +};
> > +
> > +enum drm_vs_sync_dc_mode {
> > + VS_SINGLE_DC = 0,
> > + VS_MULTI_DC_PRIMARY = 1,
> > + VS_MULTI_DC_SECONDARY = 2,
> > +};
> > +
> > +enum drm_vs_mmu_prefetch_mode {
> > + VS_MMU_PREFETCH_DISABLE = 0,
> > + VS_MMU_PREFETCH_ENABLE = 1,
> > +};
> > +
> > +struct drm_vs_watermark {
> > + __u32 watermark;
> > + __u8 qos_low;
> > + __u8 qos_high;
> > +};
> > +
> > +struct drm_vs_color_mgmt {
> > + __u32 colorkey;
> > + __u32 colorkey_high;
> > + __u32 clear_value;
> > + bool clear_enable;
> > + bool transparency;
> > +};
> > +
> > +struct drm_vs_roi {
> > + bool enable;
> > + __u16 roi_x;
> > + __u16 roi_y;
> > + __u16 roi_w;
> > + __u16 roi_h;
> > +};
> > +
> > +#endif /* __VS_DRM_H__ */
>
> --
> Thomas Zimmermann
> Graphics Driver Developer
> SUSE Software Solutions Germany GmbH
> Frankenstrasse 146, 90461 Nuernberg, Germany
> GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
> HRB 36809 (AG Nuernberg)
Hi
Am 07.07.23 um 20:09 schrieb Nicolas Dufresne:
[...]
>>> +config DRM_VERISILICON
>>> + tristate "DRM Support for VeriSilicon"
>>
>> Can you rename the driver and files? 'VeriSilicon' seems
>> unpronounceable. Simply 'StarFive' and starfive/ would be fine.
>
> Are you sure you want to request this ? If the display controller is a
> Verisilicon design, it will be super odd to use on other SoC that aren't from
> StarFive. Think about STM network driver, which is DesignWare.
It's not a hard requirement. If that's the name, so be it.
Best regards
Thomas
>
> Nicolas
>
>>
>>> + depends on DRM
>>> + select DRM_KMS_HELPER
>>> + select CMA
>>> + select DMA_CMA
>>> + help
>>> + Choose this option if you have a VeriSilicon soc chipset.
>>> + This driver provides VeriSilicon kernel mode
>>> + setting and buffer management. It does not
>>> + provide 2D or 3D acceleration.
>>> diff --git a/drivers/gpu/drm/verisilicon/Makefile b/drivers/gpu/drm/verisilicon/Makefile
>>> new file mode 100644
>>> index 000000000000..64ce1b26546c
>>> --- /dev/null
>>> +++ b/drivers/gpu/drm/verisilicon/Makefile
>>> @@ -0,0 +1,6 @@
>>> +# SPDX-License-Identifier: GPL-2.0
>>> +
>>> +vs_drm-objs := vs_drv.o
>>> +
>>> +obj-$(CONFIG_DRM_VERISILICON) += vs_drm.o
>>> +
>>> diff --git a/drivers/gpu/drm/verisilicon/vs_drv.c b/drivers/gpu/drm/verisilicon/vs_drv.c
>>> new file mode 100644
>>> index 000000000000..24d333598477
>>> --- /dev/null
>>> +++ b/drivers/gpu/drm/verisilicon/vs_drv.c
>>> @@ -0,0 +1,284 @@
>>> +// SPDX-License-Identifier: GPL-2.0
>>> +/*
>>> + * Copyright (C) 2023 VeriSilicon Holdings Co., Ltd.
>>> + */
>>> +
>>> +#include <linux/clk.h>
>>> +#include <linux/component.h>
>>> +#include <linux/delay.h>
>>> +#include <linux/dma-mapping.h>
>>> +#include <linux/iommu.h>
>>> +#include <linux/of_graph.h>
>>> +#include <linux/of_reserved_mem.h>
>>> +#include <linux/pm_runtime.h>
>>> +#include <linux/reset.h>
>>> +#include <linux/version.h>
>>> +
>>> +#include <drm/drm_aperture.h>
>>> +#include <drm/drm_crtc.h>
>>> +#include <drm/drm_crtc_helper.h>
>>> +#include <drm/drm_debugfs.h>
>>> +#include <drm/drm_drv.h>
>>> +#include <drm/drm_fb_helper.h>
>>> +#include <drm/drm_fbdev_generic.h>
>>> +#include <drm/drm_file.h>
>>> +#include <drm/drm_fourcc.h>
>>> +#include <drm/drm_ioctl.h>
>>> +#include <drm/drm_of.h>
>>> +#include <drm/drm_prime.h>
>>> +#include <drm/drm_probe_helper.h>
>>> +#include <drm/drm_vblank.h>
>>> +
>>> +#include "vs_drv.h"
>>> +
>>> +#define DRV_NAME "starfive"
>>> +#define DRV_DESC "Starfive DRM driver"
>>> +#define DRV_DATE "202305161"
>>> +#define DRV_MAJOR 1
>>> +#define DRV_MINOR 0
>>> +
>>> +static struct platform_driver vs_drm_platform_driver;
>>> +
>>> +static const struct file_operations fops = {
>>> + .owner = THIS_MODULE,
>>> + .open = drm_open,
>>> + .release = drm_release,
>>> + .unlocked_ioctl = drm_ioctl,
>>> + .compat_ioctl = drm_compat_ioctl,
>>> + .poll = drm_poll,
>>> + .read = drm_read,
>>> +};
>>> +
>>> +static struct drm_driver vs_drm_driver = {
>>> + .driver_features = DRIVER_MODESET | DRIVER_ATOMIC | DRIVER_GEM,
>>> + .lastclose = drm_fb_helper_lastclose,
>>> + .prime_handle_to_fd = drm_gem_prime_handle_to_fd,
>>> + .prime_fd_to_handle = drm_gem_prime_fd_to_handle,
>>> + .fops = &fops,
>>> + .name = DRV_NAME,
>>> + .desc = DRV_DESC,
>>> + .date = DRV_DATE,
>>> + .major = DRV_MAJOR,
>>> + .minor = DRV_MINOR,
>>> +};
>>> +
>>> +void vs_drm_update_pitch_alignment(struct drm_device *drm_dev,
>>> + unsigned int alignment)
>>> +{
>>> + struct vs_drm_private *priv = drm_dev->dev_private;
>>> +
>>> + if (alignment > priv->pitch_alignment)
>>> + priv->pitch_alignment = alignment;
>>> +}
>>> +
>>> +static int vs_drm_bind(struct device *dev)
>>> +{
>>> + struct drm_device *drm_dev;
>>> + struct vs_drm_private *priv;
>>> + int ret;
>>> + static u64 dma_mask = DMA_BIT_MASK(40);
>>> +
>>> + /* Remove existing drivers that may own the framebuffer memory. */
>>> + ret = drm_aperture_remove_framebuffers(false, &vs_drm_driver);
>>> + if (ret) {
>>> + DRM_DEV_ERROR(dev,
>>
>> drm_err(), drm_info(), drm_warn(), etc. Here and everwhere else. The
>> DRM_DEV_*() print macros are obsolete.
>>
>>> + "Failed to remove existing framebuffers - %d.\n",
>>> + ret);
>>> + return ret;
>>> + }
>>> +
>>> + drm_dev = drm_dev_alloc(&vs_drm_driver, dev);
>>> + if (IS_ERR(drm_dev))
>>> + return PTR_ERR(drm_dev);
>>> +
>>> + dev_set_drvdata(dev, drm_dev);
>>> +
>>> + priv = devm_kzalloc(drm_dev->dev, sizeof(struct vs_drm_private),
>>> + GFP_KERNEL);
>>> + if (!priv) {
>>> + ret = -ENOMEM;
>>> + goto err_put_dev;
>>> + }
>>> +
>>> + priv->pitch_alignment = 64;
>>> + priv->dma_dev = drm_dev->dev;
>>> + priv->dma_dev->coherent_dma_mask = dma_mask;
>>> + drm_dev->dev_private = priv;
>>
>> dev_private is obsolete and about to go away at some point.
>>
>> Please embed drm_device in vs_drm_private and allocate the memory with
>> devm_drm_dev_alloc().
>>
>>> +
>>> + drm_mode_config_init(drm_dev);
>>
>> drmm_mode_config_init() please.
>>
>>> +
>>> + /* Now try and bind all our sub-components */
>>> + ret = component_bind_all(dev, drm_dev);
>>> + if (ret)
>>> + goto err_mode;
>>> +
>>> + ret = drm_vblank_init(drm_dev, drm_dev->mode_config.num_crtc);
>>> + if (ret)
>>> + goto err_bind;
>>> +
>>> + drm_mode_config_reset(drm_dev);
>>> +
>>> + drm_kms_helper_poll_init(drm_dev);
>>> +
>>> + ret = drm_dev_register(drm_dev, 0);
>>> + if (ret)
>>> + goto err_helper;
>>> +
>>> + drm_fbdev_generic_setup(drm_dev, 32);
>>> +
>>> + return 0;
>>> +
>>> +err_helper:
>>> + drm_kms_helper_poll_fini(drm_dev);
>>> +err_bind:
>>> + component_unbind_all(drm_dev->dev, drm_dev);
>>> +err_mode:
>>> + drm_mode_config_cleanup(drm_dev);
>>> + if (priv->domain)
>>> + iommu_domain_free(priv->domain);
>>> +err_put_dev:
>>> + drm_dev->dev_private = NULL;
>>> + dev_set_drvdata(dev, NULL);
>>> + drm_dev_put(drm_dev);
>>> + return ret;
>>> +}
>>> +
>>> +static void vs_drm_unbind(struct device *dev)
>>> +{
>>> + struct drm_device *drm_dev = dev_get_drvdata(dev);
>>> + struct vs_drm_private *priv = drm_dev->dev_private;
>>> +
>>> + drm_dev_unregister(drm_dev);
>>> +
>>> + drm_kms_helper_poll_fini(drm_dev);
>>> +
>>> + component_unbind_all(drm_dev->dev, drm_dev);
>>> +
>>> + drm_mode_config_cleanup(drm_dev);
>>> +
>>> + if (priv->domain) {
>>> + iommu_domain_free(priv->domain);
>>> + priv->domain = NULL;
>>> + }
>>> +
>>> + drm_dev->dev_private = NULL;
>>> + dev_set_drvdata(dev, NULL);
>>> + drm_dev_put(drm_dev);
>>
>> You rather want to convert the individual steps of this cleanup to
>> managed functions (drmm_ and devm_) to automate the cleanup when as part
>> of drm_dev_put().
>>
>>> +}
>>> +
>>> +static const struct component_master_ops vs_drm_ops = {
>>> + .bind = vs_drm_bind,
>>> + .unbind = vs_drm_unbind,
>>> +};
>>> +
>>> +static struct platform_driver *drm_sub_drivers[] = {
>>> +};
>>> +
>>> +#define NUM_DRM_DRIVERS \
>>> + (sizeof(drm_sub_drivers) / sizeof(struct platform_driver *))
>>
>> Does this really work? sizeof(drm_sub_drivers) isn't know at compile
>> time. It is always assumed to be 0 AFAICT. Or do you fill this array in
>> later patches?
>>
>>
>>> +
>>> +static int compare_dev(struct device *dev, void *data)
>>> +{
>>> + return dev == (struct device *)data;
>>> +}
>>> +
>>> +static struct component_match *vs_drm_match_add(struct device *dev)
>>> +{
>>> + struct component_match *match = NULL;
>>> + int i;
>>> +
>>> + for (i = 0; i < NUM_DRM_DRIVERS; ++i) {
>>> + struct platform_driver *drv = drm_sub_drivers[i];
>>> + struct device *p = NULL, *d;
>>> +
>>> + while ((d = platform_find_device_by_driver(p, &drv->driver))) {
>>> + put_device(p);
>>> +
>>> + component_match_add(dev, &match, compare_dev, d);
>>> + p = d;
>>> + }
>>> + put_device(p);
>>> + }
>>> +
>>> + return match ?: ERR_PTR(-ENODEV);
>>> +}
>>> +
>>> +static int vs_drm_platform_probe(struct platform_device *pdev)
>>> +{
>>> + struct device *dev = &pdev->dev;
>>> + struct component_match *match;
>>> +
>>> + match = vs_drm_match_add(dev);
>>> + if (IS_ERR(match))
>>> + return PTR_ERR(match);
>>> +
>>> + return component_master_add_with_match(dev, &vs_drm_ops, match);
>>> +}
>>> +
>>> +static int vs_drm_platform_remove(struct platform_device *pdev)
>>> +{
>>> + component_master_del(&pdev->dev, &vs_drm_ops);
>>> + return 0;
>>> +}
>>> +
>>> +#ifdef CONFIG_PM_SLEEP
>>> +static int vs_drm_suspend(struct device *dev)
>>> +{
>>> + struct drm_device *drm = dev_get_drvdata(dev);
>>> +
>>> + return drm_mode_config_helper_suspend(drm);
>>> +}
>>> +
>>> +static int vs_drm_resume(struct device *dev)
>>> +{
>>> + struct drm_device *drm = dev_get_drvdata(dev);
>>> +
>>> + return drm_mode_config_helper_resume(drm);
>>> +}
>>> +#endif
>>> +
>>> +static SIMPLE_DEV_PM_OPS(vs_drm_pm_ops, vs_drm_suspend, vs_drm_resume);
>>> +
>>> +static const struct of_device_id vs_drm_dt_ids[] = {
>>> + { .compatible = "verisilicon,display-subsystem", },
>>> +};
>>> +
>>> +MODULE_DEVICE_TABLE(of, vs_drm_dt_ids);
>>> +
>>> +static struct platform_driver vs_drm_platform_driver = {
>>> + .probe = vs_drm_platform_probe,
>>> + .remove = vs_drm_platform_remove,
>>> +
>>> + .driver = {
>>> + .name = DRV_NAME,
>>> + .of_match_table = vs_drm_dt_ids,
>>> + .pm = &vs_drm_pm_ops,
>>> + },
>>> +};
>>> +
>>> +static int __init vs_drm_init(void)
>>> +{
>>> + int ret;
>>> +
>>> + ret = platform_register_drivers(drm_sub_drivers, NUM_DRM_DRIVERS);
>>> + if (ret)
>>> + return ret;
>>> +
>>> + ret = platform_driver_register(&vs_drm_platform_driver);
>>> + if (ret)
>>> + platform_unregister_drivers(drm_sub_drivers, NUM_DRM_DRIVERS);
>>> +
>>> + return ret;
>>> +}
>>> +
>>> +static void __exit vs_drm_fini(void)
>>> +{
>>> + platform_driver_unregister(&vs_drm_platform_driver);
>>> + platform_unregister_drivers(drm_sub_drivers, NUM_DRM_DRIVERS);
>>> +}
>>> +
>>> +module_init(vs_drm_init);
>>> +module_exit(vs_drm_fini);
>>> +
>>> +MODULE_DESCRIPTION("VeriSilicon DRM Driver");
>>> +MODULE_LICENSE("GPL");
>>> diff --git a/drivers/gpu/drm/verisilicon/vs_drv.h b/drivers/gpu/drm/verisilicon/vs_drv.h
>>> new file mode 100644
>>> index 000000000000..0382b44e3bf0
>>> --- /dev/null
>>> +++ b/drivers/gpu/drm/verisilicon/vs_drv.h
>>> @@ -0,0 +1,48 @@
>>> +/* SPDX-License-Identifier: GPL-2.0 */
>>> +/*
>>> + * Copyright (C) 2023 VeriSilicon Holdings Co., Ltd.
>>> + */
>>> +
>>> +#ifndef __VS_DRV_H__
>>> +#define __VS_DRV_H__
>>> +
>>> +#include <linux/module.h>
>>> +#include <linux/platform_device.h>
>>> +#include <linux/version.h>
>>> +#include <drm/drm_drv.h>
>>> +#include <drm/drm_gem.h>
>>> +
>>> +/*
>>> + *
>>> + * @dma_dev: device for DMA API.
>>> + * - use the first attached device if support iommu
>>> + else use drm device (only contiguous buffer support)
>>> + * @domain: iommu domain for DRM.
>>> + * - all DC IOMMU share same domain to reduce mapping
>>> + * @pitch_alignment: buffer pitch alignment required by sub-devices.
>>> + *
>>> + */
>>> +struct vs_drm_private {
>>> + struct device *dma_dev;
>>> + struct iommu_domain *domain;
>>> + unsigned int pitch_alignment;
>>> +};
>>
>> As mentioned, this struct needs to embed struct drm_device.
>>
>>> +
>>> +void vs_drm_update_pitch_alignment(struct drm_device *drm_dev,
>>> + unsigned int alignment);
>>> +
>>> +static inline struct device *to_dma_dev(struct drm_device *dev)
>>> +{
>>> + struct vs_drm_private *priv = dev->dev_private;
>>> +
>>> + return priv->dma_dev;
>>
>> And this needs to be an upcast via container_of().
>>
>>> +}
>>> +
>>> +static inline bool is_iommu_enabled(struct drm_device *dev)
>>> +{
>>> + struct vs_drm_private *priv = dev->dev_private;
>>> +
>>> + return priv->domain ? true : false;
>>> +}
>>> +
>>> +#endif /* __VS_DRV_H__ */
>>> diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
>>> index de703c6be969..af4fb50f9207 100644
>>> --- a/include/uapi/drm/drm_fourcc.h
>>> +++ b/include/uapi/drm/drm_fourcc.h
>>
>> The UAPI changes shouldn't be needed in this patch?
>>
>>> @@ -419,6 +419,7 @@ extern "C" {
>>> #define DRM_FORMAT_MOD_VENDOR_ARM 0x08
>>> #define DRM_FORMAT_MOD_VENDOR_ALLWINNER 0x09
>>> #define DRM_FORMAT_MOD_VENDOR_AMLOGIC 0x0a
>>> +#define DRM_FORMAT_MOD_VENDOR_VS 0x0b
>>>
>>> /* add more to the end as needed */
>>>
>>> @@ -1519,6 +1520,88 @@ drm_fourcc_canonicalize_nvidia_format_mod(__u64 modifier)
>>> #define AMD_FMT_MOD_CLEAR(field) \
>>> (~((__u64)AMD_FMT_MOD_##field##_MASK << AMD_FMT_MOD_##field##_SHIFT))
>>>
>>> +#define DRM_FORMAT_MOD_VS_TYPE_NORMAL 0x00
>>> +#define DRM_FORMAT_MOD_VS_TYPE_COMPRESSED 0x01
>>> +#define DRM_FORMAT_MOD_VS_TYPE_CUSTOM_10BIT 0x02
>>> +#define DRM_FORMAT_MOD_VS_TYPE_MASK ((__u64)0x3 << 54)
>>> +
>>> +#define fourcc_mod_vs_code(type, val) \
>>> + fourcc_mod_code(VS, ((((__u64)type) << 54) | (val)))
>>> +
>>> +#define DRM_FORMAT_MOD_VS_DEC_TILE_MODE_MASK 0x3F
>>> +#define DRM_FORMAT_MOD_VS_DEC_TILE_8X8_XMAJOR 0x00
>>> +#define DRM_FORMAT_MOD_VS_DEC_TILE_8X8_YMAJOR 0x01
>>> +#define DRM_FORMAT_MOD_VS_DEC_TILE_16X4 0x02
>>> +#define DRM_FORMAT_MOD_VS_DEC_TILE_8X4 0x03
>>> +#define DRM_FORMAT_MOD_VS_DEC_TILE_4X8 0x04
>>> +#define DRM_FORMAT_MOD_VS_DEC_RASTER_16X4 0x06
>>> +#define DRM_FORMAT_MOD_VS_DEC_TILE_64X4 0x07
>>> +#define DRM_FORMAT_MOD_VS_DEC_TILE_32X4 0x08
>>> +#define DRM_FORMAT_MOD_VS_DEC_RASTER_256X1 0x09
>>> +#define DRM_FORMAT_MOD_VS_DEC_RASTER_128X1 0x0A
>>> +#define DRM_FORMAT_MOD_VS_DEC_RASTER_64X4 0x0B
>>> +#define DRM_FORMAT_MOD_VS_DEC_RASTER_256X2 0x0C
>>> +#define DRM_FORMAT_MOD_VS_DEC_RASTER_128X2 0x0D
>>> +#define DRM_FORMAT_MOD_VS_DEC_RASTER_128X4 0x0E
>>> +#define DRM_FORMAT_MOD_VS_DEC_RASTER_64X1 0x0F
>>> +#define DRM_FORMAT_MOD_VS_DEC_TILE_16X8 0x10
>>> +#define DRM_FORMAT_MOD_VS_DEC_TILE_8X16 0x11
>>> +#define DRM_FORMAT_MOD_VS_DEC_RASTER_512X1 0x12
>>> +#define DRM_FORMAT_MOD_VS_DEC_RASTER_32X4 0x13
>>> +#define DRM_FORMAT_MOD_VS_DEC_RASTER_64X2 0x14
>>> +#define DRM_FORMAT_MOD_VS_DEC_RASTER_32X2 0x15
>>> +#define DRM_FORMAT_MOD_VS_DEC_RASTER_32X1 0x16
>>> +#define DRM_FORMAT_MOD_VS_DEC_RASTER_16X1 0x17
>>> +#define DRM_FORMAT_MOD_VS_DEC_TILE_128X4 0x18
>>> +#define DRM_FORMAT_MOD_VS_DEC_TILE_256X4 0x19
>>> +#define DRM_FORMAT_MOD_VS_DEC_TILE_512X4 0x1A
>>> +#define DRM_FORMAT_MOD_VS_DEC_TILE_16X16 0x1B
>>> +#define DRM_FORMAT_MOD_VS_DEC_TILE_32X16 0x1C
>>> +#define DRM_FORMAT_MOD_VS_DEC_TILE_64X16 0x1D
>>> +#define DRM_FORMAT_MOD_VS_DEC_TILE_128X8 0x1E
>>> +#define DRM_FORMAT_MOD_VS_DEC_TILE_8X4_S 0x1F
>>> +#define DRM_FORMAT_MOD_VS_DEC_TILE_16X4_S 0x20
>>> +#define DRM_FORMAT_MOD_VS_DEC_TILE_32X4_S 0x21
>>> +#define DRM_FORMAT_MOD_VS_DEC_TILE_16X4_LSB 0x22
>>> +#define DRM_FORMAT_MOD_VS_DEC_TILE_32X4_LSB 0x23
>>> +#define DRM_FORMAT_MOD_VS_DEC_TILE_32X8 0x24
>>> +
>>> +#define DRM_FORMAT_MOD_VS_DEC_ALIGN_32 (0x01 << 6)
>>> +#define DRM_FORMAT_MOD_VS_DEC_ALIGN_64 (0x01 << 7)
>>> +
>>> +#define fourcc_mod_vs_dec_code(tile, align) \
>>> + fourcc_mod_vs_code(DRM_FORMAT_MOD_VS_TYPE_COMPRESSED, \
>>> + ((tile) | (align)))
>>> +
>>> +#define DRM_FORMAT_MOD_VS_NORM_MODE_MASK 0x1F
>>> +#define DRM_FORMAT_MOD_VS_LINEAR 0x00
>>> +#define DRM_FORMAT_MOD_VS_TILED4x4 0x01
>>> +#define DRM_FORMAT_MOD_VS_SUPER_TILED_XMAJOR 0x02
>>> +#define DRM_FORMAT_MOD_VS_SUPER_TILED_YMAJOR 0x03
>>> +#define DRM_FORMAT_MOD_VS_TILE_8X8 0x04
>>> +#define DRM_FORMAT_MOD_VS_TILE_MODE1 0x05
>>> +#define DRM_FORMAT_MOD_VS_TILE_MODE2 0x06
>>> +#define DRM_FORMAT_MOD_VS_TILE_8X4 0x07
>>> +#define DRM_FORMAT_MOD_VS_TILE_MODE4 0x08
>>> +#define DRM_FORMAT_MOD_VS_TILE_MODE5 0x09
>>> +#define DRM_FORMAT_MOD_VS_TILE_MODE6 0x0A
>>> +#define DRM_FORMAT_MOD_VS_SUPER_TILED_XMAJOR_8X4 0x0B
>>> +#define DRM_FORMAT_MOD_VS_SUPER_TILED_YMAJOR_4X8 0x0C
>>> +#define DRM_FORMAT_MOD_VS_TILE_Y 0x0D
>>> +#define DRM_FORMAT_MOD_VS_TILE_128X1 0x0F
>>> +#define DRM_FORMAT_MOD_VS_TILE_256X1 0x10
>>> +#define DRM_FORMAT_MOD_VS_TILE_32X1 0x11
>>> +#define DRM_FORMAT_MOD_VS_TILE_64X1 0x12
>>> +#define DRM_FORMAT_MOD_VS_TILE_MODE4X4 0x15
>>> +
>>> +#define fourcc_mod_vs_norm_code(tile) \
>>> + fourcc_mod_vs_code(DRM_FORMAT_MOD_VS_TYPE_NORMAL, \
>>> + (tile))
>>> +
>>> +#define fourcc_mod_vs_custom_code(tile) \
>>> + fourcc_mod_vs_code(DRM_FORMAT_MOD_VS_TYPE_CUSTOM_10BIT, \
>>> + (tile))
>>> +
>>> #if defined(__cplusplus)
>>> }
>>> #endif
>>> diff --git a/include/uapi/drm/vs_drm.h b/include/uapi/drm/vs_drm.h
>>> new file mode 100644
>>> index 000000000000..96b7fc95d658
>>> --- /dev/null
>>> +++ b/include/uapi/drm/vs_drm.h
>>
>> Another UAPI addition that appears to be unused. Please only add things
>> that you're using.
>>
>> Best regards
>> Thomas
>>
>>> @@ -0,0 +1,50 @@
>>> +/* SPDX-License-Identifier: GPL-2.0+ WITH Linux-syscall-note */
>>> +/*
>>> + * Copyright (C) 2020 VeriSilicon Holdings Co., Ltd.
>>> + */
>>> +
>>> +#ifndef __VS_DRM_H__
>>> +#define __VS_DRM_H__
>>> +
>>> +#include "drm.h"
>>> +
>>> +enum drm_vs_degamma_mode {
>>> + VS_DEGAMMA_DISABLE = 0,
>>> + VS_DEGAMMA_BT709 = 1,
>>> + VS_DEGAMMA_BT2020 = 2,
>>> +};
>>> +
>>> +enum drm_vs_sync_dc_mode {
>>> + VS_SINGLE_DC = 0,
>>> + VS_MULTI_DC_PRIMARY = 1,
>>> + VS_MULTI_DC_SECONDARY = 2,
>>> +};
>>> +
>>> +enum drm_vs_mmu_prefetch_mode {
>>> + VS_MMU_PREFETCH_DISABLE = 0,
>>> + VS_MMU_PREFETCH_ENABLE = 1,
>>> +};
>>> +
>>> +struct drm_vs_watermark {
>>> + __u32 watermark;
>>> + __u8 qos_low;
>>> + __u8 qos_high;
>>> +};
>>> +
>>> +struct drm_vs_color_mgmt {
>>> + __u32 colorkey;
>>> + __u32 colorkey_high;
>>> + __u32 clear_value;
>>> + bool clear_enable;
>>> + bool transparency;
>>> +};
>>> +
>>> +struct drm_vs_roi {
>>> + bool enable;
>>> + __u16 roi_x;
>>> + __u16 roi_y;
>>> + __u16 roi_w;
>>> + __u16 roi_h;
>>> +};
>>> +
>>> +#endif /* __VS_DRM_H__ */
>>
>> --
>> Thomas Zimmermann
>> Graphics Driver Developer
>> SUSE Software Solutions Germany GmbH
>> Frankenstrasse 146, 90461 Nuernberg, Germany
>> GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
>> HRB 36809 (AG Nuernberg)
>
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)
Le samedi 08 juillet 2023 à 21:11 +0200, Thomas Zimmermann a écrit :
> Hi
>
> Am 07.07.23 um 20:09 schrieb Nicolas Dufresne:
> [...]
> > > > +config DRM_VERISILICON
> > > > + tristate "DRM Support for VeriSilicon"
> > >
> > > Can you rename the driver and files? 'VeriSilicon' seems
> > > unpronounceable. Simply 'StarFive' and starfive/ would be fine.
> >
> > Are you sure you want to request this ? If the display controller is a
> > Verisilicon design, it will be super odd to use on other SoC that aren't from
> > StarFive. Think about STM network driver, which is DesignWare.
>
> It's not a hard requirement. If that's the name, so be it.
If that helps you pronouncing this, it is commonly pronounced has:
very-silicon
Or just a caulking mess if you really hate it :-D
Nicolas
>
> Best regards
> Thomas
>
> >
> > Nicolas
> >
> > >
> > > > + depends on DRM
> > > > + select DRM_KMS_HELPER
> > > > + select CMA
> > > > + select DMA_CMA
> > > > + help
> > > > + Choose this option if you have a VeriSilicon soc chipset.
> > > > + This driver provides VeriSilicon kernel mode
> > > > + setting and buffer management. It does not
> > > > + provide 2D or 3D acceleration.
> > > > diff --git a/drivers/gpu/drm/verisilicon/Makefile b/drivers/gpu/drm/verisilicon/Makefile
> > > > new file mode 100644
> > > > index 000000000000..64ce1b26546c
> > > > --- /dev/null
> > > > +++ b/drivers/gpu/drm/verisilicon/Makefile
> > > > @@ -0,0 +1,6 @@
> > > > +# SPDX-License-Identifier: GPL-2.0
> > > > +
> > > > +vs_drm-objs := vs_drv.o
> > > > +
> > > > +obj-$(CONFIG_DRM_VERISILICON) += vs_drm.o
> > > > +
> > > > diff --git a/drivers/gpu/drm/verisilicon/vs_drv.c b/drivers/gpu/drm/verisilicon/vs_drv.c
> > > > new file mode 100644
> > > > index 000000000000..24d333598477
> > > > --- /dev/null
> > > > +++ b/drivers/gpu/drm/verisilicon/vs_drv.c
> > > > @@ -0,0 +1,284 @@
> > > > +// SPDX-License-Identifier: GPL-2.0
> > > > +/*
> > > > + * Copyright (C) 2023 VeriSilicon Holdings Co., Ltd.
> > > > + */
> > > > +
> > > > +#include <linux/clk.h>
> > > > +#include <linux/component.h>
> > > > +#include <linux/delay.h>
> > > > +#include <linux/dma-mapping.h>
> > > > +#include <linux/iommu.h>
> > > > +#include <linux/of_graph.h>
> > > > +#include <linux/of_reserved_mem.h>
> > > > +#include <linux/pm_runtime.h>
> > > > +#include <linux/reset.h>
> > > > +#include <linux/version.h>
> > > > +
> > > > +#include <drm/drm_aperture.h>
> > > > +#include <drm/drm_crtc.h>
> > > > +#include <drm/drm_crtc_helper.h>
> > > > +#include <drm/drm_debugfs.h>
> > > > +#include <drm/drm_drv.h>
> > > > +#include <drm/drm_fb_helper.h>
> > > > +#include <drm/drm_fbdev_generic.h>
> > > > +#include <drm/drm_file.h>
> > > > +#include <drm/drm_fourcc.h>
> > > > +#include <drm/drm_ioctl.h>
> > > > +#include <drm/drm_of.h>
> > > > +#include <drm/drm_prime.h>
> > > > +#include <drm/drm_probe_helper.h>
> > > > +#include <drm/drm_vblank.h>
> > > > +
> > > > +#include "vs_drv.h"
> > > > +
> > > > +#define DRV_NAME "starfive"
> > > > +#define DRV_DESC "Starfive DRM driver"
> > > > +#define DRV_DATE "202305161"
> > > > +#define DRV_MAJOR 1
> > > > +#define DRV_MINOR 0
> > > > +
> > > > +static struct platform_driver vs_drm_platform_driver;
> > > > +
> > > > +static const struct file_operations fops = {
> > > > + .owner = THIS_MODULE,
> > > > + .open = drm_open,
> > > > + .release = drm_release,
> > > > + .unlocked_ioctl = drm_ioctl,
> > > > + .compat_ioctl = drm_compat_ioctl,
> > > > + .poll = drm_poll,
> > > > + .read = drm_read,
> > > > +};
> > > > +
> > > > +static struct drm_driver vs_drm_driver = {
> > > > + .driver_features = DRIVER_MODESET | DRIVER_ATOMIC | DRIVER_GEM,
> > > > + .lastclose = drm_fb_helper_lastclose,
> > > > + .prime_handle_to_fd = drm_gem_prime_handle_to_fd,
> > > > + .prime_fd_to_handle = drm_gem_prime_fd_to_handle,
> > > > + .fops = &fops,
> > > > + .name = DRV_NAME,
> > > > + .desc = DRV_DESC,
> > > > + .date = DRV_DATE,
> > > > + .major = DRV_MAJOR,
> > > > + .minor = DRV_MINOR,
> > > > +};
> > > > +
> > > > +void vs_drm_update_pitch_alignment(struct drm_device *drm_dev,
> > > > + unsigned int alignment)
> > > > +{
> > > > + struct vs_drm_private *priv = drm_dev->dev_private;
> > > > +
> > > > + if (alignment > priv->pitch_alignment)
> > > > + priv->pitch_alignment = alignment;
> > > > +}
> > > > +
> > > > +static int vs_drm_bind(struct device *dev)
> > > > +{
> > > > + struct drm_device *drm_dev;
> > > > + struct vs_drm_private *priv;
> > > > + int ret;
> > > > + static u64 dma_mask = DMA_BIT_MASK(40);
> > > > +
> > > > + /* Remove existing drivers that may own the framebuffer memory. */
> > > > + ret = drm_aperture_remove_framebuffers(false, &vs_drm_driver);
> > > > + if (ret) {
> > > > + DRM_DEV_ERROR(dev,
> > >
> > > drm_err(), drm_info(), drm_warn(), etc. Here and everwhere else. The
> > > DRM_DEV_*() print macros are obsolete.
> > >
> > > > + "Failed to remove existing framebuffers - %d.\n",
> > > > + ret);
> > > > + return ret;
> > > > + }
> > > > +
> > > > + drm_dev = drm_dev_alloc(&vs_drm_driver, dev);
> > > > + if (IS_ERR(drm_dev))
> > > > + return PTR_ERR(drm_dev);
> > > > +
> > > > + dev_set_drvdata(dev, drm_dev);
> > > > +
> > > > + priv = devm_kzalloc(drm_dev->dev, sizeof(struct vs_drm_private),
> > > > + GFP_KERNEL);
> > > > + if (!priv) {
> > > > + ret = -ENOMEM;
> > > > + goto err_put_dev;
> > > > + }
> > > > +
> > > > + priv->pitch_alignment = 64;
> > > > + priv->dma_dev = drm_dev->dev;
> > > > + priv->dma_dev->coherent_dma_mask = dma_mask;
> > > > + drm_dev->dev_private = priv;
> > >
> > > dev_private is obsolete and about to go away at some point.
> > >
> > > Please embed drm_device in vs_drm_private and allocate the memory with
> > > devm_drm_dev_alloc().
> > >
> > > > +
> > > > + drm_mode_config_init(drm_dev);
> > >
> > > drmm_mode_config_init() please.
> > >
> > > > +
> > > > + /* Now try and bind all our sub-components */
> > > > + ret = component_bind_all(dev, drm_dev);
> > > > + if (ret)
> > > > + goto err_mode;
> > > > +
> > > > + ret = drm_vblank_init(drm_dev, drm_dev->mode_config.num_crtc);
> > > > + if (ret)
> > > > + goto err_bind;
> > > > +
> > > > + drm_mode_config_reset(drm_dev);
> > > > +
> > > > + drm_kms_helper_poll_init(drm_dev);
> > > > +
> > > > + ret = drm_dev_register(drm_dev, 0);
> > > > + if (ret)
> > > > + goto err_helper;
> > > > +
> > > > + drm_fbdev_generic_setup(drm_dev, 32);
> > > > +
> > > > + return 0;
> > > > +
> > > > +err_helper:
> > > > + drm_kms_helper_poll_fini(drm_dev);
> > > > +err_bind:
> > > > + component_unbind_all(drm_dev->dev, drm_dev);
> > > > +err_mode:
> > > > + drm_mode_config_cleanup(drm_dev);
> > > > + if (priv->domain)
> > > > + iommu_domain_free(priv->domain);
> > > > +err_put_dev:
> > > > + drm_dev->dev_private = NULL;
> > > > + dev_set_drvdata(dev, NULL);
> > > > + drm_dev_put(drm_dev);
> > > > + return ret;
> > > > +}
> > > > +
> > > > +static void vs_drm_unbind(struct device *dev)
> > > > +{
> > > > + struct drm_device *drm_dev = dev_get_drvdata(dev);
> > > > + struct vs_drm_private *priv = drm_dev->dev_private;
> > > > +
> > > > + drm_dev_unregister(drm_dev);
> > > > +
> > > > + drm_kms_helper_poll_fini(drm_dev);
> > > > +
> > > > + component_unbind_all(drm_dev->dev, drm_dev);
> > > > +
> > > > + drm_mode_config_cleanup(drm_dev);
> > > > +
> > > > + if (priv->domain) {
> > > > + iommu_domain_free(priv->domain);
> > > > + priv->domain = NULL;
> > > > + }
> > > > +
> > > > + drm_dev->dev_private = NULL;
> > > > + dev_set_drvdata(dev, NULL);
> > > > + drm_dev_put(drm_dev);
> > >
> > > You rather want to convert the individual steps of this cleanup to
> > > managed functions (drmm_ and devm_) to automate the cleanup when as part
> > > of drm_dev_put().
> > >
> > > > +}
> > > > +
> > > > +static const struct component_master_ops vs_drm_ops = {
> > > > + .bind = vs_drm_bind,
> > > > + .unbind = vs_drm_unbind,
> > > > +};
> > > > +
> > > > +static struct platform_driver *drm_sub_drivers[] = {
> > > > +};
> > > > +
> > > > +#define NUM_DRM_DRIVERS \
> > > > + (sizeof(drm_sub_drivers) / sizeof(struct platform_driver *))
> > >
> > > Does this really work? sizeof(drm_sub_drivers) isn't know at compile
> > > time. It is always assumed to be 0 AFAICT. Or do you fill this array in
> > > later patches?
> > >
> > >
> > > > +
> > > > +static int compare_dev(struct device *dev, void *data)
> > > > +{
> > > > + return dev == (struct device *)data;
> > > > +}
> > > > +
> > > > +static struct component_match *vs_drm_match_add(struct device *dev)
> > > > +{
> > > > + struct component_match *match = NULL;
> > > > + int i;
> > > > +
> > > > + for (i = 0; i < NUM_DRM_DRIVERS; ++i) {
> > > > + struct platform_driver *drv = drm_sub_drivers[i];
> > > > + struct device *p = NULL, *d;
> > > > +
> > > > + while ((d = platform_find_device_by_driver(p, &drv->driver))) {
> > > > + put_device(p);
> > > > +
> > > > + component_match_add(dev, &match, compare_dev, d);
> > > > + p = d;
> > > > + }
> > > > + put_device(p);
> > > > + }
> > > > +
> > > > + return match ?: ERR_PTR(-ENODEV);
> > > > +}
> > > > +
> > > > +static int vs_drm_platform_probe(struct platform_device *pdev)
> > > > +{
> > > > + struct device *dev = &pdev->dev;
> > > > + struct component_match *match;
> > > > +
> > > > + match = vs_drm_match_add(dev);
> > > > + if (IS_ERR(match))
> > > > + return PTR_ERR(match);
> > > > +
> > > > + return component_master_add_with_match(dev, &vs_drm_ops, match);
> > > > +}
> > > > +
> > > > +static int vs_drm_platform_remove(struct platform_device *pdev)
> > > > +{
> > > > + component_master_del(&pdev->dev, &vs_drm_ops);
> > > > + return 0;
> > > > +}
> > > > +
> > > > +#ifdef CONFIG_PM_SLEEP
> > > > +static int vs_drm_suspend(struct device *dev)
> > > > +{
> > > > + struct drm_device *drm = dev_get_drvdata(dev);
> > > > +
> > > > + return drm_mode_config_helper_suspend(drm);
> > > > +}
> > > > +
> > > > +static int vs_drm_resume(struct device *dev)
> > > > +{
> > > > + struct drm_device *drm = dev_get_drvdata(dev);
> > > > +
> > > > + return drm_mode_config_helper_resume(drm);
> > > > +}
> > > > +#endif
> > > > +
> > > > +static SIMPLE_DEV_PM_OPS(vs_drm_pm_ops, vs_drm_suspend, vs_drm_resume);
> > > > +
> > > > +static const struct of_device_id vs_drm_dt_ids[] = {
> > > > + { .compatible = "verisilicon,display-subsystem", },
> > > > +};
> > > > +
> > > > +MODULE_DEVICE_TABLE(of, vs_drm_dt_ids);
> > > > +
> > > > +static struct platform_driver vs_drm_platform_driver = {
> > > > + .probe = vs_drm_platform_probe,
> > > > + .remove = vs_drm_platform_remove,
> > > > +
> > > > + .driver = {
> > > > + .name = DRV_NAME,
> > > > + .of_match_table = vs_drm_dt_ids,
> > > > + .pm = &vs_drm_pm_ops,
> > > > + },
> > > > +};
> > > > +
> > > > +static int __init vs_drm_init(void)
> > > > +{
> > > > + int ret;
> > > > +
> > > > + ret = platform_register_drivers(drm_sub_drivers, NUM_DRM_DRIVERS);
> > > > + if (ret)
> > > > + return ret;
> > > > +
> > > > + ret = platform_driver_register(&vs_drm_platform_driver);
> > > > + if (ret)
> > > > + platform_unregister_drivers(drm_sub_drivers, NUM_DRM_DRIVERS);
> > > > +
> > > > + return ret;
> > > > +}
> > > > +
> > > > +static void __exit vs_drm_fini(void)
> > > > +{
> > > > + platform_driver_unregister(&vs_drm_platform_driver);
> > > > + platform_unregister_drivers(drm_sub_drivers, NUM_DRM_DRIVERS);
> > > > +}
> > > > +
> > > > +module_init(vs_drm_init);
> > > > +module_exit(vs_drm_fini);
> > > > +
> > > > +MODULE_DESCRIPTION("VeriSilicon DRM Driver");
> > > > +MODULE_LICENSE("GPL");
> > > > diff --git a/drivers/gpu/drm/verisilicon/vs_drv.h b/drivers/gpu/drm/verisilicon/vs_drv.h
> > > > new file mode 100644
> > > > index 000000000000..0382b44e3bf0
> > > > --- /dev/null
> > > > +++ b/drivers/gpu/drm/verisilicon/vs_drv.h
> > > > @@ -0,0 +1,48 @@
> > > > +/* SPDX-License-Identifier: GPL-2.0 */
> > > > +/*
> > > > + * Copyright (C) 2023 VeriSilicon Holdings Co., Ltd.
> > > > + */
> > > > +
> > > > +#ifndef __VS_DRV_H__
> > > > +#define __VS_DRV_H__
> > > > +
> > > > +#include <linux/module.h>
> > > > +#include <linux/platform_device.h>
> > > > +#include <linux/version.h>
> > > > +#include <drm/drm_drv.h>
> > > > +#include <drm/drm_gem.h>
> > > > +
> > > > +/*
> > > > + *
> > > > + * @dma_dev: device for DMA API.
> > > > + * - use the first attached device if support iommu
> > > > + else use drm device (only contiguous buffer support)
> > > > + * @domain: iommu domain for DRM.
> > > > + * - all DC IOMMU share same domain to reduce mapping
> > > > + * @pitch_alignment: buffer pitch alignment required by sub-devices.
> > > > + *
> > > > + */
> > > > +struct vs_drm_private {
> > > > + struct device *dma_dev;
> > > > + struct iommu_domain *domain;
> > > > + unsigned int pitch_alignment;
> > > > +};
> > >
> > > As mentioned, this struct needs to embed struct drm_device.
> > >
> > > > +
> > > > +void vs_drm_update_pitch_alignment(struct drm_device *drm_dev,
> > > > + unsigned int alignment);
> > > > +
> > > > +static inline struct device *to_dma_dev(struct drm_device *dev)
> > > > +{
> > > > + struct vs_drm_private *priv = dev->dev_private;
> > > > +
> > > > + return priv->dma_dev;
> > >
> > > And this needs to be an upcast via container_of().
> > >
> > > > +}
> > > > +
> > > > +static inline bool is_iommu_enabled(struct drm_device *dev)
> > > > +{
> > > > + struct vs_drm_private *priv = dev->dev_private;
> > > > +
> > > > + return priv->domain ? true : false;
> > > > +}
> > > > +
> > > > +#endif /* __VS_DRV_H__ */
> > > > diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
> > > > index de703c6be969..af4fb50f9207 100644
> > > > --- a/include/uapi/drm/drm_fourcc.h
> > > > +++ b/include/uapi/drm/drm_fourcc.h
> > >
> > > The UAPI changes shouldn't be needed in this patch?
> > >
> > > > @@ -419,6 +419,7 @@ extern "C" {
> > > > #define DRM_FORMAT_MOD_VENDOR_ARM 0x08
> > > > #define DRM_FORMAT_MOD_VENDOR_ALLWINNER 0x09
> > > > #define DRM_FORMAT_MOD_VENDOR_AMLOGIC 0x0a
> > > > +#define DRM_FORMAT_MOD_VENDOR_VS 0x0b
> > > >
> > > > /* add more to the end as needed */
> > > >
> > > > @@ -1519,6 +1520,88 @@ drm_fourcc_canonicalize_nvidia_format_mod(__u64 modifier)
> > > > #define AMD_FMT_MOD_CLEAR(field) \
> > > > (~((__u64)AMD_FMT_MOD_##field##_MASK << AMD_FMT_MOD_##field##_SHIFT))
> > > >
> > > > +#define DRM_FORMAT_MOD_VS_TYPE_NORMAL 0x00
> > > > +#define DRM_FORMAT_MOD_VS_TYPE_COMPRESSED 0x01
> > > > +#define DRM_FORMAT_MOD_VS_TYPE_CUSTOM_10BIT 0x02
> > > > +#define DRM_FORMAT_MOD_VS_TYPE_MASK ((__u64)0x3 << 54)
> > > > +
> > > > +#define fourcc_mod_vs_code(type, val) \
> > > > + fourcc_mod_code(VS, ((((__u64)type) << 54) | (val)))
> > > > +
> > > > +#define DRM_FORMAT_MOD_VS_DEC_TILE_MODE_MASK 0x3F
> > > > +#define DRM_FORMAT_MOD_VS_DEC_TILE_8X8_XMAJOR 0x00
> > > > +#define DRM_FORMAT_MOD_VS_DEC_TILE_8X8_YMAJOR 0x01
> > > > +#define DRM_FORMAT_MOD_VS_DEC_TILE_16X4 0x02
> > > > +#define DRM_FORMAT_MOD_VS_DEC_TILE_8X4 0x03
> > > > +#define DRM_FORMAT_MOD_VS_DEC_TILE_4X8 0x04
> > > > +#define DRM_FORMAT_MOD_VS_DEC_RASTER_16X4 0x06
> > > > +#define DRM_FORMAT_MOD_VS_DEC_TILE_64X4 0x07
> > > > +#define DRM_FORMAT_MOD_VS_DEC_TILE_32X4 0x08
> > > > +#define DRM_FORMAT_MOD_VS_DEC_RASTER_256X1 0x09
> > > > +#define DRM_FORMAT_MOD_VS_DEC_RASTER_128X1 0x0A
> > > > +#define DRM_FORMAT_MOD_VS_DEC_RASTER_64X4 0x0B
> > > > +#define DRM_FORMAT_MOD_VS_DEC_RASTER_256X2 0x0C
> > > > +#define DRM_FORMAT_MOD_VS_DEC_RASTER_128X2 0x0D
> > > > +#define DRM_FORMAT_MOD_VS_DEC_RASTER_128X4 0x0E
> > > > +#define DRM_FORMAT_MOD_VS_DEC_RASTER_64X1 0x0F
> > > > +#define DRM_FORMAT_MOD_VS_DEC_TILE_16X8 0x10
> > > > +#define DRM_FORMAT_MOD_VS_DEC_TILE_8X16 0x11
> > > > +#define DRM_FORMAT_MOD_VS_DEC_RASTER_512X1 0x12
> > > > +#define DRM_FORMAT_MOD_VS_DEC_RASTER_32X4 0x13
> > > > +#define DRM_FORMAT_MOD_VS_DEC_RASTER_64X2 0x14
> > > > +#define DRM_FORMAT_MOD_VS_DEC_RASTER_32X2 0x15
> > > > +#define DRM_FORMAT_MOD_VS_DEC_RASTER_32X1 0x16
> > > > +#define DRM_FORMAT_MOD_VS_DEC_RASTER_16X1 0x17
> > > > +#define DRM_FORMAT_MOD_VS_DEC_TILE_128X4 0x18
> > > > +#define DRM_FORMAT_MOD_VS_DEC_TILE_256X4 0x19
> > > > +#define DRM_FORMAT_MOD_VS_DEC_TILE_512X4 0x1A
> > > > +#define DRM_FORMAT_MOD_VS_DEC_TILE_16X16 0x1B
> > > > +#define DRM_FORMAT_MOD_VS_DEC_TILE_32X16 0x1C
> > > > +#define DRM_FORMAT_MOD_VS_DEC_TILE_64X16 0x1D
> > > > +#define DRM_FORMAT_MOD_VS_DEC_TILE_128X8 0x1E
> > > > +#define DRM_FORMAT_MOD_VS_DEC_TILE_8X4_S 0x1F
> > > > +#define DRM_FORMAT_MOD_VS_DEC_TILE_16X4_S 0x20
> > > > +#define DRM_FORMAT_MOD_VS_DEC_TILE_32X4_S 0x21
> > > > +#define DRM_FORMAT_MOD_VS_DEC_TILE_16X4_LSB 0x22
> > > > +#define DRM_FORMAT_MOD_VS_DEC_TILE_32X4_LSB 0x23
> > > > +#define DRM_FORMAT_MOD_VS_DEC_TILE_32X8 0x24
> > > > +
> > > > +#define DRM_FORMAT_MOD_VS_DEC_ALIGN_32 (0x01 << 6)
> > > > +#define DRM_FORMAT_MOD_VS_DEC_ALIGN_64 (0x01 << 7)
> > > > +
> > > > +#define fourcc_mod_vs_dec_code(tile, align) \
> > > > + fourcc_mod_vs_code(DRM_FORMAT_MOD_VS_TYPE_COMPRESSED, \
> > > > + ((tile) | (align)))
> > > > +
> > > > +#define DRM_FORMAT_MOD_VS_NORM_MODE_MASK 0x1F
> > > > +#define DRM_FORMAT_MOD_VS_LINEAR 0x00
> > > > +#define DRM_FORMAT_MOD_VS_TILED4x4 0x01
> > > > +#define DRM_FORMAT_MOD_VS_SUPER_TILED_XMAJOR 0x02
> > > > +#define DRM_FORMAT_MOD_VS_SUPER_TILED_YMAJOR 0x03
> > > > +#define DRM_FORMAT_MOD_VS_TILE_8X8 0x04
> > > > +#define DRM_FORMAT_MOD_VS_TILE_MODE1 0x05
> > > > +#define DRM_FORMAT_MOD_VS_TILE_MODE2 0x06
> > > > +#define DRM_FORMAT_MOD_VS_TILE_8X4 0x07
> > > > +#define DRM_FORMAT_MOD_VS_TILE_MODE4 0x08
> > > > +#define DRM_FORMAT_MOD_VS_TILE_MODE5 0x09
> > > > +#define DRM_FORMAT_MOD_VS_TILE_MODE6 0x0A
> > > > +#define DRM_FORMAT_MOD_VS_SUPER_TILED_XMAJOR_8X4 0x0B
> > > > +#define DRM_FORMAT_MOD_VS_SUPER_TILED_YMAJOR_4X8 0x0C
> > > > +#define DRM_FORMAT_MOD_VS_TILE_Y 0x0D
> > > > +#define DRM_FORMAT_MOD_VS_TILE_128X1 0x0F
> > > > +#define DRM_FORMAT_MOD_VS_TILE_256X1 0x10
> > > > +#define DRM_FORMAT_MOD_VS_TILE_32X1 0x11
> > > > +#define DRM_FORMAT_MOD_VS_TILE_64X1 0x12
> > > > +#define DRM_FORMAT_MOD_VS_TILE_MODE4X4 0x15
> > > > +
> > > > +#define fourcc_mod_vs_norm_code(tile) \
> > > > + fourcc_mod_vs_code(DRM_FORMAT_MOD_VS_TYPE_NORMAL, \
> > > > + (tile))
> > > > +
> > > > +#define fourcc_mod_vs_custom_code(tile) \
> > > > + fourcc_mod_vs_code(DRM_FORMAT_MOD_VS_TYPE_CUSTOM_10BIT, \
> > > > + (tile))
> > > > +
> > > > #if defined(__cplusplus)
> > > > }
> > > > #endif
> > > > diff --git a/include/uapi/drm/vs_drm.h b/include/uapi/drm/vs_drm.h
> > > > new file mode 100644
> > > > index 000000000000..96b7fc95d658
> > > > --- /dev/null
> > > > +++ b/include/uapi/drm/vs_drm.h
> > >
> > > Another UAPI addition that appears to be unused. Please only add things
> > > that you're using.
> > >
> > > Best regards
> > > Thomas
> > >
> > > > @@ -0,0 +1,50 @@
> > > > +/* SPDX-License-Identifier: GPL-2.0+ WITH Linux-syscall-note */
> > > > +/*
> > > > + * Copyright (C) 2020 VeriSilicon Holdings Co., Ltd.
> > > > + */
> > > > +
> > > > +#ifndef __VS_DRM_H__
> > > > +#define __VS_DRM_H__
> > > > +
> > > > +#include "drm.h"
> > > > +
> > > > +enum drm_vs_degamma_mode {
> > > > + VS_DEGAMMA_DISABLE = 0,
> > > > + VS_DEGAMMA_BT709 = 1,
> > > > + VS_DEGAMMA_BT2020 = 2,
> > > > +};
> > > > +
> > > > +enum drm_vs_sync_dc_mode {
> > > > + VS_SINGLE_DC = 0,
> > > > + VS_MULTI_DC_PRIMARY = 1,
> > > > + VS_MULTI_DC_SECONDARY = 2,
> > > > +};
> > > > +
> > > > +enum drm_vs_mmu_prefetch_mode {
> > > > + VS_MMU_PREFETCH_DISABLE = 0,
> > > > + VS_MMU_PREFETCH_ENABLE = 1,
> > > > +};
> > > > +
> > > > +struct drm_vs_watermark {
> > > > + __u32 watermark;
> > > > + __u8 qos_low;
> > > > + __u8 qos_high;
> > > > +};
> > > > +
> > > > +struct drm_vs_color_mgmt {
> > > > + __u32 colorkey;
> > > > + __u32 colorkey_high;
> > > > + __u32 clear_value;
> > > > + bool clear_enable;
> > > > + bool transparency;
> > > > +};
> > > > +
> > > > +struct drm_vs_roi {
> > > > + bool enable;
> > > > + __u16 roi_x;
> > > > + __u16 roi_y;
> > > > + __u16 roi_w;
> > > > + __u16 roi_h;
> > > > +};
> > > > +
> > > > +#endif /* __VS_DRM_H__ */
> > >
> > > --
> > > Thomas Zimmermann
> > > Graphics Driver Developer
> > > SUSE Software Solutions Germany GmbH
> > > Frankenstrasse 146, 90461 Nuernberg, Germany
> > > GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
> > > HRB 36809 (AG Nuernberg)
> >
>
> --
> Thomas Zimmermann
> Graphics Driver Developer
> SUSE Software Solutions Germany GmbH
> Frankenstrasse 146, 90461 Nuernberg, Germany
> GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
> HRB 36809 (AG Nuernberg)
On 2023/6/7 16:53, Lucas Stach wrote:
> Hi Keith,
>
> Am Freitag, dem 02.06.2023 um 15:40 +0800 schrieb Keith Zhao:
>> Add a basic platform driver of the DRM driver for JH7110 SoC.
>>
>> Signed-off-by: Keith Zhao <[email protected]>
>> ---
>> MAINTAINERS | 2 +
>> drivers/gpu/drm/Kconfig | 2 +
>> drivers/gpu/drm/Makefile | 1 +
>> drivers/gpu/drm/verisilicon/Kconfig | 13 ++
>> drivers/gpu/drm/verisilicon/Makefile | 6 +
>> drivers/gpu/drm/verisilicon/vs_drv.c | 284 +++++++++++++++++++++++++++
>> drivers/gpu/drm/verisilicon/vs_drv.h | 48 +++++
>> include/uapi/drm/drm_fourcc.h | 83 ++++++++
>> include/uapi/drm/vs_drm.h | 50 +++++
>> 9 files changed, 489 insertions(+)
>> create mode 100644 drivers/gpu/drm/verisilicon/Kconfig
>> create mode 100644 drivers/gpu/drm/verisilicon/Makefile
>> create mode 100644 drivers/gpu/drm/verisilicon/vs_drv.c
>> create mode 100644 drivers/gpu/drm/verisilicon/vs_drv.h
>> create mode 100644 include/uapi/drm/vs_drm.h
>>
>>
>> [...]
>> +#endif /* __VS_DRV_H__ */
>> diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
>> index de703c6be969..af4fb50f9207 100644
>> --- a/include/uapi/drm/drm_fourcc.h
>> +++ b/include/uapi/drm/drm_fourcc.h
>> @@ -419,6 +419,7 @@ extern "C" {
>> #define DRM_FORMAT_MOD_VENDOR_ARM 0x08
>> #define DRM_FORMAT_MOD_VENDOR_ALLWINNER 0x09
>> #define DRM_FORMAT_MOD_VENDOR_AMLOGIC 0x0a
>> +#define DRM_FORMAT_MOD_VENDOR_VS 0x0b
>>
>> /* add more to the end as needed */
>>
>> @@ -1519,6 +1520,88 @@ drm_fourcc_canonicalize_nvidia_format_mod(__u64 modifier)
>> #define AMD_FMT_MOD_CLEAR(field) \
>> (~((__u64)AMD_FMT_MOD_##field##_MASK << AMD_FMT_MOD_##field##_SHIFT))
>>
>> +#define DRM_FORMAT_MOD_VS_TYPE_NORMAL 0x00
>> +#define DRM_FORMAT_MOD_VS_TYPE_COMPRESSED 0x01
>> +#define DRM_FORMAT_MOD_VS_TYPE_CUSTOM_10BIT 0x02
>> +#define DRM_FORMAT_MOD_VS_TYPE_MASK ((__u64)0x3 << 54)
>> +
>> +#define fourcc_mod_vs_code(type, val) \
>> + fourcc_mod_code(VS, ((((__u64)type) << 54) | (val)))
>> +
>> +#define DRM_FORMAT_MOD_VS_DEC_TILE_MODE_MASK 0x3F
>> +#define DRM_FORMAT_MOD_VS_DEC_TILE_8X8_XMAJOR 0x00
>> +#define DRM_FORMAT_MOD_VS_DEC_TILE_8X8_YMAJOR 0x01
>> +#define DRM_FORMAT_MOD_VS_DEC_TILE_16X4 0x02
>> +#define DRM_FORMAT_MOD_VS_DEC_TILE_8X4 0x03
>> +#define DRM_FORMAT_MOD_VS_DEC_TILE_4X8 0x04
>> +#define DRM_FORMAT_MOD_VS_DEC_RASTER_16X4 0x06
>> +#define DRM_FORMAT_MOD_VS_DEC_TILE_64X4 0x07
>> +#define DRM_FORMAT_MOD_VS_DEC_TILE_32X4 0x08
>> +#define DRM_FORMAT_MOD_VS_DEC_RASTER_256X1 0x09
>> +#define DRM_FORMAT_MOD_VS_DEC_RASTER_128X1 0x0A
>> +#define DRM_FORMAT_MOD_VS_DEC_RASTER_64X4 0x0B
>> +#define DRM_FORMAT_MOD_VS_DEC_RASTER_256X2 0x0C
>> +#define DRM_FORMAT_MOD_VS_DEC_RASTER_128X2 0x0D
>> +#define DRM_FORMAT_MOD_VS_DEC_RASTER_128X4 0x0E
>> +#define DRM_FORMAT_MOD_VS_DEC_RASTER_64X1 0x0F
>> +#define DRM_FORMAT_MOD_VS_DEC_TILE_16X8 0x10
>> +#define DRM_FORMAT_MOD_VS_DEC_TILE_8X16 0x11
>> +#define DRM_FORMAT_MOD_VS_DEC_RASTER_512X1 0x12
>> +#define DRM_FORMAT_MOD_VS_DEC_RASTER_32X4 0x13
>> +#define DRM_FORMAT_MOD_VS_DEC_RASTER_64X2 0x14
>> +#define DRM_FORMAT_MOD_VS_DEC_RASTER_32X2 0x15
>> +#define DRM_FORMAT_MOD_VS_DEC_RASTER_32X1 0x16
>> +#define DRM_FORMAT_MOD_VS_DEC_RASTER_16X1 0x17
>> +#define DRM_FORMAT_MOD_VS_DEC_TILE_128X4 0x18
>> +#define DRM_FORMAT_MOD_VS_DEC_TILE_256X4 0x19
>> +#define DRM_FORMAT_MOD_VS_DEC_TILE_512X4 0x1A
>> +#define DRM_FORMAT_MOD_VS_DEC_TILE_16X16 0x1B
>> +#define DRM_FORMAT_MOD_VS_DEC_TILE_32X16 0x1C
>> +#define DRM_FORMAT_MOD_VS_DEC_TILE_64X16 0x1D
>> +#define DRM_FORMAT_MOD_VS_DEC_TILE_128X8 0x1E
>> +#define DRM_FORMAT_MOD_VS_DEC_TILE_8X4_S 0x1F
>> +#define DRM_FORMAT_MOD_VS_DEC_TILE_16X4_S 0x20
>> +#define DRM_FORMAT_MOD_VS_DEC_TILE_32X4_S 0x21
>> +#define DRM_FORMAT_MOD_VS_DEC_TILE_16X4_LSB 0x22
>> +#define DRM_FORMAT_MOD_VS_DEC_TILE_32X4_LSB 0x23
>> +#define DRM_FORMAT_MOD_VS_DEC_TILE_32X8 0x24
>> +
>> +#define DRM_FORMAT_MOD_VS_DEC_ALIGN_32 (0x01 << 6)
>> +#define DRM_FORMAT_MOD_VS_DEC_ALIGN_64 (0x01 << 7)
>> +
>> +#define fourcc_mod_vs_dec_code(tile, align) \
>> + fourcc_mod_vs_code(DRM_FORMAT_MOD_VS_TYPE_COMPRESSED, \
>> + ((tile) | (align)))
>> +
>> +#define DRM_FORMAT_MOD_VS_NORM_MODE_MASK 0x1F
>> +#define DRM_FORMAT_MOD_VS_LINEAR 0x00
>> +#define DRM_FORMAT_MOD_VS_TILED4x4 0x01
>> +#define DRM_FORMAT_MOD_VS_SUPER_TILED_XMAJOR 0x02
>> +#define DRM_FORMAT_MOD_VS_SUPER_TILED_YMAJOR 0x03
>> +#define DRM_FORMAT_MOD_VS_TILE_8X8 0x04
>> +#define DRM_FORMAT_MOD_VS_TILE_MODE1 0x05
>> +#define DRM_FORMAT_MOD_VS_TILE_MODE2 0x06
>> +#define DRM_FORMAT_MOD_VS_TILE_8X4 0x07
>> +#define DRM_FORMAT_MOD_VS_TILE_MODE4 0x08
>> +#define DRM_FORMAT_MOD_VS_TILE_MODE5 0x09
>> +#define DRM_FORMAT_MOD_VS_TILE_MODE6 0x0A
>> +#define DRM_FORMAT_MOD_VS_SUPER_TILED_XMAJOR_8X4 0x0B
>> +#define DRM_FORMAT_MOD_VS_SUPER_TILED_YMAJOR_4X8 0x0C
>> +#define DRM_FORMAT_MOD_VS_TILE_Y 0x0D
>> +#define DRM_FORMAT_MOD_VS_TILE_128X1 0x0F
>> +#define DRM_FORMAT_MOD_VS_TILE_256X1 0x10
>> +#define DRM_FORMAT_MOD_VS_TILE_32X1 0x11
>> +#define DRM_FORMAT_MOD_VS_TILE_64X1 0x12
>> +#define DRM_FORMAT_MOD_VS_TILE_MODE4X4 0x15
>> +
>> +#define fourcc_mod_vs_norm_code(tile) \
>> + fourcc_mod_vs_code(DRM_FORMAT_MOD_VS_TYPE_NORMAL, \
>> + (tile))
>> +
>> +#define fourcc_mod_vs_custom_code(tile) \
>> + fourcc_mod_vs_code(DRM_FORMAT_MOD_VS_TYPE_CUSTOM_10BIT, \
>> + (tile))
>> +
>
> You are opening a new namespace for what is effectively the VIVANTE
> tiling. While your list seems much more exhaustive than the (reverse
> engineered) list provided under the VIVANTE namespace, this is still
> unacceptable as it adds new aliases for existing modifiers.
>
hi Lucas:
I got what you mean , I will check the whether the current existence can be reused.
In principle, can existing modefiers cover my definition?
> Also any modifier additions should be in a separate patch and not
> buried in another change.
>
ok , no problem
> Regards,
> Lucas
On 2023/7/25 11:12, Keith Zhao wrote:
>
>
> On 2023/6/7 16:53, Lucas Stach wrote:
>> Hi Keith,
>>
>> Am Freitag, dem 02.06.2023 um 15:40 +0800 schrieb Keith Zhao:
>>> Add a basic platform driver of the DRM driver for JH7110 SoC.
>>>
>>> Signed-off-by: Keith Zhao <[email protected]>
>>> ---
>>> MAINTAINERS | 2 +
>>> drivers/gpu/drm/Kconfig | 2 +
>>> drivers/gpu/drm/Makefile | 1 +
>>> drivers/gpu/drm/verisilicon/Kconfig | 13 ++
>>> drivers/gpu/drm/verisilicon/Makefile | 6 +
>>> drivers/gpu/drm/verisilicon/vs_drv.c | 284 +++++++++++++++++++++++++++
>>> drivers/gpu/drm/verisilicon/vs_drv.h | 48 +++++
>>> include/uapi/drm/drm_fourcc.h | 83 ++++++++
>>> include/uapi/drm/vs_drm.h | 50 +++++
>>> 9 files changed, 489 insertions(+)
>>> create mode 100644 drivers/gpu/drm/verisilicon/Kconfig
>>> create mode 100644 drivers/gpu/drm/verisilicon/Makefile
>>> create mode 100644 drivers/gpu/drm/verisilicon/vs_drv.c
>>> create mode 100644 drivers/gpu/drm/verisilicon/vs_drv.h
>>> create mode 100644 include/uapi/drm/vs_drm.h
>>>
>>>
>>> [...]
>>> +#endif /* __VS_DRV_H__ */
>>> diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
>>> index de703c6be969..af4fb50f9207 100644
>>> --- a/include/uapi/drm/drm_fourcc.h
>>> +++ b/include/uapi/drm/drm_fourcc.h
>>> @@ -419,6 +419,7 @@ extern "C" {
>>> #define DRM_FORMAT_MOD_VENDOR_ARM 0x08
>>> #define DRM_FORMAT_MOD_VENDOR_ALLWINNER 0x09
>>> #define DRM_FORMAT_MOD_VENDOR_AMLOGIC 0x0a
>>> +#define DRM_FORMAT_MOD_VENDOR_VS 0x0b
>>>
>>> /* add more to the end as needed */
>>>
>>> @@ -1519,6 +1520,88 @@ drm_fourcc_canonicalize_nvidia_format_mod(__u64 modifier)
>>> #define AMD_FMT_MOD_CLEAR(field) \
>>> (~((__u64)AMD_FMT_MOD_##field##_MASK << AMD_FMT_MOD_##field##_SHIFT))
>>>
>>> +#define DRM_FORMAT_MOD_VS_TYPE_NORMAL 0x00
>>> +#define DRM_FORMAT_MOD_VS_TYPE_COMPRESSED 0x01
>>> +#define DRM_FORMAT_MOD_VS_TYPE_CUSTOM_10BIT 0x02
>>> +#define DRM_FORMAT_MOD_VS_TYPE_MASK ((__u64)0x3 << 54)
>>> +
>>> +#define fourcc_mod_vs_code(type, val) \
>>> + fourcc_mod_code(VS, ((((__u64)type) << 54) | (val)))
>>> +
>>> +#define DRM_FORMAT_MOD_VS_DEC_TILE_MODE_MASK 0x3F
>>> +#define DRM_FORMAT_MOD_VS_DEC_TILE_8X8_XMAJOR 0x00
>>> +#define DRM_FORMAT_MOD_VS_DEC_TILE_8X8_YMAJOR 0x01
>>> +#define DRM_FORMAT_MOD_VS_DEC_TILE_16X4 0x02
>>> +#define DRM_FORMAT_MOD_VS_DEC_TILE_8X4 0x03
>>> +#define DRM_FORMAT_MOD_VS_DEC_TILE_4X8 0x04
>>> +#define DRM_FORMAT_MOD_VS_DEC_RASTER_16X4 0x06
>>> +#define DRM_FORMAT_MOD_VS_DEC_TILE_64X4 0x07
>>> +#define DRM_FORMAT_MOD_VS_DEC_TILE_32X4 0x08
>>> +#define DRM_FORMAT_MOD_VS_DEC_RASTER_256X1 0x09
>>> +#define DRM_FORMAT_MOD_VS_DEC_RASTER_128X1 0x0A
>>> +#define DRM_FORMAT_MOD_VS_DEC_RASTER_64X4 0x0B
>>> +#define DRM_FORMAT_MOD_VS_DEC_RASTER_256X2 0x0C
>>> +#define DRM_FORMAT_MOD_VS_DEC_RASTER_128X2 0x0D
>>> +#define DRM_FORMAT_MOD_VS_DEC_RASTER_128X4 0x0E
>>> +#define DRM_FORMAT_MOD_VS_DEC_RASTER_64X1 0x0F
>>> +#define DRM_FORMAT_MOD_VS_DEC_TILE_16X8 0x10
>>> +#define DRM_FORMAT_MOD_VS_DEC_TILE_8X16 0x11
>>> +#define DRM_FORMAT_MOD_VS_DEC_RASTER_512X1 0x12
>>> +#define DRM_FORMAT_MOD_VS_DEC_RASTER_32X4 0x13
>>> +#define DRM_FORMAT_MOD_VS_DEC_RASTER_64X2 0x14
>>> +#define DRM_FORMAT_MOD_VS_DEC_RASTER_32X2 0x15
>>> +#define DRM_FORMAT_MOD_VS_DEC_RASTER_32X1 0x16
>>> +#define DRM_FORMAT_MOD_VS_DEC_RASTER_16X1 0x17
>>> +#define DRM_FORMAT_MOD_VS_DEC_TILE_128X4 0x18
>>> +#define DRM_FORMAT_MOD_VS_DEC_TILE_256X4 0x19
>>> +#define DRM_FORMAT_MOD_VS_DEC_TILE_512X4 0x1A
>>> +#define DRM_FORMAT_MOD_VS_DEC_TILE_16X16 0x1B
>>> +#define DRM_FORMAT_MOD_VS_DEC_TILE_32X16 0x1C
>>> +#define DRM_FORMAT_MOD_VS_DEC_TILE_64X16 0x1D
>>> +#define DRM_FORMAT_MOD_VS_DEC_TILE_128X8 0x1E
>>> +#define DRM_FORMAT_MOD_VS_DEC_TILE_8X4_S 0x1F
>>> +#define DRM_FORMAT_MOD_VS_DEC_TILE_16X4_S 0x20
>>> +#define DRM_FORMAT_MOD_VS_DEC_TILE_32X4_S 0x21
>>> +#define DRM_FORMAT_MOD_VS_DEC_TILE_16X4_LSB 0x22
>>> +#define DRM_FORMAT_MOD_VS_DEC_TILE_32X4_LSB 0x23
>>> +#define DRM_FORMAT_MOD_VS_DEC_TILE_32X8 0x24
>>> +
>>> +#define DRM_FORMAT_MOD_VS_DEC_ALIGN_32 (0x01 << 6)
>>> +#define DRM_FORMAT_MOD_VS_DEC_ALIGN_64 (0x01 << 7)
>>> +
>>> +#define fourcc_mod_vs_dec_code(tile, align) \
>>> + fourcc_mod_vs_code(DRM_FORMAT_MOD_VS_TYPE_COMPRESSED, \
>>> + ((tile) | (align)))
>>> +
>>> +#define DRM_FORMAT_MOD_VS_NORM_MODE_MASK 0x1F
>>> +#define DRM_FORMAT_MOD_VS_LINEAR 0x00
>>> +#define DRM_FORMAT_MOD_VS_TILED4x4 0x01
>>> +#define DRM_FORMAT_MOD_VS_SUPER_TILED_XMAJOR 0x02
>>> +#define DRM_FORMAT_MOD_VS_SUPER_TILED_YMAJOR 0x03
>>> +#define DRM_FORMAT_MOD_VS_TILE_8X8 0x04
>>> +#define DRM_FORMAT_MOD_VS_TILE_MODE1 0x05
>>> +#define DRM_FORMAT_MOD_VS_TILE_MODE2 0x06
>>> +#define DRM_FORMAT_MOD_VS_TILE_8X4 0x07
>>> +#define DRM_FORMAT_MOD_VS_TILE_MODE4 0x08
>>> +#define DRM_FORMAT_MOD_VS_TILE_MODE5 0x09
>>> +#define DRM_FORMAT_MOD_VS_TILE_MODE6 0x0A
>>> +#define DRM_FORMAT_MOD_VS_SUPER_TILED_XMAJOR_8X4 0x0B
>>> +#define DRM_FORMAT_MOD_VS_SUPER_TILED_YMAJOR_4X8 0x0C
>>> +#define DRM_FORMAT_MOD_VS_TILE_Y 0x0D
>>> +#define DRM_FORMAT_MOD_VS_TILE_128X1 0x0F
>>> +#define DRM_FORMAT_MOD_VS_TILE_256X1 0x10
>>> +#define DRM_FORMAT_MOD_VS_TILE_32X1 0x11
>>> +#define DRM_FORMAT_MOD_VS_TILE_64X1 0x12
>>> +#define DRM_FORMAT_MOD_VS_TILE_MODE4X4 0x15
>>> +
>>> +#define fourcc_mod_vs_norm_code(tile) \
>>> + fourcc_mod_vs_code(DRM_FORMAT_MOD_VS_TYPE_NORMAL, \
>>> + (tile))
>>> +
>>> +#define fourcc_mod_vs_custom_code(tile) \
>>> + fourcc_mod_vs_code(DRM_FORMAT_MOD_VS_TYPE_CUSTOM_10BIT, \
>>> + (tile))
>>> +
>>
>> You are opening a new namespace for what is effectively the VIVANTE
>> tiling. While your list seems much more exhaustive than the (reverse
>> engineered) list provided under the VIVANTE namespace, this is still
>> unacceptable as it adds new aliases for existing modifiers.
>>
> hi Lucas:
> I got what you mean , I will check the whether the current existence can be reused.
> In principle, can existing modefiers cover my definition?
>
hello Lucas:
I made it a little simpler: I removed what I didn't use in the code .
Keep only these:
#define DRM_FORMAT_MOD_VS_TYPE_NORMAL 0x00
#define DRM_FORMAT_MOD_VS_TYPE_COMPRESSED 0x01
#define DRM_FORMAT_MOD_VS_TYPE_CUSTOM_10BIT 0x02
#define DRM_FORMAT_MOD_VS_TYPE_MASK ((__u64)0x3 << 54)
#define fourcc_mod_vs_code(type, val) \
fourcc_mod_code(VS, ((((__u64)type) << 54) | (val)))
#define DRM_FORMAT_MOD_VS_NORM_MODE_MASK 0x1F
#define DRM_FORMAT_MOD_VS_LINEAR 0x00
#define DRM_FORMAT_MOD_VS_SUPER_TILED_XMAJOR 0x02
#define DRM_FORMAT_MOD_VS_SUPER_TILED_YMAJOR 0x03
#define DRM_FORMAT_MOD_VS_TILE_8X8 0x04
#define DRM_FORMAT_MOD_VS_TILE_8X4 0x07
#define DRM_FORMAT_MOD_VS_SUPER_TILED_XMAJOR_8X4 0x0B
#define DRM_FORMAT_MOD_VS_SUPER_TILED_YMAJOR_4X8 0x0C
#define DRM_FORMAT_MOD_VS_TILE_MODE4X4 0x15
#define fourcc_mod_vs_norm_code(tile) \
fourcc_mod_vs_code(DRM_FORMAT_MOD_VS_TYPE_NORMAL, \
(tile))
#define fourcc_mod_vs_custom_code(tile) \
fourcc_mod_vs_code(DRM_FORMAT_MOD_VS_TYPE_CUSTOM_10BIT, \
(tile))
I need you help to check what modifiers it should be (for existing modifiers)
DRM_FORMAT_MOD_VS_LINEAR ----> DRM_FORMAT_MOD_LINEAR
DRM_FORMAT_MOD_VS_SUPER_TILED_XMAJOR ----> ?
DRM_FORMAT_MOD_VS_SUPER_TILED_YMAJOR ----> ?
DRM_FORMAT_MOD_VS_TILE_8X8 ----> ?
DRM_FORMAT_MOD_VS_TILE_8X4 ----> ?
DRM_FORMAT_MOD_VS_SUPER_TILED_XMAJOR_8X4 ----> ?
DRM_FORMAT_MOD_VS_SUPER_TILED_YMAJOR_4X8 ----> ?
DRM_FORMAT_MOD_VS_TILE_MODE4X4 ----> DRM_FORMAT_MOD_VIVANTE_TILED
Thanks a million!!
Keith
>> Also any modifier additions should be in a separate patch and not
>> buried in another change.
>>
> ok , no problem
>> Regards,
>> Lucas