All of lore.kernel.org
 help / color / mirror / Atom feed
From: Olav Haugan <ohaugan@codeaurora.org>
To: Rob Clark <robdclark@gmail.com>
Cc: Hiroshi Doyu <hdoyu@nvidia.com>,
	"linux-arm-msm@vger.kernel.org" <linux-arm-msm@vger.kernel.org>,
	"will.deacon@arm.com" <will.deacon@arm.com>,
	"iommu@lists.linux-foundation.org"
	<iommu@lists.linux-foundation.org>,
	"thierry.reding@gmail.com" <thierry.reding@gmail.com>,
	"vgandhi@codeaurora.org" <vgandhi@codeaurora.org>,
	"linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>,
	Joerg Roedel <joro@8bytes.org>
Subject: Re: [RFC/PATCH 2/7] iommu-api: Add map_range/unmap_range functions
Date: Wed, 09 Jul 2014 17:03:00 -0700	[thread overview]
Message-ID: <53BDD834.5030405@codeaurora.org> (raw)
In-Reply-To: <CAF6AEGvpD+gvDvQte6YVOP8Q6NeXsoReQ_waWqjsMjffUzALOA@mail.gmail.com>

On 7/8/2014 4:49 PM, Rob Clark wrote:
> On Tue, Jul 8, 2014 at 5:53 PM, Olav Haugan <ohaugan@codeaurora.org> wrote:
>> Hi Hiroshi,
>>
>> On 7/3/2014 9:29 PM, Hiroshi Doyu wrote:
>>> Hi Olav,
>>>
>>> Olav Haugan <ohaugan@codeaurora.org> writes:
>>>
>>>> Mapping and unmapping are more often than not in the critical path.
>>>> map_range and unmap_range allows SMMU driver implementations to optimize
>>>> the process of mapping and unmapping buffers into the SMMU page tables.
>>>> Instead of mapping one physical address, do TLB operation (expensive),
>>>> mapping, do TLB operation, mapping, do TLB operation the driver can map
>>>> a scatter-gatherlist of physically contiguous pages into one virtual
>>>> address space and then at the end do one TLB operation.
>>>>
>>>> Additionally, the mapping operation would be faster in general since
>>>> clients does not have to keep calling map API over and over again for
>>>> each physically contiguous chunk of memory that needs to be mapped to a
>>>> virtually contiguous region.
>>>>
>>>> Signed-off-by: Olav Haugan <ohaugan@codeaurora.org>
>>>> ---
>>>>  drivers/iommu/iommu.c | 24 ++++++++++++++++++++++++
>>>>  include/linux/iommu.h | 24 ++++++++++++++++++++++++
>>>>  2 files changed, 48 insertions(+)
>>>>
>>>> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
>>>> index e5555fc..f2a6b80 100644
>>>> --- a/drivers/iommu/iommu.c
>>>> +++ b/drivers/iommu/iommu.c
>>>> @@ -898,6 +898,30 @@ size_t iommu_unmap(struct iommu_domain *domain, unsigned long iova, size_t size)
>>>>  EXPORT_SYMBOL_GPL(iommu_unmap);
>>>>
>>>>
>>>> +int iommu_map_range(struct iommu_domain *domain, unsigned int iova,
>>>> +                struct scatterlist *sg, unsigned int len, int prot)
>>>> +{
>>>> +    if (unlikely(domain->ops->map_range == NULL))
>>>> +            return -ENODEV;
>>>> +
>>>> +    BUG_ON(iova & (~PAGE_MASK));
>>>> +
>>>> +    return domain->ops->map_range(domain, iova, sg, len, prot);
>>>> +}
>>>> +EXPORT_SYMBOL_GPL(iommu_map_range);
>>>
>>> We have the similar one internally, which is named, "iommu_map_sg()",
>>> called from DMA API.
>>
>> Great, so this new API will be useful to more people!
>>
>>>> +int iommu_unmap_range(struct iommu_domain *domain, unsigned int iova,
>>>> +                  unsigned int len)
>>>> +{
>>>> +    if (unlikely(domain->ops->unmap_range == NULL))
>>>> +            return -ENODEV;
>>>> +
>>>> +    BUG_ON(iova & (~PAGE_MASK));
>>>> +
>>>> +    return domain->ops->unmap_range(domain, iova, len);
>>>> +}
>>>> +EXPORT_SYMBOL_GPL(iommu_unmap_range);
>>>
>>> Can the existing iommu_unmap() do the same?
>>
>> I believe iommu_unmap() behaves a bit differently because it will keep
>> on calling domain->ops->unmap() until everything is unmapped instead of
>> letting the iommu implementation take care of unmapping everything in
>> one call.
>>
>> I am abandoning the patch series since our driver was not accepted.
>> However, if there are no objections I will resubmit this patch (PATCH
>> 2/7) as an independent patch to add this new map_range API.
> 
> +1 for map_range().. I've seen for gpu workloads, at least, it is the
> downstream map_range() API is quite beneficial.   It was worth at
> least a few fps in xonotic.
> 
> And, possibly getting off the subject a bit, but I was wondering about
> the possibility of going one step further and batching up mapping
> and/or unmapping multiple buffers (ranges) at once.  I have a pretty
> convenient sync point in drm/msm to flush out multiple mappings before
> kicking gpu.

I think you should be able to do that with this API already - at least
the mapping part since we are passing in a sg list (this could be a
chained sglist).

Thanks,

Olav

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

WARNING: multiple messages have this Message-ID (diff)
From: ohaugan@codeaurora.org (Olav Haugan)
To: linux-arm-kernel@lists.infradead.org
Subject: [RFC/PATCH 2/7] iommu-api: Add map_range/unmap_range functions
Date: Wed, 09 Jul 2014 17:03:00 -0700	[thread overview]
Message-ID: <53BDD834.5030405@codeaurora.org> (raw)
In-Reply-To: <CAF6AEGvpD+gvDvQte6YVOP8Q6NeXsoReQ_waWqjsMjffUzALOA@mail.gmail.com>

On 7/8/2014 4:49 PM, Rob Clark wrote:
> On Tue, Jul 8, 2014 at 5:53 PM, Olav Haugan <ohaugan@codeaurora.org> wrote:
>> Hi Hiroshi,
>>
>> On 7/3/2014 9:29 PM, Hiroshi Doyu wrote:
>>> Hi Olav,
>>>
>>> Olav Haugan <ohaugan@codeaurora.org> writes:
>>>
>>>> Mapping and unmapping are more often than not in the critical path.
>>>> map_range and unmap_range allows SMMU driver implementations to optimize
>>>> the process of mapping and unmapping buffers into the SMMU page tables.
>>>> Instead of mapping one physical address, do TLB operation (expensive),
>>>> mapping, do TLB operation, mapping, do TLB operation the driver can map
>>>> a scatter-gatherlist of physically contiguous pages into one virtual
>>>> address space and then at the end do one TLB operation.
>>>>
>>>> Additionally, the mapping operation would be faster in general since
>>>> clients does not have to keep calling map API over and over again for
>>>> each physically contiguous chunk of memory that needs to be mapped to a
>>>> virtually contiguous region.
>>>>
>>>> Signed-off-by: Olav Haugan <ohaugan@codeaurora.org>
>>>> ---
>>>>  drivers/iommu/iommu.c | 24 ++++++++++++++++++++++++
>>>>  include/linux/iommu.h | 24 ++++++++++++++++++++++++
>>>>  2 files changed, 48 insertions(+)
>>>>
>>>> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
>>>> index e5555fc..f2a6b80 100644
>>>> --- a/drivers/iommu/iommu.c
>>>> +++ b/drivers/iommu/iommu.c
>>>> @@ -898,6 +898,30 @@ size_t iommu_unmap(struct iommu_domain *domain, unsigned long iova, size_t size)
>>>>  EXPORT_SYMBOL_GPL(iommu_unmap);
>>>>
>>>>
>>>> +int iommu_map_range(struct iommu_domain *domain, unsigned int iova,
>>>> +                struct scatterlist *sg, unsigned int len, int prot)
>>>> +{
>>>> +    if (unlikely(domain->ops->map_range == NULL))
>>>> +            return -ENODEV;
>>>> +
>>>> +    BUG_ON(iova & (~PAGE_MASK));
>>>> +
>>>> +    return domain->ops->map_range(domain, iova, sg, len, prot);
>>>> +}
>>>> +EXPORT_SYMBOL_GPL(iommu_map_range);
>>>
>>> We have the similar one internally, which is named, "iommu_map_sg()",
>>> called from DMA API.
>>
>> Great, so this new API will be useful to more people!
>>
>>>> +int iommu_unmap_range(struct iommu_domain *domain, unsigned int iova,
>>>> +                  unsigned int len)
>>>> +{
>>>> +    if (unlikely(domain->ops->unmap_range == NULL))
>>>> +            return -ENODEV;
>>>> +
>>>> +    BUG_ON(iova & (~PAGE_MASK));
>>>> +
>>>> +    return domain->ops->unmap_range(domain, iova, len);
>>>> +}
>>>> +EXPORT_SYMBOL_GPL(iommu_unmap_range);
>>>
>>> Can the existing iommu_unmap() do the same?
>>
>> I believe iommu_unmap() behaves a bit differently because it will keep
>> on calling domain->ops->unmap() until everything is unmapped instead of
>> letting the iommu implementation take care of unmapping everything in
>> one call.
>>
>> I am abandoning the patch series since our driver was not accepted.
>> However, if there are no objections I will resubmit this patch (PATCH
>> 2/7) as an independent patch to add this new map_range API.
> 
> +1 for map_range().. I've seen for gpu workloads, at least, it is the
> downstream map_range() API is quite beneficial.   It was worth at
> least a few fps in xonotic.
> 
> And, possibly getting off the subject a bit, but I was wondering about
> the possibility of going one step further and batching up mapping
> and/or unmapping multiple buffers (ranges) at once.  I have a pretty
> convenient sync point in drm/msm to flush out multiple mappings before
> kicking gpu.

I think you should be able to do that with this API already - at least
the mapping part since we are passing in a sg list (this could be a
chained sglist).

Thanks,

Olav

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

  reply	other threads:[~2014-07-10  0:03 UTC|newest]

Thread overview: 59+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-06-30 16:51 [RFC/PATCH 0/7] Add MSM SMMUv1 support Olav Haugan
2014-06-30 16:51 ` Olav Haugan
2014-06-30 16:51 ` [RFC/PATCH 1/7] iommu: msm: Rename iommu driver files Olav Haugan
2014-06-30 16:51   ` Olav Haugan
2014-06-30 16:51 ` [RFC/PATCH 2/7] iommu-api: Add map_range/unmap_range functions Olav Haugan
2014-06-30 16:51   ` Olav Haugan
2014-06-30 19:42   ` Thierry Reding
2014-06-30 19:42     ` Thierry Reding
2014-07-01  9:33   ` Will Deacon
2014-07-01  9:33     ` Will Deacon
2014-07-01  9:58     ` Varun Sethi
2014-07-01  9:58       ` Varun Sethi
2014-07-04  4:29   ` Hiroshi Doyu
2014-07-04  4:29     ` Hiroshi Doyu
2014-07-08 21:53     ` Olav Haugan
2014-07-08 21:53       ` Olav Haugan
2014-07-08 23:49       ` Rob Clark
2014-07-08 23:49         ` Rob Clark
2014-07-10  0:03         ` Olav Haugan [this message]
2014-07-10  0:03           ` Olav Haugan
     [not found]           ` <53BDD834.5030405-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
2014-07-10  0:40             ` Rob Clark
2014-07-10  0:40               ` Rob Clark
2014-07-10  7:10               ` Thierry Reding
2014-07-10  7:10                 ` Thierry Reding
2014-07-10 11:15                 ` Rob Clark
2014-07-10 11:15                   ` Rob Clark
     [not found]               ` <CAF6AEGucNbo7sm9oQWFq9hcfoSeR5DuwRcRUvG+Y2sxLaM7OTQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2014-07-10 22:43                 ` Olav Haugan
2014-07-10 22:43                   ` Olav Haugan
2014-07-10 23:42                   ` Rob Clark
2014-07-10 23:42                     ` Rob Clark
2014-07-11 10:20   ` Joerg Roedel
2014-07-11 10:20     ` Joerg Roedel
     [not found]     ` <20140711102053.GB1958-zLv9SwRftAIdnm+yROfE0A@public.gmane.org>
2014-07-15  1:13       ` Olav Haugan
2014-07-15  1:13         ` Olav Haugan
     [not found] ` <1404147116-4598-1-git-send-email-ohaugan-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
2014-06-30 16:51   ` [RFC/PATCH 3/7] iopoll: Introduce memory-mapped IO polling macros Olav Haugan
2014-06-30 16:51     ` Olav Haugan
     [not found]     ` <1404147116-4598-4-git-send-email-ohaugan-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
2014-06-30 19:46       ` Thierry Reding
2014-06-30 19:46         ` Thierry Reding
2014-07-01  9:40       ` Will Deacon
2014-07-01  9:40         ` Will Deacon
2014-06-30 16:51   ` [RFC/PATCH 4/7] iommu: msm: Add MSM IOMMUv1 driver Olav Haugan
     [not found]     ` <1404147116-4598-5-git-send-email-ohaugan-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
2014-06-30 17:02       ` Will Deacon
2014-06-30 17:02         ` Will Deacon
     [not found]         ` <20140630170221.GA30740-5wv7dgnIgG8@public.gmane.org>
2014-07-02 22:32           ` Olav Haugan
2014-07-02 22:32             ` Olav Haugan
2014-06-30 16:51 ` [RFC/PATCH 5/7] iommu: msm: Add support for V7L page table format Olav Haugan
2014-06-30 16:51   ` Olav Haugan
2014-06-30 16:51 ` [RFC/PATCH 6/7] defconfig: msm: Enable Qualcomm SMMUv1 driver Olav Haugan
2014-06-30 16:51   ` Olav Haugan
2014-06-30 16:51 ` [RFC/PATCH 7/7] iommu-api: Add domain attribute to enable coherent HTW Olav Haugan
2014-06-30 16:51   ` Olav Haugan
     [not found]   ` <1404147116-4598-8-git-send-email-ohaugan-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
2014-07-01  8:49     ` Varun Sethi
2014-07-01  8:49       ` Varun Sethi
2014-07-02 22:11       ` Olav Haugan
2014-07-02 22:11         ` Olav Haugan
     [not found]         ` <53B48381.9050707-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
2014-07-03 17:43           ` Will Deacon
2014-07-03 17:43             ` Will Deacon
     [not found]             ` <20140703174321.GE17372-5wv7dgnIgG8@public.gmane.org>
2014-07-08 22:24               ` Olav Haugan
2014-07-08 22:24                 ` Olav Haugan

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=53BDD834.5030405@codeaurora.org \
    --to=ohaugan@codeaurora.org \
    --cc=hdoyu@nvidia.com \
    --cc=iommu@lists.linux-foundation.org \
    --cc=joro@8bytes.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=robdclark@gmail.com \
    --cc=thierry.reding@gmail.com \
    --cc=vgandhi@codeaurora.org \
    --cc=will.deacon@arm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.