Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Wed, 29 Mar 2023 16:36:45 +0300
From:   Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
To:     Vikash Garodia <quic_vgarodia@quicinc.com>,
        Andy Gross <agross@kernel.org>,
        Mauro Carvalho Chehab <mchehab@kernel.org>,
        linux-media@vger.kernel.org, linux-arm-msm@vger.kernel.org,
        linux-kernel@vger.kernel.org, stanimir.k.varbanov@gmail.com,
        andersson@kernel.org, konrad.dybcio@linaro.org
CC:     quic_dikshita@quicinc.com, Viswanath Boma <quic_vboma@quicinc.com>
Subject: =?US-ASCII?Q?Re=3A_=5BPATCH_v4=5D_venus=3A_Enable?= =?US-ASCII?Q?_sufficient_sequence_change_s?= =?US-ASCII?Q?upport_for_sc7180_and_fix_for_Decoder_STOP_command_issue=2E?=
User-Agent: K-9 Mail for Android
In-Reply-To: <9f5bce7e-2b8a-0b71-3a80-0b4f86d9f908@quicinc.com>
References: <20230323130153.8229-1-quic_vboma@quicinc.com> <c611c390-2cf3-2abe-82aa-67538b823d62@linaro.org> <9f5bce7e-2b8a-0b71-3a80-0b4f86d9f908@quicinc.com>
Message-ID: <E98548DB-5085-4036-9F6C-DC22A604A0C2@linaro.org>
MIME-Version: 1.0
Content-Type: text/plain;
 charset=utf-8
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

29 =D0=BC=D0=B0=D1=80=D1=82=D0=B0 2023 =D0=B3=2E 10:48:23 GMT+03:00, Vikash=
 Garodia <quic_vgarodia@quicinc=2Ecom> =D0=BF=D0=B8=D1=88=D0=B5=D1=82:
>On 3/29/2023 3:49 AM, Dmitry Baryshkov wrote:
>> On 23/03/2023 15:01, Viswanath Boma wrote:
>>> For VP9 bitstreams, there could be a change in resolution at interfram=
e,
>>> for driver to get notified of such resolution change,
>>> enable the property in video firmware=2E
>>> Also, EOS handling is now made same in video firmware across all V6 SO=
Cs,
>>> hence above a certain firmware version, the driver handling is
>>> made generic for all V6s
>>=20
>> Having "Do abc=2E Also do defgh=2E" is a clear sign that this patch sho=
uld be split into two=2E
>
>I agree, it could have split into patches=2E The patch introduces way to =
store venus firmware
>
>version and take some decision for various version=2E For ex=2E here STOP=
 handling and enabling
>
>DRC event for specific firmware revision and onwards=2E Since both the ha=
ndling was primarily
>
>dependent of firmware version, and since the handlings were smaller, it w=
as combined as single
>
>patch=2E Let me know, if you have any further review comments, else, will=
 raise a new version with
>
>2 patches probably=2E

Thanks!

>
>>>=20
>>> Signed-off-by: Vikash Garodia <vgarodia@qti=2Equalcomm=2Ecom>
>>> Signed-off-by: Viswanath Boma <quic_vboma@quicinc=2Ecom>
>>> Tested-by: Nathan Hebert <nhebert@chromium=2Eorg>
>>> ---
>>> Since v3 : Addressed comments to rectify email address=2E
>>>=20
>>> =C2=A0 drivers/media/platform/qcom/venus/core=2Eh=C2=A0=C2=A0=C2=A0=C2=
=A0=C2=A0=C2=A0 | 18 ++++++++++++++++++
>>> =C2=A0 drivers/media/platform/qcom/venus/hfi_cmds=2Ec=C2=A0=C2=A0 |=C2=
=A0 1 +
>>> =C2=A0 drivers/media/platform/qcom/venus/hfi_helper=2Eh |=C2=A0 2 ++
>>> =C2=A0 drivers/media/platform/qcom/venus/hfi_msgs=2Ec=C2=A0=C2=A0 | 11=
 +++++++++--
>>> =C2=A0 drivers/media/platform/qcom/venus/vdec=2Ec=C2=A0=C2=A0=C2=A0=C2=
=A0=C2=A0=C2=A0 | 12 +++++++++++-
>>> =C2=A0 5 files changed, 41 insertions(+), 3 deletions(-)
>>>=20

(Skipped)


>>> @@ -671,6 +671,16 @@ static int vdec_set_properties(struct venus_inst =
*inst)
>>> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0=C2=A0 return ret;
>>> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 }
>>> =C2=A0 +=C2=A0=C2=A0=C2=A0 /* Enabling sufficient sequence change supp=
ort for VP9 */
>>> +=C2=A0=C2=A0=C2=A0 if (of_device_is_compatible(inst->core->dev->of_no=
de, "qcom,sc7180-venus")) {
>>=20
>> Let me repeat my question from v3:
>>=20
>> Is it really specific just to sc7180 or will it be applicable to any
>> other platform using venus-5=2E4 firmware?
>
>The HFI "HFI_PROPERTY_PARAM_VDEC_ENABLE_SUFFICIENT_SEQCHANGE_EVENT" is im=
plemented
>
>only for sc7180=2E Calling this for any other venus-5=2E4 would error out=
 the session with error as
>
>unsupported property from firmware=2E


How can we be sure that other platforms do not end up using sc7180 firmwar=
e? Or that sc7180 didn't end up using some other firmware?

I see generic  qcom/venus-5=2E4/venus=2Embn in Linux firmware=2E It's vers=
ion is VIDEO=2EVE=2E5=2E4-00053-PROD-1=2E It can be used with any unfused d=
evice which uses firmware 5=2E4

>
>>=20
>>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 if (is_fw_rev_or_newer(ins=
t->core, 5, 4, 51)) {
>>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 pt=
ype =3D HFI_PROPERTY_PARAM_VDEC_ENABLE_SUFFICIENT_SEQCHANGE_EVENT;
>>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 re=
t =3D hfi_session_set_property(inst, ptype, &en);
>>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 if=
 (ret)
>>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0=C2=A0=C2=A0=C2=A0 return ret;
>>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 }
>>> +=C2=A0=C2=A0=C2=A0 }
>>> +
>>> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 ptype =3D HFI_PROPERTY_PARAM_VDEC_CONCE=
AL_COLOR;
>>> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 conceal =3D ctr->conceal_color & 0xffff=
;
>>> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 conceal |=3D ((ctr->conceal_color >> 16=
) & 0xffff) << 10;
>>=20