Re: [PATCH v1] drm/verisilicon: add StarFive PCIe-attached DC8000 driver

[Icenowy Zheng <zhengxingda@iscas.ac.cn>]

在 2026-10-30五的 14:15 +0800，shanlong.li写道：
> From: Shanlong Li <shanlong.li@starfivetech.com>
> 
> StarFive BMC SoCs contain a Verisilicon DC8000 display controller

So you really downgraded from DC8200 to DC8000 in a new SoC?

BTW DC8000 support is being done by Joey Lu, in the form of MA35D1
DCUltraLite support (it has the same register layout and basic
functionality as the DC8000 controller I seen on Eswin EIC7700).

> which
> is exposed to the host over PCIe, much like the VGA of ASPEED BMCs.
> The
> host owns the display pipeline (primary, overlay and cursor planes,
> CRTC, gamma), scans out of the PCIe memory BAR of the device and
> relays
> EDID, hotplug, mode and pixel clock requests to the BMC firmware
> through
> a mailbox located in the CSR BAR.
> 
> The DC8000 and the two-pipeline DC8200 served by the existing
> verisilicon-dc platform driver share a common register layout, but
> the
> PCIe variant needs a different GEM backend (VRAM/TTM instead of GEM

Adding a PCIe glue for dedicated IP cores is always a challenge in
drivers/gpu/drm.

It might be interesting to thinking of different practices, although
creating yet another DRM driver with another GEM backend seems to be a
choice.

> DMA), a BMC-backed virtual connector instead of OF graph bridges and

I wonder whether DRM bridge facility could be used inside a PCI device
-- one of the DRM drivers with me as a reviewer (drm/loongson) suffers
from a similar (but more severe) problem. On that device, the DRM
driver should enumerate external I2C-controller bridges via its DDC bus
from the information of their proprietary VBIOS.

However, even in a worst situation, it would be just a new vs_bridge.c
variant combining the output programming logic of the DC and on-chip
display bridges into one source file.

> a
> different plane set with scaling, blending, gamma and de-gamma. Keep
> the
> two front ends separate: the SoC platform driver stays where it is
> and
> the PCIe driver goes into a new pci sub-directory. Each is
> selectable through its own Kconfig symbol, so a kernel can be built
> for
> either or both platforms, and the two drivers do not share any global
> symbol so they can also both be built in.
> 
> Signed-off-by: Shanlong Li <shanlong.li@starfivetech.com>
> ---
>  MAINTAINERS                                  |    7 +
>  drivers/gpu/drm/Makefile                     |    1 +
>  drivers/gpu/drm/verisilicon/Kconfig          |    2 +
>  drivers/gpu/drm/verisilicon/Makefile         |    2 +
>  drivers/gpu/drm/verisilicon/pci/Kconfig      |   30 +
>  drivers/gpu/drm/verisilicon/pci/Makefile     |    6 +
>  drivers/gpu/drm/verisilicon/pci/vs_crtc.c    |  393 +++++
>  drivers/gpu/drm/verisilicon/pci/vs_crtc.h    |   65 +
>  drivers/gpu/drm/verisilicon/pci/vs_dc.c      |  895 +++++++++++
>  drivers/gpu/drm/verisilicon/pci/vs_dc.h      |  138 ++
>  drivers/gpu/drm/verisilicon/pci/vs_dc_hw.c   | 1502
> ++++++++++++++++++
>  drivers/gpu/drm/verisilicon/pci/vs_dc_hw.h   |  426 +++++
>  drivers/gpu/drm/verisilicon/pci/vs_drv.c     |  264 +++
>  drivers/gpu/drm/verisilicon/pci/vs_drv.h     |   79 +
>  drivers/gpu/drm/verisilicon/pci/vs_plane.c   |  379 +++++
>  drivers/gpu/drm/verisilicon/pci/vs_plane.h   |   62 +
>  drivers/gpu/drm/verisilicon/pci/vs_type.h    |   70 +
>  drivers/gpu/drm/verisilicon/pci/vs_virtual.c |  531 +++++++
>  drivers/gpu/drm/verisilicon/pci/vs_virtual.h |   47 +
>  19 files changed, 4899 insertions(+)
>  create mode 100644 drivers/gpu/drm/verisilicon/pci/Kconfig
>  create mode 100644 drivers/gpu/drm/verisilicon/pci/Makefile
>  create mode 100644 drivers/gpu/drm/verisilicon/pci/vs_crtc.c
>  create mode 100644 drivers/gpu/drm/verisilicon/pci/vs_crtc.h
>  create mode 100644 drivers/gpu/drm/verisilicon/pci/vs_dc.c
>  create mode 100644 drivers/gpu/drm/verisilicon/pci/vs_dc.h
>  create mode 100644 drivers/gpu/drm/verisilicon/pci/vs_dc_hw.c
>  create mode 100644 drivers/gpu/drm/verisilicon/pci/vs_dc_hw.h

Anyway, it looks that you're just adding a dedicated driver without
reusing the current code, and your driver even contains the badly
written, magic number flooded code from Verisilicon.

Don't forget that your former workmate, Keith Zhao, worked for months
on this bad driver and still failed to get it upstreamed.

NAK on this.

Thanks,
Icenowy

>  create mode 100644 drivers/gpu/drm/verisilicon/pci/vs_drv.c
>  create mode 100644 drivers/gpu/drm/verisilicon/pci/vs_drv.h
>  create mode 100644 drivers/gpu/drm/verisilicon/pci/vs_plane.c
>  create mode 100644 drivers/gpu/drm/verisilicon/pci/vs_plane.h
>  create mode 100644 drivers/gpu/drm/verisilicon/pci/vs_type.h
>  create mode 100644 drivers/gpu/drm/verisilicon/pci/vs_virtual.c
>  create mode 100644 drivers/gpu/drm/verisilicon/pci/vs_virtual.h