-----Original Message----- From: Jani Nikula [mailto:jani.nikula@linux.intel.com] Sent: Tuesday, January 24, 2017 12:31 PM To: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com; Anand, Jerome jerome.anand@intel.com; intel-gfx@lists.freedesktop.org; alsa- devel@alsa-project.org Cc: tiwai@suse.de; broonie@kernel.org; Ughreja, Rakesh A rakesh.a.ughreja@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@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