| Message ID | 20230425142846.730-3-tzimmermann@suse.de (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | drm,fbdev: Use fbdev's I/O helpers | expand |
Thomas Zimmermann <tzimmermann@suse.de> writes: > Use info->screen_buffer when reading and writing framebuffers in > system memory. It's the correct pointer for this address space. > Maybe can expand the explanation a little bit with something like this? "The struct fb_info has a union to store the framebuffer memory. This can either be info->screen_base if the framebuffer is stored in I/O memory, or info->screen_buffer if the framebuffer is stored in system memory. Since the fb_sys_{read,write}() functions operate on the latter address space, it is wrong to use .screen_base and .screen_buffer must be used instead. This also get rids of all the casting needed due not using the correct data type." > Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> > --- Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
Tested-by: Sui Jingfeng <suijingfeng@loongson.cn> On 2023/4/25 22:28, Thomas Zimmermann wrote: > Use info->screen_buffer when reading and writing framebuffers in > system memory. It's the correct pointer for this address space. > > Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> > Reviewed-by: Javier Martinez Canillas <javierm@redhat.com> > --- > drivers/video/fbdev/core/fb_sys_fops.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/video/fbdev/core/fb_sys_fops.c b/drivers/video/fbdev/core/fb_sys_fops.c > index cefb77b9546d..6589123f4127 100644 > --- a/drivers/video/fbdev/core/fb_sys_fops.c > +++ b/drivers/video/fbdev/core/fb_sys_fops.c > @@ -39,7 +39,7 @@ ssize_t fb_sys_read(struct fb_info *info, char __user *buf, size_t count, > if (count + p > total_size) > count = total_size - p; > > - src = (void __force *)(info->screen_base + p); > + src = info->screen_buffer + p; > > if (info->fbops->fb_sync) > info->fbops->fb_sync(info); > @@ -87,7 +87,7 @@ ssize_t fb_sys_write(struct fb_info *info, const char __user *buf, > count = total_size - p; > } > > - dst = (void __force *) (info->screen_base + p); > + dst = info->screen_buffer + p; > > if (info->fbops->fb_sync) > info->fbops->fb_sync(info);
Hi Am 25.04.23 um 18:35 schrieb Javier Martinez Canillas: > Thomas Zimmermann <tzimmermann@suse.de> writes: > >> Use info->screen_buffer when reading and writing framebuffers in >> system memory. It's the correct pointer for this address space. >> > > Maybe can expand the explanation a little bit with something like this? > > "The struct fb_info has a union to store the framebuffer memory. This can > either be info->screen_base if the framebuffer is stored in I/O memory, > or info->screen_buffer if the framebuffer is stored in system memory. > > Since the fb_sys_{read,write}() functions operate on the latter address > space, it is wrong to use .screen_base and .screen_buffer must be used > instead. This also get rids of all the casting needed due not using the > correct data type." Thanks. I'll add this text as-is in the next iteration. Best regards Thomas > >> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> >> --- > > Reviewed-by: Javier Martinez Canillas <javierm@redhat.com> >
On Tue, Apr 25, 2023 at 6:36 PM Javier Martinez Canillas <javierm@redhat.com> wrote: > Thomas Zimmermann <tzimmermann@suse.de> writes: > > > Use info->screen_buffer when reading and writing framebuffers in > > system memory. It's the correct pointer for this address space. > > > > Maybe can expand the explanation a little bit with something like this? > > "The struct fb_info has a union to store the framebuffer memory. This can > either be info->screen_base if the framebuffer is stored in I/O memory, > or info->screen_buffer if the framebuffer is stored in system memory. > > Since the fb_sys_{read,write}() functions operate on the latter address > space, it is wrong to use .screen_base and .screen_buffer must be used > instead. This also get rids of all the casting needed due not using the ... due to not ... > correct data type." +1 Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> Gr{oetje,eeting}s, Geert
diff --git a/drivers/video/fbdev/core/fb_sys_fops.c b/drivers/video/fbdev/core/fb_sys_fops.c index cefb77b9546d..6589123f4127 100644 --- a/drivers/video/fbdev/core/fb_sys_fops.c +++ b/drivers/video/fbdev/core/fb_sys_fops.c @@ -39,7 +39,7 @@ ssize_t fb_sys_read(struct fb_info *info, char __user *buf, size_t count, if (count + p > total_size) count = total_size - p; - src = (void __force *)(info->screen_base + p); + src = info->screen_buffer + p; if (info->fbops->fb_sync) info->fbops->fb_sync(info); @@ -87,7 +87,7 @@ ssize_t fb_sys_write(struct fb_info *info, const char __user *buf, count = total_size - p; } - dst = (void __force *) (info->screen_base + p); + dst = info->screen_buffer + p; if (info->fbops->fb_sync) info->fbops->fb_sync(info);
Use info->screen_buffer when reading and writing framebuffers in system memory. It's the correct pointer for this address space. Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> --- drivers/video/fbdev/core/fb_sys_fops.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)