| 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: <0eb4b6ba8ef3b47cd28eb8d652b05eb5a6d1409c.camel@icenowy.me>
Date: Thu, 22 Jan 2026 17:23:15 +0800
From: Icenowy Zheng <uwu@...nowy.me>
To: Thomas Zimmermann <tzimmermann@...e.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>, Maarten Lankhorst
<maarten.lankhorst@...ux.intel.com>, Maxime Ripard <mripard@...nel.org>,
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>
Cc: Philipp Zabel <p.zabel@...gutronix.de>, Dmitry Baryshkov
<lumag@...nel.org>, Michal Wilczynski <m.wilczynski@...sung.com>, Luca
Ceresoli <luca.ceresoli@...tlin.com>, Han Gao <rabenda.cn@...il.com>, Yao
Zi <ziyao@...root.org>, linux-kernel@...r.kernel.org,
dri-devel@...ts.freedesktop.org, devicetree@...r.kernel.org,
linux-riscv@...ts.infradead.org, Han Gao <gaohan@...as.ac.cn>
Subject: Re: [PATCH v5 3/9] drm: verisilicon: add a driver for Verisilicon
display controllers
在 2026-01-21星期三的 13:56 +0100，Thomas Zimmermann写道：
=============== 8< ===================
> > +
> > + dev_info(dev, "DC%x rev %x customer %x\n", dc-
> > >identity.model,
> > + dc->identity.revision, dc->identity.customer_id);
>
> drm_dbg(). Drivers should be quite by default.
Well for this kind of identification information I think driver is
usually not quiet, at least amdgpu (when doing IP discovery), panfrost
and etnaviv (which shares the same set of identification registers with
this driver) is reporting it.
>
> > +
> > + if (port_count > dc->identity.display_count) {
> > + dev_err(dev, "too many downstream ports than HW
> > capability\n");
> > + ret = -EINVAL;
> > + goto err_rst_assert;
> > + }
> > +
=============== 8< ===================
> > +
> > + if (!state->visible || !fb) {
> > + regmap_write(dc->regs, VSDC_FB_CONFIG(output), 0);
> > + regmap_write(dc->regs, VSDC_FB_CONFIG_EX(output),
> > 0);
> > + goto commit;
> > + } else {
> > + regmap_set_bits(dc->regs,
> > VSDC_FB_CONFIG_EX(output),
> > + VSDC_FB_CONFIG_EX_FB_EN);
> > + }
>
> Since you handle the plane on/plane off cases here, I'd advise to
> implement atomic_enable and atomic_disable for the plane, if the
> hardware allows it. (There is hardware were the current pattern makes
> sense though.)
If atomic_*able is going to be implemented, should atomic_update() keep
the plane on/off code?
BTW it seems that DC8200 has the shadow register capability that could
be manually commited but the older DC8000 has no (the
VSDC_FB_CONFIG_EX_COMMIT bit written below is a new thing) -- the
VSDC_FB_CONFIG register on DC8000 has a write-only bit that when
written with 0 the display is put to reset and when written with 1 the
display will start to run. This pattern seems to be not able to keep
the enabled state while programming (at least part of) new plane
configuration, see [1].
[1]
https://github.com/milkv-megrez/rockos-u-boot/blob/c9221cf2fa77d39c0b241ab4b030c708e7ebe279/drivers/video/eswin/eswin_dc_reg.h#L3579
>
> > +
> > + drm_format_to_vs_format(state->fb->format->format, &fmt);
> > +
> > + regmap_update_bits(dc->regs, VSDC_FB_CONFIG(output),
> > + VSDC_FB_CONFIG_FMT_MASK,
> > + VSDC_FB_CONFIG_FMT(fmt.color));
> > + regmap_update_bits(dc->regs, VSDC_FB_CONFIG(output),
> > + VSDC_FB_CONFIG_SWIZZLE_MASK,
> > + VSDC_FB_CONFIG_SWIZZLE(fmt.swizzle));
> > + regmap_assign_bits(dc->regs, VSDC_FB_CONFIG(output),
> > + VSDC_FB_CONFIG_UV_SWIZZLE_EN,
> > fmt.uv_swizzle);
> > +
> > + /* Get the physical address of the buffer in memory */
> > + gem = drm_fb_dma_get_gem_obj(fb, 0);
> > +
> > + /* Compute the start of the displayed memory */
> > + bpp = fb->format->cpp[0];
> > + dma_addr = gem->dma_addr + fb->offsets[0];
> > +
> > + /* Fixup framebuffer address for src coordinates */
> > + dma_addr += (state->src.x1 >> 16) * bpp;
>
> bpp is deprecated and should be avoided in new code. You can compute
> the
> offset with drm_format_min_pitch():
>
> drm_format_min_pitch(fb->format, 0, state->src.x1 >> 16 )
>
> Best regards
> Thomas
>
> > + dma_addr += (state->src.y1 >> 16) * fb->pitches[0];
> > +
> > + regmap_write(dc->regs, VSDC_FB_ADDRESS(output),
> > + lower_32_bits(dma_addr));
> > + regmap_write(dc->regs, VSDC_FB_STRIDE(output),
> > + fb->pitches[0]);
> > +
> > + regmap_write(dc->regs, VSDC_FB_TOP_LEFT(output),
> > + VSDC_MAKE_PLANE_POS(state->crtc_x, state-
> > >crtc_y));
> > + regmap_write(dc->regs, VSDC_FB_BOTTOM_RIGHT(output),
> > + VSDC_MAKE_PLANE_POS(state->crtc_x + state-
> > >crtc_w,
> > + state->crtc_y + state-
> > >crtc_h));
> > + regmap_write(dc->regs, VSDC_FB_SIZE(output),
> > + VSDC_MAKE_PLANE_SIZE(state->crtc_w, state-
> > >crtc_h));
> > +
> > + regmap_write(dc->regs, VSDC_FB_BLEND_CONFIG(output),
> > + VSDC_FB_BLEND_CONFIG_BLEND_DISABLE);
> > +commit:
> > + regmap_set_bits(dc->regs, VSDC_FB_CONFIG_EX(output),
> > + VSDC_FB_CONFIG_EX_COMMIT);
> > +}
> > +
=============== 8< ===================
Powered by blists - more mailing lists