mbox series

[v6,00/15] Using component framework to support multi hardware decode

Message ID 20210901083215.25984-1-yunfei.dong@mediatek.com
Headers show
Series Using component framework to support multi hardware decode | expand

Message

Yunfei Dong (董云飞) Sept. 1, 2021, 8:32 a.m. UTC
This series adds support for multi hardware decode into mtk-vcodec, by first
adding component framework to manage each hardware information: interrupt,
clock, register bases and power. Secondly add core thread to deal with core
hardware message, at the same time, add msg queue for different hardware
share messages. Lastly, the architecture of different specs are not the same,
using specs type to separate them.

This series has been tested with both MT8183 and MT8173. Decoding was working
for both chips.

Patches 1~3 rewrite get register bases and power on/off interface.

Patch 4 add component framework to support multi hardware.

Patch 5 separate video encoder and decoder document

Patches 6-15 add interfaces to support core hardware.
----
This patch dependents on : "media: mtk-vcodec: support for MT8183 decoder"[1] and
"Mediatek MT8192 clock support"[2].

1: Multi hardware decode is based on stateless decoder, MT8183 is the first time
to add stateless decoder. Otherwise it will cause conflict. This patch will be
accepted in 5.15[1].

2: The definition of decoder clocks are in mt8192-clk.h, this patch already in clk tree[2].

[1]https://patchwork.linuxtv.org/project/linux-media/list/?series=5826
[2]https://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git/commit/?h=clk-next&id=f35f1a23e0e12e3173e9e9dedbc150d139027189
----
Changes compared with v5:
-Add decoder hardware block diagram for patch 13/15

Changes compared with v4:
- Fix comments for patch 4/15
  >> +     if (dev->is_comp_supported) {
  >> +             ret = mtk_vcodec_init_master(dev);
  >> +             if (ret < 0)
  >> +                     goto err_component_match;
  >> +     } else {
  >> +             platform_set_drvdata(pdev, dev);
  >> +     }
  Fix platform_set_drvdata.
- Fix build error for patch 9/15
- Add depend patch in case of error header file for patch 13/15

Changes compared with v3:
- Fix return value for patch 1/15
- Fix comments for patch 4/15
  > Looking up "mediatek,mtk-vcodec-core" to determine if it uses component framwork sounds like...
  Add prameter in pdata, for all platform will use compoent after mt8183

  >> +     if (dev->is_comp_supported) {
  >> +             ret = mtk_vcodec_init_master(dev);
  >> +             if (ret < 0)
  >> +                     goto err_component_match;
  >> +     } else {
  >> +             platform_set_drvdata(pdev, dev);
  >> +     }
  > + Has asked the same question in [1].  Why it removes the
  > +platform_set_drvdata() above?  mtk_vcodec_init_master() also calls platform_set_drvdata().
  Must call component_master_add_with_match after platform_set_drvdata for component architecture.
- Fix yaml files check fail for patch 5/15
- Fix yaml file check fail for patch 14/15

Changes compared with v1:
- Fix many comments for patch 3/14
- Remove unnecessary code for patch 4/14
- Using enum mtk_vdec_hw_count instead of magic numbers for patch 6/14
- Reconstructed get/put lat buffer for lat and core hardware for patch 7/14
- Using yaml format to instead of txt file for patch 12/14

Yunfei Dong (15):
  media: mtk-vcodec: Get numbers of register bases from DT
  media: mtk-vcodec: Align vcodec wake up interrupt interface
  media: mtk-vcodec: Refactor vcodec pm interface
  media: mtk-vcodec: Use component framework to manage each hardware
    information
  dt-bindings: media: mtk-vcodec: Separate video encoder and decoder
    dt-bindings
  media: mtk-vcodec: Use pure single core for MT8183
  media: mtk-vcodec: Add irq interface for multi hardware
  media: mtk-vcodec: Add msg queue feature for lat and core architecture
  media: mtk-vcodec: Generalize power and clock on/off interfaces
  media: mtk-vcodec: Add new interface to lock different hardware
  media: mtk-vcodec: Add core thread
  media: mtk-vcodec: Support 34bits dma address for vdec
  dt-bindings: media: mtk-vcodec: Adds decoder dt-bindings for mt8192
  media: mtk-vcodec: Add core dec and dec end ipi msg
  media: mtk-vcodec: Use codec type to separate different hardware

 .../media/mediatek,vcodec-comp-decoder.yaml   | 192 ++++++++++++
 .../media/mediatek,vcodec-decoder.yaml        | 175 +++++++++++
 .../media/mediatek,vcodec-encoder.yaml        | 185 +++++++++++
 .../bindings/media/mediatek-vcodec.txt        | 130 --------
 drivers/media/platform/mtk-vcodec/Makefile    |   2 +
 .../platform/mtk-vcodec/mtk_vcodec_dec.c      |   4 +-
 .../platform/mtk-vcodec/mtk_vcodec_dec.h      |   1 +
 .../platform/mtk-vcodec/mtk_vcodec_dec_drv.c  | 276 ++++++++++++++---
 .../platform/mtk-vcodec/mtk_vcodec_dec_hw.c   | 184 +++++++++++
 .../platform/mtk-vcodec/mtk_vcodec_dec_hw.h   |  53 ++++
 .../platform/mtk-vcodec/mtk_vcodec_dec_pm.c   |  98 ++++--
 .../platform/mtk-vcodec/mtk_vcodec_dec_pm.h   |  13 +-
 .../mtk-vcodec/mtk_vcodec_dec_stateful.c      |   2 +
 .../mtk-vcodec/mtk_vcodec_dec_stateless.c     |   2 +
 .../platform/mtk-vcodec/mtk_vcodec_drv.h      |  71 ++++-
 .../platform/mtk-vcodec/mtk_vcodec_enc_drv.c  |  12 +-
 .../platform/mtk-vcodec/mtk_vcodec_enc_pm.c   |   1 -
 .../platform/mtk-vcodec/mtk_vcodec_intr.c     |  27 +-
 .../platform/mtk-vcodec/mtk_vcodec_intr.h     |   4 +-
 .../platform/mtk-vcodec/mtk_vcodec_util.c     |  87 +++++-
 .../platform/mtk-vcodec/mtk_vcodec_util.h     |   8 +-
 .../platform/mtk-vcodec/vdec/vdec_h264_if.c   |   2 +-
 .../mtk-vcodec/vdec/vdec_h264_req_if.c        |   2 +-
 .../platform/mtk-vcodec/vdec/vdec_vp8_if.c    |   2 +-
 .../platform/mtk-vcodec/vdec/vdec_vp9_if.c    |   2 +-
 .../media/platform/mtk-vcodec/vdec_drv_if.c   |  21 +-
 .../media/platform/mtk-vcodec/vdec_ipi_msg.h  |  16 +-
 .../platform/mtk-vcodec/vdec_msg_queue.c      | 290 ++++++++++++++++++
 .../platform/mtk-vcodec/vdec_msg_queue.h      | 157 ++++++++++
 .../media/platform/mtk-vcodec/vdec_vpu_if.c   |  46 ++-
 .../media/platform/mtk-vcodec/vdec_vpu_if.h   |  22 ++
 .../platform/mtk-vcodec/venc/venc_h264_if.c   |   2 +-
 .../platform/mtk-vcodec/venc/venc_vp8_if.c    |   2 +-
 33 files changed, 1812 insertions(+), 279 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/media/mediatek,vcodec-comp-decoder.yaml
 create mode 100644 Documentation/devicetree/bindings/media/mediatek,vcodec-decoder.yaml
 create mode 100644 Documentation/devicetree/bindings/media/mediatek,vcodec-encoder.yaml
 delete mode 100644 Documentation/devicetree/bindings/media/mediatek-vcodec.txt
 create mode 100644 drivers/media/platform/mtk-vcodec/mtk_vcodec_dec_hw.c
 create mode 100644 drivers/media/platform/mtk-vcodec/mtk_vcodec_dec_hw.h
 create mode 100644 drivers/media/platform/mtk-vcodec/vdec_msg_queue.c
 create mode 100644 drivers/media/platform/mtk-vcodec/vdec_msg_queue.h

Comments

Yunfei Dong (董云飞) Sept. 3, 2021, 3:08 a.m. UTC | #1
Hi Ezequiel,

Thanks for your suggestion.
On Thu, 2021-09-02 at 13:30 -0300, Ezequiel Garcia wrote:
> On Wed, 1 Sept 2021 at 05:32, Yunfei Dong <yunfei.dong@mediatek.com>

> wrote:

> > 

> > This series adds support for multi hardware decode into mtk-vcodec, 

> > by first

> > adding component framework to manage each hardware information:

> > interrupt,

> > clock, register bases and power. Secondly add core thread to deal

> > with core

> > hardware message, at the same time, add msg queue for different

> > hardware

> > share messages. Lastly, the architecture of different specs are not

> > the same,

> > using specs type to separate them.

> > 

> > This series has been tested with both MT8183 and MT8173. Decoding

> > was working

> > for both chips.

> > 

> > Patches 1~3 rewrite get register bases and power on/off interface.

> > 

> > Patch 4 add component framework to support multi hardware.

> > 

> > Patch 5 separate video encoder and decoder document

> > 

> > Patches 6-15 add interfaces to support core hardware.

> > ----

> > This patch dependents on : "media: mtk-vcodec: support for MT8183

> > decoder"[1] and

> > "Mediatek MT8192 clock support"[2].

> > 

> > 1: Multi hardware decode is based on stateless decoder, MT8183 is

> > the first time

> > to add stateless decoder. Otherwise it will cause conflict. This

> > patch will be

> > accepted in 5.15[1].

> > 

> > 2: The definition of decoder clocks are in mt8192-clk.h, this patch

> > already in clk tree[2].

> > 

> > [1]

> > https://patchwork.linuxtv.org/project/linux-media/list/?series=5826

> > [2]

> > https://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git/commit/?h=clk-next&id=f35f1a23e0e12e3173e9e9dedbc150d139027189

> > ----

> > Changes compared with v5:

> > -Add decoder hardware block diagram for patch 13/15

> > 

> 

> 

> The discussion on v5 was still on-going, so sending this v6

> is not helpful. The context for v5's discussion is now harder to

> find.

> 


> Please avoid sending a new version without properly

> discussing all the feedback, and without reaching consensus.

> This is very important, please keep it in mind.

> 

Thanks for your remind, I will keep this patch until get the solution.

> Specifically, the feedback on v5 was NAK, with the request to avoid

> using any async framework, and instead try to find a simpler

> solution.

> 

> For instance, you can model things with a bus-like pattern,

> which ties all the devices together, under a parent node.

> This pattern is common in the kernel, the parent

> node can use of_platform_populate or similar

> (git grep of_platform_populate, you will see plenty of examples).

> 

> You will still have to do some work to have the proper

> regs resources, but this is doable. Each child is a device,

> so it can have its own resources (clocks, interrupts, iommus).

> 

> You don't need any async framework.

> 

If put the lat and core node in the parent node, need to check the
below things after discussed with iommu owner.

If putting the iommus property in the child node, then is the child
device a standard platform device?

The iommu registe like this:  
ret = bus_set_iommu(&platform_bus_type, &mtk_iommu_ops); 
It expect the consumer is a standard platform device. otherwise it
could not enter to the iommu of_xlate.)

Thanks
Yunfei Dong
>     vcodec_dec: vcodec_dec@16000000 {

>         compatible = "mediatek,mt8192-vcodec-dec";

>         reg = <something>;

>         mediatek,scp = <&scp>;

>         iommus = <&iommu0 M4U_PORT_L4_VDEC_MC_EXT>;

>         dma-ranges = <0x1 0x0 0x0 0x40000000 0x0 0xfff00000>;

> 

>         vcodec_lat@0x10000 {

>             compatible = "mediatek,mtk-vcodec-lat";

>             reg = <0x10000 0x800>;      /* VDEC_MISC */

>             interrupts = <GIC_SPI 426 IRQ_TYPE_LEVEL_HIGH 0>;

>             // etc

>         };

> 

>         vcodec_core@0x25000 {

>            compatible = "mediatek,mtk-vcodec-core";

>            reg = <0x25000 0x1000>;      /* VDEC_CORE_MISC */

>            interrupts = <GIC_SPI 425 IRQ_TYPE_LEVEL_HIGH 0>;

>            // etc

>         };

>     };

> 

> Thanks,

> Ezequiel
Chen-Yu Tsai Sept. 3, 2021, 4:09 a.m. UTC | #2
On Fri, Sep 3, 2021 at 12:31 AM Ezequiel Garcia
<ezequiel@vanguardiasur.com.ar> wrote:
>
> On Wed, 1 Sept 2021 at 05:32, Yunfei Dong <yunfei.dong@mediatek.com> wrote:
> >
> > This series adds support for multi hardware decode into mtk-vcodec, by first
> > adding component framework to manage each hardware information: interrupt,
> > clock, register bases and power. Secondly add core thread to deal with core
> > hardware message, at the same time, add msg queue for different hardware
> > share messages. Lastly, the architecture of different specs are not the same,
> > using specs type to separate them.
> >
> > This series has been tested with both MT8183 and MT8173. Decoding was working
> > for both chips.
> >
> > Patches 1~3 rewrite get register bases and power on/off interface.
> >
> > Patch 4 add component framework to support multi hardware.
> >
> > Patch 5 separate video encoder and decoder document
> >
> > Patches 6-15 add interfaces to support core hardware.
> > ----
> > This patch dependents on : "media: mtk-vcodec: support for MT8183 decoder"[1] and
> > "Mediatek MT8192 clock support"[2].
> >
> > 1: Multi hardware decode is based on stateless decoder, MT8183 is the first time
> > to add stateless decoder. Otherwise it will cause conflict. This patch will be
> > accepted in 5.15[1].
> >
> > 2: The definition of decoder clocks are in mt8192-clk.h, this patch already in clk tree[2].
> >
> > [1]https://patchwork.linuxtv.org/project/linux-media/list/?series=5826
> > [2]https://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git/commit/?h=clk-next&id=f35f1a23e0e12e3173e9e9dedbc150d139027189
> > ----
> > Changes compared with v5:
> > -Add decoder hardware block diagram for patch 13/15
> >
>
>
> The discussion on v5 was still on-going, so sending this v6
> is not helpful. The context for v5's discussion is now harder to find.
>
> Please avoid sending a new version without properly
> discussing all the feedback, and without reaching consensus.
> This is very important, please keep it in mind.
>
> Specifically, the feedback on v5 was NAK, with the request to avoid
> using any async framework, and instead try to find a simpler solution.
>
> For instance, you can model things with a bus-like pattern,
> which ties all the devices together, under a parent node.
> This pattern is common in the kernel, the parent
> node can use of_platform_populate or similar
> (git grep of_platform_populate, you will see plenty of examples).
>
> You will still have to do some work to have the proper
> regs resources, but this is doable. Each child is a device,
> so it can have its own resources (clocks, interrupts, iommus).

AFAIK of_platform_populate doesn't guarantee the drivers actually
probed (modules loaded late, missing modules, deferred probe, etc.),
only that the devices are created, so you still need some sort of
(async) mechanism to wait for the subdevices to be in operational
state. Most of the examples using of_platform_populate are there
to ensure that the parent device is operational before creating
the sub-devices, not the other way around.


ChenYu


> You don't need any async framework.
>
>     vcodec_dec: vcodec_dec@16000000 {
>         compatible = "mediatek,mt8192-vcodec-dec";
>         reg = <something>;
>         mediatek,scp = <&scp>;
>         iommus = <&iommu0 M4U_PORT_L4_VDEC_MC_EXT>;
>         dma-ranges = <0x1 0x0 0x0 0x40000000 0x0 0xfff00000>;
>
>         vcodec_lat@0x10000 {
>             compatible = "mediatek,mtk-vcodec-lat";
>             reg = <0x10000 0x800>;      /* VDEC_MISC */
>             interrupts = <GIC_SPI 426 IRQ_TYPE_LEVEL_HIGH 0>;
>             // etc
>         };
>
>         vcodec_core@0x25000 {
>            compatible = "mediatek,mtk-vcodec-core";
>            reg = <0x25000 0x1000>;      /* VDEC_CORE_MISC */
>            interrupts = <GIC_SPI 425 IRQ_TYPE_LEVEL_HIGH 0>;
>            // etc
>         };
>     };
>
> Thanks,
> Ezequiel
>
> _______________________________________________
> Linux-mediatek mailing list
> Linux-mediatek@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-mediatek
Ezequiel Garcia Sept. 26, 2021, 2:51 p.m. UTC | #3
On Sun, 26 Sept 2021 at 05:27, yunfei.dong@mediatek.com
<yunfei.dong@mediatek.com> wrote:
>

> Hi Ezequiel,

>

> Could you please help to give some feedback when you are free for iommu

> limitation?

>


How about you work on the architecture I originally suggested?

As the saying goes, talk is cheap, show us the code.
So let's see the code and let's discuss the limitations
with the code.

> According to google's suggestion, it's better not to use v4l2 async

> also.


Hum? I haven't seen such objection on the mailing list.

Thanks,
Ezequiel