Re: [PATCH] media: s5p-mfc: Corrected NV12M/NV21M plane-sizes

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Jain,

I haven't dig much, but I have a quick question below.

Le mardi 06 août 2024 à 17:27 +0530, Aakarsh Jain a écrit :
> There is a possibility of getting page fault if the overall
> buffer size is not aligned to 256bytes. Since MFC does read
> operation only and it won't corrupt the data values even if
> it reads the extra bytes.
> Corrected luma and chroma plane sizes for V4L2_PIX_FMT_NV12M
> and V4L2_PIX_FMT_NV21M pixel format.

Have you re-run v4l2 compliance ? (better be safe then sorry).

> 
> Signed-off-by: Aakarsh Jain <aakarsh.jain@xxxxxxxxxxx>
> ---
>  .../media/platform/samsung/s5p-mfc/s5p_mfc_opr_v6.c    | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/media/platform/samsung/s5p-mfc/s5p_mfc_opr_v6.c b/drivers/media/platform/samsung/s5p-mfc/s5p_mfc_opr_v6.c
> index 73f7af674c01..03c957221fc4 100644
> --- a/drivers/media/platform/samsung/s5p-mfc/s5p_mfc_opr_v6.c
> +++ b/drivers/media/platform/samsung/s5p-mfc/s5p_mfc_opr_v6.c
> @@ -498,8 +498,8 @@ static void s5p_mfc_dec_calc_dpb_size_v6(struct s5p_mfc_ctx *ctx)
>  	case V4L2_PIX_FMT_NV21M:
>  		ctx->stride[0] = ALIGN(ctx->img_width, S5P_FIMV_NV12MT_HALIGN_V6);
>  		ctx->stride[1] = ALIGN(ctx->img_width, S5P_FIMV_NV12MT_HALIGN_V6);
> -		ctx->luma_size = calc_plane(ctx->stride[0], ctx->img_height);
> -		ctx->chroma_size = calc_plane(ctx->stride[1], (ctx->img_height / 2));
> +		ctx->luma_size = calc_plane(ctx->img_width, ctx->img_height);
> +		ctx->chroma_size = calc_plane(ctx->img_width, (ctx->img_height >> 1));

These size needs to match the sizes reported through TRY_FMT (and S_FMT)
sizeimage for each planes. Is this code being call withing try_fmt ? Will these
value match or will this change cause the value to miss-match ?

The reason is that correct value is needed for allocating this memory from the
outside (like using a DMAbuf Heap). Perhaps its all right, let me know.

Nicolas

>  		break;
>  	case V4L2_PIX_FMT_YUV420M:
>  	case V4L2_PIX_FMT_YVU420M:
> @@ -539,9 +539,11 @@ static void s5p_mfc_dec_calc_dpb_size_v6(struct s5p_mfc_ctx *ctx)
>  static void s5p_mfc_enc_calc_src_size_v6(struct s5p_mfc_ctx *ctx)
>  {
>  	unsigned int mb_width, mb_height;
> +	unsigned int default_size;
>  
>  	mb_width = MB_WIDTH(ctx->img_width);
>  	mb_height = MB_HEIGHT(ctx->img_height);
> +	default_size = (mb_width * mb_height) * 256;
>  
>  	if (IS_MFCV12(ctx->dev)) {
>  		switch (ctx->src_fmt->fourcc) {
> @@ -549,8 +551,8 @@ static void s5p_mfc_enc_calc_src_size_v6(struct s5p_mfc_ctx *ctx)
>  		case V4L2_PIX_FMT_NV21M:
>  			ctx->stride[0] = ALIGN(ctx->img_width, S5P_FIMV_NV12M_HALIGN_V6);
>  			ctx->stride[1] = ALIGN(ctx->img_width, S5P_FIMV_NV12M_HALIGN_V6);
> -			ctx->luma_size = ctx->stride[0] * ALIGN(ctx->img_height, 16);
> -			ctx->chroma_size =  ctx->stride[0] * ALIGN(ctx->img_height / 2, 16);
> +			ctx->luma_size = ALIGN(default_size, 256);
> +			ctx->chroma_size = ALIGN(default_size / 2, 256);
>  			break;
>  		case V4L2_PIX_FMT_YUV420M:
>  		case V4L2_PIX_FMT_YVU420M:






[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux