All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v6 0/3] Freesync video mode optimization
@ 2021-02-13  1:08 ` Aurabindo Pillai
  0 siblings, 0 replies; 13+ messages in thread
From: Aurabindo Pillai @ 2021-02-13  1:08 UTC (permalink / raw)
  To: amd-gfx, dri-devel
  Cc: stylon.wang, shashank.sharma, aurabindo.pillai, wayne.lin,
	alexander.deucher, nicholas.kazlauskas

Changes in V6
=============

1) Skip modeset for front porch change
  * Minor optimizations

2) Add freesync video modes based on preferred modes:
  * Remove edid parsing and use the already parsed valuescached in dm
    for the monitor's freesync range

3) Add module parameter for freesync video mode
  * Add info about potential higher power consumption when using this
    feature in the commit message as a rationale for keeping this option
    disabled by default.

Changes in V5
=============

* More info in commit messages on the rationale of changes being added
to the kernel.
* Minor fixes

Changes in V4
=============

1) Add module parameter for freesync video mode

* Change module parameter name to freesync_video

2) Add freesync video modes based on preferred modes:

* Cosmetic fixes
* Added comments about all modes being added by the driver.

3) Skip modeset for front porch change

* Added more conditions for checking freesync video mode

Changes in V3
=============

1) Add freesync video modes based on preferred modes:

* Cache base freesync video mode during the first iteration to avoid
  iterating over modelist again later.
* Add mode for 60 fps videos

2) Skip modeset for front porch change

* Fixes for bug exposed by caching of modes.

Changes in V2
=============

1) Add freesync video modes based on preferred modes:

* Remove check for connector type before adding freesync compatible
  modes as VRR support is being checked, and there is no reason to block
  freesync video support on eDP.
* use drm_mode_equal() instead of creating same functionality.
* Additional null pointer deference check
* Removed unnecessary variables.
* Cosmetic fixes.

2) Skip modeset for front porch change

* Remove _FSV string being appended to freesync video modes so as to not
  define new policies or break existing application that might use the
  mode name to figure out mode resolution.
* Remove unnecessary variables
* Cosmetic fixes.


This patchset enables freesync video mode usecase where the userspace
can request a freesync compatible video mode such that switching to this
mode does not trigger blanking.

This feature is guarded by a module parameter which is disabled by
default (due to slightly higher power consumption). Enabling this
paramters adds additional modes to the driver modelist, and also
enables the optimization to skip modeset when using one of these modes.

---
---

Aurabindo Pillai (3):
  drm/amd/display: Add module parameter for freesync video mode
  drm/amd/display: Add freesync video modes based on preferred modes
  drm/amd/display: Skip modeset for front porch change

 drivers/gpu/drm/amd/amdgpu/amdgpu.h           |   1 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c       |  12 +
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 369 ++++++++++++++++--
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h |   2 +
 4 files changed, 349 insertions(+), 35 deletions(-)

-- 
2.30.0

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH v6 0/3] Freesync video mode optimization
@ 2021-02-13  1:08 ` Aurabindo Pillai
  0 siblings, 0 replies; 13+ messages in thread
From: Aurabindo Pillai @ 2021-02-13  1:08 UTC (permalink / raw)
  To: amd-gfx, dri-devel
  Cc: stylon.wang, shashank.sharma, aurabindo.pillai, wayne.lin,
	alexander.deucher, Harry.Wentland, nicholas.kazlauskas

Changes in V6
=============

1) Skip modeset for front porch change
  * Minor optimizations

2) Add freesync video modes based on preferred modes:
  * Remove edid parsing and use the already parsed valuescached in dm
    for the monitor's freesync range

3) Add module parameter for freesync video mode
  * Add info about potential higher power consumption when using this
    feature in the commit message as a rationale for keeping this option
    disabled by default.

Changes in V5
=============

* More info in commit messages on the rationale of changes being added
to the kernel.
* Minor fixes

Changes in V4
=============

1) Add module parameter for freesync video mode

* Change module parameter name to freesync_video

2) Add freesync video modes based on preferred modes:

* Cosmetic fixes
* Added comments about all modes being added by the driver.

3) Skip modeset for front porch change

* Added more conditions for checking freesync video mode

Changes in V3
=============

1) Add freesync video modes based on preferred modes:

* Cache base freesync video mode during the first iteration to avoid
  iterating over modelist again later.
* Add mode for 60 fps videos

2) Skip modeset for front porch change

* Fixes for bug exposed by caching of modes.

Changes in V2
=============

1) Add freesync video modes based on preferred modes:

* Remove check for connector type before adding freesync compatible
  modes as VRR support is being checked, and there is no reason to block
  freesync video support on eDP.
* use drm_mode_equal() instead of creating same functionality.
* Additional null pointer deference check
* Removed unnecessary variables.
* Cosmetic fixes.

2) Skip modeset for front porch change

* Remove _FSV string being appended to freesync video modes so as to not
  define new policies or break existing application that might use the
  mode name to figure out mode resolution.
* Remove unnecessary variables
* Cosmetic fixes.


This patchset enables freesync video mode usecase where the userspace
can request a freesync compatible video mode such that switching to this
mode does not trigger blanking.

This feature is guarded by a module parameter which is disabled by
default (due to slightly higher power consumption). Enabling this
paramters adds additional modes to the driver modelist, and also
enables the optimization to skip modeset when using one of these modes.

---
---

Aurabindo Pillai (3):
  drm/amd/display: Add module parameter for freesync video mode
  drm/amd/display: Add freesync video modes based on preferred modes
  drm/amd/display: Skip modeset for front porch change

 drivers/gpu/drm/amd/amdgpu/amdgpu.h           |   1 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c       |  12 +
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 369 ++++++++++++++++--
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h |   2 +
 4 files changed, 349 insertions(+), 35 deletions(-)

-- 
2.30.0

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH v6 1/3] drm/amd/display: Add module parameter for freesync video mode
  2021-02-13  1:08 ` Aurabindo Pillai
@ 2021-02-13  1:08   ` Aurabindo Pillai
  -1 siblings, 0 replies; 13+ messages in thread
From: Aurabindo Pillai @ 2021-02-13  1:08 UTC (permalink / raw)
  To: amd-gfx, dri-devel
  Cc: stylon.wang, shashank.sharma, aurabindo.pillai, wayne.lin,
	alexander.deucher, nicholas.kazlauskas

[Why]
This option shall be opt-in by default since it is a temporary solution
until long term solution is agreed upon which may require userspace interface
changes. This feature give the user a seamless experience when freesync aware
programs (media players for instance) switches to a compatible freesync mode
when playing videos. Enabling this feature also have the potential side effect
of causing higher power consumption due to running a mode with lower resolution
and base clock frequency with the highest base clock supported on the monitor as
per its advertised modes. There has been precedent of manufacturing modes in the
kernel. In AMDGPU, the existing usage are for common modes and scaling modes.
Other driver have a similar approach as well.

[How]
Adds a module parameter to enable freesync video mode modeset
optimization. Enabling this mode allows the driver to skip a full modeset when a
freesync compatible mode is requested by the userspace. This parameter will also
add some additional modes that are within the connected monitor's VRR range
corresponding to common video modes, which media players can use for a seamless
experience while making use of freesync.

Signed-off-by: Aurabindo Pillai <aurabindo.pillai@amd.com>
Acked-by: Christian König <christian.koenig at amd.com>
Reviewed-by: Shashank Sharma <shashank.sharma@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h     |  1 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 12 ++++++++++++
 2 files changed, 13 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index e3d4d2dcb3a0..e242b7607dca 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -179,6 +179,7 @@ extern int amdgpu_gpu_recovery;
 extern int amdgpu_emu_mode;
 extern uint amdgpu_smu_memory_pool_size;
 extern uint amdgpu_dc_feature_mask;
+extern uint amdgpu_freesync_vid_mode;
 extern uint amdgpu_dc_debug_mask;
 extern uint amdgpu_dm_abm_level;
 extern struct amdgpu_mgpu_info mgpu_info;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index b7ee587484b2..aefbe14c30fa 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -162,6 +162,7 @@ int amdgpu_mes;
 int amdgpu_noretry = -1;
 int amdgpu_force_asic_type = -1;
 int amdgpu_tmz;
+uint amdgpu_freesync_vid_mode;
 int amdgpu_reset_method = -1; /* auto */
 int amdgpu_num_kcq = -1;
 
@@ -790,6 +791,17 @@ module_param_named(abmlevel, amdgpu_dm_abm_level, uint, 0444);
 MODULE_PARM_DESC(tmz, "Enable TMZ feature (-1 = auto, 0 = off (default), 1 = on)");
 module_param_named(tmz, amdgpu_tmz, int, 0444);
 
+/**
+ * DOC: freesync_video (uint)
+ * Enabled the optimization to adjust front porch timing to achieve seamless mode change experience
+ * when setting a freesync supported mode for which full modeset is not needed.
+ * The default value: 0 (off).
+ */
+MODULE_PARM_DESC(
+	freesync_video,
+	"Enable freesync modesetting optimization feature (0 = off (default), 1 = on)");
+module_param_named(freesync_video, amdgpu_freesync_vid_mode, uint, 0444);
+
 /**
  * DOC: reset_method (int)
  * GPU reset method (-1 = auto (default), 0 = legacy, 1 = mode0, 2 = mode1, 3 = mode2, 4 = baco, 5 = pci)
-- 
2.30.0

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

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v6 1/3] drm/amd/display: Add module parameter for freesync video mode
@ 2021-02-13  1:08   ` Aurabindo Pillai
  0 siblings, 0 replies; 13+ messages in thread
From: Aurabindo Pillai @ 2021-02-13  1:08 UTC (permalink / raw)
  To: amd-gfx, dri-devel
  Cc: stylon.wang, shashank.sharma, aurabindo.pillai, wayne.lin,
	alexander.deucher, Harry.Wentland, nicholas.kazlauskas

[Why]
This option shall be opt-in by default since it is a temporary solution
until long term solution is agreed upon which may require userspace interface
changes. This feature give the user a seamless experience when freesync aware
programs (media players for instance) switches to a compatible freesync mode
when playing videos. Enabling this feature also have the potential side effect
of causing higher power consumption due to running a mode with lower resolution
and base clock frequency with the highest base clock supported on the monitor as
per its advertised modes. There has been precedent of manufacturing modes in the
kernel. In AMDGPU, the existing usage are for common modes and scaling modes.
Other driver have a similar approach as well.

[How]
Adds a module parameter to enable freesync video mode modeset
optimization. Enabling this mode allows the driver to skip a full modeset when a
freesync compatible mode is requested by the userspace. This parameter will also
add some additional modes that are within the connected monitor's VRR range
corresponding to common video modes, which media players can use for a seamless
experience while making use of freesync.

Signed-off-by: Aurabindo Pillai <aurabindo.pillai@amd.com>
Acked-by: Christian König <christian.koenig at amd.com>
Reviewed-by: Shashank Sharma <shashank.sharma@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h     |  1 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 12 ++++++++++++
 2 files changed, 13 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index e3d4d2dcb3a0..e242b7607dca 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -179,6 +179,7 @@ extern int amdgpu_gpu_recovery;
 extern int amdgpu_emu_mode;
 extern uint amdgpu_smu_memory_pool_size;
 extern uint amdgpu_dc_feature_mask;
+extern uint amdgpu_freesync_vid_mode;
 extern uint amdgpu_dc_debug_mask;
 extern uint amdgpu_dm_abm_level;
 extern struct amdgpu_mgpu_info mgpu_info;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index b7ee587484b2..aefbe14c30fa 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -162,6 +162,7 @@ int amdgpu_mes;
 int amdgpu_noretry = -1;
 int amdgpu_force_asic_type = -1;
 int amdgpu_tmz;
+uint amdgpu_freesync_vid_mode;
 int amdgpu_reset_method = -1; /* auto */
 int amdgpu_num_kcq = -1;
 
@@ -790,6 +791,17 @@ module_param_named(abmlevel, amdgpu_dm_abm_level, uint, 0444);
 MODULE_PARM_DESC(tmz, "Enable TMZ feature (-1 = auto, 0 = off (default), 1 = on)");
 module_param_named(tmz, amdgpu_tmz, int, 0444);
 
+/**
+ * DOC: freesync_video (uint)
+ * Enabled the optimization to adjust front porch timing to achieve seamless mode change experience
+ * when setting a freesync supported mode for which full modeset is not needed.
+ * The default value: 0 (off).
+ */
+MODULE_PARM_DESC(
+	freesync_video,
+	"Enable freesync modesetting optimization feature (0 = off (default), 1 = on)");
+module_param_named(freesync_video, amdgpu_freesync_vid_mode, uint, 0444);
+
 /**
  * DOC: reset_method (int)
  * GPU reset method (-1 = auto (default), 0 = legacy, 1 = mode0, 2 = mode1, 3 = mode2, 4 = baco, 5 = pci)
-- 
2.30.0

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v6 2/3] drm/amd/display: Add freesync video modes based on preferred modes
  2021-02-13  1:08 ` Aurabindo Pillai
@ 2021-02-13  1:08   ` Aurabindo Pillai
  -1 siblings, 0 replies; 13+ messages in thread
From: Aurabindo Pillai @ 2021-02-13  1:08 UTC (permalink / raw)
  To: amd-gfx, dri-devel
  Cc: stylon.wang, shashank.sharma, Christian König,
	aurabindo.pillai, wayne.lin, alexander.deucher,
	nicholas.kazlauskas

[Why]
While possible for userspace to create and add custom mode based off the
optimized mode for the connected display which differs only in front porch
timing, this patch set adds a list of common video modes in advance.

The list of common video refresh rates is small, well known and the optimized
mode has specific requirements to be able to enable HW frame doubling and
tripling so it makes most sense to create the modes that video players will need
in advance. The optimized mode matches the preferred mode resolution but has the
highest refresh rate available to enable the largest front porch extension.

[How]
Find the optimized mode and store it on the connector so we can check it
later during our optimized modeset.

Prepopulate the mode list with a list of common video mades based on the
optimized mode (but with a longer front porch) if the panel doesn't support a
variant of the mode natively.

Signed-off-by: Aurabindo Pillai <aurabindo.pillai@amd.com>
Acked-by: Christian König <christian.koenig@amd.com>
Reviewed-by: Shashank Sharma <shashank.sharma@amd.com>
---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 159 ++++++++++++++++++
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h |   2 +
 2 files changed, 161 insertions(+)

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 626a8cc92d65..c472905c7d72 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -5187,6 +5187,59 @@ static void dm_enable_per_frame_crtc_master_sync(struct dc_state *context)
 	set_master_stream(context->streams, context->stream_count);
 }
 
+static struct drm_display_mode *
+get_highest_refresh_rate_mode(struct amdgpu_dm_connector *aconnector,
+			  bool use_probed_modes)
+{
+	struct drm_display_mode *m, *m_pref = NULL;
+	u16 current_refresh, highest_refresh;
+	struct list_head *list_head = use_probed_modes ?
+						    &aconnector->base.probed_modes :
+						    &aconnector->base.modes;
+
+	if (aconnector->freesync_vid_base.clock != 0)
+		return &aconnector->freesync_vid_base;
+
+	/* Find the preferred mode */
+	list_for_each_entry (m, list_head, head) {
+		if (m->type & DRM_MODE_TYPE_PREFERRED) {
+			m_pref = m;
+			break;
+		}
+	}
+
+	if (!m_pref) {
+		/* Probably an EDID with no preferred mode. Fallback to first entry */
+		m_pref = list_first_entry_or_null(
+			&aconnector->base.modes, struct drm_display_mode, head);
+		if (!m_pref) {
+			DRM_DEBUG_DRIVER("No preferred mode found in EDID\n");
+			return NULL;
+		}
+	}
+
+	highest_refresh = drm_mode_vrefresh(m_pref);
+
+	/*
+	 * Find the mode with highest refresh rate with same resolution.
+	 * For some monitors, preferred mode is not the mode with highest
+	 * supported refresh rate.
+	 */
+	list_for_each_entry (m, list_head, head) {
+		current_refresh  = drm_mode_vrefresh(m);
+
+		if (m->hdisplay == m_pref->hdisplay &&
+		    m->vdisplay == m_pref->vdisplay &&
+		    highest_refresh < current_refresh) {
+			highest_refresh = current_refresh;
+			m_pref = m;
+		}
+	}
+
+	aconnector->freesync_vid_base = *m_pref;
+	return m_pref;
+}
+
 static struct dc_stream_state *
 create_stream_for_sink(struct amdgpu_dm_connector *aconnector,
 		       const struct drm_display_mode *drm_mode,
@@ -7000,6 +7053,111 @@ static void amdgpu_dm_connector_ddc_get_modes(struct drm_connector *connector,
 	}
 }
 
+static bool is_duplicate_mode(struct amdgpu_dm_connector *aconnector,
+			      struct drm_display_mode *mode)
+{
+	struct drm_display_mode *m;
+
+	list_for_each_entry (m, &aconnector->base.probed_modes, head) {
+		if (drm_mode_equal(m, mode))
+			return true;
+	}
+
+	return false;
+}
+
+static uint add_fs_modes(struct amdgpu_dm_connector *aconnector)
+{
+	const struct drm_display_mode *m;
+	struct drm_display_mode *new_mode;
+	uint i;
+	uint32_t new_modes_count = 0;
+
+	/* Standard FPS values
+	 *
+	 * 23.976   - TV/NTSC
+	 * 24 	    - Cinema
+	 * 25 	    - TV/PAL
+	 * 29.97    - TV/NTSC
+	 * 30 	    - TV/NTSC
+	 * 48 	    - Cinema HFR
+	 * 50 	    - TV/PAL
+	 * 60 	    - Commonly used
+	 * 48,72,96 - Multiples of 24
+	 */
+	const uint32_t common_rates[] = { 23976, 24000, 25000, 29970, 30000,
+					 48000, 50000, 60000, 72000, 96000 };
+
+	/*
+	 * Find mode with highest refresh rate with the same resolution
+	 * as the preferred mode. Some monitors report a preferred mode
+	 * with lower resolution than the highest refresh rate supported.
+	 */
+
+	m = get_highest_refresh_rate_mode(aconnector, true);
+	if (!m)
+		return 0;
+
+	for (i = 0; i < ARRAY_SIZE(common_rates); i++) {
+		uint64_t target_vtotal, target_vtotal_diff;
+		uint64_t num, den;
+
+		if (drm_mode_vrefresh(m) * 1000 < common_rates[i])
+			continue;
+
+		if (common_rates[i] < aconnector->min_vfreq * 1000 ||
+		    common_rates[i] > aconnector->max_vfreq * 1000)
+			continue;
+
+		num = (unsigned long long)m->clock * 1000 * 1000;
+		den = common_rates[i] * (unsigned long long)m->htotal;
+		target_vtotal = div_u64(num, den);
+		target_vtotal_diff = target_vtotal - m->vtotal;
+
+		/* Check for illegal modes */
+		if (m->vsync_start + target_vtotal_diff < m->vdisplay ||
+		    m->vsync_end + target_vtotal_diff < m->vsync_start ||
+		    m->vtotal + target_vtotal_diff < m->vsync_end)
+			continue;
+
+		new_mode = drm_mode_duplicate(aconnector->base.dev, m);
+		if (!new_mode)
+			goto out;
+
+		new_mode->vtotal += (u16)target_vtotal_diff;
+		new_mode->vsync_start += (u16)target_vtotal_diff;
+		new_mode->vsync_end += (u16)target_vtotal_diff;
+		new_mode->type &= ~DRM_MODE_TYPE_PREFERRED;
+		new_mode->type |= DRM_MODE_TYPE_DRIVER;
+
+		if (!is_duplicate_mode(aconnector, new_mode)) {
+			drm_mode_probed_add(&aconnector->base, new_mode);
+			new_modes_count += 1;
+		} else
+			drm_mode_destroy(aconnector->base.dev, new_mode);
+	}
+ out:
+	return new_modes_count;
+}
+
+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))
+		return;
+	
+	if (amdgpu_dm_connector->max_vfreq - amdgpu_dm_connector->min_vfreq > 10)
+		amdgpu_dm_connector->num_modes +=
+			add_fs_modes(amdgpu_dm_connector);
+}
+
 static int amdgpu_dm_connector_get_modes(struct drm_connector *connector)
 {
 	struct amdgpu_dm_connector *amdgpu_dm_connector =
@@ -7015,6 +7173,7 @@ static int amdgpu_dm_connector_get_modes(struct drm_connector *connector)
 	} else {
 		amdgpu_dm_connector_ddc_get_modes(connector, edid);
 		amdgpu_dm_connector_add_common_modes(encoder, connector);
+		amdgpu_dm_connector_add_freesync_modes(connector, edid);
 	}
 	amdgpu_dm_fbc_init(connector);
 
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
index 38bc0f88b29c..747e410f50b9 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
@@ -413,6 +413,8 @@ struct amdgpu_dm_connector {
 #endif
 	bool force_yuv420_output;
 	struct dsc_preferred_settings dsc_settings;
+	/* Cached display modes */
+	struct drm_display_mode freesync_vid_base;
 };
 
 #define to_amdgpu_dm_connector(x) container_of(x, struct amdgpu_dm_connector, base)
-- 
2.30.0

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

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v6 2/3] drm/amd/display: Add freesync video modes based on preferred modes
@ 2021-02-13  1:08   ` Aurabindo Pillai
  0 siblings, 0 replies; 13+ messages in thread
From: Aurabindo Pillai @ 2021-02-13  1:08 UTC (permalink / raw)
  To: amd-gfx, dri-devel
  Cc: stylon.wang, shashank.sharma, Christian König,
	aurabindo.pillai, wayne.lin, alexander.deucher, Harry.Wentland,
	nicholas.kazlauskas

[Why]
While possible for userspace to create and add custom mode based off the
optimized mode for the connected display which differs only in front porch
timing, this patch set adds a list of common video modes in advance.

The list of common video refresh rates is small, well known and the optimized
mode has specific requirements to be able to enable HW frame doubling and
tripling so it makes most sense to create the modes that video players will need
in advance. The optimized mode matches the preferred mode resolution but has the
highest refresh rate available to enable the largest front porch extension.

[How]
Find the optimized mode and store it on the connector so we can check it
later during our optimized modeset.

Prepopulate the mode list with a list of common video mades based on the
optimized mode (but with a longer front porch) if the panel doesn't support a
variant of the mode natively.

Signed-off-by: Aurabindo Pillai <aurabindo.pillai@amd.com>
Acked-by: Christian König <christian.koenig@amd.com>
Reviewed-by: Shashank Sharma <shashank.sharma@amd.com>
---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 159 ++++++++++++++++++
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h |   2 +
 2 files changed, 161 insertions(+)

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 626a8cc92d65..c472905c7d72 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -5187,6 +5187,59 @@ static void dm_enable_per_frame_crtc_master_sync(struct dc_state *context)
 	set_master_stream(context->streams, context->stream_count);
 }
 
+static struct drm_display_mode *
+get_highest_refresh_rate_mode(struct amdgpu_dm_connector *aconnector,
+			  bool use_probed_modes)
+{
+	struct drm_display_mode *m, *m_pref = NULL;
+	u16 current_refresh, highest_refresh;
+	struct list_head *list_head = use_probed_modes ?
+						    &aconnector->base.probed_modes :
+						    &aconnector->base.modes;
+
+	if (aconnector->freesync_vid_base.clock != 0)
+		return &aconnector->freesync_vid_base;
+
+	/* Find the preferred mode */
+	list_for_each_entry (m, list_head, head) {
+		if (m->type & DRM_MODE_TYPE_PREFERRED) {
+			m_pref = m;
+			break;
+		}
+	}
+
+	if (!m_pref) {
+		/* Probably an EDID with no preferred mode. Fallback to first entry */
+		m_pref = list_first_entry_or_null(
+			&aconnector->base.modes, struct drm_display_mode, head);
+		if (!m_pref) {
+			DRM_DEBUG_DRIVER("No preferred mode found in EDID\n");
+			return NULL;
+		}
+	}
+
+	highest_refresh = drm_mode_vrefresh(m_pref);
+
+	/*
+	 * Find the mode with highest refresh rate with same resolution.
+	 * For some monitors, preferred mode is not the mode with highest
+	 * supported refresh rate.
+	 */
+	list_for_each_entry (m, list_head, head) {
+		current_refresh  = drm_mode_vrefresh(m);
+
+		if (m->hdisplay == m_pref->hdisplay &&
+		    m->vdisplay == m_pref->vdisplay &&
+		    highest_refresh < current_refresh) {
+			highest_refresh = current_refresh;
+			m_pref = m;
+		}
+	}
+
+	aconnector->freesync_vid_base = *m_pref;
+	return m_pref;
+}
+
 static struct dc_stream_state *
 create_stream_for_sink(struct amdgpu_dm_connector *aconnector,
 		       const struct drm_display_mode *drm_mode,
@@ -7000,6 +7053,111 @@ static void amdgpu_dm_connector_ddc_get_modes(struct drm_connector *connector,
 	}
 }
 
+static bool is_duplicate_mode(struct amdgpu_dm_connector *aconnector,
+			      struct drm_display_mode *mode)
+{
+	struct drm_display_mode *m;
+
+	list_for_each_entry (m, &aconnector->base.probed_modes, head) {
+		if (drm_mode_equal(m, mode))
+			return true;
+	}
+
+	return false;
+}
+
+static uint add_fs_modes(struct amdgpu_dm_connector *aconnector)
+{
+	const struct drm_display_mode *m;
+	struct drm_display_mode *new_mode;
+	uint i;
+	uint32_t new_modes_count = 0;
+
+	/* Standard FPS values
+	 *
+	 * 23.976   - TV/NTSC
+	 * 24 	    - Cinema
+	 * 25 	    - TV/PAL
+	 * 29.97    - TV/NTSC
+	 * 30 	    - TV/NTSC
+	 * 48 	    - Cinema HFR
+	 * 50 	    - TV/PAL
+	 * 60 	    - Commonly used
+	 * 48,72,96 - Multiples of 24
+	 */
+	const uint32_t common_rates[] = { 23976, 24000, 25000, 29970, 30000,
+					 48000, 50000, 60000, 72000, 96000 };
+
+	/*
+	 * Find mode with highest refresh rate with the same resolution
+	 * as the preferred mode. Some monitors report a preferred mode
+	 * with lower resolution than the highest refresh rate supported.
+	 */
+
+	m = get_highest_refresh_rate_mode(aconnector, true);
+	if (!m)
+		return 0;
+
+	for (i = 0; i < ARRAY_SIZE(common_rates); i++) {
+		uint64_t target_vtotal, target_vtotal_diff;
+		uint64_t num, den;
+
+		if (drm_mode_vrefresh(m) * 1000 < common_rates[i])
+			continue;
+
+		if (common_rates[i] < aconnector->min_vfreq * 1000 ||
+		    common_rates[i] > aconnector->max_vfreq * 1000)
+			continue;
+
+		num = (unsigned long long)m->clock * 1000 * 1000;
+		den = common_rates[i] * (unsigned long long)m->htotal;
+		target_vtotal = div_u64(num, den);
+		target_vtotal_diff = target_vtotal - m->vtotal;
+
+		/* Check for illegal modes */
+		if (m->vsync_start + target_vtotal_diff < m->vdisplay ||
+		    m->vsync_end + target_vtotal_diff < m->vsync_start ||
+		    m->vtotal + target_vtotal_diff < m->vsync_end)
+			continue;
+
+		new_mode = drm_mode_duplicate(aconnector->base.dev, m);
+		if (!new_mode)
+			goto out;
+
+		new_mode->vtotal += (u16)target_vtotal_diff;
+		new_mode->vsync_start += (u16)target_vtotal_diff;
+		new_mode->vsync_end += (u16)target_vtotal_diff;
+		new_mode->type &= ~DRM_MODE_TYPE_PREFERRED;
+		new_mode->type |= DRM_MODE_TYPE_DRIVER;
+
+		if (!is_duplicate_mode(aconnector, new_mode)) {
+			drm_mode_probed_add(&aconnector->base, new_mode);
+			new_modes_count += 1;
+		} else
+			drm_mode_destroy(aconnector->base.dev, new_mode);
+	}
+ out:
+	return new_modes_count;
+}
+
+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))
+		return;
+	
+	if (amdgpu_dm_connector->max_vfreq - amdgpu_dm_connector->min_vfreq > 10)
+		amdgpu_dm_connector->num_modes +=
+			add_fs_modes(amdgpu_dm_connector);
+}
+
 static int amdgpu_dm_connector_get_modes(struct drm_connector *connector)
 {
 	struct amdgpu_dm_connector *amdgpu_dm_connector =
@@ -7015,6 +7173,7 @@ static int amdgpu_dm_connector_get_modes(struct drm_connector *connector)
 	} else {
 		amdgpu_dm_connector_ddc_get_modes(connector, edid);
 		amdgpu_dm_connector_add_common_modes(encoder, connector);
+		amdgpu_dm_connector_add_freesync_modes(connector, edid);
 	}
 	amdgpu_dm_fbc_init(connector);
 
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
index 38bc0f88b29c..747e410f50b9 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
@@ -413,6 +413,8 @@ struct amdgpu_dm_connector {
 #endif
 	bool force_yuv420_output;
 	struct dsc_preferred_settings dsc_settings;
+	/* Cached display modes */
+	struct drm_display_mode freesync_vid_base;
 };
 
 #define to_amdgpu_dm_connector(x) container_of(x, struct amdgpu_dm_connector, base)
-- 
2.30.0

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v6 3/3] drm/amd/display: Skip modeset for front porch change
  2021-02-13  1:08 ` Aurabindo Pillai
@ 2021-02-13  1:08   ` Aurabindo Pillai
  -1 siblings, 0 replies; 13+ messages in thread
From: Aurabindo Pillai @ 2021-02-13  1:08 UTC (permalink / raw)
  To: amd-gfx, dri-devel
  Cc: stylon.wang, shashank.sharma, Christian König,
	aurabindo.pillai, wayne.lin, alexander.deucher,
	nicholas.kazlauskas

[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@amd.com>
Acked-by: Christian König <christian.koenig@amd.com>
---
 .../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;
 	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;
-- 
2.30.0

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

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v6 3/3] drm/amd/display: Skip modeset for front porch change
@ 2021-02-13  1:08   ` Aurabindo Pillai
  0 siblings, 0 replies; 13+ messages in thread
From: Aurabindo Pillai @ 2021-02-13  1:08 UTC (permalink / raw)
  To: amd-gfx, dri-devel
  Cc: stylon.wang, shashank.sharma, Christian König,
	aurabindo.pillai, wayne.lin, alexander.deucher, Harry.Wentland,
	nicholas.kazlauskas

[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@amd.com>
Acked-by: Christian König <christian.koenig@amd.com>
---
 .../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;
 	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;
-- 
2.30.0

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH v6 2/3] drm/amd/display: Add freesync video modes based on preferred modes
  2021-02-13  1:08   ` Aurabindo Pillai
  (?)
@ 2021-02-13  6:10     ` kernel test robot
  -1 siblings, 0 replies; 13+ messages in thread
From: kernel test robot @ 2021-02-13  6:10 UTC (permalink / raw)
  To: Aurabindo Pillai, amd-gfx, dri-devel
  Cc: stylon.wang, kbuild-all, shashank.sharma, nicholas.kazlauskas,
	aurabindo.pillai, wayne.lin, alexander.deucher,
	Christian König

[-- Attachment #1: Type: text/plain, Size: 4789 bytes --]

Hi Aurabindo,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on next-20210211]
[also build test ERROR on v5.11-rc7]
[cannot apply to drm-intel/for-linux-next drm-tip/drm-tip tegra-drm/drm/tegra/for-next linus/master drm-exynos/exynos-drm-next drm/drm-next v5.11-rc7 v5.11-rc6 v5.11-rc5]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Aurabindo-Pillai/Freesync-video-mode-optimization/20210213-091059
base:    671176b0016c80b3943cb5387312c886aba3308d
config: ia64-allmodconfig (attached as .config)
compiler: ia64-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/1f1eda26f52f0550c11969b7df546591683bda0f
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Aurabindo-Pillai/Freesync-video-mode-optimization/20210213-091059
        git checkout 1f1eda26f52f0550c11969b7df546591683bda0f
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=ia64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

Note: the linux-review/Aurabindo-Pillai/Freesync-video-mode-optimization/20210213-091059 HEAD 93e6ddbee724ff17e78a905f919aae4aa280c67b builds fine.
      It only hurts bisectibility.

All errors (new ones prefixed by >>):

   drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c: In function 'amdgpu_dm_connector_add_freesync_modes':
>> drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c:7066:8: error: 'amdgpu_exp_freesync_vid_mode' undeclared (first use in this function); did you mean 'amdgpu_freesync_vid_mode'?
    7066 |  if (!(amdgpu_exp_freesync_vid_mode && edid))
         |        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
         |        amdgpu_freesync_vid_mode
   drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c:7066:8: note: each undeclared identifier is reported only once for each function it appears in
   drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c:7062:38: warning: unused variable 'range' [-Wunused-variable]
    7062 |  struct detailed_data_monitor_range *range;
         |                                      ^~~~~
   drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c:7061:29: warning: unused variable 'data' [-Wunused-variable]
    7061 |  struct detailed_non_pixel *data;
         |                             ^~~~
   drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c:7060:26: warning: unused variable 'timing' [-Wunused-variable]
    7060 |  struct detailed_timing *timing;
         |                          ^~~~~~
   drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c:7059:10: warning: unused variable 'i' [-Wunused-variable]
    7059 |  uint8_t i;
         |          ^
   drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c: In function 'amdgpu_dm_commit_cursors':
   drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c:7805:44: warning: variable 'new_plane_state' set but not used [-Wunused-but-set-variable]
    7805 |  struct drm_plane_state *old_plane_state, *new_plane_state;
         |                                            ^~~~~~~~~~~~~~~

Kconfig warnings: (for reference only)
   WARNING: unmet direct dependencies detected for FRAME_POINTER
   Depends on DEBUG_KERNEL && (M68K || UML || SUPERH) || ARCH_WANT_FRAME_POINTERS
   Selected by
   - FAULT_INJECTION_STACKTRACE_FILTER && FAULT_INJECTION_DEBUG_FS && STACKTRACE_SUPPORT && !X86_64 && !MIPS && !PPC && !S390 && !MICROBLAZE && !ARM && !ARC && !X86


vim +7066 drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c

  7055	
  7056	static void amdgpu_dm_connector_add_freesync_modes(struct drm_connector *connector,
  7057							   struct edid *edid)
  7058	{
  7059		uint8_t i;
  7060		struct detailed_timing *timing;
  7061		struct detailed_non_pixel *data;
  7062		struct detailed_data_monitor_range *range;
  7063		struct amdgpu_dm_connector *amdgpu_dm_connector =
  7064			to_amdgpu_dm_connector(connector);
  7065	
> 7066		if (!(amdgpu_exp_freesync_vid_mode && edid))
  7067			return;
  7068		
  7069		if (amdgpu_dm_connector->max_vfreq - amdgpu_dm_connector->min_vfreq > 10)
  7070			amdgpu_dm_connector->num_modes +=
  7071				add_fs_modes(amdgpu_dm_connector);
  7072	}
  7073	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 63769 bytes --]

[-- Attachment #3: Type: text/plain, Size: 160 bytes --]

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v6 2/3] drm/amd/display: Add freesync video modes based on preferred modes
@ 2021-02-13  6:10     ` kernel test robot
  0 siblings, 0 replies; 13+ messages in thread
From: kernel test robot @ 2021-02-13  6:10 UTC (permalink / raw)
  To: Aurabindo Pillai, amd-gfx, dri-devel
  Cc: stylon.wang, kbuild-all, shashank.sharma, nicholas.kazlauskas,
	aurabindo.pillai, wayne.lin, alexander.deucher,
	Christian König

[-- Attachment #1: Type: text/plain, Size: 4789 bytes --]

Hi Aurabindo,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on next-20210211]
[also build test ERROR on v5.11-rc7]
[cannot apply to drm-intel/for-linux-next drm-tip/drm-tip tegra-drm/drm/tegra/for-next linus/master drm-exynos/exynos-drm-next drm/drm-next v5.11-rc7 v5.11-rc6 v5.11-rc5]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Aurabindo-Pillai/Freesync-video-mode-optimization/20210213-091059
base:    671176b0016c80b3943cb5387312c886aba3308d
config: ia64-allmodconfig (attached as .config)
compiler: ia64-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/1f1eda26f52f0550c11969b7df546591683bda0f
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Aurabindo-Pillai/Freesync-video-mode-optimization/20210213-091059
        git checkout 1f1eda26f52f0550c11969b7df546591683bda0f
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=ia64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

Note: the linux-review/Aurabindo-Pillai/Freesync-video-mode-optimization/20210213-091059 HEAD 93e6ddbee724ff17e78a905f919aae4aa280c67b builds fine.
      It only hurts bisectibility.

All errors (new ones prefixed by >>):

   drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c: In function 'amdgpu_dm_connector_add_freesync_modes':
>> drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c:7066:8: error: 'amdgpu_exp_freesync_vid_mode' undeclared (first use in this function); did you mean 'amdgpu_freesync_vid_mode'?
    7066 |  if (!(amdgpu_exp_freesync_vid_mode && edid))
         |        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
         |        amdgpu_freesync_vid_mode
   drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c:7066:8: note: each undeclared identifier is reported only once for each function it appears in
   drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c:7062:38: warning: unused variable 'range' [-Wunused-variable]
    7062 |  struct detailed_data_monitor_range *range;
         |                                      ^~~~~
   drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c:7061:29: warning: unused variable 'data' [-Wunused-variable]
    7061 |  struct detailed_non_pixel *data;
         |                             ^~~~
   drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c:7060:26: warning: unused variable 'timing' [-Wunused-variable]
    7060 |  struct detailed_timing *timing;
         |                          ^~~~~~
   drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c:7059:10: warning: unused variable 'i' [-Wunused-variable]
    7059 |  uint8_t i;
         |          ^
   drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c: In function 'amdgpu_dm_commit_cursors':
   drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c:7805:44: warning: variable 'new_plane_state' set but not used [-Wunused-but-set-variable]
    7805 |  struct drm_plane_state *old_plane_state, *new_plane_state;
         |                                            ^~~~~~~~~~~~~~~

Kconfig warnings: (for reference only)
   WARNING: unmet direct dependencies detected for FRAME_POINTER
   Depends on DEBUG_KERNEL && (M68K || UML || SUPERH) || ARCH_WANT_FRAME_POINTERS
   Selected by
   - FAULT_INJECTION_STACKTRACE_FILTER && FAULT_INJECTION_DEBUG_FS && STACKTRACE_SUPPORT && !X86_64 && !MIPS && !PPC && !S390 && !MICROBLAZE && !ARM && !ARC && !X86


vim +7066 drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c

  7055	
  7056	static void amdgpu_dm_connector_add_freesync_modes(struct drm_connector *connector,
  7057							   struct edid *edid)
  7058	{
  7059		uint8_t i;
  7060		struct detailed_timing *timing;
  7061		struct detailed_non_pixel *data;
  7062		struct detailed_data_monitor_range *range;
  7063		struct amdgpu_dm_connector *amdgpu_dm_connector =
  7064			to_amdgpu_dm_connector(connector);
  7065	
> 7066		if (!(amdgpu_exp_freesync_vid_mode && edid))
  7067			return;
  7068		
  7069		if (amdgpu_dm_connector->max_vfreq - amdgpu_dm_connector->min_vfreq > 10)
  7070			amdgpu_dm_connector->num_modes +=
  7071				add_fs_modes(amdgpu_dm_connector);
  7072	}
  7073	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 63769 bytes --]

[-- Attachment #3: Type: text/plain, Size: 154 bytes --]

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v6 2/3] drm/amd/display: Add freesync video modes based on preferred modes
@ 2021-02-13  6:10     ` kernel test robot
  0 siblings, 0 replies; 13+ messages in thread
From: kernel test robot @ 2021-02-13  6:10 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 4879 bytes --]

Hi Aurabindo,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on next-20210211]
[also build test ERROR on v5.11-rc7]
[cannot apply to drm-intel/for-linux-next drm-tip/drm-tip tegra-drm/drm/tegra/for-next linus/master drm-exynos/exynos-drm-next drm/drm-next v5.11-rc7 v5.11-rc6 v5.11-rc5]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Aurabindo-Pillai/Freesync-video-mode-optimization/20210213-091059
base:    671176b0016c80b3943cb5387312c886aba3308d
config: ia64-allmodconfig (attached as .config)
compiler: ia64-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/1f1eda26f52f0550c11969b7df546591683bda0f
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Aurabindo-Pillai/Freesync-video-mode-optimization/20210213-091059
        git checkout 1f1eda26f52f0550c11969b7df546591683bda0f
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=ia64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

Note: the linux-review/Aurabindo-Pillai/Freesync-video-mode-optimization/20210213-091059 HEAD 93e6ddbee724ff17e78a905f919aae4aa280c67b builds fine.
      It only hurts bisectibility.

All errors (new ones prefixed by >>):

   drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c: In function 'amdgpu_dm_connector_add_freesync_modes':
>> drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c:7066:8: error: 'amdgpu_exp_freesync_vid_mode' undeclared (first use in this function); did you mean 'amdgpu_freesync_vid_mode'?
    7066 |  if (!(amdgpu_exp_freesync_vid_mode && edid))
         |        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
         |        amdgpu_freesync_vid_mode
   drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c:7066:8: note: each undeclared identifier is reported only once for each function it appears in
   drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c:7062:38: warning: unused variable 'range' [-Wunused-variable]
    7062 |  struct detailed_data_monitor_range *range;
         |                                      ^~~~~
   drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c:7061:29: warning: unused variable 'data' [-Wunused-variable]
    7061 |  struct detailed_non_pixel *data;
         |                             ^~~~
   drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c:7060:26: warning: unused variable 'timing' [-Wunused-variable]
    7060 |  struct detailed_timing *timing;
         |                          ^~~~~~
   drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c:7059:10: warning: unused variable 'i' [-Wunused-variable]
    7059 |  uint8_t i;
         |          ^
   drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c: In function 'amdgpu_dm_commit_cursors':
   drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c:7805:44: warning: variable 'new_plane_state' set but not used [-Wunused-but-set-variable]
    7805 |  struct drm_plane_state *old_plane_state, *new_plane_state;
         |                                            ^~~~~~~~~~~~~~~

Kconfig warnings: (for reference only)
   WARNING: unmet direct dependencies detected for FRAME_POINTER
   Depends on DEBUG_KERNEL && (M68K || UML || SUPERH) || ARCH_WANT_FRAME_POINTERS
   Selected by
   - FAULT_INJECTION_STACKTRACE_FILTER && FAULT_INJECTION_DEBUG_FS && STACKTRACE_SUPPORT && !X86_64 && !MIPS && !PPC && !S390 && !MICROBLAZE && !ARM && !ARC && !X86


vim +7066 drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c

  7055	
  7056	static void amdgpu_dm_connector_add_freesync_modes(struct drm_connector *connector,
  7057							   struct edid *edid)
  7058	{
  7059		uint8_t i;
  7060		struct detailed_timing *timing;
  7061		struct detailed_non_pixel *data;
  7062		struct detailed_data_monitor_range *range;
  7063		struct amdgpu_dm_connector *amdgpu_dm_connector =
  7064			to_amdgpu_dm_connector(connector);
  7065	
> 7066		if (!(amdgpu_exp_freesync_vid_mode && edid))
  7067			return;
  7068		
  7069		if (amdgpu_dm_connector->max_vfreq - amdgpu_dm_connector->min_vfreq > 10)
  7070			amdgpu_dm_connector->num_modes +=
  7071				add_fs_modes(amdgpu_dm_connector);
  7072	}
  7073	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 63769 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v6 3/3] drm/amd/display: Skip modeset for front porch change
  2021-02-13  1:08   ` Aurabindo Pillai
@ 2021-02-25 17:42     ` Kazlauskas, Nicholas
  -1 siblings, 0 replies; 13+ messages in thread
From: Kazlauskas, Nicholas @ 2021-02-25 17:42 UTC (permalink / raw)
  To: Aurabindo Pillai, amd-gfx, dri-devel
  Cc: stylon.wang, alexander.deucher, wayne.lin, Christian König,
	shashank.sharma

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@amd.com>
> Acked-by: Christian König <christian.koenig@amd.com>
> ---
>   .../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@amd.com>

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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v6 3/3] drm/amd/display: Skip modeset for front porch change
@ 2021-02-25 17:42     ` Kazlauskas, Nicholas
  0 siblings, 0 replies; 13+ messages in thread
From: Kazlauskas, Nicholas @ 2021-02-25 17:42 UTC (permalink / raw)
  To: Aurabindo Pillai, amd-gfx, dri-devel
  Cc: stylon.wang, alexander.deucher, wayne.lin, Christian König,
	shashank.sharma

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@amd.com>
> Acked-by: Christian König <christian.koenig@amd.com>
> ---
>   .../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@amd.com>

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;
> 

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2021-02-25 17:42 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-13  1:08 [PATCH v6 0/3] Freesync video mode optimization Aurabindo Pillai
2021-02-13  1:08 ` Aurabindo Pillai
2021-02-13  1:08 ` [PATCH v6 1/3] drm/amd/display: Add module parameter for freesync video mode Aurabindo Pillai
2021-02-13  1:08   ` Aurabindo Pillai
2021-02-13  1:08 ` [PATCH v6 2/3] drm/amd/display: Add freesync video modes based on preferred modes Aurabindo Pillai
2021-02-13  1:08   ` Aurabindo Pillai
2021-02-13  6:10   ` kernel test robot
2021-02-13  6:10     ` kernel test robot
2021-02-13  6:10     ` kernel test robot
2021-02-13  1:08 ` [PATCH v6 3/3] drm/amd/display: Skip modeset for front porch change Aurabindo Pillai
2021-02-13  1:08   ` Aurabindo Pillai
2021-02-25 17:42   ` Kazlauskas, Nicholas
2021-02-25 17:42     ` Kazlauskas, Nicholas

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.