Re: [PATCH v3 1/4] media: rkvdec: Introduce a global bitwriter helper

[Nicolas Dufresne <nicolas.dufresne@collabora.com>]

[-- Attachment #1: Type: text/plain, Size: 2559 bytes --]

Le jeudi 02 avril 2026 à 10:06 -0400, Detlev Casanova a écrit :
> The use of structures with bitfields is good when the values are
> somewhat aligned.
> More mis-alignement means that compilers need to do more gymanstics
                                                           gymnastics
> to edit the fields values.
> 
> Some cases have been reported with CLang on specific architectures
> like armhf and hexagon, where the compiler would allocate a bigger
> local stack than needed or even completely freeze during compilation.
> 
> Some fixes have been provided to ease the issues, but the real fix
> here is to use a bitwriter instead of heavily unaligned bitfields.
> 
> This is a preparation commit to provide a global bitwriter interface
> for the whole driver.
> 
> Signed-off-by: Detlev Casanova <detlev.casanova@collabora.com>
> ---
>  .../platform/rockchip/rkvdec/rkvdec-bitwriter.h    | 39 ++++++++++++++++++++++
>  1 file changed, 39 insertions(+)
> 
> diff --git a/drivers/media/platform/rockchip/rkvdec/rkvdec-bitwriter.h b/drivers/media/platform/rockchip/rkvdec/rkvdec-bitwriter.h
> new file mode 100644
> index 000000000000..2a5c271ade91
> --- /dev/null
> +++ b/drivers/media/platform/rockchip/rkvdec/rkvdec-bitwriter.h
> @@ -0,0 +1,39 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Rockchip Video Decoder bit writer
> + *
> + * Copyright (C) 2026 Collabora, Ltd.
> + *      Detlev Casanova <detlev.casanova@collabora.com>
> + * Copyright (C) 2019 Collabora, Ltd.
> + *	Boris Brezillon <boris.brezillon@collabora.com>

nit: indent it the same for for your name.

I will do the edits if I find now other issues in the series.

Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>

> + */
> +
> +#ifndef RKVDEC_BIT_WRITER_H_
> +#define RKVDEC_BIT_WRITER_H_
> +
> +#include <linux/types.h>
> +#include <linux/bits.h>
> +
> +struct rkvdec_bw_field {
> +	u16 offset;
> +	u8 len;
> +};
> +
> +#define BW_FIELD(_offset, _len) ((struct rkvdec_bw_field){ _offset, _len })
> +
> +static inline void rkvdec_set_bw_field(u32 *buf, struct rkvdec_bw_field field, u32 value)
> +{
> +	u8 bit = field.offset % 32;
> +	u16 word = field.offset / 32;
> +	u64 mask = GENMASK_ULL(bit + field.len - 1, bit);
> +	u64 val = ((u64)value << bit) & mask;
> +
> +	buf[word] &= ~mask;
> +	buf[word] |= val;
> +	if (bit + field.len > 32) {
> +		buf[word + 1] &= ~(mask >> 32);
> +		buf[word + 1] |= val >> 32;
> +	}
> +}
> +
> +#endif /* RKVDEC_BIT_WRITER_H_ */

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 228 bytes --]