On Mon, Aug 16, 2021 at 06:59:29PM +0800, Irui Wang wrote:
> --- a/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc.c
> +++ b/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc.c
> @@ -16,6 +16,7 @@
> #include "mtk_vcodec_intr.h"
> #include "mtk_vcodec_util.h"
> #include "venc_drv_if.h"
> +#include "mtk_vcodec_enc_pm.h"
Please try to maintain the order.
> @@ -285,11 +291,12 @@ static int fops_vcodec_release(struct file *file)
> mtk_v4l2_debug(1, "[%d] encoder", ctx->id);
> mutex_lock(&dev->dev_mutex);
>
> + v4l2_m2m_ctx_release(ctx->m2m_ctx);
> mtk_vcodec_enc_release(ctx);
> v4l2_fh_del(&ctx->fh);
> v4l2_fh_exit(&ctx->fh);
> v4l2_ctrl_handler_free(&ctx->ctrl_hdl);
> - v4l2_m2m_ctx_release(ctx->m2m_ctx);
> + mtk_vcodec_enc_power_off(ctx);
Any reason to move the v4l2_m2m_ctx_release()?
> --- a/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc_pm.c
> +++ b/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc_pm.c
> @@ -12,6 +12,7 @@
>
> #include "mtk_vcodec_enc_pm.h"
> #include "mtk_vcodec_util.h"
> +#include "mtk_vcodec_enc_hw.h"
Please try to maintain the order.
> +int mtk_venc_enable_comp_hw(struct mtk_vcodec_dev *dev)
> +{
> + int i, ret;
> + struct mtk_venc_comp_dev *venc_comp;
> +
> + /*
> + * frame_racing mode needs power on all available component devices.
> + */
> + for (i = 0; i < MTK_VENC_HW_MAX; i++) {
> + venc_comp = (struct mtk_venc_comp_dev *)dev->enc_comp_dev[i];
> + if (!venc_comp)
> + return 0;
> +
> + ret = mtk_smi_larb_get(venc_comp->pm.larbvenc);
> + if (ret < 0) {
> + mtk_v4l2_err("power on core[%d] fail %d", i, ret);
> + goto pw_err;
The goto statement has extra indent layer.
> +int mtk_venc_disable_comp_hw(struct mtk_vcodec_dev *dev)
> +{
> + int i;
> + struct mtk_venc_comp_dev *venc_comp;
> +
> + /*power off all available component device*/
Need extra space between /* */.
> +int mtk_vcodec_enc_power_on(struct mtk_vcodec_ctx *ctx)
> +{
> + int ret;
> + struct mtk_vcodec_dev *dev = ctx->dev;
> +
> + if (dev->venc_pdata->hw_mode == VENC_FRAME_RACING_MODE) {
> + ret = mtk_venc_enable_comp_hw(dev);
> + if (ret < 0) {
> + mtk_v4l2_err("enable venc comp hw fail :%d", ret);
> + return ret;
> + }
> + } else {
> + ret = mtk_smi_larb_get(dev->pm.larbvenc);
> + if (ret < 0) {
> + mtk_v4l2_err("pm_runtime_resume_and_get fail %d", ret);
> + return ret;
> + }
> + }
> + return 0;
> +}
It seems the function doesn't need struct mtk_vcodec_ctx.
Does mtk_vcodec_enc_power_on(struct mtk_vcodec_dev* ) make more sense?
> +int mtk_vcodec_enc_power_off(struct mtk_vcodec_ctx *ctx)
> +{
> + struct mtk_vcodec_dev *dev = ctx->dev;
> +
> + if (dev->venc_pdata->hw_mode == VENC_FRAME_RACING_MODE)
> + mtk_venc_disable_comp_hw(dev);
> + else
> + mtk_smi_larb_put(dev->pm.larbvenc);
> +
> + return 0;
> +}
It seems the function doesn't need struct mtk_vcodec_ctx.
