On 2018-09-21 10:48 AM, Zhu, Rex wrote:
-----Original Message-----
From: amd-gfx <amd-gfx-boun...@lists.freedesktop.org> On Behalf Of
James Zhu
Sent: Friday, September 21, 2018 10:22 PM
To: Zhu, Rex <rex....@amd.com>; Koenig, Christian
<christian.koe...@amd.com>; Zhu, James <james....@amd.com>; Alex
Deucher <alexdeuc...@gmail.com>
Cc: James Zhu <jzh...@gmail.com>; amd-gfx list <amd-
g...@lists.freedesktop.org>
Subject: Re: [PATCH v2] drm/amdgpu:No action when VCN PG state is
unchanged
On 2018-09-21 10:05 AM, Zhu, Rex wrote:
In my understand, when dpg mode enabled, we don't need to power
up/down VCN via SMU and Also don't need to set vcn power gate/ungate
state.
Just need to enable the dpg mode.
If not true, please correct me.
You are right. Under DPG mode, vcn_v1_0_start is used to setup DPG mode
instead of ungating power.
For code reuse purpose, the function/variable name may have different
connotation under different mode.
Did your patch base on tip drm-next code?
No based on amd-temp-pco. Since DPG mode is enabled only on PCO at this
moment.
James
Rex
James
Best Regards
Rex
-----Original Message-----
From: amd-gfx <amd-gfx-boun...@lists.freedesktop.org> On Behalf Of
Christian König
Sent: Friday, September 21, 2018 8:53 PM
To: Zhu, James <james....@amd.com>; Alex Deucher
<alexdeuc...@gmail.com>
Cc: James Zhu <jzh...@gmail.com>; Zhu, James <james....@amd.com>;
amd-gfx list <amd-gfx@lists.freedesktop.org>
Subject: Re: [PATCH v2] drm/amdgpu:No action when VCN PG state is
unchanged
Am 21.09.2018 um 14:28 schrieb James Zhu:
On 2018-09-20 01:54 PM, Christian König wrote:
Am 20.09.2018 um 18:24 schrieb James Zhu:
On 2018-09-20 11:49 AM, Alex Deucher wrote:
On Thu, Sep 20, 2018 at 11:27 AM James Zhu <jam...@amd.com>
wrote:
On 2018-09-20 11:14 AM, Alex Deucher wrote:
On Thu, Sep 13, 2018 at 4:56 PM James Zhu <jzh...@gmail.com>
wrote:
When VCN PG state is unchanged, it is unnecessary to reset
power gate state
Signed-off-by: James Zhu <james....@amd.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h | 1 +
drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c | 12 ++++++++++--
2 files changed, 11 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
index 0b0b863..d2219ab 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
@@ -69,6 +69,7 @@ struct amdgpu_vcn {
struct amdgpu_ring ring_jpeg;
struct amdgpu_irq_src irq;
unsigned num_enc_rings;
+ enum amd_powergating_state cur_state;
Does the default value (0) at init time properly reflect the
default powergating state? If so,
Acked-by: Alex Deucher <alexander.deuc...@amd.com>
Yes, the below code shows it will be set to 0 during driver load
stage.
Yes, I understand that. Is 0 (AMD_PG_STATE_GATE) what we want
as
the default though? The first time the code runs are we going to
do the right thing or is the code going to return early? IIRC,
the hw default is ungated.
cur_state is used for tracking driver SW PG state, not HW PG state.
I though no matter what HW PG state is after device powers up,
when first vcn ring is scheduled to run,
begin_use->set_powergating_state->vcn_v1_0_start->ungate
power/clock->Boot_VCPU will be tried.
For DPG mode, the ungate power/clock , boot VCPU will not actually
be activated during start setup stage, and only be activated
during ring run stage.
Mhm, I wonder if it wouldn't be better to have that functionality
one layer up.
E.g. in amdgpu_device_ip_set_powergating_state so that it applies
to all IP blocks in the same way.
If necessary, I can add support for IP blocks later.
Yeah, agree. In general I'm not doing much with power management, so
can't judge if that is a good idea or not.
Anyway feel free to add my Acked-by to the patch.
Regards,
Christian.
James
But on the other hand the correct solution looks good to me as
well, so feel free to add my Acked-by as well.
Christian.
James
Alex
int amdgpu_driver_load_kms(struct drm_device *dev, unsigned
long
flags)
....
adev = kzalloc(sizeof(struct amdgpu_device), GFP_KERNEL);
struct amdgpu_device {
....
struct amdgpu_vcn vcn;
Best Regards!
James zhu
};
int amdgpu_vcn_sw_init(struct amdgpu_device *adev); diff
--git a/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
b/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
index 2664bb2..2cde0b4 100644
--- a/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
@@ -1633,12 +1633,20 @@ static int
vcn_v1_0_set_powergating_state(void *handle,
* revisit this when there is a cleaner line
between
* the smc and the hw blocks
*/
+ int ret;
struct amdgpu_device *adev = (struct amdgpu_device
*)handle;
+ if(state == adev->vcn.cur_state)
+ return 0;
+
if (state == AMD_PG_STATE_GATE)
- return vcn_v1_0_stop(adev);
+ ret = vcn_v1_0_stop(adev);
else
- return vcn_v1_0_start(adev);
+ ret = vcn_v1_0_start(adev);
+
+ if(!ret)
+ adev->vcn.cur_state = state;
+ return ret;
}
static const struct amd_ip_funcs vcn_v1_0_ip_funcs = {
--
2.7.4
_______________________________________________
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
_______________________________________________
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
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
