| Message ID | 20240309151528.ayphvdpnj2crvycv@basti-XPS-13-9310 |
|---|---|
| State | New |
| Headers | show |
| Series | [GIT,PULL,FOR,6.9] Imagination E5010 JPEG encoder | expand |
+Andrzej, Hi Hans, Andrzej, On 18/03/24 19:13, Hans Verkuil wrote: > On 09/03/2024 4:15 pm, Sebastian Fricke wrote: >> Hey Hans & Mauro, >> >> These patches add support for the Imagination E5010 JPEG encoder. >> >> Please pull. >> >> The following changes since commit b14257abe7057def6127f6fb2f14f9adc8acabdb: >> >> media: rcar-isp: Disallow unbind of devices (2024-03-07 16:35:13 +0100) >> >> are available in the Git repository at: >> >> https://gitlab.collabora.com/sebastianfricke/linux.git tags/for-6.9-e5010-jpeg-encoder >> >> for you to fetch changes up to 146a5dc5f8baee4178a1cdfa483aa3c94273ce5e: >> >> media: imagination: Add E5010 JPEG Encoder driver (2024-03-09 16:10:43 +0100) >> >> ---------------------------------------------------------------- >> Adds support for the E5010-JPEG-encoder >> >> ---------------------------------------------------------------- >> Devarsh Thakkar (3): >> media: dt-bindings: Add Imagination E5010 JPEG Encoder >> media: jpeg: Add reference quantization and huffman tables > > This should be added to v4l2-jpeg.c. There are also a few other > changes I'd like to see, see my reply to this patch. > > The other two patches are OK, but since a v7 is needed anyway, I'll > add a few comments to patch 3/3 as well. > I assume you are suggesting here to move quantization tables and huffman tables to v4l2-jpeg.c as static arrays and implement new helper functions which in turn use these tables to write jpeg headers? I think Andrzej had suggested as similar thing earlier [0] to have a separate jpeg helper library for writing jpeg headers (which in turn will use these quantization and huffman tables) which can be used by other drivers too (for e.g. e5010_jpeg_enc and hantro_jpeg can use these helper functions for writing jpeg headers), so do we want to implement this helper functions directly inside v4l2_jpeg.c or create a new file? Kindly let me know your opinion on this. [0] : https://lore.kernel.org/all/307911e9-7e0e-4a10-aeb1-6c72896c1454@collabora.com/ Regards Devarsh > Regards, > > Hans > >> media: imagination: Add E5010 JPEG Encoder driver >> >> .../bindings/media/img,e5010-jpeg-enc.yaml | 75 + >> MAINTAINERS | 7 + >> drivers/media/platform/Kconfig | 1 + >> drivers/media/platform/Makefile | 1 + >> drivers/media/platform/imagination/Kconfig | 12 + >> drivers/media/platform/imagination/Makefile | 3 + >> .../media/platform/imagination/e5010-core-regs.h | 585 ++++++++ >> .../media/platform/imagination/e5010-jpeg-enc-hw.c | 267 ++++ >> .../media/platform/imagination/e5010-jpeg-enc-hw.h | 42 + >> .../media/platform/imagination/e5010-jpeg-enc.c | 1553 ++++++++++++++++++++ >> .../media/platform/imagination/e5010-jpeg-enc.h | 169 +++ >> .../media/platform/imagination/e5010-mmu-regs.h | 311 ++++ >> include/media/jpeg-enc-reftables.h | 112 ++ >> include/media/jpeg.h | 4 + >> 14 files changed, 3142 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/media/img,e5010-jpeg-enc.yaml >> create mode 100644 drivers/media/platform/imagination/Kconfig >> create mode 100644 drivers/media/platform/imagination/Makefile >> create mode 100644 drivers/media/platform/imagination/e5010-core-regs.h >> create mode 100644 drivers/media/platform/imagination/e5010-jpeg-enc-hw.c >> create mode 100644 drivers/media/platform/imagination/e5010-jpeg-enc-hw.h >> create mode 100644 drivers/media/platform/imagination/e5010-jpeg-enc.c >> create mode 100644 drivers/media/platform/imagination/e5010-jpeg-enc.h >> create mode 100644 drivers/media/platform/imagination/e5010-mmu-regs.h >> create mode 100644 include/media/jpeg-enc-reftables.h >> > >
Hi Hans, Sebastian, On 18/03/24 20:11, Hans Verkuil wrote: > On 18/03/2024 2:43 pm, Hans Verkuil wrote: >> On 09/03/2024 4:15 pm, Sebastian Fricke wrote: [...] >> The other two patches are OK, but since a v7 is needed anyway, I'll >> add a few comments to patch 3/3 as well. > > Actually, patch 3/3 has some issues as well, esp. regarding the selection > API. > > Note that I recommend that you test the selection API before posting a > v7, since it is clear that it has never been used since it currently > always returns -EINVAL. > > Regards, > > Hans Just wanted to check if the latest series [1] looks ok to pull in as I had fixed the cropping related issues and implemented API to export reference JPEG tables as part of v4l2-jpeg.c itself as suggested by Hans along with kernel-doc upgrades. There has been no major changes in v4l2-jpeg or jpeg driver (e5010-jpeg-enc) related code since last 3 revisions as no comments were received, so just wanted to get your opinion on this. [1]: https://lore.kernel.org/all/20240604105402.2258395-1-devarsht@ti.com/#r Regards Devarsh
Hey Hans & Mauro, These patches add support for the Imagination E5010 JPEG encoder. Please pull. The following changes since commit b14257abe7057def6127f6fb2f14f9adc8acabdb: media: rcar-isp: Disallow unbind of devices (2024-03-07 16:35:13 +0100) are available in the Git repository at: https://gitlab.collabora.com/sebastianfricke/linux.git tags/for-6.9-e5010-jpeg-encoder for you to fetch changes up to 146a5dc5f8baee4178a1cdfa483aa3c94273ce5e: media: imagination: Add E5010 JPEG Encoder driver (2024-03-09 16:10:43 +0100) ---------------------------------------------------------------- Adds support for the E5010-JPEG-encoder ---------------------------------------------------------------- Devarsh Thakkar (3): media: dt-bindings: Add Imagination E5010 JPEG Encoder media: jpeg: Add reference quantization and huffman tables media: imagination: Add E5010 JPEG Encoder driver .../bindings/media/img,e5010-jpeg-enc.yaml | 75 + MAINTAINERS | 7 + drivers/media/platform/Kconfig | 1 + drivers/media/platform/Makefile | 1 + drivers/media/platform/imagination/Kconfig | 12 + drivers/media/platform/imagination/Makefile | 3 + .../media/platform/imagination/e5010-core-regs.h | 585 ++++++++ .../media/platform/imagination/e5010-jpeg-enc-hw.c | 267 ++++ .../media/platform/imagination/e5010-jpeg-enc-hw.h | 42 + .../media/platform/imagination/e5010-jpeg-enc.c | 1553 ++++++++++++++++++++ .../media/platform/imagination/e5010-jpeg-enc.h | 169 +++ .../media/platform/imagination/e5010-mmu-regs.h | 311 ++++ include/media/jpeg-enc-reftables.h | 112 ++ include/media/jpeg.h | 4 + 14 files changed, 3142 insertions(+) create mode 100644 Documentation/devicetree/bindings/media/img,e5010-jpeg-enc.yaml create mode 100644 drivers/media/platform/imagination/Kconfig create mode 100644 drivers/media/platform/imagination/Makefile create mode 100644 drivers/media/platform/imagination/e5010-core-regs.h create mode 100644 drivers/media/platform/imagination/e5010-jpeg-enc-hw.c create mode 100644 drivers/media/platform/imagination/e5010-jpeg-enc-hw.h create mode 100644 drivers/media/platform/imagination/e5010-jpeg-enc.c create mode 100644 drivers/media/platform/imagination/e5010-jpeg-enc.h create mode 100644 drivers/media/platform/imagination/e5010-mmu-regs.h create mode 100644 include/media/jpeg-enc-reftables.h