From: Imre Deak <imre.deak@intel.com>
To: Jani Nikula <jani.nikula@linux.intel.com>,
intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH 4/7] drm/i915: Move load time display/audio callback init earlier
Date: Fri, 11 Mar 2016 16:17:30 +0200 [thread overview]
Message-ID: <1457705850.20248.3.camel@intel.com> (raw)
In-Reply-To: <87r3fhnmgc.fsf@intel.com>
On pe, 2016-03-11 at 16:00 +0200, Jani Nikula wrote:
> On Wed, 09 Mar 2016, Imre Deak <imre.deak@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@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);
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
next prev parent reply other threads:[~2016-03-11 14:17 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-03-09 15:31 [PATCH 0/7] drm/i915: Move some load time init steps earlier Imre Deak
2016-03-09 15:31 ` [PATCH 1/7] drm/i915: Add comments marking the start of load time init phases Imre Deak
2016-03-09 15:31 ` [PATCH 2/7] drm/i915: Move laod time PCH detect, DPIO, power domain SW init earlier Imre Deak
2016-03-09 15:31 ` [PATCH 3/7] drm/i915: Move load time IRQ " Imre Deak
2016-03-09 15:31 ` [PATCH 4/7] drm/i915: Move load time display/audio callback " Imre Deak
2016-03-09 17:14 ` kbuild test robot
2016-03-10 8:58 ` Ander Conselvan De Oliveira
2016-03-10 11:24 ` Imre Deak
2016-03-11 14:00 ` Jani Nikula
2016-03-11 14:17 ` Imre Deak [this message]
2016-03-09 15:31 ` [PATCH 5/7] drm/i915: Move load time clock gating " Imre Deak
2016-03-09 15:57 ` Chris Wilson
2016-03-09 16:01 ` Imre Deak
2016-03-09 16:09 ` Chris Wilson
2016-03-09 15:31 ` [PATCH 6/7] drm/i915: Move load time runtime device info " Imre Deak
2016-03-09 15:31 ` [PATCH 7/7] drm/i915: Move load time runtime PM get later Imre Deak
2016-03-09 16:02 ` ✗ Fi.CI.BAT: warning for drm/i915: Move some load time init steps earlier Patchwork
2016-03-09 16:03 ` [PATCH 0/7] " Chris Wilson
2016-03-10 18:37 ` Imre Deak
2016-03-11 13:59 ` Jani Nikula
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1457705850.20248.3.camel@intel.com \
--to=imre.deak@intel.com \
--cc=intel-gfx@lists.freedesktop.org \
--cc=jani.nikula@linux.intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox