Re: [PATCHv3 01/11] staging: tidspbridge: replace iommu custom for opensource implementation
Hi, On Mon, 2010-10-18 at 01:36 +0300, Felipe Contreras wrote: On Tue, Oct 5, 2010 at 11:35 PM, Fernando Guzman Lugo x0095...@ti.com wrote: Now the tidspbridge uses the API's from iovmm module. Signed-off-by: Fernando Guzman Lugo x0095...@ti.com NAK. This patch doesn't work... I guess it's supposed to. So far I've found these errors, but it still doesn't work, maybe we should start thinking of reverting the whole iommu stuff: I also tested it with the userspace-dspbridge tools and it doesn't work for me either. Maybe it's better to revert this patch series for now and re-submit it when all necessary patches get merged and it actually works on top of the staging-next tree? Regards, Ionut. -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv3 01/11] staging: tidspbridge: replace iommu custom for opensource implementation
On Mon, Oct 18, 2010 at 3:06 PM, Ionut Nicu ionut.n...@mindbit.ro wrote: On Mon, 2010-10-18 at 01:36 +0300, Felipe Contreras wrote: On Tue, Oct 5, 2010 at 11:35 PM, Fernando Guzman Lugo x0095...@ti.com wrote: Now the tidspbridge uses the API's from iovmm module. Signed-off-by: Fernando Guzman Lugo x0095...@ti.com NAK. This patch doesn't work... I guess it's supposed to. So far I've found these errors, but it still doesn't work, maybe we should start thinking of reverting the whole iommu stuff: I also tested it with the userspace-dspbridge tools and it doesn't work for me either. Maybe it's better to revert this patch series for now and re-submit it when all necessary patches get merged and it actually works on top of the staging-next tree? I was thinking that rather than reverting it might make sense to have configuration option to choose from one to the other. I'm trying to minimize the changes on this patch to see what else I can catch, apply the fixes on top of staging-next, and then add support for the old mmu part, hopefully in a form that is similar to the current iommu. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv3 01/11] staging: tidspbridge: replace iommu custom for opensource implementation
On Tue, Oct 5, 2010 at 11:35 PM, Fernando Guzman Lugo x0095...@ti.com wrote:
Now the tidspbridge uses the API's from
iovmm module.
Signed-off-by: Fernando Guzman Lugo x0095...@ti.com
NAK. This patch doesn't work... I guess it's supposed to.
So far I've found these errors, but it still doesn't work, maybe we
should start thinking of reverting the whole iommu stuff:
diff --git a/drivers/staging/tidspbridge/core/io_sm.c
b/drivers/staging/tidspbridge/core/io_sm.c
index 842b8db..16cf246 100644
--- a/drivers/staging/tidspbridge/core/io_sm.c
+++ b/drivers/staging/tidspbridge/core/io_sm.c
@@ -541,6 +541,14 @@ int bridge_io_on_loaded(struct io_mgr *hio_mgr)
ae_proc[ndx].ul_dsp_va *
hio_mgr-word_size, page_size[i]);
ndx++;
+ } else {
+ u32 tmp_curr;
+ tmp_curr = iommu_kmap(mmu, va_curr, pa_curr,
page_size[i],
+ IOVMF_ENDIAN_LITTLE |
IOVMF_ELSZ_32);
+ if (IS_ERR_VALUE(tmp_curr)) {
+ status = (int)tmp_curr;
+ goto func_end;
+ }
}
pa_curr += page_size[i];
va_curr += page_size[i];
@@ -593,6 +601,15 @@ int bridge_io_on_loaded(struct io_mgr *hio_mgr)
DSP_VA 0x%x\n, ae_proc[ndx].ul_gpp_pa,
ae_proc[ndx].ul_dsp_va);
ndx++;
+ } else {
+ u32 tmp_curr;
+ tmp_curr = iommu_kmap(mmu,
+
hio_mgr-ext_proc_info.ty_tlb[i].ul_dsp_virt,
+
hio_mgr-ext_proc_info.ty_tlb[i].ul_gpp_phys,
+ 0x10,
+ IOVMF_ENDIAN_LITTLE |
IOVMF_ELSZ_32);
+ if (IS_ERR_VALUE(tmp_curr))
+ status = (int)tmp_curr;
}
}
if (status)
diff --git a/drivers/staging/tidspbridge/core/tiomap3430.c
b/drivers/staging/tidspbridge/core/tiomap3430.c
index c2f5105..92774fc 100644
--- a/drivers/staging/tidspbridge/core/tiomap3430.c
+++ b/drivers/staging/tidspbridge/core/tiomap3430.c
@@ -1195,7 +1195,7 @@ static int get_io_pages(struct mm_struct *mm,
u32 uva, unsigned pages,
struct page *pg;
for (i = 0; i pages; i++) {
- pa = user_va2_pa(mm, uva);
+ pa = user_va2_pa(mm, uva + i * PAGE_SIZE);
if (!pfn_valid(__phys_to_pfn(pa)))
break;
--
Felipe Contreras
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv3 01/11] staging: tidspbridge: replace iommu custom for opensource implementation
Hi Fernando,
I have few comments below.
diff --git a/drivers/staging/tidspbridge/core/io_sm.c
b/drivers/staging/tidspbridge/core/io_sm.c
index 5718645..842b8db 100644
--- a/drivers/staging/tidspbridge/core/io_sm.c
+++ b/drivers/staging/tidspbridge/core/io_sm.c
@@ -291,6 +291,7 @@ int bridge_io_on_loaded(struct io_mgr *hio_mgr)
struct cod_manager *cod_man;
struct chnl_mgr *hchnl_mgr;
struct msg_mgr *hmsg_mgr;
+ struct iommu *mmu;
u32 ul_shm_base;
u32 ul_shm_base_offset;
u32 ul_shm_limit;
@@ -313,7 +314,6 @@ int bridge_io_on_loaded(struct io_mgr *hio_mgr)
struct bridge_ioctl_extproc ae_proc[BRDIOCTL_NUMOFMMUTLB];
struct cfg_hostres *host_res;
struct bridge_dev_context *pbridge_context;
- u32 map_attrs;
u32 shm0_end;
u32 ul_dyn_ext_base;
u32 ul_seg1_size = 0;
@@ -337,6 +337,20 @@ int bridge_io_on_loaded(struct io_mgr *hio_mgr)
status = -EFAULT;
goto func_end;
}
+ mmu = pbridge_context-dsp_mmu;
+
+ if (mmu)
+ iommu_put(mmu);
+ mmu = iommu_get(iva2);
+
+ if (IS_ERR_OR_NULL(mmu)) {
You can use IS_ERR() instead.
[snip]
@@ -1122,217 +1081,81 @@ static int bridge_brd_mem_write(struct
bridge_dev_context *dev_ctxt,
*
* TODO: Disable MMU while updating the page tables (but that'll stall DSP)
*/
-static int bridge_brd_mem_map(struct bridge_dev_context *dev_ctxt,
- u32 ul_mpu_addr, u32 virt_addr,
- u32 ul_num_bytes, u32 ul_map_attr,
- struct page **mapped_pages)
+static int bridge_brd_mem_map(struct bridge_dev_context *dev_ctx,
+ u32 uva, u32 da, u32 size, u32 attr,
+ struct page **usr_pgs)
+
{
- u32 attrs;
- int status = 0;
- struct bridge_dev_context *dev_context = dev_ctxt;
- struct hw_mmu_map_attrs_t hw_attrs;
+ int res, w;
+ unsigned pages, i;
+ struct iommu *mmu = dev_ctx-dsp_mmu;
struct vm_area_struct *vma;
struct mm_struct *mm = current-mm;
- u32 write = 0;
- u32 num_usr_pgs = 0;
- struct page *mapped_page, *pg;
- s32 pg_num;
- u32 va = virt_addr;
- struct task_struct *curr_task = current;
- u32 pg_i = 0;
- u32 mpu_addr, pa;
-
- dev_dbg(bridge,
- %s hDevCtxt %p, pa %x, va %x, size %x, ul_map_attr %x\n,
- __func__, dev_ctxt, ul_mpu_addr, virt_addr, ul_num_bytes,
- ul_map_attr);
- if (ul_num_bytes == 0)
- return -EINVAL;
+ struct sg_table *sgt;
+ struct scatterlist *sg;
- if (ul_map_attr DSP_MAP_DIR_MASK) {
- attrs = ul_map_attr;
- } else {
- /* Assign default attributes */
- attrs = ul_map_attr | (DSP_MAPVIRTUALADDR |
DSP_MAPELEMSIZE16);
- }
- /* Take mapping properties */
- if (attrs DSP_MAPBIGENDIAN)
- hw_attrs.endianism = HW_BIG_ENDIAN;
- else
- hw_attrs.endianism = HW_LITTLE_ENDIAN;
-
- hw_attrs.mixed_size = (enum hw_mmu_mixed_size_t)
- ((attrs DSP_MAPMIXEDELEMSIZE) 2);
- /* Ignore element_size if mixed_size is enabled */
- if (hw_attrs.mixed_size == 0) {
- if (attrs DSP_MAPELEMSIZE8) {
- /* Size is 8 bit */
- hw_attrs.element_size = HW_ELEM_SIZE8BIT;
- } else if (attrs DSP_MAPELEMSIZE16) {
- /* Size is 16 bit */
- hw_attrs.element_size = HW_ELEM_SIZE16BIT;
- } else if (attrs DSP_MAPELEMSIZE32) {
- /* Size is 32 bit */
- hw_attrs.element_size = HW_ELEM_SIZE32BIT;
- } else if (attrs DSP_MAPELEMSIZE64) {
- /* Size is 64 bit */
- hw_attrs.element_size = HW_ELEM_SIZE64BIT;
- } else {
- /*
-* Mixedsize isn't enabled, so size can't be
-* zero here
-*/
- return -EINVAL;
- }
- }
- if (attrs DSP_MAPDONOTLOCK)
- hw_attrs.donotlockmpupage = 1;
- else
- hw_attrs.donotlockmpupage = 0;
+ if (!size || !usr_pgs)
+ return -EINVAL;
- if (attrs DSP_MAPVMALLOCADDR) {
- return mem_map_vmalloc(dev_ctxt, ul_mpu_addr, virt_addr,
- ul_num_bytes, hw_attrs);
- }
- /*
-* Do OS-specific user-va to pa translation.
-* Combine physically contiguous regions to reduce TLBs.
-* Pass the translated pa to pte_update.
-*/
-
RE: [PATCHv3 01/11] staging: tidspbridge: replace iommu custom for opensource implementation
-Original Message-
From: David Cohen [mailto:david.co...@nokia.com]
Sent: Wednesday, October 06, 2010 12:33 PM
To: Guzman Lugo, Fernando
Cc: gre...@suse.de; Contreras Felipe (Nokia-MS/Helsinki);
Palande Ameya (Nokia-MS/Helsinki); Menon, Nishanth; Doyu
Hiroshi (Nokia-MS/Espoo); o...@wizery.com;
linux-ker...@vger.kernel.org; andy.shevche...@gmail.com;
linux-omap@vger.kernel.org
Subject: Re: [PATCHv3 01/11] staging: tidspbridge: replace
iommu custom for opensource implementation
Hi Fernando,
I have few comments below.
Hi David,
Thanks for review.
diff --git a/drivers/staging/tidspbridge/core/io_sm.c
b/drivers/staging/tidspbridge/core/io_sm.c
index 5718645..842b8db 100644
--- a/drivers/staging/tidspbridge/core/io_sm.c
+++ b/drivers/staging/tidspbridge/core/io_sm.c
@@ -291,6 +291,7 @@ int bridge_io_on_loaded(struct io_mgr *hio_mgr)
struct cod_manager *cod_man;
struct chnl_mgr *hchnl_mgr;
struct msg_mgr *hmsg_mgr;
+ struct iommu *mmu;
u32 ul_shm_base;
u32 ul_shm_base_offset;
u32 ul_shm_limit;
@@ -313,7 +314,6 @@ int bridge_io_on_loaded(struct io_mgr *hio_mgr)
struct bridge_ioctl_extproc ae_proc[BRDIOCTL_NUMOFMMUTLB];
struct cfg_hostres *host_res;
struct bridge_dev_context *pbridge_context;
- u32 map_attrs;
u32 shm0_end;
u32 ul_dyn_ext_base;
u32 ul_seg1_size = 0;
@@ -337,6 +337,20 @@ int bridge_io_on_loaded(struct io_mgr *hio_mgr)
status = -EFAULT;
goto func_end;
}
+ mmu = pbridge_context-dsp_mmu;
+
+ if (mmu)
+ iommu_put(mmu);
+ mmu = iommu_get(iva2);
+
+ if (IS_ERR_OR_NULL(mmu)) {
You can use IS_ERR() instead.
The patch are already merged, so I cannot modified this patch. Even tought
That change is done when all related code is change to the new file
Dsp-mmu.c. Please check patch 7/11.
[snip]
@@ -1122,217 +1081,81 @@ static int
bridge_brd_mem_write(struct bridge_dev_context *dev_ctxt,
*
* TODO: Disable MMU while updating the page tables (but
that'll stall DSP)
*/
-static int bridge_brd_mem_map(struct bridge_dev_context *dev_ctxt,
- u32 ul_mpu_addr, u32 virt_addr,
- u32 ul_num_bytes, u32 ul_map_attr,
- struct page **mapped_pages)
+static int bridge_brd_mem_map(struct bridge_dev_context *dev_ctx,
+ u32 uva, u32 da, u32 size, u32 attr,
+ struct page **usr_pgs)
+
{
- u32 attrs;
- int status = 0;
- struct bridge_dev_context *dev_context = dev_ctxt;
- struct hw_mmu_map_attrs_t hw_attrs;
+ int res, w;
+ unsigned pages, i;
+ struct iommu *mmu = dev_ctx-dsp_mmu;
struct vm_area_struct *vma;
struct mm_struct *mm = current-mm;
- u32 write = 0;
- u32 num_usr_pgs = 0;
- struct page *mapped_page, *pg;
- s32 pg_num;
- u32 va = virt_addr;
- struct task_struct *curr_task = current;
- u32 pg_i = 0;
- u32 mpu_addr, pa;
-
- dev_dbg(bridge,
- %s hDevCtxt %p, pa %x, va %x, size %x,
ul_map_attr %x\n,
- __func__, dev_ctxt, ul_mpu_addr, virt_addr,
ul_num_bytes,
- ul_map_attr);
- if (ul_num_bytes == 0)
- return -EINVAL;
+ struct sg_table *sgt;
+ struct scatterlist *sg;
- if (ul_map_attr DSP_MAP_DIR_MASK) {
- attrs = ul_map_attr;
- } else {
- /* Assign default attributes */
- attrs = ul_map_attr | (DSP_MAPVIRTUALADDR |
DSP_MAPELEMSIZE16);
- }
- /* Take mapping properties */
- if (attrs DSP_MAPBIGENDIAN)
- hw_attrs.endianism = HW_BIG_ENDIAN;
- else
- hw_attrs.endianism = HW_LITTLE_ENDIAN;
-
- hw_attrs.mixed_size = (enum hw_mmu_mixed_size_t)
- ((attrs DSP_MAPMIXEDELEMSIZE) 2);
- /* Ignore element_size if mixed_size is enabled */
- if (hw_attrs.mixed_size == 0) {
- if (attrs DSP_MAPELEMSIZE8) {
- /* Size is 8 bit */
- hw_attrs.element_size = HW_ELEM_SIZE8BIT;
- } else if (attrs DSP_MAPELEMSIZE16) {
- /* Size is 16 bit */
- hw_attrs.element_size = HW_ELEM_SIZE16BIT;
- } else if (attrs DSP_MAPELEMSIZE32) {
- /* Size is 32 bit */
- hw_attrs.element_size = HW_ELEM_SIZE32BIT;
- } else if (attrs DSP_MAPELEMSIZE64) {
- /* Size is 64 bit */
- hw_attrs.element_size = HW_ELEM_SIZE64BIT;
- } else
