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

[Simon Glass]

Hi Heinrich,

On 4 February 2018 at 15:55, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
> 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?

A machine with a 32-bit register has to mask its argument on entry to the
function to ensure that the caller does not pass an invalid value.

>
> 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.

In general the natural word size is best for arguments and return values I
think (int or unsigned int).

>
> 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
>>
>

Regards,
Simon