Skip to content

Frame buffer device information -- Accelerator : No #1067

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
pssc opened this issue Jul 16, 2015 · 20 comments
Closed

Frame buffer device information -- Accelerator : No #1067

pssc opened this issue Jul 16, 2015 · 20 comments

Comments

@pssc
Copy link

pssc commented Jul 16, 2015

Frame buffer device information:
Name : BCM2708 FB
Address : 0x1e875000
Size : 1843200
Type : PACKED PIXELS
Visual : TRUECOLOR
XPanStep : 1
YPanStep : 1
YWrapStep : 0
LineLength : 2560
Accelerator : No

Could our should this be turned to something else to aid auto detection of dispmanx.

@popcornmix
Copy link
Collaborator

I don't understand the question.
What is your kernel issue?

@pssc
Copy link
Author

pssc commented Jul 16, 2015

Frame buffer device information from linux/fb.h

fb_fix_screeninfo {
....  
__u32 accel;                    /* Indicate to driver which     */
                                        /*  specific chip/card we have  */
};

The Pi kernels don’t currently indicate that the frame buffer has an accelerator variable

@popcornmix
Copy link
Collaborator

What are you suggesting is returned there?
What software would make use of it, and how would it help?

@pssc
Copy link
Author

pssc commented Jul 16, 2015

#define FB_ACCEL_DISPMANX 0xb0 ?
theirs a whole list in linux/fb.h for other framebuffers

I am currently hacking the SDL other acceleration based on the frame buffer hangs off the fact that the FB indicated it has an accelerator. this seems to be the std way of indicating this is available.

@popcornmix
Copy link
Collaborator

Perhaps submit a PR for the kernel and point at how you intend to use it in SDL and we'll consider it.

@pssc
Copy link
Author

pssc commented Jul 16, 2015

diff --git a/drivers/video/fbdev/bcm2708_fb.c b/drivers/video/fbdev/bcm2708_fb.c
index 62fe57d..93d8f3a 100644
--- a/drivers/video/fbdev/bcm2708_fb.c
+++ b/drivers/video/fbdev/bcm2708_fb.c
@@ -648,7 +648,7 @@ static int bcm2708_fb_register(struct bcm2708_fb *fb)
    fb->fb.fix.xpanstep = 1;
    fb->fb.fix.ypanstep = 1;
    fb->fb.fix.ywrapstep = 0;
-   fb->fb.fix.accel = FB_ACCEL_NONE;
+   fb->fb.fix.accel = FB_ACCEL_DISPMANX;

    fb->fb.var.xres = fbwidth;
    fb->fb.var.yres = fbheight;
diff --git a/include/uapi/linux/fb.h b/include/uapi/linux/fb.h
index fa72af0..279bfc7 100644
--- a/include/uapi/linux/fb.h
+++ b/include/uapi/linux/fb.h
@@ -155,6 +155,7 @@
 #define FB_ACCEL_PROSAVAGE_DDRK 0x8e   /* S3 ProSavage DDR-K           */

 #define FB_ACCEL_PUV3_UNIGFX   0xa0    /* PKUnity-v3 Unigfx        */
+#define FB_ACCEL_DISPMANX       0xb0    /* DISPMANX Pi and friends      */

 #define FB_CAP_FOURCC      1   /* Device supports FOURCC-based formats */

@popcornmix
Copy link
Collaborator

Ping @pelwell @notro
I'm a little unsure about this. Technically the kernel framebuffer driver doesn't use dispmanx and there isn't really GPU acceleration (something vaguer like FB_ACCEL_BCM2835 might be better).

If it makes platform specific code in SDL more straightforward, then I guess it is an option, but I'd quite like to see the SDL patches.

Presumably if you are using dispmanx then the changes can't be a runtime decision (SDL won't link on other platforms), so presumably you need some sort of #ifdef USE_DISPMANX magic, which suggests runtime detection from the kernel isn't essential.

@pelwell
Copy link
Contributor

pelwell commented Jul 18, 2015

I would welcome a clearer explanation of the issues. It seems odd to have to add a runtime flag to indicate the presence of a feature that is common to all Pi's.

Is the aim to have a single SDL binary for each architecture, with runtime detection of the platform?

@pssc
Copy link
Author

pssc commented Jul 20, 2015

@pelwell Well in theory this will allow for a common hf based library with specific optimisations at runtime for the pi

@popcornmix
Copy link
Collaborator

Won't the linkage with libs for dispmanx support be a problem for other platforms.

@pssc
Copy link
Author

pssc commented Jul 20, 2015

@popcornmix yes though the main reason for me is to move to the SDL std fb acceleration framework for fall-back to plain FBcon with the std env var settings and eliminate as much duplicated/redundant code a possible. I have to link to a static/private copy of the sdl anyway atm due to hacks for being driven via libcec-deamon.

@pssc
Copy link
Author

pssc commented Oct 30, 2015

Ok I have a working driver in the SDL that supports rendering directly into a dispmanx buffer that supports multiple displays and copying to a secondary fb... I have had to put a specific hack in the app started up as auto detection is different for the Pi.

@pssc
Copy link
Author

pssc commented Mar 31, 2016

@popcornmix any chance of this happening its a small change just do indicate the frame buffer accel type within the standard FB framework.

Accel Code here:-
https://github.com/pssc/squeezeplay/blob/public/7.8/src/SDL-1.2.15/src/video/fbcon/SDL_fbdispmanx.c

Detection Code here:-
https://github.com/pssc/squeezeplay/blob/public/7.8/src/SDL-1.2.15/src/video/fbcon/SDL_fbvideo.c

Currently I have to force accel support rather than rely on SDL auto detection

So I have in my startup script-

# DISPMAX ACCEL if available auto detection broken atm see fbset -i (Accelerator)
# To force No ACCEL
# SDL_FBACCEL=0
# Most options are now in the SqueezePi Advanced menu.
if [ -r "/boot/bcm2708-rpi-b.dtb" ]; then
        # Work round borked auto detection for Pi
        SDL_FBACCEL=${SDL_FBACCEL:-176}
        export SDL_FBACCEL
fi

@popcornmix
Copy link
Collaborator

I don't really understand what's wrong with adding to SDL_fbvideo.c

#ifdef SDL_VIDEO_DRIVER_FBCON_ACCEL_DISPMANX
finfo.accel = FB_ACCEL_DISPMANX;
#endif

That seems a more obvious solution than making the kernel driver claim to have some acceleration feature that it doesn't have.

@pssc
Copy link
Author

pssc commented Mar 31, 2016

@popcornmix you can't turn it off in the standard in the standard manner(which is fixable I am just trying to contain any DSIPMANX/Pi specific code ie use standard methods/routines where possible, thinking of porting to SDL2...), but as said there is acceleration there, other drivers directly use mmaped hardware registers... there's nothing in the kernel FB driver for them either other than and indicator of the accel type we are discussing here.

@popcornmix
Copy link
Collaborator

But the kernel FB driver doesn't use dispmanx and doesn't support a FB_ACCEL_DISPMANX feature. It feels wrong to report it there.

When you are using code in SDL_fbdispmanx.c you are actually bypassing the kernel FB and rendering through a different mechanism. That code would actually work if you compiled the kernel with no framebuffer. It seems wrong to use the framebuffer driver to decide if dispmanx is supported.

@pssc
Copy link
Author

pssc commented Mar 31, 2016

@popcornmix yes graphically you are correct, however we share all the FB based console input and mapping code at this point and can easily default to standard fbcon graphics code if there are issues,

Other accelerators are by enlarge doing direct hw access by passing the kernel in most cased to do HW blitting moves, fills, syncs, and to idle.

I can see your point, but my argument would where else in the standard kernel FB api would indicate you are able to do something else, this mechanism seems to exist to indicate you can to something HW graphics specific in a uniform manner. You could call it FB_ACCEL_PI / FB_ACCEL_BCM2708.

@Ruffio
Copy link

Ruffio commented Aug 16, 2016

@pssc has your issue been resolved? If so, please close this issue. Thanks.

@pssc
Copy link
Author

pssc commented Aug 16, 2016

@Ruffio nope

@JamesH65
Copy link
Contributor

I'm going to close this as reading the above, it does not appear the proposal is appropriate. It also been a while since any updates.

If you want run time detection, isn't is easier just to ask if you are running on a Pi, in which case dispmanx is always available. Or use @popcornmix suggestion above.

sigmaris pushed a commit to sigmaris/linux that referenced this issue Feb 9, 2020
Expand dummy prog generation such that we can easily check on return
codes and add few more test cases to make sure we keep on tracking
pruning behavior.

  # ./test_verifier
  [...]
  raspberrypi#1066/p XDP pkt read, pkt_data <= pkt_meta', bad access 1 OK
  raspberrypi#1067/p XDP pkt read, pkt_data <= pkt_meta', bad access 2 OK
  Summary: 1580 PASSED, 0 SKIPPED, 0 FAILED

Also verified that JIT dump of added test cases looks good.

Signed-off-by: Daniel Borkmann <[email protected]>
Signed-off-by: Alexei Starovoitov <[email protected]>
Link: https://lore.kernel.org/bpf/df7200b6021444fd369376d227de917357285b65.1576789878.git.daniel@iogearbox.net
popcornmix pushed a commit that referenced this issue Aug 11, 2021
Add several test cases for checking update_alu_sanitation_state() under
multiple paths:

  # ./test_verifier
  [...]
  #1061/u map access: known scalar += value_ptr unknown vs const OK
  #1061/p map access: known scalar += value_ptr unknown vs const OK
  #1062/u map access: known scalar += value_ptr const vs unknown OK
  #1062/p map access: known scalar += value_ptr const vs unknown OK
  #1063/u map access: known scalar += value_ptr const vs const (ne) OK
  #1063/p map access: known scalar += value_ptr const vs const (ne) OK
  #1064/u map access: known scalar += value_ptr const vs const (eq) OK
  #1064/p map access: known scalar += value_ptr const vs const (eq) OK
  #1065/u map access: known scalar += value_ptr unknown vs unknown (eq) OK
  #1065/p map access: known scalar += value_ptr unknown vs unknown (eq) OK
  #1066/u map access: known scalar += value_ptr unknown vs unknown (lt) OK
  #1066/p map access: known scalar += value_ptr unknown vs unknown (lt) OK
  #1067/u map access: known scalar += value_ptr unknown vs unknown (gt) OK
  #1067/p map access: known scalar += value_ptr unknown vs unknown (gt) OK
  [...]
  Summary: 1762 PASSED, 0 SKIPPED, 0 FAILED

Signed-off-by: Daniel Borkmann <[email protected]>
Acked-by: Alexei Starovoitov <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants