Re: [PATCH] drm/i915: Ensure proper HDA suspend/resume ordering with a device link

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

On Thu, Oct 18, 2018 at 05:25:58PM +0300, Imre Deak wrote:
> In order to ensure that our system suspend and resume callbacks are
> called in the correct order wrt. those of the HDA driver add a device
> link to the HDA driver during audio component binding time. With i915 as
> the supplier and HDA as the consumer the PM framework will guarantee
> the HDA->i915 suspend (and shutdown) and i915->HDA resume order.
> 
> Atm, the lack of this ordering is not a problem, since all the i915
> suspend/resume steps that need to be ordered wrt. the HDA driver's
> suspend/resume steps are separated out to the i915
> suspend_late/resume_early hooks. That will change in a follow-up
> patchset where we'll need this ordering guarantee for steps that are in
> the i915 suspend/resume hooks (and which can't be moved to
> suspend_late/resume_early for other reasons). So this patch is a
> preparation for that follow-up patchset.
> 
> The change also allows us to move towards removing the i915
> suspend_late/resume_early hooks alltogether.
> 
> Since we only need to ensure the ordering during suspend/resume and not
> during driver probing create the link with DL_FLAG_STATELESS. Since the
> probe time ordering has to be optional we use the component framework
> for that.

And we manage runtime pm explicitly too so don't need/want
DL_FLAG_PM_RUNTIME.

> 
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Takashi Iwai <tiwai@suse.de>
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_audio.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
> index 769f3f586661..391b88701610 100644
> --- a/drivers/gpu/drm/i915/intel_audio.c
> +++ b/drivers/gpu/drm/i915/intel_audio.c
> @@ -921,6 +921,8 @@ static int i915_audio_component_bind(struct device *i915_kdev,
>  	dev_priv->audio_component = acomp;
>  	drm_modeset_unlock_all(&dev_priv->drm);
>  
> +	WARN_ON(!device_link_add(hda_kdev, i915_kdev, DL_FLAG_STATELESS));
> +

I guess it's just the kzalloc() that could theoretically fail. Probably
not a practical concern.

Didn't read through the entire device_link code, but this seems to
agree with the docs.

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

>  	return 0;
>  }
>  
> @@ -930,6 +932,8 @@ static void i915_audio_component_unbind(struct device *i915_kdev,
>  	struct i915_audio_component *acomp = data;
>  	struct drm_i915_private *dev_priv = kdev_to_i915(i915_kdev);
>  
> +	device_link_remove(hda_kdev, i915_kdev);
> +
>  	drm_modeset_lock_all(&dev_priv->drm);
>  	acomp->base.ops = NULL;
>  	acomp->base.dev = NULL;
> -- 
> 2.13.2

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