| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <a46szgmjrwgqhv5issuijyvih4tof3xa45tdhxv4qjplekszpz@55tgbkeby7zr>
Date: Tue, 23 Sep 2025 03:53:36 +0300
From: Dmitry Baryshkov <dmitry.baryshkov@....qualcomm.com>
To: Icenowy Zheng <uwu@...nowy.me>
Cc: Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
Maxime Ripard <mripard@...nel.org>,
Thomas Zimmermann <tzimmermann@...e.de>,
David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>,
Rob Herring <robh@...nel.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>,
Conor Dooley <conor+dt@...nel.org>, Drew Fustini <fustini@...nel.org>,
Guo Ren <guoren@...nel.org>, Fu Wei <wefu@...hat.com>,
Philipp Zabel <p.zabel@...gutronix.de>,
Heiko Stuebner <heiko@...ech.de>,
Andrzej Hajda <andrzej.hajda@...el.com>,
Neil Armstrong <neil.armstrong@...aro.org>,
Robert Foss <rfoss@...nel.org>,
Laurent Pinchart <Laurent.pinchart@...asonboard.com>,
Jonas Karlman <jonas@...boo.se>,
Jernej Skrabec <jernej.skrabec@...il.com>,
Michal Wilczynski <m.wilczynski@...sung.com>,
Han Gao <rabenda.cn@...il.com>, Yao Zi <ziyao@...root.org>,
dri-devel@...ts.freedesktop.org, devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-riscv@...ts.infradead.org
Subject: Re: [PATCH v2 3/8] drm: verisilicon: add a driver for Verisilicon
display controllers
On Sun, Sep 21, 2025 at 04:34:41PM +0800, Icenowy Zheng wrote:
> This is a from-scratch driver targeting Verisilicon DC-series display
> controllers, which feature self-identification functionality like their
> GC-series GPUs.
>
> Only DC8200 is being supported now, and only the main framebuffer is set
> up (as the DRM primary plane). Support for more DC models and more
> features is my further targets.
>
> As the display controller is delivered to SoC vendors as a whole part,
> this driver does not use component framework and extra bridges inside a
> SoC is expected to be implemented as dedicated bridges (this driver
> properly supports bridge chaining).
>
> Signed-off-by: Icenowy Zheng <uwu@...nowy.me>
> ---
> Changes in v2:
> - Changed some Control flows according to previous reviews.
> - Added missing of_node_put when checking of endpoints for output type.
> - Switched all userspace-visible modeset objects to be managed by drmm
> instead of devm.
> - Utilize devm_drm_bridge_alloc() in internal bridge.
> - Prevented the usage of simple encoder helpers by passing a NULL funcs pointer.
> - Let devm enable clocks when getting them.
> - Removed explicit `.cache_type = REGCACHE_NONE` in regmap config.
> - Fixed a debug print using a variable before initialization.
> - Fixed a wrong index when using bulk to handle resets.
> - Added missing configuration for DPI format (currently fixed RGB888).
>
> drivers/gpu/drm/Kconfig | 2 +
> drivers/gpu/drm/Makefile | 1 +
> drivers/gpu/drm/verisilicon/Kconfig | 15 +
> drivers/gpu/drm/verisilicon/Makefile | 5 +
> drivers/gpu/drm/verisilicon/vs_bridge.c | 330 ++++++++++++++++++
> drivers/gpu/drm/verisilicon/vs_bridge.h | 40 +++
> drivers/gpu/drm/verisilicon/vs_bridge_regs.h | 54 +++
> drivers/gpu/drm/verisilicon/vs_crtc.c | 217 ++++++++++++
> drivers/gpu/drm/verisilicon/vs_crtc.h | 29 ++
> drivers/gpu/drm/verisilicon/vs_crtc_regs.h | 60 ++++
> drivers/gpu/drm/verisilicon/vs_dc.c | 205 +++++++++++
> drivers/gpu/drm/verisilicon/vs_dc.h | 39 +++
> drivers/gpu/drm/verisilicon/vs_dc_top_regs.h | 27 ++
> drivers/gpu/drm/verisilicon/vs_drm.c | 177 ++++++++++
> drivers/gpu/drm/verisilicon/vs_drm.h | 29 ++
> drivers/gpu/drm/verisilicon/vs_hwdb.c | 150 ++++++++
> drivers/gpu/drm/verisilicon/vs_hwdb.h | 29 ++
> drivers/gpu/drm/verisilicon/vs_plane.c | 102 ++++++
> drivers/gpu/drm/verisilicon/vs_plane.h | 68 ++++
> .../gpu/drm/verisilicon/vs_primary_plane.c | 157 +++++++++
> .../drm/verisilicon/vs_primary_plane_regs.h | 53 +++
> 21 files changed, 1789 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_bridge.c
> create mode 100644 drivers/gpu/drm/verisilicon/vs_bridge.h
> create mode 100644 drivers/gpu/drm/verisilicon/vs_bridge_regs.h
> create mode 100644 drivers/gpu/drm/verisilicon/vs_crtc.c
> create mode 100644 drivers/gpu/drm/verisilicon/vs_crtc.h
> create mode 100644 drivers/gpu/drm/verisilicon/vs_crtc_regs.h
> create mode 100644 drivers/gpu/drm/verisilicon/vs_dc.c
> create mode 100644 drivers/gpu/drm/verisilicon/vs_dc.h
> create mode 100644 drivers/gpu/drm/verisilicon/vs_dc_top_regs.h
> create mode 100644 drivers/gpu/drm/verisilicon/vs_drm.c
> create mode 100644 drivers/gpu/drm/verisilicon/vs_drm.h
> create mode 100644 drivers/gpu/drm/verisilicon/vs_hwdb.c
> create mode 100644 drivers/gpu/drm/verisilicon/vs_hwdb.h
> create mode 100644 drivers/gpu/drm/verisilicon/vs_plane.c
> create mode 100644 drivers/gpu/drm/verisilicon/vs_plane.h
> create mode 100644 drivers/gpu/drm/verisilicon/vs_primary_plane.c
> create mode 100644 drivers/gpu/drm/verisilicon/vs_primary_plane_regs.h
>
> +
> +static int vs_bridge_atomic_check(struct drm_bridge *bridge,
> + struct drm_bridge_state *bridge_state,
> + struct drm_crtc_state *crtc_state,
> + struct drm_connector_state *conn_state)
> +{
> + struct vs_bridge *vbridge = drm_bridge_to_vs_bridge(bridge);
> +
> + if (vbridge->intf == VSDC_OUTPUT_INTERFACE_DP &&
> + !vs_bridge_out_dp_fmt_supported(bridge_state->output_bus_cfg.format))
> + return -EINVAL;
I still think that it's better to have per-interface type bridge funcs
rather than checking for the interface type inside the function.
> +
> + vbridge->output_bus_fmt = bridge_state->output_bus_cfg.format;
> +
> + return 0;
> +}
> +
> +
> + bridge = devm_drm_bridge_alloc(drm_dev->dev, struct vs_bridge, base,
> + &vs_bridge_funcs);
> + if (!bridge)
> + return ERR_PTR(-ENOMEM);
> +
> + bridge->crtc = crtc;
> + bridge->intf = intf;
> + bridge->next = next;
> +
> + if (intf == VSDC_OUTPUT_INTERFACE_DPI)
> + enctype = DRM_MODE_ENCODER_DPI;
> + else
> + enctype = DRM_MODE_ENCODER_NONE;
Nit: DRM_MODE_ENCODER_TMDS ?
> +
> + bridge->enc = drmm_plain_encoder_alloc(drm_dev, NULL, enctype, NULL);
> + if (IS_ERR(bridge->enc)) {
> + dev_err(drm_dev->dev,
> + "Cannot initialize encoder for output %u\n", output);
> + ret = PTR_ERR(bridge->enc);
> + return ERR_PTR(ret);
> + }
> +
> + bridge->enc->possible_crtcs = drm_crtc_mask(&crtc->base);
> +
> + ret = drm_bridge_attach(bridge->enc, &bridge->base, NULL,
> + DRM_BRIDGE_ATTACH_NO_CONNECTOR);
> + if (ret) {
> + dev_err(drm_dev->dev,
> + "Cannot attach bridge for output %u\n", output);
> + return ERR_PTR(ret);
> + }
> +
> + bridge->conn = drm_bridge_connector_init(drm_dev, bridge->enc);
> + if (IS_ERR(bridge->conn)) {
> + dev_err(drm_dev->dev,
> + "Cannot create connector for output %u\n", output);
> + ret = PTR_ERR(bridge->conn);
> + return ERR_PTR(ret);
> + }
> + drm_connector_attach_encoder(bridge->conn, bridge->enc);
> +
> + return bridge;
> +}
Other than that LGTM.
--
With best wishes
Dmitry
Powered by blists - more mailing lists