Re: [PATCH 06/21] fbcon: delete delayed loading code
Hi Daniel, On Tue, Feb 08, 2022 at 02:42:25PM +0100, Daniel Vetter wrote: > On Thu, Feb 03, 2022 at 09:45:29PM +0100, Sam Ravnborg wrote: > > Hi Daniel, > > > > On Mon, Jan 31, 2022 at 10:05:37PM +0100, Daniel Vetter wrote: > > > Before > > > > > > commit 6104c37094e729f3d4ce65797002112735d49cd1 > > > Author: Daniel Vetter > > > Date: Tue Aug 1 17:32:07 2017 +0200 > > > > > > fbcon: Make fbcon a built-time depency for fbdev > > > > > > it was possible to load fbcon and fbdev drivers in any order, which > > > means that fbcon init had to handle the case where fbdev drivers where > > > already registered. > > > > > > This is no longer possible, hence delete that code. > > > > > > Note that the exit case is a bit more complex and will be done in a > > > separate patch. > > > > > > Signed-off-by: Daniel Vetter > > > Cc: Helge Deller > > > Cc: Daniel Vetter > > > Cc: Claudio Suarez > > > Cc: Greg Kroah-Hartman > > > Cc: Tetsuo Handa > > > Cc: Du Cheng > > > --- > > > drivers/video/fbdev/core/fbcon.c | 13 + > > > 1 file changed, 1 insertion(+), 12 deletions(-) > > > > > > diff --git a/drivers/video/fbdev/core/fbcon.c > > > b/drivers/video/fbdev/core/fbcon.c > > > index 8f971de35885..814b648e8f09 100644 > > > --- a/drivers/video/fbdev/core/fbcon.c > > > +++ b/drivers/video/fbdev/core/fbcon.c > > > @@ -942,7 +942,7 @@ static const char *fbcon_startup(void) > > > return display_desc; > > > /* > > >* Instead of blindly using registered_fb[0], we use info_idx, set by > > > - * fb_console_init(); > > > + * fbcon_fb_registered(); > > >*/ > > This comment change looks like it does not belong in this patch. > > Also, the comment is wrong as info_idx is set in several places. > > Like set_con2fb_map(), fbcon_remap_all(), and more. > > Yeah I can split this out into a separate patch, but I spotted this wrong > comment as part of reviewing this code change here - essentially you have > to check how fb_info for fbcon are registered and fbcon init happens to > validate that deleting the below code is correct. > > Ok if I put this explainer into the commit message, or do you want me to > split this out fully? Keep it and add my a-b Sam
Re: [PATCH 06/21] fbcon: delete delayed loading code
On Thu, Feb 03, 2022 at 09:45:29PM +0100, Sam Ravnborg wrote:
> Hi Daniel,
>
> On Mon, Jan 31, 2022 at 10:05:37PM +0100, Daniel Vetter wrote:
> > Before
> >
> > commit 6104c37094e729f3d4ce65797002112735d49cd1
> > Author: Daniel Vetter
> > Date: Tue Aug 1 17:32:07 2017 +0200
> >
> > fbcon: Make fbcon a built-time depency for fbdev
> >
> > it was possible to load fbcon and fbdev drivers in any order, which
> > means that fbcon init had to handle the case where fbdev drivers where
> > already registered.
> >
> > This is no longer possible, hence delete that code.
> >
> > Note that the exit case is a bit more complex and will be done in a
> > separate patch.
> >
> > Signed-off-by: Daniel Vetter
> > Cc: Helge Deller
> > Cc: Daniel Vetter
> > Cc: Claudio Suarez
> > Cc: Greg Kroah-Hartman
> > Cc: Tetsuo Handa
> > Cc: Du Cheng
> > ---
> > drivers/video/fbdev/core/fbcon.c | 13 +
> > 1 file changed, 1 insertion(+), 12 deletions(-)
> >
> > diff --git a/drivers/video/fbdev/core/fbcon.c
> > b/drivers/video/fbdev/core/fbcon.c
> > index 8f971de35885..814b648e8f09 100644
> > --- a/drivers/video/fbdev/core/fbcon.c
> > +++ b/drivers/video/fbdev/core/fbcon.c
> > @@ -942,7 +942,7 @@ static const char *fbcon_startup(void)
> > return display_desc;
> > /*
> > * Instead of blindly using registered_fb[0], we use info_idx, set by
> > -* fb_console_init();
> > +* fbcon_fb_registered();
> > */
> This comment change looks like it does not belong in this patch.
> Also, the comment is wrong as info_idx is set in several places.
> Like set_con2fb_map(), fbcon_remap_all(), and more.
Yeah I can split this out into a separate patch, but I spotted this wrong
comment as part of reviewing this code change here - essentially you have
to check how fb_info for fbcon are registered and fbcon init happens to
validate that deleting the below code is correct.
Ok if I put this explainer into the commit message, or do you want me to
split this out fully?
-Daniel
>
> Though it is not set by fb_console_init - so partly OK.
>
> With the comment adjustment dropped.
> Acked-by: Sam Ravnborg
>
> at least the code deletion looked OK, I failed to follow all the logic.
> So would be good if someone else could ack it too.
>
> Sam
>
>
>
> > info = registered_fb[info_idx];
> > if (!info)
> > @@ -3316,17 +3316,6 @@ static void fbcon_start(void)
> > return;
> > }
> > #endif
> > -
> > - if (num_registered_fb) {
> > - int i;
> > -
> > - for_each_registered_fb(i) {
> > - info_idx = i;
> > - break;
> > - }
> > -
> > - do_fbcon_takeover(0);
> > - }
> > }
> >
> > static void fbcon_exit(void)
> > --
> > 2.33.0
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
Re: [PATCH 06/21] fbcon: delete delayed loading code
Hi Daniel,
On Mon, Jan 31, 2022 at 10:05:37PM +0100, Daniel Vetter wrote:
> Before
>
> commit 6104c37094e729f3d4ce65797002112735d49cd1
> Author: Daniel Vetter
> Date: Tue Aug 1 17:32:07 2017 +0200
>
> fbcon: Make fbcon a built-time depency for fbdev
>
> it was possible to load fbcon and fbdev drivers in any order, which
> means that fbcon init had to handle the case where fbdev drivers where
> already registered.
>
> This is no longer possible, hence delete that code.
>
> Note that the exit case is a bit more complex and will be done in a
> separate patch.
>
> Signed-off-by: Daniel Vetter
> Cc: Helge Deller
> Cc: Daniel Vetter
> Cc: Claudio Suarez
> Cc: Greg Kroah-Hartman
> Cc: Tetsuo Handa
> Cc: Du Cheng
> ---
> drivers/video/fbdev/core/fbcon.c | 13 +
> 1 file changed, 1 insertion(+), 12 deletions(-)
>
> diff --git a/drivers/video/fbdev/core/fbcon.c
> b/drivers/video/fbdev/core/fbcon.c
> index 8f971de35885..814b648e8f09 100644
> --- a/drivers/video/fbdev/core/fbcon.c
> +++ b/drivers/video/fbdev/core/fbcon.c
> @@ -942,7 +942,7 @@ static const char *fbcon_startup(void)
> return display_desc;
> /*
>* Instead of blindly using registered_fb[0], we use info_idx, set by
> - * fb_console_init();
> + * fbcon_fb_registered();
>*/
This comment change looks like it does not belong in this patch.
Also, the comment is wrong as info_idx is set in several places.
Like set_con2fb_map(), fbcon_remap_all(), and more.
Though it is not set by fb_console_init - so partly OK.
With the comment adjustment dropped.
Acked-by: Sam Ravnborg
at least the code deletion looked OK, I failed to follow all the logic.
So would be good if someone else could ack it too.
Sam
> info = registered_fb[info_idx];
> if (!info)
> @@ -3316,17 +3316,6 @@ static void fbcon_start(void)
> return;
> }
> #endif
> -
> - if (num_registered_fb) {
> - int i;
> -
> - for_each_registered_fb(i) {
> - info_idx = i;
> - break;
> - }
> -
> - do_fbcon_takeover(0);
> - }
> }
>
> static void fbcon_exit(void)
> --
> 2.33.0
[PATCH 06/21] fbcon: delete delayed loading code
Before
commit 6104c37094e729f3d4ce65797002112735d49cd1
Author: Daniel Vetter
Date: Tue Aug 1 17:32:07 2017 +0200
fbcon: Make fbcon a built-time depency for fbdev
it was possible to load fbcon and fbdev drivers in any order, which
means that fbcon init had to handle the case where fbdev drivers where
already registered.
This is no longer possible, hence delete that code.
Note that the exit case is a bit more complex and will be done in a
separate patch.
Signed-off-by: Daniel Vetter
Cc: Helge Deller
Cc: Daniel Vetter
Cc: Claudio Suarez
Cc: Greg Kroah-Hartman
Cc: Tetsuo Handa
Cc: Du Cheng
---
drivers/video/fbdev/core/fbcon.c | 13 +
1 file changed, 1 insertion(+), 12 deletions(-)
diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
index 8f971de35885..814b648e8f09 100644
--- a/drivers/video/fbdev/core/fbcon.c
+++ b/drivers/video/fbdev/core/fbcon.c
@@ -942,7 +942,7 @@ static const char *fbcon_startup(void)
return display_desc;
/*
* Instead of blindly using registered_fb[0], we use info_idx, set by
-* fb_console_init();
+* fbcon_fb_registered();
*/
info = registered_fb[info_idx];
if (!info)
@@ -3316,17 +3316,6 @@ static void fbcon_start(void)
return;
}
#endif
-
- if (num_registered_fb) {
- int i;
-
- for_each_registered_fb(i) {
- info_idx = i;
- break;
- }
-
- do_fbcon_takeover(0);
- }
}
static void fbcon_exit(void)
--
2.33.0
