Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [PATCH 2/3] venus: vdec: Make decoder return LAST flag for
 sufficient event
To:     vgarodia@codeaurora.org,
        Stanimir Varbanov <stanimir.varbanov@linaro.org>
Cc:     linux-media@vger.kernel.org, linux-arm-msm@vger.kernel.org,
        linux-kernel@vger.kernel.org,
        Alexandre Courbot <acourbot@chromium.org>,
        Mansur Alisha Shaik <mansur@codeaurora.org>
References: <20200928164431.21884-1-stanimir.varbanov@linaro.org>
 <20200928164431.21884-3-stanimir.varbanov@linaro.org>
 <5a823acc60d4c5cace1d2562adc548ff@codeaurora.org>
From:   Stanimir Varbanov <stanimir.varbanov@linaro.org>
Message-ID: <5af74a17-6a93-5ac2-533e-8fca5c8d2faf@linaro.org>
Date:   Sat, 10 Oct 2020 04:21:45 +0300
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101
 Thunderbird/68.10.0
MIME-Version: 1.0
In-Reply-To: <5a823acc60d4c5cace1d2562adc548ff@codeaurora.org>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 8bit
Precedence: bulk

Hi Vikash,

On 10/7/20 10:53 PM, vgarodia@codeaurora.org wrote:
> Hi Stan,
> 
> On 2020-09-28 22:14, Stanimir Varbanov wrote:
>> This makes the decoder to behaives equally for sufficient and
> behaves
> 
>> insufficient events. After this change the LAST buffer flag will be set
>> when the new resolution (in dynamic-resolution-change state) is smaller
>> then the old bitstream resolution.
>>
>> Signed-off-by: Stanimir Varbanov <stanimir.varbanov@linaro.org>
>> ---
>>  drivers/media/platform/qcom/venus/vdec.c | 41 ++++++++++++++++--------
>>  1 file changed, 27 insertions(+), 14 deletions(-)
>>
>> diff --git a/drivers/media/platform/qcom/venus/vdec.c
>> b/drivers/media/platform/qcom/venus/vdec.c
>> index c11bdf3ca21b..c006401255dc 100644
>> --- a/drivers/media/platform/qcom/venus/vdec.c
>> +++ b/drivers/media/platform/qcom/venus/vdec.c
>> @@ -634,6 +634,7 @@ static int vdec_output_conf(struct venus_inst *inst)
>>  {
>>      struct venus_core *core = inst->core;
>>      struct hfi_enable en = { .enable = 1 };
>> +    struct hfi_buffer_requirements bufreq;
>>      u32 width = inst->out_width;
>>      u32 height = inst->out_height;
>>      u32 out_fmt, out2_fmt;
>> @@ -709,6 +710,22 @@ static int vdec_output_conf(struct venus_inst *inst)
>>      }
>>
>>      if (IS_V3(core) || IS_V4(core)) {
>> +        ret = venus_helper_get_bufreq(inst, HFI_BUFFER_OUTPUT, &bufreq);
>> +        if (ret)
>> +            return ret;
>> +
>> +        if (bufreq.size > inst->output_buf_size)
>> +            return -EINVAL;
>> +
>> +        if (inst->dpb_fmt) {
>> +            ret = venus_helper_get_bufreq(inst, HFI_BUFFER_OUTPUT2,
>> &bufreq);
>> +            if (ret)
>> +                return ret;
>> +
>> +            if (bufreq.size > inst->output2_buf_size)
>> +                return -EINVAL;
>> +        }
>> +
>>          if (inst->output2_buf_size) {
>>              ret = venus_helper_set_bufsize(inst,
>>                                 inst->output2_buf_size,
>> @@ -1327,19 +1344,15 @@ static void vdec_event_change(struct
>> venus_inst *inst,
>>      dev_dbg(dev, VDBGM "event %s sufficient resources (%ux%u)\n",
>>          sufficient ? "" : "not", ev_data->width, ev_data->height);
>>
>> -    if (sufficient) {
>> -        hfi_session_continue(inst);
>> -    } else {
>> -        switch (inst->codec_state) {
>> -        case VENUS_DEC_STATE_INIT:
>> -            inst->codec_state = VENUS_DEC_STATE_CAPTURE_SETUP;
>> -            break;
>> -        case VENUS_DEC_STATE_DECODING:
>> -            inst->codec_state = VENUS_DEC_STATE_DRC;
>> -            break;
>> -        default:
>> -            break;
>> -        }
>> +    switch (inst->codec_state) {
>> +    case VENUS_DEC_STATE_INIT:
>> +        inst->codec_state = VENUS_DEC_STATE_CAPTURE_SETUP;
>> +        break;
>> +    case VENUS_DEC_STATE_DECODING:
>> +        inst->codec_state = VENUS_DEC_STATE_DRC;
> 
> Video firmware would raise reconfig event to driver even for cases like
> interlace detection, color space change in the bitstream. If not with
> this patch,
> we can optimize by sending reconfig event only for resolution and
> bitdepth update,
> in a followup patch.

Good point. Sure, I can do that in this series as separate patch.

> 
>> +        break;
>> +    default:
>> +        break;
>>      }
>>
>>      /*
>> @@ -1348,7 +1361,7 @@ static void vdec_event_change(struct venus_inst
>> *inst,
>>       * itself doesn't mark the last decoder output buffer with HFI
>> EOS flag.
>>       */
>>
>> -    if (!sufficient && inst->codec_state == VENUS_DEC_STATE_DRC) {
>> +    if (inst->codec_state == VENUS_DEC_STATE_DRC) {
>>          struct vb2_v4l2_buffer *last;
>>          int ret;

-- 
regards,
Stan