[Intel-gfx] [PATCH 4/7] drm/i915: Move load time display/audio callback init earlier

Imre Deak imre.deak at intel.com
Fri Mar 11 14:17:30 UTC 2016


On pe, 2016-03-11 at 16:00 +0200, Jani Nikula wrote:
> On Wed, 09 Mar 2016, Imre Deak <imre.deak at intel.com> wrote:
> > All of this is SW only initialization so we can move them earlier.
> > Move
> > the mutex init where the rest of the locks are inited. While at it
> > also
> > convert dev to dev_priv.
> 
> Bikeshedding on this patch and the next: I don't think the function
> pointers are *callbacks* in any way. There's no "call back". Maybe
> "hooks"?

Yea, callback isn't a correct term here. I have no preference for an
alternative, I can rename it to hooks both here and the next patch.

--Imre

> 
> BR,
> Jani.
> 
> 
> > 
> > Signed-off-by: Imre Deak <imre.deak at intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_dma.c      |  3 ++
> >  drivers/gpu/drm/i915/intel_audio.c   | 12 +++---
> >  drivers/gpu/drm/i915/intel_display.c | 77 ++++++++++++++++------
> > --------------
> >  drivers/gpu/drm/i915/intel_drv.h     |  3 +-
> >  4 files changed, 45 insertions(+), 50 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_dma.c
> > b/drivers/gpu/drm/i915/i915_dma.c
> > index 9b3ed00..55b0c62 100644
> > --- a/drivers/gpu/drm/i915/i915_dma.c
> > +++ b/drivers/gpu/drm/i915/i915_dma.c
> > @@ -1016,6 +1016,7 @@ int i915_driver_load(struct drm_device *dev,
> > unsigned long flags)
> >  	mutex_init(&dev_priv->modeset_restore_lock);
> >  	mutex_init(&dev_priv->av_mutex);
> >  	mutex_init(&dev_priv->wm.wm_mutex);
> > +	mutex_init(&dev_priv->pps_mutex);
> >  
> >  	ret = i915_workqueues_init(dev_priv);
> >  	if (ret < 0)
> > @@ -1028,6 +1029,8 @@ int i915_driver_load(struct drm_device *dev,
> > unsigned long flags)
> >  	intel_init_dpio(dev_priv);
> >  	intel_power_domains_init(dev_priv);
> >  	intel_irq_init(dev_priv);
> > +	intel_init_display_callbacks(dev_priv);
> > +	intel_init_audio_callbacks(dev_priv);
> >  
> >  	intel_runtime_pm_get(dev_priv);
> >  
> > diff --git a/drivers/gpu/drm/i915/intel_audio.c
> > b/drivers/gpu/drm/i915/intel_audio.c
> > index 30f9214..1936752 100644
> > --- a/drivers/gpu/drm/i915/intel_audio.c
> > +++ b/drivers/gpu/drm/i915/intel_audio.c
> > @@ -567,20 +567,18 @@ void intel_audio_codec_disable(struct
> > intel_encoder *intel_encoder)
> >   * intel_init_audio - Set up chip specific audio functions
> >   * @dev: drm device
> >   */
> > -void intel_init_audio(struct drm_device *dev)
> > +void intel_init_audio_callbacks(struct drm_i915_private *dev_priv)
> >  {
> > -	struct drm_i915_private *dev_priv = dev->dev_private;
> > -
> > -	if (IS_G4X(dev)) {
> > +	if (IS_G4X(dev_priv)) {
> >  		dev_priv->display.audio_codec_enable =
> > g4x_audio_codec_enable;
> >  		dev_priv->display.audio_codec_disable =
> > g4x_audio_codec_disable;
> > -	} else if (IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev)) {
> > +	} else if (IS_VALLEYVIEW(dev_priv) ||
> > IS_CHERRYVIEW(dev_priv)) {
> >  		dev_priv->display.audio_codec_enable =
> > ilk_audio_codec_enable;
> >  		dev_priv->display.audio_codec_disable =
> > ilk_audio_codec_disable;
> > -	} else if (IS_HASWELL(dev) || INTEL_INFO(dev)->gen >= 8) {
> > +	} else if (IS_HASWELL(dev_priv) || INTEL_INFO(dev_priv)-
> > >gen >= 8) {
> >  		dev_priv->display.audio_codec_enable =
> > hsw_audio_codec_enable;
> >  		dev_priv->display.audio_codec_disable =
> > hsw_audio_codec_disable;
> > -	} else if (HAS_PCH_SPLIT(dev)) {
> > +	} else if (HAS_PCH_SPLIT(dev_priv)) {
> >  		dev_priv->display.audio_codec_enable =
> > ilk_audio_codec_enable;
> >  		dev_priv->display.audio_codec_disable =
> > ilk_audio_codec_disable;
> >  	}
> > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > b/drivers/gpu/drm/i915/intel_display.c
> > index 62d36a7..5170718 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -15095,22 +15095,20 @@ static const struct drm_mode_config_funcs
> > intel_mode_funcs = {
> >  };
> >  
> >  /* Set up chip specific display functions */
> > -static void intel_init_display(struct drm_device *dev)
> > +void intel_init_display_callbacks(struct drm_i915_private
> > *dev_priv)
> >  {
> > -	struct drm_i915_private *dev_priv = dev->dev_private;
> > -
> > -	if (HAS_PCH_SPLIT(dev) || IS_G4X(dev))
> > +	if (HAS_PCH_SPLIT(dev_priv) || IS_G4X(dev_priv))
> >  		dev_priv->display.find_dpll = g4x_find_best_dpll;
> > -	else if (IS_CHERRYVIEW(dev))
> > +	else if (IS_CHERRYVIEW(dev_priv))
> >  		dev_priv->display.find_dpll = chv_find_best_dpll;
> > -	else if (IS_VALLEYVIEW(dev))
> > +	else if (IS_VALLEYVIEW(dev_priv))
> >  		dev_priv->display.find_dpll = vlv_find_best_dpll;
> > -	else if (IS_PINEVIEW(dev))
> > +	else if (IS_PINEVIEW(dev_priv))
> >  		dev_priv->display.find_dpll = pnv_find_best_dpll;
> >  	else
> >  		dev_priv->display.find_dpll = i9xx_find_best_dpll;
> >  
> > -	if (INTEL_INFO(dev)->gen >= 9) {
> > +	if (INTEL_INFO(dev_priv)->gen >= 9) {
> >  		dev_priv->display.get_pipe_config =
> > haswell_get_pipe_config;
> >  		dev_priv->display.get_initial_plane_config =
> >  			skylake_get_initial_plane_config;
> > @@ -15118,7 +15116,7 @@ static void intel_init_display(struct
> > drm_device *dev)
> >  			haswell_crtc_compute_clock;
> >  		dev_priv->display.crtc_enable =
> > haswell_crtc_enable;
> >  		dev_priv->display.crtc_disable =
> > haswell_crtc_disable;
> > -	} else if (HAS_DDI(dev)) {
> > +	} else if (HAS_DDI(dev_priv)) {
> >  		dev_priv->display.get_pipe_config =
> > haswell_get_pipe_config;
> >  		dev_priv->display.get_initial_plane_config =
> >  			ironlake_get_initial_plane_config;
> > @@ -15126,7 +15124,7 @@ static void intel_init_display(struct
> > drm_device *dev)
> >  			haswell_crtc_compute_clock;
> >  		dev_priv->display.crtc_enable =
> > haswell_crtc_enable;
> >  		dev_priv->display.crtc_disable =
> > haswell_crtc_disable;
> > -	} else if (HAS_PCH_SPLIT(dev)) {
> > +	} else if (HAS_PCH_SPLIT(dev_priv)) {
> >  		dev_priv->display.get_pipe_config =
> > ironlake_get_pipe_config;
> >  		dev_priv->display.get_initial_plane_config =
> >  			ironlake_get_initial_plane_config;
> > @@ -15134,7 +15132,7 @@ static void intel_init_display(struct
> > drm_device *dev)
> >  			ironlake_crtc_compute_clock;
> >  		dev_priv->display.crtc_enable =
> > ironlake_crtc_enable;
> >  		dev_priv->display.crtc_disable =
> > ironlake_crtc_disable;
> > -	} else if (IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev)) {
> > +	} else if (IS_VALLEYVIEW(dev_priv) ||
> > IS_CHERRYVIEW(dev_priv)) {
> >  		dev_priv->display.get_pipe_config =
> > i9xx_get_pipe_config;
> >  		dev_priv->display.get_initial_plane_config =
> >  			i9xx_get_initial_plane_config;
> > @@ -15151,89 +15149,89 @@ static void intel_init_display(struct
> > drm_device *dev)
> >  	}
> >  
> >  	/* Returns the core display clock speed */
> > -	if (IS_SKYLAKE(dev) || IS_KABYLAKE(dev))
> > +	if (IS_SKYLAKE(dev_priv) || IS_KABYLAKE(dev_priv))
> >  		dev_priv->display.get_display_clock_speed =
> >  			skylake_get_display_clock_speed;
> > -	else if (IS_BROXTON(dev))
> > +	else if (IS_BROXTON(dev_priv))
> >  		dev_priv->display.get_display_clock_speed =
> >  			broxton_get_display_clock_speed;
> > -	else if (IS_BROADWELL(dev))
> > +	else if (IS_BROADWELL(dev_priv))
> >  		dev_priv->display.get_display_clock_speed =
> >  			broadwell_get_display_clock_speed;
> > -	else if (IS_HASWELL(dev))
> > +	else if (IS_HASWELL(dev_priv))
> >  		dev_priv->display.get_display_clock_speed =
> >  			haswell_get_display_clock_speed;
> > -	else if (IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev))
> > +	else if (IS_VALLEYVIEW(dev_priv) ||
> > IS_CHERRYVIEW(dev_priv))
> >  		dev_priv->display.get_display_clock_speed =
> >  			valleyview_get_display_clock_speed;
> > -	else if (IS_GEN5(dev))
> > +	else if (IS_GEN5(dev_priv))
> >  		dev_priv->display.get_display_clock_speed =
> >  			ilk_get_display_clock_speed;
> > -	else if (IS_I945G(dev) || IS_BROADWATER(dev) ||
> > -		 IS_GEN6(dev) || IS_IVYBRIDGE(dev))
> > +	else if (IS_I945G(dev_priv) || IS_BROADWATER(dev_priv) ||
> > +		 IS_GEN6(dev_priv) || IS_IVYBRIDGE(dev_priv))
> >  		dev_priv->display.get_display_clock_speed =
> >  			i945_get_display_clock_speed;
> > -	else if (IS_GM45(dev))
> > +	else if (IS_GM45(dev_priv))
> >  		dev_priv->display.get_display_clock_speed =
> >  			gm45_get_display_clock_speed;
> > -	else if (IS_CRESTLINE(dev))
> > +	else if (IS_CRESTLINE(dev_priv))
> >  		dev_priv->display.get_display_clock_speed =
> >  			i965gm_get_display_clock_speed;
> > -	else if (IS_PINEVIEW(dev))
> > +	else if (IS_PINEVIEW(dev_priv))
> >  		dev_priv->display.get_display_clock_speed =
> >  			pnv_get_display_clock_speed;
> > -	else if (IS_G33(dev) || IS_G4X(dev))
> > +	else if (IS_G33(dev_priv) || IS_G4X(dev_priv))
> >  		dev_priv->display.get_display_clock_speed =
> >  			g33_get_display_clock_speed;
> > -	else if (IS_I915G(dev))
> > +	else if (IS_I915G(dev_priv))
> >  		dev_priv->display.get_display_clock_speed =
> >  			i915_get_display_clock_speed;
> > -	else if (IS_I945GM(dev) || IS_845G(dev))
> > +	else if (IS_I945GM(dev_priv) || IS_845G(dev_priv))
> >  		dev_priv->display.get_display_clock_speed =
> >  			i9xx_misc_get_display_clock_speed;
> > -	else if (IS_I915GM(dev))
> > +	else if (IS_I915GM(dev_priv))
> >  		dev_priv->display.get_display_clock_speed =
> >  			i915gm_get_display_clock_speed;
> > -	else if (IS_I865G(dev))
> > +	else if (IS_I865G(dev_priv))
> >  		dev_priv->display.get_display_clock_speed =
> >  			i865_get_display_clock_speed;
> > -	else if (IS_I85X(dev))
> > +	else if (IS_I85X(dev_priv))
> >  		dev_priv->display.get_display_clock_speed =
> >  			i85x_get_display_clock_speed;
> >  	else { /* 830 */
> > -		WARN(!IS_I830(dev), "Unknown platform. Assuming
> > 133 MHz CDCLK\n");
> > +		WARN(!IS_I830(dev_priv), "Unknown platform.
> > Assuming 133 MHz CDCLK\n");
> >  		dev_priv->display.get_display_clock_speed =
> >  			i830_get_display_clock_speed;
> >  	}
> >  
> > -	if (IS_GEN5(dev)) {
> > +	if (IS_GEN5(dev_priv)) {
> >  		dev_priv->display.fdi_link_train =
> > ironlake_fdi_link_train;
> > -	} else if (IS_GEN6(dev)) {
> > +	} else if (IS_GEN6(dev_priv)) {
> >  		dev_priv->display.fdi_link_train =
> > gen6_fdi_link_train;
> > -	} else if (IS_IVYBRIDGE(dev)) {
> > +	} else if (IS_IVYBRIDGE(dev_priv)) {
> >  		/* FIXME: detect B0+ stepping and use auto
> > training */
> >  		dev_priv->display.fdi_link_train =
> > ivb_manual_fdi_link_train;
> > -	} else if (IS_HASWELL(dev) || IS_BROADWELL(dev)) {
> > +	} else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
> > {
> >  		dev_priv->display.fdi_link_train =
> > hsw_fdi_link_train;
> > -		if (IS_BROADWELL(dev)) {
> > +		if (IS_BROADWELL(dev_priv)) {
> >  			dev_priv->display.modeset_commit_cdclk =
> >  				broadwell_modeset_commit_cdclk;
> >  			dev_priv->display.modeset_calc_cdclk =
> >  				broadwell_modeset_calc_cdclk;
> >  		}
> > -	} else if (IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev)) {
> > +	} else if (IS_VALLEYVIEW(dev_priv) ||
> > IS_CHERRYVIEW(dev_priv)) {
> >  		dev_priv->display.modeset_commit_cdclk =
> >  			valleyview_modeset_commit_cdclk;
> >  		dev_priv->display.modeset_calc_cdclk =
> >  			valleyview_modeset_calc_cdclk;
> > -	} else if (IS_BROXTON(dev)) {
> > +	} else if (IS_BROXTON(dev_priv)) {
> >  		dev_priv->display.modeset_commit_cdclk =
> >  			broxton_modeset_commit_cdclk;
> >  		dev_priv->display.modeset_calc_cdclk =
> >  			broxton_modeset_calc_cdclk;
> >  	}
> >  
> > -	switch (INTEL_INFO(dev)->gen) {
> > +	switch (INTEL_INFO(dev_priv)->gen) {
> >  	case 2:
> >  		dev_priv->display.queue_flip =
> > intel_gen2_queue_flip;
> >  		break;
> > @@ -15260,8 +15258,6 @@ static void intel_init_display(struct
> > drm_device *dev)
> >  		/* Default just returns -ENODEV to indicate
> > unsupported */
> >  		dev_priv->display.queue_flip =
> > intel_default_queue_flip;
> >  	}
> > -
> > -	mutex_init(&dev_priv->pps_mutex);
> >  }
> >  
> >  /*
> > @@ -15588,9 +15584,6 @@ void intel_modeset_init(struct drm_device
> > *dev)
> >  		}
> >  	}
> >  
> > -	intel_init_display(dev);
> > -	intel_init_audio(dev);
> > -
> >  	if (IS_GEN2(dev)) {
> >  		dev->mode_config.max_width = 2048;
> >  		dev->mode_config.max_height = 2048;
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h
> > b/drivers/gpu/drm/i915/intel_drv.h
> > index 7b2d66d..5264901 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -1110,7 +1110,7 @@ u32 intel_fb_stride_alignment(const struct
> > drm_i915_private *dev_priv,
> >  			      uint64_t fb_modifier, uint32_t
> > pixel_format);
> >  
> >  /* intel_audio.c */
> > -void intel_init_audio(struct drm_device *dev);
> > +void intel_init_audio_callbacks(struct drm_i915_private
> > *dev_priv);
> >  void intel_audio_codec_enable(struct intel_encoder *encoder);
> >  void intel_audio_codec_disable(struct intel_encoder *encoder);
> >  void i915_audio_component_init(struct drm_i915_private *dev_priv);
> > @@ -1118,6 +1118,7 @@ void i915_audio_component_cleanup(struct
> > drm_i915_private *dev_priv);
> >  
> >  /* intel_display.c */
> >  extern const struct drm_plane_funcs intel_plane_funcs;
> > +void intel_init_display_callbacks(struct drm_i915_private
> > *dev_priv);
> >  unsigned int intel_rotation_info_size(const struct
> > intel_rotation_info *rot_info);
> >  bool intel_has_pending_fb_unpin(struct drm_device *dev);
> >  void intel_mark_busy(struct drm_device *dev);
> 


More information about the Intel-gfx mailing list