| Message ID | 20230425142846.730-7-tzimmermann@suse.de |
|---|---|
| State | Superseded |
| Headers | show |
| Series | drm,fbdev: Use fbdev's I/O helpers | expand |
Thomas Zimmermann <tzimmermann@suse.de> writes: > Implement DRM fbdev helpers for reading and writing framebuffer > memory with the respective fbdev functions. Removes duplicate > code. > > Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> > --- > drivers/gpu/drm/drm_fb_helper.c | 174 +------------------------------- > 1 file changed, 4 insertions(+), 170 deletions(-) > Very nice cleanup! Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
Tested-by: Sui Jingfeng <suijingfeng@loongson.cn> On 2023/4/25 22:28, Thomas Zimmermann wrote: > Implement DRM fbdev helpers for reading and writing framebuffer > memory with the respective fbdev functions. Removes duplicate > code. > > Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> > Reviewed-by: Javier Martinez Canillas <javierm@redhat.com> > --- > drivers/gpu/drm/drm_fb_helper.c | 174 +------------------------------- > 1 file changed, 4 insertions(+), 170 deletions(-) > > diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c > index 6bb1b8b27d7a..e11858470fa7 100644 > --- a/drivers/gpu/drm/drm_fb_helper.c > +++ b/drivers/gpu/drm/drm_fb_helper.c > @@ -714,95 +714,6 @@ void drm_fb_helper_deferred_io(struct fb_info *info, struct list_head *pagerefli > } > EXPORT_SYMBOL(drm_fb_helper_deferred_io); > > -typedef ssize_t (*drm_fb_helper_read_screen)(struct fb_info *info, char __user *buf, > - size_t count, loff_t pos); > - > -static ssize_t __drm_fb_helper_read(struct fb_info *info, char __user *buf, size_t count, > - loff_t *ppos, drm_fb_helper_read_screen read_screen) > -{ > - loff_t pos = *ppos; > - size_t total_size; > - ssize_t ret; > - > - if (info->screen_size) > - total_size = info->screen_size; > - else > - total_size = info->fix.smem_len; > - > - if (pos >= total_size) > - return 0; > - if (count >= total_size) > - count = total_size; > - if (total_size - count < pos) > - count = total_size - pos; > - > - if (info->fbops->fb_sync) > - info->fbops->fb_sync(info); > - > - ret = read_screen(info, buf, count, pos); > - if (ret > 0) > - *ppos += ret; > - > - return ret; > -} > - > -typedef ssize_t (*drm_fb_helper_write_screen)(struct fb_info *info, const char __user *buf, > - size_t count, loff_t pos); > - > -static ssize_t __drm_fb_helper_write(struct fb_info *info, const char __user *buf, size_t count, > - loff_t *ppos, drm_fb_helper_write_screen write_screen) > -{ > - loff_t pos = *ppos; > - size_t total_size; > - ssize_t ret; > - int err = 0; > - > - if (info->screen_size) > - total_size = info->screen_size; > - else > - total_size = info->fix.smem_len; > - > - if (pos > total_size) > - return -EFBIG; > - if (count > total_size) { > - err = -EFBIG; > - count = total_size; > - } > - if (total_size - count < pos) { > - if (!err) > - err = -ENOSPC; > - count = total_size - pos; > - } > - > - if (info->fbops->fb_sync) > - info->fbops->fb_sync(info); > - > - /* > - * Copy to framebuffer even if we already logged an error. Emulates > - * the behavior of the original fbdev implementation. > - */ > - ret = write_screen(info, buf, count, pos); > - if (ret < 0) > - return ret; /* return last error, if any */ > - else if (!ret) > - return err; /* return previous error, if any */ > - > - *ppos += ret; > - > - return ret; > -} > - > -static ssize_t drm_fb_helper_read_screen_buffer(struct fb_info *info, char __user *buf, > - size_t count, loff_t pos) > -{ > - const char *src = info->screen_buffer + pos; > - > - if (copy_to_user(buf, src, count)) > - return -EFAULT; > - > - return count; > -} > - > /** > * drm_fb_helper_sys_read - Implements struct &fb_ops.fb_read for system memory > * @info: fb_info struct pointer > @@ -816,21 +727,10 @@ static ssize_t drm_fb_helper_read_screen_buffer(struct fb_info *info, char __use > ssize_t drm_fb_helper_sys_read(struct fb_info *info, char __user *buf, > size_t count, loff_t *ppos) > { > - return __drm_fb_helper_read(info, buf, count, ppos, drm_fb_helper_read_screen_buffer); > + return fb_sys_read(info, buf, count, ppos); > } > EXPORT_SYMBOL(drm_fb_helper_sys_read); > > -static ssize_t drm_fb_helper_write_screen_buffer(struct fb_info *info, const char __user *buf, > - size_t count, loff_t pos) > -{ > - char *dst = info->screen_buffer + pos; > - > - if (copy_from_user(dst, buf, count)) > - return -EFAULT; > - > - return count; > -} > - > /** > * drm_fb_helper_sys_write - Implements struct &fb_ops.fb_write for system memory > * @info: fb_info struct pointer > @@ -849,7 +749,7 @@ ssize_t drm_fb_helper_sys_write(struct fb_info *info, const char __user *buf, > ssize_t ret; > struct drm_rect damage_area; > > - ret = __drm_fb_helper_write(info, buf, count, ppos, drm_fb_helper_write_screen_buffer); > + ret = fb_sys_write(info, buf, count, ppos); > if (ret <= 0) > return ret; > > @@ -921,39 +821,6 @@ void drm_fb_helper_sys_imageblit(struct fb_info *info, > } > EXPORT_SYMBOL(drm_fb_helper_sys_imageblit); > > -static ssize_t fb_read_screen_base(struct fb_info *info, char __user *buf, size_t count, > - loff_t pos) > -{ > - const char __iomem *src = info->screen_base + pos; > - size_t alloc_size = min_t(size_t, count, PAGE_SIZE); > - ssize_t ret = 0; > - int err = 0; > - char *tmp; > - > - tmp = kmalloc(alloc_size, GFP_KERNEL); > - if (!tmp) > - return -ENOMEM; > - > - while (count) { > - size_t c = min_t(size_t, count, alloc_size); > - > - memcpy_fromio(tmp, src, c); > - if (copy_to_user(buf, tmp, c)) { > - err = -EFAULT; > - break; > - } > - > - src += c; > - buf += c; > - ret += c; > - count -= c; > - } > - > - kfree(tmp); > - > - return ret ? ret : err; > -} > - > /** > * drm_fb_helper_cfb_read - Implements struct &fb_ops.fb_read for I/O memory > * @info: fb_info struct pointer > @@ -967,43 +834,10 @@ static ssize_t fb_read_screen_base(struct fb_info *info, char __user *buf, size_ > ssize_t drm_fb_helper_cfb_read(struct fb_info *info, char __user *buf, > size_t count, loff_t *ppos) > { > - return __drm_fb_helper_read(info, buf, count, ppos, fb_read_screen_base); > + return fb_cfb_read(info, buf, count, ppos); > } > EXPORT_SYMBOL(drm_fb_helper_cfb_read); > > -static ssize_t fb_write_screen_base(struct fb_info *info, const char __user *buf, size_t count, > - loff_t pos) > -{ > - char __iomem *dst = info->screen_base + pos; > - size_t alloc_size = min_t(size_t, count, PAGE_SIZE); > - ssize_t ret = 0; > - int err = 0; > - u8 *tmp; > - > - tmp = kmalloc(alloc_size, GFP_KERNEL); > - if (!tmp) > - return -ENOMEM; > - > - while (count) { > - size_t c = min_t(size_t, count, alloc_size); > - > - if (copy_from_user(tmp, buf, c)) { > - err = -EFAULT; > - break; > - } > - memcpy_toio(dst, tmp, c); > - > - dst += c; > - buf += c; > - ret += c; > - count -= c; > - } > - > - kfree(tmp); > - > - return ret ? ret : err; > -} > - > /** > * drm_fb_helper_cfb_write - Implements struct &fb_ops.fb_write for I/O memory > * @info: fb_info struct pointer > @@ -1022,7 +856,7 @@ ssize_t drm_fb_helper_cfb_write(struct fb_info *info, const char __user *buf, > ssize_t ret; > struct drm_rect damage_area; > > - ret = __drm_fb_helper_write(info, buf, count, ppos, fb_write_screen_base); > + ret = fb_cfb_write(info, buf, count, ppos); > if (ret <= 0) > return ret; >
Hi Thomas, On Tue, Apr 25, 2023 at 4:28 PM Thomas Zimmermann <tzimmermann@suse.de> wrote: > Implement DRM fbdev helpers for reading and writing framebuffer > memory with the respective fbdev functions. Removes duplicate > code. > > Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> Thanks for your patch! > --- a/drivers/gpu/drm/drm_fb_helper.c > +++ b/drivers/gpu/drm/drm_fb_helper.c > @@ -816,21 +727,10 @@ static ssize_t drm_fb_helper_read_screen_buffer(struct fb_info *info, char __use > ssize_t drm_fb_helper_sys_read(struct fb_info *info, char __user *buf, > size_t count, loff_t *ppos) > { > - return __drm_fb_helper_read(info, buf, count, ppos, drm_fb_helper_read_screen_buffer); > + return fb_sys_read(info, buf, count, ppos); > } > EXPORT_SYMBOL(drm_fb_helper_sys_read); I guess drm_fb_helper_sys_read() can just be removed? > @@ -849,7 +749,7 @@ ssize_t drm_fb_helper_sys_write(struct fb_info *info, const char __user *buf, > ssize_t ret; > struct drm_rect damage_area; > > - ret = __drm_fb_helper_write(info, buf, count, ppos, drm_fb_helper_write_screen_buffer); > + ret = fb_sys_write(info, buf, count, ppos); > if (ret <= 0) > return ret; > drm_fb_helper_sys_write() cannot be removed yet, because it does damage handling below. If the fb_ops.fb_sync() callback would be enhanced to pass a region, drm_fb_helper could implement .fb_sync() instead of .fb_write(). Likewise for the "cfb" (which is a misnomer) variants below. Gr{oetje,eeting}s, Geert
Hi Am 26.04.23 um 17:15 schrieb Geert Uytterhoeven: > Hi Thomas, > > On Tue, Apr 25, 2023 at 4:28 PM Thomas Zimmermann <tzimmermann@suse.de> wrote: >> Implement DRM fbdev helpers for reading and writing framebuffer >> memory with the respective fbdev functions. Removes duplicate >> code. >> >> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> > > Thanks for your patch! > >> --- a/drivers/gpu/drm/drm_fb_helper.c >> +++ b/drivers/gpu/drm/drm_fb_helper.c > >> @@ -816,21 +727,10 @@ static ssize_t drm_fb_helper_read_screen_buffer(struct fb_info *info, char __use >> ssize_t drm_fb_helper_sys_read(struct fb_info *info, char __user *buf, >> size_t count, loff_t *ppos) >> { >> - return __drm_fb_helper_read(info, buf, count, ppos, drm_fb_helper_read_screen_buffer); >> + return fb_sys_read(info, buf, count, ppos); >> } >> EXPORT_SYMBOL(drm_fb_helper_sys_read); > > I guess drm_fb_helper_sys_read() can just be removed? Yes, soon. > >> @@ -849,7 +749,7 @@ ssize_t drm_fb_helper_sys_write(struct fb_info *info, const char __user *buf, >> ssize_t ret; >> struct drm_rect damage_area; >> >> - ret = __drm_fb_helper_write(info, buf, count, ppos, drm_fb_helper_write_screen_buffer); >> + ret = fb_sys_write(info, buf, count, ppos); >> if (ret <= 0) >> return ret; >> > > drm_fb_helper_sys_write() cannot be removed yet, because it does damage > handling below. If the fb_ops.fb_sync() callback would be enhanced to pass > a region, drm_fb_helper could implement .fb_sync() instead of .fb_write(). Most DRM's fbdev support can soon use regular fbdev helpers from this patchset. Only DRM's i915 and the generic fbdev need damage handling. But they are both special in their own way, so each will get its own implementation. I have prototype patches to make this happen. Best regards Thomas > > Likewise for the "cfb" (which is a misnomer) variants below. > > Gr{oetje,eeting}s, > > Geert >
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c index 6bb1b8b27d7a..e11858470fa7 100644 --- a/drivers/gpu/drm/drm_fb_helper.c +++ b/drivers/gpu/drm/drm_fb_helper.c @@ -714,95 +714,6 @@ void drm_fb_helper_deferred_io(struct fb_info *info, struct list_head *pagerefli } EXPORT_SYMBOL(drm_fb_helper_deferred_io); -typedef ssize_t (*drm_fb_helper_read_screen)(struct fb_info *info, char __user *buf, - size_t count, loff_t pos); - -static ssize_t __drm_fb_helper_read(struct fb_info *info, char __user *buf, size_t count, - loff_t *ppos, drm_fb_helper_read_screen read_screen) -{ - loff_t pos = *ppos; - size_t total_size; - ssize_t ret; - - if (info->screen_size) - total_size = info->screen_size; - else - total_size = info->fix.smem_len; - - if (pos >= total_size) - return 0; - if (count >= total_size) - count = total_size; - if (total_size - count < pos) - count = total_size - pos; - - if (info->fbops->fb_sync) - info->fbops->fb_sync(info); - - ret = read_screen(info, buf, count, pos); - if (ret > 0) - *ppos += ret; - - return ret; -} - -typedef ssize_t (*drm_fb_helper_write_screen)(struct fb_info *info, const char __user *buf, - size_t count, loff_t pos); - -static ssize_t __drm_fb_helper_write(struct fb_info *info, const char __user *buf, size_t count, - loff_t *ppos, drm_fb_helper_write_screen write_screen) -{ - loff_t pos = *ppos; - size_t total_size; - ssize_t ret; - int err = 0; - - if (info->screen_size) - total_size = info->screen_size; - else - total_size = info->fix.smem_len; - - if (pos > total_size) - return -EFBIG; - if (count > total_size) { - err = -EFBIG; - count = total_size; - } - if (total_size - count < pos) { - if (!err) - err = -ENOSPC; - count = total_size - pos; - } - - if (info->fbops->fb_sync) - info->fbops->fb_sync(info); - - /* - * Copy to framebuffer even if we already logged an error. Emulates - * the behavior of the original fbdev implementation. - */ - ret = write_screen(info, buf, count, pos); - if (ret < 0) - return ret; /* return last error, if any */ - else if (!ret) - return err; /* return previous error, if any */ - - *ppos += ret; - - return ret; -} - -static ssize_t drm_fb_helper_read_screen_buffer(struct fb_info *info, char __user *buf, - size_t count, loff_t pos) -{ - const char *src = info->screen_buffer + pos; - - if (copy_to_user(buf, src, count)) - return -EFAULT; - - return count; -} - /** * drm_fb_helper_sys_read - Implements struct &fb_ops.fb_read for system memory * @info: fb_info struct pointer @@ -816,21 +727,10 @@ static ssize_t drm_fb_helper_read_screen_buffer(struct fb_info *info, char __use ssize_t drm_fb_helper_sys_read(struct fb_info *info, char __user *buf, size_t count, loff_t *ppos) { - return __drm_fb_helper_read(info, buf, count, ppos, drm_fb_helper_read_screen_buffer); + return fb_sys_read(info, buf, count, ppos); } EXPORT_SYMBOL(drm_fb_helper_sys_read); -static ssize_t drm_fb_helper_write_screen_buffer(struct fb_info *info, const char __user *buf, - size_t count, loff_t pos) -{ - char *dst = info->screen_buffer + pos; - - if (copy_from_user(dst, buf, count)) - return -EFAULT; - - return count; -} - /** * drm_fb_helper_sys_write - Implements struct &fb_ops.fb_write for system memory * @info: fb_info struct pointer @@ -849,7 +749,7 @@ ssize_t drm_fb_helper_sys_write(struct fb_info *info, const char __user *buf, ssize_t ret; struct drm_rect damage_area; - ret = __drm_fb_helper_write(info, buf, count, ppos, drm_fb_helper_write_screen_buffer); + ret = fb_sys_write(info, buf, count, ppos); if (ret <= 0) return ret; @@ -921,39 +821,6 @@ void drm_fb_helper_sys_imageblit(struct fb_info *info, } EXPORT_SYMBOL(drm_fb_helper_sys_imageblit); -static ssize_t fb_read_screen_base(struct fb_info *info, char __user *buf, size_t count, - loff_t pos) -{ - const char __iomem *src = info->screen_base + pos; - size_t alloc_size = min_t(size_t, count, PAGE_SIZE); - ssize_t ret = 0; - int err = 0; - char *tmp; - - tmp = kmalloc(alloc_size, GFP_KERNEL); - if (!tmp) - return -ENOMEM; - - while (count) { - size_t c = min_t(size_t, count, alloc_size); - - memcpy_fromio(tmp, src, c); - if (copy_to_user(buf, tmp, c)) { - err = -EFAULT; - break; - } - - src += c; - buf += c; - ret += c; - count -= c; - } - - kfree(tmp); - - return ret ? ret : err; -} - /** * drm_fb_helper_cfb_read - Implements struct &fb_ops.fb_read for I/O memory * @info: fb_info struct pointer @@ -967,43 +834,10 @@ static ssize_t fb_read_screen_base(struct fb_info *info, char __user *buf, size_ ssize_t drm_fb_helper_cfb_read(struct fb_info *info, char __user *buf, size_t count, loff_t *ppos) { - return __drm_fb_helper_read(info, buf, count, ppos, fb_read_screen_base); + return fb_cfb_read(info, buf, count, ppos); } EXPORT_SYMBOL(drm_fb_helper_cfb_read); -static ssize_t fb_write_screen_base(struct fb_info *info, const char __user *buf, size_t count, - loff_t pos) -{ - char __iomem *dst = info->screen_base + pos; - size_t alloc_size = min_t(size_t, count, PAGE_SIZE); - ssize_t ret = 0; - int err = 0; - u8 *tmp; - - tmp = kmalloc(alloc_size, GFP_KERNEL); - if (!tmp) - return -ENOMEM; - - while (count) { - size_t c = min_t(size_t, count, alloc_size); - - if (copy_from_user(tmp, buf, c)) { - err = -EFAULT; - break; - } - memcpy_toio(dst, tmp, c); - - dst += c; - buf += c; - ret += c; - count -= c; - } - - kfree(tmp); - - return ret ? ret : err; -} - /** * drm_fb_helper_cfb_write - Implements struct &fb_ops.fb_write for I/O memory * @info: fb_info struct pointer @@ -1022,7 +856,7 @@ ssize_t drm_fb_helper_cfb_write(struct fb_info *info, const char __user *buf, ssize_t ret; struct drm_rect damage_area; - ret = __drm_fb_helper_write(info, buf, count, ppos, fb_write_screen_base); + ret = fb_cfb_write(info, buf, count, ppos); if (ret <= 0) return ret;
Implement DRM fbdev helpers for reading and writing framebuffer memory with the respective fbdev functions. Removes duplicate code. Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> --- drivers/gpu/drm/drm_fb_helper.c | 174 +------------------------------- 1 file changed, 4 insertions(+), 170 deletions(-)