You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by "yf13 (via GitHub)" <gi...@apache.org> on 2024/04/27 00:19:55 UTC
[PR] video/fb: user-space framebuffer support [nuttx]
yf13 opened a new pull request, #12238:
URL: https://github.com/apache/nuttx/pull/12238
## Summary
This enables user-space framebuffer access in kernel mode. It supports the demo examples/fb on rv-virt device like below:
```
ABC
NuttShell (NSH) NuttX-12.4.0-RC0
nsh> cat /proc/version
NuttX version 12.4.0 63ad343ed7 Apr 27 2024 08:05:46 rv-virt/kfb64
nsh> fb
VideoInfo:
fmt: 13
xres: 640
yres: 480
nplanes: 1
PlaneInfo (plane 0):
fbmem: 0x80219000
fblen: 1228800
stride: 2560
display: 0
bpp: 32
Mapped FB: 0xc2000000
0: ( 0, 0) (640,480)
1: ( 58, 43) (524,394)
2: (116, 86) (408,308)
3: (174,129) (292,222)
4: (232,172) (176,136)
5: (290,215) ( 60, 50)
Test finished
nsh>
```
## Impact
video/fb and virtio-gpu driver
## Testing
with `rv-virt/kfb64` and CI checks
--
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
Re: [PR] video/virtio-gpu: userspace framebuffer support [nuttx]
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #12238:
URL: https://github.com/apache/nuttx/pull/12238#discussion_r1581981487
##########
mm/map/Make.defs:
##########
@@ -23,7 +23,7 @@
CSRCS += mm_map.c
ifeq ($(CONFIG_ARCH_VMA_MAPPING),y)
-CSRCS += vm_region.c
+CSRCS += vm_region.c vm_map.c
Review Comment:
why not put vm_map_region into vm_region.c?
##########
drivers/video/fb.c:
##########
@@ -1032,7 +1032,13 @@ static int fb_mmap(FAR struct file *filep, FAR struct mm_map_entry_s *map)
if (map->offset >= 0 && map->offset < panelinfo.fblen &&
map->length && map->offset + map->length <= panelinfo.fblen)
{
+#ifdef CONFIG_BUILD_KERNEL
+ map->vaddr = vm_map_region((uintptr_t)panelinfo.fbmem,
+ panelinfo.fblen);
+ map->vaddr += map->offset;
Review Comment:
can vm_map_region support unaligned adress
##########
drivers/virtio/virtio-gpu.c:
##########
@@ -528,7 +535,8 @@ static int virtio_gpu_probe(FAR struct virtio_device *vdev)
priv->stride = priv->xres * VIRTIO_GPU_BPP >> 3;
priv->fblen = priv->stride * priv->yres;
- priv->fbmem = (FAR uint8_t *)virtio_zalloc_buf(vdev, priv->fblen, 16);
+ priv->fbmem = (FAR uint8_t *)virtio_zalloc_buf(vdev, priv->fblen,
Review Comment:
don't need change if vm_map_region support the unaligned address.
--
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
Re: [PR] video/virtio-gpu: userspace framebuffer support [nuttx]
Posted by "yf13 (via GitHub)" <gi...@apache.org>.
yf13 commented on code in PR #12238:
URL: https://github.com/apache/nuttx/pull/12238#discussion_r1585785015
##########
drivers/video/fb.c:
##########
@@ -1032,7 +1032,13 @@ static int fb_mmap(FAR struct file *filep, FAR struct mm_map_entry_s *map)
if (map->offset >= 0 && map->offset < panelinfo.fblen &&
map->length && map->offset + map->length <= panelinfo.fblen)
{
+#ifdef CONFIG_BUILD_KERNEL
+ map->vaddr = vm_map_region((uintptr_t)panelinfo.fbmem,
+ panelinfo.fblen);
+ map->vaddr += map->offset;
Review Comment:
Can you confirm which one below is what you mean:
- a) new interface is `vm_map_region(paddr, size, offset)`? here offset can be bigger than MM_PGSIZE.
- b) new interface is `vm_map_region(paddr+offset, size)`? here offset is smaller than MM_PGSIZE.
- c) none-of above, please teach explicitly.
The offset in `man mmap` of Linux is multiple of MM_PGSIZE or zero, is this the same in NuttX?
--
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
Re: [PR] video/fb: userspace framebuffer support [nuttx]
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 merged PR #12238:
URL: https://github.com/apache/nuttx/pull/12238
--
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
Re: [PR] video/virtio-gpu: userspace framebuffer support [nuttx]
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #12238:
URL: https://github.com/apache/nuttx/pull/12238#discussion_r1582799273
##########
drivers/video/fb.c:
##########
@@ -1032,7 +1032,13 @@ static int fb_mmap(FAR struct file *filep, FAR struct mm_map_entry_s *map)
if (map->offset >= 0 && map->offset < panelinfo.fblen &&
map->length && map->offset + map->length <= panelinfo.fblen)
{
+#ifdef CONFIG_BUILD_KERNEL
+ map->vaddr = vm_map_region((uintptr_t)panelinfo.fbmem,
+ panelinfo.fblen);
+ map->vaddr += map->offset;
Review Comment:
Yes, just mask out offset before creating virtual-physical page setup, and add offset to the returned virtual address before returrn.
--
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
Re: [PR] video/virtio-gpu: userspace framebuffer support [nuttx]
Posted by "yf13 (via GitHub)" <gi...@apache.org>.
yf13 commented on code in PR #12238:
URL: https://github.com/apache/nuttx/pull/12238#discussion_r1585785015
##########
drivers/video/fb.c:
##########
@@ -1032,7 +1032,13 @@ static int fb_mmap(FAR struct file *filep, FAR struct mm_map_entry_s *map)
if (map->offset >= 0 && map->offset < panelinfo.fblen &&
map->length && map->offset + map->length <= panelinfo.fblen)
{
+#ifdef CONFIG_BUILD_KERNEL
+ map->vaddr = vm_map_region((uintptr_t)panelinfo.fbmem,
+ panelinfo.fblen);
+ map->vaddr += map->offset;
Review Comment:
Can you confirm which one below is what you mean:
- a) new interface is `vm_map_region(paddr, size, offset)`? here offset can be bigger than MM_PGSIZE.
- b) new interface is `vm_map_region(paddr+offset, size)`? here offset is smaller than MM_PGSIZE.
- c) none-of above, please teach explicitly.
The offset in `man mmap` of Linux is multiple of MM_PGSIZE or zero, is this the same in NuttX?
--
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
Re: [PR] video/virtio-gpu: userspace framebuffer support [nuttx]
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #12238:
URL: https://github.com/apache/nuttx/pull/12238#discussion_r1582595072
##########
drivers/virtio/virtio-gpu.c:
##########
@@ -528,7 +535,8 @@ static int virtio_gpu_probe(FAR struct virtio_device *vdev)
priv->stride = priv->xres * VIRTIO_GPU_BPP >> 3;
priv->fblen = priv->stride * priv->yres;
- priv->fbmem = (FAR uint8_t *)virtio_zalloc_buf(vdev, priv->fblen, 16);
+ priv->fbmem = (FAR uint8_t *)virtio_zalloc_buf(vdev, priv->fblen,
Review Comment:
and here 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
Re: [PR] video/virtio-gpu: userspace framebuffer support [nuttx]
Posted by "yf13 (via GitHub)" <gi...@apache.org>.
yf13 commented on code in PR #12238:
URL: https://github.com/apache/nuttx/pull/12238#discussion_r1582773740
##########
drivers/video/fb.c:
##########
@@ -1032,7 +1032,13 @@ static int fb_mmap(FAR struct file *filep, FAR struct mm_map_entry_s *map)
if (map->offset >= 0 && map->offset < panelinfo.fblen &&
map->length && map->offset + map->length <= panelinfo.fblen)
{
+#ifdef CONFIG_BUILD_KERNEL
+ map->vaddr = vm_map_region((uintptr_t)panelinfo.fbmem,
+ panelinfo.fblen);
+ map->vaddr += map->offset;
Review Comment:
Did you mean that we should revise `vm_map_regoin()` to support unaligned physical and virtual addresses?
Sorry I don't see how to do that yet.
--
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
Re: [PR] video/virtio-gpu: userspace framebuffer support [nuttx]
Posted by "yf13 (via GitHub)" <gi...@apache.org>.
yf13 commented on code in PR #12238:
URL: https://github.com/apache/nuttx/pull/12238#discussion_r1581993161
##########
drivers/video/fb.c:
##########
@@ -1032,7 +1032,13 @@ static int fb_mmap(FAR struct file *filep, FAR struct mm_map_entry_s *map)
if (map->offset >= 0 && map->offset < panelinfo.fblen &&
map->length && map->offset + map->length <= panelinfo.fblen)
{
+#ifdef CONFIG_BUILD_KERNEL
+ map->vaddr = vm_map_region((uintptr_t)panelinfo.fbmem,
+ panelinfo.fblen);
+ map->vaddr += map->offset;
Review Comment:
I don't know how to support unaligned address yet. It seems that RISC-V MMU requires aligned addresses, also granule allocator operates on aligned address 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
Re: [PR] video/virtio-gpu: userspace framebuffer support [nuttx]
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #12238:
URL: https://github.com/apache/nuttx/pull/12238#discussion_r1587244026
##########
drivers/video/fb.c:
##########
@@ -1032,7 +1032,13 @@ static int fb_mmap(FAR struct file *filep, FAR struct mm_map_entry_s *map)
if (map->offset >= 0 && map->offset < panelinfo.fblen &&
map->length && map->offset + map->length <= panelinfo.fblen)
{
+#ifdef CONFIG_BUILD_KERNEL
+ map->vaddr = vm_map_region((uintptr_t)panelinfo.fbmem,
+ panelinfo.fblen);
+ map->vaddr += map->offset;
Review Comment:
no change here?
--
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
Re: [PR] video/virtio-gpu: userspace framebuffer support [nuttx]
Posted by "yf13 (via GitHub)" <gi...@apache.org>.
yf13 commented on code in PR #12238:
URL: https://github.com/apache/nuttx/pull/12238#discussion_r1585785015
##########
drivers/video/fb.c:
##########
@@ -1032,7 +1032,13 @@ static int fb_mmap(FAR struct file *filep, FAR struct mm_map_entry_s *map)
if (map->offset >= 0 && map->offset < panelinfo.fblen &&
map->length && map->offset + map->length <= panelinfo.fblen)
{
+#ifdef CONFIG_BUILD_KERNEL
+ map->vaddr = vm_map_region((uintptr_t)panelinfo.fbmem,
+ panelinfo.fblen);
+ map->vaddr += map->offset;
Review Comment:
Can you confirm which one below is what you mean:
a) new interface is ` vm_map_region(paddr, size, offset)`? here offset can be bigger than MM_PGSIZE.
b) new interface is `vm_map_region(paddr+offset, size)`? here offset is smaller than MM_PGSIZE.
c) none-of above, please teach explicitly.
The offset in `man mmap` is multiple of MM_PGSIZE or zero, is this the same in NuttX?
--
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
Re: [PR] video/virtio-gpu: userspace framebuffer support [nuttx]
Posted by "yf13 (via GitHub)" <gi...@apache.org>.
yf13 commented on code in PR #12238:
URL: https://github.com/apache/nuttx/pull/12238#discussion_r1585788363
##########
drivers/virtio/virtio-gpu.c:
##########
@@ -528,7 +535,8 @@ static int virtio_gpu_probe(FAR struct virtio_device *vdev)
priv->stride = priv->xres * VIRTIO_GPU_BPP >> 3;
priv->fblen = priv->stride * priv->yres;
- priv->fbmem = (FAR uint8_t *)virtio_zalloc_buf(vdev, priv->fblen, 16);
+ priv->fbmem = (FAR uint8_t *)virtio_zalloc_buf(vdev, priv->fblen,
Review Comment:
If fbmem is not paged aligned, then the kernel memory region before fbmem will be exposed to user space, which looks like a security hole.
--
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
Re: [PR] video/virtio-gpu: userspace framebuffer support [nuttx]
Posted by "yf13 (via GitHub)" <gi...@apache.org>.
yf13 commented on code in PR #12238:
URL: https://github.com/apache/nuttx/pull/12238#discussion_r1581993161
##########
drivers/video/fb.c:
##########
@@ -1032,7 +1032,13 @@ static int fb_mmap(FAR struct file *filep, FAR struct mm_map_entry_s *map)
if (map->offset >= 0 && map->offset < panelinfo.fblen &&
map->length && map->offset + map->length <= panelinfo.fblen)
{
+#ifdef CONFIG_BUILD_KERNEL
+ map->vaddr = vm_map_region((uintptr_t)panelinfo.fbmem,
+ panelinfo.fblen);
+ map->vaddr += map->offset;
Review Comment:
I don't know how to support unaligned address yet. It seems that at RISC-V MMU requires aligned addresses, also granule allocator operates on aligned address 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
Re: [PR] video/virtio-gpu: userspace framebuffer support [nuttx]
Posted by "yf13 (via GitHub)" <gi...@apache.org>.
yf13 commented on code in PR #12238:
URL: https://github.com/apache/nuttx/pull/12238#discussion_r1585788363
##########
drivers/virtio/virtio-gpu.c:
##########
@@ -528,7 +535,8 @@ static int virtio_gpu_probe(FAR struct virtio_device *vdev)
priv->stride = priv->xres * VIRTIO_GPU_BPP >> 3;
priv->fblen = priv->stride * priv->yres;
- priv->fbmem = (FAR uint8_t *)virtio_zalloc_buf(vdev, priv->fblen, 16);
+ priv->fbmem = (FAR uint8_t *)virtio_zalloc_buf(vdev, priv->fblen,
Review Comment:
If fbmem is not paged aligned, then the kernel memory region before fbmem will be exposed to user space, which looks like a security hole.
--
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
Re: [PR] video/fb: userspace framebuffer support [nuttx]
Posted by "yf13 (via GitHub)" <gi...@apache.org>.
yf13 commented on code in PR #12238:
URL: https://github.com/apache/nuttx/pull/12238#discussion_r1588744223
##########
drivers/video/fb.c:
##########
@@ -1032,7 +1032,13 @@ static int fb_mmap(FAR struct file *filep, FAR struct mm_map_entry_s *map)
if (map->offset >= 0 && map->offset < panelinfo.fblen &&
map->length && map->offset + map->length <= panelinfo.fblen)
{
+#ifdef CONFIG_BUILD_KERNEL
+ map->vaddr = vm_map_region((uintptr_t)panelinfo.fbmem,
+ panelinfo.fblen);
+ map->vaddr += map->offset;
Review Comment:
rolled back to confirm CI status... just pushed an update supporting unaligned paddr mapping and moved `vm_map_xxx()` into ` vm_region.c`.
--
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
Re: [PR] video/virtio-gpu: userspace framebuffer support [nuttx]
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #12238:
URL: https://github.com/apache/nuttx/pull/12238#discussion_r1581996645
##########
mm/map/Make.defs:
##########
@@ -23,7 +23,7 @@
CSRCS += mm_map.c
ifeq ($(CONFIG_ARCH_VMA_MAPPING),y)
-CSRCS += vm_region.c
+CSRCS += vm_region.c vm_map.c
Review Comment:
OK
--
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
Re: [PR] video/virtio-gpu: userspace framebuffer support [nuttx]
Posted by "yf13 (via GitHub)" <gi...@apache.org>.
yf13 commented on code in PR #12238:
URL: https://github.com/apache/nuttx/pull/12238#discussion_r1581993161
##########
drivers/video/fb.c:
##########
@@ -1032,7 +1032,13 @@ static int fb_mmap(FAR struct file *filep, FAR struct mm_map_entry_s *map)
if (map->offset >= 0 && map->offset < panelinfo.fblen &&
map->length && map->offset + map->length <= panelinfo.fblen)
{
+#ifdef CONFIG_BUILD_KERNEL
+ map->vaddr = vm_map_region((uintptr_t)panelinfo.fbmem,
+ panelinfo.fblen);
+ map->vaddr += map->offset;
Review Comment:
It seems that RISC-V MMU requires aligned addresses, also the virtual page allocator operates on aligned address 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
Re: [PR] video/virtio-gpu: userspace framebuffer support [nuttx]
Posted by "yf13 (via GitHub)" <gi...@apache.org>.
yf13 commented on code in PR #12238:
URL: https://github.com/apache/nuttx/pull/12238#discussion_r1581994602
##########
mm/map/Make.defs:
##########
@@ -23,7 +23,7 @@
CSRCS += mm_map.c
ifeq ($(CONFIG_ARCH_VMA_MAPPING),y)
-CSRCS += vm_region.c
+CSRCS += vm_region.c vm_map.c
Review Comment:
I am open for that., Must we do it here or can we do that separately?
--
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
Re: [PR] video/virtio-gpu: userspace framebuffer support [nuttx]
Posted by "yf13 (via GitHub)" <gi...@apache.org>.
yf13 commented on code in PR #12238:
URL: https://github.com/apache/nuttx/pull/12238#discussion_r1581994602
##########
mm/map/Make.defs:
##########
@@ -23,7 +23,7 @@
CSRCS += mm_map.c
ifeq ($(CONFIG_ARCH_VMA_MAPPING),y)
-CSRCS += vm_region.c
+CSRCS += vm_region.c vm_map.c
Review Comment:
To focus this patch on drivers layer, can we do that separately?
--
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
Re: [PR] video/virtio-gpu: userspace framebuffer support [nuttx]
Posted by "yf13 (via GitHub)" <gi...@apache.org>.
yf13 commented on code in PR #12238:
URL: https://github.com/apache/nuttx/pull/12238#discussion_r1585785015
##########
drivers/video/fb.c:
##########
@@ -1032,7 +1032,13 @@ static int fb_mmap(FAR struct file *filep, FAR struct mm_map_entry_s *map)
if (map->offset >= 0 && map->offset < panelinfo.fblen &&
map->length && map->offset + map->length <= panelinfo.fblen)
{
+#ifdef CONFIG_BUILD_KERNEL
+ map->vaddr = vm_map_region((uintptr_t)panelinfo.fbmem,
+ panelinfo.fblen);
+ map->vaddr += map->offset;
Review Comment:
Can you confirm which one below is what you mean:
- a) new interface is ` vm_map_region(paddr, size, offset)`? here offset can be bigger than MM_PGSIZE.
- b) new interface is `vm_map_region(paddr+offset, size)`? here offset is smaller than MM_PGSIZE.
- c) none-of above, please teach explicitly.
The offset in `man mmap` is multiple of MM_PGSIZE or zero, is this the same in NuttX?
--
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
Re: [PR] video/virtio-gpu: userspace framebuffer support [nuttx]
Posted by "yf13 (via GitHub)" <gi...@apache.org>.
yf13 commented on code in PR #12238:
URL: https://github.com/apache/nuttx/pull/12238#discussion_r1581994602
##########
mm/map/Make.defs:
##########
@@ -23,7 +23,7 @@
CSRCS += mm_map.c
ifeq ($(CONFIG_ARCH_VMA_MAPPING),y)
-CSRCS += vm_region.c
+CSRCS += vm_region.c vm_map.c
Review Comment:
I am open for that. Must we do it here or can we do that separately?
--
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
Re: [PR] video/virtio-gpu: userspace framebuffer support [nuttx]
Posted by "yf13 (via GitHub)" <gi...@apache.org>.
yf13 commented on code in PR #12238:
URL: https://github.com/apache/nuttx/pull/12238#discussion_r1582464389
##########
mm/map/Make.defs:
##########
@@ -23,7 +23,7 @@
CSRCS += mm_map.c
ifeq ($(CONFIG_ARCH_VMA_MAPPING),y)
-CSRCS += vm_region.c
+CSRCS += vm_region.c vm_map.c
Review Comment:
thanks, will do it later separately.
--
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
Re: [PR] video/virtio-gpu: userspace framebuffer support [nuttx]
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #12238:
URL: https://github.com/apache/nuttx/pull/12238#discussion_r1582479867
##########
drivers/video/fb.c:
##########
@@ -1032,7 +1032,13 @@ static int fb_mmap(FAR struct file *filep, FAR struct mm_map_entry_s *map)
if (map->offset >= 0 && map->offset < panelinfo.fblen &&
map->length && map->offset + map->length <= panelinfo.fblen)
{
+#ifdef CONFIG_BUILD_KERNEL
+ map->vaddr = vm_map_region((uintptr_t)panelinfo.fbmem,
+ panelinfo.fblen);
+ map->vaddr += map->offset;
Review Comment:
@yf13 I mean passing (panelinfo.fbmem + map->offset) to vm_map_region.
--
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
Re: [PR] video/virtio-gpu: userspace framebuffer support [nuttx]
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #12238:
URL: https://github.com/apache/nuttx/pull/12238#discussion_r1587244026
##########
drivers/video/fb.c:
##########
@@ -1032,7 +1032,13 @@ static int fb_mmap(FAR struct file *filep, FAR struct mm_map_entry_s *map)
if (map->offset >= 0 && map->offset < panelinfo.fblen &&
map->length && map->offset + map->length <= panelinfo.fblen)
{
+#ifdef CONFIG_BUILD_KERNEL
+ map->vaddr = vm_map_region((uintptr_t)panelinfo.fbmem,
+ panelinfo.fblen);
+ map->vaddr += map->offset;
Review Comment:
nochangehere?
--
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
Re: [PR] video/virtio-gpu: userspace framebuffer support [nuttx]
Posted by "yf13 (via GitHub)" <gi...@apache.org>.
yf13 commented on code in PR #12238:
URL: https://github.com/apache/nuttx/pull/12238#discussion_r1585785015
##########
drivers/video/fb.c:
##########
@@ -1032,7 +1032,13 @@ static int fb_mmap(FAR struct file *filep, FAR struct mm_map_entry_s *map)
if (map->offset >= 0 && map->offset < panelinfo.fblen &&
map->length && map->offset + map->length <= panelinfo.fblen)
{
+#ifdef CONFIG_BUILD_KERNEL
+ map->vaddr = vm_map_region((uintptr_t)panelinfo.fbmem,
+ panelinfo.fblen);
+ map->vaddr += map->offset;
Review Comment:
Can you confirm which one below is what you mean:
a) new interface is ` vm_map_region(paddr, size, offset)`? here offset can be bigger than MM_PGSIZE.
b) new interface is `vm_map_region(paddr+offset, size)`? here offset is smaller than MM_PGSIZE.
c) none-of above, please teach explicitly.
The offset in `man mmap()` is multiple of MM_PGSIZE or zero, is this the same in NuttX?
--
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
Re: [PR] video/virtio-gpu: userspace framebuffer support [nuttx]
Posted by "yf13 (via GitHub)" <gi...@apache.org>.
yf13 commented on code in PR #12238:
URL: https://github.com/apache/nuttx/pull/12238#discussion_r1581994602
##########
mm/map/Make.defs:
##########
@@ -23,7 +23,7 @@
CSRCS += mm_map.c
ifeq ($(CONFIG_ARCH_VMA_MAPPING),y)
-CSRCS += vm_region.c
+CSRCS += vm_region.c vm_map.c
Review Comment:
To make this patch focus on drivers layer, can we do that separately?
--
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
Re: [PR] video/virtio-gpu: userspace framebuffer support [nuttx]
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #12238:
URL: https://github.com/apache/nuttx/pull/12238#discussion_r1581996835
##########
drivers/video/fb.c:
##########
@@ -1032,7 +1032,13 @@ static int fb_mmap(FAR struct file *filep, FAR struct mm_map_entry_s *map)
if (map->offset >= 0 && map->offset < panelinfo.fblen &&
map->length && map->offset + map->length <= panelinfo.fblen)
{
+#ifdef CONFIG_BUILD_KERNEL
+ map->vaddr = vm_map_region((uintptr_t)panelinfo.fbmem,
+ panelinfo.fblen);
+ map->vaddr += map->offset;
Review Comment:
it's simple to move the offset addition to vm_map_region
--
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
Re: [PR] video/virtio-gpu: userspace framebuffer support [nuttx]
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #12238:
URL: https://github.com/apache/nuttx/pull/12238#discussion_r1582594903
##########
drivers/video/fb.c:
##########
@@ -1032,7 +1032,13 @@ static int fb_mmap(FAR struct file *filep, FAR struct mm_map_entry_s *map)
if (map->offset >= 0 && map->offset < panelinfo.fblen &&
map->length && map->offset + map->length <= panelinfo.fblen)
{
+#ifdef CONFIG_BUILD_KERNEL
+ map->vaddr = vm_map_region((uintptr_t)panelinfo.fbmem,
+ panelinfo.fblen);
+ map->vaddr += map->offset;
Review Comment:
@yf13 this comment isn't addressed yet.
--
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
Re: [PR] video/virtio-gpu: userspace framebuffer support [nuttx]
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #12238:
URL: https://github.com/apache/nuttx/pull/12238#discussion_r1582594017
##########
drivers/video/fb.c:
##########
@@ -1032,7 +1032,12 @@ static int fb_mmap(FAR struct file *filep, FAR struct mm_map_entry_s *map)
if (map->offset >= 0 && map->offset < panelinfo.fblen &&
map->length && map->offset + map->length <= panelinfo.fblen)
{
+#ifdef CONFIG_BUILD_KERNEL
+ map->vaddr = (FAR char *)vm_map_region((uintptr_t)panelinfo.fbmem,
Review Comment:
don't need cast
--
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
Re: [PR] video/virtio-gpu: userspace framebuffer support [nuttx]
Posted by "yf13 (via GitHub)" <gi...@apache.org>.
yf13 commented on code in PR #12238:
URL: https://github.com/apache/nuttx/pull/12238#discussion_r1582773740
##########
drivers/video/fb.c:
##########
@@ -1032,7 +1032,13 @@ static int fb_mmap(FAR struct file *filep, FAR struct mm_map_entry_s *map)
if (map->offset >= 0 && map->offset < panelinfo.fblen &&
map->length && map->offset + map->length <= panelinfo.fblen)
{
+#ifdef CONFIG_BUILD_KERNEL
+ map->vaddr = vm_map_region((uintptr_t)panelinfo.fbmem,
+ panelinfo.fblen);
+ map->vaddr += map->offset;
Review Comment:
Did you mean that we should revise `vm_map_regoin()` to accept unaligned physical addresses?
Sorry I don't know how to do that yet.
--
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
Re: [PR] video/virtio-gpu: userspace framebuffer support [nuttx]
Posted by "yf13 (via GitHub)" <gi...@apache.org>.
yf13 commented on code in PR #12238:
URL: https://github.com/apache/nuttx/pull/12238#discussion_r1585785015
##########
drivers/video/fb.c:
##########
@@ -1032,7 +1032,13 @@ static int fb_mmap(FAR struct file *filep, FAR struct mm_map_entry_s *map)
if (map->offset >= 0 && map->offset < panelinfo.fblen &&
map->length && map->offset + map->length <= panelinfo.fblen)
{
+#ifdef CONFIG_BUILD_KERNEL
+ map->vaddr = vm_map_region((uintptr_t)panelinfo.fbmem,
+ panelinfo.fblen);
+ map->vaddr += map->offset;
Review Comment:
Can you confirm which one below is what you mean:
- a) new interface is `vm_map_region(paddr, size, offset)`? here offset can be bigger than MM_PGSIZE.
- b) new interface is `vm_map_region(paddr+offset, size)`? here offset is smaller than MM_PGSIZE.
- c) none-of above, please teach explicitly.
The offset in `man mmap` is multiple of MM_PGSIZE or zero, is this the same in NuttX?
--
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