Re: [PATCH v6 1/3] media: dt-bindings: Add Imagination E5010 JPEG Encoder
From: Sebastian Fricke
Date: Thu Mar 07 2024 - 07:05:24 EST
Hey Devarsh,
On 06.03.2024 21:27, Devarsh Thakkar wrote:
Hi Sebastian,
On 01/03/24 22:36, Sebastian Fricke wrote:
Hey Devarsh,
On 01.03.2024 22:02, Devarsh Thakkar wrote:
Hi Sebastian,
On 29/02/24 19:00, Sebastian Fricke wrote:
Hey Devarsh,
On 29.02.2024 16:50, Devarsh Thakkar wrote:
Hi Sebastian,
Thanks for the review.
On 29/02/24 15:56, Sebastian Fricke wrote:
Hey Devarsh,
On 28.02.2024 19:41, Devarsh Thakkar wrote:
Add dt-bindings for Imagination E5010 JPEG Encoder [1] which is implemented
as stateful V4L2 M2M driver.
The device supports baseline encoding with two different quantization
tables and compression ratio as demanded.
Minimum resolution supported is 64x64 and Maximum resolution supported is
8192x8192.
[1]: AM62A TRM (Section 7.6 is for JPEG Encoder)
Link: https://www.ti.com/lit/pdf/spruj16
Co-developed-by: David Huang <d-huang@xxxxxx>
Signed-off-by: David Huang <d-huang@xxxxxx>
Signed-off-by: Devarsh Thakkar <devarsht@xxxxxx>
Reviewed-by: Rob Herring <robh@xxxxxxxxxx>
hmmm when did Rob give his reviewed by on this patch? (As this is not a
DT binding I find that odd)
[PATCH v6 1/3] media: dt-bindings: Add Imagination E5010 JPEG Encoder : This
is indeed the dt-binding patch. Also As shared in version history it is at V4
where Rob Herring added a Reviewed-By as seen here [0]
And where is the Reviewed by tag from Benjamin that he provided on V5?
As captured in patch version history here [1] I thought to remove the
Reviewed-By since the Reviewed-By tag was on V5 and with V6 the driver got
updated with some changes to handle reported sparse warnings and so I have
asked Benjamin to check the range-diff and help with a quick review again if
possible.
Kindly let me know if I missed something or anything needs to be done from
my end.
Yes thanks I was a bit too swift to write here, sorry for the noise.
We'll have a look.
Sorry for the back and forth, but on the hindsight and re-looking at the
kernel patch guidelines [0] they suggest that Reviewed-By tag should only be
removed if substantial changes were made in further revisions.
So looks to me in-fact it was a mistake on my part to remove the Reviewed-by
considering the change made in the following patch series was not a
substantial one as seen in the range-diff [1].
Considering this, just wanted to check with you if it's possible for you to
consider the Reviewed-by tag :
`Reviewed-by: Benjamin Gaignard <benjamin.gaignard@xxxxxxxxxxxxx`
if it helps consolidate things faster to get this series in given we are close
to final RC's ?
Yup I think we can keep it as the changes are very minor. Otherwise the
series is pretty much good to go, I'll prepare the PR asap.
Thanks, so I assume this could still make it to 6.9 merge window if it gets
pulled in before 6.8 rc8 ? Also do you think it would be possible to pull in
https://lore.kernel.org/all/20240305160529.4152865-1-devarsht@xxxxxx/ as here
too nothing much has changed w.r.t initial posting apart from commit message.
Yes I'd assume so but I sadly got sick this week, I'll get to this as
soon as I can.
Regards
Devarsh
Greetings,
Sebastian
Greetings,
Sebastian
[0]:
https://docs.kernel.org/process/submitting-patches.html#using-reported-by-tested-by-reviewed-by-suggested-by-and-fixes:~:text=changed%20substantially
[1]: https://gist.github.com/devarsht/c89180ac2b0d2814614f2b59d0705c19
Regards
Devarsh
Greetings,
Sebastian
[0] :
https://lore.kernel.org/all/170716378412.295212.11603162949482063011.robh@xxxxxxxxxx/
[1] : https://lore.kernel.org/all/20240228141140.3530612-4-devarsht@xxxxxx/
Regards
Devarsh
---
V2: No change
V3:
- Add vendor specific compatible
- Update reg names
- Update clocks to 1
- Fix dts example with proper naming
V4:
- Use ti-specific compatible ti,am62a-jpeg-enc as secondary one
- Update commit message and title
- Remove clock-names as only single clock
V5:
- Add Reviewed-By tag
V6:
- No change
.../bindings/media/img,e5010-jpeg-enc.yaml | 75 +++++++++++++++++++
MAINTAINERS | 5 ++
2 files changed, 80 insertions(+)
create mode 100644
Documentation/devicetree/bindings/media/img,e5010-jpeg-enc.yaml
diff --git
a/Documentation/devicetree/bindings/media/img,e5010-jpeg-enc.yaml
b/Documentation/devicetree/bindings/media/img,e5010-jpeg-enc.yaml
new file mode 100644
index 000000000000..085020cb9e61
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/img,e5010-jpeg-enc.yaml
@@ -0,0 +1,75 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/media/img,e5010-jpeg-enc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Imagination E5010 JPEG Encoder
+
+maintainers:
+ - Devarsh Thakkar <devarsht@xxxxxx>
+
+description: |
+ The E5010 is a JPEG encoder from Imagination Technologies implemented on
+ TI's AM62A SoC. It is capable of real time encoding of YUV420 and YUV422
+ inputs to JPEG and M-JPEG. It supports baseline JPEG Encoding up to
+ 8Kx8K resolution.
+
+properties:
+ compatible:
+ oneOf:
+ - items:
+ - const: ti,am62a-jpeg-enc
+ - const: img,e5010-jpeg-enc
+ - const: img,e5010-jpeg-enc
+
+ reg:
+ items:
+ - description: The E5010 core register region
+ - description: The E5010 mmu register region
+
+ reg-names:
+ items:
+ - const: core
+ - const: mmu
+
+ power-domains:
+ maxItems: 1
+
+ resets:
+ maxItems: 1
+
+ clocks:
+ maxItems: 1
+
+ interrupts:
+ maxItems: 1
+
+required:
+ - compatible
+ - reg
+ - reg-names
+ - interrupts
+ - clocks
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/soc/ti,sci_pm_domain.h>
+ #include <dt-bindings/interrupt-controller/arm-gic.h>
+ #include <dt-bindings/interrupt-controller/irq.h>
+
+ soc {
+ #address-cells = <2>;
+ #size-cells = <2>;
+ jpeg-encoder@fd20000 {
+ compatible = "img,e5010-jpeg-enc";
+ reg = <0x00 0xfd20000 0x00 0x100>,
+ <0x00 0xfd20200 0x00 0x200>;
+ reg-names = "core", "mmu";
+ clocks = <&k3_clks 201 0>;
+ power-domains = <&k3_pds 201 TI_SCI_PD_EXCLUSIVE>;
+ interrupts = <GIC_SPI 98 IRQ_TYPE_LEVEL_HIGH>;
+ };
+ };
diff --git a/MAINTAINERS b/MAINTAINERS
index e1475ca38ff2..6b34ee8d92b5 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -10572,6 +10572,11 @@ S: Maintained
F: Documentation/devicetree/bindings/auxdisplay/img,ascii-lcd.yaml
F: drivers/auxdisplay/img-ascii-lcd.c
+IMGTEC JPEG ENCODER DRIVER
+M: Devarsh Thakkar <devarsht@xxxxxx>
+S: Supported
+F: Documentation/devicetree/bindings/media/img,e5010-jpeg-enc.yaml
+
IMGTEC IR DECODER DRIVER
S: Orphan
F: drivers/media/rc/img-ir/
--
2.39.1