On 01/21/2013 09:12 AM, Nikita Kiryanov wrote:
+ else if (!strncmp(displaytype, "dvi800x600", 10))
+ return set_dvi_preset(preset_dvi_800X600, 800, 600);
+ else if (!strncmp(displaytype, "dvi1024x768", 11))
+ return set_dvi_preset(preset_dvi_1024X768, 1024, 768);
+ else if (!strncmp(displaytype, "dvi1152x864", 11))
+ return set_dvi_preset(preset_dvi_1152X864, 1152, 864);
+ else if (!strncmp(displaytype, "dvi1280x960", 11))
+ return set_dvi_preset(preset_dvi_1280X960, 1280, 960);
+ else if (!strncmp(displaytype, "dvi1280x1024", 12))
+ return set_dvi_preset(preset_dvi_1280X1024, 1280, 1024);
+
+ return NONE;
+}
I think the lcd_line_length is no longer needed. ( but I haven't
checked)
Wondering if this should be board specific..
These variables are here because at the time the implementation of
lcd.c required them to be defined by the board. If you succeed in
removing them it will be a simple matter of a clean up patch.
What I meant is that Stephan Warren posted a patch to fix the
lcd_line_length
update in general, see http://patchwork.ozlabs.org/patch/212378/. (Which I
thought was picked up, but isn't yet), instead of fixing it in boards,
like you do.
+void lcd_ctrl_init(void *lcdbase)
+{
+ struct dispc_regs *dispc = (struct dispc_regs *)OMAP3_DISPC_BASE;
+ struct prcm *prcm = (struct prcm *)PRCM_BASE;
+ char *displaytype = getenv("displaytype");
+
+ if (displaytype == NULL)
+ return;
+
+ lcd_def = env_parse_displaytype(displaytype);
+ if (lcd_def == NONE)
+ return;
+
+ panel_cfg.frame_buffer = lcdbase;
+ omap3_dss_panel_config(&panel_cfg);
+ /*
+ * Pixel clock is defined with many divisions and only few
+ * multiplications of the system clock. Since DSS FCLK divisor is
set
+ * to 16 by default, we need to set it to a smaller value, like 3
+ * (chosen via trial and error).
+ */
bah, guessing timer values, this might help you
https://github.com/jhofstee/u-boot/blob/bpp3_dev2/board/victron/bpp3/lcd_timing.c
(a bit old, but simply downloading the file should work, and yes it
might warn a bit)
Thanks.
I'll consider it for future extensions to this code, but for the time
being the guess serves its purpose.
What purpose does it serve exactly? The link I mentioned is not to
update the
code per definition, but to be a bit more explicit why the timer
adjustments are
needed instead of "this seems to work.."
The whole routine should go to the video driver in my opinion.
What this function is, is predefines + call to omap3_dss_panel_config()
+ some corrections.
Anything related to the predefines is not generic. They have many
assumptions in them (such as whether the screen is active or passive)
and they are selected using a user interface that is also specific to
our board.
The adjustment after the call to omap3_dss_panel_config() is, once
again, specific to our own choices.
So overall, I don't think this is fit to be a generic driver function.
The idea would be that the driver could optionally check the env for a
display configuration. If not found or not enabled call board_video_init.
So there is a single driver for video and lcd and configurable by the
environment and you can also have all control of it in the board. I don't
have the time currently to check if this would actually work, but
I don't see a reason why not (perhaps I can check something during
the weekend).
And I didn't mean the predefined lcd configs. I am fine with you having
them in you board / tested to work / delivered with them etc. But you
add custom omap frame buffers settings, set by the user and I don't
think that part should go into a board, since it is at least common to
omap(3/4?) (or at least should be in my opinion).
+ clrsetbits_le32(&prcm->clksel_dss, 0xF, 3);
+ /*
+ * Some of what the omap3_dss_panel_config() function did, needs
to be
+ * adjusted, otherwise the image will be messed up/not appear at
all.
+ */
+ clrsetbits_le32(&dispc->gfx_attributes, 0xF << 1, GFXFORMAT_RGB16
<< 1);
+ clrsetbits_le32(&dispc->gfx_attributes, 0x3 << 6, GFXBURSTSIZE16
<< 6);
+}
mmm, do you really need 16 bit support?
Yes, we do want 16 bit support.
Why?
lcd.c is easily extended
to 32 bit support (I have a patch for it) and then you can drop the
driver
adjustment. Anyway, if you want 16 bit support it should go into the
driver
and not in your board in my opinion.
Addressed above.
well the same why... you drive them as 24 bit panels, why do you want a
lower
resolution in software?
Regards,
Jeroen
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
