Re: [PATCHv3 7/7] drm/msm/a6xx: Add support for using system cache(LLC)

From: Sai Prakash Ranjan
Date: Fri Jul 03 2020 - 12:59:25 EST


On 2020-07-03 21:34, Rob Clark wrote:
On Fri, Jul 3, 2020 at 7:53 AM Sai Prakash Ranjan
<saiprakash.ranjan@xxxxxxxxxxxxxx> wrote:

Hi Will,

On 2020-07-03 19:07, Will Deacon wrote:
> On Mon, Jun 29, 2020 at 09:22:50PM +0530, Sai Prakash Ranjan wrote:
>> diff --git a/drivers/gpu/drm/msm/msm_iommu.c
>> b/drivers/gpu/drm/msm/msm_iommu.c
>> index f455c597f76d..bd1d58229cc2 100644
>> --- a/drivers/gpu/drm/msm/msm_iommu.c
>> +++ b/drivers/gpu/drm/msm/msm_iommu.c
>> @@ -218,6 +218,9 @@ static int msm_iommu_map(struct msm_mmu *mmu,
>> uint64_t iova,
>> iova |= GENMASK_ULL(63, 49);
>>
>>
>> + if (mmu->features & MMU_FEATURE_USE_SYSTEM_CACHE)
>> + prot |= IOMMU_SYS_CACHE_ONLY;
>
> Given that I think this is the only user of IOMMU_SYS_CACHE_ONLY, then
> it
> looks like it should actually be a property on the domain because we
> never
> need to configure it on a per-mapping basis within a domain, and
> therefore
> it shouldn't be exposed by the IOMMU API as a prot flag.
>
> Do you agree?
>

GPU being the only user is for now, but there are other clients which
can use this.
Plus how do we set the memory attributes if we do not expose this as
prot flag?

It does appear that the downstream kgsl driver sets this for basically
all mappings.. well there is some conditional stuff around
DOMAIN_ATTR_USE_LLC_NWA but it seems based on the property of the
domain. (Jordan may know more about what that is about.) But looks
like there are a lot of different paths into iommu_map in kgsl so I
might have missed something.

Assuming there isn't some case where we specifically don't want to use
the system cache for some mapping, I think it could be a domain
attribute that sets an io_pgtable_cfg::quirks flag


Ok then we are good to remove unused sys cache prot flag which Will has posted.

Thanks,
Sai

--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation