From: Takashi Iwai <tiwai@suse.de>
To: Vinod Koul <vinod.koul@intel.com>
Cc: alsa-devel@alsa-project.org, patches.audio@intel.com,
Hardik T Shah <hardik.t.shah@intel.com>,
Guneshwor Singh <guneshwor.o.singh@intel.com>,
liam.r.girdwood@linux.intel.com, broonie@kernel.org
Subject: Re: [PATCH v2 1/2] ALSA - hda: Add support for parsing new HDA capabilities
Date: Wed, 20 Jul 2016 07:31:17 +0200 [thread overview]
Message-ID: <s5hlh0w98pm.wl-tiwai@suse.de> (raw)
In-Reply-To: <1468925202-29445-2-git-send-email-vinod.koul@intel.com>
On Tue, 19 Jul 2016 12:46:41 +0200,
Vinod Koul wrote:
>
> From: Guneshwor Singh <guneshwor.o.singh@intel.com>
>
> Skylake onwards HDA controller supports new capabilities like
> Global Time Stamping (GTS) capability. So add support to parse
> these new capabilities.
>
> Signed-off-by: Guneshwor Singh <guneshwor.o.singh@intel.com>
> Signed-off-by: Hardik T Shah <hardik.t.shah@intel.com>
> Signed-off-by: Vinod Koul <vinod.koul@intel.com>
> ---
> include/sound/hda_register.h | 23 ++++++++++++++++++++
> sound/pci/hda/hda_controller.c | 49 ++++++++++++++++++++++++++++++++++++++++++
> sound/pci/hda/hda_controller.h | 27 +++++++++++++++++++++++
> sound/pci/hda/hda_intel.c | 12 +++++++++++
> 4 files changed, 111 insertions(+)
>
> diff --git a/include/sound/hda_register.h b/include/sound/hda_register.h
> index ff1aecf325e8..e4178328e8c8 100644
> --- a/include/sound/hda_register.h
> +++ b/include/sound/hda_register.h
> @@ -242,6 +242,29 @@ enum { SDI0, SDI1, SDI2, SDI3, SDO0, SDO1, SDO2, SDO3 };
> /* Interval used to calculate the iterating register offset */
> #define AZX_DRSM_INTERVAL 0x08
>
> +/* Global time synchronization registers */
> +#define GTSCC_TSCCD_MASK 0x80000000
> +#define GTSCC_TSCCD_SHIFT 31
> +#define GTSCC_TSCCI_MASK 0x20
> +#define GTSCC_CDMAS_DMA_DIR_SHIFT 4
> +
> +#define WALFCC_CIF_MASK 0x1FF
> +#define WALFCC_FN_SHIFT 9
> +#define HDA_CLK_CYCLES_PER_FRAME 512
> +
> +/*
> + * An error occurs near frame "rollover". The clocks in frame value indicates
> + * whether this error may have occurred. Here we use the value of 10. Please
> + * see the errata for the right number [<10]
> + */
> +#define HDA_MAX_CYCLE_VALUE 499
> +#define HDA_MAX_CYCLE_OFFSET 10
> +#define HDA_MAX_CYCLE_READ_RETRY 10
> +
> +#define TSCCU_CCU_SHIFT 32
> +#define LLPC_CCU_SHIFT 32
> +
> +
> /*
> * helpers to read the stream position
> */
> diff --git a/sound/pci/hda/hda_controller.c b/sound/pci/hda/hda_controller.c
> index 27de8015717d..7e5e4c261e51 100644
> --- a/sound/pci/hda/hda_controller.c
> +++ b/sound/pci/hda/hda_controller.c
> @@ -393,6 +393,50 @@ static struct snd_pcm_hardware azx_pcm_hw = {
> .fifo_size = 0,
> };
>
> +#define AZX_MAX_CAPS 10
> +
> +/**
> + * azx_parse_capabilities - parse the additional HDA capabilities for HDA
> + * controller. HDA controller defines capabilities as link list which can be
> + * parsed to find the controller support.
> + *
> + * @chip: azx controller
> + */
> +void azx_parse_capabilities(struct azx *chip)
> +{
> + unsigned int cur_cap;
> + unsigned int offset; unsigned int counter = 0;
Need a line break.
> +
> + offset = azx_readl(chip, LLCH);
> +
> + /* Lets walk the linked capabilities list */
> + do {
> + cur_cap = _snd_hdac_chip_read(l, azx_bus(chip), offset);
> +
> + switch ((cur_cap & CAP_HDR_ID_MASK) >> CAP_HDR_ID_OFF) {
> + case GTS_CAP_ID:
> + dev_dbg(chip->card->dev, "Found GTS capability");
> + chip->gts_present = 1;
> + break;
> +
> + default:
> + break;
> + }
> +
> + counter++;
> +
> + if (counter > AZX_MAX_CAPS) {
> + dev_err(chip->card->dev, "We exceeded azx capabilities!!!\n");
> + break;
> + }
> +
> + /* read the offset of next capabiity */
> + offset = cur_cap & CAP_HDR_NXT_PTR_MASK;
> +
> + } while (offset);
Wouldn't it be safer to use a normal while () {} loop?
The first LLCH read might be zero, in theory.
> --- a/sound/pci/hda/hda_intel.c
> +++ b/sound/pci/hda/hda_intel.c
> @@ -1655,6 +1655,18 @@ static int azx_first_init(struct azx *chip)
> return -ENXIO;
> }
>
> + if (IS_SKL_PLUS(pci))
> + azx_parse_capabilities(chip);
> +
> + /*
> + * Some Intel CPUs has always running timer (ART) feature and
> + * controller may have Global time sync reporting capability, so
> + * check both of these before declaring synchronized time reporting
> + * capability SNDRV_PCM_INFO_HAS_LINK_SYNCHRONIZED_ATIME
> + */
> + if (!(chip->gts_present && boot_cpu_has(X86_FEATURE_ART)))
> + chip->gts_present = false;
Need #ifdef CONFIG_X86 guard here, too.
Also, the inclusion of <asm/cpufeature.h> isn't needed? (Again
X86-only.)
thanks,
Takashi
next prev parent reply other threads:[~2016-07-20 5:31 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-07-19 10:46 [PATCH v2 0/2] ALSA - hda: Add support for link audio time reporting Vinod Koul
2016-07-19 10:46 ` [PATCH v2 1/2] ALSA - hda: Add support for parsing new HDA capabilities Vinod Koul
2016-07-20 5:31 ` Takashi Iwai [this message]
2016-07-21 4:24 ` Vinod Koul
2016-07-21 5:08 ` Takashi Iwai
2016-07-19 10:46 ` [PATCH v2 2/2] ALSA - hda: Add support for link audio time reporting Vinod Koul
2016-07-20 5:36 ` Takashi Iwai
2016-07-21 4:25 ` Vinod Koul
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=s5hlh0w98pm.wl-tiwai@suse.de \
--to=tiwai@suse.de \
--cc=alsa-devel@alsa-project.org \
--cc=broonie@kernel.org \
--cc=guneshwor.o.singh@intel.com \
--cc=hardik.t.shah@intel.com \
--cc=liam.r.girdwood@linux.intel.com \
--cc=patches.audio@intel.com \
--cc=vinod.koul@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.