Skip to content

Commit

Permalink
drm/amd/display: Add fast path for cursor plane updates
Browse files Browse the repository at this point in the history
[Why]
Legacy cursor plane updates from drm helpers go through the full
atomic codepath. A high volume of cursor updates through this slow
code path can cause subsequent page-flips to skip vblank intervals
since each individual update is slow.

This problem is particularly noticeable for the compton compositor.

[How]
A fast path for cursor plane updates is added by using DRM asynchronous
commit support provided by async_check and async_update. These don't do
a full state/flip_done dependency stall and they don't block other
commit work.

However, DC still expects itself to be single-threaded for anything
that can issue register writes. Screen corruption or hangs can occur
if write sequences overlap. Every call that potentially perform
register writes needs to be guarded for asynchronous updates to work.
The dc_lock mutex was added for this.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=106175

Signed-off-by: Nicholas Kazlauskas <[email protected]>
Acked-by: Andrey Grodzovsky <[email protected]>
Reviewed-by Leo Li <[email protected]>
Signed-off-by: Alex Deucher <[email protected]>
  • Loading branch information
Nicholas Kazlauskas authored and alexdeucher committed Dec 12, 2018
1 parent fc42d47 commit 674e78a
Show file tree
Hide file tree
Showing 2 changed files with 73 additions and 2 deletions.
67 changes: 65 additions & 2 deletions drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@

#include <drm/drmP.h>
#include <drm/drm_atomic.h>
#include <drm/drm_atomic_uapi.h>
#include <drm/drm_atomic_helper.h>
#include <drm/drm_dp_mst_helper.h>
#include <drm/drm_fb_helper.h>
Expand Down Expand Up @@ -133,6 +134,8 @@ static void amdgpu_dm_atomic_commit_tail(struct drm_atomic_state *state);
static int amdgpu_dm_atomic_check(struct drm_device *dev,
struct drm_atomic_state *state);

static void handle_cursor_update(struct drm_plane *plane,
struct drm_plane_state *old_plane_state);



Expand Down Expand Up @@ -402,6 +405,8 @@ static int amdgpu_dm_init(struct amdgpu_device *adev)
/* Zero all the fields */
memset(&init_data, 0, sizeof(init_data));

mutex_init(&adev->dm.dc_lock);

if(amdgpu_dm_irq_init(adev)) {
DRM_ERROR("amdgpu: failed to initialize DM IRQ support.\n");
goto error;
Expand Down Expand Up @@ -516,6 +521,9 @@ static void amdgpu_dm_fini(struct amdgpu_device *adev)
/* DC Destroy TODO: Replace destroy DAL */
if (adev->dm.dc)
dc_destroy(&adev->dm.dc);

mutex_destroy(&adev->dm.dc_lock);

return;
}

Expand Down Expand Up @@ -3617,10 +3625,43 @@ static int dm_plane_atomic_check(struct drm_plane *plane,
return -EINVAL;
}

static int dm_plane_atomic_async_check(struct drm_plane *plane,
struct drm_plane_state *new_plane_state)
{
/* Only support async updates on cursor planes. */
if (plane->type != DRM_PLANE_TYPE_CURSOR)
return -EINVAL;

return 0;
}

static void dm_plane_atomic_async_update(struct drm_plane *plane,
struct drm_plane_state *new_state)
{
struct drm_plane_state *old_state =
drm_atomic_get_old_plane_state(new_state->state, plane);

if (plane->state->fb != new_state->fb)
drm_atomic_set_fb_for_plane(plane->state, new_state->fb);

plane->state->src_x = new_state->src_x;
plane->state->src_y = new_state->src_y;
plane->state->src_w = new_state->src_w;
plane->state->src_h = new_state->src_h;
plane->state->crtc_x = new_state->crtc_x;
plane->state->crtc_y = new_state->crtc_y;
plane->state->crtc_w = new_state->crtc_w;
plane->state->crtc_h = new_state->crtc_h;

handle_cursor_update(plane, old_state);
}

static const struct drm_plane_helper_funcs dm_plane_helper_funcs = {
.prepare_fb = dm_plane_helper_prepare_fb,
.cleanup_fb = dm_plane_helper_cleanup_fb,
.atomic_check = dm_plane_atomic_check,
.atomic_async_check = dm_plane_atomic_async_check,
.atomic_async_update = dm_plane_atomic_async_update
};

/*
Expand Down Expand Up @@ -4309,6 +4350,7 @@ static int get_cursor_position(struct drm_plane *plane, struct drm_crtc *crtc,
static void handle_cursor_update(struct drm_plane *plane,
struct drm_plane_state *old_plane_state)
{
struct amdgpu_device *adev = plane->dev->dev_private;
struct amdgpu_framebuffer *afb = to_amdgpu_framebuffer(plane->state->fb);
struct drm_crtc *crtc = afb ? plane->state->crtc : old_plane_state->crtc;
struct dm_crtc_state *crtc_state = crtc ? to_dm_crtc_state(crtc->state) : NULL;
Expand All @@ -4333,9 +4375,12 @@ static void handle_cursor_update(struct drm_plane *plane,

if (!position.enable) {
/* turn off cursor */
if (crtc_state && crtc_state->stream)
if (crtc_state && crtc_state->stream) {
mutex_lock(&adev->dm.dc_lock);
dc_stream_set_cursor_position(crtc_state->stream,
&position);
mutex_unlock(&adev->dm.dc_lock);
}
return;
}

Expand All @@ -4353,13 +4398,15 @@ static void handle_cursor_update(struct drm_plane *plane,
attributes.pitch = attributes.width;

if (crtc_state->stream) {
mutex_lock(&adev->dm.dc_lock);
if (!dc_stream_set_cursor_attributes(crtc_state->stream,
&attributes))
DRM_ERROR("DC failed to set cursor attributes\n");

if (!dc_stream_set_cursor_position(crtc_state->stream,
&position))
DRM_ERROR("DC failed to set cursor position\n");
mutex_unlock(&adev->dm.dc_lock);
}
}

Expand Down Expand Up @@ -4575,13 +4622,15 @@ static void amdgpu_dm_do_flip(struct drm_crtc *crtc,
&acrtc_state->stream->vrr_infopacket;
}

mutex_lock(&adev->dm.dc_lock);
dc_commit_updates_for_stream(adev->dm.dc,
surface_updates,
1,
acrtc_state->stream,
&stream_update,
&surface_updates->surface,
state);
mutex_unlock(&adev->dm.dc_lock);

DRM_DEBUG_DRIVER("%s Flipping to hi: 0x%x, low: 0x%x \n",
__func__,
Expand All @@ -4596,6 +4645,7 @@ static void amdgpu_dm_do_flip(struct drm_crtc *crtc,
* with a dc_plane_state and follow the atomic model a bit more closely here.
*/
static bool commit_planes_to_stream(
struct amdgpu_display_manager *dm,
struct dc *dc,
struct dc_plane_state **plane_states,
uint8_t new_plane_count,
Expand Down Expand Up @@ -4672,11 +4722,13 @@ static bool commit_planes_to_stream(
updates[i].scaling_info = &scaling_info[i];
}

mutex_lock(&dm->dc_lock);
dc_commit_updates_for_stream(
dc,
updates,
new_plane_count,
dc_stream, stream_update, plane_states, state);
mutex_unlock(&dm->dc_lock);

kfree(flip_addr);
kfree(plane_info);
Expand Down Expand Up @@ -4782,7 +4834,8 @@ static void amdgpu_dm_commit_planes(struct drm_atomic_state *state,

dc_stream_attach->abm_level = acrtc_state->abm_level;

if (false == commit_planes_to_stream(dm->dc,
if (false == commit_planes_to_stream(dm,
dm->dc,
plane_states_constructed,
planes_count,
acrtc_state,
Expand Down Expand Up @@ -4952,7 +5005,9 @@ static void amdgpu_dm_atomic_commit_tail(struct drm_atomic_state *state)

if (dc_state) {
dm_enable_per_frame_crtc_master_sync(dc_state);
mutex_lock(&dm->dc_lock);
WARN_ON(!dc_commit_state(dm->dc, dc_state));
mutex_unlock(&dm->dc_lock);
}

for_each_new_crtc_in_state(state, crtc, new_crtc_state, i) {
Expand Down Expand Up @@ -5014,6 +5069,7 @@ static void amdgpu_dm_atomic_commit_tail(struct drm_atomic_state *state)

/*TODO How it works with MPO ?*/
if (!commit_planes_to_stream(
dm,
dm->dc,
status->plane_states,
status->plane_count,
Expand Down Expand Up @@ -5906,6 +5962,13 @@ static int amdgpu_dm_atomic_check(struct drm_device *dev,
ret = -EINVAL;
goto fail;
}
} else if (state->legacy_cursor_update) {
/*
* This is a fast cursor update coming from the plane update
* helper, check if it can be done asynchronously for better
* performance.
*/
state->async_update = !drm_atomic_helper_async_check(dev, state);
}

/* Must be success */
Expand Down
8 changes: 8 additions & 0 deletions drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,14 @@ struct amdgpu_display_manager {

struct drm_modeset_lock atomic_obj_lock;

/**
* @dc_lock:
*
* Guards access to DC functions that can issue register write
* sequences.
*/
struct mutex dc_lock;

/**
* @irq_handler_list_low_tab:
*
Expand Down

0 comments on commit 674e78a

Please sign in to comment.