[Intel-gfx] [PATCH 02/10] drm/i915: disable power well support on platforms without runtime PM support

[Daniel Vetter]

On Tue, Dec 15, 2015 at 10:57:31PM +0200, Ville Syrjälä wrote:
> On Tue, Dec 15, 2015 at 08:10:30PM +0200, Imre Deak wrote:
> > All platforms with power well support have runtime PM support, so
> > simplify things by explicitly disabling power well support on platforms
> > without runtime PM support. This results in holding the init power domain
> > reference whenever the driver is loaded in addition to an RPM reference,
> > which reflects the reality better and makes it possible to simplify
> > things by removing the HAS_RUNTIME_PM special casing from more places in
> > a follow-up patch.
> > 
> > Signed-off-by: Imre Deak <imre.deak at intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_runtime_pm.c | 5 +++++
> >  1 file changed, 5 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > index 9945040..f4ff5f5 100644
> > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > @@ -1908,6 +1908,11 @@ static int
> >  sanitize_disable_power_well_option(const struct drm_i915_private *dev_priv,
> >  				   int disable_power_well)
> >  {
> > +	if (!HAS_RUNTIME_PM(dev_priv)) {
> > +		DRM_DEBUG_KMS("No runtime PM support, disabling display power well support\n");
> > +		return 0;
> > +	}
> 
> Feels a bit too magic to me. Needs a comment at least, otherwise someone
> is going to change disable_power_well back into something you can disable
> at runtime and then all the old stuff might break.
> 
> Grabbing an extra rpm reference explicitly for this purpose might be
> less confusing.

Changing module options that are marked as debug taints your kernel.
Keeping them read-only (so that at least nothing can change at runtime) is
imo much preferred, and it's what I started to require on the gem side.
Really not worth to have all these checks and complexity around for the
off chance some developers want to change the option without reloading the
driver.

But yeah comment might be useful here.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch