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 <[email protected]>
> ---
> 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
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
