Re: [PATCH v6 3/3] arm64: dts: qcom: sm8250: Add JPEG encoder node

[Dmitry Baryshkov]

On Tue, Aug 04, 2026 at 08:48:13AM +0300, Gjorgji Rosikopulos (Consultant) wrote:
> Hi Dmitry,
> 
> On 8/3/2026 9:44 PM, Dmitry Baryshkov wrote:
> > On Thu, Jul 30, 2026 at 09:02:26PM +0300, Atanas Filipov wrote:
> >> On 7/30/2026 7:01 PM, Dmitry Baryshkov wrote:
> >>> On Thu, Jul 30, 2026 at 06:05:24PM +0300, Atanas Filipov wrote:
> >>>> Add the JPEG encoder hardware node to the SM8250 device tree so the
> >>>> qcom-jpeg V4L2 encoder driver can bind and operate on this platform.
> >>>>
> >>>> Signed-off-by: Atanas Filipov <atanas.filipov@xxxxxxxxxxxxxxxx>
> >>>> ---
> >>>>   arch/arm64/boot/dts/qcom/sm8250.dtsi | 72 ++++++++++++++++++++++++++++
> >>>>   1 file changed, 72 insertions(+)
> >>>>
> >>>> diff --git a/arch/arm64/boot/dts/qcom/sm8250.dtsi b/arch/arm64/boot/dts/qcom/sm8250.dtsi
> >>>> index f11da2ca4181..d42b29419320 100644
> >>>> --- a/arch/arm64/boot/dts/qcom/sm8250.dtsi
> >>>> +++ b/arch/arm64/boot/dts/qcom/sm8250.dtsi
> >>>> @@ -13,6 +13,7 @@
> >>>>   #include <dt-bindings/firmware/qcom,scm.h>
> >>>>   #include <dt-bindings/gpio/gpio.h>
> >>>>   #include <dt-bindings/interconnect/qcom,osm-l3.h>
> >>>> +#include <dt-bindings/interconnect/qcom,icc.h>
> >>>>   #include <dt-bindings/interconnect/qcom,sm8250.h>
> >>>>   #include <dt-bindings/mailbox/qcom-ipcc.h>
> >>>>   #include <dt-bindings/phy/phy-qcom-qmp.h>
> >>>> @@ -4486,6 +4487,77 @@ cci1_i2c1: i2c-bus@1 {
> >>>>   			};
> >>>>   		};
> >>>> +		jpeg-encoder@ac53000 {
> >>>> +			compatible = "qcom,sm8250-jpeg-enc";
> >>>> +			status = "disabled";
> >>>> +
> >>>> +			reg = <0x0 0x0ac53000 0x0 0x1000>;
> >>>> +
> >>>> +			interrupts = <GIC_SPI 474 IRQ_TYPE_EDGE_RISING>;
> >>>> +
> >>>> +			power-domains = <&camcc TITAN_TOP_GDSC>,
> >>>> +					<&rpmhpd RPMHPD_MMCX>,
> >>>
> >>> Is TITAN_TOP a part of the MMCX or CX domain? In the former case,
> >>> please drop separate mmcx vote here and let it come through the
> >>> titan_top (it might require camcc driver and node modifications).
> >>>
> >>
> >> The change was requested by Brian, most likely in connection with upcoming
> >> migrations. I think he would be able to explain the exact reasons. As far as
> >> the facts are concerned, this is not currently necessary for the driver to
> >> be functional.
> > 
> > There is no word _driver_ in the _dt bindings_. What is the actual
> > hardware wrapping? What is being powered by which domain? Is MMCX
> > directly powering something? Or is it gated by TITAN_TOP_GDSC? I assume
> > that MX is powering on PLLs, correct?
> 
> Yes i agree there is no word driver in the dt_bindings but you get my point,
> with only TITAN_TOP_GDSC hw is fully functional there is no need of additional MMCX 
> power domain. 

This will be true if:
- MMCX is a parent of the TITAN_TOP_GDSC (in Linux kernel)
- the driver casts the vote on the TITAN_TOP_GDSC using the opp tables.

> 
> > 
> >>>> +
> >>>> +				opp-100000000 {
> >>>> +					opp-hz = /bits/ 64 <400000000>,
> >>>> +						 /bits/ 64 <100000000>;
> >>>> +					required-opps = <&rpmhpd_opp_min_svs>;
> >>>
> >>> This looks a bit strange. Are you sure that PLL levels match the MMCX /
> >>> internal clock levels? I think you need to vote on both domains.
> > 
> > This is not addressed / responded.
> 
> Yes this need to be checked. I will get back when we get info for this...

Checking the 4.19 kernel. cam_cc_jpeg_clk_src casts votes on the vdd_mm
(MMCX) power domain, while its parents (pll0, pll2) cast votes on
vdd_mx (MX) power domain. Note, that frequencies used by those clocks do
not match, so we need separate entries at each of the corners.

-- 
With best wishes
Dmitry