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

From: Jonathan Marek <jonathan@marek.ca>
To: Akhil P Oommen <akhilpo@codeaurora.org>, freedreno@lists.freedesktop.org
Cc: Rob Clark <robdclark@gmail.com>, Sean Paul <sean@poorly.run>,
	David Airlie <airlied@linux.ie>, Daniel Vetter <daniel@ffwll.ch>,
	Jordan Crouse <jordan@cosmicpenguin.net>,
	Eric Anholt <eric@anholt.net>,
	Sharat Masetty <smasetty@codeaurora.org>,
	"open list:DRM DRIVER FOR MSM ADRENO GPU" 
	<linux-arm-msm@vger.kernel.org>,
	"open list:DRM DRIVER FOR MSM ADRENO GPU" 
	<dri-devel@lists.freedesktop.org>,
	open list <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v2 2/8] drm/msm/a6xx: use AOP-initialized PDC for a650
Date: Tue, 8 Jun 2021 11:54:44 -0400	[thread overview]
Message-ID: <b16de5ab-1485-a814-0885-c266a8706f0c@marek.ca> (raw)
In-Reply-To: <bd5dc0e5-2c49-31fe-a290-0d8e75b45c94@codeaurora.org>

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).