* [PATCH v3] drm/i915: Honor SSC quirk table over the default, unless set by user @ 2011-11-09 16:12 Michel Alexandre Salim 2011-11-09 16:30 ` Michel Alexandre Salim 0 siblings, 1 reply; 5+ messages in thread From: Michel Alexandre Salim @ 2011-11-09 16:12 UTC (permalink / raw) To: intel-gfx; +Cc: Keith Packard, Jesse Barnes, Chris Wilson, linux-kernel >From a90bf9ac2a40869242a79c88958c99dacc3da2a9 Mon Sep 17 00:00:00 2001 From: Michel Alexandre Salim <salimma@fedoraproject.org> Date: Wed, 9 Nov 2011 14:18:45 +0100 Subject: [PATCH v3] drm/i915: Honor SSC quirk table over the default, unless set by user Commit 72bbe58cd9568c7766cc219a779ea68a02132797 makes the check against the quirk table unreachable if i915_panel_use_ssc is automatically set to a non-zero value. This patch changes the tests so that unless i915_panel_use_ssc is set to exactly 1, the quirk table is checked. This ensures that on known quirky hardware, SSC is disabled unless specifically enabled by the user. Signed-off-by: Michel Alexandre Salim <salimma@fedoraproject.org> --- drivers/gpu/drm/i915/intel_display.c | 7 +++++-- 1 files changed, 5 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 981b1f1..aa7b0c8 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -4660,10 +4660,13 @@ static void intel_update_watermarks(struct drm_device *dev) static inline bool intel_panel_use_ssc(struct drm_i915_private *dev_priv) { + if (i915_panel_use_ssc == 1) + return true; + if (dev_priv->quirks & QUIRK_LVDS_SSC_DISABLE) + return false; if (i915_panel_use_ssc >= 0) return i915_panel_use_ssc != 0; - return dev_priv->lvds_use_ssc - && !(dev_priv->quirks & QUIRK_LVDS_SSC_DISABLE); + return dev_priv->lvds_use_ssc; } /** -- 1.7.7 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v3] drm/i915: Honor SSC quirk table over the default, unless set by user 2011-11-09 16:12 [PATCH v3] drm/i915: Honor SSC quirk table over the default, unless set by user Michel Alexandre Salim @ 2011-11-09 16:30 ` Michel Alexandre Salim 2011-11-09 18:07 ` Keith Packard 0 siblings, 1 reply; 5+ messages in thread From: Michel Alexandre Salim @ 2011-11-09 16:30 UTC (permalink / raw) To: intel-gfx; +Cc: Keith Packard, Jesse Barnes, Chris Wilson, linux-kernel Additional note: while I've not touched the line since it does not affect me, it seems that i915_panel_use_ssc *cannot* be less than 0 since that variable is declared as unsigned. So the last line (the value in dev_priv) will never be used to determine whether SSC is used anyway. Keith probably knows more since he introduces that check in the commit that my patch referred to. On 11/09/2011 05:12 PM, Michel Alexandre Salim wrote: > static inline bool intel_panel_use_ssc(struct drm_i915_private *dev_priv) > { > + if (i915_panel_use_ssc == 1) > + return true; > + if (dev_priv->quirks & QUIRK_LVDS_SSC_DISABLE) > + return false; > if (i915_panel_use_ssc >= 0) > return i915_panel_use_ssc != 0; > - return dev_priv->lvds_use_ssc > - && !(dev_priv->quirks & QUIRK_LVDS_SSC_DISABLE); > + return dev_priv->lvds_use_ssc; > } > > /** -- Michel Alexandre Salim µblog: http://identi.ca/hircus http://twitter.com/hircus GPG key ID: 78884778 () ascii ribbon campaign - against html e-mail /\ www.asciiribbon.org - against proprietary attachments ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v3] drm/i915: Honor SSC quirk table over the default, unless set by user 2011-11-09 16:30 ` Michel Alexandre Salim @ 2011-11-09 18:07 ` Keith Packard 2011-11-10 9:46 ` Michel Alexandre Salim 2011-11-16 10:49 ` Michel Alexandre Salim 0 siblings, 2 replies; 5+ messages in thread From: Keith Packard @ 2011-11-09 18:07 UTC (permalink / raw) To: Michel Alexandre Salim, intel-gfx Cc: Jesse Barnes, Chris Wilson, linux-kernel [-- Attachment #1: Type: text/plain, Size: 2970 bytes --] On Wed, 09 Nov 2011 17:30:29 +0100, Michel Alexandre Salim <salimma@fedoraproject.org> wrote: > Additional note: while I've not touched the line since it does not > affect me, it seems that i915_panel_use_ssc *cannot* be less than 0 > since that variable is declared as unsigned. Oops. That's the bug here -- we're supposed to make it so that the command line can override the quirks, but there's no way to use a quirk given the mis-declared parameter. This is untested... From e64ecadef40e3c2035cd4e9b967ffd83489bdea0 Mon Sep 17 00:00:00 2001 From: Keith Packard <keithp@keithp.com> Date: Wed, 9 Nov 2011 09:57:50 -0800 Subject: [PATCH] drm/i915: Module parameters using '-1' as default must be signed type Testing i915_panel_use_ssc for the default value was broken, so the driver would never autodetect the correct value. Signed-off-by: Keith Packard <keithp@keithp.com> --- drivers/gpu/drm/i915/i915_drv.c | 4 ++-- drivers/gpu/drm/i915/i915_drv.h | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 548e04b..13488be 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -67,7 +67,7 @@ module_param_named(i915_enable_rc6, i915_enable_rc6, int, 0600); MODULE_PARM_DESC(i915_enable_rc6, "Enable power-saving render C-state 6 (default: true)"); -unsigned int i915_enable_fbc __read_mostly = -1; +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 " @@ -79,7 +79,7 @@ MODULE_PARM_DESC(lvds_downclock, "Use panel (LVDS/eDP) downclocking for power savings " "(default: false)"); -unsigned int i915_panel_use_ssc __read_mostly = -1; +int i915_panel_use_ssc __read_mostly = -1; module_param_named(lvds_use_ssc, i915_panel_use_ssc, int, 0600); MODULE_PARM_DESC(lvds_use_ssc, "Use Spread Spectrum Clock with panels [LVDS/eDP] " diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index d2da91f..4a9c1b9 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1000,10 +1000,10 @@ extern int i915_panel_ignore_lid __read_mostly; extern unsigned int i915_powersave __read_mostly; extern unsigned int i915_semaphores __read_mostly; extern unsigned int i915_lvds_downclock __read_mostly; -extern unsigned int i915_panel_use_ssc __read_mostly; +extern int i915_panel_use_ssc __read_mostly; extern int i915_vbt_sdvo_panel_type __read_mostly; extern unsigned int i915_enable_rc6 __read_mostly; -extern unsigned int i915_enable_fbc __read_mostly; +extern int i915_enable_fbc __read_mostly; extern bool i915_enable_hangcheck __read_mostly; extern int i915_suspend(struct drm_device *dev, pm_message_t state); -- 1.7.7 -- keith.packard@intel.com [-- Attachment #2: Type: application/pgp-signature, Size: 827 bytes --] ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v3] drm/i915: Honor SSC quirk table over the default, unless set by user 2011-11-09 18:07 ` Keith Packard @ 2011-11-10 9:46 ` Michel Alexandre Salim 2011-11-16 10:49 ` Michel Alexandre Salim 1 sibling, 0 replies; 5+ messages in thread From: Michel Alexandre Salim @ 2011-11-10 9:46 UTC (permalink / raw) To: Keith Packard; +Cc: intel-gfx, Jesse Barnes, Chris Wilson, linux-kernel On Wed, 2011-11-09 at 10:07 -0800, Keith Packard wrote: > On Wed, 09 Nov 2011 17:30:29 +0100, Michel Alexandre Salim <salimma@fedoraproject.org> wrote: > > Additional note: while I've not touched the line since it does not > > affect me, it seems that i915_panel_use_ssc *cannot* be less than 0 > > since that variable is declared as unsigned. > > Oops. That's the bug here -- we're supposed to make it so that the > command line can override the quirks, but there's no way to use a quirk > given the mis-declared parameter. Ah, now everything makes sense. > > This is untested... > Tested and it works fine: - without extra parameter, the blacklist is used - with i915.lvds_use_ssc=1, SSC use is enforced and the display turns black on my unsupported hardware - with i915.lvds_use_ssc=0, SSC is disabled (ps the PGP/MIME signature made it hard to just extract the git-formatted email; I ended up just editing the message by hand before 'git am') > From e64ecadef40e3c2035cd4e9b967ffd83489bdea0 Mon Sep 17 00:00:00 2001 > From: Keith Packard <keithp@keithp.com> > Date: Wed, 9 Nov 2011 09:57:50 -0800 > Subject: [PATCH] drm/i915: Module parameters using '-1' as default must be > signed type > > Testing i915_panel_use_ssc for the default value was broken, so the > driver would never autodetect the correct value. > > Signed-off-by: Keith Packard <keithp@keithp.com> Reviewed-by: Michel Alexandre Salim <salimma@fedoraproject.org> Tested-by: Michel Alexandre Salim <salimma@fedoraproject.org> Should I send the patch that I applied with those added lines? Probably not necessary. Thanks, -- Michel ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v3] drm/i915: Honor SSC quirk table over the default, unless set by user 2011-11-09 18:07 ` Keith Packard 2011-11-10 9:46 ` Michel Alexandre Salim @ 2011-11-16 10:49 ` Michel Alexandre Salim 1 sibling, 0 replies; 5+ messages in thread From: Michel Alexandre Salim @ 2011-11-16 10:49 UTC (permalink / raw) To: Keith Packard; +Cc: intel-gfx, linux-kernel Hi Keith, That patch is still not in 3.2-rc2, drm-intel-fixes or drm-intel-next. I've been using it successfully on i915 (both SSC-blacklisted and not) and non-i915 machines; feel free to set the Tested-by and Reviewed-by tags. Thanks, -- Michel On 11/09/2011 07:07 PM, Keith Packard wrote: > On Wed, 09 Nov 2011 17:30:29 +0100, Michel Alexandre Salim <salimma@fedoraproject.org> wrote: >> Additional note: while I've not touched the line since it does not >> affect me, it seems that i915_panel_use_ssc *cannot* be less than 0 >> since that variable is declared as unsigned. > > Oops. That's the bug here -- we're supposed to make it so that the > command line can override the quirks, but there's no way to use a quirk > given the mis-declared parameter. > > This is untested... > > From e64ecadef40e3c2035cd4e9b967ffd83489bdea0 Mon Sep 17 00:00:00 2001 > From: Keith Packard <keithp@keithp.com> > Date: Wed, 9 Nov 2011 09:57:50 -0800 > Subject: [PATCH] drm/i915: Module parameters using '-1' as default must be > signed type > > Testing i915_panel_use_ssc for the default value was broken, so the > driver would never autodetect the correct value. > > Signed-off-by: Keith Packard <keithp@keithp.com> > --- > drivers/gpu/drm/i915/i915_drv.c | 4 ++-- > drivers/gpu/drm/i915/i915_drv.h | 4 ++-- > 2 files changed, 4 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c > index 548e04b..13488be 100644 > --- a/drivers/gpu/drm/i915/i915_drv.c > +++ b/drivers/gpu/drm/i915/i915_drv.c > @@ -67,7 +67,7 @@ module_param_named(i915_enable_rc6, i915_enable_rc6, int, 0600); > MODULE_PARM_DESC(i915_enable_rc6, > "Enable power-saving render C-state 6 (default: true)"); > > -unsigned int i915_enable_fbc __read_mostly = -1; > +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 " > @@ -79,7 +79,7 @@ MODULE_PARM_DESC(lvds_downclock, > "Use panel (LVDS/eDP) downclocking for power savings " > "(default: false)"); > > -unsigned int i915_panel_use_ssc __read_mostly = -1; > +int i915_panel_use_ssc __read_mostly = -1; > module_param_named(lvds_use_ssc, i915_panel_use_ssc, int, 0600); > MODULE_PARM_DESC(lvds_use_ssc, > "Use Spread Spectrum Clock with panels [LVDS/eDP] " > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index d2da91f..4a9c1b9 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -1000,10 +1000,10 @@ extern int i915_panel_ignore_lid __read_mostly; > extern unsigned int i915_powersave __read_mostly; > extern unsigned int i915_semaphores __read_mostly; > extern unsigned int i915_lvds_downclock __read_mostly; > -extern unsigned int i915_panel_use_ssc __read_mostly; > +extern int i915_panel_use_ssc __read_mostly; > extern int i915_vbt_sdvo_panel_type __read_mostly; > extern unsigned int i915_enable_rc6 __read_mostly; > -extern unsigned int i915_enable_fbc __read_mostly; > +extern int i915_enable_fbc __read_mostly; > extern bool i915_enable_hangcheck __read_mostly; > > extern int i915_suspend(struct drm_device *dev, pm_message_t state); > > > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Michel Alexandre Salim µblog: http://identi.ca/hircus http://twitter.com/hircus GPG key ID: 78884778 () ascii ribbon campaign - against html e-mail /\ www.asciiribbon.org - against proprietary attachments ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2011-11-16 10:49 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2011-11-09 16:12 [PATCH v3] drm/i915: Honor SSC quirk table over the default, unless set by user Michel Alexandre Salim 2011-11-09 16:30 ` Michel Alexandre Salim 2011-11-09 18:07 ` Keith Packard 2011-11-10 9:46 ` Michel Alexandre Salim 2011-11-16 10:49 ` Michel Alexandre Salim
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox