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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, 2016-03-09 at 17:31 +0200, Imre Deak 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.
> 
> Signed-off-by: Imre Deak <imre.deak@xxxxxxxxx>
> ---
>  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;
>  	}

Would it make sense to change these if ladders into a structs of function
pointers and point to those from struct intel_device_info? The HAS_PCH_SPLIT()
check could be a bit tricky, but those usually go as

  if (HAS_DDI())
	...
  else if (HAS_PCH_SPLIT())
	...,

so they usually mean ILK, SNB and IVB.

I'm not saying this should be part of this series, but just wanted to throw the
idea out there.


Ander


>  
> -	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);
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux