Re: [PATCH 1/2] dt-bindings: media: sun4i-csi: Drop the module clock
On Mon, Oct 07, 2019 at 07:11:03PM +0800, Chen-Yu Tsai wrote:
> On Mon, Oct 7, 2019 at 7:09 PM Maxime Ripard wrote:
> >
> > Hi,
> >
> > On Fri, Oct 04, 2019 at 02:33:41PM +0800, Chen-Yu Tsai wrote:
> > > On Fri, Oct 4, 2019 at 12:37 AM Maxime Ripard wrote:
> > > > On Thu, Oct 03, 2019 at 11:51:05PM +0800, Chen-Yu Tsai wrote:
> > > > > On Thu, Oct 3, 2019 at 11:48 PM Maxime Ripard
> > > > > wrote:
> > > > > >
> > > > > > It turns out that what was thought to be the module clock was
> > > > > > actually the
> > > > > > clock meant to be used by the sensor, and isn't playing any role
> > > > > > with the
> > > > > > CSI controller itself. Let's drop that clock from our binding.
> > > > > >
> > > > > > Fixes: c5e8f4ccd775 ("media: dt-bindings: media: Add Allwinner A10
> > > > > > CSI binding")
> > > > > > Reported-by: Chen-Yu Tsai
> > > > > > Signed-off-by: Maxime Ripard
> > > > > > ---
> > > > > > .../devicetree/bindings/media/allwinner,sun4i-a10-csi.yaml | 7
> > > > > > ++-
> > > > > > 1 file changed, 2 insertions(+), 5 deletions(-)
> > > > > >
> > > > > > diff --git
> > > > > > a/Documentation/devicetree/bindings/media/allwinner,sun4i-a10-csi.yaml
> > > > > >
> > > > > > b/Documentation/devicetree/bindings/media/allwinner,sun4i-a10-csi.yaml
> > > > > > index 5dd1cf490cd9..d3e423fcb6c2 100644
> > > > > > ---
> > > > > > a/Documentation/devicetree/bindings/media/allwinner,sun4i-a10-csi.yaml
> > > > > > +++
> > > > > > b/Documentation/devicetree/bindings/media/allwinner,sun4i-a10-csi.yaml
> > > > > > @@ -27,14 +27,12 @@ properties:
> > > > > >clocks:
> > > > > > items:
> > > > > >- description: The CSI interface clock
> > > > > > - - description: The CSI module clock
> > > > > >- description: The CSI ISP clock
> > > > > >- description: The CSI DRAM clock
> > > > > >
> > > > > >clock-names:
> > > > > > items:
> > > > > >- const: bus
> > > > > > - - const: mod
> > > > > >- const: isp
> > > > > >- const: ram
> > > > > >
> > > > > > @@ -89,9 +87,8 @@ examples:
> > > > > > compatible = "allwinner,sun7i-a20-csi0";
> > > > > > reg = <0x01c09000 0x1000>;
> > > > > > interrupts = ;
> > > > > > -clocks = <&ccu CLK_AHB_CSI0>, <&ccu CLK_CSI0>,
> > > > > > - <&ccu CLK_CSI_SCLK>, <&ccu CLK_DRAM_CSI0>;
> > > > > > -clock-names = "bus", "mod", "isp", "ram";
> > > > > > +clocks = <&ccu CLK_AHB_CSI0>, <&ccu CLK_CSI_SCLK>, <&ccu
> > > > > > CLK_DRAM_CSI0>;
> > > > > > +clock-names = "bus", "isp", "ram";
> > > > >
> > > > > I believe what we thought was the ISP clock is actually the module
> > > > > clock
> > > > > for the whole CSI subsystem. So we should still use the module clock
> > > > > entry,
> > > > > and remove the ISP entry.
> > > >
> > > > I'm really not sure it is. CSI1 on the A20 (possibly the A10 as well,
> > > > I haven't checked), and the one on the A13 don't have any ISP embedded
> > > > in the CSI controller.
> > > >
> > > > It makes some difference, because according to the BSP, you can see
> > > > that the ISP clock is taken for CSI0:
> > > > https://github.com/linux-sunxi/linux-sunxi/blob/sunxi-3.4/drivers/media/video/sun4i_csi/csi0/sun4i_drv_csi.c#L389
> > > >
> > > > While for CSI1, that block is commented out, and the ISP clock never
> > > > used:
> > > > https://github.com/linux-sunxi/linux-sunxi/blob/sunxi-3.4/drivers/media/video/sun4i_csi/csi1/sun4i_drv_csi.c#L396
> > > >
> > > > So I really believe that the ISP clock is here to feed the ISP block
> > > > within the CSI controller if there's any. But it's far from always the
> > > > case, as opposed to a module clock for the whole CSI controller that
> > > > would be here in both cases.
> > >
> > > I guess we should try to test this with CSI1 one, either on a Cubieboard
> > > or OlinuXino with a simple camera like the OV7670?
> > >
> > > Do you have any hardware on hand for this? I have the Cubieboard but I
> > > need to get some proper 2.0mm connector blocks.
> >
> > I've tested it with the A13 before, and it doesn't need the ISP clock
>
> OK! That clears things up!
I've added both patches as fixes then. Thanks!
Maxime
signature.asc
Description: PGP signature
Re: [PATCH 1/2] dt-bindings: media: sun4i-csi: Drop the module clock
Hi,
On Fri, Oct 04, 2019 at 02:33:41PM +0800, Chen-Yu Tsai wrote:
> On Fri, Oct 4, 2019 at 12:37 AM Maxime Ripard wrote:
> > On Thu, Oct 03, 2019 at 11:51:05PM +0800, Chen-Yu Tsai wrote:
> > > On Thu, Oct 3, 2019 at 11:48 PM Maxime Ripard wrote:
> > > >
> > > > It turns out that what was thought to be the module clock was actually
> > > > the
> > > > clock meant to be used by the sensor, and isn't playing any role with
> > > > the
> > > > CSI controller itself. Let's drop that clock from our binding.
> > > >
> > > > Fixes: c5e8f4ccd775 ("media: dt-bindings: media: Add Allwinner A10 CSI
> > > > binding")
> > > > Reported-by: Chen-Yu Tsai
> > > > Signed-off-by: Maxime Ripard
> > > > ---
> > > > .../devicetree/bindings/media/allwinner,sun4i-a10-csi.yaml | 7 ++-
> > > > 1 file changed, 2 insertions(+), 5 deletions(-)
> > > >
> > > > diff --git
> > > > a/Documentation/devicetree/bindings/media/allwinner,sun4i-a10-csi.yaml
> > > > b/Documentation/devicetree/bindings/media/allwinner,sun4i-a10-csi.yaml
> > > > index 5dd1cf490cd9..d3e423fcb6c2 100644
> > > > ---
> > > > a/Documentation/devicetree/bindings/media/allwinner,sun4i-a10-csi.yaml
> > > > +++
> > > > b/Documentation/devicetree/bindings/media/allwinner,sun4i-a10-csi.yaml
> > > > @@ -27,14 +27,12 @@ properties:
> > > >clocks:
> > > > items:
> > > >- description: The CSI interface clock
> > > > - - description: The CSI module clock
> > > >- description: The CSI ISP clock
> > > >- description: The CSI DRAM clock
> > > >
> > > >clock-names:
> > > > items:
> > > >- const: bus
> > > > - - const: mod
> > > >- const: isp
> > > >- const: ram
> > > >
> > > > @@ -89,9 +87,8 @@ examples:
> > > > compatible = "allwinner,sun7i-a20-csi0";
> > > > reg = <0x01c09000 0x1000>;
> > > > interrupts = ;
> > > > -clocks = <&ccu CLK_AHB_CSI0>, <&ccu CLK_CSI0>,
> > > > - <&ccu CLK_CSI_SCLK>, <&ccu CLK_DRAM_CSI0>;
> > > > -clock-names = "bus", "mod", "isp", "ram";
> > > > +clocks = <&ccu CLK_AHB_CSI0>, <&ccu CLK_CSI_SCLK>, <&ccu
> > > > CLK_DRAM_CSI0>;
> > > > +clock-names = "bus", "isp", "ram";
> > >
> > > I believe what we thought was the ISP clock is actually the module clock
> > > for the whole CSI subsystem. So we should still use the module clock
> > > entry,
> > > and remove the ISP entry.
> >
> > I'm really not sure it is. CSI1 on the A20 (possibly the A10 as well,
> > I haven't checked), and the one on the A13 don't have any ISP embedded
> > in the CSI controller.
> >
> > It makes some difference, because according to the BSP, you can see
> > that the ISP clock is taken for CSI0:
> > https://github.com/linux-sunxi/linux-sunxi/blob/sunxi-3.4/drivers/media/video/sun4i_csi/csi0/sun4i_drv_csi.c#L389
> >
> > While for CSI1, that block is commented out, and the ISP clock never
> > used:
> > https://github.com/linux-sunxi/linux-sunxi/blob/sunxi-3.4/drivers/media/video/sun4i_csi/csi1/sun4i_drv_csi.c#L396
> >
> > So I really believe that the ISP clock is here to feed the ISP block
> > within the CSI controller if there's any. But it's far from always the
> > case, as opposed to a module clock for the whole CSI controller that
> > would be here in both cases.
>
> I guess we should try to test this with CSI1 one, either on a Cubieboard
> or OlinuXino with a simple camera like the OV7670?
>
> Do you have any hardware on hand for this? I have the Cubieboard but I
> need to get some proper 2.0mm connector blocks.
I've tested it with the A13 before, and it doesn't need the ISP clock
Maxime
signature.asc
Description: PGP signature
Re: [PATCH 1/2] dt-bindings: media: sun4i-csi: Drop the module clock
Hi,
On Thu, Oct 03, 2019 at 11:51:05PM +0800, Chen-Yu Tsai wrote:
> On Thu, Oct 3, 2019 at 11:48 PM Maxime Ripard wrote:
> >
> > It turns out that what was thought to be the module clock was actually the
> > clock meant to be used by the sensor, and isn't playing any role with the
> > CSI controller itself. Let's drop that clock from our binding.
> >
> > Fixes: c5e8f4ccd775 ("media: dt-bindings: media: Add Allwinner A10 CSI
> > binding")
> > Reported-by: Chen-Yu Tsai
> > Signed-off-by: Maxime Ripard
> > ---
> > .../devicetree/bindings/media/allwinner,sun4i-a10-csi.yaml | 7 ++-
> > 1 file changed, 2 insertions(+), 5 deletions(-)
> >
> > diff --git
> > a/Documentation/devicetree/bindings/media/allwinner,sun4i-a10-csi.yaml
> > b/Documentation/devicetree/bindings/media/allwinner,sun4i-a10-csi.yaml
> > index 5dd1cf490cd9..d3e423fcb6c2 100644
> > --- a/Documentation/devicetree/bindings/media/allwinner,sun4i-a10-csi.yaml
> > +++ b/Documentation/devicetree/bindings/media/allwinner,sun4i-a10-csi.yaml
> > @@ -27,14 +27,12 @@ properties:
> >clocks:
> > items:
> >- description: The CSI interface clock
> > - - description: The CSI module clock
> >- description: The CSI ISP clock
> >- description: The CSI DRAM clock
> >
> >clock-names:
> > items:
> >- const: bus
> > - - const: mod
> >- const: isp
> >- const: ram
> >
> > @@ -89,9 +87,8 @@ examples:
> > compatible = "allwinner,sun7i-a20-csi0";
> > reg = <0x01c09000 0x1000>;
> > interrupts = ;
> > -clocks = <&ccu CLK_AHB_CSI0>, <&ccu CLK_CSI0>,
> > - <&ccu CLK_CSI_SCLK>, <&ccu CLK_DRAM_CSI0>;
> > -clock-names = "bus", "mod", "isp", "ram";
> > +clocks = <&ccu CLK_AHB_CSI0>, <&ccu CLK_CSI_SCLK>, <&ccu
> > CLK_DRAM_CSI0>;
> > +clock-names = "bus", "isp", "ram";
>
> I believe what we thought was the ISP clock is actually the module clock
> for the whole CSI subsystem. So we should still use the module clock entry,
> and remove the ISP entry.
I'm really not sure it is. CSI1 on the A20 (possibly the A10 as well,
I haven't checked), and the one on the A13 don't have any ISP embedded
in the CSI controller.
It makes some difference, because according to the BSP, you can see
that the ISP clock is taken for CSI0:
https://github.com/linux-sunxi/linux-sunxi/blob/sunxi-3.4/drivers/media/video/sun4i_csi/csi0/sun4i_drv_csi.c#L389
While for CSI1, that block is commented out, and the ISP clock never
used:
https://github.com/linux-sunxi/linux-sunxi/blob/sunxi-3.4/drivers/media/video/sun4i_csi/csi1/sun4i_drv_csi.c#L396
So I really believe that the ISP clock is here to feed the ISP block
within the CSI controller if there's any. But it's far from always the
case, as opposed to a module clock for the whole CSI controller that
would be here in both cases.
Maxime
signature.asc
Description: PGP signature
[PATCH 1/2] dt-bindings: media: sun4i-csi: Drop the module clock
It turns out that what was thought to be the module clock was actually the
clock meant to be used by the sensor, and isn't playing any role with the
CSI controller itself. Let's drop that clock from our binding.
Fixes: c5e8f4ccd775 ("media: dt-bindings: media: Add Allwinner A10 CSI binding")
Reported-by: Chen-Yu Tsai
Signed-off-by: Maxime Ripard
---
.../devicetree/bindings/media/allwinner,sun4i-a10-csi.yaml | 7 ++-
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git
a/Documentation/devicetree/bindings/media/allwinner,sun4i-a10-csi.yaml
b/Documentation/devicetree/bindings/media/allwinner,sun4i-a10-csi.yaml
index 5dd1cf490cd9..d3e423fcb6c2 100644
--- a/Documentation/devicetree/bindings/media/allwinner,sun4i-a10-csi.yaml
+++ b/Documentation/devicetree/bindings/media/allwinner,sun4i-a10-csi.yaml
@@ -27,14 +27,12 @@ properties:
clocks:
items:
- description: The CSI interface clock
- - description: The CSI module clock
- description: The CSI ISP clock
- description: The CSI DRAM clock
clock-names:
items:
- const: bus
- - const: mod
- const: isp
- const: ram
@@ -89,9 +87,8 @@ examples:
compatible = "allwinner,sun7i-a20-csi0";
reg = <0x01c09000 0x1000>;
interrupts = ;
-clocks = <&ccu CLK_AHB_CSI0>, <&ccu CLK_CSI0>,
- <&ccu CLK_CSI_SCLK>, <&ccu CLK_DRAM_CSI0>;
-clock-names = "bus", "mod", "isp", "ram";
+clocks = <&ccu CLK_AHB_CSI0>, <&ccu CLK_CSI_SCLK>, <&ccu
CLK_DRAM_CSI0>;
+clock-names = "bus", "isp", "ram";
resets = <&ccu RST_CSI0>;
port {
--
2.23.0
[PATCH 2/2] ARM: dts: sun7i: Drop the module clock from the device tree
What we thought would be the module clock is actually the clock meant to be
used by the sensors, and play no role in the CSI controller. Now that the
binding has been updated to reflect that, let's update the device tree too.
Fixes: d2b9c6444301 ("ARM: dts: sun7i: Add CSI0 controller")
Reported-by: Chen-Yu Tsai
Signed-off-by: Maxime Ripard
---
arch/arm/boot/dts/sun7i-a20.dtsi | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/arch/arm/boot/dts/sun7i-a20.dtsi b/arch/arm/boot/dts/sun7i-a20.dtsi
index 874231be04e4..8aebefd6accf 100644
--- a/arch/arm/boot/dts/sun7i-a20.dtsi
+++ b/arch/arm/boot/dts/sun7i-a20.dtsi
@@ -380,9 +380,8 @@
compatible = "allwinner,sun7i-a20-csi0";
reg = <0x01c09000 0x1000>;
interrupts = ;
- clocks = <&ccu CLK_AHB_CSI0>, <&ccu CLK_CSI0>,
-<&ccu CLK_CSI_SCLK>, <&ccu CLK_DRAM_CSI0>;
- clock-names = "bus", "mod", "isp", "ram";
+ clocks = <&ccu CLK_AHB_CSI0>, <&ccu CLK_CSI_SCLK>,
<&ccu CLK_DRAM_CSI0>;
+ clock-names = "bus", "isp", "ram";
resets = <&ccu RST_CSI0>;
status = "disabled";
};
--
2.23.0
[PATCH] dt-bindings: media: rc: Fix redundant string
The linux,rc-map-name property is described using an enum, yet a value has
been put in that enum twice, resulting in a warning. Let's fix that.
Fixes: 7c31b9d67342 ("media: dt-bindings: media: Add YAML schemas for the
generic RC bindings")
Signed-off-by: Maxime Ripard
---
Documentation/devicetree/bindings/media/rc.yaml | 1 -
1 file changed, 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/media/rc.yaml
b/Documentation/devicetree/bindings/media/rc.yaml
index 3d5c154fd230..9054555e6608 100644
--- a/Documentation/devicetree/bindings/media/rc.yaml
+++ b/Documentation/devicetree/bindings/media/rc.yaml
@@ -73,7 +73,6 @@ properties:
- rc-genius-tvgo-a11mce
- rc-gotview7135
- rc-hauppauge
- - rc-hauppauge
- rc-hisi-poplar
- rc-hisi-tv-demo
- rc-imon-mce
--
2.23.0
Re: [ANN] Media sessions in Lyon in October: codecs
Hi, On Mon, Sep 23, 2019 at 07:19:34PM +0300, Laurent Pinchart wrote: > On Mon, Sep 23, 2019 at 05:02:13PM +0200, Jacopo Mondi wrote: > > On Mon, Sep 23, 2019 at 04:12:55PM +0200, Hans Verkuil wrote: > > > Hi all, > > > > > > Since we have three separate half-day sessions for different topics I > > > decided > > > to split the announcement for this in three emails as well, so these > > > things > > > can be discussed in separate threads. > > > > > > All sessions are in room Terreaux VIP Lounge - Level 0. > > > There is a maximum of 15 people. > > > > > > The first session deals with the codec API and is on Tuesday morning from > > > 8:30 (tentative, might change) to 12:00 (we have to vacate the room at > > > that > > > time). > > > > > > Confirmed attendees for this session: > > > > > > Boris Brezillon > > > Alexandre Courbot > > > Nicolas Dufresne > > > Tomasz Figa > > > Ezequiel Garcia > > > Daniel Gomez > > > Dafna Hirschfeld > > > Eugen Hristev > > > Paul Kocialkowski > > > Helen Koike > > > Michael Tretter > > > Hans Verkuil > > > > > > Tentative: > > > > > > Laurent Pinchart > > > Jacopo Mondi > > > > > > Jacopo, please confirm if you want to attend this session. I didn't find > > > an email with explicit confirmation, so it was probably done via irc. But > > > since > > > this session is getting close to capacity I would prefer to keep > > > attendance to > > > those are actually working with codecs (or will work with it in the near > > > future). > > > > I'm not really working on codecs, so if you're running almost at full > > capacity please feel free to re-assign my seat. > > Same here. I think that Maxime Ripard, who isn't in the above list, > would be able to contribute more than I could. I wasn't sure I would be able to make it, but I can, so if it's still an option count me in :) Maxime signature.asc Description: PGP signature
Re: [GIT PULL for 5.4] A10 CSI driver and atmel-isi fix
Hi Hans, On Fri, Aug 23, 2019 at 01:55:07PM +0200, Hans Verkuil wrote: > Hi Maxime, > > On 8/23/19 9:31 AM, Sakari Ailus wrote: > > Hi Mauro, > > > > Here's a driver for A10 CSI parallel receiver and a fix for atmel-isi. > > > > There are three checkpatch.pl warnings; two on Makefile / Kconfig on > > MAINTAINERS (i.e. not worth mentioning in MAINTAINERS) while the third is > > on a spinlock missing a comment. If you'd like the last one to be > > addressed, I'm proposing doing that in a separate patch. > > When running sparse I get this warning: > > drivers/media/platform/sunxi/sun4i-csi/sun4i_v4l2.c:21:31: warning: symbol > 'sun4i_csi_formats' was not declared. Should it be static? > > Can you post a follow-up patch for this? Sorry for that, I just sent a patch fixing it. Maxime -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com signature.asc Description: PGP signature
[PATCH] media: sun4i: Make sun4i_csi_formats static
From: Maxime Ripard
The sun4i_csi_formats array is only used in sun4i_v4l2.c, so it doesn't
make any sense to have it !static.
Reported-by: Hans Verkuil
Signed-off-by: Maxime Ripard
---
drivers/media/platform/sunxi/sun4i-csi/sun4i_v4l2.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/media/platform/sunxi/sun4i-csi/sun4i_v4l2.c
b/drivers/media/platform/sunxi/sun4i-csi/sun4i_v4l2.c
index 772b0fc5920f..967bdb99221c 100644
--- a/drivers/media/platform/sunxi/sun4i-csi/sun4i_v4l2.c
+++ b/drivers/media/platform/sunxi/sun4i-csi/sun4i_v4l2.c
@@ -18,7 +18,7 @@
#define CSI_DEFAULT_WIDTH 640
#define CSI_DEFAULT_HEIGHT 480
-const struct sun4i_csi_format sun4i_csi_formats[] = {
+const static struct sun4i_csi_format sun4i_csi_formats[] = {
/* YUV422 inputs */
{
.mbus = MEDIA_BUS_FMT_YUYV8_2X8,
--
2.21.0
Re: [PATCH v8 0/5] media: Allwinner A10 CSI support
On Thu, Aug 22, 2019 at 10:21:11AM +0200, Maxime Ripard wrote: > From: Maxime Ripard > > Hi, > > Here is a series introducing the support for the A10 (and SoCs of the same > generation) CMOS Sensor Interface (called CSI, not to be confused with > MIPI-CSI, which isn't support by that IP). > > That interface is pretty straightforward, but the driver has a few issues > that I wanted to bring up: > > * The only board I've been testing this with has an ov5640 sensor > attached, which doesn't work with the upstream driver. Copying the > Allwinner init sequence works though, and this is how it has been > tested. Testing with a second sensor would allow to see if it's an > issue on the CSI side or the sensor side. > * We don't have support for the ISP at the moment, but this can be added > eventually. Applied patch 4. Maxime -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com signature.asc Description: PGP signature
Re: [PATCH v2 1/2] dt-bindings: media: Add YAML schemas for the generic RC bindings
Hi Sean, On Tue, Aug 20, 2019 at 09:15:26AM +0100, Sean Young wrote: > On Mon, Aug 19, 2019 at 08:26:18PM +0200, Maxime Ripard wrote: > > From: Maxime Ripard > > > > The RC controllers have a bunch of generic properties that are needed in a > > device tree. Add a YAML schemas for those. > > > > Reviewed-by: Rob Herring > > Signed-off-by: Maxime Ripard > > For the series (both 1/2 and 2.2): > > Reviewed-by: Sean Young > > How's tree should this go through? Either yours or Rob's, I guess? Maxime -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com signature.asc Description: PGP signature
[PATCH] media: dt-bindings: Fix vendor-prefixes YAML
Commit 8df39e16877f ("media: dt-bindings: media: Add vendor prefix for
allegro") introduced a new devicetree binding vendors, however with an
improper syntax making the resulting YAML impossible to parse. Let's fix
this.
Cc: Hans Verkuil
Cc: Michael Tretter
Fixes: 8df39e16877f ("media: dt-bindings: media: Add vendor prefix for allegro")
Signed-off-by: Maxime Ripard
---
Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml
b/Documentation/devicetree/bindings/vendor-prefixes.yaml
index cb983eee4576..432bababc9bf 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
+++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
@@ -49,7 +49,7 @@ patternProperties:
description: Aeroflex Gaisler AB
"^al,.*":
description: Annapurna Labs
- "^allegro,.*"
+ "^allegro,.*":
description: Allegro DVT
"^allo,.*":
description: Allo.com
--
2.21.0
Re: [PATCH 0/2] media: Introduce pixel format helpers
Hi Ezequiel, On Thu, Mar 28, 2019 at 03:07:02PM -0300, Ezequiel Garcia wrote: > Following the "Add MPEG-2 decoding to Rockchip VPU"[1], and given we want to > start > using the pixel format helpers rather soon, I've decided to submit them > separatedly. > > Aside from documenting the block_w and block_h, as suggested by Emil Velikov, > these > two patches are unmodified compared to v2 [1]. > > This series contains the minimum patches to introduce the pixel format > helpers, it currently will have no users, but we expect users to start > popping-up soon. > very shortly [2]. > > To avoid confusing the patches here with those on v2 [1], I'm marking them as > v3. You might have missed it, but have you looked at the RFC "drm: Split out the formats API and move it to a common place" I sent a couple of weeks ago? It seems to cover the same use cases. Maxime -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com signature.asc Description: PGP signature
[PATCH v5 0/2] media: cedrus: Add H264 decoding support
Hi, Here is a new version of the H264 decoding support in the cedrus driver. As you might already know, the cedrus driver relies on the Request API, and is a reverse engineered driver for the video decoding engine found on the Allwinner SoCs. This work has been possible thanks to the work done by the people behind libvdpau-sunxi found here: https://github.com/linux-sunxi/libvdpau-sunxi/ I've tested the various ABI using this gdb script: http://code.bulix.org/jl4se4-505620?raw And this test script: http://code.bulix.org/8zle4s-505623?raw The application compiled is quite trivial: http://code.bulix.org/e34zp8-505624?raw The output is: arm:builds/arm-test-v4l2-h264-structures SHA1: fd15b30328765c2caac877e8ea8452c829b2b1d8 x86:builds/x86-test-v4l2-h264-structures SHA1: fd15b30328765c2caac877e8ea8452c829b2b1d8 x64:builds/x64-test-v4l2-h264-structures SHA1: fd15b30328765c2caac877e8ea8452c829b2b1d8 arm64: builds/arm64-test-v4l2-h264-structures SHA1: fd15b30328765c2caac877e8ea8452c829b2b1d8 Let me know if there's any flaw using that test setup, or if you have any comments on the patches. Maxime Changes from v4: - Changed the luma and chroma weight and offset from s8 to s16 - Adjusted chroma and luma denominators masks in the driver - Casted the luma and chroma offset to prevent an overflow - ALways write the interrupt status register - Fix a bug in the sram write routine that would write something even if the length was 0 - Make the scaling lists mandatory - Made the reference list order explicit in the documentation - Made the fact that the slice structure can be an array - Renamed the slice format to V4L2_PIX_FMT_H264_SLICE_RAW - Rebased on Hans' tag br-v5.1s Changes from v3: - Reintroduced long term reference flag and documented it - Reintroduced ref_pic_list_p0/b0/b1 and documented it - Documented the DPB flags - Treat the scaling matrix as optional in the driver, as documented - Free the neighbor buffer - Increase the control IDs by a large margin to be safe of collisions - Reorder the fields documentation according to the structure layout - Change the tag documentation by the timestamp - Convert the sram array to size_t - Simplify the buffer retrieval from timestamp - Rebase Changes from v2: - Simplified _cedrus_write_ref_list as suggested by Jernej - Set whether the frame is used as reference using nal_ref_idc - Respect chroma_format_idc - Fixes for the scaling list and prediction tables - Wrote the documentation for the flags - Added a bunch of defines to the driver bit fields - Reworded the controls and data format descriptions as suggested by Hans - Reworked the controls' structure field size to avoid padding - Removed the long term reference flag - Reintroduced the neighbor info buffer - Removed the ref_pic_list_p0/b0/b1 arrays that are redundant with the one in the DPB - used the timestamps instead of tags - Rebased on 5.0-rc1 Changes from v1: - Rebased on 4.20 - Did the documentation for the userspace API - Used the tags instead of buffer IDs - Added a comment to explain why we still needed the swdec trigger - Reworked the MV col buffer in order to have one slot per frame - Removed the unused neighbor info buffer - Made sure to have the same structure offset and alignments across 32 bits and 64 bits architecture Maxime Ripard (1): media: cedrus: Add H264 decoding support Pawel Osciak (1): media: uapi: Add H264 low-level decoder API compound controls. Documentation/media/uapi/v4l/biblio.rst| 9 +- Documentation/media/uapi/v4l/ext-ctrls-codec.rst | 549 +- Documentation/media/uapi/v4l/pixfmt-compressed.rst | 19 +- Documentation/media/uapi/v4l/vidioc-queryctrl.rst | 30 +- Documentation/media/videodev2.h.rst.exceptions | 5 +- drivers/media/v4l2-core/v4l2-ctrls.c | 42 +- drivers/media/v4l2-core/v4l2-ioctl.c | 1 +- drivers/staging/media/sunxi/cedrus/Makefile| 3 +- drivers/staging/media/sunxi/cedrus/cedrus.c| 31 +- drivers/staging/media/sunxi/cedrus/cedrus.h| 38 +- drivers/staging/media/sunxi/cedrus/cedrus_dec.c| 13 +- drivers/staging/media/sunxi/cedrus/cedrus_h264.c | 577 ++- drivers/staging/media/sunxi/cedrus/cedrus_hw.c | 4 +- drivers/staging/media/sunxi/cedrus/cedrus_regs.h | 91 ++- drivers/staging/media/sunxi/cedrus/cedrus_video.c | 9 +- include/media/h264-ctrls.h | 190 +- include/media/v4l2-ctrls.h | 13 +- include/uapi/linux/videodev2.h | 1 +- 18 files changed, 1622 insertions(+), 3 deletions(-) create mode 100644 drivers/staging/media/sunxi/cedrus/cedrus_h264.c create mode 100644 include/media/h264-ctrls.h base-commit: ae95367b7a685b0b8659494dabb4b494e87b467d -- git-series 0.9.1
[PATCH v5 1/2] media: uapi: Add H264 low-level decoder API compound controls.
From: Pawel Osciak Stateless video codecs will require both the H264 metadata and slices in order to be able to decode frames. This introduces the definitions for a new pixel format for H264 slices that have been parsed, as well as the structures used to pass the metadata from the userspace to the kernel. Co-Developped-by: Maxime Ripard Signed-off-by: Pawel Osciak Signed-off-by: Guenter Roeck Signed-off-by: Maxime Ripard --- Documentation/media/uapi/v4l/biblio.rst| 9 +- Documentation/media/uapi/v4l/ext-ctrls-codec.rst | 549 ++- Documentation/media/uapi/v4l/pixfmt-compressed.rst | 19 +- Documentation/media/uapi/v4l/vidioc-queryctrl.rst | 30 +- Documentation/media/videodev2.h.rst.exceptions | 5 +- drivers/media/v4l2-core/v4l2-ctrls.c | 42 +- drivers/media/v4l2-core/v4l2-ioctl.c | 1 +- include/media/h264-ctrls.h | 190 +- include/media/v4l2-ctrls.h | 13 +- include/uapi/linux/videodev2.h | 1 +- 10 files changed, 858 insertions(+), 1 deletion(-) create mode 100644 include/media/h264-ctrls.h diff --git a/Documentation/media/uapi/v4l/biblio.rst b/Documentation/media/uapi/v4l/biblio.rst index ec33768c055e..3fc3f7ff338a 100644 --- a/Documentation/media/uapi/v4l/biblio.rst +++ b/Documentation/media/uapi/v4l/biblio.rst @@ -122,6 +122,15 @@ ITU BT.1119 :author:International Telecommunication Union (http://www.itu.ch) +.. _h264: + +ITU H.264 += + +:title: ITU-T Recommendation H.264 "Advanced Video Coding for Generic Audiovisual Services" + +:author:International Telecommunication Union (http://www.itu.ch) + .. _jfif: JFIF diff --git a/Documentation/media/uapi/v4l/ext-ctrls-codec.rst b/Documentation/media/uapi/v4l/ext-ctrls-codec.rst index 54b3797b67dd..685942f0300e 100644 --- a/Documentation/media/uapi/v4l/ext-ctrls-codec.rst +++ b/Documentation/media/uapi/v4l/ext-ctrls-codec.rst @@ -1343,6 +1343,555 @@ enum v4l2_mpeg_video_h264_hierarchical_coding_type - - Layer number +.. _v4l2-mpeg-h264: + +``V4L2_CID_MPEG_VIDEO_H264_SPS (struct)`` +Specifies the sequence parameter set (as extracted from the +bitstream) for the associated H264 slice data. This includes the +necessary parameters for configuring a stateless hardware decoding +pipeline for H264. The bitstream parameters are defined according +to :ref:`h264`. Unless there's a specific comment, refer to the +specification for the documentation of these fields, section 7.4.2.1.1 +"Sequence Parameter Set Data Semantics". + +.. note:: + + This compound control is not yet part of the public kernel API and + it is expected to change. + +.. c:type:: v4l2_ctrl_h264_sps + +.. cssclass:: longtable + +.. flat-table:: struct v4l2_ctrl_h264_sps +:header-rows: 0 +:stub-columns: 0 +:widths: 1 1 2 + +* - __u8 + - ``profile_idc`` + - +* - __u8 + - ``constraint_set_flags`` + - See :ref:`Sequence Parameter Set Constraints Set Flags ` +* - __u8 + - ``level_idc`` + - +* - __u8 + - ``seq_parameter_set_id`` + - +* - __u8 + - ``chroma_format_idc`` + - +* - __u8 + - ``bit_depth_luma_minus8`` + - +* - __u8 + - ``bit_depth_chroma_minus8`` + - +* - __u8 + - ``log2_max_frame_num_minus4`` + - +* - __u8 + - ``pic_order_cnt_type`` + - +* - __u8 + - ``log2_max_pic_order_cnt_lsb_minus4`` + - +* - __u8 + - ``max_num_ref_frames`` + - +* - __u8 + - ``num_ref_frames_in_pic_order_cnt_cycle`` + - +* - __s32 + - ``offset_for_ref_frame[255]`` + - +* - __s32 + - ``offset_for_non_ref_pic`` + - +* - __s32 + - ``offset_for_top_to_bottom_field`` + - +* - __u16 + - ``pic_width_in_mbs_minus1`` + - +* - __u16 + - ``pic_height_in_map_units_minus1`` + - +* - __u32 + - ``flags`` + - See :ref:`Sequence Parameter Set Flags ` + +.. _h264_sps_constraints_set_flags: + +``Sequence Parameter Set Constraints Set Flags`` + +.. cssclass:: longtable + +.. flat-table:: +:header-rows: 0 +:stub-columns: 0 +:widths: 1 1 2 + +* - ``V4L2_H264_SPS_CONSTRAINT_SET0_FLAG`` + - 0x0001 + - +* - ``V4L2_H264_SPS_CONSTRAINT_SET1_FLAG`` + - 0x0002 + - +* - ``V4L2_H264_SPS_CONSTRAINT_SET2_FLAG`` + - 0x0004 + - +* - ``V4L2_H264_SPS_CONSTRAINT_SET3_FLAG`` + - 0x0008 + - +* - ``V4L2_H264_SPS_CONSTRAINT_SET4_FLAG`` + - 0x0010 + - +* - ``V4L2_H264_SPS_CONSTRAINT_SET5_FLAG`` + - 0x0020 + - + +.. _h264_sps_flags: + +``Sequence Parameter Set Flags`` + +.. cssclass:: longtable + +.. flat-table:: +:header-rows: 0 +:stub-columns: 0 +:widths: 1 1 2 + +* - ``V4L2_H264_SPS_F
[PATCH v5 2/2] media: cedrus: Add H264 decoding support
Introduce some basic H264 decoding support in cedrus. So far, only the
baseline profile videos have been tested, and some more advanced features
used in higher profiles are not even implemented.
Reviewed-by: Jernej Skrabec
Signed-off-by: Maxime Ripard
---
drivers/staging/media/sunxi/cedrus/Makefile | 3 +-
drivers/staging/media/sunxi/cedrus/cedrus.c | 31 +-
drivers/staging/media/sunxi/cedrus/cedrus.h | 38 +-
drivers/staging/media/sunxi/cedrus/cedrus_dec.c | 13 +-
drivers/staging/media/sunxi/cedrus/cedrus_h264.c | 577 +++-
drivers/staging/media/sunxi/cedrus/cedrus_hw.c| 4 +-
drivers/staging/media/sunxi/cedrus/cedrus_regs.h | 91 ++-
drivers/staging/media/sunxi/cedrus/cedrus_video.c | 9 +-
8 files changed, 764 insertions(+), 2 deletions(-)
create mode 100644 drivers/staging/media/sunxi/cedrus/cedrus_h264.c
diff --git a/drivers/staging/media/sunxi/cedrus/Makefile
b/drivers/staging/media/sunxi/cedrus/Makefile
index e9dc68b7bcb6..aaf141fc58b6 100644
--- a/drivers/staging/media/sunxi/cedrus/Makefile
+++ b/drivers/staging/media/sunxi/cedrus/Makefile
@@ -1,3 +1,4 @@
obj-$(CONFIG_VIDEO_SUNXI_CEDRUS) += sunxi-cedrus.o
-sunxi-cedrus-y = cedrus.o cedrus_video.o cedrus_hw.o cedrus_dec.o
cedrus_mpeg2.o
+sunxi-cedrus-y = cedrus.o cedrus_video.o cedrus_hw.o cedrus_dec.o \
+cedrus_mpeg2.o cedrus_h264.o
diff --git a/drivers/staging/media/sunxi/cedrus/cedrus.c
b/drivers/staging/media/sunxi/cedrus/cedrus.c
index ff11cbeba205..b275607b8111 100644
--- a/drivers/staging/media/sunxi/cedrus/cedrus.c
+++ b/drivers/staging/media/sunxi/cedrus/cedrus.c
@@ -40,6 +40,36 @@ static const struct cedrus_control cedrus_controls[] = {
.codec = CEDRUS_CODEC_MPEG2,
.required = false,
},
+ {
+ .id = V4L2_CID_MPEG_VIDEO_H264_DECODE_PARAMS,
+ .elem_size = sizeof(struct v4l2_ctrl_h264_decode_param),
+ .codec = CEDRUS_CODEC_H264,
+ .required = true,
+ },
+ {
+ .id = V4L2_CID_MPEG_VIDEO_H264_SLICE_PARAMS,
+ .elem_size = sizeof(struct v4l2_ctrl_h264_slice_param),
+ .codec = CEDRUS_CODEC_H264,
+ .required = true,
+ },
+ {
+ .id = V4L2_CID_MPEG_VIDEO_H264_SPS,
+ .elem_size = sizeof(struct v4l2_ctrl_h264_sps),
+ .codec = CEDRUS_CODEC_H264,
+ .required = true,
+ },
+ {
+ .id = V4L2_CID_MPEG_VIDEO_H264_PPS,
+ .elem_size = sizeof(struct v4l2_ctrl_h264_pps),
+ .codec = CEDRUS_CODEC_H264,
+ .required = true,
+ },
+ {
+ .id = V4L2_CID_MPEG_VIDEO_H264_SCALING_MATRIX,
+ .elem_size = sizeof(struct v4l2_ctrl_h264_scaling_matrix),
+ .codec = CEDRUS_CODEC_H264,
+ .required = true,
+ },
};
#define CEDRUS_CONTROLS_COUNT ARRAY_SIZE(cedrus_controls)
@@ -278,6 +308,7 @@ static int cedrus_probe(struct platform_device *pdev)
}
dev->dec_ops[CEDRUS_CODEC_MPEG2] = &cedrus_dec_ops_mpeg2;
+ dev->dec_ops[CEDRUS_CODEC_H264] = &cedrus_dec_ops_h264;
mutex_init(&dev->dev_mutex);
diff --git a/drivers/staging/media/sunxi/cedrus/cedrus.h
b/drivers/staging/media/sunxi/cedrus/cedrus.h
index 4aedd24a9848..8c64f9a27e9d 100644
--- a/drivers/staging/media/sunxi/cedrus/cedrus.h
+++ b/drivers/staging/media/sunxi/cedrus/cedrus.h
@@ -30,7 +30,7 @@
enum cedrus_codec {
CEDRUS_CODEC_MPEG2,
-
+ CEDRUS_CODEC_H264,
CEDRUS_CODEC_LAST,
};
@@ -40,6 +40,12 @@ enum cedrus_irq_status {
CEDRUS_IRQ_OK,
};
+enum cedrus_h264_pic_type {
+ CEDRUS_H264_PIC_TYPE_FRAME = 0,
+ CEDRUS_H264_PIC_TYPE_FIELD,
+ CEDRUS_H264_PIC_TYPE_MBAFF,
+};
+
struct cedrus_control {
u32 id;
u32 elem_size;
@@ -47,6 +53,14 @@ struct cedrus_control {
unsigned char required:1;
};
+struct cedrus_h264_run {
+ const struct v4l2_ctrl_h264_decode_param*decode_param;
+ const struct v4l2_ctrl_h264_pps *pps;
+ const struct v4l2_ctrl_h264_scaling_matrix *scaling_matrix;
+ const struct v4l2_ctrl_h264_slice_param *slice_param;
+ const struct v4l2_ctrl_h264_sps *sps;
+};
+
struct cedrus_mpeg2_run {
const struct v4l2_ctrl_mpeg2_slice_params *slice_params;
const struct v4l2_ctrl_mpeg2_quantization *quantization;
@@ -57,12 +71,20 @@ struct cedrus_run {
struct vb2_v4l2_buffer *dst;
union {
+ struct cedrus_h264_run h264;
struct cedrus_mpeg2_run mpeg2;
};
};
Re: [PATCH v4 2/2] media: cedrus: Add H264 decoding support
Hi,
On Tue, Mar 05, 2019 at 06:05:08PM +0100, Jernej Škrabec wrote:
> Dne torek, 05. marec 2019 ob 11:17:32 CET je Maxime Ripard napisal(a):
> > Hi Jernej,
> >
> > On Wed, Feb 20, 2019 at 06:50:54PM +0100, Jernej Škrabec wrote:
> > > I really wanted to do another review on previous series but got distracted
> > > by analyzing one particulary troublesome H264 sample. It still doesn't
> > > work correctly, so I would ask you if you can test it with your stack (it
> > > might be userspace issue):
> > >
> > > http://jernej.libreelec.tv/videos/problematic/test.mkv
> > >
> > > Please take a look at my comments below.
> >
> > I'd really prefer to focus on getting this merged at this point, and
> > then fixing odd videos and / or setups we can find later
> > on. Especially when new stacks are going to be developped on top of
> > this, I'm sure we're going to have plenty of bugs to address :)
>
> I forgot to mention, you can add:
> Reviewed-by: Jernej Skrabec
>
> once you fix issues.
Great, thanks :)
> > > > + for (i = 0; i < ARRAY_SIZE(pred_weight->weight_factors); i++) {
> > > > + const struct v4l2_h264_weight_factors *factors =
> > > > + &pred_weight->weight_factors[i];
> > > > +
> > > > + for (j = 0; j < ARRAY_SIZE(factors->luma_weight); j++)
> {
> > > > + u32 val;
> > > > +
> > > > + val = ((factors->luma_offset[j] & 0x1ff) <<
> 16)
> > > >
> > > > + (factors->luma_weight[j] & 0x1ff);
> > > > + cedrus_write(dev, VE_AVC_SRAM_PORT_DATA,
> > >
> > > val);
> > >
> > > You should cast offset varible to wider type. Currently some videos which
> > > use prediction weight table don't work for me, unless offset is casted to
> > > u32 first. Shifting 8 bit variable for 16 places gives you 0 every time.
> >
> > I'll do it.
> >
> > > Luma offset and weight are defined as s8, so having wider mask doesn't
> > > really make sense. However, I think weight should be s16 anyway, because
> > > standard says that it's value could be 2^denominator for default value or
> > > in range -128..127. Worst case would be 2^7 = 128 and -128. To cover both
> > > values you need at least 9 bits.
> >
> > But if I understood the spec right, in that case you would just have
> > the denominator set, and not the offset, while the offset is used if
> > you don't use the default formula (and therefore remains in the -128
> > 127 range which is covered by the s8), right?
>
> Yeah, default offset is 0 and s8 is sufficient for that. I'm talking about
> weight. Default weight is "1 << denominator", which might be 1 << 7 or 128.
>
> We could also add a flag, which would signal default table. In that case we
> could just set a bit to tell VPU to use default values. Even if some VPUs
> need
> default table to be set explicitly, it's very easy to calculate values as
> mentioned in previous paragraph.
Yeah, sorry, I meant weight. Would that make any difference? Can we
have situations where both the denominator and the weight would be
set, with the weight set to 128?
I've checked in the libva and ffmpeg, and libva uses a short, while
ffmpeg uses an int, both for the weight and offset. For consistency I
guess we could change both to shorts just like libva?
What do you think?
Maxime
--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
signature.asc
Description: PGP signature
Re: [PATCH v4 1/2] media: uapi: Add H264 low-level decoder API compound controls.
On Fri, Feb 22, 2019 at 04:46:17PM +0900, Tomasz Figa wrote: > Hi Maxime, > > On Wed, Feb 20, 2019 at 11:17 PM Maxime Ripard > wrote: > > > > From: Pawel Osciak > > > > Stateless video codecs will require both the H264 metadata and slices in > > order to be able to decode frames. > > > > This introduces the definitions for a new pixel format for H264 slices that > > have been parsed, as well as the structures used to pass the metadata from > > the userspace to the kernel. > > > > Co-Developped-by: Maxime Ripard > > Signed-off-by: Pawel Osciak > > Signed-off-by: Guenter Roeck > > Signed-off-by: Maxime Ripard > > Thanks for the patch. Some comments inline. > > [snip] > > +``V4L2_CID_MPEG_VIDEO_H264_SLICE_PARAMS (struct)`` > > +Specifies the slice parameters (as extracted from the bitstream) > > +for the associated H264 slice data. This includes the necessary > > +parameters for configuring a stateless hardware decoding pipeline > > +for H264. The bitstream parameters are defined according to > > +:ref:`h264`. Unless there's a specific comment, refer to the > > +specification for the documentation of these fields, section 7.4.3 > > +"Slice Header Semantics". > > Note that this is expected to be an array, with entries for all the > slices included in the bitstream buffer. > > > + > > +.. note:: > > + > > + This compound control is not yet part of the public kernel API and > > + it is expected to change. > > + > > +.. c:type:: v4l2_ctrl_h264_slice_param > > + > > +.. cssclass:: longtable > > + > > +.. flat-table:: struct v4l2_ctrl_h264_slice_param > > +:header-rows: 0 > > +:stub-columns: 0 > > +:widths: 1 1 2 > > + > > +* - __u32 > > + - ``size`` > > + - > > +* - __u32 > > + - ``header_bit_size`` > > + - > > +* - __u16 > > + - ``first_mb_in_slice`` > > + - > > +* - __u8 > > + - ``slice_type`` > > + - > > +* - __u8 > > + - ``pic_parameter_set_id`` > > + - > > +* - __u8 > > + - ``colour_plane_id`` > > + - > > +* - __u8 > > + - ``redundant_pic_cnt`` > > + - > > +* - __u16 > > + - ``frame_num`` > > + - > > +* - __u16 > > + - ``idr_pic_id`` > > + - > > +* - __u16 > > + - ``pic_order_cnt_lsb`` > > + - > > +* - __s32 > > + - ``delta_pic_order_cnt_bottom`` > > + - > > +* - __s32 > > + - ``delta_pic_order_cnt0`` > > + - > > +* - __s32 > > + - ``delta_pic_order_cnt1`` > > + - > > +* - struct :c:type:`v4l2_h264_pred_weight_table` > > + - ``pred_weight_table`` > > + - > > +* - __u32 > > + - ``dec_ref_pic_marking_bit_size`` > > + - > > +* - __u32 > > + - ``pic_order_cnt_bit_size`` > > + - > > +* - __u8 > > + - ``cabac_init_idc`` > > + - > > +* - __s8 > > + - ``slice_qp_delta`` > > + - > > +* - __s8 > > + - ``slice_qs_delta`` > > + - > > +* - __u8 > > + - ``disable_deblocking_filter_idc`` > > + - > > +* - __s8 > > + - ``slice_alpha_c0_offset_div2`` > > + - > > +* - __s8 > > + - ``slice_beta_offset_div2`` > > + - > > +* - __u8 > > + - ``num_ref_idx_l0_active_minus1`` > > + - > > +* - __u8 > > + - ``num_ref_idx_l1_active_minus1`` > > + - > > +* - __u32 > > + - ``slice_group_change_cycle`` > > + - > > +* - __u8 > > + - ``ref_pic_list0[32]`` > > + - > > +* - __u8 > > + - ``ref_pic_list1[32]`` > > + - > > Should we explicitly document that these are the lists after applying > the per-slice modifications, as opposed to the original order from > v4l2_ctrl_h264_decode_param? > > [snip] > > +* .. _V4L2-PIX-FMT-H264-SLICE: > > + > > + - ``V4L2_PIX_FMT_H264_SLICE`` > > + - 'S264' > > + - H264 parsed slice data, as extracted from the H264 bitstream. > > + This format is adapted for stateless video decoders that > > + implement an H264 pipeline (using the :ref:`codec` and > > + :ref:`media-request-api`). Metadata associated with t
Re: [PATCH v4 2/2] media: cedrus: Add H264 decoding support
Hi Jernej,
On Wed, Feb 20, 2019 at 06:50:54PM +0100, Jernej Škrabec wrote:
> I really wanted to do another review on previous series but got distracted by
> analyzing one particulary troublesome H264 sample. It still doesn't work
> correctly, so I would ask you if you can test it with your stack (it might be
> userspace issue):
>
> http://jernej.libreelec.tv/videos/problematic/test.mkv
>
> Please take a look at my comments below.
I'd really prefer to focus on getting this merged at this point, and
then fixing odd videos and / or setups we can find later
on. Especially when new stacks are going to be developped on top of
this, I'm sure we're going to have plenty of bugs to address :)
> Dne sreda, 20. februar 2019 ob 15:17:34 CET je Maxime Ripard napisal(a):
> > Introduce some basic H264 decoding support in cedrus. So far, only the
> > baseline profile videos have been tested, and some more advanced features
> > used in higher profiles are not even implemented.
>
> What is not yet implemented? Multi slice frame decoding, interlaced frames
> and
> decoding frames with width > 2048. Anything else?
Off the top of my head, nope.
> > +static void cedrus_h264_write_sram(struct cedrus_dev *dev,
> > + enum cedrus_h264_sram_off off,
> > + const void *data, size_t len)
> > +{
> > + const u32 *buffer = data;
> > + size_t count = DIV_ROUND_UP(len, 4);
> > +
> > + cedrus_write(dev, VE_AVC_SRAM_PORT_OFFSET, off << 2);
> > +
> > + do {
> > + cedrus_write(dev, VE_AVC_SRAM_PORT_DATA, *buffer++);
> > + } while (--count);
>
> Above loop will still write one word for count = 0. I propose following:
>
> while (count--)
> cedrus_write(dev, VE_AVC_SRAM_PORT_DATA, *buffer++);
Good catch, thanks!
> > + position = find_next_zero_bit(&used_dpbs, CEDRUS_H264_FRAME_NUM,
> > + output);
> > + if (position >= CEDRUS_H264_FRAME_NUM)
> > + position = find_first_zero_bit(&used_dpbs,
> CEDRUS_H264_FRAME_NUM);
>
> I guess you didn't try any interlaced videos? Sometimes it happens that
> buffer
> is reference and output at the same time. In such cases, above code would
> make
> two entries, which doesn't work based on Kwiboo's and my experiments.
>
> I guess decoding interlaced videos is out of scope at this time?
Yep, and that should be pretty easy to fix.
> > +
> > + output_buf = vb2_to_cedrus_buffer(&run->dst->vb2_buf);
> > + output_buf->codec.h264.position = position;
> > +
> > + if (slice->flags & V4L2_H264_SLICE_FLAG_FIELD_PIC)
> > + output_buf->codec.h264.pic_type =
> CEDRUS_H264_PIC_TYPE_FIELD;
> > + else if (sps->flags & V4L2_H264_SPS_FLAG_MB_ADAPTIVE_FRAME_FIELD)
> > + output_buf->codec.h264.pic_type =
> CEDRUS_H264_PIC_TYPE_MBAFF;
> > + else
> > + output_buf->codec.h264.pic_type =
> CEDRUS_H264_PIC_TYPE_FRAME;
> > +
> > + cedrus_fill_ref_pic(ctx, output_buf,
> > + dec_param->top_field_order_cnt,
> > + dec_param->bottom_field_order_cnt,
> > + &pic_list[position]);
> > +
> > + cedrus_h264_write_sram(dev, CEDRUS_SRAM_H264_FRAMEBUFFER_LIST,
> > + pic_list, sizeof(pic_list));
> > +
> > + cedrus_write(dev, VE_H264_OUTPUT_FRAME_IDX, position);
> > +}
> > +
> > +#define CEDRUS_MAX_REF_IDX 32
> > +
> > +static void _cedrus_write_ref_list(struct cedrus_ctx *ctx,
> > + struct cedrus_run *run,
> > + const u8 *ref_list, u8 num_ref,
> > + enum cedrus_h264_sram_off sram)
> > +{
> > + const struct v4l2_ctrl_h264_decode_param *decode = run-
> >h264.decode_param;
> > + struct vb2_queue *cap_q = &ctx->fh.m2m_ctx->cap_q_ctx.q;
> > + const struct vb2_buffer *dst_buf = &run->dst->vb2_buf;
> > + struct cedrus_dev *dev = ctx->dev;
> > + u8 sram_array[CEDRUS_MAX_REF_IDX];
> > + unsigned int i;
> > + size_t size;
> > +
> > + memset(sram_array, 0, sizeof(sram_array));
> > +
> > + for (i = 0; i < num_ref; i++) {
> > + const struct v4l2_h264_dpb_entry *dpb;
> > + const struct cedrus_buffer *cedrus_buf;
> > + const struct vb2_v4l2_buffer *ref_buf;
> > + unsigned int position;
> > + int buf_idx;
> > +
Re: [PATCH v4 1/2] media: uapi: Add H264 low-level decoder API compound controls.
Hi,
On Mon, Mar 04, 2019 at 03:49:11PM -0300, Ezequiel Garcia wrote:
> On Wed, 2019-02-20 at 15:17 +0100, Maxime Ripard wrote:
> > From: Pawel Osciak
> >
> > Stateless video codecs will require both the H264 metadata and slices in
> > order to be able to decode frames.
> >
> > This introduces the definitions for a new pixel format for H264 slices that
> > have been parsed, as well as the structures used to pass the metadata from
> > the userspace to the kernel.
> >
> > Co-Developped-by: Maxime Ripard
> > Signed-off-by: Pawel Osciak
> > Signed-off-by: Guenter Roeck
> > Signed-off-by: Maxime Ripard
> > ---
> > Documentation/media/uapi/v4l/biblio.rst| 9 +-
> > Documentation/media/uapi/v4l/extended-controls.rst | 547 ++-
>
> It seems Hans splitted the documentation and so this should now
> go to Documentation/media/uapi/v4l/ext-ctrls-codec.rst.
Thanks for letting me know, it will be fixed in the next version.
> >
> > +#define V4L2_PIX_FMT_H264_SLICE v4l2_fourcc('S', '2', '6', '4') /* H264
> > parsed slices */
> > #define V4L2_PIX_FMT_H263 v4l2_fourcc('H', '2', '6', '3') /* H263
> > */
> > #define V4L2_PIX_FMT_MPEG1v4l2_fourcc('M', 'P', 'G', '1') /* MPEG-1 ES
> > */
> > #define V4L2_PIX_FMT_MPEG2v4l2_fourcc('M', 'P', 'G', '2') /* MPEG-2 ES
> > */
>
> I haven't seen any objections to renaming this to V4L2_PIX_FMT_H264_SLICE_RAW,
> so if you could be so kind to push v5 with this rename (or similar), and also
> rebasing to the master branch, I could then submit the H264 decoder support
> for
> the Rockchip VPU.
I don't remember it, but yeah, this is fine by me. I'll adjust it and
send a new version.
Maxime
--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
signature.asc
Description: PGP signature
Re: [PATCH v4 1/2] media: uapi: Add H264 low-level decoder API compound controls.
Hi Ezequiel,
On Fri, Feb 22, 2019 at 01:59:33PM -0300, Ezequiel Garcia wrote:
> On Fri, 2019-02-22 at 16:46 +0900, Tomasz Figa wrote:
> > > diff --git a/include/uapi/linux/videodev2.h
> > > b/include/uapi/linux/videodev2.h
> > > index 9a920f071ff9..6443ae53597f 100644
> > > --- a/include/uapi/linux/videodev2.h
> > > +++ b/include/uapi/linux/videodev2.h
> > > @@ -653,6 +653,7 @@ struct v4l2_pix_format {
> > > #define V4L2_PIX_FMT_H264 v4l2_fourcc('H', '2', '6', '4') /* H264
> > > with start codes */
> > > #define V4L2_PIX_FMT_H264_NO_SC v4l2_fourcc('A', 'V', 'C', '1') /* H264
> > > without start codes */
> > > #define V4L2_PIX_FMT_H264_MVC v4l2_fourcc('M', '2', '6', '4') /* H264
> > > MVC */
> > > +#define V4L2_PIX_FMT_H264_SLICE v4l2_fourcc('S', '2', '6', '4') /* H264
> > > parsed slices */
> >
> > Are we okay with adding here already, without going through staging first?
>
> Also regarding the pixel formats. I still think we should have two
> pixel formats: V4L2_PIX_FMT_H264_SLICE_RAW and
> V4L2_PIX_FMT_H264_SLICE_ANNEX_B, to properly represent "raw" NALUs
> and "annex B" formatted NALUs.
I agree with that, but I was under the impression that it would be
part of your series, since you would be the prime user (at first at
least).
Maxime
--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
signature.asc
Description: PGP signature
Re: [PATCH v4 05/12] media: ov5640: Compute the clock rate at runtime
On Mon, Feb 25, 2019 at 10:21:51AM +0100, Jacopo Mondi wrote: > Hello Maxime, Benoit, > sorry for chiming in, but I'm a bit confused... > > On Fri, Feb 22, 2019 at 04:04:21PM +0100, Maxime Ripard wrote: > > On Fri, Feb 22, 2019 at 08:54:56AM -0600, Benoit Parrot wrote: > > > Maxime Ripard wrote on Fri [2019-Feb-22 > > > 15:39:59 +0100]: > > > > On Thu, Feb 21, 2019 at 10:20:20AM -0600, Benoit Parrot wrote: > > > > > Hi Maxime, > > > > > > > > > > A couple of questions, > > > > > > > > > > Maxime Ripard wrote on Thu [2018-Oct-11 > > > > > 04:21:00 -0500]: > > > > > > The clock rate, while hardcoded until now, is actually a function > > > > > > of the > > > > > > resolution, framerate and bytes per pixel. Now that we have an > > > > > > algorithm to > > > > > > adjust our clock rate, we can select it dynamically when we change > > > > > > the > > > > > > mode. > > > > > > > > > > > > This changes a bit the clock rate being used, with the following > > > > > > effect: > > > > > > > > > > > > +--+--+--+--+-+-++---+ > > > > > > | Hact | Vact | Htot | Vtot | FPS | Hardcoded clock | Computed > > > > > > clock | Deviation | > > > > > > +--+--+--+--+-+-++---+ > > > > > > | 640 | 480 | 1896 | 1080 | 15 |5600 | > > > > > > 61430400 | 8.84 %| > > > > > > | 640 | 480 | 1896 | 1080 | 30 | 11200 | > > > > > > 122860800 | 8.84 %| > > > > > > | 1024 | 768 | 1896 | 1080 | 15 |5600 | > > > > > > 61430400 | 8.84 %| > > > > > > | 1024 | 768 | 1896 | 1080 | 30 | 11200 | > > > > > > 122860800 | 8.84 %| > > > > > > | 320 | 240 | 1896 | 984 | 15 |5600 | > > > > > > 55969920 | 0.05 %| > > > > > > | 320 | 240 | 1896 | 984 | 30 | 11200 | > > > > > > 111939840 | 0.05 %| > > > > > > | 176 | 144 | 1896 | 984 | 15 |5600 | > > > > > > 55969920 | 0.05 %| > > > > > > | 176 | 144 | 1896 | 984 | 30 | 11200 | > > > > > > 111939840 | 0.05 %| > > > > > > | 720 | 480 | 1896 | 984 | 15 |5600 | > > > > > > 55969920 | 0.05 %| > > > > > > | 720 | 480 | 1896 | 984 | 30 | 11200 | > > > > > > 111939840 | 0.05 %| > > > > > > | 720 | 576 | 1896 | 984 | 15 |5600 | > > > > > > 55969920 | 0.05 %| > > > > > > | 720 | 576 | 1896 | 984 | 30 | 11200 | > > > > > > 111939840 | 0.05 %| > > > > > > | 1280 | 720 | 1892 | 740 | 15 |4200 | > > > > > > 42002400 | 0.01 %| > > > > > > | 1280 | 720 | 1892 | 740 | 30 |8400 | > > > > > > 84004800 | 0.01 %| > > > > > > | 1920 | 1080 | 2500 | 1120 | 15 |8400 | > > > > > > 8400 | 0.00 %| > > > > > > | 1920 | 1080 | 2500 | 1120 | 30 | 16800 | > > > > > > 16800 | 0.00 %| > > > > > > | 2592 | 1944 | 2844 | 1944 | 15 |8400 | > > > > > > 165862080 | 49.36 % | > > > > > > +--+--+--+--+-+-++---+ > > > > > > > > > > Is the computed clock above the same for both parallel and CSI2? > > > > > > > > > > I want to add controls for PIXEL_RATE and LINK_FREQ, would you have > > > > > any > > > > > quick pointer on taking the computed clock and translating that into > > > > > the > > > > > PIXEL_RATE and LINK_FREQ values? > > > > > > > > > > I am trying to use this sensor with TI CAL driver which at the moment > > > > > uses > > > > > the PIXEL_RATE values in order to compute ths_settle and ths_term > > > > > values &
Re: [PATCH v4 05/12] media: ov5640: Compute the clock rate at runtime
On Fri, Feb 22, 2019 at 08:54:56AM -0600, Benoit Parrot wrote: > Maxime Ripard wrote on Fri [2019-Feb-22 15:39:59 > +0100]: > > On Thu, Feb 21, 2019 at 10:20:20AM -0600, Benoit Parrot wrote: > > > Hi Maxime, > > > > > > A couple of questions, > > > > > > Maxime Ripard wrote on Thu [2018-Oct-11 > > > 04:21:00 -0500]: > > > > The clock rate, while hardcoded until now, is actually a function of the > > > > resolution, framerate and bytes per pixel. Now that we have an > > > > algorithm to > > > > adjust our clock rate, we can select it dynamically when we change the > > > > mode. > > > > > > > > This changes a bit the clock rate being used, with the following effect: > > > > > > > > +--+--+--+--+-+-++---+ > > > > | Hact | Vact | Htot | Vtot | FPS | Hardcoded clock | Computed clock | > > > > Deviation | > > > > +--+--+--+--+-+-++---+ > > > > | 640 | 480 | 1896 | 1080 | 15 |5600 | 61430400 | > > > > 8.84 %| > > > > | 640 | 480 | 1896 | 1080 | 30 | 11200 | 122860800 | > > > > 8.84 %| > > > > | 1024 | 768 | 1896 | 1080 | 15 |5600 | 61430400 | > > > > 8.84 %| > > > > | 1024 | 768 | 1896 | 1080 | 30 | 11200 | 122860800 | > > > > 8.84 %| > > > > | 320 | 240 | 1896 | 984 | 15 |5600 | 55969920 | > > > > 0.05 %| > > > > | 320 | 240 | 1896 | 984 | 30 | 11200 | 111939840 | > > > > 0.05 %| > > > > | 176 | 144 | 1896 | 984 | 15 |5600 | 55969920 | > > > > 0.05 %| > > > > | 176 | 144 | 1896 | 984 | 30 | 11200 | 111939840 | > > > > 0.05 %| > > > > | 720 | 480 | 1896 | 984 | 15 |5600 | 55969920 | > > > > 0.05 %| > > > > | 720 | 480 | 1896 | 984 | 30 | 11200 | 111939840 | > > > > 0.05 %| > > > > | 720 | 576 | 1896 | 984 | 15 |5600 | 55969920 | > > > > 0.05 %| > > > > | 720 | 576 | 1896 | 984 | 30 | 11200 | 111939840 | > > > > 0.05 %| > > > > | 1280 | 720 | 1892 | 740 | 15 |4200 | 42002400 | > > > > 0.01 %| > > > > | 1280 | 720 | 1892 | 740 | 30 |8400 | 84004800 | > > > > 0.01 %| > > > > | 1920 | 1080 | 2500 | 1120 | 15 |8400 | 8400 | > > > > 0.00 %| > > > > | 1920 | 1080 | 2500 | 1120 | 30 | 16800 | 16800 | > > > > 0.00 %| > > > > | 2592 | 1944 | 2844 | 1944 | 15 |8400 | 165862080 | > > > > 49.36 % | > > > > +--+--+--+--+-+-++---+ > > > > > > Is the computed clock above the same for both parallel and CSI2? > > > > > > I want to add controls for PIXEL_RATE and LINK_FREQ, would you have any > > > quick pointer on taking the computed clock and translating that into the > > > PIXEL_RATE and LINK_FREQ values? > > > > > > I am trying to use this sensor with TI CAL driver which at the moment uses > > > the PIXEL_RATE values in order to compute ths_settle and ths_term values > > > needed to program the DPHY properly. This is similar in behavior as the > > > way > > > omap3isp relies on this info as well. > > > > I haven't looked that much into the csi-2 case, but the pixel rate > > should be the same at least. > > I'll have to study the way the computed clock is actually calculated for > either case, but if they yield the same number then I would be surprised > that the pixel rate would be the same as in parallel mode you get 8 data > bits per clock whereas in CSI2 using 2 data lanes you get 4 data bits per > clock. The bus rate will be different, but the pixel rate is the same: you have as many pixels per frames and as many frames per seconds in the parallel and CSI cases. > So just to be certain here the "Computed clock" column above would be the > pixel clock frequency? it is Maxime -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com signature.asc Description: PGP signature
Re: [PATCH v4 05/12] media: ov5640: Compute the clock rate at runtime
On Thu, Feb 21, 2019 at 10:20:20AM -0600, Benoit Parrot wrote: > Hi Maxime, > > A couple of questions, > > Maxime Ripard wrote on Thu [2018-Oct-11 04:21:00 > -0500]: > > The clock rate, while hardcoded until now, is actually a function of the > > resolution, framerate and bytes per pixel. Now that we have an algorithm to > > adjust our clock rate, we can select it dynamically when we change the > > mode. > > > > This changes a bit the clock rate being used, with the following effect: > > > > +--+--+--+--+-+-++---+ > > | Hact | Vact | Htot | Vtot | FPS | Hardcoded clock | Computed clock | > > Deviation | > > +--+--+--+--+-+-++---+ > > | 640 | 480 | 1896 | 1080 | 15 |5600 | 61430400 | 8.84 > > %| > > | 640 | 480 | 1896 | 1080 | 30 | 11200 | 122860800 | 8.84 > > %| > > | 1024 | 768 | 1896 | 1080 | 15 |5600 | 61430400 | 8.84 > > %| > > | 1024 | 768 | 1896 | 1080 | 30 | 11200 | 122860800 | 8.84 > > %| > > | 320 | 240 | 1896 | 984 | 15 |5600 | 55969920 | 0.05 > > %| > > | 320 | 240 | 1896 | 984 | 30 | 11200 | 111939840 | 0.05 > > %| > > | 176 | 144 | 1896 | 984 | 15 |5600 | 55969920 | 0.05 > > %| > > | 176 | 144 | 1896 | 984 | 30 | 11200 | 111939840 | 0.05 > > %| > > | 720 | 480 | 1896 | 984 | 15 |5600 | 55969920 | 0.05 > > %| > > | 720 | 480 | 1896 | 984 | 30 | 11200 | 111939840 | 0.05 > > %| > > | 720 | 576 | 1896 | 984 | 15 |5600 | 55969920 | 0.05 > > %| > > | 720 | 576 | 1896 | 984 | 30 | 11200 | 111939840 | 0.05 > > %| > > | 1280 | 720 | 1892 | 740 | 15 |4200 | 42002400 | 0.01 > > %| > > | 1280 | 720 | 1892 | 740 | 30 |8400 | 84004800 | 0.01 > > %| > > | 1920 | 1080 | 2500 | 1120 | 15 |8400 | 8400 | 0.00 > > %| > > | 1920 | 1080 | 2500 | 1120 | 30 | 16800 | 16800 | 0.00 > > %| > > | 2592 | 1944 | 2844 | 1944 | 15 |8400 | 165862080 | > > 49.36 % | > > +--+--+--+--+-+-++---+ > > Is the computed clock above the same for both parallel and CSI2? > > I want to add controls for PIXEL_RATE and LINK_FREQ, would you have any > quick pointer on taking the computed clock and translating that into the > PIXEL_RATE and LINK_FREQ values? > > I am trying to use this sensor with TI CAL driver which at the moment uses > the PIXEL_RATE values in order to compute ths_settle and ths_term values > needed to program the DPHY properly. This is similar in behavior as the way > omap3isp relies on this info as well. I haven't looked that much into the csi-2 case, but the pixel rate should be the same at least. Maxime -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com signature.asc Description: PGP signature
[PATCH v4 1/2] media: uapi: Add H264 low-level decoder API compound controls.
From: Pawel Osciak Stateless video codecs will require both the H264 metadata and slices in order to be able to decode frames. This introduces the definitions for a new pixel format for H264 slices that have been parsed, as well as the structures used to pass the metadata from the userspace to the kernel. Co-Developped-by: Maxime Ripard Signed-off-by: Pawel Osciak Signed-off-by: Guenter Roeck Signed-off-by: Maxime Ripard --- Documentation/media/uapi/v4l/biblio.rst| 9 +- Documentation/media/uapi/v4l/extended-controls.rst | 547 ++- Documentation/media/uapi/v4l/pixfmt-compressed.rst | 20 +- Documentation/media/uapi/v4l/vidioc-queryctrl.rst | 30 +- Documentation/media/videodev2.h.rst.exceptions | 5 +- drivers/media/v4l2-core/v4l2-ctrls.c | 42 +- drivers/media/v4l2-core/v4l2-ioctl.c | 1 +- include/media/h264-ctrls.h | 190 +- include/media/v4l2-ctrls.h | 13 +- include/uapi/linux/videodev2.h | 1 +- 10 files changed, 857 insertions(+), 1 deletion(-) create mode 100644 include/media/h264-ctrls.h diff --git a/Documentation/media/uapi/v4l/biblio.rst b/Documentation/media/uapi/v4l/biblio.rst index ec33768c055e..3fc3f7ff338a 100644 --- a/Documentation/media/uapi/v4l/biblio.rst +++ b/Documentation/media/uapi/v4l/biblio.rst @@ -122,6 +122,15 @@ ITU BT.1119 :author:International Telecommunication Union (http://www.itu.ch) +.. _h264: + +ITU H.264 += + +:title: ITU-T Recommendation H.264 "Advanced Video Coding for Generic Audiovisual Services" + +:author:International Telecommunication Union (http://www.itu.ch) + .. _jfif: JFIF diff --git a/Documentation/media/uapi/v4l/extended-controls.rst b/Documentation/media/uapi/v4l/extended-controls.rst index 00934efdc9e4..7e27a5139732 100644 --- a/Documentation/media/uapi/v4l/extended-controls.rst +++ b/Documentation/media/uapi/v4l/extended-controls.rst @@ -1712,6 +1712,553 @@ enum v4l2_mpeg_video_h264_hierarchical_coding_type - non-intra-coded frames, in zigzag scanning order. Only relevant for non-4:2:0 YUV formats. +.. _v4l2-mpeg-h264: + +``V4L2_CID_MPEG_VIDEO_H264_SPS (struct)`` +Specifies the sequence parameter set (as extracted from the +bitstream) for the associated H264 slice data. This includes the +necessary parameters for configuring a stateless hardware decoding +pipeline for H264. The bitstream parameters are defined according +to :ref:`h264`. Unless there's a specific comment, refer to the +specification for the documentation of these fields, section 7.4.2.1.1 +"Sequence Parameter Set Data Semantics". + +.. note:: + + This compound control is not yet part of the public kernel API and + it is expected to change. + +.. c:type:: v4l2_ctrl_h264_sps + +.. cssclass:: longtable + +.. flat-table:: struct v4l2_ctrl_h264_sps +:header-rows: 0 +:stub-columns: 0 +:widths: 1 1 2 + +* - __u8 + - ``profile_idc`` + - +* - __u8 + - ``constraint_set_flags`` + - See :ref:`Sequence Parameter Set Constraints Set Flags ` +* - __u8 + - ``level_idc`` + - +* - __u8 + - ``seq_parameter_set_id`` + - +* - __u8 + - ``chroma_format_idc`` + - +* - __u8 + - ``bit_depth_luma_minus8`` + - +* - __u8 + - ``bit_depth_chroma_minus8`` + - +* - __u8 + - ``log2_max_frame_num_minus4`` + - +* - __u8 + - ``pic_order_cnt_type`` + - +* - __u8 + - ``log2_max_pic_order_cnt_lsb_minus4`` + - +* - __u8 + - ``max_num_ref_frames`` + - +* - __u8 + - ``num_ref_frames_in_pic_order_cnt_cycle`` + - +* - __s32 + - ``offset_for_ref_frame[255]`` + - +* - __s32 + - ``offset_for_non_ref_pic`` + - +* - __s32 + - ``offset_for_top_to_bottom_field`` + - +* - __u16 + - ``pic_width_in_mbs_minus1`` + - +* - __u16 + - ``pic_height_in_map_units_minus1`` + - +* - __u32 + - ``flags`` + - See :ref:`Sequence Parameter Set Flags ` + +.. _h264_sps_constraints_set_flags: + +``Sequence Parameter Set Constraints Set Flags`` + +.. cssclass:: longtable + +.. flat-table:: +:header-rows: 0 +:stub-columns: 0 +:widths: 1 1 2 + +* - ``V4L2_H264_SPS_CONSTRAINT_SET0_FLAG`` + - 0x0001 + - +* - ``V4L2_H264_SPS_CONSTRAINT_SET1_FLAG`` + - 0x0002 + - +* - ``V4L2_H264_SPS_CONSTRAINT_SET2_FLAG`` + - 0x0004 + - +* - ``V4L2_H264_SPS_CONSTRAINT_SET3_FLAG`` + - 0x0008 + - +* - ``V4L2_H264_SPS_CONSTRAINT_SET4_FLAG`` + - 0x0010 + - +* - ``V4L2_H264_SPS_CONSTRAINT_SET5_FLAG`` + - 0x0020 + - + +.. _h264_sps_flags: + +``Sequence Parameter Set Flags`` + +.. cssclass:: longtable + +.. flat-table:: +:h
[PATCH v4 2/2] media: cedrus: Add H264 decoding support
Introduce some basic H264 decoding support in cedrus. So far, only the
baseline profile videos have been tested, and some more advanced features
used in higher profiles are not even implemented.
Signed-off-by: Maxime Ripard
---
drivers/staging/media/sunxi/cedrus/Makefile | 3 +-
drivers/staging/media/sunxi/cedrus/cedrus.c | 30 +-
drivers/staging/media/sunxi/cedrus/cedrus.h | 38 +-
drivers/staging/media/sunxi/cedrus/cedrus_dec.c | 13 +-
drivers/staging/media/sunxi/cedrus/cedrus_h264.c | 584 +++-
drivers/staging/media/sunxi/cedrus/cedrus_hw.c| 4 +-
drivers/staging/media/sunxi/cedrus/cedrus_regs.h | 91 ++-
drivers/staging/media/sunxi/cedrus/cedrus_video.c | 9 +-
8 files changed, 770 insertions(+), 2 deletions(-)
create mode 100644 drivers/staging/media/sunxi/cedrus/cedrus_h264.c
diff --git a/drivers/staging/media/sunxi/cedrus/Makefile
b/drivers/staging/media/sunxi/cedrus/Makefile
index e9dc68b7bcb6..aaf141fc58b6 100644
--- a/drivers/staging/media/sunxi/cedrus/Makefile
+++ b/drivers/staging/media/sunxi/cedrus/Makefile
@@ -1,3 +1,4 @@
obj-$(CONFIG_VIDEO_SUNXI_CEDRUS) += sunxi-cedrus.o
-sunxi-cedrus-y = cedrus.o cedrus_video.o cedrus_hw.o cedrus_dec.o
cedrus_mpeg2.o
+sunxi-cedrus-y = cedrus.o cedrus_video.o cedrus_hw.o cedrus_dec.o \
+cedrus_mpeg2.o cedrus_h264.o
diff --git a/drivers/staging/media/sunxi/cedrus/cedrus.c
b/drivers/staging/media/sunxi/cedrus/cedrus.c
index ff11cbeba205..c1607142d998 100644
--- a/drivers/staging/media/sunxi/cedrus/cedrus.c
+++ b/drivers/staging/media/sunxi/cedrus/cedrus.c
@@ -40,6 +40,35 @@ static const struct cedrus_control cedrus_controls[] = {
.codec = CEDRUS_CODEC_MPEG2,
.required = false,
},
+ {
+ .id = V4L2_CID_MPEG_VIDEO_H264_DECODE_PARAMS,
+ .elem_size = sizeof(struct v4l2_ctrl_h264_decode_param),
+ .codec = CEDRUS_CODEC_H264,
+ .required = true,
+ },
+ {
+ .id = V4L2_CID_MPEG_VIDEO_H264_SLICE_PARAMS,
+ .elem_size = sizeof(struct v4l2_ctrl_h264_slice_param),
+ .codec = CEDRUS_CODEC_H264,
+ .required = true,
+ },
+ {
+ .id = V4L2_CID_MPEG_VIDEO_H264_SPS,
+ .elem_size = sizeof(struct v4l2_ctrl_h264_sps),
+ .codec = CEDRUS_CODEC_H264,
+ .required = true,
+ },
+ {
+ .id = V4L2_CID_MPEG_VIDEO_H264_PPS,
+ .elem_size = sizeof(struct v4l2_ctrl_h264_pps),
+ .codec = CEDRUS_CODEC_H264,
+ .required = true,
+ },
+ {
+ .id = V4L2_CID_MPEG_VIDEO_H264_SCALING_MATRIX,
+ .elem_size = sizeof(struct v4l2_ctrl_h264_scaling_matrix),
+ .codec = CEDRUS_CODEC_H264,
+ },
};
#define CEDRUS_CONTROLS_COUNT ARRAY_SIZE(cedrus_controls)
@@ -278,6 +307,7 @@ static int cedrus_probe(struct platform_device *pdev)
}
dev->dec_ops[CEDRUS_CODEC_MPEG2] = &cedrus_dec_ops_mpeg2;
+ dev->dec_ops[CEDRUS_CODEC_H264] = &cedrus_dec_ops_h264;
mutex_init(&dev->dev_mutex);
diff --git a/drivers/staging/media/sunxi/cedrus/cedrus.h
b/drivers/staging/media/sunxi/cedrus/cedrus.h
index 4aedd24a9848..8c64f9a27e9d 100644
--- a/drivers/staging/media/sunxi/cedrus/cedrus.h
+++ b/drivers/staging/media/sunxi/cedrus/cedrus.h
@@ -30,7 +30,7 @@
enum cedrus_codec {
CEDRUS_CODEC_MPEG2,
-
+ CEDRUS_CODEC_H264,
CEDRUS_CODEC_LAST,
};
@@ -40,6 +40,12 @@ enum cedrus_irq_status {
CEDRUS_IRQ_OK,
};
+enum cedrus_h264_pic_type {
+ CEDRUS_H264_PIC_TYPE_FRAME = 0,
+ CEDRUS_H264_PIC_TYPE_FIELD,
+ CEDRUS_H264_PIC_TYPE_MBAFF,
+};
+
struct cedrus_control {
u32 id;
u32 elem_size;
@@ -47,6 +53,14 @@ struct cedrus_control {
unsigned char required:1;
};
+struct cedrus_h264_run {
+ const struct v4l2_ctrl_h264_decode_param*decode_param;
+ const struct v4l2_ctrl_h264_pps *pps;
+ const struct v4l2_ctrl_h264_scaling_matrix *scaling_matrix;
+ const struct v4l2_ctrl_h264_slice_param *slice_param;
+ const struct v4l2_ctrl_h264_sps *sps;
+};
+
struct cedrus_mpeg2_run {
const struct v4l2_ctrl_mpeg2_slice_params *slice_params;
const struct v4l2_ctrl_mpeg2_quantization *quantization;
@@ -57,12 +71,20 @@ struct cedrus_run {
struct vb2_v4l2_buffer *dst;
union {
+ struct cedrus_h264_run h264;
struct cedrus_mpeg2_run mpeg2;
};
};
struct cedrus_buffer {
struct v4l2_m2m_buffer m2m
[PATCH v4 0/2] media: cedrus: Add H264 decoding support
Hi, Here is a new version of the H264 decoding support in the cedrus driver. As you might already know, the cedrus driver relies on the Request API, and is a reverse engineered driver for the video decoding engine found on the Allwinner SoCs. This work has been possible thanks to the work done by the people behind libvdpau-sunxi found here: https://github.com/linux-sunxi/libvdpau-sunxi/ I've tested the various ABI using this gdb script: http://code.bulix.org/jl4se4-505620?raw And this test script: http://code.bulix.org/8zle4s-505623?raw The application compiled is quite trivial: http://code.bulix.org/e34zp8-505624?raw The output is: arm:builds/arm-test-v4l2-h264-structures SHA1: fd15b30328765c2caac877e8ea8452c829b2b1d8 x86:builds/x86-test-v4l2-h264-structures SHA1: fd15b30328765c2caac877e8ea8452c829b2b1d8 x64:builds/x64-test-v4l2-h264-structures SHA1: fd15b30328765c2caac877e8ea8452c829b2b1d8 arm64: builds/arm64-test-v4l2-h264-structures SHA1: fd15b30328765c2caac877e8ea8452c829b2b1d8 Let me know if there's any flaw using that test setup, or if you have any comments on the patches. Maxime Changes from v3: - Reintroduced long term reference flag and documented it - Reintroduced ref_pic_list_p0/b0/b1 and documented it - Documented the DPB flags - Treat the scaling matrix as optional in the driver, as documented - Free the neighbor buffer - Increase the control IDs by a large margin to be safe of collisions - Reorder the fields documentation according to the structure layout - Change the tag documentation by the timestamp - Convert the sram array to size_t - Simplify the buffer retrieval from timestamp - Rebase Changes from v2: - Simplified _cedrus_write_ref_list as suggested by Jernej - Set whether the frame is used as reference using nal_ref_idc - Respect chroma_format_idc - Fixes for the scaling list and prediction tables - Wrote the documentation for the flags - Added a bunch of defines to the driver bit fields - Reworded the controls and data format descriptions as suggested by Hans - Reworked the controls' structure field size to avoid padding - Removed the long term reference flag - Reintroduced the neighbor info buffer - Removed the ref_pic_list_p0/b0/b1 arrays that are redundant with the one in the DPB - used the timestamps instead of tags - Rebased on 5.0-rc1 Changes from v1: - Rebased on 4.20 - Did the documentation for the userspace API - Used the tags instead of buffer IDs - Added a comment to explain why we still needed the swdec trigger - Reworked the MV col buffer in order to have one slot per frame - Removed the unused neighbor info buffer - Made sure to have the same structure offset and alignments across 32 bits and 64 bits architecture Maxime Ripard (1): media: cedrus: Add H264 decoding support Pawel Osciak (1): media: uapi: Add H264 low-level decoder API compound controls. Documentation/media/uapi/v4l/biblio.rst| 9 +- Documentation/media/uapi/v4l/extended-controls.rst | 547 +- Documentation/media/uapi/v4l/pixfmt-compressed.rst | 20 +- Documentation/media/uapi/v4l/vidioc-queryctrl.rst | 30 +- Documentation/media/videodev2.h.rst.exceptions | 5 +- drivers/media/v4l2-core/v4l2-ctrls.c | 42 +- drivers/media/v4l2-core/v4l2-ioctl.c | 1 +- drivers/staging/media/sunxi/cedrus/Makefile| 3 +- drivers/staging/media/sunxi/cedrus/cedrus.c| 30 +- drivers/staging/media/sunxi/cedrus/cedrus.h| 38 +- drivers/staging/media/sunxi/cedrus/cedrus_dec.c| 13 +- drivers/staging/media/sunxi/cedrus/cedrus_h264.c | 584 ++- drivers/staging/media/sunxi/cedrus/cedrus_hw.c | 4 +- drivers/staging/media/sunxi/cedrus/cedrus_regs.h | 91 ++- drivers/staging/media/sunxi/cedrus/cedrus_video.c | 9 +- include/media/h264-ctrls.h | 190 +- include/media/v4l2-ctrls.h | 13 +- include/uapi/linux/videodev2.h | 1 +- 18 files changed, 1627 insertions(+), 3 deletions(-) create mode 100644 drivers/staging/media/sunxi/cedrus/cedrus_h264.c create mode 100644 include/media/h264-ctrls.h base-commit: aa89b613a1c52cfd25076acfbbf6266c4b6f411b -- git-series 0.9.1
Re: [PATCH v3 0/2] media: cedrus: Add H264 decoding support
On Wed, Feb 13, 2019 at 01:28:34PM -0300, Ezequiel Garcia wrote: > On Wed, 2019-02-13 at 12:02 +0900, Tomasz Figa wrote: > > On Wed, Feb 13, 2019 at 6:22 AM Ezequiel Garcia > > wrote: > > > Hey Tomasz, > > > > > > On Tue, 2019-02-12 at 21:50 +0900, Tomasz Figa wrote: > > > > Hi Maxime, > > > > > > > > On Mon, Feb 11, 2019 at 11:39 PM Maxime Ripard > > > > wrote: > > > > > Hi, > > > > > > > > > > Here is a new version of the H264 decoding support in the cedrus > > > > > driver. > > > > > > > > Thanks for working on this. Please see my comments below. > > > > > > > > > As you might already know, the cedrus driver relies on the Request > > > > > API, and is a reverse engineered driver for the video decoding engine > > > > > found on the Allwinner SoCs. > > > > > > > > > > This work has been possible thanks to the work done by the people > > > > > behind libvdpau-sunxi found here: > > > > > https://github.com/linux-sunxi/libvdpau-sunxi/ > > > > > > > > > > I've tested the various ABI using this gdb script: > > > > > http://code.bulix.org/jl4se4-505620?raw > > > > > > > > > > And this test script: > > > > > http://code.bulix.org/8zle4s-505623?raw > > > > > > > > > > The application compiled is quite trivial: > > > > > http://code.bulix.org/e34zp8-505624?raw > > > > > > > > > > The output is: > > > > > arm:builds/arm-test-v4l2-h264-structures > > > > > SHA1: 88cbf7485ba81831fc3b93772b215599b3b38318 > > > > > x86:builds/x86-test-v4l2-h264-structures > > > > > SHA1: 88cbf7485ba81831fc3b93772b215599b3b38318 > > > > > x64:builds/x64-test-v4l2-h264-structures > > > > > SHA1: 88cbf7485ba81831fc3b93772b215599b3b38318 > > > > > arm64: builds/arm64-test-v4l2-h264-structures > > > > > SHA1: 88cbf7485ba81831fc3b93772b215599b3b38318 > > > > > > > > > > Let me know if there's any flaw using that test setup, or if you have > > > > > any comments on the patches. > > > > > > > > > > Maxime > > > > > > > > > > Changes from v2: > > > > > - Simplified _cedrus_write_ref_list as suggested by Jernej > > > > > - Set whether the frame is used as reference using nal_ref_idc > > > > > - Respect chroma_format_idc > > > > > - Fixes for the scaling list and prediction tables > > > > > - Wrote the documentation for the flags > > > > > - Added a bunch of defines to the driver bit fields > > > > > - Reworded the controls and data format descriptions as suggested > > > > > by Hans > > > > > - Reworked the controls' structure field size to avoid padding > > > > > - Removed the long term reference flag > > > > > > > > This and... > > > > > > > > > > Maxime has dropped this because of Ayaka's mail about long term references > > > not making much sense in stateless decoders. > > > > I haven't seen any argument confirming that thesis, though. I should > > have kicked in earlier, sorry. > > > > OK, in that case, we need to have this flag back. > > > > I noticed that RK3399 TRM has a field to specify long term refs and > > > so was wondering about this item as well. > > > > > > > > - Reintroduced the neighbor info buffer > > > > > - Removed the ref_pic_list_p0/b0/b1 arrays that are redundant with > > > > > the > > > > > one in the DPB > > > > > > > > these are used in our Rockchip VDEC driver. > > > > > > > > Could you elaborate on the reasons why they got removed? > > > > > > > > > > If I understood correctly, there are two reference picture lists. > > > P-frames will populate ref_pic_list0 and B-frames will populate both. > > > > > > According to this, v4l2_ctrl_h264_slice_param.ref_pic_list0 and > > > .ref_pic_list1 > > > should be enough and ref_pic_list_p0/b0/b1 are not needed. > > > > > > What do you think? > > > > The lists in v4l2_ctrl_h264_slice_param are expected
Re: [PATCH v3 1/2] media: uapi: Add H264 low-level decoder API compound controls.
Hi, On Mon, Feb 11, 2019 at 04:21:47PM +0100, Hans Verkuil wrote: > > I think the API should be designed with 4k video in mind. So if some of > > these constants would be too small when dealing with 4k (even if the > > current HW doesn't support this yet), then these constants would have to > > be increased. > > > > And yes, I know 8k video is starting to appear, but I think it is OK > > that additional control(s) would be needed to support 8k. > > Hmm, 4k (and up) is much more likely to use HEVC. So perhaps designing this > for 4k is overkill. > > Does anyone know if H.264 is used for 4k video at all? If not (or if very > rare), then just ignore this. I don't know the state of it right now, but until quite recently youtube at least was encoding their 4k videos in both VP9 and H264. They might have moved to h265 since, but considering 4k doesn't seem unreasonable. Maxime -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com signature.asc Description: PGP signature
Re: [PATCH v3 2/2] media: cedrus: Add H264 decoding support
On Mon, Feb 11, 2019 at 04:48:17PM -0300, Ezequiel Garcia wrote:
> On Mon, 2019-02-11 at 15:39 +0100, Maxime Ripard wrote:
> > Introduce some basic H264 decoding support in cedrus. So far, only the
> > baseline profile videos have been tested, and some more advanced features
> > used in higher profiles are not even implemented.
> >
> > Signed-off-by: Maxime Ripard
> [..]
>
>
> > +static void _cedrus_write_ref_list(struct cedrus_ctx *ctx,
> > + struct cedrus_run *run,
> > + const u8 *ref_list, u8 num_ref,
> > + enum cedrus_h264_sram_off sram)
> > +{
> > + const struct v4l2_ctrl_h264_decode_param *decode =
> > run->h264.decode_param;
> > + struct vb2_queue *cap_q = &ctx->fh.m2m_ctx->cap_q_ctx.q;
> > + const struct vb2_buffer *dst_buf = &run->dst->vb2_buf;
> > + struct cedrus_dev *dev = ctx->dev;
> > + u8 sram_array[CEDRUS_MAX_REF_IDX];
> > + unsigned int size, i;
> > +
> > + memset(sram_array, 0, sizeof(sram_array));
> > +
> > + for (i = 0; i < num_ref; i++) {
> > + const struct v4l2_h264_dpb_entry *dpb;
> > + const struct cedrus_buffer *cedrus_buf;
> > + const struct vb2_v4l2_buffer *ref_buf;
> > + unsigned int position;
> > + int buf_idx;
> > + u8 dpb_idx;
> > +
> > + dpb_idx = ref_list[i];
> > + dpb = &decode->dpb[dpb_idx];
> > +
> > + if (!(dpb->flags & V4L2_H264_DPB_ENTRY_FLAG_ACTIVE))
> > + continue;
> > +
> > + if (dst_buf->timestamp == dpb->timestamp)
> > + buf_idx = dst_buf->index;
> > + else
> > + buf_idx = vb2_find_timestamp(cap_q, dpb->timestamp, 0);
> > +
> > + if (buf_idx < 0)
> > + continue;
> > +
> > + ref_buf = to_vb2_v4l2_buffer(ctx->dst_bufs[buf_idx]);
> > + cedrus_buf = vb2_v4l2_to_cedrus_buffer(ref_buf);
> > + position = cedrus_buf->codec.h264.position;
> > +
> > + sram_array[i] |= position << 1;
> > + if (ref_buf->field == V4L2_FIELD_BOTTOM)
> > + sram_array[i] |= BIT(0);
> > + }
> > +
> > + size = min((unsigned int)ALIGN(num_ref, 4), sizeof(sram_array));
>
> Perhaps s/unsigned int/size_t, so the arguments to min() have the same type?
>
> Otherwise, I got this warning:
>
> /home/zeta/repos/linux/media_tree/drivers/staging/media/sunxi/cedrus/cedrus_h264.c:
> In function ‘_cedrus_write_ref_list’:
> /home/zeta/repos/linux/media_tree/include/linux/kernel.h:846:29: warning:
> comparison of distinct pointer types lacks a cast
>(!!(sizeof((typeof(x) *)1 == (typeof(y) *)1)))
Strange, I didn't notice any warning. I'll make sure to fix it.
Thanks!
Maxime
--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
Re: [linux-sunxi] [PATCH v3 2/2] media: cedrus: Add H264 decoding support
Hi, On Mon, Feb 11, 2019 at 08:21:31PM +0100, Jernej Škrabec wrote: > > + reg = 0; > > + /* > > +* FIXME: This bit tells the video engine to use the default > > +* quantization matrices. This will obviously need to be > > +* changed to support the profiles supporting custom > > +* quantization matrices. > > +*/ > > + reg |= VE_H264_SHS_QP_SCALING_MATRIX_DEFAULT; > > This flag should not be needed anymore. From what I see, you correctly set > scaling matrix every time. The scaling matrix control is optional, so I guess we should protect that by a check on whether that control has been set or not. What do you think? Thanks! Maxime -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com signature.asc Description: PGP signature
Re: [PATCH v3 1/2] media: uapi: Add H264 low-level decoder API compound controls.
On Mon, Feb 11, 2019 at 02:12:25PM -0300, Ezequiel Garcia wrote:
> On Mon, 2019-02-11 at 15:39 +0100, Maxime Ripard wrote:
> >
> > diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
> > index d6eed479c3a6..6fc955926bdb 100644
> > --- a/include/uapi/linux/videodev2.h
> > +++ b/include/uapi/linux/videodev2.h
> > @@ -645,6 +645,7 @@ struct v4l2_pix_format {
> > #define V4L2_PIX_FMT_H264 v4l2_fourcc('H', '2', '6', '4') /* H264 with
> > start codes */
> > #define V4L2_PIX_FMT_H264_NO_SC v4l2_fourcc('A', 'V', 'C', '1') /* H264
> > without start codes */
> > #define V4L2_PIX_FMT_H264_MVC v4l2_fourcc('M', '2', '6', '4') /* H264 MVC
> > */
> > +#define V4L2_PIX_FMT_H264_SLICE v4l2_fourcc('S', '2', '6', '4') /* H264
> > parsed slices */
>
> Nicolas and I have discussed the pixel format, and came up with
> the following proposal.
>
> Given this format represents H264 parsed slices, without any start code,
> perhpaps we name it as:
>
> V4L2_PIX_FMT_H264_SLICE_RAW
>
> Then, we'd also add:
>
> V4L2_PIX_FMT_H264_SLICE_ANNEX_B
>
> To represent H264 parsed slices with annex B (3- or 4-byte) start code.
> This one is what the Rockchip VPU driver would expose.
>
> Ideas?
I think we discussed that idea already, and I'm all for it.
Maxime
--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
signature.asc
Description: PGP signature
[PATCH v3 0/2] media: cedrus: Add H264 decoding support
Hi, Here is a new version of the H264 decoding support in the cedrus driver. As you might already know, the cedrus driver relies on the Request API, and is a reverse engineered driver for the video decoding engine found on the Allwinner SoCs. This work has been possible thanks to the work done by the people behind libvdpau-sunxi found here: https://github.com/linux-sunxi/libvdpau-sunxi/ I've tested the various ABI using this gdb script: http://code.bulix.org/jl4se4-505620?raw And this test script: http://code.bulix.org/8zle4s-505623?raw The application compiled is quite trivial: http://code.bulix.org/e34zp8-505624?raw The output is: arm:builds/arm-test-v4l2-h264-structures SHA1: 88cbf7485ba81831fc3b93772b215599b3b38318 x86:builds/x86-test-v4l2-h264-structures SHA1: 88cbf7485ba81831fc3b93772b215599b3b38318 x64:builds/x64-test-v4l2-h264-structures SHA1: 88cbf7485ba81831fc3b93772b215599b3b38318 arm64: builds/arm64-test-v4l2-h264-structures SHA1: 88cbf7485ba81831fc3b93772b215599b3b38318 Let me know if there's any flaw using that test setup, or if you have any comments on the patches. Maxime Changes from v2: - Simplified _cedrus_write_ref_list as suggested by Jernej - Set whether the frame is used as reference using nal_ref_idc - Respect chroma_format_idc - Fixes for the scaling list and prediction tables - Wrote the documentation for the flags - Added a bunch of defines to the driver bit fields - Reworded the controls and data format descriptions as suggested by Hans - Reworked the controls' structure field size to avoid padding - Removed the long term reference flag - Reintroduced the neighbor info buffer - Removed the ref_pic_list_p0/b0/b1 arrays that are redundant with the one in the DPB - used the timestamps instead of tags - Rebased on 5.0-rc1 Changes from v1: - Rebased on 4.20 - Did the documentation for the userspace API - Used the tags instead of buffer IDs - Added a comment to explain why we still needed the swdec trigger - Reworked the MV col buffer in order to have one slot per frame - Removed the unused neighbor info buffer - Made sure to have the same structure offset and alignments across 32 bits and 64 bits architecture Maxime Ripard (1): media: cedrus: Add H264 decoding support Pawel Osciak (1): media: uapi: Add H264 low-level decoder API compound controls. Documentation/media/uapi/v4l/biblio.rst| 9 +- Documentation/media/uapi/v4l/extended-controls.rst | 530 +- Documentation/media/uapi/v4l/pixfmt-compressed.rst | 20 +- Documentation/media/uapi/v4l/vidioc-queryctrl.rst | 30 +- Documentation/media/videodev2.h.rst.exceptions | 5 +- drivers/media/v4l2-core/v4l2-ctrls.c | 42 +- drivers/media/v4l2-core/v4l2-ioctl.c | 1 +- drivers/staging/media/sunxi/cedrus/Makefile| 3 +- drivers/staging/media/sunxi/cedrus/cedrus.c| 31 +- drivers/staging/media/sunxi/cedrus/cedrus.h| 38 +- drivers/staging/media/sunxi/cedrus/cedrus_dec.c| 15 +- drivers/staging/media/sunxi/cedrus/cedrus_h264.c | 589 ++- drivers/staging/media/sunxi/cedrus/cedrus_hw.c | 4 +- drivers/staging/media/sunxi/cedrus/cedrus_regs.h | 91 ++- drivers/staging/media/sunxi/cedrus/cedrus_video.c | 9 +- include/media/h264-ctrls.h | 179 - include/media/v4l2-ctrls.h | 13 +- include/uapi/linux/videodev2.h | 1 +- 18 files changed, 1607 insertions(+), 3 deletions(-) create mode 100644 drivers/staging/media/sunxi/cedrus/cedrus_h264.c create mode 100644 include/media/h264-ctrls.h base-commit: e7552cc33320523b660dd1891bceb616ced7b47c -- git-series 0.9.1
[PATCH v3 2/2] media: cedrus: Add H264 decoding support
Introduce some basic H264 decoding support in cedrus. So far, only the
baseline profile videos have been tested, and some more advanced features
used in higher profiles are not even implemented.
Signed-off-by: Maxime Ripard
---
drivers/staging/media/sunxi/cedrus/Makefile | 3 +-
drivers/staging/media/sunxi/cedrus/cedrus.c | 31 +-
drivers/staging/media/sunxi/cedrus/cedrus.h | 38 +-
drivers/staging/media/sunxi/cedrus/cedrus_dec.c | 15 +-
drivers/staging/media/sunxi/cedrus/cedrus_h264.c | 589 +++-
drivers/staging/media/sunxi/cedrus/cedrus_hw.c| 4 +-
drivers/staging/media/sunxi/cedrus/cedrus_regs.h | 91 ++-
drivers/staging/media/sunxi/cedrus/cedrus_video.c | 9 +-
8 files changed, 778 insertions(+), 2 deletions(-)
create mode 100644 drivers/staging/media/sunxi/cedrus/cedrus_h264.c
diff --git a/drivers/staging/media/sunxi/cedrus/Makefile
b/drivers/staging/media/sunxi/cedrus/Makefile
index e9dc68b7bcb6..aaf141fc58b6 100644
--- a/drivers/staging/media/sunxi/cedrus/Makefile
+++ b/drivers/staging/media/sunxi/cedrus/Makefile
@@ -1,3 +1,4 @@
obj-$(CONFIG_VIDEO_SUNXI_CEDRUS) += sunxi-cedrus.o
-sunxi-cedrus-y = cedrus.o cedrus_video.o cedrus_hw.o cedrus_dec.o
cedrus_mpeg2.o
+sunxi-cedrus-y = cedrus.o cedrus_video.o cedrus_hw.o cedrus_dec.o \
+cedrus_mpeg2.o cedrus_h264.o
diff --git a/drivers/staging/media/sunxi/cedrus/cedrus.c
b/drivers/staging/media/sunxi/cedrus/cedrus.c
index ff11cbeba205..b275607b8111 100644
--- a/drivers/staging/media/sunxi/cedrus/cedrus.c
+++ b/drivers/staging/media/sunxi/cedrus/cedrus.c
@@ -40,6 +40,36 @@ static const struct cedrus_control cedrus_controls[] = {
.codec = CEDRUS_CODEC_MPEG2,
.required = false,
},
+ {
+ .id = V4L2_CID_MPEG_VIDEO_H264_DECODE_PARAMS,
+ .elem_size = sizeof(struct v4l2_ctrl_h264_decode_param),
+ .codec = CEDRUS_CODEC_H264,
+ .required = true,
+ },
+ {
+ .id = V4L2_CID_MPEG_VIDEO_H264_SLICE_PARAMS,
+ .elem_size = sizeof(struct v4l2_ctrl_h264_slice_param),
+ .codec = CEDRUS_CODEC_H264,
+ .required = true,
+ },
+ {
+ .id = V4L2_CID_MPEG_VIDEO_H264_SPS,
+ .elem_size = sizeof(struct v4l2_ctrl_h264_sps),
+ .codec = CEDRUS_CODEC_H264,
+ .required = true,
+ },
+ {
+ .id = V4L2_CID_MPEG_VIDEO_H264_PPS,
+ .elem_size = sizeof(struct v4l2_ctrl_h264_pps),
+ .codec = CEDRUS_CODEC_H264,
+ .required = true,
+ },
+ {
+ .id = V4L2_CID_MPEG_VIDEO_H264_SCALING_MATRIX,
+ .elem_size = sizeof(struct v4l2_ctrl_h264_scaling_matrix),
+ .codec = CEDRUS_CODEC_H264,
+ .required = true,
+ },
};
#define CEDRUS_CONTROLS_COUNT ARRAY_SIZE(cedrus_controls)
@@ -278,6 +308,7 @@ static int cedrus_probe(struct platform_device *pdev)
}
dev->dec_ops[CEDRUS_CODEC_MPEG2] = &cedrus_dec_ops_mpeg2;
+ dev->dec_ops[CEDRUS_CODEC_H264] = &cedrus_dec_ops_h264;
mutex_init(&dev->dev_mutex);
diff --git a/drivers/staging/media/sunxi/cedrus/cedrus.h
b/drivers/staging/media/sunxi/cedrus/cedrus.h
index 4aedd24a9848..8c64f9a27e9d 100644
--- a/drivers/staging/media/sunxi/cedrus/cedrus.h
+++ b/drivers/staging/media/sunxi/cedrus/cedrus.h
@@ -30,7 +30,7 @@
enum cedrus_codec {
CEDRUS_CODEC_MPEG2,
-
+ CEDRUS_CODEC_H264,
CEDRUS_CODEC_LAST,
};
@@ -40,6 +40,12 @@ enum cedrus_irq_status {
CEDRUS_IRQ_OK,
};
+enum cedrus_h264_pic_type {
+ CEDRUS_H264_PIC_TYPE_FRAME = 0,
+ CEDRUS_H264_PIC_TYPE_FIELD,
+ CEDRUS_H264_PIC_TYPE_MBAFF,
+};
+
struct cedrus_control {
u32 id;
u32 elem_size;
@@ -47,6 +53,14 @@ struct cedrus_control {
unsigned char required:1;
};
+struct cedrus_h264_run {
+ const struct v4l2_ctrl_h264_decode_param*decode_param;
+ const struct v4l2_ctrl_h264_pps *pps;
+ const struct v4l2_ctrl_h264_scaling_matrix *scaling_matrix;
+ const struct v4l2_ctrl_h264_slice_param *slice_param;
+ const struct v4l2_ctrl_h264_sps *sps;
+};
+
struct cedrus_mpeg2_run {
const struct v4l2_ctrl_mpeg2_slice_params *slice_params;
const struct v4l2_ctrl_mpeg2_quantization *quantization;
@@ -57,12 +71,20 @@ struct cedrus_run {
struct vb2_v4l2_buffer *dst;
union {
+ struct cedrus_h264_run h264;
struct cedrus_mpeg2_run mpeg2;
};
};
struct cedrus_buffer {
s
[PATCH v3 1/2] media: uapi: Add H264 low-level decoder API compound controls.
From: Pawel Osciak Stateless video codecs will require both the H264 metadata and slices in order to be able to decode frames. This introduces the definitions for a new pixel format for H264 slices that have been parsed, as well as the structures used to pass the metadata from the userspace to the kernel. Co-Developed-by: Maxime Ripard Signed-off-by: Pawel Osciak Signed-off-by: Guenter Roeck Signed-off-by: Maxime Ripard --- Documentation/media/uapi/v4l/biblio.rst| 9 +- Documentation/media/uapi/v4l/extended-controls.rst | 530 ++- Documentation/media/uapi/v4l/pixfmt-compressed.rst | 20 +- Documentation/media/uapi/v4l/vidioc-queryctrl.rst | 30 +- Documentation/media/videodev2.h.rst.exceptions | 5 +- drivers/media/v4l2-core/v4l2-ctrls.c | 42 +- drivers/media/v4l2-core/v4l2-ioctl.c | 1 +- include/media/h264-ctrls.h | 179 +- include/media/v4l2-ctrls.h | 13 +- include/uapi/linux/videodev2.h | 1 +- 10 files changed, 829 insertions(+), 1 deletion(-) create mode 100644 include/media/h264-ctrls.h diff --git a/Documentation/media/uapi/v4l/biblio.rst b/Documentation/media/uapi/v4l/biblio.rst index ec33768c055e..3fc3f7ff338a 100644 --- a/Documentation/media/uapi/v4l/biblio.rst +++ b/Documentation/media/uapi/v4l/biblio.rst @@ -122,6 +122,15 @@ ITU BT.1119 :author:International Telecommunication Union (http://www.itu.ch) +.. _h264: + +ITU H.264 += + +:title: ITU-T Recommendation H.264 "Advanced Video Coding for Generic Audiovisual Services" + +:author:International Telecommunication Union (http://www.itu.ch) + .. _jfif: JFIF diff --git a/Documentation/media/uapi/v4l/extended-controls.rst b/Documentation/media/uapi/v4l/extended-controls.rst index af4273aa5e85..7e306aa0d0a6 100644 --- a/Documentation/media/uapi/v4l/extended-controls.rst +++ b/Documentation/media/uapi/v4l/extended-controls.rst @@ -1703,6 +1703,536 @@ enum v4l2_mpeg_video_h264_hierarchical_coding_type - non-intra-coded frames, in zigzag scanning order. Only relevant for non-4:2:0 YUV formats. +.. _v4l2-mpeg-h264: + +``V4L2_CID_MPEG_VIDEO_H264_SPS (struct)`` +Specifies the sequence parameter set (as extracted from the +bitstream) for the associated H264 slice data. This includes the +necessary parameters for configuring a stateless hardware decoding +pipeline for H264. The bitstream parameters are defined according +to :ref:`h264`. Unless there's a specific comment, refer to the +specification for the documentation of these fields, section 7.4.2.1.1 +"Sequence Parameter Set Data Semantics". + +.. note:: + + This compound control is not yet part of the public kernel API and + it is expected to change. + +.. c:type:: v4l2_ctrl_h264_sps + +.. cssclass:: longtable + +.. flat-table:: struct v4l2_ctrl_h264_sps +:header-rows: 0 +:stub-columns: 0 +:widths: 1 1 2 + +* - __u8 + - ``profile_idc`` + - +* - __u8 + - ``constraint_set_flags`` + - See :ref:`Sequence Parameter Set Constraints Set Flags ` +* - __u8 + - ``level_idc`` + - +* - __u8 + - ``seq_parameter_set_id`` + - +* - __u8 + - ``chroma_format_idc`` + - +* - __u8 + - ``bit_depth_luma_minus8`` + - +* - __u8 + - ``bit_depth_chroma_minus8`` + - +* - __u8 + - ``log2_max_frame_num_minus4`` + - +* - __u8 + - ``pic_order_cnt_type`` + - +* - __u8 + - ``log2_max_pic_order_cnt_lsb_minus4`` + - +* - __u8 + - ``max_num_ref_frames`` + - +* - __u8 + - ``num_ref_frames_in_pic_order_cnt_cycle`` + - +* - __s32 + - ``offset_for_ref_frame[255]`` + - +* - __s32 + - ``offset_for_non_ref_pic`` + - +* - __s32 + - ``offset_for_top_to_bottom_field`` + - +* - __u16 + - ``pic_width_in_mbs_minus1`` + - +* - __u16 + - ``pic_height_in_map_units_minus1`` + - +* - __u32 + - ``flags`` + - See :ref:`Sequence Parameter Set Flags ` + +.. _h264_sps_constraints_set_flags: + +``Sequence Parameter Set Constraints Set Flags`` + +.. cssclass:: longtable + +.. flat-table:: +:header-rows: 0 +:stub-columns: 0 +:widths: 1 1 2 + +* - ``V4L2_H264_SPS_CONSTRAINT_SET0_FLAG`` + - 0x0001 + - +* - ``V4L2_H264_SPS_CONSTRAINT_SET1_FLAG`` + - 0x0002 + - +* - ``V4L2_H264_SPS_CONSTRAINT_SET2_FLAG`` + - 0x0004 + - +* - ``V4L2_H264_SPS_CONSTRAINT_SET3_FLAG`` + - 0x0008 + - +* - ``V4L2_H264_SPS_CONSTRAINT_SET4_FLAG`` + - 0x0010 + - +* - ``V4L2_H264_SPS_CONSTRAINT_SET5_FLAG`` + - 0x0020 + - + +.. _h264_sps_flags: + +``Sequence Parameter Set Flags`` + +.. cssclass:: longtable + +.. flat-table:: +:h
Re: [PATCH v2 3/5] media: sunxi: Add A10 CSI driver
Hi Ezequiel,
On Wed, Feb 06, 2019 at 07:59:43PM -0300, Ezequiel Garcia wrote:
> > + csi->isp_clk = devm_clk_get(&pdev->dev, "isp");
> > + if (IS_ERR(csi->isp_clk)) {
> > + dev_err(&pdev->dev, "Couldn't get our ISP clock\n");
> > + return PTR_ERR(csi->isp_clk);
> > + }
> > +
> > + csi->mod_clk = devm_clk_get(&pdev->dev, "mod");
> > + if (IS_ERR(csi->mod_clk)) {
> > + dev_err(&pdev->dev, "Couldn't get our mod clock\n");
> > + return PTR_ERR(csi->mod_clk);
> > + }
> > +
> > + csi->ram_clk = devm_clk_get(&pdev->dev, "ram");
> > + if (IS_ERR(csi->ram_clk)) {
> > + dev_err(&pdev->dev, "Couldn't get our ram clock\n");
> > + return PTR_ERR(csi->ram_clk);
> > + }
> > +
>
> Minor comment: perhaps you can take advantage
> of the clock bulk API and simplify the clock management.
Our clocks have usually very different usages for each IP (the RAM
controls the DMA side of the IP, the mod one controls the "logic" part
of it, the bus one the register, etc.) so they needed to be handled
quite differently. I'd rather stick with the current API.
Thanks!
Maxime
--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
signature.asc
Description: PGP signature
Re: [PATCH v5 0/9] phy: Add configuration interface for MIPI D-PHY devices
Hi Kishon, On Wed, Feb 06, 2019 at 06:00:19PM +0530, Kishon Vijay Abraham I wrote: > On 06/02/19 5:55 PM, Maxime Ripard wrote: > > On Wed, Feb 06, 2019 at 05:43:12PM +0530, Kishon Vijay Abraham I wrote: > >> On 05/02/19 2:16 PM, Daniel Vetter wrote: > >>> On Mon, Feb 04, 2019 at 03:33:31PM +0530, Kishon Vijay Abraham I wrote: > >>>> > >>>> > >>>> On 21/01/19 9:15 PM, Maxime Ripard wrote: > >>>>> Hi, > >>>>> > >>>>> Here is a set of patches to allow the phy framework consumers to test > >>>>> and > >>>>> apply runtime configurations. > >>>>> > >>>>> This is needed to support more phy classes that require tuning based on > >>>>> parameters depending on the current use case of the device, in addition > >>>>> to > >>>>> the power state management already provided by the current functions. > >>>>> > >>>>> A first test bed for that API are the MIPI D-PHY devices. There's a > >>>>> number > >>>>> of solutions that have been used so far to support these phy, most of > >>>>> the > >>>>> time being an ad-hoc driver in the consumer. > >>>>> > >>>>> That approach has a big shortcoming though, which is that this is quite > >>>>> difficult to deal with consumers integrated with multiple variants of > >>>>> phy, > >>>>> of multiple consumers integrated with the same phy. > >>>>> > >>>>> The latter case can be found in the Cadence DSI bridge, and the CSI > >>>>> transceiver and receivers. All of them are integrated with the same > >>>>> phy, or > >>>>> can be integrated with different phy, depending on the implementation. > >>>>> > >>>>> I've looked at all the MIPI DSI drivers I could find, and gathered all > >>>>> the > >>>>> parameters I could find. The interface should be complete, and most of > >>>>> the > >>>>> drivers can be converted in the future. The current set converts two of > >>>>> them: the above mentionned Cadence DSI driver so that the v4l2 drivers > >>>>> can > >>>>> use them, and the Allwinner MIPI-DSI driver. > >>>> > >>>> Can the PHY changes go independently of the consumer drivers? or else > >>>> I'll need > >>>> ACKs from the GPU MAINTAINER. > >>> > >>> Maxime is a gpu maintainer, so you're all good :-) > >> > >> cool.. I've merged all the patches except drm/bridge. > >> > >> Please see if everything looks okay once it shows up in phy -next (give a > >> day) > > > > Thanks! > > > > If possible (and if that's still an option), it would be better if the > > sun6i related patches (patches 4 and 5) would go through the DRM tree > > (with your Acked-by of course). > > > > We have a number of patches in flight that have a decent chance to > > conflict with patch 4. > > Sure. Dropped patches 4 and 5 from my tree. Thanks! I've pushed the rest into drm-misc. Maxime -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com signature.asc Description: PGP signature
Re: [PATCH v5 6/9] drm/bridge: cdns: Separate DSI and D-PHY configuration
On Thu, Feb 07, 2019 at 09:44:46AM +0100, Paul Kocialkowski wrote: > Hi, > > On Mon, 2019-01-21 at 16:45 +0100, Maxime Ripard wrote: > > The current configuration of the DSI bridge and its associated D-PHY is > > intertwined. In order to ease the future conversion to the phy framework > > for the D-PHY part, let's split the configuration in two. > > See below about a silly mistake when refactoring. Looks good otherwise, > so with that fixed: > > Reviewed-by: Paul Kocialkowski I've fixed it while applying, thanks! Maxime -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com signature.asc Description: PGP signature
Re: [PATCH v2 3/5] media: sunxi: Add A10 CSI driver
Hi Sakari,
Thanks for your review, I have a few questions though, and the rest
will be addressed in the next version.
On Tue, Jan 29, 2019 at 02:39:49PM +0200, Sakari Ailus wrote:
> > +static int csi_notify_complete(struct v4l2_async_notifier *notifier)
> > +{
> > + struct sun4i_csi *csi = container_of(notifier, struct sun4i_csi,
> > +notifier);
> > + int ret;
> > +
> > + ret = v4l2_device_register_subdev_nodes(&csi->v4l);
> > + if (ret < 0)
> > + return ret;
> > +
> > + ret = sun4i_csi_v4l2_register(csi);
> > + if (ret < 0)
> > + return ret;
> > +
> > + return media_create_pad_link(&csi->src_subdev->entity, csi->src_pad,
> > +&csi->vdev.entity, 0,
> > +MEDIA_LNK_FL_ENABLED |
> > +MEDIA_LNK_FL_IMMUTABLE);
>
> This appears to create a link directly from the sensor entity to the video
> device entity. Is that intentional? I'd expect to see a CSI-2 receiver
> sub-device as well, which I don't see being created by the driver.
>
> This is indeed a novel proposal. I have some concerns though.
>
> The user doesn't have access to the configured media bus format (reflecting
> the format on the CSI-2 bus on receiver's side). It's thus difficult to
> figure out whether the V4L2 pixel format configured on the video node
> matches what the sensor outputs. Admittedly, we don't have a perfect
> solution to that whenever the DMA hardware supports multiple V4L2 pixel
> formats on a single media bus format. We might need to have a different
> solution for this one, should it be without that receiver sub-device.
>
> Could you add the CSI-2 receiver sub-device, please?
Even though the name of the controller is *very* confusing, this isn't
a MIPI-CSI receiver, but a parallel one that supports RGB and BT656
buses.
> > + csi->pad.flags = MEDIA_PAD_FL_SINK | MEDIA_PAD_FL_MUST_CONNECT;
>
> Could you make it IMMUTABLE and ENABLED? If there is no need to disable it,
> that is.
The link is already created with those flags, and as far as I know it
doesn't exist for the pads
> > +static int csi_release(struct file *file)
> > +{
> > + struct sun4i_csi *csi = video_drvdata(file);
> > + int ret;
> > +
> > + mutex_lock(&csi->lock);
> > +
> > + ret = v4l2_fh_release(file);
>
> v4l2_fh_release() always returns 0. I guess it could be changed to return
> void. The reason it has int is that it could be used as the release
> callback as such.
>
> > + v4l2_pipeline_pm_use(&csi->vdev.entity, 0);
> > + pm_runtime_put(csi->dev);
> > +
> > + mutex_unlock(&csi->lock);
> > +
> > + return ret;
> > +}
Do you want me to change the construct then?
Thanks!
Maxime
--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
Re: [PATCH v5 0/9] phy: Add configuration interface for MIPI D-PHY devices
Hi Kishon, On Wed, Feb 06, 2019 at 05:43:12PM +0530, Kishon Vijay Abraham I wrote: > On 05/02/19 2:16 PM, Daniel Vetter wrote: > > On Mon, Feb 04, 2019 at 03:33:31PM +0530, Kishon Vijay Abraham I wrote: > >> > >> > >> On 21/01/19 9:15 PM, Maxime Ripard wrote: > >>> Hi, > >>> > >>> Here is a set of patches to allow the phy framework consumers to test and > >>> apply runtime configurations. > >>> > >>> This is needed to support more phy classes that require tuning based on > >>> parameters depending on the current use case of the device, in addition to > >>> the power state management already provided by the current functions. > >>> > >>> A first test bed for that API are the MIPI D-PHY devices. There's a number > >>> of solutions that have been used so far to support these phy, most of the > >>> time being an ad-hoc driver in the consumer. > >>> > >>> That approach has a big shortcoming though, which is that this is quite > >>> difficult to deal with consumers integrated with multiple variants of phy, > >>> of multiple consumers integrated with the same phy. > >>> > >>> The latter case can be found in the Cadence DSI bridge, and the CSI > >>> transceiver and receivers. All of them are integrated with the same phy, > >>> or > >>> can be integrated with different phy, depending on the implementation. > >>> > >>> I've looked at all the MIPI DSI drivers I could find, and gathered all the > >>> parameters I could find. The interface should be complete, and most of the > >>> drivers can be converted in the future. The current set converts two of > >>> them: the above mentionned Cadence DSI driver so that the v4l2 drivers can > >>> use them, and the Allwinner MIPI-DSI driver. > >> > >> Can the PHY changes go independently of the consumer drivers? or else I'll > >> need > >> ACKs from the GPU MAINTAINER. > > > > Maxime is a gpu maintainer, so you're all good :-) > > cool.. I've merged all the patches except drm/bridge. > > Please see if everything looks okay once it shows up in phy -next (give a day) Thanks! If possible (and if that's still an option), it would be better if the sun6i related patches (patches 4 and 5) would go through the DRM tree (with your Acked-by of course). We have a number of patches in flight that have a decent chance to conflict with patch 4. Maxime -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com signature.asc Description: PGP signature
Re: [PATCH v5 0/9] phy: Add configuration interface for MIPI D-PHY devices
Hi Kishon, On Mon, Feb 04, 2019 at 03:33:31PM +0530, Kishon Vijay Abraham I wrote: > On 21/01/19 9:15 PM, Maxime Ripard wrote: > > Here is a set of patches to allow the phy framework consumers to test and > > apply runtime configurations. > > > > This is needed to support more phy classes that require tuning based on > > parameters depending on the current use case of the device, in addition to > > the power state management already provided by the current functions. > > > > A first test bed for that API are the MIPI D-PHY devices. There's a number > > of solutions that have been used so far to support these phy, most of the > > time being an ad-hoc driver in the consumer. > > > > That approach has a big shortcoming though, which is that this is quite > > difficult to deal with consumers integrated with multiple variants of phy, > > of multiple consumers integrated with the same phy. > > > > The latter case can be found in the Cadence DSI bridge, and the CSI > > transceiver and receivers. All of them are integrated with the same phy, or > > can be integrated with different phy, depending on the implementation. > > > > I've looked at all the MIPI DSI drivers I could find, and gathered all the > > parameters I could find. The interface should be complete, and most of the > > drivers can be converted in the future. The current set converts two of > > them: the above mentionned Cadence DSI driver so that the v4l2 drivers can > > use them, and the Allwinner MIPI-DSI driver. > > Can the PHY changes go independently of the consumer drivers? or else I'll > need > ACKs from the GPU MAINTAINER. At least for the Allwinner driver, they can go through through the drm-misc tree. Since we have a number of patches in flight for that driver, it would even be easier to handle there. For the cadence driver, since it doesn't really work on any system but simulators for now, I guess the wakeup regression isn't super important either. So I'd say we can have the phy related patches go through your tree, and the other through drm-misc. Would that work for you? Maxime -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com signature.asc Description: PGP signature
Re: [PATCH 2/3] media: sun6i: Add support for RGB565 formats
On Mon, Feb 04, 2019 at 12:03:57AM +0800, Chen-Yu Tsai wrote: > The CSI controller can take raw data from the data bus and output RGB565 > format. The controller does not distinguish between RGB565 LE and BE. > Instead this is determined by the media bus format, i.e. the format or > order the sensor is sending data in. > > Signed-off-by: Chen-Yu Tsai Acked-by: Maxime Ripard Maxime -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com signature.asc Description: PGP signature
Re: [PATCH 1/3] media: sun6i: Fix CSI regmap's max_register
On Mon, Feb 04, 2019 at 12:03:56AM +0800, Chen-Yu Tsai wrote:
> max_register is currently set to 0x1000. This is beyond the mapped
> address range of the hardware, so attempts to dump the regmap from
> debugfs would trigger a kernel exception.
>
> Furthermore, the useful registers only occupy a small section at the
> beginning of the full range. Change the value to 0x9c, the last known
> register on the V3s and H3.
>
> On the A31, the register range is extended to support additional
> capture channels. Since this is not yet supported, ignore it for now.
>
> Fixes: 5cc7522d8965 ("media: sun6i: Add support for Allwinner CSI V3s")
> Cc:
> Signed-off-by: Chen-Yu Tsai
Acked-by: Maxime Ripard
Maxime
--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
signature.asc
Description: PGP signature
Re: [linux-sunxi] [PATCH v2 1/2] media: v4l: Add definitions for the HEVC slice format and controls
you see another > > > > > possibility? > > > > > > > > Is reconstructing the bitstream so bad? The v4l2 controls provide a > > > > generic interface to an encoded format which the driver needs to > > > > convert into a sequence that the hardware can understand. Typically > > > > this is done by populating hardware-specific structures. Can't we > > > > consider that in this specific instance, the hardware-specific > > > > structure just happens to be identical to the original bitstream > > > > format? > > > > > > At maximum allowed bitrate for let's say HEVC (940MB/s iirc), yes, it > > > would be really really bad. In GStreamer project we have discussed for > > > a while (but have never done anything about) adding the ability through > > > a bitmask to select which part of the stream need to be parsed, as > > > parsing itself was causing some overhead. Maybe similar thing applies, > > > though as per our new design, it's the fourcc that dictate the driver > > > behaviour, we'd need yet another fourcc for drivers that wants the full > > > bitstream (which seems odd if you have already parsed everything, I > > > think this need some clarification). > > > > Note that I am not proposing to rebuild the *entire* bitstream > > in-kernel. What I am saying is that if the hardware interprets some > > structures (like SPS/PPS) in their raw format, this raw format could > > be reconstructed from the structures passed by userspace at negligible > > cost. Such manipulation would only happen on a small amount of data. > > > > Exposing finer-grained driver requirements through a bitmask may > > deserve more exploring. Maybe we could end with a spectrum of > > capabilities that would allow us to cover the range from fully > > stateless to fully stateful IPs more smoothly. Right now we have two > > specifications that only consider the extremes of that range. > > I gave it a bit more thought and if we combine what Nicolas suggested > about the bitmask control with the userspace providing the full > bitstream in the OUTPUT buffers, split into some logical units and > "tagged" with their type (e.g. SPS, PPS, slice, etc.), we could > potentially get an interface that would work for any kind of decoder I > can think of, actually eliminating the boundary between stateful and > stateless decoders. > > For example, a fully stateful decoder would have the bitmask control > set to 0 and accept data from all the OUTPUT buffers as they come. A > decoder that doesn't do any parsing on its own would have all the > valid bits in the bitmask set and ignore the data in OUTPUT buffers > tagged as any kind of metadata. And then, we could have any cases in > between, including stateful decoders which just can't parse the stream > on their own, but still manage anything else themselves, or stateless > ones which can parse parts of the stream, like the rk3399 vdec can > parse the H.264 slice headers on its own. > > That could potentially let us completely eliminate the distinction > between the stateful and stateless interfaces and just have one that > covers both. > > Thoughts? If we have to provide the whole bitstream in the buffers, then it entirely breaks the sole software stack we have running and working currently, for a use case and a driver that hasn't seen a single line of code. Seriously, this is a *private* API that we did that way so that we can change it and only make it public. Why not do just that? Maxime -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com signature.asc Description: PGP signature
Re: [PATCH 5/6] arm64: dts: allwinner: h6: Add support for the SRAM C1 section
On Mon, Jan 28, 2019 at 09:55:03PM +0100, Jernej Skrabec wrote: > Add a node for H6 SRAM C1 section. > > Manual calls it VE SRAM, but for consistency with older SoCs, SRAM C1 > name is used. > > Signed-off-by: Jernej Skrabec Applied, thanks! Maxime -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com signature.asc Description: PGP signature
Re: [PATCH 4/6] dt-bindings: sram: sunxi: Add compatible for the H6 SRAM C1
On Mon, Jan 28, 2019 at 09:55:02PM +0100, Jernej Skrabec wrote: > This introduces a new compatible for the H6 SRAM C1 section, that is > compatible with the SRAM C1 section as found on the A10. > > Signed-off-by: Jernej Skrabec Applied, thanks! Maxime -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com signature.asc Description: PGP signature
Re: [PATCH 3/6] media: cedrus: Add support for H6
On Mon, Jan 28, 2019 at 09:55:01PM +0100, Jernej Skrabec wrote: > H6 has improved VPU. It supports 10-bit HEVC decoding and AFBC output > format for HEVC. > > Signed-off-by: Jernej Skrabec Acked-by: Maxime Ripard Maxime -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com signature.asc Description: PGP signature
Re: [PATCH 2/6] media: cedrus: Add a quirk for not setting DMA offset
On Mon, Jan 28, 2019 at 09:55:00PM +0100, Jernej Skrabec wrote: > H6 VPU doesn't work if DMA offset is set. > > Add a quirk for it. > > Signed-off-by: Jernej Skrabec Acked-by: Maxime Ripard Maxime -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com signature.asc Description: PGP signature
Re: [PATCH 1/6] dt-bindings: media: cedrus: Add H6 compatible
On Mon, Jan 28, 2019 at 09:54:59PM +0100, Jernej Skrabec wrote: > This adds a compatible for H6. H6 VPU supports 10-bit HEVC decoding and > additional AFBC output format for HEVC. > > Signed-off-by: Jernej Skrabec Acked-by: Maxime Ripard Maxime -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com signature.asc Description: PGP signature
Re: [PATCH] media: ov5640: Fix set 15fps regression
On Mon, Jan 28, 2019 at 09:33:04AM +0100, Jacopo Mondi wrote:
> Hi everyone,
>
> On Mon, Jan 28, 2019 at 01:20:37PM +0530, Jagan Teki wrote:
> > On Fri, Jan 25, 2019 at 9:10 PM Maxime Ripard
> > wrote:
> > >
> > > On Thu, Jan 24, 2019 at 11:28:01PM +0530, Jagan Teki wrote:
> > > > The ov5640_try_frame_interval operation updates the FPS as per user
> > > > input based on default ov5640_frame_rate, OV5640_30_FPS which is failed
> > > > to update when user trigger 15fps.
> > > >
> > > > So, initialize the default ov5640_frame_rate to OV5640_15_FPS so-that
> > > > it can satisfy to update all fps.
> > > >
> > > > Fixes: 5a3ad937bc78 ("media: ov5640: Make the return rate type more
> > > > explicit")
> > > > Signed-off-by: Jagan Teki
> > >
> > > I'm pretty sure I tested this and it was working fine. You're
> > > mentionning a regression, but what regression is there exactly (ie,
> > > what was working before that commit that doesn't work anymore?). What
> > > tools/commands are you using to see this behaviour?
> >
>
> I think Jagan's right, here.
>
> For the 15FPS use case, the below condition in the for loop of
> ov5640_try_frame_interval() never gets satisfied (0 < 0 ?) and 'rate' retains
> its initial value of 30FPS.
>
> if (abs(curr_fps - fps) < abs(best_fps - fps)) {
> best_fps = curr_fps;
> rate = i;
> }
>
> To make more clear what's happening, I would initialize 'rate' just
> before 'best_fps' before the for loop. Anyway, please add:
> Acked-by: Jacopo Mondi
>
> Maxime, does this make sense to you?
With that explanation in the commit log, yes.
Maxime
--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
signature.asc
Description: PGP signature
Re: [linux-sunxi] [PATCH v2 1/2] media: v4l: Add definitions for the HEVC slice format and controls
On Tue, Jan 29, 2019 at 04:44:35PM +0900, Alexandre Courbot wrote: > On Fri, Jan 25, 2019 at 10:04 PM Paul Kocialkowski > > On Thu, 2019-01-24 at 20:23 +0800, Ayaka wrote: > > > > > > Sent from my iPad > > > > > > > On Jan 24, 2019, at 6:27 PM, Paul Kocialkowski > > > > wrote: > > > > > > > > Hi, > > > > > > > > > On Thu, 2019-01-10 at 21:32 +0800, ayaka wrote: > > > > > I forget a important thing, for the rkvdec and rk hevc decoder, it > > > > > would > > > > > requests cabac table, scaling list, picture parameter set and > > > > > reference > > > > > picture storing in one or various of DMA buffers. I am not talking > > > > > about > > > > > the data been parsed, the decoder would requests a raw data. > > > > > > > > > > For the pps and rps, it is possible to reuse the slice header, just > > > > > let > > > > > the decoder know the offset from the bitstream bufer, I would suggest > > > > > to > > > > > add three properties(with sps) for them. But I think we need a method > > > > > to > > > > > mark a OUTPUT side buffer for those aux data. > > > > > > > > I'm quite confused about the hardware implementation then. From what > > > > you're saying, it seems that it takes the raw bitstream elements rather > > > > than parsed elements. Is it really a stateless implementation? > > > > > > > > The stateless implementation was designed with the idea that only the > > > > raw slice data should be passed in bitstream form to the decoder. For > > > > H.264, it seems that some decoders also need the slice header in raw > > > > bitstream form (because they take the full slice NAL unit), see the > > > > discussions in this thread: > > > > media: docs-rst: Document m2m stateless video decoder interface > > > > > > Stateless just mean it won’t track the previous result, but I don’t > > > think you can define what a date the hardware would need. Even you > > > just build a dpb for the decoder, it is still stateless, but parsing > > > less or more data from the bitstream doesn’t stop a decoder become a > > > stateless decoder. > > > > Yes fair enough, the format in which the hardware decoder takes the > > bitstream parameters does not make it stateless or stateful per-se. > > It's just that stateless decoders should have no particular reason for > > parsing the bitstream on their own since the hardware can be designed > > with registers for each relevant bitstream element to configure the > > decoding pipeline. That's how GPU-based decoder implementations are > > implemented (VAAPI/VDPAU/NVDEC, etc). > > > > So the format we have agreed on so far for the stateless interface is > > to pass parsed elements via v4l2 control structures. > > > > If the hardware can only work by parsing the bitstream itself, I'm not > > sure what the best solution would be. Reconstructing the bitstream in > > the kernel is a pretty bad option, but so is parsing in the kernel or > > having the data both in parsed and raw forms. Do you see another > > possibility? > > Is reconstructing the bitstream so bad? The v4l2 controls provide a > generic interface to an encoded format which the driver needs to > convert into a sequence that the hardware can understand. Typically > this is done by populating hardware-specific structures. Can't we > consider that in this specific instance, the hardware-specific > structure just happens to be identical to the original bitstream > format? > > I agree that this is not strictly optimal for that particular > hardware, but such is the cost of abstractions, and in this specific > case I don't believe the cost would be particularly high? I mean, that argument can be made for the rockchip driver as well. If reconstructing the bitstream is something we can do, and if we don't care about being suboptimal for one particular hardware, then why the rockchip driver doesn't just recreate the bitstream from that API? After all, this is just a hardware specific header that happens to be identical to the original bitstream format Maxime -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
[PATCH v2 2/5] media: sunxi: Refactor the Makefile and Kconfig
The Makefile and Kconfig for the sun6i CSI driver are included in the main Makefile / KConfig file. Since we're going to add a new CSI driver for an older chip, and the Cedrus driver eventually, it makes more sense to put those in our directory. Signed-off-by: Maxime Ripard --- drivers/media/platform/Kconfig| 2 +- drivers/media/platform/Makefile | 2 +- drivers/media/platform/sunxi/Kconfig | 1 + drivers/media/platform/sunxi/Makefile | 1 + 4 files changed, 4 insertions(+), 2 deletions(-) create mode 100644 drivers/media/platform/sunxi/Kconfig create mode 100644 drivers/media/platform/sunxi/Makefile diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig index a505e9f5a1e2..70d5a9ba2863 100644 --- a/drivers/media/platform/Kconfig +++ b/drivers/media/platform/Kconfig @@ -147,7 +147,7 @@ source "drivers/media/platform/am437x/Kconfig" source "drivers/media/platform/xilinx/Kconfig" source "drivers/media/platform/rcar-vin/Kconfig" source "drivers/media/platform/atmel/Kconfig" -source "drivers/media/platform/sunxi/sun6i-csi/Kconfig" +source "drivers/media/platform/sunxi/Kconfig" config VIDEO_TI_CAL tristate "TI CAL (Camera Adaptation Layer) driver" diff --git a/drivers/media/platform/Makefile b/drivers/media/platform/Makefile index e6deb2597738..43841df5c57d 100644 --- a/drivers/media/platform/Makefile +++ b/drivers/media/platform/Makefile @@ -102,4 +102,4 @@ obj-y += meson/ obj-y += cros-ec-cec/ -obj-$(CONFIG_VIDEO_SUN6I_CSI) += sunxi/sun6i-csi/ +obj-y += sunxi/ diff --git a/drivers/media/platform/sunxi/Kconfig b/drivers/media/platform/sunxi/Kconfig new file mode 100644 index ..1b6e89cb78b2 --- /dev/null +++ b/drivers/media/platform/sunxi/Kconfig @@ -0,0 +1 @@ +source "drivers/media/platform/sunxi/sun6i-csi/Kconfig" diff --git a/drivers/media/platform/sunxi/Makefile b/drivers/media/platform/sunxi/Makefile new file mode 100644 index ..8d06f98500ee --- /dev/null +++ b/drivers/media/platform/sunxi/Makefile @@ -0,0 +1 @@ +obj-y += sun6i-csi/ -- git-series 0.9.1
[PATCH v2 0/5] media: Allwinner A10 CSI support
tride 2, Field None: OK test MMAP for Format YM12, Frame Size 640x480: Stride 640, Field None: OK Total: 52, Succeeded: 52, Failed: 0, Warnings: 0 Let me know what you think, Maxime Changes from v1: - Make sure it's compliant with a much newer v4l2-compliance - Conversion of the DT bindings to a JSON schema - Drop the vendor properties and use a separate compatible instead - Fix an issue on the last frame where we would not have any buffer queued and would report an error by using a scratch buffer - Fix the warnings reported by v4l2-compliance - Rebase on top of 5.0-rc1 - Added a MAINTAINERS entry - Switched to strscpy - Fixed SPDX header Maxime Ripard (5): dt-bindings: media: Add Allwinner A10 CSI binding media: sunxi: Refactor the Makefile and Kconfig media: sunxi: Add A10 CSI driver ARM: dts: sun7i: Add CSI0 controller DO NOT MERGE: ARM: dts: bananapi: Add Camera support Documentation/devicetree/bindings/media/allwinner,sun4i-a10-csi.yaml | 73 - MAINTAINERS | 8 +- arch/arm/boot/dts/sun7i-a20-bananapi.dts | 94 - arch/arm/boot/dts/sun7i-a20.dtsi | 11 ++- drivers/media/platform/Kconfig | 2 +- drivers/media/platform/Makefile | 2 +- drivers/media/platform/sunxi/Kconfig | 2 +- drivers/media/platform/sunxi/Makefile| 2 +- drivers/media/platform/sunxi/sun4i-csi/Kconfig | 12 ++- drivers/media/platform/sunxi/sun4i-csi/Makefile | 5 +- drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.c | 261 +++- drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.h | 142 - drivers/media/platform/sunxi/sun4i-csi/sun4i_dma.c | 435 - drivers/media/platform/sunxi/sun4i-csi/sun4i_v4l2.c | 305 ++- 14 files changed, 1352 insertions(+), 2 deletions(-) create mode 100644 Documentation/devicetree/bindings/media/allwinner,sun4i-a10-csi.yaml create mode 100644 drivers/media/platform/sunxi/Kconfig create mode 100644 drivers/media/platform/sunxi/Makefile create mode 100644 drivers/media/platform/sunxi/sun4i-csi/Kconfig create mode 100644 drivers/media/platform/sunxi/sun4i-csi/Makefile create mode 100644 drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.c create mode 100644 drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.h create mode 100644 drivers/media/platform/sunxi/sun4i-csi/sun4i_dma.c create mode 100644 drivers/media/platform/sunxi/sun4i-csi/sun4i_v4l2.c base-commit: 372df08972ca8fb397f82fdc63d9669281c50aee -- git-series 0.9.1
[PATCH v2 5/5] DO NOT MERGE: ARM: dts: bananapi: Add Camera support
Signed-off-by: Maxime Ripard
---
arch/arm/boot/dts/sun7i-a20-bananapi.dts | 94 +-
1 file changed, 94 insertions(+)
diff --git a/arch/arm/boot/dts/sun7i-a20-bananapi.dts
b/arch/arm/boot/dts/sun7i-a20-bananapi.dts
index 556b1b591c5d..f5fee1f95900 100644
--- a/arch/arm/boot/dts/sun7i-a20-bananapi.dts
+++ b/arch/arm/boot/dts/sun7i-a20-bananapi.dts
@@ -54,6 +54,9 @@
compatible = "lemaker,bananapi", "allwinner,sun7i-a20";
aliases {
+ i2c0 = &i2c0;
+ i2c1 = &i2c1;
+ i2c2 = &i2c2;
serial0 = &uart0;
serial1 = &uart3;
serial2 = &uart7;
@@ -63,6 +66,41 @@
stdout-path = "serial0:115200n8";
};
+ reg_cam: cam {
+ compatible = "regulator-fixed";
+ regulator-name = "cam";
+ regulator-min-microvolt = <500>;
+ regulator-max-microvolt = <500>;
+ vin-supply = <®_vcc5v0>;
+ gpio = <&pio 7 16 GPIO_ACTIVE_HIGH>;
+ enable-active-high;
+ regulator-always-on;
+ };
+
+reg_cam_avdd: cam-avdd {
+compatible = "regulator-fixed";
+regulator-name = "cam500b-avdd";
+regulator-min-microvolt = <280>;
+regulator-max-microvolt = <280>;
+vin-supply = <®_cam>;
+};
+
+reg_cam_dovdd: cam-dovdd {
+compatible = "regulator-fixed";
+regulator-name = "cam500b-dovdd";
+regulator-min-microvolt = <180>;
+regulator-max-microvolt = <180>;
+vin-supply = <®_cam>;
+};
+
+reg_cam_dvdd: cam-dvdd {
+compatible = "regulator-fixed";
+regulator-name = "cam500b-dvdd";
+regulator-min-microvolt = <150>;
+regulator-max-microvolt = <150>;
+vin-supply = <®_cam>;
+};
+
hdmi-connector {
compatible = "hdmi-connector";
type = "a";
@@ -116,6 +154,26 @@
>;
};
+&csi0 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&csi0_pins_a>;
+ status = "okay";
+
+ port {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ csi_from_ov5640: endpoint {
+remote-endpoint = <&ov5640_to_csi>;
+bus-width = <8>;
+hsync-active = <1>; /* Active high */
+vsync-active = <0>; /* Active low */
+data-active = <1>; /* Active high */
+pclk-sample = <1>; /* Rising */
+};
+ };
+};
+
&de {
status = "okay";
};
@@ -161,6 +219,36 @@
};
};
+&i2c1 {
+ status = "okay";
+
+ camera: camera@21 {
+ compatible = "ovti,ov5640";
+ reg = <0x21>;
+clocks = <&ccu CLK_CSI0>;
+clock-names = "xclk";
+ assigned-clocks = <&ccu CLK_CSI0>;
+ assigned-clock-rates = <2400>;
+
+reset-gpios = <&pio 7 14 GPIO_ACTIVE_LOW>;
+powerdown-gpios = <&pio 7 19 GPIO_ACTIVE_HIGH>;
+AVDD-supply = <®_cam_avdd>;
+DOVDD-supply = <®_cam_dovdd>;
+DVDD-supply = <®_cam_dvdd>;
+
+port {
+ov5640_to_csi: endpoint {
+remote-endpoint = <&csi_from_ov5640>;
+bus-width = <8>;
+hsync-active = <1>; /* Active high */
+vsync-active = <0>; /* Active low */
+data-active = <1>; /* Active high */
+pclk-sample = <1>; /* Rising */
+};
+};
+ };
+};
+
&i2c2 {
status = "okay";
};
@@ -242,6 +330,12 @@
"IO-6", "IO-3", "IO-2", "IO-0", "", "", "", "",
"", "", "", "", "", "", "", "";
+ csi0_pins_a: csi_pins_a@0 {
+ pins = "PE0", "PE1", "PE2", "PE3", "PE4", "PE5",
+ "PE6", "PE7", "PE8", "PE9", "PE10", "PE11";
+ function = "csi0";
+ };
+
usb0_id_detect_pin: usb0-id-detect-pin {
pins = "PH4";
function = "gpio_in";
--
git-series 0.9.1
[PATCH v2 3/5] media: sunxi: Add A10 CSI driver
The older CSI drivers have camera capture interface different from the one
in the newer ones.
This IP is pretty simple. Some variants (one controller out of two
instances on some SoCs) have an ISP embedded, but there's no code that make
use of it, so we ignored that part for now.
Signed-off-by: Maxime Ripard
---
MAINTAINERS | 8 +-
drivers/media/platform/sunxi/Kconfig| 1 +-
drivers/media/platform/sunxi/Makefile | 1 +-
drivers/media/platform/sunxi/sun4i-csi/Kconfig | 12 +-
drivers/media/platform/sunxi/sun4i-csi/Makefile | 5 +-
drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.c | 261 -
drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.h | 142 -
drivers/media/platform/sunxi/sun4i-csi/sun4i_dma.c | 435 +-
drivers/media/platform/sunxi/sun4i-csi/sun4i_v4l2.c | 305 +-
9 files changed, 1170 insertions(+)
create mode 100644 drivers/media/platform/sunxi/sun4i-csi/Kconfig
create mode 100644 drivers/media/platform/sunxi/sun4i-csi/Makefile
create mode 100644 drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.c
create mode 100644 drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.h
create mode 100644 drivers/media/platform/sunxi/sun4i-csi/sun4i_dma.c
create mode 100644 drivers/media/platform/sunxi/sun4i-csi/sun4i_v4l2.c
diff --git a/MAINTAINERS b/MAINTAINERS
index 32d76a90..5f703ed9adb1 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1295,6 +1295,14 @@ F: drivers/pinctrl/sunxi/
F: drivers/soc/sunxi/
T: git git://git.kernel.org/pub/scm/linux/kernel/git/sunxi/linux.git
+Allwinner A10 CSI driver
+M: Maxime Ripard
+L: linux-media@vger.kernel.org
+T: git git://linuxtv.org/media_tree.git
+S: Maintained
+F: drivers/media/platform/sunxi/sun4i-csi/
+F: Documentation/devicetree/bindings/media/sun4i-csi.txt
+
ARM/Amlogic Meson SoC CLOCK FRAMEWORK
M: Neil Armstrong
M: Jerome Brunet
diff --git a/drivers/media/platform/sunxi/Kconfig
b/drivers/media/platform/sunxi/Kconfig
index 1b6e89cb78b2..71808e93ac2e 100644
--- a/drivers/media/platform/sunxi/Kconfig
+++ b/drivers/media/platform/sunxi/Kconfig
@@ -1 +1,2 @@
+source "drivers/media/platform/sunxi/sun4i-csi/Kconfig"
source "drivers/media/platform/sunxi/sun6i-csi/Kconfig"
diff --git a/drivers/media/platform/sunxi/Makefile
b/drivers/media/platform/sunxi/Makefile
index 8d06f98500ee..a05127529006 100644
--- a/drivers/media/platform/sunxi/Makefile
+++ b/drivers/media/platform/sunxi/Makefile
@@ -1 +1,2 @@
+obj-y += sun4i-csi/
obj-y += sun6i-csi/
diff --git a/drivers/media/platform/sunxi/sun4i-csi/Kconfig
b/drivers/media/platform/sunxi/sun4i-csi/Kconfig
new file mode 100644
index ..841a6f4d9c99
--- /dev/null
+++ b/drivers/media/platform/sunxi/sun4i-csi/Kconfig
@@ -0,0 +1,12 @@
+config VIDEO_SUN4I_CSI
+ tristate "Allwinner A10 CMOS Sensor Interface Support"
+ depends on VIDEO_DEV && VIDEO_V4L2 && HAS_DMA
+ depends on ARCH_SUNXI || COMPILE_TEST
+ select VIDEOBUF2_DMA_CONTIG
+ select V4L2_FWNODE
+ select V4L2_MEM2MEM_DEV
+ help
+ This is a V4L2 driver for the Allwinner A10 CSI
+
+ To compile this driver as a module, choose M here: the module
+ will be called sun4i_csi.
diff --git a/drivers/media/platform/sunxi/sun4i-csi/Makefile
b/drivers/media/platform/sunxi/sun4i-csi/Makefile
new file mode 100644
index ..7c790a57f5ee
--- /dev/null
+++ b/drivers/media/platform/sunxi/sun4i-csi/Makefile
@@ -0,0 +1,5 @@
+sun4i-csi-y += sun4i_csi.o
+sun4i-csi-y += sun4i_dma.o
+sun4i-csi-y += sun4i_v4l2.o
+
+obj-$(CONFIG_VIDEO_SUN4I_CSI) += sun4i-csi.o
diff --git a/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.c
b/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.c
new file mode 100644
index ..9b58b42c0043
--- /dev/null
+++ b/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.c
@@ -0,0 +1,261 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) 2016 NextThing Co
+ * Copyright (C) 2016-2018 Bootlin
+ *
+ * Author: Maxime Ripard
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include
+#include
+#include
+#include
+#include
+
+#include
+#include
+
+#include "sun4i_csi.h"
+
+static int csi_notify_bound(struct v4l2_async_notifier *notifier,
+ struct v4l2_subdev *subdev,
+ struct v4l2_async_subdev *asd)
+{
+ struct sun4i_csi *csi = container_of(notifier, struct sun4i_csi,
+notifier);
+
+ csi->src_subdev = subdev;
+ csi->src_pad = media_entity_get_fwnode_pad(&subdev->entity,
+ subdev->fwnode,
+ MEDIA_PAD_FL_SOURCE);
+ if (cs
[PATCH v2 1/5] dt-bindings: media: Add Allwinner A10 CSI binding
The Allwinner A10 CMOS Sensor Interface is a camera capture interface also used in later (A10s, A13, A20, R8 and GR8) SoCs. On some SoCs, like the A10, there's multiple instances of that controller, with one instance supporting more channels and having an ISP. Signed-off-by: Maxime Ripard --- Documentation/devicetree/bindings/media/allwinner,sun4i-a10-csi.yaml | 73 - 1 file changed, 73 insertions(+) create mode 100644 Documentation/devicetree/bindings/media/allwinner,sun4i-a10-csi.yaml diff --git a/Documentation/devicetree/bindings/media/allwinner,sun4i-a10-csi.yaml b/Documentation/devicetree/bindings/media/allwinner,sun4i-a10-csi.yaml new file mode 100644 index ..f550fefa074f --- /dev/null +++ b/Documentation/devicetree/bindings/media/allwinner,sun4i-a10-csi.yaml @@ -0,0 +1,73 @@ +# SPDX-License-Identifier: (GPL-2.0+ OR X11) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/arm/allwinner,sun4i-a10-csi.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Allwinner A10 CMOS Sensor Interface (CSI) Device Tree Bindings + +maintainers: + - Chen-Yu Tsai + - Maxime Ripard + +properties: + compatible: +oneOf: + - description: Allwinner A10 CSI0 Controller +items: +- const: allwinner,sun4i-a10-csi0 + + - description: Allwinner A20 CSI0 Controller +items: +- const: allwinner,sun7i-a20-csi0 +- const: allwinner,sun4i-a10-csi0 + + reg: +description: The base address and size of the memory-mapped region +maxItems: 1 + + interrupts: +description: The interrupt associated to this IP +maxItems: 1 + + clocks: +minItems: 4 +maxItems: 4 +items: + - description: The CSI interface clock + - description: The CSI module clock + - description: The CSI ISP clock + - description: The CSI DRAM clock + + clock-names: +minItems: 4 +maxItems: 4 +items: + - const: bus + - const: mod + - const: isp + - const: ram + + resets: +description: The reset line driver this IP +maxItems: 1 + + pinctrl-0: true + + pinctrl-names: +description: + When present, must have one state named "default" that sets up + pins for ordinary operations. +minItems: 1 +maxItems: 1 +items: + - const: default + +required: + - compatible + - reg + - interrupts + - clocks + +# The media OF-graph binding hasn't been described yet +# additionalProperties: false -- git-series 0.9.1
[PATCH v2 4/5] ARM: dts: sun7i: Add CSI0 controller
The CSI controller embedded in the A20 can be supported by our new driver.
Let's add it to our DT.
Signed-off-by: Maxime Ripard
---
arch/arm/boot/dts/sun7i-a20.dtsi | 11 +++
1 file changed, 11 insertions(+)
diff --git a/arch/arm/boot/dts/sun7i-a20.dtsi b/arch/arm/boot/dts/sun7i-a20.dtsi
index 641a8fa6d428..0eade0805fb0 100644
--- a/arch/arm/boot/dts/sun7i-a20.dtsi
+++ b/arch/arm/boot/dts/sun7i-a20.dtsi
@@ -376,6 +376,17 @@
num-cs = <1>;
};
+ csi0: csi@1c09000 {
+ compatible = "allwinner,sun7i-a20-csi0",
+"allwinner,sun4i-a10-csi0";
+ reg = <0x01c09000 0x1000>;
+ interrupts = ;
+ clocks = <&ccu CLK_AHB_CSI0>, <&ccu CLK_CSI0>,
+<&ccu CLK_CSI_SCLK>, <&ccu CLK_DRAM_CSI0>;
+ clock-names = "bus", "mod", "isp", "ram";
+ resets = <&ccu RST_CSI0>;
+ };
+
emac: ethernet@1c0b000 {
compatible = "allwinner,sun4i-a10-emac";
reg = <0x01c0b000 0x1000>;
--
git-series 0.9.1
Re: [PATCH v8 3/5] arm64: dts: allwinner: a64: Add A64 CSI controller
On Mon, Jan 28, 2019 at 02:28:45PM +0530, Jagan Teki wrote: > Add dts node details for Allwinner A64 CSI controller. > > A64 CSI has similar features as like in H3, but the CSI_SCLK > need to update it to 300MHz than default clock rate. > > Signed-off-by: Jagan Teki > Acked-by: Maxime Ripard Applied, thanks! Maxime -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com signature.asc Description: PGP signature
Re: [PATCH v8 2/5] media: sun6i: Add A64 CSI block support
On Mon, Jan 28, 2019 at 02:28:44PM +0530, Jagan Teki wrote: > CSI block in Allwinner A64 has similar features as like in H3, > but the default CSI_SCLK rate cannot work properly to drive the > connected sensor interface. > > The tested mod cock rate is 300 MHz and BSP vfe media driver is also > using the same rate. Unfortunately there is no valid information about > clock rate in manual or any other sources except the BSP driver. so more > faith on BSP code, because same has tested in mainline. > > So, add support for A64 CSI block by setting updated mod clock rate. > > Signed-off-by: Jagan Teki Acked-by: Maxime Ripard Maxime -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com signature.asc Description: PGP signature
Re: [PATCH 1/3] media: dt: bindings: sunxi-ir: Add A64 compatible
On Fri, Jan 25, 2019 at 10:49:58AM +0800, Chen-Yu Tsai wrote: > On Fri, Jan 25, 2019 at 2:57 AM Jernej Škrabec > wrote: > > > > Dne ponedeljek, 21. januar 2019 ob 10:57:57 CET je Chen-Yu Tsai napisal(a): > > > On Mon, Jan 21, 2019 at 5:50 PM Maxime Ripard > > wrote: > > > > Hi, > > > > > > > > I'm a bit late to the party, sorry for that. > > > > > > > > On Sat, Jan 12, 2019 at 09:56:11AM +0800, Chen-Yu Tsai wrote: > > > > > On Sat, Jan 12, 2019 at 1:30 AM Jernej Skrabec > > > > > > > wrote: > > > > > > A64 IR is compatible with A13, so add A64 compatible with A13 as a > > > > > > fallback. > > > > > > > > > > We ask people to add the SoC-specific compatible as a contigency, > > > > > in case things turn out to be not so "compatible". > > > > > > > > > > To be consistent with all the other SoCs and other peripherals, > > > > > unless you already spotted a "compatible" difference in the > > > > > hardware, i.e. the hardware isn't completely the same, this > > > > > patch isn't needed. On the other hand, if you did, please mention > > > > > the differences in the commit log. > > > > > > > > Even if we don't spot things, since we have the stable DT now, if we > > > > ever had that compatible in the DT from day 1, it's much easier to > > > > deal with. > > > > > > > > I'd really like to have that pattern for all the IPs even if we didn't > > > > spot any issue, since we can't really say that the datasheet are > > > > complete, and one can always make a mistake and overlook something. > > > > > > > > I'm fine with this version, and can apply it as is if we all agree. > > > > > > I'm OK with having the fallback compatible. I'm just pointing out > > > that there are and will be a whole bunch of them, and we don't need > > > to document all of them unless we are actually doing something to > > > support them. > > > > > > On the other hand, the compatible string situation for IR needs a > > > bit of cleaning up at the moment. Right now we have sun4i-a10 and > > > sun5i-a13. As Jernej pointed out, the A13's register definition is > > > different from A64 (or any other SoCs later than sun6i). So we need > > > someone with an A10s/A13 device that has IR to test it and see if > > > the driver or the manual is wrong, and we'd likely add a compatible > > > for the A20. > > > > > > Also, the earlier SoCs (A10/sun5i/A20) have IR TX capability. This > > > was lost in A31, and also all of sun8i / sun50i. So we're going to > > > need to add an A31 compatible that all later platforms would need > > > to switch to. > > > > Actually, A13 also doesn't have IR TX capability. So I still think it's best > > having A13 compatible as a fallback and not A31. Unless A31 was released > > before A13? > > No, but the A31 IR receiver has some additional bits in the FIFO control > and status registers, as well as the config register (which controls > sampling parameters). Looks like the A31 has an improved version. That > would make it backward compatible, if not for the fact that the FIFO > level bits are at a different offset, which might have been moved to > make way for the extra bits. That would make them incompatible. But > this should really be tested. > > So the fallback compatible should be the A31's, not the A13's. > > The A64's looks like the same hardware as the A31, with two extra bits: > > - CGPO: register 0x00, bit offset 8. Controls output level of > "non-existing" TX pin > > - DRQ_EN: register 0x2c, bit offset 5. Controls DRQ usage for DMA. > Not really useful as there isn't a DMA request line for > the hardware. > > Both bits are also togglable on the A31, but since actual hardware > don't support these two features, I think we can ignore them. > > So it looks like for the A64 has the same IP block as the A31, in > which case we won't need the per-SoC compatible as we've done the > work to compare them. > > Maxime, what do you think? Even though no hardware support those two features, I'd really prefer to have an A64 compatible in addition to the A31's in the DT to be future proof and being able to deal nicely with backward compatibility. But of course, the driver can only use the A31 for now. > And do you guys have any A10s/A13 hardware to test the FIFO level > bits? I don't think I have one with an IR receiver Maxime -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com signature.asc Description: PGP signature
Re: [PATCH v7 3/5] arm64: dts: allwinner: a64: Add A64 CSI controller
On Thu, Jan 24, 2019 at 11:37:34PM +0530, Jagan Teki wrote: > Add dts node details for Allwinner A64 CSI controller. > > A64 CSI has similar features as like in H3, but the CSI_SCLK > need to update it to 300MHz than default clock rate. > > Signed-off-by: Jagan Teki Acked-by: Maxime Ripard Maxime -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com signature.asc Description: PGP signature
Re: [PATCH v7 2/5] media: sun6i: Add A64 CSI block support
On Thu, Jan 24, 2019 at 11:37:33PM +0530, Jagan Teki wrote:
> CSI block in Allwinner A64 has similar features as like in H3,
> but the default CSI_SCLK rate cannot work properly to drive the
> connected sensor interface.
>
> The tested mod cock rate is 300 MHz and BSP vfe media driver is also
> using the same rate. Unfortunately there is no valid information about
> clock rate in manual or any other sources except the BSP driver. so more
> faith on BSP code, because same has tested in mainline.
>
> So, add support for A64 CSI block by setting updated mod clock rate.
>
> Signed-off-by: Jagan Teki
> ---
> drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c | 13 -
> 1 file changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c
> b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c
> index ee882b66a5ea..cd2d33242c17 100644
> --- a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c
> +++ b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c
> @@ -15,6 +15,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -154,6 +155,7 @@ bool sun6i_csi_is_format_supported(struct sun6i_csi *csi,
> int sun6i_csi_set_power(struct sun6i_csi *csi, bool enable)
> {
> struct sun6i_csi_dev *sdev = sun6i_csi_to_dev(csi);
> + struct device *dev = sdev->dev;
> struct regmap *regmap = sdev->regmap;
> int ret;
>
> @@ -161,15 +163,20 @@ int sun6i_csi_set_power(struct sun6i_csi *csi, bool
> enable)
> regmap_update_bits(regmap, CSI_EN_REG, CSI_EN_CSI_EN, 0);
>
> clk_disable_unprepare(sdev->clk_ram);
> + if (of_device_is_compatible(dev->of_node,
> "allwinner,sun50i-a64-csi"))
> + clk_rate_exclusive_put(sdev->clk_mod);
> clk_disable_unprepare(sdev->clk_mod);
> reset_control_assert(sdev->rstc_bus);
> return 0;
> }
>
> + if (of_device_is_compatible(dev->of_node, "allwinner,sun50i-a64-csi"))
> + clk_set_rate_exclusive(sdev->clk_mod, 3);
> +
> ret = clk_prepare_enable(sdev->clk_mod);
> if (ret) {
> dev_err(sdev->dev, "Enable csi clk err %d\n", ret);
> - return ret;
> + goto clk_mod_put;
> }
>
> ret = clk_prepare_enable(sdev->clk_ram);
> @@ -192,6 +199,9 @@ int sun6i_csi_set_power(struct sun6i_csi *csi, bool
> enable)
> clk_disable_unprepare(sdev->clk_ram);
> clk_mod_disable:
> clk_disable_unprepare(sdev->clk_mod);
> +clk_mod_put:
> + if (of_device_is_compatible(dev->of_node, "allwinner,sun50i-a64-csi"))
> + clk_rate_exclusive_put(sdev->clk_mod);
> return ret;
The sequence in the error path and in the disable path aren't the same, why?
Maxime
--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
signature.asc
Description: PGP signature
Re: [PATCH v7 1/5] dt-bindings: media: sun6i: Add A64 CSI compatible
On Thu, Jan 24, 2019 at 11:37:32PM +0530, Jagan Teki wrote: > Allwinner A64 CSI is a single channel time-multiplexed BT.656 > protocol interface. > > Add separate compatible string for A64 since it require explicit > change in sun6i_csi driver to update default CSI_SCLK rate. > > Reviewed-by: Rob Herring > Signed-off-by: Jagan Teki Acked-by: Maxime Ripard Maxime -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com signature.asc Description: PGP signature
Re: [PATCH] media: ov5640: Fix set 15fps regression
On Thu, Jan 24, 2019 at 11:28:01PM +0530, Jagan Teki wrote:
> The ov5640_try_frame_interval operation updates the FPS as per user
> input based on default ov5640_frame_rate, OV5640_30_FPS which is failed
> to update when user trigger 15fps.
>
> So, initialize the default ov5640_frame_rate to OV5640_15_FPS so-that
> it can satisfy to update all fps.
>
> Fixes: 5a3ad937bc78 ("media: ov5640: Make the return rate type more explicit")
> Signed-off-by: Jagan Teki
I'm pretty sure I tested this and it was working fine. You're
mentionning a regression, but what regression is there exactly (ie,
what was working before that commit that doesn't work anymore?). What
tools/commands are you using to see this behaviour?
It really isn't obvious from your patch and the patch you mention what
could go wrong or be improved.
Maxime
--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
signature.asc
Description: PGP signature
Re: [PATCH v2 1/2] media: uapi: Add H264 low-level decoder API compound controls.
On Thu, Jan 24, 2019 at 10:37:23PM +0800, Ayaka wrote:
> >>>>> +#define V4L2_H264_DPB_ENTRY_FLAG_VALID0x01
> >>>>> +#define V4L2_H264_DPB_ENTRY_FLAG_ACTIVE0x02
> >>>>> +#define V4L2_H264_DPB_ENTRY_FLAG_LONG_TERM0x04
> >>>>> +
> >>>>> +struct v4l2_h264_dpb_entry {
> >>>>> +__u32 tag;
> >>>>> +__u16 frame_num;
> >>>>> +__u16 pic_num;
> >>>> Although the long term reference would use picture order count
> >>>> and short term for frame num, but only one of them is used
> >>>> for a entry of a dpb.
> >>>>
> >>>> Besides, for a frame picture frame_num = pic_num * 2,
> >>>> and frame_num = pic_num * 2 + 1 for a filed.
> >>>
> >>> I'm not sure what is your point?
> >>
> >> I found I was wrong at the last email.
> >>
> >> But stateless hardware decoder usually don't care about whether it is long
> >> term or short term, as the real dpb updating or management work are not
> >> done
> >> by the the driver or device and decoding job would only use the two list(or
> >> one list for slice P) for reference pictures. So those flag for long term
> >> or
> >> status can be removed as well.
> >
> > I'll remove the LONG_TERM flag then. We do need the other two for the
> > Allwinner driver though.
> >
>
> I would ask Paulk and check the manual and vendor library later.
>
> Even there are two register fields, it don’t mean they would be used
> and required at the same times. Because it don’t follow ISO manual.
It's not a matter of decoding per se, but how the hardware
behaves. All the buffers needed for one particular frame to be decoded
are uploaded to an SRAM, and the position of each buffer in that SRAM
cannot change during the time when it has been decoded, and then later
on when it's used as a reference. If you only have the frames needed
to decode the current frame, you will have no idea which slot in the
SRAM can be reused, whereas having the full DPB allows you to do
that. And that's what _FLAG_ACTIVE gives you.
> >> And I agree above with my last mail, so I would suggest to keep a property
> >> as index for both frame_num and pic_num, as only one of them would be used
> >> for a picture decoding once time.
> >
> > I'd really prefer to keep everything that is in the bitstream defined
> > here. We don't want to cover the usual cases, but all of them even the
> > one that haven't been designed yet, so we should be really
> > conservative.
>
> As I mention in the other mail, a stateless decoder or encoder like
> means the device won’t track the previous result. But you have no
> idea on what data the device would need to process this picture. It
> is hard to define a standard structure for it.
>
> As you see, even allwinner doesn’t obey all the standard the IOS
> document said.
It's not that it disobeys it, it's that it requires the full blown DPB
to have a working driver.
> In my original suggestion, I would just to add more reservation
> fields then future driver can use it.
This interface is not stable at the moment, so it doesn't really
matter does it?
Maxime
--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
signature.asc
Description: PGP signature
Re: [PATCH v2 2/2] media: cedrus: Add HEVC/H.265 decoding support
Hi,
On Thu, Jan 24, 2019 at 02:10:25PM +0100, Paul Kocialkowski wrote:
> On Tue, 2018-11-27 at 09:21 +0100, Maxime Ripard wrote:
> > Hi!
> >
> > On Fri, Nov 23, 2018 at 02:02:09PM +0100, Paul Kocialkowski wrote:
> > > This introduces support for HEVC/H.265 to the Cedrus VPU driver, with
> > > both uni-directional and bi-directional prediction modes supported.
> > >
> > > Field-coded (interlaced) pictures, custom quantization matrices and
> > > 10-bit output are not supported at this point.
> > >
> > > Signed-off-by: Paul Kocialkowski
> >
> > Output from checkpatch:
> > total: 0 errors, 68 warnings, 14 checks, 999 lines checked
>
> Looks like many of the "line over 80 chars" are due to macros. I don't
> think it would be a good idea to break them down or to change the
> macros names since they are directly inherited from the bitstream
> elements.
>
> What do you think?
Yeah, the 80-chars limit can be ignored. But there's more warnings and
checks that should be addressed.
> > > + /* Output frame. */
> > > +
> > > + output_pic_list_index = V4L2_HEVC_DPB_ENTRIES_NUM_MAX;
> > > + pic_order_cnt[0] = pic_order_cnt[1] = slice_params->slice_pic_order_cnt;
> > > + mv_col_buf_addr[0] = cedrus_h265_frame_info_mv_col_buf_addr(ctx,
> > > + run->dst->vb2_buf.index, 0) - PHYS_OFFSET;
> > > + mv_col_buf_addr[1] = cedrus_h265_frame_info_mv_col_buf_addr(ctx,
> > > + run->dst->vb2_buf.index, 1) - PHYS_OFFSET;
> > > + dst_luma_addr = cedrus_dst_buf_addr(ctx, run->dst->vb2_buf.index, 0) -
> > > + PHYS_OFFSET;
> > > + dst_chroma_addr = cedrus_dst_buf_addr(ctx, run->dst->vb2_buf.index, 1) -
> > > + PHYS_OFFSET;
> > > +
> > > + cedrus_h265_frame_info_write_single(dev, output_pic_list_index,
> > > + slice_params->pic_struct != 0,
> > > + pic_order_cnt, mv_col_buf_addr,
> > > + dst_luma_addr, dst_chroma_addr);
> >
> > You can only pass the run and slice_params pointers to that function.
>
> The point is to make it independent from the context, so that the same
> function can be called with either the slice_params or the dpb info.
> I don't think making two variants or even two wrappers would bring any
> significant benefit.
Then you can still pass directly the vb2 buffer pointer, that would
remove the mv_col_buf_addr, dst_luma_addr and dst_chroma_addr. The
idea here is that the less arguments you have in your function, the
easier it is to understand.
> > > +
> > > + cedrus_write(dev, VE_DEC_H265_OUTPUT_FRAME_IDX, output_pic_list_index);
> > > +
> > > + /* Reference picture list 0 (for P/B frames). */
> > > + if (slice_params->slice_type != V4L2_HEVC_SLICE_TYPE_I) {
> > > + cedrus_h265_ref_pic_list_write(dev, slice_params->ref_idx_l0,
> > > + slice_params->num_ref_idx_l0_active_minus1 + 1,
> > > + slice_params->dpb, slice_params->num_active_dpb_entries,
> > > + VE_DEC_H265_SRAM_OFFSET_REF_PIC_LIST0);
> > > +
> >
> > slice_params is enough.
>
> The rationale is similar to the one above: being able to use the same
> helper with either L0 or L1, which implies passing the relevant
> elements directly.
The DPB and num_active_dpb_entries will not change from one run to the
other though. And having intermediate functions if that allows to be
clearer is fine as well.
Maxime
--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
signature.asc
Description: PGP signature
Re: [PATCH v2 1/2] media: uapi: Add H264 low-level decoder API compound controls.
> > > + __u16 frame_num;
> > > > + __u16 pic_num;
> > > Although the long term reference would use picture order count
> > > and short term for frame num, but only one of them is used
> > > for a entry of a dpb.
> > >
> > > Besides, for a frame picture frame_num = pic_num * 2,
> > > and frame_num = pic_num * 2 + 1 for a filed.
> >
> > I'm not sure what is your point?
>
> I found I was wrong at the last email.
>
> But stateless hardware decoder usually don't care about whether it is long
> term or short term, as the real dpb updating or management work are not done
> by the the driver or device and decoding job would only use the two list(or
> one list for slice P) for reference pictures. So those flag for long term or
> status can be removed as well.
I'll remove the LONG_TERM flag then. We do need the other two for the
Allwinner driver though.
> And I agree above with my last mail, so I would suggest to keep a property
> as index for both frame_num and pic_num, as only one of them would be used
> for a picture decoding once time.
I'd really prefer to keep everything that is in the bitstream defined
here. We don't want to cover the usual cases, but all of them even the
one that haven't been designed yet, so we should be really
conservative.
> > > > + /* Note that field is indicated by v4l2_buffer.field */
> > > > + __s32 top_field_order_cnt;
> > > > + __s32 bottom_field_order_cnt;
> > > > + __u8 flags; /* V4L2_H264_DPB_ENTRY_FLAG_* */
> > > > +};
> > > > +
> > > > +struct v4l2_ctrl_h264_decode_param {
> > > > + __u32 num_slices;
> > > > + __u8 idr_pic_flag;
> > > > + __u8 nal_ref_idc;
> > > > + __s32 top_field_order_cnt;
> > > > + __s32 bottom_field_order_cnt;
> > > > + __u8 ref_pic_list_p0[32];
> > > > + __u8 ref_pic_list_b0[32];
> > > > + __u8 ref_pic_list_b1[32];
> > >
> > > I would prefer to keep only two list, list0 and list 1.
> >
> > I'm not even sure why this is needed in the first place
> > anymore. It's not part of the bitstream, and it seems to come from
> > ChromeOS' Rockchip driver that uses it though. Do you know why?
>
> You see the P frame would use only a list and B for two list. So for
> the parameter of a picture, two lists are max. I would suggest only
> keep two arrays here and rename them as list0 and list1, it would
> reduce the conflict.
Right, but those lists are already in v4l2_ctrl_h264_slice_param (with
the construct you are suggesting). I'm not sure about why the
redundancy is needed. Alex, Tomasz, do you have any idea why this was
needed at some point?
Thanks!
Maxime
--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
signature.asc
Description: PGP signature
Re: [PATCH v6 2/6] media: sun6i: Add mod_rate quirk
On Fri, Jan 18, 2019 at 10:01:54PM +0530, Jagan Teki wrote: > Unfortunately default CSI_SCLK rate cannot work properly to > drive the connected sensor interface, particularly on few > Allwinner SoC's like A64. > > So, add mod_rate quirk via driver data so-that the respective > SoC's which require to alter the default mod clock rate can assign > the operating clock rate. > > Signed-off-by: Jagan Teki You still don't need the variant stuff. If the sole difference is that we need that clock rate to be fixed, then the following patch is enough. http://code.bulix.org/9au998-562745?raw Maxime -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com signature.asc Description: PGP signature
[PATCH v5 3/9] phy: dphy: Clarify lanes parameter documentation
The lanes parameter is not solely about the number of lanes, but it also
carries the fact that those are the first lanes in use during the
transmission.
It was implicit so far, so make sure it's explicit now.
Suggested-by: Sakari Ailus
Acked-by: Sakari Ailus
Signed-off-by: Maxime Ripard
---
include/linux/phy/phy-mipi-dphy.h | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/include/linux/phy/phy-mipi-dphy.h
b/include/linux/phy/phy-mipi-dphy.h
index 627d28080d3a..a877ffee845d 100644
--- a/include/linux/phy/phy-mipi-dphy.h
+++ b/include/linux/phy/phy-mipi-dphy.h
@@ -269,7 +269,8 @@ struct phy_configure_opts_mipi_dphy {
/**
* @lanes:
*
-* Number of active data lanes used for the transmissions.
+* Number of active, consecutive, data lanes, starting from
+* lane 0, used for the transmissions.
*/
unsigned char lanes;
};
--
git-series 0.9.1
[PATCH v5 4/9] sun6i: dsi: Convert to generic phy handling
Now that we have everything in place in the PHY framework to deal in a
generic way with MIPI D-PHY phys, let's convert our PHY driver and its
associated DSI driver to that new API.
Signed-off-by: Maxime Ripard
---
drivers/gpu/drm/sun4i/Kconfig | 11 +-
drivers/gpu/drm/sun4i/Makefile | 6 +-
drivers/gpu/drm/sun4i/sun6i_mipi_dphy.c | 164 ++---
drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 31 ++---
drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h | 17 +---
5 files changed, 126 insertions(+), 103 deletions(-)
diff --git a/drivers/gpu/drm/sun4i/Kconfig b/drivers/gpu/drm/sun4i/Kconfig
index c2c042287c19..2b8db82c4bab 100644
--- a/drivers/gpu/drm/sun4i/Kconfig
+++ b/drivers/gpu/drm/sun4i/Kconfig
@@ -45,10 +45,19 @@ config DRM_SUN6I_DSI
default MACH_SUN8I
select CRC_CCITT
select DRM_MIPI_DSI
+ select DRM_SUN6I_DPHY
help
Choose this option if you want have an Allwinner SoC with
MIPI-DSI support. If M is selected the module will be called
- sun6i-dsi
+ sun6i_mipi_dsi.
+
+config DRM_SUN6I_DPHY
+ tristate "Allwinner A31 MIPI D-PHY Support"
+ select GENERIC_PHY_MIPI_DPHY
+ help
+ Choose this option if you have an Allwinner SoC with
+ MIPI-DSI support. If M is selected, the module will be
+ called sun6i_mipi_dphy.
config DRM_SUN8I_DW_HDMI
tristate "Support for Allwinner version of DesignWare HDMI"
diff --git a/drivers/gpu/drm/sun4i/Makefile b/drivers/gpu/drm/sun4i/Makefile
index 0eb38ac8e86e..1e2320d824b5 100644
--- a/drivers/gpu/drm/sun4i/Makefile
+++ b/drivers/gpu/drm/sun4i/Makefile
@@ -24,9 +24,6 @@ sun4i-tcon-y += sun4i_lvds.o
sun4i-tcon-y += sun4i_tcon.o
sun4i-tcon-y += sun4i_rgb.o
-sun6i-dsi-y+= sun6i_mipi_dphy.o
-sun6i-dsi-y+= sun6i_mipi_dsi.o
-
obj-$(CONFIG_DRM_SUN4I)+= sun4i-drm.o
obj-$(CONFIG_DRM_SUN4I)+= sun4i-tcon.o
obj-$(CONFIG_DRM_SUN4I)+= sun4i_tv.o
@@ -37,7 +34,8 @@ ifdef CONFIG_DRM_SUN4I_BACKEND
obj-$(CONFIG_DRM_SUN4I)+= sun4i-frontend.o
endif
obj-$(CONFIG_DRM_SUN4I_HDMI) += sun4i-drm-hdmi.o
-obj-$(CONFIG_DRM_SUN6I_DSI)+= sun6i-dsi.o
+obj-$(CONFIG_DRM_SUN6I_DPHY) += sun6i_mipi_dphy.o
+obj-$(CONFIG_DRM_SUN6I_DSI)+= sun6i_mipi_dsi.o
obj-$(CONFIG_DRM_SUN8I_DW_HDMI)+= sun8i-drm-hdmi.o
obj-$(CONFIG_DRM_SUN8I_MIXER) += sun8i-mixer.o
obj-$(CONFIG_DRM_SUN8I_TCON_TOP) += sun8i_tcon_top.o
diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dphy.c
b/drivers/gpu/drm/sun4i/sun6i_mipi_dphy.c
index e4d19431fa0e..79c8af5c7c1d 100644
--- a/drivers/gpu/drm/sun4i/sun6i_mipi_dphy.c
+++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dphy.c
@@ -8,11 +8,14 @@
#include
#include
+#include
#include
+#include
#include
#include
-#include "sun6i_mipi_dsi.h"
+#include
+#include
#define SUN6I_DPHY_GCTL_REG0x00
#define SUN6I_DPHY_GCTL_LANE_NUM(n)n) - 1) & 3) << 4)
@@ -81,12 +84,46 @@
#define SUN6I_DPHY_DBG5_REG0xf4
-int sun6i_dphy_init(struct sun6i_dphy *dphy, unsigned int lanes)
+struct sun6i_dphy {
+ struct clk *bus_clk;
+ struct clk *mod_clk;
+ struct regmap *regs;
+ struct reset_control*reset;
+
+ struct phy *phy;
+ struct phy_configure_opts_mipi_dphy config;
+};
+
+static int sun6i_dphy_init(struct phy *phy)
{
+ struct sun6i_dphy *dphy = phy_get_drvdata(phy);
+
reset_control_deassert(dphy->reset);
clk_prepare_enable(dphy->mod_clk);
clk_set_rate_exclusive(dphy->mod_clk, 15000);
+ return 0;
+}
+
+static int sun6i_dphy_configure(struct phy *phy, union phy_configure_opts
*opts)
+{
+ struct sun6i_dphy *dphy = phy_get_drvdata(phy);
+ int ret;
+
+ ret = phy_mipi_dphy_config_validate(&opts->mipi_dphy);
+ if (ret)
+ return ret;
+
+ memcpy(&dphy->config, opts, sizeof(dphy->config));
+
+ return 0;
+}
+
+static int sun6i_dphy_power_on(struct phy *phy)
+{
+ struct sun6i_dphy *dphy = phy_get_drvdata(phy);
+ u8 lanes_mask = GENMASK(dphy->config.lanes - 1, 0);
+
regmap_write(dphy->regs, SUN6I_DPHY_TX_CTL_REG,
SUN6I_DPHY_TX_CTL_HS_TX_CLK_CONT);
@@ -111,16 +148,9 @@ int sun6i_dphy_init(struct sun6i_dphy *dphy, unsigned int
lanes)
SUN6I_DPHY_TX_TIME4_HS_TX_ANA1(3));
regmap_write(dphy->regs, SUN6I_DPHY_GCTL_REG,
-SUN6I_DPHY_GCTL_LANE_NUM(lanes) |
+SUN6I_DPHY_GCTL_LANE_NUM(dphy->config.lanes) |
SUN6I_DPHY_GCTL_EN);
- return 0;
-}
-
-int sun6i_dphy_power_on(struct su
[PATCH v5 2/9] phy: dphy: Change units of wakeup and init parameters
The Init and wakeup D-PHY parameters are in the micro/milliseconds range,
putting the values real close to the types limits if they were in
picoseconds.
Move them to microseconds which should be better fit.
Suggested-by: Sakari Ailus
Acked-by: Sakari Ailus
Signed-off-by: Maxime Ripard
---
drivers/phy/phy-core-mipi-dphy.c | 8
include/linux/phy/phy-mipi-dphy.h | 8
2 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/phy/phy-core-mipi-dphy.c b/drivers/phy/phy-core-mipi-dphy.c
index 465fa1b91a5f..14e0551cd319 100644
--- a/drivers/phy/phy-core-mipi-dphy.c
+++ b/drivers/phy/phy-core-mipi-dphy.c
@@ -65,12 +65,12 @@ int phy_mipi_dphy_get_default_config(unsigned long
pixel_clock,
*/
cfg->hs_trail = max(4 * 8 * ui, 6 + 4 * 4 * ui);
- cfg->init = 1;
+ cfg->init = 100;
cfg->lpx = 6;
cfg->ta_get = 5 * cfg->lpx;
cfg->ta_go = 4 * cfg->lpx;
cfg->ta_sure = 2 * cfg->lpx;
- cfg->wakeup = 10;
+ cfg->wakeup = 1000;
cfg->hs_clk_rate = hs_clk_rate;
cfg->lanes = lanes;
@@ -143,7 +143,7 @@ int phy_mipi_dphy_config_validate(struct
phy_configure_opts_mipi_dphy *cfg)
if (cfg->hs_trail < max(8 * ui, 6 + 4 * ui))
return -EINVAL;
- if (cfg->init < 1)
+ if (cfg->init < 100)
return -EINVAL;
if (cfg->lpx < 5)
@@ -158,7 +158,7 @@ int phy_mipi_dphy_config_validate(struct
phy_configure_opts_mipi_dphy *cfg)
if (cfg->ta_sure < cfg->lpx || cfg->ta_sure > (2 * cfg->lpx))
return -EINVAL;
- if (cfg->wakeup < 10)
+ if (cfg->wakeup < 1000)
return -EINVAL;
return 0;
diff --git a/include/linux/phy/phy-mipi-dphy.h
b/include/linux/phy/phy-mipi-dphy.h
index 9cf97cd1d303..627d28080d3a 100644
--- a/include/linux/phy/phy-mipi-dphy.h
+++ b/include/linux/phy/phy-mipi-dphy.h
@@ -190,10 +190,10 @@ struct phy_configure_opts_mipi_dphy {
/**
* @init:
*
-* Time, in picoseconds for the initialization period to
+* Time, in microseconds for the initialization period to
* complete.
*
-* Minimum value: 1 ps
+* Minimum value: 100 us
*/
unsigned intinit;
@@ -244,11 +244,11 @@ struct phy_configure_opts_mipi_dphy {
/**
* @wakeup:
*
-* Time, in picoseconds, that a transmitter drives a Mark-1
+* Time, in microseconds, that a transmitter drives a Mark-1
* state prior to a Stop state in order to initiate an exit
* from ULPS.
*
-* Minimum value: 10 ps
+* Minimum value: 1000 us
*/
unsigned intwakeup;
--
git-series 0.9.1
[PATCH v5 0/9] phy: Add configuration interface for MIPI D-PHY devices
Hi, Here is a set of patches to allow the phy framework consumers to test and apply runtime configurations. This is needed to support more phy classes that require tuning based on parameters depending on the current use case of the device, in addition to the power state management already provided by the current functions. A first test bed for that API are the MIPI D-PHY devices. There's a number of solutions that have been used so far to support these phy, most of the time being an ad-hoc driver in the consumer. That approach has a big shortcoming though, which is that this is quite difficult to deal with consumers integrated with multiple variants of phy, of multiple consumers integrated with the same phy. The latter case can be found in the Cadence DSI bridge, and the CSI transceiver and receivers. All of them are integrated with the same phy, or can be integrated with different phy, depending on the implementation. I've looked at all the MIPI DSI drivers I could find, and gathered all the parameters I could find. The interface should be complete, and most of the drivers can be converted in the future. The current set converts two of them: the above mentionned Cadence DSI driver so that the v4l2 drivers can use them, and the Allwinner MIPI-DSI driver. Let me know what you think, Maxime Changes from v4: - Removed regression on the variable calculation - Fixed the wakeup unit - Collected Sean Acked-by on the last patch - Collected Sakari Reviewed-by on the first patch Changes from v3 - Rebased on 5.0-rc1 - Added the fixes suggested by Sakari Changes from v2: - Rebased on next - Changed the interface to accomodate for the new submodes - Changed the timings units from nanoseconds to picoseconds - Added minimum and maximum boundaries to the documentation - Moved the clock enabling to phy_power_on in the Cadence DPHY driver - Exported the phy_configure and phy_validate symbols - Rework the phy pll divider computation in the cadence dphy driver Changes from v1: - Rebased on top of 4.20-rc1 - Removed the bus mode and timings parameters from the MIPI D-PHY parameters, since that shouldn't have any impact on the PHY itself. - Reworked the Cadence DSI and D-PHY drivers to take this into account. - Remove the mode parameter from phy_configure - Added phy_configure and phy_validate stubs - Return -EOPNOTSUPP in phy_configure and phy_validate when the operation is not implemented Maxime Ripard (9): phy: dphy: Remove unused header phy: dphy: Change units of wakeup and init parameters phy: dphy: Clarify lanes parameter documentation sun6i: dsi: Convert to generic phy handling phy: Move Allwinner A31 D-PHY driver to drivers/phy/ drm/bridge: cdns: Separate DSI and D-PHY configuration dt-bindings: phy: Move the Cadence D-PHY bindings phy: Add Cadence D-PHY support drm/bridge: cdns: Convert to phy framework Documentation/devicetree/bindings/display/bridge/cdns,dsi.txt | 21 +- Documentation/devicetree/bindings/phy/cdns,dphy.txt | 20 +- drivers/gpu/drm/bridge/Kconfig| 1 +- drivers/gpu/drm/bridge/cdns-dsi.c | 538 +-- drivers/gpu/drm/sun4i/Kconfig | 3 +- drivers/gpu/drm/sun4i/Makefile| 5 +- drivers/gpu/drm/sun4i/sun6i_mipi_dphy.c | 292 + drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c| 31 +- drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h| 17 +- drivers/phy/allwinner/Kconfig | 12 +- drivers/phy/allwinner/Makefile| 1 +- drivers/phy/allwinner/phy-sun6i-mipi-dphy.c | 318 - drivers/phy/cadence/Kconfig | 13 +- drivers/phy/cadence/Makefile | 1 +- drivers/phy/cadence/cdns-dphy.c | 389 +- drivers/phy/phy-core-mipi-dphy.c | 8 +- include/linux/phy/phy-mipi-dphy.h | 13 +- 17 files changed, 894 insertions(+), 789 deletions(-) create mode 100644 Documentation/devicetree/bindings/phy/cdns,dphy.txt delete mode 100644 drivers/gpu/drm/sun4i/sun6i_mipi_dphy.c create mode 100644 drivers/phy/allwinner/phy-sun6i-mipi-dphy.c create mode 100644 drivers/phy/cadence/cdns-dphy.c base-commit: bfeffd155283772bbe78c6a05dec7c0128ee500c -- git-series 0.9.1
[PATCH v5 5/9] phy: Move Allwinner A31 D-PHY driver to drivers/phy/
Now that our MIPI D-PHY driver has been converted to the phy framework, let's move it into the drivers/phy directory. Signed-off-by: Maxime Ripard --- drivers/gpu/drm/sun4i/Kconfig | 10 +- drivers/gpu/drm/sun4i/Makefile | 1 +- drivers/gpu/drm/sun4i/sun6i_mipi_dphy.c | 318 +- drivers/phy/allwinner/Kconfig | 12 +- drivers/phy/allwinner/Makefile | 1 +- drivers/phy/allwinner/phy-sun6i-mipi-dphy.c | 318 +- 6 files changed, 332 insertions(+), 328 deletions(-) delete mode 100644 drivers/gpu/drm/sun4i/sun6i_mipi_dphy.c create mode 100644 drivers/phy/allwinner/phy-sun6i-mipi-dphy.c diff --git a/drivers/gpu/drm/sun4i/Kconfig b/drivers/gpu/drm/sun4i/Kconfig index 2b8db82c4bab..1dbbc3a1b763 100644 --- a/drivers/gpu/drm/sun4i/Kconfig +++ b/drivers/gpu/drm/sun4i/Kconfig @@ -45,20 +45,12 @@ config DRM_SUN6I_DSI default MACH_SUN8I select CRC_CCITT select DRM_MIPI_DSI - select DRM_SUN6I_DPHY + select PHY_SUN6I_MIPI_DPHY help Choose this option if you want have an Allwinner SoC with MIPI-DSI support. If M is selected the module will be called sun6i_mipi_dsi. -config DRM_SUN6I_DPHY - tristate "Allwinner A31 MIPI D-PHY Support" - select GENERIC_PHY_MIPI_DPHY - help - Choose this option if you have an Allwinner SoC with - MIPI-DSI support. If M is selected, the module will be - called sun6i_mipi_dphy. - config DRM_SUN8I_DW_HDMI tristate "Support for Allwinner version of DesignWare HDMI" depends on DRM_SUN4I diff --git a/drivers/gpu/drm/sun4i/Makefile b/drivers/gpu/drm/sun4i/Makefile index 1e2320d824b5..0d04f2447b01 100644 --- a/drivers/gpu/drm/sun4i/Makefile +++ b/drivers/gpu/drm/sun4i/Makefile @@ -34,7 +34,6 @@ ifdef CONFIG_DRM_SUN4I_BACKEND obj-$(CONFIG_DRM_SUN4I)+= sun4i-frontend.o endif obj-$(CONFIG_DRM_SUN4I_HDMI) += sun4i-drm-hdmi.o -obj-$(CONFIG_DRM_SUN6I_DPHY) += sun6i_mipi_dphy.o obj-$(CONFIG_DRM_SUN6I_DSI)+= sun6i_mipi_dsi.o obj-$(CONFIG_DRM_SUN8I_DW_HDMI)+= sun8i-drm-hdmi.o obj-$(CONFIG_DRM_SUN8I_MIXER) += sun8i-mixer.o diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dphy.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dphy.c deleted file mode 100644 index 79c8af5c7c1d.. --- a/drivers/gpu/drm/sun4i/sun6i_mipi_dphy.c +++ /dev/null @@ -1,318 +0,0 @@ -// SPDX-License-Identifier: GPL-2.0+ -/* - * Copyright (c) 2016 Allwinnertech Co., Ltd. - * Copyright (C) 2017-2018 Bootlin - * - * Maxime Ripard - */ - -#include -#include -#include -#include -#include -#include -#include - -#include -#include - -#define SUN6I_DPHY_GCTL_REG0x00 -#define SUN6I_DPHY_GCTL_LANE_NUM(n)n) - 1) & 3) << 4) -#define SUN6I_DPHY_GCTL_EN BIT(0) - -#define SUN6I_DPHY_TX_CTL_REG 0x04 -#define SUN6I_DPHY_TX_CTL_HS_TX_CLK_CONT BIT(28) - -#define SUN6I_DPHY_TX_TIME0_REG0x10 -#define SUN6I_DPHY_TX_TIME0_HS_TRAIL(n)(((n) & 0xff) << 24) -#define SUN6I_DPHY_TX_TIME0_HS_PREPARE(n) (((n) & 0xff) << 16) -#define SUN6I_DPHY_TX_TIME0_LP_CLK_DIV(n) ((n) & 0xff) - -#define SUN6I_DPHY_TX_TIME1_REG0x14 -#define SUN6I_DPHY_TX_TIME1_CLK_POST(n)(((n) & 0xff) << 24) -#define SUN6I_DPHY_TX_TIME1_CLK_PRE(n) (((n) & 0xff) << 16) -#define SUN6I_DPHY_TX_TIME1_CLK_ZERO(n)(((n) & 0xff) << 8) -#define SUN6I_DPHY_TX_TIME1_CLK_PREPARE(n) ((n) & 0xff) - -#define SUN6I_DPHY_TX_TIME2_REG0x18 -#define SUN6I_DPHY_TX_TIME2_CLK_TRAIL(n) ((n) & 0xff) - -#define SUN6I_DPHY_TX_TIME3_REG0x1c - -#define SUN6I_DPHY_TX_TIME4_REG0x20 -#define SUN6I_DPHY_TX_TIME4_HS_TX_ANA1(n) (((n) & 0xff) << 8) -#define SUN6I_DPHY_TX_TIME4_HS_TX_ANA0(n) ((n) & 0xff) - -#define SUN6I_DPHY_ANA0_REG0x4c -#define SUN6I_DPHY_ANA0_REG_PWSBIT(31) -#define SUN6I_DPHY_ANA0_REG_DMPC BIT(28) -#define SUN6I_DPHY_ANA0_REG_DMPD(n)(((n) & 0xf) << 24) -#define SUN6I_DPHY_ANA0_REG_SLV(n) (((n) & 7) << 12) -#define SUN6I_DPHY_ANA0_REG_DEN(n) (((n) & 0xf) << 8) - -#define SUN6I_DPHY_ANA1_REG0x50 -#define SUN6I_DPHY_ANA1_REG_VTTMODEBIT(31) -#define SUN6I_DPHY_ANA1_REG_CSMPS(n) (((n) & 3) << 28) -#define SUN6I_DPHY_ANA1_REG_SVTT(n)(((n) & 0xf) << 24) - -#define SUN6I_DPHY_ANA2_REG0x54 -#define SUN6I_DPHY_ANA2_EN_P2S_CPU(n) (((n) & 0xf) << 24) -#define SUN6I_DPHY_ANA2_EN_P2S_CPU_MASKGENMASK(27, 24) -#define SUN6I_DPHY_ANA2_EN_CK_CPU BIT(4) -#defin
[PATCH v5 7/9] dt-bindings: phy: Move the Cadence D-PHY bindings
The Cadence D-PHY bindings was defined as part of the DSI block so far.
However, since it's now going to be a separate driver, we need to move the
binding to a file of its own.
Acked-by: Sakari Ailus
Signed-off-by: Maxime Ripard
---
Documentation/devicetree/bindings/display/bridge/cdns,dsi.txt | 21 +---
Documentation/devicetree/bindings/phy/cdns,dphy.txt | 20 +++-
2 files changed, 20 insertions(+), 21 deletions(-)
create mode 100644 Documentation/devicetree/bindings/phy/cdns,dphy.txt
diff --git a/Documentation/devicetree/bindings/display/bridge/cdns,dsi.txt
b/Documentation/devicetree/bindings/display/bridge/cdns,dsi.txt
index f5725bb6c61c..525a4bfd8634 100644
--- a/Documentation/devicetree/bindings/display/bridge/cdns,dsi.txt
+++ b/Documentation/devicetree/bindings/display/bridge/cdns,dsi.txt
@@ -31,28 +31,7 @@ Required subnodes:
- one subnode per DSI device connected on the DSI bus. Each DSI device should
contain a reg property encoding its virtual channel.
-Cadence DPHY
-
-
-Cadence DPHY block.
-
-Required properties:
-- compatible: should be set to "cdns,dphy".
-- reg: physical base address and length of the DPHY registers.
-- clocks: DPHY reference clocks.
-- clock-names: must contain "psm" and "pll_ref".
-- #phy-cells: must be set to 0.
-
-
Example:
- dphy0: dphy@fd0e{
- compatible = "cdns,dphy";
- reg = <0x0 0xfd0e 0x0 0x1000>;
- clocks = <&psm_clk>, <&pll_ref_clk>;
- clock-names = "psm", "pll_ref";
- #phy-cells = <0>;
- };
-
dsi0: dsi@fd0c {
compatible = "cdns,dsi";
reg = <0x0 0xfd0c 0x0 0x1000>;
diff --git a/Documentation/devicetree/bindings/phy/cdns,dphy.txt
b/Documentation/devicetree/bindings/phy/cdns,dphy.txt
new file mode 100644
index ..1095bc4e72d9
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/cdns,dphy.txt
@@ -0,0 +1,20 @@
+Cadence DPHY
+
+
+Cadence DPHY block.
+
+Required properties:
+- compatible: should be set to "cdns,dphy".
+- reg: physical base address and length of the DPHY registers.
+- clocks: DPHY reference clocks.
+- clock-names: must contain "psm" and "pll_ref".
+- #phy-cells: must be set to 0.
+
+Example:
+ dphy0: dphy@fd0e{
+ compatible = "cdns,dphy";
+ reg = <0x0 0xfd0e 0x0 0x1000>;
+ clocks = <&psm_clk>, <&pll_ref_clk>;
+ clock-names = "psm", "pll_ref";
+ #phy-cells = <0>;
+ };
--
git-series 0.9.1
[PATCH v5 1/9] phy: dphy: Remove unused header
The videomode.h header inclusion is an artifact from the patches development, remove it. Suggested-by: Sakari Ailus Acked-by: Sakari Ailus Signed-off-by: Maxime Ripard --- include/linux/phy/phy-mipi-dphy.h | 2 -- 1 file changed, 2 deletions(-) diff --git a/include/linux/phy/phy-mipi-dphy.h b/include/linux/phy/phy-mipi-dphy.h index c08aacc0ac35..9cf97cd1d303 100644 --- a/include/linux/phy/phy-mipi-dphy.h +++ b/include/linux/phy/phy-mipi-dphy.h @@ -6,8 +6,6 @@ #ifndef __PHY_MIPI_DPHY_H_ #define __PHY_MIPI_DPHY_H_ -#include - /** * struct phy_configure_opts_mipi_dphy - MIPI D-PHY configuration set * -- git-series 0.9.1
[PATCH v5 9/9] drm/bridge: cdns: Convert to phy framework
Now that we have everything we need in the phy framework to allow to tune
the phy parameters, let's convert the Cadence DSI bridge to that API
instead of creating a ad-hoc driver for its phy.
Acked-by: Sean Paul
Signed-off-by: Maxime Ripard
---
drivers/gpu/drm/bridge/Kconfig| 1 +-
drivers/gpu/drm/bridge/cdns-dsi.c | 485 +++
2 files changed, 60 insertions(+), 426 deletions(-)
diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
index 2fee47b0d50b..8840f396a7b6 100644
--- a/drivers/gpu/drm/bridge/Kconfig
+++ b/drivers/gpu/drm/bridge/Kconfig
@@ -30,6 +30,7 @@ config DRM_CDNS_DSI
select DRM_KMS_HELPER
select DRM_MIPI_DSI
select DRM_PANEL_BRIDGE
+ select GENERIC_PHY_MIPI_DPHY
depends on OF
help
Support Cadence DPI to DSI bridge. This is an internal
diff --git a/drivers/gpu/drm/bridge/cdns-dsi.c
b/drivers/gpu/drm/bridge/cdns-dsi.c
index 796874e76308..713a70b86046 100644
--- a/drivers/gpu/drm/bridge/cdns-dsi.c
+++ b/drivers/gpu/drm/bridge/cdns-dsi.c
@@ -21,6 +21,9 @@
#include
#include
+#include
+#include
+
#define IP_CONF0x0
#define SP_HS_FIFO_DEPTH(x)(((x) & GENMASK(30, 26)) >> 26)
#define SP_LP_FIFO_DEPTH(x)(((x) & GENMASK(25, 21)) >> 21)
@@ -419,44 +422,11 @@
#define DSI_NULL_FRAME_OVERHEAD6
#define DSI_EOT_PKT_SIZE 4
-#define REG_WAKEUP_TIME_NS 800
-#define DPHY_PLL_RATE_HZ 10800
-
-/* DPHY registers */
-#define DPHY_PMA_CMN(reg) (reg)
-#define DPHY_PMA_LCLK(reg) (0x100 + (reg))
-#define DPHY_PMA_LDATA(lane, reg) (0x200 + ((lane) * 0x100) + (reg))
-#define DPHY_PMA_RCLK(reg) (0x600 + (reg))
-#define DPHY_PMA_RDATA(lane, reg) (0x700 + ((lane) * 0x100) + (reg))
-#define DPHY_PCS(reg) (0xb00 + (reg))
-
-#define DPHY_CMN_SSM DPHY_PMA_CMN(0x20)
-#define DPHY_CMN_SSM_ENBIT(0)
-#define DPHY_CMN_TX_MODE_ENBIT(9)
-
-#define DPHY_CMN_PWM DPHY_PMA_CMN(0x40)
-#define DPHY_CMN_PWM_DIV(x)((x) << 20)
-#define DPHY_CMN_PWM_LOW(x)((x) << 10)
-#define DPHY_CMN_PWM_HIGH(x) (x)
-
-#define DPHY_CMN_FBDIV DPHY_PMA_CMN(0x4c)
-#define DPHY_CMN_FBDIV_VAL(low, high) (((high) << 11) | ((low) << 22))
-#define DPHY_CMN_FBDIV_FROM_REG(BIT(10) | BIT(21))
-
-#define DPHY_CMN_OPIPDIV DPHY_PMA_CMN(0x50)
-#define DPHY_CMN_IPDIV_FROM_REGBIT(0)
-#define DPHY_CMN_IPDIV(x) ((x) << 1)
-#define DPHY_CMN_OPDIV_FROM_REGBIT(6)
-#define DPHY_CMN_OPDIV(x) ((x) << 7)
-
-#define DPHY_PSM_CFG DPHY_PCS(0x4)
-#define DPHY_PSM_CFG_FROM_REG BIT(0)
-#define DPHY_PSM_CLK_DIV(x)((x) << 1)
-
struct cdns_dsi_output {
struct mipi_dsi_device *dev;
struct drm_panel *panel;
struct drm_bridge *bridge;
+ union phy_configure_opts phy_opts;
};
enum cdns_dsi_input_id {
@@ -465,14 +435,6 @@ enum cdns_dsi_input_id {
CDNS_DSC_INPUT,
};
-struct cdns_dphy_cfg {
- u8 pll_ipdiv;
- u8 pll_opdiv;
- u16 pll_fbdiv;
- unsigned long lane_bps;
- unsigned int nlanes;
-};
-
struct cdns_dsi_cfg {
unsigned int hfp;
unsigned int hsa;
@@ -481,34 +443,6 @@ struct cdns_dsi_cfg {
unsigned int htotal;
};
-struct cdns_dphy;
-
-enum cdns_dphy_clk_lane_cfg {
- DPHY_CLK_CFG_LEFT_DRIVES_ALL = 0,
- DPHY_CLK_CFG_LEFT_DRIVES_RIGHT = 1,
- DPHY_CLK_CFG_LEFT_DRIVES_LEFT = 2,
- DPHY_CLK_CFG_RIGHT_DRIVES_ALL = 3,
-};
-
-struct cdns_dphy_ops {
- int (*probe)(struct cdns_dphy *dphy);
- void (*remove)(struct cdns_dphy *dphy);
- void (*set_psm_div)(struct cdns_dphy *dphy, u8 div);
- void (*set_clk_lane_cfg)(struct cdns_dphy *dphy,
-enum cdns_dphy_clk_lane_cfg cfg);
- void (*set_pll_cfg)(struct cdns_dphy *dphy,
- const struct cdns_dphy_cfg *cfg);
- unsigned long (*get_wakeup_time_ns)(struct cdns_dphy *dphy);
-};
-
-struct cdns_dphy {
- struct cdns_dphy_cfg cfg;
- void __iomem *regs;
- struct clk *psm_clk;
- struct clk *pll_ref_clk;
- const struct cdns_dphy_ops *ops;
-};
-
struct cdns_dsi_input {
enum cdns_dsi_input_id id;
struct drm_bridge bridge;
@@ -526,7 +460,7 @@ struct cdns_dsi {
struct reset_control *dsi_p_rst;
struct clk *dsi_sys_clk;
bool link_initialized;
- struct cdns_dphy *dphy;
+ struct phy *dphy;
};
static inline struct cdns_dsi *input_to_dsi(struct cdns_dsi_input *input)
@@ -554,175 +488,6 @@ static unsigned int mode_to_dpi_hfp(const struct
drm_display_mode *mode,
re
[PATCH v5 6/9] drm/bridge: cdns: Separate DSI and D-PHY configuration
The current configuration of the DSI bridge and its associated D-PHY is
intertwined. In order to ease the future conversion to the phy framework
for the D-PHY part, let's split the configuration in two.
Signed-off-by: Maxime Ripard
---
drivers/gpu/drm/bridge/cdns-dsi.c | 101 ++-
1 file changed, 73 insertions(+), 28 deletions(-)
diff --git a/drivers/gpu/drm/bridge/cdns-dsi.c
b/drivers/gpu/drm/bridge/cdns-dsi.c
index ce9496d13986..796874e76308 100644
--- a/drivers/gpu/drm/bridge/cdns-dsi.c
+++ b/drivers/gpu/drm/bridge/cdns-dsi.c
@@ -545,6 +545,15 @@ bridge_to_cdns_dsi_input(struct drm_bridge *bridge)
return container_of(bridge, struct cdns_dsi_input, bridge);
}
+static unsigned int mode_to_dpi_hfp(const struct drm_display_mode *mode,
+ bool mode_valid_check)
+{
+ if (mode_valid_check)
+ return mode->hsync_start - mode->hdisplay;
+
+ return mode->crtc_hsync_start - mode->crtc_hdisplay;
+}
+
static int cdns_dsi_get_dphy_pll_cfg(struct cdns_dphy *dphy,
struct cdns_dphy_cfg *cfg,
unsigned int dpi_htotal,
@@ -731,14 +740,12 @@ static unsigned int dpi_to_dsi_timing(unsigned int
dpi_timing,
static int cdns_dsi_mode2cfg(struct cdns_dsi *dsi,
const struct drm_display_mode *mode,
struct cdns_dsi_cfg *dsi_cfg,
-struct cdns_dphy_cfg *dphy_cfg,
bool mode_valid_check)
{
- unsigned long dsi_htotal = 0, dsi_hss_hsa_hse_hbp = 0;
struct cdns_dsi_output *output = &dsi->output;
- unsigned int dsi_hfp_ext = 0, dpi_hfp, tmp;
+ unsigned int tmp;
bool sync_pulse = false;
- int bpp, nlanes, ret;
+ int bpp, nlanes;
memset(dsi_cfg, 0, sizeof(*dsi_cfg));
@@ -757,8 +764,6 @@ static int cdns_dsi_mode2cfg(struct cdns_dsi *dsi,
mode->crtc_hsync_end : mode->crtc_hsync_start);
dsi_cfg->hbp = dpi_to_dsi_timing(tmp, bpp, DSI_HBP_FRAME_OVERHEAD);
- dsi_htotal += dsi_cfg->hbp + DSI_HBP_FRAME_OVERHEAD;
- dsi_hss_hsa_hse_hbp += dsi_cfg->hbp + DSI_HBP_FRAME_OVERHEAD;
if (sync_pulse) {
if (mode_valid_check)
@@ -768,49 +773,91 @@ static int cdns_dsi_mode2cfg(struct cdns_dsi *dsi,
dsi_cfg->hsa = dpi_to_dsi_timing(tmp, bpp,
DSI_HSA_FRAME_OVERHEAD);
- dsi_htotal += dsi_cfg->hsa + DSI_HSA_FRAME_OVERHEAD;
- dsi_hss_hsa_hse_hbp += dsi_cfg->hsa + DSI_HSA_FRAME_OVERHEAD;
}
dsi_cfg->hact = dpi_to_dsi_timing(mode_valid_check ?
mode->hdisplay : mode->crtc_hdisplay,
bpp, 0);
- dsi_htotal += dsi_cfg->hact;
+ dsi_cfg->hfp = dpi_to_dsi_timing(mode_to_dpi_hfp(mode,
mode_valid_check),
+bpp, DSI_HFP_FRAME_OVERHEAD);
- if (mode_valid_check)
- dpi_hfp = mode->hsync_start - mode->hdisplay;
- else
- dpi_hfp = mode->crtc_hsync_start - mode->crtc_hdisplay;
+ return 0;
+}
+
+static int cdns_dphy_validate(struct cdns_dsi *dsi,
+ struct cdns_dsi_cfg *dsi_cfg,
+ struct cdns_dphy_cfg *dphy_cfg,
+ const struct drm_display_mode *mode,
+ bool mode_valid_check)
+{
+ struct cdns_dsi_output *output = &dsi->output;
+ unsigned long dsi_htotal;
+ unsigned int dsi_hfp_ext = 0;
+
+ int ret;
+
+ dsi_htotal = dsi_cfg->hbp + DSI_HBP_FRAME_OVERHEAD;
+ if (output->dev->mode_flags & MIPI_DSI_MODE_VIDEO_SYNC_PULSE)
+ dsi_htotal += dsi_cfg->hsa + DSI_HSA_FRAME_OVERHEAD;
- dsi_cfg->hfp = dpi_to_dsi_timing(dpi_hfp, bpp, DSI_HFP_FRAME_OVERHEAD);
+ dsi_htotal += dsi_cfg->hact;
dsi_htotal += dsi_cfg->hfp + DSI_HFP_FRAME_OVERHEAD;
if (mode_valid_check)
ret = cdns_dsi_get_dphy_pll_cfg(dsi->dphy, dphy_cfg,
- mode->htotal, bpp,
+ mode->htotal,
mode->clock * 1000,
- dsi_htotal, nlanes,
+
mipi_dsi_pixel_format_to_bpp(output->dev->format),
+ dsi_htotal,
+ output->dev->lanes,
&dsi_hfp_ext);
else
ret = cdns_dsi_get_dphy_pll_cfg(dsi->dphy, dphy_cfg,
[PATCH v5 8/9] phy: Add Cadence D-PHY support
Cadence has designed a D-PHY that can be used by the, currently in tree,
DSI bridge (DRM), CSI Transceiver and CSI Receiver (v4l2) drivers.
Only the DSI driver has an ad-hoc driver for that phy at the moment, while
the v4l2 drivers are completely missing any phy support. In order to make
that phy support available to all these drivers, without having to
duplicate that code three times, let's create a generic phy framework
driver.
Acked-by: Sakari Ailus
Signed-off-by: Maxime Ripard
---
drivers/phy/cadence/Kconfig | 13 +-
drivers/phy/cadence/Makefile| 1 +-
drivers/phy/cadence/cdns-dphy.c | 389 +-
3 files changed, 402 insertions(+), 1 deletion(-)
create mode 100644 drivers/phy/cadence/cdns-dphy.c
diff --git a/drivers/phy/cadence/Kconfig b/drivers/phy/cadence/Kconfig
index 2b8c0851ff33..31f18b67dd7c 100644
--- a/drivers/phy/cadence/Kconfig
+++ b/drivers/phy/cadence/Kconfig
@@ -1,6 +1,7 @@
#
# Phy drivers for Cadence PHYs
#
+
config PHY_CADENCE_DP
tristate "Cadence MHDP DisplayPort PHY driver"
depends on OF
@@ -9,9 +10,19 @@ config PHY_CADENCE_DP
help
Support for Cadence MHDP DisplayPort PHY.
+config PHY_CADENCE_DPHY
+ tristate "Cadence D-PHY Support"
+ depends on HAS_IOMEM && OF
+ select GENERIC_PHY
+ select GENERIC_PHY_MIPI_DPHY
+ help
+ Choose this option if you have a Cadence D-PHY in your
+ system. If M is selected, the module will be called
+ cdns-dphy.
+
config PHY_CADENCE_SIERRA
tristate "Cadence Sierra PHY Driver"
depends on OF && HAS_IOMEM && RESET_CONTROLLER
select GENERIC_PHY
help
- Enable this to support the Cadence Sierra PHY driver
\ No newline at end of file
+ Enable this to support the Cadence Sierra PHY driver
diff --git a/drivers/phy/cadence/Makefile b/drivers/phy/cadence/Makefile
index 412349af0492..2f9e3457b954 100644
--- a/drivers/phy/cadence/Makefile
+++ b/drivers/phy/cadence/Makefile
@@ -1,2 +1,3 @@
obj-$(CONFIG_PHY_CADENCE_DP) += phy-cadence-dp.o
+obj-$(CONFIG_PHY_CADENCE_DPHY) += cdns-dphy.o
obj-$(CONFIG_PHY_CADENCE_SIERRA) += phy-cadence-sierra.o
diff --git a/drivers/phy/cadence/cdns-dphy.c b/drivers/phy/cadence/cdns-dphy.c
new file mode 100644
index ..cde12b3aa4d4
--- /dev/null
+++ b/drivers/phy/cadence/cdns-dphy.c
@@ -0,0 +1,389 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright: 2017-2018 Cadence Design Systems, Inc.
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include
+#include
+
+#define REG_WAKEUP_TIME_NS 800
+#define DPHY_PLL_RATE_HZ 10800
+
+/* DPHY registers */
+#define DPHY_PMA_CMN(reg) (reg)
+#define DPHY_PMA_LCLK(reg) (0x100 + (reg))
+#define DPHY_PMA_LDATA(lane, reg) (0x200 + ((lane) * 0x100) + (reg))
+#define DPHY_PMA_RCLK(reg) (0x600 + (reg))
+#define DPHY_PMA_RDATA(lane, reg) (0x700 + ((lane) * 0x100) + (reg))
+#define DPHY_PCS(reg) (0xb00 + (reg))
+
+#define DPHY_CMN_SSM DPHY_PMA_CMN(0x20)
+#define DPHY_CMN_SSM_ENBIT(0)
+#define DPHY_CMN_TX_MODE_ENBIT(9)
+
+#define DPHY_CMN_PWM DPHY_PMA_CMN(0x40)
+#define DPHY_CMN_PWM_DIV(x)((x) << 20)
+#define DPHY_CMN_PWM_LOW(x)((x) << 10)
+#define DPHY_CMN_PWM_HIGH(x) (x)
+
+#define DPHY_CMN_FBDIV DPHY_PMA_CMN(0x4c)
+#define DPHY_CMN_FBDIV_VAL(low, high) (((high) << 11) | ((low) << 22))
+#define DPHY_CMN_FBDIV_FROM_REG(BIT(10) | BIT(21))
+
+#define DPHY_CMN_OPIPDIV DPHY_PMA_CMN(0x50)
+#define DPHY_CMN_IPDIV_FROM_REGBIT(0)
+#define DPHY_CMN_IPDIV(x) ((x) << 1)
+#define DPHY_CMN_OPDIV_FROM_REGBIT(6)
+#define DPHY_CMN_OPDIV(x) ((x) << 7)
+
+#define DPHY_PSM_CFG DPHY_PCS(0x4)
+#define DPHY_PSM_CFG_FROM_REG BIT(0)
+#define DPHY_PSM_CLK_DIV(x)((x) << 1)
+
+#define DSI_HBP_FRAME_OVERHEAD 12
+#define DSI_HSA_FRAME_OVERHEAD 14
+#define DSI_HFP_FRAME_OVERHEAD 6
+#define DSI_HSS_VSS_VSE_FRAME_OVERHEAD 4
+#define DSI_BLANKING_FRAME_OVERHEAD6
+#define DSI_NULL_FRAME_OVERHEAD6
+#define DSI_EOT_PKT_SIZE 4
+
+struct cdns_dphy_cfg {
+ u8 pll_ipdiv;
+ u8 pll_opdiv;
+ u16 pll_fbdiv;
+ unsigned int nlanes;
+};
+
+enum cdns_dphy_clk_lane_cfg {
+ DPHY_CLK_CFG_LEFT_DRIVES_ALL = 0,
+ DPHY_CLK_CFG_LEFT_DRIVES_RIGHT = 1,
+ DPHY_CLK_CFG_LEFT_DRIVES_LEFT = 2,
+ DPHY_CLK_CFG_RIGHT_DRIVES_ALL = 3,
+};
+
+struct cdns_dphy;
+struct cdns_dphy_ops {
+ int (*probe)(struct cdns_dphy *dphy);
+ void (*remove)(struct cdns_dphy *dphy);
Re: [PATCH v4 6/9] drm/bridge: cdns: Separate DSI and D-PHY configuration
On Thu, Jan 17, 2019 at 08:33:38AM -0500, Sean Paul wrote: > > @@ -768,49 +769,90 @@ static int cdns_dsi_mode2cfg(struct cdns_dsi *dsi, > > > > dsi_cfg->hsa = dpi_to_dsi_timing(tmp, bpp, > > DSI_HSA_FRAME_OVERHEAD); > > - dsi_htotal += dsi_cfg->hsa + DSI_HSA_FRAME_OVERHEAD; > > - dsi_hss_hsa_hse_hbp += dsi_cfg->hsa + DSI_HSA_FRAME_OVERHEAD; > > } > > > > dsi_cfg->hact = dpi_to_dsi_timing(mode_valid_check ? > > mode->hdisplay : mode->crtc_hdisplay, > > bpp, 0); > > - dsi_htotal += dsi_cfg->hact; > > + dsi_cfg->hfp = dpi_to_dsi_timing(mode_to_dpi_hfp(mode), bpp, > > +DSI_HFP_FRAME_OVERHEAD); > > We're throwing away the mode_valid_check switch here to flip between crtc_h* > value and h* value. Is that intentional? We're using it above for hdisplay, so > it's a bit confusing. ah, right, thanks for spotting this. I'll resend a version with those changes, thanks! Maxime -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com signature.asc Description: PGP signature
Re: [PATCH v4 8/9] phy: Add Cadence D-PHY support
Hi Sean, On Thu, Jan 17, 2019 at 08:53:22AM -0500, Sean Paul wrote: > On Wed, Jan 09, 2019 at 10:33:25AM +0100, Maxime Ripard wrote: > > + opts->wakeup = cdns_dphy_get_wakeup_time_ns(dphy) * 1000; > > This should be "/ 1000" since the units of wakeup is us now (thanks to patch > 2). > You've made this change in patch 9, so I think it's just a misplaced fixup. Good catch, this was fixed in patch 9, but it definitely belongs there. Thanks! Maxime -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com signature.asc Description: PGP signature
Re: [PATCH 1/3] media: dt: bindings: sunxi-ir: Add A64 compatible
Hi, I'm a bit late to the party, sorry for that. On Sat, Jan 12, 2019 at 09:56:11AM +0800, Chen-Yu Tsai wrote: > On Sat, Jan 12, 2019 at 1:30 AM Jernej Skrabec > wrote: > > > > A64 IR is compatible with A13, so add A64 compatible with A13 as a > > fallback. > > We ask people to add the SoC-specific compatible as a contigency, > in case things turn out to be not so "compatible". > > To be consistent with all the other SoCs and other peripherals, > unless you already spotted a "compatible" difference in the > hardware, i.e. the hardware isn't completely the same, this > patch isn't needed. On the other hand, if you did, please mention > the differences in the commit log. Even if we don't spot things, since we have the stable DT now, if we ever had that compatible in the DT from day 1, it's much easier to deal with. I'd really like to have that pattern for all the IPs even if we didn't spot any issue, since we can't really say that the datasheet are complete, and one can always make a mistake and overlook something. I'm fine with this version, and can apply it as is if we all agree. Maxime -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com signature.asc Description: PGP signature
Re: [PATCH v2 1/2] media: uapi: Add H264 low-level decoder API compound controls.
Hi,
On Wed, Jan 09, 2019 at 01:01:22AM +0800, ayaka wrote:
> On 1/8/19 5:52 PM, Randy 'ayaka' Li wrote:
> > On Thu, Nov 15, 2018 at 03:56:49PM +0100, Maxime Ripard wrote:
> > > From: Pawel Osciak
> > >
> > > Stateless video codecs will require both the H264 metadata and slices in
> > > order to be able to decode frames.
> > >
> > > This introduces the definitions for a new pixel format for H264 slices
> > > that
> > > have been parsed, as well as the structures used to pass the metadata from
> > > the userspace to the kernel.
> > >
> > > Co-Developed-by: Maxime Ripard
> > > Signed-off-by: Pawel Osciak
> > > Signed-off-by: Guenter Roeck
> > > Signed-off-by: Maxime Ripard
> > > ---
> > > Documentation/media/uapi/v4l/biblio.rst | 9 +
> > > .../media/uapi/v4l/extended-controls.rst | 364 ++
> > > .../media/uapi/v4l/pixfmt-compressed.rst | 20 +
> > > .../media/uapi/v4l/vidioc-queryctrl.rst | 30 ++
> > > .../media/videodev2.h.rst.exceptions | 5 +
> > > drivers/media/v4l2-core/v4l2-ctrls.c | 42 ++
> > > drivers/media/v4l2-core/v4l2-ioctl.c | 1 +
> > > include/media/v4l2-ctrls.h| 10 +
> > > include/uapi/linux/v4l2-controls.h| 166
> > > include/uapi/linux/videodev2.h| 11 +
> > > 10 files changed, 658 insertions(+)
> > > +#define V4L2_H264_DPB_ENTRY_FLAG_VALID 0x01
> > > +#define V4L2_H264_DPB_ENTRY_FLAG_ACTIVE 0x02
> > > +#define V4L2_H264_DPB_ENTRY_FLAG_LONG_TERM 0x04
> > > +
> > > +struct v4l2_h264_dpb_entry {
> > > + __u32 tag;
> > > + __u16 frame_num;
> > > + __u16 pic_num;
> > Although the long term reference would use picture order count
> > and short term for frame num, but only one of them is used
> > for a entry of a dpb.
> >
> > Besides, for a frame picture frame_num = pic_num * 2,
> > and frame_num = pic_num * 2 + 1 for a filed.
>
> I mistook something before and something Herman told me is wrong, I read the
> book explaining the ITU standard.
>
> The index of a short term reference picture would be frame_num or POC and
> LongTermPicNum for long term.
>
> But stateless hardware decoder usually don't care about whether it is long
> term or short term, as the real dpb updating or management work are not done
> by the the driver or device and decoding job would only use the two list(or
> one list for slice P) for reference pictures. So those flag for long term or
> status can be removed as well.
>
> Stateless decoder would care about just reference index of this picture and
> maybe some extra property for the filed coded below. Keeping a property here
> for the index of a picture is enough.
It doesn't look like it's part of the bitstream, the rockchip driver
seem like it's using the long term flags in the chromeos
driver. Tomasz, do you know why it's needed?
Maxime
--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
signature.asc
Description: PGP signature
Re: [PATCH v2 1/2] media: uapi: Add H264 low-level decoder API compound controls.
Hi, On Thu, Jan 10, 2019 at 09:33:01PM +0800, ayaka wrote: > I forget a important thing, for the rkvdec and rk hevc decoder, it would > requests cabac table, scaling list, picture parameter set and reference > picture storing in one or various of DMA buffers. I am not talking about the > data been parsed, the decoder would requests a raw data. > > For the pps and rps, it is possible to reuse the slice header, just let the > decoder know the offset from the bitstream bufer, I would suggest to add > three properties(with sps) for them. But I think we need a method to mark a > OUTPUT side buffer for those aux data. I'm not sure this is something we actually want. The whole design decision was that we wouldn't have a bitstream parser in the kernel, and doing as you suggest goes against that design. And either if it is something that turns out to be useful, this is really out of scope for this series. Maxime -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com signature.asc Description: PGP signature
Re: [PATCH v2 1/2] media: uapi: Add H264 low-level decoder API compound controls.
Hi,
On Tue, Jan 08, 2019 at 05:52:28PM +0800, Randy 'ayaka' Li wrote:
> > +struct v4l2_ctrl_h264_scaling_matrix {
> > + __u8 scaling_list_4x4[6][16];
> > + __u8 scaling_list_8x8[6][64];
> > +};
>
> I wonder which decoder want this.
I'm not sure I follow you, scaling lists are an important part of the
decoding process, so all of them?
> > +struct v4l2_ctrl_h264_slice_param {
> > + /* Size in bytes, including header */
> > + __u32 size;
> > + /* Offset in bits to slice_data() from the beginning of this slice. */
> > + __u32 header_bit_size;
> > +
> > + __u16 first_mb_in_slice;
> > + __u8 slice_type;
> > + __u8 pic_parameter_set_id;
> > + __u8 colour_plane_id;
> > + __u16 frame_num;
> > + __u16 idr_pic_id;
> > + __u16 pic_order_cnt_lsb;
> > + __s32 delta_pic_order_cnt_bottom;
> > + __s32 delta_pic_order_cnt0;
> > + __s32 delta_pic_order_cnt1;
> > + __u8 redundant_pic_cnt;
> > +
> > + struct v4l2_h264_pred_weight_table pred_weight_table;
> > + /* Size in bits of dec_ref_pic_marking() syntax element. */
> > + __u32 dec_ref_pic_marking_bit_size;
> > + /* Size in bits of pic order count syntax. */
> > + __u32 pic_order_cnt_bit_size;
> > +
> > + __u8 cabac_init_idc;
> > + __s8 slice_qp_delta;
> > + __s8 slice_qs_delta;
> > + __u8 disable_deblocking_filter_idc;
> > + __s8 slice_alpha_c0_offset_div2;
> > + __s8 slice_beta_offset_div2;
> > + __u32 slice_group_change_cycle;
> > +
> > + __u8 num_ref_idx_l0_active_minus1;
> > + __u8 num_ref_idx_l1_active_minus1;
> > + /* Entries on each list are indices
> > +* into v4l2_ctrl_h264_decode_param.dpb[]. */
> > + __u8 ref_pic_list0[32];
> > + __u8 ref_pic_list1[32];
> > +
> > + __u8 flags;
> > +};
> > +
> We need some addtional properties or the Rockchip won't work.
> 1. u16 idr_pic_id for identifies IDR (instantaneous decoding refresh)
> picture
idr_pic_id is already there
> 2. u16 ref_pic_mk_len for length of decoded reference picture marking bits
> 3. u8 poc_length for length of picture order count field in stream
>
> The last two are used for the hardware to skip a part stream.
I'm not sure what you mean here, those parameters are not in the
bitstream, what do you want to use them for?
> > +#define V4L2_H264_DPB_ENTRY_FLAG_VALID 0x01
> > +#define V4L2_H264_DPB_ENTRY_FLAG_ACTIVE0x02
> > +#define V4L2_H264_DPB_ENTRY_FLAG_LONG_TERM 0x04
> > +
> > +struct v4l2_h264_dpb_entry {
> > + __u32 tag;
> > + __u16 frame_num;
> > + __u16 pic_num;
>
> Although the long term reference would use picture order count
> and short term for frame num, but only one of them is used
> for a entry of a dpb.
>
> Besides, for a frame picture frame_num = pic_num * 2,
> and frame_num = pic_num * 2 + 1 for a filed.
I'm not sure what is your point?
> > + /* Note that field is indicated by v4l2_buffer.field */
> > + __s32 top_field_order_cnt;
> > + __s32 bottom_field_order_cnt;
> > + __u8 flags; /* V4L2_H264_DPB_ENTRY_FLAG_* */
> > +};
> > +
> > +struct v4l2_ctrl_h264_decode_param {
> > + __u32 num_slices;
> > + __u8 idr_pic_flag;
> > + __u8 nal_ref_idc;
> > + __s32 top_field_order_cnt;
> > + __s32 bottom_field_order_cnt;
> > + __u8 ref_pic_list_p0[32];
> > + __u8 ref_pic_list_b0[32];
> > + __u8 ref_pic_list_b1[32];
>
> I would prefer to keep only two list, list0 and list 1.
I'm not even sure why this is needed in the first place anymore. It's
not part of the bitstream, and it seems to come from ChromeOS' Rockchip driver
that uses it though. Do you know why?
Thanks!
Maxime
--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
signature.asc
Description: PGP signature
Re: [PATCH v5 2/6] media: sun6i: Add mod_rate quirk
On Fri, Jan 11, 2019 at 11:54:12AM +0530, Jagan Teki wrote:
> On Mon, Jan 7, 2019 at 6:59 PM Maxime Ripard
> wrote:
> > On Mon, Dec 24, 2018 at 08:57:48PM +0530, Jagan Teki wrote:
> > > On Fri, Dec 21, 2018 at 6:30 PM Maxime Ripard
> > > wrote:
> > > >
> > > > On Thu, Dec 20, 2018 at 06:24:34PM +0530, Jagan Teki wrote:
> > > > > Unfortunately default CSI_SCLK rate cannot work properly to
> > > > > drive the connected sensor interface, particularly on few
> > > > > Allwinner SoC's like A64.
> > > > >
> > > > > So, add mod_rate quirk via driver data so-that the respective
> > > > > SoC's which require to alter the default mod clock rate can assign
> > > > > the operating clock rate.
> > > > >
> > > > > Signed-off-by: Jagan Teki
> > > > > ---
> > > > > .../platform/sunxi/sun6i-csi/sun6i_csi.c | 34
> > > > > +++
> > > > > 1 file changed, 28 insertions(+), 6 deletions(-)
> > > > >
> > > > > diff --git a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c
> > > > > b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c
> > > > > index ee882b66a5ea..fe002beae09c 100644
> > > > > --- a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c
> > > > > +++ b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c
> > > > > @@ -15,6 +15,7 @@
> > > > > #include
> > > > > #include
> > > > > #include
> > > > > +#include
> > > > > #include
> > > > > #include
> > > > > #include
> > > > > @@ -28,8 +29,13 @@
> > > > >
> > > > > #define MODULE_NAME "sun6i-csi"
> > > > >
> > > > > +struct sun6i_csi_variant {
> > > > > + unsigned long mod_rate;
> > > > > +};
> > > > > +
> > > > > struct sun6i_csi_dev {
> > > > > struct sun6i_csicsi;
> > > > > + const struct sun6i_csi_variant *variant;
> > > > > struct device *dev;
> > > > >
> > > > > struct regmap *regmap;
> > > > > @@ -822,33 +828,43 @@ static int sun6i_csi_resource_request(struct
> > > > > sun6i_csi_dev *sdev,
> > > > > return PTR_ERR(sdev->clk_mod);
> > > > > }
> > > > >
> > > > > + if (sdev->variant->mod_rate)
> > > > > + clk_set_rate_exclusive(sdev->clk_mod,
> > > > > sdev->variant->mod_rate);
> > > > > +
> > > >
> > > > It still doesn't make any sense to do it in the probe function...
> > >
> > > I'm not sure we discussed about the context wrt probe, we discussed
> > > about exclusive put clock.
> >
> > https://lkml.org/lkml/2018/12/18/584
> >
> > "Doing it here is not really optimal either, since you'll put a
> > constraint on the system (maintaining that clock at 300MHz), while
> > it's not in use."
>
> But this constraint is only set, for SoC's who need mod_rate change
> not for whole SoCs.
Still, that constraint is there for the whole system on affected
SoCs. Whether it applies to one SoC or not is not really relevant.
Maxime
--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
signature.asc
Description: PGP signature
[PATCH v4 0/9] phy: Add configuration interface for MIPI D-PHY devices
Hi, Here is a set of patches to allow the phy framework consumers to test and apply runtime configurations. This is needed to support more phy classes that require tuning based on parameters depending on the current use case of the device, in addition to the power state management already provided by the current functions. A first test bed for that API are the MIPI D-PHY devices. There's a number of solutions that have been used so far to support these phy, most of the time being an ad-hoc driver in the consumer. That approach has a big shortcoming though, which is that this is quite difficult to deal with consumers integrated with multiple variants of phy, of multiple consumers integrated with the same phy. The latter case can be found in the Cadence DSI bridge, and the CSI transceiver and receivers. All of them are integrated with the same phy, or can be integrated with different phy, depending on the implementation. I've looked at all the MIPI DSI drivers I could find, and gathered all the parameters I could find. The interface should be complete, and most of the drivers can be converted in the future. The current set converts two of them: the above mentionned Cadence DSI driver so that the v4l2 drivers can use them, and the Allwinner MIPI-DSI driver. Let me know what you think, Maxime Changes from v3 - Rebased on 5.0-rc1 - Added the fixes suggested by Sakari Changes from v2: - Rebased on next - Changed the interface to accomodate for the new submodes - Changed the timings units from nanoseconds to picoseconds - Added minimum and maximum boundaries to the documentation - Moved the clock enabling to phy_power_on in the Cadence DPHY driver - Exported the phy_configure and phy_validate symbols - Rework the phy pll divider computation in the cadence dphy driver Changes from v1: - Rebased on top of 4.20-rc1 - Removed the bus mode and timings parameters from the MIPI D-PHY parameters, since that shouldn't have any impact on the PHY itself. - Reworked the Cadence DSI and D-PHY drivers to take this into account. - Remove the mode parameter from phy_configure - Added phy_configure and phy_validate stubs - Return -EOPNOTSUPP in phy_configure and phy_validate when the operation is not implemented Maxime Ripard (9): phy: dphy: Remove unused header phy: dphy: Change units of wakeup and init parameters phy: dphy: Clarify lanes parameter documentation sun6i: dsi: Convert to generic phy handling phy: Move Allwinner A31 D-PHY driver to drivers/phy/ drm/bridge: cdns: Separate DSI and D-PHY configuration dt-bindings: phy: Move the Cadence D-PHY bindings phy: Add Cadence D-PHY support drm/bridge: cdns: Convert to phy framework Documentation/devicetree/bindings/display/bridge/cdns,dsi.txt | 21 +- Documentation/devicetree/bindings/phy/cdns,dphy.txt | 20 +- drivers/gpu/drm/bridge/Kconfig| 1 +- drivers/gpu/drm/bridge/cdns-dsi.c | 535 +-- drivers/gpu/drm/sun4i/Kconfig | 3 +- drivers/gpu/drm/sun4i/Makefile| 5 +- drivers/gpu/drm/sun4i/sun6i_mipi_dphy.c | 292 + drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c| 31 +- drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h| 17 +- drivers/phy/allwinner/Kconfig | 12 +- drivers/phy/allwinner/Makefile| 1 +- drivers/phy/allwinner/phy-sun6i-mipi-dphy.c | 318 - drivers/phy/cadence/Kconfig | 13 +- drivers/phy/cadence/Makefile | 1 +- drivers/phy/cadence/cdns-dphy.c | 389 +- drivers/phy/phy-core-mipi-dphy.c | 8 +- include/linux/phy/phy-mipi-dphy.h | 13 +- 17 files changed, 890 insertions(+), 790 deletions(-) create mode 100644 Documentation/devicetree/bindings/phy/cdns,dphy.txt delete mode 100644 drivers/gpu/drm/sun4i/sun6i_mipi_dphy.c create mode 100644 drivers/phy/allwinner/phy-sun6i-mipi-dphy.c create mode 100644 drivers/phy/cadence/cdns-dphy.c base-commit: bfeffd155283772bbe78c6a05dec7c0128ee500c -- git-series 0.9.1
[PATCH v4 1/9] phy: dphy: Remove unused header
The videomode.h header inclusion is an artifact from the patches development, remove it. Suggested-by: Sakari Ailus Signed-off-by: Maxime Ripard --- include/linux/phy/phy-mipi-dphy.h | 2 -- 1 file changed, 2 deletions(-) diff --git a/include/linux/phy/phy-mipi-dphy.h b/include/linux/phy/phy-mipi-dphy.h index c08aacc0ac35..9cf97cd1d303 100644 --- a/include/linux/phy/phy-mipi-dphy.h +++ b/include/linux/phy/phy-mipi-dphy.h @@ -6,8 +6,6 @@ #ifndef __PHY_MIPI_DPHY_H_ #define __PHY_MIPI_DPHY_H_ -#include - /** * struct phy_configure_opts_mipi_dphy - MIPI D-PHY configuration set * -- git-series 0.9.1
[PATCH v4 3/9] phy: dphy: Clarify lanes parameter documentation
The lanes parameter is not solely about the number of lanes, but it also
carries the fact that those are the first lanes in use during the
transmission.
It was implicit so far, so make sure it's explicit now.
Suggested-by: Sakari Ailus
Signed-off-by: Maxime Ripard
---
include/linux/phy/phy-mipi-dphy.h | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/include/linux/phy/phy-mipi-dphy.h
b/include/linux/phy/phy-mipi-dphy.h
index 627d28080d3a..a877ffee845d 100644
--- a/include/linux/phy/phy-mipi-dphy.h
+++ b/include/linux/phy/phy-mipi-dphy.h
@@ -269,7 +269,8 @@ struct phy_configure_opts_mipi_dphy {
/**
* @lanes:
*
-* Number of active data lanes used for the transmissions.
+* Number of active, consecutive, data lanes, starting from
+* lane 0, used for the transmissions.
*/
unsigned char lanes;
};
--
git-series 0.9.1
[PATCH v4 4/9] sun6i: dsi: Convert to generic phy handling
Now that we have everything in place in the PHY framework to deal in a
generic way with MIPI D-PHY phys, let's convert our PHY driver and its
associated DSI driver to that new API.
Signed-off-by: Maxime Ripard
---
drivers/gpu/drm/sun4i/Kconfig | 11 +-
drivers/gpu/drm/sun4i/Makefile | 6 +-
drivers/gpu/drm/sun4i/sun6i_mipi_dphy.c | 164 ++---
drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 31 ++---
drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h | 17 +---
5 files changed, 126 insertions(+), 103 deletions(-)
diff --git a/drivers/gpu/drm/sun4i/Kconfig b/drivers/gpu/drm/sun4i/Kconfig
index c2c042287c19..2b8db82c4bab 100644
--- a/drivers/gpu/drm/sun4i/Kconfig
+++ b/drivers/gpu/drm/sun4i/Kconfig
@@ -45,10 +45,19 @@ config DRM_SUN6I_DSI
default MACH_SUN8I
select CRC_CCITT
select DRM_MIPI_DSI
+ select DRM_SUN6I_DPHY
help
Choose this option if you want have an Allwinner SoC with
MIPI-DSI support. If M is selected the module will be called
- sun6i-dsi
+ sun6i_mipi_dsi.
+
+config DRM_SUN6I_DPHY
+ tristate "Allwinner A31 MIPI D-PHY Support"
+ select GENERIC_PHY_MIPI_DPHY
+ help
+ Choose this option if you have an Allwinner SoC with
+ MIPI-DSI support. If M is selected, the module will be
+ called sun6i_mipi_dphy.
config DRM_SUN8I_DW_HDMI
tristate "Support for Allwinner version of DesignWare HDMI"
diff --git a/drivers/gpu/drm/sun4i/Makefile b/drivers/gpu/drm/sun4i/Makefile
index 0eb38ac8e86e..1e2320d824b5 100644
--- a/drivers/gpu/drm/sun4i/Makefile
+++ b/drivers/gpu/drm/sun4i/Makefile
@@ -24,9 +24,6 @@ sun4i-tcon-y += sun4i_lvds.o
sun4i-tcon-y += sun4i_tcon.o
sun4i-tcon-y += sun4i_rgb.o
-sun6i-dsi-y+= sun6i_mipi_dphy.o
-sun6i-dsi-y+= sun6i_mipi_dsi.o
-
obj-$(CONFIG_DRM_SUN4I)+= sun4i-drm.o
obj-$(CONFIG_DRM_SUN4I)+= sun4i-tcon.o
obj-$(CONFIG_DRM_SUN4I)+= sun4i_tv.o
@@ -37,7 +34,8 @@ ifdef CONFIG_DRM_SUN4I_BACKEND
obj-$(CONFIG_DRM_SUN4I)+= sun4i-frontend.o
endif
obj-$(CONFIG_DRM_SUN4I_HDMI) += sun4i-drm-hdmi.o
-obj-$(CONFIG_DRM_SUN6I_DSI)+= sun6i-dsi.o
+obj-$(CONFIG_DRM_SUN6I_DPHY) += sun6i_mipi_dphy.o
+obj-$(CONFIG_DRM_SUN6I_DSI)+= sun6i_mipi_dsi.o
obj-$(CONFIG_DRM_SUN8I_DW_HDMI)+= sun8i-drm-hdmi.o
obj-$(CONFIG_DRM_SUN8I_MIXER) += sun8i-mixer.o
obj-$(CONFIG_DRM_SUN8I_TCON_TOP) += sun8i_tcon_top.o
diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dphy.c
b/drivers/gpu/drm/sun4i/sun6i_mipi_dphy.c
index e4d19431fa0e..79c8af5c7c1d 100644
--- a/drivers/gpu/drm/sun4i/sun6i_mipi_dphy.c
+++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dphy.c
@@ -8,11 +8,14 @@
#include
#include
+#include
#include
+#include
#include
#include
-#include "sun6i_mipi_dsi.h"
+#include
+#include
#define SUN6I_DPHY_GCTL_REG0x00
#define SUN6I_DPHY_GCTL_LANE_NUM(n)n) - 1) & 3) << 4)
@@ -81,12 +84,46 @@
#define SUN6I_DPHY_DBG5_REG0xf4
-int sun6i_dphy_init(struct sun6i_dphy *dphy, unsigned int lanes)
+struct sun6i_dphy {
+ struct clk *bus_clk;
+ struct clk *mod_clk;
+ struct regmap *regs;
+ struct reset_control*reset;
+
+ struct phy *phy;
+ struct phy_configure_opts_mipi_dphy config;
+};
+
+static int sun6i_dphy_init(struct phy *phy)
{
+ struct sun6i_dphy *dphy = phy_get_drvdata(phy);
+
reset_control_deassert(dphy->reset);
clk_prepare_enable(dphy->mod_clk);
clk_set_rate_exclusive(dphy->mod_clk, 15000);
+ return 0;
+}
+
+static int sun6i_dphy_configure(struct phy *phy, union phy_configure_opts
*opts)
+{
+ struct sun6i_dphy *dphy = phy_get_drvdata(phy);
+ int ret;
+
+ ret = phy_mipi_dphy_config_validate(&opts->mipi_dphy);
+ if (ret)
+ return ret;
+
+ memcpy(&dphy->config, opts, sizeof(dphy->config));
+
+ return 0;
+}
+
+static int sun6i_dphy_power_on(struct phy *phy)
+{
+ struct sun6i_dphy *dphy = phy_get_drvdata(phy);
+ u8 lanes_mask = GENMASK(dphy->config.lanes - 1, 0);
+
regmap_write(dphy->regs, SUN6I_DPHY_TX_CTL_REG,
SUN6I_DPHY_TX_CTL_HS_TX_CLK_CONT);
@@ -111,16 +148,9 @@ int sun6i_dphy_init(struct sun6i_dphy *dphy, unsigned int
lanes)
SUN6I_DPHY_TX_TIME4_HS_TX_ANA1(3));
regmap_write(dphy->regs, SUN6I_DPHY_GCTL_REG,
-SUN6I_DPHY_GCTL_LANE_NUM(lanes) |
+SUN6I_DPHY_GCTL_LANE_NUM(dphy->config.lanes) |
SUN6I_DPHY_GCTL_EN);
- return 0;
-}
-
-int sun6i_dphy_power_on(struct su
[PATCH v4 9/9] drm/bridge: cdns: Convert to phy framework
Now that we have everything we need in the phy framework to allow to tune
the phy parameters, let's convert the Cadence DSI bridge to that API
instead of creating a ad-hoc driver for its phy.
Signed-off-by: Maxime Ripard
---
drivers/gpu/drm/bridge/Kconfig| 1 +-
drivers/gpu/drm/bridge/cdns-dsi.c | 485 +++
drivers/phy/cadence/cdns-dphy.c | 2 +-
3 files changed, 61 insertions(+), 427 deletions(-)
diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
index 2fee47b0d50b..8840f396a7b6 100644
--- a/drivers/gpu/drm/bridge/Kconfig
+++ b/drivers/gpu/drm/bridge/Kconfig
@@ -30,6 +30,7 @@ config DRM_CDNS_DSI
select DRM_KMS_HELPER
select DRM_MIPI_DSI
select DRM_PANEL_BRIDGE
+ select GENERIC_PHY_MIPI_DPHY
depends on OF
help
Support Cadence DPI to DSI bridge. This is an internal
diff --git a/drivers/gpu/drm/bridge/cdns-dsi.c
b/drivers/gpu/drm/bridge/cdns-dsi.c
index 3ac6dd524b6d..7b432257ffbe 100644
--- a/drivers/gpu/drm/bridge/cdns-dsi.c
+++ b/drivers/gpu/drm/bridge/cdns-dsi.c
@@ -21,6 +21,9 @@
#include
#include
+#include
+#include
+
#define IP_CONF0x0
#define SP_HS_FIFO_DEPTH(x)(((x) & GENMASK(30, 26)) >> 26)
#define SP_LP_FIFO_DEPTH(x)(((x) & GENMASK(25, 21)) >> 21)
@@ -419,44 +422,11 @@
#define DSI_NULL_FRAME_OVERHEAD6
#define DSI_EOT_PKT_SIZE 4
-#define REG_WAKEUP_TIME_NS 800
-#define DPHY_PLL_RATE_HZ 10800
-
-/* DPHY registers */
-#define DPHY_PMA_CMN(reg) (reg)
-#define DPHY_PMA_LCLK(reg) (0x100 + (reg))
-#define DPHY_PMA_LDATA(lane, reg) (0x200 + ((lane) * 0x100) + (reg))
-#define DPHY_PMA_RCLK(reg) (0x600 + (reg))
-#define DPHY_PMA_RDATA(lane, reg) (0x700 + ((lane) * 0x100) + (reg))
-#define DPHY_PCS(reg) (0xb00 + (reg))
-
-#define DPHY_CMN_SSM DPHY_PMA_CMN(0x20)
-#define DPHY_CMN_SSM_ENBIT(0)
-#define DPHY_CMN_TX_MODE_ENBIT(9)
-
-#define DPHY_CMN_PWM DPHY_PMA_CMN(0x40)
-#define DPHY_CMN_PWM_DIV(x)((x) << 20)
-#define DPHY_CMN_PWM_LOW(x)((x) << 10)
-#define DPHY_CMN_PWM_HIGH(x) (x)
-
-#define DPHY_CMN_FBDIV DPHY_PMA_CMN(0x4c)
-#define DPHY_CMN_FBDIV_VAL(low, high) (((high) << 11) | ((low) << 22))
-#define DPHY_CMN_FBDIV_FROM_REG(BIT(10) | BIT(21))
-
-#define DPHY_CMN_OPIPDIV DPHY_PMA_CMN(0x50)
-#define DPHY_CMN_IPDIV_FROM_REGBIT(0)
-#define DPHY_CMN_IPDIV(x) ((x) << 1)
-#define DPHY_CMN_OPDIV_FROM_REGBIT(6)
-#define DPHY_CMN_OPDIV(x) ((x) << 7)
-
-#define DPHY_PSM_CFG DPHY_PCS(0x4)
-#define DPHY_PSM_CFG_FROM_REG BIT(0)
-#define DPHY_PSM_CLK_DIV(x)((x) << 1)
-
struct cdns_dsi_output {
struct mipi_dsi_device *dev;
struct drm_panel *panel;
struct drm_bridge *bridge;
+ union phy_configure_opts phy_opts;
};
enum cdns_dsi_input_id {
@@ -465,14 +435,6 @@ enum cdns_dsi_input_id {
CDNS_DSC_INPUT,
};
-struct cdns_dphy_cfg {
- u8 pll_ipdiv;
- u8 pll_opdiv;
- u16 pll_fbdiv;
- unsigned long lane_bps;
- unsigned int nlanes;
-};
-
struct cdns_dsi_cfg {
unsigned int hfp;
unsigned int hsa;
@@ -481,34 +443,6 @@ struct cdns_dsi_cfg {
unsigned int htotal;
};
-struct cdns_dphy;
-
-enum cdns_dphy_clk_lane_cfg {
- DPHY_CLK_CFG_LEFT_DRIVES_ALL = 0,
- DPHY_CLK_CFG_LEFT_DRIVES_RIGHT = 1,
- DPHY_CLK_CFG_LEFT_DRIVES_LEFT = 2,
- DPHY_CLK_CFG_RIGHT_DRIVES_ALL = 3,
-};
-
-struct cdns_dphy_ops {
- int (*probe)(struct cdns_dphy *dphy);
- void (*remove)(struct cdns_dphy *dphy);
- void (*set_psm_div)(struct cdns_dphy *dphy, u8 div);
- void (*set_clk_lane_cfg)(struct cdns_dphy *dphy,
-enum cdns_dphy_clk_lane_cfg cfg);
- void (*set_pll_cfg)(struct cdns_dphy *dphy,
- const struct cdns_dphy_cfg *cfg);
- unsigned long (*get_wakeup_time_ns)(struct cdns_dphy *dphy);
-};
-
-struct cdns_dphy {
- struct cdns_dphy_cfg cfg;
- void __iomem *regs;
- struct clk *psm_clk;
- struct clk *pll_ref_clk;
- const struct cdns_dphy_ops *ops;
-};
-
struct cdns_dsi_input {
enum cdns_dsi_input_id id;
struct drm_bridge bridge;
@@ -526,7 +460,7 @@ struct cdns_dsi {
struct reset_control *dsi_p_rst;
struct clk *dsi_sys_clk;
bool link_initialized;
- struct cdns_dphy *dphy;
+ struct phy *dphy;
};
static inline struct cdns_dsi *input_to_dsi(struct cdns_dsi_input *input)
@@ -550,175 +484,6 @@ static unsigned int mode_to_dpi_hfp(const struct
d
[PATCH v4 7/9] dt-bindings: phy: Move the Cadence D-PHY bindings
The Cadence D-PHY bindings was defined as part of the DSI block so far.
However, since it's now going to be a separate driver, we need to move the
binding to a file of its own.
Signed-off-by: Maxime Ripard
---
Documentation/devicetree/bindings/display/bridge/cdns,dsi.txt | 21 +---
Documentation/devicetree/bindings/phy/cdns,dphy.txt | 20 +++-
2 files changed, 20 insertions(+), 21 deletions(-)
create mode 100644 Documentation/devicetree/bindings/phy/cdns,dphy.txt
diff --git a/Documentation/devicetree/bindings/display/bridge/cdns,dsi.txt
b/Documentation/devicetree/bindings/display/bridge/cdns,dsi.txt
index f5725bb6c61c..525a4bfd8634 100644
--- a/Documentation/devicetree/bindings/display/bridge/cdns,dsi.txt
+++ b/Documentation/devicetree/bindings/display/bridge/cdns,dsi.txt
@@ -31,28 +31,7 @@ Required subnodes:
- one subnode per DSI device connected on the DSI bus. Each DSI device should
contain a reg property encoding its virtual channel.
-Cadence DPHY
-
-
-Cadence DPHY block.
-
-Required properties:
-- compatible: should be set to "cdns,dphy".
-- reg: physical base address and length of the DPHY registers.
-- clocks: DPHY reference clocks.
-- clock-names: must contain "psm" and "pll_ref".
-- #phy-cells: must be set to 0.
-
-
Example:
- dphy0: dphy@fd0e{
- compatible = "cdns,dphy";
- reg = <0x0 0xfd0e 0x0 0x1000>;
- clocks = <&psm_clk>, <&pll_ref_clk>;
- clock-names = "psm", "pll_ref";
- #phy-cells = <0>;
- };
-
dsi0: dsi@fd0c {
compatible = "cdns,dsi";
reg = <0x0 0xfd0c 0x0 0x1000>;
diff --git a/Documentation/devicetree/bindings/phy/cdns,dphy.txt
b/Documentation/devicetree/bindings/phy/cdns,dphy.txt
new file mode 100644
index ..1095bc4e72d9
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/cdns,dphy.txt
@@ -0,0 +1,20 @@
+Cadence DPHY
+
+
+Cadence DPHY block.
+
+Required properties:
+- compatible: should be set to "cdns,dphy".
+- reg: physical base address and length of the DPHY registers.
+- clocks: DPHY reference clocks.
+- clock-names: must contain "psm" and "pll_ref".
+- #phy-cells: must be set to 0.
+
+Example:
+ dphy0: dphy@fd0e{
+ compatible = "cdns,dphy";
+ reg = <0x0 0xfd0e 0x0 0x1000>;
+ clocks = <&psm_clk>, <&pll_ref_clk>;
+ clock-names = "psm", "pll_ref";
+ #phy-cells = <0>;
+ };
--
git-series 0.9.1
[PATCH v4 5/9] phy: Move Allwinner A31 D-PHY driver to drivers/phy/
Now that our MIPI D-PHY driver has been converted to the phy framework, let's move it into the drivers/phy directory. Signed-off-by: Maxime Ripard --- drivers/gpu/drm/sun4i/Kconfig | 10 +- drivers/gpu/drm/sun4i/Makefile | 1 +- drivers/gpu/drm/sun4i/sun6i_mipi_dphy.c | 318 +- drivers/phy/allwinner/Kconfig | 12 +- drivers/phy/allwinner/Makefile | 1 +- drivers/phy/allwinner/phy-sun6i-mipi-dphy.c | 318 +- 6 files changed, 332 insertions(+), 328 deletions(-) delete mode 100644 drivers/gpu/drm/sun4i/sun6i_mipi_dphy.c create mode 100644 drivers/phy/allwinner/phy-sun6i-mipi-dphy.c diff --git a/drivers/gpu/drm/sun4i/Kconfig b/drivers/gpu/drm/sun4i/Kconfig index 2b8db82c4bab..1dbbc3a1b763 100644 --- a/drivers/gpu/drm/sun4i/Kconfig +++ b/drivers/gpu/drm/sun4i/Kconfig @@ -45,20 +45,12 @@ config DRM_SUN6I_DSI default MACH_SUN8I select CRC_CCITT select DRM_MIPI_DSI - select DRM_SUN6I_DPHY + select PHY_SUN6I_MIPI_DPHY help Choose this option if you want have an Allwinner SoC with MIPI-DSI support. If M is selected the module will be called sun6i_mipi_dsi. -config DRM_SUN6I_DPHY - tristate "Allwinner A31 MIPI D-PHY Support" - select GENERIC_PHY_MIPI_DPHY - help - Choose this option if you have an Allwinner SoC with - MIPI-DSI support. If M is selected, the module will be - called sun6i_mipi_dphy. - config DRM_SUN8I_DW_HDMI tristate "Support for Allwinner version of DesignWare HDMI" depends on DRM_SUN4I diff --git a/drivers/gpu/drm/sun4i/Makefile b/drivers/gpu/drm/sun4i/Makefile index 1e2320d824b5..0d04f2447b01 100644 --- a/drivers/gpu/drm/sun4i/Makefile +++ b/drivers/gpu/drm/sun4i/Makefile @@ -34,7 +34,6 @@ ifdef CONFIG_DRM_SUN4I_BACKEND obj-$(CONFIG_DRM_SUN4I)+= sun4i-frontend.o endif obj-$(CONFIG_DRM_SUN4I_HDMI) += sun4i-drm-hdmi.o -obj-$(CONFIG_DRM_SUN6I_DPHY) += sun6i_mipi_dphy.o obj-$(CONFIG_DRM_SUN6I_DSI)+= sun6i_mipi_dsi.o obj-$(CONFIG_DRM_SUN8I_DW_HDMI)+= sun8i-drm-hdmi.o obj-$(CONFIG_DRM_SUN8I_MIXER) += sun8i-mixer.o diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dphy.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dphy.c deleted file mode 100644 index 79c8af5c7c1d.. --- a/drivers/gpu/drm/sun4i/sun6i_mipi_dphy.c +++ /dev/null @@ -1,318 +0,0 @@ -// SPDX-License-Identifier: GPL-2.0+ -/* - * Copyright (c) 2016 Allwinnertech Co., Ltd. - * Copyright (C) 2017-2018 Bootlin - * - * Maxime Ripard - */ - -#include -#include -#include -#include -#include -#include -#include - -#include -#include - -#define SUN6I_DPHY_GCTL_REG0x00 -#define SUN6I_DPHY_GCTL_LANE_NUM(n)n) - 1) & 3) << 4) -#define SUN6I_DPHY_GCTL_EN BIT(0) - -#define SUN6I_DPHY_TX_CTL_REG 0x04 -#define SUN6I_DPHY_TX_CTL_HS_TX_CLK_CONT BIT(28) - -#define SUN6I_DPHY_TX_TIME0_REG0x10 -#define SUN6I_DPHY_TX_TIME0_HS_TRAIL(n)(((n) & 0xff) << 24) -#define SUN6I_DPHY_TX_TIME0_HS_PREPARE(n) (((n) & 0xff) << 16) -#define SUN6I_DPHY_TX_TIME0_LP_CLK_DIV(n) ((n) & 0xff) - -#define SUN6I_DPHY_TX_TIME1_REG0x14 -#define SUN6I_DPHY_TX_TIME1_CLK_POST(n)(((n) & 0xff) << 24) -#define SUN6I_DPHY_TX_TIME1_CLK_PRE(n) (((n) & 0xff) << 16) -#define SUN6I_DPHY_TX_TIME1_CLK_ZERO(n)(((n) & 0xff) << 8) -#define SUN6I_DPHY_TX_TIME1_CLK_PREPARE(n) ((n) & 0xff) - -#define SUN6I_DPHY_TX_TIME2_REG0x18 -#define SUN6I_DPHY_TX_TIME2_CLK_TRAIL(n) ((n) & 0xff) - -#define SUN6I_DPHY_TX_TIME3_REG0x1c - -#define SUN6I_DPHY_TX_TIME4_REG0x20 -#define SUN6I_DPHY_TX_TIME4_HS_TX_ANA1(n) (((n) & 0xff) << 8) -#define SUN6I_DPHY_TX_TIME4_HS_TX_ANA0(n) ((n) & 0xff) - -#define SUN6I_DPHY_ANA0_REG0x4c -#define SUN6I_DPHY_ANA0_REG_PWSBIT(31) -#define SUN6I_DPHY_ANA0_REG_DMPC BIT(28) -#define SUN6I_DPHY_ANA0_REG_DMPD(n)(((n) & 0xf) << 24) -#define SUN6I_DPHY_ANA0_REG_SLV(n) (((n) & 7) << 12) -#define SUN6I_DPHY_ANA0_REG_DEN(n) (((n) & 0xf) << 8) - -#define SUN6I_DPHY_ANA1_REG0x50 -#define SUN6I_DPHY_ANA1_REG_VTTMODEBIT(31) -#define SUN6I_DPHY_ANA1_REG_CSMPS(n) (((n) & 3) << 28) -#define SUN6I_DPHY_ANA1_REG_SVTT(n)(((n) & 0xf) << 24) - -#define SUN6I_DPHY_ANA2_REG0x54 -#define SUN6I_DPHY_ANA2_EN_P2S_CPU(n) (((n) & 0xf) << 24) -#define SUN6I_DPHY_ANA2_EN_P2S_CPU_MASKGENMASK(27, 24) -#define SUN6I_DPHY_ANA2_EN_CK_CPU BIT(4) -#defin
[PATCH v4 8/9] phy: Add Cadence D-PHY support
Cadence has designed a D-PHY that can be used by the, currently in tree,
DSI bridge (DRM), CSI Transceiver and CSI Receiver (v4l2) drivers.
Only the DSI driver has an ad-hoc driver for that phy at the moment, while
the v4l2 drivers are completely missing any phy support. In order to make
that phy support available to all these drivers, without having to
duplicate that code three times, let's create a generic phy framework
driver.
Signed-off-by: Maxime Ripard
---
drivers/phy/cadence/Kconfig | 13 +-
drivers/phy/cadence/Makefile| 1 +-
drivers/phy/cadence/cdns-dphy.c | 389 +-
3 files changed, 402 insertions(+), 1 deletion(-)
create mode 100644 drivers/phy/cadence/cdns-dphy.c
diff --git a/drivers/phy/cadence/Kconfig b/drivers/phy/cadence/Kconfig
index 2b8c0851ff33..31f18b67dd7c 100644
--- a/drivers/phy/cadence/Kconfig
+++ b/drivers/phy/cadence/Kconfig
@@ -1,6 +1,7 @@
#
# Phy drivers for Cadence PHYs
#
+
config PHY_CADENCE_DP
tristate "Cadence MHDP DisplayPort PHY driver"
depends on OF
@@ -9,9 +10,19 @@ config PHY_CADENCE_DP
help
Support for Cadence MHDP DisplayPort PHY.
+config PHY_CADENCE_DPHY
+ tristate "Cadence D-PHY Support"
+ depends on HAS_IOMEM && OF
+ select GENERIC_PHY
+ select GENERIC_PHY_MIPI_DPHY
+ help
+ Choose this option if you have a Cadence D-PHY in your
+ system. If M is selected, the module will be called
+ cdns-dphy.
+
config PHY_CADENCE_SIERRA
tristate "Cadence Sierra PHY Driver"
depends on OF && HAS_IOMEM && RESET_CONTROLLER
select GENERIC_PHY
help
- Enable this to support the Cadence Sierra PHY driver
\ No newline at end of file
+ Enable this to support the Cadence Sierra PHY driver
diff --git a/drivers/phy/cadence/Makefile b/drivers/phy/cadence/Makefile
index 412349af0492..2f9e3457b954 100644
--- a/drivers/phy/cadence/Makefile
+++ b/drivers/phy/cadence/Makefile
@@ -1,2 +1,3 @@
obj-$(CONFIG_PHY_CADENCE_DP) += phy-cadence-dp.o
+obj-$(CONFIG_PHY_CADENCE_DPHY) += cdns-dphy.o
obj-$(CONFIG_PHY_CADENCE_SIERRA) += phy-cadence-sierra.o
diff --git a/drivers/phy/cadence/cdns-dphy.c b/drivers/phy/cadence/cdns-dphy.c
new file mode 100644
index ..1d0abba03f37
--- /dev/null
+++ b/drivers/phy/cadence/cdns-dphy.c
@@ -0,0 +1,389 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright: 2017-2018 Cadence Design Systems, Inc.
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include
+#include
+
+#define REG_WAKEUP_TIME_NS 800
+#define DPHY_PLL_RATE_HZ 10800
+
+/* DPHY registers */
+#define DPHY_PMA_CMN(reg) (reg)
+#define DPHY_PMA_LCLK(reg) (0x100 + (reg))
+#define DPHY_PMA_LDATA(lane, reg) (0x200 + ((lane) * 0x100) + (reg))
+#define DPHY_PMA_RCLK(reg) (0x600 + (reg))
+#define DPHY_PMA_RDATA(lane, reg) (0x700 + ((lane) * 0x100) + (reg))
+#define DPHY_PCS(reg) (0xb00 + (reg))
+
+#define DPHY_CMN_SSM DPHY_PMA_CMN(0x20)
+#define DPHY_CMN_SSM_ENBIT(0)
+#define DPHY_CMN_TX_MODE_ENBIT(9)
+
+#define DPHY_CMN_PWM DPHY_PMA_CMN(0x40)
+#define DPHY_CMN_PWM_DIV(x)((x) << 20)
+#define DPHY_CMN_PWM_LOW(x)((x) << 10)
+#define DPHY_CMN_PWM_HIGH(x) (x)
+
+#define DPHY_CMN_FBDIV DPHY_PMA_CMN(0x4c)
+#define DPHY_CMN_FBDIV_VAL(low, high) (((high) << 11) | ((low) << 22))
+#define DPHY_CMN_FBDIV_FROM_REG(BIT(10) | BIT(21))
+
+#define DPHY_CMN_OPIPDIV DPHY_PMA_CMN(0x50)
+#define DPHY_CMN_IPDIV_FROM_REGBIT(0)
+#define DPHY_CMN_IPDIV(x) ((x) << 1)
+#define DPHY_CMN_OPDIV_FROM_REGBIT(6)
+#define DPHY_CMN_OPDIV(x) ((x) << 7)
+
+#define DPHY_PSM_CFG DPHY_PCS(0x4)
+#define DPHY_PSM_CFG_FROM_REG BIT(0)
+#define DPHY_PSM_CLK_DIV(x)((x) << 1)
+
+#define DSI_HBP_FRAME_OVERHEAD 12
+#define DSI_HSA_FRAME_OVERHEAD 14
+#define DSI_HFP_FRAME_OVERHEAD 6
+#define DSI_HSS_VSS_VSE_FRAME_OVERHEAD 4
+#define DSI_BLANKING_FRAME_OVERHEAD6
+#define DSI_NULL_FRAME_OVERHEAD6
+#define DSI_EOT_PKT_SIZE 4
+
+struct cdns_dphy_cfg {
+ u8 pll_ipdiv;
+ u8 pll_opdiv;
+ u16 pll_fbdiv;
+ unsigned int nlanes;
+};
+
+enum cdns_dphy_clk_lane_cfg {
+ DPHY_CLK_CFG_LEFT_DRIVES_ALL = 0,
+ DPHY_CLK_CFG_LEFT_DRIVES_RIGHT = 1,
+ DPHY_CLK_CFG_LEFT_DRIVES_LEFT = 2,
+ DPHY_CLK_CFG_RIGHT_DRIVES_ALL = 3,
+};
+
+struct cdns_dphy;
+struct cdns_dphy_ops {
+ int (*probe)(struct cdns_dphy *dphy);
+ void (*remove)(struct cdns_dphy *dphy);
+ void (*
[PATCH v4 2/9] phy: dphy: Change units of wakeup and init parameters
The Init and wakeup D-PHY parameters are in the micro/milliseconds range,
putting the values real close to the types limits if they were in
picoseconds.
Move them to microseconds which should be better fit.
Suggested-by: Sakari Ailus
Signed-off-by: Maxime Ripard
---
drivers/phy/phy-core-mipi-dphy.c | 8
include/linux/phy/phy-mipi-dphy.h | 8
2 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/phy/phy-core-mipi-dphy.c b/drivers/phy/phy-core-mipi-dphy.c
index 465fa1b91a5f..14e0551cd319 100644
--- a/drivers/phy/phy-core-mipi-dphy.c
+++ b/drivers/phy/phy-core-mipi-dphy.c
@@ -65,12 +65,12 @@ int phy_mipi_dphy_get_default_config(unsigned long
pixel_clock,
*/
cfg->hs_trail = max(4 * 8 * ui, 6 + 4 * 4 * ui);
- cfg->init = 1;
+ cfg->init = 100;
cfg->lpx = 6;
cfg->ta_get = 5 * cfg->lpx;
cfg->ta_go = 4 * cfg->lpx;
cfg->ta_sure = 2 * cfg->lpx;
- cfg->wakeup = 10;
+ cfg->wakeup = 1000;
cfg->hs_clk_rate = hs_clk_rate;
cfg->lanes = lanes;
@@ -143,7 +143,7 @@ int phy_mipi_dphy_config_validate(struct
phy_configure_opts_mipi_dphy *cfg)
if (cfg->hs_trail < max(8 * ui, 6 + 4 * ui))
return -EINVAL;
- if (cfg->init < 1)
+ if (cfg->init < 100)
return -EINVAL;
if (cfg->lpx < 5)
@@ -158,7 +158,7 @@ int phy_mipi_dphy_config_validate(struct
phy_configure_opts_mipi_dphy *cfg)
if (cfg->ta_sure < cfg->lpx || cfg->ta_sure > (2 * cfg->lpx))
return -EINVAL;
- if (cfg->wakeup < 10)
+ if (cfg->wakeup < 1000)
return -EINVAL;
return 0;
diff --git a/include/linux/phy/phy-mipi-dphy.h
b/include/linux/phy/phy-mipi-dphy.h
index 9cf97cd1d303..627d28080d3a 100644
--- a/include/linux/phy/phy-mipi-dphy.h
+++ b/include/linux/phy/phy-mipi-dphy.h
@@ -190,10 +190,10 @@ struct phy_configure_opts_mipi_dphy {
/**
* @init:
*
-* Time, in picoseconds for the initialization period to
+* Time, in microseconds for the initialization period to
* complete.
*
-* Minimum value: 1 ps
+* Minimum value: 100 us
*/
unsigned intinit;
@@ -244,11 +244,11 @@ struct phy_configure_opts_mipi_dphy {
/**
* @wakeup:
*
-* Time, in picoseconds, that a transmitter drives a Mark-1
+* Time, in microseconds, that a transmitter drives a Mark-1
* state prior to a Stop state in order to initiate an exit
* from ULPS.
*
-* Minimum value: 10 ps
+* Minimum value: 1000 us
*/
unsigned intwakeup;
--
git-series 0.9.1
[PATCH v4 6/9] drm/bridge: cdns: Separate DSI and D-PHY configuration
The current configuration of the DSI bridge and its associated D-PHY is
intertwined. In order to ease the future conversion to the phy framework
for the D-PHY part, let's split the configuration in two.
Signed-off-by: Maxime Ripard
---
drivers/gpu/drm/bridge/cdns-dsi.c | 96 ++--
1 file changed, 68 insertions(+), 28 deletions(-)
diff --git a/drivers/gpu/drm/bridge/cdns-dsi.c
b/drivers/gpu/drm/bridge/cdns-dsi.c
index ce9496d13986..3ac6dd524b6d 100644
--- a/drivers/gpu/drm/bridge/cdns-dsi.c
+++ b/drivers/gpu/drm/bridge/cdns-dsi.c
@@ -545,6 +545,11 @@ bridge_to_cdns_dsi_input(struct drm_bridge *bridge)
return container_of(bridge, struct cdns_dsi_input, bridge);
}
+static unsigned int mode_to_dpi_hfp(const struct drm_display_mode *mode)
+{
+ return mode->crtc_hsync_start - mode->crtc_hdisplay;
+}
+
static int cdns_dsi_get_dphy_pll_cfg(struct cdns_dphy *dphy,
struct cdns_dphy_cfg *cfg,
unsigned int dpi_htotal,
@@ -731,14 +736,12 @@ static unsigned int dpi_to_dsi_timing(unsigned int
dpi_timing,
static int cdns_dsi_mode2cfg(struct cdns_dsi *dsi,
const struct drm_display_mode *mode,
struct cdns_dsi_cfg *dsi_cfg,
-struct cdns_dphy_cfg *dphy_cfg,
bool mode_valid_check)
{
- unsigned long dsi_htotal = 0, dsi_hss_hsa_hse_hbp = 0;
struct cdns_dsi_output *output = &dsi->output;
- unsigned int dsi_hfp_ext = 0, dpi_hfp, tmp;
+ unsigned int tmp;
bool sync_pulse = false;
- int bpp, nlanes, ret;
+ int bpp, nlanes;
memset(dsi_cfg, 0, sizeof(*dsi_cfg));
@@ -757,8 +760,6 @@ static int cdns_dsi_mode2cfg(struct cdns_dsi *dsi,
mode->crtc_hsync_end : mode->crtc_hsync_start);
dsi_cfg->hbp = dpi_to_dsi_timing(tmp, bpp, DSI_HBP_FRAME_OVERHEAD);
- dsi_htotal += dsi_cfg->hbp + DSI_HBP_FRAME_OVERHEAD;
- dsi_hss_hsa_hse_hbp += dsi_cfg->hbp + DSI_HBP_FRAME_OVERHEAD;
if (sync_pulse) {
if (mode_valid_check)
@@ -768,49 +769,90 @@ static int cdns_dsi_mode2cfg(struct cdns_dsi *dsi,
dsi_cfg->hsa = dpi_to_dsi_timing(tmp, bpp,
DSI_HSA_FRAME_OVERHEAD);
- dsi_htotal += dsi_cfg->hsa + DSI_HSA_FRAME_OVERHEAD;
- dsi_hss_hsa_hse_hbp += dsi_cfg->hsa + DSI_HSA_FRAME_OVERHEAD;
}
dsi_cfg->hact = dpi_to_dsi_timing(mode_valid_check ?
mode->hdisplay : mode->crtc_hdisplay,
bpp, 0);
- dsi_htotal += dsi_cfg->hact;
+ dsi_cfg->hfp = dpi_to_dsi_timing(mode_to_dpi_hfp(mode), bpp,
+DSI_HFP_FRAME_OVERHEAD);
- if (mode_valid_check)
- dpi_hfp = mode->hsync_start - mode->hdisplay;
- else
- dpi_hfp = mode->crtc_hsync_start - mode->crtc_hdisplay;
+ return 0;
+}
+
+static int cdns_dphy_validate(struct cdns_dsi *dsi,
+ struct cdns_dsi_cfg *dsi_cfg,
+ struct cdns_dphy_cfg *dphy_cfg,
+ const struct drm_display_mode *mode,
+ bool mode_valid_check)
+{
+ struct cdns_dsi_output *output = &dsi->output;
+ unsigned long dsi_htotal;
+ unsigned int dsi_hfp_ext = 0;
+
+ int ret;
- dsi_cfg->hfp = dpi_to_dsi_timing(dpi_hfp, bpp, DSI_HFP_FRAME_OVERHEAD);
+ dsi_htotal = dsi_cfg->hbp + DSI_HBP_FRAME_OVERHEAD;
+ if (output->dev->mode_flags & MIPI_DSI_MODE_VIDEO_SYNC_PULSE)
+ dsi_htotal += dsi_cfg->hsa + DSI_HSA_FRAME_OVERHEAD;
+
+ dsi_htotal += dsi_cfg->hact;
dsi_htotal += dsi_cfg->hfp + DSI_HFP_FRAME_OVERHEAD;
if (mode_valid_check)
ret = cdns_dsi_get_dphy_pll_cfg(dsi->dphy, dphy_cfg,
- mode->htotal, bpp,
+ mode->htotal,
mode->clock * 1000,
- dsi_htotal, nlanes,
+
mipi_dsi_pixel_format_to_bpp(output->dev->format),
+ dsi_htotal,
+ output->dev->lanes,
&dsi_hfp_ext);
else
ret = cdns_dsi_get_dphy_pll_cfg(dsi->dphy, dphy_cfg,
- mode->crtc_htotal, bpp,
+ mode->crtc_htotal,
+
Re: [PATCH v5 2/6] media: sun6i: Add mod_rate quirk
On Mon, Dec 24, 2018 at 08:57:48PM +0530, Jagan Teki wrote:
> On Fri, Dec 21, 2018 at 6:30 PM Maxime Ripard
> wrote:
> >
> > On Thu, Dec 20, 2018 at 06:24:34PM +0530, Jagan Teki wrote:
> > > Unfortunately default CSI_SCLK rate cannot work properly to
> > > drive the connected sensor interface, particularly on few
> > > Allwinner SoC's like A64.
> > >
> > > So, add mod_rate quirk via driver data so-that the respective
> > > SoC's which require to alter the default mod clock rate can assign
> > > the operating clock rate.
> > >
> > > Signed-off-by: Jagan Teki
> > > ---
> > > .../platform/sunxi/sun6i-csi/sun6i_csi.c | 34 +++
> > > 1 file changed, 28 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c
> > > b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c
> > > index ee882b66a5ea..fe002beae09c 100644
> > > --- a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c
> > > +++ b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c
> > > @@ -15,6 +15,7 @@
> > > #include
> > > #include
> > > #include
> > > +#include
> > > #include
> > > #include
> > > #include
> > > @@ -28,8 +29,13 @@
> > >
> > > #define MODULE_NAME "sun6i-csi"
> > >
> > > +struct sun6i_csi_variant {
> > > + unsigned long mod_rate;
> > > +};
> > > +
> > > struct sun6i_csi_dev {
> > > struct sun6i_csicsi;
> > > + const struct sun6i_csi_variant *variant;
> > > struct device *dev;
> > >
> > > struct regmap *regmap;
> > > @@ -822,33 +828,43 @@ static int sun6i_csi_resource_request(struct
> > > sun6i_csi_dev *sdev,
> > > return PTR_ERR(sdev->clk_mod);
> > > }
> > >
> > > + if (sdev->variant->mod_rate)
> > > + clk_set_rate_exclusive(sdev->clk_mod,
> > > sdev->variant->mod_rate);
> > > +
> >
> > It still doesn't make any sense to do it in the probe function...
>
> I'm not sure we discussed about the context wrt probe, we discussed
> about exclusive put clock.
https://lkml.org/lkml/2018/12/18/584
"Doing it here is not really optimal either, since you'll put a
constraint on the system (maintaining that clock at 300MHz), while
it's not in use."
> Since clocks were enabling in set_power and clock rate can be set
> during probe in single time instead of setting it in set_power for
> every power enablement. anything wrong with that.
See above.
Plus, a clock running draws power. It doesn't really make sense to
draw power for something that is unused.
> > We discussed this in the previous iteration already.
> >
> > What we didn't discuss is the variant function that you introduce,
> > while the previous approach was enough.
>
> We discussed about clk_rate_exclusive_put, and that even handle it in
> .remove right? so I have variant to handle it in sun6i_csi_remove.
We indeed discussed the clk_rate_exclusive_put. However, you chose to
implement it using a variant structure which really isn't needed.
Maxime
--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
signature.asc
Description: PGP signature
Re: [PATCH v5 2/6] media: sun6i: Add mod_rate quirk
On Thu, Dec 20, 2018 at 06:24:34PM +0530, Jagan Teki wrote:
> Unfortunately default CSI_SCLK rate cannot work properly to
> drive the connected sensor interface, particularly on few
> Allwinner SoC's like A64.
>
> So, add mod_rate quirk via driver data so-that the respective
> SoC's which require to alter the default mod clock rate can assign
> the operating clock rate.
>
> Signed-off-by: Jagan Teki
> ---
> .../platform/sunxi/sun6i-csi/sun6i_csi.c | 34 +++
> 1 file changed, 28 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c
> b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c
> index ee882b66a5ea..fe002beae09c 100644
> --- a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c
> +++ b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c
> @@ -15,6 +15,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -28,8 +29,13 @@
>
> #define MODULE_NAME "sun6i-csi"
>
> +struct sun6i_csi_variant {
> + unsigned long mod_rate;
> +};
> +
> struct sun6i_csi_dev {
> struct sun6i_csicsi;
> + const struct sun6i_csi_variant *variant;
> struct device *dev;
>
> struct regmap *regmap;
> @@ -822,33 +828,43 @@ static int sun6i_csi_resource_request(struct
> sun6i_csi_dev *sdev,
> return PTR_ERR(sdev->clk_mod);
> }
>
> + if (sdev->variant->mod_rate)
> + clk_set_rate_exclusive(sdev->clk_mod, sdev->variant->mod_rate);
> +
It still doesn't make any sense to do it in the probe function...
We discussed this in the previous iteration already.
What we didn't discuss is the variant function that you introduce,
while the previous approach was enough.
Maxime
--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
Re: [PATCH v3 03/10] phy: Add MIPI D-PHY configuration options
On Mon, Dec 17, 2018 at 10:20:39PM +0200, sakari.ai...@iki.fi wrote: > Hi Maxime, > > On Mon, Dec 17, 2018 at 04:49:21PM +0100, Maxime Ripard wrote: > > Hi Sakari, > > > > Thanks for your feedback. > > > > On Thu, Dec 13, 2018 at 10:49:28PM +0200, sakari.ai...@iki.fi wrote: > > > > + /** > > > > +* @lanes: > > > > +* > > > > +* Number of active data lanes used for the transmissions. > > > > > > Could you add that these are the first "lanes" number of lanes from what > > > are available? > > > > I'm not quite sure I understood this part though, what did you mean? > > A number of lanes are routed between the two devices on hardware, and this > field is specifying how many of them are in use. In order for the bus to > function, both ends need to be in agreement on which of these lanes are > actually being used. The current practice I've seen without exceptions is > that these are the first n lanes. Ah, right, I get it now, thanks! Maxime -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com signature.asc Description: PGP signature
Re: [PATCH v4 0/6] media/sun6i: Allwinner A64 CSI support
65;5402;1c On Wed, Dec 19, 2018 at 04:11:50PM +0530, Jagan Teki wrote: > On Wed, Dec 19, 2018 at 3:55 PM Maxime Ripard > wrote: > > > > On Tue, Dec 18, 2018 at 08:58:22PM +0530, Jagan Teki wrote: > > > On Tue, Dec 18, 2018 at 8:51 PM Maxime Ripard > > > wrote: > > > > > > > > On Tue, Dec 18, 2018 at 05:03:14PM +0530, Jagan Teki wrote: > > > > > This series support CSI on Allwinner A64. > > > > > > > > > > Tested 640x480, 320x240, 720p, 1080p resolutions UYVY8_2X8 format. > > > > > > > > > > Changes for v4: > > > > > - update the compatible string order > > > > > - add proper commit message > > > > > - included BPI-M64 patch > > > > > - skipped amarula-a64 patch > > > > > Changes for v3: > > > > > - update dt-bindings for A64 > > > > > - set mod clock via csi driver > > > > > - remove assign clocks from dtsi > > > > > - remove i2c-gpio opendrian > > > > > - fix avdd and dovdd supplies > > > > > - remove vcc-csi pin group supply > > > > > > > > > > Note: This series created on top of H3 changes [1] > > > > > > > > > > [1] https://patchwork.kernel.org/cover/10705905/ > > > > > > > > You had memory corruption before, how was this fixed? > > > > > > Memory corruption observed with default 600MHz on 1080p. It worked > > > fine on BPI-M64 (with 300MHz) > > > > I don't get it. In the previous version of those patches, you were > > mentionning you were still having this issue, even though you had the > > clock running at 300MHz, and then you tried to convince us to merge > > the patches nonetheless. > > > > Why would you say that then if that issue was fixed? > > Previous version has A64-Relic board, which has some xclk issue on > sensor side wrt 1080p. I have tried 300MHz on the same hardware, it's > failing to capture on 30fps and so I tried 600MHz(which is default) on > the same configuration but it encounter memory corruption. > > So, for checking whether there is an issue with hardware on A64-Relic > I moved with BPI-M64 dev board. which is working 1080p with 300MHz, ie > reason I have not included A64-Relic on this version and included > BPI-M64. We processed A64-Relic to hardware team to figure out the > clock and once ie fixed I'm planning to send DTS patch for that. > > This is overall summary, hope you understand. Ok, great, thanks! Maxime -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com signature.asc Description: PGP signature
Re: [PATCH v4 0/6] media/sun6i: Allwinner A64 CSI support
On Tue, Dec 18, 2018 at 08:58:22PM +0530, Jagan Teki wrote: > On Tue, Dec 18, 2018 at 8:51 PM Maxime Ripard > wrote: > > > > On Tue, Dec 18, 2018 at 05:03:14PM +0530, Jagan Teki wrote: > > > This series support CSI on Allwinner A64. > > > > > > Tested 640x480, 320x240, 720p, 1080p resolutions UYVY8_2X8 format. > > > > > > Changes for v4: > > > - update the compatible string order > > > - add proper commit message > > > - included BPI-M64 patch > > > - skipped amarula-a64 patch > > > Changes for v3: > > > - update dt-bindings for A64 > > > - set mod clock via csi driver > > > - remove assign clocks from dtsi > > > - remove i2c-gpio opendrian > > > - fix avdd and dovdd supplies > > > - remove vcc-csi pin group supply > > > > > > Note: This series created on top of H3 changes [1] > > > > > > [1] https://patchwork.kernel.org/cover/10705905/ > > > > You had memory corruption before, how was this fixed? > > Memory corruption observed with default 600MHz on 1080p. It worked > fine on BPI-M64 (with 300MHz) I don't get it. In the previous version of those patches, you were mentionning you were still having this issue, even though you had the clock running at 300MHz, and then you tried to convince us to merge the patches nonetheless. Why would you say that then if that issue was fixed? Maxime -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com signature.asc Description: PGP signature
