From: Doug Anderson <dianders@chromium.org>
To: Kalyan Thota <kalyan_t@codeaurora.org>
Cc: dri-devel <dri-devel@lists.freedesktop.org>,
linux-arm-msm <linux-arm-msm@vger.kernel.org>,
freedreno <freedreno@lists.freedesktop.org>,
"open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS"
<devicetree@vger.kernel.org>, LKML <linux-kernel@vger.kernel.org>,
Rob Clark <robdclark@gmail.com>,
Sean Paul <seanpaul@chromium.org>,
"Kristian H. Kristensen" <hoegsberg@chromium.org>,
Jeykumar Sankaran <jsanka@codeaurora.org>,
mkrishn@codeaurora.org, travitej@codeaurora.org,
nganji@codeaurora.org
Subject: Re: [PATCH v6] drm/msm/dpu: ensure device suspend happens during PM sleep
Date: Thu, 4 Jun 2020 13:34:05 -0700 [thread overview]
Message-ID: <CAD=FV=V7ce7EGqJh6aga4yH3NqdgXMHBe=EOONtcd2LFDX75_A@mail.gmail.com> (raw)
In-Reply-To: <1591276775-13949-1-git-send-email-kalyan_t@codeaurora.org>
Hi,
On Thu, Jun 4, 2020 at 6:20 AM Kalyan Thota <kalyan_t@codeaurora.org> wrote:
>
> -#ifdef CONFIG_PM
> -static int msm_runtime_suspend(struct device *dev)
> +#ifdef CONFIG_PM_SLEEP
> +static int msm_pm_suspend(struct device *dev)
> {
> - struct drm_device *ddev = dev_get_drvdata(dev);
> - struct msm_drm_private *priv = ddev->dev_private;
> - struct msm_mdss *mdss = priv->mdss;
>
nit: remove blank line at the start of this function
> static const struct dev_pm_ops msm_pm_ops = {
> SET_SYSTEM_SLEEP_PM_OPS(msm_pm_suspend, msm_pm_resume)
> SET_RUNTIME_PM_OPS(msm_runtime_suspend, msm_runtime_resume, NULL)
> + .prepare = msm_pm_prepare,
> + .complete = msm_pm_complete,
Presumably you will get a compile failure if someone compiles without
CONFIG_PM_SLEEP since msm_pm_prepare() and msm_pm_complete() won't be
defined but you refer to them unconditionally. Probably the best
solution is to just add "__maybe_unused" to your prepare/complete
function and then always define them.
I can't say I've thought through every corner case but at least this
change no longer raises alarm bells in my mind when I look at it. ;-)
If it works for you and nobody else has objections then it seems good
enough and we can always make more improvements later. Feel free to
add my Reviewed-by tag when my nit is fixed and you make sure it
compiles even if CONFIG_PM_SLEEP isn't defined.
-Doug
WARNING: multiple messages have this Message-ID (diff)
From: Doug Anderson <dianders@chromium.org>
To: Kalyan Thota <kalyan_t@codeaurora.org>
Cc: "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS"
<devicetree@vger.kernel.org>,
mkrishn@codeaurora.org,
linux-arm-msm <linux-arm-msm@vger.kernel.org>,
travitej@codeaurora.org, LKML <linux-kernel@vger.kernel.org>,
dri-devel <dri-devel@lists.freedesktop.org>,
Sean Paul <seanpaul@chromium.org>,
"Kristian H. Kristensen" <hoegsberg@chromium.org>,
freedreno <freedreno@lists.freedesktop.org>
Subject: Re: [PATCH v6] drm/msm/dpu: ensure device suspend happens during PM sleep
Date: Thu, 4 Jun 2020 13:34:05 -0700 [thread overview]
Message-ID: <CAD=FV=V7ce7EGqJh6aga4yH3NqdgXMHBe=EOONtcd2LFDX75_A@mail.gmail.com> (raw)
In-Reply-To: <1591276775-13949-1-git-send-email-kalyan_t@codeaurora.org>
Hi,
On Thu, Jun 4, 2020 at 6:20 AM Kalyan Thota <kalyan_t@codeaurora.org> wrote:
>
> -#ifdef CONFIG_PM
> -static int msm_runtime_suspend(struct device *dev)
> +#ifdef CONFIG_PM_SLEEP
> +static int msm_pm_suspend(struct device *dev)
> {
> - struct drm_device *ddev = dev_get_drvdata(dev);
> - struct msm_drm_private *priv = ddev->dev_private;
> - struct msm_mdss *mdss = priv->mdss;
>
nit: remove blank line at the start of this function
> static const struct dev_pm_ops msm_pm_ops = {
> SET_SYSTEM_SLEEP_PM_OPS(msm_pm_suspend, msm_pm_resume)
> SET_RUNTIME_PM_OPS(msm_runtime_suspend, msm_runtime_resume, NULL)
> + .prepare = msm_pm_prepare,
> + .complete = msm_pm_complete,
Presumably you will get a compile failure if someone compiles without
CONFIG_PM_SLEEP since msm_pm_prepare() and msm_pm_complete() won't be
defined but you refer to them unconditionally. Probably the best
solution is to just add "__maybe_unused" to your prepare/complete
function and then always define them.
I can't say I've thought through every corner case but at least this
change no longer raises alarm bells in my mind when I look at it. ;-)
If it works for you and nobody else has objections then it seems good
enough and we can always make more improvements later. Feel free to
add my Reviewed-by tag when my nit is fixed and you make sure it
compiles even if CONFIG_PM_SLEEP isn't defined.
-Doug
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
next prev parent reply other threads:[~2020-06-04 20:34 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-06-04 13:19 [PATCH v6] drm/msm/dpu: ensure device suspend happens during PM sleep Kalyan Thota
2020-06-04 13:19 ` Kalyan Thota
2020-06-04 20:34 ` Doug Anderson [this message]
2020-06-04 20:34 ` Doug Anderson
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAD=FV=V7ce7EGqJh6aga4yH3NqdgXMHBe=EOONtcd2LFDX75_A@mail.gmail.com' \
--to=dianders@chromium.org \
--cc=devicetree@vger.kernel.org \
--cc=dri-devel@lists.freedesktop.org \
--cc=freedreno@lists.freedesktop.org \
--cc=hoegsberg@chromium.org \
--cc=jsanka@codeaurora.org \
--cc=kalyan_t@codeaurora.org \
--cc=linux-arm-msm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mkrishn@codeaurora.org \
--cc=nganji@codeaurora.org \
--cc=robdclark@gmail.com \
--cc=seanpaul@chromium.org \
--cc=travitej@codeaurora.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.