On 12/1/20 5:09 AM, Alexandre Courbot wrote:
> On Sat, Nov 28, 2020 at 3:40 PM Fritz Koenig <frkoenig@xxxxxxxxxxxx> wrote:
>>
>> If the DRC is near the end of the stream the client
>> may send a V4L2_DEC_CMD_STOP before the DRC occurs.
>> V4L2_DEC_CMD_STOP puts the driver into the
>> VENUS_DEC_STATE_DRAIN state. DRC must be aware so
>> that after the DRC event the state can be restored
>> correctly.
>>
>> Signed-off-by: Fritz Koenig <frkoenig@xxxxxxxxxxxx>
>>
>> ---
>>
>> This is an attempt to fix the logic for when a DRC occurs
>> after the driver is in VENUS_DEC_STATE_DRAIN state. This
>> works for me, but I'm not sure if I covered all the cases.
>> Specifically I'm not sure if I reset |drain_active| in all
>> the right places.
>>
>> drivers/media/platform/qcom/venus/core.h | 1 +
>> drivers/media/platform/qcom/venus/vdec.c | 19 +++++++++++++++++--
>> 2 files changed, 18 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/media/platform/qcom/venus/core.h b/drivers/media/platform/qcom/venus/core.h
>> index 2b0899bf5b05f..1680c936c06fb 100644
>> --- a/drivers/media/platform/qcom/venus/core.h
>> +++ b/drivers/media/platform/qcom/venus/core.h
>> @@ -406,6 +406,7 @@ struct venus_inst {
>> unsigned int core_acquired: 1;
>> unsigned int bit_depth;
>> bool next_buf_last;
>> + bool drain_active;
>> };
>>
>> #define IS_V1(core) ((core)->res->hfi_version == HFI_VERSION_1XX)
>> diff --git a/drivers/media/platform/qcom/venus/vdec.c b/drivers/media/platform/qcom/venus/vdec.c
>> index 5671cf3458a68..7edbefbd75210 100644
>> --- a/drivers/media/platform/qcom/venus/vdec.c
>> +++ b/drivers/media/platform/qcom/venus/vdec.c
>> @@ -523,8 +523,10 @@ vdec_decoder_cmd(struct file *file, void *fh, struct v4l2_decoder_cmd *cmd)
>>
>> ret = hfi_session_process_buf(inst, &fdata);
>>
>> - if (!ret && inst->codec_state == VENUS_DEC_STATE_DECODING)
>> + if (!ret && inst->codec_state == VENUS_DEC_STATE_DECODING) {
>> inst->codec_state = VENUS_DEC_STATE_DRAIN;
>> + inst->drain_active = true;
>> + }
>> }
>>
>> unlock:
>> @@ -976,10 +978,14 @@ static int vdec_start_capture(struct venus_inst *inst)
>>
>> inst->codec_state = VENUS_DEC_STATE_DECODING;
>>
>> + if (inst->drain_active)
>> + inst->codec_state = VENUS_DEC_STATE_DRAIN;
>> +
>> inst->streamon_cap = 1;
>> inst->sequence_cap = 0;
>> inst->reconfig = false;
>> inst->next_buf_last = false;
>> + inst->drain_active = false;
>>
>> return 0;
>>
>> @@ -1105,6 +1111,7 @@ static int vdec_stop_capture(struct venus_inst *inst)
>> /* fallthrough */
>> case VENUS_DEC_STATE_DRAIN:
>> inst->codec_state = VENUS_DEC_STATE_STOPPED;
>> + inst->drain_active = false;
>> /* fallthrough */
>> case VENUS_DEC_STATE_SEEK:
>> vdec_cancel_dst_buffers(inst);
>> @@ -1304,8 +1311,10 @@ static void vdec_buf_done(struct venus_inst *inst, unsigned int buf_type,
>>
>> v4l2_event_queue_fh(&inst->fh, &ev);
>>
>> - if (inst->codec_state == VENUS_DEC_STATE_DRAIN)
>> + if (inst->codec_state == VENUS_DEC_STATE_DRAIN) {
>> + inst->drain_active = false;
>> inst->codec_state = VENUS_DEC_STATE_STOPPED;
>> + }
>> }
>>
>> if (!bytesused)
>> @@ -1429,11 +1438,17 @@ static void vdec_event_notify(struct venus_inst *inst, u32 event,
>> case EVT_SYS_EVENT_CHANGE:
>> switch (data->event_type) {
>> case HFI_EVENT_DATA_SEQUENCE_CHANGED_SUFFICIENT_BUF_RESOURCES:
>> + if (inst->codec_state == VENUS_DEC_STATE_DRAIN)
>> + inst->codec_state = VENUS_DEC_STATE_DECODING;
>> vdec_event_change(inst, data, true);
>> break;
>> case HFI_EVENT_DATA_SEQUENCE_CHANGED_INSUFFICIENT_BUF_RESOURCES:
>> + if (inst->codec_state == VENUS_DEC_STATE_DRAIN)
>> + inst->codec_state = VENUS_DEC_STATE_DECODING;
>> vdec_event_change(inst, data, false);
>> break;
>> + // TODO(fritz) : does HFI_EVENT_RELEASE_BUFFER_REFERENCE also need to
>> + // change the codec_state to VENUS_DEC_STATE_DECODING?
>
> I don't think it does, but Stanimir can confirm probably. In any case
> we should remove this TODO in the next version. :)
No, we don't need state change for release buffer reference.
>
>> case HFI_EVENT_RELEASE_BUFFER_REFERENCE:
>> venus_helper_release_buf_ref(inst, data->tag);
>> break;
>> --
>> 2.29.2.454.gaff20da3a2-goog
>>
--
regards,
Stan
