On Thu, Nov 04, 2021 at 06:18:57PM +0200, Jani Nikula wrote:
> Unify audio init/cleanup paths wrt LPE audio, and base the logic on the
> return values from LPE audio calls. Move the platform device check on
> cleanup to intel_lpe_audio.c, thereby limiting all audio.lpe substruct
> access to that file.
>
> Signed-off-by: Jani Nikula <jani.nikula@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/display/intel_audio.c | 14 ++++++++------
> drivers/gpu/drm/i915/display/intel_lpe_audio.c | 6 ++++--
> drivers/gpu/drm/i915/display/intel_lpe_audio.h | 4 ++--
> 3 files changed, 14 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_audio.c b/drivers/gpu/drm/i915/display/intel_audio.c
> index 24e76657d561..aa7037021376 100644
> --- a/drivers/gpu/drm/i915/display/intel_audio.c
> +++ b/drivers/gpu/drm/i915/display/intel_audio.c
> @@ -1403,8 +1403,10 @@ static void i915_audio_component_cleanup(struct drm_i915_private *dev_priv)
> */
> void intel_audio_init(struct drm_i915_private *dev_priv)
> {
> - if (intel_lpe_audio_init(dev_priv) < 0)
> - i915_audio_component_init(dev_priv);
> + if (!intel_lpe_audio_init(dev_priv))
> + return;
> +
> + i915_audio_component_init(dev_priv);
The logic here is already a bit funky. Technically we should not
init the component stuff except when LPE audio is not present.
Ie. we should only do it when intel_lpe_audio_init() returns
-ENODEV.
> }
>
> /**
> @@ -1414,8 +1416,8 @@ void intel_audio_init(struct drm_i915_private *dev_priv)
> */
> void intel_audio_deinit(struct drm_i915_private *dev_priv)
> {
> - if ((dev_priv)->audio.lpe.platdev != NULL)
> - intel_lpe_audio_teardown(dev_priv);
> - else
> - i915_audio_component_cleanup(dev_priv);
> + if (!intel_lpe_audio_teardown(dev_priv))
> + return;
> +
> + i915_audio_component_cleanup(dev_priv);
Here it would probably make more sense to just call both
unconditionally so we don't have to care what happened during
init.
> }
> diff --git a/drivers/gpu/drm/i915/display/intel_lpe_audio.c b/drivers/gpu/drm/i915/display/intel_lpe_audio.c
> index 4970bf146c4a..a2984718d136 100644
> --- a/drivers/gpu/drm/i915/display/intel_lpe_audio.c
> +++ b/drivers/gpu/drm/i915/display/intel_lpe_audio.c
> @@ -296,10 +296,10 @@ int intel_lpe_audio_init(struct drm_i915_private *dev_priv)
> *
> * release all the resources for LPE audio <-> i915 bridge.
> */
> -void intel_lpe_audio_teardown(struct drm_i915_private *dev_priv)
> +int intel_lpe_audio_teardown(struct drm_i915_private *dev_priv)
> {
> if (!HAS_LPE_AUDIO(dev_priv))
> - return;
> + return -ENODEV;
>
> lpe_audio_platdev_destroy(dev_priv);
>
> @@ -307,6 +307,8 @@ void intel_lpe_audio_teardown(struct drm_i915_private *dev_priv)
>
> dev_priv->audio.lpe.irq = -1;
> dev_priv->audio.lpe.platdev = NULL;
> +
> + return 0;
> }
>
> /**
> diff --git a/drivers/gpu/drm/i915/display/intel_lpe_audio.h b/drivers/gpu/drm/i915/display/intel_lpe_audio.h
> index f848c5038714..030874623872 100644
> --- a/drivers/gpu/drm/i915/display/intel_lpe_audio.h
> +++ b/drivers/gpu/drm/i915/display/intel_lpe_audio.h
> @@ -12,8 +12,8 @@ enum pipe;
> enum port;
> struct drm_i915_private;
>
> -int intel_lpe_audio_init(struct drm_i915_private *dev_priv);
> -void intel_lpe_audio_teardown(struct drm_i915_private *dev_priv);
> +int intel_lpe_audio_init(struct drm_i915_private *dev_priv);
> +int intel_lpe_audio_teardown(struct drm_i915_private *dev_priv);
> void intel_lpe_audio_irq_handler(struct drm_i915_private *dev_priv);
> void intel_lpe_audio_notify(struct drm_i915_private *dev_priv,
> enum pipe pipe, enum port port,
> --
> 2.30.2
--
Ville Syrjälä
Intel
