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

[Jonathan Marek]

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