* Re: [PATCH v2 06/15] drm/mxsfb: Update mxsfb with additional pixel formats
From: Daniel Stone @ 2019-08-14 11:44 UTC (permalink / raw)
To: Robert Chiras
Cc: Marek Vasut, Mark Rutland, Pengutronix Kernel Team, dri-devel,
devicetree, David Airlie, Fabio Estevam, Guido Günther,
Linux Kernel Mailing List, Stefan Agner, Rob Herring,
NXP Linux Team, Daniel Vetter, Shawn Guo, Sascha Hauer,
linux-arm-kernel
In-Reply-To: <1565779731-1300-7-git-send-email-robert.chiras@nxp.com>
Hi Robert,
On Wed, 14 Aug 2019 at 11:49, Robert Chiras <robert.chiras@nxp.com> wrote:
> + case DRM_FORMAT_BGR565: /* BG16 */
> + if (mxsfb->devdata->ipversion < 4)
> + goto err;
> + writel(CTRL2_ODD_LINE_PATTERN(CTRL2_LINE_PATTERN_BGR) |
> + CTRL2_EVEN_LINE_PATTERN(CTRL2_LINE_PATTERN_BGR),
> + mxsfb->base + LCDC_V4_CTRL2 + REG_SET);
> + /* Fall through */
> + case DRM_FORMAT_RGB565: /* RG16 */
> + ctrl |= CTRL_SET_WORD_LENGTH(0);
> + ctrl &= ~CTRL_DF16;
> + ctrl1 |= CTRL1_SET_BYTE_PACKAGING(0xf);
> + break;
For non-BGR formats, do you need to write RGB line-pattern back to the
CTRL2 register? Otherwise, if you start with BGR565 then switch back
to RGB565, presumably CTRL2 would still be programmed for BGR so you
would display inverted channels.
Same goes for all the other BGR/RGB format pairs below.
Cheers,
Daniel
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [EXT] Re: [PATCH v2 04/15] drm/mxsfb: Reset vital register for a proper initialization
From: Robert Chiras @ 2019-08-14 11:38 UTC (permalink / raw)
To: stefan@agner.ch
Cc: marex@denx.de, devicetree@vger.kernel.org, kernel@pengutronix.de,
airlied@linux.ie, shawnguo@kernel.org, agx@sigxcpu.org,
linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org,
robh+dt@kernel.org, dl-linux-imx, daniel@ffwll.ch,
mark.rutland@arm.com, festevam@gmail.com, s.hauer@pengutronix.de,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <18d5f58deba8044042ab7b8d98a72803@agner.ch>
Hi Stefan,
On Mi, 2019-08-14 at 13:11 +0200, Stefan Agner wrote:
> On 2019-08-14 12:48, Robert Chiras wrote:
> >
> > Some of the regiters need, like LCDC_CTRL and
> > CTRL2_OUTSTANDING_REQS
> Typo in registers, and there is a need to many.
Thanks, will fix this.
>
> >
> > needs to be properly cleared and initialized for a better start and
> > stop
> > routine.
>
>
> >
> >
> > Signed-off-by: Robert Chiras <robert.chiras@nxp.com>
> > ---
> > drivers/gpu/drm/mxsfb/mxsfb_crtc.c | 12 ++++++++++++
> > 1 file changed, 12 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/mxsfb/mxsfb_crtc.c
> > b/drivers/gpu/drm/mxsfb/mxsfb_crtc.c
> > index b69ace8..5e44f57 100644
> > --- a/drivers/gpu/drm/mxsfb/mxsfb_crtc.c
> > +++ b/drivers/gpu/drm/mxsfb/mxsfb_crtc.c
> > @@ -127,6 +127,10 @@ static void mxsfb_enable_controller(struct
> > mxsfb_drm_private *mxsfb)
> > clk_prepare_enable(mxsfb->clk_disp_axi);
> > clk_prepare_enable(mxsfb->clk);
> >
> > + if (mxsfb->devdata->ipversion >= 4)
> > + writel(CTRL2_OUTSTANDING_REQS(REQ_16),
> > + mxsfb->base + LCDC_V4_CTRL2 + REG_SET);
> > +
> > /* If it was disabled, re-enable the mode again */
> > writel(CTRL_DOTCLK_MODE, mxsfb->base + LCDC_CTRL + REG_SET);
> >
> > @@ -136,12 +140,19 @@ static void mxsfb_enable_controller(struct
> > mxsfb_drm_private *mxsfb)
> > writel(reg, mxsfb->base + LCDC_VDCTRL4);
> >
> > writel(CTRL_RUN, mxsfb->base + LCDC_CTRL + REG_SET);
> > + writel(CTRL1_RECOVERY_ON_UNDERFLOW, mxsfb->base + LCDC_CTRL1
> > + REG_SET);
> This seems not to be accounted for in the commit message. Can you do
> this in a separate commit?
>
> Also I suggest to introduce CTRL1_RECOVERY_ON_UNDERFLOW in that same
> commit.
You are right, I missed this one in the description. I will add this
one too.
>
> --
> Stefan
>
> >
> > }
> >
> > static void mxsfb_disable_controller(struct mxsfb_drm_private
> > *mxsfb)
> > {
> > u32 reg;
> >
> > + if (mxsfb->devdata->ipversion >= 4)
> > + writel(CTRL2_OUTSTANDING_REQS(0x7),
> > + mxsfb->base + LCDC_V4_CTRL2 + REG_CLR);
> > +
> > + writel(CTRL_RUN, mxsfb->base + LCDC_CTRL + REG_CLR);
> > +
> > /*
> > * Even if we disable the controller here, it will still
> > continue
> > * until its FIFOs are running out of data
> > @@ -295,6 +306,7 @@ void mxsfb_crtc_enable(struct mxsfb_drm_private
> > *mxsfb)
> > dma_addr_t paddr;
> >
> > mxsfb_enable_axi_clk(mxsfb);
> > + writel(0, mxsfb->base + LCDC_CTRL);
> > mxsfb_crtc_mode_set_nofb(mxsfb);
> >
> > /* Write cur_buf as well to avoid an initial corrupt frame */
Thanks,
Robert
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [EXT] Re: [PATCH v2 12/15] drm/mxsfb: Improve the axi clock usage
From: Robert Chiras @ 2019-08-14 11:35 UTC (permalink / raw)
To: stefan@agner.ch
Cc: marex@denx.de, devicetree@vger.kernel.org, kernel@pengutronix.de,
airlied@linux.ie, shawnguo@kernel.org, agx@sigxcpu.org,
linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org,
robh+dt@kernel.org, dl-linux-imx, daniel@ffwll.ch,
mark.rutland@arm.com, festevam@gmail.com, s.hauer@pengutronix.de,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <425a854f41248b083ff0c6c93673d696@agner.ch>
Hi Stefan,
On Mi, 2019-08-14 at 13:06 +0200, Stefan Agner wrote:
> On 2019-08-14 12:48, Robert Chiras wrote:
> >
> > Currently, the enable of the axi clock return status is ignored,
> > causing
> > issues when the enable fails then we try to disable it. Therefore,
> > it is
> > better to check the return status and disable it only when enable
> > succeeded.
> Is this actually the case in real world sometimes? Why is it failing?
When I noticed that fail, we had some restrictions in SCU firmware, so
that the clock cannot be enabled if the power domain was down. Still,
at that time I noticed it is redundant to have functions that checks if
a clock is NULL or not, since the clk_* functions are already doing
this.
>
> I guess if we do this in one place, we should do it in all places
> (e.g.
> also in mxsfb_crtc_enable, mxsfb_plane_atomic_update..)
I add specific checks only in the vblank functions, because those
functions can be called before calling the pipe enable/disable
functions which enables the spefic power domain and, at this point, the
clock enable/disable calls should not fail.
>
> --
> Stefan
>
> >
> > Also, remove the helper functions around clk_axi, since we can
> > directly
> > use the clk API function for enable/disable the clock. Those
> > functions
> > are already checking for NULL clk and returning 0 if that's the
> > case.
>
> >
> >
> > Signed-off-by: Robert Chiras <robert.chiras@nxp.com>
> > Acked-by: Leonard Crestez <leonard.crestez@nxp.com>
> > ---
> > drivers/gpu/drm/mxsfb/mxsfb_crtc.c | 8 ++++----
> > drivers/gpu/drm/mxsfb/mxsfb_drv.c | 32 +++++++++++++-------------
> > ------
> > drivers/gpu/drm/mxsfb/mxsfb_drv.h | 3 ---
> > 3 files changed, 17 insertions(+), 26 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/mxsfb/mxsfb_crtc.c
> > b/drivers/gpu/drm/mxsfb/mxsfb_crtc.c
> > index a4ba368..e727f5e 100644
> > --- a/drivers/gpu/drm/mxsfb/mxsfb_crtc.c
> > +++ b/drivers/gpu/drm/mxsfb/mxsfb_crtc.c
> > @@ -408,7 +408,7 @@ void mxsfb_crtc_enable(struct mxsfb_drm_private
> > *mxsfb)
> > {
> > dma_addr_t paddr;
> >
> > - mxsfb_enable_axi_clk(mxsfb);
> > + clk_prepare_enable(mxsfb->clk_axi);
> > writel(0, mxsfb->base + LCDC_CTRL);
> > mxsfb_crtc_mode_set_nofb(mxsfb);
> >
> > @@ -425,7 +425,7 @@ void mxsfb_crtc_enable(struct mxsfb_drm_private
> > *mxsfb)
> > void mxsfb_crtc_disable(struct mxsfb_drm_private *mxsfb)
> > {
> > mxsfb_disable_controller(mxsfb);
> > - mxsfb_disable_axi_clk(mxsfb);
> > + clk_disable_unprepare(mxsfb->clk_axi);
> > }
> >
> > void mxsfb_plane_atomic_update(struct mxsfb_drm_private *mxsfb,
> > @@ -451,8 +451,8 @@ void mxsfb_plane_atomic_update(struct
> > mxsfb_drm_private *mxsfb,
> >
> > paddr = mxsfb_get_fb_paddr(mxsfb);
> > if (paddr) {
> > - mxsfb_enable_axi_clk(mxsfb);
> > + clk_prepare_enable(mxsfb->clk_axi);
> > writel(paddr, mxsfb->base + mxsfb->devdata-
> > >next_buf);
> > - mxsfb_disable_axi_clk(mxsfb);
> > + clk_disable_unprepare(mxsfb->clk_axi);
> > }
> > }
> > diff --git a/drivers/gpu/drm/mxsfb/mxsfb_drv.c
> > b/drivers/gpu/drm/mxsfb/mxsfb_drv.c
> > index 6dae2bd..694b287 100644
> > --- a/drivers/gpu/drm/mxsfb/mxsfb_drv.c
> > +++ b/drivers/gpu/drm/mxsfb/mxsfb_drv.c
> > @@ -97,18 +97,6 @@ drm_pipe_to_mxsfb_drm_private(struct
> > drm_simple_display_pipe *pipe)
> > return container_of(pipe, struct mxsfb_drm_private, pipe);
> > }
> >
> > -void mxsfb_enable_axi_clk(struct mxsfb_drm_private *mxsfb)
> > -{
> > - if (mxsfb->clk_axi)
> > - clk_prepare_enable(mxsfb->clk_axi);
> > -}
> > -
> > -void mxsfb_disable_axi_clk(struct mxsfb_drm_private *mxsfb)
> > -{
> > - if (mxsfb->clk_axi)
> > - clk_disable_unprepare(mxsfb->clk_axi);
> > -}
> > -
> > /**
> > * mxsfb_atomic_helper_check - validate state object
> > * @dev: DRM device
> > @@ -229,25 +217,31 @@ static void mxsfb_pipe_update(struct
> > drm_simple_display_pipe *pipe,
> > static int mxsfb_pipe_enable_vblank(struct drm_simple_display_pipe
> > *pipe)
> > {
> > struct mxsfb_drm_private *mxsfb =
> > drm_pipe_to_mxsfb_drm_private(pipe);
> > + int ret = 0;
> > +
> > + ret = clk_prepare_enable(mxsfb->clk_axi);
> > + if (ret)
> > + return ret;
> >
> > /* Clear and enable VBLANK IRQ */
> > - mxsfb_enable_axi_clk(mxsfb);
> > writel(CTRL1_CUR_FRAME_DONE_IRQ, mxsfb->base + LCDC_CTRL1 +
> > REG_CLR);
> > writel(CTRL1_CUR_FRAME_DONE_IRQ_EN, mxsfb->base + LCDC_CTRL1
> > + REG_SET);
> > - mxsfb_disable_axi_clk(mxsfb);
> > + clk_disable_unprepare(mxsfb->clk_axi);
> >
> > - return 0;
> > + return ret;
> > }
> >
> > static void mxsfb_pipe_disable_vblank(struct
> > drm_simple_display_pipe *pipe)
> > {
> > struct mxsfb_drm_private *mxsfb =
> > drm_pipe_to_mxsfb_drm_private(pipe);
> >
> > + if (clk_prepare_enable(mxsfb->clk_axi))
> > + return;
> > +
> > /* Disable and clear VBLANK IRQ */
> > - mxsfb_enable_axi_clk(mxsfb);
> > writel(CTRL1_CUR_FRAME_DONE_IRQ_EN, mxsfb->base + LCDC_CTRL1
> > + REG_CLR);
> > writel(CTRL1_CUR_FRAME_DONE_IRQ, mxsfb->base + LCDC_CTRL1 +
> > REG_CLR);
> > - mxsfb_disable_axi_clk(mxsfb);
> > + clk_disable_unprepare(mxsfb->clk_axi);
> > }
> >
> > static struct drm_simple_display_pipe_funcs mxsfb_funcs = {
> > @@ -413,7 +407,7 @@ static irqreturn_t mxsfb_irq_handler(int irq,
> > void *data)
> > struct mxsfb_drm_private *mxsfb = drm->dev_private;
> > u32 reg;
> >
> > - mxsfb_enable_axi_clk(mxsfb);
> > + clk_prepare_enable(mxsfb->clk_axi);
> >
> > reg = readl(mxsfb->base + LCDC_CTRL1);
> >
> > @@ -422,7 +416,7 @@ static irqreturn_t mxsfb_irq_handler(int irq,
> > void *data)
> >
> > writel(CTRL1_CUR_FRAME_DONE_IRQ, mxsfb->base + LCDC_CTRL1 +
> > REG_CLR);
> >
> > - mxsfb_disable_axi_clk(mxsfb);
> > + clk_disable_unprepare(mxsfb->clk_axi);
> >
> > return IRQ_HANDLED;
> > }
> > diff --git a/drivers/gpu/drm/mxsfb/mxsfb_drv.h
> > b/drivers/gpu/drm/mxsfb/mxsfb_drv.h
> > index 8fb65d3..d6df8fe 100644
> > --- a/drivers/gpu/drm/mxsfb/mxsfb_drv.h
> > +++ b/drivers/gpu/drm/mxsfb/mxsfb_drv.h
> > @@ -37,9 +37,6 @@ struct mxsfb_drm_private {
> > int mxsfb_setup_crtc(struct drm_device *dev);
> > int mxsfb_create_output(struct drm_device *dev);
> >
> > -void mxsfb_enable_axi_clk(struct mxsfb_drm_private *mxsfb);
> > -void mxsfb_disable_axi_clk(struct mxsfb_drm_private *mxsfb);
> > -
> > void mxsfb_crtc_enable(struct mxsfb_drm_private *mxsfb);
> > void mxsfb_crtc_disable(struct mxsfb_drm_private *mxsfb);
> > void mxsfb_plane_atomic_update(struct mxsfb_drm_private *mxsfb,
Thanks,
Robert
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v1] MAINTAINERS: i2c-imx: take over maintainership
From: Uwe Kleine-König @ 2019-08-14 11:33 UTC (permalink / raw)
To: Wolfram Sang
Cc: Aisheng Dong, linux-arm-kernel, Andrey Smirnov, Sascha Hauer,
Russell King - ARM Linux admin, Oleksij Rempel, linux-i2c,
Pengutronix Kernel Team, Fabio Estevam, Shawn Guo, Chris Healy,
NXP Linux Team
In-Reply-To: <20190814100224.GE1511@ninjato>
On Wed, Aug 14, 2019 at 12:02:25PM +0200, Wolfram Sang wrote:
>
> > Even without this patch the generic "ARM/FREESCALE IMX / MXC ARM
> > ARCHITECTURE" entry matches the i2c-imx driver.
>
> It matches, but it didn't work well, I am afraid.
I didn't intend to imply it worked well. Just thought it was sensible to
point out that even with the newly added entry in MAINTAINERS the old
entry still matches.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | http://www.pengutronix.de/ |
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v5 15/15] ASoC: sun4i-i2s: Adjust regmap settings
From: Chen-Yu Tsai @ 2019-08-14 11:31 UTC (permalink / raw)
To: Code Kipper
Cc: Linux-ALSA, linux-kernel, Liam Girdwood, Andrea Venturi (pers),
linux-sunxi, Mark Brown, Maxime Ripard, linux-arm-kernel
In-Reply-To: <20190814060854.26345-16-codekipper@gmail.com>
On Wed, Aug 14, 2019 at 2:09 PM <codekipper@gmail.com> wrote:
>
> From: Marcus Cooper <codekipper@gmail.com>
>
> Bypass the regmap cache when flushing the i2s FIFOs and modify the tables
> to reflect this.
>
> Signed-off-by: Marcus Cooper <codekipper@gmail.com>
> ---
> sound/soc/sunxi/sun4i-i2s.c | 31 ++++++++++---------------------
> 1 file changed, 10 insertions(+), 21 deletions(-)
>
> diff --git a/sound/soc/sunxi/sun4i-i2s.c b/sound/soc/sunxi/sun4i-i2s.c
> index d3c8789f70bb..ecfc1ed79379 100644
> --- a/sound/soc/sunxi/sun4i-i2s.c
> +++ b/sound/soc/sunxi/sun4i-i2s.c
> @@ -876,9 +876,11 @@ static int sun4i_i2s_set_fmt(struct snd_soc_dai *dai, unsigned int fmt)
> static void sun4i_i2s_start_capture(struct sun4i_i2s *i2s)
> {
> /* Flush RX FIFO */
> + regcache_cache_bypass(i2s->regmap, true);
> regmap_update_bits(i2s->regmap, SUN4I_I2S_FIFO_CTRL_REG,
> SUN4I_I2S_FIFO_CTRL_FLUSH_RX,
> SUN4I_I2S_FIFO_CTRL_FLUSH_RX);
> + regcache_cache_bypass(i2s->regmap, false);
>
> /* Clear RX counter */
> regmap_write(i2s->regmap, SUN4I_I2S_RX_CNT_REG, 0);
> @@ -897,9 +899,11 @@ static void sun4i_i2s_start_capture(struct sun4i_i2s *i2s)
> static void sun4i_i2s_start_playback(struct sun4i_i2s *i2s)
> {
> /* Flush TX FIFO */
> + regcache_cache_bypass(i2s->regmap, true);
> regmap_update_bits(i2s->regmap, SUN4I_I2S_FIFO_CTRL_REG,
> SUN4I_I2S_FIFO_CTRL_FLUSH_TX,
> SUN4I_I2S_FIFO_CTRL_FLUSH_TX);
> + regcache_cache_bypass(i2s->regmap, false);
>
> /* Clear TX counter */
> regmap_write(i2s->regmap, SUN4I_I2S_TX_CNT_REG, 0);
> @@ -1053,13 +1057,7 @@ static const struct snd_soc_component_driver sun4i_i2s_component = {
>
> static bool sun4i_i2s_rd_reg(struct device *dev, unsigned int reg)
> {
> - switch (reg) {
> - case SUN4I_I2S_FIFO_TX_REG:
> - return false;
> -
> - default:
> - return true;
> - }
> + return true;
The commit log needs to explain why this is relevant. And I'm not sure why one
would read back the TX FIFO. Also, if it's always true, just drop the callback.
ChenYu
> }
>
> static bool sun4i_i2s_wr_reg(struct device *dev, unsigned int reg)
> @@ -1078,6 +1076,8 @@ static bool sun4i_i2s_volatile_reg(struct device *dev, unsigned int reg)
> {
> switch (reg) {
> case SUN4I_I2S_FIFO_RX_REG:
> + case SUN4I_I2S_FIFO_TX_REG:
> + case SUN4I_I2S_FIFO_STA_REG:
> case SUN4I_I2S_INT_STA_REG:
> case SUN4I_I2S_RX_CNT_REG:
> case SUN4I_I2S_TX_CNT_REG:
> @@ -1088,23 +1088,12 @@ static bool sun4i_i2s_volatile_reg(struct device *dev, unsigned int reg)
> }
> }
>
> -static bool sun8i_i2s_rd_reg(struct device *dev, unsigned int reg)
> -{
> - switch (reg) {
> - case SUN8I_I2S_FIFO_TX_REG:
> - return false;
> -
> - default:
> - return true;
> - }
> -}
> -
> static bool sun8i_i2s_volatile_reg(struct device *dev, unsigned int reg)
> {
> if (reg == SUN8I_I2S_INT_STA_REG)
> return true;
> if (reg == SUN8I_I2S_FIFO_TX_REG)
> - return false;
> + return true;
>
> return sun4i_i2s_volatile_reg(dev, reg);
> }
> @@ -1175,7 +1164,7 @@ static const struct regmap_config sun8i_i2s_regmap_config = {
> .reg_defaults = sun8i_i2s_reg_defaults,
> .num_reg_defaults = ARRAY_SIZE(sun8i_i2s_reg_defaults),
> .writeable_reg = sun4i_i2s_wr_reg,
> - .readable_reg = sun8i_i2s_rd_reg,
> + .readable_reg = sun4i_i2s_rd_reg,
> .volatile_reg = sun8i_i2s_volatile_reg,
> };
>
> @@ -1188,7 +1177,7 @@ static const struct regmap_config sun50i_i2s_regmap_config = {
> .reg_defaults = sun50i_i2s_reg_defaults,
> .num_reg_defaults = ARRAY_SIZE(sun50i_i2s_reg_defaults),
> .writeable_reg = sun4i_i2s_wr_reg,
> - .readable_reg = sun8i_i2s_rd_reg,
> + .readable_reg = sun4i_i2s_rd_reg,
> .volatile_reg = sun8i_i2s_volatile_reg,
> };
>
> --
> 2.22.0
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [linux-sunxi] Re: [PATCH v5 15/15] ASoC: sun4i-i2s: Adjust regmap settings
From: Jernej Škrabec @ 2019-08-14 11:31 UTC (permalink / raw)
To: linux-sunxi, maxime.ripard
Cc: alsa-devel, be17068, lgirdwood, linux-kernel, codekipper, wens,
broonie, linux-arm-kernel
In-Reply-To: <20190814072007.6tfvhzsw4oxbwpc2@flea>
Dne sreda, 14. avgust 2019 ob 09:20:07 CEST je Maxime Ripard napisal(a):
> On Wed, Aug 14, 2019 at 08:08:54AM +0200, codekipper@gmail.com wrote:
> > From: Marcus Cooper <codekipper@gmail.com>
> >
> > Bypass the regmap cache when flushing the i2s FIFOs and modify the tables
> > to reflect this.
> >
> > Signed-off-by: Marcus Cooper <codekipper@gmail.com>
>
> This patch looks like it's fixing something while the commit log
> doesn't mention what is being fixed.
Main issue addressed here is that SUN4I_I2S_FIFO_CTRL_REG has two self-clear
registers (SUN4I_I2S_FIFO_CTRL_FLUSH_RX and SUN4I_I2S_FIFO_CTRL_FLUSH_TX) and
thus it should be marked as volatile.
Best regards,
Jernej
>
> Having some context here would be great.
>
> Maxime
>
> --
> Maxime Ripard, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH 0/8] Fix issues with 52-bit kernel virtual addressing
From: Mark Rutland @ 2019-08-14 11:29 UTC (permalink / raw)
To: Will Deacon
Cc: Steve Capper, Andrey Konovalov, Geert Uytterhoeven,
Catalin Marinas, Qian Cai, linux-arm-kernel
In-Reply-To: <20190813170149.26037-1-will@kernel.org>
On Tue, Aug 13, 2019 at 06:01:41PM +0100, Will Deacon wrote:
> Hi all,
>
> This patch series addresses some issues with 52-bit kernel VAs reported
> by Qian Cai and Geert. It's all confined to asm/memory.h and I got a bit
> carried away cleaning that thing up so the patches get more worthless
> as you go through the series. Still, I'd like to queue this on top of
> the 52-bit VA stuff currently sitting in -next.
>
> Although Geert and Steve tested my initial hacks, I dropped the tags
> because I've split things up and could've easily broken things again.
>
> Cheers,
>
> Will
>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Steve Capper <steve.capper@arm.com>
> Cc: Qian Cai <cai@lca.pw>
> Cc: Andrey Konovalov <andreyknvl@google.com>
> Cc: Geert Uytterhoeven <geert@linux-m68k.org>
Other than the comments I've made, for the series:
Reviewed-by: Mark Rutland <mark.rutland@arm.com>
If you want, I can spin a fix / cleanup for the VA_START issues I
mentioned atop of this series.
Thanks,
Mark.
>
> --->8
>
> Will Deacon (8):
> arm64: memory: Fix virt_addr_valid() using __is_lm_address()
> arm64: memory: Ensure address tag is masked in conversion macros
> arm64: memory: Rewrite default page_to_virt()/virt_to_page()
> arm64: memory: Simplify virt_to_page() implementation
> arm64: memory: Simplify _VA_START and _PAGE_OFFSET definitions
> arm64: memory: Implement __tag_set() as common function
> arm64: memory: Add comments to end of non-trivial #ifdef blocks
> arm64: memory: Cosmetic cleanups
>
> arch/arm64/include/asm/memory.h | 89 ++++++++++++++++++++---------------------
> 1 file changed, 44 insertions(+), 45 deletions(-)
>
> --
> 2.11.0
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH 3/8] arm64: memory: Rewrite default page_to_virt()/virt_to_page()
From: Mark Rutland @ 2019-08-14 11:26 UTC (permalink / raw)
To: Will Deacon
Cc: Steve Capper, Andrey Konovalov, Geert Uytterhoeven,
Catalin Marinas, Qian Cai, linux-arm-kernel
In-Reply-To: <20190814111722.r5xomirfanmxcor6@willie-the-truck>
On Wed, Aug 14, 2019 at 12:17:22PM +0100, Will Deacon wrote:
> On Wed, Aug 14, 2019 at 11:56:39AM +0100, Mark Rutland wrote:
> > On Wed, Aug 14, 2019 at 10:41:19AM +0100, Will Deacon wrote:
> > > On Wed, Aug 14, 2019 at 10:30:19AM +0100, Catalin Marinas wrote:
> > > > On Tue, Aug 13, 2019 at 06:01:44PM +0100, Will Deacon wrote:
> > > > > diff --git a/arch/arm64/include/asm/memory.h b/arch/arm64/include/asm/memory.h
> > > > > index 47b4dc73b8bf..77074b3a1025 100644
> > > > > --- a/arch/arm64/include/asm/memory.h
> > > > > +++ b/arch/arm64/include/asm/memory.h
> > > > > @@ -313,19 +313,18 @@ static inline void *phys_to_virt(phys_addr_t x)
> > > > > #if !defined(CONFIG_SPARSEMEM_VMEMMAP) || defined(CONFIG_DEBUG_VIRTUAL)
> > > > > #define virt_to_page(kaddr) pfn_to_page(__pa(kaddr) >> PAGE_SHIFT)
> > > > > #else
> > > > > -#define __virt_to_pgoff(kaddr) (((u64)(kaddr) - PAGE_OFFSET) / PAGE_SIZE * sizeof(struct page))
> > > > > -#define __page_to_voff(kaddr) (((u64)(kaddr) - VMEMMAP_START) * PAGE_SIZE / sizeof(struct page))
> > > > > -
> > > > > -#define page_to_virt(page) ({ \
> > > > > - unsigned long __addr = \
> > > > > - ((__page_to_voff(page)) + PAGE_OFFSET); \
> > > > > - const void *__addr_tag = \
> > > > > - __tag_set((void *)__addr, page_kasan_tag(page)); \
> > > > > - ((void *)__addr_tag); \
> > > > > +#define page_to_virt(x) ({ \
> > > > > + __typeof__(x) __page = x; \
> > > >
> > > > Why not struct page * directly here?
> > >
> > > I started out with that, but then you have to deal with const struct page *
> > > as well and it gets pretty messy.
> >
> > What goes wrong if you always use const struct page *__page?
>
> It would probably work, but then I wondered about the possibility of
> volatile and decided that __typeof__ was cleaner.
Ok, but I'd suggest that volatile struct page * that's never sane to use
in the first place.
If you don't want to change this, then no worries -- I just can't
follow.
Thanks,
Mark.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [EXT] Re: [PATCH v2 09/15] dt-bindings: display: Add max-res property for mxsfb
From: Stefan Agner @ 2019-08-14 11:25 UTC (permalink / raw)
To: Robert Chiras
Cc: mark.rutland, marex, kernel, devicetree, airlied, shawnguo, agx,
linux-kernel, dri-devel, robh+dt, dl-linux-imx, daniel, festevam,
s.hauer, linux-arm-kernel
In-Reply-To: <1565781243.3209.55.camel@nxp.com>
On 2019-08-14 13:14, Robert Chiras wrote:
> Hi Stefan,
> On Mi, 2019-08-14 at 13:03 +0200, Stefan Agner wrote:
>> On 2019-08-14 12:48, Robert Chiras wrote:
>> >
>> > Add new optional property 'max-res', to limit the maximum supported
>> > resolution by the MXSFB_DRM driver.
>> I would also mention the reason why we need this.
>>
>> I guess this needs a vendor prefix as well (fsl,max-res). I also
>> would
>> like to have the ack of the device tree folks here.
> Rob Herring also aked be about this, and I'll copy here the reply, with
> explanations:
>
> Indeed, this limitation is actually due to bandwidth limitation, but
> the problem is that this limitation comes on i.MX8M (known as mScale
> 850D), where the memory bandwidth cannot support: GPU/VPU workload in
> the same time with both DCSS driving 4k@60 and eLCDIF driving 1080p@60.
> Since eLCDIF is a secondary display we though to add the posibility to
> limit it's bandwidth by limiting the resolution.
> If you say that more details are needed, I can add them in the
> description.
Oh sorry I missed that.
Rob Herring also wrote:
> I suppose what you are after is bandwidth limits? IIRC, there's already
> some bindings expressing such limits. Also, wouldn't you need to account
> for bpp and using the 2nd plane (IIRC that there is one).
I guess the binding he refers to is max-memory-bandwidth, which is used
in multiple driver already. It makes sense to reuse this property
instead of inventing a new set of property which is also not taking bpp
into account...
The pl111 driver implements this property, it should be fairly easy to
adopt that code.
--
Stefan
>>
>> --
>> Stefan
>>
>> >
>> >
>> > Signed-off-by: Robert Chiras <robert.chiras@nxp.com>
>> > ---
>> > Documentation/devicetree/bindings/display/mxsfb.txt | 6 ++++++
>> > 1 file changed, 6 insertions(+)
>> >
>> > diff --git a/Documentation/devicetree/bindings/display/mxsfb.txt
>> > b/Documentation/devicetree/bindings/display/mxsfb.txt
>> > index 472e1ea..55e22ed 100644
>> > --- a/Documentation/devicetree/bindings/display/mxsfb.txt
>> > +++ b/Documentation/devicetree/bindings/display/mxsfb.txt
>> > @@ -17,6 +17,12 @@ Required properties:
>> > Required sub-nodes:
>> > - port: The connection to an encoder chip.
>> >
>> > +Optional properties:
>> > +- max-res: an array with a maximum of two integers, representing
>> > the
>> > + maximum supported resolution, in the form of
>> > + <maxX>, <maxY>; if one of the item is <0>, the
>> > default
>> > + driver-defined maximum resolution for that axis is
>> > used
>> > +
>> > Example:
>> >
>> > lcdif1: display-controller@2220000 {
>
> Thanks,
> Robert
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v5 01/15] ASoC: sun4i-i2s: Add regmap field to sign extend sample
From: Code Kipper @ 2019-08-14 11:24 UTC (permalink / raw)
To: Maxime Ripard
Cc: Linux-ALSA, linux-sunxi, linux-kernel, Liam Girdwood,
Andrea Venturi (pers), Chen-Yu Tsai, Mark Brown, linux-arm-kernel
In-Reply-To: <20190814064339.lgfngdkiaalygolk@flea>
On Wed, 14 Aug 2019 at 13:08, Maxime Ripard <maxime.ripard@bootlin.com> wrote:
>
> Hi,
>
> On Wed, Aug 14, 2019 at 08:08:40AM +0200, codekipper@gmail.com wrote:
> > From: Marcus Cooper <codekipper@gmail.com>
> >
> > On the newer SoCs such as the H3 and A64 this is set by default
> > to transfer a 0 after each sample in each slot. However the A10
> > and A20 SoCs that this driver was developed on had a default
> > setting where it padded the audio gain with zeros.
> >
> > This isn't a problem whilst we have only support for 16bit audio
> > but with larger sample resolution rates in the pipeline then SEXT
> > bits should be cleared so that they also pad at the LSB. Without
> > this the audio gets distorted.
> >
> > Signed-off-by: Marcus Cooper <codekipper@gmail.com>
> > ---
> > sound/soc/sunxi/sun4i-i2s.c | 16 ++++++++++++++++
> > 1 file changed, 16 insertions(+)
> >
> > diff --git a/sound/soc/sunxi/sun4i-i2s.c b/sound/soc/sunxi/sun4i-i2s.c
> > index 793457394efe..8201334a059b 100644
> > --- a/sound/soc/sunxi/sun4i-i2s.c
> > +++ b/sound/soc/sunxi/sun4i-i2s.c
> > @@ -135,6 +135,7 @@ struct sun4i_i2s;
> > * @field_fmt_bclk: regmap field to set clk polarity.
> > * @field_fmt_lrclk: regmap field to set frame polarity.
> > * @field_fmt_mode: regmap field to set the operational mode.
> > + * @field_fmt_sext: regmap field to set the sign extension.
> > * @field_txchanmap: location of the tx channel mapping register.
> > * @field_rxchanmap: location of the rx channel mapping register.
> > * @field_txchansel: location of the tx channel select bit fields.
> > @@ -159,6 +160,7 @@ struct sun4i_i2s_quirks {
> > struct reg_field field_fmt_bclk;
> > struct reg_field field_fmt_lrclk;
> > struct reg_field field_fmt_mode;
> > + struct reg_field field_fmt_sext;
> > struct reg_field field_txchanmap;
> > struct reg_field field_rxchanmap;
> > struct reg_field field_txchansel;
> > @@ -186,6 +188,7 @@ struct sun4i_i2s {
> > struct regmap_field *field_fmt_bclk;
> > struct regmap_field *field_fmt_lrclk;
> > struct regmap_field *field_fmt_mode;
> > + struct regmap_field *field_fmt_sext;
> > struct regmap_field *field_txchanmap;
> > struct regmap_field *field_rxchanmap;
> > struct regmap_field *field_txchansel;
> > @@ -345,6 +348,9 @@ static int sun4i_i2s_set_clk_rate(struct snd_soc_dai *dai,
> > SUN8I_I2S_FMT0_LRCK_PERIOD_MASK,
> > SUN8I_I2S_FMT0_LRCK_PERIOD(32));
> >
> > + /* Set sign extension to pad out LSB with 0 */
> > + regmap_field_write(i2s->field_fmt_sext, 0);
> > +
> > return 0;
> > }
> >
> > @@ -917,6 +923,7 @@ static const struct sun4i_i2s_quirks sun4i_a10_i2s_quirks = {
> > .field_fmt_lrclk = REG_FIELD(SUN4I_I2S_FMT0_REG, 7, 7),
> > .has_slave_select_bit = true,
> > .field_fmt_mode = REG_FIELD(SUN4I_I2S_FMT0_REG, 0, 1),
> > + .field_fmt_sext = REG_FIELD(SUN4I_I2S_FMT1_REG, 8, 8),
> > .field_txchanmap = REG_FIELD(SUN4I_I2S_TX_CHAN_MAP_REG, 0, 31),
> > .field_rxchanmap = REG_FIELD(SUN4I_I2S_RX_CHAN_MAP_REG, 0, 31),
> > .field_txchansel = REG_FIELD(SUN4I_I2S_TX_CHAN_SEL_REG, 0, 2),
> > @@ -936,6 +943,7 @@ static const struct sun4i_i2s_quirks sun6i_a31_i2s_quirks = {
> > .field_fmt_lrclk = REG_FIELD(SUN4I_I2S_FMT0_REG, 7, 7),
> > .has_slave_select_bit = true,
> > .field_fmt_mode = REG_FIELD(SUN4I_I2S_FMT0_REG, 0, 1),
> > + .field_fmt_sext = REG_FIELD(SUN4I_I2S_FMT1_REG, 8, 8),
> > .field_txchanmap = REG_FIELD(SUN4I_I2S_TX_CHAN_MAP_REG, 0, 31),
> > .field_rxchanmap = REG_FIELD(SUN4I_I2S_RX_CHAN_MAP_REG, 0, 31),
> > .field_txchansel = REG_FIELD(SUN4I_I2S_TX_CHAN_SEL_REG, 0, 2),
> > @@ -979,6 +987,7 @@ static const struct sun4i_i2s_quirks sun8i_h3_i2s_quirks = {
> > .field_fmt_bclk = REG_FIELD(SUN4I_I2S_FMT0_REG, 7, 7),
> > .field_fmt_lrclk = REG_FIELD(SUN4I_I2S_FMT0_REG, 19, 19),
> > .field_fmt_mode = REG_FIELD(SUN4I_I2S_CTRL_REG, 4, 5),
> > + .field_fmt_sext = REG_FIELD(SUN4I_I2S_FMT1_REG, 4, 5),
> > .field_txchanmap = REG_FIELD(SUN8I_I2S_TX_CHAN_MAP_REG, 0, 31),
> > .field_rxchanmap = REG_FIELD(SUN8I_I2S_RX_CHAN_MAP_REG, 0, 31),
> > .field_txchansel = REG_FIELD(SUN8I_I2S_TX_CHAN_SEL_REG, 0, 2),
> > @@ -998,6 +1007,7 @@ static const struct sun4i_i2s_quirks sun50i_a64_codec_i2s_quirks = {
> > .field_fmt_bclk = REG_FIELD(SUN4I_I2S_FMT0_REG, 6, 6),
> > .field_fmt_lrclk = REG_FIELD(SUN4I_I2S_FMT0_REG, 7, 7),
> > .field_fmt_mode = REG_FIELD(SUN4I_I2S_FMT0_REG, 0, 1),
> > + .field_fmt_sext = REG_FIELD(SUN4I_I2S_FMT1_REG, 8, 8),
> > .field_txchanmap = REG_FIELD(SUN4I_I2S_TX_CHAN_MAP_REG, 0, 31),
> > .field_rxchanmap = REG_FIELD(SUN4I_I2S_RX_CHAN_MAP_REG, 0, 31),
> > .field_txchansel = REG_FIELD(SUN4I_I2S_TX_CHAN_SEL_REG, 0, 2),
>
> You're missing the A83t here
ARRGGGHHHHH...ACK...thanks,
CK
>
> Maxime
>
> --
> Maxime Ripard, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH 5/8] arm64: memory: Simplify _VA_START and _PAGE_OFFSET definitions
From: Mark Rutland @ 2019-08-14 11:23 UTC (permalink / raw)
To: Will Deacon
Cc: Steve Capper, Andrey Konovalov, Geert Uytterhoeven,
Catalin Marinas, Qian Cai, linux-arm-kernel
In-Reply-To: <20190813170149.26037-6-will@kernel.org>
On Tue, Aug 13, 2019 at 06:01:46PM +0100, Will Deacon wrote:
> Rather than subtracting from -1 and then adding 1, we can simply
> subtract from 0.
>
> Cc: Steve Capper <steve.capper@arm.com>
> Signed-off-by: Will Deacon <will@kernel.org>
> ---
> arch/arm64/include/asm/memory.h | 6 ++----
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm64/include/asm/memory.h b/arch/arm64/include/asm/memory.h
> index 56be462c69ce..5552c8cba1e2 100644
> --- a/arch/arm64/include/asm/memory.h
> +++ b/arch/arm64/include/asm/memory.h
> @@ -44,8 +44,7 @@
> * VA_START - the first kernel virtual address.
> */
> #define VA_BITS (CONFIG_ARM64_VA_BITS)
> -#define _PAGE_OFFSET(va) (UL(0xffffffffffffffff) - \
> - (UL(1) << (va)) + 1)
> +#define _PAGE_OFFSET(va) (-(UL(1) << (va)))
> #define PAGE_OFFSET (_PAGE_OFFSET(VA_BITS))
> #define KIMAGE_VADDR (MODULES_END)
> #define BPF_JIT_REGION_START (KASAN_SHADOW_END)
> @@ -63,8 +62,7 @@
> #else
> #define VA_BITS_MIN (VA_BITS)
> #endif
> -#define _VA_START(va) (UL(0xffffffffffffffff) - \
> - (UL(1) << ((va) - 1)) + 1)
> +#define _VA_START(va) (-(UL(1) << ((va) - 1)))
This didn't make any sense to me until I realised that we changed the
meaning of VA_START when flippnig the VA space. Given that, this cleanup
looks sound to me.
However...
VA_START used to be the start of the TTBR1 address space, which was what
the "first kernel virtual address" comment was trying to say. Now it's
the first non-linear kernel virtual addres, which I think is very
confusing.
AFAICT, that change breaks at least:
* is_ttbr1_addr() -- now returns false for linear map addresses
* ptdump_check_wx() -- now skips the linear map
* ptdump_init() -- initialises start_address inccorrectly.
... so could we please find a new name for the first non-linear address,
e.g. PAGE_END, and leave VA_START as the first TTBR1 address?
Thanks,
Mark.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v5 04/15] ASoC: sun4i-i2s: Support more formats on newer SoCs
From: Code Kipper @ 2019-08-14 11:23 UTC (permalink / raw)
To: Maxime Ripard
Cc: Linux-ALSA, linux-sunxi, linux-kernel, Liam Girdwood,
Andrea Venturi (pers), Chen-Yu Tsai, Mark Brown, linux-arm-kernel
In-Reply-To: <20190814071645.33qe7bvwpbakjg2e@flea>
On Wed, 14 Aug 2019 at 13:08, Maxime Ripard <maxime.ripard@bootlin.com> wrote:
>
> On Wed, Aug 14, 2019 at 08:08:43AM +0200, codekipper@gmail.com wrote:
> > From: Marcus Cooper <codekipper@gmail.com>
> >
> > There is a need to support more formats on the newer SoCs(H3 and later).
> > Extend the formats supported to include DSP_A and DSP_B modes.
> >
> > Signed-off-by: Marcus Cooper <codekipper@gmail.com>
> > ---
> > sound/soc/sunxi/sun4i-i2s.c | 87 +++++++++++++++++++++++++++----------
> > 1 file changed, 63 insertions(+), 24 deletions(-)
> >
> > diff --git a/sound/soc/sunxi/sun4i-i2s.c b/sound/soc/sunxi/sun4i-i2s.c
> > index 34f31439ae7b..3553c17318b0 100644
> > --- a/sound/soc/sunxi/sun4i-i2s.c
> > +++ b/sound/soc/sunxi/sun4i-i2s.c
> > @@ -27,6 +27,8 @@
> > #define SUN4I_I2S_CTRL_MODE_MASK BIT(5)
> > #define SUN4I_I2S_CTRL_MODE_SLAVE (1 << 5)
> > #define SUN4I_I2S_CTRL_MODE_MASTER (0 << 5)
> > +#define SUN4I_I2S_CTRL_PCM BIT(4)
> > +#define SUN4I_I2S_CTRL_LOOP BIT(3)
> > #define SUN4I_I2S_CTRL_TX_EN BIT(2)
> > #define SUN4I_I2S_CTRL_RX_EN BIT(1)
> > #define SUN4I_I2S_CTRL_GL_EN BIT(0)
> > @@ -91,6 +93,9 @@
> > /* Defines required for sun8i-h3 support */
> > #define SUN8I_I2S_CTRL_BCLK_OUT BIT(18)
> > #define SUN8I_I2S_CTRL_LRCK_OUT BIT(17)
> > +#define SUN8I_I2S_CTRL_MODE_RIGHT_J (2 << 0)
> > +#define SUN8I_I2S_CTRL_MODE_I2S_LEFT_J (1 << 0)
> > +#define SUN8I_I2S_CTRL_MODE_PCM (0 << 0)
> >
> > #define SUN8I_I2S_FMT0_LRCK_PERIOD_MASK GENMASK(17, 8)
> > #define SUN8I_I2S_FMT0_LRCK_PERIOD(period) ((period - 1) << 8)
> > @@ -164,6 +169,7 @@ struct sun4i_i2s_quirks {
> >
> > s8 (*get_sr)(const struct sun4i_i2s *, int);
> > s8 (*get_wss)(const struct sun4i_i2s *, int);
> > + int (*set_format)(struct sun4i_i2s *, unsigned int);
> > };
> >
> > struct sun4i_i2s {
> > @@ -194,6 +200,7 @@ struct sun4i_i2s {
> >
> > unsigned int tdm_slots;
> > unsigned int slot_width;
> > + unsigned int offset;
> > };
> >
> > struct sun4i_i2s_clk_div {
> > @@ -484,19 +491,14 @@ static int sun4i_i2s_hw_params(struct snd_pcm_substream *substream,
> > i2s->slot_width : params_width(params));
> > }
> >
> > -static int sun4i_i2s_set_fmt(struct snd_soc_dai *dai, unsigned int fmt)
> > +static int sun4i_i2s_set_format(struct sun4i_i2s *i2s, unsigned int fmt)
> > {
> > - struct sun4i_i2s *i2s = snd_soc_dai_get_drvdata(dai);
> > u32 val;
> > - u32 offset = 0;
> > - u32 bclk_polarity = SUN4I_I2S_FMT0_POLARITY_NORMAL;
> > - u32 lrclk_polarity = SUN4I_I2S_FMT0_POLARITY_NORMAL;
> >
> > /* DAI Mode */
> > switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
> > case SND_SOC_DAIFMT_I2S:
> > val = SUN4I_I2S_FMT0_FMT_I2S;
> > - offset = 1;
> > break;
> > case SND_SOC_DAIFMT_LEFT_J:
> > val = SUN4I_I2S_FMT0_FMT_LEFT_J;
> > @@ -505,32 +507,64 @@ static int sun4i_i2s_set_fmt(struct snd_soc_dai *dai, unsigned int fmt)
> > val = SUN4I_I2S_FMT0_FMT_RIGHT_J;
> > break;
> > default:
> > - dev_err(dai->dev, "Unsupported format: %d\n",
> > - fmt & SND_SOC_DAIFMT_FORMAT_MASK);
> > return -EINVAL;
> > }
> >
> > - if (i2s->variant->has_chsel_offset) {
> > - /*
> > - * offset being set indicates that we're connected to an i2s
> > - * device, however offset is only used on the sun8i block and
> > - * i2s shares the same setting with the LJ format. Increment
> > - * val so that the bit to value to write is correct.
> > - */
> > - if (offset > 0)
> > - val++;
> > - /* blck offset determines whether i2s or LJ */
> > - regmap_update_bits(i2s->regmap, SUN8I_I2S_TX_CHAN_SEL_REG,
> > - SUN8I_I2S_TX_CHAN_OFFSET_MASK,
> > - SUN8I_I2S_TX_CHAN_OFFSET(offset));
> > + regmap_field_write(i2s->field_fmt_mode, val);
> > +
> > + return 0;
> > +}
> > +
> > +static int sun8i_i2s_set_format(struct sun4i_i2s *i2s, unsigned int fmt)
> > +{
> > + u32 val;
> >
> > - regmap_update_bits(i2s->regmap, SUN8I_I2S_RX_CHAN_SEL_REG,
> > - SUN8I_I2S_TX_CHAN_OFFSET_MASK,
> > - SUN8I_I2S_TX_CHAN_OFFSET(offset));
> > + /* DAI Mode */
> > + switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
> > + case SND_SOC_DAIFMT_I2S:
> > + i2s->offset = 1;
> > + case SND_SOC_DAIFMT_LEFT_J:
> > + val = SUN8I_I2S_CTRL_MODE_I2S_LEFT_J;
> > + break;
> > + case SND_SOC_DAIFMT_RIGHT_J:
> > + val = SUN8I_I2S_CTRL_MODE_RIGHT_J;
> > + break;
> > + case SND_SOC_DAIFMT_DSP_A:
> > + i2s->offset = 1;
> > + case SND_SOC_DAIFMT_DSP_B:
> > + val = SUN8I_I2S_CTRL_MODE_PCM;
> > + break;
> > +
> > + default:
> > + return -EINVAL;
> > }
> >
> > + /*
> > + * bclk offset determines whether i2s or LJ if in i2s mode and
> > + * DSP_A or DSP_B if in PCM mode.
> > + */
> > + i2s->variant->set_txchanoffset(i2s, 0);
> > + i2s->variant->set_rxchanoffset(i2s);
> > +
> > regmap_field_write(i2s->field_fmt_mode, val);
>
> It's a bit more complicated in the sun8i case. The LRCK period also
> needs to be changed when in PCM / DSP_* mode since it changes from a
> number of periods for one channel to a number of periods for all the
> channels.
Yeah I was thinking that but I don't have any hardware to test this with (been
helping out someone trying to connect to a modem).
>
> I have patches that still need a bit of rework and take care of all of
> that, I'll try to post them by the end of the week
This patch could be dropped for now or at least we just keep the
offset parts in.
BR,
CK
>
> Maxime
>
> --
> Maxime Ripard, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH 3/8] arm64: memory: Rewrite default page_to_virt()/virt_to_page()
From: Will Deacon @ 2019-08-14 11:17 UTC (permalink / raw)
To: Mark Rutland
Cc: Steve Capper, Andrey Konovalov, Geert Uytterhoeven,
Catalin Marinas, Qian Cai, linux-arm-kernel
In-Reply-To: <20190814105638.GA17931@lakrids.cambridge.arm.com>
On Wed, Aug 14, 2019 at 11:56:39AM +0100, Mark Rutland wrote:
> On Wed, Aug 14, 2019 at 10:41:19AM +0100, Will Deacon wrote:
> > On Wed, Aug 14, 2019 at 10:30:19AM +0100, Catalin Marinas wrote:
> > > On Tue, Aug 13, 2019 at 06:01:44PM +0100, Will Deacon wrote:
> > > > diff --git a/arch/arm64/include/asm/memory.h b/arch/arm64/include/asm/memory.h
> > > > index 47b4dc73b8bf..77074b3a1025 100644
> > > > --- a/arch/arm64/include/asm/memory.h
> > > > +++ b/arch/arm64/include/asm/memory.h
> > > > @@ -313,19 +313,18 @@ static inline void *phys_to_virt(phys_addr_t x)
> > > > #if !defined(CONFIG_SPARSEMEM_VMEMMAP) || defined(CONFIG_DEBUG_VIRTUAL)
> > > > #define virt_to_page(kaddr) pfn_to_page(__pa(kaddr) >> PAGE_SHIFT)
> > > > #else
> > > > -#define __virt_to_pgoff(kaddr) (((u64)(kaddr) - PAGE_OFFSET) / PAGE_SIZE * sizeof(struct page))
> > > > -#define __page_to_voff(kaddr) (((u64)(kaddr) - VMEMMAP_START) * PAGE_SIZE / sizeof(struct page))
> > > > -
> > > > -#define page_to_virt(page) ({ \
> > > > - unsigned long __addr = \
> > > > - ((__page_to_voff(page)) + PAGE_OFFSET); \
> > > > - const void *__addr_tag = \
> > > > - __tag_set((void *)__addr, page_kasan_tag(page)); \
> > > > - ((void *)__addr_tag); \
> > > > +#define page_to_virt(x) ({ \
> > > > + __typeof__(x) __page = x; \
> > >
> > > Why not struct page * directly here?
> >
> > I started out with that, but then you have to deal with const struct page *
> > as well and it gets pretty messy.
>
> What goes wrong if you always use const struct page *__page?
It would probably work, but then I wondered about the possibility of
volatile and decided that __typeof__ was cleaner.
Will
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v5 12/15] ASoC: sun4i-i2s: Add multi-lane functionality
From: Code Kipper @ 2019-08-14 11:16 UTC (permalink / raw)
To: Maxime Ripard
Cc: Linux-ALSA, linux-sunxi, linux-kernel, Liam Girdwood,
Andrea Venturi (pers), Chen-Yu Tsai, Mark Brown, linux-arm-kernel
In-Reply-To: <20190814072046.metavychqvhuohwy@flea>
On Wed, 14 Aug 2019 at 13:08, Maxime Ripard <maxime.ripard@bootlin.com> wrote:
>
> On Wed, Aug 14, 2019 at 08:08:51AM +0200, codekipper@gmail.com wrote:
> > From: Marcus Cooper <codekipper@gmail.com>
> >
> > The i2s block supports multi-lane i2s output however this functionality
> > is only possible in earlier SoCs where the pins are exposed and for
> > the i2s block used for HDMI audio on the later SoCs.
> >
> > To enable this functionality, an optional property has been added to
> > the bindings.
> >
> > Signed-off-by: Marcus Cooper <codekipper@gmail.com>
>
> Wasn't the plan to support only stereo for now?
Stereo HDMI can be introduced on the H3 and later if we get the first
three patches
merged. Post those patches is the work to get multi-channel working.
>
> Either way, that property should be documented.
I can do this...but I'm thinking we should bang our heads together to
find a solution
that we all agree on...especially if we're considering multi-channel
tdm support.
Thanks,
CK
>
> Maxime
> --
> Maxime Ripard, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH] iommu/arm-smmu-v3: add nr_ats_masters to avoid unnecessary operations
From: Will Deacon @ 2019-08-14 11:14 UTC (permalink / raw)
To: Zhen Lei
Cc: Jean-Philippe Brucker, Joerg Roedel, John Garry, linux-kernel,
iommu, Robin Murphy, linux-arm-kernel
In-Reply-To: <20190801122040.26024-1-thunder.leizhen@huawei.com>
Hi,
I've been struggling with the memory ordering requirements here. More below.
On Thu, Aug 01, 2019 at 08:20:40PM +0800, Zhen Lei wrote:
> When (smmu_domain->smmu->features & ARM_SMMU_FEAT_ATS) is true, even if a
> smmu domain does not contain any ats master, the operations of
> arm_smmu_atc_inv_to_cmd() and lock protection in arm_smmu_atc_inv_domain()
> are always executed. This will impact performance, especially in
> multi-core and stress scenarios. For my FIO test scenario, about 8%
> performance reduced.
>
> In fact, we can use a atomic member to record how many ats masters the
> smmu contains. And check that without traverse the list and check all
> masters one by one in the lock protection.
>
> Fixes: 9ce27afc0830 ("iommu/arm-smmu-v3: Add support for PCI ATS")
> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
> ---
> drivers/iommu/arm-smmu-v3.c | 10 ++++++++--
> 1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
> index a9a9fabd396804a..1b370d9aca95f94 100644
> --- a/drivers/iommu/arm-smmu-v3.c
> +++ b/drivers/iommu/arm-smmu-v3.c
> @@ -631,6 +631,7 @@ struct arm_smmu_domain {
>
> struct io_pgtable_ops *pgtbl_ops;
> bool non_strict;
> + atomic_t nr_ats_masters;
>
> enum arm_smmu_domain_stage stage;
> union {
> @@ -1531,7 +1532,7 @@ static int arm_smmu_atc_inv_domain(struct arm_smmu_domain *smmu_domain,
> struct arm_smmu_cmdq_ent cmd;
> struct arm_smmu_master *master;
>
> - if (!(smmu_domain->smmu->features & ARM_SMMU_FEAT_ATS))
> + if (!atomic_read(&smmu_domain->nr_ats_masters))
> return 0;
This feels wrong to me: the CPU can speculate ahead of time that
'nr_ats_masters' is 0, but we could have a concurrent call to '->attach()'
for an ATS-enabled device. Wouldn't it then be possible for the new device
to populate its ATC as a result of speculative accesses for the mapping that
we're tearing down?
The devices lock solves this problem by serialising invalidation with
'->attach()/->detach()' operations.
John's suggestion of RCU might work better, but I think you'll need to call
synchronize_rcu() between adding yourself to the 'devices' list and enabling
ATS.
What do you think?
> arm_smmu_atc_inv_to_cmd(ssid, iova, size, &cmd);
> @@ -1869,6 +1870,7 @@ static int arm_smmu_enable_ats(struct arm_smmu_master *master)
> size_t stu;
> struct pci_dev *pdev;
> struct arm_smmu_device *smmu = master->smmu;
> + struct arm_smmu_domain *smmu_domain = master->domain;
> struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(master->dev);
>
> if (!(smmu->features & ARM_SMMU_FEAT_ATS) || !dev_is_pci(master->dev) ||
> @@ -1887,12 +1889,15 @@ static int arm_smmu_enable_ats(struct arm_smmu_master *master)
> return ret;
>
> master->ats_enabled = true;
> + atomic_inc(&smmu_domain->nr_ats_masters);
Here, we need to make sure that concurrent invalidation sees the updated
'nr_ats_masters' value before ATS is enabled for the device, otherwise we
could miss an ATC invalidation.
I think the code above gets this guarantee because of the way that ATS is
enabled in the STE, which ensures that we issue invalidation commands before
making the STE 'live'; this has the side-effect of a write barrier before
updating PROD, which I think we also rely on for installing the CD pointer.
Put another way: writes are ordered before a subsequent command insertion.
Do you agree? If so, I'll add a comment because this is subtle and easily
overlooked.
> static void arm_smmu_disable_ats(struct arm_smmu_master *master)
> {
> struct arm_smmu_cmdq_ent cmd;
> + struct arm_smmu_domain *smmu_domain = master->domain;
>
> if (!master->ats_enabled || !dev_is_pci(master->dev))
> return;
> @@ -1901,6 +1906,7 @@ static void arm_smmu_disable_ats(struct arm_smmu_master *master)
> arm_smmu_atc_inv_master(master, &cmd);
> pci_disable_ats(to_pci_dev(master->dev));
> master->ats_enabled = false;
> + atomic_dec(&smmu_domain->nr_ats_masters);
This part is the other way around: now we need to ensure that we don't
decrement 'nr_ats_masters' until we've disabled ATS. This works for a
number of reasons, none of which are obvious:
- The control dependency from completing the prior CMD_SYNCs for tearing
down the STE and invalidating the ATC
- The spinlock handover from the CMD_SYNCs above
- The writel() when poking PCI configuration space in pci_disable_ats()
happens to be implemented with a write-write barrier
I suppose the control dependency is the most compelling one: we can't let
stores out whilst we're awaiting completion of a CMD_SYNC.
Put another way: writes are ordered after the completion of a prior CMD_SYNC.
But yeah, I need to write this down.
> static void arm_smmu_detach_dev(struct arm_smmu_master *master)
> @@ -1915,10 +1921,10 @@ static void arm_smmu_detach_dev(struct arm_smmu_master *master)
> list_del(&master->domain_head);
> spin_unlock_irqrestore(&smmu_domain->devices_lock, flags);
>
> - master->domain = NULL;
> arm_smmu_install_ste_for_dev(master);
>
> arm_smmu_disable_ats(master);
> + master->domain = NULL;
As you mentioned, this is broken. Can you simply drop this hunk completely?
Will
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [EXT] Re: [PATCH v2 09/15] dt-bindings: display: Add max-res property for mxsfb
From: Robert Chiras @ 2019-08-14 11:14 UTC (permalink / raw)
To: stefan@agner.ch, mark.rutland@arm.com, robh+dt@kernel.org
Cc: marex@denx.de, devicetree@vger.kernel.org, kernel@pengutronix.de,
airlied@linux.ie, shawnguo@kernel.org, agx@sigxcpu.org,
linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org,
dl-linux-imx, daniel@ffwll.ch, festevam@gmail.com,
s.hauer@pengutronix.de, linux-arm-kernel@lists.infradead.org
In-Reply-To: <491aff3d08f24ab4d79a4f8c139d2e44@agner.ch>
Hi Stefan,
On Mi, 2019-08-14 at 13:03 +0200, Stefan Agner wrote:
> On 2019-08-14 12:48, Robert Chiras wrote:
> >
> > Add new optional property 'max-res', to limit the maximum supported
> > resolution by the MXSFB_DRM driver.
> I would also mention the reason why we need this.
>
> I guess this needs a vendor prefix as well (fsl,max-res). I also
> would
> like to have the ack of the device tree folks here.
Rob Herring also aked be about this, and I'll copy here the reply, with
explanations:
Indeed, this limitation is actually due to bandwidth limitation, but
the problem is that this limitation comes on i.MX8M (known as mScale
850D), where the memory bandwidth cannot support: GPU/VPU workload in
the same time with both DCSS driving 4k@60 and eLCDIF driving 1080p@60.
Since eLCDIF is a secondary display we though to add the posibility to
limit it's bandwidth by limiting the resolution.
If you say that more details are needed, I can add them in the
description.
>
> --
> Stefan
>
> >
> >
> > Signed-off-by: Robert Chiras <robert.chiras@nxp.com>
> > ---
> > Documentation/devicetree/bindings/display/mxsfb.txt | 6 ++++++
> > 1 file changed, 6 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/display/mxsfb.txt
> > b/Documentation/devicetree/bindings/display/mxsfb.txt
> > index 472e1ea..55e22ed 100644
> > --- a/Documentation/devicetree/bindings/display/mxsfb.txt
> > +++ b/Documentation/devicetree/bindings/display/mxsfb.txt
> > @@ -17,6 +17,12 @@ Required properties:
> > Required sub-nodes:
> > - port: The connection to an encoder chip.
> >
> > +Optional properties:
> > +- max-res: an array with a maximum of two integers, representing
> > the
> > + maximum supported resolution, in the form of
> > + <maxX>, <maxY>; if one of the item is <0>, the
> > default
> > + driver-defined maximum resolution for that axis is
> > used
> > +
> > Example:
> >
> > lcdif1: display-controller@2220000 {
Thanks,
Robert
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v2 04/15] drm/mxsfb: Reset vital register for a proper initialization
From: Stefan Agner @ 2019-08-14 11:11 UTC (permalink / raw)
To: Robert Chiras
Cc: Marek Vasut, Mark Rutland, Pengutronix Kernel Team, devicetree,
David Airlie, Fabio Estevam, Guido Günther, linux-kernel,
dri-devel, Rob Herring, NXP Linux Team, Daniel Vetter, Shawn Guo,
Sascha Hauer, linux-arm-kernel
In-Reply-To: <1565779731-1300-5-git-send-email-robert.chiras@nxp.com>
On 2019-08-14 12:48, Robert Chiras wrote:
> Some of the regiters need, like LCDC_CTRL and CTRL2_OUTSTANDING_REQS
Typo in registers, and there is a need to many.
> needs to be properly cleared and initialized for a better start and stop
> routine.
>
> Signed-off-by: Robert Chiras <robert.chiras@nxp.com>
> ---
> drivers/gpu/drm/mxsfb/mxsfb_crtc.c | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
> diff --git a/drivers/gpu/drm/mxsfb/mxsfb_crtc.c
> b/drivers/gpu/drm/mxsfb/mxsfb_crtc.c
> index b69ace8..5e44f57 100644
> --- a/drivers/gpu/drm/mxsfb/mxsfb_crtc.c
> +++ b/drivers/gpu/drm/mxsfb/mxsfb_crtc.c
> @@ -127,6 +127,10 @@ static void mxsfb_enable_controller(struct
> mxsfb_drm_private *mxsfb)
> clk_prepare_enable(mxsfb->clk_disp_axi);
> clk_prepare_enable(mxsfb->clk);
>
> + if (mxsfb->devdata->ipversion >= 4)
> + writel(CTRL2_OUTSTANDING_REQS(REQ_16),
> + mxsfb->base + LCDC_V4_CTRL2 + REG_SET);
> +
> /* If it was disabled, re-enable the mode again */
> writel(CTRL_DOTCLK_MODE, mxsfb->base + LCDC_CTRL + REG_SET);
>
> @@ -136,12 +140,19 @@ static void mxsfb_enable_controller(struct
> mxsfb_drm_private *mxsfb)
> writel(reg, mxsfb->base + LCDC_VDCTRL4);
>
> writel(CTRL_RUN, mxsfb->base + LCDC_CTRL + REG_SET);
> + writel(CTRL1_RECOVERY_ON_UNDERFLOW, mxsfb->base + LCDC_CTRL1 + REG_SET);
This seems not to be accounted for in the commit message. Can you do
this in a separate commit?
Also I suggest to introduce CTRL1_RECOVERY_ON_UNDERFLOW in that same
commit.
--
Stefan
> }
>
> static void mxsfb_disable_controller(struct mxsfb_drm_private *mxsfb)
> {
> u32 reg;
>
> + if (mxsfb->devdata->ipversion >= 4)
> + writel(CTRL2_OUTSTANDING_REQS(0x7),
> + mxsfb->base + LCDC_V4_CTRL2 + REG_CLR);
> +
> + writel(CTRL_RUN, mxsfb->base + LCDC_CTRL + REG_CLR);
> +
> /*
> * Even if we disable the controller here, it will still continue
> * until its FIFOs are running out of data
> @@ -295,6 +306,7 @@ void mxsfb_crtc_enable(struct mxsfb_drm_private *mxsfb)
> dma_addr_t paddr;
>
> mxsfb_enable_axi_clk(mxsfb);
> + writel(0, mxsfb->base + LCDC_CTRL);
> mxsfb_crtc_mode_set_nofb(mxsfb);
>
> /* Write cur_buf as well to avoid an initial corrupt frame */
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v5 11/15] ASoC: sun4i-i2s: Add support for H6 I2S
From: Code Kipper @ 2019-08-14 11:08 UTC (permalink / raw)
To: Jernej Škrabec
Cc: Linux-ALSA, linux-kernel, Chen-Yu Tsai, Liam Girdwood,
Andrea Venturi (pers), linux-sunxi, Mark Brown, Maxime Ripard,
linux-arm-kernel
In-Reply-To: <13079463.kjevBeenX1@jernej-laptop>
On Wed, 14 Aug 2019 at 09:57, Jernej Škrabec <jernej.skrabec@siol.net> wrote:
>
> Hi!
>
> Dne sreda, 14. avgust 2019 ob 08:08:50 CEST je codekipper@gmail.com
> napisal(a):
> > From: Jernej Skrabec <jernej.skrabec@siol.net>
> >
> > H6 I2S is very similar to that in H3, except it supports up to 16
> > channels.
> >
> > Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net>
>
> Your Signed-off-by is missing here and on all other patches made originally by
> me.
ACK
>
> Best regards,
> Jernej
>
> > ---
> > sound/soc/sunxi/sun4i-i2s.c | 148 ++++++++++++++++++++++++++++++++++++
> > 1 file changed, 148 insertions(+)
> >
> > diff --git a/sound/soc/sunxi/sun4i-i2s.c b/sound/soc/sunxi/sun4i-i2s.c
> > index 6de3cb41aaf6..a8d98696fe7c 100644
> > --- a/sound/soc/sunxi/sun4i-i2s.c
> > +++ b/sound/soc/sunxi/sun4i-i2s.c
> > @@ -121,6 +121,21 @@
> > #define SUN8I_I2S_RX_CHAN_SEL_REG 0x54
> > #define SUN8I_I2S_RX_CHAN_MAP_REG 0x58
> >
> > +/* Defines required for sun50i-h6 support */
> > +#define SUN50I_H6_I2S_TX_CHAN_SEL_OFFSET_MASK GENMASK(21, 20)
> > +#define SUN50I_H6_I2S_TX_CHAN_SEL_OFFSET(offset) ((offset) << 20)
> > +#define SUN50I_H6_I2S_TX_CHAN_SEL_MASK GENMASK(19, 16)
> > +#define SUN50I_H6_I2S_TX_CHAN_SEL(chan) ((chan - 1) << 16)
> > +#define SUN50I_H6_I2S_TX_CHAN_EN_MASK GENMASK(15, 0)
> > +#define SUN50I_H6_I2S_TX_CHAN_EN(num_chan) (((1 << num_chan) - 1))
> > +
> > +#define SUN50I_H6_I2S_TX_CHAN_MAP0_REG 0x44
> > +#define SUN50I_H6_I2S_TX_CHAN_MAP1_REG 0x48
> > +
> > +#define SUN50I_H6_I2S_RX_CHAN_SEL_REG 0x64
> > +#define SUN50I_H6_I2S_RX_CHAN_MAP0_REG 0x68
> > +#define SUN50I_H6_I2S_RX_CHAN_MAP1_REG 0x6C
> > +
> > struct sun4i_i2s;
> >
> > /**
> > @@ -440,6 +455,25 @@ static void sun8i_i2s_set_rxchanoffset(const struct
> > sun4i_i2s *i2s) SUN8I_I2S_TX_CHAN_OFFSET(i2s->offset));
> > }
> >
> > +static void sun50i_h6_i2s_set_txchanoffset(const struct sun4i_i2s *i2s, int
> > output) +{
> > + if (output >= 0 && output < 4) {
> > + regmap_update_bits(i2s->regmap,
> > + SUN8I_I2S_TX_CHAN_SEL_REG +
> (output * 4),
> > +
> SUN50I_H6_I2S_TX_CHAN_SEL_OFFSET_MASK,
> > +
> SUN50I_H6_I2S_TX_CHAN_SEL_OFFSET(i2s->offset));
> > + }
> > +
> > +}
> > +
> > +static void sun50i_h6_i2s_set_rxchanoffset(const struct sun4i_i2s *i2s)
> > +{
> > + regmap_update_bits(i2s->regmap,
> > + SUN50I_H6_I2S_RX_CHAN_SEL_REG,
> > + SUN50I_H6_I2S_TX_CHAN_SEL_OFFSET_MASK,
> > + SUN50I_H6_I2S_TX_CHAN_SEL_OFFSET(i2s-
> >offset));
> > +}
> > +
> > static void sun8i_i2s_set_txchanen(const struct sun4i_i2s *i2s, int output,
> > int channel)
> > {
> > @@ -459,6 +493,26 @@ static void sun8i_i2s_set_rxchanen(const struct
> > sun4i_i2s *i2s, int channel) SUN8I_I2S_TX_CHAN_EN(channel));
> > }
> >
> > +
> > +static void sun50i_h6_i2s_set_txchanen(const struct sun4i_i2s *i2s, int
> > output, + int channel)
> > +{
> > + if (output >= 0 && output < 4) {
> > + regmap_update_bits(i2s->regmap,
> > + SUN8I_I2S_TX_CHAN_SEL_REG +
> (output * 4),
> > + SUN50I_H6_I2S_TX_CHAN_EN_MASK,
> > +
> SUN50I_H6_I2S_TX_CHAN_EN(channel));
> > + }
> > +}
> > +
> > +static void sun50i_h6_i2s_set_rxchanen(const struct sun4i_i2s *i2s, int
> > channel) +{
> > + regmap_update_bits(i2s->regmap,
> > + SUN50I_H6_I2S_RX_CHAN_SEL_REG,
> > + SUN50I_H6_I2S_TX_CHAN_EN_MASK,
> > + SUN50I_H6_I2S_TX_CHAN_EN(channel));
> > +}
> > +
> > static void sun4i_i2s_set_txchansel(const struct sun4i_i2s *i2s, int
> > output, int channel)
> > {
> > @@ -495,6 +549,25 @@ static void sun8i_i2s_set_rxchansel(const struct
> > sun4i_i2s *i2s, int channel) SUN8I_I2S_TX_CHAN_SEL(channel));
> > }
> >
> > +static void sun50i_h6_i2s_set_txchansel(const struct sun4i_i2s *i2s, int
> > output, + int channel)
> > +{
> > + if (output >= 0 && output < 4) {
> > + regmap_update_bits(i2s->regmap,
> > + SUN8I_I2S_TX_CHAN_SEL_REG +
> (output * 4),
> > + SUN50I_H6_I2S_TX_CHAN_SEL_MASK,
> > +
> SUN50I_H6_I2S_TX_CHAN_SEL(channel));
> > + }
> > +}
> > +
> > +static void sun50i_h6_i2s_set_rxchansel(const struct sun4i_i2s *i2s, int
> > channel) +{
> > + regmap_update_bits(i2s->regmap,
> > + SUN50I_H6_I2S_RX_CHAN_SEL_REG,
> > + SUN50I_H6_I2S_TX_CHAN_SEL_MASK,
> > + SUN50I_H6_I2S_TX_CHAN_SEL(channel));
> > +}
> > +
> > static void sun4i_i2s_set_txchanmap(const struct sun4i_i2s *i2s, int
> > output, int channel)
> > {
> > @@ -520,6 +593,20 @@ static void sun8i_i2s_set_rxchanmap(const struct
> > sun4i_i2s *i2s, int channel) regmap_write(i2s->regmap,
> > SUN8I_I2S_RX_CHAN_MAP_REG, channel);
> > }
> >
> > +static void sun50i_h6_i2s_set_txchanmap(const struct sun4i_i2s *i2s, int
> > output, + int channel)
> > +{
> > + if (output >= 0 && output < 4) {
> > + regmap_write(i2s->regmap,
> > + SUN50I_H6_I2S_TX_CHAN_MAP1_REG + (output
> * 8), channel);
> > + }
> > +}
> > +
> > +static void sun50i_h6_i2s_set_rxchanmap(const struct sun4i_i2s *i2s, int
> > channel) +{
> > + regmap_write(i2s->regmap, SUN50I_H6_I2S_RX_CHAN_MAP1_REG, channel);
> > +}
> > +
> > static int sun4i_i2s_hw_params(struct snd_pcm_substream *substream,
> > struct snd_pcm_hw_params *params,
> > struct snd_soc_dai *dai)
> > @@ -996,6 +1083,22 @@ static const struct reg_default
> > sun8i_i2s_reg_defaults[] = { { SUN8I_I2S_RX_CHAN_MAP_REG, 0x00000000 },
> > };
> >
> > +static const struct reg_default sun50i_i2s_reg_defaults[] = {
> > + { SUN4I_I2S_CTRL_REG, 0x00060000 },
> > + { SUN4I_I2S_FMT0_REG, 0x00000033 },
> > + { SUN4I_I2S_FMT1_REG, 0x00000030 },
> > + { SUN4I_I2S_FIFO_CTRL_REG, 0x000400f0 },
> > + { SUN4I_I2S_DMA_INT_CTRL_REG, 0x00000000 },
> > + { SUN4I_I2S_CLK_DIV_REG, 0x00000000 },
> > + { SUN8I_I2S_CHAN_CFG_REG, 0x00000000 },
> > + { SUN8I_I2S_TX_CHAN_SEL_REG, 0x00000000 },
> > + { SUN50I_H6_I2S_TX_CHAN_MAP0_REG, 0x00000000 },
> > + { SUN50I_H6_I2S_TX_CHAN_MAP1_REG, 0x00000000 },
> > + { SUN50I_H6_I2S_RX_CHAN_SEL_REG, 0x00000000 },
> > + { SUN50I_H6_I2S_RX_CHAN_MAP0_REG, 0x00000000 },
> > + { SUN50I_H6_I2S_RX_CHAN_MAP1_REG, 0x00000000 },
> > +};
> > +
> > static const struct regmap_config sun4i_i2s_regmap_config = {
> > .reg_bits = 32,
> > .reg_stride = 4,
> > @@ -1023,6 +1126,19 @@ static const struct regmap_config
> > sun8i_i2s_regmap_config = { .volatile_reg = sun8i_i2s_volatile_reg,
> > };
> >
> > +static const struct regmap_config sun50i_i2s_regmap_config = {
> > + .reg_bits = 32,
> > + .reg_stride = 4,
> > + .val_bits = 32,
> > + .max_register = SUN50I_H6_I2S_RX_CHAN_MAP1_REG,
> > + .cache_type = REGCACHE_FLAT,
> > + .reg_defaults = sun50i_i2s_reg_defaults,
> > + .num_reg_defaults = ARRAY_SIZE(sun50i_i2s_reg_defaults),
> > + .writeable_reg = sun4i_i2s_wr_reg,
> > + .readable_reg = sun8i_i2s_rd_reg,
> > + .volatile_reg = sun8i_i2s_volatile_reg,
> > +};
> > +
> > static int sun4i_i2s_runtime_resume(struct device *dev)
> > {
> > struct sun4i_i2s *i2s = dev_get_drvdata(dev);
> > @@ -1197,6 +1313,34 @@ static const struct sun4i_i2s_quirks
> > sun50i_a64_codec_i2s_quirks = { .set_rxchanmap =
> sun4i_i2s_set_rxchanmap,
> > };
> >
> > +static const struct sun4i_i2s_quirks sun50i_h6_i2s_quirks = {
> > + .has_reset = true,
> > + .reg_offset_txdata = SUN8I_I2S_FIFO_TX_REG,
> > + .sun4i_i2s_regmap = &sun50i_i2s_regmap_config,
> > + .has_fmt_set_lrck_period = true,
> > + .has_chcfg = true,
> > + .has_chsel_tx_chen = true,
> > + .has_chsel_offset = true,
> > + .field_clkdiv_mclk_en = REG_FIELD(SUN4I_I2S_CLK_DIV_REG, 8, 8),
> > + .field_fmt_wss = REG_FIELD(SUN4I_I2S_FMT0_REG,
> 0, 2),
> > + .field_fmt_sr = REG_FIELD(SUN4I_I2S_FMT0_REG, 4, 6),
> > + .field_fmt_bclk = REG_FIELD(SUN4I_I2S_FMT0_REG,
> 7, 7),
> > + .field_fmt_lrclk = REG_FIELD(SUN4I_I2S_FMT0_REG, 19, 19),
> > + .field_fmt_mode = REG_FIELD(SUN4I_I2S_CTRL_REG, 4,
> 5),
> > + .field_fmt_sext = REG_FIELD(SUN4I_I2S_FMT1_REG,
> 4, 5),
> > + .get_sr = sun8i_i2s_get_sr_wss,
> > + .get_wss = sun8i_i2s_get_sr_wss,
> > + .set_format = sun8i_i2s_set_format,
> > + .set_txchanoffset = sun50i_h6_i2s_set_txchanoffset,
> > + .set_rxchanoffset = sun50i_h6_i2s_set_rxchanoffset,
> > + .set_txchanen = sun50i_h6_i2s_set_txchanen,
> > + .set_rxchanen = sun50i_h6_i2s_set_rxchanen,
> > + .set_txchansel = sun50i_h6_i2s_set_txchansel,
> > + .set_rxchansel = sun50i_h6_i2s_set_rxchansel,
> > + .set_txchanmap = sun50i_h6_i2s_set_txchanmap,
> > + .set_rxchanmap = sun50i_h6_i2s_set_rxchanmap,
> > +};
> > +
> > static int sun4i_i2s_init_regmap_fields(struct device *dev,
> > struct sun4i_i2s *i2s)
> > {
> > @@ -1389,6 +1533,10 @@ static const struct of_device_id sun4i_i2s_match[] =
> > { .compatible = "allwinner,sun50i-a64-codec-i2s",
> > .data = &sun50i_a64_codec_i2s_quirks,
> > },
> > + {
> > + .compatible = "allwinner,sun50i-h6-i2s",
> > + .data = &sun50i_h6_i2s_quirks,
> > + },
> > {}
> > };
> > MODULE_DEVICE_TABLE(of, sun4i_i2s_match);
>
>
>
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v5 02/15] ASoC: sun4i-i2s: Add set_tdm_slot functionality
From: Maxime Ripard @ 2019-08-14 7:09 UTC (permalink / raw)
To: codekipper
Cc: alsa-devel, linux-sunxi, linux-kernel, lgirdwood, be17068, wens,
broonie, linux-arm-kernel
In-Reply-To: <20190814060854.26345-3-codekipper@gmail.com>
[-- Attachment #1.1: Type: text/plain, Size: 1992 bytes --]
Hi,
On Wed, Aug 14, 2019 at 08:08:41AM +0200, codekipper@gmail.com wrote:
> From: Marcus Cooper <codekipper@gmail.com>
>
> Codecs without a control connection such as i2s based HDMI audio and
> the Pine64 DAC require a different amount of bit clocks per frame than
> what is calculated by the sample width. Use the tdm slot bindings to
> provide this mechanism.
>
> Signed-off-by: Marcus Cooper <codekipper@gmail.com>
> ---
> sound/soc/sunxi/sun4i-i2s.c | 23 +++++++++++++++++++++--
> 1 file changed, 21 insertions(+), 2 deletions(-)
>
> diff --git a/sound/soc/sunxi/sun4i-i2s.c b/sound/soc/sunxi/sun4i-i2s.c
> index 8201334a059b..7c37b6291df0 100644
> --- a/sound/soc/sunxi/sun4i-i2s.c
> +++ b/sound/soc/sunxi/sun4i-i2s.c
> @@ -195,6 +195,9 @@ struct sun4i_i2s {
> struct regmap_field *field_rxchansel;
>
> const struct sun4i_i2s_quirks *variant;
> +
> + unsigned int tdm_slots;
> + unsigned int slot_width;
> };
>
> struct sun4i_i2s_clk_div {
> @@ -346,7 +349,7 @@ static int sun4i_i2s_set_clk_rate(struct snd_soc_dai *dai,
> if (i2s->variant->has_fmt_set_lrck_period)
> regmap_update_bits(i2s->regmap, SUN4I_I2S_FMT0_REG,
> SUN8I_I2S_FMT0_LRCK_PERIOD_MASK,
> - SUN8I_I2S_FMT0_LRCK_PERIOD(32));
> + SUN8I_I2S_FMT0_LRCK_PERIOD(word_size));
>
>
> /* Set sign extension to pad out LSB with 0 */
> regmap_field_write(i2s->field_fmt_sext, 0);
> @@ -450,7 +453,8 @@ static int sun4i_i2s_hw_params(struct snd_pcm_substream *substream,
> regmap_field_write(i2s->field_fmt_sr, sr);
>
> return sun4i_i2s_set_clk_rate(dai, params_rate(params),
> - params_width(params));
> + i2s->tdm_slots ?
> + i2s->slot_width : params_width(params));
This is slightly more complicated than that.
On the H3 (and all related ones), the CHAN_CFG_TX_SLOT_NUM and
_RX_SLOT_NUM fields in the CHAN_CFG register need to be set to the
number of slots as well.
Maxime
--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
[-- Attachment #2: Type: text/plain, Size: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v5 04/15] ASoC: sun4i-i2s: Support more formats on newer SoCs
From: Maxime Ripard @ 2019-08-14 7:16 UTC (permalink / raw)
To: codekipper
Cc: alsa-devel, linux-sunxi, linux-kernel, lgirdwood, be17068, wens,
broonie, linux-arm-kernel
In-Reply-To: <20190814060854.26345-5-codekipper@gmail.com>
[-- Attachment #1.1: Type: text/plain, Size: 4892 bytes --]
On Wed, Aug 14, 2019 at 08:08:43AM +0200, codekipper@gmail.com wrote:
> From: Marcus Cooper <codekipper@gmail.com>
>
> There is a need to support more formats on the newer SoCs(H3 and later).
> Extend the formats supported to include DSP_A and DSP_B modes.
>
> Signed-off-by: Marcus Cooper <codekipper@gmail.com>
> ---
> sound/soc/sunxi/sun4i-i2s.c | 87 +++++++++++++++++++++++++++----------
> 1 file changed, 63 insertions(+), 24 deletions(-)
>
> diff --git a/sound/soc/sunxi/sun4i-i2s.c b/sound/soc/sunxi/sun4i-i2s.c
> index 34f31439ae7b..3553c17318b0 100644
> --- a/sound/soc/sunxi/sun4i-i2s.c
> +++ b/sound/soc/sunxi/sun4i-i2s.c
> @@ -27,6 +27,8 @@
> #define SUN4I_I2S_CTRL_MODE_MASK BIT(5)
> #define SUN4I_I2S_CTRL_MODE_SLAVE (1 << 5)
> #define SUN4I_I2S_CTRL_MODE_MASTER (0 << 5)
> +#define SUN4I_I2S_CTRL_PCM BIT(4)
> +#define SUN4I_I2S_CTRL_LOOP BIT(3)
> #define SUN4I_I2S_CTRL_TX_EN BIT(2)
> #define SUN4I_I2S_CTRL_RX_EN BIT(1)
> #define SUN4I_I2S_CTRL_GL_EN BIT(0)
> @@ -91,6 +93,9 @@
> /* Defines required for sun8i-h3 support */
> #define SUN8I_I2S_CTRL_BCLK_OUT BIT(18)
> #define SUN8I_I2S_CTRL_LRCK_OUT BIT(17)
> +#define SUN8I_I2S_CTRL_MODE_RIGHT_J (2 << 0)
> +#define SUN8I_I2S_CTRL_MODE_I2S_LEFT_J (1 << 0)
> +#define SUN8I_I2S_CTRL_MODE_PCM (0 << 0)
>
> #define SUN8I_I2S_FMT0_LRCK_PERIOD_MASK GENMASK(17, 8)
> #define SUN8I_I2S_FMT0_LRCK_PERIOD(period) ((period - 1) << 8)
> @@ -164,6 +169,7 @@ struct sun4i_i2s_quirks {
>
> s8 (*get_sr)(const struct sun4i_i2s *, int);
> s8 (*get_wss)(const struct sun4i_i2s *, int);
> + int (*set_format)(struct sun4i_i2s *, unsigned int);
> };
>
> struct sun4i_i2s {
> @@ -194,6 +200,7 @@ struct sun4i_i2s {
>
> unsigned int tdm_slots;
> unsigned int slot_width;
> + unsigned int offset;
> };
>
> struct sun4i_i2s_clk_div {
> @@ -484,19 +491,14 @@ static int sun4i_i2s_hw_params(struct snd_pcm_substream *substream,
> i2s->slot_width : params_width(params));
> }
>
> -static int sun4i_i2s_set_fmt(struct snd_soc_dai *dai, unsigned int fmt)
> +static int sun4i_i2s_set_format(struct sun4i_i2s *i2s, unsigned int fmt)
> {
> - struct sun4i_i2s *i2s = snd_soc_dai_get_drvdata(dai);
> u32 val;
> - u32 offset = 0;
> - u32 bclk_polarity = SUN4I_I2S_FMT0_POLARITY_NORMAL;
> - u32 lrclk_polarity = SUN4I_I2S_FMT0_POLARITY_NORMAL;
>
> /* DAI Mode */
> switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
> case SND_SOC_DAIFMT_I2S:
> val = SUN4I_I2S_FMT0_FMT_I2S;
> - offset = 1;
> break;
> case SND_SOC_DAIFMT_LEFT_J:
> val = SUN4I_I2S_FMT0_FMT_LEFT_J;
> @@ -505,32 +507,64 @@ static int sun4i_i2s_set_fmt(struct snd_soc_dai *dai, unsigned int fmt)
> val = SUN4I_I2S_FMT0_FMT_RIGHT_J;
> break;
> default:
> - dev_err(dai->dev, "Unsupported format: %d\n",
> - fmt & SND_SOC_DAIFMT_FORMAT_MASK);
> return -EINVAL;
> }
>
> - if (i2s->variant->has_chsel_offset) {
> - /*
> - * offset being set indicates that we're connected to an i2s
> - * device, however offset is only used on the sun8i block and
> - * i2s shares the same setting with the LJ format. Increment
> - * val so that the bit to value to write is correct.
> - */
> - if (offset > 0)
> - val++;
> - /* blck offset determines whether i2s or LJ */
> - regmap_update_bits(i2s->regmap, SUN8I_I2S_TX_CHAN_SEL_REG,
> - SUN8I_I2S_TX_CHAN_OFFSET_MASK,
> - SUN8I_I2S_TX_CHAN_OFFSET(offset));
> + regmap_field_write(i2s->field_fmt_mode, val);
> +
> + return 0;
> +}
> +
> +static int sun8i_i2s_set_format(struct sun4i_i2s *i2s, unsigned int fmt)
> +{
> + u32 val;
>
> - regmap_update_bits(i2s->regmap, SUN8I_I2S_RX_CHAN_SEL_REG,
> - SUN8I_I2S_TX_CHAN_OFFSET_MASK,
> - SUN8I_I2S_TX_CHAN_OFFSET(offset));
> + /* DAI Mode */
> + switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
> + case SND_SOC_DAIFMT_I2S:
> + i2s->offset = 1;
> + case SND_SOC_DAIFMT_LEFT_J:
> + val = SUN8I_I2S_CTRL_MODE_I2S_LEFT_J;
> + break;
> + case SND_SOC_DAIFMT_RIGHT_J:
> + val = SUN8I_I2S_CTRL_MODE_RIGHT_J;
> + break;
> + case SND_SOC_DAIFMT_DSP_A:
> + i2s->offset = 1;
> + case SND_SOC_DAIFMT_DSP_B:
> + val = SUN8I_I2S_CTRL_MODE_PCM;
> + break;
> +
> + default:
> + return -EINVAL;
> }
>
> + /*
> + * bclk offset determines whether i2s or LJ if in i2s mode and
> + * DSP_A or DSP_B if in PCM mode.
> + */
> + i2s->variant->set_txchanoffset(i2s, 0);
> + i2s->variant->set_rxchanoffset(i2s);
> +
> regmap_field_write(i2s->field_fmt_mode, val);
It's a bit more complicated in the sun8i case. The LRCK period also
needs to be changed when in PCM / DSP_* mode since it changes from a
number of periods for one channel to a number of periods for all the
channels.
I have patches that still need a bit of rework and take care of all of
that, I'll try to post them by the end of the week
Maxime
--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
[-- Attachment #2: Type: text/plain, Size: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v5 03/15] ASoC: sun4i-i2s: Correct divider calculations
From: Maxime Ripard @ 2019-08-14 7:13 UTC (permalink / raw)
To: codekipper
Cc: alsa-devel, linux-sunxi, linux-kernel, lgirdwood, be17068, wens,
broonie, linux-arm-kernel
In-Reply-To: <20190814060854.26345-4-codekipper@gmail.com>
[-- Attachment #1.1: Type: text/plain, Size: 3322 bytes --]
Hi,
(I just noticed this, but can you update my mail address, it's not
@free-electrons for quite a while, you probably want to change your
scripts to use mripard@kernel.org)
On Wed, Aug 14, 2019 at 08:08:42AM +0200, codekipper@gmail.com wrote:
> From: Marcus Cooper <codekipper@gmail.com>
>
> The clock division circuitry is different on the H3 and later SoCs.
> The division of bclk is now based on pll2.
>
> Signed-off-by: Marcus Cooper <codekipper@gmail.com>
> ---
> sound/soc/sunxi/sun4i-i2s.c | 73 +++++++++++++++++++++++++------------
> 1 file changed, 49 insertions(+), 24 deletions(-)
>
> diff --git a/sound/soc/sunxi/sun4i-i2s.c b/sound/soc/sunxi/sun4i-i2s.c
> index 7c37b6291df0..34f31439ae7b 100644
> --- a/sound/soc/sunxi/sun4i-i2s.c
> +++ b/sound/soc/sunxi/sun4i-i2s.c
> @@ -127,8 +127,6 @@ struct sun4i_i2s;
> * @has_chsel_offset: SoC uses offset for selecting dai operational mode.
> * @reg_offset_txdata: offset of the tx fifo.
> * @sun4i_i2s_regmap: regmap config to use.
> - * @mclk_offset: Value by which mclkdiv needs to be adjusted.
> - * @bclk_offset: Value by which bclkdiv needs to be adjusted.
> * @field_clkdiv_mclk_en: regmap field to enable mclk output.
> * @field_fmt_wss: regmap field to set word select size.
> * @field_fmt_sr: regmap field to set sample resolution.
> @@ -150,8 +148,6 @@ struct sun4i_i2s_quirks {
> bool has_chsel_offset;
> unsigned int reg_offset_txdata; /* TX FIFO */
> const struct regmap_config *sun4i_i2s_regmap;
> - unsigned int mclk_offset;
> - unsigned int bclk_offset;
>
> /* Register fields for i2s */
> struct reg_field field_clkdiv_mclk_en;
> @@ -212,7 +208,25 @@ static const struct sun4i_i2s_clk_div sun4i_i2s_bclk_div[] = {
> { .div = 8, .val = 3 },
> { .div = 12, .val = 4 },
> { .div = 16, .val = 5 },
> - /* TODO - extend divide ratio supported by newer SoCs */
> +};
> +
> +static const struct sun4i_i2s_clk_div sun8i_i2s_clk_div[] = {
> + { .div = 0, .val = 0 },
Having a divider of 0 seems like a bad idea.
> + { .div = 1, .val = 1 },
> + { .div = 2, .val = 2 },
> + { .div = 4, .val = 3 },
> + { .div = 6, .val = 4 },
> + { .div = 8, .val = 5 },
> + { .div = 12, .val = 6 },
> + { .div = 16, .val = 7 },
> + { .div = 24, .val = 8 },
> + { .div = 32, .val = 9 },
> + { .div = 48, .val = 10 },
> + { .div = 64, .val = 11 },
> + { .div = 96, .val = 12 },
> + { .div = 128, .val = 13 },
> + { .div = 176, .val = 14 },
> + { .div = 192, .val = 15 },
> };
>
> static const struct sun4i_i2s_clk_div sun4i_i2s_mclk_div[] = {
> @@ -224,21 +238,21 @@ static const struct sun4i_i2s_clk_div sun4i_i2s_mclk_div[] = {
> { .div = 12, .val = 5 },
> { .div = 16, .val = 6 },
> { .div = 24, .val = 7 },
> - /* TODO - extend divide ratio supported by newer SoCs */
> };
>
> static int sun4i_i2s_get_bclk_div(struct sun4i_i2s *i2s,
> unsigned int oversample_rate,
> - unsigned int word_size)
> + unsigned int word_size,
> + const struct sun4i_i2s_clk_div *bdiv,
> + unsigned int size)
Wouldn't it be simpler to just have the divider list in the variant
structure? It would avoid having to refactor all the functions, and
it's not like it's really going to change from one call to another
anyway.
Maxime
--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
[-- Attachment #2: Type: text/plain, Size: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v5 12/15] ASoC: sun4i-i2s: Add multi-lane functionality
From: Maxime Ripard @ 2019-08-14 7:20 UTC (permalink / raw)
To: codekipper
Cc: alsa-devel, linux-sunxi, linux-kernel, lgirdwood, be17068, wens,
broonie, linux-arm-kernel
In-Reply-To: <20190814060854.26345-13-codekipper@gmail.com>
[-- Attachment #1.1: Type: text/plain, Size: 651 bytes --]
On Wed, Aug 14, 2019 at 08:08:51AM +0200, codekipper@gmail.com wrote:
> From: Marcus Cooper <codekipper@gmail.com>
>
> The i2s block supports multi-lane i2s output however this functionality
> is only possible in earlier SoCs where the pins are exposed and for
> the i2s block used for HDMI audio on the later SoCs.
>
> To enable this functionality, an optional property has been added to
> the bindings.
>
> Signed-off-by: Marcus Cooper <codekipper@gmail.com>
Wasn't the plan to support only stereo for now?
Either way, that property should be documented.
Maxime
--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
[-- Attachment #2: Type: text/plain, Size: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v5 15/15] ASoC: sun4i-i2s: Adjust regmap settings
From: Maxime Ripard @ 2019-08-14 7:20 UTC (permalink / raw)
To: codekipper
Cc: alsa-devel, linux-sunxi, linux-kernel, lgirdwood, be17068, wens,
broonie, linux-arm-kernel
In-Reply-To: <20190814060854.26345-16-codekipper@gmail.com>
[-- Attachment #1.1: Type: text/plain, Size: 506 bytes --]
On Wed, Aug 14, 2019 at 08:08:54AM +0200, codekipper@gmail.com wrote:
> From: Marcus Cooper <codekipper@gmail.com>
>
> Bypass the regmap cache when flushing the i2s FIFOs and modify the tables
> to reflect this.
>
> Signed-off-by: Marcus Cooper <codekipper@gmail.com>
This patch looks like it's fixing something while the commit log
doesn't mention what is being fixed.
Having some context here would be great.
Maxime
--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
[-- Attachment #2: Type: text/plain, Size: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v2 12/15] drm/mxsfb: Improve the axi clock usage
From: Stefan Agner @ 2019-08-14 11:06 UTC (permalink / raw)
To: Robert Chiras
Cc: Marek Vasut, Mark Rutland, Pengutronix Kernel Team, devicetree,
David Airlie, Fabio Estevam, Guido Günther, linux-kernel,
dri-devel, Rob Herring, NXP Linux Team, Daniel Vetter, Shawn Guo,
Sascha Hauer, linux-arm-kernel
In-Reply-To: <1565779731-1300-13-git-send-email-robert.chiras@nxp.com>
On 2019-08-14 12:48, Robert Chiras wrote:
> Currently, the enable of the axi clock return status is ignored, causing
> issues when the enable fails then we try to disable it. Therefore, it is
> better to check the return status and disable it only when enable
> succeeded.
Is this actually the case in real world sometimes? Why is it failing?
I guess if we do this in one place, we should do it in all places (e.g.
also in mxsfb_crtc_enable, mxsfb_plane_atomic_update..)
--
Stefan
> Also, remove the helper functions around clk_axi, since we can directly
> use the clk API function for enable/disable the clock. Those functions
> are already checking for NULL clk and returning 0 if that's the case.
>
> Signed-off-by: Robert Chiras <robert.chiras@nxp.com>
> Acked-by: Leonard Crestez <leonard.crestez@nxp.com>
> ---
> drivers/gpu/drm/mxsfb/mxsfb_crtc.c | 8 ++++----
> drivers/gpu/drm/mxsfb/mxsfb_drv.c | 32 +++++++++++++-------------------
> drivers/gpu/drm/mxsfb/mxsfb_drv.h | 3 ---
> 3 files changed, 17 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/gpu/drm/mxsfb/mxsfb_crtc.c
> b/drivers/gpu/drm/mxsfb/mxsfb_crtc.c
> index a4ba368..e727f5e 100644
> --- a/drivers/gpu/drm/mxsfb/mxsfb_crtc.c
> +++ b/drivers/gpu/drm/mxsfb/mxsfb_crtc.c
> @@ -408,7 +408,7 @@ void mxsfb_crtc_enable(struct mxsfb_drm_private *mxsfb)
> {
> dma_addr_t paddr;
>
> - mxsfb_enable_axi_clk(mxsfb);
> + clk_prepare_enable(mxsfb->clk_axi);
> writel(0, mxsfb->base + LCDC_CTRL);
> mxsfb_crtc_mode_set_nofb(mxsfb);
>
> @@ -425,7 +425,7 @@ void mxsfb_crtc_enable(struct mxsfb_drm_private *mxsfb)
> void mxsfb_crtc_disable(struct mxsfb_drm_private *mxsfb)
> {
> mxsfb_disable_controller(mxsfb);
> - mxsfb_disable_axi_clk(mxsfb);
> + clk_disable_unprepare(mxsfb->clk_axi);
> }
>
> void mxsfb_plane_atomic_update(struct mxsfb_drm_private *mxsfb,
> @@ -451,8 +451,8 @@ void mxsfb_plane_atomic_update(struct
> mxsfb_drm_private *mxsfb,
>
> paddr = mxsfb_get_fb_paddr(mxsfb);
> if (paddr) {
> - mxsfb_enable_axi_clk(mxsfb);
> + clk_prepare_enable(mxsfb->clk_axi);
> writel(paddr, mxsfb->base + mxsfb->devdata->next_buf);
> - mxsfb_disable_axi_clk(mxsfb);
> + clk_disable_unprepare(mxsfb->clk_axi);
> }
> }
> diff --git a/drivers/gpu/drm/mxsfb/mxsfb_drv.c
> b/drivers/gpu/drm/mxsfb/mxsfb_drv.c
> index 6dae2bd..694b287 100644
> --- a/drivers/gpu/drm/mxsfb/mxsfb_drv.c
> +++ b/drivers/gpu/drm/mxsfb/mxsfb_drv.c
> @@ -97,18 +97,6 @@ drm_pipe_to_mxsfb_drm_private(struct
> drm_simple_display_pipe *pipe)
> return container_of(pipe, struct mxsfb_drm_private, pipe);
> }
>
> -void mxsfb_enable_axi_clk(struct mxsfb_drm_private *mxsfb)
> -{
> - if (mxsfb->clk_axi)
> - clk_prepare_enable(mxsfb->clk_axi);
> -}
> -
> -void mxsfb_disable_axi_clk(struct mxsfb_drm_private *mxsfb)
> -{
> - if (mxsfb->clk_axi)
> - clk_disable_unprepare(mxsfb->clk_axi);
> -}
> -
> /**
> * mxsfb_atomic_helper_check - validate state object
> * @dev: DRM device
> @@ -229,25 +217,31 @@ static void mxsfb_pipe_update(struct
> drm_simple_display_pipe *pipe,
> static int mxsfb_pipe_enable_vblank(struct drm_simple_display_pipe *pipe)
> {
> struct mxsfb_drm_private *mxsfb = drm_pipe_to_mxsfb_drm_private(pipe);
> + int ret = 0;
> +
> + ret = clk_prepare_enable(mxsfb->clk_axi);
> + if (ret)
> + return ret;
>
> /* Clear and enable VBLANK IRQ */
> - mxsfb_enable_axi_clk(mxsfb);
> writel(CTRL1_CUR_FRAME_DONE_IRQ, mxsfb->base + LCDC_CTRL1 + REG_CLR);
> writel(CTRL1_CUR_FRAME_DONE_IRQ_EN, mxsfb->base + LCDC_CTRL1 + REG_SET);
> - mxsfb_disable_axi_clk(mxsfb);
> + clk_disable_unprepare(mxsfb->clk_axi);
>
> - return 0;
> + return ret;
> }
>
> static void mxsfb_pipe_disable_vblank(struct drm_simple_display_pipe *pipe)
> {
> struct mxsfb_drm_private *mxsfb = drm_pipe_to_mxsfb_drm_private(pipe);
>
> + if (clk_prepare_enable(mxsfb->clk_axi))
> + return;
> +
> /* Disable and clear VBLANK IRQ */
> - mxsfb_enable_axi_clk(mxsfb);
> writel(CTRL1_CUR_FRAME_DONE_IRQ_EN, mxsfb->base + LCDC_CTRL1 + REG_CLR);
> writel(CTRL1_CUR_FRAME_DONE_IRQ, mxsfb->base + LCDC_CTRL1 + REG_CLR);
> - mxsfb_disable_axi_clk(mxsfb);
> + clk_disable_unprepare(mxsfb->clk_axi);
> }
>
> static struct drm_simple_display_pipe_funcs mxsfb_funcs = {
> @@ -413,7 +407,7 @@ static irqreturn_t mxsfb_irq_handler(int irq, void *data)
> struct mxsfb_drm_private *mxsfb = drm->dev_private;
> u32 reg;
>
> - mxsfb_enable_axi_clk(mxsfb);
> + clk_prepare_enable(mxsfb->clk_axi);
>
> reg = readl(mxsfb->base + LCDC_CTRL1);
>
> @@ -422,7 +416,7 @@ static irqreturn_t mxsfb_irq_handler(int irq, void *data)
>
> writel(CTRL1_CUR_FRAME_DONE_IRQ, mxsfb->base + LCDC_CTRL1 + REG_CLR);
>
> - mxsfb_disable_axi_clk(mxsfb);
> + clk_disable_unprepare(mxsfb->clk_axi);
>
> return IRQ_HANDLED;
> }
> diff --git a/drivers/gpu/drm/mxsfb/mxsfb_drv.h
> b/drivers/gpu/drm/mxsfb/mxsfb_drv.h
> index 8fb65d3..d6df8fe 100644
> --- a/drivers/gpu/drm/mxsfb/mxsfb_drv.h
> +++ b/drivers/gpu/drm/mxsfb/mxsfb_drv.h
> @@ -37,9 +37,6 @@ struct mxsfb_drm_private {
> int mxsfb_setup_crtc(struct drm_device *dev);
> int mxsfb_create_output(struct drm_device *dev);
>
> -void mxsfb_enable_axi_clk(struct mxsfb_drm_private *mxsfb);
> -void mxsfb_disable_axi_clk(struct mxsfb_drm_private *mxsfb);
> -
> void mxsfb_crtc_enable(struct mxsfb_drm_private *mxsfb);
> void mxsfb_crtc_disable(struct mxsfb_drm_private *mxsfb);
> void mxsfb_plane_atomic_update(struct mxsfb_drm_private *mxsfb,
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v5 01/15] ASoC: sun4i-i2s: Add regmap field to sign extend sample
From: Maxime Ripard @ 2019-08-14 6:43 UTC (permalink / raw)
To: codekipper
Cc: alsa-devel, linux-sunxi, linux-kernel, lgirdwood, be17068, wens,
broonie, linux-arm-kernel
In-Reply-To: <20190814060854.26345-2-codekipper@gmail.com>
[-- Attachment #1.1: Type: text/plain, Size: 4571 bytes --]
Hi,
On Wed, Aug 14, 2019 at 08:08:40AM +0200, codekipper@gmail.com wrote:
> From: Marcus Cooper <codekipper@gmail.com>
>
> On the newer SoCs such as the H3 and A64 this is set by default
> to transfer a 0 after each sample in each slot. However the A10
> and A20 SoCs that this driver was developed on had a default
> setting where it padded the audio gain with zeros.
>
> This isn't a problem whilst we have only support for 16bit audio
> but with larger sample resolution rates in the pipeline then SEXT
> bits should be cleared so that they also pad at the LSB. Without
> this the audio gets distorted.
>
> Signed-off-by: Marcus Cooper <codekipper@gmail.com>
> ---
> sound/soc/sunxi/sun4i-i2s.c | 16 ++++++++++++++++
> 1 file changed, 16 insertions(+)
>
> diff --git a/sound/soc/sunxi/sun4i-i2s.c b/sound/soc/sunxi/sun4i-i2s.c
> index 793457394efe..8201334a059b 100644
> --- a/sound/soc/sunxi/sun4i-i2s.c
> +++ b/sound/soc/sunxi/sun4i-i2s.c
> @@ -135,6 +135,7 @@ struct sun4i_i2s;
> * @field_fmt_bclk: regmap field to set clk polarity.
> * @field_fmt_lrclk: regmap field to set frame polarity.
> * @field_fmt_mode: regmap field to set the operational mode.
> + * @field_fmt_sext: regmap field to set the sign extension.
> * @field_txchanmap: location of the tx channel mapping register.
> * @field_rxchanmap: location of the rx channel mapping register.
> * @field_txchansel: location of the tx channel select bit fields.
> @@ -159,6 +160,7 @@ struct sun4i_i2s_quirks {
> struct reg_field field_fmt_bclk;
> struct reg_field field_fmt_lrclk;
> struct reg_field field_fmt_mode;
> + struct reg_field field_fmt_sext;
> struct reg_field field_txchanmap;
> struct reg_field field_rxchanmap;
> struct reg_field field_txchansel;
> @@ -186,6 +188,7 @@ struct sun4i_i2s {
> struct regmap_field *field_fmt_bclk;
> struct regmap_field *field_fmt_lrclk;
> struct regmap_field *field_fmt_mode;
> + struct regmap_field *field_fmt_sext;
> struct regmap_field *field_txchanmap;
> struct regmap_field *field_rxchanmap;
> struct regmap_field *field_txchansel;
> @@ -345,6 +348,9 @@ static int sun4i_i2s_set_clk_rate(struct snd_soc_dai *dai,
> SUN8I_I2S_FMT0_LRCK_PERIOD_MASK,
> SUN8I_I2S_FMT0_LRCK_PERIOD(32));
>
> + /* Set sign extension to pad out LSB with 0 */
> + regmap_field_write(i2s->field_fmt_sext, 0);
> +
> return 0;
> }
>
> @@ -917,6 +923,7 @@ static const struct sun4i_i2s_quirks sun4i_a10_i2s_quirks = {
> .field_fmt_lrclk = REG_FIELD(SUN4I_I2S_FMT0_REG, 7, 7),
> .has_slave_select_bit = true,
> .field_fmt_mode = REG_FIELD(SUN4I_I2S_FMT0_REG, 0, 1),
> + .field_fmt_sext = REG_FIELD(SUN4I_I2S_FMT1_REG, 8, 8),
> .field_txchanmap = REG_FIELD(SUN4I_I2S_TX_CHAN_MAP_REG, 0, 31),
> .field_rxchanmap = REG_FIELD(SUN4I_I2S_RX_CHAN_MAP_REG, 0, 31),
> .field_txchansel = REG_FIELD(SUN4I_I2S_TX_CHAN_SEL_REG, 0, 2),
> @@ -936,6 +943,7 @@ static const struct sun4i_i2s_quirks sun6i_a31_i2s_quirks = {
> .field_fmt_lrclk = REG_FIELD(SUN4I_I2S_FMT0_REG, 7, 7),
> .has_slave_select_bit = true,
> .field_fmt_mode = REG_FIELD(SUN4I_I2S_FMT0_REG, 0, 1),
> + .field_fmt_sext = REG_FIELD(SUN4I_I2S_FMT1_REG, 8, 8),
> .field_txchanmap = REG_FIELD(SUN4I_I2S_TX_CHAN_MAP_REG, 0, 31),
> .field_rxchanmap = REG_FIELD(SUN4I_I2S_RX_CHAN_MAP_REG, 0, 31),
> .field_txchansel = REG_FIELD(SUN4I_I2S_TX_CHAN_SEL_REG, 0, 2),
> @@ -979,6 +987,7 @@ static const struct sun4i_i2s_quirks sun8i_h3_i2s_quirks = {
> .field_fmt_bclk = REG_FIELD(SUN4I_I2S_FMT0_REG, 7, 7),
> .field_fmt_lrclk = REG_FIELD(SUN4I_I2S_FMT0_REG, 19, 19),
> .field_fmt_mode = REG_FIELD(SUN4I_I2S_CTRL_REG, 4, 5),
> + .field_fmt_sext = REG_FIELD(SUN4I_I2S_FMT1_REG, 4, 5),
> .field_txchanmap = REG_FIELD(SUN8I_I2S_TX_CHAN_MAP_REG, 0, 31),
> .field_rxchanmap = REG_FIELD(SUN8I_I2S_RX_CHAN_MAP_REG, 0, 31),
> .field_txchansel = REG_FIELD(SUN8I_I2S_TX_CHAN_SEL_REG, 0, 2),
> @@ -998,6 +1007,7 @@ static const struct sun4i_i2s_quirks sun50i_a64_codec_i2s_quirks = {
> .field_fmt_bclk = REG_FIELD(SUN4I_I2S_FMT0_REG, 6, 6),
> .field_fmt_lrclk = REG_FIELD(SUN4I_I2S_FMT0_REG, 7, 7),
> .field_fmt_mode = REG_FIELD(SUN4I_I2S_FMT0_REG, 0, 1),
> + .field_fmt_sext = REG_FIELD(SUN4I_I2S_FMT1_REG, 8, 8),
> .field_txchanmap = REG_FIELD(SUN4I_I2S_TX_CHAN_MAP_REG, 0, 31),
> .field_rxchanmap = REG_FIELD(SUN4I_I2S_RX_CHAN_MAP_REG, 0, 31),
> .field_txchansel = REG_FIELD(SUN4I_I2S_TX_CHAN_SEL_REG, 0, 2),
You're missing the A83t here
Maxime
--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
[-- Attachment #2: Type: text/plain, Size: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox