drm/vc4: Add 2712 support to vc4_plane_async_set_fb

[6by9]

vc4_plane_async_set_fb directly overwrites the plane address in the dlist entry, but hadn't been updated for the GEN6 / 2712 dlist format, corrupting the address in the process.

Add support for the 2712 dlist format to the function.

Fixes: 1ab1fbb ("drm/vc4: hvs: Support BCM2712 HVS")

[pelwell]

It's surprising there isn't a helper function to update hw_dlist and the shadow dlist.

[6by9]

This it the only place that the dlist gets mangled in this way.

The fragments for each plane get created in vc[46]_plane_set_mode.
vc4_hvs_atomic_flush then assembles the fragments into a complete list in the dlist memory.

This async update is the only place that does both in one hit.

Reviewing the code it feels a little fragile based on a number of major assumptions.
AIUI it only works on the primary plane, which would normally be the bottom-most but that isn't guaranteed. vc4_state->ptr0_offset[0] is the offset within the fragment, so it only works as long as it is the bottom-most plane and hence first in the dlist.

[pelwell]

There's also this:

linux/drivers/gpu/drm/vc4/vc4_plane.c

Lines 2170 to 2186 in e2020fe

	vc4_state->dlist[vc4_state->pos0_offset] =
	new_vc4_state->dlist[vc4_state->pos0_offset];
	vc4_state->dlist[vc4_state->pos2_offset] =
	new_vc4_state->dlist[vc4_state->pos2_offset];
	vc4_state->dlist[vc4_state->ptr0_offset[0]] =
	new_vc4_state->dlist[vc4_state->ptr0_offset[0]];
	/* Note that we can't just call vc4_plane_write_dlist()
	* because that would smash the context data that the HVS is
	* currently using.
	*/
	writel(vc4_state->dlist[vc4_state->pos0_offset],
	&vc4_state->hw_dlist[vc4_state->pos0_offset]);
	writel(vc4_state->dlist[vc4_state->pos2_offset],
	&vc4_state->hw_dlist[vc4_state->pos2_offset]);
	writel(vc4_state->dlist[vc4_state->ptr0_offset[0]],
	&vc4_state->hw_dlist[vc4_state->ptr0_offset[0]]);

[6by9]

As amending that one needs to be a separate patch that is ideally applied before any 2712 stuff starts to be merged, can I take it as a task to be dealt with in the mainline patches?

[pelwell]

Of course.

[bluestang2006]

@6by9 I've tested your branch recently and it does fix the issue at hand. I've added some more info to the issue report...the corruption happens when vsync is disabled only.

[6by9]

Had missed that I hadn't updated this after the review comments.
Done now to address popcornmix's comment about upper address bit masking. pelwell's comment on helper function will be left to deal with when upstreaming.

[6by9]

Are we planning on another 6.1 release, and hence does this warrant a backport to 6.1?

[pelwell]

Yes we are, and yes it does. We need to bump the 6.1 stable so that it gets feature parity with the upcoming 6.6 stable.

[6by9]

Yes we are, and yes it does. We need to bump the 6.1 stable so that it gets feature parity with the upcoming 6.6 stable.

OK, I'll create a backport PR.

Checking the email thread with Igalia, there's a 2nd cleanup required in the async path too, but less critical.