Re: [PATCH 5/7] drm/i915: Move load time clock gating callback init earlier

[Imre Deak <imre.deak@intel.com>]

On ke, 2016-03-09 at 15:57 +0000, Chris Wilson wrote:
> On Wed, Mar 09, 2016 at 05:31:44PM +0200, Imre Deak wrote:
> > Split out the part initing the clock gating callbacks and move it
> > earlier.
> > 
> > The rest of the callbacks in intel_init_pm() should be inited in
> > the
> > same way, but atm some of the callbacks are set only conditionally,
> > so
> > before doing this we need to make the setup unconditional and use
> > instead some flags.
> > 
> > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_dma.c  |  1 +
> >  drivers/gpu/drm/i915/intel_drv.h |  1 +
> >  drivers/gpu/drm/i915/intel_pm.c  | 65 ++++++++++++++++++++------
> > --------------
> >  3 files changed, 34 insertions(+), 33 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_dma.c
> > b/drivers/gpu/drm/i915/i915_dma.c
> > index 55b0c62..8cbe9ef 100644
> > --- a/drivers/gpu/drm/i915/i915_dma.c
> > +++ b/drivers/gpu/drm/i915/i915_dma.c
> > @@ -1030,6 +1030,7 @@ int i915_driver_load(struct drm_device *dev,
> > unsigned long flags)
> >  	intel_power_domains_init(dev_priv);
> >  	intel_irq_init(dev_priv);
> >  	intel_init_display_callbacks(dev_priv);
> > +	intel_init_clock_gating_callbacks(dev_priv);
> >  	intel_init_audio_callbacks(dev_priv);
> >  
> >  	intel_runtime_pm_get(dev_priv);
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h
> > b/drivers/gpu/drm/i915/intel_drv.h
> > index 5264901..d3d31cc 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -1596,6 +1596,7 @@ void intel_suspend_hw(struct drm_device
> > *dev);
> >  int ilk_wm_max_level(const struct drm_device *dev);
> >  void intel_update_watermarks(struct drm_crtc *crtc);
> >  void intel_init_pm(struct drm_device *dev);
> > +void intel_init_clock_gating_callbacks(struct drm_i915_private
> > *dev_priv);
> >  void intel_pm_setup(struct drm_device *dev);
> >  void intel_gpu_ips_init(struct drm_i915_private *dev_priv);
> >  void intel_gpu_ips_teardown(void);
> > diff --git a/drivers/gpu/drm/i915/intel_pm.c
> > b/drivers/gpu/drm/i915/intel_pm.c
> > index d7aef17..02d3598 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -7107,6 +7107,38 @@ void intel_suspend_hw(struct drm_device
> > *dev)
> >  		lpt_suspend_hw(dev);
> >  }
> >  
> > +void intel_init_clock_gating_callbacks(struct drm_i915_private
> > *dev_priv)
> > +{
> > +	if (IS_BROXTON(dev_priv))
> > +		dev_priv->display.init_clock_gating =
> > bxt_init_clock_gating;
> > +	else if (IS_BROADWELL(dev_priv))
> > +		dev_priv->display.init_clock_gating =
> > broadwell_init_clock_gating;
> > +	else if (IS_CHERRYVIEW(dev_priv))
> > +		dev_priv->display.init_clock_gating =
> > cherryview_init_clock_gating;
> > +	else if (IS_HASWELL(dev_priv))
> > +		dev_priv->display.init_clock_gating =
> > haswell_init_clock_gating;
> > +	else if (IS_IVYBRIDGE(dev_priv))
> > +		dev_priv->display.init_clock_gating =
> > ivybridge_init_clock_gating;
> > +	else if (IS_VALLEYVIEW(dev_priv))
> > +		dev_priv->display.init_clock_gating =
> > valleyview_init_clock_gating;
> > +	else if (IS_GEN6(dev_priv))
> > +		dev_priv->display.init_clock_gating =
> > gen6_init_clock_gating;
> > +	else if (IS_GEN5(dev_priv))
> > +		dev_priv->display.init_clock_gating =
> > ironlake_init_clock_gating;
> > +	else if (IS_G4X(dev_priv))
> > +		dev_priv->display.init_clock_gating =
> > g4x_init_clock_gating;
> > +	else if (IS_CRESTLINE(dev_priv))
> > +		dev_priv->display.init_clock_gating =
> > crestline_init_clock_gating;
> > +	else if (IS_BROADWATER(dev_priv))
> > +		dev_priv->display.init_clock_gating =
> > broadwater_init_clock_gating;
> > +	else if (IS_GEN3(dev_priv))
> > +		dev_priv->display.init_clock_gating =
> > gen3_init_clock_gating;
> > +	else if (IS_I85X(dev_priv) || IS_I865G(dev_priv))
> > +		dev_priv->display.init_clock_gating =
> > i85x_init_clock_gating;
> > +	else if (IS_GEN2(dev_priv))
> > +		dev_priv->display.init_clock_gating =
> > i830_init_clock_gating;
> 
> else
> MISSING_CASE()
> 
> We definitely need a warning here in case we fall through and leave a
> most unexpected NULL pointer.

Ok, can add that. At least SKL doesn't have a callback, but I can check
for such platforms explicitly.

--Imre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx