Re: [PATCH 2/2] drm/i915/opregion: Rename init/fini functions to register/unregister

[Jani Nikula <jani.nikula@linux.intel.com>]

On Mon, 23 May 2016, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> Current intel_opregion_init is called during the driver registration
> phase and intel_opregion_fini from the unregistration phase. Rename the
> functions show that this is clear from their names. The phases tell us
> what we expect the existing hw state to be, e.g. whether interrupts are
> still enabled etc.

Okay, for the naming per se,

Reviewed-by: Jani Nikula <jani.nikula@intel.com>

While not a problem in this patch, the whole init/cleanup of opregion is
annoyingly asymmetric. You need to call both setup and init to make it
work, but fini cleans up for both of them. So repeated init/fini pairs
will fail. The setup also does some initialization that is only needed
once (like INIT_WORK) so fini is not a complete counter-operation of
setup+init either.

BR,
Jani.

>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Jani Nikula <jani.nikula@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_dma.c       | 4 ++--
>  drivers/gpu/drm/i915/i915_drv.c       | 4 ++--
>  drivers/gpu/drm/i915/i915_drv.h       | 4 ++--
>  drivers/gpu/drm/i915/intel_opregion.c | 4 ++--
>  4 files changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> index 363bd5884a56..07edaed9d5a2 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -1376,7 +1376,7 @@ static void i915_driver_register(struct drm_i915_private *dev_priv)
>  
>  	if (INTEL_INFO(dev_priv)->num_pipes) {
>  		/* Must be done after probing outputs */
> -		intel_opregion_init(dev_priv);
> +		intel_opregion_register(dev_priv);
>  		acpi_video_register();
>  	}
>  
> @@ -1395,7 +1395,7 @@ static void i915_driver_unregister(struct drm_i915_private *dev_priv)
>  	i915_audio_component_cleanup(dev_priv);
>  	intel_gpu_ips_teardown();
>  	acpi_video_unregister();
> -	intel_opregion_fini(dev_priv);
> +	intel_opregion_unregister(dev_priv);
>  	i915_teardown_sysfs(dev_priv->dev);
>  	i915_gem_shrinker_cleanup(dev_priv);
>  }
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 7627bbec8e37..943d7b222fd4 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -631,7 +631,7 @@ static int i915_drm_suspend(struct drm_device *dev)
>  	intel_opregion_notify_adapter(dev_priv, opregion_target_state);
>  
>  	intel_uncore_forcewake_reset(dev_priv, false);
> -	intel_opregion_fini(dev_priv);
> +	intel_opregion_unregister(dev_priv);
>  
>  	intel_fbdev_set_suspend(dev, FBINFO_STATE_SUSPENDED, true);
>  
> @@ -794,7 +794,7 @@ static int i915_drm_resume(struct drm_device *dev)
>  	/* Config may have changed between suspend and resume */
>  	drm_helper_hpd_irq_event(dev);
>  
> -	intel_opregion_init(dev_priv);
> +	intel_opregion_register(dev_priv);
>  
>  	intel_fbdev_set_suspend(dev, FBINFO_STATE_RUNNING, false);
>  
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index caf7e45ae663..17fe272e9ef8 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -3609,8 +3609,8 @@ bool intel_bios_is_port_hpd_inverted(struct drm_i915_private *dev_priv,
>  /* intel_opregion.c */
>  #ifdef CONFIG_ACPI
>  extern int intel_opregion_setup(struct drm_i915_private *dev_priv);
> -extern void intel_opregion_init(struct drm_i915_private *dev_priv);
> -extern void intel_opregion_fini(struct drm_i915_private *dev_priv);
> +extern void intel_opregion_register(struct drm_i915_private *dev_priv);
> +extern void intel_opregion_unregister(struct drm_i915_private *dev_priv);
>  extern void intel_opregion_asle_intr(struct drm_i915_private *dev_priv);
>  extern int intel_opregion_notify_encoder(struct intel_encoder *intel_encoder,
>  					 bool enable);
> diff --git a/drivers/gpu/drm/i915/intel_opregion.c b/drivers/gpu/drm/i915/intel_opregion.c
> index f9cdec866e49..f6d8a21d2c49 100644
> --- a/drivers/gpu/drm/i915/intel_opregion.c
> +++ b/drivers/gpu/drm/i915/intel_opregion.c
> @@ -778,7 +778,7 @@ static void intel_setup_cadls(struct drm_i915_private *dev_priv)
>  	} while (++i < 8 && disp_id != 0);
>  }
>  
> -void intel_opregion_init(struct drm_i915_private *dev_priv)
> +void intel_opregion_register(struct drm_i915_private *dev_priv)
>  {
>  	struct intel_opregion *opregion = &dev_priv->opregion;
>  
> @@ -805,7 +805,7 @@ void intel_opregion_init(struct drm_i915_private *dev_priv)
>  	}
>  }
>  
> -void intel_opregion_fini(struct drm_i915_private *dev_priv)
> +void intel_opregion_unregister(struct drm_i915_private *dev_priv)
>  {
>  	struct intel_opregion *opregion = &dev_priv->opregion;

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx