[PATCH V6 08/13] cmd: bmp: Split bmp commands and functions
Nikhil M Jain
n-jain1 at ti.com
Thu Apr 6 08:27:26 CEST 2023
Hi Simon,
On 06/04/23 00:07, Simon Glass wrote:
> On Wed, 5 Apr 2023 at 01:06, Nikhil M Jain <n-jain1 at ti.com> wrote:
>>
>> To enable splash screen at SPL, need to compile cmd/bmp.c which also
>> includes bmp commands, since SPL doesn't use commands split bmp.c into
>> common/bmp.c which includes all bmp functions and cmd/bmp.c which only
>> contains bmp commands.
>>
>> Add function delclaration for bmp_info in video.h.
>>
>> Signed-off-by: Nikhil M Jain <n-jain1 at ti.com>
>> ---
>> V6:
>> - Fix commit message.
>> - Remove unused header files.
>>
>> V5:
>> - Rename cmd/bmp_cmd to cmd/bmp.
>>
>> V4:
>> - No change.
>>
>> V3 (patch introduced):
>> - Split bmp functions and commands.
>>
>> cmd/bmp.c | 162 +-----------------------------------------------
>> common/bmp.c | 152 +++++++++++++++++++++++++++++++++++++++++++++
>> include/video.h | 7 +++
>> 3 files changed, 161 insertions(+), 160 deletions(-)
>> create mode 100644 common/bmp.c
>
> Reviewed-by: Simon Glass <sjg at chromium.org>
>
> Please see below
>
>
>> diff --git a/common/bmp.c b/common/bmp.c
>> new file mode 100644
>> index 0000000000..6134ada5a7
>> --- /dev/null
>> +++ b/common/bmp.c
>> @@ -0,0 +1,152 @@
>> +// SPDX-License-Identifier: GPL-2.0+
>> +/*
>> + * (C) Copyright 2002
>> + * Detlev Zundel, DENX Software Engineering, dzu at denx.de.
>> + */
>> +
>> +/*
>> + * BMP handling routines
>> + */
>> +
>> +#include <bmp_layout.h>
>> +#include <command.h>
>> +#include <dm.h>
>> +#include <gzip.h>
>> +#include <log.h>
>> +#include <malloc.h>
>> +#include <mapmem.h>
>> +#include <splash.h>
>> +#include <video.h>
>> +#include <asm/byteorder.h>
>> +
>> +/*
>> + * Allocate and decompress a BMP image using gunzip().
>> + *
>> + * Returns a pointer to the decompressed image data. This pointer is
>> + * aligned to 32-bit-aligned-address + 2.
>> + * See doc/README.displaying-bmps for explanation.
>> + *
>> + * The allocation address is passed to 'alloc_addr' and must be freed
>> + * by the caller after use.
>> + *
>> + * Returns NULL if decompression failed, or if the decompressed data
>> + * didn't contain a valid BMP signature.
>> + */
>> +#ifdef CONFIG_VIDEO_BMP_GZIP
>
> You can drop this #ifdef and see below
>
>> +struct bmp_image *gunzip_bmp(unsigned long addr, unsigned long *lenp,
>> + void **alloc_addr)
>> +{
>> + void *dst;
>> + unsigned long len;
>> + struct bmp_image *bmp;
>> +
>
> if (!IS_ENABLED(CONFIG_VIDEO_BMP_GZIP))
> return NULL;
>
I preferred to use #if to avoid compilation of code when not required.
For example, if someone doesn't want to display a gzip bmp image they
wouldn't want the code to be compiled, so that binary size doesn't
increase.
>> + /*
>> + * Decompress bmp image
>> + */
>> + len = CONFIG_VIDEO_LOGO_MAX_SIZE;
>> + /* allocate extra 3 bytes for 32-bit-aligned-address + 2 alignment */
>> + dst = malloc(CONFIG_VIDEO_LOGO_MAX_SIZE + 3);
>> + if (!dst) {
>> + puts("Error: malloc in gunzip failed!\n");
>> + return NULL;
>> + }
>> +
>> + /* align to 32-bit-aligned-address + 2 */
>> + bmp = dst + 2;
>> +
>> + if (gunzip(bmp, CONFIG_VIDEO_LOGO_MAX_SIZE, map_sysmem(addr, 0),
>> + &len)) {
>> + free(dst);
>> + return NULL;
>> + }
>> + if (len == CONFIG_VIDEO_LOGO_MAX_SIZE)
>> + puts("Image could be truncated (increase CONFIG_VIDEO_LOGO_MAX_SIZE)!\n");
>> +
>> + /*
>> + * Check for bmp mark 'BM'
>> + */
>> + if (!((bmp->header.signature[0] == 'B') &&
>> + (bmp->header.signature[1] == 'M'))) {
>> + free(dst);
>> + return NULL;
>> + }
>> +
>> + debug("Gzipped BMP image detected!\n");
>> +
>> + *alloc_addr = dst;
>> + return bmp;
>> +}
>> +#else
>> +struct bmp_image *gunzip_bmp(unsigned long addr, unsigned long *lenp,
>> + void **alloc_addr)
>> +{
>> + return NULL;
>> +}
>> +#endif
>> +
>> +
>> +#ifdef CONFIG_NEEDS_MANUAL_RELOC
>> +void bmp_reloc(void) {
>> + fixup_cmdtable(cmd_bmp_sub, ARRAY_SIZE(cmd_bmp_sub));
>> +}
>> +#endif
>> +
>> +int bmp_info(ulong addr)
>> +{
>> + struct bmp_image *bmp = (struct bmp_image *)map_sysmem(addr, 0);
>> + void *bmp_alloc_addr = NULL;
>> + unsigned long len;
>> +
>> + if (!((bmp->header.signature[0]=='B') &&
>> + (bmp->header.signature[1]=='M')))
>> + bmp = gunzip_bmp(addr, &len, &bmp_alloc_addr);
>> +
>> + if (bmp == NULL) {
>> + printf("There is no valid bmp file at the given address\n");
>> + return 1;
>> + }
>> +
>> + printf("Image size : %d x %d\n", le32_to_cpu(bmp->header.width),
>> + le32_to_cpu(bmp->header.height));
>> + printf("Bits per pixel: %d\n", le16_to_cpu(bmp->header.bit_count));
>> + printf("Compression : %d\n", le32_to_cpu(bmp->header.compression));
>> +
>> + if (bmp_alloc_addr)
>> + free(bmp_alloc_addr);
>> +
>> + return(0);
>> +}
>> +
>> +int bmp_display(ulong addr, int x, int y)
>> +{
>> + struct udevice *dev;
>> + int ret;
>> + struct bmp_image *bmp = map_sysmem(addr, 0);
>> + void *bmp_alloc_addr = NULL;
>> + unsigned long len;
>> +
>> + if (!((bmp->header.signature[0]=='B') &&
>> + (bmp->header.signature[1]=='M')))
>> + bmp = gunzip_bmp(addr, &len, &bmp_alloc_addr);
>> +
>> + if (!bmp) {
>> + printf("There is no valid bmp file at the given address\n");
>> + return 1;
>> + }
>> + addr = map_to_sysmem(bmp);
>> +
>> + ret = uclass_first_device_err(UCLASS_VIDEO, &dev);
>> + if (!ret) {
>> + bool align = false;
>> +
>> + if (x == BMP_ALIGN_CENTER || y == BMP_ALIGN_CENTER)
>> + align = true;
>> +
>> + ret = video_bmp_display(dev, addr, x, y, align);
>> + }
>> +
>> + if (bmp_alloc_addr)
>> + free(bmp_alloc_addr);
>> +
>> + return ret ? CMD_RET_FAILURE : 0;
>> +}
>> diff --git a/include/video.h b/include/video.h
>> index 4d99e5dc27..b38fb9081a 100644
>> --- a/include/video.h
>> +++ b/include/video.h
>> @@ -357,4 +357,11 @@ void *video_get_u_boot_logo(void);
>> */
>> int bmp_display(ulong addr, int x, int y);
>>
>> +/*
>> + * bmp_info() - Show information about bmp file
>> + *
>> + * @addr: address of bmp file
>
> Return: ...
>
>> + */
>> +int bmp_info(ulong addr);
>> +
>> #endif
>> --
>> 2.34.1
>>
>
> Regards,
> SImon
More information about the U-Boot
mailing list