From: Ander Conselvan De Oliveira <conselvan2@gmail.com>
To: Imre Deak <imre.deak@intel.com>, intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH 4/7] drm/i915: Move load time display/audio callback init earlier
Date: Thu, 10 Mar 2016 10:58:39 +0200 [thread overview]
Message-ID: <1457600319.4631.25.camel@gmail.com> (raw)
In-Reply-To: <1457537506-12455-5-git-send-email-imre.deak@intel.com>
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@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;
> }
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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
next prev parent reply other threads:[~2016-03-10 8:58 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 [this message]
2016-03-10 11:24 ` Imre Deak
2016-03-11 14:00 ` Jani Nikula
2016-03-11 14:17 ` Imre Deak
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=1457600319.4631.25.camel@gmail.com \
--to=conselvan2@gmail.com \
--cc=imre.deak@intel.com \
--cc=intel-gfx@lists.freedesktop.org \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.