| Message ID | 1359415936-2732-2-git-send-email-ben@bwidawsk.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Jan 28, 2013 at 03:32:16PM -0800, Ben Widawsky wrote: > It should only be used with caution... > > Signed-off-by: Ben Widawsky <ben@bwidawsk.net> Isn't that like the general assumption of these module parameters that they can have pretty massive bad side-effects? Meaning I'm routinely checking these already anyway in bug reports, same for non-standard rc6 settings. -Daniel > --- > drivers/gpu/drm/i915/i915_drv.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c > index 9cc8f87..912db3a 100644 > --- a/drivers/gpu/drm/i915/i915_drv.c > +++ b/drivers/gpu/drm/i915/i915_drv.c > @@ -76,6 +76,7 @@ int i915_enable_fbc __read_mostly = -1; > module_param_named(i915_enable_fbc, i915_enable_fbc, int, 0600); > MODULE_PARM_DESC(i915_enable_fbc, > "Enable frame buffer compression for power savings " > + "WARNING: FBC has been implicated in hangs over the years, and should likely be left to the default value. " > "(default: -1 (use per-chip default))"); > > unsigned int i915_lvds_downclock __read_mostly = 0; > -- > 1.8.1.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Tue, 29 Jan 2013 09:49:13 +0100 Daniel Vetter <daniel@ffwll.ch> wrote: > On Mon, Jan 28, 2013 at 03:32:16PM -0800, Ben Widawsky wrote: > > It should only be used with caution... > > > > Signed-off-by: Ben Widawsky <ben@bwidawsk.net> > > Isn't that like the general assumption of these module parameters that > they can have pretty massive bad side-effects? Meaning I'm routinely > checking these already anyway in bug reports, same for non-standard > rc6 settings. > -Daniel > I felt that way too, until we got a complain in #intel-gfx and it changed my mind. Whatevs, I ain't gonna fight for this one, I'll just punt the bug reporter to you next time.
On Tue, 29 Jan 2013, Ben Widawsky <ben@bwidawsk.net> wrote: > On Tue, 29 Jan 2013 09:49:13 +0100 > Daniel Vetter <daniel@ffwll.ch> wrote: > >> On Mon, Jan 28, 2013 at 03:32:16PM -0800, Ben Widawsky wrote: >> > It should only be used with caution... >> > >> > Signed-off-by: Ben Widawsky <ben@bwidawsk.net> >> >> Isn't that like the general assumption of these module parameters that >> they can have pretty massive bad side-effects? Meaning I'm routinely >> checking these already anyway in bug reports, same for non-standard >> rc6 settings. >> -Daniel >> > > I felt that way too, until we got a complain in #intel-gfx and it > changed my mind. > > Whatevs, I ain't gonna fight for this one, I'll just punt the bug > reporter to you next time. Yeah, I agree it's *our* general assumption that these may have bad side-effects. People just see the random forum posts recommending this and that module param, and stick them in... Hmm, which means they won't read that warning anyway. DRM_INFO("don't report a bug about this") when enabling a feature that's disabled by default on a platform? Jani.
On Wed, Jan 30, 2013 at 04:18:39PM +0200, Jani Nikula wrote: > On Tue, 29 Jan 2013, Ben Widawsky <ben@bwidawsk.net> wrote: > > On Tue, 29 Jan 2013 09:49:13 +0100 > > Daniel Vetter <daniel@ffwll.ch> wrote: > > > >> On Mon, Jan 28, 2013 at 03:32:16PM -0800, Ben Widawsky wrote: > >> > It should only be used with caution... > >> > > >> > Signed-off-by: Ben Widawsky <ben@bwidawsk.net> > >> > >> Isn't that like the general assumption of these module parameters that > >> they can have pretty massive bad side-effects? Meaning I'm routinely > >> checking these already anyway in bug reports, same for non-standard > >> rc6 settings. > >> -Daniel > >> > > > > I felt that way too, until we got a complain in #intel-gfx and it > > changed my mind. > > > > Whatevs, I ain't gonna fight for this one, I'll just punt the bug > > reporter to you next time. > > Yeah, I agree it's *our* general assumption that these may have bad > side-effects. People just see the random forum posts recommending this > and that module param, and stick them in... Hmm, which means they won't > read that warning anyway. DRM_INFO("don't report a bug about this") when > enabling a feature that's disabled by default on a platform? > > Jani. Now that's the kind of pessimism I like to hear! OTOH, the current message: "Enable frame buffer compression for power savings" wouldn't indicate any reason to not use it. And yes, I know, what git blame says. I was on a personal crusade to fix FBC on ILK when I wrote that. I like DRM_INFO as well, but I don't really see a reason not to change the modinfo (DRM_INFO requires loading the wrong setting first).
On Thu, Jan 31, 2013 at 4:13 AM, Ben Widawsky <ben@bwidawsk.net> wrote: >> Yeah, I agree it's *our* general assumption that these may have bad >> side-effects. People just see the random forum posts recommending this >> and that module param, and stick them in... Hmm, which means they won't >> read that warning anyway. DRM_INFO("don't report a bug about this") when >> enabling a feature that's disabled by default on a platform? >> >> Jani. > > Now that's the kind of pessimism I like to hear! OTOH, the current > message: "Enable frame buffer compression for power savings" wouldn't > indicate any reason to not use it. And yes, I know, what git blame says. > I was on a personal crusade to fix FBC on ILK when I wrote that. > > I like DRM_INFO as well, but I don't really see a reason not to change > the modinfo (DRM_INFO requires loading the wrong setting first). Ok, count me convinced to merge such patches. Though I personally don't care and don't mind checking bug reports for these, it looks like I'm in the minority opinion. I'd like though that such a solution not just takes care of fbc, but also some of the other dangerous options (which are most): rc6 can cause hangs, advanced power features can lead to flashing displays, ... Cheers, Daniel
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 9cc8f87..912db3a 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -76,6 +76,7 @@ int i915_enable_fbc __read_mostly = -1; module_param_named(i915_enable_fbc, i915_enable_fbc, int, 0600); MODULE_PARM_DESC(i915_enable_fbc, "Enable frame buffer compression for power savings " + "WARNING: FBC has been implicated in hangs over the years, and should likely be left to the default value. " "(default: -1 (use per-chip default))"); unsigned int i915_lvds_downclock __read_mostly = 0;
It should only be used with caution... Signed-off-by: Ben Widawsky <ben@bwidawsk.net> --- drivers/gpu/drm/i915/i915_drv.c | 1 + 1 file changed, 1 insertion(+)