Re: [PATCH v6 3/3] drm/amd/display: Skip modeset for front porch change

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 2021-02-12 8:08 p.m., Aurabindo Pillai wrote:
[Why]
A seamless transition between modes can be performed if the new incoming
mode has the same timing parameters as the optimized mode on a display with a
variable vtotal min/max.

Smooth video playback usecases can be enabled with this seamless transition by
switching to a new mode which has a refresh rate matching the video.

[How]
Skip full modeset if userspace requested a compatible freesync mode which only
differs in the front porch timing from the current mode.

Signed-off-by: Aurabindo Pillai <aurabindo.pillai@xxxxxxx>
Acked-by: Christian König <christian.koenig@xxxxxxx>
---
  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 220 ++++++++++++++----
  1 file changed, 180 insertions(+), 40 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index c472905c7d72..628fec855e14 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -212,6 +212,9 @@ static bool amdgpu_dm_psr_disable_all(struct amdgpu_display_manager *dm);
  static const struct drm_format_info *
  amd_get_format_info(const struct drm_mode_fb_cmd2 *cmd);
+static bool
+is_timing_unchanged_for_freesync(struct drm_crtc_state *old_crtc_state,
+				 struct drm_crtc_state *new_crtc_state);
  /*
   * dm_vblank_get_counter
   *
@@ -335,6 +338,17 @@ static inline bool amdgpu_dm_vrr_active(struct dm_crtc_state *dm_state)
  	       dm_state->freesync_config.state == VRR_STATE_ACTIVE_FIXED;
  }
+static inline bool is_dc_timing_adjust_needed(struct dm_crtc_state *old_state,
+					      struct dm_crtc_state *new_state)
+{
+	if (new_state->freesync_config.state ==  VRR_STATE_ACTIVE_FIXED)
+		return true;
+	else if (amdgpu_dm_vrr_active(old_state) != amdgpu_dm_vrr_active(new_state))
+		return true;
+	else
+		return false;
+}
+
  /**
   * dm_pflip_high_irq() - Handle pageflip interrupt
   * @interrupt_params: ignored
@@ -5008,19 +5022,16 @@ static void fill_stream_properties_from_drm_display_mode(
  		timing_out->hdmi_vic = hv_frame.vic;
  	}
- timing_out->h_addressable = mode_in->crtc_hdisplay;
-	timing_out->h_total = mode_in->crtc_htotal;
-	timing_out->h_sync_width =
-		mode_in->crtc_hsync_end - mode_in->crtc_hsync_start;
-	timing_out->h_front_porch =
-		mode_in->crtc_hsync_start - mode_in->crtc_hdisplay;
-	timing_out->v_total = mode_in->crtc_vtotal;
-	timing_out->v_addressable = mode_in->crtc_vdisplay;
-	timing_out->v_front_porch =
-		mode_in->crtc_vsync_start - mode_in->crtc_vdisplay;
-	timing_out->v_sync_width =
-		mode_in->crtc_vsync_end - mode_in->crtc_vsync_start;
-	timing_out->pix_clk_100hz = mode_in->crtc_clock * 10;
+	timing_out->h_addressable = mode_in->hdisplay;
+	timing_out->h_total = mode_in->htotal;
+	timing_out->h_sync_width = mode_in->hsync_end - mode_in->hsync_start;
+	timing_out->h_front_porch = mode_in->hsync_start - mode_in->hdisplay;
+	timing_out->v_total = mode_in->vtotal;
+	timing_out->v_addressable = mode_in->vdisplay;
+	timing_out->v_front_porch = mode_in->vsync_start - mode_in->vdisplay;
+	timing_out->v_sync_width = mode_in->vsync_end - mode_in->vsync_start;
+	timing_out->pix_clk_100hz = mode_in->clock * 10;
+
  	timing_out->aspect_ratio = get_aspect_ratio(mode_in);
stream->output_color_space = get_output_color_space(timing_out);
@@ -5240,6 +5251,33 @@ get_highest_refresh_rate_mode(struct amdgpu_dm_connector *aconnector,
  	return m_pref;
  }
+static bool is_freesync_video_mode(struct drm_display_mode *mode,
+				   struct amdgpu_dm_connector *aconnector)
+{
+	struct drm_display_mode *high_mode;
+	int timing_diff;
+
+	high_mode = get_highest_refresh_rate_mode(aconnector, false);
+	if (!high_mode || !mode)
+		return false;
+
+	timing_diff = high_mode->vtotal - mode->vtotal;
+
+	if (high_mode->clock == 0 || high_mode->clock != mode->clock ||
+	    high_mode->hdisplay != mode->hdisplay ||
+	    high_mode->vdisplay != mode->vdisplay ||
+	    high_mode->hsync_start != mode->hsync_start ||
+	    high_mode->hsync_end != mode->hsync_end ||
+	    high_mode->htotal != mode->htotal ||
+	    high_mode->hskew != mode->hskew ||
+	    high_mode->vscan != mode->vscan ||
+	    high_mode->vsync_start - mode->vsync_start != timing_diff ||
+	    high_mode->vsync_end - mode->vsync_end != timing_diff)
+		return false;
+	else
+		return true;
+}
+
  static struct dc_stream_state *
  create_stream_for_sink(struct amdgpu_dm_connector *aconnector,
  		       const struct drm_display_mode *drm_mode,
@@ -5253,8 +5291,10 @@ create_stream_for_sink(struct amdgpu_dm_connector *aconnector,
  		dm_state ? &dm_state->base : NULL;
  	struct dc_stream_state *stream = NULL;
  	struct drm_display_mode mode = *drm_mode;
+	struct drm_display_mode saved_mode;
+	struct drm_display_mode *freesync_mode = NULL;
  	bool native_mode_found = false;
-	bool scale = dm_state ? (dm_state->scaling != RMX_OFF) : false;
+	bool timing_changed = dm_state ? (dm_state->scaling != RMX_OFF) : false;

Would be easier to understand if this was named "recalculate_timing".

The mode's timing we're applying to the hardware does not actually change in the case of scaling or video modes - we're only modifying the incoming timing to match the preferred mode's.

With that fixed this series is:

Reviewed-by: Nicholas Kazlauskas <nicholas.kazlauskas@xxxxxxx>

Regards,
Nicholas Kazlauskas

  	int mode_refresh;
  	int preferred_refresh = 0;
  #if defined(CONFIG_DRM_AMD_DC_DCN)
@@ -5262,6 +5302,9 @@ create_stream_for_sink(struct amdgpu_dm_connector *aconnector,
  	uint32_t link_bandwidth_kbps;
  #endif
  	struct dc_sink *sink = NULL;
+
+	memset(&saved_mode, 0, sizeof(saved_mode));
+
  	if (aconnector == NULL) {
  		DRM_ERROR("aconnector is NULL!\n");
  		return stream;
@@ -5314,25 +5357,38 @@ create_stream_for_sink(struct amdgpu_dm_connector *aconnector,
  		 */
  		DRM_DEBUG_DRIVER("No preferred mode found\n");
  	} else {
-		decide_crtc_timing_for_drm_display_mode(
+		timing_changed |= amdgpu_freesync_vid_mode &&
+				 is_freesync_video_mode(&mode, aconnector);
+		if (timing_changed) {
+			freesync_mode = get_highest_refresh_rate_mode(aconnector, false);
+			saved_mode = mode;
+			mode = *freesync_mode;
+		} else {
+			decide_crtc_timing_for_drm_display_mode(
  				&mode, preferred_mode,
  				dm_state ? (dm_state->scaling != RMX_OFF) : false);
+		}
+
  		preferred_refresh = drm_mode_vrefresh(preferred_mode);
  	}
- if (!dm_state)
+	if (timing_changed)
+		drm_mode_set_crtcinfo(&saved_mode, 0);
+	else
  		drm_mode_set_crtcinfo(&mode, 0);
- /*
+       /*
  	* If scaling is enabled and refresh rate didn't change
  	* we copy the vic and polarities of the old timings
  	*/
-	if (!scale || mode_refresh != preferred_refresh)
-		fill_stream_properties_from_drm_display_mode(stream,
-			&mode, &aconnector->base, con_state, NULL, requested_bpc);
+	if (!timing_changed || mode_refresh != preferred_refresh)
+		fill_stream_properties_from_drm_display_mode(
+			stream, &mode, &aconnector->base, con_state, NULL,
+			requested_bpc);
  	else
-		fill_stream_properties_from_drm_display_mode(stream,
-			&mode, &aconnector->base, con_state, old_stream, requested_bpc);
+		fill_stream_properties_from_drm_display_mode(
+			stream, &mode, &aconnector->base, con_state, old_stream,
+			requested_bpc);
stream->timing.flags.DSC = 0; @@ -7143,14 +7199,10 @@ static uint add_fs_modes(struct amdgpu_dm_connector *aconnector)
  static void amdgpu_dm_connector_add_freesync_modes(struct drm_connector *connector,
  						   struct edid *edid)
  {
-	uint8_t i;
-	struct detailed_timing *timing;
-	struct detailed_non_pixel *data;
-	struct detailed_data_monitor_range *range;
  	struct amdgpu_dm_connector *amdgpu_dm_connector =
  		to_amdgpu_dm_connector(connector);
- if (!(amdgpu_exp_freesync_vid_mode && edid))
+	if (!(amdgpu_freesync_vid_mode && edid))
  		return;
  	
  	if (amdgpu_dm_connector->max_vfreq - amdgpu_dm_connector->min_vfreq > 10)
@@ -7837,9 +7889,22 @@ static void update_stream_irq_parameters(
  	if (new_crtc_state->vrr_supported &&
  	    config.min_refresh_in_uhz &&
  	    config.max_refresh_in_uhz) {
-		config.state = new_crtc_state->base.vrr_enabled ?
-			VRR_STATE_ACTIVE_VARIABLE :
-			VRR_STATE_INACTIVE;
+		/*
+		 * if freesync compatible mode was set, config.state will be set
+		 * in atomic check
+		 */
+		if (config.state == VRR_STATE_ACTIVE_FIXED && config.fixed_refresh_in_uhz &&
+		    (!drm_atomic_crtc_needs_modeset(&new_crtc_state->base) ||
+		     new_crtc_state->freesync_config.state == VRR_STATE_ACTIVE_FIXED)) {
+			vrr_params.max_refresh_in_uhz = config.max_refresh_in_uhz;
+			vrr_params.min_refresh_in_uhz = config.min_refresh_in_uhz;
+			vrr_params.fixed_refresh_in_uhz = config.fixed_refresh_in_uhz;
+			vrr_params.state = VRR_STATE_ACTIVE_FIXED;
+		} else {
+			config.state = new_crtc_state->base.vrr_enabled ?
+						     VRR_STATE_ACTIVE_VARIABLE :
+						     VRR_STATE_INACTIVE;
+		}
  	} else {
  		config.state = VRR_STATE_UNSUPPORTED;
  	}
@@ -8160,8 +8225,7 @@ static void amdgpu_dm_commit_planes(struct drm_atomic_state *state,
  		 * re-adjust the min/max bounds now that DC doesn't handle this
  		 * as part of commit.
  		 */
-		if (amdgpu_dm_vrr_active(dm_old_crtc_state) !=
-		    amdgpu_dm_vrr_active(acrtc_state)) {
+		if (is_dc_timing_adjust_needed(dm_old_crtc_state, acrtc_state)) {
  			spin_lock_irqsave(&pcrtc->dev->event_lock, flags);
  			dc_stream_adjust_vmin_vmax(
  				dm->dc, acrtc_state->stream,
@@ -8445,6 +8509,7 @@ static void amdgpu_dm_atomic_commit_tail(struct drm_atomic_state *state)
  			/* i.e. reset mode */
  			if (dm_old_crtc_state->stream)
  				remove_stream(adev, acrtc, dm_old_crtc_state->stream);
+
  			mode_set_reset_required = true;
  		}
  	} /* for_each_crtc_in_state() */
@@ -8843,6 +8908,7 @@ static void get_freesync_config_for_crtc(
  			to_amdgpu_dm_connector(new_con_state->base.connector);
  	struct drm_display_mode *mode = &new_crtc_state->base.mode;
  	int vrefresh = drm_mode_vrefresh(mode);
+	bool fs_vid_mode = false;
new_crtc_state->vrr_supported = new_con_state->freesync_capable &&
  					vrefresh >= aconnector->min_vfreq &&
@@ -8850,17 +8916,24 @@ static void get_freesync_config_for_crtc(
if (new_crtc_state->vrr_supported) {
  		new_crtc_state->stream->ignore_msa_timing_param = true;
-		config.state = new_crtc_state->base.vrr_enabled ?
-				VRR_STATE_ACTIVE_VARIABLE :
-				VRR_STATE_INACTIVE;
-		config.min_refresh_in_uhz =
-				aconnector->min_vfreq * 1000000;
-		config.max_refresh_in_uhz =
-				aconnector->max_vfreq * 1000000;
+		fs_vid_mode = new_crtc_state->freesync_config.state == VRR_STATE_ACTIVE_FIXED;
+
+		config.min_refresh_in_uhz = aconnector->min_vfreq * 1000000;
+		config.max_refresh_in_uhz = aconnector->max_vfreq * 1000000;
  		config.vsif_supported = true;
  		config.btr = true;
-	}
+ if (fs_vid_mode) {
+			config.state = VRR_STATE_ACTIVE_FIXED;
+			config.fixed_refresh_in_uhz = new_crtc_state->freesync_config.fixed_refresh_in_uhz;
+			goto out;
+		} else if (new_crtc_state->base.vrr_enabled) {
+			config.state = VRR_STATE_ACTIVE_VARIABLE;
+		} else {
+			config.state = VRR_STATE_INACTIVE;
+		}
+	}
+out:
  	new_crtc_state->freesync_config = config;
  }
@@ -8873,6 +8946,50 @@ static void reset_freesync_config_for_crtc(
  	       sizeof(new_crtc_state->vrr_infopacket));
  }
+static bool
+is_timing_unchanged_for_freesync(struct drm_crtc_state *old_crtc_state,
+				 struct drm_crtc_state *new_crtc_state)
+{
+	struct drm_display_mode old_mode, new_mode;
+
+	if (!old_crtc_state || !new_crtc_state)
+		return false;
+
+	old_mode = old_crtc_state->mode;
+	new_mode = new_crtc_state->mode;
+
+	if (old_mode.clock       == new_mode.clock &&
+	    old_mode.hdisplay    == new_mode.hdisplay &&
+	    old_mode.vdisplay    == new_mode.vdisplay &&
+	    old_mode.htotal      == new_mode.htotal &&
+	    old_mode.vtotal      != new_mode.vtotal &&
+	    old_mode.hsync_start == new_mode.hsync_start &&
+	    old_mode.vsync_start != new_mode.vsync_start &&
+	    old_mode.hsync_end   == new_mode.hsync_end &&
+	    old_mode.vsync_end   != new_mode.vsync_end &&
+	    old_mode.hskew       == new_mode.hskew &&
+	    old_mode.vscan       == new_mode.vscan &&
+	    (old_mode.vsync_end - old_mode.vsync_start) ==
+	    (new_mode.vsync_end - new_mode.vsync_start))
+		return true;
+
+	return false;
+}
+
+static void set_freesync_fixed_config(struct dm_crtc_state *dm_new_crtc_state) {
+	uint64_t num, den, res;
+	struct drm_crtc_state *new_crtc_state = &dm_new_crtc_state->base;
+
+	dm_new_crtc_state->freesync_config.state = VRR_STATE_ACTIVE_FIXED;
+
+	num = (unsigned long long)new_crtc_state->mode.clock * 1000 * 1000000;
+	den = (unsigned long long)new_crtc_state->mode.htotal *
+	      (unsigned long long)new_crtc_state->mode.vtotal;
+
+	res = div_u64(num, den);
+	dm_new_crtc_state->freesync_config.fixed_refresh_in_uhz = res;
+}
+
  static int dm_update_crtc_state(struct amdgpu_display_manager *dm,
  				struct drm_atomic_state *state,
  				struct drm_crtc *crtc,
@@ -8963,6 +9080,11 @@ static int dm_update_crtc_state(struct amdgpu_display_manager *dm,
  		 * TODO: Refactor this function to allow this check to work
  		 * in all conditions.
  		 */
+		if (amdgpu_freesync_vid_mode &&
+		    dm_new_crtc_state->stream &&
+		    is_timing_unchanged_for_freesync(new_crtc_state, old_crtc_state))
+			goto skip_modeset;
+
  		if (dm_new_crtc_state->stream &&
  		    dc_is_stream_unchanged(new_stream, dm_old_crtc_state->stream) &&
  		    dc_is_stream_scaling_unchanged(new_stream, dm_old_crtc_state->stream)) {
@@ -8994,6 +9116,24 @@ static int dm_update_crtc_state(struct amdgpu_display_manager *dm,
  		if (!dm_old_crtc_state->stream)
  			goto skip_modeset;
+ if (amdgpu_freesync_vid_mode && dm_new_crtc_state->stream &&
+		    is_timing_unchanged_for_freesync(new_crtc_state,
+						     old_crtc_state)) {
+			new_crtc_state->mode_changed = false;
+			DRM_DEBUG_DRIVER(
+				"Mode change not required for front porch change, "
+				"setting mode_changed to %d",
+				new_crtc_state->mode_changed);
+
+			set_freesync_fixed_config(dm_new_crtc_state);
+
+			goto skip_modeset;
+		} else if (amdgpu_freesync_vid_mode && aconnector &&
+			   is_freesync_video_mode(&new_crtc_state->mode,
+						  aconnector)) {
+			set_freesync_fixed_config(dm_new_crtc_state);
+		}
+
  		ret = dm_atomic_get_state(state, &dm_state);
  		if (ret)
  			goto fail;


_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux