Commit d6ef9b41 authored by Nicholas Kazlauskas's avatar Nicholas Kazlauskas Committed by Alex Deucher

drm/amd/display: Refactor CRTC interrupt toggling logic

[Why]
The vblank and pageflip interrupts should only be enabled for a CRTC
that's enabled and has active planes.

The current logic takes care of this, but isn't setup to handle the case
where the active plane count goes to zero but the stream remains
enabled.

We currently block this case since we don't allow commits that enable a
CRTC with no active planes, but shouldn't be any reason we can't support
this from a hardware perspective and many userspace applications expect
to be able to do it (like IGT).

[How]
The count_crtc_active_planes function fills in the number of
"active_planes" on the dm_crtc_state. This should be the same as
DC's plane_count on the stream_status but easier to access since we
don't need to lock the private atomic state with the DC context.

Add the "interrupts_enabled" flag to the dm_crtc_state and set it based
on whether the stream exists and if there are active planes on the
stream.

Update the disable and enable logic to make use of this new flag.

There shouldn't be any functional change (yet) with this patch.
Signed-off-by: default avatarNicholas Kazlauskas <nicholas.kazlauskas@amd.com>
Reviewed-by: default avatarDavid Francis <David.Francis@amd.com>
Acked-by: default avatarLeo Li <sunpeng.li@amd.com>
Signed-off-by: default avatarAlex Deucher <alexander.deucher@amd.com>
parent edf1e000
...@@ -3438,6 +3438,8 @@ dm_crtc_duplicate_state(struct drm_crtc *crtc) ...@@ -3438,6 +3438,8 @@ dm_crtc_duplicate_state(struct drm_crtc *crtc)
dc_stream_retain(state->stream); dc_stream_retain(state->stream);
} }
state->active_planes = cur->active_planes;
state->interrupts_enabled = cur->interrupts_enabled;
state->vrr_params = cur->vrr_params; state->vrr_params = cur->vrr_params;
state->vrr_infopacket = cur->vrr_infopacket; state->vrr_infopacket = cur->vrr_infopacket;
state->abm_level = cur->abm_level; state->abm_level = cur->abm_level;
...@@ -3862,7 +3864,7 @@ static void dm_crtc_helper_disable(struct drm_crtc *crtc) ...@@ -3862,7 +3864,7 @@ static void dm_crtc_helper_disable(struct drm_crtc *crtc)
{ {
} }
static bool does_crtc_have_active_plane(struct drm_crtc_state *new_crtc_state) static int count_crtc_active_planes(struct drm_crtc_state *new_crtc_state)
{ {
struct drm_atomic_state *state = new_crtc_state->state; struct drm_atomic_state *state = new_crtc_state->state;
struct drm_plane *plane; struct drm_plane *plane;
...@@ -3891,7 +3893,32 @@ static bool does_crtc_have_active_plane(struct drm_crtc_state *new_crtc_state) ...@@ -3891,7 +3893,32 @@ static bool does_crtc_have_active_plane(struct drm_crtc_state *new_crtc_state)
num_active += (new_plane_state->fb != NULL); num_active += (new_plane_state->fb != NULL);
} }
return num_active > 0; return num_active;
}
/*
* Sets whether interrupts should be enabled on a specific CRTC.
* We require that the stream be enabled and that there exist active
* DC planes on the stream.
*/
static void
dm_update_crtc_interrupt_state(struct drm_crtc *crtc,
struct drm_crtc_state *new_crtc_state)
{
struct dm_crtc_state *dm_new_crtc_state =
to_dm_crtc_state(new_crtc_state);
dm_new_crtc_state->active_planes = 0;
dm_new_crtc_state->interrupts_enabled = false;
if (!dm_new_crtc_state->stream)
return;
dm_new_crtc_state->active_planes =
count_crtc_active_planes(new_crtc_state);
dm_new_crtc_state->interrupts_enabled =
dm_new_crtc_state->active_planes > 0;
} }
static int dm_crtc_helper_atomic_check(struct drm_crtc *crtc, static int dm_crtc_helper_atomic_check(struct drm_crtc *crtc,
...@@ -3902,6 +3929,14 @@ static int dm_crtc_helper_atomic_check(struct drm_crtc *crtc, ...@@ -3902,6 +3929,14 @@ static int dm_crtc_helper_atomic_check(struct drm_crtc *crtc,
struct dm_crtc_state *dm_crtc_state = to_dm_crtc_state(state); struct dm_crtc_state *dm_crtc_state = to_dm_crtc_state(state);
int ret = -EINVAL; int ret = -EINVAL;
/*
* Update interrupt state for the CRTC. This needs to happen whenever
* the CRTC has changed or whenever any of its planes have changed.
* Atomic check satisfies both of these requirements since the CRTC
* is added to the state by DRM during drm_atomic_helper_check_planes.
*/
dm_update_crtc_interrupt_state(crtc, state);
if (unlikely(!dm_crtc_state->stream && if (unlikely(!dm_crtc_state->stream &&
modeset_required(state, NULL, dm_crtc_state->stream))) { modeset_required(state, NULL, dm_crtc_state->stream))) {
WARN_ON(1); WARN_ON(1);
...@@ -3914,7 +3949,7 @@ static int dm_crtc_helper_atomic_check(struct drm_crtc *crtc, ...@@ -3914,7 +3949,7 @@ static int dm_crtc_helper_atomic_check(struct drm_crtc *crtc,
/* We want at least one hardware plane enabled to use the stream. */ /* We want at least one hardware plane enabled to use the stream. */
if (state->enable && state->active && if (state->enable && state->active &&
!does_crtc_have_active_plane(state)) dm_crtc_state->active_planes == 0)
return -EINVAL; return -EINVAL;
if (dc_validate_stream(dc, dm_crtc_state->stream) == DC_OK) if (dc_validate_stream(dc, dm_crtc_state->stream) == DC_OK)
...@@ -5390,24 +5425,33 @@ static int amdgpu_dm_atomic_commit(struct drm_device *dev, ...@@ -5390,24 +5425,33 @@ static int amdgpu_dm_atomic_commit(struct drm_device *dev,
int i; int i;
/* /*
* We evade vblanks and pflips on crtc that * We evade vblank and pflip interrupts on CRTCs that are undergoing
* should be changed. We do it here to flush & disable * a modeset, being disabled, or have no active planes.
* interrupts before drm_swap_state is called in drm_atomic_helper_commit *
* it will update crtc->dm_crtc_state->stream pointer which is used in * It's done in atomic commit rather than commit tail for now since
* the ISRs. * some of these interrupt handlers access the current CRTC state and
* potentially the stream pointer itself.
*
* Since the atomic state is swapped within atomic commit and not within
* commit tail this would leave to new state (that hasn't been committed yet)
* being accesssed from within the handlers.
*
* TODO: Fix this so we can do this in commit tail and not have to block
* in atomic check.
*/ */
for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) { for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) {
struct dm_crtc_state *dm_old_crtc_state = to_dm_crtc_state(old_crtc_state); struct dm_crtc_state *dm_old_crtc_state = to_dm_crtc_state(old_crtc_state);
struct dm_crtc_state *dm_new_crtc_state = to_dm_crtc_state(new_crtc_state); struct dm_crtc_state *dm_new_crtc_state = to_dm_crtc_state(new_crtc_state);
struct amdgpu_crtc *acrtc = to_amdgpu_crtc(crtc); struct amdgpu_crtc *acrtc = to_amdgpu_crtc(crtc);
if (drm_atomic_crtc_needs_modeset(new_crtc_state) if (dm_old_crtc_state->interrupts_enabled &&
&& dm_old_crtc_state->stream) { (!dm_new_crtc_state->interrupts_enabled ||
drm_atomic_crtc_needs_modeset(new_crtc_state))) {
/* /*
* If the stream is removed and CRC capture was * If the stream is removed and CRC capture was
* enabled on the CRTC the extra vblank reference * enabled on the CRTC the extra vblank reference
* needs to be dropped since CRC capture will be * needs to be dropped since CRC capture will not
* disabled. * be re-enabled.
*/ */
if (!dm_new_crtc_state->stream if (!dm_new_crtc_state->stream
&& dm_new_crtc_state->crc_enabled) { && dm_new_crtc_state->crc_enabled) {
...@@ -5635,13 +5679,14 @@ static void amdgpu_dm_atomic_commit_tail(struct drm_atomic_state *state) ...@@ -5635,13 +5679,14 @@ static void amdgpu_dm_atomic_commit_tail(struct drm_atomic_state *state)
pre_update_freesync_state_on_stream(dm, dm_new_crtc_state); pre_update_freesync_state_on_stream(dm, dm_new_crtc_state);
} }
for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state,
new_crtc_state, i) {
/* /*
* loop to enable interrupts on newly arrived crtc * Enable interrupts on CRTCs that are newly active, undergone
* a modeset, or have active planes again.
*/ */
for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state,
new_crtc_state, i) {
struct amdgpu_crtc *acrtc = to_amdgpu_crtc(crtc); struct amdgpu_crtc *acrtc = to_amdgpu_crtc(crtc);
bool modeset_needed; bool enable;
if (old_crtc_state->active && !new_crtc_state->active) if (old_crtc_state->active && !new_crtc_state->active)
crtc_disable_count++; crtc_disable_count++;
...@@ -5653,12 +5698,11 @@ static void amdgpu_dm_atomic_commit_tail(struct drm_atomic_state *state) ...@@ -5653,12 +5698,11 @@ static void amdgpu_dm_atomic_commit_tail(struct drm_atomic_state *state)
amdgpu_dm_handle_vrr_transition(dm_old_crtc_state, amdgpu_dm_handle_vrr_transition(dm_old_crtc_state,
dm_new_crtc_state); dm_new_crtc_state);
modeset_needed = modeset_required( enable = dm_new_crtc_state->interrupts_enabled &&
new_crtc_state, (!dm_old_crtc_state->interrupts_enabled ||
dm_new_crtc_state->stream, drm_atomic_crtc_needs_modeset(new_crtc_state));
dm_old_crtc_state->stream);
if (dm_new_crtc_state->stream == NULL || !modeset_needed) if (!enable)
continue; continue;
manage_dm_interrupts(adev, acrtc, true); manage_dm_interrupts(adev, acrtc, true);
......
...@@ -271,6 +271,9 @@ struct dm_crtc_state { ...@@ -271,6 +271,9 @@ struct dm_crtc_state {
struct drm_crtc_state base; struct drm_crtc_state base;
struct dc_stream_state *stream; struct dc_stream_state *stream;
int active_planes;
bool interrupts_enabled;
int crc_skip_count; int crc_skip_count;
bool crc_enabled; bool crc_enabled;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment