Re: [U-Boot] [PATCH] [v2] powerpc: 'monitor' environment variable contains full video configuration
Wolfgang Denk wrote: > Please re-read the conversation then. Mind the context (U-Boot versus > Linux). Mind what I wrote about defaults and users being able to > chose the behaviour they want. And keep in mind what I wrote about > enforcing desicions "in the best interest" of your users (who may > happen to disagree). After re-reading it, I've come to the conclusion is that as far as my patch is concerned, I just need to make the following changes: 1) Modify video_get_params() (in videomodes.c) to parse the "video-mode "environment variable, using the same format that Linux uses. Right now, it parses the variable "video", so that variable is now deprecated. 2) Update the DIU driver to use this feature of videomodes.c. This means taking the functionality of my function diu_get_video_params() and putting it into video_get_params(). 3) Remove the code that sets diubootargs. If the user wants to pass parameters to Linux, he has to add "video=$video-mode console=tty0" to his command-line. If "video-mode" is not set, then he must *not* add "video=$video-mode console=tty0" to his command-line, otherwise the kernel will get confused. There is no way to automate this for the user. And that's it. If this is wrong, please be detailed in what I have wrong, because I can't guess any more what you want. So I have one concern: My patch has a function called diu_get_video_params() which is like video_get_params(), but it returns a lot more information. video_get_params() is currently used by several other drivers, none of which I can test. I'm very hesitant to rewrite video_get_params() so that it returns the same data that diu_get_video_params() returns, because I can't test these other boards. -- Timur Tabi Linux kernel developer at Freescale ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] [v2] powerpc: 'monitor' environment variable contains full video configuration
Dear Timur Tabi, In message <4d7fd08c.2020...@freescale.com> you wrote: > > Then I'm confused, because video_hw_init() is what configures and enables the > video display. So if that function succeeds (i.e. returns non-NULL), then > U-Boot will put the console onto the video display. You said you wanted me to > implement some mechanism where I enable the display, based on the value of the > video-mode variable, without putting console on the display. However, from > you > just said, it appears that this mechanism already exists. Please re-read the conversation then. Mind the context (U-Boot versus Linux). Mind what I wrote about defaults and users being able to chose the behaviour they want. And keep in mind what I wrote about enforcing desicions "in the best interest" of your users (who may happen to disagree). Best regards, Wolfgang Denk -- DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de News is what a chap who doesn't care much about anything wants to read. And it's only news until he's read it. After that it's dead. - Evelyn Waugh _Scoop_ (1938) bk. 1, ch. 5 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] [v2] powerpc: 'monitor' environment variable contains full video configuration
Wolfgang Denk wrote: >> > What is the standard mechanism for enabling the console on the video >> > display? >> > From what I can tell, if video_hw_init() returns non-NULL, then the >> > console is >> > set to the video display, regardless as to what the "stdout" environment >> > variable says. > That's the default. Users can overwrite it, for example in "preboot". > See also CONFIG_ENV_OVERWRITE Then I'm confused, because video_hw_init() is what configures and enables the video display. So if that function succeeds (i.e. returns non-NULL), then U-Boot will put the console onto the video display. You said you wanted me to implement some mechanism where I enable the display, based on the value of the video-mode variable, without putting console on the display. However, from you just said, it appears that this mechanism already exists. -- Timur Tabi Linux kernel developer at Freescale ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] [v2] powerpc: 'monitor' environment variable contains full video configuration
Dear Timur Tabi, In message <4d7fc2c5.2020...@freescale.com> you wrote: > > Ok, I think I get it. But I'm going to miss this merge window, because now I > have to rewrite half of my patch. You won't. We have a delay of at least 2 weeks. > What is the standard mechanism for enabling the console on the video display? > From what I can tell, if video_hw_init() returns non-NULL, then the console is > set to the video display, regardless as to what the "stdout" environment > variable says. That's the default. Users can overwrite it, for example in "preboot". See also CONFIG_ENV_OVERWRITE Best regards, Wolfgang Denk -- DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de People are very flexible and learn to adjust to strange surroundings -- they can become accustomed to read Lisp and Fortran programs, for example. - Leon Sterling and Ehud Shapiro, Art of Prolog, MIT Press ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] [v2] powerpc: 'monitor' environment variable contains full video configuration
Wolfgang Denk wrote: > It may be a blank screen, or a (previously loaded) splash screen, or > (a bit later) some graphics application outputting data. > > It's up to the user to decide that. Ok, I think I get it. But I'm going to miss this merge window, because now I have to rewrite half of my patch. What is the standard mechanism for enabling the console on the video display? >From what I can tell, if video_hw_init() returns non-NULL, then the console is set to the video display, regardless as to what the "stdout" environment variable says. -- Timur Tabi Linux kernel developer at Freescale ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] [v2] powerpc: 'monitor' environment variable contains full video configuration
Dear Timur Tabi, In message <4d7fb987.20...@freescale.com> you wrote: > > > But there is no reason to always and unconditionally put the console > > on that device - that is a completely separate and independent > > decision. > > I still don't understand the point of enabling the video display in U-Boot if > we're not going to put the U-Boot console on it. The monitor will just have a > blank screen. It may be a blank screen, or a (previously loaded) splash screen, or (a bit later) some graphics application outputting data. It's up to the user to decide that. Best regards, Wolfgang Denk -- DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de "I've finally learned what `upward compatible' means. It means we get to keep all our old mistakes." - Dennie van Tassel ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] [v2] powerpc: 'monitor' environment variable contains full video configuration
Wolfgang Denk wrote: > You misinterpret what I wrote. Of course we're going to enable the > video display then. > > But there is no reason to always and unconditionally put the console > on that device - that is a completely separate and independent > decision. I still don't understand the point of enabling the video display in U-Boot if we're not going to put the U-Boot console on it. The monitor will just have a blank screen. We don't need to enable it for Linux' sake, because Linux can enable the monitor itself if it wants to. It just needs to be told what resolution to use. -- Timur Tabi Linux kernel developer at Freescale ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] [v2] powerpc: 'monitor' environment variable contains full video configuration
Dear Timur Tabi, In message <4d7f8b3c.4080...@freescale.com> you wrote: > > > Don't try to be more clever than the user. Instead of helping, you > > restrict him. That's bad. > > I'm not being more clever. The code is setting a variable (diubootargs) that > is > guaranteed to be the same video mode that U-Boot is running. If you want to > ensure that Linux set to the same video mode, then use the variable. > Otherwise, > don't use the variable and set the command line manually. You don't need another variable for setting the video mode, because we (will) have "video-mode" for that very purpose, that can and shall be used both in U-Boot and Linux. > > NAK, NAK, NAK. All such automatic and unconditional editing is bad > > and should strictly be avoided. > > You didn't understand my post. I was saying that I tried to implement it, but > gave up because it got too complicated. I did understand your posting. I wanted to tell you that you should not even try doing such things. > > Leave the decision which device to use as console to the user. > > That's what the 'monitor' environment variable is for. "monitor" has nothing to do with the console, right? typically we (here at DENX) use helper macros like setenv addcons 'setenv bootargs ${bootargs} console=${consdev},${baudrate}' setenv consdev ttyS0 Then you can have the "addcons" in some command sequence that builds up the bootargs. "monitor"? No, this has _nothing_ to do with any console settings. > >> > 2) The video display needs to be enabled and the U-Boot console needs to > >> > be > >> > routed to it > > NAK. > > > > Wether the U-Boot console is attached to the serial port or the video > > console or netconsole or anything else should be left to the user. > > Again, that's what the variable is for. What's the point of configuring the > video display if you're not going to enable it? You misinterpret what I wrote. Of course we're going to enable the video display then. But there is no reason to always and unconditionally put the console on that device - that is a completely separate and independent decision. > > A default setting is OK, but the user must be able to set anything he > > likes. > > Are we speaking the same language? It doesn't appear that you're > understanding > anything I'm saying. And vice versa. Probably you don't read what I'm writing either. > I still don't understand what you actually want. I want that you keep the console settings out of this topic. It has nothing to do with it. And I want to make sure that we don't have several environment variables doing more or less the same thing. Best regards, Wolfgang Denk -- DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de A Chairman was as necessary to a Board planet as the zero was in mathematics, but being a zero had big disadvantages... - Terry Pratchett, _The Dark Side of the Sun_ ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] [v2] powerpc: 'monitor' environment variable contains full video configuration
Wolfgang Denk wrote: >> Well, that's why I have two variables. My patch has the ability to set the >> > kernel command line appropriately if the video display is configured and >> > enabled >> > in U-Boot. The second variable is used to assist in setting the actual >> > kernel >> > command-line, because that's the easiest and safest way to do it. An > Don't try to be more clever than the user. Instead of helping, you > restrict him. That's bad. I'm not being more clever. The code is setting a variable (diubootargs) that is guaranteed to be the same video mode that U-Boot is running. If you want to ensure that Linux set to the same video mode, then use the variable. Otherwise, don't use the variable and set the command line manually. >> > alternative that I tried to implement is to have do_bootm_linux() edit the >> > kernel command line directly, removing any existing video= option and >> > putting a > NAK, NAK, NAK. All such automatic and unconditional editing is bad > and should strictly be avoided. You didn't understand my post. I was saying that I tried to implement it, but gave up because it got too complicated. > Leave the decision which device to use as console to the user. That's what the 'monitor' environment variable is for. >> > There are three things that need to be done: >> > >> > 1) The video mode needs to be configured > ACK. > >> > 2) The video display needs to be enabled and the U-Boot console needs to be >> > routed to it > NAK. > > Wether the U-Boot console is attached to the serial port or the video > console or netconsole or anything else should be left to the user. Again, that's what the variable is for. What's the point of configuring the video display if you're not going to enable it? > A default setting is OK, but the user must be able to set anything he > likes. Are we speaking the same language? It doesn't appear that you're understanding anything I'm saying. >> > 3) The kernel command line needs to be set > ACK. Again, the user must have free choice of options. > >> > For #1, we use video-mode. How do you want #2 and #3 handled? > See above. I still don't understand what you actually want. -- Timur Tabi Linux kernel developer at Freescale ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] [v2] powerpc: 'monitor' environment variable contains full video configuration
Dear Timur Tabi, In message <4d7f83ec.4050...@freescale.com> you wrote: > > > NAK. video-mode settings and console settings are two separate things > > and must not be mangled into a single variable. > > Well, that's why I have two variables. My patch has the ability to set the > kernel command line appropriately if the video display is configured and > enabled > in U-Boot. The second variable is used to assist in setting the actual kernel > command-line, because that's the easiest and safest way to do it. An Don't try to be more clever than the user. Instead of helping, you restrict him. That's bad. > alternative that I tried to implement is to have do_bootm_linux() edit the > kernel command line directly, removing any existing video= option and putting > a NAK, NAK, NAK. All such automatic and unconditional editing is bad and should strictly be avoided. Leave the decision which device to use as console to the user. > There are three things that need to be done: > > 1) The video mode needs to be configured ACK. > 2) The video display needs to be enabled and the U-Boot console needs to be > routed to it NAK. Wether the U-Boot console is attached to the serial port or the video console or netconsole or anything else should be left to the user. A default setting is OK, but the user must be able to set anything he likes. > 3) The kernel command line needs to be set ACK. Again, the user must have free choice of options. > For #1, we use video-mode. How do you want #2 and #3 handled? See above. Best regards, Wolfgang Denk -- DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de The alternative to genuflecting before the god of code-bumming is finding a better algorithm. It should be clear that none such was available. If your code is too slow, you must make it faster. If no better algorithm is available, you must trim cycles. - t...@alice.uucp (Tom Duff) 29 Aug 88 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] [v2] powerpc: 'monitor' environment variable contains full video configuration
Wolfgang Denk wrote: >> > because the kernel also needs to see "console=tty0" on the command line >> > only if > This may or may not be the case. Frequently we still use a serial > console even when booting with a graphics display enabled. > > This is a different option, and does not belong into that setting. > >> > video is supposed to be enabled. I want to make video mode completely >> > dynamic, >> > so that if the 'video-mode' variable is set, then the console is switched >> > to the >> > video device, and the kernel is told to do the same. Otherwise, all >> > output will >> > go to the serial port. > NAK. video-mode settings and console settings are two separate things > and must not be mangled into a single variable. Well, that's why I have two variables. My patch has the ability to set the kernel command line appropriately if the video display is configured and enabled in U-Boot. The second variable is used to assist in setting the actual kernel command-line, because that's the easiest and safest way to do it. An alternative that I tried to implement is to have do_bootm_linux() edit the kernel command line directly, removing any existing video= option and putting a new one in, but I found that to be too intrusive, especially since we don't have any good string editing functions already in U-Boot. There are three things that need to be done: 1) The video mode needs to be configured 2) The video display needs to be enabled and the U-Boot console needs to be routed to it 3) The kernel command line needs to be set For #1, we use video-mode. How do you want #2 and #3 handled? -- Timur Tabi Linux kernel developer at Freescale ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] [v2] powerpc: 'monitor' environment variable contains full video configuration
Dear Timur Tabi, In message <4d7f7cb9.8090...@freescale.com> you wrote: > > ("monitor" -> "video" and "diubootargs" -> something else more generic) and video-mode ? > update the videomodes.c code to parse the same video string that the kernel > uses. I also gathered that no one has chosen to update videomodes.c, so I > would > have to do it. > > Do I have that right? I think so. Anatolij? Stefano? > > At this point I wonder which use ther eis left for your "monitor" > > variable - it should be completely redundant now? > > Well, it may still be necessary to have two variables. One for the mode that No, this is exactly what should be avoided. > U-Boot parses, and one for the string that is passed to the kernel. This is U-Boot and the Kernel should parse exactly the same variable. > because the kernel also needs to see "console=tty0" on the command line only > if This may or may not be the case. Frequently we still use a serial console even when booting with a graphics display enabled. This is a different option, and does not belong into that setting. > video is supposed to be enabled. I want to make video mode completely > dynamic, > so that if the 'video-mode' variable is set, then the console is switched to > the > video device, and the kernel is told to do the same. Otherwise, all output > will > go to the serial port. NAK. video-mode settings and console settings are two separate things and must not be mangled into a single variable. Best regards, Wolfgang Denk -- DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de And now remains That we find out the cause of this effect, Or rather say, the cause of this defect... -- Hamlet, Act II, Scene 2 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] [v2] powerpc: 'monitor' environment variable contains full video configuration
Wolfgang Denk wrote: > Agreed, but please not by introducing lots of new, probably later > incompatible code. Fair enough. >> > Can you point me to the thread or at least tell me the subject line? I >> > have >> > no idea what you're talking about. > See http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/88169/focus=88240 Ok, so what I get from that thread is to change the name of the variables ("monitor" -> "video" and "diubootargs" -> something else more generic) and update the videomodes.c code to parse the same video string that the kernel uses. I also gathered that no one has chosen to update videomodes.c, so I would have to do it. Do I have that right? > OK, so please split that patch: one patch should do this cleanup, and > another one should contain the rest. I can't make any promises, since a lot of the code is intermingled. > Instead of a non-standard and undocumented 'diubootargs' environment > variable please use something (probably called "video-mode" :-) that > can be passed as "video-mode=" boot argument to Linux. > > At this point I wonder which use ther eis left for your "monitor" > variable - it should be completely redundant now? Well, it may still be necessary to have two variables. One for the mode that U-Boot parses, and one for the string that is passed to the kernel. This is because the kernel also needs to see "console=tty0" on the command line only if video is supposed to be enabled. I want to make video mode completely dynamic, so that if the 'video-mode' variable is set, then the console is switched to the video device, and the kernel is told to do the same. Otherwise, all output will go to the serial port. -- Timur Tabi Linux kernel developer at Freescale -- Timur Tabi Linux kernel developer at Freescale ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] [v2] powerpc: 'monitor' environment variable contains full video configuration
Dear Tabi Timur-B04825, In message <4d7ea72c.9040...@freescale.com> you wrote: > > > Can this not be turned into generic code, usable for other boards / > > systems as well? > > Possibly, but I would like to fix one bug at a time. Agreed, but please not by introducing lots of new, probably later incompatible code. > > Please see the discussion we had about video modes for the i.MX > > systems. > > Can you point me to the thread or at least tell me the subject line? I have > no idea what you're talking about. See http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/88169/focus=88240 > > I want to see handled most of this with generic code, not with > > processor / architecture specific one. > > This patch also cleans up a lot of the DIU code to make it handle more gene- > ric options. OK, so please split that patch: one patch should do this cleanup, and another one should contain the rest. Instead of a non-standard and undocumented 'diubootargs' environment variable please use something (probably called "video-mode" :-) that can be passed as "video-mode=" boot argument to Linux. At this point I wonder which use ther eis left for your "monitor" variable - it should be completely redundant now? Best regards, Wolfgang Denk -- DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de A Freudian slip is when you say one thing but mean your mother. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] [v2] powerpc: 'monitor' environment variable contains full video configuration
Wolfgang Denk wrote: > Looks as if this was a Freescale specific implementation? Yes. > Can this not be turned into generic code, usable for other boards / > systems as well? Possibly, but I would like to fix one bug at a time. > Please see the discussion we had about video modes for the i.MX > systems. Can you point me to the thread or at least tell me the subject line? I have no idea what you're talking about. > I want to see handled most of this with generic code, not with > processor / architecture specific one. This patch also cleans up a lot of the DIU code to make it handle more generic options. -- Timur Tabi Linux kernel developer ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] [v2] powerpc: 'monitor' environment variable contains full video configuration
Dear Timur Tabi, In message <1300133949-1115-1-git-send-email-ti...@freescale.com> you wrote: > Update the "monitor" environment variable (for Freescale chips that have a > DIU display controller) to designate the full video configuration, instead > of just the output monitor. > > The old definition of the "monitor" environment variable only determines > which video port to use for output. This variable is set to a number (0, > 1, or sometimes 2) to specify a DVI, LVDS, or Dual-LVDS port. The > resolution is hard-coded into board-specific code. The Linux command-line > arguments need to be hard-coded to the proper video definition string. > > Instead, the "monitor" variable is now set to a string that indicates not > only the part name, but also the resolution, color depth, and refresh rate. > This string is then parsed for all needed information, and can be passed to > the kernel via the 'diubootargs' environment variable. Looks as if this was a Freescale specific implementation? Can this not be turned into generic code, usable for other boards / systems as well? Please see the discussion we had about video modes for the i.MX systems. I want to see handled most of this with generic code, not with processor / architecture specific one. Best regards, Wolfgang Denk -- DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de Even historians fail to learn from history -- they repeat the same mistakes. -- John Gill, "Patterns of Force", stardate 2534.7 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot