diff mbox series

[2/3] venus: enable sufficient sequence change support for vp9

Message ID 1680589032-26046-3-git-send-email-quic_dikshita@quicinc.com (mailing list archive)
State Superseded
Headers show
Series fix decoder issues with firmware version check | expand

Commit Message

Dikshita Agarwal April 4, 2023, 6:17 a.m. UTC 
VP9 supports resolution change at interframe.
Currenlty, if sequence change is detected at interframe and
resources are sufficient, sequence change event is not raised
by firmware to driver until the next keyframe.
This change add the HFI to notify the sequence change in this
case to driver.

Signed-off-by: Dikshita Agarwal <quic_dikshita@quicinc.com>
Signed-off-by: Vikash Garodia <quic_vgarodia@quicinc.com>
Signed-off-by: Viswanath Boma <quic_vboma@quicinc.com>
Tested-by: Nathan Hebert <nhebert@chromium.org>
---
 drivers/media/platform/qcom/venus/hfi_cmds.c   | 1 +
 drivers/media/platform/qcom/venus/hfi_helper.h | 2 ++
 drivers/media/platform/qcom/venus/vdec.c       | 8 ++++++++
 3 files changed, 11 insertions(+)

Comments

Konrad Dybcio April 4, 2023, 6:22 p.m. UTC | #1 
On 4.04.2023 08:17, Dikshita Agarwal wrote:
> VP9 supports resolution change at interframe.
> Currenlty, if sequence change is detected at interframe and
> resources are sufficient, sequence change event is not raised
> by firmware to driver until the next keyframe.
> This change add the HFI to notify the sequence change in this
> case to driver.
> 
> Signed-off-by: Dikshita Agarwal <quic_dikshita@quicinc.com>
> Signed-off-by: Vikash Garodia <quic_vgarodia@quicinc.com>
> Signed-off-by: Viswanath Boma <quic_vboma@quicinc.com>
> Tested-by: Nathan Hebert <nhebert@chromium.org>
> ---
>  drivers/media/platform/qcom/venus/hfi_cmds.c   | 1 +
>  drivers/media/platform/qcom/venus/hfi_helper.h | 2 ++
>  drivers/media/platform/qcom/venus/vdec.c       | 8 ++++++++
>  3 files changed, 11 insertions(+)
> 
> diff --git a/drivers/media/platform/qcom/venus/hfi_cmds.c b/drivers/media/platform/qcom/venus/hfi_cmds.c
> index 930b743..e2539b5 100644
> --- a/drivers/media/platform/qcom/venus/hfi_cmds.c
> +++ b/drivers/media/platform/qcom/venus/hfi_cmds.c
> @@ -521,6 +521,7 @@ static int pkt_session_set_property_1x(struct hfi_session_set_property_pkt *pkt,
>  		pkt->shdr.hdr.size += sizeof(u32) + sizeof(*en);
>  		break;
>  	}
> +	case HFI_PROPERTY_PARAM_VDEC_ENABLE_SUFFICIENT_SEQCHANGE_EVENT:
>  	case HFI_PROPERTY_CONFIG_VDEC_POST_LOOP_DEBLOCKER: {
>  		struct hfi_enable *in = pdata;
>  		struct hfi_enable *en = prop_data;
> diff --git a/drivers/media/platform/qcom/venus/hfi_helper.h b/drivers/media/platform/qcom/venus/hfi_helper.h
> index d2d6719..20516b4 100644
> --- a/drivers/media/platform/qcom/venus/hfi_helper.h
> +++ b/drivers/media/platform/qcom/venus/hfi_helper.h
> @@ -469,6 +469,8 @@
>  #define HFI_PROPERTY_PARAM_VDEC_PIXEL_BITDEPTH			0x1003007
>  #define HFI_PROPERTY_PARAM_VDEC_PIC_STRUCT			0x1003009
>  #define HFI_PROPERTY_PARAM_VDEC_COLOUR_SPACE			0x100300a
> +#define HFI_PROPERTY_PARAM_VDEC_ENABLE_SUFFICIENT_SEQCHANGE_EVENT \
> +								0x0100300b
>  
>  /*
>   * HFI_PROPERTY_CONFIG_VDEC_COMMON_START
> diff --git a/drivers/media/platform/qcom/venus/vdec.c b/drivers/media/platform/qcom/venus/vdec.c
> index 4ceaba3..f0394b9 100644
> --- a/drivers/media/platform/qcom/venus/vdec.c
> +++ b/drivers/media/platform/qcom/venus/vdec.c
> @@ -671,6 +671,14 @@ static int vdec_set_properties(struct venus_inst *inst)
>  			return ret;
>  	}
>  
> +	/* Enabling sufficient sequence change support for VP9 */
> +	if (is_fw_rev_or_newer(inst->core, 5, 4, 51)) {
> +		ptype = HFI_PROPERTY_PARAM_VDEC_ENABLE_SUFFICIENT_SEQCHANGE_EVENT;
> +		ret = hfi_session_set_property(inst, ptype, &en);
> +		if (ret)
> +			return ret;
> +	}
Does it never have to be turned off? Or does it happen automatically
at session closure?

Konrad
> +
>  	ptype = HFI_PROPERTY_PARAM_VDEC_CONCEAL_COLOR;
>  	conceal = ctr->conceal_color & 0xffff;
>  	conceal |= ((ctr->conceal_color >> 16) & 0xffff) << 10;
Konrad Dybcio April 4, 2023, 7:05 p.m. UTC | #2 
On 4.04.2023 08:17, Dikshita Agarwal wrote:
> VP9 supports resolution change at interframe.
> Currenlty, if sequence change is detected at interframe and
> resources are sufficient, sequence change event is not raised
> by firmware to driver until the next keyframe.
> This change add the HFI to notify the sequence change in this
> case to driver.
> 
> Signed-off-by: Dikshita Agarwal <quic_dikshita@quicinc.com>
> Signed-off-by: Vikash Garodia <quic_vgarodia@quicinc.com>
> Signed-off-by: Viswanath Boma <quic_vboma@quicinc.com>
> Tested-by: Nathan Hebert <nhebert@chromium.org>
> ---
>  drivers/media/platform/qcom/venus/hfi_cmds.c   | 1 +
>  drivers/media/platform/qcom/venus/hfi_helper.h | 2 ++
>  drivers/media/platform/qcom/venus/vdec.c       | 8 ++++++++
>  3 files changed, 11 insertions(+)
> 
> diff --git a/drivers/media/platform/qcom/venus/hfi_cmds.c b/drivers/media/platform/qcom/venus/hfi_cmds.c
> index 930b743..e2539b5 100644
> --- a/drivers/media/platform/qcom/venus/hfi_cmds.c
> +++ b/drivers/media/platform/qcom/venus/hfi_cmds.c
> @@ -521,6 +521,7 @@ static int pkt_session_set_property_1x(struct hfi_session_set_property_pkt *pkt,
>  		pkt->shdr.hdr.size += sizeof(u32) + sizeof(*en);
>  		break;
>  	}
> +	case HFI_PROPERTY_PARAM_VDEC_ENABLE_SUFFICIENT_SEQCHANGE_EVENT:
>  	case HFI_PROPERTY_CONFIG_VDEC_POST_LOOP_DEBLOCKER: {
>  		struct hfi_enable *in = pdata;
>  		struct hfi_enable *en = prop_data;
> diff --git a/drivers/media/platform/qcom/venus/hfi_helper.h b/drivers/media/platform/qcom/venus/hfi_helper.h
> index d2d6719..20516b4 100644
> --- a/drivers/media/platform/qcom/venus/hfi_helper.h
> +++ b/drivers/media/platform/qcom/venus/hfi_helper.h
> @@ -469,6 +469,8 @@
>  #define HFI_PROPERTY_PARAM_VDEC_PIXEL_BITDEPTH			0x1003007
>  #define HFI_PROPERTY_PARAM_VDEC_PIC_STRUCT			0x1003009
>  #define HFI_PROPERTY_PARAM_VDEC_COLOUR_SPACE			0x100300a
> +#define HFI_PROPERTY_PARAM_VDEC_ENABLE_SUFFICIENT_SEQCHANGE_EVENT \
> +								0x0100300b
Also, nit: this one has a leading zero, whereas other properties don't

Konrad
>  
>  /*
>   * HFI_PROPERTY_CONFIG_VDEC_COMMON_START
> diff --git a/drivers/media/platform/qcom/venus/vdec.c b/drivers/media/platform/qcom/venus/vdec.c
> index 4ceaba3..f0394b9 100644
> --- a/drivers/media/platform/qcom/venus/vdec.c
> +++ b/drivers/media/platform/qcom/venus/vdec.c
> @@ -671,6 +671,14 @@ static int vdec_set_properties(struct venus_inst *inst)
>  			return ret;
>  	}
>  
> +	/* Enabling sufficient sequence change support for VP9 */
> +	if (is_fw_rev_or_newer(inst->core, 5, 4, 51)) {
> +		ptype = HFI_PROPERTY_PARAM_VDEC_ENABLE_SUFFICIENT_SEQCHANGE_EVENT;
> +		ret = hfi_session_set_property(inst, ptype, &en);
> +		if (ret)
> +			return ret;
> +	}
> +
>  	ptype = HFI_PROPERTY_PARAM_VDEC_CONCEAL_COLOR;
>  	conceal = ctr->conceal_color & 0xffff;
>  	conceal |= ((ctr->conceal_color >> 16) & 0xffff) << 10;
Dikshita Agarwal April 5, 2023, 5:46 a.m. UTC | #3 
On 4/4/2023 11:52 PM, Konrad Dybcio wrote:
>
> On 4.04.2023 08:17, Dikshita Agarwal wrote:
>> VP9 supports resolution change at interframe.
>> Currenlty, if sequence change is detected at interframe and
>> resources are sufficient, sequence change event is not raised
>> by firmware to driver until the next keyframe.
>> This change add the HFI to notify the sequence change in this
>> case to driver.
>>
>> Signed-off-by: Dikshita Agarwal <quic_dikshita@quicinc.com>
>> Signed-off-by: Vikash Garodia <quic_vgarodia@quicinc.com>
>> Signed-off-by: Viswanath Boma <quic_vboma@quicinc.com>
>> Tested-by: Nathan Hebert <nhebert@chromium.org>
>> ---
>>   drivers/media/platform/qcom/venus/hfi_cmds.c   | 1 +
>>   drivers/media/platform/qcom/venus/hfi_helper.h | 2 ++
>>   drivers/media/platform/qcom/venus/vdec.c       | 8 ++++++++
>>   3 files changed, 11 insertions(+)
>>
>> diff --git a/drivers/media/platform/qcom/venus/hfi_cmds.c b/drivers/media/platform/qcom/venus/hfi_cmds.c
>> index 930b743..e2539b5 100644
>> --- a/drivers/media/platform/qcom/venus/hfi_cmds.c
>> +++ b/drivers/media/platform/qcom/venus/hfi_cmds.c
>> @@ -521,6 +521,7 @@ static int pkt_session_set_property_1x(struct hfi_session_set_property_pkt *pkt,
>>   		pkt->shdr.hdr.size += sizeof(u32) + sizeof(*en);
>>   		break;
>>   	}
>> +	case HFI_PROPERTY_PARAM_VDEC_ENABLE_SUFFICIENT_SEQCHANGE_EVENT:
>>   	case HFI_PROPERTY_CONFIG_VDEC_POST_LOOP_DEBLOCKER: {
>>   		struct hfi_enable *in = pdata;
>>   		struct hfi_enable *en = prop_data;
>> diff --git a/drivers/media/platform/qcom/venus/hfi_helper.h b/drivers/media/platform/qcom/venus/hfi_helper.h
>> index d2d6719..20516b4 100644
>> --- a/drivers/media/platform/qcom/venus/hfi_helper.h
>> +++ b/drivers/media/platform/qcom/venus/hfi_helper.h
>> @@ -469,6 +469,8 @@
>>   #define HFI_PROPERTY_PARAM_VDEC_PIXEL_BITDEPTH			0x1003007
>>   #define HFI_PROPERTY_PARAM_VDEC_PIC_STRUCT			0x1003009
>>   #define HFI_PROPERTY_PARAM_VDEC_COLOUR_SPACE			0x100300a
>> +#define HFI_PROPERTY_PARAM_VDEC_ENABLE_SUFFICIENT_SEQCHANGE_EVENT \
>> +								0x0100300b
>>   
>>   /*
>>    * HFI_PROPERTY_CONFIG_VDEC_COMMON_START
>> diff --git a/drivers/media/platform/qcom/venus/vdec.c b/drivers/media/platform/qcom/venus/vdec.c
>> index 4ceaba3..f0394b9 100644
>> --- a/drivers/media/platform/qcom/venus/vdec.c
>> +++ b/drivers/media/platform/qcom/venus/vdec.c
>> @@ -671,6 +671,14 @@ static int vdec_set_properties(struct venus_inst *inst)
>>   			return ret;
>>   	}
>>   
>> +	/* Enabling sufficient sequence change support for VP9 */
>> +	if (is_fw_rev_or_newer(inst->core, 5, 4, 51)) {
>> +		ptype = HFI_PROPERTY_PARAM_VDEC_ENABLE_SUFFICIENT_SEQCHANGE_EVENT;
>> +		ret = hfi_session_set_property(inst, ptype, &en);
>> +		if (ret)
>> +			return ret;
>> +	}
> Does it never have to be turned off? Or does it happen automatically
> at session closure?
>
> Konrad

Any property set to FW is applied for entire video session and it 
doesn't need to change so

there is no need to turn it off or unset it.

Thanks,

Dikshita

>> +
>>   	ptype = HFI_PROPERTY_PARAM_VDEC_CONCEAL_COLOR;
>>   	conceal = ctr->conceal_color & 0xffff;
>>   	conceal |= ((ctr->conceal_color >> 16) & 0xffff) << 10;
Dikshita Agarwal April 5, 2023, 8:21 a.m. UTC | #4 
On 4/5/2023 12:35 AM, Konrad Dybcio wrote:
>
> On 4.04.2023 08:17, Dikshita Agarwal wrote:
>> VP9 supports resolution change at interframe.
>> Currenlty, if sequence change is detected at interframe and
>> resources are sufficient, sequence change event is not raised
>> by firmware to driver until the next keyframe.
>> This change add the HFI to notify the sequence change in this
>> case to driver.
>>
>> Signed-off-by: Dikshita Agarwal <quic_dikshita@quicinc.com>
>> Signed-off-by: Vikash Garodia <quic_vgarodia@quicinc.com>
>> Signed-off-by: Viswanath Boma <quic_vboma@quicinc.com>
>> Tested-by: Nathan Hebert <nhebert@chromium.org>
>> ---
>>   drivers/media/platform/qcom/venus/hfi_cmds.c   | 1 +
>>   drivers/media/platform/qcom/venus/hfi_helper.h | 2 ++
>>   drivers/media/platform/qcom/venus/vdec.c       | 8 ++++++++
>>   3 files changed, 11 insertions(+)
>>
>> diff --git a/drivers/media/platform/qcom/venus/hfi_cmds.c b/drivers/media/platform/qcom/venus/hfi_cmds.c
>> index 930b743..e2539b5 100644
>> --- a/drivers/media/platform/qcom/venus/hfi_cmds.c
>> +++ b/drivers/media/platform/qcom/venus/hfi_cmds.c
>> @@ -521,6 +521,7 @@ static int pkt_session_set_property_1x(struct hfi_session_set_property_pkt *pkt,
>>   		pkt->shdr.hdr.size += sizeof(u32) + sizeof(*en);
>>   		break;
>>   	}
>> +	case HFI_PROPERTY_PARAM_VDEC_ENABLE_SUFFICIENT_SEQCHANGE_EVENT:
>>   	case HFI_PROPERTY_CONFIG_VDEC_POST_LOOP_DEBLOCKER: {
>>   		struct hfi_enable *in = pdata;
>>   		struct hfi_enable *en = prop_data;
>> diff --git a/drivers/media/platform/qcom/venus/hfi_helper.h b/drivers/media/platform/qcom/venus/hfi_helper.h
>> index d2d6719..20516b4 100644
>> --- a/drivers/media/platform/qcom/venus/hfi_helper.h
>> +++ b/drivers/media/platform/qcom/venus/hfi_helper.h
>> @@ -469,6 +469,8 @@
>>   #define HFI_PROPERTY_PARAM_VDEC_PIXEL_BITDEPTH			0x1003007
>>   #define HFI_PROPERTY_PARAM_VDEC_PIC_STRUCT			0x1003009
>>   #define HFI_PROPERTY_PARAM_VDEC_COLOUR_SPACE			0x100300a
>> +#define HFI_PROPERTY_PARAM_VDEC_ENABLE_SUFFICIENT_SEQCHANGE_EVENT \
>> +								0x0100300b
> Also, nit: this one has a leading zero, whereas other properties don't
>
> Konrad

Sure, Will fix in next version.

Thanks,

Dikshita

>>   
>>   /*
>>    * HFI_PROPERTY_CONFIG_VDEC_COMMON_START
>> diff --git a/drivers/media/platform/qcom/venus/vdec.c b/drivers/media/platform/qcom/venus/vdec.c
>> index 4ceaba3..f0394b9 100644
>> --- a/drivers/media/platform/qcom/venus/vdec.c
>> +++ b/drivers/media/platform/qcom/venus/vdec.c
>> @@ -671,6 +671,14 @@ static int vdec_set_properties(struct venus_inst *inst)
>>   			return ret;
>>   	}
>>   
>> +	/* Enabling sufficient sequence change support for VP9 */
>> +	if (is_fw_rev_or_newer(inst->core, 5, 4, 51)) {
>> +		ptype = HFI_PROPERTY_PARAM_VDEC_ENABLE_SUFFICIENT_SEQCHANGE_EVENT;
>> +		ret = hfi_session_set_property(inst, ptype, &en);
>> +		if (ret)
>> +			return ret;
>> +	}
>> +
>>   	ptype = HFI_PROPERTY_PARAM_VDEC_CONCEAL_COLOR;
>>   	conceal = ctr->conceal_color & 0xffff;
>>   	conceal |= ((ctr->conceal_color >> 16) & 0xffff) << 10;
diff mbox series

Patch

diff --git a/drivers/media/platform/qcom/venus/hfi_cmds.c b/drivers/media/platform/qcom/venus/hfi_cmds.c
index 930b743..e2539b5 100644
--- a/drivers/media/platform/qcom/venus/hfi_cmds.c
+++ b/drivers/media/platform/qcom/venus/hfi_cmds.c
@@ -521,6 +521,7 @@  static int pkt_session_set_property_1x(struct hfi_session_set_property_pkt *pkt,
 		pkt->shdr.hdr.size += sizeof(u32) + sizeof(*en);
 		break;
 	}
+	case HFI_PROPERTY_PARAM_VDEC_ENABLE_SUFFICIENT_SEQCHANGE_EVENT:
 	case HFI_PROPERTY_CONFIG_VDEC_POST_LOOP_DEBLOCKER: {
 		struct hfi_enable *in = pdata;
 		struct hfi_enable *en = prop_data;
diff --git a/drivers/media/platform/qcom/venus/hfi_helper.h b/drivers/media/platform/qcom/venus/hfi_helper.h
index d2d6719..20516b4 100644
--- a/drivers/media/platform/qcom/venus/hfi_helper.h
+++ b/drivers/media/platform/qcom/venus/hfi_helper.h
@@ -469,6 +469,8 @@ 
 #define HFI_PROPERTY_PARAM_VDEC_PIXEL_BITDEPTH			0x1003007
 #define HFI_PROPERTY_PARAM_VDEC_PIC_STRUCT			0x1003009
 #define HFI_PROPERTY_PARAM_VDEC_COLOUR_SPACE			0x100300a
+#define HFI_PROPERTY_PARAM_VDEC_ENABLE_SUFFICIENT_SEQCHANGE_EVENT \
+								0x0100300b
 
 /*
  * HFI_PROPERTY_CONFIG_VDEC_COMMON_START
diff --git a/drivers/media/platform/qcom/venus/vdec.c b/drivers/media/platform/qcom/venus/vdec.c
index 4ceaba3..f0394b9 100644
--- a/drivers/media/platform/qcom/venus/vdec.c
+++ b/drivers/media/platform/qcom/venus/vdec.c
@@ -671,6 +671,14 @@  static int vdec_set_properties(struct venus_inst *inst)
 			return ret;
 	}
 
+	/* Enabling sufficient sequence change support for VP9 */
+	if (is_fw_rev_or_newer(inst->core, 5, 4, 51)) {
+		ptype = HFI_PROPERTY_PARAM_VDEC_ENABLE_SUFFICIENT_SEQCHANGE_EVENT;
+		ret = hfi_session_set_property(inst, ptype, &en);
+		if (ret)
+			return ret;
+	}
+
 	ptype = HFI_PROPERTY_PARAM_VDEC_CONCEAL_COLOR;
 	conceal = ctr->conceal_color & 0xffff;
 	conceal |= ((ctr->conceal_color >> 16) & 0xffff) << 10;