<html xmlns:v="urn:schemas-microsoft-com:vml" xmlns:o="urn:schemas-microsoft-com:office:office" xmlns:w="urn:schemas-microsoft-com:office:word" xmlns:m="http://schemas.microsoft.com/office/2004/12/omml" xmlns="http://www.w3.org/TR/REC-html40">
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
<meta name="Generator" content="Microsoft Word 15 (filtered medium)">
<style><!--
/* Font Definitions */
@font-face
{font-family:"Cambria Math";
panose-1:2 4 5 3 5 4 6 3 2 4;}
@font-face
{font-family:Calibri;
panose-1:2 15 5 2 2 2 4 3 2 4;}
@font-face
{font-family:Aptos;}
@font-face
{font-family:Consolas;
panose-1:2 11 6 9 2 2 4 3 2 4;}
/* Style Definitions */
p.MsoNormal, li.MsoNormal, div.MsoNormal
{margin:0in;
font-size:12.0pt;
font-family:"Aptos",sans-serif;}
a:link, span.MsoHyperlink
{mso-style-priority:99;
color:blue;
text-decoration:underline;}
pre
{mso-style-priority:99;
mso-style-link:"HTML Preformatted Char";
margin:0in;
font-size:10.0pt;
font-family:"Courier New";}
span.HTMLPreformattedChar
{mso-style-name:"HTML Preformatted Char";
mso-style-priority:99;
mso-style-link:"HTML Preformatted";
font-family:Consolas;}
span.EmailStyle21
{mso-style-type:personal-reply;
font-family:"Aptos",sans-serif;
color:windowtext;}
.MsoChpDefault
{mso-style-type:export-only;
font-size:10.0pt;
mso-ligatures:none;}
@page WordSection1
{size:8.5in 11.0in;
margin:1.0in 1.0in 1.0in 1.0in;}
div.WordSection1
{page:WordSection1;}
--></style><!--[if gte mso 9]><xml>
<o:shapedefaults v:ext="edit" spidmax="1026" />
</xml><![endif]--><!--[if gte mso 9]><xml>
<o:shapelayout v:ext="edit">
<o:idmap v:ext="edit" data="1" />
</o:shapelayout></xml><![endif]-->
</head>
<body lang="EN-US" link="blue" vlink="purple" style="word-wrap:break-word">
<div class="WordSection1">
<p class="MsoNormal"><span style="font-size:11.0pt">Hi Teja, <o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt">I have revised this patch based on your feedback but please don’t reply/comment on patch series in the below format we don’t really support it this also makes the subsequent replies to you comment come in
the same format. Please make sure you use the option to include the original message prefixed with a “>” instead of a space.<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt">Regards,<br>
Suraj Kandpal<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt"><o:p> </o:p></span></p>
<div style="border:none;border-left:solid blue 1.5pt;padding:0in 0in 0in 4.0pt">
<div>
<div style="border:none;border-top:solid #E1E1E1 1.0pt;padding:3.0pt 0in 0in 0in">
<p class="MsoNormal"><b><span style="font-size:11.0pt;font-family:"Calibri",sans-serif">From:</span></b><span style="font-size:11.0pt;font-family:"Calibri",sans-serif"> Pottumuttu, Sai Teja <sai.teja.pottumuttu@intel.com>
<br>
<b>Sent:</b> Friday, November 8, 2024 12:20 PM<br>
<b>To:</b> Kandpal, Suraj <suraj.kandpal@intel.com>; intel-xe@lists.freedesktop.org; intel-gfx@lists.freedesktop.org<br>
<b>Cc:</b> Govindapillai, Vinod <vinod.govindapillai@intel.com><br>
<b>Subject:</b> Re: [PATCH] drm/i914/watermark: Modify latency programmed into PKG_C_LATENCY<o:p></o:p></span></p>
</div>
</div>
<p class="MsoNormal"><o:p> </o:p></p>
<p><o:p> </o:p></p>
<div>
<p class="MsoNormal">On 07-11-2024 17:02, Suraj Kandpal wrote:<o:p></o:p></p>
</div>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<pre>Increase the latency programmed into PKG_C_LATENCY latency to be<o:p></o:p></pre>
<pre>a multiple of line time which is written into WM_LINETIME.<o:p></o:p></pre>
</blockquote>
<p>The commit subject prefix should be drm/i915/watermark (its i914 currently) <o:p>
</o:p></p>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<pre><o:p> </o:p></pre>
<pre>WA: 22020299601<o:p></o:p></pre>
<pre>Signed-off-by: Suraj Kandpal <a href="mailto:suraj.kandpal@intel.com"><suraj.kandpal@intel.com></a><o:p></o:p></pre>
<pre>---<o:p></o:p></pre>
<pre> drivers/gpu/drm/i915/display/skl_watermark.c | 26 ++++++++++++++------<o:p></o:p></pre>
<pre> 1 file changed, 18 insertions(+), 8 deletions(-)<o:p></o:p></pre>
<pre><o:p> </o:p></pre>
<pre>diff --git a/drivers/gpu/drm/i915/display/skl_watermark.c b/drivers/gpu/drm/i915/display/skl_watermark.c<o:p></o:p></pre>
<pre>index d3bbf335c749..856b20a683fd 100644<o:p></o:p></pre>
<pre>--- a/drivers/gpu/drm/i915/display/skl_watermark.c<o:p></o:p></pre>
<pre>+++ b/drivers/gpu/drm/i915/display/skl_watermark.c<o:p></o:p></pre>
<pre>@@ -2848,9 +2848,11 @@ static int skl_wm_add_affected_planes(struct intel_atomic_state *state,<o:p></o:p></pre>
<pre> * Program PKG_C_LATENCY Added Wake Time = 0<o:p></o:p></pre>
<pre> */<o:p></o:p></pre>
<pre> static void<o:p></o:p></pre>
<pre>-skl_program_dpkgc_latency(struct drm_i915_private *i915, bool enable_dpkgc)<o:p></o:p></pre>
<pre>+skl_program_dpkgc_latency(struct drm_i915_private *i915,<o:p></o:p></pre>
<pre>+ bool enable_dpkgc,<o:p></o:p></pre>
<pre>+ u32 max_linetime)<o:p></o:p></pre>
</blockquote>
<p>Nit: This ^ argument can fit in the line before.<o:p></o:p></p>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<pre><o:p> </o:p></pre>
<pre> {<o:p></o:p></pre>
<pre>- u32 max_latency = 0;<o:p></o:p></pre>
<pre>+ u32 adjusted_latency = 0;<o:p></o:p></pre>
<pre> u32 clear = 0, val = 0;<o:p></o:p></pre>
<pre> u32 added_wake_time = 0;<o:p></o:p></pre>
<pre> <o:p></o:p></pre>
<pre>@@ -2858,18 +2860,23 @@ skl_program_dpkgc_latency(struct drm_i915_private *i915, bool enable_dpkgc)<o:p></o:p></pre>
<pre> return;<o:p></o:p></pre>
<pre> <o:p></o:p></pre>
<pre> if (enable_dpkgc) {<o:p></o:p></pre>
<pre>- max_latency = skl_watermark_max_latency(i915, 1);<o:p></o:p></pre>
<pre>- if (max_latency == 0)<o:p></o:p></pre>
<pre>- max_latency = LNL_PKG_C_LATENCY_MASK;<o:p></o:p></pre>
<pre>+ adjusted_latency = skl_watermark_max_latency(i915, 1);<o:p></o:p></pre>
<pre>+ if (adjusted_latency == 0)<o:p></o:p></pre>
<pre>+ adjusted_latency = LNL_PKG_C_LATENCY_MASK;<o:p></o:p></pre>
<pre>+<o:p></o:p></pre>
<pre>+ /* Wa_22020299601 */<o:p></o:p></pre>
<pre>+ if (IS_DISPLAY_VERx100(i915, 2000, 3000))<o:p></o:p></pre>
</blockquote>
<p>This wa is applicable only to IP versions 2000 and 3000. So, shouldn't we be limiting this to only those IP versions instead of the full range?<o:p></o:p></p>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<pre><o:p> </o:p></pre>
<pre>+ adjusted_latency = max_linetime *<o:p></o:p></pre>
<pre>+ DIV_ROUND_UP(adjusted_latency, max_linetime);<o:p></o:p></pre>
<pre> added_wake_time = DSB_EXE_TIME +<o:p></o:p></pre>
<pre> i915->display.sagv.block_time_us;<o:p></o:p></pre>
<pre> } else {<o:p></o:p></pre>
<pre>- max_latency = LNL_PKG_C_LATENCY_MASK;<o:p></o:p></pre>
<pre>+ adjusted_latency = LNL_PKG_C_LATENCY_MASK;<o:p></o:p></pre>
<pre> added_wake_time = 0;<o:p></o:p></pre>
<pre> }<o:p></o:p></pre>
<pre> <o:p></o:p></pre>
<pre> clear |= LNL_ADDED_WAKE_TIME_MASK | LNL_PKG_C_LATENCY_MASK;<o:p></o:p></pre>
<pre>- val |= REG_FIELD_PREP(LNL_PKG_C_LATENCY_MASK, max_latency);<o:p></o:p></pre>
<pre>+ val |= REG_FIELD_PREP(LNL_PKG_C_LATENCY_MASK, adjusted_latency);<o:p></o:p></pre>
<pre> val |= REG_FIELD_PREP(LNL_ADDED_WAKE_TIME_MASK, added_wake_time);<o:p></o:p></pre>
<pre> <o:p></o:p></pre>
<pre> intel_uncore_rmw(&i915->uncore, LNL_PKG_C_LATENCY, clear, val);<o:p></o:p></pre>
<pre>@@ -2882,6 +2889,7 @@ skl_compute_wm(struct intel_atomic_state *state)<o:p></o:p></pre>
<pre> struct intel_crtc_state __maybe_unused *new_crtc_state;<o:p></o:p></pre>
<pre> int ret, i;<o:p></o:p></pre>
<pre> bool enable_dpkgc = false;<o:p></o:p></pre>
<pre>+ u32 max_linetime;<o:p></o:p></pre>
</blockquote>
<p>max_linetime should be initialized to 0 I believe so that the max comparison works correctly below.<o:p></o:p></p>
<p>Thanks<br>
Sai Teja<o:p></o:p></p>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<pre><o:p> </o:p></pre>
<pre> <o:p></o:p></pre>
<pre> for_each_new_intel_crtc_in_state(state, crtc, new_crtc_state, i) {<o:p></o:p></pre>
<pre> ret = skl_build_pipe_wm(state, crtc);<o:p></o:p></pre>
<pre>@@ -2911,9 +2919,11 @@ skl_compute_wm(struct intel_atomic_state *state)<o:p></o:p></pre>
<pre> new_crtc_state->vrr.vmin == new_crtc_state->vrr.flipline) ||<o:p></o:p></pre>
<pre> !new_crtc_state->vrr.enable)<o:p></o:p></pre>
<pre> enable_dpkgc = true;<o:p></o:p></pre>
<pre>+<o:p></o:p></pre>
<pre>+ max_linetime = max(new_crtc_state->linetime, max_linetime);<o:p></o:p></pre>
<pre> }<o:p></o:p></pre>
<pre> <o:p></o:p></pre>
<pre>- skl_program_dpkgc_latency(to_i915(state->base.dev), enable_dpkgc);<o:p></o:p></pre>
<pre>+ skl_program_dpkgc_latency(to_i915(state->base.dev), enable_dpkgc, max_linetime);<o:p></o:p></pre>
<pre> <o:p></o:p></pre>
<pre> skl_print_wm_changes(state);<o:p></o:p></pre>
<pre> <o:p></o:p></pre>
</blockquote>
</div>
</div>
</body>
</html>