diff mbox series

[08/18] media: venus: hfi_venus: Fix version checks in venus_halt_axi()

Message ID 20230228-topic-venus-v1-8-58c2c88384e9@linaro.org
State New
Headers
Series Venus QoL / maintainability fixes |

Commit Message

Konrad Dybcio Feb. 28, 2023, 3:24 p.m. UTC 
  Only IRIS2(_1) should enter the until-now-IS_V6() path and the
condition for skipping part of it should be IS_IRIS2_1 and not the
number of VPP pipes. Fix that.

Fixes: 4b0b6e147dc9 ("media: venus: hfi: Add 6xx AXI halt logic")
Fixes: 78d434ba8659 ("media: venus: hfi: Skip AON register programming for V6 1pipe")
Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
---
 drivers/media/platform/qcom/venus/hfi_venus.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Bryan O'Donoghue Feb. 28, 2023, 3:36 p.m. UTC | #1 
On 28/02/2023 15:24, Konrad Dybcio wrote:
> Only IRIS2(_1) should enter the until-now-IS_V6() path and the
> condition for skipping part of it should be IS_IRIS2_1 and not the
> number of VPP pipes. Fix that.
> 
> Fixes: 4b0b6e147dc9 ("media: venus: hfi: Add 6xx AXI halt logic")
> Fixes: 78d434ba8659 ("media: venus: hfi: Skip AON register programming for V6 1pipe")
> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
> ---
>   drivers/media/platform/qcom/venus/hfi_venus.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/platform/qcom/venus/hfi_venus.c b/drivers/media/platform/qcom/venus/hfi_venus.c
> index 4d785e53aa0b..0d137e070407 100644
> --- a/drivers/media/platform/qcom/venus/hfi_venus.c
> +++ b/drivers/media/platform/qcom/venus/hfi_venus.c
> @@ -550,10 +550,10 @@ static int venus_halt_axi(struct venus_hfi_device *hdev)
>   	u32 mask_val;
>   	int ret;
>   
> -	if (IS_V6(hdev->core)) {
> +	if (IS_IRIS2(hdev->core) || IS_IRIS2_1(hdev->core)) {
>   		writel(0x3, cpu_cs_base + CPU_CS_X2RPMH_V6);
>   
> -		if (hdev->core->res->num_vpp_pipes == 1)
> +		if (IS_IRIS2_1(hdev->core))
>   			goto skip_aon_mvp_noc;
>   
>   		writel(0x1, aon_base + AON_WRAPPER_MVP_NOC_LPI_CONTROL);
> 

If you want to denote these as fixes, you need your patch 02/18 as a 
depend...

---
bod
Konrad Dybcio Feb. 28, 2023, 4:01 p.m. UTC | #2 
On 28.02.2023 16:36, Bryan O'Donoghue wrote:
> On 28/02/2023 15:24, Konrad Dybcio wrote:
>> Only IRIS2(_1) should enter the until-now-IS_V6() path and the
>> condition for skipping part of it should be IS_IRIS2_1 and not the
>> number of VPP pipes. Fix that.
>>
>> Fixes: 4b0b6e147dc9 ("media: venus: hfi: Add 6xx AXI halt logic")
>> Fixes: 78d434ba8659 ("media: venus: hfi: Skip AON register programming for V6 1pipe")
>> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
>> ---
>>   drivers/media/platform/qcom/venus/hfi_venus.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/media/platform/qcom/venus/hfi_venus.c b/drivers/media/platform/qcom/venus/hfi_venus.c
>> index 4d785e53aa0b..0d137e070407 100644
>> --- a/drivers/media/platform/qcom/venus/hfi_venus.c
>> +++ b/drivers/media/platform/qcom/venus/hfi_venus.c
>> @@ -550,10 +550,10 @@ static int venus_halt_axi(struct venus_hfi_device *hdev)
>>       u32 mask_val;
>>       int ret;
>>   -    if (IS_V6(hdev->core)) {
>> +    if (IS_IRIS2(hdev->core) || IS_IRIS2_1(hdev->core)) {
>>           writel(0x3, cpu_cs_base + CPU_CS_X2RPMH_V6);
>>   -        if (hdev->core->res->num_vpp_pipes == 1)
>> +        if (IS_IRIS2_1(hdev->core))
>>               goto skip_aon_mvp_noc;
>>             writel(0x1, aon_base + AON_WRAPPER_MVP_NOC_LPI_CONTROL);
>>
> 
> If you want to denote these as fixes, you need your patch 02/18 as a depend...
The main purpose of the Fixes tag is to mark commits that fix bugs in
existing code and it only assists in autoselecting stable patches.
Backporting this makes little sense, as we only support IRIS2 (8250)
and IRIS2_1 (7280) HFI6 platforms and new additions won't be backported.

Konrad
> 
> ---
> bod
Dikshita Agarwal March 2, 2023, 12:35 p.m. UTC | #3 
On 2/28/2023 9:31 PM, Konrad Dybcio wrote:
>
> On 28.02.2023 16:36, Bryan O'Donoghue wrote:
>> On 28/02/2023 15:24, Konrad Dybcio wrote:
>>> Only IRIS2(_1) should enter the until-now-IS_V6() path and the
>>> condition for skipping part of it should be IS_IRIS2_1 and not the
>>> number of VPP pipes. Fix that.
>>>
>>> Fixes: 4b0b6e147dc9 ("media: venus: hfi: Add 6xx AXI halt logic")
>>> Fixes: 78d434ba8659 ("media: venus: hfi: Skip AON register programming for V6 1pipe")
>>> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
>>> ---
>>>    drivers/media/platform/qcom/venus/hfi_venus.c | 4 ++--
>>>    1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/media/platform/qcom/venus/hfi_venus.c b/drivers/media/platform/qcom/venus/hfi_venus.c
>>> index 4d785e53aa0b..0d137e070407 100644
>>> --- a/drivers/media/platform/qcom/venus/hfi_venus.c
>>> +++ b/drivers/media/platform/qcom/venus/hfi_venus.c
>>> @@ -550,10 +550,10 @@ static int venus_halt_axi(struct venus_hfi_device *hdev)
>>>        u32 mask_val;
>>>        int ret;
>>>    -    if (IS_V6(hdev->core)) {
>>> +    if (IS_IRIS2(hdev->core) || IS_IRIS2_1(hdev->core)) {
>>>            writel(0x3, cpu_cs_base + CPU_CS_X2RPMH_V6);
>>>    -        if (hdev->core->res->num_vpp_pipes == 1)
>>> +        if (IS_IRIS2_1(hdev->core))
>>>                goto skip_aon_mvp_noc;
>>>              writel(0x1, aon_base + AON_WRAPPER_MVP_NOC_LPI_CONTROL);
>>>
>> If you want to denote these as fixes, you need your patch 02/18 as a depend...
> The main purpose of the Fixes tag is to mark commits that fix bugs in
> existing code and it only assists in autoselecting stable patches.
> Backporting this makes little sense, as we only support IRIS2 (8250)
> and IRIS2_1 (7280) HFI6 platforms and new additions won't be backported.
>
> Konrad

IRIS2_1 is nothing but IRIS2 with 1 VPP pipe (just a downstream notation 
to differentiate between two IRIS2 based chips).

So having the num_vpp_pipes check was fine, nothing wrong with that.

but since now you are introducing new VPU based check, it can be 
replaced with IRIS2_1 check.

Thanks,

Dikshita

>> ---
>> bod
diff mbox series

Patch

diff --git a/drivers/media/platform/qcom/venus/hfi_venus.c b/drivers/media/platform/qcom/venus/hfi_venus.c
index 4d785e53aa0b..0d137e070407 100644
--- a/drivers/media/platform/qcom/venus/hfi_venus.c
+++ b/drivers/media/platform/qcom/venus/hfi_venus.c
@@ -550,10 +550,10 @@  static int venus_halt_axi(struct venus_hfi_device *hdev)
 	u32 mask_val;
 	int ret;
 
-	if (IS_V6(hdev->core)) {
+	if (IS_IRIS2(hdev->core) || IS_IRIS2_1(hdev->core)) {
 		writel(0x3, cpu_cs_base + CPU_CS_X2RPMH_V6);
 
-		if (hdev->core->res->num_vpp_pipes == 1)
+		if (IS_IRIS2_1(hdev->core))
 			goto skip_aon_mvp_noc;
 
 		writel(0x1, aon_base + AON_WRAPPER_MVP_NOC_LPI_CONTROL);