<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=iso-8859-1">
<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:Consolas;
panose-1:2 11 6 9 2 2 4 3 2 4;}
/* Style Definitions */
p.MsoNormal, li.MsoNormal, div.MsoNormal
{margin:0in;
margin-bottom:.0001pt;
font-size:11.0pt;
font-family:"Calibri",sans-serif;
color:black;}
a:link, span.MsoHyperlink
{mso-style-priority:99;
color:blue;
text-decoration:underline;}
a:visited, span.MsoHyperlinkFollowed
{mso-style-priority:99;
color:purple;
text-decoration:underline;}
p.MsoPlainText, li.MsoPlainText, div.MsoPlainText
{mso-style-priority:99;
mso-style-link:"Plain Text Char";
margin:0in;
margin-bottom:.0001pt;
font-size:11.0pt;
font-family:"Calibri",sans-serif;
color:black;}
p
{mso-style-priority:99;
mso-margin-top-alt:auto;
margin-right:0in;
mso-margin-bottom-alt:auto;
margin-left:0in;
font-size:11.0pt;
font-family:"Calibri",sans-serif;
color:black;}
pre
{mso-style-priority:99;
mso-style-link:"HTML Preformatted Char";
margin:0in;
margin-bottom:.0001pt;
font-size:10.0pt;
font-family:"Courier New";
color:black;}
span.HTMLPreformattedChar
{mso-style-name:"HTML Preformatted Char";
mso-style-priority:99;
mso-style-link:"HTML Preformatted";
font-family:Consolas;
color:black;}
span.PlainTextChar
{mso-style-name:"Plain Text Char";
mso-style-priority:99;
mso-style-link:"Plain Text";
font-family:"Calibri",sans-serif;}
span.EmailStyle22
{mso-style-type:personal-reply;
font-family:"Calibri",sans-serif;
color:#1F497D;}
.MsoChpDefault
{mso-style-type:export-only;
font-size:10.0pt;}
@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 bgcolor="white" lang="EN-US" link="blue" vlink="purple">
<div class="WordSection1">
<p class="MsoNormal"><span style="color:#1F497D">+Susanta<o:p></o:p></span></p>
<p class="MsoNormal"><a name="_MailEndCompose"><span style="color:#1F497D"><o:p> </o:p></span></a></p>
<div>
<div style="border:none;border-top:solid #E1E1E1 1.0pt;padding:3.0pt 0in 0in 0in">
<p class="MsoNormal"><a name="_____replyseparator"></a><b><span style="color:windowtext">From:</span></b><span style="color:windowtext"> Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org]
<b>On Behalf Of </b>Kumar, Abhay<br>
<b>Sent:</b> Tuesday, August 28, 2018 5:55 PM<br>
<b>To:</b> Ville Syrjälä <ville.syrjala@linux.intel.com><br>
<b>Cc:</b> intel-gfx@lists.freedesktop.org<br>
<b>Subject:</b> Re: [Intel-gfx] [PATCH v1] drm/i915: Skip modeset for cdclk changes if possible<o:p></o:p></span></p>
</div>
</div>
<p class="MsoNormal"><o:p> </o:p></p>
<p><span style="font-size:12.0pt"><o:p> </o:p></span></p>
<p class="MsoNormal"><o:p> </o:p></p>
<div>
<p class="MsoNormal">On 8/28/2018 5:39 AM, Ville Syrjälä wrote:<o:p></o:p></p>
</div>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<pre>On Mon, Aug 27, 2018 at 11:50:32AM -0700, Abhay Kumar wrote:<o:p></o:p></pre>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<pre>From: Ville Syrjälä <a href="mailto:ville.syrjala@linux.intel.com"><ville.syrjala@linux.intel.com></a><o:p></o:p></pre>
<pre><o:p> </o:p></pre>
<pre>If we have only a single active pipe and the cdclk change only requires<o:p></o:p></pre>
<pre>the cd2x divider to be updated bxt+ can do the update with forcing a full<o:p></o:p></pre>
<pre>modeset on the pipe. Try to hook that up.<o:p></o:p></pre>
<pre><o:p> </o:p></pre>
<pre>Signed-off-by: Ville Syrjälä <a href="mailto:ville.syrjala@linux.intel.com"><ville.syrjala@linux.intel.com></a><o:p></o:p></pre>
<pre>Signed-off-by: Abhay Kumar <a href="mailto:abhay.kumar@intel.com"><abhay.kumar@intel.com></a><o:p></o:p></pre>
<pre>---<o:p></o:p></pre>
<pre> drivers/gpu/drm/i915/i915_drv.h | 3 +-<o:p></o:p></pre>
<pre> drivers/gpu/drm/i915/i915_reg.h | 3 +-<o:p></o:p></pre>
<pre> drivers/gpu/drm/i915/intel_cdclk.c | 105 +++++++++++++++++++++++++----------<o:p></o:p></pre>
<pre> drivers/gpu/drm/i915/intel_display.c | 20 ++++++-<o:p></o:p></pre>
<pre> drivers/gpu/drm/i915/intel_drv.h | 9 ++-<o:p></o:p></pre>
<pre> 5 files changed, 105 insertions(+), 35 deletions(-)<o:p></o:p></pre>
<pre><o:p> </o:p></pre>
</blockquote>
<pre><snip><o:p></o:p></pre>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<pre>@@ -12252,12 +12253,24 @@ static int intel_modeset_checks(struct drm_atomic_state *state)<o:p></o:p></pre>
<pre> return ret;<o:p></o:p></pre>
<pre> }<o:p></o:p></pre>
<pre> <o:p></o:p></pre>
<pre>+<o:p></o:p></pre>
<pre> /* All pipes must be switched off while we change the cdclk. */<o:p></o:p></pre>
<pre>- if (intel_cdclk_needs_modeset(&dev_priv->cdclk.actual,<o:p></o:p></pre>
<pre>- &intel_state->cdclk.actual)) {<o:p></o:p></pre>
<pre>+ if (is_power_of_2(intel_state->active_crtcs) &&<o:p></o:p></pre>
<pre>+ intel_cdclk_needs_cd2x_update(dev_priv,<o:p></o:p></pre>
<pre>+ &dev_priv->cdclk.actual,<o:p></o:p></pre>
<pre>+ &intel_state->cdclk.actual)) {<o:p></o:p></pre>
<pre>+ ret = intel_lock_all_pipes(state);<o:p></o:p></pre>
<pre>+ if (ret < 0)<o:p></o:p></pre>
<pre>+ return ret;<o:p></o:p></pre>
<pre>+<o:p></o:p></pre>
<pre>+ intel_state->cdclk.pipe = ilog2(intel_state->active_crtcs);<o:p></o:p></pre>
</blockquote>
<pre><o:p> </o:p></pre>
<pre>BTW on further reflection this probably isn't quite sufficient. Let's<o:p></o:p></pre>
<pre>say we have a commit with allow_modeset=true, but we aren't actually<o:p></o:p></pre>
<pre>required to do a modeset based on any of the state changes. If we still<o:p></o:p></pre>
<pre>have to change cdclk we should actually be doing the cd2x update<o:p></o:p></pre>
<pre>atomically with the plane updates, or we should do it before or after<o:p></o:p></pre>
<pre>the plane updates depending on whether the cdclk freq is going up or<o:p></o:p></pre>
<pre>down.<o:p></o:p></pre>
<pre><o:p> </o:p></pre>
<pre>Doing the update atomically with the plane updates might be nicer in the<o:p></o:p></pre>
<pre>end, but for that we would likely need to split the .set_cdclk() hooks<o:p></o:p></pre>
<pre>into three parts (pre+commit+post). Whereas just doing the update before<o:p></o:p></pre>
<pre>or after the plane updates as needed would probably be a little simpler.<o:p></o:p></pre>
</blockquote>
<p class="MsoNormal" style="margin-bottom:12.0pt">Yeah. That might also get rid of cdclk mismatch warning during multiple suspend resume cycle.<o:p></o:p></p>
<p class="MsoPlainText">[ 280.600259] cdclk state doesn't match!<o:p></o:p></p>
<p class="MsoPlainText">[ 280.600270] calling 1-8+ @ 3110, parent: usb1, cb: usb_dev_resume<o:p></o:p></p>
<p class="MsoPlainText">[ 280.600276] WARNING: CPU: 3 PID: 5224 at /mnt/host/source/src/third_party/ker<o:p></o:p></p>
<p class="MsoPlainText">nel/v4.14/drivers/gpu/drm/i915/intel_cdclk.c:1867 intel_set_cdclk+0xaa/0xdb<o:p></o:p></p>
<p class="MsoPlainText">[ 280.600277] Modules linked in: cmac rfcomm uinput snd_soc_sst_bxt_da7219_max9<o:p></o:p></p>
<p class="MsoPlainText">8357a snd_soc_hdac_hdmi snd_soc_dmic lzo lzo_compress snd_soc_skl snd_soc_skl_ip<o:p></o:p></p>
<p class="MsoPlainText">c snd_soc_sst_ipc snd_soc_sst_dsp snd_soc_acpi snd_hda_ext_core snd_hda_core<o:p></o:p></p>
<p class="MsoPlainText">[ 280.600307] calling phy0+ @ 3102, parent: 0000:00:0c.0, cb: wiphy_resume [cf<o:p></o:p></p>
<p class="MsoPlainText">g80211]<o:p></o:p></p>
<p class="MsoPlainText">[ 280.600308] zram snd_soc_max98357a acpi_als snd_soc_da7219 bridge stp llc ip<o:p></o:p></p>
<p class="MsoPlainText">t_MASQUERADE nf_nat_masquerade_ipv4 xt_mark fuse snd_seq_dummy snd_seq snd_seq_d<o:p></o:p></p>
<p class="MsoPlainText">evice btusb btrtl btbcm iio_trig_sysfs btintel uvcvideo bluetooth videobuf2_vmal<o:p></o:p></p>
<p class="MsoPlainText">loc videobuf2_memops videobuf2_v4l2 ecdh_generic videobuf2_core cros_ec_sensors<o:p></o:p></p>
<p class="MsoPlainText">cros_ec_sensors_ring cros_ec_sensors_core industrialio_triggered_buffer kfifo_bu<o:p></o:p></p>
<p class="MsoPlainText">[ 280.600346] RDX: ffffffffb8258dd0 RSI: 0000000000000002 RDI: ffffffffb8258db0<o:p></o:p></p>
<p class="MsoPlainText">[ 280.600347] RBP: ffffbb9546b73aa0 R08: 0000000000000000 R09: 0000000000000000<o:p></o:p></p>
<p class="MsoPlainText">[ 280.600348] R10: 0000000000000000 R11: ffffffffb86d8518 R12: ffffa22075790000<o:p></o:p></p>
<p class="MsoPlainText">[ 280.600349] R13: ffffa22075793d24 R14: 00000000ffffffff R15: ffffa2202eec9800<o:p></o:p></p>
<p class="MsoPlainText">00000000000<o:p></o:p></p>
<p class="MsoPlainText">[ 280.600352] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033<o:p></o:p></p>
<p class="MsoPlainText">[ 280.600353] CR2: 00007f43c04d0e50 CR3: 0000000224112000 CR4: 00000000003406e0<o:p></o:p></p>
<p class="MsoPlainText">[ 280.600354] Call Trace:<o:p></o:p></p>
<p class="MsoPlainText">[ 280.600361] intel_atomic_commit_tail+0x20a/0xacb<o:p></o:p></p>
<p class="MsoPlainText">[ 280.600363] ? intel_atomic_commit_ready+0x44/0x4c<o:p></o:p></p>
<p class="MsoPlainText">[ 280.600365] intel_atomic_commit+0x227/0x238<o:p></o:p></p>
<p class="MsoPlainText">[ 280.600368] glk_force_audio_cdclk+0x9f/0x119<o:p></o:p></p>
<p class="MsoPlainText">[ 280.600370] i915_audio_component_get_power+0x3e/0x4d<o:p></o:p></p>
<p class="MsoPlainText">[ 280.600376] snd_hdac_display_power+0x53/0x97 [snd_hda_core]<o:p></o:p></p>
<p class="MsoPlainText">[ 280.600379] calling 1-9+ @ 3086, parent: usb1, cb: usb_dev_resume<o:p></o:p></p>
<p class="MsoPlainText">[ 280.600384] skl_resume+0x3a/0x17a [snd_soc_skl]<o:p></o:p></p>
<p class="MsoPlainText">[ 280.600387] ? pci_pm_suspend_noirq+0x1e9/0x1e9<o:p></o:p></p>
<p class="MsoPlainText">[ 280.600391] dpm_run_callback+0x59/0xbf<o:p></o:p></p>
<p class="MsoPlainText">[ 280.600394] device_resume+0x192/0x1d4<o:p></o:p></p>
<p class="MsoPlainText">[ 280.600396] dpm_resume+0x145/0x1da<o:p></o:p></p>
<p class="MsoPlainText">[ 280.600398] dpm_resume_end+0x11/0x1a<o:p></o:p></p>
<p class="MsoPlainText">[ 280.600403] suspend_devices_and_enter+0x354/0x5c2<o:p></o:p></p>
<p class="MsoPlainText">[ 280.600407] ? remove_wait_queue+0x51/0x51<o:p></o:p></p>
<p class="MsoPlainText">[ 280.600409] pm_suspend+0x29c/0x2e2<o:p></o:p></p>
<p class="MsoPlainText">[ 280.600411] state_store+0xa2/0xcb<o:p></o:p></p>
<p class="MsoPlainText">[ 280.600415] kernfs_fop_write+0x103/0x14a<o:p></o:p></p>
<p class="MsoPlainText">[ 280.600420] __vfs_write+0x37/0xd0<o:p></o:p></p>
<p class="MsoPlainText">[ 280.600424] ? inode_security+0x19/0x20<o:p></o:p></p>
<p class="MsoPlainText">[ 280.600426] ? selinux_file_permission+0x78/0xad<o:p></o:p></p>
<p class="MsoPlainText">[ 280.600428] vfs_write+0xb9/0xfd<o:p></o:p></p>
<p class="MsoPlainText">[ 280.600430] SyS_write+0x5f/0xa3<o:p></o:p></p>
<p class="MsoPlainText">[ 280.600434] do_syscall_64+0x64/0x72<o:p></o:p></p>
<p class="MsoPlainText">[ 280.600438] entry_SYSCALL_64_after_hwframe+0x3d/0xa2<o:p></o:p></p>
<p class="MsoPlainText">[ 280.600441] RIP: 0033:0x7f969d8af3b0<o:p></o:p></p>
<p class="MsoPlainText">00001<o:p></o:p></p>
<p class="MsoPlainText">[ 280.600443] RAX: ffffffffffffffda RBX: 00007f969ddf0000 RCX: 00007f969d8af3b0<o:p></o:p></p>
<p class="MsoPlainText">[ 280.600444] RDX: 0000000000000006 RSI: 00007f969ddf0000 RDI: 0000000000000001<o:p></o:p></p>
<p class="MsoPlainText">[ 280.600445] RBP: 00007ffd9e3f4880 R08: ffffffffffffffff R09: 0000000000000000<o:p></o:p></p>
<p class="MsoPlainText">[ 280.600446] R10: 0000000000001000 R11: 0000000000000246 R12: 0000000000000006<o:p></o:p></p>
<p class="MsoPlainText">[ 280.600447] R13: 0000000000020000 R14: 00007f969ddf0000 R15: 0000000000000001<o:p></o:p></p>
<p class="MsoNormal"><span style="font-size:12.0pt;font-family:"Times New Roman",serif"><br>
<br>
<o:p></o:p></span></p>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<pre><o:p> </o:p></pre>
<pre><o:p> </o:p></pre>
<pre><o:p> </o:p></pre>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<pre>+ } else if (intel_cdclk_needs_modeset(&dev_priv->cdclk.actual,<o:p></o:p></pre>
<pre>+ &intel_state->cdclk.actual)) {<o:p></o:p></pre>
<pre> ret = intel_modeset_all_pipes(state);<o:p></o:p></pre>
<pre> if (ret < 0)<o:p></o:p></pre>
<pre> return ret;<o:p></o:p></pre>
<pre>+<o:p></o:p></pre>
<pre>+ intel_state->cdclk.pipe = INVALID_PIPE;<o:p></o:p></pre>
<pre> }<o:p></o:p></pre>
<pre> <o:p></o:p></pre>
<pre> DRM_DEBUG_KMS("New cdclk calculated to be logical %u kHz, actual %u kHz\n",<o:p></o:p></pre>
</blockquote>
<pre><o:p> </o:p></pre>
</blockquote>
<p class="MsoNormal"><span style="font-size:12.0pt;font-family:"Times New Roman",serif"><o:p> </o:p></span></p>
</div>
</body>
</html>