You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by "terry0012 (via GitHub)" <gi...@apache.org> on 2023/09/07 12:13:34 UTC
[GitHub] [nuttx] terry0012 opened a new pull request, #10526: Vsync queue
terry0012 opened a new pull request, #10526:
URL: https://github.com/apache/nuttx/pull/10526
## Summary
Refactor vsync queue and adapt to sim and goldfish modules.
In the new mechanism, the vendor driver does not need to create its own queue management mechanism. it is uniformly handled by the upper layer .
It also supports multiple file opening and polling for fb poll, such as polling for primary fb and overlay at the same time.
## Impact
## Testing
--
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] [nuttx] xiaoxiang781216 commented on a diff in pull request #10526: fb: add vsync queue mechanism
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #10526:
URL: https://github.com/apache/nuttx/pull/10526#discussion_r1321891090
##########
arch/sim/Kconfig:
##########
@@ -355,6 +355,13 @@ config SIM_FBBPP
If you use the X11 display emulation, the selected BPP must match the BPP
of your graphics hardware (probably 32 bits). Default: 8
+config SIM_FBCOUNT
Review Comment:
change to SIM_FRAMEBUFFER_COUNT and depends on SIM_FRAMEBUFFER
--
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] [nuttx] pkarashchenko commented on a diff in pull request #10526: fb: add vsync queue mechanism
Posted by "pkarashchenko (via GitHub)" <gi...@apache.org>.
pkarashchenko commented on code in PR #10526:
URL: https://github.com/apache/nuttx/pull/10526#discussion_r1327639494
##########
arch/sim/src/sim/sim_framebuffer.c:
##########
@@ -455,9 +462,12 @@ int up_fbinitialize(int display)
int ret = OK;
#ifdef CONFIG_SIM_X11FB
+ g_planeinfo.xres_virtual = CONFIG_SIM_FBWIDTH;
+ g_planeinfo.yres_virtual = CONFIG_SIM_FBHEIGHT * CONFIG_SIM_FRAMEBUFFER_COUNT;
Review Comment:
Good to fix this
##########
drivers/video/fb.c:
##########
@@ -41,17 +41,30 @@
#include <nuttx/video/fb.h>
#include <nuttx/clock.h>
#include <nuttx/wdog.h>
+#include <nuttx/mm/circbuf.h>
/****************************************************************************
* Pre-processor definitions
****************************************************************************/
-#define FB_NO_OVERLAY -1
-
/****************************************************************************
* Private Types
****************************************************************************/
+struct fb_priv_s
+{
+ int overlay; /* Overlay number */
+};
+
+struct fb_paninfo_s
+{
+ FAR struct circbuf_s buf; /* Pan buffer queued list */
Review Comment:
`FAR` is not needed 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
[GitHub] [nuttx] terry0012 commented on pull request #10526: fb: add vsync queue mechanism
Posted by "terry0012 (via GitHub)" <gi...@apache.org>.
terry0012 commented on PR #10526:
URL: https://github.com/apache/nuttx/pull/10526#issuecomment-1712956737
The following errors are caused by the naming conventions of X11 and can probably be ignored.
`Error: /home/runner/work/nuttx/nuttx/nuttx/arch/sim/src/sim/posix/sim_x11framebuffer.c:109:2: error: Mixed case identifier found
Error: /home/runner/work/nuttx/nuttx/nuttx/arch/sim/src/sim/posix/sim_x11framebuffer.c:110:2: error: Mixed case identifier found
Error: Process completed with exit code 1.`
--
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] [nuttx] xiaoxiang781216 commented on a diff in pull request #10526: fb: add vsync queue mechanism
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #10526:
URL: https://github.com/apache/nuttx/pull/10526#discussion_r1318949247
##########
drivers/video/fb.c:
##########
@@ -217,11 +345,14 @@ static ssize_t fb_read(FAR struct file *filep, FAR char *buffer, size_t len)
/* Get the framebuffer instance */
inode = filep->f_inode;
- fb = inode->i_private;
+ fb = (FAR struct fb_chardev_s *)inode->i_private;
+ priv = (FAR struct fb_priv_s *)filep->f_priv;
Review Comment:
ditto
##########
drivers/video/fb.c:
##########
@@ -271,11 +403,14 @@ static ssize_t fb_write(FAR struct file *filep, FAR const char *buffer,
/* Get the framebuffer instance */
inode = filep->f_inode;
- fb = inode->i_private;
+ fb = (FAR struct fb_chardev_s *)inode->i_private;
+ priv = (FAR struct fb_priv_s *)filep->f_priv;
Review Comment:
ditto
##########
arch/sim/src/sim/sim_framebuffer.c:
##########
@@ -215,6 +219,19 @@ static int sim_closewindow(struct fb_vtable_s *vtable)
return ret;
}
+#ifdef CONFIG_FB_SYNC
+static int sim_waitforvsync(struct fb_vtable_s *vtable)
Review Comment:
let's implement in the fb driver?
##########
drivers/video/fb.c:
##########
@@ -892,69 +1044,121 @@ static int fb_poll(FAR struct file *filep, struct pollfd *fds, bool setup)
{
FAR struct inode *inode;
FAR struct fb_chardev_s *fb;
+ FAR struct fb_priv_s *priv;
+ FAR struct circbuf_s *panbuf;
+ FAR struct pollfd **pollfds;
+ irqstate_t flags;
+ int ret = OK;
/* Get the framebuffer instance */
inode = filep->f_inode;
- fb = inode->i_private;
+ fb = (FAR struct fb_chardev_s *)inode->i_private;
+ priv = (FAR struct fb_priv_s *)filep->f_priv;
Review Comment:
ditto
##########
drivers/video/fb.c:
##########
@@ -507,14 +646,15 @@ static int fb_ioctl(FAR struct file *filep, int cmd, unsigned long arg)
case FBIO_SELECT_OVERLAY: /* Select video overlay */
{
struct fb_overlayinfo_s oinfo;
+ FAR struct fb_priv_s *priv = (FAR struct fb_priv_s *)filep->f_priv;
Review Comment:
remove the cast
##########
drivers/video/fb.c:
##########
@@ -851,17 +999,21 @@ static int fb_mmap(FAR struct file *filep, FAR struct mm_map_entry_s *map)
{
FAR struct inode *inode;
FAR struct fb_chardev_s *fb;
+ FAR struct fb_priv_s *priv;
struct fb_panelinfo_s panelinfo;
int ret;
/* Get the framebuffer instance */
inode = filep->f_inode;
- fb = inode->i_private;
+ fb = (FAR struct fb_chardev_s *)inode->i_private;
+ priv = (FAR struct fb_priv_s *)filep->f_priv;
Review Comment:
ditto
##########
drivers/video/fb.c:
##########
@@ -167,12 +291,14 @@ static int fb_close(FAR struct file *filep)
{
FAR struct inode *inode;
FAR struct fb_chardev_s *fb;
+ FAR struct fb_priv_s *priv;
int ret;
inode = filep->f_inode;
- fb = inode->i_private;
+ fb = (FAR struct fb_chardev_s *)inode->i_private;
+ priv = (FAR struct fb_priv_s *)filep->f_priv;
Review Comment:
let's remove both cast
##########
drivers/video/fb.c:
##########
@@ -330,7 +466,10 @@ static off_t fb_seek(FAR struct file *filep, off_t offset, int whence)
/* Get the framebuffer instance */
inode = filep->f_inode;
- fb = inode->i_private;
+ fb = (FAR struct fb_chardev_s *)inode->i_private;
+ priv = (FAR struct fb_priv_s *)filep->f_priv;
Review Comment:
ditto
--
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] [nuttx] xiaoxiang781216 commented on a diff in pull request #10526: fb: add vsync queue mechanism
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #10526:
URL: https://github.com/apache/nuttx/pull/10526#discussion_r1327957492
##########
drivers/video/fb.c:
##########
@@ -41,17 +41,30 @@
#include <nuttx/video/fb.h>
#include <nuttx/clock.h>
#include <nuttx/wdog.h>
+#include <nuttx/mm/circbuf.h>
/****************************************************************************
* Pre-processor definitions
****************************************************************************/
-#define FB_NO_OVERLAY -1
-
/****************************************************************************
* Private Types
****************************************************************************/
+struct fb_priv_s
+{
+ int overlay; /* Overlay number */
+};
+
+struct fb_paninfo_s
+{
+ FAR struct circbuf_s buf; /* Pan buffer queued list */
Review Comment:
Fixed here: https://github.com/apache/nuttx/pull/10648
--
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] [nuttx] pkarashchenko commented on pull request #10526: fb: add vsync queue mechanism
Posted by "pkarashchenko (via GitHub)" <gi...@apache.org>.
pkarashchenko commented on PR #10526:
URL: https://github.com/apache/nuttx/pull/10526#issuecomment-1711605386
Please fix CI build
--
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] [nuttx] terry0012 commented on a diff in pull request #10526: fb: add vsync queue mechanism
Posted by "terry0012 (via GitHub)" <gi...@apache.org>.
terry0012 commented on code in PR #10526:
URL: https://github.com/apache/nuttx/pull/10526#discussion_r1320418509
##########
arch/sim/src/sim/sim_framebuffer.c:
##########
@@ -215,6 +219,19 @@ static int sim_closewindow(struct fb_vtable_s *vtable)
return ret;
}
+#ifdef CONFIG_FB_SYNC
+static int sim_waitforvsync(struct fb_vtable_s *vtable)
Review Comment:
OK, we remove it first and add it to fb driver in the next commit.
--
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] [nuttx] terry0012 commented on a diff in pull request #10526: fb: add vsync queue mechanism
Posted by "terry0012 (via GitHub)" <gi...@apache.org>.
terry0012 commented on code in PR #10526:
URL: https://github.com/apache/nuttx/pull/10526#discussion_r1322114590
##########
arch/sim/Kconfig:
##########
@@ -355,6 +355,13 @@ config SIM_FBBPP
If you use the X11 display emulation, the selected BPP must match the BPP
of your graphics hardware (probably 32 bits). Default: 8
+config SIM_FBCOUNT
Review Comment:
Done.
--
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] [nuttx] terry0012 commented on pull request #10526: fb: add vsync queue mechanism
Posted by "terry0012 (via GitHub)" <gi...@apache.org>.
terry0012 commented on PR #10526:
URL: https://github.com/apache/nuttx/pull/10526#issuecomment-1712328771
> @terry0012 please fix the following error:
>
> ```
> /github/workspace/sources/nuttx/drivers/video/fb.c: In function 'fb_write':
> Error: /github/workspace/sources/nuttx/drivers/video/fb.c:420:5: error: 'struct fb_chardev_s' has no member named 'pollready'
> 420 | fb->pollready = false;
> | ^~
> /github/workspace/sources/nuttx/drivers/video/fb.c: In function 'fb_get_planeinfo':
> Warning: /github/workspace/sources/nuttx/drivers/video/fb.c:1166:57: warning: passing argument 3 of 'fb->vtable->getplaneinfo' from incompatible pointer type [-Wincompatible-pointer-types]
> 1166 | ret = fb->vtable->getplaneinfo(fb->vtable, fb->plane, &pinfo);
> | ^~~~~~
> | |
> | struct fb_planeinfo_s **
> /github/workspace/sources/nuttx/drivers/video/fb.c:1166:57: note: expected 'struct fb_planeinfo_s *' but argument is of type 'struct fb_planeinfo_s **'
> Error: /github/workspace/sources/nuttx/drivers/video/fb.c:1174:3: error: 'panelinfo' undeclared (first use in this function); did you mean 'mallinfo'?
> 1174 | panelinfo->fbmem = pinfo.fbmem;
> | ^~~~~~~~~
> | mallinfo
> /github/workspace/sources/nuttx/drivers/video/fb.c:1174:3: note: each undeclared identifier is reported only once for each function it appears in
> Error: /github/workspace/sources/nuttx/drivers/video/fb.c:1174:28: error: 'pinfo' is a pointer; did you mean to use '->'?
> 1174 | panelinfo->fbmem = pinfo.fbmem;
> | ^
> | ->
> Error: /github/workspace/sources/nuttx/drivers/video/fb.c:1175:28: error: 'pinfo' is a pointer; did you mean to use '->'?
> 1175 | panelinfo->fblen = pinfo.fblen;
> | ^
> | ->
> Error: /github/workspace/sources/nuttx/drivers/video/fb.c:1176:28: error: 'pinfo' is a pointer; did you mean to use '->'?
> 1176 | panelinfo->bpp = pinfo.bpp;
> | ^
> |
> ```
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] [nuttx] xiaoxiang781216 commented on pull request #10526: fb: add vsync queue mechanism
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on PR #10526:
URL: https://github.com/apache/nuttx/pull/10526#issuecomment-1710579476
@terry0012 please fix the following error:
```
/github/workspace/sources/nuttx/drivers/video/fb.c: In function 'fb_write':
Error: /github/workspace/sources/nuttx/drivers/video/fb.c:420:5: error: 'struct fb_chardev_s' has no member named 'pollready'
420 | fb->pollready = false;
| ^~
/github/workspace/sources/nuttx/drivers/video/fb.c: In function 'fb_get_planeinfo':
Warning: /github/workspace/sources/nuttx/drivers/video/fb.c:1166:57: warning: passing argument 3 of 'fb->vtable->getplaneinfo' from incompatible pointer type [-Wincompatible-pointer-types]
1166 | ret = fb->vtable->getplaneinfo(fb->vtable, fb->plane, &pinfo);
| ^~~~~~
| |
| struct fb_planeinfo_s **
/github/workspace/sources/nuttx/drivers/video/fb.c:1166:57: note: expected 'struct fb_planeinfo_s *' but argument is of type 'struct fb_planeinfo_s **'
Error: /github/workspace/sources/nuttx/drivers/video/fb.c:1174:3: error: 'panelinfo' undeclared (first use in this function); did you mean 'mallinfo'?
1174 | panelinfo->fbmem = pinfo.fbmem;
| ^~~~~~~~~
| mallinfo
/github/workspace/sources/nuttx/drivers/video/fb.c:1174:3: note: each undeclared identifier is reported only once for each function it appears in
Error: /github/workspace/sources/nuttx/drivers/video/fb.c:1174:28: error: 'pinfo' is a pointer; did you mean to use '->'?
1174 | panelinfo->fbmem = pinfo.fbmem;
| ^
| ->
Error: /github/workspace/sources/nuttx/drivers/video/fb.c:1175:28: error: 'pinfo' is a pointer; did you mean to use '->'?
1175 | panelinfo->fblen = pinfo.fblen;
| ^
| ->
Error: /github/workspace/sources/nuttx/drivers/video/fb.c:1176:28: error: 'pinfo' is a pointer; did you mean to use '->'?
1176 | panelinfo->bpp = pinfo.bpp;
| ^
|
```
--
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] [nuttx] xiaoxiang781216 commented on pull request #10526: fb: add vsync queue mechanism
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on PR #10526:
URL: https://github.com/apache/nuttx/pull/10526#issuecomment-1713050438
> The following errors are caused by the naming conventions of X11 and can probably be ignored.
>
> `Error: /home/runner/work/nuttx/nuttx/nuttx/arch/sim/src/sim/posix/sim_x11framebuffer.c:109:2: error: Mixed case identifier found Error: /home/runner/work/nuttx/nuttx/nuttx/arch/sim/src/sim/posix/sim_x11framebuffer.c:110:2: error: Mixed case identifier found Error: Process completed with exit code 1.`
let's ignore "Mixed case identifier" warning, but fix the compiler error:
```
Error: sim/sim_lcd.c:468:9: error: too few arguments to function 'sim_x11initialize'
468 | ret = sim_x11initialize(CONFIG_SIM_FBWIDTH, CONFIG_SIM_FBHEIGHT,
| ^~~~~~~~~~~~~~~~~
```
--
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] [nuttx] xiaoxiang781216 merged pull request #10526: fb: add vsync queue mechanism
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 merged PR #10526:
URL: https://github.com/apache/nuttx/pull/10526
--
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