[U-Boot] [PATCH v2 4/4] common/lcd_console: introduce display/framebuffer rotation

[Igor Grinberg <grinberg@compulab.co.il>]

On 03/18/15 16:59, Hannes Petermaier wrote:
> "U-Boot" <u-boot-bounces@lists.denx.de> 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 <common.h>
>>> +#include <lcd.h>
>>> +#include <video_font.h>      /* 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.