Re: [Intel-gfx] [PATCH 8/9] drm/i915: move more display related probe/remove stuff to display

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Tue, Feb 04, 2020 at 03:42:27PM +0200, Jani Nikula wrote:
> With the intel_display_* prove/remove functions clarified, we can

"probe", though I wouldn't mind functions that prove the
display code/hw works ;)

> continue with moving more related calls to the right layer:
> 
> - drm_vblank_init()
> - intel_bios_init() and intel_bios_driver_remove()
> - intel_vga_register() and intel_vga_unregister()
> - intel_csr_ucode_init() and intel_csr_ucode_fini()
> 
> Unfortunately, for the time being, we also need to move a call to the
> *wrong* layer: the power domain init.
> 
> v2: move probe failure while at it, power domain init
> 
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display.c | 31 ++++++++++++++++
>  drivers/gpu/drm/i915/i915_drv.c              | 39 +-------------------
>  2 files changed, 32 insertions(+), 38 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 39c12a608103..bf99964d8e4c 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -66,6 +66,7 @@
>  #include "intel_bw.h"
>  #include "intel_cdclk.h"
>  #include "intel_color.h"
> +#include "intel_csr.h"
>  #include "intel_display_types.h"
>  #include "intel_dp_link_training.h"
>  #include "intel_fbc.h"
> @@ -17859,6 +17860,27 @@ int intel_modeset_init_noirq(struct drm_i915_private *i915)
>  {
>  	int ret;
>  
> +	if (i915_inject_probe_failure(i915))
> +		return -ENODEV;
> +
> +	if (HAS_DISPLAY(i915) && INTEL_DISPLAY_ENABLED(i915)) {
> +		ret = drm_vblank_init(&i915->drm,
> +				      INTEL_NUM_PIPES(i915));
> +		if (ret)
> +			goto out;
> +	}
> +
> +	intel_bios_init(i915);
> +
> +	ret = intel_vga_register(i915);
> +	if (ret)
> +		goto out;
> +
> +	/* FIXME: completely on the wrong abstraction layer */
> +	intel_power_domains_init_hw(i915, false);
> +
> +	intel_csr_ucode_init(i915);
> +
>  	i915->modeset_wq = alloc_ordered_workqueue("i915_modeset", 0);
>  	i915->flip_wq = alloc_workqueue("i915_flip", WQ_HIGHPRI |
>  					WQ_UNBOUND, WQ_UNBOUND_MAX_ACTIVE);
> @@ -17882,6 +17904,9 @@ int intel_modeset_init_noirq(struct drm_i915_private *i915)
>  	intel_fbc_init(i915);
>  
>  	return 0;
> +
> +out:
> +	return ret;

Empty label seems a bit pointless. But I guess we're at least missing a
intel_bios_driver_remove() call here. Hmm, the remove doesn't seem to be
the reverse of this either. Oh well.

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

>  }
>  
>  /* part #2: call after irq install, but before gem init */
> @@ -18905,6 +18930,12 @@ void intel_modeset_driver_remove_noirq(struct drm_i915_private *i915)
>  	destroy_workqueue(i915->modeset_wq);
>  
>  	intel_fbc_cleanup_cfb(i915);
> +
> +	intel_bios_driver_remove(i915);
> +
> +	intel_vga_unregister(i915);
> +
> +	intel_csr_ucode_fini(i915);
>  }
>  
>  #if IS_ENABLED(CONFIG_DRM_I915_CAPTURE_ERROR)
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 21f07709dc9e..fca693ad6cd7 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -275,38 +275,7 @@ intel_teardown_mchbar(struct drm_i915_private *dev_priv)
>  /* part #1: call before irq install */
>  static int i915_driver_modeset_probe_noirq(struct drm_i915_private *i915)
>  {
> -	int ret;
> -
> -	if (i915_inject_probe_failure(i915))
> -		return -ENODEV;
> -
> -	if (HAS_DISPLAY(i915) && INTEL_DISPLAY_ENABLED(i915)) {
> -		ret = drm_vblank_init(&i915->drm,
> -				      INTEL_NUM_PIPES(i915));
> -		if (ret)
> -			goto out;
> -	}
> -
> -	intel_bios_init(i915);
> -
> -	ret = intel_vga_register(i915);
> -	if (ret)
> -		goto out;
> -
> -	intel_power_domains_init_hw(i915, false);
> -
> -	intel_csr_ucode_init(i915);
> -
> -	ret = intel_modeset_init_noirq(i915);
> -	if (ret)
> -		goto cleanup_vga_client;
> -
> -	return 0;
> -
> -cleanup_vga_client:
> -	intel_vga_unregister(i915);
> -out:
> -	return ret;
> +	return intel_modeset_init_noirq(i915);
>  }
>  
>  /* part #2: call after irq install */
> @@ -353,12 +322,6 @@ static void i915_driver_modeset_remove(struct drm_i915_private *i915)
>  static void i915_driver_modeset_remove_noirq(struct drm_i915_private *i915)
>  {
>  	intel_modeset_driver_remove_noirq(i915);
> -
> -	intel_bios_driver_remove(i915);
> -
> -	intel_vga_unregister(i915);
> -
> -	intel_csr_ucode_fini(i915);
>  }
>  
>  static void intel_init_dpio(struct drm_i915_private *dev_priv)
> -- 
> 2.20.1

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx