<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=us-ascii">
<style type="text/css" style="display:none;"> P {margin-top:0;margin-bottom:0;} </style>
</head>
<body dir="ltr">
<p style="font-family:Calibri;font-size:10pt;color:#0000FF;margin:5pt;font-style:normal;font-weight:normal;text-decoration:none;" align="Left">
[AMD Official Use Only - AMD Internal Distribution Only]<br>
</p>
<br>
<div>
<div class="elementToProof" style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
Hi John,</div>
<div class="elementToProof" style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
<br>
</div>
<div class="elementToProof" style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
Sure, the debug print is not really necessary. We'll update it before merging.</div>
<div class="elementToProof" style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
<br>
</div>
<div id="Signature" class="elementToProof" style="color: inherit;">
<div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
--</div>
<div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
<br>
</div>
<div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
Thanks & Regards,</div>
<div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
Aurabindo Pillai </div>
</div>
<div id="appendonsend"></div>
<hr style="display:inline-block;width:98%" tabindex="-1">
<div id="divRplyFwdMsg" dir="ltr"><font face="Calibri, sans-serif" style="font-size:11pt" color="#000000"><b>From:</b> John Olender <john.olender@gmail.com><br>
<b>Sent:</b> Thursday, May 8, 2025 5:51 AM<br>
<b>To:</b> Pillai, Aurabindo <Aurabindo.Pillai@amd.com>; Wu, Ray <Ray.Wu@amd.com>; Li, Sun peng (Leo) <Sunpeng.Li@amd.com><br>
<b>Cc:</b> Wentland, Harry <Harry.Wentland@amd.com>; Li, Roman <Roman.Li@amd.com>; Lin, Wayne <Wayne.Lin@amd.com>; Chung, ChiaHsuan (Tom) <ChiaHsuan.Chung@amd.com>; Zuo, Jerry <Jerry.Zuo@amd.com>; Mohamed, Zaeem <Zaeem.Mohamed@amd.com>; Wheeler, Daniel <Daniel.Wheeler@amd.com>;
Hung, Alex <Alex.Hung@amd.com>; amd-gfx@lists.freedesktop.org <amd-gfx@lists.freedesktop.org><br>
<b>Subject:</b> Re: [PATCH 05/14] drm/amd/display: Defer BW-optimization-blocked DRR adjustments</font>
<div> </div>
</div>
<div class="BodyFragment"><font size="2"><span style="font-size:11pt;">
<div class="PlainText">On 5/6/25 10:34 PM, Ray Wu wrote:<br>
> From: John Olender <john.olender@gmail.com><br>
> <br>
> [Why & How]<br>
> Instead of dropping DRR updates, defer them. This fixes issues where<br>
> monitor continues to see incorrect refresh rate after VRR was turned off<br>
> by userspace.<br>
> <br>
> Fixes: 32953485c558 ("drm/amd/display: Do not update DRR while BW optimizations pending")<br>
> Link: <a href="https://gitlab.freedesktop.org/drm/amd/-/issues/3546">https://gitlab.freedesktop.org/drm/amd/-/issues/3546</a><br>
> <br>
> Reviewed-by: Sun peng Li <sunpeng.li@amd.com><br>
> Signed-off-by: John Olender <john.olender@gmail.com><br>
> Signed-off-by: Aurabindo Pillai <aurabindo.pillai@amd.com><br>
> Signed-off-by: Ray Wu <ray.wu@amd.com><br>
<br>
Thank you for reviewing and revising the original patch. This commit<br>
message is a clear improvement over the original.<br>
<br>
I do have a concern about the debug print clean up though. Please see<br>
below.<br>
<br>
> ---<br>
> drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 2 ++<br>
> drivers/gpu/drm/amd/display/dc/core/dc.c | 13 ++++++++++---<br>
> 2 files changed, 12 insertions(+), 3 deletions(-)<br>
> <br>
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c<br>
> index 36c16030fca9..5a38748703b3 100644<br>
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c<br>
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c<br>
> @@ -374,6 +374,8 @@ get_crtc_by_otg_inst(struct amdgpu_device *adev,<br>
> static inline bool is_dc_timing_adjust_needed(struct dm_crtc_state *old_state,<br>
> struct dm_crtc_state *new_state)<br>
> {<br>
> + if (new_state->stream->adjust.timing_adjust_pending)<br>
> + return true;<br>
> if (new_state->freesync_config.state == VRR_STATE_ACTIVE_FIXED)<br>
> return true;<br>
> else if (amdgpu_dm_crtc_vrr_active(old_state) != amdgpu_dm_crtc_vrr_active(new_state))<br>
> diff --git a/drivers/gpu/drm/amd/display/dc/core/dc.c b/drivers/gpu/drm/amd/display/dc/core/dc.c<br>
> index 528e6fd546c5..6ec22c0d5b97 100644<br>
> --- a/drivers/gpu/drm/amd/display/dc/core/dc.c<br>
> +++ b/drivers/gpu/drm/amd/display/dc/core/dc.c<br>
> @@ -441,9 +441,15 @@ bool dc_stream_adjust_vmin_vmax(struct dc *dc,<br>
> * Don't adjust DRR while there's bandwidth optimizations pending to<br>
> * avoid conflicting with firmware updates.<br>
> */<br>
> - if (dc->ctx->dce_version > DCE_VERSION_MAX)<br>
> - if (dc->optimized_required || dc->wm_optimized_required)<br>
> + if (dc->ctx->dce_version > DCE_VERSION_MAX) {<br>
> + if (dc->optimized_required || dc->wm_optimized_required) {<br>
> + if (!stream->adjust.timing_adjust_pending) {<br>
> + stream->adjust.timing_adjust_pending = true;<br>
> + DC_LOG_DEBUG("%s: deferring DRR update\n", __func__);<br>
> + }<br>
> return false;<br>
> + }<br>
> + }<br>
<br>
Printing the start of a string of blocked updates without also printing<br>
the end can result in misleading debug messages.<br>
<br>
The original patch got around this by spamming out a debug print for<br>
every blocked update. Not ideal, but it let me to keep the focus of the<br>
patch on the fix itself.<br>
<br>
If the spam from the original patch is too much and adding an end<br>
message is too messy, please consider removing the debug message entirely.<br>
<br>
Thanks,<br>
John<br>
<br>
> <br>
> dc_exit_ips_for_hw_access(dc);<br>
> <br>
> @@ -3241,7 +3247,8 @@ static void copy_stream_update_to_stream(struct dc *dc,<br>
> <br>
> if (update->crtc_timing_adjust) {<br>
> if (stream->adjust.v_total_min != update->crtc_timing_adjust->v_total_min ||<br>
> - stream->adjust.v_total_max != update->crtc_timing_adjust->v_total_max)<br>
> + stream->adjust.v_total_max != update->crtc_timing_adjust->v_total_max ||<br>
> + stream->adjust.timing_adjust_pending)<br>
> update->crtc_timing_adjust->timing_adjust_pending = true;<br>
> stream->adjust = *update->crtc_timing_adjust;<br>
> update->crtc_timing_adjust->timing_adjust_pending = false;<br>
<br>
</div>
</span></font></div>
</div>
</body>
</html>