Re: [Linux-fbdev-devel] [RFC 2.6.20 1/1] fbdev, mm: Deferred IO and hecubafb driver

2007-02-24 Thread Jaya Kumar

On 2/24/07, Antonino A. Daplas <[EMAIL PROTECTED]> wrote:

On Fri, 2007-02-23 at 07:32 +0100, Jaya Kumar wrote:
Can you create 2 separate patches, one for the deferred_io and another
for the driver that uses it?


Will do.


> +static struct vm_operations_struct hecubafb_vm_ops = {
> + .nopage = hecubafb_vm_nopage,
> + .page_mkwrite   = fb_deferred_io_mkwrite,
> +};
> +

It would seem to me that the above can be made generic, so we have this
instead:


We could. But I think fb_deferred_io_vm_nopage would then have to
handle the different types or possible combinations of framebuffer
allocations, if they kmalloced or vmalloced or maybe combinations. Ok.
I'll try to implement that. Maybe we'll need a flag where the driver
informs us of the type of allocation.



> +static struct fb_deferred_io hecubafb_defio = {
> + .delay  = HZ,
> + .deferred_io= hecubafb_dpy_deferred_io,
> +};

Leaving the drivers to just fill up the above. This would result in a
decrease of code duplication and it will be easier for driver writers.


I agree it'll be much cleaner that way but was worried if having
fb_defio do that would prevent driver writers from having their own
nopage and controlling other vm related functionality. Looking at
drivers/video/* shows no one touching vm_ops, so I guess we'll be
fine. If fb_defio takes ownership of the driver's nopage, vm_ops, etc,
then it could be just the above struct and the
fb_deferred_io_init/cleanup functions that are exposed to the driver.
ie: if an fbdev driver calls fb_deferred_io_init, we setup their mmap,
vm_ops and the necessary bits. I'll give that a shot.



I would prefer to have the init and cleanup functions called by the
driver themselves, instead of piggy-backing them to the
framebuffer_register/unregister.



Understood. I had done it that way originally but changed just before
posting. I'll revert back. :)


This basically dumps the entire framebuffer to the hardware, doesn't it?
This framebuffer has only 2 pages at the most, so it doesn't matter. But
for hardware with MB's of RAM, I don't think this is feasible.


Yup, hecuba doesn't have clean partial update functionality. But the
callback exposed to drivers is:

+   void (*deferred_io)(struct fb_info *info, struct list_head *pagelist);

which gives the drivers the list of dirty pages in pagelist. So other
drivers for hardware with the right capabilities can implement
selective or partial updates. As you mentioned, I imagine that the
drivers internally could have a bit array to enable them to mark and
find sequential partial updates. I would leave the page bitmap for
drivers to do since it would be specific to their hardware.

Thanks,
jaya



Is there a way to selectively update only the touched pages, ie from the
fbdevio->pagelist? struct page has a field (pgoff_t index), is this
usable? If not, can we just create a bit array, just to tell the driver
which are the dirty pages?

Tony



-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [Linux-fbdev-devel] [RFC 2.6.20 1/1] fbdev, mm: Deferred IO and hecubafb driver

2007-02-24 Thread Jaya Kumar

On 2/24/07, Antonino A. Daplas [EMAIL PROTECTED] wrote:

On Fri, 2007-02-23 at 07:32 +0100, Jaya Kumar wrote:
Can you create 2 separate patches, one for the deferred_io and another
for the driver that uses it?


Will do.


 +static struct vm_operations_struct hecubafb_vm_ops = {
 + .nopage = hecubafb_vm_nopage,
 + .page_mkwrite   = fb_deferred_io_mkwrite,
 +};
 +

It would seem to me that the above can be made generic, so we have this
instead:


We could. But I think fb_deferred_io_vm_nopage would then have to
handle the different types or possible combinations of framebuffer
allocations, if they kmalloced or vmalloced or maybe combinations. Ok.
I'll try to implement that. Maybe we'll need a flag where the driver
informs us of the type of allocation.



 +static struct fb_deferred_io hecubafb_defio = {
 + .delay  = HZ,
 + .deferred_io= hecubafb_dpy_deferred_io,
 +};

Leaving the drivers to just fill up the above. This would result in a
decrease of code duplication and it will be easier for driver writers.


I agree it'll be much cleaner that way but was worried if having
fb_defio do that would prevent driver writers from having their own
nopage and controlling other vm related functionality. Looking at
drivers/video/* shows no one touching vm_ops, so I guess we'll be
fine. If fb_defio takes ownership of the driver's nopage, vm_ops, etc,
then it could be just the above struct and the
fb_deferred_io_init/cleanup functions that are exposed to the driver.
ie: if an fbdev driver calls fb_deferred_io_init, we setup their mmap,
vm_ops and the necessary bits. I'll give that a shot.



I would prefer to have the init and cleanup functions called by the
driver themselves, instead of piggy-backing them to the
framebuffer_register/unregister.



Understood. I had done it that way originally but changed just before
posting. I'll revert back. :)


This basically dumps the entire framebuffer to the hardware, doesn't it?
This framebuffer has only 2 pages at the most, so it doesn't matter. But
for hardware with MB's of RAM, I don't think this is feasible.


Yup, hecuba doesn't have clean partial update functionality. But the
callback exposed to drivers is:

+   void (*deferred_io)(struct fb_info *info, struct list_head *pagelist);

which gives the drivers the list of dirty pages in pagelist. So other
drivers for hardware with the right capabilities can implement
selective or partial updates. As you mentioned, I imagine that the
drivers internally could have a bit array to enable them to mark and
find sequential partial updates. I would leave the page bitmap for
drivers to do since it would be specific to their hardware.

Thanks,
jaya



Is there a way to selectively update only the touched pages, ie from the
fbdevio-pagelist? struct page has a field (pgoff_t index), is this
usable? If not, can we just create a bit array, just to tell the driver
which are the dirty pages?

Tony



-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [Linux-fbdev-devel] [RFC 2.6.20 1/1] fbdev, mm: Deferred IO and hecubafb driver

2007-02-23 Thread Antonino A. Daplas
On Fri, 2007-02-23 at 07:32 +0100, Jaya Kumar wrote:
> Hi Tony, Paul, Peter, fbdev, lkml, and mm,
> 
> This is a first pass at abstracting deferred IO out from hecubafb and
> into fbdev as was discussed before: 
> http://marc.theaimsgroup.com/?l=linux-fbdev-devel=117187443327466=2
> 
> Please let me know your feedback and if it looks okay so far.
> 

Can you create 2 separate patches, one for the deferred_io and another
for the driver that uses it?

> +Another one may be if one has a device framebuffer that is in an usual 
> format,
> +say diagonally shifting RGB, this may then be a mechanism for you to allow
> +apps to pretend to have a normal framebuffer but reswizzle for the device
> +framebuffer at vsync time based on the touched pagelist.

Hmm, yes, it can be used to implement a shadow framebuffer :-)

> +
> +How to use it: (for applications)
> +-
> +No changes needed. mmap the framebuffer like normal and just use it.
> +
> +How to use it: (for fbdev drivers)
> +--
> +The following example may be helpful.
> +
> +1. Setup your mmap and vm_ops structures. Eg:
> +
> +
> +The delay is the minimum delay between when the page_mkwrite trigger occurs
> +and when the deferred_io callback is called. The deferred_io callback is
> +explained below.
> +
> +static struct vm_operations_struct hecubafb_vm_ops = {
> + .nopage = hecubafb_vm_nopage,
> + .page_mkwrite   = fb_deferred_io_mkwrite,
> +};
> +

It would seem to me that the above can be made generic, so we have this
instead:

static struct vm_operations_struct fb_deferred_vm_ops = {
.nopage = fb_deferred_io_vm_nopage,
.page_mkwrite   = fb_deferred_io_mkwrite,
};

> +You will need a nopage routine to find and retrive the struct page for your
> +framebuffer pages. You must set page_mkwrite to fb_deferred_io_mkwrite.
> +Here's the example nopage for hecubafb where it is a vmalloced framebuffer. 
> +
> +static int hecubafb_mmap(struct fb_info *info, struct vm_area_struct *vma)
> +{
> + vma->vm_ops = _vm_ops;
> + vma->vm_flags |= ( VM_IO | VM_RESERVED | VM_DONTEXPAND );
> + vma->vm_private_data = info;
> + return 0;
> +}

And this too as fb_deferred_io_mmap.

> +
> +static struct page* hecubafb_vm_nopage(struct vm_area_struct *vma, 
> + unsigned long vaddr, int *type)
> +{
> + unsigned long offset;
> + struct page *page;
> + struct fb_info *info = vma->vm_private_data;
> +
> + offset = (vaddr - vma->vm_start) + (vma->vm_pgoff << PAGE_SHIFT);
> + if (offset >= (DPY_W*DPY_H)/8)
> + return NOPAGE_SIGBUS;
> +

To make it generic, this can simply be:

if (offset >= info->fix.smem_len)
return NOPAGE_SIGBUS.

> + page = vmalloc_to_page(info->screen_base + offset);
> + if (!page)
> + return NOPAGE_OOM;
> +
> + get_page(page);
> + if (type)
> + *type = VM_FAULT_MINOR;
> + return page;
> +}
> +
> 

> +static struct fb_deferred_io hecubafb_defio = {
> + .delay  = HZ,
> + .deferred_io= hecubafb_dpy_deferred_io,
> +};

Leaving the drivers to just fill up the above. This would result in a
decrease of code duplication and it will be easier for driver writers.


> diff --git a/drivers/video/fbmem.c b/drivers/video/fbmem.c
> index 2822526..863126a 100644
> --- a/drivers/video/fbmem.c
> +++ b/drivers/video/fbmem.c
> @@ -1325,6 +1325,7 @@ register_framebuffer(struct fb_info *fb_info)
>  
>   event.info = fb_info;
>   fb_notifier_call_chain(FB_EVENT_FB_REGISTERED, );
> + fb_deferred_io_init(fb_info);
>   return 0;
>  }
>  
> @@ -1355,6 +1356,7 @@ unregister_framebuffer(struct fb_info *fb_info)
>   fb_destroy_modelist(_info->modelist);
>   registered_fb[i]=NULL;
>   num_registered_fb--;
> + fb_deferred_io_cleanup(fb_info);
>   fb_cleanup_device(fb_info);
>   device_destroy(fb_class, MKDEV(FB_MAJOR, i));
>   event.info = fb_info;

I would prefer to have the init and cleanup functions called by the
driver themselves, instead of piggy-backing them to the
framebuffer_register/unregister.


> +static void hecubafb_dpy_update(struct hecubafb_par *par)
> +{
> + int i;
> + unsigned char *buf = par->info->screen_base;
> +
> + apollo_send_command(par, 0xA0);
> +
> + for (i=0; i < (DPY_W*DPY_H/8); i++) {
> + apollo_send_data(par, *(buf++));
> + }
> +

This basically dumps the entire framebuffer to the hardware, doesn't it?
This framebuffer has only 2 pages at the most, so it doesn't matter. But
for hardware with MB's of RAM, I don't think this is feasible.

Is there a way to selectively update only the touched pages, ie from the
fbdevio->pagelist? struct page has a field (pgoff_t index), is this
usable? If not, can we just create a bit array, just to tell the driver
which are the dirty pages?

Tony

-
To unsubscribe from this list: send the line 

Re: [Linux-fbdev-devel] [RFC 2.6.20 1/1] fbdev, mm: Deferred IO and hecubafb driver

2007-02-23 Thread Antonino A. Daplas
On Fri, 2007-02-23 at 07:32 +0100, Jaya Kumar wrote:
 Hi Tony, Paul, Peter, fbdev, lkml, and mm,
 
 This is a first pass at abstracting deferred IO out from hecubafb and
 into fbdev as was discussed before: 
 http://marc.theaimsgroup.com/?l=linux-fbdev-develm=117187443327466w=2
 
 Please let me know your feedback and if it looks okay so far.
 

Can you create 2 separate patches, one for the deferred_io and another
for the driver that uses it?

 +Another one may be if one has a device framebuffer that is in an usual 
 format,
 +say diagonally shifting RGB, this may then be a mechanism for you to allow
 +apps to pretend to have a normal framebuffer but reswizzle for the device
 +framebuffer at vsync time based on the touched pagelist.

Hmm, yes, it can be used to implement a shadow framebuffer :-)

 +
 +How to use it: (for applications)
 +-
 +No changes needed. mmap the framebuffer like normal and just use it.
 +
 +How to use it: (for fbdev drivers)
 +--
 +The following example may be helpful.
 +
 +1. Setup your mmap and vm_ops structures. Eg:
 +
 +
 +The delay is the minimum delay between when the page_mkwrite trigger occurs
 +and when the deferred_io callback is called. The deferred_io callback is
 +explained below.
 +
 +static struct vm_operations_struct hecubafb_vm_ops = {
 + .nopage = hecubafb_vm_nopage,
 + .page_mkwrite   = fb_deferred_io_mkwrite,
 +};
 +

It would seem to me that the above can be made generic, so we have this
instead:

static struct vm_operations_struct fb_deferred_vm_ops = {
.nopage = fb_deferred_io_vm_nopage,
.page_mkwrite   = fb_deferred_io_mkwrite,
};

 +You will need a nopage routine to find and retrive the struct page for your
 +framebuffer pages. You must set page_mkwrite to fb_deferred_io_mkwrite.
 +Here's the example nopage for hecubafb where it is a vmalloced framebuffer. 
 +
 +static int hecubafb_mmap(struct fb_info *info, struct vm_area_struct *vma)
 +{
 + vma-vm_ops = hecubafb_vm_ops;
 + vma-vm_flags |= ( VM_IO | VM_RESERVED | VM_DONTEXPAND );
 + vma-vm_private_data = info;
 + return 0;
 +}

And this too as fb_deferred_io_mmap.

 +
 +static struct page* hecubafb_vm_nopage(struct vm_area_struct *vma, 
 + unsigned long vaddr, int *type)
 +{
 + unsigned long offset;
 + struct page *page;
 + struct fb_info *info = vma-vm_private_data;
 +
 + offset = (vaddr - vma-vm_start) + (vma-vm_pgoff  PAGE_SHIFT);
 + if (offset = (DPY_W*DPY_H)/8)
 + return NOPAGE_SIGBUS;
 +

To make it generic, this can simply be:

if (offset = info-fix.smem_len)
return NOPAGE_SIGBUS.

 + page = vmalloc_to_page(info-screen_base + offset);
 + if (!page)
 + return NOPAGE_OOM;
 +
 + get_page(page);
 + if (type)
 + *type = VM_FAULT_MINOR;
 + return page;
 +}
 +
 

 +static struct fb_deferred_io hecubafb_defio = {
 + .delay  = HZ,
 + .deferred_io= hecubafb_dpy_deferred_io,
 +};

Leaving the drivers to just fill up the above. This would result in a
decrease of code duplication and it will be easier for driver writers.


 diff --git a/drivers/video/fbmem.c b/drivers/video/fbmem.c
 index 2822526..863126a 100644
 --- a/drivers/video/fbmem.c
 +++ b/drivers/video/fbmem.c
 @@ -1325,6 +1325,7 @@ register_framebuffer(struct fb_info *fb_info)
  
   event.info = fb_info;
   fb_notifier_call_chain(FB_EVENT_FB_REGISTERED, event);
 + fb_deferred_io_init(fb_info);
   return 0;
  }
  
 @@ -1355,6 +1356,7 @@ unregister_framebuffer(struct fb_info *fb_info)
   fb_destroy_modelist(fb_info-modelist);
   registered_fb[i]=NULL;
   num_registered_fb--;
 + fb_deferred_io_cleanup(fb_info);
   fb_cleanup_device(fb_info);
   device_destroy(fb_class, MKDEV(FB_MAJOR, i));
   event.info = fb_info;

I would prefer to have the init and cleanup functions called by the
driver themselves, instead of piggy-backing them to the
framebuffer_register/unregister.


 +static void hecubafb_dpy_update(struct hecubafb_par *par)
 +{
 + int i;
 + unsigned char *buf = par-info-screen_base;
 +
 + apollo_send_command(par, 0xA0);
 +
 + for (i=0; i  (DPY_W*DPY_H/8); i++) {
 + apollo_send_data(par, *(buf++));
 + }
 +

This basically dumps the entire framebuffer to the hardware, doesn't it?
This framebuffer has only 2 pages at the most, so it doesn't matter. But
for hardware with MB's of RAM, I don't think this is feasible.

Is there a way to selectively update only the touched pages, ie from the
fbdevio-pagelist? struct page has a field (pgoff_t index), is this
usable? If not, can we just create a bit array, just to tell the driver
which are the dirty pages?

Tony

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at