<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">
<div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
Follow Evan's review, add smu->mutex. <br>
</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);">
<br>
</div>
<div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
This interface is for dGPU Navi1x. Linux dc-pplib interface depends<br>
<div> on window driver dc implementation.<br>
</div>
<div><br>
</div>
<div> For Navi1x, clock settings of dcn watermarks are fixed. the settings<br>
</div>
<div> should be passed to smu during boot up and resume from s3.<br>
</div>
<div> boot up: dc calculate dcn watermark clock settings within dc_create,<br>
</div>
<div> dcn20_resource_construct, then call pplib functions below to pass<br>
</div>
<div> the settings to smu:<br>
</div>
<div> smu_set_watermarks_for_clock_ranges<br>
</div>
<div> smu_set_watermarks_table<br>
</div>
<div> navi10_set_watermarks_table<br>
</div>
<div> smu_write_watermarks_table<br>
</div>
<div><br>
</div>
<div> For Renoir, clock settings of dcn watermark are also fixed values.<br>
</div>
<div> dc has implemented different flow for window driver:<br>
</div>
<div> dc_hardware_init / dc_set_power_state<br>
</div>
<div> dcn10_init_hw<br>
</div>
<div> notify_wm_ranges<br>
</div>
<div> set_wm_ranges<br>
</div>
<div><br>
</div>
<div> For Linux<br>
</div>
<div> smu_set_watermarks_for_clock_ranges<br>
</div>
<div> renoir_set_watermarks_table<br>
</div>
<div> smu_write_watermarks_table<br>
</div>
<div><br>
</div>
<div> dc_hardware_init -> amdgpu_dm_init<br>
</div>
<div> dc_set_power_state --> dm_resume<br>
</div>
<div><br>
</div>
<div> therefore, linux dc-pplib interface of navi10/12/14 is different<br>
</div>
<div> from that of Renoir.<br>
</div>
<div><br>
</div>
<div>Signed-off-by: Hersen Wu <hersenxs.wu@amd.com><br>
</div>
<div>---<br>
</div>
<div> .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 68 +++++++++++++++++++<br>
</div>
<div> 1 file changed, 68 insertions(+)<br>
</div>
<div><br>
</div>
<div>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>
</div>
<div>index 931cbd7b372e..1ee1d6ff2782 100644<br>
</div>
<div>--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c<br>
</div>
<div>+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c<br>
</div>
<div>@@ -1435,6 +1435,72 @@ static void s3_handle_mst(struct drm_device *dev, bool suspend)<br>
</div>
<div> drm_kms_helper_hotplug_event(dev);<br>
</div>
<div> }<br>
</div>
<div> <br>
</div>
<div>+static int amdgpu_dm_smu_write_watermarks_table(struct amdgpu_device *adev)<br>
</div>
<div>+{<br>
</div>
<div>+ struct smu_context *smu = &adev->smu;<br>
</div>
<div>+ int ret = 0;<br>
</div>
<div>+<br>
</div>
<div>+ if (!is_support_sw_smu(adev))<br>
</div>
<div>+ return 0;<br>
</div>
<div>+<br>
</div>
<div>+ /* This interface is for dGPU Navi1x.Linux dc-pplib interface depends<br>
</div>
<div>+ * on window driver dc implementation.<br>
</div>
<div>+ * For Navi1x, clock settings of dcn watermarks are fixed. the settings<br>
</div>
<div>+ * should be passed to smu during boot up and resume from s3.<br>
</div>
<div>+ * boot up: dc calculate dcn watermark clock settings within dc_create,<br>
</div>
<div>+ * dcn20_resource_construct<br>
</div>
<div>+ * then call pplib functions below to pass the settings to smu:<br>
</div>
<div>+ * smu_set_watermarks_for_clock_ranges<br>
</div>
<div>+ * smu_set_watermarks_table<br>
</div>
<div>+ * navi10_set_watermarks_table<br>
</div>
<div>+ * smu_write_watermarks_table<br>
</div>
<div>+ *<br>
</div>
<div>+ * For Renoir, clock settings of dcn watermark are also fixed values.<br>
</div>
<div>+ * dc has implemented different flow for window driver:<br>
</div>
<div>+ * dc_hardware_init / dc_set_power_state<br>
</div>
<div>+ * dcn10_init_hw<br>
</div>
<div>+ * notify_wm_ranges<br>
</div>
<div>+ * set_wm_ranges<br>
</div>
<div>+ * -- Linux<br>
</div>
<div>+ * smu_set_watermarks_for_clock_ranges<br>
</div>
<div>+ * renoir_set_watermarks_table<br>
</div>
<div>+ * smu_write_watermarks_table<br>
</div>
<div>+ *<br>
</div>
<div>+ * For Linux,<br>
</div>
<div>+ * dc_hardware_init -> amdgpu_dm_init<br>
</div>
<div>+ * dc_set_power_state --> dm_resume<br>
</div>
<div>+ *<br>
</div>
<div>+ * therefore, this function apply to navi10/12/14 but not Renoir<br>
</div>
<div>+ * *<br>
</div>
<div>+ */<br>
</div>
<div>+ switch(adev->asic_type) {<br>
</div>
<div>+ case CHIP_NAVI10:<br>
</div>
<div>+ case CHIP_NAVI14:<br>
</div>
<div>+ case CHIP_NAVI12:<br>
</div>
<div>+ break;<br>
</div>
<div>+ default:<br>
</div>
<div>+ return 0;<br>
</div>
<div>+ }<br>
</div>
<div>+<br>
</div>
<div>+ mutex_lock(&smu->mutex);<br>
</div>
<div>+<br>
</div>
<div>+ /* pass data to smu controller */<br>
</div>
<div>+ if ((smu->watermarks_bitmap & WATERMARKS_EXIST) &&<br>
</div>
<div>+ !(smu->watermarks_bitmap & WATERMARKS_LOADED)) {<br>
</div>
<div>+ ret = smu_write_watermarks_table(smu);<br>
</div>
<div>+<br>
</div>
<div>+ if (ret) {<br>
</div>
<div>+ DRM_ERROR("Failed to update WMTABLE!\n");<br>
</div>
<div>+ return ret;<br>
</div>
<div>+ }<br>
</div>
<div>+ smu->watermarks_bitmap |= WATERMARKS_LOADED;<br>
</div>
<div>+ }<br>
</div>
<div>+<br>
</div>
<div>+ mutex_unlock(&smu->mutex);<br>
</div>
<div>+<br>
</div>
<div>+ return 0;<br>
</div>
<div>+}<br>
</div>
<div>+<br>
</div>
<div> /**<br>
</div>
<div> * dm_hw_init() - Initialize DC device<br>
</div>
<div> * @handle: The base driver device containing the amdgpu_dm device.<br>
</div>
<div>@@ -1713,6 +1779,8 @@ static int dm_resume(void *handle)<br>
</div>
<div> <br>
</div>
<div> amdgpu_dm_irq_resume_late(adev);<br>
</div>
<div> <br>
</div>
<div>+ amdgpu_dm_smu_write_watermarks_table(adev);<br>
</div>
<div>+<br>
</div>
<div> return 0;<br>
</div>
<div> }<br>
</div>
<div> <br>
</div>
<div>-- <br>
</div>
<div>2.17.1<br>
</div>
<span></span><br>
</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> Quan, Evan <Evan.Quan@amd.com><br>
<b>Sent:</b> February 27, 2020 9:58 PM<br>
<b>To:</b> Wu, Hersen <hersenxs.wu@amd.com>; amd-gfx@lists.freedesktop.org <amd-gfx@lists.freedesktop.org><br>
<b>Cc:</b> Feng, Kenneth <Kenneth.Feng@amd.com>; Wu, Hersen <hersenxs.wu@amd.com><br>
<b>Subject:</b> RE: [PATCH 2/2] drm/amdgpu/display: navi1x copy dcn watermark clock settings to smu resume from s3</font>
<div> </div>
</div>
<div class="BodyFragment"><font size="2"><span style="font-size:11pt;">
<div class="PlainText">Thanks. But could you help to confirm whether this is correctly protected by "mutex_lock(&smu->mutex)"?<br>
<br>
-----Original Message-----<br>
From: Hersen Wu <hersenxs.wu@amd.com> <br>
Sent: Thursday, February 27, 2020 11:54 PM<br>
To: amd-gfx@lists.freedesktop.org<br>
Cc: Quan, Evan <Evan.Quan@amd.com>; Feng, Kenneth <Kenneth.Feng@amd.com>; Wu, Hersen <hersenxs.wu@amd.com><br>
Subject: [PATCH 2/2] drm/amdgpu/display: navi1x copy dcn watermark clock settings to smu resume from s3<br>
<br>
This interface is for dGPU Navi1x. Linux dc-pplib interface depends on window driver dc implementation.<br>
<br>
For Navi1x, clock settings of dcn watermarks are fixed. the settings should be passed to smu during boot up and resume from s3.<br>
boot up: dc calculate dcn watermark clock settings within dc_create, dcn20_resource_construct, then call pplib functions below to pass the settings to smu:<br>
smu_set_watermarks_for_clock_ranges<br>
smu_set_watermarks_table<br>
navi10_set_watermarks_table<br>
smu_write_watermarks_table<br>
<br>
For Renoir, clock settings of dcn watermark are also fixed values.<br>
dc has implemented different flow for window driver:<br>
dc_hardware_init / dc_set_power_state<br>
dcn10_init_hw<br>
notify_wm_ranges<br>
set_wm_ranges<br>
<br>
For Linux<br>
smu_set_watermarks_for_clock_ranges<br>
renoir_set_watermarks_table<br>
smu_write_watermarks_table<br>
<br>
dc_hardware_init -> amdgpu_dm_init<br>
dc_set_power_state --> dm_resume<br>
<br>
therefore, linux dc-pplib interface of navi10/12/14 is different from that of Renoir.<br>
<br>
Signed-off-by: Hersen Wu <hersenxs.wu@amd.com><br>
---<br>
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 64 +++++++++++++++++++<br>
1 file changed, 64 insertions(+)<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 931cbd7b372e..c58c0e95735e 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>
@@ -1435,6 +1435,68 @@ static void s3_handle_mst(struct drm_device *dev, bool suspend)<br>
drm_kms_helper_hotplug_event(dev);<br>
}<br>
<br>
+static int amdgpu_dm_smu_write_watermarks_table(struct amdgpu_device <br>
+*adev) {<br>
+ struct smu_context *smu = &adev->smu;<br>
+ int ret = 0;<br>
+<br>
+ if (!is_support_sw_smu(adev))<br>
+ return 0;<br>
+<br>
+ /* This interface is for dGPU Navi1x.Linux dc-pplib interface depends<br>
+ * on window driver dc implementation.<br>
+ * For Navi1x, clock settings of dcn watermarks are fixed. the settings<br>
+ * should be passed to smu during boot up and resume from s3.<br>
+ * boot up: dc calculate dcn watermark clock settings within dc_create,<br>
+ * dcn20_resource_construct<br>
+ * then call pplib functions below to pass the settings to smu:<br>
+ * smu_set_watermarks_for_clock_ranges<br>
+ * smu_set_watermarks_table<br>
+ * navi10_set_watermarks_table<br>
+ * smu_write_watermarks_table<br>
+ *<br>
+ * For Renoir, clock settings of dcn watermark are also fixed values.<br>
+ * dc has implemented different flow for window driver:<br>
+ * dc_hardware_init / dc_set_power_state<br>
+ * dcn10_init_hw<br>
+ * notify_wm_ranges<br>
+ * set_wm_ranges<br>
+ * -- Linux<br>
+ * smu_set_watermarks_for_clock_ranges<br>
+ * renoir_set_watermarks_table<br>
+ * smu_write_watermarks_table<br>
+ *<br>
+ * For Linux,<br>
+ * dc_hardware_init -> amdgpu_dm_init<br>
+ * dc_set_power_state --> dm_resume<br>
+ *<br>
+ * therefore, this function apply to navi10/12/14 but not Renoir<br>
+ * *<br>
+ */<br>
+ switch(adev->asic_type) {<br>
+ case CHIP_NAVI10:<br>
+ case CHIP_NAVI14:<br>
+ case CHIP_NAVI12:<br>
+ break;<br>
+ default:<br>
+ return 0;<br>
+ }<br>
+<br>
+ /* pass data to smu controller */<br>
+ if ((smu->watermarks_bitmap & WATERMARKS_EXIST) &&<br>
+ !(smu->watermarks_bitmap & WATERMARKS_LOADED)) {<br>
+ ret = smu_write_watermarks_table(smu);<br>
+<br>
+ if (ret) {<br>
+ DRM_ERROR("Failed to update WMTABLE!\n");<br>
+ return ret;<br>
+ }<br>
+ smu->watermarks_bitmap |= WATERMARKS_LOADED;<br>
+ }<br>
+<br>
+ return 0;<br>
+}<br>
+<br>
/**<br>
* dm_hw_init() - Initialize DC device<br>
* @handle: The base driver device containing the amdgpu_dm device.<br>
@@ -1713,6 +1775,8 @@ static int dm_resume(void *handle)<br>
<br>
amdgpu_dm_irq_resume_late(adev);<br>
<br>
+ amdgpu_dm_smu_write_watermarks_table(adev);<br>
+<br>
return 0;<br>
}<br>
<br>
--<br>
2.17.1<br>
<br>
</div>
</span></font></div>
</body>
</html>