[0/3] 32-bit Meson: add the canvas module
mbox series

Message ID 20190413171532.25967-1-martin.blumenstingl@googlemail.com
Headers show
Series
  • 32-bit Meson: add the canvas module
Related show

Message

Martin Blumenstingl April 13, 2019, 5:15 p.m. UTC
This adds the canvas module on Meson8, Meson8b and Meson8m2. The canvas
IP is used by the video decoder hardware as well as the VPU (video
output) hardware.

Neither the VPU nor the video decoder driver support the 32-bit SoCs
yet. However, we can still add the canvas module to have it available
once these drivers gain support for the older SoCs.

I have tested this on my Meson8m2 board by hacking the VPU driver to
not re-initialize the VPU (and to use the configuration set by u-boot).
With that hack I could get some image out of the CVBS connector. No
changes to the canvas driver were required.

Due to lack of hardware I could not test Meson8, but I'm following (as
always) what the Amlogic 3.10 vendor kernel uses.
Meson8b is also not tested because u-boot of my EC-100 doesn't have
video output enabled (so I couldn't use the same hack I used on my
Meson8m2 board).

This series meant to be applied on top of "Meson8b: add support for the
RTC on EC-100 and Odroid-C1" from [0]


[0] https://patchwork.kernel.org/cover/10899509/


Martin Blumenstingl (3):
  ARM: dts: meson8: add the canvas module
  ARM: dts: meson8m2: update the offset of the canvas module
  ARM: dts: meson8b: add the canvas module

 arch/arm/boot/dts/meson8.dtsi   | 21 +++++++++++++++++++++
 arch/arm/boot/dts/meson8b.dtsi  | 21 +++++++++++++++++++++
 arch/arm/boot/dts/meson8m2.dtsi | 10 ++++++++++
 3 files changed, 52 insertions(+)

Comments

Maxime Jourdan April 13, 2019, 6:54 p.m. UTC | #1
Hi Martin,
On Sat, Apr 13, 2019 at 7:15 PM Martin Blumenstingl
<martin.blumenstingl@googlemail.com> wrote:
>
> This adds the canvas module on Meson8, Meson8b and Meson8m2. The canvas
> IP is used by the video decoder hardware as well as the VPU (video
> output) hardware.
>
> Neither the VPU nor the video decoder driver support the 32-bit SoCs
> yet. However, we can still add the canvas module to have it available
> once these drivers gain support for the older SoCs.
>
> I have tested this on my Meson8m2 board by hacking the VPU driver to
> not re-initialize the VPU (and to use the configuration set by u-boot).
> With that hack I could get some image out of the CVBS connector. No
> changes to the canvas driver were required.
>
> Due to lack of hardware I could not test Meson8, but I'm following (as
> always) what the Amlogic 3.10 vendor kernel uses.
> Meson8b is also not tested because u-boot of my EC-100 doesn't have
> video output enabled (so I couldn't use the same hack I used on my
> Meson8m2 board).
>
> This series meant to be applied on top of "Meson8b: add support for the
> RTC on EC-100 and Odroid-C1" from [0]
>
>

The series looks good to me, however I wonder if we should maybe add a
new compatible ?

The canvas IP before the GX* generation does not handle what Amlogic
calls "endianness", the field that allows doing some byte-switching to
get proper NV12/NV21. So the following defines are unusable:

#define MESON_CANVAS_ENDIAN_SWAP16 0x1
#define MESON_CANVAS_ENDIAN_SWAP32 0x3
#define MESON_CANVAS_ENDIAN_SWAP64 0x7
#define MESON_CANVAS_ENDIAN_SWAP128 0xf

It wouldn't change much functionally, but we could have e.g a warning
if a m8 canvas user tries to set endianness even though it does
nothing.

Maxime

> [0] https://patchwork.kernel.org/cover/10899509/
>
>
> Martin Blumenstingl (3):
>   ARM: dts: meson8: add the canvas module
>   ARM: dts: meson8m2: update the offset of the canvas module
>   ARM: dts: meson8b: add the canvas module
>
>  arch/arm/boot/dts/meson8.dtsi   | 21 +++++++++++++++++++++
>  arch/arm/boot/dts/meson8b.dtsi  | 21 +++++++++++++++++++++
>  arch/arm/boot/dts/meson8m2.dtsi | 10 ++++++++++
>  3 files changed, 52 insertions(+)
>
> --
> 2.21.0
>
>
> _______________________________________________
> linux-amlogic mailing list
> linux-amlogic@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-amlogic
Martin Blumenstingl April 18, 2019, 7:50 p.m. UTC | #2
Hi Maxime,

On Sat, Apr 13, 2019 at 8:54 PM Maxime Jourdan <maxi.jourdan@wanadoo.fr> wrote:
>
> Hi Martin,
> On Sat, Apr 13, 2019 at 7:15 PM Martin Blumenstingl
> <martin.blumenstingl@googlemail.com> wrote:
> >
> > This adds the canvas module on Meson8, Meson8b and Meson8m2. The canvas
> > IP is used by the video decoder hardware as well as the VPU (video
> > output) hardware.
> >
> > Neither the VPU nor the video decoder driver support the 32-bit SoCs
> > yet. However, we can still add the canvas module to have it available
> > once these drivers gain support for the older SoCs.
> >
> > I have tested this on my Meson8m2 board by hacking the VPU driver to
> > not re-initialize the VPU (and to use the configuration set by u-boot).
> > With that hack I could get some image out of the CVBS connector. No
> > changes to the canvas driver were required.
> >
> > Due to lack of hardware I could not test Meson8, but I'm following (as
> > always) what the Amlogic 3.10 vendor kernel uses.
> > Meson8b is also not tested because u-boot of my EC-100 doesn't have
> > video output enabled (so I couldn't use the same hack I used on my
> > Meson8m2 board).
> >
> > This series meant to be applied on top of "Meson8b: add support for the
> > RTC on EC-100 and Odroid-C1" from [0]
> >
> >
>
> The series looks good to me, however I wonder if we should maybe add a
> new compatible ?
>
> The canvas IP before the GX* generation does not handle what Amlogic
> calls "endianness", the field that allows doing some byte-switching to
> get proper NV12/NV21. So the following defines are unusable:
>
> #define MESON_CANVAS_ENDIAN_SWAP16 0x1
> #define MESON_CANVAS_ENDIAN_SWAP32 0x3
> #define MESON_CANVAS_ENDIAN_SWAP64 0x7
> #define MESON_CANVAS_ENDIAN_SWAP128 0xf
I didn't know about this - thank you for pointing this out.

your suggestions to add new compatible strings is a good idea for that case.
Amlogic uses different defines for Meson8 and Meson8m2 in their vendor
kernel and they keep Meson8b different.
I will add three new compatibles, one for each SoC (Meson8, Meson8b,
Meson8m2) just to be on the safe side if we discover differences in
the canvas IP on these SoCs.

what do you think?

> It wouldn't change much functionally, but we could have e.g a warning
> if a m8 canvas user tries to set endianness even though it does
> nothing.
this is a good idea, that will make it easier to spot why something
doesn't work.
we can also return -EINVAL, like you already do for the case where the
canvas ID is already used.


Martin
Maxime Jourdan May 20, 2019, 7:21 p.m. UTC | #3
Hey Martin, so sorry for forgetting about this.

On Thu, Apr 18, 2019 at 9:50 PM Martin Blumenstingl
<martin.blumenstingl@googlemail.com> wrote:
>
> Hi Maxime,
>
> On Sat, Apr 13, 2019 at 8:54 PM Maxime Jourdan <maxi.jourdan@wanadoo.fr> wrote:
> >
> > Hi Martin,
> > On Sat, Apr 13, 2019 at 7:15 PM Martin Blumenstingl
> > <martin.blumenstingl@googlemail.com> wrote:
> > >
> > > This adds the canvas module on Meson8, Meson8b and Meson8m2. The canvas
> > > IP is used by the video decoder hardware as well as the VPU (video
> > > output) hardware.
> > >
> > > Neither the VPU nor the video decoder driver support the 32-bit SoCs
> > > yet. However, we can still add the canvas module to have it available
> > > once these drivers gain support for the older SoCs.
> > >
> > > I have tested this on my Meson8m2 board by hacking the VPU driver to
> > > not re-initialize the VPU (and to use the configuration set by u-boot).
> > > With that hack I could get some image out of the CVBS connector. No
> > > changes to the canvas driver were required.
> > >
> > > Due to lack of hardware I could not test Meson8, but I'm following (as
> > > always) what the Amlogic 3.10 vendor kernel uses.
> > > Meson8b is also not tested because u-boot of my EC-100 doesn't have
> > > video output enabled (so I couldn't use the same hack I used on my
> > > Meson8m2 board).
> > >
> > > This series meant to be applied on top of "Meson8b: add support for the
> > > RTC on EC-100 and Odroid-C1" from [0]
> > >
> > >
> >
> > The series looks good to me, however I wonder if we should maybe add a
> > new compatible ?
> >
> > The canvas IP before the GX* generation does not handle what Amlogic
> > calls "endianness", the field that allows doing some byte-switching to
> > get proper NV12/NV21. So the following defines are unusable:
> >
> > #define MESON_CANVAS_ENDIAN_SWAP16 0x1
> > #define MESON_CANVAS_ENDIAN_SWAP32 0x3
> > #define MESON_CANVAS_ENDIAN_SWAP64 0x7
> > #define MESON_CANVAS_ENDIAN_SWAP128 0xf
> I didn't know about this - thank you for pointing this out.
>
> your suggestions to add new compatible strings is a good idea for that case.
> Amlogic uses different defines for Meson8 and Meson8m2 in their vendor
> kernel and they keep Meson8b different.
> I will add three new compatibles, one for each SoC (Meson8, Meson8b,
> Meson8m2) just to be on the safe side if we discover differences in
> the canvas IP on these SoCs.
>
> what do you think?
>

Sure thing. Keep an eye out for any hints regarding the amount of
canvases as well, I *think* I remember some old SoCs having only 192
but I haven't been able to find it again.

> > It wouldn't change much functionally, but we could have e.g a warning
> > if a m8 canvas user tries to set endianness even though it does
> > nothing.
> this is a good idea, that will make it easier to spot why something
> doesn't work.
> we can also return -EINVAL, like you already do for the case where the
> canvas ID is already used.
>

Yes, returning an error is a good idea.

Maxime

>
> Martin
Martin Blumenstingl May 20, 2019, 7:36 p.m. UTC | #4
Hi Maxime,

On Mon, May 20, 2019 at 9:21 PM Maxime Jourdan <maxi.jourdan@wanadoo.fr> wrote:
>
> Hey Martin, so sorry for forgetting about this.
>
> On Thu, Apr 18, 2019 at 9:50 PM Martin Blumenstingl
> <martin.blumenstingl@googlemail.com> wrote:
> >
> > Hi Maxime,
> >
> > On Sat, Apr 13, 2019 at 8:54 PM Maxime Jourdan <maxi.jourdan@wanadoo.fr> wrote:
> > >
> > > Hi Martin,
> > > On Sat, Apr 13, 2019 at 7:15 PM Martin Blumenstingl
> > > <martin.blumenstingl@googlemail.com> wrote:
> > > >
> > > > This adds the canvas module on Meson8, Meson8b and Meson8m2. The canvas
> > > > IP is used by the video decoder hardware as well as the VPU (video
> > > > output) hardware.
> > > >
> > > > Neither the VPU nor the video decoder driver support the 32-bit SoCs
> > > > yet. However, we can still add the canvas module to have it available
> > > > once these drivers gain support for the older SoCs.
> > > >
> > > > I have tested this on my Meson8m2 board by hacking the VPU driver to
> > > > not re-initialize the VPU (and to use the configuration set by u-boot).
> > > > With that hack I could get some image out of the CVBS connector. No
> > > > changes to the canvas driver were required.
> > > >
> > > > Due to lack of hardware I could not test Meson8, but I'm following (as
> > > > always) what the Amlogic 3.10 vendor kernel uses.
> > > > Meson8b is also not tested because u-boot of my EC-100 doesn't have
> > > > video output enabled (so I couldn't use the same hack I used on my
> > > > Meson8m2 board).
> > > >
> > > > This series meant to be applied on top of "Meson8b: add support for the
> > > > RTC on EC-100 and Odroid-C1" from [0]
> > > >
> > > >
> > >
> > > The series looks good to me, however I wonder if we should maybe add a
> > > new compatible ?
> > >
> > > The canvas IP before the GX* generation does not handle what Amlogic
> > > calls "endianness", the field that allows doing some byte-switching to
> > > get proper NV12/NV21. So the following defines are unusable:
> > >
> > > #define MESON_CANVAS_ENDIAN_SWAP16 0x1
> > > #define MESON_CANVAS_ENDIAN_SWAP32 0x3
> > > #define MESON_CANVAS_ENDIAN_SWAP64 0x7
> > > #define MESON_CANVAS_ENDIAN_SWAP128 0xf
> > I didn't know about this - thank you for pointing this out.
> >
> > your suggestions to add new compatible strings is a good idea for that case.
> > Amlogic uses different defines for Meson8 and Meson8m2 in their vendor
> > kernel and they keep Meson8b different.
> > I will add three new compatibles, one for each SoC (Meson8, Meson8b,
> > Meson8m2) just to be on the safe side if we discover differences in
> > the canvas IP on these SoCs.
> >
> > what do you think?
> >
>
> Sure thing. Keep an eye out for any hints regarding the amount of
> canvases as well, I *think* I remember some old SoCs having only 192
> but I haven't been able to find it again.
Meson6 and older are limited to 192, Meson8 and newer already support
256. source: [0]

> > > It wouldn't change much functionally, but we could have e.g a warning
> > > if a m8 canvas user tries to set endianness even though it does
> > > nothing.
> > this is a good idea, that will make it easier to spot why something
> > doesn't work.
> > we can also return -EINVAL, like you already do for the case where the
> > canvas ID is already used.
> >
>
> Yes, returning an error is a good idea.
OK, I'll send an updated series later.


Martin


[0] https://github.com/endlessm/linux-meson/blob/5cb4882cdda584878a29132aeb9a90497a121df9/drivers/amlogic/canvas/canvas.c#L41