From mboxrd@z Thu Jan 1 00:00:00 1970 From: Igor Grinberg Date: Wed, 18 Mar 2015 17:27:11 +0200 Subject: [U-Boot] [PATCH v2 4/4] common/lcd_console: introduce display/framebuffer rotation In-Reply-To: References: <1426664243-30998-1-git-send-email-oe5hpm@oevsv.at> <1426664243-30998-5-git-send-email-oe5hpm@oevsv.at> <55097611.50606@compulab.co.il> Message-ID: <5509994F.7060501@compulab.co.il> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On 03/18/15 16:59, Hannes Petermaier wrote: > "U-Boot" schrieb am 18.03.2015 13:56:49: > >> >> Hi Hannes, > Hi Igor, > thanks for response - we come closer to the final solution :-) Indeed :-) > >> >>> +} >>> + >>> +void __weak lcd_init_console_rot(struct console_t *pcons) >>> +{ >>> + return; >>> +} >>> + >>> +void lcd_init_console(void *address, int vl_cols, int vl_rows, int > vl_rot) >>> +{ >>> + memset(&cons, 0, sizeof(cons)); >>> + cons.lcd_address = address; >>> + >>> + cons.lcdsizex = vl_cols; >>> + cons.lcdsizey = vl_rows; >>> + cons.lcdrot = vl_rot; >>> + >>> + cons.fp_putc_xy = &lcd_putc_xy0; >>> + cons.fp_console_moverow = &console_moverow0; >>> + cons.fp_console_setrow = &console_setrow0; >>> + console_calc_rowcol(&cons); >> >> I think the above four lines is exactly what should be placed in the >> __weak variant of lcd_init_console_rot() function (the one just above >> this one). > I think not so. > If the lcd_console_rotation.c is compiled in, the __weak function isn't > called anymore. > And if user wants to have 0 degree rotation, the init-function from > lcd_console_rotation.c doesn't anything. > Therefore it is necessary to initialize these pointers here. Right. Forgot about the 0 case... > >>> + >>> + lcd_init_console_rot(&cons); >>> + >>> + debug("lcd_console: have %d/%d col/rws on scr %dx%d (%d deg > rotated)\n", >>> + cons.cols, cons.rows, cons.lcdsizex, cons.lcdsizey, vl_rot); >>> + >> >> no need for the empty line here. > Will be changed in v3. > >>> + >>> +#include >>> +#include >>> +#include /* Get font data, width and height */ >>> + >>> +#if LCD_BPP == LCD_COLOR16 >>> + #define fbptr_t ushort >>> +#elif LCD_BPP == LCD_COLOR32 >>> + #define fbptr_t u32 >>> +#else >>> + #define fbptr_t uchar >>> +#endif >> >> That is a duplication of the code in lcd_console.c. >> If we can get rid of these size games, probably we should have in the > lcd.h, >> or lcd_console.h, or ... > It might be possible to move this into lcd.h, in every case it is > necesarry that common.h > is included before - from here the information about LCD_BPP is coming. > I will have a 2nd look to that to avoid this duplication. > >>> + >>> +void lcd_init_console_rot(struct console_t *pcons) >>> +{ >>> + if (pcons->lcdrot == 0) { >>> + return; >>> + } else if (pcons->lcdrot == 1) { >>> + pcons->fp_putc_xy = &lcd_putc_xy90; >>> + pcons->fp_console_moverow = &console_moverow90; >>> + pcons->fp_console_setrow = &console_setrow90; >>> + } else if (pcons->lcdrot == 2) { >>> + pcons->fp_putc_xy = &lcd_putc_xy180; >>> + pcons->fp_console_moverow = &console_moverow180; >>> + pcons->fp_console_setrow = &console_setrow180; >>> + } else if (pcons->lcdrot == 3) { >>> + pcons->fp_putc_xy = &lcd_putc_xy270; >>> + pcons->fp_console_moverow = &console_moverow270; >>> + pcons->fp_console_setrow = &console_setrow270; >>> + } else { >>> + puts("lcd_init_console_rot: invalid framebuffer rotation!\n"); >> >> How about >> printf("%s: invalid framebuffer rotation!\n", __func__); >> ? > Okay, i will change that. > Sometime ago, somebody told me on the mailing list that i should prefer > the puts function if i don't have to print out some values. Well, this is no longer true as we have discussed this during the last U-Boot summit. > If we want to use the printf we can also printout the given rotation > pcons->lcdrot. Good. -- Regards, Igor.