2023-11-16 15:49:28

by Andrzej Pietrasiewicz

[permalink] [raw]
Subject: [RFC 3/6] media: verisilicon: Improve constant's name

For VP8 BIT(18) of this register is for enabling the boolean encoder.

Signed-off-by: Andrzej Pietrasiewicz <[email protected]>
---
drivers/media/platform/verisilicon/hantro_h1_regs.h | 2 +-
drivers/media/platform/verisilicon/hantro_h1_vp8_enc.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/verisilicon/hantro_h1_regs.h b/drivers/media/platform/verisilicon/hantro_h1_regs.h
index 7752d1291c0e..c1c66c934a24 100644
--- a/drivers/media/platform/verisilicon/hantro_h1_regs.h
+++ b/drivers/media/platform/verisilicon/hantro_h1_regs.h
@@ -70,7 +70,7 @@
#define H1_REG_ENC_CTRL2_DISABLE_QUARTER_PIXMV BIT(22)
#define H1_REG_ENC_CTRL2_TRANS8X8_MODE_EN BIT(21)
#define H1_REG_ENC_CTRL2_CABAC_INIT_IDC(x) ((x) << 19)
-#define H1_REG_ENC_CTRL2_ENTROPY_CODING_MODE BIT(18)
+#define H1_REG_ENC_CTRL2_VP8_BOOLENC_ENABLE BIT(18)
#define H1_REG_ENC_CTRL2_H264_INTER4X4_MODE BIT(17)
#define H1_REG_ENC_CTRL2_H264_STREAM_MODE BIT(16)
#define H1_REG_ENC_CTRL2_INTRA16X16_MODE(x) ((x))
diff --git a/drivers/media/platform/verisilicon/hantro_h1_vp8_enc.c b/drivers/media/platform/verisilicon/hantro_h1_vp8_enc.c
index 05aa0dd9c09c..08c5079fbfd0 100644
--- a/drivers/media/platform/verisilicon/hantro_h1_vp8_enc.c
+++ b/drivers/media/platform/verisilicon/hantro_h1_vp8_enc.c
@@ -1226,7 +1226,7 @@ static void hantro_h1_vp8_enc_set_params(struct hantro_dev *vpu, struct hantro_c
reg = 0;
if (mb_width * mb_height > MAX_MB_COUNT_TO_DISABLE_QUARTER_PIXEL_MV)
reg = H1_REG_ENC_CTRL2_DISABLE_QUARTER_PIXMV;
- reg |= H1_REG_ENC_CTRL2_ENTROPY_CODING_MODE;
+ reg |= H1_REG_ENC_CTRL2_VP8_BOOLENC_ENABLE;

inter_favor = 128 - ctx->vp8_enc.prob_intra;
if (inter_favor >= 0)
--
2.25.1


2023-11-17 06:12:48

by Chen-Yu Tsai

[permalink] [raw]
Subject: Re: [RFC 3/6] media: verisilicon: Improve constant's name

On Thu, Nov 16, 2023 at 11:48 PM Andrzej Pietrasiewicz
<[email protected]> wrote:
>
> For VP8 BIT(18) of this register is for enabling the boolean encoder.

Yes, but for H.264 it selects the entropy coding mode, 0 for CAVLC
and 1 for CABAC. You even add it back in the last patch. I'd do it
here, so you disambiguate the definition within one patch.

ChenYu


> Signed-off-by: Andrzej Pietrasiewicz <[email protected]>
> ---
> drivers/media/platform/verisilicon/hantro_h1_regs.h | 2 +-
> drivers/media/platform/verisilicon/hantro_h1_vp8_enc.c | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/media/platform/verisilicon/hantro_h1_regs.h b/drivers/media/platform/verisilicon/hantro_h1_regs.h
> index 7752d1291c0e..c1c66c934a24 100644
> --- a/drivers/media/platform/verisilicon/hantro_h1_regs.h
> +++ b/drivers/media/platform/verisilicon/hantro_h1_regs.h
> @@ -70,7 +70,7 @@
> #define H1_REG_ENC_CTRL2_DISABLE_QUARTER_PIXMV BIT(22)
> #define H1_REG_ENC_CTRL2_TRANS8X8_MODE_EN BIT(21)
> #define H1_REG_ENC_CTRL2_CABAC_INIT_IDC(x) ((x) << 19)
> -#define H1_REG_ENC_CTRL2_ENTROPY_CODING_MODE BIT(18)
> +#define H1_REG_ENC_CTRL2_VP8_BOOLENC_ENABLE BIT(18)
> #define H1_REG_ENC_CTRL2_H264_INTER4X4_MODE BIT(17)
> #define H1_REG_ENC_CTRL2_H264_STREAM_MODE BIT(16)
> #define H1_REG_ENC_CTRL2_INTRA16X16_MODE(x) ((x))
> diff --git a/drivers/media/platform/verisilicon/hantro_h1_vp8_enc.c b/drivers/media/platform/verisilicon/hantro_h1_vp8_enc.c
> index 05aa0dd9c09c..08c5079fbfd0 100644
> --- a/drivers/media/platform/verisilicon/hantro_h1_vp8_enc.c
> +++ b/drivers/media/platform/verisilicon/hantro_h1_vp8_enc.c
> @@ -1226,7 +1226,7 @@ static void hantro_h1_vp8_enc_set_params(struct hantro_dev *vpu, struct hantro_c
> reg = 0;
> if (mb_width * mb_height > MAX_MB_COUNT_TO_DISABLE_QUARTER_PIXEL_MV)
> reg = H1_REG_ENC_CTRL2_DISABLE_QUARTER_PIXMV;
> - reg |= H1_REG_ENC_CTRL2_ENTROPY_CODING_MODE;
> + reg |= H1_REG_ENC_CTRL2_VP8_BOOLENC_ENABLE;
>
> inter_favor = 128 - ctx->vp8_enc.prob_intra;
> if (inter_favor >= 0)
> --
> 2.25.1
>
>

2023-11-17 08:44:44

by Andrzej Pietrasiewicz

[permalink] [raw]
Subject: Re: [RFC 3/6] media: verisilicon: Improve constant's name

Hi,

Thanks for looking at it.

W dniu 17.11.2023 o 07:11, Chen-Yu Tsai pisze:
> On Thu, Nov 16, 2023 at 11:48 PM Andrzej Pietrasiewicz
> <[email protected]> wrote:
>>
>> For VP8 BIT(18) of this register is for enabling the boolean encoder.
>
> Yes, but for H.264 it selects the entropy coding mode, 0 for CAVLC
> and 1 for CABAC. You even add it back in the last patch. I'd do it
> here, so you disambiguate the definition within one patch.
>

The rationale behind doing what I did is this:

At this moment the H1 H.264 encoder is non-existent in the kernel,
so why would we keep H.264-related definitions? I re-introduce it
when the encoder appears.

That said, other H.264-specific constants do exist at this moment anyway.
So I'm fine with either approach.

Andrzej

> ChenYu
>
>
>> Signed-off-by: Andrzej Pietrasiewicz <[email protected]>
>> ---
>> drivers/media/platform/verisilicon/hantro_h1_regs.h | 2 +-
>> drivers/media/platform/verisilicon/hantro_h1_vp8_enc.c | 2 +-
>> 2 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/media/platform/verisilicon/hantro_h1_regs.h b/drivers/media/platform/verisilicon/hantro_h1_regs.h
>> index 7752d1291c0e..c1c66c934a24 100644
>> --- a/drivers/media/platform/verisilicon/hantro_h1_regs.h
>> +++ b/drivers/media/platform/verisilicon/hantro_h1_regs.h
>> @@ -70,7 +70,7 @@
>> #define H1_REG_ENC_CTRL2_DISABLE_QUARTER_PIXMV BIT(22)
>> #define H1_REG_ENC_CTRL2_TRANS8X8_MODE_EN BIT(21)
>> #define H1_REG_ENC_CTRL2_CABAC_INIT_IDC(x) ((x) << 19)
>> -#define H1_REG_ENC_CTRL2_ENTROPY_CODING_MODE BIT(18)
>> +#define H1_REG_ENC_CTRL2_VP8_BOOLENC_ENABLE BIT(18)
>> #define H1_REG_ENC_CTRL2_H264_INTER4X4_MODE BIT(17)
>> #define H1_REG_ENC_CTRL2_H264_STREAM_MODE BIT(16)
>> #define H1_REG_ENC_CTRL2_INTRA16X16_MODE(x) ((x))
>> diff --git a/drivers/media/platform/verisilicon/hantro_h1_vp8_enc.c b/drivers/media/platform/verisilicon/hantro_h1_vp8_enc.c
>> index 05aa0dd9c09c..08c5079fbfd0 100644
>> --- a/drivers/media/platform/verisilicon/hantro_h1_vp8_enc.c
>> +++ b/drivers/media/platform/verisilicon/hantro_h1_vp8_enc.c
>> @@ -1226,7 +1226,7 @@ static void hantro_h1_vp8_enc_set_params(struct hantro_dev *vpu, struct hantro_c
>> reg = 0;
>> if (mb_width * mb_height > MAX_MB_COUNT_TO_DISABLE_QUARTER_PIXEL_MV)
>> reg = H1_REG_ENC_CTRL2_DISABLE_QUARTER_PIXMV;
>> - reg |= H1_REG_ENC_CTRL2_ENTROPY_CODING_MODE;
>> + reg |= H1_REG_ENC_CTRL2_VP8_BOOLENC_ENABLE;
>>
>> inter_favor = 128 - ctx->vp8_enc.prob_intra;
>> if (inter_favor >= 0)
>> --
>> 2.25.1
>>
>>
> _______________________________________________
> Kernel mailing list -- [email protected]
> To unsubscribe send an email to [email protected]