RE: [Linux-fbdev-devel] fbdev API problems

[Sottek, M. J <mat...@in...>]

>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