| Message ID | 20220502204117.32938-3-jose.souza@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2,1/3] drm/i915/display: Allow DRRS to be enabled during driver load | expand |
Hello Jose, Thanks much for the patch. I tested it on chrome system and the patch works. Adding my Tested-by. Tested-by: Vidya Srinivas <vidya.srinivas@intel.com> Regards Vidya > -----Original Message----- > From: Souza, Jose <jose.souza@intel.com> > Sent: Tuesday, May 3, 2022 2:11 AM > To: intel-gfx@lists.freedesktop.org > Cc: Srinivas, Vidya <vidya.srinivas@intel.com>; Sean Paul > <seanpaul@chromium.org>; Ville Syrjälä <ville.syrjala@linux.intel.com>; > Souza, Jose <jose.souza@intel.com> > Subject: [PATCH v2 3/3] drm/i915/display: Implement seamless mode switch > > So far the i915's DRRS feature was automatically changing between preferred > panel mode and downclock mode based on idleness but ChromeOS > compositor team is asking to be in control of the mode switch. > So for certain types of content it can switch to a mode with a lower refresh > rate without the user noticing a thing and saving more power. > > This seamless mode switch will be triggered when user-space dispatches an > atomic commit with the new mode and clears the > DRM_MODE_ATOMIC_ALLOW_MODESET flag. > > The main steps to acomplish that are: > > - as mode changed in atomic state drm_atomic_helper_check_modeset() will > set mode_changed and it will trigger the crtc state computation so > intel_dp_compute_config() will be called and dp_m_n will be computed for > the new mode > > - then intel_dp_drrs_compute_config() will check for the necessary > conditions to do a seamless mode switch, if possible crtc_state- > >seamless_mode_switch will be set and has_drrs will not be set, so i915 will > not automatically switch between modes > > - then intel_crtc_compute_config() will call > intel_crtc_compute_pixel_rate() that will take the hint that it is trying to do a > seamless mode switch and set pixel_rate to the pixel_rate of the old state > > - then if nothing else changed in the state intel_crtc_check_fastset() will be > able to set mode_changed to false and i915 can do fastset otherwise the > commit will fail during the check phase > > - now on the atomic commit phase, intel_ddi_update_pipe_dp() will be > called and will program the new dp_m_n > > - nothing else is different in the commit phase until the step to verify > programmed state, the most important change here is that need to save the > pixel_rate in DRRS global state as there is no other way to get that from > hardware or atomic state > > v2: > - not overwritten dp_m_n values in intel_crtc_copy_fastset() when doing a > seamless_mode_switch > > Cc: Vidya Srinivas <vidya.srinivas@intel.com> > Cc: Sean Paul <seanpaul@chromium.org> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com> > --- > drivers/gpu/drm/i915/display/intel_ddi.c | 10 +++ > drivers/gpu/drm/i915/display/intel_display.c | 61 ++++++++++++++++--- > .../drm/i915/display/intel_display_debugfs.c | 3 + > .../drm/i915/display/intel_display_types.h | 3 + > drivers/gpu/drm/i915/display/intel_dp.c | 56 ++++++++++++++--- > drivers/gpu/drm/i915/display/intel_drrs.c | 38 +++++++++--- > drivers/gpu/drm/i915/display/intel_drrs.h | 3 + > 7 files changed, 151 insertions(+), 23 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c > b/drivers/gpu/drm/i915/display/intel_ddi.c > index 9e6fa59eabba7..732e5d425412e 100644 > --- a/drivers/gpu/drm/i915/display/intel_ddi.c > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c > @@ -46,6 +46,7 @@ > #include "intel_dp_link_training.h" > #include "intel_dp_mst.h" > #include "intel_dpio_phy.h" > +#include "intel_drrs.h" > #include "intel_dsi.h" > #include "intel_fdi.h" > #include "intel_fifo_underrun.h" > @@ -3010,6 +3011,14 @@ static void intel_ddi_update_pipe_dp(struct > intel_atomic_state *state, > > intel_backlight_update(state, encoder, crtc_state, conn_state); > drm_connector_update_privacy_screen(conn_state); > + > + if (crtc_state->seamless_mode_switch) { > + struct intel_crtc *intel_crtc = to_intel_crtc(crtc_state- > >uapi.crtc); > + > + intel_cpu_transcoder_set_m1_n1(intel_crtc, > + crtc_state->cpu_transcoder, > + &crtc_state->dp_m_n); > + } > } > > void intel_ddi_update_pipe(struct intel_atomic_state *state, @@ -3484,6 > +3493,7 @@ static void intel_ddi_get_config(struct intel_encoder *encoder, > intel_read_dp_sdp(encoder, pipe_config, DP_SDP_VSC); > > intel_psr_get_config(encoder, pipe_config); > + intel_drrs_get_config(encoder, pipe_config); > } > > void intel_ddi_get_clock(struct intel_encoder *encoder, diff --git > a/drivers/gpu/drm/i915/display/intel_display.c > b/drivers/gpu/drm/i915/display/intel_display.c > index 17d0cad9e1686..e54fbe3b1d394 100644 > --- a/drivers/gpu/drm/i915/display/intel_display.c > +++ b/drivers/gpu/drm/i915/display/intel_display.c > @@ -2629,6 +2629,44 @@ static void intel_crtc_compute_pixel_rate(struct > intel_crtc_state *crtc_state) > else > crtc_state->pixel_rate = > ilk_pipe_pixel_rate(crtc_state); > + > + /* > + * Do not change pixel_rate when doing seamless mode switch, > otherwise > + * it will change port_clock and other stuff that will need a modeset > + * to be programmed > + */ > + if (crtc_state->seamless_mode_switch) { > + struct intel_atomic_state *state = > to_intel_atomic_state(crtc_state->uapi.state); > + struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc); > + const struct intel_crtc_state *old_crtc_state; > + > + old_crtc_state = intel_atomic_get_old_crtc_state(state, crtc); > + > + /* > + * It can only seamless switch if pixel rate of the new mode is > + * <= than old one, if not commit will fail with pixel_rate > + * mismatch. > + */ > + if (crtc_state->pixel_rate < old_crtc_state->pixel_rate || > + intel_fuzzy_clock_check(crtc_state->pixel_rate, > + old_crtc_state->pixel_rate)) > + crtc_state->pixel_rate = old_crtc_state->pixel_rate; > + } > +} > + > +static void intel_crtc_get_pixel_rate(struct intel_crtc_state > +*crtc_state) { > + if (crtc_state->seamless_mode_switch) { > + struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc); > + > + mutex_lock(&crtc->drrs.mutex); > + crtc_state->pixel_rate = crtc- > >drrs.seamless_mode_switch_pixel_rate; > + mutex_unlock(&crtc->drrs.mutex); > + > + return; > + } > + > + intel_crtc_compute_pixel_rate(crtc_state); > } > > static void intel_bigjoiner_adjust_timings(const struct intel_crtc_state > *crtc_state, @@ -2705,7 +2743,7 @@ static void > intel_crtc_readout_derived_state(struct intel_crtc_state *crtc_state > intel_bigjoiner_adjust_timings(crtc_state, pipe_mode); > intel_mode_from_crtc_timings(pipe_mode, pipe_mode); > > - intel_crtc_compute_pixel_rate(crtc_state); > + intel_crtc_get_pixel_rate(crtc_state); > } > > static void intel_encoder_get_config(struct intel_encoder *encoder, @@ - > 6182,11 +6220,13 @@ intel_pipe_config_compare(const struct > intel_crtc_state *current_config, > PIPE_CONF_CHECK_I(lane_count); > PIPE_CONF_CHECK_X(lane_lat_optim_mask); > > - if (DISPLAY_VER(dev_priv) >= 9 || IS_BROADWELL(dev_priv)) { > - PIPE_CONF_CHECK_M_N_ALT(dp_m_n, dp_m2_n2); > - } else { > - PIPE_CONF_CHECK_M_N(dp_m_n); > - PIPE_CONF_CHECK_M_N(dp_m2_n2); > + if (!pipe_config->seamless_mode_switch) { > + if (DISPLAY_VER(dev_priv) >= 9 || IS_BROADWELL(dev_priv)) > { > + PIPE_CONF_CHECK_M_N_ALT(dp_m_n, dp_m2_n2); > + } else { > + PIPE_CONF_CHECK_M_N(dp_m_n); > + PIPE_CONF_CHECK_M_N(dp_m2_n2); > + } > } > > PIPE_CONF_CHECK_X(output_types); > @@ -6348,8 +6388,10 @@ intel_pipe_config_compare(const struct > intel_crtc_state *current_config, > if (IS_G4X(dev_priv) || DISPLAY_VER(dev_priv) >= 5) > PIPE_CONF_CHECK_I(pipe_bpp); > > - PIPE_CONF_CHECK_CLOCK_FUZZY(hw.pipe_mode.crtc_clock); > - PIPE_CONF_CHECK_CLOCK_FUZZY(hw.adjusted_mode.crtc_clock); > + if (!pipe_config->seamless_mode_switch) { > + PIPE_CONF_CHECK_CLOCK_FUZZY(hw.pipe_mode.crtc_clock); > + > PIPE_CONF_CHECK_CLOCK_FUZZY(hw.adjusted_mode.crtc_clock); > + } > PIPE_CONF_CHECK_CLOCK_FUZZY(port_clock); > > PIPE_CONF_CHECK_I(min_voltage_level); > @@ -7085,11 +7127,12 @@ static void intel_crtc_copy_fastset(const struct > intel_crtc_state *old_crtc_stat > * FIXME: should really copy more fuzzy state here > */ > new_crtc_state->fdi_m_n = old_crtc_state->fdi_m_n; > - new_crtc_state->dp_m_n = old_crtc_state->dp_m_n; > > if (!intel_drrs_crtc_copy_fastset(old_crtc_state, new_crtc_state)) { > + new_crtc_state->dp_m_n = old_crtc_state->dp_m_n; > new_crtc_state->dp_m2_n2 = old_crtc_state->dp_m2_n2; > new_crtc_state->has_drrs = old_crtc_state->has_drrs; > + new_crtc_state->seamless_mode_switch = > +old_crtc_state->seamless_mode_switch; > } > } > > diff --git a/drivers/gpu/drm/i915/display/intel_display_debugfs.c > b/drivers/gpu/drm/i915/display/intel_display_debugfs.c > index 452d773fd4e34..92099e1f0b23e 100644 > --- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c > +++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c > @@ -1108,6 +1108,9 @@ static int i915_drrs_status(struct seq_file *m, void > *unused) > crtc->drrs.refresh_rate == > DRRS_REFRESH_RATE_LOW ? > "low" : "high"); > > + seq_printf(m, "Seamless mode switch enabled: %s\n", > + str_yes_no(crtc- > >drrs.seamless_mode_switch_pixel_rate)); > + > mutex_unlock(&crtc->drrs.mutex); > } > > diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h > b/drivers/gpu/drm/i915/display/intel_display_types.h > index 408152f9f46a4..e290366adfc27 100644 > --- a/drivers/gpu/drm/i915/display/intel_display_types.h > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h > @@ -1057,6 +1057,7 @@ struct intel_crtc_state { > /* m2_n2 for eDP downclock */ > struct intel_link_m_n dp_m2_n2; > bool has_drrs; > + bool seamless_mode_switch; > > /* PSR is supported but might not be enabled due the lack of enabled > planes */ > bool has_psr; > @@ -1314,6 +1315,8 @@ struct intel_crtc { > unsigned int busy_frontbuffer_bits; > enum transcoder cpu_transcoder; > struct intel_link_m_n m_n, m2_n2; > + /* Only used to verify if programmed state matches */ > + unsigned int seamless_mode_switch_pixel_rate; > } drrs; > > int scanline_offset; > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c > b/drivers/gpu/drm/i915/display/intel_dp.c > index cc9be82e128f4..691d362dde95e 100644 > --- a/drivers/gpu/drm/i915/display/intel_dp.c > +++ b/drivers/gpu/drm/i915/display/intel_dp.c > @@ -1844,8 +1844,7 @@ static bool cpu_transcoder_has_drrs(struct > drm_i915_private *i915, } > > static bool can_enable_drrs(struct intel_connector *connector, > - const struct intel_crtc_state *pipe_config, > - const struct drm_display_mode > *downclock_mode) > + const struct intel_crtc_state *pipe_config) > { > struct drm_i915_private *i915 = to_i915(connector->base.dev); > > @@ -1868,8 +1867,40 @@ static bool can_enable_drrs(struct > intel_connector *connector, > if (!cpu_transcoder_has_drrs(i915, pipe_config->cpu_transcoder)) > return false; > > - return downclock_mode && > - intel_panel_drrs_type(connector) == DRRS_TYPE_SEAMLESS; > + return intel_panel_drrs_type(connector) == DRRS_TYPE_SEAMLESS; } > + > +static bool can_seamless_switch_mode(struct intel_crtc_state > +*pipe_config) { > + struct drm_i915_private *i915 = to_i915(pipe_config->uapi.crtc- > >dev); > + const struct intel_crtc_state *old_pipe_config; > + struct intel_atomic_state *state; > + struct intel_crtc *intel_crtc; > + > + /* Only supported in platforms that can switch m_n on the fly */ > + if (intel_cpu_transcoder_has_m2_n2(i915, pipe_config- > >cpu_transcoder)) > + return false; > + > + state = to_intel_atomic_state(pipe_config->uapi.state); > + if (state->base.allow_modeset) > + return false; > + > + intel_crtc = to_intel_crtc(pipe_config->uapi.crtc); > + old_pipe_config = intel_atomic_get_old_crtc_state(state, intel_crtc); > + if (drm_mode_equal(&old_pipe_config->hw.adjusted_mode, > + &pipe_config->hw.adjusted_mode)) > + return false; > + > + /* Only clock can change */ > + if (!drm_mode_match(&old_pipe_config->hw.adjusted_mode, > + &pipe_config->hw.adjusted_mode, > + DRM_MODE_MATCH_TIMINGS | > + DRM_MODE_MATCH_FLAGS | > + DRM_MODE_MATCH_3D_FLAGS | > + DRM_MODE_MATCH_ASPECT_RATIO)) > + return false; > + > + return true; > } > > static void > @@ -1882,12 +1913,17 @@ intel_dp_drrs_compute_config(struct > intel_connector *connector, > intel_panel_downclock_mode(connector, &pipe_config- > >hw.adjusted_mode); > int pixel_clock; > > - if (!can_enable_drrs(connector, pipe_config, downclock_mode)) { > - if (intel_cpu_transcoder_has_m2_n2(i915, pipe_config- > >cpu_transcoder)) > - intel_zero_m_n(&pipe_config->dp_m2_n2); > + if (!can_enable_drrs(connector, pipe_config)) > + goto not_supported; > + > + if (can_seamless_switch_mode(pipe_config)) { > + pipe_config->seamless_mode_switch = true; > return; > } > > + if (!downclock_mode) > + goto not_supported; > + > if (IS_IRONLAKE(i915) || IS_SANDYBRIDGE(i915) || > IS_IVYBRIDGE(i915)) > pipe_config->msa_timing_delay = i915- > >vbt.edp.drrs_msa_timing_delay; > > @@ -1904,6 +1940,12 @@ intel_dp_drrs_compute_config(struct > intel_connector *connector, > /* FIXME: abstract this better */ > if (pipe_config->splitter.enable) > pipe_config->dp_m2_n2.data_m *= pipe_config- > >splitter.link_count; > + > + return; > + > +not_supported: > + if (intel_cpu_transcoder_has_m2_n2(i915, pipe_config- > >cpu_transcoder)) > + intel_zero_m_n(&pipe_config->dp_m2_n2); > } > > static bool intel_dp_has_audio(struct intel_encoder *encoder, diff --git > a/drivers/gpu/drm/i915/display/intel_drrs.c > b/drivers/gpu/drm/i915/display/intel_drrs.c > index d266fad83a086..ffd51d2f5998d 100644 > --- a/drivers/gpu/drm/i915/display/intel_drrs.c > +++ b/drivers/gpu/drm/i915/display/intel_drrs.c > @@ -144,16 +144,19 @@ void intel_drrs_activate(const struct > intel_crtc_state *crtc_state) { > struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc); > > + mutex_lock(&crtc->drrs.mutex); > + > + crtc->drrs.seamless_mode_switch_pixel_rate = crtc_state- > >seamless_mode_switch ? > + crtc_state->pixel_rate : 0; > + > if (!crtc_state->has_drrs) > - return; > + goto unlock; > > if (!crtc_state->hw.active) > - return; > + goto unlock; > > if (intel_crtc_is_bigjoiner_slave(crtc_state)) > - return; > - > - mutex_lock(&crtc->drrs.mutex); > + goto unlock; > > crtc->drrs.cpu_transcoder = crtc_state->cpu_transcoder; > crtc->drrs.m_n = crtc_state->dp_m_n; > @@ -163,6 +166,7 @@ void intel_drrs_activate(const struct intel_crtc_state > *crtc_state) > > intel_drrs_schedule_work(crtc); > > +unlock: > mutex_unlock(&crtc->drrs.mutex); > } > > @@ -176,7 +180,7 @@ void intel_drrs_deactivate(const struct > intel_crtc_state *old_crtc_state) { > struct intel_crtc *crtc = to_intel_crtc(old_crtc_state->uapi.crtc); > > - if (!old_crtc_state->has_drrs) > + if (!old_crtc_state->has_drrs && > +!old_crtc_state->seamless_mode_switch) > return; > > if (!old_crtc_state->hw.active) > @@ -187,12 +191,14 @@ void intel_drrs_deactivate(const struct > intel_crtc_state *old_crtc_state) > > mutex_lock(&crtc->drrs.mutex); > > - if (intel_drrs_is_active(crtc)) > + if (intel_drrs_is_active(crtc) && > + old_crtc_state->seamless_mode_switch == 0) > intel_drrs_set_state(crtc, DRRS_REFRESH_RATE_HIGH); > > crtc->drrs.cpu_transcoder = INVALID_TRANSCODER; > crtc->drrs.frontbuffer_bits = 0; > crtc->drrs.busy_frontbuffer_bits = 0; > + crtc->drrs.seamless_mode_switch_pixel_rate = 0; > > mutex_unlock(&crtc->drrs.mutex); > > @@ -322,5 +328,23 @@ bool intel_drrs_crtc_copy_fastset(const struct > intel_crtc_state *old_crtc_state, > if (intel_cpu_transcoder_has_m2_n2(i915, new_crtc_state- > >cpu_transcoder)) > return false; > > + if (!new_crtc_state->seamless_mode_switch) > + new_crtc_state->dp_m_n = old_crtc_state->dp_m_n; > + > return true; > } > + > +void intel_drrs_get_config(struct intel_encoder *encoder, > + struct intel_crtc_state *pipe_config) { > + struct intel_crtc *crtc; > + > + crtc = to_intel_crtc(pipe_config->uapi.crtc); > + > + mutex_lock(&crtc->drrs.mutex); > + > + pipe_config->seamless_mode_switch = crtc- > >drrs.seamless_mode_switch_pixel_rate; > + pipe_config->has_drrs = intel_drrs_is_active(crtc); > + > + mutex_unlock(&crtc->drrs.mutex); > +} > diff --git a/drivers/gpu/drm/i915/display/intel_drrs.h > b/drivers/gpu/drm/i915/display/intel_drrs.h > index 749ac717db063..a100023658a5a 100644 > --- a/drivers/gpu/drm/i915/display/intel_drrs.h > +++ b/drivers/gpu/drm/i915/display/intel_drrs.h > @@ -14,6 +14,7 @@ struct intel_atomic_state; struct intel_crtc; struct > intel_crtc_state; struct intel_connector; > +struct intel_encoder; > > const char *intel_drrs_type_str(enum drrs_type drrs_type); bool > intel_drrs_is_active(struct intel_crtc *crtc); @@ -26,5 +27,7 @@ void > intel_drrs_flush(struct drm_i915_private *dev_priv, void > intel_crtc_drrs_init(struct intel_crtc *crtc); bool > intel_drrs_crtc_copy_fastset(const struct intel_crtc_state *old_crtc_state, > struct intel_crtc_state *new_crtc_state); > +void intel_drrs_get_config(struct intel_encoder *encoder, > + struct intel_crtc_state *pipe_config); > > #endif /* __INTEL_DRRS_H__ */ > -- > 2.36.0
Hello, Apologies for bothering. May we kindly know if this solution is approved? I have provided the Tested-by. Thanks much. Regards Vidya > -----Original Message----- > From: Srinivas, Vidya > Sent: Monday, May 9, 2022 8:44 PM > To: Souza, Jose <jose.souza@intel.com>; intel-gfx@lists.freedesktop.org > Cc: Sean Paul <seanpaul@chromium.org>; Ville Syrjälä > <ville.syrjala@linux.intel.com> > Subject: RE: [PATCH v2 3/3] drm/i915/display: Implement seamless mode > switch > > Hello Jose, > > Thanks much for the patch. I tested it on chrome system and the patch > works. > Adding my Tested-by. > Tested-by: Vidya Srinivas <vidya.srinivas@intel.com> > > Regards > Vidya > > > -----Original Message----- > > From: Souza, Jose <jose.souza@intel.com> > > Sent: Tuesday, May 3, 2022 2:11 AM > > To: intel-gfx@lists.freedesktop.org > > Cc: Srinivas, Vidya <vidya.srinivas@intel.com>; Sean Paul > > <seanpaul@chromium.org>; Ville Syrjälä > > <ville.syrjala@linux.intel.com>; Souza, Jose <jose.souza@intel.com> > > Subject: [PATCH v2 3/3] drm/i915/display: Implement seamless mode > > switch > > > > So far the i915's DRRS feature was automatically changing between > > preferred panel mode and downclock mode based on idleness but > ChromeOS > > compositor team is asking to be in control of the mode switch. > > So for certain types of content it can switch to a mode with a lower > > refresh rate without the user noticing a thing and saving more power. > > > > This seamless mode switch will be triggered when user-space dispatches > > an atomic commit with the new mode and clears the > > DRM_MODE_ATOMIC_ALLOW_MODESET flag. > > > > The main steps to acomplish that are: > > > > - as mode changed in atomic state drm_atomic_helper_check_modeset() > > will set mode_changed and it will trigger the crtc state computation > > so > > intel_dp_compute_config() will be called and dp_m_n will be computed > > for the new mode > > > > - then intel_dp_drrs_compute_config() will check for the necessary > > conditions to do a seamless mode switch, if possible crtc_state- > > >seamless_mode_switch will be set and has_drrs will not be set, so > > >i915 will > > not automatically switch between modes > > > > - then intel_crtc_compute_config() will call > > intel_crtc_compute_pixel_rate() that will take the hint that it is > > trying to do a seamless mode switch and set pixel_rate to the > > pixel_rate of the old state > > > > - then if nothing else changed in the state intel_crtc_check_fastset() > > will be able to set mode_changed to false and i915 can do fastset > > otherwise the commit will fail during the check phase > > > > - now on the atomic commit phase, intel_ddi_update_pipe_dp() will be > > called and will program the new dp_m_n > > > > - nothing else is different in the commit phase until the step to > > verify programmed state, the most important change here is that need > > to save the pixel_rate in DRRS global state as there is no other way > > to get that from hardware or atomic state > > > > v2: > > - not overwritten dp_m_n values in intel_crtc_copy_fastset() when > > doing a seamless_mode_switch > > > > Cc: Vidya Srinivas <vidya.srinivas@intel.com> > > Cc: Sean Paul <seanpaul@chromium.org> > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > > Signed-off-by: José Roberto de Souza <jose.souza@intel.com> > > --- > > drivers/gpu/drm/i915/display/intel_ddi.c | 10 +++ > > drivers/gpu/drm/i915/display/intel_display.c | 61 > > ++++++++++++++++--- .../drm/i915/display/intel_display_debugfs.c | 3 + > > .../drm/i915/display/intel_display_types.h | 3 + > > drivers/gpu/drm/i915/display/intel_dp.c | 56 ++++++++++++++--- > > drivers/gpu/drm/i915/display/intel_drrs.c | 38 +++++++++--- > > drivers/gpu/drm/i915/display/intel_drrs.h | 3 + > > 7 files changed, 151 insertions(+), 23 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c > > b/drivers/gpu/drm/i915/display/intel_ddi.c > > index 9e6fa59eabba7..732e5d425412e 100644 > > --- a/drivers/gpu/drm/i915/display/intel_ddi.c > > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c > > @@ -46,6 +46,7 @@ > > #include "intel_dp_link_training.h" > > #include "intel_dp_mst.h" > > #include "intel_dpio_phy.h" > > +#include "intel_drrs.h" > > #include "intel_dsi.h" > > #include "intel_fdi.h" > > #include "intel_fifo_underrun.h" > > @@ -3010,6 +3011,14 @@ static void intel_ddi_update_pipe_dp(struct > > intel_atomic_state *state, > > > > intel_backlight_update(state, encoder, crtc_state, conn_state); > > drm_connector_update_privacy_screen(conn_state); > > + > > + if (crtc_state->seamless_mode_switch) { > > + struct intel_crtc *intel_crtc = to_intel_crtc(crtc_state- > > >uapi.crtc); > > + > > + intel_cpu_transcoder_set_m1_n1(intel_crtc, > > + crtc_state->cpu_transcoder, > > + &crtc_state->dp_m_n); > > + } > > } > > > > void intel_ddi_update_pipe(struct intel_atomic_state *state, @@ > > -3484,6 > > +3493,7 @@ static void intel_ddi_get_config(struct intel_encoder > > +*encoder, > > intel_read_dp_sdp(encoder, pipe_config, DP_SDP_VSC); > > > > intel_psr_get_config(encoder, pipe_config); > > + intel_drrs_get_config(encoder, pipe_config); > > } > > > > void intel_ddi_get_clock(struct intel_encoder *encoder, diff --git > > a/drivers/gpu/drm/i915/display/intel_display.c > > b/drivers/gpu/drm/i915/display/intel_display.c > > index 17d0cad9e1686..e54fbe3b1d394 100644 > > --- a/drivers/gpu/drm/i915/display/intel_display.c > > +++ b/drivers/gpu/drm/i915/display/intel_display.c > > @@ -2629,6 +2629,44 @@ static void > > intel_crtc_compute_pixel_rate(struct > > intel_crtc_state *crtc_state) > > else > > crtc_state->pixel_rate = > > ilk_pipe_pixel_rate(crtc_state); > > + > > + /* > > + * Do not change pixel_rate when doing seamless mode switch, > > otherwise > > + * it will change port_clock and other stuff that will need a modeset > > + * to be programmed > > + */ > > + if (crtc_state->seamless_mode_switch) { > > + struct intel_atomic_state *state = > > to_intel_atomic_state(crtc_state->uapi.state); > > + struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc); > > + const struct intel_crtc_state *old_crtc_state; > > + > > + old_crtc_state = intel_atomic_get_old_crtc_state(state, crtc); > > + > > + /* > > + * It can only seamless switch if pixel rate of the new mode is > > + * <= than old one, if not commit will fail with pixel_rate > > + * mismatch. > > + */ > > + if (crtc_state->pixel_rate < old_crtc_state->pixel_rate || > > + intel_fuzzy_clock_check(crtc_state->pixel_rate, > > + old_crtc_state->pixel_rate)) > > + crtc_state->pixel_rate = old_crtc_state->pixel_rate; > > + } > > +} > > + > > +static void intel_crtc_get_pixel_rate(struct intel_crtc_state > > +*crtc_state) { > > + if (crtc_state->seamless_mode_switch) { > > + struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc); > > + > > + mutex_lock(&crtc->drrs.mutex); > > + crtc_state->pixel_rate = crtc- > > >drrs.seamless_mode_switch_pixel_rate; > > + mutex_unlock(&crtc->drrs.mutex); > > + > > + return; > > + } > > + > > + intel_crtc_compute_pixel_rate(crtc_state); > > } > > > > static void intel_bigjoiner_adjust_timings(const struct > > intel_crtc_state *crtc_state, @@ -2705,7 +2743,7 @@ static void > > intel_crtc_readout_derived_state(struct intel_crtc_state *crtc_state > > intel_bigjoiner_adjust_timings(crtc_state, pipe_mode); > > intel_mode_from_crtc_timings(pipe_mode, pipe_mode); > > > > - intel_crtc_compute_pixel_rate(crtc_state); > > + intel_crtc_get_pixel_rate(crtc_state); > > } > > > > static void intel_encoder_get_config(struct intel_encoder *encoder, > > @@ - > > 6182,11 +6220,13 @@ intel_pipe_config_compare(const struct > > intel_crtc_state *current_config, > > PIPE_CONF_CHECK_I(lane_count); > > PIPE_CONF_CHECK_X(lane_lat_optim_mask); > > > > - if (DISPLAY_VER(dev_priv) >= 9 || IS_BROADWELL(dev_priv)) { > > - PIPE_CONF_CHECK_M_N_ALT(dp_m_n, dp_m2_n2); > > - } else { > > - PIPE_CONF_CHECK_M_N(dp_m_n); > > - PIPE_CONF_CHECK_M_N(dp_m2_n2); > > + if (!pipe_config->seamless_mode_switch) { > > + if (DISPLAY_VER(dev_priv) >= 9 || IS_BROADWELL(dev_priv)) > > { > > + PIPE_CONF_CHECK_M_N_ALT(dp_m_n, dp_m2_n2); > > + } else { > > + PIPE_CONF_CHECK_M_N(dp_m_n); > > + PIPE_CONF_CHECK_M_N(dp_m2_n2); > > + } > > } > > > > PIPE_CONF_CHECK_X(output_types); > > @@ -6348,8 +6388,10 @@ intel_pipe_config_compare(const struct > > intel_crtc_state *current_config, > > if (IS_G4X(dev_priv) || DISPLAY_VER(dev_priv) >= 5) > > PIPE_CONF_CHECK_I(pipe_bpp); > > > > - PIPE_CONF_CHECK_CLOCK_FUZZY(hw.pipe_mode.crtc_clock); > > - PIPE_CONF_CHECK_CLOCK_FUZZY(hw.adjusted_mode.crtc_clock); > > + if (!pipe_config->seamless_mode_switch) { > > + PIPE_CONF_CHECK_CLOCK_FUZZY(hw.pipe_mode.crtc_clock); > > + > > PIPE_CONF_CHECK_CLOCK_FUZZY(hw.adjusted_mode.crtc_clock); > > + } > > PIPE_CONF_CHECK_CLOCK_FUZZY(port_clock); > > > > PIPE_CONF_CHECK_I(min_voltage_level); > > @@ -7085,11 +7127,12 @@ static void intel_crtc_copy_fastset(const > > struct intel_crtc_state *old_crtc_stat > > * FIXME: should really copy more fuzzy state here > > */ > > new_crtc_state->fdi_m_n = old_crtc_state->fdi_m_n; > > - new_crtc_state->dp_m_n = old_crtc_state->dp_m_n; > > > > if (!intel_drrs_crtc_copy_fastset(old_crtc_state, new_crtc_state)) { > > + new_crtc_state->dp_m_n = old_crtc_state->dp_m_n; > > new_crtc_state->dp_m2_n2 = old_crtc_state->dp_m2_n2; > > new_crtc_state->has_drrs = old_crtc_state->has_drrs; > > + new_crtc_state->seamless_mode_switch = > > +old_crtc_state->seamless_mode_switch; > > } > > } > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display_debugfs.c > > b/drivers/gpu/drm/i915/display/intel_display_debugfs.c > > index 452d773fd4e34..92099e1f0b23e 100644 > > --- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c > > +++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c > > @@ -1108,6 +1108,9 @@ static int i915_drrs_status(struct seq_file *m, > > void > > *unused) > > crtc->drrs.refresh_rate == > > DRRS_REFRESH_RATE_LOW ? > > "low" : "high"); > > > > + seq_printf(m, "Seamless mode switch enabled: %s\n", > > + str_yes_no(crtc- > > >drrs.seamless_mode_switch_pixel_rate)); > > + > > mutex_unlock(&crtc->drrs.mutex); > > } > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h > > b/drivers/gpu/drm/i915/display/intel_display_types.h > > index 408152f9f46a4..e290366adfc27 100644 > > --- a/drivers/gpu/drm/i915/display/intel_display_types.h > > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h > > @@ -1057,6 +1057,7 @@ struct intel_crtc_state { > > /* m2_n2 for eDP downclock */ > > struct intel_link_m_n dp_m2_n2; > > bool has_drrs; > > + bool seamless_mode_switch; > > > > /* PSR is supported but might not be enabled due the lack of enabled > > planes */ > > bool has_psr; > > @@ -1314,6 +1315,8 @@ struct intel_crtc { > > unsigned int busy_frontbuffer_bits; > > enum transcoder cpu_transcoder; > > struct intel_link_m_n m_n, m2_n2; > > + /* Only used to verify if programmed state matches */ > > + unsigned int seamless_mode_switch_pixel_rate; > > } drrs; > > > > int scanline_offset; > > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c > > b/drivers/gpu/drm/i915/display/intel_dp.c > > index cc9be82e128f4..691d362dde95e 100644 > > --- a/drivers/gpu/drm/i915/display/intel_dp.c > > +++ b/drivers/gpu/drm/i915/display/intel_dp.c > > @@ -1844,8 +1844,7 @@ static bool cpu_transcoder_has_drrs(struct > > drm_i915_private *i915, } > > > > static bool can_enable_drrs(struct intel_connector *connector, > > - const struct intel_crtc_state *pipe_config, > > - const struct drm_display_mode > > *downclock_mode) > > + const struct intel_crtc_state *pipe_config) > > { > > struct drm_i915_private *i915 = to_i915(connector->base.dev); > > > > @@ -1868,8 +1867,40 @@ static bool can_enable_drrs(struct > > intel_connector *connector, > > if (!cpu_transcoder_has_drrs(i915, pipe_config->cpu_transcoder)) > > return false; > > > > - return downclock_mode && > > - intel_panel_drrs_type(connector) == DRRS_TYPE_SEAMLESS; > > + return intel_panel_drrs_type(connector) == DRRS_TYPE_SEAMLESS; } > > + > > +static bool can_seamless_switch_mode(struct intel_crtc_state > > +*pipe_config) { > > + struct drm_i915_private *i915 = to_i915(pipe_config->uapi.crtc- > > >dev); > > + const struct intel_crtc_state *old_pipe_config; > > + struct intel_atomic_state *state; > > + struct intel_crtc *intel_crtc; > > + > > + /* Only supported in platforms that can switch m_n on the fly */ > > + if (intel_cpu_transcoder_has_m2_n2(i915, pipe_config- > > >cpu_transcoder)) > > + return false; > > + > > + state = to_intel_atomic_state(pipe_config->uapi.state); > > + if (state->base.allow_modeset) > > + return false; > > + > > + intel_crtc = to_intel_crtc(pipe_config->uapi.crtc); > > + old_pipe_config = intel_atomic_get_old_crtc_state(state, intel_crtc); > > + if (drm_mode_equal(&old_pipe_config->hw.adjusted_mode, > > + &pipe_config->hw.adjusted_mode)) > > + return false; > > + > > + /* Only clock can change */ > > + if (!drm_mode_match(&old_pipe_config->hw.adjusted_mode, > > + &pipe_config->hw.adjusted_mode, > > + DRM_MODE_MATCH_TIMINGS | > > + DRM_MODE_MATCH_FLAGS | > > + DRM_MODE_MATCH_3D_FLAGS | > > + DRM_MODE_MATCH_ASPECT_RATIO)) > > + return false; > > + > > + return true; > > } > > > > static void > > @@ -1882,12 +1913,17 @@ intel_dp_drrs_compute_config(struct > > intel_connector *connector, > > intel_panel_downclock_mode(connector, &pipe_config- > > >hw.adjusted_mode); > > int pixel_clock; > > > > - if (!can_enable_drrs(connector, pipe_config, downclock_mode)) { > > - if (intel_cpu_transcoder_has_m2_n2(i915, pipe_config- > > >cpu_transcoder)) > > - intel_zero_m_n(&pipe_config->dp_m2_n2); > > + if (!can_enable_drrs(connector, pipe_config)) > > + goto not_supported; > > + > > + if (can_seamless_switch_mode(pipe_config)) { > > + pipe_config->seamless_mode_switch = true; > > return; > > } > > > > + if (!downclock_mode) > > + goto not_supported; > > + > > if (IS_IRONLAKE(i915) || IS_SANDYBRIDGE(i915) || > > IS_IVYBRIDGE(i915)) > > pipe_config->msa_timing_delay = i915- > > >vbt.edp.drrs_msa_timing_delay; > > > > @@ -1904,6 +1940,12 @@ intel_dp_drrs_compute_config(struct > > intel_connector *connector, > > /* FIXME: abstract this better */ > > if (pipe_config->splitter.enable) > > pipe_config->dp_m2_n2.data_m *= pipe_config- > > >splitter.link_count; > > + > > + return; > > + > > +not_supported: > > + if (intel_cpu_transcoder_has_m2_n2(i915, pipe_config- > > >cpu_transcoder)) > > + intel_zero_m_n(&pipe_config->dp_m2_n2); > > } > > > > static bool intel_dp_has_audio(struct intel_encoder *encoder, diff > > --git a/drivers/gpu/drm/i915/display/intel_drrs.c > > b/drivers/gpu/drm/i915/display/intel_drrs.c > > index d266fad83a086..ffd51d2f5998d 100644 > > --- a/drivers/gpu/drm/i915/display/intel_drrs.c > > +++ b/drivers/gpu/drm/i915/display/intel_drrs.c > > @@ -144,16 +144,19 @@ void intel_drrs_activate(const struct > > intel_crtc_state *crtc_state) { > > struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc); > > > > + mutex_lock(&crtc->drrs.mutex); > > + > > + crtc->drrs.seamless_mode_switch_pixel_rate = crtc_state- > > >seamless_mode_switch ? > > + crtc_state->pixel_rate : 0; > > + > > if (!crtc_state->has_drrs) > > - return; > > + goto unlock; > > > > if (!crtc_state->hw.active) > > - return; > > + goto unlock; > > > > if (intel_crtc_is_bigjoiner_slave(crtc_state)) > > - return; > > - > > - mutex_lock(&crtc->drrs.mutex); > > + goto unlock; > > > > crtc->drrs.cpu_transcoder = crtc_state->cpu_transcoder; > > crtc->drrs.m_n = crtc_state->dp_m_n; @@ -163,6 +166,7 @@ void > > intel_drrs_activate(const struct intel_crtc_state > > *crtc_state) > > > > intel_drrs_schedule_work(crtc); > > > > +unlock: > > mutex_unlock(&crtc->drrs.mutex); > > } > > > > @@ -176,7 +180,7 @@ void intel_drrs_deactivate(const struct > > intel_crtc_state *old_crtc_state) { > > struct intel_crtc *crtc = to_intel_crtc(old_crtc_state->uapi.crtc); > > > > - if (!old_crtc_state->has_drrs) > > + if (!old_crtc_state->has_drrs && > > +!old_crtc_state->seamless_mode_switch) > > return; > > > > if (!old_crtc_state->hw.active) > > @@ -187,12 +191,14 @@ void intel_drrs_deactivate(const struct > > intel_crtc_state *old_crtc_state) > > > > mutex_lock(&crtc->drrs.mutex); > > > > - if (intel_drrs_is_active(crtc)) > > + if (intel_drrs_is_active(crtc) && > > + old_crtc_state->seamless_mode_switch == 0) > > intel_drrs_set_state(crtc, DRRS_REFRESH_RATE_HIGH); > > > > crtc->drrs.cpu_transcoder = INVALID_TRANSCODER; > > crtc->drrs.frontbuffer_bits = 0; > > crtc->drrs.busy_frontbuffer_bits = 0; > > + crtc->drrs.seamless_mode_switch_pixel_rate = 0; > > > > mutex_unlock(&crtc->drrs.mutex); > > > > @@ -322,5 +328,23 @@ bool intel_drrs_crtc_copy_fastset(const struct > > intel_crtc_state *old_crtc_state, > > if (intel_cpu_transcoder_has_m2_n2(i915, new_crtc_state- > > >cpu_transcoder)) > > return false; > > > > + if (!new_crtc_state->seamless_mode_switch) > > + new_crtc_state->dp_m_n = old_crtc_state->dp_m_n; > > + > > return true; > > } > > + > > +void intel_drrs_get_config(struct intel_encoder *encoder, > > + struct intel_crtc_state *pipe_config) { > > + struct intel_crtc *crtc; > > + > > + crtc = to_intel_crtc(pipe_config->uapi.crtc); > > + > > + mutex_lock(&crtc->drrs.mutex); > > + > > + pipe_config->seamless_mode_switch = crtc- > > >drrs.seamless_mode_switch_pixel_rate; > > + pipe_config->has_drrs = intel_drrs_is_active(crtc); > > + > > + mutex_unlock(&crtc->drrs.mutex); > > +} > > diff --git a/drivers/gpu/drm/i915/display/intel_drrs.h > > b/drivers/gpu/drm/i915/display/intel_drrs.h > > index 749ac717db063..a100023658a5a 100644 > > --- a/drivers/gpu/drm/i915/display/intel_drrs.h > > +++ b/drivers/gpu/drm/i915/display/intel_drrs.h > > @@ -14,6 +14,7 @@ struct intel_atomic_state; struct intel_crtc; > > struct intel_crtc_state; struct intel_connector; > > +struct intel_encoder; > > > > const char *intel_drrs_type_str(enum drrs_type drrs_type); bool > > intel_drrs_is_active(struct intel_crtc *crtc); @@ -26,5 +27,7 @@ void > > intel_drrs_flush(struct drm_i915_private *dev_priv, void > > intel_crtc_drrs_init(struct intel_crtc *crtc); bool > > intel_drrs_crtc_copy_fastset(const struct intel_crtc_state *old_crtc_state, > > struct intel_crtc_state *new_crtc_state); > > +void intel_drrs_get_config(struct intel_encoder *encoder, > > + struct intel_crtc_state *pipe_config); > > > > #endif /* __INTEL_DRRS_H__ */ > > -- > > 2.36.0
On Wed, 25 May 2022, "Srinivas, Vidya" <vidya.srinivas@intel.com> wrote: > Hello, > > Apologies for bothering. May we kindly know if this solution is approved? > I have provided the Tested-by. Thanks much. IIUC the more complete solution is [1]. BR, Jani. [1] https://patchwork.freedesktop.org/series/103491/ > > Regards > Vidya > >> -----Original Message----- >> From: Srinivas, Vidya >> Sent: Monday, May 9, 2022 8:44 PM >> To: Souza, Jose <jose.souza@intel.com>; intel-gfx@lists.freedesktop.org >> Cc: Sean Paul <seanpaul@chromium.org>; Ville Syrjälä >> <ville.syrjala@linux.intel.com> >> Subject: RE: [PATCH v2 3/3] drm/i915/display: Implement seamless mode >> switch >> >> Hello Jose, >> >> Thanks much for the patch. I tested it on chrome system and the patch >> works. >> Adding my Tested-by. >> Tested-by: Vidya Srinivas <vidya.srinivas@intel.com> >> >> Regards >> Vidya >> >> > -----Original Message----- >> > From: Souza, Jose <jose.souza@intel.com> >> > Sent: Tuesday, May 3, 2022 2:11 AM >> > To: intel-gfx@lists.freedesktop.org >> > Cc: Srinivas, Vidya <vidya.srinivas@intel.com>; Sean Paul >> > <seanpaul@chromium.org>; Ville Syrjälä >> > <ville.syrjala@linux.intel.com>; Souza, Jose <jose.souza@intel.com> >> > Subject: [PATCH v2 3/3] drm/i915/display: Implement seamless mode >> > switch >> > >> > So far the i915's DRRS feature was automatically changing between >> > preferred panel mode and downclock mode based on idleness but >> ChromeOS >> > compositor team is asking to be in control of the mode switch. >> > So for certain types of content it can switch to a mode with a lower >> > refresh rate without the user noticing a thing and saving more power. >> > >> > This seamless mode switch will be triggered when user-space dispatches >> > an atomic commit with the new mode and clears the >> > DRM_MODE_ATOMIC_ALLOW_MODESET flag. >> > >> > The main steps to acomplish that are: >> > >> > - as mode changed in atomic state drm_atomic_helper_check_modeset() >> > will set mode_changed and it will trigger the crtc state computation >> > so >> > intel_dp_compute_config() will be called and dp_m_n will be computed >> > for the new mode >> > >> > - then intel_dp_drrs_compute_config() will check for the necessary >> > conditions to do a seamless mode switch, if possible crtc_state- >> > >seamless_mode_switch will be set and has_drrs will not be set, so >> > >i915 will >> > not automatically switch between modes >> > >> > - then intel_crtc_compute_config() will call >> > intel_crtc_compute_pixel_rate() that will take the hint that it is >> > trying to do a seamless mode switch and set pixel_rate to the >> > pixel_rate of the old state >> > >> > - then if nothing else changed in the state intel_crtc_check_fastset() >> > will be able to set mode_changed to false and i915 can do fastset >> > otherwise the commit will fail during the check phase >> > >> > - now on the atomic commit phase, intel_ddi_update_pipe_dp() will be >> > called and will program the new dp_m_n >> > >> > - nothing else is different in the commit phase until the step to >> > verify programmed state, the most important change here is that need >> > to save the pixel_rate in DRRS global state as there is no other way >> > to get that from hardware or atomic state >> > >> > v2: >> > - not overwritten dp_m_n values in intel_crtc_copy_fastset() when >> > doing a seamless_mode_switch >> > >> > Cc: Vidya Srinivas <vidya.srinivas@intel.com> >> > Cc: Sean Paul <seanpaul@chromium.org> >> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> >> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com> >> > --- >> > drivers/gpu/drm/i915/display/intel_ddi.c | 10 +++ >> > drivers/gpu/drm/i915/display/intel_display.c | 61 >> > ++++++++++++++++--- .../drm/i915/display/intel_display_debugfs.c | 3 + >> > .../drm/i915/display/intel_display_types.h | 3 + >> > drivers/gpu/drm/i915/display/intel_dp.c | 56 ++++++++++++++--- >> > drivers/gpu/drm/i915/display/intel_drrs.c | 38 +++++++++--- >> > drivers/gpu/drm/i915/display/intel_drrs.h | 3 + >> > 7 files changed, 151 insertions(+), 23 deletions(-) >> > >> > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c >> > b/drivers/gpu/drm/i915/display/intel_ddi.c >> > index 9e6fa59eabba7..732e5d425412e 100644 >> > --- a/drivers/gpu/drm/i915/display/intel_ddi.c >> > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c >> > @@ -46,6 +46,7 @@ >> > #include "intel_dp_link_training.h" >> > #include "intel_dp_mst.h" >> > #include "intel_dpio_phy.h" >> > +#include "intel_drrs.h" >> > #include "intel_dsi.h" >> > #include "intel_fdi.h" >> > #include "intel_fifo_underrun.h" >> > @@ -3010,6 +3011,14 @@ static void intel_ddi_update_pipe_dp(struct >> > intel_atomic_state *state, >> > >> > intel_backlight_update(state, encoder, crtc_state, conn_state); >> > drm_connector_update_privacy_screen(conn_state); >> > + >> > + if (crtc_state->seamless_mode_switch) { >> > + struct intel_crtc *intel_crtc = to_intel_crtc(crtc_state- >> > >uapi.crtc); >> > + >> > + intel_cpu_transcoder_set_m1_n1(intel_crtc, >> > + crtc_state->cpu_transcoder, >> > + &crtc_state->dp_m_n); >> > + } >> > } >> > >> > void intel_ddi_update_pipe(struct intel_atomic_state *state, @@ >> > -3484,6 >> > +3493,7 @@ static void intel_ddi_get_config(struct intel_encoder >> > +*encoder, >> > intel_read_dp_sdp(encoder, pipe_config, DP_SDP_VSC); >> > >> > intel_psr_get_config(encoder, pipe_config); >> > + intel_drrs_get_config(encoder, pipe_config); >> > } >> > >> > void intel_ddi_get_clock(struct intel_encoder *encoder, diff --git >> > a/drivers/gpu/drm/i915/display/intel_display.c >> > b/drivers/gpu/drm/i915/display/intel_display.c >> > index 17d0cad9e1686..e54fbe3b1d394 100644 >> > --- a/drivers/gpu/drm/i915/display/intel_display.c >> > +++ b/drivers/gpu/drm/i915/display/intel_display.c >> > @@ -2629,6 +2629,44 @@ static void >> > intel_crtc_compute_pixel_rate(struct >> > intel_crtc_state *crtc_state) >> > else >> > crtc_state->pixel_rate = >> > ilk_pipe_pixel_rate(crtc_state); >> > + >> > + /* >> > + * Do not change pixel_rate when doing seamless mode switch, >> > otherwise >> > + * it will change port_clock and other stuff that will need a modeset >> > + * to be programmed >> > + */ >> > + if (crtc_state->seamless_mode_switch) { >> > + struct intel_atomic_state *state = >> > to_intel_atomic_state(crtc_state->uapi.state); >> > + struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc); >> > + const struct intel_crtc_state *old_crtc_state; >> > + >> > + old_crtc_state = intel_atomic_get_old_crtc_state(state, crtc); >> > + >> > + /* >> > + * It can only seamless switch if pixel rate of the new mode is >> > + * <= than old one, if not commit will fail with pixel_rate >> > + * mismatch. >> > + */ >> > + if (crtc_state->pixel_rate < old_crtc_state->pixel_rate || >> > + intel_fuzzy_clock_check(crtc_state->pixel_rate, >> > + old_crtc_state->pixel_rate)) >> > + crtc_state->pixel_rate = old_crtc_state->pixel_rate; >> > + } >> > +} >> > + >> > +static void intel_crtc_get_pixel_rate(struct intel_crtc_state >> > +*crtc_state) { >> > + if (crtc_state->seamless_mode_switch) { >> > + struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc); >> > + >> > + mutex_lock(&crtc->drrs.mutex); >> > + crtc_state->pixel_rate = crtc- >> > >drrs.seamless_mode_switch_pixel_rate; >> > + mutex_unlock(&crtc->drrs.mutex); >> > + >> > + return; >> > + } >> > + >> > + intel_crtc_compute_pixel_rate(crtc_state); >> > } >> > >> > static void intel_bigjoiner_adjust_timings(const struct >> > intel_crtc_state *crtc_state, @@ -2705,7 +2743,7 @@ static void >> > intel_crtc_readout_derived_state(struct intel_crtc_state *crtc_state >> > intel_bigjoiner_adjust_timings(crtc_state, pipe_mode); >> > intel_mode_from_crtc_timings(pipe_mode, pipe_mode); >> > >> > - intel_crtc_compute_pixel_rate(crtc_state); >> > + intel_crtc_get_pixel_rate(crtc_state); >> > } >> > >> > static void intel_encoder_get_config(struct intel_encoder *encoder, >> > @@ - >> > 6182,11 +6220,13 @@ intel_pipe_config_compare(const struct >> > intel_crtc_state *current_config, >> > PIPE_CONF_CHECK_I(lane_count); >> > PIPE_CONF_CHECK_X(lane_lat_optim_mask); >> > >> > - if (DISPLAY_VER(dev_priv) >= 9 || IS_BROADWELL(dev_priv)) { >> > - PIPE_CONF_CHECK_M_N_ALT(dp_m_n, dp_m2_n2); >> > - } else { >> > - PIPE_CONF_CHECK_M_N(dp_m_n); >> > - PIPE_CONF_CHECK_M_N(dp_m2_n2); >> > + if (!pipe_config->seamless_mode_switch) { >> > + if (DISPLAY_VER(dev_priv) >= 9 || IS_BROADWELL(dev_priv)) >> > { >> > + PIPE_CONF_CHECK_M_N_ALT(dp_m_n, dp_m2_n2); >> > + } else { >> > + PIPE_CONF_CHECK_M_N(dp_m_n); >> > + PIPE_CONF_CHECK_M_N(dp_m2_n2); >> > + } >> > } >> > >> > PIPE_CONF_CHECK_X(output_types); >> > @@ -6348,8 +6388,10 @@ intel_pipe_config_compare(const struct >> > intel_crtc_state *current_config, >> > if (IS_G4X(dev_priv) || DISPLAY_VER(dev_priv) >= 5) >> > PIPE_CONF_CHECK_I(pipe_bpp); >> > >> > - PIPE_CONF_CHECK_CLOCK_FUZZY(hw.pipe_mode.crtc_clock); >> > - PIPE_CONF_CHECK_CLOCK_FUZZY(hw.adjusted_mode.crtc_clock); >> > + if (!pipe_config->seamless_mode_switch) { >> > + PIPE_CONF_CHECK_CLOCK_FUZZY(hw.pipe_mode.crtc_clock); >> > + >> > PIPE_CONF_CHECK_CLOCK_FUZZY(hw.adjusted_mode.crtc_clock); >> > + } >> > PIPE_CONF_CHECK_CLOCK_FUZZY(port_clock); >> > >> > PIPE_CONF_CHECK_I(min_voltage_level); >> > @@ -7085,11 +7127,12 @@ static void intel_crtc_copy_fastset(const >> > struct intel_crtc_state *old_crtc_stat >> > * FIXME: should really copy more fuzzy state here >> > */ >> > new_crtc_state->fdi_m_n = old_crtc_state->fdi_m_n; >> > - new_crtc_state->dp_m_n = old_crtc_state->dp_m_n; >> > >> > if (!intel_drrs_crtc_copy_fastset(old_crtc_state, new_crtc_state)) { >> > + new_crtc_state->dp_m_n = old_crtc_state->dp_m_n; >> > new_crtc_state->dp_m2_n2 = old_crtc_state->dp_m2_n2; >> > new_crtc_state->has_drrs = old_crtc_state->has_drrs; >> > + new_crtc_state->seamless_mode_switch = >> > +old_crtc_state->seamless_mode_switch; >> > } >> > } >> > >> > diff --git a/drivers/gpu/drm/i915/display/intel_display_debugfs.c >> > b/drivers/gpu/drm/i915/display/intel_display_debugfs.c >> > index 452d773fd4e34..92099e1f0b23e 100644 >> > --- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c >> > +++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c >> > @@ -1108,6 +1108,9 @@ static int i915_drrs_status(struct seq_file *m, >> > void >> > *unused) >> > crtc->drrs.refresh_rate == >> > DRRS_REFRESH_RATE_LOW ? >> > "low" : "high"); >> > >> > + seq_printf(m, "Seamless mode switch enabled: %s\n", >> > + str_yes_no(crtc- >> > >drrs.seamless_mode_switch_pixel_rate)); >> > + >> > mutex_unlock(&crtc->drrs.mutex); >> > } >> > >> > diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h >> > b/drivers/gpu/drm/i915/display/intel_display_types.h >> > index 408152f9f46a4..e290366adfc27 100644 >> > --- a/drivers/gpu/drm/i915/display/intel_display_types.h >> > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h >> > @@ -1057,6 +1057,7 @@ struct intel_crtc_state { >> > /* m2_n2 for eDP downclock */ >> > struct intel_link_m_n dp_m2_n2; >> > bool has_drrs; >> > + bool seamless_mode_switch; >> > >> > /* PSR is supported but might not be enabled due the lack of enabled >> > planes */ >> > bool has_psr; >> > @@ -1314,6 +1315,8 @@ struct intel_crtc { >> > unsigned int busy_frontbuffer_bits; >> > enum transcoder cpu_transcoder; >> > struct intel_link_m_n m_n, m2_n2; >> > + /* Only used to verify if programmed state matches */ >> > + unsigned int seamless_mode_switch_pixel_rate; >> > } drrs; >> > >> > int scanline_offset; >> > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c >> > b/drivers/gpu/drm/i915/display/intel_dp.c >> > index cc9be82e128f4..691d362dde95e 100644 >> > --- a/drivers/gpu/drm/i915/display/intel_dp.c >> > +++ b/drivers/gpu/drm/i915/display/intel_dp.c >> > @@ -1844,8 +1844,7 @@ static bool cpu_transcoder_has_drrs(struct >> > drm_i915_private *i915, } >> > >> > static bool can_enable_drrs(struct intel_connector *connector, >> > - const struct intel_crtc_state *pipe_config, >> > - const struct drm_display_mode >> > *downclock_mode) >> > + const struct intel_crtc_state *pipe_config) >> > { >> > struct drm_i915_private *i915 = to_i915(connector->base.dev); >> > >> > @@ -1868,8 +1867,40 @@ static bool can_enable_drrs(struct >> > intel_connector *connector, >> > if (!cpu_transcoder_has_drrs(i915, pipe_config->cpu_transcoder)) >> > return false; >> > >> > - return downclock_mode && >> > - intel_panel_drrs_type(connector) == DRRS_TYPE_SEAMLESS; >> > + return intel_panel_drrs_type(connector) == DRRS_TYPE_SEAMLESS; } >> > + >> > +static bool can_seamless_switch_mode(struct intel_crtc_state >> > +*pipe_config) { >> > + struct drm_i915_private *i915 = to_i915(pipe_config->uapi.crtc- >> > >dev); >> > + const struct intel_crtc_state *old_pipe_config; >> > + struct intel_atomic_state *state; >> > + struct intel_crtc *intel_crtc; >> > + >> > + /* Only supported in platforms that can switch m_n on the fly */ >> > + if (intel_cpu_transcoder_has_m2_n2(i915, pipe_config- >> > >cpu_transcoder)) >> > + return false; >> > + >> > + state = to_intel_atomic_state(pipe_config->uapi.state); >> > + if (state->base.allow_modeset) >> > + return false; >> > + >> > + intel_crtc = to_intel_crtc(pipe_config->uapi.crtc); >> > + old_pipe_config = intel_atomic_get_old_crtc_state(state, intel_crtc); >> > + if (drm_mode_equal(&old_pipe_config->hw.adjusted_mode, >> > + &pipe_config->hw.adjusted_mode)) >> > + return false; >> > + >> > + /* Only clock can change */ >> > + if (!drm_mode_match(&old_pipe_config->hw.adjusted_mode, >> > + &pipe_config->hw.adjusted_mode, >> > + DRM_MODE_MATCH_TIMINGS | >> > + DRM_MODE_MATCH_FLAGS | >> > + DRM_MODE_MATCH_3D_FLAGS | >> > + DRM_MODE_MATCH_ASPECT_RATIO)) >> > + return false; >> > + >> > + return true; >> > } >> > >> > static void >> > @@ -1882,12 +1913,17 @@ intel_dp_drrs_compute_config(struct >> > intel_connector *connector, >> > intel_panel_downclock_mode(connector, &pipe_config- >> > >hw.adjusted_mode); >> > int pixel_clock; >> > >> > - if (!can_enable_drrs(connector, pipe_config, downclock_mode)) { >> > - if (intel_cpu_transcoder_has_m2_n2(i915, pipe_config- >> > >cpu_transcoder)) >> > - intel_zero_m_n(&pipe_config->dp_m2_n2); >> > + if (!can_enable_drrs(connector, pipe_config)) >> > + goto not_supported; >> > + >> > + if (can_seamless_switch_mode(pipe_config)) { >> > + pipe_config->seamless_mode_switch = true; >> > return; >> > } >> > >> > + if (!downclock_mode) >> > + goto not_supported; >> > + >> > if (IS_IRONLAKE(i915) || IS_SANDYBRIDGE(i915) || >> > IS_IVYBRIDGE(i915)) >> > pipe_config->msa_timing_delay = i915- >> > >vbt.edp.drrs_msa_timing_delay; >> > >> > @@ -1904,6 +1940,12 @@ intel_dp_drrs_compute_config(struct >> > intel_connector *connector, >> > /* FIXME: abstract this better */ >> > if (pipe_config->splitter.enable) >> > pipe_config->dp_m2_n2.data_m *= pipe_config- >> > >splitter.link_count; >> > + >> > + return; >> > + >> > +not_supported: >> > + if (intel_cpu_transcoder_has_m2_n2(i915, pipe_config- >> > >cpu_transcoder)) >> > + intel_zero_m_n(&pipe_config->dp_m2_n2); >> > } >> > >> > static bool intel_dp_has_audio(struct intel_encoder *encoder, diff >> > --git a/drivers/gpu/drm/i915/display/intel_drrs.c >> > b/drivers/gpu/drm/i915/display/intel_drrs.c >> > index d266fad83a086..ffd51d2f5998d 100644 >> > --- a/drivers/gpu/drm/i915/display/intel_drrs.c >> > +++ b/drivers/gpu/drm/i915/display/intel_drrs.c >> > @@ -144,16 +144,19 @@ void intel_drrs_activate(const struct >> > intel_crtc_state *crtc_state) { >> > struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc); >> > >> > + mutex_lock(&crtc->drrs.mutex); >> > + >> > + crtc->drrs.seamless_mode_switch_pixel_rate = crtc_state- >> > >seamless_mode_switch ? >> > + crtc_state->pixel_rate : 0; >> > + >> > if (!crtc_state->has_drrs) >> > - return; >> > + goto unlock; >> > >> > if (!crtc_state->hw.active) >> > - return; >> > + goto unlock; >> > >> > if (intel_crtc_is_bigjoiner_slave(crtc_state)) >> > - return; >> > - >> > - mutex_lock(&crtc->drrs.mutex); >> > + goto unlock; >> > >> > crtc->drrs.cpu_transcoder = crtc_state->cpu_transcoder; >> > crtc->drrs.m_n = crtc_state->dp_m_n; @@ -163,6 +166,7 @@ void >> > intel_drrs_activate(const struct intel_crtc_state >> > *crtc_state) >> > >> > intel_drrs_schedule_work(crtc); >> > >> > +unlock: >> > mutex_unlock(&crtc->drrs.mutex); >> > } >> > >> > @@ -176,7 +180,7 @@ void intel_drrs_deactivate(const struct >> > intel_crtc_state *old_crtc_state) { >> > struct intel_crtc *crtc = to_intel_crtc(old_crtc_state->uapi.crtc); >> > >> > - if (!old_crtc_state->has_drrs) >> > + if (!old_crtc_state->has_drrs && >> > +!old_crtc_state->seamless_mode_switch) >> > return; >> > >> > if (!old_crtc_state->hw.active) >> > @@ -187,12 +191,14 @@ void intel_drrs_deactivate(const struct >> > intel_crtc_state *old_crtc_state) >> > >> > mutex_lock(&crtc->drrs.mutex); >> > >> > - if (intel_drrs_is_active(crtc)) >> > + if (intel_drrs_is_active(crtc) && >> > + old_crtc_state->seamless_mode_switch == 0) >> > intel_drrs_set_state(crtc, DRRS_REFRESH_RATE_HIGH); >> > >> > crtc->drrs.cpu_transcoder = INVALID_TRANSCODER; >> > crtc->drrs.frontbuffer_bits = 0; >> > crtc->drrs.busy_frontbuffer_bits = 0; >> > + crtc->drrs.seamless_mode_switch_pixel_rate = 0; >> > >> > mutex_unlock(&crtc->drrs.mutex); >> > >> > @@ -322,5 +328,23 @@ bool intel_drrs_crtc_copy_fastset(const struct >> > intel_crtc_state *old_crtc_state, >> > if (intel_cpu_transcoder_has_m2_n2(i915, new_crtc_state- >> > >cpu_transcoder)) >> > return false; >> > >> > + if (!new_crtc_state->seamless_mode_switch) >> > + new_crtc_state->dp_m_n = old_crtc_state->dp_m_n; >> > + >> > return true; >> > } >> > + >> > +void intel_drrs_get_config(struct intel_encoder *encoder, >> > + struct intel_crtc_state *pipe_config) { >> > + struct intel_crtc *crtc; >> > + >> > + crtc = to_intel_crtc(pipe_config->uapi.crtc); >> > + >> > + mutex_lock(&crtc->drrs.mutex); >> > + >> > + pipe_config->seamless_mode_switch = crtc- >> > >drrs.seamless_mode_switch_pixel_rate; >> > + pipe_config->has_drrs = intel_drrs_is_active(crtc); >> > + >> > + mutex_unlock(&crtc->drrs.mutex); >> > +} >> > diff --git a/drivers/gpu/drm/i915/display/intel_drrs.h >> > b/drivers/gpu/drm/i915/display/intel_drrs.h >> > index 749ac717db063..a100023658a5a 100644 >> > --- a/drivers/gpu/drm/i915/display/intel_drrs.h >> > +++ b/drivers/gpu/drm/i915/display/intel_drrs.h >> > @@ -14,6 +14,7 @@ struct intel_atomic_state; struct intel_crtc; >> > struct intel_crtc_state; struct intel_connector; >> > +struct intel_encoder; >> > >> > const char *intel_drrs_type_str(enum drrs_type drrs_type); bool >> > intel_drrs_is_active(struct intel_crtc *crtc); @@ -26,5 +27,7 @@ void >> > intel_drrs_flush(struct drm_i915_private *dev_priv, void >> > intel_crtc_drrs_init(struct intel_crtc *crtc); bool >> > intel_drrs_crtc_copy_fastset(const struct intel_crtc_state *old_crtc_state, >> > struct intel_crtc_state *new_crtc_state); >> > +void intel_drrs_get_config(struct intel_encoder *encoder, >> > + struct intel_crtc_state *pipe_config); >> > >> > #endif /* __INTEL_DRRS_H__ */ >> > -- >> > 2.36.0 >
> -----Original Message----- > From: Jani Nikula <jani.nikula@linux.intel.com> > Sent: Wednesday, May 25, 2022 2:26 PM > To: Srinivas, Vidya <vidya.srinivas@intel.com>; Souza, Jose > <jose.souza@intel.com>; intel-gfx@lists.freedesktop.org > Cc: Sean Paul <seanpaul@chromium.org>; Syrjala, Ville > <ville.syrjala@intel.com> > Subject: Re: [Intel-gfx] [PATCH v2 3/3] drm/i915/display: Implement seamless > mode switch > > On Wed, 25 May 2022, "Srinivas, Vidya" <vidya.srinivas@intel.com> wrote: > > Hello, > > > > Apologies for bothering. May we kindly know if this solution is approved? > > I have provided the Tested-by. Thanks much. > > IIUC the more complete solution is [1]. > > BR, > Jani. > > > [1] https://patchwork.freedesktop.org/series/103491/ > Hello Jani, Thank you very much. I will have a check of this. For user space triggered seamless modeset WITHOUT setting "DRM_MODE_ATOMIC_ALLOW_MODESET", for switch between 2 refresh rates, Jose patches is working. Jose please correct me if I am wrong. Thank you. Regards Vidya > > > > > > Regards > > Vidya > > > >> -----Original Message----- > >> From: Srinivas, Vidya > >> Sent: Monday, May 9, 2022 8:44 PM > >> To: Souza, Jose <jose.souza@intel.com>; > >> intel-gfx@lists.freedesktop.org > >> Cc: Sean Paul <seanpaul@chromium.org>; Ville Syrjälä > >> <ville.syrjala@linux.intel.com> > >> Subject: RE: [PATCH v2 3/3] drm/i915/display: Implement seamless mode > >> switch > >> > >> Hello Jose, > >> > >> Thanks much for the patch. I tested it on chrome system and the patch > >> works. > >> Adding my Tested-by. > >> Tested-by: Vidya Srinivas <vidya.srinivas@intel.com> > >> > >> Regards > >> Vidya > >> > >> > -----Original Message----- > >> > From: Souza, Jose <jose.souza@intel.com> > >> > Sent: Tuesday, May 3, 2022 2:11 AM > >> > To: intel-gfx@lists.freedesktop.org > >> > Cc: Srinivas, Vidya <vidya.srinivas@intel.com>; Sean Paul > >> > <seanpaul@chromium.org>; Ville Syrjälä > >> > <ville.syrjala@linux.intel.com>; Souza, Jose <jose.souza@intel.com> > >> > Subject: [PATCH v2 3/3] drm/i915/display: Implement seamless mode > >> > switch > >> > > >> > So far the i915's DRRS feature was automatically changing between > >> > preferred panel mode and downclock mode based on idleness but > >> ChromeOS > >> > compositor team is asking to be in control of the mode switch. > >> > So for certain types of content it can switch to a mode with a > >> > lower refresh rate without the user noticing a thing and saving more > power. > >> > > >> > This seamless mode switch will be triggered when user-space > >> > dispatches an atomic commit with the new mode and clears the > >> > DRM_MODE_ATOMIC_ALLOW_MODESET flag. > >> > > >> > The main steps to acomplish that are: > >> > > >> > - as mode changed in atomic state > drm_atomic_helper_check_modeset() > >> > will set mode_changed and it will trigger the crtc state > >> > computation so > >> > intel_dp_compute_config() will be called and dp_m_n will be > >> > computed for the new mode > >> > > >> > - then intel_dp_drrs_compute_config() will check for the necessary > >> > conditions to do a seamless mode switch, if possible crtc_state- > >> > >seamless_mode_switch will be set and has_drrs will not be set, so > >> > >i915 will > >> > not automatically switch between modes > >> > > >> > - then intel_crtc_compute_config() will call > >> > intel_crtc_compute_pixel_rate() that will take the hint that it is > >> > trying to do a seamless mode switch and set pixel_rate to the > >> > pixel_rate of the old state > >> > > >> > - then if nothing else changed in the state > >> > intel_crtc_check_fastset() will be able to set mode_changed to > >> > false and i915 can do fastset otherwise the commit will fail during > >> > the check phase > >> > > >> > - now on the atomic commit phase, intel_ddi_update_pipe_dp() will > >> > be called and will program the new dp_m_n > >> > > >> > - nothing else is different in the commit phase until the step to > >> > verify programmed state, the most important change here is that > >> > need to save the pixel_rate in DRRS global state as there is no > >> > other way to get that from hardware or atomic state > >> > > >> > v2: > >> > - not overwritten dp_m_n values in intel_crtc_copy_fastset() when > >> > doing a seamless_mode_switch > >> > > >> > Cc: Vidya Srinivas <vidya.srinivas@intel.com> > >> > Cc: Sean Paul <seanpaul@chromium.org> > >> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > >> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com> > >> > --- > >> > drivers/gpu/drm/i915/display/intel_ddi.c | 10 +++ > >> > drivers/gpu/drm/i915/display/intel_display.c | 61 > >> > ++++++++++++++++--- .../drm/i915/display/intel_display_debugfs.c | > >> > ++++++++++++++++3 + > >> > .../drm/i915/display/intel_display_types.h | 3 + > >> > drivers/gpu/drm/i915/display/intel_dp.c | 56 ++++++++++++++--- > >> > drivers/gpu/drm/i915/display/intel_drrs.c | 38 +++++++++--- > >> > drivers/gpu/drm/i915/display/intel_drrs.h | 3 + > >> > 7 files changed, 151 insertions(+), 23 deletions(-) > >> > > >> > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c > >> > b/drivers/gpu/drm/i915/display/intel_ddi.c > >> > index 9e6fa59eabba7..732e5d425412e 100644 > >> > --- a/drivers/gpu/drm/i915/display/intel_ddi.c > >> > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c > >> > @@ -46,6 +46,7 @@ > >> > #include "intel_dp_link_training.h" > >> > #include "intel_dp_mst.h" > >> > #include "intel_dpio_phy.h" > >> > +#include "intel_drrs.h" > >> > #include "intel_dsi.h" > >> > #include "intel_fdi.h" > >> > #include "intel_fifo_underrun.h" > >> > @@ -3010,6 +3011,14 @@ static void intel_ddi_update_pipe_dp(struct > >> > intel_atomic_state *state, > >> > > >> > intel_backlight_update(state, encoder, crtc_state, conn_state); > >> > drm_connector_update_privacy_screen(conn_state); > >> > + > >> > + if (crtc_state->seamless_mode_switch) { > >> > + struct intel_crtc *intel_crtc = to_intel_crtc(crtc_state- > >> > >uapi.crtc); > >> > + > >> > + intel_cpu_transcoder_set_m1_n1(intel_crtc, > >> > + crtc_state->cpu_transcoder, > >> > + &crtc_state->dp_m_n); > >> > + } > >> > } > >> > > >> > void intel_ddi_update_pipe(struct intel_atomic_state *state, @@ > >> > -3484,6 > >> > +3493,7 @@ static void intel_ddi_get_config(struct intel_encoder > >> > +*encoder, > >> > intel_read_dp_sdp(encoder, pipe_config, DP_SDP_VSC); > >> > > >> > intel_psr_get_config(encoder, pipe_config); > >> > + intel_drrs_get_config(encoder, pipe_config); > >> > } > >> > > >> > void intel_ddi_get_clock(struct intel_encoder *encoder, diff --git > >> > a/drivers/gpu/drm/i915/display/intel_display.c > >> > b/drivers/gpu/drm/i915/display/intel_display.c > >> > index 17d0cad9e1686..e54fbe3b1d394 100644 > >> > --- a/drivers/gpu/drm/i915/display/intel_display.c > >> > +++ b/drivers/gpu/drm/i915/display/intel_display.c > >> > @@ -2629,6 +2629,44 @@ static void > >> > intel_crtc_compute_pixel_rate(struct > >> > intel_crtc_state *crtc_state) > >> > else > >> > crtc_state->pixel_rate = > >> > ilk_pipe_pixel_rate(crtc_state); > >> > + > >> > + /* > >> > + * Do not change pixel_rate when doing seamless mode switch, > >> > otherwise > >> > + * it will change port_clock and other stuff that will need a modeset > >> > + * to be programmed > >> > + */ > >> > + if (crtc_state->seamless_mode_switch) { > >> > + struct intel_atomic_state *state = > >> > to_intel_atomic_state(crtc_state->uapi.state); > >> > + struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc); > >> > + const struct intel_crtc_state *old_crtc_state; > >> > + > >> > + old_crtc_state = intel_atomic_get_old_crtc_state(state, crtc); > >> > + > >> > + /* > >> > + * It can only seamless switch if pixel rate of the new mode is > >> > + * <= than old one, if not commit will fail with pixel_rate > >> > + * mismatch. > >> > + */ > >> > + if (crtc_state->pixel_rate < old_crtc_state->pixel_rate || > >> > + intel_fuzzy_clock_check(crtc_state->pixel_rate, > >> > + old_crtc_state->pixel_rate)) > >> > + crtc_state->pixel_rate = old_crtc_state->pixel_rate; > >> > + } > >> > +} > >> > + > >> > +static void intel_crtc_get_pixel_rate(struct intel_crtc_state > >> > +*crtc_state) { > >> > + if (crtc_state->seamless_mode_switch) { > >> > + struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc); > >> > + > >> > + mutex_lock(&crtc->drrs.mutex); > >> > + crtc_state->pixel_rate = crtc- > >> > >drrs.seamless_mode_switch_pixel_rate; > >> > + mutex_unlock(&crtc->drrs.mutex); > >> > + > >> > + return; > >> > + } > >> > + > >> > + intel_crtc_compute_pixel_rate(crtc_state); > >> > } > >> > > >> > static void intel_bigjoiner_adjust_timings(const struct > >> > intel_crtc_state *crtc_state, @@ -2705,7 +2743,7 @@ static void > >> > intel_crtc_readout_derived_state(struct intel_crtc_state *crtc_state > >> > intel_bigjoiner_adjust_timings(crtc_state, pipe_mode); > >> > intel_mode_from_crtc_timings(pipe_mode, pipe_mode); > >> > > >> > - intel_crtc_compute_pixel_rate(crtc_state); > >> > + intel_crtc_get_pixel_rate(crtc_state); > >> > } > >> > > >> > static void intel_encoder_get_config(struct intel_encoder > >> > *encoder, @@ - > >> > 6182,11 +6220,13 @@ intel_pipe_config_compare(const struct > >> > intel_crtc_state *current_config, > >> > PIPE_CONF_CHECK_I(lane_count); > >> > PIPE_CONF_CHECK_X(lane_lat_optim_mask); > >> > > >> > - if (DISPLAY_VER(dev_priv) >= 9 || IS_BROADWELL(dev_priv)) { > >> > - PIPE_CONF_CHECK_M_N_ALT(dp_m_n, dp_m2_n2); > >> > - } else { > >> > - PIPE_CONF_CHECK_M_N(dp_m_n); > >> > - PIPE_CONF_CHECK_M_N(dp_m2_n2); > >> > + if (!pipe_config->seamless_mode_switch) { > >> > + if (DISPLAY_VER(dev_priv) >= 9 || IS_BROADWELL(dev_priv)) > >> > { > >> > + PIPE_CONF_CHECK_M_N_ALT(dp_m_n, dp_m2_n2); > >> > + } else { > >> > + PIPE_CONF_CHECK_M_N(dp_m_n); > >> > + PIPE_CONF_CHECK_M_N(dp_m2_n2); > >> > + } > >> > } > >> > > >> > PIPE_CONF_CHECK_X(output_types); > >> > @@ -6348,8 +6388,10 @@ intel_pipe_config_compare(const struct > >> > intel_crtc_state *current_config, > >> > if (IS_G4X(dev_priv) || DISPLAY_VER(dev_priv) >= 5) > >> > PIPE_CONF_CHECK_I(pipe_bpp); > >> > > >> > - PIPE_CONF_CHECK_CLOCK_FUZZY(hw.pipe_mode.crtc_clock); > >> > - PIPE_CONF_CHECK_CLOCK_FUZZY(hw.adjusted_mode.crtc_clock); > >> > + if (!pipe_config->seamless_mode_switch) { > >> > + PIPE_CONF_CHECK_CLOCK_FUZZY(hw.pipe_mode.crtc_clock); > >> > + > >> > PIPE_CONF_CHECK_CLOCK_FUZZY(hw.adjusted_mode.crtc_clock); > >> > + } > >> > PIPE_CONF_CHECK_CLOCK_FUZZY(port_clock); > >> > > >> > PIPE_CONF_CHECK_I(min_voltage_level); > >> > @@ -7085,11 +7127,12 @@ static void intel_crtc_copy_fastset(const > >> > struct intel_crtc_state *old_crtc_stat > >> > * FIXME: should really copy more fuzzy state here > >> > */ > >> > new_crtc_state->fdi_m_n = old_crtc_state->fdi_m_n; > >> > - new_crtc_state->dp_m_n = old_crtc_state->dp_m_n; > >> > > >> > if (!intel_drrs_crtc_copy_fastset(old_crtc_state, > >> > new_crtc_state)) { > >> > + new_crtc_state->dp_m_n = old_crtc_state->dp_m_n; > >> > new_crtc_state->dp_m2_n2 = old_crtc_state->dp_m2_n2; > >> > new_crtc_state->has_drrs = old_crtc_state->has_drrs; > >> > + new_crtc_state->seamless_mode_switch = > >> > +old_crtc_state->seamless_mode_switch; > >> > } > >> > } > >> > > >> > diff --git a/drivers/gpu/drm/i915/display/intel_display_debugfs.c > >> > b/drivers/gpu/drm/i915/display/intel_display_debugfs.c > >> > index 452d773fd4e34..92099e1f0b23e 100644 > >> > --- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c > >> > +++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c > >> > @@ -1108,6 +1108,9 @@ static int i915_drrs_status(struct seq_file > >> > *m, void > >> > *unused) > >> > crtc->drrs.refresh_rate == > >> > DRRS_REFRESH_RATE_LOW ? > >> > "low" : "high"); > >> > > >> > + seq_printf(m, "Seamless mode switch enabled: %s\n", > >> > + str_yes_no(crtc- > >> > >drrs.seamless_mode_switch_pixel_rate)); > >> > + > >> > mutex_unlock(&crtc->drrs.mutex); > >> > } > >> > > >> > diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h > >> > b/drivers/gpu/drm/i915/display/intel_display_types.h > >> > index 408152f9f46a4..e290366adfc27 100644 > >> > --- a/drivers/gpu/drm/i915/display/intel_display_types.h > >> > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h > >> > @@ -1057,6 +1057,7 @@ struct intel_crtc_state { > >> > /* m2_n2 for eDP downclock */ > >> > struct intel_link_m_n dp_m2_n2; > >> > bool has_drrs; > >> > + bool seamless_mode_switch; > >> > > >> > /* PSR is supported but might not be enabled due the lack of > >> > enabled planes */ > >> > bool has_psr; > >> > @@ -1314,6 +1315,8 @@ struct intel_crtc { > >> > unsigned int busy_frontbuffer_bits; > >> > enum transcoder cpu_transcoder; > >> > struct intel_link_m_n m_n, m2_n2; > >> > + /* Only used to verify if programmed state matches */ > >> > + unsigned int seamless_mode_switch_pixel_rate; > >> > } drrs; > >> > > >> > int scanline_offset; > >> > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c > >> > b/drivers/gpu/drm/i915/display/intel_dp.c > >> > index cc9be82e128f4..691d362dde95e 100644 > >> > --- a/drivers/gpu/drm/i915/display/intel_dp.c > >> > +++ b/drivers/gpu/drm/i915/display/intel_dp.c > >> > @@ -1844,8 +1844,7 @@ static bool cpu_transcoder_has_drrs(struct > >> > drm_i915_private *i915, } > >> > > >> > static bool can_enable_drrs(struct intel_connector *connector, > >> > - const struct intel_crtc_state *pipe_config, > >> > - const struct drm_display_mode > >> > *downclock_mode) > >> > + const struct intel_crtc_state *pipe_config) > >> > { > >> > struct drm_i915_private *i915 = to_i915(connector->base.dev); > >> > > >> > @@ -1868,8 +1867,40 @@ static bool can_enable_drrs(struct > >> > intel_connector *connector, > >> > if (!cpu_transcoder_has_drrs(i915, pipe_config->cpu_transcoder)) > >> > return false; > >> > > >> > - return downclock_mode && > >> > - intel_panel_drrs_type(connector) == DRRS_TYPE_SEAMLESS; > >> > + return intel_panel_drrs_type(connector) == DRRS_TYPE_SEAMLESS; } > >> > + > >> > +static bool can_seamless_switch_mode(struct intel_crtc_state > >> > +*pipe_config) { > >> > + struct drm_i915_private *i915 = to_i915(pipe_config->uapi.crtc- > >> > >dev); > >> > + const struct intel_crtc_state *old_pipe_config; > >> > + struct intel_atomic_state *state; > >> > + struct intel_crtc *intel_crtc; > >> > + > >> > + /* Only supported in platforms that can switch m_n on the fly */ > >> > + if (intel_cpu_transcoder_has_m2_n2(i915, pipe_config- > >> > >cpu_transcoder)) > >> > + return false; > >> > + > >> > + state = to_intel_atomic_state(pipe_config->uapi.state); > >> > + if (state->base.allow_modeset) > >> > + return false; > >> > + > >> > + intel_crtc = to_intel_crtc(pipe_config->uapi.crtc); > >> > + old_pipe_config = intel_atomic_get_old_crtc_state(state, intel_crtc); > >> > + if (drm_mode_equal(&old_pipe_config->hw.adjusted_mode, > >> > + &pipe_config->hw.adjusted_mode)) > >> > + return false; > >> > + > >> > + /* Only clock can change */ > >> > + if (!drm_mode_match(&old_pipe_config->hw.adjusted_mode, > >> > + &pipe_config->hw.adjusted_mode, > >> > + DRM_MODE_MATCH_TIMINGS | > >> > + DRM_MODE_MATCH_FLAGS | > >> > + DRM_MODE_MATCH_3D_FLAGS | > >> > + DRM_MODE_MATCH_ASPECT_RATIO)) > >> > + return false; > >> > + > >> > + return true; > >> > } > >> > > >> > static void > >> > @@ -1882,12 +1913,17 @@ intel_dp_drrs_compute_config(struct > >> > intel_connector *connector, > >> > intel_panel_downclock_mode(connector, &pipe_config- > >> > >hw.adjusted_mode); > >> > int pixel_clock; > >> > > >> > - if (!can_enable_drrs(connector, pipe_config, downclock_mode)) { > >> > - if (intel_cpu_transcoder_has_m2_n2(i915, pipe_config- > >> > >cpu_transcoder)) > >> > - intel_zero_m_n(&pipe_config->dp_m2_n2); > >> > + if (!can_enable_drrs(connector, pipe_config)) > >> > + goto not_supported; > >> > + > >> > + if (can_seamless_switch_mode(pipe_config)) { > >> > + pipe_config->seamless_mode_switch = true; > >> > return; > >> > } > >> > > >> > + if (!downclock_mode) > >> > + goto not_supported; > >> > + > >> > if (IS_IRONLAKE(i915) || IS_SANDYBRIDGE(i915) || > >> > IS_IVYBRIDGE(i915)) > >> > pipe_config->msa_timing_delay = i915- > >> > >vbt.edp.drrs_msa_timing_delay; > >> > > >> > @@ -1904,6 +1940,12 @@ intel_dp_drrs_compute_config(struct > >> > intel_connector *connector, > >> > /* FIXME: abstract this better */ > >> > if (pipe_config->splitter.enable) > >> > pipe_config->dp_m2_n2.data_m *= pipe_config- > >> > >splitter.link_count; > >> > + > >> > + return; > >> > + > >> > +not_supported: > >> > + if (intel_cpu_transcoder_has_m2_n2(i915, pipe_config- > >> > >cpu_transcoder)) > >> > + intel_zero_m_n(&pipe_config->dp_m2_n2); > >> > } > >> > > >> > static bool intel_dp_has_audio(struct intel_encoder *encoder, diff > >> > --git a/drivers/gpu/drm/i915/display/intel_drrs.c > >> > b/drivers/gpu/drm/i915/display/intel_drrs.c > >> > index d266fad83a086..ffd51d2f5998d 100644 > >> > --- a/drivers/gpu/drm/i915/display/intel_drrs.c > >> > +++ b/drivers/gpu/drm/i915/display/intel_drrs.c > >> > @@ -144,16 +144,19 @@ void intel_drrs_activate(const struct > >> > intel_crtc_state *crtc_state) { > >> > struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc); > >> > > >> > + mutex_lock(&crtc->drrs.mutex); > >> > + > >> > + crtc->drrs.seamless_mode_switch_pixel_rate = crtc_state- > >> > >seamless_mode_switch ? > >> > + crtc_state->pixel_rate : 0; > >> > + > >> > if (!crtc_state->has_drrs) > >> > - return; > >> > + goto unlock; > >> > > >> > if (!crtc_state->hw.active) > >> > - return; > >> > + goto unlock; > >> > > >> > if (intel_crtc_is_bigjoiner_slave(crtc_state)) > >> > - return; > >> > - > >> > - mutex_lock(&crtc->drrs.mutex); > >> > + goto unlock; > >> > > >> > crtc->drrs.cpu_transcoder = crtc_state->cpu_transcoder; > >> > crtc->drrs.m_n = crtc_state->dp_m_n; @@ -163,6 +166,7 @@ void > >> > intel_drrs_activate(const struct intel_crtc_state > >> > *crtc_state) > >> > > >> > intel_drrs_schedule_work(crtc); > >> > > >> > +unlock: > >> > mutex_unlock(&crtc->drrs.mutex); > >> > } > >> > > >> > @@ -176,7 +180,7 @@ void intel_drrs_deactivate(const struct > >> > intel_crtc_state *old_crtc_state) { > >> > struct intel_crtc *crtc = > >> > to_intel_crtc(old_crtc_state->uapi.crtc); > >> > > >> > - if (!old_crtc_state->has_drrs) > >> > + if (!old_crtc_state->has_drrs && > >> > +!old_crtc_state->seamless_mode_switch) > >> > return; > >> > > >> > if (!old_crtc_state->hw.active) > >> > @@ -187,12 +191,14 @@ void intel_drrs_deactivate(const struct > >> > intel_crtc_state *old_crtc_state) > >> > > >> > mutex_lock(&crtc->drrs.mutex); > >> > > >> > - if (intel_drrs_is_active(crtc)) > >> > + if (intel_drrs_is_active(crtc) && > >> > + old_crtc_state->seamless_mode_switch == 0) > >> > intel_drrs_set_state(crtc, DRRS_REFRESH_RATE_HIGH); > >> > > >> > crtc->drrs.cpu_transcoder = INVALID_TRANSCODER; > >> > crtc->drrs.frontbuffer_bits = 0; > >> > crtc->drrs.busy_frontbuffer_bits = 0; > >> > + crtc->drrs.seamless_mode_switch_pixel_rate = 0; > >> > > >> > mutex_unlock(&crtc->drrs.mutex); > >> > > >> > @@ -322,5 +328,23 @@ bool intel_drrs_crtc_copy_fastset(const struct > >> > intel_crtc_state *old_crtc_state, > >> > if (intel_cpu_transcoder_has_m2_n2(i915, new_crtc_state- > >> > >cpu_transcoder)) > >> > return false; > >> > > >> > + if (!new_crtc_state->seamless_mode_switch) > >> > + new_crtc_state->dp_m_n = old_crtc_state->dp_m_n; > >> > + > >> > return true; > >> > } > >> > + > >> > +void intel_drrs_get_config(struct intel_encoder *encoder, > >> > + struct intel_crtc_state *pipe_config) { > >> > + struct intel_crtc *crtc; > >> > + > >> > + crtc = to_intel_crtc(pipe_config->uapi.crtc); > >> > + > >> > + mutex_lock(&crtc->drrs.mutex); > >> > + > >> > + pipe_config->seamless_mode_switch = crtc- > >> > >drrs.seamless_mode_switch_pixel_rate; > >> > + pipe_config->has_drrs = intel_drrs_is_active(crtc); > >> > + > >> > + mutex_unlock(&crtc->drrs.mutex); > >> > +} > >> > diff --git a/drivers/gpu/drm/i915/display/intel_drrs.h > >> > b/drivers/gpu/drm/i915/display/intel_drrs.h > >> > index 749ac717db063..a100023658a5a 100644 > >> > --- a/drivers/gpu/drm/i915/display/intel_drrs.h > >> > +++ b/drivers/gpu/drm/i915/display/intel_drrs.h > >> > @@ -14,6 +14,7 @@ struct intel_atomic_state; struct intel_crtc; > >> > struct intel_crtc_state; struct intel_connector; > >> > +struct intel_encoder; > >> > > >> > const char *intel_drrs_type_str(enum drrs_type drrs_type); bool > >> > intel_drrs_is_active(struct intel_crtc *crtc); @@ -26,5 +27,7 @@ > >> > void intel_drrs_flush(struct drm_i915_private *dev_priv, void > >> > intel_crtc_drrs_init(struct intel_crtc *crtc); bool > >> > intel_drrs_crtc_copy_fastset(const struct intel_crtc_state > *old_crtc_state, > >> > struct intel_crtc_state *new_crtc_state); > >> > +void intel_drrs_get_config(struct intel_encoder *encoder, > >> > + struct intel_crtc_state *pipe_config); > >> > > >> > #endif /* __INTEL_DRRS_H__ */ > >> > -- > >> > 2.36.0 > > > > -- > Jani Nikula, Intel Open Source Graphics Center
> -----Original Message----- > From: Srinivas, Vidya > Sent: Wednesday, May 25, 2022 2:50 PM > To: Jani Nikula <jani.nikula@linux.intel.com>; Souza, Jose > <jose.souza@intel.com>; intel-gfx@lists.freedesktop.org > Cc: Sean Paul <seanpaul@chromium.org>; Syrjala, Ville > <ville.syrjala@intel.com> > Subject: RE: [Intel-gfx] [PATCH v2 3/3] drm/i915/display: Implement seamless > mode switch > > > > > -----Original Message----- > > From: Jani Nikula <jani.nikula@linux.intel.com> > > Sent: Wednesday, May 25, 2022 2:26 PM > > To: Srinivas, Vidya <vidya.srinivas@intel.com>; Souza, Jose > > <jose.souza@intel.com>; intel-gfx@lists.freedesktop.org > > Cc: Sean Paul <seanpaul@chromium.org>; Syrjala, Ville > > <ville.syrjala@intel.com> > > Subject: Re: [Intel-gfx] [PATCH v2 3/3] drm/i915/display: Implement > > seamless mode switch > > > > On Wed, 25 May 2022, "Srinivas, Vidya" <vidya.srinivas@intel.com> wrote: > > > Hello, > > > > > > Apologies for bothering. May we kindly know if this solution is approved? > > > I have provided the Tested-by. Thanks much. > > > > IIUC the more complete solution is [1]. > > > > BR, > > Jani. > > > > > > [1] https://patchwork.freedesktop.org/series/103491/ > > > Hello Jani, > > Thank you very much. I will have a check of this. > For user space triggered seamless modeset WITHOUT setting > "DRM_MODE_ATOMIC_ALLOW_MODESET", for switch between 2 refresh > rates, Jose patches is working. > Jose please correct me if I am wrong. Thank you. > > Regards > Vidya Hello Jani, I checked the new series https://patchwork.freedesktop.org/series/103491/ and this is working as well. I tested it on ADL-P RVP with 144/60Hz panel. Both solutions from Jose and Ville are working. Many thanks to both. Regards Vidya > > > > > > > > > > Regards > > > Vidya > > > > > >> -----Original Message----- > > >> From: Srinivas, Vidya > > >> Sent: Monday, May 9, 2022 8:44 PM > > >> To: Souza, Jose <jose.souza@intel.com>; > > >> intel-gfx@lists.freedesktop.org > > >> Cc: Sean Paul <seanpaul@chromium.org>; Ville Syrjälä > > >> <ville.syrjala@linux.intel.com> > > >> Subject: RE: [PATCH v2 3/3] drm/i915/display: Implement seamless > > >> mode switch > > >> > > >> Hello Jose, > > >> > > >> Thanks much for the patch. I tested it on chrome system and the > > >> patch works. > > >> Adding my Tested-by. > > >> Tested-by: Vidya Srinivas <vidya.srinivas@intel.com> > > >> > > >> Regards > > >> Vidya > > >> > > >> > -----Original Message----- > > >> > From: Souza, Jose <jose.souza@intel.com> > > >> > Sent: Tuesday, May 3, 2022 2:11 AM > > >> > To: intel-gfx@lists.freedesktop.org > > >> > Cc: Srinivas, Vidya <vidya.srinivas@intel.com>; Sean Paul > > >> > <seanpaul@chromium.org>; Ville Syrjälä > > >> > <ville.syrjala@linux.intel.com>; Souza, Jose > > >> > <jose.souza@intel.com> > > >> > Subject: [PATCH v2 3/3] drm/i915/display: Implement seamless mode > > >> > switch > > >> > > > >> > So far the i915's DRRS feature was automatically changing between > > >> > preferred panel mode and downclock mode based on idleness but > > >> ChromeOS > > >> > compositor team is asking to be in control of the mode switch. > > >> > So for certain types of content it can switch to a mode with a > > >> > lower refresh rate without the user noticing a thing and saving > > >> > more > > power. > > >> > > > >> > This seamless mode switch will be triggered when user-space > > >> > dispatches an atomic commit with the new mode and clears the > > >> > DRM_MODE_ATOMIC_ALLOW_MODESET flag. > > >> > > > >> > The main steps to acomplish that are: > > >> > > > >> > - as mode changed in atomic state > > drm_atomic_helper_check_modeset() > > >> > will set mode_changed and it will trigger the crtc state > > >> > computation so > > >> > intel_dp_compute_config() will be called and dp_m_n will be > > >> > computed for the new mode > > >> > > > >> > - then intel_dp_drrs_compute_config() will check for the > > >> > necessary conditions to do a seamless mode switch, if possible > > >> > crtc_state- > > >> > >seamless_mode_switch will be set and has_drrs will not be set, > > >> > >so > > >> > >i915 will > > >> > not automatically switch between modes > > >> > > > >> > - then intel_crtc_compute_config() will call > > >> > intel_crtc_compute_pixel_rate() that will take the hint that it > > >> > is trying to do a seamless mode switch and set pixel_rate to the > > >> > pixel_rate of the old state > > >> > > > >> > - then if nothing else changed in the state > > >> > intel_crtc_check_fastset() will be able to set mode_changed to > > >> > false and i915 can do fastset otherwise the commit will fail > > >> > during the check phase > > >> > > > >> > - now on the atomic commit phase, intel_ddi_update_pipe_dp() will > > >> > be called and will program the new dp_m_n > > >> > > > >> > - nothing else is different in the commit phase until the step to > > >> > verify programmed state, the most important change here is that > > >> > need to save the pixel_rate in DRRS global state as there is no > > >> > other way to get that from hardware or atomic state > > >> > > > >> > v2: > > >> > - not overwritten dp_m_n values in intel_crtc_copy_fastset() when > > >> > doing a seamless_mode_switch > > >> > > > >> > Cc: Vidya Srinivas <vidya.srinivas@intel.com> > > >> > Cc: Sean Paul <seanpaul@chromium.org> > > >> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > > >> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com> > > >> > --- > > >> > drivers/gpu/drm/i915/display/intel_ddi.c | 10 +++ > > >> > drivers/gpu/drm/i915/display/intel_display.c | 61 > > >> > ++++++++++++++++--- .../drm/i915/display/intel_display_debugfs.c > > >> > ++++++++++++++++| > > >> > ++++++++++++++++3 + > > >> > .../drm/i915/display/intel_display_types.h | 3 + > > >> > drivers/gpu/drm/i915/display/intel_dp.c | 56 ++++++++++++++--- > > >> > drivers/gpu/drm/i915/display/intel_drrs.c | 38 +++++++++--- > > >> > drivers/gpu/drm/i915/display/intel_drrs.h | 3 + > > >> > 7 files changed, 151 insertions(+), 23 deletions(-) > > >> > > > >> > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c > > >> > b/drivers/gpu/drm/i915/display/intel_ddi.c > > >> > index 9e6fa59eabba7..732e5d425412e 100644 > > >> > --- a/drivers/gpu/drm/i915/display/intel_ddi.c > > >> > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c > > >> > @@ -46,6 +46,7 @@ > > >> > #include "intel_dp_link_training.h" > > >> > #include "intel_dp_mst.h" > > >> > #include "intel_dpio_phy.h" > > >> > +#include "intel_drrs.h" > > >> > #include "intel_dsi.h" > > >> > #include "intel_fdi.h" > > >> > #include "intel_fifo_underrun.h" > > >> > @@ -3010,6 +3011,14 @@ static void > > >> > intel_ddi_update_pipe_dp(struct intel_atomic_state *state, > > >> > > > >> > intel_backlight_update(state, encoder, crtc_state, conn_state); > > >> > drm_connector_update_privacy_screen(conn_state); > > >> > + > > >> > + if (crtc_state->seamless_mode_switch) { > > >> > + struct intel_crtc *intel_crtc = to_intel_crtc(crtc_state- > > >> > >uapi.crtc); > > >> > + > > >> > + intel_cpu_transcoder_set_m1_n1(intel_crtc, > > >> > + crtc_state->cpu_transcoder, > > >> > + &crtc_state->dp_m_n); > > >> > + } > > >> > } > > >> > > > >> > void intel_ddi_update_pipe(struct intel_atomic_state *state, @@ > > >> > -3484,6 > > >> > +3493,7 @@ static void intel_ddi_get_config(struct intel_encoder > > >> > +*encoder, > > >> > intel_read_dp_sdp(encoder, pipe_config, DP_SDP_VSC); > > >> > > > >> > intel_psr_get_config(encoder, pipe_config); > > >> > + intel_drrs_get_config(encoder, pipe_config); > > >> > } > > >> > > > >> > void intel_ddi_get_clock(struct intel_encoder *encoder, diff > > >> > --git a/drivers/gpu/drm/i915/display/intel_display.c > > >> > b/drivers/gpu/drm/i915/display/intel_display.c > > >> > index 17d0cad9e1686..e54fbe3b1d394 100644 > > >> > --- a/drivers/gpu/drm/i915/display/intel_display.c > > >> > +++ b/drivers/gpu/drm/i915/display/intel_display.c > > >> > @@ -2629,6 +2629,44 @@ static void > > >> > intel_crtc_compute_pixel_rate(struct > > >> > intel_crtc_state *crtc_state) > > >> > else > > >> > crtc_state->pixel_rate = > > >> > ilk_pipe_pixel_rate(crtc_state); > > >> > + > > >> > + /* > > >> > + * Do not change pixel_rate when doing seamless mode switch, > > >> > otherwise > > >> > + * it will change port_clock and other stuff that will need a modeset > > >> > + * to be programmed > > >> > + */ > > >> > + if (crtc_state->seamless_mode_switch) { > > >> > + struct intel_atomic_state *state = > > >> > to_intel_atomic_state(crtc_state->uapi.state); > > >> > + struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc); > > >> > + const struct intel_crtc_state *old_crtc_state; > > >> > + > > >> > + old_crtc_state = intel_atomic_get_old_crtc_state(state, crtc); > > >> > + > > >> > + /* > > >> > + * It can only seamless switch if pixel rate of the new mode is > > >> > + * <= than old one, if not commit will fail with pixel_rate > > >> > + * mismatch. > > >> > + */ > > >> > + if (crtc_state->pixel_rate < old_crtc_state->pixel_rate || > > >> > + intel_fuzzy_clock_check(crtc_state->pixel_rate, > > >> > + old_crtc_state->pixel_rate)) > > >> > + crtc_state->pixel_rate = old_crtc_state->pixel_rate; > > >> > + } > > >> > +} > > >> > + > > >> > +static void intel_crtc_get_pixel_rate(struct intel_crtc_state > > >> > +*crtc_state) { > > >> > + if (crtc_state->seamless_mode_switch) { > > >> > + struct intel_crtc *crtc = > > >> > +to_intel_crtc(crtc_state->uapi.crtc); > > >> > + > > >> > + mutex_lock(&crtc->drrs.mutex); > > >> > + crtc_state->pixel_rate = crtc- > > >> > >drrs.seamless_mode_switch_pixel_rate; > > >> > + mutex_unlock(&crtc->drrs.mutex); > > >> > + > > >> > + return; > > >> > + } > > >> > + > > >> > + intel_crtc_compute_pixel_rate(crtc_state); > > >> > } > > >> > > > >> > static void intel_bigjoiner_adjust_timings(const struct > > >> > intel_crtc_state *crtc_state, @@ -2705,7 +2743,7 @@ static void > > >> > intel_crtc_readout_derived_state(struct intel_crtc_state *crtc_state > > >> > intel_bigjoiner_adjust_timings(crtc_state, pipe_mode); > > >> > intel_mode_from_crtc_timings(pipe_mode, pipe_mode); > > >> > > > >> > - intel_crtc_compute_pixel_rate(crtc_state); > > >> > + intel_crtc_get_pixel_rate(crtc_state); > > >> > } > > >> > > > >> > static void intel_encoder_get_config(struct intel_encoder > > >> > *encoder, @@ - > > >> > 6182,11 +6220,13 @@ intel_pipe_config_compare(const struct > > >> > intel_crtc_state *current_config, > > >> > PIPE_CONF_CHECK_I(lane_count); > > >> > PIPE_CONF_CHECK_X(lane_lat_optim_mask); > > >> > > > >> > - if (DISPLAY_VER(dev_priv) >= 9 || IS_BROADWELL(dev_priv)) { > > >> > - PIPE_CONF_CHECK_M_N_ALT(dp_m_n, dp_m2_n2); > > >> > - } else { > > >> > - PIPE_CONF_CHECK_M_N(dp_m_n); > > >> > - PIPE_CONF_CHECK_M_N(dp_m2_n2); > > >> > + if (!pipe_config->seamless_mode_switch) { > > >> > + if (DISPLAY_VER(dev_priv) >= 9 || IS_BROADWELL(dev_priv)) > > >> > { > > >> > + PIPE_CONF_CHECK_M_N_ALT(dp_m_n, dp_m2_n2); > > >> > + } else { > > >> > + PIPE_CONF_CHECK_M_N(dp_m_n); > > >> > + PIPE_CONF_CHECK_M_N(dp_m2_n2); > > >> > + } > > >> > } > > >> > > > >> > PIPE_CONF_CHECK_X(output_types); @@ -6348,8 +6388,10 @@ > > >> > intel_pipe_config_compare(const struct intel_crtc_state > > >> > *current_config, > > >> > if (IS_G4X(dev_priv) || DISPLAY_VER(dev_priv) >= 5) > > >> > PIPE_CONF_CHECK_I(pipe_bpp); > > >> > > > >> > - PIPE_CONF_CHECK_CLOCK_FUZZY(hw.pipe_mode.crtc_clock); > > >> > - PIPE_CONF_CHECK_CLOCK_FUZZY(hw.adjusted_mode.crtc_clock); > > >> > + if (!pipe_config->seamless_mode_switch) { > > >> > + PIPE_CONF_CHECK_CLOCK_FUZZY(hw.pipe_mode.crtc_clock); > > >> > + > > >> > PIPE_CONF_CHECK_CLOCK_FUZZY(hw.adjusted_mode.crtc_clock); > > >> > + } > > >> > PIPE_CONF_CHECK_CLOCK_FUZZY(port_clock); > > >> > > > >> > PIPE_CONF_CHECK_I(min_voltage_level); > > >> > @@ -7085,11 +7127,12 @@ static void intel_crtc_copy_fastset(const > > >> > struct intel_crtc_state *old_crtc_stat > > >> > * FIXME: should really copy more fuzzy state here > > >> > */ > > >> > new_crtc_state->fdi_m_n = old_crtc_state->fdi_m_n; > > >> > - new_crtc_state->dp_m_n = old_crtc_state->dp_m_n; > > >> > > > >> > if (!intel_drrs_crtc_copy_fastset(old_crtc_state, > > >> > new_crtc_state)) { > > >> > + new_crtc_state->dp_m_n = old_crtc_state->dp_m_n; > > >> > new_crtc_state->dp_m2_n2 = old_crtc_state->dp_m2_n2; > > >> > new_crtc_state->has_drrs = old_crtc_state->has_drrs; > > >> > + new_crtc_state->seamless_mode_switch = > > >> > +old_crtc_state->seamless_mode_switch; > > >> > } > > >> > } > > >> > > > >> > diff --git a/drivers/gpu/drm/i915/display/intel_display_debugfs.c > > >> > b/drivers/gpu/drm/i915/display/intel_display_debugfs.c > > >> > index 452d773fd4e34..92099e1f0b23e 100644 > > >> > --- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c > > >> > +++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c > > >> > @@ -1108,6 +1108,9 @@ static int i915_drrs_status(struct seq_file > > >> > *m, void > > >> > *unused) > > >> > crtc->drrs.refresh_rate == > DRRS_REFRESH_RATE_LOW ? > > >> > "low" : "high"); > > >> > > > >> > + seq_printf(m, "Seamless mode switch enabled: %s\n", > > >> > + str_yes_no(crtc- > > >> > >drrs.seamless_mode_switch_pixel_rate)); > > >> > + > > >> > mutex_unlock(&crtc->drrs.mutex); > > >> > } > > >> > > > >> > diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h > > >> > b/drivers/gpu/drm/i915/display/intel_display_types.h > > >> > index 408152f9f46a4..e290366adfc27 100644 > > >> > --- a/drivers/gpu/drm/i915/display/intel_display_types.h > > >> > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h > > >> > @@ -1057,6 +1057,7 @@ struct intel_crtc_state { > > >> > /* m2_n2 for eDP downclock */ > > >> > struct intel_link_m_n dp_m2_n2; > > >> > bool has_drrs; > > >> > + bool seamless_mode_switch; > > >> > > > >> > /* PSR is supported but might not be enabled due the lack of > > >> > enabled planes */ > > >> > bool has_psr; > > >> > @@ -1314,6 +1315,8 @@ struct intel_crtc { > > >> > unsigned int busy_frontbuffer_bits; > > >> > enum transcoder cpu_transcoder; > > >> > struct intel_link_m_n m_n, m2_n2; > > >> > + /* Only used to verify if programmed state matches */ > > >> > + unsigned int seamless_mode_switch_pixel_rate; > > >> > } drrs; > > >> > > > >> > int scanline_offset; > > >> > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c > > >> > b/drivers/gpu/drm/i915/display/intel_dp.c > > >> > index cc9be82e128f4..691d362dde95e 100644 > > >> > --- a/drivers/gpu/drm/i915/display/intel_dp.c > > >> > +++ b/drivers/gpu/drm/i915/display/intel_dp.c > > >> > @@ -1844,8 +1844,7 @@ static bool cpu_transcoder_has_drrs(struct > > >> > drm_i915_private *i915, } > > >> > > > >> > static bool can_enable_drrs(struct intel_connector *connector, > > >> > - const struct intel_crtc_state *pipe_config, > > >> > - const struct drm_display_mode > > >> > *downclock_mode) > > >> > + const struct intel_crtc_state *pipe_config) > > >> > { > > >> > struct drm_i915_private *i915 = to_i915(connector->base.dev); > > >> > > > >> > @@ -1868,8 +1867,40 @@ static bool can_enable_drrs(struct > > >> > intel_connector *connector, > > >> > if (!cpu_transcoder_has_drrs(i915, pipe_config->cpu_transcoder)) > > >> > return false; > > >> > > > >> > - return downclock_mode && > > >> > - intel_panel_drrs_type(connector) == DRRS_TYPE_SEAMLESS; > > >> > + return intel_panel_drrs_type(connector) == DRRS_TYPE_SEAMLESS; > > >> > +} > > >> > + > > >> > +static bool can_seamless_switch_mode(struct intel_crtc_state > > >> > +*pipe_config) { > > >> > + struct drm_i915_private *i915 = to_i915(pipe_config->uapi.crtc- > > >> > >dev); > > >> > + const struct intel_crtc_state *old_pipe_config; > > >> > + struct intel_atomic_state *state; > > >> > + struct intel_crtc *intel_crtc; > > >> > + > > >> > + /* Only supported in platforms that can switch m_n on the fly */ > > >> > + if (intel_cpu_transcoder_has_m2_n2(i915, pipe_config- > > >> > >cpu_transcoder)) > > >> > + return false; > > >> > + > > >> > + state = to_intel_atomic_state(pipe_config->uapi.state); > > >> > + if (state->base.allow_modeset) > > >> > + return false; > > >> > + > > >> > + intel_crtc = to_intel_crtc(pipe_config->uapi.crtc); > > >> > + old_pipe_config = intel_atomic_get_old_crtc_state(state, intel_crtc); > > >> > + if (drm_mode_equal(&old_pipe_config->hw.adjusted_mode, > > >> > + &pipe_config->hw.adjusted_mode)) > > >> > + return false; > > >> > + > > >> > + /* Only clock can change */ > > >> > + if (!drm_mode_match(&old_pipe_config->hw.adjusted_mode, > > >> > + &pipe_config->hw.adjusted_mode, > > >> > + DRM_MODE_MATCH_TIMINGS | > > >> > + DRM_MODE_MATCH_FLAGS | > > >> > + DRM_MODE_MATCH_3D_FLAGS | > > >> > + DRM_MODE_MATCH_ASPECT_RATIO)) > > >> > + return false; > > >> > + > > >> > + return true; > > >> > } > > >> > > > >> > static void > > >> > @@ -1882,12 +1913,17 @@ intel_dp_drrs_compute_config(struct > > >> > intel_connector *connector, > > >> > intel_panel_downclock_mode(connector, &pipe_config- > > >> > >hw.adjusted_mode); > > >> > int pixel_clock; > > >> > > > >> > - if (!can_enable_drrs(connector, pipe_config, downclock_mode)) { > > >> > - if (intel_cpu_transcoder_has_m2_n2(i915, pipe_config- > > >> > >cpu_transcoder)) > > >> > - intel_zero_m_n(&pipe_config->dp_m2_n2); > > >> > + if (!can_enable_drrs(connector, pipe_config)) > > >> > + goto not_supported; > > >> > + > > >> > + if (can_seamless_switch_mode(pipe_config)) { > > >> > + pipe_config->seamless_mode_switch = true; > > >> > return; > > >> > } > > >> > > > >> > + if (!downclock_mode) > > >> > + goto not_supported; > > >> > + > > >> > if (IS_IRONLAKE(i915) || IS_SANDYBRIDGE(i915) || > > >> > IS_IVYBRIDGE(i915)) > > >> > pipe_config->msa_timing_delay = i915- > > >> > >vbt.edp.drrs_msa_timing_delay; > > >> > > > >> > @@ -1904,6 +1940,12 @@ intel_dp_drrs_compute_config(struct > > >> > intel_connector *connector, > > >> > /* FIXME: abstract this better */ > > >> > if (pipe_config->splitter.enable) > > >> > pipe_config->dp_m2_n2.data_m *= pipe_config- > > >> > >splitter.link_count; > > >> > + > > >> > + return; > > >> > + > > >> > +not_supported: > > >> > + if (intel_cpu_transcoder_has_m2_n2(i915, pipe_config- > > >> > >cpu_transcoder)) > > >> > + intel_zero_m_n(&pipe_config->dp_m2_n2); > > >> > } > > >> > > > >> > static bool intel_dp_has_audio(struct intel_encoder *encoder, > > >> > diff --git a/drivers/gpu/drm/i915/display/intel_drrs.c > > >> > b/drivers/gpu/drm/i915/display/intel_drrs.c > > >> > index d266fad83a086..ffd51d2f5998d 100644 > > >> > --- a/drivers/gpu/drm/i915/display/intel_drrs.c > > >> > +++ b/drivers/gpu/drm/i915/display/intel_drrs.c > > >> > @@ -144,16 +144,19 @@ void intel_drrs_activate(const struct > > >> > intel_crtc_state *crtc_state) { > > >> > struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc); > > >> > > > >> > + mutex_lock(&crtc->drrs.mutex); > > >> > + > > >> > + crtc->drrs.seamless_mode_switch_pixel_rate = crtc_state- > > >> > >seamless_mode_switch ? > > >> > + crtc_state->pixel_rate : 0; > > >> > + > > >> > if (!crtc_state->has_drrs) > > >> > - return; > > >> > + goto unlock; > > >> > > > >> > if (!crtc_state->hw.active) > > >> > - return; > > >> > + goto unlock; > > >> > > > >> > if (intel_crtc_is_bigjoiner_slave(crtc_state)) > > >> > - return; > > >> > - > > >> > - mutex_lock(&crtc->drrs.mutex); > > >> > + goto unlock; > > >> > > > >> > crtc->drrs.cpu_transcoder = crtc_state->cpu_transcoder; > > >> > crtc->drrs.m_n = crtc_state->dp_m_n; @@ -163,6 +166,7 @@ void > > >> > intel_drrs_activate(const struct intel_crtc_state > > >> > *crtc_state) > > >> > > > >> > intel_drrs_schedule_work(crtc); > > >> > > > >> > +unlock: > > >> > mutex_unlock(&crtc->drrs.mutex); } > > >> > > > >> > @@ -176,7 +180,7 @@ void intel_drrs_deactivate(const struct > > >> > intel_crtc_state *old_crtc_state) { > > >> > struct intel_crtc *crtc = > > >> > to_intel_crtc(old_crtc_state->uapi.crtc); > > >> > > > >> > - if (!old_crtc_state->has_drrs) > > >> > + if (!old_crtc_state->has_drrs && > > >> > +!old_crtc_state->seamless_mode_switch) > > >> > return; > > >> > > > >> > if (!old_crtc_state->hw.active) @@ -187,12 +191,14 @@ void > > >> > intel_drrs_deactivate(const struct intel_crtc_state > > >> > *old_crtc_state) > > >> > > > >> > mutex_lock(&crtc->drrs.mutex); > > >> > > > >> > - if (intel_drrs_is_active(crtc)) > > >> > + if (intel_drrs_is_active(crtc) && > > >> > + old_crtc_state->seamless_mode_switch == 0) > > >> > intel_drrs_set_state(crtc, DRRS_REFRESH_RATE_HIGH); > > >> > > > >> > crtc->drrs.cpu_transcoder = INVALID_TRANSCODER; > > >> > crtc->drrs.frontbuffer_bits = 0; > > >> > crtc->drrs.busy_frontbuffer_bits = 0; > > >> > + crtc->drrs.seamless_mode_switch_pixel_rate = 0; > > >> > > > >> > mutex_unlock(&crtc->drrs.mutex); > > >> > > > >> > @@ -322,5 +328,23 @@ bool intel_drrs_crtc_copy_fastset(const > > >> > struct intel_crtc_state *old_crtc_state, > > >> > if (intel_cpu_transcoder_has_m2_n2(i915, new_crtc_state- > > >> > >cpu_transcoder)) > > >> > return false; > > >> > > > >> > + if (!new_crtc_state->seamless_mode_switch) > > >> > + new_crtc_state->dp_m_n = old_crtc_state->dp_m_n; > > >> > + > > >> > return true; > > >> > } > > >> > + > > >> > +void intel_drrs_get_config(struct intel_encoder *encoder, > > >> > + struct intel_crtc_state *pipe_config) { > > >> > + struct intel_crtc *crtc; > > >> > + > > >> > + crtc = to_intel_crtc(pipe_config->uapi.crtc); > > >> > + > > >> > + mutex_lock(&crtc->drrs.mutex); > > >> > + > > >> > + pipe_config->seamless_mode_switch = crtc- > > >> > >drrs.seamless_mode_switch_pixel_rate; > > >> > + pipe_config->has_drrs = intel_drrs_is_active(crtc); > > >> > + > > >> > + mutex_unlock(&crtc->drrs.mutex); } > > >> > diff --git a/drivers/gpu/drm/i915/display/intel_drrs.h > > >> > b/drivers/gpu/drm/i915/display/intel_drrs.h > > >> > index 749ac717db063..a100023658a5a 100644 > > >> > --- a/drivers/gpu/drm/i915/display/intel_drrs.h > > >> > +++ b/drivers/gpu/drm/i915/display/intel_drrs.h > > >> > @@ -14,6 +14,7 @@ struct intel_atomic_state; struct intel_crtc; > > >> > struct intel_crtc_state; struct intel_connector; > > >> > +struct intel_encoder; > > >> > > > >> > const char *intel_drrs_type_str(enum drrs_type drrs_type); bool > > >> > intel_drrs_is_active(struct intel_crtc *crtc); @@ -26,5 +27,7 @@ > > >> > void intel_drrs_flush(struct drm_i915_private *dev_priv, void > > >> > intel_crtc_drrs_init(struct intel_crtc *crtc); bool > > >> > intel_drrs_crtc_copy_fastset(const struct intel_crtc_state > > *old_crtc_state, > > >> > struct intel_crtc_state *new_crtc_state); > > >> > +void intel_drrs_get_config(struct intel_encoder *encoder, > > >> > + struct intel_crtc_state *pipe_config); > > >> > > > >> > #endif /* __INTEL_DRRS_H__ */ > > >> > -- > > >> > 2.36.0 > > > > > > > -- > > Jani Nikula, Intel Open Source Graphics Center
diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c index 9e6fa59eabba7..732e5d425412e 100644 --- a/drivers/gpu/drm/i915/display/intel_ddi.c +++ b/drivers/gpu/drm/i915/display/intel_ddi.c @@ -46,6 +46,7 @@ #include "intel_dp_link_training.h" #include "intel_dp_mst.h" #include "intel_dpio_phy.h" +#include "intel_drrs.h" #include "intel_dsi.h" #include "intel_fdi.h" #include "intel_fifo_underrun.h" @@ -3010,6 +3011,14 @@ static void intel_ddi_update_pipe_dp(struct intel_atomic_state *state, intel_backlight_update(state, encoder, crtc_state, conn_state); drm_connector_update_privacy_screen(conn_state); + + if (crtc_state->seamless_mode_switch) { + struct intel_crtc *intel_crtc = to_intel_crtc(crtc_state->uapi.crtc); + + intel_cpu_transcoder_set_m1_n1(intel_crtc, + crtc_state->cpu_transcoder, + &crtc_state->dp_m_n); + } } void intel_ddi_update_pipe(struct intel_atomic_state *state, @@ -3484,6 +3493,7 @@ static void intel_ddi_get_config(struct intel_encoder *encoder, intel_read_dp_sdp(encoder, pipe_config, DP_SDP_VSC); intel_psr_get_config(encoder, pipe_config); + intel_drrs_get_config(encoder, pipe_config); } void intel_ddi_get_clock(struct intel_encoder *encoder, diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c index 17d0cad9e1686..e54fbe3b1d394 100644 --- a/drivers/gpu/drm/i915/display/intel_display.c +++ b/drivers/gpu/drm/i915/display/intel_display.c @@ -2629,6 +2629,44 @@ static void intel_crtc_compute_pixel_rate(struct intel_crtc_state *crtc_state) else crtc_state->pixel_rate = ilk_pipe_pixel_rate(crtc_state); + + /* + * Do not change pixel_rate when doing seamless mode switch, otherwise + * it will change port_clock and other stuff that will need a modeset + * to be programmed + */ + if (crtc_state->seamless_mode_switch) { + struct intel_atomic_state *state = to_intel_atomic_state(crtc_state->uapi.state); + struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc); + const struct intel_crtc_state *old_crtc_state; + + old_crtc_state = intel_atomic_get_old_crtc_state(state, crtc); + + /* + * It can only seamless switch if pixel rate of the new mode is + * <= than old one, if not commit will fail with pixel_rate + * mismatch. + */ + if (crtc_state->pixel_rate < old_crtc_state->pixel_rate || + intel_fuzzy_clock_check(crtc_state->pixel_rate, + old_crtc_state->pixel_rate)) + crtc_state->pixel_rate = old_crtc_state->pixel_rate; + } +} + +static void intel_crtc_get_pixel_rate(struct intel_crtc_state *crtc_state) +{ + if (crtc_state->seamless_mode_switch) { + struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc); + + mutex_lock(&crtc->drrs.mutex); + crtc_state->pixel_rate = crtc->drrs.seamless_mode_switch_pixel_rate; + mutex_unlock(&crtc->drrs.mutex); + + return; + } + + intel_crtc_compute_pixel_rate(crtc_state); } static void intel_bigjoiner_adjust_timings(const struct intel_crtc_state *crtc_state, @@ -2705,7 +2743,7 @@ static void intel_crtc_readout_derived_state(struct intel_crtc_state *crtc_state intel_bigjoiner_adjust_timings(crtc_state, pipe_mode); intel_mode_from_crtc_timings(pipe_mode, pipe_mode); - intel_crtc_compute_pixel_rate(crtc_state); + intel_crtc_get_pixel_rate(crtc_state); } static void intel_encoder_get_config(struct intel_encoder *encoder, @@ -6182,11 +6220,13 @@ intel_pipe_config_compare(const struct intel_crtc_state *current_config, PIPE_CONF_CHECK_I(lane_count); PIPE_CONF_CHECK_X(lane_lat_optim_mask); - if (DISPLAY_VER(dev_priv) >= 9 || IS_BROADWELL(dev_priv)) { - PIPE_CONF_CHECK_M_N_ALT(dp_m_n, dp_m2_n2); - } else { - PIPE_CONF_CHECK_M_N(dp_m_n); - PIPE_CONF_CHECK_M_N(dp_m2_n2); + if (!pipe_config->seamless_mode_switch) { + if (DISPLAY_VER(dev_priv) >= 9 || IS_BROADWELL(dev_priv)) { + PIPE_CONF_CHECK_M_N_ALT(dp_m_n, dp_m2_n2); + } else { + PIPE_CONF_CHECK_M_N(dp_m_n); + PIPE_CONF_CHECK_M_N(dp_m2_n2); + } } PIPE_CONF_CHECK_X(output_types); @@ -6348,8 +6388,10 @@ intel_pipe_config_compare(const struct intel_crtc_state *current_config, if (IS_G4X(dev_priv) || DISPLAY_VER(dev_priv) >= 5) PIPE_CONF_CHECK_I(pipe_bpp); - PIPE_CONF_CHECK_CLOCK_FUZZY(hw.pipe_mode.crtc_clock); - PIPE_CONF_CHECK_CLOCK_FUZZY(hw.adjusted_mode.crtc_clock); + if (!pipe_config->seamless_mode_switch) { + PIPE_CONF_CHECK_CLOCK_FUZZY(hw.pipe_mode.crtc_clock); + PIPE_CONF_CHECK_CLOCK_FUZZY(hw.adjusted_mode.crtc_clock); + } PIPE_CONF_CHECK_CLOCK_FUZZY(port_clock); PIPE_CONF_CHECK_I(min_voltage_level); @@ -7085,11 +7127,12 @@ static void intel_crtc_copy_fastset(const struct intel_crtc_state *old_crtc_stat * FIXME: should really copy more fuzzy state here */ new_crtc_state->fdi_m_n = old_crtc_state->fdi_m_n; - new_crtc_state->dp_m_n = old_crtc_state->dp_m_n; if (!intel_drrs_crtc_copy_fastset(old_crtc_state, new_crtc_state)) { + new_crtc_state->dp_m_n = old_crtc_state->dp_m_n; new_crtc_state->dp_m2_n2 = old_crtc_state->dp_m2_n2; new_crtc_state->has_drrs = old_crtc_state->has_drrs; + new_crtc_state->seamless_mode_switch = old_crtc_state->seamless_mode_switch; } } diff --git a/drivers/gpu/drm/i915/display/intel_display_debugfs.c b/drivers/gpu/drm/i915/display/intel_display_debugfs.c index 452d773fd4e34..92099e1f0b23e 100644 --- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c +++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c @@ -1108,6 +1108,9 @@ static int i915_drrs_status(struct seq_file *m, void *unused) crtc->drrs.refresh_rate == DRRS_REFRESH_RATE_LOW ? "low" : "high"); + seq_printf(m, "Seamless mode switch enabled: %s\n", + str_yes_no(crtc->drrs.seamless_mode_switch_pixel_rate)); + mutex_unlock(&crtc->drrs.mutex); } diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h index 408152f9f46a4..e290366adfc27 100644 --- a/drivers/gpu/drm/i915/display/intel_display_types.h +++ b/drivers/gpu/drm/i915/display/intel_display_types.h @@ -1057,6 +1057,7 @@ struct intel_crtc_state { /* m2_n2 for eDP downclock */ struct intel_link_m_n dp_m2_n2; bool has_drrs; + bool seamless_mode_switch; /* PSR is supported but might not be enabled due the lack of enabled planes */ bool has_psr; @@ -1314,6 +1315,8 @@ struct intel_crtc { unsigned int busy_frontbuffer_bits; enum transcoder cpu_transcoder; struct intel_link_m_n m_n, m2_n2; + /* Only used to verify if programmed state matches */ + unsigned int seamless_mode_switch_pixel_rate; } drrs; int scanline_offset; diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c index cc9be82e128f4..691d362dde95e 100644 --- a/drivers/gpu/drm/i915/display/intel_dp.c +++ b/drivers/gpu/drm/i915/display/intel_dp.c @@ -1844,8 +1844,7 @@ static bool cpu_transcoder_has_drrs(struct drm_i915_private *i915, } static bool can_enable_drrs(struct intel_connector *connector, - const struct intel_crtc_state *pipe_config, - const struct drm_display_mode *downclock_mode) + const struct intel_crtc_state *pipe_config) { struct drm_i915_private *i915 = to_i915(connector->base.dev); @@ -1868,8 +1867,40 @@ static bool can_enable_drrs(struct intel_connector *connector, if (!cpu_transcoder_has_drrs(i915, pipe_config->cpu_transcoder)) return false; - return downclock_mode && - intel_panel_drrs_type(connector) == DRRS_TYPE_SEAMLESS; + return intel_panel_drrs_type(connector) == DRRS_TYPE_SEAMLESS; +} + +static bool can_seamless_switch_mode(struct intel_crtc_state *pipe_config) +{ + struct drm_i915_private *i915 = to_i915(pipe_config->uapi.crtc->dev); + const struct intel_crtc_state *old_pipe_config; + struct intel_atomic_state *state; + struct intel_crtc *intel_crtc; + + /* Only supported in platforms that can switch m_n on the fly */ + if (intel_cpu_transcoder_has_m2_n2(i915, pipe_config->cpu_transcoder)) + return false; + + state = to_intel_atomic_state(pipe_config->uapi.state); + if (state->base.allow_modeset) + return false; + + intel_crtc = to_intel_crtc(pipe_config->uapi.crtc); + old_pipe_config = intel_atomic_get_old_crtc_state(state, intel_crtc); + if (drm_mode_equal(&old_pipe_config->hw.adjusted_mode, + &pipe_config->hw.adjusted_mode)) + return false; + + /* Only clock can change */ + if (!drm_mode_match(&old_pipe_config->hw.adjusted_mode, + &pipe_config->hw.adjusted_mode, + DRM_MODE_MATCH_TIMINGS | + DRM_MODE_MATCH_FLAGS | + DRM_MODE_MATCH_3D_FLAGS | + DRM_MODE_MATCH_ASPECT_RATIO)) + return false; + + return true; } static void @@ -1882,12 +1913,17 @@ intel_dp_drrs_compute_config(struct intel_connector *connector, intel_panel_downclock_mode(connector, &pipe_config->hw.adjusted_mode); int pixel_clock; - if (!can_enable_drrs(connector, pipe_config, downclock_mode)) { - if (intel_cpu_transcoder_has_m2_n2(i915, pipe_config->cpu_transcoder)) - intel_zero_m_n(&pipe_config->dp_m2_n2); + if (!can_enable_drrs(connector, pipe_config)) + goto not_supported; + + if (can_seamless_switch_mode(pipe_config)) { + pipe_config->seamless_mode_switch = true; return; } + if (!downclock_mode) + goto not_supported; + if (IS_IRONLAKE(i915) || IS_SANDYBRIDGE(i915) || IS_IVYBRIDGE(i915)) pipe_config->msa_timing_delay = i915->vbt.edp.drrs_msa_timing_delay; @@ -1904,6 +1940,12 @@ intel_dp_drrs_compute_config(struct intel_connector *connector, /* FIXME: abstract this better */ if (pipe_config->splitter.enable) pipe_config->dp_m2_n2.data_m *= pipe_config->splitter.link_count; + + return; + +not_supported: + if (intel_cpu_transcoder_has_m2_n2(i915, pipe_config->cpu_transcoder)) + intel_zero_m_n(&pipe_config->dp_m2_n2); } static bool intel_dp_has_audio(struct intel_encoder *encoder, diff --git a/drivers/gpu/drm/i915/display/intel_drrs.c b/drivers/gpu/drm/i915/display/intel_drrs.c index d266fad83a086..ffd51d2f5998d 100644 --- a/drivers/gpu/drm/i915/display/intel_drrs.c +++ b/drivers/gpu/drm/i915/display/intel_drrs.c @@ -144,16 +144,19 @@ void intel_drrs_activate(const struct intel_crtc_state *crtc_state) { struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc); + mutex_lock(&crtc->drrs.mutex); + + crtc->drrs.seamless_mode_switch_pixel_rate = crtc_state->seamless_mode_switch ? + crtc_state->pixel_rate : 0; + if (!crtc_state->has_drrs) - return; + goto unlock; if (!crtc_state->hw.active) - return; + goto unlock; if (intel_crtc_is_bigjoiner_slave(crtc_state)) - return; - - mutex_lock(&crtc->drrs.mutex); + goto unlock; crtc->drrs.cpu_transcoder = crtc_state->cpu_transcoder; crtc->drrs.m_n = crtc_state->dp_m_n; @@ -163,6 +166,7 @@ void intel_drrs_activate(const struct intel_crtc_state *crtc_state) intel_drrs_schedule_work(crtc); +unlock: mutex_unlock(&crtc->drrs.mutex); } @@ -176,7 +180,7 @@ void intel_drrs_deactivate(const struct intel_crtc_state *old_crtc_state) { struct intel_crtc *crtc = to_intel_crtc(old_crtc_state->uapi.crtc); - if (!old_crtc_state->has_drrs) + if (!old_crtc_state->has_drrs && !old_crtc_state->seamless_mode_switch) return; if (!old_crtc_state->hw.active) @@ -187,12 +191,14 @@ void intel_drrs_deactivate(const struct intel_crtc_state *old_crtc_state) mutex_lock(&crtc->drrs.mutex); - if (intel_drrs_is_active(crtc)) + if (intel_drrs_is_active(crtc) && + old_crtc_state->seamless_mode_switch == 0) intel_drrs_set_state(crtc, DRRS_REFRESH_RATE_HIGH); crtc->drrs.cpu_transcoder = INVALID_TRANSCODER; crtc->drrs.frontbuffer_bits = 0; crtc->drrs.busy_frontbuffer_bits = 0; + crtc->drrs.seamless_mode_switch_pixel_rate = 0; mutex_unlock(&crtc->drrs.mutex); @@ -322,5 +328,23 @@ bool intel_drrs_crtc_copy_fastset(const struct intel_crtc_state *old_crtc_state, if (intel_cpu_transcoder_has_m2_n2(i915, new_crtc_state->cpu_transcoder)) return false; + if (!new_crtc_state->seamless_mode_switch) + new_crtc_state->dp_m_n = old_crtc_state->dp_m_n; + return true; } + +void intel_drrs_get_config(struct intel_encoder *encoder, + struct intel_crtc_state *pipe_config) +{ + struct intel_crtc *crtc; + + crtc = to_intel_crtc(pipe_config->uapi.crtc); + + mutex_lock(&crtc->drrs.mutex); + + pipe_config->seamless_mode_switch = crtc->drrs.seamless_mode_switch_pixel_rate; + pipe_config->has_drrs = intel_drrs_is_active(crtc); + + mutex_unlock(&crtc->drrs.mutex); +} diff --git a/drivers/gpu/drm/i915/display/intel_drrs.h b/drivers/gpu/drm/i915/display/intel_drrs.h index 749ac717db063..a100023658a5a 100644 --- a/drivers/gpu/drm/i915/display/intel_drrs.h +++ b/drivers/gpu/drm/i915/display/intel_drrs.h @@ -14,6 +14,7 @@ struct intel_atomic_state; struct intel_crtc; struct intel_crtc_state; struct intel_connector; +struct intel_encoder; const char *intel_drrs_type_str(enum drrs_type drrs_type); bool intel_drrs_is_active(struct intel_crtc *crtc); @@ -26,5 +27,7 @@ void intel_drrs_flush(struct drm_i915_private *dev_priv, void intel_crtc_drrs_init(struct intel_crtc *crtc); bool intel_drrs_crtc_copy_fastset(const struct intel_crtc_state *old_crtc_state, struct intel_crtc_state *new_crtc_state); +void intel_drrs_get_config(struct intel_encoder *encoder, + struct intel_crtc_state *pipe_config); #endif /* __INTEL_DRRS_H__ */
So far the i915's DRRS feature was automatically changing between preferred panel mode and downclock mode based on idleness but ChromeOS compositor team is asking to be in control of the mode switch. So for certain types of content it can switch to a mode with a lower refresh rate without the user noticing a thing and saving more power. This seamless mode switch will be triggered when user-space dispatches an atomic commit with the new mode and clears the DRM_MODE_ATOMIC_ALLOW_MODESET flag. The main steps to acomplish that are: - as mode changed in atomic state drm_atomic_helper_check_modeset() will set mode_changed and it will trigger the crtc state computation so intel_dp_compute_config() will be called and dp_m_n will be computed for the new mode - then intel_dp_drrs_compute_config() will check for the necessary conditions to do a seamless mode switch, if possible crtc_state->seamless_mode_switch will be set and has_drrs will not be set, so i915 will not automatically switch between modes - then intel_crtc_compute_config() will call intel_crtc_compute_pixel_rate() that will take the hint that it is trying to do a seamless mode switch and set pixel_rate to the pixel_rate of the old state - then if nothing else changed in the state intel_crtc_check_fastset() will be able to set mode_changed to false and i915 can do fastset otherwise the commit will fail during the check phase - now on the atomic commit phase, intel_ddi_update_pipe_dp() will be called and will program the new dp_m_n - nothing else is different in the commit phase until the step to verify programmed state, the most important change here is that need to save the pixel_rate in DRRS global state as there is no other way to get that from hardware or atomic state v2: - not overwritten dp_m_n values in intel_crtc_copy_fastset() when doing a seamless_mode_switch Cc: Vidya Srinivas <vidya.srinivas@intel.com> Cc: Sean Paul <seanpaul@chromium.org> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Signed-off-by: José Roberto de Souza <jose.souza@intel.com> --- drivers/gpu/drm/i915/display/intel_ddi.c | 10 +++ drivers/gpu/drm/i915/display/intel_display.c | 61 ++++++++++++++++--- .../drm/i915/display/intel_display_debugfs.c | 3 + .../drm/i915/display/intel_display_types.h | 3 + drivers/gpu/drm/i915/display/intel_dp.c | 56 ++++++++++++++--- drivers/gpu/drm/i915/display/intel_drrs.c | 38 +++++++++--- drivers/gpu/drm/i915/display/intel_drrs.h | 3 + 7 files changed, 151 insertions(+), 23 deletions(-)