mbox series

[v3,0/3] Add support for qcom msm8998-venus (HW vdec / venc)

Message ID 72860c1d-7434-4be6-8c1d-9ea177602802@freebox.fr
Headers show
Series Add support for qcom msm8998-venus (HW vdec / venc) | expand

Message

Marc Gonzalez May 6, 2024, 1:44 p.m. UTC 
Changes in v3
- Add Reviewed-by tags for patch 1
- Align msm8998_freq_table to downstream code

Marc Gonzalez (1):
  dt-bindings: media: add qcom,msm8998-venus

Pierre-Hugues Husson (2):
  arm64: dts: qcom: msm8998: add venus node
  media: venus: add msm8998 support

 Documentation/devicetree/bindings/media/qcom,msm8996-venus.yaml |  4 ++-
 arch/arm64/boot/dts/qcom/msm8998.dtsi                           | 48 +++++++++++++++++++++++++++++
 drivers/media/platform/qcom/venus/core.c                        | 48 +++++++++++++++++++++++++++++
 3 files changed, 99 insertions(+), 1 deletion(-)

Comments

Marc Gonzalez May 6, 2024, 2:51 p.m. UTC | #1 
On 06/05/2024 16:43, Vikash Garodia wrote:

> On 5/6/2024 7:17 PM, Marc Gonzalez wrote:
>
>> From: Pierre-Hugues Husson <phhusson@freebox.fr>
>>
>> Add the missing bits for msm8998 support.
>>
>> Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
>> Signed-off-by: Pierre-Hugues Husson <phhusson@freebox.fr>
>> Signed-off-by: Marc Gonzalez <mgonzalez@freebox.fr>
>> ---
>>  drivers/media/platform/qcom/venus/core.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 48 insertions(+)
>>
>> diff --git a/drivers/media/platform/qcom/venus/core.c b/drivers/media/platform/qcom/venus/core.c
>> index ce206b7097541..064120127cb86 100644
>> --- a/drivers/media/platform/qcom/venus/core.c
>> +++ b/drivers/media/platform/qcom/venus/core.c
>> @@ -554,6 +554,9 @@ static const struct venus_resources msm8916_res = {
>>  	.fwname = "qcom/venus-1.8/venus.mbn",
>>  };
>>  
>> +/*
>> + * https://git.codelinaro.org/clo/la/kernel/msm-4.4/-/blame/caf_migration/kernel.lnx.4.4.r38-rel/arch/arm/boot/dts/qcom/msm8996-v3.dtsi#L403-414
>> + */
>
> There is no need to add the link to downstream code in comments. Please remove them.

They are needed somewhere, to double check the values.
Otherwise, it's just voodoo programming.

If not in the code, then maybe in the commit message?

Since qcom doesn't publish datasheets, downstream code
is the best we've got.

Regards
Vikash Garodia May 7, 2024, 9:06 a.m. UTC | #2 
On 5/6/2024 9:27 PM, Bryan O'Donoghue wrote:
> On 06/05/2024 15:51, Marc Gonzalez wrote:
>> On 06/05/2024 16:43, Vikash Garodia wrote:
>>
>>> On 5/6/2024 7:17 PM, Marc Gonzalez wrote:
>>>
>>>> From: Pierre-Hugues Husson <phhusson@freebox.fr>
>>>>
>>>> Add the missing bits for msm8998 support.
>>>>
>>>> Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
>>>> Signed-off-by: Pierre-Hugues Husson <phhusson@freebox.fr>
>>>> Signed-off-by: Marc Gonzalez <mgonzalez@freebox.fr>
>>>> ---
>>>>   drivers/media/platform/qcom/venus/core.c | 48
>>>> ++++++++++++++++++++++++++++++++++++++++++++++++
>>>>   1 file changed, 48 insertions(+)
>>>>
>>>> diff --git a/drivers/media/platform/qcom/venus/core.c
>>>> b/drivers/media/platform/qcom/venus/core.c
>>>> index ce206b7097541..064120127cb86 100644
>>>> --- a/drivers/media/platform/qcom/venus/core.c
>>>> +++ b/drivers/media/platform/qcom/venus/core.c
>>>> @@ -554,6 +554,9 @@ static const struct venus_resources msm8916_res = {
>>>>       .fwname = "qcom/venus-1.8/venus.mbn",
>>>>   };
>>>>   +/*
>>>> + *
>>>> https://git.codelinaro.org/clo/la/kernel/msm-4.4/-/blame/caf_migration/kernel.lnx.4.4.r38-rel/arch/arm/boot/dts/qcom/msm8996-v3.dtsi#L403-414
>>>> + */
>>>
>>> There is no need to add the link to downstream code in comments. Please
>>> remove them.
>>
>> They are needed somewhere, to double check the values.
>> Otherwise, it's just voodoo programming.
>>
>> If not in the code, then maybe in the commit message?
>>
>> Since qcom doesn't publish datasheets, downstream code
>> is the best we've got.
>>
>> Regards
>>
> 
> Commit message is a good idea.
> 
> Do that.
I am good with this, incase you are waiting for a confirmation from my end.

Regards,
Vikash