Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH] media: meson: Add NULL check after the call to kmalloc()
From: Greg KH @ 2019-09-04  8:45 UTC (permalink / raw)
  To: Austin Kim
  Cc: mjourdan, devel, khilman, linux-kernel, linux-amlogic, mchehab,
	linux-arm-kernel, linux-media
In-Reply-To: <20190904082232.GA171180@LGEARND20B15>

On Wed, Sep 04, 2019 at 05:22:32PM +0900, Austin Kim wrote:
> If the kmalloc() return NULL, the NULL pointer dereference will occur.
> 	new_ts->ts = ts;
> 
> Add exception check after the call to kmalloc() is made.
> 
> Signed-off-by: Austin Kim <austindh.kim@gmail.com>
> ---
>  drivers/staging/media/meson/vdec/vdec_helpers.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/staging/media/meson/vdec/vdec_helpers.c b/drivers/staging/media/meson/vdec/vdec_helpers.c
> index f16948b..e7e56d5 100644
> --- a/drivers/staging/media/meson/vdec/vdec_helpers.c
> +++ b/drivers/staging/media/meson/vdec/vdec_helpers.c
> @@ -206,6 +206,10 @@ void amvdec_add_ts_reorder(struct amvdec_session *sess, u64 ts, u32 offset)
>  	unsigned long flags;
>  
>  	new_ts = kmalloc(sizeof(*new_ts), GFP_KERNEL);
> +	if (!new_ts) {
> +		dev_err(sess->core->dev, "Failed to kmalloc()\n");

Did you run this through checkpatch?  I think it will say that this line
is not ok.

> +		return;

Shouldn't you return an -ENOMEM error somehow?

thanks,

greg k-h

_______________________________________________
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 1/1] soc: qcom: geni: Provide parameter error checking
From: Lee Jones @ 2019-09-04  8:45 UTC (permalink / raw)
  To: Bjorn Andersson; +Cc: linux-arm-msm, agross, linux-kernel, linux-arm-kernel
In-Reply-To: <20190904031922.GC574@tuxbook-pro>

On Tue, 03 Sep 2019, Bjorn Andersson wrote:

> On Tue 03 Sep 06:50 PDT 2019, Lee Jones wrote:
> 
> > When booting with ACPI, the Geni Serial Engine is not set as the I2C/SPI
> > parent and thus, the wrapper (parent device) is unassigned.  This causes
> > the kernel to crash with a null dereference error.
> > 
> 
> Now I see what you did in 8bc529b25354; i.e. stubbed all the other calls
> between the SE and wrapper.
> 
> Do you think it would be possible to resolve the _DEP link to QGP[01]
> somehow?

I looked at QGP{0,1}, but did not see it represented in the current
Device Tree implementation and thus failed to identify it.  Do you
know what it is?  Does it have a driver in Linux already?

> For the clocks workarounds this could be resolved by us
> representing that relationship using device_link and just rely on
> pm_runtime to propagate the clock state.

That is not allowed when booting ACPI.  The Clock/Regulator frameworks
are not to be used in this use-case, hence why all of the calls to
these frameworks are "stubbed out".  If we wanted to properly
implement power management, we would have to create a driver/subsystem
similar to the "Windows-compatible System Power Management Controller"
(PEP).  Without documentation for the PEP, this would be an impossible
task.  A request for the aforementioned documentation has been put in
to Lenovo/Qualcomm.  Hopefully something appears soon.

> For the DMA operation, iiuc it's the wrapper that implements the DMA
> engine involved, but I'm guessing the main reason for mapping buffers on
> the wrapper is so that it ends up being associated with the iommu
> context of the wrapper.

Judging by the code alone, the wrapper doesn't sound like it does much
at all.  It seems to only have a single (version) register (at least
that is the only register that's used).  The only registers it
reads/writes are those of the calling device, whether that be I2C, SPI
or UART.

Device Tree represents the wrapper's relationship with the I2C (and
SPI/UART) Serial Engine (SE) devices as parent-child ones, with the
wrapper being the parent and SE the child.  Whether this is a true
representation of the hardware or just a tactic used for convenience
is not clear, but the same representation does not exist in ACPI.

In the current Linux implementation, the buffer belongs to the SE
(obtained by the child (e.g. I2C) SE by fetching the parent's
(wrapper's) device data using the standard platform helpers) but the
register-set used to control the DMA transactions belong to the SE
devices.

> Are the SMMU contexts at all represented in the ACPI world and if so do
> you know how the wrapper vs SEs are bound to contexts? Can we map on
> se->dev when wrapper is NULL (or perhaps always?)?

Yes, the SMMU devices are represented in ACPI (MMU0) and (MMU1).  They
share the same register addresses as the SMMU devices located in
arch/arm64/boot/dts/qcom/sdm845.dtsi.

With this simple parameter checking patch, the SE falls back to using
FIFO mode to transmit data and continues to work flawlessly.  IMHO
this should be applied in the first instance, as it fixes a real (null
dereference) bug which currently resides in the Mainline kernel.

Moving forward we can try to come up with a suitable plan to implement
DMA in the ACPI use-case - but again, this is feature adding work
which should be carried out against -next, where as this patch needs
to go in via the current -rcs ASAP.

> > Fixes: 8bc529b25354 ("soc: qcom: geni: Add support for ACPI")
> > Signed-off-by: Lee Jones <lee.jones@linaro.org>
> > ---
> > Since we are already at -rc7 this patch should be processed ASAP - thank you.
> > 
> > drivers/soc/qcom/qcom-geni-se.c | 6 ++++++
> >  1 file changed, 6 insertions(+)
> > 
> > diff --git a/drivers/soc/qcom/qcom-geni-se.c b/drivers/soc/qcom/qcom-geni-se.c
> > index d5cf953b4337..7d622ea1274e 100644
> > --- a/drivers/soc/qcom/qcom-geni-se.c
> > +++ b/drivers/soc/qcom/qcom-geni-se.c
> > @@ -630,6 +630,9 @@ int geni_se_tx_dma_prep(struct geni_se *se, void *buf, size_t len,
> >  	struct geni_wrapper *wrapper = se->wrapper;
> >  	u32 val;
> >  
> > +	if (!wrapper)
> > +		return -EINVAL;
> > +
> >  	*iova = dma_map_single(wrapper->dev, buf, len, DMA_TO_DEVICE);
> >  	if (dma_mapping_error(wrapper->dev, *iova))
> >  		return -EIO;
> > @@ -663,6 +666,9 @@ int geni_se_rx_dma_prep(struct geni_se *se, void *buf, size_t len,
> >  	struct geni_wrapper *wrapper = se->wrapper;
> >  	u32 val;
> >  
> > +	if (!wrapper)
> > +		return -EINVAL;
> > +
> >  	*iova = dma_map_single(wrapper->dev, buf, len, DMA_FROM_DEVICE);
> >  	if (dma_mapping_error(wrapper->dev, *iova))
> >  		return -EIO;

-- 
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* [PATCH -next] usb: phy: mxs: use devm_platform_ioremap_resource() to simplify code
From: YueHaibing @ 2019-09-04  8:48 UTC (permalink / raw)
  To: balbi, gregkh, shawnguo, s.hauer, kernel, festevam, linux-imx
  Cc: linux-usb, YueHaibing, linux-kernel, linux-arm-kernel

Use devm_platform_ioremap_resource() to simplify the code a bit.
This is detected by coccinelle.

Reported-by: Hulk Robot <hulkci@huawei.com>
Signed-off-by: YueHaibing <yuehaibing@huawei.com>
---
 drivers/usb/phy/phy-mxs-usb.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/usb/phy/phy-mxs-usb.c b/drivers/usb/phy/phy-mxs-usb.c
index 70b8c82..67b39dc 100644
--- a/drivers/usb/phy/phy-mxs-usb.c
+++ b/drivers/usb/phy/phy-mxs-usb.c
@@ -710,7 +710,6 @@ static enum usb_charger_type mxs_phy_charger_detect(struct usb_phy *phy)
 
 static int mxs_phy_probe(struct platform_device *pdev)
 {
-	struct resource *res;
 	void __iomem *base;
 	struct clk *clk;
 	struct mxs_phy *mxs_phy;
@@ -723,8 +722,7 @@ static int mxs_phy_probe(struct platform_device *pdev)
 	if (!of_id)
 		return -ENODEV;
 
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	base = devm_ioremap_resource(&pdev->dev, res);
+	base = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(base))
 		return PTR_ERR(base);
 
-- 
2.7.4



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* [PATCH] soc: samsung: chipid: Make exynos_chipid_early_init() static
From: Sylwester Nawrocki @ 2019-09-04  8:49 UTC (permalink / raw)
  To: krzk
  Cc: linux-samsung-soc, b.zolnierkie, linux-kernel, kgene,
	Sylwester Nawrocki, linux-arm-kernel, m.szyprowski
In-Reply-To: <CGME20190904085001eucas1p2b3a120d6206983d47f0084b872042342@eucas1p2.samsung.com>

Add missing static qualifier to the chipid initcall function.

Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
---
 drivers/soc/samsung/exynos-chipid.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/soc/samsung/exynos-chipid.c b/drivers/soc/samsung/exynos-chipid.c
index c55a47cfe617..25562dd0b206 100644
--- a/drivers/soc/samsung/exynos-chipid.c
+++ b/drivers/soc/samsung/exynos-chipid.c
@@ -45,7 +45,7 @@ static const char * __init product_id_to_soc_id(unsigned int product_id)
 	return NULL;
 }
 
-int __init exynos_chipid_early_init(void)
+static int __init exynos_chipid_early_init(void)
 {
 	struct soc_device_attribute *soc_dev_attr;
 	struct soc_device *soc_dev;
-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* Re: [PATCH v5] perf machine: arm/arm64: Improve completeness for kernel address space
From: Leo Yan @ 2019-09-04  8:59 UTC (permalink / raw)
  To: Adrian Hunter
  Cc: Song Liu, Mathieu Poirier, Daniel Borkmann, Suzuki Poulouse,
	Alexander Shishkin, netdev, coresight, Alexei Starovoitov,
	Arnaldo Carvalho de Melo, linux-kernel, clang-built-linux,
	Peter Zijlstra, Yonghong Song, Namhyung Kim, bpf, Jiri Olsa,
	Martin KaFai Lau, linux-arm-kernel
In-Reply-To: <c16ee888-73cc-588d-6156-bb5528d635cf@intel.com>

Hi Adrian,

On Wed, Sep 04, 2019 at 10:26:13AM +0300, Adrian Hunter wrote:

[...]

> > Could you take chance to review my below replying?  I'd like to get
> > your confirmation before I send out offical patch.
> 
> It is not necessary to do kallsyms__parse for x86_64, so it would be better
> to check the arch before calling that.

Thanks for suggestion, will do it in the formal patch.

> However in general, having to copy and use kallsyms with perf.data if on a
> different arch does not seem very user friendly.

Agree.  Seems it's more reasonable to save related info in
perf.data; TBH, I have no idea for how to do that.

> But really that is up to Arnaldo.

@Arnaldo, if possible could you take a look for below change?

If you don't think below code is the right thing and it's not a common
issue, then maybe it's more feasible to resolve this issue only for
Arm CoreSight specific.

Please let me know how about you think for this?

Thanks,
Leo Yan

> >> For your question for taking a perf.data file to a machine with a
> >> different architecture, we can firstly use command 'perf buildid-list'
> >> to print out the buildid for kallsyms, based on the dumped buildid we
> >> can find out the location for the saved kallsyms file; then we can use
> >> option '--kallsyms' to specify the offline kallsyms file and use the
> >> offline kallsyms to fixup kernel start address.  The detailed commands
> >> are listed as below:
> >>
> >> root@debian:~# perf buildid-list
> >> 7b36dfca8317ef74974ebd7ee5ec0a8b35c97640 [kernel.kallsyms]
> >> 56b84aa88a1bcfe222a97a53698b92723a3977ca /usr/lib/systemd/systemd
> >> 0956b952e9cd673d48ff2cfeb1a9dbd0c853e686 /usr/lib/aarch64-linux-gnu/libm-2.28.so
> >> [...]
> >>
> >> root@debian:~# perf script --kallsyms ~/.debug/\[kernel.kallsyms\]/7b36dfca8317ef74974ebd7ee5ec0a8b35c97640/kallsyms
> >>
> >> The amended patch is as below, please review and always welcome
> >> any suggestions or comments!
> >>
> >> diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
> >> index 5734460fc89e..593f05cc453f 100644
> >> --- a/tools/perf/util/machine.c
> >> +++ b/tools/perf/util/machine.c
> >> @@ -2672,9 +2672,26 @@ int machine__nr_cpus_avail(struct machine *machine)
> >>  	return machine ? perf_env__nr_cpus_avail(machine->env) : 0;
> >>  }
> >>  
> >> +static int machine__fixup_kernel_start(void *arg,
> >> +				       const char *name __maybe_unused,
> >> +				       char type,
> >> +				       u64 start)
> >> +{
> >> +	struct machine *machine = arg;
> >> +
> >> +	type = toupper(type);
> >> +
> >> +	/* Fixup for text, weak, data and bss sections. */
> >> +	if (type == 'T' || type == 'W' || type == 'D' || type == 'B')
> >> +		machine->kernel_start = min(machine->kernel_start, start);
> >> +
> >> +	return 0;
> >> +}
> >> +
> >>  int machine__get_kernel_start(struct machine *machine)
> >>  {
> >>  	struct map *map = machine__kernel_map(machine);
> >> +	char filename[PATH_MAX];
> >>  	int err = 0;
> >>  
> >>  	/*
> >> @@ -2696,6 +2713,22 @@ int machine__get_kernel_start(struct machine *machine)
> >>  		if (!err && !machine__is(machine, "x86_64"))
> >>  			machine->kernel_start = map->start;
> >>  	}
> >> +
> >> +	if (symbol_conf.kallsyms_name != NULL) {
> >> +		strncpy(filename, symbol_conf.kallsyms_name, PATH_MAX);
> >> +	} else {
> >> +		machine__get_kallsyms_filename(machine, filename, PATH_MAX);
> >> +
> >> +		if (symbol__restricted_filename(filename, "/proc/kallsyms"))
> >> +			goto out;
> >> +	}
> >> +
> >> +	if (kallsyms__parse(filename, machine, machine__fixup_kernel_start))
> >> +		pr_warning("Fail to fixup kernel start address. skipping...\n");
> >> +
> >> +out:
> >>  	return err;
> >>  }
> >>  
> >>
> >> Thanks,
> >> Leo Yan
> > 
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* arm64: ls1028a-qds: correct bus of rtc
From: Biwen Li @ 2019-09-04  8:51 UTC (permalink / raw)
  To: shawnguo, leoyang.li, robh+dt, mark.rutland
  Cc: devicetree, linux-kernel, linux-arm-kernel, Biwen Li

The rtc is on i2c2 bus(hardware), not on i2c1 channel 3,
so correct it

Signed-off-by: Biwen Li <biwen.li@nxp.com>
---
 arch/arm64/boot/dts/freescale/fsl-ls1028a-qds.dts | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1028a-qds.dts b/arch/arm64/boot/dts/freescale/fsl-ls1028a-qds.dts
index de6ef39f3118..6c0540ad9c59 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls1028a-qds.dts
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1028a-qds.dts
@@ -133,11 +133,6 @@
 				vcc-supply = <&sb_3v3>;
 			};
 
-			rtc@51 {
-				compatible = "nxp,pcf2129";
-				reg = <0x51>;
-			};
-
 			eeprom@56 {
 				compatible = "atmel,24c512";
 				reg = <0x56>;
@@ -166,6 +161,14 @@
 	};
 };
 
+&i2c1 {
+	status = "okay";
+	rtc@51 {
+		compatible = "nxp,pcf2129";
+		reg = <0x51>;
+	};
+};
+
 &sai1 {
 	status = "okay";
 };
-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* Re: [RFC PATCH V2 4/4] platform: mtk-isp: Add Mediatek FD driver
From: Jerry-ch Chen @ 2019-09-04  9:01 UTC (permalink / raw)
  To: Tomasz Figa
  Cc: devicetree@vger.kernel.org, Sean Cheng (鄭昇弘),
	laurent.pinchart+renesas@ideasonboard.com,
	Rynn Wu (吳育恩), srv_heupstream,
	Po-Yang Huang (黃柏陽), mchehab@kernel.org,
	suleiman@chromium.org, shik@chromium.org,
	Jungo Lin (林明俊),
	Sj Huang (黃信璋), yuzhao@chromium.org,
	linux-mediatek@lists.infradead.org, zwisler@chromium.org,
	matthias.bgg@gmail.com, Christie Yu (游雅惠),
	Frederic Chen (陳俊元), hans.verkuil@cisco.com,
	linux-arm-kernel@lists.infradead.org, linux-media@vger.kernel.org
In-Reply-To: <CAAFQd5Dzxy10g-MKHMnNbVO6kp9_L_jm1m+gtN+p=YF2LyBiag@mail.gmail.com>

Hi Tomasz,

On Wed, 2019-09-04 at 16:25 +0800, Tomasz Figa wrote:
> On Wed, Sep 4, 2019 at 5:09 PM Jerry-ch Chen <Jerry-ch.Chen@mediatek.com> wrote:
> >
> > Hi Tomasz,
> >
> > On Wed, 2019-09-04 at 14:34 +0800, Tomasz Figa wrote:
> > > On Wed, Sep 4, 2019 at 3:09 PM Jerry-ch Chen <Jerry-ch.Chen@mediatek.com> wrote:
> > > >
> > > > Hi Tomasz,
> > > >
> > > > On Wed, 2019-09-04 at 12:15 +0800, Tomasz Figa wrote:
> > > > > On Wed, Sep 4, 2019 at 12:38 PM Jerry-ch Chen
> > > > > <Jerry-ch.Chen@mediatek.com> wrote:
> > > > > >
> > > > > > Hi Tomasz,
> > > > > >
> > > > > > On Tue, 2019-09-03 at 20:05 +0800, Tomasz Figa wrote:
> > > > > > > On Tue, Sep 3, 2019 at 8:46 PM Jerry-ch Chen <Jerry-ch.Chen@mediatek.com> wrote:
> > > > > > > >
> > > > > > > > Hi Tomasz,
> > > > > > > >
> > > > > > > > On Tue, 2019-09-03 at 15:04 +0800, Tomasz Figa wrote:
> > > > > > > > > On Tue, Sep 3, 2019 at 3:44 PM Jerry-ch Chen <Jerry-ch.Chen@mediatek.com> wrote:
> > > > > > > > > >
> > > > > > > > > > On Tue, 2019-09-03 at 13:19 +0800, Tomasz Figa wrote:
> > > > > > > > > > > On Mon, Sep 2, 2019 at 8:47 PM Jerry-ch Chen <Jerry-ch.Chen@mediatek.com> wrote:
> > > > > > > > > > > >
> > > > > > > > > > > > Hi Tomasz,
> > > > > > > > > > > >
> > > > > > > > > > > > On Fri, 2019-08-30 at 16:33 +0800, Tomasz Figa wrote:
> > > > > > > > > > > > > On Wed, Aug 28, 2019 at 11:00 AM Jerry-ch Chen
> > > > > > > > > > > > > <Jerry-ch.Chen@mediatek.com> wrote:
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > Hi Tomasz,
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > On Mon, 2019-08-26 at 14:36 +0800, Tomasz Figa wrote:
> > > > > > > > > > > > > > > Hi Jerry,
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > On Sun, Aug 25, 2019 at 6:18 PM Jerry-ch Chen
> > > > > > > > > > > > > > > <Jerry-ch.Chen@mediatek.com> wrote:
> > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > Hi Tomasz,
> > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > On Fri, 2019-08-02 at 16:28 +0800, Tomasz Figa wrote:
> > > > > > > > > > > > > > > > > Hi Jerry,
> > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > On Tue, Jul 09, 2019 at 04:41:12PM +0800, Jerry-ch Chen wrote:
> > > > > > [snip]
> > > > > > > > > > > > > > > [snip]
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > +static void mtk_fd_vb2_stop_streaming(struct vb2_queue *vq)
> > > > > > > > > > > > > > > > > > +{
> > > > > > > > > > > > > > > > > > +   struct mtk_fd_ctx *ctx = vb2_get_drv_priv(vq);
> > > > > > > > > > > > > > > > > > +   struct vb2_buffer *vb;
> > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > How do we guarantee here that the hardware isn't still accessing the buffers
> > > > > > > > > > > > > > > > > removed below?
> > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > Maybe we can check the driver state flag and aborting the unfinished
> > > > > > > > > > > > > > > > jobs?
> > > > > > > > > > > > > > > > (fd_hw->state == FD_ENQ)
> > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > Yes, we need to either cancel or wait for the currently processing
> > > > > > > > > > > > > > > job. It depends on hardware capabilities, but cancelling is generally
> > > > > > > > > > > > > > > preferred for the lower latency.
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > Ok, it the state is ENQ, then we can disable the FD hw by controlling
> > > > > > > > > > > > > > the registers.
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > for example:
> > > > > > > > > > > > > >         writel(0x0, fd->fd_base + FD_HW_ENABLE);
> > > > > > > > > > > > > >         writel(0x0, fd->fd_base + FD_INT_EN);
> > > > > > > > > > > > > >
> > > > > > > > > > > > >
> > > > > > > > > > > > > What's exactly the effect of writing 0 to FD_HW_ENABLE?
> > > > > > > > > > > > >
> > > > > > > > > > > > Sorry, my last reply didn't solve the question,
> > > > > > > > > > > > we should implement a mtk_fd_job_abort() for v4l2_m2m_ops().
> > > > > > > > > > > >
> > > > > > > > > > > > which is able to readl_poll_timeout_atomic()
> > > > > > > > > > > > and check the HW busy bits in the register FD_INT_EN;
> > > > > > > > > > > >
> > > > > > > > > > > > if they are not cleared until timeout, we could handle the last
> > > > > > > > > > > > processing job.
> > > > > > > > > > > > Otherwise, the FD irq handler should have handled the last processing
> > > > > > > > > > > > job and we could continue the stop_streaming().
> > > > > > > > > > > >
> > > > > > > > > > > > For job_abort():
> > > > > > > > > > > > static void mtk_fd_job_abort(void *priv)
> > > > > > > > > > > > {
> > > > > > > > > > > >         struct mtk_fd_ctx *ctx = priv;
> > > > > > > > > > > >         struct mtk_fd_dev *fd = ctx->fd_dev;
> > > > > > > > > > > >         u32 val;
> > > > > > > > > > > >         u32 ret;
> > > > > > > > > > > >
> > > > > > > > > > > >         ret = readl_poll_timeout_atomic(fd->fd_base + MTK_FD_REG_OFFSET_INT_EN,
> > > > > > > > > > > >                                         val,
> > > > > > > > > > > >                                         (val & MTK_FD_HW_BUSY_MASK) ==
> > > > > > > > > > > >                                         MTK_FD_HW_STATE_IS_BUSY,
> > > > > > > > > > > >                                         USEC_PER_MSEC, MTK_FD_STOP_HW_TIMEOUT);
> > > > > > > > > > >
> > > > > > > > > > > Hmm, would it be possible to avoid the busy wait by having a
> > > > > > > > > > > completion that could be signalled from the interrupt handler?
> > > > > > > > > > >
> > > > > > > > > > > Best regards,
> > > > > > > > > > > Tomasz
> > > > > > > > > >
> > > > > > > > > > I suppose that would be wakeup a wait queue in the interrupt handler,
> > > > > > > > > > the the wait_event_interrupt_timeout() will be used in here and system
> > > > > > > > > > suspend e.g. mtk_fd_suspend().
> > > > > > > > >
> > > > > > > > > Yes, that should work.
> > > > > > > > >
> > > > > > > > > > Or do you suggest to wait_event_interrupt_timeout() every frame in the
> > > > > > > > > > mtk_fd_ipi_handler()?
> > > > > > > > >
> > > > > > > > > Nope, we shouldn't need that.
> > > > > > > > >
> > > > > > > > > > I think maybe the readl_poll_timeout_atomic would be good enough.
> > > > > > > > > >
> > > > > > > > >
> > > > > > > > > Not really. Busy waiting should be avoided as much as possible. What's
> > > > > > > > > the point of entering suspend if you end up burning the power by
> > > > > > > > > spinning the CPU for some milliseconds?
> > > > > > > > >
> > > > > > > > Ok, I see, busy waiting is not a good idea, I will use the wait queue
> > > > > > > > instead. the job abort will refine as following:
> > > > > > > >
> > > > > > > > static void mtk_fd_job_abort(void *priv)
> > > > > > > > {
> > > > > > > >         struct mtk_fd_ctx *ctx = priv;
> > > > > > > >         struct mtk_fd_dev *fd = ctx->fd_dev;
> > > > > > > >         u32 ret;
> > > > > > > >
> > > > > > > >         ret = wait_event_interruptible_timeout
> > > > > > > >                 (fd->wq, (fd->fd_irq_result & MTK_FD_HW_IRQ_MASK),
> > > > > > > >                  usecs_to_jiffies(50000));
> > > > > > > >         if (ret)
> > > > > > > >                 mtk_fd_hw_job_finish(fd, VB2_BUF_STATE_ERROR);
> > > > > > > >         dev_dbg(fd->dev, "%s, ret:%d\n", __func__, ret);
> > > > > > > >
> > > > > > > >         fd->fd_irq_result = 0;
> > > > > > > > }
> > > > > > > >
> > > > > > > > static struct v4l2_m2m_ops fd_m2m_ops = {
> > > > > > > >         .device_run = mtk_fd_device_run,
> > > > > > > >         .job_abort = mtk_fd_job_abort,
> > > > > > >
> > > > > > > I'm not sure we should be using the functon above as the .job_abort
> > > > > > > callback. It's expected to abort the job, but we're just waiting for
> > > > > > > it to finish. I think we should just call mtk_fd_job_abort() manually
> > > > > > > from .stop_streaming.
> > > > > > >
> > > > > >
> > > > > > Ok, I will fix it.
> > > > > >
> > > > > > > > };
> > > > > > > >
> > > > > > > > and we could use it in suspend.
> > > > > > > > static int mtk_fd_suspend(struct device *dev)
> > > > > > > > {
> > > > > > > >         struct mtk_fd_dev *fd = dev_get_drvdata(dev);
> > > > > > > >
> > > > > > > >         if (pm_runtime_suspended(dev))
> > > > > > > >                 return 0;
> > > > > > > >
> > > > > > > >         if (fd->fd_stream_count)
> > > > > > > >                 mtk_fd_job_abort(fd->ctx);
> > > > > > > >
> > > > > > > >         /* suspend FD HW */
> > > > > > > >         writel(0x0, fd->fd_base + MTK_FD_REG_OFFSET_INT_EN);
> > > > > > > >         writel(0x0, fd->fd_base + MTK_FD_REG_OFFSET_HW_ENABLE);
> > > > > > > >         clk_disable_unprepare(fd->fd_clk);
> > > > > > > >         dev_dbg(dev, "%s:disable clock\n", __func__);
> > > > > > > >
> > > > > > > >         return 0;
> > > > > > > > }
> > > > > > > >
> > > > > > > > static irqreturn_t mtk_fd_irq(int irq, void *data)
> > > > > > > > {
> > > > > > > >         struct mtk_fd_dev *fd = (struct mtk_fd_dev *)data;
> > > > > > > >
> > > > > > > >         fd->fd_irq_result = readl(fd->fd_base + MTK_FD_REG_OFFSET_INT_VAL);
> > > > > > > >         wake_up_interruptible(&fd->wq);
> > > > > > >
> > > > > > > The wake up should be done at the very end of this function. Otherwise
> > > > > > > we end up with m2m framework racing with the mtk_fd_hw_job_finish()
> > > > > > > below.
> > > > > > >
> > > > > > > Also, I'd use a completion here rather than an open coded wait and
> > > > > > > wake-up. The driver could reinit_completion() before queuing a job to
> > > > > > > the hardware and the IRQ handler would complete(). There would be no
> > > > > > > need to store the IRQ flags in driver data anymore.
> > > > > > Ok, It will be refined as following:
> > > > > >
> > > > > > suspend and stop streaming will call mtk_fd_job_abort()
> > > > > >
> > > > > > static void mtk_fd_job_abort(struct mtk_fd_dev *fd)
> > > > > > {
> > > > > >         u32 ret;
> > > > > >
> > > > > >         ret = wait_for_completion_timeout(&fd->fd_irq_done,
> > > > > >                                           msecs_to_jiffies(MTK_FD_HW_TIMEOUT));
> > > > > >         if (ret)
> > > > >
> > > > > For the _timeout variants, !ret means the timeout and ret > 0 means
> > > > > that the wait ended successfully before.
> > > > >
> > > > Thanks, fixed.
> > > >
> > > > > Also please make sure that the timeout is big enough not to happen
> > > > > normally on a heavily loaded system. Something like 1 second should be
> > > > > good.
> > > > >
> > > > Ok, I will make it 1 second
> > > > #define MTK_FD_HW_TIMEOUT 1000
> > > >
> > > > Also, I will add a condition in mtk_fd_vb2_stop_streaming(), since it
> > > > would be called twice, now it works fine whether I reuse the condition
> > > > with mtk_fd_hw_disconnect or not, however, I think do it before buffer
> > > > remove looks more reasonable.
> > > >
> > > > static void mtk_fd_vb2_stop_streaming(struct vb2_queue *vq)
> > > > {
> > > >         struct mtk_fd_ctx *ctx = vb2_get_drv_priv(vq);
> > > >         struct mtk_fd_dev *fd = ctx->fd_dev;
> > > >         struct vb2_v4l2_buffer *vb;
> > > >         struct v4l2_m2m_ctx *m2m_ctx = ctx->fh.m2m_ctx;
> > > >         struct v4l2_m2m_queue_ctx *queue_ctx;
> > > >
> > > >         if (vq->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE)
> > > >                 mtk_fd_job_abort(fd);
> > >
> > > We need to do it regardless of the queue type, otherwise we could
> > > still free CAPTURE buffers before the hardware releases them.
> > >
> >
> > I think we may read the fd->fd_irq_done.done and do wait for completion
> > if it's not being done.
> > How do you think?
> >
> > static void mtk_fd_vb2_stop_streaming(struct vb2_queue *vq)
> > {
> >         struct mtk_fd_ctx *ctx = vb2_get_drv_priv(vq);
> >         struct mtk_fd_dev *fd = ctx->fd_dev;
> >         struct vb2_v4l2_buffer *vb;
> >         struct v4l2_m2m_ctx *m2m_ctx = ctx->fh.m2m_ctx;
> >         struct v4l2_m2m_queue_ctx *queue_ctx;
> >         u32 ret;
> >
> >         if (!fd->fd_irq_done.done)
> 
> We shouldn't access internal fields of completion.
> 
> >                 ret = wait_for_completion_timeout(&fd->fd_irq_done,
> >                                                   msecs_to_jiffies(
> >                                                         MTK_FD_HW_TIMEOUT));
> >         queue_ctx = V4L2_TYPE_IS_OUTPUT(vq->type) ?
> >                                         &m2m_ctx->out_q_ctx :
> >                                         &m2m_ctx->cap_q_ctx;
> >         while ((vb = v4l2_m2m_buf_remove(queue_ctx)))
> >                 v4l2_m2m_buf_done(vb, VB2_BUF_STATE_ERROR);
> >
> >         if (vq->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE)
> >                 mtk_fd_hw_disconnect(fd);
> > }
> >
> > I've also tried to wait completion unconditionally for both queues and
> > the second time will wait until timeout, as a result, it takes longer to
> > swap the camera every time and close the camera app.
> 
> I think it should work better if we call complete_all() instead of complete().
> 
Thanks,

I use complete_all(), and it works fine now.

static void mtk_fd_vb2_stop_streaming(struct vb2_queue *vq)
{
	struct mtk_fd_ctx *ctx = vb2_get_drv_priv(vq);
	struct mtk_fd_dev *fd = ctx->fd_dev;
	struct vb2_v4l2_buffer *vb;
	struct v4l2_m2m_ctx *m2m_ctx = ctx->fh.m2m_ctx;
	struct v4l2_m2m_queue_ctx *queue_ctx;

	wait_for_completion_timeout(&fd->fd_irq_done,
					  msecs_to_jiffies(MTK_FD_HW_TIMEOUT));
	queue_ctx = V4L2_TYPE_IS_OUTPUT(vq->type) ?
					&m2m_ctx->out_q_ctx :
					&m2m_ctx->cap_q_ctx;
	while ((vb = v4l2_m2m_buf_remove(queue_ctx)))
		v4l2_m2m_buf_done(vb, VB2_BUF_STATE_ERROR);

	if (vq->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE)
		mtk_fd_hw_disconnect(fd);
}

Best regards,
Jerry

> Best regards,
> Tomasz



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* [PATCH -next] usb: gadget: at91_udc: use devm_platform_ioremap_resource() to simplify code
From: YueHaibing @ 2019-09-04  9:02 UTC (permalink / raw)
  To: balbi, gregkh, nicolas.ferre, alexandre.belloni,
	ludovic.desroches, hariprasad.kelam, yuehaibing
  Cc: linux-usb, linux-kernel, linux-arm-kernel

Use devm_platform_ioremap_resource() to simplify the code a bit.
This is detected by coccinelle.

Reported-by: Hulk Robot <hulkci@huawei.com>
Signed-off-by: YueHaibing <yuehaibing@huawei.com>
---
 drivers/usb/gadget/udc/at91_udc.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/usb/gadget/udc/at91_udc.c b/drivers/usb/gadget/udc/at91_udc.c
index 194ffb1..1b2b548 100644
--- a/drivers/usb/gadget/udc/at91_udc.c
+++ b/drivers/usb/gadget/udc/at91_udc.c
@@ -1808,7 +1808,6 @@ static int at91udc_probe(struct platform_device *pdev)
 	struct device	*dev = &pdev->dev;
 	struct at91_udc	*udc;
 	int		retval;
-	struct resource	*res;
 	struct at91_ep	*ep;
 	int		i;
 
@@ -1839,8 +1838,7 @@ static int at91udc_probe(struct platform_device *pdev)
 			ep->is_pingpong = 1;
 	}
 
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	udc->udp_baseaddr = devm_ioremap_resource(dev, res);
+	udc->udp_baseaddr = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(udc->udp_baseaddr))
 		return PTR_ERR(udc->udp_baseaddr);
 
-- 
2.7.4



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* Re: [PATCH] drm: bridge/dw_hdmi: add audio sample channel status setting
From: Cheng-yi Chiang @ 2019-09-04  9:09 UTC (permalink / raw)
  To: Neil Armstrong
  Cc: moderated list:SOUND - SOC LAYER / DYNAMIC AUDIO POWER MANAGEM...,
	tzungbi, kuninori.morimoto.gx, Xing Zheng, cain.cai, David Airlie,
	sam, Jeffy Chen, linux-kernel, dri-devel, Doug Anderson,
	Andrzej Hajda, 蔡枫, Laurent Pinchart, Daniel Vetter,
	Yakir Yang, Enric Balletbo i Serra, linux-rockchip, Dylan Reid,
	kuankuan.y, linux-arm-kernel
In-Reply-To: <e1c3483c-baa6-c726-e547-fadf40d259f4@baylibre.com>

Hi,

On Tue, Sep 3, 2019 at 5:53 PM Neil Armstrong <narmstrong@baylibre.com> wrote:
>
> Hi,
>
> On 03/09/2019 07:51, Cheng-Yi Chiang wrote:
> > From: Yakir Yang <ykk@rock-chips.com>
> >
> > When transmitting IEC60985 linear PCM audio, we configure the
> > Audio Sample Channel Status information of all the channel
> > status bits in the IEC60958 frame.
> > Refer to 60958-3 page 10 for frequency, original frequency, and
> > wordlength setting.
> >
> > This fix the issue that audio does not come out on some monitors
> > (e.g. LG 22CV241)
> >
> > Signed-off-by: Yakir Yang <ykk@rock-chips.com>
> > Signed-off-by: Cheng-Yi Chiang <cychiang@chromium.org>
> > ---
> >  drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 59 +++++++++++++++++++++++
> >  drivers/gpu/drm/bridge/synopsys/dw-hdmi.h | 20 ++++++++
> >  2 files changed, 79 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> > index bd65d0479683..34d46e25d610 100644
> > --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> > +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> > @@ -582,6 +582,63 @@ static unsigned int hdmi_compute_n(unsigned int freq, unsigned long pixel_clk)
> >       return n;
> >  }
> >
> > +static void hdmi_set_schnl(struct dw_hdmi *hdmi)
> > +{
> > +     u8 aud_schnl_samplerate;
> > +     u8 aud_schnl_8;
> > +
> > +     /* These registers are on RK3288 using version 2.0a. */
> > +     if (hdmi->version != 0x200a)
> > +             return;
>
> Are these limited to the 2.0a version *in* RK3288, or 2.0a version on all
> SoCs ?
>

In the original patch by Yakir,

https://lore.kernel.org/patchwork/patch/539653/   (sorry, I should
have added this link in the "after the cut" note)

The fix is limited to version 2.0.
Since I am only testing on RK3288 with 2.0a, I change the check to 2.0a only.
I can not test 2.0a version on other SoCs.
The databook I have at hand is 2.0a (not specific to RK3288) so I
think all 2.0a should have this register.

As for other version like version 1.3 on iMX6, there is no such
register, as stated by Russell

http://lkml.iu.edu/hypermail/linux/kernel/1501.3/06268.html.

So at least we should check the version.
Maybe we can set the criteria as version 2.0 or above to make it a safe patch ?
If there is the same need on other SoC with version < 2.0, it can be
added later.
Presumably, there will be databook of that version to help confirming
this setting.

Thanks!
> > +
> > +     switch (hdmi->sample_rate) {
> > +     case 32000:
> > +             aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_32K;
> > +             break;
> > +     case 44100:
> > +             aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_44K1;
> > +             break;
> > +     case 48000:
> > +             aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_48K;
> > +             break;
> > +     case 88200:
> > +             aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_88K2;
> > +             break;
> > +     case 96000:
> > +             aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_96K;
> > +             break;
> > +     case 176400:
> > +             aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_176K4;
> > +             break;
> > +     case 192000:
> > +             aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_192K;
> > +             break;
> > +     case 768000:
> > +             aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_768K;
> > +             break;
> > +     default:
> > +             dev_warn(hdmi->dev, "Unsupported audio sample rate (%u)\n",
> > +                      hdmi->sample_rate);
> > +             return;
> > +     }
> > +
> > +     /* set channel status register */
> > +     hdmi_modb(hdmi, aud_schnl_samplerate, HDMI_FC_AUDSCHNLS7_SMPRATE_MASK,
> > +               HDMI_FC_AUDSCHNLS7);
> > +
> > +     /*
> > +      * Set original frequency to be the same as frequency.
> > +      * Use one-complement value as stated in IEC60958-3 page 13.
> > +      */
> > +     aud_schnl_8 = (~aud_schnl_samplerate) <<
> > +                     HDMI_FC_AUDSCHNLS8_ORIGSAMPFREQ_OFFSET;
> > +
> > +     /* This means word length is 16 bit. Refer to IEC60958-3 page 12. */
> > +     aud_schnl_8 |= 2 << HDMI_FC_AUDSCHNLS8_WORDLEGNTH_OFFSET;
> > +
> > +     hdmi_writeb(hdmi, aud_schnl_8, HDMI_FC_AUDSCHNLS8);
> > +}
> > +
> >  static void hdmi_set_clk_regenerator(struct dw_hdmi *hdmi,
> >       unsigned long pixel_clk, unsigned int sample_rate)
> >  {
> > @@ -620,6 +677,8 @@ static void hdmi_set_clk_regenerator(struct dw_hdmi *hdmi,
> >       hdmi->audio_cts = cts;
> >       hdmi_set_cts_n(hdmi, cts, hdmi->audio_enable ? n : 0);
> >       spin_unlock_irq(&hdmi->audio_lock);
> > +
> > +     hdmi_set_schnl(hdmi);
> >  }
> >
> >  static void hdmi_init_clk_regenerator(struct dw_hdmi *hdmi)
> > diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h
> > index 6988f12d89d9..619ebc1c8354 100644
> > --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h
> > +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h
> > @@ -158,6 +158,17 @@
> >  #define HDMI_FC_SPDDEVICEINF                    0x1062
> >  #define HDMI_FC_AUDSCONF                        0x1063
> >  #define HDMI_FC_AUDSSTAT                        0x1064
> > +#define HDMI_FC_AUDSV                           0x1065
> > +#define HDMI_FC_AUDSU                           0x1066
> > +#define HDMI_FC_AUDSCHNLS0                      0x1067
> > +#define HDMI_FC_AUDSCHNLS1                      0x1068
> > +#define HDMI_FC_AUDSCHNLS2                      0x1069
> > +#define HDMI_FC_AUDSCHNLS3                      0x106a
> > +#define HDMI_FC_AUDSCHNLS4                      0x106b
> > +#define HDMI_FC_AUDSCHNLS5                      0x106c
> > +#define HDMI_FC_AUDSCHNLS6                      0x106d
> > +#define HDMI_FC_AUDSCHNLS7                      0x106e
> > +#define HDMI_FC_AUDSCHNLS8                      0x106f
> >  #define HDMI_FC_DATACH0FILL                     0x1070
> >  #define HDMI_FC_DATACH1FILL                     0x1071
> >  #define HDMI_FC_DATACH2FILL                     0x1072
> > @@ -706,6 +717,15 @@ enum {
> >  /* HDMI_FC_AUDSCHNLS7 field values */
> >       HDMI_FC_AUDSCHNLS7_ACCURACY_OFFSET = 4,
> >       HDMI_FC_AUDSCHNLS7_ACCURACY_MASK = 0x30,
> > +     HDMI_FC_AUDSCHNLS7_SMPRATE_MASK = 0x0f,
> > +     HDMI_FC_AUDSCHNLS7_SMPRATE_192K = 0xe,
> > +     HDMI_FC_AUDSCHNLS7_SMPRATE_176K4 = 0xc,
> > +     HDMI_FC_AUDSCHNLS7_SMPRATE_96K = 0xa,
> > +     HDMI_FC_AUDSCHNLS7_SMPRATE_768K = 0x9,
> > +     HDMI_FC_AUDSCHNLS7_SMPRATE_88K2 = 0x8,
> > +     HDMI_FC_AUDSCHNLS7_SMPRATE_32K = 0x3,
> > +     HDMI_FC_AUDSCHNLS7_SMPRATE_48K = 0x2,
> > +     HDMI_FC_AUDSCHNLS7_SMPRATE_44K1 = 0x0,
> >
> >  /* HDMI_FC_AUDSCHNLS8 field values */
> >       HDMI_FC_AUDSCHNLS8_ORIGSAMPFREQ_MASK = 0xf0,
> >
>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [RFC PATCH V2 4/4] platform: mtk-isp: Add Mediatek FD driver
From: Tomasz Figa @ 2019-09-04  9:03 UTC (permalink / raw)
  To: Jerry-ch Chen
  Cc: devicetree@vger.kernel.org, Sean Cheng (鄭昇弘),
	laurent.pinchart+renesas@ideasonboard.com,
	Rynn Wu (吳育恩), srv_heupstream,
	Po-Yang Huang (黃柏陽), mchehab@kernel.org,
	suleiman@chromium.org, shik@chromium.org,
	Jungo Lin (林明俊),
	Sj Huang (黃信璋), yuzhao@chromium.org,
	linux-mediatek@lists.infradead.org, zwisler@chromium.org,
	matthias.bgg@gmail.com, Christie Yu (游雅惠),
	Frederic Chen (陳俊元), hans.verkuil@cisco.com,
	linux-arm-kernel@lists.infradead.org, linux-media@vger.kernel.org
In-Reply-To: <1567587708.22453.15.camel@mtksdccf07>

On Wed, Sep 4, 2019 at 6:02 PM Jerry-ch Chen <Jerry-ch.Chen@mediatek.com> wrote:
>
> Hi Tomasz,
>
> On Wed, 2019-09-04 at 16:25 +0800, Tomasz Figa wrote:
> > On Wed, Sep 4, 2019 at 5:09 PM Jerry-ch Chen <Jerry-ch.Chen@mediatek.com> wrote:
> > >
> > > Hi Tomasz,
> > >
> > > On Wed, 2019-09-04 at 14:34 +0800, Tomasz Figa wrote:
> > > > On Wed, Sep 4, 2019 at 3:09 PM Jerry-ch Chen <Jerry-ch.Chen@mediatek.com> wrote:
> > > > >
> > > > > Hi Tomasz,
> > > > >
> > > > > On Wed, 2019-09-04 at 12:15 +0800, Tomasz Figa wrote:
> > > > > > On Wed, Sep 4, 2019 at 12:38 PM Jerry-ch Chen
> > > > > > <Jerry-ch.Chen@mediatek.com> wrote:
> > > > > > >
> > > > > > > Hi Tomasz,
> > > > > > >
> > > > > > > On Tue, 2019-09-03 at 20:05 +0800, Tomasz Figa wrote:
> > > > > > > > On Tue, Sep 3, 2019 at 8:46 PM Jerry-ch Chen <Jerry-ch.Chen@mediatek.com> wrote:
> > > > > > > > >
> > > > > > > > > Hi Tomasz,
> > > > > > > > >
> > > > > > > > > On Tue, 2019-09-03 at 15:04 +0800, Tomasz Figa wrote:
> > > > > > > > > > On Tue, Sep 3, 2019 at 3:44 PM Jerry-ch Chen <Jerry-ch.Chen@mediatek.com> wrote:
> > > > > > > > > > >
> > > > > > > > > > > On Tue, 2019-09-03 at 13:19 +0800, Tomasz Figa wrote:
> > > > > > > > > > > > On Mon, Sep 2, 2019 at 8:47 PM Jerry-ch Chen <Jerry-ch.Chen@mediatek.com> wrote:
> > > > > > > > > > > > >
> > > > > > > > > > > > > Hi Tomasz,
> > > > > > > > > > > > >
> > > > > > > > > > > > > On Fri, 2019-08-30 at 16:33 +0800, Tomasz Figa wrote:
> > > > > > > > > > > > > > On Wed, Aug 28, 2019 at 11:00 AM Jerry-ch Chen
> > > > > > > > > > > > > > <Jerry-ch.Chen@mediatek.com> wrote:
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > Hi Tomasz,
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > On Mon, 2019-08-26 at 14:36 +0800, Tomasz Figa wrote:
> > > > > > > > > > > > > > > > Hi Jerry,
> > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > On Sun, Aug 25, 2019 at 6:18 PM Jerry-ch Chen
> > > > > > > > > > > > > > > > <Jerry-ch.Chen@mediatek.com> wrote:
> > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > Hi Tomasz,
> > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > On Fri, 2019-08-02 at 16:28 +0800, Tomasz Figa wrote:
> > > > > > > > > > > > > > > > > > Hi Jerry,
> > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > On Tue, Jul 09, 2019 at 04:41:12PM +0800, Jerry-ch Chen wrote:
> > > > > > > [snip]
> > > > > > > > > > > > > > > > [snip]
> > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > +static void mtk_fd_vb2_stop_streaming(struct vb2_queue *vq)
> > > > > > > > > > > > > > > > > > > +{
> > > > > > > > > > > > > > > > > > > +   struct mtk_fd_ctx *ctx = vb2_get_drv_priv(vq);
> > > > > > > > > > > > > > > > > > > +   struct vb2_buffer *vb;
> > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > How do we guarantee here that the hardware isn't still accessing the buffers
> > > > > > > > > > > > > > > > > > removed below?
> > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > Maybe we can check the driver state flag and aborting the unfinished
> > > > > > > > > > > > > > > > > jobs?
> > > > > > > > > > > > > > > > > (fd_hw->state == FD_ENQ)
> > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > Yes, we need to either cancel or wait for the currently processing
> > > > > > > > > > > > > > > > job. It depends on hardware capabilities, but cancelling is generally
> > > > > > > > > > > > > > > > preferred for the lower latency.
> > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > Ok, it the state is ENQ, then we can disable the FD hw by controlling
> > > > > > > > > > > > > > > the registers.
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > for example:
> > > > > > > > > > > > > > >         writel(0x0, fd->fd_base + FD_HW_ENABLE);
> > > > > > > > > > > > > > >         writel(0x0, fd->fd_base + FD_INT_EN);
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > What's exactly the effect of writing 0 to FD_HW_ENABLE?
> > > > > > > > > > > > > >
> > > > > > > > > > > > > Sorry, my last reply didn't solve the question,
> > > > > > > > > > > > > we should implement a mtk_fd_job_abort() for v4l2_m2m_ops().
> > > > > > > > > > > > >
> > > > > > > > > > > > > which is able to readl_poll_timeout_atomic()
> > > > > > > > > > > > > and check the HW busy bits in the register FD_INT_EN;
> > > > > > > > > > > > >
> > > > > > > > > > > > > if they are not cleared until timeout, we could handle the last
> > > > > > > > > > > > > processing job.
> > > > > > > > > > > > > Otherwise, the FD irq handler should have handled the last processing
> > > > > > > > > > > > > job and we could continue the stop_streaming().
> > > > > > > > > > > > >
> > > > > > > > > > > > > For job_abort():
> > > > > > > > > > > > > static void mtk_fd_job_abort(void *priv)
> > > > > > > > > > > > > {
> > > > > > > > > > > > >         struct mtk_fd_ctx *ctx = priv;
> > > > > > > > > > > > >         struct mtk_fd_dev *fd = ctx->fd_dev;
> > > > > > > > > > > > >         u32 val;
> > > > > > > > > > > > >         u32 ret;
> > > > > > > > > > > > >
> > > > > > > > > > > > >         ret = readl_poll_timeout_atomic(fd->fd_base + MTK_FD_REG_OFFSET_INT_EN,
> > > > > > > > > > > > >                                         val,
> > > > > > > > > > > > >                                         (val & MTK_FD_HW_BUSY_MASK) ==
> > > > > > > > > > > > >                                         MTK_FD_HW_STATE_IS_BUSY,
> > > > > > > > > > > > >                                         USEC_PER_MSEC, MTK_FD_STOP_HW_TIMEOUT);
> > > > > > > > > > > >
> > > > > > > > > > > > Hmm, would it be possible to avoid the busy wait by having a
> > > > > > > > > > > > completion that could be signalled from the interrupt handler?
> > > > > > > > > > > >
> > > > > > > > > > > > Best regards,
> > > > > > > > > > > > Tomasz
> > > > > > > > > > >
> > > > > > > > > > > I suppose that would be wakeup a wait queue in the interrupt handler,
> > > > > > > > > > > the the wait_event_interrupt_timeout() will be used in here and system
> > > > > > > > > > > suspend e.g. mtk_fd_suspend().
> > > > > > > > > >
> > > > > > > > > > Yes, that should work.
> > > > > > > > > >
> > > > > > > > > > > Or do you suggest to wait_event_interrupt_timeout() every frame in the
> > > > > > > > > > > mtk_fd_ipi_handler()?
> > > > > > > > > >
> > > > > > > > > > Nope, we shouldn't need that.
> > > > > > > > > >
> > > > > > > > > > > I think maybe the readl_poll_timeout_atomic would be good enough.
> > > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > Not really. Busy waiting should be avoided as much as possible. What's
> > > > > > > > > > the point of entering suspend if you end up burning the power by
> > > > > > > > > > spinning the CPU for some milliseconds?
> > > > > > > > > >
> > > > > > > > > Ok, I see, busy waiting is not a good idea, I will use the wait queue
> > > > > > > > > instead. the job abort will refine as following:
> > > > > > > > >
> > > > > > > > > static void mtk_fd_job_abort(void *priv)
> > > > > > > > > {
> > > > > > > > >         struct mtk_fd_ctx *ctx = priv;
> > > > > > > > >         struct mtk_fd_dev *fd = ctx->fd_dev;
> > > > > > > > >         u32 ret;
> > > > > > > > >
> > > > > > > > >         ret = wait_event_interruptible_timeout
> > > > > > > > >                 (fd->wq, (fd->fd_irq_result & MTK_FD_HW_IRQ_MASK),
> > > > > > > > >                  usecs_to_jiffies(50000));
> > > > > > > > >         if (ret)
> > > > > > > > >                 mtk_fd_hw_job_finish(fd, VB2_BUF_STATE_ERROR);
> > > > > > > > >         dev_dbg(fd->dev, "%s, ret:%d\n", __func__, ret);
> > > > > > > > >
> > > > > > > > >         fd->fd_irq_result = 0;
> > > > > > > > > }
> > > > > > > > >
> > > > > > > > > static struct v4l2_m2m_ops fd_m2m_ops = {
> > > > > > > > >         .device_run = mtk_fd_device_run,
> > > > > > > > >         .job_abort = mtk_fd_job_abort,
> > > > > > > >
> > > > > > > > I'm not sure we should be using the functon above as the .job_abort
> > > > > > > > callback. It's expected to abort the job, but we're just waiting for
> > > > > > > > it to finish. I think we should just call mtk_fd_job_abort() manually
> > > > > > > > from .stop_streaming.
> > > > > > > >
> > > > > > >
> > > > > > > Ok, I will fix it.
> > > > > > >
> > > > > > > > > };
> > > > > > > > >
> > > > > > > > > and we could use it in suspend.
> > > > > > > > > static int mtk_fd_suspend(struct device *dev)
> > > > > > > > > {
> > > > > > > > >         struct mtk_fd_dev *fd = dev_get_drvdata(dev);
> > > > > > > > >
> > > > > > > > >         if (pm_runtime_suspended(dev))
> > > > > > > > >                 return 0;
> > > > > > > > >
> > > > > > > > >         if (fd->fd_stream_count)
> > > > > > > > >                 mtk_fd_job_abort(fd->ctx);
> > > > > > > > >
> > > > > > > > >         /* suspend FD HW */
> > > > > > > > >         writel(0x0, fd->fd_base + MTK_FD_REG_OFFSET_INT_EN);
> > > > > > > > >         writel(0x0, fd->fd_base + MTK_FD_REG_OFFSET_HW_ENABLE);
> > > > > > > > >         clk_disable_unprepare(fd->fd_clk);
> > > > > > > > >         dev_dbg(dev, "%s:disable clock\n", __func__);
> > > > > > > > >
> > > > > > > > >         return 0;
> > > > > > > > > }
> > > > > > > > >
> > > > > > > > > static irqreturn_t mtk_fd_irq(int irq, void *data)
> > > > > > > > > {
> > > > > > > > >         struct mtk_fd_dev *fd = (struct mtk_fd_dev *)data;
> > > > > > > > >
> > > > > > > > >         fd->fd_irq_result = readl(fd->fd_base + MTK_FD_REG_OFFSET_INT_VAL);
> > > > > > > > >         wake_up_interruptible(&fd->wq);
> > > > > > > >
> > > > > > > > The wake up should be done at the very end of this function. Otherwise
> > > > > > > > we end up with m2m framework racing with the mtk_fd_hw_job_finish()
> > > > > > > > below.
> > > > > > > >
> > > > > > > > Also, I'd use a completion here rather than an open coded wait and
> > > > > > > > wake-up. The driver could reinit_completion() before queuing a job to
> > > > > > > > the hardware and the IRQ handler would complete(). There would be no
> > > > > > > > need to store the IRQ flags in driver data anymore.
> > > > > > > Ok, It will be refined as following:
> > > > > > >
> > > > > > > suspend and stop streaming will call mtk_fd_job_abort()
> > > > > > >
> > > > > > > static void mtk_fd_job_abort(struct mtk_fd_dev *fd)
> > > > > > > {
> > > > > > >         u32 ret;
> > > > > > >
> > > > > > >         ret = wait_for_completion_timeout(&fd->fd_irq_done,
> > > > > > >                                           msecs_to_jiffies(MTK_FD_HW_TIMEOUT));
> > > > > > >         if (ret)
> > > > > >
> > > > > > For the _timeout variants, !ret means the timeout and ret > 0 means
> > > > > > that the wait ended successfully before.
> > > > > >
> > > > > Thanks, fixed.
> > > > >
> > > > > > Also please make sure that the timeout is big enough not to happen
> > > > > > normally on a heavily loaded system. Something like 1 second should be
> > > > > > good.
> > > > > >
> > > > > Ok, I will make it 1 second
> > > > > #define MTK_FD_HW_TIMEOUT 1000
> > > > >
> > > > > Also, I will add a condition in mtk_fd_vb2_stop_streaming(), since it
> > > > > would be called twice, now it works fine whether I reuse the condition
> > > > > with mtk_fd_hw_disconnect or not, however, I think do it before buffer
> > > > > remove looks more reasonable.
> > > > >
> > > > > static void mtk_fd_vb2_stop_streaming(struct vb2_queue *vq)
> > > > > {
> > > > >         struct mtk_fd_ctx *ctx = vb2_get_drv_priv(vq);
> > > > >         struct mtk_fd_dev *fd = ctx->fd_dev;
> > > > >         struct vb2_v4l2_buffer *vb;
> > > > >         struct v4l2_m2m_ctx *m2m_ctx = ctx->fh.m2m_ctx;
> > > > >         struct v4l2_m2m_queue_ctx *queue_ctx;
> > > > >
> > > > >         if (vq->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE)
> > > > >                 mtk_fd_job_abort(fd);
> > > >
> > > > We need to do it regardless of the queue type, otherwise we could
> > > > still free CAPTURE buffers before the hardware releases them.
> > > >
> > >
> > > I think we may read the fd->fd_irq_done.done and do wait for completion
> > > if it's not being done.
> > > How do you think?
> > >
> > > static void mtk_fd_vb2_stop_streaming(struct vb2_queue *vq)
> > > {
> > >         struct mtk_fd_ctx *ctx = vb2_get_drv_priv(vq);
> > >         struct mtk_fd_dev *fd = ctx->fd_dev;
> > >         struct vb2_v4l2_buffer *vb;
> > >         struct v4l2_m2m_ctx *m2m_ctx = ctx->fh.m2m_ctx;
> > >         struct v4l2_m2m_queue_ctx *queue_ctx;
> > >         u32 ret;
> > >
> > >         if (!fd->fd_irq_done.done)
> >
> > We shouldn't access internal fields of completion.
> >
> > >                 ret = wait_for_completion_timeout(&fd->fd_irq_done,
> > >                                                   msecs_to_jiffies(
> > >                                                         MTK_FD_HW_TIMEOUT));
> > >         queue_ctx = V4L2_TYPE_IS_OUTPUT(vq->type) ?
> > >                                         &m2m_ctx->out_q_ctx :
> > >                                         &m2m_ctx->cap_q_ctx;
> > >         while ((vb = v4l2_m2m_buf_remove(queue_ctx)))
> > >                 v4l2_m2m_buf_done(vb, VB2_BUF_STATE_ERROR);
> > >
> > >         if (vq->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE)
> > >                 mtk_fd_hw_disconnect(fd);
> > > }
> > >
> > > I've also tried to wait completion unconditionally for both queues and
> > > the second time will wait until timeout, as a result, it takes longer to
> > > swap the camera every time and close the camera app.
> >
> > I think it should work better if we call complete_all() instead of complete().
> >
> Thanks,
>
> I use complete_all(), and it works fine now.
>
> static void mtk_fd_vb2_stop_streaming(struct vb2_queue *vq)
> {
>         struct mtk_fd_ctx *ctx = vb2_get_drv_priv(vq);
>         struct mtk_fd_dev *fd = ctx->fd_dev;
>         struct vb2_v4l2_buffer *vb;
>         struct v4l2_m2m_ctx *m2m_ctx = ctx->fh.m2m_ctx;
>         struct v4l2_m2m_queue_ctx *queue_ctx;
>
>         wait_for_completion_timeout(&fd->fd_irq_done,
>                                           msecs_to_jiffies(MTK_FD_HW_TIMEOUT));

Shouldn't we still send some command to the hardware to stop? Like a
reset. Otherwise we don't know if it isn't still accessing the memory.

>         queue_ctx = V4L2_TYPE_IS_OUTPUT(vq->type) ?
>                                         &m2m_ctx->out_q_ctx :
>                                         &m2m_ctx->cap_q_ctx;
>         while ((vb = v4l2_m2m_buf_remove(queue_ctx)))
>                 v4l2_m2m_buf_done(vb, VB2_BUF_STATE_ERROR);
>
>         if (vq->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE)
>                 mtk_fd_hw_disconnect(fd);
> }
>
> Best regards,
> Jerry
>
> > Best regards,
> > Tomasz
>
>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH] drm: bridge/dw_hdmi: add audio sample channel status setting
From: Cheng-yi Chiang @ 2019-09-04  9:13 UTC (permalink / raw)
  To: Neil Armstrong
  Cc: moderated list:SOUND - SOC LAYER / DYNAMIC AUDIO POWER MANAGEM...,
	kuninori.morimoto.gx, cain.cai, David Airlie, dri-devel,
	linux-kernel, Andrzej Hajda, Laurent Pinchart, sam, Xing Zheng,
	linux-rockchip, Dylan Reid, tzungbi, Jonas Karlman, Jeffy Chen,
	蔡枫, linux-arm-kernel, Jernej Skrabec, Doug Anderson,
	Daniel Vetter, Enric Balletbo i Serra, kuankuan.y
In-Reply-To: <d8a80ba5-dd2b-f84d-bbfc-9dd5ccbc26e9@baylibre.com>

On Wed, Sep 4, 2019 at 2:00 AM Neil Armstrong <narmstrong@baylibre.com> wrote:
>
> Hi,
>
> Le 03/09/2019 à 11:53, Neil Armstrong a écrit :
> > Hi,
> >
> > On 03/09/2019 07:51, Cheng-Yi Chiang wrote:
> >> From: Yakir Yang <ykk@rock-chips.com>
> >>
> >> When transmitting IEC60985 linear PCM audio, we configure the
> >> Audio Sample Channel Status information of all the channel
> >> status bits in the IEC60958 frame.
> >> Refer to 60958-3 page 10 for frequency, original frequency, and
> >> wordlength setting.
> >>
> >> This fix the issue that audio does not come out on some monitors
> >> (e.g. LG 22CV241)
> >>
> >> Signed-off-by: Yakir Yang <ykk@rock-chips.com>
> >> Signed-off-by: Cheng-Yi Chiang <cychiang@chromium.org>
> >> ---
> >>  drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 59 +++++++++++++++++++++++
> >>  drivers/gpu/drm/bridge/synopsys/dw-hdmi.h | 20 ++++++++
> >>  2 files changed, 79 insertions(+)
> >>
> >> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> >> index bd65d0479683..34d46e25d610 100644
> >> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> >> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> >> @@ -582,6 +582,63 @@ static unsigned int hdmi_compute_n(unsigned int freq, unsigned long pixel_clk)
> >>      return n;
> >>  }
> >>
> >> +static void hdmi_set_schnl(struct dw_hdmi *hdmi)
> >> +{
> >> +    u8 aud_schnl_samplerate;
> >> +    u8 aud_schnl_8;
> >> +
> >> +    /* These registers are on RK3288 using version 2.0a. */
> >> +    if (hdmi->version != 0x200a)
> >> +            return;
> >
> > Are these limited to the 2.0a version *in* RK3288, or 2.0a version on all
> > SoCs ?
>
> After investigations, Amlogic sets these registers on their 2.0a version
> aswell, and Jernej (added in Cc) reported me Allwinner sets them on their
> < 2.0a and > 2.0a IPs versions.
>
> Can you check on the Rockchip IP versions in RK3399 ?
>
Sorry, the RK3399 board I am using is using DP, not HDMI.
But I found that on rockchip's tree at

https://github.com/rockchip-linux/kernel/commit/924f480383c982da9908fb96d6bbb580b25545a5#diff-f74b4cfb23436a137a9338a5af3fbb3dR172

There is such register setting, so I think RK3399 should have the same register.


> For reference, the HDMI 1.4a IP version allwinner setups is:
> https://github.com/Allwinner-Homlet/H3-BSP4.4-linux/blob/master/drivers/video/fbdev/sunxi/disp2/hdmi/hdmi_bsp_sun8iw7.c#L531-L539
> (registers a "scrambled" but a custom bit can reset to the original mapping,
> 0x1066 ... 0x106f)

I see.. so 1.4 has this register.
I can modify the check to be >= 1.4 then.
Will fix in v2.

Thanks!

>
> Neil
>
> >
> >> +
> >> +    switch (hdmi->sample_rate) {
> >> +    case 32000:
> >> +            aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_32K;
> >> +            break;
> >> +    case 44100:
> >> +            aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_44K1;
> >> +            break;
> >> +    case 48000:
> >> +            aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_48K;
> >> +            break;
> >> +    case 88200:
> >> +            aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_88K2;
> >> +            break;
> >> +    case 96000:
> >> +            aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_96K;
> >> +            break;
> >> +    case 176400:
> >> +            aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_176K4;
> >> +            break;
> >> +    case 192000:
> >> +            aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_192K;
> >> +            break;
> >> +    case 768000:
> >> +            aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_768K;
> >> +            break;
> >> +    default:
> >> +            dev_warn(hdmi->dev, "Unsupported audio sample rate (%u)\n",
> >> +                     hdmi->sample_rate);
> >> +            return;
> >> +    }
> >> +
> >> +    /* set channel status register */
> >> +    hdmi_modb(hdmi, aud_schnl_samplerate, HDMI_FC_AUDSCHNLS7_SMPRATE_MASK,
> >> +              HDMI_FC_AUDSCHNLS7);
> >> +
> >> +    /*
> >> +     * Set original frequency to be the same as frequency.
> >> +     * Use one-complement value as stated in IEC60958-3 page 13.
> >> +     */
> >> +    aud_schnl_8 = (~aud_schnl_samplerate) <<
> >> +                    HDMI_FC_AUDSCHNLS8_ORIGSAMPFREQ_OFFSET;
> >> +
> >> +    /* This means word length is 16 bit. Refer to IEC60958-3 page 12. */
> >> +    aud_schnl_8 |= 2 << HDMI_FC_AUDSCHNLS8_WORDLEGNTH_OFFSET;
> >> +
> >> +    hdmi_writeb(hdmi, aud_schnl_8, HDMI_FC_AUDSCHNLS8);
> >> +}
> >> +
> >>  static void hdmi_set_clk_regenerator(struct dw_hdmi *hdmi,
> >>      unsigned long pixel_clk, unsigned int sample_rate)
> >>  {
> >> @@ -620,6 +677,8 @@ static void hdmi_set_clk_regenerator(struct dw_hdmi *hdmi,
> >>      hdmi->audio_cts = cts;
> >>      hdmi_set_cts_n(hdmi, cts, hdmi->audio_enable ? n : 0);
> >>      spin_unlock_irq(&hdmi->audio_lock);
> >> +
> >> +    hdmi_set_schnl(hdmi);
> >>  }
> >>
> >>  static void hdmi_init_clk_regenerator(struct dw_hdmi *hdmi)
> >> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h
> >> index 6988f12d89d9..619ebc1c8354 100644
> >> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h
> >> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h
> >> @@ -158,6 +158,17 @@
> >>  #define HDMI_FC_SPDDEVICEINF                    0x1062
> >>  #define HDMI_FC_AUDSCONF                        0x1063
> >>  #define HDMI_FC_AUDSSTAT                        0x1064
> >> +#define HDMI_FC_AUDSV                           0x1065
> >> +#define HDMI_FC_AUDSU                           0x1066
> >> +#define HDMI_FC_AUDSCHNLS0                      0x1067
> >> +#define HDMI_FC_AUDSCHNLS1                      0x1068
> >> +#define HDMI_FC_AUDSCHNLS2                      0x1069
> >> +#define HDMI_FC_AUDSCHNLS3                      0x106a
> >> +#define HDMI_FC_AUDSCHNLS4                      0x106b
> >> +#define HDMI_FC_AUDSCHNLS5                      0x106c
> >> +#define HDMI_FC_AUDSCHNLS6                      0x106d
> >> +#define HDMI_FC_AUDSCHNLS7                      0x106e
> >> +#define HDMI_FC_AUDSCHNLS8                      0x106f
> >>  #define HDMI_FC_DATACH0FILL                     0x1070
> >>  #define HDMI_FC_DATACH1FILL                     0x1071
> >>  #define HDMI_FC_DATACH2FILL                     0x1072
> >> @@ -706,6 +717,15 @@ enum {
> >>  /* HDMI_FC_AUDSCHNLS7 field values */
> >>      HDMI_FC_AUDSCHNLS7_ACCURACY_OFFSET = 4,
> >>      HDMI_FC_AUDSCHNLS7_ACCURACY_MASK = 0x30,
> >> +    HDMI_FC_AUDSCHNLS7_SMPRATE_MASK = 0x0f,
> >> +    HDMI_FC_AUDSCHNLS7_SMPRATE_192K = 0xe,
> >> +    HDMI_FC_AUDSCHNLS7_SMPRATE_176K4 = 0xc,
> >> +    HDMI_FC_AUDSCHNLS7_SMPRATE_96K = 0xa,
> >> +    HDMI_FC_AUDSCHNLS7_SMPRATE_768K = 0x9,
> >> +    HDMI_FC_AUDSCHNLS7_SMPRATE_88K2 = 0x8,
> >> +    HDMI_FC_AUDSCHNLS7_SMPRATE_32K = 0x3,
> >> +    HDMI_FC_AUDSCHNLS7_SMPRATE_48K = 0x2,
> >> +    HDMI_FC_AUDSCHNLS7_SMPRATE_44K1 = 0x0,
> >>
> >>  /* HDMI_FC_AUDSCHNLS8 field values */
> >>      HDMI_FC_AUDSCHNLS8_ORIGSAMPFREQ_MASK = 0xf0,
> >>
> >

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v1 1/3] perf cs-etm: Refactor instruction size handling
From: Leo Yan @ 2019-09-04  9:19 UTC (permalink / raw)
  To: Mathieu Poirier
  Cc: Suzuki K Poulose, Alexander Shishkin, linux-kernel,
	Arnaldo Carvalho de Melo, Adrian Hunter, Namhyung Kim,
	Robert Walker, Jiri Olsa, linux-arm-kernel, Mike Leach
In-Reply-To: <20190903222215.GD25787@xps15>

Hi Mathieu,

On Tue, Sep 03, 2019 at 04:22:15PM -0600, Mathieu Poirier wrote:
> On Fri, Aug 30, 2019 at 02:24:19PM +0800, Leo Yan wrote:
> > There has several code pieces need to know the instruction size, but
> > now every place calculates the instruction size separately.
> > 
> > This patch refactors to create a new function cs_etm__instr_size() as
> > a central place to analyze the instruction length based on ISA type
> > and instruction value.
> > 
> > Signed-off-by: Leo Yan <leo.yan@linaro.org>
> > ---
> >  tools/perf/util/cs-etm.c | 44 +++++++++++++++++++++++++++-------------
> >  1 file changed, 30 insertions(+), 14 deletions(-)
> > 
> > diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
> > index b3a5daaf1a8f..882a0718033d 100644
> > --- a/tools/perf/util/cs-etm.c
> > +++ b/tools/perf/util/cs-etm.c
> > @@ -914,6 +914,26 @@ static inline int cs_etm__t32_instr_size(struct cs_etm_queue *etmq,
> >  	return ((instrBytes[1] & 0xF8) >= 0xE8) ? 4 : 2;
> >  }
> >  
> > +static inline int cs_etm__instr_size(struct cs_etm_queue *etmq,
> > +				     u8 trace_chan_id,
> > +				     enum cs_etm_isa isa,
> > +				     u64 addr)
> > +{
> > +	int insn_len;
> > +
> > +	/*
> > +	 * T32 instruction size might be 32-bit or 16-bit, decide by calling
> > +	 * cs_etm__t32_instr_size().
> > +	 */
> > +	if (isa == CS_ETM_ISA_T32)
> > +		insn_len = cs_etm__t32_instr_size(etmq, trace_chan_id, addr);
> > +	/* Otherwise, A64 and A32 instruction size are always 32-bit. */
> > +	else
> > +		insn_len = 4;
> > +
> > +	return insn_len;
> > +}
> > +
> >  static inline u64 cs_etm__first_executed_instr(struct cs_etm_packet *packet)
> >  {
> >  	/* Returns 0 for the CS_ETM_DISCONTINUITY packet */
> > @@ -938,19 +958,23 @@ static inline u64 cs_etm__instr_addr(struct cs_etm_queue *etmq,
> >  				     const struct cs_etm_packet *packet,
> >  				     u64 offset)
> >  {
> > +	int insn_len;
> > +
> >  	if (packet->isa == CS_ETM_ISA_T32) {
> >  		u64 addr = packet->start_addr;
> >  
> >  		while (offset > 0) {
> > -			addr += cs_etm__t32_instr_size(etmq,
> > -						       trace_chan_id, addr);
> > +			addr += cs_etm__instr_size(etmq, trace_chan_id,
> > +						   packet->isa, addr);
> >  			offset--;
> >  		}
> >  		return addr;
> >  	}
> >  
> > -	/* Assume a 4 byte instruction size (A32/A64) */
> > -	return packet->start_addr + offset * 4;
> > +	/* Return instruction size for A32/A64 */
> > +	insn_len = cs_etm__instr_size(etmq, trace_chan_id,
> > +				      packet->isa, packet->start_addr);
> > +	return packet->start_addr + offset * insn_len;
> 
> This patch will work but from where I stand it makes things difficult to
> understand more than anything else.  It is also adding coupling between function
> cs_etm__instr_addr() and cs_etm__instr_size(), meaning the code needs to be
> carefully inspected in order to make changes to either one.

My purpose is to use a same place to calculate the instruction
size, rather than to spread the duplicate codes in several different
functions.

> Last but not least function cs_etm__instr_size() isn't used in the upcoming
> patches.  I really don't see what is gained here. 

Sorry that I forgot to commit my final change into patch 02.

I planed to use cs_etm__instr_size() in patch 02; patch 02 has
function cs_etm__add_stack_event(), which also needs to get the
instruction size when it sends stack event.

After apply patch 02, tools/perf/util/cs-etm.c will have below three
functions to caculate instruction size; this is the main reason I want
to refactor the code for instruction size.

  cs_etm__instr_addr()
  cs_etm__copy_insn()
  cs_etm__add_stack_event()

If this lets code more difficult to understand, will drop it.

Thanks,
Leo Yan

> >  }
> >  
> >  static void cs_etm__update_last_branch_rb(struct cs_etm_queue *etmq,
> > @@ -1090,16 +1114,8 @@ static void cs_etm__copy_insn(struct cs_etm_queue *etmq,
> >  		return;
> >  	}
> >  
> > -	/*
> > -	 * T32 instruction size might be 32-bit or 16-bit, decide by calling
> > -	 * cs_etm__t32_instr_size().
> > -	 */
> > -	if (packet->isa == CS_ETM_ISA_T32)
> > -		sample->insn_len = cs_etm__t32_instr_size(etmq, trace_chan_id,
> > -							  sample->ip);
> > -	/* Otherwise, A64 and A32 instruction size are always 32-bit. */
> > -	else
> > -		sample->insn_len = 4;
> > +	sample->insn_len = cs_etm__instr_size(etmq, trace_chan_id,
> > +					      packet->isa, sample->ip);
> >  
> >  	cs_etm__mem_access(etmq, trace_chan_id, sample->ip,
> >  			   sample->insn_len, (void *)sample->insn);
> > -- 
> > 2.17.1
> > 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH] drm: bridge/dw_hdmi: add audio sample channel status setting
From: Cheng-yi Chiang @ 2019-09-04  9:24 UTC (permalink / raw)
  To: Jernej Škrabec
  Cc: moderated list:SOUND - SOC LAYER / DYNAMIC AUDIO POWER MANAGEM...,
	kuninori.morimoto.gx, Neil Armstrong, David Airlie, dri-devel,
	Doug Anderson, Andrzej Hajda, Laurent Pinchart, sam, cain.cai,
	Xing Zheng, linux-rockchip, Dylan Reid, tzungbi, Jonas Karlman,
	Jeffy Chen, 蔡枫, linux-arm-kernel, linux-kernel,
	Daniel Vetter, Enric Balletbo i Serra, kuankuan.y
In-Reply-To: <19353031.SdOy5F5fmg@jernej-laptop>

On Wed, Sep 4, 2019 at 2:08 AM Jernej Škrabec <jernej.skrabec@siol.net> wrote:
>
> Hi!
>
> Dne torek, 03. september 2019 ob 20:00:33 CEST je Neil Armstrong napisal(a):
> > Hi,
> >
> > Le 03/09/2019 à 11:53, Neil Armstrong a écrit :
> > > Hi,
> > >
> > > On 03/09/2019 07:51, Cheng-Yi Chiang wrote:
> > >> From: Yakir Yang <ykk@rock-chips.com>
> > >>
> > >> When transmitting IEC60985 linear PCM audio, we configure the
> > >> Audio Sample Channel Status information of all the channel
> > >> status bits in the IEC60958 frame.
> > >> Refer to 60958-3 page 10 for frequency, original frequency, and
> > >> wordlength setting.
> > >>
> > >> This fix the issue that audio does not come out on some monitors
> > >> (e.g. LG 22CV241)
> > >>
> > >> Signed-off-by: Yakir Yang <ykk@rock-chips.com>
> > >> Signed-off-by: Cheng-Yi Chiang <cychiang@chromium.org>
> > >> ---
> > >>
> > >>  drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 59 +++++++++++++++++++++++
> > >>  drivers/gpu/drm/bridge/synopsys/dw-hdmi.h | 20 ++++++++
> > >>  2 files changed, 79 insertions(+)
> > >>
> > >> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> > >> b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c index
> > >> bd65d0479683..34d46e25d610 100644
> > >> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> > >> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> > >> @@ -582,6 +582,63 @@ static unsigned int hdmi_compute_n(unsigned int
> > >> freq, unsigned long pixel_clk)>>
> > >>    return n;
> > >>
> > >>  }
> > >>
> > >> +static void hdmi_set_schnl(struct dw_hdmi *hdmi)
> > >> +{
> > >> +  u8 aud_schnl_samplerate;
> > >> +  u8 aud_schnl_8;
> > >> +
> > >> +  /* These registers are on RK3288 using version 2.0a. */
> > >> +  if (hdmi->version != 0x200a)
> > >> +          return;
> > >
> > > Are these limited to the 2.0a version *in* RK3288, or 2.0a version on all
> > > SoCs ?
> >
> > After investigations, Amlogic sets these registers on their 2.0a version
> > aswell, and Jernej (added in Cc) reported me Allwinner sets them on their
> > < 2.0a and > 2.0a IPs versions.
> >
> > Can you check on the Rockchip IP versions in RK3399 ?
> >
> > For reference, the HDMI 1.4a IP version allwinner setups is:
> > https://github.com/Allwinner-Homlet/H3-BSP4.4-linux/blob/master/drivers/vide
> > o/fbdev/sunxi/disp2/hdmi/hdmi_bsp_sun8iw7.c#L531-L539 (registers a
> > "scrambled" but a custom bit can reset to the original mapping, 0x1066 ...
> > 0x106f)
>
> For easier reading, here is similar, but annotated version: http://ix.io/1Ub6
> Check function bsp_hdmi_audio().
>
> Unless there is a special reason, you can just remove that check.
>

Thanks for the great reference.
I also see that I need to set the word length according to the desired
value passed by dw_hdmi_i2s_hw_params in dw-hdmi-i2s-audio.c.
Will fix in v2.

> Best regards,
> Jernej
>
> >
> > Neil
> >
> > >> +
> > >> +  switch (hdmi->sample_rate) {
> > >> +  case 32000:
> > >> +          aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_32K;
> > >> +          break;
> > >> +  case 44100:
> > >> +          aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_44K1;
> > >> +          break;
> > >> +  case 48000:
> > >> +          aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_48K;
> > >> +          break;
> > >> +  case 88200:
> > >> +          aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_88K2;
> > >> +          break;
> > >> +  case 96000:
> > >> +          aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_96K;
> > >> +          break;
> > >> +  case 176400:
> > >> +          aud_schnl_samplerate =
> HDMI_FC_AUDSCHNLS7_SMPRATE_176K4;
> > >> +          break;
> > >> +  case 192000:
> > >> +          aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_192K;
> > >> +          break;
> > >> +  case 768000:
> > >> +          aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_768K;
> > >> +          break;
> > >> +  default:
> > >> +          dev_warn(hdmi->dev, "Unsupported audio sample rate (%u)
> \n",
> > >> +                   hdmi->sample_rate);
> > >> +          return;
> > >> +  }
> > >> +
> > >> +  /* set channel status register */
> > >> +  hdmi_modb(hdmi, aud_schnl_samplerate,
> HDMI_FC_AUDSCHNLS7_SMPRATE_MASK,
> > >> +            HDMI_FC_AUDSCHNLS7);
> > >> +
> > >> +  /*
> > >> +   * Set original frequency to be the same as frequency.
> > >> +   * Use one-complement value as stated in IEC60958-3 page 13.
> > >> +   */
> > >> +  aud_schnl_8 = (~aud_schnl_samplerate) <<
> > >> +                  HDMI_FC_AUDSCHNLS8_ORIGSAMPFREQ_OFFSET;
> > >> +
> > >> +  /* This means word length is 16 bit. Refer to IEC60958-3 page 12.
> */
> > >> +  aud_schnl_8 |= 2 << HDMI_FC_AUDSCHNLS8_WORDLEGNTH_OFFSET;
> > >> +
> > >> +  hdmi_writeb(hdmi, aud_schnl_8, HDMI_FC_AUDSCHNLS8);
> > >> +}
> > >> +
> > >>
> > >>  static void hdmi_set_clk_regenerator(struct dw_hdmi *hdmi,
> > >>
> > >>    unsigned long pixel_clk, unsigned int sample_rate)
> > >>
> > >>  {
> > >>
> > >> @@ -620,6 +677,8 @@ static void hdmi_set_clk_regenerator(struct dw_hdmi
> > >> *hdmi,>>
> > >>    hdmi->audio_cts = cts;
> > >>    hdmi_set_cts_n(hdmi, cts, hdmi->audio_enable ? n : 0);
> > >>    spin_unlock_irq(&hdmi->audio_lock);
> > >>
> > >> +
> > >> +  hdmi_set_schnl(hdmi);
> > >>
> > >>  }
> > >>
> > >>  static void hdmi_init_clk_regenerator(struct dw_hdmi *hdmi)
> > >>
> > >> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h
> > >> b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h index
> > >> 6988f12d89d9..619ebc1c8354 100644
> > >> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h
> > >> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h
> > >> @@ -158,6 +158,17 @@
> > >>
> > >>  #define HDMI_FC_SPDDEVICEINF                    0x1062
> > >>  #define HDMI_FC_AUDSCONF                        0x1063
> > >>  #define HDMI_FC_AUDSSTAT                        0x1064
> > >>
> > >> +#define HDMI_FC_AUDSV                           0x1065
> > >> +#define HDMI_FC_AUDSU                           0x1066
> > >> +#define HDMI_FC_AUDSCHNLS0                      0x1067
> > >> +#define HDMI_FC_AUDSCHNLS1                      0x1068
> > >> +#define HDMI_FC_AUDSCHNLS2                      0x1069
> > >> +#define HDMI_FC_AUDSCHNLS3                      0x106a
> > >> +#define HDMI_FC_AUDSCHNLS4                      0x106b
> > >> +#define HDMI_FC_AUDSCHNLS5                      0x106c
> > >> +#define HDMI_FC_AUDSCHNLS6                      0x106d
> > >> +#define HDMI_FC_AUDSCHNLS7                      0x106e
> > >> +#define HDMI_FC_AUDSCHNLS8                      0x106f
> > >>
> > >>  #define HDMI_FC_DATACH0FILL                     0x1070
> > >>  #define HDMI_FC_DATACH1FILL                     0x1071
> > >>  #define HDMI_FC_DATACH2FILL                     0x1072
> > >>
> > >> @@ -706,6 +717,15 @@ enum {
> > >>
> > >>  /* HDMI_FC_AUDSCHNLS7 field values */
> > >>
> > >>    HDMI_FC_AUDSCHNLS7_ACCURACY_OFFSET = 4,
> > >>    HDMI_FC_AUDSCHNLS7_ACCURACY_MASK = 0x30,
> > >>
> > >> +  HDMI_FC_AUDSCHNLS7_SMPRATE_MASK = 0x0f,
> > >> +  HDMI_FC_AUDSCHNLS7_SMPRATE_192K = 0xe,
> > >> +  HDMI_FC_AUDSCHNLS7_SMPRATE_176K4 = 0xc,
> > >> +  HDMI_FC_AUDSCHNLS7_SMPRATE_96K = 0xa,
> > >> +  HDMI_FC_AUDSCHNLS7_SMPRATE_768K = 0x9,
> > >> +  HDMI_FC_AUDSCHNLS7_SMPRATE_88K2 = 0x8,
> > >> +  HDMI_FC_AUDSCHNLS7_SMPRATE_32K = 0x3,
> > >> +  HDMI_FC_AUDSCHNLS7_SMPRATE_48K = 0x2,
> > >> +  HDMI_FC_AUDSCHNLS7_SMPRATE_44K1 = 0x0,
> > >>
> > >>  /* HDMI_FC_AUDSCHNLS8 field values */
> > >>
> > >>    HDMI_FC_AUDSCHNLS8_ORIGSAMPFREQ_MASK = 0xf0,
>
>
>
>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [RFC PATCH V2 4/4] platform: mtk-isp: Add Mediatek FD driver
From: Jerry-ch Chen @ 2019-09-04  9:26 UTC (permalink / raw)
  To: Tomasz Figa
  Cc: devicetree@vger.kernel.org, Sean Cheng (鄭昇弘),
	laurent.pinchart+renesas@ideasonboard.com,
	Rynn Wu (吳育恩), srv_heupstream,
	Po-Yang Huang (黃柏陽), mchehab@kernel.org,
	suleiman@chromium.org, shik@chromium.org,
	Jungo Lin (林明俊),
	Sj Huang (黃信璋), yuzhao@chromium.org,
	linux-mediatek@lists.infradead.org, zwisler@chromium.org,
	matthias.bgg@gmail.com, Christie Yu (游雅惠),
	Frederic Chen (陳俊元), hans.verkuil@cisco.com,
	linux-arm-kernel@lists.infradead.org, linux-media@vger.kernel.org
In-Reply-To: <CAAFQd5DWfEEiGthPi=qoxD-mpAWa68GOCi55mqpmagS-tsGYkA@mail.gmail.com>

Hi Tomasz,

On Wed, 2019-09-04 at 17:03 +0800, Tomasz Figa wrote:
> On Wed, Sep 4, 2019 at 6:02 PM Jerry-ch Chen <Jerry-ch.Chen@mediatek.com> wrote:
> >
> > Hi Tomasz,
> >
> > On Wed, 2019-09-04 at 16:25 +0800, Tomasz Figa wrote:
> > > On Wed, Sep 4, 2019 at 5:09 PM Jerry-ch Chen <Jerry-ch.Chen@mediatek.com> wrote:
> > > >
> > > > Hi Tomasz,
> > > >
> > > > On Wed, 2019-09-04 at 14:34 +0800, Tomasz Figa wrote:
> > > > > On Wed, Sep 4, 2019 at 3:09 PM Jerry-ch Chen <Jerry-ch.Chen@mediatek.com> wrote:
> > > > > >
> > > > > > Hi Tomasz,
> > > > > >
> > > > > > On Wed, 2019-09-04 at 12:15 +0800, Tomasz Figa wrote:
> > > > > > > On Wed, Sep 4, 2019 at 12:38 PM Jerry-ch Chen
> > > > > > > <Jerry-ch.Chen@mediatek.com> wrote:
> > > > > > > >
> > > > > > > > Hi Tomasz,
> > > > > > > >
> > > > > > > > On Tue, 2019-09-03 at 20:05 +0800, Tomasz Figa wrote:
> > > > > > > > > On Tue, Sep 3, 2019 at 8:46 PM Jerry-ch Chen <Jerry-ch.Chen@mediatek.com> wrote:
> > > > > > > > > >
> > > > > > > > > > Hi Tomasz,
> > > > > > > > > >
> > > > > > > > > > On Tue, 2019-09-03 at 15:04 +0800, Tomasz Figa wrote:
> > > > > > > > > > > On Tue, Sep 3, 2019 at 3:44 PM Jerry-ch Chen <Jerry-ch.Chen@mediatek.com> wrote:
> > > > > > > > > > > >
> > > > > > > > > > > > On Tue, 2019-09-03 at 13:19 +0800, Tomasz Figa wrote:
> > > > > > > > > > > > > On Mon, Sep 2, 2019 at 8:47 PM Jerry-ch Chen <Jerry-ch.Chen@mediatek.com> wrote:
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > Hi Tomasz,
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > On Fri, 2019-08-30 at 16:33 +0800, Tomasz Figa wrote:
> > > > > > > > > > > > > > > On Wed, Aug 28, 2019 at 11:00 AM Jerry-ch Chen
> > > > > > > > > > > > > > > <Jerry-ch.Chen@mediatek.com> wrote:
> > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > Hi Tomasz,
> > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > On Mon, 2019-08-26 at 14:36 +0800, Tomasz Figa wrote:
> > > > > > > > > > > > > > > > > Hi Jerry,
> > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > On Sun, Aug 25, 2019 at 6:18 PM Jerry-ch Chen
> > > > > > > > > > > > > > > > > <Jerry-ch.Chen@mediatek.com> wrote:
> > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > Hi Tomasz,
> > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > On Fri, 2019-08-02 at 16:28 +0800, Tomasz Figa wrote:
> > > > > > > > > > > > > > > > > > > Hi Jerry,
> > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > On Tue, Jul 09, 2019 at 04:41:12PM +0800, Jerry-ch Chen wrote:
[snip]
> > > > static void mtk_fd_vb2_stop_streaming(struct vb2_queue *vq)
> > > > {
> > > >         struct mtk_fd_ctx *ctx = vb2_get_drv_priv(vq);
> > > >         struct mtk_fd_dev *fd = ctx->fd_dev;
> > > >         struct vb2_v4l2_buffer *vb;
> > > >         struct v4l2_m2m_ctx *m2m_ctx = ctx->fh.m2m_ctx;
> > > >         struct v4l2_m2m_queue_ctx *queue_ctx;
> > > >         u32 ret;
> > > >
> > > >         if (!fd->fd_irq_done.done)
> > >
> > > We shouldn't access internal fields of completion.
> > >
> > > >                 ret = wait_for_completion_timeout(&fd->fd_irq_done,
> > > >                                                   msecs_to_jiffies(
> > > >                                                         MTK_FD_HW_TIMEOUT));
> > > >         queue_ctx = V4L2_TYPE_IS_OUTPUT(vq->type) ?
> > > >                                         &m2m_ctx->out_q_ctx :
> > > >                                         &m2m_ctx->cap_q_ctx;
> > > >         while ((vb = v4l2_m2m_buf_remove(queue_ctx)))
> > > >                 v4l2_m2m_buf_done(vb, VB2_BUF_STATE_ERROR);
> > > >
> > > >         if (vq->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE)
> > > >                 mtk_fd_hw_disconnect(fd);
> > > > }
> > > >
> > > > I've also tried to wait completion unconditionally for both queues and
> > > > the second time will wait until timeout, as a result, it takes longer to
> > > > swap the camera every time and close the camera app.
> > >
> > > I think it should work better if we call complete_all() instead of complete().
> > >
> > Thanks,
> >
> > I use complete_all(), and it works fine now.
> >
> > static void mtk_fd_vb2_stop_streaming(struct vb2_queue *vq)
> > {
> >         struct mtk_fd_ctx *ctx = vb2_get_drv_priv(vq);
> >         struct mtk_fd_dev *fd = ctx->fd_dev;
> >         struct vb2_v4l2_buffer *vb;
> >         struct v4l2_m2m_ctx *m2m_ctx = ctx->fh.m2m_ctx;
> >         struct v4l2_m2m_queue_ctx *queue_ctx;
> >
> >         wait_for_completion_timeout(&fd->fd_irq_done,
> >                                           msecs_to_jiffies(MTK_FD_HW_TIMEOUT));
> 
> Shouldn't we still send some command to the hardware to stop? Like a
> reset. Otherwise we don't know if it isn't still accessing the memory.
> 
I thought no more jobs will be enqueued here when stop_streaming so we
don't need it.
We still could send an ipi command to reset the HW, and wait for it's
callback or we could set the register MTK_FD_REG_OFFSET_HW_ENABLE to
zero to disable the HW.

Best regards,
Jerry

> >         queue_ctx = V4L2_TYPE_IS_OUTPUT(vq->type) ?
> >                                         &m2m_ctx->out_q_ctx :
> >                                         &m2m_ctx->cap_q_ctx;
> >         while ((vb = v4l2_m2m_buf_remove(queue_ctx)))
> >                 v4l2_m2m_buf_done(vb, VB2_BUF_STATE_ERROR);
> >
> >         if (vq->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE)
> >                 mtk_fd_hw_disconnect(fd);
> > }
> >
> > Best regards,
> > Jerry
> >
> > > Best regards,
> > > Tomasz
> >
> >



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* issue when compile 3.18 kernel with later gcc
From: Belisko Marek @ 2019-09-04  9:27 UTC (permalink / raw)
  To: linux-arm-kernel

Hi,

I'm using hisilicon kernel for camera and I'm trying to be able to
build it with some later gcc7. Kernel version is 3.18 and it builds
fine with gcc 4.9 (linaro armhf). When building with gcc7 I got an
error:

arch/arm/mach-hisi/built-in.o: In function `hi_pmc_clear_a17_ac':
arch/arm/mach-hisi/cpu_helper_a7.o:(.text+0x1b4): undefined reference
to `pmc_phys_addr'

In sources is variable defined like:

static u32  pmc_phys_addr;

and then used in this function:

/* call from assable context */
asmlinkage void __naked hi_pmc_clear_a17_ac(void)
{
        asm volatile("\n"
                ..
                ".align  2\n"
                "1:     .word   .\n"
                "       .word   pmc_phys_addr\n"
                );

        unreachable();
}

and when disassemble generated object file variable is not present in
.bss section (while with old gcc it's perfectly fine). Is there any
gcc option how to resolve this issue or should it be solved some other
way. Thanks.

BR,

marek

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH] drm: bridge/dw_hdmi: add audio sample channel status setting
From: Russell King - ARM Linux admin @ 2019-09-04  9:27 UTC (permalink / raw)
  To: Cheng-yi Chiang
  Cc: moderated list:SOUND - SOC LAYER / DYNAMIC AUDIO POWER MANAGEM...,
	kuninori.morimoto.gx, Neil Armstrong, David Airlie, dri-devel,
	cain.cai, Andrzej Hajda, Laurent Pinchart, Yakir Yang, sam,
	Xing Zheng, linux-rockchip, Dylan Reid, tzungbi, Jeffy Chen,
	蔡枫, linux-arm-kernel, Doug Anderson, linux-kernel,
	Daniel Vetter, Enric Balletbo i Serra, kuankuan.y
In-Reply-To: <CAFv8NwKHZM+zTu7GF_J0Xk6hubA2JK4cCsdhsDPOGk=3rnbCZw@mail.gmail.com>

On Wed, Sep 04, 2019 at 05:09:29PM +0800, Cheng-yi Chiang wrote:
> Hi,
> 
> On Tue, Sep 3, 2019 at 5:53 PM Neil Armstrong <narmstrong@baylibre.com> wrote:
> >
> > Hi,
> >
> > On 03/09/2019 07:51, Cheng-Yi Chiang wrote:
> > > From: Yakir Yang <ykk@rock-chips.com>
> > >
> > > When transmitting IEC60985 linear PCM audio, we configure the
> > > Audio Sample Channel Status information of all the channel
> > > status bits in the IEC60958 frame.
> > > Refer to 60958-3 page 10 for frequency, original frequency, and
> > > wordlength setting.
> > >
> > > This fix the issue that audio does not come out on some monitors
> > > (e.g. LG 22CV241)
> > >
> > > Signed-off-by: Yakir Yang <ykk@rock-chips.com>
> > > Signed-off-by: Cheng-Yi Chiang <cychiang@chromium.org>
> > > ---
> > >  drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 59 +++++++++++++++++++++++
> > >  drivers/gpu/drm/bridge/synopsys/dw-hdmi.h | 20 ++++++++
> > >  2 files changed, 79 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> > > index bd65d0479683..34d46e25d610 100644
> > > --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> > > +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> > > @@ -582,6 +582,63 @@ static unsigned int hdmi_compute_n(unsigned int freq, unsigned long pixel_clk)
> > >       return n;
> > >  }
> > >
> > > +static void hdmi_set_schnl(struct dw_hdmi *hdmi)
> > > +{
> > > +     u8 aud_schnl_samplerate;
> > > +     u8 aud_schnl_8;
> > > +
> > > +     /* These registers are on RK3288 using version 2.0a. */
> > > +     if (hdmi->version != 0x200a)
> > > +             return;
> >
> > Are these limited to the 2.0a version *in* RK3288, or 2.0a version on all
> > SoCs ?
> >
> 
> In the original patch by Yakir,
> 
> https://lore.kernel.org/patchwork/patch/539653/   (sorry, I should
> have added this link in the "after the cut" note)
> 
> The fix is limited to version 2.0.
> Since I am only testing on RK3288 with 2.0a, I change the check to 2.0a only.
> I can not test 2.0a version on other SoCs.
> The databook I have at hand is 2.0a (not specific to RK3288) so I
> think all 2.0a should have this register.
> 
> As for other version like version 1.3 on iMX6, there is no such
> register, as stated by Russell
> 
> http://lkml.iu.edu/hypermail/linux/kernel/1501.3/06268.html.

It's likely more to do with how the IP is configured rather than the
version.  The big difference between dw-hdmi used in iMX6 and elsewhere
is that iMX6 uses a built-in AHB audio interface and not I2S.  Elsewhere
uses I2S.

I2S does not have the capability to convey channel status information
(which is required by HDMI).  With AHB, it is encoded into the data in
memory.

So, I think this setup should be done in the I2S driver and not in the
core driver.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH] drm: bridge/dw_hdmi: add audio sample channel status setting
From: Neil Armstrong @ 2019-09-04  9:28 UTC (permalink / raw)
  To: Cheng-yi Chiang
  Cc: moderated list:SOUND - SOC LAYER / DYNAMIC AUDIO POWER MANAGEM...,
	Doug Anderson, kuninori.morimoto.gx, David Airlie, dri-devel,
	linux-kernel, Andrzej Hajda, Laurent Pinchart, Yakir Yang, sam,
	Xing Zheng, linux-rockchip, Dylan Reid, tzungbi, Jonas Karlman,
	Jeffy Chen, 蔡枫, linux-arm-kernel, Jernej Skrabec,
	cain.cai, Daniel Vetter, Enric Balletbo i Serra, kuankuan.y
In-Reply-To: <CAFv8NwKHZM+zTu7GF_J0Xk6hubA2JK4cCsdhsDPOGk=3rnbCZw@mail.gmail.com>

Hi,

On 04/09/2019 11:09, Cheng-yi Chiang wrote:
> Hi,
> 
> On Tue, Sep 3, 2019 at 5:53 PM Neil Armstrong <narmstrong@baylibre.com> wrote:
>>
>> Hi,
>>
>> On 03/09/2019 07:51, Cheng-Yi Chiang wrote:
>>> From: Yakir Yang <ykk@rock-chips.com>
>>>
>>> When transmitting IEC60985 linear PCM audio, we configure the
>>> Audio Sample Channel Status information of all the channel
>>> status bits in the IEC60958 frame.
>>> Refer to 60958-3 page 10 for frequency, original frequency, and
>>> wordlength setting.
>>>
>>> This fix the issue that audio does not come out on some monitors
>>> (e.g. LG 22CV241)
>>>
>>> Signed-off-by: Yakir Yang <ykk@rock-chips.com>
>>> Signed-off-by: Cheng-Yi Chiang <cychiang@chromium.org>
>>> ---
>>>  drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 59 +++++++++++++++++++++++
>>>  drivers/gpu/drm/bridge/synopsys/dw-hdmi.h | 20 ++++++++
>>>  2 files changed, 79 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>>> index bd65d0479683..34d46e25d610 100644
>>> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>>> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>>> @@ -582,6 +582,63 @@ static unsigned int hdmi_compute_n(unsigned int freq, unsigned long pixel_clk)
>>>       return n;
>>>  }
>>>
>>> +static void hdmi_set_schnl(struct dw_hdmi *hdmi)
>>> +{
>>> +     u8 aud_schnl_samplerate;
>>> +     u8 aud_schnl_8;
>>> +
>>> +     /* These registers are on RK3288 using version 2.0a. */
>>> +     if (hdmi->version != 0x200a)
>>> +             return;
>>
>> Are these limited to the 2.0a version *in* RK3288, or 2.0a version on all
>> SoCs ?
>>
> 
> In the original patch by Yakir,
> 
> https://lore.kernel.org/patchwork/patch/539653/   (sorry, I should
> have added this link in the "after the cut" note)
> 
> The fix is limited to version 2.0.
> Since I am only testing on RK3288 with 2.0a, I change the check to 2.0a only.
> I can not test 2.0a version on other SoCs.
> The databook I have at hand is 2.0a (not specific to RK3288) so I
> think all 2.0a should have this register.
> 
> As for other version like version 1.3 on iMX6, there is no such
> register, as stated by Russell
> 
> http://lkml.iu.edu/hypermail/linux/kernel/1501.3/06268.html.

Russell assumes the registers are not present on the iMX6 IP not having
the I2S registers, but they are present on the IPs configured with I2S
at any versions.

My databook version (1.40.a) specifies :

fc_audschnls0 to fc_audschnls8

```
When transmitting IEC60958 linear PCM audio, this registers allow to configure the channel status
information of all the channel status bits in the IEC60958 frame. For the moment this configuration is only
used when the I2S audio interface, General Purpose Audio (GPA), or AHB audio DMA (AHBAUDDMA)
interface is active (for S/PDIF interface this information comes from the stream).
```

But the databook Revision History shows these registers were present since version 1.10a.

I propose you remove the version check, but you only setup these registers when I2S is enabled:
(hdmi_readb(hdmi, HDMI_CONFIG0_ID) & HDMI_CONFIG0_I2S) until a AHBAUDDMA user needs this,
with a small comment explaining the situation.

Neil

> 
> So at least we should check the version.
> Maybe we can set the criteria as version 2.0 or above to make it a safe patch ?
> If there is the same need on other SoC with version < 2.0, it can be
> added later.
> Presumably, there will be databook of that version to help confirming
> this setting.
> 
> Thanks!
>>> +
>>> +     switch (hdmi->sample_rate) {
>>> +     case 32000:
>>> +             aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_32K;
>>> +             break;
>>> +     case 44100:
>>> +             aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_44K1;
>>> +             break;
>>> +     case 48000:
>>> +             aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_48K;
>>> +             break;
>>> +     case 88200:
>>> +             aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_88K2;
>>> +             break;
>>> +     case 96000:
>>> +             aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_96K;
>>> +             break;
>>> +     case 176400:
>>> +             aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_176K4;
>>> +             break;
>>> +     case 192000:
>>> +             aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_192K;
>>> +             break;
>>> +     case 768000:
>>> +             aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_768K;
>>> +             break;
>>> +     default:
>>> +             dev_warn(hdmi->dev, "Unsupported audio sample rate (%u)\n",
>>> +                      hdmi->sample_rate);
>>> +             return;
>>> +     }
>>> +
>>> +     /* set channel status register */
>>> +     hdmi_modb(hdmi, aud_schnl_samplerate, HDMI_FC_AUDSCHNLS7_SMPRATE_MASK,
>>> +               HDMI_FC_AUDSCHNLS7);
>>> +
>>> +     /*
>>> +      * Set original frequency to be the same as frequency.
>>> +      * Use one-complement value as stated in IEC60958-3 page 13.
>>> +      */
>>> +     aud_schnl_8 = (~aud_schnl_samplerate) <<
>>> +                     HDMI_FC_AUDSCHNLS8_ORIGSAMPFREQ_OFFSET;
>>> +
>>> +     /* This means word length is 16 bit. Refer to IEC60958-3 page 12. */
>>> +     aud_schnl_8 |= 2 << HDMI_FC_AUDSCHNLS8_WORDLEGNTH_OFFSET;
>>> +
>>> +     hdmi_writeb(hdmi, aud_schnl_8, HDMI_FC_AUDSCHNLS8);
>>> +}
>>> +
>>>  static void hdmi_set_clk_regenerator(struct dw_hdmi *hdmi,
>>>       unsigned long pixel_clk, unsigned int sample_rate)
>>>  {
>>> @@ -620,6 +677,8 @@ static void hdmi_set_clk_regenerator(struct dw_hdmi *hdmi,
>>>       hdmi->audio_cts = cts;
>>>       hdmi_set_cts_n(hdmi, cts, hdmi->audio_enable ? n : 0);
>>>       spin_unlock_irq(&hdmi->audio_lock);
>>> +
>>> +     hdmi_set_schnl(hdmi);
>>>  }
>>>
>>>  static void hdmi_init_clk_regenerator(struct dw_hdmi *hdmi)
>>> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h
>>> index 6988f12d89d9..619ebc1c8354 100644
>>> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h
>>> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h
>>> @@ -158,6 +158,17 @@
>>>  #define HDMI_FC_SPDDEVICEINF                    0x1062
>>>  #define HDMI_FC_AUDSCONF                        0x1063
>>>  #define HDMI_FC_AUDSSTAT                        0x1064
>>> +#define HDMI_FC_AUDSV                           0x1065
>>> +#define HDMI_FC_AUDSU                           0x1066
>>> +#define HDMI_FC_AUDSCHNLS0                      0x1067
>>> +#define HDMI_FC_AUDSCHNLS1                      0x1068
>>> +#define HDMI_FC_AUDSCHNLS2                      0x1069
>>> +#define HDMI_FC_AUDSCHNLS3                      0x106a
>>> +#define HDMI_FC_AUDSCHNLS4                      0x106b
>>> +#define HDMI_FC_AUDSCHNLS5                      0x106c
>>> +#define HDMI_FC_AUDSCHNLS6                      0x106d
>>> +#define HDMI_FC_AUDSCHNLS7                      0x106e
>>> +#define HDMI_FC_AUDSCHNLS8                      0x106f
>>>  #define HDMI_FC_DATACH0FILL                     0x1070
>>>  #define HDMI_FC_DATACH1FILL                     0x1071
>>>  #define HDMI_FC_DATACH2FILL                     0x1072
>>> @@ -706,6 +717,15 @@ enum {
>>>  /* HDMI_FC_AUDSCHNLS7 field values */
>>>       HDMI_FC_AUDSCHNLS7_ACCURACY_OFFSET = 4,
>>>       HDMI_FC_AUDSCHNLS7_ACCURACY_MASK = 0x30,
>>> +     HDMI_FC_AUDSCHNLS7_SMPRATE_MASK = 0x0f,
>>> +     HDMI_FC_AUDSCHNLS7_SMPRATE_192K = 0xe,
>>> +     HDMI_FC_AUDSCHNLS7_SMPRATE_176K4 = 0xc,
>>> +     HDMI_FC_AUDSCHNLS7_SMPRATE_96K = 0xa,
>>> +     HDMI_FC_AUDSCHNLS7_SMPRATE_768K = 0x9,
>>> +     HDMI_FC_AUDSCHNLS7_SMPRATE_88K2 = 0x8,
>>> +     HDMI_FC_AUDSCHNLS7_SMPRATE_32K = 0x3,
>>> +     HDMI_FC_AUDSCHNLS7_SMPRATE_48K = 0x2,
>>> +     HDMI_FC_AUDSCHNLS7_SMPRATE_44K1 = 0x0,
>>>
>>>  /* HDMI_FC_AUDSCHNLS8 field values */
>>>       HDMI_FC_AUDSCHNLS8_ORIGSAMPFREQ_MASK = 0xf0,
>>>
>>


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [GIT PULL 1/3] soc: samsung: Exynos for v5.4
From: Arnd Bergmann @ 2019-09-04  9:31 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: linux-samsung-soc@vger.kernel.org, linux-kernel@vger.kernel.org,
	SoC Team, arm-soc, Kukjin Kim, Olof Johansson, Linux ARM
In-Reply-To: <CAJKOXPfRMXkm_pT560Ry5k-zFWpkRDmFHSs2Fb3RL7d4h=ka9g@mail.gmail.com>

On Wed, Sep 4, 2019 at 10:37 AM Krzysztof Kozlowski <krzk@kernel.org> wrote:
> On Tue, 3 Sep 2019 at 19:21, Arnd Bergmann <arnd@arndb.de> wrote:
> > On Thu, Aug 22, 2019 at 8:35 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
> > I have drafted a related patch recently, regarding the related
> > arch/arm/plat-samsung/cpu.c file. This is part of a longer series
> > I'm working on, see https://pastebin.com/ZqeU3Mth for the
> > current version of this patch.
>
> You can then also adjust the include path in arch/arm/mach-exynos/Makefile.

Yes, that is part of the following patch, along with replacing all the
'depends on PLAT_SAMSUNG' in Kconfig with 'depends on PLAT_SAMSUNG ||
ARCH_EXYNOS'.

> > The observation is that mach-exynos
> > is almost completely independent of plat-samsung these days, and my
> > patch removes the last obstacle from separating the two. I have
> > another set of patches to do the same for mach-s5pv210 (which shares
> > half of its pm.c with plat-samsung, but nothing else).
>
> Great!

FYI, the other parts of the series include:

- Changing all s3c24xx drivers to no longer use mach/*.h or plat/*.h header
  files, as preparation for multiplatform support. This is work in progress,
  but mostly done, with cpufreq and ASoC as the notable exceptions.
- Merging mach-s3c24xx, mach-s3c64xx and plat-samsung into a common
  mach-s3c directory. This seems to work fine and looks nicer than having
  three tightly connected directories, but the downside is that all path
  names change and the directory becomes fairly large.
  I think we can actually do the same thing for all remaining plat-*
directories.
- eventually making all ARMv5 platforms link into a single kernel, like we do
  with ARMv6 and ARMv7 (more to be done there, but s3c24xx and pxa are
  the harder bits here).

      Arnd

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* [PATCH -next] usb: gadget: bcm63xx_udc: use devm_platform_ioremap_resource() to simplify code
From: YueHaibing @ 2019-09-04  9:32 UTC (permalink / raw)
  To: cernekee, balbi, gregkh, f.fainelli
  Cc: YueHaibing, linux-usb, bcm-kernel-feedback-list, linux-kernel,
	linux-arm-kernel

Use devm_platform_ioremap_resource() to simplify the code a bit.
This is detected by coccinelle.

Reported-by: Hulk Robot <hulkci@huawei.com>
Signed-off-by: YueHaibing <yuehaibing@huawei.com>
---
 drivers/usb/gadget/udc/bcm63xx_udc.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/gadget/udc/bcm63xx_udc.c b/drivers/usb/gadget/udc/bcm63xx_udc.c
index 97b1646..7fcf4a8 100644
--- a/drivers/usb/gadget/udc/bcm63xx_udc.c
+++ b/drivers/usb/gadget/udc/bcm63xx_udc.c
@@ -2282,7 +2282,6 @@ static int bcm63xx_udc_probe(struct platform_device *pdev)
 	struct device *dev = &pdev->dev;
 	struct bcm63xx_usbd_platform_data *pd = dev_get_platdata(dev);
 	struct bcm63xx_udc *udc;
-	struct resource *res;
 	int rc = -ENOMEM, i, irq;
 
 	udc = devm_kzalloc(dev, sizeof(*udc), GFP_KERNEL);
@@ -2298,13 +2297,11 @@ static int bcm63xx_udc_probe(struct platform_device *pdev)
 		return -EINVAL;
 	}
 
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	udc->usbd_regs = devm_ioremap_resource(dev, res);
+	udc->usbd_regs = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(udc->usbd_regs))
 		return PTR_ERR(udc->usbd_regs);
 
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
-	udc->iudma_regs = devm_ioremap_resource(dev, res);
+	udc->iudma_regs = devm_platform_ioremap_resource(pdev, 1);
 	if (IS_ERR(udc->iudma_regs))
 		return PTR_ERR(udc->iudma_regs);
 
-- 
2.7.4



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* Re: [PATCH] drm: bridge/dw_hdmi: add audio sample channel status setting
From: Cheng-yi Chiang @ 2019-09-04  9:35 UTC (permalink / raw)
  To: Jonas Karlman
  Cc: alsa-devel@alsa-project.org, kuninori.morimoto.gx@renesas.com,
	Neil Armstrong, airlied@linux.ie, dri-devel@lists.freedesktop.org,
	linux-kernel@vger.kernel.org, a.hajda@samsung.com,
	Laurent.pinchart@ideasonboard.com, sam@ravnborg.org,
	cain.cai@rock-chips.com, zhengxing@rock-chips.com,
	linux-rockchip@lists.infradead.org, dgreid@chromium.org,
	tzungbi@chromium.org, jeffy.chen@rock-chips.com,
	eddie.cai@rock-chips.com, linux-arm-kernel@lists.infradead.org,
	Jernej Škrabec, dianders@chromium.org, daniel@ffwll.ch,
	enric.balletbo@collabora.com, kuankuan.y@gmail.com
In-Reply-To: <HE1PR06MB40112AD52DAF0E837F23B441ACB90@HE1PR06MB4011.eurprd06.prod.outlook.com>

On Wed, Sep 4, 2019 at 4:33 AM Jonas Karlman <jonas@kwiboo.se> wrote:
>
> On 2019-09-03 20:08, Jernej Škrabec wrote:
> > Hi!
> >
> > Dne torek, 03. september 2019 ob 20:00:33 CEST je Neil Armstrong napisal(a):
> >> Hi,
> >>
> >> Le 03/09/2019 à 11:53, Neil Armstrong a écrit :
> >>> Hi,
> >>>
> >>> On 03/09/2019 07:51, Cheng-Yi Chiang wrote:
> >>>> From: Yakir Yang <ykk@rock-chips.com>
> >>>>
> >>>> When transmitting IEC60985 linear PCM audio, we configure the
> >>>> Audio Sample Channel Status information of all the channel
> >>>> status bits in the IEC60958 frame.
> >>>> Refer to 60958-3 page 10 for frequency, original frequency, and
> >>>> wordlength setting.
> >>>>
> >>>> This fix the issue that audio does not come out on some monitors
> >>>> (e.g. LG 22CV241)
> >>>>
> >>>> Signed-off-by: Yakir Yang <ykk@rock-chips.com>
> >>>> Signed-off-by: Cheng-Yi Chiang <cychiang@chromium.org>
> >>>> ---
> >>>>
> >>>>  drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 59 +++++++++++++++++++++++
> >>>>  drivers/gpu/drm/bridge/synopsys/dw-hdmi.h | 20 ++++++++
> >>>>  2 files changed, 79 insertions(+)
> >>>>
> >>>> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> >>>> b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c index
> >>>> bd65d0479683..34d46e25d610 100644
> >>>> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> >>>> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> >>>> @@ -582,6 +582,63 @@ static unsigned int hdmi_compute_n(unsigned int
> >>>> freq, unsigned long pixel_clk)>>
> >>>>    return n;
> >>>>
> >>>>  }
> >>>>
> >>>> +static void hdmi_set_schnl(struct dw_hdmi *hdmi)
> >>>> +{
> >>>> +  u8 aud_schnl_samplerate;
> >>>> +  u8 aud_schnl_8;
> >>>> +
> >>>> +  /* These registers are on RK3288 using version 2.0a. */
> >>>> +  if (hdmi->version != 0x200a)
> >>>> +          return;
> >>> Are these limited to the 2.0a version *in* RK3288, or 2.0a version on all
> >>> SoCs ?
> >> After investigations, Amlogic sets these registers on their 2.0a version
> >> aswell, and Jernej (added in Cc) reported me Allwinner sets them on their
> >> < 2.0a and > 2.0a IPs versions.
> >>
> >> Can you check on the Rockchip IP versions in RK3399 ?
> >>
> >> For reference, the HDMI 1.4a IP version allwinner setups is:
> >> https://github.com/Allwinner-Homlet/H3-BSP4.4-linux/blob/master/drivers/vide
> >> o/fbdev/sunxi/disp2/hdmi/hdmi_bsp_sun8iw7.c#L531-L539 (registers a
> >> "scrambled" but a custom bit can reset to the original mapping, 0x1066 ...
> >> 0x106f)
> > For easier reading, here is similar, but annotated version: http://ix.io/1Ub6
> > Check function bsp_hdmi_audio().
> >
> > Unless there is a special reason, you can just remove that check.
>
> Agree, this check should not be needed, AUDSCHNLS7 used to be configured in my old
> multi-channel patches that have seen lot of testing on Amlogic, Allwinner and Rockchip SoCs.
>

As stated in previous mail, I will modify the check for version >=1.4
since I know that 1.3 does not have such register, at least on iMX6.

> >
> > Best regards,
> > Jernej
> >
> >> Neil
> >>
> >>>> +
> >>>> +  switch (hdmi->sample_rate) {
> >>>> +  case 32000:
> >>>> +          aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_32K;
> >>>> +          break;
> >>>> +  case 44100:
> >>>> +          aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_44K1;
> >>>> +          break;
> >>>> +  case 48000:
> >>>> +          aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_48K;
> >>>> +          break;
> >>>> +  case 88200:
> >>>> +          aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_88K2;
> >>>> +          break;
> >>>> +  case 96000:
> >>>> +          aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_96K;
> >>>> +          break;
> >>>> +  case 176400:
> >>>> +          aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_176K4;
> >>>> +          break;
> >>>> +  case 192000:
> >>>> +          aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_192K;
> >>>> +          break;
> >>>> +  case 768000:
> >>>> +          aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_768K;
> >>>> +          break;
> >>>> +  default:
> >>>> +          dev_warn(hdmi->dev, "Unsupported audio sample rate (%u)\n",
> >>>> +                   hdmi->sample_rate);
> >>>> +          return;
> >>>> +  }
> >>>> +
> >>>> +  /* set channel status register */
> >>>> +  hdmi_modb(hdmi, aud_schnl_samplerate, HDMI_FC_AUDSCHNLS7_SMPRATE_MASK,
> >>>> +            HDMI_FC_AUDSCHNLS7);
> >>>> +
> >>>> +  /*
> >>>> +   * Set original frequency to be the same as frequency.
> >>>> +   * Use one-complement value as stated in IEC60958-3 page 13.
> >>>> +   */
> >>>> +  aud_schnl_8 = (~aud_schnl_samplerate) <<
> >>>> +                  HDMI_FC_AUDSCHNLS8_ORIGSAMPFREQ_OFFSET;
> >>>> +
> >>>> +  /* This means word length is 16 bit. Refer to IEC60958-3 page 12. */
> >>>> +  aud_schnl_8 |= 2 << HDMI_FC_AUDSCHNLS8_WORDLEGNTH_OFFSET;
>
> This looks wrong, user can use 16 and 24 bit wide audio streams.
>

Thanks for spotting this issue.
I will fix it in v2 (following how http://ix.io/1Ub6 set it for 16 and 24 bit)

> >>>> +
> >>>> +  hdmi_writeb(hdmi, aud_schnl_8, HDMI_FC_AUDSCHNLS8);
> >>>> +}
> >>>> +
> >>>>
> >>>>  static void hdmi_set_clk_regenerator(struct dw_hdmi *hdmi,
> >>>>
> >>>>    unsigned long pixel_clk, unsigned int sample_rate)
> >>>>
> >>>>  {
> >>>>
> >>>> @@ -620,6 +677,8 @@ static void hdmi_set_clk_regenerator(struct dw_hdmi
> >>>> *hdmi,>>
> >>>>    hdmi->audio_cts = cts;
> >>>>    hdmi_set_cts_n(hdmi, cts, hdmi->audio_enable ? n : 0);
> >>>>    spin_unlock_irq(&hdmi->audio_lock);
> >>>>
> >>>> +
> >>>> +  hdmi_set_schnl(hdmi);
>
> I will suggest this function is called from or merged with dw_hdmi_set_sample_rate().
> Similar to how AUDSCONF and AUDICONF0 is configured from dw_hdmi_set_channel_count().
>

I see. I think it will make sense to add a function
set_channel_status() for dw-hdmi-i2s-audio.c to call,
since this function is more than just setting rate.
Will fix in v2.

Thanks!

> Regards,
> Jonas
>
> >>>>
> >>>>  }
> >>>>
> >>>>  static void hdmi_init_clk_regenerator(struct dw_hdmi *hdmi)
> >>>>
> >>>> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h
> >>>> b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h index
> >>>> 6988f12d89d9..619ebc1c8354 100644
> >>>> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h
> >>>> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h
> >>>> @@ -158,6 +158,17 @@
> >>>>
> >>>>  #define HDMI_FC_SPDDEVICEINF                    0x1062
> >>>>  #define HDMI_FC_AUDSCONF                        0x1063
> >>>>  #define HDMI_FC_AUDSSTAT                        0x1064
> >>>>
> >>>> +#define HDMI_FC_AUDSV                           0x1065
> >>>> +#define HDMI_FC_AUDSU                           0x1066
> >>>> +#define HDMI_FC_AUDSCHNLS0                      0x1067
> >>>> +#define HDMI_FC_AUDSCHNLS1                      0x1068
> >>>> +#define HDMI_FC_AUDSCHNLS2                      0x1069
> >>>> +#define HDMI_FC_AUDSCHNLS3                      0x106a
> >>>> +#define HDMI_FC_AUDSCHNLS4                      0x106b
> >>>> +#define HDMI_FC_AUDSCHNLS5                      0x106c
> >>>> +#define HDMI_FC_AUDSCHNLS6                      0x106d
> >>>> +#define HDMI_FC_AUDSCHNLS7                      0x106e
> >>>> +#define HDMI_FC_AUDSCHNLS8                      0x106f
> >>>>
> >>>>  #define HDMI_FC_DATACH0FILL                     0x1070
> >>>>  #define HDMI_FC_DATACH1FILL                     0x1071
> >>>>  #define HDMI_FC_DATACH2FILL                     0x1072
> >>>>
> >>>> @@ -706,6 +717,15 @@ enum {
> >>>>
> >>>>  /* HDMI_FC_AUDSCHNLS7 field values */
> >>>>
> >>>>    HDMI_FC_AUDSCHNLS7_ACCURACY_OFFSET = 4,
> >>>>    HDMI_FC_AUDSCHNLS7_ACCURACY_MASK = 0x30,
> >>>>
> >>>> +  HDMI_FC_AUDSCHNLS7_SMPRATE_MASK = 0x0f,
> >>>> +  HDMI_FC_AUDSCHNLS7_SMPRATE_192K = 0xe,
> >>>> +  HDMI_FC_AUDSCHNLS7_SMPRATE_176K4 = 0xc,
> >>>> +  HDMI_FC_AUDSCHNLS7_SMPRATE_96K = 0xa,
> >>>> +  HDMI_FC_AUDSCHNLS7_SMPRATE_768K = 0x9,
> >>>> +  HDMI_FC_AUDSCHNLS7_SMPRATE_88K2 = 0x8,
> >>>> +  HDMI_FC_AUDSCHNLS7_SMPRATE_32K = 0x3,
> >>>> +  HDMI_FC_AUDSCHNLS7_SMPRATE_48K = 0x2,
> >>>> +  HDMI_FC_AUDSCHNLS7_SMPRATE_44K1 = 0x0,
> >>>>
> >>>>  /* HDMI_FC_AUDSCHNLS8 field values */
> >>>>
> >>>>    HDMI_FC_AUDSCHNLS8_ORIGSAMPFREQ_MASK = 0xf0,

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* [PATCH -next] usb: gadget: pxa25x_udc: use devm_platform_ioremap_resource() to simplify code
From: YueHaibing @ 2019-09-04  9:42 UTC (permalink / raw)
  To: daniel, haojian.zhuang, robert.jarzmik, balbi, gregkh
  Cc: linux-usb, YueHaibing, linux-kernel, linux-arm-kernel

Use devm_platform_ioremap_resource() to simplify the code a bit.
This is detected by coccinelle.

Reported-by: Hulk Robot <hulkci@huawei.com>
Signed-off-by: YueHaibing <yuehaibing@huawei.com>
---
 drivers/usb/gadget/udc/pxa25x_udc.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/usb/gadget/udc/pxa25x_udc.c b/drivers/usb/gadget/udc/pxa25x_udc.c
index d4be535..cfafdd9 100644
--- a/drivers/usb/gadget/udc/pxa25x_udc.c
+++ b/drivers/usb/gadget/udc/pxa25x_udc.c
@@ -2321,7 +2321,6 @@ static int pxa25x_udc_probe(struct platform_device *pdev)
 	struct pxa25x_udc *dev = &memory;
 	int retval, irq;
 	u32 chiprev;
-	struct resource *res;
 
 	pr_info("%s: version %s\n", driver_name, DRIVER_VERSION);
 
@@ -2367,8 +2366,7 @@ static int pxa25x_udc_probe(struct platform_device *pdev)
 	if (irq < 0)
 		return -ENODEV;
 
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	dev->regs = devm_ioremap_resource(&pdev->dev, res);
+	dev->regs = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(dev->regs))
 		return PTR_ERR(dev->regs);
 
-- 
2.7.4



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* Re: [PATCH] drm: bridge/dw_hdmi: add audio sample channel status setting
From: Cheng-yi Chiang @ 2019-09-04  9:45 UTC (permalink / raw)
  To: Neil Armstrong
  Cc: moderated list:SOUND - SOC LAYER / DYNAMIC AUDIO POWER MANAGEM...,
	Doug Anderson, kuninori.morimoto.gx, David Airlie, dri-devel,
	linux-kernel, Andrzej Hajda, Laurent Pinchart, sam, Xing Zheng,
	linux-rockchip, Dylan Reid, tzungbi, Jonas Karlman, Jeffy Chen,
	蔡枫, linux-arm-kernel, Jernej Skrabec, cain.cai,
	Daniel Vetter, Enric Balletbo i Serra, kuankuan.y
In-Reply-To: <1a167659-2eb1-d9be-c1ae-4958ac3f7929@baylibre.com>

On Wed, Sep 4, 2019 at 5:28 PM Neil Armstrong <narmstrong@baylibre.com> wrote:
>
> Hi,
>
> On 04/09/2019 11:09, Cheng-yi Chiang wrote:
> > Hi,
> >
> > On Tue, Sep 3, 2019 at 5:53 PM Neil Armstrong <narmstrong@baylibre.com> wrote:
> >>
> >> Hi,
> >>
> >> On 03/09/2019 07:51, Cheng-Yi Chiang wrote:
> >>> From: Yakir Yang <ykk@rock-chips.com>
> >>>
> >>> When transmitting IEC60985 linear PCM audio, we configure the
> >>> Audio Sample Channel Status information of all the channel
> >>> status bits in the IEC60958 frame.
> >>> Refer to 60958-3 page 10 for frequency, original frequency, and
> >>> wordlength setting.
> >>>
> >>> This fix the issue that audio does not come out on some monitors
> >>> (e.g. LG 22CV241)
> >>>
> >>> Signed-off-by: Yakir Yang <ykk@rock-chips.com>
> >>> Signed-off-by: Cheng-Yi Chiang <cychiang@chromium.org>
> >>> ---
> >>>  drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 59 +++++++++++++++++++++++
> >>>  drivers/gpu/drm/bridge/synopsys/dw-hdmi.h | 20 ++++++++
> >>>  2 files changed, 79 insertions(+)
> >>>
> >>> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> >>> index bd65d0479683..34d46e25d610 100644
> >>> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> >>> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> >>> @@ -582,6 +582,63 @@ static unsigned int hdmi_compute_n(unsigned int freq, unsigned long pixel_clk)
> >>>       return n;
> >>>  }
> >>>
> >>> +static void hdmi_set_schnl(struct dw_hdmi *hdmi)
> >>> +{
> >>> +     u8 aud_schnl_samplerate;
> >>> +     u8 aud_schnl_8;
> >>> +
> >>> +     /* These registers are on RK3288 using version 2.0a. */
> >>> +     if (hdmi->version != 0x200a)
> >>> +             return;
> >>
> >> Are these limited to the 2.0a version *in* RK3288, or 2.0a version on all
> >> SoCs ?
> >>
> >
> > In the original patch by Yakir,
> >
> > https://lore.kernel.org/patchwork/patch/539653/   (sorry, I should
> > have added this link in the "after the cut" note)
> >
> > The fix is limited to version 2.0.
> > Since I am only testing on RK3288 with 2.0a, I change the check to 2.0a only.
> > I can not test 2.0a version on other SoCs.
> > The databook I have at hand is 2.0a (not specific to RK3288) so I
> > think all 2.0a should have this register.
> >
> > As for other version like version 1.3 on iMX6, there is no such
> > register, as stated by Russell
> >
> > http://lkml.iu.edu/hypermail/linux/kernel/1501.3/06268.html.
>
> Russell assumes the registers are not present on the iMX6 IP not having
> the I2S registers, but they are present on the IPs configured with I2S
> at any versions.

I see. Thanks for the check.

>
> My databook version (1.40.a) specifies :
>
> fc_audschnls0 to fc_audschnls8
>
> ```
> When transmitting IEC60958 linear PCM audio, this registers allow to configure the channel status
> information of all the channel status bits in the IEC60958 frame. For the moment this configuration is only
> used when the I2S audio interface, General Purpose Audio (GPA), or AHB audio DMA (AHBAUDDMA)
> interface is active (for S/PDIF interface this information comes from the stream).
> ```
>
> But the databook Revision History shows these registers were present since version 1.10a.
>
> I propose you remove the version check, but you only setup these registers when I2S is enabled:
> (hdmi_readb(hdmi, HDMI_CONFIG0_ID) & HDMI_CONFIG0_I2S) until a AHBAUDDMA user needs this,
> with a small comment explaining the situation.

I see. Sound like a good plan.
In sum, I will add
set_channel_status()
in dw_hdmi.c
And in the beginning of this function,
check it is using I2S
 (hdmi_readb(hdmi, HDMI_CONFIG0_ID) & HDMI_CONFIG0_I2S)
with a comment explaining the situation.

And let dw-hdmi-audio-i2s dw_hdmi_i2s_hw_params() to call this
function after dw_hdmi_set_sample_rate, with word length (or
sample_bit) passed it as argument.
I have not tested setting this register here as in the original patch
it was set in hdmi_set_clk_regenerator.
I will test it and update in my v2.

Thanks again to everyone for the prompt reply and help.

>
> Neil
>
> >
> > So at least we should check the version.
> > Maybe we can set the criteria as version 2.0 or above to make it a safe patch ?
> > If there is the same need on other SoC with version < 2.0, it can be
> > added later.
> > Presumably, there will be databook of that version to help confirming
> > this setting.
> >
> > Thanks!
> >>> +
> >>> +     switch (hdmi->sample_rate) {
> >>> +     case 32000:
> >>> +             aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_32K;
> >>> +             break;
> >>> +     case 44100:
> >>> +             aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_44K1;
> >>> +             break;
> >>> +     case 48000:
> >>> +             aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_48K;
> >>> +             break;
> >>> +     case 88200:
> >>> +             aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_88K2;
> >>> +             break;
> >>> +     case 96000:
> >>> +             aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_96K;
> >>> +             break;
> >>> +     case 176400:
> >>> +             aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_176K4;
> >>> +             break;
> >>> +     case 192000:
> >>> +             aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_192K;
> >>> +             break;
> >>> +     case 768000:
> >>> +             aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_768K;
> >>> +             break;
> >>> +     default:
> >>> +             dev_warn(hdmi->dev, "Unsupported audio sample rate (%u)\n",
> >>> +                      hdmi->sample_rate);
> >>> +             return;
> >>> +     }
> >>> +
> >>> +     /* set channel status register */
> >>> +     hdmi_modb(hdmi, aud_schnl_samplerate, HDMI_FC_AUDSCHNLS7_SMPRATE_MASK,
> >>> +               HDMI_FC_AUDSCHNLS7);
> >>> +
> >>> +     /*
> >>> +      * Set original frequency to be the same as frequency.
> >>> +      * Use one-complement value as stated in IEC60958-3 page 13.
> >>> +      */
> >>> +     aud_schnl_8 = (~aud_schnl_samplerate) <<
> >>> +                     HDMI_FC_AUDSCHNLS8_ORIGSAMPFREQ_OFFSET;
> >>> +
> >>> +     /* This means word length is 16 bit. Refer to IEC60958-3 page 12. */
> >>> +     aud_schnl_8 |= 2 << HDMI_FC_AUDSCHNLS8_WORDLEGNTH_OFFSET;
> >>> +
> >>> +     hdmi_writeb(hdmi, aud_schnl_8, HDMI_FC_AUDSCHNLS8);
> >>> +}
> >>> +
> >>>  static void hdmi_set_clk_regenerator(struct dw_hdmi *hdmi,
> >>>       unsigned long pixel_clk, unsigned int sample_rate)
> >>>  {
> >>> @@ -620,6 +677,8 @@ static void hdmi_set_clk_regenerator(struct dw_hdmi *hdmi,
> >>>       hdmi->audio_cts = cts;
> >>>       hdmi_set_cts_n(hdmi, cts, hdmi->audio_enable ? n : 0);
> >>>       spin_unlock_irq(&hdmi->audio_lock);
> >>> +
> >>> +     hdmi_set_schnl(hdmi);
> >>>  }
> >>>
> >>>  static void hdmi_init_clk_regenerator(struct dw_hdmi *hdmi)
> >>> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h
> >>> index 6988f12d89d9..619ebc1c8354 100644
> >>> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h
> >>> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h
> >>> @@ -158,6 +158,17 @@
> >>>  #define HDMI_FC_SPDDEVICEINF                    0x1062
> >>>  #define HDMI_FC_AUDSCONF                        0x1063
> >>>  #define HDMI_FC_AUDSSTAT                        0x1064
> >>> +#define HDMI_FC_AUDSV                           0x1065
> >>> +#define HDMI_FC_AUDSU                           0x1066
> >>> +#define HDMI_FC_AUDSCHNLS0                      0x1067
> >>> +#define HDMI_FC_AUDSCHNLS1                      0x1068
> >>> +#define HDMI_FC_AUDSCHNLS2                      0x1069
> >>> +#define HDMI_FC_AUDSCHNLS3                      0x106a
> >>> +#define HDMI_FC_AUDSCHNLS4                      0x106b
> >>> +#define HDMI_FC_AUDSCHNLS5                      0x106c
> >>> +#define HDMI_FC_AUDSCHNLS6                      0x106d
> >>> +#define HDMI_FC_AUDSCHNLS7                      0x106e
> >>> +#define HDMI_FC_AUDSCHNLS8                      0x106f
> >>>  #define HDMI_FC_DATACH0FILL                     0x1070
> >>>  #define HDMI_FC_DATACH1FILL                     0x1071
> >>>  #define HDMI_FC_DATACH2FILL                     0x1072
> >>> @@ -706,6 +717,15 @@ enum {
> >>>  /* HDMI_FC_AUDSCHNLS7 field values */
> >>>       HDMI_FC_AUDSCHNLS7_ACCURACY_OFFSET = 4,
> >>>       HDMI_FC_AUDSCHNLS7_ACCURACY_MASK = 0x30,
> >>> +     HDMI_FC_AUDSCHNLS7_SMPRATE_MASK = 0x0f,
> >>> +     HDMI_FC_AUDSCHNLS7_SMPRATE_192K = 0xe,
> >>> +     HDMI_FC_AUDSCHNLS7_SMPRATE_176K4 = 0xc,
> >>> +     HDMI_FC_AUDSCHNLS7_SMPRATE_96K = 0xa,
> >>> +     HDMI_FC_AUDSCHNLS7_SMPRATE_768K = 0x9,
> >>> +     HDMI_FC_AUDSCHNLS7_SMPRATE_88K2 = 0x8,
> >>> +     HDMI_FC_AUDSCHNLS7_SMPRATE_32K = 0x3,
> >>> +     HDMI_FC_AUDSCHNLS7_SMPRATE_48K = 0x2,
> >>> +     HDMI_FC_AUDSCHNLS7_SMPRATE_44K1 = 0x0,
> >>>
> >>>  /* HDMI_FC_AUDSCHNLS8 field values */
> >>>       HDMI_FC_AUDSCHNLS8_ORIGSAMPFREQ_MASK = 0xf0,
> >>>
> >>
>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* [PATCH -next] usb: gadget: pxa27x_udc: use devm_platform_ioremap_resource() to simplify code
From: YueHaibing @ 2019-09-04  9:45 UTC (permalink / raw)
  To: daniel, haojian.zhuang, robert.jarzmik, balbi, gregkh
  Cc: linux-usb, YueHaibing, linux-kernel, linux-arm-kernel

Use devm_platform_ioremap_resource() to simplify the code a bit.
This is detected by coccinelle.

Reported-by: Hulk Robot <hulkci@huawei.com>
Signed-off-by: YueHaibing <yuehaibing@huawei.com>
---
 drivers/usb/gadget/udc/pxa27x_udc.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/usb/gadget/udc/pxa27x_udc.c b/drivers/usb/gadget/udc/pxa27x_udc.c
index 0142332..5f107f2 100644
--- a/drivers/usb/gadget/udc/pxa27x_udc.c
+++ b/drivers/usb/gadget/udc/pxa27x_udc.c
@@ -2356,7 +2356,6 @@ MODULE_DEVICE_TABLE(of, udc_pxa_dt_ids);
  */
 static int pxa_udc_probe(struct platform_device *pdev)
 {
-	struct resource *regs;
 	struct pxa_udc *udc = &memory;
 	int retval = 0, gpio;
 	struct pxa2xx_udc_mach_info *mach = dev_get_platdata(&pdev->dev);
@@ -2378,8 +2377,7 @@ static int pxa_udc_probe(struct platform_device *pdev)
 		udc->gpiod = devm_gpiod_get(&pdev->dev, NULL, GPIOD_ASIS);
 	}
 
-	regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	udc->regs = devm_ioremap_resource(&pdev->dev, regs);
+	udc->regs = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(udc->regs))
 		return PTR_ERR(udc->regs);
 	udc->irq = platform_get_irq(pdev, 0);
-- 
2.7.4



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* [PATCH] clk: imx: pll14xx: Fix quick switch of S/K parameter
From: Leonard Crestez @ 2019-09-04  9:49 UTC (permalink / raw)
  To: Stephen Boyd, Shawn Guo, Peng Fan
  Cc: Dong Aisheng, Jacky Bai, Michael Turquette, linux-clk, linux-imx,
	Viorel Suman, Fabio Estevam, Daniel Baluta, kernel,
	linux-arm-kernel, Abel Vesa

The PLL14xx on imx8m can change the S and K parameter without requiring
a reset and relock of the whole PLL.

Fix clk_pll144xx_mp_change register reading and use it for pll1443 as
well since no reset+relock is required on K changes either.

Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
---
 drivers/clk/imx/clk-pll14xx.c | 40 +++++++----------------------------
 1 file changed, 8 insertions(+), 32 deletions(-)

The PLLs are currently table-based and none of the entries differ only
in S/K so further work would be required to make use of this. The
prospective user is audio doing tiny freq adjustments and there is no
standard API for that.

Lacking users is not a good reason to carry broken code around.

diff --git a/drivers/clk/imx/clk-pll14xx.c b/drivers/clk/imx/clk-pll14xx.c
index b7213023b238..25342297e5a6 100644
--- a/drivers/clk/imx/clk-pll14xx.c
+++ b/drivers/clk/imx/clk-pll14xx.c
@@ -110,47 +110,21 @@ static unsigned long clk_pll1443x_recalc_rate(struct clk_hw *hw,
 	do_div(fvco, pdiv << sdiv);
 
 	return fvco;
 }
 
-static inline bool clk_pll1416x_mp_change(const struct imx_pll14xx_rate_table *rate,
+static inline bool clk_pll14xx_mp_change(const struct imx_pll14xx_rate_table *rate,
 					  u32 pll_div)
 {
 	u32 old_mdiv, old_pdiv;
 
-	old_mdiv = (pll_div >> MDIV_SHIFT) & MDIV_MASK;
-	old_pdiv = (pll_div >> PDIV_SHIFT) & PDIV_MASK;
+	old_mdiv = (pll_div & MDIV_MASK) >> MDIV_SHIFT;
+	old_pdiv = (pll_div & PDIV_MASK) >> PDIV_SHIFT;
 
 	return rate->mdiv != old_mdiv || rate->pdiv != old_pdiv;
 }
 
-static inline bool clk_pll1443x_mpk_change(const struct imx_pll14xx_rate_table *rate,
-					  u32 pll_div_ctl0, u32 pll_div_ctl1)
-{
-	u32 old_mdiv, old_pdiv, old_kdiv;
-
-	old_mdiv = (pll_div_ctl0 >> MDIV_SHIFT) & MDIV_MASK;
-	old_pdiv = (pll_div_ctl0 >> PDIV_SHIFT) & PDIV_MASK;
-	old_kdiv = (pll_div_ctl1 >> KDIV_SHIFT) & KDIV_MASK;
-
-	return rate->mdiv != old_mdiv || rate->pdiv != old_pdiv ||
-		rate->kdiv != old_kdiv;
-}
-
-static inline bool clk_pll1443x_mp_change(const struct imx_pll14xx_rate_table *rate,
-					  u32 pll_div_ctl0, u32 pll_div_ctl1)
-{
-	u32 old_mdiv, old_pdiv, old_kdiv;
-
-	old_mdiv = (pll_div_ctl0 >> MDIV_SHIFT) & MDIV_MASK;
-	old_pdiv = (pll_div_ctl0 >> PDIV_SHIFT) & PDIV_MASK;
-	old_kdiv = (pll_div_ctl1 >> KDIV_SHIFT) & KDIV_MASK;
-
-	return rate->mdiv != old_mdiv || rate->pdiv != old_pdiv ||
-		rate->kdiv != old_kdiv;
-}
-
 static int clk_pll14xx_wait_lock(struct clk_pll14xx *pll)
 {
 	u32 val;
 
 	return readl_poll_timeout(pll->base, val, val & LOCK_TIMEOUT_US, 0,
@@ -172,11 +146,11 @@ static int clk_pll1416x_set_rate(struct clk_hw *hw, unsigned long drate,
 		return -EINVAL;
 	}
 
 	tmp = readl_relaxed(pll->base + 4);
 
-	if (!clk_pll1416x_mp_change(rate, tmp)) {
+	if (!clk_pll14xx_mp_change(rate, tmp)) {
 		tmp &= ~(SDIV_MASK) << SDIV_SHIFT;
 		tmp |= rate->sdiv << SDIV_SHIFT;
 		writel_relaxed(tmp, pll->base + 4);
 
 		return 0;
@@ -233,17 +207,19 @@ static int clk_pll1443x_set_rate(struct clk_hw *hw, unsigned long drate,
 			drate, clk_hw_get_name(hw));
 		return -EINVAL;
 	}
 
 	tmp = readl_relaxed(pll->base + 4);
-	div_val = readl_relaxed(pll->base + 8);
 
-	if (!clk_pll1443x_mpk_change(rate, tmp, div_val)) {
+	if (!clk_pll14xx_mp_change(rate, tmp)) {
 		tmp &= ~(SDIV_MASK) << SDIV_SHIFT;
 		tmp |= rate->sdiv << SDIV_SHIFT;
 		writel_relaxed(tmp, pll->base + 4);
 
+		tmp = rate->kdiv << KDIV_SHIFT;
+		writel_relaxed(tmp, pll->base + 8);
+
 		return 0;
 	}
 
 	/* Enable RST */
 	tmp = readl_relaxed(pll->base);
-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* Re: [PATCH 0/5] dmaengine: ti: edma: Multicore usage related fixes
From: Vinod Koul @ 2019-09-04  9:49 UTC (permalink / raw)
  To: Peter Ujfalusi
  Cc: devicetree, linux-kernel, robh+dt, dmaengine, dan.j.williams,
	linux-omap, linux-arm-kernel
In-Reply-To: <20190823125618.8133-1-peter.ujfalusi@ti.com>

On 23-08-19, 15:56, Peter Ujfalusi wrote:
> Hi,
> 
> When other cores want to use EDMA for their use cases Linux was not playing
> nicely.
> By design EDMA is supporting shared use with shadow regions. Linux is using
> region0, others can be used by other cores.
> 
> In order to not break multicore shared usage of EDMA:
> - do not reset paRAM slots which is not allocated for Linux (reserved paRAM
>   slots)
> - Only reset region0 access registers, do not touch other regions
> - Add option for reserved channels which should not be used by Linux in a similar
>   fashion as we already have for reserved paRAM slots.

Applied 1 to 3, thanks

-- 
~Vinod

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox