* [PATCH 2/2] drm/amdgpu/display: navi1x copy dcn watermark clock settings to smu resume from s3
@ 2020-02-27 15:53 Hersen Wu
2020-02-27 16:06 ` Alex Deucher
2020-02-28 2:58 ` Quan, Evan
0 siblings, 2 replies; 7+ messages in thread
From: Hersen Wu @ 2020-02-27 15:53 UTC (permalink / raw)
To: amd-gfx; +Cc: Hersen Wu, evan.quan, kenneth.feng
This interface is for dGPU Navi1x. Linux dc-pplib interface depends
on window driver dc implementation.
For Navi1x, clock settings of dcn watermarks are fixed. the settings
should be passed to smu during boot up and resume from s3.
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:
smu_set_watermarks_for_clock_ranges
smu_set_watermarks_table
navi10_set_watermarks_table
smu_write_watermarks_table
For Renoir, clock settings of dcn watermark are also fixed values.
dc has implemented different flow for window driver:
dc_hardware_init / dc_set_power_state
dcn10_init_hw
notify_wm_ranges
set_wm_ranges
For Linux
smu_set_watermarks_for_clock_ranges
renoir_set_watermarks_table
smu_write_watermarks_table
dc_hardware_init -> amdgpu_dm_init
dc_set_power_state --> dm_resume
therefore, linux dc-pplib interface of navi10/12/14 is different
from that of Renoir.
Signed-off-by: Hersen Wu <hersenxs.wu@amd.com>
---
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 64 +++++++++++++++++++
1 file changed, 64 insertions(+)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 931cbd7b372e..c58c0e95735e 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -1435,6 +1435,68 @@ static void s3_handle_mst(struct drm_device *dev, bool suspend)
drm_kms_helper_hotplug_event(dev);
}
+static int amdgpu_dm_smu_write_watermarks_table(struct amdgpu_device *adev)
+{
+ struct smu_context *smu = &adev->smu;
+ int ret = 0;
+
+ if (!is_support_sw_smu(adev))
+ return 0;
+
+ /* This interface is for dGPU Navi1x.Linux dc-pplib interface depends
+ * on window driver dc implementation.
+ * For Navi1x, clock settings of dcn watermarks are fixed. the settings
+ * should be passed to smu during boot up and resume from s3.
+ * 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:
+ * smu_set_watermarks_for_clock_ranges
+ * smu_set_watermarks_table
+ * navi10_set_watermarks_table
+ * smu_write_watermarks_table
+ *
+ * For Renoir, clock settings of dcn watermark are also fixed values.
+ * dc has implemented different flow for window driver:
+ * dc_hardware_init / dc_set_power_state
+ * dcn10_init_hw
+ * notify_wm_ranges
+ * set_wm_ranges
+ * -- Linux
+ * smu_set_watermarks_for_clock_ranges
+ * renoir_set_watermarks_table
+ * smu_write_watermarks_table
+ *
+ * For Linux,
+ * dc_hardware_init -> amdgpu_dm_init
+ * dc_set_power_state --> dm_resume
+ *
+ * therefore, this function apply to navi10/12/14 but not Renoir
+ * *
+ */
+ switch(adev->asic_type) {
+ case CHIP_NAVI10:
+ case CHIP_NAVI14:
+ case CHIP_NAVI12:
+ break;
+ default:
+ return 0;
+ }
+
+ /* pass data to smu controller */
+ if ((smu->watermarks_bitmap & WATERMARKS_EXIST) &&
+ !(smu->watermarks_bitmap & WATERMARKS_LOADED)) {
+ ret = smu_write_watermarks_table(smu);
+
+ if (ret) {
+ DRM_ERROR("Failed to update WMTABLE!\n");
+ return ret;
+ }
+ smu->watermarks_bitmap |= WATERMARKS_LOADED;
+ }
+
+ return 0;
+}
+
/**
* dm_hw_init() - Initialize DC device
* @handle: The base driver device containing the amdgpu_dm device.
@@ -1713,6 +1775,8 @@ static int dm_resume(void *handle)
amdgpu_dm_irq_resume_late(adev);
+ amdgpu_dm_smu_write_watermarks_table(adev);
+
return 0;
}
--
2.17.1
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] drm/amdgpu/display: navi1x copy dcn watermark clock settings to smu resume from s3
2020-02-27 15:53 [PATCH 2/2] drm/amdgpu/display: navi1x copy dcn watermark clock settings to smu resume from s3 Hersen Wu
@ 2020-02-27 16:06 ` Alex Deucher
2020-02-28 2:58 ` Quan, Evan
1 sibling, 0 replies; 7+ messages in thread
From: Alex Deucher @ 2020-02-27 16:06 UTC (permalink / raw)
To: Hersen Wu; +Cc: Quan, Evan, Kenneth Feng, amd-gfx list
On Thu, Feb 27, 2020 at 10:54 AM Hersen Wu <hersenxs.wu@amd.com> wrote:
>
> This interface is for dGPU Navi1x. Linux dc-pplib interface depends
> on window driver dc implementation.
>
> For Navi1x, clock settings of dcn watermarks are fixed. the settings
> should be passed to smu during boot up and resume from s3.
> 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:
> smu_set_watermarks_for_clock_ranges
> smu_set_watermarks_table
> navi10_set_watermarks_table
> smu_write_watermarks_table
>
> For Renoir, clock settings of dcn watermark are also fixed values.
> dc has implemented different flow for window driver:
> dc_hardware_init / dc_set_power_state
> dcn10_init_hw
> notify_wm_ranges
> set_wm_ranges
>
> For Linux
> smu_set_watermarks_for_clock_ranges
> renoir_set_watermarks_table
> smu_write_watermarks_table
>
> dc_hardware_init -> amdgpu_dm_init
> dc_set_power_state --> dm_resume
>
> therefore, linux dc-pplib interface of navi10/12/14 is different
> from that of Renoir.
>
> Signed-off-by: Hersen Wu <hersenxs.wu@amd.com>
Acked-by: Alex Deucher <alexander.deucher@amd.com>
> ---
> .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 64 +++++++++++++++++++
> 1 file changed, 64 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> index 931cbd7b372e..c58c0e95735e 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -1435,6 +1435,68 @@ static void s3_handle_mst(struct drm_device *dev, bool suspend)
> drm_kms_helper_hotplug_event(dev);
> }
>
> +static int amdgpu_dm_smu_write_watermarks_table(struct amdgpu_device *adev)
> +{
> + struct smu_context *smu = &adev->smu;
> + int ret = 0;
> +
> + if (!is_support_sw_smu(adev))
> + return 0;
> +
> + /* This interface is for dGPU Navi1x.Linux dc-pplib interface depends
> + * on window driver dc implementation.
> + * For Navi1x, clock settings of dcn watermarks are fixed. the settings
> + * should be passed to smu during boot up and resume from s3.
> + * 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:
> + * smu_set_watermarks_for_clock_ranges
> + * smu_set_watermarks_table
> + * navi10_set_watermarks_table
> + * smu_write_watermarks_table
> + *
> + * For Renoir, clock settings of dcn watermark are also fixed values.
> + * dc has implemented different flow for window driver:
> + * dc_hardware_init / dc_set_power_state
> + * dcn10_init_hw
> + * notify_wm_ranges
> + * set_wm_ranges
> + * -- Linux
> + * smu_set_watermarks_for_clock_ranges
> + * renoir_set_watermarks_table
> + * smu_write_watermarks_table
> + *
> + * For Linux,
> + * dc_hardware_init -> amdgpu_dm_init
> + * dc_set_power_state --> dm_resume
> + *
> + * therefore, this function apply to navi10/12/14 but not Renoir
> + * *
> + */
> + switch(adev->asic_type) {
> + case CHIP_NAVI10:
> + case CHIP_NAVI14:
> + case CHIP_NAVI12:
> + break;
> + default:
> + return 0;
> + }
> +
> + /* pass data to smu controller */
> + if ((smu->watermarks_bitmap & WATERMARKS_EXIST) &&
> + !(smu->watermarks_bitmap & WATERMARKS_LOADED)) {
> + ret = smu_write_watermarks_table(smu);
> +
> + if (ret) {
> + DRM_ERROR("Failed to update WMTABLE!\n");
> + return ret;
> + }
> + smu->watermarks_bitmap |= WATERMARKS_LOADED;
> + }
> +
> + return 0;
> +}
> +
> /**
> * dm_hw_init() - Initialize DC device
> * @handle: The base driver device containing the amdgpu_dm device.
> @@ -1713,6 +1775,8 @@ static int dm_resume(void *handle)
>
> amdgpu_dm_irq_resume_late(adev);
>
> + amdgpu_dm_smu_write_watermarks_table(adev);
> +
> return 0;
> }
>
> --
> 2.17.1
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 7+ messages in thread
* RE: [PATCH 2/2] drm/amdgpu/display: navi1x copy dcn watermark clock settings to smu resume from s3
2020-02-27 15:53 [PATCH 2/2] drm/amdgpu/display: navi1x copy dcn watermark clock settings to smu resume from s3 Hersen Wu
2020-02-27 16:06 ` Alex Deucher
@ 2020-02-28 2:58 ` Quan, Evan
2020-02-28 20:59 ` Wu, Hersen
1 sibling, 1 reply; 7+ messages in thread
From: Quan, Evan @ 2020-02-28 2:58 UTC (permalink / raw)
To: Wu, Hersen, amd-gfx; +Cc: Wu, Hersen, Feng, Kenneth
Thanks. But could you help to confirm whether this is correctly protected by "mutex_lock(&smu->mutex)"?
-----Original Message-----
From: Hersen Wu <hersenxs.wu@amd.com>
Sent: Thursday, February 27, 2020 11:54 PM
To: amd-gfx@lists.freedesktop.org
Cc: Quan, Evan <Evan.Quan@amd.com>; Feng, Kenneth <Kenneth.Feng@amd.com>; Wu, Hersen <hersenxs.wu@amd.com>
Subject: [PATCH 2/2] drm/amdgpu/display: navi1x copy dcn watermark clock settings to smu resume from s3
This interface is for dGPU Navi1x. Linux dc-pplib interface depends on window driver dc implementation.
For Navi1x, clock settings of dcn watermarks are fixed. the settings should be passed to smu during boot up and resume from s3.
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:
smu_set_watermarks_for_clock_ranges
smu_set_watermarks_table
navi10_set_watermarks_table
smu_write_watermarks_table
For Renoir, clock settings of dcn watermark are also fixed values.
dc has implemented different flow for window driver:
dc_hardware_init / dc_set_power_state
dcn10_init_hw
notify_wm_ranges
set_wm_ranges
For Linux
smu_set_watermarks_for_clock_ranges
renoir_set_watermarks_table
smu_write_watermarks_table
dc_hardware_init -> amdgpu_dm_init
dc_set_power_state --> dm_resume
therefore, linux dc-pplib interface of navi10/12/14 is different from that of Renoir.
Signed-off-by: Hersen Wu <hersenxs.wu@amd.com>
---
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 64 +++++++++++++++++++
1 file changed, 64 insertions(+)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 931cbd7b372e..c58c0e95735e 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -1435,6 +1435,68 @@ static void s3_handle_mst(struct drm_device *dev, bool suspend)
drm_kms_helper_hotplug_event(dev);
}
+static int amdgpu_dm_smu_write_watermarks_table(struct amdgpu_device
+*adev) {
+ struct smu_context *smu = &adev->smu;
+ int ret = 0;
+
+ if (!is_support_sw_smu(adev))
+ return 0;
+
+ /* This interface is for dGPU Navi1x.Linux dc-pplib interface depends
+ * on window driver dc implementation.
+ * For Navi1x, clock settings of dcn watermarks are fixed. the settings
+ * should be passed to smu during boot up and resume from s3.
+ * 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:
+ * smu_set_watermarks_for_clock_ranges
+ * smu_set_watermarks_table
+ * navi10_set_watermarks_table
+ * smu_write_watermarks_table
+ *
+ * For Renoir, clock settings of dcn watermark are also fixed values.
+ * dc has implemented different flow for window driver:
+ * dc_hardware_init / dc_set_power_state
+ * dcn10_init_hw
+ * notify_wm_ranges
+ * set_wm_ranges
+ * -- Linux
+ * smu_set_watermarks_for_clock_ranges
+ * renoir_set_watermarks_table
+ * smu_write_watermarks_table
+ *
+ * For Linux,
+ * dc_hardware_init -> amdgpu_dm_init
+ * dc_set_power_state --> dm_resume
+ *
+ * therefore, this function apply to navi10/12/14 but not Renoir
+ * *
+ */
+ switch(adev->asic_type) {
+ case CHIP_NAVI10:
+ case CHIP_NAVI14:
+ case CHIP_NAVI12:
+ break;
+ default:
+ return 0;
+ }
+
+ /* pass data to smu controller */
+ if ((smu->watermarks_bitmap & WATERMARKS_EXIST) &&
+ !(smu->watermarks_bitmap & WATERMARKS_LOADED)) {
+ ret = smu_write_watermarks_table(smu);
+
+ if (ret) {
+ DRM_ERROR("Failed to update WMTABLE!\n");
+ return ret;
+ }
+ smu->watermarks_bitmap |= WATERMARKS_LOADED;
+ }
+
+ return 0;
+}
+
/**
* dm_hw_init() - Initialize DC device
* @handle: The base driver device containing the amdgpu_dm device.
@@ -1713,6 +1775,8 @@ static int dm_resume(void *handle)
amdgpu_dm_irq_resume_late(adev);
+ amdgpu_dm_smu_write_watermarks_table(adev);
+
return 0;
}
--
2.17.1
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] drm/amdgpu/display: navi1x copy dcn watermark clock settings to smu resume from s3
2020-02-28 2:58 ` Quan, Evan
@ 2020-02-28 20:59 ` Wu, Hersen
2020-03-02 14:26 ` Alex Deucher
0 siblings, 1 reply; 7+ messages in thread
From: Wu, Hersen @ 2020-02-28 20:59 UTC (permalink / raw)
To: Quan, Evan, amd-gfx; +Cc: Feng, Kenneth
[-- Attachment #1.1: Type: text/plain, Size: 8762 bytes --]
Follow Evan's review, add smu->mutex.
This interface is for dGPU Navi1x. Linux dc-pplib interface depends
on window driver dc implementation.
For Navi1x, clock settings of dcn watermarks are fixed. the settings
should be passed to smu during boot up and resume from s3.
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:
smu_set_watermarks_for_clock_ranges
smu_set_watermarks_table
navi10_set_watermarks_table
smu_write_watermarks_table
For Renoir, clock settings of dcn watermark are also fixed values.
dc has implemented different flow for window driver:
dc_hardware_init / dc_set_power_state
dcn10_init_hw
notify_wm_ranges
set_wm_ranges
For Linux
smu_set_watermarks_for_clock_ranges
renoir_set_watermarks_table
smu_write_watermarks_table
dc_hardware_init -> amdgpu_dm_init
dc_set_power_state --> dm_resume
therefore, linux dc-pplib interface of navi10/12/14 is different
from that of Renoir.
Signed-off-by: Hersen Wu <hersenxs.wu@amd.com>
---
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 68 +++++++++++++++++++
1 file changed, 68 insertions(+)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 931cbd7b372e..1ee1d6ff2782 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -1435,6 +1435,72 @@ static void s3_handle_mst(struct drm_device *dev, bool suspend)
drm_kms_helper_hotplug_event(dev);
}
+static int amdgpu_dm_smu_write_watermarks_table(struct amdgpu_device *adev)
+{
+ struct smu_context *smu = &adev->smu;
+ int ret = 0;
+
+ if (!is_support_sw_smu(adev))
+ return 0;
+
+ /* This interface is for dGPU Navi1x.Linux dc-pplib interface depends
+ * on window driver dc implementation.
+ * For Navi1x, clock settings of dcn watermarks are fixed. the settings
+ * should be passed to smu during boot up and resume from s3.
+ * 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:
+ * smu_set_watermarks_for_clock_ranges
+ * smu_set_watermarks_table
+ * navi10_set_watermarks_table
+ * smu_write_watermarks_table
+ *
+ * For Renoir, clock settings of dcn watermark are also fixed values.
+ * dc has implemented different flow for window driver:
+ * dc_hardware_init / dc_set_power_state
+ * dcn10_init_hw
+ * notify_wm_ranges
+ * set_wm_ranges
+ * -- Linux
+ * smu_set_watermarks_for_clock_ranges
+ * renoir_set_watermarks_table
+ * smu_write_watermarks_table
+ *
+ * For Linux,
+ * dc_hardware_init -> amdgpu_dm_init
+ * dc_set_power_state --> dm_resume
+ *
+ * therefore, this function apply to navi10/12/14 but not Renoir
+ * *
+ */
+ switch(adev->asic_type) {
+ case CHIP_NAVI10:
+ case CHIP_NAVI14:
+ case CHIP_NAVI12:
+ break;
+ default:
+ return 0;
+ }
+
+ mutex_lock(&smu->mutex);
+
+ /* pass data to smu controller */
+ if ((smu->watermarks_bitmap & WATERMARKS_EXIST) &&
+ !(smu->watermarks_bitmap & WATERMARKS_LOADED)) {
+ ret = smu_write_watermarks_table(smu);
+
+ if (ret) {
+ DRM_ERROR("Failed to update WMTABLE!\n");
+ return ret;
+ }
+ smu->watermarks_bitmap |= WATERMARKS_LOADED;
+ }
+
+ mutex_unlock(&smu->mutex);
+
+ return 0;
+}
+
/**
* dm_hw_init() - Initialize DC device
* @handle: The base driver device containing the amdgpu_dm device.
@@ -1713,6 +1779,8 @@ static int dm_resume(void *handle)
amdgpu_dm_irq_resume_late(adev);
+ amdgpu_dm_smu_write_watermarks_table(adev);
+
return 0;
}
--
2.17.1
________________________________
From: Quan, Evan <Evan.Quan@amd.com>
Sent: February 27, 2020 9:58 PM
To: Wu, Hersen <hersenxs.wu@amd.com>; amd-gfx@lists.freedesktop.org <amd-gfx@lists.freedesktop.org>
Cc: Feng, Kenneth <Kenneth.Feng@amd.com>; Wu, Hersen <hersenxs.wu@amd.com>
Subject: RE: [PATCH 2/2] drm/amdgpu/display: navi1x copy dcn watermark clock settings to smu resume from s3
Thanks. But could you help to confirm whether this is correctly protected by "mutex_lock(&smu->mutex)"?
-----Original Message-----
From: Hersen Wu <hersenxs.wu@amd.com>
Sent: Thursday, February 27, 2020 11:54 PM
To: amd-gfx@lists.freedesktop.org
Cc: Quan, Evan <Evan.Quan@amd.com>; Feng, Kenneth <Kenneth.Feng@amd.com>; Wu, Hersen <hersenxs.wu@amd.com>
Subject: [PATCH 2/2] drm/amdgpu/display: navi1x copy dcn watermark clock settings to smu resume from s3
This interface is for dGPU Navi1x. Linux dc-pplib interface depends on window driver dc implementation.
For Navi1x, clock settings of dcn watermarks are fixed. the settings should be passed to smu during boot up and resume from s3.
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:
smu_set_watermarks_for_clock_ranges
smu_set_watermarks_table
navi10_set_watermarks_table
smu_write_watermarks_table
For Renoir, clock settings of dcn watermark are also fixed values.
dc has implemented different flow for window driver:
dc_hardware_init / dc_set_power_state
dcn10_init_hw
notify_wm_ranges
set_wm_ranges
For Linux
smu_set_watermarks_for_clock_ranges
renoir_set_watermarks_table
smu_write_watermarks_table
dc_hardware_init -> amdgpu_dm_init
dc_set_power_state --> dm_resume
therefore, linux dc-pplib interface of navi10/12/14 is different from that of Renoir.
Signed-off-by: Hersen Wu <hersenxs.wu@amd.com>
---
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 64 +++++++++++++++++++
1 file changed, 64 insertions(+)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 931cbd7b372e..c58c0e95735e 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -1435,6 +1435,68 @@ static void s3_handle_mst(struct drm_device *dev, bool suspend)
drm_kms_helper_hotplug_event(dev);
}
+static int amdgpu_dm_smu_write_watermarks_table(struct amdgpu_device
+*adev) {
+ struct smu_context *smu = &adev->smu;
+ int ret = 0;
+
+ if (!is_support_sw_smu(adev))
+ return 0;
+
+ /* This interface is for dGPU Navi1x.Linux dc-pplib interface depends
+ * on window driver dc implementation.
+ * For Navi1x, clock settings of dcn watermarks are fixed. the settings
+ * should be passed to smu during boot up and resume from s3.
+ * 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:
+ * smu_set_watermarks_for_clock_ranges
+ * smu_set_watermarks_table
+ * navi10_set_watermarks_table
+ * smu_write_watermarks_table
+ *
+ * For Renoir, clock settings of dcn watermark are also fixed values.
+ * dc has implemented different flow for window driver:
+ * dc_hardware_init / dc_set_power_state
+ * dcn10_init_hw
+ * notify_wm_ranges
+ * set_wm_ranges
+ * -- Linux
+ * smu_set_watermarks_for_clock_ranges
+ * renoir_set_watermarks_table
+ * smu_write_watermarks_table
+ *
+ * For Linux,
+ * dc_hardware_init -> amdgpu_dm_init
+ * dc_set_power_state --> dm_resume
+ *
+ * therefore, this function apply to navi10/12/14 but not Renoir
+ * *
+ */
+ switch(adev->asic_type) {
+ case CHIP_NAVI10:
+ case CHIP_NAVI14:
+ case CHIP_NAVI12:
+ break;
+ default:
+ return 0;
+ }
+
+ /* pass data to smu controller */
+ if ((smu->watermarks_bitmap & WATERMARKS_EXIST) &&
+ !(smu->watermarks_bitmap & WATERMARKS_LOADED)) {
+ ret = smu_write_watermarks_table(smu);
+
+ if (ret) {
+ DRM_ERROR("Failed to update WMTABLE!\n");
+ return ret;
+ }
+ smu->watermarks_bitmap |= WATERMARKS_LOADED;
+ }
+
+ return 0;
+}
+
/**
* dm_hw_init() - Initialize DC device
* @handle: The base driver device containing the amdgpu_dm device.
@@ -1713,6 +1775,8 @@ static int dm_resume(void *handle)
amdgpu_dm_irq_resume_late(adev);
+ amdgpu_dm_smu_write_watermarks_table(adev);
+
return 0;
}
--
2.17.1
[-- Attachment #1.2: Type: text/html, Size: 16201 bytes --]
[-- Attachment #2: Type: text/plain, Size: 154 bytes --]
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] drm/amdgpu/display: navi1x copy dcn watermark clock settings to smu resume from s3
2020-02-28 20:59 ` Wu, Hersen
@ 2020-03-02 14:26 ` Alex Deucher
2020-03-03 13:19 ` Wu, Hersen
0 siblings, 1 reply; 7+ messages in thread
From: Alex Deucher @ 2020-03-02 14:26 UTC (permalink / raw)
To: Wu, Hersen; +Cc: Quan, Evan, Feng, Kenneth, amd-gfx
On Fri, Feb 28, 2020 at 3:59 PM Wu, Hersen <hersenxs.wu@amd.com> wrote:
>
> Follow Evan's review, add smu->mutex.
>
>
> This interface is for dGPU Navi1x. Linux dc-pplib interface depends
> on window driver dc implementation.
>
> For Navi1x, clock settings of dcn watermarks are fixed. the settings
> should be passed to smu during boot up and resume from s3.
> 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:
> smu_set_watermarks_for_clock_ranges
> smu_set_watermarks_table
> navi10_set_watermarks_table
> smu_write_watermarks_table
>
> For Renoir, clock settings of dcn watermark are also fixed values.
> dc has implemented different flow for window driver:
> dc_hardware_init / dc_set_power_state
> dcn10_init_hw
> notify_wm_ranges
> set_wm_ranges
>
> For Linux
> smu_set_watermarks_for_clock_ranges
> renoir_set_watermarks_table
> smu_write_watermarks_table
>
> dc_hardware_init -> amdgpu_dm_init
> dc_set_power_state --> dm_resume
>
> therefore, linux dc-pplib interface of navi10/12/14 is different
> from that of Renoir.
>
> Signed-off-by: Hersen Wu <hersenxs.wu@amd.com>
> ---
> .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 68 +++++++++++++++++++
> 1 file changed, 68 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> index 931cbd7b372e..1ee1d6ff2782 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -1435,6 +1435,72 @@ static void s3_handle_mst(struct drm_device *dev, bool suspend)
> drm_kms_helper_hotplug_event(dev);
> }
>
> +static int amdgpu_dm_smu_write_watermarks_table(struct amdgpu_device *adev)
> +{
> + struct smu_context *smu = &adev->smu;
> + int ret = 0;
> +
> + if (!is_support_sw_smu(adev))
> + return 0;
> +
> + /* This interface is for dGPU Navi1x.Linux dc-pplib interface depends
> + * on window driver dc implementation.
> + * For Navi1x, clock settings of dcn watermarks are fixed. the settings
> + * should be passed to smu during boot up and resume from s3.
> + * 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:
> + * smu_set_watermarks_for_clock_ranges
> + * smu_set_watermarks_table
> + * navi10_set_watermarks_table
> + * smu_write_watermarks_table
> + *
> + * For Renoir, clock settings of dcn watermark are also fixed values.
> + * dc has implemented different flow for window driver:
> + * dc_hardware_init / dc_set_power_state
> + * dcn10_init_hw
> + * notify_wm_ranges
> + * set_wm_ranges
> + * -- Linux
> + * smu_set_watermarks_for_clock_ranges
> + * renoir_set_watermarks_table
> + * smu_write_watermarks_table
> + *
> + * For Linux,
> + * dc_hardware_init -> amdgpu_dm_init
> + * dc_set_power_state --> dm_resume
> + *
> + * therefore, this function apply to navi10/12/14 but not Renoir
> + * *
> + */
> + switch(adev->asic_type) {
> + case CHIP_NAVI10:
> + case CHIP_NAVI14:
> + case CHIP_NAVI12:
> + break;
> + default:
> + return 0;
> + }
> +
> + mutex_lock(&smu->mutex);
> +
> + /* pass data to smu controller */
> + if ((smu->watermarks_bitmap & WATERMARKS_EXIST) &&
> + !(smu->watermarks_bitmap & WATERMARKS_LOADED)) {
> + ret = smu_write_watermarks_table(smu);
> +
> + if (ret) {
> + DRM_ERROR("Failed to update WMTABLE!\n");
> + return ret;
You need to unlock the mutex here in the failure case.
Alex
> + }
> + smu->watermarks_bitmap |= WATERMARKS_LOADED;
> + }
> +
> + mutex_unlock(&smu->mutex);
> +
> + return 0;
> +}
> +
> /**
> * dm_hw_init() - Initialize DC device
> * @handle: The base driver device containing the amdgpu_dm device.
> @@ -1713,6 +1779,8 @@ static int dm_resume(void *handle)
>
> amdgpu_dm_irq_resume_late(adev);
>
> + amdgpu_dm_smu_write_watermarks_table(adev);
> +
> return 0;
> }
>
> --
> 2.17.1
>
> ________________________________
> From: Quan, Evan <Evan.Quan@amd.com>
> Sent: February 27, 2020 9:58 PM
> To: Wu, Hersen <hersenxs.wu@amd.com>; amd-gfx@lists.freedesktop.org <amd-gfx@lists.freedesktop.org>
> Cc: Feng, Kenneth <Kenneth.Feng@amd.com>; Wu, Hersen <hersenxs.wu@amd.com>
> Subject: RE: [PATCH 2/2] drm/amdgpu/display: navi1x copy dcn watermark clock settings to smu resume from s3
>
> Thanks. But could you help to confirm whether this is correctly protected by "mutex_lock(&smu->mutex)"?
>
> -----Original Message-----
> From: Hersen Wu <hersenxs.wu@amd.com>
> Sent: Thursday, February 27, 2020 11:54 PM
> To: amd-gfx@lists.freedesktop.org
> Cc: Quan, Evan <Evan.Quan@amd.com>; Feng, Kenneth <Kenneth.Feng@amd.com>; Wu, Hersen <hersenxs.wu@amd.com>
> Subject: [PATCH 2/2] drm/amdgpu/display: navi1x copy dcn watermark clock settings to smu resume from s3
>
> This interface is for dGPU Navi1x. Linux dc-pplib interface depends on window driver dc implementation.
>
> For Navi1x, clock settings of dcn watermarks are fixed. the settings should be passed to smu during boot up and resume from s3.
> 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:
> smu_set_watermarks_for_clock_ranges
> smu_set_watermarks_table
> navi10_set_watermarks_table
> smu_write_watermarks_table
>
> For Renoir, clock settings of dcn watermark are also fixed values.
> dc has implemented different flow for window driver:
> dc_hardware_init / dc_set_power_state
> dcn10_init_hw
> notify_wm_ranges
> set_wm_ranges
>
> For Linux
> smu_set_watermarks_for_clock_ranges
> renoir_set_watermarks_table
> smu_write_watermarks_table
>
> dc_hardware_init -> amdgpu_dm_init
> dc_set_power_state --> dm_resume
>
> therefore, linux dc-pplib interface of navi10/12/14 is different from that of Renoir.
>
> Signed-off-by: Hersen Wu <hersenxs.wu@amd.com>
> ---
> .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 64 +++++++++++++++++++
> 1 file changed, 64 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> index 931cbd7b372e..c58c0e95735e 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -1435,6 +1435,68 @@ static void s3_handle_mst(struct drm_device *dev, bool suspend)
> drm_kms_helper_hotplug_event(dev);
> }
>
> +static int amdgpu_dm_smu_write_watermarks_table(struct amdgpu_device
> +*adev) {
> + struct smu_context *smu = &adev->smu;
> + int ret = 0;
> +
> + if (!is_support_sw_smu(adev))
> + return 0;
> +
> + /* This interface is for dGPU Navi1x.Linux dc-pplib interface depends
> + * on window driver dc implementation.
> + * For Navi1x, clock settings of dcn watermarks are fixed. the settings
> + * should be passed to smu during boot up and resume from s3.
> + * 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:
> + * smu_set_watermarks_for_clock_ranges
> + * smu_set_watermarks_table
> + * navi10_set_watermarks_table
> + * smu_write_watermarks_table
> + *
> + * For Renoir, clock settings of dcn watermark are also fixed values.
> + * dc has implemented different flow for window driver:
> + * dc_hardware_init / dc_set_power_state
> + * dcn10_init_hw
> + * notify_wm_ranges
> + * set_wm_ranges
> + * -- Linux
> + * smu_set_watermarks_for_clock_ranges
> + * renoir_set_watermarks_table
> + * smu_write_watermarks_table
> + *
> + * For Linux,
> + * dc_hardware_init -> amdgpu_dm_init
> + * dc_set_power_state --> dm_resume
> + *
> + * therefore, this function apply to navi10/12/14 but not Renoir
> + * *
> + */
> + switch(adev->asic_type) {
> + case CHIP_NAVI10:
> + case CHIP_NAVI14:
> + case CHIP_NAVI12:
> + break;
> + default:
> + return 0;
> + }
> +
> + /* pass data to smu controller */
> + if ((smu->watermarks_bitmap & WATERMARKS_EXIST) &&
> + !(smu->watermarks_bitmap & WATERMARKS_LOADED)) {
> + ret = smu_write_watermarks_table(smu);
> +
> + if (ret) {
> + DRM_ERROR("Failed to update WMTABLE!\n");
> + return ret;
> + }
> + smu->watermarks_bitmap |= WATERMARKS_LOADED;
> + }
> +
> + return 0;
> +}
> +
> /**
> * dm_hw_init() - Initialize DC device
> * @handle: The base driver device containing the amdgpu_dm device.
> @@ -1713,6 +1775,8 @@ static int dm_resume(void *handle)
>
> amdgpu_dm_irq_resume_late(adev);
>
> + amdgpu_dm_smu_write_watermarks_table(adev);
> +
> return 0;
> }
>
> --
> 2.17.1
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 7+ messages in thread
* RE: [PATCH 2/2] drm/amdgpu/display: navi1x copy dcn watermark clock settings to smu resume from s3
2020-03-02 14:26 ` Alex Deucher
@ 2020-03-03 13:19 ` Wu, Hersen
2020-03-04 3:04 ` Quan, Evan
0 siblings, 1 reply; 7+ messages in thread
From: Wu, Hersen @ 2020-03-03 13:19 UTC (permalink / raw)
To: Alex Deucher; +Cc: Quan, Evan, Feng, Kenneth, amd-gfx
[AMD Official Use Only - Internal Distribution Only]
Hi Evan, Kenneth,
Would you please help review this patch again?
Thanks!
Hersen
-----Original Message-----
From: Alex Deucher <alexdeucher@gmail.com>
Sent: Monday, March 2, 2020 9:27 AM
To: Wu, Hersen <hersenxs.wu@amd.com>
Cc: Quan, Evan <Evan.Quan@amd.com>; amd-gfx@lists.freedesktop.org; Feng, Kenneth <Kenneth.Feng@amd.com>
Subject: Re: [PATCH 2/2] drm/amdgpu/display: navi1x copy dcn watermark clock settings to smu resume from s3
On Fri, Feb 28, 2020 at 3:59 PM Wu, Hersen <hersenxs.wu@amd.com> wrote:
>
> Follow Evan's review, add smu->mutex.
>
>
> This interface is for dGPU Navi1x. Linux dc-pplib interface depends
> on window driver dc implementation.
>
> For Navi1x, clock settings of dcn watermarks are fixed. the settings
> should be passed to smu during boot up and resume from s3.
> 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:
> smu_set_watermarks_for_clock_ranges
> smu_set_watermarks_table
> navi10_set_watermarks_table
> smu_write_watermarks_table
>
> For Renoir, clock settings of dcn watermark are also fixed values.
> dc has implemented different flow for window driver:
> dc_hardware_init / dc_set_power_state dcn10_init_hw
> notify_wm_ranges set_wm_ranges
>
> For Linux
> smu_set_watermarks_for_clock_ranges
> renoir_set_watermarks_table
> smu_write_watermarks_table
>
> dc_hardware_init -> amdgpu_dm_init
> dc_set_power_state --> dm_resume
>
> therefore, linux dc-pplib interface of navi10/12/14 is different
> from that of Renoir.
>
> Signed-off-by: Hersen Wu <hersenxs.wu@amd.com>
> ---
> .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 68
> +++++++++++++++++++
> 1 file changed, 68 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> index 931cbd7b372e..1ee1d6ff2782 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -1435,6 +1435,72 @@ static void s3_handle_mst(struct drm_device *dev, bool suspend)
> drm_kms_helper_hotplug_event(dev);
> }
>
> +static int amdgpu_dm_smu_write_watermarks_table(struct amdgpu_device
> +*adev) { struct smu_context *smu = &adev->smu; int ret = 0;
> +
> + if (!is_support_sw_smu(adev))
> + return 0;
> +
> + /* This interface is for dGPU Navi1x.Linux dc-pplib interface
> + depends
> + * on window driver dc implementation.
> + * For Navi1x, clock settings of dcn watermarks are fixed. the
> + settings
> + * should be passed to smu during boot up and resume from s3.
> + * 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:
> + * smu_set_watermarks_for_clock_ranges
> + * smu_set_watermarks_table
> + * navi10_set_watermarks_table
> + * smu_write_watermarks_table
> + *
> + * For Renoir, clock settings of dcn watermark are also fixed values.
> + * dc has implemented different flow for window driver:
> + * dc_hardware_init / dc_set_power_state
> + * dcn10_init_hw
> + * notify_wm_ranges
> + * set_wm_ranges
> + * -- Linux
> + * smu_set_watermarks_for_clock_ranges
> + * renoir_set_watermarks_table
> + * smu_write_watermarks_table
> + *
> + * For Linux,
> + * dc_hardware_init -> amdgpu_dm_init
> + * dc_set_power_state --> dm_resume
> + *
> + * therefore, this function apply to navi10/12/14 but not Renoir
> + * *
> + */
> + switch(adev->asic_type) {
> + case CHIP_NAVI10:
> + case CHIP_NAVI14:
> + case CHIP_NAVI12:
> + break;
> + default:
> + return 0;
> + }
> +
> + mutex_lock(&smu->mutex);
> +
> + /* pass data to smu controller */
> + if ((smu->watermarks_bitmap & WATERMARKS_EXIST) &&
> + !(smu->watermarks_bitmap & WATERMARKS_LOADED)) { ret =
> + smu_write_watermarks_table(smu);
> +
> + if (ret) {
> + DRM_ERROR("Failed to update WMTABLE!\n"); return ret;
You need to unlock the mutex here in the failure case.
Alex
> + }
> + smu->watermarks_bitmap |= WATERMARKS_LOADED;
> + }
> +
> + mutex_unlock(&smu->mutex);
> +
> + return 0;
> +}
> +
> /**
> * dm_hw_init() - Initialize DC device
> * @handle: The base driver device containing the amdgpu_dm device.
> @@ -1713,6 +1779,8 @@ static int dm_resume(void *handle)
>
> amdgpu_dm_irq_resume_late(adev);
>
> + amdgpu_dm_smu_write_watermarks_table(adev);
> +
> return 0;
> }
>
> --
> 2.17.1
>
> ________________________________
> From: Quan, Evan <Evan.Quan@amd.com>
> Sent: February 27, 2020 9:58 PM
> To: Wu, Hersen <hersenxs.wu@amd.com>; amd-gfx@lists.freedesktop.org
> <amd-gfx@lists.freedesktop.org>
> Cc: Feng, Kenneth <Kenneth.Feng@amd.com>; Wu, Hersen
> <hersenxs.wu@amd.com>
> Subject: RE: [PATCH 2/2] drm/amdgpu/display: navi1x copy dcn watermark
> clock settings to smu resume from s3
>
> Thanks. But could you help to confirm whether this is correctly protected by "mutex_lock(&smu->mutex)"?
>
> -----Original Message-----
> From: Hersen Wu <hersenxs.wu@amd.com>
> Sent: Thursday, February 27, 2020 11:54 PM
> To: amd-gfx@lists.freedesktop.org
> Cc: Quan, Evan <Evan.Quan@amd.com>; Feng, Kenneth
> <Kenneth.Feng@amd.com>; Wu, Hersen <hersenxs.wu@amd.com>
> Subject: [PATCH 2/2] drm/amdgpu/display: navi1x copy dcn watermark
> clock settings to smu resume from s3
>
> This interface is for dGPU Navi1x. Linux dc-pplib interface depends on window driver dc implementation.
>
> For Navi1x, clock settings of dcn watermarks are fixed. the settings should be passed to smu during boot up and resume from s3.
> 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:
> smu_set_watermarks_for_clock_ranges
> smu_set_watermarks_table
> navi10_set_watermarks_table
> smu_write_watermarks_table
>
> For Renoir, clock settings of dcn watermark are also fixed values.
> dc has implemented different flow for window driver:
> dc_hardware_init / dc_set_power_state dcn10_init_hw
> notify_wm_ranges set_wm_ranges
>
> For Linux
> smu_set_watermarks_for_clock_ranges
> renoir_set_watermarks_table
> smu_write_watermarks_table
>
> dc_hardware_init -> amdgpu_dm_init
> dc_set_power_state --> dm_resume
>
> therefore, linux dc-pplib interface of navi10/12/14 is different from that of Renoir.
>
> Signed-off-by: Hersen Wu <hersenxs.wu@amd.com>
> ---
> .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 64
> +++++++++++++++++++
> 1 file changed, 64 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> index 931cbd7b372e..c58c0e95735e 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -1435,6 +1435,68 @@ static void s3_handle_mst(struct drm_device *dev, bool suspend)
> drm_kms_helper_hotplug_event(dev);
> }
>
> +static int amdgpu_dm_smu_write_watermarks_table(struct amdgpu_device
> +*adev) {
> + struct smu_context *smu = &adev->smu;
> + int ret = 0;
> +
> + if (!is_support_sw_smu(adev))
> + return 0;
> +
> + /* This interface is for dGPU Navi1x.Linux dc-pplib interface depends
> + * on window driver dc implementation.
> + * For Navi1x, clock settings of dcn watermarks are fixed. the settings
> + * should be passed to smu during boot up and resume from s3.
> + * 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:
> + * smu_set_watermarks_for_clock_ranges
> + * smu_set_watermarks_table
> + * navi10_set_watermarks_table
> + * smu_write_watermarks_table
> + *
> + * For Renoir, clock settings of dcn watermark are also fixed values.
> + * dc has implemented different flow for window driver:
> + * dc_hardware_init / dc_set_power_state
> + * dcn10_init_hw
> + * notify_wm_ranges
> + * set_wm_ranges
> + * -- Linux
> + * smu_set_watermarks_for_clock_ranges
> + * renoir_set_watermarks_table
> + * smu_write_watermarks_table
> + *
> + * For Linux,
> + * dc_hardware_init -> amdgpu_dm_init
> + * dc_set_power_state --> dm_resume
> + *
> + * therefore, this function apply to navi10/12/14 but not Renoir
> + * *
> + */
> + switch(adev->asic_type) {
> + case CHIP_NAVI10:
> + case CHIP_NAVI14:
> + case CHIP_NAVI12:
> + break;
> + default:
> + return 0;
> + }
> +
> + /* pass data to smu controller */
> + if ((smu->watermarks_bitmap & WATERMARKS_EXIST) &&
> + !(smu->watermarks_bitmap & WATERMARKS_LOADED)) {
> + ret = smu_write_watermarks_table(smu);
> +
> + if (ret) {
> + DRM_ERROR("Failed to update WMTABLE!\n");
> + return ret;
> + }
> + smu->watermarks_bitmap |= WATERMARKS_LOADED;
> + }
> +
> + return 0;
> +}
> +
> /**
> * dm_hw_init() - Initialize DC device
> * @handle: The base driver device containing the amdgpu_dm device.
> @@ -1713,6 +1775,8 @@ static int dm_resume(void *handle)
>
> amdgpu_dm_irq_resume_late(adev);
>
> + amdgpu_dm_smu_write_watermarks_table(adev);
> +
> return 0;
> }
>
> --
> 2.17.1
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flist
> s.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&data=02%7C01%7Che
> rsenxs.wu%40amd.com%7C4709bedf60fc41d1ae0508d7beb5c120%7C3dd8961fe4884
> e608e11a82d994e183d%7C0%7C0%7C637187560164248086&sdata=6wH59U4RtXQ
> lL6Z2EcIKQWvBNfV0shDAUD05ARVs2MA%3D&reserved=0
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 7+ messages in thread
* RE: [PATCH 2/2] drm/amdgpu/display: navi1x copy dcn watermark clock settings to smu resume from s3
2020-03-03 13:19 ` Wu, Hersen
@ 2020-03-04 3:04 ` Quan, Evan
0 siblings, 0 replies; 7+ messages in thread
From: Quan, Evan @ 2020-03-04 3:04 UTC (permalink / raw)
To: Wu, Hersen, Alex Deucher; +Cc: Feng, Kenneth, amd-gfx
Sorry for miss this. With the Alex's comment addressed, the patch is reviewed-by: Evan Quan <evan.quan@amd.com>
-----Original Message-----
From: Wu, Hersen <hersenxs.wu@amd.com>
Sent: Tuesday, March 3, 2020 9:19 PM
To: Alex Deucher <alexdeucher@gmail.com>
Cc: Quan, Evan <Evan.Quan@amd.com>; amd-gfx@lists.freedesktop.org; Feng, Kenneth <Kenneth.Feng@amd.com>
Subject: RE: [PATCH 2/2] drm/amdgpu/display: navi1x copy dcn watermark clock settings to smu resume from s3
[AMD Official Use Only - Internal Distribution Only]
Hi Evan, Kenneth,
Would you please help review this patch again?
Thanks!
Hersen
-----Original Message-----
From: Alex Deucher <alexdeucher@gmail.com>
Sent: Monday, March 2, 2020 9:27 AM
To: Wu, Hersen <hersenxs.wu@amd.com>
Cc: Quan, Evan <Evan.Quan@amd.com>; amd-gfx@lists.freedesktop.org; Feng, Kenneth <Kenneth.Feng@amd.com>
Subject: Re: [PATCH 2/2] drm/amdgpu/display: navi1x copy dcn watermark clock settings to smu resume from s3
On Fri, Feb 28, 2020 at 3:59 PM Wu, Hersen <hersenxs.wu@amd.com> wrote:
>
> Follow Evan's review, add smu->mutex.
>
>
> This interface is for dGPU Navi1x. Linux dc-pplib interface depends on
> window driver dc implementation.
>
> For Navi1x, clock settings of dcn watermarks are fixed. the settings
> should be passed to smu during boot up and resume from s3.
> 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:
> smu_set_watermarks_for_clock_ranges
> smu_set_watermarks_table
> navi10_set_watermarks_table
> smu_write_watermarks_table
>
> For Renoir, clock settings of dcn watermark are also fixed values.
> dc has implemented different flow for window driver:
> dc_hardware_init / dc_set_power_state dcn10_init_hw notify_wm_ranges
> set_wm_ranges
>
> For Linux
> smu_set_watermarks_for_clock_ranges
> renoir_set_watermarks_table
> smu_write_watermarks_table
>
> dc_hardware_init -> amdgpu_dm_init
> dc_set_power_state --> dm_resume
>
> therefore, linux dc-pplib interface of navi10/12/14 is different from
> that of Renoir.
>
> Signed-off-by: Hersen Wu <hersenxs.wu@amd.com>
> ---
> .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 68
> +++++++++++++++++++
> 1 file changed, 68 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> index 931cbd7b372e..1ee1d6ff2782 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -1435,6 +1435,72 @@ static void s3_handle_mst(struct drm_device *dev, bool suspend)
> drm_kms_helper_hotplug_event(dev);
> }
>
> +static int amdgpu_dm_smu_write_watermarks_table(struct amdgpu_device
> +*adev) { struct smu_context *smu = &adev->smu; int ret = 0;
> +
> + if (!is_support_sw_smu(adev))
> + return 0;
> +
> + /* This interface is for dGPU Navi1x.Linux dc-pplib interface
> + depends
> + * on window driver dc implementation.
> + * For Navi1x, clock settings of dcn watermarks are fixed. the
> + settings
> + * should be passed to smu during boot up and resume from s3.
> + * 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:
> + * smu_set_watermarks_for_clock_ranges
> + * smu_set_watermarks_table
> + * navi10_set_watermarks_table
> + * smu_write_watermarks_table
> + *
> + * For Renoir, clock settings of dcn watermark are also fixed values.
> + * dc has implemented different flow for window driver:
> + * dc_hardware_init / dc_set_power_state
> + * dcn10_init_hw
> + * notify_wm_ranges
> + * set_wm_ranges
> + * -- Linux
> + * smu_set_watermarks_for_clock_ranges
> + * renoir_set_watermarks_table
> + * smu_write_watermarks_table
> + *
> + * For Linux,
> + * dc_hardware_init -> amdgpu_dm_init
> + * dc_set_power_state --> dm_resume
> + *
> + * therefore, this function apply to navi10/12/14 but not Renoir
> + * *
> + */
> + switch(adev->asic_type) {
> + case CHIP_NAVI10:
> + case CHIP_NAVI14:
> + case CHIP_NAVI12:
> + break;
> + default:
> + return 0;
> + }
> +
> + mutex_lock(&smu->mutex);
> +
> + /* pass data to smu controller */
> + if ((smu->watermarks_bitmap & WATERMARKS_EXIST) &&
> + !(smu->watermarks_bitmap & WATERMARKS_LOADED)) { ret =
> + smu_write_watermarks_table(smu);
> +
> + if (ret) {
> + DRM_ERROR("Failed to update WMTABLE!\n"); return ret;
You need to unlock the mutex here in the failure case.
Alex
> + }
> + smu->watermarks_bitmap |= WATERMARKS_LOADED;
> + }
> +
> + mutex_unlock(&smu->mutex);
> +
> + return 0;
> +}
> +
> /**
> * dm_hw_init() - Initialize DC device
> * @handle: The base driver device containing the amdgpu_dm device.
> @@ -1713,6 +1779,8 @@ static int dm_resume(void *handle)
>
> amdgpu_dm_irq_resume_late(adev);
>
> + amdgpu_dm_smu_write_watermarks_table(adev);
> +
> return 0;
> }
>
> --
> 2.17.1
>
> ________________________________
> From: Quan, Evan <Evan.Quan@amd.com>
> Sent: February 27, 2020 9:58 PM
> To: Wu, Hersen <hersenxs.wu@amd.com>; amd-gfx@lists.freedesktop.org
> <amd-gfx@lists.freedesktop.org>
> Cc: Feng, Kenneth <Kenneth.Feng@amd.com>; Wu, Hersen
> <hersenxs.wu@amd.com>
> Subject: RE: [PATCH 2/2] drm/amdgpu/display: navi1x copy dcn watermark
> clock settings to smu resume from s3
>
> Thanks. But could you help to confirm whether this is correctly protected by "mutex_lock(&smu->mutex)"?
>
> -----Original Message-----
> From: Hersen Wu <hersenxs.wu@amd.com>
> Sent: Thursday, February 27, 2020 11:54 PM
> To: amd-gfx@lists.freedesktop.org
> Cc: Quan, Evan <Evan.Quan@amd.com>; Feng, Kenneth
> <Kenneth.Feng@amd.com>; Wu, Hersen <hersenxs.wu@amd.com>
> Subject: [PATCH 2/2] drm/amdgpu/display: navi1x copy dcn watermark
> clock settings to smu resume from s3
>
> This interface is for dGPU Navi1x. Linux dc-pplib interface depends on window driver dc implementation.
>
> For Navi1x, clock settings of dcn watermarks are fixed. the settings should be passed to smu during boot up and resume from s3.
> 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:
> smu_set_watermarks_for_clock_ranges
> smu_set_watermarks_table
> navi10_set_watermarks_table
> smu_write_watermarks_table
>
> For Renoir, clock settings of dcn watermark are also fixed values.
> dc has implemented different flow for window driver:
> dc_hardware_init / dc_set_power_state dcn10_init_hw notify_wm_ranges
> set_wm_ranges
>
> For Linux
> smu_set_watermarks_for_clock_ranges
> renoir_set_watermarks_table
> smu_write_watermarks_table
>
> dc_hardware_init -> amdgpu_dm_init
> dc_set_power_state --> dm_resume
>
> therefore, linux dc-pplib interface of navi10/12/14 is different from that of Renoir.
>
> Signed-off-by: Hersen Wu <hersenxs.wu@amd.com>
> ---
> .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 64
> +++++++++++++++++++
> 1 file changed, 64 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> index 931cbd7b372e..c58c0e95735e 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -1435,6 +1435,68 @@ static void s3_handle_mst(struct drm_device *dev, bool suspend)
> drm_kms_helper_hotplug_event(dev);
> }
>
> +static int amdgpu_dm_smu_write_watermarks_table(struct amdgpu_device
> +*adev) {
> + struct smu_context *smu = &adev->smu;
> + int ret = 0;
> +
> + if (!is_support_sw_smu(adev))
> + return 0;
> +
> + /* This interface is for dGPU Navi1x.Linux dc-pplib interface depends
> + * on window driver dc implementation.
> + * For Navi1x, clock settings of dcn watermarks are fixed. the settings
> + * should be passed to smu during boot up and resume from s3.
> + * 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:
> + * smu_set_watermarks_for_clock_ranges
> + * smu_set_watermarks_table
> + * navi10_set_watermarks_table
> + * smu_write_watermarks_table
> + *
> + * For Renoir, clock settings of dcn watermark are also fixed values.
> + * dc has implemented different flow for window driver:
> + * dc_hardware_init / dc_set_power_state
> + * dcn10_init_hw
> + * notify_wm_ranges
> + * set_wm_ranges
> + * -- Linux
> + * smu_set_watermarks_for_clock_ranges
> + * renoir_set_watermarks_table
> + * smu_write_watermarks_table
> + *
> + * For Linux,
> + * dc_hardware_init -> amdgpu_dm_init
> + * dc_set_power_state --> dm_resume
> + *
> + * therefore, this function apply to navi10/12/14 but not Renoir
> + * *
> + */
> + switch(adev->asic_type) {
> + case CHIP_NAVI10:
> + case CHIP_NAVI14:
> + case CHIP_NAVI12:
> + break;
> + default:
> + return 0;
> + }
> +
> + /* pass data to smu controller */
> + if ((smu->watermarks_bitmap & WATERMARKS_EXIST) &&
> + !(smu->watermarks_bitmap & WATERMARKS_LOADED)) {
> + ret = smu_write_watermarks_table(smu);
> +
> + if (ret) {
> + DRM_ERROR("Failed to update WMTABLE!\n");
> + return ret;
> + }
> + smu->watermarks_bitmap |= WATERMARKS_LOADED;
> + }
> +
> + return 0;
> +}
> +
> /**
> * dm_hw_init() - Initialize DC device
> * @handle: The base driver device containing the amdgpu_dm device.
> @@ -1713,6 +1775,8 @@ static int dm_resume(void *handle)
>
> amdgpu_dm_irq_resume_late(adev);
>
> + amdgpu_dm_smu_write_watermarks_table(adev);
> +
> return 0;
> }
>
> --
> 2.17.1
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flist
> s.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&data=02%7C01%7Che
> rsenxs.wu%40amd.com%7C4709bedf60fc41d1ae0508d7beb5c120%7C3dd8961fe4884
> e608e11a82d994e183d%7C0%7C0%7C637187560164248086&sdata=6wH59U4RtXQ
> lL6Z2EcIKQWvBNfV0shDAUD05ARVs2MA%3D&reserved=0
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2020-03-04 3:04 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-27 15:53 [PATCH 2/2] drm/amdgpu/display: navi1x copy dcn watermark clock settings to smu resume from s3 Hersen Wu
2020-02-27 16:06 ` Alex Deucher
2020-02-28 2:58 ` Quan, Evan
2020-02-28 20:59 ` Wu, Hersen
2020-03-02 14:26 ` Alex Deucher
2020-03-03 13:19 ` Wu, Hersen
2020-03-04 3:04 ` Quan, Evan
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.