Re: [PATCH v2 08/14] ASoC: hdac_hdmi - create dais based on number of streams

From: Takashi Iwai <tiwai@suse.de>
To: "Subhransu S. Prusty" <subhransu.s.prusty@intel.com>
Cc: alsa-devel@alsa-project.org, patches.audio@intel.com,
	lgirdwood@gmail.com, dri-devel@lists.freedesktop.org,
	broonie@kernel.org, Daniel Vetter <daniel.vetter@intel.com>,
	Vinod Koul <vinod.koul@intel.com>
Subject: Re: [PATCH v2 08/14] ASoC: hdac_hdmi - create dais based on number of streams
Date: Thu, 03 Dec 2015 17:13:58 +0100	[thread overview]
Message-ID: <s5hfuzjse61.wl-tiwai@suse.de> (raw)
In-Reply-To: <1449176942-3441-8-git-send-email-subhransu.s.prusty@intel.com>

On Thu, 03 Dec 2015 22:08:56 +0100,
Subhransu S. Prusty wrote:
> 
> A stream is mapped to a converter. So based on the converters
> queried, dais are created.
> 
> The streams can be dynamically routed to any converter. For
> now it is mapped statically. The dynamic mapping of stream
> to converter will be added when required.
> 
> Signed-off-by: Subhransu S. Prusty <subhransu.s.prusty@intel.com>
> Signed-off-by: Vinod Koul <vinod.koul@intel.com>
> ---
>  sound/soc/codecs/hdac_hdmi.c | 124 ++++++++++++++++++++++++++++++++-----------
>  1 file changed, 92 insertions(+), 32 deletions(-)
> 
> diff --git a/sound/soc/codecs/hdac_hdmi.c b/sound/soc/codecs/hdac_hdmi.c
> index 325d22b..83248ad 100644
> --- a/sound/soc/codecs/hdac_hdmi.c
> +++ b/sound/soc/codecs/hdac_hdmi.c
> @@ -29,6 +29,8 @@
>  #include <sound/pcm_drm_eld.h>
>  #include "../../hda/local.h"
>  
> +#define NAME_SIZE	32
> +
>  #define AMP_OUT_MUTE		0xb080
>  #define AMP_OUT_UNMUTE		0xb000
>  #define PIN_OUT			(AC_PINCTL_OUT_EN)
> @@ -640,11 +642,84 @@ static void hdac_hdmi_skl_enable_dp12(struct hdac_device *hdac)
>  
>  }
>  
> +static struct snd_soc_dai_ops hdmi_dai_ops = {
> +	.startup = hdac_hdmi_pcm_open,
> +	.shutdown = hdac_hdmi_pcm_close,
> +	.hw_params = hdac_hdmi_set_hw_params,
> +	.prepare = hdac_hdmi_playback_prepare,
> +	.hw_free = hdac_hdmi_playback_cleanup,
> +};
> +
> +static int hdac_hdmi_create_dais(struct hdac_device *hdac,
> +		struct snd_soc_dai_driver **dais,
> +		struct hdac_hdmi_priv *hdmi, int num_dais)
> +{
> +	struct snd_soc_dai_driver *hdmi_dais;
> +	struct hdac_hdmi_cvt *cvt;
> +	char name[NAME_SIZE], dai_name[NAME_SIZE];
> +	int i = 0, j;
> +	u32 rates, bps;
> +	unsigned int rate_max = 384000, rate_min = 8000;
> +	u64 formats;
> +	int ret;
> +	static unsigned int rate_pcm[] = {
> +		8000, 11025, 16000, 22050, 32000, 44100, 48000, 88200,
> +		96000, 176400, 192000, 384000
> +	};
> +
> +	hdmi_dais = devm_kzalloc(&hdac->dev, (sizeof(*hdmi_dais) * num_dais),
> +			GFP_KERNEL);
> +	if (!hdmi_dais)
> +		return -ENOMEM;
> +
> +	list_for_each_entry(cvt, &hdmi->cvt_list, head) {
> +		ret = snd_hdac_query_supported_pcm(hdac, cvt->nid, &rates,
> +			&formats, &bps);
> +		if (ret)
> +			return ret;
> +		for (j = 0; j < ARRAY_SIZE(rate_pcm); j++) {
> +			if (rates & (1 << j)) {
> +				rate_min = rate_pcm[j];
> +				break;
> +			}
> +		}
> +
> +		for (j = ARRAY_SIZE(rate_pcm) - 1; j >= 0; j--) {
> +			if (rates & (1 << j)) {
> +				rate_max = rate_pcm[j];
> +				break;
> +			}
> +		}

This is ugly...  Can't we just set rate_min/max with the fixed numbers
that cover the all possible ranges?  The refinement is done by the
rates bitmap and hw_constraint in anyway.

Takashi
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel