All of lore.kernel.org
 help / color / mirror / Atom feed
From: Thierry Reding <thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
To: Mark Zhang <markz-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
Cc: Sean Paul <seanpaul-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>,
	tbergstrom-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org,
	Dave Airlie <airlied-cv59FeDIM0c@public.gmane.org>,
	Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>,
	gnurou-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org,
	dri-devel
	<dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org>,
	"linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org"
	<linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	Linux Kernel Mailing List
	<linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>
Subject: Re: [PATCH] drm/tegra: dsi: Add suspend/resume support
Date: Fri, 19 Dec 2014 16:30:53 +0100	[thread overview]
Message-ID: <20141219153051.GA5046@ulmo.nvidia.com> (raw)
In-Reply-To: <5487AB39.1050706-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>

[-- Attachment #1: Type: text/plain, Size: 1996 bytes --]

On Wed, Dec 10, 2014 at 10:08:57AM +0800, Mark Zhang wrote:
> On 12/10/2014 03:29 AM, Sean Paul wrote:
> > On Sun, Dec 7, 2014 at 10:40 PM, Mark Zhang <markz-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> wrote:
[...]
> >>
> >>         err = clk_set_rate(dsi->clk_parent, plld);
> >>         if (err < 0) {
> [...]
> >> +
> >> +       drm_modeset_lock_all(tegra->drm);
> >> +       list_for_each_entry(connector, &tegra->drm->mode_config.connector_list,
> >> +                           head) {
> >> +               if (connector->funcs->dpms)
> >> +                       connector->funcs->dpms(connector, connector->dpms);
> >> +       }
> >> +       drm_modeset_unlock_all(tegra->drm);
> >> +
> >> +       drm_helper_resume_force_mode(tegra->drm);
> >> +
> >> +       return 0;
> >> +}
> >> +#endif
> > 
> > So this is the tricky chunk, it definitely does not belong here. I
> > think it makes most sense for this to live in drm.c, however
> > host1x_driver doesn't have suspend/resume hooks.
> > 
> 
> Agree. drm.c is the best place for putting this.
> 
> > I suspect the correct thing to do here is to add them to
> > host1x_driver, but I'm not sure how much effort is involved in doing
> > that.
> > 
> 
> I thought about this before. If we do it in host1x driver, IIUC, it
> means that when host1x starts suspending, it will suspend all host1x
> client devices, right? Honestly I feel this is not a good thing despite
> I can't tell what's the problem in this right now...

I've just sent out patches for review that add the missing
infrastructure to properly do suspend/resume. The idea behind this is
that the DRM host1x device's ->pm_ops are responsible for dealing with
the suspend/resume at a subsystem level (call connectors' ->dpms()
callbacks, etc.) whereas more fine-grained suspend/resume can further be
done in the ->pm_ops of the individual subdevices. The infrastructure
makes sure that these get called in the right order.

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]

WARNING: multiple messages have this Message-ID (diff)
From: Thierry Reding <thierry.reding@gmail.com>
To: Mark Zhang <markz@nvidia.com>
Cc: Sean Paul <seanpaul@chromium.org>,
	tbergstrom@nvidia.com, Dave Airlie <airlied@linux.ie>,
	Stephen Warren <swarren@wwwdotorg.org>,
	gnurou@gmail.com, dri-devel <dri-devel@lists.freedesktop.org>,
	"linux-tegra@vger.kernel.org" <linux-tegra@vger.kernel.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] drm/tegra: dsi: Add suspend/resume support
Date: Fri, 19 Dec 2014 16:30:53 +0100	[thread overview]
Message-ID: <20141219153051.GA5046@ulmo.nvidia.com> (raw)
In-Reply-To: <5487AB39.1050706@nvidia.com>

[-- Attachment #1: Type: text/plain, Size: 1967 bytes --]

On Wed, Dec 10, 2014 at 10:08:57AM +0800, Mark Zhang wrote:
> On 12/10/2014 03:29 AM, Sean Paul wrote:
> > On Sun, Dec 7, 2014 at 10:40 PM, Mark Zhang <markz@nvidia.com> wrote:
[...]
> >>
> >>         err = clk_set_rate(dsi->clk_parent, plld);
> >>         if (err < 0) {
> [...]
> >> +
> >> +       drm_modeset_lock_all(tegra->drm);
> >> +       list_for_each_entry(connector, &tegra->drm->mode_config.connector_list,
> >> +                           head) {
> >> +               if (connector->funcs->dpms)
> >> +                       connector->funcs->dpms(connector, connector->dpms);
> >> +       }
> >> +       drm_modeset_unlock_all(tegra->drm);
> >> +
> >> +       drm_helper_resume_force_mode(tegra->drm);
> >> +
> >> +       return 0;
> >> +}
> >> +#endif
> > 
> > So this is the tricky chunk, it definitely does not belong here. I
> > think it makes most sense for this to live in drm.c, however
> > host1x_driver doesn't have suspend/resume hooks.
> > 
> 
> Agree. drm.c is the best place for putting this.
> 
> > I suspect the correct thing to do here is to add them to
> > host1x_driver, but I'm not sure how much effort is involved in doing
> > that.
> > 
> 
> I thought about this before. If we do it in host1x driver, IIUC, it
> means that when host1x starts suspending, it will suspend all host1x
> client devices, right? Honestly I feel this is not a good thing despite
> I can't tell what's the problem in this right now...

I've just sent out patches for review that add the missing
infrastructure to properly do suspend/resume. The idea behind this is
that the DRM host1x device's ->pm_ops are responsible for dealing with
the suspend/resume at a subsystem level (call connectors' ->dpms()
callbacks, etc.) whereas more fine-grained suspend/resume can further be
done in the ->pm_ops of the individual subdevices. The infrastructure
makes sure that these get called in the right order.

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]

  parent reply	other threads:[~2014-12-19 15:30 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-12-08  6:40 [PATCH] drm/tegra: dsi: Add suspend/resume support Mark Zhang
2014-12-08  6:40 ` Mark Zhang
2014-12-09 19:29 ` Sean Paul
2014-12-09 19:29   ` Sean Paul
     [not found]   ` <CAOw6vbKegB8cgmqgRit+XdvYNtdEXy3Pcv5=1bYSCJv4v1F2AQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2014-12-10  2:08     ` Mark Zhang
2014-12-10  2:08       ` Mark Zhang
     [not found]       ` <5487AB39.1050706-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
2014-12-19 15:30         ` Thierry Reding [this message]
2014-12-19 15:30           ` Thierry Reding

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=20141219153051.GA5046@ulmo.nvidia.com \
    --to=thierry.reding-re5jqeeqqe8avxtiumwx3w@public.gmane.org \
    --cc=airlied-cv59FeDIM0c@public.gmane.org \
    --cc=dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org \
    --cc=gnurou-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org \
    --cc=linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=markz-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org \
    --cc=seanpaul-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org \
    --cc=swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org \
    --cc=tbergstrom-DDmLM1+adcrQT0dZR+AlfA@public.gmane.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.