| Message ID | 1457537506-12455-6-git-send-email-imre.deak@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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. -Chris
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
On Wed, Mar 09, 2016 at 06:01:32PM +0200, Imre Deak wrote: > > 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. imo, set it to a nop_init_clock_gating as it is very much the exception rather than the rule (and expect we'll find something to put in there eventually!) -Chris
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; +} + /* Set up chip specific power management-related functions */ void intel_init_pm(struct drm_device *dev) { @@ -7123,10 +7155,6 @@ void intel_init_pm(struct drm_device *dev) /* For FIFO watermark updates */ if (INTEL_INFO(dev)->gen >= 9) { skl_setup_wm_latency(dev); - - if (IS_BROXTON(dev)) - dev_priv->display.init_clock_gating = - bxt_init_clock_gating; dev_priv->display.update_wm = skl_update_wm; } else if (HAS_PCH_SPLIT(dev)) { ilk_setup_wm_latency(dev); @@ -7146,29 +7174,12 @@ void intel_init_pm(struct drm_device *dev) DRM_DEBUG_KMS("Failed to read display plane latency. " "Disable CxSR\n"); } - - if (IS_GEN5(dev)) - dev_priv->display.init_clock_gating = ironlake_init_clock_gating; - else if (IS_GEN6(dev)) - dev_priv->display.init_clock_gating = gen6_init_clock_gating; - else if (IS_IVYBRIDGE(dev)) - dev_priv->display.init_clock_gating = ivybridge_init_clock_gating; - else if (IS_HASWELL(dev)) - dev_priv->display.init_clock_gating = haswell_init_clock_gating; - else if (INTEL_INFO(dev)->gen == 8) - dev_priv->display.init_clock_gating = broadwell_init_clock_gating; } else if (IS_CHERRYVIEW(dev)) { vlv_setup_wm_latency(dev); - dev_priv->display.update_wm = vlv_update_wm; - dev_priv->display.init_clock_gating = - cherryview_init_clock_gating; } else if (IS_VALLEYVIEW(dev)) { vlv_setup_wm_latency(dev); - dev_priv->display.update_wm = vlv_update_wm; - dev_priv->display.init_clock_gating = - valleyview_init_clock_gating; } else if (IS_PINEVIEW(dev)) { if (!intel_get_cxsr_latency(IS_PINEVIEW_G(dev), dev_priv->is_ddr3, @@ -7184,20 +7195,13 @@ void intel_init_pm(struct drm_device *dev) dev_priv->display.update_wm = NULL; } else dev_priv->display.update_wm = pineview_update_wm; - dev_priv->display.init_clock_gating = gen3_init_clock_gating; } else if (IS_G4X(dev)) { dev_priv->display.update_wm = g4x_update_wm; - dev_priv->display.init_clock_gating = g4x_init_clock_gating; } else if (IS_GEN4(dev)) { dev_priv->display.update_wm = i965_update_wm; - if (IS_CRESTLINE(dev)) - dev_priv->display.init_clock_gating = crestline_init_clock_gating; - else if (IS_BROADWATER(dev)) - dev_priv->display.init_clock_gating = broadwater_init_clock_gating; } else if (IS_GEN3(dev)) { dev_priv->display.update_wm = i9xx_update_wm; dev_priv->display.get_fifo_size = i9xx_get_fifo_size; - dev_priv->display.init_clock_gating = gen3_init_clock_gating; } else if (IS_GEN2(dev)) { if (INTEL_INFO(dev)->num_pipes == 1) { dev_priv->display.update_wm = i845_update_wm; @@ -7206,11 +7210,6 @@ void intel_init_pm(struct drm_device *dev) dev_priv->display.update_wm = i9xx_update_wm; dev_priv->display.get_fifo_size = i830_get_fifo_size; } - - if (IS_I85X(dev) || IS_I865G(dev)) - dev_priv->display.init_clock_gating = i85x_init_clock_gating; - else - dev_priv->display.init_clock_gating = i830_init_clock_gating; } else { DRM_ERROR("unexpected fall-through in intel_init_pm\n"); }
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(-)