Re: 回复: [PATCH v1] drm/verisilicon: add StarFive PCIe-attached DC8000 driver
From: Icenowy Zheng
Date: Tue Nov 10 2026 - 05:38:46 EST
在 2026-11-10二的 10:04 +0000,Shanlong Li写道:
>
>
> > -----邮件原件-----
> > 发件人: Icenowy Zheng <zhengxingda@xxxxxxxxxxx>
> > 发送时间: 2026年10月30日 15:11
> > 收件人: Shanlong Li <shanlong.li@xxxxxxxxxxxxxxxx>; Maarten Lankhorst
> > <maarten.lankhorst@xxxxxxxxxxxxxxx>; Maxime Ripard
> > <mripard@xxxxxxxxxx>;
> > Thomas Zimmermann <tzimmermann@xxxxxxx>
> > 抄送: David Airlie <airlied@xxxxxxxxx>; Simona Vetter
> > <simona@xxxxxxxx>;
> > linux-kernel@xxxxxxxxxxxxxxx; Mason Huo
> > <mason.huo@xxxxxxxxxxxxxxxx>;
> > Changhuang Liang <changhuang.liang@xxxxxxxxxxxxxxxx>
> > 主题: Re: [PATCH v1] drm/verisilicon: add StarFive PCIe-attached
> > DC8000 driver
> >
> > 在 2026-10-30五的 14:15 +0800,shanlong.li写道:
> > > From: Shanlong Li <shanlong.li@xxxxxxxxxxxxxxxx>
> > >
> > > StarFive BMC SoCs contain a Verisilicon DC8000 display controller
> >
> > So you really downgraded from DC8200 to DC8000 in a new SoC?
> Yes. This is a BMC SoC it only needs one output for the host console
> / remote
> KVM and a single-pipeline DC8000 was chosen instead of the DC8200.
> >
> > 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).
> Thanks for the pointer, I was not aware of that series. So I need
> rebase
> on top of it instead of carrying my own DC8000 register code?
> If that's the case, where can I view his current submission? I will
> adapt my DC driver based on his modifications.
> BTW i need to wait until his upstream work is completed before I can
> submit my next version?
> >
> > > 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.
> >
> Yes, I think it fits, the OF graph is only how vs_bridge.c discovers
> its next bridge.
> On the PCI device the topology is fixed, so the PCI front end can
> hand vs_bridge_init() the
> downstream bridge directly.
> Our case is simpler than the drm/loongson one you describe: there is
> no
> external I2C transmitter to discover, the firmware behind the mailbox
> is
> the only "bridge", so no VBIOS or I2C enumeration is needed.
BTW I wonder how the "virtual connector" thing coexist with a real
display connector.
What output interface is configured by this driver? DPI/DP (DP is a bus
type called by Verisilicon, not related to DisplayPort) or a writeback
to memory?
The VCE thing looks quite confusing, it seems to be only sending the
main plane instead of fully composited display content (with multiple
planes)? Have you checked that hardware overlay / cursor is working
with it?
In addition, I wonder whether the connector should be advertised as a
DisplayPort one instead of a Virtual one. On my PC with AST2500, the
display connector is advertised as a VGA one (even when I didn't plug
in any VGA cable because I added an extra graphics card for desktop
usage).
>
> > 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@xxxxxxxxxxxxxxxx>
> > > ---
> > > 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.
> Yes, it derived from the vendor code, May I ask which specific "magic
> number" are,
> Is it something like the " horKernel[]" ?
Nearly every register field isn't documented in the vendor code. Things
like horKernel isn't so bad, but the register fields are bad.
And even if this copy of code is properly written, it's still code
duplication unwanted by the mainline Linux.
Thanks,
Icenowy
> I will make adjustments based on joye hu's revisions. Maybe Try to
> provide an explanation,
> Or drop it
> >
> > 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
>
> Thanks,
> Shanlong.li