Re: [PATCH v2 6/8] drm/msm/a6xx: add support for Adreno 660 GPU

[Jonathan Marek]

On 5/31/21 11:05 AM, Akhil P Oommen wrote:
> On 5/13/2021 10:44 PM, Jonathan Marek wrote:

...

> > @@ -519,7 +519,7 @@ static void a6xx_gmu_rpmh_init(struct a6xx_gmu *gmu)
> >       if (!pdcptr)
> >           goto err;
> > -    if (adreno_is_a650(adreno_gpu))
> > +    if (adreno_is_a650(adreno_gpu) || adreno_is_a660(adreno_gpu))
>
> why not adreno_is_a650_family() here?

Based on downstream, a620 is part of a650_family but does not have 
pdc_in_aop flag.

> > @@ -751,7 +751,7 @@ static int a6xx_gmu_fw_start(struct a6xx_gmu *gmu, 
> > unsigned int state)
> >       int ret;
> >       u32 chipid;
> We need to program this register here:
> gmu_write(gmu, REG_A6XX_GPU_GMU_CX_GMU_CX_FALNEXT_INTF, 1);

msm-4.19 does not have this write for a650, but msm-5.4 then adds it. 
Will make it a separate change since it affects a650 and not just a660.

> > -    if (adreno_is_a650(adreno_gpu))
> > +    if (adreno_is_a650(adreno_gpu) || adreno_is_a660(adreno_gpu))
> >           gmu_write(gmu, REG_A6XX_GPU_GMU_CX_GMU_CX_FAL_INTF, 1);
> >       if (state == GMU_WARM_BOOT) {
> > @@ -1494,12 +1494,28 @@ int a6xx_gmu_init(struct a6xx_gpu *a6xx_gpu, 
> > struct device_node *node)
> >       if (ret)
> >           goto err_put_device;
> > +
> > +    /* A660 now requires handling "prealloc requests" in GMU firmware
> > +     * For now just hardcode allocations based on the known firmware.
> > +     * note: there is no indication that these correspond to "dummy" or
> > +     * "debug" regions, but this "guess" allows reusing these BOs which
> > +     * are otherwise unused by a660.
> > +     */
> > +    gmu->dummy.size = SZ_4K;
> > +    if (adreno_is_a660(adreno_gpu)) {
> > +        ret = a6xx_gmu_memory_alloc(gmu, &gmu->debug, SZ_4K * 7, 
> > 0x60400000);
> > +        if (ret)
> > +            goto err_memory;
>
> I think we can simply ignore this allocation for a660 because it was 
> required for an unused feature. Do you see any issue if you ignore this 
> allocation?

Yes, without it there will be an error:

arm-smmu 3da0000.iommu: Unhandled context fault: fsr=0x402, 
iova=0x60400000, fsynr=0x32, cbfrsynra=0x5, cb=2

> > +
> > +        gmu->dummy.size = SZ_8K;
> > +    }
> > +
> >       /* Allocate memory for the GMU dummy page */
> > -    ret = a6xx_gmu_memory_alloc(gmu, &gmu->dummy, SZ_4K, 0x60000000);
> > +    ret = a6xx_gmu_memory_alloc(gmu, &gmu->dummy, gmu->dummy.size, 
> > 0x60000000);
> >       if (ret)
> >           goto err_memory;
> > -    if (adreno_is_a650(adreno_gpu)) {
> > +    if (adreno_is_a650_family(adreno_gpu)) {
> >           ret = a6xx_gmu_memory_alloc(gmu, &gmu->icache,
> >               SZ_16M - SZ_16K, 0x04000);
> >           if (ret)
> > @@ -885,6 +937,13 @@ static int a6xx_hw_init(struct msm_gpu *gpu)
> >       /* Protect registers from the CP */
> >       a6xx_set_cp_protect(gpu);
> > +    if (adreno_is_a660(adreno_gpu)) {
> > +        gpu_write(gpu, REG_A6XX_CP_CHICKEN_DBG, 0x1);
> > +        gpu_write(gpu, REG_A6XX_RBBM_GBIF_CLIENT_QOS_CNTL, 0x0);
> > +        /* Set dualQ + disable afull for A660 GPU but not for A635 */
> > +        gpu_write(gpu, REG_A6XX_UCHE_CMDQ_CONFIG, 0x66906);
> > +    }
> > +
> gpu_rmw(gpu, REG_A6XX_PC_DBG_ECO_CNTL, 0, (1 << 9));
> We need this for a640, a650 and a660.

Will make this a separate patch too, since it affects non-a660 GPUs too.

> >       /* Enable expanded apriv for targets that support it */
> >       if (gpu->hw_apriv) {
> >           gpu_write(gpu, REG_A6XX_CP_APRIV_CNTL,
> > +/* check for a650, a660, or any derivatives */
> > +static inline int adreno_is_a650_family(struct adreno_gpu *gpu)
> > +{
> > +       return gpu->revn == 650 || gpu->revn == 620 ||
> > +              gpu->revn == 660 || gpu->revn == 635;
> We can remove 635 references throughout since that is not a valid adreno 
> chipid anymore.
>
> -Akhil

I will remove it for my patch (it can discussed when adding 635 support, 
but I think you will need to have a 6xx ID for the GPU)

> > +}
> > +
> >   int adreno_get_param(struct msm_gpu *gpu, uint32_t param, uint64_t 
> > *value);
> >   const struct firmware *adreno_request_fw(struct adreno_gpu *adreno_gpu,
> >           const char *fwname);