[alsa-devel] [Intel-gfx] [PATCH v4 2/5] drm/i915: Add support for audio driver notifications

[Anand, Jerome]

> -----Original Message-----
> From: Jani Nikula [mailto:jani.nikula at linux.intel.com]
> Sent: Tuesday, January 24, 2017 12:31 PM
> To: Pierre-Louis Bossart <pierre-louis.bossart at linux.intel.com>; Anand,
> Jerome <jerome.anand at intel.com>; intel-gfx at lists.freedesktop.org; alsa-
> devel at alsa-project.org
> Cc: tiwai at suse.de; broonie at kernel.org; Ughreja, Rakesh A
> <rakesh.a.ughreja at intel.com>
> Subject: Re: [Intel-gfx] [PATCH v4 2/5] drm/i915: Add support for audio driver
> notifications
> 
> On Mon, 23 Jan 2017, Pierre-Louis Bossart <pierre-
> louis.bossart at linux.intel.com> wrote:
> >>>  #include <drm/drmP.h>
> >>> @@ -630,6 +631,10 @@ void intel_audio_codec_enable(struct
> intel_encoder *intel_encoder,
> >>>  	if (acomp && acomp->audio_ops && acomp->audio_ops-
> >pin_eld_notify)
> >>>  		acomp->audio_ops->pin_eld_notify(acomp->audio_ops-
> >audio_ptr,
> >>>  						 (int) port, (int) pipe);
> >>> +
> >>> +	if (HAS_LPE_AUDIO(dev_priv))
> >>> +		intel_lpe_audio_notify(dev_priv, connector->eld, port,
> >>> +			crtc_state->port_clock);
> >>
> >> Seems unnecessary to check for HAS_LPE_AUDIO (which you'll change to
> >> dev_priv->lpe_audio.platdev, right ;) both in the caller and callee.
> >> Pick one.
> >
> > If we test inside of the function, it'd mean an unconditional jump to
> > test a feature that exists on only two platforms out of the dozen or
> > so that this i915 driver handles. No objection to do the change but is
> > this really desired?
> 
> *shrug* no big deal to check in both.
> 

Thanks for the ack Jani.
It was added based on one review comment. I'll remove it here and have the check only inside the API

> BR,
> Jani.
> 
> >
> >
> 
> --
> Jani Nikula, Intel Open Source Technology Center