| Message ID | 1624428350-1424-2-git-send-email-kyrie.wu@mediatek.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Support jpeg encode for MT8195 | expand |
On Wed, Jun 23, 2021 at 2:06 PM kyrie.wu <kyrie.wu@mediatek.com> wrote: > Documentation/devicetree/bindings/media/mediatek-jpeg-encoder.txt | 3 +++ > 1 file changed, 3 insertions(+) Note: the patch won't apply after [1]. [1]: https://lore.kernel.org/patchwork/patch/1445298/ > Required properties: > - compatible : "mediatek,mt2701-jpgenc" > +- compatible : "mediatek,mt8195-jpgenc" > +- compatible : "mediatek,mt8195-jpgenc0" > +- compatible : "mediatek,mt8195-jpgenc1" Why it needs 3 compatible strings?
Dear Tzung-Bi, Mt8195 has two hardwares, "mediatek,mt8195-jpgenc0" for hw0 and "mediatek,mt8195-jpgenc1" for HW1. These two nodes will register hardware interrupt, initialize clock, power domain, remap register base addr and other operations. But the device node is not registered. "mediatek,mt8195-jpgenc" will register the device node to represent jpeg encode device. Then the component framework is used to manage the above two hardwares. Sincerely, Kyrie On Fri, 2021-06-25 at 17:18 +0800, Tzung-Bi Shih wrote: > On Wed, Jun 23, 2021 at 2:06 PM kyrie.wu <kyrie.wu@mediatek.com> wrote: > > Documentation/devicetree/bindings/media/mediatek-jpeg-encoder.txt | 3 +++ > > 1 file changed, 3 insertions(+) > Note: the patch won't apply after [1]. > > [1]: https://lore.kernel.org/patchwork/patch/1445298/ > > > Required properties: > > - compatible : "mediatek,mt2701-jpgenc" > > +- compatible : "mediatek,mt8195-jpgenc" > > +- compatible : "mediatek,mt8195-jpgenc0" > > +- compatible : "mediatek,mt8195-jpgenc1" > Why it needs 3 compatible strings?
On Wed, Jun 30, 2021 at 3:56 PM kyrie.wu <kyrie.wu@mediatek.com> wrote: > Mt8195 has two hardwares, "mediatek,mt8195-jpgenc0" for hw0 and > "mediatek,mt8195-jpgenc1" for HW1. These two nodes will register > hardware interrupt, initialize clock, power domain, remap register base > addr and other operations. But the device node is not registered. > "mediatek,mt8195-jpgenc" will register the device node to represent jpeg > encode device. Then the component framework is used to manage the above > two hardwares. Please don't top-posting. Inline your replies so that people can easily follow the discussion. I still don't quite understand why it needs to introduce 2 compatible strings. If hw0 and hw1 are different from interrupts, clocks, power domain, and etc, couldn't you use the same compatible string (e.g. "mt8195-jpgenc") and provide them different DT attributes? > On Fri, 2021-06-25 at 17:18 +0800, Tzung-Bi Shih wrote: > > On Wed, Jun 23, 2021 at 2:06 PM kyrie.wu <kyrie.wu@mediatek.com> wrote: > > > Documentation/devicetree/bindings/media/mediatek-jpeg-encoder.txt | 3 +++ > > > 1 file changed, 3 insertions(+) > > Note: the patch won't apply after [1]. > > > > [1]: https://lore.kernel.org/patchwork/patch/1445298/ > > > > > Required properties: > > > - compatible : "mediatek,mt2701-jpgenc" > > > +- compatible : "mediatek,mt8195-jpgenc" > > > +- compatible : "mediatek,mt8195-jpgenc0" > > > +- compatible : "mediatek,mt8195-jpgenc1" > > Why it needs 3 compatible strings?
On Mon, 2021-07-05 at 16:09 +0800, Tzung-Bi Shih wrote: > On Wed, Jun 30, 2021 at 3:56 PM kyrie.wu <kyrie.wu@mediatek.com> wrote: > > Mt8195 has two hardwares, "mediatek,mt8195-jpgenc0" for hw0 and > > "mediatek,mt8195-jpgenc1" for HW1. These two nodes will register > > hardware interrupt, initialize clock, power domain, remap register base > > addr and other operations. But the device node is not registered. > > "mediatek,mt8195-jpgenc" will register the device node to represent jpeg > > encode device. Then the component framework is used to manage the above > > two hardwares. > Please don't top-posting. Inline your replies so that people can > easily follow the discussion. > > I still don't quite understand why it needs to introduce 2 compatible > strings. If hw0 and hw1 are different from interrupts, clocks, power > domain, and etc, couldn't you use the same compatible string (e.g. > "mt8195-jpgenc") and provide them different DT attributes? Dear Tzung-Bi, As you replied, I can use the same compatible string (e.g "mt8195-jpgenc") and provide resources with different DT attributes. But there are the following problems: The following description takes the resource of clock as an example: 1. If different DT attributes are used to represent different clock information,the structure array (e.g.struct clkinfo) can be used in the driver to save the corresponding information, which will increase the complexity of the software; 2. When using hw0, you only need to turn on clock0, but not clock1, which can reduce power consumption. When the driver uses one of the clocks, it needs to extract each structure from structure array (e.g.struct clkinfo) and compare the attributes to confirming which one to use. The software architecture is as follows: if (...) { [...] } else if (...) { [...] } ...... At present, there are only two HWS, and the software architecture is not complex. With the increase of the number of hardware, the software architecture will become more and more complex. To sum up, using different compatibles to represent different HW can reduce power consumption, decrease code complexity and enhance code readability. > > > On Fri, 2021-06-25 at 17:18 +0800, Tzung-Bi Shih wrote: > > > On Wed, Jun 23, 2021 at 2:06 PM kyrie.wu <kyrie.wu@mediatek.com> wrote: > > > > Documentation/devicetree/bindings/media/mediatek-jpeg-encoder.txt | 3 +++ > > > > 1 file changed, 3 insertions(+) > > > Note: the patch won't apply after [1]. > > > > > > [1]: https://lore.kernel.org/patchwork/patch/1445298/ > > > > > > > Required properties: > > > > - compatible : "mediatek,mt2701-jpgenc" > > > > +- compatible : "mediatek,mt8195-jpgenc" > > > > +- compatible : "mediatek,mt8195-jpgenc0" > > > > +- compatible : "mediatek,mt8195-jpgenc1" > > > Why it needs 3 compatible strings?
diff --git a/Documentation/devicetree/bindings/media/mediatek-jpeg-encoder.txt b/Documentation/devicetree/bindings/media/mediatek-jpeg-encoder.txt index 5e53c6a..ba30525 100644 --- a/Documentation/devicetree/bindings/media/mediatek-jpeg-encoder.txt +++ b/Documentation/devicetree/bindings/media/mediatek-jpeg-encoder.txt @@ -4,6 +4,9 @@ MediaTek JPEG Encoder is the JPEG encode hardware present in MediaTek SoCs Required properties: - compatible : "mediatek,mt2701-jpgenc" +- compatible : "mediatek,mt8195-jpgenc" +- compatible : "mediatek,mt8195-jpgenc0" +- compatible : "mediatek,mt8195-jpgenc1" followed by "mediatek,mtk-jpgenc" - reg : physical base address of the JPEG encoder registers and length of memory mapped region.
Updates binding document for mt8195 jpeg driver. Signed-off-by: kyrie.wu <kyrie.wu@mediatek.com> --- Documentation/devicetree/bindings/media/mediatek-jpeg-encoder.txt | 3 +++ 1 file changed, 3 insertions(+)