Menu
▾
▴
linux-fbdev-devel
|
From: Sottek, M. J <mat...@in...> - 2001-11-19 16:11:10
|
>In fact imageblit is from a newer implementation, and meant for 2.5.x. Geert, Does this imply that in the newer version there is no high level drawing functions that are touching my mmio and memory pointers. It is all contained inside the driver? I certainly hope so. To add to the point these pointers should not even be visible to the outside-of-driver world. It someone can see a virtual pointer, at some point they will try to use it. In some drivers this would cause bad things to happen. So what am I to do in the mean time? Is there a way to keep people from using the pointers? -Matt |
|
From: Sottek, M. J <mat...@in...> - 2001-11-20 19:48:49
|
>You don't intent to run X ontop of your fbdev, do you ?
>X uses the mmio & memory pointer to do its stuff, the same happens
>for other apps (all of them) that use the fbdev. Even the DRI/DRM
>stuff is mostly userspace, and the OpenGL access the board
>memory/mmio, trough the dma engine though.
Yes I am aware of how the X server works. You have misunderstood
the issues I want to resolve. see below...
BTW: The X server gets mmio access because it runs as root. The
DRM interfaces do not allow the userspace drivers to access anything
that is insecure, most mmio regions fall into the insecure category.
Also, the DRM does not have any driver private information visible
in public data structures. Only device independent information is
in public structures the rest are in structures that are privately
defined in both the kernel portion and the X server portion of the
driver.
>And getting all image stuff into the driver is nto the _right way_,
>at least that is what linus decreted as he boycotted every attemps
>to do so, and most people would agree with him, doing the minimal
>thing in the driver/kernel space and the rest in userpsace.
It isn't a user vs. kernel space issue. It is a public vs. private
data structure issue. Drivers have intimate knowledge of the hardware
regardless of if they reside in userspace or are part of the kernel.
Currently mmio and fb pointers are in a PUBLIC data structure, a data
structure that is defined for everyone using the fb interfaces, both
kernel side and user side. This is the wrong thing to do. The current
fb interfaces have traded away code separation in order to achieve
code reuse. This could have been done better without losing any
of the easy-to-implement code reuse features.
Let me offer some other ways:
#1 Nobody without intimate hardware knowledge should have access to
the mmio region. If they don't know how to use it, they have no reason
to touch it, right? So why put it in a user visible, public data
structure? It should be up to the kernel driver and the userspace
driver to work out a way of passing this information. A device private
ioctl is a way to go. Even a well defined get_driver_private() ioctl
is fine.
#2 What about frambuffer size/location pointers? This is the same issue,
but with far greater consequences. A user driver can obtain the physical
pointers through a driver private structure, but the var structure
currently has a kernel virtual address to the framebuffer! This allows
parts of the kernel outside the driver to write to the framebuffer!
(The logo code is an example) This is a real problem if for instance,
like the i810, you can make use of banked memory modes. In those modes
(The only ones available without programming the gart) there IS NO
framebuffer pointer/size combination that will work. You cannot write
to the framebuffer without intimate knowledge of the hardware.
If you give people easy access to pointers they WILL use them.
#3 Keep your code reuse. The fb drivers have three layers (at least)
the fb infrastructure, the hardware driver, and various subdrivers.
The struct display_switch is one set of subdrivers that are reused
for most drivers (and rightly so). However they obtain their driver
layer pointers from the fb infrastructure layer, skipping a layer of
separation. The logo code accesses pointers (that belong to the driver)
to go straight to hardware, again skipping a layer. Something like
this fixes up this problem.
struct display_switch {
(some function pointer *)setup;
(some function pointer *)putc;
...
void *priv;
}
struct generic_ds_priv {
u32 fb_base;
u32 fb_size;
u32 fb_pitch;
u32 *pseudo_palette;
...
}
Driver calls something like this:
my_fb_info.display_sw = create_generic_ds(base,size,pitch,depth);
The generic code keeps the "secret" pointers in a private data
structure. These pointers are provided directly from the driver layer.
Each display_switch function is passed the "void *priv" as an argument
which is then cast to the generic_sw_priv type. Drivers that don't use
the generic display_switch subdirver can create their own private
strictures to pass to the subdriver, or they can pass their whole
driver private structure.
Let me sum up the problem discussed here as I see it. The #1 rule
of driver writing is that only a driver should touch the hardware.
Anything that is device independent should go through a driver
interface to access the hardware. This interface could be in
userspace (i.e. OpenGL which then has a userspace hardware driver
that is allowed to access secure features of the hardware from
userspace) or in the kernel.
-Matt
|
|
From: James S. <jsi...@tr...> - 2001-11-22 00:23:22
|
> Currently mmio and fb pointers are in a PUBLIC data structure, a data > structure that is defined for everyone using the fb interfaces, both > kernel side and user side. That is so you can do things like use the physical memory address of the framebuffer to display a image from a TV card into it. The frmaebuffer itself is pretty safe to export to userland. MMIO can be tricker. The area of graphics resource management is really complex. For now I'm just aiming to get a clean seperation of the console system out of the framebuffer layer. Personally I like to see the day when DRI and the fbdev layer are merged but that will be awhile. |
|
From: Sottek, M. J <mat...@in...> - 2001-11-27 01:06:38
|
>Set the fb_fix fields for mmio and smem to zero. As for internally with
>the new api you don't need to use the screen_base field.
>Just ignore it.
After further review (correct me If I am wrong James) it seems that
screen base (a kernel virtual address to the framebuffer) is needed
for the fb_read and fb_write functions. It was suggested that
there isn't a way around this.
I have to differ. It seems that in order to make writing a fb driver
easy, sometimes the fb interface layer pretends it is a driver and
makes direct access to the device. This is another case where this
just isn't the right thing to do for everyone. You have to allow
for a driver to be created that does ALL the driver work itself.
I think the base set of fb interfaces should include:
fb_put: Put data from somewhere into the framebuffer. This is called
imageblit in the new API. This should have a parameter to tell you
to use copy_from_user() or memcpy()
fb_get: Get data from the framebuffer and put it in a provided pointer.
This should have a flag to tell you to use copy_to_user() or
memcpy()
fb_set: Set a region of the frambuffer with the value provided. This
is called fillrect(?) in the new api.
I have thought of a compromise that should be very easy for most
drivers to do, and will allow us to keep the driver private stuff
hidden.
void init_generic_ops(&my_fb_info,fb_base,fb_size,fb_pitch);
void destroy_generic_ops(&my_fb_info);
This stores the base,size,pitch information in a hidden data
structure that is used for all generic function implementations.
This structure could be static to the generic ops code or
could be passed around in some context structure, but shouldn't
be directly accessible by drivers.
Here is some example code:
/* fb_private.h */
struct fb_generic {
u8 *fb_base;
u32 size;
u32 pitch;
anything else?
}
struct fb_context {
struct fb_generic generic;
other stuff?
}
fb_context *fb_get_context(u32 node);
/* fb_private.c */
static struct fb_context[MAX_FRAMEBUFFERS];
fb_context *fb_get_context(u32 node) {
return &fb_context[node];
}
/* fb_generic.c */
#include <fb_private.h>
void init_generic_ops(struct fb_info *info,u8 *fb_base,
u32 size, u32 pitch) {
struct fb_context *context = fb_get_context(fb_info->node);
context->generic.fb_base = fb_base;
context->generic.fb_size = fb_size;
context->generic.fb_pitch = fb_pitch;
}
/* somewhere in fbmem.c */
...
Break down the read into an offset and size.
...
if(!fb->fb_read) {
struct fb_context *context = fb_get_context(info->node);
generic_fb_read(offset, size, &context->generic, COPY_TO_USER);
}
else {
fb->fb_read(offset, size, info->par, COPY_TO_USER);
}
The only thing a driver would have to do is leave the fb_put,fb_set,
fb_get == NULL and call init_generic_ops() whenever their framebuffer
changes size, pitch or location.
Then, once we have a context, we can hide anything in there that should
not be seen by the driver or the kernel (outside the fb interface)
or users.
-Matt
|
|
From: James S. <jsi...@tr...> - 2001-11-27 19:18:38
|
> >Set the fb_fix fields for mmio and smem to zero. As for internally with > >the new api you don't need to use the screen_base field. > >Just ignore it. > > After further review (correct me If I am wrong James) it seems that > screen base (a kernel virtual address to the framebuffer) is needed > for the fb_read and fb_write functions. It was suggested that > there isn't a way around this. Well at present true. Actually I have been thinking about this problem. Plus someone talked about this a few days ago on the list. I personally have run inot the the problem of data write size restrictions. For me it was the Epson 1385 framebuffer. It only allows 16 bit accesses to the framebuffer. So if I do a raw cat I get strips down my screen. Lets not forget that use can't use memset on PPC on a framebuffer. So I agree we need to have fbdev functions for access to the framebuffer and they should be made avaliable to userland. The question is where to place these macros? So for fb_read and fb_write we need to copy the userland data into a temporary buffer and then use fb_memset to actually draw the image. > fb_put: Put data from somewhere into the framebuffer. This is called > imageblit in the new API. This should have a parameter to tell you > to use copy_from_user() or memcpy() Yeap. > fb_get: Get data from the framebuffer and put it in a provided pointer. > This should have a flag to tell you to use copy_to_user() or > memcpy() See what I said above. We should also consider the endian of the buffers to. > fb_set: Set a region of the frambuffer with the value provided. This > is called fillrect(?) in the new api. Yes. Remember tho it only does a rectangle of one color. We also nedd fb_write functions. > I have thought of a compromise that should be very easy for most > drivers to do, and will allow us to keep the driver private stuff > hidden. [snip]... Oh my. I really have to think about this. For now lets just cleanup what we have. |
|
From: Sottek, M. J <mat...@in...> - 2001-11-27 20:12:06
|
>Well at present true. Actually I have been thinking about this
>problem. Plus someone talked about this a few days ago on the list.
>I personally have run inot the the problem of data write size
>restrictions. For me it was the Epson 1385 framebuffer. It only
>allows 16 bit accesses to the framebuffer. So if I do a raw cat
>I get strips down my screen. Lets not forget that use can't use
>memset on PPC on a framebuffer. So I agree we need to have fbdev
>functions for access to the framebuffer and they should
>be made avaliable to userland. The question is where to place
>these macros?
I've though of several use cases that make the current way of doing
things just unworkable.
#1 Banked memory. The whole reason I got involved in this was to
complete a stolen memory driver for the i810. It only has banked
memory in this mode and just can't work with the way things are.
#2 Network console? What happens if you want to write a driver that
is actually a networked console display? There is no framebuffer
locally so you can't access if without driver interfaces.
>So for fb_read and fb_write we need to copy the userland data into
>a temporary buffer and then use fb_memset to actually draw the image.
No, I was thinking that the functions below would be of this type
#define COPY_TO_USER 0x00000001
#define MEMCPY 0x00000002
void *fb_put(u8 *data, u32 srcx, u32 srcy, u32 width, u32 height,
u32 destx, u32 desty, u32 flags)
The driver would check flags and either memcpy or copy_to_user.
We probably need an fb_copy_to_user to handle byte swapping on
multiplatform drivers.
So no intermediate copy. That would be too slow.
>Yes. Remember tho it only does a rectangle of one color. We also
>nedd fb_write functions.
I think you misinterpreted one of my functions....plus I left one
out.
fb_put (Set a chunk of the framebuffer from the data provided. The
data comes fro userland or kernelland)
fb_set (The a chunk of the framebuffer to a single value)
fb_get (Get a chunk of data fro the framebuffer to the place
requested. It can be userland or kernelland)
fb_copy (aka blit, screen_to_screen)
>Oh my. I really have to think about this. For now lets just
>cleanup what we have.
This is hard for me to swallow since "what we have" cannot possibly
work for what I'm trying to do. It will only get worse if mmap
goes away.
I am writing up all the concepts I have floating around in my head
and I'll post it when I'm done. I have to believe that fixing all
the major issues is easier than trying to fix small parts.
-Matt
|
|
From: James S. <jsi...@tr...> - 2001-11-27 22:05:58
|
> I've though of several use cases that make the current way of doing > things just unworkable. Okay. We have many cases where this is broken. [snip].. > I think you misinterpreted one of my functions....plus I left one > out. > > fb_put (Set a chunk of the framebuffer from the data provided. The > data comes fro userland or kernelland) > fb_set (The a chunk of the framebuffer to a single value) > fb_get (Get a chunk of data fro the framebuffer to the place > requested. It can be userland or kernelland) > fb_copy (aka blit, screen_to_screen) Okay. I'm warming up to this idea. Actually what I'm thinking is we have header filies for various graphics cards and each graphics card has a #include <linux/fb.h> Now in say each graphics card header we place a inline function that userland can use as well as the kernel for doing the basic functions: fb_put_pixels fb_get_pixels fb_memset fb_rectfill fb_copyarea fb_imageblit Just have the basic functions for the kernel tho. Lets not get crazy here. > This is hard for me to swallow since "what we have" cannot possibly > work for what I'm trying to do. It will only get worse if mmap > goes away. What I mean is the ideas are floating around but I can't keep track of everything. > I am writing up all the concepts I have floating around in my head > and I'll post it when I'm done. I have to believe that fixing all > the major issues is easier than trying to fix small parts. Okay. That would be good. |
|
From: Sven <lu...@dp...> - 2001-11-20 18:35:34
|
On Mon, Nov 19, 2001 at 08:10:09AM -0800, Sottek, Matthew J wrote: > > >In fact imageblit is from a newer implementation, and meant for 2.5.x. > > Geert, > Does this imply that in the newer version there is no high level > drawing functions that are touching my mmio and memory pointers. It > is all contained inside the driver? I certainly hope so. > To add to the point these pointers should not even be visible to > the outside-of-driver world. It someone can see a virtual pointer, > at some point they will try to use it. In some drivers this would > cause bad things to happen. > > So what am I to do in the mean time? Is there a way to keep people > from using the pointers? You don't intent to run X ontop of your fbdev, do you ? X uses the mmio & memory pointer to do its stuff, the same happens for other apps (all of them) that use the fbdev. Even the DRI/DRM stuff is mostly userspace, and the OpenGL access the board memory/mmio, trough the dma engine though. And getting all image stuff into the driver is nto the _right way_, at least that is what linus decreted as he boycotted every attemps to do so, and most people would agree with him, doing the minimal thing in the driver/kernel space and the rest in userpsace. Friendly, Sven Luther |
|
From: James S. <jsi...@tr...> - 2001-11-21 20:17:56
|
> You don't intent to run X ontop of your fbdev, do you ? > X uses the mmio & memory pointer to do its stuff, the same happens for other > apps (all of them) that use the fbdev. Even the DRI/DRM stuff is mostly > userspace, and the OpenGL access the board memory/mmio, trough the dma engine > though. Sottek it isn't such a bad system if you place in the driver safety nets. For example if a userland process hose the graphics engine then the driver would reset the engine. > And getting all image stuff into the driver is nto the _right way_, at least > that is what linus decreted as he boycotted every attemps to do so, and most > people would agree with him, doing the minimal thing in the driver/kernel > space and the rest in userpsace. Agree. The only reason we are moving to basic accel functions is because it is lighter weight then the current system and its more driver transparent. The accel functions choosen are the ones we absolutely need. |
|
From: James S. <jsi...@tr...> - 2001-11-21 20:12:34
|
> >In fact imageblit is from a newer implementation, and meant for 2.5.x. > > Geert, > Does this imply that in the newer version there is no high level > drawing functions that are touching my mmio and memory pointers. It > is all contained inside the driver? I certainly hope so. TIme to explain the new api a little bit. The new api is designed to seperate the console code from the framebuffer code. This allows you two things, a simpler api to work with and second you can run fbdev by itself on embedded devices. To do this the first thing to go was the fbcon-cfb* stuff. Instead the driver writer provides 3 basic accel functions which if you use the fbcon layer uses these functions to preform console functions. I abstracted it to be based around a graphics cards abilties to encourge people to implement and plus it does hide more the hardware features. It will avoid the problems you are having. > To add to the point these pointers should not even be visible to > the outside-of-driver world. It someone can see a virtual pointer, > at some point they will try to use it. In some drivers this would > cause bad things to happen. Another nice featue planned for 2.5.X is that the console system will be "shutdown" when you explictly open /dev/fb. This will prevent any conflicts between the console system and using graphics hardware in userland. As for userland programming hardware. Well that is DRI domain and it is a touchy subject I like to avoid. |
×
Want the latest updates on software, tech news, and AI?
Get latest updates about software, tech news, and AI from SourceForge directly in your inbox once a month.
Thanks for helping keep SourceForge clean.
X