Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

drm/vc4: Add 2712 support to vc4_plane_async_set_fb #5827

Merged
merged 1 commit into from
Jan 17, 2024
Merged

drm/vc4: Add 2712 support to vc4_plane_async_set_fb #5827

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 33 additions & 13 deletions drivers/gpu/drm/vc4/vc4_plane.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1877,7 +1877,7 @@ static int vc6_plane_mode_set(struct drm_plane *plane,
* The UPM buffer will be allocated in
* vc6_plane_allocate_upm().
*/
VC4_SET_FIELD(upper_32_bits(paddr) & 0xf,
VC4_SET_FIELD(upper_32_bits(paddr) & 0xff,
SCALER6_PTR0_UPPER_ADDR));

/* Pointer Word 1 */
Expand Down Expand Up @@ -2079,7 +2079,8 @@ void vc4_plane_async_set_fb(struct drm_plane *plane, struct drm_framebuffer *fb)
{
struct vc4_plane_state *vc4_state = to_vc4_plane_state(plane->state);
struct drm_gem_dma_object *bo = drm_fb_dma_get_gem_obj(fb, 0);
uint32_t addr;
struct vc4_dev *vc4 = to_vc4_dev(plane->dev);
dma_addr_t dma_addr = bo->dma_addr + fb->offsets[0];
int idx;

if (!drm_dev_enter(plane->dev, &idx))
Expand All @@ -2089,19 +2090,38 @@ void vc4_plane_async_set_fb(struct drm_plane *plane, struct drm_framebuffer *fb)
* because this is only called on the primary plane.
*/
WARN_ON_ONCE(plane->state->crtc_x < 0 || plane->state->crtc_y < 0);
addr = bo->dma_addr + fb->offsets[0];

/* Write the new address into the hardware immediately. The
* scanout will start from this address as soon as the FIFO
* needs to refill with pixels.
*/
writel(addr, &vc4_state->hw_dlist[vc4_state->ptr0_offset[0]]);
if (vc4->gen == VC4_GEN_6) {
u32 value;

/* Also update the CPU-side dlist copy, so that any later
* atomic updates that don't do a new modeset on our plane
* also use our updated address.
*/
vc4_state->dlist[vc4_state->ptr0_offset[0]] = addr;
value = vc4_state->dlist[vc4_state->ptr0_offset[0]] &
~SCALER6_PTR0_UPPER_ADDR_MASK;
value |= VC4_SET_FIELD(upper_32_bits(dma_addr) & 0xff,
SCALER6_PTR0_UPPER_ADDR);

writel(value, &vc4_state->hw_dlist[vc4_state->ptr0_offset[0]]);
vc4_state->dlist[vc4_state->ptr0_offset[0]] = value;

value = lower_32_bits(dma_addr);
writel(value, &vc4_state->hw_dlist[vc4_state->ptr0_offset[0] + 1]);
vc4_state->dlist[vc4_state->ptr0_offset[0] + 1] = value;
} else {
u32 addr;

addr = (u32)dma_addr;
6by9 marked this conversation as resolved.
Show resolved Hide resolved

/* Write the new address into the hardware immediately. The
* scanout will start from this address as soon as the FIFO
* needs to refill with pixels.
*/
writel(addr, &vc4_state->hw_dlist[vc4_state->ptr0_offset[0]]);

/* Also update the CPU-side dlist copy, so that any later
* atomic updates that don't do a new modeset on our plane
* also use our updated address.
*/
vc4_state->dlist[vc4_state->ptr0_offset[0]] = addr;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

@6by9 6by9 Jan 4, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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]]);

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Of course.

}

drm_dev_exit(idx);
}
Expand Down
Loading