Hi Wolfgang Denk,
On 01/29/2013 04:00 PM, Wolfgang Denk wrote:
Dear Nikita Kiryanov,
In message 1359463349-11649-6-git-send-email-nikita@compulab.co.il you wrote:
Add support for dvi displays with user selectable dvi presets.
...
--- a/board/cm_t35/cm_t35.c +++ b/board/cm_t35/cm_t35.c @@ -216,6 +216,9 @@ static void cm_t3x_set_common_muxconf(void) /* SB-T35 Ethernet */ MUX_VAL(CP(GPMC_NCS4), (IEN | PTU | EN | M0)); /*GPMC_nCS4*/
- /* DVI enable */
- MUX_VAL(CP(GPMC_NCS3), (IDIS | PTU | DIS | M4));/*GPMC_nCS3*/
Is it intentional and correct to always enable DVI, even when this is not configured by the user?
This simply prepares the mux that makes it possible to enable DVI. The actual enabling of DVI is indeed conditional on user setup.
+/*
- The frame buffer is allocated before we have the chance to parse user input.
This seems broken to me.
Agreed.
Please explain why you think so?
This isn't just my impression. A patch was submitted by Stephen Warren to address this issue (http://patchwork.ozlabs.org/patch/212378/), but it isn't in mainline yet.
- vl_{col | row} to the maximal resolution supported by OMAP3.
- */
+vidinfo_t panel_info = {
- .vl_col = 1400,
- .vl_row = 1050,
- .vl_bpix = LCD_BPP,
- .cmap = (ushort *)0x80100000,
Can we please avoid such hard coded magic numbers?
OK.
+/*
- env_parse_displaytype() - parse display type.
- Parses the environment variable "displaytype", which contains the
- name of the display type or preset, in which case it applies its
- configurations.
So we have yet another custom implementation for setting the display type? Can we please agree on using common standard methods? thanks.
Most of what is custom in this patchset is parsing of environment variables. For actually setting up the DSS I do use common code (specifically omap3_dss_panel_config() and related structs/defines).
Best regards,
Wolfgang Denk