diff mbox series

[v2,03/18] media: venus: Remap bufreq fields on HFI6XX

Message ID 20230228-topic-venus-v2-3-d95d14949c79@linaro.org
State New
Headers
Series Venus QoL / maintainability fixes |

Commit Message

Konrad Dybcio May 4, 2023, 8 a.m. UTC 
  Similarly to HFI4XX, the fields are remapped on 6XX as well. Fix it.

Cc: stable@vger.kernel.org # v5.12+
Fixes: 7ed9e0b3393c ("media: venus: hfi, vdec: v6 Add IS_V6() to existing IS_V4() if locations")
Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
---
 drivers/media/platform/qcom/venus/hfi_helper.h | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

Comments

Vikash Garodia May 5, 2023, 12:38 p.m. UTC | #1 
On 5/4/2023 1:30 PM, Konrad Dybcio wrote:
> Similarly to HFI4XX, the fields are remapped on 6XX as well. Fix it.
>
> Cc: stable@vger.kernel.org # v5.12+
> Fixes: 7ed9e0b3393c ("media: venus: hfi, vdec: v6 Add IS_V6() to existing IS_V4() if locations")
> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
> ---
>   drivers/media/platform/qcom/venus/hfi_helper.h | 9 ++++++---
>   1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/media/platform/qcom/venus/hfi_helper.h b/drivers/media/platform/qcom/venus/hfi_helper.h
> index 105792a68060..e0c8f15644df 100644
> --- a/drivers/media/platform/qcom/venus/hfi_helper.h
> +++ b/drivers/media/platform/qcom/venus/hfi_helper.h
> @@ -1170,11 +1170,14 @@ struct hfi_buffer_display_hold_count_actual {
>   
>   /* HFI 4XX reorder the fields, use these macros */
>   #define HFI_BUFREQ_HOLD_COUNT(bufreq, ver)	\
> -	((ver) == HFI_VERSION_4XX ? 0 : (bufreq)->hold_count)
> +	((ver) == HFI_VERSION_4XX || (ver) == HFI_VERSION_6XX \
> +	? 0 : (bufreq)->hold_count)
>   #define HFI_BUFREQ_COUNT_MIN(bufreq, ver)	\
> -	((ver) == HFI_VERSION_4XX ? (bufreq)->hold_count : (bufreq)->count_min)
> +	((ver) == HFI_VERSION_4XX || (ver) == HFI_VERSION_6XX \
> +	? (bufreq)->hold_count : (bufreq)->count_min)
>   #define HFI_BUFREQ_COUNT_MIN_HOST(bufreq, ver)	\
> -	((ver) == HFI_VERSION_4XX ? (bufreq)->count_min : 0)
> +	((ver) == HFI_VERSION_4XX || (ver) == HFI_VERSION_6XX \
> +	? (bufreq)->count_min : 0)

This patch is not correct. The existing code handles the disparity of 
buffer requirement payload received from firmware.

Its applicable only for V4.

For V6, driver does not rely on firmware to get the buffer requirement. 
Refer the buffer platform code for more details.

-Vikash

>   struct hfi_buffer_requirements {
>   	u32 type;
>
Konrad Dybcio May 5, 2023, 7:02 p.m. UTC | #2 
On 5.05.2023 14:38, Vikash Garodia wrote:
> On 5/4/2023 1:30 PM, Konrad Dybcio wrote:
>> Similarly to HFI4XX, the fields are remapped on 6XX as well. Fix it.
>>
>> Cc: stable@vger.kernel.org # v5.12+
>> Fixes: 7ed9e0b3393c ("media: venus: hfi, vdec: v6 Add IS_V6() to existing IS_V4() if locations")
>> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
>> ---
>>   drivers/media/platform/qcom/venus/hfi_helper.h | 9 ++++++---
>>   1 file changed, 6 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/media/platform/qcom/venus/hfi_helper.h b/drivers/media/platform/qcom/venus/hfi_helper.h
>> index 105792a68060..e0c8f15644df 100644
>> --- a/drivers/media/platform/qcom/venus/hfi_helper.h
>> +++ b/drivers/media/platform/qcom/venus/hfi_helper.h
>> @@ -1170,11 +1170,14 @@ struct hfi_buffer_display_hold_count_actual {
>>     /* HFI 4XX reorder the fields, use these macros */
>>   #define HFI_BUFREQ_HOLD_COUNT(bufreq, ver)    \
>> -    ((ver) == HFI_VERSION_4XX ? 0 : (bufreq)->hold_count)
>> +    ((ver) == HFI_VERSION_4XX || (ver) == HFI_VERSION_6XX \
>> +    ? 0 : (bufreq)->hold_count)
>>   #define HFI_BUFREQ_COUNT_MIN(bufreq, ver)    \
>> -    ((ver) == HFI_VERSION_4XX ? (bufreq)->hold_count : (bufreq)->count_min)
>> +    ((ver) == HFI_VERSION_4XX || (ver) == HFI_VERSION_6XX \
>> +    ? (bufreq)->hold_count : (bufreq)->count_min)
>>   #define HFI_BUFREQ_COUNT_MIN_HOST(bufreq, ver)    \
>> -    ((ver) == HFI_VERSION_4XX ? (bufreq)->count_min : 0)
>> +    ((ver) == HFI_VERSION_4XX || (ver) == HFI_VERSION_6XX \
>> +    ? (bufreq)->count_min : 0)
> 
> This patch is not correct. The existing code handles the disparity of buffer requirement payload received from firmware.
> 
> Its applicable only for V4.
> 
> For V6, driver does not rely on firmware to get the buffer requirement. Refer the buffer platform code for more details.
OK right I can see, downstream has a condition for IRIS2/IRIS2_1,
thanks for pointing this out!

Konrad
> 
> -Vikash
> 
>>   struct hfi_buffer_requirements {
>>       u32 type;
>>
diff mbox series

Patch

diff --git a/drivers/media/platform/qcom/venus/hfi_helper.h b/drivers/media/platform/qcom/venus/hfi_helper.h
index 105792a68060..e0c8f15644df 100644
--- a/drivers/media/platform/qcom/venus/hfi_helper.h
+++ b/drivers/media/platform/qcom/venus/hfi_helper.h
@@ -1170,11 +1170,14 @@  struct hfi_buffer_display_hold_count_actual {
 
 /* HFI 4XX reorder the fields, use these macros */
 #define HFI_BUFREQ_HOLD_COUNT(bufreq, ver)	\
-	((ver) == HFI_VERSION_4XX ? 0 : (bufreq)->hold_count)
+	((ver) == HFI_VERSION_4XX || (ver) == HFI_VERSION_6XX \
+	? 0 : (bufreq)->hold_count)
 #define HFI_BUFREQ_COUNT_MIN(bufreq, ver)	\
-	((ver) == HFI_VERSION_4XX ? (bufreq)->hold_count : (bufreq)->count_min)
+	((ver) == HFI_VERSION_4XX || (ver) == HFI_VERSION_6XX \
+	? (bufreq)->hold_count : (bufreq)->count_min)
 #define HFI_BUFREQ_COUNT_MIN_HOST(bufreq, ver)	\
-	((ver) == HFI_VERSION_4XX ? (bufreq)->count_min : 0)
+	((ver) == HFI_VERSION_4XX || (ver) == HFI_VERSION_6XX \
+	? (bufreq)->count_min : 0)
 
 struct hfi_buffer_requirements {
 	u32 type;