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
