Re: [PATCH v3 4/4] media: qcom: jpeg: Add Qualcomm JPEG V4L2 encoder

[Dmitry Baryshkov]

On Tue, Jun 30, 2026 at 05:14:14PM +0300, Atanas Filipov wrote:
> On 6/30/2026 4:06 PM, Dmitry Baryshkov wrote:
> > On Mon, Jun 29, 2026 at 03:17:50PM +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.
> > > 
> > > This series targets SM8250 (Kona) platforms.
> > > 
> > > The jpeg-encoder node is described as a child node of the CAMSS block
> > > and is probed automatically via of_platform_populate() in camss_probe().
> > > 
> > > Signed-off-by: Atanas Filipov <atanas.filipov@xxxxxxxxxxxxxxxx>

[...]

> > 
> > > + */
> > > +enum qcom_soc_perf_level {
> > > +	QCOM_SOC_PERF_LOWSVS = 0,
> > > +	QCOM_SOC_PERF_SVS,
> > > +	QCOM_SOC_PERF_SVS_L1,
> > > +	QCOM_SOC_PERF_NOMINAL
> > > +};
> > > +
> > > +/* hardware register field mask identifiers */
> > > +enum qcom_jpeg_mask_id {
> > 
> > Why do you need to keep all of them abstracted? Just use the registers
> > as is, assuming that the registers don't change a lot between platforms.
> > 
> 
> Hi Dmitry,
> 
> Regarding the register abstraction — I'd like to make a case for keeping it.
> 
> The Qualcomm JPEG hardware exists in multiple versions across different
> SoCs. I don't have access to all of them, but Qualcomm tends to reuse and
> recombine IP blocks across platforms, and assuming register-level uniformity
> seems premature.
> 
> Adding a new platform only requires a new ops/mask table — existing ones
> remain untouched. Would you be open to keeping this abstraction as-is?

No. This is like putting the cart in front of the horse. You are
creating an abstraction, while you are not even sure if it is required
or not. Moreover, the difference between different hardware generations
is typically bigger than just registers+offsets. Adding support for
other versions of the core would likely require one to extend the
functions to write different data. So, just having the registers in the
huge table would make little sense.

So, no. Drop these tables, use registers and masks (where required)
directly.


-- 
With best wishes
Dmitry