diff mbox series

[v2,1/1] drm/msm/adreno: Add support for SM7150 SoC machine

Message ID 20230926174243.161422-2-danila@jiaxyga.com (mailing list archive)
State Not Applicable
Headers show
Series drm/msm/adreno: Add support for SM7150 | expand

Commit Message

Danila Tikhonov Sept. 26, 2023, 5:42 p.m. UTC 
SM7150 has 5 power levels which correspond to 5 speed-bin values: 0,
128, 146, 167, 172. Speed-bin value is calulated as FMAX/4.8MHz round up
to zero decimal places.

Also a618 on SM7150 uses a615 zapfw. Add a squashed version (.mbn).

Add this as machine = "qcom,sm7150", because speed-bin values are
different from atoll (sc7180/sm7125).

Signed-off-by: Danila Tikhonov <danila@jiaxyga.com>
---
 drivers/gpu/drm/msm/adreno/adreno_device.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

Comments

Konrad Dybcio Sept. 26, 2023, 6:40 p.m. UTC | #1 
On 26.09.2023 19:42, Danila Tikhonov wrote:
> SM7150 has 5 power levels which correspond to 5 speed-bin values: 0,
> 128, 146, 167, 172. Speed-bin value is calulated as FMAX/4.8MHz round up
> to zero decimal places.
> 
> Also a618 on SM7150 uses a615 zapfw. Add a squashed version (.mbn).
> 
> Add this as machine = "qcom,sm7150", because speed-bin values are
> different from atoll (sc7180/sm7125).
> 
> Signed-off-by: Danila Tikhonov <danila@jiaxyga.com>
> ---
What's the downstream dt name for 7150?

Do you have some more complete tree published somewhere?

Konrad
Konrad Dybcio Sept. 26, 2023, 8:03 p.m. UTC | #2 
On 26.09.2023 21:10, Danila Tikhonov wrote:
> 
> I think you mean by name downstream dt - sdmmagpie-gpu.dtsi
> 
> You can see the forked version of the mainline here:
> https://github.com/sm7150-mainline/linux/blob/next/arch/arm64/boot/dts/qcom/sm7150.dtsi
> 
> All fdt that we got here, if it is useful for you:
> https://github.com/sm7150-mainline/downstream-fdt
> 
> Best wishes, Danila
Taking a look at downstream, atoll.dtsi (SC7180) includes
sdmmagpie-gpu.dtsi.

Bottom line is, they share the speed bins, so it should be
fine to just extend the existing entry.

Konrad
Dmitry Baryshkov Oct. 16, 2023, 2:32 p.m. UTC | #3 
On 26/09/2023 23:03, Konrad Dybcio wrote:
> On 26.09.2023 21:10, Danila Tikhonov wrote:
>>
>> I think you mean by name downstream dt - sdmmagpie-gpu.dtsi
>>
>> You can see the forked version of the mainline here:
>> https://github.com/sm7150-mainline/linux/blob/next/arch/arm64/boot/dts/qcom/sm7150.dtsi
>>
>> All fdt that we got here, if it is useful for you:
>> https://github.com/sm7150-mainline/downstream-fdt
>>
>> Best wishes, Danila
> Taking a look at downstream, atoll.dtsi (SC7180) includes
> sdmmagpie-gpu.dtsi.
> 
> Bottom line is, they share the speed bins, so it should be
> fine to just extend the existing entry.

But then atoll.dtsi rewrites speed bins and pwrlevel bins. So they are 
not shared.
Konrad Dybcio Nov. 22, 2023, 8:28 p.m. UTC | #4 
On 10/16/23 16:32, Dmitry Baryshkov wrote:
> On 26/09/2023 23:03, Konrad Dybcio wrote:
>> On 26.09.2023 21:10, Danila Tikhonov wrote:
>>>
>>> I think you mean by name downstream dt - sdmmagpie-gpu.dtsi
>>>
>>> You can see the forked version of the mainline here:
>>> https://github.com/sm7150-mainline/linux/blob/next/arch/arm64/boot/dts/qcom/sm7150.dtsi
>>>
>>> All fdt that we got here, if it is useful for you:
>>> https://github.com/sm7150-mainline/downstream-fdt
>>>
>>> Best wishes, Danila
>> Taking a look at downstream, atoll.dtsi (SC7180) includes
>> sdmmagpie-gpu.dtsi.
>>
>> Bottom line is, they share the speed bins, so it should be
>> fine to just extend the existing entry.
> 
> But then atoll.dtsi rewrites speed bins and pwrlevel bins. So they are not shared.
+Akhil

could you please check internally?

Konrad
Danila Tikhonov Nov. 22, 2023, 9:03 p.m. UTC | #5 
sc7180/sm7125 (atoll) expects speedbins from atoll.dtsi:
And has a parameter: /delete-property/ qcom,gpu-speed-bin;
107 for 504Mhz max freq, pwrlevel 4
130 for 610Mhz max freq, pwrlevel 3
159 for 750Mhz max freq, pwrlevel 5
169 for 800Mhz max freq, pwrlevel 2
174 for 825Mhz max freq, pwrlevel 1 (Downstream says 172, but thats 
probably typo)
For rest of the speed bins, speed-bin value is calulated as
FMAX/4.8MHz + 2 round up to zero decimal places.

sm7150 (sdmmagpie) expects speedbins from sdmmagpie-gpu.dtsi:
128 for 610Mhz max freq, pwrlevel 3
146 for 700Mhz max freq, pwrlevel 2
167 for 800Mhz max freq, pwrlevel 4
172 for 504Mhz max freq, pwrlevel 1
For rest of the speed bins, speed-bin value is calulated as
FMAX/4.8 MHz round up to zero decimal places.

Creating a new entry does not make much sense.
I can suggest expanding the standard entry:

.speedbins = ADRENO_SPEEDBINS(
     { 0, 0 },
     /* sc7180/sm7125 */
     { 107, 3 },
     { 130, 4 },
     { 159, 5 },
     { 168, 1 }, has already
     { 174, 2 }, has already
     /* sm7150 */
     { 128, 1 },
     { 146, 2 },
     { 167, 3 },
     { 172, 4 }, ),

All the best,
Danila

On 11/22/23 23:28, Konrad Dybcio wrote:
>
>
> On 10/16/23 16:32, Dmitry Baryshkov wrote:
>> On 26/09/2023 23:03, Konrad Dybcio wrote:
>>> On 26.09.2023 21:10, Danila Tikhonov wrote:
>>>>
>>>> I think you mean by name downstream dt - sdmmagpie-gpu.dtsi
>>>>
>>>> You can see the forked version of the mainline here:
>>>> https://github.com/sm7150-mainline/linux/blob/next/arch/arm64/boot/dts/qcom/sm7150.dtsi 
>>>>
>>>>
>>>> All fdt that we got here, if it is useful for you:
>>>> https://github.com/sm7150-mainline/downstream-fdt
>>>>
>>>> Best wishes, Danila
>>> Taking a look at downstream, atoll.dtsi (SC7180) includes
>>> sdmmagpie-gpu.dtsi.
>>>
>>> Bottom line is, they share the speed bins, so it should be
>>> fine to just extend the existing entry.
>>
>> But then atoll.dtsi rewrites speed bins and pwrlevel bins. So they 
>> are not shared.
> +Akhil
>
> could you please check internally?
>
> Konrad
Akhil P Oommen Dec. 7, 2023, 7:46 p.m. UTC | #6 
On Thu, Nov 23, 2023 at 12:03:56AM +0300, Danila Tikhonov wrote:
> 
> sc7180/sm7125 (atoll) expects speedbins from atoll.dtsi:
> And has a parameter: /delete-property/ qcom,gpu-speed-bin;
> 107 for 504Mhz max freq, pwrlevel 4
> 130 for 610Mhz max freq, pwrlevel 3
> 159 for 750Mhz max freq, pwrlevel 5
> 169 for 800Mhz max freq, pwrlevel 2
> 174 for 825Mhz max freq, pwrlevel 1 (Downstream says 172, but thats probably
> typo)
A bit confused. where do you see 172 in downstream code? It is 174 in the downstream
code when I checked.
> For rest of the speed bins, speed-bin value is calulated as
> FMAX/4.8MHz + 2 round up to zero decimal places.
> 
> sm7150 (sdmmagpie) expects speedbins from sdmmagpie-gpu.dtsi:
> 128 for 610Mhz max freq, pwrlevel 3
> 146 for 700Mhz max freq, pwrlevel 2
> 167 for 800Mhz max freq, pwrlevel 4
> 172 for 504Mhz max freq, pwrlevel 1
> For rest of the speed bins, speed-bin value is calulated as
> FMAX/4.8 MHz round up to zero decimal places.
> 
> Creating a new entry does not make much sense.
> I can suggest expanding the standard entry:
> 
> .speedbins = ADRENO_SPEEDBINS(
>     { 0, 0 },
>     /* sc7180/sm7125 */
>     { 107, 3 },
>     { 130, 4 },
>     { 159, 5 },
>     { 168, 1 }, has already
>     { 174, 2 }, has already
>     /* sm7150 */
>     { 128, 1 },
>     { 146, 2 },
>     { 167, 3 },
>     { 172, 4 }, ),
> 

A difference I see between atoll and sdmmagpie is that the former
doesn't support 180Mhz. If you want to do the same, then you need to use
a new bit in the supported-hw bitfield instead of reusing an existing one.
Generally it is better to stick to exactly what downstream does.

-Akhil.

> All the best,
> Danila
> 
> On 11/22/23 23:28, Konrad Dybcio wrote:
> > 
> > 
> > On 10/16/23 16:32, Dmitry Baryshkov wrote:
> > > On 26/09/2023 23:03, Konrad Dybcio wrote:
> > > > On 26.09.2023 21:10, Danila Tikhonov wrote:
> > > > > 
> > > > > I think you mean by name downstream dt - sdmmagpie-gpu.dtsi
> > > > > 
> > > > > You can see the forked version of the mainline here:
> > > > > https://github.com/sm7150-mainline/linux/blob/next/arch/arm64/boot/dts/qcom/sm7150.dtsi
> > > > > 
> > > > > 
> > > > > All fdt that we got here, if it is useful for you:
> > > > > https://github.com/sm7150-mainline/downstream-fdt
> > > > > 
> > > > > Best wishes, Danila
> > > > Taking a look at downstream, atoll.dtsi (SC7180) includes
> > > > sdmmagpie-gpu.dtsi.
> > > > 
> > > > Bottom line is, they share the speed bins, so it should be
> > > > fine to just extend the existing entry.
> > > 
> > > But then atoll.dtsi rewrites speed bins and pwrlevel bins. So they
> > > are not shared.
> > +Akhil
> > 
> > could you please check internally?
> > 
> > Konrad
>
Konrad Dybcio Dec. 7, 2023, 7:48 p.m. UTC | #7 
On 12/7/23 20:46, Akhil P Oommen wrote:
> On Thu, Nov 23, 2023 at 12:03:56AM +0300, Danila Tikhonov wrote:
>>
>> sc7180/sm7125 (atoll) expects speedbins from atoll.dtsi:
>> And has a parameter: /delete-property/ qcom,gpu-speed-bin;
>> 107 for 504Mhz max freq, pwrlevel 4
>> 130 for 610Mhz max freq, pwrlevel 3
>> 159 for 750Mhz max freq, pwrlevel 5
>> 169 for 800Mhz max freq, pwrlevel 2
>> 174 for 825Mhz max freq, pwrlevel 1 (Downstream says 172, but thats probably
>> typo)
> A bit confused. where do you see 172 in downstream code? It is 174 in the downstream
> code when I checked.
>> For rest of the speed bins, speed-bin value is calulated as
>> FMAX/4.8MHz + 2 round up to zero decimal places.
>>
>> sm7150 (sdmmagpie) expects speedbins from sdmmagpie-gpu.dtsi:
>> 128 for 610Mhz max freq, pwrlevel 3
>> 146 for 700Mhz max freq, pwrlevel 2
>> 167 for 800Mhz max freq, pwrlevel 4
>> 172 for 504Mhz max freq, pwrlevel 1
>> For rest of the speed bins, speed-bin value is calulated as
>> FMAX/4.8 MHz round up to zero decimal places.
>>
>> Creating a new entry does not make much sense.
>> I can suggest expanding the standard entry:
>>
>> .speedbins = ADRENO_SPEEDBINS(
>>      { 0, 0 },
>>      /* sc7180/sm7125 */
>>      { 107, 3 },
>>      { 130, 4 },
>>      { 159, 5 },
>>      { 168, 1 }, has already
>>      { 174, 2 }, has already
>>      /* sm7150 */
>>      { 128, 1 },
>>      { 146, 2 },
>>      { 167, 3 },
>>      { 172, 4 }, ),
>>
> 
> A difference I see between atoll and sdmmagpie is that the former
> doesn't support 180Mhz. If you want to do the same, then you need to use
> a new bit in the supported-hw bitfield instead of reusing an existing one.
> Generally it is better to stick to exactly what downstream does.
OK I take my doubts back, let's go with adding a new one.

Konrad
diff mbox series

Patch

diff --git a/drivers/gpu/drm/msm/adreno/adreno_device.c b/drivers/gpu/drm/msm/adreno/adreno_device.c
index fa527935ffd4..cb2f459cbcc4 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_device.c
+++ b/drivers/gpu/drm/msm/adreno/adreno_device.c
@@ -293,6 +293,27 @@  static const struct adreno_info gpulist[] = {
 			{ 157, 3 },
 			{ 127, 4 },
 		),
+	}, {
+		.machine = "qcom,sm7150",
+		.chip_ids = ADRENO_CHIP_IDS(0x06010800),
+		.family = ADRENO_6XX_GEN1,
+		.fw = {
+			[ADRENO_FW_SQE] = "a630_sqe.fw",
+			[ADRENO_FW_GMU] = "a630_gmu.bin",
+		},
+		.gmem = SZ_512K,
+		.inactive_period = DRM_MSM_INACTIVE_PERIOD,
+		.quirks = ADRENO_QUIRK_HAS_CACHED_COHERENT,
+		.init = a6xx_gpu_init,
+		.zapfw = "a615_zap.mbn",
+		.hwcg = a615_hwcg,
+		.speedbins = ADRENO_SPEEDBINS(
+			{ 0,   0 },
+			{ 128, 1 },
+			{ 146, 2 },
+			{ 167, 3 },
+			{ 172, 4 },
+		),
 	}, {
 		.chip_ids = ADRENO_CHIP_IDS(0x06010800),
 		.family = ADRENO_6XX_GEN1,
@@ -507,6 +528,7 @@  MODULE_FIRMWARE("qcom/a530_zap.b00");
 MODULE_FIRMWARE("qcom/a530_zap.b01");
 MODULE_FIRMWARE("qcom/a530_zap.b02");
 MODULE_FIRMWARE("qcom/a540_gpmu.fw2");
+MODULE_FIRMWARE("qcom/a615_zap.mbn");
 MODULE_FIRMWARE("qcom/a619_gmu.bin");
 MODULE_FIRMWARE("qcom/a630_sqe.fw");
 MODULE_FIRMWARE("qcom/a630_gmu.bin");