Re: [PATCH v2 2/8] drm/msm/a6xx: use AOP-initialized PDC for a650

Jonathan Marek
2021-06-08T15:56:51Z

On 5/31/21 3:24 AM, Akhil P Oommen wrote:
> On 5/13/2021 10:43 PM, Jonathan Marek wrote:
>> SM8250 AOP firmware already sets up PDC registers for us, and it only
>> needs
>> to be enabled. This path will be used for other newer GPUs.
>>
>> Signed-off-by: Jonathan Marek <jonathan@marek.ca>
>> ---
>>   drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 21 ++++++++++++++++-----
>>   1 file changed, 16 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
>> b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
>> index 3d55e153fa9c..c1ee02d6371d 100644
>> --- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
>> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
>> @@ -512,19 +512,26 @@ static void a6xx_gmu_rpmh_init(struct a6xx_gmu
>> *gmu)
>>       struct adreno_gpu *adreno_gpu = &a6xx_gpu->base;
>>       struct platform_device *pdev = to_platform_device(gmu->dev);
>>       void __iomem *pdcptr = a6xx_gmu_get_mmio(pdev, "gmu_pdc");
>> -    void __iomem *seqptr = a6xx_gmu_get_mmio(pdev, "gmu_pdc_seq");
>> +    void __iomem *seqptr;
>>       uint32_t pdc_address_offset;
>> +    bool pdc_in_aop = false;
>> -    if (!pdcptr || !seqptr)
>> +    if (!pdcptr)
>>           goto err;
>> -    if (adreno_is_a618(adreno_gpu) || adreno_is_a640(adreno_gpu))
>> +    if (adreno_is_a650(adreno_gpu))
>> +        pdc_in_aop = true;
>> +    else if (adreno_is_a618(adreno_gpu) || adreno_is_a640(adreno_gpu))
>>           pdc_address_offset = 0x30090;
>> -    else if (adreno_is_a650(adreno_gpu))
>> -        pdc_address_offset = 0x300a0;
>>       else
>>           pdc_address_offset = 0x30080;
>> +    if (!pdc_in_aop) {
>> +        seqptr = a6xx_gmu_get_mmio(pdev, "gmu_pdc_seq");
>> +        if (!seqptr)
>> +            goto err;
>> +    }
>> +
>>       /* Disable SDE clock gating */
>>       gmu_write_rscc(gmu, REG_A6XX_GPU_RSCC_RSC_STATUS0_DRV0, BIT(24));
>> @@ -556,6 +563,9 @@ static void a6xx_gmu_rpmh_init(struct a6xx_gmu *gmu)
>>           gmu_write_rscc(gmu, REG_A6XX_RSCC_SEQ_MEM_0_DRV0 + 4,
>> 0x0020e8a8);
>>       }
>> +    if (pdc_in_aop)
>> +        goto setup_pdc;
>> +
>>       /* Load PDC sequencer uCode for power up and power down sequence */
>>       pdc_write(seqptr, REG_A6XX_PDC_GPU_SEQ_MEM_0, 0xfebea1e1);
>>       pdc_write(seqptr, REG_A6XX_PDC_GPU_SEQ_MEM_0 + 1, 0xa5a4a3a2);
>> @@ -596,6 +606,7 @@ static void a6xx_gmu_rpmh_init(struct a6xx_gmu *gmu)
>>       pdc_write(pdcptr, REG_A6XX_PDC_GPU_TCS3_CMD0_DATA + 8, 0x3);
>>       /* Setup GPU PDC */
>> +setup_pdc:
>>       pdc_write(pdcptr, REG_A6XX_PDC_GPU_SEQ_START_ADDR, 0);
>>       pdc_write(pdcptr, REG_A6XX_PDC_GPU_ENABLE_PDC, 0x80000001);
>>
>
> We can simply swap the order of PDC and rsc programming here and skip
> pdc sequence to jump to the rscc programming for a650. This is the order
> followed in the downstream driver anyway.
>
> -Akhil.

The order is the same as the msm-4.19 kernel (msm-4.19 is what a650
hardware are using).

Looks like the order was swapped for the msm-5.4 kernel, but if the
order isn't important, I think it is preferable to keep the current
order (to avoid a large diff mainly).