Re: [PATCH v6 2/3] media: qcom: jpeg: Add Qualcomm JPEG V4L2 encoder

On Tue, Aug 04, 2026 at 08:53:36AM +0300, Gjorgji Rosikopulos (Consultant) wrote:
> Hi Dmitry,
> 
> On 8/3/2026 9:37 PM, Dmitry Baryshkov wrote:
> > On Mon, Aug 03, 2026 at 06:57:09PM +0300, Gjorgji Rosikopulos (Consultant) wrote:
> >> Hi Dmitry, 
> >>
> >> On 7/30/2026 6:57 PM, Dmitry Baryshkov wrote:
> >>> On Thu, Jul 30, 2026 at 06:05:23PM +0300, Atanas Filipov wrote:
> >>>> Add a Qualcomm JPEG encoder driver implemented on top of the
> >>>> V4L2 mem2mem framework.
> >>>>
> >>>> The driver wires vb2 queue handling, format negotiation, JPEG header
> >>>> handling, interrupt-driven job completion, and runtime PM/clock/ICC
> >>>> integration for the standalone JPEG encode hardware block.
> >>>>
> >>
> >> <snip>
> >>
> >>>> + */
> >>>> +
> >>>> +#define JFIF_HEADER_WIDTH_OFFS		0x07
> >>>> +#define JFIF_HEADER_HEIGHT_OFFS		0x05
> >>>
> >>> And you've ignored feedback here. PLEASE move all standard-related
> >>> defines and code to the common helpers. Are there any other drivers
> >>> which construct JPEG files manually? If not, you are lucky and you can
> >>> just push you code. If they are, find a way to unify the codebase.
> >>>
> >>> At the very least, it would make you split this commit into at least
> >>> two, making them more readable.
> >>
> >> Yes, this comment was incorporated, maybe partially, or it wasn't fully understood.
> >> The helper is used for the quantization tables and wherever helpers are available, 
> >> similar to other drivers: hantro_jpeg.c, e5010-jpeg-enc.c.
> >>
> >> The missing JFIF tags and additional helpers can certainly be added,
> >> but is holding up the current driver just for that a reasonable ask? 
> >> This discussion may continue for a long time — is it reasonable to wait that long?
> > 
> > From my point of view, yes. From your comment it feels like each driver
> > having their own way of wriing JPEG framings.
> > 
> >>
> >> I agree it would be good to have all other upstream JPEG encoder drivers move to shared helpers,
> >> but the effort isn't uniform across them. 
> >>
> >> Five drivers — hantro_jpeg.c, mxc-jpeg.c, rcar_jpu.c, gspca/jpeg.h, and solo6x10-jpeg.h,
> >> build a fixed byte-array template and patch width/height/table values at hardcoded offsets, 
> >> so they could plausibly migrate to a shared builder with a similar shape to what we're proposing. 
> > 
> > Can we start with something as simple as this for our driver too?
> 
> Yes i agree we can add helpers, and qcom jpeg to be first driver to use them.
> 
> > 
> > Then you can converge all these drivers to use those simple helpers
> > (this should not require the actual hardware to test), then improve the
> > helpers.
> 
> I don not fell confident to touch other platform drivers which i can not verify, but i think
> that can be done as part of separate patchset after initial helpers are reviewed-merged.

Ok.


-- 
With best wishes
Dmitry