* [PATCH 2/3] [media] coda: first step at error recovery
2017-04-05 13:09 [PATCH 1/3] [media] coda: use correct offset for mvcol buffer Lucas Stach
@ 2017-04-05 13:09 ` Lucas Stach
2017-04-05 13:49 ` Philipp Zabel
2017-04-05 13:09 ` [PATCH 3/3] [media] coda/imx-vdoa: always wait for job completion Lucas Stach
2017-04-05 13:49 ` [PATCH 1/3] [media] coda: use correct offset for mvcol buffer Philipp Zabel
2 siblings, 1 reply; 7+ messages in thread
From: Lucas Stach @ 2017-04-05 13:09 UTC (permalink / raw)
To: Philipp Zabel; +Cc: Mauro Carvalho Chehab, linux-media, kernel, patchwork-lst
This implements a simple handler for the case where decode did not finish
sucessfully. This might be helpful during normal streaming, but for now it
only handles the case where the context would deadlock with userspace,
i.e. userspace issued DEC_CMD_STOP and waits for EOS, but after the failed
decode run we would hold the context and wait for userspace to queue more
buffers.
Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
drivers/media/platform/coda/coda-bit.c | 20 ++++++++++++++++++++
drivers/media/platform/coda/coda-common.c | 3 +++
drivers/media/platform/coda/coda.h | 1 +
3 files changed, 24 insertions(+)
diff --git a/drivers/media/platform/coda/coda-bit.c b/drivers/media/platform/coda/coda-bit.c
index 36062fc494e3..6a088f9343bb 100644
--- a/drivers/media/platform/coda/coda-bit.c
+++ b/drivers/media/platform/coda/coda-bit.c
@@ -2113,12 +2113,32 @@ static void coda_finish_decode(struct coda_ctx *ctx)
ctx->display_idx = display_idx;
}
+static void coda_error_decode(struct coda_ctx *ctx)
+{
+ struct vb2_v4l2_buffer *dst_buf;
+
+ /*
+ * For now this only handles the case where we would deadlock with
+ * userspace, i.e. userspace issued DEC_CMD_STOP and waits for EOS,
+ * but after a failed decode run we would hold the context and wait for
+ * userspace to queue more buffers.
+ */
+ if (!(ctx->bit_stream_param & CODA_BIT_STREAM_END_FLAG))
+ return;
+
+ dst_buf = v4l2_m2m_dst_buf_remove(ctx->fh.m2m_ctx);
+ dst_buf->sequence = ctx->qsequence - 1;
+
+ coda_m2m_buf_done(ctx, dst_buf, VB2_BUF_STATE_ERROR);
+}
+
const struct coda_context_ops coda_bit_decode_ops = {
.queue_init = coda_decoder_queue_init,
.reqbufs = coda_decoder_reqbufs,
.start_streaming = coda_start_decoding,
.prepare_run = coda_prepare_decode,
.finish_run = coda_finish_decode,
+ .error_run = coda_error_decode,
.seq_end_work = coda_seq_end_work,
.release = coda_bit_release,
};
diff --git a/drivers/media/platform/coda/coda-common.c b/drivers/media/platform/coda/coda-common.c
index eb6548f46cba..0bbf155f9783 100644
--- a/drivers/media/platform/coda/coda-common.c
+++ b/drivers/media/platform/coda/coda-common.c
@@ -1100,6 +1100,9 @@ static void coda_pic_run_work(struct work_struct *work)
ctx->hold = true;
coda_hw_reset(ctx);
+
+ if (ctx->ops->error_run)
+ ctx->ops->error_run(ctx);
} else if (!ctx->aborting) {
ctx->ops->finish_run(ctx);
}
diff --git a/drivers/media/platform/coda/coda.h b/drivers/media/platform/coda/coda.h
index 4b831c91ae4a..799ffca72203 100644
--- a/drivers/media/platform/coda/coda.h
+++ b/drivers/media/platform/coda/coda.h
@@ -180,6 +180,7 @@ struct coda_context_ops {
int (*start_streaming)(struct coda_ctx *ctx);
int (*prepare_run)(struct coda_ctx *ctx);
void (*finish_run)(struct coda_ctx *ctx);
+ void (*error_run)(struct coda_ctx *ctx);
void (*seq_end_work)(struct work_struct *work);
void (*release)(struct coda_ctx *ctx);
};
--
2.11.0
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH 2/3] [media] coda: first step at error recovery
2017-04-05 13:09 ` [PATCH 2/3] [media] coda: first step at error recovery Lucas Stach
@ 2017-04-05 13:49 ` Philipp Zabel
2017-06-07 7:21 ` Hans Verkuil
0 siblings, 1 reply; 7+ messages in thread
From: Philipp Zabel @ 2017-04-05 13:49 UTC (permalink / raw)
To: Lucas Stach; +Cc: Mauro Carvalho Chehab, linux-media, kernel, patchwork-lst
Hi Lucas,
On Wed, 2017-04-05 at 15:09 +0200, Lucas Stach wrote:
> This implements a simple handler for the case where decode did not finish
> sucessfully. This might be helpful during normal streaming, but for now it
> only handles the case where the context would deadlock with userspace,
> i.e. userspace issued DEC_CMD_STOP and waits for EOS, but after the failed
> decode run we would hold the context and wait for userspace to queue more
> buffers.
>
> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
Just a naming nitpick below.
Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>
> ---
> drivers/media/platform/coda/coda-bit.c | 20 ++++++++++++++++++++
> drivers/media/platform/coda/coda-common.c | 3 +++
> drivers/media/platform/coda/coda.h | 1 +
> 3 files changed, 24 insertions(+)
>
> diff --git a/drivers/media/platform/coda/coda-bit.c b/drivers/media/platform/coda/coda-bit.c
> index 36062fc494e3..6a088f9343bb 100644
> --- a/drivers/media/platform/coda/coda-bit.c
> +++ b/drivers/media/platform/coda/coda-bit.c
> @@ -2113,12 +2113,32 @@ static void coda_finish_decode(struct coda_ctx *ctx)
> ctx->display_idx = display_idx;
> }
>
> +static void coda_error_decode(struct coda_ctx *ctx)
This sounds a bit like we are decoding an error code. Could we maybe
rename this any of coda_fail_decode or coda_decode_error/failure or
similar?
> +{
> + struct vb2_v4l2_buffer *dst_buf;
> +
> + /*
> + * For now this only handles the case where we would deadlock with
> + * userspace, i.e. userspace issued DEC_CMD_STOP and waits for EOS,
> + * but after a failed decode run we would hold the context and wait for
> + * userspace to queue more buffers.
> + */
> + if (!(ctx->bit_stream_param & CODA_BIT_STREAM_END_FLAG))
> + return;
> +
> + dst_buf = v4l2_m2m_dst_buf_remove(ctx->fh.m2m_ctx);
> + dst_buf->sequence = ctx->qsequence - 1;
> +
> + coda_m2m_buf_done(ctx, dst_buf, VB2_BUF_STATE_ERROR);
> +}
> +
> const struct coda_context_ops coda_bit_decode_ops = {
> .queue_init = coda_decoder_queue_init,
> .reqbufs = coda_decoder_reqbufs,
> .start_streaming = coda_start_decoding,
> .prepare_run = coda_prepare_decode,
> .finish_run = coda_finish_decode,
> + .error_run = coda_error_decode,
How about .fail_run to follow the <verb>_run pattern, or
.run_error/failure to break it?
> .seq_end_work = coda_seq_end_work,
> .release = coda_bit_release,
> };
> diff --git a/drivers/media/platform/coda/coda-common.c b/drivers/media/platform/coda/coda-common.c
> index eb6548f46cba..0bbf155f9783 100644
> --- a/drivers/media/platform/coda/coda-common.c
> +++ b/drivers/media/platform/coda/coda-common.c
> @@ -1100,6 +1100,9 @@ static void coda_pic_run_work(struct work_struct *work)
> ctx->hold = true;
>
> coda_hw_reset(ctx);
> +
> + if (ctx->ops->error_run)
> + ctx->ops->error_run(ctx);
> } else if (!ctx->aborting) {
> ctx->ops->finish_run(ctx);
> }
> diff --git a/drivers/media/platform/coda/coda.h b/drivers/media/platform/coda/coda.h
> index 4b831c91ae4a..799ffca72203 100644
> --- a/drivers/media/platform/coda/coda.h
> +++ b/drivers/media/platform/coda/coda.h
> @@ -180,6 +180,7 @@ struct coda_context_ops {
> int (*start_streaming)(struct coda_ctx *ctx);
> int (*prepare_run)(struct coda_ctx *ctx);
> void (*finish_run)(struct coda_ctx *ctx);
> + void (*error_run)(struct coda_ctx *ctx);
> void (*seq_end_work)(struct work_struct *work);
> void (*release)(struct coda_ctx *ctx);
> };
regards
Philipp
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH 2/3] [media] coda: first step at error recovery
2017-04-05 13:49 ` Philipp Zabel
@ 2017-06-07 7:21 ` Hans Verkuil
0 siblings, 0 replies; 7+ messages in thread
From: Hans Verkuil @ 2017-06-07 7:21 UTC (permalink / raw)
To: Philipp Zabel, Lucas Stach
Cc: Mauro Carvalho Chehab, linux-media, kernel, patchwork-lst
Lucas,
Can you address these nitpicks?
Thanks!
Hans
On 05/04/17 15:49, Philipp Zabel wrote:
> Hi Lucas,
>
> On Wed, 2017-04-05 at 15:09 +0200, Lucas Stach wrote:
>> This implements a simple handler for the case where decode did not finish
>> sucessfully. This might be helpful during normal streaming, but for now it
>> only handles the case where the context would deadlock with userspace,
>> i.e. userspace issued DEC_CMD_STOP and waits for EOS, but after the failed
>> decode run we would hold the context and wait for userspace to queue more
>> buffers.
>>
>> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
>
> Just a naming nitpick below.
>
> Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>
>
>> ---
>> drivers/media/platform/coda/coda-bit.c | 20 ++++++++++++++++++++
>> drivers/media/platform/coda/coda-common.c | 3 +++
>> drivers/media/platform/coda/coda.h | 1 +
>> 3 files changed, 24 insertions(+)
>>
>> diff --git a/drivers/media/platform/coda/coda-bit.c b/drivers/media/platform/coda/coda-bit.c
>> index 36062fc494e3..6a088f9343bb 100644
>> --- a/drivers/media/platform/coda/coda-bit.c
>> +++ b/drivers/media/platform/coda/coda-bit.c
>> @@ -2113,12 +2113,32 @@ static void coda_finish_decode(struct coda_ctx *ctx)
>> ctx->display_idx = display_idx;
>> }
>>
>> +static void coda_error_decode(struct coda_ctx *ctx)
>
> This sounds a bit like we are decoding an error code. Could we maybe
> rename this any of coda_fail_decode or coda_decode_error/failure or
> similar?
>
>> +{
>> + struct vb2_v4l2_buffer *dst_buf;
>> +
>> + /*
>> + * For now this only handles the case where we would deadlock with
>> + * userspace, i.e. userspace issued DEC_CMD_STOP and waits for EOS,
>> + * but after a failed decode run we would hold the context and wait for
>> + * userspace to queue more buffers.
>> + */
>> + if (!(ctx->bit_stream_param & CODA_BIT_STREAM_END_FLAG))
>> + return;
>> +
>> + dst_buf = v4l2_m2m_dst_buf_remove(ctx->fh.m2m_ctx);
>> + dst_buf->sequence = ctx->qsequence - 1;
>> +
>> + coda_m2m_buf_done(ctx, dst_buf, VB2_BUF_STATE_ERROR);
>> +}
>> +
>> const struct coda_context_ops coda_bit_decode_ops = {
>> .queue_init = coda_decoder_queue_init,
>> .reqbufs = coda_decoder_reqbufs,
>> .start_streaming = coda_start_decoding,
>> .prepare_run = coda_prepare_decode,
>> .finish_run = coda_finish_decode,
>> + .error_run = coda_error_decode,
>
> How about .fail_run to follow the <verb>_run pattern, or
> .run_error/failure to break it?
>
>> .seq_end_work = coda_seq_end_work,
>> .release = coda_bit_release,
>> };
>> diff --git a/drivers/media/platform/coda/coda-common.c b/drivers/media/platform/coda/coda-common.c
>> index eb6548f46cba..0bbf155f9783 100644
>> --- a/drivers/media/platform/coda/coda-common.c
>> +++ b/drivers/media/platform/coda/coda-common.c
>> @@ -1100,6 +1100,9 @@ static void coda_pic_run_work(struct work_struct *work)
>> ctx->hold = true;
>>
>> coda_hw_reset(ctx);
>> +
>> + if (ctx->ops->error_run)
>> + ctx->ops->error_run(ctx);
>> } else if (!ctx->aborting) {
>> ctx->ops->finish_run(ctx);
>> }
>> diff --git a/drivers/media/platform/coda/coda.h b/drivers/media/platform/coda/coda.h
>> index 4b831c91ae4a..799ffca72203 100644
>> --- a/drivers/media/platform/coda/coda.h
>> +++ b/drivers/media/platform/coda/coda.h
>> @@ -180,6 +180,7 @@ struct coda_context_ops {
>> int (*start_streaming)(struct coda_ctx *ctx);
>> int (*prepare_run)(struct coda_ctx *ctx);
>> void (*finish_run)(struct coda_ctx *ctx);
>> + void (*error_run)(struct coda_ctx *ctx);
>> void (*seq_end_work)(struct work_struct *work);
>> void (*release)(struct coda_ctx *ctx);
>> };
>
> regards
> Philipp
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 3/3] [media] coda/imx-vdoa: always wait for job completion
2017-04-05 13:09 [PATCH 1/3] [media] coda: use correct offset for mvcol buffer Lucas Stach
2017-04-05 13:09 ` [PATCH 2/3] [media] coda: first step at error recovery Lucas Stach
@ 2017-04-05 13:09 ` Lucas Stach
2017-04-05 14:13 ` Philipp Zabel
2017-04-05 13:49 ` [PATCH 1/3] [media] coda: use correct offset for mvcol buffer Philipp Zabel
2 siblings, 1 reply; 7+ messages in thread
From: Lucas Stach @ 2017-04-05 13:09 UTC (permalink / raw)
To: Philipp Zabel; +Cc: Mauro Carvalho Chehab, linux-media, kernel, patchwork-lst
As long as only one CODA context is running we get alternating device_run()
and wait_for_completion() calls, but when more then one CODA context is
active, other VDOA slots can be inserted between those calls for one context.
Make sure to wait on job completion before running a different context and
before destroying the currently active context.
Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
drivers/media/platform/coda/imx-vdoa.c | 49 +++++++++++++++++++++++-----------
1 file changed, 33 insertions(+), 16 deletions(-)
diff --git a/drivers/media/platform/coda/imx-vdoa.c b/drivers/media/platform/coda/imx-vdoa.c
index 67fd8ffa60a4..ab69a0a9d38b 100644
--- a/drivers/media/platform/coda/imx-vdoa.c
+++ b/drivers/media/platform/coda/imx-vdoa.c
@@ -101,6 +101,8 @@ struct vdoa_ctx {
struct vdoa_data *vdoa;
struct completion completion;
struct vdoa_q_data q_data[2];
+ unsigned int submitted_job;
+ unsigned int completed_job;
};
static irqreturn_t vdoa_irq_handler(int irq, void *data)
@@ -114,7 +116,7 @@ static irqreturn_t vdoa_irq_handler(int irq, void *data)
curr_ctx = vdoa->curr_ctx;
if (!curr_ctx) {
- dev_dbg(vdoa->dev,
+ dev_warn(vdoa->dev,
"Instance released before the end of transaction\n");
return IRQ_HANDLED;
}
@@ -127,19 +129,44 @@ static irqreturn_t vdoa_irq_handler(int irq, void *data)
} else if (!(val & VDOAIST_EOT)) {
dev_warn(vdoa->dev, "Spurious interrupt\n");
}
+ curr_ctx->completed_job++;
complete(&curr_ctx->completion);
return IRQ_HANDLED;
}
+int vdoa_wait_for_completion(struct vdoa_ctx *ctx)
+{
+ struct vdoa_data *vdoa = ctx->vdoa;
+
+ if (ctx->submitted_job == ctx->completed_job)
+ return 0;
+
+ if (!wait_for_completion_timeout(&ctx->completion,
+ msecs_to_jiffies(300))) {
+ dev_err(vdoa->dev,
+ "Timeout waiting for transfer result\n");
+ return -ETIMEDOUT;
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL(vdoa_wait_for_completion);
+
void vdoa_device_run(struct vdoa_ctx *ctx, dma_addr_t dst, dma_addr_t src)
{
struct vdoa_q_data *src_q_data, *dst_q_data;
struct vdoa_data *vdoa = ctx->vdoa;
u32 val;
+ if (vdoa->curr_ctx)
+ vdoa_wait_for_completion(vdoa->curr_ctx);
+
vdoa->curr_ctx = ctx;
+ reinit_completion(&ctx->completion);
+ ctx->submitted_job++;
+
src_q_data = &ctx->q_data[V4L2_M2M_SRC];
dst_q_data = &ctx->q_data[V4L2_M2M_DST];
@@ -177,21 +204,6 @@ void vdoa_device_run(struct vdoa_ctx *ctx, dma_addr_t dst, dma_addr_t src)
}
EXPORT_SYMBOL(vdoa_device_run);
-int vdoa_wait_for_completion(struct vdoa_ctx *ctx)
-{
- struct vdoa_data *vdoa = ctx->vdoa;
-
- if (!wait_for_completion_timeout(&ctx->completion,
- msecs_to_jiffies(300))) {
- dev_err(vdoa->dev,
- "Timeout waiting for transfer result\n");
- return -ETIMEDOUT;
- }
-
- return 0;
-}
-EXPORT_SYMBOL(vdoa_wait_for_completion);
-
struct vdoa_ctx *vdoa_context_create(struct vdoa_data *vdoa)
{
struct vdoa_ctx *ctx;
@@ -218,6 +230,11 @@ void vdoa_context_destroy(struct vdoa_ctx *ctx)
{
struct vdoa_data *vdoa = ctx->vdoa;
+ if (vdoa->curr_ctx == ctx) {
+ vdoa_wait_for_completion(vdoa->curr_ctx);
+ vdoa->curr_ctx = NULL;
+ }
+
clk_disable_unprepare(vdoa->vdoa_clk);
kfree(ctx);
}
--
2.11.0
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH 3/3] [media] coda/imx-vdoa: always wait for job completion
2017-04-05 13:09 ` [PATCH 3/3] [media] coda/imx-vdoa: always wait for job completion Lucas Stach
@ 2017-04-05 14:13 ` Philipp Zabel
0 siblings, 0 replies; 7+ messages in thread
From: Philipp Zabel @ 2017-04-05 14:13 UTC (permalink / raw)
To: Lucas Stach; +Cc: Mauro Carvalho Chehab, linux-media, kernel, patchwork-lst
On Wed, 2017-04-05 at 15:09 +0200, Lucas Stach wrote:
> As long as only one CODA context is running we get alternating device_run()
> and wait_for_completion() calls, but when more then one CODA context is
> active, other VDOA slots can be inserted between those calls for one context.
>
> Make sure to wait on job completion before running a different context and
> before destroying the currently active context.
>
> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>
> ---
> drivers/media/platform/coda/imx-vdoa.c | 49 +++++++++++++++++++++++-----------
> 1 file changed, 33 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/media/platform/coda/imx-vdoa.c b/drivers/media/platform/coda/imx-vdoa.c
> index 67fd8ffa60a4..ab69a0a9d38b 100644
> --- a/drivers/media/platform/coda/imx-vdoa.c
> +++ b/drivers/media/platform/coda/imx-vdoa.c
> @@ -101,6 +101,8 @@ struct vdoa_ctx {
> struct vdoa_data *vdoa;
> struct completion completion;
> struct vdoa_q_data q_data[2];
> + unsigned int submitted_job;
> + unsigned int completed_job;
> };
>
> static irqreturn_t vdoa_irq_handler(int irq, void *data)
> @@ -114,7 +116,7 @@ static irqreturn_t vdoa_irq_handler(int irq, void *data)
>
> curr_ctx = vdoa->curr_ctx;
> if (!curr_ctx) {
> - dev_dbg(vdoa->dev,
> + dev_warn(vdoa->dev,
> "Instance released before the end of transaction\n");
> return IRQ_HANDLED;
> }
> @@ -127,19 +129,44 @@ static irqreturn_t vdoa_irq_handler(int irq, void *data)
> } else if (!(val & VDOAIST_EOT)) {
> dev_warn(vdoa->dev, "Spurious interrupt\n");
> }
> + curr_ctx->completed_job++;
> complete(&curr_ctx->completion);
>
> return IRQ_HANDLED;
> }
>
> +int vdoa_wait_for_completion(struct vdoa_ctx *ctx)
> +{
> + struct vdoa_data *vdoa = ctx->vdoa;
> +
> + if (ctx->submitted_job == ctx->completed_job)
> + return 0;
> +
> + if (!wait_for_completion_timeout(&ctx->completion,
> + msecs_to_jiffies(300))) {
> + dev_err(vdoa->dev,
> + "Timeout waiting for transfer result\n");
> + return -ETIMEDOUT;
> + }
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(vdoa_wait_for_completion);
> +
> void vdoa_device_run(struct vdoa_ctx *ctx, dma_addr_t dst, dma_addr_t src)
> {
> struct vdoa_q_data *src_q_data, *dst_q_data;
> struct vdoa_data *vdoa = ctx->vdoa;
> u32 val;
>
> + if (vdoa->curr_ctx)
> + vdoa_wait_for_completion(vdoa->curr_ctx);
> +
> vdoa->curr_ctx = ctx;
>
> + reinit_completion(&ctx->completion);
> + ctx->submitted_job++;
> +
> src_q_data = &ctx->q_data[V4L2_M2M_SRC];
> dst_q_data = &ctx->q_data[V4L2_M2M_DST];
>
> @@ -177,21 +204,6 @@ void vdoa_device_run(struct vdoa_ctx *ctx, dma_addr_t dst, dma_addr_t src)
> }
> EXPORT_SYMBOL(vdoa_device_run);
>
> -int vdoa_wait_for_completion(struct vdoa_ctx *ctx)
> -{
> - struct vdoa_data *vdoa = ctx->vdoa;
> -
> - if (!wait_for_completion_timeout(&ctx->completion,
> - msecs_to_jiffies(300))) {
> - dev_err(vdoa->dev,
> - "Timeout waiting for transfer result\n");
> - return -ETIMEDOUT;
> - }
> -
> - return 0;
> -}
> -EXPORT_SYMBOL(vdoa_wait_for_completion);
> -
> struct vdoa_ctx *vdoa_context_create(struct vdoa_data *vdoa)
> {
> struct vdoa_ctx *ctx;
> @@ -218,6 +230,11 @@ void vdoa_context_destroy(struct vdoa_ctx *ctx)
> {
> struct vdoa_data *vdoa = ctx->vdoa;
>
> + if (vdoa->curr_ctx == ctx) {
> + vdoa_wait_for_completion(vdoa->curr_ctx);
> + vdoa->curr_ctx = NULL;
> + }
> +
> clk_disable_unprepare(vdoa->vdoa_clk);
> kfree(ctx);
> }
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/3] [media] coda: use correct offset for mvcol buffer
2017-04-05 13:09 [PATCH 1/3] [media] coda: use correct offset for mvcol buffer Lucas Stach
2017-04-05 13:09 ` [PATCH 2/3] [media] coda: first step at error recovery Lucas Stach
2017-04-05 13:09 ` [PATCH 3/3] [media] coda/imx-vdoa: always wait for job completion Lucas Stach
@ 2017-04-05 13:49 ` Philipp Zabel
2 siblings, 0 replies; 7+ messages in thread
From: Philipp Zabel @ 2017-04-05 13:49 UTC (permalink / raw)
To: Lucas Stach; +Cc: Mauro Carvalho Chehab, linux-media, kernel, patchwork-lst
On Wed, 2017-04-05 at 15:09 +0200, Lucas Stach wrote:
> The mvcol buffer needs to be placed behind the chroma plane(s), so
> use the real offset including any required rounding.
>
> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>
regards
Philipp
> ---
> drivers/media/platform/coda/coda-bit.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/media/platform/coda/coda-bit.c b/drivers/media/platform/coda/coda-bit.c
> index 466a44e4549e..36062fc494e3 100644
> --- a/drivers/media/platform/coda/coda-bit.c
> +++ b/drivers/media/platform/coda/coda-bit.c
> @@ -387,14 +387,16 @@ static int coda_alloc_framebuffers(struct coda_ctx *ctx,
>
> /* Register frame buffers in the parameter buffer */
> for (i = 0; i < ctx->num_internal_frames; i++) {
> - u32 y, cb, cr;
> + u32 y, cb, cr, mvcol;
>
> /* Start addresses of Y, Cb, Cr planes */
> y = ctx->internal_frames[i].paddr;
> cb = y + ysize;
> cr = y + ysize + ysize/4;
> + mvcol = y + ysize + ysize/4 + ysize/4;
> if (ctx->tiled_map_type == GDI_TILED_FRAME_MB_RASTER_MAP) {
> cb = round_up(cb, 4096);
> + mvcol = cb + ysize/2;
> cr = 0;
> /* Packed 20-bit MSB of base addresses */
> /* YYYYYCCC, CCyyyyyc, cccc.... */
> @@ -408,9 +410,7 @@ static int coda_alloc_framebuffers(struct coda_ctx *ctx,
> /* mvcol buffer for h.264 */
> if (ctx->codec->src_fourcc == V4L2_PIX_FMT_H264 &&
> dev->devtype->product != CODA_DX6)
> - coda_parabuf_write(ctx, 96 + i,
> - ctx->internal_frames[i].paddr +
> - ysize + ysize/4 + ysize/4);
> + coda_parabuf_write(ctx, 96 + i, mvcol);
> }
>
> /* mvcol buffer for mpeg4 */
^ permalink raw reply [flat|nested] 7+ messages in thread