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

[Atanas Filipov <atanas.filipov@oss.qualcomm.com>]

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@oss.qualcomm.com>
>> ---
>>   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.

Regards,
Atanas
>> +					<&rpmhpd RPMHPD_MX>;
>> +			power-domain-names = "titan_top", "mmcx", "mx";
>> +
>> +			clocks = <&gcc GCC_CAMERA_SF_AXI_CLK>,
>> +				 <&camcc CAM_CC_CORE_AHB_CLK>,
>> +				 <&camcc CAM_CC_CPAS_AHB_CLK>,
>> +				 <&camcc CAM_CC_CAMNOC_AXI_CLK>,
>> +				 <&camcc CAM_CC_JPEG_CLK>;
>> +
>> +			clock-names = "sf_axi",
>> +				      "core_ahb",
>> +				      "cpas_ahb",
>> +				      "cnoc_axi",
>> +				      "jpeg";
>> +
>> +			interconnects = <&gem_noc MASTER_AMPSS_M0 QCOM_ICC_TAG_ACTIVE_ONLY
>> +					 &config_noc SLAVE_CAMERA_CFG QCOM_ICC_TAG_ACTIVE_ONLY>,
>> +					<&mmss_noc MASTER_CAMNOC_SF QCOM_ICC_TAG_ALWAYS
>> +					 &mc_virt SLAVE_EBI_CH0 QCOM_ICC_TAG_ALWAYS>;
>> +			interconnect-names = "cpu-cfg",
>> +					     "sf-mnoc";
>> +
>> +			iommus = <&apps_smmu 0x2040 0x400>;
>> +
>> +			operating-points-v2 = <&jpeg_opp_table>;
>> +
>> +			jpeg_opp_table: opp-table {
>> +				compatible = "operating-points-v2";
>> +
>> +				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.
> 
>> +				};
>> +
>> +				opp-200000000 {
>> +					opp-hz = /bits/ 64 <400000000>,
>> +						 /bits/ 64 <200000000>;
>> +					required-opps = <&rpmhpd_opp_low_svs>;
>> +				};
>> +
>> +				opp-400000000 {
>> +					opp-hz = /bits/ 64 <400000000>,
>> +						 /bits/ 64 <400000000>;
>> +					required-opps = <&rpmhpd_opp_svs>;
>> +				};
>> +
>> +				opp-480000000 {
>> +					opp-hz = /bits/ 64 <400000000>,
>> +						 /bits/ 64 <480000000>;
>> +					required-opps = <&rpmhpd_opp_svs_l1>;
>> +				};
>> +
>> +				opp-600000000 {
>> +					opp-hz = /bits/ 64 <400000000>,
>> +						 /bits/ 64 <600000000>;
>> +					required-opps = <&rpmhpd_opp_nom>;
>> +				};
>> +			};
>> +		};
>> +
>>   		camss: camss@ac6a000 {
>>   			compatible = "qcom,sm8250-camss";
>>   			status = "disabled";
>> -- 
>> 2.34.1
>>
>