diff mbox series

[1/2] media: venus: core: Add sdm660 DT compatible and resource struct

Message ID 20210115185252.333562-2-angelogioacchino.delregno@somainline.org
State New
Headers show
Series SDM630/660 Venus hardware video decoder/encoder | expand

Commit Message

AngeloGioacchino Del Regno Jan. 15, 2021, 6:52 p.m. UTC
Add the SDM660 DT compatible and its resource structure, also
including support for the Venus pmdomains, in order to support
the Venus block in SDM630, SDM636, SDM660 and SDA variants.

This SoC features Venus 4.4 (HFI3XX), with one vcodec used for
both encoding and decoding, switched on through two GDSCs.
The core clock for this Venus chip is powered by the RPM VDD_CX
power domain.

Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@somainline.org>
---
 drivers/media/platform/qcom/venus/core.c | 66 ++++++++++++++++++++++++
 1 file changed, 66 insertions(+)

Comments

Stanimir Varbanov Jan. 18, 2021, 5:21 p.m. UTC | #1
Hi Angelo,

Thanks for the patch!

On 1/15/21 8:52 PM, AngeloGioacchino Del Regno wrote:
> Add the SDM660 DT compatible and its resource structure, also
> including support for the Venus pmdomains, in order to support
> the Venus block in SDM630, SDM636, SDM660 and SDA variants.
> 
> This SoC features Venus 4.4 (HFI3XX), with one vcodec used for
> both encoding and decoding, switched on through two GDSCs.
> The core clock for this Venus chip is powered by the RPM VDD_CX
> power domain.
> 
> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@somainline.org>
> ---
>  drivers/media/platform/qcom/venus/core.c | 66 ++++++++++++++++++++++++
>  1 file changed, 66 insertions(+)
> 
> diff --git a/drivers/media/platform/qcom/venus/core.c b/drivers/media/platform/qcom/venus/core.c
> index bdd293faaad0..83ca86a63241 100644
> --- a/drivers/media/platform/qcom/venus/core.c
> +++ b/drivers/media/platform/qcom/venus/core.c
> @@ -565,6 +565,71 @@ static const struct venus_resources sdm845_res_v2 = {
>  	.fwname = "qcom/venus-5.2/venus.mdt",
>  };
>  
> +static const struct freq_tbl sdm660_freq_table[] = {
> +	{ 0, 518400000 },
> +	{ 0, 441600000 },
> +	{ 0, 404000000 },
> +	{ 0, 320000000 },
> +	{ 0, 269330000 },
> +	{ 0, 133330000 },
> +};
> +
> +static const struct reg_val sdm660_reg_preset[] = {
> +	{ 0x80010, 0x001f001f },
> +	{ 0x80018, 0x00000156 },
> +	{ 0x8001C, 0x00000156 },
> +};
> +
> +static const struct bw_tbl sdm660_bw_table_enc[] = {
> +	{  979200,  1044000, 0, 2446336, 0 },	/* 4k UHD @ 30 */
> +	{  864000,   887000, 0, 2108416, 0 },	/* 720p @ 240 */
> +	{  489600,   666000, 0, 1207296, 0 },	/* 1080p @ 60 */
> +	{  432000,   578000, 0, 1058816, 0 },	/* 720p @ 120 */
> +	{  244800,   346000, 0,  616448, 0 },	/* 1080p @ 30 */
> +	{  216000,   293000, 0,  534528, 0 },	/* 720p @ 60 */
> +	{  108000,   151000, 0,  271360, 0 },	/* 720p @ 30 */
> +};
> +
> +static const struct bw_tbl sdm660_bw_table_dec[] = {
> +	{  979200,  2365000, 0, 1892000, 0 },	/* 4k UHD @ 30 */
> +	{  864000,  1978000, 0, 1554000, 0 },	/* 720p @ 240 */
> +	{  489600,  1133000, 0,  895000, 0 },	/* 1080p @ 60 */
> +	{  432000,   994000, 0,  781000, 0 },	/* 720p @ 120 */
> +	{  244800,   580000, 0,  460000, 0 },	/* 1080p @ 30 */
> +	{  216000,   501000, 0,  301000, 0 },	/* 720p @ 60 */
> +	{  108000,   255000, 0,  202000, 0 },	/* 720p @ 30 */
> +};
> +
> +static const struct venus_resources sdm660_res = {
> +	.freq_tbl = sdm660_freq_table,
> +	.freq_tbl_size = ARRAY_SIZE(sdm660_freq_table),
> +	.reg_tbl = sdm660_reg_preset,
> +	.reg_tbl_size = ARRAY_SIZE(sdm660_reg_preset),
> +	.bw_tbl_enc = sdm660_bw_table_enc,
> +	.bw_tbl_enc_size = ARRAY_SIZE(sdm660_bw_table_enc),
> +	.bw_tbl_dec = sdm660_bw_table_dec,
> +	.bw_tbl_dec_size = ARRAY_SIZE(sdm660_bw_table_dec),
> +	.clks = {"core", "iface", "bus_throttle", "bus" },
> +	.clks_num = 4,
> +	.vcodec0_clks = { "vcodec0_core" },
> +	.vcodec_clks_num = 1,
> +	.vcodec_pmdomains = { "venus", "vcodec0" },
> +	.vcodec_pmdomains_num = 2,
> +	.opp_pmdomain = (const char *[]) { "cx", NULL },
> +	.vcodec_num = 1,
> +	.max_load = 1036800,
> +	.hfi_version = HFI_VERSION_3XX,
> +	.vmem_id = VIDC_RESOURCE_NONE,
> +	.vmem_size = 0,
> +	.vmem_addr = 0,
> +	.cp_start = 0,
> +	.cp_size = 0x79000000,
> +	.cp_nonpixel_start = 0x1000000,
> +	.cp_nonpixel_size = 0x28000000,
> +	.dma_mask = 0xd9000000 - 1,
> +	.fwname = "qcom/venus-4.4/venus.mdt",

Did you try venus-4.2 firmware from linux-firmware tree [1] ?

> +};
> +
>  static const struct freq_tbl sc7180_freq_table[] = {
>  	{  0, 500000000 },
>  	{  0, 434000000 },
> @@ -613,6 +678,7 @@ static const struct venus_resources sc7180_res = {
>  static const struct of_device_id venus_dt_match[] = {
>  	{ .compatible = "qcom,msm8916-venus", .data = &msm8916_res, },
>  	{ .compatible = "qcom,msm8996-venus", .data = &msm8996_res, },
> +	{ .compatible = "qcom,sdm660-venus", .data = &sdm660_res, },
>  	{ .compatible = "qcom,sdm845-venus", .data = &sdm845_res, },
>  	{ .compatible = "qcom,sdm845-venus-v2", .data = &sdm845_res_v2, },
>  	{ .compatible = "qcom,sc7180-venus", .data = &sc7180_res, },
> 

Reviewed-by: Stanimir Varbanov <stanimir.varbanov@linaro.org>
Hans Verkuil Jan. 25, 2021, 10:40 a.m. UTC | #2
On 18/01/2021 18:45, AngeloGioacchino Del Regno wrote:
> Il 18/01/21 18:21, Stanimir Varbanov ha scritto:
>> Hi Angelo,
>>
>> Thanks for the patch!
>>
>> On 1/15/21 8:52 PM, AngeloGioacchino Del Regno wrote:
>>> Add the SDM660 DT compatible and its resource structure, also
>>> including support for the Venus pmdomains, in order to support
>>> the Venus block in SDM630, SDM636, SDM660 and SDA variants.
>>>
>>> This SoC features Venus 4.4 (HFI3XX), with one vcodec used for
>>> both encoding and decoding, switched on through two GDSCs.
>>> The core clock for this Venus chip is powered by the RPM VDD_CX
>>> power domain.
>>>
>>> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@somainline.org>
>>> ---
>>>   drivers/media/platform/qcom/venus/core.c | 66 ++++++++++++++++++++++++
>>>   1 file changed, 66 insertions(+)
>>>
>>> diff --git a/drivers/media/platform/qcom/venus/core.c b/drivers/media/platform/qcom/venus/core.c
>>> index bdd293faaad0..83ca86a63241 100644
>>> --- a/drivers/media/platform/qcom/venus/core.c
>>> +++ b/drivers/media/platform/qcom/venus/core.c
>>> @@ -565,6 +565,71 @@ static const struct venus_resources sdm845_res_v2 = {
>>>   	.fwname = "qcom/venus-5.2/venus.mdt",
>>>   };
>>>   
>>> +static const struct freq_tbl sdm660_freq_table[] = {
>>> +	{ 0, 518400000 },
>>> +	{ 0, 441600000 },
>>> +	{ 0, 404000000 },
>>> +	{ 0, 320000000 },
>>> +	{ 0, 269330000 },
>>> +	{ 0, 133330000 },
>>> +};
>>> +
>>> +static const struct reg_val sdm660_reg_preset[] = {
>>> +	{ 0x80010, 0x001f001f },
>>> +	{ 0x80018, 0x00000156 },
>>> +	{ 0x8001C, 0x00000156 },
>>> +};
>>> +
>>> +static const struct bw_tbl sdm660_bw_table_enc[] = {
>>> +	{  979200,  1044000, 0, 2446336, 0 },	/* 4k UHD @ 30 */
>>> +	{  864000,   887000, 0, 2108416, 0 },	/* 720p @ 240 */
>>> +	{  489600,   666000, 0, 1207296, 0 },	/* 1080p @ 60 */
>>> +	{  432000,   578000, 0, 1058816, 0 },	/* 720p @ 120 */
>>> +	{  244800,   346000, 0,  616448, 0 },	/* 1080p @ 30 */
>>> +	{  216000,   293000, 0,  534528, 0 },	/* 720p @ 60 */
>>> +	{  108000,   151000, 0,  271360, 0 },	/* 720p @ 30 */
>>> +};
>>> +
>>> +static const struct bw_tbl sdm660_bw_table_dec[] = {
>>> +	{  979200,  2365000, 0, 1892000, 0 },	/* 4k UHD @ 30 */
>>> +	{  864000,  1978000, 0, 1554000, 0 },	/* 720p @ 240 */
>>> +	{  489600,  1133000, 0,  895000, 0 },	/* 1080p @ 60 */
>>> +	{  432000,   994000, 0,  781000, 0 },	/* 720p @ 120 */
>>> +	{  244800,   580000, 0,  460000, 0 },	/* 1080p @ 30 */
>>> +	{  216000,   501000, 0,  301000, 0 },	/* 720p @ 60 */
>>> +	{  108000,   255000, 0,  202000, 0 },	/* 720p @ 30 */
>>> +};
>>> +
>>> +static const struct venus_resources sdm660_res = {
>>> +	.freq_tbl = sdm660_freq_table,
>>> +	.freq_tbl_size = ARRAY_SIZE(sdm660_freq_table),
>>> +	.reg_tbl = sdm660_reg_preset,
>>> +	.reg_tbl_size = ARRAY_SIZE(sdm660_reg_preset),
>>> +	.bw_tbl_enc = sdm660_bw_table_enc,
>>> +	.bw_tbl_enc_size = ARRAY_SIZE(sdm660_bw_table_enc),
>>> +	.bw_tbl_dec = sdm660_bw_table_dec,
>>> +	.bw_tbl_dec_size = ARRAY_SIZE(sdm660_bw_table_dec),
>>> +	.clks = {"core", "iface", "bus_throttle", "bus" },
>>> +	.clks_num = 4,
>>> +	.vcodec0_clks = { "vcodec0_core" },
>>> +	.vcodec_clks_num = 1,
>>> +	.vcodec_pmdomains = { "venus", "vcodec0" },
>>> +	.vcodec_pmdomains_num = 2,
>>> +	.opp_pmdomain = (const char *[]) { "cx", NULL },
>>> +	.vcodec_num = 1,
>>> +	.max_load = 1036800,
>>> +	.hfi_version = HFI_VERSION_3XX,
>>> +	.vmem_id = VIDC_RESOURCE_NONE,
>>> +	.vmem_size = 0,
>>> +	.vmem_addr = 0,
>>> +	.cp_start = 0,
>>> +	.cp_size = 0x79000000,
>>> +	.cp_nonpixel_start = 0x1000000,
>>> +	.cp_nonpixel_size = 0x28000000,
>>> +	.dma_mask = 0xd9000000 - 1,
>>> +	.fwname = "qcom/venus-4.4/venus.mdt",
>>
>> Did you try venus-4.2 firmware from linux-firmware tree [1] ?
>>
> 
> No I haven't.. and I can't... my Sony devices (but I think that this is
> a practice of all OEMs/ODMs) are using a Sony signed venus firmware, so
> I am totally limited to use the firmware that comes with the device.
> 
> Besides that, the version is still different so, even if I had any
> possibility to try that, I don't think that it would work anyway...

I'm a bit confused. "qcom/venus-4.4/venus.mdt" is the Sony signed FW?

This patch can't be merged unless there is a corresponding firmware available
in linux-firmware. Is the current 4.2 firmware in linux-firmware signed by
Qualcomm? Can they provided 4.4 firmware as well?

I have no idea how this works for the venus driver, but I hope Stanimir does.

Regards,

	Hans

> 
>>> +};
>>> +
>>>   static const struct freq_tbl sc7180_freq_table[] = {
>>>   	{  0, 500000000 },
>>>   	{  0, 434000000 },
>>> @@ -613,6 +678,7 @@ static const struct venus_resources sc7180_res = {
>>>   static const struct of_device_id venus_dt_match[] = {
>>>   	{ .compatible = "qcom,msm8916-venus", .data = &msm8916_res, },
>>>   	{ .compatible = "qcom,msm8996-venus", .data = &msm8996_res, },
>>> +	{ .compatible = "qcom,sdm660-venus", .data = &sdm660_res, },
>>>   	{ .compatible = "qcom,sdm845-venus", .data = &sdm845_res, },
>>>   	{ .compatible = "qcom,sdm845-venus-v2", .data = &sdm845_res_v2, },
>>>   	{ .compatible = "qcom,sc7180-venus", .data = &sc7180_res, },
>>>
>>
>> Reviewed-by: Stanimir Varbanov <stanimir.varbanov@linaro.org>
>>
> 
> Thank you!
> - Angelo
>
Hans Verkuil Jan. 25, 2021, 3:01 p.m. UTC | #3
On 25/01/2021 15:51, AngeloGioacchino Del Regno wrote:
> Il 25/01/21 11:40, Hans Verkuil ha scritto:

>> On 18/01/2021 18:45, AngeloGioacchino Del Regno wrote:

>>> Il 18/01/21 18:21, Stanimir Varbanov ha scritto:

>>>> Hi Angelo,

>>>>

>>>> Thanks for the patch!

>>>>

>>>> On 1/15/21 8:52 PM, AngeloGioacchino Del Regno wrote:

>>>>> Add the SDM660 DT compatible and its resource structure, also

>>>>> including support for the Venus pmdomains, in order to support

>>>>> the Venus block in SDM630, SDM636, SDM660 and SDA variants.

>>>>>

>>>>> This SoC features Venus 4.4 (HFI3XX), with one vcodec used for

>>>>> both encoding and decoding, switched on through two GDSCs.

>>>>> The core clock for this Venus chip is powered by the RPM VDD_CX

>>>>> power domain.

>>>>>

>>>>> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@somainline.org>

>>>>> ---

>>>>>    drivers/media/platform/qcom/venus/core.c | 66 ++++++++++++++++++++++++

>>>>>    1 file changed, 66 insertions(+)

>>>>>

>>>>> diff --git a/drivers/media/platform/qcom/venus/core.c b/drivers/media/platform/qcom/venus/core.c

>>>>> index bdd293faaad0..83ca86a63241 100644

>>>>> --- a/drivers/media/platform/qcom/venus/core.c

>>>>> +++ b/drivers/media/platform/qcom/venus/core.c

>>>>> @@ -565,6 +565,71 @@ static const struct venus_resources sdm845_res_v2 = {

>>>>>    	.fwname = "qcom/venus-5.2/venus.mdt",

>>>>>    };

>>>>>    

>>>>> +static const struct freq_tbl sdm660_freq_table[] = {

>>>>> +	{ 0, 518400000 },

>>>>> +	{ 0, 441600000 },

>>>>> +	{ 0, 404000000 },

>>>>> +	{ 0, 320000000 },

>>>>> +	{ 0, 269330000 },

>>>>> +	{ 0, 133330000 },

>>>>> +};

>>>>> +

>>>>> +static const struct reg_val sdm660_reg_preset[] = {

>>>>> +	{ 0x80010, 0x001f001f },

>>>>> +	{ 0x80018, 0x00000156 },

>>>>> +	{ 0x8001C, 0x00000156 },

>>>>> +};

>>>>> +

>>>>> +static const struct bw_tbl sdm660_bw_table_enc[] = {

>>>>> +	{  979200,  1044000, 0, 2446336, 0 },	/* 4k UHD @ 30 */

>>>>> +	{  864000,   887000, 0, 2108416, 0 },	/* 720p @ 240 */

>>>>> +	{  489600,   666000, 0, 1207296, 0 },	/* 1080p @ 60 */

>>>>> +	{  432000,   578000, 0, 1058816, 0 },	/* 720p @ 120 */

>>>>> +	{  244800,   346000, 0,  616448, 0 },	/* 1080p @ 30 */

>>>>> +	{  216000,   293000, 0,  534528, 0 },	/* 720p @ 60 */

>>>>> +	{  108000,   151000, 0,  271360, 0 },	/* 720p @ 30 */

>>>>> +};

>>>>> +

>>>>> +static const struct bw_tbl sdm660_bw_table_dec[] = {

>>>>> +	{  979200,  2365000, 0, 1892000, 0 },	/* 4k UHD @ 30 */

>>>>> +	{  864000,  1978000, 0, 1554000, 0 },	/* 720p @ 240 */

>>>>> +	{  489600,  1133000, 0,  895000, 0 },	/* 1080p @ 60 */

>>>>> +	{  432000,   994000, 0,  781000, 0 },	/* 720p @ 120 */

>>>>> +	{  244800,   580000, 0,  460000, 0 },	/* 1080p @ 30 */

>>>>> +	{  216000,   501000, 0,  301000, 0 },	/* 720p @ 60 */

>>>>> +	{  108000,   255000, 0,  202000, 0 },	/* 720p @ 30 */

>>>>> +};

>>>>> +

>>>>> +static const struct venus_resources sdm660_res = {

>>>>> +	.freq_tbl = sdm660_freq_table,

>>>>> +	.freq_tbl_size = ARRAY_SIZE(sdm660_freq_table),

>>>>> +	.reg_tbl = sdm660_reg_preset,

>>>>> +	.reg_tbl_size = ARRAY_SIZE(sdm660_reg_preset),

>>>>> +	.bw_tbl_enc = sdm660_bw_table_enc,

>>>>> +	.bw_tbl_enc_size = ARRAY_SIZE(sdm660_bw_table_enc),

>>>>> +	.bw_tbl_dec = sdm660_bw_table_dec,

>>>>> +	.bw_tbl_dec_size = ARRAY_SIZE(sdm660_bw_table_dec),

>>>>> +	.clks = {"core", "iface", "bus_throttle", "bus" },

>>>>> +	.clks_num = 4,

>>>>> +	.vcodec0_clks = { "vcodec0_core" },

>>>>> +	.vcodec_clks_num = 1,

>>>>> +	.vcodec_pmdomains = { "venus", "vcodec0" },

>>>>> +	.vcodec_pmdomains_num = 2,

>>>>> +	.opp_pmdomain = (const char *[]) { "cx", NULL },

>>>>> +	.vcodec_num = 1,

>>>>> +	.max_load = 1036800,

>>>>> +	.hfi_version = HFI_VERSION_3XX,

>>>>> +	.vmem_id = VIDC_RESOURCE_NONE,

>>>>> +	.vmem_size = 0,

>>>>> +	.vmem_addr = 0,

>>>>> +	.cp_start = 0,

>>>>> +	.cp_size = 0x79000000,

>>>>> +	.cp_nonpixel_start = 0x1000000,

>>>>> +	.cp_nonpixel_size = 0x28000000,

>>>>> +	.dma_mask = 0xd9000000 - 1,

>>>>> +	.fwname = "qcom/venus-4.4/venus.mdt",

>>>>

>>>> Did you try venus-4.2 firmware from linux-firmware tree [1] ?

>>>>

>>>

>>> No I haven't.. and I can't... my Sony devices (but I think that this is

>>> a practice of all OEMs/ODMs) are using a Sony signed venus firmware, so

>>> I am totally limited to use the firmware that comes with the device.

>>>

>>> Besides that, the version is still different so, even if I had any

>>> possibility to try that, I don't think that it would work anyway...

> 

> Hello!

> 

>>

>> I'm a bit confused. "qcom/venus-4.4/venus.mdt" is the Sony signed FW?

>>

> 

> In my case it is, but this follows the generic firmware path as was done

> for all the other Venus firmwares, so my code is not pointing at Sony

> specific things, but just generic ones.

> 

> Every Qualcomm-powered consumer device (smartphones, tablets etc) have

> got a double sigcheck: one for qcom, one for OEM specific and most of

> the times the TZ is configured to accept only firmwares that also have

> the OEM signature.

> 

> This is not true for all the firmwares - for example, Adreno has this

> mechanism only for the ZAP part - but unfortunately I'm not aware of

> any consumer device accepting a Venus firmware with the "generic"

> Qualcomm signature only (so - without the OEM signature).

> 

> Short answer:

> 1. qcom/venus-4.4/venus.mdt is a generic firmware for Venus


So can this FW be made available in the linux-firmware repo? Stanimir?

> 2. 99% of the people needs a different firmware for signature issues

> 

> 

>> This patch can't be merged unless there is a corresponding firmware available

>> in linux-firmware. Is the current 4.2 firmware in linux-firmware signed by

>> Qualcomm? Can they provided 4.4 firmware as well?

>>

> 

> If there is such issue, then maybe we should do "something" about it: I

> would then propose to remove all references to fwname and just get this

> done in DT, where every qcom board already specifies its own path for

> its own firmware.

> 

> In any case, the issue that you're raising here has been raised multiple

> times on LKML, I don't precisely remember, but I recall seeing this for

> something like 4 years (or even more) being raised every now and then...

> 

>> I have no idea how this works for the venus driver, but I hope Stanimir does.

>>

> 

> As far as I've understood, this driver just uses the firmware which

> path is hardcoded in fwname, even though at this point I would like

> to get an opinion from Stanimir.

> 

> Would you be ok if we start parsing firmware-name from DT for this

> driver? The flow would be something like:

> 

> Is firmware-name DT property present?

>     Yes -> Use FW path from firmware-name property

>     No  -> Use the FW path from the fwname field of struct

>            venus_resources

> 

> This is a common flow in at least freedreno and remoteproc (modem).


I would have no problem with this, but it is up to Stanimir to decide.

Regards,

	Hans

> 

> -- Angelo

> 

>> Regards,

>>

>> 	Hans

>>

>>>

>>>>> +};

>>>>> +

>>>>>    static const struct freq_tbl sc7180_freq_table[] = {

>>>>>    	{  0, 500000000 },

>>>>>    	{  0, 434000000 },

>>>>> @@ -613,6 +678,7 @@ static const struct venus_resources sc7180_res = {

>>>>>    static const struct of_device_id venus_dt_match[] = {

>>>>>    	{ .compatible = "qcom,msm8916-venus", .data = &msm8916_res, },

>>>>>    	{ .compatible = "qcom,msm8996-venus", .data = &msm8996_res, },

>>>>> +	{ .compatible = "qcom,sdm660-venus", .data = &sdm660_res, },

>>>>>    	{ .compatible = "qcom,sdm845-venus", .data = &sdm845_res, },

>>>>>    	{ .compatible = "qcom,sdm845-venus-v2", .data = &sdm845_res_v2, },

>>>>>    	{ .compatible = "qcom,sc7180-venus", .data = &sc7180_res, },

>>>>>

>>>>

>>>> Reviewed-by: Stanimir Varbanov <stanimir.varbanov@linaro.org>

>>>>

>>>

>>> Thank you!

>>> - Angelo

>>>

>>

>
Bjorn Andersson Jan. 25, 2021, 3:18 p.m. UTC | #4
On Mon 25 Jan 08:51 CST 2021, AngeloGioacchino Del Regno wrote:

> Il 25/01/21 11:40, Hans Verkuil ha scritto:

> > On 18/01/2021 18:45, AngeloGioacchino Del Regno wrote:

> > > Il 18/01/21 18:21, Stanimir Varbanov ha scritto:

> > > > > diff --git a/drivers/media/platform/qcom/venus/core.c b/drivers/media/platform/qcom/venus/core.c

[..]
> > > > > +	.fwname = "qcom/venus-4.4/venus.mdt",

[..]
> > This patch can't be merged unless there is a corresponding firmware available

> > in linux-firmware. Is the current 4.2 firmware in linux-firmware signed by

> > Qualcomm? Can they provided 4.4 firmware as well?

> > 

> 

> If there is such issue, then maybe we should do "something" about it: I

> would then propose to remove all references to fwname and just get this

> done in DT, where every qcom board already specifies its own path for

> its own firmware.

> 


We have the same problem with production devices on e.g. SDM845, where
the firmware referenced by fw_name and present in linux-firmware won't
work on any real devices.

As such, providing means for specifying the firmware name in DT would be
a very reasonable thing, and in line with how we handle this in other
subsystems (using the firmware-name property, containing the full
relative path).

Regards,
Bjorn
Stanimir Varbanov Jan. 26, 2021, 8:44 a.m. UTC | #5
On 1/25/21 5:01 PM, Hans Verkuil wrote:
> On 25/01/2021 15:51, AngeloGioacchino Del Regno wrote:

>> Il 25/01/21 11:40, Hans Verkuil ha scritto:

>>> On 18/01/2021 18:45, AngeloGioacchino Del Regno wrote:

>>>> Il 18/01/21 18:21, Stanimir Varbanov ha scritto:

>>>>> Hi Angelo,

>>>>>

>>>>> Thanks for the patch!

>>>>>

>>>>> On 1/15/21 8:52 PM, AngeloGioacchino Del Regno wrote:

>>>>>> Add the SDM660 DT compatible and its resource structure, also

>>>>>> including support for the Venus pmdomains, in order to support

>>>>>> the Venus block in SDM630, SDM636, SDM660 and SDA variants.

>>>>>>

>>>>>> This SoC features Venus 4.4 (HFI3XX), with one vcodec used for

>>>>>> both encoding and decoding, switched on through two GDSCs.

>>>>>> The core clock for this Venus chip is powered by the RPM VDD_CX

>>>>>> power domain.

>>>>>>

>>>>>> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@somainline.org>

>>>>>> ---

>>>>>>    drivers/media/platform/qcom/venus/core.c | 66 ++++++++++++++++++++++++

>>>>>>    1 file changed, 66 insertions(+)

>>>>>>

>>>>>> diff --git a/drivers/media/platform/qcom/venus/core.c b/drivers/media/platform/qcom/venus/core.c

>>>>>> index bdd293faaad0..83ca86a63241 100644

>>>>>> --- a/drivers/media/platform/qcom/venus/core.c

>>>>>> +++ b/drivers/media/platform/qcom/venus/core.c

>>>>>> @@ -565,6 +565,71 @@ static const struct venus_resources sdm845_res_v2 = {

>>>>>>    	.fwname = "qcom/venus-5.2/venus.mdt",

>>>>>>    };

>>>>>>    

>>>>>> +static const struct freq_tbl sdm660_freq_table[] = {

>>>>>> +	{ 0, 518400000 },

>>>>>> +	{ 0, 441600000 },

>>>>>> +	{ 0, 404000000 },

>>>>>> +	{ 0, 320000000 },

>>>>>> +	{ 0, 269330000 },

>>>>>> +	{ 0, 133330000 },

>>>>>> +};

>>>>>> +

>>>>>> +static const struct reg_val sdm660_reg_preset[] = {

>>>>>> +	{ 0x80010, 0x001f001f },

>>>>>> +	{ 0x80018, 0x00000156 },

>>>>>> +	{ 0x8001C, 0x00000156 },

>>>>>> +};

>>>>>> +

>>>>>> +static const struct bw_tbl sdm660_bw_table_enc[] = {

>>>>>> +	{  979200,  1044000, 0, 2446336, 0 },	/* 4k UHD @ 30 */

>>>>>> +	{  864000,   887000, 0, 2108416, 0 },	/* 720p @ 240 */

>>>>>> +	{  489600,   666000, 0, 1207296, 0 },	/* 1080p @ 60 */

>>>>>> +	{  432000,   578000, 0, 1058816, 0 },	/* 720p @ 120 */

>>>>>> +	{  244800,   346000, 0,  616448, 0 },	/* 1080p @ 30 */

>>>>>> +	{  216000,   293000, 0,  534528, 0 },	/* 720p @ 60 */

>>>>>> +	{  108000,   151000, 0,  271360, 0 },	/* 720p @ 30 */

>>>>>> +};

>>>>>> +

>>>>>> +static const struct bw_tbl sdm660_bw_table_dec[] = {

>>>>>> +	{  979200,  2365000, 0, 1892000, 0 },	/* 4k UHD @ 30 */

>>>>>> +	{  864000,  1978000, 0, 1554000, 0 },	/* 720p @ 240 */

>>>>>> +	{  489600,  1133000, 0,  895000, 0 },	/* 1080p @ 60 */

>>>>>> +	{  432000,   994000, 0,  781000, 0 },	/* 720p @ 120 */

>>>>>> +	{  244800,   580000, 0,  460000, 0 },	/* 1080p @ 30 */

>>>>>> +	{  216000,   501000, 0,  301000, 0 },	/* 720p @ 60 */

>>>>>> +	{  108000,   255000, 0,  202000, 0 },	/* 720p @ 30 */

>>>>>> +};

>>>>>> +

>>>>>> +static const struct venus_resources sdm660_res = {

>>>>>> +	.freq_tbl = sdm660_freq_table,

>>>>>> +	.freq_tbl_size = ARRAY_SIZE(sdm660_freq_table),

>>>>>> +	.reg_tbl = sdm660_reg_preset,

>>>>>> +	.reg_tbl_size = ARRAY_SIZE(sdm660_reg_preset),

>>>>>> +	.bw_tbl_enc = sdm660_bw_table_enc,

>>>>>> +	.bw_tbl_enc_size = ARRAY_SIZE(sdm660_bw_table_enc),

>>>>>> +	.bw_tbl_dec = sdm660_bw_table_dec,

>>>>>> +	.bw_tbl_dec_size = ARRAY_SIZE(sdm660_bw_table_dec),

>>>>>> +	.clks = {"core", "iface", "bus_throttle", "bus" },

>>>>>> +	.clks_num = 4,

>>>>>> +	.vcodec0_clks = { "vcodec0_core" },

>>>>>> +	.vcodec_clks_num = 1,

>>>>>> +	.vcodec_pmdomains = { "venus", "vcodec0" },

>>>>>> +	.vcodec_pmdomains_num = 2,

>>>>>> +	.opp_pmdomain = (const char *[]) { "cx", NULL },

>>>>>> +	.vcodec_num = 1,

>>>>>> +	.max_load = 1036800,

>>>>>> +	.hfi_version = HFI_VERSION_3XX,

>>>>>> +	.vmem_id = VIDC_RESOURCE_NONE,

>>>>>> +	.vmem_size = 0,

>>>>>> +	.vmem_addr = 0,

>>>>>> +	.cp_start = 0,

>>>>>> +	.cp_size = 0x79000000,

>>>>>> +	.cp_nonpixel_start = 0x1000000,

>>>>>> +	.cp_nonpixel_size = 0x28000000,

>>>>>> +	.dma_mask = 0xd9000000 - 1,

>>>>>> +	.fwname = "qcom/venus-4.4/venus.mdt",

>>>>>

>>>>> Did you try venus-4.2 firmware from linux-firmware tree [1] ?

>>>>>

>>>>

>>>> No I haven't.. and I can't... my Sony devices (but I think that this is

>>>> a practice of all OEMs/ODMs) are using a Sony signed venus firmware, so

>>>> I am totally limited to use the firmware that comes with the device.

>>>>

>>>> Besides that, the version is still different so, even if I had any

>>>> possibility to try that, I don't think that it would work anyway...

>>

>> Hello!

>>

>>>

>>> I'm a bit confused. "qcom/venus-4.4/venus.mdt" is the Sony signed FW?

>>>

>>

>> In my case it is, but this follows the generic firmware path as was done

>> for all the other Venus firmwares, so my code is not pointing at Sony

>> specific things, but just generic ones.

>>

>> Every Qualcomm-powered consumer device (smartphones, tablets etc) have

>> got a double sigcheck: one for qcom, one for OEM specific and most of

>> the times the TZ is configured to accept only firmwares that also have

>> the OEM signature.

>>

>> This is not true for all the firmwares - for example, Adreno has this

>> mechanism only for the ZAP part - but unfortunately I'm not aware of

>> any consumer device accepting a Venus firmware with the "generic"

>> Qualcomm signature only (so - without the OEM signature).

>>

>> Short answer:

>> 1. qcom/venus-4.4/venus.mdt is a generic firmware for Venus

> 

> So can this FW be made available in the linux-firmware repo? Stanimir?

> 

>> 2. 99% of the people needs a different firmware for signature issues

>>

>>

>>> This patch can't be merged unless there is a corresponding firmware available

>>> in linux-firmware. Is the current 4.2 firmware in linux-firmware signed by

>>> Qualcomm? Can they provided 4.4 firmware as well?

>>>

>>

>> If there is such issue, then maybe we should do "something" about it: I

>> would then propose to remove all references to fwname and just get this

>> done in DT, where every qcom board already specifies its own path for

>> its own firmware.

>>

>> In any case, the issue that you're raising here has been raised multiple

>> times on LKML, I don't precisely remember, but I recall seeing this for

>> something like 4 years (or even more) being raised every now and then...

>>

>>> I have no idea how this works for the venus driver, but I hope Stanimir does.

>>>

>>

>> As far as I've understood, this driver just uses the firmware which

>> path is hardcoded in fwname, even though at this point I would like

>> to get an opinion from Stanimir.

>>

>> Would you be ok if we start parsing firmware-name from DT for this

>> driver? The flow would be something like:

>>

>> Is firmware-name DT property present?

>>     Yes -> Use FW path from firmware-name property

>>     No  -> Use the FW path from the fwname field of struct

>>            venus_resources

>>

>> This is a common flow in at least freedreno and remoteproc (modem).

> 

> I would have no problem with this, but it is up to Stanimir to decide.

> 


I sent a patch for that. Thanks for raising this issue.

-- 
regards,
Stan
diff mbox series

Patch

diff --git a/drivers/media/platform/qcom/venus/core.c b/drivers/media/platform/qcom/venus/core.c
index bdd293faaad0..83ca86a63241 100644
--- a/drivers/media/platform/qcom/venus/core.c
+++ b/drivers/media/platform/qcom/venus/core.c
@@ -565,6 +565,71 @@  static const struct venus_resources sdm845_res_v2 = {
 	.fwname = "qcom/venus-5.2/venus.mdt",
 };
 
+static const struct freq_tbl sdm660_freq_table[] = {
+	{ 0, 518400000 },
+	{ 0, 441600000 },
+	{ 0, 404000000 },
+	{ 0, 320000000 },
+	{ 0, 269330000 },
+	{ 0, 133330000 },
+};
+
+static const struct reg_val sdm660_reg_preset[] = {
+	{ 0x80010, 0x001f001f },
+	{ 0x80018, 0x00000156 },
+	{ 0x8001C, 0x00000156 },
+};
+
+static const struct bw_tbl sdm660_bw_table_enc[] = {
+	{  979200,  1044000, 0, 2446336, 0 },	/* 4k UHD @ 30 */
+	{  864000,   887000, 0, 2108416, 0 },	/* 720p @ 240 */
+	{  489600,   666000, 0, 1207296, 0 },	/* 1080p @ 60 */
+	{  432000,   578000, 0, 1058816, 0 },	/* 720p @ 120 */
+	{  244800,   346000, 0,  616448, 0 },	/* 1080p @ 30 */
+	{  216000,   293000, 0,  534528, 0 },	/* 720p @ 60 */
+	{  108000,   151000, 0,  271360, 0 },	/* 720p @ 30 */
+};
+
+static const struct bw_tbl sdm660_bw_table_dec[] = {
+	{  979200,  2365000, 0, 1892000, 0 },	/* 4k UHD @ 30 */
+	{  864000,  1978000, 0, 1554000, 0 },	/* 720p @ 240 */
+	{  489600,  1133000, 0,  895000, 0 },	/* 1080p @ 60 */
+	{  432000,   994000, 0,  781000, 0 },	/* 720p @ 120 */
+	{  244800,   580000, 0,  460000, 0 },	/* 1080p @ 30 */
+	{  216000,   501000, 0,  301000, 0 },	/* 720p @ 60 */
+	{  108000,   255000, 0,  202000, 0 },	/* 720p @ 30 */
+};
+
+static const struct venus_resources sdm660_res = {
+	.freq_tbl = sdm660_freq_table,
+	.freq_tbl_size = ARRAY_SIZE(sdm660_freq_table),
+	.reg_tbl = sdm660_reg_preset,
+	.reg_tbl_size = ARRAY_SIZE(sdm660_reg_preset),
+	.bw_tbl_enc = sdm660_bw_table_enc,
+	.bw_tbl_enc_size = ARRAY_SIZE(sdm660_bw_table_enc),
+	.bw_tbl_dec = sdm660_bw_table_dec,
+	.bw_tbl_dec_size = ARRAY_SIZE(sdm660_bw_table_dec),
+	.clks = {"core", "iface", "bus_throttle", "bus" },
+	.clks_num = 4,
+	.vcodec0_clks = { "vcodec0_core" },
+	.vcodec_clks_num = 1,
+	.vcodec_pmdomains = { "venus", "vcodec0" },
+	.vcodec_pmdomains_num = 2,
+	.opp_pmdomain = (const char *[]) { "cx", NULL },
+	.vcodec_num = 1,
+	.max_load = 1036800,
+	.hfi_version = HFI_VERSION_3XX,
+	.vmem_id = VIDC_RESOURCE_NONE,
+	.vmem_size = 0,
+	.vmem_addr = 0,
+	.cp_start = 0,
+	.cp_size = 0x79000000,
+	.cp_nonpixel_start = 0x1000000,
+	.cp_nonpixel_size = 0x28000000,
+	.dma_mask = 0xd9000000 - 1,
+	.fwname = "qcom/venus-4.4/venus.mdt",
+};
+
 static const struct freq_tbl sc7180_freq_table[] = {
 	{  0, 500000000 },
 	{  0, 434000000 },
@@ -613,6 +678,7 @@  static const struct venus_resources sc7180_res = {
 static const struct of_device_id venus_dt_match[] = {
 	{ .compatible = "qcom,msm8916-venus", .data = &msm8916_res, },
 	{ .compatible = "qcom,msm8996-venus", .data = &msm8996_res, },
+	{ .compatible = "qcom,sdm660-venus", .data = &sdm660_res, },
 	{ .compatible = "qcom,sdm845-venus", .data = &sdm845_res, },
 	{ .compatible = "qcom,sdm845-venus-v2", .data = &sdm845_res_v2, },
 	{ .compatible = "qcom,sc7180-venus", .data = &sc7180_res, },