linux-kernel - Re: [PATCH v3 1/1] media: mediatek: jpeg: support 34bits

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <ae3a3925-0c34-4e05-8786-f76debadab9a@collabora.com>
Date: Mon, 29 Jul 2024 14:41:58 +0200
From: AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>
To: Jianhua Lin <jianhua.lin@...iatek.com>, daniel.almeida@...labora.com,
 mchehab@...nel.org
Cc: linux-media@...r.kernel.org, linux-kernel@...r.kernel.org,
 linux-arm-kernel@...ts.infradead.org, linux-mediatek@...ts.infradead.org,
 Project_Global_Chrome_Upstream_Group@...iatek.com
Subject: Re: [PATCH v3 1/1] media: mediatek: jpeg: support 34bits

Il 28/07/24 14:01, Jianhua Lin ha scritto:
> the iommu HW supported 34bits iova space(16GB),
> but mediatek jpeg enc/dec driver still is 32bit,
> then need to set the bit32/bit33 iova to jpeg HW.
> 
> Signed-off-by: Jianhua Lin <jianhua.lin@...iatek.com>
> ---
> Changes compared with v2:
> - remove dev_info() to print 34bits support info.
> - keep of_property_read_bool() to check 34bits status,
>    because need to compatible old soc.
> - use FIELD_PREP() and GENMASK() to generate bit mask.
> - remove rdma pandig enable code, because the code not
>    related to 34bit support.
>   
>   .../platform/mediatek/jpeg/mtk_jpeg_core.c    |  8 ++-
>   .../platform/mediatek/jpeg/mtk_jpeg_core.h    |  4 ++
>   .../platform/mediatek/jpeg/mtk_jpeg_dec_hw.c  | 59 +++++++++++++++----
>   .../platform/mediatek/jpeg/mtk_jpeg_dec_hw.h  |  1 +
>   .../platform/mediatek/jpeg/mtk_jpeg_dec_reg.h |  8 +++
>   .../platform/mediatek/jpeg/mtk_jpeg_enc_hw.c  | 44 ++++++++++++--
>   .../platform/mediatek/jpeg/mtk_jpeg_enc_hw.h  |  7 ++-
>   7 files changed, 111 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
> index ac48658e2de4..abee5ee5c6e3 100644
> --- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
> +++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
> @@ -1030,6 +1030,7 @@ static void mtk_jpeg_dec_device_run(void *priv)
>   	spin_lock_irqsave(&jpeg->hw_lock, flags);
>   	mtk_jpeg_dec_reset(jpeg->reg_base);
>   	mtk_jpeg_dec_set_config(jpeg->reg_base,
> +				jpeg->support_34bit,
>   				&jpeg_src_buf->dec_param,
>   				jpeg_src_buf->bs_size,
>   				&bs,
> @@ -1317,6 +1318,9 @@ static int mtk_jpeg_probe(struct platform_device *pdev)
>   		return -EINVAL;
>   	}
>   
> +	if (of_property_read_bool(pdev->dev.of_node, "mediatek,34bits"))

I seem to remember that I already reviewed this part and said that this is a SoC
capability, meaning that one SoC either always does, or always doesn't support
34-bits IOVA.

This means that this **shall not** be a devicetree property, but *must* be set in
the platform data (mtk_jpeg_variant) for the specific SoC supporting that.

> +		jpeg->support_34bit = true;
> +
>   	if (!jpeg->variant->multi_core) {
>   		ret = mtk_jpeg_single_core_init(pdev, jpeg);
>   		if (ret) {
> @@ -1564,7 +1568,8 @@ static irqreturn_t mtk_jpeg_enc_done(struct mtk_jpeg_dev *jpeg)
>   	src_buf = v4l2_m2m_src_buf_remove(ctx->fh.m2m_ctx);
>   	dst_buf = v4l2_m2m_dst_buf_remove(ctx->fh.m2m_ctx);
>   
> -	result_size = mtk_jpeg_enc_get_file_size(jpeg->reg_base);
> +	result_size = mtk_jpeg_enc_get_file_size(jpeg->reg_base,
> +			ctx->jpeg->support_34bit);
>   	vb2_set_plane_payload(&dst_buf->vb2_buf, 0, result_size);
>   
>   	buf_state = VB2_BUF_STATE_DONE;
> @@ -1764,6 +1769,7 @@ static void mtk_jpegdec_worker(struct work_struct *work)
>   	ctx->total_frame_num++;
>   	mtk_jpeg_dec_reset(comp_jpeg[hw_id]->reg_base);
>   	mtk_jpeg_dec_set_config(comp_jpeg[hw_id]->reg_base,
> +				jpeg->support_34bit,
>   				&jpeg_src_buf->dec_param,
>   				jpeg_src_buf->bs_size,
>   				&bs,
> diff --git a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.h b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.h
> index 8ba6e757e11a..bf65bb54a035 100644
> --- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.h
> +++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.h
> @@ -34,6 +34,8 @@
>   
>   #define MTK_JPEG_MAX_EXIF_SIZE	(64 * 1024)
>   
> +#define MTK_JPEG_ADDR_MASK GENMASK(1, 0)
> +
>   /**
>    * enum mtk_jpeg_ctx_state - states of the context state machine
>    * @MTK_JPEG_INIT:		current state is initialized
> @@ -209,6 +211,7 @@ struct mtk_jpegdec_comp_dev {
>    * @reg_decbase:	jpg decode register base addr
>    * @dec_hw_dev:	jpg decode hardware device
>    * @hw_index:		jpg hw index
> + * @support_34bit:	flag to check if support dma_address 34bit
>    */
>   struct mtk_jpeg_dev {
>   	struct mutex		lock;
> @@ -231,6 +234,7 @@ struct mtk_jpeg_dev {
>   	void __iomem *reg_decbase[MTK_JPEGDEC_HW_MAX];
>   	struct mtk_jpegdec_comp_dev *dec_hw_dev[MTK_JPEGDEC_HW_MAX];
>   	atomic_t hw_index;
> +	bool support_34bit;

...and you don't need to add this member here, as struct mtk_jpeg_dev
already has a pointer to mtk_jpeg_variant, which will contain the same
member from platform data.

Regards,
Angelo