From: Daniel Vetter <daniel.vetter@ffwll.ch> To: DRI Development <dri-devel@lists.freedesktop.org> Cc: Du Cheng <ducheng2@gmail.com>, Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>, Daniel Vetter <daniel.vetter@ffwll.ch>, Intel Graphics Development <intel-gfx@lists.freedesktop.org>, Claudio Suarez <cssk@net-c.es>, Greg Kroah-Hartman <gregkh@linuxfoundation.org>, Daniel Vetter <daniel.vetter@intel.com>, Sam Ravnborg <sam@ravnborg.org> Subject: [PATCH v3 09/17] fbcon: Extract fbcon_open/release helpers Date: Tue, 5 Apr 2022 23:03:27 +0200 [thread overview] Message-ID: <20220405210335.3434130-10-daniel.vetter@ffwll.ch> (raw) In-Reply-To: <20220405210335.3434130-1-daniel.vetter@ffwll.ch> There's two minor behaviour changes in here: - in error paths we now consistently call fb_ops->fb_release - fb_release really can't fail (fbmem.c ignores it too) and there's no reasonable cleanup we can do anyway. Note that everything in fbcon.c is protected by the big console_lock() lock (especially all the global variables), so the minor changes in ordering of setup/cleanup do not matter. v2: Explain a bit better why this is all correct (Sam) Acked-by: Sam Ravnborg <sam@ravnborg.org> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> Cc: Daniel Vetter <daniel@ffwll.ch> Cc: Claudio Suarez <cssk@net-c.es> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Cc: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> Cc: Du Cheng <ducheng2@gmail.com> --- drivers/video/fbdev/core/fbcon.c | 107 +++++++++++++++---------------- 1 file changed, 53 insertions(+), 54 deletions(-) diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c index 81860ca80838..d50154b62007 100644 --- a/drivers/video/fbdev/core/fbcon.c +++ b/drivers/video/fbdev/core/fbcon.c @@ -682,19 +682,37 @@ static int fbcon_invalid_charcount(struct fb_info *info, unsigned charcount) #endif /* CONFIG_MISC_TILEBLITTING */ +static int fbcon_open(struct fb_info *info) +{ + if (!try_module_get(info->fbops->owner)) + return -ENODEV; + + if (info->fbops->fb_open && + info->fbops->fb_open(info, 0)) { + module_put(info->fbops->owner); + return -ENODEV; + } + + return 0; +} + +static void fbcon_release(struct fb_info *info) +{ + if (info->fbops->fb_release) + info->fbops->fb_release(info, 0); + + module_put(info->fbops->owner); +} static int con2fb_acquire_newinfo(struct vc_data *vc, struct fb_info *info, int unit, int oldidx) { struct fbcon_ops *ops = NULL; - int err = 0; - - if (!try_module_get(info->fbops->owner)) - err = -ENODEV; + int err; - if (!err && info->fbops->fb_open && - info->fbops->fb_open(info, 0)) - err = -ENODEV; + err = fbcon_open(info); + if (err) + return err; if (!err) { ops = kzalloc(sizeof(struct fbcon_ops), GFP_KERNEL); @@ -715,7 +733,7 @@ static int con2fb_acquire_newinfo(struct vc_data *vc, struct fb_info *info, if (err) { con2fb_map[unit] = oldidx; - module_put(info->fbops->owner); + fbcon_release(info); } return err; @@ -726,45 +744,34 @@ static int con2fb_release_oldinfo(struct vc_data *vc, struct fb_info *oldinfo, int oldidx, int found) { struct fbcon_ops *ops = oldinfo->fbcon_par; - int err = 0, ret; + int ret; - if (oldinfo->fbops->fb_release && - oldinfo->fbops->fb_release(oldinfo, 0)) { - con2fb_map[unit] = oldidx; - if (!found && newinfo->fbops->fb_release) - newinfo->fbops->fb_release(newinfo, 0); - if (!found) - module_put(newinfo->fbops->owner); - err = -ENODEV; - } + fbcon_release(oldinfo); - if (!err) { - fbcon_del_cursor_work(oldinfo); - kfree(ops->cursor_state.mask); - kfree(ops->cursor_data); - kfree(ops->cursor_src); - kfree(ops->fontbuffer); - kfree(oldinfo->fbcon_par); - oldinfo->fbcon_par = NULL; - module_put(oldinfo->fbops->owner); - /* - If oldinfo and newinfo are driving the same hardware, - the fb_release() method of oldinfo may attempt to - restore the hardware state. This will leave the - newinfo in an undefined state. Thus, a call to - fb_set_par() may be needed for the newinfo. - */ - if (newinfo && newinfo->fbops->fb_set_par) { - ret = newinfo->fbops->fb_set_par(newinfo); + fbcon_del_cursor_work(oldinfo); + kfree(ops->cursor_state.mask); + kfree(ops->cursor_data); + kfree(ops->cursor_src); + kfree(ops->fontbuffer); + kfree(oldinfo->fbcon_par); + oldinfo->fbcon_par = NULL; + /* + If oldinfo and newinfo are driving the same hardware, + the fb_release() method of oldinfo may attempt to + restore the hardware state. This will leave the + newinfo in an undefined state. Thus, a call to + fb_set_par() may be needed for the newinfo. + */ + if (newinfo && newinfo->fbops->fb_set_par) { + ret = newinfo->fbops->fb_set_par(newinfo); - if (ret) - printk(KERN_ERR "con2fb_release_oldinfo: " - "detected unhandled fb_set_par error, " - "error code %d\n", ret); - } + if (ret) + printk(KERN_ERR "con2fb_release_oldinfo: " + "detected unhandled fb_set_par error, " + "error code %d\n", ret); } - return err; + return 0; } static void con2fb_init_display(struct vc_data *vc, struct fb_info *info, @@ -919,7 +926,6 @@ static const char *fbcon_startup(void) struct fbcon_display *p = &fb_display[fg_console]; struct vc_data *vc = vc_cons[fg_console].d; const struct font_desc *font = NULL; - struct module *owner; struct fb_info *info = NULL; struct fbcon_ops *ops; int rows, cols; @@ -938,17 +944,12 @@ static const char *fbcon_startup(void) if (!info) return NULL; - owner = info->fbops->owner; - if (!try_module_get(owner)) + if (fbcon_open(info)) return NULL; - if (info->fbops->fb_open && info->fbops->fb_open(info, 0)) { - module_put(owner); - return NULL; - } ops = kzalloc(sizeof(struct fbcon_ops), GFP_KERNEL); if (!ops) { - module_put(owner); + fbcon_release(info); return NULL; } @@ -3314,10 +3315,6 @@ static void fbcon_exit(void) } if (mapped) { - if (info->fbops->fb_release) - info->fbops->fb_release(info, 0); - module_put(info->fbops->owner); - if (info->fbcon_par) { struct fbcon_ops *ops = info->fbcon_par; @@ -3327,6 +3324,8 @@ static void fbcon_exit(void) kfree(info->fbcon_par); info->fbcon_par = NULL; } + + fbcon_release(info); } } } -- 2.34.1
WARNING: multiple messages have this Message-ID (diff)
From: Daniel Vetter <daniel.vetter@ffwll.ch> To: DRI Development <dri-devel@lists.freedesktop.org> Cc: Du Cheng <ducheng2@gmail.com>, Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>, Daniel Vetter <daniel.vetter@ffwll.ch>, Intel Graphics Development <intel-gfx@lists.freedesktop.org>, Greg Kroah-Hartman <gregkh@linuxfoundation.org>, Daniel Vetter <daniel.vetter@intel.com>, Sam Ravnborg <sam@ravnborg.org> Subject: [Intel-gfx] [PATCH v3 09/17] fbcon: Extract fbcon_open/release helpers Date: Tue, 5 Apr 2022 23:03:27 +0200 [thread overview] Message-ID: <20220405210335.3434130-10-daniel.vetter@ffwll.ch> (raw) In-Reply-To: <20220405210335.3434130-1-daniel.vetter@ffwll.ch> There's two minor behaviour changes in here: - in error paths we now consistently call fb_ops->fb_release - fb_release really can't fail (fbmem.c ignores it too) and there's no reasonable cleanup we can do anyway. Note that everything in fbcon.c is protected by the big console_lock() lock (especially all the global variables), so the minor changes in ordering of setup/cleanup do not matter. v2: Explain a bit better why this is all correct (Sam) Acked-by: Sam Ravnborg <sam@ravnborg.org> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> Cc: Daniel Vetter <daniel@ffwll.ch> Cc: Claudio Suarez <cssk@net-c.es> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Cc: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> Cc: Du Cheng <ducheng2@gmail.com> --- drivers/video/fbdev/core/fbcon.c | 107 +++++++++++++++---------------- 1 file changed, 53 insertions(+), 54 deletions(-) diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c index 81860ca80838..d50154b62007 100644 --- a/drivers/video/fbdev/core/fbcon.c +++ b/drivers/video/fbdev/core/fbcon.c @@ -682,19 +682,37 @@ static int fbcon_invalid_charcount(struct fb_info *info, unsigned charcount) #endif /* CONFIG_MISC_TILEBLITTING */ +static int fbcon_open(struct fb_info *info) +{ + if (!try_module_get(info->fbops->owner)) + return -ENODEV; + + if (info->fbops->fb_open && + info->fbops->fb_open(info, 0)) { + module_put(info->fbops->owner); + return -ENODEV; + } + + return 0; +} + +static void fbcon_release(struct fb_info *info) +{ + if (info->fbops->fb_release) + info->fbops->fb_release(info, 0); + + module_put(info->fbops->owner); +} static int con2fb_acquire_newinfo(struct vc_data *vc, struct fb_info *info, int unit, int oldidx) { struct fbcon_ops *ops = NULL; - int err = 0; - - if (!try_module_get(info->fbops->owner)) - err = -ENODEV; + int err; - if (!err && info->fbops->fb_open && - info->fbops->fb_open(info, 0)) - err = -ENODEV; + err = fbcon_open(info); + if (err) + return err; if (!err) { ops = kzalloc(sizeof(struct fbcon_ops), GFP_KERNEL); @@ -715,7 +733,7 @@ static int con2fb_acquire_newinfo(struct vc_data *vc, struct fb_info *info, if (err) { con2fb_map[unit] = oldidx; - module_put(info->fbops->owner); + fbcon_release(info); } return err; @@ -726,45 +744,34 @@ static int con2fb_release_oldinfo(struct vc_data *vc, struct fb_info *oldinfo, int oldidx, int found) { struct fbcon_ops *ops = oldinfo->fbcon_par; - int err = 0, ret; + int ret; - if (oldinfo->fbops->fb_release && - oldinfo->fbops->fb_release(oldinfo, 0)) { - con2fb_map[unit] = oldidx; - if (!found && newinfo->fbops->fb_release) - newinfo->fbops->fb_release(newinfo, 0); - if (!found) - module_put(newinfo->fbops->owner); - err = -ENODEV; - } + fbcon_release(oldinfo); - if (!err) { - fbcon_del_cursor_work(oldinfo); - kfree(ops->cursor_state.mask); - kfree(ops->cursor_data); - kfree(ops->cursor_src); - kfree(ops->fontbuffer); - kfree(oldinfo->fbcon_par); - oldinfo->fbcon_par = NULL; - module_put(oldinfo->fbops->owner); - /* - If oldinfo and newinfo are driving the same hardware, - the fb_release() method of oldinfo may attempt to - restore the hardware state. This will leave the - newinfo in an undefined state. Thus, a call to - fb_set_par() may be needed for the newinfo. - */ - if (newinfo && newinfo->fbops->fb_set_par) { - ret = newinfo->fbops->fb_set_par(newinfo); + fbcon_del_cursor_work(oldinfo); + kfree(ops->cursor_state.mask); + kfree(ops->cursor_data); + kfree(ops->cursor_src); + kfree(ops->fontbuffer); + kfree(oldinfo->fbcon_par); + oldinfo->fbcon_par = NULL; + /* + If oldinfo and newinfo are driving the same hardware, + the fb_release() method of oldinfo may attempt to + restore the hardware state. This will leave the + newinfo in an undefined state. Thus, a call to + fb_set_par() may be needed for the newinfo. + */ + if (newinfo && newinfo->fbops->fb_set_par) { + ret = newinfo->fbops->fb_set_par(newinfo); - if (ret) - printk(KERN_ERR "con2fb_release_oldinfo: " - "detected unhandled fb_set_par error, " - "error code %d\n", ret); - } + if (ret) + printk(KERN_ERR "con2fb_release_oldinfo: " + "detected unhandled fb_set_par error, " + "error code %d\n", ret); } - return err; + return 0; } static void con2fb_init_display(struct vc_data *vc, struct fb_info *info, @@ -919,7 +926,6 @@ static const char *fbcon_startup(void) struct fbcon_display *p = &fb_display[fg_console]; struct vc_data *vc = vc_cons[fg_console].d; const struct font_desc *font = NULL; - struct module *owner; struct fb_info *info = NULL; struct fbcon_ops *ops; int rows, cols; @@ -938,17 +944,12 @@ static const char *fbcon_startup(void) if (!info) return NULL; - owner = info->fbops->owner; - if (!try_module_get(owner)) + if (fbcon_open(info)) return NULL; - if (info->fbops->fb_open && info->fbops->fb_open(info, 0)) { - module_put(owner); - return NULL; - } ops = kzalloc(sizeof(struct fbcon_ops), GFP_KERNEL); if (!ops) { - module_put(owner); + fbcon_release(info); return NULL; } @@ -3314,10 +3315,6 @@ static void fbcon_exit(void) } if (mapped) { - if (info->fbops->fb_release) - info->fbops->fb_release(info, 0); - module_put(info->fbops->owner); - if (info->fbcon_par) { struct fbcon_ops *ops = info->fbcon_par; @@ -3327,6 +3324,8 @@ static void fbcon_exit(void) kfree(info->fbcon_par); info->fbcon_par = NULL; } + + fbcon_release(info); } } } -- 2.34.1
next prev parent reply other threads:[~2022-04-05 21:04 UTC|newest] Thread overview: 50+ messages / expand[flat|nested] mbox.gz Atom feed top 2022-04-05 21:03 [PATCH v3 00/17] fbcon cleanups Daniel Vetter 2022-04-05 21:03 ` [Intel-gfx] " Daniel Vetter 2022-04-05 21:03 ` [Intel-gfx] [PATCH v3 01/17] fbcon: delete a few unneeded forward decl Daniel Vetter 2022-04-05 21:03 ` Daniel Vetter 2022-04-05 23:36 ` Fabio Estevam 2022-04-05 23:36 ` [Intel-gfx] " Fabio Estevam 2022-04-05 21:03 ` [PATCH v3 02/17] fbcon: Move fbcon_bmove(_rec) functions Daniel Vetter 2022-04-05 21:03 ` [Intel-gfx] " Daniel Vetter 2022-04-05 21:03 ` [PATCH v3 03/17] fbcon: Introduce wrapper for console->fb_info lookup Daniel Vetter 2022-04-05 21:03 ` [Intel-gfx] " Daniel Vetter 2022-04-05 21:03 ` [PATCH v3 04/17] fbcon: delete delayed loading code Daniel Vetter 2022-04-05 21:03 ` [Intel-gfx] " Daniel Vetter 2022-04-05 21:03 ` [PATCH v3 05/17] fbdev/sysfs: Fix locking Daniel Vetter 2022-04-05 21:03 ` [Intel-gfx] " Daniel Vetter 2022-04-05 21:03 ` [PATCH v3 06/17] fbcon: Use delayed work for cursor Daniel Vetter 2022-04-05 21:03 ` [Intel-gfx] " Daniel Vetter 2022-04-05 21:03 ` [PATCH v3 07/17] fbcon: Replace FBCON_FLAGS_INIT with a boolean Daniel Vetter 2022-04-05 21:03 ` [Intel-gfx] " Daniel Vetter 2022-04-05 21:03 ` [PATCH v3 08/17] fb: Delete fb_info->queue Daniel Vetter 2022-04-05 21:03 ` Daniel Vetter 2022-04-05 21:03 ` [Intel-gfx] " Daniel Vetter 2022-04-05 21:03 ` Daniel Vetter [this message] 2022-04-05 21:03 ` [Intel-gfx] [PATCH v3 09/17] fbcon: Extract fbcon_open/release helpers Daniel Vetter 2022-04-05 21:03 ` [PATCH v3 10/17] fbcon: Ditch error handling for con2fb_release_oldinfo Daniel Vetter 2022-04-05 21:03 ` [Intel-gfx] " Daniel Vetter 2022-04-05 21:03 ` [PATCH v3 11/17] fbcon: move more common code into fb_open() Daniel Vetter 2022-04-05 21:03 ` [Intel-gfx] " Daniel Vetter 2022-04-05 21:03 ` [PATCH v3 12/17] fbcon: use lock_fb_info in fbcon_open/release Daniel Vetter 2022-04-05 21:03 ` [Intel-gfx] " Daniel Vetter 2022-04-05 21:03 ` [PATCH v3 13/17] fbcon: Consistently protect deferred_takeover with console_lock() Daniel Vetter 2022-04-05 21:03 ` [Intel-gfx] " Daniel Vetter 2022-04-05 21:03 ` [PATCH v3 14/17] fbcon: Move console_lock for register/unlink/unregister Daniel Vetter 2022-04-05 21:03 ` [Intel-gfx] " Daniel Vetter 2022-04-11 22:27 ` Nathan Chancellor 2022-04-11 22:27 ` [Intel-gfx] " Nathan Chancellor 2022-04-13 8:13 ` Daniel Vetter 2022-04-13 8:13 ` [Intel-gfx] " Daniel Vetter 2022-04-05 21:03 ` [PATCH v3 15/17] fbcon: Move more code into fbcon_release Daniel Vetter 2022-04-05 21:03 ` [Intel-gfx] " Daniel Vetter 2022-04-05 21:03 ` [PATCH v3 16/17] fbcon: untangle fbcon_exit Daniel Vetter 2022-04-05 21:03 ` [Intel-gfx] " Daniel Vetter 2022-04-05 21:03 ` [PATCH v3 17/17] fbcon: Maintain a private array of fb_info Daniel Vetter 2022-04-05 21:03 ` [Intel-gfx] " Daniel Vetter 2022-04-06 1:15 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for fbcon cleanups Patchwork 2022-04-06 1:17 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork 2022-04-06 1:21 ` [Intel-gfx] ✗ Fi.CI.DOCS: " Patchwork 2022-04-06 1:45 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork 2022-04-06 7:09 ` [Intel-gfx] ✗ Fi.CI.IGT: failure " Patchwork 2022-04-08 7:51 ` [PATCH v3 00/17] " Daniel Vetter 2022-04-08 7:51 ` [Intel-gfx] " Daniel Vetter
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20220405210335.3434130-10-daniel.vetter@ffwll.ch \ --to=daniel.vetter@ffwll.ch \ --cc=cssk@net-c.es \ --cc=daniel.vetter@intel.com \ --cc=dri-devel@lists.freedesktop.org \ --cc=ducheng2@gmail.com \ --cc=gregkh@linuxfoundation.org \ --cc=intel-gfx@lists.freedesktop.org \ --cc=penguin-kernel@I-love.SAKURA.ne.jp \ --cc=sam@ravnborg.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.