| Message ID | 20230123100559.12351-11-tzimmermann@suse.de (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | drm/fb-helper: Various cleanups | expand |
Hi Thomas, a quick drive-by comment. On Mon, Jan 23, 2023 at 11:05:59AM +0100, Thomas Zimmermann wrote: > The generic fbdev emulation names variables of type struct fb_info > both 'fbi' and 'info'. The latter seems to be more common in fbdev > code, so name fbi accordingly. > > Also replace the duplicate variable in drm_fbdev_fb_destroy(). > > Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> > --- > drivers/gpu/drm/drm_fbdev_generic.c | 49 ++++++++++++++--------------- > 1 file changed, 24 insertions(+), 25 deletions(-) > > diff --git a/drivers/gpu/drm/drm_fbdev_generic.c b/drivers/gpu/drm/drm_fbdev_generic.c > index 49a0bba86ce7..7633da5c13c3 100644 > --- a/drivers/gpu/drm/drm_fbdev_generic.c > +++ b/drivers/gpu/drm/drm_fbdev_generic.c > @@ -46,17 +46,16 @@ static int drm_fbdev_fb_release(struct fb_info *info, int user) > static void drm_fbdev_fb_destroy(struct fb_info *info) > { > struct drm_fb_helper *fb_helper = info->par; > - struct fb_info *fbi = fb_helper->info; > void *shadow = NULL; > > if (!fb_helper->dev) > return; > > - if (fbi) { > - if (fbi->fbdefio) > - fb_deferred_io_cleanup(fbi); > + if (info) { As info is already used above to find fb_helper, this check is redundant. Sam > + if (info->fbdefio) > + fb_deferred_io_cleanup(info); > if (drm_fbdev_use_shadow_fb(fb_helper)) > - shadow = fbi->screen_buffer; > + shadow = info->screen_buffer; > } > > drm_fb_helper_fini(fb_helper); > @@ -173,7 +172,7 @@ static int drm_fbdev_fb_probe(struct drm_fb_helper *fb_helper, > struct drm_device *dev = fb_helper->dev; > struct drm_client_buffer *buffer; > struct drm_framebuffer *fb; > - struct fb_info *fbi; > + struct fb_info *info; > u32 format; > struct iosys_map map; > int ret; > @@ -192,35 +191,35 @@ static int drm_fbdev_fb_probe(struct drm_fb_helper *fb_helper, > fb_helper->fb = buffer->fb; > fb = buffer->fb; > > - fbi = drm_fb_helper_alloc_info(fb_helper); > - if (IS_ERR(fbi)) > - return PTR_ERR(fbi); > + info = drm_fb_helper_alloc_info(fb_helper); > + if (IS_ERR(info)) > + return PTR_ERR(info); > > - fbi->fbops = &drm_fbdev_fb_ops; > - fbi->screen_size = sizes->surface_height * fb->pitches[0]; > - fbi->fix.smem_len = fbi->screen_size; > - fbi->flags = FBINFO_DEFAULT; > + info->fbops = &drm_fbdev_fb_ops; > + info->screen_size = sizes->surface_height * fb->pitches[0]; > + info->fix.smem_len = info->screen_size; > + info->flags = FBINFO_DEFAULT; > > - drm_fb_helper_fill_info(fbi, fb_helper, sizes); > + drm_fb_helper_fill_info(info, fb_helper, sizes); > > if (drm_fbdev_use_shadow_fb(fb_helper)) { > - fbi->screen_buffer = vzalloc(fbi->screen_size); > - if (!fbi->screen_buffer) > + info->screen_buffer = vzalloc(info->screen_size); > + if (!info->screen_buffer) > return -ENOMEM; > - fbi->flags |= FBINFO_VIRTFB | FBINFO_READS_FAST; > + info->flags |= FBINFO_VIRTFB | FBINFO_READS_FAST; > > - fbi->fbdefio = &drm_fbdev_defio; > - fb_deferred_io_init(fbi); > + info->fbdefio = &drm_fbdev_defio; > + fb_deferred_io_init(info); > } else { > /* buffer is mapped for HW framebuffer */ > ret = drm_client_buffer_vmap(fb_helper->buffer, &map); > if (ret) > return ret; > if (map.is_iomem) { > - fbi->screen_base = map.vaddr_iomem; > + info->screen_base = map.vaddr_iomem; > } else { > - fbi->screen_buffer = map.vaddr; > - fbi->flags |= FBINFO_VIRTFB; > + info->screen_buffer = map.vaddr; > + info->flags |= FBINFO_VIRTFB; > } > > /* > @@ -229,10 +228,10 @@ static int drm_fbdev_fb_probe(struct drm_fb_helper *fb_helper, > * case. > */ > #if IS_ENABLED(CONFIG_DRM_FBDEV_LEAK_PHYS_SMEM) > - if (fb_helper->hint_leak_smem_start && fbi->fix.smem_start == 0 && > + if (fb_helper->hint_leak_smem_start && info->fix.smem_start == 0 && > !drm_WARN_ON_ONCE(dev, map.is_iomem)) > - fbi->fix.smem_start = > - page_to_phys(virt_to_page(fbi->screen_buffer)); > + info->fix.smem_start = > + page_to_phys(virt_to_page(info->screen_buffer)); > #endif > } > > -- > 2.39.0
Hi Am 23.01.23 um 21:50 schrieb Sam Ravnborg: > Hi Thomas, > > a quick drive-by comment. > > On Mon, Jan 23, 2023 at 11:05:59AM +0100, Thomas Zimmermann wrote: >> The generic fbdev emulation names variables of type struct fb_info >> both 'fbi' and 'info'. The latter seems to be more common in fbdev >> code, so name fbi accordingly. >> >> Also replace the duplicate variable in drm_fbdev_fb_destroy(). >> >> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> >> --- >> drivers/gpu/drm/drm_fbdev_generic.c | 49 ++++++++++++++--------------- >> 1 file changed, 24 insertions(+), 25 deletions(-) >> >> diff --git a/drivers/gpu/drm/drm_fbdev_generic.c b/drivers/gpu/drm/drm_fbdev_generic.c >> index 49a0bba86ce7..7633da5c13c3 100644 >> --- a/drivers/gpu/drm/drm_fbdev_generic.c >> +++ b/drivers/gpu/drm/drm_fbdev_generic.c >> @@ -46,17 +46,16 @@ static int drm_fbdev_fb_release(struct fb_info *info, int user) >> static void drm_fbdev_fb_destroy(struct fb_info *info) >> { >> struct drm_fb_helper *fb_helper = info->par; >> - struct fb_info *fbi = fb_helper->info; >> void *shadow = NULL; >> >> if (!fb_helper->dev) >> return; >> >> - if (fbi) { >> - if (fbi->fbdefio) >> - fb_deferred_io_cleanup(fbi); >> + if (info) { > As info is already used above to find fb_helper, this check is > redundant. Oh indeed; will fix. This change belongs to patch 8, which streamlines the cleanup a bit. Best regards Thomas > > Sam > >> + if (info->fbdefio) >> + fb_deferred_io_cleanup(info); >> if (drm_fbdev_use_shadow_fb(fb_helper)) >> - shadow = fbi->screen_buffer; >> + shadow = info->screen_buffer; >> } >> >> drm_fb_helper_fini(fb_helper); >> @@ -173,7 +172,7 @@ static int drm_fbdev_fb_probe(struct drm_fb_helper *fb_helper, >> struct drm_device *dev = fb_helper->dev; >> struct drm_client_buffer *buffer; >> struct drm_framebuffer *fb; >> - struct fb_info *fbi; >> + struct fb_info *info; >> u32 format; >> struct iosys_map map; >> int ret; >> @@ -192,35 +191,35 @@ static int drm_fbdev_fb_probe(struct drm_fb_helper *fb_helper, >> fb_helper->fb = buffer->fb; >> fb = buffer->fb; >> >> - fbi = drm_fb_helper_alloc_info(fb_helper); >> - if (IS_ERR(fbi)) >> - return PTR_ERR(fbi); >> + info = drm_fb_helper_alloc_info(fb_helper); >> + if (IS_ERR(info)) >> + return PTR_ERR(info); >> >> - fbi->fbops = &drm_fbdev_fb_ops; >> - fbi->screen_size = sizes->surface_height * fb->pitches[0]; >> - fbi->fix.smem_len = fbi->screen_size; >> - fbi->flags = FBINFO_DEFAULT; >> + info->fbops = &drm_fbdev_fb_ops; >> + info->screen_size = sizes->surface_height * fb->pitches[0]; >> + info->fix.smem_len = info->screen_size; >> + info->flags = FBINFO_DEFAULT; >> >> - drm_fb_helper_fill_info(fbi, fb_helper, sizes); >> + drm_fb_helper_fill_info(info, fb_helper, sizes); >> >> if (drm_fbdev_use_shadow_fb(fb_helper)) { >> - fbi->screen_buffer = vzalloc(fbi->screen_size); >> - if (!fbi->screen_buffer) >> + info->screen_buffer = vzalloc(info->screen_size); >> + if (!info->screen_buffer) >> return -ENOMEM; >> - fbi->flags |= FBINFO_VIRTFB | FBINFO_READS_FAST; >> + info->flags |= FBINFO_VIRTFB | FBINFO_READS_FAST; >> >> - fbi->fbdefio = &drm_fbdev_defio; >> - fb_deferred_io_init(fbi); >> + info->fbdefio = &drm_fbdev_defio; >> + fb_deferred_io_init(info); >> } else { >> /* buffer is mapped for HW framebuffer */ >> ret = drm_client_buffer_vmap(fb_helper->buffer, &map); >> if (ret) >> return ret; >> if (map.is_iomem) { >> - fbi->screen_base = map.vaddr_iomem; >> + info->screen_base = map.vaddr_iomem; >> } else { >> - fbi->screen_buffer = map.vaddr; >> - fbi->flags |= FBINFO_VIRTFB; >> + info->screen_buffer = map.vaddr; >> + info->flags |= FBINFO_VIRTFB; >> } >> >> /* >> @@ -229,10 +228,10 @@ static int drm_fbdev_fb_probe(struct drm_fb_helper *fb_helper, >> * case. >> */ >> #if IS_ENABLED(CONFIG_DRM_FBDEV_LEAK_PHYS_SMEM) >> - if (fb_helper->hint_leak_smem_start && fbi->fix.smem_start == 0 && >> + if (fb_helper->hint_leak_smem_start && info->fix.smem_start == 0 && >> !drm_WARN_ON_ONCE(dev, map.is_iomem)) >> - fbi->fix.smem_start = >> - page_to_phys(virt_to_page(fbi->screen_buffer)); >> + info->fix.smem_start = >> + page_to_phys(virt_to_page(info->screen_buffer)); >> #endif >> } >> >> -- >> 2.39.0
diff --git a/drivers/gpu/drm/drm_fbdev_generic.c b/drivers/gpu/drm/drm_fbdev_generic.c index 49a0bba86ce7..7633da5c13c3 100644 --- a/drivers/gpu/drm/drm_fbdev_generic.c +++ b/drivers/gpu/drm/drm_fbdev_generic.c @@ -46,17 +46,16 @@ static int drm_fbdev_fb_release(struct fb_info *info, int user) static void drm_fbdev_fb_destroy(struct fb_info *info) { struct drm_fb_helper *fb_helper = info->par; - struct fb_info *fbi = fb_helper->info; void *shadow = NULL; if (!fb_helper->dev) return; - if (fbi) { - if (fbi->fbdefio) - fb_deferred_io_cleanup(fbi); + if (info) { + if (info->fbdefio) + fb_deferred_io_cleanup(info); if (drm_fbdev_use_shadow_fb(fb_helper)) - shadow = fbi->screen_buffer; + shadow = info->screen_buffer; } drm_fb_helper_fini(fb_helper); @@ -173,7 +172,7 @@ static int drm_fbdev_fb_probe(struct drm_fb_helper *fb_helper, struct drm_device *dev = fb_helper->dev; struct drm_client_buffer *buffer; struct drm_framebuffer *fb; - struct fb_info *fbi; + struct fb_info *info; u32 format; struct iosys_map map; int ret; @@ -192,35 +191,35 @@ static int drm_fbdev_fb_probe(struct drm_fb_helper *fb_helper, fb_helper->fb = buffer->fb; fb = buffer->fb; - fbi = drm_fb_helper_alloc_info(fb_helper); - if (IS_ERR(fbi)) - return PTR_ERR(fbi); + info = drm_fb_helper_alloc_info(fb_helper); + if (IS_ERR(info)) + return PTR_ERR(info); - fbi->fbops = &drm_fbdev_fb_ops; - fbi->screen_size = sizes->surface_height * fb->pitches[0]; - fbi->fix.smem_len = fbi->screen_size; - fbi->flags = FBINFO_DEFAULT; + info->fbops = &drm_fbdev_fb_ops; + info->screen_size = sizes->surface_height * fb->pitches[0]; + info->fix.smem_len = info->screen_size; + info->flags = FBINFO_DEFAULT; - drm_fb_helper_fill_info(fbi, fb_helper, sizes); + drm_fb_helper_fill_info(info, fb_helper, sizes); if (drm_fbdev_use_shadow_fb(fb_helper)) { - fbi->screen_buffer = vzalloc(fbi->screen_size); - if (!fbi->screen_buffer) + info->screen_buffer = vzalloc(info->screen_size); + if (!info->screen_buffer) return -ENOMEM; - fbi->flags |= FBINFO_VIRTFB | FBINFO_READS_FAST; + info->flags |= FBINFO_VIRTFB | FBINFO_READS_FAST; - fbi->fbdefio = &drm_fbdev_defio; - fb_deferred_io_init(fbi); + info->fbdefio = &drm_fbdev_defio; + fb_deferred_io_init(info); } else { /* buffer is mapped for HW framebuffer */ ret = drm_client_buffer_vmap(fb_helper->buffer, &map); if (ret) return ret; if (map.is_iomem) { - fbi->screen_base = map.vaddr_iomem; + info->screen_base = map.vaddr_iomem; } else { - fbi->screen_buffer = map.vaddr; - fbi->flags |= FBINFO_VIRTFB; + info->screen_buffer = map.vaddr; + info->flags |= FBINFO_VIRTFB; } /* @@ -229,10 +228,10 @@ static int drm_fbdev_fb_probe(struct drm_fb_helper *fb_helper, * case. */ #if IS_ENABLED(CONFIG_DRM_FBDEV_LEAK_PHYS_SMEM) - if (fb_helper->hint_leak_smem_start && fbi->fix.smem_start == 0 && + if (fb_helper->hint_leak_smem_start && info->fix.smem_start == 0 && !drm_WARN_ON_ONCE(dev, map.is_iomem)) - fbi->fix.smem_start = - page_to_phys(virt_to_page(fbi->screen_buffer)); + info->fix.smem_start = + page_to_phys(virt_to_page(info->screen_buffer)); #endif }
The generic fbdev emulation names variables of type struct fb_info both 'fbi' and 'info'. The latter seems to be more common in fbdev code, so name fbi accordingly. Also replace the duplicate variable in drm_fbdev_fb_destroy(). Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> --- drivers/gpu/drm/drm_fbdev_generic.c | 49 ++++++++++++++--------------- 1 file changed, 24 insertions(+), 25 deletions(-)