Hi,
Is there any reason for not adding these ioctls to fbdev? I searched
the net and couldn't any. Anyway, these patches simply implement those
ioctls.
Ali Gholami Rudi (2):
fbdev: add FBIOFILLRECT ioctl
fbdev: add FBIOCOPYAREA ioctl
drivers/video/fbmem.c | 50 +++++++++++++++++++++++++++++++++++++++++++++++++
include/linux/fb.h | 2 +
2 files changed, 52 insertions(+), 0 deletions(-)
Signed-off-by: Ali Gholami Rudi <[email protected]>
---
drivers/video/fbmem.c | 25 +++++++++++++++++++++++++
include/linux/fb.h | 1 +
2 files changed, 26 insertions(+), 0 deletions(-)
diff --git a/drivers/video/fbmem.c b/drivers/video/fbmem.c
index a85c818..a90cd0f 100644
--- a/drivers/video/fbmem.c
+++ b/drivers/video/fbmem.c
@@ -1016,6 +1016,25 @@ fb_blank(struct fb_info *info, int blank)
return ret;
}
+static int fb_fillrect_user(struct fb_info *info,
+ struct fb_fillrect *fill)
+{
+ int ret = 0;
+ if (fill->rop != ROP_COPY && fill->rop != ROP_XOR)
+ return -EINVAL;
+ if (!lock_fb_info(info))
+ return -ENODEV;
+ if (fill->dx + fill->width > info->var.xres ||
+ fill->dy + fill->height > info->var.yres) {
+ ret = -EINVAL;
+ goto out;
+ }
+ info->fbops->fb_fillrect(info, fill);
+out:
+ unlock_fb_info(info);
+ return ret;
+}
+
static long do_fb_ioctl(struct fb_info *info, unsigned int cmd,
unsigned long arg)
{
@@ -1026,6 +1045,7 @@ static long do_fb_ioctl(struct fb_info *info, unsigned int cmd,
struct fb_cmap cmap_from;
struct fb_cmap_user cmap;
struct fb_event event;
+ struct fb_fillrect fill;
void __user *argp = (void __user *)arg;
long ret = 0;
@@ -1133,6 +1153,11 @@ static long do_fb_ioctl(struct fb_info *info, unsigned int cmd,
release_console_sem();
unlock_fb_info(info);
break;
+ case FBIOFILLRECT:
+ if (copy_from_user(&fill, argp, sizeof(fill)))
+ return -EFAULT;
+ ret = fb_fillrect_user(info, &fill);
+ break;
default:
if (!lock_fb_info(info))
return -ENODEV;
diff --git a/include/linux/fb.h b/include/linux/fb.h
index f847df9..40cc99a 100644
--- a/include/linux/fb.h
+++ b/include/linux/fb.h
@@ -37,6 +37,7 @@ struct dentry;
#define FBIOGET_HWCINFO 0x4616
#define FBIOPUT_MODEINFO 0x4617
#define FBIOGET_DISPINFO 0x4618
+#define FBIOFILLRECT 0x4619
#define FB_TYPE_PACKED_PIXELS 0 /* Packed Pixels */
Signed-off-by: Ali Gholami Rudi <[email protected]>
---
drivers/video/fbmem.c | 25 +++++++++++++++++++++++++
include/linux/fb.h | 1 +
2 files changed, 26 insertions(+), 0 deletions(-)
diff --git a/drivers/video/fbmem.c b/drivers/video/fbmem.c
index a90cd0f..518119a 100644
--- a/drivers/video/fbmem.c
+++ b/drivers/video/fbmem.c
@@ -1035,6 +1035,25 @@ out:
return ret;
}
+static int fb_copyarea_user(struct fb_info *info,
+ struct fb_copyarea *copy)
+{
+ int ret = 0;
+ if (!lock_fb_info(info))
+ return -ENODEV;
+ if (copy->dx + copy->width > info->var.xres ||
+ copy->sx + copy->width > info->var.xres ||
+ copy->dy + copy->height > info->var.yres ||
+ copy->sy + copy->height > info->var.yres) {
+ ret = -EINVAL;
+ goto out;
+ }
+ info->fbops->fb_copyarea(info, copy);
+out:
+ unlock_fb_info(info);
+ return ret;
+}
+
static long do_fb_ioctl(struct fb_info *info, unsigned int cmd,
unsigned long arg)
{
@@ -1046,6 +1065,7 @@ static long do_fb_ioctl(struct fb_info *info, unsigned int cmd,
struct fb_cmap_user cmap;
struct fb_event event;
struct fb_fillrect fill;
+ struct fb_copyarea copy;
void __user *argp = (void __user *)arg;
long ret = 0;
@@ -1158,6 +1178,11 @@ static long do_fb_ioctl(struct fb_info *info, unsigned int cmd,
return -EFAULT;
ret = fb_fillrect_user(info, &fill);
break;
+ case FBIOCOPYAREA:
+ if (copy_from_user(©, argp, sizeof(copy)))
+ return -EFAULT;
+ ret = fb_copyarea_user(info, ©);
+ break;
default:
if (!lock_fb_info(info))
return -ENODEV;
diff --git a/include/linux/fb.h b/include/linux/fb.h
index 40cc99a..f1cf8df 100644
--- a/include/linux/fb.h
+++ b/include/linux/fb.h
@@ -38,6 +38,7 @@ struct dentry;
#define FBIOPUT_MODEINFO 0x4617
#define FBIOGET_DISPINFO 0x4618
#define FBIOFILLRECT 0x4619
+#define FBIOCOPYAREA 0x461A
#define FB_TYPE_PACKED_PIXELS 0 /* Packed Pixels */
On Mon, 13 Jul 2009 19:47:09 +0430
Ali Gholami Rudi <[email protected]> wrote:
> Hi,
>
> Is there any reason for not adding these ioctls to fbdev? I searched
> the net and couldn't any. Anyway, these patches simply implement
> those ioctls.
>
can we turn this around, is there a reason to add them?
or in other words, how / where would these be used ?
--
Arjan van de Ven Intel Open Source Technology Centre
For development, discussion and tips for power savings,
visit http://www.lesswatts.org
Arjan van de Ven <[email protected]> wrote:
> On Mon, 13 Jul 2009 19:47:09 +0430
> Ali Gholami Rudi <[email protected]> wrote:
> > Is there any reason for not adding these ioctls to fbdev? I searched
> > the net and couldn't any. Anyway, these patches simply implement
> > those ioctls.
> >
>
> can we turn this around, is there a reason to add them?
> or in other words, how / where would these be used ?
User-space programs that use framebuffer directly can use them. I was
writing a simple framebuffer virtual terminal (using libfreetype for
fonts; like fbterm); scrolling and painting boxes would be faster if
there was someway of using hardware accelerated operations. I think
other similar programs can benefit, too.
Ali
On Mon, Jul 13, 2009 at 07:47:09PM +0430, Ali Gholami Rudi wrote:
> Hi,
>
> Is there any reason for not adding these ioctls to fbdev? I searched
> the net and couldn't any. Anyway, these patches simply implement those
> ioctls.
Have you also added the needed 32 vs. 64 bit handlers for these ioctls?
thanks,
greg k-h
On Monday 13 July 2009, Greg KH wrote:
> On Mon, Jul 13, 2009 at 07:47:09PM +0430, Ali Gholami Rudi wrote:
> > Hi,
> >
> > Is there any reason for not adding these ioctls to fbdev? I searched
> > the net and couldn't any. Anyway, these patches simply implement those
> > ioctls.
>
> Have you also added the needed 32 vs. 64 bit handlers for these ioctls?
>
They are not there, but could be trivially added with
diff --git a/drivers/video/fbmem.c b/drivers/video/fbmem.c
index f8a09bf..ce107c4 100644
--- a/drivers/video/fbmem.c
+++ b/drivers/video/fbmem.c
@@ -1285,6 +1285,8 @@ static long fb_compat_ioctl(struct file *file, unsigned int cmd,
case FBIOPAN_DISPLAY:
case FBIOGET_CON2FBMAP:
case FBIOPUT_CON2FBMAP:
+ case FBIOCOPYAREA:
+ case FBIOFILLRECT:
arg = (unsigned long) compat_ptr(arg);
case FBIOBLANK:
ret = do_fb_ioctl(info, cmd, arg);
Unfortunately, it would not be as easy to add an FBIOIMAGE, which sounds
equally useful.
Arnd <><
Arnd Bergmann <[email protected]> wrote:
> On Monday 13 July 2009, Greg KH wrote:
> > On Mon, Jul 13, 2009 at 07:47:09PM +0430, Ali Gholami Rudi wrote:
> > > Is there any reason for not adding these ioctls to fbdev? I searched
> > > the net and couldn't any. Anyway, these patches simply implement those
> > > ioctls.
> >
> > Have you also added the needed 32 vs. 64 bit handlers for these ioctls?
> >
>
> They are not there, but could be trivially added with
>
> diff --git a/drivers/video/fbmem.c b/drivers/video/fbmem.c
> index f8a09bf..ce107c4 100644
> --- a/drivers/video/fbmem.c
> +++ b/drivers/video/fbmem.c
> @@ -1285,6 +1285,8 @@ static long fb_compat_ioctl(struct file *file, unsigned int cmd,
> case FBIOPAN_DISPLAY:
> case FBIOGET_CON2FBMAP:
> case FBIOPUT_CON2FBMAP:
> + case FBIOCOPYAREA:
> + case FBIOFILLRECT:
> arg = (unsigned long) compat_ptr(arg);
> case FBIOBLANK:
> ret = do_fb_ioctl(info, cmd, arg);
Thanks, I'll update the patches.
Ali
Greg KH <[email protected]> wrote:
> On Mon, Jul 13, 2009 at 07:47:09PM +0430, Ali Gholami Rudi wrote:
> > Is there any reason for not adding these ioctls to fbdev? I searched
> > the net and couldn't any. Anyway, these patches simply implement those
> > ioctls.
>
> Have you also added the needed 32 vs. 64 bit handlers for these ioctls?
I guess Arnd's patch will do.
By the way, it should check for overflows, too? I mean should I change
this:
> + if (copy->dx + copy->width > info->var.xres ||
> + copy->sx + copy->width > info->var.xres ||
> + copy->dy + copy->height > info->var.yres ||
> + copy->sy + copy->height > info->var.yres) {
#define ISSUMLESSTHAN(a, b, s) (((a) <= (s)) && ((b) <= (s)) && \
((a) + (b) <= (s)))
if (!ISSUMLESSTHAN(copy->dx, copy->width, info->var.xres) || ...
Ali
On Tue, Jul 14, 2009 at 2:25 AM, Ali Gholami Rudi<[email protected]> wrote:
> Arjan van de Ven <[email protected]> wrote:
>> On Mon, 13 Jul 2009 19:47:09 +0430
>> Ali Gholami Rudi <[email protected]> wrote:
>> > Is there any reason for not adding these ioctls to fbdev? ?I searched
>> > the net and couldn't any. ?Anyway, these patches simply implement
>> > those ioctls.
>> >
>>
>> can we turn this around, is there a reason to add them?
>> or in other words, how / where would these be used ?
>
> User-space programs that use framebuffer directly can use them. ?I was
> writing a simple framebuffer virtual terminal (using libfreetype for
> fonts; like fbterm); scrolling and painting boxes would be faster if
> there was someway of using hardware accelerated operations. ?I think
> other similar programs can benefit, too.
The general opinion is we should keep acceleration in userspace if at
all possible.
Not all hw can implement these usefully in the kernel, directfb
already does some
things for this.
Dave.
Dave Airlie <[email protected]> wrote:
> On Tue, Jul 14, 2009 at 2:25 AM, Ali Gholami Rudi<[email protected]> wrote:
> > Arjan van de Ven <[email protected]> wrote:
> >> can we turn this around, is there a reason to add them?
> >> or in other words, how / where would these be used ?
> >
> > User-space programs that use framebuffer directly can use them. ?I was
> > writing a simple framebuffer virtual terminal (using libfreetype for
> > fonts; like fbterm); scrolling and painting boxes would be faster if
> > there was someway of using hardware accelerated operations. ?I think
> > other similar programs can benefit, too.
>
> The general opinion is we should keep acceleration in userspace if at
> all possible.
I see. The line between user- and kernel-space for graphic applications
is very blurred to me :-)
> Not all hw can implement these usefully in the kernel, directfb
AFAICT, many major ones like intelfb, radeonfb and nv implement them and
those that can't, use a software implementation. You mean they are
unreliable or that there is little performance improvement because of
the way those operations are implemented in the kernel?
> already does some
> things for this.
Sometime ago I did try running directfb on a radeon r300 and it failed.
I didn't try hard to see what's wrong but I got the impression that
userspace apps are not good at using the hardware directly.
Ali
On Tue, Jul 14, 2009 at 1:43 PM, Ali Gholami Rudi<[email protected]> wrote:
> Dave Airlie <[email protected]> wrote:
>> On Tue, Jul 14, 2009 at 2:25 AM, Ali Gholami Rudi<[email protected]> wrote:
>> > Arjan van de Ven <[email protected]> wrote:
>> >> can we turn this around, is there a reason to add them?
>> >> or in other words, how / where would these be used ?
>> >
>> > User-space programs that use framebuffer directly can use them. ?I was
>> > writing a simple framebuffer virtual terminal (using libfreetype for
>> > fonts; like fbterm); scrolling and painting boxes would be faster if
>> > there was someway of using hardware accelerated operations. ?I think
>> > other similar programs can benefit, too.
>>
>> The general opinion is we should keep acceleration in userspace if at
>> all possible.
>
> I see. ?The line between user- and kernel-space for graphic applications
> is very blurred to me :-)
>
>> Not all hw can implement these usefully in the kernel, directfb
>
> AFAICT, many major ones like intelfb, radeonfb and nv implement them and
> those that can't, use a software implementation. ?You mean they are
> unreliable or that there is little performance improvement because of
> the way those operations are implemented in the kernel?
Generally newer consumer HW can't implement a simple blit without using the
3D engine which requires a large amount of state to be sent to the device.
Keeping the state encoding and stuff out of the kernel and in userspace is
generally seen as the right thing to do.
>> already does some
>> things for this.
>
> Sometime ago I did try running directfb on a radeon r300 and it failed.
> I didn't try hard to see what's wrong but I got the impression that
> userspace apps are not good at using the hardware directly.
directfb has hw accel for different cards, really porting the code out of
the kernel for the cards you are interested in would probably benefit
more.
Dave.