[U-Boot] [PATCH v3 3/4] dm: video: use constants to refer to colors

[Heinrich Schuchardt]

On 02/04/2018 02:40 PM, Simon Glass wrote:
> Hi Heinrich,
> 
> On 29 January 2018 at 00:19, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
>> Use constants to refer to colors.
>> Adjust initialization of foreground and background color to avoid
>> setting reserved bits.
>> Consistently u32 instead of unsigned for color bit mask.
>>
>> Signed-off-by: Heinrich Schuchardt <xypron.glpk at gmx.de>
>> ---
>> v3
>>         Use color constants for initalizing the console.
>> v2
>>         no change
>> ---
>>  drivers/video/vidconsole-uclass.c | 55 +++++++++++++++++++++++----------------
>>  drivers/video/video-uclass.c      | 19 +++++++++-----
>>  include/video.h                   | 11 ++++++--
>>  include/video_console.h           | 31 ++++++++++++++++++++++
>>  4 files changed, 85 insertions(+), 31 deletions(-)
> 
> Reviewed-by: Simon Glass <sjg at chromium.org>
> 
> Regarding my point about using u32 in function return values and args,
> I don't think I explained it very well.
> 
> IMO is makes no sense to structure all the intermediate code which
> generates pixel values to use u32, when an unsigned int is enough on
> all machines that U-Boot supports. The packing / unpacking into a
> 32-bit word in memory is something that is done once when the pixel is
> accessed. Thereafter I don't see a need to push things around in a
> particular format.

The definition I found was
typedef unsigned int u32;

What makes you believe there is any packing and unpacking or masking
overhead avoided by using unsigned int or any other 32bit integer type?

For struct vid_rgb other integer types could be used. I not sure if
accessing a char is any faster in this context than using 32bit integers.

Regards

Heinrich

> 
> I have an aversion to code which forces the compiler to mask every
> variable access just to pass the data around.
> 
> So I would prefer to use u32 only when accessing the hardware, or for
> pointers which do that.
> 
> Reards,
> Simon
>