diff mbox series

[2/6] media: mediatek: vcodec: add the definition of decoder status

Message ID 20230417054816.17097-3-yunfei.dong@mediatek.com
State New
Headers
Series media: mediatek: vcodec: Fix decoder under flow and plt test fails randomly |

Commit Message

Yunfei Dong (董云飞) April 17, 2023, 5:48 a.m. UTC 
  Adding the status used to separate different decoder period for
core hardware.

Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com>
---
 drivers/media/platform/mediatek/vcodec/vdec_msg_queue.h | 7 +++++++
 1 file changed, 7 insertions(+)

Comments

AngeloGioacchino Del Regno April 17, 2023, 9:26 a.m. UTC | #1 
Il 17/04/23 07:48, Yunfei Dong ha scritto:
> Adding the status used to separate different decoder period for
> core hardware.
> 
> Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com>
> ---
>   drivers/media/platform/mediatek/vcodec/vdec_msg_queue.h | 7 +++++++
>   1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/media/platform/mediatek/vcodec/vdec_msg_queue.h b/drivers/media/platform/mediatek/vcodec/vdec_msg_queue.h
> index a5d44bc97c16..19508be08566 100644
> --- a/drivers/media/platform/mediatek/vcodec/vdec_msg_queue.h
> +++ b/drivers/media/platform/mediatek/vcodec/vdec_msg_queue.h
> @@ -21,6 +21,13 @@ struct mtk_vcodec_ctx;
>   struct mtk_vcodec_dev;
>   typedef int (*core_decode_cb_t)(struct vdec_lat_buf *lat_buf);
>   
> +/* current context isn't work */
> +#define CONTEXT_LIST_EMPTY           (0)
> +/* queued to the core work list */
> +#define CONTEXT_LIST_QUEUED          (1)
> +/* context decode done */
> +#define CONTEXT_LIST_DEC_DONE        (2)

I would prefer an enumeration instead; you can keep the documentation for those
status signals with kerneldoc on the new enum.

/**
  * enum core_ctx_status - Context decode status for core hardware
  * @CONTEXT_LIST_EMPTY:    No buffer queued on Core HW (must always be 0)
  * @CONTEXT_LIST_QUEUED:   Buffer queued to the core work list
  * @CONTEXT_LIST_DEC_DONE: Context decode done
  */
enum core_ctx_status {
	CONTEXT_LIST_EMPTY = 0,
	CONTEXT_LIST_QUEUED,
	CONTEXT_LIST_DEC_DONE,
}

Moreover, since this is a rather simple addition, please squash this commit with
patch [3/6] where you actually also introduce the actual usage of the new enum.

Cheers,
Angelo
Yunfei Dong (董云飞) April 17, 2023, 12:10 p.m. UTC | #2 
Hi AngeloGioacchino,

Thanks for your suggestion.

On Mon, 2023-04-17 at 11:26 +0200, AngeloGioacchino Del Regno wrote:
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
> 
> 
> Il 17/04/23 07:48, Yunfei Dong ha scritto:
> > Adding the status used to separate different decoder period for
> > core hardware.
> > 
> > Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com>
> > ---
> >   drivers/media/platform/mediatek/vcodec/vdec_msg_queue.h | 7
> > +++++++
> >   1 file changed, 7 insertions(+)
> > 
> > diff --git
> > a/drivers/media/platform/mediatek/vcodec/vdec_msg_queue.h
> > b/drivers/media/platform/mediatek/vcodec/vdec_msg_queue.h
> > index a5d44bc97c16..19508be08566 100644
> > --- a/drivers/media/platform/mediatek/vcodec/vdec_msg_queue.h
> > +++ b/drivers/media/platform/mediatek/vcodec/vdec_msg_queue.h
> > @@ -21,6 +21,13 @@ struct mtk_vcodec_ctx;
> >   struct mtk_vcodec_dev;
> >   typedef int (*core_decode_cb_t)(struct vdec_lat_buf *lat_buf);
> > 
> > +/* current context isn't work */
> > +#define CONTEXT_LIST_EMPTY           (0)
> > +/* queued to the core work list */
> > +#define CONTEXT_LIST_QUEUED          (1)
> > +/* context decode done */
> > +#define CONTEXT_LIST_DEC_DONE        (2)
> 
> I would prefer an enumeration instead; you can keep the documentation
> for those
> status signals with kerneldoc on the new enum.
> 
> /**
>   * enum core_ctx_status - Context decode status for core hardware
>   * @CONTEXT_LIST_EMPTY:    No buffer queued on Core HW (must always
> be 0)
>   * @CONTEXT_LIST_QUEUED:   Buffer queued to the core work list
>   * @CONTEXT_LIST_DEC_DONE: Context decode done
>   */
> enum core_ctx_status {
>         CONTEXT_LIST_EMPTY = 0,
>         CONTEXT_LIST_QUEUED,
>         CONTEXT_LIST_DEC_DONE,
> }
> 
> Moreover, since this is a rather simple addition, please squash this
> commit with
> patch [3/6] where you actually also introduce the actual usage of the
> new enum.
> 
Fixed in patch v2.

Best Regards,
Yunfei Dong
> Cheers,
> Angelo
>
diff mbox series

Patch

diff --git a/drivers/media/platform/mediatek/vcodec/vdec_msg_queue.h b/drivers/media/platform/mediatek/vcodec/vdec_msg_queue.h
index a5d44bc97c16..19508be08566 100644
--- a/drivers/media/platform/mediatek/vcodec/vdec_msg_queue.h
+++ b/drivers/media/platform/mediatek/vcodec/vdec_msg_queue.h
@@ -21,6 +21,13 @@  struct mtk_vcodec_ctx;
 struct mtk_vcodec_dev;
 typedef int (*core_decode_cb_t)(struct vdec_lat_buf *lat_buf);
 
+/* current context isn't work */
+#define CONTEXT_LIST_EMPTY           (0)
+/* queued to the core work list */
+#define CONTEXT_LIST_QUEUED          (1)
+/* context decode done */
+#define CONTEXT_LIST_DEC_DONE        (2)
+
 /**
  * struct vdec_msg_queue_ctx - represents a queue for buffers ready to be processed
  * @ready_to_use: ready used queue used to signalize when get a job queue