On pe, 2016-03-11 at 16:00 +0200, Jani Nikula wrote:
> On Wed, 09 Mar 2016, Imre Deak <imre.deak@xxxxxxxxx> 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@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;
> > }
> >
> > - 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
