+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@xxxxxxxxxxxxx/ 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 >> > >