* Re: [PATCH v3 06/11] kselftest: arm64: fake_sigreturn_bad_magic
From: Cristian Marussi @ 2019-09-04 10:37 UTC (permalink / raw)
To: Dave Martin; +Cc: andreyknvl, shuah, linux-kselftest, linux-arm-kernel
In-Reply-To: <20190904100516.GN27757@arm.com>
On 04/09/2019 11:05, Dave Martin wrote:
> On Fri, Aug 30, 2019 at 03:29:29PM +0100, Cristian Marussi wrote:
>> Hi
>>
>> On 13/08/2019 17:25, Dave Martin wrote:
>>> On Fri, Aug 02, 2019 at 06:02:55PM +0100, Cristian Marussi wrote:
>
> [...]
>
>>>> diff --git a/tools/testing/selftests/arm64/signal/testcases/fake_sigreturn_bad_magic.c b/tools/testing/selftests/arm64/signal/testcases/fake_sigreturn_bad_magic.c
>
> [...]
>
>>>> +static int fake_sigreturn_bad_magic_run(struct tdescr *td,
>>>> + siginfo_t *si, ucontext_t *uc)
>>>> +{
>>>> + size_t resv_sz, offset;
>>>> + struct _aarch64_ctx *shead = GET_SF_RESV_HEAD(sf), *head;
>>>> +
>>>> + /* just to fill the ucontext_t with something real */
>>>> + if (!get_current_context(td, &sf.uc))
>>>> + return 1;
>>>> +
>>>> + resv_sz = GET_SF_RESV_SIZE(sf);
>>>> + /*
>>>> + * find the terminator, preserving existing headers
>>>> + * and verify amount of spare room in __reserved area.
>>>> + */
>>>> + head = get_terminator(shead, resv_sz, &offset);
>>>> + /*
>>>> + * try stripping extra_context header when low on space:
>>>> + * we need at least 2*HDR_SZ space ... one for the KSFT_BAD_MAGIC
>>>> + * and the other for the usual terminator.
>>>> + */
>>>> + if (head && resv_sz - offset < HDR_SZ * 2) {
>>>
>>> Can we factor out this logic for finding space in the signal frame?
>>>
>>> We do pretty much the same thing in all the fake_sigreturn tests...
>>
>> Ok
>>>
>>>> + fprintf(stderr, "Low on space:%zd. Discarding extra_context.\n",
>>>> + resv_sz - offset);
>>>> + head = get_header(shead, EXTRA_MAGIC, resv_sz, &offset);
>>>> + }
>>>> + /* just give up and timeout if still not enough space */
>>>
>>> Do we actually time out? I don't see where we actually wait, so doesn't
>>> test_run() just fail immediately?
>>>
>>> The same applies to all the other fake_sigreturn tests too.
>>>
>> Right. It is probably a leftover.
>>
>> SIGALRM is used as an extreme measure to kill tests gone bad, but this
>> can happen only once the fake sigframe has been effectively placed on the stack
>> and sigreturned.
>
> OK, so this gets reported as a test failure because with no SIGSEGV,
> nothing ever sets td->pass?
Yes exactly. End result is based on value on td->pass, in case of abrupt
termination or timeout nobody sets td->pass ever.
>
> This is probably OK for now, though I wonder whether this should be
> reported as a skipped test instead.
>
> In case of doubt, reporting a failure is preferable anyway, since that
> will encourage people actually to investigate what went wrong.
>
As of now I never skip a test in fact...also tests for unsupported features
are built and run expecting a SIGILL, and reported as PASS in that case.
Cristian
> [...]
>
> Cheers
> ---Dave
>
_______________________________________________
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] drm: bridge/dw_hdmi: add audio sample channel status setting
From: Russell King - ARM Linux admin @ 2019-09-04 10:31 UTC (permalink / raw)
To: Cheng-yi Chiang
Cc: moderated list:SOUND - SOC LAYER / DYNAMIC AUDIO POWER MANAGEM...,
kuninori.morimoto.gx, Neil Armstrong, David Airlie, dri-devel,
linux-kernel, Andrzej Hajda, Laurent Pinchart, sam, cain.cai,
Xing Zheng, linux-rockchip, Dylan Reid, tzungbi, Jonas Karlman,
Jeffy Chen, 蔡枫, linux-arm-kernel, Jernej Skrabec,
Doug Anderson, Daniel Vetter, Enric Balletbo i Serra, kuankuan.y
In-Reply-To: <CAFv8NwLFd4EZY5tcYeaagRiHWPx_QWDrtKs3WPT4ouJBMvM-LQ@mail.gmail.com>
On Wed, Sep 04, 2019 at 05:45:20PM +0800, Cheng-yi Chiang wrote:
> On Wed, Sep 4, 2019 at 5:28 PM Neil Armstrong <narmstrong@baylibre.com> wrote:
> >
> > Hi,
> >
> > On 04/09/2019 11:09, Cheng-yi Chiang wrote:
> > > Hi,
> > >
> > > On Tue, Sep 3, 2019 at 5:53 PM Neil Armstrong <narmstrong@baylibre.com> wrote:
> > >>
> > >> Hi,
> > >>
> > >> On 03/09/2019 07:51, Cheng-Yi Chiang wrote:
> > >>> From: Yakir Yang <ykk@rock-chips.com>
> > >>>
> > >>> When transmitting IEC60985 linear PCM audio, we configure the
> > >>> Audio Sample Channel Status information of all the channel
> > >>> status bits in the IEC60958 frame.
> > >>> Refer to 60958-3 page 10 for frequency, original frequency, and
> > >>> wordlength setting.
> > >>>
> > >>> This fix the issue that audio does not come out on some monitors
> > >>> (e.g. LG 22CV241)
> > >>>
> > >>> Signed-off-by: Yakir Yang <ykk@rock-chips.com>
> > >>> Signed-off-by: Cheng-Yi Chiang <cychiang@chromium.org>
> > >>> ---
> > >>> drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 59 +++++++++++++++++++++++
> > >>> drivers/gpu/drm/bridge/synopsys/dw-hdmi.h | 20 ++++++++
> > >>> 2 files changed, 79 insertions(+)
> > >>>
> > >>> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> > >>> index bd65d0479683..34d46e25d610 100644
> > >>> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> > >>> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> > >>> @@ -582,6 +582,63 @@ static unsigned int hdmi_compute_n(unsigned int freq, unsigned long pixel_clk)
> > >>> return n;
> > >>> }
> > >>>
> > >>> +static void hdmi_set_schnl(struct dw_hdmi *hdmi)
> > >>> +{
> > >>> + u8 aud_schnl_samplerate;
> > >>> + u8 aud_schnl_8;
> > >>> +
> > >>> + /* These registers are on RK3288 using version 2.0a. */
> > >>> + if (hdmi->version != 0x200a)
> > >>> + return;
> > >>
> > >> Are these limited to the 2.0a version *in* RK3288, or 2.0a version on all
> > >> SoCs ?
> > >>
> > >
> > > In the original patch by Yakir,
> > >
> > > https://lore.kernel.org/patchwork/patch/539653/ (sorry, I should
> > > have added this link in the "after the cut" note)
> > >
> > > The fix is limited to version 2.0.
> > > Since I am only testing on RK3288 with 2.0a, I change the check to 2.0a only.
> > > I can not test 2.0a version on other SoCs.
> > > The databook I have at hand is 2.0a (not specific to RK3288) so I
> > > think all 2.0a should have this register.
> > >
> > > As for other version like version 1.3 on iMX6, there is no such
> > > register, as stated by Russell
> > >
> > > http://lkml.iu.edu/hypermail/linux/kernel/1501.3/06268.html.
> >
> > Russell assumes the registers are not present on the iMX6 IP not having
> > the I2S registers, but they are present on the IPs configured with I2S
> > at any versions.
>
> I see. Thanks for the check.
>
> >
> > My databook version (1.40.a) specifies :
> >
> > fc_audschnls0 to fc_audschnls8
> >
> > ```
> > When transmitting IEC60958 linear PCM audio, this registers allow to configure the channel status
> > information of all the channel status bits in the IEC60958 frame. For the moment this configuration is only
> > used when the I2S audio interface, General Purpose Audio (GPA), or AHB audio DMA (AHBAUDDMA)
> > interface is active (for S/PDIF interface this information comes from the stream).
> > ```
> >
> > But the databook Revision History shows these registers were present since version 1.10a.
> >
> > I propose you remove the version check, but you only setup these registers when I2S is enabled:
> > (hdmi_readb(hdmi, HDMI_CONFIG0_ID) & HDMI_CONFIG0_I2S) until a AHBAUDDMA user needs this,
> > with a small comment explaining the situation.
>
> I see. Sound like a good plan.
> In sum, I will add
> set_channel_status()
> in dw_hdmi.c
> And in the beginning of this function,
> check it is using I2S
> (hdmi_readb(hdmi, HDMI_CONFIG0_ID) & HDMI_CONFIG0_I2S)
> with a comment explaining the situation.
>
> And let dw-hdmi-audio-i2s dw_hdmi_i2s_hw_params() to call this
> function after dw_hdmi_set_sample_rate, with word length (or
> sample_bit) passed it as argument.
If you're going to do it this way, there's little point having the
check. The dw-hdmi-audio-i2s device will only be created if that
ID bit is already set. So, provided set_channel_status() (which
should probably be dw_hdmi_set_channel_status()) is only called from
the I2S driver, then everything should be fine without such a check.
However, it is worth noting in the docbook comments above the function
that it is only for I2S setups.
> I have not tested setting this register here as in the original patch
> it was set in hdmi_set_clk_regenerator.
> I will test it and update in my v2.
>
> Thanks again to everyone for the prompt reply and help.
>
> >
> > Neil
> >
> > >
> > > So at least we should check the version.
> > > Maybe we can set the criteria as version 2.0 or above to make it a safe patch ?
> > > If there is the same need on other SoC with version < 2.0, it can be
> > > added later.
> > > Presumably, there will be databook of that version to help confirming
> > > this setting.
> > >
> > > Thanks!
> > >>> +
> > >>> + switch (hdmi->sample_rate) {
> > >>> + case 32000:
> > >>> + aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_32K;
> > >>> + break;
> > >>> + case 44100:
> > >>> + aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_44K1;
> > >>> + break;
> > >>> + case 48000:
> > >>> + aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_48K;
> > >>> + break;
> > >>> + case 88200:
> > >>> + aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_88K2;
> > >>> + break;
> > >>> + case 96000:
> > >>> + aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_96K;
> > >>> + break;
> > >>> + case 176400:
> > >>> + aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_176K4;
> > >>> + break;
> > >>> + case 192000:
> > >>> + aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_192K;
> > >>> + break;
> > >>> + case 768000:
> > >>> + aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_768K;
> > >>> + break;
> > >>> + default:
> > >>> + dev_warn(hdmi->dev, "Unsupported audio sample rate (%u)\n",
> > >>> + hdmi->sample_rate);
> > >>> + return;
> > >>> + }
> > >>> +
> > >>> + /* set channel status register */
> > >>> + hdmi_modb(hdmi, aud_schnl_samplerate, HDMI_FC_AUDSCHNLS7_SMPRATE_MASK,
> > >>> + HDMI_FC_AUDSCHNLS7);
> > >>> +
> > >>> + /*
> > >>> + * Set original frequency to be the same as frequency.
> > >>> + * Use one-complement value as stated in IEC60958-3 page 13.
> > >>> + */
> > >>> + aud_schnl_8 = (~aud_schnl_samplerate) <<
> > >>> + HDMI_FC_AUDSCHNLS8_ORIGSAMPFREQ_OFFSET;
> > >>> +
> > >>> + /* This means word length is 16 bit. Refer to IEC60958-3 page 12. */
> > >>> + aud_schnl_8 |= 2 << HDMI_FC_AUDSCHNLS8_WORDLEGNTH_OFFSET;
> > >>> +
> > >>> + hdmi_writeb(hdmi, aud_schnl_8, HDMI_FC_AUDSCHNLS8);
> > >>> +}
> > >>> +
> > >>> static void hdmi_set_clk_regenerator(struct dw_hdmi *hdmi,
> > >>> unsigned long pixel_clk, unsigned int sample_rate)
> > >>> {
> > >>> @@ -620,6 +677,8 @@ static void hdmi_set_clk_regenerator(struct dw_hdmi *hdmi,
> > >>> hdmi->audio_cts = cts;
> > >>> hdmi_set_cts_n(hdmi, cts, hdmi->audio_enable ? n : 0);
> > >>> spin_unlock_irq(&hdmi->audio_lock);
> > >>> +
> > >>> + hdmi_set_schnl(hdmi);
> > >>> }
> > >>>
> > >>> static void hdmi_init_clk_regenerator(struct dw_hdmi *hdmi)
> > >>> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h
> > >>> index 6988f12d89d9..619ebc1c8354 100644
> > >>> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h
> > >>> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h
> > >>> @@ -158,6 +158,17 @@
> > >>> #define HDMI_FC_SPDDEVICEINF 0x1062
> > >>> #define HDMI_FC_AUDSCONF 0x1063
> > >>> #define HDMI_FC_AUDSSTAT 0x1064
> > >>> +#define HDMI_FC_AUDSV 0x1065
> > >>> +#define HDMI_FC_AUDSU 0x1066
> > >>> +#define HDMI_FC_AUDSCHNLS0 0x1067
> > >>> +#define HDMI_FC_AUDSCHNLS1 0x1068
> > >>> +#define HDMI_FC_AUDSCHNLS2 0x1069
> > >>> +#define HDMI_FC_AUDSCHNLS3 0x106a
> > >>> +#define HDMI_FC_AUDSCHNLS4 0x106b
> > >>> +#define HDMI_FC_AUDSCHNLS5 0x106c
> > >>> +#define HDMI_FC_AUDSCHNLS6 0x106d
> > >>> +#define HDMI_FC_AUDSCHNLS7 0x106e
> > >>> +#define HDMI_FC_AUDSCHNLS8 0x106f
> > >>> #define HDMI_FC_DATACH0FILL 0x1070
> > >>> #define HDMI_FC_DATACH1FILL 0x1071
> > >>> #define HDMI_FC_DATACH2FILL 0x1072
> > >>> @@ -706,6 +717,15 @@ enum {
> > >>> /* HDMI_FC_AUDSCHNLS7 field values */
> > >>> HDMI_FC_AUDSCHNLS7_ACCURACY_OFFSET = 4,
> > >>> HDMI_FC_AUDSCHNLS7_ACCURACY_MASK = 0x30,
> > >>> + HDMI_FC_AUDSCHNLS7_SMPRATE_MASK = 0x0f,
> > >>> + HDMI_FC_AUDSCHNLS7_SMPRATE_192K = 0xe,
> > >>> + HDMI_FC_AUDSCHNLS7_SMPRATE_176K4 = 0xc,
> > >>> + HDMI_FC_AUDSCHNLS7_SMPRATE_96K = 0xa,
> > >>> + HDMI_FC_AUDSCHNLS7_SMPRATE_768K = 0x9,
> > >>> + HDMI_FC_AUDSCHNLS7_SMPRATE_88K2 = 0x8,
> > >>> + HDMI_FC_AUDSCHNLS7_SMPRATE_32K = 0x3,
> > >>> + HDMI_FC_AUDSCHNLS7_SMPRATE_48K = 0x2,
> > >>> + HDMI_FC_AUDSCHNLS7_SMPRATE_44K1 = 0x0,
> > >>>
> > >>> /* HDMI_FC_AUDSCHNLS8 field values */
> > >>> HDMI_FC_AUDSCHNLS8_ORIGSAMPFREQ_MASK = 0xf0,
> > >>>
> > >>
> >
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up
_______________________________________________
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] crypto: arm64: Use PTR_ERR_OR_ZERO rather than its implementation.
From: Will Deacon @ 2019-09-04 10:25 UTC (permalink / raw)
To: zhong jiang
Cc: herbert, catalin.marinas, linux-kernel, linux-crypto, davem,
linux-arm-kernel
In-Reply-To: <1567493656-19916-1-git-send-email-zhongjiang@huawei.com>
On Tue, Sep 03, 2019 at 02:54:16PM +0800, zhong jiang wrote:
> PTR_ERR_OR_ZERO contains if(IS_ERR(...)) + PTR_ERR. It is better to
> use it directly. hence just replace it.
>
> Signed-off-by: zhong jiang <zhongjiang@huawei.com>
> ---
> arch/arm64/crypto/aes-glue.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/arch/arm64/crypto/aes-glue.c b/arch/arm64/crypto/aes-glue.c
> index ca0c84d..2a2e0a3 100644
> --- a/arch/arm64/crypto/aes-glue.c
> +++ b/arch/arm64/crypto/aes-glue.c
> @@ -409,10 +409,8 @@ static int essiv_cbc_init_tfm(struct crypto_skcipher *tfm)
> struct crypto_aes_essiv_cbc_ctx *ctx = crypto_skcipher_ctx(tfm);
>
> ctx->hash = crypto_alloc_shash("sha256", 0, 0);
> - if (IS_ERR(ctx->hash))
> - return PTR_ERR(ctx->hash);
>
> - return 0;
> + return PTR_ERR_OR_ZERO(ctx->hash);
> }
>
> static void essiv_cbc_exit_tfm(struct crypto_skcipher *tfm)
Acked-by: Will Deacon <will@kernel.org>
Assuming this will go via Herbert.
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 03/10] arm64: atomics: avoid out-of-line ll/sc atomics
From: Will Deacon @ 2019-09-04 10:20 UTC (permalink / raw)
To: Nick Desaulniers
Cc: Mark Rutland, Peter Zijlstra, Catalin Marinas, Ard.Biesheuvel,
Andrew Murray, Itaru Kitayama, Nathan Chancellor, Robin Murphy,
Linux ARM
In-Reply-To: <CAKwvOd=QDeKeOnn8Hw-p5mdhT3JPfwwLVnf7=xZ1wyc=v54AEQ@mail.gmail.com>
On Tue, Sep 03, 2019 at 03:53:34PM -0700, Nick Desaulniers wrote:
> > On Wed, Sep 4, 2019 at 7:35 AM Nick Desaulniers <ndesaulniers@google.com> wrote:
> >> Thanks for the report. We squashed many bugs related to asm goto, but
> >> it's difficult to say with 100% certainty that the current
> >> implementation is bug free. Simply throwing more exotic forms of
> >> control flow at it often shake out corner cases. Thank you very much
> >> for the reduced test case, and I'll look into getting a fix ready
> >> hopefully in time to make the clang-9 release train.
>
> On Tue, Sep 3, 2019 at 3:49 PM Itaru Kitayama <itaru.kitayama@gmail.com> wrote:
> >
> > Do you mean that you'd do a backport to Clang 9 as well as the trunk contribution?
>
> Yes; I think the window for merging things in the 9.0 release is still
> open, though they are late in the -rc cycle. If not 9.1 bugfix is
> possible.
Thanks, Nick. If you run out of time to get it fixed then it would probably
be best to disable 'asm goto' support by default for arm64, since at least
you'll have a functional kernel in that case.
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 v3 06/11] kselftest: arm64: fake_sigreturn_bad_magic
From: Dave Martin @ 2019-09-04 10:05 UTC (permalink / raw)
To: Cristian Marussi; +Cc: andreyknvl, shuah, linux-kselftest, linux-arm-kernel
In-Reply-To: <245a3d64-2790-1768-94ab-d1ea56aa3d3d@arm.com>
On Fri, Aug 30, 2019 at 03:29:29PM +0100, Cristian Marussi wrote:
> Hi
>
> On 13/08/2019 17:25, Dave Martin wrote:
> > On Fri, Aug 02, 2019 at 06:02:55PM +0100, Cristian Marussi wrote:
[...]
> >> diff --git a/tools/testing/selftests/arm64/signal/testcases/fake_sigreturn_bad_magic.c b/tools/testing/selftests/arm64/signal/testcases/fake_sigreturn_bad_magic.c
[...]
> >> +static int fake_sigreturn_bad_magic_run(struct tdescr *td,
> >> + siginfo_t *si, ucontext_t *uc)
> >> +{
> >> + size_t resv_sz, offset;
> >> + struct _aarch64_ctx *shead = GET_SF_RESV_HEAD(sf), *head;
> >> +
> >> + /* just to fill the ucontext_t with something real */
> >> + if (!get_current_context(td, &sf.uc))
> >> + return 1;
> >> +
> >> + resv_sz = GET_SF_RESV_SIZE(sf);
> >> + /*
> >> + * find the terminator, preserving existing headers
> >> + * and verify amount of spare room in __reserved area.
> >> + */
> >> + head = get_terminator(shead, resv_sz, &offset);
> >> + /*
> >> + * try stripping extra_context header when low on space:
> >> + * we need at least 2*HDR_SZ space ... one for the KSFT_BAD_MAGIC
> >> + * and the other for the usual terminator.
> >> + */
> >> + if (head && resv_sz - offset < HDR_SZ * 2) {
> >
> > Can we factor out this logic for finding space in the signal frame?
> >
> > We do pretty much the same thing in all the fake_sigreturn tests...
>
> Ok
> >
> >> + fprintf(stderr, "Low on space:%zd. Discarding extra_context.\n",
> >> + resv_sz - offset);
> >> + head = get_header(shead, EXTRA_MAGIC, resv_sz, &offset);
> >> + }
> >> + /* just give up and timeout if still not enough space */
> >
> > Do we actually time out? I don't see where we actually wait, so doesn't
> > test_run() just fail immediately?
> >
> > The same applies to all the other fake_sigreturn tests too.
> >
> Right. It is probably a leftover.
>
> SIGALRM is used as an extreme measure to kill tests gone bad, but this
> can happen only once the fake sigframe has been effectively placed on the stack
> and sigreturned.
OK, so this gets reported as a test failure because with no SIGSEGV,
nothing ever sets td->pass?
This is probably OK for now, though I wonder whether this should be
reported as a skipped test instead.
In case of doubt, reporting a failure is preferable anyway, since that
will encourage people actually to investigate what went wrong.
[...]
Cheers
---Dave
_______________________________________________
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/5] dmaengine: ti: edma: Multicore usage related fixes
From: Vinod Koul @ 2019-09-04 9:49 UTC (permalink / raw)
To: Peter Ujfalusi
Cc: devicetree, linux-kernel, robh+dt, dmaengine, dan.j.williams,
linux-omap, linux-arm-kernel
In-Reply-To: <20190823125618.8133-1-peter.ujfalusi@ti.com>
On 23-08-19, 15:56, Peter Ujfalusi wrote:
> Hi,
>
> When other cores want to use EDMA for their use cases Linux was not playing
> nicely.
> By design EDMA is supporting shared use with shadow regions. Linux is using
> region0, others can be used by other cores.
>
> In order to not break multicore shared usage of EDMA:
> - do not reset paRAM slots which is not allocated for Linux (reserved paRAM
> slots)
> - Only reset region0 access registers, do not touch other regions
> - Add option for reserved channels which should not be used by Linux in a similar
> fashion as we already have for reserved paRAM slots.
Applied 1 to 3, thanks
--
~Vinod
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH] clk: imx: pll14xx: Fix quick switch of S/K parameter
From: Leonard Crestez @ 2019-09-04 9:49 UTC (permalink / raw)
To: Stephen Boyd, Shawn Guo, Peng Fan
Cc: Dong Aisheng, Jacky Bai, Michael Turquette, linux-clk, linux-imx,
Viorel Suman, Fabio Estevam, Daniel Baluta, kernel,
linux-arm-kernel, Abel Vesa
The PLL14xx on imx8m can change the S and K parameter without requiring
a reset and relock of the whole PLL.
Fix clk_pll144xx_mp_change register reading and use it for pll1443 as
well since no reset+relock is required on K changes either.
Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
---
drivers/clk/imx/clk-pll14xx.c | 40 +++++++----------------------------
1 file changed, 8 insertions(+), 32 deletions(-)
The PLLs are currently table-based and none of the entries differ only
in S/K so further work would be required to make use of this. The
prospective user is audio doing tiny freq adjustments and there is no
standard API for that.
Lacking users is not a good reason to carry broken code around.
diff --git a/drivers/clk/imx/clk-pll14xx.c b/drivers/clk/imx/clk-pll14xx.c
index b7213023b238..25342297e5a6 100644
--- a/drivers/clk/imx/clk-pll14xx.c
+++ b/drivers/clk/imx/clk-pll14xx.c
@@ -110,47 +110,21 @@ static unsigned long clk_pll1443x_recalc_rate(struct clk_hw *hw,
do_div(fvco, pdiv << sdiv);
return fvco;
}
-static inline bool clk_pll1416x_mp_change(const struct imx_pll14xx_rate_table *rate,
+static inline bool clk_pll14xx_mp_change(const struct imx_pll14xx_rate_table *rate,
u32 pll_div)
{
u32 old_mdiv, old_pdiv;
- old_mdiv = (pll_div >> MDIV_SHIFT) & MDIV_MASK;
- old_pdiv = (pll_div >> PDIV_SHIFT) & PDIV_MASK;
+ old_mdiv = (pll_div & MDIV_MASK) >> MDIV_SHIFT;
+ old_pdiv = (pll_div & PDIV_MASK) >> PDIV_SHIFT;
return rate->mdiv != old_mdiv || rate->pdiv != old_pdiv;
}
-static inline bool clk_pll1443x_mpk_change(const struct imx_pll14xx_rate_table *rate,
- u32 pll_div_ctl0, u32 pll_div_ctl1)
-{
- u32 old_mdiv, old_pdiv, old_kdiv;
-
- old_mdiv = (pll_div_ctl0 >> MDIV_SHIFT) & MDIV_MASK;
- old_pdiv = (pll_div_ctl0 >> PDIV_SHIFT) & PDIV_MASK;
- old_kdiv = (pll_div_ctl1 >> KDIV_SHIFT) & KDIV_MASK;
-
- return rate->mdiv != old_mdiv || rate->pdiv != old_pdiv ||
- rate->kdiv != old_kdiv;
-}
-
-static inline bool clk_pll1443x_mp_change(const struct imx_pll14xx_rate_table *rate,
- u32 pll_div_ctl0, u32 pll_div_ctl1)
-{
- u32 old_mdiv, old_pdiv, old_kdiv;
-
- old_mdiv = (pll_div_ctl0 >> MDIV_SHIFT) & MDIV_MASK;
- old_pdiv = (pll_div_ctl0 >> PDIV_SHIFT) & PDIV_MASK;
- old_kdiv = (pll_div_ctl1 >> KDIV_SHIFT) & KDIV_MASK;
-
- return rate->mdiv != old_mdiv || rate->pdiv != old_pdiv ||
- rate->kdiv != old_kdiv;
-}
-
static int clk_pll14xx_wait_lock(struct clk_pll14xx *pll)
{
u32 val;
return readl_poll_timeout(pll->base, val, val & LOCK_TIMEOUT_US, 0,
@@ -172,11 +146,11 @@ static int clk_pll1416x_set_rate(struct clk_hw *hw, unsigned long drate,
return -EINVAL;
}
tmp = readl_relaxed(pll->base + 4);
- if (!clk_pll1416x_mp_change(rate, tmp)) {
+ if (!clk_pll14xx_mp_change(rate, tmp)) {
tmp &= ~(SDIV_MASK) << SDIV_SHIFT;
tmp |= rate->sdiv << SDIV_SHIFT;
writel_relaxed(tmp, pll->base + 4);
return 0;
@@ -233,17 +207,19 @@ static int clk_pll1443x_set_rate(struct clk_hw *hw, unsigned long drate,
drate, clk_hw_get_name(hw));
return -EINVAL;
}
tmp = readl_relaxed(pll->base + 4);
- div_val = readl_relaxed(pll->base + 8);
- if (!clk_pll1443x_mpk_change(rate, tmp, div_val)) {
+ if (!clk_pll14xx_mp_change(rate, tmp)) {
tmp &= ~(SDIV_MASK) << SDIV_SHIFT;
tmp |= rate->sdiv << SDIV_SHIFT;
writel_relaxed(tmp, pll->base + 4);
+ tmp = rate->kdiv << KDIV_SHIFT;
+ writel_relaxed(tmp, pll->base + 8);
+
return 0;
}
/* Enable RST */
tmp = readl_relaxed(pll->base);
--
2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH -next] usb: gadget: pxa27x_udc: use devm_platform_ioremap_resource() to simplify code
From: YueHaibing @ 2019-09-04 9:45 UTC (permalink / raw)
To: daniel, haojian.zhuang, robert.jarzmik, balbi, gregkh
Cc: linux-usb, YueHaibing, linux-kernel, linux-arm-kernel
Use devm_platform_ioremap_resource() to simplify the code a bit.
This is detected by coccinelle.
Reported-by: Hulk Robot <hulkci@huawei.com>
Signed-off-by: YueHaibing <yuehaibing@huawei.com>
---
drivers/usb/gadget/udc/pxa27x_udc.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/usb/gadget/udc/pxa27x_udc.c b/drivers/usb/gadget/udc/pxa27x_udc.c
index 0142332..5f107f2 100644
--- a/drivers/usb/gadget/udc/pxa27x_udc.c
+++ b/drivers/usb/gadget/udc/pxa27x_udc.c
@@ -2356,7 +2356,6 @@ MODULE_DEVICE_TABLE(of, udc_pxa_dt_ids);
*/
static int pxa_udc_probe(struct platform_device *pdev)
{
- struct resource *regs;
struct pxa_udc *udc = &memory;
int retval = 0, gpio;
struct pxa2xx_udc_mach_info *mach = dev_get_platdata(&pdev->dev);
@@ -2378,8 +2377,7 @@ static int pxa_udc_probe(struct platform_device *pdev)
udc->gpiod = devm_gpiod_get(&pdev->dev, NULL, GPIOD_ASIS);
}
- regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- udc->regs = devm_ioremap_resource(&pdev->dev, regs);
+ udc->regs = devm_platform_ioremap_resource(pdev, 0);
if (IS_ERR(udc->regs))
return PTR_ERR(udc->regs);
udc->irq = platform_get_irq(pdev, 0);
--
2.7.4
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* Re: [PATCH] drm: bridge/dw_hdmi: add audio sample channel status setting
From: Cheng-yi Chiang @ 2019-09-04 9:45 UTC (permalink / raw)
To: Neil Armstrong
Cc: moderated list:SOUND - SOC LAYER / DYNAMIC AUDIO POWER MANAGEM...,
Doug Anderson, kuninori.morimoto.gx, David Airlie, dri-devel,
linux-kernel, Andrzej Hajda, Laurent Pinchart, sam, Xing Zheng,
linux-rockchip, Dylan Reid, tzungbi, Jonas Karlman, Jeffy Chen,
蔡枫, linux-arm-kernel, Jernej Skrabec, cain.cai,
Daniel Vetter, Enric Balletbo i Serra, kuankuan.y
In-Reply-To: <1a167659-2eb1-d9be-c1ae-4958ac3f7929@baylibre.com>
On Wed, Sep 4, 2019 at 5:28 PM Neil Armstrong <narmstrong@baylibre.com> wrote:
>
> Hi,
>
> On 04/09/2019 11:09, Cheng-yi Chiang wrote:
> > Hi,
> >
> > On Tue, Sep 3, 2019 at 5:53 PM Neil Armstrong <narmstrong@baylibre.com> wrote:
> >>
> >> Hi,
> >>
> >> On 03/09/2019 07:51, Cheng-Yi Chiang wrote:
> >>> From: Yakir Yang <ykk@rock-chips.com>
> >>>
> >>> When transmitting IEC60985 linear PCM audio, we configure the
> >>> Audio Sample Channel Status information of all the channel
> >>> status bits in the IEC60958 frame.
> >>> Refer to 60958-3 page 10 for frequency, original frequency, and
> >>> wordlength setting.
> >>>
> >>> This fix the issue that audio does not come out on some monitors
> >>> (e.g. LG 22CV241)
> >>>
> >>> Signed-off-by: Yakir Yang <ykk@rock-chips.com>
> >>> Signed-off-by: Cheng-Yi Chiang <cychiang@chromium.org>
> >>> ---
> >>> drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 59 +++++++++++++++++++++++
> >>> drivers/gpu/drm/bridge/synopsys/dw-hdmi.h | 20 ++++++++
> >>> 2 files changed, 79 insertions(+)
> >>>
> >>> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> >>> index bd65d0479683..34d46e25d610 100644
> >>> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> >>> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> >>> @@ -582,6 +582,63 @@ static unsigned int hdmi_compute_n(unsigned int freq, unsigned long pixel_clk)
> >>> return n;
> >>> }
> >>>
> >>> +static void hdmi_set_schnl(struct dw_hdmi *hdmi)
> >>> +{
> >>> + u8 aud_schnl_samplerate;
> >>> + u8 aud_schnl_8;
> >>> +
> >>> + /* These registers are on RK3288 using version 2.0a. */
> >>> + if (hdmi->version != 0x200a)
> >>> + return;
> >>
> >> Are these limited to the 2.0a version *in* RK3288, or 2.0a version on all
> >> SoCs ?
> >>
> >
> > In the original patch by Yakir,
> >
> > https://lore.kernel.org/patchwork/patch/539653/ (sorry, I should
> > have added this link in the "after the cut" note)
> >
> > The fix is limited to version 2.0.
> > Since I am only testing on RK3288 with 2.0a, I change the check to 2.0a only.
> > I can not test 2.0a version on other SoCs.
> > The databook I have at hand is 2.0a (not specific to RK3288) so I
> > think all 2.0a should have this register.
> >
> > As for other version like version 1.3 on iMX6, there is no such
> > register, as stated by Russell
> >
> > http://lkml.iu.edu/hypermail/linux/kernel/1501.3/06268.html.
>
> Russell assumes the registers are not present on the iMX6 IP not having
> the I2S registers, but they are present on the IPs configured with I2S
> at any versions.
I see. Thanks for the check.
>
> My databook version (1.40.a) specifies :
>
> fc_audschnls0 to fc_audschnls8
>
> ```
> When transmitting IEC60958 linear PCM audio, this registers allow to configure the channel status
> information of all the channel status bits in the IEC60958 frame. For the moment this configuration is only
> used when the I2S audio interface, General Purpose Audio (GPA), or AHB audio DMA (AHBAUDDMA)
> interface is active (for S/PDIF interface this information comes from the stream).
> ```
>
> But the databook Revision History shows these registers were present since version 1.10a.
>
> I propose you remove the version check, but you only setup these registers when I2S is enabled:
> (hdmi_readb(hdmi, HDMI_CONFIG0_ID) & HDMI_CONFIG0_I2S) until a AHBAUDDMA user needs this,
> with a small comment explaining the situation.
I see. Sound like a good plan.
In sum, I will add
set_channel_status()
in dw_hdmi.c
And in the beginning of this function,
check it is using I2S
(hdmi_readb(hdmi, HDMI_CONFIG0_ID) & HDMI_CONFIG0_I2S)
with a comment explaining the situation.
And let dw-hdmi-audio-i2s dw_hdmi_i2s_hw_params() to call this
function after dw_hdmi_set_sample_rate, with word length (or
sample_bit) passed it as argument.
I have not tested setting this register here as in the original patch
it was set in hdmi_set_clk_regenerator.
I will test it and update in my v2.
Thanks again to everyone for the prompt reply and help.
>
> Neil
>
> >
> > So at least we should check the version.
> > Maybe we can set the criteria as version 2.0 or above to make it a safe patch ?
> > If there is the same need on other SoC with version < 2.0, it can be
> > added later.
> > Presumably, there will be databook of that version to help confirming
> > this setting.
> >
> > Thanks!
> >>> +
> >>> + switch (hdmi->sample_rate) {
> >>> + case 32000:
> >>> + aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_32K;
> >>> + break;
> >>> + case 44100:
> >>> + aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_44K1;
> >>> + break;
> >>> + case 48000:
> >>> + aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_48K;
> >>> + break;
> >>> + case 88200:
> >>> + aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_88K2;
> >>> + break;
> >>> + case 96000:
> >>> + aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_96K;
> >>> + break;
> >>> + case 176400:
> >>> + aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_176K4;
> >>> + break;
> >>> + case 192000:
> >>> + aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_192K;
> >>> + break;
> >>> + case 768000:
> >>> + aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_768K;
> >>> + break;
> >>> + default:
> >>> + dev_warn(hdmi->dev, "Unsupported audio sample rate (%u)\n",
> >>> + hdmi->sample_rate);
> >>> + return;
> >>> + }
> >>> +
> >>> + /* set channel status register */
> >>> + hdmi_modb(hdmi, aud_schnl_samplerate, HDMI_FC_AUDSCHNLS7_SMPRATE_MASK,
> >>> + HDMI_FC_AUDSCHNLS7);
> >>> +
> >>> + /*
> >>> + * Set original frequency to be the same as frequency.
> >>> + * Use one-complement value as stated in IEC60958-3 page 13.
> >>> + */
> >>> + aud_schnl_8 = (~aud_schnl_samplerate) <<
> >>> + HDMI_FC_AUDSCHNLS8_ORIGSAMPFREQ_OFFSET;
> >>> +
> >>> + /* This means word length is 16 bit. Refer to IEC60958-3 page 12. */
> >>> + aud_schnl_8 |= 2 << HDMI_FC_AUDSCHNLS8_WORDLEGNTH_OFFSET;
> >>> +
> >>> + hdmi_writeb(hdmi, aud_schnl_8, HDMI_FC_AUDSCHNLS8);
> >>> +}
> >>> +
> >>> static void hdmi_set_clk_regenerator(struct dw_hdmi *hdmi,
> >>> unsigned long pixel_clk, unsigned int sample_rate)
> >>> {
> >>> @@ -620,6 +677,8 @@ static void hdmi_set_clk_regenerator(struct dw_hdmi *hdmi,
> >>> hdmi->audio_cts = cts;
> >>> hdmi_set_cts_n(hdmi, cts, hdmi->audio_enable ? n : 0);
> >>> spin_unlock_irq(&hdmi->audio_lock);
> >>> +
> >>> + hdmi_set_schnl(hdmi);
> >>> }
> >>>
> >>> static void hdmi_init_clk_regenerator(struct dw_hdmi *hdmi)
> >>> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h
> >>> index 6988f12d89d9..619ebc1c8354 100644
> >>> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h
> >>> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h
> >>> @@ -158,6 +158,17 @@
> >>> #define HDMI_FC_SPDDEVICEINF 0x1062
> >>> #define HDMI_FC_AUDSCONF 0x1063
> >>> #define HDMI_FC_AUDSSTAT 0x1064
> >>> +#define HDMI_FC_AUDSV 0x1065
> >>> +#define HDMI_FC_AUDSU 0x1066
> >>> +#define HDMI_FC_AUDSCHNLS0 0x1067
> >>> +#define HDMI_FC_AUDSCHNLS1 0x1068
> >>> +#define HDMI_FC_AUDSCHNLS2 0x1069
> >>> +#define HDMI_FC_AUDSCHNLS3 0x106a
> >>> +#define HDMI_FC_AUDSCHNLS4 0x106b
> >>> +#define HDMI_FC_AUDSCHNLS5 0x106c
> >>> +#define HDMI_FC_AUDSCHNLS6 0x106d
> >>> +#define HDMI_FC_AUDSCHNLS7 0x106e
> >>> +#define HDMI_FC_AUDSCHNLS8 0x106f
> >>> #define HDMI_FC_DATACH0FILL 0x1070
> >>> #define HDMI_FC_DATACH1FILL 0x1071
> >>> #define HDMI_FC_DATACH2FILL 0x1072
> >>> @@ -706,6 +717,15 @@ enum {
> >>> /* HDMI_FC_AUDSCHNLS7 field values */
> >>> HDMI_FC_AUDSCHNLS7_ACCURACY_OFFSET = 4,
> >>> HDMI_FC_AUDSCHNLS7_ACCURACY_MASK = 0x30,
> >>> + HDMI_FC_AUDSCHNLS7_SMPRATE_MASK = 0x0f,
> >>> + HDMI_FC_AUDSCHNLS7_SMPRATE_192K = 0xe,
> >>> + HDMI_FC_AUDSCHNLS7_SMPRATE_176K4 = 0xc,
> >>> + HDMI_FC_AUDSCHNLS7_SMPRATE_96K = 0xa,
> >>> + HDMI_FC_AUDSCHNLS7_SMPRATE_768K = 0x9,
> >>> + HDMI_FC_AUDSCHNLS7_SMPRATE_88K2 = 0x8,
> >>> + HDMI_FC_AUDSCHNLS7_SMPRATE_32K = 0x3,
> >>> + HDMI_FC_AUDSCHNLS7_SMPRATE_48K = 0x2,
> >>> + HDMI_FC_AUDSCHNLS7_SMPRATE_44K1 = 0x0,
> >>>
> >>> /* HDMI_FC_AUDSCHNLS8 field values */
> >>> HDMI_FC_AUDSCHNLS8_ORIGSAMPFREQ_MASK = 0xf0,
> >>>
> >>
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH -next] usb: gadget: pxa25x_udc: use devm_platform_ioremap_resource() to simplify code
From: YueHaibing @ 2019-09-04 9:42 UTC (permalink / raw)
To: daniel, haojian.zhuang, robert.jarzmik, balbi, gregkh
Cc: linux-usb, YueHaibing, linux-kernel, linux-arm-kernel
Use devm_platform_ioremap_resource() to simplify the code a bit.
This is detected by coccinelle.
Reported-by: Hulk Robot <hulkci@huawei.com>
Signed-off-by: YueHaibing <yuehaibing@huawei.com>
---
drivers/usb/gadget/udc/pxa25x_udc.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/usb/gadget/udc/pxa25x_udc.c b/drivers/usb/gadget/udc/pxa25x_udc.c
index d4be535..cfafdd9 100644
--- a/drivers/usb/gadget/udc/pxa25x_udc.c
+++ b/drivers/usb/gadget/udc/pxa25x_udc.c
@@ -2321,7 +2321,6 @@ static int pxa25x_udc_probe(struct platform_device *pdev)
struct pxa25x_udc *dev = &memory;
int retval, irq;
u32 chiprev;
- struct resource *res;
pr_info("%s: version %s\n", driver_name, DRIVER_VERSION);
@@ -2367,8 +2366,7 @@ static int pxa25x_udc_probe(struct platform_device *pdev)
if (irq < 0)
return -ENODEV;
- res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- dev->regs = devm_ioremap_resource(&pdev->dev, res);
+ dev->regs = devm_platform_ioremap_resource(pdev, 0);
if (IS_ERR(dev->regs))
return PTR_ERR(dev->regs);
--
2.7.4
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* Re: [PATCH] drm: bridge/dw_hdmi: add audio sample channel status setting
From: Cheng-yi Chiang @ 2019-09-04 9:35 UTC (permalink / raw)
To: Jonas Karlman
Cc: alsa-devel@alsa-project.org, kuninori.morimoto.gx@renesas.com,
Neil Armstrong, airlied@linux.ie, dri-devel@lists.freedesktop.org,
linux-kernel@vger.kernel.org, a.hajda@samsung.com,
Laurent.pinchart@ideasonboard.com, sam@ravnborg.org,
cain.cai@rock-chips.com, zhengxing@rock-chips.com,
linux-rockchip@lists.infradead.org, dgreid@chromium.org,
tzungbi@chromium.org, jeffy.chen@rock-chips.com,
eddie.cai@rock-chips.com, linux-arm-kernel@lists.infradead.org,
Jernej Škrabec, dianders@chromium.org, daniel@ffwll.ch,
enric.balletbo@collabora.com, kuankuan.y@gmail.com
In-Reply-To: <HE1PR06MB40112AD52DAF0E837F23B441ACB90@HE1PR06MB4011.eurprd06.prod.outlook.com>
On Wed, Sep 4, 2019 at 4:33 AM Jonas Karlman <jonas@kwiboo.se> wrote:
>
> On 2019-09-03 20:08, Jernej Škrabec wrote:
> > Hi!
> >
> > Dne torek, 03. september 2019 ob 20:00:33 CEST je Neil Armstrong napisal(a):
> >> Hi,
> >>
> >> Le 03/09/2019 à 11:53, Neil Armstrong a écrit :
> >>> Hi,
> >>>
> >>> On 03/09/2019 07:51, Cheng-Yi Chiang wrote:
> >>>> From: Yakir Yang <ykk@rock-chips.com>
> >>>>
> >>>> When transmitting IEC60985 linear PCM audio, we configure the
> >>>> Audio Sample Channel Status information of all the channel
> >>>> status bits in the IEC60958 frame.
> >>>> Refer to 60958-3 page 10 for frequency, original frequency, and
> >>>> wordlength setting.
> >>>>
> >>>> This fix the issue that audio does not come out on some monitors
> >>>> (e.g. LG 22CV241)
> >>>>
> >>>> Signed-off-by: Yakir Yang <ykk@rock-chips.com>
> >>>> Signed-off-by: Cheng-Yi Chiang <cychiang@chromium.org>
> >>>> ---
> >>>>
> >>>> drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 59 +++++++++++++++++++++++
> >>>> drivers/gpu/drm/bridge/synopsys/dw-hdmi.h | 20 ++++++++
> >>>> 2 files changed, 79 insertions(+)
> >>>>
> >>>> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> >>>> b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c index
> >>>> bd65d0479683..34d46e25d610 100644
> >>>> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> >>>> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> >>>> @@ -582,6 +582,63 @@ static unsigned int hdmi_compute_n(unsigned int
> >>>> freq, unsigned long pixel_clk)>>
> >>>> return n;
> >>>>
> >>>> }
> >>>>
> >>>> +static void hdmi_set_schnl(struct dw_hdmi *hdmi)
> >>>> +{
> >>>> + u8 aud_schnl_samplerate;
> >>>> + u8 aud_schnl_8;
> >>>> +
> >>>> + /* These registers are on RK3288 using version 2.0a. */
> >>>> + if (hdmi->version != 0x200a)
> >>>> + return;
> >>> Are these limited to the 2.0a version *in* RK3288, or 2.0a version on all
> >>> SoCs ?
> >> After investigations, Amlogic sets these registers on their 2.0a version
> >> aswell, and Jernej (added in Cc) reported me Allwinner sets them on their
> >> < 2.0a and > 2.0a IPs versions.
> >>
> >> Can you check on the Rockchip IP versions in RK3399 ?
> >>
> >> For reference, the HDMI 1.4a IP version allwinner setups is:
> >> https://github.com/Allwinner-Homlet/H3-BSP4.4-linux/blob/master/drivers/vide
> >> o/fbdev/sunxi/disp2/hdmi/hdmi_bsp_sun8iw7.c#L531-L539 (registers a
> >> "scrambled" but a custom bit can reset to the original mapping, 0x1066 ...
> >> 0x106f)
> > For easier reading, here is similar, but annotated version: http://ix.io/1Ub6
> > Check function bsp_hdmi_audio().
> >
> > Unless there is a special reason, you can just remove that check.
>
> Agree, this check should not be needed, AUDSCHNLS7 used to be configured in my old
> multi-channel patches that have seen lot of testing on Amlogic, Allwinner and Rockchip SoCs.
>
As stated in previous mail, I will modify the check for version >=1.4
since I know that 1.3 does not have such register, at least on iMX6.
> >
> > Best regards,
> > Jernej
> >
> >> Neil
> >>
> >>>> +
> >>>> + switch (hdmi->sample_rate) {
> >>>> + case 32000:
> >>>> + aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_32K;
> >>>> + break;
> >>>> + case 44100:
> >>>> + aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_44K1;
> >>>> + break;
> >>>> + case 48000:
> >>>> + aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_48K;
> >>>> + break;
> >>>> + case 88200:
> >>>> + aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_88K2;
> >>>> + break;
> >>>> + case 96000:
> >>>> + aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_96K;
> >>>> + break;
> >>>> + case 176400:
> >>>> + aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_176K4;
> >>>> + break;
> >>>> + case 192000:
> >>>> + aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_192K;
> >>>> + break;
> >>>> + case 768000:
> >>>> + aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_768K;
> >>>> + break;
> >>>> + default:
> >>>> + dev_warn(hdmi->dev, "Unsupported audio sample rate (%u)\n",
> >>>> + hdmi->sample_rate);
> >>>> + return;
> >>>> + }
> >>>> +
> >>>> + /* set channel status register */
> >>>> + hdmi_modb(hdmi, aud_schnl_samplerate, HDMI_FC_AUDSCHNLS7_SMPRATE_MASK,
> >>>> + HDMI_FC_AUDSCHNLS7);
> >>>> +
> >>>> + /*
> >>>> + * Set original frequency to be the same as frequency.
> >>>> + * Use one-complement value as stated in IEC60958-3 page 13.
> >>>> + */
> >>>> + aud_schnl_8 = (~aud_schnl_samplerate) <<
> >>>> + HDMI_FC_AUDSCHNLS8_ORIGSAMPFREQ_OFFSET;
> >>>> +
> >>>> + /* This means word length is 16 bit. Refer to IEC60958-3 page 12. */
> >>>> + aud_schnl_8 |= 2 << HDMI_FC_AUDSCHNLS8_WORDLEGNTH_OFFSET;
>
> This looks wrong, user can use 16 and 24 bit wide audio streams.
>
Thanks for spotting this issue.
I will fix it in v2 (following how http://ix.io/1Ub6 set it for 16 and 24 bit)
> >>>> +
> >>>> + hdmi_writeb(hdmi, aud_schnl_8, HDMI_FC_AUDSCHNLS8);
> >>>> +}
> >>>> +
> >>>>
> >>>> static void hdmi_set_clk_regenerator(struct dw_hdmi *hdmi,
> >>>>
> >>>> unsigned long pixel_clk, unsigned int sample_rate)
> >>>>
> >>>> {
> >>>>
> >>>> @@ -620,6 +677,8 @@ static void hdmi_set_clk_regenerator(struct dw_hdmi
> >>>> *hdmi,>>
> >>>> hdmi->audio_cts = cts;
> >>>> hdmi_set_cts_n(hdmi, cts, hdmi->audio_enable ? n : 0);
> >>>> spin_unlock_irq(&hdmi->audio_lock);
> >>>>
> >>>> +
> >>>> + hdmi_set_schnl(hdmi);
>
> I will suggest this function is called from or merged with dw_hdmi_set_sample_rate().
> Similar to how AUDSCONF and AUDICONF0 is configured from dw_hdmi_set_channel_count().
>
I see. I think it will make sense to add a function
set_channel_status() for dw-hdmi-i2s-audio.c to call,
since this function is more than just setting rate.
Will fix in v2.
Thanks!
> Regards,
> Jonas
>
> >>>>
> >>>> }
> >>>>
> >>>> static void hdmi_init_clk_regenerator(struct dw_hdmi *hdmi)
> >>>>
> >>>> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h
> >>>> b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h index
> >>>> 6988f12d89d9..619ebc1c8354 100644
> >>>> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h
> >>>> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h
> >>>> @@ -158,6 +158,17 @@
> >>>>
> >>>> #define HDMI_FC_SPDDEVICEINF 0x1062
> >>>> #define HDMI_FC_AUDSCONF 0x1063
> >>>> #define HDMI_FC_AUDSSTAT 0x1064
> >>>>
> >>>> +#define HDMI_FC_AUDSV 0x1065
> >>>> +#define HDMI_FC_AUDSU 0x1066
> >>>> +#define HDMI_FC_AUDSCHNLS0 0x1067
> >>>> +#define HDMI_FC_AUDSCHNLS1 0x1068
> >>>> +#define HDMI_FC_AUDSCHNLS2 0x1069
> >>>> +#define HDMI_FC_AUDSCHNLS3 0x106a
> >>>> +#define HDMI_FC_AUDSCHNLS4 0x106b
> >>>> +#define HDMI_FC_AUDSCHNLS5 0x106c
> >>>> +#define HDMI_FC_AUDSCHNLS6 0x106d
> >>>> +#define HDMI_FC_AUDSCHNLS7 0x106e
> >>>> +#define HDMI_FC_AUDSCHNLS8 0x106f
> >>>>
> >>>> #define HDMI_FC_DATACH0FILL 0x1070
> >>>> #define HDMI_FC_DATACH1FILL 0x1071
> >>>> #define HDMI_FC_DATACH2FILL 0x1072
> >>>>
> >>>> @@ -706,6 +717,15 @@ enum {
> >>>>
> >>>> /* HDMI_FC_AUDSCHNLS7 field values */
> >>>>
> >>>> HDMI_FC_AUDSCHNLS7_ACCURACY_OFFSET = 4,
> >>>> HDMI_FC_AUDSCHNLS7_ACCURACY_MASK = 0x30,
> >>>>
> >>>> + HDMI_FC_AUDSCHNLS7_SMPRATE_MASK = 0x0f,
> >>>> + HDMI_FC_AUDSCHNLS7_SMPRATE_192K = 0xe,
> >>>> + HDMI_FC_AUDSCHNLS7_SMPRATE_176K4 = 0xc,
> >>>> + HDMI_FC_AUDSCHNLS7_SMPRATE_96K = 0xa,
> >>>> + HDMI_FC_AUDSCHNLS7_SMPRATE_768K = 0x9,
> >>>> + HDMI_FC_AUDSCHNLS7_SMPRATE_88K2 = 0x8,
> >>>> + HDMI_FC_AUDSCHNLS7_SMPRATE_32K = 0x3,
> >>>> + HDMI_FC_AUDSCHNLS7_SMPRATE_48K = 0x2,
> >>>> + HDMI_FC_AUDSCHNLS7_SMPRATE_44K1 = 0x0,
> >>>>
> >>>> /* HDMI_FC_AUDSCHNLS8 field values */
> >>>>
> >>>> HDMI_FC_AUDSCHNLS8_ORIGSAMPFREQ_MASK = 0xf0,
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH -next] usb: gadget: bcm63xx_udc: use devm_platform_ioremap_resource() to simplify code
From: YueHaibing @ 2019-09-04 9:32 UTC (permalink / raw)
To: cernekee, balbi, gregkh, f.fainelli
Cc: YueHaibing, linux-usb, bcm-kernel-feedback-list, linux-kernel,
linux-arm-kernel
Use devm_platform_ioremap_resource() to simplify the code a bit.
This is detected by coccinelle.
Reported-by: Hulk Robot <hulkci@huawei.com>
Signed-off-by: YueHaibing <yuehaibing@huawei.com>
---
drivers/usb/gadget/udc/bcm63xx_udc.c | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/drivers/usb/gadget/udc/bcm63xx_udc.c b/drivers/usb/gadget/udc/bcm63xx_udc.c
index 97b1646..7fcf4a8 100644
--- a/drivers/usb/gadget/udc/bcm63xx_udc.c
+++ b/drivers/usb/gadget/udc/bcm63xx_udc.c
@@ -2282,7 +2282,6 @@ static int bcm63xx_udc_probe(struct platform_device *pdev)
struct device *dev = &pdev->dev;
struct bcm63xx_usbd_platform_data *pd = dev_get_platdata(dev);
struct bcm63xx_udc *udc;
- struct resource *res;
int rc = -ENOMEM, i, irq;
udc = devm_kzalloc(dev, sizeof(*udc), GFP_KERNEL);
@@ -2298,13 +2297,11 @@ static int bcm63xx_udc_probe(struct platform_device *pdev)
return -EINVAL;
}
- res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- udc->usbd_regs = devm_ioremap_resource(dev, res);
+ udc->usbd_regs = devm_platform_ioremap_resource(pdev, 0);
if (IS_ERR(udc->usbd_regs))
return PTR_ERR(udc->usbd_regs);
- res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
- udc->iudma_regs = devm_ioremap_resource(dev, res);
+ udc->iudma_regs = devm_platform_ioremap_resource(pdev, 1);
if (IS_ERR(udc->iudma_regs))
return PTR_ERR(udc->iudma_regs);
--
2.7.4
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* Re: [GIT PULL 1/3] soc: samsung: Exynos for v5.4
From: Arnd Bergmann @ 2019-09-04 9:31 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: linux-samsung-soc@vger.kernel.org, linux-kernel@vger.kernel.org,
SoC Team, arm-soc, Kukjin Kim, Olof Johansson, Linux ARM
In-Reply-To: <CAJKOXPfRMXkm_pT560Ry5k-zFWpkRDmFHSs2Fb3RL7d4h=ka9g@mail.gmail.com>
On Wed, Sep 4, 2019 at 10:37 AM Krzysztof Kozlowski <krzk@kernel.org> wrote:
> On Tue, 3 Sep 2019 at 19:21, Arnd Bergmann <arnd@arndb.de> wrote:
> > On Thu, Aug 22, 2019 at 8:35 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
> > I have drafted a related patch recently, regarding the related
> > arch/arm/plat-samsung/cpu.c file. This is part of a longer series
> > I'm working on, see https://pastebin.com/ZqeU3Mth for the
> > current version of this patch.
>
> You can then also adjust the include path in arch/arm/mach-exynos/Makefile.
Yes, that is part of the following patch, along with replacing all the
'depends on PLAT_SAMSUNG' in Kconfig with 'depends on PLAT_SAMSUNG ||
ARCH_EXYNOS'.
> > The observation is that mach-exynos
> > is almost completely independent of plat-samsung these days, and my
> > patch removes the last obstacle from separating the two. I have
> > another set of patches to do the same for mach-s5pv210 (which shares
> > half of its pm.c with plat-samsung, but nothing else).
>
> Great!
FYI, the other parts of the series include:
- Changing all s3c24xx drivers to no longer use mach/*.h or plat/*.h header
files, as preparation for multiplatform support. This is work in progress,
but mostly done, with cpufreq and ASoC as the notable exceptions.
- Merging mach-s3c24xx, mach-s3c64xx and plat-samsung into a common
mach-s3c directory. This seems to work fine and looks nicer than having
three tightly connected directories, but the downside is that all path
names change and the directory becomes fairly large.
I think we can actually do the same thing for all remaining plat-*
directories.
- eventually making all ARMv5 platforms link into a single kernel, like we do
with ARMv6 and ARMv7 (more to be done there, but s3c24xx and pxa are
the harder bits here).
Arnd
_______________________________________________
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] drm: bridge/dw_hdmi: add audio sample channel status setting
From: Neil Armstrong @ 2019-09-04 9:28 UTC (permalink / raw)
To: Cheng-yi Chiang
Cc: moderated list:SOUND - SOC LAYER / DYNAMIC AUDIO POWER MANAGEM...,
Doug Anderson, kuninori.morimoto.gx, David Airlie, dri-devel,
linux-kernel, Andrzej Hajda, Laurent Pinchart, Yakir Yang, sam,
Xing Zheng, linux-rockchip, Dylan Reid, tzungbi, Jonas Karlman,
Jeffy Chen, 蔡枫, linux-arm-kernel, Jernej Skrabec,
cain.cai, Daniel Vetter, Enric Balletbo i Serra, kuankuan.y
In-Reply-To: <CAFv8NwKHZM+zTu7GF_J0Xk6hubA2JK4cCsdhsDPOGk=3rnbCZw@mail.gmail.com>
Hi,
On 04/09/2019 11:09, Cheng-yi Chiang wrote:
> Hi,
>
> On Tue, Sep 3, 2019 at 5:53 PM Neil Armstrong <narmstrong@baylibre.com> wrote:
>>
>> Hi,
>>
>> On 03/09/2019 07:51, Cheng-Yi Chiang wrote:
>>> From: Yakir Yang <ykk@rock-chips.com>
>>>
>>> When transmitting IEC60985 linear PCM audio, we configure the
>>> Audio Sample Channel Status information of all the channel
>>> status bits in the IEC60958 frame.
>>> Refer to 60958-3 page 10 for frequency, original frequency, and
>>> wordlength setting.
>>>
>>> This fix the issue that audio does not come out on some monitors
>>> (e.g. LG 22CV241)
>>>
>>> Signed-off-by: Yakir Yang <ykk@rock-chips.com>
>>> Signed-off-by: Cheng-Yi Chiang <cychiang@chromium.org>
>>> ---
>>> drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 59 +++++++++++++++++++++++
>>> drivers/gpu/drm/bridge/synopsys/dw-hdmi.h | 20 ++++++++
>>> 2 files changed, 79 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>>> index bd65d0479683..34d46e25d610 100644
>>> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>>> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>>> @@ -582,6 +582,63 @@ static unsigned int hdmi_compute_n(unsigned int freq, unsigned long pixel_clk)
>>> return n;
>>> }
>>>
>>> +static void hdmi_set_schnl(struct dw_hdmi *hdmi)
>>> +{
>>> + u8 aud_schnl_samplerate;
>>> + u8 aud_schnl_8;
>>> +
>>> + /* These registers are on RK3288 using version 2.0a. */
>>> + if (hdmi->version != 0x200a)
>>> + return;
>>
>> Are these limited to the 2.0a version *in* RK3288, or 2.0a version on all
>> SoCs ?
>>
>
> In the original patch by Yakir,
>
> https://lore.kernel.org/patchwork/patch/539653/ (sorry, I should
> have added this link in the "after the cut" note)
>
> The fix is limited to version 2.0.
> Since I am only testing on RK3288 with 2.0a, I change the check to 2.0a only.
> I can not test 2.0a version on other SoCs.
> The databook I have at hand is 2.0a (not specific to RK3288) so I
> think all 2.0a should have this register.
>
> As for other version like version 1.3 on iMX6, there is no such
> register, as stated by Russell
>
> http://lkml.iu.edu/hypermail/linux/kernel/1501.3/06268.html.
Russell assumes the registers are not present on the iMX6 IP not having
the I2S registers, but they are present on the IPs configured with I2S
at any versions.
My databook version (1.40.a) specifies :
fc_audschnls0 to fc_audschnls8
```
When transmitting IEC60958 linear PCM audio, this registers allow to configure the channel status
information of all the channel status bits in the IEC60958 frame. For the moment this configuration is only
used when the I2S audio interface, General Purpose Audio (GPA), or AHB audio DMA (AHBAUDDMA)
interface is active (for S/PDIF interface this information comes from the stream).
```
But the databook Revision History shows these registers were present since version 1.10a.
I propose you remove the version check, but you only setup these registers when I2S is enabled:
(hdmi_readb(hdmi, HDMI_CONFIG0_ID) & HDMI_CONFIG0_I2S) until a AHBAUDDMA user needs this,
with a small comment explaining the situation.
Neil
>
> So at least we should check the version.
> Maybe we can set the criteria as version 2.0 or above to make it a safe patch ?
> If there is the same need on other SoC with version < 2.0, it can be
> added later.
> Presumably, there will be databook of that version to help confirming
> this setting.
>
> Thanks!
>>> +
>>> + switch (hdmi->sample_rate) {
>>> + case 32000:
>>> + aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_32K;
>>> + break;
>>> + case 44100:
>>> + aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_44K1;
>>> + break;
>>> + case 48000:
>>> + aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_48K;
>>> + break;
>>> + case 88200:
>>> + aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_88K2;
>>> + break;
>>> + case 96000:
>>> + aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_96K;
>>> + break;
>>> + case 176400:
>>> + aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_176K4;
>>> + break;
>>> + case 192000:
>>> + aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_192K;
>>> + break;
>>> + case 768000:
>>> + aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_768K;
>>> + break;
>>> + default:
>>> + dev_warn(hdmi->dev, "Unsupported audio sample rate (%u)\n",
>>> + hdmi->sample_rate);
>>> + return;
>>> + }
>>> +
>>> + /* set channel status register */
>>> + hdmi_modb(hdmi, aud_schnl_samplerate, HDMI_FC_AUDSCHNLS7_SMPRATE_MASK,
>>> + HDMI_FC_AUDSCHNLS7);
>>> +
>>> + /*
>>> + * Set original frequency to be the same as frequency.
>>> + * Use one-complement value as stated in IEC60958-3 page 13.
>>> + */
>>> + aud_schnl_8 = (~aud_schnl_samplerate) <<
>>> + HDMI_FC_AUDSCHNLS8_ORIGSAMPFREQ_OFFSET;
>>> +
>>> + /* This means word length is 16 bit. Refer to IEC60958-3 page 12. */
>>> + aud_schnl_8 |= 2 << HDMI_FC_AUDSCHNLS8_WORDLEGNTH_OFFSET;
>>> +
>>> + hdmi_writeb(hdmi, aud_schnl_8, HDMI_FC_AUDSCHNLS8);
>>> +}
>>> +
>>> static void hdmi_set_clk_regenerator(struct dw_hdmi *hdmi,
>>> unsigned long pixel_clk, unsigned int sample_rate)
>>> {
>>> @@ -620,6 +677,8 @@ static void hdmi_set_clk_regenerator(struct dw_hdmi *hdmi,
>>> hdmi->audio_cts = cts;
>>> hdmi_set_cts_n(hdmi, cts, hdmi->audio_enable ? n : 0);
>>> spin_unlock_irq(&hdmi->audio_lock);
>>> +
>>> + hdmi_set_schnl(hdmi);
>>> }
>>>
>>> static void hdmi_init_clk_regenerator(struct dw_hdmi *hdmi)
>>> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h
>>> index 6988f12d89d9..619ebc1c8354 100644
>>> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h
>>> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h
>>> @@ -158,6 +158,17 @@
>>> #define HDMI_FC_SPDDEVICEINF 0x1062
>>> #define HDMI_FC_AUDSCONF 0x1063
>>> #define HDMI_FC_AUDSSTAT 0x1064
>>> +#define HDMI_FC_AUDSV 0x1065
>>> +#define HDMI_FC_AUDSU 0x1066
>>> +#define HDMI_FC_AUDSCHNLS0 0x1067
>>> +#define HDMI_FC_AUDSCHNLS1 0x1068
>>> +#define HDMI_FC_AUDSCHNLS2 0x1069
>>> +#define HDMI_FC_AUDSCHNLS3 0x106a
>>> +#define HDMI_FC_AUDSCHNLS4 0x106b
>>> +#define HDMI_FC_AUDSCHNLS5 0x106c
>>> +#define HDMI_FC_AUDSCHNLS6 0x106d
>>> +#define HDMI_FC_AUDSCHNLS7 0x106e
>>> +#define HDMI_FC_AUDSCHNLS8 0x106f
>>> #define HDMI_FC_DATACH0FILL 0x1070
>>> #define HDMI_FC_DATACH1FILL 0x1071
>>> #define HDMI_FC_DATACH2FILL 0x1072
>>> @@ -706,6 +717,15 @@ enum {
>>> /* HDMI_FC_AUDSCHNLS7 field values */
>>> HDMI_FC_AUDSCHNLS7_ACCURACY_OFFSET = 4,
>>> HDMI_FC_AUDSCHNLS7_ACCURACY_MASK = 0x30,
>>> + HDMI_FC_AUDSCHNLS7_SMPRATE_MASK = 0x0f,
>>> + HDMI_FC_AUDSCHNLS7_SMPRATE_192K = 0xe,
>>> + HDMI_FC_AUDSCHNLS7_SMPRATE_176K4 = 0xc,
>>> + HDMI_FC_AUDSCHNLS7_SMPRATE_96K = 0xa,
>>> + HDMI_FC_AUDSCHNLS7_SMPRATE_768K = 0x9,
>>> + HDMI_FC_AUDSCHNLS7_SMPRATE_88K2 = 0x8,
>>> + HDMI_FC_AUDSCHNLS7_SMPRATE_32K = 0x3,
>>> + HDMI_FC_AUDSCHNLS7_SMPRATE_48K = 0x2,
>>> + HDMI_FC_AUDSCHNLS7_SMPRATE_44K1 = 0x0,
>>>
>>> /* HDMI_FC_AUDSCHNLS8 field values */
>>> HDMI_FC_AUDSCHNLS8_ORIGSAMPFREQ_MASK = 0xf0,
>>>
>>
_______________________________________________
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] drm: bridge/dw_hdmi: add audio sample channel status setting
From: Russell King - ARM Linux admin @ 2019-09-04 9:27 UTC (permalink / raw)
To: Cheng-yi Chiang
Cc: moderated list:SOUND - SOC LAYER / DYNAMIC AUDIO POWER MANAGEM...,
kuninori.morimoto.gx, Neil Armstrong, David Airlie, dri-devel,
cain.cai, Andrzej Hajda, Laurent Pinchart, Yakir Yang, sam,
Xing Zheng, linux-rockchip, Dylan Reid, tzungbi, Jeffy Chen,
蔡枫, linux-arm-kernel, Doug Anderson, linux-kernel,
Daniel Vetter, Enric Balletbo i Serra, kuankuan.y
In-Reply-To: <CAFv8NwKHZM+zTu7GF_J0Xk6hubA2JK4cCsdhsDPOGk=3rnbCZw@mail.gmail.com>
On Wed, Sep 04, 2019 at 05:09:29PM +0800, Cheng-yi Chiang wrote:
> Hi,
>
> On Tue, Sep 3, 2019 at 5:53 PM Neil Armstrong <narmstrong@baylibre.com> wrote:
> >
> > Hi,
> >
> > On 03/09/2019 07:51, Cheng-Yi Chiang wrote:
> > > From: Yakir Yang <ykk@rock-chips.com>
> > >
> > > When transmitting IEC60985 linear PCM audio, we configure the
> > > Audio Sample Channel Status information of all the channel
> > > status bits in the IEC60958 frame.
> > > Refer to 60958-3 page 10 for frequency, original frequency, and
> > > wordlength setting.
> > >
> > > This fix the issue that audio does not come out on some monitors
> > > (e.g. LG 22CV241)
> > >
> > > Signed-off-by: Yakir Yang <ykk@rock-chips.com>
> > > Signed-off-by: Cheng-Yi Chiang <cychiang@chromium.org>
> > > ---
> > > drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 59 +++++++++++++++++++++++
> > > drivers/gpu/drm/bridge/synopsys/dw-hdmi.h | 20 ++++++++
> > > 2 files changed, 79 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> > > index bd65d0479683..34d46e25d610 100644
> > > --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> > > +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> > > @@ -582,6 +582,63 @@ static unsigned int hdmi_compute_n(unsigned int freq, unsigned long pixel_clk)
> > > return n;
> > > }
> > >
> > > +static void hdmi_set_schnl(struct dw_hdmi *hdmi)
> > > +{
> > > + u8 aud_schnl_samplerate;
> > > + u8 aud_schnl_8;
> > > +
> > > + /* These registers are on RK3288 using version 2.0a. */
> > > + if (hdmi->version != 0x200a)
> > > + return;
> >
> > Are these limited to the 2.0a version *in* RK3288, or 2.0a version on all
> > SoCs ?
> >
>
> In the original patch by Yakir,
>
> https://lore.kernel.org/patchwork/patch/539653/ (sorry, I should
> have added this link in the "after the cut" note)
>
> The fix is limited to version 2.0.
> Since I am only testing on RK3288 with 2.0a, I change the check to 2.0a only.
> I can not test 2.0a version on other SoCs.
> The databook I have at hand is 2.0a (not specific to RK3288) so I
> think all 2.0a should have this register.
>
> As for other version like version 1.3 on iMX6, there is no such
> register, as stated by Russell
>
> http://lkml.iu.edu/hypermail/linux/kernel/1501.3/06268.html.
It's likely more to do with how the IP is configured rather than the
version. The big difference between dw-hdmi used in iMX6 and elsewhere
is that iMX6 uses a built-in AHB audio interface and not I2S. Elsewhere
uses I2S.
I2S does not have the capability to convey channel status information
(which is required by HDMI). With AHB, it is encoded into the data in
memory.
So, I think this setup should be done in the I2S driver and not in the
core driver.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* issue when compile 3.18 kernel with later gcc
From: Belisko Marek @ 2019-09-04 9:27 UTC (permalink / raw)
To: linux-arm-kernel
Hi,
I'm using hisilicon kernel for camera and I'm trying to be able to
build it with some later gcc7. Kernel version is 3.18 and it builds
fine with gcc 4.9 (linaro armhf). When building with gcc7 I got an
error:
arch/arm/mach-hisi/built-in.o: In function `hi_pmc_clear_a17_ac':
arch/arm/mach-hisi/cpu_helper_a7.o:(.text+0x1b4): undefined reference
to `pmc_phys_addr'
In sources is variable defined like:
static u32 pmc_phys_addr;
and then used in this function:
/* call from assable context */
asmlinkage void __naked hi_pmc_clear_a17_ac(void)
{
asm volatile("\n"
..
".align 2\n"
"1: .word .\n"
" .word pmc_phys_addr\n"
);
unreachable();
}
and when disassemble generated object file variable is not present in
.bss section (while with old gcc it's perfectly fine). Is there any
gcc option how to resolve this issue or should it be solved some other
way. Thanks.
BR,
marek
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [RFC PATCH V2 4/4] platform: mtk-isp: Add Mediatek FD driver
From: Jerry-ch Chen @ 2019-09-04 9:26 UTC (permalink / raw)
To: Tomasz Figa
Cc: devicetree@vger.kernel.org, Sean Cheng (鄭昇弘),
laurent.pinchart+renesas@ideasonboard.com,
Rynn Wu (吳育恩), srv_heupstream,
Po-Yang Huang (黃柏陽), mchehab@kernel.org,
suleiman@chromium.org, shik@chromium.org,
Jungo Lin (林明俊),
Sj Huang (黃信璋), yuzhao@chromium.org,
linux-mediatek@lists.infradead.org, zwisler@chromium.org,
matthias.bgg@gmail.com, Christie Yu (游雅惠),
Frederic Chen (陳俊元), hans.verkuil@cisco.com,
linux-arm-kernel@lists.infradead.org, linux-media@vger.kernel.org
In-Reply-To: <CAAFQd5DWfEEiGthPi=qoxD-mpAWa68GOCi55mqpmagS-tsGYkA@mail.gmail.com>
Hi Tomasz,
On Wed, 2019-09-04 at 17:03 +0800, Tomasz Figa wrote:
> On Wed, Sep 4, 2019 at 6:02 PM Jerry-ch Chen <Jerry-ch.Chen@mediatek.com> wrote:
> >
> > Hi Tomasz,
> >
> > On Wed, 2019-09-04 at 16:25 +0800, Tomasz Figa wrote:
> > > On Wed, Sep 4, 2019 at 5:09 PM Jerry-ch Chen <Jerry-ch.Chen@mediatek.com> wrote:
> > > >
> > > > Hi Tomasz,
> > > >
> > > > On Wed, 2019-09-04 at 14:34 +0800, Tomasz Figa wrote:
> > > > > On Wed, Sep 4, 2019 at 3:09 PM Jerry-ch Chen <Jerry-ch.Chen@mediatek.com> wrote:
> > > > > >
> > > > > > Hi Tomasz,
> > > > > >
> > > > > > On Wed, 2019-09-04 at 12:15 +0800, Tomasz Figa wrote:
> > > > > > > On Wed, Sep 4, 2019 at 12:38 PM Jerry-ch Chen
> > > > > > > <Jerry-ch.Chen@mediatek.com> wrote:
> > > > > > > >
> > > > > > > > Hi Tomasz,
> > > > > > > >
> > > > > > > > On Tue, 2019-09-03 at 20:05 +0800, Tomasz Figa wrote:
> > > > > > > > > On Tue, Sep 3, 2019 at 8:46 PM Jerry-ch Chen <Jerry-ch.Chen@mediatek.com> wrote:
> > > > > > > > > >
> > > > > > > > > > Hi Tomasz,
> > > > > > > > > >
> > > > > > > > > > On Tue, 2019-09-03 at 15:04 +0800, Tomasz Figa wrote:
> > > > > > > > > > > On Tue, Sep 3, 2019 at 3:44 PM Jerry-ch Chen <Jerry-ch.Chen@mediatek.com> wrote:
> > > > > > > > > > > >
> > > > > > > > > > > > On Tue, 2019-09-03 at 13:19 +0800, Tomasz Figa wrote:
> > > > > > > > > > > > > On Mon, Sep 2, 2019 at 8:47 PM Jerry-ch Chen <Jerry-ch.Chen@mediatek.com> wrote:
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > Hi Tomasz,
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > On Fri, 2019-08-30 at 16:33 +0800, Tomasz Figa wrote:
> > > > > > > > > > > > > > > On Wed, Aug 28, 2019 at 11:00 AM Jerry-ch Chen
> > > > > > > > > > > > > > > <Jerry-ch.Chen@mediatek.com> wrote:
> > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > Hi Tomasz,
> > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > On Mon, 2019-08-26 at 14:36 +0800, Tomasz Figa wrote:
> > > > > > > > > > > > > > > > > Hi Jerry,
> > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > On Sun, Aug 25, 2019 at 6:18 PM Jerry-ch Chen
> > > > > > > > > > > > > > > > > <Jerry-ch.Chen@mediatek.com> wrote:
> > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > Hi Tomasz,
> > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > On Fri, 2019-08-02 at 16:28 +0800, Tomasz Figa wrote:
> > > > > > > > > > > > > > > > > > > Hi Jerry,
> > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > On Tue, Jul 09, 2019 at 04:41:12PM +0800, Jerry-ch Chen wrote:
[snip]
> > > > static void mtk_fd_vb2_stop_streaming(struct vb2_queue *vq)
> > > > {
> > > > struct mtk_fd_ctx *ctx = vb2_get_drv_priv(vq);
> > > > struct mtk_fd_dev *fd = ctx->fd_dev;
> > > > struct vb2_v4l2_buffer *vb;
> > > > struct v4l2_m2m_ctx *m2m_ctx = ctx->fh.m2m_ctx;
> > > > struct v4l2_m2m_queue_ctx *queue_ctx;
> > > > u32 ret;
> > > >
> > > > if (!fd->fd_irq_done.done)
> > >
> > > We shouldn't access internal fields of completion.
> > >
> > > > ret = wait_for_completion_timeout(&fd->fd_irq_done,
> > > > msecs_to_jiffies(
> > > > MTK_FD_HW_TIMEOUT));
> > > > queue_ctx = V4L2_TYPE_IS_OUTPUT(vq->type) ?
> > > > &m2m_ctx->out_q_ctx :
> > > > &m2m_ctx->cap_q_ctx;
> > > > while ((vb = v4l2_m2m_buf_remove(queue_ctx)))
> > > > v4l2_m2m_buf_done(vb, VB2_BUF_STATE_ERROR);
> > > >
> > > > if (vq->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE)
> > > > mtk_fd_hw_disconnect(fd);
> > > > }
> > > >
> > > > I've also tried to wait completion unconditionally for both queues and
> > > > the second time will wait until timeout, as a result, it takes longer to
> > > > swap the camera every time and close the camera app.
> > >
> > > I think it should work better if we call complete_all() instead of complete().
> > >
> > Thanks,
> >
> > I use complete_all(), and it works fine now.
> >
> > static void mtk_fd_vb2_stop_streaming(struct vb2_queue *vq)
> > {
> > struct mtk_fd_ctx *ctx = vb2_get_drv_priv(vq);
> > struct mtk_fd_dev *fd = ctx->fd_dev;
> > struct vb2_v4l2_buffer *vb;
> > struct v4l2_m2m_ctx *m2m_ctx = ctx->fh.m2m_ctx;
> > struct v4l2_m2m_queue_ctx *queue_ctx;
> >
> > wait_for_completion_timeout(&fd->fd_irq_done,
> > msecs_to_jiffies(MTK_FD_HW_TIMEOUT));
>
> Shouldn't we still send some command to the hardware to stop? Like a
> reset. Otherwise we don't know if it isn't still accessing the memory.
>
I thought no more jobs will be enqueued here when stop_streaming so we
don't need it.
We still could send an ipi command to reset the HW, and wait for it's
callback or we could set the register MTK_FD_REG_OFFSET_HW_ENABLE to
zero to disable the HW.
Best regards,
Jerry
> > queue_ctx = V4L2_TYPE_IS_OUTPUT(vq->type) ?
> > &m2m_ctx->out_q_ctx :
> > &m2m_ctx->cap_q_ctx;
> > while ((vb = v4l2_m2m_buf_remove(queue_ctx)))
> > v4l2_m2m_buf_done(vb, VB2_BUF_STATE_ERROR);
> >
> > if (vq->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE)
> > mtk_fd_hw_disconnect(fd);
> > }
> >
> > Best regards,
> > Jerry
> >
> > > Best regards,
> > > Tomasz
> >
> >
_______________________________________________
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] drm: bridge/dw_hdmi: add audio sample channel status setting
From: Cheng-yi Chiang @ 2019-09-04 9:24 UTC (permalink / raw)
To: Jernej Škrabec
Cc: moderated list:SOUND - SOC LAYER / DYNAMIC AUDIO POWER MANAGEM...,
kuninori.morimoto.gx, Neil Armstrong, David Airlie, dri-devel,
Doug Anderson, Andrzej Hajda, Laurent Pinchart, sam, cain.cai,
Xing Zheng, linux-rockchip, Dylan Reid, tzungbi, Jonas Karlman,
Jeffy Chen, 蔡枫, linux-arm-kernel, linux-kernel,
Daniel Vetter, Enric Balletbo i Serra, kuankuan.y
In-Reply-To: <19353031.SdOy5F5fmg@jernej-laptop>
On Wed, Sep 4, 2019 at 2:08 AM Jernej Škrabec <jernej.skrabec@siol.net> wrote:
>
> Hi!
>
> Dne torek, 03. september 2019 ob 20:00:33 CEST je Neil Armstrong napisal(a):
> > Hi,
> >
> > Le 03/09/2019 à 11:53, Neil Armstrong a écrit :
> > > Hi,
> > >
> > > On 03/09/2019 07:51, Cheng-Yi Chiang wrote:
> > >> From: Yakir Yang <ykk@rock-chips.com>
> > >>
> > >> When transmitting IEC60985 linear PCM audio, we configure the
> > >> Audio Sample Channel Status information of all the channel
> > >> status bits in the IEC60958 frame.
> > >> Refer to 60958-3 page 10 for frequency, original frequency, and
> > >> wordlength setting.
> > >>
> > >> This fix the issue that audio does not come out on some monitors
> > >> (e.g. LG 22CV241)
> > >>
> > >> Signed-off-by: Yakir Yang <ykk@rock-chips.com>
> > >> Signed-off-by: Cheng-Yi Chiang <cychiang@chromium.org>
> > >> ---
> > >>
> > >> drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 59 +++++++++++++++++++++++
> > >> drivers/gpu/drm/bridge/synopsys/dw-hdmi.h | 20 ++++++++
> > >> 2 files changed, 79 insertions(+)
> > >>
> > >> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> > >> b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c index
> > >> bd65d0479683..34d46e25d610 100644
> > >> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> > >> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> > >> @@ -582,6 +582,63 @@ static unsigned int hdmi_compute_n(unsigned int
> > >> freq, unsigned long pixel_clk)>>
> > >> return n;
> > >>
> > >> }
> > >>
> > >> +static void hdmi_set_schnl(struct dw_hdmi *hdmi)
> > >> +{
> > >> + u8 aud_schnl_samplerate;
> > >> + u8 aud_schnl_8;
> > >> +
> > >> + /* These registers are on RK3288 using version 2.0a. */
> > >> + if (hdmi->version != 0x200a)
> > >> + return;
> > >
> > > Are these limited to the 2.0a version *in* RK3288, or 2.0a version on all
> > > SoCs ?
> >
> > After investigations, Amlogic sets these registers on their 2.0a version
> > aswell, and Jernej (added in Cc) reported me Allwinner sets them on their
> > < 2.0a and > 2.0a IPs versions.
> >
> > Can you check on the Rockchip IP versions in RK3399 ?
> >
> > For reference, the HDMI 1.4a IP version allwinner setups is:
> > https://github.com/Allwinner-Homlet/H3-BSP4.4-linux/blob/master/drivers/vide
> > o/fbdev/sunxi/disp2/hdmi/hdmi_bsp_sun8iw7.c#L531-L539 (registers a
> > "scrambled" but a custom bit can reset to the original mapping, 0x1066 ...
> > 0x106f)
>
> For easier reading, here is similar, but annotated version: http://ix.io/1Ub6
> Check function bsp_hdmi_audio().
>
> Unless there is a special reason, you can just remove that check.
>
Thanks for the great reference.
I also see that I need to set the word length according to the desired
value passed by dw_hdmi_i2s_hw_params in dw-hdmi-i2s-audio.c.
Will fix in v2.
> Best regards,
> Jernej
>
> >
> > Neil
> >
> > >> +
> > >> + switch (hdmi->sample_rate) {
> > >> + case 32000:
> > >> + aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_32K;
> > >> + break;
> > >> + case 44100:
> > >> + aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_44K1;
> > >> + break;
> > >> + case 48000:
> > >> + aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_48K;
> > >> + break;
> > >> + case 88200:
> > >> + aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_88K2;
> > >> + break;
> > >> + case 96000:
> > >> + aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_96K;
> > >> + break;
> > >> + case 176400:
> > >> + aud_schnl_samplerate =
> HDMI_FC_AUDSCHNLS7_SMPRATE_176K4;
> > >> + break;
> > >> + case 192000:
> > >> + aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_192K;
> > >> + break;
> > >> + case 768000:
> > >> + aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_768K;
> > >> + break;
> > >> + default:
> > >> + dev_warn(hdmi->dev, "Unsupported audio sample rate (%u)
> \n",
> > >> + hdmi->sample_rate);
> > >> + return;
> > >> + }
> > >> +
> > >> + /* set channel status register */
> > >> + hdmi_modb(hdmi, aud_schnl_samplerate,
> HDMI_FC_AUDSCHNLS7_SMPRATE_MASK,
> > >> + HDMI_FC_AUDSCHNLS7);
> > >> +
> > >> + /*
> > >> + * Set original frequency to be the same as frequency.
> > >> + * Use one-complement value as stated in IEC60958-3 page 13.
> > >> + */
> > >> + aud_schnl_8 = (~aud_schnl_samplerate) <<
> > >> + HDMI_FC_AUDSCHNLS8_ORIGSAMPFREQ_OFFSET;
> > >> +
> > >> + /* This means word length is 16 bit. Refer to IEC60958-3 page 12.
> */
> > >> + aud_schnl_8 |= 2 << HDMI_FC_AUDSCHNLS8_WORDLEGNTH_OFFSET;
> > >> +
> > >> + hdmi_writeb(hdmi, aud_schnl_8, HDMI_FC_AUDSCHNLS8);
> > >> +}
> > >> +
> > >>
> > >> static void hdmi_set_clk_regenerator(struct dw_hdmi *hdmi,
> > >>
> > >> unsigned long pixel_clk, unsigned int sample_rate)
> > >>
> > >> {
> > >>
> > >> @@ -620,6 +677,8 @@ static void hdmi_set_clk_regenerator(struct dw_hdmi
> > >> *hdmi,>>
> > >> hdmi->audio_cts = cts;
> > >> hdmi_set_cts_n(hdmi, cts, hdmi->audio_enable ? n : 0);
> > >> spin_unlock_irq(&hdmi->audio_lock);
> > >>
> > >> +
> > >> + hdmi_set_schnl(hdmi);
> > >>
> > >> }
> > >>
> > >> static void hdmi_init_clk_regenerator(struct dw_hdmi *hdmi)
> > >>
> > >> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h
> > >> b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h index
> > >> 6988f12d89d9..619ebc1c8354 100644
> > >> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h
> > >> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h
> > >> @@ -158,6 +158,17 @@
> > >>
> > >> #define HDMI_FC_SPDDEVICEINF 0x1062
> > >> #define HDMI_FC_AUDSCONF 0x1063
> > >> #define HDMI_FC_AUDSSTAT 0x1064
> > >>
> > >> +#define HDMI_FC_AUDSV 0x1065
> > >> +#define HDMI_FC_AUDSU 0x1066
> > >> +#define HDMI_FC_AUDSCHNLS0 0x1067
> > >> +#define HDMI_FC_AUDSCHNLS1 0x1068
> > >> +#define HDMI_FC_AUDSCHNLS2 0x1069
> > >> +#define HDMI_FC_AUDSCHNLS3 0x106a
> > >> +#define HDMI_FC_AUDSCHNLS4 0x106b
> > >> +#define HDMI_FC_AUDSCHNLS5 0x106c
> > >> +#define HDMI_FC_AUDSCHNLS6 0x106d
> > >> +#define HDMI_FC_AUDSCHNLS7 0x106e
> > >> +#define HDMI_FC_AUDSCHNLS8 0x106f
> > >>
> > >> #define HDMI_FC_DATACH0FILL 0x1070
> > >> #define HDMI_FC_DATACH1FILL 0x1071
> > >> #define HDMI_FC_DATACH2FILL 0x1072
> > >>
> > >> @@ -706,6 +717,15 @@ enum {
> > >>
> > >> /* HDMI_FC_AUDSCHNLS7 field values */
> > >>
> > >> HDMI_FC_AUDSCHNLS7_ACCURACY_OFFSET = 4,
> > >> HDMI_FC_AUDSCHNLS7_ACCURACY_MASK = 0x30,
> > >>
> > >> + HDMI_FC_AUDSCHNLS7_SMPRATE_MASK = 0x0f,
> > >> + HDMI_FC_AUDSCHNLS7_SMPRATE_192K = 0xe,
> > >> + HDMI_FC_AUDSCHNLS7_SMPRATE_176K4 = 0xc,
> > >> + HDMI_FC_AUDSCHNLS7_SMPRATE_96K = 0xa,
> > >> + HDMI_FC_AUDSCHNLS7_SMPRATE_768K = 0x9,
> > >> + HDMI_FC_AUDSCHNLS7_SMPRATE_88K2 = 0x8,
> > >> + HDMI_FC_AUDSCHNLS7_SMPRATE_32K = 0x3,
> > >> + HDMI_FC_AUDSCHNLS7_SMPRATE_48K = 0x2,
> > >> + HDMI_FC_AUDSCHNLS7_SMPRATE_44K1 = 0x0,
> > >>
> > >> /* HDMI_FC_AUDSCHNLS8 field values */
> > >>
> > >> HDMI_FC_AUDSCHNLS8_ORIGSAMPFREQ_MASK = 0xf0,
>
>
>
>
_______________________________________________
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 1/3] perf cs-etm: Refactor instruction size handling
From: Leo Yan @ 2019-09-04 9:19 UTC (permalink / raw)
To: Mathieu Poirier
Cc: Suzuki K Poulose, Alexander Shishkin, linux-kernel,
Arnaldo Carvalho de Melo, Adrian Hunter, Namhyung Kim,
Robert Walker, Jiri Olsa, linux-arm-kernel, Mike Leach
In-Reply-To: <20190903222215.GD25787@xps15>
Hi Mathieu,
On Tue, Sep 03, 2019 at 04:22:15PM -0600, Mathieu Poirier wrote:
> On Fri, Aug 30, 2019 at 02:24:19PM +0800, Leo Yan wrote:
> > There has several code pieces need to know the instruction size, but
> > now every place calculates the instruction size separately.
> >
> > This patch refactors to create a new function cs_etm__instr_size() as
> > a central place to analyze the instruction length based on ISA type
> > and instruction value.
> >
> > Signed-off-by: Leo Yan <leo.yan@linaro.org>
> > ---
> > tools/perf/util/cs-etm.c | 44 +++++++++++++++++++++++++++-------------
> > 1 file changed, 30 insertions(+), 14 deletions(-)
> >
> > diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
> > index b3a5daaf1a8f..882a0718033d 100644
> > --- a/tools/perf/util/cs-etm.c
> > +++ b/tools/perf/util/cs-etm.c
> > @@ -914,6 +914,26 @@ static inline int cs_etm__t32_instr_size(struct cs_etm_queue *etmq,
> > return ((instrBytes[1] & 0xF8) >= 0xE8) ? 4 : 2;
> > }
> >
> > +static inline int cs_etm__instr_size(struct cs_etm_queue *etmq,
> > + u8 trace_chan_id,
> > + enum cs_etm_isa isa,
> > + u64 addr)
> > +{
> > + int insn_len;
> > +
> > + /*
> > + * T32 instruction size might be 32-bit or 16-bit, decide by calling
> > + * cs_etm__t32_instr_size().
> > + */
> > + if (isa == CS_ETM_ISA_T32)
> > + insn_len = cs_etm__t32_instr_size(etmq, trace_chan_id, addr);
> > + /* Otherwise, A64 and A32 instruction size are always 32-bit. */
> > + else
> > + insn_len = 4;
> > +
> > + return insn_len;
> > +}
> > +
> > static inline u64 cs_etm__first_executed_instr(struct cs_etm_packet *packet)
> > {
> > /* Returns 0 for the CS_ETM_DISCONTINUITY packet */
> > @@ -938,19 +958,23 @@ static inline u64 cs_etm__instr_addr(struct cs_etm_queue *etmq,
> > const struct cs_etm_packet *packet,
> > u64 offset)
> > {
> > + int insn_len;
> > +
> > if (packet->isa == CS_ETM_ISA_T32) {
> > u64 addr = packet->start_addr;
> >
> > while (offset > 0) {
> > - addr += cs_etm__t32_instr_size(etmq,
> > - trace_chan_id, addr);
> > + addr += cs_etm__instr_size(etmq, trace_chan_id,
> > + packet->isa, addr);
> > offset--;
> > }
> > return addr;
> > }
> >
> > - /* Assume a 4 byte instruction size (A32/A64) */
> > - return packet->start_addr + offset * 4;
> > + /* Return instruction size for A32/A64 */
> > + insn_len = cs_etm__instr_size(etmq, trace_chan_id,
> > + packet->isa, packet->start_addr);
> > + return packet->start_addr + offset * insn_len;
>
> This patch will work but from where I stand it makes things difficult to
> understand more than anything else. It is also adding coupling between function
> cs_etm__instr_addr() and cs_etm__instr_size(), meaning the code needs to be
> carefully inspected in order to make changes to either one.
My purpose is to use a same place to calculate the instruction
size, rather than to spread the duplicate codes in several different
functions.
> Last but not least function cs_etm__instr_size() isn't used in the upcoming
> patches. I really don't see what is gained here.
Sorry that I forgot to commit my final change into patch 02.
I planed to use cs_etm__instr_size() in patch 02; patch 02 has
function cs_etm__add_stack_event(), which also needs to get the
instruction size when it sends stack event.
After apply patch 02, tools/perf/util/cs-etm.c will have below three
functions to caculate instruction size; this is the main reason I want
to refactor the code for instruction size.
cs_etm__instr_addr()
cs_etm__copy_insn()
cs_etm__add_stack_event()
If this lets code more difficult to understand, will drop it.
Thanks,
Leo Yan
> > }
> >
> > static void cs_etm__update_last_branch_rb(struct cs_etm_queue *etmq,
> > @@ -1090,16 +1114,8 @@ static void cs_etm__copy_insn(struct cs_etm_queue *etmq,
> > return;
> > }
> >
> > - /*
> > - * T32 instruction size might be 32-bit or 16-bit, decide by calling
> > - * cs_etm__t32_instr_size().
> > - */
> > - if (packet->isa == CS_ETM_ISA_T32)
> > - sample->insn_len = cs_etm__t32_instr_size(etmq, trace_chan_id,
> > - sample->ip);
> > - /* Otherwise, A64 and A32 instruction size are always 32-bit. */
> > - else
> > - sample->insn_len = 4;
> > + sample->insn_len = cs_etm__instr_size(etmq, trace_chan_id,
> > + packet->isa, sample->ip);
> >
> > cs_etm__mem_access(etmq, trace_chan_id, sample->ip,
> > sample->insn_len, (void *)sample->insn);
> > --
> > 2.17.1
> >
_______________________________________________
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] drm: bridge/dw_hdmi: add audio sample channel status setting
From: Cheng-yi Chiang @ 2019-09-04 9:13 UTC (permalink / raw)
To: Neil Armstrong
Cc: moderated list:SOUND - SOC LAYER / DYNAMIC AUDIO POWER MANAGEM...,
kuninori.morimoto.gx, cain.cai, David Airlie, dri-devel,
linux-kernel, Andrzej Hajda, Laurent Pinchart, sam, Xing Zheng,
linux-rockchip, Dylan Reid, tzungbi, Jonas Karlman, Jeffy Chen,
蔡枫, linux-arm-kernel, Jernej Skrabec, Doug Anderson,
Daniel Vetter, Enric Balletbo i Serra, kuankuan.y
In-Reply-To: <d8a80ba5-dd2b-f84d-bbfc-9dd5ccbc26e9@baylibre.com>
On Wed, Sep 4, 2019 at 2:00 AM Neil Armstrong <narmstrong@baylibre.com> wrote:
>
> Hi,
>
> Le 03/09/2019 à 11:53, Neil Armstrong a écrit :
> > Hi,
> >
> > On 03/09/2019 07:51, Cheng-Yi Chiang wrote:
> >> From: Yakir Yang <ykk@rock-chips.com>
> >>
> >> When transmitting IEC60985 linear PCM audio, we configure the
> >> Audio Sample Channel Status information of all the channel
> >> status bits in the IEC60958 frame.
> >> Refer to 60958-3 page 10 for frequency, original frequency, and
> >> wordlength setting.
> >>
> >> This fix the issue that audio does not come out on some monitors
> >> (e.g. LG 22CV241)
> >>
> >> Signed-off-by: Yakir Yang <ykk@rock-chips.com>
> >> Signed-off-by: Cheng-Yi Chiang <cychiang@chromium.org>
> >> ---
> >> drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 59 +++++++++++++++++++++++
> >> drivers/gpu/drm/bridge/synopsys/dw-hdmi.h | 20 ++++++++
> >> 2 files changed, 79 insertions(+)
> >>
> >> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> >> index bd65d0479683..34d46e25d610 100644
> >> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> >> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> >> @@ -582,6 +582,63 @@ static unsigned int hdmi_compute_n(unsigned int freq, unsigned long pixel_clk)
> >> return n;
> >> }
> >>
> >> +static void hdmi_set_schnl(struct dw_hdmi *hdmi)
> >> +{
> >> + u8 aud_schnl_samplerate;
> >> + u8 aud_schnl_8;
> >> +
> >> + /* These registers are on RK3288 using version 2.0a. */
> >> + if (hdmi->version != 0x200a)
> >> + return;
> >
> > Are these limited to the 2.0a version *in* RK3288, or 2.0a version on all
> > SoCs ?
>
> After investigations, Amlogic sets these registers on their 2.0a version
> aswell, and Jernej (added in Cc) reported me Allwinner sets them on their
> < 2.0a and > 2.0a IPs versions.
>
> Can you check on the Rockchip IP versions in RK3399 ?
>
Sorry, the RK3399 board I am using is using DP, not HDMI.
But I found that on rockchip's tree at
https://github.com/rockchip-linux/kernel/commit/924f480383c982da9908fb96d6bbb580b25545a5#diff-f74b4cfb23436a137a9338a5af3fbb3dR172
There is such register setting, so I think RK3399 should have the same register.
> For reference, the HDMI 1.4a IP version allwinner setups is:
> https://github.com/Allwinner-Homlet/H3-BSP4.4-linux/blob/master/drivers/video/fbdev/sunxi/disp2/hdmi/hdmi_bsp_sun8iw7.c#L531-L539
> (registers a "scrambled" but a custom bit can reset to the original mapping,
> 0x1066 ... 0x106f)
I see.. so 1.4 has this register.
I can modify the check to be >= 1.4 then.
Will fix in v2.
Thanks!
>
> Neil
>
> >
> >> +
> >> + switch (hdmi->sample_rate) {
> >> + case 32000:
> >> + aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_32K;
> >> + break;
> >> + case 44100:
> >> + aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_44K1;
> >> + break;
> >> + case 48000:
> >> + aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_48K;
> >> + break;
> >> + case 88200:
> >> + aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_88K2;
> >> + break;
> >> + case 96000:
> >> + aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_96K;
> >> + break;
> >> + case 176400:
> >> + aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_176K4;
> >> + break;
> >> + case 192000:
> >> + aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_192K;
> >> + break;
> >> + case 768000:
> >> + aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_768K;
> >> + break;
> >> + default:
> >> + dev_warn(hdmi->dev, "Unsupported audio sample rate (%u)\n",
> >> + hdmi->sample_rate);
> >> + return;
> >> + }
> >> +
> >> + /* set channel status register */
> >> + hdmi_modb(hdmi, aud_schnl_samplerate, HDMI_FC_AUDSCHNLS7_SMPRATE_MASK,
> >> + HDMI_FC_AUDSCHNLS7);
> >> +
> >> + /*
> >> + * Set original frequency to be the same as frequency.
> >> + * Use one-complement value as stated in IEC60958-3 page 13.
> >> + */
> >> + aud_schnl_8 = (~aud_schnl_samplerate) <<
> >> + HDMI_FC_AUDSCHNLS8_ORIGSAMPFREQ_OFFSET;
> >> +
> >> + /* This means word length is 16 bit. Refer to IEC60958-3 page 12. */
> >> + aud_schnl_8 |= 2 << HDMI_FC_AUDSCHNLS8_WORDLEGNTH_OFFSET;
> >> +
> >> + hdmi_writeb(hdmi, aud_schnl_8, HDMI_FC_AUDSCHNLS8);
> >> +}
> >> +
> >> static void hdmi_set_clk_regenerator(struct dw_hdmi *hdmi,
> >> unsigned long pixel_clk, unsigned int sample_rate)
> >> {
> >> @@ -620,6 +677,8 @@ static void hdmi_set_clk_regenerator(struct dw_hdmi *hdmi,
> >> hdmi->audio_cts = cts;
> >> hdmi_set_cts_n(hdmi, cts, hdmi->audio_enable ? n : 0);
> >> spin_unlock_irq(&hdmi->audio_lock);
> >> +
> >> + hdmi_set_schnl(hdmi);
> >> }
> >>
> >> static void hdmi_init_clk_regenerator(struct dw_hdmi *hdmi)
> >> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h
> >> index 6988f12d89d9..619ebc1c8354 100644
> >> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h
> >> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h
> >> @@ -158,6 +158,17 @@
> >> #define HDMI_FC_SPDDEVICEINF 0x1062
> >> #define HDMI_FC_AUDSCONF 0x1063
> >> #define HDMI_FC_AUDSSTAT 0x1064
> >> +#define HDMI_FC_AUDSV 0x1065
> >> +#define HDMI_FC_AUDSU 0x1066
> >> +#define HDMI_FC_AUDSCHNLS0 0x1067
> >> +#define HDMI_FC_AUDSCHNLS1 0x1068
> >> +#define HDMI_FC_AUDSCHNLS2 0x1069
> >> +#define HDMI_FC_AUDSCHNLS3 0x106a
> >> +#define HDMI_FC_AUDSCHNLS4 0x106b
> >> +#define HDMI_FC_AUDSCHNLS5 0x106c
> >> +#define HDMI_FC_AUDSCHNLS6 0x106d
> >> +#define HDMI_FC_AUDSCHNLS7 0x106e
> >> +#define HDMI_FC_AUDSCHNLS8 0x106f
> >> #define HDMI_FC_DATACH0FILL 0x1070
> >> #define HDMI_FC_DATACH1FILL 0x1071
> >> #define HDMI_FC_DATACH2FILL 0x1072
> >> @@ -706,6 +717,15 @@ enum {
> >> /* HDMI_FC_AUDSCHNLS7 field values */
> >> HDMI_FC_AUDSCHNLS7_ACCURACY_OFFSET = 4,
> >> HDMI_FC_AUDSCHNLS7_ACCURACY_MASK = 0x30,
> >> + HDMI_FC_AUDSCHNLS7_SMPRATE_MASK = 0x0f,
> >> + HDMI_FC_AUDSCHNLS7_SMPRATE_192K = 0xe,
> >> + HDMI_FC_AUDSCHNLS7_SMPRATE_176K4 = 0xc,
> >> + HDMI_FC_AUDSCHNLS7_SMPRATE_96K = 0xa,
> >> + HDMI_FC_AUDSCHNLS7_SMPRATE_768K = 0x9,
> >> + HDMI_FC_AUDSCHNLS7_SMPRATE_88K2 = 0x8,
> >> + HDMI_FC_AUDSCHNLS7_SMPRATE_32K = 0x3,
> >> + HDMI_FC_AUDSCHNLS7_SMPRATE_48K = 0x2,
> >> + HDMI_FC_AUDSCHNLS7_SMPRATE_44K1 = 0x0,
> >>
> >> /* HDMI_FC_AUDSCHNLS8 field values */
> >> HDMI_FC_AUDSCHNLS8_ORIGSAMPFREQ_MASK = 0xf0,
> >>
> >
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [RFC PATCH V2 4/4] platform: mtk-isp: Add Mediatek FD driver
From: Tomasz Figa @ 2019-09-04 9:03 UTC (permalink / raw)
To: Jerry-ch Chen
Cc: devicetree@vger.kernel.org, Sean Cheng (鄭昇弘),
laurent.pinchart+renesas@ideasonboard.com,
Rynn Wu (吳育恩), srv_heupstream,
Po-Yang Huang (黃柏陽), mchehab@kernel.org,
suleiman@chromium.org, shik@chromium.org,
Jungo Lin (林明俊),
Sj Huang (黃信璋), yuzhao@chromium.org,
linux-mediatek@lists.infradead.org, zwisler@chromium.org,
matthias.bgg@gmail.com, Christie Yu (游雅惠),
Frederic Chen (陳俊元), hans.verkuil@cisco.com,
linux-arm-kernel@lists.infradead.org, linux-media@vger.kernel.org
In-Reply-To: <1567587708.22453.15.camel@mtksdccf07>
On Wed, Sep 4, 2019 at 6:02 PM Jerry-ch Chen <Jerry-ch.Chen@mediatek.com> wrote:
>
> Hi Tomasz,
>
> On Wed, 2019-09-04 at 16:25 +0800, Tomasz Figa wrote:
> > On Wed, Sep 4, 2019 at 5:09 PM Jerry-ch Chen <Jerry-ch.Chen@mediatek.com> wrote:
> > >
> > > Hi Tomasz,
> > >
> > > On Wed, 2019-09-04 at 14:34 +0800, Tomasz Figa wrote:
> > > > On Wed, Sep 4, 2019 at 3:09 PM Jerry-ch Chen <Jerry-ch.Chen@mediatek.com> wrote:
> > > > >
> > > > > Hi Tomasz,
> > > > >
> > > > > On Wed, 2019-09-04 at 12:15 +0800, Tomasz Figa wrote:
> > > > > > On Wed, Sep 4, 2019 at 12:38 PM Jerry-ch Chen
> > > > > > <Jerry-ch.Chen@mediatek.com> wrote:
> > > > > > >
> > > > > > > Hi Tomasz,
> > > > > > >
> > > > > > > On Tue, 2019-09-03 at 20:05 +0800, Tomasz Figa wrote:
> > > > > > > > On Tue, Sep 3, 2019 at 8:46 PM Jerry-ch Chen <Jerry-ch.Chen@mediatek.com> wrote:
> > > > > > > > >
> > > > > > > > > Hi Tomasz,
> > > > > > > > >
> > > > > > > > > On Tue, 2019-09-03 at 15:04 +0800, Tomasz Figa wrote:
> > > > > > > > > > On Tue, Sep 3, 2019 at 3:44 PM Jerry-ch Chen <Jerry-ch.Chen@mediatek.com> wrote:
> > > > > > > > > > >
> > > > > > > > > > > On Tue, 2019-09-03 at 13:19 +0800, Tomasz Figa wrote:
> > > > > > > > > > > > On Mon, Sep 2, 2019 at 8:47 PM Jerry-ch Chen <Jerry-ch.Chen@mediatek.com> wrote:
> > > > > > > > > > > > >
> > > > > > > > > > > > > Hi Tomasz,
> > > > > > > > > > > > >
> > > > > > > > > > > > > On Fri, 2019-08-30 at 16:33 +0800, Tomasz Figa wrote:
> > > > > > > > > > > > > > On Wed, Aug 28, 2019 at 11:00 AM Jerry-ch Chen
> > > > > > > > > > > > > > <Jerry-ch.Chen@mediatek.com> wrote:
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > Hi Tomasz,
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > On Mon, 2019-08-26 at 14:36 +0800, Tomasz Figa wrote:
> > > > > > > > > > > > > > > > Hi Jerry,
> > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > On Sun, Aug 25, 2019 at 6:18 PM Jerry-ch Chen
> > > > > > > > > > > > > > > > <Jerry-ch.Chen@mediatek.com> wrote:
> > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > Hi Tomasz,
> > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > On Fri, 2019-08-02 at 16:28 +0800, Tomasz Figa wrote:
> > > > > > > > > > > > > > > > > > Hi Jerry,
> > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > On Tue, Jul 09, 2019 at 04:41:12PM +0800, Jerry-ch Chen wrote:
> > > > > > > [snip]
> > > > > > > > > > > > > > > > [snip]
> > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > +static void mtk_fd_vb2_stop_streaming(struct vb2_queue *vq)
> > > > > > > > > > > > > > > > > > > +{
> > > > > > > > > > > > > > > > > > > + struct mtk_fd_ctx *ctx = vb2_get_drv_priv(vq);
> > > > > > > > > > > > > > > > > > > + struct vb2_buffer *vb;
> > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > How do we guarantee here that the hardware isn't still accessing the buffers
> > > > > > > > > > > > > > > > > > removed below?
> > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > Maybe we can check the driver state flag and aborting the unfinished
> > > > > > > > > > > > > > > > > jobs?
> > > > > > > > > > > > > > > > > (fd_hw->state == FD_ENQ)
> > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > Yes, we need to either cancel or wait for the currently processing
> > > > > > > > > > > > > > > > job. It depends on hardware capabilities, but cancelling is generally
> > > > > > > > > > > > > > > > preferred for the lower latency.
> > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > Ok, it the state is ENQ, then we can disable the FD hw by controlling
> > > > > > > > > > > > > > > the registers.
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > for example:
> > > > > > > > > > > > > > > writel(0x0, fd->fd_base + FD_HW_ENABLE);
> > > > > > > > > > > > > > > writel(0x0, fd->fd_base + FD_INT_EN);
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > What's exactly the effect of writing 0 to FD_HW_ENABLE?
> > > > > > > > > > > > > >
> > > > > > > > > > > > > Sorry, my last reply didn't solve the question,
> > > > > > > > > > > > > we should implement a mtk_fd_job_abort() for v4l2_m2m_ops().
> > > > > > > > > > > > >
> > > > > > > > > > > > > which is able to readl_poll_timeout_atomic()
> > > > > > > > > > > > > and check the HW busy bits in the register FD_INT_EN;
> > > > > > > > > > > > >
> > > > > > > > > > > > > if they are not cleared until timeout, we could handle the last
> > > > > > > > > > > > > processing job.
> > > > > > > > > > > > > Otherwise, the FD irq handler should have handled the last processing
> > > > > > > > > > > > > job and we could continue the stop_streaming().
> > > > > > > > > > > > >
> > > > > > > > > > > > > For job_abort():
> > > > > > > > > > > > > static void mtk_fd_job_abort(void *priv)
> > > > > > > > > > > > > {
> > > > > > > > > > > > > struct mtk_fd_ctx *ctx = priv;
> > > > > > > > > > > > > struct mtk_fd_dev *fd = ctx->fd_dev;
> > > > > > > > > > > > > u32 val;
> > > > > > > > > > > > > u32 ret;
> > > > > > > > > > > > >
> > > > > > > > > > > > > ret = readl_poll_timeout_atomic(fd->fd_base + MTK_FD_REG_OFFSET_INT_EN,
> > > > > > > > > > > > > val,
> > > > > > > > > > > > > (val & MTK_FD_HW_BUSY_MASK) ==
> > > > > > > > > > > > > MTK_FD_HW_STATE_IS_BUSY,
> > > > > > > > > > > > > USEC_PER_MSEC, MTK_FD_STOP_HW_TIMEOUT);
> > > > > > > > > > > >
> > > > > > > > > > > > Hmm, would it be possible to avoid the busy wait by having a
> > > > > > > > > > > > completion that could be signalled from the interrupt handler?
> > > > > > > > > > > >
> > > > > > > > > > > > Best regards,
> > > > > > > > > > > > Tomasz
> > > > > > > > > > >
> > > > > > > > > > > I suppose that would be wakeup a wait queue in the interrupt handler,
> > > > > > > > > > > the the wait_event_interrupt_timeout() will be used in here and system
> > > > > > > > > > > suspend e.g. mtk_fd_suspend().
> > > > > > > > > >
> > > > > > > > > > Yes, that should work.
> > > > > > > > > >
> > > > > > > > > > > Or do you suggest to wait_event_interrupt_timeout() every frame in the
> > > > > > > > > > > mtk_fd_ipi_handler()?
> > > > > > > > > >
> > > > > > > > > > Nope, we shouldn't need that.
> > > > > > > > > >
> > > > > > > > > > > I think maybe the readl_poll_timeout_atomic would be good enough.
> > > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > Not really. Busy waiting should be avoided as much as possible. What's
> > > > > > > > > > the point of entering suspend if you end up burning the power by
> > > > > > > > > > spinning the CPU for some milliseconds?
> > > > > > > > > >
> > > > > > > > > Ok, I see, busy waiting is not a good idea, I will use the wait queue
> > > > > > > > > instead. the job abort will refine as following:
> > > > > > > > >
> > > > > > > > > static void mtk_fd_job_abort(void *priv)
> > > > > > > > > {
> > > > > > > > > struct mtk_fd_ctx *ctx = priv;
> > > > > > > > > struct mtk_fd_dev *fd = ctx->fd_dev;
> > > > > > > > > u32 ret;
> > > > > > > > >
> > > > > > > > > ret = wait_event_interruptible_timeout
> > > > > > > > > (fd->wq, (fd->fd_irq_result & MTK_FD_HW_IRQ_MASK),
> > > > > > > > > usecs_to_jiffies(50000));
> > > > > > > > > if (ret)
> > > > > > > > > mtk_fd_hw_job_finish(fd, VB2_BUF_STATE_ERROR);
> > > > > > > > > dev_dbg(fd->dev, "%s, ret:%d\n", __func__, ret);
> > > > > > > > >
> > > > > > > > > fd->fd_irq_result = 0;
> > > > > > > > > }
> > > > > > > > >
> > > > > > > > > static struct v4l2_m2m_ops fd_m2m_ops = {
> > > > > > > > > .device_run = mtk_fd_device_run,
> > > > > > > > > .job_abort = mtk_fd_job_abort,
> > > > > > > >
> > > > > > > > I'm not sure we should be using the functon above as the .job_abort
> > > > > > > > callback. It's expected to abort the job, but we're just waiting for
> > > > > > > > it to finish. I think we should just call mtk_fd_job_abort() manually
> > > > > > > > from .stop_streaming.
> > > > > > > >
> > > > > > >
> > > > > > > Ok, I will fix it.
> > > > > > >
> > > > > > > > > };
> > > > > > > > >
> > > > > > > > > and we could use it in suspend.
> > > > > > > > > static int mtk_fd_suspend(struct device *dev)
> > > > > > > > > {
> > > > > > > > > struct mtk_fd_dev *fd = dev_get_drvdata(dev);
> > > > > > > > >
> > > > > > > > > if (pm_runtime_suspended(dev))
> > > > > > > > > return 0;
> > > > > > > > >
> > > > > > > > > if (fd->fd_stream_count)
> > > > > > > > > mtk_fd_job_abort(fd->ctx);
> > > > > > > > >
> > > > > > > > > /* suspend FD HW */
> > > > > > > > > writel(0x0, fd->fd_base + MTK_FD_REG_OFFSET_INT_EN);
> > > > > > > > > writel(0x0, fd->fd_base + MTK_FD_REG_OFFSET_HW_ENABLE);
> > > > > > > > > clk_disable_unprepare(fd->fd_clk);
> > > > > > > > > dev_dbg(dev, "%s:disable clock\n", __func__);
> > > > > > > > >
> > > > > > > > > return 0;
> > > > > > > > > }
> > > > > > > > >
> > > > > > > > > static irqreturn_t mtk_fd_irq(int irq, void *data)
> > > > > > > > > {
> > > > > > > > > struct mtk_fd_dev *fd = (struct mtk_fd_dev *)data;
> > > > > > > > >
> > > > > > > > > fd->fd_irq_result = readl(fd->fd_base + MTK_FD_REG_OFFSET_INT_VAL);
> > > > > > > > > wake_up_interruptible(&fd->wq);
> > > > > > > >
> > > > > > > > The wake up should be done at the very end of this function. Otherwise
> > > > > > > > we end up with m2m framework racing with the mtk_fd_hw_job_finish()
> > > > > > > > below.
> > > > > > > >
> > > > > > > > Also, I'd use a completion here rather than an open coded wait and
> > > > > > > > wake-up. The driver could reinit_completion() before queuing a job to
> > > > > > > > the hardware and the IRQ handler would complete(). There would be no
> > > > > > > > need to store the IRQ flags in driver data anymore.
> > > > > > > Ok, It will be refined as following:
> > > > > > >
> > > > > > > suspend and stop streaming will call mtk_fd_job_abort()
> > > > > > >
> > > > > > > static void mtk_fd_job_abort(struct mtk_fd_dev *fd)
> > > > > > > {
> > > > > > > u32 ret;
> > > > > > >
> > > > > > > ret = wait_for_completion_timeout(&fd->fd_irq_done,
> > > > > > > msecs_to_jiffies(MTK_FD_HW_TIMEOUT));
> > > > > > > if (ret)
> > > > > >
> > > > > > For the _timeout variants, !ret means the timeout and ret > 0 means
> > > > > > that the wait ended successfully before.
> > > > > >
> > > > > Thanks, fixed.
> > > > >
> > > > > > Also please make sure that the timeout is big enough not to happen
> > > > > > normally on a heavily loaded system. Something like 1 second should be
> > > > > > good.
> > > > > >
> > > > > Ok, I will make it 1 second
> > > > > #define MTK_FD_HW_TIMEOUT 1000
> > > > >
> > > > > Also, I will add a condition in mtk_fd_vb2_stop_streaming(), since it
> > > > > would be called twice, now it works fine whether I reuse the condition
> > > > > with mtk_fd_hw_disconnect or not, however, I think do it before buffer
> > > > > remove looks more reasonable.
> > > > >
> > > > > static void mtk_fd_vb2_stop_streaming(struct vb2_queue *vq)
> > > > > {
> > > > > struct mtk_fd_ctx *ctx = vb2_get_drv_priv(vq);
> > > > > struct mtk_fd_dev *fd = ctx->fd_dev;
> > > > > struct vb2_v4l2_buffer *vb;
> > > > > struct v4l2_m2m_ctx *m2m_ctx = ctx->fh.m2m_ctx;
> > > > > struct v4l2_m2m_queue_ctx *queue_ctx;
> > > > >
> > > > > if (vq->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE)
> > > > > mtk_fd_job_abort(fd);
> > > >
> > > > We need to do it regardless of the queue type, otherwise we could
> > > > still free CAPTURE buffers before the hardware releases them.
> > > >
> > >
> > > I think we may read the fd->fd_irq_done.done and do wait for completion
> > > if it's not being done.
> > > How do you think?
> > >
> > > static void mtk_fd_vb2_stop_streaming(struct vb2_queue *vq)
> > > {
> > > struct mtk_fd_ctx *ctx = vb2_get_drv_priv(vq);
> > > struct mtk_fd_dev *fd = ctx->fd_dev;
> > > struct vb2_v4l2_buffer *vb;
> > > struct v4l2_m2m_ctx *m2m_ctx = ctx->fh.m2m_ctx;
> > > struct v4l2_m2m_queue_ctx *queue_ctx;
> > > u32 ret;
> > >
> > > if (!fd->fd_irq_done.done)
> >
> > We shouldn't access internal fields of completion.
> >
> > > ret = wait_for_completion_timeout(&fd->fd_irq_done,
> > > msecs_to_jiffies(
> > > MTK_FD_HW_TIMEOUT));
> > > queue_ctx = V4L2_TYPE_IS_OUTPUT(vq->type) ?
> > > &m2m_ctx->out_q_ctx :
> > > &m2m_ctx->cap_q_ctx;
> > > while ((vb = v4l2_m2m_buf_remove(queue_ctx)))
> > > v4l2_m2m_buf_done(vb, VB2_BUF_STATE_ERROR);
> > >
> > > if (vq->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE)
> > > mtk_fd_hw_disconnect(fd);
> > > }
> > >
> > > I've also tried to wait completion unconditionally for both queues and
> > > the second time will wait until timeout, as a result, it takes longer to
> > > swap the camera every time and close the camera app.
> >
> > I think it should work better if we call complete_all() instead of complete().
> >
> Thanks,
>
> I use complete_all(), and it works fine now.
>
> static void mtk_fd_vb2_stop_streaming(struct vb2_queue *vq)
> {
> struct mtk_fd_ctx *ctx = vb2_get_drv_priv(vq);
> struct mtk_fd_dev *fd = ctx->fd_dev;
> struct vb2_v4l2_buffer *vb;
> struct v4l2_m2m_ctx *m2m_ctx = ctx->fh.m2m_ctx;
> struct v4l2_m2m_queue_ctx *queue_ctx;
>
> wait_for_completion_timeout(&fd->fd_irq_done,
> msecs_to_jiffies(MTK_FD_HW_TIMEOUT));
Shouldn't we still send some command to the hardware to stop? Like a
reset. Otherwise we don't know if it isn't still accessing the memory.
> queue_ctx = V4L2_TYPE_IS_OUTPUT(vq->type) ?
> &m2m_ctx->out_q_ctx :
> &m2m_ctx->cap_q_ctx;
> while ((vb = v4l2_m2m_buf_remove(queue_ctx)))
> v4l2_m2m_buf_done(vb, VB2_BUF_STATE_ERROR);
>
> if (vq->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE)
> mtk_fd_hw_disconnect(fd);
> }
>
> Best regards,
> Jerry
>
> > Best regards,
> > Tomasz
>
>
_______________________________________________
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] drm: bridge/dw_hdmi: add audio sample channel status setting
From: Cheng-yi Chiang @ 2019-09-04 9:09 UTC (permalink / raw)
To: Neil Armstrong
Cc: moderated list:SOUND - SOC LAYER / DYNAMIC AUDIO POWER MANAGEM...,
tzungbi, kuninori.morimoto.gx, Xing Zheng, cain.cai, David Airlie,
sam, Jeffy Chen, linux-kernel, dri-devel, Doug Anderson,
Andrzej Hajda, 蔡枫, Laurent Pinchart, Daniel Vetter,
Yakir Yang, Enric Balletbo i Serra, linux-rockchip, Dylan Reid,
kuankuan.y, linux-arm-kernel
In-Reply-To: <e1c3483c-baa6-c726-e547-fadf40d259f4@baylibre.com>
Hi,
On Tue, Sep 3, 2019 at 5:53 PM Neil Armstrong <narmstrong@baylibre.com> wrote:
>
> Hi,
>
> On 03/09/2019 07:51, Cheng-Yi Chiang wrote:
> > From: Yakir Yang <ykk@rock-chips.com>
> >
> > When transmitting IEC60985 linear PCM audio, we configure the
> > Audio Sample Channel Status information of all the channel
> > status bits in the IEC60958 frame.
> > Refer to 60958-3 page 10 for frequency, original frequency, and
> > wordlength setting.
> >
> > This fix the issue that audio does not come out on some monitors
> > (e.g. LG 22CV241)
> >
> > Signed-off-by: Yakir Yang <ykk@rock-chips.com>
> > Signed-off-by: Cheng-Yi Chiang <cychiang@chromium.org>
> > ---
> > drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 59 +++++++++++++++++++++++
> > drivers/gpu/drm/bridge/synopsys/dw-hdmi.h | 20 ++++++++
> > 2 files changed, 79 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> > index bd65d0479683..34d46e25d610 100644
> > --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> > +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> > @@ -582,6 +582,63 @@ static unsigned int hdmi_compute_n(unsigned int freq, unsigned long pixel_clk)
> > return n;
> > }
> >
> > +static void hdmi_set_schnl(struct dw_hdmi *hdmi)
> > +{
> > + u8 aud_schnl_samplerate;
> > + u8 aud_schnl_8;
> > +
> > + /* These registers are on RK3288 using version 2.0a. */
> > + if (hdmi->version != 0x200a)
> > + return;
>
> Are these limited to the 2.0a version *in* RK3288, or 2.0a version on all
> SoCs ?
>
In the original patch by Yakir,
https://lore.kernel.org/patchwork/patch/539653/ (sorry, I should
have added this link in the "after the cut" note)
The fix is limited to version 2.0.
Since I am only testing on RK3288 with 2.0a, I change the check to 2.0a only.
I can not test 2.0a version on other SoCs.
The databook I have at hand is 2.0a (not specific to RK3288) so I
think all 2.0a should have this register.
As for other version like version 1.3 on iMX6, there is no such
register, as stated by Russell
http://lkml.iu.edu/hypermail/linux/kernel/1501.3/06268.html.
So at least we should check the version.
Maybe we can set the criteria as version 2.0 or above to make it a safe patch ?
If there is the same need on other SoC with version < 2.0, it can be
added later.
Presumably, there will be databook of that version to help confirming
this setting.
Thanks!
> > +
> > + switch (hdmi->sample_rate) {
> > + case 32000:
> > + aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_32K;
> > + break;
> > + case 44100:
> > + aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_44K1;
> > + break;
> > + case 48000:
> > + aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_48K;
> > + break;
> > + case 88200:
> > + aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_88K2;
> > + break;
> > + case 96000:
> > + aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_96K;
> > + break;
> > + case 176400:
> > + aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_176K4;
> > + break;
> > + case 192000:
> > + aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_192K;
> > + break;
> > + case 768000:
> > + aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_768K;
> > + break;
> > + default:
> > + dev_warn(hdmi->dev, "Unsupported audio sample rate (%u)\n",
> > + hdmi->sample_rate);
> > + return;
> > + }
> > +
> > + /* set channel status register */
> > + hdmi_modb(hdmi, aud_schnl_samplerate, HDMI_FC_AUDSCHNLS7_SMPRATE_MASK,
> > + HDMI_FC_AUDSCHNLS7);
> > +
> > + /*
> > + * Set original frequency to be the same as frequency.
> > + * Use one-complement value as stated in IEC60958-3 page 13.
> > + */
> > + aud_schnl_8 = (~aud_schnl_samplerate) <<
> > + HDMI_FC_AUDSCHNLS8_ORIGSAMPFREQ_OFFSET;
> > +
> > + /* This means word length is 16 bit. Refer to IEC60958-3 page 12. */
> > + aud_schnl_8 |= 2 << HDMI_FC_AUDSCHNLS8_WORDLEGNTH_OFFSET;
> > +
> > + hdmi_writeb(hdmi, aud_schnl_8, HDMI_FC_AUDSCHNLS8);
> > +}
> > +
> > static void hdmi_set_clk_regenerator(struct dw_hdmi *hdmi,
> > unsigned long pixel_clk, unsigned int sample_rate)
> > {
> > @@ -620,6 +677,8 @@ static void hdmi_set_clk_regenerator(struct dw_hdmi *hdmi,
> > hdmi->audio_cts = cts;
> > hdmi_set_cts_n(hdmi, cts, hdmi->audio_enable ? n : 0);
> > spin_unlock_irq(&hdmi->audio_lock);
> > +
> > + hdmi_set_schnl(hdmi);
> > }
> >
> > static void hdmi_init_clk_regenerator(struct dw_hdmi *hdmi)
> > diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h
> > index 6988f12d89d9..619ebc1c8354 100644
> > --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h
> > +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h
> > @@ -158,6 +158,17 @@
> > #define HDMI_FC_SPDDEVICEINF 0x1062
> > #define HDMI_FC_AUDSCONF 0x1063
> > #define HDMI_FC_AUDSSTAT 0x1064
> > +#define HDMI_FC_AUDSV 0x1065
> > +#define HDMI_FC_AUDSU 0x1066
> > +#define HDMI_FC_AUDSCHNLS0 0x1067
> > +#define HDMI_FC_AUDSCHNLS1 0x1068
> > +#define HDMI_FC_AUDSCHNLS2 0x1069
> > +#define HDMI_FC_AUDSCHNLS3 0x106a
> > +#define HDMI_FC_AUDSCHNLS4 0x106b
> > +#define HDMI_FC_AUDSCHNLS5 0x106c
> > +#define HDMI_FC_AUDSCHNLS6 0x106d
> > +#define HDMI_FC_AUDSCHNLS7 0x106e
> > +#define HDMI_FC_AUDSCHNLS8 0x106f
> > #define HDMI_FC_DATACH0FILL 0x1070
> > #define HDMI_FC_DATACH1FILL 0x1071
> > #define HDMI_FC_DATACH2FILL 0x1072
> > @@ -706,6 +717,15 @@ enum {
> > /* HDMI_FC_AUDSCHNLS7 field values */
> > HDMI_FC_AUDSCHNLS7_ACCURACY_OFFSET = 4,
> > HDMI_FC_AUDSCHNLS7_ACCURACY_MASK = 0x30,
> > + HDMI_FC_AUDSCHNLS7_SMPRATE_MASK = 0x0f,
> > + HDMI_FC_AUDSCHNLS7_SMPRATE_192K = 0xe,
> > + HDMI_FC_AUDSCHNLS7_SMPRATE_176K4 = 0xc,
> > + HDMI_FC_AUDSCHNLS7_SMPRATE_96K = 0xa,
> > + HDMI_FC_AUDSCHNLS7_SMPRATE_768K = 0x9,
> > + HDMI_FC_AUDSCHNLS7_SMPRATE_88K2 = 0x8,
> > + HDMI_FC_AUDSCHNLS7_SMPRATE_32K = 0x3,
> > + HDMI_FC_AUDSCHNLS7_SMPRATE_48K = 0x2,
> > + HDMI_FC_AUDSCHNLS7_SMPRATE_44K1 = 0x0,
> >
> > /* HDMI_FC_AUDSCHNLS8 field values */
> > HDMI_FC_AUDSCHNLS8_ORIGSAMPFREQ_MASK = 0xf0,
> >
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH -next] usb: gadget: at91_udc: use devm_platform_ioremap_resource() to simplify code
From: YueHaibing @ 2019-09-04 9:02 UTC (permalink / raw)
To: balbi, gregkh, nicolas.ferre, alexandre.belloni,
ludovic.desroches, hariprasad.kelam, yuehaibing
Cc: linux-usb, linux-kernel, linux-arm-kernel
Use devm_platform_ioremap_resource() to simplify the code a bit.
This is detected by coccinelle.
Reported-by: Hulk Robot <hulkci@huawei.com>
Signed-off-by: YueHaibing <yuehaibing@huawei.com>
---
drivers/usb/gadget/udc/at91_udc.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/usb/gadget/udc/at91_udc.c b/drivers/usb/gadget/udc/at91_udc.c
index 194ffb1..1b2b548 100644
--- a/drivers/usb/gadget/udc/at91_udc.c
+++ b/drivers/usb/gadget/udc/at91_udc.c
@@ -1808,7 +1808,6 @@ static int at91udc_probe(struct platform_device *pdev)
struct device *dev = &pdev->dev;
struct at91_udc *udc;
int retval;
- struct resource *res;
struct at91_ep *ep;
int i;
@@ -1839,8 +1838,7 @@ static int at91udc_probe(struct platform_device *pdev)
ep->is_pingpong = 1;
}
- res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- udc->udp_baseaddr = devm_ioremap_resource(dev, res);
+ udc->udp_baseaddr = devm_platform_ioremap_resource(pdev, 0);
if (IS_ERR(udc->udp_baseaddr))
return PTR_ERR(udc->udp_baseaddr);
--
2.7.4
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* Re: [RFC PATCH V2 4/4] platform: mtk-isp: Add Mediatek FD driver
From: Jerry-ch Chen @ 2019-09-04 9:01 UTC (permalink / raw)
To: Tomasz Figa
Cc: devicetree@vger.kernel.org, Sean Cheng (鄭昇弘),
laurent.pinchart+renesas@ideasonboard.com,
Rynn Wu (吳育恩), srv_heupstream,
Po-Yang Huang (黃柏陽), mchehab@kernel.org,
suleiman@chromium.org, shik@chromium.org,
Jungo Lin (林明俊),
Sj Huang (黃信璋), yuzhao@chromium.org,
linux-mediatek@lists.infradead.org, zwisler@chromium.org,
matthias.bgg@gmail.com, Christie Yu (游雅惠),
Frederic Chen (陳俊元), hans.verkuil@cisco.com,
linux-arm-kernel@lists.infradead.org, linux-media@vger.kernel.org
In-Reply-To: <CAAFQd5Dzxy10g-MKHMnNbVO6kp9_L_jm1m+gtN+p=YF2LyBiag@mail.gmail.com>
Hi Tomasz,
On Wed, 2019-09-04 at 16:25 +0800, Tomasz Figa wrote:
> On Wed, Sep 4, 2019 at 5:09 PM Jerry-ch Chen <Jerry-ch.Chen@mediatek.com> wrote:
> >
> > Hi Tomasz,
> >
> > On Wed, 2019-09-04 at 14:34 +0800, Tomasz Figa wrote:
> > > On Wed, Sep 4, 2019 at 3:09 PM Jerry-ch Chen <Jerry-ch.Chen@mediatek.com> wrote:
> > > >
> > > > Hi Tomasz,
> > > >
> > > > On Wed, 2019-09-04 at 12:15 +0800, Tomasz Figa wrote:
> > > > > On Wed, Sep 4, 2019 at 12:38 PM Jerry-ch Chen
> > > > > <Jerry-ch.Chen@mediatek.com> wrote:
> > > > > >
> > > > > > Hi Tomasz,
> > > > > >
> > > > > > On Tue, 2019-09-03 at 20:05 +0800, Tomasz Figa wrote:
> > > > > > > On Tue, Sep 3, 2019 at 8:46 PM Jerry-ch Chen <Jerry-ch.Chen@mediatek.com> wrote:
> > > > > > > >
> > > > > > > > Hi Tomasz,
> > > > > > > >
> > > > > > > > On Tue, 2019-09-03 at 15:04 +0800, Tomasz Figa wrote:
> > > > > > > > > On Tue, Sep 3, 2019 at 3:44 PM Jerry-ch Chen <Jerry-ch.Chen@mediatek.com> wrote:
> > > > > > > > > >
> > > > > > > > > > On Tue, 2019-09-03 at 13:19 +0800, Tomasz Figa wrote:
> > > > > > > > > > > On Mon, Sep 2, 2019 at 8:47 PM Jerry-ch Chen <Jerry-ch.Chen@mediatek.com> wrote:
> > > > > > > > > > > >
> > > > > > > > > > > > Hi Tomasz,
> > > > > > > > > > > >
> > > > > > > > > > > > On Fri, 2019-08-30 at 16:33 +0800, Tomasz Figa wrote:
> > > > > > > > > > > > > On Wed, Aug 28, 2019 at 11:00 AM Jerry-ch Chen
> > > > > > > > > > > > > <Jerry-ch.Chen@mediatek.com> wrote:
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > Hi Tomasz,
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > On Mon, 2019-08-26 at 14:36 +0800, Tomasz Figa wrote:
> > > > > > > > > > > > > > > Hi Jerry,
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > On Sun, Aug 25, 2019 at 6:18 PM Jerry-ch Chen
> > > > > > > > > > > > > > > <Jerry-ch.Chen@mediatek.com> wrote:
> > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > Hi Tomasz,
> > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > On Fri, 2019-08-02 at 16:28 +0800, Tomasz Figa wrote:
> > > > > > > > > > > > > > > > > Hi Jerry,
> > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > On Tue, Jul 09, 2019 at 04:41:12PM +0800, Jerry-ch Chen wrote:
> > > > > > [snip]
> > > > > > > > > > > > > > > [snip]
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > +static void mtk_fd_vb2_stop_streaming(struct vb2_queue *vq)
> > > > > > > > > > > > > > > > > > +{
> > > > > > > > > > > > > > > > > > + struct mtk_fd_ctx *ctx = vb2_get_drv_priv(vq);
> > > > > > > > > > > > > > > > > > + struct vb2_buffer *vb;
> > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > How do we guarantee here that the hardware isn't still accessing the buffers
> > > > > > > > > > > > > > > > > removed below?
> > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > Maybe we can check the driver state flag and aborting the unfinished
> > > > > > > > > > > > > > > > jobs?
> > > > > > > > > > > > > > > > (fd_hw->state == FD_ENQ)
> > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > Yes, we need to either cancel or wait for the currently processing
> > > > > > > > > > > > > > > job. It depends on hardware capabilities, but cancelling is generally
> > > > > > > > > > > > > > > preferred for the lower latency.
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > Ok, it the state is ENQ, then we can disable the FD hw by controlling
> > > > > > > > > > > > > > the registers.
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > for example:
> > > > > > > > > > > > > > writel(0x0, fd->fd_base + FD_HW_ENABLE);
> > > > > > > > > > > > > > writel(0x0, fd->fd_base + FD_INT_EN);
> > > > > > > > > > > > > >
> > > > > > > > > > > > >
> > > > > > > > > > > > > What's exactly the effect of writing 0 to FD_HW_ENABLE?
> > > > > > > > > > > > >
> > > > > > > > > > > > Sorry, my last reply didn't solve the question,
> > > > > > > > > > > > we should implement a mtk_fd_job_abort() for v4l2_m2m_ops().
> > > > > > > > > > > >
> > > > > > > > > > > > which is able to readl_poll_timeout_atomic()
> > > > > > > > > > > > and check the HW busy bits in the register FD_INT_EN;
> > > > > > > > > > > >
> > > > > > > > > > > > if they are not cleared until timeout, we could handle the last
> > > > > > > > > > > > processing job.
> > > > > > > > > > > > Otherwise, the FD irq handler should have handled the last processing
> > > > > > > > > > > > job and we could continue the stop_streaming().
> > > > > > > > > > > >
> > > > > > > > > > > > For job_abort():
> > > > > > > > > > > > static void mtk_fd_job_abort(void *priv)
> > > > > > > > > > > > {
> > > > > > > > > > > > struct mtk_fd_ctx *ctx = priv;
> > > > > > > > > > > > struct mtk_fd_dev *fd = ctx->fd_dev;
> > > > > > > > > > > > u32 val;
> > > > > > > > > > > > u32 ret;
> > > > > > > > > > > >
> > > > > > > > > > > > ret = readl_poll_timeout_atomic(fd->fd_base + MTK_FD_REG_OFFSET_INT_EN,
> > > > > > > > > > > > val,
> > > > > > > > > > > > (val & MTK_FD_HW_BUSY_MASK) ==
> > > > > > > > > > > > MTK_FD_HW_STATE_IS_BUSY,
> > > > > > > > > > > > USEC_PER_MSEC, MTK_FD_STOP_HW_TIMEOUT);
> > > > > > > > > > >
> > > > > > > > > > > Hmm, would it be possible to avoid the busy wait by having a
> > > > > > > > > > > completion that could be signalled from the interrupt handler?
> > > > > > > > > > >
> > > > > > > > > > > Best regards,
> > > > > > > > > > > Tomasz
> > > > > > > > > >
> > > > > > > > > > I suppose that would be wakeup a wait queue in the interrupt handler,
> > > > > > > > > > the the wait_event_interrupt_timeout() will be used in here and system
> > > > > > > > > > suspend e.g. mtk_fd_suspend().
> > > > > > > > >
> > > > > > > > > Yes, that should work.
> > > > > > > > >
> > > > > > > > > > Or do you suggest to wait_event_interrupt_timeout() every frame in the
> > > > > > > > > > mtk_fd_ipi_handler()?
> > > > > > > > >
> > > > > > > > > Nope, we shouldn't need that.
> > > > > > > > >
> > > > > > > > > > I think maybe the readl_poll_timeout_atomic would be good enough.
> > > > > > > > > >
> > > > > > > > >
> > > > > > > > > Not really. Busy waiting should be avoided as much as possible. What's
> > > > > > > > > the point of entering suspend if you end up burning the power by
> > > > > > > > > spinning the CPU for some milliseconds?
> > > > > > > > >
> > > > > > > > Ok, I see, busy waiting is not a good idea, I will use the wait queue
> > > > > > > > instead. the job abort will refine as following:
> > > > > > > >
> > > > > > > > static void mtk_fd_job_abort(void *priv)
> > > > > > > > {
> > > > > > > > struct mtk_fd_ctx *ctx = priv;
> > > > > > > > struct mtk_fd_dev *fd = ctx->fd_dev;
> > > > > > > > u32 ret;
> > > > > > > >
> > > > > > > > ret = wait_event_interruptible_timeout
> > > > > > > > (fd->wq, (fd->fd_irq_result & MTK_FD_HW_IRQ_MASK),
> > > > > > > > usecs_to_jiffies(50000));
> > > > > > > > if (ret)
> > > > > > > > mtk_fd_hw_job_finish(fd, VB2_BUF_STATE_ERROR);
> > > > > > > > dev_dbg(fd->dev, "%s, ret:%d\n", __func__, ret);
> > > > > > > >
> > > > > > > > fd->fd_irq_result = 0;
> > > > > > > > }
> > > > > > > >
> > > > > > > > static struct v4l2_m2m_ops fd_m2m_ops = {
> > > > > > > > .device_run = mtk_fd_device_run,
> > > > > > > > .job_abort = mtk_fd_job_abort,
> > > > > > >
> > > > > > > I'm not sure we should be using the functon above as the .job_abort
> > > > > > > callback. It's expected to abort the job, but we're just waiting for
> > > > > > > it to finish. I think we should just call mtk_fd_job_abort() manually
> > > > > > > from .stop_streaming.
> > > > > > >
> > > > > >
> > > > > > Ok, I will fix it.
> > > > > >
> > > > > > > > };
> > > > > > > >
> > > > > > > > and we could use it in suspend.
> > > > > > > > static int mtk_fd_suspend(struct device *dev)
> > > > > > > > {
> > > > > > > > struct mtk_fd_dev *fd = dev_get_drvdata(dev);
> > > > > > > >
> > > > > > > > if (pm_runtime_suspended(dev))
> > > > > > > > return 0;
> > > > > > > >
> > > > > > > > if (fd->fd_stream_count)
> > > > > > > > mtk_fd_job_abort(fd->ctx);
> > > > > > > >
> > > > > > > > /* suspend FD HW */
> > > > > > > > writel(0x0, fd->fd_base + MTK_FD_REG_OFFSET_INT_EN);
> > > > > > > > writel(0x0, fd->fd_base + MTK_FD_REG_OFFSET_HW_ENABLE);
> > > > > > > > clk_disable_unprepare(fd->fd_clk);
> > > > > > > > dev_dbg(dev, "%s:disable clock\n", __func__);
> > > > > > > >
> > > > > > > > return 0;
> > > > > > > > }
> > > > > > > >
> > > > > > > > static irqreturn_t mtk_fd_irq(int irq, void *data)
> > > > > > > > {
> > > > > > > > struct mtk_fd_dev *fd = (struct mtk_fd_dev *)data;
> > > > > > > >
> > > > > > > > fd->fd_irq_result = readl(fd->fd_base + MTK_FD_REG_OFFSET_INT_VAL);
> > > > > > > > wake_up_interruptible(&fd->wq);
> > > > > > >
> > > > > > > The wake up should be done at the very end of this function. Otherwise
> > > > > > > we end up with m2m framework racing with the mtk_fd_hw_job_finish()
> > > > > > > below.
> > > > > > >
> > > > > > > Also, I'd use a completion here rather than an open coded wait and
> > > > > > > wake-up. The driver could reinit_completion() before queuing a job to
> > > > > > > the hardware and the IRQ handler would complete(). There would be no
> > > > > > > need to store the IRQ flags in driver data anymore.
> > > > > > Ok, It will be refined as following:
> > > > > >
> > > > > > suspend and stop streaming will call mtk_fd_job_abort()
> > > > > >
> > > > > > static void mtk_fd_job_abort(struct mtk_fd_dev *fd)
> > > > > > {
> > > > > > u32 ret;
> > > > > >
> > > > > > ret = wait_for_completion_timeout(&fd->fd_irq_done,
> > > > > > msecs_to_jiffies(MTK_FD_HW_TIMEOUT));
> > > > > > if (ret)
> > > > >
> > > > > For the _timeout variants, !ret means the timeout and ret > 0 means
> > > > > that the wait ended successfully before.
> > > > >
> > > > Thanks, fixed.
> > > >
> > > > > Also please make sure that the timeout is big enough not to happen
> > > > > normally on a heavily loaded system. Something like 1 second should be
> > > > > good.
> > > > >
> > > > Ok, I will make it 1 second
> > > > #define MTK_FD_HW_TIMEOUT 1000
> > > >
> > > > Also, I will add a condition in mtk_fd_vb2_stop_streaming(), since it
> > > > would be called twice, now it works fine whether I reuse the condition
> > > > with mtk_fd_hw_disconnect or not, however, I think do it before buffer
> > > > remove looks more reasonable.
> > > >
> > > > static void mtk_fd_vb2_stop_streaming(struct vb2_queue *vq)
> > > > {
> > > > struct mtk_fd_ctx *ctx = vb2_get_drv_priv(vq);
> > > > struct mtk_fd_dev *fd = ctx->fd_dev;
> > > > struct vb2_v4l2_buffer *vb;
> > > > struct v4l2_m2m_ctx *m2m_ctx = ctx->fh.m2m_ctx;
> > > > struct v4l2_m2m_queue_ctx *queue_ctx;
> > > >
> > > > if (vq->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE)
> > > > mtk_fd_job_abort(fd);
> > >
> > > We need to do it regardless of the queue type, otherwise we could
> > > still free CAPTURE buffers before the hardware releases them.
> > >
> >
> > I think we may read the fd->fd_irq_done.done and do wait for completion
> > if it's not being done.
> > How do you think?
> >
> > static void mtk_fd_vb2_stop_streaming(struct vb2_queue *vq)
> > {
> > struct mtk_fd_ctx *ctx = vb2_get_drv_priv(vq);
> > struct mtk_fd_dev *fd = ctx->fd_dev;
> > struct vb2_v4l2_buffer *vb;
> > struct v4l2_m2m_ctx *m2m_ctx = ctx->fh.m2m_ctx;
> > struct v4l2_m2m_queue_ctx *queue_ctx;
> > u32 ret;
> >
> > if (!fd->fd_irq_done.done)
>
> We shouldn't access internal fields of completion.
>
> > ret = wait_for_completion_timeout(&fd->fd_irq_done,
> > msecs_to_jiffies(
> > MTK_FD_HW_TIMEOUT));
> > queue_ctx = V4L2_TYPE_IS_OUTPUT(vq->type) ?
> > &m2m_ctx->out_q_ctx :
> > &m2m_ctx->cap_q_ctx;
> > while ((vb = v4l2_m2m_buf_remove(queue_ctx)))
> > v4l2_m2m_buf_done(vb, VB2_BUF_STATE_ERROR);
> >
> > if (vq->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE)
> > mtk_fd_hw_disconnect(fd);
> > }
> >
> > I've also tried to wait completion unconditionally for both queues and
> > the second time will wait until timeout, as a result, it takes longer to
> > swap the camera every time and close the camera app.
>
> I think it should work better if we call complete_all() instead of complete().
>
Thanks,
I use complete_all(), and it works fine now.
static void mtk_fd_vb2_stop_streaming(struct vb2_queue *vq)
{
struct mtk_fd_ctx *ctx = vb2_get_drv_priv(vq);
struct mtk_fd_dev *fd = ctx->fd_dev;
struct vb2_v4l2_buffer *vb;
struct v4l2_m2m_ctx *m2m_ctx = ctx->fh.m2m_ctx;
struct v4l2_m2m_queue_ctx *queue_ctx;
wait_for_completion_timeout(&fd->fd_irq_done,
msecs_to_jiffies(MTK_FD_HW_TIMEOUT));
queue_ctx = V4L2_TYPE_IS_OUTPUT(vq->type) ?
&m2m_ctx->out_q_ctx :
&m2m_ctx->cap_q_ctx;
while ((vb = v4l2_m2m_buf_remove(queue_ctx)))
v4l2_m2m_buf_done(vb, VB2_BUF_STATE_ERROR);
if (vq->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE)
mtk_fd_hw_disconnect(fd);
}
Best regards,
Jerry
> Best regards,
> Tomasz
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* arm64: ls1028a-qds: correct bus of rtc
From: Biwen Li @ 2019-09-04 8:51 UTC (permalink / raw)
To: shawnguo, leoyang.li, robh+dt, mark.rutland
Cc: devicetree, linux-kernel, linux-arm-kernel, Biwen Li
The rtc is on i2c2 bus(hardware), not on i2c1 channel 3,
so correct it
Signed-off-by: Biwen Li <biwen.li@nxp.com>
---
arch/arm64/boot/dts/freescale/fsl-ls1028a-qds.dts | 13 ++++++++-----
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1028a-qds.dts b/arch/arm64/boot/dts/freescale/fsl-ls1028a-qds.dts
index de6ef39f3118..6c0540ad9c59 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls1028a-qds.dts
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1028a-qds.dts
@@ -133,11 +133,6 @@
vcc-supply = <&sb_3v3>;
};
- rtc@51 {
- compatible = "nxp,pcf2129";
- reg = <0x51>;
- };
-
eeprom@56 {
compatible = "atmel,24c512";
reg = <0x56>;
@@ -166,6 +161,14 @@
};
};
+&i2c1 {
+ status = "okay";
+ rtc@51 {
+ compatible = "nxp,pcf2129";
+ reg = <0x51>;
+ };
+};
+
&sai1 {
status = "okay";
};
--
2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
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