From mboxrd@z Thu Jan  1 00:00:00 1970
From: Nikita Kiryanov <nikita@compulab.co.il>
Date: Thu, 26 Mar 2015 18:32:51 +0200
Subject: [U-Boot] [PATCH v3 4/4] common/lcd_console: introduce
 display/framebuffer rotation
In-Reply-To: <55132789.2020609@petermaier.org>
References: <1426664243-30998-2-git-send-email-oe5hpm@oevsv.at>
 <1426754263-29073-1-git-send-email-oe5hpm@oevsv.at>
 <1426754263-29073-5-git-send-email-oe5hpm@oevsv.at>
 <5512E13A.8000902@compulab.co.il> <55132789.2020609@petermaier.org>
Message-ID: <551434B3.7040208@compulab.co.il>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

Hi Hannes,

On 03/25/2015 11:24 PM, Hannes Petermaier wrote:
>>> +static void console_calc_rowcol(struct console_t *pcons)
>>> +{
>>> +    pcons->cols = pcons->lcdsizex / VIDEO_FONT_WIDTH;
>>> +#if defined(CONFIG_LCD_LOGO) && !defined(CONFIG_LCD_INFO_BELOW_LOGO)
>>> +    pcons->rows = (pcons->lcdsizey - BMP_LOGO_HEIGHT);
>>> +    pcons->rows /= VIDEO_FONT_HEIGHT;
>>> +#else
>>> +    pcons->rows = pcons->lcdsizey / VIDEO_FONT_HEIGHT;
>>> +#endif
>>> +}
> Okay, i will fixup the description in v4 ... maybe these troubles are coming from, lets say, sub-optimal english-language practise :-)
> In original i speak german.

That's perfectly understandable, don't worry about it :)

>>> +static void console_calc_rowcol_rot(struct console_t *pcons)
>>> +{
>>> +    u32 cols, rows;
>>> +
>>> +    if (pcons->lcdrot == 1 || pcons->lcdrot == 3) {
>>> +        cols = pcons->lcdsizey;
>>> +        rows = pcons->lcdsizex;
>>> +    } else {
>>> +        cols = pcons->lcdsizex;
>>> +        rows = pcons->lcdsizey;
>>> +    }
>>> +
>>> +    pcons->cols = cols / VIDEO_FONT_WIDTH;
>>> +#if defined(CONFIG_LCD_LOGO) && !defined(CONFIG_LCD_INFO_BELOW_LOGO)
>>> +    pcons->rows = (rows - BMP_LOGO_HEIGHT);
>>> +    pcons->rows /= VIDEO_FONT_HEIGHT;
>>> +#else
>>> +    pcons->rows = rows / VIDEO_FONT_HEIGHT;
>>> +#endif
>>
>> This duplication with console_calc_rowcol() exists because the lcdsizey and
>> lcdsizex data is expected by the functions to be already in pcons. If you
>> change console_calc_rowcol() to accept both variables as additional
>> arguments, then console_calc_rowcol() could be reused in
>> console_calc_rowcol_rot() and we'll get rid of the code duplication.
> I'm not sure about what is more uggly or better.
> To avoid this duplication and use one function i have to make this function non-static and make a mix of rotation-code into lcd_console.c - i wouldn't prefer this.
> Maybe the actual way of this (little) duplication is the beautiful one and gives most readability of the code.
>
> what do you mean about?
>

Actually I was thinking about keeping the two functions, but have console_calc_rowcol_rot() use console_calc_rowcol(), like so:

void console_calc_rowcol(struct console_t *pcons, u32 lcdsizex, lcdsizey)
{
	pcons->cols = lcdsizex / VIDEO_FONT_WIDTH;
#if defined(CONFIG_LCD_LOGO) && !defined(CONFIG_LCD_INFO_BELOW_LOGO)
	pcons->rows = (lcdsizey - BMP_LOGO_HEIGHT);
	pcons->rows /= VIDEO_FONT_HEIGHT;
#else
	pcons->rows = lcdsizey / VIDEO_FONT_HEIGHT;
#endif
}

static void console_calc_rowcol_rot(struct console_t *pcons)
{
	u32 cols, rows;

	if (pcons->lcdrot == 1 || pcons->lcdrot == 3) {
		cols = pcons->lcdsizey;
		rows = pcons->lcdsizex;
	} else {
		cols = pcons->lcdsizex;
		rows = pcons->lcdsizey;
	}

	console_calc_rowcol(&cons, cols, rows);
}

-- 
Regards,
Nikita Kiryanov