Re: [V10,5/7] mtk-jpegenc: add jpeg encode worker interface

[AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>]

Il 14/07/22 10:51, kyrie.wu ha scritto:
> On Tue, 2022-07-05 at 15:27 +0200, AngeloGioacchino Del Regno wrote:
>> Il 27/06/22 04:56, Irui Wang ha scritto:
>>> From: kyrie wu <kyrie.wu@mediatek.com>
>>>
>>> Add jpeg encoding worker to ensure that two HWs
>>> run in parallel in MT8195.
>>>
>>> Signed-off-by: kyrie wu <kyrie.wu@mediatek.com>
>>> ---
>>>    .../platform/mediatek/jpeg/mtk_jpeg_core.c    | 208
>>> ++++++++++++++++--
>>>    .../platform/mediatek/jpeg/mtk_jpeg_core.h    |  30 ++-
>>>    .../platform/mediatek/jpeg/mtk_jpeg_enc_hw.c  |  39 +++-
>>>    3 files changed, 240 insertions(+), 37 deletions(-)
>>>
>>> diff --git a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
>>> b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
>>> index 0c5c85a112ca..544673a527a0 100644
>>> --- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
>>> +++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
>>
>> ..snip..
>>

..snip..

>>> @@ -218,30 +222,37 @@ static irqreturn_t
>>> mtk_jpegenc_hw_irq_handler(int irq, void *priv)
>>>    
>>>    	cancel_delayed_work(&jpeg->job_timeout_work);
>>>    
>>> +	ctx = jpeg->hw_param.curr_ctx;
>>> +	src_buf = jpeg->hw_param.src_buffer;
>>> +	dst_buf = jpeg->hw_param.dst_buffer;
>>> +	v4l2_m2m_buf_copy_metadata(src_buf, dst_buf, true);
>>> +
>>>    	irq_status = readl(jpeg->reg_base + JPEG_ENC_INT_STS) &
>>>    		JPEG_ENC_INT_STATUS_MASK_ALLIRQ;
>>>    	if (irq_status)
>>>    		writel(0, jpeg->reg_base + JPEG_ENC_INT_STS);
>>> -	if (!(irq_status & JPEG_ENC_INT_STATUS_DONE))
>>> -		return IRQ_NONE;
>>> -
>>> -	ctx = v4l2_m2m_get_curr_priv(master_jpeg->m2m_dev);
>>> -	if (!ctx) {
>>> -		v4l2_err(&master_jpeg->v4l2_dev, "Context is NULL\n");
>>> -		return IRQ_HANDLED;
>>> +	if (!(irq_status & JPEG_ENC_INT_STATUS_DONE)) {
>>> +		dev_err(jpeg->dev, " Not JPEG_ENC_INT_STATUS_DONE\n");
>>
>> Please be more descriptive in error messages. Also, should this
>> really be
>> a dev_err?!?
> maybe dev_warn is right, I will fix it.
> 

I think that v4l2_err() would be more appropriate.

Regards,
Angelo