| Message ID | 20190329120057.1472-2-mario.kleiner.de@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/5] drm/amd/display: Update VRR state earlier in atomic_commit_tail. | expand |
On 3/29/19 8:00 AM, Mario Kleiner wrote: > We need the VRR active/inactive state info earlier in > the commit sequence, so VRR related setup functions like > amdgpu_dm_handle_vrr_transition() know the final VRR state > when they need to do their hw setup work. > > Split update_freesync_state_on_stream() into an early part, > that can run at the beginning of commit tail before the > vrr transition handling, and a late part that must run after > vrr transition handling inside the commit planes code for > enabled crtc's. > > Suggested by Nicholas Kazlauskas. > Signed-off-by: Mario Kleiner <mario.kleiner.de@gmail.com> Reviewed-by: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com> This works fine as far as I can tell even though my preference was still on doing it on atomic check. I can still refactor or take a look at that later if I find some time at least. Nicholas Kazlauskas > --- > .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 61 ++++++++++++++----- > 1 file changed, 46 insertions(+), 15 deletions(-) > > diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > index 7d1c782072ee..6528258f8975 100644 > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > @@ -4645,7 +4645,6 @@ static void update_freesync_state_on_stream( > { > struct mod_vrr_params vrr_params = new_crtc_state->vrr_params; > struct dc_info_packet vrr_infopacket = {0}; > - struct mod_freesync_config config = new_crtc_state->freesync_config; > > if (!new_stream) > return; > @@ -4658,20 +4657,6 @@ static void update_freesync_state_on_stream( > if (!new_stream->timing.h_total || !new_stream->timing.v_total) > return; > > - if (new_crtc_state->vrr_supported && > - config.min_refresh_in_uhz && > - config.max_refresh_in_uhz) { > - config.state = new_crtc_state->base.vrr_enabled ? > - VRR_STATE_ACTIVE_VARIABLE : > - VRR_STATE_INACTIVE; > - } else { > - config.state = VRR_STATE_UNSUPPORTED; > - } > - > - mod_freesync_build_vrr_params(dm->freesync_module, > - new_stream, > - &config, &vrr_params); > - > if (surface) { > mod_freesync_handle_preflip( > dm->freesync_module, > @@ -4712,6 +4697,46 @@ static void update_freesync_state_on_stream( > (int)vrr_params.state); > } > > +static void pre_update_freesync_state_on_stream( > + struct amdgpu_display_manager *dm, > + struct dm_crtc_state *new_crtc_state) > +{ > + struct dc_stream_state *new_stream = new_crtc_state->stream; > + struct mod_vrr_params vrr_params = new_crtc_state->vrr_params; > + struct mod_freesync_config config = new_crtc_state->freesync_config; > + > + if (!new_stream) > + return; > + > + /* > + * TODO: Determine why min/max totals and vrefresh can be 0 here. > + * For now it's sufficient to just guard against these conditions. > + */ > + if (!new_stream->timing.h_total || !new_stream->timing.v_total) > + return; > + > + if (new_crtc_state->vrr_supported && > + config.min_refresh_in_uhz && > + config.max_refresh_in_uhz) { > + config.state = new_crtc_state->base.vrr_enabled ? > + VRR_STATE_ACTIVE_VARIABLE : > + VRR_STATE_INACTIVE; > + } else { > + config.state = VRR_STATE_UNSUPPORTED; > + } > + > + mod_freesync_build_vrr_params(dm->freesync_module, > + new_stream, > + &config, &vrr_params); > + > + new_crtc_state->freesync_timing_changed |= > + (memcmp(&new_crtc_state->vrr_params.adjust, > + &vrr_params.adjust, > + sizeof(vrr_params.adjust)) != 0); > + > + new_crtc_state->vrr_params = vrr_params; > +} > + > static void amdgpu_dm_commit_planes(struct drm_atomic_state *state, > struct dc_state *dc_state, > struct drm_device *dev, > @@ -5233,6 +5258,12 @@ static void amdgpu_dm_atomic_commit_tail(struct drm_atomic_state *state) > mutex_unlock(&dm->dc_lock); > } > > + /* Update freesync state before amdgpu_dm_handle_vrr_transition(). */ > + for_each_new_crtc_in_state(state, crtc, new_crtc_state, i) { > + dm_new_crtc_state = to_dm_crtc_state(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) { > /* >
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c index 7d1c782072ee..6528258f8975 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -4645,7 +4645,6 @@ static void update_freesync_state_on_stream( { struct mod_vrr_params vrr_params = new_crtc_state->vrr_params; struct dc_info_packet vrr_infopacket = {0}; - struct mod_freesync_config config = new_crtc_state->freesync_config; if (!new_stream) return; @@ -4658,20 +4657,6 @@ static void update_freesync_state_on_stream( if (!new_stream->timing.h_total || !new_stream->timing.v_total) return; - if (new_crtc_state->vrr_supported && - config.min_refresh_in_uhz && - config.max_refresh_in_uhz) { - config.state = new_crtc_state->base.vrr_enabled ? - VRR_STATE_ACTIVE_VARIABLE : - VRR_STATE_INACTIVE; - } else { - config.state = VRR_STATE_UNSUPPORTED; - } - - mod_freesync_build_vrr_params(dm->freesync_module, - new_stream, - &config, &vrr_params); - if (surface) { mod_freesync_handle_preflip( dm->freesync_module, @@ -4712,6 +4697,46 @@ static void update_freesync_state_on_stream( (int)vrr_params.state); } +static void pre_update_freesync_state_on_stream( + struct amdgpu_display_manager *dm, + struct dm_crtc_state *new_crtc_state) +{ + struct dc_stream_state *new_stream = new_crtc_state->stream; + struct mod_vrr_params vrr_params = new_crtc_state->vrr_params; + struct mod_freesync_config config = new_crtc_state->freesync_config; + + if (!new_stream) + return; + + /* + * TODO: Determine why min/max totals and vrefresh can be 0 here. + * For now it's sufficient to just guard against these conditions. + */ + if (!new_stream->timing.h_total || !new_stream->timing.v_total) + return; + + if (new_crtc_state->vrr_supported && + config.min_refresh_in_uhz && + config.max_refresh_in_uhz) { + config.state = new_crtc_state->base.vrr_enabled ? + VRR_STATE_ACTIVE_VARIABLE : + VRR_STATE_INACTIVE; + } else { + config.state = VRR_STATE_UNSUPPORTED; + } + + mod_freesync_build_vrr_params(dm->freesync_module, + new_stream, + &config, &vrr_params); + + new_crtc_state->freesync_timing_changed |= + (memcmp(&new_crtc_state->vrr_params.adjust, + &vrr_params.adjust, + sizeof(vrr_params.adjust)) != 0); + + new_crtc_state->vrr_params = vrr_params; +} + static void amdgpu_dm_commit_planes(struct drm_atomic_state *state, struct dc_state *dc_state, struct drm_device *dev, @@ -5233,6 +5258,12 @@ static void amdgpu_dm_atomic_commit_tail(struct drm_atomic_state *state) mutex_unlock(&dm->dc_lock); } + /* Update freesync state before amdgpu_dm_handle_vrr_transition(). */ + for_each_new_crtc_in_state(state, crtc, new_crtc_state, i) { + dm_new_crtc_state = to_dm_crtc_state(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) { /*
We need the VRR active/inactive state info earlier in the commit sequence, so VRR related setup functions like amdgpu_dm_handle_vrr_transition() know the final VRR state when they need to do their hw setup work. Split update_freesync_state_on_stream() into an early part, that can run at the beginning of commit tail before the vrr transition handling, and a late part that must run after vrr transition handling inside the commit planes code for enabled crtc's. Suggested by Nicholas Kazlauskas. Signed-off-by: Mario Kleiner <mario.kleiner.de@gmail.com> --- .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 61 ++++++++++++++----- 1 file changed, 46 insertions(+), 15 deletions(-)