[Nouveau] [PATCH 2/6] instmem/gk20a: refer to IOMMU physical translation bit

[Alexandre Courbot]

On Fri, Apr 17, 2015 at 4:19 PM, Vince Hsu <vinceh at nvidia.com> wrote:
>
> On 04/17/2015 02:26 PM, Alexandre Courbot wrote:
>>
>> On Thu, Apr 16, 2015 at 8:06 PM, Vince Hsu <vinceh at nvidia.com> wrote:
>>>
>>> Instead of hard-coding the translation bit in subdev driver, we refer to
>>> the platform data.
>>>
>>> Signed-off-by: Vince Hsu <vinceh at nvidia.com>
>>> ---
>>>   drm/nouveau/nvkm/subdev/instmem/gk20a.c | 13 +++++++++----
>>>   1 file changed, 9 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drm/nouveau/nvkm/subdev/instmem/gk20a.c
>>> b/drm/nouveau/nvkm/subdev/instmem/gk20a.c
>>> index dd0994d9ebfc..69ef5eae3279 100644
>>> --- a/drm/nouveau/nvkm/subdev/instmem/gk20a.c
>>> +++ b/drm/nouveau/nvkm/subdev/instmem/gk20a.c
>>> @@ -89,6 +89,7 @@ struct gk20a_instmem_priv {
>>>          struct nvkm_mm *mm;
>>>          struct iommu_domain *domain;
>>>          unsigned long iommu_pgshift;
>>> +       unsigned long iommu_phys_addr_bit;
>>>
>>>          /* Only used by DMA API */
>>>          struct dma_attrs attrs;
>>> @@ -169,8 +170,8 @@ gk20a_instobj_dtor_iommu(struct gk20a_instobj_priv
>>> *_node)
>>>          r = list_first_entry(&_node->mem->regions, struct nvkm_mm_node,
>>>                               rl_entry);
>>>
>>> -       /* clear bit 34 to unmap pages */
>>> -       r->offset &= ~BIT(34 - priv->iommu_pgshift);
>>> +       /* clear IOMMU translation bit to unmap pages */
>>> +       r->offset &= ~BIT(priv->iommu_phys_addr_bit -
>>> priv->iommu_pgshift);
>>>
>>>          /* Unmap pages from GPU address space and free them */
>>>          for (i = 0; i < _node->mem->size; i++) {
>>> @@ -298,8 +299,11 @@ gk20a_instobj_ctor_iommu(struct nvkm_object *parent,
>>> struct nvkm_object *engine,
>>>                  }
>>>          }
>>>
>>> -       /* Bit 34 tells that an address is to be resolved through the
>>> IOMMU */
>>> -       r->offset |= BIT(34 - priv->iommu_pgshift);
>>> +       /*
>>> +        * The iommu_phys_addr_bit tells that an address is to be
>>> resolved
>>> +        * through the IOMMU
>>> +        */
>>> +       r->offset |= BIT(priv->iommu_phys_addr_bit -
>>> priv->iommu_pgshift);
>>>
>>>          node->base._mem.offset = ((u64)r->offset) <<
>>> priv->iommu_pgshift;
>>>
>>> @@ -407,6 +411,7 @@ gk20a_instmem_ctor(struct nvkm_object *parent, struct
>>> nvkm_object *engine,
>>>                  priv->domain = plat->gpu->iommu.domain;
>>>                  priv->mm = plat->gpu->iommu.mm;
>>>                  priv->iommu_pgshift = plat->gpu->iommu.pgshift;
>>> +               priv->iommu_phys_addr_bit =
>>> plat->gpu->iommu.phys_addr_bit;
>>
>> Looks good, but I think I would definitely prefer this to be a mask
>> instead of a bit index, i.e:
>>
>>      r->offset &= ~(priv->iommu_addr_mask >> priv->iommu_pgshift);
>>
>> and
>>
>>      r->offset |= (priv->iommu_addr_mask >> priv->iommu_pgshift);
>
> Which one is for setting up an IOMMU address If the iommu_addr_mask is
> defined as ~BIT_ULL(34)?

I would define iomm_addr_mask as BIT_ULL(34). Then the |= is for
setting the mask and &= to remove it.