Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1204119

Re: [PATCH 1/9] drm: bridge/dw_hdmi-ahb-audio: add audio driver

From Russell King - ARM Linux <linux@arm.linux.org.uk>
Newsgroups linux.kernel
Subject Re: [PATCH 1/9] drm: bridge/dw_hdmi-ahb-audio: add audio driver
Date 2015-08-10 12:40 +0200
Message-ID <pVSEP-3Az-59@gated-at.bofh.it> (permalink)
References <pVeR3-3EB-9@gated-at.bofh.it> <pVf0J-3Qj-17@gated-at.bofh.it> <pVSbM-2Zz-11@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


On Mon, Aug 10, 2015 at 12:05:07PM +0200, Takashi Iwai wrote:
> On Sat, 08 Aug 2015 18:10:06 +0200,
> Russell King wrote:
> > +static irqreturn_t snd_dw_hdmi_irq(int irq, void *data)
> > +{
> > +	struct snd_dw_hdmi *dw = data;
> > +	struct snd_pcm_substream *substream;
> > +	unsigned stat;
> > +
> > +	stat = readb_relaxed(dw->data.base + HDMI_IH_AHBDMAAUD_STAT0);
> > +	if (!stat)
> > +		return IRQ_NONE;
> > +
> > +	writeb_relaxed(stat, dw->data.base + HDMI_IH_AHBDMAAUD_STAT0);
> > +
> > +	substream = dw->substream;
> > +	if (stat & HDMI_IH_AHBDMAAUD_STAT0_DONE && substream) {
> > +		snd_pcm_period_elapsed(substream);
> > +		if (dw->substream)
> > +			dw_hdmi_start_dma(dw);
> > +	}
> 
> Don't we need locking?

Possibly.

> In theory, the trigger can be issued while the irq is being handled.

Well, we can't have a lock around the whole of the above, because that
results in deadlock (as snd_pcm_period_elapsed() can end up calling into
the trigger method.)  I'm not happy to throw a spinlock around this
because of the in-built format conversion (something else I'm really not
happy about - which has to exist here because alsalib is soo painful
to add custom sample reformatting to - such modules have to be built
as part of alsalib itself rather than an add-on module.)

> > +static int dw_hdmi_trigger(struct snd_pcm_substream *substream, int cmd)
> > +{
> > +	struct snd_dw_hdmi *dw = substream->private_data;
> > +	int ret = 0;
> > +
> > +	switch (cmd) {
> > +	case SNDRV_PCM_TRIGGER_START:
> > +		dw->buf_offset = 0;
> > +		dw->substream = substream;
> > +		dw_hdmi_start_dma(dw);
> > +		dw_hdmi_audio_enable(dw->data.hdmi);
> > +		substream->runtime->delay = substream->runtime->period_size;
> > +		break;
> > +
> > +	case SNDRV_PCM_TRIGGER_STOP:
> > +		dw_hdmi_stop_dma(dw);
> > +		dw_hdmi_audio_disable(dw->data.hdmi);
> > +		break;
> > +
> > +	default:
> > +		ret = -EINVAL;
> > +		break;
> 
> SNDRV_PCM_TRIGGER_SUSPEND may be passed at suspend, too.

I think rather than adding code which would be difficult for me to test,
I'd instead remove the suspend/resume callbacks, or at least disable them
until someone can test that feature, or is willing to implement it.

> > +static snd_pcm_uframes_t dw_hdmi_pointer(struct snd_pcm_substream *substream)
> > +{
> > +	struct snd_pcm_runtime *runtime = substream->runtime;
> > +	struct snd_dw_hdmi *dw = substream->private_data;
> > +
> > +	return bytes_to_frames(runtime, dw->buf_offset);
> 
> So, this returns the offset that has been reformatted.  Does the
> hardware support any better position reporting?  We may give the delay
> from the driver if possible.

Basically, no.  Reading a 32-bit DMA position as separate bytes while
DMA is active is racy.

This is the best we can do, and the way we report the position has been
arrived at after what's getting on for two years of testing with
pulseaudio, vlc direct access & spdif pass-through, aplay, etc:

Author: Russell King <rmk+kernel@arm.linux.org.uk>
Date:   Thu Nov 7 16:01:45 2013 +0000

    drm: bridge/dw_hdmi-ahb-audio: add audio driver

-- 
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Back to linux.kernel | Previous | NextPrevious in thread | Next in thread | Find similar | Unroll thread


Thread

[PATCH 00/12] dw-hdmi development Russell King - ARM Linux <linux@arm.linux.org.uk> - 2015-08-08 18:10 +0200
  [PATCH 05/12] drm: bridge/dw_hdmi: add support for interlaced video  modes Russell King <rmk+kernel@arm.linux.org.uk> - 2015-08-08 18:10 +0200
  [PATCH 12/12] drm: bridge/dw_hdmi: improve HDMI enable/disable  handling Russell King <rmk+kernel@arm.linux.org.uk> - 2015-08-08 18:10 +0200
  Re: [PATCH 0/9] dw-hdmi audio support Russell King - ARM Linux <linux@arm.linux.org.uk> - 2015-08-08 18:10 +0200
    [PATCH 2/9] drm: bridge/dw_hdmi-ahb-audio: parse ELD from HDMI driver Russell King <rmk+kernel@arm.linux.org.uk> - 2015-08-08 18:20 +0200
    [PATCH 6/9] drm: bridge/dw_hdmi: adjust pixel clock values in N  calculation Russell King <rmk+kernel@arm.linux.org.uk> - 2015-08-08 18:20 +0200
    [PATCH 5/9] drm: bridge/dw_hdmi: avoid being recursive in N  calculation Russell King <rmk+kernel@arm.linux.org.uk> - 2015-08-08 18:20 +0200
    [PATCH 4/9] drm: bridge/dw_hdmi-ahb-audio: allow larger buffer sizes Russell King <rmk+kernel@arm.linux.org.uk> - 2015-08-08 18:20 +0200
    [PATCH 8/9] drm: bridge/dw_hdmi: replace CTS calculation for the ACR Russell King <rmk+kernel@arm.linux.org.uk> - 2015-08-08 18:20 +0200
    [PATCH 7/9] drm: bridge/dw_hdmi: remove ratio support from ACR code Russell King <rmk+kernel@arm.linux.org.uk> - 2015-08-08 18:20 +0200
    [PATCH 9/9] drm: bridge/dw_hdmi-i2s-audio: add audio driver Russell King <rmk+kernel@arm.linux.org.uk> - 2015-08-08 18:20 +0200
      Re: [PATCH 9/9] drm: bridge/dw_hdmi-i2s-audio: add audio driver Russell King - ARM Linux <linux@arm.linux.org.uk> - 2015-08-10 17:50 +0200
        Re: [PATCH 9/9] drm: bridge/dw_hdmi-i2s-audio: add audio driver Yakir Yang <ykk@rock-chips.com> - 2015-08-10 18:30 +0200
    [PATCH 1/9] drm: bridge/dw_hdmi-ahb-audio: add audio driver Russell King <rmk+kernel@arm.linux.org.uk> - 2015-08-08 18:20 +0200
      Re: [PATCH 1/9] drm: bridge/dw_hdmi-ahb-audio: add audio driver Takashi Iwai <tiwai@suse.de> - 2015-08-10 12:10 +0200
        Re: [PATCH 1/9] drm: bridge/dw_hdmi-ahb-audio: add audio driver Russell King - ARM Linux <linux@arm.linux.org.uk> - 2015-08-10 12:40 +0200
          Re: [PATCH 1/9] drm: bridge/dw_hdmi-ahb-audio: add audio driver Takashi Iwai <tiwai@suse.de> - 2015-08-10 14:30 +0200
            Re: [PATCH 1/9] drm: bridge/dw_hdmi-ahb-audio: add audio driver Russell King - ARM Linux <linux@arm.linux.org.uk> - 2015-08-10 19:00 +0200
              Re: [PATCH 1/9] drm: bridge/dw_hdmi-ahb-audio: add audio driver Mark Brown <broonie@kernel.org> - 2015-08-10 20:20 +0200
        Re: [PATCH v2 1/9] drm: bridge/dw_hdmi-ahb-audio: add audio driver         David Airlie <airlied@linux.ie>, Sascha Hauer <s.hauer@pengutronix.de>,         linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, Jaroslav         Kysela <perex@perex.cz>, linux-rockchip@lists.infradead.org, Mark Brown         <broonie@kernel.org>, Philipp Zabel <p.zabel@pengutronix.de>, Yakir         Yang <ykk@rock-chips.com>, Andy Yan <andy.yan@rock-chips.com>, Jon         Nettleton <jon.nettleton@gmail.com>,         linux-arm-kernel@lists.infradead.org Russell King <rmk+kernel@arm.linux.org.uk> - 2015-08-14 16:00 +0200
        Re: [PATCH v2 1/9] drm: bridge/dw_hdmi-ahb-audio: add audio driver Russell King <rmk+kernel@arm.linux.org.uk> - 2015-08-14 16:10 +0200
          Re: [alsa-devel] [PATCH v2 1/9] drm: bridge/dw_hdmi-ahb-audio: add audio driver Takashi Iwai <tiwai@suse.de> - 2015-08-14 16:40 +0200
    [PATCH 3/9] drm: bridge/dw_hdmi-ahb-audio: basic support for  multi-channel PCM audio Russell King <rmk+kernel@arm.linux.org.uk> - 2015-08-08 18:20 +0200
  [PATCH 08/12] drm: bridge/dw_hdmi: avoid enabling interface in  mode_set Russell King <rmk+kernel@arm.linux.org.uk> - 2015-08-08 18:10 +0200
  [PATCH 04/12] gpu: imx: fix support for interlaced modes Russell King <rmk+kernel@arm.linux.org.uk> - 2015-08-08 18:10 +0200
  [PATCH 09/12] drm: bridge/dw_hdmi: rename dw_hdmi_phy_enable_power() Russell King <rmk+kernel@arm.linux.org.uk> - 2015-08-08 18:10 +0200
  [PATCH 03/12] gpu: imx: simplify sync polarity setting Russell King <rmk+kernel@arm.linux.org.uk> - 2015-08-08 18:10 +0200
  [PATCH 01/12] drm: bridge/dw_hdmi: remove pixel repetition setting  for all VICs Russell King <rmk+kernel@arm.linux.org.uk> - 2015-08-08 18:10 +0200
  [PATCH 02/12] drm: bridge/dw_hdmi: don't support any pixel doubled  modes Russell King <rmk+kernel@arm.linux.org.uk> - 2015-08-08 18:10 +0200
  [PATCH 06/12] drm: bridge/dw_hdmi: clean up HDMI vs DVI mode handling Russell King <rmk+kernel@arm.linux.org.uk> - 2015-08-08 18:10 +0200
  [PATCH 11/12] drm: bridge/dw_hdmi: add connector mode forcing Russell King <rmk+kernel@arm.linux.org.uk> - 2015-08-08 18:10 +0200
  [PATCH 10/12] drm: bridge/dw_hdmi: fix phy enable/disable handling Russell King <rmk+kernel@arm.linux.org.uk> - 2015-08-08 18:20 +0200
  Re: [PATCH 00/12] dw-hdmi development Thierry Reding <thierry.reding@gmail.com> - 2015-08-10 14:30 +0200
    Re: [PATCH 00/12] dw-hdmi development Russell King - ARM Linux <linux@arm.linux.org.uk> - 2015-08-18 12:40 +0200

csiph-web