* Re: [PATCH] arm64/cpufeature: Move BUG_ON() inside get_arm64_ftr_reg()
From: Catalin Marinas @ 2020-05-21 16:59 UTC (permalink / raw)
To: Will Deacon
Cc: mark.rutland, Suzuki K Poulose, Anshuman Khandual, linux-kernel,
Mark Brown, linux-arm-kernel
In-Reply-To: <20200521162212.GK6608@willie-the-truck>
On Thu, May 21, 2020 at 05:22:15PM +0100, Will Deacon wrote:
> On Thu, May 21, 2020 at 08:45:38AM +0530, Anshuman Khandual wrote:
> > On 05/20/2020 11:09 PM, Will Deacon wrote:
> > > On Wed, May 20, 2020 at 04:47:11PM +0100, Catalin Marinas wrote:
> > >> On Wed, May 20, 2020 at 01:20:13PM +0100, Will Deacon wrote:
> > >>> On Wed, May 20, 2020 at 06:52:54AM +0530, Anshuman Khandual wrote:
> > >>>> There is no way to proceed when requested register could not be searched in
> > >>>> arm64_ftr_reg[]. Requesting for a non present register would be an error as
> > >>>> well. Hence lets just BUG_ON() when the search fails in get_arm64_ftr_reg()
> > >>>> rather than checking for return value and doing the same in some individual
> > >>>> callers.
> > >>>>
> > >>>> But there are some callers that dont BUG_ON() upon search failure. It adds
> > >>>> an argument 'failsafe' that provides required switch between callers based
> > >>>> on whether they could proceed or not.
> > >>>>
> > >>>> Cc: Catalin Marinas <catalin.marinas@arm.com>
> > >>>> Cc: Will Deacon <will@kernel.org>
> > >>>> Cc: Suzuki K Poulose <suzuki.poulose@arm.com>
> > >>>> Cc: Mark Brown <broonie@kernel.org>
> > >>>> Cc: linux-arm-kernel@lists.infradead.org
> > >>>> Cc: linux-kernel@vger.kernel.org
> > >>>>
> > >>>> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
> > >>>> ---
> > >>>> Applies on next-20200518 that has recent cpufeature changes from Will.
> > >>>>
> > >>>> arch/arm64/kernel/cpufeature.c | 26 +++++++++++++-------------
> > >>>> 1 file changed, 13 insertions(+), 13 deletions(-)
> > >>>>
> > >>>> diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
> > >>>> index bc5048f152c1..62767cc540c3 100644
> > >>>> --- a/arch/arm64/kernel/cpufeature.c
> > >>>> +++ b/arch/arm64/kernel/cpufeature.c
> > >>>> @@ -557,7 +557,7 @@ static int search_cmp_ftr_reg(const void *id, const void *regp)
> > >>>> * - NULL on failure. It is upto the caller to decide
> > >>>> * the impact of a failure.
> > >>>> */
> > >>>> -static struct arm64_ftr_reg *get_arm64_ftr_reg(u32 sys_id)
> > >>>> +static struct arm64_ftr_reg *get_arm64_ftr_reg(u32 sys_id, bool failsafe)
> > >>>
> > >>> Generally, I'm not a big fan of boolean arguments because they are really
> > >>> opaque at the callsite. It also seems bogus to me that we don't trust the
> > >>> caller to pass a valid sys_id, but we trust it to get "failsafe" right,
> > >>> which seems to mean "I promise to check the result isn't NULL before
> > >>> dereferencing it."
> > >>>
> > >>> So I don't see how this patch improves anything. I'd actually be more
> > >>> inclined to stick a WARN() in get_arm64_ftr_reg() when it returns NULL and
> > >>> have the callers handle NULL by returning early, getting rid of all the
> > >>> BUG_ONs in here. Sure, the system might end up in a funny state, but we
> > >>> WARN()d about it and tried to keep going (and Linus has some strong opinions
> > >>> on this too).
> > >>
> > >> Such WARN can be triggered by the user via emulate_sys_reg(), so we
> > >> can't really have it in get_arm64_ftr_reg() without a 'failsafe' option.
> > >
> > > Ah yes, that would be bad. In which case, I don't think the existing code
> > > should change.
> >
> > The existing code has BUG_ON() in three different callers doing exactly the
> > same thing that can easily be taken care in get_arm64_ftr_reg() itself. As
> > mentioned before an enum variable (as preferred - over a bool) can still
> > preserve the existing behavior for emulate_sys_reg().
> >
> > IMHO these are very good reasons for us to change the code which will make
> > it cleaner while also removing three redundant BUG_ON() instances. Hence I
> > will request you to please reconsider this proposal.
>
> Hmm, then how about trying my proposal with the WARN_ON(), but having a
> get_arm64_ftr_reg_nowarn() variant for the user emulation case?
That works for me, get_arm64_ftr_reg() would be a wrapper over the
_nowarn function with the added WARN_ON.
read_sanitised_ftr_reg() would need to return something though. Would
all 0s be ok? I think it works as long as we don't have negative CPUID
fields.
--
Catalin
_______________________________________________
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 v9 10/10] arm64: dts: Add node for ufs exynos7
From: Alim Akhtar @ 2020-05-21 16:49 UTC (permalink / raw)
To: 'Krzysztof Kozlowski'
Cc: robh, linux-samsung-soc, linux-scsi, martin.petersen, devicetree,
linux-kernel, kwmad.kim, avri.altman, cang, stanley.chu,
linux-arm-kernel
In-Reply-To: <20200519071636.GA6971@kozik-lap>
> -----Original Message-----
> From: Krzysztof Kozlowski <krzk@kernel.org>
> Sent: 19 May 2020 12:47
> To: Alim Akhtar <alim.akhtar@samsung.com>
> Cc: robh@kernel.org; devicetree@vger.kernel.org; linux-scsi@vger.kernel.org;
> avri.altman@wdc.com; martin.petersen@oracle.com;
> kwmad.kim@samsung.com; stanley.chu@mediatek.com;
> cang@codeaurora.org; linux-samsung-soc@vger.kernel.org; linux-arm-
> kernel@lists.infradead.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH v9 10/10] arm64: dts: Add node for ufs exynos7
>
> On Thu, May 14, 2020 at 06:09:14AM +0530, Alim Akhtar wrote:
> > Adding dt node foe UFS and UFS-PHY for exynos7 SoC.
> >
> > Signed-off-by: Alim Akhtar <alim.akhtar@samsung.com>
> > Tested-by: Paweł Chmiel <pawel.mikolaj.chmiel@gmail.com>
> > ---
> > .../boot/dts/exynos/exynos7-espresso.dts | 4 ++
> > arch/arm64/boot/dts/exynos/exynos7.dtsi | 43 ++++++++++++++++++-
> > 2 files changed, 45 insertions(+), 2 deletions(-)
>
> I will pick it up after all bindings get Rob's ack (or are picked up as well). The
> second bindings patch are still pending on that.
>
Thank Krzysztof,
Yes, one binding still awaiting Rob's ack, I have addressed his comment in this v9 series.
Hoping he will find some time to review the same.
> Best regards,
> Krzysztof
_______________________________________________
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 v6 00/19] mtd: spi-nor: add xSPI Octal DTR support
From: Mark Brown @ 2020-05-21 16:39 UTC (permalink / raw)
To: Pratyush Yadav
Cc: Alexandre Belloni, Mason Yang, Vignesh Raghavendra, Tudor Ambarus,
Richard Weinberger, Sekhar Nori, linux-kernel, Ludovic Desroches,
Boris Brezillon, linux-mtd, Miquel Raynal, Matthias Brugger,
linux-mediatek, linux-spi, linux-arm-kernel
In-Reply-To: <20200520163053.24357-1-p.yadav@ti.com>
[-- Attachment #1.1: Type: text/plain, Size: 536 bytes --]
On Wed, May 20, 2020 at 10:00:34PM +0530, Pratyush Yadav wrote:
> Hi,
>
> This series adds support for octal DTR flashes in the spi-nor framework,
> and then adds hooks for the Cypress Semper and Mircom Xcella flashes to
> allow running them in octal DTR mode. This series assumes that the flash
> is handed to the kernel in Legacy SPI mode.
Are people happy with the SPI bits of this from a MTD point of view? I
don't have any concerns and could apply them on a branch even if more
revisions are needed for the MTD side.
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
[-- Attachment #2: Type: text/plain, Size: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH] arm64/cpufeature: Move BUG_ON() inside get_arm64_ftr_reg()
From: Will Deacon @ 2020-05-21 16:22 UTC (permalink / raw)
To: Anshuman Khandual
Cc: mark.rutland, Suzuki K Poulose, Catalin Marinas, linux-kernel,
Mark Brown, linux-arm-kernel
In-Reply-To: <cdea1cc5-41be-c125-d4d1-f63ff1989ec6@arm.com>
On Thu, May 21, 2020 at 08:45:38AM +0530, Anshuman Khandual wrote:
>
>
> On 05/20/2020 11:09 PM, Will Deacon wrote:
> > On Wed, May 20, 2020 at 04:47:11PM +0100, Catalin Marinas wrote:
> >> On Wed, May 20, 2020 at 01:20:13PM +0100, Will Deacon wrote:
> >>> On Wed, May 20, 2020 at 06:52:54AM +0530, Anshuman Khandual wrote:
> >>>> There is no way to proceed when requested register could not be searched in
> >>>> arm64_ftr_reg[]. Requesting for a non present register would be an error as
> >>>> well. Hence lets just BUG_ON() when the search fails in get_arm64_ftr_reg()
> >>>> rather than checking for return value and doing the same in some individual
> >>>> callers.
> >>>>
> >>>> But there are some callers that dont BUG_ON() upon search failure. It adds
> >>>> an argument 'failsafe' that provides required switch between callers based
> >>>> on whether they could proceed or not.
> >>>>
> >>>> Cc: Catalin Marinas <catalin.marinas@arm.com>
> >>>> Cc: Will Deacon <will@kernel.org>
> >>>> Cc: Suzuki K Poulose <suzuki.poulose@arm.com>
> >>>> Cc: Mark Brown <broonie@kernel.org>
> >>>> Cc: linux-arm-kernel@lists.infradead.org
> >>>> Cc: linux-kernel@vger.kernel.org
> >>>>
> >>>> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
> >>>> ---
> >>>> Applies on next-20200518 that has recent cpufeature changes from Will.
> >>>>
> >>>> arch/arm64/kernel/cpufeature.c | 26 +++++++++++++-------------
> >>>> 1 file changed, 13 insertions(+), 13 deletions(-)
> >>>>
> >>>> diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
> >>>> index bc5048f152c1..62767cc540c3 100644
> >>>> --- a/arch/arm64/kernel/cpufeature.c
> >>>> +++ b/arch/arm64/kernel/cpufeature.c
> >>>> @@ -557,7 +557,7 @@ static int search_cmp_ftr_reg(const void *id, const void *regp)
> >>>> * - NULL on failure. It is upto the caller to decide
> >>>> * the impact of a failure.
> >>>> */
> >>>> -static struct arm64_ftr_reg *get_arm64_ftr_reg(u32 sys_id)
> >>>> +static struct arm64_ftr_reg *get_arm64_ftr_reg(u32 sys_id, bool failsafe)
> >>>
> >>> Generally, I'm not a big fan of boolean arguments because they are really
> >>> opaque at the callsite. It also seems bogus to me that we don't trust the
> >>> caller to pass a valid sys_id, but we trust it to get "failsafe" right,
> >>> which seems to mean "I promise to check the result isn't NULL before
> >>> dereferencing it."
> >>>
> >>> So I don't see how this patch improves anything. I'd actually be more
> >>> inclined to stick a WARN() in get_arm64_ftr_reg() when it returns NULL and
> >>> have the callers handle NULL by returning early, getting rid of all the
> >>> BUG_ONs in here. Sure, the system might end up in a funny state, but we
> >>> WARN()d about it and tried to keep going (and Linus has some strong opinions
> >>> on this too).
> >>
> >> Such WARN can be triggered by the user via emulate_sys_reg(), so we
> >> can't really have it in get_arm64_ftr_reg() without a 'failsafe' option.
> >
> > Ah yes, that would be bad. In which case, I don't think the existing code
> > should change.
>
> The existing code has BUG_ON() in three different callers doing exactly the
> same thing that can easily be taken care in get_arm64_ftr_reg() itself. As
> mentioned before an enum variable (as preferred - over a bool) can still
> preserve the existing behavior for emulate_sys_reg().
>
> IMHO these are very good reasons for us to change the code which will make
> it cleaner while also removing three redundant BUG_ON() instances. Hence I
> will request you to please reconsider this proposal.
Hmm, then how about trying my proposal with the WARN_ON(), but having a
get_arm64_ftr_reg_nowarn() variant for the user emulation 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 v11] arm64: dts: qcom: sc7180: Add WCN3990 WLAN module device node
From: Evan Green @ 2020-05-21 16:19 UTC (permalink / raw)
To: Rakesh Pillai
Cc: open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, LKML,
linux-arm Mailing List, linux-arm-msm
In-Reply-To: <1589946996-31264-1-git-send-email-pillair@codeaurora.org>
On Tue, May 19, 2020 at 8:57 PM Rakesh Pillai <pillair@codeaurora.org> wrote:
>
> Add device node for the ath10k SNOC platform driver probe
> and add resources required for WCN3990 on sc7180 soc.
>
> Signed-off-by: Rakesh Pillai <pillair@codeaurora.org>
Reviewed-by: Evan Green <evgreen@chromium.org>
_______________________________________________
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 04/11] ARM: Allow IPIs to be handled as normal interrupts
From: Valentin Schneider @ 2020-05-21 16:11 UTC (permalink / raw)
To: Russell King - ARM Linux admin
Cc: Sumit Garg, kernel-team, Jason Cooper, Marc Zyngier, linux-kernel,
Catalin Marinas, Thomas Gleixner, Will Deacon, linux-arm-kernel
In-Reply-To: <20200521151212.GT1551@shell.armlinux.org.uk>
On 21/05/20 16:12, Russell King - ARM Linux admin wrote:
> On Thu, May 21, 2020 at 03:03:49PM +0100, Valentin Schneider wrote:
>>
>> On 19/05/20 23:24, Russell King - ARM Linux admin wrote:
>> > On Tue, May 19, 2020 at 05:17:48PM +0100, Marc Zyngier wrote:
>> >> In order to deal with IPIs as normal interrupts, let's add
>> >> a new way to register them with the architecture code.
>> >>
>> >> set_smp_ipi_range() takes a range of interrupts, and allows
>> >> the arch code to request them as if the were normal interrupts.
>> >> A standard handler is then called by the core IRQ code to deal
>> >> with the IPI.
>> >>
>> >> This means that we don't need to call irq_enter/irq_exit, and
>> >> that we don't need to deal with set_irq_regs either. So let's
>> >> move the dispatcher into its own function, and leave handle_IPI()
>> >> as a compatibility function.
>> >>
>> >> On the sending side, let's make use of ipi_send_mask, which
>> >> already exists for this purpose.
>> >
>> > You say nothing about the nesting of irq_enter() and irq_exit()
>> > for scheduler_ipi().
>> >
>> > Given that lockdep introduced the requirement that hard IRQs can't
>> > be nested, are we sure that calling irq_exit() twice is safe?
>> >
>> > Looking at irqtime_account_irq(), it seems that will cause double-
>> > accounting of in-interrupt time, since we will increment
>> > irq_start_time by just over twice the the period spent handling
>> > the IPI.
>> >
>> > I think the rest of irq_exit() should be safe, but still, this
>> > behaviour should be documented at the very least, if not avoided.
>> >
>>
>> x86 does the same (though IIUC only when tracing reschedule IPI's),
>
> Right, so when the system is operating normally, then the accounting is
> correct. When the reschedule path is being explicitly traced, then
> the accounting will be doubled for it.
>
Right, it's true that they are only affected when tracing.
That said, AFAICT the accounting nests correctly. Consider:
irq_enter() @t0
irq_enter() @t1
...
irq_exit() @t2
irq_exit() @t3
Entering irqtime_account_irq() at time t, we get something like:
delta = t - irq_start_time;
irq_start_time = t;
if (hardirq_count())
total += delta;
Since we go through the accounting on both irq_enter() and irq_exit(), we'd
have something like:
irq_enter() @t0
irq_start_time = t0
irq_enter() @t1
delta = t1 - t0
irq_start_time = t1
total += t1 - t0
irq_exit() @t2
delta = t2 - t1
irq_start_time = t2
total += t2 - t1
irq_exit() @t3
delta = t3 - t2
irq_start_time = t3
total += t3 - t2
So at the end we have incremented the total by
t1-t0 + t2-t1 + t3-t2 = t3 - t0
IOW the duration of the outermost pair (... Unless I goofed up).
> What's being proposed for ARM is to always have this mis-accounting,
> where no mis-accounting was present before - and some of us (me) /do/
> enable IRQ accounting in our kernels as standard. So, you can take
> this as a kernel regression report from a user.
>
>> and MIPS has the same issue as it also uses generic IRQ IPI's - so
>> although it's not ideal, I think we can live with it.
>
> Yes, but is there anyone who cares about this for MIPS?
_______________________________________________
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 v8 14/14] media: platform: Add jpeg dec/enc feature
From: Tomasz Figa @ 2020-05-21 16:08 UTC (permalink / raw)
To: Hans Verkuil, Xia Jiang
Cc: Xia Jiang, devicetree, mojahsu, srv_heupstream, Rick Chang,
senozhatsky, drinkcat, linux-kernel, maoguang.meng,
Matthias Brugger, sj.huang, Rob Herring, linux-mediatek,
Mauro Carvalho Chehab, Marek Szyprowski, linux-arm-kernel,
linux-media
In-Reply-To: <b62b303c-10cd-fdf6-52fa-90d63124487a@xs4all.nl>
On Mon, May 11, 2020 at 11:04:19AM +0200, Hans Verkuil wrote:
> On 03/04/2020 11:40, Xia Jiang wrote:
> > Add mtk jpeg encode v4l2 driver based on jpeg decode, because that jpeg
> > decode and encode have great similarities with function operation.
> >
> > Signed-off-by: Xia Jiang <xia.jiang@mediatek.com>
> > ---
> > v8:jpeg encoder and decoder use separate callbacks instead of repeating
> > the if/else in every callback.
> > improve vidioc_try_fmt() implementation that can be shared by jpeg
> > encoder and decoder.
> > fix the bug of jpeg encoder s_selection implementation.
> > cancel the state of the jpeg encoder.
> > improve jpeg encoder and decoder set default params flow.
> > put the clock names and other datas in a match_data struct.
> > fix the bug of geting correctly quality value.
> > do the all the bits' settings of one register in one function.
> > move the code of mtk_jpeg_enc_reg.h to mtk_jpeg_enc_hw.h and delete
> > mtk_jpeg_enc_reg.h.
> >
> > v7: reverse spin lock and unlock operation in device run function for
> > multi-instance.
> >
> > v6: add space to arounding '+'.
> > alignment 'struct mtk_jpeg_fmt *fmt' match open parenthesis.
> > change 'mtk_jpeg_enc_set_encFormat' to 'mtk_jpeg_enc_set_enc_format'.
> > make 'mtk_jpeg_ctrls_setup' to static prototype.
> > delete unused variables 'jpeg'/'align_h'/'align_w'/'flags'.
> > initialize 'yuv_format'/'enc_quality' variables.
> >
> > v5: support crop for encoder and compose for decoder in s_selection and
> > g_selection function.
> > use clamp() to replace mtk_jpeg_bound_align_image() and round_up()
> > to replace mtk_jpeg_align().
> > delete jpeg_enc_param/mtk_jpeg_enc_param structure and
> > mtk_jpeg_set_param(), program the registers directly based on
> > the original V4L2 values.
> > move macro definition about hw to mtk_jpeg_enc_reg.h.
> > delete unnecessary V4L2 logs in driver.
> > cancel spin lock and unlock operation in deviec run function.
> > change jpeg enc register offset hex numberals from upercase to lowercase.
> >
> > v4: split mtk_jpeg_try_fmt_mplane() to two functions, one for encoder,
> > one for decoder.
> > split mtk_jpeg_set_default_params() to two functions, one for
> > encoder, one for decoder.
> > add cropping support for encoder in g/s_selection ioctls.
> > change exif mode support by using V4L2_JPEG_ACTIVE_MARKER_APP1.
> > change MTK_JPEG_MAX_WIDTH/MTK_JPEG_MAX_HEIGH from 8192 to 65535 by
> > specification.
> > move width shifting operation behind aligning operation in
> > mtk_jpeg_try_enc_fmt_mplane() for bug fix.
> > fix user abuseing data_offset issue for DMABUF in
> > mtk_jpeg_set_enc_src().
> > fix kbuild warings: change MTK_JPEG_MIN_HEIGHT/MTK_JPEG_MAX_HEIGHT
> > and MTK_JPEG_MIN_WIDTH/MTK_JPEG_MAX_WIDTH from
> > 'int' type to 'unsigned int' type.
> > fix msleadingly indented of 'else'.
> > v3: delete Change-Id.
> > only test once handler->error after the last v4l2_ctrl_new_std().
> > seperate changes of v4l2-ctrls.c and v4l2-controls.h to new patch.
> > v2: fix compliance test fail, check created buffer size in driver.
> > ---
> > drivers/media/platform/mtk-jpeg/Makefile | 5 +-
> > .../media/platform/mtk-jpeg/mtk_jpeg_core.c | 1038 +++++++++++++----
> > .../media/platform/mtk-jpeg/mtk_jpeg_core.h | 51 +-
> > .../media/platform/mtk-jpeg/mtk_jpeg_dec_hw.h | 7 +-
> > .../media/platform/mtk-jpeg/mtk_jpeg_enc_hw.c | 193 +++
> > .../media/platform/mtk-jpeg/mtk_jpeg_enc_hw.h | 123 ++
> > 6 files changed, 1188 insertions(+), 229 deletions(-)
> > create mode 100644 drivers/media/platform/mtk-jpeg/mtk_jpeg_enc_hw.c
> > create mode 100644 drivers/media/platform/mtk-jpeg/mtk_jpeg_enc_hw.h
> >
> > diff --git a/drivers/media/platform/mtk-jpeg/Makefile b/drivers/media/platform/mtk-jpeg/Makefile
> > index 48516dcf96e6..76c33aad0f3f 100644
> > --- a/drivers/media/platform/mtk-jpeg/Makefile
> > +++ b/drivers/media/platform/mtk-jpeg/Makefile
> > @@ -1,3 +1,6 @@
> > # SPDX-License-Identifier: GPL-2.0-only
> > -mtk_jpeg-objs := mtk_jpeg_core.o mtk_jpeg_dec_hw.o mtk_jpeg_dec_parse.o
> > +mtk_jpeg-objs := mtk_jpeg_core.o \
> > + mtk_jpeg_dec_hw.o \
> > + mtk_jpeg_dec_parse.o \
> > + mtk_jpeg_enc_hw.o
> > obj-$(CONFIG_VIDEO_MEDIATEK_JPEG) += mtk_jpeg.o
> > diff --git a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
> > index 77a95185584c..18a759ce2c46 100644
> > --- a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
> > +++ b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
> > @@ -3,6 +3,7 @@
> > * Copyright (c) 2016 MediaTek Inc.
> > * Author: Ming Hsiu Tsai <minghsiu.tsai@mediatek.com>
> > * Rick Chang <rick.chang@mediatek.com>
> > + * Xia Jiang <xia.jiang@mediatek.com>
> > */
> >
> > #include <linux/clk.h>
> > @@ -23,11 +24,60 @@
> > #include <media/videobuf2-dma-contig.h>
> > #include <soc/mediatek/smi.h>
> >
> > +#include "mtk_jpeg_enc_hw.h"
> > #include "mtk_jpeg_dec_hw.h"
> > #include "mtk_jpeg_core.h"
> > #include "mtk_jpeg_dec_parse.h"
> >
> > -static struct mtk_jpeg_fmt mtk_jpeg_formats[] = {
> > +static struct mtk_jpeg_fmt mtk_jpeg_enc_formats[] = {
> > + {
> > + .fourcc = V4L2_PIX_FMT_JPEG,
> > + .colplanes = 1,
> > + .flags = MTK_JPEG_FMT_FLAG_ENC_CAPTURE,
> > + },
> > + {
> > + .fourcc = V4L2_PIX_FMT_NV12M,
> > + .hw_format = JPEG_ENC_YUV_FORMAT_NV12,
> > + .h_sample = {4, 4},
> > + .v_sample = {4, 2},
> > + .colplanes = 2,
> > + .h_align = 4,
> > + .v_align = 4,
> > + .flags = MTK_JPEG_FMT_FLAG_ENC_OUTPUT,
> > + },
> > + {
> > + .fourcc = V4L2_PIX_FMT_NV21M,
> > + .hw_format = JEPG_ENC_YUV_FORMAT_NV21,
> > + .h_sample = {4, 4},
> > + .v_sample = {4, 2},
> > + .colplanes = 2,
> > + .h_align = 4,
> > + .v_align = 4,
> > + .flags = MTK_JPEG_FMT_FLAG_ENC_OUTPUT,
> > + },
> > + {
> > + .fourcc = V4L2_PIX_FMT_YUYV,
> > + .hw_format = JPEG_ENC_YUV_FORMAT_YUYV,
> > + .h_sample = {8},
> > + .v_sample = {4},
> > + .colplanes = 1,
> > + .h_align = 5,
> > + .v_align = 3,
> > + .flags = MTK_JPEG_FMT_FLAG_ENC_OUTPUT,
> > + },
> > + {
> > + .fourcc = V4L2_PIX_FMT_YVYU,
> > + .hw_format = JPEG_ENC_YUV_FORMAT_YVYU,
> > + .h_sample = {8},
> > + .v_sample = {4},
> > + .colplanes = 1,
> > + .h_align = 5,
> > + .v_align = 3,
> > + .flags = MTK_JPEG_FMT_FLAG_ENC_OUTPUT,
> > + },
> > +};
> > +
> > +static struct mtk_jpeg_fmt mtk_jpeg_dec_formats[] = {
> > {
> > .fourcc = V4L2_PIX_FMT_JPEG,
> > .colplanes = 1,
> > @@ -53,7 +103,8 @@ static struct mtk_jpeg_fmt mtk_jpeg_formats[] = {
> > },
> > };
> >
> > -#define MTK_JPEG_NUM_FORMATS ARRAY_SIZE(mtk_jpeg_formats)
> > +#define MTK_JPEG_ENC_NUM_FORMATS ARRAY_SIZE(mtk_jpeg_enc_formats)
> > +#define MTK_JPEG_DEC_NUM_FORMATS ARRAY_SIZE(mtk_jpeg_dec_formats)
> >
> > enum {
> > MTK_JPEG_BUF_FLAGS_INIT = 0,
> > @@ -70,6 +121,11 @@ struct mtk_jpeg_src_buf {
> > static int debug;
> > module_param(debug, int, 0644);
> >
> > +static inline struct mtk_jpeg_ctx *ctrl_to_ctx(struct v4l2_ctrl *ctrl)
> > +{
> > + return container_of(ctrl->handler, struct mtk_jpeg_ctx, ctrl_hdl);
> > +}
> > +
> > static inline struct mtk_jpeg_ctx *mtk_jpeg_fh_to_ctx(struct v4l2_fh *fh)
> > {
> > return container_of(fh, struct mtk_jpeg_ctx, fh);
> > @@ -81,12 +137,25 @@ static inline struct mtk_jpeg_src_buf *mtk_jpeg_vb2_to_srcbuf(
> > return container_of(to_vb2_v4l2_buffer(vb), struct mtk_jpeg_src_buf, b);
> > }
> >
> > -static int mtk_jpeg_querycap(struct file *file, void *priv,
> > - struct v4l2_capability *cap)
> > +static int mtk_jpeg_enc_querycap(struct file *file, void *priv,
> > + struct v4l2_capability *cap)
> > +{
> > + struct mtk_jpeg_dev *jpeg = video_drvdata(file);
> > +
> > + strscpy(cap->driver, MTK_JPEG_NAME, sizeof(cap->driver));
> > + strscpy(cap->card, MTK_JPEG_NAME " encoder", sizeof(cap->card));
> > + snprintf(cap->bus_info, sizeof(cap->bus_info), "platform:%s",
> > + dev_name(jpeg->dev));
> > +
> > + return 0;
> > +}
> > +
> > +static int mtk_jpeg_dec_querycap(struct file *file, void *priv,
> > + struct v4l2_capability *cap)
> > {
> > struct mtk_jpeg_dev *jpeg = video_drvdata(file);
> >
> > - strscpy(cap->driver, MTK_JPEG_NAME " decoder", sizeof(cap->driver));
> > + strscpy(cap->driver, MTK_JPEG_NAME, sizeof(cap->driver));
> > strscpy(cap->card, MTK_JPEG_NAME " decoder", sizeof(cap->card));
> > snprintf(cap->bus_info, sizeof(cap->bus_info), "platform:%s",
> > dev_name(jpeg->dev));
> > @@ -94,6 +163,54 @@ static int mtk_jpeg_querycap(struct file *file, void *priv,
> > return 0;
> > }
> >
> > +static int vidioc_jpeg_enc_s_ctrl(struct v4l2_ctrl *ctrl)
> > +{
> > + struct mtk_jpeg_ctx *ctx = ctrl_to_ctx(ctrl);
> > +
> > + switch (ctrl->id) {
> > + case V4L2_CID_JPEG_RESTART_INTERVAL:
> > + ctx->restart_interval = ctrl->val;
> > + break;
> > + case V4L2_CID_JPEG_COMPRESSION_QUALITY:
> > + ctx->enc_quality = ctrl->val;
> > + break;
> > + case V4L2_CID_JPEG_ACTIVE_MARKER:
> > + ctx->enable_exif = ctrl->val & V4L2_JPEG_ACTIVE_MARKER_APP1 ?
> > + true : false;
> > + break;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static const struct v4l2_ctrl_ops mtk_jpeg_enc_ctrl_ops = {
> > + .s_ctrl = vidioc_jpeg_enc_s_ctrl,
> > +};
> > +
> > +static int mtk_jpeg_enc_ctrls_setup(struct mtk_jpeg_ctx *ctx)
> > +{
> > + const struct v4l2_ctrl_ops *ops = &mtk_jpeg_enc_ctrl_ops;
> > + struct v4l2_ctrl_handler *handler = &ctx->ctrl_hdl;
> > +
> > + v4l2_ctrl_handler_init(handler, 3);
> > +
> > + v4l2_ctrl_new_std(handler, ops, V4L2_CID_JPEG_RESTART_INTERVAL, 0, 100,
> > + 1, 0);
> > + v4l2_ctrl_new_std(handler, ops, V4L2_CID_JPEG_COMPRESSION_QUALITY, 48,
> > + 100, 1, 90);
> > + v4l2_ctrl_new_std(handler, ops, V4L2_CID_JPEG_ACTIVE_MARKER, 0,
> > + V4L2_JPEG_ACTIVE_MARKER_APP1, 0, 0);
> > +
> > + if (handler->error) {
> > + v4l2_ctrl_handler_free(&ctx->ctrl_hdl);
> > + return handler->error;
> > + }
> > +
> > + v4l2_ctrl_handler_setup(&ctx->ctrl_hdl);
> > +
> > + return 0;
> > +}
> > +
> > static int mtk_jpeg_enum_fmt(struct mtk_jpeg_fmt *mtk_jpeg_formats, int n,
> > struct v4l2_fmtdesc *f, u32 type)
> > {
> > @@ -115,117 +232,105 @@ static int mtk_jpeg_enum_fmt(struct mtk_jpeg_fmt *mtk_jpeg_formats, int n,
> > return 0;
> > }
> >
> > -static int mtk_jpeg_enum_fmt_vid_cap(struct file *file, void *priv,
> > - struct v4l2_fmtdesc *f)
> > +static int mtk_jpeg_enc_enum_fmt_vid_cap(struct file *file, void *priv,
> > + struct v4l2_fmtdesc *f)
> > {
> > - return mtk_jpeg_enum_fmt(mtk_jpeg_formats, MTK_JPEG_NUM_FORMATS, f,
> > + return mtk_jpeg_enum_fmt(mtk_jpeg_enc_formats,
> > + MTK_JPEG_ENC_NUM_FORMATS, f,
> > + MTK_JPEG_FMT_FLAG_ENC_CAPTURE);
> > +}
> > +
> > +static int mtk_jpeg_dec_enum_fmt_vid_cap(struct file *file, void *priv,
> > + struct v4l2_fmtdesc *f)
> > +{
> > + return mtk_jpeg_enum_fmt(mtk_jpeg_dec_formats,
> > + MTK_JPEG_DEC_NUM_FORMATS, f,
> > MTK_JPEG_FMT_FLAG_DEC_CAPTURE);
> > }
> >
> > -static int mtk_jpeg_enum_fmt_vid_out(struct file *file, void *priv,
> > - struct v4l2_fmtdesc *f)
> > +static int mtk_jpeg_enc_enum_fmt_vid_out(struct file *file, void *priv,
> > + struct v4l2_fmtdesc *f)
> > +{
> > + return mtk_jpeg_enum_fmt(mtk_jpeg_enc_formats,
> > + MTK_JPEG_ENC_NUM_FORMATS, f,
> > + MTK_JPEG_FMT_FLAG_ENC_OUTPUT);
> > +}
> > +
> > +static int mtk_jpeg_dec_enum_fmt_vid_out(struct file *file, void *priv,
> > + struct v4l2_fmtdesc *f)
> > {
> > - return mtk_jpeg_enum_fmt(mtk_jpeg_formats, MTK_JPEG_NUM_FORMATS, f,
> > - MTK_JPEG_FMT_FLAG_DEC_OUTPUT);
> > + return mtk_jpeg_enum_fmt(mtk_jpeg_dec_formats, MTK_JPEG_DEC_NUM_FORMATS,
> > + f, MTK_JPEG_FMT_FLAG_DEC_OUTPUT);
> > }
>
> OK, so this patch is very hard to read because there are two independent changes
> taking place:
>
> 1) rename existing functions/defines/variables with a _dec prefix to prepare
> for the addition of the encoder feature.
>
> 2) add the encoder feature.
>
> Please split up this patch into two parts: one that does the rename and as much of
> the preparation to support both decoder and encoder without changing the
> functionality, and a second one that actually adds the new encoder feature.
>
> In fact, once that's done it is likely that most of this patch series can be
> merged, even if there are still things that need to be changed for the last
> patch adding the encoder support. I see nothing objectionable in patches 1-10
> and 13. So merging those together with a new rename patch wouldn't be an issue,
> I think.
>
> In any case, the diffs should be a lot cleaner and easier to review by splitting
> it up like that.
Agreed with Hans. The split will make it much easier to review this one
for me, so I'll wait for it. Thanks in advance.
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] arch/{mips,sparc,microblaze,powerpc}: Don't enable pagefault/preempt twice
From: Guenter Roeck @ 2020-05-21 16:05 UTC (permalink / raw)
To: Ira Weiny
Cc: Peter Zijlstra, Benjamin Herrenschmidt, Dave Hansen, dri-devel,
linux-mips, James E.J. Bottomley, Max Filippov, Paul Mackerras,
H. Peter Anvin, sparclinux, Dan Williams, Helge Deller, x86,
linux-csky, Christoph Hellwig, Ingo Molnar, linux-snps-arc,
linux-xtensa, Borislav Petkov, Al Viro, Andy Lutomirski,
Thomas Gleixner, linux-arm-kernel, Chris Zankel,
Thomas Bogendoerfer, linux-parisc, linux-kernel, Christian Koenig,
Andrew Morton, linuxppc-dev, David S. Miller
In-Reply-To: <20200520051315.GA3660833@iweiny-DESK2.sc.intel.com>
On 5/19/20 10:13 PM, Ira Weiny wrote:
> On Tue, May 19, 2020 at 12:42:15PM -0700, Guenter Roeck wrote:
>> On Tue, May 19, 2020 at 11:40:32AM -0700, Ira Weiny wrote:
>>> On Tue, May 19, 2020 at 09:54:22AM -0700, Guenter Roeck wrote:
>>>> On Mon, May 18, 2020 at 11:48:43AM -0700, ira.weiny@intel.com wrote:
>>>>> From: Ira Weiny <ira.weiny@intel.com>
>>>>>
>>>>> The kunmap_atomic clean up failed to remove one set of pagefault/preempt
>>>>> enables when vaddr is not in the fixmap.
>>>>>
>>>>> Fixes: bee2128a09e6 ("arch/kunmap_atomic: consolidate duplicate code")
>>>>> Signed-off-by: Ira Weiny <ira.weiny@intel.com>
>>>>
>>>> microblazeel works with this patch,
>>>
>>> Awesome... Andrew in my rush yesterday I should have put a reported by on the
>>> patch for Guenter as well.
>>>
>>> Sorry about that Guenter,
>>
>> No worries.
>>
>>> Ira
>>>
>>>> as do the nosmp sparc32 boot tests,
>>>> but sparc32 boot tests with SMP enabled still fail with lots of messages
>>>> such as:
>>>>
>>>> BUG: Bad page state in process swapper/0 pfn:006a1
>>>> page:f0933420 refcount:0 mapcount:1 mapping:(ptrval) index:0x1
>>>> flags: 0x0()
>>>> raw: 00000000 00000100 00000122 00000000 00000001 00000000 00000000 00000000
>>>> page dumped because: nonzero mapcount
>>>> Modules linked in:
>>>> CPU: 0 PID: 1 Comm: swapper/0 Tainted: G B 5.7.0-rc6-next-20200518-00002-gb178d2d56f29 #1
>>>> [f00e7ab8 :
>>>> bad_page+0xa8/0x108 ]
>>>> [f00e8b54 :
>>>> free_pcppages_bulk+0x154/0x52c ]
>>>> [f00ea024 :
>>>> free_unref_page+0x54/0x6c ]
>>>> [f00ed864 :
>>>> free_reserved_area+0x58/0xec ]
>>>> [f0527104 :
>>>> kernel_init+0x14/0x110 ]
>>>> [f000b77c :
>>>> ret_from_kernel_thread+0xc/0x38 ]
>>>> [00000000 :
>>>> 0x0 ]
>>>>
>>>> Code path leading to that message is different but always the same
>>>> from free_unref_page().
>
> Actually it occurs to me that the patch consolidating kmap_prot is odd for
> sparc 32 bit...
>
> Its a long shot but could you try reverting this patch?
>
> 4ea7d2419e3f kmap: consolidate kmap_prot definitions
>
That is not easy to revert, unfortunately, due to several follow-up patches.
Guenter
> Alternately I will need to figure out how to run the sparc on qemu here...
>
> Thanks very much for all the testing though! :-D
>
> Ira
>
>>>>
>>>> Still testing ppc images.
>>>>
>>
>> ppc image tests are passing with this patch.
>>
>> Guenter
_______________________________________________
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] soc: imx: Add power domain driver support for i.mx8m family
From: Adam Ford @ 2020-05-21 16:02 UTC (permalink / raw)
To: Tim Harvey
Cc: Aisheng Dong, Jacky Bai, Liam Girdwood, Linux Kernel Mailing List,
Mark Brown, dl-linux-imx, Sascha Hauer, Lucas Stach, Shawn Guo,
linux-arm-kernel@lists.infradead.org, Abel Vesa
In-Reply-To: <CAJ+vNU3oMSpbysJ+BXyGGFknZs6BHH6hZ_qLpnsxmSBJ1Log7A@mail.gmail.com>
On Thu, May 21, 2020 at 10:56 AM Tim Harvey <tharvey@gateworks.com> wrote:
>
> On Mon, May 4, 2020 at 2:19 AM Lucas Stach <l.stach@pengutronix.de> wrote:
> >
> > Am Montag, den 27.04.2020, 15:37 +0000 schrieb Jacky Bai:
> > > > -----Original Message-----
> > > > From: Lucas Stach <l.stach@pengutronix.de>
> > > > Sent: Monday, April 27, 2020 11:11 PM
> > > > To: Abel Vesa <abel.vesa@nxp.com>; Jacky Bai <ping.bai@nxp.com>; Shawn
> > > > Guo <shawnguo@kernel.org>; Sascha Hauer <kernel@pengutronix.de>; Liam
> > > > Girdwood <lgirdwood@gmail.com>; Mark Brown <broonie@kernel.org>
> > > > Cc: Aisheng Dong <aisheng.dong@nxp.com>; dl-linux-imx
> > > > <linux-imx@nxp.com>; linux-arm-kernel@lists.infradead.org; Linux Kernel
> > > > Mailing List <linux-kernel@vger.kernel.org>
> > > > Subject: Re: [PATCH] soc: imx: Add power domain driver support for i.mx8m
> > > > family
> > > >
> > > > Am Montag, den 27.04.2020, 17:58 +0300 schrieb Abel Vesa:
> > > > > From: Jacky Bai <ping.bai@nxp.com>
> > > > >
> > > > > The i.MX8M family is a set of NXP product focus on delivering the
> > > > > latest and greatest video and audio experience combining
> > > > > state-of-the-art media-specific features with high-performance
> > > > > processing while optimized for lowest power consumption.
> > > > >
> > > > > i.MX8MQ, i.MX8MM, i.MX8MN, even the furture i.MX8MP are all belong to
> > > > > this family. A GPC module is used to manage all the PU power domain
> > > > > on/off. But the situation is that the number of power domains & the
> > > > > power up sequence has significate difference on those SoCs. Even on
> > > > > the same SoC. The power up sequence still has big difference. It makes
> > > > > us hard to reuse the GPCv2 driver to cover the whole i.MX8M family.
> > > > > Each time a new SoC is supported in the mainline kernel, we need to
> > > > > modify the GPCv2 driver to support it. We need to add or modify
> > > > > hundred lines of code in worst case.
> > > > > It is a bad practice for the driver maintainability.
> > > > >
> > > > > This driver add a more generic power domain driver that the actual
> > > > > power on/off is done by TF-A code. the abstraction give us the
> > > > > possibility that using one driver to cover the whole i.MX8M family in
> > > > > kernel side.
> > > > >
> > > >
> > > > Again: what does this driver bring to the table, other than moving a fraction of
> > > > the power domain functionality into the firmware?
> > > >
> > > > The discussions on the last submissions of this driver already established that
> > > > we can't move all functionality for the power domains into the firmware, as
> > > > controlling regulators is probably not easy to do from this context. Also the
> > > > TF-A side implementation of this driver is "interesting" IMHO, it does stuff like
> > > > accessing the clock controller registers without any locking or other means of
> > > > mutual exclusion with the Linux kernel clock controller driver.
> > > >
> > >
> > > The clock handling is in kernel side through CCF, not in ATF. See the patch below.
> > >
> > > > Why can't we just extend the existing GPCv2 driver with support for the other
> > > > i.MX8M family members?
> > > >
> > >
> > > The reason that why I don’t like to extend the GPCv2 is that when doing domain on/off,
> > > We need to access some special control register in each domain & do some special flow,
> > > These control register(mediamix block control, vpumix block control) is not in GPC
> > > module's address range. No benefit to reuse the GPCv2. Only bring complexity to the
> > > GPCv2 driver each time a new SoC is added.
> > >
> > > Yes, the i.MX8M power domain support has been pending for a while. ARM guys rejected this patchset
> > > because they suggest us to use SCMI rather than SiP. But SCMI is only partial suitable for our
> > > case.
> >
> > Can you please point me to the most resent version of the TF-A side
> > implementation of this? The i.MX8MM implementation in the
> > imx_5.4.3_2.0.0 branch in the codeaurora imx-atf repo still contains
> > writes to the clock controller register range.
> >
> > Also I would love to learn why the GPC needs to access Mediamix and
> > VPUmix domain registers. If you are talking about the NOC configuration
> > I would strongly suggest that those should be handled by a Linux side
> > interconnect driver, this has nothing to do with the power domain
> > sequencing, it just happens to lose state over the power down and needs
> > to be reprogrammed after power on. The NOC configuration though is use-
> > case dependent, so this should be properly handled in a rich OS driver.
> >
> > Sure we needs to extend the Linux side GPC driver for each new SoC
> > generation, but that's no different from any other hardware driver in
> > Linux. Drivers are the abstraction around the hardware, there is no
> > need to invent another one if there are no clear benefits.
> >
>
> Jacky / Abel,
>
> Any movement on this? As I see it the lack of imx8mm power-domain
> support in the kernel is holding up USB, PCIe, VPU, and perhaps
> GPU/CSI as well. I would tend to agree that hiding this functionality
> in the TF-A is probably not the best approach especially as that
> requires a NXP version of the TF-A. I really don't see the issue with
> the gpc driver getting a little more complicated if it needs to. There
> is bound to be some complication as there is such a large variation of
> IMX8 products out there! (talk about confusing!).
NXP -
Please don't let perfect be the enemy of the good. We have some
patches floating around which enable much of the functionality. It
may not be perfect, but it's an improvement from what we currently
have - which is nothing. Please let the patches in, and when the
alternative methods become available, we can remove these.
If TF_A is arguing about how to push the patches upstream, then maybe
Tim is wright and it's not a good idea, and we should consider
something else.
adam
>
> Best Regards,
>
> Tim
_______________________________________________
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 v8 13/14] media: platform: Rename jpeg dec file name
From: Tomasz Figa @ 2020-05-21 16:02 UTC (permalink / raw)
To: Xia Jiang
Cc: drinkcat, devicetree, mojahsu, srv_heupstream, Rick Chang,
senozhatsky, linux-kernel, maoguang.meng, Mauro Carvalho Chehab,
sj.huang, Rob Herring, Matthias Brugger, Hans Verkuil,
linux-mediatek, Marek Szyprowski, linux-arm-kernel, linux-media
In-Reply-To: <20200403094033.8288-14-xia.jiang@mediatek.com>
Hi Xia,
On Fri, Apr 03, 2020 at 05:40:32PM +0800, Xia Jiang wrote:
> Rename the files which are for decode feature. This is preparing
> path since the jpeg enc patch will be added later.
>
> Signed-off-by: Xia Jiang <xia.jiang@mediatek.com>
> ---
> v8: no changes
>
> v7: no changes
>
> v6: no changes
>
> v5: no changes
>
> v4: no changes
>
> v3: no changes
>
> v2: no changes
> ---
> drivers/media/platform/mtk-jpeg/Makefile | 2 +-
> drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c | 4 ++--
> .../platform/mtk-jpeg/{mtk_jpeg_hw.c => mtk_jpeg_dec_hw.c} | 2 +-
> .../platform/mtk-jpeg/{mtk_jpeg_hw.h => mtk_jpeg_dec_hw.h} | 2 +-
> .../mtk-jpeg/{mtk_jpeg_parse.c => mtk_jpeg_dec_parse.c} | 2 +-
> .../mtk-jpeg/{mtk_jpeg_parse.h => mtk_jpeg_dec_parse.h} | 2 +-
> .../platform/mtk-jpeg/{mtk_jpeg_reg.h => mtk_jpeg_dec_reg.h} | 0
> 7 files changed, 7 insertions(+), 7 deletions(-)
> rename drivers/media/platform/mtk-jpeg/{mtk_jpeg_hw.c => mtk_jpeg_dec_hw.c} (99%)
> rename drivers/media/platform/mtk-jpeg/{mtk_jpeg_hw.h => mtk_jpeg_dec_hw.h} (98%)
> rename drivers/media/platform/mtk-jpeg/{mtk_jpeg_parse.c => mtk_jpeg_dec_parse.c} (98%)
> rename drivers/media/platform/mtk-jpeg/{mtk_jpeg_parse.h => mtk_jpeg_dec_parse.h} (92%)
> rename drivers/media/platform/mtk-jpeg/{mtk_jpeg_reg.h => mtk_jpeg_dec_reg.h} (100%)
>
Reviewed-by: Tomasz Figa <tfiga@chromium.org>
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 v8 11/14] media: dt-bindings: Add jpeg enc device tree node document
From: Tomasz Figa @ 2020-05-21 16:00 UTC (permalink / raw)
To: Xia Jiang
Cc: drinkcat, devicetree, mojahsu, srv_heupstream, Rick Chang,
senozhatsky, linux-kernel, maoguang.meng, Mauro Carvalho Chehab,
sj.huang, Rob Herring, Matthias Brugger, yong.wu, Hans Verkuil,
linux-mediatek, Marek Szyprowski, linux-arm-kernel, linux-media
In-Reply-To: <20200403094033.8288-12-xia.jiang@mediatek.com>
Hi Xia,
On Fri, Apr 03, 2020 at 05:40:30PM +0800, Xia Jiang wrote:
> Add jpeg enc device tree node document
>
> Reviewed-by: Rob Herring <robh@kernel.org>
> Signed-off-by: Xia Jiang <xia.jiang@mediatek.com>
> ---
> v8: no changes
>
> v7: no changes
>
> v6: no changes
>
> v5: no changes
>
> v4: no changes
>
> v3: change compatible to SoC specific compatible
>
> v2: no changes
> ---
> .../bindings/media/mediatek-jpeg-encoder.txt | 37 +++++++++++++++++++
> 1 file changed, 37 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/media/mediatek-jpeg-encoder.txt
>
Thank you for the patch. Please see my comments inline.
> diff --git a/Documentation/devicetree/bindings/media/mediatek-jpeg-encoder.txt b/Documentation/devicetree/bindings/media/mediatek-jpeg-encoder.txt
> new file mode 100644
> index 000000000000..fa8da699493b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/mediatek-jpeg-encoder.txt
> @@ -0,0 +1,37 @@
> +* MediaTek JPEG Encoder
> +
> +MediaTek JPEG Encoder is the JPEG encode hardware present in MediaTek SoCs
> +
> +Required properties:
> +- compatible : should be one of:
> + "mediatek,mt2701-jpgenc"
> + ...
What does this "..." mean?
> + followed by "mediatek,mtk-jpgenc"
> +- reg : physical base address of the JPEG encoder registers and length of
> + memory mapped region.
> +- interrupts : interrupt number to the interrupt controller.
> +- clocks: device clocks, see
> + Documentation/devicetree/bindings/clock/clock-bindings.txt for details.
> +- clock-names: must contain "jpgenc". It is the clock of JPEG encoder.
nit: In principle the clocks should be named after the function the clock
performs on the consumer side, i.e. the JPEG block in this case, I guess
here it's just a generic clock that does everything, but I guess it comes
from somewhere. Is it the AHB clock or something? In that case it would
normally be called "ahb".
> +- power-domains: a phandle to the power domain, see
> + Documentation/devicetree/bindings/power/power_domain.txt for details.
> +- mediatek,larb: must contain the local arbiters in the current SoCs, see
> + Documentation/devicetree/bindings/memory-controllers/mediatek,smi-larb.txt
> + for details.
I believe this isn't necessary anymore, because larbs are added
automatically by the MTK IOMMU driver using device links. +CC Yong who
worked on that.
> +- iommus: should point to the respective IOMMU block with master port as
> + argument, see Documentation/devicetree/bindings/iommu/mediatek,iommu.txt
> + for details.
> +
> +Example:
> + jpegenc: jpegenc@1500a000 {
> + compatible = "mediatek,mt2701-jpgenc",
> + "mediatek,mtk-jpgenc";
> + reg = <0 0x1500a000 0 0x1000>;
> + interrupts = <GIC_SPI 141 IRQ_TYPE_LEVEL_LOW>;
> + clocks = <&imgsys CLK_IMG_VENC>;
> + clock-names = "jpgenc";
> + power-domains = <&scpsys MT2701_POWER_DOMAIN_ISP>;
> + mediatek,larb = <&larb2>;
Ditto.
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 0/4] Update K3 DSP remoteproc driver for C71x DSPs
From: Suman Anna @ 2020-05-21 15:57 UTC (permalink / raw)
To: Bjorn Andersson, Rob Herring, Mathieu Poirier
Cc: devicetree, Loic Pallardy, Lokesh Vutla, Arnaud Pouliquen,
linux-remoteproc, linux-kernel, Clement Leger, linux-arm-kernel
In-Reply-To: <20200325204701.16862-1-s-anna@ti.com>
On 3/25/20 3:46 PM, Suman Anna wrote:
> Hi All,
>
> This series adds support for a new next generation 64-bit TI DSP based on
> the TMS320C71x CorePac processor subsystem called the C71x. The support is
> enabled through couple of enhancements to the remoteproc core (primarily to
> support a 64-bit trace resource entry), and does depend on the K3 DSP
> remoteproc driver posted earlier today [1].
>
> The loading support leveraged the 64-bit ELF loader support code added by
> Clement and already staged on the rproc-next branch. I am posting this
> series separate from the C66x series because of the new 64-bit resource
> type enhancement needs (patches 2 and 3). I have leveraged the existing
> resource types as is by introducing a new version element, and am open to
> ideas if it is desired to just define it as a separate resource type.
>
> The C71x DSP boots using firmware segments loaded into the DDR with a 2 MB
> aligned address requirement on the boot vectors. There is no support for
> internal memory loading, and all internal memories shall be used as fast
> RAMs/scatchpads by the firmware executing on the DSPs. IPC is through the
> virtio-rpmsg transport. There is no support for Error Recovery, Power
> Management or loading into on-chip SRAMs at present.
>
> Following is the patch summary:
> - Patch 1 updates the K3 DSP bindings for C71x cores
> - Patch 2 introduces a concept of version element into existing resource types
> - Patch 3 adds support for a new 64-bit trace resource entry
> - Patch 4 enhances the K3 DSP remoteproc driver for C71x
I have separated out the C71 platform driver pieces (patches 1 & 4) and
posted a v2 for those.
Appreciate any feedback on the core patches (patches 2 & 3) that add the
minimal 64-bit trace support, as this also sets the direction for
resource extensions. I can post the next version for those based on
feedback.
regards
Suman
>
> regards
> Suman
>
> [1] https://patchwork.kernel.org/cover/11458573/
>
> Suman Anna (4):
> dt-bindings: remoteproc: k3-dsp: Update bindings for C71x DSPs
> remoteproc: introduce version element into resource type field
> remoteproc: add support for a new 64-bit trace version
> remoteproc/k3-dsp: Add support for C71x DSPs
>
> .../bindings/remoteproc/ti,k3-dsp-rproc.yaml | 78 ++++++++++++++++---
> drivers/remoteproc/remoteproc_core.c | 65 +++++++++++-----
> drivers/remoteproc/remoteproc_debugfs.c | 50 ++++++++----
> drivers/remoteproc/ti_k3_dsp_remoteproc.c | 17 ++++
> include/linux/remoteproc.h | 34 +++++++-
> 5 files changed, 203 insertions(+), 41 deletions(-)
>
_______________________________________________
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/2] ARM: Allow either FLATMEM or SPARSEMEM on the multiplatform build
From: Mike Rapoport @ 2020-05-21 15:56 UTC (permalink / raw)
To: Russell King - ARM Linux admin
Cc: Florian Fainelli, Arnd Bergmann, Stephen Boyd, Kevin Cernekee,
Doug Berger, Gregory Fong, linux-arm-kernel
In-Reply-To: <20200521145020.GS1551@shell.armlinux.org.uk>
On Thu, May 21, 2020 at 03:50:20PM +0100, Russell King - ARM Linux admin wrote:
> On Thu, May 21, 2020 at 05:07:45PM +0300, Mike Rapoport wrote:
> > On Thu, May 21, 2020 at 03:33:29PM +0300, Mike Rapoport wrote:
> > > On Thu, May 21, 2020 at 01:03:08PM +0100, Russell King - ARM Linux admin wrote:
> > > > On Thu, May 21, 2020 at 11:18:23AM +0300, Mike Rapoport wrote:
> > > > > (resendig for the correct address and with mailing list cc'ed, sorry for
> > > > > the noise)
> > > > >
> > > > > Hi,
> > > > >
> > > > > Following the discussion at [1], I'm resending the patches that enable
> > > > > memory model selection in menuconfig and such.
> > > > >
> > > > > These patches do not change the way the configuration is generated from the
> > > > > defconfigs and they do not change explicit selection of SPARSEMEM for
> > > > > platforms that have "select ARCH_ENABLE_SPARSEMEM".
> > > > >
> > > > > The mere change is that when a user runs an interactive configuration they
> > > > > will be allowed to select between FLATMEM and SPARSMEM, which is not the
> > > > > case today.
> > > > >
> > > > > There is indeed some awkwardness in, e.g. removal of
> > > > > ARCH_SPARSEMEM_DEFAULT, but this is what memory model selection logic in
> > > > > mm/Kconfig imposes.
> > > > >
> > > > Right, but the question is whether we want to offer flatmem for rpc.
> > > > It isn't allowed today, and so far no one has said why it's a
> > > > desirable change to make.
> > >
> > > With ARCH_RPC=y (or ARCH_SA1100 or ARCH_EP93XX for that matter)
> > > ARCH_MULTIPLATFORM=n which prevents ARCH_SELECT_MEMORY_MODEL from being
> > > enabled and since any of these machines explicitly selects
> > > ARCH_SPARSEMEM_ENABLE, the only available memory model would be
> > > SPARSEMEM.
> > >
> > > I played a bit with menuconfig and if any of the platforms requiring
> > > sparsemem is selected, the menu allowing the user to choose the memory
> > > model disappears.
> >
> > Ah, when either of these patforms will become a part of the
> > multiplatform build, the only option for multiplatform build will be
> > sparsemem.
> > So it would be nice if somebody could check the cost of using sparsemem
> > vs flatmem, espessially on low end machines.
>
> Do you think they will become part of multiplatform?
>
> If they're low-end machines, then adding:
>
> (a) the additional memory overhead of a multiplatform kernel
> (b) the additional runtime overhead of the complexities of multiplatform
> kernels
>
> is surely an odd thing to do, especially when few really care about
> these platforms becoming part of a multiplatform kernel, except those
> who like the idea of multiplat.
I honestly don't know, it was Arnd who was talking about adding ep93xx
to multiplat.
So, probably better phrasing would have been "if either of these
patforms will become a part of the multiplatform build...".
Anyway, benchmarking sparsemem vs flatmem on a platform that is already
a part of multiplat might be useful to understand whether we need both
of them or we can simply make everything use sparsemem.
> --
> RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> FTTC for 0.8m (est. 1762m) line in suburbia: sync at 13.1Mbps down 424kbps up
--
Sincerely yours,
Mike.
_______________________________________________
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] soc: imx: Add power domain driver support for i.mx8m family
From: Tim Harvey @ 2020-05-21 15:56 UTC (permalink / raw)
To: Jacky Bai, Abel Vesa, Aisheng Dong
Cc: Adam Ford, Liam Girdwood, Linux Kernel Mailing List, Mark Brown,
dl-linux-imx, Sascha Hauer, Shawn Guo,
linux-arm-kernel@lists.infradead.org, Lucas Stach
In-Reply-To: <97aa62e8aaf8fa1c462c7db117fca9b6ea9bfec0.camel@pengutronix.de>
On Mon, May 4, 2020 at 2:19 AM Lucas Stach <l.stach@pengutronix.de> wrote:
>
> Am Montag, den 27.04.2020, 15:37 +0000 schrieb Jacky Bai:
> > > -----Original Message-----
> > > From: Lucas Stach <l.stach@pengutronix.de>
> > > Sent: Monday, April 27, 2020 11:11 PM
> > > To: Abel Vesa <abel.vesa@nxp.com>; Jacky Bai <ping.bai@nxp.com>; Shawn
> > > Guo <shawnguo@kernel.org>; Sascha Hauer <kernel@pengutronix.de>; Liam
> > > Girdwood <lgirdwood@gmail.com>; Mark Brown <broonie@kernel.org>
> > > Cc: Aisheng Dong <aisheng.dong@nxp.com>; dl-linux-imx
> > > <linux-imx@nxp.com>; linux-arm-kernel@lists.infradead.org; Linux Kernel
> > > Mailing List <linux-kernel@vger.kernel.org>
> > > Subject: Re: [PATCH] soc: imx: Add power domain driver support for i.mx8m
> > > family
> > >
> > > Am Montag, den 27.04.2020, 17:58 +0300 schrieb Abel Vesa:
> > > > From: Jacky Bai <ping.bai@nxp.com>
> > > >
> > > > The i.MX8M family is a set of NXP product focus on delivering the
> > > > latest and greatest video and audio experience combining
> > > > state-of-the-art media-specific features with high-performance
> > > > processing while optimized for lowest power consumption.
> > > >
> > > > i.MX8MQ, i.MX8MM, i.MX8MN, even the furture i.MX8MP are all belong to
> > > > this family. A GPC module is used to manage all the PU power domain
> > > > on/off. But the situation is that the number of power domains & the
> > > > power up sequence has significate difference on those SoCs. Even on
> > > > the same SoC. The power up sequence still has big difference. It makes
> > > > us hard to reuse the GPCv2 driver to cover the whole i.MX8M family.
> > > > Each time a new SoC is supported in the mainline kernel, we need to
> > > > modify the GPCv2 driver to support it. We need to add or modify
> > > > hundred lines of code in worst case.
> > > > It is a bad practice for the driver maintainability.
> > > >
> > > > This driver add a more generic power domain driver that the actual
> > > > power on/off is done by TF-A code. the abstraction give us the
> > > > possibility that using one driver to cover the whole i.MX8M family in
> > > > kernel side.
> > > >
> > >
> > > Again: what does this driver bring to the table, other than moving a fraction of
> > > the power domain functionality into the firmware?
> > >
> > > The discussions on the last submissions of this driver already established that
> > > we can't move all functionality for the power domains into the firmware, as
> > > controlling regulators is probably not easy to do from this context. Also the
> > > TF-A side implementation of this driver is "interesting" IMHO, it does stuff like
> > > accessing the clock controller registers without any locking or other means of
> > > mutual exclusion with the Linux kernel clock controller driver.
> > >
> >
> > The clock handling is in kernel side through CCF, not in ATF. See the patch below.
> >
> > > Why can't we just extend the existing GPCv2 driver with support for the other
> > > i.MX8M family members?
> > >
> >
> > The reason that why I don’t like to extend the GPCv2 is that when doing domain on/off,
> > We need to access some special control register in each domain & do some special flow,
> > These control register(mediamix block control, vpumix block control) is not in GPC
> > module's address range. No benefit to reuse the GPCv2. Only bring complexity to the
> > GPCv2 driver each time a new SoC is added.
> >
> > Yes, the i.MX8M power domain support has been pending for a while. ARM guys rejected this patchset
> > because they suggest us to use SCMI rather than SiP. But SCMI is only partial suitable for our
> > case.
>
> Can you please point me to the most resent version of the TF-A side
> implementation of this? The i.MX8MM implementation in the
> imx_5.4.3_2.0.0 branch in the codeaurora imx-atf repo still contains
> writes to the clock controller register range.
>
> Also I would love to learn why the GPC needs to access Mediamix and
> VPUmix domain registers. If you are talking about the NOC configuration
> I would strongly suggest that those should be handled by a Linux side
> interconnect driver, this has nothing to do with the power domain
> sequencing, it just happens to lose state over the power down and needs
> to be reprogrammed after power on. The NOC configuration though is use-
> case dependent, so this should be properly handled in a rich OS driver.
>
> Sure we needs to extend the Linux side GPC driver for each new SoC
> generation, but that's no different from any other hardware driver in
> Linux. Drivers are the abstraction around the hardware, there is no
> need to invent another one if there are no clear benefits.
>
Jacky / Abel,
Any movement on this? As I see it the lack of imx8mm power-domain
support in the kernel is holding up USB, PCIe, VPU, and perhaps
GPU/CSI as well. I would tend to agree that hiding this functionality
in the TF-A is probably not the best approach especially as that
requires a NXP version of the TF-A. I really don't see the issue with
the gpc driver getting a little more complicated if it needs to. There
is bound to be some complication as there is such a large variation of
IMX8 products out there! (talk about confusing!).
Best Regards,
Tim
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCHv3 2/2] dt-bindings: arm: coresight: Add support to skip trace unit power up
From: Mathieu Poirier @ 2020-05-21 15:53 UTC (permalink / raw)
To: Sai Prakash Ranjan
Cc: devicetree, Suzuki K Poulose, linux-arm-msm, Coresight ML,
Linux Kernel Mailing List, Stephen Boyd, Rob Herring,
Tingwei Zhang, Leo Yan, linux-arm-kernel, Mike Leach
In-Reply-To: <7b69c9752713ce22f04688e83ec78f8aa67c63dc.1589558615.git.saiprakash.ranjan@codeaurora.org>
On Fri, 15 May 2020 at 10:23, Sai Prakash Ranjan
<saiprakash.ranjan@codeaurora.org> wrote:
>
> From: Tingwei Zhang <tingwei@codeaurora.org>
>
> Add "qcom,skip-power-up" property to identify systems which can
> skip powering up of trace unit since they share the same power
> domain as their CPU core. This is required to identify such
> systems with hardware errata which stops the CPU watchdog counter
> when the power up bit is set (TRCPDCR.PU).
>
> Signed-off-by: Tingwei Zhang <tingwei@codeaurora.org>
> Co-developed-by: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org>
> Signed-off-by: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org>
> ---
> Documentation/devicetree/bindings/arm/coresight.txt | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/arm/coresight.txt b/Documentation/devicetree/bindings/arm/coresight.txt
> index 846f6daae71b..e4b2eda0b53b 100644
> --- a/Documentation/devicetree/bindings/arm/coresight.txt
> +++ b/Documentation/devicetree/bindings/arm/coresight.txt
> @@ -108,6 +108,13 @@ its hardware characteristcs.
> * arm,cp14: must be present if the system accesses ETM/PTM management
> registers via co-processor 14.
>
> + * qcom,skip-power-up: boolean. Indicates that an implementation can
> + skip powering up the trace unit. TRCPDCR.PU does not have to be set
> + on Qualcomm Technologies Inc. systems since ETMs are in the same power
> + domain as their CPU cores. This property is required to identify such
> + systems with hardware errata where the CPU watchdog counter is stopped
> + when TRCPDCR.PU is set.
> +
> * Optional property for TMC:
Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org>
>
> * arm,buffer-size: size of contiguous buffer space for TMC ETR
> --
> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
> of Code Aurora Forum, hosted by The Linux Foundation
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v2] perf: arm_dsu: Support DSU ACPI devices.
From: Tuan Phan @ 2020-05-21 15:50 UTC (permalink / raw)
To: Sudeep Holla
Cc: Mark Rutland, Tuan Phan, linux-kernel, patches, Will Deacon,
linux-arm-kernel
In-Reply-To: <20200521135557.GB12282@bogus>
> On May 21, 2020, at 6:55 AM, Sudeep Holla <sudeep.holla@arm.com> wrote:
>
> On Mon, May 11, 2020 at 01:32:40PM -0700, Tuan Phan wrote:
>> Add ACPI node probing device support. Each DSU ACPI node
>> defines a "cpus" package with a per cpu MPIDR element.
>>
>> Signed-off-by: Tuan Phan <tuanphan@os.amperecomputing.com>
>> ---
>> Changes in v2:
>> - Removed IRQF_SHARED.
>> - Fixed ACPI runtime detection.
>>
>> The ACPI binding spec for DSU ACPI node is under beta and located
>> in ARM server group under project "ACPI on ARM".
>>
>> drivers/perf/arm_dsu_pmu.c | 71 ++++++++++++++++++++++++++++++++++++++++------
>> 1 file changed, 63 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/perf/arm_dsu_pmu.c b/drivers/perf/arm_dsu_pmu.c
>> index 70968c8..784d177 100644
>> --- a/drivers/perf/arm_dsu_pmu.c
>> +++ b/drivers/perf/arm_dsu_pmu.c
>> @@ -11,6 +11,7 @@
>> #define DRVNAME PMUNAME "_pmu"
>> #define pr_fmt(fmt) DRVNAME ": " fmt
>>
>> +#include <linux/acpi.h>
>> #include <linux/bitmap.h>
>> #include <linux/bitops.h>
>> #include <linux/bug.h>
>> @@ -603,18 +604,21 @@ static struct dsu_pmu *dsu_pmu_alloc(struct platform_device *pdev)
>> }
>>
>> /**
>> - * dsu_pmu_dt_get_cpus: Get the list of CPUs in the cluster.
>> + * dsu_pmu_dt_get_cpus: Get the list of CPUs in the cluster
>> + * from device tree.
>> */
>> -static int dsu_pmu_dt_get_cpus(struct device_node *dev, cpumask_t *mask)
>> +static int dsu_pmu_dt_get_cpus(struct platform_device *pdev)
>> {
>> int i = 0, n, cpu;
>> struct device_node *cpu_node;
>> + struct dsu_pmu *dsu_pmu =
>> + (struct dsu_pmu *) platform_get_drvdata(pdev);
>>
>> - n = of_count_phandle_with_args(dev, "cpus", NULL);
>> + n = of_count_phandle_with_args(pdev->dev.of_node, "cpus", NULL);
>> if (n <= 0)
>> return -ENODEV;
>> for (; i < n; i++) {
>> - cpu_node = of_parse_phandle(dev, "cpus", i);
>> + cpu_node = of_parse_phandle(pdev->dev.of_node, "cpus", i);
>> if (!cpu_node)
>> break;
>> cpu = of_cpu_node_to_id(cpu_node);
>> @@ -626,11 +630,54 @@ static int dsu_pmu_dt_get_cpus(struct device_node *dev, cpumask_t *mask)
>> */
>> if (cpu < 0)
>> continue;
>> - cpumask_set_cpu(cpu, mask);
>> + cpumask_set_cpu(cpu, &dsu_pmu->associated_cpus);
>> }
>> return 0;
>> }
>>
>> +/**
>> + * dsu_pmu_acpi_get_cpus: Get the list of CPUs in the cluster
>> + * from ACPI.
>> + */
>> +static int dsu_pmu_acpi_get_cpus(struct platform_device *pdev)
>> +{
>> + int i, cpu, ret;
>> + const union acpi_object *obj;
>> + struct acpi_device *adev = ACPI_COMPANION(&pdev->dev);
>> + struct dsu_pmu *dsu_pmu =
>> + (struct dsu_pmu *) platform_get_drvdata(pdev);
>> +
>> + ret = acpi_dev_get_property(adev, "cpus", ACPI_TYPE_PACKAGE, &obj);
>
> I don't see any property "cpus" in the document:
> DEN 0093 A (Generic ACPI for Arm Components 1.0) [1]
>
> Is there any newer updates that I need to look at ?
>
There is a newer DEN 0093 A version but it has not been public yet. Let abandon this
patch and I will post new one when the document has been finalized and available to public.
Thanks,
> --
> Regards,
> Sudeep
>
> [1] https://static.docs.arm.com/den0093/a/DEN0093_ACPI_Arm_Components_1.0.pdf
_______________________________________________
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 v8 10/14] media: platform: Delete redundant code for improving code quality
From: Tomasz Figa @ 2020-05-21 15:49 UTC (permalink / raw)
To: Xia Jiang
Cc: drinkcat, devicetree, mojahsu, srv_heupstream, Rick Chang,
senozhatsky, linux-kernel, maoguang.meng, Mauro Carvalho Chehab,
sj.huang, Rob Herring, Matthias Brugger, Hans Verkuil,
linux-mediatek, Marek Szyprowski, linux-arm-kernel, linux-media
In-Reply-To: <20200403094033.8288-11-xia.jiang@mediatek.com>
Hi Xia,
On Fri, Apr 03, 2020 at 05:40:29PM +0800, Xia Jiang wrote:
> Delete unused member variables annotation.
> Delete unused variable definition.
> Delete redundant log print, because V4L2 debug logs already print it.
>
> Signed-off-by: Xia Jiang <xia.jiang@mediatek.com>
> ---
> v8: no changes
> ---
> drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c | 16 ++--------------
> drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h | 5 +++--
> 2 files changed, 5 insertions(+), 16 deletions(-)
>
Thank you for the patch. Please see my comments inline.
> diff --git a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
> index 4e64046a6854..9e59b9a51ef0 100644
> --- a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
> +++ b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
> @@ -182,7 +182,6 @@ static int mtk_jpeg_try_fmt_mplane(struct v4l2_format *f,
> struct mtk_jpeg_ctx *ctx, int q_type)
> {
> struct v4l2_pix_format_mplane *pix_mp = &f->fmt.pix_mp;
> - struct mtk_jpeg_dev *jpeg = ctx->jpeg;
> int i;
>
> memset(pix_mp->reserved, 0, sizeof(pix_mp->reserved));
> @@ -190,7 +189,7 @@ static int mtk_jpeg_try_fmt_mplane(struct v4l2_format *f,
>
> if (ctx->state != MTK_JPEG_INIT) {
> mtk_jpeg_adjust_fmt_mplane(ctx, f);
> - goto end;
> + return 0;
> }
>
> pix_mp->num_planes = fmt->colplanes;
> @@ -210,7 +209,7 @@ static int mtk_jpeg_try_fmt_mplane(struct v4l2_format *f,
> pfmt->sizeimage = round_up(pfmt->sizeimage, 128);
> if (pfmt->sizeimage == 0)
> pfmt->sizeimage = MTK_JPEG_DEFAULT_SIZEIMAGE;
> - goto end;
> + return 0;
> }
>
> /* type is MTK_JPEG_FMT_TYPE_CAPTURE */
> @@ -224,20 +223,9 @@ static int mtk_jpeg_try_fmt_mplane(struct v4l2_format *f,
> u32 stride = pix_mp->width * fmt->h_sample[i] / 4;
> u32 h = pix_mp->height * fmt->v_sample[i] / 4;
>
> - memset(pfmt->reserved, 0, sizeof(pfmt->reserved));
This change is not mentioned in the description. I'd suggest moving it
to a separate patch, because it's a functional change.
> pfmt->bytesperline = stride;
> pfmt->sizeimage = stride * h;
> }
> -end:
> - v4l2_dbg(2, debug, &jpeg->v4l2_dev, "wxh:%ux%u\n",
> - pix_mp->width, pix_mp->height);
> - for (i = 0; i < pix_mp->num_planes; i++) {
> - v4l2_dbg(2, debug, &jpeg->v4l2_dev,
> - "plane[%d] bpl=%u, size=%u\n",
> - i,
> - pix_mp->plane_fmt[i].bytesperline,
> - pix_mp->plane_fmt[i].sizeimage);
> - }
> return 0;
> }
>
> diff --git a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h
> index 64a731261214..9bbd615b1067 100644
> --- a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h
> +++ b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h
> @@ -30,6 +30,9 @@
>
> #define MTK_JPEG_DEFAULT_SIZEIMAGE (1 * 1024 * 1024)
>
> +/**
> + * enum mtk_jpeg_ctx_state - contex state of jpeg
typo: s/contex/context/
But I'd rephrase it to "states of the context state machine".
> + */
Not mentioned in the description. Also, the documentation of an enum
should have descriptions for the values.
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 v8 09/14] media: platform: Change MTK_JPEG_COMP_MAX macro definition location
From: Tomasz Figa @ 2020-05-21 15:44 UTC (permalink / raw)
To: Xia Jiang
Cc: drinkcat, devicetree, mojahsu, srv_heupstream, Rick Chang,
senozhatsky, linux-kernel, maoguang.meng, Mauro Carvalho Chehab,
sj.huang, Rob Herring, Matthias Brugger, Hans Verkuil,
linux-mediatek, Marek Szyprowski, linux-arm-kernel, linux-media
In-Reply-To: <20200403094033.8288-10-xia.jiang@mediatek.com>
On Fri, Apr 03, 2020 at 05:40:28PM +0800, Xia Jiang wrote:
> Move MTK_JPEG_COMP_MAX definition to mtk_jpeg_core.h file, because it
> is used by mtk_jpeg_core.c file.
>
> Signed-off-by: Xia Jiang <xia.jiang@mediatek.com>
> ---
> v8: no changes
> ---
> drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h | 2 ++
> drivers/media/platform/mtk-jpeg/mtk_jpeg_reg.h | 1 -
> 2 files changed, 2 insertions(+), 1 deletion(-)
>
Reviewed-by: Tomasz Figa <tfiga@chromium.org>
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 v8 07/14] media: platform: Use kernel native functions for improving code quality
From: Tomasz Figa @ 2020-05-21 15:41 UTC (permalink / raw)
To: Xia Jiang
Cc: drinkcat, devicetree, mojahsu, srv_heupstream, Rick Chang,
senozhatsky, linux-kernel, maoguang.meng, Mauro Carvalho Chehab,
sj.huang, Rob Herring, Matthias Brugger, Hans Verkuil,
linux-mediatek, Marek Szyprowski, linux-arm-kernel, linux-media
In-Reply-To: <20200403094033.8288-8-xia.jiang@mediatek.com>
Hi Xia,
On Fri, Apr 03, 2020 at 05:40:26PM +0800, Xia Jiang wrote:
Thank you for the patch. Please see my comments inline.
nit: I'd remove "for improving code quality" from the subject, as it's
obvious that we don't intend to make the code quality worse. ;)
On the contrary, I'd make it more specific, e.g.
media: mtk-jpeg: Use generic rounding helpers
WDYT?
> Use clamp() to replace mtk_jpeg_bound_align_image() and round() to
> replace mtk_jpeg_align().
>
> Signed-off-by: Xia Jiang <xia.jiang@mediatek.com>
> ---
> v8: no changes
> ---
> .../media/platform/mtk-jpeg/mtk_jpeg_core.c | 41 +++++--------------
> .../media/platform/mtk-jpeg/mtk_jpeg_core.h | 8 ++--
> drivers/media/platform/mtk-jpeg/mtk_jpeg_hw.c | 8 ++--
> drivers/media/platform/mtk-jpeg/mtk_jpeg_hw.h | 5 ---
> 4 files changed, 19 insertions(+), 43 deletions(-)
>
> diff --git a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
> index 2fa3711fdc9b..4e64046a6854 100644
> --- a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
> +++ b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
> @@ -157,25 +157,6 @@ static struct mtk_jpeg_fmt *mtk_jpeg_find_format(struct mtk_jpeg_ctx *ctx,
> return NULL;
> }
>
> -static void mtk_jpeg_bound_align_image(u32 *w, unsigned int wmin,
> - unsigned int wmax, unsigned int walign,
> - u32 *h, unsigned int hmin,
> - unsigned int hmax, unsigned int halign)
> -{
> - int width, height, w_step, h_step;
> -
> - width = *w;
> - height = *h;
> - w_step = 1 << walign;
> - h_step = 1 << halign;
> -
> - v4l_bound_align_image(w, wmin, wmax, walign, h, hmin, hmax, halign, 0);
> - if (*w < width && (*w + w_step) <= wmax)
> - *w += w_step;
> - if (*h < height && (*h + h_step) <= hmax)
> - *h += h_step;
> -}
> -
> static void mtk_jpeg_adjust_fmt_mplane(struct mtk_jpeg_ctx *ctx,
> struct v4l2_format *f)
> {
> @@ -218,25 +199,25 @@ static int mtk_jpeg_try_fmt_mplane(struct v4l2_format *f,
> if (q_type == MTK_JPEG_FMT_TYPE_OUTPUT) {
> struct v4l2_plane_pix_format *pfmt = &pix_mp->plane_fmt[0];
>
> - mtk_jpeg_bound_align_image(&pix_mp->width, MTK_JPEG_MIN_WIDTH,
> - MTK_JPEG_MAX_WIDTH, 0,
> - &pix_mp->height, MTK_JPEG_MIN_HEIGHT,
> - MTK_JPEG_MAX_HEIGHT, 0);
> + pix_mp->height = clamp(pix_mp->height, MTK_JPEG_MIN_HEIGHT,
> + MTK_JPEG_MAX_HEIGHT);
> + pix_mp->width = clamp(pix_mp->width, MTK_JPEG_MIN_WIDTH,
> + MTK_JPEG_MAX_WIDTH);
>
> memset(pfmt->reserved, 0, sizeof(pfmt->reserved));
> pfmt->bytesperline = 0;
> /* Source size must be aligned to 128 */
> - pfmt->sizeimage = mtk_jpeg_align(pfmt->sizeimage, 128);
> + pfmt->sizeimage = round_up(pfmt->sizeimage, 128);
> if (pfmt->sizeimage == 0)
> pfmt->sizeimage = MTK_JPEG_DEFAULT_SIZEIMAGE;
> goto end;
> }
>
> /* type is MTK_JPEG_FMT_TYPE_CAPTURE */
> - mtk_jpeg_bound_align_image(&pix_mp->width, MTK_JPEG_MIN_WIDTH,
> - MTK_JPEG_MAX_WIDTH, fmt->h_align,
> - &pix_mp->height, MTK_JPEG_MIN_HEIGHT,
> - MTK_JPEG_MAX_HEIGHT, fmt->v_align);
> + pix_mp->height = clamp(round_up(pix_mp->height, fmt->v_align),
> + MTK_JPEG_MIN_HEIGHT, MTK_JPEG_MAX_HEIGHT);
> + pix_mp->width = clamp(round_up(pix_mp->width, fmt->h_align),
> + MTK_JPEG_MIN_WIDTH, MTK_JPEG_MAX_WIDTH);
>
> for (i = 0; i < fmt->colplanes; i++) {
> struct v4l2_plane_pix_format *pfmt = &pix_mp->plane_fmt[i];
> @@ -751,8 +732,8 @@ static void mtk_jpeg_set_dec_src(struct mtk_jpeg_ctx *ctx,
> {
> bs->str_addr = vb2_dma_contig_plane_dma_addr(src_buf, 0);
> bs->end_addr = bs->str_addr +
> - mtk_jpeg_align(vb2_get_plane_payload(src_buf, 0), 16);
> - bs->size = mtk_jpeg_align(vb2_plane_size(src_buf, 0), 128);
> + round_up(vb2_get_plane_payload(src_buf, 0), 16);
> + bs->size = round_up(vb2_plane_size(src_buf, 0), 128);
> }
>
> static int mtk_jpeg_set_dec_dst(struct mtk_jpeg_ctx *ctx,
> diff --git a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h
> index 999bd1427809..28e9b30ad5c3 100644
> --- a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h
> +++ b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h
> @@ -21,10 +21,10 @@
> #define MTK_JPEG_FMT_TYPE_OUTPUT 1
> #define MTK_JPEG_FMT_TYPE_CAPTURE 2
>
> -#define MTK_JPEG_MIN_WIDTH 32
> -#define MTK_JPEG_MIN_HEIGHT 32
> -#define MTK_JPEG_MAX_WIDTH 8192
> -#define MTK_JPEG_MAX_HEIGHT 8192
> +#define MTK_JPEG_MIN_WIDTH 32U
> +#define MTK_JPEG_MIN_HEIGHT 32U
> +#define MTK_JPEG_MAX_WIDTH 8192U
> +#define MTK_JPEG_MAX_HEIGHT 8192U
This change is not mentioned in the commit message. It should go to a
separate patch, possibly merged with other really minor stylistic changes
like this, e.g. patch 08/14.
Otherwise the patch looks good, so after addressing the above minor changes
please feel free to add
Reviewed-by: Tomasz Figa <tfiga@chromium.org>
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 2/4] arm: dts: mt2712: add uart APDMA to device tree
From: Matthias Brugger @ 2020-05-21 15:35 UTC (permalink / raw)
To: Long Cheng, Vinod Koul, Randy Dunlap, Rob Herring, Mark Rutland,
Ryder Lee, Sean Wang, Nicolas Boichat
Cc: Zhenbao Liu, devicetree, srv_heupstream, Greg Kroah-Hartman,
Sean Wang, linux-kernel, YT Shen, linux-mediatek, linux-serial,
Jiri Slaby, dmaengine, Yingjoe Chen, Dan Williams,
linux-arm-kernel
In-Reply-To: <1556336193-15198-3-git-send-email-long.cheng@mediatek.com>
On 27/04/2019 05:36, Long Cheng wrote:
> 1. add uart APDMA controller device node
> 2. add uart 0/1/2/3/4/5 DMA function
>
> Signed-off-by: Long Cheng <long.cheng@mediatek.com>
Queued now for v5.7-next/dts64
Thanks!
> ---
> arch/arm64/boot/dts/mediatek/mt2712e.dtsi | 51 +++++++++++++++++++++++++++++
> 1 file changed, 51 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/mediatek/mt2712e.dtsi b/arch/arm64/boot/dts/mediatek/mt2712e.dtsi
> index 976d92a..f1e419e 100644
> --- a/arch/arm64/boot/dts/mediatek/mt2712e.dtsi
> +++ b/arch/arm64/boot/dts/mediatek/mt2712e.dtsi
> @@ -300,6 +300,9 @@
> interrupts = <GIC_SPI 127 IRQ_TYPE_LEVEL_LOW>;
> clocks = <&baud_clk>, <&sys_clk>;
> clock-names = "baud", "bus";
> + dmas = <&apdma 10
> + &apdma 11>;
> + dma-names = "tx", "rx";
> status = "disabled";
> };
>
> @@ -369,6 +372,39 @@
> (GIC_CPU_MASK_RAW(0x13) | IRQ_TYPE_LEVEL_HIGH)>;
> };
>
> + apdma: dma-controller@11000400 {
> + compatible = "mediatek,mt2712-uart-dma",
> + "mediatek,mt6577-uart-dma";
> + reg = <0 0x11000400 0 0x80>,
> + <0 0x11000480 0 0x80>,
> + <0 0x11000500 0 0x80>,
> + <0 0x11000580 0 0x80>,
> + <0 0x11000600 0 0x80>,
> + <0 0x11000680 0 0x80>,
> + <0 0x11000700 0 0x80>,
> + <0 0x11000780 0 0x80>,
> + <0 0x11000800 0 0x80>,
> + <0 0x11000880 0 0x80>,
> + <0 0x11000900 0 0x80>,
> + <0 0x11000980 0 0x80>;
> + interrupts = <GIC_SPI 103 IRQ_TYPE_LEVEL_LOW>,
> + <GIC_SPI 104 IRQ_TYPE_LEVEL_LOW>,
> + <GIC_SPI 105 IRQ_TYPE_LEVEL_LOW>,
> + <GIC_SPI 106 IRQ_TYPE_LEVEL_LOW>,
> + <GIC_SPI 107 IRQ_TYPE_LEVEL_LOW>,
> + <GIC_SPI 108 IRQ_TYPE_LEVEL_LOW>,
> + <GIC_SPI 109 IRQ_TYPE_LEVEL_LOW>,
> + <GIC_SPI 110 IRQ_TYPE_LEVEL_LOW>,
> + <GIC_SPI 111 IRQ_TYPE_LEVEL_LOW>,
> + <GIC_SPI 112 IRQ_TYPE_LEVEL_LOW>,
> + <GIC_SPI 113 IRQ_TYPE_LEVEL_LOW>,
> + <GIC_SPI 114 IRQ_TYPE_LEVEL_LOW>;
> + dma-requests = <12>;
> + clocks = <&pericfg CLK_PERI_AP_DMA>;
> + clock-names = "apdma";
> + #dma-cells = <1>;
> + };
> +
> auxadc: adc@11001000 {
> compatible = "mediatek,mt2712-auxadc";
> reg = <0 0x11001000 0 0x1000>;
> @@ -385,6 +421,9 @@
> interrupts = <GIC_SPI 91 IRQ_TYPE_LEVEL_LOW>;
> clocks = <&baud_clk>, <&sys_clk>;
> clock-names = "baud", "bus";
> + dmas = <&apdma 0
> + &apdma 1>;
> + dma-names = "tx", "rx";
> status = "disabled";
> };
>
> @@ -395,6 +434,9 @@
> interrupts = <GIC_SPI 92 IRQ_TYPE_LEVEL_LOW>;
> clocks = <&baud_clk>, <&sys_clk>;
> clock-names = "baud", "bus";
> + dmas = <&apdma 2
> + &apdma 3>;
> + dma-names = "tx", "rx";
> status = "disabled";
> };
>
> @@ -405,6 +447,9 @@
> interrupts = <GIC_SPI 93 IRQ_TYPE_LEVEL_LOW>;
> clocks = <&baud_clk>, <&sys_clk>;
> clock-names = "baud", "bus";
> + dmas = <&apdma 4
> + &apdma 5>;
> + dma-names = "tx", "rx";
> status = "disabled";
> };
>
> @@ -415,6 +460,9 @@
> interrupts = <GIC_SPI 94 IRQ_TYPE_LEVEL_LOW>;
> clocks = <&baud_clk>, <&sys_clk>;
> clock-names = "baud", "bus";
> + dmas = <&apdma 6
> + &apdma 7>;
> + dma-names = "tx", "rx";
> status = "disabled";
> };
>
> @@ -629,6 +677,9 @@
> interrupts = <GIC_SPI 126 IRQ_TYPE_LEVEL_LOW>;
> clocks = <&baud_clk>, <&sys_clk>;
> clock-names = "baud", "bus";
> + dmas = <&apdma 8
> + &apdma 9>;
> + dma-names = "tx", "rx";
> status = "disabled";
> };
>
>
_______________________________________________
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 v8 06/14] media: platform: Improve the implementation of the system PM ops
From: Tomasz Figa @ 2020-05-21 15:32 UTC (permalink / raw)
To: Xia Jiang
Cc: drinkcat, devicetree, mojahsu, srv_heupstream, Rick Chang,
senozhatsky, linux-kernel, maoguang.meng, Mauro Carvalho Chehab,
sj.huang, Rob Herring, Matthias Brugger, Hans Verkuil,
linux-mediatek, Marek Szyprowski, linux-arm-kernel, linux-media
In-Reply-To: <20200403094033.8288-7-xia.jiang@mediatek.com>
Hi Xia,
On Fri, Apr 03, 2020 at 05:40:25PM +0800, Xia Jiang wrote:
> Cancel reset hw operation in suspend and resume function because this
> will be done in device_run().
This and...
> Add spin_lock and unlock operation in irq and resume function to make
> sure that the current frame is processed completely before suspend.
...this are two separate changes. Please split.
>
> Signed-off-by: Xia Jiang <xia.jiang@mediatek.com>
> ---
> drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c | 11 +++++++++--
> 1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
> index dd5cadd101ef..2fa3711fdc9b 100644
> --- a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
> +++ b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
> @@ -911,6 +911,8 @@ static irqreturn_t mtk_jpeg_dec_irq(int irq, void *priv)
> u32 dec_ret;
> int i;
>
> + spin_lock(&jpeg->hw_lock);
> +
nit: For consistency, it is recommended to always use the same, i.e. the
strongest, spin_(un)lock_ primitives when operating on the same spinlock.
In this case it would be the irqsave(restore) variants.
> dec_ret = mtk_jpeg_dec_get_int_status(jpeg->dec_reg_base);
> dec_irq_ret = mtk_jpeg_dec_enum_result(dec_ret);
> ctx = v4l2_m2m_get_curr_priv(jpeg->m2m_dev);
> @@ -941,6 +943,7 @@ static irqreturn_t mtk_jpeg_dec_irq(int irq, void *priv)
> v4l2_m2m_buf_done(src_buf, buf_state);
> v4l2_m2m_buf_done(dst_buf, buf_state);
> v4l2_m2m_job_finish(jpeg->m2m_dev, ctx->fh.m2m_ctx);
> + spin_unlock(&jpeg->hw_lock);
> pm_runtime_put_sync(ctx->jpeg->dev);
> return IRQ_HANDLED;
> }
> @@ -1191,7 +1194,6 @@ static __maybe_unused int mtk_jpeg_pm_suspend(struct device *dev)
> {
> struct mtk_jpeg_dev *jpeg = dev_get_drvdata(dev);
>
> - mtk_jpeg_dec_reset(jpeg->dec_reg_base);
> mtk_jpeg_clk_off(jpeg);
>
> return 0;
> @@ -1202,19 +1204,24 @@ static __maybe_unused int mtk_jpeg_pm_resume(struct device *dev)
> struct mtk_jpeg_dev *jpeg = dev_get_drvdata(dev);
>
> mtk_jpeg_clk_on(jpeg);
> - mtk_jpeg_dec_reset(jpeg->dec_reg_base);
>
> return 0;
> }
>
> static __maybe_unused int mtk_jpeg_suspend(struct device *dev)
> {
> + struct mtk_jpeg_dev *jpeg = dev_get_drvdata(dev);
> + unsigned long flags;
> int ret;
>
> if (pm_runtime_suspended(dev))
> return 0;
>
> + spin_lock_irqsave(&jpeg->hw_lock, flags);
What does this spinlock protect us from? I can see that it would prevent
the interrupt handler from being called, but is it okay to suspend the
system without handling the interrupt?
> +
> ret = mtk_jpeg_pm_suspend(dev);
> +
Looking at the implementation of mtk_jpeg_pm_suspend(), all it does is
disabling the clock. How do we make sure that there is no frame currently
being processed by the hardware?
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 v8 05/14] media: platform: Improve power on and power off flow
From: Tomasz Figa @ 2020-05-21 15:22 UTC (permalink / raw)
To: Xia Jiang
Cc: drinkcat, devicetree, mojahsu, srv_heupstream, Rick Chang,
senozhatsky, linux-kernel, maoguang.meng, Mauro Carvalho Chehab,
sj.huang, Rob Herring, Matthias Brugger, Hans Verkuil,
linux-mediatek, Marek Szyprowski, linux-arm-kernel, linux-media
In-Reply-To: <20200403094033.8288-6-xia.jiang@mediatek.com>
Hi Xia,
On Fri, Apr 03, 2020 at 05:40:24PM +0800, Xia Jiang wrote:
> Call pm_runtime_get_sync() before starting a frame and then
> pm_runtime_put() after completing it. This can save power for the time
> between processing two frames.
>
> Signed-off-by: Xia Jiang <xia.jiang@mediatek.com>
> ---
> .../media/platform/mtk-jpeg/mtk_jpeg_core.c | 27 +++++--------------
> 1 file changed, 6 insertions(+), 21 deletions(-)
>
Thank you for the patch. Please see my comments inline.
> diff --git a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
> index a536fa95b3d6..dd5cadd101ef 100644
> --- a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
> +++ b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
> @@ -710,23 +710,6 @@ static struct vb2_v4l2_buffer *mtk_jpeg_buf_remove(struct mtk_jpeg_ctx *ctx,
> return v4l2_m2m_dst_buf_remove(ctx->fh.m2m_ctx);
> }
>
> -static int mtk_jpeg_start_streaming(struct vb2_queue *q, unsigned int count)
> -{
> - struct mtk_jpeg_ctx *ctx = vb2_get_drv_priv(q);
> - struct vb2_v4l2_buffer *vb;
> - int ret = 0;
> -
> - ret = pm_runtime_get_sync(ctx->jpeg->dev);
> - if (ret < 0)
> - goto err;
> -
> - return 0;
> -err:
> - while ((vb = mtk_jpeg_buf_remove(ctx, q->type)))
> - v4l2_m2m_buf_done(vb, VB2_BUF_STATE_QUEUED);
> - return ret;
> -}
> -
> static void mtk_jpeg_stop_streaming(struct vb2_queue *q)
> {
> struct mtk_jpeg_ctx *ctx = vb2_get_drv_priv(q);
> @@ -751,8 +734,6 @@ static void mtk_jpeg_stop_streaming(struct vb2_queue *q)
>
> while ((vb = mtk_jpeg_buf_remove(ctx, q->type)))
> v4l2_m2m_buf_done(vb, VB2_BUF_STATE_ERROR);
> -
> - pm_runtime_put_sync(ctx->jpeg->dev);
> }
>
> static const struct vb2_ops mtk_jpeg_qops = {
> @@ -761,7 +742,6 @@ static const struct vb2_ops mtk_jpeg_qops = {
> .buf_queue = mtk_jpeg_buf_queue,
> .wait_prepare = vb2_ops_wait_prepare,
> .wait_finish = vb2_ops_wait_finish,
> - .start_streaming = mtk_jpeg_start_streaming,
> .stop_streaming = mtk_jpeg_stop_streaming,
> };
>
> @@ -812,7 +792,7 @@ static void mtk_jpeg_device_run(void *priv)
> struct mtk_jpeg_src_buf *jpeg_src_buf;
> struct mtk_jpeg_bs bs;
> struct mtk_jpeg_fb fb;
> - int i;
> + int i, ret;
>
> src_buf = v4l2_m2m_next_src_buf(ctx->fh.m2m_ctx);
> dst_buf = v4l2_m2m_next_dst_buf(ctx->fh.m2m_ctx);
> @@ -832,6 +812,10 @@ static void mtk_jpeg_device_run(void *priv)
> return;
> }
>
> + ret = pm_runtime_get_sync(jpeg->dev);
> + if (ret < 0)
> + goto dec_end;
> +
> mtk_jpeg_set_dec_src(ctx, &src_buf->vb2_buf, &bs);
> if (mtk_jpeg_set_dec_dst(ctx, &jpeg_src_buf->dec_param, &dst_buf->vb2_buf, &fb))
> goto dec_end;
> @@ -957,6 +941,7 @@ static irqreturn_t mtk_jpeg_dec_irq(int irq, void *priv)
> v4l2_m2m_buf_done(src_buf, buf_state);
> v4l2_m2m_buf_done(dst_buf, buf_state);
> v4l2_m2m_job_finish(jpeg->m2m_dev, ctx->fh.m2m_ctx);
> + pm_runtime_put_sync(ctx->jpeg->dev);
The _sync variant explicitly waits until the asynchronous PM operation
completes. This is usually undesired, because the CPU stays blocked for
no good reason. In this context it is actually a bug, because this is an
interrupt handler and it's not allowed to sleep. I wonder why this
actually didn't crash in your testing. Please change to the regular
pm_runtime_put().
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] firmware: smccc: Fix missing prototype warning for arm_smccc_version_init
From: Will Deacon @ 2020-05-21 15:19 UTC (permalink / raw)
To: Sudeep Holla, linux-arm-kernel
Cc: Mark Rutland, Lorenzo Pieralisi, Arnd Bergmann, catalin.marinas,
linux-kernel, Steven Price, Will Deacon
In-Reply-To: <20200521110836.57252-1-sudeep.holla@arm.com>
On Thu, 21 May 2020 12:08:36 +0100, Sudeep Holla wrote:
> Commit f2ae97062a48 ("firmware: smccc: Refactor SMCCC specific bits into
> separate file") introduced the following build warning:
>
> drivers/firmware/smccc/smccc.c:14:13: warning: no previous prototype for
> function 'arm_smccc_version_init' [-Wmissing-prototypes]
> void __init arm_smccc_version_init(u32 version, enum arm_smccc_conduit conduit)
> ^~~~~~~~~~~~~~~~~~~~~~
>
> [...]
Applied to arm64 (for-next/smccc), thanks!
[1/1] firmware: smccc: Fix missing prototype warning for arm_smccc_version_init
https://git.kernel.org/arm64/c/269fd61e15d7
Cheers,
--
Will
https://fixes.arm64.dev
https://next.arm64.dev
https://will.arm64.dev
_______________________________________________
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 V4 00/17] arm64/cpufeature: Introduce ID_PFR2, ID_DFR1, ID_MMFR5 and other changes
From: Will Deacon @ 2020-05-21 15:19 UTC (permalink / raw)
To: Anshuman Khandual, linux-arm-kernel
Cc: catalin.marinas, Will Deacon, kvmarm, linux-kernel, maz
In-Reply-To: <1589881254-10082-1-git-send-email-anshuman.khandual@arm.com>
On Tue, 19 May 2020 15:10:37 +0530, Anshuman Khandual wrote:
> This series is primarily motivated from an adhoc list from Mark Rutland
> during our previous ID_ISAR6 discussion [1]. The current proposal also
> accommodates some more suggestions from Will and Suzuki.
>
> This series adds missing 32 bit system registers (ID_PFR2, ID_DFR1 and
> ID_MMFR5), adds missing features bits on all existing system registers
> (32 and 64 bit) and some other miscellaneous changes. While here it also
> includes a patch which does macro replacement for various open bits shift
> encodings for various CPU ID registers. There is a slight re-order of the
> patches here as compared to the previous version (V1).
>
> [...]
Applied to arm64 (for-next/cpufeature), thanks!
[01/17] arm64/cpufeature: Add explicit ftr_id_isar0[] for ID_ISAR0 register
https://git.kernel.org/arm64/c/2a5bc6c47bc3
[02/17] arm64/cpufeature: Drop TraceFilt feature exposure from ID_DFR0 register
https://git.kernel.org/arm64/c/1ed1b90a0594
[03/17] arm64/cpufeature: Make doublelock a signed feature in ID_AA64DFR0
https://git.kernel.org/arm64/c/e965bcb06256
[04/17] arm64/cpufeature: Introduce ID_PFR2 CPU register
https://git.kernel.org/arm64/c/16824085a7dd
[05/17] arm64/cpufeature: Introduce ID_DFR1 CPU register
https://git.kernel.org/arm64/c/dd35ec070457
[06/17] arm64/cpufeature: Introduce ID_MMFR5 CPU register
https://git.kernel.org/arm64/c/152accf8476f
[07/17] arm64/cpufeature: Add remaining feature bits in ID_PFR0 register
https://git.kernel.org/arm64/c/0ae43a99fe91
[08/17] arm64/cpufeature: Add remaining feature bits in ID_MMFR4 register
https://git.kernel.org/arm64/c/fcd6535322cc
[09/17] arm64/cpufeature: Add remaining feature bits in ID_AA64ISAR0 register
https://git.kernel.org/arm64/c/7cd51a5a84d1
[10/17] arm64/cpufeature: Add remaining feature bits in ID_AA64PFR0 register
https://git.kernel.org/arm64/c/011e5f5bf529
[11/17] arm64/cpufeature: Add remaining feature bits in ID_AA64PFR1 register
https://git.kernel.org/arm64/c/14e270fa5c4c
[12/17] arm64/cpufeature: Add remaining feature bits in ID_AA64MMFR0 register
(no commit info)
[13/17] arm64/cpufeature: Add remaining feature bits in ID_AA64MMFR1 register
(no commit info)
[14/17] arm64/cpufeature: Add remaining feature bits in ID_AA64MMFR2 register
(no commit info)
[15/17] arm64/cpufeature: Add remaining feature bits in ID_AA64DFR0 register
(no commit info)
[16/17] arm64/cpufeature: Replace all open bits shift encodings with macros
(no commit info)
[17/17] arm64/cpuinfo: Add ID_MMFR4_EL1 into the cpuinfo_arm64 context
https://git.kernel.org/arm64/c/858b8a8039d0
Note that Suzuki had comments on 12-16, so assume you'll respin those (I fixed
up the trivial comments on earlier patches myself).
Cheers,
--
Will
https://fixes.arm64.dev
https://next.arm64.dev
https://will.arm64.dev
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH v2 2/2] remoteproc/k3-dsp: Add support for C71x DSPs
From: Suman Anna @ 2020-05-21 15:16 UTC (permalink / raw)
To: Bjorn Andersson, Rob Herring, Mathieu Poirier
Cc: devicetree, Lokesh Vutla, linux-remoteproc, linux-kernel,
Suman Anna, linux-arm-kernel
In-Reply-To: <20200521151636.28260-1-s-anna@ti.com>
The Texas Instrument's K3 J721E SoCs have a newer next-generation
C71x DSP Subsystem in the MAIN voltage domain in addition to the
previous generation C66x DSP subsystems. The C71x DSP subsystem is
based on the TMS320C71x DSP CorePac module. The C71x CPU is a true
64-bit machine including 64-bit memory addressing and single-cycle
64-bit base arithmetic operations and supports vector signal processing
providing a significant lift in DSP processing power over C66x DSPs.
J721E SoCs use a C711 (a one-core 512-bit vector width CPU core) DSP
that is cache coherent with the A72 Arm cores.
Each subsystem has one or more Fixed/Floating-Point DSP CPUs, with 32 KB
of L1P Cache, 48 KB of L1D SRAM that can be configured and partitioned as
either RAM and/or Cache, and 512 KB of L2 SRAM configurable as either RAM
and/or Cache. The CorePac also includes a Matrix Multiplication Accelerator
(MMA), a Stream Engine (SE) and a C71x Memory Management Unit (CMMU), an
Interrupt Controller (INTC) and a Powerdown Management Unit (PMU) modules.
Update the existing K3 DSP remoteproc driver to add support for this C71x
DSP subsystem. The firmware loading support is provided by using the newly
added 64-bit ELF loader support, and is limited to images using only
external DDR memory at the moment. The L1D and L2 SRAMs are used as scratch
memory when using as RAMs, and cannot be used for loadable segments. The
CMMU is also not supported to begin with, and the driver is designed to
treat the MMU as if it is in bypass mode.
Signed-off-by: Suman Anna <s-anna@ti.com>
---
v2:
- k3_dsp_rproc_prepare/unprepare plugged in dynamically based on local reset,
C71x doesn't use local resets
- Dropped the sanity_check ops override, not needed on latest codebase
v1: https://patchwork.kernel.org/patch/11458595/
drivers/remoteproc/ti_k3_dsp_remoteproc.c | 20 ++++++++++++++++++--
1 file changed, 18 insertions(+), 2 deletions(-)
diff --git a/drivers/remoteproc/ti_k3_dsp_remoteproc.c b/drivers/remoteproc/ti_k3_dsp_remoteproc.c
index 610fbbf85ee6..2dbed316b6ac 100644
--- a/drivers/remoteproc/ti_k3_dsp_remoteproc.c
+++ b/drivers/remoteproc/ti_k3_dsp_remoteproc.c
@@ -406,8 +406,6 @@ static void *k3_dsp_rproc_da_to_va(struct rproc *rproc, u64 da, size_t len)
}
static const struct rproc_ops k3_dsp_rproc_ops = {
- .prepare = k3_dsp_rproc_prepare,
- .unprepare = k3_dsp_rproc_unprepare,
.start = k3_dsp_rproc_start,
.stop = k3_dsp_rproc_stop,
.kick = k3_dsp_rproc_kick,
@@ -617,6 +615,10 @@ static int k3_dsp_rproc_probe(struct platform_device *pdev)
rproc->has_iommu = false;
rproc->recovery_disabled = true;
+ if (data->uses_lreset) {
+ rproc->ops->prepare = k3_dsp_rproc_prepare;
+ rproc->ops->unprepare = k3_dsp_rproc_unprepare;
+ }
kproc = rproc->priv;
kproc->rproc = rproc;
kproc->dev = dev;
@@ -744,6 +746,12 @@ static const struct k3_dsp_mem_data c66_mems[] = {
{ .name = "l1dram", .dev_addr = 0xf00000 },
};
+/* C71x cores only have a L1P Cache, there are no L1P SRAMs */
+static const struct k3_dsp_mem_data c71_mems[] = {
+ { .name = "l2sram", .dev_addr = 0x800000 },
+ { .name = "l1dram", .dev_addr = 0xe00000 },
+};
+
static const struct k3_dsp_dev_data c66_data = {
.mems = c66_mems,
.num_mems = ARRAY_SIZE(c66_mems),
@@ -751,8 +759,16 @@ static const struct k3_dsp_dev_data c66_data = {
.uses_lreset = true,
};
+static const struct k3_dsp_dev_data c71_data = {
+ .mems = c71_mems,
+ .num_mems = ARRAY_SIZE(c71_mems),
+ .boot_align_addr = SZ_2M,
+ .uses_lreset = false,
+};
+
static const struct of_device_id k3_dsp_of_match[] = {
{ .compatible = "ti,j721e-c66-dsp", .data = &c66_data, },
+ { .compatible = "ti,j721e-c71-dsp", .data = &c71_data, },
{ /* sentinel */ },
};
MODULE_DEVICE_TABLE(of, k3_dsp_of_match);
--
2.26.0
_______________________________________________
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