Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
In-Reply-To: <20180327095907.GA18146@arm.com>
References: <1522083800-30100-1-git-send-email-ayan.halder@arm.com>
 <1522083800-30100-9-git-send-email-ayan.halder@arm.com> <20180327082903.GS14155@phenom.ffwll.local>
 <20180327095907.GA18146@arm.com>
From:   Daniel Vetter <daniel@ffwll.ch>
Date:   Tue, 27 Mar 2018 13:09:36 +0200
Message-ID: <CAKMK7uF8mmLOBQtwfQgkdpPHfoy7yM9UApWgc9YiEudoP5WEyw@mail.gmail.com>
Subject: Re: [PATCH 8/8] drm/arm/malidp: Added the late system pm functions
To:     Ayan Halder <ayan.halder@arm.com>
Cc:     Liviu Dudau <liviu.dudau@arm.com>,
        Brian Starkey <brian.starkey@arm.com>,
        Mali DP Maintainers <malidp@foss.arm.com>,
        Dave Airlie <airlied@linux.ie>,
        dri-devel <dri-devel@lists.freedesktop.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        nd <nd@arm.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Tue, Mar 27, 2018 at 11:59 AM, Ayan Halder <ayan.halder@arm.com> wrote:
> On Tue, Mar 27, 2018 at 10:29:03AM +0200, Daniel Vetter wrote:
>> On Mon, Mar 26, 2018 at 06:03:20PM +0100, Ayan Kumar Halder wrote:
>> > malidp_pm_suspend_late checks if the runtime status is not suspended
>> > and if so, invokes malidp_runtime_pm_suspend which disables the
>> > display engine/core interrupts and the clocks. It sets the runtime sta=
tus
>> > as suspended. Subsequently, malidp_pm_resume_early will invoke
>> > malidp_runtime_pm_resume which enables the clocks and the interrupts
>> > (previously disabled) and sets the runtime status as active.
>> >
>> > Signed-off-by: Ayan Kumar Halder <ayan.halder@arm.com>
>> > Change-Id: I5f8c3d28f076314a1c9da2a46760a9c37039ccda
>>
>> Why exactly do you need late/early hooks? If you have dependencies with
>> other devices, pls consider adding device_links instead. This here
>> shouldn't be necessary.
>> -Daniel
> We need to late/early hooks to disable malidp interrupts and the
> clocks.

Yes, but why this ordering constraint? Why can't you just disable the
interrupts/clocks in the normal suspend code. I see that the patch
does this, I want to understand why it does it.
-Daniel

>> > ---
>> >  drivers/gpu/drm/arm/malidp_drv.c | 17 +++++++++++++++++
>> >  1 file changed, 17 insertions(+)
>> >
>> > diff --git a/drivers/gpu/drm/arm/malidp_drv.c b/drivers/gpu/drm/arm/ma=
lidp_drv.c
>> > index bd44a6d..f6124d8 100644
>> > --- a/drivers/gpu/drm/arm/malidp_drv.c
>> > +++ b/drivers/gpu/drm/arm/malidp_drv.c
>> > @@ -766,8 +766,25 @@ static int __maybe_unused malidp_pm_resume(struct=
 device *dev)
>> >     return 0;
>> >  }
>> >
>> > +static int __maybe_unused malidp_pm_suspend_late(struct device *dev)
>> > +{
>> > +   if (!pm_runtime_status_suspended(dev)) {
>> > +           malidp_runtime_pm_suspend(dev);
>> > +           pm_runtime_set_suspended(dev);
>> > +   }
>> > +   return 0;
>> > +}
>> > +
>> > +static int __maybe_unused malidp_pm_resume_early(struct device *dev)
>> > +{
>> > +   malidp_runtime_pm_resume(dev);
>> > +   pm_runtime_set_active(dev);
>> > +   return 0;
>> > +}
>> > +
>> >  static const struct dev_pm_ops malidp_pm_ops =3D {
>> >     SET_SYSTEM_SLEEP_PM_OPS(malidp_pm_suspend, malidp_pm_resume) \
>> > +   SET_LATE_SYSTEM_SLEEP_PM_OPS(malidp_pm_suspend_late, malidp_pm_res=
ume_early) \
>> >     SET_RUNTIME_PM_OPS(malidp_runtime_pm_suspend, malidp_runtime_pm_re=
sume, NULL)
>> >  };
>> >
>> > --
>> > 2.7.4
>> >
>> > _______________________________________________
>> > dri-devel mailing list
>> > dri-devel@lists.freedesktop.org
>> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
>>
>> --
>> Daniel Vetter
>> Software Engineer, Intel Corporation
>> http://blog.ffwll.ch
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> IMPORTANT NOTICE: The contents of this email and any attachments are conf=
idential and may also be privileged. If you are not the intended recipient,=
 please notify the sender immediately and do not disclose the contents to a=
ny other person, use it for any purpose, or store or copy the information i=
n any medium. Thank you.
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel


--=20
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch