[PATCH] drm/amd/display: Enable PSR by default on DCN3.1

[PATCH] drm/amd/display: Enable PSR by default on DCN3.1

[Nicholas Kazlauskas]

[Why]
New idle optimizations for DCN3.1 require PSR for optimal power savings
on panels that support it.

This was previously left disabled by default because of issues with
compositors that do not pageflip and scan out directly to the
frontbuffer.

For these compositors we now have detection methods that wait for x
number of pageflips after a full update - triggered by a buffer or
format change typically.

This may introduce bugs or new cases not tested by users so this is
only currently targeting DCN31.

[How]
Add code in DM to set PSR state by default for DCN3.1 while falling
back to the feature mask for older DCN.

Add a global debug flag that can be set to disable it for either.

Cc: Harry Wentland <harry.wentland@amd.com>
Cc: Roman Li <roman.li@amd.com>
Signed-off-by: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>
---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c   | 17 ++++++++++++++++-
 drivers/gpu/drm/amd/include/amd_shared.h        |  5 +++--
 2 files changed, 19 insertions(+), 3 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 dc595ecec595..ff545503a6ed 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -4031,6 +4031,7 @@ static int amdgpu_dm_initialize_drm_device(struct amdgpu_device *adev)
 	int32_t primary_planes;
 	enum dc_connection_type new_connection_type = dc_connection_none;
 	const struct dc_plane_cap *plane;
+	bool psr_feature_enabled = false;
 
 	dm->display_indexes_num = dm->dc->caps.max_streams;
 	/* Update the actual used number of crtc */
@@ -4113,6 +4114,19 @@ static int amdgpu_dm_initialize_drm_device(struct amdgpu_device *adev)
 		DRM_DEBUG_KMS("Unsupported DCN IP version for outbox: 0x%X\n",
 			      adev->ip_versions[DCE_HWIP][0]);
 	}
+
+	/* Determine whether to enable PSR support by default. */
+	if (!(amdgpu_dc_debug_mask & DC_DISABLE_PSR)) {
+		switch (adev->ip_versions[DCE_HWIP][0]) {
+		case IP_VERSION(3, 1, 2):
+		case IP_VERSION(3, 1, 3):
+			psr_feature_enabled = true;
+			break;
+		default:
+			psr_feature_enabled = amdgpu_dc_feature_mask & DC_PSR_MASK;
+			break;
+		}
+	}
 #endif
 
 	/* loops over all connectors on the board */
@@ -4156,7 +4170,8 @@ static int amdgpu_dm_initialize_drm_device(struct amdgpu_device *adev)
 		} else if (dc_link_detect(link, DETECT_REASON_BOOT)) {
 			amdgpu_dm_update_connector_after_detect(aconnector);
 			register_backlight_device(dm, link);
-			if (amdgpu_dc_feature_mask & DC_PSR_MASK)
+
+			if (psr_feature_enabled)
 				amdgpu_dm_set_psr_caps(link);
 		}
 
diff --git a/drivers/gpu/drm/amd/include/amd_shared.h b/drivers/gpu/drm/amd/include/amd_shared.h
index 257f280d3d53..f1a46d16f7ea 100644
--- a/drivers/gpu/drm/amd/include/amd_shared.h
+++ b/drivers/gpu/drm/amd/include/amd_shared.h
@@ -228,7 +228,7 @@ enum DC_FEATURE_MASK {
 	DC_FBC_MASK = (1 << 0), //0x1, disabled by default
 	DC_MULTI_MON_PP_MCLK_SWITCH_MASK = (1 << 1), //0x2, enabled by default
 	DC_DISABLE_FRACTIONAL_PWM_MASK = (1 << 2), //0x4, disabled by default
-	DC_PSR_MASK = (1 << 3), //0x8, disabled by default
+	DC_PSR_MASK = (1 << 3), //0x8, disabled by default for dcn < 3.1
 	DC_EDP_NO_POWER_SEQUENCING = (1 << 4), //0x10, disabled by default
 };
 
@@ -236,7 +236,8 @@ enum DC_DEBUG_MASK {
 	DC_DISABLE_PIPE_SPLIT = 0x1,
 	DC_DISABLE_STUTTER = 0x2,
 	DC_DISABLE_DSC = 0x4,
-	DC_DISABLE_CLOCK_GATING = 0x8
+	DC_DISABLE_CLOCK_GATING = 0x8,
+	DC_DISABLE_PSR = 0x10,
 };
 
 enum amd_dpm_forced_level;
-- 
2.25.1

Re: [PATCH] drm/amd/display: Enable PSR by default on DCN3.1

[Vishwakarma, Pratik]

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


On 10/8/2021 9:44 PM, Nicholas Kazlauskas wrote:
> [Why]
> New idle optimizations for DCN3.1 require PSR for optimal power savings
> on panels that support it.
>
> This was previously left disabled by default because of issues with
> compositors that do not pageflip and scan out directly to the
> frontbuffer.
>
> For these compositors we now have detection methods that wait for x
> number of pageflips after a full update - triggered by a buffer or
> format change typically.
>
> This may introduce bugs or new cases not tested by users so this is
> only currently targeting DCN31.
>
> [How]
> Add code in DM to set PSR state by default for DCN3.1 while falling
> back to the feature mask for older DCN.
>
> Add a global debug flag that can be set to disable it for either.
>
> Cc: Harry Wentland<harry.wentland@amd.com>
> Cc: Roman Li<roman.li@amd.com>
> Signed-off-by: Nicholas Kazlauskas<nicholas.kazlauskas@amd.com>
> ---
>   .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c   | 17 ++++++++++++++++-
>   drivers/gpu/drm/amd/include/amd_shared.h        |  5 +++--
>   2 files changed, 19 insertions(+), 3 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 dc595ecec595..ff545503a6ed 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -4031,6 +4031,7 @@ static int amdgpu_dm_initialize_drm_device(struct amdgpu_device *adev)
>   	int32_t primary_planes;
>   	enum dc_connection_type new_connection_type = dc_connection_none;
>   	const struct dc_plane_cap *plane;
> +	bool psr_feature_enabled = false;
>   
>   	dm->display_indexes_num = dm->dc->caps.max_streams;
>   	/* Update the actual used number of crtc */
> @@ -4113,6 +4114,19 @@ static int amdgpu_dm_initialize_drm_device(struct amdgpu_device *adev)
>   		DRM_DEBUG_KMS("Unsupported DCN IP version for outbox: 0x%X\n",
>   			      adev->ip_versions[DCE_HWIP][0]);
>   	}
> +
> +	/* Determine whether to enable PSR support by default. */
> +	if (!(amdgpu_dc_debug_mask & DC_DISABLE_PSR)) {
> +		switch (adev->ip_versions[DCE_HWIP][0]) {
> +		case IP_VERSION(3, 1, 2):
> +		case IP_VERSION(3, 1, 3):
> +			psr_feature_enabled = true;
> +			break;
> +		default:
> +			psr_feature_enabled = amdgpu_dc_feature_mask & DC_PSR_MASK;
> +			break;
> +		}
> +	}
>   #endif
>   
>   	/* loops over all connectors on the board */
> @@ -4156,7 +4170,8 @@ static int amdgpu_dm_initialize_drm_device(struct amdgpu_device *adev)
>   		} else if (dc_link_detect(link, DETECT_REASON_BOOT)) {
>   			amdgpu_dm_update_connector_after_detect(aconnector);
>   			register_backlight_device(dm, link);
> -			if (amdgpu_dc_feature_mask & DC_PSR_MASK)
> +
> +			if (psr_feature_enabled)
>   				amdgpu_dm_set_psr_caps(link);
>   		}
>   
> diff --git a/drivers/gpu/drm/amd/include/amd_shared.h b/drivers/gpu/drm/amd/include/amd_shared.h
> index 257f280d3d53..f1a46d16f7ea 100644
> --- a/drivers/gpu/drm/amd/include/amd_shared.h
> +++ b/drivers/gpu/drm/amd/include/amd_shared.h
> @@ -228,7 +228,7 @@ enum DC_FEATURE_MASK {
>   	DC_FBC_MASK = (1 << 0), //0x1, disabled by default
>   	DC_MULTI_MON_PP_MCLK_SWITCH_MASK = (1 << 1), //0x2, enabled by default
>   	DC_DISABLE_FRACTIONAL_PWM_MASK = (1 << 2), //0x4, disabled by default
> -	DC_PSR_MASK = (1 << 3), //0x8, disabled by default
> +	DC_PSR_MASK = (1 << 3), //0x8, disabled by default for dcn < 3.1
>   	DC_EDP_NO_POWER_SEQUENCING = (1 << 4), //0x10, disabled by default
>   };
>   
> @@ -236,7 +236,8 @@ enum DC_DEBUG_MASK {
>   	DC_DISABLE_PIPE_SPLIT = 0x1,
>   	DC_DISABLE_STUTTER = 0x2,
>   	DC_DISABLE_DSC = 0x4,
> -	DC_DISABLE_CLOCK_GATING = 0x8
> +	DC_DISABLE_CLOCK_GATING = 0x8,
> +	DC_DISABLE_PSR = 0x10,

Don't we need a corresponding check in amdgpu_dm_init() to disable PSR 
in runtime?

Also, how does it handle conflicting declarations from feature mask and 
debug mask?

/BR
/

/Pratik
/

>   };
>   
>   enum amd_dpm_forced_level;

[-- Attachment #2: Type: text/html, Size: 4698 bytes --]

Re: [PATCH] drm/amd/display: Enable PSR by default on DCN3.1

[Kazlauskas, Nicholas]

On 2021-10-11 1:04 a.m., Vishwakarma, Pratik wrote:
> 
> On 10/8/2021 9:44 PM, Nicholas Kazlauskas wrote:
>> [Why]
>> New idle optimizations for DCN3.1 require PSR for optimal power savings
>> on panels that support it.
>>
>> This was previously left disabled by default because of issues with
>> compositors that do not pageflip and scan out directly to the
>> frontbuffer.
>>
>> For these compositors we now have detection methods that wait for x
>> number of pageflips after a full update - triggered by a buffer or
>> format change typically.
>>
>> This may introduce bugs or new cases not tested by users so this is
>> only currently targeting DCN31.
>>
>> [How]
>> Add code in DM to set PSR state by default for DCN3.1 while falling
>> back to the feature mask for older DCN.
>>
>> Add a global debug flag that can be set to disable it for either.
>>
>> Cc: Harry Wentland<harry.wentland@amd.com>
>> Cc: Roman Li<roman.li@amd.com>
>> Signed-off-by: Nicholas Kazlauskas<nicholas.kazlauskas@amd.com>
>> ---
>>   .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c   | 17 ++++++++++++++++-
>>   drivers/gpu/drm/amd/include/amd_shared.h        |  5 +++--
>>   2 files changed, 19 insertions(+), 3 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 dc595ecec595..ff545503a6ed 100644
>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> @@ -4031,6 +4031,7 @@ static int amdgpu_dm_initialize_drm_device(struct amdgpu_device *adev)
>>   	int32_t primary_planes;
>>   	enum dc_connection_type new_connection_type = dc_connection_none;
>>   	const struct dc_plane_cap *plane;
>> +	bool psr_feature_enabled = false;
>>   
>>   	dm->display_indexes_num = dm->dc->caps.max_streams;
>>   	/* Update the actual used number of crtc */
>> @@ -4113,6 +4114,19 @@ static int amdgpu_dm_initialize_drm_device(struct amdgpu_device *adev)
>>   		DRM_DEBUG_KMS("Unsupported DCN IP version for outbox: 0x%X\n",
>>   			      adev->ip_versions[DCE_HWIP][0]);
>>   	}
>> +
>> +	/* Determine whether to enable PSR support by default. */
>> +	if (!(amdgpu_dc_debug_mask & DC_DISABLE_PSR)) {
>> +		switch (adev->ip_versions[DCE_HWIP][0]) {
>> +		case IP_VERSION(3, 1, 2):
>> +		case IP_VERSION(3, 1, 3):
>> +			psr_feature_enabled = true;
>> +			break;
>> +		default:
>> +			psr_feature_enabled = amdgpu_dc_feature_mask & DC_PSR_MASK;
>> +			break;
>> +		}
>> +	}
>>   #endif
>>   
>>   	/* loops over all connectors on the board */
>> @@ -4156,7 +4170,8 @@ static int amdgpu_dm_initialize_drm_device(struct amdgpu_device *adev)
>>   		} else if (dc_link_detect(link, DETECT_REASON_BOOT)) {
>>   			amdgpu_dm_update_connector_after_detect(aconnector);
>>   			register_backlight_device(dm, link);
>> -			if (amdgpu_dc_feature_mask & DC_PSR_MASK)
>> +
>> +			if (psr_feature_enabled)
>>   				amdgpu_dm_set_psr_caps(link);
>>   		}
>>   
>> diff --git a/drivers/gpu/drm/amd/include/amd_shared.h b/drivers/gpu/drm/amd/include/amd_shared.h
>> index 257f280d3d53..f1a46d16f7ea 100644
>> --- a/drivers/gpu/drm/amd/include/amd_shared.h
>> +++ b/drivers/gpu/drm/amd/include/amd_shared.h
>> @@ -228,7 +228,7 @@ enum DC_FEATURE_MASK {
>>   	DC_FBC_MASK = (1 << 0), //0x1, disabled by default
>>   	DC_MULTI_MON_PP_MCLK_SWITCH_MASK = (1 << 1), //0x2, enabled by default
>>   	DC_DISABLE_FRACTIONAL_PWM_MASK = (1 << 2), //0x4, disabled by default
>> -	DC_PSR_MASK = (1 << 3), //0x8, disabled by default
>> +	DC_PSR_MASK = (1 << 3), //0x8, disabled by default for dcn < 3.1
>>   	DC_EDP_NO_POWER_SEQUENCING = (1 << 4), //0x10, disabled by default
>>   };
>>   
>> @@ -236,7 +236,8 @@ enum DC_DEBUG_MASK {
>>   	DC_DISABLE_PIPE_SPLIT = 0x1,
>>   	DC_DISABLE_STUTTER = 0x2,
>>   	DC_DISABLE_DSC = 0x4,
>> -	DC_DISABLE_CLOCK_GATING = 0x8
>> +	DC_DISABLE_CLOCK_GATING = 0x8,
>> +	DC_DISABLE_PSR = 0x10,
> 
> Don't we need a corresponding check in amdgpu_dm_init() to disable PSR 
> in runtime?

The check is `if (psr_feature_enabled)` above.
> 
> Also, how does it handle conflicting declarations from feature mask and 
> debug mask?

Feature enable mask is used for older ASIC to allow PSR to be enabled.

For both old and new ASIC the DISABLE mask takes priority as a debug 
option for disabling PSR support.

Regards,
Nicholas Kazlauskas

> 
> /BR
> /
> 
> /Pratik
> /
> 
>>   };
>>   
>>   enum amd_dpm_forced_level;

Re: [PATCH] drm/amd/display: Enable PSR by default on DCN3.1

[Harry Wentland]

On 2021-10-08 12:14, Nicholas Kazlauskas wrote:
> [Why]
> New idle optimizations for DCN3.1 require PSR for optimal power savings
> on panels that support it.
> 
> This was previously left disabled by default because of issues with
> compositors that do not pageflip and scan out directly to the
> frontbuffer.
> 
> For these compositors we now have detection methods that wait for x
> number of pageflips after a full update - triggered by a buffer or
> format change typically.
> 
> This may introduce bugs or new cases not tested by users so this is
> only currently targeting DCN31.
> 
> [How]
> Add code in DM to set PSR state by default for DCN3.1 while falling
> back to the feature mask for older DCN.
> 
> Add a global debug flag that can be set to disable it for either.
> 
> Cc: Harry Wentland <harry.wentland@amd.com>
> Cc: Roman Li <roman.li@amd.com>
> Signed-off-by: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>

Reviewed-by: Harry Wentland <harry.wentland@amd.com>

Harry

> ---
>  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c   | 17 ++++++++++++++++-
>  drivers/gpu/drm/amd/include/amd_shared.h        |  5 +++--
>  2 files changed, 19 insertions(+), 3 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 dc595ecec595..ff545503a6ed 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -4031,6 +4031,7 @@ static int amdgpu_dm_initialize_drm_device(struct amdgpu_device *adev)
>  	int32_t primary_planes;
>  	enum dc_connection_type new_connection_type = dc_connection_none;
>  	const struct dc_plane_cap *plane;
> +	bool psr_feature_enabled = false;
>  
>  	dm->display_indexes_num = dm->dc->caps.max_streams;
>  	/* Update the actual used number of crtc */
> @@ -4113,6 +4114,19 @@ static int amdgpu_dm_initialize_drm_device(struct amdgpu_device *adev)
>  		DRM_DEBUG_KMS("Unsupported DCN IP version for outbox: 0x%X\n",
>  			      adev->ip_versions[DCE_HWIP][0]);
>  	}
> +
> +	/* Determine whether to enable PSR support by default. */
> +	if (!(amdgpu_dc_debug_mask & DC_DISABLE_PSR)) {
> +		switch (adev->ip_versions[DCE_HWIP][0]) {
> +		case IP_VERSION(3, 1, 2):
> +		case IP_VERSION(3, 1, 3):
> +			psr_feature_enabled = true;
> +			break;
> +		default:
> +			psr_feature_enabled = amdgpu_dc_feature_mask & DC_PSR_MASK;
> +			break;
> +		}
> +	}
>  #endif
>  
>  	/* loops over all connectors on the board */
> @@ -4156,7 +4170,8 @@ static int amdgpu_dm_initialize_drm_device(struct amdgpu_device *adev)
>  		} else if (dc_link_detect(link, DETECT_REASON_BOOT)) {
>  			amdgpu_dm_update_connector_after_detect(aconnector);
>  			register_backlight_device(dm, link);
> -			if (amdgpu_dc_feature_mask & DC_PSR_MASK)
> +
> +			if (psr_feature_enabled)
>  				amdgpu_dm_set_psr_caps(link);
>  		}
>  
> diff --git a/drivers/gpu/drm/amd/include/amd_shared.h b/drivers/gpu/drm/amd/include/amd_shared.h
> index 257f280d3d53..f1a46d16f7ea 100644
> --- a/drivers/gpu/drm/amd/include/amd_shared.h
> +++ b/drivers/gpu/drm/amd/include/amd_shared.h
> @@ -228,7 +228,7 @@ enum DC_FEATURE_MASK {
>  	DC_FBC_MASK = (1 << 0), //0x1, disabled by default
>  	DC_MULTI_MON_PP_MCLK_SWITCH_MASK = (1 << 1), //0x2, enabled by default
>  	DC_DISABLE_FRACTIONAL_PWM_MASK = (1 << 2), //0x4, disabled by default
> -	DC_PSR_MASK = (1 << 3), //0x8, disabled by default
> +	DC_PSR_MASK = (1 << 3), //0x8, disabled by default for dcn < 3.1
>  	DC_EDP_NO_POWER_SEQUENCING = (1 << 4), //0x10, disabled by default
>  };
>  
> @@ -236,7 +236,8 @@ enum DC_DEBUG_MASK {
>  	DC_DISABLE_PIPE_SPLIT = 0x1,
>  	DC_DISABLE_STUTTER = 0x2,
>  	DC_DISABLE_DSC = 0x4,
> -	DC_DISABLE_CLOCK_GATING = 0x8
> +	DC_DISABLE_CLOCK_GATING = 0x8,
> +	DC_DISABLE_PSR = 0x10,
>  };
>  
>  enum amd_dpm_forced_level;
>