Re: [v1,v1,5/7] drm/vs: Register DRM device

2023-08-01 Thread suijingfeng 

Hi,

On 2023/8/1 21:40, suijingfeng wrote:

+#define DRV_DATE    "202305161"


The date is not correct here, generally it should have 8 numbers,

while you have 9 digits, why you are so special ? 



I'm also interesting in RISCV arch, I got attracted by your patch.

I just want to join into the discussion at here (at my spare time),

So when you see my comments, I hoping that you will not interpret it as 
hostility.


Welcome contributing. :-)

Re: [v1,v1,5/7] drm/vs: Register DRM device

2023-08-01 Thread suijingfeng 

Hi,

On 2023/8/1 21:40, suijingfeng wrote:

+if DRM_VERISILICON
+
+config STARFIVE_HDMI
+    bool "Starfive specific extensions HDMI"
+    help
+   This selects support for StarFive SoC specific extensions
+   for the Innosilicon HDMI driver. If you want to enable
+   HDMI on JH7110 based SoC, you should select this option.
+
+   To compile this driver as a module, choose M here.
+endif


Why not use 


 Why not use the 'depends on DRM_VERISILICON'  here ?


```

config STARFIVE_HDMI

    depends on DRM_VERISILICON
    bool "Starfive specific extensions HDMI"

    help

```


I see the Kconfig of VC4 using the 'depends on', and most  driver using 
the 'depends on'

Re: [v1,v1,5/7] drm/vs: Register DRM device

2023-08-01 Thread suijingfeng 

Hi,

On 2023/8/1 21:40, suijingfeng wrote:
So, you patch will be pass the compile test, I guess. 


You patch will *NOT* pass the compile test, I guess.

Re: [v1,v1,5/7] drm/vs: Register DRM device

2023-08-01 Thread suijingfeng 

Hi,


On 2023/8/1 18:10, Keith Zhao wrote:

Implement drm device registration interface

Signed-off-by: Keith Zhao 
---
  drivers/gpu/drm/Kconfig  |   2 +
  drivers/gpu/drm/Makefile |   1 +
  drivers/gpu/drm/verisilicon/Kconfig  |  25 ++
  drivers/gpu/drm/verisilicon/Makefile |  13 +
  drivers/gpu/drm/verisilicon/vs_drv.c | 273 +
  drivers/gpu/drm/verisilicon/vs_drv.h |  54 
  drivers/gpu/drm/verisilicon/vs_gem.c | 298 +++
  drivers/gpu/drm/verisilicon/vs_gem.h |  50 
  drivers/gpu/drm/verisilicon/vs_modeset.c |  92 +++
  drivers/gpu/drm/verisilicon/vs_modeset.h |  13 +
  include/uapi/drm/vs_drm.h|  50 
  11 files changed, 871 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 drivers/gpu/drm/verisilicon/vs_gem.c
  create mode 100644 drivers/gpu/drm/verisilicon/vs_gem.h
  create mode 100644 drivers/gpu/drm/verisilicon/vs_modeset.c
  create mode 100644 drivers/gpu/drm/verisilicon/vs_modeset.h
  create mode 100644 include/uapi/drm/vs_drm.h

diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index afb3b2f5f..9ede80ef9 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -363,6 +363,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 7a09a89b4..6db3bc397 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 0..fcc39dded
--- /dev/null
+++ b/drivers/gpu/drm/verisilicon/Kconfig
@@ -0,0 +1,25 @@
+# SPDX-License-Identifier: GPL-2.0
+config DRM_VERISILICON
+   tristate "DRM Support for VeriSilicon"
+   depends on DRM
+   select DRM_KMS_HELPER
+   select DRM_GEM_DMA_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.
+
+if DRM_VERISILICON
+
+config STARFIVE_HDMI
+   bool "Starfive specific extensions HDMI"
+   help
+  This selects support for StarFive SoC specific extensions
+  for the Innosilicon HDMI driver. If you want to enable
+  HDMI on JH7110 based SoC, you should select this option.
+
+  To compile this driver as a module, choose M here.
+endif


Why not use



diff --git a/drivers/gpu/drm/verisilicon/Makefile 
b/drivers/gpu/drm/verisilicon/Makefile
new file mode 100644
index 0..26cc7e21b
--- /dev/null
+++ b/drivers/gpu/drm/verisilicon/Makefile
@@ -0,0 +1,13 @@
+# SPDX-License-Identifier: GPL-2.0
+
+vs_drm-objs := vs_dc_hw.o \
+   vs_dc.o \
+   vs_crtc.o \
+   vs_drv.o \
+   vs_modeset.o \
+   vs_gem.o \
+   vs_plane.o
+
+vs_drm-$(CONFIG_STARFIVE_HDMI) += starfive_hdmi.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 0..69591e640
--- /dev/null
+++ b/drivers/gpu/drm/verisilicon/vs_drv.c
@@ -0,0 +1,273 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2023 VeriSilicon Holdings Co., Ltd.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "vs_drv.h"
+#include "vs_modeset.h"
+#include "vs_gem.h"
+
+#define DRV_NAME   "starfive"
+#define DRV_DESC   "Starfive DRM driver"
+#define DRV_DATE   "202305161"


The date is not correct here, generally it should have 8 numbers,

while you have 9 digits, why you are so special ?



+#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_com