From: Andrew Morton <akpm@linux-foundation.org>
To: Andres Salomon <dilinger@queued.net>
Cc: linux-fbdev-devel@lists.sourceforge.net, adaplas@gmail.com,
linux-kernel@vger.kernel.org, info-linux@geode.amd.com,
rjw@sisk.pl, jordan.crouse@amd.com, samuel.thibault@ens-lyon.org
Subject: Re: [PATCH 6/6] PM/FB/gxfb: add config option that allows disabling of VT switch during suspend
Date: Mon, 10 Mar 2008 15:28:28 -0700 [thread overview]
Message-ID: <20080310152828.8f102f52.akpm@linux-foundation.org> (raw)
In-Reply-To: <20080310181446.2fb85eea@ephemeral>
On Mon, 10 Mar 2008 18:14:46 -0400 Andres Salomon <dilinger@queued.net> wrote:
> On Mon, 10 Mar 2008 14:51:18 -0700
> Andrew Morton <akpm@linux-foundation.org> wrote:
>
> > On Mon, 10 Mar 2008 17:44:24 -0400
> > Andres Salomon <dilinger@queued.net> wrote:
> >
> > > On Mon, 10 Mar 2008 14:19:39 -0700
> > > Andrew Morton <akpm@linux-foundation.org> wrote:
> > >
> > > >
> > > > It would be far preferable to do this via a kernel boot parameter rather
> > > > than via a kernel rebuild.
> > > >
> > >
> > > Well, it shouldn't be supported at _all_ unless the framebuffer driver
> > > supports it; you're asking for it to be converted to something like the
> > > following?
> > >
> > >
> > >
> > > #ifdef CONFIG_FBDEV_SUPPORTS_NOSWITCH
> > > static int no_vt_switch;
> > > #endif
> > >
> > > int pm_prepare_console(void)
> > > {
> > > #ifdef CONFIG_FBDEV_SUPPORTS_NOSWITCH
> > > if (no_vt_switch)
> > > return 0;
> > > #endif
> > > ...
> > > }
> > >
> > > #ifdef CONFIG_FBDEV_SUPPORTS_NOSWITCH
> > > module_param(no_vt_switch, int, 0);
> > > MODULE_PARM_DESC(no_vt_switch, "...");
> > > #endif
> > >
> > >
> > > If so, that means extra #ifdef's sprinkled throughout the code. Note
> > > that most framebuffer drivers won't be supporting this, and those that
> > > do will almost certainly always want vt switching turned off..
> > >
> >
> > I don't think you need any ifdefs or config options at all, do you? Just
> > add a new module parameter to this driver and some little interface into
> > the PM core which allows the driver to set no_vt_switch?
> >
>
> That's a possibility, but doesn't adding hooks into PM core for two
> drivers (lxfb and gxfb) seems like overkill?
It's just a few bytes of code?
> I also don't really see
> anyone needing to switch back and forth between VT_SWITCH and
> NO_VT_SWITCH other than to work around possible bugs in the register
> save/restore code..
Making driver behaviour depend upon compile-time settings is really to be
avoided. A distributor has to choose one setting or the other, and a) that
means that some people will get the wrong setting and b) there's a decent
chance that the distributor will get it wrong.
-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
WARNING: multiple messages have this Message-ID (diff)
From: Andrew Morton <akpm@linux-foundation.org>
To: Andres Salomon <dilinger@queued.net>
Cc: adaplas@gmail.com, linux-kernel@vger.kernel.org,
linux-fbdev-devel@lists.sourceforge.net,
info-linux@geode.amd.com, jordan.crouse@amd.com, rjw@sisk.pl,
samuel.thibault@ens-lyon.org
Subject: Re: [PATCH 6/6] PM/FB/gxfb: add config option that allows disabling of VT switch during suspend
Date: Mon, 10 Mar 2008 15:28:28 -0700 [thread overview]
Message-ID: <20080310152828.8f102f52.akpm@linux-foundation.org> (raw)
In-Reply-To: <20080310181446.2fb85eea@ephemeral>
On Mon, 10 Mar 2008 18:14:46 -0400 Andres Salomon <dilinger@queued.net> wrote:
> On Mon, 10 Mar 2008 14:51:18 -0700
> Andrew Morton <akpm@linux-foundation.org> wrote:
>
> > On Mon, 10 Mar 2008 17:44:24 -0400
> > Andres Salomon <dilinger@queued.net> wrote:
> >
> > > On Mon, 10 Mar 2008 14:19:39 -0700
> > > Andrew Morton <akpm@linux-foundation.org> wrote:
> > >
> > > >
> > > > It would be far preferable to do this via a kernel boot parameter rather
> > > > than via a kernel rebuild.
> > > >
> > >
> > > Well, it shouldn't be supported at _all_ unless the framebuffer driver
> > > supports it; you're asking for it to be converted to something like the
> > > following?
> > >
> > >
> > >
> > > #ifdef CONFIG_FBDEV_SUPPORTS_NOSWITCH
> > > static int no_vt_switch;
> > > #endif
> > >
> > > int pm_prepare_console(void)
> > > {
> > > #ifdef CONFIG_FBDEV_SUPPORTS_NOSWITCH
> > > if (no_vt_switch)
> > > return 0;
> > > #endif
> > > ...
> > > }
> > >
> > > #ifdef CONFIG_FBDEV_SUPPORTS_NOSWITCH
> > > module_param(no_vt_switch, int, 0);
> > > MODULE_PARM_DESC(no_vt_switch, "...");
> > > #endif
> > >
> > >
> > > If so, that means extra #ifdef's sprinkled throughout the code. Note
> > > that most framebuffer drivers won't be supporting this, and those that
> > > do will almost certainly always want vt switching turned off..
> > >
> >
> > I don't think you need any ifdefs or config options at all, do you? Just
> > add a new module parameter to this driver and some little interface into
> > the PM core which allows the driver to set no_vt_switch?
> >
>
> That's a possibility, but doesn't adding hooks into PM core for two
> drivers (lxfb and gxfb) seems like overkill?
It's just a few bytes of code?
> I also don't really see
> anyone needing to switch back and forth between VT_SWITCH and
> NO_VT_SWITCH other than to work around possible bugs in the register
> save/restore code..
Making driver behaviour depend upon compile-time settings is really to be
avoided. A distributor has to choose one setting or the other, and a) that
means that some people will get the wrong setting and b) there's a decent
chance that the distributor will get it wrong.
next prev parent reply other threads:[~2008-03-10 22:30 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2008-03-09 1:49 [PATCH 6/6] PM/FB/gxfb: add config option that allows disabling of VT switch during suspend Andres Salomon
2008-03-09 1:49 ` Andres Salomon
2008-03-10 21:19 ` Andrew Morton
2008-03-10 21:19 ` Andrew Morton
2008-03-10 21:44 ` Andres Salomon
2008-03-10 21:44 ` Andres Salomon
2008-03-10 21:51 ` Andrew Morton
2008-03-10 21:51 ` Andrew Morton
2008-03-10 22:14 ` Andres Salomon
2008-03-10 22:14 ` Andres Salomon
2008-03-10 22:24 ` Jordan Crouse
2008-03-10 22:24 ` Jordan Crouse
2008-03-10 23:06 ` Pavel Machek
2008-03-10 23:06 ` Pavel Machek
2008-03-10 22:28 ` [PATCH 6/6] " Rafael J. Wysocki
2008-03-10 22:28 ` Rafael J. Wysocki
2008-03-10 23:37 ` Andrew Morton
2008-03-10 22:28 ` Andrew Morton [this message]
2008-03-10 22:28 ` Andrew Morton
2008-03-10 23:05 ` Pavel Machek
2008-03-10 23:05 ` Pavel Machek
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=20080310152828.8f102f52.akpm@linux-foundation.org \
--to=akpm@linux-foundation.org \
--cc=adaplas@gmail.com \
--cc=dilinger@queued.net \
--cc=info-linux@geode.amd.com \
--cc=jordan.crouse@amd.com \
--cc=linux-fbdev-devel@lists.sourceforge.net \
--cc=linux-kernel@vger.kernel.org \
--cc=rjw@sisk.pl \
--cc=samuel.thibault@ens-lyon.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.