Re: [PATCH v7 7/7] drm/verisilicon: fix DC8200 primary plane disable clearing FB_EN

在 2026-09-29二的 09:45 +0800，Joey Lu写道：

Icenowy Zheng 於 2026/9/26 下午 12:48 寫道:

quoted

在 2026-09-25五的 22:55 +0800，Icenowy Zheng写道：

quoted

在 2026-09-21一的 15:49 +0800，Joey Lu写道：

quoted

Icenowy Zheng 於 2026/9/21 下午 03:30 寫道:

quoted

Maybe it's better to just make it the 2nd patch in this
patchset,
just
after the binding change.

Waiting for something into drm-misc-fixes again will need
another
fixes
pull and another RC back merge, which can consume weeks and
miss
the
current merging window.

In addition, it's possible that `drm/verisilicon: introduce
per-
variant
hardware ops table` also gets backported for a more clean
primary
plane
atomic_update disabling fix.

Understood. I'll fold the FB_EN fix in as patch 2, right after
the

Should I just apply this revision of patch with this reorder?

Oh I think more fixes are coming.

There's one more DC8200-specific problem (which might be
problematic on
DC8000 but I am not sure): The vs_dc8200_panel_disable_ex sequence
must
be executed before vs_dc8200_panel_enable_ex, because some timing
parameters seem to be latched when the value going from 0 to 1, and
when a stale state is programmed before the driver is loaded (e.g.
a
firmware driving the display) and the timing isn't the same with
the
timing of the first modeset, the modeset will fail.

I'm thinking about sending a fix patchset first, which will pick
the
FB_EN fix here, implement a fix for the stale state problem and
maybe
implement fixes for the old state dereference problems for hidden
planes in their atomic_update callback (although this fix will
introduce a helper, which will then be moved and renamed in the
DC8200
ops patch).

Thanks,
Icenowy

Understood, happy to hold the DCUltraLite series until your fix
patchset lands, and rebase the ops-table patch on top of it then.

On DC8000: its panel enable/disable path just toggles a plain
FB_CONFIG_RESET bit, not the staged PANEL_CONFIG/PANEL_START latch
registers DC8200 uses, so it doesn't look like it shares this
specific
ordering hazard from the register layout alone. I'll run the tests
again 
after
applying our variant on top of it.

Ah it's not race -- it's that some timing configuration is latched when
it starts, and the programming won't take effect if the panel isn't
shut down once. 

This only becomes a problem when the firmware also powers on the
display, and sets a different timing. On real DPI panels this problem
seems to be not happening because they cannot adapt to different
timings (but monitors can sync to different timings).

Thanks,
Icenowy

quoted

quoted

Thanks,
Icenowy

quoted

dt-bindings patch, targeting the current vs_primary_plane.c
directly,
so the ops-table patch just carries the already-corrected code
forward
into vs_dc8200.c. Agreed that's faster than round-tripping.

On primary_plane_disable_ex - I'll keep the "_ex" suffix.
DC8200
still
has a real per-variant operation there (clearing FB_EN +
commit),
while
DC8000 leaves it NULL and does nothing, so the suffix still
reflects
an actual per-variant difference, not just structure introduced
by
the
refactor.

While testing the cursor plane, I found the same class of bug
there:
the disable register write was being triggered from the plane's
atomic_update() invisible-branch using the old plane state to
look
up
the CRTC/output, which can be stale or NULL on a plane's very
first
commit if it's already invisible then. I've fixed it locally by
having
atomic_update() derive the CRTC/output from the state it
already
holds
before checking visibility, instead of falling through to the
old-state-based disable path. Just flagging it here for now -
vs_cursor_plane.c isn't touched by any of my patches.

Thanks.