Re: [PATCH v2 3/3] drm/i915: skip display initialization when there is no display

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

 



On Mon, Mar 22, 2021 at 01:58:05PM -0700, José Roberto de Souza wrote:
> Display features should not be initialized or de-initialized when there
> is no display. Skip modeset initialization, output setup, plane, crtc,
> encoder, connector registration, display cdclk and rawclk
> initialization, display core initialization, etc.
> 
> Skip the functionality at as high level as possible, and remove any
> redundant checks. If the functionality is conditional to *other* display
> checks, do not add more. If the un-initialization has checks for
> initialization, do not add more.
> 
> We explicitly do not care about any GMCH/VLV/CHV code paths, as they've
> always had and will have display.
> 
> Cc: Lucas De Marchi <lucas.demarchi@xxxxxxxxx>
Reviewed-by: Radhakrishna Sripada <radhakrishna.sripada@xxxxxxxxx>

> Signed-off-by: José Roberto de Souza <jose.souza@xxxxxxxxx>
> Signed-off-by: Jani Nikula <jani.nikula@xxxxxxxxx>
> Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> ---
>  drivers/gpu/drm/i915/display/intel_display.c  | 44 +++++++++++++++----
>  .../drm/i915/display/intel_display_power.c    | 36 +++++++++++++++
>  drivers/gpu/drm/i915/display/intel_dp.c       |  6 +++
>  drivers/gpu/drm/i915/display/intel_gmbus.c    |  3 --
>  drivers/gpu/drm/i915/display/intel_hotplug.c  | 12 +++++
>  drivers/gpu/drm/i915/display/intel_pps.c      |  5 ++-
>  drivers/gpu/drm/i915/i915_drv.c               | 28 +++++++++---
>  drivers/gpu/drm/i915/i915_suspend.c           |  6 +++
>  8 files changed, 120 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 7b38b9a38b85..b10bf7e06ee7 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -1613,6 +1613,9 @@ u32 intel_plane_fb_max_stride(struct drm_i915_private *dev_priv,
>  	struct intel_crtc *crtc;
>  	struct intel_plane *plane;
>  
> +	if (!HAS_DISPLAY(dev_priv))
> +		return 0;
> +
>  	/*
>  	 * We assume the primary plane for pipe A has
>  	 * the highest stride limits of them all,
> @@ -4629,6 +4632,9 @@ int intel_display_suspend(struct drm_device *dev)
>  	struct drm_atomic_state *state;
>  	int ret;
>  
> +	if (!HAS_DISPLAY(dev_priv))
> +		return 0;
> +
>  	state = drm_atomic_helper_suspend(dev);
>  	ret = PTR_ERR_OR_ZERO(state);
>  	if (ret)
> @@ -12239,6 +12245,9 @@ static const struct drm_mode_config_funcs intel_mode_funcs = {
>   */
>  void intel_init_display_hooks(struct drm_i915_private *dev_priv)
>  {
> +	if (!HAS_DISPLAY(dev_priv))
> +		return;
> +
>  	intel_init_cdclk_hooks(dev_priv);
>  	intel_init_audio_hooks(dev_priv);
>  
> @@ -12281,8 +12290,12 @@ void intel_init_display_hooks(struct drm_i915_private *dev_priv)
>  
>  void intel_modeset_init_hw(struct drm_i915_private *i915)
>  {
> -	struct intel_cdclk_state *cdclk_state =
> -		to_intel_cdclk_state(i915->cdclk.obj.state);
> +	struct intel_cdclk_state *cdclk_state;
> +
> +	if (!HAS_DISPLAY(i915))
> +		return;
> +
> +	cdclk_state = to_intel_cdclk_state(i915->cdclk.obj.state);
>  
>  	intel_update_cdclk(i915);
>  	intel_dump_cdclk_config(&i915->cdclk.hw, "Current CDCLK");
> @@ -12598,6 +12611,9 @@ int intel_modeset_init_noirq(struct drm_i915_private *i915)
>  	/* FIXME: completely on the wrong abstraction layer */
>  	intel_power_domains_init_hw(i915, false);
>  
> +	if (!HAS_DISPLAY(i915))
> +		return 0;
> +
>  	intel_csr_ucode_init(i915);
>  
>  	i915->modeset_wq = alloc_ordered_workqueue("i915_modeset", 0);
> @@ -12648,6 +12664,9 @@ int intel_modeset_init_nogem(struct drm_i915_private *i915)
>  	struct intel_crtc *crtc;
>  	int ret;
>  
> +	if (!HAS_DISPLAY(i915))
> +		return 0;
> +
>  	intel_init_pm(i915);
>  
>  	intel_panel_sanitize_ssc(i915);
> @@ -12660,13 +12679,11 @@ int intel_modeset_init_nogem(struct drm_i915_private *i915)
>  		    INTEL_NUM_PIPES(i915),
>  		    INTEL_NUM_PIPES(i915) > 1 ? "s" : "");
>  
> -	if (HAS_DISPLAY(i915)) {
> -		for_each_pipe(i915, pipe) {
> -			ret = intel_crtc_init(i915, pipe);
> -			if (ret) {
> -				intel_mode_config_cleanup(i915);
> -				return ret;
> -			}
> +	for_each_pipe(i915, pipe) {
> +		ret = intel_crtc_init(i915, pipe);
> +		if (ret) {
> +			intel_mode_config_cleanup(i915);
> +			return ret;
>  		}
>  	}
>  
> @@ -13602,6 +13619,9 @@ void intel_display_resume(struct drm_device *dev)
>  	struct drm_modeset_acquire_ctx ctx;
>  	int ret;
>  
> +	if (!HAS_DISPLAY(dev_priv))
> +		return;
> +
>  	dev_priv->modeset_restore_state = NULL;
>  	if (state)
>  		state->acquire_ctx = &ctx;
> @@ -13651,6 +13671,9 @@ static void intel_hpd_poll_fini(struct drm_i915_private *i915)
>  /* part #1: call before irq uninstall */
>  void intel_modeset_driver_remove(struct drm_i915_private *i915)
>  {
> +	if (!HAS_DISPLAY(i915))
> +		return;
> +
>  	flush_workqueue(i915->flip_wq);
>  	flush_workqueue(i915->modeset_wq);
>  
> @@ -13661,6 +13684,9 @@ void intel_modeset_driver_remove(struct drm_i915_private *i915)
>  /* part #2: call after irq uninstall */
>  void intel_modeset_driver_remove_noirq(struct drm_i915_private *i915)
>  {
> +	if (!HAS_DISPLAY(i915))
> +		return;
> +
>  	/*
>  	 * Due to the hpd irq storm handling the hotplug work can re-arm the
>  	 * poll handlers. Hence disable polling after hpd handling is shut down.
> diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c
> index e6a3b3e6b1f7..efacd6aba3d0 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_power.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_power.c
> @@ -821,6 +821,9 @@ static void gen9_sanitize_dc_state(struct drm_i915_private *dev_priv)
>  {
>  	u32 val;
>  
> +	if (!HAS_DISPLAY(dev_priv))
> +		return;
> +
>  	val = intel_de_read(dev_priv, DC_STATE_EN) & gen9_dc_mask(dev_priv);
>  
>  	drm_dbg_kms(&dev_priv->drm,
> @@ -857,6 +860,9 @@ static void gen9_set_dc_state(struct drm_i915_private *dev_priv, u32 state)
>  	u32 val;
>  	u32 mask;
>  
> +	if (!HAS_DISPLAY(dev_priv))
> +		return;
> +
>  	if (drm_WARN_ON_ONCE(&dev_priv->drm,
>  			     state & ~dev_priv->csr.allowed_dc_mask))
>  		state &= dev_priv->csr.allowed_dc_mask;
> @@ -1181,6 +1187,9 @@ static void gen9_disable_dc_states(struct drm_i915_private *dev_priv)
>  
>  	gen9_set_dc_state(dev_priv, DC_STATE_DISABLE);
>  
> +	if (!HAS_DISPLAY(dev_priv))
> +		return;
> +
>  	dev_priv->display.get_cdclk(dev_priv, &cdclk_config);
>  	/* Can't read out voltage_level so can't use intel_cdclk_changed() */
>  	drm_WARN_ON(&dev_priv->drm,
> @@ -4533,6 +4542,9 @@ static u32 get_allowed_dc_mask(const struct drm_i915_private *dev_priv,
>  	int requested_dc;
>  	int max_dc;
>  
> +	if (!HAS_DISPLAY(dev_priv))
> +		return 0;
> +
>  	if (IS_DG1(dev_priv))
>  		max_dc = 3;
>  	else if (INTEL_GEN(dev_priv) >= 12)
> @@ -5125,6 +5137,9 @@ static void skl_display_core_init(struct drm_i915_private *dev_priv,
>  	/* enable PCH reset handshake */
>  	intel_pch_reset_handshake(dev_priv, !HAS_PCH_NOP(dev_priv));
>  
> +	if (!HAS_DISPLAY(dev_priv))
> +		return;
> +
>  	/* enable PG1 and Misc I/O */
>  	mutex_lock(&power_domains->lock);
>  
> @@ -5149,6 +5164,9 @@ static void skl_display_core_uninit(struct drm_i915_private *dev_priv)
>  	struct i915_power_domains *power_domains = &dev_priv->power_domains;
>  	struct i915_power_well *well;
>  
> +	if (!HAS_DISPLAY(dev_priv))
> +		return;
> +
>  	gen9_disable_dc_states(dev_priv);
>  
>  	gen9_dbuf_disable(dev_priv);
> @@ -5189,6 +5207,9 @@ static void bxt_display_core_init(struct drm_i915_private *dev_priv, bool resume
>  	 */
>  	intel_pch_reset_handshake(dev_priv, false);
>  
> +	if (!HAS_DISPLAY(dev_priv))
> +		return;
> +
>  	/* Enable PG1 */
>  	mutex_lock(&power_domains->lock);
>  
> @@ -5210,6 +5231,9 @@ static void bxt_display_core_uninit(struct drm_i915_private *dev_priv)
>  	struct i915_power_domains *power_domains = &dev_priv->power_domains;
>  	struct i915_power_well *well;
>  
> +	if (!HAS_DISPLAY(dev_priv))
> +		return;
> +
>  	gen9_disable_dc_states(dev_priv);
>  
>  	gen9_dbuf_disable(dev_priv);
> @@ -5243,6 +5267,9 @@ static void cnl_display_core_init(struct drm_i915_private *dev_priv, bool resume
>  	/* 1. Enable PCH Reset Handshake */
>  	intel_pch_reset_handshake(dev_priv, !HAS_PCH_NOP(dev_priv));
>  
> +	if (!HAS_DISPLAY(dev_priv))
> +		return;
> +
>  	/* 2-3. */
>  	intel_combo_phy_init(dev_priv);
>  
> @@ -5270,6 +5297,9 @@ static void cnl_display_core_uninit(struct drm_i915_private *dev_priv)
>  	struct i915_power_domains *power_domains = &dev_priv->power_domains;
>  	struct i915_power_well *well;
>  
> +	if (!HAS_DISPLAY(dev_priv))
> +		return;
> +
>  	gen9_disable_dc_states(dev_priv);
>  
>  	/* 1. Disable all display engine functions -> aready done */
> @@ -5384,6 +5414,9 @@ static void icl_display_core_init(struct drm_i915_private *dev_priv,
>  	/* 1. Enable PCH reset handshake. */
>  	intel_pch_reset_handshake(dev_priv, !HAS_PCH_NOP(dev_priv));
>  
> +	if (!HAS_DISPLAY(dev_priv))
> +		return;
> +
>  	/* 2. Initialize all combo phys */
>  	intel_combo_phy_init(dev_priv);
>  
> @@ -5428,6 +5461,9 @@ static void icl_display_core_uninit(struct drm_i915_private *dev_priv)
>  	struct i915_power_domains *power_domains = &dev_priv->power_domains;
>  	struct i915_power_well *well;
>  
> +	if (!HAS_DISPLAY(dev_priv))
> +		return;
> +
>  	gen9_disable_dc_states(dev_priv);
>  
>  	/* 1. Disable all display engine functions -> aready done */
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index 1400c5b44c83..fc00bacd00c4 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -5427,6 +5427,9 @@ void intel_dp_mst_suspend(struct drm_i915_private *dev_priv)
>  {
>  	struct intel_encoder *encoder;
>  
> +	if (!HAS_DISPLAY(dev_priv))
> +		return;
> +
>  	for_each_intel_encoder(&dev_priv->drm, encoder) {
>  		struct intel_dp *intel_dp;
>  
> @@ -5447,6 +5450,9 @@ void intel_dp_mst_resume(struct drm_i915_private *dev_priv)
>  {
>  	struct intel_encoder *encoder;
>  
> +	if (!HAS_DISPLAY(dev_priv))
> +		return;
> +
>  	for_each_intel_encoder(&dev_priv->drm, encoder) {
>  		struct intel_dp *intel_dp;
>  		int ret;
> diff --git a/drivers/gpu/drm/i915/display/intel_gmbus.c b/drivers/gpu/drm/i915/display/intel_gmbus.c
> index 0c952e1d720e..5ee599d20050 100644
> --- a/drivers/gpu/drm/i915/display/intel_gmbus.c
> +++ b/drivers/gpu/drm/i915/display/intel_gmbus.c
> @@ -845,9 +845,6 @@ int intel_gmbus_setup(struct drm_i915_private *dev_priv)
>  	unsigned int pin;
>  	int ret;
>  
> -	if (!HAS_DISPLAY(dev_priv))
> -		return 0;
> -
>  	if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
>  		dev_priv->gpio_mmio_base = VLV_DISPLAY_BASE;
>  	else if (!HAS_GMCH(dev_priv))
> diff --git a/drivers/gpu/drm/i915/display/intel_hotplug.c b/drivers/gpu/drm/i915/display/intel_hotplug.c
> index f46a1b7190b8..47c85ac97c87 100644
> --- a/drivers/gpu/drm/i915/display/intel_hotplug.c
> +++ b/drivers/gpu/drm/i915/display/intel_hotplug.c
> @@ -595,6 +595,9 @@ void intel_hpd_init(struct drm_i915_private *dev_priv)
>  {
>  	int i;
>  
> +	if (!HAS_DISPLAY(dev_priv))
> +		return;
> +
>  	for_each_hpd_pin(i) {
>  		dev_priv->hotplug.stats[i].count = 0;
>  		dev_priv->hotplug.stats[i].state = HPD_ENABLED;
> @@ -670,6 +673,9 @@ static void i915_hpd_poll_init_work(struct work_struct *work)
>   */
>  void intel_hpd_poll_enable(struct drm_i915_private *dev_priv)
>  {
> +	if (!HAS_DISPLAY(dev_priv))
> +		return;
> +
>  	WRITE_ONCE(dev_priv->hotplug.poll_enabled, true);
>  
>  	/*
> @@ -702,6 +708,9 @@ void intel_hpd_poll_enable(struct drm_i915_private *dev_priv)
>   */
>  void intel_hpd_poll_disable(struct drm_i915_private *dev_priv)
>  {
> +	if (!HAS_DISPLAY(dev_priv))
> +		return;
> +
>  	WRITE_ONCE(dev_priv->hotplug.poll_enabled, false);
>  	schedule_work(&dev_priv->hotplug.poll_init_work);
>  }
> @@ -718,6 +727,9 @@ void intel_hpd_init_work(struct drm_i915_private *dev_priv)
>  
>  void intel_hpd_cancel_work(struct drm_i915_private *dev_priv)
>  {
> +	if (!HAS_DISPLAY(dev_priv))
> +		return;
> +
>  	spin_lock_irq(&dev_priv->irq_lock);
>  
>  	dev_priv->hotplug.long_port_mask = 0;
> diff --git a/drivers/gpu/drm/i915/display/intel_pps.c b/drivers/gpu/drm/i915/display/intel_pps.c
> index 932f56951914..e1aacd25c9d8 100644
> --- a/drivers/gpu/drm/i915/display/intel_pps.c
> +++ b/drivers/gpu/drm/i915/display/intel_pps.c
> @@ -319,6 +319,9 @@ void intel_pps_reset_all(struct drm_i915_private *dev_priv)
>  			  IS_GEN9_LP(dev_priv))))
>  		return;
>  
> +	if (!HAS_DISPLAY(dev_priv))
> +		return;
> +
>  	/*
>  	 * We can't grab pps_mutex here due to deadlock with power_domain
>  	 * mutex when power_domain functions are called while holding pps_mutex.
> @@ -1378,7 +1381,7 @@ void intel_pps_unlock_regs_wa(struct drm_i915_private *dev_priv)
>  	int pps_num;
>  	int pps_idx;
>  
> -	if (HAS_DDI(dev_priv))
> +	if (!HAS_DISPLAY(dev_priv) || HAS_DDI(dev_priv))
>  		return;
>  	/*
>  	 * This w/a is needed at least on CPT/PPT, but to be sure apply it
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 64edcab59fe1..ee27962a6044 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -977,8 +977,12 @@ static int i915_driver_open(struct drm_device *dev, struct drm_file *file)
>   */
>  static void i915_driver_lastclose(struct drm_device *dev)
>  {
> +	struct drm_i915_private *i915 = to_i915(dev);
> +
>  	intel_fbdev_restore_mode(dev);
> -	vga_switcheroo_process_delayed_switch();
> +
> +	if (HAS_DISPLAY(i915))
> +		vga_switcheroo_process_delayed_switch();
>  }
>  
>  static void i915_driver_postclose(struct drm_device *dev, struct drm_file *file)
> @@ -999,6 +1003,9 @@ static void intel_suspend_encoders(struct drm_i915_private *dev_priv)
>  	struct drm_device *dev = &dev_priv->drm;
>  	struct intel_encoder *encoder;
>  
> +	if (!HAS_DISPLAY(dev_priv))
> +		return;
> +
>  	drm_modeset_lock_all(dev);
>  	for_each_intel_encoder(dev, encoder)
>  		if (encoder->suspend)
> @@ -1011,6 +1018,9 @@ static void intel_shutdown_encoders(struct drm_i915_private *dev_priv)
>  	struct drm_device *dev = &dev_priv->drm;
>  	struct intel_encoder *encoder;
>  
> +	if (!HAS_DISPLAY(dev_priv))
> +		return;
> +
>  	drm_modeset_lock_all(dev);
>  	for_each_intel_encoder(dev, encoder)
>  		if (encoder->shutdown)
> @@ -1026,9 +1036,11 @@ void i915_driver_shutdown(struct drm_i915_private *i915)
>  
>  	i915_gem_suspend(i915);
>  
> -	drm_kms_helper_poll_disable(&i915->drm);
> +	if (HAS_DISPLAY(i915)) {
> +		drm_kms_helper_poll_disable(&i915->drm);
>  
> -	drm_atomic_helper_shutdown(&i915->drm);
> +		drm_atomic_helper_shutdown(&i915->drm);
> +	}
>  
>  	intel_dp_mst_suspend(i915);
>  
> @@ -1084,8 +1096,8 @@ static int i915_drm_suspend(struct drm_device *dev)
>  	/* We do a lot of poking in a lot of registers, make sure they work
>  	 * properly. */
>  	intel_power_domains_disable(dev_priv);
> -
> -	drm_kms_helper_poll_disable(dev);
> +	if (HAS_DISPLAY(dev_priv))
> +		drm_kms_helper_poll_disable(dev);
>  
>  	pci_save_state(pdev);
>  
> @@ -1232,7 +1244,8 @@ static int i915_drm_resume(struct drm_device *dev)
>  	 */
>  	intel_runtime_pm_enable_interrupts(dev_priv);
>  
> -	drm_mode_config_reset(dev);
> +	if (HAS_DISPLAY(dev_priv))
> +		drm_mode_config_reset(dev);
>  
>  	i915_gem_resume(dev_priv);
>  
> @@ -1245,7 +1258,8 @@ static int i915_drm_resume(struct drm_device *dev)
>  	intel_display_resume(dev);
>  
>  	intel_hpd_poll_disable(dev_priv);
> -	drm_kms_helper_poll_enable(dev);
> +	if (HAS_DISPLAY(dev_priv))
> +		drm_kms_helper_poll_enable(dev);
>  
>  	intel_opregion_resume(dev_priv);
>  
> diff --git a/drivers/gpu/drm/i915/i915_suspend.c b/drivers/gpu/drm/i915/i915_suspend.c
> index 0bc7b49f843c..5fcc32821e18 100644
> --- a/drivers/gpu/drm/i915/i915_suspend.c
> +++ b/drivers/gpu/drm/i915/i915_suspend.c
> @@ -87,6 +87,9 @@ void i915_save_display(struct drm_i915_private *dev_priv)
>  {
>  	struct pci_dev *pdev = to_pci_dev(dev_priv->drm.dev);
>  
> +	if (!HAS_DISPLAY(dev_priv))
> +		return;
> +
>  	/* Display arbitration control */
>  	if (INTEL_GEN(dev_priv) <= 4)
>  		dev_priv->regfile.saveDSPARB = intel_de_read(dev_priv, DSPARB);
> @@ -102,6 +105,9 @@ void i915_restore_display(struct drm_i915_private *dev_priv)
>  {
>  	struct pci_dev *pdev = to_pci_dev(dev_priv->drm.dev);
>  
> +	if (!HAS_DISPLAY(dev_priv))
> +		return;
> +
>  	intel_restore_swf(dev_priv);
>  
>  	if (IS_GEN(dev_priv, 4))
> -- 
> 2.31.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux