| Message ID | 1356246228-26732-5-git-send-email-nikita@compulab.co.il |
|---|---|
| State | Superseded |
| Delegated to: | Tom Rini |
| Headers | show |
On 12/23/2012 08:03 AM, Nikita Kiryanov wrote: > Add support for user defined lcd parameters for cm-t35 splash screen. > > Signed-off-by: Nikita Kiryanov <nikita@compulab.co.il> > Signed-off-by: Igor Grinberg <grinberg@compulab.co.il> > --- > board/cm_t35/display.c | 213 +++++++++++++++++++++++++++++++++++++++++++++++- > 1 file changed, 210 insertions(+), 3 deletions(-) > > diff --git a/board/cm_t35/display.c b/board/cm_t35/display.c > index 11b8ed9..7b09bce 100644 > --- a/board/cm_t35/display.c > +++ b/board/cm_t35/display.c > @@ -3,6 +3,8 @@ > * > * Authors: Nikita Kiryanov <nikita@compulab.co.il> > * > + * Parsing code based on linux/drivers/video/pxafb.c > + * > * See file CREDITS for list of people who contributed to this > * project. > * > @@ -33,6 +35,7 @@ DECLARE_GLOBAL_DATA_PTR; > enum display_type { > NONE, > DVI, > + DVI_CUSTOM, > }; > > /* > @@ -138,6 +141,205 @@ static enum display_type set_dvi_preset(const struct panel_config preset, > } > > /* > + * parse_mode() - parse the mode parameter of custom lcd settings > + * > + * @mode: <res_x>x<res_y> > + * > + * Returns -1 on error, 0 on success. > + */ > +static int parse_mode(const char *mode) > +{ > + unsigned int modelen = strlen(mode); > + int res_specified = 0; > + unsigned int xres = 0, yres = 0; > + int yres_specified = 0; > + int i; > + > + for (i = modelen - 1; i >= 0; i--) { > + switch (mode[i]) { > + case 'x': > + if (!yres_specified) { > + yres = simple_strtoul(&mode[i + 1], NULL, 0); > + yres_specified = 1; > + } else { > + goto done_parsing; > + } > + > + break; > + case '0' ... '9': > + break; > + default: > + goto done_parsing; > + } > + } > + > + if (i < 0 && yres_specified) { > + xres = simple_strtoul(mode, NULL, 0); > + res_specified = 1; > + } > + > +done_parsing: > + if (res_specified) { > + set_resolution_params(xres, yres); > + } else { > + printf("LCD: invalid mode: %s\n", mode); > + return -1; > + } > + > + return 0; > +} > + > +#define PIXEL_CLK_NUMERATOR (26 * 432 / 39) > +/* > + * parse_pixclock() - Parse the pixclock parameter of custom lcd settings > + * > + * @pixclock: the desired pixel clock > + * > + * Returns -1 on error, 0 on success. > + * > + * Handling the pixel_clock: > + * > + * Pixel clock is defined in the OMAP35x TRM as follows: > + * pixel_clock = > + * (SYS_CLK * 2 * PRCM.CM_CLKSEL2_PLL[18:8]) / > + * (DSS.DISPC_DIVISOR[23:16] * DSS.DISPC_DIVISOR[6:0] * > + * PRCM.CM_CLKSEL_DSS[4:0] * (PRCM.CM_CLKSEL2_PLL[6:0] + 1)) > + * > + * In practice, this means that in order to set the > + * divisor for the desired pixel clock one needs to > + * solve the following equation: > + * > + * 26 * 432 / (39 * <pixel_clock>) = DSS.DISPC_DIVISOR[6:0] > + * > + * NOTE: the explicit equation above is reduced. Do not > + * try to infer anything from these numbers. > + */ > +static int parse_pixclock(char *pixclock) > +{ > + int divisor, pixclock_val; > + char *pixclk_start = pixclock; > + > + pixclock_val = simple_strtoul(pixclock, &pixclock, 10); > + divisor = DIV_ROUND_UP(PIXEL_CLK_NUMERATOR, pixclock_val); > + /* 0 and 1 are illegal values for PCD */ > + if (divisor <= 1) > + divisor = 2; > + > + panel_cfg.divisor = divisor | (1 << 16); > + if (pixclock[0] != '\0') { > + printf("LCD: invalid value for pixclock:%s\n", pixclk_start); > + return -1; > + } > + > + return 0; > +} > + > +/* > + * parse_setting() - parse a single setting of custom lcd parameters > + * > + * @setting: The custom lcd setting <name>:<value> > + * > + * Returns -1 on failure, 0 on success. > + */ > +static int parse_setting(char *setting) > +{ > + int num_val; > + char *setting_start = setting; > + > + if (!strncmp(setting, "mode:", 5)) { > + return parse_mode(setting + 5); > + } else if (!strncmp(setting, "pixclock:", 9)) { > + return parse_pixclock(setting + 9); > + } else if (!strncmp(setting, "left:", 5)) { > + num_val = simple_strtoul(setting + 5, &setting, 0); > + panel_cfg.timing_h |= DSS_HBP(num_val); > + } else if (!strncmp(setting, "right:", 6)) { > + num_val = simple_strtoul(setting + 6, &setting, 0); > + panel_cfg.timing_h |= DSS_HFP(num_val); > + } else if (!strncmp(setting, "upper:", 6)) { > + num_val = simple_strtoul(setting + 6, &setting, 0); > + panel_cfg.timing_v |= DSS_VBP(num_val); > + } else if (!strncmp(setting, "lower:", 6)) { > + num_val = simple_strtoul(setting + 6, &setting, 0); > + panel_cfg.timing_v |= DSS_VFP(num_val); > + } else if (!strncmp(setting, "hsynclen:", 9)) { > + num_val = simple_strtoul(setting + 9, &setting, 0); > + panel_cfg.timing_h |= DSS_HSW(num_val); > + } else if (!strncmp(setting, "vsynclen:", 9)) { > + num_val = simple_strtoul(setting + 9, &setting, 0); > + panel_cfg.timing_v |= DSS_VSW(num_val); > + } else if (!strncmp(setting, "hsync:", 6)) { > + if (simple_strtoul(setting + 6, &setting, 0) == 0) > + panel_cfg.pol_freq |= DSS_IHS; > + else > + panel_cfg.pol_freq &= ~DSS_IHS; > + } else if (!strncmp(setting, "vsync:", 6)) { > + if (simple_strtoul(setting + 6, &setting, 0) == 0) > + panel_cfg.pol_freq |= DSS_IVS; > + else > + panel_cfg.pol_freq &= ~DSS_IVS; > + } else if (!strncmp(setting, "outputen:", 9)) { > + if (simple_strtoul(setting + 9, &setting, 0) == 0) > + panel_cfg.pol_freq |= DSS_IEO; > + else > + panel_cfg.pol_freq &= ~DSS_IEO; > + } else if (!strncmp(setting, "pixclockpol:", 12)) { > + if (simple_strtoul(setting + 12, &setting, 0) == 0) > + panel_cfg.pol_freq |= DSS_IPC; > + else > + panel_cfg.pol_freq &= ~DSS_IPC; > + } else if (!strncmp(setting, "active", 6)) { > + panel_cfg.panel_type = ACTIVE_DISPLAY; > + return 0; /* Avoid sanity check below */ > + } else if (!strncmp(setting, "passive", 7)) { > + panel_cfg.panel_type = PASSIVE_DISPLAY; > + return 0; /* Avoid sanity check below */ > + } else if (!strncmp(setting, "display:", 8)) { > + if (!strncmp(setting + 8, "dvi", 3)) { > + lcd_def = DVI_CUSTOM; > + return 0; /* Avoid sanity check below */ > + } > + } else { > + printf("LCD: unknown option %s\n", setting_start); > + return -1; > + } > + > + if (setting[0] != '\0') { > + printf("LCD: invalid value for %s\n", setting_start); > + return -1; > + } > + > + return 0; > +} > + > +/* > + * env_parse_customlcd() - parse custom lcd params from an environment variable. > + * > + * @custom_lcd_params: The environment variable containing the lcd params. > + * > + * Returns -1 on failure, 0 on success. > + */ > +static int parse_customlcd(char *custom_lcd_params) > +{ > + char params_cpy[160]; > + char *setting; > + > + strncpy(params_cpy, custom_lcd_params, 160); I fail to understand why you want to copy this. > + setting = strtok(params_cpy, ","); > + while (setting) { > + if (parse_setting(setting) < 0) > + return -1; > + > + setting = strtok(NULL, ","); > + } > + > + /* Currently we don't support changing this via custom lcd params */ > + panel_cfg.data_lines = LCD_INTERFACE_24_BIT; > + again, if you only support 24 panels, why not drive them as such? > + return 0; > +} > + Is above really board specific or should it be in omap_videomodes.c or whatever? > +/* > * env_parse_displaytype() - parse display type. > * > * Parses the environment variable "displaytype", which contains the > @@ -176,14 +378,19 @@ void lcd_ctrl_init(void *lcdbase) > { > struct dispc_regs *dispc = (struct dispc_regs *)OMAP3_DISPC_BASE; > struct prcm *prcm = (struct prcm *)PRCM_BASE; > + char *custom_lcd; > char *displaytype = getenv("displaytype"); > > if (displaytype == NULL) > return; > > lcd_def = env_parse_displaytype(displaytype); > - if (lcd_def == NONE) > - return; > + /* If we did not recognize the preset, check if it's an env variable */ > + if (lcd_def == NONE) { > + custom_lcd = getenv(displaytype); > + if (custom_lcd == NULL || parse_customlcd(custom_lcd) < 0) > + return; > + } > > panel_cfg.frame_buffer = lcdbase; > omap3_dss_panel_config(&panel_cfg); > @@ -204,7 +411,7 @@ void lcd_ctrl_init(void *lcdbase) > > void lcd_enable(void) > { > - if (lcd_def == DVI) { > + if (lcd_def == DVI || lcd_def == DVI_CUSTOM) { > gpio_direction_output(54, 0); /* Turn on DVI */ > omap3_dss_enable(); > } Regards, Jeroen
Hi Jeroen, On 01/20/2013 11:08 PM, Jeroen Hofstee wrote: > On 12/23/2012 08:03 AM, Nikita Kiryanov wrote: [...] >> + * Returns -1 on failure, 0 on success. >> + */ >> +static int parse_customlcd(char *custom_lcd_params) >> +{ >> + char params_cpy[160]; >> + char *setting; >> + >> + strncpy(params_cpy, custom_lcd_params, 160); > I fail to understand why you want to copy this. strtok modifies the string it operates on. The documentation for getenv states that you must not modify the string it returns. >> + setting = strtok(params_cpy, ","); >> + while (setting) { >> + if (parse_setting(setting) < 0) >> + return -1; >> + >> + setting = strtok(NULL, ","); >> + } >> + >> + /* Currently we don't support changing this via custom lcd params */ >> + panel_cfg.data_lines = LCD_INTERFACE_24_BIT; >> + > again, if you only support 24 panels, why not drive them as such? Can you please elaborate on this comment? I'm not entirely sure what inconsistencies you are referring to. >> + return 0; >> +} >> + > Is above really board specific or should it be in omap_videomodes.c or > whatever? Well, most of it is parsing for a custom feature, so I would say this is board specific. >> +/* >> * env_parse_displaytype() - parse display type. >> * >> * Parses the environment variable "displaytype", which contains the >> @@ -176,14 +378,19 @@ void lcd_ctrl_init(void *lcdbase) >> { >> struct dispc_regs *dispc = (struct dispc_regs *)OMAP3_DISPC_BASE; >> struct prcm *prcm = (struct prcm *)PRCM_BASE; >> + char *custom_lcd; >> char *displaytype = getenv("displaytype"); >> if (displaytype == NULL) >> return; >> lcd_def = env_parse_displaytype(displaytype); >> - if (lcd_def == NONE) >> - return; >> + /* If we did not recognize the preset, check if it's an env >> variable */ >> + if (lcd_def == NONE) { >> + custom_lcd = getenv(displaytype); >> + if (custom_lcd == NULL || parse_customlcd(custom_lcd) < 0) >> + return; >> + } >> panel_cfg.frame_buffer = lcdbase; >> omap3_dss_panel_config(&panel_cfg); >> @@ -204,7 +411,7 @@ void lcd_ctrl_init(void *lcdbase) >> void lcd_enable(void) >> { >> - if (lcd_def == DVI) { >> + if (lcd_def == DVI || lcd_def == DVI_CUSTOM) { >> gpio_direction_output(54, 0); /* Turn on DVI */ >> omap3_dss_enable(); >> } > Regards, > Jeroen >
Hi Nikita, On 01/21/2013 09:25 AM, Nikita Kiryanov wrote: > Hi Jeroen, > > On 01/20/2013 11:08 PM, Jeroen Hofstee wrote: >> On 12/23/2012 08:03 AM, Nikita Kiryanov wrote: > [...] >>> + * Returns -1 on failure, 0 on success. >>> + */ >>> +static int parse_customlcd(char *custom_lcd_params) >>> +{ >>> + char params_cpy[160]; >>> + char *setting; >>> + >>> + strncpy(params_cpy, custom_lcd_params, 160); >> I fail to understand why you want to copy this. > > strtok modifies the string it operates on. The documentation for > getenv states that you must not modify the string it returns. > that seems to make sense... >>> + setting = strtok(params_cpy, ","); >>> + while (setting) { >>> + if (parse_setting(setting) < 0) >>> + return -1; >>> + >>> + setting = strtok(NULL, ","); >>> + } >>> + >>> + /* Currently we don't support changing this via custom lcd >>> params */ >>> + panel_cfg.data_lines = LCD_INTERFACE_24_BIT; >>> + >> again, if you only support 24 panels, why not drive them as such? > > Can you please elaborate on this comment? I'm not entirely sure what > inconsistencies you are referring to. You're driving only 24 bit panels, yet you want the software to drive 16 bit panels. Why not keep the software frame buffer at 32 bits? > >>> + return 0; >>> +} >>> + >> Is above really board specific or should it be in omap_videomodes.c or >> whatever? > > Well, most of it is parsing for a custom feature, so I would say this is > board specific. I can't understand how custom settings can be board specific, unless you mess with timer of course (but even then....). Regards, Jeroen
On 01/24/13 00:36, Jeroen Hofstee wrote: > Hi Nikita, > > On 01/21/2013 09:25 AM, Nikita Kiryanov wrote: >> Hi Jeroen, >> >> On 01/20/2013 11:08 PM, Jeroen Hofstee wrote: >>> On 12/23/2012 08:03 AM, Nikita Kiryanov wrote: >> [...] >>>> + * Returns -1 on failure, 0 on success. >>>> + */ >>>> +static int parse_customlcd(char *custom_lcd_params) >>>> +{ >>>> + char params_cpy[160]; >>>> + char *setting; >>>> + >>>> + strncpy(params_cpy, custom_lcd_params, 160); >>> I fail to understand why you want to copy this. >> >> strtok modifies the string it operates on. The documentation for >> getenv states that you must not modify the string it returns. >> > that seems to make sense... >>>> + setting = strtok(params_cpy, ","); >>>> + while (setting) { >>>> + if (parse_setting(setting) < 0) >>>> + return -1; >>>> + >>>> + setting = strtok(NULL, ","); >>>> + } >>>> + >>>> + /* Currently we don't support changing this via custom lcd params */ >>>> + panel_cfg.data_lines = LCD_INTERFACE_24_BIT; >>>> + >>> again, if you only support 24 panels, why not drive them as such? >> >> Can you please elaborate on this comment? I'm not entirely sure what >> inconsistencies you are referring to. > You're driving only 24 bit panels, yet you want the software to drive 16 bit panels. > Why not keep the software frame buffer at 32 bits? >> >>>> + return 0; >>>> +} >>>> + >>> Is above really board specific or should it be in omap_videomodes.c or >>> whatever? >> >> Well, most of it is parsing for a custom feature, so I would say this is >> board specific. > I can't understand how custom settings can be board specific, unless you > mess with timer of course (but even then....). I fail to understand which timer are you talking about... Probably, you mean the DPLLs and the clocks?
diff --git a/board/cm_t35/display.c b/board/cm_t35/display.c index 11b8ed9..7b09bce 100644 --- a/board/cm_t35/display.c +++ b/board/cm_t35/display.c @@ -3,6 +3,8 @@ * * Authors: Nikita Kiryanov <nikita@compulab.co.il> * + * Parsing code based on linux/drivers/video/pxafb.c + * * See file CREDITS for list of people who contributed to this * project. * @@ -33,6 +35,7 @@ DECLARE_GLOBAL_DATA_PTR; enum display_type { NONE, DVI, + DVI_CUSTOM, }; /* @@ -138,6 +141,205 @@ static enum display_type set_dvi_preset(const struct panel_config preset, } /* + * parse_mode() - parse the mode parameter of custom lcd settings + * + * @mode: <res_x>x<res_y> + * + * Returns -1 on error, 0 on success. + */ +static int parse_mode(const char *mode) +{ + unsigned int modelen = strlen(mode); + int res_specified = 0; + unsigned int xres = 0, yres = 0; + int yres_specified = 0; + int i; + + for (i = modelen - 1; i >= 0; i--) { + switch (mode[i]) { + case 'x': + if (!yres_specified) { + yres = simple_strtoul(&mode[i + 1], NULL, 0); + yres_specified = 1; + } else { + goto done_parsing; + } + + break; + case '0' ... '9': + break; + default: + goto done_parsing; + } + } + + if (i < 0 && yres_specified) { + xres = simple_strtoul(mode, NULL, 0); + res_specified = 1; + } + +done_parsing: + if (res_specified) { + set_resolution_params(xres, yres); + } else { + printf("LCD: invalid mode: %s\n", mode); + return -1; + } + + return 0; +} + +#define PIXEL_CLK_NUMERATOR (26 * 432 / 39) +/* + * parse_pixclock() - Parse the pixclock parameter of custom lcd settings + * + * @pixclock: the desired pixel clock + * + * Returns -1 on error, 0 on success. + * + * Handling the pixel_clock: + * + * Pixel clock is defined in the OMAP35x TRM as follows: + * pixel_clock = + * (SYS_CLK * 2 * PRCM.CM_CLKSEL2_PLL[18:8]) / + * (DSS.DISPC_DIVISOR[23:16] * DSS.DISPC_DIVISOR[6:0] * + * PRCM.CM_CLKSEL_DSS[4:0] * (PRCM.CM_CLKSEL2_PLL[6:0] + 1)) + * + * In practice, this means that in order to set the + * divisor for the desired pixel clock one needs to + * solve the following equation: + * + * 26 * 432 / (39 * <pixel_clock>) = DSS.DISPC_DIVISOR[6:0] + * + * NOTE: the explicit equation above is reduced. Do not + * try to infer anything from these numbers. + */ +static int parse_pixclock(char *pixclock) +{ + int divisor, pixclock_val; + char *pixclk_start = pixclock; + + pixclock_val = simple_strtoul(pixclock, &pixclock, 10); + divisor = DIV_ROUND_UP(PIXEL_CLK_NUMERATOR, pixclock_val); + /* 0 and 1 are illegal values for PCD */ + if (divisor <= 1) + divisor = 2; + + panel_cfg.divisor = divisor | (1 << 16); + if (pixclock[0] != '\0') { + printf("LCD: invalid value for pixclock:%s\n", pixclk_start); + return -1; + } + + return 0; +} + +/* + * parse_setting() - parse a single setting of custom lcd parameters + * + * @setting: The custom lcd setting <name>:<value> + * + * Returns -1 on failure, 0 on success. + */ +static int parse_setting(char *setting) +{ + int num_val; + char *setting_start = setting; + + if (!strncmp(setting, "mode:", 5)) { + return parse_mode(setting + 5); + } else if (!strncmp(setting, "pixclock:", 9)) { + return parse_pixclock(setting + 9); + } else if (!strncmp(setting, "left:", 5)) { + num_val = simple_strtoul(setting + 5, &setting, 0); + panel_cfg.timing_h |= DSS_HBP(num_val); + } else if (!strncmp(setting, "right:", 6)) { + num_val = simple_strtoul(setting + 6, &setting, 0); + panel_cfg.timing_h |= DSS_HFP(num_val); + } else if (!strncmp(setting, "upper:", 6)) { + num_val = simple_strtoul(setting + 6, &setting, 0); + panel_cfg.timing_v |= DSS_VBP(num_val); + } else if (!strncmp(setting, "lower:", 6)) { + num_val = simple_strtoul(setting + 6, &setting, 0); + panel_cfg.timing_v |= DSS_VFP(num_val); + } else if (!strncmp(setting, "hsynclen:", 9)) { + num_val = simple_strtoul(setting + 9, &setting, 0); + panel_cfg.timing_h |= DSS_HSW(num_val); + } else if (!strncmp(setting, "vsynclen:", 9)) { + num_val = simple_strtoul(setting + 9, &setting, 0); + panel_cfg.timing_v |= DSS_VSW(num_val); + } else if (!strncmp(setting, "hsync:", 6)) { + if (simple_strtoul(setting + 6, &setting, 0) == 0) + panel_cfg.pol_freq |= DSS_IHS; + else + panel_cfg.pol_freq &= ~DSS_IHS; + } else if (!strncmp(setting, "vsync:", 6)) { + if (simple_strtoul(setting + 6, &setting, 0) == 0) + panel_cfg.pol_freq |= DSS_IVS; + else + panel_cfg.pol_freq &= ~DSS_IVS; + } else if (!strncmp(setting, "outputen:", 9)) { + if (simple_strtoul(setting + 9, &setting, 0) == 0) + panel_cfg.pol_freq |= DSS_IEO; + else + panel_cfg.pol_freq &= ~DSS_IEO; + } else if (!strncmp(setting, "pixclockpol:", 12)) { + if (simple_strtoul(setting + 12, &setting, 0) == 0) + panel_cfg.pol_freq |= DSS_IPC; + else + panel_cfg.pol_freq &= ~DSS_IPC; + } else if (!strncmp(setting, "active", 6)) { + panel_cfg.panel_type = ACTIVE_DISPLAY; + return 0; /* Avoid sanity check below */ + } else if (!strncmp(setting, "passive", 7)) { + panel_cfg.panel_type = PASSIVE_DISPLAY; + return 0; /* Avoid sanity check below */ + } else if (!strncmp(setting, "display:", 8)) { + if (!strncmp(setting + 8, "dvi", 3)) { + lcd_def = DVI_CUSTOM; + return 0; /* Avoid sanity check below */ + } + } else { + printf("LCD: unknown option %s\n", setting_start); + return -1; + } + + if (setting[0] != '\0') { + printf("LCD: invalid value for %s\n", setting_start); + return -1; + } + + return 0; +} + +/* + * env_parse_customlcd() - parse custom lcd params from an environment variable. + * + * @custom_lcd_params: The environment variable containing the lcd params. + * + * Returns -1 on failure, 0 on success. + */ +static int parse_customlcd(char *custom_lcd_params) +{ + char params_cpy[160]; + char *setting; + + strncpy(params_cpy, custom_lcd_params, 160); + setting = strtok(params_cpy, ","); + while (setting) { + if (parse_setting(setting) < 0) + return -1; + + setting = strtok(NULL, ","); + } + + /* Currently we don't support changing this via custom lcd params */ + panel_cfg.data_lines = LCD_INTERFACE_24_BIT; + + return 0; +} + +/* * env_parse_displaytype() - parse display type. * * Parses the environment variable "displaytype", which contains the @@ -176,14 +378,19 @@ void lcd_ctrl_init(void *lcdbase) { struct dispc_regs *dispc = (struct dispc_regs *)OMAP3_DISPC_BASE; struct prcm *prcm = (struct prcm *)PRCM_BASE; + char *custom_lcd; char *displaytype = getenv("displaytype"); if (displaytype == NULL) return; lcd_def = env_parse_displaytype(displaytype); - if (lcd_def == NONE) - return; + /* If we did not recognize the preset, check if it's an env variable */ + if (lcd_def == NONE) { + custom_lcd = getenv(displaytype); + if (custom_lcd == NULL || parse_customlcd(custom_lcd) < 0) + return; + } panel_cfg.frame_buffer = lcdbase; omap3_dss_panel_config(&panel_cfg); @@ -204,7 +411,7 @@ void lcd_ctrl_init(void *lcdbase) void lcd_enable(void) { - if (lcd_def == DVI) { + if (lcd_def == DVI || lcd_def == DVI_CUSTOM) { gpio_direction_output(54, 0); /* Turn on DVI */ omap3_dss_enable(); }