On Wed, Jun 30, 2021 at 3:32 PM kyrie.wu <kyrie.wu@xxxxxxxxxxxx> wrote: > modify jpegenc device run func interface and add worker for encode The description makes less sense. > +static void mtk_jpegenc_worker(struct work_struct *work) > { > - struct mtk_jpeg_ctx *ctx = priv; > + struct mtk_jpeg_ctx *ctx = container_of(work, struct mtk_jpeg_ctx, > + jpeg_work); > struct mtk_jpeg_dev *jpeg = ctx->jpeg; > + struct mtk_jpeg_dev *comp_jpeg[MTK_JPEGENC_HW_MAX]; > struct vb2_v4l2_buffer *src_buf, *dst_buf; > enum vb2_buffer_state buf_state = VB2_BUF_STATE_ERROR; > unsigned long flags; > - int ret; > + struct mtk_jpeg_src_buf *jpeg_src_buf, *jpeg_dst_buf; > + int ret, i, hw_id = 0; hw_id doesn't need to be initialized. > +retry_select: > + hw_id = mtk_jpeg_select_hw(ctx); > + if (hw_id < 0) { > + ret = wait_event_interruptible(jpeg->hw_wq, > + (atomic_read(hw_rdy[0]) || > + atomic_read(hw_rdy[1])) > 0); Hard-coded refers to hw_rdy[0] and hw_rdy[1] here? > - mtk_jpeg_enc_reset(jpeg->reg_base); > - > - mtk_jpeg_set_enc_src(ctx, jpeg->reg_base, &src_buf->vb2_buf); > - mtk_jpeg_set_enc_dst(ctx, jpeg->reg_base, &dst_buf->vb2_buf); > - mtk_jpeg_set_enc_params(ctx, jpeg->reg_base); > - mtk_jpeg_enc_start(jpeg->reg_base); > - spin_unlock_irqrestore(&jpeg->hw_lock, flags); > + spin_lock_irqsave(&comp_jpeg[hw_id]->hw_lock, flags); > + mtk_jpeg_enc_reset(comp_jpeg[hw_id]->reg_base[0]); > + mtk_jpeg_set_enc_dst(ctx, > + comp_jpeg[hw_id]->reg_base[0], > + &dst_buf->vb2_buf); > + mtk_jpeg_set_enc_src(ctx, > + comp_jpeg[hw_id]->reg_base[0], > + &src_buf->vb2_buf); > + mtk_jpeg_set_enc_params(ctx, comp_jpeg[hw_id]->reg_base[0]); > + mtk_jpeg_enc_start(comp_jpeg[hw_id]->reg_base[0]); Why it uses reg_base[0]?