media: verisilicon: av1: Store chroma and mv offsets

[PATCH] media: verisilicon: av1: Store chroma and mv offsets

Posted by Benjamin Gaignard 1 month ago

Store chroma and motion vectors offsets for each frame so
they can be used later when resolution change.

Signed-off-by: Benjamin Gaignard <benjamin.gaignard@collabora.com>
---
 drivers/media/platform/verisilicon/hantro.h              | 7 +++++++
 .../platform/verisilicon/rockchip_vpu981_hw_av1_dec.c    | 9 +++++----
 2 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/drivers/media/platform/verisilicon/hantro.h b/drivers/media/platform/verisilicon/hantro.h
index 811260dc3c77..6d36371c1d13 100644
--- a/drivers/media/platform/verisilicon/hantro.h
+++ b/drivers/media/platform/verisilicon/hantro.h
@@ -332,12 +332,19 @@ struct hantro_vp9_decoded_buffer_info {
 	u32 bit_depth : 4;
 };
 
+struct hantro_av1_decoded_buffer_info {
+	/* Info needed when the decoded frame serves as a reference frame. */
+	size_t chroma_offset;
+	size_t mv_offset;
+};
+
 struct hantro_decoded_buffer {
 	/* Must be the first field in this struct. */
 	struct v4l2_m2m_buffer base;
 
 	union {
 		struct hantro_vp9_decoded_buffer_info vp9;
+		struct hantro_av1_decoded_buffer_info av1;
 	};
 };
 
diff --git a/drivers/media/platform/verisilicon/rockchip_vpu981_hw_av1_dec.c b/drivers/media/platform/verisilicon/rockchip_vpu981_hw_av1_dec.c
index e54f5fac325b..69b5d9e12926 100644
--- a/drivers/media/platform/verisilicon/rockchip_vpu981_hw_av1_dec.c
+++ b/drivers/media/platform/verisilicon/rockchip_vpu981_hw_av1_dec.c
@@ -686,8 +686,6 @@ rockchip_vpu981_av1_dec_set_ref(struct hantro_ctx *ctx, int ref, int idx,
 	struct hantro_dev *vpu = ctx->dev;
 	struct hantro_decoded_buffer *dst;
 	dma_addr_t luma_addr, chroma_addr, mv_addr = 0;
-	size_t cr_offset = rockchip_vpu981_av1_dec_luma_size(ctx);
-	size_t mv_offset = rockchip_vpu981_av1_dec_chroma_size(ctx);
 	int cur_width = frame->frame_width_minus_1 + 1;
 	int cur_height = frame->frame_height_minus_1 + 1;
 	int scale_width =
@@ -744,8 +742,8 @@ rockchip_vpu981_av1_dec_set_ref(struct hantro_ctx *ctx, int ref, int idx,
 
 	dst = vb2_to_hantro_decoded_buf(&av1_dec->frame_refs[idx].vb2_ref->vb2_buf);
 	luma_addr = hantro_get_dec_buf_addr(ctx, &dst->base.vb.vb2_buf);
-	chroma_addr = luma_addr + cr_offset;
-	mv_addr = luma_addr + mv_offset;
+	chroma_addr = luma_addr + dst->av1.chroma_offset;
+	mv_addr = luma_addr + dst->av1.mv_offset;
 
 	hantro_write_addr(vpu, AV1_REFERENCE_Y(ref), luma_addr);
 	hantro_write_addr(vpu, AV1_REFERENCE_CB(ref), chroma_addr);
@@ -2089,6 +2087,9 @@ rockchip_vpu981_av1_dec_set_output_buffer(struct hantro_ctx *ctx)
 	chroma_addr = luma_addr + cr_offset;
 	mv_addr = luma_addr + mv_offset;
 
+	dst->av1.chroma_offset = cr_offset;
+	dst->av1.mv_offset = mv_offset;
+
 	hantro_write_addr(vpu, AV1_TILE_OUT_LU, luma_addr);
 	hantro_write_addr(vpu, AV1_TILE_OUT_CH, chroma_addr);
 	hantro_write_addr(vpu, AV1_TILE_OUT_MV, mv_addr);
-- 
2.43.0

Re: [PATCH] media: verisilicon: av1: Store chroma and mv offsets

Posted by Nicolas Dufresne 1 month ago

Hi,

Le lundi 21 octobre 2024 à 15:49 +0000, Benjamin Gaignard a écrit :
> Store chroma and motion vectors offsets for each frame so
> they can be used later when resolution change.
> 
> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@collabora.com>

This is nicer then recalculating it from width/height of the ref frame like we
do in RK VP9 driver. I think this patch could have a Fixes tag. As we discuss, I
believe AOM test suite does not cover this case ? and thus the fluster score is
unchanged ?

I still think this patch is correct, so:

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

> ---
>  drivers/media/platform/verisilicon/hantro.h              | 7 +++++++
>  .../platform/verisilicon/rockchip_vpu981_hw_av1_dec.c    | 9 +++++----
>  2 files changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/media/platform/verisilicon/hantro.h b/drivers/media/platform/verisilicon/hantro.h
> index 811260dc3c77..6d36371c1d13 100644
> --- a/drivers/media/platform/verisilicon/hantro.h
> +++ b/drivers/media/platform/verisilicon/hantro.h
> @@ -332,12 +332,19 @@ struct hantro_vp9_decoded_buffer_info {
>  	u32 bit_depth : 4;
>  };
>  
> +struct hantro_av1_decoded_buffer_info {
> +	/* Info needed when the decoded frame serves as a reference frame. */
> +	size_t chroma_offset;
> +	size_t mv_offset;
> +};
> +
>  struct hantro_decoded_buffer {
>  	/* Must be the first field in this struct. */
>  	struct v4l2_m2m_buffer base;
>  
>  	union {
>  		struct hantro_vp9_decoded_buffer_info vp9;
> +		struct hantro_av1_decoded_buffer_info av1;
>  	};
>  };
>  
> diff --git a/drivers/media/platform/verisilicon/rockchip_vpu981_hw_av1_dec.c b/drivers/media/platform/verisilicon/rockchip_vpu981_hw_av1_dec.c
> index e54f5fac325b..69b5d9e12926 100644
> --- a/drivers/media/platform/verisilicon/rockchip_vpu981_hw_av1_dec.c
> +++ b/drivers/media/platform/verisilicon/rockchip_vpu981_hw_av1_dec.c
> @@ -686,8 +686,6 @@ rockchip_vpu981_av1_dec_set_ref(struct hantro_ctx *ctx, int ref, int idx,
>  	struct hantro_dev *vpu = ctx->dev;
>  	struct hantro_decoded_buffer *dst;
>  	dma_addr_t luma_addr, chroma_addr, mv_addr = 0;
> -	size_t cr_offset = rockchip_vpu981_av1_dec_luma_size(ctx);
> -	size_t mv_offset = rockchip_vpu981_av1_dec_chroma_size(ctx);
>  	int cur_width = frame->frame_width_minus_1 + 1;
>  	int cur_height = frame->frame_height_minus_1 + 1;
>  	int scale_width =
> @@ -744,8 +742,8 @@ rockchip_vpu981_av1_dec_set_ref(struct hantro_ctx *ctx, int ref, int idx,
>  
>  	dst = vb2_to_hantro_decoded_buf(&av1_dec->frame_refs[idx].vb2_ref->vb2_buf);
>  	luma_addr = hantro_get_dec_buf_addr(ctx, &dst->base.vb.vb2_buf);
> -	chroma_addr = luma_addr + cr_offset;
> -	mv_addr = luma_addr + mv_offset;
> +	chroma_addr = luma_addr + dst->av1.chroma_offset;
> +	mv_addr = luma_addr + dst->av1.mv_offset;
>  
>  	hantro_write_addr(vpu, AV1_REFERENCE_Y(ref), luma_addr);
>  	hantro_write_addr(vpu, AV1_REFERENCE_CB(ref), chroma_addr);
> @@ -2089,6 +2087,9 @@ rockchip_vpu981_av1_dec_set_output_buffer(struct hantro_ctx *ctx)
>  	chroma_addr = luma_addr + cr_offset;
>  	mv_addr = luma_addr + mv_offset;
>  
> +	dst->av1.chroma_offset = cr_offset;
> +	dst->av1.mv_offset = mv_offset;
> +
>  	hantro_write_addr(vpu, AV1_TILE_OUT_LU, luma_addr);
>  	hantro_write_addr(vpu, AV1_TILE_OUT_CH, chroma_addr);
>  	hantro_write_addr(vpu, AV1_TILE_OUT_MV, mv_addr);

Re: [PATCH] media: verisilicon: av1: Store chroma and mv offsets

Posted by Benjamin Gaignard 1 month ago

Le 22/10/2024 à 14:46, Nicolas Dufresne a écrit :
> Hi,
>
> Le lundi 21 octobre 2024 à 15:49 +0000, Benjamin Gaignard a écrit :
>> Store chroma and motion vectors offsets for each frame so
>> they can be used later when resolution change.
>>
>> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@collabora.com>
> This is nicer then recalculating it from width/height of the ref frame like we
> do in RK VP9 driver. I think this patch could have a Fixes tag. As we discuss, I
> believe AOM test suite does not cover this case ? and thus the fluster score is
> unchanged ?

This case isn't covered by Fluster test suites so the score is still the same.
At least I haven't introduce regressions ;-)

Benjamin

>
> I still think this patch is correct, so:
>
> Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
>
>> ---
>>   drivers/media/platform/verisilicon/hantro.h              | 7 +++++++
>>   .../platform/verisilicon/rockchip_vpu981_hw_av1_dec.c    | 9 +++++----
>>   2 files changed, 12 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/media/platform/verisilicon/hantro.h b/drivers/media/platform/verisilicon/hantro.h
>> index 811260dc3c77..6d36371c1d13 100644
>> --- a/drivers/media/platform/verisilicon/hantro.h
>> +++ b/drivers/media/platform/verisilicon/hantro.h
>> @@ -332,12 +332,19 @@ struct hantro_vp9_decoded_buffer_info {
>>   	u32 bit_depth : 4;
>>   };
>>   
>> +struct hantro_av1_decoded_buffer_info {
>> +	/* Info needed when the decoded frame serves as a reference frame. */
>> +	size_t chroma_offset;
>> +	size_t mv_offset;
>> +};
>> +
>>   struct hantro_decoded_buffer {
>>   	/* Must be the first field in this struct. */
>>   	struct v4l2_m2m_buffer base;
>>   
>>   	union {
>>   		struct hantro_vp9_decoded_buffer_info vp9;
>> +		struct hantro_av1_decoded_buffer_info av1;
>>   	};
>>   };
>>   
>> diff --git a/drivers/media/platform/verisilicon/rockchip_vpu981_hw_av1_dec.c b/drivers/media/platform/verisilicon/rockchip_vpu981_hw_av1_dec.c
>> index e54f5fac325b..69b5d9e12926 100644
>> --- a/drivers/media/platform/verisilicon/rockchip_vpu981_hw_av1_dec.c
>> +++ b/drivers/media/platform/verisilicon/rockchip_vpu981_hw_av1_dec.c
>> @@ -686,8 +686,6 @@ rockchip_vpu981_av1_dec_set_ref(struct hantro_ctx *ctx, int ref, int idx,
>>   	struct hantro_dev *vpu = ctx->dev;
>>   	struct hantro_decoded_buffer *dst;
>>   	dma_addr_t luma_addr, chroma_addr, mv_addr = 0;
>> -	size_t cr_offset = rockchip_vpu981_av1_dec_luma_size(ctx);
>> -	size_t mv_offset = rockchip_vpu981_av1_dec_chroma_size(ctx);
>>   	int cur_width = frame->frame_width_minus_1 + 1;
>>   	int cur_height = frame->frame_height_minus_1 + 1;
>>   	int scale_width =
>> @@ -744,8 +742,8 @@ rockchip_vpu981_av1_dec_set_ref(struct hantro_ctx *ctx, int ref, int idx,
>>   
>>   	dst = vb2_to_hantro_decoded_buf(&av1_dec->frame_refs[idx].vb2_ref->vb2_buf);
>>   	luma_addr = hantro_get_dec_buf_addr(ctx, &dst->base.vb.vb2_buf);
>> -	chroma_addr = luma_addr + cr_offset;
>> -	mv_addr = luma_addr + mv_offset;
>> +	chroma_addr = luma_addr + dst->av1.chroma_offset;
>> +	mv_addr = luma_addr + dst->av1.mv_offset;
>>   
>>   	hantro_write_addr(vpu, AV1_REFERENCE_Y(ref), luma_addr);
>>   	hantro_write_addr(vpu, AV1_REFERENCE_CB(ref), chroma_addr);
>> @@ -2089,6 +2087,9 @@ rockchip_vpu981_av1_dec_set_output_buffer(struct hantro_ctx *ctx)
>>   	chroma_addr = luma_addr + cr_offset;
>>   	mv_addr = luma_addr + mv_offset;
>>   
>> +	dst->av1.chroma_offset = cr_offset;
>> +	dst->av1.mv_offset = mv_offset;
>> +
>>   	hantro_write_addr(vpu, AV1_TILE_OUT_LU, luma_addr);
>>   	hantro_write_addr(vpu, AV1_TILE_OUT_CH, chroma_addr);
>>   	hantro_write_addr(vpu, AV1_TILE_OUT_MV, mv_addr);
>