Re: [PATCH] drm/amd/display: disable psr whenever applicable

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

 





On 2022-10-03 11:26, S, Shirish wrote:
Ping!

Regards,

Shirish S

On 9/30/2022 7:17 PM, S, Shirish wrote:


On 9/30/2022 6:59 PM, Harry Wentland wrote:
+Leo

On 9/30/22 06:27, Shirish S wrote:
[Why]
psr feature continues to be enabled for non capable links.

Do you have more info on what issues you're seeing with this?

Code wise without this change we end up setting "vblank_disable_immediate" parameter to false for the failing links also.

Issue wise there is a remote chance of this leading to eDP/connected monitor not lighting up.

I'm surprised psr_settings.psr_feature_enabled can be 'true' before
amdgpu_dm_set_psr_caps() runs. it should default to 'false', and it's
set early on during amdgpu_dm_initialize_drm_device() before any other
psr-related code runs.

In other words, I don't expect psr_settings.psr_feature_enabled to be
'true' on early return of dm_set_psr_caps().

What are the sequence of events that causes an issue for you?



[How]
disable the feature on links that are not capable of the same.

Signed-off-by: Shirish S<shirish.s@xxxxxxx>
---
  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_psr.c | 10 ++++++++--
  1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_psr.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_psr.c
index 8ca10ab3dfc1..f73af028f312 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_psr.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_psr.c
@@ -60,11 +60,17 @@ static bool link_supports_psrsu(struct dc_link *link)
   */
  void amdgpu_dm_set_psr_caps(struct dc_link *link)
  {
-	if (!(link->connector_signal & SIGNAL_TYPE_EDP))
+	if (!(link->connector_signal & SIGNAL_TYPE_EDP)) {
+		DRM_ERROR("Disabling PSR as connector is not eDP\n")
I don't think we should log an error here.

My objective of logging an error was to inform user/developer that this boot PSR enablement had issues.

It's not really an issue, PSR simply cannot be enabled on non-eDP or
disconnected links. However, it is concerning if we enter this function
with psr_feature_enabled == true.

Thanks,
Leo


Am fine with moving it to INFO or remove it, if you insist.

Thanks for your comments.

Regards,

Shirish S

+		link->psr_settings.psr_feature_enabled = false;
  		return;
+	}
- if (link->type == dc_connection_none)
+	if (link->type == dc_connection_none) {
+		DRM_ERROR("Disabling PSR as eDP connection type is invalid\n")
Same here, this doesn't warrant an error log.

Harry

+		link->psr_settings.psr_feature_enabled = false;
  		return;
+	}
if (link->dpcd_caps.psr_info.psr_version == 0) {
  		link->psr_settings.psr_version = DC_PSR_VERSION_UNSUPPORTED;



[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux