* [PATCH] [media] s5p-mfc: Align stream buffer and CPB buffer to 512
[not found] <CGME20170118094212epcas5p22e588016d2b330dcd0b99b6e1012c744@epcas5p2.samsung.com>
@ 2017-01-18 9:37 ` Smitha T Murthy
2017-01-18 14:37 ` Andrzej Hajda
0 siblings, 1 reply; 3+ messages in thread
From: Smitha T Murthy @ 2017-01-18 9:37 UTC (permalink / raw)
To: linux-arm-kernel, linux-media, linux-kernel
Cc: kyungmin.park, kamil, jtp.park, a.hajda, mchehab, pankaj.dubey,
krzk, m.szyprowski, Smitha T Murthy
>From MFCv6 onwards encoder stream buffer and decoder CPB buffer
need to be aligned with 512.
Signed-off-by: Smitha T Murthy <smitha.t@samsung.com>
---
drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c | 9 +++++++++
drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.h | 3 +++
2 files changed, 12 insertions(+), 0 deletions(-)
diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c b/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c
index d6f207e..57da798 100644
--- a/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c
@@ -408,8 +408,15 @@ static int s5p_mfc_set_dec_stream_buffer_v6(struct s5p_mfc_ctx *ctx,
struct s5p_mfc_dev *dev = ctx->dev;
const struct s5p_mfc_regs *mfc_regs = dev->mfc_regs;
struct s5p_mfc_buf_size *buf_size = dev->variant->buf_size;
+ size_t cpb_buf_size;
mfc_debug_enter();
+ cpb_buf_size = ALIGN(buf_size->cpb, CPB_ALIGN);
+ if (strm_size >= set_strm_size_max(cpb_buf_size)) {
+ mfc_debug(2, "Decrease strm_size : %u -> %zu, gap : %d\n",
+ strm_size, set_strm_size_max(cpb_buf_size), CPB_ALIGN);
+ strm_size = set_strm_size_max(cpb_buf_size);
+ }
mfc_debug(2, "inst_no: %d, buf_addr: 0x%08x,\n"
"buf_size: 0x%08x (%d)\n",
ctx->inst_no, buf_addr, strm_size, strm_size);
@@ -519,6 +526,8 @@ static int s5p_mfc_set_enc_stream_buffer_v6(struct s5p_mfc_ctx *ctx,
struct s5p_mfc_dev *dev = ctx->dev;
const struct s5p_mfc_regs *mfc_regs = dev->mfc_regs;
+ size = ALIGN(size, 512);
+
writel(addr, mfc_regs->e_stream_buffer_addr); /* 16B align */
writel(size, mfc_regs->e_stream_buffer_size);
diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.h b/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.h
index 8055848..16a7b1d 100644
--- a/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.h
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.h
@@ -40,6 +40,9 @@
#define FRAME_DELTA_H264_H263 1
#define TIGHT_CBR_MAX 10
+#define CPB_ALIGN 512
+#define set_strm_size_max(cpb_max) ((cpb_max) - CPB_ALIGN)
+
struct s5p_mfc_hw_ops *s5p_mfc_init_hw_ops_v6(void);
const struct s5p_mfc_regs *s5p_mfc_init_regs_v6_plus(struct s5p_mfc_dev *dev);
#endif /* S5P_MFC_OPR_V6_H_ */
--
1.7.2.3
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] [media] s5p-mfc: Align stream buffer and CPB buffer to 512
2017-01-18 9:37 ` [PATCH] [media] s5p-mfc: Align stream buffer and CPB buffer to 512 Smitha T Murthy
@ 2017-01-18 14:37 ` Andrzej Hajda
2017-01-31 9:12 ` Smitha T Murthy
0 siblings, 1 reply; 3+ messages in thread
From: Andrzej Hajda @ 2017-01-18 14:37 UTC (permalink / raw)
To: Smitha T Murthy, linux-arm-kernel, linux-media, linux-kernel
Cc: kyungmin.park, kamil, jtp.park, mchehab, pankaj.dubey, krzk,
m.szyprowski
Hi Smitha,
On 18.01.2017 10:37, Smitha T Murthy wrote:
> >From MFCv6 onwards encoder stream buffer and decoder CPB buffer
Unexpected char at the beginning.
> need to be aligned with 512.
Patch below adds checks only if buffer size is multiple of 512, am I right?
If yes, please precise the subject, for example "...CPB buffer size need
to be...".
>
> Signed-off-by: Smitha T Murthy <smitha.t@samsung.com>
> ---
> drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c | 9 +++++++++
> drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.h | 3 +++
> 2 files changed, 12 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c b/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c
> index d6f207e..57da798 100644
> --- a/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c
> +++ b/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c
> @@ -408,8 +408,15 @@ static int s5p_mfc_set_dec_stream_buffer_v6(struct s5p_mfc_ctx *ctx,
> struct s5p_mfc_dev *dev = ctx->dev;
> const struct s5p_mfc_regs *mfc_regs = dev->mfc_regs;
> struct s5p_mfc_buf_size *buf_size = dev->variant->buf_size;
> + size_t cpb_buf_size;
>
> mfc_debug_enter();
> + cpb_buf_size = ALIGN(buf_size->cpb, CPB_ALIGN);
Since buf_size->cpb is constant of know size there is no need to align
it here.
> + if (strm_size >= set_strm_size_max(cpb_buf_size)) {
> + mfc_debug(2, "Decrease strm_size : %u -> %zu, gap : %d\n",
> + strm_size, set_strm_size_max(cpb_buf_size), CPB_ALIGN);
> + strm_size = set_strm_size_max(cpb_buf_size);
> + }
As I understand strm_size here is a size of buffer to be decoded, why it
cannot be equal to buf_size->cpb? Commit message says nothing about it.
> mfc_debug(2, "inst_no: %d, buf_addr: 0x%08x,\n"
> "buf_size: 0x%08x (%d)\n",
> ctx->inst_no, buf_addr, strm_size, strm_size);
> @@ -519,6 +526,8 @@ static int s5p_mfc_set_enc_stream_buffer_v6(struct s5p_mfc_ctx *ctx,
> struct s5p_mfc_dev *dev = ctx->dev;
> const struct s5p_mfc_regs *mfc_regs = dev->mfc_regs;
>
> + size = ALIGN(size, 512);
> +
Shouldn't be CPB_ALIGN instead of 512? And more importantly size is a
length of buffer for encoded stream,
by up-aligning you tell MFC that it can write beyond the buffer, it
could potentially overwrite random memory? Am I right?
> writel(addr, mfc_regs->e_stream_buffer_addr); /* 16B align */
> writel(size, mfc_regs->e_stream_buffer_size);
>
> diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.h b/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.h
> index 8055848..16a7b1d 100644
> --- a/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.h
> +++ b/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.h
> @@ -40,6 +40,9 @@
> #define FRAME_DELTA_H264_H263 1
> #define TIGHT_CBR_MAX 10
>
> +#define CPB_ALIGN 512
> +#define set_strm_size_max(cpb_max) ((cpb_max) - CPB_ALIGN)
Name of the macro is misleading.
Regards
Andrzej
> +
> struct s5p_mfc_hw_ops *s5p_mfc_init_hw_ops_v6(void);
> const struct s5p_mfc_regs *s5p_mfc_init_regs_v6_plus(struct s5p_mfc_dev *dev);
> #endif /* S5P_MFC_OPR_V6_H_ */
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] [media] s5p-mfc: Align stream buffer and CPB buffer to 512
2017-01-18 14:37 ` Andrzej Hajda
@ 2017-01-31 9:12 ` Smitha T Murthy
0 siblings, 0 replies; 3+ messages in thread
From: Smitha T Murthy @ 2017-01-31 9:12 UTC (permalink / raw)
To: Andrzej Hajda
Cc: linux-arm-kernel, linux-media, linux-kernel, kyungmin.park,
kamil, jtp.park, mchehab, pankaj.dubey, krzk, m.szyprowski
On Wed, 2017-01-18 at 15:37 +0100, Andrzej Hajda wrote:
> Hi Smitha,
>
> On 18.01.2017 10:37, Smitha T Murthy wrote:
> > >From MFCv6 onwards encoder stream buffer and decoder CPB buffer
>
> Unexpected char at the beginning.
>
> > need to be aligned with 512.
>
> Patch below adds checks only if buffer size is multiple of 512, am I right?
> If yes, please precise the subject, for example "...CPB buffer size need
> to be...".
>
Thank you for the review, after further analysis I found this patch is
not required. So I will drop it.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2017-01-31 9:38 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <CGME20170118094212epcas5p22e588016d2b330dcd0b99b6e1012c744@epcas5p2.samsung.com>
2017-01-18 9:37 ` [PATCH] [media] s5p-mfc: Align stream buffer and CPB buffer to 512 Smitha T Murthy
2017-01-18 14:37 ` Andrzej Hajda
2017-01-31 9:12 ` Smitha T Murthy
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).