You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by GitBox <gi...@apache.org> on 2022/03/28 10:24:11 UTC
[GitHub] [incubator-nuttx] no1wudi opened a new pull request #5878: arch/risc-v: Use riscv_fpuconfig to enable FPU
no1wudi opened a new pull request #5878:
URL: https://github.com/apache/incubator-nuttx/pull/5878
## Summary
* Rename up_fpuconfig to riscv_fpuconfig
* Use common function instead of chip specified code to enable FPU
## Impact
RISC-V with FPU support
## Testing
QEMU
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #5878: arch/risc-v: Use riscv_fpuconfig to enable FPU
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #5878:
URL: https://github.com/apache/incubator-nuttx/pull/5878#discussion_r837023910
##########
File path: arch/risc-v/src/c906/c906_start.c
##########
@@ -73,6 +73,10 @@ void __c906_start(uint32_t mhartid)
const uint32_t *src;
uint32_t *dest;
+ /* Configure FPU */
+
+ riscv_fpuconfig();
Review comment:
@pussuw could you look the change in mpfs is good?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #5878: arch/risc-v: Use riscv_fpuconfig to enable FPU
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #5878:
URL: https://github.com/apache/incubator-nuttx/pull/5878#discussion_r837002872
##########
File path: arch/risc-v/src/c906/c906_start.c
##########
@@ -73,6 +73,10 @@ void __c906_start(uint32_t mhartid)
const uint32_t *src;
uint32_t *dest;
+ /* Configure FPU */
+
+ riscv_fpuconfig();
Review comment:
But should we remove FPU related config from MPFS/head.S too to unify the FPU initialization process and avoid the duplication?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] no1wudi commented on a change in pull request #5878: arch/risc-v: Use riscv_fpuconfig to enable FPU
Posted by GitBox <gi...@apache.org>.
no1wudi commented on a change in pull request #5878:
URL: https://github.com/apache/incubator-nuttx/pull/5878#discussion_r837104272
##########
File path: arch/risc-v/src/mpfs/mpfs_start.c
##########
@@ -116,6 +116,10 @@ void __mpfs_start(uint64_t mhartid)
const uint32_t *src;
uint32_t *dest;
+ /* Configure FPU */
+
+ riscv_fpuconfig();
Review comment:
Got it
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pussuw commented on a change in pull request #5878: arch/risc-v: Use riscv_fpuconfig to enable FPU
Posted by GitBox <gi...@apache.org>.
pussuw commented on a change in pull request #5878:
URL: https://github.com/apache/incubator-nuttx/pull/5878#discussion_r837097550
##########
File path: arch/risc-v/src/c906/c906_start.c
##########
@@ -73,6 +73,10 @@ void __c906_start(uint32_t mhartid)
const uint32_t *src;
uint32_t *dest;
+ /* Configure FPU */
+
+ riscv_fpuconfig();
Review comment:
I also do not have a very simple way to test this change but at least the differences I noticed is that hart0 in mpfs does not have an fpu and we handle that in mpfs_head.S. The other change is that we mark the FPU as dirty, but init should be fine too.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #5878: arch/risc-v: Use riscv_fpuconfig to enable FPU
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #5878:
URL: https://github.com/apache/incubator-nuttx/pull/5878#discussion_r837035678
##########
File path: arch/risc-v/src/mpfs/mpfs_head.S
##########
@@ -116,20 +116,15 @@ __start:
sfence.vma x0, x0
- /* enable FPU and accelerator if present, setting ignored on E51
+ /* Enable accelerator if present, setting ignored on E51
* 15,16 = MSTATUS_XS, 17,18 = MSTATUS_MPRV
* not defined on riscv-v/include/csr.h
*/
- /* li t0, MSTATUS_FS_DIRTY | (1 << 15) | (1 << 16) | (1 << 17) | (1 << 18) */
- li t0, 0x00006000 | 0x00018000 /* MSTATUS_FS | MSTATUS_XS */
+ /* li t0, (1 << 15) | (1 << 16) | (1 << 17) | (1 << 18) */
+ li t0, 0x00018000 /* MSTATUS_XS */
Review comment:
remove extra space before 0x00018000
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] gustavonihei commented on a change in pull request #5878: arch/risc-v: Use riscv_fpuconfig to enable FPU
Posted by GitBox <gi...@apache.org>.
gustavonihei commented on a change in pull request #5878:
URL: https://github.com/apache/incubator-nuttx/pull/5878#discussion_r836774192
##########
File path: arch/risc-v/src/c906/c906_start.c
##########
@@ -73,6 +73,10 @@ void __c906_start(uint32_t mhartid)
const uint32_t *src;
uint32_t *dest;
+ /* Configure FPU */
+
+ riscv_fpuconfig();
Review comment:
It seems FPU is already enabled here:
https://github.com/apache/incubator-nuttx/blob/master/arch/risc-v/src/c906/c906_head.S#L55-L60
**MPFS** seems to also enable the FPU prior to jumping to C code:
https://github.com/apache/incubator-nuttx/blob/master/arch/risc-v/src/mpfs/mpfs_head.S#L119-L131
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pussuw commented on a change in pull request #5878: arch/risc-v: Use riscv_fpuconfig to enable FPU
Posted by GitBox <gi...@apache.org>.
pussuw commented on a change in pull request #5878:
URL: https://github.com/apache/incubator-nuttx/pull/5878#discussion_r837097550
##########
File path: arch/risc-v/src/c906/c906_start.c
##########
@@ -73,6 +73,10 @@ void __c906_start(uint32_t mhartid)
const uint32_t *src;
uint32_t *dest;
+ /* Configure FPU */
+
+ riscv_fpuconfig();
Review comment:
I also do not have a very simple way to test this change but at least the differences I noticed is that 1. hart0 in mpfs does not have an fpu and we handle that in mpfs_head.S. The other change is that we mark the FPU as dirty, but init should be fine too.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #5878: arch/risc-v: Use riscv_fpuconfig to enable FPU
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #5878:
URL: https://github.com/apache/incubator-nuttx/pull/5878#discussion_r837002872
##########
File path: arch/risc-v/src/c906/c906_start.c
##########
@@ -73,6 +73,10 @@ void __c906_start(uint32_t mhartid)
const uint32_t *src;
uint32_t *dest;
+ /* Configure FPU */
+
+ riscv_fpuconfig();
Review comment:
But should we remove FPU related config from MPFS/head.S too to unify the FPU initialization process?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] no1wudi commented on a change in pull request #5878: arch/risc-v: Use riscv_fpuconfig to enable FPU
Posted by GitBox <gi...@apache.org>.
no1wudi commented on a change in pull request #5878:
URL: https://github.com/apache/incubator-nuttx/pull/5878#discussion_r836998642
##########
File path: arch/risc-v/src/c906/c906_start.c
##########
@@ -73,6 +73,10 @@ void __c906_start(uint32_t mhartid)
const uint32_t *src;
uint32_t *dest;
+ /* Configure FPU */
+
+ riscv_fpuconfig();
Review comment:
For C906, I'll update this patch to remove relative code in head.S
For MPFS, it not only enabled the FPU but also set XS bit, so it's better to keep origin implementation.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pussuw commented on a change in pull request #5878: arch/risc-v: Use riscv_fpuconfig to enable FPU
Posted by GitBox <gi...@apache.org>.
pussuw commented on a change in pull request #5878:
URL: https://github.com/apache/incubator-nuttx/pull/5878#discussion_r837098925
##########
File path: arch/risc-v/src/mpfs/mpfs_start.c
##########
@@ -116,6 +116,10 @@ void __mpfs_start(uint64_t mhartid)
const uint32_t *src;
uint32_t *dest;
+ /* Configure FPU */
+
+ riscv_fpuconfig();
Review comment:
Just to prevent some future change from breaking this, can you please add check for
```
if (mhartid != 0)
{
/* Configure FPU */
riscv_fpuconfig();
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 merged pull request #5878: arch/risc-v: Use riscv_fpuconfig to enable FPU
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 merged pull request #5878:
URL: https://github.com/apache/incubator-nuttx/pull/5878
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] jlaitine commented on a change in pull request #5878: arch/risc-v: Use riscv_fpuconfig to enable FPU
Posted by GitBox <gi...@apache.org>.
jlaitine commented on a change in pull request #5878:
URL: https://github.com/apache/incubator-nuttx/pull/5878#discussion_r837092551
##########
File path: arch/risc-v/src/c906/c906_start.c
##########
@@ -73,6 +73,10 @@ void __c906_start(uint32_t mhartid)
const uint32_t *src;
uint32_t *dest;
+ /* Configure FPU */
+
+ riscv_fpuconfig();
Review comment:
This does change the functionality of our "bootloader" app, which is compiled without FPU, but still enables it for harts 1,2,3,4. The new functionality in this PR if probably more correct in that respect (not enable the FPU if built without the FPU support), but I am not able to test it this week if it breaks something since I am travelling without HW. If @pussuw is ok with it, it is fine with me as well.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pussuw commented on a change in pull request #5878: arch/risc-v: Use riscv_fpuconfig to enable FPU
Posted by GitBox <gi...@apache.org>.
pussuw commented on a change in pull request #5878:
URL: https://github.com/apache/incubator-nuttx/pull/5878#discussion_r837107564
##########
File path: arch/risc-v/src/mpfs/mpfs_start.c
##########
@@ -116,6 +116,10 @@ void __mpfs_start(uint64_t mhartid)
const uint32_t *src;
uint32_t *dest;
+ /* Configure FPU */
+
+ riscv_fpuconfig();
Review comment:
LGTM!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] no1wudi commented on a change in pull request #5878: arch/risc-v: Use riscv_fpuconfig to enable FPU
Posted by GitBox <gi...@apache.org>.
no1wudi commented on a change in pull request #5878:
URL: https://github.com/apache/incubator-nuttx/pull/5878#discussion_r837008183
##########
File path: arch/risc-v/src/c906/c906_start.c
##########
@@ -73,6 +73,10 @@ void __c906_start(uint32_t mhartid)
const uint32_t *src;
uint32_t *dest;
+ /* Configure FPU */
+
+ riscv_fpuconfig();
Review comment:
OK, FPU relative flags setting removed from mpfs_head.S.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org